* [PATCH v2 09/15] drm: Add helpers to turn off CRTCs
2016-06-08 16:47 [PATCH v2 00/15] Runtime pm ref leak bonanza Lukas Wunner
@ 2016-06-08 16:47 ` Lukas Wunner
[not found] ` <cover.1465392124.git.lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
` (11 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Lukas Wunner @ 2016-06-08 16:47 UTC (permalink / raw)
To: dri-devel, nouveau
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
^ permalink raw reply related [flat|nested] 24+ messages in thread
[parent not found: <cover.1465392124.git.lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>]
* [PATCH v2 02/15] drm/nouveau: Forbid runtime pm on driver unload
[not found] ` <cover.1465392124.git.lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
@ 2016-06-08 16:47 ` Lukas Wunner
2016-06-08 16:47 ` [PATCH v2 15/15] drm/nouveau/dispnv04: Use helper to turn off CRTC Lukas Wunner
` (2 subsequent siblings)
3 siblings, 0 replies; 24+ messages in thread
From: Lukas Wunner @ 2016-06-08 16:47 UTC (permalink / raw)
To: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: Ben Skeggs
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
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 15/15] drm/nouveau/dispnv04: Use helper to turn off CRTC
[not found] ` <cover.1465392124.git.lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
2016-06-08 16:47 ` [PATCH v2 02/15] drm/nouveau: Forbid runtime pm on driver unload Lukas Wunner
@ 2016-06-08 16:47 ` Lukas Wunner
2016-06-08 16:47 ` [PATCH v2 10/15] drm/nouveau: Turn off CRTCs on driver unload Lukas Wunner
2016-06-09 6:50 ` [PATCH v2 00/15] Runtime pm ref leak bonanza Daniel Vetter
3 siblings, 0 replies; 24+ messages in thread
From: Lukas Wunner @ 2016-06-08 16:47 UTC (permalink / raw)
To: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: Ben Skeggs
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
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 10/15] drm/nouveau: Turn off CRTCs on driver unload
[not found] ` <cover.1465392124.git.lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
2016-06-08 16:47 ` [PATCH v2 02/15] drm/nouveau: Forbid runtime pm on driver unload Lukas Wunner
2016-06-08 16:47 ` [PATCH v2 15/15] drm/nouveau/dispnv04: Use helper to turn off CRTC Lukas Wunner
@ 2016-06-08 16:47 ` Lukas Wunner
2016-06-09 6:50 ` [PATCH v2 00/15] Runtime pm ref leak bonanza Daniel Vetter
3 siblings, 0 replies; 24+ messages in thread
From: Lukas Wunner @ 2016-06-08 16:47 UTC (permalink / raw)
To: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: Ben Skeggs
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
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v2 00/15] Runtime pm ref leak bonanza
[not found] ` <cover.1465392124.git.lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
` (2 preceding siblings ...)
2016-06-08 16:47 ` [PATCH v2 10/15] drm/nouveau: Turn off CRTCs on driver unload Lukas Wunner
@ 2016-06-09 6:50 ` Daniel Vetter
[not found] ` <20160609065032.GI3363-dv86pmgwkMBes7Z6vYuT8azUEOm+Xw19@public.gmane.org>
3 siblings, 1 reply; 24+ messages in thread
From: Daniel Vetter @ 2016-06-09 6:50 UTC (permalink / raw)
To: Lukas Wunner
Cc: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Ben Skeggs,
Daniel Vetter, Alex Deucher
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 14/15] drm/i2c/ch7006: Use helper to turn off CRTC
2016-06-08 16:47 [PATCH v2 00/15] Runtime pm ref leak bonanza Lukas Wunner
2016-06-08 16:47 ` [PATCH v2 09/15] drm: Add helpers to turn off CRTCs Lukas Wunner
[not found] ` <cover.1465392124.git.lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
@ 2016-06-08 16:47 ` Lukas Wunner
2016-06-11 21:57 ` Francisco Jerez
2016-06-08 16:47 ` [PATCH v2 08/15] drm/amdgpu: Forbid runtime pm on driver unload Lukas Wunner
` (9 subsequent siblings)
12 siblings, 1 reply; 24+ messages in thread
From: Lukas Wunner @ 2016-06-08 16:47 UTC (permalink / raw)
To: dri-devel
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
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v2 14/15] drm/i2c/ch7006: Use helper to turn off CRTC
2016-06-08 16:47 ` [PATCH v2 14/15] drm/i2c/ch7006: Use helper to turn off CRTC Lukas Wunner
@ 2016-06-11 21:57 ` Francisco Jerez
0 siblings, 0 replies; 24+ messages in thread
From: Francisco Jerez @ 2016-06-11 21:57 UTC (permalink / raw)
To: Lukas Wunner, dri-devel
[-- 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
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 08/15] drm/amdgpu: Forbid runtime pm on driver unload
2016-06-08 16:47 [PATCH v2 00/15] Runtime pm ref leak bonanza Lukas Wunner
` (2 preceding siblings ...)
2016-06-08 16:47 ` [PATCH v2 14/15] drm/i2c/ch7006: Use helper to turn off CRTC Lukas Wunner
@ 2016-06-08 16:47 ` Lukas Wunner
2016-06-08 16:47 ` [PATCH v2 07/15] drm/amdgpu: Don't leak runtime pm ref on driver load Lukas Wunner
` (8 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Lukas Wunner @ 2016-06-08 16:47 UTC (permalink / raw)
To: dri-devel; +Cc: Alex Deucher
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
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 07/15] drm/amdgpu: Don't leak runtime pm ref on driver load
2016-06-08 16:47 [PATCH v2 00/15] Runtime pm ref leak bonanza Lukas Wunner
` (3 preceding siblings ...)
2016-06-08 16:47 ` [PATCH v2 08/15] drm/amdgpu: Forbid runtime pm on driver unload Lukas Wunner
@ 2016-06-08 16:47 ` Lukas Wunner
2016-06-08 16:47 ` [PATCH v2 03/15] drm/radeon: Don't leak runtime pm ref on driver unload Lukas Wunner
` (7 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Lukas Wunner @ 2016-06-08 16:47 UTC (permalink / raw)
To: dri-devel; +Cc: Alex Deucher
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
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 03/15] drm/radeon: Don't leak runtime pm ref on driver unload
2016-06-08 16:47 [PATCH v2 00/15] Runtime pm ref leak bonanza Lukas Wunner
` (4 preceding siblings ...)
2016-06-08 16:47 ` [PATCH v2 07/15] drm/amdgpu: Don't leak runtime pm ref on driver load Lukas Wunner
@ 2016-06-08 16:47 ` Lukas Wunner
2016-06-08 16:47 ` [PATCH v2 04/15] drm/radeon: Don't leak runtime pm ref on driver load Lukas Wunner
` (6 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Lukas Wunner @ 2016-06-08 16:47 UTC (permalink / raw)
To: dri-devel; +Cc: Alex Deucher
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
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 04/15] drm/radeon: Don't leak runtime pm ref on driver load
2016-06-08 16:47 [PATCH v2 00/15] Runtime pm ref leak bonanza Lukas Wunner
` (5 preceding siblings ...)
2016-06-08 16:47 ` [PATCH v2 03/15] drm/radeon: Don't leak runtime pm ref on driver unload Lukas Wunner
@ 2016-06-08 16:47 ` Lukas Wunner
2016-06-08 16:47 ` [PATCH v2 05/15] drm/radeon: Forbid runtime pm on driver unload Lukas Wunner
` (5 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Lukas Wunner @ 2016-06-08 16:47 UTC (permalink / raw)
To: dri-devel; +Cc: Alex Deucher
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
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 05/15] drm/radeon: Forbid runtime pm on driver unload
2016-06-08 16:47 [PATCH v2 00/15] Runtime pm ref leak bonanza Lukas Wunner
` (6 preceding siblings ...)
2016-06-08 16:47 ` [PATCH v2 04/15] drm/radeon: Don't leak runtime pm ref on driver load Lukas Wunner
@ 2016-06-08 16:47 ` Lukas Wunner
2016-06-08 16:47 ` [PATCH v2 11/15] drm/radeon: Turn off CRTCs " Lukas Wunner
` (4 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Lukas Wunner @ 2016-06-08 16:47 UTC (permalink / raw)
To: dri-devel; +Cc: Alex Deucher
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
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 11/15] drm/radeon: Turn off CRTCs on driver unload
2016-06-08 16:47 [PATCH v2 00/15] Runtime pm ref leak bonanza Lukas Wunner
` (7 preceding siblings ...)
2016-06-08 16:47 ` [PATCH v2 05/15] drm/radeon: Forbid runtime pm on driver unload Lukas Wunner
@ 2016-06-08 16:47 ` Lukas Wunner
2016-06-08 16:47 ` [PATCH v2 13/15] drm: Use helper to turn off CRTC Lukas Wunner
` (3 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Lukas Wunner @ 2016-06-08 16:47 UTC (permalink / raw)
To: dri-devel; +Cc: Alex Deucher
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
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 13/15] drm: Use helper to turn off CRTC
2016-06-08 16:47 [PATCH v2 00/15] Runtime pm ref leak bonanza Lukas Wunner
` (8 preceding siblings ...)
2016-06-08 16:47 ` [PATCH v2 11/15] drm/radeon: Turn off CRTCs " Lukas Wunner
@ 2016-06-08 16:47 ` Lukas Wunner
2016-06-08 16:47 ` [PATCH v2 01/15] drm/nouveau: Don't leak runtime pm ref on driver unload Lukas Wunner
` (2 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Lukas Wunner @ 2016-06-08 16:47 UTC (permalink / raw)
To: dri-devel
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
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 01/15] drm/nouveau: Don't leak runtime pm ref on driver unload
2016-06-08 16:47 [PATCH v2 00/15] Runtime pm ref leak bonanza Lukas Wunner
` (9 preceding siblings ...)
2016-06-08 16:47 ` [PATCH v2 13/15] drm: Use helper to turn off CRTC Lukas Wunner
@ 2016-06-08 16:47 ` Lukas Wunner
2016-06-08 16:47 ` [PATCH v2 12/15] drm/amdgpu: Turn off CRTCs " Lukas Wunner
2016-06-08 16:47 ` [PATCH v2 06/15] drm/amdgpu: Don't leak runtime pm ref " Lukas Wunner
12 siblings, 0 replies; 24+ messages in thread
From: Lukas Wunner @ 2016-06-08 16:47 UTC (permalink / raw)
To: dri-devel, nouveau; +Cc: Ben Skeggs
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
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 12/15] drm/amdgpu: Turn off CRTCs on driver unload
2016-06-08 16:47 [PATCH v2 00/15] Runtime pm ref leak bonanza Lukas Wunner
` (10 preceding siblings ...)
2016-06-08 16:47 ` [PATCH v2 01/15] drm/nouveau: Don't leak runtime pm ref on driver unload Lukas Wunner
@ 2016-06-08 16:47 ` Lukas Wunner
2016-06-08 16:47 ` [PATCH v2 06/15] drm/amdgpu: Don't leak runtime pm ref " Lukas Wunner
12 siblings, 0 replies; 24+ messages in thread
From: Lukas Wunner @ 2016-06-08 16:47 UTC (permalink / raw)
To: dri-devel; +Cc: Alex Deucher
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
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 06/15] drm/amdgpu: Don't leak runtime pm ref on driver unload
2016-06-08 16:47 [PATCH v2 00/15] Runtime pm ref leak bonanza Lukas Wunner
` (11 preceding siblings ...)
2016-06-08 16:47 ` [PATCH v2 12/15] drm/amdgpu: Turn off CRTCs " Lukas Wunner
@ 2016-06-08 16:47 ` Lukas Wunner
12 siblings, 0 replies; 24+ messages in thread
From: Lukas Wunner @ 2016-06-08 16:47 UTC (permalink / raw)
To: dri-devel; +Cc: Alex Deucher
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
^ permalink raw reply related [flat|nested] 24+ messages in thread