drm/atomic-helper: Fix leak in disable_all
authorDaniel Vetter <daniel.vetter@ffwll.ch>
Sat, 15 Jul 2017 09:31:06 +0000 (11:31 +0200)
committerDaniel Vetter <daniel.vetter@ffwll.ch>
Wed, 19 Jul 2017 12:29:25 +0000 (14:29 +0200)
commit49d70aeaeca8f62b72b7712ecd1e29619a445866
treef78caf5e86bc26e0deb62d5818455b9dc8c85a11
parent1a0c19248a2f692af352f8b4b3e9b7f8a43bfd52
drm/atomic-helper: Fix leak in disable_all

The legacy plane->fb pointer is refcounted by calling
drm_atomic_clean_old_fb().

In practice this isn't a real problem because:
- The caller in the i915 gpu reset code restores the original state
  again, which means the plane->fb pointer won't change, hence can't
  leak.
- Drivers using drm_atomic_helper_shutdown call the fbdev cleanup
  first, and that usually cleans up the fb through
  drm_remove_framebuffer, which does this correctly.
- Without fbdev the only framebuffers are from userspace, and those
  get cleaned up (again using drm_remove_framebuffer) befor the driver
  can even be unloaded.

But in i915 I've switched the cleanup sequence around so that the
_shutdown() calls happens after the drm_remove_framebuffer(), which is
how I discovered this issue.

v2: My analysis why the current code was ok for gpu reset and
suspend/resume was correct, but then I totally failed to realize that
we better keep this symmetric. Thanksfully CI noticed that for
balance, a refcounting bug must exist at 2 places if previously there
was no issue ...

v3: Don't be lazy and compute the plane_mask in
commit_duplicated_state properly too, instead of just using ~0U.

Cc: martin.peres@free.fr
Cc: chris@chris-wilson.co.uk
Acked-by: Dave Airlie <airlied@gmail.com> (v1)
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20170715093106.19873-1-daniel.vetter@ffwll.ch
drivers/gpu/drm/drm_atomic_helper.c