Re: [PATCH v3 2/7] drm/exynos: make zpos property configurable

[Marek Szyprowski <m.szyprowski@samsung.com>]

Hello,

On 2015-12-16 15:21, Daniel Vetter wrote:
> On Wed, Dec 16, 2015 at 02:54:04PM +0100, Marek Szyprowski wrote:
>> On 2015-12-16 14:28, Daniel Vetter wrote:
>>> On Wed, Dec 16, 2015 at 01:21:43PM +0100, Marek Szyprowski wrote:
>>>> This patch adds all infrastructure to make zpos plane property
>>>> configurable from userspace.
>>>>
>>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>> Imo zpos should be a generic atomic property with well-defined semantics
>>> shared across drivers. So
>>> - storead (in decoded form) in drm_plane_state
>>> - common functions to register/attach zpos prop to a plane, with
>>>    full-blown kerneldoc explaining how it should work
>>> - generic kms igt to validate that interface
>>>
>>> One of the big things that always comes up when we talk about zpos is how
>>> equal zpos should be handled, and how exactly they should be sorted. I
>>> think for that we should have a drm function which computes a normalized
>>> zpos. Or the core check code should reject such nonsense.
>> IMHO it will be enough to state that the case of equal zpos is HW specific.
>> This might simplify some driver logic. For example, in case of Exynos Mixer,
>> equal priority (zpos) means that HW predefined order will be used, so there
>> is no need to normalize zpos values.
>>
>> If you want I can move zpos to drm core and add a function to normalize
>> zpos,
>> although for this particular driver normalization is not needed.
>>
>> It should be quite easy to convert other drivers to use the generic zpos
>> property. The only problem I see is how to handle omap driver, which use
>> 'zorder' property.
>>
>> What about some other typical properties related to blending:
>> - global plane alpha,
>> - colorkey,
>> - alpha mode (standard or pre-multiplied) for alpha-enabled modes,
>> - crtc background color.
>>
>> Do you also want to handle them as generic or driver-specific properties?
> Should all be generic really. And it's also kinda ABI, so userspace
> needed, and preferrably a proper/automated testcase. i915 has
> infrastructure to use display pipeline CRCs to really measure it's all
> correct even, and that's the standard I'd like to see for all KMS API
> extensions like this.

Could you tell a bit more about this pipeline CRCs? What is it?

> Since if we don't do this we'll end up in a giant mess, and it will be
> impossible to write a kms compositor that's generic and uses all this.
>
> And since this stuff is supposed to be generic, fluffy/unspecified
> semantics aren't good. Especially if your hw can just somehow deal with it
> all.
>
>> I plan to add support for them to Exynos Mixer and I would like to avoid
>> doing this twice.
> It's a lot more work than just adding a few members to drm_plane_state.

I see. Could you elaborate a bit more on what you want to have in drm 
core for
handling all the mentioned features?

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland