| Message ID | 1438847501-28454-6-git-send-email-xiong.y.zhang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 >
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
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 '*'
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. */
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(-)