On Tue, 2025-08-05 at 10:36 +0300, Imre Deak wrote: > The TypeC PHY HW readout during driver loading and system resume > determines which TypeC mode the PHY is in (legacy/DP-alt/TBT-alt) and > whether the PHY is connected, based on the PHY's Owned and Ready flags. > For the PHY to be in DP-alt or legacy mode and for the PHY to be in the > connected state in these modes, both the Owned (set by the BIOS/driver) > and the Ready (set by the HW) flags should be set. > > On ICL-MTL the HW kept the PHY's Ready flag set after the driver > connected the PHY by acquiring the PHY ownership (by setting the Owned > flag), until the driver disconnected the PHY by releasing the PHY > ownership (by clearing the Owned flag). On LNL+ this has changed, in > that the HW clears the Ready flag as soon as the sink gets disconnected, > even if the PHY ownership was acquired already and hence the PHY is > being used by the display. > > When inheriting the HW state from BIOS for a PHY connected in DP-alt > mode on which the sink got disconnected - i.e. in a case where the sink > was connected while BIOS/GOP was running and so the sink got enabled > connecting the PHY, but the user disconnected the sink by the time the > driver loaded - the PHY Owned but not Ready state must be accounted for > on LNL+ according to the above. Do that by assuming on LNL+ that the PHY > is connected in DP-alt mode whenever the PHY Owned flag is set, > regardless of the PHY Ready flag. > > This fixes a problem on LNL+, where the PHY TypeC mode / connected state > was detected incorrectly for a DP-alt sink, which got connected and then > disconnected by the user in the above way. > > Cc: sta...@vger.kernel.org # v6.8+ > Reported-by: Charlton Lin <charlton....@intel.com> > Tested-by: Khaled Almahallawy <khaled.almahall...@intel.com> > Signed-off-by: Imre Deak <imre.d...@intel.com> > --- > drivers/gpu/drm/i915/display/intel_tc.c | 16 ++++++++++------ > 1 file changed, 10 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_tc.c > b/drivers/gpu/drm/i915/display/intel_tc.c > index 3bc57579fe53e..73a08bd84a70a 100644 > --- a/drivers/gpu/drm/i915/display/intel_tc.c > +++ b/drivers/gpu/drm/i915/display/intel_tc.c > @@ -1226,14 +1226,18 @@ static void tc_phy_get_hw_state(struct intel_tc_port > *tc) > tc->phy_ops->get_hw_state(tc); > } > > -static bool tc_phy_is_ready_and_owned(struct intel_tc_port *tc, > - bool phy_is_ready, bool phy_is_owned) > +static bool tc_phy_in_legacy_or_dp_alt_mode(struct intel_tc_port *tc, > + bool phy_is_ready, bool > phy_is_owned)
Personally I don't like the "or" in the function name. You're returning a boolean which is true or false. The return value is akin to answering "Yes/No" to the question "Is it black or white". This is a nitpick, obviously, so I'll leave it up to you. Regardless: Reviewed-by: Luca Coelho <luciano.coe...@intel.com> -- Cheers, Luca. > { > struct intel_display *display = to_intel_display(tc->dig_port); > > - drm_WARN_ON(display->drm, phy_is_owned && !phy_is_ready); > + if (DISPLAY_VER(display) < 20) { > + drm_WARN_ON(display->drm, phy_is_owned && !phy_is_ready); > > - return phy_is_ready && phy_is_owned; > + return phy_is_ready && phy_is_owned; > + } else { > + return phy_is_owned; > + } > } > > static bool tc_phy_is_connected(struct intel_tc_port *tc, > @@ -1244,7 +1248,7 @@ static bool tc_phy_is_connected(struct intel_tc_port > *tc, > bool phy_is_owned = tc_phy_is_owned(tc); > bool is_connected; > > - if (tc_phy_is_ready_and_owned(tc, phy_is_ready, phy_is_owned)) > + if (tc_phy_in_legacy_or_dp_alt_mode(tc, phy_is_ready, phy_is_owned)) > is_connected = port_pll_type == ICL_PORT_DPLL_MG_PHY; > else > is_connected = port_pll_type == ICL_PORT_DPLL_DEFAULT; > @@ -1352,7 +1356,7 @@ tc_phy_get_current_mode(struct intel_tc_port *tc) > phy_is_ready = tc_phy_is_ready(tc); > phy_is_owned = tc_phy_is_owned(tc); > > - if (!tc_phy_is_ready_and_owned(tc, phy_is_ready, phy_is_owned)) { > + if (!tc_phy_in_legacy_or_dp_alt_mode(tc, phy_is_ready, phy_is_owned)) { > mode = get_tc_mode_in_phy_not_owned_state(tc, live_mode); > } else { > drm_WARN_ON(display->drm, live_mode == TC_PORT_TBT_ALT);