On Tue, Jan 18, 2022 at 11:23:48AM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> 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@xxxxxxxxx>
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> .../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
>
