Re: [PATCH] drm: rcar-du: Revert "drm: rcar-du: Use __drm_atomic_helper_plane_reset instead of copying the logic"
On Mon, Sep 17, 2018 at 01:25:49PM +0100, Alexandru-Cosmin Gheorghe wrote:
> Hi Kieran,
>
> On Mon, Sep 17, 2018 at 11:56:23AM +0100, Kieran Bingham wrote:
> > Hi Alexandru,
> >
> > On 17/09/18 10:10, Alexandru-Cosmin Gheorghe wrote:
> > > Hi Kieran,
> > >
> > > Sorry for that and thanks for getting to the bottom of it.
> >
> > No worries,
> >
> >
> > > On Fri, Sep 14, 2018 at 11:28:05PM +0100, Kieran Bingham wrote:
> > >> Hi Laurent,
> > >>
> > >> I've looked through a bit further to try to understand this issue and I
> > >> think I have identified a possible/probable cause.
> > >>
> > >> Before commit 161ad653d6c9, we *always* set the plane->state->alpha as
> > >> DRM_BLEND_ALPHA_OPAQUE. (0x)
> > >>
> > >> After this commit, the __drm_atomic_helper_plane_reset() call will only
> > >> set state->alpha to 0x if the alpha_property is set:
> > >>
> > >> if (plane->alpha_property)
> > >> state->alpha = plane->alpha_property->values[1];
> > >>
> > >> Then the state->alpha value is always referenced in
> > >> rcar_du_vsp_plane_setup() and passed to the VSP for that layer.
> > >>
> > >>
> > >> We can see in rcar_du_planes_init(), that all OVERLAY planes are
> > >> configured to have this propery with a call to
> > >> drm_plane_create_alpha_property(>plane); however - the PRIMARY
> > >> plane is skipped over before setting this.
> > >>
> > >>
> > >> I believe I recall seeing that the kms-test-planeposition.py
> > >> successfully showed other planes which were not the back one (I'm now
> > >> going from hazy memory of this afternoon - but I am fairly sure I saw
> > >> this)
> > >>
> > >>
> > >> This implies that the primary planes are being incorrectly configured -
> > >> but the overlay planes are still functioning as expected.
> > >>
> > >> So I believe we could move the call to create the alpha property:
> > >> drm_plane_create_alpha_property(>plane);
> > >>
> > >> to occur before the if (type == DRM_PLANE_TYPE_PRIMARY) continue;
> > >> statement.
> > >>
> > >
> > > I don't see any reson why the primary plane shouldn't advertise an
> > > alpha as well.
> >
> >
> > OK - so I think we perhaps should make sure that we enable alpha for our
> > primary plane in rcar-du.
> >
> >
> > >> It may or may not make sense to have alpha blending support on the
> > >> PRIMARY plane. At the risk of sounding silly - can we have anything else
> > >> behind the PRIMARY plane ? (I doubt this, I don't think we have any
> > >> extra layers hiding anywhere)
> > >
> > > I think the same question could apply to situations where PRIMARY is
> > > disabled and you have just one OVERLAY plane enabled.
> > >
> > >>
> > >> Otherwise, I think we would need to intercept the configuration of the
> > >> alpha blending and make sure that all PRIMARY planes are configured to
> > >> be fully opaque in our layers. I think this is happening in
> > >> rcar_du_vsp_plane_setup().
> > >>
> > >> But rather than put in a conditional in there, I think I'd rather just
> > >> initialise the plane->state->alpha = DRM_BLEND_ALPHA_OPAQUE in our
> > >> rcar_du_vsp_plane_reset() call and be done with it.
> > >
> > > I think you could do more and just go in
> > > __drm_atomic_helper_plane_reset and always initializes plane->alpha
> > > with DRM_BLEND_ALPHA_OPAQUE, this way nobody else hits this problem.
> >
> > I think this sounds like a good thing too.
> >
> > Is DRM_BLEND_ALPHA_OPAQUE a suitable initial value for all of the other
> > users of the helper ?
> >
> > I suspect the answer is yes, but I'll try to do a bit of digging through
> > the other drivers tomorrow.
> >
>
> Yes, but it doesn't hurt for another pair of eyes to have a look.
Just hard-code init alpha to OPAQUE is imo the correct fix. The current is
not matching the general style we use for state parameters at all, and I
don't see a reason why it's different. The comment is what the code should
actually do I think.
-Daniel
>
> > I presume we could then just remove the conditional check and always
> > initialise to OPAQUE ...
> >
> > Or otherwise perhaps maybe initialising as an 'else' if no alpha
> > property is provided.
> >
> > --
> > Regards
> >
> > Kieran
> >
> >
> >
> >
> >
> >
> > >>
> > >> Anyway - just some musings and thoughts at this stage, we can
> > >> investigate in more detail next week.
> > >>
> > >> --
> > >> Regards
> > >>
> > >> Kieran
> > >>
> > >>
> > >> On 14/09/18 21:09, Kieran Bingham wrote:
> > >>> Commit: 161ad653d6c9 ("drm: rcar-du: Use __drm_atomic_helper_plane_reset
> > >>> instead of copying the logic") causes a regression in the R-Car DU
> > >>> display driver, and prevents any output from being displayed.
> > >>>
> > >>> The display appears to function correctly but only a black screen is
> > >>> ever visible.
> > >>>
> > >>> Revert the commit.
> > >>>
> > >>> Signed-off-by: Kieran Bingham
> > >>>
> > >>> ---
> > >>>
> > >>> Looking through the code, the reason for this issue isn't particularly
> > >>>
Re: [PATCH] drm: rcar-du: Revert "drm: rcar-du: Use __drm_atomic_helper_plane_reset instead of copying the logic"
Hi Kieran,
On Mon, Sep 17, 2018 at 11:56:23AM +0100, Kieran Bingham wrote:
> Hi Alexandru,
>
> On 17/09/18 10:10, Alexandru-Cosmin Gheorghe wrote:
> > Hi Kieran,
> >
> > Sorry for that and thanks for getting to the bottom of it.
>
> No worries,
>
>
> > On Fri, Sep 14, 2018 at 11:28:05PM +0100, Kieran Bingham wrote:
> >> Hi Laurent,
> >>
> >> I've looked through a bit further to try to understand this issue and I
> >> think I have identified a possible/probable cause.
> >>
> >> Before commit 161ad653d6c9, we *always* set the plane->state->alpha as
> >> DRM_BLEND_ALPHA_OPAQUE. (0x)
> >>
> >> After this commit, the __drm_atomic_helper_plane_reset() call will only
> >> set state->alpha to 0x if the alpha_property is set:
> >>
> >> if (plane->alpha_property)
> >> state->alpha = plane->alpha_property->values[1];
> >>
> >> Then the state->alpha value is always referenced in
> >> rcar_du_vsp_plane_setup() and passed to the VSP for that layer.
> >>
> >>
> >> We can see in rcar_du_planes_init(), that all OVERLAY planes are
> >> configured to have this propery with a call to
> >> drm_plane_create_alpha_property(>plane); however - the PRIMARY
> >> plane is skipped over before setting this.
> >>
> >>
> >> I believe I recall seeing that the kms-test-planeposition.py
> >> successfully showed other planes which were not the back one (I'm now
> >> going from hazy memory of this afternoon - but I am fairly sure I saw this)
> >>
> >>
> >> This implies that the primary planes are being incorrectly configured -
> >> but the overlay planes are still functioning as expected.
> >>
> >> So I believe we could move the call to create the alpha property:
> >>drm_plane_create_alpha_property(>plane);
> >>
> >> to occur before the if (type == DRM_PLANE_TYPE_PRIMARY) continue;
> >> statement.
> >>
> >
> > I don't see any reson why the primary plane shouldn't advertise an
> > alpha as well.
>
>
> OK - so I think we perhaps should make sure that we enable alpha for our
> primary plane in rcar-du.
>
>
> >> It may or may not make sense to have alpha blending support on the
> >> PRIMARY plane. At the risk of sounding silly - can we have anything else
> >> behind the PRIMARY plane ? (I doubt this, I don't think we have any
> >> extra layers hiding anywhere)
> >
> > I think the same question could apply to situations where PRIMARY is
> > disabled and you have just one OVERLAY plane enabled.
> >
> >>
> >> Otherwise, I think we would need to intercept the configuration of the
> >> alpha blending and make sure that all PRIMARY planes are configured to
> >> be fully opaque in our layers. I think this is happening in
> >> rcar_du_vsp_plane_setup().
> >>
> >> But rather than put in a conditional in there, I think I'd rather just
> >> initialise the plane->state->alpha = DRM_BLEND_ALPHA_OPAQUE in our
> >> rcar_du_vsp_plane_reset() call and be done with it.
> >
> > I think you could do more and just go in
> > __drm_atomic_helper_plane_reset and always initializes plane->alpha
> > with DRM_BLEND_ALPHA_OPAQUE, this way nobody else hits this problem.
>
> I think this sounds like a good thing too.
>
> Is DRM_BLEND_ALPHA_OPAQUE a suitable initial value for all of the other
> users of the helper ?
>
> I suspect the answer is yes, but I'll try to do a bit of digging through
> the other drivers tomorrow.
>
Yes, but it doesn't hurt for another pair of eyes to have a look.
> I presume we could then just remove the conditional check and always
> initialise to OPAQUE ...
>
> Or otherwise perhaps maybe initialising as an 'else' if no alpha
> property is provided.
>
> --
> Regards
>
> Kieran
>
>
>
>
>
>
> >>
> >> Anyway - just some musings and thoughts at this stage, we can
> >> investigate in more detail next week.
> >>
> >> --
> >> Regards
> >>
> >> Kieran
> >>
> >>
> >> On 14/09/18 21:09, Kieran Bingham wrote:
> >>> Commit: 161ad653d6c9 ("drm: rcar-du: Use __drm_atomic_helper_plane_reset
> >>> instead of copying the logic") causes a regression in the R-Car DU
> >>> display driver, and prevents any output from being displayed.
> >>>
> >>> The display appears to function correctly but only a black screen is
> >>> ever visible.
> >>>
> >>> Revert the commit.
> >>>
> >>> Signed-off-by: Kieran Bingham
> >>>
> >>> ---
> >>>
> >>> Looking through the code, the reason for this issue isn't particularly
> >>> obvious - and will need some further exploration, which I can't look at
> >>> until Tuesday. So I'm posting this revert patch to
> >>>
> >>> A) Report the issue
> >>> B) Provide a temporary fix
> >>>
> >>> I suspect either the initial alpha value is not set correctly or setting
> >>>
> >>> state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
> >>>
> >>> causes some side effect perhaps. There's not much else that could be
> >>> different between the helper, and the original code.
> >>>
> >>> drivers/gpu/drm/rcar-du/rcar_du_plane.c | 6 --
> >>> drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 5
Re: [PATCH] drm: rcar-du: Revert "drm: rcar-du: Use __drm_atomic_helper_plane_reset instead of copying the logic"
Hi Alexandru,
On 17/09/18 10:10, Alexandru-Cosmin Gheorghe wrote:
> Hi Kieran,
>
> Sorry for that and thanks for getting to the bottom of it.
No worries,
> On Fri, Sep 14, 2018 at 11:28:05PM +0100, Kieran Bingham wrote:
>> Hi Laurent,
>>
>> I've looked through a bit further to try to understand this issue and I
>> think I have identified a possible/probable cause.
>>
>> Before commit 161ad653d6c9, we *always* set the plane->state->alpha as
>> DRM_BLEND_ALPHA_OPAQUE. (0x)
>>
>> After this commit, the __drm_atomic_helper_plane_reset() call will only
>> set state->alpha to 0x if the alpha_property is set:
>>
>> if (plane->alpha_property)
>> state->alpha = plane->alpha_property->values[1];
>>
>> Then the state->alpha value is always referenced in
>> rcar_du_vsp_plane_setup() and passed to the VSP for that layer.
>>
>>
>> We can see in rcar_du_planes_init(), that all OVERLAY planes are
>> configured to have this propery with a call to
>> drm_plane_create_alpha_property(>plane); however - the PRIMARY
>> plane is skipped over before setting this.
>>
>>
>> I believe I recall seeing that the kms-test-planeposition.py
>> successfully showed other planes which were not the back one (I'm now
>> going from hazy memory of this afternoon - but I am fairly sure I saw this)
>>
>>
>> This implies that the primary planes are being incorrectly configured -
>> but the overlay planes are still functioning as expected.
>>
>> So I believe we could move the call to create the alpha property:
>> drm_plane_create_alpha_property(>plane);
>>
>> to occur before the if (type == DRM_PLANE_TYPE_PRIMARY) continue; statement.
>>
>
> I don't see any reson why the primary plane shouldn't advertise an
> alpha as well.
OK - so I think we perhaps should make sure that we enable alpha for our
primary plane in rcar-du.
>> It may or may not make sense to have alpha blending support on the
>> PRIMARY plane. At the risk of sounding silly - can we have anything else
>> behind the PRIMARY plane ? (I doubt this, I don't think we have any
>> extra layers hiding anywhere)
>
> I think the same question could apply to situations where PRIMARY is
> disabled and you have just one OVERLAY plane enabled.
>
>>
>> Otherwise, I think we would need to intercept the configuration of the
>> alpha blending and make sure that all PRIMARY planes are configured to
>> be fully opaque in our layers. I think this is happening in
>> rcar_du_vsp_plane_setup().
>>
>> But rather than put in a conditional in there, I think I'd rather just
>> initialise the plane->state->alpha = DRM_BLEND_ALPHA_OPAQUE in our
>> rcar_du_vsp_plane_reset() call and be done with it.
>
> I think you could do more and just go in
> __drm_atomic_helper_plane_reset and always initializes plane->alpha
> with DRM_BLEND_ALPHA_OPAQUE, this way nobody else hits this problem.
I think this sounds like a good thing too.
Is DRM_BLEND_ALPHA_OPAQUE a suitable initial value for all of the other
users of the helper ?
I suspect the answer is yes, but I'll try to do a bit of digging through
the other drivers tomorrow.
I presume we could then just remove the conditional check and always
initialise to OPAQUE ...
Or otherwise perhaps maybe initialising as an 'else' if no alpha
property is provided.
--
Regards
Kieran
>>
>> Anyway - just some musings and thoughts at this stage, we can
>> investigate in more detail next week.
>>
>> --
>> Regards
>>
>> Kieran
>>
>>
>> On 14/09/18 21:09, Kieran Bingham wrote:
>>> Commit: 161ad653d6c9 ("drm: rcar-du: Use __drm_atomic_helper_plane_reset
>>> instead of copying the logic") causes a regression in the R-Car DU
>>> display driver, and prevents any output from being displayed.
>>>
>>> The display appears to function correctly but only a black screen is
>>> ever visible.
>>>
>>> Revert the commit.
>>>
>>> Signed-off-by: Kieran Bingham
>>>
>>> ---
>>>
>>> Looking through the code, the reason for this issue isn't particularly
>>> obvious - and will need some further exploration, which I can't look at
>>> until Tuesday. So I'm posting this revert patch to
>>>
>>> A) Report the issue
>>> B) Provide a temporary fix
>>>
>>> I suspect either the initial alpha value is not set correctly or setting
>>>
>>> state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
>>>
>>> causes some side effect perhaps. There's not much else that could be
>>> different between the helper, and the original code.
>>>
>>> drivers/gpu/drm/rcar-du/rcar_du_plane.c | 6 --
>>> drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 5 -
>>> 2 files changed, 8 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
>>> b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
>>> index 9e07758a755c..5c2462afe408 100644
>>> --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
>>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
>>> @@ -686,12 +686,14 @@ static void rcar_du_plane_reset(struct drm_plane
>>> *plane)
>>> if (state == NULL)
>>>
Re: [PATCH] drm: rcar-du: Revert "drm: rcar-du: Use __drm_atomic_helper_plane_reset instead of copying the logic"
Hi Kieran,
Sorry for that and thanks for getting to the bottom of it.
On Fri, Sep 14, 2018 at 11:28:05PM +0100, Kieran Bingham wrote:
> Hi Laurent,
>
> I've looked through a bit further to try to understand this issue and I
> think I have identified a possible/probable cause.
>
> Before commit 161ad653d6c9, we *always* set the plane->state->alpha as
> DRM_BLEND_ALPHA_OPAQUE. (0x)
>
> After this commit, the __drm_atomic_helper_plane_reset() call will only
> set state->alpha to 0x if the alpha_property is set:
>
> if (plane->alpha_property)
> state->alpha = plane->alpha_property->values[1];
>
> Then the state->alpha value is always referenced in
> rcar_du_vsp_plane_setup() and passed to the VSP for that layer.
>
>
> We can see in rcar_du_planes_init(), that all OVERLAY planes are
> configured to have this propery with a call to
> drm_plane_create_alpha_property(>plane); however - the PRIMARY
> plane is skipped over before setting this.
>
>
> I believe I recall seeing that the kms-test-planeposition.py
> successfully showed other planes which were not the back one (I'm now
> going from hazy memory of this afternoon - but I am fairly sure I saw this)
>
>
> This implies that the primary planes are being incorrectly configured -
> but the overlay planes are still functioning as expected.
>
> So I believe we could move the call to create the alpha property:
> drm_plane_create_alpha_property(>plane);
>
> to occur before the if (type == DRM_PLANE_TYPE_PRIMARY) continue; statement.
>
I don't see any reson why the primary plane shouldn't advertise an
alpha as well.
> It may or may not make sense to have alpha blending support on the
> PRIMARY plane. At the risk of sounding silly - can we have anything else
> behind the PRIMARY plane ? (I doubt this, I don't think we have any
> extra layers hiding anywhere)
I think the same question could apply to situations where PRIMARY is
disabled and you have just one OVERLAY plane enabled.
>
> Otherwise, I think we would need to intercept the configuration of the
> alpha blending and make sure that all PRIMARY planes are configured to
> be fully opaque in our layers. I think this is happening in
> rcar_du_vsp_plane_setup().
>
> But rather than put in a conditional in there, I think I'd rather just
> initialise the plane->state->alpha = DRM_BLEND_ALPHA_OPAQUE in our
> rcar_du_vsp_plane_reset() call and be done with it.
I think you could do more and just go in
__drm_atomic_helper_plane_reset and always initializes plane->alpha
with DRM_BLEND_ALPHA_OPAQUE, this way nobody else hits this problem.
>
> Anyway - just some musings and thoughts at this stage, we can
> investigate in more detail next week.
>
> --
> Regards
>
> Kieran
>
>
> On 14/09/18 21:09, Kieran Bingham wrote:
> > Commit: 161ad653d6c9 ("drm: rcar-du: Use __drm_atomic_helper_plane_reset
> > instead of copying the logic") causes a regression in the R-Car DU
> > display driver, and prevents any output from being displayed.
> >
> > The display appears to function correctly but only a black screen is
> > ever visible.
> >
> > Revert the commit.
> >
> > Signed-off-by: Kieran Bingham
> >
> > ---
> >
> > Looking through the code, the reason for this issue isn't particularly
> > obvious - and will need some further exploration, which I can't look at
> > until Tuesday. So I'm posting this revert patch to
> >
> > A) Report the issue
> > B) Provide a temporary fix
> >
> > I suspect either the initial alpha value is not set correctly or setting
> >
> > state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
> >
> > causes some side effect perhaps. There's not much else that could be
> > different between the helper, and the original code.
> >
> > drivers/gpu/drm/rcar-du/rcar_du_plane.c | 6 --
> > drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 5 -
> > 2 files changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> > b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> > index 9e07758a755c..5c2462afe408 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> > @@ -686,12 +686,14 @@ static void rcar_du_plane_reset(struct drm_plane
> > *plane)
> > if (state == NULL)
> > return;
> >
> > - __drm_atomic_helper_plane_reset(plane, >state);
> > -
> > state->hwindex = -1;
> > state->source = RCAR_DU_PLANE_MEMORY;
> > state->colorkey = RCAR_DU_COLORKEY_NONE;
> > state->state.zpos = plane->type == DRM_PLANE_TYPE_PRIMARY ? 0 : 1;
> > +
> > + plane->state = >state;
> > + plane->state->alpha = DRM_BLEND_ALPHA_OPAQUE;
> > + plane->state->plane = plane;
> > }
> >
> > static int rcar_du_plane_atomic_set_property(struct drm_plane *plane,
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> > b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> > index 4576119e..3170b126cfba 100644
> > ---
Re: [PATCH] drm: rcar-du: Revert "drm: rcar-du: Use __drm_atomic_helper_plane_reset instead of copying the logic"
Hi Laurent,
I've looked through a bit further to try to understand this issue and I
think I have identified a possible/probable cause.
Before commit 161ad653d6c9, we *always* set the plane->state->alpha as
DRM_BLEND_ALPHA_OPAQUE. (0x)
After this commit, the __drm_atomic_helper_plane_reset() call will only
set state->alpha to 0x if the alpha_property is set:
if (plane->alpha_property)
state->alpha = plane->alpha_property->values[1];
Then the state->alpha value is always referenced in
rcar_du_vsp_plane_setup() and passed to the VSP for that layer.
We can see in rcar_du_planes_init(), that all OVERLAY planes are
configured to have this propery with a call to
drm_plane_create_alpha_property(>plane); however - the PRIMARY
plane is skipped over before setting this.
I believe I recall seeing that the kms-test-planeposition.py
successfully showed other planes which were not the back one (I'm now
going from hazy memory of this afternoon - but I am fairly sure I saw this)
This implies that the primary planes are being incorrectly configured -
but the overlay planes are still functioning as expected.
So I believe we could move the call to create the alpha property:
drm_plane_create_alpha_property(>plane);
to occur before the if (type == DRM_PLANE_TYPE_PRIMARY) continue; statement.
It may or may not make sense to have alpha blending support on the
PRIMARY plane. At the risk of sounding silly - can we have anything else
behind the PRIMARY plane ? (I doubt this, I don't think we have any
extra layers hiding anywhere)
Otherwise, I think we would need to intercept the configuration of the
alpha blending and make sure that all PRIMARY planes are configured to
be fully opaque in our layers. I think this is happening in
rcar_du_vsp_plane_setup().
But rather than put in a conditional in there, I think I'd rather just
initialise the plane->state->alpha = DRM_BLEND_ALPHA_OPAQUE in our
rcar_du_vsp_plane_reset() call and be done with it.
Anyway - just some musings and thoughts at this stage, we can
investigate in more detail next week.
--
Regards
Kieran
On 14/09/18 21:09, Kieran Bingham wrote:
> Commit: 161ad653d6c9 ("drm: rcar-du: Use __drm_atomic_helper_plane_reset
> instead of copying the logic") causes a regression in the R-Car DU
> display driver, and prevents any output from being displayed.
>
> The display appears to function correctly but only a black screen is
> ever visible.
>
> Revert the commit.
>
> Signed-off-by: Kieran Bingham
>
> ---
>
> Looking through the code, the reason for this issue isn't particularly
> obvious - and will need some further exploration, which I can't look at
> until Tuesday. So I'm posting this revert patch to
>
> A) Report the issue
> B) Provide a temporary fix
>
> I suspect either the initial alpha value is not set correctly or setting
>
> state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
>
> causes some side effect perhaps. There's not much else that could be
> different between the helper, and the original code.
>
> drivers/gpu/drm/rcar-du/rcar_du_plane.c | 6 --
> drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 5 -
> 2 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> index 9e07758a755c..5c2462afe408 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> @@ -686,12 +686,14 @@ static void rcar_du_plane_reset(struct drm_plane *plane)
> if (state == NULL)
> return;
>
> - __drm_atomic_helper_plane_reset(plane, >state);
> -
> state->hwindex = -1;
> state->source = RCAR_DU_PLANE_MEMORY;
> state->colorkey = RCAR_DU_COLORKEY_NONE;
> state->state.zpos = plane->type == DRM_PLANE_TYPE_PRIMARY ? 0 : 1;
> +
> + plane->state = >state;
> + plane->state->alpha = DRM_BLEND_ALPHA_OPAQUE;
> + plane->state->plane = plane;
> }
>
> static int rcar_du_plane_atomic_set_property(struct drm_plane *plane,
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> index 4576119e..3170b126cfba 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> @@ -341,8 +341,11 @@ static void rcar_du_vsp_plane_reset(struct drm_plane
> *plane)
> if (state == NULL)
> return;
>
> - __drm_atomic_helper_plane_reset(plane, >state);
> + state->state.alpha = DRM_BLEND_ALPHA_OPAQUE;
> state->state.zpos = plane->type == DRM_PLANE_TYPE_PRIMARY ? 0 : 1;
> +
> + plane->state = >state;
> + plane->state->plane = plane;
> }
>
> static const struct drm_plane_funcs rcar_du_vsp_plane_funcs = {
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: rcar-du: Revert "drm: rcar-du: Use __drm_atomic_helper_plane_reset instead of copying the logic"
Commit: 161ad653d6c9 ("drm: rcar-du: Use __drm_atomic_helper_plane_reset
instead of copying the logic") causes a regression in the R-Car DU
display driver, and prevents any output from being displayed.
The display appears to function correctly but only a black screen is
ever visible.
Revert the commit.
Signed-off-by: Kieran Bingham
---
Looking through the code, the reason for this issue isn't particularly
obvious - and will need some further exploration, which I can't look at
until Tuesday. So I'm posting this revert patch to
A) Report the issue
B) Provide a temporary fix
I suspect either the initial alpha value is not set correctly or setting
state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
causes some side effect perhaps. There's not much else that could be
different between the helper, and the original code.
drivers/gpu/drm/rcar-du/rcar_du_plane.c | 6 --
drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 5 -
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
index 9e07758a755c..5c2462afe408 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
@@ -686,12 +686,14 @@ static void rcar_du_plane_reset(struct drm_plane *plane)
if (state == NULL)
return;
- __drm_atomic_helper_plane_reset(plane, >state);
-
state->hwindex = -1;
state->source = RCAR_DU_PLANE_MEMORY;
state->colorkey = RCAR_DU_COLORKEY_NONE;
state->state.zpos = plane->type == DRM_PLANE_TYPE_PRIMARY ? 0 : 1;
+
+ plane->state = >state;
+ plane->state->alpha = DRM_BLEND_ALPHA_OPAQUE;
+ plane->state->plane = plane;
}
static int rcar_du_plane_atomic_set_property(struct drm_plane *plane,
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
index 4576119e..3170b126cfba 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
@@ -341,8 +341,11 @@ static void rcar_du_vsp_plane_reset(struct drm_plane
*plane)
if (state == NULL)
return;
- __drm_atomic_helper_plane_reset(plane, >state);
+ state->state.alpha = DRM_BLEND_ALPHA_OPAQUE;
state->state.zpos = plane->type == DRM_PLANE_TYPE_PRIMARY ? 0 : 1;
+
+ plane->state = >state;
+ plane->state->plane = plane;
}
static const struct drm_plane_funcs rcar_du_vsp_plane_funcs = {
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
