public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Increase WM memory latency values on SNB with high pixel clock
@ 2014-03-21  9:00 Jani Nikula
  2014-03-21  9:29 ` Chris Wilson
  2014-03-21 10:19 ` Daniel Vetter
  0 siblings, 2 replies; 18+ messages in thread
From: Jani Nikula @ 2014-03-21  9:00 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

From: Ville Syrjälä <ville.syrjala@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@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@gmail.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=70254
Signed-off-by: Ville Syrjälä <ville.syrjala@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);
+
+	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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH] drm/i915: Increase WM memory latency values on SNB with high pixel clock
  2014-03-21  9:00 [PATCH] drm/i915: Increase WM memory latency values on SNB with high pixel clock Jani Nikula
@ 2014-03-21  9:29 ` Chris Wilson
  2014-03-21 10:19 ` Daniel Vetter
  1 sibling, 0 replies; 18+ messages in thread
From: Chris Wilson @ 2014-03-21  9:29 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On Fri, Mar 21, 2014 at 11:00:48AM +0200, Jani Nikula wrote:
> From: Ville Syrjälä <ville.syrjala@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@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@gmail.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=70254
> Signed-off-by: Ville Syrjälä <ville.syrjala@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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] drm/i915: Increase WM memory latency values on SNB with high pixel clock
  2014-03-21  9:00 [PATCH] drm/i915: Increase WM memory latency values on SNB with high pixel clock Jani Nikula
  2014-03-21  9:29 ` Chris Wilson
@ 2014-03-21 10:19 ` Daniel Vetter
  2014-03-21 17:28   ` Runyan, Arthur J
  2014-05-08 12:09   ` [PATCH v2] drm/i915: Increase WM memory latency values on SNB ville.syrjala
  1 sibling, 2 replies; 18+ messages in thread
From: Daniel Vetter @ 2014-03-21 10:19 UTC (permalink / raw)
  To: Jani Nikula, Arthur Ranyan, Syrjala, Ville; +Cc: intel-gfx

On Fri, Mar 21, 2014 at 11:00:48AM +0200, Jani Nikula wrote:
> From: Ville Syrjälä <ville.syrjala@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@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@gmail.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=70254
> Signed-off-by: Ville Syrjälä <ville.syrjala@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@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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] drm/i915: Increase WM memory latency values on SNB with high pixel clock
  2014-03-21 10:19 ` Daniel Vetter
@ 2014-03-21 17:28   ` Runyan, Arthur J
  2014-03-31 18:29     ` Robert Navarro
  2014-05-08 12:09   ` [PATCH v2] drm/i915: Increase WM memory latency values on SNB ville.syrjala
  1 sibling, 1 reply; 18+ messages in thread
From: Runyan, Arthur J @ 2014-03-21 17:28 UTC (permalink / raw)
  To: Daniel Vetter, Nikula, Jani, Syrjala, Ville; +Cc: intel-gfx

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@ffwll.ch [mailto:daniel.vetter@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@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@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@gmail.com>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=70254
>> Signed-off-by: Ville Syrjälä <ville.syrjala@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@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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] drm/i915: Increase WM memory latency values on SNB with high pixel clock
  2014-03-21 17:28   ` Runyan, Arthur J
@ 2014-03-31 18:29     ` Robert Navarro
  2014-04-14  9:16       ` Ville Syrjälä
  0 siblings, 1 reply; 18+ messages in thread
From: Robert Navarro @ 2014-03-31 18:29 UTC (permalink / raw)
  To: intel-gfx

Runyan, Arthur J <arthur.j.runyan <at> intel.com> writes:

> 
> 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.
> 

What other information do we need to get this included?

The DRAM config, is this something that I have to/should check? How do I get 
this information to you?

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] drm/i915: Increase WM memory latency values on SNB with high pixel clock
  2014-03-31 18:29     ` Robert Navarro
@ 2014-04-14  9:16       ` Ville Syrjälä
  0 siblings, 0 replies; 18+ messages in thread
From: Ville Syrjälä @ 2014-04-14  9:16 UTC (permalink / raw)
  To: Robert Navarro; +Cc: intel-gfx

On Mon, Mar 31, 2014 at 06:29:51PM +0000, Robert Navarro wrote:
> Runyan, Arthur J <arthur.j.runyan <at> intel.com> writes:
> 
> > 
> > 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.
> > 
> 
> What other information do we need to get this included?
> 
> The DRAM config, is this something that I have to/should check? How do I get 
> this information to you?

If decode-dimms works on your machine it might print out potentially
interesting stuff. It doesn't seem to work on most of my machines. It
does work on the x220 I have here though, so I'm assuming it should work
on your x220 too. decode-dimms comes from the i2c-tools package.

Here's what I get on mine:

# modprobe i2c-i801
# modprobe eeprom
# decode-dimms 
# decode-dimms version 6231 (2014-02-20 10:54:34 +0100)

Memory Serial Presence Detect Decoder
By Philip Edelbrock, Christian Zuckschwerdt, Burkart Lingner,
Jean Delvare, Trent Piepho and others


Decoding EEPROM: /sys/bus/i2c/drivers/eeprom/0-0050
Guessing DIMM is in                             bank 1

---=== SPD EEPROM Information ===---
EEPROM CRC of bytes 0-116                       OK (0xFC7B)
# of bytes written to SDRAM EEPROM              176
Total number of bytes in EEPROM                 256
Fundamental Memory type                         DDR3 SDRAM
Module Type                                     SO-DIMM

---=== Memory Characteristics ===---
Fine time base                                  1.000 ps
Medium time base                                0.125 ns
Maximum module speed                            1333 MHz (PC3-10600)
Size                                            4096 MB
Banks x Rows x Columns x Bits                   8 x 15 x 10 x 64
Ranks                                           2
SDRAM Device Width                              8 bits
Bus Width Extension                             0 bits
tCL-tRCD-tRP-tRAS                               9-9-9-24
Supported CAS Latencies (tCL)                   9T, 8T, 7T, 6T, 5T

---=== Timing Parameters ===---
Minimum Write Recovery time (tWR)               15.000 ns
Minimum Row Active to Row Active Delay (tRRD)   6.000 ns
Minimum Active to Auto-Refresh Delay (tRC)      49.125 ns
Minimum Recovery Delay (tRFC)                   160.000 ns
Minimum Write to Read CMD Delay (tWTR)          7.500 ns
Minimum Read to Pre-charge CMD Delay (tRTP)     7.500 ns
Minimum Four Activate Window Delay (tFAW)       30.000 ns

---=== Optional Features ===---
Operable voltages                               1.5V
RZQ/6 supported?                                Yes
RZQ/7 supported?                                Yes
DLL-Off Mode supported?                         Yes
Operating temperature range                     0-95 degrees C
Refresh Rate in extended temp range             1X
Auto Self-Refresh?                              No
On-Die Thermal Sensor readout?                  No
Partial Array Self-Refresh?                     No
Thermal Sensor Accuracy                         Not implemented
SDRAM Device Type                               Standard Monolithic

---=== Physical Characteristics ===---
Module Height (mm)                              30
Module Thickness (mm)                           2 front, 2 back
Module Width (mm)                               67.6
Module Reference Card                           F

---=== Manufacturer Data ===---
Module Manufacturer                             Samsung
DRAM Manufacturer                               Samsung
Manufacturing Location Code                     0x03
Manufacturing Date                              2012-W04
Assembly Serial Number                          0x009B48F1
Part Number                                     M471B5273DH0-CH9  


Number of SDRAM DIMMs detected and decoded: 1



Just for figuring out what tRFC is I think the following would do it:
# intel_reg_read 0x144298 0x144698 0x144a98 0x145d10 0x145e04

These are from the machines I have lying around on my desk:

// dell xps (snb)
# ./intel_reg_read 0x144298 0x144698 0x144a98 0x145d10 0x145e04
0x144298 : 0x5A6B1450
0x144698 : 0x5A6B1450
0x144A98 : 0x0
0x145D10 : 0x16040307
0x145E04 : 0x5

// x220 (snb)
# ./intel_reg_read 0x144298 0x144698 0x144a98 0x145d10 0x145e04
0x144298 : 0x5A6B1450
0x144698 : 0x46B41004
0x144A98 : 0x0
0x145D10 : 0x16040307
0x145E04 : 0x5

// some ivb desktop
# ./intel_reg_read 0x144298 0x144698 0x144a98 0x145d10 0x145e04
0x144298 : 0x5A6B1450
0x144698 : 0x5A6B1450
0x144A98 : 0x0
0x145D10 : 0x2010040C
0x145E04 : 0x5

So on for most things I get an answer of tRFC=107, which means 160ns
if my math is any good. And that matches the SPD info from decode-dimms
on the x220. The second channel for the x220 has something different
but it actually looks like the reset value for the register which makes
sense since decode-dimms says it only has one DIMM.

No idea if my tRFC is particularly low or high. The latency values
are quite different between the ivb and snb machines however. Also
I must admit that I've never tried plugging in a 25x14 monitor to
these machines, and in fact with the dell xps I can't since it
doesn't even have a DP connector.

-- 
Ville Syrjälä
Intel OTC

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2] drm/i915: Increase WM memory latency values on SNB
  2014-03-21 10:19 ` Daniel Vetter
  2014-03-21 17:28   ` Runyan, Arthur J
@ 2014-05-08 12:09   ` ville.syrjala
  2014-05-09 15:23     ` Robert Navarro
  2014-05-15 10:13     ` [PATCH v2] drm/i915: Increase WM memory latency values on SNB Jani Nikula
  1 sibling, 2 replies; 18+ messages in thread
From: ville.syrjala @ 2014-05-08 12:09 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula, Robert N, Arthur Ranyan

From: Ville Syrjälä <ville.syrjala@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@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 at least 1.2 usec on SNB.
Hopefully this won't have a significant effect on power consumption.

v2: Increase the latency values regardless of the pixel clock

Cc: Robert N <crshman@gmail.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=70254
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 834c49c..2809365 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2113,6 +2113,43 @@ static void intel_print_wm_latency(struct drm_device *dev,
 	}
 }
 
+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;
+}
+
+static void snb_wm_latency_quirk(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	bool changed;
+
+	/*
+	 * The BIOS provided WM memory latency values are often
+	 * inadequate for high resolution displays. Adjust them.
+	 */
+	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 to avoid potential underruns\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_setup_wm_latency(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -2130,6 +2167,9 @@ static void ilk_setup_wm_latency(struct drm_device *dev)
 	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);
+
+	if (IS_GEN6(dev))
+		snb_wm_latency_quirk(dev);
 }
 
 static void ilk_compute_wm_parameters(struct drm_crtc *crtc,
-- 
1.8.3.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] drm/i915: Increase WM memory latency values on SNB
  2014-05-08 12:09   ` [PATCH v2] drm/i915: Increase WM memory latency values on SNB ville.syrjala
@ 2014-05-09 15:23     ` Robert Navarro
  2014-05-09 15:38       ` [PATCH v2] drm/i915: Increase WM memory latency values?on SNB Ville Syrjälä
  2014-05-15 10:13     ` [PATCH v2] drm/i915: Increase WM memory latency values on SNB Jani Nikula
  1 sibling, 1 reply; 18+ messages in thread
From: Robert Navarro @ 2014-05-09 15:23 UTC (permalink / raw)
  To: intel-gfx

Thanks for this Ville.

Should this apply to 3.14 and 3.15?

I'll try it on 3.15 first and report back.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] drm/i915: Increase WM memory latency values?on SNB
  2014-05-09 15:23     ` Robert Navarro
@ 2014-05-09 15:38       ` Ville Syrjälä
  2014-05-09 17:46         ` Robert Navarro
  0 siblings, 1 reply; 18+ messages in thread
From: Ville Syrjälä @ 2014-05-09 15:38 UTC (permalink / raw)
  To: Robert Navarro; +Cc: intel-gfx

On Fri, May 09, 2014 at 03:23:41PM +0000, Robert Navarro wrote:
> Thanks for this Ville.
> 
> Should this apply to 3.14 and 3.15?
> 
> I'll try it on 3.15 first and report back.

I think it should apply to 3.13+. If not directly then with a bit of
manual frobbery. Which reminds me that we should perhaps slap a cc
stable on it to get it included in 3.13+. For older kernels the patch
would have to look totally different, so I'm not going to bother
about those.

-- 
Ville Syrjälä
Intel OTC

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] drm/i915: Increase WM memory latency values?on SNB
  2014-05-09 15:38       ` [PATCH v2] drm/i915: Increase WM memory latency values?on SNB Ville Syrjälä
@ 2014-05-09 17:46         ` Robert Navarro
  2014-05-09 17:54           ` Ville Syrjälä
  0 siblings, 1 reply; 18+ messages in thread
From: Robert Navarro @ 2014-05-09 17:46 UTC (permalink / raw)
  To: intel-gfx

Ville Syrjälä <ville.syrjala <at> linux.intel.com> writes:

> I think it should apply to 3.13+. If not directly then with a bit of
> manual frobbery. Which reminds me that we should perhaps slap a cc
> stable on it to get it included in 3.13+. For older kernels the patch
> would have to look totally different, so I'm not going to bother
> about those.
> 

Sounds good, one more question. Does this replace the previous patch or are 
they used together?




_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] drm/i915: Increase WM memory latency values?on SNB
  2014-05-09 17:46         ` Robert Navarro
@ 2014-05-09 17:54           ` Ville Syrjälä
  2014-05-14  0:55             ` Robert Navarro
  0 siblings, 1 reply; 18+ messages in thread
From: Ville Syrjälä @ 2014-05-09 17:54 UTC (permalink / raw)
  To: Robert Navarro; +Cc: intel-gfx

On Fri, May 09, 2014 at 05:46:43PM +0000, Robert Navarro wrote:
> Ville Syrjälä <ville.syrjala <at> linux.intel.com> writes:
> 
> > I think it should apply to 3.13+. If not directly then with a bit of
> > manual frobbery. Which reminds me that we should perhaps slap a cc
> > stable on it to get it included in 3.13+. For older kernels the patch
> > would have to look totally different, so I'm not going to bother
> > about those.
> > 
> 
> Sounds good, one more question. Does this replace the previous patch or are 
> they used together?

Replace.

-- 
Ville Syrjälä
Intel OTC

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] drm/i915: Increase WM memory latency values?on SNB
  2014-05-09 17:54           ` Ville Syrjälä
@ 2014-05-14  0:55             ` Robert Navarro
  2014-05-15 14:18               ` Robert Navarro
  0 siblings, 1 reply; 18+ messages in thread
From: Robert Navarro @ 2014-05-14  0:55 UTC (permalink / raw)
  To: intel-gfx

Ville Syrjälä <ville.syrjala <at> linux.intel.com> writes:

> 
> Replace.
> 

Finally got around to compiling this for my system, there were a few issues 
with the build scripts on the latest Ubuntu.

Currently running 3.15.0-rc3-custom-drm-intel-nightly-bug70254+ with no 
issues thus far.

I'll give a few more days just to make sure.

Thanks for all your hard work so far!

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] drm/i915: Increase WM memory latency values on SNB
  2014-05-08 12:09   ` [PATCH v2] drm/i915: Increase WM memory latency values on SNB ville.syrjala
  2014-05-09 15:23     ` Robert Navarro
@ 2014-05-15 10:13     ` Jani Nikula
  2014-05-15 10:16       ` Chris Wilson
  1 sibling, 1 reply; 18+ messages in thread
From: Jani Nikula @ 2014-05-15 10:13 UTC (permalink / raw)
  To: ville.syrjala, intel-gfx; +Cc: Robert N, Arthur Ranyan

On Thu, 08 May 2014, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@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@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 at least 1.2 usec on SNB.
> Hopefully this won't have a significant effect on power consumption.
>
> v2: Increase the latency values regardless of the pixel clock
>
> Cc: Robert N <crshman@gmail.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=70254
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 40 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 834c49c..2809365 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2113,6 +2113,43 @@ static void intel_print_wm_latency(struct drm_device *dev,
>  	}
>  }
>  
> +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));

DIV_ROUND_UP(min, 5)?

I don't get the units, and the code vs. comments in the wm code aren't
helping. Please clarify.

> +
> +	return true;
> +}
> +
> +static void snb_wm_latency_quirk(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	bool changed;
> +
> +	/*
> +	 * The BIOS provided WM memory latency values are often
> +	 * inadequate for high resolution displays. Adjust them.
> +	 */
> +	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);

Nitpick, s/|/||/g for bools.

BR,
Jani.

> +
> +	if (!changed)
> +		return;
> +
> +	DRM_DEBUG_KMS("WM latency values increased to avoid potential underruns\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_setup_wm_latency(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -2130,6 +2167,9 @@ static void ilk_setup_wm_latency(struct drm_device *dev)
>  	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);
> +
> +	if (IS_GEN6(dev))
> +		snb_wm_latency_quirk(dev);
>  }
>  
>  static void ilk_compute_wm_parameters(struct drm_crtc *crtc,
> -- 
> 1.8.3.2
>

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] drm/i915: Increase WM memory latency values on SNB
  2014-05-15 10:13     ` [PATCH v2] drm/i915: Increase WM memory latency values on SNB Jani Nikula
@ 2014-05-15 10:16       ` Chris Wilson
  2014-05-15 10:34         ` Jani Nikula
  0 siblings, 1 reply; 18+ messages in thread
From: Chris Wilson @ 2014-05-15 10:16 UTC (permalink / raw)
  To: Jani Nikula; +Cc: Robert N, intel-gfx, Arthur Ranyan

On Thu, May 15, 2014 at 01:13:21PM +0300, Jani Nikula wrote:
> On Thu, 08 May 2014, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > +static void snb_wm_latency_quirk(struct drm_device *dev)
> > +{
> > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > +	bool changed;
> > +
> > +	/*
> > +	 * The BIOS provided WM memory latency values are often
> > +	 * inadequate for high resolution displays. Adjust them.
> > +	 */
> > +	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);
> 
> Nitpick, s/|/||/g for bools.

Consider side effects.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] drm/i915: Increase WM memory latency values on SNB
  2014-05-15 10:16       ` Chris Wilson
@ 2014-05-15 10:34         ` Jani Nikula
  2014-05-15 10:45           ` Ville Syrjälä
  0 siblings, 1 reply; 18+ messages in thread
From: Jani Nikula @ 2014-05-15 10:34 UTC (permalink / raw)
  To: Chris Wilson; +Cc: Robert N, intel-gfx, Arthur Ranyan

On Thu, 15 May 2014, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Thu, May 15, 2014 at 01:13:21PM +0300, Jani Nikula wrote:
>> On Thu, 08 May 2014, ville.syrjala@linux.intel.com wrote:
>> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> > +static void snb_wm_latency_quirk(struct drm_device *dev)
>> > +{
>> > +	struct drm_i915_private *dev_priv = dev->dev_private;
>> > +	bool changed;
>> > +
>> > +	/*
>> > +	 * The BIOS provided WM memory latency values are often
>> > +	 * inadequate for high resolution displays. Adjust them.
>> > +	 */
>> > +	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);
>> 
>> Nitpick, s/|/||/g for bools.
>
> Consider side effects.

Ugh I'm slow today. Some might claim business as usual. I'll hide
somewhere.

Before I head under the rock, may I say bitops on bools are still ugly?

BR,
Jani.

> -Chris
>
> -- 
> Chris Wilson, Intel Open Source Technology Centre

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] drm/i915: Increase WM memory latency values on SNB
  2014-05-15 10:34         ` Jani Nikula
@ 2014-05-15 10:45           ` Ville Syrjälä
  2014-05-15 10:58             ` Jani Nikula
  0 siblings, 1 reply; 18+ messages in thread
From: Ville Syrjälä @ 2014-05-15 10:45 UTC (permalink / raw)
  To: Jani Nikula; +Cc: Robert N, intel-gfx, Arthur Ranyan

On Thu, May 15, 2014 at 01:34:44PM +0300, Jani Nikula wrote:
> On Thu, 15 May 2014, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > On Thu, May 15, 2014 at 01:13:21PM +0300, Jani Nikula wrote:
> >> On Thu, 08 May 2014, ville.syrjala@linux.intel.com wrote:
> >> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> > +static void snb_wm_latency_quirk(struct drm_device *dev)
> >> > +{
> >> > +	struct drm_i915_private *dev_priv = dev->dev_private;
> >> > +	bool changed;
> >> > +
> >> > +	/*
> >> > +	 * The BIOS provided WM memory latency values are often
> >> > +	 * inadequate for high resolution displays. Adjust them.
> >> > +	 */
> >> > +	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);
> >> 
> >> Nitpick, s/|/||/g for bools.
> >
> > Consider side effects.
> 
> Ugh I'm slow today. Some might claim business as usual. I'll hide
> somewhere.
> 
> Before I head under the rock, may I say bitops on bools are still ugly?

I tend to use them but Paulo was also confused by them somewhere else
in the watermark code, so maybe I should stop using them?

I can rewrite as:
changed |= ilk_increase_wm_latency(dev_priv, dev_priv->wm.pri_latency, 12);
changed |= ilk_increase_wm_latency(dev_priv, dev_priv->wm.spr_latency, 12);
changed |= ilk_increase_wm_latency(dev_priv, dev_priv->wm.cur_latency, 12);

or just 3x
if (ilk_increase_wm_latency(...))
	changed = true;

if that helps.

-- 
Ville Syrjälä
Intel OTC

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] drm/i915: Increase WM memory latency values on SNB
  2014-05-15 10:45           ` Ville Syrjälä
@ 2014-05-15 10:58             ` Jani Nikula
  0 siblings, 0 replies; 18+ messages in thread
From: Jani Nikula @ 2014-05-15 10:58 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: Robert N, intel-gfx, Arthur Ranyan

On Thu, 15 May 2014, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Thu, May 15, 2014 at 01:34:44PM +0300, Jani Nikula wrote:
>> On Thu, 15 May 2014, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>> > On Thu, May 15, 2014 at 01:13:21PM +0300, Jani Nikula wrote:
>> >> On Thu, 08 May 2014, ville.syrjala@linux.intel.com wrote:
>> >> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >> > +static void snb_wm_latency_quirk(struct drm_device *dev)
>> >> > +{
>> >> > +	struct drm_i915_private *dev_priv = dev->dev_private;
>> >> > +	bool changed;
>> >> > +
>> >> > +	/*
>> >> > +	 * The BIOS provided WM memory latency values are often
>> >> > +	 * inadequate for high resolution displays. Adjust them.
>> >> > +	 */
>> >> > +	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);
>> >> 
>> >> Nitpick, s/|/||/g for bools.
>> >
>> > Consider side effects.
>> 
>> Ugh I'm slow today. Some might claim business as usual. I'll hide
>> somewhere.
>> 
>> Before I head under the rock, may I say bitops on bools are still ugly?
>
> I tend to use them but Paulo was also confused by them somewhere else
> in the watermark code, so maybe I should stop using them?
>
> I can rewrite as:
> changed |= ilk_increase_wm_latency(dev_priv, dev_priv->wm.pri_latency, 12);
> changed |= ilk_increase_wm_latency(dev_priv, dev_priv->wm.spr_latency, 12);
> changed |= ilk_increase_wm_latency(dev_priv, dev_priv->wm.cur_latency, 12);
>
> or just 3x
> if (ilk_increase_wm_latency(...))
> 	changed = true;
>
> if that helps.

Too late, patch pushed to -fixes, thanks for the patch and ridicule. ;)
You can send cleanups later for -next if you like.

BR,
Jani.


>
> -- 
> Ville Syrjälä
> Intel OTC

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] drm/i915: Increase WM memory latency values?on SNB
  2014-05-14  0:55             ` Robert Navarro
@ 2014-05-15 14:18               ` Robert Navarro
  0 siblings, 0 replies; 18+ messages in thread
From: Robert Navarro @ 2014-05-15 14:18 UTC (permalink / raw)
  To: intel-gfx

Robert Navarro <crshman <at> gmail.com> writes:

>
> Finally got around to compiling this for my system, there were a few issues 
> with the build scripts on the latest Ubuntu.
> 
> Currently running 3.15.0-rc3-custom-drm-intel-nightly-bug70254+ with no 
> issues thus far.
> 
> I'll give a few more days just to make sure.
> 
> Thanks for all your hard work so far!
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx <at> lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 


Just reporting back, I see you've included the fix, thanks!

A few days (and really hot temperature days for the laptop) not a single 
flicker or anything.

Thanks again for everyone's hard work in making this happen!

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2014-05-15 14:18 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-21  9:00 [PATCH] drm/i915: Increase WM memory latency values on SNB with high pixel clock Jani Nikula
2014-03-21  9:29 ` Chris Wilson
2014-03-21 10:19 ` Daniel Vetter
2014-03-21 17:28   ` Runyan, Arthur J
2014-03-31 18:29     ` Robert Navarro
2014-04-14  9:16       ` Ville Syrjälä
2014-05-08 12:09   ` [PATCH v2] drm/i915: Increase WM memory latency values on SNB ville.syrjala
2014-05-09 15:23     ` Robert Navarro
2014-05-09 15:38       ` [PATCH v2] drm/i915: Increase WM memory latency values?on SNB Ville Syrjälä
2014-05-09 17:46         ` Robert Navarro
2014-05-09 17:54           ` Ville Syrjälä
2014-05-14  0:55             ` Robert Navarro
2014-05-15 14:18               ` Robert Navarro
2014-05-15 10:13     ` [PATCH v2] drm/i915: Increase WM memory latency values on SNB Jani Nikula
2014-05-15 10:16       ` Chris Wilson
2014-05-15 10:34         ` Jani Nikula
2014-05-15 10:45           ` Ville Syrjälä
2014-05-15 10:58             ` Jani Nikula

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox