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@xxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
