[PATCH v2 00/15] Runtime pm ref leak bonanza

[PATCH v2 00/15] Runtime pm ref leak bonanza

[Lukas Wunner]

Second iteration of my endeavour to rid nouveau, radeon and amdgpu of
runtime pm ref leaks.

Patches 1 to 8 are identical to v1.

Patch 9 of v1 modified the DRM core to turn off all CRTCs on driver
unload. Based on feedback by Daniel Vetter, I've replaced this with
a helper to turn off all CRTCs, which is called by nouveau, radeon
and amdgpu on unload. In other words, this is now opt-in.
So patch 9 of v1 is replaced with new patches 9 to 12.

A by-product of patch 9 is a helper which turns off a *single* CRTC.
This is open coded in three other places in the DRM tree and patches
13 to 15 refactor those to use the new helper.

To ease reviewing, I've pushed this series to GitHub:
https://github.com/l1k/linux/commits/drm_runpm_fixes_v2

The discussion on v1 is archived here:
https://lists.freedesktop.org/archives/dri-devel/2016-May/thread.html#108278

Thanks,

Lukas

Lukas Wunner (15):
  drm/nouveau: Don't leak runtime pm ref on driver unload
  drm/nouveau: Forbid runtime pm on driver unload
  drm/radeon: Don't leak runtime pm ref on driver unload
  drm/radeon: Don't leak runtime pm ref on driver load
  drm/radeon: Forbid runtime pm on driver unload
  drm/amdgpu: Don't leak runtime pm ref on driver unload
  drm/amdgpu: Don't leak runtime pm ref on driver load
  drm/amdgpu: Forbid runtime pm on driver unload
  drm: Add helpers to turn off CRTCs
  drm/nouveau: Turn off CRTCs on driver unload
  drm/radeon: Turn off CRTCs on driver unload
  drm/amdgpu: Turn off CRTCs on driver unload
  drm: Use helper to turn off CRTC
  drm/i2c/ch7006: Use helper to turn off CRTC
  drm/nouveau/dispnv04: Use helper to turn off CRTC

 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c    | 12 +++++--
 drivers/gpu/drm/drm_crtc.c                 | 53 ++++++++++++++++++++++++++----
 drivers/gpu/drm/i2c/ch7006_drv.c           |  9 ++---
 drivers/gpu/drm/nouveau/dispnv04/disp.c    | 10 ------
 drivers/gpu/drm/nouveau/dispnv04/tvnv17.c  |  9 ++---
 drivers/gpu/drm/nouveau/nouveau_display.c  |  1 +
 drivers/gpu/drm/nouveau/nouveau_drm.c      |  6 +++-
 drivers/gpu/drm/radeon/radeon_device.c     |  4 +++
 drivers/gpu/drm/radeon/radeon_display.c    |  1 +
 drivers/gpu/drm/radeon/radeon_kms.c        |  5 ++-
 include/drm/drm_crtc.h                     |  2 ++
 12 files changed, 77 insertions(+), 36 deletions(-)

-- 
2.8.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 09/15] drm: Add helpers to turn off CRTCs

[Lukas Wunner]

Turning off a single CRTC or all active CRTCs of a DRM device is a
fairly common pattern. Add helpers to avoid open coding this everywhere.

The name was chosen to be consistent with drm_plane_force_disable().

Cc: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/gpu/drm/drm_crtc.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_crtc.h     |  2 ++
 2 files changed, 47 insertions(+)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 37427b2..1edc1f4 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -426,6 +426,51 @@ void drm_mode_object_reference(struct drm_mode_object *obj)
 }
 EXPORT_SYMBOL(drm_mode_object_reference);
 
+/**
+ * drm_crtc_force_disable - Forcibly turn off a CRTC
+ * @crtc: CRTC to turn off
+ *
+ * Returns:
+ * Zero on success, error code on failure.
+ */
+int drm_crtc_force_disable(struct drm_crtc *crtc)
+{
+	struct drm_mode_set set = {
+		.crtc = crtc,
+	};
+
+	return drm_mode_set_config_internal(&set);
+}
+EXPORT_SYMBOL(drm_crtc_force_disable);
+
+/**
+ * drm_crtc_force_disable_all - Forcibly turn off all enabled CRTCs
+ * @dev: DRM device whose CRTCs to turn off
+ *
+ * Drivers may want to call this on unload to ensure that all displays are
+ * unlit and the GPU is in a consistent, low power state. Takes modeset locks.
+ *
+ * Returns:
+ * Zero on success, error code on failure.
+ */
+int drm_crtc_force_disable_all(struct drm_device *dev)
+{
+	struct drm_crtc *crtc;
+	int ret = 0;
+
+	drm_modeset_lock_all(dev);
+	drm_for_each_crtc(crtc, dev)
+		if (crtc->enabled) {
+			ret = drm_crtc_force_disable(crtc);
+			if (ret)
+				goto out;
+		}
+out:
+	drm_modeset_unlock_all(dev);
+	return ret;
+}
+EXPORT_SYMBOL(drm_crtc_force_disable_all);
+
 static void drm_framebuffer_free(struct kref *kref)
 {
 	struct drm_framebuffer *fb =
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index d1559cd..c192f2c 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -2326,6 +2326,8 @@ extern int drm_crtc_check_viewport(const struct drm_crtc *crtc,
 				   int x, int y,
 				   const struct drm_display_mode *mode,
 				   const struct drm_framebuffer *fb);
+extern int drm_crtc_force_disable(struct drm_crtc *crtc);
+extern int drm_crtc_force_disable_all(struct drm_device *dev);
 
 extern void drm_encoder_cleanup(struct drm_encoder *encoder);
 
-- 
2.8.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 02/15] drm/nouveau: Forbid runtime pm on driver unload

[Lukas Wunner]

The PCI core calls pm_runtime_forbid() on device probe in pci_pm_init(),
making this the default state when nouveau is loaded. nouveau_drm_load()
therefore calls pm_runtime_allow(), but there's no pm_runtime_forbid()
in nouveau_drm_unload() to balance it. Add it so that we leave the
device in the same state that we found it.

This isn't a bug, it's just good housekeeping. When nouveau is first
loaded with runpm=1, then unloaded and loaded again with runpm=0,
pm_runtime_forbid() will be called from nouveau_pmops_runtime_idle() or
nouveau_pmops_runtime_suspend(), so the behaviour is correct. The nvidia
blob doesn't use runtime pm, but if it ever does, this commit avoids
that it has to clean up behind nouveau.

Cc: Ben Skeggs <bskeggs@redhat.com>
Tested-by: Karol Herbst <karolherbst@gmail.com>
Tested-by: Peter Wu <peter@lekensteyn.nl>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/gpu/drm/nouveau/nouveau_drm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
index 2b0f441..e1d1d9c 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -492,6 +492,7 @@ nouveau_drm_unload(struct drm_device *dev)
 
 	if (nouveau_runtime_pm != 0) {
 		pm_runtime_get_sync(dev->dev);
+		pm_runtime_forbid(dev->dev);
 	}
 
 	nouveau_fbcon_fini(dev);
-- 
2.8.1

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

[PATCH v2 15/15] drm/nouveau/dispnv04: Use helper to turn off CRTC

[Lukas Wunner]

Use shiny new drm_crtc_force_disable() instead of open coding the same.
No functional change intended.

Cc: Ben Skeggs <bskeggs@redhat.com>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/gpu/drm/nouveau/dispnv04/tvnv17.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv04/tvnv17.c b/drivers/gpu/drm/nouveau/dispnv04/tvnv17.c
index a665b78..434d1e2 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/tvnv17.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/tvnv17.c
@@ -749,13 +749,8 @@ static int nv17_tv_set_property(struct drm_encoder *encoder,
 
 		/* Disable the crtc to ensure a full modeset is
 		 * performed whenever it's turned on again. */
-		if (crtc) {
-			struct drm_mode_set modeset = {
-				.crtc = crtc,
-			};
-
-			drm_mode_set_config_internal(&modeset);
-		}
+		if (crtc)
+			drm_crtc_force_disable(crtc);
 	}
 
 	return 0;
-- 
2.8.1

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

[PATCH v2 10/15] drm/nouveau: Turn off CRTCs on driver unload

[Lukas Wunner]

nouveau leaks a runtime pm ref if at least one CRTC is enabled on
unload. The ref is taken by nouveau_crtc_set_config() and held as long
as a CRTC is in use.

nv04_display_destroy() should solve this by turning off all CRTCs, but

(1) nv50_display_destroy() doesn't do the same and

(2) it's broken since commit d6bf2f370703 ("drm/nouveau: run mode_config
    destructor before destroying internal display state") because the
    crtc structs are torn down by drm_mode_config_cleanup() before being
    turned off. Also, there's no locking.

Move the code to turn off all CRTCs from nv04_display_destroy() to
nouveau_display_destroy() so that it's called for both nv04 and nv50
and before drm_mode_config_cleanup(). Use drm_crtc_force_disable_all()
helper to save on code and have proper locking.

Cc: Ben Skeggs <bskeggs@redhat.com>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/gpu/drm/nouveau/dispnv04/disp.c   | 10 ----------
 drivers/gpu/drm/nouveau/nouveau_display.c |  1 +
 2 files changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv04/disp.c b/drivers/gpu/drm/nouveau/dispnv04/disp.c
index aea81a5..34c0f2f6 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/disp.c
@@ -125,18 +125,8 @@ nv04_display_destroy(struct drm_device *dev)
 	struct nv04_display *disp = nv04_display(dev);
 	struct nouveau_drm *drm = nouveau_drm(dev);
 	struct nouveau_encoder *encoder;
-	struct drm_crtc *crtc;
 	struct nouveau_crtc *nv_crtc;
 
-	/* Turn every CRTC off. */
-	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
-		struct drm_mode_set modeset = {
-			.crtc = crtc,
-		};
-
-		drm_mode_set_config_internal(&modeset);
-	}
-
 	/* Restore state */
 	list_for_each_entry(encoder, &dev->mode_config.encoder_list, base.base.head)
 		encoder->enc_restore(&encoder->base.base);
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
index 7c77f96..cbdb3d4 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -554,6 +554,7 @@ nouveau_display_destroy(struct drm_device *dev)
 	nouveau_display_vblank_fini(dev);
 
 	drm_kms_helper_poll_fini(dev);
+	drm_crtc_force_disable_all(dev);
 	drm_mode_config_cleanup(dev);
 
 	if (disp->dtor)
-- 
2.8.1

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH v2 00/15] Runtime pm ref leak bonanza

[Daniel Vetter]

On Wed, Jun 08, 2016 at 06:47:27PM +0200, Lukas Wunner wrote:
> Second iteration of my endeavour to rid nouveau, radeon and amdgpu of
> runtime pm ref leaks.
> 
> Patches 1 to 8 are identical to v1.
> 
> Patch 9 of v1 modified the DRM core to turn off all CRTCs on driver
> unload. Based on feedback by Daniel Vetter, I've replaced this with
> a helper to turn off all CRTCs, which is called by nouveau, radeon
> and amdgpu on unload. In other words, this is now opt-in.
> So patch 9 of v1 is replaced with new patches 9 to 12.
> 
> A by-product of patch 9 is a helper which turns off a *single* CRTC.
> This is open coded in three other places in the DRM tree and patches
> 13 to 15 refactor those to use the new helper.

Yeah I think this makes much more sense. Please poke amd/nouveau folks for
reviews/acks, then I can merge.
-Daniel

> 
> To ease reviewing, I've pushed this series to GitHub:
> https://github.com/l1k/linux/commits/drm_runpm_fixes_v2
> 
> The discussion on v1 is archived here:
> https://lists.freedesktop.org/archives/dri-devel/2016-May/thread.html#108278
> 
> Thanks,
> 
> Lukas
> 
> Lukas Wunner (15):
>   drm/nouveau: Don't leak runtime pm ref on driver unload
>   drm/nouveau: Forbid runtime pm on driver unload
>   drm/radeon: Don't leak runtime pm ref on driver unload
>   drm/radeon: Don't leak runtime pm ref on driver load
>   drm/radeon: Forbid runtime pm on driver unload
>   drm/amdgpu: Don't leak runtime pm ref on driver unload
>   drm/amdgpu: Don't leak runtime pm ref on driver load
>   drm/amdgpu: Forbid runtime pm on driver unload
>   drm: Add helpers to turn off CRTCs
>   drm/nouveau: Turn off CRTCs on driver unload
>   drm/radeon: Turn off CRTCs on driver unload
>   drm/amdgpu: Turn off CRTCs on driver unload
>   drm: Use helper to turn off CRTC
>   drm/i2c/ch7006: Use helper to turn off CRTC
>   drm/nouveau/dispnv04: Use helper to turn off CRTC
> 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  1 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c    | 12 +++++--
>  drivers/gpu/drm/drm_crtc.c                 | 53 ++++++++++++++++++++++++++----
>  drivers/gpu/drm/i2c/ch7006_drv.c           |  9 ++---
>  drivers/gpu/drm/nouveau/dispnv04/disp.c    | 10 ------
>  drivers/gpu/drm/nouveau/dispnv04/tvnv17.c  |  9 ++---
>  drivers/gpu/drm/nouveau/nouveau_display.c  |  1 +
>  drivers/gpu/drm/nouveau/nouveau_drm.c      |  6 +++-
>  drivers/gpu/drm/radeon/radeon_device.c     |  4 +++
>  drivers/gpu/drm/radeon/radeon_display.c    |  1 +
>  drivers/gpu/drm/radeon/radeon_kms.c        |  5 ++-
>  include/drm/drm_crtc.h                     |  2 ++
>  12 files changed, 77 insertions(+), 36 deletions(-)
> 
> -- 
> 2.8.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH v2 00/15] Runtime pm ref leak bonanza

[Ben Skeggs]

[-- Attachment #1.1.1: Type: text/plain, Size: 2872 bytes --]

On 06/09/2016 04:50 PM, Daniel Vetter wrote:
> On Wed, Jun 08, 2016 at 06:47:27PM +0200, Lukas Wunner wrote:
>> Second iteration of my endeavour to rid nouveau, radeon and amdgpu of
>> runtime pm ref leaks.
>>
>> Patches 1 to 8 are identical to v1.
>>
>> Patch 9 of v1 modified the DRM core to turn off all CRTCs on driver
>> unload. Based on feedback by Daniel Vetter, I've replaced this with
>> a helper to turn off all CRTCs, which is called by nouveau, radeon
>> and amdgpu on unload. In other words, this is now opt-in.
>> So patch 9 of v1 is replaced with new patches 9 to 12.
>>
>> A by-product of patch 9 is a helper which turns off a *single* CRTC.
>> This is open coded in three other places in the DRM tree and patches
>> 13 to 15 refactor those to use the new helper.
> 
> Yeah I think this makes much more sense. Please poke amd/nouveau folks for
> reviews/acks, then I can merge.
Looks fine to me.

Ben.

> -Daniel
> 
>>
>> To ease reviewing, I've pushed this series to GitHub:
>> https://github.com/l1k/linux/commits/drm_runpm_fixes_v2
>>
>> The discussion on v1 is archived here:
>> https://lists.freedesktop.org/archives/dri-devel/2016-May/thread.html#108278
>>
>> Thanks,
>>
>> Lukas
>>
>> Lukas Wunner (15):
>>   drm/nouveau: Don't leak runtime pm ref on driver unload
>>   drm/nouveau: Forbid runtime pm on driver unload
>>   drm/radeon: Don't leak runtime pm ref on driver unload
>>   drm/radeon: Don't leak runtime pm ref on driver load
>>   drm/radeon: Forbid runtime pm on driver unload
>>   drm/amdgpu: Don't leak runtime pm ref on driver unload
>>   drm/amdgpu: Don't leak runtime pm ref on driver load
>>   drm/amdgpu: Forbid runtime pm on driver unload
>>   drm: Add helpers to turn off CRTCs
>>   drm/nouveau: Turn off CRTCs on driver unload
>>   drm/radeon: Turn off CRTCs on driver unload
>>   drm/amdgpu: Turn off CRTCs on driver unload
>>   drm: Use helper to turn off CRTC
>>   drm/i2c/ch7006: Use helper to turn off CRTC
>>   drm/nouveau/dispnv04: Use helper to turn off CRTC
>>
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  1 +
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c    | 12 +++++--
>>  drivers/gpu/drm/drm_crtc.c                 | 53 ++++++++++++++++++++++++++----
>>  drivers/gpu/drm/i2c/ch7006_drv.c           |  9 ++---
>>  drivers/gpu/drm/nouveau/dispnv04/disp.c    | 10 ------
>>  drivers/gpu/drm/nouveau/dispnv04/tvnv17.c  |  9 ++---
>>  drivers/gpu/drm/nouveau/nouveau_display.c  |  1 +
>>  drivers/gpu/drm/nouveau/nouveau_drm.c      |  6 +++-
>>  drivers/gpu/drm/radeon/radeon_device.c     |  4 +++
>>  drivers/gpu/drm/radeon/radeon_display.c    |  1 +
>>  drivers/gpu/drm/radeon/radeon_kms.c        |  5 ++-
>>  include/drm/drm_crtc.h                     |  2 ++
>>  12 files changed, 77 insertions(+), 36 deletions(-)
>>
>> -- 
>> 2.8.1
>>
> 


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH v2 00/15] Runtime pm ref leak bonanza

[Alex Deucher]

On Thu, Jun 9, 2016 at 2:50 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Wed, Jun 08, 2016 at 06:47:27PM +0200, Lukas Wunner wrote:
>> Second iteration of my endeavour to rid nouveau, radeon and amdgpu of
>> runtime pm ref leaks.
>>
>> Patches 1 to 8 are identical to v1.
>>
>> Patch 9 of v1 modified the DRM core to turn off all CRTCs on driver
>> unload. Based on feedback by Daniel Vetter, I've replaced this with
>> a helper to turn off all CRTCs, which is called by nouveau, radeon
>> and amdgpu on unload. In other words, this is now opt-in.
>> So patch 9 of v1 is replaced with new patches 9 to 12.
>>
>> A by-product of patch 9 is a helper which turns off a *single* CRTC.
>> This is open coded in three other places in the DRM tree and patches
>> 13 to 15 refactor those to use the new helper.
>
> Yeah I think this makes much more sense. Please poke amd/nouveau folks for
> reviews/acks, then I can merge.

The amdgpu, radeon, and drm patches are:
Acked-by: Alex Deucher <alexander.deucher@amd.com>

> -Daniel
>
>>
>> To ease reviewing, I've pushed this series to GitHub:
>> https://github.com/l1k/linux/commits/drm_runpm_fixes_v2
>>
>> The discussion on v1 is archived here:
>> https://lists.freedesktop.org/archives/dri-devel/2016-May/thread.html#108278
>>
>> Thanks,
>>
>> Lukas
>>
>> Lukas Wunner (15):
>>   drm/nouveau: Don't leak runtime pm ref on driver unload
>>   drm/nouveau: Forbid runtime pm on driver unload
>>   drm/radeon: Don't leak runtime pm ref on driver unload
>>   drm/radeon: Don't leak runtime pm ref on driver load
>>   drm/radeon: Forbid runtime pm on driver unload
>>   drm/amdgpu: Don't leak runtime pm ref on driver unload
>>   drm/amdgpu: Don't leak runtime pm ref on driver load
>>   drm/amdgpu: Forbid runtime pm on driver unload
>>   drm: Add helpers to turn off CRTCs
>>   drm/nouveau: Turn off CRTCs on driver unload
>>   drm/radeon: Turn off CRTCs on driver unload
>>   drm/amdgpu: Turn off CRTCs on driver unload
>>   drm: Use helper to turn off CRTC
>>   drm/i2c/ch7006: Use helper to turn off CRTC
>>   drm/nouveau/dispnv04: Use helper to turn off CRTC
>>
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  1 +
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c    | 12 +++++--
>>  drivers/gpu/drm/drm_crtc.c                 | 53 ++++++++++++++++++++++++++----
>>  drivers/gpu/drm/i2c/ch7006_drv.c           |  9 ++---
>>  drivers/gpu/drm/nouveau/dispnv04/disp.c    | 10 ------
>>  drivers/gpu/drm/nouveau/dispnv04/tvnv17.c  |  9 ++---
>>  drivers/gpu/drm/nouveau/nouveau_display.c  |  1 +
>>  drivers/gpu/drm/nouveau/nouveau_drm.c      |  6 +++-
>>  drivers/gpu/drm/radeon/radeon_device.c     |  4 +++
>>  drivers/gpu/drm/radeon/radeon_display.c    |  1 +
>>  drivers/gpu/drm/radeon/radeon_kms.c        |  5 ++-
>>  include/drm/drm_crtc.h                     |  2 ++
>>  12 files changed, 77 insertions(+), 36 deletions(-)
>>
>> --
>> 2.8.1
>>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH v2 00/15] Runtime pm ref leak bonanza

[Lukas Wunner]

On Tue, Jun 14, 2016 at 04:18:00PM -0400, Alex Deucher wrote:
> On Thu, Jun 9, 2016 at 2:50 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Wed, Jun 08, 2016 at 06:47:27PM +0200, Lukas Wunner wrote:
> >> Second iteration of my endeavour to rid nouveau, radeon and amdgpu of
> >> runtime pm ref leaks.
> >>
> >> Patches 1 to 8 are identical to v1.
> >>
> >> Patch 9 of v1 modified the DRM core to turn off all CRTCs on driver
> >> unload. Based on feedback by Daniel Vetter, I've replaced this with
> >> a helper to turn off all CRTCs, which is called by nouveau, radeon
> >> and amdgpu on unload. In other words, this is now opt-in.
> >> So patch 9 of v1 is replaced with new patches 9 to 12.
> >>
> >> A by-product of patch 9 is a helper which turns off a *single* CRTC.
> >> This is open coded in three other places in the DRM tree and patches
> >> 13 to 15 refactor those to use the new helper.
> >
> > Yeah I think this makes much more sense. Please poke amd/nouveau folks for
> > reviews/acks, then I can merge.
> 
> The amdgpu, radeon, and drm patches are:
> Acked-by: Alex Deucher <alexander.deucher@amd.com>

Great, thank you. That means all patches are either acked or reviewed:

* Patches 1, 2, 10 and 15 are
  Acked-by: Ben Skeggs <bskeggs@redhat.com>
  Message-ID: <b4e3a8a6-cb44-80bf-8834-419afc2d1cb4@gmail.com>
  Link: https://lists.freedesktop.org/archives/nouveau/2016-June/025350.html

* Patches 3 - 9 and 11 - 13 are
  Acked-by: Alex Deucher <alexander.deucher@amd.com>
  Message-ID: <CADnq5_NENZukwJg-CACQ-djWsP4e299gnr4aWrEJkdcVMeaZWA@mail.gmail.com>
  Link: https://lists.freedesktop.org/archives/dri-devel/2016-June/110876.html

* Patch 14 is
  Reviewed-by: Francisco Jerez <currojerez@riseup.net>
  Message-ID: <87ziqrtml5.fsf@riseup.net>
  Link: https://lists.freedesktop.org/archives/dri-devel/2016-June/110588.html


Dave, Daniel, could one of you pick this up?

I've pushed another branch to GitHub which is amended with all the
Acked-by and Reviewed-by tags. It's also rebased on latest drm-next.
Otherwise this is identical to what I've posted. So in case you don't
want to apply all tags manually, you can cherry-pick from or merge
this branch (barring any objections of course):
https://github.com/l1k/linux/commits/drm_runpm_fixes_v2_acked

Thanks everyone!

Lukas
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH v2 00/15] Runtime pm ref leak bonanza

[Daniel Vetter]

On Wed, Jun 15, 2016 at 01:37:35PM +0200, Lukas Wunner wrote:
> On Tue, Jun 14, 2016 at 04:18:00PM -0400, Alex Deucher wrote:
> > On Thu, Jun 9, 2016 at 2:50 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> > > On Wed, Jun 08, 2016 at 06:47:27PM +0200, Lukas Wunner wrote:
> > >> Second iteration of my endeavour to rid nouveau, radeon and amdgpu of
> > >> runtime pm ref leaks.
> > >>
> > >> Patches 1 to 8 are identical to v1.
> > >>
> > >> Patch 9 of v1 modified the DRM core to turn off all CRTCs on driver
> > >> unload. Based on feedback by Daniel Vetter, I've replaced this with
> > >> a helper to turn off all CRTCs, which is called by nouveau, radeon
> > >> and amdgpu on unload. In other words, this is now opt-in.
> > >> So patch 9 of v1 is replaced with new patches 9 to 12.
> > >>
> > >> A by-product of patch 9 is a helper which turns off a *single* CRTC.
> > >> This is open coded in three other places in the DRM tree and patches
> > >> 13 to 15 refactor those to use the new helper.
> > >
> > > Yeah I think this makes much more sense. Please poke amd/nouveau folks for
> > > reviews/acks, then I can merge.
> > 
> > The amdgpu, radeon, and drm patches are:
> > Acked-by: Alex Deucher <alexander.deucher@amd.com>
> 
> Great, thank you. That means all patches are either acked or reviewed:
> 
> * Patches 1, 2, 10 and 15 are
>   Acked-by: Ben Skeggs <bskeggs@redhat.com>
>   Message-ID: <b4e3a8a6-cb44-80bf-8834-419afc2d1cb4@gmail.com>
>   Link: https://lists.freedesktop.org/archives/nouveau/2016-June/025350.html
> 
> * Patches 3 - 9 and 11 - 13 are
>   Acked-by: Alex Deucher <alexander.deucher@amd.com>
>   Message-ID: <CADnq5_NENZukwJg-CACQ-djWsP4e299gnr4aWrEJkdcVMeaZWA@mail.gmail.com>
>   Link: https://lists.freedesktop.org/archives/dri-devel/2016-June/110876.html
> 
> * Patch 14 is
>   Reviewed-by: Francisco Jerez <currojerez@riseup.net>
>   Message-ID: <87ziqrtml5.fsf@riseup.net>
>   Link: https://lists.freedesktop.org/archives/dri-devel/2016-June/110588.html
> 
> 
> Dave, Daniel, could one of you pick this up?
> 
> I've pushed another branch to GitHub which is amended with all the
> Acked-by and Reviewed-by tags. It's also rebased on latest drm-next.
> Otherwise this is identical to what I've posted. So in case you don't
> want to apply all tags manually, you can cherry-pick from or merge
> this branch (barring any objections of course):
> https://github.com/l1k/linux/commits/drm_runpm_fixes_v2_acked
> 
> Thanks everyone!

Yeah will pick up later this week, right now I have a big drm-misc pull
that's pending. Would like to get that landed first. Please ping me if
your patches haven't landed in drm-misc by next week.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH v2 00/15] Runtime pm ref leak bonanza

[Lukas Wunner]

On Wed, Jun 15, 2016 at 05:11:54PM +0200, Daniel Vetter wrote:
> On Wed, Jun 15, 2016 at 01:37:35PM +0200, Lukas Wunner wrote:
> > On Tue, Jun 14, 2016 at 04:18:00PM -0400, Alex Deucher wrote:
> > > On Thu, Jun 9, 2016 at 2:50 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> > > > On Wed, Jun 08, 2016 at 06:47:27PM +0200, Lukas Wunner wrote:
> > > >> Second iteration of my endeavour to rid nouveau, radeon and amdgpu of
> > > >> runtime pm ref leaks.
> > > >>
> > > >> Patches 1 to 8 are identical to v1.
> > > >>
> > > >> Patch 9 of v1 modified the DRM core to turn off all CRTCs on driver
> > > >> unload. Based on feedback by Daniel Vetter, I've replaced this with
> > > >> a helper to turn off all CRTCs, which is called by nouveau, radeon
> > > >> and amdgpu on unload. In other words, this is now opt-in.
> > > >> So patch 9 of v1 is replaced with new patches 9 to 12.
> > > >>
> > > >> A by-product of patch 9 is a helper which turns off a *single* CRTC.
> > > >> This is open coded in three other places in the DRM tree and patches
> > > >> 13 to 15 refactor those to use the new helper.
> > > >
> > > > Yeah I think this makes much more sense. Please poke amd/nouveau folks for
> > > > reviews/acks, then I can merge.
> > > 
> > > The amdgpu, radeon, and drm patches are:
> > > Acked-by: Alex Deucher <alexander.deucher@amd.com>
> > 
> > Great, thank you. That means all patches are either acked or reviewed:
> > 
> > * Patches 1, 2, 10 and 15 are
> >   Acked-by: Ben Skeggs <bskeggs@redhat.com>
> >   Message-ID: <b4e3a8a6-cb44-80bf-8834-419afc2d1cb4@gmail.com>
> >   Link: https://lists.freedesktop.org/archives/nouveau/2016-June/025350.html
> > 
> > * Patches 3 - 9 and 11 - 13 are
> >   Acked-by: Alex Deucher <alexander.deucher@amd.com>
> >   Message-ID: <CADnq5_NENZukwJg-CACQ-djWsP4e299gnr4aWrEJkdcVMeaZWA@mail.gmail.com>
> >   Link: https://lists.freedesktop.org/archives/dri-devel/2016-June/110876.html
> > 
> > * Patch 14 is
> >   Reviewed-by: Francisco Jerez <currojerez@riseup.net>
> >   Message-ID: <87ziqrtml5.fsf@riseup.net>
> >   Link: https://lists.freedesktop.org/archives/dri-devel/2016-June/110588.html
> > 
> > 
> > Dave, Daniel, could one of you pick this up?
> > 
> > I've pushed another branch to GitHub which is amended with all the
> > Acked-by and Reviewed-by tags. It's also rebased on latest drm-next.
> > Otherwise this is identical to what I've posted. So in case you don't
> > want to apply all tags manually, you can cherry-pick from or merge
> > this branch (barring any objections of course):
> > https://github.com/l1k/linux/commits/drm_runpm_fixes_v2_acked
> > 
> > Thanks everyone!
> 
> Yeah will pick up later this week, right now I have a big drm-misc pull
> that's pending. Would like to get that landed first. Please ping me if
> your patches haven't landed in drm-misc by next week.

If you can be bothered to update your drm-misc pull: Ping!
(Otherwise I think Dave or Thierry/Sumit could pick it up instead.)

Thanks and have a pleasant vacation,

Lukas
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH v2 00/15] Runtime pm ref leak bonanza

[Daniel Vetter]

On Wed, Jun 22, 2016 at 2:44 PM, Lukas Wunner <lukas@wunner.de> wrote:
>> Yeah will pick up later this week, right now I have a big drm-misc pull
>> that's pending. Would like to get that landed first. Please ping me if
>> your patches haven't landed in drm-misc by next week.
>
> If you can be bothered to update your drm-misc pull: Ping!
> (Otherwise I think Dave or Thierry/Sumit could pick it up instead.)

So much for my 5 second attention span :( I'll merge them right now,
but I think it's better to let them soak a bit more. Maybe Thierry can
send a pull request next week (if more patches show up). Note I'll
wait with pushing out until Dave merged the previous pull, to avoid a
mess in case another fixup is needed.

Thanks for reminding me.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

[PATCH v2 14/15] drm/i2c/ch7006: Use helper to turn off CRTC

[Lukas Wunner]

Use shiny new drm_crtc_force_disable() instead of open coding the same.
No functional change intended.

Cc: Francisco Jerez <currojerez@riseup.net>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/gpu/drm/i2c/ch7006_drv.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i2c/ch7006_drv.c b/drivers/gpu/drm/i2c/ch7006_drv.c
index 0594c45..e9e8ae2 100644
--- a/drivers/gpu/drm/i2c/ch7006_drv.c
+++ b/drivers/gpu/drm/i2c/ch7006_drv.c
@@ -361,13 +361,8 @@ static int ch7006_encoder_set_property(struct drm_encoder *encoder,
 
 		/* Disable the crtc to ensure a full modeset is
 		 * performed whenever it's turned on again. */
-		if (crtc) {
-			struct drm_mode_set modeset = {
-				.crtc = crtc,
-			};
-
-			drm_mode_set_config_internal(&modeset);
-		}
+		if (crtc)
+			drm_crtc_force_disable(crtc);
 	}
 
 	return 0;
-- 
2.8.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 14/15] drm/i2c/ch7006: Use helper to turn off CRTC

[Francisco Jerez]

[-- Attachment #1.1.1: Type: text/plain, Size: 1071 bytes --]

Lukas Wunner <lukas@wunner.de> writes:

> Use shiny new drm_crtc_force_disable() instead of open coding the same.
> No functional change intended.
>
> Cc: Francisco Jerez <currojerez@riseup.net>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>

Reviewed-by: Francisco Jerez <currojerez@riseup.net>

> ---
>  drivers/gpu/drm/i2c/ch7006_drv.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i2c/ch7006_drv.c b/drivers/gpu/drm/i2c/ch7006_drv.c
> index 0594c45..e9e8ae2 100644
> --- a/drivers/gpu/drm/i2c/ch7006_drv.c
> +++ b/drivers/gpu/drm/i2c/ch7006_drv.c
> @@ -361,13 +361,8 @@ static int ch7006_encoder_set_property(struct drm_encoder *encoder,
>  
>  		/* Disable the crtc to ensure a full modeset is
>  		 * performed whenever it's turned on again. */
> -		if (crtc) {
> -			struct drm_mode_set modeset = {
> -				.crtc = crtc,
> -			};
> -
> -			drm_mode_set_config_internal(&modeset);
> -		}
> +		if (crtc)
> +			drm_crtc_force_disable(crtc);
>  	}
>  
>  	return 0;
> -- 
> 2.8.1

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 212 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 08/15] drm/amdgpu: Forbid runtime pm on driver unload

[Lukas Wunner]

The PCI core calls pm_runtime_forbid() on device probe in pci_pm_init(),
making this the default state when amdgpu is loaded.
amdgpu_driver_load_kms() therefore calls pm_runtime_allow(), but there's
no pm_runtime_forbid() in amdgpu_driver_unload_kms() to balance it. Add
it so that we leave the device in the same state that we found it.

This isn't a bug, it's just good housekeeping. When amdgpu is first
loaded with runpm=1, then unloaded and loaded again with runpm=0,
pm_runtime_forbid() will be called from amdgpu_pmops_runtime_idle() or
amdgpu_pmops_runtime_suspend(), so the behaviour is correct. If there
ever is a third party driver for AMD cards, this commit avoids that it
has to clean up behind amdgpu.

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 0db692e..38a28d1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -62,6 +62,7 @@ int amdgpu_driver_unload_kms(struct drm_device *dev)
 
 	if (amdgpu_device_is_px(dev)) {
 		pm_runtime_get_sync(dev->dev);
+		pm_runtime_forbid(dev->dev);
 	}
 
 	amdgpu_amdkfd_device_fini(adev);
-- 
2.8.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 07/15] drm/amdgpu: Don't leak runtime pm ref on driver load

[Lukas Wunner]

If an error occurs in amdgpu_device_init() after adev->rmmio has been
set, its caller amdgpu_driver_load_kms() will skip runtime pm
initialization and call amdgpu_driver_unload_kms(), which acquires a
runtime pm ref that is leaked.

Balance by releasing a runtime pm ref in the error path of
amdgpu_driver_load_kms().

Fixes: d38ceaf99ed0 ("drm/amdgpu: add core driver (v4)")
Cc: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 9b1f979..0db692e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -137,9 +137,12 @@ int amdgpu_driver_load_kms(struct drm_device *dev, unsigned long flags)
 	}
 
 out:
-	if (r)
+	if (r) {
+		/* balance pm_runtime_get_sync in amdgpu_driver_unload_kms */
+		if (adev->rmmio && amdgpu_device_is_px(dev))
+			pm_runtime_put_noidle(dev->dev);
 		amdgpu_driver_unload_kms(dev);
-
+	}
 
 	return r;
 }
-- 
2.8.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 03/15] drm/radeon: Don't leak runtime pm ref on driver unload

[Lukas Wunner]

radeon_driver_load_kms() calls pm_runtime_put_autosuspend() if
radeon_is_px(dev), but radeon_driver_unload_kms() calls
pm_runtime_get_sync() unconditionally. We therefore leak a runtime pm
ref whenever radeon is unloaded on a non-PX machine or if runpm=0. The
GPU will subsequently never runtime suspend after loading radeon again.

Fix by taking the runtime pm ref under the same condition that it was
released on driver load.

Fixes: 10ebc0bc0934 ("drm/radeon: add runtime PM support (v2)")
Cc: Dave Airlie <airlied@redhat.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/gpu/drm/radeon/radeon_kms.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c
index 414953c..51998a4 100644
--- a/drivers/gpu/drm/radeon/radeon_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_kms.c
@@ -63,7 +63,9 @@ int radeon_driver_unload_kms(struct drm_device *dev)
 	if (rdev->rmmio == NULL)
 		goto done_free;
 
-	pm_runtime_get_sync(dev->dev);
+	if (radeon_is_px(dev)) {
+		pm_runtime_get_sync(dev->dev);
+	}
 
 	radeon_kfd_device_fini(rdev);
 
-- 
2.8.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 04/15] drm/radeon: Don't leak runtime pm ref on driver load

[Lukas Wunner]

radeon_device_init() returns an error if either of the two calls to
radeon_init() fail. One level up in the call stack,
radeon_driver_load_kms() will then skip runtime pm initialization and
call radeon_driver_unload_kms(), which acquires a runtime pm ref that
is leaked.

Balance by releasing a runtime pm ref in the error path of
radeon_device_init().

Fixes: 10ebc0bc0934 ("drm/radeon: add runtime PM support (v2)")
Cc: Dave Airlie <airlied@redhat.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/gpu/drm/radeon/radeon_device.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
index e721e6b..e0bf778 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -30,6 +30,7 @@
 #include <drm/drmP.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/radeon_drm.h>
+#include <linux/pm_runtime.h>
 #include <linux/vgaarb.h>
 #include <linux/vga_switcheroo.h>
 #include <linux/efi.h>
@@ -1505,6 +1506,9 @@ int radeon_device_init(struct radeon_device *rdev,
 	return 0;
 
 failed:
+	/* balance pm_runtime_get_sync() in radeon_driver_unload_kms() */
+	if (radeon_is_px(ddev))
+		pm_runtime_put_noidle(ddev->dev);
 	if (runtime)
 		vga_switcheroo_fini_domain_pm_ops(rdev->dev);
 	return r;
-- 
2.8.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 05/15] drm/radeon: Forbid runtime pm on driver unload

[Lukas Wunner]

The PCI core calls pm_runtime_forbid() on device probe in pci_pm_init(),
making this the default state when radeon is loaded.
radeon_driver_load_kms() therefore calls pm_runtime_allow(), but there's
no pm_runtime_forbid() in radeon_driver_unload_kms() to balance it. Add
it so that we leave the device in the same state that we found it.

This isn't a bug, it's just good housekeeping. When radeon is first
loaded with runpm=1, then unloaded and loaded again with runpm=0,
pm_runtime_forbid() will be called from radeon_pmops_runtime_idle() or
radeon_pmops_runtime_suspend(), so the behaviour is correct. If there
ever is a third party driver for AMD cards, this commit avoids that it
has to clean up behind radeon.

Cc: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/gpu/drm/radeon/radeon_kms.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c
index 51998a4..835563c 100644
--- a/drivers/gpu/drm/radeon/radeon_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_kms.c
@@ -65,6 +65,7 @@ int radeon_driver_unload_kms(struct drm_device *dev)
 
 	if (radeon_is_px(dev)) {
 		pm_runtime_get_sync(dev->dev);
+		pm_runtime_forbid(dev->dev);
 	}
 
 	radeon_kfd_device_fini(rdev);
-- 
2.8.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 11/15] drm/radeon: Turn off CRTCs on driver unload

[Lukas Wunner]

radeon leaks a runtime pm ref if at least one CRTC is enabled on unload.
The ref is taken by radeon_crtc_set_config() and held as long as a CRTC
is in use. Fix by turning off all CRTCs on unload.

Cc: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/gpu/drm/radeon/radeon_display.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c
index 6a41b49..1922c96 100644
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -1708,6 +1708,7 @@ void radeon_modeset_fini(struct radeon_device *rdev)
 		radeon_afmt_fini(rdev);
 		drm_kms_helper_poll_fini(rdev->ddev);
 		radeon_hpd_fini(rdev);
+		drm_crtc_force_disable_all(rdev->ddev);
 		drm_mode_config_cleanup(rdev->ddev);
 		rdev->mode_info.mode_config_initialized = false;
 	}
-- 
2.8.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 13/15] drm: Use helper to turn off CRTC

[Lukas Wunner]

Use shiny new drm_crtc_force_disable() instead of open coding the same.
No functional change intended.

Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/gpu/drm/drm_crtc.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 1edc1f4..3bd32a1 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -619,8 +619,6 @@ void drm_framebuffer_remove(struct drm_framebuffer *fb)
 	struct drm_device *dev;
 	struct drm_crtc *crtc;
 	struct drm_plane *plane;
-	struct drm_mode_set set;
-	int ret;
 
 	if (!fb)
 		return;
@@ -650,11 +648,7 @@ void drm_framebuffer_remove(struct drm_framebuffer *fb)
 		drm_for_each_crtc(crtc, dev) {
 			if (crtc->primary->fb == fb) {
 				/* should turn off the crtc */
-				memset(&set, 0, sizeof(struct drm_mode_set));
-				set.crtc = crtc;
-				set.fb = NULL;
-				ret = drm_mode_set_config_internal(&set);
-				if (ret)
+				if (drm_crtc_force_disable(crtc))
 					DRM_ERROR("failed to reset crtc %p when fb was deleted\n", crtc);
 			}
 		}
-- 
2.8.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 01/15] drm/nouveau: Don't leak runtime pm ref on driver unload

[Lukas Wunner]

nouveau_drm_load() calls pm_runtime_put() if nouveau_runtime_pm != 0,
but nouveau_drm_unload() calls pm_runtime_get_sync() unconditionally.
We therefore leak a runtime pm ref whenever nouveau is loaded with
runpm=0 and then unloaded. The GPU will subsequently never runtime
suspend even if nouveau is loaded again with runpm=1.

Fix by taking the runtime pm ref under the same condition that it was
released on driver load.

Fixes: 5addcf0a5f0f ("nouveau: add runtime PM support (v0.9)")
Cc: Dave Airlie <airlied@redhat.com>
Cc: Ben Skeggs <bskeggs@redhat.com>
Reported-by: Karol Herbst <karolherbst@gmail.com>
Tested-by: Karol Herbst <karolherbst@gmail.com>
Tested-by: Peter Wu <peter@lekensteyn.nl>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/gpu/drm/nouveau/nouveau_drm.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
index 5c4d4df..2b0f441 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -490,7 +490,10 @@ nouveau_drm_unload(struct drm_device *dev)
 {
 	struct nouveau_drm *drm = nouveau_drm(dev);
 
-	pm_runtime_get_sync(dev->dev);
+	if (nouveau_runtime_pm != 0) {
+		pm_runtime_get_sync(dev->dev);
+	}
+
 	nouveau_fbcon_fini(dev);
 	nouveau_accel_fini(drm);
 	nouveau_hwmon_fini(dev);
-- 
2.8.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 12/15] drm/amdgpu: Turn off CRTCs on driver unload

[Lukas Wunner]

amdgpu leaks a runtime pm ref if at least one CRTC is enabled on unload.
The ref is taken by amdgpu_crtc_set_config() and held as long as a CRTC
is in use. Fix by turning off all CRTCs on unload.

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index bb8b149..2ab5e0b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1624,6 +1624,7 @@ void amdgpu_device_fini(struct amdgpu_device *adev)
 	amdgpu_bo_evict_vram(adev);
 	amdgpu_ib_pool_fini(adev);
 	amdgpu_fence_driver_fini(adev);
+	drm_crtc_force_disable_all(adev->ddev);
 	amdgpu_fbdev_fini(adev);
 	r = amdgpu_fini(adev);
 	kfree(adev->ip_block_status);
-- 
2.8.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 06/15] drm/amdgpu: Don't leak runtime pm ref on driver unload

[Lukas Wunner]

amdgpu_driver_load_kms() calls pm_runtime_put_autosuspend() if
amdgpu_device_is_px(dev), but amdgpu_driver_unload_kms() calls
pm_runtime_get_sync() unconditionally. We therefore leak a runtime pm
ref whenever amdgpu is unloaded on a non-PX machine or if runpm=0. The
GPU will subsequently never runtime suspend after loading amdgpu again.

Fix by taking the runtime pm ref under the same condition that it was
released on driver load.

Fixes: d38ceaf99ed0 ("drm/amdgpu: add core driver (v4)")
Cc: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 40a2370..9b1f979 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -60,7 +60,9 @@ int amdgpu_driver_unload_kms(struct drm_device *dev)
 	if (adev->rmmio == NULL)
 		goto done_free;
 
-	pm_runtime_get_sync(dev->dev);
+	if (amdgpu_device_is_px(dev)) {
+		pm_runtime_get_sync(dev->dev);
+	}
 
 	amdgpu_amdkfd_device_fini(adev);
 
-- 
2.8.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel