drm/meson: Module removal fixes

Removing the meson DRM module results in the following splat:

[ 2179.451346] Hardware name:  , BIOS 2021.01-rc2-00012-gde865f7ee1 11/16/2020
[ 2179.458316] Workqueue: events drm_mode_rmfb_work_fn [drm]
[ 2179.463597] pstate: 80c00009 (Nzcv daif +PAN +UAO -TCO BTYPE=--)
[ 2179.469558] pc : meson_rdma_writel_sync+0x44/0xb0 [meson_drm]
[ 2179.475243] lr : meson_g12a_afbcd_reset+0x34/0x60 [meson_drm]
[ 2179.480930] sp : ffffffc01212bb70
[ 2179.484207] x29: ffffffc01212bb70 x28: ffffff8044f66f00
[ 2179.489469] x27: ffffff8045b13800 x26: 0000000000000001
[ 2179.494730] x25: 0000000000000000 x24: 0000000000000001
[ 2179.499991] x23: 0000000000000000 x22: 0000000000000000
[ 2179.505252] x21: 0000000000280000 x20: 0000000000001a01
[ 2179.510513] x19: ffffff8046029480 x18: 0000000000000000
[ 2179.515775] x17: 0000000000000000 x16: 0000000000000000
[ 2179.521036] x15: 0000000000000000 x14: 0000000000000000
[ 2179.526297] x13: 0040000000000326 x12: 0309030303260300
[ 2179.531558] x11: 03000000054004a0 x10: 0418054004000400
[ 2179.536820] x9 : ffffffc008fe4914 x8 : ffffff8040a1adc0
[ 2179.542081] x7 : 0000000000000000 x6 : ffffff8042aa0080
[ 2179.547342] x5 : ffffff8044f66f00 x4 : ffffffc008fe5bc8
[ 2179.552603] x3 : 0000000000010101 x2 : 0000000000000001
[ 2179.557865] x1 : 0000000000000000 x0 : 0000000000000000
[ 2179.563127] Call trace:
[ 2179.565548]  meson_rdma_writel_sync+0x44/0xb0 [meson_drm]
[ 2179.570894]  meson_g12a_afbcd_reset+0x34/0x60 [meson_drm]
[ 2179.576241]  meson_plane_atomic_disable+0x38/0xb0 [meson_drm]
[ 2179.581966]  drm_atomic_helper_commit_planes+0x1e0/0x21c [drm_kms_helper]
[ 2179.588684]  drm_atomic_helper_commit_tail_rpm+0x68/0xb0 [drm_kms_helper]
[ 2179.595410]  commit_tail+0xac/0x190 [drm_kms_helper]
[ 2179.600326]  drm_atomic_helper_commit+0x16c/0x390 [drm_kms_helper]
[ 2179.606484]  drm_atomic_commit+0x58/0x70 [drm]
[ 2179.610880]  drm_framebuffer_remove+0x398/0x434 [drm]
[ 2179.615881]  drm_mode_rmfb_work_fn+0x68/0x8c [drm]
[ 2179.620575]  process_one_work+0x1cc/0x49c
[ 2179.624538]  worker_thread+0x200/0x444
[ 2179.628246]  kthread+0x14c/0x160
[ 2179.631439]  ret_from_fork+0x10/0x38

caused by the fact that the RDMA buffer has already been freed, resulting in meson_rdma_writel_sync() getting a NULL pointer.

Move the afbcd reset and meson_rdma_free calls after the DRM unregistration is complete so that the teardown can safely complete.

Signed-off-by Marc Zyngier <[email protected]>

---
 drivers/gpu/drm/meson/meson_drv.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

drivers/gpu/drm/meson/meson_drv.c +0 -5

index 8b9c8dd788c4..324fa489f1c4 100644
@@ -389,15 +389,15 @@ static void meson_drv_unbind(struct device *dev)
 		meson_canvas_free(priv->canvas, priv->canvas_id_vd1_2);
 	}
 
-	if (priv->afbcd.ops) {
-		priv->afbcd.ops->reset(priv);
-		meson_rdma_free(priv);
-	}
-
 	drm_dev_unregister(drm);
 	drm_irq_uninstall(drm);

Removing the meson DRM module results in the following splats:

[   42.689228] WARNING: CPU: 0 PID: 572 at drivers/gpu/drm/drm_irq.c:192 drm_irq_uninstall+0x130/0x160 [drm]
[...]
[   42.812820] Hardware name:  , BIOS 2021.01-rc2-00012-gde865f7ee1 11/16/2020
[   42.819723] pstate: 80400089 (Nzcv daIf +PAN -UAO -TCO BTYPE=--)
[   42.825737] pc : drm_irq_uninstall+0x130/0x160 [drm]
[   42.830647] lr : drm_irq_uninstall+0xc4/0x160 [drm]
[...]
[   42.917614] Call trace:
[   42.920086]  drm_irq_uninstall+0x130/0x160 [drm]
[   42.924612]  meson_drv_unbind+0x68/0xa4 [meson_drm]
[   42.929436]  component_del+0xc0/0x180
[   42.933058]  meson_dw_hdmi_remove+0x28/0x40 [meson_dw_hdmi]
[   42.938576]  platform_drv_remove+0x38/0x60
[   42.942628]  __device_release_driver+0x190/0x23c
[   42.947198]  driver_detach+0xcc/0x160
[   42.950822]  bus_remove_driver+0x68/0xe0
[   42.954702]  driver_unregister+0x3c/0x6c
[   42.958583]  platform_driver_unregister+0x20/0x2c
[   42.963243]  meson_dw_hdmi_platform_driver_exit+0x18/0x4a8 [meson_dw_hdmi]
[   42.970057]  __arm64_sys_delete_module+0x1bc/0x294
[   42.974801]  el0_svc_common.constprop.0+0x80/0x240
[   42.979542]  do_el0_svc+0x30/0xa0
[   42.982821]  el0_svc+0x18/0x50
[   42.985839]  el0_sync_handler+0x198/0x404
[   42.989806]  el0_sync+0x158/0x180

immediatelly followed by

[   43.002296] WARNING: CPU: 0 PID: 572 at drivers/gpu/drm/drm_mode_config.c:504 drm_mode_config_cleanup+0x2a8/0x304 [drm]
[...]
[   43.128150] Hardware name:  , BIOS 2021.01-rc2-00012-gde865f7ee1 11/16/2020
[   43.135052] pstate: 80400009 (Nzcv daif +PAN -UAO -TCO BTYPE=--)
[   43.141062] pc : drm_mode_config_cleanup+0x2a8/0x304 [drm]
[   43.146492] lr : drm_mode_config_cleanup+0xac/0x304 [drm]
[...]
[   43.233979] Call trace:
[   43.236451]  drm_mode_config_cleanup+0x2a8/0x304 [drm]
[   43.241538]  drm_mode_config_init_release+0x1c/0x2c [drm]
[   43.246886]  drm_managed_release+0xa8/0x120 [drm]
[   43.251543]  drm_dev_put+0x94/0xc0 [drm]
[   43.255380]  meson_drv_unbind+0x78/0xa4 [meson_drm]
[   43.260204]  component_del+0xc0/0x180
[   43.263829]  meson_dw_hdmi_remove+0x28/0x40 [meson_dw_hdmi]
[   43.269344]  platform_drv_remove+0x38/0x60
[   43.273398]  __device_release_driver+0x190/0x23c
[   43.277967]  driver_detach+0xcc/0x160
[   43.281590]  bus_remove_driver+0x68/0xe0
[   43.285471]  driver_unregister+0x3c/0x6c
[   43.289352]  platform_driver_unregister+0x20/0x2c
[   43.294011]  meson_dw_hdmi_platform_driver_exit+0x18/0x4a8 [meson_dw_hdmi]
[   43.300826]  __arm64_sys_delete_module+0x1bc/0x294
[   43.305570]  el0_svc_common.constprop.0+0x80/0x240
[   43.310312]  do_el0_svc+0x30/0xa0
[   43.313590]  el0_svc+0x18/0x50
[   43.316608]  el0_sync_handler+0x198/0x404
[   43.320574]  el0_sync+0x158/0x180
[   43.323852] ---[ end trace d796a3072dab01da ]---
[   43.328561] [drm:drm_mode_config_cleanup [drm]] *ERROR* connector HDMI-A-1 leaked!

both triggered by the fact that the HDMI subsystem is still active, and the DRM removal doesn't result in the connectors being torn down.

Call drm_atomic_helper_shutdown() and component_unbind_all() to safely tear the module down.

Signed-off-by Marc Zyngier <[email protected]>

---
 drivers/gpu/drm/meson/meson_drv.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

drivers/gpu/drm/meson/meson_drv.c +0 -0

index 324fa489f1c4..3d1de9cbb1c8 100644
@@ -390,8 +390,10 @@ static void meson_drv_unbind(struct device *dev)
 	}

Removing the meson-dw-hdmi module results in the following splat:

i[ 43.340509] WARNING: CPU: 0 PID: 572 at drivers/regulator/core.c:2125 _regulator_put.part.0+0x16c/0x174
[...]

[   43.454870] CPU: 0 PID: 572 Comm: modprobe Tainted: G        W   E     5.10.0-rc4-00049-gd274813a4de3-dirty #2147
[   43.465042] Hardware name:  , BIOS 2021.01-rc2-00012-gde865f7ee1 11/16/2020
[   43.471945] pstate: 80400009 (Nzcv daif +PAN -UAO -TCO BTYPE=--)
[   43.477896] pc : _regulator_put.part.0+0x16c/0x174
[   43.482638] lr : regulator_put+0x44/0x60
[...]
[   43.568715] Call trace:
[   43.571132]  _regulator_put.part.0+0x16c/0x174
[   43.575529]  regulator_put+0x44/0x60
[   43.579067]  devm_regulator_release+0x20/0x2c
[   43.583380]  release_nodes+0x1c8/0x2b4
[   43.587087]  devres_release_all+0x44/0x6c
[   43.591056]  __device_release_driver+0x1a0/0x23c
[   43.595626]  driver_detach+0xcc/0x160
[   43.599249]  bus_remove_driver+0x68/0xe0
[   43.603130]  driver_unregister+0x3c/0x6c
[   43.607011]  platform_driver_unregister+0x20/0x2c
[   43.611678]  meson_dw_hdmi_platform_driver_exit+0x18/0x4a8 [meson_dw_hdmi]
[   43.618485]  __arm64_sys_delete_module+0x1bc/0x294

as the HDMI regulator is still enabled on release.

In order to address this, register a callback that will deal with the disabling when the driver is unbound, solving the problem.

Signed-off-by Marc Zyngier <[email protected]>

---
 drivers/gpu/drm/meson/meson_dw_hdmi.c | 9 +++++++++
 1 file changed, 9 insertions(+)

drivers/gpu/drm/meson/meson_dw_hdmi.c +5 -0

index 29a8ff41595d..68826cf9993f 100644
@@ -941,6 +941,11 @@ static void meson_dw_hdmi_init(struct meson_dw_hdmi *meson_dw_hdmi)
 
 }
 
+static void meson_disable_regulator(void *data)
+{
+	regulator_disable(data);
+}
+
 static int meson_dw_hdmi_bind(struct device *dev, struct device *master,
 				void *data)
 {

Removing the meson-dw-hdmi module and re-inserting it results in a hang as the driver writes to HDMITX_TOP_SW_RESET. Similar effects can be seen when booting with mainline u-boot and using the u-boot provided DT (which is highly desirable).

The reason for the hang seem to be that the clocks are not always enabled by the time we enter meson_dw_hdmi_init(). Moving this call *after* dw_hdmi_probe() ensures that the clocks are enabled.

Signed-off-by Marc Zyngier <[email protected]>

---
 drivers/gpu/drm/meson/meson_dw_hdmi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/gpu/drm/meson/meson_dw_hdmi.c +0 -2

index 68826cf9993f..7f8eea494147 100644
@@ -1073,8 +1073,6 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master,
 
 	DRM_DEBUG_DRIVER("encoder initialized\n");
 
-	meson_dw_hdmi_init(meson_dw_hdmi);
-
 	/* Bridge / Connector */
 
 	dw_plat_data->priv_data = meson_dw_hdmi;

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(-)

Hi Marc,

Quoted 16 lines · Marc Zyngier

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

Quoted 4 lines

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.

Hi Neil,

Quoted 24 lines · Neil Armstrong

Hi Marc,

Quoted 20 lines · Marc Zyngier

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...

Quoted 2 lines

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