Re: [PATCH 16/29] drm/encoder: Create get_current_crtc hook

[Simona Vetter <simona.vetter@ffwll.ch>]

On Tue, Sep 02, 2025 at 10:32:44AM +0200, Maxime Ripard wrote:
> In order for drivers to implement drm_connectors atomic_state_readout
> hooks, they need to query the hardware and lookup the CRTC to set
> drm_connector_state.crtc.
> 
> It should be easy enough for drivers that are tightly integrated from
> the CRTC to the connectors, but if the driver uses bridges, there's no
> coupling between the CRTC and encoder, and the bridge driver.
> 
> The only thing the bridge has access to is the encoder, but the
> relationship between a CRTC and an encoder isn't a fixed mapping at the
> framework level, and thus the bridge can't deduce which CRTC is feeding
> its encoder.
> 
> Thus, let's create a new hook for encoders to implement to return the
> CRTC they are currently connected to.
> 
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---
>  include/drm/drm_encoder.h | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/include/drm/drm_encoder.h b/include/drm/drm_encoder.h
> index 977a9381c8ba943b4d3e021635ea14856df8a17d..7abb5e35ac950c941b56fc311a52789bbc427df6 100644
> --- a/include/drm/drm_encoder.h
> +++ b/include/drm/drm_encoder.h
> @@ -86,10 +86,23 @@ struct drm_encoder_funcs {
>  	 * @debugfs_init:
>  	 *
>  	 * Allows encoders to create encoder-specific debugfs files.
>  	 */
>  	void (*debugfs_init)(struct drm_encoder *encoder, struct dentry *root);
> +
> +	/**
> +	 * @get_current_crtc:

Bit a bikeshed, but crossed my mind reading this: I think some kind of
prefix for all the state-readout/comparison related stuff and maybe even
putting all the new helpers into a new c/h files would be good. That way
we have a nice place for a DOC: section and people have a better chance of
understanding what they can ignore. Might still be good to put a standard
one-sentence explainer here that this is part of the state
readout/fastboot support and maybe link to the main helper function for
that.

For the prefix I'm lacking good ideas, but maybe asro for atomic state
readout? It's at least unique (to my knowledge), unlike all the other
shorthands I could come up with (like atomic fast boot and afb). Spelling
it out is way too long unfortunately.

Oh also, since this is part of the optional helpers it should be in
drm_encoder_helper_funcs, only uabi interfaces should be put into these
here. This also applies to all the other hooks.
-Sima


> +	 *
> +	 * This optional hook is used during initialization to read out
> +	 * the initial state by connectors that support atomic state
> +	 * hardware readout.
> +	 *
> +	 * Returns:
> +	 *
> +	 * The CRTC currently associated with the encoder if enabled, NULL otherwise.
> +	 */
> +	struct drm_crtc *(*get_current_crtc)(struct drm_encoder *encoder);
>  };
>  
>  /**
>   * struct drm_encoder - central DRM encoder structure
>   * @dev: parent DRM device
> 
> -- 
> 2.50.1
> 

-- 
Simona Vetter
Software Engineer
http://blog.ffwll.ch