On Tue, Jul 12, 2016 at 11:21:39AM -0700, Matt Roper wrote:
On Tue, Jul 12, 2016 at 01:36:03PM -0400, Lyude wrote:
Since the watermark calculations for Skylake are still broken, we're apt to hitting underruns very easily under multi-monitor configurations. While it would be lovely if this was fixed, it's not. Another problem that's been coming from this however, is the mysterious issue of underruns causing full system hangs. An easy way to reproduce this with a skylake system:
- Get a laptop with a skylake GPU, and hook up two external monitors to it
- Move the cursor from the built-in LCD to one of the external displays as quickly as you can
- You'll get a few pipe underruns, and eventually the entire system will just freeze.
After doing a lot of investigation and reading through the bspec, I found the existence of the SAGV, which is responsible for adjusting the system agent voltage and clock frequencies depending on how much power we need. According to the bspec:
"The display engine access to system memory is blocked during the adjustment time. SAGV defaults to enabled. Software must use the GT-driver pcode mailbox to disable SAGV when the display engine is not able to tolerate the blocking time."
The rest of the bspec goes on to explain that software can simply leave the SAGV enabled, and disable it when we use interlaced pipes/have more then one pipe active.
Sure enough, with this patchset the system hangs resulting from pipe underruns on Skylake have completely vanished on my T460s. Additionally, the bspec mentions turning off the SAGV with more then one pipe enabled as a workaround for display underruns. While this patch doesn't entirely fix that, it looks like it does improve the situation a little bit so it's likely this is going to be required to make watermarks on Skylake fully functional.
Changes since v2:
- Really apply minor style nitpicks to patch this time
Changes since v1:
- Added comments about this probably being one of the requirements to fixing Skylake's watermark issues
- Minor style nitpicks from Matt Roper
- Disable these functions on Broxton, since it doesn't have an SAGV
Cc: Matt Roper matthew.d.roper@intel.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Ville Syrjälä ville.syrjala@linux.intel.com Signed-off-by: Lyude cpaul@redhat.com
I don't have a SKL to try this out on (only BXT here), but this matches my interpretation of the current bspec text, so
Reviewed-by: Matt Roper matthew.d.roper@intel.com
I think this also applies to
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94625
And maybe a +cc stable?
Oops, one more minor issue below that I missed the first time around.
...
- /* bspec says to keep retrying for at least 1 ms */
- timeout = jiffies + msecs_to_jiffies(1);
- do {
ret = sandybridge_pcode_write(dev_priv, GEN9_PCODE_SAGV_CONTROL,GEN9_SAGV_DISABLE);if (ret) {DRM_ERROR("Failed to disable the SAGV\n");goto out;}ret = sandybridge_pcode_read(dev_priv, GEN9_PCODE_SAGV_CONTROL,&temp);if (ret) {DRM_ERROR("Failed to check the status of the SAGV\n");goto out;}- } while (!(temp & 0x1) && jiffies < timeout);
We shouldn't compare jiffies directly here due to wraparound; you can use time_before() to handle that safely.
Matt
- if (temp & 0x1) {
dev_priv->skl_sagv_enabled = false;- } else {
ret = -1;DRM_ERROR("Request to disable SAGV timed out\n");- }
+out:
- mutex_unlock(&dev_priv->rps.hw_lock);
- return ret;
+}
+static void skl_ddb_get_pipe_allocation_limits(struct drm_device *dev, const struct intel_crtc_state *cstate, struct skl_ddb_entry *alloc, /* out */ @@ -3686,6 +3789,11 @@ static void skl_write_wm_values(struct drm_i915_private *dev_priv, struct drm_device *dev = &dev_priv->drm; struct intel_crtc *crtc;
- if (dev_priv->active_crtcs == 1)
skl_enable_sagv(dev_priv);- else
skl_disable_sagv(dev_priv);- for_each_intel_crtc(dev, crtc) { int i, level, max_level = ilk_wm_max_level(dev); enum pipe pipe = crtc->pipe;
@@ -4228,6 +4336,8 @@ void skl_wm_get_hw_state(struct drm_device *dev) /* Easy/common case; just sanitize DDB now if everything off */ memset(ddb, 0, sizeof(*ddb)); }
- skl_sagv_get_hw_state(dev_priv);
}
static void ilk_pipe_wm_get_hw_state(struct drm_crtc *crtc)
2.7.4
-- Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx