Hi Daniel,
On Tue, May 24, 2016 at 11:30:42PM +0200, Daniel Vetter wrote:
On Tue, May 24, 2016 at 06:03:27PM +0200, Lukas Wunner wrote:
When a drm_crtc structure is destroyed with drm_crtc_cleanup(), the DRM core does not turn off the crtc first and neither do the drivers. With nouveau, radeon and amdgpu, this causes a runtime pm ref to be leaked on driver unload if at least one crtc was enabled.
(See usage of have_disp_power_ref in nouveau_crtc_set_config(), radeon_crtc_set_config() and amdgpu_crtc_set_config()).
Fixes: 5addcf0a5f0f ("nouveau: add runtime PM support (v0.9)") Cc: Dave Airlie airlied@redhat.com Tested-by: Karol Herbst nouveau@karolherbst.de Signed-off-by: Lukas Wunner lukas@wunner.de
This is a core regression, we fixed it again. Previously when unreference drm_planes the core made sure that it's not longer in use, which had the side effect of shutting everything off in module unload.
For a bunch of reasons we've stopped doing that, but that turned out to be a mistake. It's fixed since
commit f2d580b9a8149735cbc4b59c4a8df60173658140 Author: Maarten Lankhorst maarten.lankhorst@linux.intel.com Date: Wed May 4 14:38:26 2016 +0200
drm/core: Do not preserve framebuffer on rmfb, v4.Your patch shouldn't be needed with that any more. If it still is it's most likely the fbdev cleanup done too late, but you /should/ get a big WARNING splat in that case from drm_mode_config_cleanup().
I tested it and at least with nouveau, the above-mentioned commit does *not* solve the issue, so patch [9/9] of this series is still needed. I do not get a WARN splat when unloading nouveau.
Best regards,
Lukas
drivers/gpu/drm/drm_crtc.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index d2a6d95..0cd6f00 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -716,12 +716,23 @@ EXPORT_SYMBOL(drm_crtc_init_with_planes);
- This function cleans up @crtc and removes it from the DRM mode setting
- core. Note that the function does *not* free the crtc structure itself,
- this is the responsibility of the caller.
- this is the responsibility of the caller. If @crtc is currently enabled,
*/
- it is turned off first.
void drm_crtc_cleanup(struct drm_crtc *crtc) { struct drm_device *dev = crtc->dev;
- if (crtc->enabled) {
struct drm_mode_set modeset = {.crtc = crtc,};drm_modeset_lock_all(dev);drm_mode_set_config_internal(&modeset);drm_modeset_unlock_all(dev);- }
- kfree(crtc->gamma_store); crtc->gamma_store = NULL;
-- 2.8.1
Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch