[PATCH v2] drm/amd/display: Use oriented source size when checking cursor scaling

[Alex Deucher]

Applied.  thanks!

Alex

On Thu, Dec 2, 2021 at 10:09 AM Kazlauskas, Nicholas
<nicholas.kazlauskas at amd.com> wrote:
>
> On 2021-12-02 7:52 a.m., Vlad Zahorodnii wrote:
> > dm_check_crtc_cursor() doesn't take into account plane transforms when
> > calculating plane scaling, this can result in false positives.
> >
> > For example, if there's an output with resolution 3840x2160 and the
> > output is rotated 90 degrees, CRTC_W and CRTC_H will be 3840 and 2160,
> > respectively, but SRC_W and SRC_H will be 2160 and 3840, respectively.
> >
> > Since the cursor plane usually has a square buffer attached to it, the
> > dm_check_crtc_cursor() will think that there's a scale factor mismatch
> > even though there isn't really.
> >
> > This fixes an issue where kwin fails to use hardware plane transforms.
> >
> > Changes since version 1:
> > - s/orientated/oriented/g
> >
> > Signed-off-by: Vlad Zahorodnii <vlad.zahorodnii at kde.org>
>
> This looks correct to me. I guess it's also not modifying the actual
> programming position, just the check to ensure that the cursor is going
> to be unscaled in the correct orientation.
>
> Would be good to have some IGT tests for these scaled cases to verify
> atomic check pass/fail assumptions, but for now:
>
> Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas at amd.com>
>
> Regards,
> Nicholas Kazlauskas
>
> > ---
> >   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 35 ++++++++++++++-----
> >   1 file changed, 27 insertions(+), 8 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 a3c0f2e4f4c1..c009c668fbe2 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > @@ -10736,6 +10736,24 @@ static int dm_update_plane_state(struct dc *dc,
> >       return ret;
> >   }
> >
> > +static void dm_get_oriented_plane_size(struct drm_plane_state *plane_state,
> > +                                    int *src_w, int *src_h)
> > +{
> > +     switch (plane_state->rotation & DRM_MODE_ROTATE_MASK) {
> > +     case DRM_MODE_ROTATE_90:
> > +     case DRM_MODE_ROTATE_270:
> > +             *src_w = plane_state->src_h >> 16;
> > +             *src_h = plane_state->src_w >> 16;
> > +             break;
> > +     case DRM_MODE_ROTATE_0:
> > +     case DRM_MODE_ROTATE_180:
> > +     default:
> > +             *src_w = plane_state->src_w >> 16;
> > +             *src_h = plane_state->src_h >> 16;
> > +             break;
> > +     }
> > +}
> > +
> >   static int dm_check_crtc_cursor(struct drm_atomic_state *state,
> >                               struct drm_crtc *crtc,
> >                               struct drm_crtc_state *new_crtc_state)
> > @@ -10744,6 +10762,8 @@ static int dm_check_crtc_cursor(struct drm_atomic_state *state,
> >       struct drm_plane_state *new_cursor_state, *new_underlying_state;
> >       int i;
> >       int cursor_scale_w, cursor_scale_h, underlying_scale_w, underlying_scale_h;
> > +     int cursor_src_w, cursor_src_h;
> > +     int underlying_src_w, underlying_src_h;
> >
> >       /* On DCE and DCN there is no dedicated hardware cursor plane. We get a
> >        * cursor per pipe but it's going to inherit the scaling and
> > @@ -10755,10 +10775,9 @@ static int dm_check_crtc_cursor(struct drm_atomic_state *state,
> >               return 0;
> >       }
> >
> > -     cursor_scale_w = new_cursor_state->crtc_w * 1000 /
> > -                      (new_cursor_state->src_w >> 16);
> > -     cursor_scale_h = new_cursor_state->crtc_h * 1000 /
> > -                      (new_cursor_state->src_h >> 16);
> > +     dm_get_oriented_plane_size(new_cursor_state, &cursor_src_w, &cursor_src_h);
> > +     cursor_scale_w = new_cursor_state->crtc_w * 1000 / cursor_src_w;
> > +     cursor_scale_h = new_cursor_state->crtc_h * 1000 / cursor_src_h;
> >
> >       for_each_new_plane_in_state_reverse(state, underlying, new_underlying_state, i) {
> >               /* Narrow down to non-cursor planes on the same CRTC as the cursor */
> > @@ -10769,10 +10788,10 @@ static int dm_check_crtc_cursor(struct drm_atomic_state *state,
> >               if (!new_underlying_state->fb)
> >                       continue;
> >
> > -             underlying_scale_w = new_underlying_state->crtc_w * 1000 /
> > -                                  (new_underlying_state->src_w >> 16);
> > -             underlying_scale_h = new_underlying_state->crtc_h * 1000 /
> > -                                  (new_underlying_state->src_h >> 16);
> > +             dm_get_oriented_plane_size(new_underlying_state,
> > +                                        &underlying_src_w, &underlying_src_h);
> > +             underlying_scale_w = new_underlying_state->crtc_w * 1000 / underlying_src_w;
> > +             underlying_scale_h = new_underlying_state->crtc_h * 1000 / underlying_src_h;
> >
> >               if (cursor_scale_w != underlying_scale_w ||
> >                   cursor_scale_h != underlying_scale_h) {
> >
>