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.lisovskiy@xxxxxxxxx> > --- > 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