[PATCH 0/4] drm/i915: Trust VBT aux/ddc information

[PATCH 0/4] drm/i915: Trust VBT aux/ddc information

[ville.syrjala]

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

So we have a bug or two ([1] at least) about using an incorrect gmbus pin for
a HDMI connector. So I cooked up some patches to expand our use of VBT for this
information. So far I've kept it limited to DDI platforms, but I think I've seen
another bug about CHV using the incorrect gmbus pin as well, not 100% sure though.

I slapped cc:stable on them since they do fix an actual issue. My track record
with this sort of firmware information hasn't been great as of late, so some
fallout might occur.


Anyways here's the entire series:
git://github.com/vsyrjala/linux.git trust_vbt_ddc_aux_pins_2

[1] https://bugs.freedesktop.org/show_bug.cgi?id=97877

Cc: Maarten Maathuis <madman2003@gmail.com>

Ville Syrjälä (4):
  drm/i915: Respect alternate_aux_channel for all DDI ports
  drm/i915: Respect alternate_ddc_pin for all DDI ports
  drm/i915: Clean up DDI DDC/AUX CH sanitation
  drm/i915: Fix whitespace issues

 drivers/gpu/drm/i915/intel_bios.c | 116 +++++++++++++++++++++++---------------
 drivers/gpu/drm/i915/intel_dp.c   |  87 +++++++++++++++-------------
 drivers/gpu/drm/i915/intel_hdmi.c |  84 +++++++++++++++------------
 3 files changed, 167 insertions(+), 120 deletions(-)

-- 
2.7.4

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

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

[ville.syrjala]

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

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: stable@vger.kernel.org
Cc: Maarten Maathuis <madman2003@gmail.com>
Tested-by: Maarten Maathuis <madman2003@gmail.com>
References: https://bugs.freedesktop.org/show_bug.cgi?id=97877
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 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 =
+		&dev_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 =
-		&dev_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

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

[ville.syrjala]

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

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: stable@vger.kernel.org
Cc: Maarten Maathuis <madman2003@gmail.com>
Tested-by: Maarten Maathuis <madman2003@gmail.com>
References: https://bugs.freedesktop.org/show_bug.cgi?id=97877
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 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 =
+		&dev_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 =
-		&dev_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

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.syrjala@linux.intel.com wrote:
> From: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> 
> 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: stable@vger.kernel.org
> Cc: Maarten Maathuis <madman2003@gmail.com>
> Tested-by: Maarten Maathuis <madman2003@gmail.com>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=97877
> Signed-off-by: Ville Syrj�l� <ville.syrjala@linux.intel.com>

Reviewed-by: Jim Bride <jim.bride@linux.intel.com>

> ---
>  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 =
> +		&dev_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 =
> -		&dev_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

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.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> 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: stable@vger.kernel.org
> Cc: Maarten Maathuis <madman2003@gmail.com>
> Tested-by: Maarten Maathuis <madman2003@gmail.com>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=97877
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Jim Bride <jim.bride@linux.intel.com>

> ---
>  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 =
> +		&dev_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 =
> -		&dev_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

[PATCH 2/4] drm/i915: Respect alternate_ddc_pin for all DDI ports

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

The VBT provides the platform a way to mix and match the DDI ports vs.
GMBUS pins. Currently we only trust the VBT for DDI E, which I suppose
has no standard GMBUS pin assignment. However, there are machines out
there that use a non-standard mapping for the other ports as well.
Let's start trusting the VBT on this one for all ports on DDI platforms.

I've structured the code such that other platforms could easily start
using this as well, by simply filling in the ddi_port_info. IIRC there
may be CHV system that might actually need this.

v2: Include a commit message, include a debug message during init

Cc: stable@vger.kernel.org
Cc: Maarten Maathuis <madman2003@gmail.com>
Tested-by: Maarten Maatt show huis <madman2003@gmail.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97877
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_hdmi.c | 84 ++++++++++++++++++++++-----------------
 1 file changed, 48 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 8d46f5836746..9ca86e901fc8 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1799,6 +1799,50 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c
 	intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
 }
 
+static u8 intel_hdmi_ddc_pin(struct drm_i915_private *dev_priv,
+			     enum port port)
+{
+	const struct ddi_vbt_port_info *info =
+		&dev_priv->vbt.ddi_port_info[port];
+	u8 ddc_pin;
+
+	if (info->alternate_ddc_pin) {
+		DRM_DEBUG_KMS("Using DDC pin 0x%x for port %c (VBT)\n",
+			      info->alternate_ddc_pin, port_name(port));
+		return info->alternate_ddc_pin;
+	}
+
+	switch (port) {
+	case PORT_B:
+		if (IS_BROXTON(dev_priv))
+			ddc_pin = GMBUS_PIN_1_BXT;
+		else
+			ddc_pin = GMBUS_PIN_DPB;
+		break;
+	case PORT_C:
+		if (IS_BROXTON(dev_priv))
+			ddc_pin = GMBUS_PIN_2_BXT;
+		else
+			ddc_pin = GMBUS_PIN_DPC;
+		break;
+	case PORT_D:
+		if (IS_CHERRYVIEW(dev_priv))
+			ddc_pin = GMBUS_PIN_DPD_CHV;
+		else
+			ddc_pin = GMBUS_PIN_DPD;
+		break;
+	default:
+		MISSING_CASE(port);
+		ddc_pin = GMBUS_PIN_DPB;
+		break;
+	}
+
+	DRM_DEBUG_KMS("Using DDC pin 0x%x for port %c (platform default)\n",
+		      ddc_pin, port_name(port));
+
+	return ddc_pin;
+}
+
 void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
 			       struct intel_connector *intel_connector)
 {
@@ -1808,7 +1852,6 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
 	struct drm_device *dev = intel_encoder->base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	enum port port = intel_dig_port->port;
-	uint8_t alternate_ddc_pin;
 
 	DRM_DEBUG_KMS("Adding HDMI connector on port %c\n",
 		      port_name(port));
@@ -1826,12 +1869,10 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
 	connector->doublescan_allowed = 0;
 	connector->stereo_allowed = 1;
 
+	intel_hdmi->ddc_bus = intel_hdmi_ddc_pin(dev_priv, port);
+
 	switch (port) {
 	case PORT_B:
-		if (IS_BROXTON(dev_priv))
-			intel_hdmi->ddc_bus = GMBUS_PIN_1_BXT;
-		else
-			intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
 		/*
 		 * On BXT A0/A1, sw needs to activate DDIA HPD logic and
 		 * interrupts to check the external panel connection.
@@ -1842,46 +1883,17 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
 			intel_encoder->hpd_pin = HPD_PORT_B;
 		break;
 	case PORT_C:
-		if (IS_BROXTON(dev_priv))
-			intel_hdmi->ddc_bus = GMBUS_PIN_2_BXT;
-		else
-			intel_hdmi->ddc_bus = GMBUS_PIN_DPC;
 		intel_encoder->hpd_pin = HPD_PORT_C;
 		break;
 	case PORT_D:
-		if (WARN_ON(IS_BROXTON(dev_priv)))
-			intel_hdmi->ddc_bus = GMBUS_PIN_DISABLED;
-		else if (IS_CHERRYVIEW(dev_priv))
-			intel_hdmi->ddc_bus = GMBUS_PIN_DPD_CHV;
-		else
-			intel_hdmi->ddc_bus = GMBUS_PIN_DPD;
 		intel_encoder->hpd_pin = HPD_PORT_D;
 		break;
 	case PORT_E:
-		/* On SKL PORT E doesn't have seperate GMBUS pin
-		 *  We rely on VBT to set a proper alternate GMBUS pin. */
-		alternate_ddc_pin =
-			dev_priv->vbt.ddi_port_info[PORT_E].alternate_ddc_pin;
-		switch (alternate_ddc_pin) {
-		case DDC_PIN_B:
-			intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
-			break;
-		case DDC_PIN_C:
-			intel_hdmi->ddc_bus = GMBUS_PIN_DPC;
-			break;
-		case DDC_PIN_D:
-			intel_hdmi->ddc_bus = GMBUS_PIN_DPD;
-			break;
-		default:
-			MISSING_CASE(alternate_ddc_pin);
-		}
 		intel_encoder->hpd_pin = HPD_PORT_E;
 		break;
-	case PORT_A:
-		intel_encoder->hpd_pin = HPD_PORT_A;
-		/* Internal port only for eDP. */
 	default:
-		BUG();
+		MISSING_CASE(port);
+		return;
 	}
 
 	if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
-- 
2.7.4

[PATCH 2/4] drm/i915: Respect alternate_ddc_pin for all DDI ports

[ville.syrjala]

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

The VBT provides the platform a way to mix and match the DDI ports vs.
GMBUS pins. Currently we only trust the VBT for DDI E, which I suppose
has no standard GMBUS pin assignment. However, there are machines out
there that use a non-standard mapping for the other ports as well.
Let's start trusting the VBT on this one for all ports on DDI platforms.

I've structured the code such that other platforms could easily start
using this as well, by simply filling in the ddi_port_info. IIRC there
may be CHV system that might actually need this.

v2: Include a commit message, include a debug message during init

Cc: stable@vger.kernel.org
Cc: Maarten Maathuis <madman2003@gmail.com>
Tested-by: Maarten Maatt show huis <madman2003@gmail.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97877
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_hdmi.c | 84 ++++++++++++++++++++++-----------------
 1 file changed, 48 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 8d46f5836746..9ca86e901fc8 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1799,6 +1799,50 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c
 	intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
 }
 
+static u8 intel_hdmi_ddc_pin(struct drm_i915_private *dev_priv,
+			     enum port port)
+{
+	const struct ddi_vbt_port_info *info =
+		&dev_priv->vbt.ddi_port_info[port];
+	u8 ddc_pin;
+
+	if (info->alternate_ddc_pin) {
+		DRM_DEBUG_KMS("Using DDC pin 0x%x for port %c (VBT)\n",
+			      info->alternate_ddc_pin, port_name(port));
+		return info->alternate_ddc_pin;
+	}
+
+	switch (port) {
+	case PORT_B:
+		if (IS_BROXTON(dev_priv))
+			ddc_pin = GMBUS_PIN_1_BXT;
+		else
+			ddc_pin = GMBUS_PIN_DPB;
+		break;
+	case PORT_C:
+		if (IS_BROXTON(dev_priv))
+			ddc_pin = GMBUS_PIN_2_BXT;
+		else
+			ddc_pin = GMBUS_PIN_DPC;
+		break;
+	case PORT_D:
+		if (IS_CHERRYVIEW(dev_priv))
+			ddc_pin = GMBUS_PIN_DPD_CHV;
+		else
+			ddc_pin = GMBUS_PIN_DPD;
+		break;
+	default:
+		MISSING_CASE(port);
+		ddc_pin = GMBUS_PIN_DPB;
+		break;
+	}
+
+	DRM_DEBUG_KMS("Using DDC pin 0x%x for port %c (platform default)\n",
+		      ddc_pin, port_name(port));
+
+	return ddc_pin;
+}
+
 void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
 			       struct intel_connector *intel_connector)
 {
@@ -1808,7 +1852,6 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
 	struct drm_device *dev = intel_encoder->base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	enum port port = intel_dig_port->port;
-	uint8_t alternate_ddc_pin;
 
 	DRM_DEBUG_KMS("Adding HDMI connector on port %c\n",
 		      port_name(port));
@@ -1826,12 +1869,10 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
 	connector->doublescan_allowed = 0;
 	connector->stereo_allowed = 1;
 
+	intel_hdmi->ddc_bus = intel_hdmi_ddc_pin(dev_priv, port);
+
 	switch (port) {
 	case PORT_B:
-		if (IS_BROXTON(dev_priv))
-			intel_hdmi->ddc_bus = GMBUS_PIN_1_BXT;
-		else
-			intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
 		/*
 		 * On BXT A0/A1, sw needs to activate DDIA HPD logic and
 		 * interrupts to check the external panel connection.
@@ -1842,46 +1883,17 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
 			intel_encoder->hpd_pin = HPD_PORT_B;
 		break;
 	case PORT_C:
-		if (IS_BROXTON(dev_priv))
-			intel_hdmi->ddc_bus = GMBUS_PIN_2_BXT;
-		else
-			intel_hdmi->ddc_bus = GMBUS_PIN_DPC;
 		intel_encoder->hpd_pin = HPD_PORT_C;
 		break;
 	case PORT_D:
-		if (WARN_ON(IS_BROXTON(dev_priv)))
-			intel_hdmi->ddc_bus = GMBUS_PIN_DISABLED;
-		else if (IS_CHERRYVIEW(dev_priv))
-			intel_hdmi->ddc_bus = GMBUS_PIN_DPD_CHV;
-		else
-			intel_hdmi->ddc_bus = GMBUS_PIN_DPD;
 		intel_encoder->hpd_pin = HPD_PORT_D;
 		break;
 	case PORT_E:
-		/* On SKL PORT E doesn't have seperate GMBUS pin
-		 *  We rely on VBT to set a proper alternate GMBUS pin. */
-		alternate_ddc_pin =
-			dev_priv->vbt.ddi_port_info[PORT_E].alternate_ddc_pin;
-		switch (alternate_ddc_pin) {
-		case DDC_PIN_B:
-			intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
-			break;
-		case DDC_PIN_C:
-			intel_hdmi->ddc_bus = GMBUS_PIN_DPC;
-			break;
-		case DDC_PIN_D:
-			intel_hdmi->ddc_bus = GMBUS_PIN_DPD;
-			break;
-		default:
-			MISSING_CASE(alternate_ddc_pin);
-		}
 		intel_encoder->hpd_pin = HPD_PORT_E;
 		break;
-	case PORT_A:
-		intel_encoder->hpd_pin = HPD_PORT_A;
-		/* Internal port only for eDP. */
 	default:
-		BUG();
+		MISSING_CASE(port);
+		return;
 	}
 
 	if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
-- 
2.7.4

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

Re: [PATCH 2/4] drm/i915: Respect alternate_ddc_pin for all DDI ports

[Maarten Maathuis]

[-- Attachment #1.1: Type: text/plain, Size: 6837 bytes --]

My name does not include the word "show" (Tested-by tag).

On Tue, Oct 11, 2016 at 7:52 PM, <ville.syrjala@linux.intel.com> wrote:

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The VBT provides the platform a way to mix and match the DDI ports vs.
> GMBUS pins. Currently we only trust the VBT for DDI E, which I suppose
> has no standard GMBUS pin assignment. However, there are machines out
> there that use a non-standard mapping for the other ports as well.
> Let's start trusting the VBT on this one for all ports on DDI platforms.
>
> I've structured the code such that other platforms could easily start
> using this as well, by simply filling in the ddi_port_info. IIRC there
> may be CHV system that might actually need this.
>
> v2: Include a commit message, include a debug message during init
>
> Cc: stable@vger.kernel.org
> Cc: Maarten Maathuis <madman2003@gmail.com>
> Tested-by: Maarten Maatt show huis <madman2003@gmail.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97877
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_hdmi.c | 84 ++++++++++++++++++++++--------
> ---------
>  1 file changed, 48 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> b/drivers/gpu/drm/i915/intel_hdmi.c
> index 8d46f5836746..9ca86e901fc8 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1799,6 +1799,50 @@ intel_hdmi_add_properties(struct intel_hdmi
> *intel_hdmi, struct drm_connector *c
>         intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
>  }
>
> +static u8 intel_hdmi_ddc_pin(struct drm_i915_private *dev_priv,
> +                            enum port port)
> +{
> +       const struct ddi_vbt_port_info *info =
> +               &dev_priv->vbt.ddi_port_info[port];
> +       u8 ddc_pin;
> +
> +       if (info->alternate_ddc_pin) {
> +               DRM_DEBUG_KMS("Using DDC pin 0x%x for port %c (VBT)\n",
> +                             info->alternate_ddc_pin, port_name(port));
> +               return info->alternate_ddc_pin;
> +       }
> +
> +       switch (port) {
> +       case PORT_B:
> +               if (IS_BROXTON(dev_priv))
> +                       ddc_pin = GMBUS_PIN_1_BXT;
> +               else
> +                       ddc_pin = GMBUS_PIN_DPB;
> +               break;
> +       case PORT_C:
> +               if (IS_BROXTON(dev_priv))
> +                       ddc_pin = GMBUS_PIN_2_BXT;
> +               else
> +                       ddc_pin = GMBUS_PIN_DPC;
> +               break;
> +       case PORT_D:
> +               if (IS_CHERRYVIEW(dev_priv))
> +                       ddc_pin = GMBUS_PIN_DPD_CHV;
> +               else
> +                       ddc_pin = GMBUS_PIN_DPD;
> +               break;
> +       default:
> +               MISSING_CASE(port);
> +               ddc_pin = GMBUS_PIN_DPB;
> +               break;
> +       }
> +
> +       DRM_DEBUG_KMS("Using DDC pin 0x%x for port %c (platform
> default)\n",
> +                     ddc_pin, port_name(port));
> +
> +       return ddc_pin;
> +}
> +
>  void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
>                                struct intel_connector *intel_connector)
>  {
> @@ -1808,7 +1852,6 @@ void intel_hdmi_init_connector(struct
> intel_digital_port *intel_dig_port,
>         struct drm_device *dev = intel_encoder->base.dev;
>         struct drm_i915_private *dev_priv = to_i915(dev);
>         enum port port = intel_dig_port->port;
> -       uint8_t alternate_ddc_pin;
>
>         DRM_DEBUG_KMS("Adding HDMI connector on port %c\n",
>                       port_name(port));
> @@ -1826,12 +1869,10 @@ void intel_hdmi_init_connector(struct
> intel_digital_port *intel_dig_port,
>         connector->doublescan_allowed = 0;
>         connector->stereo_allowed = 1;
>
> +       intel_hdmi->ddc_bus = intel_hdmi_ddc_pin(dev_priv, port);
> +
>         switch (port) {
>         case PORT_B:
> -               if (IS_BROXTON(dev_priv))
> -                       intel_hdmi->ddc_bus = GMBUS_PIN_1_BXT;
> -               else
> -                       intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
>                 /*
>                  * On BXT A0/A1, sw needs to activate DDIA HPD logic and
>                  * interrupts to check the external panel connection.
> @@ -1842,46 +1883,17 @@ void intel_hdmi_init_connector(struct
> intel_digital_port *intel_dig_port,
>                         intel_encoder->hpd_pin = HPD_PORT_B;
>                 break;
>         case PORT_C:
> -               if (IS_BROXTON(dev_priv))
> -                       intel_hdmi->ddc_bus = GMBUS_PIN_2_BXT;
> -               else
> -                       intel_hdmi->ddc_bus = GMBUS_PIN_DPC;
>                 intel_encoder->hpd_pin = HPD_PORT_C;
>                 break;
>         case PORT_D:
> -               if (WARN_ON(IS_BROXTON(dev_priv)))
> -                       intel_hdmi->ddc_bus = GMBUS_PIN_DISABLED;
> -               else if (IS_CHERRYVIEW(dev_priv))
> -                       intel_hdmi->ddc_bus = GMBUS_PIN_DPD_CHV;
> -               else
> -                       intel_hdmi->ddc_bus = GMBUS_PIN_DPD;
>                 intel_encoder->hpd_pin = HPD_PORT_D;
>                 break;
>         case PORT_E:
> -               /* On SKL PORT E doesn't have seperate GMBUS pin
> -                *  We rely on VBT to set a proper alternate GMBUS pin. */
> -               alternate_ddc_pin =
> -                       dev_priv->vbt.ddi_port_info[
> PORT_E].alternate_ddc_pin;
> -               switch (alternate_ddc_pin) {
> -               case DDC_PIN_B:
> -                       intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
> -                       break;
> -               case DDC_PIN_C:
> -                       intel_hdmi->ddc_bus = GMBUS_PIN_DPC;
> -                       break;
> -               case DDC_PIN_D:
> -                       intel_hdmi->ddc_bus = GMBUS_PIN_DPD;
> -                       break;
> -               default:
> -                       MISSING_CASE(alternate_ddc_pin);
> -               }
>                 intel_encoder->hpd_pin = HPD_PORT_E;
>                 break;
> -       case PORT_A:
> -               intel_encoder->hpd_pin = HPD_PORT_A;
> -               /* Internal port only for eDP. */
>         default:
> -               BUG();
> +               MISSING_CASE(port);
> +               return;
>         }
>
>         if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
> --
> 2.7.4
>
>


-- 
Far away from the primal instinct, the song seems to fade away, the river
get wider between your thoughts and the things we do and say.

[-- Attachment #1.2: Type: text/html, Size: 8981 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH 2/4] drm/i915: Respect alternate_ddc_pin for all DDI ports

[Ville Syrjälä]

On Tue, Oct 11, 2016 at 10:04:00PM +0200, Maarten Maathuis wrote:
> My name does not include the word "show" (Tested-by tag).

Sorry about that. Some copy-paste fail I suspect. I'll fix it up.

And you actually tested the v1 patches, so I totally forgot to note that
in the tested-by tags :( Care to re-test these v2 versions, just to make
sure I didn't seriously fumble anything?

> 
> On Tue, Oct 11, 2016 at 7:52 PM, <ville.syrjala@linux.intel.com> wrote:
> 
> > From: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> >
> > The VBT provides the platform a way to mix and match the DDI ports vs.
> > GMBUS pins. Currently we only trust the VBT for DDI E, which I suppose
> > has no standard GMBUS pin assignment. However, there are machines out
> > there that use a non-standard mapping for the other ports as well.
> > Let's start trusting the VBT on this one for all ports on DDI platforms.
> >
> > I've structured the code such that other platforms could easily start
> > using this as well, by simply filling in the ddi_port_info. IIRC there
> > may be CHV system that might actually need this.
> >
> > v2: Include a commit message, include a debug message during init
> >
> > Cc: stable@vger.kernel.org
> > Cc: Maarten Maathuis <madman2003@gmail.com>
> > Tested-by: Maarten Maatt show huis <madman2003@gmail.com>
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97877
> > Signed-off-by: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_hdmi.c | 84 ++++++++++++++++++++++--------
> > ---------
> >  1 file changed, 48 insertions(+), 36 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> > b/drivers/gpu/drm/i915/intel_hdmi.c
> > index 8d46f5836746..9ca86e901fc8 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -1799,6 +1799,50 @@ intel_hdmi_add_properties(struct intel_hdmi
> > *intel_hdmi, struct drm_connector *c
> >         intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
> >  }
> >
> > +static u8 intel_hdmi_ddc_pin(struct drm_i915_private *dev_priv,
> > +                            enum port port)
> > +{
> > +       const struct ddi_vbt_port_info *info =
> > +               &dev_priv->vbt.ddi_port_info[port];
> > +       u8 ddc_pin;
> > +
> > +       if (info->alternate_ddc_pin) {
> > +               DRM_DEBUG_KMS("Using DDC pin 0x%x for port %c (VBT)\n",
> > +                             info->alternate_ddc_pin, port_name(port));
> > +               return info->alternate_ddc_pin;
> > +       }
> > +
> > +       switch (port) {
> > +       case PORT_B:
> > +               if (IS_BROXTON(dev_priv))
> > +                       ddc_pin = GMBUS_PIN_1_BXT;
> > +               else
> > +                       ddc_pin = GMBUS_PIN_DPB;
> > +               break;
> > +       case PORT_C:
> > +               if (IS_BROXTON(dev_priv))
> > +                       ddc_pin = GMBUS_PIN_2_BXT;
> > +               else
> > +                       ddc_pin = GMBUS_PIN_DPC;
> > +               break;
> > +       case PORT_D:
> > +               if (IS_CHERRYVIEW(dev_priv))
> > +                       ddc_pin = GMBUS_PIN_DPD_CHV;
> > +               else
> > +                       ddc_pin = GMBUS_PIN_DPD;
> > +               break;
> > +       default:
> > +               MISSING_CASE(port);
> > +               ddc_pin = GMBUS_PIN_DPB;
> > +               break;
> > +       }
> > +
> > +       DRM_DEBUG_KMS("Using DDC pin 0x%x for port %c (platform
> > default)\n",
> > +                     ddc_pin, port_name(port));
> > +
> > +       return ddc_pin;
> > +}
> > +
> >  void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
> >                                struct intel_connector *intel_connector)
> >  {
> > @@ -1808,7 +1852,6 @@ void intel_hdmi_init_connector(struct
> > intel_digital_port *intel_dig_port,
> >         struct drm_device *dev = intel_encoder->base.dev;
> >         struct drm_i915_private *dev_priv = to_i915(dev);
> >         enum port port = intel_dig_port->port;
> > -       uint8_t alternate_ddc_pin;
> >
> >         DRM_DEBUG_KMS("Adding HDMI connector on port %c\n",
> >                       port_name(port));
> > @@ -1826,12 +1869,10 @@ void intel_hdmi_init_connector(struct
> > intel_digital_port *intel_dig_port,
> >         connector->doublescan_allowed = 0;
> >         connector->stereo_allowed = 1;
> >
> > +       intel_hdmi->ddc_bus = intel_hdmi_ddc_pin(dev_priv, port);
> > +
> >         switch (port) {
> >         case PORT_B:
> > -               if (IS_BROXTON(dev_priv))
> > -                       intel_hdmi->ddc_bus = GMBUS_PIN_1_BXT;
> > -               else
> > -                       intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
> >                 /*
> >                  * On BXT A0/A1, sw needs to activate DDIA HPD logic and
> >                  * interrupts to check the external panel connection.
> > @@ -1842,46 +1883,17 @@ void intel_hdmi_init_connector(struct
> > intel_digital_port *intel_dig_port,
> >                         intel_encoder->hpd_pin = HPD_PORT_B;
> >                 break;
> >         case PORT_C:
> > -               if (IS_BROXTON(dev_priv))
> > -                       intel_hdmi->ddc_bus = GMBUS_PIN_2_BXT;
> > -               else
> > -                       intel_hdmi->ddc_bus = GMBUS_PIN_DPC;
> >                 intel_encoder->hpd_pin = HPD_PORT_C;
> >                 break;
> >         case PORT_D:
> > -               if (WARN_ON(IS_BROXTON(dev_priv)))
> > -                       intel_hdmi->ddc_bus = GMBUS_PIN_DISABLED;
> > -               else if (IS_CHERRYVIEW(dev_priv))
> > -                       intel_hdmi->ddc_bus = GMBUS_PIN_DPD_CHV;
> > -               else
> > -                       intel_hdmi->ddc_bus = GMBUS_PIN_DPD;
> >                 intel_encoder->hpd_pin = HPD_PORT_D;
> >                 break;
> >         case PORT_E:
> > -               /* On SKL PORT E doesn't have seperate GMBUS pin
> > -                *  We rely on VBT to set a proper alternate GMBUS pin. */
> > -               alternate_ddc_pin =
> > -                       dev_priv->vbt.ddi_port_info[
> > PORT_E].alternate_ddc_pin;
> > -               switch (alternate_ddc_pin) {
> > -               case DDC_PIN_B:
> > -                       intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
> > -                       break;
> > -               case DDC_PIN_C:
> > -                       intel_hdmi->ddc_bus = GMBUS_PIN_DPC;
> > -                       break;
> > -               case DDC_PIN_D:
> > -                       intel_hdmi->ddc_bus = GMBUS_PIN_DPD;
> > -                       break;
> > -               default:
> > -                       MISSING_CASE(alternate_ddc_pin);
> > -               }
> >                 intel_encoder->hpd_pin = HPD_PORT_E;
> >                 break;
> > -       case PORT_A:
> > -               intel_encoder->hpd_pin = HPD_PORT_A;
> > -               /* Internal port only for eDP. */
> >         default:
> > -               BUG();
> > +               MISSING_CASE(port);
> > +               return;
> >         }
> >
> >         if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
> > --
> > 2.7.4
> >
> >
> 
> 
> -- 
> Far away from the primal instinct, the song seems to fade away, the river
> get wider between your thoughts and the things we do and say.

-- 
Ville Syrj�l�
Intel OTC

Re: [PATCH 2/4] drm/i915: Respect alternate_ddc_pin for all DDI ports

[Ville Syrjälä]

On Tue, Oct 11, 2016 at 10:04:00PM +0200, Maarten Maathuis wrote:
> My name does not include the word "show" (Tested-by tag).

Sorry about that. Some copy-paste fail I suspect. I'll fix it up.

And you actually tested the v1 patches, so I totally forgot to note that
in the tested-by tags :( Care to re-test these v2 versions, just to make
sure I didn't seriously fumble anything?

> 
> On Tue, Oct 11, 2016 at 7:52 PM, <ville.syrjala@linux.intel.com> wrote:
> 
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > The VBT provides the platform a way to mix and match the DDI ports vs.
> > GMBUS pins. Currently we only trust the VBT for DDI E, which I suppose
> > has no standard GMBUS pin assignment. However, there are machines out
> > there that use a non-standard mapping for the other ports as well.
> > Let's start trusting the VBT on this one for all ports on DDI platforms.
> >
> > I've structured the code such that other platforms could easily start
> > using this as well, by simply filling in the ddi_port_info. IIRC there
> > may be CHV system that might actually need this.
> >
> > v2: Include a commit message, include a debug message during init
> >
> > Cc: stable@vger.kernel.org
> > Cc: Maarten Maathuis <madman2003@gmail.com>
> > Tested-by: Maarten Maatt show huis <madman2003@gmail.com>
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97877
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_hdmi.c | 84 ++++++++++++++++++++++--------
> > ---------
> >  1 file changed, 48 insertions(+), 36 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> > b/drivers/gpu/drm/i915/intel_hdmi.c
> > index 8d46f5836746..9ca86e901fc8 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -1799,6 +1799,50 @@ intel_hdmi_add_properties(struct intel_hdmi
> > *intel_hdmi, struct drm_connector *c
> >         intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
> >  }
> >
> > +static u8 intel_hdmi_ddc_pin(struct drm_i915_private *dev_priv,
> > +                            enum port port)
> > +{
> > +       const struct ddi_vbt_port_info *info =
> > +               &dev_priv->vbt.ddi_port_info[port];
> > +       u8 ddc_pin;
> > +
> > +       if (info->alternate_ddc_pin) {
> > +               DRM_DEBUG_KMS("Using DDC pin 0x%x for port %c (VBT)\n",
> > +                             info->alternate_ddc_pin, port_name(port));
> > +               return info->alternate_ddc_pin;
> > +       }
> > +
> > +       switch (port) {
> > +       case PORT_B:
> > +               if (IS_BROXTON(dev_priv))
> > +                       ddc_pin = GMBUS_PIN_1_BXT;
> > +               else
> > +                       ddc_pin = GMBUS_PIN_DPB;
> > +               break;
> > +       case PORT_C:
> > +               if (IS_BROXTON(dev_priv))
> > +                       ddc_pin = GMBUS_PIN_2_BXT;
> > +               else
> > +                       ddc_pin = GMBUS_PIN_DPC;
> > +               break;
> > +       case PORT_D:
> > +               if (IS_CHERRYVIEW(dev_priv))
> > +                       ddc_pin = GMBUS_PIN_DPD_CHV;
> > +               else
> > +                       ddc_pin = GMBUS_PIN_DPD;
> > +               break;
> > +       default:
> > +               MISSING_CASE(port);
> > +               ddc_pin = GMBUS_PIN_DPB;
> > +               break;
> > +       }
> > +
> > +       DRM_DEBUG_KMS("Using DDC pin 0x%x for port %c (platform
> > default)\n",
> > +                     ddc_pin, port_name(port));
> > +
> > +       return ddc_pin;
> > +}
> > +
> >  void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
> >                                struct intel_connector *intel_connector)
> >  {
> > @@ -1808,7 +1852,6 @@ void intel_hdmi_init_connector(struct
> > intel_digital_port *intel_dig_port,
> >         struct drm_device *dev = intel_encoder->base.dev;
> >         struct drm_i915_private *dev_priv = to_i915(dev);
> >         enum port port = intel_dig_port->port;
> > -       uint8_t alternate_ddc_pin;
> >
> >         DRM_DEBUG_KMS("Adding HDMI connector on port %c\n",
> >                       port_name(port));
> > @@ -1826,12 +1869,10 @@ void intel_hdmi_init_connector(struct
> > intel_digital_port *intel_dig_port,
> >         connector->doublescan_allowed = 0;
> >         connector->stereo_allowed = 1;
> >
> > +       intel_hdmi->ddc_bus = intel_hdmi_ddc_pin(dev_priv, port);
> > +
> >         switch (port) {
> >         case PORT_B:
> > -               if (IS_BROXTON(dev_priv))
> > -                       intel_hdmi->ddc_bus = GMBUS_PIN_1_BXT;
> > -               else
> > -                       intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
> >                 /*
> >                  * On BXT A0/A1, sw needs to activate DDIA HPD logic and
> >                  * interrupts to check the external panel connection.
> > @@ -1842,46 +1883,17 @@ void intel_hdmi_init_connector(struct
> > intel_digital_port *intel_dig_port,
> >                         intel_encoder->hpd_pin = HPD_PORT_B;
> >                 break;
> >         case PORT_C:
> > -               if (IS_BROXTON(dev_priv))
> > -                       intel_hdmi->ddc_bus = GMBUS_PIN_2_BXT;
> > -               else
> > -                       intel_hdmi->ddc_bus = GMBUS_PIN_DPC;
> >                 intel_encoder->hpd_pin = HPD_PORT_C;
> >                 break;
> >         case PORT_D:
> > -               if (WARN_ON(IS_BROXTON(dev_priv)))
> > -                       intel_hdmi->ddc_bus = GMBUS_PIN_DISABLED;
> > -               else if (IS_CHERRYVIEW(dev_priv))
> > -                       intel_hdmi->ddc_bus = GMBUS_PIN_DPD_CHV;
> > -               else
> > -                       intel_hdmi->ddc_bus = GMBUS_PIN_DPD;
> >                 intel_encoder->hpd_pin = HPD_PORT_D;
> >                 break;
> >         case PORT_E:
> > -               /* On SKL PORT E doesn't have seperate GMBUS pin
> > -                *  We rely on VBT to set a proper alternate GMBUS pin. */
> > -               alternate_ddc_pin =
> > -                       dev_priv->vbt.ddi_port_info[
> > PORT_E].alternate_ddc_pin;
> > -               switch (alternate_ddc_pin) {
> > -               case DDC_PIN_B:
> > -                       intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
> > -                       break;
> > -               case DDC_PIN_C:
> > -                       intel_hdmi->ddc_bus = GMBUS_PIN_DPC;
> > -                       break;
> > -               case DDC_PIN_D:
> > -                       intel_hdmi->ddc_bus = GMBUS_PIN_DPD;
> > -                       break;
> > -               default:
> > -                       MISSING_CASE(alternate_ddc_pin);
> > -               }
> >                 intel_encoder->hpd_pin = HPD_PORT_E;
> >                 break;
> > -       case PORT_A:
> > -               intel_encoder->hpd_pin = HPD_PORT_A;
> > -               /* Internal port only for eDP. */
> >         default:
> > -               BUG();
> > +               MISSING_CASE(port);
> > +               return;
> >         }
> >
> >         if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
> > --
> > 2.7.4
> >
> >
> 
> 
> -- 
> Far away from the primal instinct, the song seems to fade away, the river
> get wider between your thoughts and the things we do and say.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/4] drm/i915: Respect alternate_ddc_pin for all DDI ports

[Maarten Maathuis]

[-- Attachment #1.1: Type: text/plain, Size: 8208 bytes --]

Retested the _2 branch, works fine as well.

On Wed, Oct 12, 2016 at 12:57 PM, Ville Syrjälä <
ville.syrjala@linux.intel.com> wrote:

> On Tue, Oct 11, 2016 at 10:04:00PM +0200, Maarten Maathuis wrote:
> > My name does not include the word "show" (Tested-by tag).
>
> Sorry about that. Some copy-paste fail I suspect. I'll fix it up.
>
> And you actually tested the v1 patches, so I totally forgot to note that
> in the tested-by tags :( Care to re-test these v2 versions, just to make
> sure I didn't seriously fumble anything?
>
> >
> > On Tue, Oct 11, 2016 at 7:52 PM, <ville.syrjala@linux.intel.com> wrote:
> >
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > The VBT provides the platform a way to mix and match the DDI ports vs.
> > > GMBUS pins. Currently we only trust the VBT for DDI E, which I suppose
> > > has no standard GMBUS pin assignment. However, there are machines out
> > > there that use a non-standard mapping for the other ports as well.
> > > Let's start trusting the VBT on this one for all ports on DDI
> platforms.
> > >
> > > I've structured the code such that other platforms could easily start
> > > using this as well, by simply filling in the ddi_port_info. IIRC there
> > > may be CHV system that might actually need this.
> > >
> > > v2: Include a commit message, include a debug message during init
> > >
> > > Cc: stable@vger.kernel.org
> > > Cc: Maarten Maathuis <madman2003@gmail.com>
> > > Tested-by: Maarten Maatt show huis <madman2003@gmail.com>
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97877
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_hdmi.c | 84 ++++++++++++++++++++++--------
> > > ---------
> > >  1 file changed, 48 insertions(+), 36 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> > > b/drivers/gpu/drm/i915/intel_hdmi.c
> > > index 8d46f5836746..9ca86e901fc8 100644
> > > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > > @@ -1799,6 +1799,50 @@ intel_hdmi_add_properties(struct intel_hdmi
> > > *intel_hdmi, struct drm_connector *c
> > >         intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
> > >  }
> > >
> > > +static u8 intel_hdmi_ddc_pin(struct drm_i915_private *dev_priv,
> > > +                            enum port port)
> > > +{
> > > +       const struct ddi_vbt_port_info *info =
> > > +               &dev_priv->vbt.ddi_port_info[port];
> > > +       u8 ddc_pin;
> > > +
> > > +       if (info->alternate_ddc_pin) {
> > > +               DRM_DEBUG_KMS("Using DDC pin 0x%x for port %c (VBT)\n",
> > > +                             info->alternate_ddc_pin,
> port_name(port));
> > > +               return info->alternate_ddc_pin;
> > > +       }
> > > +
> > > +       switch (port) {
> > > +       case PORT_B:
> > > +               if (IS_BROXTON(dev_priv))
> > > +                       ddc_pin = GMBUS_PIN_1_BXT;
> > > +               else
> > > +                       ddc_pin = GMBUS_PIN_DPB;
> > > +               break;
> > > +       case PORT_C:
> > > +               if (IS_BROXTON(dev_priv))
> > > +                       ddc_pin = GMBUS_PIN_2_BXT;
> > > +               else
> > > +                       ddc_pin = GMBUS_PIN_DPC;
> > > +               break;
> > > +       case PORT_D:
> > > +               if (IS_CHERRYVIEW(dev_priv))
> > > +                       ddc_pin = GMBUS_PIN_DPD_CHV;
> > > +               else
> > > +                       ddc_pin = GMBUS_PIN_DPD;
> > > +               break;
> > > +       default:
> > > +               MISSING_CASE(port);
> > > +               ddc_pin = GMBUS_PIN_DPB;
> > > +               break;
> > > +       }
> > > +
> > > +       DRM_DEBUG_KMS("Using DDC pin 0x%x for port %c (platform
> > > default)\n",
> > > +                     ddc_pin, port_name(port));
> > > +
> > > +       return ddc_pin;
> > > +}
> > > +
> > >  void intel_hdmi_init_connector(struct intel_digital_port
> *intel_dig_port,
> > >                                struct intel_connector *intel_connector)
> > >  {
> > > @@ -1808,7 +1852,6 @@ void intel_hdmi_init_connector(struct
> > > intel_digital_port *intel_dig_port,
> > >         struct drm_device *dev = intel_encoder->base.dev;
> > >         struct drm_i915_private *dev_priv = to_i915(dev);
> > >         enum port port = intel_dig_port->port;
> > > -       uint8_t alternate_ddc_pin;
> > >
> > >         DRM_DEBUG_KMS("Adding HDMI connector on port %c\n",
> > >                       port_name(port));
> > > @@ -1826,12 +1869,10 @@ void intel_hdmi_init_connector(struct
> > > intel_digital_port *intel_dig_port,
> > >         connector->doublescan_allowed = 0;
> > >         connector->stereo_allowed = 1;
> > >
> > > +       intel_hdmi->ddc_bus = intel_hdmi_ddc_pin(dev_priv, port);
> > > +
> > >         switch (port) {
> > >         case PORT_B:
> > > -               if (IS_BROXTON(dev_priv))
> > > -                       intel_hdmi->ddc_bus = GMBUS_PIN_1_BXT;
> > > -               else
> > > -                       intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
> > >                 /*
> > >                  * On BXT A0/A1, sw needs to activate DDIA HPD logic
> and
> > >                  * interrupts to check the external panel connection.
> > > @@ -1842,46 +1883,17 @@ void intel_hdmi_init_connector(struct
> > > intel_digital_port *intel_dig_port,
> > >                         intel_encoder->hpd_pin = HPD_PORT_B;
> > >                 break;
> > >         case PORT_C:
> > > -               if (IS_BROXTON(dev_priv))
> > > -                       intel_hdmi->ddc_bus = GMBUS_PIN_2_BXT;
> > > -               else
> > > -                       intel_hdmi->ddc_bus = GMBUS_PIN_DPC;
> > >                 intel_encoder->hpd_pin = HPD_PORT_C;
> > >                 break;
> > >         case PORT_D:
> > > -               if (WARN_ON(IS_BROXTON(dev_priv)))
> > > -                       intel_hdmi->ddc_bus = GMBUS_PIN_DISABLED;
> > > -               else if (IS_CHERRYVIEW(dev_priv))
> > > -                       intel_hdmi->ddc_bus = GMBUS_PIN_DPD_CHV;
> > > -               else
> > > -                       intel_hdmi->ddc_bus = GMBUS_PIN_DPD;
> > >                 intel_encoder->hpd_pin = HPD_PORT_D;
> > >                 break;
> > >         case PORT_E:
> > > -               /* On SKL PORT E doesn't have seperate GMBUS pin
> > > -                *  We rely on VBT to set a proper alternate GMBUS
> pin. */
> > > -               alternate_ddc_pin =
> > > -                       dev_priv->vbt.ddi_port_info[
> > > PORT_E].alternate_ddc_pin;
> > > -               switch (alternate_ddc_pin) {
> > > -               case DDC_PIN_B:
> > > -                       intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
> > > -                       break;
> > > -               case DDC_PIN_C:
> > > -                       intel_hdmi->ddc_bus = GMBUS_PIN_DPC;
> > > -                       break;
> > > -               case DDC_PIN_D:
> > > -                       intel_hdmi->ddc_bus = GMBUS_PIN_DPD;
> > > -                       break;
> > > -               default:
> > > -                       MISSING_CASE(alternate_ddc_pin);
> > > -               }
> > >                 intel_encoder->hpd_pin = HPD_PORT_E;
> > >                 break;
> > > -       case PORT_A:
> > > -               intel_encoder->hpd_pin = HPD_PORT_A;
> > > -               /* Internal port only for eDP. */
> > >         default:
> > > -               BUG();
> > > +               MISSING_CASE(port);
> > > +               return;
> > >         }
> > >
> > >         if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
> > > --
> > > 2.7.4
> > >
> > >
> >
> >
> > --
> > Far away from the primal instinct, the song seems to fade away, the river
> > get wider between your thoughts and the things we do and say.
>
> --
> Ville Syrjälä
> Intel OTC
>



-- 
Far away from the primal instinct, the song seems to fade away, the river
get wider between your thoughts and the things we do and say.

[-- Attachment #1.2: Type: text/html, Size: 11566 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

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

[Jim Bride]

On Tue, Oct 11, 2016 at 08:52:46PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> 
> The VBT provides the platform a way to mix and match the DDI ports vs.
> GMBUS pins. Currently we only trust the VBT for DDI E, which I suppose
> has no standard GMBUS pin assignment. However, there are machines out
> there that use a non-standard mapping for the other ports as well.
> Let's start trusting the VBT on this one for all ports on DDI platforms.
> 
> I've structured the code such that other platforms could easily start
> using this as well, by simply filling in the ddi_port_info. IIRC there
> may be CHV system that might actually need this.
> 
> v2: Include a commit message, include a debug message during init
> 
> Cc: stable@vger.kernel.org
> Cc: Maarten Maathuis <madman2003@gmail.com>
> Tested-by: Maarten Maatt show huis <madman2003@gmail.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97877
> Signed-off-by: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_hdmi.c | 84 ++++++++++++++++++++++-----------------
>  1 file changed, 48 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 8d46f5836746..9ca86e901fc8 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1799,6 +1799,50 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c
>  	intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
>  }
>  
> +static u8 intel_hdmi_ddc_pin(struct drm_i915_private *dev_priv,
> +			     enum port port)
> +{
> +	const struct ddi_vbt_port_info *info =
> +		&dev_priv->vbt.ddi_port_info[port];
> +	u8 ddc_pin;
> +
> +	if (info->alternate_ddc_pin) {
> +		DRM_DEBUG_KMS("Using DDC pin 0x%x for port %c (VBT)\n",
> +			      info->alternate_ddc_pin, port_name(port));
> +		return info->alternate_ddc_pin;
> +	}
> +
> +	switch (port) {
> +	case PORT_B:
> +		if (IS_BROXTON(dev_priv))
> +			ddc_pin = GMBUS_PIN_1_BXT;
> +		else
> +			ddc_pin = GMBUS_PIN_DPB;
> +		break;
> +	case PORT_C:
> +		if (IS_BROXTON(dev_priv))
> +			ddc_pin = GMBUS_PIN_2_BXT;
> +		else
> +			ddc_pin = GMBUS_PIN_DPC;
> +		break;
> +	case PORT_D:
> +		if (IS_CHERRYVIEW(dev_priv))
> +			ddc_pin = GMBUS_PIN_DPD_CHV;
> +		else
> +			ddc_pin = GMBUS_PIN_DPD;

In the code removed below there's a specific case covering Broxton that
isn't accounted for here.  Are we sure that we don't need that logic here?

Jim

> +		break;
> +	default:
> +		MISSING_CASE(port);
> +		ddc_pin = GMBUS_PIN_DPB;
> +		break;
> +	}
> +
> +	DRM_DEBUG_KMS("Using DDC pin 0x%x for port %c (platform default)\n",
> +		      ddc_pin, port_name(port));
> +
> +	return ddc_pin;
> +}
> +
>  void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
>  			       struct intel_connector *intel_connector)
>  {
> @@ -1808,7 +1852,6 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
>  	struct drm_device *dev = intel_encoder->base.dev;
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	enum port port = intel_dig_port->port;
> -	uint8_t alternate_ddc_pin;
>  
>  	DRM_DEBUG_KMS("Adding HDMI connector on port %c\n",
>  		      port_name(port));
> @@ -1826,12 +1869,10 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
>  	connector->doublescan_allowed = 0;
>  	connector->stereo_allowed = 1;
>  
> +	intel_hdmi->ddc_bus = intel_hdmi_ddc_pin(dev_priv, port);
> +
>  	switch (port) {
>  	case PORT_B:
> -		if (IS_BROXTON(dev_priv))
> -			intel_hdmi->ddc_bus = GMBUS_PIN_1_BXT;
> -		else
> -			intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
>  		/*
>  		 * On BXT A0/A1, sw needs to activate DDIA HPD logic and
>  		 * interrupts to check the external panel connection.
> @@ -1842,46 +1883,17 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
>  			intel_encoder->hpd_pin = HPD_PORT_B;
>  		break;
>  	case PORT_C:
> -		if (IS_BROXTON(dev_priv))
> -			intel_hdmi->ddc_bus = GMBUS_PIN_2_BXT;
> -		else
> -			intel_hdmi->ddc_bus = GMBUS_PIN_DPC;
>  		intel_encoder->hpd_pin = HPD_PORT_C;
>  		break;
>  	case PORT_D:
> -		if (WARN_ON(IS_BROXTON(dev_priv)))
> -			intel_hdmi->ddc_bus = GMBUS_PIN_DISABLED;
> -		else if (IS_CHERRYVIEW(dev_priv))
> -			intel_hdmi->ddc_bus = GMBUS_PIN_DPD_CHV;
> -		else
> -			intel_hdmi->ddc_bus = GMBUS_PIN_DPD;
>  		intel_encoder->hpd_pin = HPD_PORT_D;
>  		break;
>  	case PORT_E:
> -		/* On SKL PORT E doesn't have seperate GMBUS pin
> -		 *  We rely on VBT to set a proper alternate GMBUS pin. */
> -		alternate_ddc_pin =
> -			dev_priv->vbt.ddi_port_info[PORT_E].alternate_ddc_pin;
> -		switch (alternate_ddc_pin) {
> -		case DDC_PIN_B:
> -			intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
> -			break;
> -		case DDC_PIN_C:
> -			intel_hdmi->ddc_bus = GMBUS_PIN_DPC;
> -			break;
> -		case DDC_PIN_D:
> -			intel_hdmi->ddc_bus = GMBUS_PIN_DPD;
> -			break;
> -		default:
> -			MISSING_CASE(alternate_ddc_pin);
> -		}
>  		intel_encoder->hpd_pin = HPD_PORT_E;
>  		break;
> -	case PORT_A:
> -		intel_encoder->hpd_pin = HPD_PORT_A;
> -		/* Internal port only for eDP. */
>  	default:
> -		BUG();
> +		MISSING_CASE(port);
> +		return;
>  	}
>  
>  	if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/4] drm/i915: Respect alternate_ddc_pin for all DDI ports

[Jim Bride]

On Tue, Oct 11, 2016 at 08:52:46PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The VBT provides the platform a way to mix and match the DDI ports vs.
> GMBUS pins. Currently we only trust the VBT for DDI E, which I suppose
> has no standard GMBUS pin assignment. However, there are machines out
> there that use a non-standard mapping for the other ports as well.
> Let's start trusting the VBT on this one for all ports on DDI platforms.
> 
> I've structured the code such that other platforms could easily start
> using this as well, by simply filling in the ddi_port_info. IIRC there
> may be CHV system that might actually need this.
> 
> v2: Include a commit message, include a debug message during init
> 
> Cc: stable@vger.kernel.org
> Cc: Maarten Maathuis <madman2003@gmail.com>
> Tested-by: Maarten Maatt show huis <madman2003@gmail.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97877
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_hdmi.c | 84 ++++++++++++++++++++++-----------------
>  1 file changed, 48 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 8d46f5836746..9ca86e901fc8 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1799,6 +1799,50 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c
>  	intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
>  }
>  
> +static u8 intel_hdmi_ddc_pin(struct drm_i915_private *dev_priv,
> +			     enum port port)
> +{
> +	const struct ddi_vbt_port_info *info =
> +		&dev_priv->vbt.ddi_port_info[port];
> +	u8 ddc_pin;
> +
> +	if (info->alternate_ddc_pin) {
> +		DRM_DEBUG_KMS("Using DDC pin 0x%x for port %c (VBT)\n",
> +			      info->alternate_ddc_pin, port_name(port));
> +		return info->alternate_ddc_pin;
> +	}
> +
> +	switch (port) {
> +	case PORT_B:
> +		if (IS_BROXTON(dev_priv))
> +			ddc_pin = GMBUS_PIN_1_BXT;
> +		else
> +			ddc_pin = GMBUS_PIN_DPB;
> +		break;
> +	case PORT_C:
> +		if (IS_BROXTON(dev_priv))
> +			ddc_pin = GMBUS_PIN_2_BXT;
> +		else
> +			ddc_pin = GMBUS_PIN_DPC;
> +		break;
> +	case PORT_D:
> +		if (IS_CHERRYVIEW(dev_priv))
> +			ddc_pin = GMBUS_PIN_DPD_CHV;
> +		else
> +			ddc_pin = GMBUS_PIN_DPD;

In the code removed below there's a specific case covering Broxton that
isn't accounted for here.  Are we sure that we don't need that logic here?

Jim

> +		break;
> +	default:
> +		MISSING_CASE(port);
> +		ddc_pin = GMBUS_PIN_DPB;
> +		break;
> +	}
> +
> +	DRM_DEBUG_KMS("Using DDC pin 0x%x for port %c (platform default)\n",
> +		      ddc_pin, port_name(port));
> +
> +	return ddc_pin;
> +}
> +
>  void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
>  			       struct intel_connector *intel_connector)
>  {
> @@ -1808,7 +1852,6 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
>  	struct drm_device *dev = intel_encoder->base.dev;
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	enum port port = intel_dig_port->port;
> -	uint8_t alternate_ddc_pin;
>  
>  	DRM_DEBUG_KMS("Adding HDMI connector on port %c\n",
>  		      port_name(port));
> @@ -1826,12 +1869,10 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
>  	connector->doublescan_allowed = 0;
>  	connector->stereo_allowed = 1;
>  
> +	intel_hdmi->ddc_bus = intel_hdmi_ddc_pin(dev_priv, port);
> +
>  	switch (port) {
>  	case PORT_B:
> -		if (IS_BROXTON(dev_priv))
> -			intel_hdmi->ddc_bus = GMBUS_PIN_1_BXT;
> -		else
> -			intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
>  		/*
>  		 * On BXT A0/A1, sw needs to activate DDIA HPD logic and
>  		 * interrupts to check the external panel connection.
> @@ -1842,46 +1883,17 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
>  			intel_encoder->hpd_pin = HPD_PORT_B;
>  		break;
>  	case PORT_C:
> -		if (IS_BROXTON(dev_priv))
> -			intel_hdmi->ddc_bus = GMBUS_PIN_2_BXT;
> -		else
> -			intel_hdmi->ddc_bus = GMBUS_PIN_DPC;
>  		intel_encoder->hpd_pin = HPD_PORT_C;
>  		break;
>  	case PORT_D:
> -		if (WARN_ON(IS_BROXTON(dev_priv)))
> -			intel_hdmi->ddc_bus = GMBUS_PIN_DISABLED;
> -		else if (IS_CHERRYVIEW(dev_priv))
> -			intel_hdmi->ddc_bus = GMBUS_PIN_DPD_CHV;
> -		else
> -			intel_hdmi->ddc_bus = GMBUS_PIN_DPD;
>  		intel_encoder->hpd_pin = HPD_PORT_D;
>  		break;
>  	case PORT_E:
> -		/* On SKL PORT E doesn't have seperate GMBUS pin
> -		 *  We rely on VBT to set a proper alternate GMBUS pin. */
> -		alternate_ddc_pin =
> -			dev_priv->vbt.ddi_port_info[PORT_E].alternate_ddc_pin;
> -		switch (alternate_ddc_pin) {
> -		case DDC_PIN_B:
> -			intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
> -			break;
> -		case DDC_PIN_C:
> -			intel_hdmi->ddc_bus = GMBUS_PIN_DPC;
> -			break;
> -		case DDC_PIN_D:
> -			intel_hdmi->ddc_bus = GMBUS_PIN_DPD;
> -			break;
> -		default:
> -			MISSING_CASE(alternate_ddc_pin);
> -		}
>  		intel_encoder->hpd_pin = HPD_PORT_E;
>  		break;
> -	case PORT_A:
> -		intel_encoder->hpd_pin = HPD_PORT_A;
> -		/* Internal port only for eDP. */
>  	default:
> -		BUG();
> +		MISSING_CASE(port);
> +		return;
>  	}
>  
>  	if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Ville Syrjälä]

On Thu, Oct 13, 2016 at 11:06:55AM -0700, Jim Bride wrote:
> On Tue, Oct 11, 2016 at 08:52:46PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> > 
> > The VBT provides the platform a way to mix and match the DDI ports vs.
> > GMBUS pins. Currently we only trust the VBT for DDI E, which I suppose
> > has no standard GMBUS pin assignment. However, there are machines out
> > there that use a non-standard mapping for the other ports as well.
> > Let's start trusting the VBT on this one for all ports on DDI platforms.
> > 
> > I've structured the code such that other platforms could easily start
> > using this as well, by simply filling in the ddi_port_info. IIRC there
> > may be CHV system that might actually need this.
> > 
> > v2: Include a commit message, include a debug message during init
> > 
> > Cc: stable@vger.kernel.org
> > Cc: Maarten Maathuis <madman2003@gmail.com>
> > Tested-by: Maarten Maatt show huis <madman2003@gmail.com>
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97877
> > Signed-off-by: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_hdmi.c | 84 ++++++++++++++++++++++-----------------
> >  1 file changed, 48 insertions(+), 36 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> > index 8d46f5836746..9ca86e901fc8 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -1799,6 +1799,50 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c
> >  	intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
> >  }
> >  
> > +static u8 intel_hdmi_ddc_pin(struct drm_i915_private *dev_priv,
> > +			     enum port port)
> > +{
> > +	const struct ddi_vbt_port_info *info =
> > +		&dev_priv->vbt.ddi_port_info[port];
> > +	u8 ddc_pin;
> > +
> > +	if (info->alternate_ddc_pin) {
> > +		DRM_DEBUG_KMS("Using DDC pin 0x%x for port %c (VBT)\n",
> > +			      info->alternate_ddc_pin, port_name(port));
> > +		return info->alternate_ddc_pin;
> > +	}
> > +
> > +	switch (port) {
> > +	case PORT_B:
> > +		if (IS_BROXTON(dev_priv))
> > +			ddc_pin = GMBUS_PIN_1_BXT;
> > +		else
> > +			ddc_pin = GMBUS_PIN_DPB;
> > +		break;
> > +	case PORT_C:
> > +		if (IS_BROXTON(dev_priv))
> > +			ddc_pin = GMBUS_PIN_2_BXT;
> > +		else
> > +			ddc_pin = GMBUS_PIN_DPC;
> > +		break;
> > +	case PORT_D:
> > +		if (IS_CHERRYVIEW(dev_priv))
> > +			ddc_pin = GMBUS_PIN_DPD_CHV;
> > +		else
> > +			ddc_pin = GMBUS_PIN_DPD;
> 
> In the code removed below there's a specific case covering Broxton that
> isn't accounted for here.  Are we sure that we don't need that logic here?

Yeah, BXT doesn't have port D.

I guess we could split this up into a few platform specific functions that
could then have the MISSING_CASE() for the impossible ports. A bit like
the AUX ctl/data register setup functions we have. Do pople want that
level of paranoia here?

> 
> Jim
> 
> > +		break;
> > +	default:
> > +		MISSING_CASE(port);
> > +		ddc_pin = GMBUS_PIN_DPB;
> > +		break;
> > +	}
> > +
> > +	DRM_DEBUG_KMS("Using DDC pin 0x%x for port %c (platform default)\n",
> > +		      ddc_pin, port_name(port));
> > +
> > +	return ddc_pin;
> > +}
> > +
> >  void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
> >  			       struct intel_connector *intel_connector)
> >  {
> > @@ -1808,7 +1852,6 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
> >  	struct drm_device *dev = intel_encoder->base.dev;
> >  	struct drm_i915_private *dev_priv = to_i915(dev);
> >  	enum port port = intel_dig_port->port;
> > -	uint8_t alternate_ddc_pin;
> >  
> >  	DRM_DEBUG_KMS("Adding HDMI connector on port %c\n",
> >  		      port_name(port));
> > @@ -1826,12 +1869,10 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
> >  	connector->doublescan_allowed = 0;
> >  	connector->stereo_allowed = 1;
> >  
> > +	intel_hdmi->ddc_bus = intel_hdmi_ddc_pin(dev_priv, port);
> > +
> >  	switch (port) {
> >  	case PORT_B:
> > -		if (IS_BROXTON(dev_priv))
> > -			intel_hdmi->ddc_bus = GMBUS_PIN_1_BXT;
> > -		else
> > -			intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
> >  		/*
> >  		 * On BXT A0/A1, sw needs to activate DDIA HPD logic and
> >  		 * interrupts to check the external panel connection.
> > @@ -1842,46 +1883,17 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
> >  			intel_encoder->hpd_pin = HPD_PORT_B;
> >  		break;
> >  	case PORT_C:
> > -		if (IS_BROXTON(dev_priv))
> > -			intel_hdmi->ddc_bus = GMBUS_PIN_2_BXT;
> > -		else
> > -			intel_hdmi->ddc_bus = GMBUS_PIN_DPC;
> >  		intel_encoder->hpd_pin = HPD_PORT_C;
> >  		break;
> >  	case PORT_D:
> > -		if (WARN_ON(IS_BROXTON(dev_priv)))
> > -			intel_hdmi->ddc_bus = GMBUS_PIN_DISABLED;
> > -		else if (IS_CHERRYVIEW(dev_priv))
> > -			intel_hdmi->ddc_bus = GMBUS_PIN_DPD_CHV;
> > -		else
> > -			intel_hdmi->ddc_bus = GMBUS_PIN_DPD;
> >  		intel_encoder->hpd_pin = HPD_PORT_D;
> >  		break;
> >  	case PORT_E:
> > -		/* On SKL PORT E doesn't have seperate GMBUS pin
> > -		 *  We rely on VBT to set a proper alternate GMBUS pin. */
> > -		alternate_ddc_pin =
> > -			dev_priv->vbt.ddi_port_info[PORT_E].alternate_ddc_pin;
> > -		switch (alternate_ddc_pin) {
> > -		case DDC_PIN_B:
> > -			intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
> > -			break;
> > -		case DDC_PIN_C:
> > -			intel_hdmi->ddc_bus = GMBUS_PIN_DPC;
> > -			break;
> > -		case DDC_PIN_D:
> > -			intel_hdmi->ddc_bus = GMBUS_PIN_DPD;
> > -			break;
> > -		default:
> > -			MISSING_CASE(alternate_ddc_pin);
> > -		}
> >  		intel_encoder->hpd_pin = HPD_PORT_E;
> >  		break;
> > -	case PORT_A:
> > -		intel_encoder->hpd_pin = HPD_PORT_A;
> > -		/* Internal port only for eDP. */
> >  	default:
> > -		BUG();
> > +		MISSING_CASE(port);
> > +		return;
> >  	}
> >  
> >  	if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
> > -- 
> > 2.7.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrj�l�
Intel OTC

Re: [PATCH 2/4] drm/i915: Respect alternate_ddc_pin for all DDI ports

[Ville Syrjälä]

On Thu, Oct 13, 2016 at 11:06:55AM -0700, Jim Bride wrote:
> On Tue, Oct 11, 2016 at 08:52:46PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > The VBT provides the platform a way to mix and match the DDI ports vs.
> > GMBUS pins. Currently we only trust the VBT for DDI E, which I suppose
> > has no standard GMBUS pin assignment. However, there are machines out
> > there that use a non-standard mapping for the other ports as well.
> > Let's start trusting the VBT on this one for all ports on DDI platforms.
> > 
> > I've structured the code such that other platforms could easily start
> > using this as well, by simply filling in the ddi_port_info. IIRC there
> > may be CHV system that might actually need this.
> > 
> > v2: Include a commit message, include a debug message during init
> > 
> > Cc: stable@vger.kernel.org
> > Cc: Maarten Maathuis <madman2003@gmail.com>
> > Tested-by: Maarten Maatt show huis <madman2003@gmail.com>
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97877
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_hdmi.c | 84 ++++++++++++++++++++++-----------------
> >  1 file changed, 48 insertions(+), 36 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> > index 8d46f5836746..9ca86e901fc8 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -1799,6 +1799,50 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c
> >  	intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
> >  }
> >  
> > +static u8 intel_hdmi_ddc_pin(struct drm_i915_private *dev_priv,
> > +			     enum port port)
> > +{
> > +	const struct ddi_vbt_port_info *info =
> > +		&dev_priv->vbt.ddi_port_info[port];
> > +	u8 ddc_pin;
> > +
> > +	if (info->alternate_ddc_pin) {
> > +		DRM_DEBUG_KMS("Using DDC pin 0x%x for port %c (VBT)\n",
> > +			      info->alternate_ddc_pin, port_name(port));
> > +		return info->alternate_ddc_pin;
> > +	}
> > +
> > +	switch (port) {
> > +	case PORT_B:
> > +		if (IS_BROXTON(dev_priv))
> > +			ddc_pin = GMBUS_PIN_1_BXT;
> > +		else
> > +			ddc_pin = GMBUS_PIN_DPB;
> > +		break;
> > +	case PORT_C:
> > +		if (IS_BROXTON(dev_priv))
> > +			ddc_pin = GMBUS_PIN_2_BXT;
> > +		else
> > +			ddc_pin = GMBUS_PIN_DPC;
> > +		break;
> > +	case PORT_D:
> > +		if (IS_CHERRYVIEW(dev_priv))
> > +			ddc_pin = GMBUS_PIN_DPD_CHV;
> > +		else
> > +			ddc_pin = GMBUS_PIN_DPD;
> 
> In the code removed below there's a specific case covering Broxton that
> isn't accounted for here.  Are we sure that we don't need that logic here?

Yeah, BXT doesn't have port D.

I guess we could split this up into a few platform specific functions that
could then have the MISSING_CASE() for the impossible ports. A bit like
the AUX ctl/data register setup functions we have. Do pople want that
level of paranoia here?

> 
> Jim
> 
> > +		break;
> > +	default:
> > +		MISSING_CASE(port);
> > +		ddc_pin = GMBUS_PIN_DPB;
> > +		break;
> > +	}
> > +
> > +	DRM_DEBUG_KMS("Using DDC pin 0x%x for port %c (platform default)\n",
> > +		      ddc_pin, port_name(port));
> > +
> > +	return ddc_pin;
> > +}
> > +
> >  void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
> >  			       struct intel_connector *intel_connector)
> >  {
> > @@ -1808,7 +1852,6 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
> >  	struct drm_device *dev = intel_encoder->base.dev;
> >  	struct drm_i915_private *dev_priv = to_i915(dev);
> >  	enum port port = intel_dig_port->port;
> > -	uint8_t alternate_ddc_pin;
> >  
> >  	DRM_DEBUG_KMS("Adding HDMI connector on port %c\n",
> >  		      port_name(port));
> > @@ -1826,12 +1869,10 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
> >  	connector->doublescan_allowed = 0;
> >  	connector->stereo_allowed = 1;
> >  
> > +	intel_hdmi->ddc_bus = intel_hdmi_ddc_pin(dev_priv, port);
> > +
> >  	switch (port) {
> >  	case PORT_B:
> > -		if (IS_BROXTON(dev_priv))
> > -			intel_hdmi->ddc_bus = GMBUS_PIN_1_BXT;
> > -		else
> > -			intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
> >  		/*
> >  		 * On BXT A0/A1, sw needs to activate DDIA HPD logic and
> >  		 * interrupts to check the external panel connection.
> > @@ -1842,46 +1883,17 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
> >  			intel_encoder->hpd_pin = HPD_PORT_B;
> >  		break;
> >  	case PORT_C:
> > -		if (IS_BROXTON(dev_priv))
> > -			intel_hdmi->ddc_bus = GMBUS_PIN_2_BXT;
> > -		else
> > -			intel_hdmi->ddc_bus = GMBUS_PIN_DPC;
> >  		intel_encoder->hpd_pin = HPD_PORT_C;
> >  		break;
> >  	case PORT_D:
> > -		if (WARN_ON(IS_BROXTON(dev_priv)))
> > -			intel_hdmi->ddc_bus = GMBUS_PIN_DISABLED;
> > -		else if (IS_CHERRYVIEW(dev_priv))
> > -			intel_hdmi->ddc_bus = GMBUS_PIN_DPD_CHV;
> > -		else
> > -			intel_hdmi->ddc_bus = GMBUS_PIN_DPD;
> >  		intel_encoder->hpd_pin = HPD_PORT_D;
> >  		break;
> >  	case PORT_E:
> > -		/* On SKL PORT E doesn't have seperate GMBUS pin
> > -		 *  We rely on VBT to set a proper alternate GMBUS pin. */
> > -		alternate_ddc_pin =
> > -			dev_priv->vbt.ddi_port_info[PORT_E].alternate_ddc_pin;
> > -		switch (alternate_ddc_pin) {
> > -		case DDC_PIN_B:
> > -			intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
> > -			break;
> > -		case DDC_PIN_C:
> > -			intel_hdmi->ddc_bus = GMBUS_PIN_DPC;
> > -			break;
> > -		case DDC_PIN_D:
> > -			intel_hdmi->ddc_bus = GMBUS_PIN_DPD;
> > -			break;
> > -		default:
> > -			MISSING_CASE(alternate_ddc_pin);
> > -		}
> >  		intel_encoder->hpd_pin = HPD_PORT_E;
> >  		break;
> > -	case PORT_A:
> > -		intel_encoder->hpd_pin = HPD_PORT_A;
> > -		/* Internal port only for eDP. */
> >  	default:
> > -		BUG();
> > +		MISSING_CASE(port);
> > +		return;
> >  	}
> >  
> >  	if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
> > -- 
> > 2.7.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Daniel Vetter]

On Thu, Oct 13, 2016 at 09:29:30PM +0300, Ville Syrj�l� wrote:
> On Thu, Oct 13, 2016 at 11:06:55AM -0700, Jim Bride wrote:
> > On Tue, Oct 11, 2016 at 08:52:46PM +0300, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> > > 
> > > The VBT provides the platform a way to mix and match the DDI ports vs.
> > > GMBUS pins. Currently we only trust the VBT for DDI E, which I suppose
> > > has no standard GMBUS pin assignment. However, there are machines out
> > > there that use a non-standard mapping for the other ports as well.
> > > Let's start trusting the VBT on this one for all ports on DDI platforms.
> > > 
> > > I've structured the code such that other platforms could easily start
> > > using this as well, by simply filling in the ddi_port_info. IIRC there
> > > may be CHV system that might actually need this.
> > > 
> > > v2: Include a commit message, include a debug message during init
> > > 
> > > Cc: stable@vger.kernel.org
> > > Cc: Maarten Maathuis <madman2003@gmail.com>
> > > Tested-by: Maarten Maatt show huis <madman2003@gmail.com>
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97877
> > > Signed-off-by: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_hdmi.c | 84 ++++++++++++++++++++++-----------------
> > >  1 file changed, 48 insertions(+), 36 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> > > index 8d46f5836746..9ca86e901fc8 100644
> > > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > > @@ -1799,6 +1799,50 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c
> > >  	intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
> > >  }
> > >  
> > > +static u8 intel_hdmi_ddc_pin(struct drm_i915_private *dev_priv,
> > > +			     enum port port)
> > > +{
> > > +	const struct ddi_vbt_port_info *info =
> > > +		&dev_priv->vbt.ddi_port_info[port];
> > > +	u8 ddc_pin;
> > > +
> > > +	if (info->alternate_ddc_pin) {
> > > +		DRM_DEBUG_KMS("Using DDC pin 0x%x for port %c (VBT)\n",
> > > +			      info->alternate_ddc_pin, port_name(port));
> > > +		return info->alternate_ddc_pin;
> > > +	}
> > > +
> > > +	switch (port) {
> > > +	case PORT_B:
> > > +		if (IS_BROXTON(dev_priv))
> > > +			ddc_pin = GMBUS_PIN_1_BXT;
> > > +		else
> > > +			ddc_pin = GMBUS_PIN_DPB;
> > > +		break;
> > > +	case PORT_C:
> > > +		if (IS_BROXTON(dev_priv))
> > > +			ddc_pin = GMBUS_PIN_2_BXT;
> > > +		else
> > > +			ddc_pin = GMBUS_PIN_DPC;
> > > +		break;
> > > +	case PORT_D:
> > > +		if (IS_CHERRYVIEW(dev_priv))
> > > +			ddc_pin = GMBUS_PIN_DPD_CHV;
> > > +		else
> > > +			ddc_pin = GMBUS_PIN_DPD;
> > 
> > In the code removed below there's a specific case covering Broxton that
> > isn't accounted for here.  Are we sure that we don't need that logic here?
> 
> Yeah, BXT doesn't have port D.
> 
> I guess we could split this up into a few platform specific functions that
> could then have the MISSING_CASE() for the impossible ports. A bit like
> the AUX ctl/data register setup functions we have. Do pople want that
> level of paranoia here?

tbh seems overkill, we should go boom in the platform gmbus code if you
end up with an impossible port.
-Daniel

> 
> > 
> > Jim
> > 
> > > +		break;
> > > +	default:
> > > +		MISSING_CASE(port);
> > > +		ddc_pin = GMBUS_PIN_DPB;
> > > +		break;
> > > +	}
> > > +
> > > +	DRM_DEBUG_KMS("Using DDC pin 0x%x for port %c (platform default)\n",
> > > +		      ddc_pin, port_name(port));
> > > +
> > > +	return ddc_pin;
> > > +}
> > > +
> > >  void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
> > >  			       struct intel_connector *intel_connector)
> > >  {
> > > @@ -1808,7 +1852,6 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
> > >  	struct drm_device *dev = intel_encoder->base.dev;
> > >  	struct drm_i915_private *dev_priv = to_i915(dev);
> > >  	enum port port = intel_dig_port->port;
> > > -	uint8_t alternate_ddc_pin;
> > >  
> > >  	DRM_DEBUG_KMS("Adding HDMI connector on port %c\n",
> > >  		      port_name(port));
> > > @@ -1826,12 +1869,10 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
> > >  	connector->doublescan_allowed = 0;
> > >  	connector->stereo_allowed = 1;
> > >  
> > > +	intel_hdmi->ddc_bus = intel_hdmi_ddc_pin(dev_priv, port);
> > > +
> > >  	switch (port) {
> > >  	case PORT_B:
> > > -		if (IS_BROXTON(dev_priv))
> > > -			intel_hdmi->ddc_bus = GMBUS_PIN_1_BXT;
> > > -		else
> > > -			intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
> > >  		/*
> > >  		 * On BXT A0/A1, sw needs to activate DDIA HPD logic and
> > >  		 * interrupts to check the external panel connection.
> > > @@ -1842,46 +1883,17 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
> > >  			intel_encoder->hpd_pin = HPD_PORT_B;
> > >  		break;
> > >  	case PORT_C:
> > > -		if (IS_BROXTON(dev_priv))
> > > -			intel_hdmi->ddc_bus = GMBUS_PIN_2_BXT;
> > > -		else
> > > -			intel_hdmi->ddc_bus = GMBUS_PIN_DPC;
> > >  		intel_encoder->hpd_pin = HPD_PORT_C;
> > >  		break;
> > >  	case PORT_D:
> > > -		if (WARN_ON(IS_BROXTON(dev_priv)))
> > > -			intel_hdmi->ddc_bus = GMBUS_PIN_DISABLED;
> > > -		else if (IS_CHERRYVIEW(dev_priv))
> > > -			intel_hdmi->ddc_bus = GMBUS_PIN_DPD_CHV;
> > > -		else
> > > -			intel_hdmi->ddc_bus = GMBUS_PIN_DPD;
> > >  		intel_encoder->hpd_pin = HPD_PORT_D;
> > >  		break;
> > >  	case PORT_E:
> > > -		/* On SKL PORT E doesn't have seperate GMBUS pin
> > > -		 *  We rely on VBT to set a proper alternate GMBUS pin. */
> > > -		alternate_ddc_pin =
> > > -			dev_priv->vbt.ddi_port_info[PORT_E].alternate_ddc_pin;
> > > -		switch (alternate_ddc_pin) {
> > > -		case DDC_PIN_B:
> > > -			intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
> > > -			break;
> > > -		case DDC_PIN_C:
> > > -			intel_hdmi->ddc_bus = GMBUS_PIN_DPC;
> > > -			break;
> > > -		case DDC_PIN_D:
> > > -			intel_hdmi->ddc_bus = GMBUS_PIN_DPD;
> > > -			break;
> > > -		default:
> > > -			MISSING_CASE(alternate_ddc_pin);
> > > -		}
> > >  		intel_encoder->hpd_pin = HPD_PORT_E;
> > >  		break;
> > > -	case PORT_A:
> > > -		intel_encoder->hpd_pin = HPD_PORT_A;
> > > -		/* Internal port only for eDP. */
> > >  	default:
> > > -		BUG();
> > > +		MISSING_CASE(port);
> > > +		return;
> > >  	}
> > >  
> > >  	if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
> > > -- 
> > > 2.7.4
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrj�l�
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

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

[Daniel Vetter]

On Thu, Oct 13, 2016 at 09:29:30PM +0300, Ville Syrjälä wrote:
> On Thu, Oct 13, 2016 at 11:06:55AM -0700, Jim Bride wrote:
> > On Tue, Oct 11, 2016 at 08:52:46PM +0300, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > The VBT provides the platform a way to mix and match the DDI ports vs.
> > > GMBUS pins. Currently we only trust the VBT for DDI E, which I suppose
> > > has no standard GMBUS pin assignment. However, there are machines out
> > > there that use a non-standard mapping for the other ports as well.
> > > Let's start trusting the VBT on this one for all ports on DDI platforms.
> > > 
> > > I've structured the code such that other platforms could easily start
> > > using this as well, by simply filling in the ddi_port_info. IIRC there
> > > may be CHV system that might actually need this.
> > > 
> > > v2: Include a commit message, include a debug message during init
> > > 
> > > Cc: stable@vger.kernel.org
> > > Cc: Maarten Maathuis <madman2003@gmail.com>
> > > Tested-by: Maarten Maatt show huis <madman2003@gmail.com>
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97877
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_hdmi.c | 84 ++++++++++++++++++++++-----------------
> > >  1 file changed, 48 insertions(+), 36 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> > > index 8d46f5836746..9ca86e901fc8 100644
> > > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > > @@ -1799,6 +1799,50 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c
> > >  	intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
> > >  }
> > >  
> > > +static u8 intel_hdmi_ddc_pin(struct drm_i915_private *dev_priv,
> > > +			     enum port port)
> > > +{
> > > +	const struct ddi_vbt_port_info *info =
> > > +		&dev_priv->vbt.ddi_port_info[port];
> > > +	u8 ddc_pin;
> > > +
> > > +	if (info->alternate_ddc_pin) {
> > > +		DRM_DEBUG_KMS("Using DDC pin 0x%x for port %c (VBT)\n",
> > > +			      info->alternate_ddc_pin, port_name(port));
> > > +		return info->alternate_ddc_pin;
> > > +	}
> > > +
> > > +	switch (port) {
> > > +	case PORT_B:
> > > +		if (IS_BROXTON(dev_priv))
> > > +			ddc_pin = GMBUS_PIN_1_BXT;
> > > +		else
> > > +			ddc_pin = GMBUS_PIN_DPB;
> > > +		break;
> > > +	case PORT_C:
> > > +		if (IS_BROXTON(dev_priv))
> > > +			ddc_pin = GMBUS_PIN_2_BXT;
> > > +		else
> > > +			ddc_pin = GMBUS_PIN_DPC;
> > > +		break;
> > > +	case PORT_D:
> > > +		if (IS_CHERRYVIEW(dev_priv))
> > > +			ddc_pin = GMBUS_PIN_DPD_CHV;
> > > +		else
> > > +			ddc_pin = GMBUS_PIN_DPD;
> > 
> > In the code removed below there's a specific case covering Broxton that
> > isn't accounted for here.  Are we sure that we don't need that logic here?
> 
> Yeah, BXT doesn't have port D.
> 
> I guess we could split this up into a few platform specific functions that
> could then have the MISSING_CASE() for the impossible ports. A bit like
> the AUX ctl/data register setup functions we have. Do pople want that
> level of paranoia here?

tbh seems overkill, we should go boom in the platform gmbus code if you
end up with an impossible port.
-Daniel

> 
> > 
> > Jim
> > 
> > > +		break;
> > > +	default:
> > > +		MISSING_CASE(port);
> > > +		ddc_pin = GMBUS_PIN_DPB;
> > > +		break;
> > > +	}
> > > +
> > > +	DRM_DEBUG_KMS("Using DDC pin 0x%x for port %c (platform default)\n",
> > > +		      ddc_pin, port_name(port));
> > > +
> > > +	return ddc_pin;
> > > +}
> > > +
> > >  void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
> > >  			       struct intel_connector *intel_connector)
> > >  {
> > > @@ -1808,7 +1852,6 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
> > >  	struct drm_device *dev = intel_encoder->base.dev;
> > >  	struct drm_i915_private *dev_priv = to_i915(dev);
> > >  	enum port port = intel_dig_port->port;
> > > -	uint8_t alternate_ddc_pin;
> > >  
> > >  	DRM_DEBUG_KMS("Adding HDMI connector on port %c\n",
> > >  		      port_name(port));
> > > @@ -1826,12 +1869,10 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
> > >  	connector->doublescan_allowed = 0;
> > >  	connector->stereo_allowed = 1;
> > >  
> > > +	intel_hdmi->ddc_bus = intel_hdmi_ddc_pin(dev_priv, port);
> > > +
> > >  	switch (port) {
> > >  	case PORT_B:
> > > -		if (IS_BROXTON(dev_priv))
> > > -			intel_hdmi->ddc_bus = GMBUS_PIN_1_BXT;
> > > -		else
> > > -			intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
> > >  		/*
> > >  		 * On BXT A0/A1, sw needs to activate DDIA HPD logic and
> > >  		 * interrupts to check the external panel connection.
> > > @@ -1842,46 +1883,17 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
> > >  			intel_encoder->hpd_pin = HPD_PORT_B;
> > >  		break;
> > >  	case PORT_C:
> > > -		if (IS_BROXTON(dev_priv))
> > > -			intel_hdmi->ddc_bus = GMBUS_PIN_2_BXT;
> > > -		else
> > > -			intel_hdmi->ddc_bus = GMBUS_PIN_DPC;
> > >  		intel_encoder->hpd_pin = HPD_PORT_C;
> > >  		break;
> > >  	case PORT_D:
> > > -		if (WARN_ON(IS_BROXTON(dev_priv)))
> > > -			intel_hdmi->ddc_bus = GMBUS_PIN_DISABLED;
> > > -		else if (IS_CHERRYVIEW(dev_priv))
> > > -			intel_hdmi->ddc_bus = GMBUS_PIN_DPD_CHV;
> > > -		else
> > > -			intel_hdmi->ddc_bus = GMBUS_PIN_DPD;
> > >  		intel_encoder->hpd_pin = HPD_PORT_D;
> > >  		break;
> > >  	case PORT_E:
> > > -		/* On SKL PORT E doesn't have seperate GMBUS pin
> > > -		 *  We rely on VBT to set a proper alternate GMBUS pin. */
> > > -		alternate_ddc_pin =
> > > -			dev_priv->vbt.ddi_port_info[PORT_E].alternate_ddc_pin;
> > > -		switch (alternate_ddc_pin) {
> > > -		case DDC_PIN_B:
> > > -			intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
> > > -			break;
> > > -		case DDC_PIN_C:
> > > -			intel_hdmi->ddc_bus = GMBUS_PIN_DPC;
> > > -			break;
> > > -		case DDC_PIN_D:
> > > -			intel_hdmi->ddc_bus = GMBUS_PIN_DPD;
> > > -			break;
> > > -		default:
> > > -			MISSING_CASE(alternate_ddc_pin);
> > > -		}
> > >  		intel_encoder->hpd_pin = HPD_PORT_E;
> > >  		break;
> > > -	case PORT_A:
> > > -		intel_encoder->hpd_pin = HPD_PORT_A;
> > > -		/* Internal port only for eDP. */
> > >  	default:
> > > -		BUG();
> > > +		MISSING_CASE(port);
> > > +		return;
> > >  	}
> > >  
> > >  	if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
> > > -- 
> > > 2.7.4
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[PATCH 3/4] drm/i915: Clean up DDI DDC/AUX CH sanitation

[ville.syrjala]

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

Now that we use the AUX and GMBUS assignment from VBT for all ports,
let's clean up the sanitization of the port information a bit.
Previosuly we only did this for port E, and only complained about a
non-standard assignment for the other ports. But as we know that
non-standard assignments are a fact of life, let's expand the
sanitization to all the ports.

v2: Include a commit message, fix up the comments a bit

Cc: stable@vger.kernel.org
Cc: Maarten Maathuis <madman2003@gmail.com>
Tested-by: Maarten Maathuis <madman2003@gmail.com>
References: https://bugs.freedesktop.org/show_bug.cgi?id=97877
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_bios.c | 116 +++++++++++++++++++++++---------------
 1 file changed, 71 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 83667e8cdd6b..6d1ffa815e97 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1035,6 +1035,71 @@ static u8 translate_iboost(u8 val)
 	return mapping[val];
 }
 
+static void sanitize_ddc_pin(struct drm_i915_private *dev_priv,
+			     enum port port)
+{
+	const struct ddi_vbt_port_info *info =
+		&dev_priv->vbt.ddi_port_info[port];
+	enum port p;
+
+	for_each_port_masked(p, (1 << port) - 1) {
+		struct ddi_vbt_port_info *i = &dev_priv->vbt.ddi_port_info[p];
+
+		if (info->alternate_ddc_pin != i->alternate_ddc_pin)
+			continue;
+
+		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(p), i->alternate_ddc_pin,
+			      port_name(port), port_name(p));
+
+		/*
+		 * If we have multiple ports supposedly sharing the
+		 * pin, then dvi/hdmi couldn't exist on the shared
+		 * port. Otherwise they share the same ddc bin and
+		 * system couldn't communicate with them separately.
+		 *
+		 * Due to parsing the ports in alphabetical order,
+		 * a higher port will always clobber a lower one.
+		 */
+		i->supports_dvi = false;
+		i->supports_hdmi = false;
+		i->alternate_ddc_pin = 0;
+	}
+}
+
+static void sanitize_aux_ch(struct drm_i915_private *dev_priv,
+			    enum port port)
+{
+	const struct ddi_vbt_port_info *info =
+		&dev_priv->vbt.ddi_port_info[port];
+	enum port p;
+
+	for_each_port_masked(p, (1 << port) - 1) {
+		struct ddi_vbt_port_info *i = &dev_priv->vbt.ddi_port_info[p];
+
+		if (info->alternate_aux_channel != i->alternate_aux_channel)
+			continue;
+
+		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(p), i->alternate_aux_channel,
+			      port_name(port), port_name(p));
+
+		/*
+		 * If we have multiple ports supposedlt sharing the
+		 * aux channel, then DP couldn't exist on the shared
+		 * port. Otherwise they share the same aux channel
+		 * and system couldn't communicate with them separately.
+		 *
+		 * Due to parsing the ports in alphabetical order,
+		 * a higher port will always clobber a lower one.
+		 */
+		i->supports_dp = false;
+		i->alternate_aux_channel = 0;
+	}
+}
+
 static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
 			   const struct bdb_header *bdb)
 {
@@ -1109,54 +1174,15 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
 		DRM_DEBUG_KMS("Port %c is internal DP\n", port_name(port));
 
 	if (is_dvi) {
-		if (port == PORT_E) {
-			info->alternate_ddc_pin = ddc_pin;
-			/* if DDIE share ddc pin with other port, then
-			 * dvi/hdmi couldn't exist on the shared port.
-			 * Otherwise they share the same ddc bin and system
-			 * couldn't communicate with them seperately. */
-			if (ddc_pin == DDC_PIN_B) {
-				dev_priv->vbt.ddi_port_info[PORT_B].supports_dvi = 0;
-				dev_priv->vbt.ddi_port_info[PORT_B].supports_hdmi = 0;
-			} else if (ddc_pin == DDC_PIN_C) {
-				dev_priv->vbt.ddi_port_info[PORT_C].supports_dvi = 0;
-				dev_priv->vbt.ddi_port_info[PORT_C].supports_hdmi = 0;
-			} else if (ddc_pin == DDC_PIN_D) {
-				dev_priv->vbt.ddi_port_info[PORT_D].supports_dvi = 0;
-				dev_priv->vbt.ddi_port_info[PORT_D].supports_hdmi = 0;
-			}
-		} else if (ddc_pin == DDC_PIN_B && port != PORT_B)
-			DRM_DEBUG_KMS("Unexpected DDC pin for port B\n");
-		else if (ddc_pin == DDC_PIN_C && port != PORT_C)
-			DRM_DEBUG_KMS("Unexpected DDC pin for port C\n");
-		else if (ddc_pin == DDC_PIN_D && port != PORT_D)
-			DRM_DEBUG_KMS("Unexpected DDC pin for port D\n");
+		info->alternate_ddc_pin = ddc_pin;
+
+		sanitize_ddc_pin(dev_priv, port);
 	}
 
 	if (is_dp) {
-		if (port == PORT_E) {
-			info->alternate_aux_channel = aux_channel;
-			/* if DDIE share aux channel with other port, then
-			 * DP couldn't exist on the shared port. Otherwise
-			 * they share the same aux channel and system
-			 * couldn't communicate with them seperately. */
-			if (aux_channel == DP_AUX_A)
-				dev_priv->vbt.ddi_port_info[PORT_A].supports_dp = 0;
-			else if (aux_channel == DP_AUX_B)
-				dev_priv->vbt.ddi_port_info[PORT_B].supports_dp = 0;
-			else if (aux_channel == DP_AUX_C)
-				dev_priv->vbt.ddi_port_info[PORT_C].supports_dp = 0;
-			else if (aux_channel == DP_AUX_D)
-				dev_priv->vbt.ddi_port_info[PORT_D].supports_dp = 0;
-		}
-		else if (aux_channel == DP_AUX_A && port != PORT_A)
-			DRM_DEBUG_KMS("Unexpected AUX channel for port A\n");
-		else if (aux_channel == DP_AUX_B && port != PORT_B)
-			DRM_DEBUG_KMS("Unexpected AUX channel for port B\n");
-		else if (aux_channel == DP_AUX_C && port != PORT_C)
-			DRM_DEBUG_KMS("Unexpected AUX channel for port C\n");
-		else if (aux_channel == DP_AUX_D && port != PORT_D)
-			DRM_DEBUG_KMS("Unexpected AUX channel for port D\n");
+		info->alternate_aux_channel = aux_channel;
+
+		sanitize_aux_ch(dev_priv, port);
 	}
 
 	if (bdb->version >= 158) {
-- 
2.7.4

[PATCH 3/4] drm/i915: Clean up DDI DDC/AUX CH sanitation

[ville.syrjala]

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

Now that we use the AUX and GMBUS assignment from VBT for all ports,
let's clean up the sanitization of the port information a bit.
Previosuly we only did this for port E, and only complained about a
non-standard assignment for the other ports. But as we know that
non-standard assignments are a fact of life, let's expand the
sanitization to all the ports.

v2: Include a commit message, fix up the comments a bit

Cc: stable@vger.kernel.org
Cc: Maarten Maathuis <madman2003@gmail.com>
Tested-by: Maarten Maathuis <madman2003@gmail.com>
References: https://bugs.freedesktop.org/show_bug.cgi?id=97877
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_bios.c | 116 +++++++++++++++++++++++---------------
 1 file changed, 71 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 83667e8cdd6b..6d1ffa815e97 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1035,6 +1035,71 @@ static u8 translate_iboost(u8 val)
 	return mapping[val];
 }
 
+static void sanitize_ddc_pin(struct drm_i915_private *dev_priv,
+			     enum port port)
+{
+	const struct ddi_vbt_port_info *info =
+		&dev_priv->vbt.ddi_port_info[port];
+	enum port p;
+
+	for_each_port_masked(p, (1 << port) - 1) {
+		struct ddi_vbt_port_info *i = &dev_priv->vbt.ddi_port_info[p];
+
+		if (info->alternate_ddc_pin != i->alternate_ddc_pin)
+			continue;
+
+		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(p), i->alternate_ddc_pin,
+			      port_name(port), port_name(p));
+
+		/*
+		 * If we have multiple ports supposedly sharing the
+		 * pin, then dvi/hdmi couldn't exist on the shared
+		 * port. Otherwise they share the same ddc bin and
+		 * system couldn't communicate with them separately.
+		 *
+		 * Due to parsing the ports in alphabetical order,
+		 * a higher port will always clobber a lower one.
+		 */
+		i->supports_dvi = false;
+		i->supports_hdmi = false;
+		i->alternate_ddc_pin = 0;
+	}
+}
+
+static void sanitize_aux_ch(struct drm_i915_private *dev_priv,
+			    enum port port)
+{
+	const struct ddi_vbt_port_info *info =
+		&dev_priv->vbt.ddi_port_info[port];
+	enum port p;
+
+	for_each_port_masked(p, (1 << port) - 1) {
+		struct ddi_vbt_port_info *i = &dev_priv->vbt.ddi_port_info[p];
+
+		if (info->alternate_aux_channel != i->alternate_aux_channel)
+			continue;
+
+		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(p), i->alternate_aux_channel,
+			      port_name(port), port_name(p));
+
+		/*
+		 * If we have multiple ports supposedlt sharing the
+		 * aux channel, then DP couldn't exist on the shared
+		 * port. Otherwise they share the same aux channel
+		 * and system couldn't communicate with them separately.
+		 *
+		 * Due to parsing the ports in alphabetical order,
+		 * a higher port will always clobber a lower one.
+		 */
+		i->supports_dp = false;
+		i->alternate_aux_channel = 0;
+	}
+}
+
 static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
 			   const struct bdb_header *bdb)
 {
@@ -1109,54 +1174,15 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
 		DRM_DEBUG_KMS("Port %c is internal DP\n", port_name(port));
 
 	if (is_dvi) {
-		if (port == PORT_E) {
-			info->alternate_ddc_pin = ddc_pin;
-			/* if DDIE share ddc pin with other port, then
-			 * dvi/hdmi couldn't exist on the shared port.
-			 * Otherwise they share the same ddc bin and system
-			 * couldn't communicate with them seperately. */
-			if (ddc_pin == DDC_PIN_B) {
-				dev_priv->vbt.ddi_port_info[PORT_B].supports_dvi = 0;
-				dev_priv->vbt.ddi_port_info[PORT_B].supports_hdmi = 0;
-			} else if (ddc_pin == DDC_PIN_C) {
-				dev_priv->vbt.ddi_port_info[PORT_C].supports_dvi = 0;
-				dev_priv->vbt.ddi_port_info[PORT_C].supports_hdmi = 0;
-			} else if (ddc_pin == DDC_PIN_D) {
-				dev_priv->vbt.ddi_port_info[PORT_D].supports_dvi = 0;
-				dev_priv->vbt.ddi_port_info[PORT_D].supports_hdmi = 0;
-			}
-		} else if (ddc_pin == DDC_PIN_B && port != PORT_B)
-			DRM_DEBUG_KMS("Unexpected DDC pin for port B\n");
-		else if (ddc_pin == DDC_PIN_C && port != PORT_C)
-			DRM_DEBUG_KMS("Unexpected DDC pin for port C\n");
-		else if (ddc_pin == DDC_PIN_D && port != PORT_D)
-			DRM_DEBUG_KMS("Unexpected DDC pin for port D\n");
+		info->alternate_ddc_pin = ddc_pin;
+
+		sanitize_ddc_pin(dev_priv, port);
 	}
 
 	if (is_dp) {
-		if (port == PORT_E) {
-			info->alternate_aux_channel = aux_channel;
-			/* if DDIE share aux channel with other port, then
-			 * DP couldn't exist on the shared port. Otherwise
-			 * they share the same aux channel and system
-			 * couldn't communicate with them seperately. */
-			if (aux_channel == DP_AUX_A)
-				dev_priv->vbt.ddi_port_info[PORT_A].supports_dp = 0;
-			else if (aux_channel == DP_AUX_B)
-				dev_priv->vbt.ddi_port_info[PORT_B].supports_dp = 0;
-			else if (aux_channel == DP_AUX_C)
-				dev_priv->vbt.ddi_port_info[PORT_C].supports_dp = 0;
-			else if (aux_channel == DP_AUX_D)
-				dev_priv->vbt.ddi_port_info[PORT_D].supports_dp = 0;
-		}
-		else if (aux_channel == DP_AUX_A && port != PORT_A)
-			DRM_DEBUG_KMS("Unexpected AUX channel for port A\n");
-		else if (aux_channel == DP_AUX_B && port != PORT_B)
-			DRM_DEBUG_KMS("Unexpected AUX channel for port B\n");
-		else if (aux_channel == DP_AUX_C && port != PORT_C)
-			DRM_DEBUG_KMS("Unexpected AUX channel for port C\n");
-		else if (aux_channel == DP_AUX_D && port != PORT_D)
-			DRM_DEBUG_KMS("Unexpected AUX channel for port D\n");
+		info->alternate_aux_channel = aux_channel;
+
+		sanitize_aux_ch(dev_priv, port);
 	}
 
 	if (bdb->version >= 158) {
-- 
2.7.4

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

Re: [Intel-gfx] [PATCH 3/4] drm/i915: Clean up DDI DDC/AUX CH sanitation

[Jim Bride]

On Tue, Oct 11, 2016 at 08:52:47PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> 
> Now that we use the AUX and GMBUS assignment from VBT for all ports,
> let's clean up the sanitization of the port information a bit.
> Previosuly we only did this for port E, and only complained about a
> non-standard assignment for the other ports. But as we know that
> non-standard assignments are a fact of life, let's expand the
> sanitization to all the ports.
> 
> v2: Include a commit message, fix up the comments a bit
> 
> Cc: stable@vger.kernel.org
> Cc: Maarten Maathuis <madman2003@gmail.com>
> Tested-by: Maarten Maathuis <madman2003@gmail.com>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=97877
> Signed-off-by: Ville Syrj�l� <ville.syrjala@linux.intel.com>

Reviewed-by: Jim Bride <jim.bride@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_bios.c | 116 +++++++++++++++++++++++---------------
>  1 file changed, 71 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 83667e8cdd6b..6d1ffa815e97 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1035,6 +1035,71 @@ static u8 translate_iboost(u8 val)
>  	return mapping[val];
>  }
>  
> +static void sanitize_ddc_pin(struct drm_i915_private *dev_priv,
> +			     enum port port)
> +{
> +	const struct ddi_vbt_port_info *info =
> +		&dev_priv->vbt.ddi_port_info[port];
> +	enum port p;
> +
> +	for_each_port_masked(p, (1 << port) - 1) {
> +		struct ddi_vbt_port_info *i = &dev_priv->vbt.ddi_port_info[p];
> +
> +		if (info->alternate_ddc_pin != i->alternate_ddc_pin)
> +			continue;
> +
> +		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(p), i->alternate_ddc_pin,
> +			      port_name(port), port_name(p));
> +
> +		/*
> +		 * If we have multiple ports supposedly sharing the
> +		 * pin, then dvi/hdmi couldn't exist on the shared
> +		 * port. Otherwise they share the same ddc bin and
> +		 * system couldn't communicate with them separately.
> +		 *
> +		 * Due to parsing the ports in alphabetical order,
> +		 * a higher port will always clobber a lower one.
> +		 */
> +		i->supports_dvi = false;
> +		i->supports_hdmi = false;
> +		i->alternate_ddc_pin = 0;
> +	}
> +}
> +
> +static void sanitize_aux_ch(struct drm_i915_private *dev_priv,
> +			    enum port port)
> +{
> +	const struct ddi_vbt_port_info *info =
> +		&dev_priv->vbt.ddi_port_info[port];
> +	enum port p;
> +
> +	for_each_port_masked(p, (1 << port) - 1) {
> +		struct ddi_vbt_port_info *i = &dev_priv->vbt.ddi_port_info[p];
> +
> +		if (info->alternate_aux_channel != i->alternate_aux_channel)
> +			continue;
> +
> +		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(p), i->alternate_aux_channel,
> +			      port_name(port), port_name(p));
> +
> +		/*
> +		 * If we have multiple ports supposedlt sharing the
> +		 * aux channel, then DP couldn't exist on the shared
> +		 * port. Otherwise they share the same aux channel
> +		 * and system couldn't communicate with them separately.
> +		 *
> +		 * Due to parsing the ports in alphabetical order,
> +		 * a higher port will always clobber a lower one.
> +		 */
> +		i->supports_dp = false;
> +		i->alternate_aux_channel = 0;
> +	}
> +}
> +
>  static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
>  			   const struct bdb_header *bdb)
>  {
> @@ -1109,54 +1174,15 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
>  		DRM_DEBUG_KMS("Port %c is internal DP\n", port_name(port));
>  
>  	if (is_dvi) {
> -		if (port == PORT_E) {
> -			info->alternate_ddc_pin = ddc_pin;
> -			/* if DDIE share ddc pin with other port, then
> -			 * dvi/hdmi couldn't exist on the shared port.
> -			 * Otherwise they share the same ddc bin and system
> -			 * couldn't communicate with them seperately. */
> -			if (ddc_pin == DDC_PIN_B) {
> -				dev_priv->vbt.ddi_port_info[PORT_B].supports_dvi = 0;
> -				dev_priv->vbt.ddi_port_info[PORT_B].supports_hdmi = 0;
> -			} else if (ddc_pin == DDC_PIN_C) {
> -				dev_priv->vbt.ddi_port_info[PORT_C].supports_dvi = 0;
> -				dev_priv->vbt.ddi_port_info[PORT_C].supports_hdmi = 0;
> -			} else if (ddc_pin == DDC_PIN_D) {
> -				dev_priv->vbt.ddi_port_info[PORT_D].supports_dvi = 0;
> -				dev_priv->vbt.ddi_port_info[PORT_D].supports_hdmi = 0;
> -			}
> -		} else if (ddc_pin == DDC_PIN_B && port != PORT_B)
> -			DRM_DEBUG_KMS("Unexpected DDC pin for port B\n");
> -		else if (ddc_pin == DDC_PIN_C && port != PORT_C)
> -			DRM_DEBUG_KMS("Unexpected DDC pin for port C\n");
> -		else if (ddc_pin == DDC_PIN_D && port != PORT_D)
> -			DRM_DEBUG_KMS("Unexpected DDC pin for port D\n");
> +		info->alternate_ddc_pin = ddc_pin;
> +
> +		sanitize_ddc_pin(dev_priv, port);
>  	}
>  
>  	if (is_dp) {
> -		if (port == PORT_E) {
> -			info->alternate_aux_channel = aux_channel;
> -			/* if DDIE share aux channel with other port, then
> -			 * DP couldn't exist on the shared port. Otherwise
> -			 * they share the same aux channel and system
> -			 * couldn't communicate with them seperately. */
> -			if (aux_channel == DP_AUX_A)
> -				dev_priv->vbt.ddi_port_info[PORT_A].supports_dp = 0;
> -			else if (aux_channel == DP_AUX_B)
> -				dev_priv->vbt.ddi_port_info[PORT_B].supports_dp = 0;
> -			else if (aux_channel == DP_AUX_C)
> -				dev_priv->vbt.ddi_port_info[PORT_C].supports_dp = 0;
> -			else if (aux_channel == DP_AUX_D)
> -				dev_priv->vbt.ddi_port_info[PORT_D].supports_dp = 0;
> -		}
> -		else if (aux_channel == DP_AUX_A && port != PORT_A)
> -			DRM_DEBUG_KMS("Unexpected AUX channel for port A\n");
> -		else if (aux_channel == DP_AUX_B && port != PORT_B)
> -			DRM_DEBUG_KMS("Unexpected AUX channel for port B\n");
> -		else if (aux_channel == DP_AUX_C && port != PORT_C)
> -			DRM_DEBUG_KMS("Unexpected AUX channel for port C\n");
> -		else if (aux_channel == DP_AUX_D && port != PORT_D)
> -			DRM_DEBUG_KMS("Unexpected AUX channel for port D\n");
> +		info->alternate_aux_channel = aux_channel;
> +
> +		sanitize_aux_ch(dev_priv, port);
>  	}
>  
>  	if (bdb->version >= 158) {
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/4] drm/i915: Clean up DDI DDC/AUX CH sanitation

[Jim Bride]

On Tue, Oct 11, 2016 at 08:52:47PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Now that we use the AUX and GMBUS assignment from VBT for all ports,
> let's clean up the sanitization of the port information a bit.
> Previosuly we only did this for port E, and only complained about a
> non-standard assignment for the other ports. But as we know that
> non-standard assignments are a fact of life, let's expand the
> sanitization to all the ports.
> 
> v2: Include a commit message, fix up the comments a bit
> 
> Cc: stable@vger.kernel.org
> Cc: Maarten Maathuis <madman2003@gmail.com>
> Tested-by: Maarten Maathuis <madman2003@gmail.com>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=97877
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Jim Bride <jim.bride@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_bios.c | 116 +++++++++++++++++++++++---------------
>  1 file changed, 71 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 83667e8cdd6b..6d1ffa815e97 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1035,6 +1035,71 @@ static u8 translate_iboost(u8 val)
>  	return mapping[val];
>  }
>  
> +static void sanitize_ddc_pin(struct drm_i915_private *dev_priv,
> +			     enum port port)
> +{
> +	const struct ddi_vbt_port_info *info =
> +		&dev_priv->vbt.ddi_port_info[port];
> +	enum port p;
> +
> +	for_each_port_masked(p, (1 << port) - 1) {
> +		struct ddi_vbt_port_info *i = &dev_priv->vbt.ddi_port_info[p];
> +
> +		if (info->alternate_ddc_pin != i->alternate_ddc_pin)
> +			continue;
> +
> +		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(p), i->alternate_ddc_pin,
> +			      port_name(port), port_name(p));
> +
> +		/*
> +		 * If we have multiple ports supposedly sharing the
> +		 * pin, then dvi/hdmi couldn't exist on the shared
> +		 * port. Otherwise they share the same ddc bin and
> +		 * system couldn't communicate with them separately.
> +		 *
> +		 * Due to parsing the ports in alphabetical order,
> +		 * a higher port will always clobber a lower one.
> +		 */
> +		i->supports_dvi = false;
> +		i->supports_hdmi = false;
> +		i->alternate_ddc_pin = 0;
> +	}
> +}
> +
> +static void sanitize_aux_ch(struct drm_i915_private *dev_priv,
> +			    enum port port)
> +{
> +	const struct ddi_vbt_port_info *info =
> +		&dev_priv->vbt.ddi_port_info[port];
> +	enum port p;
> +
> +	for_each_port_masked(p, (1 << port) - 1) {
> +		struct ddi_vbt_port_info *i = &dev_priv->vbt.ddi_port_info[p];
> +
> +		if (info->alternate_aux_channel != i->alternate_aux_channel)
> +			continue;
> +
> +		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(p), i->alternate_aux_channel,
> +			      port_name(port), port_name(p));
> +
> +		/*
> +		 * If we have multiple ports supposedlt sharing the
> +		 * aux channel, then DP couldn't exist on the shared
> +		 * port. Otherwise they share the same aux channel
> +		 * and system couldn't communicate with them separately.
> +		 *
> +		 * Due to parsing the ports in alphabetical order,
> +		 * a higher port will always clobber a lower one.
> +		 */
> +		i->supports_dp = false;
> +		i->alternate_aux_channel = 0;
> +	}
> +}
> +
>  static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
>  			   const struct bdb_header *bdb)
>  {
> @@ -1109,54 +1174,15 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
>  		DRM_DEBUG_KMS("Port %c is internal DP\n", port_name(port));
>  
>  	if (is_dvi) {
> -		if (port == PORT_E) {
> -			info->alternate_ddc_pin = ddc_pin;
> -			/* if DDIE share ddc pin with other port, then
> -			 * dvi/hdmi couldn't exist on the shared port.
> -			 * Otherwise they share the same ddc bin and system
> -			 * couldn't communicate with them seperately. */
> -			if (ddc_pin == DDC_PIN_B) {
> -				dev_priv->vbt.ddi_port_info[PORT_B].supports_dvi = 0;
> -				dev_priv->vbt.ddi_port_info[PORT_B].supports_hdmi = 0;
> -			} else if (ddc_pin == DDC_PIN_C) {
> -				dev_priv->vbt.ddi_port_info[PORT_C].supports_dvi = 0;
> -				dev_priv->vbt.ddi_port_info[PORT_C].supports_hdmi = 0;
> -			} else if (ddc_pin == DDC_PIN_D) {
> -				dev_priv->vbt.ddi_port_info[PORT_D].supports_dvi = 0;
> -				dev_priv->vbt.ddi_port_info[PORT_D].supports_hdmi = 0;
> -			}
> -		} else if (ddc_pin == DDC_PIN_B && port != PORT_B)
> -			DRM_DEBUG_KMS("Unexpected DDC pin for port B\n");
> -		else if (ddc_pin == DDC_PIN_C && port != PORT_C)
> -			DRM_DEBUG_KMS("Unexpected DDC pin for port C\n");
> -		else if (ddc_pin == DDC_PIN_D && port != PORT_D)
> -			DRM_DEBUG_KMS("Unexpected DDC pin for port D\n");
> +		info->alternate_ddc_pin = ddc_pin;
> +
> +		sanitize_ddc_pin(dev_priv, port);
>  	}
>  
>  	if (is_dp) {
> -		if (port == PORT_E) {
> -			info->alternate_aux_channel = aux_channel;
> -			/* if DDIE share aux channel with other port, then
> -			 * DP couldn't exist on the shared port. Otherwise
> -			 * they share the same aux channel and system
> -			 * couldn't communicate with them seperately. */
> -			if (aux_channel == DP_AUX_A)
> -				dev_priv->vbt.ddi_port_info[PORT_A].supports_dp = 0;
> -			else if (aux_channel == DP_AUX_B)
> -				dev_priv->vbt.ddi_port_info[PORT_B].supports_dp = 0;
> -			else if (aux_channel == DP_AUX_C)
> -				dev_priv->vbt.ddi_port_info[PORT_C].supports_dp = 0;
> -			else if (aux_channel == DP_AUX_D)
> -				dev_priv->vbt.ddi_port_info[PORT_D].supports_dp = 0;
> -		}
> -		else if (aux_channel == DP_AUX_A && port != PORT_A)
> -			DRM_DEBUG_KMS("Unexpected AUX channel for port A\n");
> -		else if (aux_channel == DP_AUX_B && port != PORT_B)
> -			DRM_DEBUG_KMS("Unexpected AUX channel for port B\n");
> -		else if (aux_channel == DP_AUX_C && port != PORT_C)
> -			DRM_DEBUG_KMS("Unexpected AUX channel for port C\n");
> -		else if (aux_channel == DP_AUX_D && port != PORT_D)
> -			DRM_DEBUG_KMS("Unexpected AUX channel for port D\n");
> +		info->alternate_aux_channel = aux_channel;
> +
> +		sanitize_aux_ch(dev_priv, port);
>  	}
>  
>  	if (bdb->version >= 158) {
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/4] drm/i915: Clean up DDI DDC/AUX CH sanitation

[Ville Syrjälä]

On Tue, Oct 11, 2016 at 08:52:47PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> 
> Now that we use the AUX and GMBUS assignment from VBT for all ports,
> let's clean up the sanitization of the port information a bit.
> Previosuly we only did this for port E, and only complained about a
> non-standard assignment for the other ports. But as we know that
> non-standard assignments are a fact of life, let's expand the
> sanitization to all the ports.
> 
> v2: Include a commit message, fix up the comments a bit
> 
> Cc: stable@vger.kernel.org
> Cc: Maarten Maathuis <madman2003@gmail.com>
> Tested-by: Maarten Maathuis <madman2003@gmail.com>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=97877
> Signed-off-by: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_bios.c | 116 +++++++++++++++++++++++---------------
>  1 file changed, 71 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 83667e8cdd6b..6d1ffa815e97 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1035,6 +1035,71 @@ static u8 translate_iboost(u8 val)
>  	return mapping[val];
>  }
>  
> +static void sanitize_ddc_pin(struct drm_i915_private *dev_priv,
> +			     enum port port)
> +{
> +	const struct ddi_vbt_port_info *info =
> +		&dev_priv->vbt.ddi_port_info[port];
> +	enum port p;
> +

Hmm. And I just realized this code is probably slightly garbage. If the
current port doesn't have alternate_ddc_pin/alternate_aux_channel, then
we'd proceed to clobber any lower port without the thing as well. Now,
potentially all DDI platforms have this stuff fully decked out in VBT so
it might not matter in practice. But better safe that sorry, so I'll
send out a new version of this patch that checks here for the zero
case in both of these functions.

> +	for_each_port_masked(p, (1 << port) - 1) {
> +		struct ddi_vbt_port_info *i = &dev_priv->vbt.ddi_port_info[p];
> +
> +		if (info->alternate_ddc_pin != i->alternate_ddc_pin)
> +			continue;
> +
> +		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(p), i->alternate_ddc_pin,
> +			      port_name(port), port_name(p));
> +
> +		/*
> +		 * If we have multiple ports supposedly sharing the
> +		 * pin, then dvi/hdmi couldn't exist on the shared
> +		 * port. Otherwise they share the same ddc bin and
> +		 * system couldn't communicate with them separately.
> +		 *
> +		 * Due to parsing the ports in alphabetical order,
> +		 * a higher port will always clobber a lower one.
> +		 */
> +		i->supports_dvi = false;
> +		i->supports_hdmi = false;
> +		i->alternate_ddc_pin = 0;
> +	}
> +}
> +
> +static void sanitize_aux_ch(struct drm_i915_private *dev_priv,
> +			    enum port port)
> +{
> +	const struct ddi_vbt_port_info *info =
> +		&dev_priv->vbt.ddi_port_info[port];
> +	enum port p;
> +
> +	for_each_port_masked(p, (1 << port) - 1) {
> +		struct ddi_vbt_port_info *i = &dev_priv->vbt.ddi_port_info[p];
> +
> +		if (info->alternate_aux_channel != i->alternate_aux_channel)
> +			continue;
> +
> +		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(p), i->alternate_aux_channel,
> +			      port_name(port), port_name(p));
> +
> +		/*
> +		 * If we have multiple ports supposedlt sharing the
> +		 * aux channel, then DP couldn't exist on the shared
> +		 * port. Otherwise they share the same aux channel
> +		 * and system couldn't communicate with them separately.
> +		 *
> +		 * Due to parsing the ports in alphabetical order,
> +		 * a higher port will always clobber a lower one.
> +		 */
> +		i->supports_dp = false;
> +		i->alternate_aux_channel = 0;
> +	}
> +}
> +
>  static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
>  			   const struct bdb_header *bdb)
>  {
> @@ -1109,54 +1174,15 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
>  		DRM_DEBUG_KMS("Port %c is internal DP\n", port_name(port));
>  
>  	if (is_dvi) {
> -		if (port == PORT_E) {
> -			info->alternate_ddc_pin = ddc_pin;
> -			/* if DDIE share ddc pin with other port, then
> -			 * dvi/hdmi couldn't exist on the shared port.
> -			 * Otherwise they share the same ddc bin and system
> -			 * couldn't communicate with them seperately. */
> -			if (ddc_pin == DDC_PIN_B) {
> -				dev_priv->vbt.ddi_port_info[PORT_B].supports_dvi = 0;
> -				dev_priv->vbt.ddi_port_info[PORT_B].supports_hdmi = 0;
> -			} else if (ddc_pin == DDC_PIN_C) {
> -				dev_priv->vbt.ddi_port_info[PORT_C].supports_dvi = 0;
> -				dev_priv->vbt.ddi_port_info[PORT_C].supports_hdmi = 0;
> -			} else if (ddc_pin == DDC_PIN_D) {
> -				dev_priv->vbt.ddi_port_info[PORT_D].supports_dvi = 0;
> -				dev_priv->vbt.ddi_port_info[PORT_D].supports_hdmi = 0;
> -			}
> -		} else if (ddc_pin == DDC_PIN_B && port != PORT_B)
> -			DRM_DEBUG_KMS("Unexpected DDC pin for port B\n");
> -		else if (ddc_pin == DDC_PIN_C && port != PORT_C)
> -			DRM_DEBUG_KMS("Unexpected DDC pin for port C\n");
> -		else if (ddc_pin == DDC_PIN_D && port != PORT_D)
> -			DRM_DEBUG_KMS("Unexpected DDC pin for port D\n");
> +		info->alternate_ddc_pin = ddc_pin;
> +
> +		sanitize_ddc_pin(dev_priv, port);
>  	}
>  
>  	if (is_dp) {
> -		if (port == PORT_E) {
> -			info->alternate_aux_channel = aux_channel;
> -			/* if DDIE share aux channel with other port, then
> -			 * DP couldn't exist on the shared port. Otherwise
> -			 * they share the same aux channel and system
> -			 * couldn't communicate with them seperately. */
> -			if (aux_channel == DP_AUX_A)
> -				dev_priv->vbt.ddi_port_info[PORT_A].supports_dp = 0;
> -			else if (aux_channel == DP_AUX_B)
> -				dev_priv->vbt.ddi_port_info[PORT_B].supports_dp = 0;
> -			else if (aux_channel == DP_AUX_C)
> -				dev_priv->vbt.ddi_port_info[PORT_C].supports_dp = 0;
> -			else if (aux_channel == DP_AUX_D)
> -				dev_priv->vbt.ddi_port_info[PORT_D].supports_dp = 0;
> -		}
> -		else if (aux_channel == DP_AUX_A && port != PORT_A)
> -			DRM_DEBUG_KMS("Unexpected AUX channel for port A\n");
> -		else if (aux_channel == DP_AUX_B && port != PORT_B)
> -			DRM_DEBUG_KMS("Unexpected AUX channel for port B\n");
> -		else if (aux_channel == DP_AUX_C && port != PORT_C)
> -			DRM_DEBUG_KMS("Unexpected AUX channel for port C\n");
> -		else if (aux_channel == DP_AUX_D && port != PORT_D)
> -			DRM_DEBUG_KMS("Unexpected AUX channel for port D\n");
> +		info->alternate_aux_channel = aux_channel;
> +
> +		sanitize_aux_ch(dev_priv, port);
>  	}
>  
>  	if (bdb->version >= 158) {
> -- 
> 2.7.4

-- 
Ville Syrj�l�
Intel OTC

Re: [PATCH 3/4] drm/i915: Clean up DDI DDC/AUX CH sanitation

[Ville Syrjälä]

On Tue, Oct 11, 2016 at 08:52:47PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Now that we use the AUX and GMBUS assignment from VBT for all ports,
> let's clean up the sanitization of the port information a bit.
> Previosuly we only did this for port E, and only complained about a
> non-standard assignment for the other ports. But as we know that
> non-standard assignments are a fact of life, let's expand the
> sanitization to all the ports.
> 
> v2: Include a commit message, fix up the comments a bit
> 
> Cc: stable@vger.kernel.org
> Cc: Maarten Maathuis <madman2003@gmail.com>
> Tested-by: Maarten Maathuis <madman2003@gmail.com>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=97877
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_bios.c | 116 +++++++++++++++++++++++---------------
>  1 file changed, 71 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 83667e8cdd6b..6d1ffa815e97 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1035,6 +1035,71 @@ static u8 translate_iboost(u8 val)
>  	return mapping[val];
>  }
>  
> +static void sanitize_ddc_pin(struct drm_i915_private *dev_priv,
> +			     enum port port)
> +{
> +	const struct ddi_vbt_port_info *info =
> +		&dev_priv->vbt.ddi_port_info[port];
> +	enum port p;
> +

Hmm. And I just realized this code is probably slightly garbage. If the
current port doesn't have alternate_ddc_pin/alternate_aux_channel, then
we'd proceed to clobber any lower port without the thing as well. Now,
potentially all DDI platforms have this stuff fully decked out in VBT so
it might not matter in practice. But better safe that sorry, so I'll
send out a new version of this patch that checks here for the zero
case in both of these functions.

> +	for_each_port_masked(p, (1 << port) - 1) {
> +		struct ddi_vbt_port_info *i = &dev_priv->vbt.ddi_port_info[p];
> +
> +		if (info->alternate_ddc_pin != i->alternate_ddc_pin)
> +			continue;
> +
> +		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(p), i->alternate_ddc_pin,
> +			      port_name(port), port_name(p));
> +
> +		/*
> +		 * If we have multiple ports supposedly sharing the
> +		 * pin, then dvi/hdmi couldn't exist on the shared
> +		 * port. Otherwise they share the same ddc bin and
> +		 * system couldn't communicate with them separately.
> +		 *
> +		 * Due to parsing the ports in alphabetical order,
> +		 * a higher port will always clobber a lower one.
> +		 */
> +		i->supports_dvi = false;
> +		i->supports_hdmi = false;
> +		i->alternate_ddc_pin = 0;
> +	}
> +}
> +
> +static void sanitize_aux_ch(struct drm_i915_private *dev_priv,
> +			    enum port port)
> +{
> +	const struct ddi_vbt_port_info *info =
> +		&dev_priv->vbt.ddi_port_info[port];
> +	enum port p;
> +
> +	for_each_port_masked(p, (1 << port) - 1) {
> +		struct ddi_vbt_port_info *i = &dev_priv->vbt.ddi_port_info[p];
> +
> +		if (info->alternate_aux_channel != i->alternate_aux_channel)
> +			continue;
> +
> +		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(p), i->alternate_aux_channel,
> +			      port_name(port), port_name(p));
> +
> +		/*
> +		 * If we have multiple ports supposedlt sharing the
> +		 * aux channel, then DP couldn't exist on the shared
> +		 * port. Otherwise they share the same aux channel
> +		 * and system couldn't communicate with them separately.
> +		 *
> +		 * Due to parsing the ports in alphabetical order,
> +		 * a higher port will always clobber a lower one.
> +		 */
> +		i->supports_dp = false;
> +		i->alternate_aux_channel = 0;
> +	}
> +}
> +
>  static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
>  			   const struct bdb_header *bdb)
>  {
> @@ -1109,54 +1174,15 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
>  		DRM_DEBUG_KMS("Port %c is internal DP\n", port_name(port));
>  
>  	if (is_dvi) {
> -		if (port == PORT_E) {
> -			info->alternate_ddc_pin = ddc_pin;
> -			/* if DDIE share ddc pin with other port, then
> -			 * dvi/hdmi couldn't exist on the shared port.
> -			 * Otherwise they share the same ddc bin and system
> -			 * couldn't communicate with them seperately. */
> -			if (ddc_pin == DDC_PIN_B) {
> -				dev_priv->vbt.ddi_port_info[PORT_B].supports_dvi = 0;
> -				dev_priv->vbt.ddi_port_info[PORT_B].supports_hdmi = 0;
> -			} else if (ddc_pin == DDC_PIN_C) {
> -				dev_priv->vbt.ddi_port_info[PORT_C].supports_dvi = 0;
> -				dev_priv->vbt.ddi_port_info[PORT_C].supports_hdmi = 0;
> -			} else if (ddc_pin == DDC_PIN_D) {
> -				dev_priv->vbt.ddi_port_info[PORT_D].supports_dvi = 0;
> -				dev_priv->vbt.ddi_port_info[PORT_D].supports_hdmi = 0;
> -			}
> -		} else if (ddc_pin == DDC_PIN_B && port != PORT_B)
> -			DRM_DEBUG_KMS("Unexpected DDC pin for port B\n");
> -		else if (ddc_pin == DDC_PIN_C && port != PORT_C)
> -			DRM_DEBUG_KMS("Unexpected DDC pin for port C\n");
> -		else if (ddc_pin == DDC_PIN_D && port != PORT_D)
> -			DRM_DEBUG_KMS("Unexpected DDC pin for port D\n");
> +		info->alternate_ddc_pin = ddc_pin;
> +
> +		sanitize_ddc_pin(dev_priv, port);
>  	}
>  
>  	if (is_dp) {
> -		if (port == PORT_E) {
> -			info->alternate_aux_channel = aux_channel;
> -			/* if DDIE share aux channel with other port, then
> -			 * DP couldn't exist on the shared port. Otherwise
> -			 * they share the same aux channel and system
> -			 * couldn't communicate with them seperately. */
> -			if (aux_channel == DP_AUX_A)
> -				dev_priv->vbt.ddi_port_info[PORT_A].supports_dp = 0;
> -			else if (aux_channel == DP_AUX_B)
> -				dev_priv->vbt.ddi_port_info[PORT_B].supports_dp = 0;
> -			else if (aux_channel == DP_AUX_C)
> -				dev_priv->vbt.ddi_port_info[PORT_C].supports_dp = 0;
> -			else if (aux_channel == DP_AUX_D)
> -				dev_priv->vbt.ddi_port_info[PORT_D].supports_dp = 0;
> -		}
> -		else if (aux_channel == DP_AUX_A && port != PORT_A)
> -			DRM_DEBUG_KMS("Unexpected AUX channel for port A\n");
> -		else if (aux_channel == DP_AUX_B && port != PORT_B)
> -			DRM_DEBUG_KMS("Unexpected AUX channel for port B\n");
> -		else if (aux_channel == DP_AUX_C && port != PORT_C)
> -			DRM_DEBUG_KMS("Unexpected AUX channel for port C\n");
> -		else if (aux_channel == DP_AUX_D && port != PORT_D)
> -			DRM_DEBUG_KMS("Unexpected AUX channel for port D\n");
> +		info->alternate_aux_channel = aux_channel;
> +
> +		sanitize_aux_ch(dev_priv, port);
>  	}
>  
>  	if (bdb->version >= 158) {
> -- 
> 2.7.4

-- 
Ville Syrjälä
Intel OTC

[PATCH v3 3/4] drm/i915: Clean up DDI DDC/AUX CH sanitation

[ville.syrjala]

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

Now that we use the AUX and GMBUS assignment from VBT for all ports,
let's clean up the sanitization of the port information a bit.
Previosuly we only did this for port E, and only complained about a
non-standard assignment for the other ports. But as we know that
non-standard assignments are a fact of life, let's expand the
sanitization to all the ports.

v2: Include a commit message, fix up the comments a bit
v3: Don't clobber other ports if the current port has no alternate aux ch/ddc pin

Cc: stable@vger.kernel.org
Cc: Maarten Maathuis <madman2003@gmail.com>
Tested-by: Maarten Maathuis <madman2003@gmail.com>
References: https://bugs.freedesktop.org/show_bug.cgi?id=97877
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1476208368-5710-4-git-send-email-ville.syrjala@linux.intel.com
Reviewed-by: Jim Bride <jim.bride@linux.intel.com> (v2)
---
 drivers/gpu/drm/i915/intel_bios.c | 122 ++++++++++++++++++++++++--------------
 1 file changed, 77 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 83667e8cdd6b..a8ff8c099685 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1035,6 +1035,77 @@ static u8 translate_iboost(u8 val)
 	return mapping[val];
 }
 
+static void sanitize_ddc_pin(struct drm_i915_private *dev_priv,
+			     enum port port)
+{
+	const struct ddi_vbt_port_info *info =
+		&dev_priv->vbt.ddi_port_info[port];
+	enum port p;
+
+	if (!info->alternate_ddc_pin)
+		return;
+
+	for_each_port_masked(p, (1 << port) - 1) {
+		struct ddi_vbt_port_info *i = &dev_priv->vbt.ddi_port_info[p];
+
+		if (info->alternate_ddc_pin != i->alternate_ddc_pin)
+			continue;
+
+		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(p), i->alternate_ddc_pin,
+			      port_name(port), port_name(p));
+
+		/*
+		 * If we have multiple ports supposedly sharing the
+		 * pin, then dvi/hdmi couldn't exist on the shared
+		 * port. Otherwise they share the same ddc bin and
+		 * system couldn't communicate with them separately.
+		 *
+		 * Due to parsing the ports in alphabetical order,
+		 * a higher port will always clobber a lower one.
+		 */
+		i->supports_dvi = false;
+		i->supports_hdmi = false;
+		i->alternate_ddc_pin = 0;
+	}
+}
+
+static void sanitize_aux_ch(struct drm_i915_private *dev_priv,
+			    enum port port)
+{
+	const struct ddi_vbt_port_info *info =
+		&dev_priv->vbt.ddi_port_info[port];
+	enum port p;
+
+	if (!info->alternate_aux_channel)
+		return;
+
+	for_each_port_masked(p, (1 << port) - 1) {
+		struct ddi_vbt_port_info *i = &dev_priv->vbt.ddi_port_info[p];
+
+		if (info->alternate_aux_channel != i->alternate_aux_channel)
+			continue;
+
+		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(p), i->alternate_aux_channel,
+			      port_name(port), port_name(p));
+
+		/*
+		 * If we have multiple ports supposedlt sharing the
+		 * aux channel, then DP couldn't exist on the shared
+		 * port. Otherwise they share the same aux channel
+		 * and system couldn't communicate with them separately.
+		 *
+		 * Due to parsing the ports in alphabetical order,
+		 * a higher port will always clobber a lower one.
+		 */
+		i->supports_dp = false;
+		i->alternate_aux_channel = 0;
+	}
+}
+
 static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
 			   const struct bdb_header *bdb)
 {
@@ -1109,54 +1180,15 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
 		DRM_DEBUG_KMS("Port %c is internal DP\n", port_name(port));
 
 	if (is_dvi) {
-		if (port == PORT_E) {
-			info->alternate_ddc_pin = ddc_pin;
-			/* if DDIE share ddc pin with other port, then
-			 * dvi/hdmi couldn't exist on the shared port.
-			 * Otherwise they share the same ddc bin and system
-			 * couldn't communicate with them seperately. */
-			if (ddc_pin == DDC_PIN_B) {
-				dev_priv->vbt.ddi_port_info[PORT_B].supports_dvi = 0;
-				dev_priv->vbt.ddi_port_info[PORT_B].supports_hdmi = 0;
-			} else if (ddc_pin == DDC_PIN_C) {
-				dev_priv->vbt.ddi_port_info[PORT_C].supports_dvi = 0;
-				dev_priv->vbt.ddi_port_info[PORT_C].supports_hdmi = 0;
-			} else if (ddc_pin == DDC_PIN_D) {
-				dev_priv->vbt.ddi_port_info[PORT_D].supports_dvi = 0;
-				dev_priv->vbt.ddi_port_info[PORT_D].supports_hdmi = 0;
-			}
-		} else if (ddc_pin == DDC_PIN_B && port != PORT_B)
-			DRM_DEBUG_KMS("Unexpected DDC pin for port B\n");
-		else if (ddc_pin == DDC_PIN_C && port != PORT_C)
-			DRM_DEBUG_KMS("Unexpected DDC pin for port C\n");
-		else if (ddc_pin == DDC_PIN_D && port != PORT_D)
-			DRM_DEBUG_KMS("Unexpected DDC pin for port D\n");
+		info->alternate_ddc_pin = ddc_pin;
+
+		sanitize_ddc_pin(dev_priv, port);
 	}
 
 	if (is_dp) {
-		if (port == PORT_E) {
-			info->alternate_aux_channel = aux_channel;
-			/* if DDIE share aux channel with other port, then
-			 * DP couldn't exist on the shared port. Otherwise
-			 * they share the same aux channel and system
-			 * couldn't communicate with them seperately. */
-			if (aux_channel == DP_AUX_A)
-				dev_priv->vbt.ddi_port_info[PORT_A].supports_dp = 0;
-			else if (aux_channel == DP_AUX_B)
-				dev_priv->vbt.ddi_port_info[PORT_B].supports_dp = 0;
-			else if (aux_channel == DP_AUX_C)
-				dev_priv->vbt.ddi_port_info[PORT_C].supports_dp = 0;
-			else if (aux_channel == DP_AUX_D)
-				dev_priv->vbt.ddi_port_info[PORT_D].supports_dp = 0;
-		}
-		else if (aux_channel == DP_AUX_A && port != PORT_A)
-			DRM_DEBUG_KMS("Unexpected AUX channel for port A\n");
-		else if (aux_channel == DP_AUX_B && port != PORT_B)
-			DRM_DEBUG_KMS("Unexpected AUX channel for port B\n");
-		else if (aux_channel == DP_AUX_C && port != PORT_C)
-			DRM_DEBUG_KMS("Unexpected AUX channel for port C\n");
-		else if (aux_channel == DP_AUX_D && port != PORT_D)
-			DRM_DEBUG_KMS("Unexpected AUX channel for port D\n");
+		info->alternate_aux_channel = aux_channel;
+
+		sanitize_aux_ch(dev_priv, port);
 	}
 
 	if (bdb->version >= 158) {
-- 
2.7.4

[PATCH v3 3/4] drm/i915: Clean up DDI DDC/AUX CH sanitation

[ville.syrjala]

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

Now that we use the AUX and GMBUS assignment from VBT for all ports,
let's clean up the sanitization of the port information a bit.
Previosuly we only did this for port E, and only complained about a
non-standard assignment for the other ports. But as we know that
non-standard assignments are a fact of life, let's expand the
sanitization to all the ports.

v2: Include a commit message, fix up the comments a bit
v3: Don't clobber other ports if the current port has no alternate aux ch/ddc pin

Cc: stable@vger.kernel.org
Cc: Maarten Maathuis <madman2003@gmail.com>
Tested-by: Maarten Maathuis <madman2003@gmail.com>
References: https://bugs.freedesktop.org/show_bug.cgi?id=97877
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1476208368-5710-4-git-send-email-ville.syrjala@linux.intel.com
Reviewed-by: Jim Bride <jim.bride@linux.intel.com> (v2)
---
 drivers/gpu/drm/i915/intel_bios.c | 122 ++++++++++++++++++++++++--------------
 1 file changed, 77 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 83667e8cdd6b..a8ff8c099685 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1035,6 +1035,77 @@ static u8 translate_iboost(u8 val)
 	return mapping[val];
 }
 
+static void sanitize_ddc_pin(struct drm_i915_private *dev_priv,
+			     enum port port)
+{
+	const struct ddi_vbt_port_info *info =
+		&dev_priv->vbt.ddi_port_info[port];
+	enum port p;
+
+	if (!info->alternate_ddc_pin)
+		return;
+
+	for_each_port_masked(p, (1 << port) - 1) {
+		struct ddi_vbt_port_info *i = &dev_priv->vbt.ddi_port_info[p];
+
+		if (info->alternate_ddc_pin != i->alternate_ddc_pin)
+			continue;
+
+		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(p), i->alternate_ddc_pin,
+			      port_name(port), port_name(p));
+
+		/*
+		 * If we have multiple ports supposedly sharing the
+		 * pin, then dvi/hdmi couldn't exist on the shared
+		 * port. Otherwise they share the same ddc bin and
+		 * system couldn't communicate with them separately.
+		 *
+		 * Due to parsing the ports in alphabetical order,
+		 * a higher port will always clobber a lower one.
+		 */
+		i->supports_dvi = false;
+		i->supports_hdmi = false;
+		i->alternate_ddc_pin = 0;
+	}
+}
+
+static void sanitize_aux_ch(struct drm_i915_private *dev_priv,
+			    enum port port)
+{
+	const struct ddi_vbt_port_info *info =
+		&dev_priv->vbt.ddi_port_info[port];
+	enum port p;
+
+	if (!info->alternate_aux_channel)
+		return;
+
+	for_each_port_masked(p, (1 << port) - 1) {
+		struct ddi_vbt_port_info *i = &dev_priv->vbt.ddi_port_info[p];
+
+		if (info->alternate_aux_channel != i->alternate_aux_channel)
+			continue;
+
+		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(p), i->alternate_aux_channel,
+			      port_name(port), port_name(p));
+
+		/*
+		 * If we have multiple ports supposedlt sharing the
+		 * aux channel, then DP couldn't exist on the shared
+		 * port. Otherwise they share the same aux channel
+		 * and system couldn't communicate with them separately.
+		 *
+		 * Due to parsing the ports in alphabetical order,
+		 * a higher port will always clobber a lower one.
+		 */
+		i->supports_dp = false;
+		i->alternate_aux_channel = 0;
+	}
+}
+
 static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
 			   const struct bdb_header *bdb)
 {
@@ -1109,54 +1180,15 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
 		DRM_DEBUG_KMS("Port %c is internal DP\n", port_name(port));
 
 	if (is_dvi) {
-		if (port == PORT_E) {
-			info->alternate_ddc_pin = ddc_pin;
-			/* if DDIE share ddc pin with other port, then
-			 * dvi/hdmi couldn't exist on the shared port.
-			 * Otherwise they share the same ddc bin and system
-			 * couldn't communicate with them seperately. */
-			if (ddc_pin == DDC_PIN_B) {
-				dev_priv->vbt.ddi_port_info[PORT_B].supports_dvi = 0;
-				dev_priv->vbt.ddi_port_info[PORT_B].supports_hdmi = 0;
-			} else if (ddc_pin == DDC_PIN_C) {
-				dev_priv->vbt.ddi_port_info[PORT_C].supports_dvi = 0;
-				dev_priv->vbt.ddi_port_info[PORT_C].supports_hdmi = 0;
-			} else if (ddc_pin == DDC_PIN_D) {
-				dev_priv->vbt.ddi_port_info[PORT_D].supports_dvi = 0;
-				dev_priv->vbt.ddi_port_info[PORT_D].supports_hdmi = 0;
-			}
-		} else if (ddc_pin == DDC_PIN_B && port != PORT_B)
-			DRM_DEBUG_KMS("Unexpected DDC pin for port B\n");
-		else if (ddc_pin == DDC_PIN_C && port != PORT_C)
-			DRM_DEBUG_KMS("Unexpected DDC pin for port C\n");
-		else if (ddc_pin == DDC_PIN_D && port != PORT_D)
-			DRM_DEBUG_KMS("Unexpected DDC pin for port D\n");
+		info->alternate_ddc_pin = ddc_pin;
+
+		sanitize_ddc_pin(dev_priv, port);
 	}
 
 	if (is_dp) {
-		if (port == PORT_E) {
-			info->alternate_aux_channel = aux_channel;
-			/* if DDIE share aux channel with other port, then
-			 * DP couldn't exist on the shared port. Otherwise
-			 * they share the same aux channel and system
-			 * couldn't communicate with them seperately. */
-			if (aux_channel == DP_AUX_A)
-				dev_priv->vbt.ddi_port_info[PORT_A].supports_dp = 0;
-			else if (aux_channel == DP_AUX_B)
-				dev_priv->vbt.ddi_port_info[PORT_B].supports_dp = 0;
-			else if (aux_channel == DP_AUX_C)
-				dev_priv->vbt.ddi_port_info[PORT_C].supports_dp = 0;
-			else if (aux_channel == DP_AUX_D)
-				dev_priv->vbt.ddi_port_info[PORT_D].supports_dp = 0;
-		}
-		else if (aux_channel == DP_AUX_A && port != PORT_A)
-			DRM_DEBUG_KMS("Unexpected AUX channel for port A\n");
-		else if (aux_channel == DP_AUX_B && port != PORT_B)
-			DRM_DEBUG_KMS("Unexpected AUX channel for port B\n");
-		else if (aux_channel == DP_AUX_C && port != PORT_C)
-			DRM_DEBUG_KMS("Unexpected AUX channel for port C\n");
-		else if (aux_channel == DP_AUX_D && port != PORT_D)
-			DRM_DEBUG_KMS("Unexpected AUX channel for port D\n");
+		info->alternate_aux_channel = aux_channel;
+
+		sanitize_aux_ch(dev_priv, port);
 	}
 
 	if (bdb->version >= 158) {
-- 
2.7.4

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

Re: [PATCH v3 3/4] drm/i915: Clean up DDI DDC/AUX CH sanitation

[Maarten Maathuis]

[-- Attachment #1.1: Type: text/plain, Size: 9076 bytes --]

Also tested v3 on top of 4.8.3 (mainline git is a mess right now for
booting).

I did encounter a seemingly unrelated message during boot (including a
WARN_ON):
[drm:skylake_pfit_enable [i915]] *ERROR* Requesting pfit without getting a
scaler first

I suspect any causal relation with these patches.

On Mon, Oct 17, 2016 at 8:07 PM, <ville.syrjala@linux.intel.com> wrote:

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Now that we use the AUX and GMBUS assignment from VBT for all ports,
> let's clean up the sanitization of the port information a bit.
> Previosuly we only did this for port E, and only complained about a
> non-standard assignment for the other ports. But as we know that
> non-standard assignments are a fact of life, let's expand the
> sanitization to all the ports.
>
> v2: Include a commit message, fix up the comments a bit
> v3: Don't clobber other ports if the current port has no alternate aux
> ch/ddc pin
>
> Cc: stable@vger.kernel.org
> Cc: Maarten Maathuis <madman2003@gmail.com>
> Tested-by: Maarten Maathuis <madman2003@gmail.com>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=97877
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Link: http://patchwork.freedesktop.org/patch/msgid/1476208368-
> 5710-4-git-send-email-ville.syrjala@linux.intel.com
> Reviewed-by: Jim Bride <jim.bride@linux.intel.com> (v2)
> ---
>  drivers/gpu/drm/i915/intel_bios.c | 122 ++++++++++++++++++++++++------
> --------
>  1 file changed, 77 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_bios.c
> b/drivers/gpu/drm/i915/intel_bios.c
> index 83667e8cdd6b..a8ff8c099685 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1035,6 +1035,77 @@ static u8 translate_iboost(u8 val)
>         return mapping[val];
>  }
>
> +static void sanitize_ddc_pin(struct drm_i915_private *dev_priv,
> +                            enum port port)
> +{
> +       const struct ddi_vbt_port_info *info =
> +               &dev_priv->vbt.ddi_port_info[port];
> +       enum port p;
> +
> +       if (!info->alternate_ddc_pin)
> +               return;
> +
> +       for_each_port_masked(p, (1 << port) - 1) {
> +               struct ddi_vbt_port_info *i = &dev_priv->vbt.ddi_port_info[
> p];
> +
> +               if (info->alternate_ddc_pin != i->alternate_ddc_pin)
> +                       continue;
> +
> +               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(p), i->alternate_ddc_pin,
> +                             port_name(port), port_name(p));
> +
> +               /*
> +                * If we have multiple ports supposedly sharing the
> +                * pin, then dvi/hdmi couldn't exist on the shared
> +                * port. Otherwise they share the same ddc bin and
> +                * system couldn't communicate with them separately.
> +                *
> +                * Due to parsing the ports in alphabetical order,
> +                * a higher port will always clobber a lower one.
> +                */
> +               i->supports_dvi = false;
> +               i->supports_hdmi = false;
> +               i->alternate_ddc_pin = 0;
> +       }
> +}
> +
> +static void sanitize_aux_ch(struct drm_i915_private *dev_priv,
> +                           enum port port)
> +{
> +       const struct ddi_vbt_port_info *info =
> +               &dev_priv->vbt.ddi_port_info[port];
> +       enum port p;
> +
> +       if (!info->alternate_aux_channel)
> +               return;
> +
> +       for_each_port_masked(p, (1 << port) - 1) {
> +               struct ddi_vbt_port_info *i = &dev_priv->vbt.ddi_port_info[
> p];
> +
> +               if (info->alternate_aux_channel !=
> i->alternate_aux_channel)
> +                       continue;
> +
> +               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(p), i->alternate_aux_channel,
> +                             port_name(port), port_name(p));
> +
> +               /*
> +                * If we have multiple ports supposedlt sharing the
> +                * aux channel, then DP couldn't exist on the shared
> +                * port. Otherwise they share the same aux channel
> +                * and system couldn't communicate with them separately.
> +                *
> +                * Due to parsing the ports in alphabetical order,
> +                * a higher port will always clobber a lower one.
> +                */
> +               i->supports_dp = false;
> +               i->alternate_aux_channel = 0;
> +       }
> +}
> +
>  static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port
> port,
>                            const struct bdb_header *bdb)
>  {
> @@ -1109,54 +1180,15 @@ static void parse_ddi_port(struct drm_i915_private
> *dev_priv, enum port port,
>                 DRM_DEBUG_KMS("Port %c is internal DP\n", port_name(port));
>
>         if (is_dvi) {
> -               if (port == PORT_E) {
> -                       info->alternate_ddc_pin = ddc_pin;
> -                       /* if DDIE share ddc pin with other port, then
> -                        * dvi/hdmi couldn't exist on the shared port.
> -                        * Otherwise they share the same ddc bin and system
> -                        * couldn't communicate with them seperately. */
> -                       if (ddc_pin == DDC_PIN_B) {
> -                               dev_priv->vbt.ddi_port_info[PORT_B].supports_dvi
> = 0;
> -                               dev_priv->vbt.ddi_port_info[PORT_B].supports_hdmi
> = 0;
> -                       } else if (ddc_pin == DDC_PIN_C) {
> -                               dev_priv->vbt.ddi_port_info[PORT_C].supports_dvi
> = 0;
> -                               dev_priv->vbt.ddi_port_info[PORT_C].supports_hdmi
> = 0;
> -                       } else if (ddc_pin == DDC_PIN_D) {
> -                               dev_priv->vbt.ddi_port_info[PORT_D].supports_dvi
> = 0;
> -                               dev_priv->vbt.ddi_port_info[PORT_D].supports_hdmi
> = 0;
> -                       }
> -               } else if (ddc_pin == DDC_PIN_B && port != PORT_B)
> -                       DRM_DEBUG_KMS("Unexpected DDC pin for port B\n");
> -               else if (ddc_pin == DDC_PIN_C && port != PORT_C)
> -                       DRM_DEBUG_KMS("Unexpected DDC pin for port C\n");
> -               else if (ddc_pin == DDC_PIN_D && port != PORT_D)
> -                       DRM_DEBUG_KMS("Unexpected DDC pin for port D\n");
> +               info->alternate_ddc_pin = ddc_pin;
> +
> +               sanitize_ddc_pin(dev_priv, port);
>         }
>
>         if (is_dp) {
> -               if (port == PORT_E) {
> -                       info->alternate_aux_channel = aux_channel;
> -                       /* if DDIE share aux channel with other port, then
> -                        * DP couldn't exist on the shared port. Otherwise
> -                        * they share the same aux channel and system
> -                        * couldn't communicate with them seperately. */
> -                       if (aux_channel == DP_AUX_A)
> -                               dev_priv->vbt.ddi_port_info[PORT_A].supports_dp
> = 0;
> -                       else if (aux_channel == DP_AUX_B)
> -                               dev_priv->vbt.ddi_port_info[PORT_B].supports_dp
> = 0;
> -                       else if (aux_channel == DP_AUX_C)
> -                               dev_priv->vbt.ddi_port_info[PORT_C].supports_dp
> = 0;
> -                       else if (aux_channel == DP_AUX_D)
> -                               dev_priv->vbt.ddi_port_info[PORT_D].supports_dp
> = 0;
> -               }
> -               else if (aux_channel == DP_AUX_A && port != PORT_A)
> -                       DRM_DEBUG_KMS("Unexpected AUX channel for port
> A\n");
> -               else if (aux_channel == DP_AUX_B && port != PORT_B)
> -                       DRM_DEBUG_KMS("Unexpected AUX channel for port
> B\n");
> -               else if (aux_channel == DP_AUX_C && port != PORT_C)
> -                       DRM_DEBUG_KMS("Unexpected AUX channel for port
> C\n");
> -               else if (aux_channel == DP_AUX_D && port != PORT_D)
> -                       DRM_DEBUG_KMS("Unexpected AUX channel for port
> D\n");
> +               info->alternate_aux_channel = aux_channel;
> +
> +               sanitize_aux_ch(dev_priv, port);
>         }
>
>         if (bdb->version >= 158) {
> --
> 2.7.4
>
>


-- 
Far away from the primal instinct, the song seems to fade away, the river
get wider between your thoughts and the things we do and say.

[-- Attachment #1.2: Type: text/html, Size: 12078 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH v3 3/4] drm/i915: Clean up DDI DDC/AUX CH sanitation

[Maarten Maathuis]

[-- Attachment #1.1: Type: text/plain, Size: 9566 bytes --]

I meant DON't suspect

On Thu, Oct 20, 2016 at 11:53 PM, Maarten Maathuis <madman2003@gmail.com>
wrote:

> Also tested v3 on top of 4.8.3 (mainline git is a mess right now for
> booting).
>
> I did encounter a seemingly unrelated message during boot (including a
> WARN_ON):
> [drm:skylake_pfit_enable [i915]] *ERROR* Requesting pfit without getting a
> scaler first
>
> I suspect any causal relation with these patches.
>
> On Mon, Oct 17, 2016 at 8:07 PM, <ville.syrjala@linux.intel.com> wrote:
>
>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
>> Now that we use the AUX and GMBUS assignment from VBT for all ports,
>> let's clean up the sanitization of the port information a bit.
>> Previosuly we only did this for port E, and only complained about a
>> non-standard assignment for the other ports. But as we know that
>> non-standard assignments are a fact of life, let's expand the
>> sanitization to all the ports.
>>
>> v2: Include a commit message, fix up the comments a bit
>> v3: Don't clobber other ports if the current port has no alternate aux
>> ch/ddc pin
>>
>> Cc: stable@vger.kernel.org
>> Cc: Maarten Maathuis <madman2003@gmail.com>
>> Tested-by: Maarten Maathuis <madman2003@gmail.com>
>> References: https://bugs.freedesktop.org/show_bug.cgi?id=97877
>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Link: http://patchwork.freedesktop.org/patch/msgid/1476208368-5710
>> -4-git-send-email-ville.syrjala@linux.intel.com
>> Reviewed-by: Jim Bride <jim.bride@linux.intel.com> (v2)
>> ---
>>  drivers/gpu/drm/i915/intel_bios.c | 122 ++++++++++++++++++++++++------
>> --------
>>  1 file changed, 77 insertions(+), 45 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_bios.c
>> b/drivers/gpu/drm/i915/intel_bios.c
>> index 83667e8cdd6b..a8ff8c099685 100644
>> --- a/drivers/gpu/drm/i915/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/intel_bios.c
>> @@ -1035,6 +1035,77 @@ static u8 translate_iboost(u8 val)
>>         return mapping[val];
>>  }
>>
>> +static void sanitize_ddc_pin(struct drm_i915_private *dev_priv,
>> +                            enum port port)
>> +{
>> +       const struct ddi_vbt_port_info *info =
>> +               &dev_priv->vbt.ddi_port_info[port];
>> +       enum port p;
>> +
>> +       if (!info->alternate_ddc_pin)
>> +               return;
>> +
>> +       for_each_port_masked(p, (1 << port) - 1) {
>> +               struct ddi_vbt_port_info *i =
>> &dev_priv->vbt.ddi_port_info[p];
>> +
>> +               if (info->alternate_ddc_pin != i->alternate_ddc_pin)
>> +                       continue;
>> +
>> +               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(p), i->alternate_ddc_pin,
>> +                             port_name(port), port_name(p));
>> +
>> +               /*
>> +                * If we have multiple ports supposedly sharing the
>> +                * pin, then dvi/hdmi couldn't exist on the shared
>> +                * port. Otherwise they share the same ddc bin and
>> +                * system couldn't communicate with them separately.
>> +                *
>> +                * Due to parsing the ports in alphabetical order,
>> +                * a higher port will always clobber a lower one.
>> +                */
>> +               i->supports_dvi = false;
>> +               i->supports_hdmi = false;
>> +               i->alternate_ddc_pin = 0;
>> +       }
>> +}
>> +
>> +static void sanitize_aux_ch(struct drm_i915_private *dev_priv,
>> +                           enum port port)
>> +{
>> +       const struct ddi_vbt_port_info *info =
>> +               &dev_priv->vbt.ddi_port_info[port];
>> +       enum port p;
>> +
>> +       if (!info->alternate_aux_channel)
>> +               return;
>> +
>> +       for_each_port_masked(p, (1 << port) - 1) {
>> +               struct ddi_vbt_port_info *i =
>> &dev_priv->vbt.ddi_port_info[p];
>> +
>> +               if (info->alternate_aux_channel !=
>> i->alternate_aux_channel)
>> +                       continue;
>> +
>> +               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(p), i->alternate_aux_channel,
>> +                             port_name(port), port_name(p));
>> +
>> +               /*
>> +                * If we have multiple ports supposedlt sharing the
>> +                * aux channel, then DP couldn't exist on the shared
>> +                * port. Otherwise they share the same aux channel
>> +                * and system couldn't communicate with them separately.
>> +                *
>> +                * Due to parsing the ports in alphabetical order,
>> +                * a higher port will always clobber a lower one.
>> +                */
>> +               i->supports_dp = false;
>> +               i->alternate_aux_channel = 0;
>> +       }
>> +}
>> +
>>  static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port
>> port,
>>                            const struct bdb_header *bdb)
>>  {
>> @@ -1109,54 +1180,15 @@ static void parse_ddi_port(struct
>> drm_i915_private *dev_priv, enum port port,
>>                 DRM_DEBUG_KMS("Port %c is internal DP\n",
>> port_name(port));
>>
>>         if (is_dvi) {
>> -               if (port == PORT_E) {
>> -                       info->alternate_ddc_pin = ddc_pin;
>> -                       /* if DDIE share ddc pin with other port, then
>> -                        * dvi/hdmi couldn't exist on the shared port.
>> -                        * Otherwise they share the same ddc bin and
>> system
>> -                        * couldn't communicate with them seperately. */
>> -                       if (ddc_pin == DDC_PIN_B) {
>> -                               dev_priv->vbt.ddi_port_info[PORT_B].supports_dvi
>> = 0;
>> -                               dev_priv->vbt.ddi_port_info[PORT_B].supports_hdmi
>> = 0;
>> -                       } else if (ddc_pin == DDC_PIN_C) {
>> -                               dev_priv->vbt.ddi_port_info[PORT_C].supports_dvi
>> = 0;
>> -                               dev_priv->vbt.ddi_port_info[PORT_C].supports_hdmi
>> = 0;
>> -                       } else if (ddc_pin == DDC_PIN_D) {
>> -                               dev_priv->vbt.ddi_port_info[PORT_D].supports_dvi
>> = 0;
>> -                               dev_priv->vbt.ddi_port_info[PORT_D].supports_hdmi
>> = 0;
>> -                       }
>> -               } else if (ddc_pin == DDC_PIN_B && port != PORT_B)
>> -                       DRM_DEBUG_KMS("Unexpected DDC pin for port B\n");
>> -               else if (ddc_pin == DDC_PIN_C && port != PORT_C)
>> -                       DRM_DEBUG_KMS("Unexpected DDC pin for port C\n");
>> -               else if (ddc_pin == DDC_PIN_D && port != PORT_D)
>> -                       DRM_DEBUG_KMS("Unexpected DDC pin for port D\n");
>> +               info->alternate_ddc_pin = ddc_pin;
>> +
>> +               sanitize_ddc_pin(dev_priv, port);
>>         }
>>
>>         if (is_dp) {
>> -               if (port == PORT_E) {
>> -                       info->alternate_aux_channel = aux_channel;
>> -                       /* if DDIE share aux channel with other port, then
>> -                        * DP couldn't exist on the shared port. Otherwise
>> -                        * they share the same aux channel and system
>> -                        * couldn't communicate with them seperately. */
>> -                       if (aux_channel == DP_AUX_A)
>> -                               dev_priv->vbt.ddi_port_info[PORT_A].supports_dp
>> = 0;
>> -                       else if (aux_channel == DP_AUX_B)
>> -                               dev_priv->vbt.ddi_port_info[PORT_B].supports_dp
>> = 0;
>> -                       else if (aux_channel == DP_AUX_C)
>> -                               dev_priv->vbt.ddi_port_info[PORT_C].supports_dp
>> = 0;
>> -                       else if (aux_channel == DP_AUX_D)
>> -                               dev_priv->vbt.ddi_port_info[PORT_D].supports_dp
>> = 0;
>> -               }
>> -               else if (aux_channel == DP_AUX_A && port != PORT_A)
>> -                       DRM_DEBUG_KMS("Unexpected AUX channel for port
>> A\n");
>> -               else if (aux_channel == DP_AUX_B && port != PORT_B)
>> -                       DRM_DEBUG_KMS("Unexpected AUX channel for port
>> B\n");
>> -               else if (aux_channel == DP_AUX_C && port != PORT_C)
>> -                       DRM_DEBUG_KMS("Unexpected AUX channel for port
>> C\n");
>> -               else if (aux_channel == DP_AUX_D && port != PORT_D)
>> -                       DRM_DEBUG_KMS("Unexpected AUX channel for port
>> D\n");
>> +               info->alternate_aux_channel = aux_channel;
>> +
>> +               sanitize_aux_ch(dev_priv, port);
>>         }
>>
>>         if (bdb->version >= 158) {
>> --
>> 2.7.4
>>
>>
>
>
> --
> Far away from the primal instinct, the song seems to fade away, the river
> get wider between your thoughts and the things we do and say.
>



-- 
Far away from the primal instinct, the song seems to fade away, the river
get wider between your thoughts and the things we do and say.

[-- Attachment #1.2: Type: text/html, Size: 12999 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH v3 3/4] drm/i915: Clean up DDI DDC/AUX CH sanitation

[Ville Syrjälä]

On Fri, Oct 21, 2016 at 12:00:56AM +0200, Maarten Maathuis wrote:
> I meant DON't suspect
> 
> On Thu, Oct 20, 2016 at 11:53 PM, Maarten Maathuis <madman2003@gmail.com>
> wrote:
> 
> > Also tested v3 on top of 4.8.3 (mainline git is a mess right now for
> > booting).
> >
> > I did encounter a seemingly unrelated message during boot (including a
> > WARN_ON):
> > [drm:skylake_pfit_enable [i915]] *ERROR* Requesting pfit without getting a
> > scaler first
> >
> > I suspect any causal relation with these patches.

Yeah shouldn't be related.

I've pushed the patches to dinq. Thanks for testing and reviews.

> >
> > On Mon, Oct 17, 2016 at 8:07 PM, <ville.syrjala@linux.intel.com> wrote:
> >
> >> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>
> >> Now that we use the AUX and GMBUS assignment from VBT for all ports,
> >> let's clean up the sanitization of the port information a bit.
> >> Previosuly we only did this for port E, and only complained about a
> >> non-standard assignment for the other ports. But as we know that
> >> non-standard assignments are a fact of life, let's expand the
> >> sanitization to all the ports.
> >>
> >> v2: Include a commit message, fix up the comments a bit
> >> v3: Don't clobber other ports if the current port has no alternate aux
> >> ch/ddc pin
> >>
> >> Cc: stable@vger.kernel.org
> >> Cc: Maarten Maathuis <madman2003@gmail.com>
> >> Tested-by: Maarten Maathuis <madman2003@gmail.com>
> >> References: https://bugs.freedesktop.org/show_bug.cgi?id=97877
> >> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> Link: http://patchwork.freedesktop.org/patch/msgid/1476208368-5710
> >> -4-git-send-email-ville.syrjala@linux.intel.com
> >> Reviewed-by: Jim Bride <jim.bride@linux.intel.com> (v2)
> >> ---
> >>  drivers/gpu/drm/i915/intel_bios.c | 122 ++++++++++++++++++++++++------
> >> --------
> >>  1 file changed, 77 insertions(+), 45 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_bios.c
> >> b/drivers/gpu/drm/i915/intel_bios.c
> >> index 83667e8cdd6b..a8ff8c099685 100644
> >> --- a/drivers/gpu/drm/i915/intel_bios.c
> >> +++ b/drivers/gpu/drm/i915/intel_bios.c
> >> @@ -1035,6 +1035,77 @@ static u8 translate_iboost(u8 val)
> >>         return mapping[val];
> >>  }
> >>
> >> +static void sanitize_ddc_pin(struct drm_i915_private *dev_priv,
> >> +                            enum port port)
> >> +{
> >> +       const struct ddi_vbt_port_info *info =
> >> +               &dev_priv->vbt.ddi_port_info[port];
> >> +       enum port p;
> >> +
> >> +       if (!info->alternate_ddc_pin)
> >> +               return;
> >> +
> >> +       for_each_port_masked(p, (1 << port) - 1) {
> >> +               struct ddi_vbt_port_info *i =
> >> &dev_priv->vbt.ddi_port_info[p];
> >> +
> >> +               if (info->alternate_ddc_pin != i->alternate_ddc_pin)
> >> +                       continue;
> >> +
> >> +               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(p), i->alternate_ddc_pin,
> >> +                             port_name(port), port_name(p));
> >> +
> >> +               /*
> >> +                * If we have multiple ports supposedly sharing the
> >> +                * pin, then dvi/hdmi couldn't exist on the shared
> >> +                * port. Otherwise they share the same ddc bin and
> >> +                * system couldn't communicate with them separately.
> >> +                *
> >> +                * Due to parsing the ports in alphabetical order,
> >> +                * a higher port will always clobber a lower one.
> >> +                */
> >> +               i->supports_dvi = false;
> >> +               i->supports_hdmi = false;
> >> +               i->alternate_ddc_pin = 0;
> >> +       }
> >> +}
> >> +
> >> +static void sanitize_aux_ch(struct drm_i915_private *dev_priv,
> >> +                           enum port port)
> >> +{
> >> +       const struct ddi_vbt_port_info *info =
> >> +               &dev_priv->vbt.ddi_port_info[port];
> >> +       enum port p;
> >> +
> >> +       if (!info->alternate_aux_channel)
> >> +               return;
> >> +
> >> +       for_each_port_masked(p, (1 << port) - 1) {
> >> +               struct ddi_vbt_port_info *i =
> >> &dev_priv->vbt.ddi_port_info[p];
> >> +
> >> +               if (info->alternate_aux_channel !=
> >> i->alternate_aux_channel)
> >> +                       continue;
> >> +
> >> +               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(p), i->alternate_aux_channel,
> >> +                             port_name(port), port_name(p));
> >> +
> >> +               /*
> >> +                * If we have multiple ports supposedlt sharing the
> >> +                * aux channel, then DP couldn't exist on the shared
> >> +                * port. Otherwise they share the same aux channel
> >> +                * and system couldn't communicate with them separately.
> >> +                *
> >> +                * Due to parsing the ports in alphabetical order,
> >> +                * a higher port will always clobber a lower one.
> >> +                */
> >> +               i->supports_dp = false;
> >> +               i->alternate_aux_channel = 0;
> >> +       }
> >> +}
> >> +
> >>  static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port
> >> port,
> >>                            const struct bdb_header *bdb)
> >>  {
> >> @@ -1109,54 +1180,15 @@ static void parse_ddi_port(struct
> >> drm_i915_private *dev_priv, enum port port,
> >>                 DRM_DEBUG_KMS("Port %c is internal DP\n",
> >> port_name(port));
> >>
> >>         if (is_dvi) {
> >> -               if (port == PORT_E) {
> >> -                       info->alternate_ddc_pin = ddc_pin;
> >> -                       /* if DDIE share ddc pin with other port, then
> >> -                        * dvi/hdmi couldn't exist on the shared port.
> >> -                        * Otherwise they share the same ddc bin and
> >> system
> >> -                        * couldn't communicate with them seperately. */
> >> -                       if (ddc_pin == DDC_PIN_B) {
> >> -                               dev_priv->vbt.ddi_port_info[PORT_B].supports_dvi
> >> = 0;
> >> -                               dev_priv->vbt.ddi_port_info[PORT_B].supports_hdmi
> >> = 0;
> >> -                       } else if (ddc_pin == DDC_PIN_C) {
> >> -                               dev_priv->vbt.ddi_port_info[PORT_C].supports_dvi
> >> = 0;
> >> -                               dev_priv->vbt.ddi_port_info[PORT_C].supports_hdmi
> >> = 0;
> >> -                       } else if (ddc_pin == DDC_PIN_D) {
> >> -                               dev_priv->vbt.ddi_port_info[PORT_D].supports_dvi
> >> = 0;
> >> -                               dev_priv->vbt.ddi_port_info[PORT_D].supports_hdmi
> >> = 0;
> >> -                       }
> >> -               } else if (ddc_pin == DDC_PIN_B && port != PORT_B)
> >> -                       DRM_DEBUG_KMS("Unexpected DDC pin for port B\n");
> >> -               else if (ddc_pin == DDC_PIN_C && port != PORT_C)
> >> -                       DRM_DEBUG_KMS("Unexpected DDC pin for port C\n");
> >> -               else if (ddc_pin == DDC_PIN_D && port != PORT_D)
> >> -                       DRM_DEBUG_KMS("Unexpected DDC pin for port D\n");
> >> +               info->alternate_ddc_pin = ddc_pin;
> >> +
> >> +               sanitize_ddc_pin(dev_priv, port);
> >>         }
> >>
> >>         if (is_dp) {
> >> -               if (port == PORT_E) {
> >> -                       info->alternate_aux_channel = aux_channel;
> >> -                       /* if DDIE share aux channel with other port, then
> >> -                        * DP couldn't exist on the shared port. Otherwise
> >> -                        * they share the same aux channel and system
> >> -                        * couldn't communicate with them seperately. */
> >> -                       if (aux_channel == DP_AUX_A)
> >> -                               dev_priv->vbt.ddi_port_info[PORT_A].supports_dp
> >> = 0;
> >> -                       else if (aux_channel == DP_AUX_B)
> >> -                               dev_priv->vbt.ddi_port_info[PORT_B].supports_dp
> >> = 0;
> >> -                       else if (aux_channel == DP_AUX_C)
> >> -                               dev_priv->vbt.ddi_port_info[PORT_C].supports_dp
> >> = 0;
> >> -                       else if (aux_channel == DP_AUX_D)
> >> -                               dev_priv->vbt.ddi_port_info[PORT_D].supports_dp
> >> = 0;
> >> -               }
> >> -               else if (aux_channel == DP_AUX_A && port != PORT_A)
> >> -                       DRM_DEBUG_KMS("Unexpected AUX channel for port
> >> A\n");
> >> -               else if (aux_channel == DP_AUX_B && port != PORT_B)
> >> -                       DRM_DEBUG_KMS("Unexpected AUX channel for port
> >> B\n");
> >> -               else if (aux_channel == DP_AUX_C && port != PORT_C)
> >> -                       DRM_DEBUG_KMS("Unexpected AUX channel for port
> >> C\n");
> >> -               else if (aux_channel == DP_AUX_D && port != PORT_D)
> >> -                       DRM_DEBUG_KMS("Unexpected AUX channel for port
> >> D\n");
> >> +               info->alternate_aux_channel = aux_channel;
> >> +
> >> +               sanitize_aux_ch(dev_priv, port);
> >>         }
> >>
> >>         if (bdb->version >= 158) {
> >> --
> >> 2.7.4
> >>
> >>
> >
> >
> > --
> > Far away from the primal instinct, the song seems to fade away, the river
> > get wider between your thoughts and the things we do and say.
> >
> 
> 
> 
> -- 
> Far away from the primal instinct, the song seems to fade away, the river
> get wider between your thoughts and the things we do and say.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 4/4] drm/i915: Fix whitespace issues

[ville.syrjala]

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

Fix the poorly indented port parameters to the aux ctl and data
reg functions. This was fallout from the s/i915_mmio_reg_t/i915_reg_t/
that happened during the review of commit f0f59a00a1c9 ("drm/i915:
Type safe register read/write")

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b0753b272101..25cde7356b59 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1147,7 +1147,7 @@ static enum port intel_aux_port(struct drm_i915_private *dev_priv,
 }
 
 static i915_reg_t g4x_aux_ctl_reg(struct drm_i915_private *dev_priv,
-				       enum port port)
+				  enum port port)
 {
 	switch (port) {
 	case PORT_B:
@@ -1161,7 +1161,7 @@ static i915_reg_t g4x_aux_ctl_reg(struct drm_i915_private *dev_priv,
 }
 
 static i915_reg_t g4x_aux_data_reg(struct drm_i915_private *dev_priv,
-					enum port port, int index)
+				   enum port port, int index)
 {
 	switch (port) {
 	case PORT_B:
@@ -1175,7 +1175,7 @@ static i915_reg_t g4x_aux_data_reg(struct drm_i915_private *dev_priv,
 }
 
 static i915_reg_t ilk_aux_ctl_reg(struct drm_i915_private *dev_priv,
-				       enum port port)
+				  enum port port)
 {
 	switch (port) {
 	case PORT_A:
@@ -1191,7 +1191,7 @@ static i915_reg_t ilk_aux_ctl_reg(struct drm_i915_private *dev_priv,
 }
 
 static i915_reg_t ilk_aux_data_reg(struct drm_i915_private *dev_priv,
-					enum port port, int index)
+				   enum port port, int index)
 {
 	switch (port) {
 	case PORT_A:
@@ -1207,7 +1207,7 @@ static i915_reg_t ilk_aux_data_reg(struct drm_i915_private *dev_priv,
 }
 
 static i915_reg_t skl_aux_ctl_reg(struct drm_i915_private *dev_priv,
-				       enum port port)
+				  enum port port)
 {
 	switch (port) {
 	case PORT_A:
@@ -1222,7 +1222,7 @@ 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)
+				   enum port port, int index)
 {
 	switch (port) {
 	case PORT_A:
@@ -1237,7 +1237,7 @@ static i915_reg_t skl_aux_data_reg(struct drm_i915_private *dev_priv,
 }
 
 static i915_reg_t intel_aux_ctl_reg(struct drm_i915_private *dev_priv,
-					 enum port port)
+				    enum port port)
 {
 	if (INTEL_INFO(dev_priv)->gen >= 9)
 		return skl_aux_ctl_reg(dev_priv, port);
@@ -1248,7 +1248,7 @@ static i915_reg_t intel_aux_ctl_reg(struct drm_i915_private *dev_priv,
 }
 
 static i915_reg_t intel_aux_data_reg(struct drm_i915_private *dev_priv,
-					  enum port port, int index)
+				     enum port port, int index)
 {
 	if (INTEL_INFO(dev_priv)->gen >= 9)
 		return skl_aux_data_reg(dev_priv, port, index);
-- 
2.7.4

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

Re: [PATCH 4/4] drm/i915: Fix whitespace issues

[Jim Bride]

On Tue, Oct 11, 2016 at 08:52:48PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Fix the poorly indented port parameters to the aux ctl and data
> reg functions. This was fallout from the s/i915_mmio_reg_t/i915_reg_t/
> that happened during the review of commit f0f59a00a1c9 ("drm/i915:
> Type safe register read/write")
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Jim Bride <jim.bride@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_dp.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index b0753b272101..25cde7356b59 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1147,7 +1147,7 @@ static enum port intel_aux_port(struct drm_i915_private *dev_priv,
>  }
>  
>  static i915_reg_t g4x_aux_ctl_reg(struct drm_i915_private *dev_priv,
> -				       enum port port)
> +				  enum port port)
>  {
>  	switch (port) {
>  	case PORT_B:
> @@ -1161,7 +1161,7 @@ static i915_reg_t g4x_aux_ctl_reg(struct drm_i915_private *dev_priv,
>  }
>  
>  static i915_reg_t g4x_aux_data_reg(struct drm_i915_private *dev_priv,
> -					enum port port, int index)
> +				   enum port port, int index)
>  {
>  	switch (port) {
>  	case PORT_B:
> @@ -1175,7 +1175,7 @@ static i915_reg_t g4x_aux_data_reg(struct drm_i915_private *dev_priv,
>  }
>  
>  static i915_reg_t ilk_aux_ctl_reg(struct drm_i915_private *dev_priv,
> -				       enum port port)
> +				  enum port port)
>  {
>  	switch (port) {
>  	case PORT_A:
> @@ -1191,7 +1191,7 @@ static i915_reg_t ilk_aux_ctl_reg(struct drm_i915_private *dev_priv,
>  }
>  
>  static i915_reg_t ilk_aux_data_reg(struct drm_i915_private *dev_priv,
> -					enum port port, int index)
> +				   enum port port, int index)
>  {
>  	switch (port) {
>  	case PORT_A:
> @@ -1207,7 +1207,7 @@ static i915_reg_t ilk_aux_data_reg(struct drm_i915_private *dev_priv,
>  }
>  
>  static i915_reg_t skl_aux_ctl_reg(struct drm_i915_private *dev_priv,
> -				       enum port port)
> +				  enum port port)
>  {
>  	switch (port) {
>  	case PORT_A:
> @@ -1222,7 +1222,7 @@ 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)
> +				   enum port port, int index)
>  {
>  	switch (port) {
>  	case PORT_A:
> @@ -1237,7 +1237,7 @@ static i915_reg_t skl_aux_data_reg(struct drm_i915_private *dev_priv,
>  }
>  
>  static i915_reg_t intel_aux_ctl_reg(struct drm_i915_private *dev_priv,
> -					 enum port port)
> +				    enum port port)
>  {
>  	if (INTEL_INFO(dev_priv)->gen >= 9)
>  		return skl_aux_ctl_reg(dev_priv, port);
> @@ -1248,7 +1248,7 @@ static i915_reg_t intel_aux_ctl_reg(struct drm_i915_private *dev_priv,
>  }
>  
>  static i915_reg_t intel_aux_data_reg(struct drm_i915_private *dev_priv,
> -					  enum port port, int index)
> +				     enum port port, int index)
>  {
>  	if (INTEL_INFO(dev_priv)->gen >= 9)
>  		return skl_aux_data_reg(dev_priv, port, index);
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: warning for drm/i915: Trust VBT aux/ddc information

[Patchwork]

== Series Details ==

Series: drm/i915: Trust VBT aux/ddc information
URL   : https://patchwork.freedesktop.org/series/13600/
State : warning

== Summary ==

Series 13600v1 drm/i915: Trust VBT aux/ddc information
https://patchwork.freedesktop.org/api/1.0/series/13600/revisions/1/mbox/

Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                dmesg-warn -> PASS       (fi-byt-j1900)
        Subgroup suspend-read-crc-pipe-c:
                pass       -> DMESG-WARN (fi-skl-6700k)
Test kms_psr_sink_crc:
        Subgroup psr_basic:
                pass       -> DMESG-WARN (fi-skl-6700hq)
Test pm_backlight:
        Subgroup basic-brightness:
                skip       -> PASS       (fi-skl-6700k)
Test vgem_basic:
        Subgroup unload:
                pass       -> SKIP       (fi-ivb-3770)
                pass       -> SKIP       (fi-byt-n2820)
                skip       -> PASS       (fi-bsw-n3050)
                skip       -> PASS       (fi-skl-6700k)
                skip       -> PASS       (fi-bdw-5557u)

fi-bdw-5557u     total:248  pass:232  dwarn:0   dfail:0   fail:0   skip:16 
fi-bsw-n3050     total:248  pass:205  dwarn:0   dfail:0   fail:0   skip:43 
fi-bxt-t5700     total:248  pass:217  dwarn:0   dfail:0   fail:0   skip:31 
fi-byt-j1900     total:248  pass:215  dwarn:0   dfail:0   fail:1   skip:32 
fi-byt-n2820     total:248  pass:210  dwarn:0   dfail:0   fail:1   skip:37 
fi-hsw-4770      total:248  pass:224  dwarn:0   dfail:0   fail:0   skip:24 
fi-hsw-4770r     total:248  pass:224  dwarn:0   dfail:0   fail:0   skip:24 
fi-ilk-650       total:248  pass:185  dwarn:0   dfail:0   fail:2   skip:61 
fi-ivb-3520m     total:248  pass:221  dwarn:0   dfail:0   fail:0   skip:27 
fi-ivb-3770      total:248  pass:221  dwarn:0   dfail:0   fail:0   skip:27 
fi-kbl-7200u     total:248  pass:222  dwarn:0   dfail:0   fail:0   skip:26 
fi-skl-6700hq    total:248  pass:223  dwarn:1   dfail:0   fail:0   skip:24 
fi-skl-6700k     total:248  pass:222  dwarn:2   dfail:0   fail:0   skip:24 
fi-snb-2520m     total:248  pass:211  dwarn:0   dfail:0   fail:0   skip:37 
fi-snb-2600      total:248  pass:209  dwarn:0   dfail:0   fail:0   skip:39 

Results at /archive/results/CI_IGT_test/Patchwork_2676/

41409515b7b3365bcb2c2e9239fdfaa286a51333 drm-intel-nightly: 2016y-10m-11d-17h-55m-34s UTC integration manifest
8e39dff drm/i915: Fix whitespace issues
536a3b5 drm/i915: Clean up DDI DDC/AUX CH sanitation
a01faf4 drm/i915: Respect alternate_ddc_pin for all DDI ports
8e81858 drm/i915: Respect alternate_aux_channel for all DDI ports

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

Re: ✗ Fi.CI.BAT: warning for drm/i915: Trust VBT aux/ddc information

[Ville Syrjälä]

On Tue, Oct 11, 2016 at 06:49:46PM -0000, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915: Trust VBT aux/ddc information
> URL   : https://patchwork.freedesktop.org/series/13600/
> State : warning
> 
> == Summary ==
> 
> Series 13600v1 drm/i915: Trust VBT aux/ddc information
> https://patchwork.freedesktop.org/api/1.0/series/13600/revisions/1/mbox/
> 
> Test kms_pipe_crc_basic:
>         Subgroup suspend-read-crc-pipe-b:
>                 dmesg-warn -> PASS       (fi-byt-j1900)
>         Subgroup suspend-read-crc-pipe-c:
>                 pass       -> DMESG-WARN (fi-skl-6700k)

[  631.216836] [drm:intel_cpu_fifo_underrun_irq_handler [i915]] *ERROR* CPU pipe C FIFO underrun

moar SKL underruns

> Test kms_psr_sink_crc:
>         Subgroup psr_basic:
>                 pass       -> DMESG-WARN (fi-skl-6700hq)

[  647.478464] [drm:intel_pipe_update_start [i915]] *ERROR* Potential atomic update failure on pipe A

DMC making life difficult for vblank irqs I guess.

> Test pm_backlight:
>         Subgroup basic-brightness:
>                 skip       -> PASS       (fi-skl-6700k)
> Test vgem_basic:
>         Subgroup unload:
>                 pass       -> SKIP       (fi-ivb-3770)
>                 pass       -> SKIP       (fi-byt-n2820)
>                 skip       -> PASS       (fi-bsw-n3050)
>                 skip       -> PASS       (fi-skl-6700k)
>                 skip       -> PASS       (fi-bdw-5557u)
> 
> fi-bdw-5557u     total:248  pass:232  dwarn:0   dfail:0   fail:0   skip:16 
> fi-bsw-n3050     total:248  pass:205  dwarn:0   dfail:0   fail:0   skip:43 
> fi-bxt-t5700     total:248  pass:217  dwarn:0   dfail:0   fail:0   skip:31 
> fi-byt-j1900     total:248  pass:215  dwarn:0   dfail:0   fail:1   skip:32 
> fi-byt-n2820     total:248  pass:210  dwarn:0   dfail:0   fail:1   skip:37 
> fi-hsw-4770      total:248  pass:224  dwarn:0   dfail:0   fail:0   skip:24 
> fi-hsw-4770r     total:248  pass:224  dwarn:0   dfail:0   fail:0   skip:24 
> fi-ilk-650       total:248  pass:185  dwarn:0   dfail:0   fail:2   skip:61 
> fi-ivb-3520m     total:248  pass:221  dwarn:0   dfail:0   fail:0   skip:27 
> fi-ivb-3770      total:248  pass:221  dwarn:0   dfail:0   fail:0   skip:27 
> fi-kbl-7200u     total:248  pass:222  dwarn:0   dfail:0   fail:0   skip:26 
> fi-skl-6700hq    total:248  pass:223  dwarn:1   dfail:0   fail:0   skip:24 
> fi-skl-6700k     total:248  pass:222  dwarn:2   dfail:0   fail:0   skip:24 
> fi-snb-2520m     total:248  pass:211  dwarn:0   dfail:0   fail:0   skip:37 
> fi-snb-2600      total:248  pass:209  dwarn:0   dfail:0   fail:0   skip:39 
> 
> Results at /archive/results/CI_IGT_test/Patchwork_2676/
> 
> 41409515b7b3365bcb2c2e9239fdfaa286a51333 drm-intel-nightly: 2016y-10m-11d-17h-55m-34s UTC integration manifest
> 8e39dff drm/i915: Fix whitespace issues
> 536a3b5 drm/i915: Clean up DDI DDC/AUX CH sanitation
> a01faf4 drm/i915: Respect alternate_ddc_pin for all DDI ports
> 8e81858 drm/i915: Respect alternate_aux_channel for all DDI ports

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for drm/i915: Trust VBT aux/ddc information (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Trust VBT aux/ddc information (rev2)
URL   : https://patchwork.freedesktop.org/series/13600/
State : failure

== Summary ==

Series 13600v2 drm/i915: Trust VBT aux/ddc information
https://patchwork.freedesktop.org/api/1.0/series/13600/revisions/2/mbox/

Test gem_exec_suspend:
        Subgroup basic-s3:
                incomplete -> DMESG-WARN (fi-byt-n2820)
                dmesg-warn -> INCOMPLETE (fi-byt-j1900)
Test pm_backlight:
        Subgroup basic-brightness:
                skip       -> PASS       (fi-skl-6700k)
Test vgem_basic:
        Subgroup unload:
                pass       -> SKIP       (fi-bdw-5557u)
                skip       -> PASS       (fi-hsw-4770r)

fi-bdw-5557u     total:246  pass:230  dwarn:0   dfail:0   fail:0   skip:16 
fi-bsw-n3050     total:246  pass:203  dwarn:1   dfail:0   fail:0   skip:42 
fi-bxt-t5700     total:246  pass:216  dwarn:0   dfail:0   fail:0   skip:30 
fi-byt-j1900     total:76   pass:62   dwarn:0   dfail:0   fail:1   skip:12 
fi-byt-n2820     total:246  pass:209  dwarn:1   dfail:0   fail:1   skip:35 
fi-hsw-4770      total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-hsw-4770r     total:246  pass:223  dwarn:1   dfail:0   fail:0   skip:22 
fi-ilk-650       total:246  pass:181  dwarn:0   dfail:0   fail:5   skip:60 
fi-ivb-3770      total:246  pass:221  dwarn:0   dfail:0   fail:0   skip:25 
fi-skl-6700hq    total:246  pass:223  dwarn:0   dfail:0   fail:0   skip:23 
fi-skl-6700k     total:246  pass:222  dwarn:1   dfail:0   fail:0   skip:23 
fi-snb-2600      total:246  pass:209  dwarn:0   dfail:0   fail:0   skip:37 
fi-ivb-3520m failed to connect after reboot
fi-skl-6260u failed to connect after reboot
fi-skl-6770hq failed to connect after reboot
fi-snb-2520m failed to connect after reboot

Results at /archive/results/CI_IGT_test/Patchwork_2739/

7ec75289774dec48c46c37271fb334b7caed3d32 drm-intel-nightly: 2016y-10m-17d-14h-07m-32s UTC integration manifest
13ae602 drm/i915: Fix whitespace issues
ebded98 drm/i915: Clean up DDI DDC/AUX CH sanitation
6ea77f6 drm/i915: Respect alternate_ddc_pin for all DDI ports
49a7f8a drm/i915: Respect alternate_aux_channel for all DDI ports

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