> -----Original Message----- > From: Intel-xe <[email protected]> On Behalf Of Ville > Syrjala > Sent: Wednesday, October 8, 2025 11:56 PM > To: [email protected] > Cc: [email protected] > Subject: [RFC][PATCH 02/11] drm/i915/cdclk: Add prefill helpers for CDCLK > > From: Ville Syrjälä <[email protected]> > > Add helpers to compute the CDCLKl adjustment factor for prefill calculations. > The > adjustment factor is always <= 1.0. That is, a faster that strictly necessary > CDCLK speeds up the pipe prefill.
Nit: Typo in CDCLK and last line can be clarified better. Overall changes look good to me. Reviewed-by: Uma Shankar <[email protected]> > intel_cdclk_prefill_adjustment_worst() gives out a worst case estimate, meant > to > be used during guardband sizing. We can actually do better than 1.0 here > because > the absolute minimum CDCLK is limited by the dotclock. This will still allow > planes, pfit, etc. to be changed any which way without having to resize the > guardband yet again. > > intel_cdclk_prefill_adjustment() gives out a potentially more optimal value, > purely > based on the final minimum CDCLK (also considering > planes/pfit/etc.) for the current pipe. We can't actually check against the > current > CDCLK frequency as that might be much higher due to some other pipe, and said > other pipe might later reduce the CDCLK below what the current pipe would find > acceptable (given which WM levels are enabled). Ie. we don't consider any > global > constraints (other pipes, dbuf bandwidth, etc) on the mimimum CDCLK frequency > here. > > The returned numbers are in .16 binary fixed point. > > TODO: the intel_cdclk_prefill_adjustment_worst() approach here > can result in guardband changes for DRRS. But I'm thinking > that is fine since M/N changes will always happen on the > legacy timing generator so guardband doesn't actually matter. > May need to think about this a bit more though... > Signed-off-by: Ville Syrjälä <[email protected]> > --- > drivers/gpu/drm/i915/display/intel_cdclk.c | 87 +++++++++++++++++++++- > drivers/gpu/drm/i915/display/intel_cdclk.h | 4 + > 2 files changed, 89 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c > b/drivers/gpu/drm/i915/display/intel_cdclk.c > index b54b1006aeb0..598593eafdf5 100644 > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c > @@ -2791,16 +2791,20 @@ static int intel_cdclk_guardband(struct intel_display > *display) > return 90; > } > > -static int intel_pixel_rate_to_cdclk(const struct intel_crtc_state > *crtc_state) > +static int _intel_pixel_rate_to_cdclk(const struct intel_crtc_state > +*crtc_state, int pixel_rate) > { > struct intel_display *display = to_intel_display(crtc_state); > int ppc = intel_cdclk_ppc(display, crtc_state->double_wide); > int guardband = intel_cdclk_guardband(display); > - int pixel_rate = crtc_state->pixel_rate; > > return DIV_ROUND_UP(pixel_rate * 100, guardband * ppc); } > > +static int intel_pixel_rate_to_cdclk(const struct intel_crtc_state > +*crtc_state) { > + return _intel_pixel_rate_to_cdclk(crtc_state, crtc_state->pixel_rate); > +} > + > static int intel_planes_min_cdclk(const struct intel_crtc_state *crtc_state) > { > struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > @@ -3917,3 +3921,82 @@ void intel_cdclk_read_hw(struct intel_display > *display) > cdclk_state->actual = display->cdclk.hw; > cdclk_state->logical = display->cdclk.hw; } > + > +static int calc_cdclk(const struct intel_crtc_state *crtc_state, int > +min_cdclk) { > + struct intel_display *display = to_intel_display(crtc_state); > + > + if (DISPLAY_VER(display) >= 10 || display->platform.broxton) { > + return bxt_calc_cdclk(display, min_cdclk); > + } else if (DISPLAY_VER(display) == 9) { > + int vco; > + > + vco = display->cdclk.skl_preferred_vco_freq; > + if (vco == 0) > + vco = 8100000; > + > + return skl_calc_cdclk(min_cdclk, vco); > + } else if (display->platform.broadwell) { > + return bdw_calc_cdclk(min_cdclk); > + } else if (display->platform.cherryview || > display->platform.valleyview) { > + return vlv_calc_cdclk(display, min_cdclk); > + } else { > + return display->cdclk.max_cdclk_freq; > + } > +} > + > +static unsigned int _intel_cdclk_prefill_adj(const struct intel_crtc_state > *crtc_state, > + int clock, int min_cdclk) > +{ > + struct intel_display *display = to_intel_display(crtc_state); > + int ppc = intel_cdclk_ppc(display, crtc_state->double_wide); > + int cdclk = calc_cdclk(crtc_state, min_cdclk); > + > + return min(0x10000, DIV_ROUND_UP_ULL((u64)clock << 16, ppc * > cdclk)); > +} > + > +unsigned int intel_cdclk_prefill_adjustment(const struct intel_crtc_state > *crtc_state, > + const struct intel_cdclk_state > *cdclk_state) { > + struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > + int clock = crtc_state->hw.pipe_mode.crtc_clock; > + int min_cdclk; > + > + /* > + * Only consider the current pipe's minimum cdclk here as a safe > + * lower bound. This must *not* be based on the actual/logical cdclk > + * frequency here as that may get reduced later due to eg. a modeset > + * on a different pipe, and that would completely invalidate the > + * guardband length checks we did on this pipe previously. That > + * could lead to prefill exceeding the guardband which would result > + * in underruns. > + */ > + min_cdclk = intel_cdclk_min_cdclk(cdclk_state, crtc->pipe); > + > + return _intel_cdclk_prefill_adj(crtc_state, clock, min_cdclk); } > + > +unsigned int intel_cdclk_prefill_adjustment_worst(const struct > +intel_crtc_state *crtc_state) { > + int clock = crtc_state->hw.pipe_mode.crtc_clock; > + int min_cdclk; > + > + /* > + * FIXME could perhaps consider a few more of the factors > + * that go into cdclk_state->min_cdclk[] here. Namely anything > + * that only changes during full modesets. > + * > + * Should perhaps just cache those into a crtc_state->min_cdclk > + * at .compute_config() time. Then we could also skip recomputing > + * those parts during intel_cdclk_atomic_check(). > + * > + * FIXME this assumes 1:1 scaling, but the other _worst() stuff > + * assumes max downscaling, so the final result will be > + * unrealistically bad. Figure out where the actual maximum value > + * lies and use that to compute a more realistic worst case > + * estimate... > + */ > + min_cdclk = _intel_pixel_rate_to_cdclk(crtc_state, clock); > + > + return _intel_cdclk_prefill_adj(crtc_state, clock, min_cdclk); } > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h > b/drivers/gpu/drm/i915/display/intel_cdclk.h > index cacee598af0e..d97f725a0160 100644 > --- a/drivers/gpu/drm/i915/display/intel_cdclk.h > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.h > @@ -69,4 +69,8 @@ bool intel_cdclk_pmdemand_needs_update(struct > intel_atomic_state *state); void intel_cdclk_force_min_cdclk(struct > intel_cdclk_state *cdclk_state, int force_min_cdclk); void > intel_cdclk_read_hw(struct intel_display *display); > > +unsigned int intel_cdclk_prefill_adjustment(const struct intel_crtc_state > *crtc_state, > + const struct intel_cdclk_state > *cdclk_state); unsigned int > +intel_cdclk_prefill_adjustment_worst(const struct intel_crtc_state > +*crtc_state); > + > #endif /* __INTEL_CDCLK_H__ */ > -- > 2.49.1
