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 09/15] drm/i915: Pre-calculate plane relative data rate
Date: Tue, 1 Feb 2022 10:11:14 +0200 [thread overview]
Message-ID: <20220201081114.GC9466@intel.com> (raw)
In-Reply-To: <20220118092354.11631-10-ville.syrjala@linux.intel.com>
On Tue, Jan 18, 2022 at 11:23:48AM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Handle the plane relative data rate in exactly the same
> way as we already handle the real data rate. Ie. pre-calculate
> it during intel_plane_atomic_check_with_state(), and assign/clear
> it for the Y plane as needed. This should guarantee that the
> tracking is 100% consistent, and makes me have to think less
> when the same apporach is used by both types of data rate.
>
> We might even want to consider replacing the relative
> data rate with the real data rate entirely, but it's not
> clear if that will produce less optimal plane ddb
> allocations. So for now lets keep using the current approach.
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 | 37 ++++
> drivers/gpu/drm/i915/display/intel_display.c | 5 +
> .../drm/i915/display/intel_display_types.h | 6 +-
> drivers/gpu/drm/i915/intel_pm.c | 170 ++++--------------
> 4 files changed, 80 insertions(+), 138 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> index cd18155830d4..a61344dcfb94 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> @@ -192,6 +192,33 @@ unsigned int intel_plane_data_rate(const struct intel_crtc_state *crtc_state,
> fb->format->cpp[color_plane];
> }
>
> +static unsigned int
> +intel_plane_relative_data_rate(const struct intel_plane_state *plane_state,
> + int color_plane)
> +{
> + const struct drm_framebuffer *fb = plane_state->hw.fb;
> + int width, height;
> +
> + if (!plane_state->uapi.visible)
> + return 0;
> +
> + /*
> + * Src coordinates are already rotated by 270 degrees for
> + * the 90/270 degree plane rotation cases (to match the
> + * GTT mapping), hence no need to account for rotation here.
> + */
> + width = drm_rect_width(&plane_state->uapi.src) >> 16;
> + height = drm_rect_height(&plane_state->uapi.src) >> 16;
> +
> + /* UV plane does 1/2 pixel sub-sampling */
> + if (color_plane == 1) {
> + width /= 2;
> + height /= 2;
> + }
> +
> + return width * height * fb->format->cpp[color_plane];
> +}
> +
> int intel_plane_calc_min_cdclk(struct intel_atomic_state *state,
> struct intel_plane *plane,
> bool *need_cdclk_calc)
> @@ -312,6 +339,8 @@ void intel_plane_set_invisible(struct intel_crtc_state *crtc_state,
> crtc_state->c8_planes &= ~BIT(plane->id);
> crtc_state->data_rate[plane->id] = 0;
> crtc_state->data_rate_y[plane->id] = 0;
> + crtc_state->rel_data_rate[plane->id] = 0;
> + crtc_state->rel_data_rate_y[plane->id] = 0;
> crtc_state->min_cdclk[plane->id] = 0;
>
> plane_state->uapi.visible = false;
> @@ -360,9 +389,17 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_
> 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);
> +
> + new_crtc_state->rel_data_rate_y[plane->id] =
> + intel_plane_relative_data_rate(new_plane_state, 0);
> + new_crtc_state->rel_data_rate[plane->id] =
> + intel_plane_relative_data_rate(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);
> +
> + new_crtc_state->rel_data_rate[plane->id] =
> + intel_plane_relative_data_rate(new_plane_state, 0);
> }
>
> return intel_plane_atomic_calc_changes(old_crtc_state, new_crtc_state,
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 39dd2e7383e0..8f3034713c56 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -762,6 +762,8 @@ void intel_plane_disable_noatomic(struct intel_crtc *crtc,
> fixup_plane_bitmasks(crtc_state);
> crtc_state->data_rate[plane->id] = 0;
> crtc_state->data_rate_y[plane->id] = 0;
> + crtc_state->rel_data_rate[plane->id] = 0;
> + crtc_state->rel_data_rate_y[plane->id] = 0;
> crtc_state->min_cdclk[plane->id] = 0;
>
> if (plane->id == PLANE_PRIMARY)
> @@ -5112,6 +5114,7 @@ static int icl_check_nv12_planes(struct intel_crtc_state *crtc_state)
> crtc_state->active_planes &= ~BIT(plane->id);
> crtc_state->update_planes |= BIT(plane->id);
> crtc_state->data_rate[plane->id] = 0;
> + crtc_state->rel_data_rate[plane->id] = 0;
> }
>
> plane_state->planar_slave = false;
> @@ -5158,6 +5161,8 @@ static int icl_check_nv12_planes(struct intel_crtc_state *crtc_state)
> crtc_state->update_planes |= BIT(linked->id);
> crtc_state->data_rate[linked->id] =
> crtc_state->data_rate_y[plane->id];
> + crtc_state->rel_data_rate[linked->id] =
> + crtc_state->rel_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 7e147e110059..871485af14d4 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1153,9 +1153,9 @@ struct intel_crtc_state {
> /* for planar Y */
> u32 data_rate_y[I915_MAX_PLANES];
>
> - /* FIXME unify with data_rate[] */
> - u64 plane_data_rate[I915_MAX_PLANES];
> - u64 uv_plane_data_rate[I915_MAX_PLANES];
> + /* FIXME unify with data_rate[]? */
> + u64 rel_data_rate[I915_MAX_PLANES];
> + u64 rel_data_rate_y[I915_MAX_PLANES];
>
> /* Gamma mode programmed on the pipe */
> u32 gamma_mode;
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 8a115b4c9e71..134584c77697 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4862,126 +4862,24 @@ static u8 skl_compute_dbuf_slices(struct intel_crtc *crtc, u8 active_pipes)
> }
>
> static u64
> -skl_plane_relative_data_rate(const struct intel_crtc_state *crtc_state,
> - const struct intel_plane_state *plane_state,
> - int color_plane)
> +skl_total_relative_data_rate(const struct intel_crtc_state *crtc_state)
> {
> - struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane);
> - const struct drm_framebuffer *fb = plane_state->hw.fb;
> - int width, height;
> -
> - if (!plane_state->uapi.visible)
> - return 0;
> -
> - if (plane->id == PLANE_CURSOR)
> - return 0;
> -
> - if (color_plane == 1 &&
> - !intel_format_info_is_yuv_semiplanar(fb->format, fb->modifier))
> - return 0;
> -
> - /*
> - * Src coordinates are already rotated by 270 degrees for
> - * the 90/270 degree plane rotation cases (to match the
> - * GTT mapping), hence no need to account for rotation here.
> - */
> - width = drm_rect_width(&plane_state->uapi.src) >> 16;
> - height = drm_rect_height(&plane_state->uapi.src) >> 16;
> -
> - /* UV plane does 1/2 pixel sub-sampling */
> - if (color_plane == 1) {
> - width /= 2;
> - height /= 2;
> - }
> -
> - return width * height * fb->format->cpp[color_plane];
> -}
> -
> -static u64
> -skl_get_total_relative_data_rate(struct intel_atomic_state *state,
> - struct intel_crtc *crtc)
> -{
> - struct intel_crtc_state *crtc_state =
> - intel_atomic_get_new_crtc_state(state, crtc);
> - const struct intel_plane_state *plane_state;
> - struct intel_plane *plane;
> - u64 total_data_rate = 0;
> + struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> + struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> enum plane_id plane_id;
> - int i;
> -
> - /* Calculate and cache data rate for each plane */
> - for_each_new_intel_plane_in_state(state, plane, plane_state, i) {
> - if (plane->pipe != crtc->pipe)
> - continue;
> -
> - plane_id = plane->id;
> -
> - /* packed/y */
> - crtc_state->plane_data_rate[plane_id] =
> - skl_plane_relative_data_rate(crtc_state, plane_state, 0);
> -
> - /* uv-plane */
> - crtc_state->uv_plane_data_rate[plane_id] =
> - skl_plane_relative_data_rate(crtc_state, plane_state, 1);
> - }
> + u64 data_rate = 0;
>
> for_each_plane_id_on_crtc(crtc, plane_id) {
> - total_data_rate += crtc_state->plane_data_rate[plane_id];
> - total_data_rate += crtc_state->uv_plane_data_rate[plane_id];
> - }
> -
> - return total_data_rate;
> -}
> -
> -static u64
> -icl_get_total_relative_data_rate(struct intel_atomic_state *state,
> - struct intel_crtc *crtc)
> -{
> - struct intel_crtc_state *crtc_state =
> - intel_atomic_get_new_crtc_state(state, crtc);
> - const struct intel_plane_state *plane_state;
> - struct intel_plane *plane;
> - u64 total_data_rate = 0;
> - enum plane_id plane_id;
> - int i;
> -
> - /* Calculate and cache data rate for each plane */
> - for_each_new_intel_plane_in_state(state, plane, plane_state, i) {
> - if (plane->pipe != crtc->pipe)
> + if (plane_id == PLANE_CURSOR)
> continue;
>
> - plane_id = plane->id;
> + data_rate += crtc_state->rel_data_rate[plane_id];
>
> - if (!plane_state->planar_linked_plane) {
> - crtc_state->plane_data_rate[plane_id] =
> - skl_plane_relative_data_rate(crtc_state, plane_state, 0);
> - } else {
> - enum plane_id y_plane_id;
> -
> - /*
> - * The slave plane might not iterate in
> - * intel_atomic_crtc_state_for_each_plane_state(),
> - * and needs the master plane state which may be
> - * NULL if we try get_new_plane_state(), so we
> - * always calculate from the master.
> - */
> - if (plane_state->planar_slave)
> - continue;
> -
> - /* Y plane rate is calculated on the slave */
> - y_plane_id = plane_state->planar_linked_plane->id;
> - crtc_state->plane_data_rate[y_plane_id] =
> - skl_plane_relative_data_rate(crtc_state, plane_state, 0);
> -
> - crtc_state->plane_data_rate[plane_id] =
> - skl_plane_relative_data_rate(crtc_state, plane_state, 1);
> - }
> + if (DISPLAY_VER(i915) < 11)
> + data_rate += crtc_state->rel_data_rate_y[plane_id];
> }
>
> - for_each_plane_id_on_crtc(crtc, plane_id)
> - total_data_rate += crtc_state->plane_data_rate[plane_id];
> -
> - return total_data_rate;
> + return data_rate;
> }
>
> const struct skl_wm_level *
> @@ -5096,11 +4994,6 @@ skl_crtc_allocate_plane_ddb(struct intel_atomic_state *state,
> if (!crtc_state->hw.active)
> return 0;
>
> - if (DISPLAY_VER(dev_priv) >= 11)
> - iter.data_rate = icl_get_total_relative_data_rate(state, crtc);
> - else
> - iter.data_rate = skl_get_total_relative_data_rate(state, crtc);
> -
> iter.size = skl_ddb_entry_size(alloc);
> if (iter.size == 0)
> return 0;
> @@ -5111,6 +5004,7 @@ skl_crtc_allocate_plane_ddb(struct intel_atomic_state *state,
> skl_ddb_entry_init(&crtc_state->wm.skl.plane_ddb[PLANE_CURSOR],
> alloc->end - iter.total[PLANE_CURSOR], alloc->end);
>
> + iter.data_rate = skl_total_relative_data_rate(crtc_state);
> if (iter.data_rate == 0)
> return 0;
>
> @@ -5171,16 +5065,19 @@ skl_crtc_allocate_plane_ddb(struct intel_atomic_state *state,
> if (iter.data_rate == 0)
> break;
>
> - iter.total[plane_id] =
> - skl_allocate_plane_ddb(&iter, &wm->wm[level],
> - crtc_state->plane_data_rate[plane_id]);
> -
> - if (iter.data_rate == 0)
> - break;
> -
> - iter.uv_total[plane_id] =
> - skl_allocate_plane_ddb(&iter, &wm->uv_wm[level],
> - crtc_state->uv_plane_data_rate[plane_id]);
> + if (DISPLAY_VER(dev_priv) < 11 &&
> + crtc_state->nv12_planes & BIT(plane_id)) {
> + iter.total[plane_id] =
> + skl_allocate_plane_ddb(&iter, &wm->wm[level],
> + crtc_state->rel_data_rate_y[plane_id]);
> + iter.uv_total[plane_id] =
> + skl_allocate_plane_ddb(&iter, &wm->uv_wm[level],
> + crtc_state->rel_data_rate[plane_id]);
> + } else {
> + iter.total[plane_id] =
> + skl_allocate_plane_ddb(&iter, &wm->wm[level],
> + crtc_state->rel_data_rate[plane_id]);
> + }
> }
> drm_WARN_ON(&dev_priv->drm, iter.size != 0 || iter.data_rate != 0);
>
> @@ -5200,15 +5097,18 @@ skl_crtc_allocate_plane_ddb(struct intel_atomic_state *state,
> DISPLAY_VER(dev_priv) >= 11 && iter.uv_total[plane_id]);
>
> /* Leave disabled planes at (0,0) */
> - if (iter.total[plane_id])
> - iter.start = skl_ddb_entry_init(ddb, iter.start,
> - iter.start + iter.total[plane_id]);
> -
> - if (iter.uv_total[plane_id]) {
> - /* hardware wants these swapped */
> - *ddb_y = *ddb;
> - iter.start = skl_ddb_entry_init(ddb, iter.start,
> - iter.start + iter.uv_total[plane_id]);
> + if (DISPLAY_VER(dev_priv) < 11 &&
> + crtc_state->nv12_planes & BIT(plane_id)) {
> + if (iter.total[plane_id])
> + iter.start = skl_ddb_entry_init(ddb_y, iter.start,
> + iter.start + iter.total[plane_id]);
> + if (iter.uv_total[plane_id])
> + iter.start = skl_ddb_entry_init(ddb, iter.start,
> + iter.start + iter.uv_total[plane_id]);
> + } else {
> + if (iter.total[plane_id])
> + iter.start = skl_ddb_entry_init(ddb, iter.start,
> + iter.start + iter.total[plane_id]);
> }
> }
>
> --
> 2.32.0
>
next prev parent reply other threads:[~2022-02-01 8:11 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
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 [this message]
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=20220201081114.GC9466@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.