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 08/15] drm/i915: Split plane data_rate into data_rate+data_rate_y
Date: Tue, 1 Feb 2022 10:08:39 +0200	[thread overview]
Message-ID: <20220201080839.GB9466@intel.com> (raw)
In-Reply-To: <20220118092354.11631-9-ville.syrjala@linux.intel.com>

On Tue, Jan 18, 2022 at 11:23:47AM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Split the currently combined plane data_rate into the proper
> Y vs. CbCr components. This matches how we now track the
> plane dbuf allocations, and thus will make the dbuf bandwidth
> calculations actually produce the correct numbers for each
> dbuf slice.

Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>

> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  .../gpu/drm/i915/display/intel_atomic_plane.c | 34 ++++++++----------
>  .../gpu/drm/i915/display/intel_atomic_plane.h |  3 +-
>  drivers/gpu/drm/i915/display/intel_bw.c       | 36 +++++++++----------
>  drivers/gpu/drm/i915/display/intel_display.c  |  4 +++
>  .../drm/i915/display/intel_display_types.h    |  3 ++
>  5 files changed, 42 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> index 52239351931c..cd18155830d4 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> @@ -180,29 +180,16 @@ unsigned int intel_plane_pixel_rate(const struct intel_crtc_state *crtc_state,
>  }
>  
>  unsigned int intel_plane_data_rate(const struct intel_crtc_state *crtc_state,
> -				   const struct intel_plane_state *plane_state)
> +				   const struct intel_plane_state *plane_state,
> +				   int color_plane)
>  {
>  	const struct drm_framebuffer *fb = plane_state->hw.fb;
> -	unsigned int cpp;
> -	unsigned int pixel_rate;
>  
>  	if (!plane_state->uapi.visible)
>  		return 0;
>  
> -	pixel_rate = intel_plane_pixel_rate(crtc_state, plane_state);
> -
> -	cpp = fb->format->cpp[0];
> -
> -	/*
> -	 * Based on HSD#:1408715493
> -	 * NV12 cpp == 4, P010 cpp == 8
> -	 *
> -	 * FIXME what is the logic behind this?
> -	 */
> -	if (fb->format->is_yuv && fb->format->num_planes > 1)
> -		cpp *= 4;
> -
> -	return pixel_rate * cpp;
> +	return intel_plane_pixel_rate(crtc_state, plane_state) *
> +		fb->format->cpp[color_plane];
>  }
>  
>  int intel_plane_calc_min_cdclk(struct intel_atomic_state *state,
> @@ -324,6 +311,7 @@ void intel_plane_set_invisible(struct intel_crtc_state *crtc_state,
>  	crtc_state->nv12_planes &= ~BIT(plane->id);
>  	crtc_state->c8_planes &= ~BIT(plane->id);
>  	crtc_state->data_rate[plane->id] = 0;
> +	crtc_state->data_rate_y[plane->id] = 0;
>  	crtc_state->min_cdclk[plane->id] = 0;
>  
>  	plane_state->uapi.visible = false;
> @@ -366,8 +354,16 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_
>  	if (new_plane_state->uapi.visible || old_plane_state->uapi.visible)
>  		new_crtc_state->update_planes |= BIT(plane->id);
>  
> -	new_crtc_state->data_rate[plane->id] =
> -		intel_plane_data_rate(new_crtc_state, new_plane_state);
> +	if (new_plane_state->uapi.visible &&
> +	    intel_format_info_is_yuv_semiplanar(fb->format, fb->modifier)) {
> +		new_crtc_state->data_rate_y[plane->id] =
> +			intel_plane_data_rate(new_crtc_state, new_plane_state, 0);
> +		new_crtc_state->data_rate[plane->id] =
> +			intel_plane_data_rate(new_crtc_state, new_plane_state, 1);
> +	} else if (new_plane_state->uapi.visible) {
> +		new_crtc_state->data_rate[plane->id] =
> +			intel_plane_data_rate(new_crtc_state, new_plane_state, 0);
> +	}
>  
>  	return intel_plane_atomic_calc_changes(old_crtc_state, new_crtc_state,
>  					       old_plane_state, new_plane_state);
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.h b/drivers/gpu/drm/i915/display/intel_atomic_plane.h
> index 7907f601598e..aa26ce5fb654 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.h
> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.h
> @@ -24,7 +24,8 @@ unsigned int intel_plane_pixel_rate(const struct intel_crtc_state *crtc_state,
>  				    const struct intel_plane_state *plane_state);
>  
>  unsigned int intel_plane_data_rate(const struct intel_crtc_state *crtc_state,
> -				   const struct intel_plane_state *plane_state);
> +				   const struct intel_plane_state *plane_state,
> +				   int color_plane);
>  void intel_plane_copy_uapi_to_hw_state(struct intel_plane_state *plane_state,
>  				       const struct intel_plane_state *from_plane_state,
>  				       struct intel_crtc *crtc);
> diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
> index 82f0435bcb6d..93feab671c29 100644
> --- a/drivers/gpu/drm/i915/display/intel_bw.c
> +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> @@ -576,6 +576,7 @@ static unsigned int intel_bw_crtc_num_active_planes(const struct intel_crtc_stat
>  static unsigned int intel_bw_crtc_data_rate(const struct intel_crtc_state *crtc_state)
>  {
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> +	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
>  	unsigned int data_rate = 0;
>  	enum plane_id plane_id;
>  
> @@ -588,6 +589,9 @@ static unsigned int intel_bw_crtc_data_rate(const struct intel_crtc_state *crtc_
>  			continue;
>  
>  		data_rate += crtc_state->data_rate[plane_id];
> +
> +		if (DISPLAY_VER(i915) < 11)
> +			data_rate += crtc_state->data_rate_y[plane_id];
>  	}
>  
>  	return data_rate;
> @@ -688,28 +692,24 @@ static void skl_crtc_calc_dbuf_bw(struct intel_bw_state *bw_state,
>  	for_each_plane_id_on_crtc(crtc, plane_id) {
>  		const struct skl_ddb_entry *ddb =
>  			&crtc_state->wm.skl.plane_ddb[plane_id];
> -		const struct skl_ddb_entry *ddb_y =
> -			&crtc_state->wm.skl.plane_ddb_y[plane_id];
>  		unsigned int data_rate = crtc_state->data_rate[plane_id];
> -		unsigned int dbuf_mask = 0;
> +		unsigned int dbuf_mask = skl_ddb_dbuf_slice_mask(i915, ddb);
>  		enum dbuf_slice slice;
>  
> -		dbuf_mask |= skl_ddb_dbuf_slice_mask(i915, ddb);
> -		dbuf_mask |= skl_ddb_dbuf_slice_mask(i915, ddb_y);
> +		for_each_dbuf_slice_in_mask(i915, slice, dbuf_mask)
> +			crtc_bw->used_bw[slice] += data_rate;
> +	}
> +
> +	if (DISPLAY_VER(i915) >= 11)
> +		return;
> +
> +	for_each_plane_id_on_crtc(crtc, plane_id) {
> +		const struct skl_ddb_entry *ddb =
> +			&crtc_state->wm.skl.plane_ddb_y[plane_id];
> +		unsigned int data_rate = crtc_state->data_rate_y[plane_id];
> +		unsigned int dbuf_mask = skl_ddb_dbuf_slice_mask(i915, ddb);
> +		enum dbuf_slice slice;
>  
> -		/*
> -		 * FIXME: To calculate that more properly we probably
> -		 * need to split per plane data_rate into data_rate_y
> -		 * and data_rate_uv for multiplanar formats in order not
> -		 * to get accounted those twice if they happen to reside
> -		 * on different slices.
> -		 * However for pre-icl this would work anyway because
> -		 * we have only single slice and for icl+ uv plane has
> -		 * non-zero data rate.
> -		 * So in worst case those calculation are a bit
> -		 * pessimistic, which shouldn't pose any significant
> -		 * problem anyway.
> -		 */
>  		for_each_dbuf_slice_in_mask(i915, slice, dbuf_mask)
>  			crtc_bw->used_bw[slice] += data_rate;
>  	}
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index af23153f6502..39dd2e7383e0 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -761,6 +761,7 @@ void intel_plane_disable_noatomic(struct intel_crtc *crtc,
>  	intel_set_plane_visible(crtc_state, plane_state, false);
>  	fixup_plane_bitmasks(crtc_state);
>  	crtc_state->data_rate[plane->id] = 0;
> +	crtc_state->data_rate_y[plane->id] = 0;
>  	crtc_state->min_cdclk[plane->id] = 0;
>  
>  	if (plane->id == PLANE_PRIMARY)
> @@ -5110,6 +5111,7 @@ static int icl_check_nv12_planes(struct intel_crtc_state *crtc_state)
>  			crtc_state->enabled_planes &= ~BIT(plane->id);
>  			crtc_state->active_planes &= ~BIT(plane->id);
>  			crtc_state->update_planes |= BIT(plane->id);
> +			crtc_state->data_rate[plane->id] = 0;
>  		}
>  
>  		plane_state->planar_slave = false;
> @@ -5154,6 +5156,8 @@ static int icl_check_nv12_planes(struct intel_crtc_state *crtc_state)
>  		crtc_state->enabled_planes |= BIT(linked->id);
>  		crtc_state->active_planes |= BIT(linked->id);
>  		crtc_state->update_planes |= BIT(linked->id);
> +		crtc_state->data_rate[linked->id] =
> +			crtc_state->data_rate_y[plane->id];
>  		drm_dbg_kms(&dev_priv->drm, "Using %s as Y plane for %s\n",
>  			    linked->base.name, plane->base.name);
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 578c6069376b..7e147e110059 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1148,7 +1148,10 @@ struct intel_crtc_state {
>  
>  	int min_cdclk[I915_MAX_PLANES];
>  
> +	/* for packed/planar CbCr */
>  	u32 data_rate[I915_MAX_PLANES];
> +	/* for planar Y */
> +	u32 data_rate_y[I915_MAX_PLANES];
>  
>  	/* FIXME unify with data_rate[] */
>  	u64 plane_data_rate[I915_MAX_PLANES];
> -- 
> 2.32.0
> 

  reply	other threads:[~2022-02-01  8:08 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-18  9:23 [Intel-gfx] [PATCH 00/15] drm/i915: Fix bandwith related cdclk calculations Ville Syrjala
2022-01-18  9:23 ` [Intel-gfx] [PATCH 01/15] drm/i915: Drop pointless dev_priv argument Ville Syrjala
2022-01-27  8:15   ` Lisovskiy, Stanislav
2022-01-18  9:23 ` [Intel-gfx] [PATCH 02/15] drm/i915: Extract skl_ddb_entry_init() Ville Syrjala
2022-01-27  8:16   ` Lisovskiy, Stanislav
2022-01-18  9:23 ` [Intel-gfx] [PATCH 03/15] drm/i915: Fix plane relative_data_rate calculation Ville Syrjala
2022-01-27  8:21   ` Lisovskiy, Stanislav
2022-01-27  8:50     ` Ville Syrjälä
2022-01-18  9:23 ` [Intel-gfx] [PATCH 04/15] drm/i915: Introduce skl_plane_ddb_iter Ville Syrjala
2022-01-27  8:22   ` Lisovskiy, Stanislav
2022-01-18  9:23 ` [Intel-gfx] [PATCH 05/15] drm/i915: Extract skl_allocate_plane_ddb() Ville Syrjala
2022-01-27  8:24   ` Lisovskiy, Stanislav
2022-01-18  9:23 ` [Intel-gfx] [PATCH 06/15] drm/i915: Extract skl_crtc_calc_dbuf_bw() Ville Syrjala
2022-01-27  8:24   ` Lisovskiy, Stanislav
2022-01-18  9:23 ` [Intel-gfx] [PATCH 07/15] drm/i915: Tweak plane ddb allocation tracking Ville Syrjala
2022-02-01  8:06   ` Lisovskiy, Stanislav
2022-01-18  9:23 ` [Intel-gfx] [PATCH 08/15] drm/i915: Split plane data_rate into data_rate+data_rate_y Ville Syrjala
2022-02-01  8:08   ` Lisovskiy, Stanislav [this message]
2022-01-18  9:23 ` [Intel-gfx] [PATCH 09/15] drm/i915: Pre-calculate plane relative data rate Ville Syrjala
2022-02-01  8:11   ` Lisovskiy, Stanislav
2022-01-18  9:23 ` [Intel-gfx] [PATCH 10/15] drm/i915: Remove total[] and uv_total[] from ddb allocation Ville Syrjala
2022-02-01  8:26   ` Lisovskiy, Stanislav
2022-01-18  9:23 ` [Intel-gfx] [PATCH 11/15] drm/i915: Nuke intel_bw_calc_min_cdclk() Ville Syrjala
2022-02-01  8:52   ` Lisovskiy, Stanislav
2022-02-01 10:05     ` Ville Syrjälä
2022-02-01 11:18       ` Lisovskiy, Stanislav
2022-02-01 13:45         ` Ville Syrjälä
2022-02-01 14:38           ` Lisovskiy, Stanislav
2022-01-18  9:23 ` [Intel-gfx] [PATCH 12/15] drm/i915: Round up when calculating display bandwidth requirements Ville Syrjala
2022-01-18  9:23 ` [Intel-gfx] [PATCH 13/15] drm/i915: Properly write lock bw_state when it changes Ville Syrjala
2022-01-18  9:23 ` [Intel-gfx] [PATCH 14/15] drm/i915: Fix DBUF bandwidth vs. cdclk handling Ville Syrjala
2022-01-18  9:23 ` [Intel-gfx] [PATCH 15/15] drm/i915: Add "maximum pipe read bandwidth" checks Ville Syrjala
2022-01-18  9:46 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Fix bandwith related cdclk calculations Patchwork
2022-01-18  9:47 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2022-01-18 10:12 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2022-01-18 11:32 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " 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=20220201080839.GB9466@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.