[Intel-gfx] [PATCH] drm/i915: Increase WM memory latency values on SNB with high pixel clock

[Chris Wilson]

On Fri, Mar 21, 2014 at 11:00:48AM +0200, Jani Nikula wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> On SNB the BIOS provided WM memory latency values seem insufficient to
> handle high resolution displays.
> 
> In this particular case the display mode was a 2560x1440 at 60Hz, which
> makes the pixel clock 241.5 MHz. It was empirically found that a memory
> latency value if 1.2 usec is enough to avoid underruns, whereas the BIOS
> provided value of 0.7 usec was clearly too low. Incidentally 1.2 usec
> is what the typical BIOS provided values are on IVB systems.
> 
> Increase the WM memory latency values to 1.2 usec when encountering a
> display mode with pixel clock exceeding 225 MHz. 225 MHz was chosen as
> the threshold simply because that's the documented limit of SNB with
> HDMI, so one might surmise that the hardware may have been tested up
> to that frequency. In theory the latency shouldn't depend on the pixel
> clock at all. So it may be that we should just increase the latency
> values across the board for all SNB systems. But for now I'm inclined
> to limit this quirk to only those cases that are proven to need it,
> as doing otherwise might cause some increase in power consumption for
> everyone.
> 
> Cc: Robert N <crshman at gmail.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=70254
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> ---
> 
> I'm just relaying the patch from bugzilla to the list as it seems to fix
> the bug. Don't ask me anything about it.
> 
> BR,
> Jani.
> ---
>  drivers/gpu/drm/i915/intel_display.c |   10 ++++++++++
>  drivers/gpu/drm/i915/intel_drv.h     |    1 +
>  drivers/gpu/drm/i915/intel_pm.c      |   33 +++++++++++++++++++++++++++++++++
>  3 files changed, 44 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 0b19afdfbaa7..b0ac92d8f461 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9933,6 +9933,16 @@ static int __intel_set_mode(struct drm_crtc *crtc,
>  		 */
>  		drm_calc_timestamping_constants(crtc,
>  						&pipe_config->adjusted_mode);
> +
> +
> +		/*
> +		 * The BIOS provided WM memory latency values are often
> +		 * inadequate for high resolution displays. Adjust them.
> +		 *
> +		 * FIXME not sure 225MHz is a good threshold.
> +		 */
> +		if (IS_GEN6(dev) && pipe_config->adjusted_mode.crtc_clock > 225000)
> +			snb_wm_latency_quirk(dev);
>  	}
>  
>  	/* Only after disabling all output pipelines that will be changed can we
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 60ffad376390..b1d631c9dfa5 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -903,6 +903,7 @@ void intel_runtime_pm_put(struct drm_i915_private *dev_priv);
>  void intel_init_runtime_pm(struct drm_i915_private *dev_priv);
>  void intel_fini_runtime_pm(struct drm_i915_private *dev_priv);
>  void ilk_wm_get_hw_state(struct drm_device *dev);
> +void snb_wm_latency_quirk(struct drm_device *dev);
>  
>  
>  /* intel_sdvo.c */
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 39f3238bf1c3..226750452637 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2104,6 +2104,39 @@ static void ilk_setup_wm_latency(struct drm_device *dev)
>  	intel_print_wm_latency(dev, "Cursor", dev_priv->wm.cur_latency);
>  }
>  
> +static bool ilk_increase_wm_latency(struct drm_i915_private *dev_priv,
> +				    uint16_t wm[5], uint16_t min)
> +{
> +	int level, max_level = ilk_wm_max_level(dev_priv->dev);
> +
> +	if (wm[0] >= min)
> +		return false;
> +
> +	wm[0] = max(wm[0], min);
> +	for (level = 1; level <= max_level; level++)
> +		wm[level] = max_t(uint16_t, wm[level], DIV_ROUND_UP(min, 5));
> +
> +	return true;
> +}
> +
> +void snb_wm_latency_quirk(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	bool changed;
> +
> +	changed = ilk_increase_wm_latency(dev_priv, dev_priv->wm.pri_latency, 12) |
> +		ilk_increase_wm_latency(dev_priv, dev_priv->wm.spr_latency, 12) |
> +		ilk_increase_wm_latency(dev_priv, dev_priv->wm.cur_latency, 12);

Unless I missed something this hereafter permanently bumps the latency
for computations.

At the 225MHz pixel clock, what are the differences in final WM values?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre