Re: [PATCH 2/5] drm/i915/tc: Add separate intel_tc_phy_port_to_tc() for TC DDI/PHY ports

[Imre Deak <imre.deak@intel.com>]

On Fri, Nov 21, 2025 at 01:19:38PM +0200, Jani Nikula wrote:
> On Thu, 20 Nov 2025, Imre Deak <imre.deak@intel.com> wrote:
> > intel_port_to_tc() returns the PORT_TC1..6 -> TC_PORT_1..6 mapping only
> > for DDI ports that are connected to a TypeC PHY. In some cases this
> > mapping is also required for TypeC DDI ports which are not connected to
> > a TypeC PHY. Such DDI ports are the PORT_TC1..4 ports on RKL/ADLS/BMG.
> >
> > Add a separate intel_tc_phy_to_tc() helper to return the mapping for
> > ports connected to a TypeC PHY, and make all the current users - which
> > expect this semantic - call this helper. A follow-up change will need to
> > get the same mapping for TypeC DDI ports not connected to a TypeC PHY,
> > leave intel_port_to_tc() exported for that.
> 
> The TC port and phy stuff in our driver never cease to confuse me. And I
> know you've explained all this to me several times.
> 
> I think we need some "TC port and phy for dummies" (that's me)
> documentation comment somewhere in intel_tc.c that we (I) can refer to.

I documented in this patch how the HW can connect a "TypeC" DDI to a
non-TypeC PHY. But I agree a documentation about all the kinds of DDIs
and PHYs used on each platform and the way these DDIs can be connected
to different PHYs would be good. I'll try to add that.

> I also feel like the whole mess of intel_ddi_encoder_name() underlines
> how problematic the concepts are.

That function should determine the name of the DDI and the PHY connected
to that DDI in a way, you can look up the corresponding DDIs and PHYs
from Bspec. I think the names are correct now, but I agree the
platform/port if-ladder is not great and should be improved. Will also
think more about this.

> Moreover, I think intel_bios.c logs the ports incorrectly.

In print_ddi_port(), I suppose. Yes, it's incorrect, imo it should match
the corresponding intel_ddi_encoder_name().

> BR,
> Jani.
> 
> 
> > Cc: Suraj Kandpal <suraj.kandpal@intel.com>
> > Cc: Mika Kahola <mika.kahola@intel.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_ddi.c     |  4 ++--
> >  drivers/gpu/drm/i915/display/intel_display.c | 23 ++++++++++++++++----
> >  drivers/gpu/drm/i915/display/intel_display.h |  1 +
> >  3 files changed, 22 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index 8471bdab5c62f..ed9798b0ec009 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -5148,7 +5148,7 @@ static const char *intel_ddi_encoder_name(struct intel_display *display,
> >  			       port_name(port - PORT_D_XELPD + PORT_D),
> >  			       phy_name(phy));
> >  	} else if (DISPLAY_VER(display) >= 12) {
> > -		enum tc_port tc_port = intel_port_to_tc(display, port);
> > +		enum tc_port tc_port = intel_tc_phy_port_to_tc(display, port);
> >  
> >  		seq_buf_printf(s, "DDI %s%c/PHY %s%c",
> >  			       port >= PORT_TC1 ? "TC" : "",
> > @@ -5156,7 +5156,7 @@ static const char *intel_ddi_encoder_name(struct intel_display *display,
> >  			       tc_port != TC_PORT_NONE ? "TC" : "",
> >  			       tc_port != TC_PORT_NONE ? tc_port_name(tc_port) : phy_name(phy));
> >  	} else if (DISPLAY_VER(display) >= 11) {
> > -		enum tc_port tc_port = intel_port_to_tc(display, port);
> > +		enum tc_port tc_port = intel_tc_phy_port_to_tc(display, port);
> >  
> >  		seq_buf_printf(s, "DDI %c%s/PHY %s%c",
> >  			       port_name(port),
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index 6c8a7f63111ec..a8a3e80001870 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -1859,17 +1859,32 @@ enum phy intel_port_to_phy(struct intel_display *display, enum port port)
> >  }
> >  
> >  /* Prefer intel_encoder_to_tc() */
> > +/*
> > + * Return TC_PORT_1..I915_MAX_TC_PORTS for any TypeC DDI port. The function
> > + * can be also called for TypeC DDI ports not connected to a TypeC PHY such as
> > + * the PORT_TC1..4 ports on RKL/ADLS/BMG.
> > + */
> >  enum tc_port intel_port_to_tc(struct intel_display *display, enum port port)
> >  {
> > -	if (!intel_phy_is_tc(display, intel_port_to_phy(display, port)))
> > -		return TC_PORT_NONE;
> > -
> >  	if (DISPLAY_VER(display) >= 12)
> >  		return TC_PORT_1 + port - PORT_TC1;
> >  	else
> >  		return TC_PORT_1 + port - PORT_C;
> >  }
> >  
> > +/*
> > + * Return TC_PORT_1..I915_MAX_TC_PORTS for TypeC DDI ports connected to a TypeC PHY.
> > + * Note that on RKL, ADLS, BMG the PORT_TC1..4 ports are connected to a non-TypeC
> > + * PHY, so on those platforms the function returns TC_PORT_NONE.
> > + */
> > +enum tc_port intel_tc_phy_port_to_tc(struct intel_display *display, enum port port)
> > +{
> > +	if (!intel_phy_is_tc(display, intel_port_to_phy(display, port)))
> > +		return TC_PORT_NONE;
> > +
> > +	return intel_port_to_tc(display, port);
> > +}
> > +
> >  enum phy intel_encoder_to_phy(struct intel_encoder *encoder)
> >  {
> >  	struct intel_display *display = to_intel_display(encoder);
> > @@ -1902,7 +1917,7 @@ enum tc_port intel_encoder_to_tc(struct intel_encoder *encoder)
> >  {
> >  	struct intel_display *display = to_intel_display(encoder);
> >  
> > -	return intel_port_to_tc(display, encoder->port);
> > +	return intel_tc_phy_port_to_tc(display, encoder->port);
> >  }
> >  
> >  enum intel_display_power_domain
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> > index bcc6ccb69d2b5..f8e6e4e827222 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display.h
> > @@ -451,6 +451,7 @@ bool intel_phy_is_combo(struct intel_display *display, enum phy phy);
> >  bool intel_phy_is_tc(struct intel_display *display, enum phy phy);
> >  bool intel_phy_is_snps(struct intel_display *display, enum phy phy);
> >  enum tc_port intel_port_to_tc(struct intel_display *display, enum port port);
> > +enum tc_port intel_tc_phy_port_to_tc(struct intel_display *display, enum port port);
> >  
> >  enum phy intel_encoder_to_phy(struct intel_encoder *encoder);
> >  bool intel_encoder_is_combo(struct intel_encoder *encoder);
> 
> -- 
> Jani Nikula, Intel