On Wed, May 25, 2016 at 12:51 PM, Lukas Wunner <lukas@xxxxxxxxx> wrote:
>
> 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@xxxxxxxxxx>
>> > Tested-by: Karol Herbst <nouveau@xxxxxxxxxxxxxx>
>> > Signed-off-by: Lukas Wunner <lukas@xxxxxxxxx>
>>
>> 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@xxxxxxxxxxxxxxx>
>> 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.
With legacy kms the only way to keep a crtc enabled is to display a
drm_framebuffer on it. And drm_mode_config_cleanup has a WARN_ON if
framebuffers are left behind. There's a bunch of options:
- nouveau somehow manages to keep the crtc on without a framebuffer
- nouveau somehow leaks a drm_framebuffer, but removes it from the fb_list
- something else
There's still no need to forcefully shut down crtc at cleanup time in
the core, this is still a driver bug. So yes your patch might be
needed, but it's not the right fix.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
