On 2020-11-12 3:56 p.m., Alex Deucher wrote:
On Thu, Nov 12, 2020 at 3:07 PM Simon Ser <contact@xxxxxxxxxxx> wrote:
CC Daniel Vetter and Bas, see below…
On Thursday, November 12, 2020 8:56 PM, Kazlauskas, Nicholas <nicholas.kazlauskas@xxxxxxx> wrote:
Reviewed-by: Nicholas Kazlauskasnicholas.kazlauskas@xxxxxxx
Thanks for the review!
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.
Interesting.
Daniel Vetter: what would be the best way to expose this to user-space?
Maybe we should just make atomic commits with a cursor plane fail when
scaling is used on the primary plane?
Disabling the cursor plane sounds better than displaying the wrong
image.
Cursor can be independently rotated in hardware but this isn't something
we expose support for to userspace.
Hmm, I see that cursor planes have the "rotation" property exposed:
"rotation": bitmask {rotate-0, rotate-90, rotate-180, rotate-270}
In fact all planes have it. It's done in amdgpu_dm_plane_init (behind a
`dm->adev->asic_type >= CHIP_BONAIRE` condition).
Is this an oversight?
The pitch check of 64/128/256 is OK but we don't support 256 on DCE.
Yeah, I've noticed that. The size check right above should catch it
in most cases I think, because max_cursor_size is 128 on DCE. Side
note, max_cursor_size is 64 on DCE 6.0.
Maybe something like:
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 77c06f999040..a1ea195a7041 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -8999,6 +8999,43 @@ static int dm_update_plane_state(struct dc *dc,
return -EINVAL;
}
+ switch(adev->family) {
+ case AMDGPU_FAMILY_SI:
+ /* DCE6 only supports 64x64 cursors */
+ if (new_plane_state->fb->pitches[0] == 64) {
+ break;
+ } else {
+ DRM_DEBUG_ATOMIC("Bad cursor pitch %d\n",
+
new_plane_state->fb->pitches[0]);
+ return -EINVAL;
+ }
+ case AMDGPU_FAMILY_KV:
+ case AMDGPU_FAMILY_CI:
+ case AMDGPU_FAMILY_CZ:
+ case AMDGPU_FAMILY_VI:
+ case AMDGPU_FAMILY_AI:
+ /* DCE8-12 only supports 64x64 and 128x128 cursors */
+ if ((new_plane_state->fb->pitches[0] == 64) ||
+ (new_plane_state->fb->pitches[0] == 128)) {
+ break;
+ } else {
+ DRM_DEBUG_ATOMIC("Bad cursor pitch %d\n",
+
new_plane_state->fb->pitches[0]);
+ return -EINVAL;
+ }
+ default:
+ /* DCN supports 64x64, 128x128, and 256x256 cursors */
+ if ((new_plane_state->fb->pitches[0] == 64) ||
+ (new_plane_state->fb->pitches[0] == 128) ||
+ (new_plane_state->fb->pitches[0] == 256)) {
+ break;
+ } else {
+ DRM_DEBUG_ATOMIC("Bad cursor pitch %d\n",
+
new_plane_state->fb->pitches[0]);
+ return -EINVAL;
+ }
+ }
+
If we're going to extend it to something like this I think that this
should be extracted out to its own function to reduce some of this
indenting.
I think the simpler approach here is to just block 256 if the
max_cursor_size in amdgpu is 128.
Regards,
Nicholas Kazlauskas
return 0;
}
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=04%7C01%7Cnicholas.kazlauskas%40amd.com%7C70872c6a2aa944b67ac408d8874d6871%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637408113864433577%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=nIkL9sjlwRKQl3akctzIlTUBD0fBTsx9TfQ9GBtZhqE%3D&reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
