On Wed, 10 Jun 2026, Ville Syrjala <[email protected]> wrote: > From: Ville Syrjälä <[email protected]> > > Now that intel_cdclk_{pre,post}_notify() are implemented via vfuncs > there is no need to keep them inside the .set_cdclk() hooks. Move > the calls one level up to intel_cdclk_set_cdclk(). > > We do need to adjust {skl,bxt}_cdclk_(un)init_hw() to call the wrapper > rather than the low level implementation directly, or else they would > not do the pcode notification anymore. > > The two slight functions changes here are: > - bdw_set_cdclk() might theoretically bail out after doing the > pre notification, but that codepath would only come into play > if the hardware is seriously misprogrammed, so should never happen > - cdclk hw readout is still done from .set_cdclk(), so that now > happens before the post notify vs. previously the readout happened > before it. This should not matter as the readout is not affected > by the post notify (since we can't actually read out anything from > pcode).
Fingers crossed. Reviewed-by: Jani Nikula <[email protected]> > > Signed-off-by: Ville Syrjälä <[email protected]> > --- > drivers/gpu/drm/i915/display/intel_cdclk.c | 33 ++++++---------------- > 1 file changed, 9 insertions(+), 24 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c > b/drivers/gpu/drm/i915/display/intel_cdclk.c > index 4154b4888eff..617ad154505c 100644 > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c > @@ -209,7 +209,12 @@ static void intel_cdclk_set_cdclk(struct intel_display > *display, > const struct intel_cdclk_config *cdclk_config, > enum pipe pipe) > { > + if (intel_cdclk_pre_notify(display)) > + return; > + > display->cdclk.funcs->set_cdclk(display, cdclk_config, pipe); > + > + intel_cdclk_post_notify(display, cdclk_config); > } > > static int intel_cdclk_modeset_calc_cdclk(struct intel_atomic_state *state) > @@ -933,10 +938,6 @@ static void bdw_set_cdclk(struct intel_display *display, > "trying to change cdclk frequency with cdclk not > enabled\n")) > return; > > - ret = intel_cdclk_pre_notify(display); > - if (ret) > - return; > - > intel_de_rmw(display, LCPLL_CTL, > 0, LCPLL_CD_SOURCE_FCLK); > > @@ -960,8 +961,6 @@ static void bdw_set_cdclk(struct intel_display *display, > if (ret) > drm_err(display->drm, "Switching back to LCPLL failed\n"); > > - intel_cdclk_post_notify(display, cdclk_config); > - > intel_de_write(display, CDCLK_FREQ, > DIV_ROUND_CLOSEST(cdclk, 1000) - 1); > > @@ -1218,7 +1217,6 @@ static void skl_set_cdclk(struct intel_display *display, > int cdclk = cdclk_config->cdclk; > int vco = cdclk_config->vco; > u32 freq_select, cdclk_ctl; > - int ret; > > /* > * Based on WA#1183 CDCLK rates 308 and 617MHz CDCLK rates are > @@ -1231,10 +1229,6 @@ static void skl_set_cdclk(struct intel_display > *display, > drm_WARN_ON_ONCE(display->drm, > display->platform.skylake && vco == 8640000); > > - ret = intel_cdclk_pre_notify(display); > - if (ret) > - return; > - > freq_select = skl_cdclk_freq_sel(display, cdclk, vco); > > if (display->cdclk.hw.vco != 0 && > @@ -1270,8 +1264,6 @@ static void skl_set_cdclk(struct intel_display *display, > intel_de_write(display, CDCLK_CTL, cdclk_ctl); > intel_de_posting_read(display, CDCLK_CTL); > > - intel_cdclk_post_notify(display, cdclk_config); > - > intel_update_cdclk(display); > } > > @@ -1343,7 +1335,7 @@ static void skl_cdclk_init_hw(struct intel_display > *display) > cdclk_config.cdclk = skl_calc_cdclk(0, cdclk_config.vco); > cdclk_config.voltage_level = skl_calc_voltage_level(cdclk_config.cdclk); > > - skl_set_cdclk(display, &cdclk_config, INVALID_PIPE); > + intel_cdclk_set_cdclk(display, &cdclk_config, INVALID_PIPE); > } > > static void skl_cdclk_uninit_hw(struct intel_display *display) > @@ -1354,7 +1346,7 @@ static void skl_cdclk_uninit_hw(struct intel_display > *display) > cdclk_config.vco = 0; > cdclk_config.voltage_level = skl_calc_voltage_level(cdclk_config.cdclk); > > - skl_set_cdclk(display, &cdclk_config, INVALID_PIPE); > + intel_cdclk_set_cdclk(display, &cdclk_config, INVALID_PIPE); > } > > struct intel_cdclk_vals { > @@ -2308,11 +2300,6 @@ static void bxt_set_cdclk(struct intel_display > *display, > { > struct intel_cdclk_config mid_cdclk_config; > int cdclk = cdclk_config->cdclk; > - int ret; > - > - ret = intel_cdclk_pre_notify(display); > - if (ret) > - return; > > if (DISPLAY_VER(display) >= 20 && cdclk < display->cdclk.hw.cdclk) > xe2lpd_mdclk_cdclk_ratio_program(display, cdclk_config); > @@ -2328,8 +2315,6 @@ static void bxt_set_cdclk(struct intel_display *display, > if (DISPLAY_VER(display) >= 20 && cdclk > display->cdclk.hw.cdclk) > xe2lpd_mdclk_cdclk_ratio_program(display, cdclk_config); > > - intel_cdclk_post_notify(display, cdclk_config); > - > intel_update_cdclk(display); > > /* > @@ -2413,7 +2398,7 @@ static void bxt_cdclk_init_hw(struct intel_display > *display) > cdclk_config.voltage_level = > intel_cdclk_calc_voltage_level(display, cdclk_config.cdclk); > > - bxt_set_cdclk(display, &cdclk_config, INVALID_PIPE); > + intel_cdclk_set_cdclk(display, &cdclk_config, INVALID_PIPE); > } > > static void bxt_cdclk_uninit_hw(struct intel_display *display) > @@ -2425,7 +2410,7 @@ static void bxt_cdclk_uninit_hw(struct intel_display > *display) > cdclk_config.voltage_level = > intel_cdclk_calc_voltage_level(display, cdclk_config.cdclk); > > - bxt_set_cdclk(display, &cdclk_config, INVALID_PIPE); > + intel_cdclk_set_cdclk(display, &cdclk_config, INVALID_PIPE); > } > > /** -- Jani Nikula, Intel
