Re: [PATCH] drm/omap: fix primary-plane's possible_crtcs

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Tomi,

Thank you for the patch.

On Wednesday 30 Nov 2016 12:38:51 Tomi Valkeinen wrote:
> We set the possible_crtc for all planes to "(1 << priv->num_crtcs) - 1",
> which is fine as the HW planes can be used fro all crtcs. However, when
> we're doing that, we are still incrementing 'num_crtcs', and we'll end
> up with bad possible_crtcs, preventing the use of the primary planes.
> 
> We should have all crtcs in 'possible_crtc', but apparently it's not as
> easy to set as you would think. We create crtcs rather dynamically, and
> when creating the primary planes, we don't know how many crtcs we're
> going to have. This is mostly a problem with the way omapdrm creates
> crtcs and planes, and how it connects those to display outputs.
> 
> So, this patch fixes the problem the easy way, and sets the
> possible_crtcs for primary planes only to the crtc in question, which in
> practice should cover all normal use cases.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  drivers/gpu/drm/omapdrm/omap_plane.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c
> b/drivers/gpu/drm/omapdrm/omap_plane.c index 9c43cb481e62..fc1822870b26
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_plane.c
> +++ b/drivers/gpu/drm/omapdrm/omap_plane.c
> @@ -361,6 +361,7 @@ struct drm_plane *omap_plane_init(struct drm_device
> *dev, struct omap_drm_private *priv = dev->dev_private;
>  	struct drm_plane *plane;
>  	struct omap_plane *omap_plane;
> +	unsigned long possible_crtcs;
>  	int ret;
> 
>  	DBG("%s: type=%d", plane_names[id], type);
> @@ -381,7 +382,12 @@ struct drm_plane *omap_plane_init(struct drm_device
> *dev, omap_plane->error_irq.irq = omap_plane_error_irq;
>  	omap_irq_register(dev, &omap_plane->error_irq);
> 
> -	ret = drm_universal_plane_init(dev, plane, (1 << priv->num_crtcs) - 1,
> +	if (type == DRM_PLANE_TYPE_PRIMARY)
> +		possible_crtcs = 1 << id;
> +	else
> +		possible_crtcs = (1 << priv->num_crtcs) - 1;

The omap_modeset_init() function computes the number of CRTCs before creating 
any plane with

	num_crtcs = min3(num_crtc, num_mgrs, num_ovls);

and the rest of the function then creates CRTCs, as far as I understand always 
up to num_crtcs. Can't we just use that value to compute possible_crtcs as (1 
<< num_crtcs ) - 1 ?

> +
> +	ret = drm_universal_plane_init(dev, plane, possible_crtcs,
>  				       &omap_plane_funcs, omap_plane->formats,
>  				       omap_plane->nformats, type, NULL);
>  	if (ret < 0)

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel