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

[Jani Nikula <jani.nikula@linux.intel.com>]

On Tue, 07 Feb 2023, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> 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.

Okay then. $EXPLETIVE.

J.

>
>> 
>> 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

-- 
Jani Nikula, Intel Open Source Graphics Center