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

[Daniel Vetter]

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 think we should just unconditionally increase the memory latency. ivb
has a rather similar memory subsystem compared to snb, so just increasing
it always to this limits sounds ok. For lower resolutions we might simply
get away because there's more excess bandwidth available.

Maybe Art can shed some light on this.
-Daniel

>
> ---
>
> 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);
> +
> + if (!changed)
> + return;
> +
> + DRM_DEBUG_KMS("WM latency values increased due to high pixel clock\n");
> + intel_print_wm_latency(dev, "Primary", dev_priv->wm.pri_latency);
> + intel_print_wm_latency(dev, "Sprite", dev_priv->wm.spr_latency);
> + intel_print_wm_latency(dev, "Cursor", dev_priv->wm.cur_latency);
> +}
> +
>  static void ilk_compute_wm_parameters(struct drm_crtc *crtc,
>        struct ilk_pipe_wm_parameters *p,
>        struct intel_wm_config *config)
> --
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch