* [PATCH 0/2] drm: Alpha blending issue
@ 2018-09-19 15:56 ` Kieran Bingham
0 siblings, 0 replies; 20+ messages in thread
From: Kieran Bingham @ 2018-09-19 15:56 UTC (permalink / raw)
To: Laurent Pinchart, linux-renesas-soc, dri-devel
Cc: Alexandru-Cosmin Gheorghe, 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
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH 0/2] drm: Alpha blending issue @ 2018-09-19 15:56 ` Kieran Bingham 0 siblings, 0 replies; 20+ messages in thread From: Kieran Bingham @ 2018-09-19 15:56 UTC (permalink / raw) To: Laurent Pinchart, linux-renesas-soc, dri-devel Cc: Alexandru-Cosmin Gheorghe, 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/2] drm/atomic: Initialise planes with opaque alpha values 2018-09-19 15:56 ` Kieran Bingham @ 2018-09-19 15:56 ` Kieran Bingham -1 siblings, 0 replies; 20+ messages in thread From: Kieran Bingham @ 2018-09-19 15:56 UTC (permalink / raw) To: Laurent Pinchart, linux-renesas-soc, dri-devel Cc: Alexandru-Cosmin Gheorghe, Kieran Bingham, Gustavo Padovan, Maarten Lankhorst, Sean Paul, David Airlie, open list 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 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 1/2] drm/atomic: Initialise planes with opaque alpha values @ 2018-09-19 15:56 ` Kieran Bingham 0 siblings, 0 replies; 20+ messages in thread From: Kieran Bingham @ 2018-09-19 15:56 UTC (permalink / raw) To: Laurent Pinchart, linux-renesas-soc, dri-devel Cc: Alexandru-Cosmin Gheorghe, Kieran Bingham, Gustavo Padovan, Maarten Lankhorst, Sean Paul, David Airlie, open list 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 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] drm/atomic: Initialise planes with opaque alpha values 2018-09-19 15:56 ` Kieran Bingham @ 2018-09-19 16:15 ` Ville Syrjälä -1 siblings, 0 replies; 20+ messages in thread From: Ville Syrjälä @ 2018-09-19 16:15 UTC (permalink / raw) To: Kieran Bingham Cc: Laurent Pinchart, linux-renesas-soc, dri-devel, David Airlie, Alexandru-Cosmin Gheorghe, open list, Sean Paul 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] drm/atomic: Initialise planes with opaque alpha values @ 2018-09-19 16:15 ` Ville Syrjälä 0 siblings, 0 replies; 20+ messages in thread From: Ville Syrjälä @ 2018-09-19 16:15 UTC (permalink / raw) To: Kieran Bingham Cc: Laurent Pinchart, linux-renesas-soc, dri-devel, David Airlie, Alexandru-Cosmin Gheorghe, open list, Sean Paul 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] drm/atomic: Initialise planes with opaque alpha values 2018-09-19 16:15 ` Ville Syrjälä (?) @ 2018-09-19 16:43 ` Alexandru-Cosmin Gheorghe -1 siblings, 0 replies; 20+ messages in thread From: Alexandru-Cosmin Gheorghe @ 2018-09-19 16:43 UTC (permalink / raw) To: Ville Syrjälä Cc: Kieran Bingham, Laurent Pinchart, linux-renesas-soc, dri-devel, David Airlie, open list, Sean Paul, nd 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] drm/atomic: Initialise planes with opaque alpha values @ 2018-09-19 16:43 ` Alexandru-Cosmin Gheorghe 0 siblings, 0 replies; 20+ messages in thread From: Alexandru-Cosmin Gheorghe @ 2018-09-19 16:43 UTC (permalink / raw) To: Ville Syrjälä Cc: David Airlie, open list, dri-devel, linux-renesas-soc, Kieran Bingham, Laurent Pinchart, nd, Sean Paul 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] drm/atomic: Initialise planes with opaque alpha values @ 2018-09-19 16:43 ` Alexandru-Cosmin Gheorghe 0 siblings, 0 replies; 20+ messages in thread From: Alexandru-Cosmin Gheorghe @ 2018-09-19 16:43 UTC (permalink / raw) To: Ville Syrjälä Cc: Kieran Bingham, Laurent Pinchart, linux-renesas-soc, dri-devel, David Airlie, open list, Sean Paul, nd 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] drm/atomic: Initialise planes with opaque alpha values 2018-09-19 16:43 ` Alexandru-Cosmin Gheorghe @ 2018-09-20 10:03 ` Kieran Bingham -1 siblings, 0 replies; 20+ messages in thread From: Kieran Bingham @ 2018-09-20 10:03 UTC (permalink / raw) To: Alexandru-Cosmin Gheorghe, Ville Syrjälä, Laurent Pinchart Cc: linux-renesas-soc, dri-devel, David Airlie, open list, Sean Paul, nd 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 > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] drm/atomic: Initialise planes with opaque alpha values @ 2018-09-20 10:03 ` Kieran Bingham 0 siblings, 0 replies; 20+ messages in thread From: Kieran Bingham @ 2018-09-20 10:03 UTC (permalink / raw) To: Alexandru-Cosmin Gheorghe, Ville Syrjälä, Laurent Pinchart Cc: David Airlie, open list, dri-devel, linux-renesas-soc, nd, Sean Paul 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] drm/atomic: Initialise planes with opaque alpha values 2018-09-20 10:03 ` Kieran Bingham @ 2018-09-20 10:19 ` Alexandru-Cosmin Gheorghe -1 siblings, 0 replies; 20+ messages in thread From: Alexandru-Cosmin Gheorghe @ 2018-09-20 10:19 UTC (permalink / raw) To: Kieran Bingham Cc: Ville Syrjälä, Laurent Pinchart, linux-renesas-soc, dri-devel, David Airlie, open list, Sean Paul, nd 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] drm/atomic: Initialise planes with opaque alpha values @ 2018-09-20 10:19 ` Alexandru-Cosmin Gheorghe 0 siblings, 0 replies; 20+ messages in thread From: Alexandru-Cosmin Gheorghe @ 2018-09-20 10:19 UTC (permalink / raw) To: Kieran Bingham Cc: Ville Syrjälä, Laurent Pinchart, linux-renesas-soc, dri-devel, David Airlie, open list, Sean Paul, nd 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] drm/atomic: Initialise planes with opaque alpha values 2018-09-19 15:56 ` Kieran Bingham (?) (?) @ 2018-09-20 11:17 ` Laurent Pinchart -1 siblings, 0 replies; 20+ messages in thread From: Laurent Pinchart @ 2018-09-20 11:17 UTC (permalink / raw) To: Kieran Bingham Cc: linux-renesas-soc, dri-devel, Alexandru-Cosmin Gheorghe, Gustavo Padovan, Maarten Lankhorst, Sean Paul, David Airlie, open list 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 2/2] drm: rcar-du: Enable alpha property on primary planes 2018-09-19 15:56 ` Kieran Bingham @ 2018-09-19 15:56 ` Kieran Bingham -1 siblings, 0 replies; 20+ messages in thread From: Kieran Bingham @ 2018-09-19 15:56 UTC (permalink / raw) To: Laurent Pinchart, linux-renesas-soc, dri-devel Cc: Alexandru-Cosmin Gheorghe, Kieran Bingham, David Airlie, open list 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 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/2] drm: rcar-du: Enable alpha property on primary planes @ 2018-09-19 15:56 ` Kieran Bingham 0 siblings, 0 replies; 20+ messages in thread From: Kieran Bingham @ 2018-09-19 15:56 UTC (permalink / raw) To: Laurent Pinchart, linux-renesas-soc, dri-devel Cc: Alexandru-Cosmin Gheorghe, Kieran Bingham, David Airlie, open list 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 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] drm: rcar-du: Enable alpha property on primary planes 2018-09-19 15:56 ` Kieran Bingham @ 2018-09-20 11:22 ` Laurent Pinchart -1 siblings, 0 replies; 20+ messages in thread From: Laurent Pinchart @ 2018-09-20 11:22 UTC (permalink / raw) To: Kieran Bingham Cc: linux-renesas-soc, dri-devel, Alexandru-Cosmin Gheorghe, David Airlie, open list 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] drm: rcar-du: Enable alpha property on primary planes @ 2018-09-20 11:22 ` Laurent Pinchart 0 siblings, 0 replies; 20+ messages in thread From: Laurent Pinchart @ 2018-09-20 11:22 UTC (permalink / raw) To: Kieran Bingham Cc: linux-renesas-soc, David Airlie, Alexandru-Cosmin Gheorghe, open list, dri-devel 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] drm: rcar-du: Enable alpha property on primary planes 2018-09-20 11:22 ` Laurent Pinchart (?) @ 2018-11-21 18:29 ` Laurent Pinchart 2018-11-21 21:10 ` Kieran Bingham -1 siblings, 1 reply; 20+ messages in thread From: Laurent Pinchart @ 2018-11-21 18:29 UTC (permalink / raw) To: Laurent Pinchart Cc: Kieran Bingham, linux-renesas-soc, dri-devel, Alexandru-Cosmin Gheorghe, David Airlie, open list 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] drm: rcar-du: Enable alpha property on primary planes 2018-11-21 18:29 ` Laurent Pinchart @ 2018-11-21 21:10 ` Kieran Bingham 0 siblings, 0 replies; 20+ messages in thread From: Kieran Bingham @ 2018-11-21 21:10 UTC (permalink / raw) To: Laurent Pinchart Cc: linux-renesas-soc, dri-devel, Alexandru-Cosmin Gheorghe, David Airlie, open list 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); >>> } > ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2018-11-21 21:10 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-09-19 15:56 [PATCH 0/2] drm: Alpha blending issue Kieran Bingham 2018-09-19 15:56 ` Kieran Bingham 2018-09-19 15:56 ` [PATCH 1/2] drm/atomic: Initialise planes with opaque alpha values Kieran Bingham 2018-09-19 15:56 ` Kieran Bingham 2018-09-19 16:15 ` Ville Syrjälä 2018-09-19 16:15 ` Ville Syrjälä 2018-09-19 16:43 ` Alexandru-Cosmin Gheorghe 2018-09-19 16:43 ` Alexandru-Cosmin Gheorghe 2018-09-19 16:43 ` Alexandru-Cosmin Gheorghe 2018-09-20 10:03 ` Kieran Bingham 2018-09-20 10:03 ` Kieran Bingham 2018-09-20 10:19 ` Alexandru-Cosmin Gheorghe 2018-09-20 10:19 ` Alexandru-Cosmin Gheorghe 2018-09-20 11:17 ` Laurent Pinchart 2018-09-19 15:56 ` [PATCH 2/2] drm: rcar-du: Enable alpha property on primary planes Kieran Bingham 2018-09-19 15:56 ` Kieran Bingham 2018-09-20 11:22 ` Laurent Pinchart 2018-09-20 11:22 ` Laurent Pinchart 2018-11-21 18:29 ` Laurent Pinchart 2018-11-21 21:10 ` Kieran Bingham
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.