[PATCH 0/4] drm/meson: Module removal fixes

[PATCH 0/4] drm/meson: Module removal fixes

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

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

-- 
2.28.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/4] drm/meson: Free RDMA resources after tearing down DRM

[Marc Zyngier]

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 <maz@kernel.org>
---
 drivers/gpu/drm/meson/meson_drv.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
index 8b9c8dd788c4..324fa489f1c4 100644
--- a/drivers/gpu/drm/meson/meson_drv.c
+++ b/drivers/gpu/drm/meson/meson_drv.c
@@ -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);
 	drm_kms_helper_poll_fini(drm);
 	drm_dev_put(drm);
+
+	if (priv->afbcd.ops) {
+		priv->afbcd.ops->reset(priv);
+		meson_rdma_free(priv);
+	}
 }
 
 static const struct component_master_ops meson_drv_master_ops = {
-- 
2.28.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/4] drm/meson: Unbind all connectors on module removal

[Marc Zyngier]

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 <maz@kernel.org>
---
 drivers/gpu/drm/meson/meson_drv.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
index 324fa489f1c4..3d1de9cbb1c8 100644
--- a/drivers/gpu/drm/meson/meson_drv.c
+++ b/drivers/gpu/drm/meson/meson_drv.c
@@ -390,8 +390,10 @@ static void meson_drv_unbind(struct device *dev)
 	}
 
 	drm_dev_unregister(drm);
-	drm_irq_uninstall(drm);
 	drm_kms_helper_poll_fini(drm);
+	drm_atomic_helper_shutdown(drm);
+	component_unbind_all(dev, drm);
+	drm_irq_uninstall(drm);
 	drm_dev_put(drm);
 
 	if (priv->afbcd.ops) {
-- 
2.28.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/4] drm/meson: dw-hdmi: Register a callback to disable the regulator

[Marc Zyngier]

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 <maz@kernel.org>
---
 drivers/gpu/drm/meson/meson_dw_hdmi.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c
index 29a8ff41595d..68826cf9993f 100644
--- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
@@ -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)
 {
@@ -989,6 +994,10 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master,
 		ret = regulator_enable(meson_dw_hdmi->hdmi_supply);
 		if (ret)
 			return ret;
+		ret = devm_add_action_or_reset(dev, meson_disable_regulator,
+					       meson_dw_hdmi->hdmi_supply);
+		if (ret)
+			return ret;
 	}
 
 	meson_dw_hdmi->hdmitx_apb = devm_reset_control_get_exclusive(dev,
-- 
2.28.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/4] drm/meson: dw-hdmi: Ensure that clocks are enabled before touching the TOP registers

[Marc Zyngier]

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 <maz@kernel.org>
---
 drivers/gpu/drm/meson/meson_dw_hdmi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c
index 68826cf9993f..7f8eea494147 100644
--- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
@@ -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;
@@ -1097,6 +1095,8 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master,
 	if (IS_ERR(meson_dw_hdmi->hdmi))
 		return PTR_ERR(meson_dw_hdmi->hdmi);
 
+	meson_dw_hdmi_init(meson_dw_hdmi);
+
 	next_bridge = of_drm_find_bridge(pdev->dev.of_node);
 	if (next_bridge)
 		drm_bridge_attach(encoder, next_bridge,
-- 
2.28.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/4] drm/meson: Module removal fixes

[Neil Armstrong]

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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/4] drm/meson: Free RDMA resources after tearing down DRM

[Neil Armstrong]

On 16/11/2020 21:07, Marc Zyngier wrote:
> 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 <maz@kernel.org>

Fixes: d1b5e41e13a7 ("drm/meson: Add AFBCD module driver")

> ---
>  drivers/gpu/drm/meson/meson_drv.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
> index 8b9c8dd788c4..324fa489f1c4 100644
> --- a/drivers/gpu/drm/meson/meson_drv.c
> +++ b/drivers/gpu/drm/meson/meson_drv.c
> @@ -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);
>  	drm_kms_helper_poll_fini(drm);
>  	drm_dev_put(drm);
> +
> +	if (priv->afbcd.ops) {
> +		priv->afbcd.ops->reset(priv);
> +		meson_rdma_free(priv);
> +	}
>  }
>  
>  static const struct component_master_ops meson_drv_master_ops = {
> 

Acked-by: Neil Armstrong <narmstrong@baylibre.com>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/4] drm/meson: Unbind all connectors on module removal

[Neil Armstrong]

On 16/11/2020 21:07, Marc Zyngier wrote:
> 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 <maz@kernel.org>

Fixes: 2d8f92897ad8 ("drm/meson: Uninstall IRQ handler")

> ---
>  drivers/gpu/drm/meson/meson_drv.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
> index 324fa489f1c4..3d1de9cbb1c8 100644
> --- a/drivers/gpu/drm/meson/meson_drv.c
> +++ b/drivers/gpu/drm/meson/meson_drv.c
> @@ -390,8 +390,10 @@ static void meson_drv_unbind(struct device *dev)
>  	}
>  
>  	drm_dev_unregister(drm);
> -	drm_irq_uninstall(drm);
>  	drm_kms_helper_poll_fini(drm);
> +	drm_atomic_helper_shutdown(drm);
> +	component_unbind_all(dev, drm);
> +	drm_irq_uninstall(drm);
>  	drm_dev_put(drm);
>  
>  	if (priv->afbcd.ops) {
> 

Acked-by: Neil Armstrong <narmstrong@baylibre.com>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/4] drm/meson: dw-hdmi: Register a callback to disable the regulator

[Neil Armstrong]

On 16/11/2020 21:07, Marc Zyngier wrote:
> 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 <maz@kernel.org>

Fixes: 161a803fe32d ("drm/meson: dw_hdmi: Add support for an optional external 5V regulator")

> ---
>  drivers/gpu/drm/meson/meson_dw_hdmi.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> index 29a8ff41595d..68826cf9993f 100644
> --- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
> +++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> @@ -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)
>  {
> @@ -989,6 +994,10 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master,
>  		ret = regulator_enable(meson_dw_hdmi->hdmi_supply);
>  		if (ret)
>  			return ret;
> +		ret = devm_add_action_or_reset(dev, meson_disable_regulator,
> +					       meson_dw_hdmi->hdmi_supply);
> +		if (ret)
> +			return ret;
>  	}
>  
>  	meson_dw_hdmi->hdmitx_apb = devm_reset_control_get_exclusive(dev,
> 

Acked-by: Neil Armstrong <narmstrong@baylibre.com>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/4] drm/meson: dw-hdmi: Ensure that clocks are enabled before touching the TOP registers

[Neil Armstrong]

On 16/11/2020 21:07, Marc Zyngier wrote:
> 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 <maz@kernel.org>

Fixes: 1374b8375c2e ("drm/meson: dw_hdmi: add resume/suspend hooks")

> ---
>  drivers/gpu/drm/meson/meson_dw_hdmi.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> index 68826cf9993f..7f8eea494147 100644
> --- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
> +++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> @@ -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;
> @@ -1097,6 +1095,8 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master,
>  	if (IS_ERR(meson_dw_hdmi->hdmi))
>  		return PTR_ERR(meson_dw_hdmi->hdmi);
>  
> +	meson_dw_hdmi_init(meson_dw_hdmi);
> +
>  	next_bridge = of_drm_find_bridge(pdev->dev.of_node);
>  	if (next_bridge)
>  		drm_bridge_attach(encoder, next_bridge,
> 

Acked-by: Neil Armstrong <narmstrong@baylibre.com>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/4] drm/meson: Module removal fixes

[Marc Zyngier]

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
-- 
Jazz is not dead. It just smells funny...
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/4] drm/meson: Module removal fixes

[Neil Armstrong]

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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel