Re: [PATCH] drm/amd/display: fix hw rotated modes when PSR-SU is enabled

2023-12-07 Thread Bin Li
Hi Mario,

I found I missed the part
in drivers/gpu/drm/amd/display/dc/hwss/dcn10/dcn10_hwseq.c with kai.heng's
review.
I will rebuild a new kernel and test it again, and reply later, sorry about
that.



On Thu, Dec 7, 2023 at 2:58 PM Kai-Heng Feng 
wrote:

> On Thu, Dec 7, 2023 at 10:10 AM Mario Limonciello
>  wrote:
> >
> > On 12/6/2023 20:07, Kai-Heng Feng wrote:
> > > On Thu, Dec 7, 2023 at 9:57 AM Mario Limonciello
> > >  wrote:
> > >>
> > >> On 12/6/2023 19:23, Kai-Heng Feng wrote:
> > >>> On Wed, Dec 6, 2023 at 4:29 AM Mario Limonciello
> > >>>  wrote:
> > 
> >  On 12/5/2023 14:17, Hamza Mahfooz wrote:
> > > We currently don't support dirty rectangles on hardware rotated
> modes.
> > > So, if a user is using hardware rotated modes with PSR-SU enabled,
> > > use PSR-SU FFU for all rotated planes (including cursor planes).
> > >
> > 
> >  Here is the email for the original reporter to give an attribution
> tag.
> > 
> >  Reported-by: Kai-Heng Feng 
> > >>>
> > >>> For this particular issue,
> > >>> Tested-by: Kai-Heng Feng 
> > >>
> > >> Can you confirm what kernel base you tested issue against?
> > >>
> > >> I ask because Bin Li (+CC) also tested it against 6.1 based LTS kernel
> > >> but ran into problems.
> > >
> > > The patch was tested against ADSN.
> > >
> > >>
> > >> I wonder if it's because of other dependency patches.  If that's the
> > >> case it would be good to call them out in the Cc: @stable as
> > >> dependencies so when Greg or Sasha backport this 6.1 doesn't get
> broken.
> > >
> > > Probably. I haven't really tested any older kernel series.
> >
> > Since you've got a good environment to test it and reproduce it would
> > you mind double checking it against 6.7-rc, 6.5 and 6.1 trees?  If we
> > don't have confidence it works on the older trees I think we'll need to
> > drop the stable tag.
>
> Not seeing issues here when the patch is applied against 6.5 and 6.1
> (which needs to resolve a minor conflict).
>
> I am not sure what happened for Bin's case.
>
> Kai-Heng
>
> > >
> > > Kai-Heng
> > >
> > >>
> > >> Bin,
> > >>
> > >> Could you run ./scripts/decode_stacktrace.sh on your kernel trace to
> > >> give us a specific line number on the issue you hit?
> > >>
> > >> Thanks!
> > >>>
> > 
> > > Cc: sta...@vger.kernel.org
> > > Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2952
> > > Fixes: 30ebe41582d1 ("drm/amd/display: add FB_DAMAGE_CLIPS
> support")
> > > Signed-off-by: Hamza Mahfooz 
> > > ---
> > > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c|  4 
> > > drivers/gpu/drm/amd/display/dc/dc_hw_types.h |  1 +
> > > drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubp.c| 12
> ++--
> > > .../gpu/drm/amd/display/dc/hwss/dcn10/dcn10_hwseq.c  |  3 ++-
> > > 4 files changed, 17 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > > index c146dc9cba92..79f8102d2601 100644
> > > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > > @@ -5208,6 +5208,7 @@ static void fill_dc_dirty_rects(struct
> drm_plane *plane,
> > > bool bb_changed;
> > > bool fb_changed;
> > > u32 i = 0;
> > > +
> > 
> >  Looks like a spurious newline here.
> > 
> > > *dirty_regions_changed = false;
> > >
> > > /*
> > > @@ -5217,6 +5218,9 @@ static void fill_dc_dirty_rects(struct
> drm_plane *plane,
> > > if (plane->type == DRM_PLANE_TYPE_CURSOR)
> > > return;
> > >
> > > + if (new_plane_state->rotation != DRM_MODE_ROTATE_0)
> > > + goto ffu;
> > > +
> > 
> >  I noticed that the original report was specifically on 180°.  Since
> >  you're also covering 90° and 270° with this check it sounds like
> it's
> >  actually problematic on those too?
> > >>>
> > >>> 90 & 270 are problematic too. But from what I observed the issue is
> > >>> much more than just cursors.
> > >>
> > >> Got it; thanks.
> > >>
> > >>>
> > >>> Kai-Heng
> > >>>
> > 
> > > num_clips =
> drm_plane_get_damage_clips_count(new_plane_state);
> > > clips = drm_plane_get_damage_clips(new_plane_state);
> > >
> > > diff --git a/drivers/gpu/drm/amd/display/dc/dc_hw_types.h
> b/drivers/gpu/drm/amd/display/dc/dc_hw_types.h
> > > index 9649934ea186..e2a3aa8812df 100644
> > > --- a/drivers/gpu/drm/amd/display/dc/dc_hw_types.h
> > > +++ b/drivers/gpu/drm/amd/display/dc/dc_hw_types.h
> > > @@ -465,6 +465,7 @@ struct dc_cursor_mi_param {
> > > struct fixed31_32 v_scale_ratio;
> > > enum dc_rotation_angle rotation;
> > > bool mirror;
> > > + struct dc_stream_state *stream;
> > > 

Re: [PATCH] drm/amd/display: fix hw rotated modes when PSR-SU is enabled

2023-12-07 Thread Bin Li
Hi Mario,

 It's a false alarm from my side, after testing the 6.1.0-oem and
6.5.0-oem kernels, this patch works perfectly fine, sorry about that.

On Thu, Dec 7, 2023 at 3:47 PM Bin Li  wrote:
>
> Hi Mario,
>
> I found I missed the part in 
> drivers/gpu/drm/amd/display/dc/hwss/dcn10/dcn10_hwseq.c with kai.heng's 
> review.
> I will rebuild a new kernel and test it again, and reply later, sorry about 
> that.
>
>
>
> On Thu, Dec 7, 2023 at 2:58 PM Kai-Heng Feng  
> wrote:
>>
>> On Thu, Dec 7, 2023 at 10:10 AM Mario Limonciello
>>  wrote:
>> >
>> > On 12/6/2023 20:07, Kai-Heng Feng wrote:
>> > > On Thu, Dec 7, 2023 at 9:57 AM Mario Limonciello
>> > >  wrote:
>> > >>
>> > >> On 12/6/2023 19:23, Kai-Heng Feng wrote:
>> > >>> On Wed, Dec 6, 2023 at 4:29 AM Mario Limonciello
>> > >>>  wrote:
>> > 
>> >  On 12/5/2023 14:17, Hamza Mahfooz wrote:
>> > > We currently don't support dirty rectangles on hardware rotated 
>> > > modes.
>> > > So, if a user is using hardware rotated modes with PSR-SU enabled,
>> > > use PSR-SU FFU for all rotated planes (including cursor planes).
>> > >
>> > 
>> >  Here is the email for the original reporter to give an attribution 
>> >  tag.
>> > 
>> >  Reported-by: Kai-Heng Feng 
>> > >>>
>> > >>> For this particular issue,
>> > >>> Tested-by: Kai-Heng Feng 
>> > >>
>> > >> Can you confirm what kernel base you tested issue against?
>> > >>
>> > >> I ask because Bin Li (+CC) also tested it against 6.1 based LTS kernel
>> > >> but ran into problems.
>> > >
>> > > The patch was tested against ADSN.
>> > >
>> > >>
>> > >> I wonder if it's because of other dependency patches.  If that's the
>> > >> case it would be good to call them out in the Cc: @stable as
>> > >> dependencies so when Greg or Sasha backport this 6.1 doesn't get broken.
>> > >
>> > > Probably. I haven't really tested any older kernel series.
>> >
>> > Since you've got a good environment to test it and reproduce it would
>> > you mind double checking it against 6.7-rc, 6.5 and 6.1 trees?  If we
>> > don't have confidence it works on the older trees I think we'll need to
>> > drop the stable tag.
>>
>> Not seeing issues here when the patch is applied against 6.5 and 6.1
>> (which needs to resolve a minor conflict).
>>
>> I am not sure what happened for Bin's case.
>>
>> Kai-Heng
>>
>> > >
>> > > Kai-Heng
>> > >
>> > >>
>> > >> Bin,
>> > >>
>> > >> Could you run ./scripts/decode_stacktrace.sh on your kernel trace to
>> > >> give us a specific line number on the issue you hit?
>> > >>
>> > >> Thanks!
>> > >>>
>> > 
>> > > Cc: sta...@vger.kernel.org
>> > > Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2952
>> > > Fixes: 30ebe41582d1 ("drm/amd/display: add FB_DAMAGE_CLIPS support")
>> > > Signed-off-by: Hamza Mahfooz 
>> > > ---
>> > > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c|  4 
>> > > drivers/gpu/drm/amd/display/dc/dc_hw_types.h |  1 +
>> > > drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubp.c| 12 
>> > > ++--
>> > > .../gpu/drm/amd/display/dc/hwss/dcn10/dcn10_hwseq.c  |  3 ++-
>> > > 4 files changed, 17 insertions(+), 3 deletions(-)
>> > >
>> > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
>> > > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> > > index c146dc9cba92..79f8102d2601 100644
>> > > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> > > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> > > @@ -5208,6 +5208,7 @@ static void fill_dc_dirty_rects(struct 
>> > > drm_plane *plane,
>> > > bool bb_changed;
>> > > bool fb_changed;
>> > > u32 i = 0;
>> > > +
>> > 
>> >  Looks like a spurious newline here.
>> > 
>> > > *dirty_regions_changed = false;
>> > >
>> > > /*
>> > > @@ -5217,6 +5218,9 @@ static void fill_dc_dirty_rects(struct 
>> > > drm_plane *plane,
>> > > if (plane->type == DRM_PLANE_TYPE_CURSOR)
>> > > return;
>> > >
>> > > + if (new_plane_state->rotation != DRM_MODE_ROTATE_0)
>> > > + goto ffu;
>> > > +
>> > 
>> >  I noticed that the original report was specifically on 180°.  Since
>> >  you're also covering 90° and 270° with this check it sounds like it's
>> >  actually problematic on those too?
>> > >>>
>> > >>> 90 & 270 are problematic too. But from what I observed the issue is
>> > >>> much more than just cursors.
>> > >>
>> > >> Got it; thanks.
>> > >>
>> > >>>
>> > >>> Kai-Heng
>> > >>>
>> > 
>> > > num_clips = 
>> > > drm_plane_get_damage_clips_count(new_plane_state);
>> > > clips = drm_plane_get_damage_clips(new_plane_state);
>> > >
>> > > diff --git a/drivers/gpu/drm/amd/display/dc/dc_hw_types.h 
>> > > b/drivers/gpu/drm/amd/display/dc/dc_hw_types.h
>> > > 

Re: [PATCH] drm/amd/display: fix hw rotated modes when PSR-SU is enabled

2023-12-07 Thread Kai-Heng Feng
On Thu, Dec 7, 2023 at 9:57 AM Mario Limonciello
 wrote:
>
> On 12/6/2023 19:23, Kai-Heng Feng wrote:
> > On Wed, Dec 6, 2023 at 4:29 AM Mario Limonciello
> >  wrote:
> >>
> >> On 12/5/2023 14:17, Hamza Mahfooz wrote:
> >>> We currently don't support dirty rectangles on hardware rotated modes.
> >>> So, if a user is using hardware rotated modes with PSR-SU enabled,
> >>> use PSR-SU FFU for all rotated planes (including cursor planes).
> >>>
> >>
> >> Here is the email for the original reporter to give an attribution tag.
> >>
> >> Reported-by: Kai-Heng Feng 
> >
> > For this particular issue,
> > Tested-by: Kai-Heng Feng 
>
> Can you confirm what kernel base you tested issue against?
>
> I ask because Bin Li (+CC) also tested it against 6.1 based LTS kernel
> but ran into problems.

The patch was tested against ADSN.

>
> I wonder if it's because of other dependency patches.  If that's the
> case it would be good to call them out in the Cc: @stable as
> dependencies so when Greg or Sasha backport this 6.1 doesn't get broken.

Probably. I haven't really tested any older kernel series.

Kai-Heng

>
> Bin,
>
> Could you run ./scripts/decode_stacktrace.sh on your kernel trace to
> give us a specific line number on the issue you hit?
>
> Thanks!
> >
> >>
> >>> Cc: sta...@vger.kernel.org
> >>> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2952
> >>> Fixes: 30ebe41582d1 ("drm/amd/display: add FB_DAMAGE_CLIPS support")
> >>> Signed-off-by: Hamza Mahfooz 
> >>> ---
> >>>drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c|  4 
> >>>drivers/gpu/drm/amd/display/dc/dc_hw_types.h |  1 +
> >>>drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubp.c| 12 ++--
> >>>.../gpu/drm/amd/display/dc/hwss/dcn10/dcn10_hwseq.c  |  3 ++-
> >>>4 files changed, 17 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
> >>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> >>> index c146dc9cba92..79f8102d2601 100644
> >>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> >>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> >>> @@ -5208,6 +5208,7 @@ static void fill_dc_dirty_rects(struct drm_plane 
> >>> *plane,
> >>>bool bb_changed;
> >>>bool fb_changed;
> >>>u32 i = 0;
> >>> +
> >>
> >> Looks like a spurious newline here.
> >>
> >>>*dirty_regions_changed = false;
> >>>
> >>>/*
> >>> @@ -5217,6 +5218,9 @@ static void fill_dc_dirty_rects(struct drm_plane 
> >>> *plane,
> >>>if (plane->type == DRM_PLANE_TYPE_CURSOR)
> >>>return;
> >>>
> >>> + if (new_plane_state->rotation != DRM_MODE_ROTATE_0)
> >>> + goto ffu;
> >>> +
> >>
> >> I noticed that the original report was specifically on 180°.  Since
> >> you're also covering 90° and 270° with this check it sounds like it's
> >> actually problematic on those too?
> >
> > 90 & 270 are problematic too. But from what I observed the issue is
> > much more than just cursors.
>
> Got it; thanks.
>
> >
> > Kai-Heng
> >
> >>
> >>>num_clips = drm_plane_get_damage_clips_count(new_plane_state);
> >>>clips = drm_plane_get_damage_clips(new_plane_state);
> >>>
> >>> diff --git a/drivers/gpu/drm/amd/display/dc/dc_hw_types.h 
> >>> b/drivers/gpu/drm/amd/display/dc/dc_hw_types.h
> >>> index 9649934ea186..e2a3aa8812df 100644
> >>> --- a/drivers/gpu/drm/amd/display/dc/dc_hw_types.h
> >>> +++ b/drivers/gpu/drm/amd/display/dc/dc_hw_types.h
> >>> @@ -465,6 +465,7 @@ struct dc_cursor_mi_param {
> >>>struct fixed31_32 v_scale_ratio;
> >>>enum dc_rotation_angle rotation;
> >>>bool mirror;
> >>> + struct dc_stream_state *stream;
> >>>};
> >>>
> >>>/* IPP related types */
> >>> diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubp.c 
> >>> b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubp.c
> >>> index 139cf31d2e45..89c3bf0fe0c9 100644
> >>> --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubp.c
> >>> +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubp.c
> >>> @@ -1077,8 +1077,16 @@ void hubp2_cursor_set_position(
> >>>if (src_y_offset < 0)
> >>>src_y_offset = 0;
> >>>/* Save necessary cursor info x, y position. w, h is saved in 
> >>> attribute func. */
> >>> - hubp->cur_rect.x = src_x_offset + param->viewport.x;
> >>> - hubp->cur_rect.y = src_y_offset + param->viewport.y;
> >>> + if (param->stream->link->psr_settings.psr_version >= 
> >>> DC_PSR_VERSION_SU_1 &&
> >>> + param->rotation != ROTATION_ANGLE_0) {
> >>
> >> Ditto on above about 90° and 270°.
> >>
> >>> + hubp->cur_rect.x = 0;
> >>> + hubp->cur_rect.y = 0;
> >>> + hubp->cur_rect.w = param->stream->timing.h_addressable;
> >>> + hubp->cur_rect.h = param->stream->timing.v_addressable;
> >>> + } else {
> >>> + hubp->cur_rect.x = src_x_offset + param->viewport.x;
> >>> + 

Re: [PATCH] drm/amd/display: fix hw rotated modes when PSR-SU is enabled

2023-12-07 Thread Kai-Heng Feng
On Thu, Dec 7, 2023 at 10:10 AM Mario Limonciello
 wrote:
>
> On 12/6/2023 20:07, Kai-Heng Feng wrote:
> > On Thu, Dec 7, 2023 at 9:57 AM Mario Limonciello
> >  wrote:
> >>
> >> On 12/6/2023 19:23, Kai-Heng Feng wrote:
> >>> On Wed, Dec 6, 2023 at 4:29 AM Mario Limonciello
> >>>  wrote:
> 
>  On 12/5/2023 14:17, Hamza Mahfooz wrote:
> > We currently don't support dirty rectangles on hardware rotated modes.
> > So, if a user is using hardware rotated modes with PSR-SU enabled,
> > use PSR-SU FFU for all rotated planes (including cursor planes).
> >
> 
>  Here is the email for the original reporter to give an attribution tag.
> 
>  Reported-by: Kai-Heng Feng 
> >>>
> >>> For this particular issue,
> >>> Tested-by: Kai-Heng Feng 
> >>
> >> Can you confirm what kernel base you tested issue against?
> >>
> >> I ask because Bin Li (+CC) also tested it against 6.1 based LTS kernel
> >> but ran into problems.
> >
> > The patch was tested against ADSN.
> >
> >>
> >> I wonder if it's because of other dependency patches.  If that's the
> >> case it would be good to call them out in the Cc: @stable as
> >> dependencies so when Greg or Sasha backport this 6.1 doesn't get broken.
> >
> > Probably. I haven't really tested any older kernel series.
>
> Since you've got a good environment to test it and reproduce it would
> you mind double checking it against 6.7-rc, 6.5 and 6.1 trees?  If we
> don't have confidence it works on the older trees I think we'll need to
> drop the stable tag.

Not seeing issues here when the patch is applied against 6.5 and 6.1
(which needs to resolve a minor conflict).

I am not sure what happened for Bin's case.

Kai-Heng

> >
> > Kai-Heng
> >
> >>
> >> Bin,
> >>
> >> Could you run ./scripts/decode_stacktrace.sh on your kernel trace to
> >> give us a specific line number on the issue you hit?
> >>
> >> Thanks!
> >>>
> 
> > Cc: sta...@vger.kernel.org
> > Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2952
> > Fixes: 30ebe41582d1 ("drm/amd/display: add FB_DAMAGE_CLIPS support")
> > Signed-off-by: Hamza Mahfooz 
> > ---
> > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c|  4 
> > drivers/gpu/drm/amd/display/dc/dc_hw_types.h |  1 +
> > drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubp.c| 12 
> > ++--
> > .../gpu/drm/amd/display/dc/hwss/dcn10/dcn10_hwseq.c  |  3 ++-
> > 4 files changed, 17 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
> > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > index c146dc9cba92..79f8102d2601 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > @@ -5208,6 +5208,7 @@ static void fill_dc_dirty_rects(struct drm_plane 
> > *plane,
> > bool bb_changed;
> > bool fb_changed;
> > u32 i = 0;
> > +
> 
>  Looks like a spurious newline here.
> 
> > *dirty_regions_changed = false;
> >
> > /*
> > @@ -5217,6 +5218,9 @@ static void fill_dc_dirty_rects(struct drm_plane 
> > *plane,
> > if (plane->type == DRM_PLANE_TYPE_CURSOR)
> > return;
> >
> > + if (new_plane_state->rotation != DRM_MODE_ROTATE_0)
> > + goto ffu;
> > +
> 
>  I noticed that the original report was specifically on 180°.  Since
>  you're also covering 90° and 270° with this check it sounds like it's
>  actually problematic on those too?
> >>>
> >>> 90 & 270 are problematic too. But from what I observed the issue is
> >>> much more than just cursors.
> >>
> >> Got it; thanks.
> >>
> >>>
> >>> Kai-Heng
> >>>
> 
> > num_clips = drm_plane_get_damage_clips_count(new_plane_state);
> > clips = drm_plane_get_damage_clips(new_plane_state);
> >
> > diff --git a/drivers/gpu/drm/amd/display/dc/dc_hw_types.h 
> > b/drivers/gpu/drm/amd/display/dc/dc_hw_types.h
> > index 9649934ea186..e2a3aa8812df 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dc_hw_types.h
> > +++ b/drivers/gpu/drm/amd/display/dc/dc_hw_types.h
> > @@ -465,6 +465,7 @@ struct dc_cursor_mi_param {
> > struct fixed31_32 v_scale_ratio;
> > enum dc_rotation_angle rotation;
> > bool mirror;
> > + struct dc_stream_state *stream;
> > };
> >
> > /* IPP related types */
> > diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubp.c 
> > b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubp.c
> > index 139cf31d2e45..89c3bf0fe0c9 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubp.c
> > +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubp.c
> > @@ -1077,8 +1077,16 @@ void hubp2_cursor_set_position(
> > if (src_y_offset < 0)
> > 

Re: [PATCH] drm/amd/display: fix hw rotated modes when PSR-SU is enabled

2023-12-07 Thread Kai-Heng Feng
On Wed, Dec 6, 2023 at 4:29 AM Mario Limonciello
 wrote:
>
> On 12/5/2023 14:17, Hamza Mahfooz wrote:
> > We currently don't support dirty rectangles on hardware rotated modes.
> > So, if a user is using hardware rotated modes with PSR-SU enabled,
> > use PSR-SU FFU for all rotated planes (including cursor planes).
> >
>
> Here is the email for the original reporter to give an attribution tag.
>
> Reported-by: Kai-Heng Feng 

For this particular issue,
Tested-by: Kai-Heng Feng 

>
> > Cc: sta...@vger.kernel.org
> > Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2952
> > Fixes: 30ebe41582d1 ("drm/amd/display: add FB_DAMAGE_CLIPS support")
> > Signed-off-by: Hamza Mahfooz 
> > ---
> >   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c|  4 
> >   drivers/gpu/drm/amd/display/dc/dc_hw_types.h |  1 +
> >   drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubp.c| 12 ++--
> >   .../gpu/drm/amd/display/dc/hwss/dcn10/dcn10_hwseq.c  |  3 ++-
> >   4 files changed, 17 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
> > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > index c146dc9cba92..79f8102d2601 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > @@ -5208,6 +5208,7 @@ static void fill_dc_dirty_rects(struct drm_plane 
> > *plane,
> >   bool bb_changed;
> >   bool fb_changed;
> >   u32 i = 0;
> > +
>
> Looks like a spurious newline here.
>
> >   *dirty_regions_changed = false;
> >
> >   /*
> > @@ -5217,6 +5218,9 @@ static void fill_dc_dirty_rects(struct drm_plane 
> > *plane,
> >   if (plane->type == DRM_PLANE_TYPE_CURSOR)
> >   return;
> >
> > + if (new_plane_state->rotation != DRM_MODE_ROTATE_0)
> > + goto ffu;
> > +
>
> I noticed that the original report was specifically on 180°.  Since
> you're also covering 90° and 270° with this check it sounds like it's
> actually problematic on those too?

90 & 270 are problematic too. But from what I observed the issue is
much more than just cursors.

Kai-Heng

>
> >   num_clips = drm_plane_get_damage_clips_count(new_plane_state);
> >   clips = drm_plane_get_damage_clips(new_plane_state);
> >
> > diff --git a/drivers/gpu/drm/amd/display/dc/dc_hw_types.h 
> > b/drivers/gpu/drm/amd/display/dc/dc_hw_types.h
> > index 9649934ea186..e2a3aa8812df 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dc_hw_types.h
> > +++ b/drivers/gpu/drm/amd/display/dc/dc_hw_types.h
> > @@ -465,6 +465,7 @@ struct dc_cursor_mi_param {
> >   struct fixed31_32 v_scale_ratio;
> >   enum dc_rotation_angle rotation;
> >   bool mirror;
> > + struct dc_stream_state *stream;
> >   };
> >
> >   /* IPP related types */
> > diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubp.c 
> > b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubp.c
> > index 139cf31d2e45..89c3bf0fe0c9 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubp.c
> > +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubp.c
> > @@ -1077,8 +1077,16 @@ void hubp2_cursor_set_position(
> >   if (src_y_offset < 0)
> >   src_y_offset = 0;
> >   /* Save necessary cursor info x, y position. w, h is saved in 
> > attribute func. */
> > - hubp->cur_rect.x = src_x_offset + param->viewport.x;
> > - hubp->cur_rect.y = src_y_offset + param->viewport.y;
> > + if (param->stream->link->psr_settings.psr_version >= 
> > DC_PSR_VERSION_SU_1 &&
> > + param->rotation != ROTATION_ANGLE_0) {
>
> Ditto on above about 90° and 270°.
>
> > + hubp->cur_rect.x = 0;
> > + hubp->cur_rect.y = 0;
> > + hubp->cur_rect.w = param->stream->timing.h_addressable;
> > + hubp->cur_rect.h = param->stream->timing.v_addressable;
> > + } else {
> > + hubp->cur_rect.x = src_x_offset + param->viewport.x;
> > + hubp->cur_rect.y = src_y_offset + param->viewport.y;
> > + }
> >   }
> >
> >   void hubp2_clk_cntl(struct hubp *hubp, bool enable)
> > diff --git a/drivers/gpu/drm/amd/display/dc/hwss/dcn10/dcn10_hwseq.c 
> > b/drivers/gpu/drm/amd/display/dc/hwss/dcn10/dcn10_hwseq.c
> > index 2b8b8366538e..ce5613a76267 100644
> > --- a/drivers/gpu/drm/amd/display/dc/hwss/dcn10/dcn10_hwseq.c
> > +++ b/drivers/gpu/drm/amd/display/dc/hwss/dcn10/dcn10_hwseq.c
> > @@ -3417,7 +3417,8 @@ void dcn10_set_cursor_position(struct pipe_ctx 
> > *pipe_ctx)
> >   .h_scale_ratio = pipe_ctx->plane_res.scl_data.ratios.horz,
> >   .v_scale_ratio = pipe_ctx->plane_res.scl_data.ratios.vert,
> >   .rotation = pipe_ctx->plane_state->rotation,
> > - .mirror = pipe_ctx->plane_state->horizontal_mirror
> > + .mirror = pipe_ctx->plane_state->horizontal_mirror,
> > + .stream = pipe_ctx->stream
>
> As a nit; I think it's worth leaving a harmless trailing ',' so that
> there is less ping pong 

Re: [PATCH] drm/amd/display: fix hw rotated modes when PSR-SU is enabled

2023-12-07 Thread Hamza Mahfooz

On 12/5/23 15:29, Mario Limonciello wrote:

On 12/5/2023 14:17, Hamza Mahfooz wrote:

We currently don't support dirty rectangles on hardware rotated modes.
So, if a user is using hardware rotated modes with PSR-SU enabled,
use PSR-SU FFU for all rotated planes (including cursor planes).



Here is the email for the original reporter to give an attribution tag.

Reported-by: Kai-Heng Feng 


Cc: sta...@vger.kernel.org
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2952
Fixes: 30ebe41582d1 ("drm/amd/display: add FB_DAMAGE_CLIPS support")
Signed-off-by: Hamza Mahfooz 
---
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c    |  4 
  drivers/gpu/drm/amd/display/dc/dc_hw_types.h |  1 +
  drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubp.c    | 12 ++--
  .../gpu/drm/amd/display/dc/hwss/dcn10/dcn10_hwseq.c  |  3 ++-
  4 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c

index c146dc9cba92..79f8102d2601 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5208,6 +5208,7 @@ static void fill_dc_dirty_rects(struct drm_plane 
*plane,

  bool bb_changed;
  bool fb_changed;
  u32 i = 0;
+


Looks like a spurious newline here.


  *dirty_regions_changed = false;
  /*
@@ -5217,6 +5218,9 @@ static void fill_dc_dirty_rects(struct drm_plane 
*plane,

  if (plane->type == DRM_PLANE_TYPE_CURSOR)
  return;
+    if (new_plane_state->rotation != DRM_MODE_ROTATE_0)
+    goto ffu;
+


I noticed that the original report was specifically on 180°.  Since 
you're also covering 90° and 270° with this check it sounds like it's 
actually problematic on those too?


Ya it's problematic for 90 and 270 as well, though most mainstream
compositors don't use hardware rotation for those cases under any
circumstances. So, I doubt that many people would encounter this issue in
the wild for them.




  num_clips = drm_plane_get_damage_clips_count(new_plane_state);
  clips = drm_plane_get_damage_clips(new_plane_state);
diff --git a/drivers/gpu/drm/amd/display/dc/dc_hw_types.h 
b/drivers/gpu/drm/amd/display/dc/dc_hw_types.h

index 9649934ea186..e2a3aa8812df 100644
--- a/drivers/gpu/drm/amd/display/dc/dc_hw_types.h
+++ b/drivers/gpu/drm/amd/display/dc/dc_hw_types.h
@@ -465,6 +465,7 @@ struct dc_cursor_mi_param {
  struct fixed31_32 v_scale_ratio;
  enum dc_rotation_angle rotation;
  bool mirror;
+    struct dc_stream_state *stream;
  };
  /* IPP related types */
diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubp.c 
b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubp.c

index 139cf31d2e45..89c3bf0fe0c9 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubp.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubp.c
@@ -1077,8 +1077,16 @@ void hubp2_cursor_set_position(
  if (src_y_offset < 0)
  src_y_offset = 0;
  /* Save necessary cursor info x, y position. w, h is saved in 
attribute func. */

-    hubp->cur_rect.x = src_x_offset + param->viewport.x;
-    hubp->cur_rect.y = src_y_offset + param->viewport.y;
+    if (param->stream->link->psr_settings.psr_version >= 
DC_PSR_VERSION_SU_1 &&

+    param->rotation != ROTATION_ANGLE_0) {


Ditto on above about 90° and 270°.


+    hubp->cur_rect.x = 0;
+    hubp->cur_rect.y = 0;
+    hubp->cur_rect.w = param->stream->timing.h_addressable;
+    hubp->cur_rect.h = param->stream->timing.v_addressable;
+    } else {
+    hubp->cur_rect.x = src_x_offset + param->viewport.x;
+    hubp->cur_rect.y = src_y_offset + param->viewport.y;
+    }
  }
  void hubp2_clk_cntl(struct hubp *hubp, bool enable)
diff --git a/drivers/gpu/drm/amd/display/dc/hwss/dcn10/dcn10_hwseq.c 
b/drivers/gpu/drm/amd/display/dc/hwss/dcn10/dcn10_hwseq.c

index 2b8b8366538e..ce5613a76267 100644
--- a/drivers/gpu/drm/amd/display/dc/hwss/dcn10/dcn10_hwseq.c
+++ b/drivers/gpu/drm/amd/display/dc/hwss/dcn10/dcn10_hwseq.c
@@ -3417,7 +3417,8 @@ void dcn10_set_cursor_position(struct pipe_ctx 
*pipe_ctx)

  .h_scale_ratio = pipe_ctx->plane_res.scl_data.ratios.horz,
  .v_scale_ratio = pipe_ctx->plane_res.scl_data.ratios.vert,
  .rotation = pipe_ctx->plane_state->rotation,
-    .mirror = pipe_ctx->plane_state->horizontal_mirror
+    .mirror = pipe_ctx->plane_state->horizontal_mirror,
+    .stream = pipe_ctx->stream


As a nit; I think it's worth leaving a harmless trailing ',' so that 
there is less ping pong in the future when adding new members to a struct.



  };
  bool pipe_split_on = false;
  bool odm_combine_on = (pipe_ctx->next_odm_pipe != NULL) ||



--
Hamza



Re: [PATCH] drm/amd/display: fix hw rotated modes when PSR-SU is enabled

2023-12-07 Thread Mario Limonciello

Bin, KH,

Thanks for the confirmation!

Hamza,

I think you can add a Tested-by tag for Bin too.

On 12/7/2023 04:38, Bin Li wrote:

Hi Mario,

  It's a false alarm from my side, after testing the 6.1.0-oem and
6.5.0-oem kernels, this patch works perfectly fine, sorry about that.

On Thu, Dec 7, 2023 at 3:47 PM Bin Li  wrote:


Hi Mario,

I found I missed the part in 
drivers/gpu/drm/amd/display/dc/hwss/dcn10/dcn10_hwseq.c with kai.heng's review.
I will rebuild a new kernel and test it again, and reply later, sorry about 
that.



On Thu, Dec 7, 2023 at 2:58 PM Kai-Heng Feng  
wrote:


On Thu, Dec 7, 2023 at 10:10 AM Mario Limonciello
 wrote:


On 12/6/2023 20:07, Kai-Heng Feng wrote:

On Thu, Dec 7, 2023 at 9:57 AM Mario Limonciello
 wrote:


On 12/6/2023 19:23, Kai-Heng Feng wrote:

On Wed, Dec 6, 2023 at 4:29 AM Mario Limonciello
 wrote:


On 12/5/2023 14:17, Hamza Mahfooz wrote:

We currently don't support dirty rectangles on hardware rotated modes.
So, if a user is using hardware rotated modes with PSR-SU enabled,
use PSR-SU FFU for all rotated planes (including cursor planes).



Here is the email for the original reporter to give an attribution tag.

Reported-by: Kai-Heng Feng 


For this particular issue,
Tested-by: Kai-Heng Feng 


Can you confirm what kernel base you tested issue against?

I ask because Bin Li (+CC) also tested it against 6.1 based LTS kernel
but ran into problems.


The patch was tested against ADSN.



I wonder if it's because of other dependency patches.  If that's the
case it would be good to call them out in the Cc: @stable as
dependencies so when Greg or Sasha backport this 6.1 doesn't get broken.


Probably. I haven't really tested any older kernel series.


Since you've got a good environment to test it and reproduce it would
you mind double checking it against 6.7-rc, 6.5 and 6.1 trees?  If we
don't have confidence it works on the older trees I think we'll need to
drop the stable tag.


Not seeing issues here when the patch is applied against 6.5 and 6.1
(which needs to resolve a minor conflict).

I am not sure what happened for Bin's case.

Kai-Heng



Kai-Heng



Bin,

Could you run ./scripts/decode_stacktrace.sh on your kernel trace to
give us a specific line number on the issue you hit?

Thanks!





Cc: sta...@vger.kernel.org
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2952
Fixes: 30ebe41582d1 ("drm/amd/display: add FB_DAMAGE_CLIPS support")
Signed-off-by: Hamza Mahfooz 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c|  4 
 drivers/gpu/drm/amd/display/dc/dc_hw_types.h |  1 +
 drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubp.c| 12 ++--
 .../gpu/drm/amd/display/dc/hwss/dcn10/dcn10_hwseq.c  |  3 ++-
 4 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index c146dc9cba92..79f8102d2601 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5208,6 +5208,7 @@ static void fill_dc_dirty_rects(struct drm_plane *plane,
 bool bb_changed;
 bool fb_changed;
 u32 i = 0;
+


Looks like a spurious newline here.


 *dirty_regions_changed = false;

 /*
@@ -5217,6 +5218,9 @@ static void fill_dc_dirty_rects(struct drm_plane *plane,
 if (plane->type == DRM_PLANE_TYPE_CURSOR)
 return;

+ if (new_plane_state->rotation != DRM_MODE_ROTATE_0)
+ goto ffu;
+


I noticed that the original report was specifically on 180°.  Since
you're also covering 90° and 270° with this check it sounds like it's
actually problematic on those too?


90 & 270 are problematic too. But from what I observed the issue is
much more than just cursors.


Got it; thanks.



Kai-Heng




 num_clips = drm_plane_get_damage_clips_count(new_plane_state);
 clips = drm_plane_get_damage_clips(new_plane_state);

diff --git a/drivers/gpu/drm/amd/display/dc/dc_hw_types.h 
b/drivers/gpu/drm/amd/display/dc/dc_hw_types.h
index 9649934ea186..e2a3aa8812df 100644
--- a/drivers/gpu/drm/amd/display/dc/dc_hw_types.h
+++ b/drivers/gpu/drm/amd/display/dc/dc_hw_types.h
@@ -465,6 +465,7 @@ struct dc_cursor_mi_param {
 struct fixed31_32 v_scale_ratio;
 enum dc_rotation_angle rotation;
 bool mirror;
+ struct dc_stream_state *stream;
 };

 /* IPP related types */
diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubp.c 
b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubp.c
index 139cf31d2e45..89c3bf0fe0c9 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubp.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubp.c
@@ -1077,8 +1077,16 @@ void hubp2_cursor_set_position(
 if (src_y_offset < 0)
 src_y_offset = 0;
 /* Save necessary cursor info x, y position. w, h is saved in 
attribute func. */
- hubp->cur_rect.x = src_x_offset + 

Re: [PATCH] drm/amd/display: fix hw rotated modes when PSR-SU is enabled

2023-12-06 Thread Mario Limonciello

On 12/6/2023 20:07, Kai-Heng Feng wrote:

On Thu, Dec 7, 2023 at 9:57 AM Mario Limonciello
 wrote:


On 12/6/2023 19:23, Kai-Heng Feng wrote:

On Wed, Dec 6, 2023 at 4:29 AM Mario Limonciello
 wrote:


On 12/5/2023 14:17, Hamza Mahfooz wrote:

We currently don't support dirty rectangles on hardware rotated modes.
So, if a user is using hardware rotated modes with PSR-SU enabled,
use PSR-SU FFU for all rotated planes (including cursor planes).



Here is the email for the original reporter to give an attribution tag.

Reported-by: Kai-Heng Feng 


For this particular issue,
Tested-by: Kai-Heng Feng 


Can you confirm what kernel base you tested issue against?

I ask because Bin Li (+CC) also tested it against 6.1 based LTS kernel
but ran into problems.


The patch was tested against ADSN.



I wonder if it's because of other dependency patches.  If that's the
case it would be good to call them out in the Cc: @stable as
dependencies so when Greg or Sasha backport this 6.1 doesn't get broken.


Probably. I haven't really tested any older kernel series.


Since you've got a good environment to test it and reproduce it would 
you mind double checking it against 6.7-rc, 6.5 and 6.1 trees?  If we 
don't have confidence it works on the older trees I think we'll need to 
drop the stable tag.


Kai-Heng



Bin,

Could you run ./scripts/decode_stacktrace.sh on your kernel trace to
give us a specific line number on the issue you hit?

Thanks!





Cc: sta...@vger.kernel.org
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2952
Fixes: 30ebe41582d1 ("drm/amd/display: add FB_DAMAGE_CLIPS support")
Signed-off-by: Hamza Mahfooz 
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c|  4 
drivers/gpu/drm/amd/display/dc/dc_hw_types.h |  1 +
drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubp.c| 12 ++--
.../gpu/drm/amd/display/dc/hwss/dcn10/dcn10_hwseq.c  |  3 ++-
4 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index c146dc9cba92..79f8102d2601 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5208,6 +5208,7 @@ static void fill_dc_dirty_rects(struct drm_plane *plane,
bool bb_changed;
bool fb_changed;
u32 i = 0;
+


Looks like a spurious newline here.


*dirty_regions_changed = false;

/*
@@ -5217,6 +5218,9 @@ static void fill_dc_dirty_rects(struct drm_plane *plane,
if (plane->type == DRM_PLANE_TYPE_CURSOR)
return;

+ if (new_plane_state->rotation != DRM_MODE_ROTATE_0)
+ goto ffu;
+


I noticed that the original report was specifically on 180°.  Since
you're also covering 90° and 270° with this check it sounds like it's
actually problematic on those too?


90 & 270 are problematic too. But from what I observed the issue is
much more than just cursors.


Got it; thanks.



Kai-Heng




num_clips = drm_plane_get_damage_clips_count(new_plane_state);
clips = drm_plane_get_damage_clips(new_plane_state);

diff --git a/drivers/gpu/drm/amd/display/dc/dc_hw_types.h 
b/drivers/gpu/drm/amd/display/dc/dc_hw_types.h
index 9649934ea186..e2a3aa8812df 100644
--- a/drivers/gpu/drm/amd/display/dc/dc_hw_types.h
+++ b/drivers/gpu/drm/amd/display/dc/dc_hw_types.h
@@ -465,6 +465,7 @@ struct dc_cursor_mi_param {
struct fixed31_32 v_scale_ratio;
enum dc_rotation_angle rotation;
bool mirror;
+ struct dc_stream_state *stream;
};

/* IPP related types */
diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubp.c 
b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubp.c
index 139cf31d2e45..89c3bf0fe0c9 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubp.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubp.c
@@ -1077,8 +1077,16 @@ void hubp2_cursor_set_position(
if (src_y_offset < 0)
src_y_offset = 0;
/* Save necessary cursor info x, y position. w, h is saved in attribute 
func. */
- hubp->cur_rect.x = src_x_offset + param->viewport.x;
- hubp->cur_rect.y = src_y_offset + param->viewport.y;
+ if (param->stream->link->psr_settings.psr_version >= DC_PSR_VERSION_SU_1 
&&
+ param->rotation != ROTATION_ANGLE_0) {


Ditto on above about 90° and 270°.


+ hubp->cur_rect.x = 0;
+ hubp->cur_rect.y = 0;
+ hubp->cur_rect.w = param->stream->timing.h_addressable;
+ hubp->cur_rect.h = param->stream->timing.v_addressable;
+ } else {
+ hubp->cur_rect.x = src_x_offset + param->viewport.x;
+ hubp->cur_rect.y = src_y_offset + param->viewport.y;
+ }
}

void hubp2_clk_cntl(struct hubp *hubp, bool enable)
diff --git a/drivers/gpu/drm/amd/display/dc/hwss/dcn10/dcn10_hwseq.c 
b/drivers/gpu/drm/amd/display/dc/hwss/dcn10/dcn10_hwseq.c
index 2b8b8366538e..ce5613a76267 

Re: [PATCH] drm/amd/display: fix hw rotated modes when PSR-SU is enabled

2023-12-06 Thread Mario Limonciello

On 12/6/2023 19:23, Kai-Heng Feng wrote:

On Wed, Dec 6, 2023 at 4:29 AM Mario Limonciello
 wrote:


On 12/5/2023 14:17, Hamza Mahfooz wrote:

We currently don't support dirty rectangles on hardware rotated modes.
So, if a user is using hardware rotated modes with PSR-SU enabled,
use PSR-SU FFU for all rotated planes (including cursor planes).



Here is the email for the original reporter to give an attribution tag.

Reported-by: Kai-Heng Feng 


For this particular issue,
Tested-by: Kai-Heng Feng 


Can you confirm what kernel base you tested issue against?

I ask because Bin Li (+CC) also tested it against 6.1 based LTS kernel 
but ran into problems.


I wonder if it's because of other dependency patches.  If that's the 
case it would be good to call them out in the Cc: @stable as 
dependencies so when Greg or Sasha backport this 6.1 doesn't get broken.


Bin,

Could you run ./scripts/decode_stacktrace.sh on your kernel trace to 
give us a specific line number on the issue you hit?


Thanks!





Cc: sta...@vger.kernel.org
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2952
Fixes: 30ebe41582d1 ("drm/amd/display: add FB_DAMAGE_CLIPS support")
Signed-off-by: Hamza Mahfooz 
---
   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c|  4 
   drivers/gpu/drm/amd/display/dc/dc_hw_types.h |  1 +
   drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubp.c| 12 ++--
   .../gpu/drm/amd/display/dc/hwss/dcn10/dcn10_hwseq.c  |  3 ++-
   4 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index c146dc9cba92..79f8102d2601 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5208,6 +5208,7 @@ static void fill_dc_dirty_rects(struct drm_plane *plane,
   bool bb_changed;
   bool fb_changed;
   u32 i = 0;
+


Looks like a spurious newline here.


   *dirty_regions_changed = false;

   /*
@@ -5217,6 +5218,9 @@ static void fill_dc_dirty_rects(struct drm_plane *plane,
   if (plane->type == DRM_PLANE_TYPE_CURSOR)
   return;

+ if (new_plane_state->rotation != DRM_MODE_ROTATE_0)
+ goto ffu;
+


I noticed that the original report was specifically on 180°.  Since
you're also covering 90° and 270° with this check it sounds like it's
actually problematic on those too?


90 & 270 are problematic too. But from what I observed the issue is
much more than just cursors.


Got it; thanks.



Kai-Heng




   num_clips = drm_plane_get_damage_clips_count(new_plane_state);
   clips = drm_plane_get_damage_clips(new_plane_state);

diff --git a/drivers/gpu/drm/amd/display/dc/dc_hw_types.h 
b/drivers/gpu/drm/amd/display/dc/dc_hw_types.h
index 9649934ea186..e2a3aa8812df 100644
--- a/drivers/gpu/drm/amd/display/dc/dc_hw_types.h
+++ b/drivers/gpu/drm/amd/display/dc/dc_hw_types.h
@@ -465,6 +465,7 @@ struct dc_cursor_mi_param {
   struct fixed31_32 v_scale_ratio;
   enum dc_rotation_angle rotation;
   bool mirror;
+ struct dc_stream_state *stream;
   };

   /* IPP related types */
diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubp.c 
b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubp.c
index 139cf31d2e45..89c3bf0fe0c9 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubp.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubp.c
@@ -1077,8 +1077,16 @@ void hubp2_cursor_set_position(
   if (src_y_offset < 0)
   src_y_offset = 0;
   /* Save necessary cursor info x, y position. w, h is saved in attribute 
func. */
- hubp->cur_rect.x = src_x_offset + param->viewport.x;
- hubp->cur_rect.y = src_y_offset + param->viewport.y;
+ if (param->stream->link->psr_settings.psr_version >= DC_PSR_VERSION_SU_1 
&&
+ param->rotation != ROTATION_ANGLE_0) {


Ditto on above about 90° and 270°.


+ hubp->cur_rect.x = 0;
+ hubp->cur_rect.y = 0;
+ hubp->cur_rect.w = param->stream->timing.h_addressable;
+ hubp->cur_rect.h = param->stream->timing.v_addressable;
+ } else {
+ hubp->cur_rect.x = src_x_offset + param->viewport.x;
+ hubp->cur_rect.y = src_y_offset + param->viewport.y;
+ }
   }

   void hubp2_clk_cntl(struct hubp *hubp, bool enable)
diff --git a/drivers/gpu/drm/amd/display/dc/hwss/dcn10/dcn10_hwseq.c 
b/drivers/gpu/drm/amd/display/dc/hwss/dcn10/dcn10_hwseq.c
index 2b8b8366538e..ce5613a76267 100644
--- a/drivers/gpu/drm/amd/display/dc/hwss/dcn10/dcn10_hwseq.c
+++ b/drivers/gpu/drm/amd/display/dc/hwss/dcn10/dcn10_hwseq.c
@@ -3417,7 +3417,8 @@ void dcn10_set_cursor_position(struct pipe_ctx *pipe_ctx)
   .h_scale_ratio = pipe_ctx->plane_res.scl_data.ratios.horz,
   .v_scale_ratio = pipe_ctx->plane_res.scl_data.ratios.vert,
   .rotation = pipe_ctx->plane_state->rotation,
- .mirror = 

Re: [PATCH] drm/amd/display: fix hw rotated modes when PSR-SU is enabled

2023-12-05 Thread Mario Limonciello

On 12/5/2023 14:17, Hamza Mahfooz wrote:

We currently don't support dirty rectangles on hardware rotated modes.
So, if a user is using hardware rotated modes with PSR-SU enabled,
use PSR-SU FFU for all rotated planes (including cursor planes).



Here is the email for the original reporter to give an attribution tag.

Reported-by: Kai-Heng Feng 


Cc: sta...@vger.kernel.org
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2952
Fixes: 30ebe41582d1 ("drm/amd/display: add FB_DAMAGE_CLIPS support")
Signed-off-by: Hamza Mahfooz 
---
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c|  4 
  drivers/gpu/drm/amd/display/dc/dc_hw_types.h |  1 +
  drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubp.c| 12 ++--
  .../gpu/drm/amd/display/dc/hwss/dcn10/dcn10_hwseq.c  |  3 ++-
  4 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index c146dc9cba92..79f8102d2601 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5208,6 +5208,7 @@ static void fill_dc_dirty_rects(struct drm_plane *plane,
bool bb_changed;
bool fb_changed;
u32 i = 0;
+


Looks like a spurious newline here.


*dirty_regions_changed = false;
  
  	/*

@@ -5217,6 +5218,9 @@ static void fill_dc_dirty_rects(struct drm_plane *plane,
if (plane->type == DRM_PLANE_TYPE_CURSOR)
return;
  
+	if (new_plane_state->rotation != DRM_MODE_ROTATE_0)

+   goto ffu;
+


I noticed that the original report was specifically on 180°.  Since 
you're also covering 90° and 270° with this check it sounds like it's 
actually problematic on those too?



num_clips = drm_plane_get_damage_clips_count(new_plane_state);
clips = drm_plane_get_damage_clips(new_plane_state);
  
diff --git a/drivers/gpu/drm/amd/display/dc/dc_hw_types.h b/drivers/gpu/drm/amd/display/dc/dc_hw_types.h

index 9649934ea186..e2a3aa8812df 100644
--- a/drivers/gpu/drm/amd/display/dc/dc_hw_types.h
+++ b/drivers/gpu/drm/amd/display/dc/dc_hw_types.h
@@ -465,6 +465,7 @@ struct dc_cursor_mi_param {
struct fixed31_32 v_scale_ratio;
enum dc_rotation_angle rotation;
bool mirror;
+   struct dc_stream_state *stream;
  };
  
  /* IPP related types */

diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubp.c 
b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubp.c
index 139cf31d2e45..89c3bf0fe0c9 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubp.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubp.c
@@ -1077,8 +1077,16 @@ void hubp2_cursor_set_position(
if (src_y_offset < 0)
src_y_offset = 0;
/* Save necessary cursor info x, y position. w, h is saved in attribute 
func. */
-   hubp->cur_rect.x = src_x_offset + param->viewport.x;
-   hubp->cur_rect.y = src_y_offset + param->viewport.y;
+   if (param->stream->link->psr_settings.psr_version >= DC_PSR_VERSION_SU_1 
&&
+   param->rotation != ROTATION_ANGLE_0) {


Ditto on above about 90° and 270°.


+   hubp->cur_rect.x = 0;
+   hubp->cur_rect.y = 0;
+   hubp->cur_rect.w = param->stream->timing.h_addressable;
+   hubp->cur_rect.h = param->stream->timing.v_addressable;
+   } else {
+   hubp->cur_rect.x = src_x_offset + param->viewport.x;
+   hubp->cur_rect.y = src_y_offset + param->viewport.y;
+   }
  }
  
  void hubp2_clk_cntl(struct hubp *hubp, bool enable)

diff --git a/drivers/gpu/drm/amd/display/dc/hwss/dcn10/dcn10_hwseq.c 
b/drivers/gpu/drm/amd/display/dc/hwss/dcn10/dcn10_hwseq.c
index 2b8b8366538e..ce5613a76267 100644
--- a/drivers/gpu/drm/amd/display/dc/hwss/dcn10/dcn10_hwseq.c
+++ b/drivers/gpu/drm/amd/display/dc/hwss/dcn10/dcn10_hwseq.c
@@ -3417,7 +3417,8 @@ void dcn10_set_cursor_position(struct pipe_ctx *pipe_ctx)
.h_scale_ratio = pipe_ctx->plane_res.scl_data.ratios.horz,
.v_scale_ratio = pipe_ctx->plane_res.scl_data.ratios.vert,
.rotation = pipe_ctx->plane_state->rotation,
-   .mirror = pipe_ctx->plane_state->horizontal_mirror
+   .mirror = pipe_ctx->plane_state->horizontal_mirror,
+   .stream = pipe_ctx->stream


As a nit; I think it's worth leaving a harmless trailing ',' so that 
there is less ping pong in the future when adding new members to a struct.



};
bool pipe_split_on = false;
bool odm_combine_on = (pipe_ctx->next_odm_pipe != NULL) ||