All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matt Roper <matthew.d.roper@intel.com>
To: dri-devel@lists.freedesktop.org
Cc: intel-gfx@lists.freedesktop.org
Subject: [PATCH] drm/fbdev: Update legacy plane->fb refcounting for atomic restore
Date: Mon, 21 Sep 2015 17:21:48 -0700	[thread overview]
Message-ID: <1442881308-24940-1-git-send-email-matthew.d.roper@intel.com> (raw)

Starting with commit

        commit 28cc504e8d52248962f5b485bdc65f539e3fe21d
        Author: Rob Clark <robdclark@gmail.com>
        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.

Cc: Rob Clark <robdclark@gmail.com>
Cc: intel-gfx@lists.freedesktop.org
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
I don't see any existing bugzilla's (or mailing list complaints) for this,
which surprises me a bit since it seems to be very easy to reproduce for me on
latest drm-intel-nightly running on IVB...boot the system, load X, kill X,
panic.

 drivers/gpu/drm/drm_fb_helper.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 64fc5ca..2149ac7 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);
@@ -385,6 +387,14 @@ retry:
 	if (ret != 0)
 		goto fail;
 
+	drm_for_each_plane(plane, dev) {
+		if (plane->fb)
+			drm_framebuffer_reference(plane->fb);
+		if (plane->old_fb)
+			drm_framebuffer_unreference(plane->old_fb);
+		plane->old_fb = NULL;
+	}
+
 	return 0;
 
 fail:
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

             reply	other threads:[~2015-09-22  0:21 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-22  0:21 Matt Roper [this message]
2015-09-22  9:55 ` [PATCH] drm/fbdev: Update legacy plane->fb refcounting for atomic restore Daniel Vetter
2015-09-22 10:33   ` Maarten Lankhorst
2015-09-22 10:34     ` David Herrmann
2015-09-22 10:33   ` David Herrmann
2015-09-22 10:56 ` Daniel Vetter
2015-09-22 11:02   ` David Herrmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1442881308-24940-1-git-send-email-matthew.d.roper@intel.com \
    --to=matthew.d.roper@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.