| Message ID | 20230901164111.RFT.10.I7a2dd349cb52bae53280d0a49e22cc27b923274b@changeid (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: non-drm-misc drivers call drm_atomic_helper_shutdown() at the right times | expand |
On Fri, 1 Sep 2023 16:41:21 -0700, Douglas Anderson wrote: > Based on grepping through the source code, this driver appears to be > missing a call to drm_atomic_helper_shutdown() at remove time. Let's > add it. > > The fact that we should call drm_atomic_helper_shutdown() in the case > > [ ... ] Reviewed-by: Maxime Ripard <mripard@kernel.org> Thanks! Maxime
diff --git a/drivers/gpu/drm/sprd/sprd_drm.c b/drivers/gpu/drm/sprd/sprd_drm.c index 0aa39156f2fa..86a175116140 100644 --- a/drivers/gpu/drm/sprd/sprd_drm.c +++ b/drivers/gpu/drm/sprd/sprd_drm.c @@ -114,6 +114,7 @@ static int sprd_drm_bind(struct device *dev) drm_kms_helper_poll_fini(drm); err_unbind_all: component_unbind_all(drm->dev, drm); + platform_set_drvdata(pdev, NULL); return ret; } @@ -122,10 +123,11 @@ static void sprd_drm_unbind(struct device *dev) struct drm_device *drm = dev_get_drvdata(dev); drm_dev_unregister(drm); - drm_kms_helper_poll_fini(drm); + drm_atomic_helper_shutdown(drm); component_unbind_all(drm->dev, drm); + dev_set_drvdata(dev, NULL); } static const struct component_master_ops drm_component_ops = {
Based on grepping through the source code, this driver appears to be missing a call to drm_atomic_helper_shutdown() at remove time. Let's add it. The fact that we should call drm_atomic_helper_shutdown() in the case of OS driver remove comes straight out of the kernel doc "driver instance overview" in drm_drv.c. While at it, let's also fix it so that if the driver's bind fails or if a driver gets unbound that the drvdata gets set to NULL. This will make sure we can't get confused during a later shutdown(). Suggested-by: Maxime Ripard <mripard@kernel.org> Signed-off-by: Douglas Anderson <dianders@chromium.org> --- This commit is only compile-time tested. While making this patch, I noticed that the bind() function of this driver is using "devm". That's actually a bug. As per kernel docs [1] "the lifetime of the aggregate driver does not align with any of the underlying struct device instances. Therefore devm cannot be used and all resources acquired or allocated in this callback must be explicitly released in the unbind callback". Fixing that is outside the scope of this commit. [1] https://docs.kernel.org/driver-api/component.html drivers/gpu/drm/sprd/sprd_drm.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)