Re: [PATCH 2/2] drm: rcar-du: Add support for colorkey alpha blending

2017-05-07 Thread Gheorghe, Alexandru
Hi Laurent,

On Thursday, May 4, 2017 3:44 PM Laurent Pinchart wrote: 
> Hi Alexandru,
>
> On Thursday 04 May 2017 13:53:33 agheorghe wrote:
>> Add two new plane properties colorkey and colorkey_alpha for rcar gen3.
>> * colorkey:
>>   - used for specifying the color on which the filtering is done.
>>   - bits 0 to 23 are interpreted as RGB888 format, in case we are
>> dealing with an YCbCr format, only the Y componenet is
>> compared and it is represented by the G bits from RGB888
>> format.
>>   - bit 24 tells if it is enabled or not.
>> * colorkey_alpha:
>>   - the alpha to be set for matching pixels, in case it is
>> missing the pixels will be made transparent
>
> Colour keying is a feature found in most display engines, and would thus
> benefit from standardizing the properties. Instead of adding another colorkey
> property specific to rcar-du, could you make a proposal to standardize it ?

Yeah sure, I could give it a try and comeback with a RFC. By standardizing it
I'm assuming you mean something similar with what we have now in drm_blend,
with zpos and rotate properties, right? Or do you think it should get its own 
IOCTL? 

>> Signed-off-by: agheorghe 
>> ---
>>  drivers/gpu/drm/rcar-du/rcar_du_drv.h   |  1 +
>>  drivers/gpu/drm/rcar-du/rcar_du_kms.c   |  8 
>>  drivers/gpu/drm/rcar-du/rcar_du_plane.c |  3 ---
>>  drivers/gpu/drm/rcar-du/rcar_du_plane.h |  6 ++
>>  drivers/gpu/drm/rcar-du/rcar_du_vsp.c   | 22 ++
>>  drivers/gpu/drm/rcar-du/rcar_du_vsp.h   |  5 +
>>  6 files changed, 42 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
>> b/drivers/gpu/drm/rcar-du/rcar_du_drv.h index 91e8fc5..1cb92e3 100644
>> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
>> @@ -98,6 +98,7 @@ struct rcar_du_device {
>>   struct {
>>   struct drm_property *alpha;
>>   struct drm_property *colorkey;
>> + struct drm_property *colorkey_alpha;
>>   } props;
>>
>>   unsigned int dpad0_source;
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
>> b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index 1cc88ed..a733fa2 100644
>> --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
>> @@ -630,6 +630,14 @@ static int rcar_du_properties_init(struct
>> rcar_du_device *rcdu) if (rcdu->props.colorkey == NULL)
>>   return -ENOMEM;
>>
>> + if (rcdu->info->gen == 3) {
>> + rcdu->props.colorkey_alpha =
>> + drm_property_create_range(rcdu->ddev, 0,
>> +   "colorkey_alpha", 0, 255);
>> + if (!rcdu->props.colorkey_alpha)
>> + return -ENOMEM;
>> + }
>> +
>>   return 0;
>>  }
>>
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
>> b/drivers/gpu/drm/rcar-du/rcar_du_plane.c index e408aa3..df689c4 100644
>> --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
>> @@ -307,9 +307,6 @@ int rcar_du_atomic_check_planes(struct drm_device *dev,
>>   * Plane Setup
>>   */
>>
>> -#define RCAR_DU_COLORKEY_NONE(0 << 24)
>> -#define RCAR_DU_COLORKEY_SOURCE  (1 << 24)
>> -#define RCAR_DU_COLORKEY_MASK(1 << 24)
>>
>>  static void rcar_du_plane_write(struct rcar_du_group *rgrp,
>>   unsigned int index, u32 reg, u32 data)
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.h
>> b/drivers/gpu/drm/rcar-du/rcar_du_plane.h index c1de338..9e7c3b6 100644
>> --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.h
>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.h
>> @@ -49,6 +49,12 @@ static inline struct rcar_du_plane *to_rcar_plane(struct
>> drm_plane *plane) return container_of(plane, struct rcar_du_plane, plane);
>>  }
>>
>> +#define RCAR_DU_COLORKEY_NONE(0 << 24)
>> +#define RCAR_DU_COLORKEY_MASKBIT(24)
>> +#define RCAR_DU_COLORKEY_EN_MASK RCAR_DU_COLORKEY_MASK
>> +#define RCAR_DU_COLORKEY_COLOR_MASK  0xFF
>> +#define RCAR_DU_COLORKEY_ALPHA_MASK  0xFF
>> +
>>  /**
>>   * struct rcar_du_plane_state - Driver-specific plane state
>>   * @state: base DRM plane state
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
>> b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c index 4b460d4..b223be1 100644
>> --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
>> @@ -180,6 +180,11 @@ static void rcar_du_vsp_plane_setup(struct
>> rcar_du_vsp_plane *plane) .pitch = fb->pitches[0],
>>   .alpha = state->alpha,
>>   .zpos = state->state.zpos,
>> + .colorkey = state->colorkey & RCAR_DU_COLORKEY_COLOR_MASK,
>> + .colorkey_en =
>> + ((state->colorkey & RCAR_DU_COLORKEY_EN_MASK) != 0),
>> + .colorkey_alpha =
>> +   

Re: [PATCH 2/2] drm: rcar-du: Add support for colorkey alpha blending

2017-05-04 Thread Laurent Pinchart
Hi Alexandru,

On Thursday 04 May 2017 13:53:33 agheorghe wrote:
> Add two new plane properties colorkey and colorkey_alpha for rcar gen3.
> * colorkey:
>   - used for specifying the color on which the filtering is done.
>   - bits 0 to 23 are interpreted as RGB888 format, in case we are
> dealing with an YCbCr format, only the Y componenet is
> compared and it is represented by the G bits from RGB888
> format.
>   - bit 24 tells if it is enabled or not.
> * colorkey_alpha:
>   - the alpha to be set for matching pixels, in case it is
> missing the pixels will be made transparent

Colour keying is a feature found in most display engines, and would thus 
benefit from standardizing the properties. Instead of adding another colorkey 
property specific to rcar-du, could you make a proposal to standardize it ?

> Signed-off-by: agheorghe 
> ---
>  drivers/gpu/drm/rcar-du/rcar_du_drv.h   |  1 +
>  drivers/gpu/drm/rcar-du/rcar_du_kms.c   |  8 
>  drivers/gpu/drm/rcar-du/rcar_du_plane.c |  3 ---
>  drivers/gpu/drm/rcar-du/rcar_du_plane.h |  6 ++
>  drivers/gpu/drm/rcar-du/rcar_du_vsp.c   | 22 ++
>  drivers/gpu/drm/rcar-du/rcar_du_vsp.h   |  5 +
>  6 files changed, 42 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> b/drivers/gpu/drm/rcar-du/rcar_du_drv.h index 91e8fc5..1cb92e3 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> @@ -98,6 +98,7 @@ struct rcar_du_device {
>   struct {
>   struct drm_property *alpha;
>   struct drm_property *colorkey;
> + struct drm_property *colorkey_alpha;
>   } props;
> 
>   unsigned int dpad0_source;
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index 1cc88ed..a733fa2 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> @@ -630,6 +630,14 @@ static int rcar_du_properties_init(struct
> rcar_du_device *rcdu) if (rcdu->props.colorkey == NULL)
>   return -ENOMEM;
> 
> + if (rcdu->info->gen == 3) {
> + rcdu->props.colorkey_alpha =
> + drm_property_create_range(rcdu->ddev, 0,
> +   "colorkey_alpha", 0, 255);
> + if (!rcdu->props.colorkey_alpha)
> + return -ENOMEM;
> + }
> +
>   return 0;
>  }
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> b/drivers/gpu/drm/rcar-du/rcar_du_plane.c index e408aa3..df689c4 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> @@ -307,9 +307,6 @@ int rcar_du_atomic_check_planes(struct drm_device *dev,
>   * Plane Setup
>   */
> 
> -#define RCAR_DU_COLORKEY_NONE(0 << 24)
> -#define RCAR_DU_COLORKEY_SOURCE  (1 << 24)
> -#define RCAR_DU_COLORKEY_MASK(1 << 24)
> 
>  static void rcar_du_plane_write(struct rcar_du_group *rgrp,
>   unsigned int index, u32 reg, u32 data)
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.h
> b/drivers/gpu/drm/rcar-du/rcar_du_plane.h index c1de338..9e7c3b6 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.h
> @@ -49,6 +49,12 @@ static inline struct rcar_du_plane *to_rcar_plane(struct
> drm_plane *plane) return container_of(plane, struct rcar_du_plane, plane);
>  }
> 
> +#define RCAR_DU_COLORKEY_NONE(0 << 24)
> +#define RCAR_DU_COLORKEY_MASKBIT(24)
> +#define RCAR_DU_COLORKEY_EN_MASK RCAR_DU_COLORKEY_MASK
> +#define RCAR_DU_COLORKEY_COLOR_MASK  0xFF
> +#define RCAR_DU_COLORKEY_ALPHA_MASK  0xFF
> +
>  /**
>   * struct rcar_du_plane_state - Driver-specific plane state
>   * @state: base DRM plane state
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c index 4b460d4..b223be1 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> @@ -180,6 +180,11 @@ static void rcar_du_vsp_plane_setup(struct
> rcar_du_vsp_plane *plane) .pitch = fb->pitches[0],
>   .alpha = state->alpha,
>   .zpos = state->state.zpos,
> + .colorkey = state->colorkey & RCAR_DU_COLORKEY_COLOR_MASK,
> + .colorkey_en =
> + ((state->colorkey & RCAR_DU_COLORKEY_EN_MASK) != 0),
> + .colorkey_alpha =
> + (state->colorkey_alpha & RCAR_DU_COLORKEY_ALPHA_MASK),
>   };
>   unsigned int i;
> 
> @@ -379,6 +384,8 @@ static void rcar_du_vsp_plane_reset(struct drm_plane
> *plane) return;
> 
>   state->alpha = 255;
> + state->colorkey = RCAR_DU_COLORKEY_NONE;
> + state->colorkey_alpha = 0;
>   state->state.zpos = plane->type == DRM_PLANE_TYPE_PRIMARY ? 0 : 1;
> 
>   plane->state = 

[PATCH 2/2] drm: rcar-du: Add support for colorkey alpha blending

2017-05-04 Thread agheorghe
Add two new plane properties colorkey and colorkey_alpha for rcar gen3.
* colorkey:
- used for specifying the color on which the filtering is done.
- bits 0 to 23 are interpreted as RGB888 format, in case we are
  dealing with an YCbCr format, only the Y componenet is
  compared and it is represented by the G bits from RGB888
  format.
- bit 24 tells if it is enabled or not.
* colorkey_alpha:
- the alpha to be set for matching pixels, in case it is
  missing the pixels will be made transparent

Signed-off-by: agheorghe 
---
 drivers/gpu/drm/rcar-du/rcar_du_drv.h   |  1 +
 drivers/gpu/drm/rcar-du/rcar_du_kms.c   |  8 
 drivers/gpu/drm/rcar-du/rcar_du_plane.c |  3 ---
 drivers/gpu/drm/rcar-du/rcar_du_plane.h |  6 ++
 drivers/gpu/drm/rcar-du/rcar_du_vsp.c   | 22 ++
 drivers/gpu/drm/rcar-du/rcar_du_vsp.h   |  5 +
 6 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h 
b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
index 91e8fc5..1cb92e3 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
@@ -98,6 +98,7 @@ struct rcar_du_device {
struct {
struct drm_property *alpha;
struct drm_property *colorkey;
+   struct drm_property *colorkey_alpha;
} props;
 
unsigned int dpad0_source;
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c 
b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
index 1cc88ed..a733fa2 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
@@ -630,6 +630,14 @@ static int rcar_du_properties_init(struct rcar_du_device 
*rcdu)
if (rcdu->props.colorkey == NULL)
return -ENOMEM;
 
+   if (rcdu->info->gen == 3) {
+   rcdu->props.colorkey_alpha =
+   drm_property_create_range(rcdu->ddev, 0,
+ "colorkey_alpha", 0, 255);
+   if (!rcdu->props.colorkey_alpha)
+   return -ENOMEM;
+   }
+
return 0;
 }
 
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c 
b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
index e408aa3..df689c4 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
@@ -307,9 +307,6 @@ int rcar_du_atomic_check_planes(struct drm_device *dev,
  * Plane Setup
  */
 
-#define RCAR_DU_COLORKEY_NONE  (0 << 24)
-#define RCAR_DU_COLORKEY_SOURCE(1 << 24)
-#define RCAR_DU_COLORKEY_MASK  (1 << 24)
 
 static void rcar_du_plane_write(struct rcar_du_group *rgrp,
unsigned int index, u32 reg, u32 data)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.h 
b/drivers/gpu/drm/rcar-du/rcar_du_plane.h
index c1de338..9e7c3b6 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_plane.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.h
@@ -49,6 +49,12 @@ static inline struct rcar_du_plane *to_rcar_plane(struct 
drm_plane *plane)
return container_of(plane, struct rcar_du_plane, plane);
 }
 
+#define RCAR_DU_COLORKEY_NONE  (0 << 24)
+#define RCAR_DU_COLORKEY_MASK  BIT(24)
+#define RCAR_DU_COLORKEY_EN_MASK   RCAR_DU_COLORKEY_MASK
+#define RCAR_DU_COLORKEY_COLOR_MASK0xFF
+#define RCAR_DU_COLORKEY_ALPHA_MASK0xFF
+
 /**
  * struct rcar_du_plane_state - Driver-specific plane state
  * @state: base DRM plane state
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c 
b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
index 4b460d4..b223be1 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
@@ -180,6 +180,11 @@ static void rcar_du_vsp_plane_setup(struct 
rcar_du_vsp_plane *plane)
.pitch = fb->pitches[0],
.alpha = state->alpha,
.zpos = state->state.zpos,
+   .colorkey = state->colorkey & RCAR_DU_COLORKEY_COLOR_MASK,
+   .colorkey_en =
+   ((state->colorkey & RCAR_DU_COLORKEY_EN_MASK) != 0),
+   .colorkey_alpha =
+   (state->colorkey_alpha & RCAR_DU_COLORKEY_ALPHA_MASK),
};
unsigned int i;
 
@@ -379,6 +384,8 @@ static void rcar_du_vsp_plane_reset(struct drm_plane *plane)
return;
 
state->alpha = 255;
+   state->colorkey = RCAR_DU_COLORKEY_NONE;
+   state->colorkey_alpha = 0;
state->state.zpos = plane->type == DRM_PLANE_TYPE_PRIMARY ? 0 : 1;
 
plane->state = >state;
@@ -394,6 +401,10 @@ static int rcar_du_vsp_plane_atomic_set_property(struct 
drm_plane *plane,
 
if (property == rcdu->props.alpha)
rstate->alpha = val;
+   else if (property == rcdu->props.colorkey)
+   rstate->colorkey = val;
+   else if (property == rcdu->props.colorkey_alpha)
+   rstate->colorkey_alpha = val;