Re: [PATCH] drm/amd/display: add cursor pitch check

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

 



On 2020-11-12 12:37 p.m., Simon Ser wrote:
This patch expands the cursor checks added in "drm/amd/display: add basic
atomic check for cursor plane" to also include a pitch check. Without
this patch, setting a FB smaller than max_cursor_size with an invalid
pitch would result in amdgpu error messages and a fallback to a 64-byte
pitch:

     [drm:hubp1_cursor_set_attributes [amdgpu]] *ERROR* Invalid cursor pitch of 100. Only 64/128/256 is supported on DCN.

Signed-off-by: Simon Ser <contact@xxxxxxxxxxx>
Reported-by: Pierre-Loup A. Griffais <pgriffais@xxxxxxxxxxxxxxxxx>
Cc: Alex Deucher <alexander.deucher@xxxxxxx>
Cc: Harry Wentland <hwentlan@xxxxxxx>
Cc: Nicholas Kazlauskas <nicholas.kazlauskas@xxxxxxx>

Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@xxxxxxx>

But with some comments below:

---

Couple questions:

- This implements a single check for all GPU generations. Is my
   assumption correct here? It seems like this check is OK for at least
   DCN 1.0 and DCN 2.0.
- We should really implement better checks. What features are supported
   on the cursor plane? Is scaling supported? Is cropping supported? Is
   rotation always supported?

On DCE and DCN there is no dedicated hardware cursor plane. You get a cursor per pipe but it's going to inherit the scaling and positioning from the underlying pipe.

There's software logic to ensure we position the cursor in the correct location in CRTC space independent on the underlying DRM plane's scaling and positioning but there's no way for us to correct the scaling. Cursor will always be 64, 128, or 256 in the pipe's destination space.

Cursor can be independently rotated in hardware but this isn't something we expose support for to userspace.

The pitch check of 64/128/256 is OK but we don't support 256 on DCE.

Regards,
Nicholas Kazlauskas


  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 14 ++++++++++++++
  1 file changed, 14 insertions(+)

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 2855bb918535..42b0ade7de39 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -8902,6 +8902,20 @@ static int dm_update_plane_state(struct dc *dc,
  			return -EINVAL;
  		}
+ if (new_plane_state->fb) {
+			switch (new_plane_state->fb->pitches[0]) {
+			case 64:
+			case 128:
+			case 256:
+				/* Pitch is supported by cursor plane */
+				break;
+			default:
+				DRM_DEBUG_ATOMIC("Bad cursor pitch %d\n",
+						 new_plane_state->fb->pitches[0]);
+				return -EINVAL;
+			}
+		}
+
  		return 0;
  	}

_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx



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

  Powered by Linux