Re: [PATCH 05/14] drm: bridge: icn6211: Retrieve the display mode from the state

[Maxime Ripard <maxime@cerno.tech>]

[-- Attachment #1: Type: text/plain, Size: 2345 bytes --]

On Fri, Jan 14, 2022 at 04:48:29AM +0100, Marek Vasut wrote:
> Retrieve display mode structure from panel or atomic state in
> bridge_to_mode(). This completes the transition to the atomic
> API.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Jagan Teki <jagan@amarulasolutions.com>
> Cc: Robert Foss <robert.foss@linaro.org>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> To: dri-devel@lists.freedesktop.org
> ---
>  drivers/gpu/drm/bridge/chipone-icn6211.c | 26 +++++++++++++++++++++---
>  1 file changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/chipone-icn6211.c b/drivers/gpu/drm/bridge/chipone-icn6211.c
> index 14d28e7356aaa..d6db1e77b5a35 100644
> --- a/drivers/gpu/drm/bridge/chipone-icn6211.c
> +++ b/drivers/gpu/drm/bridge/chipone-icn6211.c
> @@ -146,9 +146,28 @@ static inline struct chipone *bridge_to_chipone(struct drm_bridge *bridge)
>  	return container_of(bridge, struct chipone, bridge);
>  }
>  
> -static struct drm_display_mode *bridge_to_mode(struct drm_bridge *bridge)
> +static const struct drm_display_mode *
> +bridge_to_mode(struct drm_bridge *bridge, struct drm_atomic_state *state)
>  {
> -	return &bridge->encoder->crtc->state->adjusted_mode;
> +	const struct drm_crtc_state *crtc_state;
> +	struct drm_connector *connector;
> +	struct drm_crtc *crtc;
> +
> +	/* Try to retrieve panel mode first. */
> +	connector = drm_atomic_get_new_connector_for_encoder(state,
> +							     bridge->encoder);
> +	if (!list_empty(&connector->modes)) {
> +		return list_first_entry(&connector->modes,
> +					struct drm_display_mode, head);
> +	}

If I understand this right, this will return the first mode on the
connector, which should be always set. So you always end up returning
the preferred mode for that panel?

> +	/*
> +	 * Retrieve the CRTC adjusted mode. This requires a little dance to go
> +	 * from the bridge to the encoder, to the connector and to the CRTC.
> +	 */
> +	crtc = drm_atomic_get_new_connector_state(state, connector)->crtc;
> +	crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
> +	return &crtc_state->adjusted_mode;

And thus entirely disregarding the actual mode that was set by the
userspace, or ignoring any other mode than the preferred one?

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]