Quoting Kandpal, Suraj (2025-12-31 02:10:59-03:00) >> >> Quoting Suraj Kandpal (2025-12-30 05:31:41-03:00) >> >Move the step to toggle powerdown sequence change for HDMI to enable >> >clock hook where it belongs according to its sequence. >> >Do the required changes to make that work. >> >> This should probably be a squash into the previous patch? > >So reason for separate patch is that this requires me changing the argument of >clock enable which is not because of the same logical reason that changes are >being done in patch 1, >hence a separate patch for changes that were brought about due to another >reason. Had this been just movement >of step 12 then I would have squashed them.
Hm... The previous patch is introducing intel_cx0pll_enable_clock() and says it is splitting the sequence in two, but then it ended up is leaving step 12 behind. If it is introducing intel_cx0pll_enable_clock(), it could as well have done it with a signature that allows step 12 to be done. IMO, here we are modifying that function to "make it right". This looks like a good fixup candidate to me. -- Gustavo Sousa > >Regards, >Suraj Kandpal > >> >> -- >> Gustavo Sousa >> >> > >> >Signed-off-by: Suraj Kandpal <[email protected]> >> >--- >> > drivers/gpu/drm/i915/display/intel_cx0_phy.c | 37 ++++++++++---------- >> > 1 file changed, 19 insertions(+), 18 deletions(-) >> > >> >diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c >> >b/drivers/gpu/drm/i915/display/intel_cx0_phy.c >> >index f3baba264e88..5edd293b533b 100644 >> >--- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c >> >+++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c >> >@@ -3281,21 +3281,6 @@ static void intel_cx0pll_enable(struct >> intel_encoder *encoder, >> > */ >> > intel_de_write(display, DDI_CLK_VALFREQ(encoder->port), >> >port_clock); >> > >> >- /* >> >- * 12. Toggle powerdown if HDMI is enabled on C10 PHY. >> >- * >> >- * Wa_13013502646: >> >- * Fixes: HDMI lane to lane skew violations on C10 display PHYs. >> >- * Workaround: Toggle powerdown value by setting first to P0 and >> >then >> to P2, for both >> >- * PHY lanes. >> >- */ >> >- if (!cx0pll_state_is_dp(pll_state) && pll_state->use_c10) { >> >- intel_cx0_powerdown_change_sequence(encoder, >> INTEL_CX0_BOTH_LANES, >> >- >> >XELPDP_P0_STATE_ACTIVE); >> >- intel_cx0_powerdown_change_sequence(encoder, >> INTEL_CX0_BOTH_LANES, >> >- XELPDP_P2_STATE_READY); >> >- } >> >- >> > intel_cx0_phy_transaction_end(encoder, wakeref); } >> > >> >@@ -3379,7 +3364,8 @@ static int intel_mtl_tbt_clock_select(struct >> intel_display *display, >> > } >> > } >> > >> >-static void intel_cx0pll_enable_clock(struct intel_encoder *encoder) >> >+static void intel_cx0pll_enable_clock(struct intel_encoder *encoder, >> >+ const struct intel_cx0pll_state >> >+*pll_state) >> > { >> > struct intel_display *display = to_intel_display(encoder); >> > enum phy phy = intel_encoder_to_phy(encoder); @@ -3412,6 >> >+3398,21 @@ static void intel_cx0pll_enable_clock(struct intel_encoder >> *encoder) >> > * Frequency Change. We handle this step in bxt_set_cdclk(). >> > */ >> > >> >+ /* >> >+ * 12. Toggle powerdown if HDMI is enabled on C10 PHY. >> >+ * >> >+ * Wa_13013502646: >> >+ * Fixes: HDMI lane to lane skew violations on C10 display PHYs. >> >+ * Workaround: Toggle powerdown value by setting first to P0 and >> >then >> to P2, for both >> >+ * PHY lanes. >> >+ */ >> >+ if (!cx0pll_state_is_dp(pll_state) && pll_state->use_c10) { >> >+ intel_cx0_powerdown_change_sequence(encoder, >> INTEL_CX0_BOTH_LANES, >> >+ >> >XELPDP_P0_STATE_ACTIVE); >> >+ intel_cx0_powerdown_change_sequence(encoder, >> INTEL_CX0_BOTH_LANES, >> >+ XELPDP_P2_STATE_READY); >> >+ } >> >+ >> > intel_cx0_phy_transaction_end(encoder, wakeref); } >> > >> >@@ -3485,7 +3486,7 @@ 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); >> >+ intel_cx0pll_enable_clock(encoder, >> >+ &crtc_state->dpll_hw_state.cx0pll); >> > } >> > >> > /* >> >@@ -3808,7 +3809,7 @@ 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_enable_clock(encoder, &pll_state); >> > intel_cx0pll_disable(encoder); >> > intel_cx0pll_disable_clock(encoder); >> > } >> >-- >> >2.34.1 >> >
