[PATCH] amd/display: remove ChromeOS workaround

[Paul Menzel]

Dear Simon,


Am 14.10.21 um 17:35 schrieb Simon Ser:
> 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.

Excuse my ignorance. Reading the commit message, there was a Linux 
kernel change, that broke Chrome OS userspace, right? If so, and we do 
not know if there is other userspace using the API incorrectly, 
shouldn’t the patch breaking Chrome OS userspace be reverted to adhere 
to Linux’ no-regression rule?


Kind regards,

Paul


> [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,
>