linux-kernel - [RFT PATCH 6/6] drm/hisilicon/kirin: Call drm_atomic_helper

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230901163944.RFT.6.I21e0916bbd276033f7d31979c0da171458dedd4d@changeid>
Date:   Fri,  1 Sep 2023 16:39:57 -0700
From:   Douglas Anderson <dianders@...omium.org>
To:     dri-devel@...ts.freedesktop.org, Maxime Ripard <mripard@...nel.org>
Cc:     Douglas Anderson <dianders@...omium.org>, airlied@...il.com,
        daniel@...ll.ch, javierm@...hat.com, jstultz@...gle.com,
        kong.kongxinwei@...ilicon.com, linux-kernel@...r.kernel.org,
        robh@...nel.org, steven.price@....com, sumit.semwal@...aro.org,
        tiantao6@...ilicon.com, tzimmermann@...e.de,
        u.kleine-koenig@...gutronix.de, xinliang.liu@...aro.org,
        yongqin.liu@...aro.org
Subject: [RFT PATCH 6/6] drm/hisilicon/kirin: Call drm_atomic_helper_shutdown() at shutdown/unbind 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
and at driver unbind 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 and at driver remove (or unbind) time comes
straight out of the kernel doc "driver instance overview" in
drm_drv.c.

I have attempted to put this in the right place at unbind time. In
most other DRM drivers the call is made right after the call to
drm_kms_helper_poll_fini(), so I've put it there. That means that this
call will also be made in the case that we hit errors in bind, since
kirin_drm_kms_cleanup() is called both in the bind error path and in
unbind. I believe this is harmless even though it's not needed in the
bind error path.

For handling shutdown, we rely on the common technique of seeing if
the drvdata is NULL to know whether we need to call
drm_atomic_helper_shutdown(). This makes it important to make sure
that the drvdata is NULL if bind failed or if unbind was called. We
don't need the actual check for NULL and we'll rely on the patch
("drm/atomic-helper: drm_atomic_helper_shutdown(NULL) should be a
noop").

Suggested-by: Maxime Ripard <mripard@...nel.org>
Signed-off-by: Douglas Anderson <dianders@...omium.org>
---
This commit is only compile-time tested.

 drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
index e8c77bcc6dae..75292a2f4644 100644
--- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
@@ -206,6 +206,7 @@ static int kirin_drm_kms_init(struct drm_device *dev,
 static int kirin_drm_kms_cleanup(struct drm_device *dev)
 {
 	drm_kms_helper_poll_fini(dev);
+	drm_atomic_helper_shutdown(dev);
 	kirin_drm_private_cleanup(dev);
 	drm_mode_config_cleanup(dev);

@@ -244,6 +245,7 @@ static int kirin_drm_bind(struct device *dev)
 	kirin_drm_kms_cleanup(drm_dev);
 err_drm_dev_put:
 	drm_dev_put(drm_dev);
+	dev_set_drvdata(dev, NULL);

 	return ret;
 }
@@ -255,6 +257,7 @@ static void kirin_drm_unbind(struct device *dev)
 	drm_dev_unregister(drm_dev);
 	kirin_drm_kms_cleanup(drm_dev);
 	drm_dev_put(drm_dev);
+	dev_set_drvdata(dev, NULL);
 }

 static const struct component_master_ops kirin_drm_ops = {
@@ -284,6 +287,11 @@ static void kirin_drm_platform_remove(struct platform_device *pdev)
 	component_master_del(&pdev->dev, &kirin_drm_ops);
 }

+static void kirin_drm_platform_shutdown(struct platform_device *pdev)
+{
+	drm_atomic_helper_shutdown(platform_get_drvdata(pdev));
+}
+
 static const struct of_device_id kirin_drm_dt_ids[] = {
 	{ .compatible = "hisilicon,hi6220-ade",
 	  .data = &ade_driver_data,
@@ -295,6 +303,7 @@ MODULE_DEVICE_TABLE(of, kirin_drm_dt_ids);
 static struct platform_driver kirin_drm_platform_driver = {
 	.probe = kirin_drm_platform_probe,
 	.remove_new = kirin_drm_platform_remove,
+	.shutdown = kirin_drm_platform_shutdown,
 	.driver = {
 		.name = "kirin-drm",
 		.of_match_table = kirin_drm_dt_ids,
-- 
2.42.0.283.g2d96d420d3-goog