Skip to content
This repository was archived by the owner on Oct 25, 2021. It is now read-only.

Respect cursor cap #46

Closed
subdiff opened this issue Mar 24, 2020 · 14 comments
Closed

Respect cursor cap #46

subdiff opened this issue Mar 24, 2020 · 14 comments

Comments

@subdiff
Copy link
Contributor

subdiff commented Mar 24, 2020

The cursor plane is one of the planes being allocated in libliftoff. To the cursor plane only layers with very limited dimensions can be allocated. These dimensions are queried (here the width) with a call to:

drmGetCap(drm_fd, DRM_CAP_CURSOR_WIDTH, &cap)

These dimensions seem to not be respected at atomic test commit time, at least on AMD leading to a valid allocation but corrupted depiction.

libliftoff should query the cursor cap on its own for every cursor plane and check on an allocation if the dimensions of the layer are in line with the cap.
@subdiff subdiff mentioned this issue Mar 24, 2020
Closed
@emersion
Copy link
Owner

emersion commented Mar 24, 2020 •
edited
Loading

It sounds like the atomic driver isn't checking properly these properties. Checking that the buffer chosen for the cursor plane can be displayed correctly is the kernel driver job. If it's not possible, the driver should make the atomic test-only commit fail.

So I'd say this is an amdgpu bug.

@subdiff
Copy link
Contributor Author

subdiff commented Mar 24, 2020

For reference a conversation I had some time ago about this topic on dri-devel:

[Freitag, 28. Februar 2020] [17:55:36 CET] On AMD I am trying to put a cursor image on a cursor plane via atomic mode setting. Test commit succeeds but the image is scrambled. Kernel log says:
[Freitag, 28. Februar 2020] [17:55:38 CET] [drm:hubp1_get_cursor_pitch [amdgpu]] ERROR Invalid cursor pitch of 48. Only 64/128/256 is supported on DCN
[Freitag, 28. Februar 2020] [17:56:05 CET] Normally I would say the buffer I provided is broken. But shouldn't the test commit fail in this case?
[Freitag, 28. Februar 2020] [18:00:27 CET] romangg: I agree that it should; kazlaus hwentlan ^
[Freitag, 28. Februar 2020] [18:01:17 CET] yeah
[Freitag, 28. Februar 2020] [18:02:45 CET] i don't know why we don't have the checks for pitch in atomic check
[Freitag, 28. Februar 2020] [18:02:54 CET] hopefully no userspace relies on it being broken though

So that would confirm what you said @emersion. Question is if we want a check in there anyway at least until AMD fixes their driver since it's quite an important platform for Wayland graphics (development).

@emersion
Copy link
Owner

Question is if we want a check in there anyway at least until AMD fixes their driver since it's quite an important platform for Wayland graphics (development).

One hurdle is that there's no DRM_CAP_CURSOR_* for pitch/stride.

In your compositor, are you allocating the cursor buffer with GBM_BO_USE_CURSOR and the right size?

@emersion
Copy link
Owner

Ah, or is something other than the cursor ending up in the cursor plane somehow?

@subdiff
Copy link
Contributor Author

subdiff commented Mar 24, 2020

Ah, or is something other than the cursor ending up in the cursor plane somehow?

Yes, a nice big notification message. ;)

@emersion
Copy link
Owner

emersion commented Mar 25, 2020 •
edited
Loading

Does this kernel patch fix your issue? https://l.sr.ht/3do6.patch

@emersion
Copy link
Owner

Tested and submitted the patch: https://lists.freedesktop.org/archives/amd-gfx/2020-March/047825.html

@subdiff
Copy link
Contributor Author

subdiff commented May 2, 2020 •
edited
Loading

What's the current state? If I read the mailing list thread correctly the patch is not yet accepted?

Should we try to add a check in libliftoff in the meantime?

@subdiff subdiff mentioned this issue May 2, 2020
Closed
@emersion
Copy link
Owner

emersion commented May 4, 2020

Ping'ed the AMD devs.

ruscur pushed a commit to ruscur/linux that referenced this issue May 8, 2020
@emersion @alexdeucher
drm/amd/display: add basic atomic check for cursor plane
e133020 
This patch adds a basic cursor check when an atomic test-only commit is
performed. The position and size of the cursor plane is checked.

This should fix user-space relying on atomic checks to assign buffers to
planes.

Signed-off-by: Simon Ser <[email protected]>
Reported-by: Roman Gilg <[email protected]>
References: emersion/libliftoff#46
Cc: Alex Deucher <[email protected]>
Cc: Harry Wentland <[email protected]>
Reviewed-by: Nicholas Kazlauskas <[email protected]>
Signed-off-by: Alex Deucher <[email protected]>
@emersion
Copy link
Owner

emersion commented May 8, 2020

AMD patch merged: https://cgit.freedesktop.org/~agd5f/linux/commit/?h=drm-next&id=e133020f92b9397eaad83ff1dada6d9786edcbd0

@subdiff
Copy link
Contributor Author

subdiff commented May 8, 2020

Awesome! :) You know if this will be kernel 5.8?

@emersion
Copy link
Owner

Not yet, we'll see if the next AMD pull request includes it.

StanFox1984 pushed a commit to StanFox1984/drm-tip that referenced this issue May 14, 2020
@emersion @alexdeucher
drm/amd/display: add basic atomic check for cursor plane
626bf90 
This patch adds a basic cursor check when an atomic test-only commit is
performed. The position and size of the cursor plane is checked.

This should fix user-space relying on atomic checks to assign buffers to
planes.

Signed-off-by: Simon Ser <[email protected]>
Reported-by: Roman Gilg <[email protected]>
References: emersion/libliftoff#46
Cc: Alex Deucher <[email protected]>
Cc: Harry Wentland <[email protected]>
Reviewed-by: Nicholas Kazlauskas <[email protected]>
Signed-off-by: Alex Deucher <[email protected]>
Cc: [email protected]
@emersion
Copy link
Owner

Cursor fix is included in the amdgpu fixes pull request for 5.7.

@emersion
Copy link
Owner

The patch has also been queued for 5.4-stable and 5.6-stable.

MilhouseVH pushed a commit to MilhouseVH/linux that referenced this issue May 20, 2020
@emersion @gregkh
drm/amd/display: add basic atomic check for cursor plane
36ab3ad 
commit 626bf90 upstream.

This patch adds a basic cursor check when an atomic test-only commit is
performed. The position and size of the cursor plane is checked.

This should fix user-space relying on atomic checks to assign buffers to
planes.

Signed-off-by: Simon Ser <[email protected]>
Reported-by: Roman Gilg <[email protected]>
References: emersion/libliftoff#46
Cc: Alex Deucher <[email protected]>
Cc: Harry Wentland <[email protected]>
Reviewed-by: Nicholas Kazlauskas <[email protected]>
Signed-off-by: Alex Deucher <[email protected]>
Cc: [email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
MilhouseVH pushed a commit to MilhouseVH/linux that referenced this issue May 20, 2020
@emersion @gregkh
drm/amd/display: add basic atomic check for cursor plane
57aa19a 
commit 626bf90 upstream.

This patch adds a basic cursor check when an atomic test-only commit is
performed. The position and size of the cursor plane is checked.

This should fix user-space relying on atomic checks to assign buffers to
planes.

Signed-off-by: Simon Ser <[email protected]>
Reported-by: Roman Gilg <[email protected]>
References: emersion/libliftoff#46
Cc: Alex Deucher <[email protected]>
Cc: Harry Wentland <[email protected]>
Reviewed-by: Nicholas Kazlauskas <[email protected]>
Signed-off-by: Alex Deucher <[email protected]>
Cc: [email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
cmaiolino pushed a commit to cmaiolino/linux that referenced this issue Jun 1, 2020
@emersion @cmaiolino
drm/amd/display: add basic atomic check for cursor plane
4278c1a 
This patch adds a basic cursor check when an atomic test-only commit is
performed. The position and size of the cursor plane is checked.

This should fix user-space relying on atomic checks to assign buffers to
planes.

Signed-off-by: Simon Ser <[email protected]>
Reported-by: Roman Gilg <[email protected]>
References: emersion/libliftoff#46
Cc: Alex Deucher <[email protected]>
Cc: Harry Wentland <[email protected]>
Reviewed-by: Nicholas Kazlauskas <[email protected]>
Signed-off-by: Alex Deucher <[email protected]>
Cc: [email protected]
jpuhlman pushed a commit to MontaVista-OpenSourceTechnology/linux-mvista that referenced this issue Jun 4, 2020
@emersion @cminyard
drm/amd/display: add basic atomic check for cursor plane
846d8e7 
Source: kernel.org
MR: 103894
Type: Integration
Disposition: Merged from git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable linux-5.4.y
ChangeID: 57aa19acfc220af0b7888996a3313795c37ecdac
Description:

commit 626bf90 upstream.

This patch adds a basic cursor check when an atomic test-only commit is
performed. The position and size of the cursor plane is checked.

This should fix user-space relying on atomic checks to assign buffers to
planes.

Signed-off-by: Simon Ser <[email protected]>
Reported-by: Roman Gilg <[email protected]>
References: emersion/libliftoff#46
Cc: Alex Deucher <[email protected]>
Cc: Harry Wentland <[email protected]>
Reviewed-by: Nicholas Kazlauskas <[email protected]>
Signed-off-by: Alex Deucher <[email protected]>
Cc: [email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
Signed-off-by: Corey Minyard <[email protected]>
pmatos pushed a commit to pmatos/ubuntu-focal that referenced this issue Jun 26, 2020
@emersion @marcelocerri
drm/amd/display: add basic atomic check for cursor plane
d2c8e24 
BugLink: https://bugs.launchpad.net/bugs/1879759

commit 626bf90fe03fa080d8df06bb0397c95c53ae8e27 upstream.

This patch adds a basic cursor check when an atomic test-only commit is
performed. The position and size of the cursor plane is checked.

This should fix user-space relying on atomic checks to assign buffers to
planes.

Signed-off-by: Simon Ser <[email protected]>
Reported-by: Roman Gilg <[email protected]>
References: emersion/libliftoff#46
Cc: Alex Deucher <[email protected]>
Cc: Harry Wentland <[email protected]>
Reviewed-by: Nicholas Kazlauskas <[email protected]>
Signed-off-by: Alex Deucher <[email protected]>
Cc: [email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
Signed-off-by: Kamal Mostafa <[email protected]>
Signed-off-by: Khalid Elmously <[email protected]>
kentrussell pushed a commit to ROCm/ROCK-Kernel-Driver that referenced this issue Jul 8, 2020
@emersion @yifancom
drm/amd/display: add basic atomic check for cursor plane
8fc962d 
This patch adds a basic cursor check when an atomic test-only commit is
performed. The position and size of the cursor plane is checked.

This should fix user-space relying on atomic checks to assign buffers to
planes.

Signed-off-by: Simon Ser <[email protected]>
Reported-by: Roman Gilg <[email protected]>
References: emersion/libliftoff#46
Cc: Alex Deucher <[email protected]>
Cc: Harry Wentland <[email protected]>
Reviewed-by: Nicholas Kazlauskas <[email protected]>
mseaster-wr pushed a commit to WindRiver-Labs/linux-yocto-5.2 that referenced this issue Jul 15, 2020
@emersion
drm/amd/display: add basic atomic check for cursor plane
c26c852 
commit 626bf90fe03fa080d8df06bb0397c95c53ae8e27 upstream.

This patch adds a basic cursor check when an atomic test-only commit is
performed. The position and size of the cursor plane is checked.

This should fix user-space relying on atomic checks to assign buffers to
planes.

Signed-off-by: Simon Ser <[email protected]>
Reported-by: Roman Gilg <[email protected]>
References: emersion/libliftoff#46
Cc: Alex Deucher <[email protected]>
Cc: Harry Wentland <[email protected]>
Reviewed-by: Nicholas Kazlauskas <[email protected]>
Signed-off-by: Alex Deucher <[email protected]>
Cc: [email protected]
Signed-off-by: Paul Gortmaker <[email protected]>
maurossi pushed a commit to maurossi/linux that referenced this issue Jul 16, 2020
@emersion
drm/amd/display: add basic atomic check for cursor plane
75212fd 
This patch adds a basic cursor check when an atomic test-only commit is
performed. The position and size of the cursor plane is checked.

This should fix user-space relying on atomic checks to assign buffers to
planes.

Signed-off-by: Simon Ser <[email protected]>
Reported-by: Roman Gilg <[email protected]>
References: emersion/libliftoff#46
Cc: Alex Deucher <[email protected]>
Cc: Harry Wentland <[email protected]>
Reviewed-by: Nicholas Kazlauskas <[email protected]>
evadot pushed a commit to freebsd/drm-kmod that referenced this issue Aug 18, 2020
@emersion @evadot
drm/amd/display: add basic atomic check for cursor plane
a1b32fd 
commit 626bf90fe03fa080d8df06bb0397c95c53ae8e27 upstream.

This patch adds a basic cursor check when an atomic test-only commit is
performed. The position and size of the cursor plane is checked.

This should fix user-space relying on atomic checks to assign buffers to
planes.

Signed-off-by: Simon Ser <[email protected]>
Reported-by: Roman Gilg <[email protected]>
References: emersion/libliftoff#46
Cc: Alex Deucher <[email protected]>
Cc: Harry Wentland <[email protected]>
Reviewed-by: Nicholas Kazlauskas <[email protected]>
Signed-off-by: Alex Deucher <[email protected]>
Cc: [email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
@emersion emersion closed this as completed Dec 4, 2020
wulf7 pushed a commit to wulf7/drm-kmod that referenced this issue Sep 29, 2021
@emersion @wulf7
drm/amd/display: add basic atomic check for cursor plane
ef47aea 
commit 626bf90fe03fa080d8df06bb0397c95c53ae8e27 upstream.

This patch adds a basic cursor check when an atomic test-only commit is
performed. The position and size of the cursor plane is checked.

This should fix user-space relying on atomic checks to assign buffers to
planes.

Signed-off-by: Simon Ser <[email protected]>
Reported-by: Roman Gilg <[email protected]>
References: emersion/libliftoff#46
Cc: Alex Deucher <[email protected]>
Cc: Harry Wentland <[email protected]>
Reviewed-by: Nicholas Kazlauskas <[email protected]>
Signed-off-by: Alex Deucher <[email protected]>
Cc: [email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
wulf7 pushed a commit to wulf7/drm-kmod that referenced this issue Oct 1, 2021
@emersion @wulf7
drm/amd/display: add basic atomic check for cursor plane
4c71fcf 
commit 626bf90fe03fa080d8df06bb0397c95c53ae8e27 upstream.

This patch adds a basic cursor check when an atomic test-only commit is
performed. The position and size of the cursor plane is checked.

This should fix user-space relying on atomic checks to assign buffers to
planes.

Signed-off-by: Simon Ser <[email protected]>
Reported-by: Roman Gilg <[email protected]>
References: emersion/libliftoff#46
Cc: Alex Deucher <[email protected]>
Cc: Harry Wentland <[email protected]>
Reviewed-by: Nicholas Kazlauskas <[email protected]>
Signed-off-by: Alex Deucher <[email protected]>
Cc: [email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
wulf7 pushed a commit to wulf7/drm-kmod that referenced this issue Oct 4, 2021
@emersion @wulf7
drm/amd/display: add basic atomic check for cursor plane
eab4c66 
commit 626bf90fe03fa080d8df06bb0397c95c53ae8e27 upstream.

This patch adds a basic cursor check when an atomic test-only commit is
performed. The position and size of the cursor plane is checked.

This should fix user-space relying on atomic checks to assign buffers to
planes.

Signed-off-by: Simon Ser <[email protected]>
Reported-by: Roman Gilg <[email protected]>
References: emersion/libliftoff#46
Cc: Alex Deucher <[email protected]>
Cc: Harry Wentland <[email protected]>
Reviewed-by: Nicholas Kazlauskas <[email protected]>
Signed-off-by: Alex Deucher <[email protected]>
Cc: [email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
wulf7 pushed a commit to wulf7/drm-kmod that referenced this issue Oct 9, 2021
@emersion @wulf7
drm/amd/display: add basic atomic check for cursor plane
078bbf3 
commit 626bf90fe03fa080d8df06bb0397c95c53ae8e27 upstream.

This patch adds a basic cursor check when an atomic test-only commit is
performed. The position and size of the cursor plane is checked.

This should fix user-space relying on atomic checks to assign buffers to
planes.

Signed-off-by: Simon Ser <[email protected]>
Reported-by: Roman Gilg <[email protected]>
References: emersion/libliftoff#46
Cc: Alex Deucher <[email protected]>
Cc: Harry Wentland <[email protected]>
Reviewed-by: Nicholas Kazlauskas <[email protected]>
Signed-off-by: Alex Deucher <[email protected]>
Cc: [email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
wulf7 pushed a commit to wulf7/drm-kmod that referenced this issue Oct 10, 2021
@emersion @wulf7
drm/amd/display: add basic atomic check for cursor plane
2fe28e0 
commit 626bf90fe03fa080d8df06bb0397c95c53ae8e27 upstream.

This patch adds a basic cursor check when an atomic test-only commit is
performed. The position and size of the cursor plane is checked.

This should fix user-space relying on atomic checks to assign buffers to
planes.

Signed-off-by: Simon Ser <[email protected]>
Reported-by: Roman Gilg <[email protected]>
References: emersion/libliftoff#46
Cc: Alex Deucher <[email protected]>
Cc: Harry Wentland <[email protected]>
Reviewed-by: Nicholas Kazlauskas <[email protected]>
Signed-off-by: Alex Deucher <[email protected]>
Cc: [email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
wulf7 pushed a commit to freebsd/drm-kmod that referenced this issue Oct 11, 2021
@emersion @wulf7
drm/amd/display: add basic atomic check for cursor plane
c7ec1a7 
commit 626bf90fe03fa080d8df06bb0397c95c53ae8e27 upstream.

This patch adds a basic cursor check when an atomic test-only commit is
performed. The position and size of the cursor plane is checked.

This should fix user-space relying on atomic checks to assign buffers to
planes.

Signed-off-by: Simon Ser <[email protected]>
Reported-by: Roman Gilg <[email protected]>
References: emersion/libliftoff#46
Cc: Alex Deucher <[email protected]>
Cc: Harry Wentland <[email protected]>
Reviewed-by: Nicholas Kazlauskas <[email protected]>
Signed-off-by: Alex Deucher <[email protected]>
Cc: [email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
neelchauhan pushed a commit to neelchauhan/drm-kmod that referenced this issue Oct 25, 2021
@emersion @neelchauhan
drm/amd/display: add basic atomic check for cursor plane
287b857 
This patch adds a basic cursor check when an atomic test-only commit is
performed. The position and size of the cursor plane is checked.

This should fix user-space relying on atomic checks to assign buffers to
planes.

Signed-off-by: Simon Ser <[email protected]>
Reported-by: Roman Gilg <[email protected]>
References: emersion/libliftoff#46
Cc: Alex Deucher <[email protected]>
Cc: Harry Wentland <[email protected]>
Reviewed-by: Nicholas Kazlauskas <[email protected]>
Signed-off-by: Alex Deucher <[email protected]>
Cc: [email protected]
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@emersion