drm: rockchip: Fix rockchip drm unbind crash error
Subsystems (5)
v4 cover drm: rockchip: Fix rockchip drm unbind crash error
Verified on rk3399 chromebook kevin:
1/ stop ui && pkill -9 frecon
2/ unbind/bind drm
Changes in v4:
Address Andrzej Hajda <[email protected]>'s comments.
Changes in v3:
Update commit message.
Address Sean Paul <[email protected]>'s comments. Update commit message.
Address Sean Paul <[email protected]>'s comments. Update commit message.
Address Daniel Vetter <[email protected]>'s comments. Update commit message.
Changes in v2:
Fix some commit messages.
Jeffy Chen (12):
drm: bridge: analogix: Detach panel when unbinding analogix dp
drm: bridge: analogix: Unregister dp aux when unbinding
drm: bridge: analogix: Disable clock when unbinding
drm: bridge: analogix: Destroy connector & encoder when unbinding drm/rockchip: cdn-dp: Don't try to release firmware when not loaded drm/rockchip: cdn-dp: Don't unregister audio dev when unbinding drm/rockchip: vop: Enable pm domain before vop_initial drm/rockchip: vop: Unprepare clocks when unbinding drm/rockchip: analogix_dp: Disable clock when unbinding drm/rockchip: Reoder drm bind/unbind sequence drm/rockchip: Shutdown all crtcs when unbinding drm drm/drm_ioctl.c: Break ioctl when drm device not registered
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 6 +++ drivers/gpu/drm/drm_ioctl.c | 2 +- drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 3 +- drivers/gpu/drm/rockchip/cdn-dp-core.c | 10 +++-- drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 50 ++++++++++++---------- drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 33 ++++++++++---- 6 files changed, 67 insertions(+), 37 deletions(-)
Signature · 1 line
2.1.4
PATCH v4 1/12 drm: bridge: analogix: Detach panel when unbinding analogix dp
From: Jeffy Chen <[email protected]>
The panel is attached when binding analogix dp.
---
Changes in v4: None
Changes in v3: None
Changes in v2:
Fix some commit messages.
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 2 ++ 1 file changed, 2 insertions(+)
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +2 -0
@@ -1443,6 +1443,8 @@ void analogix_dp_unbind(struct device *dev, struct device *master,if (dp->plat_data->panel) {if (drm_panel_unprepare (dp->plat_data->panel))DRM_ERROR ("failed to turnoff the panel\n" ); +if (drm_panel_detach (dp->plat_data->panel)) +DRM_ERROR ("failed to detach the panel\n" ); }pm_runtime_disable (dev);
Signature · 1 line
2.1.4
PATCH v4 2/12 drm: bridge: analogix: Unregister dp aux when unbinding
In reply to Your NameFrom: Jeffy Chen <[email protected]>
The dp aux is registered when binding analogix dp.
---
Changes in v4: None
Changes in v3: None
Changes in v2: None
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 1 + 1 file changed, 1 insertion(+)
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +1 -0
@@ -1447,6 +1447,7 @@ void analogix_dp_unbind(struct device *dev, struct device *master,DRM_ERROR ("failed to detach the panel\n" ); } +drm_dp_aux_unregister (&dp->aux);pm_runtime_disable (dev); }EXPORT_SYMBOL_GPL (analogix_dp_unbind);
Signature · 1 line
2.1.4
PATCH v4 3/12 drm: bridge: analogix: Disable clock when unbinding
In reply to Your NameFrom: Jeffy Chen <[email protected]>
The clock is enabled when binding analogix dp.
---
Changes in v4: None
Changes in v3: None
Changes in v2: None
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 1 + 1 file changed, 1 insertion(+)
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +1 -0
@@ -1449,6 +1449,7 @@ void analogix_dp_unbind(struct device *dev, struct device *master,drm_dp_aux_unregister (&dp->aux);pm_runtime_disable (dev); +clk_disable_unprepare (dp->clock); }EXPORT_SYMBOL_GPL (analogix_dp_unbind);
Signature · 1 line
2.1.4
PATCH v4 4/12 drm: bridge: analogix: Destroy connector & encoder when unbinding
In reply to Your NameFrom: Jeffy Chen <[email protected]>
Normally we do this in drm_mode_config_cleanup. But:
1/ analogix dp's connector is allocated in bind, and freed after unbind. So we need to destroy it in unbind to avoid further access. 2/ the drm bridge is attached in bind, and detached in encoder cleanup. So we need to destroy encoder in unbind.
---
Changes in v4:
Address Andrzej Hajda <[email protected]>'s comments.
Changes in v3: None
Changes in v2: None
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 2 ++ 1 file changed, 2 insertions(+)
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +2 -0
@@ -1439,6 +1439,8 @@ void analogix_dp_unbind(struct device *dev, struct device *master,struct analogix_dp_device *dp =dev_get_drvdata (dev);analogix_dp_bridge_disable (dp->bridge); + dp->connector.funcs->destroy (&dp->connector); + dp->encoder->funcs->destroy (dp->encoder);if (dp->plat_data->panel) {if (drm_panel_unprepare (dp->plat_data->panel))
Signature · 1 line
2.1.4
PATCH v4 5/12 drm/rockchip: cdn-dp: Don't try to release firmware when not loaded
In reply to Your NameFrom: Jeffy Chen <[email protected]>
---
Changes in v4: None
Changes in v3: None
Changes in v2: None
drivers/gpu/drm/rockchip/cdn-dp-core.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/gpu/drm/rockchip/cdn-dp-core.c +3 -1
@@ -1053,6 +1053,7 @@ static int cdn_dp_bind(struct device *dev, struct device *master, void *data) dp->connected = false; dp->active = false; dp->active_port =-1 ; + dp->fw_loaded = false;INIT_WORK (&dp->event_work, cdn_dp_pd_event_work);
@@ -1133,7 +1134,8 @@ static void cdn_dp_unbind(struct device *dev, struct device *master, void *data) connector->funcs->destroy (connector);pm_runtime_disable (dev); -release_firmware (dp->fw); +if (dp->fw_loaded) +release_firmware (dp->fw);kfree (dp->edid); dp->edid =NULL ; }
Signature · 1 line
2.1.4
PATCH v4 6/12 drm/rockchip: cdn-dp: Don't unregister audio dev when unbinding
In reply to Your NameFrom: Jeffy Chen <[email protected]>
After snd_soc_unregister_codec, the dai link would remain bound to the invalid codec. That would cause crashes after unbind dp driver.
Let's unregister audio codec when removing dp driver to prevent that.
---
Changes in v4: None
Changes in v3:
Update commit message.
Changes in v2: None
drivers/gpu/drm/rockchip/cdn-dp-core.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/gpu/drm/rockchip/cdn-dp-core.c +3 -3
@@ -1092,8 +1092,6 @@ static int cdn_dp_bind(struct device *dev, struct device *master, void *data) goto err_free_connector; } -cdn_dp_audio_codec_init (dp, dev); -for (i =0 ; i < dp->ports; i++) { port = dp->port[i];
@@ -1128,7 +1126,6 @@ static void cdn_dp_unbind(struct device *dev, struct device *master, void *data)struct drm_connector *connector = &dp->connector;cancel_work_sync (&dp->event_work); -platform_device_unregister (dp->audio_pdev);cdn_dp_encoder_disable (encoder); encoder->funcs->destroy (encoder); connector->funcs->destroy (connector);
@@ -1221,6 +1218,8 @@ static int cdn_dp_probe(struct platform_device *pdev)mutex_init (&dp->lock);dev_set_drvdata (dev, dp); +cdn_dp_audio_codec_init (dp, dev); +return component_add (dev, &cdn_dp_component_ops); }
@@ -1228,6 +1227,7 @@ static int cdn_dp_remove(struct platform_device *pdev) {struct cdn_dp_device *dp =platform_get_drvdata (pdev); +platform_device_unregister (dp->audio_pdev);cdn_dp_suspend (dp->dev);component_del (&pdev->dev, &cdn_dp_component_ops);
Signature · 1 line
2.1.4
PATCH v4 7/12 drm/rockchip: vop: Enable pm domain before vop_initial
In reply to Your NameFrom: Jeffy Chen <[email protected]>
We're trying to access vop registers here, so need to make sure the pm domain is on.
Normally it should be enabled by the bootloader, but there's no guarantee of it. And if we wanna do unbind/bind, it would also cause the device to hang.
And this patch also does these:
1/ move vop_initial to the end of vop_bind for eaiser error handling. 2/ correct the err_put_pm_runtime of vop_enable.
---
Changes in v4: None
Changes in v3:
Address Sean Paul <[email protected]>'s comments. Update commit message.
Changes in v2: None
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-)
drivers/gpu/drm/rockchip/rockchip_drm_vop.c +21 -8
@@ -506,7 +506,7 @@ static int vop_enable(struct drm_crtc *crtc) ret =pm_runtime_get_sync (vop->dev);if (ret <0 ) {dev_err (vop->dev,"failed to get pm runtime: %d\n" , ret); - goto err_put_pm_runtime; +return ret; } ret =clk_enable (vop->hclk);
@@ -1405,10 +1405,16 @@ static int vop_initial(struct vop *vop)return PTR_ERR (vop->dclk); } + ret =pm_runtime_get_sync (vop->dev); +if (ret <0 ) { +dev_err (vop->dev,"failed to get pm runtime: %d\n" , ret); +return ret; + } + ret =clk_prepare (vop->dclk);if (ret <0 ) {dev_err (vop->dev,"failed to prepare dclk\n" ); -return ret; + goto err_put_pm_runtime; }/* Enable both the hclk and aclk to setup the vop */
@@ -1468,6 +1474,8 @@ static int vop_initial(struct vop *vop) vop->is_enabled = false; +pm_runtime_put_sync (vop->dev); +return 0 ; err_disable_aclk:
@@ -1476,6 +1484,8 @@ static int vop_initial(struct vop *vop)clk_disable_unprepare (vop->hclk); err_unprepare_dclk:clk_unprepare (vop->dclk); +err_put_pm_runtime: +pm_runtime_put_sync (vop->dev);return ret; }
@@ -1576,12 +1586,6 @@ static int vop_bind(struct device *dev, struct device *master, void *data)if (!vop->regsbak)return -ENOMEM ; - ret =vop_initial (vop); -if (ret <0 ) { -dev_err (&pdev->dev,"cannot initial vop dev - err %d\n" , ret); -return ret; - } - irq =platform_get_irq (pdev,0 );if (irq <0 ) {dev_err (dev,"cannot find irq for vop\n" );
@@ -1608,8 +1612,17 @@ static int vop_bind(struct device *dev, struct device *master, void *data)pm_runtime_enable (&pdev->dev); + ret =vop_initial (vop); +if (ret <0 ) { +dev_err (&pdev->dev,"cannot initial vop dev - err %d\n" , ret); + goto err_disable_pm_runtime; + } +return 0 ; +err_disable_pm_runtime: +pm_runtime_disable (&pdev->dev); +vop_destroy_crtc (vop); err_enable_irq:enable_irq (vop->irq);/* To balance out the disable_irq above */ return ret;
Signature · 1 line
2.1.4
PATCH v4 8/12 drm/rockchip: vop: Unprepare clocks when unbinding
In reply to Your NameFrom: Jeffy Chen <[email protected]>
The clocks are prepared when binding vop.
---
Changes in v4: None
Changes in v3: None
Changes in v2: None
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 4 ++++ 1 file changed, 4 insertions(+)
drivers/gpu/drm/rockchip/rockchip_drm_vop.c +4 -0
@@ -1634,6 +1634,10 @@ static void vop_unbind(struct device *dev, struct device *master, void *data)pm_runtime_disable (dev);vop_destroy_crtc (vop); + +clk_unprepare (vop->aclk); +clk_unprepare (vop->hclk); +clk_unprepare (vop->dclk); }const struct component_ops vop_component_ops = {
Signature · 1 line
2.1.4
PATCH v4 9/12 drm/rockchip: analogix_dp: Disable clock when unbinding
In reply to Your NameFrom: Jeffy Chen <[email protected]>
The clock is enabled when binding cdn dp.
---
Changes in v4: None
Changes in v3: None
Changes in v2: None
drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/gpu/drm/rockchip/analogix_dp-rockchip.c +2 -1
@@ -417,7 +417,8 @@ static void rockchip_dp_unbind(struct device *dev, struct device *master,rockchip_drm_psr_unregister (&dp->encoder); -return analogix_dp_unbind (dev, master, data); +analogix_dp_unbind (dev, master, data); +clk_disable_unprepare (dp->pclk); }static const struct component_ops rockchip_dp_component_ops = {
Signature · 1 line
2.1.4
PATCH v4 10/12 drm/rockchip: Reoder drm bind/unbind sequence
In reply to Your NameFrom: Jeffy Chen <[email protected]>
Current drm bind/unbind sequence would cause some memory issues. For example we should not cleanup iommu before cleanup mode config.
Reorder bind/unbind sequence, follow exynos drm.
---
Changes in v4: None
Changes in v3:
Address Sean Paul <[email protected]>'s comments. Update commit message.
Changes in v2: None
drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 49 +++++++++++++++-------------- 1 file changed, 26 insertions(+), 23 deletions(-)
drivers/gpu/drm/rockchip/rockchip_drm_drv.c +26 -23
@@ -136,21 +136,24 @@ static int rockchip_drm_bind(struct device *dev)INIT_LIST_HEAD (&private->psr_list);spin_lock_init (&private->psr_list_lock); + ret =rockchip_drm_init_iommu (drm_dev); +if (ret) + goto err_free; +drm_mode_config_init (drm_dev);rockchip_drm_mode_config_init (drm_dev); - ret =rockchip_drm_init_iommu (drm_dev); -if (ret) - goto err_config_cleanup; -/* Try to bind all sub drivers. */ ret =component_bind_all (dev, drm_dev);if (ret) - goto err_iommu_cleanup; + goto err_mode_config_cleanup; -/* init kms poll for handling hpd */ -drm_kms_helper_poll_init (drm_dev); + ret =drm_vblank_init (drm_dev, drm_dev->mode_config.num_crtc); +if (ret) + goto err_unbind_all; + +drm_mode_config_reset (drm_dev);/* * enable drm irq mode.
@@ -158,15 +161,12 @@ static int rockchip_drm_bind(struct device *dev)*/ drm_dev->irq_enabled = true; - ret =drm_vblank_init (drm_dev,ROCKCHIP_MAX_CRTC ); -if (ret) - goto err_kms_helper_poll_fini; - -drm_mode_config_reset (drm_dev); +/* init kms poll for handling hpd */ +drm_kms_helper_poll_init (drm_dev); ret =rockchip_drm_fbdev_init (drm_dev);if (ret) - goto err_vblank_cleanup; + goto err_kms_helper_poll_fini; ret =drm_dev_register (drm_dev,0 );if (ret)
@@ -175,17 +175,17 @@ static int rockchip_drm_bind(struct device *dev)return 0 ; err_fbdev_fini:rockchip_drm_fbdev_fini (drm_dev); -err_vblank_cleanup: -drm_vblank_cleanup (drm_dev); err_kms_helper_poll_fini:drm_kms_helper_poll_fini (drm_dev); +drm_vblank_cleanup (drm_dev); +err_unbind_all:component_unbind_all (dev, drm_dev); -err_iommu_cleanup: -rockchip_iommu_cleanup (drm_dev); -err_config_cleanup: +err_mode_config_cleanup:drm_mode_config_cleanup (drm_dev); - drm_dev->dev_private =NULL ; +rockchip_iommu_cleanup (drm_dev); err_free: + drm_dev->dev_private =NULL ; +dev_set_drvdata (dev,NULL );drm_dev_unref (drm_dev);return ret; }
@@ -194,16 +194,19 @@ static void rockchip_drm_unbind(struct device *dev) {struct drm_device *drm_dev =dev_get_drvdata (dev); +drm_dev_unregister (drm_dev); +rockchip_drm_fbdev_fini (drm_dev); -drm_vblank_cleanup (drm_dev);drm_kms_helper_poll_fini (drm_dev); + +drm_vblank_cleanup (drm_dev);component_unbind_all (dev, drm_dev); -rockchip_iommu_cleanup (drm_dev);drm_mode_config_cleanup (drm_dev); +rockchip_iommu_cleanup (drm_dev); + drm_dev->dev_private =NULL ; -drm_dev_unregister (drm_dev); -drm_dev_unref (drm_dev);dev_set_drvdata (dev,NULL ); +drm_dev_unref (drm_dev); }static void rockchip_drm_lastclose (struct drm_device *dev)
Signature · 1 line
2.1.4
PATCH v4 11/12 drm/rockchip: Shutdown all crtcs when unbinding drm
In reply to Your NameFrom: Jeffy Chen <[email protected]>
---
Changes in v4: None
Changes in v3:
Address Daniel Vetter <[email protected]>'s comments. Update commit message.
Changes in v2: None
drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 1 + 1 file changed, 1 insertion(+)
drivers/gpu/drm/rockchip/rockchip_drm_drv.c +1 -0
@@ -199,6 +199,7 @@ static void rockchip_drm_unbind(struct device *dev)rockchip_drm_fbdev_fini (drm_dev);drm_kms_helper_poll_fini (drm_dev); +drm_atomic_helper_shutdown (drm_dev);drm_vblank_cleanup (drm_dev);component_unbind_all (dev, drm_dev);drm_mode_config_cleanup (drm_dev);
Signature · 1 line
2.1.4
PATCH v4 12/12 drm/drm_ioctl.c: Break ioctl when drm device not registered
In reply to Your NameFrom: Jeffy Chen <[email protected]>
After unbinding drm, the user space may still owns the drm dev fd, and may still be able to call drm ioctl.
Add a sanity check here to prevent that from happening.
---
Changes in v4: None
Changes in v3: None
Changes in v2: None
drivers/gpu/drm/drm_ioctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/drm_ioctl.c +1 -1
@@ -674,7 +674,7 @@ long drm_ioctl(struct file *filp, dev = file_priv->minor->dev; -if (drm_device_is_unplugged (dev)) +if (drm_device_is_unplugged (dev) || !dev->registered)return -ENODEV ; is_driver_ioctl = nr >=DRM_COMMAND_BASE && nr <DRM_COMMAND_END ;
Signature · 1 line
2.1.4