On Tue, Jan 18, 2022 at 12:48:39PM +0200, Stanislav Lisovskiy wrote:
> In terms of async flip optimization we don't to allocate
> extra ddb space, so lets skip it.
>
> v2: - Extracted min ddb async flip check to separate function
> (Ville Syrjälä)
> - Used this function to prevent false positive WARN
> to be triggered(Ville Syrjälä)
>
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovs...@intel.com>
> ---
> drivers/gpu/drm/i915/intel_pm.c | 34 ++++++++++++++++++++++++++-------
> 1 file changed, 27 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 5d350ddc447f..4922c9108f08 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5118,6 +5118,12 @@ static bool icl_need_wm1_wa(struct drm_i915_private
> *i915,
> (IS_DISPLAY_VER(i915, 12, 13) && plane_id == PLANE_CURSOR);
> }
>
> +static bool dg2_need_min_ddb(struct drm_i915_private *i915,
> + struct intel_crtc_state *crtc_state)
> +{
> + return IS_DG2(i915) && crtc_state->uapi.async_flip;
Why are we using IS_DG2 here vs. DISPLAY>=13 for the wm0 only decision?
> +}
> +
> static int
> skl_allocate_plane_ddb(struct intel_atomic_state *state,
> struct intel_crtc *crtc)
> @@ -5226,9 +5232,15 @@ skl_allocate_plane_ddb(struct intel_atomic_state
> *state,
> break;
>
> rate = crtc_state->plane_data_rate[plane_id];
> - extra = min_t(u16, alloc_size,
> - DIV64_U64_ROUND_UP(alloc_size * rate,
> - total_data_rate));
> +
> + if (dg2_need_min_ddb(dev_priv, crtc_state)) {
> + extra = 0;
> + } else {
> + extra = min_t(u16, alloc_size,
> + DIV64_U64_ROUND_UP(alloc_size * rate,
> + total_data_rate));
> + }
Hmm. I wonder if we should just set rate=0 instead. That would
let the other planes pick up the now unused extra ddb space. Would
also avoid having to skip the WARN since we'd still allocate the
full ddb.
> +
> total[plane_id] = wm->wm[level].min_ddb_alloc + extra;
> alloc_size -= extra;
> total_data_rate -= rate;
> @@ -5237,14 +5249,22 @@ skl_allocate_plane_ddb(struct intel_atomic_state
> *state,
> break;
>
> rate = crtc_state->uv_plane_data_rate[plane_id];
> - extra = min_t(u16, alloc_size,
> - DIV64_U64_ROUND_UP(alloc_size * rate,
> - total_data_rate));
> +
> + if (dg2_need_min_ddb(dev_priv, crtc_state)) {
> + extra = 0;
> + } else {
> + extra = min_t(u16, alloc_size,
> + DIV64_U64_ROUND_UP(alloc_size * rate,
> + total_data_rate));
> + }
> +
> uv_total[plane_id] = wm->uv_wm[level].min_ddb_alloc + extra;
> alloc_size -= extra;
> total_data_rate -= rate;
> }
> - drm_WARN_ON(&dev_priv->drm, alloc_size != 0 || total_data_rate != 0);
> +
> + if (!dg2_need_min_ddb(dev_priv, crtc_state))
> + drm_WARN_ON(&dev_priv->drm, alloc_size != 0 || total_data_rate
> != 0);
>
> /* Set the actual DDB start/end points for each plane */
> start = alloc->start;
> --
> 2.24.1.485.gad05a3d8e5
--
Ville Syrjälä
Intel
