aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorLyude <cpaul@redhat.com>2016-08-17 15:55:54 -0400
committerJani Nikula <jani.nikula@intel.com>2016-08-22 09:07:44 -0400
commitf403372658fc7b652a77885a4141e58e57d9c75a (patch)
treee1688b385fdf1ae2cf9a959f8d9f8d8e0af1e0d8
parent5bc6abe7674d9cf41dbcdaaf98a19184da181439 (diff)
drm/i915/skl: Add support for the SAGV, fix underrun hangs
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. This will still need additional work in the future: we shouldn't be enabling the SAGV if any of the currently enabled planes can't enable WM levels that introduce latencies >= 30 µs. Changes since v11: - Add skl_can_enable_sagv() - Make sure we don't enable SAGV when not all planes can enable watermarks >= the SAGV engine block time. I was originally going to save this for later, but I recently managed to run into a machine that was having problems with a single pipe configuration + SAGV. - Make comparisons to I915_SKL_SAGV_NOT_CONTROLLED explicit - Change I915_SAGV_DYNAMIC_FREQ to I915_SAGV_ENABLE - Move printks outside of mutexes - Don't print error messages twice Changes since v10: - Apparently sandybridge_pcode_read actually writes values and reads them back, despite it's misleading function name. This means we've been doing this mostly wrong and have been writing garbage to the SAGV control. Because of this, we no longer attempt to read the SAGV status during initialization (since there are no helpers for this). - mlankhorst noticed that this patch was breaking on some very early pre-release Skylake machines, which apparently don't allow you to disable the SAGV. To prevent machines from failing tests due to SAGV errors, if the first time we try to control the SAGV results in the mailbox indicating an invalid command, we just disable future attempts to control the SAGV state by setting dev_priv->skl_sagv_status to I915_SKL_SAGV_NOT_CONTROLLED and make a note of it in dmesg. - Move mutex_unlock() a little higher in skl_enable_sagv(). This doesn't actually fix anything, but lets us release the lock a little sooner since we're finished with it. Changes since v9: - Only enable/disable sagv on Skylake Changes since v8: - Add intel_state->modeset guard to the conditional for skl_enable_sagv() Changes since v7: - Remove GEN9_SAGV_LOW_FREQ, replace with GEN9_SAGV_IS_ENABLED (that's all we use it for anyway) - Use GEN9_SAGV_IS_ENABLED instead of 0x1 for clarification - Fix a styling error that snuck past me Changes since v6: - Protect skl_enable_sagv() with intel_state->modeset conditional in intel_atomic_commit_tail() Changes since v5: - Don't use is_power_of_2. Makes things confusing - Don't use the old state to figure out whether or not to enable/disable the sagv, use the new one - Split the loop in skl_disable_sagv into it's own function - Move skl_sagv_enable/disable() calls into intel_atomic_commit_tail() Changes since v4: - Use is_power_of_2 against active_crtcs to check whether we have > 1 pipe enabled - Fix skl_sagv_get_hw_state(): (temp & 0x1) indicates disabled, 0x0 enabled - Call skl_sagv_enable/disable() from pre/post-plane updates Changes since v3: - Use time_before() to compare timeout to jiffies 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 Signed-off-by: Lyude <cpaul@redhat.com> Cc: Matt Roper <matthew.d.roper@intel.com> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: stable@vger.kernel.org Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1471463761-26796-3-git-send-email-cpaul@redhat.com [mlankhorst: ENOSYS -> ENXIO, whitespace fixes] (cherry picked from commit 656d1b89e5ffb83036ab0e2a24be7558f34365c7) Signed-off-by: Jani Nikula <jani.nikula@intel.com>
-rw-r--r--drivers/gpu/drm/i915/i915_drv.h7
-rw-r--r--drivers/gpu/drm/i915/i915_reg.h4
-rw-r--r--drivers/gpu/drm/i915/intel_display.c11
-rw-r--r--drivers/gpu/drm/i915/intel_drv.h3
-rw-r--r--drivers/gpu/drm/i915/intel_pm.c148
5 files changed, 173 insertions, 0 deletions
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 46e372ece6eb..f68c78918d63 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1964,6 +1964,13 @@ struct drm_i915_private {
1964 struct i915_suspend_saved_registers regfile; 1964 struct i915_suspend_saved_registers regfile;
1965 struct vlv_s0ix_state vlv_s0ix_state; 1965 struct vlv_s0ix_state vlv_s0ix_state;
1966 1966
1967 enum {
1968 I915_SKL_SAGV_UNKNOWN = 0,
1969 I915_SKL_SAGV_DISABLED,
1970 I915_SKL_SAGV_ENABLED,
1971 I915_SKL_SAGV_NOT_CONTROLLED
1972 } skl_sagv_status;
1973
1967 struct { 1974 struct {
1968 /* 1975 /*
1969 * Raw watermark latency values: 1976 * Raw watermark latency values:
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 2b9508dda759..bf2cad3f9e1f 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7175,6 +7175,10 @@ enum {
7175#define HSW_PCODE_DE_WRITE_FREQ_REQ 0x17 7175#define HSW_PCODE_DE_WRITE_FREQ_REQ 0x17
7176#define DISPLAY_IPS_CONTROL 0x19 7176#define DISPLAY_IPS_CONTROL 0x19
7177#define HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL 0x1A 7177#define HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL 0x1A
7178#define GEN9_PCODE_SAGV_CONTROL 0x21
7179#define GEN9_SAGV_DISABLE 0x0
7180#define GEN9_SAGV_IS_DISABLED 0x1
7181#define GEN9_SAGV_ENABLE 0x3
7178#define GEN6_PCODE_DATA _MMIO(0x138128) 7182#define GEN6_PCODE_DATA _MMIO(0x138128)
7179#define GEN6_PCODE_FREQ_IA_RATIO_SHIFT 8 7183#define GEN6_PCODE_FREQ_IA_RATIO_SHIFT 8
7180#define GEN6_PCODE_FREQ_RING_RATIO_SHIFT 16 7184#define GEN6_PCODE_FREQ_RING_RATIO_SHIFT 16
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2a751b6e0253..175595fc3e45 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13759,6 +13759,13 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
13759 intel_state->cdclk_pll_vco != dev_priv->cdclk_pll.vco)) 13759 intel_state->cdclk_pll_vco != dev_priv->cdclk_pll.vco))
13760 dev_priv->display.modeset_commit_cdclk(state); 13760 dev_priv->display.modeset_commit_cdclk(state);
13761 13761
13762 /*
13763 * SKL workaround: bspec recommends we disable the SAGV when we
13764 * have more then one pipe enabled
13765 */
13766 if (IS_SKYLAKE(dev_priv) && !skl_can_enable_sagv(state))
13767 skl_disable_sagv(dev_priv);
13768
13762 intel_modeset_verify_disabled(dev); 13769 intel_modeset_verify_disabled(dev);
13763 } 13770 }
13764 13771
@@ -13832,6 +13839,10 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
13832 intel_modeset_verify_crtc(crtc, old_crtc_state, crtc->state); 13839 intel_modeset_verify_crtc(crtc, old_crtc_state, crtc->state);
13833 } 13840 }
13834 13841
13842 if (IS_SKYLAKE(dev_priv) && intel_state->modeset &&
13843 skl_can_enable_sagv(state))
13844 skl_enable_sagv(dev_priv);
13845
13835 drm_atomic_helper_commit_hw_done(state); 13846 drm_atomic_helper_commit_hw_done(state);
13836 13847
13837 if (intel_state->modeset) 13848 if (intel_state->modeset)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index cc937a19b1ba..ff399b9a5c1f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1716,6 +1716,9 @@ void ilk_wm_get_hw_state(struct drm_device *dev);
1716void skl_wm_get_hw_state(struct drm_device *dev); 1716void skl_wm_get_hw_state(struct drm_device *dev);
1717void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv, 1717void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
1718 struct skl_ddb_allocation *ddb /* out */); 1718 struct skl_ddb_allocation *ddb /* out */);
1719bool skl_can_enable_sagv(struct drm_atomic_state *state);
1720int skl_enable_sagv(struct drm_i915_private *dev_priv);
1721int skl_disable_sagv(struct drm_i915_private *dev_priv);
1719uint32_t ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config); 1722uint32_t ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config);
1720bool ilk_disable_lp_wm(struct drm_device *dev); 1723bool ilk_disable_lp_wm(struct drm_device *dev);
1721int sanitize_rc6_option(struct drm_i915_private *dev_priv, int enable_rc6); 1724int sanitize_rc6_option(struct drm_i915_private *dev_priv, int enable_rc6);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 2c88c2f50d02..496a911e5668 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2852,6 +2852,7 @@ bool ilk_disable_lp_wm(struct drm_device *dev)
2852 2852
2853#define SKL_DDB_SIZE 896 /* in blocks */ 2853#define SKL_DDB_SIZE 896 /* in blocks */
2854#define BXT_DDB_SIZE 512 2854#define BXT_DDB_SIZE 512
2855#define SKL_SAGV_BLOCK_TIME 30 /* µs */
2855 2856
2856/* 2857/*
2857 * Return the index of a plane in the SKL DDB and wm result arrays. Primary 2858 * Return the index of a plane in the SKL DDB and wm result arrays. Primary
@@ -2875,6 +2876,153 @@ skl_wm_plane_id(const struct intel_plane *plane)
2875 } 2876 }
2876} 2877}
2877 2878
2879/*
2880 * SAGV dynamically adjusts the system agent voltage and clock frequencies
2881 * depending on power and performance requirements. The display engine access
2882 * to system memory is blocked during the adjustment time. Because of the
2883 * blocking time, having this enabled can cause full system hangs and/or pipe
2884 * underruns if we don't meet all of the following requirements:
2885 *
2886 * - <= 1 pipe enabled
2887 * - All planes can enable watermarks for latencies >= SAGV engine block time
2888 * - We're not using an interlaced display configuration
2889 */
2890int
2891skl_enable_sagv(struct drm_i915_private *dev_priv)
2892{
2893 int ret;
2894
2895 if (dev_priv->skl_sagv_status == I915_SKL_SAGV_NOT_CONTROLLED ||
2896 dev_priv->skl_sagv_status == I915_SKL_SAGV_ENABLED)
2897 return 0;
2898
2899 DRM_DEBUG_KMS("Enabling the SAGV\n");
2900 mutex_lock(&dev_priv->rps.hw_lock);
2901
2902 ret = sandybridge_pcode_write(dev_priv, GEN9_PCODE_SAGV_CONTROL,
2903 GEN9_SAGV_ENABLE);
2904
2905 /* We don't need to wait for the SAGV when enabling */
2906 mutex_unlock(&dev_priv->rps.hw_lock);
2907
2908 /*
2909 * Some skl systems, pre-release machines in particular,
2910 * don't actually have an SAGV.
2911 */
2912 if (ret == -ENXIO) {
2913 DRM_DEBUG_DRIVER("No SAGV found on system, ignoring\n");
2914 dev_priv->skl_sagv_status = I915_SKL_SAGV_NOT_CONTROLLED;
2915 return 0;
2916 } else if (ret < 0) {
2917 DRM_ERROR("Failed to enable the SAGV\n");
2918 return ret;
2919 }
2920
2921 dev_priv->skl_sagv_status = I915_SKL_SAGV_ENABLED;
2922 return 0;
2923}
2924
2925static int
2926skl_do_sagv_disable(struct drm_i915_private *dev_priv)
2927{
2928 int ret;
2929 uint32_t temp = GEN9_SAGV_DISABLE;
2930
2931 ret = sandybridge_pcode_read(dev_priv, GEN9_PCODE_SAGV_CONTROL,
2932 &temp);
2933 if (ret)
2934 return ret;
2935 else
2936 return temp & GEN9_SAGV_IS_DISABLED;
2937}
2938
2939int
2940skl_disable_sagv(struct drm_i915_private *dev_priv)
2941{
2942 int ret, result;
2943
2944 if (dev_priv->skl_sagv_status == I915_SKL_SAGV_NOT_CONTROLLED ||
2945 dev_priv->skl_sagv_status == I915_SKL_SAGV_DISABLED)
2946 return 0;
2947
2948 DRM_DEBUG_KMS("Disabling the SAGV\n");
2949 mutex_lock(&dev_priv->rps.hw_lock);
2950
2951 /* bspec says to keep retrying for at least 1 ms */
2952 ret = wait_for(result = skl_do_sagv_disable(dev_priv), 1);
2953 mutex_unlock(&dev_priv->rps.hw_lock);
2954
2955 if (ret == -ETIMEDOUT) {
2956 DRM_ERROR("Request to disable SAGV timed out\n");
2957 return -ETIMEDOUT;
2958 }
2959
2960 /*
2961 * Some skl systems, pre-release machines in particular,
2962 * don't actually have an SAGV.
2963 */
2964 if (result == -ENXIO) {
2965 DRM_DEBUG_DRIVER("No SAGV found on system, ignoring\n");
2966 dev_priv->skl_sagv_status = I915_SKL_SAGV_NOT_CONTROLLED;
2967 return 0;
2968 } else if (result < 0) {
2969 DRM_ERROR("Failed to disable the SAGV\n");
2970 return result;
2971 }
2972
2973 dev_priv->skl_sagv_status = I915_SKL_SAGV_DISABLED;
2974 return 0;
2975}
2976
2977bool skl_can_enable_sagv(struct drm_atomic_state *state)
2978{
2979 struct drm_device *dev = state->dev;
2980 struct drm_i915_private *dev_priv = to_i915(dev);
2981 struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
2982 struct drm_crtc *crtc;
2983 enum pipe pipe;
2984 int level, plane;
2985
2986 /*
2987 * SKL workaround: bspec recommends we disable the SAGV when we have
2988 * more then one pipe enabled
2989 *
2990 * If there are no active CRTCs, no additional checks need be performed
2991 */
2992 if (hweight32(intel_state->active_crtcs) == 0)
2993 return true;
2994 else if (hweight32(intel_state->active_crtcs) > 1)
2995 return false;
2996
2997 /* Since we're now guaranteed to only have one active CRTC... */
2998 pipe = ffs(intel_state->active_crtcs) - 1;
2999 crtc = dev_priv->pipe_to_crtc_mapping[pipe];
3000
3001 if (crtc->state->mode.flags & DRM_MODE_FLAG_INTERLACE)
3002 return false;
3003
3004 for_each_plane(dev_priv, pipe, plane) {
3005 /* Skip this plane if it's not enabled */
3006 if (intel_state->wm_results.plane[pipe][plane][0] == 0)
3007 continue;
3008
3009 /* Find the highest enabled wm level for this plane */
3010 for (level = ilk_wm_max_level(dev);
3011 intel_state->wm_results.plane[pipe][plane][level] == 0; --level)
3012 { }
3013
3014 /*
3015 * If any of the planes on this pipe don't enable wm levels
3016 * that incur memory latencies higher then 30µs we can't enable
3017 * the SAGV
3018 */
3019 if (dev_priv->wm.skl_latency[level] < SKL_SAGV_BLOCK_TIME)
3020 return false;
3021 }
3022
3023 return true;
3024}
3025
2878static void 3026static void
2879skl_ddb_get_pipe_allocation_limits(struct drm_device *dev, 3027skl_ddb_get_pipe_allocation_limits(struct drm_device *dev,
2880 const struct intel_crtc_state *cstate, 3028 const struct intel_crtc_state *cstate,