Re: [PATCH v2 4/4] drm/i915/lt_phy: apply the PLL power save W/A on LT PHY platforms

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/i915/lt_phy: missing TBT alt mode check in power save W/A
--

commit 64f624396c6597f36365729f25b701c8533785f7
Author: Mika Kahola <mika.kahola@intel.com>

drm/i915/lt_phy: apply the PLL power save W/A on LT PHY platforms

This commit adds the LT PHY sequence for the PLL power save workaround
and dispatches based on the PHY type. The PLL state is calculated from the eDP
tables at RBR as the C10 sequence does.

> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> index e899b2eb08711..9f34574775da2 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> @@ -5245,7 +5245,10 @@ void intel_dpll_power_save_wa(struct intel_display *display)
>  		if (!intel_encoder_is_dig_port(encoder))
>  			continue;
>  
> -		intel_cx0_pll_power_save_wa(encoder);
> +		if (HAS_LT_PHY(display))
> +			intel_lt_phy_pll_power_save_wa(encoder);
> +		else
> +			intel_cx0_pll_power_save_wa(encoder);

[Severity: High]
Does this platform-wide dispatch route all digital ports to the LT PHY
workaround on platforms with HAS_LT_PHY(display), including TC ports?

It appears this checks the platform capability rather than the actual PHY type
of the encoder, which may lead to applying the LT PHY workaround unconditionally
across different digital ports.

[ ... ]

> diff --git a/drivers/gpu/drm/i915/display/intel_lt_phy.c b/drivers/gpu/drm/i915/display/intel_lt_phy.c
> index e05fd23b8a651..d286d33a03eae 100644
> --- a/drivers/gpu/drm/i915/display/intel_lt_phy.c
> +++ b/drivers/gpu/drm/i915/display/intel_lt_phy.c
> @@ -2313,6 +2313,35 @@ void intel_xe3plpd_pll_disable(struct intel_encoder *encoder)
>  
>  }
>  
> +/*
> + * WA 14022081154
> + * LT PHY part of the sequence, see intel_dpll_power_save_wa().
> + */
> +void intel_lt_phy_pll_power_save_wa(struct intel_encoder *encoder)
> +{

[ ... ]

> +	drm_dbg_kms(display->drm,
> +		    "[ENCODER:%d:%s] Applying power saving workaround on disabled PLL\n",
> +		    encoder->base.base.id, encoder->base.name);
> +
> +	intel_lt_phy_pll_enable(encoder, &hw_state);
> +	intel_lt_phy_pll_disable(encoder);

[Severity: High]
Can this bypass TBT alt mode checks and corrupt Thunderbolt-owned shared port
registers?

During driver initialization and reset sequences (such as intel_display_reset()
calling intel_dpll_power_save_wa()), this executes automatically. If the system
boots with an active TBT alt mode display, the LT PHY PLL is likely disabled
since TBT uses a separate clocking path. 

This causes intel_lt_phy_pll_is_enabled(encoder) to return false, leading to
blindly executing intel_lt_phy_pll_enable() and intel_lt_phy_pll_disable().
Without the intel_tc_port_in_tbt_alt_mode() safety guard found in other
functions like intel_xe3plpd_pll_disable(), this bypass might overwrite the
port's clock select mask and assert MacCLK resets, potentially breaking active
TBT displays.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014105306.2761683-1-mika.kahola@intel.com?part=4