On Tue, 20 Jan 2026, Jani Nikula <[email protected]> wrote: > On Fri, 16 Jan 2026, Suraj Kandpal <[email protected]> wrote: >> From: Mika Kahola <[email protected]> >> >> Move intel_pps_on() to intel_dpll_mgr PLL enabling >> .enable function hook to enable panel power earlier. >> We need to do this to make sure we are following the >> modeset sequences of Bspec. This had changed when we >> moved the PLL PHY enablement for CX0 from .enable_clock >> to dpll.enable hook > > So I really hate this. > > Yeah, maybe it follows the spec now, but what connection does the DPLL > manager have with the panel power sequencing? > > Absolutely nothing. > > The DPLL manager has no business calling PPS functions. > > Currently only the g4x and DDI encoder code does PPS power calls, and > they're the only ones who should manage PPS. > >> >> Signed-off-by: Mika Kahola <[email protected]> >> Signed-off-by: Suraj Kandpal <[email protected]> >> --- >> >> v2 -> v3: >> - Rather than splitting the PHY enablement sequence, enable PPS >> earlier (Imre) > > Please point me at the review comment. I couldn't find anything that > would suggest moving the PPS calls to the DPLL manager. > > Please let's not do this.
Cc: Imre > > BR, > Jani. > > >> >> drivers/gpu/drm/i915/display/intel_ddi.c | 6 ++++-- >> drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 5 +++++ >> 2 files changed, 9 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c >> b/drivers/gpu/drm/i915/display/intel_ddi.c >> index cb91d07cdaa6..1784fa687c03 100644 >> --- a/drivers/gpu/drm/i915/display/intel_ddi.c >> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c >> @@ -2653,8 +2653,10 @@ static void mtl_ddi_pre_enable_dp(struct >> intel_atomic_state *state, >> /* 3. Select Thunderbolt */ >> mtl_port_buf_ctl_io_selection(encoder); >> >> - /* 4. Enable Panel Power if PPS is required */ >> - intel_pps_on(intel_dp); >> + /* >> + * 4. Enable Panel Power if PPS is required >> + * moved to intel_dpll_mgr .enable hook >> + */ >> >> /* 5. Enable the port PLL */ >> intel_ddi_enable_clock(encoder, crtc_state); >> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c >> b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c >> index 9aa84a430f09..b5655c734c53 100644 >> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c >> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c >> @@ -40,6 +40,7 @@ >> #include "intel_hti.h" >> #include "intel_mg_phy_regs.h" >> #include "intel_pch_refclk.h" >> +#include "intel_pps.h" >> #include "intel_step.h" >> #include "intel_tc.h" >> >> @@ -4401,6 +4402,10 @@ static void mtl_pll_enable(struct intel_display >> *display, >> if (drm_WARN_ON(display->drm, !encoder)) >> return; >> >> + /* Enable Panel Power if PPS is required */ >> + if (intel_encoder_is_dp(encoder)) >> + intel_pps_on(enc_to_intel_dp(encoder)); >> + >> intel_mtl_pll_enable(encoder, pll, dpll_hw_state); >> } -- Jani Nikula, Intel
