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

[Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx>]

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

With that,

Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx>

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