Re: [PATCH 5/6] drm/i915/mtl+: Use PHY type to identify TypeC ports in PLL ID lookup

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/i915/dpll: broken PLL ID lookup for PORT_TC1..4 on BMG in mtl_port_to_pll_id()
--

commit cf78fabe6f011d3447934883b6ff2b74f3d46e5f
Author: Imre Deak <imre.deak@intel.com>

drm/i915/mtl+: Use PHY type to identify TypeC ports in PLL ID lookup

This commit replaces the port >= PORT_TC1 check in mtl_port_to_pll_id() with
an intel_phy_is_tc() check so that port B can take the TypeC path when its
PHY type is set to TypeC on PTL+ platforms.

> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> index bfd6f0a197253..563db12cf2ef3 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> @@ -207,7 +207,7 @@ enum intel_dpll_id icl_tc_port_to_pll_id(enum tc_port tc_port)
>  
>  enum intel_dpll_id mtl_port_to_pll_id(struct intel_display *display, enum port port)
>  {
> -	if (port >= PORT_TC1)
> +	if (intel_phy_is_tc(display, intel_port_to_phy(display, port)))
>  		return icl_tc_port_to_pll_id(intel_port_to_tc(display, port));

[Severity: High]
Does this break PLL ID lookup for PORT_TC1..PORT_TC4 on Battlemage?

On Battlemage (bmg_desc with PLATFORM_GROUP(dgfx) and xe2_hpd_display),
display->platform.dgfx is true and port_mask includes PORT_A and
PORT_TC1..PORT_TC4, while DISPLAY_VER(display) >= 14 selects mtl_pll_mgr in
intel_dpll_init().

Because BMG's PORT_TC1..PORT_TC4 DDI ports are not connected to a TypeC PHY
(as noted in the comment above intel_port_to_tc()), intel_phy_is_tc()
unconditionally returns false on dgfx:

drivers/gpu/drm/i915/display/intel_display.c:intel_phy_is_tc() {
	if (display->platform.dgfx)
		return false;
    ...
}

With port >= PORT_TC1 replaced by intel_phy_is_tc(), calling
mtl_port_to_pll_id() for PORT_TC1..PORT_TC4 on BMG falls through into the
switch statement:

drivers/gpu/drm/i915/display/intel_dpll_mgr.c:mtl_port_to_pll_id() {
	if (intel_phy_is_tc(display, intel_port_to_phy(display, port)))
		return icl_tc_port_to_pll_id(intel_port_to_tc(display, port));

	switch (port) {
	case PORT_A:
		return DPLL_ID_ICL_DPLL0;
	case PORT_B:
		return DPLL_ID_ICL_DPLL1;
	default:
		MISSING_CASE(port);
		return DPLL_ID_ICL_DPLL0;
	}
}

Wouldn't this trigger MISSING_CASE(port) and return DPLL_ID_ICL_DPLL0 instead
of DPLL_ID_ICL_MGPLL1..DPLL_ID_ICL_MGPLL4 during hardware state readout
(mtl_pll_get_hw_state(), mtl_ddi_non_tc_phy_get_config()) and modesets
(mtl_get_dplls() -> mtl_get_non_tc_phy_dpll(), get_intel_encoder() in
mtl_pll_enable(), mtl_pll_disable(), and mtl_pll_get_freq())?

Should the condition check both intel_phy_is_tc() and port >= PORT_TC1?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261029113017.618286-1-imre.deak@intel.com?part=5