[Public]
> -Original Message-
> From: Douglas Anderson
> Sent: Thursday, September 21, 2023 3:27 PM
> To: dri-devel@lists.freedesktop.org; Maxime Ripard
> Cc: Douglas Anderson ; Pan, Xinhui
> ; airl...@gmail.com; Deucher, Alexander
> ; amd-...@lists.freedesktop.org; Koenig,
> Christian ; dan...@ffwll.ch; linux-
> ker...@vger.kernel.org
> 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
> Reviewed-by: Maxime Ripard
> Signed-off-by: Douglas Anderson
> ---
> 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
>
> #include
> +#include
> #include
> #include
> #include
> @@ -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