drm/i915: add port parameter to intel_hdmi_init
diff mbox

Message ID 1342117199-26089-1-git-send-email-daniel.vetter@ffwll.ch
State New, archived
Headers show

Commit Message

Daniel Vetter July 12, 2012, 6:19 p.m. UTC
Instead of having a giant if cascade to figure this out according to
the passed-in register. We could do quite a bit more cleaning up and
all by using the port at more places, but I think this should be part
of a bigger rework to introduce a struct intel_digital_port which
would keep track of all these things. I guess this will be part of
some haswell-DP-induced refactoring.

For now this rips out the big cascade, which is what annoyed me so
much.

v2: Add port variable name back for the func decl (I've tried to trick
myself below the 80 char limit).

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_ddi.c     |    2 +-
 drivers/gpu/drm/i915/intel_display.c |   14 +++++-----
 drivers/gpu/drm/i915/intel_drv.h     |    3 +-
 drivers/gpu/drm/i915/intel_hdmi.c    |   41 ++++++++++------------------------
 4 files changed, 22 insertions(+), 38 deletions(-)

Comments

Paulo Zanoni July 12, 2012, 7:49 p.m. UTC | #1
2012/7/12 Daniel Vetter <daniel.vetter@ffwll.ch>:
> Instead of having a giant if cascade to figure this out according to
> the passed-in register. We could do quite a bit more cleaning up and
> all by using the port at more places, but I think this should be part
> of a bigger rework to introduce a struct intel_digital_port which
> would keep track of all these things. I guess this will be part of
> some haswell-DP-induced refactoring.
>
> For now this rips out the big cascade, which is what annoyed me so
> much.
>
> v2: Add port variable name back for the func decl (I've tried to trick
> myself below the 80 char limit).
>
> Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_ddi.c     |    2 +-
>  drivers/gpu/drm/i915/intel_display.c |   14 +++++-----
>  drivers/gpu/drm/i915/intel_drv.h     |    3 +-
>  drivers/gpu/drm/i915/intel_hdmi.c    |   41 ++++++++++------------------------
>  4 files changed, 22 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index fd60f48..5370d17 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -250,7 +250,7 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
>         case PORT_B:
>         case PORT_C:
>         case PORT_D:
> -               intel_hdmi_init(dev, DDI_BUF_CTL(port));
> +               intel_hdmi_init(dev, DDI_BUF_CTL(port), port);
>                 break;
>         default:
>                 DRM_DEBUG_DRIVER("No handlers defined for port %d, skipping DDI initialization\n",
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 38891ff..3769e5f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6807,16 +6807,16 @@ static void intel_setup_outputs(struct drm_device *dev)
>                         /* PCH SDVOB multiplex with HDMIB */
>                         found = intel_sdvo_init(dev, PCH_SDVOB, true);
>                         if (!found)
> -                               intel_hdmi_init(dev, HDMIB);
> +                               intel_hdmi_init(dev, HDMIB, PORT_B);
>                         if (!found && (I915_READ(PCH_DP_B) & DP_DETECTED))
>                                 intel_dp_init(dev, PCH_DP_B);
>                 }
>
>                 if (I915_READ(HDMIC) & PORT_DETECTED)
> -                       intel_hdmi_init(dev, HDMIC);
> +                       intel_hdmi_init(dev, HDMIC, PORT_C);
>
>                 if (!dpd_is_edp && I915_READ(HDMID) & PORT_DETECTED)
> -                       intel_hdmi_init(dev, HDMID);
> +                       intel_hdmi_init(dev, HDMID, PORT_D);
>
>                 if (I915_READ(PCH_DP_C) & DP_DETECTED)
>                         intel_dp_init(dev, PCH_DP_C);
> @@ -6830,13 +6830,13 @@ static void intel_setup_outputs(struct drm_device *dev)
>                         /* SDVOB multiplex with HDMIB */
>                         found = intel_sdvo_init(dev, SDVOB, true);
>                         if (!found)
> -                               intel_hdmi_init(dev, SDVOB);
> +                               intel_hdmi_init(dev, SDVOB, PORT_B);
>                         if (!found && (I915_READ(DP_B) & DP_DETECTED))
>                                 intel_dp_init(dev, DP_B);
>                 }
>
>                 if (I915_READ(SDVOC) & PORT_DETECTED)
> -                       intel_hdmi_init(dev, SDVOC);
> +                       intel_hdmi_init(dev, SDVOC, PORT_C);
>
>                 /* Shares lanes with HDMI on SDVOC */
>                 if (I915_READ(DP_C) & DP_DETECTED)
> @@ -6849,7 +6849,7 @@ static void intel_setup_outputs(struct drm_device *dev)
>                         found = intel_sdvo_init(dev, SDVOB, true);
>                         if (!found && SUPPORTS_INTEGRATED_HDMI(dev)) {
>                                 DRM_DEBUG_KMS("probing HDMI on SDVOB\n");
> -                               intel_hdmi_init(dev, SDVOB);
> +                               intel_hdmi_init(dev, SDVOB, PORT_B);
>                         }
>
>                         if (!found && SUPPORTS_INTEGRATED_DP(dev)) {
> @@ -6869,7 +6869,7 @@ static void intel_setup_outputs(struct drm_device *dev)
>
>                         if (SUPPORTS_INTEGRATED_HDMI(dev)) {
>                                 DRM_DEBUG_KMS("probing HDMI on SDVOC\n");
> -                               intel_hdmi_init(dev, SDVOC);
> +                               intel_hdmi_init(dev, SDVOC, PORT_C);
>                         }
>                         if (SUPPORTS_INTEGRATED_DP(dev)) {
>                                 DRM_DEBUG_KMS("probing DP_C\n");
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index f8c1f04..ff78679 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -336,7 +336,8 @@ extern void intel_attach_force_audio_property(struct drm_connector *connector);
>  extern void intel_attach_broadcast_rgb_property(struct drm_connector *connector);
>
>  extern void intel_crt_init(struct drm_device *dev);
> -extern void intel_hdmi_init(struct drm_device *dev, int sdvox_reg);
> +extern void intel_hdmi_init(struct drm_device *dev,
> +                           int sdvox_reg, enum port port);
>  extern struct intel_hdmi *enc_to_intel_hdmi(struct drm_encoder *encoder);
>  extern void intel_dip_infoframe_csum(struct dip_infoframe *avi_if);
>  extern bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg,
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index b01900d..1332367 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -925,7 +925,7 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c
>         intel_attach_broadcast_rgb_property(connector);
>  }
>
> -void intel_hdmi_init(struct drm_device *dev, int sdvox_reg)
> +void intel_hdmi_init(struct drm_device *dev, int sdvox_reg, enum port port)
>  {
>         struct drm_i915_private *dev_priv = dev->dev_private;
>         struct drm_connector *connector;
> @@ -961,40 +961,23 @@ void intel_hdmi_init(struct drm_device *dev, int sdvox_reg)
>
>         intel_encoder->cloneable = false;
>
> -       /* Set up the DDC bus. */
> -       if (sdvox_reg == SDVOB) {
> +       intel_hdmi->ddi_port = port;
> +       switch (port) {
> +       case PORT_B:
>                 intel_hdmi->ddc_bus = GMBUS_PORT_DPB;
>                 dev_priv->hotplug_supported_mask |= HDMIB_HOTPLUG_INT_STATUS;
> -       } else if (sdvox_reg == SDVOC) {
> -               intel_hdmi->ddc_bus = GMBUS_PORT_DPC;
> -               dev_priv->hotplug_supported_mask |= HDMIC_HOTPLUG_INT_STATUS;
> -       } else if (sdvox_reg == HDMIB) {
> -               intel_hdmi->ddc_bus = GMBUS_PORT_DPB;
> -               dev_priv->hotplug_supported_mask |= HDMIB_HOTPLUG_INT_STATUS;
> -       } else if (sdvox_reg == HDMIC) {
> -               intel_hdmi->ddc_bus = GMBUS_PORT_DPC;
> -               dev_priv->hotplug_supported_mask |= HDMIC_HOTPLUG_INT_STATUS;
> -       } else if (sdvox_reg == HDMID) {
> -               intel_hdmi->ddc_bus = GMBUS_PORT_DPD;
> -               dev_priv->hotplug_supported_mask |= HDMID_HOTPLUG_INT_STATUS;
> -       } else if (sdvox_reg == DDI_BUF_CTL(PORT_B)) {
> -               DRM_DEBUG_DRIVER("LPT: detected output on DDI B\n");
> -               intel_hdmi->ddc_bus = GMBUS_PORT_DPB;
> -               intel_hdmi->ddi_port = PORT_B;
> -               dev_priv->hotplug_supported_mask |= HDMIB_HOTPLUG_INT_STATUS;
> -       } else if (sdvox_reg == DDI_BUF_CTL(PORT_C)) {
> -               DRM_DEBUG_DRIVER("LPT: detected output on DDI C\n");
> +               break;
> +       case PORT_C:
>                 intel_hdmi->ddc_bus = GMBUS_PORT_DPC;
> -               intel_hdmi->ddi_port = PORT_C;
>                 dev_priv->hotplug_supported_mask |= HDMIC_HOTPLUG_INT_STATUS;
> -       } else if (sdvox_reg == DDI_BUF_CTL(PORT_D)) {
> -               DRM_DEBUG_DRIVER("LPT: detected output on DDI D\n");
> +               break;
> +       case PORT_D:
>                 intel_hdmi->ddc_bus = GMBUS_PORT_DPD;
> -               intel_hdmi->ddi_port = PORT_D;
>                 dev_priv->hotplug_supported_mask |= HDMID_HOTPLUG_INT_STATUS;
> -       } else {
> -               /* If we got an unknown sdvox_reg, things are pretty much broken
> -                * in a way that we should let the kernel know about it */
> +               break;
> +       case PORT_A:
> +               /* Internal port only for eDP. */
> +       default:
>                 BUG();
>         }
>
> --
> 1.7.7.6
>
Daniel Vetter July 17, 2012, 8:43 a.m. UTC | #2
On Thu, Jul 12, 2012 at 04:49:34PM -0300, Paulo Zanoni wrote:
> 2012/7/12 Daniel Vetter <daniel.vetter@ffwll.ch>:
> > Instead of having a giant if cascade to figure this out according to
> > the passed-in register. We could do quite a bit more cleaning up and
> > all by using the port at more places, but I think this should be part
> > of a bigger rework to introduce a struct intel_digital_port which
> > would keep track of all these things. I guess this will be part of
> > some haswell-DP-induced refactoring.
> >
> > For now this rips out the big cascade, which is what annoyed me so
> > much.
> >
> > v2: Add port variable name back for the func decl (I've tried to trick
> > myself below the 80 char limit).
> >
> > Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

I've merged the three patches you've reviewed already to dinq (with the
comment in patch 13 fixed up), thanks for the review.
-Daniel

Patch
diff mbox

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index fd60f48..5370d17 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -250,7 +250,7 @@  void intel_ddi_init(struct drm_device *dev, enum port port)
 	case PORT_B:
 	case PORT_C:
 	case PORT_D:
-		intel_hdmi_init(dev, DDI_BUF_CTL(port));
+		intel_hdmi_init(dev, DDI_BUF_CTL(port), port);
 		break;
 	default:
 		DRM_DEBUG_DRIVER("No handlers defined for port %d, skipping DDI initialization\n",
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 38891ff..3769e5f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6807,16 +6807,16 @@  static void intel_setup_outputs(struct drm_device *dev)
 			/* PCH SDVOB multiplex with HDMIB */
 			found = intel_sdvo_init(dev, PCH_SDVOB, true);
 			if (!found)
-				intel_hdmi_init(dev, HDMIB);
+				intel_hdmi_init(dev, HDMIB, PORT_B);
 			if (!found && (I915_READ(PCH_DP_B) & DP_DETECTED))
 				intel_dp_init(dev, PCH_DP_B);
 		}
 
 		if (I915_READ(HDMIC) & PORT_DETECTED)
-			intel_hdmi_init(dev, HDMIC);
+			intel_hdmi_init(dev, HDMIC, PORT_C);
 
 		if (!dpd_is_edp && I915_READ(HDMID) & PORT_DETECTED)
-			intel_hdmi_init(dev, HDMID);
+			intel_hdmi_init(dev, HDMID, PORT_D);
 
 		if (I915_READ(PCH_DP_C) & DP_DETECTED)
 			intel_dp_init(dev, PCH_DP_C);
@@ -6830,13 +6830,13 @@  static void intel_setup_outputs(struct drm_device *dev)
 			/* SDVOB multiplex with HDMIB */
 			found = intel_sdvo_init(dev, SDVOB, true);
 			if (!found)
-				intel_hdmi_init(dev, SDVOB);
+				intel_hdmi_init(dev, SDVOB, PORT_B);
 			if (!found && (I915_READ(DP_B) & DP_DETECTED))
 				intel_dp_init(dev, DP_B);
 		}
 
 		if (I915_READ(SDVOC) & PORT_DETECTED)
-			intel_hdmi_init(dev, SDVOC);
+			intel_hdmi_init(dev, SDVOC, PORT_C);
 
 		/* Shares lanes with HDMI on SDVOC */
 		if (I915_READ(DP_C) & DP_DETECTED)
@@ -6849,7 +6849,7 @@  static void intel_setup_outputs(struct drm_device *dev)
 			found = intel_sdvo_init(dev, SDVOB, true);
 			if (!found && SUPPORTS_INTEGRATED_HDMI(dev)) {
 				DRM_DEBUG_KMS("probing HDMI on SDVOB\n");
-				intel_hdmi_init(dev, SDVOB);
+				intel_hdmi_init(dev, SDVOB, PORT_B);
 			}
 
 			if (!found && SUPPORTS_INTEGRATED_DP(dev)) {
@@ -6869,7 +6869,7 @@  static void intel_setup_outputs(struct drm_device *dev)
 
 			if (SUPPORTS_INTEGRATED_HDMI(dev)) {
 				DRM_DEBUG_KMS("probing HDMI on SDVOC\n");
-				intel_hdmi_init(dev, SDVOC);
+				intel_hdmi_init(dev, SDVOC, PORT_C);
 			}
 			if (SUPPORTS_INTEGRATED_DP(dev)) {
 				DRM_DEBUG_KMS("probing DP_C\n");
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f8c1f04..ff78679 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -336,7 +336,8 @@  extern void intel_attach_force_audio_property(struct drm_connector *connector);
 extern void intel_attach_broadcast_rgb_property(struct drm_connector *connector);
 
 extern void intel_crt_init(struct drm_device *dev);
-extern void intel_hdmi_init(struct drm_device *dev, int sdvox_reg);
+extern void intel_hdmi_init(struct drm_device *dev,
+			    int sdvox_reg, enum port port);
 extern struct intel_hdmi *enc_to_intel_hdmi(struct drm_encoder *encoder);
 extern void intel_dip_infoframe_csum(struct dip_infoframe *avi_if);
 extern bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg,
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index b01900d..1332367 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -925,7 +925,7 @@  intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c
 	intel_attach_broadcast_rgb_property(connector);
 }
 
-void intel_hdmi_init(struct drm_device *dev, int sdvox_reg)
+void intel_hdmi_init(struct drm_device *dev, int sdvox_reg, enum port port)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_connector *connector;
@@ -961,40 +961,23 @@  void intel_hdmi_init(struct drm_device *dev, int sdvox_reg)
 
 	intel_encoder->cloneable = false;
 
-	/* Set up the DDC bus. */
-	if (sdvox_reg == SDVOB) {
+	intel_hdmi->ddi_port = port;
+	switch (port) {
+	case PORT_B:
 		intel_hdmi->ddc_bus = GMBUS_PORT_DPB;
 		dev_priv->hotplug_supported_mask |= HDMIB_HOTPLUG_INT_STATUS;
-	} else if (sdvox_reg == SDVOC) {
-		intel_hdmi->ddc_bus = GMBUS_PORT_DPC;
-		dev_priv->hotplug_supported_mask |= HDMIC_HOTPLUG_INT_STATUS;
-	} else if (sdvox_reg == HDMIB) {
-		intel_hdmi->ddc_bus = GMBUS_PORT_DPB;
-		dev_priv->hotplug_supported_mask |= HDMIB_HOTPLUG_INT_STATUS;
-	} else if (sdvox_reg == HDMIC) {
-		intel_hdmi->ddc_bus = GMBUS_PORT_DPC;
-		dev_priv->hotplug_supported_mask |= HDMIC_HOTPLUG_INT_STATUS;
-	} else if (sdvox_reg == HDMID) {
-		intel_hdmi->ddc_bus = GMBUS_PORT_DPD;
-		dev_priv->hotplug_supported_mask |= HDMID_HOTPLUG_INT_STATUS;
-	} else if (sdvox_reg == DDI_BUF_CTL(PORT_B)) {
-		DRM_DEBUG_DRIVER("LPT: detected output on DDI B\n");
-		intel_hdmi->ddc_bus = GMBUS_PORT_DPB;
-		intel_hdmi->ddi_port = PORT_B;
-		dev_priv->hotplug_supported_mask |= HDMIB_HOTPLUG_INT_STATUS;
-	} else if (sdvox_reg == DDI_BUF_CTL(PORT_C)) {
-		DRM_DEBUG_DRIVER("LPT: detected output on DDI C\n");
+		break;
+	case PORT_C:
 		intel_hdmi->ddc_bus = GMBUS_PORT_DPC;
-		intel_hdmi->ddi_port = PORT_C;
 		dev_priv->hotplug_supported_mask |= HDMIC_HOTPLUG_INT_STATUS;
-	} else if (sdvox_reg == DDI_BUF_CTL(PORT_D)) {
-		DRM_DEBUG_DRIVER("LPT: detected output on DDI D\n");
+		break;
+	case PORT_D:
 		intel_hdmi->ddc_bus = GMBUS_PORT_DPD;
-		intel_hdmi->ddi_port = PORT_D;
 		dev_priv->hotplug_supported_mask |= HDMID_HOTPLUG_INT_STATUS;
-	} else {
-		/* If we got an unknown sdvox_reg, things are pretty much broken
-		 * in a way that we should let the kernel know about it */
+		break;
+	case PORT_A:
+		/* Internal port only for eDP. */
+	default:
 		BUG();
 	}