From 942840371cde152fe57c15e0e8483b760e7763e3 Mon Sep 17 00:00:00 2001 From: Matt Roper Date: Mon, 21 Sep 2015 17:21:48 -0700 Subject: [PATCH] drm/fbdev: Update legacy plane->fb refcounting for atomic restore Starting with commit commit 28cc504e8d52248962f5b485bdc65f539e3fe21d Author: Rob Clark Date: Tue Aug 25 15:36:00 2015 -0400 drm/i915: enable atomic fb-helper I've been seeing some panics on i915 when the DRM master shuts down that appear to be caused by using an already-freed framebuffer (i.e., we're unexpectedly dropping our initial FB's reference count to 0 and freeing it, which causes a crash when we try to restore it later). Digging deeper, the state FB refcounting is working as expected, but we seem to be missing proper refcounting on the legacy plane->fb pointers in the new atomic fbdev code. Tracking plane->old_fb and then doing a ref/unref at the end of the fbdev restore like we do in the legacy ioctl's ensures we don't miscount references on plane->fb and avoids the panics. v2 from Daniel: Really do what the atomic ioctl does: - Also update plane->fb and plane->crtc. - Clear out plane->old_fb on failures too. v3: git add everything. Oops. v4: Also clear old_fb in all other failure paths, spotted by David. Cc: Rob Clark Cc: intel-gfx@lists.freedesktop.org Cc: David Herrmann Cc: Maarten Lankhorst Signed-off-by: Matt Roper (v1) Reviewd-by: David Herrmann Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_fb_helper.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 64fc5ca8fda2..abe9793d548d 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -352,6 +352,8 @@ retry: drm_for_each_plane(plane, dev) { struct drm_plane_state *plane_state; + plane->old_fb = plane->fb; + plane_state = drm_atomic_get_plane_state(state, plane); if (IS_ERR(plane_state)) { ret = PTR_ERR(plane_state); @@ -382,16 +384,27 @@ retry: } ret = drm_atomic_commit(state); - if (ret != 0) - goto fail; - - return 0; fail: + drm_for_each_plane(plane, dev) { + if (ret == 0) { + struct drm_framebuffer *new_fb = plane->state->fb; + if (new_fb) + drm_framebuffer_reference(new_fb); + plane->fb = new_fb; + plane->crtc = plane->state->crtc; + + if (plane->old_fb) + drm_framebuffer_unreference(plane->old_fb); + } + plane->old_fb = NULL; + } + if (ret == -EDEADLK) goto backoff; - drm_atomic_state_free(state); + if (ret != 0) + drm_atomic_state_free(state); return ret; -- 2.20.1