| Message ID | 20201116200744.495826-1-maz@kernel.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | drm/meson: Module removal fixes | expand |
Hi Marc, On 16/11/2020 21:07, Marc Zyngier wrote: > Hi all, > > Having recently moved over to a top-of-the-tree u-boot on one of my > VIM3L systems in order to benefit from unrelated improvements > (automatic PCIe detection, EFI...), I faced the issue that my kernel > would hang like this: > > [ OK ] Finished Helper to synchronize boot up for ifupdown. > [ OK ] Started Rule-based Manager for Device Events and Files. > [ 7.114516] VDDCPU: supplied by regulator-dummy > [ OK ] Found device /dev/ttyAML0. > [ 7.146862] meson-drm ff900000.vpu: Queued 2 outputs on vpu > [ 7.169630] fb0: switching to meson-drm-fb from simple > [ 7.169944] Console: switching to colour dummy device 80x25 > [ 7.179250] meson-drm ff900000.vpu: CVBS Output connector not available > > and that's it. > > After some poking around, I figured out that it is in the > meson-dw-hdmi module that the CPU was hanging... I'll be interested in having your kernel config, I never had such report since I enabled HDMI support in U-Boot a few years ago. > > Reverting to the kernel DT instead of u-boot's papered over it > somehow, but it turned out that removing the module (modprobe -r) > resulted in a firework. And for every issue I was fixing, another > followed. Much fun for a rainy Monday in the basement! > > I ended up with the following 4 patches, which solve all my problems: > I can now boot with the u-boot provided DT, and the hdmi and DRM > drivers can be removed and re-inserted at will. > > The first patch is a straightforward use-after-free, causing a NULL > pointer dereference. Moving things around fixes it. > > The second patch shows that I have no clue about the DRM subsystem > whatsoever. I mimicked what my Rockchip systems are doing, and the two > warnings disappeared. It can't completely be wrong (famous last > words...). > > The third patch fixes a *very* common issue with regulators (I've > fixed at least 3 drivers with a similar issue). I guess the devm > subsystem needs to grow a new helper at some point. > > The last patch finally fixes the issue I was seeing: the HDMI driver > hangs when accessing a register with clocks disabled, which they are > on module removal. It also fixes my u-boot booting for similar > reasons, I guess. Anyway, thanks for fixing this ! > > I went as far as reaching out for a HDMI cable and verifying that I > was getting a working display. Total dedication. This is very appreciated :-) > > Feedback much appreciated. > > M. > > Marc Zyngier (4): > drm/meson: Free RDMA resources after tearing down DRM > drm/meson: Unbind all connectors on module removal > drm/meson: dw-hdmi: Register a callback to disable the regulator > drm/meson: dw-hdmi: Ensure that clocks are enabled before touching the > TOP registers > > drivers/gpu/drm/meson/meson_drv.c | 12 +++++++----- > drivers/gpu/drm/meson/meson_dw_hdmi.c | 13 +++++++++++-- > 2 files changed, 18 insertions(+), 7 deletions(-) >
Hi Neil, On 2020-11-17 08:49, Neil Armstrong wrote: > Hi Marc, > > On 16/11/2020 21:07, Marc Zyngier wrote: >> Hi all, >> >> Having recently moved over to a top-of-the-tree u-boot on one of my >> VIM3L systems in order to benefit from unrelated improvements >> (automatic PCIe detection, EFI...), I faced the issue that my kernel >> would hang like this: >> >> [ OK ] Finished Helper to synchronize boot up for ifupdown. >> [ OK ] Started Rule-based Manager for Device Events and Files. >> [ 7.114516] VDDCPU: supplied by regulator-dummy >> [ OK ] Found device /dev/ttyAML0. >> [ 7.146862] meson-drm ff900000.vpu: Queued 2 outputs on vpu >> [ 7.169630] fb0: switching to meson-drm-fb from simple >> [ 7.169944] Console: switching to colour dummy device 80x25 >> [ 7.179250] meson-drm ff900000.vpu: CVBS Output connector not >> available >> >> and that's it. >> >> After some poking around, I figured out that it is in the >> meson-dw-hdmi module that the CPU was hanging... > > I'll be interested in having your kernel config, I never had such > report > since I enabled HDMI support in U-Boot a few years ago. Yeah, I was pretty surprised too. I have a hunch that this is caused by u-boot DT exposing an extra MMIO region (dubbed "hhi") that gets picked up by the kernel driver. *Not* having the region in the DT (as in the kernel's version of the same DT) makes the driver work exactly once: Decompiled u-boot DT: hdmi-tx@0 { compatible = "amlogic,meson-g12a-dw-hdmi"; reg = <0x00 0x00 0x00 0x10000 0x00 0x3c000 0x00 0x1000>; [...] reg-names = "hdmitx\0hhi"; Decompiled kernel DT: hdmi-tx@0 { compatible = "amlogic,meson-g12a-dw-hdmi"; reg = <0x00 0x00 0x00 0x10000>; There seem to be some complex interactions between the HDMI driver and the DRM driver, both using this MMIO region at any given time. But I admit not having tried very hard to follow the DRM maze of intricate callbacks. All I needed was this box to reliably boot with the firmware-provided DT. You can find a reasonably recent version of my config at [1]. M. [1] http://www.loen.fr/tmp/Config.full-arm64
On 17/11/2020 10:19, Marc Zyngier wrote: > Hi Neil, > > On 2020-11-17 08:49, Neil Armstrong wrote: >> Hi Marc, >> >> On 16/11/2020 21:07, Marc Zyngier wrote: >>> Hi all, >>> >>> Having recently moved over to a top-of-the-tree u-boot on one of my >>> VIM3L systems in order to benefit from unrelated improvements >>> (automatic PCIe detection, EFI...), I faced the issue that my kernel >>> would hang like this: >>> >>> [ OK ] Finished Helper to synchronize boot up for ifupdown. >>> [ OK ] Started Rule-based Manager for Device Events and Files. >>> [ 7.114516] VDDCPU: supplied by regulator-dummy >>> [ OK ] Found device /dev/ttyAML0. >>> [ 7.146862] meson-drm ff900000.vpu: Queued 2 outputs on vpu >>> [ 7.169630] fb0: switching to meson-drm-fb from simple >>> [ 7.169944] Console: switching to colour dummy device 80x25 >>> [ 7.179250] meson-drm ff900000.vpu: CVBS Output connector not available >>> >>> and that's it. >>> >>> After some poking around, I figured out that it is in the >>> meson-dw-hdmi module that the CPU was hanging... >> >> I'll be interested in having your kernel config, I never had such report >> since I enabled HDMI support in U-Boot a few years ago. > > Yeah, I was pretty surprised too. I have a hunch that this is caused > by u-boot DT exposing an extra MMIO region (dubbed "hhi") that gets > picked up by the kernel driver. *Not* having the region in the DT > (as in the kernel's version of the same DT) makes the driver work > exactly once: Yeah, we used this to simplify our u-boot driver, the bindings were missing this memory space, it should be fixed at some point and stop relying on the main drm driver to get this memory space. > > Decompiled u-boot DT: > > hdmi-tx@0 { > compatible = "amlogic,meson-g12a-dw-hdmi"; > reg = <0x00 0x00 0x00 0x10000 0x00 0x3c000 0x00 0x1000>; > [...] > reg-names = "hdmitx\0hhi"; > > Decompiled kernel DT: > > hdmi-tx@0 { > compatible = "amlogic,meson-g12a-dw-hdmi"; > reg = <0x00 0x00 0x00 0x10000>; > > There seem to be some complex interactions between the HDMI driver > and the DRM driver, both using this MMIO region at any given time. > But I admit not having tried very hard to follow the DRM maze of > intricate callbacks. All I needed was this box to reliably boot with > the firmware-provided DT. Yes, the HDMI stuff has some dependencies on the DRM display subsystem. I plan to reorganize stuff but I lack time... Anyway, thanks. Applying to drm-misc-next Neil > > You can find a reasonably recent version of my config at [1]. > > M. > > [1] http://www.loen.fr/tmp/Config.full-arm64
Hi all, Having recently moved over to a top-of-the-tree u-boot on one of my VIM3L systems in order to benefit from unrelated improvements (automatic PCIe detection, EFI...), I faced the issue that my kernel would hang like this: [ OK ] Finished Helper to synchronize boot up for ifupdown. [ OK ] Started Rule-based Manager for Device Events and Files. [ 7.114516] VDDCPU: supplied by regulator-dummy [ OK ] Found device /dev/ttyAML0. [ 7.146862] meson-drm ff900000.vpu: Queued 2 outputs on vpu [ 7.169630] fb0: switching to meson-drm-fb from simple [ 7.169944] Console: switching to colour dummy device 80x25 [ 7.179250] meson-drm ff900000.vpu: CVBS Output connector not available and that's it. After some poking around, I figured out that it is in the meson-dw-hdmi module that the CPU was hanging... Reverting to the kernel DT instead of u-boot's papered over it somehow, but it turned out that removing the module (modprobe -r) resulted in a firework. And for every issue I was fixing, another followed. Much fun for a rainy Monday in the basement! I ended up with the following 4 patches, which solve all my problems: I can now boot with the u-boot provided DT, and the hdmi and DRM drivers can be removed and re-inserted at will. The first patch is a straightforward use-after-free, causing a NULL pointer dereference. Moving things around fixes it. The second patch shows that I have no clue about the DRM subsystem whatsoever. I mimicked what my Rockchip systems are doing, and the two warnings disappeared. It can't completely be wrong (famous last words...). The third patch fixes a *very* common issue with regulators (I've fixed at least 3 drivers with a similar issue). I guess the devm subsystem needs to grow a new helper at some point. The last patch finally fixes the issue I was seeing: the HDMI driver hangs when accessing a register with clocks disabled, which they are on module removal. It also fixes my u-boot booting for similar reasons, I guess. I went as far as reaching out for a HDMI cable and verifying that I was getting a working display. Total dedication. Feedback much appreciated. M. Marc Zyngier (4): drm/meson: Free RDMA resources after tearing down DRM drm/meson: Unbind all connectors on module removal drm/meson: dw-hdmi: Register a callback to disable the regulator drm/meson: dw-hdmi: Ensure that clocks are enabled before touching the TOP registers drivers/gpu/drm/meson/meson_drv.c | 12 +++++++----- drivers/gpu/drm/meson/meson_dw_hdmi.c | 13 +++++++++++-- 2 files changed, 18 insertions(+), 7 deletions(-)