* [Intel-gfx] [PATCH 0/7] drm/i915: Fix up TGL+ SAGV watermarks
@ 2021-02-26 15:31 Ville Syrjala
2021-02-26 15:31 ` [Intel-gfx] [PATCH 1/7] drm/i915: Fix TGL+ plane SAGV watermark programming Ville Syrjala
` (8 more replies)
0 siblings, 9 replies; 17+ messages in thread
From: Ville Syrjala @ 2021-02-26 15:31 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Had a quick glance as the TGL+ watermark code and noticed a
bunch of fail with the SAGV wm handling. Here's an attempt
at fixing it.
Git branch here:
git://github.com/vsyrjala/linux.git tgl_sagv_wm_fixes
Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Ville Syrjälä (7):
drm/i915: Fix TGL+ plane SAGV watermark programming
drm/i915: Zero out SAGV wm when we don't have enough DDB for it
drm/i915: Print wm changes if sagv_wm0 changes
drm/i915: Stuff SAGV watermark into a sub-structure
drm/i915: Introduce SAGV transtion watermark
drm/i915: Check tgl+ SAGV watermarks properly
drm/i915: Clean up verify_wm_state()
drivers/gpu/drm/i915/display/intel_display.c | 126 ++++---------
drivers/gpu/drm/i915/display/intel_display.h | 5 -
.../drm/i915/display/intel_display_types.h | 5 +-
drivers/gpu/drm/i915/intel_pm.c | 172 +++++++++++-------
drivers/gpu/drm/i915/intel_pm.h | 5 +
5 files changed, 150 insertions(+), 163 deletions(-)
--
2.26.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Intel-gfx] [PATCH 1/7] drm/i915: Fix TGL+ plane SAGV watermark programming
2021-02-26 15:31 [Intel-gfx] [PATCH 0/7] drm/i915: Fix up TGL+ SAGV watermarks Ville Syrjala
@ 2021-02-26 15:31 ` Ville Syrjala
2021-03-01 8:38 ` Lisovskiy, Stanislav
2021-02-26 15:31 ` [Intel-gfx] [PATCH 2/7] drm/i915: Zero out SAGV wm when we don't have enough DDB for it Ville Syrjala
` (7 subsequent siblings)
8 siblings, 1 reply; 17+ messages in thread
From: Ville Syrjala @ 2021-02-26 15:31 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
When we switch between SAGV on vs. off we need to reprogram all
plane wateramrks accordingly. Currently skl_wm_add_affected_planes()
totally ignores the SAGV watermark and just assumes we will use
the normal WM0.
Fix this by utilizing skl_plane_wm_level() which picks the
correct watermark based on use_sagv_wm. Thus we will force
an update on all the planes whose watermark registers need
to be reprogrammed.
Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_pm.c | 60 ++++++++++++++++++++-------------
1 file changed, 37 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 8cc67f9c4e58..2d0e3e7f11b8 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4748,11 +4748,10 @@ icl_get_total_relative_data_rate(struct intel_atomic_state *state,
}
static const struct skl_wm_level *
-skl_plane_wm_level(const struct intel_crtc_state *crtc_state,
+skl_plane_wm_level(const struct skl_pipe_wm *pipe_wm,
enum plane_id plane_id,
int level)
{
- const struct skl_pipe_wm *pipe_wm = &crtc_state->wm.skl.optimal;
const struct skl_plane_wm *wm = &pipe_wm->planes[plane_id];
if (level == 0 && pipe_wm->use_sagv_wm)
@@ -5572,21 +5571,17 @@ void skl_write_plane_wm(struct intel_plane *plane,
int level, max_level = ilk_wm_max_level(dev_priv);
enum plane_id plane_id = plane->id;
enum pipe pipe = plane->pipe;
- const struct skl_plane_wm *wm =
- &crtc_state->wm.skl.optimal.planes[plane_id];
+ const struct skl_pipe_wm *pipe_wm = &crtc_state->wm.skl.optimal;
+ const struct skl_plane_wm *wm = &pipe_wm->planes[plane_id];
const struct skl_ddb_entry *ddb_y =
&crtc_state->wm.skl.plane_ddb_y[plane_id];
const struct skl_ddb_entry *ddb_uv =
&crtc_state->wm.skl.plane_ddb_uv[plane_id];
- for (level = 0; level <= max_level; level++) {
- const struct skl_wm_level *wm_level;
-
- wm_level = skl_plane_wm_level(crtc_state, plane_id, level);
-
+ for (level = 0; level <= max_level; level++)
skl_write_wm_level(dev_priv, PLANE_WM(pipe, plane_id, level),
- wm_level);
- }
+ skl_plane_wm_level(pipe_wm, plane_id, level));
+
skl_write_wm_level(dev_priv, PLANE_WM_TRANS(pipe, plane_id),
&wm->trans_wm);
@@ -5612,19 +5607,15 @@ void skl_write_cursor_wm(struct intel_plane *plane,
int level, max_level = ilk_wm_max_level(dev_priv);
enum plane_id plane_id = plane->id;
enum pipe pipe = plane->pipe;
- const struct skl_plane_wm *wm =
- &crtc_state->wm.skl.optimal.planes[plane_id];
+ const struct skl_pipe_wm *pipe_wm = &crtc_state->wm.skl.optimal;
+ const struct skl_plane_wm *wm = &pipe_wm->planes[plane_id];
const struct skl_ddb_entry *ddb =
&crtc_state->wm.skl.plane_ddb_y[plane_id];
- for (level = 0; level <= max_level; level++) {
- const struct skl_wm_level *wm_level;
-
- wm_level = skl_plane_wm_level(crtc_state, plane_id, level);
-
+ for (level = 0; level <= max_level; level++)
skl_write_wm_level(dev_priv, CUR_WM(pipe, level),
- wm_level);
- }
+ skl_plane_wm_level(pipe_wm, plane_id, level));
+
skl_write_wm_level(dev_priv, CUR_WM_TRANS(pipe), &wm->trans_wm);
skl_ddb_entry_write(dev_priv, CUR_BUF_CFG(pipe), ddb);
@@ -5964,6 +5955,29 @@ skl_print_wm_changes(struct intel_atomic_state *state)
}
}
+static bool skl_plane_selected_wm_equals(struct intel_plane *plane,
+ const struct skl_pipe_wm *old_pipe_wm,
+ const struct skl_pipe_wm *new_pipe_wm)
+{
+ const struct skl_plane_wm *old_wm = &old_pipe_wm->planes[plane->id];
+ const struct skl_plane_wm *new_wm = &new_pipe_wm->planes[plane->id];
+ struct drm_i915_private *i915 = to_i915(plane->base.dev);
+ int level, max_level = ilk_wm_max_level(i915);
+
+ for (level = 0; level <= max_level; level++) {
+ /*
+ * We don't check uv_wm as the hardware doesn't actually
+ * use it. It only gets used for calculating the required
+ * ddb allocation.
+ */
+ if (!skl_wm_level_equals(skl_plane_wm_level(old_pipe_wm, level, plane->id),
+ skl_plane_wm_level(new_pipe_wm, level, plane->id)))
+ return false;
+ }
+
+ return skl_wm_level_equals(&old_wm->trans_wm, &new_wm->trans_wm);
+}
+
/*
* To make sure the cursor watermark registers are always consistent
* with our computed state the following scenario needs special
@@ -6009,9 +6023,9 @@ static int skl_wm_add_affected_planes(struct intel_atomic_state *state,
* with the software state.
*/
if (!drm_atomic_crtc_needs_modeset(&new_crtc_state->uapi) &&
- skl_plane_wm_equals(dev_priv,
- &old_crtc_state->wm.skl.optimal.planes[plane_id],
- &new_crtc_state->wm.skl.optimal.planes[plane_id]))
+ skl_plane_selected_wm_equals(plane,
+ &old_crtc_state->wm.skl.optimal,
+ &new_crtc_state->wm.skl.optimal))
continue;
plane_state = intel_atomic_get_plane_state(state, plane);
--
2.26.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Intel-gfx] [PATCH 2/7] drm/i915: Zero out SAGV wm when we don't have enough DDB for it
2021-02-26 15:31 [Intel-gfx] [PATCH 0/7] drm/i915: Fix up TGL+ SAGV watermarks Ville Syrjala
2021-02-26 15:31 ` [Intel-gfx] [PATCH 1/7] drm/i915: Fix TGL+ plane SAGV watermark programming Ville Syrjala
@ 2021-02-26 15:31 ` Ville Syrjala
2021-03-01 8:42 ` Lisovskiy, Stanislav
2021-02-26 15:32 ` [Intel-gfx] [PATCH 3/7] drm/i915: Print wm changes if sagv_wm0 changes Ville Syrjala
` (6 subsequent siblings)
8 siblings, 1 reply; 17+ messages in thread
From: Ville Syrjala @ 2021-02-26 15:31 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Let's handle the SAGV WM0 more like the other wm levels and just
totally zero it out when we don't have the DDB space to back it
up.
Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_pm.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 2d0e3e7f11b8..c341fa957884 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3921,12 +3921,10 @@ static bool tgl_crtc_can_enable_sagv(const struct intel_crtc_state *crtc_state)
return true;
for_each_plane_id_on_crtc(crtc, plane_id) {
- const struct skl_ddb_entry *plane_alloc =
- &crtc_state->wm.skl.plane_ddb_y[plane_id];
const struct skl_plane_wm *wm =
&crtc_state->wm.skl.optimal.planes[plane_id];
- if (skl_ddb_entry_size(plane_alloc) < wm->sagv_wm0.min_ddb_alloc)
+ if (wm->wm[0].plane_en && !wm->sagv_wm0.plane_en)
return false;
}
@@ -4957,8 +4955,8 @@ skl_allocate_plane_ddb(struct intel_atomic_state *state,
}
/*
- * Go back and disable the transition watermark if it turns out we
- * don't have enough DDB blocks for it.
+ * Go back and disable the transition and SAGV watermarks
+ * if it turns out we don't have enough DDB blocks for them.
*/
for_each_plane_id_on_crtc(crtc, plane_id) {
struct skl_plane_wm *wm =
@@ -4966,6 +4964,9 @@ skl_allocate_plane_ddb(struct intel_atomic_state *state,
if (wm->trans_wm.plane_res_b >= total[plane_id])
memset(&wm->trans_wm, 0, sizeof(wm->trans_wm));
+
+ if (wm->sagv_wm0.plane_res_b >= total[plane_id])
+ memset(&wm->sagv_wm0, 0, sizeof(wm->sagv_wm0));
}
return 0;
--
2.26.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Intel-gfx] [PATCH 3/7] drm/i915: Print wm changes if sagv_wm0 changes
2021-02-26 15:31 [Intel-gfx] [PATCH 0/7] drm/i915: Fix up TGL+ SAGV watermarks Ville Syrjala
2021-02-26 15:31 ` [Intel-gfx] [PATCH 1/7] drm/i915: Fix TGL+ plane SAGV watermark programming Ville Syrjala
2021-02-26 15:31 ` [Intel-gfx] [PATCH 2/7] drm/i915: Zero out SAGV wm when we don't have enough DDB for it Ville Syrjala
@ 2021-02-26 15:32 ` Ville Syrjala
2021-03-01 9:14 ` Lisovskiy, Stanislav
2021-02-26 15:32 ` [Intel-gfx] [PATCH 4/7] drm/i915: Stuff SAGV watermark into a sub-structure Ville Syrjala
` (5 subsequent siblings)
8 siblings, 1 reply; 17+ messages in thread
From: Ville Syrjala @ 2021-02-26 15:32 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Let's consider sagv_wm0 as well when deciding whether to dump
out the watermark changes.
Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_pm.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index c341fa957884..06c54adc609a 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5647,7 +5647,8 @@ static bool skl_plane_wm_equals(struct drm_i915_private *dev_priv,
return false;
}
- return skl_wm_level_equals(&wm1->trans_wm, &wm2->trans_wm);
+ return skl_wm_level_equals(&wm1->trans_wm, &wm2->trans_wm) &&
+ skl_wm_level_equals(&wm1->sagv_wm0, &wm2->sagv_wm0);
}
static bool skl_ddb_entries_overlap(const struct skl_ddb_entry *a,
--
2.26.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Intel-gfx] [PATCH 4/7] drm/i915: Stuff SAGV watermark into a sub-structure
2021-02-26 15:31 [Intel-gfx] [PATCH 0/7] drm/i915: Fix up TGL+ SAGV watermarks Ville Syrjala
` (2 preceding siblings ...)
2021-02-26 15:32 ` [Intel-gfx] [PATCH 3/7] drm/i915: Print wm changes if sagv_wm0 changes Ville Syrjala
@ 2021-02-26 15:32 ` Ville Syrjala
2021-03-01 9:17 ` Lisovskiy, Stanislav
2021-02-26 15:32 ` [Intel-gfx] [PATCH 5/7] drm/i915: Introduce SAGV transtion watermark Ville Syrjala
` (4 subsequent siblings)
8 siblings, 1 reply; 17+ messages in thread
From: Ville Syrjala @ 2021-02-26 15:32 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
We'll want a SAGV transition watermark as well. Prepare
for that by collecting SAGV wm0 into a sub-strcture.
Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/display/intel_display.c | 4 +--
.../drm/i915/display/intel_display_types.h | 4 ++-
drivers/gpu/drm/i915/intel_pm.c | 30 +++++++++----------
3 files changed, 20 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index d0da88751c72..718e66f49332 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -9387,7 +9387,7 @@ static void verify_wm_state(struct intel_crtc *crtc,
if (skl_wm_level_equals(&hw_plane_wm->wm[level],
&sw_plane_wm->wm[level]) ||
(level == 0 && skl_wm_level_equals(&hw_plane_wm->wm[level],
- &sw_plane_wm->sagv_wm0)))
+ &sw_plane_wm->sagv.wm0)))
continue;
drm_err(&dev_priv->drm,
@@ -9444,7 +9444,7 @@ static void verify_wm_state(struct intel_crtc *crtc,
if (skl_wm_level_equals(&hw_plane_wm->wm[level],
&sw_plane_wm->wm[level]) ||
(level == 0 && skl_wm_level_equals(&hw_plane_wm->wm[level],
- &sw_plane_wm->sagv_wm0)))
+ &sw_plane_wm->sagv.wm0)))
continue;
drm_err(&dev_priv->drm,
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 1a76e1d9de7a..6321cd3df81e 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -732,7 +732,9 @@ struct skl_plane_wm {
struct skl_wm_level wm[8];
struct skl_wm_level uv_wm[8];
struct skl_wm_level trans_wm;
- struct skl_wm_level sagv_wm0;
+ struct {
+ struct skl_wm_level wm0;
+ } sagv;
bool is_planar;
};
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 06c54adc609a..a1591d9189a0 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3924,7 +3924,7 @@ static bool tgl_crtc_can_enable_sagv(const struct intel_crtc_state *crtc_state)
const struct skl_plane_wm *wm =
&crtc_state->wm.skl.optimal.planes[plane_id];
- if (wm->wm[0].plane_en && !wm->sagv_wm0.plane_en)
+ if (wm->wm[0].plane_en && !wm->sagv.wm0.plane_en)
return false;
}
@@ -4753,7 +4753,7 @@ skl_plane_wm_level(const struct skl_pipe_wm *pipe_wm,
const struct skl_plane_wm *wm = &pipe_wm->planes[plane_id];
if (level == 0 && pipe_wm->use_sagv_wm)
- return &wm->sagv_wm0;
+ return &wm->sagv.wm0;
return &wm->wm[level];
}
@@ -4965,8 +4965,8 @@ skl_allocate_plane_ddb(struct intel_atomic_state *state,
if (wm->trans_wm.plane_res_b >= total[plane_id])
memset(&wm->trans_wm, 0, sizeof(wm->trans_wm));
- if (wm->sagv_wm0.plane_res_b >= total[plane_id])
- memset(&wm->sagv_wm0, 0, sizeof(wm->sagv_wm0));
+ if (wm->sagv.wm0.plane_res_b >= total[plane_id])
+ memset(&wm->sagv.wm0, 0, sizeof(wm->sagv.wm0));
}
return 0;
@@ -5316,7 +5316,7 @@ static void tgl_compute_sagv_wm(const struct intel_crtc_state *crtc_state,
struct skl_plane_wm *plane_wm)
{
struct drm_i915_private *dev_priv = to_i915(crtc_state->uapi.crtc->dev);
- struct skl_wm_level *sagv_wm = &plane_wm->sagv_wm0;
+ struct skl_wm_level *sagv_wm = &plane_wm->sagv.wm0;
struct skl_wm_level *levels = plane_wm->wm;
unsigned int latency = dev_priv->wm.skl_latency[0] + dev_priv->sagv_block_time_us;
@@ -5648,7 +5648,7 @@ static bool skl_plane_wm_equals(struct drm_i915_private *dev_priv,
}
return skl_wm_level_equals(&wm1->trans_wm, &wm2->trans_wm) &&
- skl_wm_level_equals(&wm1->sagv_wm0, &wm2->sagv_wm0);
+ skl_wm_level_equals(&wm1->sagv.wm0, &wm2->sagv.wm0);
}
static bool skl_ddb_entries_overlap(const struct skl_ddb_entry *a,
@@ -5886,13 +5886,13 @@ skl_print_wm_changes(struct intel_atomic_state *state)
enast(old_wm->wm[4].plane_en), enast(old_wm->wm[5].plane_en),
enast(old_wm->wm[6].plane_en), enast(old_wm->wm[7].plane_en),
enast(old_wm->trans_wm.plane_en),
- enast(old_wm->sagv_wm0.plane_en),
+ enast(old_wm->sagv.wm0.plane_en),
enast(new_wm->wm[0].plane_en), enast(new_wm->wm[1].plane_en),
enast(new_wm->wm[2].plane_en), enast(new_wm->wm[3].plane_en),
enast(new_wm->wm[4].plane_en), enast(new_wm->wm[5].plane_en),
enast(new_wm->wm[6].plane_en), enast(new_wm->wm[7].plane_en),
enast(new_wm->trans_wm.plane_en),
- enast(new_wm->sagv_wm0.plane_en));
+ enast(new_wm->sagv.wm0.plane_en));
drm_dbg_kms(&dev_priv->drm,
"[PLANE:%d:%s] lines %c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d"
@@ -5907,7 +5907,7 @@ skl_print_wm_changes(struct intel_atomic_state *state)
enast(old_wm->wm[6].ignore_lines), old_wm->wm[6].plane_res_l,
enast(old_wm->wm[7].ignore_lines), old_wm->wm[7].plane_res_l,
enast(old_wm->trans_wm.ignore_lines), old_wm->trans_wm.plane_res_l,
- enast(old_wm->sagv_wm0.ignore_lines), old_wm->sagv_wm0.plane_res_l,
+ enast(old_wm->sagv.wm0.ignore_lines), old_wm->sagv.wm0.plane_res_l,
enast(new_wm->wm[0].ignore_lines), new_wm->wm[0].plane_res_l,
enast(new_wm->wm[1].ignore_lines), new_wm->wm[1].plane_res_l,
@@ -5918,7 +5918,7 @@ skl_print_wm_changes(struct intel_atomic_state *state)
enast(new_wm->wm[6].ignore_lines), new_wm->wm[6].plane_res_l,
enast(new_wm->wm[7].ignore_lines), new_wm->wm[7].plane_res_l,
enast(new_wm->trans_wm.ignore_lines), new_wm->trans_wm.plane_res_l,
- enast(new_wm->sagv_wm0.ignore_lines), new_wm->sagv_wm0.plane_res_l);
+ enast(new_wm->sagv.wm0.ignore_lines), new_wm->sagv.wm0.plane_res_l);
drm_dbg_kms(&dev_priv->drm,
"[PLANE:%d:%s] blocks %4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d"
@@ -5929,13 +5929,13 @@ skl_print_wm_changes(struct intel_atomic_state *state)
old_wm->wm[4].plane_res_b, old_wm->wm[5].plane_res_b,
old_wm->wm[6].plane_res_b, old_wm->wm[7].plane_res_b,
old_wm->trans_wm.plane_res_b,
- old_wm->sagv_wm0.plane_res_b,
+ old_wm->sagv.wm0.plane_res_b,
new_wm->wm[0].plane_res_b, new_wm->wm[1].plane_res_b,
new_wm->wm[2].plane_res_b, new_wm->wm[3].plane_res_b,
new_wm->wm[4].plane_res_b, new_wm->wm[5].plane_res_b,
new_wm->wm[6].plane_res_b, new_wm->wm[7].plane_res_b,
new_wm->trans_wm.plane_res_b,
- new_wm->sagv_wm0.plane_res_b);
+ new_wm->sagv.wm0.plane_res_b);
drm_dbg_kms(&dev_priv->drm,
"[PLANE:%d:%s] min_ddb %4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d"
@@ -5946,13 +5946,13 @@ skl_print_wm_changes(struct intel_atomic_state *state)
old_wm->wm[4].min_ddb_alloc, old_wm->wm[5].min_ddb_alloc,
old_wm->wm[6].min_ddb_alloc, old_wm->wm[7].min_ddb_alloc,
old_wm->trans_wm.min_ddb_alloc,
- old_wm->sagv_wm0.min_ddb_alloc,
+ old_wm->sagv.wm0.min_ddb_alloc,
new_wm->wm[0].min_ddb_alloc, new_wm->wm[1].min_ddb_alloc,
new_wm->wm[2].min_ddb_alloc, new_wm->wm[3].min_ddb_alloc,
new_wm->wm[4].min_ddb_alloc, new_wm->wm[5].min_ddb_alloc,
new_wm->wm[6].min_ddb_alloc, new_wm->wm[7].min_ddb_alloc,
new_wm->trans_wm.min_ddb_alloc,
- new_wm->sagv_wm0.min_ddb_alloc);
+ new_wm->sagv.wm0.min_ddb_alloc);
}
}
}
@@ -6189,7 +6189,7 @@ void skl_pipe_wm_get_hw_state(struct intel_crtc *crtc,
}
if (INTEL_GEN(dev_priv) >= 12)
- wm->sagv_wm0 = wm->wm[0];
+ wm->sagv.wm0 = wm->wm[0];
if (plane_id != PLANE_CURSOR)
val = intel_uncore_read(&dev_priv->uncore, PLANE_WM_TRANS(pipe, plane_id));
--
2.26.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Intel-gfx] [PATCH 5/7] drm/i915: Introduce SAGV transtion watermark
2021-02-26 15:31 [Intel-gfx] [PATCH 0/7] drm/i915: Fix up TGL+ SAGV watermarks Ville Syrjala
` (3 preceding siblings ...)
2021-02-26 15:32 ` [Intel-gfx] [PATCH 4/7] drm/i915: Stuff SAGV watermark into a sub-structure Ville Syrjala
@ 2021-02-26 15:32 ` Ville Syrjala
2021-03-01 9:21 ` Lisovskiy, Stanislav
2021-02-26 15:32 ` [Intel-gfx] [PATCH 6/7] drm/i915: Check tgl+ SAGV watermarks properly Ville Syrjala
` (3 subsequent siblings)
8 siblings, 1 reply; 17+ messages in thread
From: Ville Syrjala @ 2021-02-26 15:32 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Seems to me that if we calculate WM0 using the bumped up SAGV latency
we need to calculate the transition watermark accordingly. Track it
alongside the other watermarks.
Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/display/intel_display.c | 4 +-
.../drm/i915/display/intel_display_types.h | 1 +
drivers/gpu/drm/i915/intel_pm.c | 94 ++++++++++++-------
3 files changed, 65 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 718e66f49332..e34e5a9da5c1 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -9459,7 +9459,9 @@ static void verify_wm_state(struct intel_crtc *crtc,
}
if (!skl_wm_level_equals(&hw_plane_wm->trans_wm,
- &sw_plane_wm->trans_wm)) {
+ &sw_plane_wm->trans_wm) &&
+ !skl_wm_level_equals(&hw_plane_wm->trans_wm,
+ &sw_plane_wm->sagv.trans_wm)) {
drm_err(&dev_priv->drm,
"mismatch in trans WM pipe %c cursor (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n",
pipe_name(pipe),
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 6321cd3df81e..e2365f2d07cc 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -734,6 +734,7 @@ struct skl_plane_wm {
struct skl_wm_level trans_wm;
struct {
struct skl_wm_level wm0;
+ struct skl_wm_level trans_wm;
} sagv;
bool is_planar;
};
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a1591d9189a0..9d9ba63fc395 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4758,6 +4758,18 @@ skl_plane_wm_level(const struct skl_pipe_wm *pipe_wm,
return &wm->wm[level];
}
+static const struct skl_wm_level *
+skl_plane_trans_wm(const struct skl_pipe_wm *pipe_wm,
+ enum plane_id plane_id)
+{
+ const struct skl_plane_wm *wm = &pipe_wm->planes[plane_id];
+
+ if (pipe_wm->use_sagv_wm)
+ return &wm->sagv.trans_wm;
+
+ return &wm->trans_wm;
+}
+
static int
skl_allocate_plane_ddb(struct intel_atomic_state *state,
struct intel_crtc *crtc)
@@ -4967,6 +4979,9 @@ skl_allocate_plane_ddb(struct intel_atomic_state *state,
if (wm->sagv.wm0.plane_res_b >= total[plane_id])
memset(&wm->sagv.wm0, 0, sizeof(wm->sagv.wm0));
+
+ if (wm->sagv.trans_wm.plane_res_b >= total[plane_id])
+ memset(&wm->sagv.trans_wm, 0, sizeof(wm->sagv.trans_wm));
}
return 0;
@@ -5325,12 +5340,11 @@ static void tgl_compute_sagv_wm(const struct intel_crtc_state *crtc_state,
sagv_wm);
}
-static void skl_compute_transition_wm(const struct intel_crtc_state *crtc_state,
- const struct skl_wm_params *wp,
- struct skl_plane_wm *wm)
+static void skl_compute_transition_wm(struct drm_i915_private *dev_priv,
+ struct skl_wm_level *trans_wm,
+ const struct skl_wm_level *wm0,
+ const struct skl_wm_params *wp)
{
- struct drm_device *dev = crtc_state->uapi.crtc->dev;
- const struct drm_i915_private *dev_priv = to_i915(dev);
u16 trans_min, trans_amount, trans_y_tile_min;
u16 wm0_sel_res_b, trans_offset_b, res_blocks;
@@ -5368,7 +5382,7 @@ static void skl_compute_transition_wm(const struct intel_crtc_state *crtc_state,
* Result Blocks is Result Blocks minus 1 and it should work for the
* current platforms.
*/
- wm0_sel_res_b = wm->wm[0].plane_res_b - 1;
+ wm0_sel_res_b = wm0->plane_res_b - 1;
if (wp->y_tiled) {
trans_y_tile_min =
@@ -5384,8 +5398,8 @@ static void skl_compute_transition_wm(const struct intel_crtc_state *crtc_state,
* computing the DDB we'll come back and disable it if that
* assumption turns out to be false.
*/
- wm->trans_wm.plane_res_b = res_blocks + 1;
- wm->trans_wm.plane_en = true;
+ trans_wm->plane_res_b = res_blocks + 1;
+ trans_wm->plane_en = true;
}
static int skl_build_plane_wm_single(struct intel_crtc_state *crtc_state,
@@ -5405,10 +5419,15 @@ static int skl_build_plane_wm_single(struct intel_crtc_state *crtc_state,
skl_compute_wm_levels(crtc_state, &wm_params, wm->wm);
- if (INTEL_GEN(dev_priv) >= 12)
+ skl_compute_transition_wm(dev_priv, &wm->trans_wm,
+ &wm->wm[0], &wm_params);
+
+ if (INTEL_GEN(dev_priv) >= 12) {
tgl_compute_sagv_wm(crtc_state, &wm_params, wm);
- skl_compute_transition_wm(crtc_state, &wm_params, wm);
+ skl_compute_transition_wm(dev_priv, &wm->sagv.trans_wm,
+ &wm->sagv.wm0, &wm_params);
+ }
return 0;
}
@@ -5584,7 +5603,7 @@ void skl_write_plane_wm(struct intel_plane *plane,
skl_plane_wm_level(pipe_wm, plane_id, level));
skl_write_wm_level(dev_priv, PLANE_WM_TRANS(pipe, plane_id),
- &wm->trans_wm);
+ skl_plane_trans_wm(pipe_wm, plane_id));
if (INTEL_GEN(dev_priv) >= 11) {
skl_ddb_entry_write(dev_priv,
@@ -5609,7 +5628,6 @@ void skl_write_cursor_wm(struct intel_plane *plane,
enum plane_id plane_id = plane->id;
enum pipe pipe = plane->pipe;
const struct skl_pipe_wm *pipe_wm = &crtc_state->wm.skl.optimal;
- const struct skl_plane_wm *wm = &pipe_wm->planes[plane_id];
const struct skl_ddb_entry *ddb =
&crtc_state->wm.skl.plane_ddb_y[plane_id];
@@ -5617,7 +5635,8 @@ void skl_write_cursor_wm(struct intel_plane *plane,
skl_write_wm_level(dev_priv, CUR_WM(pipe, level),
skl_plane_wm_level(pipe_wm, plane_id, level));
- skl_write_wm_level(dev_priv, CUR_WM_TRANS(pipe), &wm->trans_wm);
+ skl_write_wm_level(dev_priv, CUR_WM_TRANS(pipe),
+ skl_plane_trans_wm(pipe_wm, plane_id));
skl_ddb_entry_write(dev_priv, CUR_BUF_CFG(pipe), ddb);
}
@@ -5648,7 +5667,8 @@ static bool skl_plane_wm_equals(struct drm_i915_private *dev_priv,
}
return skl_wm_level_equals(&wm1->trans_wm, &wm2->trans_wm) &&
- skl_wm_level_equals(&wm1->sagv.wm0, &wm2->sagv.wm0);
+ skl_wm_level_equals(&wm1->sagv.wm0, &wm2->sagv.wm0) &&
+ skl_wm_level_equals(&wm1->sagv.trans_wm, &wm2->sagv.trans_wm);
}
static bool skl_ddb_entries_overlap(const struct skl_ddb_entry *a,
@@ -5878,8 +5898,8 @@ skl_print_wm_changes(struct intel_atomic_state *state)
continue;
drm_dbg_kms(&dev_priv->drm,
- "[PLANE:%d:%s] level %cwm0,%cwm1,%cwm2,%cwm3,%cwm4,%cwm5,%cwm6,%cwm7,%ctwm,%cswm"
- " -> %cwm0,%cwm1,%cwm2,%cwm3,%cwm4,%cwm5,%cwm6,%cwm7,%ctwm,%cswm\n",
+ "[PLANE:%d:%s] level %cwm0,%cwm1,%cwm2,%cwm3,%cwm4,%cwm5,%cwm6,%cwm7,%ctwm,%cswm,%cstwm"
+ " -> %cwm0,%cwm1,%cwm2,%cwm3,%cwm4,%cwm5,%cwm6,%cwm7,%ctwm,%cswm,%cstwm\n",
plane->base.base.id, plane->base.name,
enast(old_wm->wm[0].plane_en), enast(old_wm->wm[1].plane_en),
enast(old_wm->wm[2].plane_en), enast(old_wm->wm[3].plane_en),
@@ -5887,16 +5907,18 @@ skl_print_wm_changes(struct intel_atomic_state *state)
enast(old_wm->wm[6].plane_en), enast(old_wm->wm[7].plane_en),
enast(old_wm->trans_wm.plane_en),
enast(old_wm->sagv.wm0.plane_en),
+ enast(old_wm->sagv.trans_wm.plane_en),
enast(new_wm->wm[0].plane_en), enast(new_wm->wm[1].plane_en),
enast(new_wm->wm[2].plane_en), enast(new_wm->wm[3].plane_en),
enast(new_wm->wm[4].plane_en), enast(new_wm->wm[5].plane_en),
enast(new_wm->wm[6].plane_en), enast(new_wm->wm[7].plane_en),
enast(new_wm->trans_wm.plane_en),
- enast(new_wm->sagv.wm0.plane_en));
+ enast(new_wm->sagv.wm0.plane_en),
+ enast(new_wm->sagv.trans_wm.plane_en));
drm_dbg_kms(&dev_priv->drm,
- "[PLANE:%d:%s] lines %c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d"
- " -> %c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d\n",
+ "[PLANE:%d:%s] lines %c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%4d"
+ " -> %c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%4d\n",
plane->base.base.id, plane->base.name,
enast(old_wm->wm[0].ignore_lines), old_wm->wm[0].plane_res_l,
enast(old_wm->wm[1].ignore_lines), old_wm->wm[1].plane_res_l,
@@ -5908,7 +5930,7 @@ skl_print_wm_changes(struct intel_atomic_state *state)
enast(old_wm->wm[7].ignore_lines), old_wm->wm[7].plane_res_l,
enast(old_wm->trans_wm.ignore_lines), old_wm->trans_wm.plane_res_l,
enast(old_wm->sagv.wm0.ignore_lines), old_wm->sagv.wm0.plane_res_l,
-
+ enast(old_wm->sagv.trans_wm.ignore_lines), old_wm->sagv.trans_wm.plane_res_l,
enast(new_wm->wm[0].ignore_lines), new_wm->wm[0].plane_res_l,
enast(new_wm->wm[1].ignore_lines), new_wm->wm[1].plane_res_l,
enast(new_wm->wm[2].ignore_lines), new_wm->wm[2].plane_res_l,
@@ -5918,11 +5940,12 @@ skl_print_wm_changes(struct intel_atomic_state *state)
enast(new_wm->wm[6].ignore_lines), new_wm->wm[6].plane_res_l,
enast(new_wm->wm[7].ignore_lines), new_wm->wm[7].plane_res_l,
enast(new_wm->trans_wm.ignore_lines), new_wm->trans_wm.plane_res_l,
- enast(new_wm->sagv.wm0.ignore_lines), new_wm->sagv.wm0.plane_res_l);
+ enast(new_wm->sagv.wm0.ignore_lines), new_wm->sagv.wm0.plane_res_l,
+ enast(new_wm->sagv.trans_wm.ignore_lines), new_wm->sagv.trans_wm.plane_res_l);
drm_dbg_kms(&dev_priv->drm,
- "[PLANE:%d:%s] blocks %4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d"
- " -> %4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d\n",
+ "[PLANE:%d:%s] blocks %4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%5d"
+ " -> %4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%5d\n",
plane->base.base.id, plane->base.name,
old_wm->wm[0].plane_res_b, old_wm->wm[1].plane_res_b,
old_wm->wm[2].plane_res_b, old_wm->wm[3].plane_res_b,
@@ -5930,16 +5953,18 @@ skl_print_wm_changes(struct intel_atomic_state *state)
old_wm->wm[6].plane_res_b, old_wm->wm[7].plane_res_b,
old_wm->trans_wm.plane_res_b,
old_wm->sagv.wm0.plane_res_b,
+ old_wm->sagv.trans_wm.plane_res_b,
new_wm->wm[0].plane_res_b, new_wm->wm[1].plane_res_b,
new_wm->wm[2].plane_res_b, new_wm->wm[3].plane_res_b,
new_wm->wm[4].plane_res_b, new_wm->wm[5].plane_res_b,
new_wm->wm[6].plane_res_b, new_wm->wm[7].plane_res_b,
new_wm->trans_wm.plane_res_b,
- new_wm->sagv.wm0.plane_res_b);
+ new_wm->sagv.wm0.plane_res_b,
+ new_wm->sagv.trans_wm.plane_res_b);
drm_dbg_kms(&dev_priv->drm,
- "[PLANE:%d:%s] min_ddb %4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d"
- " -> %4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d\n",
+ "[PLANE:%d:%s] min_ddb %4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%5d"
+ " -> %4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%5d\n",
plane->base.base.id, plane->base.name,
old_wm->wm[0].min_ddb_alloc, old_wm->wm[1].min_ddb_alloc,
old_wm->wm[2].min_ddb_alloc, old_wm->wm[3].min_ddb_alloc,
@@ -5947,12 +5972,14 @@ skl_print_wm_changes(struct intel_atomic_state *state)
old_wm->wm[6].min_ddb_alloc, old_wm->wm[7].min_ddb_alloc,
old_wm->trans_wm.min_ddb_alloc,
old_wm->sagv.wm0.min_ddb_alloc,
+ old_wm->sagv.trans_wm.min_ddb_alloc,
new_wm->wm[0].min_ddb_alloc, new_wm->wm[1].min_ddb_alloc,
new_wm->wm[2].min_ddb_alloc, new_wm->wm[3].min_ddb_alloc,
new_wm->wm[4].min_ddb_alloc, new_wm->wm[5].min_ddb_alloc,
new_wm->wm[6].min_ddb_alloc, new_wm->wm[7].min_ddb_alloc,
new_wm->trans_wm.min_ddb_alloc,
- new_wm->sagv.wm0.min_ddb_alloc);
+ new_wm->sagv.wm0.min_ddb_alloc,
+ new_wm->sagv.trans_wm.min_ddb_alloc);
}
}
}
@@ -5961,8 +5988,6 @@ static bool skl_plane_selected_wm_equals(struct intel_plane *plane,
const struct skl_pipe_wm *old_pipe_wm,
const struct skl_pipe_wm *new_pipe_wm)
{
- const struct skl_plane_wm *old_wm = &old_pipe_wm->planes[plane->id];
- const struct skl_plane_wm *new_wm = &new_pipe_wm->planes[plane->id];
struct drm_i915_private *i915 = to_i915(plane->base.dev);
int level, max_level = ilk_wm_max_level(i915);
@@ -5977,7 +6002,8 @@ static bool skl_plane_selected_wm_equals(struct intel_plane *plane,
return false;
}
- return skl_wm_level_equals(&old_wm->trans_wm, &new_wm->trans_wm);
+ return skl_wm_level_equals(skl_plane_trans_wm(old_pipe_wm, plane->id),
+ skl_plane_trans_wm(new_pipe_wm, plane->id));
}
/*
@@ -6188,15 +6214,17 @@ void skl_pipe_wm_get_hw_state(struct intel_crtc *crtc,
skl_wm_level_from_reg_val(val, &wm->wm[level]);
}
- if (INTEL_GEN(dev_priv) >= 12)
- wm->sagv.wm0 = wm->wm[0];
-
if (plane_id != PLANE_CURSOR)
val = intel_uncore_read(&dev_priv->uncore, PLANE_WM_TRANS(pipe, plane_id));
else
val = intel_uncore_read(&dev_priv->uncore, CUR_WM_TRANS(pipe));
skl_wm_level_from_reg_val(val, &wm->trans_wm);
+
+ if (INTEL_GEN(dev_priv) >= 12) {
+ wm->sagv.wm0 = wm->wm[0];
+ wm->sagv.trans_wm = wm->trans_wm;
+ }
}
}
--
2.26.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Intel-gfx] [PATCH 6/7] drm/i915: Check tgl+ SAGV watermarks properly
2021-02-26 15:31 [Intel-gfx] [PATCH 0/7] drm/i915: Fix up TGL+ SAGV watermarks Ville Syrjala
` (4 preceding siblings ...)
2021-02-26 15:32 ` [Intel-gfx] [PATCH 5/7] drm/i915: Introduce SAGV transtion watermark Ville Syrjala
@ 2021-02-26 15:32 ` Ville Syrjala
2021-03-01 9:24 ` Lisovskiy, Stanislav
2021-02-26 15:32 ` [Intel-gfx] [PATCH 7/7] drm/i915: Clean up verify_wm_state() Ville Syrjala
` (2 subsequent siblings)
8 siblings, 1 reply; 17+ messages in thread
From: Ville Syrjala @ 2021-02-26 15:32 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
We know which WM0 (normal vs. SAGV) we supposedly programmed
into the hardware, so just check against that instead of accepting
either watermark as valid.
Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/display/intel_display.c | 88 +++++++++-----------
drivers/gpu/drm/i915/intel_pm.c | 4 +-
drivers/gpu/drm/i915/intel_pm.h | 5 ++
3 files changed, 47 insertions(+), 50 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index e34e5a9da5c1..e2ef31a93407 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -9377,41 +9377,40 @@ static void verify_wm_state(struct intel_crtc *crtc,
/* planes */
for_each_universal_plane(dev_priv, pipe, plane) {
- struct skl_plane_wm *hw_plane_wm, *sw_plane_wm;
-
- hw_plane_wm = &hw->wm.planes[plane];
- sw_plane_wm = &sw_wm->planes[plane];
+ const struct skl_wm_level *hw_wm_level, *sw_wm_level;
/* Watermarks */
for (level = 0; level <= max_level; level++) {
- if (skl_wm_level_equals(&hw_plane_wm->wm[level],
- &sw_plane_wm->wm[level]) ||
- (level == 0 && skl_wm_level_equals(&hw_plane_wm->wm[level],
- &sw_plane_wm->sagv.wm0)))
+ hw_wm_level = &hw->wm.planes[plane].wm[level];
+ sw_wm_level = skl_plane_wm_level(sw_wm, plane, level);
+
+ if (skl_wm_level_equals(hw_wm_level, sw_wm_level))
continue;
drm_err(&dev_priv->drm,
"mismatch in WM pipe %c plane %d level %d (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n",
pipe_name(pipe), plane + 1, level,
- sw_plane_wm->wm[level].plane_en,
- sw_plane_wm->wm[level].plane_res_b,
- sw_plane_wm->wm[level].plane_res_l,
- hw_plane_wm->wm[level].plane_en,
- hw_plane_wm->wm[level].plane_res_b,
- hw_plane_wm->wm[level].plane_res_l);
+ sw_wm_level->plane_en,
+ sw_wm_level->plane_res_b,
+ sw_wm_level->plane_res_l,
+ hw_wm_level->plane_en,
+ hw_wm_level->plane_res_b,
+ hw_wm_level->plane_res_l);
}
- if (!skl_wm_level_equals(&hw_plane_wm->trans_wm,
- &sw_plane_wm->trans_wm)) {
+ hw_wm_level = &hw->wm.planes[plane].trans_wm;
+ sw_wm_level = skl_plane_trans_wm(sw_wm, plane);
+
+ if (!skl_wm_level_equals(hw_wm_level, sw_wm_level)) {
drm_err(&dev_priv->drm,
"mismatch in trans WM pipe %c plane %d (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n",
pipe_name(pipe), plane + 1,
- sw_plane_wm->trans_wm.plane_en,
- sw_plane_wm->trans_wm.plane_res_b,
- sw_plane_wm->trans_wm.plane_res_l,
- hw_plane_wm->trans_wm.plane_en,
- hw_plane_wm->trans_wm.plane_res_b,
- hw_plane_wm->trans_wm.plane_res_l);
+ sw_wm_level->plane_en,
+ sw_wm_level->plane_res_b,
+ sw_wm_level->plane_res_l,
+ hw_wm_level->plane_en,
+ hw_wm_level->plane_res_b,
+ hw_wm_level->plane_res_l);
}
/* DDB */
@@ -9434,43 +9433,36 @@ static void verify_wm_state(struct intel_crtc *crtc,
* once the plane becomes visible, we can skip this check
*/
if (1) {
- struct skl_plane_wm *hw_plane_wm, *sw_plane_wm;
-
- hw_plane_wm = &hw->wm.planes[PLANE_CURSOR];
- sw_plane_wm = &sw_wm->planes[PLANE_CURSOR];
+ const struct skl_wm_level *hw_wm_level, *sw_wm_level;
/* Watermarks */
for (level = 0; level <= max_level; level++) {
- if (skl_wm_level_equals(&hw_plane_wm->wm[level],
- &sw_plane_wm->wm[level]) ||
- (level == 0 && skl_wm_level_equals(&hw_plane_wm->wm[level],
- &sw_plane_wm->sagv.wm0)))
- continue;
-
+ hw_wm_level = &hw->wm.planes[PLANE_CURSOR].wm[level];
+ sw_wm_level = skl_plane_wm_level(sw_wm, PLANE_CURSOR, level);
drm_err(&dev_priv->drm,
"mismatch in WM pipe %c cursor level %d (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n",
pipe_name(pipe), level,
- sw_plane_wm->wm[level].plane_en,
- sw_plane_wm->wm[level].plane_res_b,
- sw_plane_wm->wm[level].plane_res_l,
- hw_plane_wm->wm[level].plane_en,
- hw_plane_wm->wm[level].plane_res_b,
- hw_plane_wm->wm[level].plane_res_l);
+ sw_wm_level->plane_en,
+ sw_wm_level->plane_res_b,
+ sw_wm_level->plane_res_l,
+ hw_wm_level->plane_en,
+ hw_wm_level->plane_res_b,
+ hw_wm_level->plane_res_l);
}
- if (!skl_wm_level_equals(&hw_plane_wm->trans_wm,
- &sw_plane_wm->trans_wm) &&
- !skl_wm_level_equals(&hw_plane_wm->trans_wm,
- &sw_plane_wm->sagv.trans_wm)) {
+ hw_wm_level = &hw->wm.planes[PLANE_CURSOR].trans_wm;
+ sw_wm_level = skl_plane_trans_wm(sw_wm, PLANE_CURSOR);
+
+ if (!skl_wm_level_equals(hw_wm_level, sw_wm_level)) {
drm_err(&dev_priv->drm,
"mismatch in trans WM pipe %c cursor (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n",
pipe_name(pipe),
- sw_plane_wm->trans_wm.plane_en,
- sw_plane_wm->trans_wm.plane_res_b,
- sw_plane_wm->trans_wm.plane_res_l,
- hw_plane_wm->trans_wm.plane_en,
- hw_plane_wm->trans_wm.plane_res_b,
- hw_plane_wm->trans_wm.plane_res_l);
+ sw_wm_level->plane_en,
+ sw_wm_level->plane_res_b,
+ sw_wm_level->plane_res_l,
+ hw_wm_level->plane_en,
+ hw_wm_level->plane_res_b,
+ hw_wm_level->plane_res_l);
}
/* DDB */
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 9d9ba63fc395..854ffecd98d9 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4745,7 +4745,7 @@ icl_get_total_relative_data_rate(struct intel_atomic_state *state,
return total_data_rate;
}
-static const struct skl_wm_level *
+const struct skl_wm_level *
skl_plane_wm_level(const struct skl_pipe_wm *pipe_wm,
enum plane_id plane_id,
int level)
@@ -4758,7 +4758,7 @@ skl_plane_wm_level(const struct skl_pipe_wm *pipe_wm,
return &wm->wm[level];
}
-static const struct skl_wm_level *
+const struct skl_wm_level *
skl_plane_trans_wm(const struct skl_pipe_wm *pipe_wm,
enum plane_id plane_id)
{
diff --git a/drivers/gpu/drm/i915/intel_pm.h b/drivers/gpu/drm/i915/intel_pm.h
index 97550cf0b6df..669c8d505677 100644
--- a/drivers/gpu/drm/i915/intel_pm.h
+++ b/drivers/gpu/drm/i915/intel_pm.h
@@ -52,6 +52,11 @@ bool intel_can_enable_sagv(struct drm_i915_private *dev_priv,
const struct intel_bw_state *bw_state);
void intel_sagv_pre_plane_update(struct intel_atomic_state *state);
void intel_sagv_post_plane_update(struct intel_atomic_state *state);
+const struct skl_wm_level *skl_plane_wm_level(const struct skl_pipe_wm *pipe_wm,
+ enum plane_id plane_id,
+ int level);
+const struct skl_wm_level *skl_plane_trans_wm(const struct skl_pipe_wm *pipe_wm,
+ enum plane_id plane_id);
bool skl_wm_level_equals(const struct skl_wm_level *l1,
const struct skl_wm_level *l2);
bool skl_ddb_allocation_overlaps(const struct skl_ddb_entry *ddb,
--
2.26.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Intel-gfx] [PATCH 7/7] drm/i915: Clean up verify_wm_state()
2021-02-26 15:31 [Intel-gfx] [PATCH 0/7] drm/i915: Fix up TGL+ SAGV watermarks Ville Syrjala
` (5 preceding siblings ...)
2021-02-26 15:32 ` [Intel-gfx] [PATCH 6/7] drm/i915: Check tgl+ SAGV watermarks properly Ville Syrjala
@ 2021-02-26 15:32 ` Ville Syrjala
2021-03-01 9:27 ` Lisovskiy, Stanislav
2021-02-26 15:44 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Fix up TGL+ SAGV watermarks Patchwork
2021-02-26 16:14 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
8 siblings, 1 reply; 17+ messages in thread
From: Ville Syrjala @ 2021-02-26 15:32 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Get rid of the nonsense cursor special case in verify_wm_state()
by just iterating through all the planes. And let's use the
canonical [PLANE:..] style in the debug prints while at it.
Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/display/intel_display.c | 88 ++++----------------
drivers/gpu/drm/i915/display/intel_display.h | 5 --
2 files changed, 17 insertions(+), 76 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index e2ef31a93407..e0ecd8eea68d 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -9348,11 +9348,10 @@ static void verify_wm_state(struct intel_crtc *crtc,
struct skl_ddb_entry ddb_uv[I915_MAX_PLANES];
struct skl_pipe_wm wm;
} *hw;
- struct skl_pipe_wm *sw_wm;
- struct skl_ddb_entry *hw_ddb_entry, *sw_ddb_entry;
+ const struct skl_pipe_wm *sw_wm = &new_crtc_state->wm.skl.optimal;
+ int level, max_level = ilk_wm_max_level(dev_priv);
+ struct intel_plane *plane;
u8 hw_enabled_slices;
- const enum pipe pipe = crtc->pipe;
- int plane, level, max_level = ilk_wm_max_level(dev_priv);
if (INTEL_GEN(dev_priv) < 9 || !new_crtc_state->hw.active)
return;
@@ -9362,7 +9361,6 @@ static void verify_wm_state(struct intel_crtc *crtc,
return;
skl_pipe_wm_get_hw_state(crtc, &hw->wm);
- sw_wm = &new_crtc_state->wm.skl.optimal;
skl_pipe_ddb_get_hw_state(crtc, hw->ddb_y, hw->ddb_uv);
@@ -9375,21 +9373,21 @@ static void verify_wm_state(struct intel_crtc *crtc,
dev_priv->dbuf.enabled_slices,
hw_enabled_slices);
- /* planes */
- for_each_universal_plane(dev_priv, pipe, plane) {
+ for_each_intel_plane_on_crtc(&dev_priv->drm, crtc, plane) {
+ const struct skl_ddb_entry *hw_ddb_entry, *sw_ddb_entry;
const struct skl_wm_level *hw_wm_level, *sw_wm_level;
/* Watermarks */
for (level = 0; level <= max_level; level++) {
- hw_wm_level = &hw->wm.planes[plane].wm[level];
- sw_wm_level = skl_plane_wm_level(sw_wm, plane, level);
+ hw_wm_level = &hw->wm.planes[plane->id].wm[level];
+ sw_wm_level = skl_plane_wm_level(sw_wm, plane->id, level);
if (skl_wm_level_equals(hw_wm_level, sw_wm_level))
continue;
drm_err(&dev_priv->drm,
- "mismatch in WM pipe %c plane %d level %d (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n",
- pipe_name(pipe), plane + 1, level,
+ "[PLANE:%d:%s] mismatch in WM%d (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n",
+ plane->base.base.id, plane->base.name, level,
sw_wm_level->plane_en,
sw_wm_level->plane_res_b,
sw_wm_level->plane_res_l,
@@ -9398,13 +9396,13 @@ static void verify_wm_state(struct intel_crtc *crtc,
hw_wm_level->plane_res_l);
}
- hw_wm_level = &hw->wm.planes[plane].trans_wm;
- sw_wm_level = skl_plane_trans_wm(sw_wm, plane);
+ hw_wm_level = &hw->wm.planes[plane->id].trans_wm;
+ sw_wm_level = skl_plane_trans_wm(sw_wm, plane->id);
if (!skl_wm_level_equals(hw_wm_level, sw_wm_level)) {
drm_err(&dev_priv->drm,
- "mismatch in trans WM pipe %c plane %d (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n",
- pipe_name(pipe), plane + 1,
+ "[PLANE:%d:%s] mismatch in trans WM (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n",
+ plane->base.base.id, plane->base.name,
sw_wm_level->plane_en,
sw_wm_level->plane_res_b,
sw_wm_level->plane_res_l,
@@ -9414,65 +9412,13 @@ static void verify_wm_state(struct intel_crtc *crtc,
}
/* DDB */
- hw_ddb_entry = &hw->ddb_y[plane];
- sw_ddb_entry = &new_crtc_state->wm.skl.plane_ddb_y[plane];
+ hw_ddb_entry = &hw->ddb_y[plane->id];
+ sw_ddb_entry = &new_crtc_state->wm.skl.plane_ddb_y[plane->id];
if (!skl_ddb_entry_equal(hw_ddb_entry, sw_ddb_entry)) {
drm_err(&dev_priv->drm,
- "mismatch in DDB state pipe %c plane %d (expected (%u,%u), found (%u,%u))\n",
- pipe_name(pipe), plane + 1,
- sw_ddb_entry->start, sw_ddb_entry->end,
- hw_ddb_entry->start, hw_ddb_entry->end);
- }
- }
-
- /*
- * cursor
- * If the cursor plane isn't active, we may not have updated it's ddb
- * allocation. In that case since the ddb allocation will be updated
- * once the plane becomes visible, we can skip this check
- */
- if (1) {
- const struct skl_wm_level *hw_wm_level, *sw_wm_level;
-
- /* Watermarks */
- for (level = 0; level <= max_level; level++) {
- hw_wm_level = &hw->wm.planes[PLANE_CURSOR].wm[level];
- sw_wm_level = skl_plane_wm_level(sw_wm, PLANE_CURSOR, level);
- drm_err(&dev_priv->drm,
- "mismatch in WM pipe %c cursor level %d (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n",
- pipe_name(pipe), level,
- sw_wm_level->plane_en,
- sw_wm_level->plane_res_b,
- sw_wm_level->plane_res_l,
- hw_wm_level->plane_en,
- hw_wm_level->plane_res_b,
- hw_wm_level->plane_res_l);
- }
-
- hw_wm_level = &hw->wm.planes[PLANE_CURSOR].trans_wm;
- sw_wm_level = skl_plane_trans_wm(sw_wm, PLANE_CURSOR);
-
- if (!skl_wm_level_equals(hw_wm_level, sw_wm_level)) {
- drm_err(&dev_priv->drm,
- "mismatch in trans WM pipe %c cursor (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n",
- pipe_name(pipe),
- sw_wm_level->plane_en,
- sw_wm_level->plane_res_b,
- sw_wm_level->plane_res_l,
- hw_wm_level->plane_en,
- hw_wm_level->plane_res_b,
- hw_wm_level->plane_res_l);
- }
-
- /* DDB */
- hw_ddb_entry = &hw->ddb_y[PLANE_CURSOR];
- sw_ddb_entry = &new_crtc_state->wm.skl.plane_ddb_y[PLANE_CURSOR];
-
- if (!skl_ddb_entry_equal(hw_ddb_entry, sw_ddb_entry)) {
- drm_err(&dev_priv->drm,
- "mismatch in DDB state pipe %c cursor (expected (%u,%u), found (%u,%u))\n",
- pipe_name(pipe),
+ "[PLANE:%d:%s] mismatch in DDB (expected (%u,%u), found (%u,%u))\n",
+ plane->base.base.id, plane->base.name,
sw_ddb_entry->start, sw_ddb_entry->end,
hw_ddb_entry->start, hw_ddb_entry->end);
}
diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
index 61080152319f..216047233a6d 100644
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -353,11 +353,6 @@ enum phy_fia {
for_each_cpu_transcoder(__dev_priv, __t) \
for_each_if ((__mask) & BIT(__t))
-#define for_each_universal_plane(__dev_priv, __pipe, __p) \
- for ((__p) = 0; \
- (__p) < RUNTIME_INFO(__dev_priv)->num_sprites[(__pipe)] + 1; \
- (__p)++)
-
#define for_each_sprite(__dev_priv, __p, __s) \
for ((__s) = 0; \
(__s) < RUNTIME_INFO(__dev_priv)->num_sprites[(__p)]; \
--
2.26.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Fix up TGL+ SAGV watermarks
2021-02-26 15:31 [Intel-gfx] [PATCH 0/7] drm/i915: Fix up TGL+ SAGV watermarks Ville Syrjala
` (6 preceding siblings ...)
2021-02-26 15:32 ` [Intel-gfx] [PATCH 7/7] drm/i915: Clean up verify_wm_state() Ville Syrjala
@ 2021-02-26 15:44 ` Patchwork
2021-02-26 16:14 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
8 siblings, 0 replies; 17+ messages in thread
From: Patchwork @ 2021-02-26 15:44 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Fix up TGL+ SAGV watermarks
URL : https://patchwork.freedesktop.org/series/87433/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
ccc6bba5464d drm/i915: Fix TGL+ plane SAGV watermark programming
f0b33a40d933 drm/i915: Zero out SAGV wm when we don't have enough DDB for it
75118b227742 drm/i915: Print wm changes if sagv_wm0 changes
7d23b537945f drm/i915: Stuff SAGV watermark into a sub-structure
-:124: WARNING:LONG_LINE: line length of 103 exceeds 100 columns
#124: FILE: drivers/gpu/drm/i915/intel_pm.c:5910:
+ enast(old_wm->sagv.wm0.ignore_lines), old_wm->sagv.wm0.plane_res_l,
-:133: WARNING:LONG_LINE: line length of 104 exceeds 100 columns
#133: FILE: drivers/gpu/drm/i915/intel_pm.c:5921:
+ enast(new_wm->sagv.wm0.ignore_lines), new_wm->sagv.wm0.plane_res_l);
total: 0 errors, 2 warnings, 0 checks, 137 lines checked
44e389eea600 drm/i915: Introduce SAGV transtion watermark
-:206: WARNING:LONG_LINE: line length of 113 exceeds 100 columns
#206: FILE: drivers/gpu/drm/i915/intel_pm.c:5933:
+ enast(old_wm->sagv.trans_wm.ignore_lines), old_wm->sagv.trans_wm.plane_res_l,
-:215: WARNING:LONG_LINE: line length of 103 exceeds 100 columns
#215: FILE: drivers/gpu/drm/i915/intel_pm.c:5943:
+ enast(new_wm->sagv.wm0.ignore_lines), new_wm->sagv.wm0.plane_res_l,
-:216: WARNING:LONG_LINE: line length of 114 exceeds 100 columns
#216: FILE: drivers/gpu/drm/i915/intel_pm.c:5944:
+ enast(new_wm->sagv.trans_wm.ignore_lines), new_wm->sagv.trans_wm.plane_res_l);
total: 0 errors, 3 warnings, 0 checks, 255 lines checked
1d2e11c235a9 drm/i915: Check tgl+ SAGV watermarks properly
2d828095e91e drm/i915: Clean up verify_wm_state()
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Fix up TGL+ SAGV watermarks
2021-02-26 15:31 [Intel-gfx] [PATCH 0/7] drm/i915: Fix up TGL+ SAGV watermarks Ville Syrjala
` (7 preceding siblings ...)
2021-02-26 15:44 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Fix up TGL+ SAGV watermarks Patchwork
@ 2021-02-26 16:14 ` Patchwork
8 siblings, 0 replies; 17+ messages in thread
From: Patchwork @ 2021-02-26 16:14 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx
[-- Attachment #1.1: Type: text/plain, Size: 5534 bytes --]
== Series Details ==
Series: drm/i915: Fix up TGL+ SAGV watermarks
URL : https://patchwork.freedesktop.org/series/87433/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_9814 -> Patchwork_19733
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19733/index.html
Known issues
------------
Here are the changes found in Patchwork_19733 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_huc_copy@huc-copy:
- fi-byt-j1900: NOTRUN -> [SKIP][1] ([fdo#109271]) +9 similar issues
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19733/fi-byt-j1900/igt@gem_huc_copy@huc-copy.html
* igt@gem_mmap_gtt@basic:
- fi-tgl-y: [PASS][2] -> [DMESG-WARN][3] ([i915#402]) +2 similar issues
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9814/fi-tgl-y/igt@gem_mmap_gtt@basic.html
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19733/fi-tgl-y/igt@gem_mmap_gtt@basic.html
* igt@gem_tiled_blits@basic:
- fi-kbl-8809g: [PASS][4] -> [TIMEOUT][5] ([i915#2502])
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9814/fi-kbl-8809g/igt@gem_tiled_blits@basic.html
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19733/fi-kbl-8809g/igt@gem_tiled_blits@basic.html
* igt@i915_pm_rpm@module-reload:
- fi-byt-j1900: NOTRUN -> [INCOMPLETE][6] ([i915#142] / [i915#2405])
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19733/fi-byt-j1900/igt@i915_pm_rpm@module-reload.html
* igt@kms_chamelium@hdmi-crc-fast:
- fi-byt-j1900: NOTRUN -> [SKIP][7] ([fdo#109271] / [fdo#111827]) +8 similar issues
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19733/fi-byt-j1900/igt@kms_chamelium@hdmi-crc-fast.html
* igt@runner@aborted:
- fi-byt-j1900: NOTRUN -> [FAIL][8] ([i915#1814] / [i915#2505])
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19733/fi-byt-j1900/igt@runner@aborted.html
#### Possible fixes ####
* igt@gem_exec_gttfill@basic:
- fi-kbl-8809g: [TIMEOUT][9] -> [PASS][10]
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9814/fi-kbl-8809g/igt@gem_exec_gttfill@basic.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19733/fi-kbl-8809g/igt@gem_exec_gttfill@basic.html
* igt@gem_tiled_blits@basic:
- fi-tgl-y: [DMESG-WARN][11] ([i915#402]) -> [PASS][12] +2 similar issues
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9814/fi-tgl-y/igt@gem_tiled_blits@basic.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19733/fi-tgl-y/igt@gem_tiled_blits@basic.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
[fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
[fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
[fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
[i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
[i915#142]: https://gitlab.freedesktop.org/drm/intel/issues/142
[i915#1814]: https://gitlab.freedesktop.org/drm/intel/issues/1814
[i915#2283]: https://gitlab.freedesktop.org/drm/intel/issues/2283
[i915#2405]: https://gitlab.freedesktop.org/drm/intel/issues/2405
[i915#2502]: https://gitlab.freedesktop.org/drm/intel/issues/2502
[i915#2505]: https://gitlab.freedesktop.org/drm/intel/issues/2505
[i915#2582]: https://gitlab.freedesktop.org/drm/intel/issues/2582
[i915#3004]: https://gitlab.freedesktop.org/drm/intel/issues/3004
[i915#3005]: https://gitlab.freedesktop.org/drm/intel/issues/3005
[i915#3011]: https://gitlab.freedesktop.org/drm/intel/issues/3011
[i915#3012]: https://gitlab.freedesktop.org/drm/intel/issues/3012
[i915#3013]: https://gitlab.freedesktop.org/drm/intel/issues/3013
[i915#3014]: https://gitlab.freedesktop.org/drm/intel/issues/3014
[i915#3015]: https://gitlab.freedesktop.org/drm/intel/issues/3015
[i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402
[i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533
Participating hosts (39 -> 37)
------------------------------
Additional (2): fi-byt-j1900 fi-hsw-gt1
Missing (4): fi-ctg-p8600 fi-ilk-m540 fi-bsw-cyan fi-bdw-samus
Build changes
-------------
* Linux: CI_DRM_9814 -> Patchwork_19733
CI-20190529: 20190529
CI_DRM_9814: 83c6d819595dc35a5058c8f71d0ee5f38e199940 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_6015: aa44cddf4ef689f8a3726fcbeedc03f08b12bd82 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_19733: 2d828095e91e29a09838241ce5ba9c78d7f4f5e8 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
2d828095e91e drm/i915: Clean up verify_wm_state()
1d2e11c235a9 drm/i915: Check tgl+ SAGV watermarks properly
44e389eea600 drm/i915: Introduce SAGV transtion watermark
7d23b537945f drm/i915: Stuff SAGV watermark into a sub-structure
75118b227742 drm/i915: Print wm changes if sagv_wm0 changes
f0b33a40d933 drm/i915: Zero out SAGV wm when we don't have enough DDB for it
ccc6bba5464d drm/i915: Fix TGL+ plane SAGV watermark programming
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19733/index.html
[-- Attachment #1.2: Type: text/html, Size: 5564 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Intel-gfx] [PATCH 1/7] drm/i915: Fix TGL+ plane SAGV watermark programming
2021-02-26 15:31 ` [Intel-gfx] [PATCH 1/7] drm/i915: Fix TGL+ plane SAGV watermark programming Ville Syrjala
@ 2021-03-01 8:38 ` Lisovskiy, Stanislav
0 siblings, 0 replies; 17+ messages in thread
From: Lisovskiy, Stanislav @ 2021-03-01 8:38 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx
On Fri, Feb 26, 2021 at 05:31:58PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> When we switch between SAGV on vs. off we need to reprogram all
> plane wateramrks accordingly. Currently skl_wm_add_affected_planes()
> totally ignores the SAGV watermark and just assumes we will use
> the normal WM0.
>
> Fix this by utilizing skl_plane_wm_level() which picks the
> correct watermark based on use_sagv_wm. Thus we will force
> an update on all the planes whose watermark registers need
> to be reprogrammed.
Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
>
> Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_pm.c | 60 ++++++++++++++++++++-------------
> 1 file changed, 37 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 8cc67f9c4e58..2d0e3e7f11b8 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4748,11 +4748,10 @@ icl_get_total_relative_data_rate(struct intel_atomic_state *state,
> }
>
> static const struct skl_wm_level *
> -skl_plane_wm_level(const struct intel_crtc_state *crtc_state,
> +skl_plane_wm_level(const struct skl_pipe_wm *pipe_wm,
> enum plane_id plane_id,
> int level)
> {
> - const struct skl_pipe_wm *pipe_wm = &crtc_state->wm.skl.optimal;
> const struct skl_plane_wm *wm = &pipe_wm->planes[plane_id];
>
> if (level == 0 && pipe_wm->use_sagv_wm)
> @@ -5572,21 +5571,17 @@ void skl_write_plane_wm(struct intel_plane *plane,
> int level, max_level = ilk_wm_max_level(dev_priv);
> enum plane_id plane_id = plane->id;
> enum pipe pipe = plane->pipe;
> - const struct skl_plane_wm *wm =
> - &crtc_state->wm.skl.optimal.planes[plane_id];
> + const struct skl_pipe_wm *pipe_wm = &crtc_state->wm.skl.optimal;
> + const struct skl_plane_wm *wm = &pipe_wm->planes[plane_id];
> const struct skl_ddb_entry *ddb_y =
> &crtc_state->wm.skl.plane_ddb_y[plane_id];
> const struct skl_ddb_entry *ddb_uv =
> &crtc_state->wm.skl.plane_ddb_uv[plane_id];
>
> - for (level = 0; level <= max_level; level++) {
> - const struct skl_wm_level *wm_level;
> -
> - wm_level = skl_plane_wm_level(crtc_state, plane_id, level);
> -
> + for (level = 0; level <= max_level; level++)
> skl_write_wm_level(dev_priv, PLANE_WM(pipe, plane_id, level),
> - wm_level);
> - }
> + skl_plane_wm_level(pipe_wm, plane_id, level));
> +
> skl_write_wm_level(dev_priv, PLANE_WM_TRANS(pipe, plane_id),
> &wm->trans_wm);
>
> @@ -5612,19 +5607,15 @@ void skl_write_cursor_wm(struct intel_plane *plane,
> int level, max_level = ilk_wm_max_level(dev_priv);
> enum plane_id plane_id = plane->id;
> enum pipe pipe = plane->pipe;
> - const struct skl_plane_wm *wm =
> - &crtc_state->wm.skl.optimal.planes[plane_id];
> + const struct skl_pipe_wm *pipe_wm = &crtc_state->wm.skl.optimal;
> + const struct skl_plane_wm *wm = &pipe_wm->planes[plane_id];
> const struct skl_ddb_entry *ddb =
> &crtc_state->wm.skl.plane_ddb_y[plane_id];
>
> - for (level = 0; level <= max_level; level++) {
> - const struct skl_wm_level *wm_level;
> -
> - wm_level = skl_plane_wm_level(crtc_state, plane_id, level);
> -
> + for (level = 0; level <= max_level; level++)
> skl_write_wm_level(dev_priv, CUR_WM(pipe, level),
> - wm_level);
> - }
> + skl_plane_wm_level(pipe_wm, plane_id, level));
> +
> skl_write_wm_level(dev_priv, CUR_WM_TRANS(pipe), &wm->trans_wm);
>
> skl_ddb_entry_write(dev_priv, CUR_BUF_CFG(pipe), ddb);
> @@ -5964,6 +5955,29 @@ skl_print_wm_changes(struct intel_atomic_state *state)
> }
> }
>
> +static bool skl_plane_selected_wm_equals(struct intel_plane *plane,
> + const struct skl_pipe_wm *old_pipe_wm,
> + const struct skl_pipe_wm *new_pipe_wm)
> +{
> + const struct skl_plane_wm *old_wm = &old_pipe_wm->planes[plane->id];
> + const struct skl_plane_wm *new_wm = &new_pipe_wm->planes[plane->id];
> + struct drm_i915_private *i915 = to_i915(plane->base.dev);
> + int level, max_level = ilk_wm_max_level(i915);
> +
> + for (level = 0; level <= max_level; level++) {
> + /*
> + * We don't check uv_wm as the hardware doesn't actually
> + * use it. It only gets used for calculating the required
> + * ddb allocation.
> + */
> + if (!skl_wm_level_equals(skl_plane_wm_level(old_pipe_wm, level, plane->id),
> + skl_plane_wm_level(new_pipe_wm, level, plane->id)))
> + return false;
> + }
> +
> + return skl_wm_level_equals(&old_wm->trans_wm, &new_wm->trans_wm);
> +}
> +
> /*
> * To make sure the cursor watermark registers are always consistent
> * with our computed state the following scenario needs special
> @@ -6009,9 +6023,9 @@ static int skl_wm_add_affected_planes(struct intel_atomic_state *state,
> * with the software state.
> */
> if (!drm_atomic_crtc_needs_modeset(&new_crtc_state->uapi) &&
> - skl_plane_wm_equals(dev_priv,
> - &old_crtc_state->wm.skl.optimal.planes[plane_id],
> - &new_crtc_state->wm.skl.optimal.planes[plane_id]))
> + skl_plane_selected_wm_equals(plane,
> + &old_crtc_state->wm.skl.optimal,
> + &new_crtc_state->wm.skl.optimal))
> continue;
>
> plane_state = intel_atomic_get_plane_state(state, plane);
> --
> 2.26.2
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Intel-gfx] [PATCH 2/7] drm/i915: Zero out SAGV wm when we don't have enough DDB for it
2021-02-26 15:31 ` [Intel-gfx] [PATCH 2/7] drm/i915: Zero out SAGV wm when we don't have enough DDB for it Ville Syrjala
@ 2021-03-01 8:42 ` Lisovskiy, Stanislav
0 siblings, 0 replies; 17+ messages in thread
From: Lisovskiy, Stanislav @ 2021-03-01 8:42 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx
On Fri, Feb 26, 2021 at 05:31:59PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Let's handle the SAGV WM0 more like the other wm levels and just
> totally zero it out when we don't have the DDB space to back it
> up.
Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
>
> Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_pm.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 2d0e3e7f11b8..c341fa957884 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3921,12 +3921,10 @@ static bool tgl_crtc_can_enable_sagv(const struct intel_crtc_state *crtc_state)
> return true;
>
> for_each_plane_id_on_crtc(crtc, plane_id) {
> - const struct skl_ddb_entry *plane_alloc =
> - &crtc_state->wm.skl.plane_ddb_y[plane_id];
> const struct skl_plane_wm *wm =
> &crtc_state->wm.skl.optimal.planes[plane_id];
>
> - if (skl_ddb_entry_size(plane_alloc) < wm->sagv_wm0.min_ddb_alloc)
> + if (wm->wm[0].plane_en && !wm->sagv_wm0.plane_en)
> return false;
> }
>
> @@ -4957,8 +4955,8 @@ skl_allocate_plane_ddb(struct intel_atomic_state *state,
> }
>
> /*
> - * Go back and disable the transition watermark if it turns out we
> - * don't have enough DDB blocks for it.
> + * Go back and disable the transition and SAGV watermarks
> + * if it turns out we don't have enough DDB blocks for them.
> */
> for_each_plane_id_on_crtc(crtc, plane_id) {
> struct skl_plane_wm *wm =
> @@ -4966,6 +4964,9 @@ skl_allocate_plane_ddb(struct intel_atomic_state *state,
>
> if (wm->trans_wm.plane_res_b >= total[plane_id])
> memset(&wm->trans_wm, 0, sizeof(wm->trans_wm));
> +
> + if (wm->sagv_wm0.plane_res_b >= total[plane_id])
> + memset(&wm->sagv_wm0, 0, sizeof(wm->sagv_wm0));
> }
>
> return 0;
> --
> 2.26.2
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Intel-gfx] [PATCH 3/7] drm/i915: Print wm changes if sagv_wm0 changes
2021-02-26 15:32 ` [Intel-gfx] [PATCH 3/7] drm/i915: Print wm changes if sagv_wm0 changes Ville Syrjala
@ 2021-03-01 9:14 ` Lisovskiy, Stanislav
0 siblings, 0 replies; 17+ messages in thread
From: Lisovskiy, Stanislav @ 2021-03-01 9:14 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx
On Fri, Feb 26, 2021 at 05:32:00PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Let's consider sagv_wm0 as well when deciding whether to dump
> out the watermark changes.
>
> Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_pm.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index c341fa957884..06c54adc609a 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5647,7 +5647,8 @@ static bool skl_plane_wm_equals(struct drm_i915_private *dev_priv,
> return false;
> }
>
> - return skl_wm_level_equals(&wm1->trans_wm, &wm2->trans_wm);
> + return skl_wm_level_equals(&wm1->trans_wm, &wm2->trans_wm) &&
> + skl_wm_level_equals(&wm1->sagv_wm0, &wm2->sagv_wm0);
> }
Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
>
> static bool skl_ddb_entries_overlap(const struct skl_ddb_entry *a,
> --
> 2.26.2
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Intel-gfx] [PATCH 4/7] drm/i915: Stuff SAGV watermark into a sub-structure
2021-02-26 15:32 ` [Intel-gfx] [PATCH 4/7] drm/i915: Stuff SAGV watermark into a sub-structure Ville Syrjala
@ 2021-03-01 9:17 ` Lisovskiy, Stanislav
0 siblings, 0 replies; 17+ messages in thread
From: Lisovskiy, Stanislav @ 2021-03-01 9:17 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx
On Fri, Feb 26, 2021 at 05:32:01PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> We'll want a SAGV transition watermark as well. Prepare
> for that by collecting SAGV wm0 into a sub-strcture.
>
> Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 4 +--
> .../drm/i915/display/intel_display_types.h | 4 ++-
> drivers/gpu/drm/i915/intel_pm.c | 30 +++++++++----------
> 3 files changed, 20 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index d0da88751c72..718e66f49332 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -9387,7 +9387,7 @@ static void verify_wm_state(struct intel_crtc *crtc,
> if (skl_wm_level_equals(&hw_plane_wm->wm[level],
> &sw_plane_wm->wm[level]) ||
> (level == 0 && skl_wm_level_equals(&hw_plane_wm->wm[level],
> - &sw_plane_wm->sagv_wm0)))
> + &sw_plane_wm->sagv.wm0)))
> continue;
>
> drm_err(&dev_priv->drm,
> @@ -9444,7 +9444,7 @@ static void verify_wm_state(struct intel_crtc *crtc,
> if (skl_wm_level_equals(&hw_plane_wm->wm[level],
> &sw_plane_wm->wm[level]) ||
> (level == 0 && skl_wm_level_equals(&hw_plane_wm->wm[level],
> - &sw_plane_wm->sagv_wm0)))
> + &sw_plane_wm->sagv.wm0)))
> continue;
>
> drm_err(&dev_priv->drm,
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 1a76e1d9de7a..6321cd3df81e 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -732,7 +732,9 @@ struct skl_plane_wm {
> struct skl_wm_level wm[8];
> struct skl_wm_level uv_wm[8];
> struct skl_wm_level trans_wm;
> - struct skl_wm_level sagv_wm0;
> + struct {
> + struct skl_wm_level wm0;
> + } sagv;
> bool is_planar;
> };
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 06c54adc609a..a1591d9189a0 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3924,7 +3924,7 @@ static bool tgl_crtc_can_enable_sagv(const struct intel_crtc_state *crtc_state)
> const struct skl_plane_wm *wm =
> &crtc_state->wm.skl.optimal.planes[plane_id];
>
> - if (wm->wm[0].plane_en && !wm->sagv_wm0.plane_en)
> + if (wm->wm[0].plane_en && !wm->sagv.wm0.plane_en)
> return false;
> }
>
> @@ -4753,7 +4753,7 @@ skl_plane_wm_level(const struct skl_pipe_wm *pipe_wm,
> const struct skl_plane_wm *wm = &pipe_wm->planes[plane_id];
>
> if (level == 0 && pipe_wm->use_sagv_wm)
> - return &wm->sagv_wm0;
> + return &wm->sagv.wm0;
>
> return &wm->wm[level];
> }
> @@ -4965,8 +4965,8 @@ skl_allocate_plane_ddb(struct intel_atomic_state *state,
> if (wm->trans_wm.plane_res_b >= total[plane_id])
> memset(&wm->trans_wm, 0, sizeof(wm->trans_wm));
>
> - if (wm->sagv_wm0.plane_res_b >= total[plane_id])
> - memset(&wm->sagv_wm0, 0, sizeof(wm->sagv_wm0));
> + if (wm->sagv.wm0.plane_res_b >= total[plane_id])
> + memset(&wm->sagv.wm0, 0, sizeof(wm->sagv.wm0));
> }
>
> return 0;
> @@ -5316,7 +5316,7 @@ static void tgl_compute_sagv_wm(const struct intel_crtc_state *crtc_state,
> struct skl_plane_wm *plane_wm)
> {
> struct drm_i915_private *dev_priv = to_i915(crtc_state->uapi.crtc->dev);
> - struct skl_wm_level *sagv_wm = &plane_wm->sagv_wm0;
> + struct skl_wm_level *sagv_wm = &plane_wm->sagv.wm0;
> struct skl_wm_level *levels = plane_wm->wm;
> unsigned int latency = dev_priv->wm.skl_latency[0] + dev_priv->sagv_block_time_us;
>
> @@ -5648,7 +5648,7 @@ static bool skl_plane_wm_equals(struct drm_i915_private *dev_priv,
> }
>
> return skl_wm_level_equals(&wm1->trans_wm, &wm2->trans_wm) &&
> - skl_wm_level_equals(&wm1->sagv_wm0, &wm2->sagv_wm0);
> + skl_wm_level_equals(&wm1->sagv.wm0, &wm2->sagv.wm0);
> }
>
> static bool skl_ddb_entries_overlap(const struct skl_ddb_entry *a,
> @@ -5886,13 +5886,13 @@ skl_print_wm_changes(struct intel_atomic_state *state)
> enast(old_wm->wm[4].plane_en), enast(old_wm->wm[5].plane_en),
> enast(old_wm->wm[6].plane_en), enast(old_wm->wm[7].plane_en),
> enast(old_wm->trans_wm.plane_en),
> - enast(old_wm->sagv_wm0.plane_en),
> + enast(old_wm->sagv.wm0.plane_en),
> enast(new_wm->wm[0].plane_en), enast(new_wm->wm[1].plane_en),
> enast(new_wm->wm[2].plane_en), enast(new_wm->wm[3].plane_en),
> enast(new_wm->wm[4].plane_en), enast(new_wm->wm[5].plane_en),
> enast(new_wm->wm[6].plane_en), enast(new_wm->wm[7].plane_en),
> enast(new_wm->trans_wm.plane_en),
> - enast(new_wm->sagv_wm0.plane_en));
> + enast(new_wm->sagv.wm0.plane_en));
>
> drm_dbg_kms(&dev_priv->drm,
> "[PLANE:%d:%s] lines %c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d"
> @@ -5907,7 +5907,7 @@ skl_print_wm_changes(struct intel_atomic_state *state)
> enast(old_wm->wm[6].ignore_lines), old_wm->wm[6].plane_res_l,
> enast(old_wm->wm[7].ignore_lines), old_wm->wm[7].plane_res_l,
> enast(old_wm->trans_wm.ignore_lines), old_wm->trans_wm.plane_res_l,
> - enast(old_wm->sagv_wm0.ignore_lines), old_wm->sagv_wm0.plane_res_l,
> + enast(old_wm->sagv.wm0.ignore_lines), old_wm->sagv.wm0.plane_res_l,
>
> enast(new_wm->wm[0].ignore_lines), new_wm->wm[0].plane_res_l,
> enast(new_wm->wm[1].ignore_lines), new_wm->wm[1].plane_res_l,
> @@ -5918,7 +5918,7 @@ skl_print_wm_changes(struct intel_atomic_state *state)
> enast(new_wm->wm[6].ignore_lines), new_wm->wm[6].plane_res_l,
> enast(new_wm->wm[7].ignore_lines), new_wm->wm[7].plane_res_l,
> enast(new_wm->trans_wm.ignore_lines), new_wm->trans_wm.plane_res_l,
> - enast(new_wm->sagv_wm0.ignore_lines), new_wm->sagv_wm0.plane_res_l);
> + enast(new_wm->sagv.wm0.ignore_lines), new_wm->sagv.wm0.plane_res_l);
>
> drm_dbg_kms(&dev_priv->drm,
> "[PLANE:%d:%s] blocks %4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d"
> @@ -5929,13 +5929,13 @@ skl_print_wm_changes(struct intel_atomic_state *state)
> old_wm->wm[4].plane_res_b, old_wm->wm[5].plane_res_b,
> old_wm->wm[6].plane_res_b, old_wm->wm[7].plane_res_b,
> old_wm->trans_wm.plane_res_b,
> - old_wm->sagv_wm0.plane_res_b,
> + old_wm->sagv.wm0.plane_res_b,
> new_wm->wm[0].plane_res_b, new_wm->wm[1].plane_res_b,
> new_wm->wm[2].plane_res_b, new_wm->wm[3].plane_res_b,
> new_wm->wm[4].plane_res_b, new_wm->wm[5].plane_res_b,
> new_wm->wm[6].plane_res_b, new_wm->wm[7].plane_res_b,
> new_wm->trans_wm.plane_res_b,
> - new_wm->sagv_wm0.plane_res_b);
> + new_wm->sagv.wm0.plane_res_b);
>
> drm_dbg_kms(&dev_priv->drm,
> "[PLANE:%d:%s] min_ddb %4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d"
> @@ -5946,13 +5946,13 @@ skl_print_wm_changes(struct intel_atomic_state *state)
> old_wm->wm[4].min_ddb_alloc, old_wm->wm[5].min_ddb_alloc,
> old_wm->wm[6].min_ddb_alloc, old_wm->wm[7].min_ddb_alloc,
> old_wm->trans_wm.min_ddb_alloc,
> - old_wm->sagv_wm0.min_ddb_alloc,
> + old_wm->sagv.wm0.min_ddb_alloc,
> new_wm->wm[0].min_ddb_alloc, new_wm->wm[1].min_ddb_alloc,
> new_wm->wm[2].min_ddb_alloc, new_wm->wm[3].min_ddb_alloc,
> new_wm->wm[4].min_ddb_alloc, new_wm->wm[5].min_ddb_alloc,
> new_wm->wm[6].min_ddb_alloc, new_wm->wm[7].min_ddb_alloc,
> new_wm->trans_wm.min_ddb_alloc,
> - new_wm->sagv_wm0.min_ddb_alloc);
> + new_wm->sagv.wm0.min_ddb_alloc);
> }
> }
> }
> @@ -6189,7 +6189,7 @@ void skl_pipe_wm_get_hw_state(struct intel_crtc *crtc,
> }
>
> if (INTEL_GEN(dev_priv) >= 12)
> - wm->sagv_wm0 = wm->wm[0];
> + wm->sagv.wm0 = wm->wm[0];
>
> if (plane_id != PLANE_CURSOR)
> val = intel_uncore_read(&dev_priv->uncore, PLANE_WM_TRANS(pipe, plane_id));
> --
> 2.26.2
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Intel-gfx] [PATCH 5/7] drm/i915: Introduce SAGV transtion watermark
2021-02-26 15:32 ` [Intel-gfx] [PATCH 5/7] drm/i915: Introduce SAGV transtion watermark Ville Syrjala
@ 2021-03-01 9:21 ` Lisovskiy, Stanislav
0 siblings, 0 replies; 17+ messages in thread
From: Lisovskiy, Stanislav @ 2021-03-01 9:21 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx
On Fri, Feb 26, 2021 at 05:32:02PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Seems to me that if we calculate WM0 using the bumped up SAGV latency
> we need to calculate the transition watermark accordingly. Track it
> alongside the other watermarks.
>
> Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 4 +-
> .../drm/i915/display/intel_display_types.h | 1 +
> drivers/gpu/drm/i915/intel_pm.c | 94 ++++++++++++-------
> 3 files changed, 65 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 718e66f49332..e34e5a9da5c1 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -9459,7 +9459,9 @@ static void verify_wm_state(struct intel_crtc *crtc,
> }
>
> if (!skl_wm_level_equals(&hw_plane_wm->trans_wm,
> - &sw_plane_wm->trans_wm)) {
> + &sw_plane_wm->trans_wm) &&
> + !skl_wm_level_equals(&hw_plane_wm->trans_wm,
> + &sw_plane_wm->sagv.trans_wm)) {
> drm_err(&dev_priv->drm,
> "mismatch in trans WM pipe %c cursor (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n",
> pipe_name(pipe),
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 6321cd3df81e..e2365f2d07cc 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -734,6 +734,7 @@ struct skl_plane_wm {
> struct skl_wm_level trans_wm;
> struct {
> struct skl_wm_level wm0;
> + struct skl_wm_level trans_wm;
> } sagv;
> bool is_planar;
> };
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index a1591d9189a0..9d9ba63fc395 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4758,6 +4758,18 @@ skl_plane_wm_level(const struct skl_pipe_wm *pipe_wm,
> return &wm->wm[level];
> }
>
> +static const struct skl_wm_level *
> +skl_plane_trans_wm(const struct skl_pipe_wm *pipe_wm,
> + enum plane_id plane_id)
> +{
> + const struct skl_plane_wm *wm = &pipe_wm->planes[plane_id];
> +
> + if (pipe_wm->use_sagv_wm)
> + return &wm->sagv.trans_wm;
> +
> + return &wm->trans_wm;
> +}
> +
> static int
> skl_allocate_plane_ddb(struct intel_atomic_state *state,
> struct intel_crtc *crtc)
> @@ -4967,6 +4979,9 @@ skl_allocate_plane_ddb(struct intel_atomic_state *state,
>
> if (wm->sagv.wm0.plane_res_b >= total[plane_id])
> memset(&wm->sagv.wm0, 0, sizeof(wm->sagv.wm0));
> +
> + if (wm->sagv.trans_wm.plane_res_b >= total[plane_id])
> + memset(&wm->sagv.trans_wm, 0, sizeof(wm->sagv.trans_wm));
> }
>
> return 0;
> @@ -5325,12 +5340,11 @@ static void tgl_compute_sagv_wm(const struct intel_crtc_state *crtc_state,
> sagv_wm);
> }
>
> -static void skl_compute_transition_wm(const struct intel_crtc_state *crtc_state,
> - const struct skl_wm_params *wp,
> - struct skl_plane_wm *wm)
> +static void skl_compute_transition_wm(struct drm_i915_private *dev_priv,
> + struct skl_wm_level *trans_wm,
> + const struct skl_wm_level *wm0,
> + const struct skl_wm_params *wp)
> {
> - struct drm_device *dev = crtc_state->uapi.crtc->dev;
> - const struct drm_i915_private *dev_priv = to_i915(dev);
> u16 trans_min, trans_amount, trans_y_tile_min;
> u16 wm0_sel_res_b, trans_offset_b, res_blocks;
>
> @@ -5368,7 +5382,7 @@ static void skl_compute_transition_wm(const struct intel_crtc_state *crtc_state,
> * Result Blocks is Result Blocks minus 1 and it should work for the
> * current platforms.
> */
> - wm0_sel_res_b = wm->wm[0].plane_res_b - 1;
> + wm0_sel_res_b = wm0->plane_res_b - 1;
>
> if (wp->y_tiled) {
> trans_y_tile_min =
> @@ -5384,8 +5398,8 @@ static void skl_compute_transition_wm(const struct intel_crtc_state *crtc_state,
> * computing the DDB we'll come back and disable it if that
> * assumption turns out to be false.
> */
> - wm->trans_wm.plane_res_b = res_blocks + 1;
> - wm->trans_wm.plane_en = true;
> + trans_wm->plane_res_b = res_blocks + 1;
> + trans_wm->plane_en = true;
> }
>
> static int skl_build_plane_wm_single(struct intel_crtc_state *crtc_state,
> @@ -5405,10 +5419,15 @@ static int skl_build_plane_wm_single(struct intel_crtc_state *crtc_state,
>
> skl_compute_wm_levels(crtc_state, &wm_params, wm->wm);
>
> - if (INTEL_GEN(dev_priv) >= 12)
> + skl_compute_transition_wm(dev_priv, &wm->trans_wm,
> + &wm->wm[0], &wm_params);
> +
> + if (INTEL_GEN(dev_priv) >= 12) {
> tgl_compute_sagv_wm(crtc_state, &wm_params, wm);
>
> - skl_compute_transition_wm(crtc_state, &wm_params, wm);
> + skl_compute_transition_wm(dev_priv, &wm->sagv.trans_wm,
> + &wm->sagv.wm0, &wm_params);
> + }
>
> return 0;
> }
> @@ -5584,7 +5603,7 @@ void skl_write_plane_wm(struct intel_plane *plane,
> skl_plane_wm_level(pipe_wm, plane_id, level));
>
> skl_write_wm_level(dev_priv, PLANE_WM_TRANS(pipe, plane_id),
> - &wm->trans_wm);
> + skl_plane_trans_wm(pipe_wm, plane_id));
>
> if (INTEL_GEN(dev_priv) >= 11) {
> skl_ddb_entry_write(dev_priv,
> @@ -5609,7 +5628,6 @@ void skl_write_cursor_wm(struct intel_plane *plane,
> enum plane_id plane_id = plane->id;
> enum pipe pipe = plane->pipe;
> const struct skl_pipe_wm *pipe_wm = &crtc_state->wm.skl.optimal;
> - const struct skl_plane_wm *wm = &pipe_wm->planes[plane_id];
> const struct skl_ddb_entry *ddb =
> &crtc_state->wm.skl.plane_ddb_y[plane_id];
>
> @@ -5617,7 +5635,8 @@ void skl_write_cursor_wm(struct intel_plane *plane,
> skl_write_wm_level(dev_priv, CUR_WM(pipe, level),
> skl_plane_wm_level(pipe_wm, plane_id, level));
>
> - skl_write_wm_level(dev_priv, CUR_WM_TRANS(pipe), &wm->trans_wm);
> + skl_write_wm_level(dev_priv, CUR_WM_TRANS(pipe),
> + skl_plane_trans_wm(pipe_wm, plane_id));
>
> skl_ddb_entry_write(dev_priv, CUR_BUF_CFG(pipe), ddb);
> }
> @@ -5648,7 +5667,8 @@ static bool skl_plane_wm_equals(struct drm_i915_private *dev_priv,
> }
>
> return skl_wm_level_equals(&wm1->trans_wm, &wm2->trans_wm) &&
> - skl_wm_level_equals(&wm1->sagv.wm0, &wm2->sagv.wm0);
> + skl_wm_level_equals(&wm1->sagv.wm0, &wm2->sagv.wm0) &&
> + skl_wm_level_equals(&wm1->sagv.trans_wm, &wm2->sagv.trans_wm);
> }
>
> static bool skl_ddb_entries_overlap(const struct skl_ddb_entry *a,
> @@ -5878,8 +5898,8 @@ skl_print_wm_changes(struct intel_atomic_state *state)
> continue;
>
> drm_dbg_kms(&dev_priv->drm,
> - "[PLANE:%d:%s] level %cwm0,%cwm1,%cwm2,%cwm3,%cwm4,%cwm5,%cwm6,%cwm7,%ctwm,%cswm"
> - " -> %cwm0,%cwm1,%cwm2,%cwm3,%cwm4,%cwm5,%cwm6,%cwm7,%ctwm,%cswm\n",
> + "[PLANE:%d:%s] level %cwm0,%cwm1,%cwm2,%cwm3,%cwm4,%cwm5,%cwm6,%cwm7,%ctwm,%cswm,%cstwm"
> + " -> %cwm0,%cwm1,%cwm2,%cwm3,%cwm4,%cwm5,%cwm6,%cwm7,%ctwm,%cswm,%cstwm\n",
> plane->base.base.id, plane->base.name,
> enast(old_wm->wm[0].plane_en), enast(old_wm->wm[1].plane_en),
> enast(old_wm->wm[2].plane_en), enast(old_wm->wm[3].plane_en),
> @@ -5887,16 +5907,18 @@ skl_print_wm_changes(struct intel_atomic_state *state)
> enast(old_wm->wm[6].plane_en), enast(old_wm->wm[7].plane_en),
> enast(old_wm->trans_wm.plane_en),
> enast(old_wm->sagv.wm0.plane_en),
> + enast(old_wm->sagv.trans_wm.plane_en),
> enast(new_wm->wm[0].plane_en), enast(new_wm->wm[1].plane_en),
> enast(new_wm->wm[2].plane_en), enast(new_wm->wm[3].plane_en),
> enast(new_wm->wm[4].plane_en), enast(new_wm->wm[5].plane_en),
> enast(new_wm->wm[6].plane_en), enast(new_wm->wm[7].plane_en),
> enast(new_wm->trans_wm.plane_en),
> - enast(new_wm->sagv.wm0.plane_en));
> + enast(new_wm->sagv.wm0.plane_en),
> + enast(new_wm->sagv.trans_wm.plane_en));
>
> drm_dbg_kms(&dev_priv->drm,
> - "[PLANE:%d:%s] lines %c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d"
> - " -> %c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d\n",
> + "[PLANE:%d:%s] lines %c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%4d"
> + " -> %c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%4d\n",
> plane->base.base.id, plane->base.name,
> enast(old_wm->wm[0].ignore_lines), old_wm->wm[0].plane_res_l,
> enast(old_wm->wm[1].ignore_lines), old_wm->wm[1].plane_res_l,
> @@ -5908,7 +5930,7 @@ skl_print_wm_changes(struct intel_atomic_state *state)
> enast(old_wm->wm[7].ignore_lines), old_wm->wm[7].plane_res_l,
> enast(old_wm->trans_wm.ignore_lines), old_wm->trans_wm.plane_res_l,
> enast(old_wm->sagv.wm0.ignore_lines), old_wm->sagv.wm0.plane_res_l,
> -
> + enast(old_wm->sagv.trans_wm.ignore_lines), old_wm->sagv.trans_wm.plane_res_l,
> enast(new_wm->wm[0].ignore_lines), new_wm->wm[0].plane_res_l,
> enast(new_wm->wm[1].ignore_lines), new_wm->wm[1].plane_res_l,
> enast(new_wm->wm[2].ignore_lines), new_wm->wm[2].plane_res_l,
> @@ -5918,11 +5940,12 @@ skl_print_wm_changes(struct intel_atomic_state *state)
> enast(new_wm->wm[6].ignore_lines), new_wm->wm[6].plane_res_l,
> enast(new_wm->wm[7].ignore_lines), new_wm->wm[7].plane_res_l,
> enast(new_wm->trans_wm.ignore_lines), new_wm->trans_wm.plane_res_l,
> - enast(new_wm->sagv.wm0.ignore_lines), new_wm->sagv.wm0.plane_res_l);
> + enast(new_wm->sagv.wm0.ignore_lines), new_wm->sagv.wm0.plane_res_l,
> + enast(new_wm->sagv.trans_wm.ignore_lines), new_wm->sagv.trans_wm.plane_res_l);
>
> drm_dbg_kms(&dev_priv->drm,
> - "[PLANE:%d:%s] blocks %4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d"
> - " -> %4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d\n",
> + "[PLANE:%d:%s] blocks %4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%5d"
> + " -> %4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%5d\n",
> plane->base.base.id, plane->base.name,
> old_wm->wm[0].plane_res_b, old_wm->wm[1].plane_res_b,
> old_wm->wm[2].plane_res_b, old_wm->wm[3].plane_res_b,
> @@ -5930,16 +5953,18 @@ skl_print_wm_changes(struct intel_atomic_state *state)
> old_wm->wm[6].plane_res_b, old_wm->wm[7].plane_res_b,
> old_wm->trans_wm.plane_res_b,
> old_wm->sagv.wm0.plane_res_b,
> + old_wm->sagv.trans_wm.plane_res_b,
> new_wm->wm[0].plane_res_b, new_wm->wm[1].plane_res_b,
> new_wm->wm[2].plane_res_b, new_wm->wm[3].plane_res_b,
> new_wm->wm[4].plane_res_b, new_wm->wm[5].plane_res_b,
> new_wm->wm[6].plane_res_b, new_wm->wm[7].plane_res_b,
> new_wm->trans_wm.plane_res_b,
> - new_wm->sagv.wm0.plane_res_b);
> + new_wm->sagv.wm0.plane_res_b,
> + new_wm->sagv.trans_wm.plane_res_b);
>
> drm_dbg_kms(&dev_priv->drm,
> - "[PLANE:%d:%s] min_ddb %4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d"
> - " -> %4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d\n",
> + "[PLANE:%d:%s] min_ddb %4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%5d"
> + " -> %4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%5d\n",
> plane->base.base.id, plane->base.name,
> old_wm->wm[0].min_ddb_alloc, old_wm->wm[1].min_ddb_alloc,
> old_wm->wm[2].min_ddb_alloc, old_wm->wm[3].min_ddb_alloc,
> @@ -5947,12 +5972,14 @@ skl_print_wm_changes(struct intel_atomic_state *state)
> old_wm->wm[6].min_ddb_alloc, old_wm->wm[7].min_ddb_alloc,
> old_wm->trans_wm.min_ddb_alloc,
> old_wm->sagv.wm0.min_ddb_alloc,
> + old_wm->sagv.trans_wm.min_ddb_alloc,
> new_wm->wm[0].min_ddb_alloc, new_wm->wm[1].min_ddb_alloc,
> new_wm->wm[2].min_ddb_alloc, new_wm->wm[3].min_ddb_alloc,
> new_wm->wm[4].min_ddb_alloc, new_wm->wm[5].min_ddb_alloc,
> new_wm->wm[6].min_ddb_alloc, new_wm->wm[7].min_ddb_alloc,
> new_wm->trans_wm.min_ddb_alloc,
> - new_wm->sagv.wm0.min_ddb_alloc);
> + new_wm->sagv.wm0.min_ddb_alloc,
> + new_wm->sagv.trans_wm.min_ddb_alloc);
> }
> }
> }
> @@ -5961,8 +5988,6 @@ static bool skl_plane_selected_wm_equals(struct intel_plane *plane,
> const struct skl_pipe_wm *old_pipe_wm,
> const struct skl_pipe_wm *new_pipe_wm)
> {
> - const struct skl_plane_wm *old_wm = &old_pipe_wm->planes[plane->id];
> - const struct skl_plane_wm *new_wm = &new_pipe_wm->planes[plane->id];
> struct drm_i915_private *i915 = to_i915(plane->base.dev);
> int level, max_level = ilk_wm_max_level(i915);
>
> @@ -5977,7 +6002,8 @@ static bool skl_plane_selected_wm_equals(struct intel_plane *plane,
> return false;
> }
>
> - return skl_wm_level_equals(&old_wm->trans_wm, &new_wm->trans_wm);
> + return skl_wm_level_equals(skl_plane_trans_wm(old_pipe_wm, plane->id),
> + skl_plane_trans_wm(new_pipe_wm, plane->id));
> }
>
> /*
> @@ -6188,15 +6214,17 @@ void skl_pipe_wm_get_hw_state(struct intel_crtc *crtc,
> skl_wm_level_from_reg_val(val, &wm->wm[level]);
> }
>
> - if (INTEL_GEN(dev_priv) >= 12)
> - wm->sagv.wm0 = wm->wm[0];
> -
> if (plane_id != PLANE_CURSOR)
> val = intel_uncore_read(&dev_priv->uncore, PLANE_WM_TRANS(pipe, plane_id));
> else
> val = intel_uncore_read(&dev_priv->uncore, CUR_WM_TRANS(pipe));
>
> skl_wm_level_from_reg_val(val, &wm->trans_wm);
> +
> + if (INTEL_GEN(dev_priv) >= 12) {
> + wm->sagv.wm0 = wm->wm[0];
> + wm->sagv.trans_wm = wm->trans_wm;
> + }
> }
> }
>
> --
> 2.26.2
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Intel-gfx] [PATCH 6/7] drm/i915: Check tgl+ SAGV watermarks properly
2021-02-26 15:32 ` [Intel-gfx] [PATCH 6/7] drm/i915: Check tgl+ SAGV watermarks properly Ville Syrjala
@ 2021-03-01 9:24 ` Lisovskiy, Stanislav
0 siblings, 0 replies; 17+ messages in thread
From: Lisovskiy, Stanislav @ 2021-03-01 9:24 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx
On Fri, Feb 26, 2021 at 05:32:03PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> We know which WM0 (normal vs. SAGV) we supposedly programmed
> into the hardware, so just check against that instead of accepting
> either watermark as valid.
>
> Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 88 +++++++++-----------
> drivers/gpu/drm/i915/intel_pm.c | 4 +-
> drivers/gpu/drm/i915/intel_pm.h | 5 ++
> 3 files changed, 47 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index e34e5a9da5c1..e2ef31a93407 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -9377,41 +9377,40 @@ static void verify_wm_state(struct intel_crtc *crtc,
>
> /* planes */
> for_each_universal_plane(dev_priv, pipe, plane) {
> - struct skl_plane_wm *hw_plane_wm, *sw_plane_wm;
> -
> - hw_plane_wm = &hw->wm.planes[plane];
> - sw_plane_wm = &sw_wm->planes[plane];
> + const struct skl_wm_level *hw_wm_level, *sw_wm_level;
>
> /* Watermarks */
> for (level = 0; level <= max_level; level++) {
> - if (skl_wm_level_equals(&hw_plane_wm->wm[level],
> - &sw_plane_wm->wm[level]) ||
> - (level == 0 && skl_wm_level_equals(&hw_plane_wm->wm[level],
> - &sw_plane_wm->sagv.wm0)))
> + hw_wm_level = &hw->wm.planes[plane].wm[level];
> + sw_wm_level = skl_plane_wm_level(sw_wm, plane, level);
> +
> + if (skl_wm_level_equals(hw_wm_level, sw_wm_level))
> continue;
>
> drm_err(&dev_priv->drm,
> "mismatch in WM pipe %c plane %d level %d (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n",
> pipe_name(pipe), plane + 1, level,
> - sw_plane_wm->wm[level].plane_en,
> - sw_plane_wm->wm[level].plane_res_b,
> - sw_plane_wm->wm[level].plane_res_l,
> - hw_plane_wm->wm[level].plane_en,
> - hw_plane_wm->wm[level].plane_res_b,
> - hw_plane_wm->wm[level].plane_res_l);
> + sw_wm_level->plane_en,
> + sw_wm_level->plane_res_b,
> + sw_wm_level->plane_res_l,
> + hw_wm_level->plane_en,
> + hw_wm_level->plane_res_b,
> + hw_wm_level->plane_res_l);
> }
>
> - if (!skl_wm_level_equals(&hw_plane_wm->trans_wm,
> - &sw_plane_wm->trans_wm)) {
> + hw_wm_level = &hw->wm.planes[plane].trans_wm;
> + sw_wm_level = skl_plane_trans_wm(sw_wm, plane);
> +
> + if (!skl_wm_level_equals(hw_wm_level, sw_wm_level)) {
> drm_err(&dev_priv->drm,
> "mismatch in trans WM pipe %c plane %d (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n",
> pipe_name(pipe), plane + 1,
> - sw_plane_wm->trans_wm.plane_en,
> - sw_plane_wm->trans_wm.plane_res_b,
> - sw_plane_wm->trans_wm.plane_res_l,
> - hw_plane_wm->trans_wm.plane_en,
> - hw_plane_wm->trans_wm.plane_res_b,
> - hw_plane_wm->trans_wm.plane_res_l);
> + sw_wm_level->plane_en,
> + sw_wm_level->plane_res_b,
> + sw_wm_level->plane_res_l,
> + hw_wm_level->plane_en,
> + hw_wm_level->plane_res_b,
> + hw_wm_level->plane_res_l);
> }
>
> /* DDB */
> @@ -9434,43 +9433,36 @@ static void verify_wm_state(struct intel_crtc *crtc,
> * once the plane becomes visible, we can skip this check
> */
> if (1) {
> - struct skl_plane_wm *hw_plane_wm, *sw_plane_wm;
> -
> - hw_plane_wm = &hw->wm.planes[PLANE_CURSOR];
> - sw_plane_wm = &sw_wm->planes[PLANE_CURSOR];
> + const struct skl_wm_level *hw_wm_level, *sw_wm_level;
>
> /* Watermarks */
> for (level = 0; level <= max_level; level++) {
> - if (skl_wm_level_equals(&hw_plane_wm->wm[level],
> - &sw_plane_wm->wm[level]) ||
> - (level == 0 && skl_wm_level_equals(&hw_plane_wm->wm[level],
> - &sw_plane_wm->sagv.wm0)))
> - continue;
> -
> + hw_wm_level = &hw->wm.planes[PLANE_CURSOR].wm[level];
> + sw_wm_level = skl_plane_wm_level(sw_wm, PLANE_CURSOR, level);
> drm_err(&dev_priv->drm,
> "mismatch in WM pipe %c cursor level %d (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n",
> pipe_name(pipe), level,
> - sw_plane_wm->wm[level].plane_en,
> - sw_plane_wm->wm[level].plane_res_b,
> - sw_plane_wm->wm[level].plane_res_l,
> - hw_plane_wm->wm[level].plane_en,
> - hw_plane_wm->wm[level].plane_res_b,
> - hw_plane_wm->wm[level].plane_res_l);
> + sw_wm_level->plane_en,
> + sw_wm_level->plane_res_b,
> + sw_wm_level->plane_res_l,
> + hw_wm_level->plane_en,
> + hw_wm_level->plane_res_b,
> + hw_wm_level->plane_res_l);
> }
>
> - if (!skl_wm_level_equals(&hw_plane_wm->trans_wm,
> - &sw_plane_wm->trans_wm) &&
> - !skl_wm_level_equals(&hw_plane_wm->trans_wm,
> - &sw_plane_wm->sagv.trans_wm)) {
> + hw_wm_level = &hw->wm.planes[PLANE_CURSOR].trans_wm;
> + sw_wm_level = skl_plane_trans_wm(sw_wm, PLANE_CURSOR);
> +
> + if (!skl_wm_level_equals(hw_wm_level, sw_wm_level)) {
> drm_err(&dev_priv->drm,
> "mismatch in trans WM pipe %c cursor (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n",
> pipe_name(pipe),
> - sw_plane_wm->trans_wm.plane_en,
> - sw_plane_wm->trans_wm.plane_res_b,
> - sw_plane_wm->trans_wm.plane_res_l,
> - hw_plane_wm->trans_wm.plane_en,
> - hw_plane_wm->trans_wm.plane_res_b,
> - hw_plane_wm->trans_wm.plane_res_l);
> + sw_wm_level->plane_en,
> + sw_wm_level->plane_res_b,
> + sw_wm_level->plane_res_l,
> + hw_wm_level->plane_en,
> + hw_wm_level->plane_res_b,
> + hw_wm_level->plane_res_l);
> }
>
> /* DDB */
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 9d9ba63fc395..854ffecd98d9 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4745,7 +4745,7 @@ icl_get_total_relative_data_rate(struct intel_atomic_state *state,
> return total_data_rate;
> }
>
> -static const struct skl_wm_level *
> +const struct skl_wm_level *
> skl_plane_wm_level(const struct skl_pipe_wm *pipe_wm,
> enum plane_id plane_id,
> int level)
> @@ -4758,7 +4758,7 @@ skl_plane_wm_level(const struct skl_pipe_wm *pipe_wm,
> return &wm->wm[level];
> }
>
> -static const struct skl_wm_level *
> +const struct skl_wm_level *
> skl_plane_trans_wm(const struct skl_pipe_wm *pipe_wm,
> enum plane_id plane_id)
> {
> diff --git a/drivers/gpu/drm/i915/intel_pm.h b/drivers/gpu/drm/i915/intel_pm.h
> index 97550cf0b6df..669c8d505677 100644
> --- a/drivers/gpu/drm/i915/intel_pm.h
> +++ b/drivers/gpu/drm/i915/intel_pm.h
> @@ -52,6 +52,11 @@ bool intel_can_enable_sagv(struct drm_i915_private *dev_priv,
> const struct intel_bw_state *bw_state);
> void intel_sagv_pre_plane_update(struct intel_atomic_state *state);
> void intel_sagv_post_plane_update(struct intel_atomic_state *state);
> +const struct skl_wm_level *skl_plane_wm_level(const struct skl_pipe_wm *pipe_wm,
> + enum plane_id plane_id,
> + int level);
> +const struct skl_wm_level *skl_plane_trans_wm(const struct skl_pipe_wm *pipe_wm,
> + enum plane_id plane_id);
> bool skl_wm_level_equals(const struct skl_wm_level *l1,
> const struct skl_wm_level *l2);
> bool skl_ddb_allocation_overlaps(const struct skl_ddb_entry *ddb,
> --
> 2.26.2
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Intel-gfx] [PATCH 7/7] drm/i915: Clean up verify_wm_state()
2021-02-26 15:32 ` [Intel-gfx] [PATCH 7/7] drm/i915: Clean up verify_wm_state() Ville Syrjala
@ 2021-03-01 9:27 ` Lisovskiy, Stanislav
0 siblings, 0 replies; 17+ messages in thread
From: Lisovskiy, Stanislav @ 2021-03-01 9:27 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx
On Fri, Feb 26, 2021 at 05:32:04PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Get rid of the nonsense cursor special case in verify_wm_state()
> by just iterating through all the planes. And let's use the
> canonical [PLANE:..] style in the debug prints while at it.
Great move!
Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
>
> Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 88 ++++----------------
> drivers/gpu/drm/i915/display/intel_display.h | 5 --
> 2 files changed, 17 insertions(+), 76 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index e2ef31a93407..e0ecd8eea68d 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -9348,11 +9348,10 @@ static void verify_wm_state(struct intel_crtc *crtc,
> struct skl_ddb_entry ddb_uv[I915_MAX_PLANES];
> struct skl_pipe_wm wm;
> } *hw;
> - struct skl_pipe_wm *sw_wm;
> - struct skl_ddb_entry *hw_ddb_entry, *sw_ddb_entry;
> + const struct skl_pipe_wm *sw_wm = &new_crtc_state->wm.skl.optimal;
> + int level, max_level = ilk_wm_max_level(dev_priv);
> + struct intel_plane *plane;
> u8 hw_enabled_slices;
> - const enum pipe pipe = crtc->pipe;
> - int plane, level, max_level = ilk_wm_max_level(dev_priv);
>
> if (INTEL_GEN(dev_priv) < 9 || !new_crtc_state->hw.active)
> return;
> @@ -9362,7 +9361,6 @@ static void verify_wm_state(struct intel_crtc *crtc,
> return;
>
> skl_pipe_wm_get_hw_state(crtc, &hw->wm);
> - sw_wm = &new_crtc_state->wm.skl.optimal;
>
> skl_pipe_ddb_get_hw_state(crtc, hw->ddb_y, hw->ddb_uv);
>
> @@ -9375,21 +9373,21 @@ static void verify_wm_state(struct intel_crtc *crtc,
> dev_priv->dbuf.enabled_slices,
> hw_enabled_slices);
>
> - /* planes */
> - for_each_universal_plane(dev_priv, pipe, plane) {
> + for_each_intel_plane_on_crtc(&dev_priv->drm, crtc, plane) {
> + const struct skl_ddb_entry *hw_ddb_entry, *sw_ddb_entry;
> const struct skl_wm_level *hw_wm_level, *sw_wm_level;
>
> /* Watermarks */
> for (level = 0; level <= max_level; level++) {
> - hw_wm_level = &hw->wm.planes[plane].wm[level];
> - sw_wm_level = skl_plane_wm_level(sw_wm, plane, level);
> + hw_wm_level = &hw->wm.planes[plane->id].wm[level];
> + sw_wm_level = skl_plane_wm_level(sw_wm, plane->id, level);
>
> if (skl_wm_level_equals(hw_wm_level, sw_wm_level))
> continue;
>
> drm_err(&dev_priv->drm,
> - "mismatch in WM pipe %c plane %d level %d (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n",
> - pipe_name(pipe), plane + 1, level,
> + "[PLANE:%d:%s] mismatch in WM%d (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n",
> + plane->base.base.id, plane->base.name, level,
> sw_wm_level->plane_en,
> sw_wm_level->plane_res_b,
> sw_wm_level->plane_res_l,
> @@ -9398,13 +9396,13 @@ static void verify_wm_state(struct intel_crtc *crtc,
> hw_wm_level->plane_res_l);
> }
>
> - hw_wm_level = &hw->wm.planes[plane].trans_wm;
> - sw_wm_level = skl_plane_trans_wm(sw_wm, plane);
> + hw_wm_level = &hw->wm.planes[plane->id].trans_wm;
> + sw_wm_level = skl_plane_trans_wm(sw_wm, plane->id);
>
> if (!skl_wm_level_equals(hw_wm_level, sw_wm_level)) {
> drm_err(&dev_priv->drm,
> - "mismatch in trans WM pipe %c plane %d (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n",
> - pipe_name(pipe), plane + 1,
> + "[PLANE:%d:%s] mismatch in trans WM (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n",
> + plane->base.base.id, plane->base.name,
> sw_wm_level->plane_en,
> sw_wm_level->plane_res_b,
> sw_wm_level->plane_res_l,
> @@ -9414,65 +9412,13 @@ static void verify_wm_state(struct intel_crtc *crtc,
> }
>
> /* DDB */
> - hw_ddb_entry = &hw->ddb_y[plane];
> - sw_ddb_entry = &new_crtc_state->wm.skl.plane_ddb_y[plane];
> + hw_ddb_entry = &hw->ddb_y[plane->id];
> + sw_ddb_entry = &new_crtc_state->wm.skl.plane_ddb_y[plane->id];
>
> if (!skl_ddb_entry_equal(hw_ddb_entry, sw_ddb_entry)) {
> drm_err(&dev_priv->drm,
> - "mismatch in DDB state pipe %c plane %d (expected (%u,%u), found (%u,%u))\n",
> - pipe_name(pipe), plane + 1,
> - sw_ddb_entry->start, sw_ddb_entry->end,
> - hw_ddb_entry->start, hw_ddb_entry->end);
> - }
> - }
> -
> - /*
> - * cursor
> - * If the cursor plane isn't active, we may not have updated it's ddb
> - * allocation. In that case since the ddb allocation will be updated
> - * once the plane becomes visible, we can skip this check
> - */
> - if (1) {
> - const struct skl_wm_level *hw_wm_level, *sw_wm_level;
> -
> - /* Watermarks */
> - for (level = 0; level <= max_level; level++) {
> - hw_wm_level = &hw->wm.planes[PLANE_CURSOR].wm[level];
> - sw_wm_level = skl_plane_wm_level(sw_wm, PLANE_CURSOR, level);
> - drm_err(&dev_priv->drm,
> - "mismatch in WM pipe %c cursor level %d (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n",
> - pipe_name(pipe), level,
> - sw_wm_level->plane_en,
> - sw_wm_level->plane_res_b,
> - sw_wm_level->plane_res_l,
> - hw_wm_level->plane_en,
> - hw_wm_level->plane_res_b,
> - hw_wm_level->plane_res_l);
> - }
> -
> - hw_wm_level = &hw->wm.planes[PLANE_CURSOR].trans_wm;
> - sw_wm_level = skl_plane_trans_wm(sw_wm, PLANE_CURSOR);
> -
> - if (!skl_wm_level_equals(hw_wm_level, sw_wm_level)) {
> - drm_err(&dev_priv->drm,
> - "mismatch in trans WM pipe %c cursor (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n",
> - pipe_name(pipe),
> - sw_wm_level->plane_en,
> - sw_wm_level->plane_res_b,
> - sw_wm_level->plane_res_l,
> - hw_wm_level->plane_en,
> - hw_wm_level->plane_res_b,
> - hw_wm_level->plane_res_l);
> - }
> -
> - /* DDB */
> - hw_ddb_entry = &hw->ddb_y[PLANE_CURSOR];
> - sw_ddb_entry = &new_crtc_state->wm.skl.plane_ddb_y[PLANE_CURSOR];
> -
> - if (!skl_ddb_entry_equal(hw_ddb_entry, sw_ddb_entry)) {
> - drm_err(&dev_priv->drm,
> - "mismatch in DDB state pipe %c cursor (expected (%u,%u), found (%u,%u))\n",
> - pipe_name(pipe),
> + "[PLANE:%d:%s] mismatch in DDB (expected (%u,%u), found (%u,%u))\n",
> + plane->base.base.id, plane->base.name,
> sw_ddb_entry->start, sw_ddb_entry->end,
> hw_ddb_entry->start, hw_ddb_entry->end);
> }
> diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> index 61080152319f..216047233a6d 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.h
> +++ b/drivers/gpu/drm/i915/display/intel_display.h
> @@ -353,11 +353,6 @@ enum phy_fia {
> for_each_cpu_transcoder(__dev_priv, __t) \
> for_each_if ((__mask) & BIT(__t))
>
> -#define for_each_universal_plane(__dev_priv, __pipe, __p) \
> - for ((__p) = 0; \
> - (__p) < RUNTIME_INFO(__dev_priv)->num_sprites[(__pipe)] + 1; \
> - (__p)++)
> -
> #define for_each_sprite(__dev_priv, __p, __s) \
> for ((__s) = 0; \
> (__s) < RUNTIME_INFO(__dev_priv)->num_sprites[(__p)]; \
> --
> 2.26.2
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2021-03-01 9:25 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-26 15:31 [Intel-gfx] [PATCH 0/7] drm/i915: Fix up TGL+ SAGV watermarks Ville Syrjala
2021-02-26 15:31 ` [Intel-gfx] [PATCH 1/7] drm/i915: Fix TGL+ plane SAGV watermark programming Ville Syrjala
2021-03-01 8:38 ` Lisovskiy, Stanislav
2021-02-26 15:31 ` [Intel-gfx] [PATCH 2/7] drm/i915: Zero out SAGV wm when we don't have enough DDB for it Ville Syrjala
2021-03-01 8:42 ` Lisovskiy, Stanislav
2021-02-26 15:32 ` [Intel-gfx] [PATCH 3/7] drm/i915: Print wm changes if sagv_wm0 changes Ville Syrjala
2021-03-01 9:14 ` Lisovskiy, Stanislav
2021-02-26 15:32 ` [Intel-gfx] [PATCH 4/7] drm/i915: Stuff SAGV watermark into a sub-structure Ville Syrjala
2021-03-01 9:17 ` Lisovskiy, Stanislav
2021-02-26 15:32 ` [Intel-gfx] [PATCH 5/7] drm/i915: Introduce SAGV transtion watermark Ville Syrjala
2021-03-01 9:21 ` Lisovskiy, Stanislav
2021-02-26 15:32 ` [Intel-gfx] [PATCH 6/7] drm/i915: Check tgl+ SAGV watermarks properly Ville Syrjala
2021-03-01 9:24 ` Lisovskiy, Stanislav
2021-02-26 15:32 ` [Intel-gfx] [PATCH 7/7] drm/i915: Clean up verify_wm_state() Ville Syrjala
2021-03-01 9:27 ` Lisovskiy, Stanislav
2021-02-26 15:44 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Fix up TGL+ SAGV watermarks Patchwork
2021-02-26 16:14 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).