All of lore.kernel.org
 help / color / mirror / Atom feed
From: Douglas Anderson <dianders@chromium.org>
To: dri-devel@lists.freedesktop.org, Maxime Ripard <mripard@kernel.org>
Cc: linux-samsung-soc@vger.kernel.org, alim.akhtar@samsung.com,
	sw0312.kim@samsung.com, Douglas Anderson <dianders@chromium.org>,
	linux-kernel@vger.kernel.org, krzysztof.kozlowski@linaro.org,
	kyungmin.park@samsung.com, linux-arm-kernel@lists.infradead.org
Subject: [RFT PATCH 11/15] drm/exynos: Call drm_atomic_helper_shutdown() at shutdown/unbind time
Date: Fri,  1 Sep 2023 16:41:22 -0700	[thread overview]
Message-ID: <20230901164111.RFT.11.Iea33274908b6b258955f45a8aaf6f5bba24ad6cd@changeid> (raw)
In-Reply-To: <20230901234202.566951-1-dianders@chromium.org>

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.

A few notes about this fix:
- When adding drm_atomic_helper_shutdown() to the unbind path, I added
  it after drm_kms_helper_poll_fini() since that's when other drivers
  seemed to have it.
- Technically with a previous patch, ("drm/atomic-helper:
  drm_atomic_helper_shutdown(NULL) should be a noop"), we don't
  actually need to check to see if our "drm" pointer is NULL before
  calling drm_atomic_helper_shutdown(). We'll leave the "if" test in,
  though, so that this patch can land without any dependencies. It
  could potentially be removed later.
- This patch also makes sure to set the drvdata to NULL in the case of
  bind errors to make sure that shutdown can't access freed data.

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

 drivers/gpu/drm/exynos/exynos_drm_drv.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index 8399256cb5c9..5380fb6c55ae 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -300,6 +300,7 @@ static int exynos_drm_bind(struct device *dev)
 	drm_mode_config_cleanup(drm);
 	exynos_drm_cleanup_dma(drm);
 	kfree(private);
+	dev_set_drvdata(dev, NULL);
 err_free_drm:
 	drm_dev_put(drm);
 
@@ -313,6 +314,7 @@ static void exynos_drm_unbind(struct device *dev)
 	drm_dev_unregister(drm);
 
 	drm_kms_helper_poll_fini(drm);
+	drm_atomic_helper_shutdown(drm);
 
 	component_unbind_all(drm->dev, drm);
 	drm_mode_config_cleanup(drm);
@@ -350,9 +352,18 @@ static int exynos_drm_platform_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static void exynos_drm_platform_shutdown(struct platform_device *pdev)
+{
+	struct drm_device *drm = platform_get_drvdata(pdev);
+
+	if (drm)
+		drm_atomic_helper_shutdown(drm);
+}
+
 static struct platform_driver exynos_drm_platform_driver = {
 	.probe	= exynos_drm_platform_probe,
 	.remove	= exynos_drm_platform_remove,
+	.shutdown = exynos_drm_platform_shutdown,
 	.driver	= {
 		.name	= "exynos-drm",
 		.pm	= &exynos_drm_pm_ops,
-- 
2.42.0.283.g2d96d420d3-goog


WARNING: multiple messages have this Message-ID (diff)
From: Douglas Anderson <dianders@chromium.org>
To: dri-devel@lists.freedesktop.org, Maxime Ripard <mripard@kernel.org>
Cc: Douglas Anderson <dianders@chromium.org>,
	airlied@gmail.com, alim.akhtar@samsung.com, daniel@ffwll.ch,
	inki.dae@samsung.com, krzysztof.kozlowski@linaro.org,
	kyungmin.park@samsung.com, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-samsung-soc@vger.kernel.org,
	sw0312.kim@samsung.com
Subject: [RFT PATCH 11/15] drm/exynos: Call drm_atomic_helper_shutdown() at shutdown/unbind time
Date: Fri,  1 Sep 2023 16:41:22 -0700	[thread overview]
Message-ID: <20230901164111.RFT.11.Iea33274908b6b258955f45a8aaf6f5bba24ad6cd@changeid> (raw)
In-Reply-To: <20230901234202.566951-1-dianders@chromium.org>

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.

A few notes about this fix:
- When adding drm_atomic_helper_shutdown() to the unbind path, I added
  it after drm_kms_helper_poll_fini() since that's when other drivers
  seemed to have it.
- Technically with a previous patch, ("drm/atomic-helper:
  drm_atomic_helper_shutdown(NULL) should be a noop"), we don't
  actually need to check to see if our "drm" pointer is NULL before
  calling drm_atomic_helper_shutdown(). We'll leave the "if" test in,
  though, so that this patch can land without any dependencies. It
  could potentially be removed later.
- This patch also makes sure to set the drvdata to NULL in the case of
  bind errors to make sure that shutdown can't access freed data.

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

 drivers/gpu/drm/exynos/exynos_drm_drv.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index 8399256cb5c9..5380fb6c55ae 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -300,6 +300,7 @@ static int exynos_drm_bind(struct device *dev)
 	drm_mode_config_cleanup(drm);
 	exynos_drm_cleanup_dma(drm);
 	kfree(private);
+	dev_set_drvdata(dev, NULL);
 err_free_drm:
 	drm_dev_put(drm);
 
@@ -313,6 +314,7 @@ static void exynos_drm_unbind(struct device *dev)
 	drm_dev_unregister(drm);
 
 	drm_kms_helper_poll_fini(drm);
+	drm_atomic_helper_shutdown(drm);
 
 	component_unbind_all(drm->dev, drm);
 	drm_mode_config_cleanup(drm);
@@ -350,9 +352,18 @@ static int exynos_drm_platform_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static void exynos_drm_platform_shutdown(struct platform_device *pdev)
+{
+	struct drm_device *drm = platform_get_drvdata(pdev);
+
+	if (drm)
+		drm_atomic_helper_shutdown(drm);
+}
+
 static struct platform_driver exynos_drm_platform_driver = {
 	.probe	= exynos_drm_platform_probe,
 	.remove	= exynos_drm_platform_remove,
+	.shutdown = exynos_drm_platform_shutdown,
 	.driver	= {
 		.name	= "exynos-drm",
 		.pm	= &exynos_drm_pm_ops,
-- 
2.42.0.283.g2d96d420d3-goog


WARNING: multiple messages have this Message-ID (diff)
From: Douglas Anderson <dianders@chromium.org>
To: dri-devel@lists.freedesktop.org, Maxime Ripard <mripard@kernel.org>
Cc: Douglas Anderson <dianders@chromium.org>,
	airlied@gmail.com, alim.akhtar@samsung.com, daniel@ffwll.ch,
	inki.dae@samsung.com, krzysztof.kozlowski@linaro.org,
	kyungmin.park@samsung.com, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-samsung-soc@vger.kernel.org,
	sw0312.kim@samsung.com
Subject: [RFT PATCH 11/15] drm/exynos: Call drm_atomic_helper_shutdown() at shutdown/unbind time
Date: Fri,  1 Sep 2023 16:41:22 -0700	[thread overview]
Message-ID: <20230901164111.RFT.11.Iea33274908b6b258955f45a8aaf6f5bba24ad6cd@changeid> (raw)
In-Reply-To: <20230901234202.566951-1-dianders@chromium.org>

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.

A few notes about this fix:
- When adding drm_atomic_helper_shutdown() to the unbind path, I added
  it after drm_kms_helper_poll_fini() since that's when other drivers
  seemed to have it.
- Technically with a previous patch, ("drm/atomic-helper:
  drm_atomic_helper_shutdown(NULL) should be a noop"), we don't
  actually need to check to see if our "drm" pointer is NULL before
  calling drm_atomic_helper_shutdown(). We'll leave the "if" test in,
  though, so that this patch can land without any dependencies. It
  could potentially be removed later.
- This patch also makes sure to set the drvdata to NULL in the case of
  bind errors to make sure that shutdown can't access freed data.

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

 drivers/gpu/drm/exynos/exynos_drm_drv.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index 8399256cb5c9..5380fb6c55ae 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -300,6 +300,7 @@ static int exynos_drm_bind(struct device *dev)
 	drm_mode_config_cleanup(drm);
 	exynos_drm_cleanup_dma(drm);
 	kfree(private);
+	dev_set_drvdata(dev, NULL);
 err_free_drm:
 	drm_dev_put(drm);
 
@@ -313,6 +314,7 @@ static void exynos_drm_unbind(struct device *dev)
 	drm_dev_unregister(drm);
 
 	drm_kms_helper_poll_fini(drm);
+	drm_atomic_helper_shutdown(drm);
 
 	component_unbind_all(drm->dev, drm);
 	drm_mode_config_cleanup(drm);
@@ -350,9 +352,18 @@ static int exynos_drm_platform_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static void exynos_drm_platform_shutdown(struct platform_device *pdev)
+{
+	struct drm_device *drm = platform_get_drvdata(pdev);
+
+	if (drm)
+		drm_atomic_helper_shutdown(drm);
+}
+
 static struct platform_driver exynos_drm_platform_driver = {
 	.probe	= exynos_drm_platform_probe,
 	.remove	= exynos_drm_platform_remove,
+	.shutdown = exynos_drm_platform_shutdown,
 	.driver	= {
 		.name	= "exynos-drm",
 		.pm	= &exynos_drm_pm_ops,
-- 
2.42.0.283.g2d96d420d3-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  parent reply	other threads:[~2023-09-01 23:42 UTC|newest]

Thread overview: 141+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-01 23:41 [Nouveau] [RFT PATCH 00/15] drm: non-drm-misc drivers call drm_atomic_helper_shutdown() at the right times Douglas Anderson
2023-09-01 23:41 ` Douglas Anderson
2023-09-01 23:41 ` Douglas Anderson
2023-09-01 23:41 ` Douglas Anderson
2023-09-01 23:41 ` [RFT PATCH 01/15] drm/armada: Call drm_atomic_helper_shutdown() at shutdown time Douglas Anderson
2023-09-01 23:41   ` Douglas Anderson
2023-09-03 15:53   ` Russell King (Oracle)
2023-09-03 15:53     ` Russell King (Oracle)
2023-09-04  7:36     ` Maxime Ripard
2023-09-04  7:36       ` Maxime Ripard
2023-09-04  7:55       ` Russell King (Oracle)
2023-09-04  7:55         ` Russell King (Oracle)
2023-09-04 15:18         ` Maxime Ripard
2023-09-04 15:18           ` Maxime Ripard
2023-09-05 14:23     ` Doug Anderson
2023-09-05 14:23       ` Doug Anderson
2023-09-13 15:34       ` Doug Anderson
2023-09-13 15:34         ` Doug Anderson
2023-09-20 18:03         ` Doug Anderson
2023-09-20 18:03           ` Doug Anderson
2023-09-20 18:58           ` Russell King (Oracle)
2023-09-20 18:58             ` Russell King (Oracle)
2023-09-21 18:46             ` Doug Anderson
2023-09-21 18:46               ` Doug Anderson
2023-09-04  7:53   ` Maxime Ripard
2023-09-04  7:53     ` Maxime Ripard
2023-09-04  7:56   ` Russell King (Oracle)
2023-09-04  7:56     ` Russell King (Oracle)
2023-09-01 23:41 ` [RFT PATCH 02/15] drm/imx/dcss: " Douglas Anderson
2023-09-01 23:41   ` Douglas Anderson
2023-09-01 23:41   ` Douglas Anderson
2023-09-04  7:36   ` Maxime Ripard
2023-09-04  7:36     ` Maxime Ripard
2023-09-04  7:36     ` Maxime Ripard
2023-09-01 23:41 ` [RFT PATCH 03/15] drm/ingenic: " Douglas Anderson
2023-09-01 23:41   ` Douglas Anderson
2023-09-04  7:36   ` Maxime Ripard
2023-09-04  7:36     ` Maxime Ripard
2023-09-04  9:15   ` Paul Cercueil
2023-09-04  9:15     ` Paul Cercueil
2023-09-05 20:16     ` Doug Anderson
2023-09-05 20:16       ` Doug Anderson
2023-09-06  8:39       ` Maxime Ripard
2023-09-06  8:39         ` Maxime Ripard
2023-09-13 16:23         ` Doug Anderson
2023-09-13 16:23           ` Doug Anderson
2023-09-14  8:14           ` Maxime Ripard
2023-09-14  8:14             ` Maxime Ripard
2023-09-14 22:29             ` Doug Anderson
2023-09-14 22:29               ` Doug Anderson
2023-09-19  9:33               ` Maxime Ripard
2023-09-19  9:33                 ` Maxime Ripard
2023-09-13 16:25       ` Doug Anderson
2023-09-13 16:25         ` Doug Anderson
2023-09-13 18:00         ` Paul Cercueil
2023-09-13 18:00           ` Paul Cercueil
2023-09-13 18:21   ` Doug Anderson
2023-09-13 18:21     ` Doug Anderson
2023-09-01 23:41 ` [RFT PATCH 04/15] drm/kmb: " Douglas Anderson
2023-09-01 23:41   ` Douglas Anderson
2023-09-04  7:26   ` Maxime Ripard
2023-09-04  7:26     ` Maxime Ripard
2023-09-01 23:41 ` [RFT PATCH 05/15] drm/mediatek: " Douglas Anderson
2023-09-01 23:41   ` Douglas Anderson
2023-09-01 23:41   ` Douglas Anderson
2023-09-04  7:27   ` Maxime Ripard
2023-09-04  7:27     ` Maxime Ripard
2023-09-04  7:27     ` Maxime Ripard
2023-09-08 11:51   ` Fei Shao
2023-09-08 11:51     ` Fei Shao
2023-09-08 11:51     ` Fei Shao
2023-09-11 16:10     ` Doug Anderson
2023-09-11 16:10       ` Doug Anderson
2023-09-11 16:10       ` Doug Anderson
2023-09-12  6:28       ` Fei Shao
2023-09-12  6:28         ` Fei Shao
2023-09-12  6:28         ` Fei Shao
2023-09-01 23:41 ` [Nouveau] [RFT PATCH 06/15] drm/nouveau: Call drm_atomic_helper_shutdown() or equiv " Douglas Anderson
2023-09-01 23:41   ` Douglas Anderson
2023-09-01 23:41   ` Douglas Anderson
2023-09-04  7:27   ` Maxime Ripard
2023-09-04  7:27     ` [Nouveau] " Maxime Ripard
2023-09-04  7:27     ` Maxime Ripard
2023-09-01 23:41 ` [RFT PATCH 07/15] drm/tegra: Call drm_atomic_helper_shutdown() " Douglas Anderson
2023-09-01 23:41   ` Douglas Anderson
2023-09-04  7:28   ` Maxime Ripard
2023-09-04  7:28     ` Maxime Ripard
2023-09-01 23:41 ` [RFT PATCH 08/15] drm/arcpgu: " Douglas Anderson
2023-09-01 23:41   ` Douglas Anderson
2023-09-04  7:28   ` Maxime Ripard
2023-09-04  7:28     ` Maxime Ripard
2023-09-01 23:41 ` [RFT PATCH 09/15] drm/amdgpu: " Douglas Anderson
2023-09-01 23:41   ` Douglas Anderson
2023-09-01 23:41   ` Douglas Anderson
2023-09-01 23:41 ` [RFT PATCH 10/15] drm/sprd: Call drm_atomic_helper_shutdown() at remove time Douglas Anderson
2023-09-01 23:41   ` Douglas Anderson
2023-09-04  7:54   ` Maxime Ripard
2023-09-04  7:54     ` Maxime Ripard
2023-09-01 23:41 ` Douglas Anderson [this message]
2023-09-01 23:41   ` [RFT PATCH 11/15] drm/exynos: Call drm_atomic_helper_shutdown() at shutdown/unbind time Douglas Anderson
2023-09-01 23:41   ` Douglas Anderson
2023-09-04  7:54   ` Maxime Ripard
2023-09-04  7:54     ` Maxime Ripard
2023-09-04  7:54     ` Maxime Ripard
2023-09-01 23:41 ` [RFT PATCH 12/15] drm/gma500: Call drm_helper_force_disable_all() at shutdown/remove time Douglas Anderson
2023-09-01 23:41   ` Douglas Anderson
2023-09-04  7:29   ` Maxime Ripard
2023-09-04  7:29     ` Maxime Ripard
2023-09-01 23:41 ` [RFT PATCH 13/15] drm/imx/ipuv3: Call drm_atomic_helper_shutdown() at shutdown/unbind time Douglas Anderson
2023-09-01 23:41   ` Douglas Anderson
2023-09-01 23:41   ` Douglas Anderson
2023-09-04  7:55   ` Maxime Ripard
2023-09-04  7:55     ` Maxime Ripard
2023-09-04  7:55     ` Maxime Ripard
2023-09-04  8:30   ` Philipp Zabel
2023-09-04  8:30     ` Philipp Zabel
2023-09-04  8:30     ` Philipp Zabel
2023-09-05 20:29     ` Doug Anderson
2023-09-05 20:29       ` Doug Anderson
2023-09-05 20:29       ` Doug Anderson
2023-09-06  5:47       ` Philipp Zabel
2023-09-06  5:47         ` Philipp Zabel
2023-09-06  5:47         ` Philipp Zabel
2023-09-06 14:30         ` Doug Anderson
2023-09-06 14:30           ` Doug Anderson
2023-09-06 14:30           ` Doug Anderson
2023-09-13 18:21   ` Doug Anderson
2023-09-13 18:21     ` Doug Anderson
2023-09-13 18:21     ` Doug Anderson
2023-09-01 23:41 ` [RFT PATCH 14/15] drm/radeon: Call drm_helper_force_disable_all() at shutdown/remove time Douglas Anderson
2023-09-01 23:41   ` Douglas Anderson
2023-09-01 23:41   ` Douglas Anderson
2023-09-04  7:30   ` Maxime Ripard
2023-09-04  7:30     ` Maxime Ripard
2023-09-04  7:30     ` Maxime Ripard
2023-09-01 23:41 ` [RFT PATCH 15/15] drm/renesas/shmobile: " Douglas Anderson
2023-09-01 23:41   ` Douglas Anderson
2023-09-04  7:27   ` Geert Uytterhoeven
2023-09-04  7:27     ` Geert Uytterhoeven
2023-09-05 21:10     ` Doug Anderson
2023-09-05 21:10       ` Doug Anderson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230901164111.RFT.11.Iea33274908b6b258955f45a8aaf6f5bba24ad6cd@changeid \
    --to=dianders@chromium.org \
    --cc=alim.akhtar@samsung.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=krzysztof.kozlowski@linaro.org \
    --cc=kyungmin.park@samsung.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=mripard@kernel.org \
    --cc=sw0312.kim@samsung.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.