Commit d6dd6843 authored by Paulo Zanoni's avatar Paulo Zanoni Committed by Jani Nikula

drm/i915: fix plane/cursor handling when runtime suspended

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@vger.kernel.org
Signed-off-by: default avatarPaulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: default avatarVille Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: default avatarJani Nikula <jani.nikula@intel.com>
parent dfb3d47b
...@@ -2233,6 +2233,15 @@ intel_pin_and_fence_fb_obj(struct drm_device *dev, ...@@ -2233,6 +2233,15 @@ intel_pin_and_fence_fb_obj(struct drm_device *dev,
if (need_vtd_wa(dev) && alignment < 256 * 1024) if (need_vtd_wa(dev) && alignment < 256 * 1024)
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; dev_priv->mm.interruptible = false;
ret = i915_gem_object_pin_to_display_plane(obj, alignment, pipelined); ret = i915_gem_object_pin_to_display_plane(obj, alignment, pipelined);
if (ret) if (ret)
...@@ -2250,12 +2259,14 @@ intel_pin_and_fence_fb_obj(struct drm_device *dev, ...@@ -2250,12 +2259,14 @@ intel_pin_and_fence_fb_obj(struct drm_device *dev,
i915_gem_object_pin_fence(obj); i915_gem_object_pin_fence(obj);
dev_priv->mm.interruptible = true; dev_priv->mm.interruptible = true;
intel_runtime_pm_put(dev_priv);
return 0; return 0;
err_unpin: err_unpin:
i915_gem_object_unpin_from_display_plane(obj); i915_gem_object_unpin_from_display_plane(obj);
err_interruptible: err_interruptible:
dev_priv->mm.interruptible = true; dev_priv->mm.interruptible = true;
intel_runtime_pm_put(dev_priv);
return ret; return ret;
} }
...@@ -8240,6 +8251,15 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc, ...@@ -8240,6 +8251,15 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc,
goto fail_locked; 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 /* Note that the w/a also requires 2 PTE of padding following
* the bo. We currently fill all unused PTE with the shadow * the bo. We currently fill all unused PTE with the shadow
* page and so we should always have valid PTE following the * page and so we should always have valid PTE following the
...@@ -8252,16 +8272,20 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc, ...@@ -8252,16 +8272,20 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc,
ret = i915_gem_object_pin_to_display_plane(obj, alignment, NULL); ret = i915_gem_object_pin_to_display_plane(obj, alignment, NULL);
if (ret) { if (ret) {
DRM_DEBUG_KMS("failed to move cursor bo into the GTT\n"); DRM_DEBUG_KMS("failed to move cursor bo into the GTT\n");
intel_runtime_pm_put(dev_priv);
goto fail_locked; goto fail_locked;
} }
ret = i915_gem_object_put_fence(obj); ret = i915_gem_object_put_fence(obj);
if (ret) { if (ret) {
DRM_DEBUG_KMS("failed to release fence for cursor"); DRM_DEBUG_KMS("failed to release fence for cursor");
intel_runtime_pm_put(dev_priv);
goto fail_unpin; goto fail_unpin;
} }
addr = i915_gem_obj_ggtt_offset(obj); addr = i915_gem_obj_ggtt_offset(obj);
intel_runtime_pm_put(dev_priv);
} else { } else {
int align = IS_I830(dev) ? 16 * 1024 : 256; int align = IS_I830(dev) ? 16 * 1024 : 256;
ret = i915_gem_object_attach_phys(obj, align); ret = i915_gem_object_attach_phys(obj, align);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment