On Thu, Apr 07, 2022 at 11:42:35AM +0300, Stanislav Lisovskiy wrote: > We had some FIFO underruns appearing on platforms like ADL, > which could be fixed though by increasing CDCLK, however we were > lacking explanation for that - we were not calculating CDCLK, > also based on cumulative bpp W/A formula, mentioned in BSpec 64631.
We already have that in intel_bw_crtc_min_cdclk(). > > With that patch no FIFO underruns appear anymore. > > Signed-off-by: Stanislav Lisovskiy <[email protected]> > --- > drivers/gpu/drm/i915/display/intel_bw.c | 71 ++++++++++++++++++++++--- > drivers/gpu/drm/i915/display/intel_bw.h | 2 + > 2 files changed, 67 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_bw.c > b/drivers/gpu/drm/i915/display/intel_bw.c > index 37bd7b17f3d0..3a2aeeffee7c 100644 > --- a/drivers/gpu/drm/i915/display/intel_bw.c > +++ b/drivers/gpu/drm/i915/display/intel_bw.c > @@ -743,20 +743,51 @@ static void skl_plane_calc_dbuf_bw(struct > intel_bw_state *bw_state, > } > } > > -static void skl_crtc_calc_dbuf_bw(struct intel_bw_state *bw_state, > +static int intel_plane_bw_bpp(const struct drm_format_info *info) > +{ > + /* > + * For the purposes of memory bandwidth calculations, > + * planar formats are treated as if both planes had the > + * same bpp (with no reduction for vertical > + * subsampling). I.e we take as usual the worst case > + * scenario. > + */ > + if (drm_format_info_is_yuv_semiplanar(info)) > + return 2 * max(info->cpp[0], info->cpp[1]); > + > + return info->cpp[0]; > +} > + > +static void skl_crtc_calc_dbuf_bw(struct intel_atomic_state *state, > + struct intel_bw_state *bw_state, > 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); > struct intel_dbuf_bw *crtc_bw = &bw_state->dbuf_bw[crtc->pipe]; > - enum plane_id plane_id; > + struct intel_plane *plane; > > memset(crtc_bw, 0, sizeof(*crtc_bw)); > > if (!crtc_state->hw.active) > return; > > - for_each_plane_id_on_crtc(crtc, plane_id) { > + for_each_intel_plane_on_crtc(&i915->drm, crtc, plane) { > + const struct drm_framebuffer *fb; > + enum plane_id plane_id = plane->id; > + unsigned int plane_bpp = 0; > + struct intel_plane_state *plane_state = > + intel_atomic_get_new_plane_state(state, plane); > + > + if (plane_state) { > + fb = plane_state->hw.fb; > + > + if (plane_state->uapi.visible && fb) > + plane_bpp = intel_plane_bw_bpp(fb->format); > + } > + > + crtc_bw->pipe_cumulative_bpp += plane_bpp; > + > /* > * We assume cursors are small enough > * to not cause bandwidth problems. > @@ -773,6 +804,10 @@ static void skl_crtc_calc_dbuf_bw(struct intel_bw_state > *bw_state, > > &crtc_state->wm.skl.plane_ddb_y[plane_id], > crtc_state->data_rate[plane_id]); > } > + > + crtc_bw->bpp_cdclk = > DIV_ROUND_UP_ULL(mul_u32_u32(crtc_state->pixel_rate, > + crtc_bw->pipe_cumulative_bpp * > 512), > + 10) / 1000; > } > > /* "Maximum Data Buffer Bandwidth" */ > @@ -782,11 +817,13 @@ intel_bw_dbuf_min_cdclk(struct drm_i915_private *i915, > { > unsigned int total_max_bw = 0; > enum dbuf_slice slice; > + enum pipe pipe; > + unsigned int bpp_cdclk = 0; > + unsigned int dbuf_bw_cdclk; > > for_each_dbuf_slice(i915, slice) { > int num_active_planes = 0; > unsigned int max_bw = 0; > - enum pipe pipe; > > /* > * The arbiter can only really guarantee an > @@ -803,7 +840,29 @@ intel_bw_dbuf_min_cdclk(struct drm_i915_private *i915, > total_max_bw = max(total_max_bw, max_bw); > } > > - return DIV_ROUND_UP(total_max_bw, 64); > + for_each_pipe(i915, pipe) { > + const struct intel_dbuf_bw *crtc_bw = &bw_state->dbuf_bw[pipe]; > + /* > + * From BSpec 64631: > + * Pipe cumulative bytes should be less or equal to > + * CDCLK / (pixel_rate * scaling_factors * 51.2) thus > + * CDCLK = pipe_cumulative_bpp * pixel_rate * scaling_factors * > 51.2. > + * Considering that intel_plane_pixel_rate already returns > adjusted pixel rate, > + * no scaling factors needed here. > + */ > + bpp_cdclk = max_t(unsigned int, crtc_bw->bpp_cdclk, > + bpp_cdclk); > + } > + > + dbuf_bw_cdclk = DIV_ROUND_UP(total_max_bw, 64); > + > + /* > + * So now we have two CDCLK estimations: > + * one is based on required DBuf BW and another is > + * based on pipe cumulative bpp W/A(BSpec 64631) > + * Traditionally take the more demanding into use(worst case) > + */ > + return max_t(unsigned int, dbuf_bw_cdclk, bpp_cdclk); > } > > int intel_bw_min_cdclk(struct drm_i915_private *i915, > @@ -842,7 +901,7 @@ int intel_bw_calc_min_cdclk(struct intel_atomic_state > *state, > > old_bw_state = intel_atomic_get_old_bw_state(state); > > - skl_crtc_calc_dbuf_bw(new_bw_state, crtc_state); > + skl_crtc_calc_dbuf_bw(state, new_bw_state, crtc_state); > > new_bw_state->min_cdclk[crtc->pipe] = > intel_bw_crtc_min_cdclk(crtc_state); > diff --git a/drivers/gpu/drm/i915/display/intel_bw.h > b/drivers/gpu/drm/i915/display/intel_bw.h > index cb7ee3a24a58..9e3a6ad03b19 100644 > --- a/drivers/gpu/drm/i915/display/intel_bw.h > +++ b/drivers/gpu/drm/i915/display/intel_bw.h > @@ -19,6 +19,8 @@ struct intel_crtc_state; > struct intel_dbuf_bw { > unsigned int max_bw[I915_MAX_DBUF_SLICES]; > u8 active_planes[I915_MAX_DBUF_SLICES]; > + unsigned int pipe_cumulative_bpp; > + unsigned int bpp_cdclk; > }; > > struct intel_bw_state { > -- > 2.24.1.485.gad05a3d8e5 -- Ville Syrjälä Intel
