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.
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@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,
