[PATCH] Revert "drm/amd/display: Fix overlay validation by considering cursors"

Sean Paul seanpaul at chromium.org
Wed Jun 16 17:14:36 UTC 2021


On Wed, Jun 16, 2021 at 12:21 PM Rodrigo Siqueira
<Rodrigo.Siqueira at amd.com> wrote:
>
> This reverts commit 04cc17a951f73f9a9092ca572b063e6292aeb085.
>
> The patch that we are reverting here was originally applied because it
> fixes multiple IGT issues and flickering in Android. However, after a
> discussion with Sean Paul and Mark, it looks like that this patch might
> cause problems on ChromeOS. For this reason, we decided to revert this
> patch.

Thanks for sending this, Siqueira!

To be clear for those unfamiliar, the issue extends beyond ChromeOS
(we're not just pushing our compositor problems on the rest of the
community).

Relying on cursor enable/disable for atomic creates non-deterministic
behavior which would be very hard for any compositor to reason out
without knowing the hardware-specific limitations. The case I'm
worried about is that the compositor has an overlay active without the
cursor and at some point the compositor enables the cursor which will
fail because of the overlay.

Reviewed-by: Sean Paul <seanpaul at chromium.org>

>
> Cc: Nicholas Kazlauskas <Nicholas.Kazlauskas at amd.com>
> Cc: Harry Wentland <Harry.Wentland at amd.com>
> Cc: Hersen Wu <hersenxs.wu at amd.com>
> Cc: Sean Paul <seanpaul at chromium.org>
> Cc: Mark Yacoub <markyacoub at chromium.org>
> Cc: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
> Signed-off-by: Rodrigo Siqueira <Rodrigo.Siqueira at amd.com>
> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 12 ++----------
>  1 file changed, 2 insertions(+), 10 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 8358112b5822..3fd41e098c90 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -10200,8 +10200,8 @@ static int validate_overlay(struct drm_atomic_state *state)
>  {
>         int i;
>         struct drm_plane *plane;
> -       struct drm_plane_state *new_plane_state;
> -       struct drm_plane_state *primary_state, *cursor_state, *overlay_state = NULL;
> +       struct drm_plane_state *old_plane_state, *new_plane_state;
> +       struct drm_plane_state *primary_state, *overlay_state = NULL;
>
>         /* Check if primary plane is contained inside overlay */
>         for_each_new_plane_in_state_reverse(state, plane, new_plane_state, i) {
> @@ -10231,14 +10231,6 @@ static int validate_overlay(struct drm_atomic_state *state)
>         if (!primary_state->crtc)
>                 return 0;
>
> -       /* check if cursor plane is enabled */
> -       cursor_state = drm_atomic_get_plane_state(state, overlay_state->crtc->cursor);
> -       if (IS_ERR(cursor_state))
> -               return PTR_ERR(cursor_state);
> -
> -       if (drm_atomic_plane_disabling(plane->state, cursor_state))
> -               return 0;
> -
>         /* Perform the bounds check to ensure the overlay plane covers the primary */
>         if (primary_state->crtc_x < overlay_state->crtc_x ||
>             primary_state->crtc_y < overlay_state->crtc_y ||
> --
> 2.25.1
>


More information about the amd-gfx mailing list