Re: [Intel-gfx] [PATCH 11/14] drm/i915/tc: Assume a TC port is legacy if VBT says the port has HDMI

From: Imre Deak <imre.deak@intel.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 11/14] drm/i915/tc: Assume a TC port is legacy if VBT says the port has HDMI
Date: Mon, 20 Mar 2023 23:33:00 +0200	[thread overview]
Message-ID: <ZBjRDHFt4MTYKMJ4@ideak-desk.fi.intel.com> (raw)
In-Reply-To: <ZBi7lM6K22AK3VXo@intel.com>

On Mon, Mar 20, 2023 at 10:01:24PM +0200, Ville Syrjälä wrote:
> On Thu, Mar 16, 2023 at 03:17:21PM +0200, Imre Deak wrote:
> > Since an HDMI output can only be enabled in legacy mode on TC ports,
> > assume that VBT is wrong and the port is legacy if VBT says the port is
> > non-legacy and has HDMI.
> > 
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_ddi.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index c531fee888a49..e79da640759c3 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -4494,6 +4494,13 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
> >  			!intel_bios_encoder_supports_typec_usb(devdata) &&
> >  			!intel_bios_encoder_supports_tbt(devdata);
> >  
> > +		if (!is_legacy && init_hdmi) {
> > +			drm_dbg_kms(&dev_priv->drm,
> > +				    "VBT says port %c is non-legacy TC and has HDMI, assume it's legacy TC\n",
> > +				    port_name(port));
> > +			is_legacy = true;
> > +		}
> 
> Have we actually seen this in practice?

Not on production machines, but possibly only due to the fixup based on
HPD later. IIRC the ICL RVP I tested has such a VBT.

> And does the port then actually work correctly?

On the above ICL yes, in general an HDMI-only TC port will only work in
legacy mode.

> If not then I think I'd just WARN an bail here.

Imo it's better to enable it in the only mode it works. However if VBT
says both DP and HDMI is present then the port may be DP-alt/TBT in
reality; so how about the above but only in case of
!is_legacy && !init_dp && init_hdmi
and relying on the HPD fixup otherwise?

> > +
> >  		intel_tc_port_init(dig_port, is_legacy);
> >  
> >  		encoder->update_prepare = intel_ddi_update_prepare;
> > -- 
> > 2.37.1
> 
> -- 
> Ville Syrjälä
> Intel