firmware icon indicating copy to clipboard operation
firmware copied to clipboard

FKMS: vu order field doesn't have sense for NV21 and YV16 pixel formats

Open wosk opened this issue 2 years ago • 5 comments

Describe the bug Fied (struct set_plane).is_vu doesn't matter for DRM_FORMAT_YVU422/VC_IMAGE_YUV422PLANAR and DRM_FORMAT_NV21/VC_IMAGE_YUV420SP. DRM_FORMAT_YVU420 works ok

To reproduce tested on yocto core-image-sato in fkms mode (set dtoverlay=vc4-fkms-v3d in /boot/config.txt).

kill 'GUI window manager' modetest -p # retrieve CRTC id 87 and plane id 31 modetest -P 31@87:800x480@YV12 # ok modetest -P 31@87:800x480@NV21 # bad colors

Expected behaviour modetest -P 31@87:800x480@NV21 should display on the screen the same colors as YV12,YU12,NV12,etc The same issue is observed for YV16, but it can be tested via modetest

Actual behaviour Incorrect colors due to incorrect processing of Cb and Cr planes. Look info about order of planes here

System

  • Which model of Raspberry Pi? e.g. Pi3B+, PiZeroW Pi 4B 8Gb + DSI LCD 7"
  • Which OS and version (cat /etc/rpi-issue)? Yocto core-image-sato
  • Which firmware version (vcgencmd version)? The latest from https://github.com/raspberrypi/firmware/tree/master/boot at the moment
  • Which kernel version (uname -a)?

Linux raspberrypi4-64 5.15.34-v8 #1 SMP PREEMPT Tue Apr 19 19:21:26 UTC 2022 aarch64 GNU/Linux

Logs root@raspberrypi4-64:~# modetest -P 31@87:800x480@NV21 trying to open device 'vc4'...done testing 800x480@NV21 overlay plane 31

modetest_p_fkms.log modetest_c_fkms.log modetest_e_fkms.log

wosk avatar Dec 21 '22 11:12 wosk

Quoting Microsoft documentation for configuring planes is irrelevant when it comes to Linux. DRM formats largely have to be decoded from https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/drm_fourcc.c#L147

The firmware is incorrectly ignoring the is_vu flag for NV21. It gets set and then cleared again due to incorrect bitmasking. YV12 was swapping the U & V pointers, and that was working fine.

DRM_FORMAT_YVU422 isn't advertised as supported by FKMS, only DRM_FORMAT_YUV422. It needs the YV12 conditional extending to cover it within the firmware. Seeing as I'm fixing the NV21 conditional, I'll do that too. It needs the diff to vc4_firmware_kms.c of

diff --git a/drivers/gpu/drm/vc4/vc4_firmware_kms.c b/drivers/gpu/drm/vc4/vc4_firmware_kms.c
index 84414a953ee5..d029d485014a 100644
--- a/drivers/gpu/drm/vc4/vc4_firmware_kms.c
+++ b/drivers/gpu/drm/vc4/vc4_firmware_kms.c
@@ -216,6 +216,11 @@ static const struct vc_image_format {
                .drm = DRM_FORMAT_YUV422,
                .vc_image = VC_IMAGE_YUV422PLANAR,
        },
+       {
+               .drm = DRM_FORMAT_YVU422,
+               .vc_image = VC_IMAGE_YUV422PLANAR,
+               .is_vu = 1,
+       },
        {
                .drm = DRM_FORMAT_YUV420,
                .vc_image = VC_IMAGE_YUV420,

I'm not proposing on merging that, but feel free to create a PR if you feel you need it.

6by9 avatar Dec 21 '22 16:12 6by9

Whilst I have created the merge request internally, please be aware that the fix may be delayed due to not wanting to make big changes just before the Christmas break.

6by9 avatar Dec 21 '22 16:12 6by9

rpi-update firmware contains support for this

popcornmix avatar Jan 18 '23 12:01 popcornmix

NV21 now fixed. The issue with vu order for DRM_FORMAT_YVU422/VC_IMAGE_YUV422PLANAR still there.

wosk avatar Jan 23 '23 11:01 wosk

DRM_FORMAT_YVU422 is not listed as supported by the driver, therefore there is minimal expectation for it to work.

FKMS is also deprecated in favour of the main KMS driver, so largely it will only receive bug fixes rather than updates. It may be fixable, but there are many other higher priorities.

6by9 avatar Jan 23 '23 17:01 6by9