Re: [PATCH 23/33] drm/vc4: hdmi: Move HDMI reset to pm_resume
On Wed, Aug 10, 2022 at 10:33:48PM +0200, Stefan Wahren wrote: > Hi Florian, > > Am 09.08.22 um 21:02 schrieb Florian Fainelli: > > On 8/4/22 16:11, Florian Fainelli wrote: > > > On 6/13/22 07:47, Maxime Ripard wrote: > > > > From: Dave Stevenson > > > > > > > > The BCM2835-37 found in the RaspberryPi 0 to 3 have a power domain > > > > attached to the HDMI block, handled in Linux through runtime_pm. > > > > > > > > That power domain is shared with the VEC block, so even if we put our > > > > runtime_pm reference in the HDMI driver it would keep being on. If the > > > > VEC is disabled though, the power domain would be disabled and we would > > > > lose any initialization done in our bind implementation. > > > > > > > > That initialization involves calling the reset function and > > > > initializing > > > > the CEC registers. > > > > > > > > Let's move the initialization to our runtime_resume implementation so > > > > that we initialize everything properly if we ever need to. > > > > > > > > Fixes: c86b41214362 ("drm/vc4: hdmi: Move the HSM clock enable > > > > to runtime_pm") > > > > Signed-off-by: Dave Stevenson > > > > Signed-off-by: Maxime Ripard > > > > > > After seeing the same warning as Stefan reported in the link below, > > > but on the Raspberry Pi 4B: > > > > > > https://www.spinics.net/lists/dri-devel/msg354170.html > > > > > > a separate bisection effort led me to this commit, before is fine, > > > after produces 4 warnings during boot, see attached log. > > > > > > Is there a fix that we can try that would also cover the Raspberry > > > Pi 4B? Is it possible that this series precipitates the problem: > > > > > > https://www.spinics.net/lists/arm-kernel/msg984638.html > > > > Maxime, Dave, anything you would want me to try? Still seeing these > > warnings with net-next-6.0-11220-g15205c2829ca > > At first this issue doesn't occur in Linux 5.19. So it's something new. I > was able to reproduce with todays linux-next, but interestingly it doesn't > occur in drm-misc-next. Yeah, it should be fixed by https://lore.kernel.org/all/20220629123510.1915022-38-max...@cerno.tech/ https://lore.kernel.org/all/20220629123510.1915022-39-max...@cerno.tech/ Both patches apparently didn't make the cut for the merge window, if it works for you we can always queue them in drm-misc-fixes Maxime signature.asc Description: PGP signature
Re: [PATCH 23/33] drm/vc4: hdmi: Move HDMI reset to pm_resume
On Mon, Aug 15, 2022 at 09:52:59AM -0700, Florian Fainelli wrote: > On 8/15/22 07:12, Maxime Ripard wrote: > > On Wed, Aug 10, 2022 at 10:33:48PM +0200, Stefan Wahren wrote: > > > Hi Florian, > > > > > > Am 09.08.22 um 21:02 schrieb Florian Fainelli: > > > > On 8/4/22 16:11, Florian Fainelli wrote: > > > > > On 6/13/22 07:47, Maxime Ripard wrote: > > > > > > From: Dave Stevenson > > > > > > > > > > > > The BCM2835-37 found in the RaspberryPi 0 to 3 have a power domain > > > > > > attached to the HDMI block, handled in Linux through runtime_pm. > > > > > > > > > > > > That power domain is shared with the VEC block, so even if we put > > > > > > our > > > > > > runtime_pm reference in the HDMI driver it would keep being on. If > > > > > > the > > > > > > VEC is disabled though, the power domain would be disabled and we > > > > > > would > > > > > > lose any initialization done in our bind implementation. > > > > > > > > > > > > That initialization involves calling the reset function and > > > > > > initializing > > > > > > the CEC registers. > > > > > > > > > > > > Let's move the initialization to our runtime_resume implementation > > > > > > so > > > > > > that we initialize everything properly if we ever need to. > > > > > > > > > > > > Fixes: c86b41214362 ("drm/vc4: hdmi: Move the HSM clock enable > > > > > > to runtime_pm") > > > > > > Signed-off-by: Dave Stevenson > > > > > > Signed-off-by: Maxime Ripard > > > > > > > > > > After seeing the same warning as Stefan reported in the link below, > > > > > but on the Raspberry Pi 4B: > > > > > > > > > > https://www.spinics.net/lists/dri-devel/msg354170.html > > > > > > > > > > a separate bisection effort led me to this commit, before is fine, > > > > > after produces 4 warnings during boot, see attached log. > > > > > > > > > > Is there a fix that we can try that would also cover the Raspberry > > > > > Pi 4B? Is it possible that this series precipitates the problem: > > > > > > > > > > https://www.spinics.net/lists/arm-kernel/msg984638.html > > > > > > > > Maxime, Dave, anything you would want me to try? Still seeing these > > > > warnings with net-next-6.0-11220-g15205c2829ca > > > > > > At first this issue doesn't occur in Linux 5.19. So it's something new. I > > > was able to reproduce with todays linux-next, but interestingly it doesn't > > > occur in drm-misc-next. > > > > Yeah, it should be fixed by > > https://lore.kernel.org/all/20220629123510.1915022-38-max...@cerno.tech/ > > https://lore.kernel.org/all/20220629123510.1915022-39-max...@cerno.tech/ > > > > Both patches apparently didn't make the cut for the merge window, if it > > works for you we can always queue them in drm-misc-fixes > > Both of these patches eliminate the warning, I don't have a good set-up yet > for ensuring that HDMI/V3dD is functional however: > > Tested-by: Florian Fainelli I ended up applying it today (without your Tested-by, since it was partial though) Maxime signature.asc Description: PGP signature
Re: [PATCH 23/33] drm/vc4: hdmi: Move HDMI reset to pm_resume
On 8/15/22 07:12, Maxime Ripard wrote: On Wed, Aug 10, 2022 at 10:33:48PM +0200, Stefan Wahren wrote: Hi Florian, Am 09.08.22 um 21:02 schrieb Florian Fainelli: On 8/4/22 16:11, Florian Fainelli wrote: On 6/13/22 07:47, Maxime Ripard wrote: From: Dave Stevenson The BCM2835-37 found in the RaspberryPi 0 to 3 have a power domain attached to the HDMI block, handled in Linux through runtime_pm. That power domain is shared with the VEC block, so even if we put our runtime_pm reference in the HDMI driver it would keep being on. If the VEC is disabled though, the power domain would be disabled and we would lose any initialization done in our bind implementation. That initialization involves calling the reset function and initializing the CEC registers. Let's move the initialization to our runtime_resume implementation so that we initialize everything properly if we ever need to. Fixes: c86b41214362 ("drm/vc4: hdmi: Move the HSM clock enable to runtime_pm") Signed-off-by: Dave Stevenson Signed-off-by: Maxime Ripard After seeing the same warning as Stefan reported in the link below, but on the Raspberry Pi 4B: https://www.spinics.net/lists/dri-devel/msg354170.html a separate bisection effort led me to this commit, before is fine, after produces 4 warnings during boot, see attached log. Is there a fix that we can try that would also cover the Raspberry Pi 4B? Is it possible that this series precipitates the problem: https://www.spinics.net/lists/arm-kernel/msg984638.html Maxime, Dave, anything you would want me to try? Still seeing these warnings with net-next-6.0-11220-g15205c2829ca At first this issue doesn't occur in Linux 5.19. So it's something new. I was able to reproduce with todays linux-next, but interestingly it doesn't occur in drm-misc-next. Yeah, it should be fixed by https://lore.kernel.org/all/20220629123510.1915022-38-max...@cerno.tech/ https://lore.kernel.org/all/20220629123510.1915022-39-max...@cerno.tech/ Both patches apparently didn't make the cut for the merge window, if it works for you we can always queue them in drm-misc-fixes Both of these patches eliminate the warning, I don't have a good set-up yet for ensuring that HDMI/V3dD is functional however: Tested-by: Florian Fainelli -- Florian
Re: [PATCH 23/33] drm/vc4: hdmi: Move HDMI reset to pm_resume
Hi Florian, Am 09.08.22 um 21:02 schrieb Florian Fainelli: On 8/4/22 16:11, Florian Fainelli wrote: On 6/13/22 07:47, Maxime Ripard wrote: From: Dave Stevenson The BCM2835-37 found in the RaspberryPi 0 to 3 have a power domain attached to the HDMI block, handled in Linux through runtime_pm. That power domain is shared with the VEC block, so even if we put our runtime_pm reference in the HDMI driver it would keep being on. If the VEC is disabled though, the power domain would be disabled and we would lose any initialization done in our bind implementation. That initialization involves calling the reset function and initializing the CEC registers. Let's move the initialization to our runtime_resume implementation so that we initialize everything properly if we ever need to. Fixes: c86b41214362 ("drm/vc4: hdmi: Move the HSM clock enable to runtime_pm") Signed-off-by: Dave Stevenson Signed-off-by: Maxime Ripard After seeing the same warning as Stefan reported in the link below, but on the Raspberry Pi 4B: https://www.spinics.net/lists/dri-devel/msg354170.html a separate bisection effort led me to this commit, before is fine, after produces 4 warnings during boot, see attached log. Is there a fix that we can try that would also cover the Raspberry Pi 4B? Is it possible that this series precipitates the problem: https://www.spinics.net/lists/arm-kernel/msg984638.html Maxime, Dave, anything you would want me to try? Still seeing these warnings with net-next-6.0-11220-g15205c2829ca At first this issue doesn't occur in Linux 5.19. So it's something new. I was able to reproduce with todays linux-next, but interestingly it doesn't occur in drm-misc-next. Would be nice to see those fixes before 6.0 final, thanks!
Re: [PATCH 23/33] drm/vc4: hdmi: Move HDMI reset to pm_resume
Hi Florian On Tue, 9 Aug 2022 at 20:02, Florian Fainelli wrote: > > On 8/4/22 16:11, Florian Fainelli wrote: > > On 6/13/22 07:47, Maxime Ripard wrote: > >> From: Dave Stevenson > >> > >> The BCM2835-37 found in the RaspberryPi 0 to 3 have a power domain > >> attached to the HDMI block, handled in Linux through runtime_pm. > >> > >> That power domain is shared with the VEC block, so even if we put our > >> runtime_pm reference in the HDMI driver it would keep being on. If the > >> VEC is disabled though, the power domain would be disabled and we would > >> lose any initialization done in our bind implementation. > >> > >> That initialization involves calling the reset function and initializing > >> the CEC registers. > >> > >> Let's move the initialization to our runtime_resume implementation so > >> that we initialize everything properly if we ever need to. > >> > >> Fixes: c86b41214362 ("drm/vc4: hdmi: Move the HSM clock enable to > >> runtime_pm") > >> Signed-off-by: Dave Stevenson > >> Signed-off-by: Maxime Ripard > > > > After seeing the same warning as Stefan reported in the link below, but > > on the Raspberry Pi 4B: > > > > https://www.spinics.net/lists/dri-devel/msg354170.html > > > > a separate bisection effort led me to this commit, before is fine, after > > produces 4 warnings during boot, see attached log. > > > > Is there a fix that we can try that would also cover the Raspberry Pi > > 4B? Is it possible that this series precipitates the problem: > > > > https://www.spinics.net/lists/arm-kernel/msg984638.html > > Maxime, Dave, anything you would want me to try? Still seeing these > warnings with net-next-6.0-11220-g15205c2829ca Strange as we don't see this warning on the vendor kernel which is doing exactly the same. We are largely still on 5.15 as LTS though, so 5.19 hasn't had much bashing in that regard. Your callstack implies it's only sequencing. vc4_hdmi_bind is manually calling vc4_hdmi_runtime_resume (and hence initialising registers) before the call to pm_runtime_set_active and pm_runtime_enable, hence the pm accounting check in vc4_hdmi_write fails. pm_runtime always seems like black magic to me :-/ Do we need to manually power up here, or can we call pm_runtime_enable without touching the state, and then resume in the normal manner? ie something simple like pm_runtime_enable(dev); pm_runtime_resume_and_get(dev); The resume_and_get will call vc4_hdmi_runtime_resume and hence initialise the block, but it will have sorted the pm accounting first. Otherwise we mess with the order to be: pm_runtime_get_noresume(dev); pm_runtime_set_active(dev); ret = vc4_hdmi_runtime_resume(dev); if (ret) goto err_put_ddc; //This error handling needs to be checked pm_runtime_enable(dev); I have no feel for which is the "correct" approach in terms of pm_runtime, so will defer to others in that regard. Dave > Would be nice to see those fixes before 6.0 final, thanks! > -- > Florian
Re: [PATCH 23/33] drm/vc4: hdmi: Move HDMI reset to pm_resume
Hallo Stefan, On 8/9/22 13:16, Stefan Wahren wrote: Hi Florian, Am 05.08.22 um 01:11 schrieb Florian Fainelli: On 6/13/22 07:47, Maxime Ripard wrote: From: Dave Stevenson The BCM2835-37 found in the RaspberryPi 0 to 3 have a power domain attached to the HDMI block, handled in Linux through runtime_pm. That power domain is shared with the VEC block, so even if we put our runtime_pm reference in the HDMI driver it would keep being on. If the VEC is disabled though, the power domain would be disabled and we would lose any initialization done in our bind implementation. That initialization involves calling the reset function and initializing the CEC registers. Let's move the initialization to our runtime_resume implementation so that we initialize everything properly if we ever need to. Fixes: c86b41214362 ("drm/vc4: hdmi: Move the HSM clock enable to runtime_pm") Signed-off-by: Dave Stevenson Signed-off-by: Maxime Ripard After seeing the same warning as Stefan reported in the link below, but on the Raspberry Pi 4B: https://www.spinics.net/lists/dri-devel/msg354170.html a separate bisection effort led me to this commit, before is fine, after produces 4 warnings during boot, see attached log. Which config do you use (multi_v7_defconfig + LPAE or arm64/defconfig)? This was actually bcm2835_defconfig copied over to arch/arm64/configs/ and slightly modified to enable PCIe, here is it: https://gist.github.com/481999edc11b823d0c3e87ecf1693d26 Is there a fix that we can try that would also cover the Raspberry Pi 4B? Is it possible that this series precipitates the problem: https://www.spinics.net/lists/arm-kernel/msg984638.html I don't think this is related because this is a different driver. Best regards -- Florian
Re: [PATCH 23/33] drm/vc4: hdmi: Move HDMI reset to pm_resume
Hi Florian, Am 05.08.22 um 01:11 schrieb Florian Fainelli: On 6/13/22 07:47, Maxime Ripard wrote: From: Dave Stevenson The BCM2835-37 found in the RaspberryPi 0 to 3 have a power domain attached to the HDMI block, handled in Linux through runtime_pm. That power domain is shared with the VEC block, so even if we put our runtime_pm reference in the HDMI driver it would keep being on. If the VEC is disabled though, the power domain would be disabled and we would lose any initialization done in our bind implementation. That initialization involves calling the reset function and initializing the CEC registers. Let's move the initialization to our runtime_resume implementation so that we initialize everything properly if we ever need to. Fixes: c86b41214362 ("drm/vc4: hdmi: Move the HSM clock enable to runtime_pm") Signed-off-by: Dave Stevenson Signed-off-by: Maxime Ripard After seeing the same warning as Stefan reported in the link below, but on the Raspberry Pi 4B: https://www.spinics.net/lists/dri-devel/msg354170.html a separate bisection effort led me to this commit, before is fine, after produces 4 warnings during boot, see attached log. Which config do you use (multi_v7_defconfig + LPAE or arm64/defconfig)? Is there a fix that we can try that would also cover the Raspberry Pi 4B? Is it possible that this series precipitates the problem: https://www.spinics.net/lists/arm-kernel/msg984638.html I don't think this is related because this is a different driver. Best regards
Re: [PATCH 23/33] drm/vc4: hdmi: Move HDMI reset to pm_resume
On 8/4/22 16:11, Florian Fainelli wrote: On 6/13/22 07:47, Maxime Ripard wrote: From: Dave Stevenson The BCM2835-37 found in the RaspberryPi 0 to 3 have a power domain attached to the HDMI block, handled in Linux through runtime_pm. That power domain is shared with the VEC block, so even if we put our runtime_pm reference in the HDMI driver it would keep being on. If the VEC is disabled though, the power domain would be disabled and we would lose any initialization done in our bind implementation. That initialization involves calling the reset function and initializing the CEC registers. Let's move the initialization to our runtime_resume implementation so that we initialize everything properly if we ever need to. Fixes: c86b41214362 ("drm/vc4: hdmi: Move the HSM clock enable to runtime_pm") Signed-off-by: Dave Stevenson Signed-off-by: Maxime Ripard After seeing the same warning as Stefan reported in the link below, but on the Raspberry Pi 4B: https://www.spinics.net/lists/dri-devel/msg354170.html a separate bisection effort led me to this commit, before is fine, after produces 4 warnings during boot, see attached log. Is there a fix that we can try that would also cover the Raspberry Pi 4B? Is it possible that this series precipitates the problem: https://www.spinics.net/lists/arm-kernel/msg984638.html Maxime, Dave, anything you would want me to try? Still seeing these warnings with net-next-6.0-11220-g15205c2829ca Would be nice to see those fixes before 6.0 final, thanks! -- Florian
Re: [PATCH 23/33] drm/vc4: hdmi: Move HDMI reset to pm_resume
On 6/13/22 07:47, Maxime Ripard wrote: From: Dave Stevenson The BCM2835-37 found in the RaspberryPi 0 to 3 have a power domain attached to the HDMI block, handled in Linux through runtime_pm. That power domain is shared with the VEC block, so even if we put our runtime_pm reference in the HDMI driver it would keep being on. If the VEC is disabled though, the power domain would be disabled and we would lose any initialization done in our bind implementation. That initialization involves calling the reset function and initializing the CEC registers. Let's move the initialization to our runtime_resume implementation so that we initialize everything properly if we ever need to. Fixes: c86b41214362 ("drm/vc4: hdmi: Move the HSM clock enable to runtime_pm") Signed-off-by: Dave Stevenson Signed-off-by: Maxime Ripard After seeing the same warning as Stefan reported in the link below, but on the Raspberry Pi 4B: https://www.spinics.net/lists/dri-devel/msg354170.html a separate bisection effort led me to this commit, before is fine, after produces 4 warnings during boot, see attached log. Is there a fix that we can try that would also cover the Raspberry Pi 4B? Is it possible that this series precipitates the problem: https://www.spinics.net/lists/arm-kernel/msg984638.html -- FlorianStarting start4.elf @ 0xfec00200 partition -1 [0.00] Booting Linux on physical CPU 0x0 [0.00] Linux version 5.19.0-rc2 (florian@silverado) (arm-buildroot-linux-gnueabi-gcc.br_real (Buildroot 2022.05-448-g7ff22c698a0d) 11.3.0, GNU ld (GNU Binutils) 2.37) #73 SMP Thu Aug 4 16:09:03 PDT 2022 [0.00] CPU: ARMv7 Processor [410fd083] revision 3 (ARMv7), cr=30c5383d [0.00] CPU: div instructions available: patching division code [0.00] CPU: PIPT / VIPT nonaliasing data cache, PIPT instruction cache [0.00] OF: fdt: Machine model: Raspberry Pi 4 Model B Rev 1.1 [0.00] earlycon: bcm2835aux0 at MMIO32 0xfe215040 (options '115200n8') [0.00] printk: bootconsole [bcm2835aux0] enabled [0.00] Memory policy: Data cache writealloc [0.00] Reserved memory: created CMA memory pool at 0x3740, size 64 MiB [0.00] OF: reserved mem: initialized node linux,cma, compatible id shared-dma-pool [0.00] Zone ranges: [0.00] DMA [mem 0x-0x2fff] [0.00] Normal empty [0.00] HighMem [mem 0x3000-0xfbff] [0.00] Movable zone start for each node [0.00] Early memory node ranges [0.00] node 0: [mem 0x-0x3b3f] [0.00] node 0: [mem 0x4000-0xfbff] [0.00] Initmem setup node 0 [mem 0x-0xfbff] [0.00] [ cut here ] [0.00] WARNING: CPU: 0 PID: 0 at arch/arm/mm/physaddr.c:40 __virt_to_phys+0x84/0xbc [0.00] virt_to_phys used for non-linear address: (0x0) [0.00] Modules linked in: [0.00] CPU: 0 PID: 0 Comm: swapper Not tainted 5.19.0-rc2 #73 [0.00] Hardware name: BCM2711 [0.00] unwind_backtrace from show_stack+0x18/0x1c [0.00] show_stack from dump_stack_lvl+0x40/0x4c [0.00] dump_stack_lvl from __warn+0xb0/0x12c [0.00] __warn from warn_slowpath_fmt+0x80/0xc0 [0.00] warn_slowpath_fmt from __virt_to_phys+0x84/0xbc [0.00] __virt_to_phys from pcpu_embed_first_chunk+0x588/0x7cc [0.00] pcpu_embed_first_chunk from setup_per_cpu_areas+0x24/0xa0 [0.00] setup_per_cpu_areas from start_kernel+0x1a8/0x6b8 [0.00] start_kernel from 0x0 [0.00] ---[ end trace ]--- [0.00] percpu: Embedded 16 pages/cpu s35860 r8192 d21484 u65536 [0.00] Built 1 zonelists, mobility grouping on. Total pages: 1011200 [0.00] Kernel command line: dma.dmachans=0x71f5 bcm2709.boardrev=0xc03111 bcm2709.serial=0x4b11cb83 bcm2709.uart_clock=4800 bcm2709.disk_led_gpio=42 bcm270 9.disk_led_active_low=0 smsc95xx.macaddr=DC:A6:32:1C:A0:82 vc_mem.mem_base=0x3ec0 vc_mem.mem_size=0x4000 earlycon earlyprintk [0.00] Unknown kernel command line parameters "earlyprintk", will be passed to user space. [0.00] Dentry cache hash table entries: 131072 (order: 7, 524288 bytes, linear) [0.00] Inode-cache hash table entries: 65536 (order: 6, 262144 bytes, linear) [0.00] mem auto-init: stack:off, heap alloc:off, heap free:off [0.00] software IO TLB: mapped [mem 0x29079000-0x2d079000] (64MB) [0.00] Memory: 3851564K/4050944K available (10240K kernel code, 1703K rwdata, 3832K rodata, 16384K init, 478K bss, 133844K reserved, 65536K cma-reserved, 319897 6K highmem) [0.00] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=4, Nodes=1 [0.00] ftrace: allocating 38049 entries in 112 pages [0.00] ftrace: allocated