Re: [PATCH] drm/amd/display: fix hw rotated modes when PSR-SU is enabled
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
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
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
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
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
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
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
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
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
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) ||