On Fri, Nov 21, 2025 at 01:19:38PM +0200, Jani Nikula wrote: > On Thu, 20 Nov 2025, Imre Deak <[email protected]> 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 <[email protected]> > > Cc: Mika Kahola <[email protected]> > > Signed-off-by: Imre Deak <[email protected]> > > --- > > 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
