| Message ID | 1342117199-26089-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 >
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
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(); }
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(-)