> 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; > > 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 Regards, Suraj Kandpal > > > > #endif /* __INTEL_LT_PHY_H__ */ > > -- > > 2.43.0
