drm/ingenic: Call drm_atomic_helper_shutdown() at shutdown time

Based on grepping through the source code this driver appears to be
missing a call to drm_atomic_helper_shutdown() at system shutdown
time. Among other things, this means that if a panel is in use that it
won't be cleanly powered off at system shutdown time.

The fact that we should call drm_atomic_helper_shutdown() in the case
of OS shutdown/restart comes straight out of the kernel doc "driver
instance overview" in drm_drv.c.

Since this driver uses the component model and shutdown happens at the
base driver, we communicate whether we have to call
drm_atomic_helper_shutdown() by seeing if drvdata is non-NULL.

Suggested-by: Maxime Ripard <mripard@kernel.org>
Acked-by: Paul Cercueil <paul@crapouillou.net>
Reviewed-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20230901164111.RFT.3.Iea742f06d8bec41598aa40378fc625fbd7e8a3d6@changeid

diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
index c2547d48d6aadea57549ed06331c51bbf67784fa..0751235007a7e2ac8fb72a0c8e6a59ad51630a04 100644 (file)
--- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
@@ -1130,7 +1130,7 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
 
        ret = drmm_mode_config_init(drm);
        if (ret)
-               return ret;
+               goto err_drvdata;
 
        drm->mode_config.min_width = 0;
        drm->mode_config.min_height = 0;
@@ -1142,7 +1142,8 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
        base = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
        if (IS_ERR(base)) {
                dev_err(dev, "Failed to get memory resource\n");
-               return PTR_ERR(base);
+               ret = PTR_ERR(base);
+               goto err_drvdata;
        }
 
        regmap_config = ingenic_drm_regmap_config;
@@ -1151,33 +1152,40 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
                                          &regmap_config);
        if (IS_ERR(priv->map)) {
                dev_err(dev, "Failed to create regmap\n");
-               return PTR_ERR(priv->map);
+               ret = PTR_ERR(priv->map);
+               goto err_drvdata;
        }
 
        irq = platform_get_irq(pdev, 0);
-       if (irq < 0)
-               return irq;
+       if (irq < 0) {
+               ret = irq;
+               goto err_drvdata;
+       }
 
        if (soc_info->needs_dev_clk) {
                priv->lcd_clk = devm_clk_get(dev, "lcd");
                if (IS_ERR(priv->lcd_clk)) {
                        dev_err(dev, "Failed to get lcd clock\n");
-                       return PTR_ERR(priv->lcd_clk);
+                       ret = PTR_ERR(priv->lcd_clk);
+                       goto err_drvdata;
                }
        }
 
        priv->pix_clk = devm_clk_get(dev, "lcd_pclk");
        if (IS_ERR(priv->pix_clk)) {
                dev_err(dev, "Failed to get pixel clock\n");
-               return PTR_ERR(priv->pix_clk);
+               ret = PTR_ERR(priv->pix_clk);
+               goto err_drvdata;
        }
 
        priv->dma_hwdescs = dmam_alloc_coherent(dev,
                                                sizeof(*priv->dma_hwdescs),
                                                &priv->dma_hwdescs_phys,
                                                GFP_KERNEL);
-       if (!priv->dma_hwdescs)
-               return -ENOMEM;
+       if (!priv->dma_hwdescs) {
+               ret = -ENOMEM;
+               goto err_drvdata;
+       }
 
        /* Configure DMA hwdesc for foreground0 plane */
        ingenic_drm_configure_hwdesc_plane(priv, 0);
@@ -1199,7 +1207,7 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
                                       NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
        if (ret) {
                dev_err(dev, "Failed to register plane: %i\n", ret);
-               return ret;
+               goto err_drvdata;
        }
 
        if (soc_info->map_noncoherent)
@@ -1211,7 +1219,7 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
                                        NULL, &ingenic_drm_crtc_funcs, NULL);
        if (ret) {
                dev_err(dev, "Failed to init CRTC: %i\n", ret);
-               return ret;
+               goto err_drvdata;
        }
 
        drm_crtc_enable_color_mgmt(&priv->crtc, 0, false,
@@ -1230,7 +1238,7 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
                if (ret) {
                        dev_err(dev, "Failed to register overlay plane: %i\n",
                                ret);
-                       return ret;
+                       goto err_drvdata;
                }
 
                if (soc_info->map_noncoherent)
@@ -1241,17 +1249,18 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
                        if (ret) {
                                if (ret != -EPROBE_DEFER)
                                        dev_err(dev, "Failed to bind components: %i\n", ret);
-                               return ret;
+                               goto err_drvdata;
                        }
 
                        ret = devm_add_action_or_reset(dev, ingenic_drm_unbind_all, priv);
                        if (ret)
-                               return ret;
+                               goto err_drvdata;
 
                        priv->ipu_plane = drm_plane_from_index(drm, 2);
                        if (!priv->ipu_plane) {
                                dev_err(dev, "Failed to retrieve IPU plane\n");
-                               return -EINVAL;
+                               ret = -EINVAL;
+                               goto err_drvdata;
                        }
                }
        }
@@ -1263,7 +1272,7 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
                                break; /* we're done */
                        if (ret != -EPROBE_DEFER)
                                dev_err(dev, "Failed to get bridge handle\n");
-                       return ret;
+                       goto err_drvdata;
                }
 
                if (panel)
@@ -1275,7 +1284,7 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
                if (IS_ERR(ib)) {
                        ret = PTR_ERR(ib);
                        dev_err(dev, "Failed to init encoder: %d\n", ret);
-                       return ret;
+                       goto err_drvdata;
                }
 
                encoder = &ib->encoder;
@@ -1290,13 +1299,14 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
                                        DRM_BRIDGE_ATTACH_NO_CONNECTOR);
                if (ret) {
                        dev_err(dev, "Unable to attach bridge\n");
-                       return ret;
+                       goto err_drvdata;
                }
 
                connector = drm_bridge_connector_init(drm, encoder);
                if (IS_ERR(connector)) {
                        dev_err(dev, "Unable to init connector\n");
-                       return PTR_ERR(connector);
+                       ret = PTR_ERR(connector);
+                       goto err_drvdata;
                }
 
                drm_connector_attach_encoder(connector, encoder);
@@ -1313,13 +1323,13 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
        ret = devm_request_irq(dev, irq, ingenic_drm_irq_handler, 0, drm->driver->name, drm);
        if (ret) {
                dev_err(dev, "Unable to install IRQ handler\n");
-               return ret;
+               goto err_drvdata;
        }
 
        ret = drm_vblank_init(drm, 1);
        if (ret) {
                dev_err(dev, "Failed calling drm_vblank_init()\n");
-               return ret;
+               goto err_drvdata;
        }
 
        drm_mode_config_reset(drm);
@@ -1327,7 +1337,7 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
        ret = clk_prepare_enable(priv->pix_clk);
        if (ret) {
                dev_err(dev, "Unable to start pixel clock\n");
-               return ret;
+               goto err_drvdata;
        }
 
        if (priv->lcd_clk) {
@@ -1402,6 +1412,8 @@ err_devclk_disable:
                clk_disable_unprepare(priv->lcd_clk);
 err_pixclk_disable:
        clk_disable_unprepare(priv->pix_clk);
+err_drvdata:
+       platform_set_drvdata(pdev, NULL);
        return ret;
 }
 
@@ -1422,6 +1434,7 @@ static void ingenic_drm_unbind(struct device *dev)
 
        drm_dev_unregister(&priv->drm);
        drm_atomic_helper_shutdown(&priv->drm);
+       dev_set_drvdata(dev, NULL);
 }
 
 static const struct component_master_ops ingenic_master_ops = {
@@ -1459,6 +1472,14 @@ static void ingenic_drm_remove(struct platform_device *pdev)
                component_master_del(dev, &ingenic_master_ops);
 }
 
+static void ingenic_drm_shutdown(struct platform_device *pdev)
+{
+       struct ingenic_drm *priv = platform_get_drvdata(pdev);
+
+       if (priv)
+               drm_atomic_helper_shutdown(&priv->drm);
+}
+
 static int ingenic_drm_suspend(struct device *dev)
 {
        struct ingenic_drm *priv = dev_get_drvdata(dev);
@@ -1610,6 +1631,7 @@ static struct platform_driver ingenic_drm_driver = {
        },
        .probe = ingenic_drm_probe,
        .remove_new = ingenic_drm_remove,
+       .shutdown = ingenic_drm_shutdown,
 };
 
 static int ingenic_drm_init(void)