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@xxxxxx>
>> ---
>> 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:
signature.asc
Description: OpenPGP digital signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
