[PATCH] drm/tegra: Expose color key and plane blending controls to userspace

[Dmitry Osipenko]

On 02.09.2016 18:51, Thierry Reding wrote:
> On Fri, Sep 02, 2016 at 05:32:19PM +0200, Thierry Reding wrote:
>> On Fri, Sep 02, 2016 at 12:33:42PM +0300, Dmitry Osipenko wrote:
>>> Chromakey is a simple way of video overlay overlap implementation. This
>>> patch adds 2 new IOCTL's: first - sets color key and is common across of
>>> all Tegra SoC's, second - sets plane blending controls and allows to
>>> utilize the color key, this one is exclusive to Tegra20/30.
>>>
>>> Signed-off-by: Dmitry Osipenko 
>>> ---
>>>  drivers/gpu/drm/tegra/dc.c   | 150 +---
>>>  drivers/gpu/drm/tegra/dc.h   |   6 ++
>>>  drivers/gpu/drm/tegra/drm.c  | 159 
>>> +++
>>>  drivers/gpu/drm/tegra/drm.h  |  14 
>>>  include/uapi/drm/tegra_drm.h |  34 +
>>>  5 files changed, 337 insertions(+), 26 deletions(-)
>>
>> I think these are really nice features to have, but these would need to
>> be exposed as properties, rather than custom driver-specific IOCTLs. It
>> seems to me like the colorkey feature could be implemented in much the
>> same way as in the Armada, RCar and Nouveau drivers.
>>

Now, instead of custom driver-specific IOCTL's we would have custom
driver-specific DRM object properties. I'm not sure how they should be 
represented.

Should we assume that primary plane is the only source of colorkey? On Tegra,
colorkey isn't matched against the underlying plane, it represents area on the
plane (that uses that colorkey) that should be blended. In case of Xv, I have to
enable colorkey matching for the primary plane and make transparent areas of the
primary plane where colorkey matches and overlapped by the overlay, areas that
are overlapped and NOT matching colorkey are displayed on top of the overlay
(like mouse cursor or some window).

Should we assign colorkey (0xkey value itself) property per overlay plane? WIN_B
will use colokey0, WIN_C colorkey1.
Or make colorkey values property of CRTC? So it would be possible (probably not
so useful) to have 2 color keys used for one overlay.

Should colorkey be enabled/disabled via properties? I.e. it will be
"use_colorkey" plane property. Feels a bit icky to me.

I see only DRM_IOCTL_MODE_OBJ_SETPROPERTY, so no way to set all properties at 
once?

>> As for the blending options, I think they should be exposed in terms of
>> the zpos property, to allow generic userspace to make use of them. Also
>> can you explain why this needs to be exclusive to Tegra20 and Tegra30?

What about the actual alpha blending? What about having different blending 
modes?

Should we sacrifice it all and support only 1-2 use cases?

Given that properties are specific to each driver, I can hardly imagine such
generic userspace. Could you give an example?

> Ah... I just realized that the blending interface changed on Tegra124.
> All the more reason to expose this more generically, that way we can
> hide the differences between a property and support the same interface
> across all generations of Tegra.
> 
> Also see this:
> 
>   
> https://chromium.googlesource.com/chromiumos/third_party/kernel/+/de9295aabdb7f80555c9b77b29ac77bcdac3280b
> 

-- 
Dmitry

[PATCH] drm/tegra: Expose color key and plane blending controls to userspace

[Thierry Reding]

On Fri, Sep 02, 2016 at 05:32:19PM +0200, Thierry Reding wrote:
> On Fri, Sep 02, 2016 at 12:33:42PM +0300, Dmitry Osipenko wrote:
> > Chromakey is a simple way of video overlay overlap implementation. This
> > patch adds 2 new IOCTL's: first - sets color key and is common across of
> > all Tegra SoC's, second - sets plane blending controls and allows to
> > utilize the color key, this one is exclusive to Tegra20/30.
> > 
> > Signed-off-by: Dmitry Osipenko 
> > ---
> >  drivers/gpu/drm/tegra/dc.c   | 150 +---
> >  drivers/gpu/drm/tegra/dc.h   |   6 ++
> >  drivers/gpu/drm/tegra/drm.c  | 159 
> > +++
> >  drivers/gpu/drm/tegra/drm.h  |  14 
> >  include/uapi/drm/tegra_drm.h |  34 +
> >  5 files changed, 337 insertions(+), 26 deletions(-)
> 
> I think these are really nice features to have, but these would need to
> be exposed as properties, rather than custom driver-specific IOCTLs. It
> seems to me like the colorkey feature could be implemented in much the
> same way as in the Armada, RCar and Nouveau drivers.
> 
> As for the blending options, I think they should be exposed in terms of
> the zpos property, to allow generic userspace to make use of them. Also
> can you explain why this needs to be exclusive to Tegra20 and Tegra30?

Ah... I just realized that the blending interface changed on Tegra124.
All the more reason to expose this more generically, that way we can
hide the differences between a property and support the same interface
across all generations of Tegra.

Also see this:


https://chromium.googlesource.com/chromiumos/third_party/kernel/+/de9295aabdb7f80555c9b77b29ac77bcdac3280b

Thierry
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: 

[PATCH] drm/tegra: Expose color key and plane blending controls to userspace

[Thierry Reding]

On Fri, Sep 02, 2016 at 12:33:42PM +0300, Dmitry Osipenko wrote:
> Chromakey is a simple way of video overlay overlap implementation. This
> patch adds 2 new IOCTL's: first - sets color key and is common across of
> all Tegra SoC's, second - sets plane blending controls and allows to
> utilize the color key, this one is exclusive to Tegra20/30.
> 
> Signed-off-by: Dmitry Osipenko 
> ---
>  drivers/gpu/drm/tegra/dc.c   | 150 +---
>  drivers/gpu/drm/tegra/dc.h   |   6 ++
>  drivers/gpu/drm/tegra/drm.c  | 159 
> +++
>  drivers/gpu/drm/tegra/drm.h  |  14 
>  include/uapi/drm/tegra_drm.h |  34 +
>  5 files changed, 337 insertions(+), 26 deletions(-)

I think these are really nice features to have, but these would need to
be exposed as properties, rather than custom driver-specific IOCTLs. It
seems to me like the colorkey feature could be implemented in much the
same way as in the Armada, RCar and Nouveau drivers.

As for the blending options, I think they should be exposed in terms of
the zpos property, to allow generic userspace to make use of them. Also
can you explain why this needs to be exclusive to Tegra20 and Tegra30?

Thierry
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: 

[PATCH] drm/tegra: Expose color key and plane blending controls to userspace

[Dmitry Osipenko]

Chromakey is a simple way of video overlay overlap implementation. This
patch adds 2 new IOCTL's: first - sets color key and is common across of
all Tegra SoC's, second - sets plane blending controls and allows to
utilize the color key, this one is exclusive to Tegra20/30.

Signed-off-by: Dmitry Osipenko 
---
 drivers/gpu/drm/tegra/dc.c   | 150 +---
 drivers/gpu/drm/tegra/dc.h   |   6 ++
 drivers/gpu/drm/tegra/drm.c  | 159 +++
 drivers/gpu/drm/tegra/drm.h  |  14 
 include/uapi/drm/tegra_drm.h |  34 +
 5 files changed, 337 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index ddac53c..5956382 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -42,6 +42,11 @@ static inline struct tegra_plane *to_tegra_plane(struct 
drm_plane *plane)
return container_of(plane, struct tegra_plane, base);
 }

+struct tegra_dc_color_key {
+   u32 upper;
+   u32 lower;
+};
+
 struct tegra_dc_state {
struct drm_crtc_state base;

@@ -50,6 +55,9 @@ struct tegra_dc_state {
unsigned int div;

u32 planes;
+
+   struct tegra_dc_color_key color_key0;
+   struct tegra_dc_color_key color_key1;
 };

 static inline struct tegra_dc_state *to_dc_state(struct drm_crtc_state *state)
@@ -66,6 +74,11 @@ struct tegra_plane_state {
struct tegra_bo_tiling tiling;
u32 format;
u32 swap;
+   u32 blend_nokey;
+   u32 blend_1win;
+   u32 blend_2win_x;
+   u32 blend_2win_y;
+   u32 blend_3win_xy;
 };

 static inline struct tegra_plane_state *
@@ -77,6 +90,66 @@ to_tegra_plane_state(struct drm_plane_state *state)
return NULL;
 }

+void tegra_dc_set_color_key(struct drm_crtc_state *crtc_state,
+   int key_id, u32 upper, u32 lower)
+{
+   struct tegra_dc_state *state = to_dc_state(crtc_state);
+   struct tegra_dc_color_key *color_key;
+
+   if (key_id == 0)
+   color_key = >color_key0;
+   else
+   color_key = >color_key1;
+
+   color_key->lower = lower;
+   color_key->upper = upper;
+}
+
+void tegra20_dc_plane_set_blending(struct drm_plane_state *plane_state,
+  unsigned int blend_config,
+  unsigned int blend_control,
+  unsigned int blend_weight0,
+  unsigned int blend_weight1,
+  bool use_color_key0,
+  bool use_color_key1)
+{
+   struct tegra_plane_state *state = to_tegra_plane_state(plane_state);
+   u32 value;
+
+   if (blend_config == DRM_TEGRA_PLANE_BLEND_CONFIG_NOKEY) {
+   value = DC_WIN_BLEND_CONTROL_NOKEY(blend_control);
+   } else {
+   value = DC_WIN_BLEND_CONTROL(blend_control);
+
+   if (use_color_key0)
+   value |= DC_WIN_BLEND_CKEY0;
+
+   if (use_color_key1)
+   value |= DC_WIN_BLEND_CKEY1;
+   }
+
+   value |= DC_WIN_BLEND_WEIGHT0(blend_weight0);
+   value |= DC_WIN_BLEND_WEIGHT1(blend_weight1);
+
+   switch (blend_config) {
+   case DRM_TEGRA_PLANE_BLEND_CONFIG_NOKEY:
+   state->blend_nokey = value;
+   break;
+   case DRM_TEGRA_PLANE_BLEND_CONFIG_1WIN:
+   state->blend_1win = value;
+   break;
+   case DRM_TEGRA_PLANE_BLEND_CONFIG_2WIN_X:
+   state->blend_2win_x = value;
+   break;
+   case DRM_TEGRA_PLANE_BLEND_CONFIG_2WIN_Y:
+   state->blend_2win_y = value;
+   break;
+   case DRM_TEGRA_PLANE_BLEND_CONFIG_3WIN_XY:
+   state->blend_3win_xy = value;
+   break;
+   }
+}
+
 static void tegra_dc_stats_reset(struct tegra_dc_stats *stats)
 {
stats->frames = 0;
@@ -381,32 +454,11 @@ static void tegra_dc_setup_window(struct tegra_dc *dc, 
unsigned int index,

tegra_dc_writel(dc, value, DC_WIN_WIN_OPTIONS);

-   /*
-* Disable blending and assume Window A is the bottom-most window,
-* Window C is the top-most window and Window B is in the middle.
-*/
-   tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_NOKEY);
-   tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_1WIN);
-
-   switch (index) {
-   case 0:
-   tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_2WIN_X);
-   tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_2WIN_Y);
-   tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_3WIN_XY);
-   break;
-
-   case 1:
-   tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_2WIN_X);
-   tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_2WIN_Y);
-   tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_3WIN_XY);
-   break;
-
-   case 2:
-