[PATCH] drm/i915: fix plane/cursor handling when runtime suspended

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>

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.

We need to get runtime PM references to pin the objects, and to
change the fences. The pin 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 written are forwarded to system memory.

Note: for a complete fix of the cursor-dpms test case, we also need
the patch named "drm/i915: Don't try to enable cursor from setplane
when crtc is disabled".

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.
v4: - Remove spurious whitespace (Ville).
v5: - Remove intel_crtc_update_cursor() chunk since Ville did an
      equivalent fix in another patch (Ville).
v6: - Remove unpin chunk: it will be on a separate patch (Ville,
      Chris, Daniel).
v7: - Same thing, new color.

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
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82603
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
---
 drivers/gpu/drm/i915/intel_display.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3f8e037..15fe3eb 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2201,6 +2201,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)
@@ -2218,12 +2227,14 @@ 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;
 }
 
@@ -8253,6 +8264,7 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc,
 				     uint32_t width, uint32_t height)
 {
 	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	enum pipe pipe = intel_crtc->pipe;
 	unsigned old_width, stride;
@@ -8292,6 +8304,15 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc,
 			goto fail_locked;
 		}
 
+		/*
+		 * 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);
+
 		/* Note that the w/a also requires 2 PTE of padding following
 		 * the bo. We currently fill all unused PTE with the shadow
 		 * page and so we should always have valid PTE following the
@@ -8304,16 +8325,20 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc,
 		ret = i915_gem_object_pin_to_display_plane(obj, alignment, NULL);
 		if (ret) {
 			DRM_DEBUG_KMS("failed to move cursor bo into the GTT\n");
+			intel_runtime_pm_put(dev_priv);
 			goto fail_locked;
 		}
 
 		ret = i915_gem_object_put_fence(obj);
 		if (ret) {
 			DRM_DEBUG_KMS("failed to release fence for cursor");
+			intel_runtime_pm_put(dev_priv);
 			goto fail_unpin;
 		}
 
 		addr = i915_gem_obj_ggtt_offset(obj);
+
+		intel_runtime_pm_put(dev_priv);
 	} else {
 		int align = IS_I830(dev) ? 16 * 1024 : 256;
 		ret = i915_gem_object_attach_phys(obj, align);
-- 
2.0.1

--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]