From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paulo Zanoni Subject: Re: [PATCH] drm/i915: add port parameter to intel_hdmi_init Date: Thu, 12 Jul 2012 16:49:34 -0300 Message-ID: References: <1342016944-23395-21-git-send-email-daniel.vetter@ffwll.ch> <1342117199-26089-1-git-send-email-daniel.vetter@ffwll.ch> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-yx0-f177.google.com (mail-yx0-f177.google.com [209.85.213.177]) by gabe.freedesktop.org (Postfix) with ESMTP id 7A6F3A0FBD for ; Thu, 12 Jul 2012 12:49:35 -0700 (PDT) Received: by yenr9 with SMTP id r9so3080982yen.36 for ; Thu, 12 Jul 2012 12:49:35 -0700 (PDT) In-Reply-To: <1342117199-26089-1-git-send-email-daniel.vetter@ffwll.ch> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Daniel Vetter Cc: Intel Graphics Development List-Id: intel-gfx@lists.freedesktop.org 2012/7/12 Daniel Vetter : > 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 Reviewed-by: Paulo Zanoni > --- > 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 > -- Paulo Zanoni