public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: Paulo Zanoni <przanoni@gmail.com>
To: intel-gfx@lists.freedesktop.org
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>, stable@vger.kernel.org
Subject: [PATCH] drm/i915: fix plane/cursor handling when runtime suspended
Date: Wed,  6 Aug 2014 18:26:01 -0300	[thread overview]
Message-ID: <1407360362-1774-1-git-send-email-przanoni@gmail.com> (raw)

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

If we're runtime suspended and try to use the plane interfaces, we
will get a lot of WARNs saying we did the wrong thing.

For intel_crtc_update_cursor(), all we need to do is return if the
CRTC is not active, since writing the registers won't really have any
effect if the screen is not visible, and we will write the registers
later when enabling the screen.

For all the other cases, we need to get runtime PM references to
pin/unpin the objects, and to change the fences. The pin/unpin
functions are the ideal places for this, but
intel_crtc_cursor_set_obj() doesn't call them, so we also have to add
get/put calls inside it. There is no problem if we runtime suspend
right after these functions are finished, because the registers
weitten are forwarded to system memory.

v2: - Narrow the put/get calls on intel_crtc_cursor_set_obj() (Daniel)
v3: - Make get/put also surround the fence and unpin calls (Daniel and
      Ville).
    - Merge all the plane changes into a single patch since they're
      the same fix.
    - Add the comment requested by Daniel.

Testcase: igt/pm_rpm/cursor
Testcase: igt/pm_rpm/cursor-dpms
Testcase: igt/pm_rpm/legacy-planes
Testcase: igt/pm_rpm/legacy-planes-dpms
Testcase: igt/pm_rpm/universal-planes
Testcase: igt/pm_rpm/universal-planes-dpms
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=81645
Cc: stable@vger.kernel.org
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 39 +++++++++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4f659eb..a86d67c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2212,6 +2212,15 @@ intel_pin_and_fence_fb_obj(struct drm_device *dev,
 	if (need_vtd_wa(dev) && alignment < 256 * 1024)
 		alignment = 256 * 1024;
 
+	/*
+	 * Global gtt pte registers are special registers which actually forward
+	 * writes to a chunk of system memory. Which means that there is no risk
+	 * that the register values disappear as soon as we call
+	 * intel_runtime_pm_put(), so it is correct to wrap only the
+	 * pin/unpin/fence and not more.
+	 */
+	intel_runtime_pm_get(dev_priv);
+
 	dev_priv->mm.interruptible = false;
 	ret = i915_gem_object_pin_to_display_plane(obj, alignment, pipelined);
 	if (ret)
@@ -2229,21 +2238,30 @@ intel_pin_and_fence_fb_obj(struct drm_device *dev,
 	i915_gem_object_pin_fence(obj);
 
 	dev_priv->mm.interruptible = true;
+	intel_runtime_pm_put(dev_priv);
 	return 0;
 
 err_unpin:
 	i915_gem_object_unpin_from_display_plane(obj);
 err_interruptible:
 	dev_priv->mm.interruptible = true;
+	intel_runtime_pm_put(dev_priv);
 	return ret;
 }
 
 void intel_unpin_fb_obj(struct drm_i915_gem_object *obj)
 {
-	WARN_ON(!mutex_is_locked(&obj->base.dev->struct_mutex));
+	struct drm_device *dev = obj->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
+
+	intel_runtime_pm_get(dev_priv);
 
 	i915_gem_object_unpin_fence(obj);
 	i915_gem_object_unpin_from_display_plane(obj);
+
+	intel_runtime_pm_put(dev_priv);
 }
 
 /* Computes the linear offset to the base tile and adjusts x, y. bytes per pixel
@@ -8285,6 +8303,9 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
 	if (base == 0 && intel_crtc->cursor_base == 0)
 		return;
 
+	if (!intel_crtc->active)
+		return;
+
 	I915_WRITE(CURPOS(pipe), pos);
 
 	if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev) || IS_BROADWELL(dev))
@@ -8340,9 +8361,20 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc,
 
 	/* we only need to pin inside GTT if cursor is non-phy */
 	mutex_lock(&dev->struct_mutex);
+
+	/*
+	 * Global gtt pte registers are special registers which actually forward
+	 * writes to a chunk of system memory. Which means that there is no risk
+	 * that the register values disappear as soon as we call
+	 * intel_runtime_pm_put(), so it is correct to wrap only the
+	 * pin/unpin/fence and not more.
+	 */
+	intel_runtime_pm_get(dev_priv);
+
 	if (!INTEL_INFO(dev)->cursor_needs_physical) {
 		unsigned alignment;
 
+
 		if (obj->tiling_mode) {
 			DRM_DEBUG_KMS("cursor cannot be tiled\n");
 			ret = -EINVAL;
@@ -8392,6 +8424,10 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc,
 
 	i915_gem_track_fb(intel_crtc->cursor_bo, obj,
 			  INTEL_FRONTBUFFER_CURSOR(pipe));
+
+	if (obj)
+		intel_runtime_pm_put(dev_priv);
+
 	mutex_unlock(&dev->struct_mutex);
 
 	old_width = intel_crtc->cursor_width;
@@ -8413,6 +8449,7 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc,
 fail_unpin:
 	i915_gem_object_unpin_from_display_plane(obj);
 fail_locked:
+	intel_runtime_pm_put(dev_priv);
 	mutex_unlock(&dev->struct_mutex);
 fail:
 	drm_gem_object_unreference_unlocked(&obj->base);
-- 
2.0.1

             reply	other threads:[~2014-08-06 21:26 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-06 21:26 Paulo Zanoni [this message]
2014-08-06 21:26 ` [PATCH] tests/pm_rpm: add subtests for planes and cursors Paulo Zanoni
2014-08-11 11:32 ` [Intel-gfx] [PATCH] drm/i915: fix plane/cursor handling when runtime suspended Ville Syrjälä
2014-08-11 14:29   ` Paulo Zanoni
2014-08-11 14:29     ` Paulo Zanoni
2014-08-11 14:42     ` [Intel-gfx] " Ville Syrjälä
2014-08-11 17:57       ` Paulo Zanoni
2014-08-12  8:20         ` Ville Syrjälä
2014-08-12 18:55           ` Paulo Zanoni
2014-08-12 19:09             ` Chris Wilson
2014-08-12 19:12               ` [Intel-gfx] " Paulo Zanoni
2014-08-12 19:28                 ` Chris Wilson
2014-08-12 19:33                   ` Paulo Zanoni
2014-08-12 20:19                     ` Daniel Vetter
2014-08-12 20:30                       ` [Intel-gfx] " Chris Wilson
2014-08-12 20:37                         ` Daniel Vetter
2014-08-12 20:51                           ` [Intel-gfx] " Chris Wilson
2014-08-13  7:59                             ` Daniel Vetter
2014-08-14 15:06                               ` Paulo Zanoni
2014-08-14 20:00                                 ` Paulo Zanoni
2014-08-15  8:39                                 ` [Intel-gfx] " Ville Syrjälä
2014-08-15 16:47                                   ` Paulo Zanoni
2014-08-15 16:55                                     ` Ville Syrjälä
2014-08-15 18:59                                       ` Paulo Zanoni
2014-08-18 14:35                                         ` Ville Syrjälä
2014-08-26 12:17                                         ` [Intel-gfx] " Jani Nikula

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=1407360362-1774-1-git-send-email-przanoni@gmail.com \
    --to=przanoni@gmail.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=paulo.r.zanoni@intel.com \
    --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