Re: [RFC PATCH v3 1/2] drm: Add generic colorkey properties for DRM planes

[Dmitry Osipenko]

On Friday, 6 July 2018 18:40:27 MSK Russell King - ARM Linux wrote:
> On Fri, Jul 06, 2018 at 05:58:50PM +0300, Dmitry Osipenko wrote:
> > On Friday, 6 July 2018 17:10:10 MSK Ville Syrjälä wrote:
> > > IIRC my earlier idea was to have different colorkey modes for the
> > > min+max and value+mask modes. That way userspace might actually have
> > > some chance of figuring out which bits of state actually do something.
> > > Although for Intel hw I think the general rule is that min+max for YUV,
> > > value+mask for RGB, so it's still not 100% clear what to pick if the
> > > plane supports both.
> > > 
> > > I guess one alternative would be to have min+max only, and the driver
> > > would reject 'min != max' if it only uses a single value?
> > 
> > You should pick both and reject unsupported property values based on the
> > planes framebuffer format. So it will be possible to set unsupported
> > values
> > while plane is disabled because it doesn't have an associated framebuffer
> > and then atomic check will fail to enable plane if property values are
> > invalid for the given format.
> 
> The colorkey which is attached to a plane 'A' is not applied to plane
> 'A', so the format of plane 'A' is not relevant.  The colorkey is
> applied to some other plane which will be below this plane in terms
> of the plane blending operation.
> 
> What if you have several planes below plane 'A' with differing
> framebuffer formats - maybe an ARGB8888 plane and a ARGB1555 plane -
> do you decide to limit the colorkey to 8bits per channel, or to
> ARGB1555 format?
> 
> The answer is, of course, hardware dependent - generic code can't
> know the details of the colorkey implementation, which could be one
> of:
> 
>   lower plane data -> expand to 8bpc -> match ARGB8888 colorkey
>   lower plane data -> match ARGB8888 reduced to plane compatible colorkey
> 
> which will give different results depending on the format of the
> lower plane data.

All unsupportable cases should be rejected in the atomic check. If your HW 
can't handle the case where multiple bottom planes have a different format, 
then in the planes atomic check you'll have to walk up all the bottom planes 
and verify their formats.

I'm not sure whether it's worth to check the planes intersection and fail only 
if top plane intersects with multiple bottom planes having different formats. 
Perhaps that could be a driver-specific implementation detail, userspace 
should take into account that atomic commit may fail on changing planes 
position.