[PATCH v4 1/4] drm: add generic zpos property

[Benjamin Gaignard]

2016-06-22 1:45 GMT+02:00 Laurent Pinchart <laurent.pinchart at ideasonboard.com>:
> Hi Benjamin,
>
> Thank you for the patch.
>
> Could you please reply to Ville's comment to v1 of this patch (posted on April
> the 25th) ?
>

For each iteration of the patches we have swap between two solutions:
- have normalization done between 0 to N-1 without taking care of zpos
property range
- take care of zpos property range while computing normalized zpos

First the solution is the most simple and the more robust so I will
definitively go for it.

[snip]
>> +     sort(states, n, sizeof(*states), drm_atomic_state_zpos_cmp, NULL);
>> +
>> +     for (i = 0, zpos = 0; i < n; i++, zpos++) {
>> +             plane = states[i]->plane;
>> +
>> +             zpos = max_t(int, zpos, states[i]->zpos);
>> +
>> +             WARN_ON(zpos > plane->zpos_property->values[1]);
>
> This crashes if the plane doesn't have a zpos property. The simplest fix is to
> write the check as
>
>                 WARN_ON(plane->zpos_property &&
>                         zpos > plane->zpos_property->values[1]);

Thanks I will add that in v5

> but I wonder how we should handle drivers that instantiate a zpos property for
> a subdev of the planes only. For drivers that don't use zpos at all you could
> maybe avoid calling this function.
>

I think it will be difficult to mix planes with and without zpos property.
My suggestion is to use immutable zpos property for primary and cursor plane.
drm_atomic_helper_crtc_normalize_zpos() is only called if plane zpos
value change
so if driver don't use it normalization will not be done.

> Additionally, this check is triggered with the rcar-du-drm driver when
> performing the following operations:
>
> - Perform a mode set with the CRTC primary plane only (that plane doesn't have
> a zpos property)
>
> - Add 7 overlay planes with zpos values 1 to 7 (their zpos property range is
> 1-7)
>
> - Modify the zpos value of all overlay planes one by one to 7 to 1 (setting
> zpos 7 for plane 1 first, then zpos 6 for plane 2, ...)
>
> I get normalized zpos values such as
>
> [   84.892927] [PLANE:39:plane-8] normalized zpos value 9
> [   85.899284] [PLANE:25:plane-0] normalized zpos value 0
> [   85.904488] [PLANE:37:plane-7] normalized zpos value 2
> [   85.909633] [PLANE:35:plane-6] normalized zpos value 3
> [   85.914793] [PLANE:33:plane-5] normalized zpos value 4
> [   85.919936] [PLANE:31:plane-4] normalized zpos value 5
> [   85.925100] [PLANE:29:plane-3] normalized zpos value 6
> [   85.930245] [PLANE:27:plane-2] normalized zpos value 7
>
> (plane 25 is the primary plane, all other planes are the overlay planes, added
> in the order 27, 29, 31, 33, 35, 37, 37 in the sequence above)
>
I'm not able to reproduce you problem on my setup with one primary and
two overlay.
When back to 0 to N-1 normalization algo you should see the problem anymore

[snip]
> --
> Regards,
>
> Laurent Pinchart
>



-- 
Benjamin Gaignard

Graphic Working Group

Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog