[PATCH 0/17] drm/i915: ILK+ watermark prep patches

[PATCH 0/17] drm/i915: ILK+ watermark prep patches

[ville.syrjala]

Here's a repost of the beginning of the ILK watermark series.

I was a bit lazy and decided to repost all the patches up to a point. I
dropped the 'bool is_lp' > 'int level' patch, and I added a few extra
patches to the end of the series.

I still left the WaDoubleCursorLP3Latency handling in because I didn't
want to change the current behaviour of the code. Killing the W/A
should be a separate patch.

[PATCH v2 01/17] drm/i915: Add scaled paramater to update_sprite_watermarks()

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

For calculating watermarks we want to know whether sprites are
scaled. Pass that information to update_sprite_watermarks() so that
eventually we may do some watermark pre-computing.

v2: Use "enabled" consistently, fix commit msg

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h     |  2 +-
 drivers/gpu/drm/i915/intel_drv.h    |  7 ++++---
 drivers/gpu/drm/i915/intel_pm.c     | 15 ++++++++-------
 drivers/gpu/drm/i915/intel_sprite.c | 11 +++++++----
 4 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 82ea281..fe466bc 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -361,7 +361,7 @@ 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,
-				 bool enable);
+				 bool enabled, bool scaled);
 	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_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 474797b..ed33976 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -349,7 +349,8 @@ struct intel_plane {
 	 * for the watermark calculations. Currently only Haswell uses this.
 	 */
 	struct {
-		bool enable;
+		bool enabled;
+		bool scaled;
 		uint8_t bytes_per_pixel;
 		uint32_t horiz_pixels;
 	} wm;
@@ -770,8 +771,8 @@ extern void intel_ddi_init(struct drm_device *dev, enum port port);
 /* For use by IVB LP watermark workaround in intel_sprite.c */
 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, bool enable);
+					   uint32_t sprite_width, int pixel_size,
+					   bool enabled, bool scaled);
 
 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 0a5ba92..5a008d1 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2388,7 +2388,7 @@ static void hsw_compute_wm_parameters(struct drm_device *dev,
 		pipe = intel_plane->pipe;
 		p = &params[pipe];
 
-		p->sprite_enabled = intel_plane->wm.enable;
+		p->sprite_enabled = intel_plane->wm.enabled;
 		p->spr_bytes_per_pixel = intel_plane->wm.bytes_per_pixel;
 		p->spr_horiz_pixels = intel_plane->wm.horiz_pixels;
 
@@ -2616,7 +2616,7 @@ static void haswell_update_wm(struct drm_device *dev)
 
 static void haswell_update_sprite_wm(struct drm_device *dev, int pipe,
 				     uint32_t sprite_width, int pixel_size,
-				     bool enable)
+				     bool enabled, bool scaled)
 {
 	struct drm_plane *plane;
 
@@ -2624,7 +2624,8 @@ static void haswell_update_sprite_wm(struct drm_device *dev, int pipe,
 		struct intel_plane *intel_plane = to_intel_plane(plane);
 
 		if (intel_plane->pipe == pipe) {
-			intel_plane->wm.enable = enable;
+			intel_plane->wm.enabled = enabled;
+			intel_plane->wm.scaled = scaled;
 			intel_plane->wm.horiz_pixels = sprite_width + 1;
 			intel_plane->wm.bytes_per_pixel = pixel_size;
 			break;
@@ -2712,7 +2713,7 @@ sandybridge_compute_sprite_srwm(struct drm_device *dev, int plane,
 
 static void sandybridge_update_sprite_wm(struct drm_device *dev, int pipe,
 					 uint32_t sprite_width, int pixel_size,
-					 bool enable)
+					 bool enabled, bool scaled)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int latency = SNB_READ_WM0_LATENCY() * 100;	/* In unit 0.1us */
@@ -2720,7 +2721,7 @@ static void sandybridge_update_sprite_wm(struct drm_device *dev, int pipe,
 	int sprite_wm, reg;
 	int ret;
 
-	if (!enable)
+	if (!enabled)
 		return;
 
 	switch (pipe) {
@@ -2835,13 +2836,13 @@ void intel_update_watermarks(struct drm_device *dev)
 
 void intel_update_sprite_watermarks(struct drm_device *dev, int pipe,
 				    uint32_t sprite_width, int pixel_size,
-				    bool enable)
+				    bool enabled, bool scaled)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
 	if (dev_priv->display.update_sprite_wm)
 		dev_priv->display.update_sprite_wm(dev, pipe, sprite_width,
-						   pixel_size, enable);
+						   pixel_size, enabled, scaled);
 }
 
 static struct drm_i915_gem_object *
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 55bdf70..069155f 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -114,7 +114,8 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_framebuffer *fb,
 	crtc_w--;
 	crtc_h--;
 
-	intel_update_sprite_watermarks(dev, pipe, crtc_w, pixel_size, true);
+	intel_update_sprite_watermarks(dev, pipe, crtc_w, pixel_size, true,
+				       src_w != crtc_w || src_h != crtc_h);
 
 	I915_WRITE(SPSTRIDE(pipe, plane), fb->pitches[0]);
 	I915_WRITE(SPPOS(pipe, plane), (crtc_y << 16) | crtc_x);
@@ -268,7 +269,8 @@ ivb_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
 	crtc_w--;
 	crtc_h--;
 
-	intel_update_sprite_watermarks(dev, pipe, crtc_w, pixel_size, true);
+	intel_update_sprite_watermarks(dev, pipe, crtc_w, pixel_size, true,
+				       src_w != crtc_w || src_h != crtc_h);
 
 	/*
 	 * IVB workaround: must disable low power watermarks for at least
@@ -336,7 +338,7 @@ ivb_disable_plane(struct drm_plane *plane)
 
 	dev_priv->sprite_scaling_enabled &= ~(1 << pipe);
 
-	intel_update_sprite_watermarks(dev, pipe, 0, 0, false);
+	intel_update_sprite_watermarks(dev, pipe, 0, 0, false, false);
 
 	/* potentially re-enable LP watermarks */
 	if (scaling_was_enabled && !dev_priv->sprite_scaling_enabled)
@@ -456,7 +458,8 @@ ilk_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
 	crtc_w--;
 	crtc_h--;
 
-	intel_update_sprite_watermarks(dev, pipe, crtc_w, pixel_size, true);
+	intel_update_sprite_watermarks(dev, pipe, crtc_w, pixel_size, true,
+				       src_w != crtc_w || src_h != crtc_h);
 
 	dvsscale = 0;
 	if (IS_GEN5(dev) || crtc_w != src_w || crtc_h != src_h)
-- 
1.8.1.5

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

[PATCH 02/17] drm/i915: Pass the actual sprite width to watermarks functions

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Don't subtract one from the sprite width before watermark calculations.

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c     |  2 +-
 drivers/gpu/drm/i915/intel_sprite.c | 18 +++++++++---------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 5a008d1..ea03934 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2626,7 +2626,7 @@ static void haswell_update_sprite_wm(struct drm_device *dev, int pipe,
 		if (intel_plane->pipe == pipe) {
 			intel_plane->wm.enabled = enabled;
 			intel_plane->wm.scaled = scaled;
-			intel_plane->wm.horiz_pixels = sprite_width + 1;
+			intel_plane->wm.horiz_pixels = sprite_width;
 			intel_plane->wm.bytes_per_pixel = pixel_size;
 			break;
 		}
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 069155f..3e3a6d0 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -108,15 +108,15 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_framebuffer *fb,
 
 	sprctl |= SP_ENABLE;
 
+	intel_update_sprite_watermarks(dev, pipe, crtc_w, pixel_size, true,
+				       src_w != crtc_w || src_h != crtc_h);
+
 	/* Sizes are 0 based */
 	src_w--;
 	src_h--;
 	crtc_w--;
 	crtc_h--;
 
-	intel_update_sprite_watermarks(dev, pipe, crtc_w, pixel_size, true,
-				       src_w != crtc_w || src_h != crtc_h);
-
 	I915_WRITE(SPSTRIDE(pipe, plane), fb->pitches[0]);
 	I915_WRITE(SPPOS(pipe, plane), (crtc_y << 16) | crtc_x);
 
@@ -263,15 +263,15 @@ ivb_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
 	if (IS_HASWELL(dev))
 		sprctl |= SPRITE_PIPE_CSC_ENABLE;
 
+	intel_update_sprite_watermarks(dev, pipe, crtc_w, pixel_size, true,
+				       src_w != crtc_w || src_h != crtc_h);
+
 	/* Sizes are 0 based */
 	src_w--;
 	src_h--;
 	crtc_w--;
 	crtc_h--;
 
-	intel_update_sprite_watermarks(dev, pipe, crtc_w, pixel_size, true,
-				       src_w != crtc_w || src_h != crtc_h);
-
 	/*
 	 * IVB workaround: must disable low power watermarks for at least
 	 * one frame before enabling scaling.  LP watermarks can be re-enabled
@@ -452,15 +452,15 @@ ilk_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
 		dvscntr |= DVS_TRICKLE_FEED_DISABLE; /* must disable */
 	dvscntr |= DVS_ENABLE;
 
+	intel_update_sprite_watermarks(dev, pipe, crtc_w, pixel_size, true,
+				       src_w != crtc_w || src_h != crtc_h);
+
 	/* Sizes are 0 based */
 	src_w--;
 	src_h--;
 	crtc_w--;
 	crtc_h--;
 
-	intel_update_sprite_watermarks(dev, pipe, crtc_w, pixel_size, true,
-				       src_w != crtc_w || src_h != crtc_h);
-
 	dvsscale = 0;
 	if (IS_GEN5(dev) || crtc_w != src_w || crtc_h != src_h)
 		dvsscale = DVS_SCALE_ENABLE | (src_w << 16) | src_h;
-- 
1.8.1.5

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

[PATCH 03/17] drm/i915: Calculate the sprite WM based on the source width instead of the destination width

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Using the destination width in the sprite WM calculations isn't correct.
We should be using the source width.

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_sprite.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 3e3a6d0..5a36afb 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -108,7 +108,7 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_framebuffer *fb,
 
 	sprctl |= SP_ENABLE;
 
-	intel_update_sprite_watermarks(dev, pipe, crtc_w, pixel_size, true,
+	intel_update_sprite_watermarks(dev, pipe, src_w, pixel_size, true,
 				       src_w != crtc_w || src_h != crtc_h);
 
 	/* Sizes are 0 based */
@@ -263,7 +263,7 @@ ivb_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
 	if (IS_HASWELL(dev))
 		sprctl |= SPRITE_PIPE_CSC_ENABLE;
 
-	intel_update_sprite_watermarks(dev, pipe, crtc_w, pixel_size, true,
+	intel_update_sprite_watermarks(dev, pipe, src_w, pixel_size, true,
 				       src_w != crtc_w || src_h != crtc_h);
 
 	/* Sizes are 0 based */
@@ -452,7 +452,7 @@ ilk_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
 		dvscntr |= DVS_TRICKLE_FEED_DISABLE; /* must disable */
 	dvscntr |= DVS_ENABLE;
 
-	intel_update_sprite_watermarks(dev, pipe, crtc_w, pixel_size, true,
+	intel_update_sprite_watermarks(dev, pipe, src_w, pixel_size, true,
 				       src_w != crtc_w || src_h != crtc_h);
 
 	/* Sizes are 0 based */
-- 
1.8.1.5

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

[PATCH 04/17] drm/i915: Rename hsw_wm_get_pixel_rate to ilk_pipe_pixel_rate

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

hsw_wm_get_pixel_rate() isn't specific to HSW. In fact it should be made
to handle all gens, but for now it depends on the PCH panel fitter
state, so give it an ilk_ prefix.

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index ea03934..a2975e9 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2080,8 +2080,8 @@ static void ivybridge_update_wm(struct drm_device *dev)
 		   cursor_wm);
 }
 
-static uint32_t hsw_wm_get_pixel_rate(struct drm_device *dev,
-				      struct drm_crtc *crtc)
+static uint32_t ilk_pipe_pixel_rate(struct drm_device *dev,
+				    struct drm_crtc *crtc)
 {
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	uint32_t pixel_rate, pfit_size;
@@ -2373,7 +2373,7 @@ static void hsw_compute_wm_parameters(struct drm_device *dev,
 		pipes_active++;
 
 		p->pipe_htotal = intel_crtc->config.adjusted_mode.htotal;
-		p->pixel_rate = hsw_wm_get_pixel_rate(dev, crtc);
+		p->pixel_rate = ilk_pipe_pixel_rate(dev, crtc);
 		p->pri_bytes_per_pixel = crtc->fb->bits_per_pixel / 8;
 		p->cur_bytes_per_pixel = 4;
 		p->pri_horiz_pixels =
-- 
1.8.1.5

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

[PATCH 05/17] drm/i915: Rename most wm compute functions to ilk_ prefix

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

These functions are appropriate for everything since ILK.

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> (SNB/IVB only)
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 40 ++++++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a2975e9..61249f2 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2111,7 +2111,7 @@ static uint32_t ilk_pipe_pixel_rate(struct drm_device *dev,
 	return pixel_rate;
 }
 
-static uint32_t hsw_wm_method1(uint32_t pixel_rate, uint8_t bytes_per_pixel,
+static uint32_t ilk_wm_method1(uint32_t pixel_rate, uint8_t bytes_per_pixel,
 			       uint32_t latency)
 {
 	uint64_t ret;
@@ -2122,7 +2122,7 @@ static uint32_t hsw_wm_method1(uint32_t pixel_rate, uint8_t bytes_per_pixel,
 	return ret;
 }
 
-static uint32_t hsw_wm_method2(uint32_t pixel_rate, uint32_t pipe_htotal,
+static uint32_t ilk_wm_method2(uint32_t pixel_rate, uint32_t pipe_htotal,
 			       uint32_t horiz_pixels, uint8_t bytes_per_pixel,
 			       uint32_t latency)
 {
@@ -2134,7 +2134,7 @@ static uint32_t hsw_wm_method2(uint32_t pixel_rate, uint32_t pipe_htotal,
 	return ret;
 }
 
-static uint32_t hsw_wm_fbc(uint32_t pri_val, uint32_t horiz_pixels,
+static uint32_t ilk_wm_fbc(uint32_t pri_val, uint32_t horiz_pixels,
 			   uint8_t bytes_per_pixel)
 {
 	return DIV_ROUND_UP(pri_val * 64, horiz_pixels * bytes_per_pixel) + 2;
@@ -2183,7 +2183,7 @@ enum hsw_data_buf_partitioning {
 };
 
 /* For both WM_PIPE and WM_LP. */
-static uint32_t hsw_compute_pri_wm(struct hsw_pipe_wm_parameters *params,
+static uint32_t ilk_compute_pri_wm(struct hsw_pipe_wm_parameters *params,
 				   uint32_t mem_value,
 				   bool is_lp)
 {
@@ -2193,14 +2193,14 @@ static uint32_t hsw_compute_pri_wm(struct hsw_pipe_wm_parameters *params,
 	if (!params->active)
 		return 0;
 
-	method1 = hsw_wm_method1(params->pixel_rate,
+	method1 = ilk_wm_method1(params->pixel_rate,
 				 params->pri_bytes_per_pixel,
 				 mem_value);
 
 	if (!is_lp)
 		return method1;
 
-	method2 = hsw_wm_method2(params->pixel_rate,
+	method2 = ilk_wm_method2(params->pixel_rate,
 				 params->pipe_htotal,
 				 params->pri_horiz_pixels,
 				 params->pri_bytes_per_pixel,
@@ -2210,7 +2210,7 @@ static uint32_t hsw_compute_pri_wm(struct hsw_pipe_wm_parameters *params,
 }
 
 /* For both WM_PIPE and WM_LP. */
-static uint32_t hsw_compute_spr_wm(struct hsw_pipe_wm_parameters *params,
+static uint32_t ilk_compute_spr_wm(struct hsw_pipe_wm_parameters *params,
 				   uint32_t mem_value)
 {
 	uint32_t method1, method2;
@@ -2218,10 +2218,10 @@ static uint32_t hsw_compute_spr_wm(struct hsw_pipe_wm_parameters *params,
 	if (!params->active || !params->sprite_enabled)
 		return 0;
 
-	method1 = hsw_wm_method1(params->pixel_rate,
+	method1 = ilk_wm_method1(params->pixel_rate,
 				 params->spr_bytes_per_pixel,
 				 mem_value);
-	method2 = hsw_wm_method2(params->pixel_rate,
+	method2 = ilk_wm_method2(params->pixel_rate,
 				 params->pipe_htotal,
 				 params->spr_horiz_pixels,
 				 params->spr_bytes_per_pixel,
@@ -2230,13 +2230,13 @@ static uint32_t hsw_compute_spr_wm(struct hsw_pipe_wm_parameters *params,
 }
 
 /* For both WM_PIPE and WM_LP. */
-static uint32_t hsw_compute_cur_wm(struct hsw_pipe_wm_parameters *params,
+static uint32_t ilk_compute_cur_wm(struct hsw_pipe_wm_parameters *params,
 				   uint32_t mem_value)
 {
 	if (!params->active)
 		return 0;
 
-	return hsw_wm_method2(params->pixel_rate,
+	return ilk_wm_method2(params->pixel_rate,
 			      params->pipe_htotal,
 			      params->cur_horiz_pixels,
 			      params->cur_bytes_per_pixel,
@@ -2244,14 +2244,14 @@ static uint32_t hsw_compute_cur_wm(struct hsw_pipe_wm_parameters *params,
 }
 
 /* Only for WM_LP. */
-static uint32_t hsw_compute_fbc_wm(struct hsw_pipe_wm_parameters *params,
+static uint32_t ilk_compute_fbc_wm(struct hsw_pipe_wm_parameters *params,
 				   uint32_t pri_val,
 				   uint32_t mem_value)
 {
 	if (!params->active)
 		return 0;
 
-	return hsw_wm_fbc(pri_val,
+	return ilk_wm_fbc(pri_val,
 			  params->pri_horiz_pixels,
 			  params->pri_bytes_per_pixel);
 }
@@ -2266,10 +2266,10 @@ static bool hsw_compute_lp_wm(uint32_t mem_value, struct hsw_wm_maximums *max,
 	for (pipe = PIPE_A; pipe <= PIPE_C; pipe++) {
 		struct hsw_pipe_wm_parameters *p = &params[pipe];
 
-		pri_val[pipe] = hsw_compute_pri_wm(p, mem_value, true);
-		spr_val[pipe] = hsw_compute_spr_wm(p, mem_value);
-		cur_val[pipe] = hsw_compute_cur_wm(p, mem_value);
-		fbc_val[pipe] = hsw_compute_fbc_wm(p, pri_val[pipe], mem_value);
+		pri_val[pipe] = ilk_compute_pri_wm(p, mem_value, true);
+		spr_val[pipe] = ilk_compute_spr_wm(p, mem_value);
+		cur_val[pipe] = ilk_compute_cur_wm(p, mem_value);
+		fbc_val[pipe] = ilk_compute_fbc_wm(p, pri_val[pipe], mem_value);
 	}
 
 	result->pri_val = max3(pri_val[0], pri_val[1], pri_val[2]);
@@ -2296,9 +2296,9 @@ static uint32_t hsw_compute_wm_pipe(struct drm_i915_private *dev_priv,
 {
 	uint32_t pri_val, cur_val, spr_val;
 
-	pri_val = hsw_compute_pri_wm(params, mem_value, false);
-	spr_val = hsw_compute_spr_wm(params, mem_value);
-	cur_val = hsw_compute_cur_wm(params, mem_value);
+	pri_val = ilk_compute_pri_wm(params, mem_value, false);
+	spr_val = ilk_compute_spr_wm(params, mem_value);
+	cur_val = ilk_compute_cur_wm(params, mem_value);
 
 	WARN(pri_val > 127,
 	     "Primary WM error, mode not supported for pipe %c\n",
-- 
1.8.1.5

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

[PATCH 06/17] drm/i915: Don't pass "mem_value" to ilk_compute_fbc_wm

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The FBC watermark doesn't depend on the latency value, so no point in
passing it in.

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 61249f2..09e2336 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2245,8 +2245,7 @@ static uint32_t ilk_compute_cur_wm(struct hsw_pipe_wm_parameters *params,
 
 /* Only for WM_LP. */
 static uint32_t ilk_compute_fbc_wm(struct hsw_pipe_wm_parameters *params,
-				   uint32_t pri_val,
-				   uint32_t mem_value)
+				   uint32_t pri_val)
 {
 	if (!params->active)
 		return 0;
@@ -2269,7 +2268,7 @@ static bool hsw_compute_lp_wm(uint32_t mem_value, struct hsw_wm_maximums *max,
 		pri_val[pipe] = ilk_compute_pri_wm(p, mem_value, true);
 		spr_val[pipe] = ilk_compute_spr_wm(p, mem_value);
 		cur_val[pipe] = ilk_compute_cur_wm(p, mem_value);
-		fbc_val[pipe] = ilk_compute_fbc_wm(p, pri_val[pipe], mem_value);
+		fbc_val[pipe] = ilk_compute_fbc_wm(p, pri_val[pipe]);
 	}
 
 	result->pri_val = max3(pri_val[0], pri_val[1], pri_val[2]);
-- 
1.8.1.5

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

[PATCH 07/17] drm/i915: Change the watermark latency type to uint16_t

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The latency values fit in uint16_t, so let's save a few bytes.

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 09e2336..9417187 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2338,7 +2338,7 @@ hsw_compute_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc)
 
 static void hsw_compute_wm_parameters(struct drm_device *dev,
 				      struct hsw_pipe_wm_parameters *params,
-				      uint32_t *wm,
+				      uint16_t *wm,
 				      struct hsw_wm_maximums *lp_max_1_2,
 				      struct hsw_wm_maximums *lp_max_5_6)
 {
@@ -2411,7 +2411,7 @@ static void hsw_compute_wm_parameters(struct drm_device *dev,
 
 static void hsw_compute_wm_results(struct drm_device *dev,
 				   struct hsw_pipe_wm_parameters *params,
-				   uint32_t *wm,
+				   uint16_t *wm,
 				   struct hsw_wm_maximums *lp_maximums,
 				   struct hsw_wm_values *results)
 {
@@ -2593,7 +2593,7 @@ static void haswell_update_wm(struct drm_device *dev)
 	struct hsw_wm_maximums lp_max_1_2, lp_max_5_6;
 	struct hsw_pipe_wm_parameters params[3];
 	struct hsw_wm_values results_1_2, results_5_6, *best_results;
-	uint32_t wm[5];
+	uint16_t wm[5];
 	enum hsw_data_buf_partitioning partitioning;
 
 	hsw_compute_wm_parameters(dev, params, wm, &lp_max_1_2, &lp_max_5_6);
-- 
1.8.1.5

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

[PATCH 08/17] drm/i915: Split out reading of HSW watermark latency values

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Move parsing of MCH_SSKPD to a separate function, we'll add other
platforms there later.

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 9417187..cfcc677 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2336,28 +2336,33 @@ hsw_compute_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc)
 	       PIPE_WM_LINETIME_TIME(linetime);
 }
 
+static void intel_read_wm_latency(struct drm_device *dev, uint16_t wm[5])
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	if (IS_HASWELL(dev)) {
+		uint64_t sskpd = I915_READ64(MCH_SSKPD);
+
+		wm[0] = (sskpd >> 56) & 0xFF;
+		if (wm[0] == 0)
+			wm[0] = sskpd & 0xF;
+		wm[1] = ((sskpd >> 4) & 0xFF) * 5;
+		wm[2] = ((sskpd >> 12) & 0xFF) * 5;
+		wm[3] = ((sskpd >> 20) & 0x1FF) * 5;
+		wm[4] = ((sskpd >> 32) & 0x1FF) * 5;
+	}
+}
+
 static void hsw_compute_wm_parameters(struct drm_device *dev,
 				      struct hsw_pipe_wm_parameters *params,
-				      uint16_t *wm,
 				      struct hsw_wm_maximums *lp_max_1_2,
 				      struct hsw_wm_maximums *lp_max_5_6)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_crtc *crtc;
 	struct drm_plane *plane;
-	uint64_t sskpd = I915_READ64(MCH_SSKPD);
 	enum pipe pipe;
 	int pipes_active = 0, sprites_enabled = 0;
 
-	if ((sskpd >> 56) & 0xFF)
-		wm[0] = (sskpd >> 56) & 0xFF;
-	else
-		wm[0] = sskpd & 0xF;
-	wm[1] = ((sskpd >> 4) & 0xFF) * 5;
-	wm[2] = ((sskpd >> 12) & 0xFF) * 5;
-	wm[3] = ((sskpd >> 20) & 0x1FF) * 5;
-	wm[4] = ((sskpd >> 32) & 0x1FF) * 5;
-
 	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
 		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 		struct hsw_pipe_wm_parameters *p;
@@ -2593,10 +2598,11 @@ static void haswell_update_wm(struct drm_device *dev)
 	struct hsw_wm_maximums lp_max_1_2, lp_max_5_6;
 	struct hsw_pipe_wm_parameters params[3];
 	struct hsw_wm_values results_1_2, results_5_6, *best_results;
-	uint16_t wm[5];
+	uint16_t wm[5] = {};
 	enum hsw_data_buf_partitioning partitioning;
 
-	hsw_compute_wm_parameters(dev, params, wm, &lp_max_1_2, &lp_max_5_6);
+	intel_read_wm_latency(dev, wm);
+	hsw_compute_wm_parameters(dev, params, &lp_max_1_2, &lp_max_5_6);
 
 	hsw_compute_wm_results(dev, params, wm, &lp_max_1_2, &results_1_2);
 	if (lp_max_1_2.pri != lp_max_5_6.pri) {
-- 
1.8.1.5

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

[PATCH 09/17] drm/i915: Don't multiply the watermark latency values too early

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The LP1+ watermark latency values need to be multiplied by 5 to
make the suitable for watermark calculations. However on pre-HSW
platforms we're going to need the raw value later when we have to
write it to the WM_LPn registers' latency field. So delay the
multiplication until it's needed.

Reviewed-by: Paulo Zanoni <paulo.r.zanonI@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index cfcc677..64dbd38 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2346,10 +2346,10 @@ static void intel_read_wm_latency(struct drm_device *dev, uint16_t wm[5])
 		wm[0] = (sskpd >> 56) & 0xFF;
 		if (wm[0] == 0)
 			wm[0] = sskpd & 0xF;
-		wm[1] = ((sskpd >> 4) & 0xFF) * 5;
-		wm[2] = ((sskpd >> 12) & 0xFF) * 5;
-		wm[3] = ((sskpd >> 20) & 0x1FF) * 5;
-		wm[4] = ((sskpd >> 32) & 0x1FF) * 5;
+		wm[1] = (sskpd >> 4) & 0xFF;
+		wm[2] = (sskpd >> 12) & 0xFF;
+		wm[3] = (sskpd >> 20) & 0x1FF;
+		wm[4] = (sskpd >> 32) & 0x1FF;
 	}
 }
 
@@ -2427,7 +2427,7 @@ static void hsw_compute_wm_results(struct drm_device *dev,
 	int level, max_level, wm_lp;
 
 	for (level = 1; level <= 4; level++)
-		if (!hsw_compute_lp_wm(wm[level], lp_maximums, params,
+		if (!hsw_compute_lp_wm(wm[level] * 5, lp_maximums, params,
 				       &lp_results[level - 1]))
 			break;
 	max_level = level - 1;
-- 
1.8.1.5

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

[PATCH 10/17] drm/i915: Add SNB/IVB support to intel_read_wm_latency

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

SNB and IVB have slightly a different way to read out the
watermark latency values.

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 64dbd38..a399ee9 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2350,6 +2350,13 @@ static void intel_read_wm_latency(struct drm_device *dev, uint16_t wm[5])
 		wm[2] = (sskpd >> 12) & 0xFF;
 		wm[3] = (sskpd >> 20) & 0x1FF;
 		wm[4] = (sskpd >> 32) & 0x1FF;
+	} else if (INTEL_INFO(dev)->gen >= 6) {
+		uint32_t sskpd = I915_READ(MCH_SSKPD);
+
+		wm[0] = (sskpd >> SSKPD_WM0_SHIFT) & SSKPD_WM_MASK;
+		wm[1] = (sskpd >> SSKPD_WM1_SHIFT) & SSKPD_WM_MASK;
+		wm[2] = (sskpd >> SSKPD_WM2_SHIFT) & SSKPD_WM_MASK;
+		wm[3] = (sskpd >> SSKPD_WM3_SHIFT) & SSKPD_WM_MASK;
 	}
 }
 
-- 
1.8.1.5

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

[PATCH 11/17] drm/i915: Add ILK support to intel_read_wm_latency

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

ILK has a slightly different way to read out the watermark
latency values. On ILK the LP0 latenciy values are in fact
not stored in any register, and instead we must use fixed
values.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a399ee9..5948f4c 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2357,6 +2357,13 @@ static void intel_read_wm_latency(struct drm_device *dev, uint16_t wm[5])
 		wm[1] = (sskpd >> SSKPD_WM1_SHIFT) & SSKPD_WM_MASK;
 		wm[2] = (sskpd >> SSKPD_WM2_SHIFT) & SSKPD_WM_MASK;
 		wm[3] = (sskpd >> SSKPD_WM3_SHIFT) & SSKPD_WM_MASK;
+	} else if (INTEL_INFO(dev)->gen >= 5) {
+		uint32_t mltr = I915_READ(MLTR_ILK);
+
+		/* ILK primary LP0 latency is 700 ns */
+		wm[0] = 7;
+		wm[1] = (mltr >> MLTR_WM1_SHIFT) & ILK_SRLT_MASK;
+		wm[2] = (mltr >> MLTR_WM2_SHIFT) & ILK_SRLT_MASK;
 	}
 }
 
-- 
1.8.1.5

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

[PATCH v2 12/17] drm/i915: Store the watermark latency values in dev_priv

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Rather than having to read the latency values out every time, just
store them in dev_priv.

On ILK and IVB there is a difference between some of the latency
values for different planes, so store the latency values for each
plane type separately, and apply the necesary fixups during init.

v2: Fix some checkpatch complaints

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h | 14 ++++++++++
 drivers/gpu/drm/i915/intel_pm.c | 62 +++++++++++++++++++++++++++++++++++------
 2 files changed, 67 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index fe466bc..bfa5d26 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1214,6 +1214,20 @@ typedef struct drm_i915_private {
 
 	struct i915_suspend_saved_registers regfile;
 
+	struct {
+		/*
+		 * Raw watermark latency values:
+		 * in 0.1us units for WM0,
+		 * in 0.5us units for WM1+.
+		 */
+		/* primary */
+		uint16_t pri_latency[5];
+		/* sprite */
+		uint16_t spr_latency[5];
+		/* cursor */
+		uint16_t cur_latency[5];
+	} wm;
+
 	/* Old dri1 support infrastructure, beware the dragons ya fools entering
 	 * here! */
 	struct i915_dri1_state dri1;
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 5948f4c..afa7f25 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2367,6 +2367,39 @@ static void intel_read_wm_latency(struct drm_device *dev, uint16_t wm[5])
 	}
 }
 
+static void intel_fixup_spr_wm_latency(struct drm_device *dev, uint16_t wm[5])
+{
+	/* ILK sprite LP0 latency is 1300 ns */
+	if (INTEL_INFO(dev)->gen == 5)
+		wm[0] = 13;
+}
+
+static void intel_fixup_cur_wm_latency(struct drm_device *dev, uint16_t wm[5])
+{
+	/* ILK cursor LP0 latency is 1300 ns */
+	if (INTEL_INFO(dev)->gen == 5)
+		wm[0] = 13;
+
+	/* WaDoubleCursorLP3Latency:ivb */
+	if (IS_IVYBRIDGE(dev))
+		wm[3] *= 2;
+}
+
+static void intel_setup_wm_latency(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	intel_read_wm_latency(dev, dev_priv->wm.pri_latency);
+
+	memcpy(dev_priv->wm.spr_latency, dev_priv->wm.pri_latency,
+	       sizeof(dev_priv->wm.pri_latency));
+	memcpy(dev_priv->wm.cur_latency, dev_priv->wm.pri_latency,
+	       sizeof(dev_priv->wm.pri_latency));
+
+	intel_fixup_spr_wm_latency(dev, dev_priv->wm.spr_latency);
+	intel_fixup_cur_wm_latency(dev, dev_priv->wm.cur_latency);
+}
+
 static void hsw_compute_wm_parameters(struct drm_device *dev,
 				      struct hsw_pipe_wm_parameters *params,
 				      struct hsw_wm_maximums *lp_max_1_2,
@@ -2612,16 +2645,17 @@ static void haswell_update_wm(struct drm_device *dev)
 	struct hsw_wm_maximums lp_max_1_2, lp_max_5_6;
 	struct hsw_pipe_wm_parameters params[3];
 	struct hsw_wm_values results_1_2, results_5_6, *best_results;
-	uint16_t wm[5] = {};
 	enum hsw_data_buf_partitioning partitioning;
 
-	intel_read_wm_latency(dev, wm);
 	hsw_compute_wm_parameters(dev, params, &lp_max_1_2, &lp_max_5_6);
 
-	hsw_compute_wm_results(dev, params, wm, &lp_max_1_2, &results_1_2);
+	hsw_compute_wm_results(dev, params,
+			       dev_priv->wm.pri_latency,
+			       &lp_max_1_2, &results_1_2);
 	if (lp_max_1_2.pri != lp_max_5_6.pri) {
-		hsw_compute_wm_results(dev, params, wm, &lp_max_5_6,
-				       &results_5_6);
+		hsw_compute_wm_results(dev, params,
+				       dev_priv->wm.pri_latency,
+				       &lp_max_5_6, &results_5_6);
 		best_results = hsw_find_best_result(&results_1_2, &results_5_6);
 	} else {
 		best_results = &results_1_2;
@@ -5214,8 +5248,12 @@ void intel_init_pm(struct drm_device *dev)
 
 	/* For FIFO watermark updates */
 	if (HAS_PCH_SPLIT(dev)) {
+		intel_setup_wm_latency(dev);
+
 		if (IS_GEN5(dev)) {
-			if (I915_READ(MLTR_ILK) & ILK_SRLT_MASK)
+			if (dev_priv->wm.pri_latency[1] &&
+			    dev_priv->wm.spr_latency[1] &&
+			    dev_priv->wm.cur_latency[1])
 				dev_priv->display.update_wm = ironlake_update_wm;
 			else {
 				DRM_DEBUG_KMS("Failed to get proper latency. "
@@ -5224,7 +5262,9 @@ void intel_init_pm(struct drm_device *dev)
 			}
 			dev_priv->display.init_clock_gating = ironlake_init_clock_gating;
 		} else if (IS_GEN6(dev)) {
-			if (SNB_READ_WM0_LATENCY()) {
+			if (dev_priv->wm.pri_latency[0] &&
+			    dev_priv->wm.spr_latency[0] &&
+			    dev_priv->wm.cur_latency[0]) {
 				dev_priv->display.update_wm = sandybridge_update_wm;
 				dev_priv->display.update_sprite_wm = sandybridge_update_sprite_wm;
 			} else {
@@ -5234,7 +5274,9 @@ void intel_init_pm(struct drm_device *dev)
 			}
 			dev_priv->display.init_clock_gating = gen6_init_clock_gating;
 		} else if (IS_IVYBRIDGE(dev)) {
-			if (SNB_READ_WM0_LATENCY()) {
+			if (dev_priv->wm.pri_latency[0] &&
+			    dev_priv->wm.spr_latency[0] &&
+			    dev_priv->wm.cur_latency[0]) {
 				dev_priv->display.update_wm = ivybridge_update_wm;
 				dev_priv->display.update_sprite_wm = sandybridge_update_sprite_wm;
 			} else {
@@ -5244,7 +5286,9 @@ void intel_init_pm(struct drm_device *dev)
 			}
 			dev_priv->display.init_clock_gating = ivybridge_init_clock_gating;
 		} else if (IS_HASWELL(dev)) {
-			if (I915_READ64(MCH_SSKPD)) {
+			if (dev_priv->wm.pri_latency[0] &&
+			    dev_priv->wm.spr_latency[0] &&
+			    dev_priv->wm.cur_latency[0]) {
 				dev_priv->display.update_wm = haswell_update_wm;
 				dev_priv->display.update_sprite_wm =
 					haswell_update_sprite_wm;
-- 
1.8.1.5

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

[PATCH v2 13/17] drm/i915: Use the stored cursor and plane latencies properly

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Rather than pass around the plane latencies, just grab them from
dev_priv nearer to where they're needed. Do the same for cursor
latencies.

v2: Add some comments about latency units

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index afa7f25..f754ca2 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2255,7 +2255,8 @@ static uint32_t ilk_compute_fbc_wm(struct hsw_pipe_wm_parameters *params,
 			  params->pri_bytes_per_pixel);
 }
 
-static bool hsw_compute_lp_wm(uint32_t mem_value, struct hsw_wm_maximums *max,
+static bool hsw_compute_lp_wm(struct drm_i915_private *dev_priv,
+			      int level, struct hsw_wm_maximums *max,
 			      struct hsw_pipe_wm_parameters *params,
 			      struct hsw_lp_wm_result *result)
 {
@@ -2264,10 +2265,14 @@ static bool hsw_compute_lp_wm(uint32_t mem_value, struct hsw_wm_maximums *max,
 
 	for (pipe = PIPE_A; pipe <= PIPE_C; pipe++) {
 		struct hsw_pipe_wm_parameters *p = &params[pipe];
+		/* WM1+ latency values stored in 0.5us units */
+		uint16_t pri_latency = dev_priv->wm.pri_latency[level] * 5;
+		uint16_t spr_latency = dev_priv->wm.spr_latency[level] * 5;
+		uint16_t cur_latency = dev_priv->wm.cur_latency[level] * 5;
 
-		pri_val[pipe] = ilk_compute_pri_wm(p, mem_value, true);
-		spr_val[pipe] = ilk_compute_spr_wm(p, mem_value);
-		cur_val[pipe] = ilk_compute_cur_wm(p, mem_value);
+		pri_val[pipe] = ilk_compute_pri_wm(p, pri_latency, true);
+		spr_val[pipe] = ilk_compute_spr_wm(p, spr_latency);
+		cur_val[pipe] = ilk_compute_cur_wm(p, cur_latency);
 		fbc_val[pipe] = ilk_compute_fbc_wm(p, pri_val[pipe]);
 	}
 
@@ -2290,14 +2295,18 @@ static bool hsw_compute_lp_wm(uint32_t mem_value, struct hsw_wm_maximums *max,
 }
 
 static uint32_t hsw_compute_wm_pipe(struct drm_i915_private *dev_priv,
-				    uint32_t mem_value, enum pipe pipe,
+				    enum pipe pipe,
 				    struct hsw_pipe_wm_parameters *params)
 {
 	uint32_t pri_val, cur_val, spr_val;
+	/* WM0 latency values stored in 0.1us units */
+	uint16_t pri_latency = dev_priv->wm.pri_latency[0];
+	uint16_t spr_latency = dev_priv->wm.spr_latency[0];
+	uint16_t cur_latency = dev_priv->wm.cur_latency[0];
 
-	pri_val = ilk_compute_pri_wm(params, mem_value, false);
-	spr_val = ilk_compute_spr_wm(params, mem_value);
-	cur_val = ilk_compute_cur_wm(params, mem_value);
+	pri_val = ilk_compute_pri_wm(params, pri_latency, false);
+	spr_val = ilk_compute_spr_wm(params, spr_latency);
+	cur_val = ilk_compute_cur_wm(params, cur_latency);
 
 	WARN(pri_val > 127,
 	     "Primary WM error, mode not supported for pipe %c\n",
@@ -2463,7 +2472,6 @@ static void hsw_compute_wm_parameters(struct drm_device *dev,
 
 static void hsw_compute_wm_results(struct drm_device *dev,
 				   struct hsw_pipe_wm_parameters *params,
-				   uint16_t *wm,
 				   struct hsw_wm_maximums *lp_maximums,
 				   struct hsw_wm_values *results)
 {
@@ -2474,7 +2482,8 @@ static void hsw_compute_wm_results(struct drm_device *dev,
 	int level, max_level, wm_lp;
 
 	for (level = 1; level <= 4; level++)
-		if (!hsw_compute_lp_wm(wm[level] * 5, lp_maximums, params,
+		if (!hsw_compute_lp_wm(dev_priv, level,
+				       lp_maximums, params,
 				       &lp_results[level - 1]))
 			break;
 	max_level = level - 1;
@@ -2506,8 +2515,7 @@ static void hsw_compute_wm_results(struct drm_device *dev,
 	}
 
 	for_each_pipe(pipe)
-		results->wm_pipe[pipe] = hsw_compute_wm_pipe(dev_priv, wm[0],
-							     pipe,
+		results->wm_pipe[pipe] = hsw_compute_wm_pipe(dev_priv, pipe,
 							     &params[pipe]);
 
 	for_each_pipe(pipe) {
@@ -2650,11 +2658,9 @@ static void haswell_update_wm(struct drm_device *dev)
 	hsw_compute_wm_parameters(dev, params, &lp_max_1_2, &lp_max_5_6);
 
 	hsw_compute_wm_results(dev, params,
-			       dev_priv->wm.pri_latency,
 			       &lp_max_1_2, &results_1_2);
 	if (lp_max_1_2.pri != lp_max_5_6.pri) {
 		hsw_compute_wm_results(dev, params,
-				       dev_priv->wm.pri_latency,
 				       &lp_max_5_6, &results_5_6);
 		best_results = hsw_find_best_result(&results_1_2, &results_5_6);
 	} else {
-- 
1.8.1.5

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

[PATCH v2 14/17] drm/i915: Print the watermark latencies during init

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Seeing the watermark latency values in dmesg might help sometimes.

v2: Use DRM_ERROR() when expected latency values are missing

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index f754ca2..53967ef 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2394,6 +2394,39 @@ static void intel_fixup_cur_wm_latency(struct drm_device *dev, uint16_t wm[5])
 		wm[3] *= 2;
 }
 
+static void intel_print_wm_latency(struct drm_device *dev,
+				   const char *name,
+				   const uint16_t wm[5])
+{
+	int level, max_level;
+
+	/* how many WM levels are we expecting */
+	if (IS_HASWELL(dev))
+		max_level = 4;
+	else if (INTEL_INFO(dev)->gen >= 6)
+		max_level = 3;
+	else
+		max_level = 2;
+
+	for (level = 0; level <= max_level; level++) {
+		unsigned int latency = wm[level];
+
+		if (latency == 0) {
+			DRM_ERROR("%s WM%d latency not provided\n",
+				  name, level);
+			continue;
+		}
+
+		/* WM1+ latency values in 0.5us units */
+		if (level > 0)
+			latency *= 5;
+
+		DRM_DEBUG_KMS("%s WM%d latency %u (%u.%u usec)\n",
+			      name, level, wm[level],
+			      latency / 10, latency % 10);
+	}
+}
+
 static void intel_setup_wm_latency(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -2407,6 +2440,10 @@ static void intel_setup_wm_latency(struct drm_device *dev)
 
 	intel_fixup_spr_wm_latency(dev, dev_priv->wm.spr_latency);
 	intel_fixup_cur_wm_latency(dev, dev_priv->wm.cur_latency);
+
+	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 hsw_compute_wm_parameters(struct drm_device *dev,
-- 
1.8.1.5

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

[PATCH v2 15/17] drm/i915: Disable specific watermark levels when latency is zero

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Return UINT_MAX for the calculated WM level if the latency is zero.
This will lead to marking the WM level as disabled.

I'm not sure if latency==0 should mean that we want to disable the
level. But that's the implication I got from the fact that we don't
even enable the watermark code of the SSKDP register is 0.

v2: Use WARN() to scare people

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 53967ef..149eb0a 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2116,6 +2116,9 @@ static uint32_t ilk_wm_method1(uint32_t pixel_rate, uint8_t bytes_per_pixel,
 {
 	uint64_t ret;
 
+	if (WARN(latency == 0, "Latency value missing\n"))
+		return UINT_MAX;
+
 	ret = (uint64_t) pixel_rate * bytes_per_pixel * latency;
 	ret = DIV_ROUND_UP_ULL(ret, 64 * 10000) + 2;
 
@@ -2128,6 +2131,9 @@ static uint32_t ilk_wm_method2(uint32_t pixel_rate, uint32_t pipe_htotal,
 {
 	uint32_t ret;
 
+	if (WARN(latency == 0, "Latency value missing\n"))
+		return UINT_MAX;
+
 	ret = (latency * pixel_rate) / (pipe_htotal * 10000);
 	ret = (ret + 1) * horiz_pixels * bytes_per_pixel;
 	ret = DIV_ROUND_UP(ret, 64) + 2;
-- 
1.8.1.5

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

[PATCH 16/17] drm/i915: Use the watermark latency values from dev_priv for ILK/SNB/IVB too

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Adjust the current ILK/SNB/IVB watermark codepaths to use the
pre-populated latency values from dev_priv instead of reading
them out from the registers every time.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h |  9 -------
 drivers/gpu/drm/i915/intel_pm.c | 57 +++++++++++++++++++----------------------
 2 files changed, 27 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 19941c6..17f6252 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3202,9 +3202,6 @@
 #define  MLTR_WM2_SHIFT		8
 /* the unit of memory self-refresh latency time is 0.5us */
 #define  ILK_SRLT_MASK		0x3f
-#define ILK_LATENCY(shift)	(I915_READ(MLTR_ILK) >> (shift) & ILK_SRLT_MASK)
-#define ILK_READ_WM1_LATENCY()	ILK_LATENCY(MLTR_WM1_SHIFT)
-#define ILK_READ_WM2_LATENCY()	ILK_LATENCY(MLTR_WM2_SHIFT)
 
 /* define the fifo size on Ironlake */
 #define ILK_DISPLAY_FIFO	128
@@ -3251,12 +3248,6 @@
 #define SSKPD_WM2_SHIFT		16
 #define SSKPD_WM3_SHIFT		24
 
-#define SNB_LATENCY(shift)	(I915_READ(MCHBAR_MIRROR_BASE_SNB + SSKPD) >> (shift) & SSKPD_WM_MASK)
-#define SNB_READ_WM0_LATENCY()		SNB_LATENCY(SSKPD_WM0_SHIFT)
-#define SNB_READ_WM1_LATENCY()		SNB_LATENCY(SSKPD_WM1_SHIFT)
-#define SNB_READ_WM2_LATENCY()		SNB_LATENCY(SSKPD_WM2_SHIFT)
-#define SNB_READ_WM3_LATENCY()		SNB_LATENCY(SSKPD_WM3_SHIFT)
-
 /*
  * The two pipe frame counter registers are not synchronized, so
  * reading a stable value is somewhat tricky. The following code
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 149eb0a..5fe8c4e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1665,9 +1665,6 @@ static void i830_update_wm(struct drm_device *dev)
 	I915_WRITE(FW_BLC, fwater_lo);
 }
 
-#define ILK_LP0_PLANE_LATENCY		700
-#define ILK_LP0_CURSOR_LATENCY		1300
-
 /*
  * Check the wm result.
  *
@@ -1782,9 +1779,9 @@ static void ironlake_update_wm(struct drm_device *dev)
 	enabled = 0;
 	if (g4x_compute_wm0(dev, PIPE_A,
 			    &ironlake_display_wm_info,
-			    ILK_LP0_PLANE_LATENCY,
+			    dev_priv->wm.pri_latency[0] * 100,
 			    &ironlake_cursor_wm_info,
-			    ILK_LP0_CURSOR_LATENCY,
+			    dev_priv->wm.cur_latency[0] * 100,
 			    &plane_wm, &cursor_wm)) {
 		I915_WRITE(WM0_PIPEA_ILK,
 			   (plane_wm << WM0_PIPE_PLANE_SHIFT) | cursor_wm);
@@ -1796,9 +1793,9 @@ static void ironlake_update_wm(struct drm_device *dev)
 
 	if (g4x_compute_wm0(dev, PIPE_B,
 			    &ironlake_display_wm_info,
-			    ILK_LP0_PLANE_LATENCY,
+			    dev_priv->wm.pri_latency[0] * 100,
 			    &ironlake_cursor_wm_info,
-			    ILK_LP0_CURSOR_LATENCY,
+			    dev_priv->wm.cur_latency[0] * 100,
 			    &plane_wm, &cursor_wm)) {
 		I915_WRITE(WM0_PIPEB_ILK,
 			   (plane_wm << WM0_PIPE_PLANE_SHIFT) | cursor_wm);
@@ -1822,7 +1819,7 @@ static void ironlake_update_wm(struct drm_device *dev)
 
 	/* WM1 */
 	if (!ironlake_compute_srwm(dev, 1, enabled,
-				   ILK_READ_WM1_LATENCY() * 500,
+				   dev_priv->wm.pri_latency[1] * 500,
 				   &ironlake_display_srwm_info,
 				   &ironlake_cursor_srwm_info,
 				   &fbc_wm, &plane_wm, &cursor_wm))
@@ -1830,14 +1827,14 @@ static void ironlake_update_wm(struct drm_device *dev)
 
 	I915_WRITE(WM1_LP_ILK,
 		   WM1_LP_SR_EN |
-		   (ILK_READ_WM1_LATENCY() << WM1_LP_LATENCY_SHIFT) |
+		   (dev_priv->wm.pri_latency[1] << WM1_LP_LATENCY_SHIFT) |
 		   (fbc_wm << WM1_LP_FBC_SHIFT) |
 		   (plane_wm << WM1_LP_SR_SHIFT) |
 		   cursor_wm);
 
 	/* WM2 */
 	if (!ironlake_compute_srwm(dev, 2, enabled,
-				   ILK_READ_WM2_LATENCY() * 500,
+				   dev_priv->wm.pri_latency[2] * 500,
 				   &ironlake_display_srwm_info,
 				   &ironlake_cursor_srwm_info,
 				   &fbc_wm, &plane_wm, &cursor_wm))
@@ -1845,7 +1842,7 @@ static void ironlake_update_wm(struct drm_device *dev)
 
 	I915_WRITE(WM2_LP_ILK,
 		   WM2_LP_EN |
-		   (ILK_READ_WM2_LATENCY() << WM1_LP_LATENCY_SHIFT) |
+		   (dev_priv->wm.pri_latency[2] << WM1_LP_LATENCY_SHIFT) |
 		   (fbc_wm << WM1_LP_FBC_SHIFT) |
 		   (plane_wm << WM1_LP_SR_SHIFT) |
 		   cursor_wm);
@@ -1859,7 +1856,7 @@ static void ironlake_update_wm(struct drm_device *dev)
 static void sandybridge_update_wm(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	int latency = SNB_READ_WM0_LATENCY() * 100;	/* In unit 0.1us */
+	int latency = dev_priv->wm.pri_latency[0] * 100;	/* In unit 0.1us */
 	u32 val;
 	int fbc_wm, plane_wm, cursor_wm;
 	unsigned int enabled;
@@ -1914,7 +1911,7 @@ static void sandybridge_update_wm(struct drm_device *dev)
 
 	/* WM1 */
 	if (!ironlake_compute_srwm(dev, 1, enabled,
-				   SNB_READ_WM1_LATENCY() * 500,
+				   dev_priv->wm.pri_latency[1] * 500,
 				   &sandybridge_display_srwm_info,
 				   &sandybridge_cursor_srwm_info,
 				   &fbc_wm, &plane_wm, &cursor_wm))
@@ -1922,14 +1919,14 @@ static void sandybridge_update_wm(struct drm_device *dev)
 
 	I915_WRITE(WM1_LP_ILK,
 		   WM1_LP_SR_EN |
-		   (SNB_READ_WM1_LATENCY() << WM1_LP_LATENCY_SHIFT) |
+		   (dev_priv->wm.pri_latency[1] << WM1_LP_LATENCY_SHIFT) |
 		   (fbc_wm << WM1_LP_FBC_SHIFT) |
 		   (plane_wm << WM1_LP_SR_SHIFT) |
 		   cursor_wm);
 
 	/* WM2 */
 	if (!ironlake_compute_srwm(dev, 2, enabled,
-				   SNB_READ_WM2_LATENCY() * 500,
+				   dev_priv->wm.pri_latency[2] * 500,
 				   &sandybridge_display_srwm_info,
 				   &sandybridge_cursor_srwm_info,
 				   &fbc_wm, &plane_wm, &cursor_wm))
@@ -1937,14 +1934,14 @@ static void sandybridge_update_wm(struct drm_device *dev)
 
 	I915_WRITE(WM2_LP_ILK,
 		   WM2_LP_EN |
-		   (SNB_READ_WM2_LATENCY() << WM1_LP_LATENCY_SHIFT) |
+		   (dev_priv->wm.pri_latency[2] << WM1_LP_LATENCY_SHIFT) |
 		   (fbc_wm << WM1_LP_FBC_SHIFT) |
 		   (plane_wm << WM1_LP_SR_SHIFT) |
 		   cursor_wm);
 
 	/* WM3 */
 	if (!ironlake_compute_srwm(dev, 3, enabled,
-				   SNB_READ_WM3_LATENCY() * 500,
+				   dev_priv->wm.pri_latency[3] * 500,
 				   &sandybridge_display_srwm_info,
 				   &sandybridge_cursor_srwm_info,
 				   &fbc_wm, &plane_wm, &cursor_wm))
@@ -1952,7 +1949,7 @@ static void sandybridge_update_wm(struct drm_device *dev)
 
 	I915_WRITE(WM3_LP_ILK,
 		   WM3_LP_EN |
-		   (SNB_READ_WM3_LATENCY() << WM1_LP_LATENCY_SHIFT) |
+		   (dev_priv->wm.pri_latency[3] << WM1_LP_LATENCY_SHIFT) |
 		   (fbc_wm << WM1_LP_FBC_SHIFT) |
 		   (plane_wm << WM1_LP_SR_SHIFT) |
 		   cursor_wm);
@@ -1961,7 +1958,7 @@ static void sandybridge_update_wm(struct drm_device *dev)
 static void ivybridge_update_wm(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	int latency = SNB_READ_WM0_LATENCY() * 100;	/* In unit 0.1us */
+	int latency = dev_priv->wm.pri_latency[0] * 100;	/* In unit 0.1us */
 	u32 val;
 	int fbc_wm, plane_wm, cursor_wm;
 	int ignore_fbc_wm, ignore_plane_wm, ignore_cursor_wm;
@@ -2031,7 +2028,7 @@ static void ivybridge_update_wm(struct drm_device *dev)
 
 	/* WM1 */
 	if (!ironlake_compute_srwm(dev, 1, enabled,
-				   SNB_READ_WM1_LATENCY() * 500,
+				   dev_priv->wm.pri_latency[1] * 500,
 				   &sandybridge_display_srwm_info,
 				   &sandybridge_cursor_srwm_info,
 				   &fbc_wm, &plane_wm, &cursor_wm))
@@ -2039,14 +2036,14 @@ static void ivybridge_update_wm(struct drm_device *dev)
 
 	I915_WRITE(WM1_LP_ILK,
 		   WM1_LP_SR_EN |
-		   (SNB_READ_WM1_LATENCY() << WM1_LP_LATENCY_SHIFT) |
+		   (dev_priv->wm.pri_latency[1] << WM1_LP_LATENCY_SHIFT) |
 		   (fbc_wm << WM1_LP_FBC_SHIFT) |
 		   (plane_wm << WM1_LP_SR_SHIFT) |
 		   cursor_wm);
 
 	/* WM2 */
 	if (!ironlake_compute_srwm(dev, 2, enabled,
-				   SNB_READ_WM2_LATENCY() * 500,
+				   dev_priv->wm.pri_latency[2] * 500,
 				   &sandybridge_display_srwm_info,
 				   &sandybridge_cursor_srwm_info,
 				   &fbc_wm, &plane_wm, &cursor_wm))
@@ -2054,19 +2051,19 @@ static void ivybridge_update_wm(struct drm_device *dev)
 
 	I915_WRITE(WM2_LP_ILK,
 		   WM2_LP_EN |
-		   (SNB_READ_WM2_LATENCY() << WM1_LP_LATENCY_SHIFT) |
+		   (dev_priv->wm.pri_latency[2] << WM1_LP_LATENCY_SHIFT) |
 		   (fbc_wm << WM1_LP_FBC_SHIFT) |
 		   (plane_wm << WM1_LP_SR_SHIFT) |
 		   cursor_wm);
 
 	/* WM3, note we have to correct the cursor latency */
 	if (!ironlake_compute_srwm(dev, 3, enabled,
-				   SNB_READ_WM3_LATENCY() * 500,
+				   dev_priv->wm.pri_latency[3] * 500,
 				   &sandybridge_display_srwm_info,
 				   &sandybridge_cursor_srwm_info,
 				   &fbc_wm, &plane_wm, &ignore_cursor_wm) ||
 	    !ironlake_compute_srwm(dev, 3, enabled,
-				   2 * SNB_READ_WM3_LATENCY() * 500,
+				   dev_priv->wm.cur_latency[3] * 500,
 				   &sandybridge_display_srwm_info,
 				   &sandybridge_cursor_srwm_info,
 				   &ignore_fbc_wm, &ignore_plane_wm, &cursor_wm))
@@ -2074,7 +2071,7 @@ static void ivybridge_update_wm(struct drm_device *dev)
 
 	I915_WRITE(WM3_LP_ILK,
 		   WM3_LP_EN |
-		   (SNB_READ_WM3_LATENCY() << WM1_LP_LATENCY_SHIFT) |
+		   (dev_priv->wm.pri_latency[3] << WM1_LP_LATENCY_SHIFT) |
 		   (fbc_wm << WM1_LP_FBC_SHIFT) |
 		   (plane_wm << WM1_LP_SR_SHIFT) |
 		   cursor_wm);
@@ -2818,7 +2815,7 @@ static void sandybridge_update_sprite_wm(struct drm_device *dev, int pipe,
 					 bool enabled, bool scaled)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	int latency = SNB_READ_WM0_LATENCY() * 100;	/* In unit 0.1us */
+	int latency = dev_priv->wm.spr_latency[0] * 100;	/* In unit 0.1us */
 	u32 val;
 	int sprite_wm, reg;
 	int ret;
@@ -2858,7 +2855,7 @@ static void sandybridge_update_sprite_wm(struct drm_device *dev, int pipe,
 	ret = sandybridge_compute_sprite_srwm(dev, pipe, sprite_width,
 					      pixel_size,
 					      &sandybridge_display_srwm_info,
-					      SNB_READ_WM1_LATENCY() * 500,
+					      dev_priv->wm.spr_latency[1] * 500,
 					      &sprite_wm);
 	if (!ret) {
 		DRM_DEBUG_KMS("failed to compute sprite lp1 wm on pipe %c\n",
@@ -2874,7 +2871,7 @@ static void sandybridge_update_sprite_wm(struct drm_device *dev, int pipe,
 	ret = sandybridge_compute_sprite_srwm(dev, pipe, sprite_width,
 					      pixel_size,
 					      &sandybridge_display_srwm_info,
-					      SNB_READ_WM2_LATENCY() * 500,
+					      dev_priv->wm.spr_latency[2] * 500,
 					      &sprite_wm);
 	if (!ret) {
 		DRM_DEBUG_KMS("failed to compute sprite lp2 wm on pipe %c\n",
@@ -2886,7 +2883,7 @@ static void sandybridge_update_sprite_wm(struct drm_device *dev, int pipe,
 	ret = sandybridge_compute_sprite_srwm(dev, pipe, sprite_width,
 					      pixel_size,
 					      &sandybridge_display_srwm_info,
-					      SNB_READ_WM3_LATENCY() * 500,
+					      dev_priv->wm.spr_latency[3] * 500,
 					      &sprite_wm);
 	if (!ret) {
 		DRM_DEBUG_KMS("failed to compute sprite lp3 wm on pipe %c\n",
-- 
1.8.1.5

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

[PATCH 17/17] drm/i915: Add comments about units of latency values

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

All the ILK+ WM compute functions take the latency values in 0.1us
units. Add a few comments to remind people about that.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 5fe8c4e..51f445f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2108,6 +2108,7 @@ static uint32_t ilk_pipe_pixel_rate(struct drm_device *dev,
 	return pixel_rate;
 }
 
+/* latency must be in 0.1us units. */
 static uint32_t ilk_wm_method1(uint32_t pixel_rate, uint8_t bytes_per_pixel,
 			       uint32_t latency)
 {
@@ -2122,6 +2123,7 @@ static uint32_t ilk_wm_method1(uint32_t pixel_rate, uint8_t bytes_per_pixel,
 	return ret;
 }
 
+/* latency must be in 0.1us units. */
 static uint32_t ilk_wm_method2(uint32_t pixel_rate, uint32_t pipe_htotal,
 			       uint32_t horiz_pixels, uint8_t bytes_per_pixel,
 			       uint32_t latency)
@@ -2185,7 +2187,10 @@ enum hsw_data_buf_partitioning {
 	HSW_DATA_BUF_PART_5_6,
 };
 
-/* For both WM_PIPE and WM_LP. */
+/*
+ * For both WM_PIPE and WM_LP.
+ * mem_value must be in 0.1us units.
+ */
 static uint32_t ilk_compute_pri_wm(struct hsw_pipe_wm_parameters *params,
 				   uint32_t mem_value,
 				   bool is_lp)
@@ -2212,7 +2217,10 @@ static uint32_t ilk_compute_pri_wm(struct hsw_pipe_wm_parameters *params,
 	return min(method1, method2);
 }
 
-/* For both WM_PIPE and WM_LP. */
+/*
+ * For both WM_PIPE and WM_LP.
+ * mem_value must be in 0.1us units.
+ */
 static uint32_t ilk_compute_spr_wm(struct hsw_pipe_wm_parameters *params,
 				   uint32_t mem_value)
 {
@@ -2232,7 +2240,10 @@ static uint32_t ilk_compute_spr_wm(struct hsw_pipe_wm_parameters *params,
 	return min(method1, method2);
 }
 
-/* For both WM_PIPE and WM_LP. */
+/*
+ * For both WM_PIPE and WM_LP.
+ * mem_value must be in 0.1us units.
+ */
 static uint32_t ilk_compute_cur_wm(struct hsw_pipe_wm_parameters *params,
 				   uint32_t mem_value)
 {
-- 
1.8.1.5

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

Re: [PATCH v2 12/17] drm/i915: Store the watermark latency values in dev_priv

[Chris Wilson]

On Thu, Aug 01, 2013 at 04:18:50PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Rather than having to read the latency values out every time, just
> store them in dev_priv.
> 
> On ILK and IVB there is a difference between some of the latency
> values for different planes, so store the latency values for each
> plane type separately, and apply the necesary fixups during init.
> 
> v2: Fix some checkpatch complaints

As you are now storing these, it would also be useful to add a debugfs
file to dump them out for sanity checking. I leave the request to adjust
them via debugfs to somebody else ;-)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 11/17] drm/i915: Add ILK support to intel_read_wm_latency

[Paulo Zanoni]

2013/8/1  <ville.syrjala@linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> ILK has a slightly different way to read out the watermark
> latency values. On ILK the LP0 latenciy values are in fact
> not stored in any register, and instead we must use fixed
> values.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

No docs, but seems consistent with the current code.

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

In a follow-up patch you could add an "} else {" statement that prints
a big error message, or even BUG().

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index a399ee9..5948f4c 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2357,6 +2357,13 @@ static void intel_read_wm_latency(struct drm_device *dev, uint16_t wm[5])
>                 wm[1] = (sskpd >> SSKPD_WM1_SHIFT) & SSKPD_WM_MASK;
>                 wm[2] = (sskpd >> SSKPD_WM2_SHIFT) & SSKPD_WM_MASK;
>                 wm[3] = (sskpd >> SSKPD_WM3_SHIFT) & SSKPD_WM_MASK;
> +       } else if (INTEL_INFO(dev)->gen >= 5) {
> +               uint32_t mltr = I915_READ(MLTR_ILK);
> +
> +               /* ILK primary LP0 latency is 700 ns */
> +               wm[0] = 7;
> +               wm[1] = (mltr >> MLTR_WM1_SHIFT) & ILK_SRLT_MASK;
> +               wm[2] = (mltr >> MLTR_WM2_SHIFT) & ILK_SRLT_MASK;
>         }
>  }
>
> --
> 1.8.1.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

Re: [PATCH v2 12/17] drm/i915: Store the watermark latency values in dev_priv

[Paulo Zanoni]

2013/8/1  <ville.syrjala@linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Rather than having to read the latency values out every time, just
> store them in dev_priv.
>
> On ILK and IVB there is a difference between some of the latency
> values for different planes, so store the latency values for each
> plane type separately, and apply the necesary fixups during init.
>
> v2: Fix some checkpatch complaints
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h | 14 ++++++++++
>  drivers/gpu/drm/i915/intel_pm.c | 62 +++++++++++++++++++++++++++++++++++------
>  2 files changed, 67 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index fe466bc..bfa5d26 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1214,6 +1214,20 @@ typedef struct drm_i915_private {
>
>         struct i915_suspend_saved_registers regfile;
>
> +       struct {
> +               /*
> +                * Raw watermark latency values:
> +                * in 0.1us units for WM0,
> +                * in 0.5us units for WM1+.
> +                */
> +               /* primary */
> +               uint16_t pri_latency[5];
> +               /* sprite */
> +               uint16_t spr_latency[5];
> +               /* cursor */
> +               uint16_t cur_latency[5];
> +       } wm;
> +
>         /* Old dri1 support infrastructure, beware the dragons ya fools entering
>          * here! */
>         struct i915_dri1_state dri1;
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 5948f4c..afa7f25 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2367,6 +2367,39 @@ static void intel_read_wm_latency(struct drm_device *dev, uint16_t wm[5])
>         }
>  }
>
> +static void intel_fixup_spr_wm_latency(struct drm_device *dev, uint16_t wm[5])
> +{
> +       /* ILK sprite LP0 latency is 1300 ns */
> +       if (INTEL_INFO(dev)->gen == 5)
> +               wm[0] = 13;
> +}
> +
> +static void intel_fixup_cur_wm_latency(struct drm_device *dev, uint16_t wm[5])
> +{
> +       /* ILK cursor LP0 latency is 1300 ns */
> +       if (INTEL_INFO(dev)->gen == 5)
> +               wm[0] = 13;
> +
> +       /* WaDoubleCursorLP3Latency:ivb */
> +       if (IS_IVYBRIDGE(dev))
> +               wm[3] *= 2;

I'm looking forward to the future separate patch that will remove this
WA, but I guess it would be better to first finish enabling this code
path on IVB first and only then remove it, so we could easily revert
in case it hurts.

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>


> +}
> +
> +static void intel_setup_wm_latency(struct drm_device *dev)
> +{
> +       struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +       intel_read_wm_latency(dev, dev_priv->wm.pri_latency);
> +
> +       memcpy(dev_priv->wm.spr_latency, dev_priv->wm.pri_latency,
> +              sizeof(dev_priv->wm.pri_latency));
> +       memcpy(dev_priv->wm.cur_latency, dev_priv->wm.pri_latency,
> +              sizeof(dev_priv->wm.pri_latency));
> +
> +       intel_fixup_spr_wm_latency(dev, dev_priv->wm.spr_latency);
> +       intel_fixup_cur_wm_latency(dev, dev_priv->wm.cur_latency);
> +}
> +
>  static void hsw_compute_wm_parameters(struct drm_device *dev,
>                                       struct hsw_pipe_wm_parameters *params,
>                                       struct hsw_wm_maximums *lp_max_1_2,
> @@ -2612,16 +2645,17 @@ static void haswell_update_wm(struct drm_device *dev)
>         struct hsw_wm_maximums lp_max_1_2, lp_max_5_6;
>         struct hsw_pipe_wm_parameters params[3];
>         struct hsw_wm_values results_1_2, results_5_6, *best_results;
> -       uint16_t wm[5] = {};
>         enum hsw_data_buf_partitioning partitioning;
>
> -       intel_read_wm_latency(dev, wm);
>         hsw_compute_wm_parameters(dev, params, &lp_max_1_2, &lp_max_5_6);
>
> -       hsw_compute_wm_results(dev, params, wm, &lp_max_1_2, &results_1_2);
> +       hsw_compute_wm_results(dev, params,
> +                              dev_priv->wm.pri_latency,
> +                              &lp_max_1_2, &results_1_2);
>         if (lp_max_1_2.pri != lp_max_5_6.pri) {
> -               hsw_compute_wm_results(dev, params, wm, &lp_max_5_6,
> -                                      &results_5_6);
> +               hsw_compute_wm_results(dev, params,
> +                                      dev_priv->wm.pri_latency,
> +                                      &lp_max_5_6, &results_5_6);
>                 best_results = hsw_find_best_result(&results_1_2, &results_5_6);
>         } else {
>                 best_results = &results_1_2;
> @@ -5214,8 +5248,12 @@ void intel_init_pm(struct drm_device *dev)
>
>         /* For FIFO watermark updates */
>         if (HAS_PCH_SPLIT(dev)) {
> +               intel_setup_wm_latency(dev);
> +
>                 if (IS_GEN5(dev)) {
> -                       if (I915_READ(MLTR_ILK) & ILK_SRLT_MASK)
> +                       if (dev_priv->wm.pri_latency[1] &&
> +                           dev_priv->wm.spr_latency[1] &&
> +                           dev_priv->wm.cur_latency[1])
>                                 dev_priv->display.update_wm = ironlake_update_wm;
>                         else {
>                                 DRM_DEBUG_KMS("Failed to get proper latency. "
> @@ -5224,7 +5262,9 @@ void intel_init_pm(struct drm_device *dev)
>                         }
>                         dev_priv->display.init_clock_gating = ironlake_init_clock_gating;
>                 } else if (IS_GEN6(dev)) {
> -                       if (SNB_READ_WM0_LATENCY()) {
> +                       if (dev_priv->wm.pri_latency[0] &&
> +                           dev_priv->wm.spr_latency[0] &&
> +                           dev_priv->wm.cur_latency[0]) {
>                                 dev_priv->display.update_wm = sandybridge_update_wm;
>                                 dev_priv->display.update_sprite_wm = sandybridge_update_sprite_wm;
>                         } else {
> @@ -5234,7 +5274,9 @@ void intel_init_pm(struct drm_device *dev)
>                         }
>                         dev_priv->display.init_clock_gating = gen6_init_clock_gating;
>                 } else if (IS_IVYBRIDGE(dev)) {
> -                       if (SNB_READ_WM0_LATENCY()) {
> +                       if (dev_priv->wm.pri_latency[0] &&
> +                           dev_priv->wm.spr_latency[0] &&
> +                           dev_priv->wm.cur_latency[0]) {
>                                 dev_priv->display.update_wm = ivybridge_update_wm;
>                                 dev_priv->display.update_sprite_wm = sandybridge_update_sprite_wm;
>                         } else {
> @@ -5244,7 +5286,9 @@ void intel_init_pm(struct drm_device *dev)
>                         }
>                         dev_priv->display.init_clock_gating = ivybridge_init_clock_gating;
>                 } else if (IS_HASWELL(dev)) {
> -                       if (I915_READ64(MCH_SSKPD)) {
> +                       if (dev_priv->wm.pri_latency[0] &&
> +                           dev_priv->wm.spr_latency[0] &&
> +                           dev_priv->wm.cur_latency[0]) {
>                                 dev_priv->display.update_wm = haswell_update_wm;
>                                 dev_priv->display.update_sprite_wm =
>                                         haswell_update_sprite_wm;
> --
> 1.8.1.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

Re: [PATCH v2 13/17] drm/i915: Use the stored cursor and plane latencies properly

[Paulo Zanoni]

2013/8/1  <ville.syrjala@linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Rather than pass around the plane latencies, just grab them from
> dev_priv nearer to where they're needed. Do the same for cursor
> latencies.
>
> v2: Add some comments about latency units
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 34 ++++++++++++++++++++--------------
>  1 file changed, 20 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index afa7f25..f754ca2 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2255,7 +2255,8 @@ static uint32_t ilk_compute_fbc_wm(struct hsw_pipe_wm_parameters *params,
>                           params->pri_bytes_per_pixel);
>  }
>
> -static bool hsw_compute_lp_wm(uint32_t mem_value, struct hsw_wm_maximums *max,
> +static bool hsw_compute_lp_wm(struct drm_i915_private *dev_priv,
> +                             int level, struct hsw_wm_maximums *max,
>                               struct hsw_pipe_wm_parameters *params,
>                               struct hsw_lp_wm_result *result)
>  {
> @@ -2264,10 +2265,14 @@ static bool hsw_compute_lp_wm(uint32_t mem_value, struct hsw_wm_maximums *max,
>
>         for (pipe = PIPE_A; pipe <= PIPE_C; pipe++) {
>                 struct hsw_pipe_wm_parameters *p = &params[pipe];
> +               /* WM1+ latency values stored in 0.5us units */
> +               uint16_t pri_latency = dev_priv->wm.pri_latency[level] * 5;
> +               uint16_t spr_latency = dev_priv->wm.spr_latency[level] * 5;
> +               uint16_t cur_latency = dev_priv->wm.cur_latency[level] * 5;
>
> -               pri_val[pipe] = ilk_compute_pri_wm(p, mem_value, true);
> -               spr_val[pipe] = ilk_compute_spr_wm(p, mem_value);
> -               cur_val[pipe] = ilk_compute_cur_wm(p, mem_value);
> +               pri_val[pipe] = ilk_compute_pri_wm(p, pri_latency, true);
> +               spr_val[pipe] = ilk_compute_spr_wm(p, spr_latency);
> +               cur_val[pipe] = ilk_compute_cur_wm(p, cur_latency);
>                 fbc_val[pipe] = ilk_compute_fbc_wm(p, pri_val[pipe]);
>         }
>
> @@ -2290,14 +2295,18 @@ static bool hsw_compute_lp_wm(uint32_t mem_value, struct hsw_wm_maximums *max,
>  }
>
>  static uint32_t hsw_compute_wm_pipe(struct drm_i915_private *dev_priv,
> -                                   uint32_t mem_value, enum pipe pipe,
> +                                   enum pipe pipe,
>                                     struct hsw_pipe_wm_parameters *params)
>  {
>         uint32_t pri_val, cur_val, spr_val;
> +       /* WM0 latency values stored in 0.1us units */
> +       uint16_t pri_latency = dev_priv->wm.pri_latency[0];
> +       uint16_t spr_latency = dev_priv->wm.spr_latency[0];
> +       uint16_t cur_latency = dev_priv->wm.cur_latency[0];
>
> -       pri_val = ilk_compute_pri_wm(params, mem_value, false);
> -       spr_val = ilk_compute_spr_wm(params, mem_value);
> -       cur_val = ilk_compute_cur_wm(params, mem_value);
> +       pri_val = ilk_compute_pri_wm(params, pri_latency, false);
> +       spr_val = ilk_compute_spr_wm(params, spr_latency);
> +       cur_val = ilk_compute_cur_wm(params, cur_latency);
>
>         WARN(pri_val > 127,
>              "Primary WM error, mode not supported for pipe %c\n",
> @@ -2463,7 +2472,6 @@ static void hsw_compute_wm_parameters(struct drm_device *dev,
>
>  static void hsw_compute_wm_results(struct drm_device *dev,
>                                    struct hsw_pipe_wm_parameters *params,
> -                                  uint16_t *wm,
>                                    struct hsw_wm_maximums *lp_maximums,
>                                    struct hsw_wm_values *results)
>  {
> @@ -2474,7 +2482,8 @@ static void hsw_compute_wm_results(struct drm_device *dev,
>         int level, max_level, wm_lp;
>
>         for (level = 1; level <= 4; level++)
> -               if (!hsw_compute_lp_wm(wm[level] * 5, lp_maximums, params,
> +               if (!hsw_compute_lp_wm(dev_priv, level,
> +                                      lp_maximums, params,
>                                        &lp_results[level - 1]))
>                         break;
>         max_level = level - 1;
> @@ -2506,8 +2515,7 @@ static void hsw_compute_wm_results(struct drm_device *dev,
>         }
>
>         for_each_pipe(pipe)
> -               results->wm_pipe[pipe] = hsw_compute_wm_pipe(dev_priv, wm[0],
> -                                                            pipe,
> +               results->wm_pipe[pipe] = hsw_compute_wm_pipe(dev_priv, pipe,
>                                                              &params[pipe]);
>
>         for_each_pipe(pipe) {
> @@ -2650,11 +2658,9 @@ static void haswell_update_wm(struct drm_device *dev)
>         hsw_compute_wm_parameters(dev, params, &lp_max_1_2, &lp_max_5_6);
>
>         hsw_compute_wm_results(dev, params,
> -                              dev_priv->wm.pri_latency,
>                                &lp_max_1_2, &results_1_2);
>         if (lp_max_1_2.pri != lp_max_5_6.pri) {
>                 hsw_compute_wm_results(dev, params,
> -                                      dev_priv->wm.pri_latency,
>                                        &lp_max_5_6, &results_5_6);
>                 best_results = hsw_find_best_result(&results_1_2, &results_5_6);
>         } else {
> --
> 1.8.1.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

Re: [PATCH v2 14/17] drm/i915: Print the watermark latencies during init

[Paulo Zanoni]

2013/8/1  <ville.syrjala@linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Seeing the watermark latency values in dmesg might help sometimes.
>
> v2: Use DRM_ERROR() when expected latency values are missing
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 37 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index f754ca2..53967ef 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2394,6 +2394,39 @@ static void intel_fixup_cur_wm_latency(struct drm_device *dev, uint16_t wm[5])
>                 wm[3] *= 2;
>  }
>
> +static void intel_print_wm_latency(struct drm_device *dev,
> +                                  const char *name,
> +                                  const uint16_t wm[5])
> +{
> +       int level, max_level;
> +
> +       /* how many WM levels are we expecting */
> +       if (IS_HASWELL(dev))
> +               max_level = 4;
> +       else if (INTEL_INFO(dev)->gen >= 6)
> +               max_level = 3;
> +       else
> +               max_level = 2;
> +
> +       for (level = 0; level <= max_level; level++) {
> +               unsigned int latency = wm[level];
> +
> +               if (latency == 0) {
> +                       DRM_ERROR("%s WM%d latency not provided\n",
> +                                 name, level);

On your last email you mentioned that we may start getting bug reports
that we can't do anything about. You're right, I guess if we start
getting these reports we should probably tune the message to
DRM_DEBUG_KMS then.

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

> +                       continue;
> +               }
> +
> +               /* WM1+ latency values in 0.5us units */
> +               if (level > 0)
> +                       latency *= 5;
> +
> +               DRM_DEBUG_KMS("%s WM%d latency %u (%u.%u usec)\n",
> +                             name, level, wm[level],
> +                             latency / 10, latency % 10);
> +       }
> +}
> +
>  static void intel_setup_wm_latency(struct drm_device *dev)
>  {
>         struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -2407,6 +2440,10 @@ static void intel_setup_wm_latency(struct drm_device *dev)
>
>         intel_fixup_spr_wm_latency(dev, dev_priv->wm.spr_latency);
>         intel_fixup_cur_wm_latency(dev, dev_priv->wm.cur_latency);
> +
> +       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 hsw_compute_wm_parameters(struct drm_device *dev,
> --
> 1.8.1.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

Re: [PATCH v2 15/17] drm/i915: Disable specific watermark levels when latency is zero

[Paulo Zanoni]

2013/8/1  <ville.syrjala@linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Return UINT_MAX for the calculated WM level if the latency is zero.
> This will lead to marking the WM level as disabled.
>
> I'm not sure if latency==0 should mean that we want to disable the
> level. But that's the implication I got from the fact that we don't
> even enable the watermark code of the SSKDP register is 0.
>
> v2: Use WARN() to scare people
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 53967ef..149eb0a 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2116,6 +2116,9 @@ static uint32_t ilk_wm_method1(uint32_t pixel_rate, uint8_t bytes_per_pixel,
>  {
>         uint64_t ret;
>
> +       if (WARN(latency == 0, "Latency value missing\n"))

I know I'm the person who asked to scream loud in case the latency is
zero, but I guess that due to the previous patch we'll already
DRM_ERROR in case the latency is zero, so this WARN will be redundant.
So maybe your previous patch 16/35 would be more adequate.

So I'll give a "Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>"
to both this and the previous version (16/35), and let you (or Daniel)
decide which one to merge.


> +               return UINT_MAX;
> +
>         ret = (uint64_t) pixel_rate * bytes_per_pixel * latency;
>         ret = DIV_ROUND_UP_ULL(ret, 64 * 10000) + 2;
>
> @@ -2128,6 +2131,9 @@ static uint32_t ilk_wm_method2(uint32_t pixel_rate, uint32_t pipe_htotal,
>  {
>         uint32_t ret;
>
> +       if (WARN(latency == 0, "Latency value missing\n"))
> +               return UINT_MAX;
> +
>         ret = (latency * pixel_rate) / (pipe_htotal * 10000);
>         ret = (ret + 1) * horiz_pixels * bytes_per_pixel;
>         ret = DIV_ROUND_UP(ret, 64) + 2;
> --
> 1.8.1.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

Re: [PATCH v2 14/17] drm/i915: Print the watermark latencies during init

[Ville Syrjälä]

On Fri, Aug 02, 2013 at 11:41:31AM -0300, Paulo Zanoni wrote:
> 2013/8/1  <ville.syrjala@linux.intel.com>:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Seeing the watermark latency values in dmesg might help sometimes.
> >
> > v2: Use DRM_ERROR() when expected latency values are missing
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 37 +++++++++++++++++++++++++++++++++++++
> >  1 file changed, 37 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index f754ca2..53967ef 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -2394,6 +2394,39 @@ static void intel_fixup_cur_wm_latency(struct drm_device *dev, uint16_t wm[5])
> >                 wm[3] *= 2;
> >  }
> >
> > +static void intel_print_wm_latency(struct drm_device *dev,
> > +                                  const char *name,
> > +                                  const uint16_t wm[5])
> > +{
> > +       int level, max_level;
> > +
> > +       /* how many WM levels are we expecting */
> > +       if (IS_HASWELL(dev))
> > +               max_level = 4;
> > +       else if (INTEL_INFO(dev)->gen >= 6)
> > +               max_level = 3;
> > +       else
> > +               max_level = 2;
> > +
> > +       for (level = 0; level <= max_level; level++) {
> > +               unsigned int latency = wm[level];
> > +
> > +               if (latency == 0) {
> > +                       DRM_ERROR("%s WM%d latency not provided\n",
> > +                                 name, level);
> 
> On your last email you mentioned that we may start getting bug reports
> that we can't do anything about. You're right, I guess if we start
> getting these reports we should probably tune the message to
> DRM_DEBUG_KMS then.

Yeah. Another idea that I had is that we could store the max WM
level into dev_priv based on which levels have non-zero latencies.
That way we'd avoid ever hitting the case where we try to compute
a watermark w/ latency==0. But I didn't implement this yet.

> 
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> > +                       continue;
> > +               }
> > +
> > +               /* WM1+ latency values in 0.5us units */
> > +               if (level > 0)
> > +                       latency *= 5;
> > +
> > +               DRM_DEBUG_KMS("%s WM%d latency %u (%u.%u usec)\n",
> > +                             name, level, wm[level],
> > +                             latency / 10, latency % 10);
> > +       }
> > +}
> > +
> >  static void intel_setup_wm_latency(struct drm_device *dev)
> >  {
> >         struct drm_i915_private *dev_priv = dev->dev_private;
> > @@ -2407,6 +2440,10 @@ static void intel_setup_wm_latency(struct drm_device *dev)
> >
> >         intel_fixup_spr_wm_latency(dev, dev_priv->wm.spr_latency);
> >         intel_fixup_cur_wm_latency(dev, dev_priv->wm.cur_latency);
> > +
> > +       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 hsw_compute_wm_parameters(struct drm_device *dev,
> > --
> > 1.8.1.5
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Paulo Zanoni

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 16/17] drm/i915: Use the watermark latency values from dev_priv for ILK/SNB/IVB too

[Paulo Zanoni]

2013/8/1  <ville.syrjala@linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Adjust the current ILK/SNB/IVB watermark codepaths to use the
> pre-populated latency values from dev_priv instead of reading
> them out from the registers every time.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h |  9 -------
>  drivers/gpu/drm/i915/intel_pm.c | 57 +++++++++++++++++++----------------------
>  2 files changed, 27 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 19941c6..17f6252 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3202,9 +3202,6 @@
>  #define  MLTR_WM2_SHIFT                8
>  /* the unit of memory self-refresh latency time is 0.5us */
>  #define  ILK_SRLT_MASK         0x3f
> -#define ILK_LATENCY(shift)     (I915_READ(MLTR_ILK) >> (shift) & ILK_SRLT_MASK)
> -#define ILK_READ_WM1_LATENCY() ILK_LATENCY(MLTR_WM1_SHIFT)
> -#define ILK_READ_WM2_LATENCY() ILK_LATENCY(MLTR_WM2_SHIFT)
>
>  /* define the fifo size on Ironlake */
>  #define ILK_DISPLAY_FIFO       128
> @@ -3251,12 +3248,6 @@
>  #define SSKPD_WM2_SHIFT                16
>  #define SSKPD_WM3_SHIFT                24
>
> -#define SNB_LATENCY(shift)     (I915_READ(MCHBAR_MIRROR_BASE_SNB + SSKPD) >> (shift) & SSKPD_WM_MASK)
> -#define SNB_READ_WM0_LATENCY()         SNB_LATENCY(SSKPD_WM0_SHIFT)
> -#define SNB_READ_WM1_LATENCY()         SNB_LATENCY(SSKPD_WM1_SHIFT)
> -#define SNB_READ_WM2_LATENCY()         SNB_LATENCY(SSKPD_WM2_SHIFT)
> -#define SNB_READ_WM3_LATENCY()         SNB_LATENCY(SSKPD_WM3_SHIFT)
> -
>  /*
>   * The two pipe frame counter registers are not synchronized, so
>   * reading a stable value is somewhat tricky. The following code
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 149eb0a..5fe8c4e 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1665,9 +1665,6 @@ static void i830_update_wm(struct drm_device *dev)
>         I915_WRITE(FW_BLC, fwater_lo);
>  }
>
> -#define ILK_LP0_PLANE_LATENCY          700
> -#define ILK_LP0_CURSOR_LATENCY         1300
> -
>  /*
>   * Check the wm result.
>   *
> @@ -1782,9 +1779,9 @@ static void ironlake_update_wm(struct drm_device *dev)
>         enabled = 0;
>         if (g4x_compute_wm0(dev, PIPE_A,
>                             &ironlake_display_wm_info,
> -                           ILK_LP0_PLANE_LATENCY,
> +                           dev_priv->wm.pri_latency[0] * 100,
>                             &ironlake_cursor_wm_info,
> -                           ILK_LP0_CURSOR_LATENCY,
> +                           dev_priv->wm.cur_latency[0] * 100,
>                             &plane_wm, &cursor_wm)) {
>                 I915_WRITE(WM0_PIPEA_ILK,
>                            (plane_wm << WM0_PIPE_PLANE_SHIFT) | cursor_wm);
> @@ -1796,9 +1793,9 @@ static void ironlake_update_wm(struct drm_device *dev)
>
>         if (g4x_compute_wm0(dev, PIPE_B,
>                             &ironlake_display_wm_info,
> -                           ILK_LP0_PLANE_LATENCY,
> +                           dev_priv->wm.pri_latency[0] * 100,
>                             &ironlake_cursor_wm_info,
> -                           ILK_LP0_CURSOR_LATENCY,
> +                           dev_priv->wm.cur_latency[0] * 100,
>                             &plane_wm, &cursor_wm)) {
>                 I915_WRITE(WM0_PIPEB_ILK,
>                            (plane_wm << WM0_PIPE_PLANE_SHIFT) | cursor_wm);
> @@ -1822,7 +1819,7 @@ static void ironlake_update_wm(struct drm_device *dev)
>
>         /* WM1 */
>         if (!ironlake_compute_srwm(dev, 1, enabled,
> -                                  ILK_READ_WM1_LATENCY() * 500,
> +                                  dev_priv->wm.pri_latency[1] * 500,
>                                    &ironlake_display_srwm_info,
>                                    &ironlake_cursor_srwm_info,
>                                    &fbc_wm, &plane_wm, &cursor_wm))
> @@ -1830,14 +1827,14 @@ static void ironlake_update_wm(struct drm_device *dev)
>
>         I915_WRITE(WM1_LP_ILK,
>                    WM1_LP_SR_EN |
> -                  (ILK_READ_WM1_LATENCY() << WM1_LP_LATENCY_SHIFT) |
> +                  (dev_priv->wm.pri_latency[1] << WM1_LP_LATENCY_SHIFT) |
>                    (fbc_wm << WM1_LP_FBC_SHIFT) |
>                    (plane_wm << WM1_LP_SR_SHIFT) |
>                    cursor_wm);
>
>         /* WM2 */
>         if (!ironlake_compute_srwm(dev, 2, enabled,
> -                                  ILK_READ_WM2_LATENCY() * 500,
> +                                  dev_priv->wm.pri_latency[2] * 500,
>                                    &ironlake_display_srwm_info,
>                                    &ironlake_cursor_srwm_info,
>                                    &fbc_wm, &plane_wm, &cursor_wm))
> @@ -1845,7 +1842,7 @@ static void ironlake_update_wm(struct drm_device *dev)
>
>         I915_WRITE(WM2_LP_ILK,
>                    WM2_LP_EN |
> -                  (ILK_READ_WM2_LATENCY() << WM1_LP_LATENCY_SHIFT) |
> +                  (dev_priv->wm.pri_latency[2] << WM1_LP_LATENCY_SHIFT) |
>                    (fbc_wm << WM1_LP_FBC_SHIFT) |
>                    (plane_wm << WM1_LP_SR_SHIFT) |
>                    cursor_wm);
> @@ -1859,7 +1856,7 @@ static void ironlake_update_wm(struct drm_device *dev)
>  static void sandybridge_update_wm(struct drm_device *dev)
>  {
>         struct drm_i915_private *dev_priv = dev->dev_private;
> -       int latency = SNB_READ_WM0_LATENCY() * 100;     /* In unit 0.1us */
> +       int latency = dev_priv->wm.pri_latency[0] * 100;        /* In unit 0.1us */
>         u32 val;
>         int fbc_wm, plane_wm, cursor_wm;
>         unsigned int enabled;
> @@ -1914,7 +1911,7 @@ static void sandybridge_update_wm(struct drm_device *dev)
>
>         /* WM1 */
>         if (!ironlake_compute_srwm(dev, 1, enabled,
> -                                  SNB_READ_WM1_LATENCY() * 500,
> +                                  dev_priv->wm.pri_latency[1] * 500,
>                                    &sandybridge_display_srwm_info,
>                                    &sandybridge_cursor_srwm_info,
>                                    &fbc_wm, &plane_wm, &cursor_wm))
> @@ -1922,14 +1919,14 @@ static void sandybridge_update_wm(struct drm_device *dev)
>
>         I915_WRITE(WM1_LP_ILK,
>                    WM1_LP_SR_EN |
> -                  (SNB_READ_WM1_LATENCY() << WM1_LP_LATENCY_SHIFT) |
> +                  (dev_priv->wm.pri_latency[1] << WM1_LP_LATENCY_SHIFT) |
>                    (fbc_wm << WM1_LP_FBC_SHIFT) |
>                    (plane_wm << WM1_LP_SR_SHIFT) |
>                    cursor_wm);
>
>         /* WM2 */
>         if (!ironlake_compute_srwm(dev, 2, enabled,
> -                                  SNB_READ_WM2_LATENCY() * 500,
> +                                  dev_priv->wm.pri_latency[2] * 500,
>                                    &sandybridge_display_srwm_info,
>                                    &sandybridge_cursor_srwm_info,
>                                    &fbc_wm, &plane_wm, &cursor_wm))
> @@ -1937,14 +1934,14 @@ static void sandybridge_update_wm(struct drm_device *dev)
>
>         I915_WRITE(WM2_LP_ILK,
>                    WM2_LP_EN |
> -                  (SNB_READ_WM2_LATENCY() << WM1_LP_LATENCY_SHIFT) |
> +                  (dev_priv->wm.pri_latency[2] << WM1_LP_LATENCY_SHIFT) |
>                    (fbc_wm << WM1_LP_FBC_SHIFT) |
>                    (plane_wm << WM1_LP_SR_SHIFT) |
>                    cursor_wm);
>
>         /* WM3 */
>         if (!ironlake_compute_srwm(dev, 3, enabled,
> -                                  SNB_READ_WM3_LATENCY() * 500,
> +                                  dev_priv->wm.pri_latency[3] * 500,
>                                    &sandybridge_display_srwm_info,
>                                    &sandybridge_cursor_srwm_info,
>                                    &fbc_wm, &plane_wm, &cursor_wm))
> @@ -1952,7 +1949,7 @@ static void sandybridge_update_wm(struct drm_device *dev)
>
>         I915_WRITE(WM3_LP_ILK,
>                    WM3_LP_EN |
> -                  (SNB_READ_WM3_LATENCY() << WM1_LP_LATENCY_SHIFT) |
> +                  (dev_priv->wm.pri_latency[3] << WM1_LP_LATENCY_SHIFT) |
>                    (fbc_wm << WM1_LP_FBC_SHIFT) |
>                    (plane_wm << WM1_LP_SR_SHIFT) |
>                    cursor_wm);
> @@ -1961,7 +1958,7 @@ static void sandybridge_update_wm(struct drm_device *dev)
>  static void ivybridge_update_wm(struct drm_device *dev)
>  {
>         struct drm_i915_private *dev_priv = dev->dev_private;
> -       int latency = SNB_READ_WM0_LATENCY() * 100;     /* In unit 0.1us */
> +       int latency = dev_priv->wm.pri_latency[0] * 100;        /* In unit 0.1us */
>         u32 val;
>         int fbc_wm, plane_wm, cursor_wm;
>         int ignore_fbc_wm, ignore_plane_wm, ignore_cursor_wm;
> @@ -2031,7 +2028,7 @@ static void ivybridge_update_wm(struct drm_device *dev)
>
>         /* WM1 */
>         if (!ironlake_compute_srwm(dev, 1, enabled,
> -                                  SNB_READ_WM1_LATENCY() * 500,
> +                                  dev_priv->wm.pri_latency[1] * 500,
>                                    &sandybridge_display_srwm_info,
>                                    &sandybridge_cursor_srwm_info,
>                                    &fbc_wm, &plane_wm, &cursor_wm))
> @@ -2039,14 +2036,14 @@ static void ivybridge_update_wm(struct drm_device *dev)
>
>         I915_WRITE(WM1_LP_ILK,
>                    WM1_LP_SR_EN |
> -                  (SNB_READ_WM1_LATENCY() << WM1_LP_LATENCY_SHIFT) |
> +                  (dev_priv->wm.pri_latency[1] << WM1_LP_LATENCY_SHIFT) |
>                    (fbc_wm << WM1_LP_FBC_SHIFT) |
>                    (plane_wm << WM1_LP_SR_SHIFT) |
>                    cursor_wm);
>
>         /* WM2 */
>         if (!ironlake_compute_srwm(dev, 2, enabled,
> -                                  SNB_READ_WM2_LATENCY() * 500,
> +                                  dev_priv->wm.pri_latency[2] * 500,
>                                    &sandybridge_display_srwm_info,
>                                    &sandybridge_cursor_srwm_info,
>                                    &fbc_wm, &plane_wm, &cursor_wm))
> @@ -2054,19 +2051,19 @@ static void ivybridge_update_wm(struct drm_device *dev)
>
>         I915_WRITE(WM2_LP_ILK,
>                    WM2_LP_EN |
> -                  (SNB_READ_WM2_LATENCY() << WM1_LP_LATENCY_SHIFT) |
> +                  (dev_priv->wm.pri_latency[2] << WM1_LP_LATENCY_SHIFT) |
>                    (fbc_wm << WM1_LP_FBC_SHIFT) |
>                    (plane_wm << WM1_LP_SR_SHIFT) |
>                    cursor_wm);
>
>         /* WM3, note we have to correct the cursor latency */
>         if (!ironlake_compute_srwm(dev, 3, enabled,
> -                                  SNB_READ_WM3_LATENCY() * 500,
> +                                  dev_priv->wm.pri_latency[3] * 500,
>                                    &sandybridge_display_srwm_info,
>                                    &sandybridge_cursor_srwm_info,
>                                    &fbc_wm, &plane_wm, &ignore_cursor_wm) ||
>             !ironlake_compute_srwm(dev, 3, enabled,
> -                                  2 * SNB_READ_WM3_LATENCY() * 500,
> +                                  dev_priv->wm.cur_latency[3] * 500,

You've killed WaDoubleCursorLP3Latency here. You should probably
revive it and add the WA name in a comment.

Everything else looks correct. With that fixed: Reviewed-by: Paulo
Zanoni <paulo.r.zanoni@intel.com>.


>                                    &sandybridge_display_srwm_info,
>                                    &sandybridge_cursor_srwm_info,
>                                    &ignore_fbc_wm, &ignore_plane_wm, &cursor_wm))
> @@ -2074,7 +2071,7 @@ static void ivybridge_update_wm(struct drm_device *dev)
>
>         I915_WRITE(WM3_LP_ILK,
>                    WM3_LP_EN |
> -                  (SNB_READ_WM3_LATENCY() << WM1_LP_LATENCY_SHIFT) |
> +                  (dev_priv->wm.pri_latency[3] << WM1_LP_LATENCY_SHIFT) |
>                    (fbc_wm << WM1_LP_FBC_SHIFT) |
>                    (plane_wm << WM1_LP_SR_SHIFT) |
>                    cursor_wm);
> @@ -2818,7 +2815,7 @@ static void sandybridge_update_sprite_wm(struct drm_device *dev, int pipe,
>                                          bool enabled, bool scaled)
>  {
>         struct drm_i915_private *dev_priv = dev->dev_private;
> -       int latency = SNB_READ_WM0_LATENCY() * 100;     /* In unit 0.1us */
> +       int latency = dev_priv->wm.spr_latency[0] * 100;        /* In unit 0.1us */
>         u32 val;
>         int sprite_wm, reg;
>         int ret;
> @@ -2858,7 +2855,7 @@ static void sandybridge_update_sprite_wm(struct drm_device *dev, int pipe,
>         ret = sandybridge_compute_sprite_srwm(dev, pipe, sprite_width,
>                                               pixel_size,
>                                               &sandybridge_display_srwm_info,
> -                                             SNB_READ_WM1_LATENCY() * 500,
> +                                             dev_priv->wm.spr_latency[1] * 500,
>                                               &sprite_wm);
>         if (!ret) {
>                 DRM_DEBUG_KMS("failed to compute sprite lp1 wm on pipe %c\n",
> @@ -2874,7 +2871,7 @@ static void sandybridge_update_sprite_wm(struct drm_device *dev, int pipe,
>         ret = sandybridge_compute_sprite_srwm(dev, pipe, sprite_width,
>                                               pixel_size,
>                                               &sandybridge_display_srwm_info,
> -                                             SNB_READ_WM2_LATENCY() * 500,
> +                                             dev_priv->wm.spr_latency[2] * 500,
>                                               &sprite_wm);
>         if (!ret) {
>                 DRM_DEBUG_KMS("failed to compute sprite lp2 wm on pipe %c\n",
> @@ -2886,7 +2883,7 @@ static void sandybridge_update_sprite_wm(struct drm_device *dev, int pipe,
>         ret = sandybridge_compute_sprite_srwm(dev, pipe, sprite_width,
>                                               pixel_size,
>                                               &sandybridge_display_srwm_info,
> -                                             SNB_READ_WM3_LATENCY() * 500,
> +                                             dev_priv->wm.spr_latency[3] * 500,
>                                               &sprite_wm);
>         if (!ret) {
>                 DRM_DEBUG_KMS("failed to compute sprite lp3 wm on pipe %c\n",
> --
> 1.8.1.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

Re: [PATCH 16/17] drm/i915: Use the watermark latency values from dev_priv for ILK/SNB/IVB too

[Ville Syrjälä]

On Fri, Aug 02, 2013 at 12:16:52PM -0300, Paulo Zanoni wrote:
> 2013/8/1  <ville.syrjala@linux.intel.com>:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Adjust the current ILK/SNB/IVB watermark codepaths to use the
> > pre-populated latency values from dev_priv instead of reading
> > them out from the registers every time.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h |  9 -------
> >  drivers/gpu/drm/i915/intel_pm.c | 57 +++++++++++++++++++----------------------
> >  2 files changed, 27 insertions(+), 39 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 19941c6..17f6252 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -3202,9 +3202,6 @@
> >  #define  MLTR_WM2_SHIFT                8
> >  /* the unit of memory self-refresh latency time is 0.5us */
> >  #define  ILK_SRLT_MASK         0x3f
> > -#define ILK_LATENCY(shift)     (I915_READ(MLTR_ILK) >> (shift) & ILK_SRLT_MASK)
> > -#define ILK_READ_WM1_LATENCY() ILK_LATENCY(MLTR_WM1_SHIFT)
> > -#define ILK_READ_WM2_LATENCY() ILK_LATENCY(MLTR_WM2_SHIFT)
> >
> >  /* define the fifo size on Ironlake */
> >  #define ILK_DISPLAY_FIFO       128
> > @@ -3251,12 +3248,6 @@
> >  #define SSKPD_WM2_SHIFT                16
> >  #define SSKPD_WM3_SHIFT                24
> >
> > -#define SNB_LATENCY(shift)     (I915_READ(MCHBAR_MIRROR_BASE_SNB + SSKPD) >> (shift) & SSKPD_WM_MASK)
> > -#define SNB_READ_WM0_LATENCY()         SNB_LATENCY(SSKPD_WM0_SHIFT)
> > -#define SNB_READ_WM1_LATENCY()         SNB_LATENCY(SSKPD_WM1_SHIFT)
> > -#define SNB_READ_WM2_LATENCY()         SNB_LATENCY(SSKPD_WM2_SHIFT)
> > -#define SNB_READ_WM3_LATENCY()         SNB_LATENCY(SSKPD_WM3_SHIFT)
> > -
> >  /*
> >   * The two pipe frame counter registers are not synchronized, so
> >   * reading a stable value is somewhat tricky. The following code
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 149eb0a..5fe8c4e 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -1665,9 +1665,6 @@ static void i830_update_wm(struct drm_device *dev)
> >         I915_WRITE(FW_BLC, fwater_lo);
> >  }
> >
> > -#define ILK_LP0_PLANE_LATENCY          700
> > -#define ILK_LP0_CURSOR_LATENCY         1300
> > -
> >  /*
> >   * Check the wm result.
> >   *
> > @@ -1782,9 +1779,9 @@ static void ironlake_update_wm(struct drm_device *dev)
> >         enabled = 0;
> >         if (g4x_compute_wm0(dev, PIPE_A,
> >                             &ironlake_display_wm_info,
> > -                           ILK_LP0_PLANE_LATENCY,
> > +                           dev_priv->wm.pri_latency[0] * 100,
> >                             &ironlake_cursor_wm_info,
> > -                           ILK_LP0_CURSOR_LATENCY,
> > +                           dev_priv->wm.cur_latency[0] * 100,
> >                             &plane_wm, &cursor_wm)) {
> >                 I915_WRITE(WM0_PIPEA_ILK,
> >                            (plane_wm << WM0_PIPE_PLANE_SHIFT) | cursor_wm);
> > @@ -1796,9 +1793,9 @@ static void ironlake_update_wm(struct drm_device *dev)
> >
> >         if (g4x_compute_wm0(dev, PIPE_B,
> >                             &ironlake_display_wm_info,
> > -                           ILK_LP0_PLANE_LATENCY,
> > +                           dev_priv->wm.pri_latency[0] * 100,
> >                             &ironlake_cursor_wm_info,
> > -                           ILK_LP0_CURSOR_LATENCY,
> > +                           dev_priv->wm.cur_latency[0] * 100,
> >                             &plane_wm, &cursor_wm)) {
> >                 I915_WRITE(WM0_PIPEB_ILK,
> >                            (plane_wm << WM0_PIPE_PLANE_SHIFT) | cursor_wm);
> > @@ -1822,7 +1819,7 @@ static void ironlake_update_wm(struct drm_device *dev)
> >
> >         /* WM1 */
> >         if (!ironlake_compute_srwm(dev, 1, enabled,
> > -                                  ILK_READ_WM1_LATENCY() * 500,
> > +                                  dev_priv->wm.pri_latency[1] * 500,
> >                                    &ironlake_display_srwm_info,
> >                                    &ironlake_cursor_srwm_info,
> >                                    &fbc_wm, &plane_wm, &cursor_wm))
> > @@ -1830,14 +1827,14 @@ static void ironlake_update_wm(struct drm_device *dev)
> >
> >         I915_WRITE(WM1_LP_ILK,
> >                    WM1_LP_SR_EN |
> > -                  (ILK_READ_WM1_LATENCY() << WM1_LP_LATENCY_SHIFT) |
> > +                  (dev_priv->wm.pri_latency[1] << WM1_LP_LATENCY_SHIFT) |
> >                    (fbc_wm << WM1_LP_FBC_SHIFT) |
> >                    (plane_wm << WM1_LP_SR_SHIFT) |
> >                    cursor_wm);
> >
> >         /* WM2 */
> >         if (!ironlake_compute_srwm(dev, 2, enabled,
> > -                                  ILK_READ_WM2_LATENCY() * 500,
> > +                                  dev_priv->wm.pri_latency[2] * 500,
> >                                    &ironlake_display_srwm_info,
> >                                    &ironlake_cursor_srwm_info,
> >                                    &fbc_wm, &plane_wm, &cursor_wm))
> > @@ -1845,7 +1842,7 @@ static void ironlake_update_wm(struct drm_device *dev)
> >
> >         I915_WRITE(WM2_LP_ILK,
> >                    WM2_LP_EN |
> > -                  (ILK_READ_WM2_LATENCY() << WM1_LP_LATENCY_SHIFT) |
> > +                  (dev_priv->wm.pri_latency[2] << WM1_LP_LATENCY_SHIFT) |
> >                    (fbc_wm << WM1_LP_FBC_SHIFT) |
> >                    (plane_wm << WM1_LP_SR_SHIFT) |
> >                    cursor_wm);
> > @@ -1859,7 +1856,7 @@ static void ironlake_update_wm(struct drm_device *dev)
> >  static void sandybridge_update_wm(struct drm_device *dev)
> >  {
> >         struct drm_i915_private *dev_priv = dev->dev_private;
> > -       int latency = SNB_READ_WM0_LATENCY() * 100;     /* In unit 0.1us */
> > +       int latency = dev_priv->wm.pri_latency[0] * 100;        /* In unit 0.1us */
> >         u32 val;
> >         int fbc_wm, plane_wm, cursor_wm;
> >         unsigned int enabled;
> > @@ -1914,7 +1911,7 @@ static void sandybridge_update_wm(struct drm_device *dev)
> >
> >         /* WM1 */
> >         if (!ironlake_compute_srwm(dev, 1, enabled,
> > -                                  SNB_READ_WM1_LATENCY() * 500,
> > +                                  dev_priv->wm.pri_latency[1] * 500,
> >                                    &sandybridge_display_srwm_info,
> >                                    &sandybridge_cursor_srwm_info,
> >                                    &fbc_wm, &plane_wm, &cursor_wm))
> > @@ -1922,14 +1919,14 @@ static void sandybridge_update_wm(struct drm_device *dev)
> >
> >         I915_WRITE(WM1_LP_ILK,
> >                    WM1_LP_SR_EN |
> > -                  (SNB_READ_WM1_LATENCY() << WM1_LP_LATENCY_SHIFT) |
> > +                  (dev_priv->wm.pri_latency[1] << WM1_LP_LATENCY_SHIFT) |
> >                    (fbc_wm << WM1_LP_FBC_SHIFT) |
> >                    (plane_wm << WM1_LP_SR_SHIFT) |
> >                    cursor_wm);
> >
> >         /* WM2 */
> >         if (!ironlake_compute_srwm(dev, 2, enabled,
> > -                                  SNB_READ_WM2_LATENCY() * 500,
> > +                                  dev_priv->wm.pri_latency[2] * 500,
> >                                    &sandybridge_display_srwm_info,
> >                                    &sandybridge_cursor_srwm_info,
> >                                    &fbc_wm, &plane_wm, &cursor_wm))
> > @@ -1937,14 +1934,14 @@ static void sandybridge_update_wm(struct drm_device *dev)
> >
> >         I915_WRITE(WM2_LP_ILK,
> >                    WM2_LP_EN |
> > -                  (SNB_READ_WM2_LATENCY() << WM1_LP_LATENCY_SHIFT) |
> > +                  (dev_priv->wm.pri_latency[2] << WM1_LP_LATENCY_SHIFT) |
> >                    (fbc_wm << WM1_LP_FBC_SHIFT) |
> >                    (plane_wm << WM1_LP_SR_SHIFT) |
> >                    cursor_wm);
> >
> >         /* WM3 */
> >         if (!ironlake_compute_srwm(dev, 3, enabled,
> > -                                  SNB_READ_WM3_LATENCY() * 500,
> > +                                  dev_priv->wm.pri_latency[3] * 500,
> >                                    &sandybridge_display_srwm_info,
> >                                    &sandybridge_cursor_srwm_info,
> >                                    &fbc_wm, &plane_wm, &cursor_wm))
> > @@ -1952,7 +1949,7 @@ static void sandybridge_update_wm(struct drm_device *dev)
> >
> >         I915_WRITE(WM3_LP_ILK,
> >                    WM3_LP_EN |
> > -                  (SNB_READ_WM3_LATENCY() << WM1_LP_LATENCY_SHIFT) |
> > +                  (dev_priv->wm.pri_latency[3] << WM1_LP_LATENCY_SHIFT) |
> >                    (fbc_wm << WM1_LP_FBC_SHIFT) |
> >                    (plane_wm << WM1_LP_SR_SHIFT) |
> >                    cursor_wm);
> > @@ -1961,7 +1958,7 @@ static void sandybridge_update_wm(struct drm_device *dev)
> >  static void ivybridge_update_wm(struct drm_device *dev)
> >  {
> >         struct drm_i915_private *dev_priv = dev->dev_private;
> > -       int latency = SNB_READ_WM0_LATENCY() * 100;     /* In unit 0.1us */
> > +       int latency = dev_priv->wm.pri_latency[0] * 100;        /* In unit 0.1us */
> >         u32 val;
> >         int fbc_wm, plane_wm, cursor_wm;
> >         int ignore_fbc_wm, ignore_plane_wm, ignore_cursor_wm;
> > @@ -2031,7 +2028,7 @@ static void ivybridge_update_wm(struct drm_device *dev)
> >
> >         /* WM1 */
> >         if (!ironlake_compute_srwm(dev, 1, enabled,
> > -                                  SNB_READ_WM1_LATENCY() * 500,
> > +                                  dev_priv->wm.pri_latency[1] * 500,
> >                                    &sandybridge_display_srwm_info,
> >                                    &sandybridge_cursor_srwm_info,
> >                                    &fbc_wm, &plane_wm, &cursor_wm))
> > @@ -2039,14 +2036,14 @@ static void ivybridge_update_wm(struct drm_device *dev)
> >
> >         I915_WRITE(WM1_LP_ILK,
> >                    WM1_LP_SR_EN |
> > -                  (SNB_READ_WM1_LATENCY() << WM1_LP_LATENCY_SHIFT) |
> > +                  (dev_priv->wm.pri_latency[1] << WM1_LP_LATENCY_SHIFT) |
> >                    (fbc_wm << WM1_LP_FBC_SHIFT) |
> >                    (plane_wm << WM1_LP_SR_SHIFT) |
> >                    cursor_wm);
> >
> >         /* WM2 */
> >         if (!ironlake_compute_srwm(dev, 2, enabled,
> > -                                  SNB_READ_WM2_LATENCY() * 500,
> > +                                  dev_priv->wm.pri_latency[2] * 500,
> >                                    &sandybridge_display_srwm_info,
> >                                    &sandybridge_cursor_srwm_info,
> >                                    &fbc_wm, &plane_wm, &cursor_wm))
> > @@ -2054,19 +2051,19 @@ static void ivybridge_update_wm(struct drm_device *dev)
> >
> >         I915_WRITE(WM2_LP_ILK,
> >                    WM2_LP_EN |
> > -                  (SNB_READ_WM2_LATENCY() << WM1_LP_LATENCY_SHIFT) |
> > +                  (dev_priv->wm.pri_latency[2] << WM1_LP_LATENCY_SHIFT) |
> >                    (fbc_wm << WM1_LP_FBC_SHIFT) |
> >                    (plane_wm << WM1_LP_SR_SHIFT) |
> >                    cursor_wm);
> >
> >         /* WM3, note we have to correct the cursor latency */
> >         if (!ironlake_compute_srwm(dev, 3, enabled,
> > -                                  SNB_READ_WM3_LATENCY() * 500,
> > +                                  dev_priv->wm.pri_latency[3] * 500,
> >                                    &sandybridge_display_srwm_info,
> >                                    &sandybridge_cursor_srwm_info,
> >                                    &fbc_wm, &plane_wm, &ignore_cursor_wm) ||
> >             !ironlake_compute_srwm(dev, 3, enabled,
> > -                                  2 * SNB_READ_WM3_LATENCY() * 500,
> > +                                  dev_priv->wm.cur_latency[3] * 500,
> 
> You've killed WaDoubleCursorLP3Latency here. You should probably
> revive it and add the WA name in a comment.

cur_latency[3] already contains the 2x adjustment. You just complained
about it in the other patch :)

Or do you mean I should add another comment here w/ the w/a name?

> Everything else looks correct. With that fixed: Reviewed-by: Paulo
> Zanoni <paulo.r.zanoni@intel.com>.
> 
> 
> >                                    &sandybridge_display_srwm_info,
> >                                    &sandybridge_cursor_srwm_info,
> >                                    &ignore_fbc_wm, &ignore_plane_wm, &cursor_wm))
> > @@ -2074,7 +2071,7 @@ static void ivybridge_update_wm(struct drm_device *dev)
> >
> >         I915_WRITE(WM3_LP_ILK,
> >                    WM3_LP_EN |
> > -                  (SNB_READ_WM3_LATENCY() << WM1_LP_LATENCY_SHIFT) |
> > +                  (dev_priv->wm.pri_latency[3] << WM1_LP_LATENCY_SHIFT) |
> >                    (fbc_wm << WM1_LP_FBC_SHIFT) |
> >                    (plane_wm << WM1_LP_SR_SHIFT) |
> >                    cursor_wm);
> > @@ -2818,7 +2815,7 @@ static void sandybridge_update_sprite_wm(struct drm_device *dev, int pipe,
> >                                          bool enabled, bool scaled)
> >  {
> >         struct drm_i915_private *dev_priv = dev->dev_private;
> > -       int latency = SNB_READ_WM0_LATENCY() * 100;     /* In unit 0.1us */
> > +       int latency = dev_priv->wm.spr_latency[0] * 100;        /* In unit 0.1us */
> >         u32 val;
> >         int sprite_wm, reg;
> >         int ret;
> > @@ -2858,7 +2855,7 @@ static void sandybridge_update_sprite_wm(struct drm_device *dev, int pipe,
> >         ret = sandybridge_compute_sprite_srwm(dev, pipe, sprite_width,
> >                                               pixel_size,
> >                                               &sandybridge_display_srwm_info,
> > -                                             SNB_READ_WM1_LATENCY() * 500,
> > +                                             dev_priv->wm.spr_latency[1] * 500,
> >                                               &sprite_wm);
> >         if (!ret) {
> >                 DRM_DEBUG_KMS("failed to compute sprite lp1 wm on pipe %c\n",
> > @@ -2874,7 +2871,7 @@ static void sandybridge_update_sprite_wm(struct drm_device *dev, int pipe,
> >         ret = sandybridge_compute_sprite_srwm(dev, pipe, sprite_width,
> >                                               pixel_size,
> >                                               &sandybridge_display_srwm_info,
> > -                                             SNB_READ_WM2_LATENCY() * 500,
> > +                                             dev_priv->wm.spr_latency[2] * 500,
> >                                               &sprite_wm);
> >         if (!ret) {
> >                 DRM_DEBUG_KMS("failed to compute sprite lp2 wm on pipe %c\n",
> > @@ -2886,7 +2883,7 @@ static void sandybridge_update_sprite_wm(struct drm_device *dev, int pipe,
> >         ret = sandybridge_compute_sprite_srwm(dev, pipe, sprite_width,
> >                                               pixel_size,
> >                                               &sandybridge_display_srwm_info,
> > -                                             SNB_READ_WM3_LATENCY() * 500,
> > +                                             dev_priv->wm.spr_latency[3] * 500,
> >                                               &sprite_wm);
> >         if (!ret) {
> >                 DRM_DEBUG_KMS("failed to compute sprite lp3 wm on pipe %c\n",
> > --
> > 1.8.1.5
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Paulo Zanoni

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 16/17] drm/i915: Use the watermark latency values from dev_priv for ILK/SNB/IVB too

[Paulo Zanoni]

2013/8/2 Ville Syrjälä <ville.syrjala@linux.intel.com>:
> On Fri, Aug 02, 2013 at 12:16:52PM -0300, Paulo Zanoni wrote:
>> 2013/8/1  <ville.syrjala@linux.intel.com>:
>> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >
>> > Adjust the current ILK/SNB/IVB watermark codepaths to use the
>> > pre-populated latency values from dev_priv instead of reading
>> > them out from the registers every time.
>> >
>> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/i915_reg.h |  9 -------
>> >  drivers/gpu/drm/i915/intel_pm.c | 57 +++++++++++++++++++----------------------
>> >  2 files changed, 27 insertions(+), 39 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> > index 19941c6..17f6252 100644
>> > --- a/drivers/gpu/drm/i915/i915_reg.h
>> > +++ b/drivers/gpu/drm/i915/i915_reg.h
>> > @@ -3202,9 +3202,6 @@
>> >  #define  MLTR_WM2_SHIFT                8
>> >  /* the unit of memory self-refresh latency time is 0.5us */
>> >  #define  ILK_SRLT_MASK         0x3f
>> > -#define ILK_LATENCY(shift)     (I915_READ(MLTR_ILK) >> (shift) & ILK_SRLT_MASK)
>> > -#define ILK_READ_WM1_LATENCY() ILK_LATENCY(MLTR_WM1_SHIFT)
>> > -#define ILK_READ_WM2_LATENCY() ILK_LATENCY(MLTR_WM2_SHIFT)
>> >
>> >  /* define the fifo size on Ironlake */
>> >  #define ILK_DISPLAY_FIFO       128
>> > @@ -3251,12 +3248,6 @@
>> >  #define SSKPD_WM2_SHIFT                16
>> >  #define SSKPD_WM3_SHIFT                24
>> >
>> > -#define SNB_LATENCY(shift)     (I915_READ(MCHBAR_MIRROR_BASE_SNB + SSKPD) >> (shift) & SSKPD_WM_MASK)
>> > -#define SNB_READ_WM0_LATENCY()         SNB_LATENCY(SSKPD_WM0_SHIFT)
>> > -#define SNB_READ_WM1_LATENCY()         SNB_LATENCY(SSKPD_WM1_SHIFT)
>> > -#define SNB_READ_WM2_LATENCY()         SNB_LATENCY(SSKPD_WM2_SHIFT)
>> > -#define SNB_READ_WM3_LATENCY()         SNB_LATENCY(SSKPD_WM3_SHIFT)
>> > -
>> >  /*
>> >   * The two pipe frame counter registers are not synchronized, so
>> >   * reading a stable value is somewhat tricky. The following code
>> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> > index 149eb0a..5fe8c4e 100644
>> > --- a/drivers/gpu/drm/i915/intel_pm.c
>> > +++ b/drivers/gpu/drm/i915/intel_pm.c
>> > @@ -1665,9 +1665,6 @@ static void i830_update_wm(struct drm_device *dev)
>> >         I915_WRITE(FW_BLC, fwater_lo);
>> >  }
>> >
>> > -#define ILK_LP0_PLANE_LATENCY          700
>> > -#define ILK_LP0_CURSOR_LATENCY         1300
>> > -
>> >  /*
>> >   * Check the wm result.
>> >   *
>> > @@ -1782,9 +1779,9 @@ static void ironlake_update_wm(struct drm_device *dev)
>> >         enabled = 0;
>> >         if (g4x_compute_wm0(dev, PIPE_A,
>> >                             &ironlake_display_wm_info,
>> > -                           ILK_LP0_PLANE_LATENCY,
>> > +                           dev_priv->wm.pri_latency[0] * 100,
>> >                             &ironlake_cursor_wm_info,
>> > -                           ILK_LP0_CURSOR_LATENCY,
>> > +                           dev_priv->wm.cur_latency[0] * 100,
>> >                             &plane_wm, &cursor_wm)) {
>> >                 I915_WRITE(WM0_PIPEA_ILK,
>> >                            (plane_wm << WM0_PIPE_PLANE_SHIFT) | cursor_wm);
>> > @@ -1796,9 +1793,9 @@ static void ironlake_update_wm(struct drm_device *dev)
>> >
>> >         if (g4x_compute_wm0(dev, PIPE_B,
>> >                             &ironlake_display_wm_info,
>> > -                           ILK_LP0_PLANE_LATENCY,
>> > +                           dev_priv->wm.pri_latency[0] * 100,
>> >                             &ironlake_cursor_wm_info,
>> > -                           ILK_LP0_CURSOR_LATENCY,
>> > +                           dev_priv->wm.cur_latency[0] * 100,
>> >                             &plane_wm, &cursor_wm)) {
>> >                 I915_WRITE(WM0_PIPEB_ILK,
>> >                            (plane_wm << WM0_PIPE_PLANE_SHIFT) | cursor_wm);
>> > @@ -1822,7 +1819,7 @@ static void ironlake_update_wm(struct drm_device *dev)
>> >
>> >         /* WM1 */
>> >         if (!ironlake_compute_srwm(dev, 1, enabled,
>> > -                                  ILK_READ_WM1_LATENCY() * 500,
>> > +                                  dev_priv->wm.pri_latency[1] * 500,
>> >                                    &ironlake_display_srwm_info,
>> >                                    &ironlake_cursor_srwm_info,
>> >                                    &fbc_wm, &plane_wm, &cursor_wm))
>> > @@ -1830,14 +1827,14 @@ static void ironlake_update_wm(struct drm_device *dev)
>> >
>> >         I915_WRITE(WM1_LP_ILK,
>> >                    WM1_LP_SR_EN |
>> > -                  (ILK_READ_WM1_LATENCY() << WM1_LP_LATENCY_SHIFT) |
>> > +                  (dev_priv->wm.pri_latency[1] << WM1_LP_LATENCY_SHIFT) |
>> >                    (fbc_wm << WM1_LP_FBC_SHIFT) |
>> >                    (plane_wm << WM1_LP_SR_SHIFT) |
>> >                    cursor_wm);
>> >
>> >         /* WM2 */
>> >         if (!ironlake_compute_srwm(dev, 2, enabled,
>> > -                                  ILK_READ_WM2_LATENCY() * 500,
>> > +                                  dev_priv->wm.pri_latency[2] * 500,
>> >                                    &ironlake_display_srwm_info,
>> >                                    &ironlake_cursor_srwm_info,
>> >                                    &fbc_wm, &plane_wm, &cursor_wm))
>> > @@ -1845,7 +1842,7 @@ static void ironlake_update_wm(struct drm_device *dev)
>> >
>> >         I915_WRITE(WM2_LP_ILK,
>> >                    WM2_LP_EN |
>> > -                  (ILK_READ_WM2_LATENCY() << WM1_LP_LATENCY_SHIFT) |
>> > +                  (dev_priv->wm.pri_latency[2] << WM1_LP_LATENCY_SHIFT) |
>> >                    (fbc_wm << WM1_LP_FBC_SHIFT) |
>> >                    (plane_wm << WM1_LP_SR_SHIFT) |
>> >                    cursor_wm);
>> > @@ -1859,7 +1856,7 @@ static void ironlake_update_wm(struct drm_device *dev)
>> >  static void sandybridge_update_wm(struct drm_device *dev)
>> >  {
>> >         struct drm_i915_private *dev_priv = dev->dev_private;
>> > -       int latency = SNB_READ_WM0_LATENCY() * 100;     /* In unit 0.1us */
>> > +       int latency = dev_priv->wm.pri_latency[0] * 100;        /* In unit 0.1us */
>> >         u32 val;
>> >         int fbc_wm, plane_wm, cursor_wm;
>> >         unsigned int enabled;
>> > @@ -1914,7 +1911,7 @@ static void sandybridge_update_wm(struct drm_device *dev)
>> >
>> >         /* WM1 */
>> >         if (!ironlake_compute_srwm(dev, 1, enabled,
>> > -                                  SNB_READ_WM1_LATENCY() * 500,
>> > +                                  dev_priv->wm.pri_latency[1] * 500,
>> >                                    &sandybridge_display_srwm_info,
>> >                                    &sandybridge_cursor_srwm_info,
>> >                                    &fbc_wm, &plane_wm, &cursor_wm))
>> > @@ -1922,14 +1919,14 @@ static void sandybridge_update_wm(struct drm_device *dev)
>> >
>> >         I915_WRITE(WM1_LP_ILK,
>> >                    WM1_LP_SR_EN |
>> > -                  (SNB_READ_WM1_LATENCY() << WM1_LP_LATENCY_SHIFT) |
>> > +                  (dev_priv->wm.pri_latency[1] << WM1_LP_LATENCY_SHIFT) |
>> >                    (fbc_wm << WM1_LP_FBC_SHIFT) |
>> >                    (plane_wm << WM1_LP_SR_SHIFT) |
>> >                    cursor_wm);
>> >
>> >         /* WM2 */
>> >         if (!ironlake_compute_srwm(dev, 2, enabled,
>> > -                                  SNB_READ_WM2_LATENCY() * 500,
>> > +                                  dev_priv->wm.pri_latency[2] * 500,
>> >                                    &sandybridge_display_srwm_info,
>> >                                    &sandybridge_cursor_srwm_info,
>> >                                    &fbc_wm, &plane_wm, &cursor_wm))
>> > @@ -1937,14 +1934,14 @@ static void sandybridge_update_wm(struct drm_device *dev)
>> >
>> >         I915_WRITE(WM2_LP_ILK,
>> >                    WM2_LP_EN |
>> > -                  (SNB_READ_WM2_LATENCY() << WM1_LP_LATENCY_SHIFT) |
>> > +                  (dev_priv->wm.pri_latency[2] << WM1_LP_LATENCY_SHIFT) |
>> >                    (fbc_wm << WM1_LP_FBC_SHIFT) |
>> >                    (plane_wm << WM1_LP_SR_SHIFT) |
>> >                    cursor_wm);
>> >
>> >         /* WM3 */
>> >         if (!ironlake_compute_srwm(dev, 3, enabled,
>> > -                                  SNB_READ_WM3_LATENCY() * 500,
>> > +                                  dev_priv->wm.pri_latency[3] * 500,
>> >                                    &sandybridge_display_srwm_info,
>> >                                    &sandybridge_cursor_srwm_info,
>> >                                    &fbc_wm, &plane_wm, &cursor_wm))
>> > @@ -1952,7 +1949,7 @@ static void sandybridge_update_wm(struct drm_device *dev)
>> >
>> >         I915_WRITE(WM3_LP_ILK,
>> >                    WM3_LP_EN |
>> > -                  (SNB_READ_WM3_LATENCY() << WM1_LP_LATENCY_SHIFT) |
>> > +                  (dev_priv->wm.pri_latency[3] << WM1_LP_LATENCY_SHIFT) |
>> >                    (fbc_wm << WM1_LP_FBC_SHIFT) |
>> >                    (plane_wm << WM1_LP_SR_SHIFT) |
>> >                    cursor_wm);
>> > @@ -1961,7 +1958,7 @@ static void sandybridge_update_wm(struct drm_device *dev)
>> >  static void ivybridge_update_wm(struct drm_device *dev)
>> >  {
>> >         struct drm_i915_private *dev_priv = dev->dev_private;
>> > -       int latency = SNB_READ_WM0_LATENCY() * 100;     /* In unit 0.1us */
>> > +       int latency = dev_priv->wm.pri_latency[0] * 100;        /* In unit 0.1us */
>> >         u32 val;
>> >         int fbc_wm, plane_wm, cursor_wm;
>> >         int ignore_fbc_wm, ignore_plane_wm, ignore_cursor_wm;
>> > @@ -2031,7 +2028,7 @@ static void ivybridge_update_wm(struct drm_device *dev)
>> >
>> >         /* WM1 */
>> >         if (!ironlake_compute_srwm(dev, 1, enabled,
>> > -                                  SNB_READ_WM1_LATENCY() * 500,
>> > +                                  dev_priv->wm.pri_latency[1] * 500,
>> >                                    &sandybridge_display_srwm_info,
>> >                                    &sandybridge_cursor_srwm_info,
>> >                                    &fbc_wm, &plane_wm, &cursor_wm))
>> > @@ -2039,14 +2036,14 @@ static void ivybridge_update_wm(struct drm_device *dev)
>> >
>> >         I915_WRITE(WM1_LP_ILK,
>> >                    WM1_LP_SR_EN |
>> > -                  (SNB_READ_WM1_LATENCY() << WM1_LP_LATENCY_SHIFT) |
>> > +                  (dev_priv->wm.pri_latency[1] << WM1_LP_LATENCY_SHIFT) |
>> >                    (fbc_wm << WM1_LP_FBC_SHIFT) |
>> >                    (plane_wm << WM1_LP_SR_SHIFT) |
>> >                    cursor_wm);
>> >
>> >         /* WM2 */
>> >         if (!ironlake_compute_srwm(dev, 2, enabled,
>> > -                                  SNB_READ_WM2_LATENCY() * 500,
>> > +                                  dev_priv->wm.pri_latency[2] * 500,
>> >                                    &sandybridge_display_srwm_info,
>> >                                    &sandybridge_cursor_srwm_info,
>> >                                    &fbc_wm, &plane_wm, &cursor_wm))
>> > @@ -2054,19 +2051,19 @@ static void ivybridge_update_wm(struct drm_device *dev)
>> >
>> >         I915_WRITE(WM2_LP_ILK,
>> >                    WM2_LP_EN |
>> > -                  (SNB_READ_WM2_LATENCY() << WM1_LP_LATENCY_SHIFT) |
>> > +                  (dev_priv->wm.pri_latency[2] << WM1_LP_LATENCY_SHIFT) |
>> >                    (fbc_wm << WM1_LP_FBC_SHIFT) |
>> >                    (plane_wm << WM1_LP_SR_SHIFT) |
>> >                    cursor_wm);
>> >
>> >         /* WM3, note we have to correct the cursor latency */
>> >         if (!ironlake_compute_srwm(dev, 3, enabled,
>> > -                                  SNB_READ_WM3_LATENCY() * 500,
>> > +                                  dev_priv->wm.pri_latency[3] * 500,
>> >                                    &sandybridge_display_srwm_info,
>> >                                    &sandybridge_cursor_srwm_info,
>> >                                    &fbc_wm, &plane_wm, &ignore_cursor_wm) ||
>> >             !ironlake_compute_srwm(dev, 3, enabled,
>> > -                                  2 * SNB_READ_WM3_LATENCY() * 500,
>> > +                                  dev_priv->wm.cur_latency[3] * 500,
>>
>> You've killed WaDoubleCursorLP3Latency here. You should probably
>> revive it and add the WA name in a comment.
>
> cur_latency[3] already contains the 2x adjustment. You just complained
> about it in the other patch :)

Oops, sorry....

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

>
> Or do you mean I should add another comment here w/ the w/a name?

No need as the WA is "implemented" in the place that has the "* 2"
multiplication.

>
>> Everything else looks correct. With that fixed: Reviewed-by: Paulo
>> Zanoni <paulo.r.zanoni@intel.com>.
>>
>>
>> >                                    &sandybridge_display_srwm_info,
>> >                                    &sandybridge_cursor_srwm_info,
>> >                                    &ignore_fbc_wm, &ignore_plane_wm, &cursor_wm))
>> > @@ -2074,7 +2071,7 @@ static void ivybridge_update_wm(struct drm_device *dev)
>> >
>> >         I915_WRITE(WM3_LP_ILK,
>> >                    WM3_LP_EN |
>> > -                  (SNB_READ_WM3_LATENCY() << WM1_LP_LATENCY_SHIFT) |
>> > +                  (dev_priv->wm.pri_latency[3] << WM1_LP_LATENCY_SHIFT) |
>> >                    (fbc_wm << WM1_LP_FBC_SHIFT) |
>> >                    (plane_wm << WM1_LP_SR_SHIFT) |
>> >                    cursor_wm);
>> > @@ -2818,7 +2815,7 @@ static void sandybridge_update_sprite_wm(struct drm_device *dev, int pipe,
>> >                                          bool enabled, bool scaled)
>> >  {
>> >         struct drm_i915_private *dev_priv = dev->dev_private;
>> > -       int latency = SNB_READ_WM0_LATENCY() * 100;     /* In unit 0.1us */
>> > +       int latency = dev_priv->wm.spr_latency[0] * 100;        /* In unit 0.1us */
>> >         u32 val;
>> >         int sprite_wm, reg;
>> >         int ret;
>> > @@ -2858,7 +2855,7 @@ static void sandybridge_update_sprite_wm(struct drm_device *dev, int pipe,
>> >         ret = sandybridge_compute_sprite_srwm(dev, pipe, sprite_width,
>> >                                               pixel_size,
>> >                                               &sandybridge_display_srwm_info,
>> > -                                             SNB_READ_WM1_LATENCY() * 500,
>> > +                                             dev_priv->wm.spr_latency[1] * 500,
>> >                                               &sprite_wm);
>> >         if (!ret) {
>> >                 DRM_DEBUG_KMS("failed to compute sprite lp1 wm on pipe %c\n",
>> > @@ -2874,7 +2871,7 @@ static void sandybridge_update_sprite_wm(struct drm_device *dev, int pipe,
>> >         ret = sandybridge_compute_sprite_srwm(dev, pipe, sprite_width,
>> >                                               pixel_size,
>> >                                               &sandybridge_display_srwm_info,
>> > -                                             SNB_READ_WM2_LATENCY() * 500,
>> > +                                             dev_priv->wm.spr_latency[2] * 500,
>> >                                               &sprite_wm);
>> >         if (!ret) {
>> >                 DRM_DEBUG_KMS("failed to compute sprite lp2 wm on pipe %c\n",
>> > @@ -2886,7 +2883,7 @@ static void sandybridge_update_sprite_wm(struct drm_device *dev, int pipe,
>> >         ret = sandybridge_compute_sprite_srwm(dev, pipe, sprite_width,
>> >                                               pixel_size,
>> >                                               &sandybridge_display_srwm_info,
>> > -                                             SNB_READ_WM3_LATENCY() * 500,
>> > +                                             dev_priv->wm.spr_latency[3] * 500,
>> >                                               &sprite_wm);
>> >         if (!ret) {
>> >                 DRM_DEBUG_KMS("failed to compute sprite lp3 wm on pipe %c\n",
>> > --
>> > 1.8.1.5
>> >
>> > _______________________________________________
>> > Intel-gfx mailing list
>> > Intel-gfx@lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>>
>>
>> --
>> Paulo Zanoni
>
> --
> Ville Syrjälä
> Intel OTC



-- 
Paulo Zanoni

Re: [PATCH 17/17] drm/i915: Add comments about units of latency values

[Paulo Zanoni]

2013/8/1  <ville.syrjala@linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> All the ILK+ WM compute functions take the latency values in 0.1us
> units. Add a few comments to remind people about that.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

(but I would have written Latency instead of latency :P )

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 5fe8c4e..51f445f 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2108,6 +2108,7 @@ static uint32_t ilk_pipe_pixel_rate(struct drm_device *dev,
>         return pixel_rate;
>  }
>
> +/* latency must be in 0.1us units. */
>  static uint32_t ilk_wm_method1(uint32_t pixel_rate, uint8_t bytes_per_pixel,
>                                uint32_t latency)
>  {
> @@ -2122,6 +2123,7 @@ static uint32_t ilk_wm_method1(uint32_t pixel_rate, uint8_t bytes_per_pixel,
>         return ret;
>  }
>
> +/* latency must be in 0.1us units. */
>  static uint32_t ilk_wm_method2(uint32_t pixel_rate, uint32_t pipe_htotal,
>                                uint32_t horiz_pixels, uint8_t bytes_per_pixel,
>                                uint32_t latency)
> @@ -2185,7 +2187,10 @@ enum hsw_data_buf_partitioning {
>         HSW_DATA_BUF_PART_5_6,
>  };
>
> -/* For both WM_PIPE and WM_LP. */
> +/*
> + * For both WM_PIPE and WM_LP.
> + * mem_value must be in 0.1us units.
> + */
>  static uint32_t ilk_compute_pri_wm(struct hsw_pipe_wm_parameters *params,
>                                    uint32_t mem_value,
>                                    bool is_lp)
> @@ -2212,7 +2217,10 @@ static uint32_t ilk_compute_pri_wm(struct hsw_pipe_wm_parameters *params,
>         return min(method1, method2);
>  }
>
> -/* For both WM_PIPE and WM_LP. */
> +/*
> + * For both WM_PIPE and WM_LP.
> + * mem_value must be in 0.1us units.
> + */
>  static uint32_t ilk_compute_spr_wm(struct hsw_pipe_wm_parameters *params,
>                                    uint32_t mem_value)
>  {
> @@ -2232,7 +2240,10 @@ static uint32_t ilk_compute_spr_wm(struct hsw_pipe_wm_parameters *params,
>         return min(method1, method2);
>  }
>
> -/* For both WM_PIPE and WM_LP. */
> +/*
> + * For both WM_PIPE and WM_LP.
> + * mem_value must be in 0.1us units.
> + */
>  static uint32_t ilk_compute_cur_wm(struct hsw_pipe_wm_parameters *params,
>                                    uint32_t mem_value)
>  {
> --
> 1.8.1.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

Re: [PATCH 17/17] drm/i915: Add comments about units of latency values

[Ville Syrjälä]

On Fri, Aug 02, 2013 at 12:58:07PM -0300, Paulo Zanoni wrote:
> 2013/8/1  <ville.syrjala@linux.intel.com>:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > All the ILK+ WM compute functions take the latency values in 0.1us
> > units. Add a few comments to remind people about that.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> (but I would have written Latency instead of latency :P )

I was considering going for full blown kernel-doc, but since we don't
currently generate docs for i915, i just used the name of the argument
in question.

> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 17 ++++++++++++++---
> >  1 file changed, 14 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 5fe8c4e..51f445f 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -2108,6 +2108,7 @@ static uint32_t ilk_pipe_pixel_rate(struct drm_device *dev,
> >         return pixel_rate;
> >  }
> >
> > +/* latency must be in 0.1us units. */
> >  static uint32_t ilk_wm_method1(uint32_t pixel_rate, uint8_t bytes_per_pixel,
> >                                uint32_t latency)
> >  {
> > @@ -2122,6 +2123,7 @@ static uint32_t ilk_wm_method1(uint32_t pixel_rate, uint8_t bytes_per_pixel,
> >         return ret;
> >  }
> >
> > +/* latency must be in 0.1us units. */
> >  static uint32_t ilk_wm_method2(uint32_t pixel_rate, uint32_t pipe_htotal,
> >                                uint32_t horiz_pixels, uint8_t bytes_per_pixel,
> >                                uint32_t latency)
> > @@ -2185,7 +2187,10 @@ enum hsw_data_buf_partitioning {
> >         HSW_DATA_BUF_PART_5_6,
> >  };
> >
> > -/* For both WM_PIPE and WM_LP. */
> > +/*
> > + * For both WM_PIPE and WM_LP.
> > + * mem_value must be in 0.1us units.
> > + */
> >  static uint32_t ilk_compute_pri_wm(struct hsw_pipe_wm_parameters *params,
> >                                    uint32_t mem_value,
> >                                    bool is_lp)
> > @@ -2212,7 +2217,10 @@ static uint32_t ilk_compute_pri_wm(struct hsw_pipe_wm_parameters *params,
> >         return min(method1, method2);
> >  }
> >
> > -/* For both WM_PIPE and WM_LP. */
> > +/*
> > + * For both WM_PIPE and WM_LP.
> > + * mem_value must be in 0.1us units.
> > + */
> >  static uint32_t ilk_compute_spr_wm(struct hsw_pipe_wm_parameters *params,
> >                                    uint32_t mem_value)
> >  {
> > @@ -2232,7 +2240,10 @@ static uint32_t ilk_compute_spr_wm(struct hsw_pipe_wm_parameters *params,
> >         return min(method1, method2);
> >  }
> >
> > -/* For both WM_PIPE and WM_LP. */
> > +/*
> > + * For both WM_PIPE and WM_LP.
> > + * mem_value must be in 0.1us units.
> > + */
> >  static uint32_t ilk_compute_cur_wm(struct hsw_pipe_wm_parameters *params,
> >                                    uint32_t mem_value)
> >  {
> > --
> > 1.8.1.5
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Paulo Zanoni

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH v2 01/17] drm/i915: Add scaled paramater to update_sprite_watermarks()

[Daniel Vetter]

On Thu, Aug 01, 2013 at 04:18:39PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> For calculating watermarks we want to know whether sprites are
> scaled. Pass that information to update_sprite_watermarks() so that
> eventually we may do some watermark pre-computing.
> 
> v2: Use "enabled" consistently, fix commit msg
> 
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Oops, already merged v1. Please extract the fixup again and resubmit ;-)
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_drv.h     |  2 +-
>  drivers/gpu/drm/i915/intel_drv.h    |  7 ++++---
>  drivers/gpu/drm/i915/intel_pm.c     | 15 ++++++++-------
>  drivers/gpu/drm/i915/intel_sprite.c | 11 +++++++----
>  4 files changed, 20 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 82ea281..fe466bc 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -361,7 +361,7 @@ 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,
> -				 bool enable);
> +				 bool enabled, bool scaled);
>  	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_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 474797b..ed33976 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -349,7 +349,8 @@ struct intel_plane {
>  	 * for the watermark calculations. Currently only Haswell uses this.
>  	 */
>  	struct {
> -		bool enable;
> +		bool enabled;
> +		bool scaled;
>  		uint8_t bytes_per_pixel;
>  		uint32_t horiz_pixels;
>  	} wm;
> @@ -770,8 +771,8 @@ extern void intel_ddi_init(struct drm_device *dev, enum port port);
>  /* For use by IVB LP watermark workaround in intel_sprite.c */
>  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, bool enable);
> +					   uint32_t sprite_width, int pixel_size,
> +					   bool enabled, bool scaled);
>  
>  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 0a5ba92..5a008d1 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2388,7 +2388,7 @@ static void hsw_compute_wm_parameters(struct drm_device *dev,
>  		pipe = intel_plane->pipe;
>  		p = &params[pipe];
>  
> -		p->sprite_enabled = intel_plane->wm.enable;
> +		p->sprite_enabled = intel_plane->wm.enabled;
>  		p->spr_bytes_per_pixel = intel_plane->wm.bytes_per_pixel;
>  		p->spr_horiz_pixels = intel_plane->wm.horiz_pixels;
>  
> @@ -2616,7 +2616,7 @@ static void haswell_update_wm(struct drm_device *dev)
>  
>  static void haswell_update_sprite_wm(struct drm_device *dev, int pipe,
>  				     uint32_t sprite_width, int pixel_size,
> -				     bool enable)
> +				     bool enabled, bool scaled)
>  {
>  	struct drm_plane *plane;
>  
> @@ -2624,7 +2624,8 @@ static void haswell_update_sprite_wm(struct drm_device *dev, int pipe,
>  		struct intel_plane *intel_plane = to_intel_plane(plane);
>  
>  		if (intel_plane->pipe == pipe) {
> -			intel_plane->wm.enable = enable;
> +			intel_plane->wm.enabled = enabled;
> +			intel_plane->wm.scaled = scaled;
>  			intel_plane->wm.horiz_pixels = sprite_width + 1;
>  			intel_plane->wm.bytes_per_pixel = pixel_size;
>  			break;
> @@ -2712,7 +2713,7 @@ sandybridge_compute_sprite_srwm(struct drm_device *dev, int plane,
>  
>  static void sandybridge_update_sprite_wm(struct drm_device *dev, int pipe,
>  					 uint32_t sprite_width, int pixel_size,
> -					 bool enable)
> +					 bool enabled, bool scaled)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	int latency = SNB_READ_WM0_LATENCY() * 100;	/* In unit 0.1us */
> @@ -2720,7 +2721,7 @@ static void sandybridge_update_sprite_wm(struct drm_device *dev, int pipe,
>  	int sprite_wm, reg;
>  	int ret;
>  
> -	if (!enable)
> +	if (!enabled)
>  		return;
>  
>  	switch (pipe) {
> @@ -2835,13 +2836,13 @@ void intel_update_watermarks(struct drm_device *dev)
>  
>  void intel_update_sprite_watermarks(struct drm_device *dev, int pipe,
>  				    uint32_t sprite_width, int pixel_size,
> -				    bool enable)
> +				    bool enabled, bool scaled)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
>  	if (dev_priv->display.update_sprite_wm)
>  		dev_priv->display.update_sprite_wm(dev, pipe, sprite_width,
> -						   pixel_size, enable);
> +						   pixel_size, enabled, scaled);
>  }
>  
>  static struct drm_i915_gem_object *
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 55bdf70..069155f 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -114,7 +114,8 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_framebuffer *fb,
>  	crtc_w--;
>  	crtc_h--;
>  
> -	intel_update_sprite_watermarks(dev, pipe, crtc_w, pixel_size, true);
> +	intel_update_sprite_watermarks(dev, pipe, crtc_w, pixel_size, true,
> +				       src_w != crtc_w || src_h != crtc_h);
>  
>  	I915_WRITE(SPSTRIDE(pipe, plane), fb->pitches[0]);
>  	I915_WRITE(SPPOS(pipe, plane), (crtc_y << 16) | crtc_x);
> @@ -268,7 +269,8 @@ ivb_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
>  	crtc_w--;
>  	crtc_h--;
>  
> -	intel_update_sprite_watermarks(dev, pipe, crtc_w, pixel_size, true);
> +	intel_update_sprite_watermarks(dev, pipe, crtc_w, pixel_size, true,
> +				       src_w != crtc_w || src_h != crtc_h);
>  
>  	/*
>  	 * IVB workaround: must disable low power watermarks for at least
> @@ -336,7 +338,7 @@ ivb_disable_plane(struct drm_plane *plane)
>  
>  	dev_priv->sprite_scaling_enabled &= ~(1 << pipe);
>  
> -	intel_update_sprite_watermarks(dev, pipe, 0, 0, false);
> +	intel_update_sprite_watermarks(dev, pipe, 0, 0, false, false);
>  
>  	/* potentially re-enable LP watermarks */
>  	if (scaling_was_enabled && !dev_priv->sprite_scaling_enabled)
> @@ -456,7 +458,8 @@ ilk_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
>  	crtc_w--;
>  	crtc_h--;
>  
> -	intel_update_sprite_watermarks(dev, pipe, crtc_w, pixel_size, true);
> +	intel_update_sprite_watermarks(dev, pipe, crtc_w, pixel_size, true,
> +				       src_w != crtc_w || src_h != crtc_h);
>  
>  	dvsscale = 0;
>  	if (IS_GEN5(dev) || crtc_w != src_w || crtc_h != src_h)
> -- 
> 1.8.1.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

Re: [PATCH v2 12/17] drm/i915: Store the watermark latency values in dev_priv

[Daniel Vetter]

On Thu, Aug 01, 2013 at 03:23:56PM +0100, Chris Wilson wrote:
> On Thu, Aug 01, 2013 at 04:18:50PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Rather than having to read the latency values out every time, just
> > store them in dev_priv.
> > 
> > On ILK and IVB there is a difference between some of the latency
> > values for different planes, so store the latency values for each
> > plane type separately, and apply the necesary fixups during init.
> > 
> > v2: Fix some checkpatch complaints
> 
> As you are now storing these, it would also be useful to add a debugfs
> file to dump them out for sanity checking. I leave the request to adjust
> them via debugfs to somebody else ;-)

I guess since this is ramping up to be the furture plane_config we might
just need a bit of plane config dump support. Of course we need to be
careful to not redump on every nuclear pageflip, but only if something
interesting changes ...

Or did you just think of the underlying latency values that steer the wm
comuptation?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH v2 15/17] drm/i915: Disable specific watermark levels when latency is zero

[Daniel Vetter]

On Fri, Aug 02, 2013 at 11:48:29AM -0300, Paulo Zanoni wrote:
> 2013/8/1  <ville.syrjala@linux.intel.com>:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Return UINT_MAX for the calculated WM level if the latency is zero.
> > This will lead to marking the WM level as disabled.
> >
> > I'm not sure if latency==0 should mean that we want to disable the
> > level. But that's the implication I got from the fact that we don't
> > even enable the watermark code of the SSKDP register is 0.
> >
> > v2: Use WARN() to scare people
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 53967ef..149eb0a 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -2116,6 +2116,9 @@ static uint32_t ilk_wm_method1(uint32_t pixel_rate, uint8_t bytes_per_pixel,
> >  {
> >         uint64_t ret;
> >
> > +       if (WARN(latency == 0, "Latency value missing\n"))
> 
> I know I'm the person who asked to scream loud in case the latency is
> zero, but I guess that due to the previous patch we'll already
> DRM_ERROR in case the latency is zero, so this WARN will be redundant.
> So maybe your previous patch 16/35 would be more adequate.
> 
> So I'll give a "Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>"
> to both this and the previous version (16/35), and let you (or Daniel)
> decide which one to merge.

We have a few other cases where we both DRM_ERROR and then WARN into
dmesg, so I'm ok with this one here.
-Daniel

> 
> 
> > +               return UINT_MAX;
> > +
> >         ret = (uint64_t) pixel_rate * bytes_per_pixel * latency;
> >         ret = DIV_ROUND_UP_ULL(ret, 64 * 10000) + 2;
> >
> > @@ -2128,6 +2131,9 @@ static uint32_t ilk_wm_method2(uint32_t pixel_rate, uint32_t pipe_htotal,
> >  {
> >         uint32_t ret;
> >
> > +       if (WARN(latency == 0, "Latency value missing\n"))
> > +               return UINT_MAX;
> > +
> >         ret = (latency * pixel_rate) / (pipe_htotal * 10000);
> >         ret = (ret + 1) * horiz_pixels * bytes_per_pixel;
> >         ret = DIV_ROUND_UP(ret, 64) + 2;
> > --
> > 1.8.1.5
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Paulo Zanoni
> _______________________________________________
> 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 17/17] drm/i915: Add comments about units of latency values

[Daniel Vetter]

On Fri, Aug 02, 2013 at 07:09:31PM +0300, Ville Syrjälä wrote:
> On Fri, Aug 02, 2013 at 12:58:07PM -0300, Paulo Zanoni wrote:
> > 2013/8/1  <ville.syrjala@linux.intel.com>:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > All the ILK+ WM compute functions take the latency values in 0.1us
> > > units. Add a few comments to remind people about that.
> > >
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > 
> > (but I would have written Latency instead of latency :P )
> 
> I was considering going for full blown kernel-doc, but since we don't
> currently generate docs for i915, i just used the name of the argument
> in question.

We'll get there ;-) For kerneldoc I think we should first still plug some
holes in drm core, but those get filled fast. Next up is properly
documenting the userspace ioctl interface and related stuff. Then we can
tackle real kerneldoc for important functions. Probably still a few years
off ;-)

I've (hopefully) merged all the remaining patches in this series with the
exception of the fixup for the very first patch. Please resend that cute
little bikeshed.

Thanks for the patches&reivew.
-Daniel

> 
> > > ---
> > >  drivers/gpu/drm/i915/intel_pm.c | 17 ++++++++++++++---
> > >  1 file changed, 14 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > > index 5fe8c4e..51f445f 100644
> > > --- a/drivers/gpu/drm/i915/intel_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > > @@ -2108,6 +2108,7 @@ static uint32_t ilk_pipe_pixel_rate(struct drm_device *dev,
> > >         return pixel_rate;
> > >  }
> > >
> > > +/* latency must be in 0.1us units. */
> > >  static uint32_t ilk_wm_method1(uint32_t pixel_rate, uint8_t bytes_per_pixel,
> > >                                uint32_t latency)
> > >  {
> > > @@ -2122,6 +2123,7 @@ static uint32_t ilk_wm_method1(uint32_t pixel_rate, uint8_t bytes_per_pixel,
> > >         return ret;
> > >  }
> > >
> > > +/* latency must be in 0.1us units. */
> > >  static uint32_t ilk_wm_method2(uint32_t pixel_rate, uint32_t pipe_htotal,
> > >                                uint32_t horiz_pixels, uint8_t bytes_per_pixel,
> > >                                uint32_t latency)
> > > @@ -2185,7 +2187,10 @@ enum hsw_data_buf_partitioning {
> > >         HSW_DATA_BUF_PART_5_6,
> > >  };
> > >
> > > -/* For both WM_PIPE and WM_LP. */
> > > +/*
> > > + * For both WM_PIPE and WM_LP.
> > > + * mem_value must be in 0.1us units.
> > > + */
> > >  static uint32_t ilk_compute_pri_wm(struct hsw_pipe_wm_parameters *params,
> > >                                    uint32_t mem_value,
> > >                                    bool is_lp)
> > > @@ -2212,7 +2217,10 @@ static uint32_t ilk_compute_pri_wm(struct hsw_pipe_wm_parameters *params,
> > >         return min(method1, method2);
> > >  }
> > >
> > > -/* For both WM_PIPE and WM_LP. */
> > > +/*
> > > + * For both WM_PIPE and WM_LP.
> > > + * mem_value must be in 0.1us units.
> > > + */
> > >  static uint32_t ilk_compute_spr_wm(struct hsw_pipe_wm_parameters *params,
> > >                                    uint32_t mem_value)
> > >  {
> > > @@ -2232,7 +2240,10 @@ static uint32_t ilk_compute_spr_wm(struct hsw_pipe_wm_parameters *params,
> > >         return min(method1, method2);
> > >  }
> > >
> > > -/* For both WM_PIPE and WM_LP. */
> > > +/*
> > > + * For both WM_PIPE and WM_LP.
> > > + * mem_value must be in 0.1us units.
> > > + */
> > >  static uint32_t ilk_compute_cur_wm(struct hsw_pipe_wm_parameters *params,
> > >                                    uint32_t mem_value)
> > >  {
> > > --
> > > 1.8.1.5
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > 
> > 
> > -- 
> > Paulo Zanoni
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> 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 v2 12/17] drm/i915: Store the watermark latency values in dev_priv

[Chris Wilson]

On Mon, Aug 05, 2013 at 06:25:54PM +0200, Daniel Vetter wrote:
> On Thu, Aug 01, 2013 at 03:23:56PM +0100, Chris Wilson wrote:
> > On Thu, Aug 01, 2013 at 04:18:50PM +0300, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Rather than having to read the latency values out every time, just
> > > store them in dev_priv.
> > > 
> > > On ILK and IVB there is a difference between some of the latency
> > > values for different planes, so store the latency values for each
> > > plane type separately, and apply the necesary fixups during init.
> > > 
> > > v2: Fix some checkpatch complaints
> > 
> > As you are now storing these, it would also be useful to add a debugfs
> > file to dump them out for sanity checking. I leave the request to adjust
> > them via debugfs to somebody else ;-)
> 
> I guess since this is ramping up to be the furture plane_config we might
> just need a bit of plane config dump support. Of course we need to be
> careful to not redump on every nuclear pageflip, but only if something
> interesting changes ...
> 
> Or did you just think of the underlying latency values that steer the wm
> comuptation?

Here I was just thinking of the latency values. We should be able to
eyeball them for consistency, and, if need be, use pen and paper to sanity
check our calculations. That's made a whole lot more convenient if the
kernel tells us exactly what it is using, perhaps with the raw register
values thrown in as well for the next level in paranoia.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre