On Wed, 28 Oct 2020, Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx> wrote: > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > Let's pimp the DDI encoder->name to reflect what the spec calls them. > Ie. on pre-tgl DDI A-F, on tgl+ DDI A-C or DDI TC1-6. > > Also since each encoder is really a combination of the DDI and the PHY > we include the PHY name as well. > > ICL is a bit special since it already has the two different types > of DDIs (combo or TC) but it still calls them just DDI A-F regarless > of the type. For that let's add an extra "(TC)" note to remind > is which type of DDI it really is. > > The code is darn ugly, but not sure there's much we can do about it. > > Reviewed-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx> > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > --- > drivers/gpu/drm/i915/display/intel_ddi.c | 27 ++++++++++++++++++++++-- > 1 file changed, 25 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c > index 24245157dcb9..19b16517a502 100644 > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > @@ -5174,8 +5174,31 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port) > > encoder = &dig_port->base; > > - drm_encoder_init(&dev_priv->drm, &encoder->base, &intel_ddi_funcs, > - DRM_MODE_ENCODER_TMDS, "DDI %c", port_name(port)); > + if (INTEL_GEN(dev_priv) >= 12) { > + enum tc_port tc_port = intel_port_to_tc(dev_priv, port); > + > + drm_encoder_init(&dev_priv->drm, &encoder->base, &intel_ddi_funcs, > + DRM_MODE_ENCODER_TMDS, > + "DDI %s%c/PHY %s%c", > + port >= PORT_TC1 ? "TC" : "", > + port >= PORT_TC1 ? port_name(port) : port - PORT_TC1 + '1', > + tc_port != TC_PORT_NONE ? "TC" : "", > + tc_port != TC_PORT_NONE ? phy_name(phy) : tc_port - TC_PORT_1 + '1'); Frankly, this is a really ugly way to define encoder names, and it's hard to decipher what's actually going on. Even after I see logs with obviously bogus names such as: [ENCODER:235:DDI ./PHY 0] I find it tedious to decipher what exactly is wrong here. I guess the 2nd port >= PORT_TC1 check should be reversed, but it doesn't exactly give me confidence about the rest. BR, Jani. > + } else if (INTEL_GEN(dev_priv) >= 11) { > + enum tc_port tc_port = intel_port_to_tc(dev_priv, port); > + > + drm_encoder_init(&dev_priv->drm, &encoder->base, &intel_ddi_funcs, > + DRM_MODE_ENCODER_TMDS, > + "DDI %c%s/PHY %s%c", > + port_name(port), > + port >= PORT_C ? " (TC)" : "", > + tc_port != TC_PORT_NONE ? "TC" : "", > + tc_port != TC_PORT_NONE ? phy_name(phy) : tc_port - TC_PORT_1 + '1'); > + } else { > + drm_encoder_init(&dev_priv->drm, &encoder->base, &intel_ddi_funcs, > + DRM_MODE_ENCODER_TMDS, > + "DDI %c/PHY %c", port_name(port), phy_name(phy)); > + } > > mutex_init(&dig_port->hdcp_mutex); > dig_port->num_hdcp_streams = 0; -- Jani Nikula, Intel Open Source Graphics Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx