dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: Dave Airlie <airlied@redhat.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	David Herrmann <dh.herrmann@gmail.com>,
	Daniel Vetter <daniel.vetter@ffwll.ch>,
	dri-devel@lists.freedesktop.org, stable@vger.kernel.org,
	Hans de Goede <hdegoede@redhat.com>,
	stable@vger.kenrel.org
Subject: [PATCH] Revert "drm/core: Preserve the framebuffer after removing it."
Date: Thu, 19 May 2016 17:15:01 +0200	[thread overview]
Message-ID: <1463670901-31153-1-git-send-email-hdegoede@redhat.com> (raw)

This reverts commit 13803132818c ("drm/core: Preserve the framebuffer
after removing it.").

This commit assumes that going through drm_framebuffer_remove() is not
necessary because "the fbdev code or any system compositor should restore
the planes anyway so there's no need to do it twice". But this is not true
for secondary GPUs / slave outputs.

This revert fixes the dgpu no longer suspending on laptops with
switchable graphics after an external output which is connected
to the dgpu has been used.

And it fixes the WARN_ON to detect drm_framebuffer leaks in
drm_mode_config_cleanup() triggering when unplugging an USB displaylink
device; or when rmmod-ing the secondary GPU kms driver on laptops with
switchable-graphics.

Also this part of the reverted commit's commit-msg: "The old fb_id is
zero'd, so there's no danger of being able to restore the fb from fb_id."
is no longer true, the zero-ing does not happen until drm_framebuffer_free
gets called, which does not happen until the last ref is dropped, so
if a crtc's primary->fb is still pointing to this fb, the id will not
get zero'd and userspace could potentially gain access to the removed
fb again.

Cc: stable@vger.kenrel.org
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/gpu/drm/drm_crtc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index e08f962..15f5cd7 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -3474,7 +3474,7 @@ int drm_mode_rmfb(struct drm_device *dev,
 	mutex_unlock(&dev->mode_config.fb_lock);
 	mutex_unlock(&file_priv->fbs_lock);
 
-	drm_framebuffer_unreference(fb);
+	drm_framebuffer_remove(fb);
 
 	return 0;
 
@@ -3656,8 +3656,8 @@ void drm_fb_release(struct drm_file *priv)
 	list_for_each_entry_safe(fb, tfb, &priv->fbs, filp_head) {
 		list_del_init(&fb->filp_head);
 
-		/* This drops the fpriv->fbs reference. */
-		drm_framebuffer_unreference(fb);
+		/* This will also drop the fpriv->fbs reference. */
+		drm_framebuffer_remove(fb);
 	}
 }
 
-- 
2.7.4

             reply	other threads:[~2016-05-19 15:15 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-19 15:15 Hans de Goede [this message]
2016-05-19 15:33 ` [PATCH] Revert "drm/core: Preserve the framebuffer after removing it." Daniel Vetter
2016-05-20  9:58   ` Hans de Goede

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=1463670901-31153-1-git-send-email-hdegoede@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=airlied@redhat.com \
    --cc=daniel.vetter@ffwll.ch \
    --cc=dh.herrmann@gmail.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=stable@vger.kenrel.org \
    --cc=stable@vger.kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).