[PATCH 0/9] Haswell watermarks, round 1

[PATCH 0/9] Haswell watermarks, round 1

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Hi

All our Haswell watermarks code does not follow our specifications, and this
series only does some small fixes and also fixes the linetime watermarks. For
the other watermarks we're still calling the old sandybridge update_wm
functions, so the next patches will implement Haswell-specific functions and use
them, also using the linetime watermarks fixed in this series. Consider this
series as a warm-up for the next ones :)

Cheers,
Paulo

Paulo Zanoni (9):
  drm/i915: ILK, SNB and IVB don't have linetime watermarks
  drm/i915: fix linetime_watermarks code
  drm/i915: use the mode->htotal to calculate linetime watermarks
  drm/i915: fix haswell linetime watermarks calculation
  drm/i915: use the correct clock when calculating linetime watermarks
  drm/i915: make intel_ddi_get_cdclk_freq return values in KHz
  drm/i915: set the IPS linetime watermark
  drm/i915: MCH_SSKPD is a 64 bit register on Haswell
  drm/i915: set FORCE_ARB_IDLE_PLANES workaround

 drivers/gpu/drm/i915/i915_drv.h      |    2 -
 drivers/gpu/drm/i915/i915_reg.h      |    3 ++
 drivers/gpu/drm/i915/intel_ddi.c     |   10 ++---
 drivers/gpu/drm/i915/intel_display.c |    4 --
 drivers/gpu/drm/i915/intel_dp.c      |    3 +-
 drivers/gpu/drm/i915/intel_drv.h     |    2 -
 drivers/gpu/drm/i915/intel_pm.c      |   73 ++++++++++++++++++----------------
 7 files changed, 48 insertions(+), 49 deletions(-)

-- 
1.7.10.4

[PATCH 1/9] drm/i915: ILK, SNB and IVB don't have linetime watermarks

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

So don't call intel_update_linetime_watermarks from
ironlake_crtc_mode_set. Only Haswell has these watermarks.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9af3ec2..3e742f9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5863,8 +5863,6 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 
 	intel_update_watermarks(dev);
 
-	intel_update_linetime_watermarks(dev, pipe, adjusted_mode);
-
 	return ret;
 }
 
-- 
1.7.10.4

Re: [PATCH 1/9] drm/i915: ILK, SNB and IVB don't have linetime watermarks

[Ville Syrjälä]

On Fri, May 03, 2013 at 05:23:37PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> So don't call intel_update_linetime_watermarks from
> ironlake_crtc_mode_set. Only Haswell has these watermarks.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c |    2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 9af3ec2..3e742f9 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5863,8 +5863,6 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>  
>  	intel_update_watermarks(dev);
>  
> -	intel_update_linetime_watermarks(dev, pipe, adjusted_mode);
> -
>  	return ret;
>  }
>  
> -- 
> 1.7.10.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

[PATCH 2/9] drm/i915: fix linetime_watermarks code

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

The spec says the linetime watermarks must be programmed before
enabling any display low power watermarks, but we're currently
updating the linetime watermarks after we call intel_update_watermarks
(and only at crtc_mode_set, not at crtc_{enable,disable}). So IMHO the
best way guarantee the linetime watermarks will be updated before the
low power watermarks is inside the update_wm function, because it's
the function that enables low power watermarks. And since Haswell is
the only platform that has linetime watermarks, let's completely kill
the "intel_update_linetime_watermarks" abstraction and just use the
intel_update_watermarks abstraction by creating haswell_update_wm.

For now haswell_update_wm is still calling sandybridge_update_wm, but
in the future I plan to implement a function specific to Haswell.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |    2 --
 drivers/gpu/drm/i915/intel_display.c |    2 --
 drivers/gpu/drm/i915/intel_drv.h     |    2 --
 drivers/gpu/drm/i915/intel_pm.c      |   37 ++++++++++++++++++++++------------
 4 files changed, 24 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5160e1e..f9864c7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -316,8 +316,6 @@ struct drm_i915_display_funcs {
 	void (*update_wm)(struct drm_device *dev);
 	void (*update_sprite_wm)(struct drm_device *dev, int pipe,
 				 uint32_t sprite_width, int pixel_size);
-	void (*update_linetime_wm)(struct drm_device *dev, int pipe,
-				 struct drm_display_mode *mode);
 	void (*modeset_global_resources)(struct drm_device *dev);
 	/* Returns the active state of the crtc, and if the crtc is active,
 	 * fills out the pipe-config with the hw state. */
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3e742f9..ab5690b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6013,8 +6013,6 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
 
 	intel_update_watermarks(dev);
 
-	intel_update_linetime_watermarks(dev, pipe, adjusted_mode);
-
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index ae73631..f677af6 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -732,8 +732,6 @@ extern void intel_update_watermarks(struct drm_device *dev);
 extern void intel_update_sprite_watermarks(struct drm_device *dev, int pipe,
 					   uint32_t sprite_width,
 					   int pixel_size);
-extern void intel_update_linetime_watermarks(struct drm_device *dev, int pipe,
-			 struct drm_display_mode *mode);
 
 extern unsigned long intel_gen4_compute_page_offset(int *x, int *y,
 						    unsigned int tiling_mode,
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 1afaec4..d056bc9 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2015,12 +2015,19 @@ static void ivybridge_update_wm(struct drm_device *dev)
 }
 
 static void
-haswell_update_linetime_wm(struct drm_device *dev, int pipe,
-				 struct drm_display_mode *mode)
+haswell_update_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	enum pipe pipe = intel_crtc->pipe;
+	struct drm_display_mode *mode = &intel_crtc->config.adjusted_mode;
 	u32 temp;
 
+	if (!intel_crtc_active(crtc)) {
+		I915_WRITE(PIPE_WM_LINETIME(pipe), 0);
+		return;
+	}
+
 	temp = I915_READ(PIPE_WM_LINETIME(pipe));
 	temp &= ~PIPE_WM_LINETIME_MASK;
 
@@ -2041,6 +2048,20 @@ haswell_update_linetime_wm(struct drm_device *dev, int pipe,
 	I915_WRITE(PIPE_WM_LINETIME(pipe), temp);
 }
 
+static void haswell_update_wm(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_crtc *crtc;
+	enum pipe pipe;
+
+	for_each_pipe(pipe) {
+		crtc = dev_priv->pipe_to_crtc_mapping[pipe];
+		haswell_update_linetime_wm(dev, crtc);
+	}
+
+	sandybridge_update_wm(dev);
+}
+
 static bool
 sandybridge_compute_sprite_wm(struct drm_device *dev, int plane,
 			      uint32_t sprite_width, int pixel_size,
@@ -2236,15 +2257,6 @@ void intel_update_watermarks(struct drm_device *dev)
 		dev_priv->display.update_wm(dev);
 }
 
-void intel_update_linetime_watermarks(struct drm_device *dev,
-		int pipe, struct drm_display_mode *mode)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-
-	if (dev_priv->display.update_linetime_wm)
-		dev_priv->display.update_linetime_wm(dev, pipe, mode);
-}
-
 void intel_update_sprite_watermarks(struct drm_device *dev, int pipe,
 				    uint32_t sprite_width, int pixel_size)
 {
@@ -4482,9 +4494,8 @@ void intel_init_pm(struct drm_device *dev)
 			dev_priv->display.init_clock_gating = ivybridge_init_clock_gating;
 		} else if (IS_HASWELL(dev)) {
 			if (SNB_READ_WM0_LATENCY()) {
-				dev_priv->display.update_wm = sandybridge_update_wm;
+				dev_priv->display.update_wm = haswell_update_wm;
 				dev_priv->display.update_sprite_wm = sandybridge_update_sprite_wm;
-				dev_priv->display.update_linetime_wm = haswell_update_linetime_wm;
 			} else {
 				DRM_DEBUG_KMS("Failed to read display plane latency. "
 					      "Disable CxSR\n");
-- 
1.7.10.4

Re: [PATCH 2/9] drm/i915: fix linetime_watermarks code

[Chris Wilson]

On Fri, May 03, 2013 at 05:23:38PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> The spec says the linetime watermarks must be programmed before
> enabling any display low power watermarks, but we're currently
> updating the linetime watermarks after we call intel_update_watermarks
> (and only at crtc_mode_set, not at crtc_{enable,disable}). So IMHO the
> best way guarantee the linetime watermarks will be updated before the
> low power watermarks is inside the update_wm function, because it's
> the function that enables low power watermarks. And since Haswell is
> the only platform that has linetime watermarks, let's completely kill
> the "intel_update_linetime_watermarks" abstraction and just use the
> intel_update_watermarks abstraction by creating haswell_update_wm.

> +static void haswell_update_wm(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct drm_crtc *crtc;
> +	enum pipe pipe;
> +
> +	for_each_pipe(pipe) {
> +		crtc = dev_priv->pipe_to_crtc_mapping[pipe];
> +		haswell_update_linetime_wm(dev, crtc);
> +	}

The LP watermarks are still enabled at this point.
> +
> +	sandybridge_update_wm(dev);
> +}
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 2/9] drm/i915: fix linetime_watermarks code

[Paulo Zanoni]

2013/5/5 Chris Wilson <chris@chris-wilson.co.uk>:
> On Fri, May 03, 2013 at 05:23:38PM -0300, Paulo Zanoni wrote:
>> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>>
>> The spec says the linetime watermarks must be programmed before
>> enabling any display low power watermarks, but we're currently
>> updating the linetime watermarks after we call intel_update_watermarks
>> (and only at crtc_mode_set, not at crtc_{enable,disable}). So IMHO the
>> best way guarantee the linetime watermarks will be updated before the
>> low power watermarks is inside the update_wm function, because it's
>> the function that enables low power watermarks. And since Haswell is
>> the only platform that has linetime watermarks, let's completely kill
>> the "intel_update_linetime_watermarks" abstraction and just use the
>> intel_update_watermarks abstraction by creating haswell_update_wm.
>
>> +static void haswell_update_wm(struct drm_device *dev)
>> +{
>> +     struct drm_i915_private *dev_priv = dev->dev_private;
>> +     struct drm_crtc *crtc;
>> +     enum pipe pipe;
>> +
>> +     for_each_pipe(pipe) {
>> +             crtc = dev_priv->pipe_to_crtc_mapping[pipe];
>> +             haswell_update_linetime_wm(dev, crtc);
>> +     }
>
> The LP watermarks are still enabled at this point.

Did you mean "they're not disabled yet"? Well, at least now we're
programming the linetime registers _before_ we update the LP
registers, and every time. If we want to be 100% spec-compliant
without any intermediate steps then I guess we can discard this
linetime series and merge it all inside the single patch that will
fully implement haswell_update_wm. But it will be one single big
patch. Just tell me which one you prefer.

>> +
>> +     sandybridge_update_wm(dev);
>> +}
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre



-- 
Paulo Zanoni

Re: [PATCH 2/9] drm/i915: fix linetime_watermarks code

[Chris Wilson]

On Mon, May 06, 2013 at 10:13:33AM -0300, Paulo Zanoni wrote:
> 2013/5/5 Chris Wilson <chris@chris-wilson.co.uk>:
> > On Fri, May 03, 2013 at 05:23:38PM -0300, Paulo Zanoni wrote:
> >> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >>
> >> The spec says the linetime watermarks must be programmed before
> >> enabling any display low power watermarks, but we're currently
> >> updating the linetime watermarks after we call intel_update_watermarks
> >> (and only at crtc_mode_set, not at crtc_{enable,disable}). So IMHO the
> >> best way guarantee the linetime watermarks will be updated before the
> >> low power watermarks is inside the update_wm function, because it's
> >> the function that enables low power watermarks. And since Haswell is
> >> the only platform that has linetime watermarks, let's completely kill
> >> the "intel_update_linetime_watermarks" abstraction and just use the
> >> intel_update_watermarks abstraction by creating haswell_update_wm.
> >
> >> +static void haswell_update_wm(struct drm_device *dev)
> >> +{
> >> +     struct drm_i915_private *dev_priv = dev->dev_private;
> >> +     struct drm_crtc *crtc;
> >> +     enum pipe pipe;
> >> +
> >> +     for_each_pipe(pipe) {
> >> +             crtc = dev_priv->pipe_to_crtc_mapping[pipe];
> >> +             haswell_update_linetime_wm(dev, crtc);
> >> +     }
> >
> > The LP watermarks are still enabled at this point.
> 
> Did you mean "they're not disabled yet"? Well, at least now we're
> programming the linetime registers _before_ we update the LP
> registers, and every time. If we want to be 100% spec-compliant
> without any intermediate steps then I guess we can discard this
> linetime series and merge it all inside the single patch that will
> fully implement haswell_update_wm. But it will be one single big
> patch. Just tell me which one you prefer.

Or you can just disable the 3 LP wm first before updating the linetime
wm.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[PATCH 2/9] drm/i915: remove intel_update_linetime_watermarks

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

The spec says the linetime watermarks must be programmed before
enabling any display low power watermarks, but we're currently
updating the linetime watermarks after we call intel_update_watermarks
(and only at crtc_mode_set, not at crtc_{enable,disable}). So IMHO the
best way guarantee the linetime watermarks will be updated before the
low power watermarks is inside the update_wm function, because it's
the function that enables low power watermarks. And since Haswell is
the only platform that has linetime watermarks, let's completely kill
the "intel_update_linetime_watermarks" abstraction and just use the
intel_update_watermarks abstraction by creating haswell_update_wm.

For now haswell_update_wm is still calling sandybridge_update_wm, but
in the future I plan to implement a function specific to Haswell.

v2: - Rename patch
    - Disable LP watermarks before changing linetime WMs (Chris)
    - Add a comment explaining that this is just temporary code.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |    2 --
 drivers/gpu/drm/i915/intel_display.c |    2 --
 drivers/gpu/drm/i915/intel_drv.h     |    2 --
 drivers/gpu/drm/i915/intel_pm.c      |   43 ++++++++++++++++++++++++----------
 4 files changed, 30 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c81100c..8606103 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -316,8 +316,6 @@ struct drm_i915_display_funcs {
 	void (*update_wm)(struct drm_device *dev);
 	void (*update_sprite_wm)(struct drm_device *dev, int pipe,
 				 uint32_t sprite_width, int pixel_size);
-	void (*update_linetime_wm)(struct drm_device *dev, int pipe,
-				 struct drm_display_mode *mode);
 	void (*modeset_global_resources)(struct drm_device *dev);
 	/* Returns the active state of the crtc, and if the crtc is active,
 	 * fills out the pipe-config with the hw state. */
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b796752..7bcd60c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6026,8 +6026,6 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
 
 	intel_update_watermarks(dev);
 
-	intel_update_linetime_watermarks(dev, pipe, adjusted_mode);
-
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index be9ad39..80b417a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -727,8 +727,6 @@ extern void intel_update_watermarks(struct drm_device *dev);
 extern void intel_update_sprite_watermarks(struct drm_device *dev, int pipe,
 					   uint32_t sprite_width,
 					   int pixel_size);
-extern void intel_update_linetime_watermarks(struct drm_device *dev, int pipe,
-			 struct drm_display_mode *mode);
 
 extern unsigned long intel_gen4_compute_page_offset(int *x, int *y,
 						    unsigned int tiling_mode,
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 93b8f70..236cd99 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2069,12 +2069,19 @@ static void ivybridge_update_wm(struct drm_device *dev)
 }
 
 static void
-haswell_update_linetime_wm(struct drm_device *dev, int pipe,
-				 struct drm_display_mode *mode)
+haswell_update_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	enum pipe pipe = intel_crtc->pipe;
+	struct drm_display_mode *mode = &intel_crtc->config.adjusted_mode;
 	u32 temp;
 
+	if (!intel_crtc_active(crtc)) {
+		I915_WRITE(PIPE_WM_LINETIME(pipe), 0);
+		return;
+	}
+
 	temp = I915_READ(PIPE_WM_LINETIME(pipe));
 	temp &= ~PIPE_WM_LINETIME_MASK;
 
@@ -2095,6 +2102,26 @@ haswell_update_linetime_wm(struct drm_device *dev, int pipe,
 	I915_WRITE(PIPE_WM_LINETIME(pipe), temp);
 }
 
+static void haswell_update_wm(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_crtc *crtc;
+	enum pipe pipe;
+
+	/* Disable the LP WMs before changine the linetime registers. This is
+	 * just a temporary code that will be replaced soon. */
+	I915_WRITE(WM3_LP_ILK, 0);
+	I915_WRITE(WM2_LP_ILK, 0);
+	I915_WRITE(WM1_LP_ILK, 0);
+
+	for_each_pipe(pipe) {
+		crtc = dev_priv->pipe_to_crtc_mapping[pipe];
+		haswell_update_linetime_wm(dev, crtc);
+	}
+
+	sandybridge_update_wm(dev);
+}
+
 static bool
 sandybridge_compute_sprite_wm(struct drm_device *dev, int plane,
 			      uint32_t sprite_width, int pixel_size,
@@ -2290,15 +2317,6 @@ void intel_update_watermarks(struct drm_device *dev)
 		dev_priv->display.update_wm(dev);
 }
 
-void intel_update_linetime_watermarks(struct drm_device *dev,
-		int pipe, struct drm_display_mode *mode)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-
-	if (dev_priv->display.update_linetime_wm)
-		dev_priv->display.update_linetime_wm(dev, pipe, mode);
-}
-
 void intel_update_sprite_watermarks(struct drm_device *dev, int pipe,
 				    uint32_t sprite_width, int pixel_size)
 {
@@ -4553,9 +4571,8 @@ void intel_init_pm(struct drm_device *dev)
 			dev_priv->display.init_clock_gating = ivybridge_init_clock_gating;
 		} else if (IS_HASWELL(dev)) {
 			if (SNB_READ_WM0_LATENCY()) {
-				dev_priv->display.update_wm = sandybridge_update_wm;
+				dev_priv->display.update_wm = haswell_update_wm;
 				dev_priv->display.update_sprite_wm = sandybridge_update_sprite_wm;
-				dev_priv->display.update_linetime_wm = haswell_update_linetime_wm;
 			} else {
 				DRM_DEBUG_KMS("Failed to read display plane latency. "
 					      "Disable CxSR\n");
-- 
1.7.10.4

Re: [PATCH 2/9] drm/i915: remove intel_update_linetime_watermarks

[Ville Syrjälä]

On Thu, May 09, 2013 at 04:55:50PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> The spec says the linetime watermarks must be programmed before
> enabling any display low power watermarks, but we're currently
> updating the linetime watermarks after we call intel_update_watermarks
> (and only at crtc_mode_set, not at crtc_{enable,disable}). So IMHO the
> best way guarantee the linetime watermarks will be updated before the
> low power watermarks is inside the update_wm function, because it's
> the function that enables low power watermarks. And since Haswell is
> the only platform that has linetime watermarks, let's completely kill
> the "intel_update_linetime_watermarks" abstraction and just use the
> intel_update_watermarks abstraction by creating haswell_update_wm.
> 
> For now haswell_update_wm is still calling sandybridge_update_wm, but
> in the future I plan to implement a function specific to Haswell.
> 
> v2: - Rename patch
>     - Disable LP watermarks before changing linetime WMs (Chris)
>     - Add a comment explaining that this is just temporary code.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |    2 --
>  drivers/gpu/drm/i915/intel_display.c |    2 --
>  drivers/gpu/drm/i915/intel_drv.h     |    2 --
>  drivers/gpu/drm/i915/intel_pm.c      |   43 ++++++++++++++++++++++++----------
>  4 files changed, 30 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index c81100c..8606103 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -316,8 +316,6 @@ struct drm_i915_display_funcs {
>  	void (*update_wm)(struct drm_device *dev);
>  	void (*update_sprite_wm)(struct drm_device *dev, int pipe,
>  				 uint32_t sprite_width, int pixel_size);
> -	void (*update_linetime_wm)(struct drm_device *dev, int pipe,
> -				 struct drm_display_mode *mode);
>  	void (*modeset_global_resources)(struct drm_device *dev);
>  	/* Returns the active state of the crtc, and if the crtc is active,
>  	 * fills out the pipe-config with the hw state. */
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index b796752..7bcd60c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6026,8 +6026,6 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
>  
>  	intel_update_watermarks(dev);
>  
> -	intel_update_linetime_watermarks(dev, pipe, adjusted_mode);
> -
>  	return ret;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index be9ad39..80b417a 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -727,8 +727,6 @@ extern void intel_update_watermarks(struct drm_device *dev);
>  extern void intel_update_sprite_watermarks(struct drm_device *dev, int pipe,
>  					   uint32_t sprite_width,
>  					   int pixel_size);
> -extern void intel_update_linetime_watermarks(struct drm_device *dev, int pipe,
> -			 struct drm_display_mode *mode);
>  
>  extern unsigned long intel_gen4_compute_page_offset(int *x, int *y,
>  						    unsigned int tiling_mode,
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 93b8f70..236cd99 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2069,12 +2069,19 @@ static void ivybridge_update_wm(struct drm_device *dev)
>  }
>  
>  static void
> -haswell_update_linetime_wm(struct drm_device *dev, int pipe,
> -				 struct drm_display_mode *mode)
> +haswell_update_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	enum pipe pipe = intel_crtc->pipe;
> +	struct drm_display_mode *mode = &intel_crtc->config.adjusted_mode;
>  	u32 temp;
>  
> +	if (!intel_crtc_active(crtc)) {
> +		I915_WRITE(PIPE_WM_LINETIME(pipe), 0);
> +		return;
> +	}
> +
>  	temp = I915_READ(PIPE_WM_LINETIME(pipe));
>  	temp &= ~PIPE_WM_LINETIME_MASK;
>  
> @@ -2095,6 +2102,26 @@ haswell_update_linetime_wm(struct drm_device *dev, int pipe,
>  	I915_WRITE(PIPE_WM_LINETIME(pipe), temp);
>  }
>  
> +static void haswell_update_wm(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct drm_crtc *crtc;
> +	enum pipe pipe;
> +
> +	/* Disable the LP WMs before changine the linetime registers. This is
> +	 * just a temporary code that will be replaced soon. */
> +	I915_WRITE(WM3_LP_ILK, 0);
> +	I915_WRITE(WM2_LP_ILK, 0);
> +	I915_WRITE(WM1_LP_ILK, 0);
> +
> +	for_each_pipe(pipe) {
> +		crtc = dev_priv->pipe_to_crtc_mapping[pipe];

list_for_each_entry(crtc, ...)

Otherwise:
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +		haswell_update_linetime_wm(dev, crtc);
> +	}
> +
> +	sandybridge_update_wm(dev);
> +}
> +
>  static bool
>  sandybridge_compute_sprite_wm(struct drm_device *dev, int plane,
>  			      uint32_t sprite_width, int pixel_size,
> @@ -2290,15 +2317,6 @@ void intel_update_watermarks(struct drm_device *dev)
>  		dev_priv->display.update_wm(dev);
>  }
>  
> -void intel_update_linetime_watermarks(struct drm_device *dev,
> -		int pipe, struct drm_display_mode *mode)
> -{
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -
> -	if (dev_priv->display.update_linetime_wm)
> -		dev_priv->display.update_linetime_wm(dev, pipe, mode);
> -}
> -
>  void intel_update_sprite_watermarks(struct drm_device *dev, int pipe,
>  				    uint32_t sprite_width, int pixel_size)
>  {
> @@ -4553,9 +4571,8 @@ void intel_init_pm(struct drm_device *dev)
>  			dev_priv->display.init_clock_gating = ivybridge_init_clock_gating;
>  		} else if (IS_HASWELL(dev)) {
>  			if (SNB_READ_WM0_LATENCY()) {
> -				dev_priv->display.update_wm = sandybridge_update_wm;
> +				dev_priv->display.update_wm = haswell_update_wm;
>  				dev_priv->display.update_sprite_wm = sandybridge_update_sprite_wm;
> -				dev_priv->display.update_linetime_wm = haswell_update_linetime_wm;
>  			} else {
>  				DRM_DEBUG_KMS("Failed to read display plane latency. "
>  					      "Disable CxSR\n");
> -- 
> 1.7.10.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

[PATCH 3/9] drm/i915: use the mode->htotal to calculate linetime watermarks

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

... instead of mode->crtc_display. The spec says "pipe horizontal
total number of pixels" and the "Haswell Watermark Calculator" tool
uses the "Pipe H Total" instead of "Pipe H Src" as the value.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index d056bc9..4cc5f99 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2035,7 +2035,7 @@ haswell_update_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc)
 	 * row at the given clock rate, multiplied by 8.
 	 * */
 	temp |= PIPE_WM_LINETIME_TIME(
-		((mode->crtc_hdisplay * 1000) / mode->clock) * 8);
+		((mode->htotal * 1000) / mode->clock) * 8);
 
 	/* IPS watermarks are only used by pipe A, and are ignored by
 	 * pipes B and C.  They are calculated similarly to the common
-- 
1.7.10.4

Re: [PATCH 3/9] drm/i915: use the mode->htotal to calculate linetime watermarks

[Ville Syrjälä]

On Fri, May 03, 2013 at 05:23:39PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> ... instead of mode->crtc_display. The spec says "pipe horizontal
> total number of pixels" and the "Haswell Watermark Calculator" tool
> uses the "Pipe H Total" instead of "Pipe H Src" as the value.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_pm.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index d056bc9..4cc5f99 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2035,7 +2035,7 @@ haswell_update_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc)
>  	 * row at the given clock rate, multiplied by 8.
>  	 * */
>  	temp |= PIPE_WM_LINETIME_TIME(
> -		((mode->crtc_hdisplay * 1000) / mode->clock) * 8);
> +		((mode->htotal * 1000) / mode->clock) * 8);
>  
>  	/* IPS watermarks are only used by pipe A, and are ignored by
>  	 * pipes B and C.  They are calculated similarly to the common
> -- 
> 1.7.10.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 3/9] drm/i915: use the mode->htotal to calculate linetime watermarks

[Daniel Vetter]

On Fri, May 03, 2013 at 05:23:39PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> ... instead of mode->crtc_display. The spec says "pipe horizontal
> total number of pixels" and the "Haswell Watermark Calculator" tool
> uses the "Pipe H Total" instead of "Pipe H Src" as the value.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index d056bc9..4cc5f99 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2035,7 +2035,7 @@ haswell_update_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc)
>  	 * row at the given clock rate, multiplied by 8.
>  	 * */
>  	temp |= PIPE_WM_LINETIME_TIME(
> -		((mode->crtc_hdisplay * 1000) / mode->clock) * 8);
> +		((mode->htotal * 1000) / mode->clock) * 8);

Big question: What's the right value for interlaced modes? On progressive
mode crtc_ mode values match the non-crtc_ prefixed ones, but not so much
for interlaced modes ... So if your tool expects something resembling what
we program into the pipe registers, we need to change this again.

Merged for now since the spec explicitly says "pixels", but I'd like
someone to double-check this.
-Daniel
>  
>  	/* IPS watermarks are only used by pipe A, and are ignored by
>  	 * pipes B and C.  They are calculated similarly to the common
> -- 
> 1.7.10.4
> 
> _______________________________________________
> 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

Re: [PATCH 3/9] drm/i915: use the mode->htotal to calculate linetime watermarks

[Paulo Zanoni]

2013/5/21 Daniel Vetter <daniel@ffwll.ch>:
> On Fri, May 03, 2013 at 05:23:39PM -0300, Paulo Zanoni wrote:
>> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>>
>> ... instead of mode->crtc_display. The spec says "pipe horizontal
>> total number of pixels" and the "Haswell Watermark Calculator" tool
>> uses the "Pipe H Total" instead of "Pipe H Src" as the value.
>>
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_pm.c |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> index d056bc9..4cc5f99 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -2035,7 +2035,7 @@ haswell_update_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc)
>>        * row at the given clock rate, multiplied by 8.
>>        * */
>>       temp |= PIPE_WM_LINETIME_TIME(
>> -             ((mode->crtc_hdisplay * 1000) / mode->clock) * 8);
>> +             ((mode->htotal * 1000) / mode->clock) * 8);
>
> Big question: What's the right value for interlaced modes? On progressive
> mode crtc_ mode values match the non-crtc_ prefixed ones, but not so much
> for interlaced modes ... So if your tool expects something resembling what
> we program into the pipe registers, we need to change this again.
>
> Merged for now since the spec explicitly says "pixels", but I'd like
> someone to double-check this.

The WM calculator tool says:
"Pipe H total is found in PIPE_HTOTAL horizontal field".

But mode->htotal and mode->crtc_htotal are the same even when the mode
is interlaced. So at least the values we compute are correct. I could
write a follow-up patch updating this if you want.

> -Daniel
>>
>>       /* IPS watermarks are only used by pipe A, and are ignored by
>>        * pipes B and C.  They are calculated similarly to the common
>> --
>> 1.7.10.4
>>
>> _______________________________________________
>> 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



-- 
Paulo Zanoni

Re: [PATCH 3/9] drm/i915: use the mode->htotal to calculate linetime watermarks

[Daniel Vetter]

On Tue, May 21, 2013 at 4:33 PM, Paulo Zanoni <przanoni@gmail.com> wrote:
> 2013/5/21 Daniel Vetter <daniel@ffwll.ch>:
>> On Fri, May 03, 2013 at 05:23:39PM -0300, Paulo Zanoni wrote:
>>> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>>>
>>> ... instead of mode->crtc_display. The spec says "pipe horizontal
>>> total number of pixels" and the "Haswell Watermark Calculator" tool
>>> uses the "Pipe H Total" instead of "Pipe H Src" as the value.
>>>
>>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>>> ---
>>>  drivers/gpu/drm/i915/intel_pm.c |    2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>>> index d056bc9..4cc5f99 100644
>>> --- a/drivers/gpu/drm/i915/intel_pm.c
>>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>>> @@ -2035,7 +2035,7 @@ haswell_update_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc)
>>>        * row at the given clock rate, multiplied by 8.
>>>        * */
>>>       temp |= PIPE_WM_LINETIME_TIME(
>>> -             ((mode->crtc_hdisplay * 1000) / mode->clock) * 8);
>>> +             ((mode->htotal * 1000) / mode->clock) * 8);
>>
>> Big question: What's the right value for interlaced modes? On progressive
>> mode crtc_ mode values match the non-crtc_ prefixed ones, but not so much
>> for interlaced modes ... So if your tool expects something resembling what
>> we program into the pipe registers, we need to change this again.
>>
>> Merged for now since the spec explicitly says "pixels", but I'd like
>> someone to double-check this.
>
> The WM calculator tool says:
> "Pipe H total is found in PIPE_HTOTAL horizontal field".
>
> But mode->htotal and mode->crtc_htotal are the same even when the mode
> is interlaced. So at least the values we compute are correct. I could
> write a follow-up patch updating this if you want.

Oh, I've mixed up the horizontal timings with the vertical ones.
You're right, everything is working correctly as-is.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH 4/9] drm/i915: fix haswell linetime watermarks calculation

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Move the "*8"  calculation to the left side so we don't propagate
rounding errors. Also use DIV_ROUND_CLOSEST because that's what the
spec says we need to do.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 4cc5f99..8468b40 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2035,7 +2035,7 @@ haswell_update_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc)
 	 * row at the given clock rate, multiplied by 8.
 	 * */
 	temp |= PIPE_WM_LINETIME_TIME(
-		((mode->htotal * 1000) / mode->clock) * 8);
+		DIV_ROUND_CLOSEST(mode->htotal * 1000 * 8, mode->clock));
 
 	/* IPS watermarks are only used by pipe A, and are ignored by
 	 * pipes B and C.  They are calculated similarly to the common
-- 
1.7.10.4

Re: [PATCH 4/9] drm/i915: fix haswell linetime watermarks calculation

[Ville Syrjälä]

On Fri, May 03, 2013 at 05:23:40PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> Move the "*8"  calculation to the left side so we don't propagate
> rounding errors. Also use DIV_ROUND_CLOSEST because that's what the
> spec says we need to do.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_pm.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 4cc5f99..8468b40 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2035,7 +2035,7 @@ haswell_update_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc)
>  	 * row at the given clock rate, multiplied by 8.
>  	 * */
>  	temp |= PIPE_WM_LINETIME_TIME(
> -		((mode->htotal * 1000) / mode->clock) * 8);
> +		DIV_ROUND_CLOSEST(mode->htotal * 1000 * 8, mode->clock));
>  
>  	/* IPS watermarks are only used by pipe A, and are ignored by
>  	 * pipes B and C.  They are calculated similarly to the common
> -- 
> 1.7.10.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

[PATCH 5/9] drm/i915: use the correct clock when calculating linetime watermarks

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

If we're using DP/eDP, adjusted_mode->clock may be just the port link
clock, but we also can't use mode->clock because it's wrong when we're
using the using panel fitter.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 8468b40..3ca020c 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2021,6 +2021,7 @@ haswell_update_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc)
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	enum pipe pipe = intel_crtc->pipe;
 	struct drm_display_mode *mode = &intel_crtc->config.adjusted_mode;
+	int target_clock;
 	u32 temp;
 
 	if (!intel_crtc_active(crtc)) {
@@ -2028,6 +2029,11 @@ haswell_update_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc)
 		return;
 	}
 
+	if (intel_crtc->config.pixel_target_clock)
+		target_clock = intel_crtc->config.pixel_target_clock;
+	else
+		target_clock = intel_crtc->config.adjusted_mode.clock;
+
 	temp = I915_READ(PIPE_WM_LINETIME(pipe));
 	temp &= ~PIPE_WM_LINETIME_MASK;
 
@@ -2035,7 +2041,7 @@ haswell_update_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc)
 	 * row at the given clock rate, multiplied by 8.
 	 * */
 	temp |= PIPE_WM_LINETIME_TIME(
-		DIV_ROUND_CLOSEST(mode->htotal * 1000 * 8, mode->clock));
+		DIV_ROUND_CLOSEST(mode->htotal * 1000 * 8, target_clock));
 
 	/* IPS watermarks are only used by pipe A, and are ignored by
 	 * pipes B and C.  They are calculated similarly to the common
-- 
1.7.10.4

Re: [PATCH 5/9] drm/i915: use the correct clock when calculating linetime watermarks

[Ville Syrjälä]

On Fri, May 03, 2013 at 05:23:41PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> If we're using DP/eDP, adjusted_mode->clock may be just the port link
> clock, but we also can't use mode->clock because it's wrong when we're
> using the using panel fitter.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_pm.c |    8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 8468b40..3ca020c 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2021,6 +2021,7 @@ haswell_update_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc)
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	enum pipe pipe = intel_crtc->pipe;
>  	struct drm_display_mode *mode = &intel_crtc->config.adjusted_mode;
> +	int target_clock;
>  	u32 temp;
>  
>  	if (!intel_crtc_active(crtc)) {
> @@ -2028,6 +2029,11 @@ haswell_update_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc)
>  		return;
>  	}
>  
> +	if (intel_crtc->config.pixel_target_clock)
> +		target_clock = intel_crtc->config.pixel_target_clock;
> +	else
> +		target_clock = intel_crtc->config.adjusted_mode.clock;
> +
>  	temp = I915_READ(PIPE_WM_LINETIME(pipe));
>  	temp &= ~PIPE_WM_LINETIME_MASK;
>  
> @@ -2035,7 +2041,7 @@ haswell_update_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc)
>  	 * row at the given clock rate, multiplied by 8.
>  	 * */
>  	temp |= PIPE_WM_LINETIME_TIME(
> -		DIV_ROUND_CLOSEST(mode->htotal * 1000 * 8, mode->clock));
> +		DIV_ROUND_CLOSEST(mode->htotal * 1000 * 8, target_clock));
>  
>  	/* IPS watermarks are only used by pipe A, and are ignored by
>  	 * pipes B and C.  They are calculated similarly to the common
> -- 
> 1.7.10.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 5/9] drm/i915: use the correct clock when calculating linetime watermarks

[Daniel Vetter]

On Fri, May 03, 2013 at 05:23:41PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> If we're using DP/eDP, adjusted_mode->clock may be just the port link
> clock, but we also can't use mode->clock because it's wrong when we're
> using the using panel fitter.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c |    8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 8468b40..3ca020c 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2021,6 +2021,7 @@ haswell_update_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc)
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	enum pipe pipe = intel_crtc->pipe;
>  	struct drm_display_mode *mode = &intel_crtc->config.adjusted_mode;
> +	int target_clock;
>  	u32 temp;
>  
>  	if (!intel_crtc_active(crtc)) {
> @@ -2028,6 +2029,11 @@ haswell_update_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc)
>  		return;
>  	}
>  
> +	if (intel_crtc->config.pixel_target_clock)
> +		target_clock = intel_crtc->config.pixel_target_clock;
> +	else
> +		target_clock = intel_crtc->config.adjusted_mode.clock;

I'll ignore this one here, since I already have the real fix at

http://cgit.freedesktop.org/~danvet/drm/commit/?h=fdi-dither&id=0600051ee5eb6d13d385c8629c0ab0f7809346be

I'll submit the series this patch is a part of asap to intel-gfx.

Cheers, Daniel

> +
>  	temp = I915_READ(PIPE_WM_LINETIME(pipe));
>  	temp &= ~PIPE_WM_LINETIME_MASK;
>  
> @@ -2035,7 +2041,7 @@ haswell_update_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc)
>  	 * row at the given clock rate, multiplied by 8.
>  	 * */
>  	temp |= PIPE_WM_LINETIME_TIME(
> -		DIV_ROUND_CLOSEST(mode->htotal * 1000 * 8, mode->clock));
> +		DIV_ROUND_CLOSEST(mode->htotal * 1000 * 8, target_clock));
>  
>  	/* IPS watermarks are only used by pipe A, and are ignored by
>  	 * pipes B and C.  They are calculated similarly to the common
> -- 
> 1.7.10.4
> 
> _______________________________________________
> 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

[PATCH 6/9] drm/i915: make intel_ddi_get_cdclk_freq return values in KHz

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

With this, that 338 can finally become the correct 337500.

Due to the change we need to adjust the intel_dp_aux_ch function to
set the correct value, so adjust the division and also use
DIV_ROUND_CLOSEST instead of the old "round down" behavior because the
spec says the value "should be programmed to get as close as possible
to the ideal rate of 2MHz".

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c |   10 +++++-----
 drivers/gpu/drm/i915/intel_dp.c  |    3 ++-
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 21fb852..e5b1b63 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1335,14 +1335,14 @@ static void intel_disable_ddi(struct intel_encoder *intel_encoder)
 int intel_ddi_get_cdclk_freq(struct drm_i915_private *dev_priv)
 {
 	if (I915_READ(HSW_FUSE_STRAP) & HSW_CDCLK_LIMIT)
-		return 450;
+		return 450000;
 	else if ((I915_READ(LCPLL_CTL) & LCPLL_CLK_FREQ_MASK) ==
 		 LCPLL_CLK_FREQ_450)
-		return 450;
+		return 450000;
 	else if (IS_ULT(dev_priv->dev))
-		return 338;
+		return 337500;
 	else
-		return 540;
+		return 540000;
 }
 
 void intel_ddi_pll_init(struct drm_device *dev)
@@ -1355,7 +1355,7 @@ void intel_ddi_pll_init(struct drm_device *dev)
 	 * Don't even try to turn it on.
 	 */
 
-	DRM_DEBUG_KMS("CDCLK running at %dMHz\n",
+	DRM_DEBUG_KMS("CDCLK running at %dKHz\n",
 		      intel_ddi_get_cdclk_freq(dev_priv));
 
 	if (val & LCPLL_CD_SOURCE_FCLK)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index a293523..3df1383 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -346,7 +346,8 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 	 */
 	if (is_cpu_edp(intel_dp)) {
 		if (HAS_DDI(dev))
-			aux_clock_divider = intel_ddi_get_cdclk_freq(dev_priv) >> 1;
+			aux_clock_divider = DIV_ROUND_CLOSEST(
+				intel_ddi_get_cdclk_freq(dev_priv), 2000);
 		else if (IS_VALLEYVIEW(dev))
 			aux_clock_divider = 100;
 		else if (IS_GEN6(dev) || IS_GEN7(dev))
-- 
1.7.10.4

Re: [PATCH 6/9] drm/i915: make intel_ddi_get_cdclk_freq return values in KHz

[Chris Wilson]

On Fri, May 03, 2013 at 05:23:42PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> With this, that 338 can finally become the correct 337500.
> 
> Due to the change we need to adjust the intel_dp_aux_ch function to
> set the correct value, so adjust the division and also use
> DIV_ROUND_CLOSEST instead of the old "round down" behavior because the
> spec says the value "should be programmed to get as close as possible
> to the ideal rate of 2MHz".

Can you please demonstrate an instance where this code produces a
different value? And only then correct the constants.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 6/9] drm/i915: make intel_ddi_get_cdclk_freq return values in KHz

[Paulo Zanoni]

2013/5/5 Chris Wilson <chris@chris-wilson.co.uk>:
> On Fri, May 03, 2013 at 05:23:42PM -0300, Paulo Zanoni wrote:
>> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>>
>> With this, that 338 can finally become the correct 337500.
>>
>> Due to the change we need to adjust the intel_dp_aux_ch function to
>> set the correct value, so adjust the division and also use
>> DIV_ROUND_CLOSEST instead of the old "round down" behavior because the
>> spec says the value "should be programmed to get as close as possible
>> to the ideal rate of 2MHz".
>
> Can you please demonstrate an instance where this code produces a
> different value? And only then correct the constants.

I use the 337500 value on the next patch, when setting the
ips_linetime value. The correct frequency is 337500, not 338000.

ips_linetime = DIV_ROUND_CLOSEST(mode->htotal * 1000 * 8,
intel_ddi_get_cdclk_freq);
For a mode with htotal of 2640 [0] we'll have: (i) (2640 * 1000 * 8) /
338000 = 62.48, resulting in 62 and (ii) (2640 * 1000 * 8) / 337500 =
62.57 resulting in 63.

For the case inside intel_dp.c:
Previously we were using 338. So with the old formula we were writing
338/2 = 169 to the register. And 337500 / 169 = 1997.04 (we use 337500
here because it's the real clock value). With the new value of
337500/2000 we'll have 168.75, which is 168 on the round-down case and
169 on the round-closest case. If we write 168 to the register, 337500
/ 168 = 2008.92, and 2008.92 is more distant from 2000 than 1997.04.
So with this patch we're changing the formula but still writing the
same correct value to the DP AUX register.

[0]: That's 1920x1080@50Hz on my DP monitor.

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



-- 
Paulo Zanoni

Re: [PATCH 6/9] drm/i915: make intel_ddi_get_cdclk_freq return values in KHz

[Ville Syrjälä]

On Fri, May 03, 2013 at 05:23:42PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> With this, that 338 can finally become the correct 337500.
> 
> Due to the change we need to adjust the intel_dp_aux_ch function to
> set the correct value, so adjust the division and also use
> DIV_ROUND_CLOSEST instead of the old "round down" behavior because the
> spec says the value "should be programmed to get as close as possible
> to the ideal rate of 2MHz".
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_ddi.c |   10 +++++-----
>  drivers/gpu/drm/i915/intel_dp.c  |    3 ++-
>  2 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 21fb852..e5b1b63 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1335,14 +1335,14 @@ static void intel_disable_ddi(struct intel_encoder *intel_encoder)
>  int intel_ddi_get_cdclk_freq(struct drm_i915_private *dev_priv)
>  {
>  	if (I915_READ(HSW_FUSE_STRAP) & HSW_CDCLK_LIMIT)
> -		return 450;
> +		return 450000;
>  	else if ((I915_READ(LCPLL_CTL) & LCPLL_CLK_FREQ_MASK) ==
>  		 LCPLL_CLK_FREQ_450)
> -		return 450;
> +		return 450000;
>  	else if (IS_ULT(dev_priv->dev))
> -		return 338;
> +		return 337500;
>  	else
> -		return 540;
> +		return 540000;
>  }
>  
>  void intel_ddi_pll_init(struct drm_device *dev)
> @@ -1355,7 +1355,7 @@ void intel_ddi_pll_init(struct drm_device *dev)
>  	 * Don't even try to turn it on.
>  	 */
>  
> -	DRM_DEBUG_KMS("CDCLK running at %dMHz\n",
> +	DRM_DEBUG_KMS("CDCLK running at %dKHz\n",
>  		      intel_ddi_get_cdclk_freq(dev_priv));
>  
>  	if (val & LCPLL_CD_SOURCE_FCLK)
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index a293523..3df1383 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -346,7 +346,8 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
>  	 */
>  	if (is_cpu_edp(intel_dp)) {
>  		if (HAS_DDI(dev))
> -			aux_clock_divider = intel_ddi_get_cdclk_freq(dev_priv) >> 1;
> +			aux_clock_divider = DIV_ROUND_CLOSEST(
> +				intel_ddi_get_cdclk_freq(dev_priv), 2000);
>  		else if (IS_VALLEYVIEW(dev))
>  			aux_clock_divider = 100;
>  		else if (IS_GEN6(dev) || IS_GEN7(dev))
> -- 
> 1.7.10.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

[PATCH 7/9] drm/i915: set the IPS linetime watermark

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Remove the "placeholder" comment and set the actual value described by
the specification. We still don't enable IPS, but it won't hurt to
already have the value set here.

While at it, fully set the register value instead of just masking the
values we're changing.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c |   22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 3ca020c..59bac2e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2022,7 +2022,7 @@ haswell_update_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc)
 	enum pipe pipe = intel_crtc->pipe;
 	struct drm_display_mode *mode = &intel_crtc->config.adjusted_mode;
 	int target_clock;
-	u32 temp;
+	u32 linetime, ips_linetime;
 
 	if (!intel_crtc_active(crtc)) {
 		I915_WRITE(PIPE_WM_LINETIME(pipe), 0);
@@ -2034,24 +2034,16 @@ haswell_update_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc)
 	else
 		target_clock = intel_crtc->config.adjusted_mode.clock;
 
-	temp = I915_READ(PIPE_WM_LINETIME(pipe));
-	temp &= ~PIPE_WM_LINETIME_MASK;
-
 	/* The WM are computed with base on how long it takes to fill a single
 	 * row at the given clock rate, multiplied by 8.
 	 * */
-	temp |= PIPE_WM_LINETIME_TIME(
-		DIV_ROUND_CLOSEST(mode->htotal * 1000 * 8, target_clock));
-
-	/* IPS watermarks are only used by pipe A, and are ignored by
-	 * pipes B and C.  They are calculated similarly to the common
-	 * linetime values, except that we are using CD clock frequency
-	 * in MHz instead of pixel rate for the division.
-	 *
-	 * This is a placeholder for the IPS watermark calculation code.
-	 */
+	linetime = DIV_ROUND_CLOSEST(mode->htotal * 1000 * 8, target_clock);
+	ips_linetime = DIV_ROUND_CLOSEST(mode->htotal * 1000 * 8,
+					 intel_ddi_get_cdclk_freq(dev_priv));
 
-	I915_WRITE(PIPE_WM_LINETIME(pipe), temp);
+	I915_WRITE(PIPE_WM_LINETIME(pipe),
+		   PIPE_WM_LINETIME_IPS_LINETIME(ips_linetime) |
+		   PIPE_WM_LINETIME_TIME(linetime));
 }
 
 static void haswell_update_wm(struct drm_device *dev)
-- 
1.7.10.4

Re: [PATCH 7/9] drm/i915: set the IPS linetime watermark

[Ville Syrjälä]

On Fri, May 03, 2013 at 05:23:43PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> Remove the "placeholder" comment and set the actual value described by
> the specification. We still don't enable IPS, but it won't hurt to
> already have the value set here.
> 
> While at it, fully set the register value instead of just masking the
> values we're changing.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_pm.c |   22 +++++++---------------
>  1 file changed, 7 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 3ca020c..59bac2e 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2022,7 +2022,7 @@ haswell_update_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc)
>  	enum pipe pipe = intel_crtc->pipe;
>  	struct drm_display_mode *mode = &intel_crtc->config.adjusted_mode;
>  	int target_clock;
> -	u32 temp;
> +	u32 linetime, ips_linetime;
>  
>  	if (!intel_crtc_active(crtc)) {
>  		I915_WRITE(PIPE_WM_LINETIME(pipe), 0);
> @@ -2034,24 +2034,16 @@ haswell_update_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc)
>  	else
>  		target_clock = intel_crtc->config.adjusted_mode.clock;
>  
> -	temp = I915_READ(PIPE_WM_LINETIME(pipe));
> -	temp &= ~PIPE_WM_LINETIME_MASK;
> -
>  	/* The WM are computed with base on how long it takes to fill a single
>  	 * row at the given clock rate, multiplied by 8.
>  	 * */
> -	temp |= PIPE_WM_LINETIME_TIME(
> -		DIV_ROUND_CLOSEST(mode->htotal * 1000 * 8, target_clock));
> -
> -	/* IPS watermarks are only used by pipe A, and are ignored by
> -	 * pipes B and C.  They are calculated similarly to the common
> -	 * linetime values, except that we are using CD clock frequency
> -	 * in MHz instead of pixel rate for the division.
> -	 *
> -	 * This is a placeholder for the IPS watermark calculation code.
> -	 */
> +	linetime = DIV_ROUND_CLOSEST(mode->htotal * 1000 * 8, target_clock);
> +	ips_linetime = DIV_ROUND_CLOSEST(mode->htotal * 1000 * 8,
> +					 intel_ddi_get_cdclk_freq(dev_priv));
>  
> -	I915_WRITE(PIPE_WM_LINETIME(pipe), temp);
> +	I915_WRITE(PIPE_WM_LINETIME(pipe),
> +		   PIPE_WM_LINETIME_IPS_LINETIME(ips_linetime) |
> +		   PIPE_WM_LINETIME_TIME(linetime));
>  }
>  
>  static void haswell_update_wm(struct drm_device *dev)
> -- 
> 1.7.10.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

[PATCH 8/9] drm/i915: MCH_SSKPD is a 64 bit register on Haswell

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

And the SNB_READ_WM0_LATENCY macro is not valid anymore because we
have the "New WM0" at 63:56, so the "Old WM0" could maybe be zero if
the new one is not zero.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 59bac2e..b56de92 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4491,7 +4491,7 @@ void intel_init_pm(struct drm_device *dev)
 			}
 			dev_priv->display.init_clock_gating = ivybridge_init_clock_gating;
 		} else if (IS_HASWELL(dev)) {
-			if (SNB_READ_WM0_LATENCY()) {
+			if (I915_READ64(MCH_SSKPD)) {
 				dev_priv->display.update_wm = haswell_update_wm;
 				dev_priv->display.update_sprite_wm = sandybridge_update_sprite_wm;
 			} else {
-- 
1.7.10.4

Re: [PATCH 8/9] drm/i915: MCH_SSKPD is a 64 bit register on Haswell

[Ville Syrjälä]

On Fri, May 03, 2013 at 05:23:44PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> And the SNB_READ_WM0_LATENCY macro is not valid anymore because we
> have the "New WM0" at 63:56, so the "Old WM0" could maybe be zero if
> the new one is not zero.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_pm.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 59bac2e..b56de92 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4491,7 +4491,7 @@ void intel_init_pm(struct drm_device *dev)
>  			}
>  			dev_priv->display.init_clock_gating = ivybridge_init_clock_gating;
>  		} else if (IS_HASWELL(dev)) {
> -			if (SNB_READ_WM0_LATENCY()) {
> +			if (I915_READ64(MCH_SSKPD)) {
>  				dev_priv->display.update_wm = haswell_update_wm;
>  				dev_priv->display.update_sprite_wm = sandybridge_update_sprite_wm;
>  			} else {
> -- 
> 1.7.10.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

[PATCH 9/9] drm/i915: set FORCE_ARB_IDLE_PLANES workaround

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Commit 1544d9d57396d5c0c6b7644ed5ae1f4d6caad07a added a workaround
inside haswell_init_clock_gating and mentioned it is "a workaround for
early silicon revisions and should be removed later". This workaround
is documented in bit 31 of PRI_CTL. I asked Arthur and he mentioned
that setting FORCE_ARB_IDLE_PLANES replaces that workaround for the
newer machines. So use the new one.

Also notice that there's still another workaround for PRI_CTL that
involves WM_DBG, but it's not the one we're reverting. And notice that
we were previously setting WM_DBG_DISALLOW_MULTIPIPE_LP which disables
the LP watermarks when more than one pipe is used, and we really don't
want this because we need the LP watermarks if we want to reach deeper
PC states.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h |    3 +++
 drivers/gpu/drm/i915/intel_pm.c |   10 ++--------
 2 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index aec569f..5879f23 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3697,6 +3697,9 @@
 # define CHICKEN3_DGMG_REQ_OUT_FIX_DISABLE	(1 << 5)
 # define CHICKEN3_DGMG_DONE_FIX_DISABLE		(1 << 2)
 
+#define CHICKEN_PAR1_1		0x42080
+#define  FORCE_ARB_IDLE_PLANES	(1 << 14)
+
 #define DISP_ARB_CTL	0x45000
 #define  DISP_TILE_SURFACE_SWIZZLING	(1<<13)
 #define  DISP_FBC_WM_DIS		(1<<15)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index b56de92..2297476 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4042,14 +4042,8 @@ static void haswell_init_clock_gating(struct drm_device *dev)
 	/* WaSwitchSolVfFArbitrationPriority */
 	I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | HSW_ECOCHK_ARB_PRIO_SOL);
 
-	/* XXX: This is a workaround for early silicon revisions and should be
-	 * removed later.
-	 */
-	I915_WRITE(WM_DBG,
-			I915_READ(WM_DBG) |
-			WM_DBG_DISALLOW_MULTIPLE_LP |
-			WM_DBG_DISALLOW_SPRITE |
-			WM_DBG_DISALLOW_MAXFIFO);
+	I915_WRITE(CHICKEN_PAR1_1,
+		   I915_READ(CHICKEN_PAR1_1) | FORCE_ARB_IDLE_PLANES);
 
 	lpt_init_clock_gating(dev);
 }
-- 
1.7.10.4

Re: [PATCH 9/9] drm/i915: set FORCE_ARB_IDLE_PLANES workaround

[Ville Syrjälä]

On Fri, May 03, 2013 at 05:23:45PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> Commit 1544d9d57396d5c0c6b7644ed5ae1f4d6caad07a added a workaround
> inside haswell_init_clock_gating and mentioned it is "a workaround for
> early silicon revisions and should be removed later". This workaround
> is documented in bit 31 of PRI_CTL. I asked Arthur and he mentioned
> that setting FORCE_ARB_IDLE_PLANES replaces that workaround for the
> newer machines. So use the new one.
> 
> Also notice that there's still another workaround for PRI_CTL that
> involves WM_DBG, but it's not the one we're reverting. And notice that
> we were previously setting WM_DBG_DISALLOW_MULTIPIPE_LP which disables
> the LP watermarks when more than one pipe is used, and we really don't
> want this because we need the LP watermarks if we want to reach deeper
> PC states.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_reg.h |    3 +++
>  drivers/gpu/drm/i915/intel_pm.c |   10 ++--------
>  2 files changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index aec569f..5879f23 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3697,6 +3697,9 @@
>  # define CHICKEN3_DGMG_REQ_OUT_FIX_DISABLE	(1 << 5)
>  # define CHICKEN3_DGMG_DONE_FIX_DISABLE		(1 << 2)
>  
> +#define CHICKEN_PAR1_1		0x42080
> +#define  FORCE_ARB_IDLE_PLANES	(1 << 14)
> +
>  #define DISP_ARB_CTL	0x45000
>  #define  DISP_TILE_SURFACE_SWIZZLING	(1<<13)
>  #define  DISP_FBC_WM_DIS		(1<<15)
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index b56de92..2297476 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4042,14 +4042,8 @@ static void haswell_init_clock_gating(struct drm_device *dev)
>  	/* WaSwitchSolVfFArbitrationPriority */
>  	I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | HSW_ECOCHK_ARB_PRIO_SOL);
>  
> -	/* XXX: This is a workaround for early silicon revisions and should be
> -	 * removed later.
> -	 */
> -	I915_WRITE(WM_DBG,
> -			I915_READ(WM_DBG) |
> -			WM_DBG_DISALLOW_MULTIPLE_LP |
> -			WM_DBG_DISALLOW_SPRITE |
> -			WM_DBG_DISALLOW_MAXFIFO);
> +	I915_WRITE(CHICKEN_PAR1_1,
> +		   I915_READ(CHICKEN_PAR1_1) | FORCE_ARB_IDLE_PLANES);
>  
>  	lpt_init_clock_gating(dev);
>  }
> -- 
> 1.7.10.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 9/9] drm/i915: set FORCE_ARB_IDLE_PLANES workaround

[Daniel Vetter]

On Fri, May 03, 2013 at 05:23:45PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> Commit 1544d9d57396d5c0c6b7644ed5ae1f4d6caad07a added a workaround
> inside haswell_init_clock_gating and mentioned it is "a workaround for
> early silicon revisions and should be removed later". This workaround
> is documented in bit 31 of PRI_CTL. I asked Arthur and he mentioned
> that setting FORCE_ARB_IDLE_PLANES replaces that workaround for the
> newer machines. So use the new one.
> 
> Also notice that there's still another workaround for PRI_CTL that
> involves WM_DBG, but it's not the one we're reverting. And notice that
> we were previously setting WM_DBG_DISALLOW_MULTIPIPE_LP which disables
> the LP watermarks when more than one pipe is used, and we really don't
> want this because we need the LP watermarks if we want to reach deeper
> PC states.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

I've applied all the patches in this series safe for the target_clock one.
But this patch here is missing the w/a tag, can you please supply that one
in a quick reply so that I can smash it into the patch?

Thanks, Daniel
> ---
>  drivers/gpu/drm/i915/i915_reg.h |    3 +++
>  drivers/gpu/drm/i915/intel_pm.c |   10 ++--------
>  2 files changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index aec569f..5879f23 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3697,6 +3697,9 @@
>  # define CHICKEN3_DGMG_REQ_OUT_FIX_DISABLE	(1 << 5)
>  # define CHICKEN3_DGMG_DONE_FIX_DISABLE		(1 << 2)
>  
> +#define CHICKEN_PAR1_1		0x42080
> +#define  FORCE_ARB_IDLE_PLANES	(1 << 14)
> +
>  #define DISP_ARB_CTL	0x45000
>  #define  DISP_TILE_SURFACE_SWIZZLING	(1<<13)
>  #define  DISP_FBC_WM_DIS		(1<<15)
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index b56de92..2297476 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4042,14 +4042,8 @@ static void haswell_init_clock_gating(struct drm_device *dev)
>  	/* WaSwitchSolVfFArbitrationPriority */
>  	I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | HSW_ECOCHK_ARB_PRIO_SOL);
>  
> -	/* XXX: This is a workaround for early silicon revisions and should be
> -	 * removed later.
> -	 */
> -	I915_WRITE(WM_DBG,
> -			I915_READ(WM_DBG) |
> -			WM_DBG_DISALLOW_MULTIPLE_LP |
> -			WM_DBG_DISALLOW_SPRITE |
> -			WM_DBG_DISALLOW_MAXFIFO);
> +	I915_WRITE(CHICKEN_PAR1_1,
> +		   I915_READ(CHICKEN_PAR1_1) | FORCE_ARB_IDLE_PLANES);
>  
>  	lpt_init_clock_gating(dev);
>  }
> -- 
> 1.7.10.4
> 
> _______________________________________________
> 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

Re: [PATCH 9/9] drm/i915: set FORCE_ARB_IDLE_PLANES workaround

[Paulo Zanoni]

2013/5/21 Daniel Vetter <daniel@ffwll.ch>:
> On Fri, May 03, 2013 at 05:23:45PM -0300, Paulo Zanoni wrote:
>> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>>
>> Commit 1544d9d57396d5c0c6b7644ed5ae1f4d6caad07a added a workaround
>> inside haswell_init_clock_gating and mentioned it is "a workaround for
>> early silicon revisions and should be removed later". This workaround
>> is documented in bit 31 of PRI_CTL. I asked Arthur and he mentioned
>> that setting FORCE_ARB_IDLE_PLANES replaces that workaround for the
>> newer machines. So use the new one.
>>
>> Also notice that there's still another workaround for PRI_CTL that
>> involves WM_DBG, but it's not the one we're reverting. And notice that
>> we were previously setting WM_DBG_DISALLOW_MULTIPIPE_LP which disables
>> the LP watermarks when more than one pipe is used, and we really don't
>> want this because we need the LP watermarks if we want to reach deeper
>> PC states.
>>
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> I've applied all the patches in this series safe for the target_clock one.
> But this patch here is missing the w/a tag, can you please supply that one
> in a quick reply so that I can smash it into the patch?

Just like most display workarounds, this one doesn't have a name.

>
> Thanks, Daniel
>> ---
>>  drivers/gpu/drm/i915/i915_reg.h |    3 +++
>>  drivers/gpu/drm/i915/intel_pm.c |   10 ++--------
>>  2 files changed, 5 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index aec569f..5879f23 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -3697,6 +3697,9 @@
>>  # define CHICKEN3_DGMG_REQ_OUT_FIX_DISABLE   (1 << 5)
>>  # define CHICKEN3_DGMG_DONE_FIX_DISABLE              (1 << 2)
>>
>> +#define CHICKEN_PAR1_1               0x42080
>> +#define  FORCE_ARB_IDLE_PLANES       (1 << 14)
>> +
>>  #define DISP_ARB_CTL 0x45000
>>  #define  DISP_TILE_SURFACE_SWIZZLING (1<<13)
>>  #define  DISP_FBC_WM_DIS             (1<<15)
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> index b56de92..2297476 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -4042,14 +4042,8 @@ static void haswell_init_clock_gating(struct drm_device *dev)
>>       /* WaSwitchSolVfFArbitrationPriority */
>>       I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | HSW_ECOCHK_ARB_PRIO_SOL);
>>
>> -     /* XXX: This is a workaround for early silicon revisions and should be
>> -      * removed later.
>> -      */
>> -     I915_WRITE(WM_DBG,
>> -                     I915_READ(WM_DBG) |
>> -                     WM_DBG_DISALLOW_MULTIPLE_LP |
>> -                     WM_DBG_DISALLOW_SPRITE |
>> -                     WM_DBG_DISALLOW_MAXFIFO);
>> +     I915_WRITE(CHICKEN_PAR1_1,
>> +                I915_READ(CHICKEN_PAR1_1) | FORCE_ARB_IDLE_PLANES);
>>
>>       lpt_init_clock_gating(dev);
>>  }
>> --
>> 1.7.10.4
>>
>> _______________________________________________
>> 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



-- 
Paulo Zanoni

Re: [PATCH 9/9] drm/i915: set FORCE_ARB_IDLE_PLANES workaround

[Ville Syrjälä]

On Tue, May 21, 2013 at 10:27:53AM -0300, Paulo Zanoni wrote:
> 2013/5/21 Daniel Vetter <daniel@ffwll.ch>:
> > On Fri, May 03, 2013 at 05:23:45PM -0300, Paulo Zanoni wrote:
> >> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >>
> >> Commit 1544d9d57396d5c0c6b7644ed5ae1f4d6caad07a added a workaround
> >> inside haswell_init_clock_gating and mentioned it is "a workaround for
> >> early silicon revisions and should be removed later". This workaround
> >> is documented in bit 31 of PRI_CTL. I asked Arthur and he mentioned
> >> that setting FORCE_ARB_IDLE_PLANES replaces that workaround for the
> >> newer machines. So use the new one.
> >>
> >> Also notice that there's still another workaround for PRI_CTL that
> >> involves WM_DBG, but it's not the one we're reverting. And notice that
> >> we were previously setting WM_DBG_DISALLOW_MULTIPIPE_LP which disables
> >> the LP watermarks when more than one pipe is used, and we really don't
> >> want this because we need the LP watermarks if we want to reach deeper
> >> PC states.
> >>
> >> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >
> > I've applied all the patches in this series safe for the target_clock one.
> > But this patch here is missing the w/a tag, can you please supply that one
> > in a quick reply so that I can smash it into the patch?
> 
> Just like most display workarounds, this one doesn't have a name.

Actually I found a name for it in BSpec: WaRsPkgCStateDisplayPMReq

> 
> >
> > Thanks, Daniel
> >> ---
> >>  drivers/gpu/drm/i915/i915_reg.h |    3 +++
> >>  drivers/gpu/drm/i915/intel_pm.c |   10 ++--------
> >>  2 files changed, 5 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> >> index aec569f..5879f23 100644
> >> --- a/drivers/gpu/drm/i915/i915_reg.h
> >> +++ b/drivers/gpu/drm/i915/i915_reg.h
> >> @@ -3697,6 +3697,9 @@
> >>  # define CHICKEN3_DGMG_REQ_OUT_FIX_DISABLE   (1 << 5)
> >>  # define CHICKEN3_DGMG_DONE_FIX_DISABLE              (1 << 2)
> >>
> >> +#define CHICKEN_PAR1_1               0x42080
> >> +#define  FORCE_ARB_IDLE_PLANES       (1 << 14)
> >> +
> >>  #define DISP_ARB_CTL 0x45000
> >>  #define  DISP_TILE_SURFACE_SWIZZLING (1<<13)
> >>  #define  DISP_FBC_WM_DIS             (1<<15)
> >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> >> index b56de92..2297476 100644
> >> --- a/drivers/gpu/drm/i915/intel_pm.c
> >> +++ b/drivers/gpu/drm/i915/intel_pm.c
> >> @@ -4042,14 +4042,8 @@ static void haswell_init_clock_gating(struct drm_device *dev)
> >>       /* WaSwitchSolVfFArbitrationPriority */
> >>       I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | HSW_ECOCHK_ARB_PRIO_SOL);
> >>
> >> -     /* XXX: This is a workaround for early silicon revisions and should be
> >> -      * removed later.
> >> -      */
> >> -     I915_WRITE(WM_DBG,
> >> -                     I915_READ(WM_DBG) |
> >> -                     WM_DBG_DISALLOW_MULTIPLE_LP |
> >> -                     WM_DBG_DISALLOW_SPRITE |
> >> -                     WM_DBG_DISALLOW_MAXFIFO);
> >> +     I915_WRITE(CHICKEN_PAR1_1,
> >> +                I915_READ(CHICKEN_PAR1_1) | FORCE_ARB_IDLE_PLANES);
> >>
> >>       lpt_init_clock_gating(dev);
> >>  }
> >> --
> >> 1.7.10.4
> >>
> >> _______________________________________________
> >> 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
> 
> 
> 
> -- 
> Paulo Zanoni
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 9/9] drm/i915: set FORCE_ARB_IDLE_PLANES workaround

[Daniel Vetter]

On Thu, May 23, 2013 at 12:36 PM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
>> > I've applied all the patches in this series safe for the target_clock one.
>> > But this patch here is missing the w/a tag, can you please supply that one
>> > in a quick reply so that I can smash it into the patch?
>>
>> Just like most display workarounds, this one doesn't have a name.
>
> Actually I found a name for it in BSpec: WaRsPkgCStateDisplayPMReq

Comment added, thanks for digging this one out.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch