Re: [PATCH 1/4] drm/simple_kms_helper: enable use of external encoder

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Feb 07, 2019 at 09:36:44AM +0100, Linus Walleij wrote:
> This makes it possible to pass a connector with an already
> attached external encoder into the simple KMS helper.
> 
> This is helpful for my MCDE drivers, as it is pretty simple
> but uses DSI to communicate with the displays and bridges.
> DSI requires the use of the DSI bus which in turn requires
> us to set up a custom connector from the display driver.

So the idea was that you'd just use a bridge for this, if you need more
than a dummy encoder. I'm a bit worried about mission creep for the simple
helpers, sooner or later we'll add extensions and then we're back to full
kms, except it's a hairball of classic midlayer mistake ...

See drm_simple_display_pipe_attach_bridge().

Cheers, Daniel

> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/gpu/drm/drm_simple_kms_helper.c | 23 ++++++++++++++++++-----
>  1 file changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
> index 917812448d1b..e7499b939235 100644
> --- a/drivers/gpu/drm/drm_simple_kms_helper.c
> +++ b/drivers/gpu/drm/drm_simple_kms_helper.c
> @@ -266,7 +266,7 @@ int drm_simple_display_pipe_init(struct drm_device *dev,
>  			const uint64_t *format_modifiers,
>  			struct drm_connector *connector)
>  {
> -	struct drm_encoder *encoder = &pipe->encoder;
> +	struct drm_encoder *encoder;
>  	struct drm_plane *plane = &pipe->plane;
>  	struct drm_crtc *crtc = &pipe->crtc;
>  	int ret;
> @@ -289,10 +289,23 @@ int drm_simple_display_pipe_init(struct drm_device *dev,
>  	if (ret)
>  		return ret;
>  
> -	encoder->possible_crtcs = drm_crtc_mask(crtc);
> -	ret = drm_encoder_init(dev, encoder, &drm_simple_kms_encoder_funcs,
> -			       DRM_MODE_ENCODER_NONE, NULL);
> -	if (ret || !connector)
> +	/* Other encoder already attached to the connector */
> +	if (connector->encoder_ids[0] != 0) {
> +		encoder = drm_encoder_find(connector->dev, NULL,
> +					   connector->encoder_ids[0]);
> +		encoder->possible_crtcs = drm_crtc_mask(crtc);
> +		DRM_INFO("an encoder is already attached to the connector\n");
> +	} else {
> +		encoder = &pipe->encoder;
> +		encoder->possible_crtcs = drm_crtc_mask(crtc);
> +		ret = drm_encoder_init(dev, encoder,
> +				       &drm_simple_kms_encoder_funcs,
> +				       DRM_MODE_ENCODER_NONE, NULL);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	if (!connector)
>  		return ret;
>  
>  	return drm_connector_attach_encoder(connector, encoder);
> -- 
> 2.20.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel