[PATCH] drm/amdgpu: Avoid leaking PM domain on driver unbind

[PATCH] drm/amdgpu: Avoid leaking PM domain on driver unbind

[Lukas Wunner]

amdgpu_device_init() calls vga_switcheroo_init_domain_pm_ops() either
if the device has the PowerXpress flag set or if the user has set the
"runpm" module param to 1.

However amdgpu_device_fini() calls vga_switcheroo_fini_domain_pm_ops()
only under the first of those two conditions.

(Note this issue isn't present in radeon.  It was introduced when the
driver was forked.)

Cc: stable@vger.kernel.org
Cc: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index d09c4ee9f7e1..f07eedc53761 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2083,7 +2083,7 @@ void amdgpu_device_fini(struct amdgpu_device *adev)
 	adev->bios = NULL;
 	if (!pci_is_thunderbolt_attached(adev->pdev))
 		vga_switcheroo_unregister_client(adev->pdev);
-	if (adev->flags & AMD_IS_PX)
+	if (adev->flags & AMD_IS_PX || amdgpu_runtime_pm == 1)
 		vga_switcheroo_fini_domain_pm_ops(adev->dev);
 	vga_client_register(adev->pdev, NULL, NULL, NULL);
 	if (adev->rio_mem)
-- 
2.15.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: Avoid leaking PM domain on driver unbind

[Alex Deucher]

[-- Attachment #1: Type: text/plain, Size: 1873 bytes --]

On Sun, Jan 21, 2018 at 7:46 AM, Lukas Wunner <lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org> wrote:
> amdgpu_device_init() calls vga_switcheroo_init_domain_pm_ops() either
> if the device has the PowerXpress flag set or if the user has set the
> "runpm" module param to 1.
>
> However amdgpu_device_fini() calls vga_switcheroo_fini_domain_pm_ops()
> only under the first of those two conditions.

Good catch.  Forcing runpm=1 doesn't do anything useful anyway so just
remove it in device_init().  See the attached patch.

Alex


>
> (Note this issue isn't present in radeon.  It was introduced when the
> driver was forked.)
>
> Cc: stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: Alex Deucher <alexander.deucher-5C7GfCeVMHo@public.gmane.org>
> Signed-off-by: Lukas Wunner <lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index d09c4ee9f7e1..f07eedc53761 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -2083,7 +2083,7 @@ void amdgpu_device_fini(struct amdgpu_device *adev)
>         adev->bios = NULL;
>         if (!pci_is_thunderbolt_attached(adev->pdev))
>                 vga_switcheroo_unregister_client(adev->pdev);
> -       if (adev->flags & AMD_IS_PX)
> +       if (adev->flags & AMD_IS_PX || amdgpu_runtime_pm == 1)
>                 vga_switcheroo_fini_domain_pm_ops(adev->dev);
>         vga_client_register(adev->pdev, NULL, NULL, NULL);
>         if (adev->rio_mem)
> --
> 2.15.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[-- Attachment #2: 0001-drm-amdgpu-Avoid-leaking-PM-domain-on-driver-unbind-.patch --]
[-- Type: text/x-patch, Size: 1385 bytes --]

From 420a1a3e7c1c12b63cd93d306b8ce80750d9bc8e Mon Sep 17 00:00:00 2001
From: Alex Deucher <alexander.deucher@amd.com>
Date: Mon, 22 Jan 2018 23:13:32 -0500
Subject: [PATCH] drm/amdgpu: Avoid leaking PM domain on driver unbind (v2)

We only support vga_switcheroo and runtime pm on PX/HG systems
so forcing runpm to 1 doesn't do anything useful anyway.

Only call vga_switcheroo_init_domain_pm_ops() for PX/HG so
that the cleanup path is correct as well.  This mirrors what
radeon does as well.

v2: rework the patch originally sent by Lukas (Alex)

Reported-by: Lukas Wunner <lukas@wunner.de>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Lukas Wunner <lukas@wunner.de> (v1)
Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 02a9157e72ac..7198eb9734ed 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1877,8 +1877,6 @@ int amdgpu_device_init(struct amdgpu_device *adev,
 	 * ignore it */
 	vga_client_register(adev->pdev, adev, NULL, amdgpu_device_vga_set_decode);
 
-	if (amdgpu_runtime_pm == 1)
-		runtime = true;
 	if (amdgpu_device_is_px(ddev))
 		runtime = true;
 	if (!pci_is_thunderbolt_attached(adev->pdev))
-- 
2.13.6


[-- Attachment #3: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: Avoid leaking PM domain on driver unbind

[Lukas Wunner]

On Mon, Jan 22, 2018 at 11:22:58PM -0500, Alex Deucher wrote:
> On Sun, Jan 21, 2018 at 7:46 AM, Lukas Wunner <lukas@wunner.de> wrote:
> > amdgpu_device_init() calls vga_switcheroo_init_domain_pm_ops() either
> > if the device has the PowerXpress flag set or if the user has set the
> > "runpm" module param to 1.
> >
> > However amdgpu_device_fini() calls vga_switcheroo_fini_domain_pm_ops()
> > only under the first of those two conditions.
> 
> Good catch.  Forcing runpm=1 doesn't do anything useful anyway so just
> remove it in device_init().  See the attached patch.

Ok, ack.

Lukas
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx