Applied. thanks!
Alex
On Thu, Dec 2, 2021 at 10:09 AM Kazlauskas, Nicholas
<nicholas.kazlauskas@xxxxxxx> 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@xxxxxxx>
>
> 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@xxxxxxx>
>
> 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) {
> >
>
