On 2021-05-14 7:47 a.m., Rodrigo Siqueira wrote:
> A few weeks ago, we saw a two cursor issue in a ChromeOS system. We
> fixed it in the commit:
> 
>  drm/amd/display: Fix two cursor duplication when using overlay
>  (read the commit message for more details)
> 
> After this change, we noticed that some IGT subtests related to
> kms_plane and kms_plane_scaling started to fail. After investigating
> this issue, we noticed that all subtests that fail have a primary plane
> covering the overlay plane, which is currently rejected by amdgpu dm.
> Fail those IGT tests highlight that our verification was too broad and
> compromises the overlay usage in our drive. This patch fixes this issue
> by ensuring that we only reject commits where the primary plane is not
> fully covered by the overlay when the cursor hardware is enabled. With
> this fix, all IGT tests start to pass again, which means our overlay
> support works as expected.
> 
> Cc: Tianci.Yin <tianci....@amd.com>
> Cc: Harry Wentland <harry.wentl...@amd.com>
> Cc: Nicholas Choi <nicholas.c...@amd.com>
> Cc: Bhawanpreet Lakha <bhawanpreet.la...@amd.com>
> Cc: Nicholas Kazlauskas <nicholas.kazlaus...@amd.com>
> Cc: Mark Yacoub <markyac...@google.com>
> Cc: Daniel Wheeler <daniel.whee...@amd.com>
> 
> Signed-off-by: Rodrigo Siqueira <rodrigo.sique...@amd.com>
> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> 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 ccd67003b120..9c2537a17a7b 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -10067,7 +10067,7 @@ static int validate_overlay(struct drm_atomic_state 
> *state)
>       int i;
>       struct drm_plane *plane;
>       struct drm_plane_state *old_plane_state, *new_plane_state;
> -     struct drm_plane_state *primary_state, *overlay_state = NULL;
> +     struct drm_plane_state *primary_state, *cursor_state, *overlay_state = 
> NULL;
>  
>       /* Check if primary plane is contained inside overlay */
>       for_each_oldnew_plane_in_state_reverse(state, plane, old_plane_state, 
> new_plane_state, i) {
> @@ -10097,6 +10097,14 @@ 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;
> +

I thought this breaks Chrome's compositor since it can't handle an atomic 
commit rejection
based solely on whether a cursor is enabled or not.

Harry

>       /* 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 ||
> 

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to