[PATCH] drm/i915: Favor last VBT child device with conflicting AUX ch/DDC pin

[PATCH] drm/i915: Favor last VBT child device with conflicting AUX ch/DDC pin

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The first come first served apporoach to handling the VBT
child device AUX ch conflicts has backfired. We have machines
in the wild where the VBT specifies both port A eDP and
port E DP (in that order) with port E being the real one.

So let's try to flip the preference around and let the last
child device win once again.

Cc: stable@vger.kernel.org
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Masami Ichikawa <masami256@gmail.com>
Tested-by: Torsten <freedesktop201910@liggy.de>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111966
Fixes: 36a0f92020dc ("drm/i915/bios: make child device order the priority order")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_bios.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
index 9628b485b179..f0307b04cc13 100644
--- a/drivers/gpu/drm/i915/display/intel_bios.c
+++ b/drivers/gpu/drm/i915/display/intel_bios.c
@@ -1270,7 +1270,7 @@ static void sanitize_ddc_pin(struct drm_i915_private *dev_priv,
 		DRM_DEBUG_KMS("port %c trying to use the same DDC pin (0x%x) as port %c, "
 			      "disabling port %c DVI/HDMI support\n",
 			      port_name(port), info->alternate_ddc_pin,
-			      port_name(p), port_name(port));
+			      port_name(p), port_name(p));
 
 		/*
 		 * If we have multiple ports supposedly sharing the
@@ -1278,9 +1278,14 @@ static void sanitize_ddc_pin(struct drm_i915_private *dev_priv,
 		 * port. Otherwise they share the same ddc bin and
 		 * system couldn't communicate with them separately.
 		 *
-		 * Give child device order the priority, first come first
-		 * served.
+		 * Give inverse child device order the priority,
+		 * last one wins. Yes, there are real machines
+		 * (eg. Asrock B250M-HDV) where VBT has both
+		 * port A and port E with the same AUX ch and
+		 * we must pick port E :(
 		 */
+		info = &dev_priv->vbt.ddi_port_info[p];
+
 		info->supports_dvi = false;
 		info->supports_hdmi = false;
 		info->alternate_ddc_pin = 0;
@@ -1316,7 +1321,7 @@ static void sanitize_aux_ch(struct drm_i915_private *dev_priv,
 		DRM_DEBUG_KMS("port %c trying to use the same AUX CH (0x%x) as port %c, "
 			      "disabling port %c DP support\n",
 			      port_name(port), info->alternate_aux_channel,
-			      port_name(p), port_name(port));
+			      port_name(p), port_name(p));
 
 		/*
 		 * If we have multiple ports supposedlt sharing the
@@ -1324,9 +1329,14 @@ static void sanitize_aux_ch(struct drm_i915_private *dev_priv,
 		 * port. Otherwise they share the same aux channel
 		 * and system couldn't communicate with them separately.
 		 *
-		 * Give child device order the priority, first come first
-		 * served.
+		 * Give inverse child device order the priority,
+		 * last one wins. Yes, there are real machines
+		 * (eg. Asrock B250M-HDV) where VBT has both
+		 * port A and port E with the same AUX ch and
+		 * we must pick port E :(
 		 */
+		info = &dev_priv->vbt.ddi_port_info[p];
+
 		info->supports_dp = false;
 		info->alternate_aux_channel = 0;
 	}
-- 
2.21.0

Re: [PATCH] drm/i915: Favor last VBT child device with conflicting AUX ch/DDC pin

[Jani Nikula]

On Fri, 11 Oct 2019, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The first come first served apporoach to handling the VBT
> child device AUX ch conflicts has backfired. We have machines
> in the wild where the VBT specifies both port A eDP and
> port E DP (in that order) with port E being the real one.
>
> So let's try to flip the preference around and let the last
> child device win once again.

I think there will be legitimate cases where we need first come first
served. Oh well, another VBT misery to tackle in the future.

Acked-by: Jani Nikula <jani.nikula@intel.com>


>
> Cc: stable@vger.kernel.org
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Masami Ichikawa <masami256@gmail.com>
> Tested-by: Torsten <freedesktop201910@liggy.de>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111966
> Fixes: 36a0f92020dc ("drm/i915/bios: make child device order the priority order")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_bios.c | 22 ++++++++++++++++------
>  1 file changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
> index 9628b485b179..f0307b04cc13 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> @@ -1270,7 +1270,7 @@ static void sanitize_ddc_pin(struct drm_i915_private *dev_priv,
>  		DRM_DEBUG_KMS("port %c trying to use the same DDC pin (0x%x) as port %c, "
>  			      "disabling port %c DVI/HDMI support\n",
>  			      port_name(port), info->alternate_ddc_pin,
> -			      port_name(p), port_name(port));
> +			      port_name(p), port_name(p));
>  
>  		/*
>  		 * If we have multiple ports supposedly sharing the
> @@ -1278,9 +1278,14 @@ static void sanitize_ddc_pin(struct drm_i915_private *dev_priv,
>  		 * port. Otherwise they share the same ddc bin and
>  		 * system couldn't communicate with them separately.
>  		 *
> -		 * Give child device order the priority, first come first
> -		 * served.
> +		 * Give inverse child device order the priority,
> +		 * last one wins. Yes, there are real machines
> +		 * (eg. Asrock B250M-HDV) where VBT has both
> +		 * port A and port E with the same AUX ch and
> +		 * we must pick port E :(
>  		 */
> +		info = &dev_priv->vbt.ddi_port_info[p];
> +
>  		info->supports_dvi = false;
>  		info->supports_hdmi = false;
>  		info->alternate_ddc_pin = 0;
> @@ -1316,7 +1321,7 @@ static void sanitize_aux_ch(struct drm_i915_private *dev_priv,
>  		DRM_DEBUG_KMS("port %c trying to use the same AUX CH (0x%x) as port %c, "
>  			      "disabling port %c DP support\n",
>  			      port_name(port), info->alternate_aux_channel,
> -			      port_name(p), port_name(port));
> +			      port_name(p), port_name(p));
>  
>  		/*
>  		 * If we have multiple ports supposedlt sharing the
> @@ -1324,9 +1329,14 @@ static void sanitize_aux_ch(struct drm_i915_private *dev_priv,
>  		 * port. Otherwise they share the same aux channel
>  		 * and system couldn't communicate with them separately.
>  		 *
> -		 * Give child device order the priority, first come first
> -		 * served.
> +		 * Give inverse child device order the priority,
> +		 * last one wins. Yes, there are real machines
> +		 * (eg. Asrock B250M-HDV) where VBT has both
> +		 * port A and port E with the same AUX ch and
> +		 * we must pick port E :(
>  		 */
> +		info = &dev_priv->vbt.ddi_port_info[p];
> +
>  		info->supports_dp = false;
>  		info->alternate_aux_channel = 0;
>  	}

-- 
Jani Nikula, Intel Open Source Graphics Center