[6/6] drm/i915: Enable HDMI on DDI-E
diff mbox

Message ID 1438847501-28454-6-git-send-email-xiong.y.zhang@intel.com
State New
Headers show

Commit Message

Xiong Zhang Aug. 6, 2015, 7:51 a.m. UTC
DDI-E doesn't have the correspondent GMBUS pin.

We rely on VBT to tell us which one it being used instead.

The DVI/HDMI on shared port couldn't exist.

This patch isn't tested without hardware wchich has HDMI
on DDI-E.

Signed-off-by: Xiong Zhang <xiong.y.zhang@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h   |  5 +++++
 drivers/gpu/drm/i915/intel_bios.c | 28 ++++++++++++++++++++++++----
 drivers/gpu/drm/i915/intel_hdmi.c | 18 ++++++++++++++++++
 3 files changed, 47 insertions(+), 4 deletions(-)

Comments

Rodrigo Vivi Aug. 8, 2015, 12:09 a.m. UTC | #1
On Thu, Aug 6, 2015 at 12:45 AM Xiong Zhang <xiong.y.zhang@intel.com> wrote:

> DDI-E doesn't have the correspondent GMBUS pin.
>
> We rely on VBT to tell us which one it being used instead.
>
> The DVI/HDMI on shared port couldn't exist.
>
> This patch isn't tested without hardware wchich has HDMI
> on DDI-E.
>
> Signed-off-by: Xiong Zhang <xiong.y.zhang@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h   |  5 +++++
>  drivers/gpu/drm/i915/intel_bios.c | 28 ++++++++++++++++++++++++----
>  drivers/gpu/drm/i915/intel_hdmi.c | 18 ++++++++++++++++++
>  3 files changed, 47 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index 6d93334..5d8e7fe 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1414,6 +1414,10 @@ enum modeset_restore {
>  #define DP_AUX_C 0x20
>  #define DP_AUX_D 0x30
>
> +#define DDC_PIN_B  0x05
> +#define DDC_PIN_C  0x04
> +#define DDC_PIN_D  0x06
> +
>  struct ddi_vbt_port_info {
>         /*
>          * This is an index in the HDMI/DVI DDI buffer translation table.
> @@ -1428,6 +1432,7 @@ struct ddi_vbt_port_info {
>         uint8_t supports_dp:1;
>
>         uint8_t alternate_aux_channel;
> +       uint8_t alternate_ddc_pin;
>  };
>
>  enum psr_lines_to_wait {
> diff --git a/drivers/gpu/drm/i915/intel_bios.c
> b/drivers/gpu/drm/i915/intel_bios.c
> index 16cdf17..265227f 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -894,7 +894,7 @@ static void parse_ddi_port(struct drm_i915_private
> *dev_priv, enum port port,
>         uint8_t hdmi_level_shift;
>         int i, j;
>         bool is_dvi, is_hdmi, is_dp, is_edp, is_crt;
> -       uint8_t aux_channel;
> +       uint8_t aux_channel, ddc_pin;
>         /* Each DDI port can have more than one value on the "DVO Port"
> field,
>          * so look for all the possible values for each port and abort if
> more
>          * than one is found. */
> @@ -928,6 +928,7 @@ static void parse_ddi_port(struct drm_i915_private
> *dev_priv, enum port port,
>                 return;
>
>         aux_channel = child->raw[25];
> +       ddc_pin = child->common.ddc_pin;
>
>         is_dvi = child->common.device_type &
> DEVICE_TYPE_TMDS_DVI_SIGNALING;
>         is_dp = child->common.device_type & DEVICE_TYPE_DISPLAYPORT_OUTPUT;
> @@ -959,11 +960,30 @@ 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 (child->common.ddc_pin == 0x05 && port != PORT_B)
> +               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.
>

I see a trailing space here

+                        * 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;
> +                       }
>

and here

But the concept and patch is totally right so with trailing whitespaces
fixed
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>



> +                       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");
> -               if (child->common.ddc_pin == 0x04 && port != PORT_C)
> +               else if (ddc_pin == DDC_PIN_C && port != PORT_C)
>                         DRM_DEBUG_KMS("Unexpected DDC pin for port C\n");
> -               if (child->common.ddc_pin == 0x06 && port != PORT_D)
> +               else if (ddc_pin == DDC_PIN_D && port != PORT_D)
>                         DRM_DEBUG_KMS("Unexpected DDC pin for port D\n");
>         }


> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> b/drivers/gpu/drm/i915/intel_hdmi.c
> index 70bad5b..e1f6e81 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1991,6 +1991,24 @@ void intel_hdmi_init_connector(struct
> intel_digital_port *intel_dig_port,
>                         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. */
> +               switch
> (dev_priv->vbt.ddi_port_info[PORT_E].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:
> +                       BUG();
> +               }
> +               intel_encoder->hpd_pin = HPD_PORT_E;
> +               break;
>         case PORT_A:
>                 intel_encoder->hpd_pin = HPD_PORT_A;
>                 /* Internal port only for eDP. */
> --
> 2.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
Daniel Vetter Aug. 11, 2015, 9:58 a.m. UTC | #2
On Thu, Aug 06, 2015 at 03:51:41PM +0800, Xiong Zhang wrote:
> DDI-E doesn't have the correspondent GMBUS pin.
> 
> We rely on VBT to tell us which one it being used instead.
> 
> The DVI/HDMI on shared port couldn't exist.
> 
> This patch isn't tested without hardware wchich has HDMI
> on DDI-E.
> 
> Signed-off-by: Xiong Zhang <xiong.y.zhang@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h   |  5 +++++
>  drivers/gpu/drm/i915/intel_bios.c | 28 ++++++++++++++++++++++++----
>  drivers/gpu/drm/i915/intel_hdmi.c | 18 ++++++++++++++++++
>  3 files changed, 47 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 6d93334..5d8e7fe 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1414,6 +1414,10 @@ enum modeset_restore {
>  #define DP_AUX_C 0x20
>  #define DP_AUX_D 0x30
>  
> +#define DDC_PIN_B  0x05
> +#define DDC_PIN_C  0x04
> +#define DDC_PIN_D  0x06
> +
>  struct ddi_vbt_port_info {
>  	/*
>  	 * This is an index in the HDMI/DVI DDI buffer translation table.
> @@ -1428,6 +1432,7 @@ struct ddi_vbt_port_info {
>  	uint8_t supports_dp:1;
>  
>  	uint8_t alternate_aux_channel;
> +	uint8_t alternate_ddc_pin;
>  };
>  
>  enum psr_lines_to_wait {
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 16cdf17..265227f 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -894,7 +894,7 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
>  	uint8_t hdmi_level_shift;
>  	int i, j;
>  	bool is_dvi, is_hdmi, is_dp, is_edp, is_crt;
> -	uint8_t aux_channel;
> +	uint8_t aux_channel, ddc_pin;
>  	/* Each DDI port can have more than one value on the "DVO Port" field,
>  	 * so look for all the possible values for each port and abort if more
>  	 * than one is found. */
> @@ -928,6 +928,7 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
>  		return;
>  
>  	aux_channel = child->raw[25];
> +	ddc_pin = child->common.ddc_pin;
>  
>  	is_dvi = child->common.device_type & DEVICE_TYPE_TMDS_DVI_SIGNALING;
>  	is_dp = child->common.device_type & DEVICE_TYPE_DISPLAYPORT_OUTPUT;
> @@ -959,11 +960,30 @@ 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 (child->common.ddc_pin == 0x05 && port != PORT_B)
> +		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");
> -		if (child->common.ddc_pin == 0x04 && port != PORT_C)
> +		else if (ddc_pin == DDC_PIN_C && port != PORT_C)
>  			DRM_DEBUG_KMS("Unexpected DDC pin for port C\n");
> -		if (child->common.ddc_pin == 0x06 && port != PORT_D)
> +		else if (ddc_pin == DDC_PIN_D && port != PORT_D)
>  			DRM_DEBUG_KMS("Unexpected DDC pin for port D\n");
>  	}
>  
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 70bad5b..e1f6e81 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1991,6 +1991,24 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
>  			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. */
> +		switch (dev_priv->vbt.ddi_port_info[PORT_E].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:
> +			BUG();

BUG considered evil. Please use MISSING_CASE instead. The only places
where I accept a BUG is where a) it will blow up right afterwards anyway
b) the BUG_ON adds useful information c) there's no easy way to get out
without killing the kernel.

BUG in modeset code is really bad since the first modeset is done under
console_lock, which means _nothing_ at all reaches dmesg if we die in
there.

Also your patch has some whitespace issues, please fix those too.

I merged the other reviewed patches from this series, except the other one
I commented on.

Thanks, Daniel

> +		}
> +		intel_encoder->hpd_pin = HPD_PORT_E;
> +		break;
>  	case PORT_A:
>  		intel_encoder->hpd_pin = HPD_PORT_A;
>  		/* Internal port only for eDP. */
> -- 
> 2.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Shuang He Aug. 12, 2015, 2:19 p.m. UTC | #3
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 7093
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
ILK                                  302/302              302/302
SNB                                  315/315              315/315
IVB                                  336/336              336/336
BYT                 -2              283/283              281/283
HSW                                  378/378              378/378
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*BYT  igt@gem_persistent_relocs@interruptible      PASS(1)      FAIL(1)
*BYT  igt@gem_tiled_partial_pwrite_pread@reads      PASS(1)      FAIL(1)
Note: You need to pay more attention to line start with '*'

Patch
diff mbox

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6d93334..5d8e7fe 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1414,6 +1414,10 @@  enum modeset_restore {
 #define DP_AUX_C 0x20
 #define DP_AUX_D 0x30
 
+#define DDC_PIN_B  0x05
+#define DDC_PIN_C  0x04
+#define DDC_PIN_D  0x06
+
 struct ddi_vbt_port_info {
 	/*
 	 * This is an index in the HDMI/DVI DDI buffer translation table.
@@ -1428,6 +1432,7 @@  struct ddi_vbt_port_info {
 	uint8_t supports_dp:1;
 
 	uint8_t alternate_aux_channel;
+	uint8_t alternate_ddc_pin;
 };
 
 enum psr_lines_to_wait {
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 16cdf17..265227f 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -894,7 +894,7 @@  static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
 	uint8_t hdmi_level_shift;
 	int i, j;
 	bool is_dvi, is_hdmi, is_dp, is_edp, is_crt;
-	uint8_t aux_channel;
+	uint8_t aux_channel, ddc_pin;
 	/* Each DDI port can have more than one value on the "DVO Port" field,
 	 * so look for all the possible values for each port and abort if more
 	 * than one is found. */
@@ -928,6 +928,7 @@  static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
 		return;
 
 	aux_channel = child->raw[25];
+	ddc_pin = child->common.ddc_pin;
 
 	is_dvi = child->common.device_type & DEVICE_TYPE_TMDS_DVI_SIGNALING;
 	is_dp = child->common.device_type & DEVICE_TYPE_DISPLAYPORT_OUTPUT;
@@ -959,11 +960,30 @@  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 (child->common.ddc_pin == 0x05 && port != PORT_B)
+		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");
-		if (child->common.ddc_pin == 0x04 && port != PORT_C)
+		else if (ddc_pin == DDC_PIN_C && port != PORT_C)
 			DRM_DEBUG_KMS("Unexpected DDC pin for port C\n");
-		if (child->common.ddc_pin == 0x06 && port != PORT_D)
+		else if (ddc_pin == DDC_PIN_D && port != PORT_D)
 			DRM_DEBUG_KMS("Unexpected DDC pin for port D\n");
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 70bad5b..e1f6e81 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1991,6 +1991,24 @@  void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
 			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. */
+		switch (dev_priv->vbt.ddi_port_info[PORT_E].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:
+			BUG();
+		}
+		intel_encoder->hpd_pin = HPD_PORT_E;
+		break;
 	case PORT_A:
 		intel_encoder->hpd_pin = HPD_PORT_A;
 		/* Internal port only for eDP. */