Re: [Intel-gfx] [PATCH 1/4] drm/i915: Respect alternate_aux_channel for all DDI ports

[Jim Bride]

On Tue, Oct 11, 2016 at 08:52:45PM +0300, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> The VBT provides the platform a way to mix and match the DDI ports vs.
> AUX channels. Currently we only trust the VBT for DDI E, which has no
> corresponding AUX channel of its own. However it is possible that some
> board might use some non-standard DDI vs. AUX port routing even for
> the other ports. Perhaps for signal routing reasons or something,
> So let's generalize this and trust the VBT for all ports.
> 
> For now we'll limit this to DDI platforms, as we trust the VBT a bit
> more there anyway when it comes to the DDI ports. I've structured
> the code in a way that would allow us to easily expand this to
> other platforms as well, by simply filling in the ddi_port_info.
> 
> v2: Drop whitespace changes, keep MISSING_CASE() for unknown
> aux ch assignment, include a commit message, include debug
> message during init
> 
> Cc: sta...@vger.kernel.org
> Cc: Maarten Maathuis 
> Tested-by: Maarten Maathuis 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=97877
> Signed-off-by: Ville Syrjälä 

Reviewed-by: Jim Bride 

> ---
>  drivers/gpu/drm/i915/intel_dp.c | 71 
> +++--
>  1 file changed, 40 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 5992093e1814..b0753b272101 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1108,6 +1108,44 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct 
> drm_dp_aux_msg *msg)
>   return ret;
>  }
>  
> +static enum port intel_aux_port(struct drm_i915_private *dev_priv,
> + enum port port)
> +{
> + const struct ddi_vbt_port_info *info =
> + _priv->vbt.ddi_port_info[port];
> + enum port aux_port;
> +
> + if (!info->alternate_aux_channel) {
> + DRM_DEBUG_KMS("using AUX %c for port %c (platform default)\n",
> +   port_name(port), port_name(port));
> + return port;
> + }
> +
> + switch (info->alternate_aux_channel) {
> + case DP_AUX_A:
> + aux_port = PORT_A;
> + break;
> + case DP_AUX_B:
> + aux_port = PORT_B;
> + break;
> + case DP_AUX_C:
> + aux_port = PORT_C;
> + break;
> + case DP_AUX_D:
> + aux_port = PORT_D;
> + break;
> + default:
> + MISSING_CASE(info->alternate_aux_channel);
> + aux_port = PORT_A;
> + break;
> + }
> +
> + DRM_DEBUG_KMS("using AUX %c for port %c (VBT)\n",
> +   port_name(aux_port), port_name(port));
> +
> + return aux_port;
> +}
> +
>  static i915_reg_t g4x_aux_ctl_reg(struct drm_i915_private *dev_priv,
>  enum port port)
>  {
> @@ -1168,36 +1206,9 @@ static i915_reg_t ilk_aux_data_reg(struct 
> drm_i915_private *dev_priv,
>   }
>  }
>  
> -/*
> - * On SKL we don't have Aux for port E so we rely
> - * on VBT to set a proper alternate aux channel.
> - */
> -static enum port skl_porte_aux_port(struct drm_i915_private *dev_priv)
> -{
> - const struct ddi_vbt_port_info *info =
> - _priv->vbt.ddi_port_info[PORT_E];
> -
> - switch (info->alternate_aux_channel) {
> - case DP_AUX_A:
> - return PORT_A;
> - case DP_AUX_B:
> - return PORT_B;
> - case DP_AUX_C:
> - return PORT_C;
> - case DP_AUX_D:
> - return PORT_D;
> - default:
> - MISSING_CASE(info->alternate_aux_channel);
> - return PORT_A;
> - }
> -}
> -
>  static i915_reg_t skl_aux_ctl_reg(struct drm_i915_private *dev_priv,
>  enum port port)
>  {
> - if (port == PORT_E)
> - port = skl_porte_aux_port(dev_priv);
> -
>   switch (port) {
>   case PORT_A:
>   case PORT_B:
> @@ -1213,9 +1224,6 @@ static i915_reg_t skl_aux_ctl_reg(struct 
> drm_i915_private *dev_priv,
>  static i915_reg_t skl_aux_data_reg(struct drm_i915_private *dev_priv,
>   enum port port, int index)
>  {
> - if (port == PORT_E)
> - port = skl_porte_aux_port(dev_priv);
> -
>   switch (port) {
>   case PORT_A:
>   case PORT_B:
> @@ -1253,7 +1261,8 @@ static i915_reg_t intel_aux_data_reg(struct 
> drm_i915_private *dev_priv,
>  static void intel_aux_reg_init(struct intel_dp *intel_dp)
>  {
>   struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
> - enum port port = dp_to_dig_port(intel_dp)->port;
> + enum port port = intel_aux_port(dev_priv,
> + dp_to_dig_port(intel_dp)->port);
>   int i;
>  
>   

[Intel-gfx] [PATCH 1/4] drm/i915: Respect alternate_aux_channel for all DDI ports

[ville . syrjala]

From: Ville Syrjälä 

The VBT provides the platform a way to mix and match the DDI ports vs.
AUX channels. Currently we only trust the VBT for DDI E, which has no
corresponding AUX channel of its own. However it is possible that some
board might use some non-standard DDI vs. AUX port routing even for
the other ports. Perhaps for signal routing reasons or something,
So let's generalize this and trust the VBT for all ports.

For now we'll limit this to DDI platforms, as we trust the VBT a bit
more there anyway when it comes to the DDI ports. I've structured
the code in a way that would allow us to easily expand this to
other platforms as well, by simply filling in the ddi_port_info.

v2: Drop whitespace changes, keep MISSING_CASE() for unknown
aux ch assignment, include a commit message, include debug
message during init

Cc: sta...@vger.kernel.org
Cc: Maarten Maathuis 
Tested-by: Maarten Maathuis 
References: https://bugs.freedesktop.org/show_bug.cgi?id=97877
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/intel_dp.c | 71 +++--
 1 file changed, 40 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 5992093e1814..b0753b272101 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1108,6 +1108,44 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct 
drm_dp_aux_msg *msg)
return ret;
 }
 
+static enum port intel_aux_port(struct drm_i915_private *dev_priv,
+   enum port port)
+{
+   const struct ddi_vbt_port_info *info =
+   _priv->vbt.ddi_port_info[port];
+   enum port aux_port;
+
+   if (!info->alternate_aux_channel) {
+   DRM_DEBUG_KMS("using AUX %c for port %c (platform default)\n",
+ port_name(port), port_name(port));
+   return port;
+   }
+
+   switch (info->alternate_aux_channel) {
+   case DP_AUX_A:
+   aux_port = PORT_A;
+   break;
+   case DP_AUX_B:
+   aux_port = PORT_B;
+   break;
+   case DP_AUX_C:
+   aux_port = PORT_C;
+   break;
+   case DP_AUX_D:
+   aux_port = PORT_D;
+   break;
+   default:
+   MISSING_CASE(info->alternate_aux_channel);
+   aux_port = PORT_A;
+   break;
+   }
+
+   DRM_DEBUG_KMS("using AUX %c for port %c (VBT)\n",
+ port_name(aux_port), port_name(port));
+
+   return aux_port;
+}
+
 static i915_reg_t g4x_aux_ctl_reg(struct drm_i915_private *dev_priv,
   enum port port)
 {
@@ -1168,36 +1206,9 @@ static i915_reg_t ilk_aux_data_reg(struct 
drm_i915_private *dev_priv,
}
 }
 
-/*
- * On SKL we don't have Aux for port E so we rely
- * on VBT to set a proper alternate aux channel.
- */
-static enum port skl_porte_aux_port(struct drm_i915_private *dev_priv)
-{
-   const struct ddi_vbt_port_info *info =
-   _priv->vbt.ddi_port_info[PORT_E];
-
-   switch (info->alternate_aux_channel) {
-   case DP_AUX_A:
-   return PORT_A;
-   case DP_AUX_B:
-   return PORT_B;
-   case DP_AUX_C:
-   return PORT_C;
-   case DP_AUX_D:
-   return PORT_D;
-   default:
-   MISSING_CASE(info->alternate_aux_channel);
-   return PORT_A;
-   }
-}
-
 static i915_reg_t skl_aux_ctl_reg(struct drm_i915_private *dev_priv,
   enum port port)
 {
-   if (port == PORT_E)
-   port = skl_porte_aux_port(dev_priv);
-
switch (port) {
case PORT_A:
case PORT_B:
@@ -1213,9 +1224,6 @@ static i915_reg_t skl_aux_ctl_reg(struct drm_i915_private 
*dev_priv,
 static i915_reg_t skl_aux_data_reg(struct drm_i915_private *dev_priv,
enum port port, int index)
 {
-   if (port == PORT_E)
-   port = skl_porte_aux_port(dev_priv);
-
switch (port) {
case PORT_A:
case PORT_B:
@@ -1253,7 +1261,8 @@ static i915_reg_t intel_aux_data_reg(struct 
drm_i915_private *dev_priv,
 static void intel_aux_reg_init(struct intel_dp *intel_dp)
 {
struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
-   enum port port = dp_to_dig_port(intel_dp)->port;
+   enum port port = intel_aux_port(dev_priv,
+   dp_to_dig_port(intel_dp)->port);
int i;
 
intel_dp->aux_ch_ctl_reg = intel_aux_ctl_reg(dev_priv, port);
-- 
2.7.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx