Respect cursor cap

[subdiff]

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.

[emersion]

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]

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]

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]

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

[subdiff]

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

Yes, a nice big notification message. ;)

[emersion]

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

[emersion]

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

[subdiff]

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?

[emersion]

Ping'ed the AMD devs.

[emersion]

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

[subdiff]

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

[emersion]

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

[emersion]

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

[emersion]

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