[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <BL1PR12MB51446279B27EC69BE91D865EF7FCA@BL1PR12MB5144.namprd12.prod.outlook.com>
Date: Mon, 25 Sep 2023 15:49:01 +0000
From: "Deucher, Alexander" <Alexander.Deucher@....com>
To: Douglas Anderson <dianders@...omium.org>,
"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
Maxime Ripard <mripard@...nel.org>
CC: "Pan, Xinhui" <Xinhui.Pan@....com>,
"airlied@...il.com" <airlied@...il.com>,
"amd-gfx@...ts.freedesktop.org" <amd-gfx@...ts.freedesktop.org>,
"Koenig, Christian" <Christian.Koenig@....com>,
"daniel@...ll.ch" <daniel@...ll.ch>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [RFT PATCH v2 11/12] drm/radeon: Call
drm_helper_force_disable_all() at shutdown/remove time
[Public]
> -----Original Message-----
> From: Douglas Anderson <dianders@...omium.org>
> Sent: Thursday, September 21, 2023 3:27 PM
> To: dri-devel@...ts.freedesktop.org; Maxime Ripard <mripard@...nel.org>
> Cc: Douglas Anderson <dianders@...omium.org>; Pan, Xinhui
> <Xinhui.Pan@....com>; airlied@...il.com; Deucher, Alexander
> <Alexander.Deucher@....com>; amd-gfx@...ts.freedesktop.org; Koenig,
> Christian <Christian.Koenig@....com>; daniel@...ll.ch; linux-
> kernel@...r.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 <mripard@...nel.org>
> Reviewed-by: Maxime Ripard <mripard@...nel.org>
> Signed-off-by: Douglas Anderson <dianders@...omium.org>
> ---
> 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
Powered by blists - more mailing lists