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".

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

Reply via email to