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

[v***@linux.intel.com]

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.

[v***@linux.intel.com]

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

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

Reviewed-by: Paulo Zanoni <***@intel.com>
Signed-off-by: Ville Syrjälä <***@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;

[v***@linux.intel.com]

From: Ville Syrjälä <***@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 <***@intel.com>
Signed-off-by: Ville Syrjälä <***@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 */

[v***@linux.intel.com]

From: Ville Syrjälä <***@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 <***@intel.com>
Signed-off-by: Ville Syrjälä <***@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 =

[v***@linux.intel.com]

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

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

Reviewed-by: Paulo Zanoni <***@intel.com>
Signed-off-by: Ville Syrjälä <***@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]);

[v***@linux.intel.com]

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

These functions are appropriate for everything since ILK.

Reviewed-by: Paulo Zanoni <***@intel.com> (SNB/IVB only)
Signed-off-by: Ville Syrjälä <***@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",

[v***@linux.intel.com]

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

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

Reviewed-by: Paulo Zanoni <***@intel.com>
Signed-off-by: Ville Syrjälä <***@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) {

[v***@linux.intel.com]

From: Ville Syrjälä <***@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 <***@intel.com>
Signed-off-by: Ville Syrjälä <***@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)

[Daniel Vetter]

Post by v***@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

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

Post by v***@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
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[v***@linux.intel.com]

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

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

Reviewed-by: Paulo Zanoni <***@intel.com>
Signed-off-by: Ville Syrjälä <***@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);

[v***@linux.intel.com]

From: Ville Syrjälä <***@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 <***@intel.com>
Signed-off-by: Ville Syrjälä <***@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;

[v***@linux.intel.com]

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

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

Reviewed-by: Paulo Zanoni <***@intel.com>
Signed-off-by: Ville Syrjälä <***@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;
}
}

[v***@linux.intel.com]

From: Ville Syrjälä <***@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ä <***@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;
}
}

[Paulo Zanoni]

Post by v***@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.

No docs, but seems consistent with the current code.

Reviewed-by: Paulo Zanoni <***@intel.com>

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

Post by v***@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
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[v***@linux.intel.com]

From: Ville Syrjälä <***@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ä <***@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;

[Chris Wilson]

Post by v***@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

[Daniel Vetter]

Post by Chris Wilson

Post by v***@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

[Chris Wilson]

Post by Daniel Vetter

Post by Chris Wilson

Post by v***@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

[Paulo Zanoni]

Post by v***@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
---
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 {
+ /*
+ * 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.

Post by v***@linux.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
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[v***@linux.intel.com]

From: Ville Syrjälä <***@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ä <***@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 {

[Paulo Zanoni]

Post by v***@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
---
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
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[v***@linux.intel.com]

From: Ville Syrjälä <***@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ä <***@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,

[Paulo Zanoni]

Post by v***@linux.intel.com
Seeing the watermark latency values in dmesg might help sometimes.
v2: Use DRM_ERROR() when expected latency values are missing
---
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.

Post by v***@linux.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
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Ville Syrjälä]

Post by Paulo Zanoni

Post by v***@linux.intel.com
Seeing the watermark latency values in dmesg might help sometimes.
v2: Use DRM_ERROR() when expected latency values are missing
---
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.

Post by Paulo Zanoni

Post by v***@linux.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
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Paulo Zanoni

[v***@linux.intel.com]

From: Ville Syrjälä <***@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ä <***@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;

[Paulo Zanoni]

Post by v***@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
---
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 <***@intel.com>"
to both this and the previous version (16/35), and let you (or Daniel)
decide which one to merge.

Post by v***@linux.intel.com
+ 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
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Daniel Vetter]

Post by Paulo Zanoni

Post by v***@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
---
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.
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

Post by Paulo Zanoni

Post by v***@linux.intel.com
+ 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
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[v***@linux.intel.com]

From: Ville Syrjälä <***@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ä <***@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",

[Paulo Zanoni]

Post by v***@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.
---
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

Post by v***@linux.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
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Ville Syrjälä]

Post by Paulo Zanoni

Post by v***@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.
---
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?

Post by Paulo Zanoni
Everything else looks correct. With that fixed: Reviewed-by: Paulo

Post by v***@linux.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
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Paulo Zanoni

[Paulo Zanoni]

Post by Ville Syrjälä

Post by Paulo Zanoni

Post by v***@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.
---
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....

Post by Ville Syrjälä
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.

Post by Ville Syrjälä

Post by Paulo Zanoni
Everything else looks correct. With that fixed: Reviewed-by: Paulo

Post by v***@linux.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
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Paulo Zanoni

--
Ville Syrjälä
Intel OTC

[v***@linux.intel.com]

From: Ville Syrjälä <***@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ä <***@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)
{

[Paulo Zanoni]

Post by v***@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.

Reviewed-by: Paulo Zanoni <***@intel.com>

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

Post by v***@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
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Ville Syrjälä]

Post by Paulo Zanoni

Post by v***@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.

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

Post by Paulo Zanoni

Post by v***@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
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Paulo Zanoni

[Daniel Vetter]

Post by Ville Syrjälä

Post by Paulo Zanoni

Post by v***@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.

(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

Post by Ville Syrjälä

Post by Paulo Zanoni

Post by v***@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
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Paulo Zanoni

--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
http://lists.freedesktop.org/mailman/listinfo/intel-gfx