Re: [RFC PATCH v4 1/2] drm: Add generic colorkey properties for display planes
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
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
Signed-off-by: Dmitry Osipenko
---
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.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH v4 1/2] drm: Add generic colorkey properties for display planes
On 8/16/18 2:42 PM, Maarten Lankhorst wrote: 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. It looks like a quite flexible approach. Do you have any other suggestions? Can't we make major loops? How do you want to handle those? It's up to a specific driver to accept the mask or reject it. You could make a loop if HW allows to do that, I don't see what's the problem. 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. Okay, I can add it. Though probably better to call that mode "opaque" rather than "transparent-inverted". Then again, could transparent-inverted also be handled by setting transparent on the primary? If HW allows to do that, then yes. 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. :) It is also quite involved for kernel to present a non-standard feature as something generic, pleasing everyone in the same time. ___ dri-devel
Re: [RFC PATCH v4 1/2] drm: Add generic colorkey properties for display planes
Hello Laurent,
Sorry for the late response.
On 8/14/18 12:48 PM, 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
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
Signed-off-by: Dmitry Osipenko
---
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.
I'll add couple more words.
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 ?
The reasons for that property:
1) HW limitations. IIUC, some of the Intel HW has a limitation such that only a
subset of planes could participate in the color keying. That should be also the
case for Tegra and others.
Drivers know all available HW capabilities, hence they know exactly what could
be handled.
2) Flexibility. This gives userspace more variants of how color keying could be
performed.
Whether it's the best option is questionable, I don't have better ideas for now.
+ * colorkey.mode:
+ * The mode is an enumerated property that controls how color keying
+ * operates.
A link to the drm_plane_colorkey_mode enum documentation would be useful.
Okay.
+ * colorkey.mask:
+ * This property specifies the pixel components mask. Unmasked pixel
+ * components are not participating in the matching. This mask value is
+ * applied to colorkey.min / max values. The mask value is given in a
+ * 64-bit integer in ARGB16161616 format, where A is the alpha value and
+ * R, G and B correspond to the color components. Drivers shall convert
+ * ARGB16161616 value into appropriate format within planes atomic check.
+ *
+ * Drivers return an error from their plane atomic check if mask can't be
+ * handled.
+
Re: [RFC PATCH v4 1/2] drm: Add generic colorkey properties for display planes
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? > > > ___ dri-devel mailing list
Re: [RFC PATCH v4 1/2] drm: Add generic colorkey properties for display planes
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
> >
> > 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
> > Signed-off-by: Dmitry Osipenko
> > ---
> > 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. Since the reference userspace cannot
answer how the new uapi should work in all corner-cases (failures and
stuff like that).
-Daniel
>
> > + * colorkey.mode:
> > + * The mode is an enumerated property that controls how color keying
> > + * operates.
>
> A link to the drm_plane_colorkey_mode enum documentation would be useful.
>
> > + * colorkey.mask:
> > + * This property specifies the pixel components mask. Unmasked pixel
> > + * components are not participating in the matching. This mask value is
> > + * applied to colorkey.min / max values. The mask value is given in a
> > + * 64-bit integer in ARGB16161616 format, where A is the alpha value and
> > + * R, G and B correspond to the color components. Drivers shall convert
> > + * ARGB16161616 value into appropriate format within planes atomic check.
> > + *
> > + * Drivers return an error from their plane atomic check if
Re: [RFC PATCH v4 1/2] drm: Add generic colorkey properties for display planes
Hi Dmitry,
Thank you for the patch.
On Tuesday, 7 August 2018 20:22:01 EEST Dmitry Osipenko wrote:
> From: Laurent Pinchart
>
> 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
> Signed-off-by: Dmitry Osipenko
> ---
> 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 ?
> + * colorkey.mode:
> + * The mode is an enumerated property that controls how color keying
> + * operates.
A link to the drm_plane_colorkey_mode enum documentation would be useful.
> + * colorkey.mask:
> + * This property specifies the pixel components mask. Unmasked pixel
> + * components are not participating in the matching. This mask value is
> + * applied to colorkey.min / max values. The mask value is given in a
> + * 64-bit integer in ARGB16161616 format, where A is the alpha value and
> + * R, G and B correspond to the color components. Drivers shall convert
> + * ARGB16161616 value into appropriate format within planes atomic check.
> + *
> + * Drivers return an error from their plane atomic check if mask can't be
> + * handled.
> + *
> + * colorkey.min, colorkey.max:
> + * These two properties specify the colors that are treated as the color
> + * key. Pixel whose value is in the [min, max] range is the color key
> + * matching pixel. The minimum and maximum values are expressed as a
> + * 64-bit integer in ARGB16161616 format, where A is the alpha value and
> + * R, G and B correspond to the color components. Drivers shall convert
> + * ARGB16161616 value into appropriate format within planes atomic check.
> + *
Re: [RFC PATCH v4 1/2] drm: Add generic colorkey properties for display planes
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. > 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. 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. > 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? ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH v4 1/2] drm: Add generic colorkey properties for display planes
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. 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. 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". 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. 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". -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 13.8Mbps down 630kbps up According to speedtest.net: 13Mbps down 490kbps up ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
