> -----Original Message----- > From: Deak, Imre <[email protected]> > Sent: Monday, 8 December 2025 14.59 > To: Jani Nikula <[email protected]> > Cc: [email protected]; [email protected]; Kahola, > Mika <[email protected]> > Subject: Re: [PATCH] drm/i915/cx0: Convert C10 PHY PLL SSC state mismatch > WARN to a debug message > > On Mon, Dec 08, 2025 at 02:03:15PM +0200, Jani Nikula wrote: > > On Fri, 05 Dec 2025, Imre Deak <[email protected]> wrote: > > > On C10 PHY PLLs the SSC is enabled by programming the > > > XELPDP_PORT_CLOCK_CTL / XELPDP_SSC_ENABLE_PLLB flag and the > > > PHY_C10_VDR_PLL 4..8 registers: > > > > > > - If SSC is enabled XELPDP_SSC_ENABLE_PLLB is set and the > > > PHY_C10_VDR_PLL registers are programmed to non-zero values. > > > - If SSC is disabled XELPDP_SSC_ENABLE_PLLB is cleared and the > > > PHY_C10_VDR_PLL registers are programmed to zeroed-out values. > > > > > > The driver's state checker verifies if the above settings are > > > consistent, i.e. if XELPDP_SSC_ENABLE_PLLB being set corresponds to > > > the PHY_C10_VDR_PLL registers being zeroed-out or not. > > > > > > On WCL the BIOS programs non-zero values to the PHY_C10_VDR_PLL 4..8 > > > registers, but does not set the XELPDP_SSC_ENABLE_PLLB flag. This > > > will trigger the following PLL state check warning during driver loading: > > > > > > <4>[ 44.457809] xe 0000:00:02.0: [drm] PHY B: SSC enabled state (no), > > > doesn't match PLL configuration (SSC-enabled) > > > > BTW I also think the message is really confusing. > > > > "SSC enabled state (no)" vs. "PLL configuration (SSC-enabled)". > > > > *BOTH* need to say SSC with str_enabled_disabled() and ditch the > > clumsy "SSC enabled state yes/no" and "SSC-enabled/SSC-disabled". > > Makes sense, can change it to:
> if (cx0pll_state->ssc_enabled != intel_c10pll_ssc_enabled(pll_state)) > drm_dbg_kms(display->drm, > "PHY %c: SSC state mismatch: port SSC is %s, PLL SSC is > %s\n", > phy_name(phy), > str_enabled_disabled(cx0pll_state->ssc_enabled), > str_enabled_disabled(intel_c10pll_ssc_enabled(pll_state))); With this change applied Reviewed-by: Mika Kahola <[email protected]> > > > > > BR, > > Jani. > > > > > > > <4>[ 44.457833] WARNING: CPU: 4 PID: 298 at > > > drivers/gpu/drm/i915/display/intel_cx0_phy.c:2281 > intel_cx0pll_readout_hw_state+0x221/0x620 [xe] > > > > > > It's not clear whether the HW uses the PHY_C10_VDR_PLL 4..8 register > > > values if the XELPDP_SSC_ENABLE_PLLB flag is cleared, or just > > > ignores them in this case. Since the driver always programs the > > > register values according to the above, it still makes sense to > > > verify that the programming happened correctly. > > > > > > To avoid the state check WARN during driver loading due to the way > > > BIOS programs the registers, convert the WARN to a debug message. > > > > > > Cc: Mika Kahola <[email protected]> > > > Signed-off-by: Imre Deak <[email protected]> > > > --- > > > drivers/gpu/drm/i915/display/intel_cx0_phy.c | 11 ++++++----- > > > 1 file changed, 6 insertions(+), 5 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c > > > b/drivers/gpu/drm/i915/display/intel_cx0_phy.c > > > index 7bd17723e7abb..b973a9201edda 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c > > > +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c > > > @@ -2278,11 +2278,12 @@ static void intel_c10pll_readout_hw_state(struct > > > intel_encoder *encoder, > > > pll_state->clock = intel_c10pll_calc_port_clock(encoder, > > > pll_state); > > > > > > cx0pll_state->ssc_enabled = readout_ssc_state(encoder, true); > > > - drm_WARN(display->drm, > > > - cx0pll_state->ssc_enabled != > > > intel_c10pll_ssc_enabled(pll_state), > > > - "PHY %c: SSC enabled state (%s), doesn't match PLL > > > configuration (%s)\n", > > > - phy_name(phy), str_yes_no(cx0pll_state->ssc_enabled), > > > - intel_c10pll_ssc_enabled(pll_state) ? "SSC-enabled" : > > > "SSC-disabled"); > > > + > > > + if (cx0pll_state->ssc_enabled != intel_c10pll_ssc_enabled(pll_state)) > > > + drm_dbg_kms(display->drm, > > > + "PHY %c: SSC enabled state (%s), doesn't match PLL > > > configuration (%s)\n", > > > + phy_name(phy), > > > str_yes_no(cx0pll_state->ssc_enabled), > > > + intel_c10pll_ssc_enabled(pll_state) ? "SSC-enabled" > > > : > > > +"SSC-disabled"); > > > } > > > > > > static void intel_c10_pll_program(struct intel_display *display, > > > > -- > > Jani Nikula, Intel
