vc4 drm planes should not define all CRTCs as possible

[schauveau]

In the current vc4 driver each CRTC is using 3 planes (one of each type) each created with possible_crtcs=0xff
That value comes from the 3rd argument of

linux/drivers/gpu/drm/vc4/vc4_firmware_kms.c

Line 759 in cc39f1c

ret = drm_universal_plane_init(dev, plane, 0xff,

This is incorrect for 2 reasons.

First, some unused bits are sets. This is unexpected and that could potentially confuse some client implementations that rely on the detection of the last possible crtcs.

Second, and this is the most important, having multiple bits means that the client can freely associate each plane to each CRTC. This is clearly not the intention here. For example, I used a modified version of sway/wlroots to assign the 1st crtc to the 2nd cursor plane and the 2nd crtc to the 1st cursor plane. This is supposed to be a PERFECTLY LEGAL interpretation of the DRM api. On my rpi4 with 2 monitors, the result was that my left screen was showing the right screen cursor (and the other way around).

So my advice is to change vc4_firmware_kms.c as follow:

1. Add an argument uint32_t possible_crtcs to vc4_fkms_plane_init.
2. Replace 0xff by possible_crtcs in the call of drm_universal_plane_init.
3. In each call of vc4_fkms_plane_init, use drm_crtc_mask(ctrc) for the possible_crtrs argument.

See also:
swaywm/wlroots#2333
swaywm/wlroots#1943

[6by9]

You're correct that it is wrong, but the suggested solution isn't going to work.

possible_crtcs should be set for the primary and cursor planes via drm_crtc_init_with_planes but the fact that they are set already skips that.

Plane creation order also makes life trickier.
The Z order is determined by creation order if not overridden by zpos property. We don't have the crtc index to populate into the plane until after drm_crtc_init_with_planes, which wants to take the planes properties. So you have to go back and update all the planes after the drm_crtc_init.

#3740 should hopefully fix the issue.

[6by9]

pi@raspberrypi:~ $ ./drm/build/tests/modetest/modetest -M vc4 | grep 0,0
87	157	(0,0)	(1920x1200)
151	157	(0,0)	(1280x800)
31	87	157	0,0		0,0	0       	0x00000001
38	0	0	0,0		0,0	0       	0x00000001
45	0	0	0,0		0,0	0       	0x00000001
52	0	0	0,0		0,0	0       	0x00000001
59	0	0	0,0		0,0	0       	0x00000001
66	0	0	0,0		0,0	0       	0x00000001
73	0	0	0,0		0,0	0       	0x00000001
80	0	0	0,0		0,0	0       	0x00000001
95	151	157	0,0		0,0	0       	0x00000002
102	0	0	0,0		0,0	0       	0x00000002
109	0	0	0,0		0,0	0       	0x00000002
116	0	0	0,0		0,0	0       	0x00000002
123	0	0	0,0		0,0	0       	0x00000002
130	0	0	0,0		0,0	0       	0x00000002
137	0	0	0,0		0,0	0       	0x00000002
144	0	0	0,0		0,0	0       	0x00000002

looks hopeful (I increased the number of overlay planes available to 6 per crtc whilst I was in there)

[schauveau]

If the last columns is possible_ctrcs then that looks good. Thanks.

[6by9]

If the last columns is possible_ctrcs then that looks good. Thanks.

It is.
Headings for the planes are

Planes:
id      crtc    fb      CRTC x,y        x,y     gamma size      possible crtcs

[6by9]

Patch merged ages ago - closing.