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

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni at 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.

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).

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 at vger.kernel.org
Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)


I talked with Daniel and we agreed to leave any possible fixes related with the
"unpin" functions to separate patches, possibly requiring separate IGT test
cases.

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3813526..17bc661 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2149,6 +2149,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)
@@ -2166,12 +2175,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;
 }
 
@@ -8201,6 +8212,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;
@@ -8231,6 +8243,16 @@ 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;
 
@@ -8280,6 +8302,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;
@@ -8301,6 +8327,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