drm/atomic: Document history of drm_atomic_state

After some discussions on the mailing-list for an earlier revision of
the series, it was suggested to document the evolution of
drm_atomic_state and

drm/atomic: Document history of drm_atomic_state

After some discussions on the mailing-list for an earlier revision of
the series, it was suggested to document the evolution of
drm_atomic_state and its use by drivers to explain some of the confusion
one might still encounter when reading the framework code.

Suggested-by: Simona Vetter <[email protected]>
Link: https://lore.kernel.org/dri-devel/[email protected]/
Reviewed-by: Simona Vetter <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Maxime Ripard <[email protected]>

show more ...

drm/atomic: clarify the rules around drm_atomic_state->allow_modeset

msm is automagically upgrading normal commits to full modesets, and
that's a big no-no:

- for one this results in full on->off->

drm/atomic: clarify the rules around drm_atomic_state->allow_modeset

msm is automagically upgrading normal commits to full modesets, and
that's a big no-no:

- for one this results in full on->off->on transitions on all these
crtc, at least if you're using the usual helpers. Which seems to be
the case, and is breaking uapi

- further even if the ctm change itself would not result in flicker,
this can hide modesets for other reasons. Which again breaks the
uapi

v2: I forgot the case of adding unrelated crtc state. Add that case
and link to the existing kerneldoc explainers. This has come up in an
irc discussion with Manasi and Ville about intel's bigjoiner mode.
Also cc everyone involved in the msm irc discussion, more people
joined after I sent out v1.

v3: Wording polish from Pekka and Thomas

Acked-by: Pekka Paalanen <[email protected]>
Acked-by: Dmitry Baryshkov <[email protected]>
Cc: Maarten Lankhorst <[email protected]>
Cc: Maxime Ripard <[email protected]>
Cc: Thomas Zimmermann <[email protected]>
Cc: David Airlie <[email protected]>
Cc: Daniel Vetter <[email protected]>
Cc: Pekka Paalanen <[email protected]>
Cc: Rob Clark <[email protected]>
Cc: Simon Ser <[email protected]>
Cc: Manasi Navare <[email protected]>
Cc: Ville Syrjälä <[email protected]>
Cc: Abhinav Kumar <[email protected]>
Cc: Dmitry Baryshkov <[email protected]>
Signed-off-by: Simona Vetter <[email protected]>
Signed-off-by: Simona Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]

show more ...

drm/atomic: fix kerneldoc for fake_commit field

According to the context, the function description for fake_commit
should be "prevent the atomic states from being freed too early"

Signed-off-by: re

drm/atomic: fix kerneldoc for fake_commit field

According to the context, the function description for fake_commit
should be "prevent the atomic states from being freed too early"

Signed-off-by: renjun wang <[email protected]>
Reviewed-by: Thomas Zimmermann <[email protected]>
Signed-off-by: Thomas Zimmermann <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]

show more ...

drm/atomic: Make the drm_atomic_state documentation less ambiguous

The current documentation of drm_atomic_state says that it's the "global
state object". This is confusing since, while it does cont

drm/atomic: Make the drm_atomic_state documentation less ambiguous

The current documentation of drm_atomic_state says that it's the "global
state object". This is confusing since, while it does contain all the
objects affected by an update and their respective states, if an object
isn't affected by this update it won't be part of it.

Thus, it's not truly a "global state", unlike object state structures
that do contain the entire state of a given object.

Reviewed-by: Hamza Mahfooz <[email protected]>
Acked-by: Pekka Paalanen <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Maxime Ripard <[email protected]>

show more ...

drm/atomic: Rework the object doc a bit

Commits 63e83c1dba54 ("drm: Consolidate connector arrays in
drm_atomic_state"), b8b5342b699b ("drm: Consolidate plane arrays in
drm_atomic_state") and 5d943aa

drm/atomic: Rework the object doc a bit

Commits 63e83c1dba54 ("drm: Consolidate connector arrays in
drm_atomic_state"), b8b5342b699b ("drm: Consolidate plane arrays in
drm_atomic_state") and 5d943aa6c0d4 ("drm: Consolidate crtc arrays in
drm_atomic_state") moved the object pointer and their state pointer to
an intermediate structure storing both.

The CRTC commit didn't update the doc of the crtcs field to reflect
that, and the doc for the planes and connectors fields mention that they
are pointers to an array of structures with per-$OBJECT data.

The private_objs field was added later on by commit b430c27a7de3 ("drm:
Add driver-private objects to atomic state") reusing the same sentence
than the crtcs field, probably due to copy and paste.

While these fields are indeed pointers to an array, each item of that
array contain a pointer to the object structure affected by the update,
and its old and new state. There's no per-object data there, and there's
more than just a pointer to the objects.

Let's rephrase those fields a bit to better match the current situation.

Acked-by: Sui Jingfeng <[email protected]>
Reviewed-by: Daniel Vetter <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Maxime Ripard <[email protected]>

show more ...

drm/atomic: Remove inexistent reference

Commit 63e83c1dba54 ("drm: Consolidate connector arrays in
drm_atomic_state") removed the connector_states field but didn't remove
its mention in the num_conn

drm/atomic: Remove inexistent reference

Commit 63e83c1dba54 ("drm: Consolidate connector arrays in
drm_atomic_state") removed the connector_states field but didn't remove
its mention in the num_connectors documentation.

Acked-by: Sui Jingfeng <[email protected]>
Reviewed-by: Daniel Vetter <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Maxime Ripard <[email protected]>

show more ...

drm/atomic: Move the drm_atomic_state field doc inline

Some fields of drm_atomic_state have been documented in-line, but some
were documented in the main kerneldoc block before the structure.

Since

drm/atomic: Move the drm_atomic_state field doc inline

Some fields of drm_atomic_state have been documented in-line, but some
were documented in the main kerneldoc block before the structure.

Since the former is the preferred option in DRM, let's move all the
fields to an inline documentation.

Acked-by: Sui Jingfeng <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Maxime Ripard <[email protected]>

show more ...

drm/bridge: Fix kernel-doc typo in desc of output_bus_cfg in drm_bridge_state

There's an obvious copy-paste error in the description of
output_bus_cfg. Fix it.

Fixes: f32df58acc68 ("drm/bridge: Add

drm/bridge: Fix kernel-doc typo in desc of output_bus_cfg in drm_bridge_state

There's an obvious copy-paste error in the description of
output_bus_cfg. Fix it.

Fixes: f32df58acc68 ("drm/bridge: Add the necessary bits to support bus format negotiation")
Signed-off-by: Douglas Anderson <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/20230817094808.1.I41b04c3a8305c9f1c17af886c327941c5136ca3b@changeid

show more ...

drm: add helper functions to retrieve old and new crtc

Add new helper functions, drm_atomic_get_old_crtc_for_encoder
and drm_atomic_get_new_crtc_for_encoder to retrieve the
corresponding crtc for th

drm: add helper functions to retrieve old and new crtc

Add new helper functions, drm_atomic_get_old_crtc_for_encoder
and drm_atomic_get_new_crtc_for_encoder to retrieve the
corresponding crtc for the encoder.

Signed-off-by: Sankeerth Billakanti <[email protected]>
Signed-off-by: Vinod Polimera <[email protected]>
Reviewed-by: Douglas Anderson <[email protected]>
Reviewed-by: Daniel Vetter <[email protected]>
Patchwork: https://patchwork.freedesktop.org/patch/524718/
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Dmitry Baryshkov <[email protected]>

show more ...

drm/atomic: Constify the old/new state accessors

The drm_atomic_get_(old|new)_*_state don't modify the passed
drm_atomic_state, so we can make it const.

Reviewed-by: Javier Martinez Canillas <javie

drm/atomic: Constify the old/new state accessors

The drm_atomic_get_(old|new)_*_state don't modify the passed
drm_atomic_state, so we can make it const.

Reviewed-by: Javier Martinez Canillas <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]

show more ...

drm/atomic: fix warning of unused variable

Fix the `unused-but-set-variable` warning as how other iteration
wrappers do.

Link: https://lore.kernel.org/all/[email protected]/
Repor

drm/atomic: fix warning of unused variable

Fix the `unused-but-set-variable` warning as how other iteration
wrappers do.

Link: https://lore.kernel.org/all/[email protected]/
Reported-by: kernel test robot <[email protected]>
Signed-off-by: GONG, Ruiqi <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]

show more ...

drm/atomic: Add atomic_print_state to private objects

A number of drivers (amdgpu, komeda, vc4, etc.) leverage the
drm_private_state structure, but we don't have any infrastructure to
provide debugg

drm/atomic: Add atomic_print_state to private objects

A number of drivers (amdgpu, komeda, vc4, etc.) leverage the
drm_private_state structure, but we don't have any infrastructure to
provide debugging like we do for the other components state. Let's add
an atomic_print_state hook to be consistent.

Reviewed-by: Daniel Vetter <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
Link: https://lore.kernel.org/r/[email protected]

show more ...

drm: fix doc warnings in drm_atomic.h

Add description for parameters for
for_each_new_plane_in_state_reverse to fix warnings:

include/drm/drm_atomic.h:908: warning: Function parameter or member '__

drm: fix doc warnings in drm_atomic.h

Add description for parameters for
for_each_new_plane_in_state_reverse to fix warnings:

include/drm/drm_atomic.h:908: warning: Function parameter or member '__state' not described in 'for_each_new_plane_in_state_reverse'
include/drm/drm_atomic.h:908: warning: Function parameter or member 'plane' not described in 'for_each_new_plane_in_state_reverse'
include/drm/drm_atomic.h:908: warning: Function parameter or member 'new_plane_state' not described in 'for_each_new_plane_in_state_reverse'
include/drm/drm_atomic.h:908: warning: Function parameter or member '__i' not described in 'for_each_new_plane_in_state_reverse'

Fixes: a6c3c37b661d ("drm/amd/display: fix gcc set but not used warning of variable 'old_plane_state'")
Signed-off-by: Yu Kuai <[email protected]>
Signed-off-by: Alex Deucher <[email protected]>

show more ...

drm/amd/display: fix gcc set but not used warning of variable 'old_plane_state'

define a new macro for_each_new_plane_in_state_reverse to replace
for_each_oldnew_plane_in_state_reverse, so that the

drm/amd/display: fix gcc set but not used warning of variable 'old_plane_state'

define a new macro for_each_new_plane_in_state_reverse to replace
for_each_oldnew_plane_in_state_reverse, so that the unused variable
'old_plane_state' can be removed.

Fix gcc warning:
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:10066:26: warning:
variable ‘old_plane_state’ set but not used [-Wunused-but-set-variable]

Signed-off-by: Yu Kuai <[email protected]>
Signed-off-by: Alex Deucher <[email protected]>

show more ...

drm: Introduce a drm_crtc_commit_wait helper

There's currently four users of the same logic to wait for a commit to
be flipped: three for the CRTCs, connectors and planes in
drm_atomic_helper_wait_f

drm: Introduce a drm_crtc_commit_wait helper

There's currently four users of the same logic to wait for a commit to
be flipped: three for the CRTCs, connectors and planes in
drm_atomic_helper_wait_for_dependencies, and one in vc4.

Let's consolidate this a bit to avoid any code duplication.

Suggested-by: Daniel Vetter <[email protected]>
Reviewed-by: Daniel Vetter <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]

show more ...

drm: Document use-after-free gotcha with private objects

The private objects have a gotcha that could result in a use-after-free,
make sure it's properly documented.

Reviewed-by: Daniel Vetter <dan

drm: Document use-after-free gotcha with private objects

The private objects have a gotcha that could result in a use-after-free,
make sure it's properly documented.

Reviewed-by: Daniel Vetter <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]

show more ...

include/drm/drm_atomic: Make use of 'new_crtc_state'

In the macro for_each_oldnew_crtc_in_state() 'crtc_state' is provided
as a container for state->crtcs[i].new_state, but is not utilised in
some u

include/drm/drm_atomic: Make use of 'new_crtc_state'

In the macro for_each_oldnew_crtc_in_state() 'crtc_state' is provided
as a container for state->crtcs[i].new_state, but is not utilised in
some use-cases, so we fake-use it instead.

Fixes the following W=1 kernel build warning(s):

drivers/gpu/drm/imx/ipuv3-plane.c: In function ‘ipu_planes_assign_pre’:
drivers/gpu/drm/imx/ipuv3-plane.c:746:42: warning: variable ‘crtc_state’ set but not used [-Wunused-but-set-variable]

Cc: Philipp Zabel <[email protected]>
Cc: David Airlie <[email protected]>
Cc: Daniel Vetter <[email protected]>
Cc: Shawn Guo <[email protected]>
Cc: Sascha Hauer <[email protected]>
Cc: Pengutronix Kernel Team <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: NXP Linux Team <[email protected]>
Cc: [email protected]
Signed-off-by: Lee Jones <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]

show more ...

include: drm: drm_atomic: Artificially use 'crtc' to avoid 'not used' warning

The precedent has already been set by other macros in the same file.

Fixes the following W=1 kernel build warning(s):

include: drm: drm_atomic: Artificially use 'crtc' to avoid 'not used' warning

The precedent has already been set by other macros in the same file.

Fixes the following W=1 kernel build warning(s):

drivers/gpu/drm/vkms/vkms_drv.c:55:19: warning: variable ‘crtc’ set but not used [-Wunused-but-set-variable]
55 | struct drm_crtc *crtc;
| ^~~~

Cc: Maarten Lankhorst <[email protected]>
Cc: Maxime Ripard <[email protected]>
Cc: Thomas Zimmermann <[email protected]>
Cc: David Airlie <[email protected]>
Cc: Daniel Vetter <[email protected]>
Cc: Rob Clark <[email protected]>
Cc: [email protected]
Signed-off-by: Lee Jones <[email protected]>
Signed-off-by: Sam Ravnborg <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]

show more ...

drm/doc: Document legacy_cursor_update better

It's the horror and shouldn't be used. Realized we're not clear on
this in a discussion with Rob about what msm is doing to better
support async commits

drm/doc: Document legacy_cursor_update better

It's the horror and shouldn't be used. Realized we're not clear on
this in a discussion with Rob about what msm is doing to better
support async commits.

v2: Refine existing todo item to include this (Thomas)

Cc: Sean Paul <[email protected]>
Cc: Rob Clark <[email protected]>
Acked-by: Rob Clark <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Cc: Maarten Lankhorst <[email protected]>
Cc: Maxime Ripard <[email protected]>
Cc: Thomas Zimmermann <[email protected]>
Cc: David Airlie <[email protected]>
Cc: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]

show more ...

drm: drm_atomic.h: delete duplicated word in comment

Drop doubled word "than" in a comment.

Signed-off-by: Randy Dunlap <[email protected]>
Cc: David Airlie <[email protected]>
Cc: Daniel Vetter

drm: drm_atomic.h: delete duplicated word in comment

Drop doubled word "than" in a comment.

Signed-off-by: Randy Dunlap <[email protected]>
Cc: David Airlie <[email protected]>
Cc: Daniel Vetter <[email protected]>
Cc: [email protected]
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]

show more ...

drm/bridge: Fix the bridge kernel doc

Commit 751465913f04 ("drm/bridge: Add a drm_bridge_state object")
introduced new helpers and hooks but the kernel was slightly broken.
Fix that now.

v2:
* Fix

drm/bridge: Fix the bridge kernel doc

Commit 751465913f04 ("drm/bridge: Add a drm_bridge_state object")
introduced new helpers and hooks but the kernel was slightly broken.
Fix that now.

v2:
* Fix the drm_atomic_add_encoder_bridges() doc

Fixes: 751465913f04 ("drm/bridge: Add a drm_bridge_state object")
Signed-off-by: Boris Brezillon <[email protected]>
Reviewed-by: Neil Armstrong <[email protected]>
Signed-off-by: Neil Armstrong <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]

show more ...

drm/bridge: Add the necessary bits to support bus format negotiation

drm_bridge_state is extended to describe the input and output bus
configurations. These bus configurations are exposed through th

drm/bridge: Add the necessary bits to support bus format negotiation

drm_bridge_state is extended to describe the input and output bus
configurations. These bus configurations are exposed through the
drm_bus_cfg struct which encodes the configuration of a physical
bus between two components in an output pipeline, usually between
two bridges, an encoder and a bridge, or a bridge and a connector.

The bus configuration is stored in drm_bridge_state separately for
the input and output buses, as seen from the point of view of each
bridge. The bus configuration of a bridge output is usually identical
to the configuration of the next bridge's input, but may differ if
the signals are modified between the two bridges, for instance by an
inverter on the board. The input and output configurations of a
bridge may differ if the bridge modifies the signals internally,
for instance by performing format conversion, or*modifying signals
polarities.

Bus format negotiation is automated by the core, drivers just have
to implement the ->atomic_get_{output,input}_bus_fmts() hooks if they
want to take part to this negotiation. Negotiation happens in reverse
order, starting from the last element of the chain (the one directly
connected to the display) up to the first element of the chain (the one
connected to the encoder).
During this negotiation all supported formats are tested until we find
one that works, meaning that the formats array should be in decreasing
preference order (assuming the driver has a preference order).

Note that the bus format negotiation works even if some elements in the
chain don't implement the ->atomic_get_{output,input}_bus_fmts() hooks.
In that case, the core advertises only MEDIA_BUS_FMT_FIXED and lets
the previous bridge element decide what to do (most of the time, bridge
drivers will pick a default bus format or extract this piece of
information from somewhere else, like a FW property).

v10:
* Add changelog to the commit message

v9:
* No changes

v8:
* Fix a test in drm_atomic_bridge_chain_select_bus_fmts() (Reported by
Jonas)

v7:
* Adapt the code to deal with the fact that not all bridges in the
chain have a bridge state

v5 -> v6:
* No changes

v4:
* Enhance the doc
* Fix typos
* Rename some parameters/fields
* Reword the commit message

v3:
* Fix the commit message (Reported by Laurent)
* Document the fact that bus formats should not be directly modified by
drivers (Suggested by Laurent)
* Document the fact that format order matters (Suggested by Laurent)
* Propagate bus flags by default
* Document the fact that drivers can tweak bus flags if needed
* Let ->atomic_get_{output,input}_bus_fmts() allocate the bus format
array (Suggested by Laurent)
* Add a drm_atomic_helper_bridge_propagate_bus_fmt()
* Mandate that bridge drivers return accurate input_fmts even if they
are known to be the first element in the bridge chain

v2:
* Rework things to support more complex use cases

Signed-off-by: Boris Brezillon <[email protected]>
Signed-off-by: Neil Armstrong <[email protected]>
[narmstrong: fixed doc in include/drm/drm_bridge.h:69 fmt->format]
Reviewed-by: Jernej Skrabec <[email protected]>
Tested-by: Jonas Karlman <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]

show more ...

drm/bridge: Add a drm_bridge_state object

One of the last remaining objects to not have its atomic state.

This is being motivated by our attempt to support runtime bus-format
negotiation between el

drm/bridge: Add a drm_bridge_state object

One of the last remaining objects to not have its atomic state.

This is being motivated by our attempt to support runtime bus-format
negotiation between elements of the bridge chain.
This patch just paves the road for such a feature by adding a new
drm_bridge_state object inheriting from drm_private_obj so we can
re-use some of the existing state initialization/tracking logic.

v10:
* Add changelog to the commit message

v9:
* Clarify the fact that the bridge->atomic_reset() and
{connector,plane,crtc,...}->reset() semantics are different
* Move the drm_atomic_private_obj_init() call back to
drm_bridge_attach()
* Check the presence of ->atomic_duplicate_state instead of
->atomic_reset in drm_atomic_add_encoder_bridges()
* Fix copy&paste errors in the atomic bridge state helpers doc
* Add A-b/R-b tags

v8:
* Move bridge state helpers out of the CONFIG_DEBUGFS section

v7:
* Move helpers, struct-defs, ... to atomic helper files to avoid the
drm -> drm_kms_helper -> drm circular dep
* Stop providing default implementation for atomic state reset,
duplicate and destroy hooks (has to do with the helper/core split)
* Drop all R-b/T-b as helpers have now be moved to other places

v6:
* Made helpers private, removed doc and moved them to satisfy dependencies
* Renamed helpers to _default_

v5:
* Re-introduced the helpers from v4

v4:
* Fix the doc
* Kill default helpers (inlined)
* Fix drm_atomic_get_bridge_state() to check for an ERR_PTR()
* Add Neil's R-b

v3:
* No changes

v2:
* Use drm_for_each_bridge_in_chain()
* Rename helpers to be more consistent with the rest of the DRM API
* Improve/fix the doc

Signed-off-by: Boris Brezillon <[email protected]>
Signed-off-by: Neil Armstrong <[email protected]>
Reviewed-by: Neil Armstrong <[email protected]>
Acked-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]

show more ...

Revert "drm/bridge: Add a drm_bridge_state object"

This reverts commit 6ed7e9625fa6 ("drm/bridge: Add a drm_bridge_state
object") which introduced a circular dependency between drm.ko and
drm_kms_he

Revert "drm/bridge: Add a drm_bridge_state object"

This reverts commit 6ed7e9625fa6 ("drm/bridge: Add a drm_bridge_state
object") which introduced a circular dependency between drm.ko and
drm_kms_helper.ko. Looks like the helper/core split is not appropriate
and fixing that is not simple.

Signed-off-by: Boris Brezillon <[email protected]>
Acked-by: Neil Armstrong <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]

show more ...

drm/bridge: Add a drm_bridge_state object

One of the last remaining objects to not have its atomic state.

This is being motivated by our attempt to support runtime bus-format
negotiation between el

drm/bridge: Add a drm_bridge_state object

One of the last remaining objects to not have its atomic state.

This is being motivated by our attempt to support runtime bus-format
negotiation between elements of the bridge chain.
This patch just paves the road for such a feature by adding a new
drm_bridge_state object inheriting from drm_private_obj so we can
re-use some of the existing state initialization/tracking logic.

Signed-off-by: Boris Brezillon <[email protected]>
Reviewed-by: Neil Armstrong <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
Signed-off-by: Neil Armstrong <[email protected]>
Reviewed by: Jernej Skrabec <[email protected]>
Tested-by: Jonas Karlman <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]

show more ...