Hi 2012/6/12 Daniel Vetter <[email protected]>: > This function is supposed to be used at mode set time, so prevent > against future mistakes by adding a WARN(). > > Based on a patch by Paulo Zanoni, with the check extracted into a > little assert_hdmi_port_disabled helper added to make things self > documenting and move the assert stuff out of line.
I was going to write the patch, but thanks for doing it :) Comments below: > > Cc: Paulo Zanoni <[email protected]> > Signed-Off-by: Daniel Vetter <[email protected]> > --- > drivers/gpu/drm/i915/intel_hdmi.c | 23 +++++++++++++++++++++++ > 1 files changed, 23 insertions(+), 0 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c > b/drivers/gpu/drm/i915/intel_hdmi.c > index 69637db..aac9a5d 100644 > --- a/drivers/gpu/drm/i915/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > @@ -37,6 +37,19 @@ > #include "i915_drm.h" > #include "i915_drv.h" > > +static void > +assert_hdmi_port_disabled(struct intel_hdmi *intel_hdmi) > +{ > + struct drm_device *dev = intel_hdmi->base.base.dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > + uint32_t enabled_bits; > + > + enabled_bits = IS_HASWELL(dev) ? DDI_BUF_CTL_ENABLE : SDVO_ENABLE; My crystal ball tells me a check for IS_HASWELL_OR_NEWER would prevent more changes to this code in the future. > + > + WARN(I915_READ(intel_hdmi->sdvox_reg) & enabled_bits, > + "HDMI port enabled, expectin disabled\n"); expectinG? > +} > + > struct intel_hdmi *enc_to_intel_hdmi(struct drm_encoder *encoder) > { > return container_of(encoder, struct intel_hdmi, base.base); > @@ -334,6 +347,8 @@ static void g4x_set_infoframes(struct drm_encoder > *encoder, > u32 val = I915_READ(reg); > u32 port; > > + assert_hdmi_port_disabled(intel_hdmi); > + > /* If the registers were not initialized yet, they might be zeroes, > * which means we're selecting the AVI DIP and we're setting its > * frequency to once. This seems to really confuse the HW and make > @@ -395,6 +410,8 @@ static void ibx_set_infoframes(struct drm_encoder > *encoder, > u32 val = I915_READ(reg); > u32 port; > > + assert_hdmi_port_disabled(intel_hdmi); > + > /* See the big comment in g4x_set_infoframes() */ > val |= VIDEO_DIP_SELECT_AVI | VIDEO_DIP_FREQ_VSYNC; > > @@ -451,6 +468,8 @@ static void cpt_set_infoframes(struct drm_encoder > *encoder, > u32 reg = TVIDEO_DIP_CTL(intel_crtc->pipe); > u32 val = I915_READ(reg); > > + assert_hdmi_port_disabled(intel_hdmi); > + > /* See the big comment in g4x_set_infoframes() */ > val |= VIDEO_DIP_SELECT_AVI | VIDEO_DIP_FREQ_VSYNC; > > @@ -484,6 +503,8 @@ static void vlv_set_infoframes(struct drm_encoder > *encoder, > u32 reg = VLV_TVIDEO_DIP_CTL(intel_crtc->pipe); > u32 val = I915_READ(reg); > > + assert_hdmi_port_disabled(intel_hdmi); > + > /* See the big comment in g4x_set_infoframes() */ > val |= VIDEO_DIP_SELECT_AVI | VIDEO_DIP_FREQ_VSYNC; > > @@ -516,6 +537,8 @@ static void hsw_set_infoframes(struct drm_encoder > *encoder, > u32 reg = HSW_TVIDEO_DIP_CTL(intel_crtc->pipe); > u32 val = I915_READ(reg); > > + assert_hdmi_port_disabled(intel_hdmi); > + > if (!intel_hdmi->has_hdmi_sink) { > I915_WRITE(reg, 0); > POSTING_READ(reg); > -- > 1.7.7.6 > > _______________________________________________ > Intel-gfx mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Paulo Zanoni _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
