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

[Runyan, Arthur J]

Please check the DRAM configuration for the systems that fail.  The higher latency is more likely with higher tRFC which is mainly found with 8 Gbit components.

>-----Original Message-----
>From: daniel.vetter at ffwll.ch [mailto:daniel.vetter at ffwll.ch] On Behalf Of Daniel Vetter
>Sent: Friday, March 21, 2014 3:19 AM
>To: Nikula, Jani; Runyan, Arthur J; Syrjala, Ville
>Cc: intel-gfx
>Subject: Re: [Intel-gfx] [PATCH] drm/i915: Increase WM memory latency values on SNB with
>high pixel clock
>
>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