All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Lisovskiy, Stanislav" <stanislav.lisovskiy@intel.com>
To: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 6/7] drm/i915: Check tgl+ SAGV watermarks properly
Date: Mon, 1 Mar 2021 11:24:54 +0200	[thread overview]
Message-ID: <20210301092454.GC22174@intel.com> (raw)
In-Reply-To: <20210226153204.1270-7-ville.syrjala@linux.intel.com>

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

  reply	other threads:[~2021-03-01  9:23 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-26 15:31 [Intel-gfx] [PATCH 0/7] drm/i915: Fix up TGL+ SAGV watermarks Ville Syrjala
2021-02-26 15:31 ` [Intel-gfx] [PATCH 1/7] drm/i915: Fix TGL+ plane SAGV watermark programming Ville Syrjala
2021-03-01  8:38   ` Lisovskiy, Stanislav
2021-02-26 15:31 ` [Intel-gfx] [PATCH 2/7] drm/i915: Zero out SAGV wm when we don't have enough DDB for it Ville Syrjala
2021-03-01  8:42   ` Lisovskiy, Stanislav
2021-02-26 15:32 ` [Intel-gfx] [PATCH 3/7] drm/i915: Print wm changes if sagv_wm0 changes Ville Syrjala
2021-03-01  9:14   ` Lisovskiy, Stanislav
2021-02-26 15:32 ` [Intel-gfx] [PATCH 4/7] drm/i915: Stuff SAGV watermark into a sub-structure Ville Syrjala
2021-03-01  9:17   ` Lisovskiy, Stanislav
2021-02-26 15:32 ` [Intel-gfx] [PATCH 5/7] drm/i915: Introduce SAGV transtion watermark Ville Syrjala
2021-03-01  9:21   ` Lisovskiy, Stanislav
2021-02-26 15:32 ` [Intel-gfx] [PATCH 6/7] drm/i915: Check tgl+ SAGV watermarks properly Ville Syrjala
2021-03-01  9:24   ` Lisovskiy, Stanislav [this message]
2021-02-26 15:32 ` [Intel-gfx] [PATCH 7/7] drm/i915: Clean up verify_wm_state() Ville Syrjala
2021-03-01  9:27   ` Lisovskiy, Stanislav
2021-02-26 15:44 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Fix up TGL+ SAGV watermarks Patchwork
2021-02-26 16:14 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210301092454.GC22174@intel.com \
    --to=stanislav.lisovskiy@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=ville.syrjala@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.