On Wed, Jan 15, 2025 at 10:05:31PM +0100, Maxime Ripard wrote:
> We have access to the global drm_atomic_state from a drm_bridge_state,
> but since it's fairly indirect it's not as obvious as it can be for
> other KMS entities.
>
> Provide a helper to make it easier to figure out.
>
> Signed-off-by: Maxime Ripard <mrip...@kernel.org>
> ---
> include/drm/drm_atomic.h | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> index
> 31ca88deb10d262fb3a3f8e14d2afe24f8410cb1..bd7959ae312c99c0a0034d36378ae44f04f6a374
> 100644
> --- a/include/drm/drm_atomic.h
> +++ b/include/drm/drm_atomic.h
> @@ -1183,10 +1183,26 @@ static inline struct drm_bridge_state *
> drm_priv_to_bridge_state(struct drm_private_state *priv)
> {
> return container_of(priv, struct drm_bridge_state, base);
> }
>
> +/**
> + * @drm_bridge_state_get_atomic_state() - Get the atomic state from a bridge
> state
> + * @bridge_state: bridge state object
> + *
> + * RETURNS:
> + * The global atomic state @bridge_state is a part of, or NULL if there is
> none.
> + */
> +static inline struct drm_atomic_state *
> +drm_bridge_state_get_atomic_state(struct drm_bridge_state *bridge_state)
So this one is nasty, because we clear out these backpointers once we push
the states into obj->state (because they can then outlive the
drm_atomic_state). Which means you can't use this in commit callbacks. Or
the bridge code has a really bad use-after-free when it doesn't clear out
these backpointers when we swap in the new states in
drm_atomic_helper_swap_state().
The better pattern is to just ditch passing individual states to callbacks
and just pass the entire drm_atomic_state container, and let callbacks
fish out what exactly they need. And also provide all necessary helpers to
find the right states and all that stuff.
We should probably also document that design approach in the kerneldoc for
drm_atomic_state, or wherever there's a good place for that.
See also my other reply for some of the history of why we have this mess.
Cheers, Sima
> +{
> + if (!bridge_state)
> + return NULL;
> +
> + return bridge_state->base.state;
> +}
> +
> struct drm_bridge_state *
> drm_atomic_get_bridge_state(struct drm_atomic_state *state,
> struct drm_bridge *bridge);
> struct drm_bridge_state *
> drm_atomic_get_old_bridge_state(const struct drm_atomic_state *state,
>
> --
> 2.47.1
>
--
Simona Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
