Re: [Intel-gfx] [PATCH 1/3] drm/i915: Fix VBT DSI DVO port handling

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Feb 07, 2023 at 10:59:36AM +0200, Jani Nikula wrote:
> On Tue, 07 Feb 2023, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Turns out modern (icl+) VBTs still declare their DSI ports
> > as MIPI-A and MIPI-C despite the PHYs now being A and B.
> > Remap appropriately to allow the panels declared as MIPI-C
> > to work.
> >
> > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/8016
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_bios.c | 33 ++++++++++++++++-------
> >  1 file changed, 23 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
> > index e6ca51232dcf..06a2d98d2277 100644
> > --- a/drivers/gpu/drm/i915/display/intel_bios.c
> > +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> > @@ -2467,6 +2467,22 @@ static enum port dvo_port_to_port(struct drm_i915_private *i915,
> >  					  dvo_port);
> >  }
> >  
> > +static enum port
> > +dsi_dvo_port_to_port(struct drm_i915_private *i915, u8 dvo_port)
> > +{
> > +	switch (dvo_port) {
> > +	case DVO_PORT_MIPIA:
> > +		return PORT_A;
> 
> I think I would add:
> 
> 	case DVO_PORT_MIPIB:
> 		if (DISPLAY_VER(i915) >= 11)
> 			return PORT_B;
> 		else
> 			return PORT_NONE;
> 
> just in case.

Looks like Windows doesn't expect MIPI-B to be used ever.
So I'm tempted to leave it out as well.

> 
> With that,
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> 
> > +	case DVO_PORT_MIPIC:
> > +		if (DISPLAY_VER(i915) >= 11)
> > +			return PORT_B;
> > +		else
> > +			return PORT_C;
> > +	default:
> > +		return PORT_NONE;
> > +	}
> > +}
> > +
> >  static int parse_bdb_230_dp_max_link_rate(const int vbt_max_link_rate)
> >  {
> >  	switch (vbt_max_link_rate) {
> > @@ -3442,19 +3458,16 @@ bool intel_bios_is_dsi_present(struct drm_i915_private *i915,
> >  
> >  		dvo_port = child->dvo_port;
> >  
> > -		if (dvo_port == DVO_PORT_MIPIA ||
> > -		    (dvo_port == DVO_PORT_MIPIB && DISPLAY_VER(i915) >= 11) ||
> > -		    (dvo_port == DVO_PORT_MIPIC && DISPLAY_VER(i915) < 11)) {
> > -			if (port)
> > -				*port = dvo_port - DVO_PORT_MIPIA;
> > -			return true;
> > -		} else if (dvo_port == DVO_PORT_MIPIB ||
> > -			   dvo_port == DVO_PORT_MIPIC ||
> > -			   dvo_port == DVO_PORT_MIPID) {
> > +		if (dsi_dvo_port_to_port(i915, dvo_port) == PORT_NONE) {
> 
> Yeah that monstrosity should've been a separate function a long time ago!
> 
> >  			drm_dbg_kms(&i915->drm,
> >  				    "VBT has unsupported DSI port %c\n",
> >  				    port_name(dvo_port - DVO_PORT_MIPIA));
> > +			continue;
> >  		}
> > +
> > +		if (port)
> > +			*port = dsi_dvo_port_to_port(i915, dvo_port);
> > +		return true;
> >  	}
> >  
> >  	return false;
> > @@ -3539,7 +3552,7 @@ bool intel_bios_get_dsc_params(struct intel_encoder *encoder,
> >  		if (!(child->device_type & DEVICE_TYPE_MIPI_OUTPUT))
> >  			continue;
> >  
> > -		if (child->dvo_port - DVO_PORT_MIPIA == encoder->port) {
> > +		if (dsi_dvo_port_to_port(i915, child->dvo_port) == encoder->port) {
> >  			if (!devdata->dsc)
> >  				return false;
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center

-- 
Ville Syrjälä
Intel