On Thu, 2025-08-07 at 15:50 +0300, Imre Deak wrote: > On Thu, Aug 07, 2025 at 03:33:04PM +0300, Imre Deak wrote: > > On Thu, Aug 07, 2025 at 03:19:17PM +0300, Jani Nikula wrote: > > > On Thu, 07 Aug 2025, Imre Deak <imre.d...@intel.com> wrote: > > > > On Thu, Aug 07, 2025 at 01:59:21PM +0300, Luca Coelho wrote: > > > > > 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". > > > > > > > > The question the function is meant to answer is "Is the PHY in legacy or > > > > DP-alt mode?". The return value is true (yes) if the PHY is in either > > > > legacy or DP-alt mode, false (no) if the PHY is neither in legacy or > > > > DP-alt mode. There are many other uses of "or" in function names in this > > > > sense, so not sure how else I'd name this one. Simply leaving out "or" > > > > would make it less clear that the legacy and DP-alt modes are two > > > > separate modes.
Right, I missed that. But one shouldn't have to go read the function implementation to know what it's doing. > > > > > > What's the opposite of "Is the PHY in legacy or DP-alt mode"? > > > > > > Would that lead to a simpler name, with the reversed return value? > > > > The opposite is either TBT-alt or disconnected mode, so the reversal > > would result in the same function name format. > > Would you be ok with > > tc_phy_owned_by_display() > > ? This sounds much better! And it's indeed what you're looking for. It doesn't matter if it's legacy DP-alt, TBT-alt or whatever. What you're checking is actually whether display owns the PHY. So this is a much better choice IMHO. :) -- Cheers, Luca.
