* [PATCH 0/7] drm/i915: Convert gen4- watermarks to atomic.
@ 2017-08-07 10:48 Maarten Lankhorst
2017-08-07 10:48 ` [PATCH 1/7] drm/i915: Calculate gen3- watermarks semi-atomically Maarten Lankhorst
` (8 more replies)
0 siblings, 9 replies; 14+ messages in thread
From: Maarten Lankhorst @ 2017-08-07 10:48 UTC (permalink / raw)
To: intel-gfx
Calculate watermarks for gen4 and lower atomically. This has been tested
on the pineview system in CI. It fixes bug 101597, which is a vblank wait
timed out when running kms_pipe_crc_basic. The changes to CXSR will allow
the test to pass.
Maarten Lankhorst (7):
drm/i915: Calculate gen3- watermarks semi-atomically.
drm/i915: Program gen3- watermarks atomically
drm/i915: Convert pineview watermarks to atomic, v2.
drm/i915: Calculate gen4 watermarks semiatomically.
drm/i915: Program gen4 watermarks atomically
drm/i915: Kill off intel_crtc_active.
drm/i915: Rip out legacy watermark infrastructure
drivers/gpu/drm/i915/i915_drv.h | 6 +-
drivers/gpu/drm/i915/intel_atomic.c | 2 -
drivers/gpu/drm/i915/intel_display.c | 96 +-----
drivers/gpu/drm/i915/intel_drv.h | 18 +-
drivers/gpu/drm/i915/intel_fbc.c | 2 +-
drivers/gpu/drm/i915/intel_pm.c | 639 ++++++++++++++++++++++-------------
6 files changed, 435 insertions(+), 328 deletions(-)
--
2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/7] drm/i915: Calculate gen3- watermarks semi-atomically.
2017-08-07 10:48 [PATCH 0/7] drm/i915: Convert gen4- watermarks to atomic Maarten Lankhorst
@ 2017-08-07 10:48 ` Maarten Lankhorst
2017-08-08 11:00 ` [PATCH] drm/i915: Calculate gen3- watermarks semi-atomically, v2 Maarten Lankhorst
2017-08-08 12:51 ` [PATCH 1/7] drm/i915: Calculate gen3- watermarks semi-atomically Mahesh Kumar
2017-08-07 10:48 ` [PATCH 2/7] drm/i915: Program gen3- watermarks atomically Maarten Lankhorst
` (7 subsequent siblings)
8 siblings, 2 replies; 14+ messages in thread
From: Maarten Lankhorst @ 2017-08-07 10:48 UTC (permalink / raw)
To: intel-gfx
The gen3 watermark calculations are converted to atomic,
but the wm update calls are still done through the legacy
functions.
This will make it easier to bisect things if they go wrong.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 3 +-
drivers/gpu/drm/i915/intel_drv.h | 14 +++
drivers/gpu/drm/i915/intel_pm.c | 231 +++++++++++++++++++++--------------
3 files changed, 152 insertions(+), 96 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 974d4b577189..234b94cafc7d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14709,7 +14709,8 @@ intel_modeset_setup_hw_state(struct drm_device *dev,
skl_wm_get_hw_state(dev);
} else if (HAS_PCH_SPLIT(dev_priv)) {
ilk_wm_get_hw_state(dev);
- }
+ } else if (INTEL_GEN(dev_priv) <= 3 && !IS_PINEVIEW(dev_priv))
+ i9xx_wm_get_hw_state(dev);
for_each_intel_crtc(dev, crtc) {
u64 put_domains;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f91de9cb9697..d53d34756048 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -547,6 +547,15 @@ struct g4x_wm_state {
bool fbc_en;
};
+struct i9xx_wm_state {
+ uint16_t plane_wm;
+ bool cxsr;
+
+ struct {
+ uint16_t plane;
+ } sr;
+};
+
struct intel_crtc_wm_state {
union {
struct {
@@ -591,6 +600,9 @@ struct intel_crtc_wm_state {
/* optimal watermarks */
struct g4x_wm_state optimal;
} g4x;
+ struct {
+ struct i9xx_wm_state optimal;
+ } i9xx;
};
/*
@@ -823,6 +835,7 @@ struct intel_crtc {
struct intel_pipe_wm ilk;
struct vlv_wm_state vlv;
struct g4x_wm_state g4x;
+ struct i9xx_wm_state i9xx;
} active;
} wm;
@@ -1845,6 +1858,7 @@ void gen6_rps_boost(struct drm_i915_gem_request *rq,
struct intel_rps_client *rps);
void intel_queue_rps_boost_for_request(struct drm_i915_gem_request *req);
void g4x_wm_get_hw_state(struct drm_device *dev);
+void i9xx_wm_get_hw_state(struct drm_device *dev);
void vlv_wm_get_hw_state(struct drm_device *dev);
void ilk_wm_get_hw_state(struct drm_device *dev);
void skl_wm_get_hw_state(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 6e393b217450..807c9a730020 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2224,89 +2224,154 @@ static void i965_update_wm(struct intel_crtc *unused_crtc)
#undef FW_WM
-static void i9xx_update_wm(struct intel_crtc *unused_crtc)
+static const struct intel_watermark_params *i9xx_get_wm_info(struct drm_i915_private *dev_priv,
+ struct intel_crtc *crtc)
{
- struct drm_i915_private *dev_priv = to_i915(unused_crtc->base.dev);
- const struct intel_watermark_params *wm_info;
- uint32_t fwater_lo;
- uint32_t fwater_hi;
- int cwm, srwm = 1;
- int fifo_size;
- int planea_wm, planeb_wm;
- struct intel_crtc *crtc, *enabled = NULL;
+ struct intel_plane *plane = to_intel_plane(crtc->base.primary);
if (IS_I945GM(dev_priv))
- wm_info = &i945_wm_info;
+ return &i945_wm_info;
else if (!IS_GEN2(dev_priv))
- wm_info = &i915_wm_info;
+ return &i915_wm_info;
+ else if (plane->plane == PLANE_A)
+ return &i830_a_wm_info;
else
- wm_info = &i830_a_wm_info;
+ return &i830_bc_wm_info;
+}
- fifo_size = dev_priv->display.get_fifo_size(dev_priv, 0);
- crtc = intel_get_crtc_for_plane(dev_priv, 0);
- if (intel_crtc_active(crtc)) {
+static int i9xx_compute_pipe_wm(struct intel_crtc_state *crtc_state)
+{
+ struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+ struct intel_atomic_state *state =
+ to_intel_atomic_state(crtc_state->base.state);
+ struct i9xx_wm_state *wm_state = &crtc_state->wm.i9xx.optimal;
+ struct intel_plane *plane = to_intel_plane(crtc->base.primary);
+ const struct drm_plane_state *plane_state = NULL;
+ int fifo_size;
+ const struct intel_watermark_params *wm_info;
+
+ fifo_size = dev_priv->display.get_fifo_size(dev_priv, plane->plane);
+
+ wm_info = i9xx_get_wm_info(dev_priv, crtc);
+
+ wm_state->cxsr = false;
+ memset(&wm_state->sr, 0, sizeof(wm_state->sr));
+
+ if (crtc_state->base.plane_mask & BIT(drm_plane_index(&plane->base)))
+ plane_state = __drm_atomic_get_current_plane_state(&state->base, &plane->base);
+
+ if (!plane_state || !intel_wm_plane_visible(crtc_state, to_intel_plane_state(plane_state))) {
+ wm_state->plane_wm = fifo_size - wm_info->guard_size;
+ if (wm_state->plane_wm > (long)wm_info->max_wm)
+ wm_state->plane_wm = wm_info->max_wm;
+ } else if (HAS_FW_BLC(dev_priv)) {
+ struct drm_framebuffer *fb = plane_state->fb;
+ unsigned cpp = IS_GEN2(dev_priv) ? 4 : fb->format->cpp[0];
const struct drm_display_mode *adjusted_mode =
- &crtc->config->base.adjusted_mode;
- const struct drm_framebuffer *fb =
- crtc->base.primary->state->fb;
- int cpp;
+ &crtc_state->base.adjusted_mode;
+ unsigned active_crtcs;
+ bool may_cxsr;
- if (IS_GEN2(dev_priv))
- cpp = 4;
+ if (state->modeset)
+ active_crtcs = state->active_crtcs;
else
- cpp = fb->format->cpp[0];
+ active_crtcs = dev_priv->active_crtcs;
- planea_wm = intel_calculate_wm(adjusted_mode->crtc_clock,
- wm_info, fifo_size, cpp,
- pessimal_latency_ns);
- enabled = crtc;
- } else {
- planea_wm = fifo_size - wm_info->guard_size;
- if (planea_wm > (long)wm_info->max_wm)
- planea_wm = wm_info->max_wm;
+ may_cxsr = active_crtcs == drm_crtc_mask(&crtc->base);
+
+ wm_state->plane_wm = intel_calculate_wm(adjusted_mode->crtc_clock,
+ wm_info, fifo_size, cpp,
+ pessimal_latency_ns);
+
+ DRM_DEBUG_KMS("FIFO watermarks - plane watermarks: %d\n", wm_state->plane_wm);
+
+ if (IS_I915GM(dev_priv) && i915_gem_object_is_tiled(intel_fb_obj(fb)))
+ may_cxsr = false;
+
+ if (may_cxsr) {
+ static const int sr_latency_ns = 6000;
+ unsigned long entries;
+
+ entries = intel_wm_method2(adjusted_mode->crtc_clock,
+ adjusted_mode->crtc_htotal,
+ crtc_state->pipe_src_w, cpp,
+ sr_latency_ns / 100);
+ entries = DIV_ROUND_UP(entries, wm_info->cacheline_size);
+
+ DRM_DEBUG_KMS("self-refresh entries: %ld\n", entries);
+
+ if (wm_info->fifo_size >= entries) {
+ wm_state->cxsr = true;
+ wm_state->sr.plane = wm_info->fifo_size - entries;
+ } else
+ may_cxsr = false;
+ }
+
+ DRM_DEBUG_KMS("FIFO watermarks - plane watermarks: %d, can cxsr: %s, SR size: %d\n",
+ wm_state->plane_wm, yesno(wm_state->cxsr), wm_state->sr.plane);
}
- if (IS_GEN2(dev_priv))
- wm_info = &i830_bc_wm_info;
+ return 0;
+}
- fifo_size = dev_priv->display.get_fifo_size(dev_priv, 1);
- crtc = intel_get_crtc_for_plane(dev_priv, 1);
- if (intel_crtc_active(crtc)) {
- const struct drm_display_mode *adjusted_mode =
- &crtc->config->base.adjusted_mode;
- const struct drm_framebuffer *fb =
- crtc->base.primary->state->fb;
- int cpp;
+void i9xx_wm_get_hw_state(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = to_i915(dev);
+ struct intel_crtc *crtc;
+ uint32_t fwater_lo, planea_wm, planeb_wm;
+
+ fwater_lo = I915_READ(FW_BLC);
+
+ planea_wm = fwater_lo & 0x3f;
+ planeb_wm = (fwater_lo >> 16) & 0x3f;
+
+ for_each_intel_crtc(dev, crtc) {
+ struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->base.state);
+ struct i9xx_wm_state *wm_state = &cstate->wm.i9xx.optimal;
+ struct intel_plane *plane = to_intel_plane(crtc->base.primary);
+
+ memset(&wm_state->sr, 0, sizeof(wm_state->sr));
+ wm_state->cxsr = false;
- if (IS_GEN2(dev_priv))
- cpp = 4;
+ if (plane == PLANE_A)
+ wm_state->plane_wm = planea_wm;
else
- cpp = fb->format->cpp[0];
+ wm_state->plane_wm = planeb_wm;
+
+ crtc->wm.active.i9xx = *wm_state;
+ }
+}
- planeb_wm = intel_calculate_wm(adjusted_mode->crtc_clock,
- wm_info, fifo_size, cpp,
- pessimal_latency_ns);
+static void i9xx_update_wm(struct intel_crtc *crtc)
+{
+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+ uint32_t fwater_lo;
+ uint32_t fwater_hi;
+ int cwm, srwm = -1;
+ int planea_wm, planeb_wm;
+ struct intel_crtc *enabled = NULL;
+
+ crtc->wm.active.i9xx = crtc->config->wm.i9xx.optimal;
+
+ crtc = intel_get_crtc_for_plane(dev_priv, 0);
+ planea_wm = crtc->wm.active.i9xx.plane_wm;
+ if (intel_crtc_active(crtc))
+ enabled = crtc;
+
+ crtc = intel_get_crtc_for_plane(dev_priv, 1);
+ if (intel_crtc_active(crtc)) {
if (enabled == NULL)
enabled = crtc;
else
enabled = NULL;
- } else {
- planeb_wm = fifo_size - wm_info->guard_size;
- if (planeb_wm > (long)wm_info->max_wm)
- planeb_wm = wm_info->max_wm;
}
+ planeb_wm = crtc->wm.active.i9xx.plane_wm;
DRM_DEBUG_KMS("FIFO watermarks - A: %d, B: %d\n", planea_wm, planeb_wm);
- if (IS_I915GM(dev_priv) && enabled) {
- struct drm_i915_gem_object *obj;
-
- obj = intel_fb_obj(enabled->base.primary->state->fb);
-
- /* self-refresh seems busted with untiled */
- if (!i915_gem_object_is_tiled(obj))
- enabled = NULL;
- }
+ if (!enabled->wm.active.i9xx.cxsr)
+ enabled = NULL;
/*
* Overlay gets an aggressive default since video jitter is bad.
@@ -2317,31 +2382,8 @@ static void i9xx_update_wm(struct intel_crtc *unused_crtc)
intel_set_memory_cxsr(dev_priv, false);
/* Calc sr entries for one plane configs */
- if (HAS_FW_BLC(dev_priv) && enabled) {
- /* self-refresh has much higher latency */
- static const int sr_latency_ns = 6000;
- const struct drm_display_mode *adjusted_mode =
- &enabled->config->base.adjusted_mode;
- const struct drm_framebuffer *fb =
- enabled->base.primary->state->fb;
- int clock = adjusted_mode->crtc_clock;
- int htotal = adjusted_mode->crtc_htotal;
- int hdisplay = enabled->config->pipe_src_w;
- int cpp;
- int entries;
-
- if (IS_I915GM(dev_priv) || IS_I945GM(dev_priv))
- cpp = 4;
- else
- cpp = fb->format->cpp[0];
-
- entries = intel_wm_method2(clock, htotal, hdisplay, cpp,
- sr_latency_ns / 100);
- entries = DIV_ROUND_UP(entries, wm_info->cacheline_size);
- DRM_DEBUG_KMS("self-refresh entries: %d\n", entries);
- srwm = wm_info->fifo_size - entries;
- if (srwm < 0)
- srwm = 1;
+ if (enabled) {
+ srwm = enabled->wm.active.i9xx.sr.plane;
if (IS_I945G(dev_priv) || IS_I945GM(dev_priv))
I915_WRITE(FW_BLC_SELF,
@@ -2367,23 +2409,18 @@ static void i9xx_update_wm(struct intel_crtc *unused_crtc)
intel_set_memory_cxsr(dev_priv, true);
}
-static void i845_update_wm(struct intel_crtc *unused_crtc)
+static void i845_update_wm(struct intel_crtc *crtc)
{
- struct drm_i915_private *dev_priv = to_i915(unused_crtc->base.dev);
- struct intel_crtc *crtc;
- const struct drm_display_mode *adjusted_mode;
+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
uint32_t fwater_lo;
int planea_wm;
- crtc = single_enabled_crtc(dev_priv);
- if (crtc == NULL)
+ if (!intel_crtc_active(crtc))
return;
- adjusted_mode = &crtc->config->base.adjusted_mode;
- planea_wm = intel_calculate_wm(adjusted_mode->crtc_clock,
- &i845_wm_info,
- dev_priv->display.get_fifo_size(dev_priv, 0),
- 4, pessimal_latency_ns);
+ crtc->wm.active.i9xx = crtc->config->wm.i9xx.optimal;
+ planea_wm = crtc->wm.active.i9xx.plane_wm;
+
fwater_lo = I915_READ(FW_BLC) & ~0xfff;
fwater_lo |= (3<<8) | planea_wm;
@@ -8813,9 +8850,13 @@ void intel_init_pm(struct drm_i915_private *dev_priv)
} else if (IS_GEN4(dev_priv)) {
dev_priv->display.update_wm = i965_update_wm;
} else if (IS_GEN3(dev_priv)) {
+ dev_priv->display.compute_pipe_wm = i9xx_compute_pipe_wm;
dev_priv->display.update_wm = i9xx_update_wm;
+
dev_priv->display.get_fifo_size = i9xx_get_fifo_size;
} else if (IS_GEN2(dev_priv)) {
+ dev_priv->display.compute_pipe_wm = i9xx_compute_pipe_wm;
+
if (INTEL_INFO(dev_priv)->num_pipes == 1) {
dev_priv->display.update_wm = i845_update_wm;
dev_priv->display.get_fifo_size = i845_get_fifo_size;
--
2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/7] drm/i915: Program gen3- watermarks atomically
2017-08-07 10:48 [PATCH 0/7] drm/i915: Convert gen4- watermarks to atomic Maarten Lankhorst
2017-08-07 10:48 ` [PATCH 1/7] drm/i915: Calculate gen3- watermarks semi-atomically Maarten Lankhorst
@ 2017-08-07 10:48 ` Maarten Lankhorst
2017-08-07 10:48 ` [PATCH 3/7] drm/i915: Convert pineview watermarks to atomic, v2 Maarten Lankhorst
` (6 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Maarten Lankhorst @ 2017-08-07 10:48 UTC (permalink / raw)
To: intel-gfx
With the atomic watermark calculations calculate intermediary watermark
values and update the watermarks atomically.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 5 ++
drivers/gpu/drm/i915/intel_drv.h | 2 +-
drivers/gpu/drm/i915/intel_pm.c | 103 +++++++++++++++++++++++++++++++++------
3 files changed, 95 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 907603cba447..3ac17f0f4053 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1774,6 +1774,10 @@ struct g4x_wm_values {
bool fbc_en;
};
+struct i9xx_wm_values {
+ bool cxsr;
+};
+
struct skl_ddb_entry {
uint16_t start, end; /* in number of blocks, 'end' is exclusive */
};
@@ -2438,6 +2442,7 @@ struct drm_i915_private {
struct skl_wm_values skl_hw;
struct vlv_wm_values vlv;
struct g4x_wm_values g4x;
+ struct i9xx_wm_values i9xx;
};
uint8_t max_level;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d53d34756048..176274d99ee6 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -601,7 +601,7 @@ struct intel_crtc_wm_state {
struct g4x_wm_state optimal;
} g4x;
struct {
- struct i9xx_wm_state optimal;
+ struct i9xx_wm_state optimal, intermediate;
} i9xx;
};
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 807c9a730020..08fd359307e6 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -439,6 +439,8 @@ bool intel_set_memory_cxsr(struct drm_i915_private *dev_priv, bool enable)
dev_priv->wm.vlv.cxsr = enable;
else if (IS_G4X(dev_priv))
dev_priv->wm.g4x.cxsr = enable;
+ else if (INTEL_GEN(dev_priv) <= 4)
+ dev_priv->wm.i9xx.cxsr = enable;
mutex_unlock(&dev_priv->wm.wm_mutex);
return ret;
@@ -2315,6 +2317,44 @@ static int i9xx_compute_pipe_wm(struct intel_crtc_state *crtc_state)
return 0;
}
+static int i9xx_compute_intermediate_wm(struct drm_device *dev,
+ struct intel_crtc *intel_crtc,
+ struct intel_crtc_state *newstate)
+{
+ struct i9xx_wm_state *intermediate = &newstate->wm.i9xx.intermediate;
+ const struct drm_crtc_state *old_drm_state =
+ drm_atomic_get_old_crtc_state(newstate->base.state, &intel_crtc->base);
+ const struct i9xx_wm_state *old = &to_intel_crtc_state(old_drm_state)->wm.i9xx.optimal;
+ const struct i9xx_wm_state *optimal = &newstate->wm.i9xx.optimal;
+
+ /*
+ * Start with the final, target watermarks, then combine with the
+ * currently active watermarks to get values that are safe both before
+ * and after the vblank.
+ */
+ *intermediate = *optimal;
+ if (newstate->disable_cxsr)
+ intermediate->cxsr = false;
+
+ if (!newstate->base.active ||
+ drm_atomic_crtc_needs_modeset(&newstate->base))
+ goto out;
+
+ intermediate->plane_wm = min(old->plane_wm, optimal->plane_wm);
+ intermediate->sr.plane = min(old->sr.plane, optimal->sr.plane);
+
+out:
+ /*
+ * If our intermediate WM are identical to the final WM, then we can
+ * omit the post-vblank programming; only update if it's different.
+ */
+ if (newstate->base.active &&
+ memcmp(intermediate, optimal, sizeof(*intermediate)) != 0)
+ newstate->wm.need_postvbl_update = true;
+
+ return 0;
+}
+
void i9xx_wm_get_hw_state(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = to_i915(dev);
@@ -2343,17 +2383,15 @@ void i9xx_wm_get_hw_state(struct drm_device *dev)
}
}
-static void i9xx_update_wm(struct intel_crtc *crtc)
+static void i9xx_program_watermarks(struct drm_i915_private *dev_priv)
{
- struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+ struct intel_crtc *crtc;
uint32_t fwater_lo;
uint32_t fwater_hi;
int cwm, srwm = -1;
int planea_wm, planeb_wm;
struct intel_crtc *enabled = NULL;
- crtc->wm.active.i9xx = crtc->config->wm.i9xx.optimal;
-
crtc = intel_get_crtc_for_plane(dev_priv, 0);
planea_wm = crtc->wm.active.i9xx.plane_wm;
if (intel_crtc_active(crtc))
@@ -2379,7 +2417,7 @@ static void i9xx_update_wm(struct intel_crtc *crtc)
cwm = 2;
/* Play safe and disable self-refresh before adjusting watermarks. */
- intel_set_memory_cxsr(dev_priv, false);
+ _intel_set_memory_cxsr(dev_priv, false);
/* Calc sr entries for one plane configs */
if (enabled) {
@@ -2406,19 +2444,17 @@ static void i9xx_update_wm(struct intel_crtc *crtc)
I915_WRITE(FW_BLC2, fwater_hi);
if (enabled)
- intel_set_memory_cxsr(dev_priv, true);
+ _intel_set_memory_cxsr(dev_priv, true);
+
+ dev_priv->wm.i9xx.cxsr = enabled;
}
-static void i845_update_wm(struct intel_crtc *crtc)
+static void i845_program_watermarks(struct intel_crtc *crtc)
{
struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
uint32_t fwater_lo;
int planea_wm;
- if (!intel_crtc_active(crtc))
- return;
-
- crtc->wm.active.i9xx = crtc->config->wm.i9xx.optimal;
planea_wm = crtc->wm.active.i9xx.plane_wm;
fwater_lo = I915_READ(FW_BLC) & ~0xfff;
@@ -2429,6 +2465,41 @@ static void i845_update_wm(struct intel_crtc *crtc)
I915_WRITE(FW_BLC, fwater_lo);
}
+
+static void i9xx_initial_watermarks(struct intel_atomic_state *state,
+ struct intel_crtc_state *crtc_state)
+{
+ struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
+ struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
+
+ mutex_lock(&dev_priv->wm.wm_mutex);
+ crtc->wm.active.i9xx = crtc_state->wm.i9xx.intermediate;
+ if (INTEL_INFO(dev_priv)->num_pipes == 1)
+ i845_program_watermarks(intel_crtc);
+ else
+ i9xx_program_watermarks(dev_priv);
+ mutex_unlock(&dev_priv->wm.wm_mutex);
+}
+
+static void i9xx_optimize_watermarks(struct intel_atomic_state *state,
+ struct intel_crtc_state *crtc_state)
+{
+ struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
+
+ if (!crtc_state->wm.need_postvbl_update)
+ return;
+
+ mutex_lock(&dev_priv->wm.wm_mutex);
+ intel_crtc->wm.active.i9xx = crtc_state->wm.i9xx.optimal;
+ if (INTEL_INFO(dev_priv)->num_pipes == 1)
+ i845_program_watermarks(intel_crtc);
+ else
+ i9xx_program_watermarks(dev_priv);
+ mutex_unlock(&dev_priv->wm.wm_mutex);
+}
+
/* latency must be in 0.1us units. */
static unsigned int ilk_wm_method1(unsigned int pixel_rate,
unsigned int cpp,
@@ -8851,17 +8922,21 @@ void intel_init_pm(struct drm_i915_private *dev_priv)
dev_priv->display.update_wm = i965_update_wm;
} else if (IS_GEN3(dev_priv)) {
dev_priv->display.compute_pipe_wm = i9xx_compute_pipe_wm;
- dev_priv->display.update_wm = i9xx_update_wm;
+ dev_priv->display.compute_intermediate_wm = i9xx_compute_intermediate_wm;
+
+ dev_priv->display.initial_watermarks = i9xx_initial_watermarks;
+ dev_priv->display.optimize_watermarks = i9xx_optimize_watermarks;
dev_priv->display.get_fifo_size = i9xx_get_fifo_size;
} else if (IS_GEN2(dev_priv)) {
dev_priv->display.compute_pipe_wm = i9xx_compute_pipe_wm;
+ dev_priv->display.compute_intermediate_wm = i9xx_compute_intermediate_wm;
+ dev_priv->display.initial_watermarks = i9xx_initial_watermarks;
+ dev_priv->display.optimize_watermarks = i9xx_optimize_watermarks;
if (INTEL_INFO(dev_priv)->num_pipes == 1) {
- dev_priv->display.update_wm = i845_update_wm;
dev_priv->display.get_fifo_size = i845_get_fifo_size;
} else {
- dev_priv->display.update_wm = i9xx_update_wm;
dev_priv->display.get_fifo_size = i830_get_fifo_size;
}
} else {
--
2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/7] drm/i915: Convert pineview watermarks to atomic, v2.
2017-08-07 10:48 [PATCH 0/7] drm/i915: Convert gen4- watermarks to atomic Maarten Lankhorst
2017-08-07 10:48 ` [PATCH 1/7] drm/i915: Calculate gen3- watermarks semi-atomically Maarten Lankhorst
2017-08-07 10:48 ` [PATCH 2/7] drm/i915: Program gen3- watermarks atomically Maarten Lankhorst
@ 2017-08-07 10:48 ` Maarten Lankhorst
2017-08-07 10:48 ` [PATCH 4/7] drm/i915: Calculate gen4 watermarks semiatomically Maarten Lankhorst
` (5 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Maarten Lankhorst @ 2017-08-07 10:48 UTC (permalink / raw)
To: intel-gfx
Pineview seems to have different watermarks from the other
platforms and are calculated separately.
With the change to atomic, cxsr is automatically disabled for
intermediate watermarks when required. This will fix
fd.org bug #101597 as a nice side effect.
Changes since v1:
- Fix deadlock in pnv_program_watermarks.
- Add link to bug 101597
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101597
---
drivers/gpu/drm/i915/intel_drv.h | 3 +-
drivers/gpu/drm/i915/intel_pm.c | 138 ++++++++++++++++++++++++++-------------
2 files changed, 94 insertions(+), 47 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 176274d99ee6..d0f1704e1a17 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -553,7 +553,8 @@ struct i9xx_wm_state {
struct {
uint16_t plane;
- } sr;
+ uint16_t cursor;
+ } sr, hpll;
};
struct intel_crtc_wm_state {
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 08fd359307e6..7ecf39815ab5 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -830,13 +830,17 @@ static struct intel_crtc *single_enabled_crtc(struct drm_i915_private *dev_priv)
return enabled;
}
-static void pineview_update_wm(struct intel_crtc *unused_crtc)
+static int pnv_compute_pipe_wm(struct intel_crtc_state *crtc_state)
{
- struct drm_i915_private *dev_priv = to_i915(unused_crtc->base.dev);
- struct intel_crtc *crtc;
+ struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+ struct i9xx_wm_state *wm_state = &crtc_state->wm.i9xx.optimal;
+ struct intel_plane *plane = to_intel_plane(crtc->base.primary);
+ struct intel_atomic_state *state = to_intel_atomic_state(crtc_state->base.state);
+ const struct drm_plane_state *primary_plane_state = NULL;
const struct cxsr_latency *latency;
- u32 reg;
- unsigned int wm;
+
+ memset(wm_state, 0, sizeof(*wm_state));
latency = intel_get_cxsr_latency(IS_PINEVIEW_G(dev_priv),
dev_priv->is_ddr3,
@@ -844,60 +848,90 @@ static void pineview_update_wm(struct intel_crtc *unused_crtc)
dev_priv->mem_freq);
if (!latency) {
DRM_DEBUG_KMS("Unknown FSB/MEM found, disable CxSR\n");
- intel_set_memory_cxsr(dev_priv, false);
- return;
+
+ return 0;
}
- crtc = single_enabled_crtc(dev_priv);
- if (crtc) {
- const struct drm_display_mode *adjusted_mode =
- &crtc->config->base.adjusted_mode;
+ if (crtc_state->base.plane_mask & BIT(drm_plane_index(&plane->base)))
+ primary_plane_state = __drm_atomic_get_current_plane_state(&state->base, &plane->base);
+
+ if (primary_plane_state) {
const struct drm_framebuffer *fb =
- crtc->base.primary->state->fb;
+ primary_plane_state->fb;
int cpp = fb->format->cpp[0];
- int clock = adjusted_mode->crtc_clock;
+ const struct drm_display_mode *adjusted_mode =
+ &crtc_state->base.adjusted_mode;
+ unsigned active_crtcs;
+
+ if (state->modeset)
+ active_crtcs = state->active_crtcs;
+ else
+ active_crtcs = dev_priv->active_crtcs;
+
+ wm_state->cxsr = active_crtcs == drm_crtc_mask(&crtc->base);
+
+ wm_state->sr.plane = intel_calculate_wm(adjusted_mode->crtc_clock,
+ &pineview_display_wm,
+ pineview_display_wm.fifo_size,
+ cpp, latency->display_sr);
+
+ wm_state->sr.cursor = intel_calculate_wm(adjusted_mode->crtc_clock,
+ &pineview_cursor_wm,
+ pineview_display_wm.fifo_size,
+ 4, latency->cursor_sr);
+
+ wm_state->hpll.plane = intel_calculate_wm(adjusted_mode->crtc_clock,
+ &pineview_display_hplloff_wm,
+ pineview_display_hplloff_wm.fifo_size,
+ cpp, latency->display_hpll_disable);
+
+ wm_state->hpll.cursor = intel_calculate_wm(adjusted_mode->crtc_clock,
+ &pineview_cursor_hplloff_wm,
+ pineview_display_hplloff_wm.fifo_size,
+ 4, latency->cursor_hpll_disable);
+
+ DRM_DEBUG_KMS("FIFO watermarks - can cxsr: %s, display plane %d, cursor SR size: %d\n",
+ yesno(wm_state->cxsr), wm_state->sr.plane, wm_state->sr.cursor);
+ } else
+ wm_state->cxsr = false;
+
+ return 0;
+}
+
+static void pnv_program_watermarks(struct drm_i915_private *dev_priv)
+{
+ struct intel_crtc *crtc;
+ struct i9xx_wm_state *wm_state = NULL;
+
+ crtc = single_enabled_crtc(dev_priv);
+ if (crtc)
+ wm_state = &crtc->wm.active.i9xx;
+
+ if (wm_state && wm_state->cxsr) {
+ u32 reg;
/* Display SR */
- wm = intel_calculate_wm(clock, &pineview_display_wm,
- pineview_display_wm.fifo_size,
- cpp, latency->display_sr);
reg = I915_READ(DSPFW1);
reg &= ~DSPFW_SR_MASK;
- reg |= FW_WM(wm, SR);
+ reg |= FW_WM(wm_state->sr.plane, SR);
I915_WRITE(DSPFW1, reg);
DRM_DEBUG_KMS("DSPFW1 register is %x\n", reg);
/* cursor SR */
- wm = intel_calculate_wm(clock, &pineview_cursor_wm,
- pineview_display_wm.fifo_size,
- 4, latency->cursor_sr);
reg = I915_READ(DSPFW3);
- reg &= ~DSPFW_CURSOR_SR_MASK;
- reg |= FW_WM(wm, CURSOR_SR);
+ reg &= ~(DSPFW_CURSOR_SR_MASK | DSPFW_HPLL_SR_MASK | DSPFW_HPLL_CURSOR_MASK);
+ reg |= FW_WM(wm_state->sr.cursor, CURSOR_SR);
+ reg |= FW_WM(wm_state->hpll.plane, HPLL_SR);
+ reg |= FW_WM(wm_state->hpll.cursor, HPLL_SR);
I915_WRITE(DSPFW3, reg);
- /* Display HPLL off SR */
- wm = intel_calculate_wm(clock, &pineview_display_hplloff_wm,
- pineview_display_hplloff_wm.fifo_size,
- cpp, latency->display_hpll_disable);
- reg = I915_READ(DSPFW3);
- reg &= ~DSPFW_HPLL_SR_MASK;
- reg |= FW_WM(wm, HPLL_SR);
- I915_WRITE(DSPFW3, reg);
-
- /* cursor HPLL off SR */
- wm = intel_calculate_wm(clock, &pineview_cursor_hplloff_wm,
- pineview_display_hplloff_wm.fifo_size,
- 4, latency->cursor_hpll_disable);
- reg = I915_READ(DSPFW3);
- reg &= ~DSPFW_HPLL_CURSOR_MASK;
- reg |= FW_WM(wm, HPLL_CURSOR);
- I915_WRITE(DSPFW3, reg);
DRM_DEBUG_KMS("DSPFW3 register is %x\n", reg);
- intel_set_memory_cxsr(dev_priv, true);
+ _intel_set_memory_cxsr(dev_priv, true);
+ dev_priv->wm.i9xx.cxsr = true;
} else {
- intel_set_memory_cxsr(dev_priv, false);
+ _intel_set_memory_cxsr(dev_priv, false);
+ dev_priv->wm.i9xx.cxsr = false;
}
}
@@ -2342,6 +2376,9 @@ static int i9xx_compute_intermediate_wm(struct drm_device *dev,
intermediate->plane_wm = min(old->plane_wm, optimal->plane_wm);
intermediate->sr.plane = min(old->sr.plane, optimal->sr.plane);
+ intermediate->sr.cursor = min(old->sr.cursor, optimal->sr.cursor);
+ intermediate->hpll.plane = min(old->hpll.plane, optimal->hpll.plane);
+ intermediate->hpll.cursor = min(old->hpll.cursor, optimal->hpll.cursor);
out:
/*
@@ -2475,7 +2512,9 @@ static void i9xx_initial_watermarks(struct intel_atomic_state *state,
mutex_lock(&dev_priv->wm.wm_mutex);
crtc->wm.active.i9xx = crtc_state->wm.i9xx.intermediate;
- if (INTEL_INFO(dev_priv)->num_pipes == 1)
+ if (IS_PINEVIEW(dev_priv))
+ pnv_program_watermarks(dev_priv);
+ else if (INTEL_INFO(dev_priv)->num_pipes == 1)
i845_program_watermarks(intel_crtc);
else
i9xx_program_watermarks(dev_priv);
@@ -2493,7 +2532,9 @@ static void i9xx_optimize_watermarks(struct intel_atomic_state *state,
mutex_lock(&dev_priv->wm.wm_mutex);
intel_crtc->wm.active.i9xx = crtc_state->wm.i9xx.optimal;
- if (INTEL_INFO(dev_priv)->num_pipes == 1)
+ if (IS_PINEVIEW(dev_priv))
+ pnv_program_watermarks(dev_priv);
+ else if (INTEL_INFO(dev_priv)->num_pipes == 1)
i845_program_watermarks(intel_crtc);
else
i9xx_program_watermarks(dev_priv);
@@ -8915,9 +8956,14 @@ void intel_init_pm(struct drm_i915_private *dev_priv)
dev_priv->fsb_freq, dev_priv->mem_freq);
/* Disable CxSR and never update its watermark again */
intel_set_memory_cxsr(dev_priv, false);
- dev_priv->display.update_wm = NULL;
- } else
- dev_priv->display.update_wm = pineview_update_wm;
+ dev_priv->display.compute_pipe_wm = NULL;
+ dev_priv->display.initial_watermarks = NULL;
+ dev_priv->display.optimize_watermarks = NULL;
+ } else {
+ dev_priv->display.compute_pipe_wm = pnv_compute_pipe_wm;
+ dev_priv->display.initial_watermarks = i9xx_initial_watermarks;
+ dev_priv->display.optimize_watermarks = i9xx_optimize_watermarks;
+ }
} else if (IS_GEN4(dev_priv)) {
dev_priv->display.update_wm = i965_update_wm;
} else if (IS_GEN3(dev_priv)) {
--
2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 4/7] drm/i915: Calculate gen4 watermarks semiatomically.
2017-08-07 10:48 [PATCH 0/7] drm/i915: Convert gen4- watermarks to atomic Maarten Lankhorst
` (2 preceding siblings ...)
2017-08-07 10:48 ` [PATCH 3/7] drm/i915: Convert pineview watermarks to atomic, v2 Maarten Lankhorst
@ 2017-08-07 10:48 ` Maarten Lankhorst
2017-08-07 10:48 ` [PATCH 5/7] drm/i915: Program gen4 watermarks atomically Maarten Lankhorst
` (4 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Maarten Lankhorst @ 2017-08-07 10:48 UTC (permalink / raw)
To: intel-gfx
Gen4 watermark is handled same as gen3-. Calculate
the optimal watermarks atomically first, and program
it in the legacy helper.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/intel_pm.c | 136 ++++++++++++++++++++++++++++------------
1 file changed, 95 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 7ecf39815ab5..f2062f589f41 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2187,58 +2187,109 @@ static void vlv_optimize_watermarks(struct intel_atomic_state *state,
mutex_unlock(&dev_priv->wm.wm_mutex);
}
-static void i965_update_wm(struct intel_crtc *unused_crtc)
+static int i965_compute_pipe_wm(struct intel_crtc_state *crtc_state)
{
- struct drm_i915_private *dev_priv = to_i915(unused_crtc->base.dev);
- struct intel_crtc *crtc;
+ struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+ struct intel_atomic_state *state =
+ to_intel_atomic_state(crtc_state->base.state);
+ struct i9xx_wm_state *wm_state = &crtc_state->wm.i9xx.optimal;
+ struct intel_plane *plane = to_intel_plane(crtc->base.primary);
+ const struct drm_plane_state *primary_plane_state = NULL;
+ const struct drm_plane_state *cursor_plane_state = NULL;
+
+ memset(wm_state, 0, sizeof(*wm_state));
+
+ if (crtc_state->base.plane_mask & BIT(drm_plane_index(&plane->base)))
+ primary_plane_state = __drm_atomic_get_current_plane_state(&state->base, &plane->base);
+
+ if (crtc_state->base.plane_mask & BIT(drm_plane_index(crtc->base.cursor)))
+ cursor_plane_state = __drm_atomic_get_current_plane_state(&state->base, crtc->base.cursor);
+
+ if (primary_plane_state) {
+ static const int sr_latency_ns = 12000;
+ const struct drm_display_mode *adjusted_mode =
+ &crtc_state->base.adjusted_mode;
+ unsigned active_crtcs;
+ unsigned long entries;
+ bool may_cxsr;
+
+ if (state->modeset)
+ active_crtcs = state->active_crtcs;
+ else
+ active_crtcs = dev_priv->active_crtcs;
+
+ may_cxsr = active_crtcs == drm_crtc_mask(&crtc->base);
+
+ if (may_cxsr && intel_wm_plane_visible(crtc_state, to_intel_plane_state(primary_plane_state))) {
+ struct drm_framebuffer *fb = primary_plane_state->fb;
+ unsigned cpp = fb->format->cpp[0];
+
+ entries = intel_wm_method2(adjusted_mode->crtc_clock,
+ adjusted_mode->crtc_htotal,
+ crtc_state->pipe_src_w, cpp,
+ sr_latency_ns / 100);
+ entries = DIV_ROUND_UP(entries, I915_FIFO_LINE_SIZE);
+ if (entries < I965_FIFO_SIZE)
+ wm_state->sr.plane = I965_FIFO_SIZE - entries;
+ else
+ may_cxsr = false;
+
+ DRM_DEBUG_KMS("self-refresh entries: %ld\n", entries);
+ }
+
+ /* No need to use intel_wm_plane_visible here, since cursor. */
+ if (may_cxsr && cursor_plane_state && crtc_state->base.active) {
+ entries = intel_wm_method2(adjusted_mode->crtc_clock,
+ adjusted_mode->crtc_htotal,
+ cursor_plane_state->crtc_w, 4,
+ sr_latency_ns / 100);
+
+ entries = DIV_ROUND_UP(entries,
+ i965_cursor_wm_info.cacheline_size) +
+ i965_cursor_wm_info.guard_size;
+
+ if (entries < i965_cursor_wm_info.fifo_size)
+ wm_state->sr.cursor = min(i965_cursor_wm_info.fifo_size - entries,
+ (unsigned long)(i965_cursor_wm_info.max_wm));
+ else
+ may_cxsr = false;
+ } else if (may_cxsr)
+ wm_state->sr.cursor = 16;
+
+ wm_state->cxsr = may_cxsr;
+
+ DRM_DEBUG_KMS("FIFO watermarks - can cxsr: %s, display plane %d, cursor SR size: %d\n",
+ yesno(wm_state->cxsr), wm_state->sr.plane, wm_state->sr.cursor);
+ }
+
+ return 0;
+}
+
+static void i965_update_wm(struct intel_crtc *crtc)
+{
+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
int srwm = 1;
int cursor_sr = 16;
- bool cxsr_enabled;
+ bool cxsr_enabled = false;
+
+ crtc->wm.active.i9xx = crtc->config->wm.i9xx.optimal;
/* Calc sr entries for one plane configs */
crtc = single_enabled_crtc(dev_priv);
- if (crtc) {
- /* self-refresh has much higher latency */
- static const int sr_latency_ns = 12000;
- const struct drm_display_mode *adjusted_mode =
- &crtc->config->base.adjusted_mode;
- const struct drm_framebuffer *fb =
- crtc->base.primary->state->fb;
- int clock = adjusted_mode->crtc_clock;
- int htotal = adjusted_mode->crtc_htotal;
- int hdisplay = crtc->config->pipe_src_w;
- int cpp = fb->format->cpp[0];
- int entries;
-
- entries = intel_wm_method2(clock, htotal,
- hdisplay, cpp, sr_latency_ns / 100);
- entries = DIV_ROUND_UP(entries, I915_FIFO_LINE_SIZE);
- srwm = I965_FIFO_SIZE - entries;
- if (srwm < 0)
- srwm = 1;
- srwm &= 0x1ff;
- DRM_DEBUG_KMS("self-refresh entries: %d, wm: %d\n",
- entries, srwm);
-
- entries = intel_wm_method2(clock, htotal,
- crtc->base.cursor->state->crtc_w, 4,
- sr_latency_ns / 100);
- entries = DIV_ROUND_UP(entries,
- i965_cursor_wm_info.cacheline_size) +
- i965_cursor_wm_info.guard_size;
-
- cursor_sr = i965_cursor_wm_info.fifo_size - entries;
- if (cursor_sr > i965_cursor_wm_info.max_wm)
- cursor_sr = i965_cursor_wm_info.max_wm;
+ if (crtc && crtc->wm.active.i9xx.cxsr) {
+ struct i9xx_wm_state *wm_state = &crtc->wm.active.i9xx;
+
+ srwm = wm_state->sr.plane;
+ cursor_sr = wm_state->sr.cursor;
DRM_DEBUG_KMS("self-refresh watermark: display plane %d "
"cursor %d\n", srwm, cursor_sr);
cxsr_enabled = true;
- } else {
- cxsr_enabled = false;
+ } else if (dev_priv->wm.i9xx.cxsr) {
/* Turn off self refresh if both pipes are enabled */
- intel_set_memory_cxsr(dev_priv, false);
+ _intel_set_memory_cxsr(dev_priv, false);
}
DRM_DEBUG_KMS("Setting FIFO watermarks - A: 8, B: 8, C: 8, SR %d\n",
@@ -2255,7 +2306,9 @@ static void i965_update_wm(struct intel_crtc *unused_crtc)
I915_WRITE(DSPFW3, FW_WM(cursor_sr, CURSOR_SR));
if (cxsr_enabled)
- intel_set_memory_cxsr(dev_priv, true);
+ _intel_set_memory_cxsr(dev_priv, true);
+
+ dev_priv->wm.i9xx.cxsr = cxsr_enabled;
}
#undef FW_WM
@@ -8965,6 +9018,7 @@ void intel_init_pm(struct drm_i915_private *dev_priv)
dev_priv->display.optimize_watermarks = i9xx_optimize_watermarks;
}
} else if (IS_GEN4(dev_priv)) {
+ dev_priv->display.compute_pipe_wm = i965_compute_pipe_wm;
dev_priv->display.update_wm = i965_update_wm;
} else if (IS_GEN3(dev_priv)) {
dev_priv->display.compute_pipe_wm = i9xx_compute_pipe_wm;
--
2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 5/7] drm/i915: Program gen4 watermarks atomically
2017-08-07 10:48 [PATCH 0/7] drm/i915: Convert gen4- watermarks to atomic Maarten Lankhorst
` (3 preceding siblings ...)
2017-08-07 10:48 ` [PATCH 4/7] drm/i915: Calculate gen4 watermarks semiatomically Maarten Lankhorst
@ 2017-08-07 10:48 ` Maarten Lankhorst
2017-08-07 10:48 ` [PATCH 6/7] drm/i915: Kill off intel_crtc_active Maarten Lankhorst
` (3 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Maarten Lankhorst @ 2017-08-07 10:48 UTC (permalink / raw)
To: intel-gfx
We're already calculating the watermarks correctly, now we have to
program them too.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/intel_pm.c | 25 +++++++++++++++----------
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index f2062f589f41..3484ce40f2b0 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2266,20 +2266,20 @@ static int i965_compute_pipe_wm(struct intel_crtc_state *crtc_state)
return 0;
}
-static void i965_update_wm(struct intel_crtc *crtc)
+static void i965_program_watermarks(struct drm_i915_private *dev_priv)
{
- struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+ struct intel_crtc *crtc;
+ struct i9xx_wm_state *wm_state = NULL;
int srwm = 1;
int cursor_sr = 16;
bool cxsr_enabled = false;
- crtc->wm.active.i9xx = crtc->config->wm.i9xx.optimal;
-
- /* Calc sr entries for one plane configs */
crtc = single_enabled_crtc(dev_priv);
- if (crtc && crtc->wm.active.i9xx.cxsr) {
- struct i9xx_wm_state *wm_state = &crtc->wm.active.i9xx;
+ if (crtc)
+ wm_state = &crtc->wm.active.i9xx;
+ /* Calc sr entries for one plane configs */
+ if (wm_state && wm_state->cxsr) {
srwm = wm_state->sr.plane;
cursor_sr = wm_state->sr.cursor;
@@ -2569,8 +2569,10 @@ static void i9xx_initial_watermarks(struct intel_atomic_state *state,
pnv_program_watermarks(dev_priv);
else if (INTEL_INFO(dev_priv)->num_pipes == 1)
i845_program_watermarks(intel_crtc);
- else
+ else if (INTEL_GEN(dev_priv) < 4)
i9xx_program_watermarks(dev_priv);
+ else
+ i965_program_watermarks(dev_priv);
mutex_unlock(&dev_priv->wm.wm_mutex);
}
@@ -2589,8 +2591,10 @@ static void i9xx_optimize_watermarks(struct intel_atomic_state *state,
pnv_program_watermarks(dev_priv);
else if (INTEL_INFO(dev_priv)->num_pipes == 1)
i845_program_watermarks(intel_crtc);
- else
+ else if (INTEL_GEN(dev_priv) < 4)
i9xx_program_watermarks(dev_priv);
+ else
+ i965_program_watermarks(dev_priv);
mutex_unlock(&dev_priv->wm.wm_mutex);
}
@@ -9019,7 +9023,8 @@ void intel_init_pm(struct drm_i915_private *dev_priv)
}
} else if (IS_GEN4(dev_priv)) {
dev_priv->display.compute_pipe_wm = i965_compute_pipe_wm;
- dev_priv->display.update_wm = i965_update_wm;
+ dev_priv->display.initial_watermarks = i9xx_initial_watermarks;
+ dev_priv->display.optimize_watermarks = i9xx_optimize_watermarks;
} else if (IS_GEN3(dev_priv)) {
dev_priv->display.compute_pipe_wm = i9xx_compute_pipe_wm;
dev_priv->display.compute_intermediate_wm = i9xx_compute_intermediate_wm;
--
2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 6/7] drm/i915: Kill off intel_crtc_active.
2017-08-07 10:48 [PATCH 0/7] drm/i915: Convert gen4- watermarks to atomic Maarten Lankhorst
` (4 preceding siblings ...)
2017-08-07 10:48 ` [PATCH 5/7] drm/i915: Program gen4 watermarks atomically Maarten Lankhorst
@ 2017-08-07 10:48 ` Maarten Lankhorst
2017-08-07 10:48 ` [PATCH 7/7] drm/i915: Rip out legacy watermark infrastructure Maarten Lankhorst
` (2 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Maarten Lankhorst @ 2017-08-07 10:48 UTC (permalink / raw)
To: intel-gfx
Use crtc->active directly instead. This is still not completely
optimal and needs fixing, but it's about as good as using
intel_crtc_active.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 19 -------------------
drivers/gpu/drm/i915/intel_drv.h | 1 -
drivers/gpu/drm/i915/intel_fbc.c | 2 +-
drivers/gpu/drm/i915/intel_pm.c | 6 +++---
4 files changed, 4 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 234b94cafc7d..0fa83895b8c4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -944,25 +944,6 @@ bool bxt_find_best_dpll(struct intel_crtc_state *crtc_state, int target_clock,
target_clock, refclk, NULL, best_clock);
}
-bool intel_crtc_active(struct intel_crtc *crtc)
-{
- /* Be paranoid as we can arrive here with only partial
- * state retrieved from the hardware during setup.
- *
- * We can ditch the adjusted_mode.crtc_clock check as soon
- * as Haswell has gained clock readout/fastboot support.
- *
- * We can ditch the crtc->primary->fb check as soon as we can
- * properly reconstruct framebuffers.
- *
- * FIXME: The intel_crtc->active here should be switched to
- * crtc->state->active once we have proper CRTC states wired up
- * for atomic.
- */
- return crtc->active && crtc->base.primary->state->fb &&
- crtc->config->base.adjusted_mode.crtc_clock;
-}
-
enum transcoder intel_pipe_to_cpu_transcoder(struct drm_i915_private *dev_priv,
enum pipe pipe)
{
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d0f1704e1a17..1a70939c69c5 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1464,7 +1464,6 @@ bool bxt_find_best_dpll(struct intel_crtc_state *crtc_state, int target_clock,
struct dpll *best_clock);
int chv_calc_dpll_params(int refclk, struct dpll *pll_clock);
-bool intel_crtc_active(struct intel_crtc *crtc);
void hsw_enable_ips(struct intel_crtc *crtc);
void hsw_disable_ips(struct intel_crtc *crtc);
enum intel_display_power_domain intel_port_to_power_domain(enum port port);
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 860b8c26d29b..da83629fc38f 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -1277,7 +1277,7 @@ void intel_fbc_init_pipe_state(struct drm_i915_private *dev_priv)
return;
for_each_intel_crtc(&dev_priv->drm, crtc)
- if (intel_crtc_active(crtc) &&
+ if (crtc->base.state->active &&
crtc->base.primary->state->visible)
dev_priv->fbc.visible_pipes_mask |= (1 << crtc->pipe);
}
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 3484ce40f2b0..3afaa6933c4b 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -820,7 +820,7 @@ static struct intel_crtc *single_enabled_crtc(struct drm_i915_private *dev_priv)
struct intel_crtc *crtc, *enabled = NULL;
for_each_intel_crtc(&dev_priv->drm, crtc) {
- if (intel_crtc_active(crtc)) {
+ if (crtc->active) {
if (enabled)
return NULL;
enabled = crtc;
@@ -2484,11 +2484,11 @@ static void i9xx_program_watermarks(struct drm_i915_private *dev_priv)
crtc = intel_get_crtc_for_plane(dev_priv, 0);
planea_wm = crtc->wm.active.i9xx.plane_wm;
- if (intel_crtc_active(crtc))
+ if (crtc->active)
enabled = crtc;
crtc = intel_get_crtc_for_plane(dev_priv, 1);
- if (intel_crtc_active(crtc)) {
+ if (crtc->active) {
if (enabled == NULL)
enabled = crtc;
else
--
2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 7/7] drm/i915: Rip out legacy watermark infrastructure
2017-08-07 10:48 [PATCH 0/7] drm/i915: Convert gen4- watermarks to atomic Maarten Lankhorst
` (5 preceding siblings ...)
2017-08-07 10:48 ` [PATCH 6/7] drm/i915: Kill off intel_crtc_active Maarten Lankhorst
@ 2017-08-07 10:48 ` Maarten Lankhorst
2017-08-07 12:17 ` ✗ Fi.CI.BAT: failure for drm/i915: Convert gen4- watermarks to atomic. (rev2) Patchwork
2017-08-08 11:19 ` ✗ Fi.CI.BAT: failure for drm/i915: Convert gen4- watermarks to atomic. (rev3) Patchwork
8 siblings, 0 replies; 14+ messages in thread
From: Maarten Lankhorst @ 2017-08-07 10:48 UTC (permalink / raw)
To: intel-gfx
The legacy watermark infrastructure is now unused, so remove it.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 1 -
drivers/gpu/drm/i915/intel_atomic.c | 2 -
drivers/gpu/drm/i915/intel_display.c | 74 ++----------------------------------
drivers/gpu/drm/i915/intel_drv.h | 2 -
drivers/gpu/drm/i915/intel_pm.c | 42 --------------------
5 files changed, 3 insertions(+), 118 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3ac17f0f4053..029587ccd803 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -692,7 +692,6 @@ struct drm_i915_display_funcs {
void (*optimize_watermarks)(struct intel_atomic_state *state,
struct intel_crtc_state *cstate);
int (*compute_global_watermarks)(struct drm_atomic_state *state);
- void (*update_wm)(struct intel_crtc *crtc);
int (*modeset_calc_cdclk)(struct drm_atomic_state *state);
/* 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_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
index 36d4e635e4ce..e2b4ca518a04 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -173,8 +173,6 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
crtc_state->update_pipe = false;
crtc_state->disable_lp_wm = false;
crtc_state->disable_cxsr = false;
- crtc_state->update_wm_pre = false;
- crtc_state->update_wm_post = false;
crtc_state->fb_changed = false;
crtc_state->fifo_changed = false;
crtc_state->wm.need_postvbl_update = false;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0fa83895b8c4..90c8a964c264 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4848,8 +4848,6 @@ static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state)
intel_frontbuffer_flip(to_i915(crtc->base.dev), pipe_config->fb_bits);
- if (pipe_config->update_wm_post && pipe_config->base.active)
- intel_update_watermarks(crtc);
if (old_pri_state) {
struct intel_plane_state *primary_state =
@@ -4940,8 +4938,6 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state,
if (dev_priv->display.initial_watermarks != NULL)
dev_priv->display.initial_watermarks(old_intel_state,
pipe_config);
- else if (pipe_config->update_wm_pre)
- intel_update_watermarks(crtc);
}
static void intel_crtc_disable_planes(struct drm_crtc *crtc, unsigned plane_mask)
@@ -5623,8 +5619,6 @@ static void i9xx_crtc_enable(struct intel_crtc_state *pipe_config,
if (dev_priv->display.initial_watermarks != NULL)
dev_priv->display.initial_watermarks(old_intel_state,
intel_crtc->config);
- else
- intel_update_watermarks(intel_crtc);
intel_enable_pipe(intel_crtc);
assert_vblank_disabled(crtc);
@@ -5689,9 +5683,6 @@ static void i9xx_crtc_disable(struct intel_crtc_state *old_crtc_state,
if (!IS_GEN2(dev_priv))
intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false);
- if (!dev_priv->display.initial_watermarks)
- intel_update_watermarks(intel_crtc);
-
/* clock the pipe down to 640x480@60 to potentially save power */
if (IS_I830(dev_priv))
i830_enable_pipe(dev_priv, pipe);
@@ -5752,7 +5743,6 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc,
encoder->base.crtc = NULL;
intel_fbc_disable(intel_crtc);
- intel_update_watermarks(intel_crtc);
intel_disable_shared_dpll(intel_crtc);
domains = intel_crtc->enabled_power_domains;
@@ -10019,40 +10009,6 @@ static void intel_crtc_destroy(struct drm_crtc *crtc)
kfree(intel_crtc);
}
-/**
- * intel_wm_need_update - Check whether watermarks need updating
- * @plane: drm plane
- * @state: new plane state
- *
- * Check current plane state versus the new one to determine whether
- * watermarks need to be recalculated.
- *
- * Returns true or false.
- */
-static bool intel_wm_need_update(struct drm_plane *plane,
- struct drm_plane_state *state)
-{
- struct intel_plane_state *new = to_intel_plane_state(state);
- struct intel_plane_state *cur = to_intel_plane_state(plane->state);
-
- /* Update watermarks on tiling or size changes. */
- if (new->base.visible != cur->base.visible)
- return true;
-
- if (!cur->base.fb || !new->base.fb)
- return false;
-
- if (cur->base.fb->modifier != new->base.fb->modifier ||
- cur->base.rotation != new->base.rotation ||
- drm_rect_width(&new->base.src) != drm_rect_width(&cur->base.src) ||
- drm_rect_height(&new->base.src) != drm_rect_height(&cur->base.src) ||
- drm_rect_width(&new->base.dst) != drm_rect_width(&cur->base.dst) ||
- drm_rect_height(&new->base.dst) != drm_rect_height(&cur->base.dst))
- return true;
-
- return false;
-}
-
static bool needs_scaling(struct intel_plane_state *state)
{
int src_w = drm_rect_width(&state->base.src) >> 16;
@@ -10129,27 +10085,9 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
was_visible, visible,
turn_off, turn_on, mode_changed);
- if (turn_on) {
- if (INTEL_GEN(dev_priv) < 5 && !IS_G4X(dev_priv))
- pipe_config->update_wm_pre = true;
-
- /* must disable cxsr around plane enable/disable */
- if (plane->id != PLANE_CURSOR)
- pipe_config->disable_cxsr = true;
- } else if (turn_off) {
- if (INTEL_GEN(dev_priv) < 5 && !IS_G4X(dev_priv))
- pipe_config->update_wm_post = true;
-
- /* must disable cxsr around plane enable/disable */
- if (plane->id != PLANE_CURSOR)
- pipe_config->disable_cxsr = true;
- } else if (intel_wm_need_update(&plane->base, plane_state)) {
- if (INTEL_GEN(dev_priv) < 5 && !IS_G4X(dev_priv)) {
- /* FIXME bollocks */
- pipe_config->update_wm_pre = true;
- pipe_config->update_wm_post = true;
- }
- }
+ /* must disable cxsr around plane enable/disable */
+ if ((turn_on || turn_off) && plane->id != PLANE_CURSOR)
+ pipe_config->disable_cxsr = true;
if (visible || was_visible)
pipe_config->fb_bits |= plane->frontbuffer_bit;
@@ -10210,9 +10148,6 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
int ret;
bool mode_changed = needs_modeset(crtc_state);
- if (mode_changed && !crtc_state->active)
- pipe_config->update_wm_post = true;
-
if (mode_changed && crtc_state->enable &&
dev_priv->display.crtc_compute_clock &&
!WARN_ON(pipe_config->shared_dpll)) {
@@ -11898,9 +11833,6 @@ static bool needs_vblank_wait(struct intel_crtc_state *crtc_state)
return true;
/* wm changes, need vblank before final wm's */
- if (crtc_state->update_wm_post)
- return true;
-
if (crtc_state->wm.need_postvbl_update)
return true;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 1a70939c69c5..5486134189a7 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -632,7 +632,6 @@ struct intel_crtc_state {
unsigned fb_bits; /* framebuffers to flip */
bool update_pipe; /* can a fast modeset be performed? */
bool disable_cxsr;
- bool update_wm_pre, update_wm_post; /* watermarks are updated */
bool fb_changed; /* fb on any of the planes is changed */
bool fifo_changed; /* FIFO split is changed */
@@ -1838,7 +1837,6 @@ bool chv_phy_powergate_ch(struct drm_i915_private *dev_priv, enum dpio_phy phy,
void intel_init_clock_gating(struct drm_i915_private *dev_priv);
void intel_suspend_hw(struct drm_i915_private *dev_priv);
int ilk_wm_max_level(const struct drm_i915_private *dev_priv);
-void intel_update_watermarks(struct intel_crtc *crtc);
void intel_init_pm(struct drm_i915_private *dev_priv);
void intel_init_clock_gating_hooks(struct drm_i915_private *dev_priv);
void intel_pm_setup(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 3afaa6933c4b..3aa857ba660b 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5191,8 +5191,6 @@ skl_compute_wm(struct drm_atomic_state *state)
if ((results->dirty_pipes & drm_crtc_mask(crtc)) == 0)
/* This pipe's WM's did not change */
continue;
-
- intel_cstate->update_wm_pre = true;
}
skl_print_wm_changes(state);
@@ -5878,46 +5876,6 @@ void ilk_wm_get_hw_state(struct drm_device *dev)
!(I915_READ(DISP_ARB_CTL) & DISP_FBC_WM_DIS);
}
-/**
- * intel_update_watermarks - update FIFO watermark values based on current modes
- *
- * Calculate watermark values for the various WM regs based on current mode
- * and plane configuration.
- *
- * There are several cases to deal with here:
- * - normal (i.e. non-self-refresh)
- * - self-refresh (SR) mode
- * - lines are large relative to FIFO size (buffer can hold up to 2)
- * - lines are small relative to FIFO size (buffer can hold more than 2
- * lines), so need to account for TLB latency
- *
- * The normal calculation is:
- * watermark = dotclock * bytes per pixel * latency
- * where latency is platform & configuration dependent (we assume pessimal
- * values here).
- *
- * The SR calculation is:
- * watermark = (trunc(latency/line time)+1) * surface width *
- * bytes per pixel
- * where
- * line time = htotal / dotclock
- * surface width = hdisplay for normal plane and 64 for cursor
- * and latency is assumed to be high, as above.
- *
- * The final value programmed to the register should always be rounded up,
- * and include an extra 2 entries to account for clock crossings.
- *
- * We don't use the sprite, so we can ignore that. And on Crestline we have
- * to set the non-SR watermarks to 8.
- */
-void intel_update_watermarks(struct intel_crtc *crtc)
-{
- struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-
- if (dev_priv->display.update_wm)
- dev_priv->display.update_wm(crtc);
-}
-
/*
* Lock protecting IPS related data structures
*/
--
2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915: Convert gen4- watermarks to atomic. (rev2)
2017-08-07 10:48 [PATCH 0/7] drm/i915: Convert gen4- watermarks to atomic Maarten Lankhorst
` (6 preceding siblings ...)
2017-08-07 10:48 ` [PATCH 7/7] drm/i915: Rip out legacy watermark infrastructure Maarten Lankhorst
@ 2017-08-07 12:17 ` Patchwork
2017-08-08 9:26 ` Maarten Lankhorst
2017-08-08 11:19 ` ✗ Fi.CI.BAT: failure for drm/i915: Convert gen4- watermarks to atomic. (rev3) Patchwork
8 siblings, 1 reply; 14+ messages in thread
From: Patchwork @ 2017-08-07 12:17 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Convert gen4- watermarks to atomic. (rev2)
URL : https://patchwork.freedesktop.org/series/23954/
State : failure
== Summary ==
Series 23954v2 drm/i915: Convert gen4- watermarks to atomic.
https://patchwork.freedesktop.org/api/1.0/series/23954/revisions/2/mbox/
Test gem_exec_parallel:
Subgroup basic:
fail -> PASS (fi-ilk-650) fdo#101735
Test gem_ringfill:
Subgroup basic-default:
skip -> PASS (fi-bsw-n3050) fdo#101915
Test kms_cursor_legacy:
Subgroup basic-busy-flip-before-cursor-legacy:
pass -> FAIL (fi-pnv-d510)
Subgroup basic-flip-after-cursor-legacy:
pass -> FAIL (fi-pnv-d510)
Subgroup basic-flip-after-cursor-varying-size:
pass -> FAIL (fi-pnv-d510)
Subgroup basic-flip-before-cursor-legacy:
pass -> FAIL (fi-pnv-d510)
Subgroup basic-flip-before-cursor-varying-size:
pass -> FAIL (fi-pnv-d510)
Test kms_flip:
Subgroup basic-flip-vs-modeset:
skip -> PASS (fi-skl-x1585l) fdo#101781
Subgroup basic-flip-vs-wf_vblank:
pass -> FAIL (fi-pnv-d510)
Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-a:
pass -> DMESG-WARN (fi-pnv-d510) fdo#101597 +1
fdo#101735 https://bugs.freedesktop.org/show_bug.cgi?id=101735
fdo#101915 https://bugs.freedesktop.org/show_bug.cgi?id=101915
fdo#101781 https://bugs.freedesktop.org/show_bug.cgi?id=101781
fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597
fi-bdw-5557u total:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:439s
fi-bdw-gvtdvm total:279 pass:265 dwarn:0 dfail:0 fail:0 skip:14 time:425s
fi-blb-e6850 total:279 pass:224 dwarn:1 dfail:0 fail:0 skip:54 time:365s
fi-bsw-n3050 total:279 pass:243 dwarn:0 dfail:0 fail:0 skip:36 time:489s
fi-bxt-j4205 total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:501s
fi-byt-j1900 total:279 pass:254 dwarn:1 dfail:0 fail:0 skip:24 time:521s
fi-byt-n2820 total:279 pass:250 dwarn:1 dfail:0 fail:0 skip:28 time:510s
fi-glk-2a total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:583s
fi-hsw-4770 total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:431s
fi-hsw-4770r total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:409s
fi-ilk-650 total:279 pass:229 dwarn:0 dfail:0 fail:0 skip:50 time:421s
fi-ivb-3520m total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:509s
fi-ivb-3770 total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:473s
fi-kbl-7500u total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:456s
fi-kbl-7560u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:565s
fi-kbl-r total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:578s
fi-pnv-d510 total:279 pass:215 dwarn:3 dfail:0 fail:6 skip:55 time:605s
fi-skl-6260u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:448s
fi-skl-6700k total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:639s
fi-skl-6770hq total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:458s
fi-skl-gvtdvm total:279 pass:266 dwarn:0 dfail:0 fail:0 skip:13 time:427s
fi-skl-x1585l total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:489s
fi-snb-2520m total:279 pass:251 dwarn:0 dfail:0 fail:0 skip:28 time:553s
fi-snb-2600 total:279 pass:250 dwarn:0 dfail:0 fail:0 skip:29 time:407s
96c5eac5f202920356b34ef2da8a785bb8b4f320 drm-tip: 2017y-08m-07d-10h-55m-52s UTC integration manifest
1349492b8137 drm/i915: Rip out legacy watermark infrastructure
0ce836b1f4c0 drm/i915: Kill off intel_crtc_active.
dfbc9cf45b41 drm/i915: Program gen4 watermarks atomically
7c18d8bfbf7c drm/i915: Calculate gen4 watermarks semiatomically.
d9c8a8aea5bf drm/i915: Convert pineview watermarks to atomic, v2.
bcf55b93672f drm/i915: Program gen3- watermarks atomically
eaf653e8ffa8 drm/i915: Calculate gen3- watermarks semi-atomically.
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5332/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: ✗ Fi.CI.BAT: failure for drm/i915: Convert gen4- watermarks to atomic. (rev2)
2017-08-07 12:17 ` ✗ Fi.CI.BAT: failure for drm/i915: Convert gen4- watermarks to atomic. (rev2) Patchwork
@ 2017-08-08 9:26 ` Maarten Lankhorst
0 siblings, 0 replies; 14+ messages in thread
From: Maarten Lankhorst @ 2017-08-08 9:26 UTC (permalink / raw)
To: intel-gfx
Op 07-08-17 om 14:17 schreef Patchwork:
> == Series Details ==
>
> Series: drm/i915: Convert gen4- watermarks to atomic. (rev2)
> URL : https://patchwork.freedesktop.org/series/23954/
> State : failure
>
> == Summary ==
>
> Series 23954v2 drm/i915: Convert gen4- watermarks to atomic.
> https://patchwork.freedesktop.org/api/1.0/series/23954/revisions/2/mbox/
>
> Test gem_exec_parallel:
> Subgroup basic:
> fail -> PASS (fi-ilk-650) fdo#101735
> Test gem_ringfill:
> Subgroup basic-default:
> skip -> PASS (fi-bsw-n3050) fdo#101915
> Test kms_cursor_legacy:
> Subgroup basic-busy-flip-before-cursor-legacy:
> pass -> FAIL (fi-pnv-d510)
> Subgroup basic-flip-after-cursor-legacy:
> pass -> FAIL (fi-pnv-d510)
> Subgroup basic-flip-after-cursor-varying-size:
> pass -> FAIL (fi-pnv-d510)
> Subgroup basic-flip-before-cursor-legacy:
> pass -> FAIL (fi-pnv-d510)
> Subgroup basic-flip-before-cursor-varying-size:
> pass -> FAIL (fi-pnv-d510)
> Test kms_flip:
> Subgroup basic-flip-vs-modeset:
> skip -> PASS (fi-skl-x1585l) fdo#101781
> Subgroup basic-flip-vs-wf_vblank:
> pass -> FAIL (fi-pnv-d510)
> Test kms_pipe_crc_basic:
> Subgroup hang-read-crc-pipe-a:
> pass -> DMESG-WARN (fi-pnv-d510) fdo#101597 +1
>
> fdo#101735 https://bugs.freedesktop.org/show_bug.cgi?id=101735
> fdo#101915 https://bugs.freedesktop.org/show_bug.cgi?id=101915
> fdo#101781 https://bugs.freedesktop.org/show_bug.cgi?id=101781
> fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597
>
> fi-bdw-5557u total:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:439s
> fi-bdw-gvtdvm total:279 pass:265 dwarn:0 dfail:0 fail:0 skip:14 time:425s
> fi-blb-e6850 total:279 pass:224 dwarn:1 dfail:0 fail:0 skip:54 time:365s
> fi-bsw-n3050 total:279 pass:243 dwarn:0 dfail:0 fail:0 skip:36 time:489s
> fi-bxt-j4205 total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:501s
> fi-byt-j1900 total:279 pass:254 dwarn:1 dfail:0 fail:0 skip:24 time:521s
> fi-byt-n2820 total:279 pass:250 dwarn:1 dfail:0 fail:0 skip:28 time:510s
> fi-glk-2a total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:583s
> fi-hsw-4770 total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:431s
> fi-hsw-4770r total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:409s
> fi-ilk-650 total:279 pass:229 dwarn:0 dfail:0 fail:0 skip:50 time:421s
> fi-ivb-3520m total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:509s
> fi-ivb-3770 total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:473s
> fi-kbl-7500u total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:456s
> fi-kbl-7560u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:565s
> fi-kbl-r total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:578s
> fi-pnv-d510 total:279 pass:215 dwarn:3 dfail:0 fail:6 skip:55 time:605s
> fi-skl-6260u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:448s
> fi-skl-6700k total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:639s
> fi-skl-6770hq total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:458s
> fi-skl-gvtdvm total:279 pass:266 dwarn:0 dfail:0 fail:0 skip:13 time:427s
> fi-skl-x1585l total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:489s
> fi-snb-2520m total:279 pass:251 dwarn:0 dfail:0 fail:0 skip:28 time:553s
> fi-snb-2600 total:279 pass:250 dwarn:0 dfail:0 fail:0 skip:29 time:407s
>
> 96c5eac5f202920356b34ef2da8a785bb8b4f320 drm-tip: 2017y-08m-07d-10h-55m-52s UTC integration manifest
> 1349492b8137 drm/i915: Rip out legacy watermark infrastructure
> 0ce836b1f4c0 drm/i915: Kill off intel_crtc_active.
> dfbc9cf45b41 drm/i915: Program gen4 watermarks atomically
> 7c18d8bfbf7c drm/i915: Calculate gen4 watermarks semiatomically.
> d9c8a8aea5bf drm/i915: Convert pineview watermarks to atomic, v2.
> bcf55b93672f drm/i915: Program gen3- watermarks atomically
> eaf653e8ffa8 drm/i915: Calculate gen3- watermarks semi-atomically.
Interestingly, kms_cursor_legacy is failing because of too much debug info slowing things down. With drm.debug=0xe we miss all page flips, with drm.debug=0x1f it falls over even sooner. I think this happens because of the serial console, so I'll fix the PNV patch to be less chatty..
This will probably fix the kms_flip failure too, but I'll need to look more at kms_pipe_crc_basic. This series was supposed to fix that bug, so a failure with this series is serious.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] drm/i915: Calculate gen3- watermarks semi-atomically, v2.
2017-08-07 10:48 ` [PATCH 1/7] drm/i915: Calculate gen3- watermarks semi-atomically Maarten Lankhorst
@ 2017-08-08 11:00 ` Maarten Lankhorst
2017-08-08 12:51 ` [PATCH 1/7] drm/i915: Calculate gen3- watermarks semi-atomically Mahesh Kumar
1 sibling, 0 replies; 14+ messages in thread
From: Maarten Lankhorst @ 2017-08-08 11:00 UTC (permalink / raw)
To: intel-gfx
The gen3 watermark calculations are converted to atomic,
but the wm update calls are still done through the legacy
functions.
This will make it easier to bisect things if they go wrong.
CI was having issues on the kms_cursor_legacy tests with too
much debug info printed out, in order to reduce the chattiness
we will kill the prints from intel_calculate_wm.
Changes since v1:
- Neuter intel_calculate_wm.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 3 +-
drivers/gpu/drm/i915/intel_drv.h | 14 +++
drivers/gpu/drm/i915/intel_pm.c | 233 ++++++++++++++++++++---------------
3 files changed, 152 insertions(+), 98 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 974d4b577189..234b94cafc7d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14709,7 +14709,8 @@ intel_modeset_setup_hw_state(struct drm_device *dev,
skl_wm_get_hw_state(dev);
} else if (HAS_PCH_SPLIT(dev_priv)) {
ilk_wm_get_hw_state(dev);
- }
+ } else if (INTEL_GEN(dev_priv) <= 3 && !IS_PINEVIEW(dev_priv))
+ i9xx_wm_get_hw_state(dev);
for_each_intel_crtc(dev, crtc) {
u64 put_domains;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f91de9cb9697..d53d34756048 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -547,6 +547,15 @@ struct g4x_wm_state {
bool fbc_en;
};
+struct i9xx_wm_state {
+ uint16_t plane_wm;
+ bool cxsr;
+
+ struct {
+ uint16_t plane;
+ } sr;
+};
+
struct intel_crtc_wm_state {
union {
struct {
@@ -591,6 +600,9 @@ struct intel_crtc_wm_state {
/* optimal watermarks */
struct g4x_wm_state optimal;
} g4x;
+ struct {
+ struct i9xx_wm_state optimal;
+ } i9xx;
};
/*
@@ -823,6 +835,7 @@ struct intel_crtc {
struct intel_pipe_wm ilk;
struct vlv_wm_state vlv;
struct g4x_wm_state g4x;
+ struct i9xx_wm_state i9xx;
} active;
} wm;
@@ -1845,6 +1858,7 @@ void gen6_rps_boost(struct drm_i915_gem_request *rq,
struct intel_rps_client *rps);
void intel_queue_rps_boost_for_request(struct drm_i915_gem_request *req);
void g4x_wm_get_hw_state(struct drm_device *dev);
+void i9xx_wm_get_hw_state(struct drm_device *dev);
void vlv_wm_get_hw_state(struct drm_device *dev);
void ilk_wm_get_hw_state(struct drm_device *dev);
void skl_wm_get_hw_state(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 6e393b217450..173a514cd6df 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -751,10 +751,8 @@ static unsigned int intel_calculate_wm(int pixel_rate,
latency_ns / 100);
entries = DIV_ROUND_UP(entries, wm->cacheline_size) +
wm->guard_size;
- DRM_DEBUG_KMS("FIFO entries required for mode: %d\n", entries);
wm_size = fifo_size - entries;
- DRM_DEBUG_KMS("FIFO watermark level: %d\n", wm_size);
/* Don't promote wm_size to unsigned... */
if (wm_size > wm->max_wm)
@@ -2224,89 +2222,154 @@ static void i965_update_wm(struct intel_crtc *unused_crtc)
#undef FW_WM
-static void i9xx_update_wm(struct intel_crtc *unused_crtc)
+static const struct intel_watermark_params *i9xx_get_wm_info(struct drm_i915_private *dev_priv,
+ struct intel_crtc *crtc)
{
- struct drm_i915_private *dev_priv = to_i915(unused_crtc->base.dev);
- const struct intel_watermark_params *wm_info;
- uint32_t fwater_lo;
- uint32_t fwater_hi;
- int cwm, srwm = 1;
- int fifo_size;
- int planea_wm, planeb_wm;
- struct intel_crtc *crtc, *enabled = NULL;
+ struct intel_plane *plane = to_intel_plane(crtc->base.primary);
if (IS_I945GM(dev_priv))
- wm_info = &i945_wm_info;
+ return &i945_wm_info;
else if (!IS_GEN2(dev_priv))
- wm_info = &i915_wm_info;
+ return &i915_wm_info;
+ else if (plane->plane == PLANE_A)
+ return &i830_a_wm_info;
else
- wm_info = &i830_a_wm_info;
+ return &i830_bc_wm_info;
+}
- fifo_size = dev_priv->display.get_fifo_size(dev_priv, 0);
- crtc = intel_get_crtc_for_plane(dev_priv, 0);
- if (intel_crtc_active(crtc)) {
+static int i9xx_compute_pipe_wm(struct intel_crtc_state *crtc_state)
+{
+ struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+ struct intel_atomic_state *state =
+ to_intel_atomic_state(crtc_state->base.state);
+ struct i9xx_wm_state *wm_state = &crtc_state->wm.i9xx.optimal;
+ struct intel_plane *plane = to_intel_plane(crtc->base.primary);
+ const struct drm_plane_state *plane_state = NULL;
+ int fifo_size;
+ const struct intel_watermark_params *wm_info;
+
+ fifo_size = dev_priv->display.get_fifo_size(dev_priv, plane->plane);
+
+ wm_info = i9xx_get_wm_info(dev_priv, crtc);
+
+ wm_state->cxsr = false;
+ memset(&wm_state->sr, 0, sizeof(wm_state->sr));
+
+ if (crtc_state->base.plane_mask & BIT(drm_plane_index(&plane->base)))
+ plane_state = __drm_atomic_get_current_plane_state(&state->base, &plane->base);
+
+ if (!plane_state || !intel_wm_plane_visible(crtc_state, to_intel_plane_state(plane_state))) {
+ wm_state->plane_wm = fifo_size - wm_info->guard_size;
+ if (wm_state->plane_wm > (long)wm_info->max_wm)
+ wm_state->plane_wm = wm_info->max_wm;
+ } else if (HAS_FW_BLC(dev_priv)) {
+ struct drm_framebuffer *fb = plane_state->fb;
+ unsigned cpp = IS_GEN2(dev_priv) ? 4 : fb->format->cpp[0];
const struct drm_display_mode *adjusted_mode =
- &crtc->config->base.adjusted_mode;
- const struct drm_framebuffer *fb =
- crtc->base.primary->state->fb;
- int cpp;
+ &crtc_state->base.adjusted_mode;
+ unsigned active_crtcs;
+ bool may_cxsr;
- if (IS_GEN2(dev_priv))
- cpp = 4;
+ if (state->modeset)
+ active_crtcs = state->active_crtcs;
else
- cpp = fb->format->cpp[0];
+ active_crtcs = dev_priv->active_crtcs;
- planea_wm = intel_calculate_wm(adjusted_mode->crtc_clock,
- wm_info, fifo_size, cpp,
- pessimal_latency_ns);
- enabled = crtc;
- } else {
- planea_wm = fifo_size - wm_info->guard_size;
- if (planea_wm > (long)wm_info->max_wm)
- planea_wm = wm_info->max_wm;
+ may_cxsr = active_crtcs == drm_crtc_mask(&crtc->base);
+
+ wm_state->plane_wm = intel_calculate_wm(adjusted_mode->crtc_clock,
+ wm_info, fifo_size, cpp,
+ pessimal_latency_ns);
+
+ DRM_DEBUG_KMS("FIFO watermarks - plane watermarks: %d\n", wm_state->plane_wm);
+
+ if (IS_I915GM(dev_priv) && i915_gem_object_is_tiled(intel_fb_obj(fb)))
+ may_cxsr = false;
+
+ if (may_cxsr) {
+ static const int sr_latency_ns = 6000;
+ unsigned long entries;
+
+ entries = intel_wm_method2(adjusted_mode->crtc_clock,
+ adjusted_mode->crtc_htotal,
+ crtc_state->pipe_src_w, cpp,
+ sr_latency_ns / 100);
+ entries = DIV_ROUND_UP(entries, wm_info->cacheline_size);
+
+ DRM_DEBUG_KMS("self-refresh entries: %ld\n", entries);
+
+ if (wm_info->fifo_size >= entries) {
+ wm_state->cxsr = true;
+ wm_state->sr.plane = wm_info->fifo_size - entries;
+ } else
+ may_cxsr = false;
+ }
+
+ DRM_DEBUG_KMS("FIFO watermarks - plane watermarks: %d, can cxsr: %s, SR size: %d\n",
+ wm_state->plane_wm, yesno(wm_state->cxsr), wm_state->sr.plane);
}
- if (IS_GEN2(dev_priv))
- wm_info = &i830_bc_wm_info;
+ return 0;
+}
- fifo_size = dev_priv->display.get_fifo_size(dev_priv, 1);
- crtc = intel_get_crtc_for_plane(dev_priv, 1);
- if (intel_crtc_active(crtc)) {
- const struct drm_display_mode *adjusted_mode =
- &crtc->config->base.adjusted_mode;
- const struct drm_framebuffer *fb =
- crtc->base.primary->state->fb;
- int cpp;
+void i9xx_wm_get_hw_state(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = to_i915(dev);
+ struct intel_crtc *crtc;
+ uint32_t fwater_lo, planea_wm, planeb_wm;
+
+ fwater_lo = I915_READ(FW_BLC);
+
+ planea_wm = fwater_lo & 0x3f;
+ planeb_wm = (fwater_lo >> 16) & 0x3f;
+
+ for_each_intel_crtc(dev, crtc) {
+ struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->base.state);
+ struct i9xx_wm_state *wm_state = &cstate->wm.i9xx.optimal;
+ struct intel_plane *plane = to_intel_plane(crtc->base.primary);
+
+ memset(&wm_state->sr, 0, sizeof(wm_state->sr));
+ wm_state->cxsr = false;
- if (IS_GEN2(dev_priv))
- cpp = 4;
+ if (plane == PLANE_A)
+ wm_state->plane_wm = planea_wm;
else
- cpp = fb->format->cpp[0];
+ wm_state->plane_wm = planeb_wm;
+
+ crtc->wm.active.i9xx = *wm_state;
+ }
+}
- planeb_wm = intel_calculate_wm(adjusted_mode->crtc_clock,
- wm_info, fifo_size, cpp,
- pessimal_latency_ns);
+static void i9xx_update_wm(struct intel_crtc *crtc)
+{
+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+ uint32_t fwater_lo;
+ uint32_t fwater_hi;
+ int cwm, srwm = -1;
+ int planea_wm, planeb_wm;
+ struct intel_crtc *enabled = NULL;
+
+ crtc->wm.active.i9xx = crtc->config->wm.i9xx.optimal;
+
+ crtc = intel_get_crtc_for_plane(dev_priv, 0);
+ planea_wm = crtc->wm.active.i9xx.plane_wm;
+ if (intel_crtc_active(crtc))
+ enabled = crtc;
+
+ crtc = intel_get_crtc_for_plane(dev_priv, 1);
+ if (intel_crtc_active(crtc)) {
if (enabled == NULL)
enabled = crtc;
else
enabled = NULL;
- } else {
- planeb_wm = fifo_size - wm_info->guard_size;
- if (planeb_wm > (long)wm_info->max_wm)
- planeb_wm = wm_info->max_wm;
}
+ planeb_wm = crtc->wm.active.i9xx.plane_wm;
DRM_DEBUG_KMS("FIFO watermarks - A: %d, B: %d\n", planea_wm, planeb_wm);
- if (IS_I915GM(dev_priv) && enabled) {
- struct drm_i915_gem_object *obj;
-
- obj = intel_fb_obj(enabled->base.primary->state->fb);
-
- /* self-refresh seems busted with untiled */
- if (!i915_gem_object_is_tiled(obj))
- enabled = NULL;
- }
+ if (!enabled->wm.active.i9xx.cxsr)
+ enabled = NULL;
/*
* Overlay gets an aggressive default since video jitter is bad.
@@ -2317,31 +2380,8 @@ static void i9xx_update_wm(struct intel_crtc *unused_crtc)
intel_set_memory_cxsr(dev_priv, false);
/* Calc sr entries for one plane configs */
- if (HAS_FW_BLC(dev_priv) && enabled) {
- /* self-refresh has much higher latency */
- static const int sr_latency_ns = 6000;
- const struct drm_display_mode *adjusted_mode =
- &enabled->config->base.adjusted_mode;
- const struct drm_framebuffer *fb =
- enabled->base.primary->state->fb;
- int clock = adjusted_mode->crtc_clock;
- int htotal = adjusted_mode->crtc_htotal;
- int hdisplay = enabled->config->pipe_src_w;
- int cpp;
- int entries;
-
- if (IS_I915GM(dev_priv) || IS_I945GM(dev_priv))
- cpp = 4;
- else
- cpp = fb->format->cpp[0];
-
- entries = intel_wm_method2(clock, htotal, hdisplay, cpp,
- sr_latency_ns / 100);
- entries = DIV_ROUND_UP(entries, wm_info->cacheline_size);
- DRM_DEBUG_KMS("self-refresh entries: %d\n", entries);
- srwm = wm_info->fifo_size - entries;
- if (srwm < 0)
- srwm = 1;
+ if (enabled) {
+ srwm = enabled->wm.active.i9xx.sr.plane;
if (IS_I945G(dev_priv) || IS_I945GM(dev_priv))
I915_WRITE(FW_BLC_SELF,
@@ -2367,23 +2407,18 @@ static void i9xx_update_wm(struct intel_crtc *unused_crtc)
intel_set_memory_cxsr(dev_priv, true);
}
-static void i845_update_wm(struct intel_crtc *unused_crtc)
+static void i845_update_wm(struct intel_crtc *crtc)
{
- struct drm_i915_private *dev_priv = to_i915(unused_crtc->base.dev);
- struct intel_crtc *crtc;
- const struct drm_display_mode *adjusted_mode;
+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
uint32_t fwater_lo;
int planea_wm;
- crtc = single_enabled_crtc(dev_priv);
- if (crtc == NULL)
+ if (!intel_crtc_active(crtc))
return;
- adjusted_mode = &crtc->config->base.adjusted_mode;
- planea_wm = intel_calculate_wm(adjusted_mode->crtc_clock,
- &i845_wm_info,
- dev_priv->display.get_fifo_size(dev_priv, 0),
- 4, pessimal_latency_ns);
+ crtc->wm.active.i9xx = crtc->config->wm.i9xx.optimal;
+ planea_wm = crtc->wm.active.i9xx.plane_wm;
+
fwater_lo = I915_READ(FW_BLC) & ~0xfff;
fwater_lo |= (3<<8) | planea_wm;
@@ -8813,9 +8848,13 @@ void intel_init_pm(struct drm_i915_private *dev_priv)
} else if (IS_GEN4(dev_priv)) {
dev_priv->display.update_wm = i965_update_wm;
} else if (IS_GEN3(dev_priv)) {
+ dev_priv->display.compute_pipe_wm = i9xx_compute_pipe_wm;
dev_priv->display.update_wm = i9xx_update_wm;
+
dev_priv->display.get_fifo_size = i9xx_get_fifo_size;
} else if (IS_GEN2(dev_priv)) {
+ dev_priv->display.compute_pipe_wm = i9xx_compute_pipe_wm;
+
if (INTEL_INFO(dev_priv)->num_pipes == 1) {
dev_priv->display.update_wm = i845_update_wm;
dev_priv->display.get_fifo_size = i845_get_fifo_size;
--
2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915: Convert gen4- watermarks to atomic. (rev3)
2017-08-07 10:48 [PATCH 0/7] drm/i915: Convert gen4- watermarks to atomic Maarten Lankhorst
` (7 preceding siblings ...)
2017-08-07 12:17 ` ✗ Fi.CI.BAT: failure for drm/i915: Convert gen4- watermarks to atomic. (rev2) Patchwork
@ 2017-08-08 11:19 ` Patchwork
8 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2017-08-08 11:19 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Convert gen4- watermarks to atomic. (rev3)
URL : https://patchwork.freedesktop.org/series/23954/
State : failure
== Summary ==
Series 23954v3 drm/i915: Convert gen4- watermarks to atomic.
https://patchwork.freedesktop.org/api/1.0/series/23954/revisions/3/mbox/
Test kms_cursor_legacy:
Subgroup basic-busy-flip-before-cursor-legacy:
pass -> FAIL (fi-pnv-d510)
Subgroup basic-flip-after-cursor-legacy:
pass -> FAIL (fi-pnv-d510)
Subgroup basic-flip-after-cursor-varying-size:
pass -> FAIL (fi-pnv-d510)
Subgroup basic-flip-before-cursor-legacy:
pass -> FAIL (fi-pnv-d510)
Subgroup basic-flip-before-cursor-varying-size:
pass -> FAIL (fi-pnv-d510)
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-b:
dmesg-warn -> PASS (fi-byt-n2820) fdo#101705
Test prime_vgem:
Subgroup basic-gtt:
pass -> FAIL (fi-bxt-j4205) fdo#102066
fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705
fdo#102066 https://bugs.freedesktop.org/show_bug.cgi?id=102066
fi-bdw-5557u total:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:441s
fi-bdw-gvtdvm total:279 pass:265 dwarn:0 dfail:0 fail:0 skip:14 time:419s
fi-blb-e6850 total:279 pass:224 dwarn:1 dfail:0 fail:0 skip:54 time:358s
fi-bsw-n3050 total:279 pass:243 dwarn:0 dfail:0 fail:0 skip:36 time:489s
fi-bxt-j4205 total:279 pass:259 dwarn:0 dfail:0 fail:1 skip:19 time:496s
fi-byt-j1900 total:279 pass:254 dwarn:1 dfail:0 fail:0 skip:24 time:522s
fi-byt-n2820 total:279 pass:251 dwarn:0 dfail:0 fail:0 skip:28 time:508s
fi-glk-2a total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:584s
fi-hsw-4770 total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:430s
fi-hsw-4770r total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:415s
fi-ilk-650 total:279 pass:229 dwarn:0 dfail:0 fail:0 skip:50 time:419s
fi-ivb-3520m total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:508s
fi-ivb-3770 total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:475s
fi-kbl-7500u total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:466s
fi-kbl-7560u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:567s
fi-kbl-r total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:576s
fi-pnv-d510 total:279 pass:217 dwarn:2 dfail:0 fail:5 skip:55 time:577s
fi-skl-6260u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:447s
fi-skl-6700k total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:640s
fi-skl-6770hq total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:462s
fi-skl-gvtdvm total:279 pass:266 dwarn:0 dfail:0 fail:0 skip:13 time:426s
fi-skl-x1585l total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:485s
fi-snb-2520m total:279 pass:251 dwarn:0 dfail:0 fail:0 skip:28 time:549s
fi-snb-2600 total:279 pass:250 dwarn:0 dfail:0 fail:0 skip:29 time:405s
3d87f89058607b2a2adecf99ddb637a676b4df64 drm-tip: 2017y-08m-08d-09h-05m-05s UTC integration manifest
ef2fffbfc829 drm/i915: Rip out legacy watermark infrastructure
827fd5b9f7db drm/i915: Kill off intel_crtc_active.
4cb5e76a6a94 drm/i915: Program gen4 watermarks atomically
b0236e15378b drm/i915: Calculate gen4 watermarks semiatomically.
75c242e6dbfd drm/i915: Convert pineview watermarks to atomic, v2.
2cb740d2da4e drm/i915: Program gen3- watermarks atomically
99661d1c5824 drm/i915: Calculate gen3- watermarks semi-atomically, v2.
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5339/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/7] drm/i915: Calculate gen3- watermarks semi-atomically.
2017-08-07 10:48 ` [PATCH 1/7] drm/i915: Calculate gen3- watermarks semi-atomically Maarten Lankhorst
2017-08-08 11:00 ` [PATCH] drm/i915: Calculate gen3- watermarks semi-atomically, v2 Maarten Lankhorst
@ 2017-08-08 12:51 ` Mahesh Kumar
2017-09-07 12:39 ` Maarten Lankhorst
1 sibling, 1 reply; 14+ messages in thread
From: Mahesh Kumar @ 2017-08-08 12:51 UTC (permalink / raw)
To: Maarten Lankhorst, intel-gfx
Hi,
On Monday 07 August 2017 04:18 PM, Maarten Lankhorst wrote:
> The gen3 watermark calculations are converted to atomic,
> but the wm update calls are still done through the legacy
> functions.
>
> This will make it easier to bisect things if they go wrong.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 3 +-
> drivers/gpu/drm/i915/intel_drv.h | 14 +++
> drivers/gpu/drm/i915/intel_pm.c | 231 +++++++++++++++++++++--------------
> 3 files changed, 152 insertions(+), 96 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 974d4b577189..234b94cafc7d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14709,7 +14709,8 @@ intel_modeset_setup_hw_state(struct drm_device *dev,
> skl_wm_get_hw_state(dev);
> } else if (HAS_PCH_SPLIT(dev_priv)) {
> ilk_wm_get_hw_state(dev);
> - }
> + } else if (INTEL_GEN(dev_priv) <= 3 && !IS_PINEVIEW(dev_priv))
> + i9xx_wm_get_hw_state(dev);
>
> for_each_intel_crtc(dev, crtc) {
> u64 put_domains;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index f91de9cb9697..d53d34756048 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -547,6 +547,15 @@ struct g4x_wm_state {
> bool fbc_en;
> };
>
> +struct i9xx_wm_state {
> + uint16_t plane_wm;
> + bool cxsr;
> +
> + struct {
> + uint16_t plane;
should this also be named as plane_wm, because it's again wm with SR.
just a nitpick but not a stopper.
> + } sr;
> +};
> +
> struct intel_crtc_wm_state {
> union {
> struct {
> @@ -591,6 +600,9 @@ struct intel_crtc_wm_state {
> /* optimal watermarks */
> struct g4x_wm_state optimal;
> } g4x;
new line please to match the coding style of function
> + struct {
> + struct i9xx_wm_state optimal;
> + } i9xx;
> };
>
> /*
> @@ -823,6 +835,7 @@ struct intel_crtc {
> struct intel_pipe_wm ilk;
> struct vlv_wm_state vlv;
> struct g4x_wm_state g4x;
> + struct i9xx_wm_state i9xx;
> } active;
> } wm;
>
> @@ -1845,6 +1858,7 @@ void gen6_rps_boost(struct drm_i915_gem_request *rq,
> struct intel_rps_client *rps);
> void intel_queue_rps_boost_for_request(struct drm_i915_gem_request *req);
> void g4x_wm_get_hw_state(struct drm_device *dev);
> +void i9xx_wm_get_hw_state(struct drm_device *dev);
> void vlv_wm_get_hw_state(struct drm_device *dev);
> void ilk_wm_get_hw_state(struct drm_device *dev);
> void skl_wm_get_hw_state(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 6e393b217450..807c9a730020 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2224,89 +2224,154 @@ static void i965_update_wm(struct intel_crtc *unused_crtc)
>
> #undef FW_WM
>
> -static void i9xx_update_wm(struct intel_crtc *unused_crtc)
> +static const struct intel_watermark_params *i9xx_get_wm_info(struct drm_i915_private *dev_priv,
> + struct intel_crtc *crtc)
> {
> - struct drm_i915_private *dev_priv = to_i915(unused_crtc->base.dev);
> - const struct intel_watermark_params *wm_info;
> - uint32_t fwater_lo;
> - uint32_t fwater_hi;
> - int cwm, srwm = 1;
> - int fifo_size;
> - int planea_wm, planeb_wm;
> - struct intel_crtc *crtc, *enabled = NULL;
> + struct intel_plane *plane = to_intel_plane(crtc->base.primary);
>
> if (IS_I945GM(dev_priv))
> - wm_info = &i945_wm_info;
> + return &i945_wm_info;
> else if (!IS_GEN2(dev_priv))
> - wm_info = &i915_wm_info;
> + return &i915_wm_info;
> + else if (plane->plane == PLANE_A)
> + return &i830_a_wm_info;
> else
> - wm_info = &i830_a_wm_info;
> + return &i830_bc_wm_info;
> +}
>
> - fifo_size = dev_priv->display.get_fifo_size(dev_priv, 0);
> - crtc = intel_get_crtc_for_plane(dev_priv, 0);
> - if (intel_crtc_active(crtc)) {
> +static int i9xx_compute_pipe_wm(struct intel_crtc_state *crtc_state)
> +{
> + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> + struct intel_atomic_state *state =
> + to_intel_atomic_state(crtc_state->base.state);
> + struct i9xx_wm_state *wm_state = &crtc_state->wm.i9xx.optimal;
> + struct intel_plane *plane = to_intel_plane(crtc->base.primary);
> + const struct drm_plane_state *plane_state = NULL;
> + int fifo_size;
> + const struct intel_watermark_params *wm_info;
> +
> + fifo_size = dev_priv->display.get_fifo_size(dev_priv, plane->plane);
> +
> + wm_info = i9xx_get_wm_info(dev_priv, crtc);
> +
> + wm_state->cxsr = false;
> + memset(&wm_state->sr, 0, sizeof(wm_state->sr));
> +
> + if (crtc_state->base.plane_mask & BIT(drm_plane_index(&plane->base)))
> + plane_state = __drm_atomic_get_current_plane_state(&state->base, &plane->base);
> +
> + if (!plane_state || !intel_wm_plane_visible(crtc_state, to_intel_plane_state(plane_state))) {
> + wm_state->plane_wm = fifo_size - wm_info->guard_size;
> + if (wm_state->plane_wm > (long)wm_info->max_wm)
> + wm_state->plane_wm = wm_info->max_wm;
> + } else if (HAS_FW_BLC(dev_priv)) {
This part will be called only for (GEN > 2,) So we'll never be
calculating wm for GEN2. but you are changing GEN2 wm also to 2 stages
compute & update in intel_init_pm.
> + struct drm_framebuffer *fb = plane_state->fb;
> + unsigned cpp = IS_GEN2(dev_priv) ? 4 : fb->format->cpp[0];
> const struct drm_display_mode *adjusted_mode =
> - &crtc->config->base.adjusted_mode;
> - const struct drm_framebuffer *fb =
> - crtc->base.primary->state->fb;
> - int cpp;
> + &crtc_state->base.adjusted_mode;
> + unsigned active_crtcs;
> + bool may_cxsr;
>
> - if (IS_GEN2(dev_priv))
> - cpp = 4;
> + if (state->modeset)
> + active_crtcs = state->active_crtcs;
> else
> - cpp = fb->format->cpp[0];
> + active_crtcs = dev_priv->active_crtcs;
>
> - planea_wm = intel_calculate_wm(adjusted_mode->crtc_clock,
> - wm_info, fifo_size, cpp,
> - pessimal_latency_ns);
> - enabled = crtc;
> - } else {
> - planea_wm = fifo_size - wm_info->guard_size;
> - if (planea_wm > (long)wm_info->max_wm)
> - planea_wm = wm_info->max_wm;
> + may_cxsr = active_crtcs == drm_crtc_mask(&crtc->base);
It would be good to have a comment stating if only one CRTC is enabled
we can go to cxsr.
we can use hweight32() function to check if only one CRTC is enabled
(not mandatory).
> +
> + wm_state->plane_wm = intel_calculate_wm(adjusted_mode->crtc_clock,
> + wm_info, fifo_size, cpp,
> + pessimal_latency_ns);
> +
> + DRM_DEBUG_KMS("FIFO watermarks - plane watermarks: %d\n", wm_state->plane_wm);
> +
> + if (IS_I915GM(dev_priv) && i915_gem_object_is_tiled(intel_fb_obj(fb)))
> + may_cxsr = false;
> +
> + if (may_cxsr) {
> + static const int sr_latency_ns = 6000;
> + unsigned long entries;
> +
> + entries = intel_wm_method2(adjusted_mode->crtc_clock,
> + adjusted_mode->crtc_htotal,
> + crtc_state->pipe_src_w, cpp,
> + sr_latency_ns / 100);
> + entries = DIV_ROUND_UP(entries, wm_info->cacheline_size);
> +
> + DRM_DEBUG_KMS("self-refresh entries: %ld\n", entries);
> +
> + if (wm_info->fifo_size >= entries) {
Earlier if (fifo_size < entries) we were assigning srwm = 1 & not
disabling cxsr, Is there any specific Bspec documentation to not enable
cxsr if fifo_size < entries ?
> + wm_state->cxsr = true;
> + wm_state->sr.plane = wm_info->fifo_size - entries;
> + } else
> + may_cxsr = false;
may_cxsr is not used later, so no need to overwrite the value.
> + }
> +
> + DRM_DEBUG_KMS("FIFO watermarks - plane watermarks: %d, can cxsr: %s, SR size: %d\n",
> + wm_state->plane_wm, yesno(wm_state->cxsr), wm_state->sr.plane);
> }
>
> - if (IS_GEN2(dev_priv))
> - wm_info = &i830_bc_wm_info;
> + return 0;
> +}
>
> - fifo_size = dev_priv->display.get_fifo_size(dev_priv, 1);
> - crtc = intel_get_crtc_for_plane(dev_priv, 1);
> - if (intel_crtc_active(crtc)) {
> - const struct drm_display_mode *adjusted_mode =
> - &crtc->config->base.adjusted_mode;
> - const struct drm_framebuffer *fb =
> - crtc->base.primary->state->fb;
> - int cpp;
> +void i9xx_wm_get_hw_state(struct drm_device *dev)
> +{
> + struct drm_i915_private *dev_priv = to_i915(dev);
> + struct intel_crtc *crtc;
> + uint32_t fwater_lo, planea_wm, planeb_wm;
> +
> + fwater_lo = I915_READ(FW_BLC);
> +
> + planea_wm = fwater_lo & 0x3f;
> + planeb_wm = (fwater_lo >> 16) & 0x3f;
> +
> + for_each_intel_crtc(dev, crtc) {
> + struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->base.state);
> + struct i9xx_wm_state *wm_state = &cstate->wm.i9xx.optimal;
> + struct intel_plane *plane = to_intel_plane(crtc->base.primary);
> +
> + memset(&wm_state->sr, 0, sizeof(wm_state->sr));
> + wm_state->cxsr = false;
>
> - if (IS_GEN2(dev_priv))
> - cpp = 4;
> + if (plane == PLANE_A)
here plane is of type intel_plane but you are comparing with enum plane,
I think you meant plane->plane
> + wm_state->plane_wm = planea_wm;
> else
> - cpp = fb->format->cpp[0];
> + wm_state->plane_wm = planeb_wm;
> +
> + crtc->wm.active.i9xx = *wm_state;
> + }
> +}
>
> - planeb_wm = intel_calculate_wm(adjusted_mode->crtc_clock,
> - wm_info, fifo_size, cpp,
> - pessimal_latency_ns);
> +static void i9xx_update_wm(struct intel_crtc *crtc)
> +{
> + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> + uint32_t fwater_lo;
> + uint32_t fwater_hi;
> + int cwm, srwm = -1;
> + int planea_wm, planeb_wm;
> + struct intel_crtc *enabled = NULL;
enabled is used to check if cxsr can be enabled, IMHO it would be good
to take a bool variable for same & replace use of enabled with
respective intel_crtc variable.
if it seems complex then this approach is also good with me.
> +
> + crtc->wm.active.i9xx = crtc->config->wm.i9xx.optimal;
> +
> + crtc = intel_get_crtc_for_plane(dev_priv, 0);
> + planea_wm = crtc->wm.active.i9xx.plane_wm;
> + if (intel_crtc_active(crtc))
> + enabled = crtc;
> +
> + crtc = intel_get_crtc_for_plane(dev_priv, 1);
> + if (intel_crtc_active(crtc)) {
> if (enabled == NULL)
> enabled = crtc;
> else
> enabled = NULL;
> - } else {
> - planeb_wm = fifo_size - wm_info->guard_size;
> - if (planeb_wm > (long)wm_info->max_wm)
> - planeb_wm = wm_info->max_wm;
> }
> + planeb_wm = crtc->wm.active.i9xx.plane_wm;
>
> DRM_DEBUG_KMS("FIFO watermarks - A: %d, B: %d\n", planea_wm, planeb_wm);
>
> - if (IS_I915GM(dev_priv) && enabled) {
> - struct drm_i915_gem_object *obj;
> -
> - obj = intel_fb_obj(enabled->base.primary->state->fb);
> -
> - /* self-refresh seems busted with untiled */
> - if (!i915_gem_object_is_tiled(obj))
> - enabled = NULL;
> - }
> + if (!enabled->wm.active.i9xx.cxsr)
> + enabled = NULL;
>
> /*
> * Overlay gets an aggressive default since video jitter is bad.
> @@ -2317,31 +2382,8 @@ static void i9xx_update_wm(struct intel_crtc *unused_crtc)
> intel_set_memory_cxsr(dev_priv, false);
>
> /* Calc sr entries for one plane configs */
> - if (HAS_FW_BLC(dev_priv) && enabled) {
> - /* self-refresh has much higher latency */
> - static const int sr_latency_ns = 6000;
> - const struct drm_display_mode *adjusted_mode =
> - &enabled->config->base.adjusted_mode;
> - const struct drm_framebuffer *fb =
> - enabled->base.primary->state->fb;
> - int clock = adjusted_mode->crtc_clock;
> - int htotal = adjusted_mode->crtc_htotal;
> - int hdisplay = enabled->config->pipe_src_w;
> - int cpp;
> - int entries;
> -
> - if (IS_I915GM(dev_priv) || IS_I945GM(dev_priv))
> - cpp = 4;
> - else
> - cpp = fb->format->cpp[0];
> -
> - entries = intel_wm_method2(clock, htotal, hdisplay, cpp,
> - sr_latency_ns / 100);
> - entries = DIV_ROUND_UP(entries, wm_info->cacheline_size);
> - DRM_DEBUG_KMS("self-refresh entries: %d\n", entries);
> - srwm = wm_info->fifo_size - entries;
> - if (srwm < 0)
> - srwm = 1;
> + if (enabled) {
> + srwm = enabled->wm.active.i9xx.sr.plane;
>
> if (IS_I945G(dev_priv) || IS_I945GM(dev_priv))
> I915_WRITE(FW_BLC_SELF,
> @@ -2367,23 +2409,18 @@ static void i9xx_update_wm(struct intel_crtc *unused_crtc)
> intel_set_memory_cxsr(dev_priv, true);
> }
>
> -static void i845_update_wm(struct intel_crtc *unused_crtc)
> +static void i845_update_wm(struct intel_crtc *crtc)
> {
> - struct drm_i915_private *dev_priv = to_i915(unused_crtc->base.dev);
> - struct intel_crtc *crtc;
> - const struct drm_display_mode *adjusted_mode;
> + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> uint32_t fwater_lo;
> int planea_wm;
>
> - crtc = single_enabled_crtc(dev_priv);
> - if (crtc == NULL)
earlier we were checking if enabled crtc's != 1 then return, but here
logic got changed.
Oh it seems ok, as platform calling i845 will have only one crtc.
-Mahesh
> + if (!intel_crtc_active(crtc))
> return;
>
> - adjusted_mode = &crtc->config->base.adjusted_mode;
> - planea_wm = intel_calculate_wm(adjusted_mode->crtc_clock,
> - &i845_wm_info,
> - dev_priv->display.get_fifo_size(dev_priv, 0),
> - 4, pessimal_latency_ns);
> + crtc->wm.active.i9xx = crtc->config->wm.i9xx.optimal;
> + planea_wm = crtc->wm.active.i9xx.plane_wm;
> +
> fwater_lo = I915_READ(FW_BLC) & ~0xfff;
> fwater_lo |= (3<<8) | planea_wm;
>
> @@ -8813,9 +8850,13 @@ void intel_init_pm(struct drm_i915_private *dev_priv)
> } else if (IS_GEN4(dev_priv)) {
> dev_priv->display.update_wm = i965_update_wm;
> } else if (IS_GEN3(dev_priv)) {
> + dev_priv->display.compute_pipe_wm = i9xx_compute_pipe_wm;
> dev_priv->display.update_wm = i9xx_update_wm;
> +
> dev_priv->display.get_fifo_size = i9xx_get_fifo_size;
> } else if (IS_GEN2(dev_priv)) {
> + dev_priv->display.compute_pipe_wm = i9xx_compute_pipe_wm;
> +
> if (INTEL_INFO(dev_priv)->num_pipes == 1) {
> dev_priv->display.update_wm = i845_update_wm;
> dev_priv->display.get_fifo_size = i845_get_fifo_size;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/7] drm/i915: Calculate gen3- watermarks semi-atomically.
2017-08-08 12:51 ` [PATCH 1/7] drm/i915: Calculate gen3- watermarks semi-atomically Mahesh Kumar
@ 2017-09-07 12:39 ` Maarten Lankhorst
0 siblings, 0 replies; 14+ messages in thread
From: Maarten Lankhorst @ 2017-09-07 12:39 UTC (permalink / raw)
To: Mahesh Kumar, intel-gfx
Op 08-08-17 om 14:51 schreef Mahesh Kumar:
> Hi,
>> +
>> + struct {
>> + uint16_t plane;
> should this also be named as plane_wm, because it's again wm with SR.
> just a nitpick but not a stopper.
Ack, will change (and for the other patches in this series too).
>
>> @@ -591,6 +600,9 @@ struct intel_crtc_wm_state {
>> /* optimal watermarks */
>> struct g4x_wm_state optimal;
>> } g4x;
> new line please to match the coding style of function
+1
>> + if (!plane_state || !intel_wm_plane_visible(crtc_state, to_intel_plane_state(plane_state))) {
>> + wm_state->plane_wm = fifo_size - wm_info->guard_size;
>> + if (wm_state->plane_wm > (long)wm_info->max_wm)
>> + wm_state->plane_wm = wm_info->max_wm;
>> + } else if (HAS_FW_BLC(dev_priv)) {
> This part will be called only for (GEN > 2,) So we'll never be
> calculating wm for GEN2. but you are changing GEN2 wm also to 2 stages
> compute & update in intel_init_pm.
Well spotted, should be fixed in a bit.
>> + struct drm_framebuffer *fb = plane_state->fb;
>> + unsigned cpp = IS_GEN2(dev_priv) ? 4 : fb->format->cpp[0];
>> const struct drm_display_mode *adjusted_mode =
>> - &crtc->config->base.adjusted_mode;
>> - const struct drm_framebuffer *fb =
>> - crtc->base.primary->state->fb;
>> - int cpp;
>> + &crtc_state->base.adjusted_mode;
>> + unsigned active_crtcs;
>> + bool may_cxsr;
>>
>> - if (IS_GEN2(dev_priv))
>> - cpp = 4;
>> + if (state->modeset)
>> + active_crtcs = state->active_crtcs;
>> else
>> - cpp = fb->format->cpp[0];
>> + active_crtcs = dev_priv->active_crtcs;
>>
>> - planea_wm = intel_calculate_wm(adjusted_mode->crtc_clock,
>> - wm_info, fifo_size, cpp,
>> - pessimal_latency_ns);
>> - enabled = crtc;
>> - } else {
>> - planea_wm = fifo_size - wm_info->guard_size;
>> - if (planea_wm > (long)wm_info->max_wm)
>> - planea_wm = wm_info->max_wm;
>> + may_cxsr = active_crtcs == drm_crtc_mask(&crtc->base);
> It would be good to have a comment stating if only one CRTC is enabled
> we can go to cxsr.
> we can use hweight32() function to check if only one CRTC is enabled
> (not mandatory).
Added the comment, I think this makes it more clear for now.
>> + DRM_DEBUG_KMS("self-refresh entries: %ld\n", entries);
>> +
>> + if (wm_info->fifo_size >= entries) {
> Earlier if (fifo_size < entries) we were assigning srwm = 1 & not
> disabling cxsr, Is there any specific Bspec documentation to not enable
> cxsr if fifo_size < entries ?
Not sure, but it felt like this was an overflow.. Maybe I can find something on bspec about it.
>> + wm_state->cxsr = true;
>> + wm_state->sr.plane = wm_info->fifo_size - entries;
>> + } else
>> + may_cxsr = false;
> may_cxsr is not used later, so no need to overwrite the value.
True, I dump wm_state->cxsr now.
>> - if (IS_GEN2(dev_priv))
>> - cpp = 4;
>> + if (plane == PLANE_A)
> here plane is of type intel_plane but you are comparing with enum plane,
> I think you meant plane->plane
Right, nicely caught! Compiler didn't warn because PLANE_A == 0, it did complain if I changed it to PLANE_B:
drivers/gpu/drm/i915/intel_pm.c: In function ‘i9xx_wm_get_hw_state’:
drivers/gpu/drm/i915/intel_pm.c:2351:13: error: comparison between pointer and integer [-Werror]
if (plane == PLANE_B)
>> + wm_state->plane_wm = planea_wm;
>> else
>> - cpp = fb->format->cpp[0];
>> + wm_state->plane_wm = planeb_wm;
>> +
>> + crtc->wm.active.i9xx = *wm_state;
>> + }
>> +}
>>
>> - planeb_wm = intel_calculate_wm(adjusted_mode->crtc_clock,
>> - wm_info, fifo_size, cpp,
>> - pessimal_latency_ns);
>> +static void i9xx_update_wm(struct intel_crtc *crtc)
>> +{
>> + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>> + uint32_t fwater_lo;
>> + uint32_t fwater_hi;
>> + int cwm, srwm = -1;
>> + int planea_wm, planeb_wm;
>> + struct intel_crtc *enabled = NULL;
> enabled is used to check if cxsr can be enabled, IMHO it would be good
> to take a bool variable for same & replace use of enabled with
> respective intel_crtc variable.
> if it seems complex then this approach is also good with me.
Yeah agreed, it should at least look at wm_state->cxsr. I need to keep the enabled check
in case pipe A gets enabled after pipe B. In that case watermarks on A are updated before
B, and we should really kill CXSR already at that point..
I'll just add the cxsr check, that should do it. :)
>> +
>> + crtc->wm.active.i9xx = crtc->config->wm.i9xx.optimal;
>> +
>> + crtc = intel_get_crtc_for_plane(dev_priv, 0);
>> + planea_wm = crtc->wm.active.i9xx.plane_wm;
>> + if (intel_crtc_active(crtc))
>> + enabled = crtc;
>> +
>> + crtc = intel_get_crtc_for_plane(dev_priv, 1);
>> + if (intel_crtc_active(crtc)) {
>> if (enabled == NULL)
>> enabled = crtc;
>> else
>> enabled = NULL;
>> - } else {
>> - planeb_wm = fifo_size - wm_info->guard_size;
>> - if (planeb_wm > (long)wm_info->max_wm)
>> - planeb_wm = wm_info->max_wm;
>> }
>> + planeb_wm = crtc->wm.active.i9xx.plane_wm;
>>
>> DRM_DEBUG_KMS("FIFO watermarks - A: %d, B: %d\n", planea_wm, planeb_wm);
>>
>> - if (IS_I915GM(dev_priv) && enabled) {
>> - struct drm_i915_gem_object *obj;
>> -
>> - obj = intel_fb_obj(enabled->base.primary->state->fb);
>> -
>> - /* self-refresh seems busted with untiled */
>> - if (!i915_gem_object_is_tiled(obj))
>> - enabled = NULL;
>> - }
>> + if (!enabled->wm.active.i9xx.cxsr)
>> + enabled = NULL;
>>
>> /*
>> * Overlay gets an aggressive default since video jitter is bad.
>> @@ -2317,31 +2382,8 @@ static void i9xx_update_wm(struct intel_crtc *unused_crtc)
>> intel_set_memory_cxsr(dev_priv, false);
>>
>> /* Calc sr entries for one plane configs */
>> - if (HAS_FW_BLC(dev_priv) && enabled) {
>> - /* self-refresh has much higher latency */
>> - static const int sr_latency_ns = 6000;
>> - const struct drm_display_mode *adjusted_mode =
>> - &enabled->config->base.adjusted_mode;
>> - const struct drm_framebuffer *fb =
>> - enabled->base.primary->state->fb;
>> - int clock = adjusted_mode->crtc_clock;
>> - int htotal = adjusted_mode->crtc_htotal;
>> - int hdisplay = enabled->config->pipe_src_w;
>> - int cpp;
>> - int entries;
>> -
>> - if (IS_I915GM(dev_priv) || IS_I945GM(dev_priv))
>> - cpp = 4;
>> - else
>> - cpp = fb->format->cpp[0];
>> -
>> - entries = intel_wm_method2(clock, htotal, hdisplay, cpp,
>> - sr_latency_ns / 100);
>> - entries = DIV_ROUND_UP(entries, wm_info->cacheline_size);
>> - DRM_DEBUG_KMS("self-refresh entries: %d\n", entries);
>> - srwm = wm_info->fifo_size - entries;
>> - if (srwm < 0)
>> - srwm = 1;
>> + if (enabled) {
>> + srwm = enabled->wm.active.i9xx.sr.plane;
>>
>> if (IS_I945G(dev_priv) || IS_I945GM(dev_priv))
>> I915_WRITE(FW_BLC_SELF,
>> @@ -2367,23 +2409,18 @@ static void i9xx_update_wm(struct intel_crtc *unused_crtc)
>> intel_set_memory_cxsr(dev_priv, true);
>> }
>>
>> -static void i845_update_wm(struct intel_crtc *unused_crtc)
>> +static void i845_update_wm(struct intel_crtc *crtc)
>> {
>> - struct drm_i915_private *dev_priv = to_i915(unused_crtc->base.dev);
>> - struct intel_crtc *crtc;
>> - const struct drm_display_mode *adjusted_mode;
>> + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>> uint32_t fwater_lo;
>> int planea_wm;
>>
>> - crtc = single_enabled_crtc(dev_priv);
>> - if (crtc == NULL)
> earlier we were checking if enabled crtc's != 1 then return, but here
> logic got changed.
> Oh it seems ok, as platform calling i845 will have only one crtc.
Indeed. :)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2017-09-07 12:39 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-07 10:48 [PATCH 0/7] drm/i915: Convert gen4- watermarks to atomic Maarten Lankhorst
2017-08-07 10:48 ` [PATCH 1/7] drm/i915: Calculate gen3- watermarks semi-atomically Maarten Lankhorst
2017-08-08 11:00 ` [PATCH] drm/i915: Calculate gen3- watermarks semi-atomically, v2 Maarten Lankhorst
2017-08-08 12:51 ` [PATCH 1/7] drm/i915: Calculate gen3- watermarks semi-atomically Mahesh Kumar
2017-09-07 12:39 ` Maarten Lankhorst
2017-08-07 10:48 ` [PATCH 2/7] drm/i915: Program gen3- watermarks atomically Maarten Lankhorst
2017-08-07 10:48 ` [PATCH 3/7] drm/i915: Convert pineview watermarks to atomic, v2 Maarten Lankhorst
2017-08-07 10:48 ` [PATCH 4/7] drm/i915: Calculate gen4 watermarks semiatomically Maarten Lankhorst
2017-08-07 10:48 ` [PATCH 5/7] drm/i915: Program gen4 watermarks atomically Maarten Lankhorst
2017-08-07 10:48 ` [PATCH 6/7] drm/i915: Kill off intel_crtc_active Maarten Lankhorst
2017-08-07 10:48 ` [PATCH 7/7] drm/i915: Rip out legacy watermark infrastructure Maarten Lankhorst
2017-08-07 12:17 ` ✗ Fi.CI.BAT: failure for drm/i915: Convert gen4- watermarks to atomic. (rev2) Patchwork
2017-08-08 9:26 ` Maarten Lankhorst
2017-08-08 11:19 ` ✗ Fi.CI.BAT: failure for drm/i915: Convert gen4- watermarks to atomic. (rev3) Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.