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

[Tomi Valkeinen <tomi.valkeinen@ti.com>]

[-- Attachment #1.1.1: Type: text/plain, Size: 2645 bytes --]

On 30/11/16 16:36, Laurent Pinchart wrote:
> 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 ?

Yes, I think you're correct. It's not exactly obvious from the code =).
I'll change the patch to use the calculated num-crtcs.

 Tomi


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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