> -----Original Message----- > From: Intel-gfx [mailto:intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf Of > ville.syrjala@xxxxxxxxxxxxxxx > Sent: Tuesday, May 05, 2015 7:06 AM > To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx > Subject: [PATCH v2 6/9] drm/i915: Disable all infoframes when > turning off the HDMI port > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > Currently we just disable the GCP infoframe when turning off the port. > That means if the same transcoder is used on a DP port next, we might > end up pushing infoframes over DP, which isn't intended. Just disable Wonder how it is working. May be it is ok, or never hit using a previously used transcoder for HDMI port for DP. By the way, you mean end up pushing "other" infoframes over DP? > all the infoframes when turning off the port. > > Also protect against two ports stomping on each other on g4x due to > the single video DIP instance. Now only the first port to enable > gets to send infoframes. So is, 2nd port doesn't gets to send infoframes, the expected behavior when two ports trying with a single DIP? Seems like a corner case to test thoroughly. Is this path exercised in your testing? > > v2: Rebase > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > --- > drivers/gpu/drm/i915/intel_hdmi.c | 85 ++++++++++++++++++--------------------- > 1 file changed, 40 insertions(+), 45 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c > b/drivers/gpu/drm/i915/intel_hdmi.c > index 766bdb1..03b4759 100644 > --- a/drivers/gpu/drm/i915/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > @@ -514,7 +514,13 @@ static void g4x_set_infoframes(struct drm_encoder > *encoder, > if (!enable) { > if (!(val & VIDEO_DIP_ENABLE)) > return; > - val &= ~VIDEO_DIP_ENABLE; > + if (port != (val & VIDEO_DIP_PORT_MASK)) { > + DRM_DEBUG_KMS("video DIP still enabled on port > %c\n", > + (val & VIDEO_DIP_PORT_MASK) >> 29); > + return; > + } > + val &= ~(VIDEO_DIP_ENABLE | VIDEO_DIP_ENABLE_AVI | > + VIDEO_DIP_ENABLE_VENDOR | > VIDEO_DIP_ENABLE_SPD); > I915_WRITE(reg, val); > POSTING_READ(reg); > return; > @@ -522,16 +528,17 @@ static void g4x_set_infoframes(struct drm_encoder > *encoder, > > if (port != (val & VIDEO_DIP_PORT_MASK)) { > if (val & VIDEO_DIP_ENABLE) { > - val &= ~VIDEO_DIP_ENABLE; > - I915_WRITE(reg, val); > - POSTING_READ(reg); > + DRM_DEBUG_KMS("video DIP already enabled on port > %c\n", > + (val & VIDEO_DIP_PORT_MASK) >> 29); > + return; > } > val &= ~VIDEO_DIP_PORT_MASK; > val |= port; > } > > val |= VIDEO_DIP_ENABLE; > - val &= ~VIDEO_DIP_ENABLE_VENDOR; > + val &= ~(VIDEO_DIP_ENABLE_AVI | > + VIDEO_DIP_ENABLE_VENDOR | VIDEO_DIP_ENABLE_SPD); > > I915_WRITE(reg, val); > POSTING_READ(reg); > @@ -632,23 +639,6 @@ static bool intel_hdmi_set_gcp_infoframe(struct > drm_encoder *encoder) > return val != 0; > } > > -static void intel_disable_gcp_infoframe(struct intel_crtc *crtc) > -{ > - struct drm_i915_private *dev_priv = crtc->base.dev->dev_private; > - u32 reg; > - > - if (HAS_DDI(dev_priv)) > - reg = HSW_TVIDEO_DIP_CTL(crtc->config->cpu_transcoder); > - else if (IS_VALLEYVIEW(dev_priv)) > - reg = VLV_TVIDEO_DIP_CTL(crtc->pipe); > - else if (HAS_PCH_SPLIT(dev_priv->dev)) > - reg = TVIDEO_DIP_CTL(crtc->pipe); > - else > - return; > - > - I915_WRITE(reg, I915_READ(reg) & ~VIDEO_DIP_ENABLE_GCP); > -} > - > static void ibx_set_infoframes(struct drm_encoder *encoder, > bool enable, > struct drm_display_mode *adjusted_mode) > @@ -669,25 +659,26 @@ static void ibx_set_infoframes(struct drm_encoder > *encoder, > if (!enable) { > if (!(val & VIDEO_DIP_ENABLE)) > return; > - val &= ~VIDEO_DIP_ENABLE; > + val &= ~(VIDEO_DIP_ENABLE | VIDEO_DIP_ENABLE_AVI | > + VIDEO_DIP_ENABLE_VENDOR | VIDEO_DIP_ENABLE_GAMUT | > + VIDEO_DIP_ENABLE_SPD | VIDEO_DIP_ENABLE_GCP); > I915_WRITE(reg, val); > POSTING_READ(reg); > return; > } > > if (port != (val & VIDEO_DIP_PORT_MASK)) { > - if (val & VIDEO_DIP_ENABLE) { > - val &= ~VIDEO_DIP_ENABLE; > - I915_WRITE(reg, val); > - POSTING_READ(reg); > - } > + WARN(val & VIDEO_DIP_ENABLE, > + "DIP already enabled on port %c\n", > + (val & VIDEO_DIP_PORT_MASK) >> 29); > val &= ~VIDEO_DIP_PORT_MASK; > val |= port; > } > > val |= VIDEO_DIP_ENABLE; > - val &= ~(VIDEO_DIP_ENABLE_VENDOR | VIDEO_DIP_ENABLE_GAMUT | > - VIDEO_DIP_ENABLE_GCP); > + val &= ~(VIDEO_DIP_ENABLE_AVI | > + VIDEO_DIP_ENABLE_VENDOR | VIDEO_DIP_ENABLE_GAMUT | > + VIDEO_DIP_ENABLE_SPD | VIDEO_DIP_ENABLE_GCP); > > if (intel_hdmi_set_gcp_infoframe(encoder)) > val |= VIDEO_DIP_ENABLE_GCP; > @@ -718,7 +709,9 @@ static void cpt_set_infoframes(struct drm_encoder > *encoder, > if (!enable) { > if (!(val & VIDEO_DIP_ENABLE)) > return; > - val &= ~(VIDEO_DIP_ENABLE | VIDEO_DIP_ENABLE_AVI); > + val &= ~(VIDEO_DIP_ENABLE | VIDEO_DIP_ENABLE_AVI | > + VIDEO_DIP_ENABLE_VENDOR | > VIDEO_DIP_ENABLE_GAMUT | > + VIDEO_DIP_ENABLE_SPD | VIDEO_DIP_ENABLE_GCP); > I915_WRITE(reg, val); > POSTING_READ(reg); > return; > @@ -727,7 +720,7 @@ static void cpt_set_infoframes(struct drm_encoder > *encoder, > /* Set both together, unset both together: see the spec. */ > val |= VIDEO_DIP_ENABLE | VIDEO_DIP_ENABLE_AVI; > val &= ~(VIDEO_DIP_ENABLE_VENDOR | VIDEO_DIP_ENABLE_GAMUT | > - VIDEO_DIP_ENABLE_GCP); > + VIDEO_DIP_ENABLE_SPD | VIDEO_DIP_ENABLE_GCP); > > if (intel_hdmi_set_gcp_infoframe(encoder)) > val |= VIDEO_DIP_ENABLE_GCP; > @@ -760,25 +753,26 @@ static void vlv_set_infoframes(struct drm_encoder > *encoder, > if (!enable) { > if (!(val & VIDEO_DIP_ENABLE)) > return; > - val &= ~VIDEO_DIP_ENABLE; > + val &= ~(VIDEO_DIP_ENABLE | VIDEO_DIP_ENABLE_AVI | > + VIDEO_DIP_ENABLE_VENDOR | VIDEO_DIP_ENABLE_GAMUT | > + VIDEO_DIP_ENABLE_SPD | VIDEO_DIP_ENABLE_GCP); > I915_WRITE(reg, val); > POSTING_READ(reg); > return; > } > > if (port != (val & VIDEO_DIP_PORT_MASK)) { > - if (val & VIDEO_DIP_ENABLE) { > - val &= ~VIDEO_DIP_ENABLE; > - I915_WRITE(reg, val); > - POSTING_READ(reg); > - } > + WARN(val & VIDEO_DIP_ENABLE, > + "DIP already enabled on port %c\n", > + (val & VIDEO_DIP_PORT_MASK) >> 29); > val &= ~VIDEO_DIP_PORT_MASK; > val |= port; > } > > val |= VIDEO_DIP_ENABLE; > - val &= ~(VIDEO_DIP_ENABLE_AVI | VIDEO_DIP_ENABLE_VENDOR | > - VIDEO_DIP_ENABLE_GAMUT | VIDEO_DIP_ENABLE_GCP); > + val &= ~(VIDEO_DIP_ENABLE_AVI | > + VIDEO_DIP_ENABLE_VENDOR | VIDEO_DIP_ENABLE_GAMUT | > + VIDEO_DIP_ENABLE_SPD | VIDEO_DIP_ENABLE_GCP); > > if (intel_hdmi_set_gcp_infoframe(encoder)) > val |= VIDEO_DIP_ENABLE_GCP; > @@ -803,15 +797,16 @@ static void hsw_set_infoframes(struct drm_encoder > *encoder, > > assert_hdmi_port_disabled(intel_hdmi); > > + val &= ~(VIDEO_DIP_ENABLE_VSC_HSW | VIDEO_DIP_ENABLE_AVI_HSW | > + VIDEO_DIP_ENABLE_GCP_HSW | VIDEO_DIP_ENABLE_VS_HSW | > + VIDEO_DIP_ENABLE_GMP_HSW | VIDEO_DIP_ENABLE_SPD_HSW); > + > if (!enable) { > - I915_WRITE(reg, 0); > + I915_WRITE(reg, val); > POSTING_READ(reg); > return; > } > > - val &= ~(VIDEO_DIP_ENABLE_VSC_HSW | VIDEO_DIP_ENABLE_GCP_HSW | > - VIDEO_DIP_ENABLE_VS_HSW | VIDEO_DIP_ENABLE_GMP_HSW); > - > if (intel_hdmi_set_gcp_infoframe(encoder)) > val |= VIDEO_DIP_ENABLE_GCP_HSW; > > @@ -1133,7 +1128,7 @@ static void intel_disable_hdmi(struct intel_encoder > *encoder) > if (IS_CHERRYVIEW(dev)) > chv_powergate_phy_lanes(encoder, 0xf); > > - intel_disable_gcp_infoframe(to_intel_crtc(encoder->base.crtc)); > + intel_hdmi->set_infoframes(&encoder->base, false, NULL); > } > > static int hdmi_portclock_limit(struct intel_hdmi *hdmi, bool respect_dvi_limit) > -- > 2.0.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
