[Intel-gfx] [PATCH 0/7] drm/i915: Fix up TGL+ SAGV watermarks

[Intel-gfx] [PATCH 0/7] drm/i915: Fix up TGL+ SAGV watermarks

[Ville Syrjala]

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

[Intel-gfx] [PATCH 1/7] drm/i915: Fix TGL+ plane SAGV watermark programming

[Ville Syrjala]

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

Re: [Intel-gfx] [PATCH 1/7] drm/i915: Fix TGL+ plane SAGV watermark programming

[Lisovskiy, Stanislav]

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

[Intel-gfx] [PATCH 2/7] drm/i915: Zero out SAGV wm when we don't have enough DDB for it

[Ville Syrjala]

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

Re: [Intel-gfx] [PATCH 2/7] drm/i915: Zero out SAGV wm when we don't have enough DDB for it

[Lisovskiy, Stanislav]

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

[Intel-gfx] [PATCH 3/7] drm/i915: Print wm changes if sagv_wm0 changes

[Ville Syrjala]

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

Re: [Intel-gfx] [PATCH 3/7] drm/i915: Print wm changes if sagv_wm0 changes

[Lisovskiy, Stanislav]

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

[Intel-gfx] [PATCH 4/7] drm/i915: Stuff SAGV watermark into a sub-structure

[Ville Syrjala]

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

Re: [Intel-gfx] [PATCH 4/7] drm/i915: Stuff SAGV watermark into a sub-structure

[Lisovskiy, Stanislav]

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

[Intel-gfx] [PATCH 5/7] drm/i915: Introduce SAGV transtion watermark

[Ville Syrjala]

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

Re: [Intel-gfx] [PATCH 5/7] drm/i915: Introduce SAGV transtion watermark

[Lisovskiy, Stanislav]

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

[Intel-gfx] [PATCH 6/7] drm/i915: Check tgl+ SAGV watermarks properly

[Ville Syrjala]

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

Re: [Intel-gfx] [PATCH 6/7] drm/i915: Check tgl+ SAGV watermarks properly

[Lisovskiy, Stanislav]

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

[Intel-gfx] [PATCH 7/7] drm/i915: Clean up verify_wm_state()

[Ville Syrjala]

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

Re: [Intel-gfx] [PATCH 7/7] drm/i915: Clean up verify_wm_state()

[Lisovskiy, Stanislav]

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

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Fix up TGL+ SAGV watermarks

[Patchwork]

== 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

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Fix up TGL+ SAGV watermarks

[Patchwork]

[-- 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