HDMI support for modes that require the scrambler (4k @ 60Hz, most importantly) #4302
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Maxime Ripard <[email protected]>
The current code does a binary OR on the possible_crtcs variable of the TXP encoder, while we want to set it to that value instead. Cc: <[email protected]> # v5.9+ Fixes: 39fcb28 ("drm/vc4: txp: Turn the TXP into a CRTC of its own") Acked-by: Thomas Zimmermann <[email protected]> Signed-off-by: Maxime Ripard <[email protected]>
The vc4_set_crtc_possible_masks is meant to run over all the encoders and then set their possible_crtcs mask to their associated pixelvalve. However, since the commit 39fcb28 ("drm/vc4: txp: Turn the TXP into a CRTC of its own"), the TXP has been turned to a CRTC and encoder of its own, and while it does indeed register an encoder, it no longer has an associated pixelvalve. The code will thus run over the TXP encoder and set a bogus possible_crtcs mask, overriding the one set in the TXP bind function. In order to fix this, let's skip any virtual encoder. Cc: <[email protected]> # v5.9+ Fixes: 39fcb28 ("drm/vc4: txp: Turn the TXP into a CRTC of its own") Acked-by: Thomas Zimmermann <[email protected]> Signed-off-by: Maxime Ripard <[email protected]>
Due to a FIFO that cannot be flushed between the pixelvalve and the HDMI controllers on BCM2711, we need to carefully disable both at boot time if they were left enabled by the firmware. However, at the time we're running that code, the struct drm_connector encoder pointer isn't set yet, and thus we cannot retrieve the encoder associated to our CRTC. We can however make use of the fact that we have a less flexible setup than what DRM allows where we have a 1:1 relationship between our CRTCs and encoders (and connectors), and thus store the crtc associated to our encoder at boot time. We cannot do that at the time the encoders are probed though, since the CRTCs won't be probed yet and thus we don't know at that time which CRTC index we're going to get, so let's do this in two passes: we can first bind all the components and then once they all are bound, we can iterate over all the encoders to find their associated CRTC and set the pointer. This is similar to what we're doing to set the possible_crtcs field. Fixes: 875a4d5 ("drm/vc4: drv: Disable the CRTC at boot time") Signed-off-by: Maxime Ripard <[email protected]>
Since we fixed the hooks to disable the encoder at boot, we now have an unbalanced clk_disable call at boot since we never enabled them in the first place. Let's mimic the state of the hardware and enable the clocks at boot if the controller is enabled to get the use-count right. Cc: <[email protected]> # v5.10+ Fixes: 09c4381 ("drm/vc4: hdmi: Implement finer-grained hooks") Signed-off-by: Maxime Ripard <[email protected]>
We'll need to have the HVS binding before the HDMI controllers so that we can check whether the firmware allows to run in 4kp60. Reorder a bit the component list, and document the current constraints we're aware of. Acked-by: Dave Stevenson <[email protected]> Acked-by: Thomas Zimmermann <[email protected]> Signed-off-by: Maxime Ripard <[email protected]>
The BVB clock rate computation doesn't take into account a mode clock of 594MHz that we're going to need to support 4k60. Acked-by: Thomas Zimmermann <[email protected]> Reviewed-by: Dave Stevenson <[email protected]> Signed-off-by: Maxime Ripard <[email protected]>
In order to reach the frequencies needed to output at 594MHz, the firmware needs to be configured with the appropriate parameters in the config.txt file (enable_hdmi_4kp60 and force_turbo). Let's detect it at bind time, warn the user if we can't, and filter out the relevant modes. Acked-by: Thomas Zimmermann <[email protected]> Reviewed-by: Dave Stevenson <[email protected]> Signed-off-by: Maxime Ripard <[email protected]>
The HDMI controller on the BCM2711 includes a scrambler in order to reach the HDMI 2.0 modes that require it. Let's add the support for it. Acked-by: Thomas Zimmermann <[email protected]> Signed-off-by: Maxime Ripard <[email protected]>
Now that we have the infrastructure in place, we can raise the maximum pixel rate we can reach for HDMI0 on the BCM2711. HDMI1 is left untouched since its pixelvalve has a smaller FIFO and would need a clock faster than what we can provide to support the same modes. Acked-by: Thomas Zimmermann <[email protected]> Reviewed-by: Dave Stevenson <[email protected]> Signed-off-by: Maxime Ripard <[email protected]>
|
I keep looking at the upstream patches and trying to get my head around them before sending responses. The possible_crtcs stuff does my head in, particularly how it interacts with the TXP. In response to your cover letter, the firmware fix for the HDMI I2C block is done and is in firmware releases since Mar 10, 2021. |
popcornmix
added a commit
to raspberrypi/firmware
that referenced
this pull request
Apr 21, 2021
See: raspberrypi/linux#4303 kernel: HDMI support for modes that require the scrambler (4k @ 60Hz, most importantly) See: raspberrypi/linux#4302 kernel: overlays: spi-rtc: Add ds3232 and ds3234 See: raspberrypi/linux#4292 firmware: isp: Ensure the VRF is locked when setting up video colour denoise See: raspberrypi/rpicam-apps#19 firmware: isp: Remove custom EV mappings from camera tunings firmware: Add support for board-type=0xXX conditional filters in bootloader, bootcode and firmware firmware: Two UART1 patches See: #1566
popcornmix
added a commit
to Hexxeh/rpi-firmware
that referenced
this pull request
Apr 21, 2021
See: raspberrypi/linux#4303 kernel: HDMI support for modes that require the scrambler (4k @ 60Hz, most importantly) See: raspberrypi/linux#4302 kernel: overlays: spi-rtc: Add ds3232 and ds3234 See: raspberrypi/linux#4292 firmware: isp: Ensure the VRF is locked when setting up video colour denoise See: raspberrypi/rpicam-apps#19 firmware: isp: Remove custom EV mappings from camera tunings firmware: Add support for board-type=0xXX conditional filters in bootloader, bootcode and firmware firmware: Two UART1 patches See: raspberrypi/firmware#1566
6by9
reviewed
Apr 21, 2021
| @@ -2498,6 +2498,9 @@ void clk_request_done(struct clk_request *req) | |||
| { | |||
| struct clk_core *core = req->clk->core; | |||
|
|
|||
| if (!req) | |||
| return; | |||
|
|
|||
There was a problem hiding this comment.
Minor point.
Line 2499 has already dereferenced req->clk->core to assign to core, so it's too late to be checking for NULL here.
There was a problem hiding this comment.
Interestingly it works, but you're totally right, I'll fix it upstream
There was a problem hiding this comment.
Compiler reordering might work in your favour here.
There was a problem hiding this comment.
I had a blow up on what I presume to be this, hence looking at the code
[ 5.794257] 8<--- cut here ---
[ 5.794310] Unable to handle kernel NULL pointer dereference at virtual address 00000008
[ 5.794362] pgd = (ptrval)
[ 5.794403] [00000008] *pgd=03a2e003, *pmd=00000000
[ 5.794470] Internal error: Oops: 206 [#1] SMP ARM
[ 5.794495] Modules linked in: bcm2835_codec(C) v4l2_mem2mem raspberrypi_hwmon bcm2835_v4l2(C) videobuf2_vmalloc bcm2835_isp(C) bcm2835_mmal_vchiq(C) videobuf2_dma_contig videobuf2_memops videobuf2_v4l2 videobuf2_common brcmfmac brcmutil i2c_mux_pinctrl i2c_mux sha256_generic cfg80211 rfkill v3d gpu_sched dwc2 i2c_brcmstb vc4(+) roles cec i2c_bcm2835 drm_kms_helper videodev drm mc drm_panel_orientation_quirks snd_bcm2835(C) snd_soc_core vc_sm_cma(C) snd_compress snd_pcm_dmaengine snd_pcm snd_timer snd rpivid_mem syscopyarea sysfillrect sysimgblt fb_sys_fops backlight uio_pdrv_genirq uio nvmem_rmem i2c_dev ip_tables x_tables ipv6
[ 5.795077] CPU: 2 PID: 166 Comm: systemd-udevd Tainted: G C 5.10.31-v7l+ #5
[ 5.795107] Hardware name: BCM2711
[ 5.795144] PC is at clk_request_done+0x14/0xac
[ 5.795278] LR is at vc4_hdmi_encoder_post_crtc_powerdown+0x17c/0x230 [vc4]
[ 5.795315] pc : [<c07f20e4>] lr : [<bf39536c>] psr: 60000113
[ 5.795340] sp : c3a4db18 ip : c3a4db30 fp : c3a4db2c
[ 5.795368] r10: 0000000a r9 : 00000014 r8 : 00000000
[ 5.795400] r7 : c271d290 r6 : c4164040 r5 : c3af6800 r4 : c271d290
[ 5.795431] r3 : 00000000 r2 : 00000000 r1 : 00000000 r0 : 00000000
[ 5.795467] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
[ 5.795498] Control: 30c5383d Table: 02fdf980 DAC: 55555555
[ 5.795527] Process systemd-udevd (pid: 166, stack limit = 0x(ptrval))
[ 5.795551] Stack: (0xc3a4db18 to 0xc3a4e000)
[ 5.795586] db00: c271d290 c3af6800
[ 5.795645] db20: c3a4db44 c3a4db30 bf39536c c07f20dc 00000000 c0ebcf84 c3a4db7c c3a4db48
[ 5.795701] db40: bf385ee4 bf3951fc 00000001 c271c800 c3a4db7c c4164040 c271c800 c4164040
[ 5.795750] db60: c271c9d0 00000001 c12f0a2c c37717c0 c3a4db94 c3a4db80 bf386c14 bf385d7c
[ 5.795798] db80: c271c800 c19bb810 c3a4dbbc c3a4db98 bf387574 bf386b94 bf387324 c3af2540
[ 5.795832] dba0: c2f07ac0 000000d8 00000009 c3af2800 c3a4dbf4 c3a4dbc0 c0850810 bf387330
[ 5.795883] dbc0: c12f0a10 00000000 c0b97714 c12f0a10 c3af2800 c19bb810 bf3ab95c bf3abee0
[ 5.795917] dbe0: bf3abee0 c1205048 c3a4dc1c c3a4dbf8 c0850d0c c08506a4 c0c4ce10 c19bb810
[ 5.795949] dc00: 00000000 00000000 bf3b87cc c19bb810 c3a4dc5c c3a4dc20 bf38768c c0850c38
[ 5.795980] dc20: c19b9810 c19bb810 c37717c0 98d4afc0 c086aa8c 00000000 c19bb810 bf3b8150
[ 5.796012] dc40: c1401108 bf3b8150 0000001f 00000000 c3a4dc7c c3a4dc60 c085a750 bf3875f0
[ 5.796057] dc60: c085a6d4 c1401100 c19bb810 00000000 c3a4dcac c3a4dc80 c08584ac c085a6e0
[ 5.796092] dc80: 00000000 c19bb810 bf3b8150 bf3b8150 c1205048 00000000 00000001 c3b00b08
[ 5.796124] dca0: c3a4dcc4 c3a4dcb0 c085896c c08583b4 00000000 c19bb810 c3a4dce4 c3a4dcc8
[ 5.796170] dcc0: c0858bbc c085890c c085ae34 00000000 bf3b8150 c19bb810 c3a4dd04 c3a4dce8
[ 5.796202] dce0: c0858c54 c0858b60 c19be640 00000000 bf3b8150 c0858bc4 c3a4dd34 c3a4dd08
[ 5.796238] dd00: c0856254 c0858bd0 c3a4dd40 c18eca58 c19be634 98d4afc0 bf3b8150 c12f0ff0
[ 5.796272] dd20: c3771700 00000000 c3a4dd44 c3a4dd38 c0857e0c c08561e8 c3a4dd6c c3a4dd48
[ 5.796305] dd40: c08576b8 c0857dec bf3b1864 c3a4dd58 bf3b8150 bf191000 c3a4c000 c1205048
[ 5.796337] dd60: c3a4dd84 c3a4dd70 c0859324 c0857584 bf3b8940 bf191000 c3a4dd94 c3a4dd88
[ 5.796392] dd80: c085a680 c08592a4 c3a4dda4 c3a4dd98 bf191040 c085a63c c3a4de1c c3a4dda8
[ 5.796433] dda0: c0202430 bf19100c 00000001 c0b97714 00000080 c1801e40 c3a4ddd4 c3a4ddc8
[ 5.796467] ddc0: c0b97714 c04243b8 c3a4de1c c3a4ddd8 c04243b8 c03e0f04 c3a4de14 c3a4dde8
[ 5.796499] dde0: 00001e29 00000008 c0b90d24 c3b00c00 c3b00b08 98d4afc0 bf3b8940 c3b00bc0
[ 5.796531] de00: 00000001 bf3b8940 c3b00b00 00000001 c3a4de44 c3a4de20 c0b90d60 c02023ec
[ 5.796563] de20: c3a4de44 c3a4de30 c0405918 c3a4df30 c1205048 00000001 c3a4df14 c3a4de48
[ 5.796594] de40: c02cfc1c c0b90cfc bf3b894c 00007fff bf3b8940 c02cc9d0 c3a4de84 bf1a0161
[ 5.796626] de60: bf3b8a54 bf3b894c bf3b8b4c 00000000 00000001 c1205048 00000000 00000000
[ 5.796657] de80: 00000000 00000000 00000000 00000000 c0e2aa58 00000001 00000000 c0eeb5d4
[ 5.796689] dea0: c0ed3d38 00000000 6e72656b 00006c65 00000000 00000000 00000000 00000000
[ 5.796720] dec0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 5.796753] dee0: 00000000 98d4afc0 c3a4df2c c1205048 00000000 b6dfe8e0 0000001d c0200204
[ 5.796784] df00: c3a4c000 0000017b c3a4dfa4 c3a4df18 c02d04c4 c02cdc50 c3a4df2c 7fffffff
[ 5.796817] df20: 00000000 00000002 8000cb00 f12b4000 f12e4c57 f12eb540 f12b4000 000547f4
[ 5.796867] df40: f1307fac f12ebd3d f12f4430 00035000 0003b860 0000eb04 0003fba3 00000000
[ 5.796915] df60: 00000000 00000000 0000eaf4 00000033 00000034 0000002a 00000018 00000011
[ 5.796971] df80: 00000000 98d4afc0 8000cb00 00000000 00000000 0000017b 00000000 c3a4dfa8
[ 5.797019] dfa0: c02001e4 c02d041c 8000cb00 00000000 0000001d b6dfe8e0 00000000 b6dff3f4
[ 5.797066] dfc0: 8000cb00 00000000 00000000 0000017b 022090f0 00000000 0220c5b8 00000000
[ 5.797117] dfe0: beb95180 beb95170 b6df59d8 b6ee5af0 60000010 0000001d 00000000 00000000
[ 5.797153] Backtrace:
[ 5.797286] [<c07f20d0>] (clk_request_done) from [<bf39536c>] (vc4_hdmi_encoder_post_crtc_powerdown+0x17c/0x230 [vc4])
[ 5.797322] r5:c3af6800 r4:c271d290
[ 5.797539] [<bf3951f0>] (vc4_hdmi_encoder_post_crtc_powerdown [vc4]) from [<bf385ee4>] (vc4_crtc_disable+0x174/0x1b8 [vc4])
[ 5.797577] r5:c0ebcf84 r4:00000000
[ 5.797780] [<bf385d70>] (vc4_crtc_disable [vc4]) from [<bf386c14>] (vc4_crtc_disable_at_boot+0x8c/0xb4 [vc4])
[ 5.797819] r10:c37717c0 r9:c12f0a2c r8:00000001 r7:c271c9d0 r6:c4164040 r5:c271c800
[ 5.797847] r4:c4164040
[ 5.798027] [<bf386b88>] (vc4_crtc_disable_at_boot [vc4]) from [<bf387574>] (vc4_drm_bind+0x250/0x2c0 [vc4])
[ 5.798060] r5:c19bb810 r4:c271c800
[ 5.798173] [<bf387324>] (vc4_drm_bind [vc4]) from [<c0850810>] (try_to_bring_up_master+0x178/0x1c8)
[ 5.798211] r8:c3af2800 r7:00000009 r6:000000d8 r5:c2f07ac0 r4:c3af2540 r3:bf387324
[ 5.798263] [<c0850698>] (try_to_bring_up_master) from [<c0850d0c>] (component_master_add_with_match+0xe0/0x114)
[ 5.798304] r10:c1205048 r9:bf3abee0 r8:bf3abee0 r7:bf3ab95c r6:c19bb810 r5:c3af2800
[ 5.798340] r4:c12f0a10
[ 5.798460] [<c0850c2c>] (component_master_add_with_match) from [<bf38768c>] (vc4_platform_drm_probe+0xa8/0xc8 [vc4])
[ 5.798507] r7:c19bb810 r6:bf3b87cc r5:00000000 r4:00000000
[ 5.798628] [<bf3875e4>] (vc4_platform_drm_probe [vc4]) from [<c085a750>] (platform_drv_probe+0x7c/0xb4)
[ 5.798664] r10:00000000 r9:0000001f r8:bf3b8150 r7:c1401108 r6:bf3b8150 r5:c19bb810
[ 5.798691] r4:00000000
[ 5.798716] [<c085a6d4>] (platform_drv_probe) from [<c08584ac>] (really_probe+0x104/0x3d4)
[ 5.798748] r6:00000000 r5:c19bb810 r4:c1401100 r3:c085a6d4
[ 5.798776] [<c08583a8>] (really_probe) from [<c085896c>] (driver_probe_device+0x6c/0xc4)
[ 5.798808] r10:c3b00b08 r9:00000001 r8:00000000 r7:c1205048 r6:bf3b8150 r5:bf3b8150
[ 5.798837] r4:c19bb810 r3:00000000
[ 5.798863] [<c0858900>] (driver_probe_device) from [<c0858bbc>] (device_driver_attach+0x68/0x70)
[ 5.798894] r5:c19bb810 r4:00000000
[ 5.798919] [<c0858b54>] (device_driver_attach) from [<c0858c54>] (__driver_attach+0x90/0xcc)
[ 5.798951] r6:c19bb810 r5:bf3b8150 r4:00000000 r3:c085ae34
[ 5.798983] [<c0858bc4>] (__driver_attach) from [<c0856254>] (bus_for_each_dev+0x78/0xc4)
[ 5.799014] r6:c0858bc4 r5:bf3b8150 r4:00000000 r3:c19be640
[ 5.799045] [<c08561dc>] (bus_for_each_dev) from [<c0857e0c>] (driver_attach+0x2c/0x30)
[ 5.799075] r7:00000000 r6:c3771700 r5:c12f0ff0 r4:bf3b8150
[ 5.799107] [<c0857de0>] (driver_attach) from [<c08576b8>] (bus_add_driver+0x140/0x1f8)
[ 5.799143] [<c0857578>] (bus_add_driver) from [<c0859324>] (driver_register+0x8c/0x124)
[ 5.799177] r7:c1205048 r6:c3a4c000 r5:bf191000 r4:bf3b8150
[ 5.799207] [<c0859298>] (driver_register) from [<c085a680>] (__platform_driver_register+0x50/0x58)
[ 5.799246] r5:bf191000 r4:bf3b8940
[ 5.799370] [<c085a630>] (__platform_driver_register) from [<bf191040>] (vc4_drm_register+0x40/0x1000 [vc4])
[ 5.799496] [<bf191000>] (vc4_drm_register [vc4]) from [<c0202430>] (do_one_initcall+0x50/0x254)
[ 5.799535] [<c02023e0>] (do_one_initcall) from [<c0b90d60>] (do_init_module+0x70/0x24c)
[ 5.799568] r9:00000001 r8:c3b00b00 r7:bf3b8940 r6:00000001 r5:c3b00bc0 r4:bf3b8940
[ 5.799604] [<c0b90cf0>] (do_init_module) from [<c02cfc1c>] (load_module+0x1fd8/0x2640)
[ 5.799634] r6:00000001 r5:c1205048 r4:c3a4df30
[ 5.799663] [<c02cdc44>] (load_module) from [<c02d04c4>] (sys_finit_module+0xb4/0xd8)
[ 5.799695] r10:0000017b r9:c3a4c000 r8:c0200204 r7:0000001d r6:b6dfe8e0 r5:00000000
[ 5.799723] r4:c1205048
[ 5.799756] [<c02d0410>] (sys_finit_module) from [<c02001e4>] (__sys_trace_return+0x0/0x1c)
[ 5.799798] Exception stack(0xc3a4dfa8 to 0xc3a4dff0)
[ 5.799833] dfa0: 8000cb00 00000000 0000001d b6dfe8e0 00000000 b6dff3f4
[ 5.799878] dfc0: 8000cb00 00000000 00000000 0000017b 022090f0 00000000 0220c5b8 00000000
[ 5.799920] dfe0: beb95180 beb95170 b6df59d8 b6ee5af0
[ 5.799961] r7:0000017b r6:00000000 r5:00000000 r4:8000cb00
[ 5.799990] Code: e92dd830 e24cb004 e52de004 e8bd4000 (e5903008)
[ 5.800050] ---[ end trace 50b9549224214350 ]---
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi,
I just realised I forgot to send a PR for this even though the code has been sent upstream for some time now.