[PATCH 0/2] drm: Alpha blending issue

[PATCH 0/2] drm: Alpha blending issue

[Kieran Bingham]

Testing on the RCar Salvaltor-XS using the rcar-du has identified that
commit 161ad653d6c9 ("drm: rcar-du: Use __drm_atomic_helper_plane_reset
instead of copying the logic") caused a regression in display output on
the primary planes.

The effect was that primary plane was 'invisible' though secondary
planes were displayed correctly.

This issue turned out to be a change in the initialisation of the
default alpha property value in the state object. A plane without an
alpha property would leave the alpha property unset (and thus at zero,
or 'transparent').

Two patches are thus presented, which both individually can fix this
regression - but both are suitable for integration.

The RCar-DU driver is updated to provide an alpha property on primary
planes, as it isn't unreasonable to be able to set the property there.
This alone would have the effect of ensuring that the default value was
set to the full opaque setting, and repair the regression.

As a separate patch, we also update __drm_atomic_helper_plane_reset()
call such that the alpha values are always initialised to a more
practical default value of DRM_BLEND_ALPHA_OPAQUE for all planes.


Kieran Bingham (2):
  drm/atomic: Initialise planes with opaque alpha values
  drm: rcar-du: Enable alpha property on primary planes

 drivers/gpu/drm/drm_atomic_helper.c     | 4 +---
 drivers/gpu/drm/rcar-du/rcar_du_plane.c | 7 ++++++-
 2 files changed, 7 insertions(+), 4 deletions(-)

-- 
2.17.1

[PATCH 0/2] drm: Alpha blending issue

[Kieran Bingham]

Testing on the RCar Salvaltor-XS using the rcar-du has identified that
commit 161ad653d6c9 ("drm: rcar-du: Use __drm_atomic_helper_plane_reset
instead of copying the logic") caused a regression in display output on
the primary planes.

The effect was that primary plane was 'invisible' though secondary
planes were displayed correctly.

This issue turned out to be a change in the initialisation of the
default alpha property value in the state object. A plane without an
alpha property would leave the alpha property unset (and thus at zero,
or 'transparent').

Two patches are thus presented, which both individually can fix this
regression - but both are suitable for integration.

The RCar-DU driver is updated to provide an alpha property on primary
planes, as it isn't unreasonable to be able to set the property there.
This alone would have the effect of ensuring that the default value was
set to the full opaque setting, and repair the regression.

As a separate patch, we also update __drm_atomic_helper_plane_reset()
call such that the alpha values are always initialised to a more
practical default value of DRM_BLEND_ALPHA_OPAQUE for all planes.


Kieran Bingham (2):
  drm/atomic: Initialise planes with opaque alpha values
  drm: rcar-du: Enable alpha property on primary planes

 drivers/gpu/drm/drm_atomic_helper.c     | 4 +---
 drivers/gpu/drm/rcar-du/rcar_du_plane.c | 7 ++++++-
 2 files changed, 7 insertions(+), 4 deletions(-)

-- 
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/2] drm/atomic: Initialise planes with opaque alpha values

[Kieran Bingham]

Planes without an alpha property, using __drm_atomic_helper_plane_reset
will have their plane state alpha initialised as zero, which represents
a transparent alpha.

If this value is then used for the plane, it may not be visible by
default, and thus doesn't represent a good initialisation state.

Update the default state->alpha value to DRM_BLEND_ALPHA_OPAQUE
unconditionally when the plane is reset.

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
 drivers/gpu/drm/drm_atomic_helper.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 3cf1aa132778..e49b22381048 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3569,9 +3569,7 @@ void __drm_atomic_helper_plane_reset(struct drm_plane *plane,
 	state->plane = plane;
 	state->rotation = DRM_MODE_ROTATE_0;
 
-	/* Reset the alpha value to fully opaque if it matters */
-	if (plane->alpha_property)
-		state->alpha = plane->alpha_property->values[1];
+	state->alpha = DRM_BLEND_ALPHA_OPAQUE;
 	state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
 
 	plane->state = state;
-- 
2.17.1

[PATCH 1/2] drm/atomic: Initialise planes with opaque alpha values

[Kieran Bingham]

Planes without an alpha property, using __drm_atomic_helper_plane_reset
will have their plane state alpha initialised as zero, which represents
a transparent alpha.

If this value is then used for the plane, it may not be visible by
default, and thus doesn't represent a good initialisation state.

Update the default state->alpha value to DRM_BLEND_ALPHA_OPAQUE
unconditionally when the plane is reset.

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
 drivers/gpu/drm/drm_atomic_helper.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 3cf1aa132778..e49b22381048 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3569,9 +3569,7 @@ void __drm_atomic_helper_plane_reset(struct drm_plane *plane,
 	state->plane = plane;
 	state->rotation = DRM_MODE_ROTATE_0;
 
-	/* Reset the alpha value to fully opaque if it matters */
-	if (plane->alpha_property)
-		state->alpha = plane->alpha_property->values[1];
+	state->alpha = DRM_BLEND_ALPHA_OPAQUE;
 	state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
 
 	plane->state = state;
-- 
2.17.1

[PATCH 2/2] drm: rcar-du: Enable alpha property on primary planes

[Kieran Bingham]

If the alpha property is not added to a plane, a default value will be
used, which can result in a non-visible layer if the alpha is
initialised as 0.

Provide an alpha blend property on all planes.

Fixes: 161ad653d6c9 ("drm: rcar-du: Use __drm_atomic_helper_plane_reset
instead of copying the logic")

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/rcar_du_plane.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
index 9e07758a755c..72399a19d8a6 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
@@ -783,13 +783,18 @@ int rcar_du_planes_init(struct rcar_du_group *rgrp)
 		drm_plane_helper_add(&plane->plane,
 				     &rcar_du_plane_helper_funcs);
 
+		/*
+		 * The alpha property needs to be initialised on all planes
+		 * to ensure the correct setting at the output.
+		 */
+		drm_plane_create_alpha_property(&plane->plane);
+
 		if (type == DRM_PLANE_TYPE_PRIMARY)
 			continue;
 
 		drm_object_attach_property(&plane->plane.base,
 					   rcdu->props.colorkey,
 					   RCAR_DU_COLORKEY_NONE);
-		drm_plane_create_alpha_property(&plane->plane);
 		drm_plane_create_zpos_property(&plane->plane, 1, 1, 7);
 	}
 
-- 
2.17.1

[PATCH 2/2] drm: rcar-du: Enable alpha property on primary planes

[Kieran Bingham]

If the alpha property is not added to a plane, a default value will be
used, which can result in a non-visible layer if the alpha is
initialised as 0.

Provide an alpha blend property on all planes.

Fixes: 161ad653d6c9 ("drm: rcar-du: Use __drm_atomic_helper_plane_reset
instead of copying the logic")

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/rcar_du_plane.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
index 9e07758a755c..72399a19d8a6 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
@@ -783,13 +783,18 @@ int rcar_du_planes_init(struct rcar_du_group *rgrp)
 		drm_plane_helper_add(&plane->plane,
 				     &rcar_du_plane_helper_funcs);
 
+		/*
+		 * The alpha property needs to be initialised on all planes
+		 * to ensure the correct setting at the output.
+		 */
+		drm_plane_create_alpha_property(&plane->plane);
+
 		if (type == DRM_PLANE_TYPE_PRIMARY)
 			continue;
 
 		drm_object_attach_property(&plane->plane.base,
 					   rcdu->props.colorkey,
 					   RCAR_DU_COLORKEY_NONE);
-		drm_plane_create_alpha_property(&plane->plane);
 		drm_plane_create_zpos_property(&plane->plane, 1, 1, 7);
 	}
 
-- 
2.17.1

Re: [PATCH 1/2] drm/atomic: Initialise planes with opaque alpha values

[Ville Syrjälä]

On Wed, Sep 19, 2018 at 04:56:58PM +0100, Kieran Bingham wrote:
> Planes without an alpha property, using __drm_atomic_helper_plane_reset
> will have their plane state alpha initialised as zero, which represents
> a transparent alpha.
> 
> If this value is then used for the plane, it may not be visible by
> default, and thus doesn't represent a good initialisation state.
> 
> Update the default state->alpha value to DRM_BLEND_ALPHA_OPAQUE
> unconditionally when the plane is reset.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 3cf1aa132778..e49b22381048 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3569,9 +3569,7 @@ void __drm_atomic_helper_plane_reset(struct drm_plane *plane,
>  	state->plane = plane;
>  	state->rotation = DRM_MODE_ROTATE_0;
>  
> -	/* Reset the alpha value to fully opaque if it matters */
> -	if (plane->alpha_property)
> -		state->alpha = plane->alpha_property->values[1];
> +	state->alpha = DRM_BLEND_ALPHA_OPAQUE;

I can't come up with a solid excuse for not initializing it always.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  	state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
>  
>  	plane->state = state;
> -- 
> 2.17.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel

Re: [PATCH 1/2] drm/atomic: Initialise planes with opaque alpha values

[Ville Syrjälä]

On Wed, Sep 19, 2018 at 04:56:58PM +0100, Kieran Bingham wrote:
> Planes without an alpha property, using __drm_atomic_helper_plane_reset
> will have their plane state alpha initialised as zero, which represents
> a transparent alpha.
> 
> If this value is then used for the plane, it may not be visible by
> default, and thus doesn't represent a good initialisation state.
> 
> Update the default state->alpha value to DRM_BLEND_ALPHA_OPAQUE
> unconditionally when the plane is reset.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 3cf1aa132778..e49b22381048 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3569,9 +3569,7 @@ void __drm_atomic_helper_plane_reset(struct drm_plane *plane,
>  	state->plane = plane;
>  	state->rotation = DRM_MODE_ROTATE_0;
>  
> -	/* Reset the alpha value to fully opaque if it matters */
> -	if (plane->alpha_property)
> -		state->alpha = plane->alpha_property->values[1];
> +	state->alpha = DRM_BLEND_ALPHA_OPAQUE;

I can't come up with a solid excuse for not initializing it always.

Reviewed-by: Ville Syrj�l� <ville.syrjala@linux.intel.com>

>  	state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
>  
>  	plane->state = state;
> -- 
> 2.17.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrj�l�
Intel

Re: [PATCH 1/2] drm/atomic: Initialise planes with opaque alpha values

[Alexandru-Cosmin Gheorghe]

Hi Kieran,


On Wed, Sep 19, 2018 at 07:15:45PM +0300, Ville Syrjälä wrote:
> On Wed, Sep 19, 2018 at 04:56:58PM +0100, Kieran Bingham wrote:
> > Planes without an alpha property, using __drm_atomic_helper_plane_reset
> > will have their plane state alpha initialised as zero, which represents
> > a transparent alpha.
> > 
> > If this value is then used for the plane, it may not be visible by
> > default, and thus doesn't represent a good initialisation state.
> > 
> > Update the default state->alpha value to DRM_BLEND_ALPHA_OPAQUE
> > unconditionally when the plane is reset.
> > 
> > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> > ---
> >  drivers/gpu/drm/drm_atomic_helper.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> > index 3cf1aa132778..e49b22381048 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -3569,9 +3569,7 @@ void __drm_atomic_helper_plane_reset(struct drm_plane *plane,
> >  	state->plane = plane;
> >  	state->rotation = DRM_MODE_ROTATE_0;
> >  
> > -	/* Reset the alpha value to fully opaque if it matters */
> > -	if (plane->alpha_property)
> > -		state->alpha = plane->alpha_property->values[1];
> > +	state->alpha = DRM_BLEND_ALPHA_OPAQUE;
> 
> I can't come up with a solid excuse for not initializing it always.
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Neither do I, so:
Reviewed-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com>

And thanks again.

I plan to push it tomorrow to drm-misc-next.

Now, I've seen the plane_reset patches in the pull request for drm-next
4.20, I wonder if someone could tell me what should I do to get this
patch on that train.

> 
> >  	state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
> >  
> >  	plane->state = state;
> > -- 
> > 2.17.1
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Ville Syrjälä
> Intel

-- 
Cheers,
Alex G

Re: [PATCH 1/2] drm/atomic: Initialise planes with opaque alpha values

[Alexandru-Cosmin Gheorghe]

Hi Kieran,


On Wed, Sep 19, 2018 at 07:15:45PM +0300, Ville Syrj�l� wrote:
> On Wed, Sep 19, 2018 at 04:56:58PM +0100, Kieran Bingham wrote:
> > Planes without an alpha property, using __drm_atomic_helper_plane_reset
> > will have their plane state alpha initialised as zero, which represents
> > a transparent alpha.
> > 
> > If this value is then used for the plane, it may not be visible by
> > default, and thus doesn't represent a good initialisation state.
> > 
> > Update the default state->alpha value to DRM_BLEND_ALPHA_OPAQUE
> > unconditionally when the plane is reset.
> > 
> > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> > ---
> >  drivers/gpu/drm/drm_atomic_helper.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> > index 3cf1aa132778..e49b22381048 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -3569,9 +3569,7 @@ void __drm_atomic_helper_plane_reset(struct drm_plane *plane,
> >  	state->plane = plane;
> >  	state->rotation = DRM_MODE_ROTATE_0;
> >  
> > -	/* Reset the alpha value to fully opaque if it matters */
> > -	if (plane->alpha_property)
> > -		state->alpha = plane->alpha_property->values[1];
> > +	state->alpha = DRM_BLEND_ALPHA_OPAQUE;
> 
> I can't come up with a solid excuse for not initializing it always.
> 
> Reviewed-by: Ville Syrj�l� <ville.syrjala@linux.intel.com>

Neither do I, so:
Reviewed-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com>

And thanks again.

I plan to push it tomorrow to drm-misc-next.

Now, I've seen the plane_reset patches in the pull request for drm-next
4.20, I wonder if someone could tell me what should I do to get this
patch on that train.

> 
> >  	state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
> >  
> >  	plane->state = state;
> > -- 
> > 2.17.1
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Ville Syrj�l�
> Intel

-- 
Cheers,
Alex G

Re: [PATCH 1/2] drm/atomic: Initialise planes with opaque alpha values

[Alexandru-Cosmin Gheorghe]

Hi Kieran,


On Wed, Sep 19, 2018 at 07:15:45PM +0300, Ville Syrjälä wrote:
> On Wed, Sep 19, 2018 at 04:56:58PM +0100, Kieran Bingham wrote:
> > Planes without an alpha property, using __drm_atomic_helper_plane_reset
> > will have their plane state alpha initialised as zero, which represents
> > a transparent alpha.
> > 
> > If this value is then used for the plane, it may not be visible by
> > default, and thus doesn't represent a good initialisation state.
> > 
> > Update the default state->alpha value to DRM_BLEND_ALPHA_OPAQUE
> > unconditionally when the plane is reset.
> > 
> > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> > ---
> >  drivers/gpu/drm/drm_atomic_helper.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> > index 3cf1aa132778..e49b22381048 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -3569,9 +3569,7 @@ void __drm_atomic_helper_plane_reset(struct drm_plane *plane,
> >  	state->plane = plane;
> >  	state->rotation = DRM_MODE_ROTATE_0;
> >  
> > -	/* Reset the alpha value to fully opaque if it matters */
> > -	if (plane->alpha_property)
> > -		state->alpha = plane->alpha_property->values[1];
> > +	state->alpha = DRM_BLEND_ALPHA_OPAQUE;
> 
> I can't come up with a solid excuse for not initializing it always.
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Neither do I, so:
Reviewed-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com>

And thanks again.

I plan to push it tomorrow to drm-misc-next.

Now, I've seen the plane_reset patches in the pull request for drm-next
4.20, I wonder if someone could tell me what should I do to get this
patch on that train.

> 
> >  	state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
> >  
> >  	plane->state = state;
> > -- 
> > 2.17.1
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Ville Syrjälä
> Intel

-- 
Cheers,
Alex G
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm/atomic: Initialise planes with opaque alpha values

[Kieran Bingham]

Hi Alexandru,

On 19/09/18 17:43, Alexandru-Cosmin Gheorghe wrote:
> Hi Kieran,
> 
> 
> On Wed, Sep 19, 2018 at 07:15:45PM +0300, Ville Syrjälä wrote:
>> On Wed, Sep 19, 2018 at 04:56:58PM +0100, Kieran Bingham wrote:
>>> Planes without an alpha property, using __drm_atomic_helper_plane_reset
>>> will have their plane state alpha initialised as zero, which represents
>>> a transparent alpha.
>>>
>>> If this value is then used for the plane, it may not be visible by
>>> default, and thus doesn't represent a good initialisation state.
>>>
>>> Update the default state->alpha value to DRM_BLEND_ALPHA_OPAQUE
>>> unconditionally when the plane is reset.
>>>
>>> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>>> ---
>>>  drivers/gpu/drm/drm_atomic_helper.c | 4 +---
>>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
>>> index 3cf1aa132778..e49b22381048 100644
>>> --- a/drivers/gpu/drm/drm_atomic_helper.c
>>> +++ b/drivers/gpu/drm/drm_atomic_helper.c
>>> @@ -3569,9 +3569,7 @@ void __drm_atomic_helper_plane_reset(struct drm_plane *plane,
>>>  	state->plane = plane;
>>>  	state->rotation = DRM_MODE_ROTATE_0;
>>>  
>>> -	/* Reset the alpha value to fully opaque if it matters */
>>> -	if (plane->alpha_property)
>>> -		state->alpha = plane->alpha_property->values[1];
>>> +	state->alpha = DRM_BLEND_ALPHA_OPAQUE;
>>
>> I can't come up with a solid excuse for not initializing it always.
>>
>> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Neither do I, so:
> Reviewed-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com>
> 
> And thanks again.
> 
> I plan to push it tomorrow to drm-misc-next.
> 
> Now, I've seen the plane_reset patches in the pull request for drm-next
> 4.20, I wonder if someone could tell me what should I do to get this
> patch on that train.

I've submitted a separate patch for the rcar-du which enables the alpha
property for the primary plane - and it incorporates a "Fixes:
161ad653d6c9 ("drm: rcar-du: Use __drm_atomic_helper_plane_reset
instead of copying the logic")" tag.

Technically that should be sufficient to get that fix into v4.19 I
believe ...

But if you feel that this patch should also be included - we could add
the same tag to this patch, and get it queued up for v4.19 fixes?

--
Regards

Kieran


> 
>>
>>>  	state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
>>>  
>>>  	plane->state = state;
>>> -- 
>>> 2.17.1
>>>
>>> _______________________________________________
>>> dri-devel mailing list
>>> dri-devel@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
>> -- 
>> Ville Syrjälä
>> Intel
> 

Re: [PATCH 1/2] drm/atomic: Initialise planes with opaque alpha values

[Kieran Bingham]

Hi Alexandru,

On 19/09/18 17:43, Alexandru-Cosmin Gheorghe wrote:
> Hi Kieran,
> 
> 
> On Wed, Sep 19, 2018 at 07:15:45PM +0300, Ville Syrjälä wrote:
>> On Wed, Sep 19, 2018 at 04:56:58PM +0100, Kieran Bingham wrote:
>>> Planes without an alpha property, using __drm_atomic_helper_plane_reset
>>> will have their plane state alpha initialised as zero, which represents
>>> a transparent alpha.
>>>
>>> If this value is then used for the plane, it may not be visible by
>>> default, and thus doesn't represent a good initialisation state.
>>>
>>> Update the default state->alpha value to DRM_BLEND_ALPHA_OPAQUE
>>> unconditionally when the plane is reset.
>>>
>>> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>>> ---
>>>  drivers/gpu/drm/drm_atomic_helper.c | 4 +---
>>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
>>> index 3cf1aa132778..e49b22381048 100644
>>> --- a/drivers/gpu/drm/drm_atomic_helper.c
>>> +++ b/drivers/gpu/drm/drm_atomic_helper.c
>>> @@ -3569,9 +3569,7 @@ void __drm_atomic_helper_plane_reset(struct drm_plane *plane,
>>>  	state->plane = plane;
>>>  	state->rotation = DRM_MODE_ROTATE_0;
>>>  
>>> -	/* Reset the alpha value to fully opaque if it matters */
>>> -	if (plane->alpha_property)
>>> -		state->alpha = plane->alpha_property->values[1];
>>> +	state->alpha = DRM_BLEND_ALPHA_OPAQUE;
>>
>> I can't come up with a solid excuse for not initializing it always.
>>
>> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Neither do I, so:
> Reviewed-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com>
> 
> And thanks again.
> 
> I plan to push it tomorrow to drm-misc-next.
> 
> Now, I've seen the plane_reset patches in the pull request for drm-next
> 4.20, I wonder if someone could tell me what should I do to get this
> patch on that train.

I've submitted a separate patch for the rcar-du which enables the alpha
property for the primary plane - and it incorporates a "Fixes:
161ad653d6c9 ("drm: rcar-du: Use __drm_atomic_helper_plane_reset
instead of copying the logic")" tag.

Technically that should be sufficient to get that fix into v4.19 I
believe ...

But if you feel that this patch should also be included - we could add
the same tag to this patch, and get it queued up for v4.19 fixes?

--
Regards

Kieran


> 
>>
>>>  	state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
>>>  
>>>  	plane->state = state;
>>> -- 
>>> 2.17.1
>>>
>>> _______________________________________________
>>> dri-devel mailing list
>>> dri-devel@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
>> -- 
>> Ville Syrjälä
>> Intel
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm/atomic: Initialise planes with opaque alpha values

[Alexandru-Cosmin Gheorghe]

On Thu, Sep 20, 2018 at 11:03:12AM +0100, Kieran Bingham wrote:
> Hi Alexandru,
> 
> On 19/09/18 17:43, Alexandru-Cosmin Gheorghe wrote:
> > Hi Kieran,
> > 
> > 
> > On Wed, Sep 19, 2018 at 07:15:45PM +0300, Ville Syrjälä wrote:
> >> On Wed, Sep 19, 2018 at 04:56:58PM +0100, Kieran Bingham wrote:
> >>> Planes without an alpha property, using __drm_atomic_helper_plane_reset
> >>> will have their plane state alpha initialised as zero, which represents
> >>> a transparent alpha.
> >>>
> >>> If this value is then used for the plane, it may not be visible by
> >>> default, and thus doesn't represent a good initialisation state.
> >>>
> >>> Update the default state->alpha value to DRM_BLEND_ALPHA_OPAQUE
> >>> unconditionally when the plane is reset.
> >>>
> >>> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> >>> ---
> >>>  drivers/gpu/drm/drm_atomic_helper.c | 4 +---
> >>>  1 file changed, 1 insertion(+), 3 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> >>> index 3cf1aa132778..e49b22381048 100644
> >>> --- a/drivers/gpu/drm/drm_atomic_helper.c
> >>> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> >>> @@ -3569,9 +3569,7 @@ void __drm_atomic_helper_plane_reset(struct drm_plane *plane,
> >>>  	state->plane = plane;
> >>>  	state->rotation = DRM_MODE_ROTATE_0;
> >>>  
> >>> -	/* Reset the alpha value to fully opaque if it matters */
> >>> -	if (plane->alpha_property)
> >>> -		state->alpha = plane->alpha_property->values[1];
> >>> +	state->alpha = DRM_BLEND_ALPHA_OPAQUE;
> >>
> >> I can't come up with a solid excuse for not initializing it always.
> >>
> >> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Neither do I, so:
> > Reviewed-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com>
> > 
> > And thanks again.
> > 
> > I plan to push it tomorrow to drm-misc-next.
> > 
> > Now, I've seen the plane_reset patches in the pull request for drm-next
> > 4.20, I wonder if someone could tell me what should I do to get this
> > patch on that train.
> 
> I've submitted a separate patch for the rcar-du which enables the alpha
> property for the primary plane - and it incorporates a "Fixes:
> 161ad653d6c9 ("drm: rcar-du: Use __drm_atomic_helper_plane_reset
> instead of copying the logic")" tag.
> 
> Technically that should be sufficient to get that fix into v4.19 I
> believe ...

plane_reset patches are not in v4.19, but in drm-next which as far as
I know will be sent for v4.20

> 
> But if you feel that this patch should also be included - we could add
> the same tag to this patch, and get it queued up for v4.19 fixes?

Looking here 
https://01.org/linuxgraphics/gfx-docs/maintainer-tools/repositories.html#drm-misc-next
It seems that the last pull request for v4.20 of drm-misc-next will be
sent around rc6, so I think putting it in drm-misc-next will be enough
to get this with the rest of the plane_reset patch in v4.20,  but just
to make sure I will double check with Sean on IRC.


> 
> --
> Regards
> 
> Kieran
> 
> 
> > 
> >>
> >>>  	state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
> >>>  
> >>>  	plane->state = state;
> >>> -- 
> >>> 2.17.1
> >>>
> >>> _______________________________________________
> >>> dri-devel mailing list
> >>> dri-devel@lists.freedesktop.org
> >>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >>
> >> -- 
> >> Ville Syrjälä
> >> Intel
> > 

-- 
Cheers,
Alex G

Re: [PATCH 1/2] drm/atomic: Initialise planes with opaque alpha values

[Alexandru-Cosmin Gheorghe]

On Thu, Sep 20, 2018 at 11:03:12AM +0100, Kieran Bingham wrote:
> Hi Alexandru,
> 
> On 19/09/18 17:43, Alexandru-Cosmin Gheorghe wrote:
> > Hi Kieran,
> > 
> > 
> > On Wed, Sep 19, 2018 at 07:15:45PM +0300, Ville Syrj�l� wrote:
> >> On Wed, Sep 19, 2018 at 04:56:58PM +0100, Kieran Bingham wrote:
> >>> Planes without an alpha property, using __drm_atomic_helper_plane_reset
> >>> will have their plane state alpha initialised as zero, which represents
> >>> a transparent alpha.
> >>>
> >>> If this value is then used for the plane, it may not be visible by
> >>> default, and thus doesn't represent a good initialisation state.
> >>>
> >>> Update the default state->alpha value to DRM_BLEND_ALPHA_OPAQUE
> >>> unconditionally when the plane is reset.
> >>>
> >>> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> >>> ---
> >>>  drivers/gpu/drm/drm_atomic_helper.c | 4 +---
> >>>  1 file changed, 1 insertion(+), 3 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> >>> index 3cf1aa132778..e49b22381048 100644
> >>> --- a/drivers/gpu/drm/drm_atomic_helper.c
> >>> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> >>> @@ -3569,9 +3569,7 @@ void __drm_atomic_helper_plane_reset(struct drm_plane *plane,
> >>>  	state->plane = plane;
> >>>  	state->rotation = DRM_MODE_ROTATE_0;
> >>>  
> >>> -	/* Reset the alpha value to fully opaque if it matters */
> >>> -	if (plane->alpha_property)
> >>> -		state->alpha = plane->alpha_property->values[1];
> >>> +	state->alpha = DRM_BLEND_ALPHA_OPAQUE;
> >>
> >> I can't come up with a solid excuse for not initializing it always.
> >>
> >> Reviewed-by: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> > 
> > Neither do I, so:
> > Reviewed-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com>
> > 
> > And thanks again.
> > 
> > I plan to push it tomorrow to drm-misc-next.
> > 
> > Now, I've seen the plane_reset patches in the pull request for drm-next
> > 4.20, I wonder if someone could tell me what should I do to get this
> > patch on that train.
> 
> I've submitted a separate patch for the rcar-du which enables the alpha
> property for the primary plane - and it incorporates a "Fixes:
> 161ad653d6c9 ("drm: rcar-du: Use __drm_atomic_helper_plane_reset
> instead of copying the logic")" tag.
> 
> Technically that should be sufficient to get that fix into v4.19 I
> believe ...

plane_reset patches are not in v4.19, but in drm-next which as far as
I know will be sent for v4.20

> 
> But if you feel that this patch should also be included - we could add
> the same tag to this patch, and get it queued up for v4.19 fixes?

Looking here 
https://01.org/linuxgraphics/gfx-docs/maintainer-tools/repositories.html#drm-misc-next
It seems that the last pull request for v4.20 of drm-misc-next will be
sent around rc6, so I think putting it in drm-misc-next will be enough
to get this with the rest of the plane_reset patch in v4.20,  but just
to make sure I will double check with Sean on IRC.


> 
> --
> Regards
> 
> Kieran
> 
> 
> > 
> >>
> >>>  	state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
> >>>  
> >>>  	plane->state = state;
> >>> -- 
> >>> 2.17.1
> >>>
> >>> _______________________________________________
> >>> dri-devel mailing list
> >>> dri-devel@lists.freedesktop.org
> >>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >>
> >> -- 
> >> Ville Syrj�l�
> >> Intel
> > 

-- 
Cheers,
Alex G

Re: [PATCH 1/2] drm/atomic: Initialise planes with opaque alpha values

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Wednesday, 19 September 2018 18:56:58 EEST Kieran Bingham wrote:
> Planes without an alpha property, using __drm_atomic_helper_plane_reset
> will have their plane state alpha initialised as zero, which represents
> a transparent alpha.
> 
> If this value is then used for the plane, it may not be visible by
> default, and thus doesn't represent a good initialisation state.
> 
> Update the default state->alpha value to DRM_BLEND_ALPHA_OPAQUE
> unconditionally when the plane is reset.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

I believe the decision to use plane->alpha_property->values[1] instead of 
hardcoding DRM_BLEND_ALPHA_OPAQUE comes from earlier versions of the alpha 
patch series that supported driver-specific ranges for the alpha value. The 
current implementation uses DRM_BLEND_ALPHA_OPAQUE unconditionally, and no 
driver modifies the maximum value behind the scene, so

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c index 3cf1aa132778..e49b22381048
> 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3569,9 +3569,7 @@ void __drm_atomic_helper_plane_reset(struct drm_plane
> *plane, state->plane = plane;
>  	state->rotation = DRM_MODE_ROTATE_0;
> 
> -	/* Reset the alpha value to fully opaque if it matters */
> -	if (plane->alpha_property)
> -		state->alpha = plane->alpha_property->values[1];
> +	state->alpha = DRM_BLEND_ALPHA_OPAQUE;
>  	state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
> 
>  	plane->state = state;


-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/2] drm: rcar-du: Enable alpha property on primary planes

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Wednesday, 19 September 2018 18:56:59 EEST Kieran Bingham wrote:
> If the alpha property is not added to a plane, a default value will be
> used, which can result in a non-visible layer if the alpha is
> initialised as 0.
> 
> Provide an alpha blend property on all planes.
> 
> Fixes: 161ad653d6c9 ("drm: rcar-du: Use __drm_atomic_helper_plane_reset
> instead of copying the logic")
> 
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> ---
>  drivers/gpu/drm/rcar-du/rcar_du_plane.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> b/drivers/gpu/drm/rcar-du/rcar_du_plane.c index 9e07758a755c..72399a19d8a6
> 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> @@ -783,13 +783,18 @@ int rcar_du_planes_init(struct rcar_du_group *rgrp)
>  		drm_plane_helper_add(&plane->plane,
>  				     &rcar_du_plane_helper_funcs);
> 
> +		/*
> +		 * The alpha property needs to be initialised on all planes
> +		 * to ensure the correct setting at the output.
> +		 */
> +		drm_plane_create_alpha_property(&plane->plane);
> +

As mentioned in the cover letter, both patches in this series fix the issue at 
hand. The first patch is more generic as it will fix it for all drivers, while 
this patch is specific to the R-Car DU driver. It however makes sense to merge 
it, as it adds alpha support to the primary plane, which can be useful.

Once the first patch gets merged, the above comment won't be correct anymore. 
I wonder whether we shouldn't change the patch description and comment to 
focus on usage of the alpha property for primary planes, and not on the bug 
fix. What's your opinion ?

>  		if (type == DRM_PLANE_TYPE_PRIMARY)
>  			continue;
> 
>  		drm_object_attach_property(&plane->plane.base,
>  					   rcdu->props.colorkey,
>  					   RCAR_DU_COLORKEY_NONE);
> -		drm_plane_create_alpha_property(&plane->plane);
>  		drm_plane_create_zpos_property(&plane->plane, 1, 1, 7);
>  	}

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/2] drm: rcar-du: Enable alpha property on primary planes

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Wednesday, 19 September 2018 18:56:59 EEST Kieran Bingham wrote:
> If the alpha property is not added to a plane, a default value will be
> used, which can result in a non-visible layer if the alpha is
> initialised as 0.
> 
> Provide an alpha blend property on all planes.
> 
> Fixes: 161ad653d6c9 ("drm: rcar-du: Use __drm_atomic_helper_plane_reset
> instead of copying the logic")
> 
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> ---
>  drivers/gpu/drm/rcar-du/rcar_du_plane.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> b/drivers/gpu/drm/rcar-du/rcar_du_plane.c index 9e07758a755c..72399a19d8a6
> 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> @@ -783,13 +783,18 @@ int rcar_du_planes_init(struct rcar_du_group *rgrp)
>  		drm_plane_helper_add(&plane->plane,
>  				     &rcar_du_plane_helper_funcs);
> 
> +		/*
> +		 * The alpha property needs to be initialised on all planes
> +		 * to ensure the correct setting at the output.
> +		 */
> +		drm_plane_create_alpha_property(&plane->plane);
> +

As mentioned in the cover letter, both patches in this series fix the issue at 
hand. The first patch is more generic as it will fix it for all drivers, while 
this patch is specific to the R-Car DU driver. It however makes sense to merge 
it, as it adds alpha support to the primary plane, which can be useful.

Once the first patch gets merged, the above comment won't be correct anymore. 
I wonder whether we shouldn't change the patch description and comment to 
focus on usage of the alpha property for primary planes, and not on the bug 
fix. What's your opinion ?

>  		if (type == DRM_PLANE_TYPE_PRIMARY)
>  			continue;
> 
>  		drm_object_attach_property(&plane->plane.base,
>  					   rcdu->props.colorkey,
>  					   RCAR_DU_COLORKEY_NONE);
> -		drm_plane_create_alpha_property(&plane->plane);
>  		drm_plane_create_zpos_property(&plane->plane, 1, 1, 7);
>  	}

-- 
Regards,

Laurent Pinchart



_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm: rcar-du: Enable alpha property on primary planes

[Laurent Pinchart]

Hi Kieran,

On Thursday, 20 September 2018 14:22:38 EET Laurent Pinchart wrote:
> On Wednesday, 19 September 2018 18:56:59 EEST Kieran Bingham wrote:
> > If the alpha property is not added to a plane, a default value will be
> > used, which can result in a non-visible layer if the alpha is
> > initialised as 0.
> > 
> > Provide an alpha blend property on all planes.
> > 
> > Fixes: 161ad653d6c9 ("drm: rcar-du: Use __drm_atomic_helper_plane_reset
> > instead of copying the logic")
> > 
> > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> > ---
> > 
> >  drivers/gpu/drm/rcar-du/rcar_du_plane.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> > b/drivers/gpu/drm/rcar-du/rcar_du_plane.c index 9e07758a755c..72399a19d8a6
> > 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> > @@ -783,13 +783,18 @@ int rcar_du_planes_init(struct rcar_du_group *rgrp)
> > 
> >  		drm_plane_helper_add(&plane->plane,
> >  		
> >  				     &rcar_du_plane_helper_funcs);
> > 
> > +		/*
> > +		 * The alpha property needs to be initialised on all planes
> > +		 * to ensure the correct setting at the output.
> > +		 */
> > +		drm_plane_create_alpha_property(&plane->plane);
> > +
> 
> As mentioned in the cover letter, both patches in this series fix the issue
> at hand. The first patch is more generic as it will fix it for all drivers,
> while this patch is specific to the R-Car DU driver. It however makes sense
> to merge it, as it adds alpha support to the primary plane, which can be
> useful.
> 
> Once the first patch gets merged, the above comment won't be correct
> anymore. I wonder whether we shouldn't change the patch description and
> comment to focus on usage of the alpha property for primary planes, and not
> on the bug fix. What's your opinion ?

I've removed the comment and changed the commit message to

  drm: rcar-du: Enable alpha property on primary planes
  
  The hardware supports alpha on all planes, and using it on the primary
  plane can be useful. Don't restrict the alpha property to overlay
  planes.

With this,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

and applied to my tree.

> >  		if (type == DRM_PLANE_TYPE_PRIMARY)
> >  			continue;
> >  		
> >  		drm_object_attach_property(&plane->plane.base,
> >  					   rcdu->props.colorkey,
> >  					   RCAR_DU_COLORKEY_NONE);
> > 
> > -		drm_plane_create_alpha_property(&plane->plane);
> >  		drm_plane_create_zpos_property(&plane->plane, 1, 1, 7);
> >  	}

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/2] drm: rcar-du: Enable alpha property on primary planes

[Kieran Bingham]

Hi Laurent,

On 21/11/2018 18:29, Laurent Pinchart wrote:
> Hi Kieran,
> 
> On Thursday, 20 September 2018 14:22:38 EET Laurent Pinchart wrote:
>> On Wednesday, 19 September 2018 18:56:59 EEST Kieran Bingham wrote:
>>> If the alpha property is not added to a plane, a default value will be
>>> used, which can result in a non-visible layer if the alpha is
>>> initialised as 0.
>>>
>>> Provide an alpha blend property on all planes.
>>>
>>> Fixes: 161ad653d6c9 ("drm: rcar-du: Use __drm_atomic_helper_plane_reset
>>> instead of copying the logic")
>>>
>>> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>>> ---
>>>
>>>  drivers/gpu/drm/rcar-du/rcar_du_plane.c | 7 ++++++-
>>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
>>> b/drivers/gpu/drm/rcar-du/rcar_du_plane.c index 9e07758a755c..72399a19d8a6
>>> 100644
>>> --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
>>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
>>> @@ -783,13 +783,18 @@ int rcar_du_planes_init(struct rcar_du_group *rgrp)
>>>
>>>  		drm_plane_helper_add(&plane->plane,
>>>  		
>>>  				     &rcar_du_plane_helper_funcs);
>>>
>>> +		/*
>>> +		 * The alpha property needs to be initialised on all planes
>>> +		 * to ensure the correct setting at the output.
>>> +		 */
>>> +		drm_plane_create_alpha_property(&plane->plane);
>>> +
>>
>> As mentioned in the cover letter, both patches in this series fix the issue
>> at hand. The first patch is more generic as it will fix it for all drivers,
>> while this patch is specific to the R-Car DU driver. It however makes sense
>> to merge it, as it adds alpha support to the primary plane, which can be
>> useful.
>>
>> Once the first patch gets merged, the above comment won't be correct
>> anymore. I wonder whether we shouldn't change the patch description and
>> comment to focus on usage of the alpha property for primary planes, and not
>> on the bug fix. What's your opinion ?

Aha - sorry this slipped my queue.


> I've removed the comment and changed the commit message to
> 
>   drm: rcar-du: Enable alpha property on primary planes
>   
>   The hardware supports alpha on all planes, and using it on the primary
>   plane can be useful. Don't restrict the alpha property to overlay
>   planes.
> 
> With this,
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Perfect, - That reads fine.

Thank you.



> 
> and applied to my tree.
> 
>>>  		if (type == DRM_PLANE_TYPE_PRIMARY)
>>>  			continue;
>>>  		
>>>  		drm_object_attach_property(&plane->plane.base,
>>>  					   rcdu->props.colorkey,
>>>  					   RCAR_DU_COLORKEY_NONE);
>>>
>>> -		drm_plane_create_alpha_property(&plane->plane);
>>>  		drm_plane_create_zpos_property(&plane->plane, 1, 1, 7);
>>>  	}
>