Re: [PATCH] amd/display: remove ChromeOS workaround

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxx/

Signed-off-by: Simon Ser <contact@xxxxxxxxxxx>
Cc: Alex Deucher <alexander.deucher@xxxxxxx>
Cc: Harry Wentland <hwentlan@xxxxxxx>
Cc: Nicholas Kazlauskas <nicholas.kazlauskas@xxxxxxx>
Cc: Bas Nieuwenhuizen <bas@xxxxxxxxxxxxxxxxxxx>
Cc: Rodrigo Siqueira <Rodrigo.Siqueira@xxxxxxx>
Cc: Sean Paul <seanpaul@xxxxxxxxxxxx>
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,




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux