From: Rodrigo Siqueira <[email protected]>
[ Upstream commit 33f409e60eb0c59a4d0d06a62ab4642a988e17f7 ]
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 <[email protected]>
Cc: Harry Wentland <[email protected]>
Cc: Nicholas Choi <[email protected]>
Cc: Bhawanpreet Lakha <[email protected]>
Cc: Nicholas Kazlauskas <[email protected]>
Cc: Mark Yacoub <[email protected]>
Cc: Daniel Wheeler <[email protected]>
Tested-by: Daniel Wheeler <[email protected]>
Signed-off-by: Rodrigo Siqueira <[email protected]>
Signed-off-by: Alex Deucher <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
---
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 6e31e899192c..29657844bac1 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -7272,7 +7272,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) {
@@ -7302,6 +7302,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;
+
/* 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 ||
--
2.30.2
Hi Greg
Please drop this from 5.4, 5.10, or 5.12 stable kernels as it's apt to break any userspace that is using the legacy cursor IOCTL, which is most userspace.
We are in the process of reverting this on amd-staging-drm-next. Siqueira will send a patch.
Thanks,
Harry
On 2021-06-16 11:33 a.m., Greg Kroah-Hartman wrote:
> From: Rodrigo Siqueira <[email protected]>
>
> [ Upstream commit 33f409e60eb0c59a4d0d06a62ab4642a988e17f7 ]
>
> 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 <[email protected]>
> Cc: Harry Wentland <[email protected]>
> Cc: Nicholas Choi <[email protected]>
> Cc: Bhawanpreet Lakha <[email protected]>
> Cc: Nicholas Kazlauskas <[email protected]>
> Cc: Mark Yacoub <[email protected]>
> Cc: Daniel Wheeler <[email protected]>
>
> Tested-by: Daniel Wheeler <[email protected]>
> Signed-off-by: Rodrigo Siqueira <[email protected]>
> Signed-off-by: Alex Deucher <[email protected]>
> Signed-off-by: Sasha Levin <[email protected]>
> ---
> 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 6e31e899192c..29657844bac1 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -7272,7 +7272,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) {
> @@ -7302,6 +7302,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;
> +
> /* 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 ||
On Wed, Jun 16, 2021 at 11:59:55AM -0400, Harry Wentland wrote:
> Hi Greg
>
> Please drop this from 5.4, 5.10, or 5.12 stable kernels as it's apt to break any userspace that is using the legacy cursor IOCTL, which is most userspace.
>
> We are in the process of reverting this on amd-staging-drm-next. Siqueira will send a patch.
Dropped from all 3 trees now, thanks!
greg k-h