lkml.org 
[lkml]   [2018]   [Sep]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [RFC PATCH v4 1/2] drm: Add generic colorkey properties for display planes
From
Date
On 8/14/18 1:38 PM, Daniel Vetter wrote:
> On Tue, Aug 14, 2018 at 12:48:08PM +0300, Laurent Pinchart wrote:
>> Hi Dmitry,
>>
>> Thank you for the patch.
>>
>> On Tuesday, 7 August 2018 20:22:01 EEST Dmitry Osipenko wrote:
>>> From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>>>
>>> Color keying is the action of replacing pixels matching a given color
>>> (or range of colors) with transparent pixels in an overlay when
>>> performing blitting. Depending on the hardware capabilities, the
>>> matching pixel can either become fully transparent or gain adjustment
>>> of the pixels component values.
>>>
>>> Color keying is found in a large number of devices whose capabilities
>>> often differ, but they still have enough common features in range to
>>> standardize color key properties. This commit adds new generic DRM plane
>>> properties related to the color keying, providing initial color keying
>>> support.
>>>
>>> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>> ---
>>> drivers/gpu/drm/drm_atomic.c | 20 +++++
>>> drivers/gpu/drm/drm_blend.c | 150 +++++++++++++++++++++++++++++++++++
>>> include/drm/drm_blend.h | 3 +
>>> include/drm/drm_plane.h | 91 +++++++++++++++++++++
>>> 4 files changed, 264 insertions(+)
>>
>> [snip]
>>
>>> diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
>>> index a16a74d7e15e..13c61dd0d9b7 100644
>>> --- a/drivers/gpu/drm/drm_blend.c
>>> +++ b/drivers/gpu/drm/drm_blend.c
>>> @@ -107,6 +107,11 @@
>>> * planes. Without this property the primary plane is always below the
>>> cursor * plane, and ordering between all other planes is undefined.
>>> *
>>> + * colorkey:
>>> + * Color keying is set up with drm_plane_create_colorkey_properties().
>>> + * It adds support for actions like replacing a range of colors with a
>>> + * transparent color in the plane. Color keying is disabled by default.
>>> + *
>>> * Note that all the property extensions described here apply either to the
>>> * plane or the CRTC (e.g. for the background color, which currently is not
>>> * exposed and assumed to be black).
>>> @@ -448,3 +453,148 @@ int drm_atomic_normalize_zpos(struct drm_device *dev,
>>> return 0;
>>> }
>>> EXPORT_SYMBOL(drm_atomic_normalize_zpos);
>>> +
>>> +static const char * const plane_colorkey_mode_name[] = {
>>> + [DRM_PLANE_COLORKEY_MODE_DISABLED] = "disabled",
>>> + [DRM_PLANE_COLORKEY_MODE_TRANSPARENT] = "transparent",
>>> +};
>>> +
>>> +/**
>>> + * drm_plane_create_colorkey_properties - create colorkey properties
>>> + * @plane: drm plane
>>> + * @supported_modes: bitmask of supported color keying modes
>>> + *
>>> + * This function creates the generic color keying properties and attaches
>>> them
>>> + * to the @plane to enable color keying control for blending operations.
>>> + *
>>> + * 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.
>>> + *
>>> + * Color keying is controlled by these properties:
>>> + *
>>> + * colorkey.plane_mask:
>>> + * The mask property specifies which planes participate in color key
>>> + * matching process, these planes are the color key sources.
>>> + *
>>> + * Drivers return an error from their plane atomic check if plane can't be
>>> + * handled.
>>
>> This seems fragile to me. We don't document how userspace determines which
>> planes need to be specified here, and we don't document what happens if a
>> plane underneath the destination plane is not specified in the mask. More
>> precise documentation is needed if we want to use such a property.
>>
>> It also seems quite complex. Is an explicit plane mask really the best option
>> ? What's the reason why planes couldn't be handled ? How do drivers determine
>> that ?
>
> General comment: This is why we need the reference userspace. I also
> think that any feature throwing up so many tricky questions should come
> with a full set of igt testcases.

At best I can write couple simple tests, simply because Tegra can't do more than
that.

Since the reference userspace cannot
> answer how the new uapi should work in all corner-cases (failures and
> stuff like that).

Okay.

\
 
 \ /
  Last update: 2018-09-20 18:47    [W:0.055 / U:12.512 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site