> -----Original Message----- > From: Kandpal, Suraj <[email protected]> > Sent: Wednesday, 11 March 2026 8.00 > To: Kandpal, Suraj <[email protected]>; Kahola, Mika > <[email protected]>; [email protected]; intel- > [email protected] > Cc: Kahola, Mika <[email protected]> > Subject: RE: [PATCH v2 18/24] drm/i915/lt_phy: Add .disable_clock hook on DDI > > > Subject: RE: [PATCH v2 18/24] drm/i915/lt_phy: Add .disable_clock hook > > on DDI > > > > > Subject: [PATCH v2 18/24] drm/i915/lt_phy: Add .disable_clock hook > > > on DDI > > > > > > Disable PLL clock on DDI by moving part of the PLL disabling > > > sequence into a DDI clock disabling function. > > > > > > > Commit message needs to be something like "Add new pll_disable_clock > > functions so that they can be hooked up to dpll->disable. > > This is just a wrapper over the exitisting intel_xe3plpd_pll_disable > > to make it compatible With dpll->disable function" > > > > > > > Signed-off-by: Mika Kahola <[email protected]> > > > --- > > > drivers/gpu/drm/i915/display/intel_ddi.c | 2 +- > > > drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 12 ++++++++++++ > > > drivers/gpu/drm/i915/display/intel_lt_phy.c | 11 +++++++++++ > > > drivers/gpu/drm/i915/display/intel_lt_phy.h | 1 + > > > 4 files changed, 25 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c > > > b/drivers/gpu/drm/i915/display/intel_ddi.c > > > index 51403d09c477..191ae7cf81fb 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > > > @@ -5299,7 +5299,7 @@ void intel_ddi_init(struct intel_display > > > *display, > > > > > > if (HAS_LT_PHY(display)) { > > > encoder->enable_clock = intel_xe3plpd_pll_enable_clock; > > > - encoder->disable_clock = intel_xe3plpd_pll_disable; > > > + encoder->disable_clock = intel_xe3plpd_pll_disable_clock; > > > encoder->port_pll_type = intel_mtl_port_pll_type; > > > encoder->get_config = xe3plpd_ddi_get_config; > > > } else if (DISPLAY_VER(display) >= 14) { diff --git > > > a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c > > > b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c > > > index 54c7a255b3a5..28c560417409 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c > > > +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c > > > @@ -4607,8 +4607,20 @@ static void xe3plpd_pll_enable(struct > > > intel_display *display, > > > intel_xe3plpd_pll_enable(encoder, pll, dpll_hw_state); } > > > > > > +static void xe3plpd_pll_disable(struct intel_display *display, > > > + struct intel_dpll *pll) > > > +{ > > > + struct intel_encoder *encoder = get_intel_encoder(display, pll); > > > + > > > + if (drm_WARN_ON(display->drm, !encoder)) > > > + return; > > > + > > > + intel_xe3plpd_pll_disable(encoder); > > > +} > > > + > > > static const struct intel_dpll_funcs xe3plpd_pll_funcs = { > > > .enable = xe3plpd_pll_enable, > > > + .disable = xe3plpd_pll_disable, > > > .get_hw_state = xe3plpd_pll_get_hw_state, > > > .get_freq = xe3plpd_pll_get_freq, > > > }; > > > diff --git a/drivers/gpu/drm/i915/display/intel_lt_phy.c > > > b/drivers/gpu/drm/i915/display/intel_lt_phy.c > > > index 6bc32d1734a7..3230d2e28d9c 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_lt_phy.c > > > +++ b/drivers/gpu/drm/i915/display/intel_lt_phy.c > > > @@ -2309,6 +2309,17 @@ void intel_xe3plpd_pll_disable(struct > > > intel_encoder *encoder) > > > intel_mtl_tbt_pll_disable_clock(encoder); > > > else > > > intel_lt_phy_pll_disable(encoder); > > > +} > > > + > > > +void intel_xe3plpd_pll_disable_clock(struct intel_encoder *encoder) { > > > + struct intel_digital_port *dig_port = enc_to_dig_port(encoder); > > > + > > > + if (intel_tc_port_in_tbt_alt_mode(dig_port)) > > > + intel_mtl_tbt_pll_disable_clock(encoder); > > > > This is already called inside intel_mtl_tbt_pll_disable clock. > > Is there any specific reason to add a wrapper around this other than > > naming if not You can drop this wrapper and proceed without the below change > > - encoder->disable_clock = intel_xe3plpd_pll_disable; > > + encoder->disable_clock = intel_xe3plpd_pll_disable_clock;
This was just a naming to have TBT mode as a separate as it's PLL handling is different. I can drop this for the next revision. > > > > Regards, > > Suraj Kandpal > > > > > + else > > > + /* TODO: remove when PLL mgr is in place. */ > > > + intel_xe3plpd_pll_disable(encoder); > > > > > > } > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_lt_phy.h > > > b/drivers/gpu/drm/i915/display/intel_lt_phy.h > > > index 9188ce980119..3838e9326773 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_lt_phy.h > > > +++ b/drivers/gpu/drm/i915/display/intel_lt_phy.h > > > @@ -49,5 +49,6 @@ void intel_xe3plpd_pll_disable(struct > > > intel_encoder *encoder); void intel_lt_phy_verify_plls(struct > > > intel_display *display); void intel_xe3plpd_pll_enable_clock(struct > > > intel_encoder > > *encoder, > > > const struct intel_crtc_state *crtc_state); > > > +void intel_xe3plpd_pll_disable_clock(struct intel_encoder > > > +*encoder); > > Also rearrange in ASCIIBETICAL order Yes! Thanks for the review! -Mika- > > Regards, > Suraj Kandpal > > > > > > > #endif /* __INTEL_LT_PHY_H__ */ > > > -- > > > 2.43.0
