[PATCH 06/18] drm/vc4: Rework the encoder retrieval code

[Thomas Zimmermann]

Hi

Am 17.03.21 um 16:43 schrieb Maxime Ripard:
> Due to a FIFO that cannot be flushed between the pixelvalve and the HDMI
> controllers on BCM2711, we need to carefully disable both at boot time
> if they were left enabled by the firmware.
> 
> However, at the time we're running that code, the struct drm_connector
> encoder pointer isn't set yet, and thus we cannot retrieve the encoder
> associated to our CRTC.
> 
> We can however make use of the fact that we have a less flexible setup
> than what DRM allows where we have a 1:1 relationship between our CRTCs
> and encoders (and connectors), and thus store the crtc associated to our
> encoder at boot time.
> 
> We cannot do that at the time the encoders are probed though, since the
> CRTCs won't be probed yet and thus we don't know at that time which CRTC
> index we're going to get, so let's do this in two passes: we can first
> bind all the components and then once they all are bound, we can iterate
> over all the encoders to find their associated CRTC and set the pointer.
> 
> This is similar to what we're doing to set the possible_crtcs field.
> 
> Fixes: 875a4d536842 ("drm/vc4: drv: Disable the CRTC at boot time")

Cc: <stable at vger.kernel.org> # v5.10+

> Signed-off-by: Maxime Ripard <maxime at cerno.tech>
> ---
>   drivers/gpu/drm/vc4/vc4_crtc.c | 25 +++++++++++++++++++++--
>   drivers/gpu/drm/vc4/vc4_drv.c  | 36 ++++++++++++++++++++++++++++++++++
>   drivers/gpu/drm/vc4/vc4_drv.h  |  1 +
>   3 files changed, 60 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
> index f1f2e8cbce79..e2607e1f2520 100644
> --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> @@ -255,6 +255,19 @@ static u32 vc4_crtc_get_fifo_full_level_bits(struct vc4_crtc *vc4_crtc,
>   				   PV_CONTROL_FIFO_LEVEL);
>   }
>   
> +struct drm_encoder *vc4_get_connector_encoder(struct drm_connector *connector)
> +{
> +	struct drm_encoder *encoder;
> +
> +	if (WARN_ON(hweight32(connector->possible_encoders) != 1))

drm_WARN_ON

> +		return NULL;
> +
> +	drm_connector_for_each_possible_encoder(connector, encoder)
> +		return encoder;
> +
> +	return NULL;
> +}
> +
>   /*
>    * Returns the encoder attached to the CRTC.
>    *
> @@ -269,9 +282,17 @@ static struct drm_encoder *vc4_get_crtc_encoder(struct drm_crtc *crtc)
>   
>   	drm_connector_list_iter_begin(crtc->dev, &conn_iter);
>   	drm_for_each_connector_iter(connector, &conn_iter) {
> -		if (connector->state->crtc == crtc) {
> +		struct drm_encoder *encoder;
> +		struct vc4_encoder *vc4_encoder;
> +
> +		encoder = vc4_get_connector_encoder(connector);
> +		if (!encoder)
> +			continue;
> +
> +		vc4_encoder = to_vc4_encoder(encoder);
> +		if (vc4_encoder->crtc == crtc) {
>   			drm_connector_list_iter_end(&conn_iter);
> -			return connector->encoder;
> +			return encoder;
>   		}
>   	}
>   	drm_connector_list_iter_end(&conn_iter);
> diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
> index 556ad0f02a0d..cd1fb75c66a7 100644
> --- a/drivers/gpu/drm/vc4/vc4_drv.c
> +++ b/drivers/gpu/drm/vc4/vc4_drv.c
> @@ -199,6 +199,41 @@ static int compare_dev(struct device *dev, void *data)
>   	return dev == data;
>   }
>   
> +static struct drm_crtc *vc4_drv_find_crtc(struct drm_device *drm,
> +					  struct drm_encoder *encoder)
> +{
> +	struct drm_crtc *crtc;
> +
> +	if (WARN_ON(hweight32(encoder->possible_crtcs) != 1))
> +		return NULL;
> +
> +	drm_for_each_crtc(crtc, drm) {
> +		if (!drm_encoder_crtc_ok(encoder, crtc))
> +			continue;
> +
> +		return crtc;
> +	}
> +
> +	return NULL;
> +}
> +
> +static void vc4_drv_set_encoder_data(struct drm_device *drm)
> +{
> +	struct drm_encoder *encoder;
> +
> +	drm_for_each_encoder(encoder, drm) {
> +		struct vc4_encoder *vc4_encoder;
> +		struct drm_crtc *crtc;
> +
> +		crtc = vc4_drv_find_crtc(drm, encoder);
> +		if (WARN_ON(!crtc))
> +			return;
> +
> +		vc4_encoder = to_vc4_encoder(encoder);
> +		vc4_encoder->crtc = crtc;
> +	}
> +}
> +
>   static void vc4_match_add_drivers(struct device *dev,
>   				  struct component_match **match,
>   				  struct platform_driver *const *drivers,
> @@ -261,6 +296,7 @@ static int vc4_drm_bind(struct device *dev)
>   	ret = component_bind_all(dev, drm);
>   	if (ret)
>   		return ret;
> +	vc4_drv_set_encoder_data(drm);
>   
>   	ret = vc4_plane_create_additional_planes(drm);
>   	if (ret)
> diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
> index a7500716cf3f..1b569dcc2154 100644
> --- a/drivers/gpu/drm/vc4/vc4_drv.h
> +++ b/drivers/gpu/drm/vc4/vc4_drv.h
> @@ -438,6 +438,7 @@ enum vc4_encoder_type {
>   
>   struct vc4_encoder {
>   	struct drm_encoder base;
> +	struct drm_crtc *crtc;

I'd probably deserves a comment why this is explicitly stored here.

>   	enum vc4_encoder_type type;
>   	u32 clock_select;
>   
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer

-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20210409/f4a0a72f/attachment.sig>