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

[Maarten Lankhorst <maarten.lankhorst@linux.intel.com>]

Op 08-08-18 om 16:30 schreef Dmitry Osipenko:
> On Wednesday, 8 August 2018 11:16:09 MSK Russell King - ARM Linux wrote:
>> On Tue, Aug 07, 2018 at 08:22:01PM +0300, Dmitry Osipenko wrote:
>>> + * Glossary:
>>> + *
>>> + * Destination plane:
>>> + *	Plane to which color keying properties are applied, this planes takes
>>> + *	the effect of color keying operation. The effect is determined by a
>>> + *	given color keying mode.
>>> + *
>>> + * Source plane:
>>> + *	Pixels of this plane are the source for color key matching operation.
>> ...
>>
>>> +	/**
>>> +	 * @DRM_PLANE_COLORKEY_MODE_TRANSPARENT:
>>> +	 *
>>> +	 * Destination plane pixels are completely transparent in areas
>>> +	 * where pixels of a source plane are matching a given color key
>>> +	 * range, in other cases pixels of a destination plane are 
> unaffected.
>>> +	 * In areas where two or more source planes overlap, the topmost
>>> +	 * plane takes precedence.
>>> +	 */
>> This seems confusing to me.
>>
>> What you seem to be saying is that the "destination" plane would be the
>> one which is (eg0 the graphic plane, and the "source" plane would be the
>> the plane containing (eg) the video.  You seem to be saying that the
>> colorkey matches the video and determines whether the pixels in the
>> graphic plane are opaque or transparent.
> Your example is correct.
>
> With the "plane_mask" property we can specify any plane as the "source" for 
> color key, so it can been either a video plane or graphic plane and even both 
> at the same time.
I'm not sure we should specify plane mask from userspace.

Can't we make major loops? How do you want to handle those?
>> Surely that is the wrong way round - in video overlay, you want to
>> colorkey match the contents of the graphic plane to determine which
>> pixels from the video plane to overlay.
> The "transparent" mode makes the color-matched pixels to become transparent, 
> you want the inversion effect and hence that should be called something like a 
> "transparent-inverted" mode. Maarten Lankhorst was asking for that mode in his 
> comment to v3, I'm leaving for somebody else to add that mode later since 
> there is no real use for it on Tegra right now.
I would like it to be described and included, so I can convert the existing intel_sprite_set_colorkey_ioctl to atomic.

Then again, could transparent-inverted also be handled by setting transparent on the primary?


> So in your case the graphic plane will be the "source" plane (specified via 
> the colorkey.plane_mask property), video plane will be the "destination" plane 
> (plane to which the colorkey properties are applied) and the colorkey.mode 
> will be "transparent-inverted". Pixels of the "source" plane are being matched 
> and "destination" plane takes the effect of color keying operation, i.e. the 
> color-matched pixels of graphic plane leave the video plane pixels unaffected 
> and the unmatched pixels make the video plane pixels transparent.
>
>> If it's the other way around (source is the graphic, destination is the
>> video) it makes less sense to use the "source" and "destination" terms,
>> I can't see how you could describe a plane that is being overlaid on
>> top of another plane as a "destination".
> Tegra has a bit annoying limitations in regard to a reduced plane blending 
> functionality when color keying is enabled. I found that the best variant to 
> work around the limitations is to move the graphic plane on top of the video 
> plane and to make the graphic plane to match itself. I.e. the matched pixels 
> of graphic plane become transparent and hence poked by video plane.
>
>> I guess the terminology has come from a thought about using a GPU to
>> physically do the colorkeying when combining two planes - if the GPU
>> were to write to the "destination" plane, then this would be the wrong
>> way around.  For starters, taking the above example, the video plane
>> may well be smaller than the graphic plane.  If it's the other way
>> around, that has other problems, like destroying the colorkey in the
>> graphic plane when writing the video plane's contents to it.
> It all depends on a use-case scenario. It won't be easy for userspace to 
> generalize the usage of color keying, at best the color keying interface could 
> be generalized and then userspace may choose the best fitting variant based on 
> available HW capabilities.
There's TEST_ONLY for a reason, though I guess it makes generic color keying slightly more invovled for userspace. :)
>> So, in summary, I don't think "destination" and "source" are
>> particularly good terms to describe the operation, and I think you have
>> them swapped in your description of
>> "DRM_PLANE_COLORKEY_MODE_TRANSPARENT".
> Maybe the DRM_PLANE_COLORKEY_MODE_TRANSPARENT should become 
> DRM_PLANE_COLORKEY_MODE_TRANSPARENT_INVERTED? 
>
> Any more opinions?
>
>
>