Quoting Suraj Kandpal (2025-12-30 05:31:40-03:00) >From: Mika Kahola <[email protected]> > >Split PLL enabling/disabling in two parts - one for pll setting >pll dividers and second one to enable/disable pll clock. PLL >clock enabling/disbling happens via encoder->enable_clock/disable_clock >function hook. The reason for doing this is that we need to make sure >the clock enablement happens after PPS ON step to be inline with the >sequences which we end up violating otherwise. As a result of this >violation we end up in a hanged state if machine stays idle for more >that 15 mins.
So, it appears this started happening when we Cx0 code was integrated into the DPLL framework and then the driver started enabling the PHY PLL/clock too early, right? I am lacking some context/background here due to my unfamiliarity with pre-MTL platforms, but why I exactly do we program the PLLs before the modeset sequence? Is it related to the shared nature of PLLs for platforms pre-C10/pre-C20? If so, do we really need to do the same for C10/C20 PHYs, since we have dedicated PLLs for them? (Sorry for asking here and a bit too late. Probably the better place to ask this was in series that integrated Cx0 into the DPLL framework.) > >PLL state verification happens now earlier than the clock is enabled >which causes a drm warn to be thrown. Silence this warning by >allowing this check for only earlier platforms than MeteorLake. > >Bspec: 49190 This Bspec page is not invalid for platforms using C10/C20 PHYs. We probably want to use these instead: Bspec: 65448, 68849 -- Gustavo Sousa >Signed-off-by: Mika Kahola <[email protected]> >Signed-off-by: Suraj Kandpal <[email protected]> >--- > drivers/gpu/drm/i915/display/intel_cx0_phy.c | 87 ++++++++++++------- > drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 12 +-- > 2 files changed, 64 insertions(+), 35 deletions(-) > >diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c >b/drivers/gpu/drm/i915/display/intel_cx0_phy.c >index 7288065d2461..f3baba264e88 100644 >--- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c >+++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c >@@ -3225,11 +3225,8 @@ static void intel_cx0pll_enable(struct intel_encoder >*encoder, > { > int port_clock = pll_state->use_c10 ? pll_state->c10.clock : > pll_state->c20.clock; > struct intel_display *display = to_intel_display(encoder); >- enum phy phy = intel_encoder_to_phy(encoder); > struct intel_digital_port *dig_port = enc_to_dig_port(encoder); > bool lane_reversal = dig_port->lane_reversal; >- u8 maxpclk_lane = lane_reversal ? INTEL_CX0_LANE1 : >- INTEL_CX0_LANE0; > struct ref_tracker *wakeref = > intel_cx0_phy_transaction_begin(encoder); > > /* >@@ -3284,27 +3281,6 @@ static void intel_cx0pll_enable(struct intel_encoder >*encoder, > */ > intel_de_write(display, DDI_CLK_VALFREQ(encoder->port), port_clock); > >- /* >- * 9. Set PORT_CLOCK_CTL register PCLK PLL Request >- * LN<Lane for maxPCLK> to "1" to enable PLL. >- */ >- intel_de_rmw(display, XELPDP_PORT_CLOCK_CTL(display, encoder->port), >- intel_cx0_get_pclk_pll_request(INTEL_CX0_BOTH_LANES), >- intel_cx0_get_pclk_pll_request(maxpclk_lane)); >- >- /* 10. Poll on PORT_CLOCK_CTL PCLK PLL Ack LN<Lane for maxPCLK> == >"1". */ >- if (intel_de_wait_us(display, XELPDP_PORT_CLOCK_CTL(display, >encoder->port), >- intel_cx0_get_pclk_pll_ack(INTEL_CX0_BOTH_LANES), >- intel_cx0_get_pclk_pll_ack(maxpclk_lane), >- XELPDP_PCLK_PLL_ENABLE_TIMEOUT_US, NULL)) >- drm_warn(display->drm, "Port %c PLL not locked\n", >- phy_name(phy)); >- >- /* >- * 11. Follow the Display Voltage Frequency Switching Sequence After >- * Frequency Change. We handle this step in bxt_set_cdclk(). >- */ >- > /* > * 12. Toggle powerdown if HDMI is enabled on C10 PHY. > * >@@ -3403,6 +3379,42 @@ static int intel_mtl_tbt_clock_select(struct >intel_display *display, > } > } > >+static void intel_cx0pll_enable_clock(struct intel_encoder *encoder) >+{ >+ struct intel_display *display = to_intel_display(encoder); >+ enum phy phy = intel_encoder_to_phy(encoder); >+ struct intel_digital_port *dig_port = enc_to_dig_port(encoder); >+ bool lane_reversal = dig_port->lane_reversal; >+ INTEL_CX0_LANE0; >+ u8 maxpclk_lane = lane_reversal ? INTEL_CX0_LANE1 : >+ INTEL_CX0_LANE0; >+ >+ struct ref_tracker *wakeref = >intel_cx0_phy_transaction_begin(encoder); >+ >+ /* >+ * 9. Set PORT_CLOCK_CTL register PCLK PLL Request >+ * LN<Lane for maxPCLK> to "1" to enable PLL. >+ */ >+ intel_de_rmw(display, XELPDP_PORT_CLOCK_CTL(display, encoder->port), >+ intel_cx0_get_pclk_pll_request(INTEL_CX0_BOTH_LANES), >+ intel_cx0_get_pclk_pll_request(maxpclk_lane)); >+ >+ /* 10. Poll on PORT_CLOCK_CTL PCLK PLL Ack LN<Lane for maxPCLK> == >"1". */ >+ if (intel_de_wait_us(display, XELPDP_PORT_CLOCK_CTL(display, >encoder->port), >+ intel_cx0_get_pclk_pll_ack(INTEL_CX0_BOTH_LANES), >+ intel_cx0_get_pclk_pll_ack(maxpclk_lane), >+ XELPDP_PCLK_PLL_ENABLE_TIMEOUT_US, NULL)) >+ drm_warn(display->drm, "Port %c PLL not locked\n", >+ phy_name(phy)); >+ >+ /* >+ * 11. Follow the Display Voltage Frequency Switching Sequence After >+ * Frequency Change. We handle this step in bxt_set_cdclk(). >+ */ >+ >+ intel_cx0_phy_transaction_end(encoder, wakeref); >+} >+ > void intel_mtl_tbt_pll_enable_clock(struct intel_encoder *encoder, int > port_clock) > { > struct intel_display *display = to_intel_display(encoder); >@@ -3472,6 +3484,8 @@ void intel_mtl_pll_enable_clock(struct intel_encoder >*encoder, > > if (intel_tc_port_in_tbt_alt_mode(dig_port)) > intel_mtl_tbt_pll_enable_clock(encoder, > crtc_state->port_clock); >+ else >+ intel_cx0pll_enable_clock(encoder); > } > > /* >@@ -3567,12 +3581,6 @@ static void intel_cx0pll_disable(struct intel_encoder >*encoder) > * Frequency Change. We handle this step in bxt_set_cdclk(). > */ > >- /* 7. Program PORT_CLOCK_CTL register to disable and gate clocks. */ >- intel_de_rmw(display, XELPDP_PORT_CLOCK_CTL(display, encoder->port), >- XELPDP_DDI_CLOCK_SELECT_MASK(display), 0); >- intel_de_rmw(display, XELPDP_PORT_CLOCK_CTL(display, encoder->port), >- XELPDP_FORWARD_CLOCK_UNGATE, 0); >- > intel_cx0_phy_transaction_end(encoder, wakeref); > } > >@@ -3586,6 +3594,20 @@ static bool intel_cx0_pll_is_enabled(struct >intel_encoder *encoder) > intel_cx0_get_pclk_pll_request(lane); > } > >+static void intel_cx0pll_disable_clock(struct intel_encoder *encoder) >+{ >+ struct intel_display *display = to_intel_display(encoder); >+ struct ref_tracker *wakeref = >intel_cx0_phy_transaction_begin(encoder); >+ >+ /* 7. Program PORT_CLOCK_CTL register to disable and gate clocks. */ >+ intel_de_rmw(display, XELPDP_PORT_CLOCK_CTL(display, encoder->port), >+ XELPDP_DDI_CLOCK_SELECT_MASK(display), 0); >+ intel_de_rmw(display, XELPDP_PORT_CLOCK_CTL(display, encoder->port), >+ XELPDP_FORWARD_CLOCK_UNGATE, 0); >+ >+ intel_cx0_phy_transaction_end(encoder, wakeref); >+} >+ > void intel_mtl_tbt_pll_disable_clock(struct intel_encoder *encoder) > { > struct intel_display *display = to_intel_display(encoder); >@@ -3635,6 +3657,9 @@ void intel_mtl_pll_disable_clock(struct intel_encoder >*encoder) > > if (intel_tc_port_in_tbt_alt_mode(dig_port)) > intel_mtl_tbt_pll_disable_clock(encoder); >+ else >+ intel_cx0pll_disable_clock(encoder); >+ > } > > enum icl_port_dpll_id >@@ -3783,6 +3808,8 @@ void intel_cx0_pll_power_save_wa(struct intel_display >*display) > encoder->base.base.id, encoder->base.name); > > intel_cx0pll_enable(encoder, &pll_state); >+ intel_cx0pll_enable_clock(encoder); > intel_cx0pll_disable(encoder); >+ intel_cx0pll_disable_clock(encoder); > } > } >diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c >b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c >index 9aa84a430f09..59395076103c 100644 >--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c >+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c >@@ -186,11 +186,13 @@ void assert_dpll(struct intel_display *display, > "asserting DPLL %s with no DPLL\n", str_on_off(state))) > return; > >- cur_state = intel_dpll_get_hw_state(display, pll, &hw_state); >- INTEL_DISPLAY_STATE_WARN(display, cur_state != state, >- "%s assertion failure (expected %s, current >%s)\n", >- pll->info->name, str_on_off(state), >- str_on_off(cur_state)); >+ if (DISPLAY_VER(display) < 14) { >+ cur_state = intel_dpll_get_hw_state(display, pll, &hw_state); >+ INTEL_DISPLAY_STATE_WARN(display, cur_state != state, >+ "%s assertion failure (expected %s, >current %s)\n", >+ pll->info->name, str_on_off(state), >+ str_on_off(cur_state)); >+ } > } > > static enum tc_port icl_pll_id_to_tc_port(enum intel_dpll_id id) >-- >2.34.1 >
