[PATCH] amd/display: remove ChromeOS workaround

[Alex Deucher]

On Thu, Oct 21, 2021 at 2:19 PM Harry Wentland <harry.wentland at amd.com> wrote:
>
>
>
> On 2021-10-21 13:55, Rodrigo Siqueira Jordao wrote:
> > Hi Simon,
> >
> > I tested this patch and it lgtm. I also agree to revert it.
> >
> > Btw, did you send the revert patch for "amd/display: only require overlay plane to cover whole CRTC on ChromeOS"? I think we need to revert it as well.
> >
>
> Agreed that this patch is good but we'll need to also revert the is_chromeos w/a.

I've reverted that and applied this one.  Thanks!

Alex

>
> This patch is
> Reviewed-by: Harry Wentland <harry.wentland at amd.com>
>
> Harry
>
> > Sean, Mark
> >
> > For ChromeOS, we should ignore this patch. Do we need to take any action to avoid landing this patch on ChromeOS tree?
> >
> > Thanks
> > Siqueira
> >
> > On 2021-10-14 11:35 a.m., Simon Ser wrote:
> >> This reverts commits ddab8bd788f5 ("drm/amd/display: Fix two cursor duplication
> >> when using overlay") and e7d9560aeae5 ("Revert "drm/amd/display: Fix overlay
> >> validation by considering cursors"").
> >>
> >> tl;dr ChromeOS uses the atomic interface for everything except the cursor. This
> >> is incorrect and forces amdgpu to disable some hardware features. Let's revert
> >> the ChromeOS-specific workaround in mainline and allow the Chrome team to keep
> >> it internally in their own tree.
> >>
> >> See [1] for more details. This patch is an alternative to [2], which added
> >> ChromeOS detection.
> >>
> >> [1]: https://lore.kernel.org/amd-gfx/JIQ_93_cHcshiIDsrMU1huBzx9P9LVQxucx8hQArpQu7Wk5DrCl_vTXj_Q20m_L-8C8A5dSpNcSJ8ehfcCrsQpfB5QG_Spn14EYkH9chtg0=@emersion.fr/>> [2]: https://lore.kernel.org/amd-gfx/20211011151609.452132-1-contact@emersion.fr/>> Signed-off-by: Simon Ser <contact at emersion.fr>
> >> Cc: Alex Deucher <alexander.deucher at amd.com>
> >> Cc: Harry Wentland <hwentlan at amd.com>
> >> Cc: Nicholas Kazlauskas <nicholas.kazlauskas at amd.com>
> >> Cc: Bas Nieuwenhuizen <bas at basnieuwenhuizen.nl>
> >> Cc: Rodrigo Siqueira <Rodrigo.Siqueira at amd.com>
> >> Cc: Sean Paul <seanpaul at chromium.org>
> >> Fixes: ddab8bd788f5 ("drm/amd/display: Fix two cursor duplication when using overlay")
> >> Fixes: e7d9560aeae5 ("Revert "drm/amd/display: Fix overlay validation by considering cursors"")
> >> ---
> >>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 51 -------------------
> >>   1 file changed, 51 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 20065a145851..014c5a9fe461 100644
> >> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> >> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> >> @@ -10628,53 +10628,6 @@ static int add_affected_mst_dsc_crtcs(struct drm_atomic_state *state, struct drm
> >>   }
> >>   #endif
> >>   -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, *overlay_state = NULL;
> >> -
> >> -    /* Check if primary plane is contained inside overlay */
> >> -    for_each_new_plane_in_state_reverse(state, plane, new_plane_state, i) {
> >> -        if (plane->type == DRM_PLANE_TYPE_OVERLAY) {
> >> -            if (drm_atomic_plane_disabling(plane->state, new_plane_state))
> >> -                return 0;
> >> -
> >> -            overlay_state = new_plane_state;
> >> -            continue;
> >> -        }
> >> -    }
> >> -
> >> -    /* check if we're making changes to the overlay plane */
> >> -    if (!overlay_state)
> >> -        return 0;
> >> -
> >> -    /* check if overlay plane is enabled */
> >> -    if (!overlay_state->crtc)
> >> -        return 0;
> >> -
> >> -    /* find the primary plane for the CRTC that the overlay is enabled on */
> >> -    primary_state = drm_atomic_get_plane_state(state, overlay_state->crtc->primary);
> >> -    if (IS_ERR(primary_state))
> >> -        return PTR_ERR(primary_state);
> >> -
> >> -    /* check if primary plane is enabled */
> >> -    if (!primary_state->crtc)
> >> -        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 ||
> >> -        primary_state->crtc_x + primary_state->crtc_w > overlay_state->crtc_x + overlay_state->crtc_w ||
> >> -        primary_state->crtc_y + primary_state->crtc_h > overlay_state->crtc_y + overlay_state->crtc_h) {
> >> -        DRM_DEBUG_ATOMIC("Overlay plane is enabled with hardware cursor but does not fully cover primary plane\n");
> >> -        return -EINVAL;
> >> -    }
> >> -
> >> -    return 0;
> >> -}
> >> -
> >>   /**
> >>    * amdgpu_dm_atomic_check() - Atomic check implementation for AMDgpu DM.
> >>    * @dev: The DRM device
> >> @@ -10856,10 +10809,6 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
> >>               goto fail;
> >>       }
> >>   -    ret = validate_overlay(state);
> >> -    if (ret)
> >> -        goto fail;
> >> -
> >>       /* Add new/modified planes */
> >>       for_each_oldnew_plane_in_state_reverse(state, plane, old_plane_state, new_plane_state, i) {
> >>           ret = dm_update_plane_state(dc, state, plane,
> >>
> >
>