This patch series came about after a _long_ discussion between me and
Maxime Ripard in response to a different patch I sent out [1]. As part
of that discussion, we realized that it would be good if DRM drivers
consistently called drm_atomic_helper_shutdown() properly at shutdown
and driver remove time as it's documented that they should do. The
eventual goal of this would be to enable removing some hacky code from
panel drivers where they had to hook into shutdown themselves because
the DRM driver wasn't calling them.
It turns out that quite a lot of drivers seemed to be missing
drm_atomic_helper_shutdown() in one or both places that it was
supposed to be. This patch series attempts to fix all the drivers that
I was able to identify.
NOTE: fixing this wasn't exactly cookie cutter. Each driver has its
own unique way of setting itself up and tearing itself down. Some
drivers also use the component model, which adds extra fun. I've made
my best guess at solving this and I've run a bunch of compile tests
(specifically, allmodconfig for amd64, arm64, and powerpc). That being
said, these code changes are not totally trivial and I've done zero
real testing on them. Making these patches was also a little mind
numbing and I'm certain my eyes glazed over at several points when
writing them. What I'm trying to say is to please double-check that I
didn't do anything too silly, like cast your driver's drvdata to the
wrong type. Even better, test these patches!
I've labeled this patch series as RFT (request for testing) to help
call attention to the fact that I didn't personally test any of these
patches.
I'd like to call out a few drivers that I _didn't_ fix in this series
and why. If any of these drivers should be fixed then please yell.
- DRM drivers backed by usb_driver (like gud, gm12u320, udl): I didn't
add the call to drm_atomic_helper_shutdown() at shutdown time
because there's no ".shutdown" callback for them USB drivers. Given
that USB is hotpluggable, I'm assuming that they are robust against
this and the special shutdown callback isn't needed.
- ofdrm and simpledrm: These didn't have drm_atomic_helper_shutdown()
in either shutdown or remove, but I didn't add it. I think that's OK
since they're sorta special and not really directly controlling
hardware power sequencing.
- virtio, vkms, vmwgfx, xen: I believe these are all virtual (thus
they wouldn't directly drive a panel) and adding the shutdown
didn't look straightforward, so I skipped them.
I've let each patch in the series get CCed straight from
get_maintainer. That means not everyone will have received every patch
but everyone should be on the cover letter. I know some people dislike
this but when touching this many drivers there's not much
choice. dri-devel and lkml have been CCed and lore/lei exist, so
hopefully that's enough for folks. I'm happy to add people to the
whole series for future posts.
NOTE: I landed everything I could from v1 of the patch series [2] [3]
to drm-misc. This v2 is everyone that is still left. If you'd like me
to land one of the patches here to drm-misc for you, please say
so. Otherwise I will assume maintainers will pick patches for their
particular driver and land them. There are no dependencies.
[1] https://lore.kernel.org/lkml/20230804140605.RFC.4.I930069a32baab6faf46d6b234f89613b5cec0f14@changeid
[2] https://lore.kernel.org/r/[email protected]
[3] https://lore.kernel.org/r/[email protected]
Changes in v2:
- Rebased and resolved conflicts.
Douglas Anderson (12):
drm/imx/dcss: Call drm_atomic_helper_shutdown() at shutdown time
drm/kmb: Call drm_atomic_helper_shutdown() at shutdown time
drm/mediatek: Call drm_atomic_helper_shutdown() at shutdown time
drm/nouveau: Call drm_atomic_helper_shutdown() or equiv at shutdown
time
drm/tegra: Call drm_atomic_helper_shutdown() at shutdown time
drm/arcpgu: Call drm_atomic_helper_shutdown() at shutdown time
drm/amdgpu: Call drm_atomic_helper_shutdown() at shutdown time
drm/sprd: Call drm_atomic_helper_shutdown() at remove time
drm/exynos: Call drm_atomic_helper_shutdown() at shutdown/unbind time
drm/gma500: Call drm_helper_force_disable_all() at shutdown/remove
time
drm/radeon: Call drm_helper_force_disable_all() at shutdown/remove
time
drm/renesas/shmobile: Call drm_helper_force_disable_all() at
shutdown/remove time
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 10 ++++++++++
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 2 ++
drivers/gpu/drm/exynos/exynos_drm_drv.c | 11 +++++++++++
drivers/gpu/drm/gma500/psb_drv.c | 8 ++++++++
drivers/gpu/drm/imx/dcss/dcss-drv.c | 8 ++++++++
drivers/gpu/drm/imx/dcss/dcss-kms.c | 7 +++++++
drivers/gpu/drm/imx/dcss/dcss-kms.h | 1 +
drivers/gpu/drm/kmb/kmb_drv.c | 6 ++++++
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 9 +++++++++
drivers/gpu/drm/nouveau/nouveau_display.c | 9 +++++++++
drivers/gpu/drm/nouveau/nouveau_display.h | 1 +
drivers/gpu/drm/nouveau/nouveau_drm.c | 13 +++++++++++++
drivers/gpu/drm/nouveau/nouveau_drv.h | 1 +
drivers/gpu/drm/nouveau/nouveau_platform.c | 6 ++++++
drivers/gpu/drm/radeon/radeon_drv.c | 7 ++++++-
drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c | 10 ++++++++++
drivers/gpu/drm/sprd/sprd_drm.c | 4 +++-
drivers/gpu/drm/tegra/drm.c | 6 ++++++
drivers/gpu/drm/tiny/arcpgu.c | 6 ++++++
20 files changed, 124 insertions(+), 2 deletions(-)
--
2.42.0.515.g380fc7ccd1-goog
Based on grepping through the source code, this driver appears to be
missing a call to drm_atomic_helper_shutdown(), or in this case the
non-atomic equivalent drm_helper_force_disable_all(), at system
shutdown time and at driver remove time. This is important because
drm_helper_force_disable_all() will cause panels to get disabled
cleanly which may be important for their power sequencing. Future
changes will remove any custom powering off in individual panel
drivers so the DRM drivers need to start getting this right.
The fact that we should call drm_atomic_helper_shutdown(), or in this
case the non-atomic equivalent drm_helper_force_disable_all(), in the
case of OS shutdown/restart comes straight out of the kernel doc
"driver instance overview" in drm_drv.c.
NOTE: in order to get things inserted in the right place, I had to
replace the old/deprecated drm_put_dev() function with the equivalent
new calls.
Suggested-by: Maxime Ripard <[email protected]>
Reviewed-by: Maxime Ripard <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
I honestly have no idea if I got this patch right. The shutdown()
function already had some special case logic for PPC, Loongson, and
VMs and I don't 100% for sure know how this interacts with
those. Everything here is just compile tested.
(no changes since v1)
drivers/gpu/drm/radeon/radeon_drv.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
index 39cdede460b5..67995ea24852 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -38,6 +38,7 @@
#include <linux/pci.h>
#include <drm/drm_aperture.h>
+#include <drm/drm_crtc_helper.h>
#include <drm/drm_drv.h>
#include <drm/drm_file.h>
#include <drm/drm_gem.h>
@@ -357,7 +358,9 @@ radeon_pci_remove(struct pci_dev *pdev)
{
struct drm_device *dev = pci_get_drvdata(pdev);
- drm_put_dev(dev);
+ drm_dev_unregister(dev);
+ drm_helper_force_disable_all(dev);
+ drm_dev_put(dev);
}
static void
@@ -368,6 +371,8 @@ radeon_pci_shutdown(struct pci_dev *pdev)
*/
if (radeon_device_is_virtual())
radeon_pci_remove(pdev);
+ else
+ drm_helper_force_disable_all(pci_get_drvdata(pdev));
#if defined(CONFIG_PPC64) || defined(CONFIG_MACH_LOONGSON64)
/*
--
2.42.0.515.g380fc7ccd1-goog
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.
This driver users the component model and shutdown happens in the base
driver. The "drvdata" for this driver will always be valid if
shutdown() is called and we know that if the "drm" pointer in our
private data is non-NULL then we need to call
drm_atomic_helper_shutdown(). 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.
Suggested-by: Maxime Ripard <[email protected]>
Reviewed-by: Maxime Ripard <[email protected]>
Reviewed-by: Fei Shao <[email protected]>
Tested-by: Fei Shao <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2:
- Rebased and resolved conflicts.
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index d16cc8219105..6bab360c0c1a 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -919,6 +919,14 @@ static void mtk_drm_remove(struct platform_device *pdev)
of_node_put(private->comp_node[i]);
}
+static void mtk_drm_shutdown(struct platform_device *pdev)
+{
+ struct mtk_drm_private *private = platform_get_drvdata(pdev);
+
+ if (private->drm)
+ drm_atomic_helper_shutdown(private->drm);
+}
+
static int mtk_drm_sys_prepare(struct device *dev)
{
struct mtk_drm_private *private = dev_get_drvdata(dev);
@@ -950,6 +958,7 @@ static const struct dev_pm_ops mtk_drm_pm_ops = {
static struct platform_driver mtk_drm_platform_driver = {
.probe = mtk_drm_probe,
.remove_new = mtk_drm_remove,
+ .shutdown = mtk_drm_shutdown,
.driver = {
.name = "mediatek-drm",
.pm = &mtk_drm_pm_ops,
--
2.42.0.515.g380fc7ccd1-goog
Based on grepping through the source code this driver appears to be
missing a call to drm_atomic_helper_shutdown() (or
drm_helper_force_disable_all() if not using atomic) 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.
Suggested-by: Maxime Ripard <[email protected]>
Reviewed-by: Maxime Ripard <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
This commit is only compile-time tested. I made my best guess about
how to fit this into the existing code. If someone wishes a different
style, please yell.
(no changes since v1)
drivers/gpu/drm/nouveau/nouveau_display.c | 9 +++++++++
drivers/gpu/drm/nouveau/nouveau_display.h | 1 +
drivers/gpu/drm/nouveau/nouveau_drm.c | 13 +++++++++++++
drivers/gpu/drm/nouveau/nouveau_drv.h | 1 +
drivers/gpu/drm/nouveau/nouveau_platform.c | 6 ++++++
5 files changed, 30 insertions(+)
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
index d8c92521226d..05c3688ccb76 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -642,6 +642,15 @@ nouveau_display_fini(struct drm_device *dev, bool suspend, bool runtime)
disp->fini(dev, runtime, suspend);
}
+void
+nouveau_display_shutdown(struct drm_device *dev)
+{
+ if (drm_drv_uses_atomic_modeset(dev))
+ drm_atomic_helper_shutdown(dev);
+ else
+ drm_helper_force_disable_all(dev);
+}
+
static void
nouveau_display_create_properties(struct drm_device *dev)
{
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.h b/drivers/gpu/drm/nouveau/nouveau_display.h
index 2ab2ddb1eadf..9df62e833cda 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.h
+++ b/drivers/gpu/drm/nouveau/nouveau_display.h
@@ -47,6 +47,7 @@ void nouveau_display_destroy(struct drm_device *dev);
int nouveau_display_init(struct drm_device *dev, bool resume, bool runtime);
void nouveau_display_hpd_resume(struct drm_device *dev);
void nouveau_display_fini(struct drm_device *dev, bool suspend, bool runtime);
+void nouveau_display_shutdown(struct drm_device *dev);
int nouveau_display_suspend(struct drm_device *dev, bool runtime);
void nouveau_display_resume(struct drm_device *dev, bool runtime);
int nouveau_display_vblank_enable(struct drm_crtc *crtc);
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
index 50589f982d1a..8ecfd66b7aab 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -879,6 +879,18 @@ nouveau_drm_remove(struct pci_dev *pdev)
pci_disable_device(pdev);
}
+void
+nouveau_drm_device_shutdown(struct drm_device *dev)
+{
+ nouveau_display_shutdown(dev);
+}
+
+static void
+nouveau_drm_shutdown(struct pci_dev *pdev)
+{
+ nouveau_drm_device_shutdown(pci_get_drvdata(pdev));
+}
+
static int
nouveau_do_suspend(struct drm_device *dev, bool runtime)
{
@@ -1346,6 +1358,7 @@ nouveau_drm_pci_driver = {
.id_table = nouveau_drm_pci_table,
.probe = nouveau_drm_probe,
.remove = nouveau_drm_remove,
+ .shutdown = nouveau_drm_shutdown,
.driver.pm = &nouveau_pm_ops,
};
diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.h b/drivers/gpu/drm/nouveau/nouveau_drv.h
index 3666a7403e47..aa936cabb6cf 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drv.h
+++ b/drivers/gpu/drm/nouveau/nouveau_drv.h
@@ -327,6 +327,7 @@ struct drm_device *
nouveau_platform_device_create(const struct nvkm_device_tegra_func *,
struct platform_device *, struct nvkm_device **);
void nouveau_drm_device_remove(struct drm_device *dev);
+void nouveau_drm_device_shutdown(struct drm_device *dev);
#define NV_PRINTK(l,c,f,a...) do { \
struct nouveau_cli *_cli = (c); \
diff --git a/drivers/gpu/drm/nouveau/nouveau_platform.c b/drivers/gpu/drm/nouveau/nouveau_platform.c
index 23cd43a7fd19..b2e82a96411c 100644
--- a/drivers/gpu/drm/nouveau/nouveau_platform.c
+++ b/drivers/gpu/drm/nouveau/nouveau_platform.c
@@ -50,6 +50,11 @@ static int nouveau_platform_remove(struct platform_device *pdev)
return 0;
}
+static void nouveau_platform_shutdown(struct platform_device *pdev)
+{
+ nouveau_drm_device_shutdown(platform_get_drvdata(pdev));
+}
+
#if IS_ENABLED(CONFIG_OF)
static const struct nvkm_device_tegra_func gk20a_platform_data = {
.iommu_bit = 34,
@@ -94,4 +99,5 @@ struct platform_driver nouveau_platform_driver = {
},
.probe = nouveau_platform_probe,
.remove = nouveau_platform_remove,
+ .shutdown = nouveau_platform_shutdown,
};
--
2.42.0.515.g380fc7ccd1-goog
Based on grepping through the source code, this driver appears to be
missing a call to drm_atomic_helper_shutdown(), or in this case the
non-atomic equivalent drm_helper_force_disable_all(), at system
shutdown time and at driver remove time. This is important because
drm_helper_force_disable_all() will cause panels to get disabled
cleanly which may be important for their power sequencing. Future
changes will remove any custom powering off in individual panel
drivers so the DRM drivers need to start getting this right.
The fact that we should call drm_atomic_helper_shutdown(), or in this
case the non-atomic equivalent drm_helper_force_disable_all(), in the
case of OS shutdown/restart comes straight out of the kernel doc
"driver instance overview" in drm_drv.c.
Suggested-by: Maxime Ripard <[email protected]>
Reviewed-by: Maxime Ripard <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
This commit is only compile-time tested.
(no changes since v1)
drivers/gpu/drm/gma500/psb_drv.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
index 8b64f61ffaf9..a5a399bbe8f5 100644
--- a/drivers/gpu/drm/gma500/psb_drv.c
+++ b/drivers/gpu/drm/gma500/psb_drv.c
@@ -20,6 +20,7 @@
#include <acpi/video.h>
#include <drm/drm.h>
+#include <drm/drm_crtc_helper.h>
#include <drm/drm_drv.h>
#include <drm/drm_file.h>
#include <drm/drm_ioctl.h>
@@ -485,6 +486,12 @@ static void psb_pci_remove(struct pci_dev *pdev)
struct drm_device *dev = pci_get_drvdata(pdev);
drm_dev_unregister(dev);
+ drm_helper_force_disable_all(dev);
+}
+
+static void psb_pci_shutdown(struct pci_dev *pdev)
+{
+ drm_helper_force_disable_all(pci_get_drvdata(pdev));
}
static DEFINE_RUNTIME_DEV_PM_OPS(psb_pm_ops, gma_power_suspend, gma_power_resume, NULL);
@@ -521,6 +528,7 @@ static struct pci_driver psb_pci_driver = {
.id_table = pciidlist,
.probe = psb_pci_probe,
.remove = psb_pci_remove,
+ .shutdown = psb_pci_shutdown,
.driver.pm = &psb_pm_ops,
};
--
2.42.0.515.g380fc7ccd1-goog
Based on grepping through the source code, this driver appears to be
missing a call to drm_atomic_helper_shutdown(), or in this case the
non-atomic equivalent drm_helper_force_disable_all(), at system
shutdown time and at driver remove time. This is important because
drm_helper_force_disable_all() will cause panels to get disabled
cleanly which may be important for their power sequencing. Future
changes will remove any custom powering off in individual panel
drivers so the DRM drivers need to start getting this right.
The fact that we should call drm_atomic_helper_shutdown(), or in this
case the non-atomic equivalent drm_helper_force_disable_all(), in the
case of OS shutdown/restart comes straight out of the kernel doc
"driver instance overview" in drm_drv.c.
Suggested-by: Maxime Ripard <[email protected]>
Cc: Geert Uytterhoeven <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
This commit is only compile-time tested.
As Geert pointed out in response to v1 [1], this patch conflicts with
the patches doing atomic conversion [2]. Since those patches don't
appear to be landed yet, I'm simply reposting v1. If those patches
land, I'm more than happy to re-post this one. I'm also more than
happy if someone wants to incorporate these changes into a different
patch.
[1] https://lore.kernel.org/r/CAMuHMdWOB7d-KE3F7aeZvVimNuy_U30uk=PND7=tWmPzCd7_eg@mail.gmail.com
[2] https://lore.kernel.org/dri-devel/fd7a6702490bd431f314d6591551bb39e77e3304.1692178020.git.geert+renesas@glider.be/
Changes in v2:
- Rebased and resolved conflicts.
drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c b/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c
index e5db4e0095ba..8c4c9d17a79e 100644
--- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c
+++ b/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c
@@ -15,6 +15,7 @@
#include <linux/pm.h>
#include <linux/slab.h>
+#include <drm/drm_crtc_helper.h>
#include <drm/drm_drv.h>
#include <drm/drm_fbdev_generic.h>
#include <drm/drm_gem_dma_helper.h>
@@ -179,10 +180,18 @@ static void shmob_drm_remove(struct platform_device *pdev)
drm_dev_unregister(ddev);
drm_kms_helper_poll_fini(ddev);
+ drm_helper_force_disable_all(ddev);
free_irq(sdev->irq, ddev);
drm_dev_put(ddev);
}
+static void shmob_drm_shutdown(struct platform_device *pdev)
+{
+ struct shmob_drm_device *sdev = platform_get_drvdata(pdev);
+
+ drm_helper_force_disable_all(sdev->ddev);
+}
+
static int shmob_drm_probe(struct platform_device *pdev)
{
struct shmob_drm_platform_data *pdata = pdev->dev.platform_data;
@@ -287,6 +296,7 @@ static int shmob_drm_probe(struct platform_device *pdev)
static struct platform_driver shmob_drm_platform_driver = {
.probe = shmob_drm_probe,
.remove_new = shmob_drm_remove,
+ .shutdown = shmob_drm_shutdown,
.driver = {
.name = "shmob-drm",
.pm = pm_sleep_ptr(&shmob_drm_pm_ops),
--
2.42.0.515.g380fc7ccd1-goog
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.
Suggested-by: Maxime Ripard <[email protected]>
Reviewed-by: Maxime Ripard <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
This commit is only compile-time tested.
(no changes since v1)
drivers/gpu/drm/imx/dcss/dcss-drv.c | 8 ++++++++
drivers/gpu/drm/imx/dcss/dcss-kms.c | 7 +++++++
drivers/gpu/drm/imx/dcss/dcss-kms.h | 1 +
3 files changed, 16 insertions(+)
diff --git a/drivers/gpu/drm/imx/dcss/dcss-drv.c b/drivers/gpu/drm/imx/dcss/dcss-drv.c
index c68b0d93ae9e..b61cec0cc79d 100644
--- a/drivers/gpu/drm/imx/dcss/dcss-drv.c
+++ b/drivers/gpu/drm/imx/dcss/dcss-drv.c
@@ -92,6 +92,13 @@ static int dcss_drv_platform_remove(struct platform_device *pdev)
return 0;
}
+static void dcss_drv_platform_shutdown(struct platform_device *pdev)
+{
+ struct dcss_drv *mdrv = dev_get_drvdata(&pdev->dev);
+
+ dcss_kms_shutdown(mdrv->kms);
+}
+
static struct dcss_type_data dcss_types[] = {
[DCSS_IMX8MQ] = {
.name = "DCSS_IMX8MQ",
@@ -114,6 +121,7 @@ MODULE_DEVICE_TABLE(of, dcss_of_match);
static struct platform_driver dcss_platform_driver = {
.probe = dcss_drv_platform_probe,
.remove = dcss_drv_platform_remove,
+ .shutdown = dcss_drv_platform_shutdown,
.driver = {
.name = "imx-dcss",
.of_match_table = dcss_of_match,
diff --git a/drivers/gpu/drm/imx/dcss/dcss-kms.c b/drivers/gpu/drm/imx/dcss/dcss-kms.c
index 896de946f8df..d0ea4e97cded 100644
--- a/drivers/gpu/drm/imx/dcss/dcss-kms.c
+++ b/drivers/gpu/drm/imx/dcss/dcss-kms.c
@@ -172,3 +172,10 @@ void dcss_kms_detach(struct dcss_kms_dev *kms)
dcss_crtc_deinit(&kms->crtc, drm);
drm->dev_private = NULL;
}
+
+void dcss_kms_shutdown(struct dcss_kms_dev *kms)
+{
+ struct drm_device *drm = &kms->base;
+
+ drm_atomic_helper_shutdown(drm);
+}
diff --git a/drivers/gpu/drm/imx/dcss/dcss-kms.h b/drivers/gpu/drm/imx/dcss/dcss-kms.h
index dfe5dd99eea3..62521c1fd6d2 100644
--- a/drivers/gpu/drm/imx/dcss/dcss-kms.h
+++ b/drivers/gpu/drm/imx/dcss/dcss-kms.h
@@ -34,6 +34,7 @@ struct dcss_kms_dev {
struct dcss_kms_dev *dcss_kms_attach(struct dcss_dev *dcss);
void dcss_kms_detach(struct dcss_kms_dev *kms);
+void dcss_kms_shutdown(struct dcss_kms_dev *kms);
int dcss_crtc_init(struct dcss_crtc *crtc, struct drm_device *drm);
void dcss_crtc_deinit(struct dcss_crtc *crtc, struct drm_device *drm);
struct dcss_plane *dcss_plane_init(struct drm_device *drm,
--
2.42.0.515.g380fc7ccd1-goog
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 <[email protected]>
Reviewed-by: Maxime Ripard <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
This commit is only compile-time tested.
(no changes since v1)
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.515.g380fc7ccd1-goog
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 <[email protected]>
Reviewed-by: Maxime Ripard <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
This commit is only compile-time tested.
(no changes since v1)
drivers/gpu/drm/sprd/sprd_drm.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
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 = {
--
2.42.0.515.g380fc7ccd1-goog
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.
Suggested-by: Maxime Ripard <[email protected]>
Reviewed-by: Maxime Ripard <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
This commit is only compile-time tested.
(no changes since v1)
drivers/gpu/drm/tegra/drm.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index ff36171c8fb7..ce2d4153f7bd 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -1312,6 +1312,11 @@ static int host1x_drm_remove(struct host1x_device *dev)
return 0;
}
+static void host1x_drm_shutdown(struct host1x_device *dev)
+{
+ drm_atomic_helper_shutdown(dev_get_drvdata(&dev->dev));
+}
+
#ifdef CONFIG_PM_SLEEP
static int host1x_drm_suspend(struct device *dev)
{
@@ -1380,6 +1385,7 @@ static struct host1x_driver host1x_drm_driver = {
},
.probe = host1x_drm_probe,
.remove = host1x_drm_remove,
+ .shutdown = host1x_drm_shutdown,
.subdevs = host1x_drm_subdevs,
};
--
2.42.0.515.g380fc7ccd1-goog
Hi,
On Thu, Sep 21, 2023 at 12:26:44PM -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 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.
>
> Suggested-by: Maxime Ripard <[email protected]>
> Reviewed-by: Maxime Ripard <[email protected]>
> Signed-off-by: Douglas Anderson <[email protected]>
No issues found on i.MX8MQ.
Tested-by: Laurentiu Palcu <[email protected]>
Reviewed-by: Laurentiu Palcu <[email protected]>
Thanks,
Laurentiu
> ---
> This commit is only compile-time tested.
>
> (no changes since v1)
>
> drivers/gpu/drm/imx/dcss/dcss-drv.c | 8 ++++++++
> drivers/gpu/drm/imx/dcss/dcss-kms.c | 7 +++++++
> drivers/gpu/drm/imx/dcss/dcss-kms.h | 1 +
> 3 files changed, 16 insertions(+)
>
> diff --git a/drivers/gpu/drm/imx/dcss/dcss-drv.c b/drivers/gpu/drm/imx/dcss/dcss-drv.c
> index c68b0d93ae9e..b61cec0cc79d 100644
> --- a/drivers/gpu/drm/imx/dcss/dcss-drv.c
> +++ b/drivers/gpu/drm/imx/dcss/dcss-drv.c
> @@ -92,6 +92,13 @@ static int dcss_drv_platform_remove(struct platform_device *pdev)
> return 0;
> }
>
> +static void dcss_drv_platform_shutdown(struct platform_device *pdev)
> +{
> + struct dcss_drv *mdrv = dev_get_drvdata(&pdev->dev);
> +
> + dcss_kms_shutdown(mdrv->kms);
> +}
> +
> static struct dcss_type_data dcss_types[] = {
> [DCSS_IMX8MQ] = {
> .name = "DCSS_IMX8MQ",
> @@ -114,6 +121,7 @@ MODULE_DEVICE_TABLE(of, dcss_of_match);
> static struct platform_driver dcss_platform_driver = {
> .probe = dcss_drv_platform_probe,
> .remove = dcss_drv_platform_remove,
> + .shutdown = dcss_drv_platform_shutdown,
> .driver = {
> .name = "imx-dcss",
> .of_match_table = dcss_of_match,
> diff --git a/drivers/gpu/drm/imx/dcss/dcss-kms.c b/drivers/gpu/drm/imx/dcss/dcss-kms.c
> index 896de946f8df..d0ea4e97cded 100644
> --- a/drivers/gpu/drm/imx/dcss/dcss-kms.c
> +++ b/drivers/gpu/drm/imx/dcss/dcss-kms.c
> @@ -172,3 +172,10 @@ void dcss_kms_detach(struct dcss_kms_dev *kms)
> dcss_crtc_deinit(&kms->crtc, drm);
> drm->dev_private = NULL;
> }
> +
> +void dcss_kms_shutdown(struct dcss_kms_dev *kms)
> +{
> + struct drm_device *drm = &kms->base;
> +
> + drm_atomic_helper_shutdown(drm);
> +}
> diff --git a/drivers/gpu/drm/imx/dcss/dcss-kms.h b/drivers/gpu/drm/imx/dcss/dcss-kms.h
> index dfe5dd99eea3..62521c1fd6d2 100644
> --- a/drivers/gpu/drm/imx/dcss/dcss-kms.h
> +++ b/drivers/gpu/drm/imx/dcss/dcss-kms.h
> @@ -34,6 +34,7 @@ struct dcss_kms_dev {
>
> struct dcss_kms_dev *dcss_kms_attach(struct dcss_dev *dcss);
> void dcss_kms_detach(struct dcss_kms_dev *kms);
> +void dcss_kms_shutdown(struct dcss_kms_dev *kms);
> int dcss_crtc_init(struct dcss_crtc *crtc, struct drm_device *drm);
> void dcss_crtc_deinit(struct dcss_crtc *crtc, struct drm_device *drm);
> struct dcss_plane *dcss_plane_init(struct drm_device *drm,
> --
> 2.42.0.515.g380fc7ccd1-goog
>
On 21.09.2023 21:26, Douglas Anderson wrote:
> 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 <[email protected]>
> Reviewed-by: Maxime Ripard <[email protected]>
> Signed-off-by: Douglas Anderson <[email protected]>
Seems to be working fine on all my test Exynos-based boards with display.
Tested-by: Marek Szyprowski <[email protected]>
Reviewed-by: Marek Szyprowski <[email protected]>
> ---
> This commit is only compile-time tested.
>
> (no changes since v1)
>
> 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,
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
Hi,
On Fri, Sep 22, 2023 at 12:56 AM Laurentiu Palcu
<[email protected]> wrote:
>
> Hi,
>
> On Thu, Sep 21, 2023 at 12:26:44PM -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 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.
> >
> > Suggested-by: Maxime Ripard <[email protected]>
> > Reviewed-by: Maxime Ripard <[email protected]>
> > Signed-off-by: Douglas Anderson <[email protected]>
>
> No issues found on i.MX8MQ.
>
> Tested-by: Laurentiu Palcu <[email protected]>
> Reviewed-by: Laurentiu Palcu <[email protected]>
Thanks! Would you expect this patch to land through drm-misc? If so,
I'm happy to commit it there with your tags. If patches to this driver
normally flow through drm-misc, I'm also happy to post a patch to
MAINTAINERS (or review a patch you post) adding this to the entry for
"NXP i.MX 8MQ DCSS DRIVER":
T: git git://anongit.freedesktop.org/drm/drm-misc
...which would make it obvious in the future that things should land
through drm-misc. This is similar to what I did for commit
92e62478b62c ("MAINTAINERS: Update DRM DRIVERS FOR FREESCALE IMX
entry"). :-)
-Doug
actually very glad to see this because I think I've seen one bug in the wild
as a result of things not getting shut down :)
Reviewed-by: Lyude Paul <[email protected]>
Tested-by: Lyude Paul <[email protected]>
On Thu, 2023-09-21 at 12:26 -0700, Douglas Anderson wrote:
> Based on grepping through the source code this driver appears to be
> missing a call to drm_atomic_helper_shutdown() (or
> drm_helper_force_disable_all() if not using atomic) 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.
>
> Suggested-by: Maxime Ripard <[email protected]>
> Reviewed-by: Maxime Ripard <[email protected]>
> Signed-off-by: Douglas Anderson <[email protected]>
> ---
> This commit is only compile-time tested. I made my best guess about
> how to fit this into the existing code. If someone wishes a different
> style, please yell.
>
> (no changes since v1)
>
> drivers/gpu/drm/nouveau/nouveau_display.c | 9 +++++++++
> drivers/gpu/drm/nouveau/nouveau_display.h | 1 +
> drivers/gpu/drm/nouveau/nouveau_drm.c | 13 +++++++++++++
> drivers/gpu/drm/nouveau/nouveau_drv.h | 1 +
> drivers/gpu/drm/nouveau/nouveau_platform.c | 6 ++++++
> 5 files changed, 30 insertions(+)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
> index d8c92521226d..05c3688ccb76 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_display.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_display.c
> @@ -642,6 +642,15 @@ nouveau_display_fini(struct drm_device *dev, bool suspend, bool runtime)
> disp->fini(dev, runtime, suspend);
> }
>
> +void
> +nouveau_display_shutdown(struct drm_device *dev)
> +{
> + if (drm_drv_uses_atomic_modeset(dev))
> + drm_atomic_helper_shutdown(dev);
> + else
> + drm_helper_force_disable_all(dev);
> +}
> +
> static void
> nouveau_display_create_properties(struct drm_device *dev)
> {
> diff --git a/drivers/gpu/drm/nouveau/nouveau_display.h b/drivers/gpu/drm/nouveau/nouveau_display.h
> index 2ab2ddb1eadf..9df62e833cda 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_display.h
> +++ b/drivers/gpu/drm/nouveau/nouveau_display.h
> @@ -47,6 +47,7 @@ void nouveau_display_destroy(struct drm_device *dev);
> int nouveau_display_init(struct drm_device *dev, bool resume, bool runtime);
> void nouveau_display_hpd_resume(struct drm_device *dev);
> void nouveau_display_fini(struct drm_device *dev, bool suspend, bool runtime);
> +void nouveau_display_shutdown(struct drm_device *dev);
> int nouveau_display_suspend(struct drm_device *dev, bool runtime);
> void nouveau_display_resume(struct drm_device *dev, bool runtime);
> int nouveau_display_vblank_enable(struct drm_crtc *crtc);
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
> index 50589f982d1a..8ecfd66b7aab 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> @@ -879,6 +879,18 @@ nouveau_drm_remove(struct pci_dev *pdev)
> pci_disable_device(pdev);
> }
>
> +void
> +nouveau_drm_device_shutdown(struct drm_device *dev)
> +{
> + nouveau_display_shutdown(dev);
> +}
> +
> +static void
> +nouveau_drm_shutdown(struct pci_dev *pdev)
> +{
> + nouveau_drm_device_shutdown(pci_get_drvdata(pdev));
> +}
> +
> static int
> nouveau_do_suspend(struct drm_device *dev, bool runtime)
> {
> @@ -1346,6 +1358,7 @@ nouveau_drm_pci_driver = {
> .id_table = nouveau_drm_pci_table,
> .probe = nouveau_drm_probe,
> .remove = nouveau_drm_remove,
> + .shutdown = nouveau_drm_shutdown,
> .driver.pm = &nouveau_pm_ops,
> };
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.h b/drivers/gpu/drm/nouveau/nouveau_drv.h
> index 3666a7403e47..aa936cabb6cf 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drv.h
> +++ b/drivers/gpu/drm/nouveau/nouveau_drv.h
> @@ -327,6 +327,7 @@ struct drm_device *
> nouveau_platform_device_create(const struct nvkm_device_tegra_func *,
> struct platform_device *, struct nvkm_device **);
> void nouveau_drm_device_remove(struct drm_device *dev);
> +void nouveau_drm_device_shutdown(struct drm_device *dev);
>
> #define NV_PRINTK(l,c,f,a...) do { \
> struct nouveau_cli *_cli = (c); \
> diff --git a/drivers/gpu/drm/nouveau/nouveau_platform.c b/drivers/gpu/drm/nouveau/nouveau_platform.c
> index 23cd43a7fd19..b2e82a96411c 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_platform.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_platform.c
> @@ -50,6 +50,11 @@ static int nouveau_platform_remove(struct platform_device *pdev)
> return 0;
> }
>
> +static void nouveau_platform_shutdown(struct platform_device *pdev)
> +{
> + nouveau_drm_device_shutdown(platform_get_drvdata(pdev));
> +}
> +
> #if IS_ENABLED(CONFIG_OF)
> static const struct nvkm_device_tegra_func gk20a_platform_data = {
> .iommu_bit = 34,
> @@ -94,4 +99,5 @@ struct platform_driver nouveau_platform_driver = {
> },
> .probe = nouveau_platform_probe,
> .remove = nouveau_platform_remove,
> + .shutdown = nouveau_platform_shutdown,
> };
--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat
Hi Doug,
On Fri, Sep 22, 2023 at 08:44:16AM -0700, Doug Anderson wrote:
> Hi,
>
> On Fri, Sep 22, 2023 at 12:56 AM Laurentiu Palcu
> <[email protected]> wrote:
> >
> > Hi,
> >
> > On Thu, Sep 21, 2023 at 12:26:44PM -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 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.
> > >
> > > Suggested-by: Maxime Ripard <[email protected]>
> > > Reviewed-by: Maxime Ripard <[email protected]>
> > > Signed-off-by: Douglas Anderson <[email protected]>
> >
> > No issues found on i.MX8MQ.
> >
> > Tested-by: Laurentiu Palcu <[email protected]>
> > Reviewed-by: Laurentiu Palcu <[email protected]>
>
> Thanks! Would you expect this patch to land through drm-misc? If so,
> I'm happy to commit it there with your tags.
Yes, please do. The i.MX8MQ DCSS patches go through drm-misc.
> If patches to this driver normally flow through drm-misc, I'm also
> happy to post a patch to MAINTAINERS (or review a patch you post)
> adding this to the entry for "NXP i.MX 8MQ DCSS DRIVER":
>
> T: git git://anongit.freedesktop.org/drm/drm-misc
>
> ...which would make it obvious in the future that things should land
> through drm-misc.
Thanks, that sounds good.
Laurentiu
[Public]
> -----Original Message-----
> From: Douglas Anderson <[email protected]>
> Sent: Thursday, September 21, 2023 3:27 PM
> To: [email protected]; Maxime Ripard <[email protected]>
> Cc: Douglas Anderson <[email protected]>; Pan, Xinhui
> <[email protected]>; [email protected]; Deucher, Alexander
> <[email protected]>; [email protected]; Koenig,
> Christian <[email protected]>; [email protected]; linux-
> [email protected]
> Subject: [RFT PATCH v2 11/12] drm/radeon: Call
> drm_helper_force_disable_all() at shutdown/remove time
>
> Based on grepping through the source code, this driver appears to be missing
> a call to drm_atomic_helper_shutdown(), or in this case the non-atomic
> equivalent drm_helper_force_disable_all(), at system shutdown time and at
> driver remove time. This is important because
> drm_helper_force_disable_all() will cause panels to get disabled cleanly which
> may be important for their power sequencing. Future changes will remove any
> custom powering off in individual panel drivers so the DRM drivers need to
> start getting this right.
>
> The fact that we should call drm_atomic_helper_shutdown(), or in this case
> the non-atomic equivalent drm_helper_force_disable_all(), in the case of OS
> shutdown/restart comes straight out of the kernel doc "driver instance
> overview" in drm_drv.c.
>
> NOTE: in order to get things inserted in the right place, I had to replace the
> old/deprecated drm_put_dev() function with the equivalent new calls.
>
> Suggested-by: Maxime Ripard <[email protected]>
> Reviewed-by: Maxime Ripard <[email protected]>
> Signed-off-by: Douglas Anderson <[email protected]>
> ---
> I honestly have no idea if I got this patch right. The shutdown() function
> already had some special case logic for PPC, Loongson, and VMs and I don't
> 100% for sure know how this interacts with those. Everything here is just
> compile tested.
I think the reason for most of this funniness is to reduce shutdown time. Lots of users complain if driver takes a while to shutdown and there is a point to be made that if the system is going into power down, there is not much reason to spend a lot of time messing with the hardware.
Alex
>
> (no changes since v1)
>
> drivers/gpu/drm/radeon/radeon_drv.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c
> b/drivers/gpu/drm/radeon/radeon_drv.c
> index 39cdede460b5..67995ea24852 100644
> --- a/drivers/gpu/drm/radeon/radeon_drv.c
> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> @@ -38,6 +38,7 @@
> #include <linux/pci.h>
>
> #include <drm/drm_aperture.h>
> +#include <drm/drm_crtc_helper.h>
> #include <drm/drm_drv.h>
> #include <drm/drm_file.h>
> #include <drm/drm_gem.h>
> @@ -357,7 +358,9 @@ radeon_pci_remove(struct pci_dev *pdev) {
> struct drm_device *dev = pci_get_drvdata(pdev);
>
> - drm_put_dev(dev);
> + drm_dev_unregister(dev);
> + drm_helper_force_disable_all(dev);
> + drm_dev_put(dev);
> }
>
> static void
> @@ -368,6 +371,8 @@ radeon_pci_shutdown(struct pci_dev *pdev)
> */
> if (radeon_device_is_virtual())
> radeon_pci_remove(pdev);
> + else
> + drm_helper_force_disable_all(pci_get_drvdata(pdev));
>
> #if defined(CONFIG_PPC64) || defined(CONFIG_MACH_LOONGSON64)
> /*
> --
> 2.42.0.515.g380fc7ccd1-goog
Hi,
On Mon, Sep 25, 2023 at 8:49 AM Deucher, Alexander
<[email protected]> wrote:
>
> [Public]
>
> > -----Original Message-----
> > From: Douglas Anderson <[email protected]>
> > Sent: Thursday, September 21, 2023 3:27 PM
> > To: [email protected]; Maxime Ripard <[email protected]>
> > Cc: Douglas Anderson <[email protected]>; Pan, Xinhui
> > <[email protected]>; [email protected]; Deucher, Alexander
> > <[email protected]>; [email protected]; Koenig,
> > Christian <[email protected]>; [email protected]; linux-
> > [email protected]
> > Subject: [RFT PATCH v2 11/12] drm/radeon: Call
> > drm_helper_force_disable_all() at shutdown/remove time
> >
> > Based on grepping through the source code, this driver appears to be missing
> > a call to drm_atomic_helper_shutdown(), or in this case the non-atomic
> > equivalent drm_helper_force_disable_all(), at system shutdown time and at
> > driver remove time. This is important because
> > drm_helper_force_disable_all() will cause panels to get disabled cleanly which
> > may be important for their power sequencing. Future changes will remove any
> > custom powering off in individual panel drivers so the DRM drivers need to
> > start getting this right.
> >
> > The fact that we should call drm_atomic_helper_shutdown(), or in this case
> > the non-atomic equivalent drm_helper_force_disable_all(), in the case of OS
> > shutdown/restart comes straight out of the kernel doc "driver instance
> > overview" in drm_drv.c.
> >
> > NOTE: in order to get things inserted in the right place, I had to replace the
> > old/deprecated drm_put_dev() function with the equivalent new calls.
> >
> > Suggested-by: Maxime Ripard <[email protected]>
> > Reviewed-by: Maxime Ripard <[email protected]>
> > Signed-off-by: Douglas Anderson <[email protected]>
> > ---
> > I honestly have no idea if I got this patch right. The shutdown() function
> > already had some special case logic for PPC, Loongson, and VMs and I don't
> > 100% for sure know how this interacts with those. Everything here is just
> > compile tested.
>
> I think the reason for most of this funniness is to reduce shutdown time. Lots of users complain if driver takes a while to shutdown and there is a point to be made that if the system is going into power down, there is not much reason to spend a lot of time messing with the hardware.
Sure, you don't want to do too much at shutdown time. That's the whole
reason that "shutdown" doesn't do a full remove / uninitialization of
all drivers. ...but drm_atomic_helper_shutdown() is documented to do
the things that are important for shutdown. Specifically, it cleanly
disables all of the displays. Depending on the display, this could
avoid temporary garbage on the display at reboot time or it could even
be important for the long term health of the panel.
-Doug
Hi,
On Sun, Sep 24, 2023 at 10:47 PM Laurentiu Palcu
<[email protected]> wrote:
>
> Hi Doug,
>
> On Fri, Sep 22, 2023 at 08:44:16AM -0700, Doug Anderson wrote:
> > Hi,
> >
> > On Fri, Sep 22, 2023 at 12:56 AM Laurentiu Palcu
> > <[email protected]> wrote:
> > >
> > > Hi,
> > >
> > > On Thu, Sep 21, 2023 at 12:26:44PM -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 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.
> > > >
> > > > Suggested-by: Maxime Ripard <[email protected]>
> > > > Reviewed-by: Maxime Ripard <[email protected]>
> > > > Signed-off-by: Douglas Anderson <[email protected]>
> > >
> > > No issues found on i.MX8MQ.
> > >
> > > Tested-by: Laurentiu Palcu <[email protected]>
> > > Reviewed-by: Laurentiu Palcu <[email protected]>
> >
> > Thanks! Would you expect this patch to land through drm-misc? If so,
> > I'm happy to commit it there with your tags.
>
> Yes, please do. The i.MX8MQ DCSS patches go through drm-misc.
OK, landed in drm-misc-next:
89755ee1d593 drm/imx/dcss: Call drm_atomic_helper_shutdown() at shutdown time
> > If patches to this driver normally flow through drm-misc, I'm also
> > happy to post a patch to MAINTAINERS (or review a patch you post)
> > adding this to the entry for "NXP i.MX 8MQ DCSS DRIVER":
> >
> > T: git git://anongit.freedesktop.org/drm/drm-misc
> >
> > ...which would make it obvious in the future that things should land
> > through drm-misc.
>
> Thanks, that sounds good.
https://lore.kernel.org/r/20230925154929.1.I3287e895ce8e68d41b458494a49a1b5ec5c71013@changeid
Thanks for testing. :)
Acked-by : Inki Dae <[email protected]>
2023년 9월 22일 (금) 오후 3:00, Marek Szyprowski <[email protected]>님이 작성:
>
>
> On 21.09.2023 21:26, Douglas Anderson wrote:
> > 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 <[email protected]>
> > Reviewed-by: Maxime Ripard <[email protected]>
> > Signed-off-by: Douglas Anderson <[email protected]>
>
> Seems to be working fine on all my test Exynos-based boards with display.
>
> Tested-by: Marek Szyprowski <[email protected]>
>
> Reviewed-by: Marek Szyprowski <[email protected]>
>
> > ---
> > This commit is only compile-time tested.
> >
> > (no changes since v1)
> >
> > 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,
>
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
>
Hi,
On Thu, Oct 5, 2023 at 7:20 PM Inki Dae <[email protected]> wrote:
>
> Thanks for testing. :)
>
> Acked-by : Inki Dae <[email protected]>
Inki: does that mean you'd like this to go through drm-misc? I'm happy
to do that, but there are no dependencies here so this could easily go
through your tree.
> 2023년 9월 22일 (금) 오후 3:00, Marek Szyprowski <[email protected]>님이 작성:
> >
> >
> > On 21.09.2023 21:26, Douglas Anderson wrote:
> > > 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 <[email protected]>
> > > Reviewed-by: Maxime Ripard <[email protected]>
> > > Signed-off-by: Douglas Anderson <[email protected]>
> >
> > Seems to be working fine on all my test Exynos-based boards with display.
> >
> > Tested-by: Marek Szyprowski <[email protected]>
> >
> > Reviewed-by: Marek Szyprowski <[email protected]>
> >
> > > ---
> > > This commit is only compile-time tested.
> > >
> > > (no changes since v1)
> > >
> > > 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,
> >
> > Best regards
> > --
> > Marek Szyprowski, PhD
> > Samsung R&D Institute Poland
> >
Hi,
2023년 10월 6일 (금) 오후 10:51, Doug Anderson <[email protected]>님이 작성:
>
> Hi,
>
> On Thu, Oct 5, 2023 at 7:20 PM Inki Dae <[email protected]> wrote:
> >
> > Thanks for testing. :)
> >
> > Acked-by : Inki Dae <[email protected]>
>
> Inki: does that mean you'd like this to go through drm-misc? I'm happy
> to do that, but there are no dependencies here so this could easily go
> through your tree.
Ah, you are right. No dependency here. I will pick it up.
Thanks,
Inki Dae
>
>
> > 2023년 9월 22일 (금) 오후 3:00, Marek Szyprowski <[email protected]>님이 작성:
> > >
> > >
> > > On 21.09.2023 21:26, Douglas Anderson wrote:
> > > > 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 <[email protected]>
> > > > Reviewed-by: Maxime Ripard <[email protected]>
> > > > Signed-off-by: Douglas Anderson <[email protected]>
> > >
> > > Seems to be working fine on all my test Exynos-based boards with display.
> > >
> > > Tested-by: Marek Szyprowski <[email protected]>
> > >
> > > Reviewed-by: Marek Szyprowski <[email protected]>
> > >
> > > > ---
> > > > This commit is only compile-time tested.
> > > >
> > > > (no changes since v1)
> > > >
> > > > 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,
> > >
> > > Best regards
> > > --
> > > Marek Szyprowski, PhD
> > > Samsung R&D Institute Poland
> > >
Hi,
On Fri, Sep 22, 2023 at 2:06 PM Lyude Paul <[email protected]> wrote:
>
> actually very glad to see this because I think I've seen one bug in the wild
> as a result of things not getting shut down :)
>
> Reviewed-by: Lyude Paul <[email protected]>
> Tested-by: Lyude Paul <[email protected]>
Any idea of where / how this patch should land. Would you expect me to
land it through drm-misc, or would you expect it to go through someone
else's tree?
Hi,
On Fri, Nov 17, 2023 at 3:00 PM Doug Anderson <[email protected]> wrote:
>
> Hi,
>
> On Fri, Sep 22, 2023 at 2:06 PM Lyude Paul <[email protected]> wrote:
> >
> > actually very glad to see this because I think I've seen one bug in the wild
> > as a result of things not getting shut down :)
> >
> > Reviewed-by: Lyude Paul <[email protected]>
> > Tested-by: Lyude Paul <[email protected]>
>
> Any idea of where / how this patch should land. Would you expect me to
> land it through drm-misc, or would you expect it to go through someone
> else's tree?
Still hoping to find a way to land this patch, since it's been
reviewed and tested. Would anyone object if I landed it via drm-misc?
-Doug
On Tue, Dec 05, 2023 at 12:45:07PM -0800, Doug Anderson wrote:
> Hi,
>
> On Fri, Nov 17, 2023 at 3:00 PM Doug Anderson <[email protected]> wrote:
> >
> > Hi,
> >
> > On Fri, Sep 22, 2023 at 2:06 PM Lyude Paul <[email protected]> wrote:
> > >
> > > actually very glad to see this because I think I've seen one bug in the wild
> > > as a result of things not getting shut down :)
> > >
> > > Reviewed-by: Lyude Paul <[email protected]>
> > > Tested-by: Lyude Paul <[email protected]>
> >
> > Any idea of where / how this patch should land. Would you expect me to
> > land it through drm-misc, or would you expect it to go through someone
> > else's tree?
>
> Still hoping to find a way to land this patch, since it's been
> reviewed and tested. Would anyone object if I landed it via drm-misc?
Nouveau isn't maintained in drm-misc, so I would expect it to go through
the usual nouveau tree. Lyude, Karol, Danilo?
Maxime