Re: [PATCH 4/4] drm/i915: move infoframe setting to after port enable

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Thu, 3 Apr 2014 17:19:56 +0200
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Wed, Apr 02, 2014 at 10:08:54AM -0700, Jesse Barnes wrote:
> > Needs to happen after clock is running or it doesn't behave correctly.
> > 
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > ---
> >  drivers/gpu/drm/i915/intel_hdmi.c |    6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> > index fb9839b..a4ca63b6 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -669,8 +669,6 @@ static void intel_hdmi_mode_set(struct intel_encoder *encoder)
> >  
> >  	I915_WRITE(intel_hdmi->hdmi_reg, hdmi_val);
> >  	POSTING_READ(intel_hdmi->hdmi_reg);
> > -
> > -	intel_hdmi->set_infoframes(&encoder->base, adjusted_mode);
> >  }
> >  
> >  static bool intel_hdmi_get_hw_state(struct intel_encoder *encoder,
> > @@ -738,9 +736,13 @@ static void intel_enable_hdmi(struct intel_encoder *encoder)
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >  	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> >  	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
> > +	struct drm_display_mode *adjusted_mode =
> > +		&intel_crtc->config.adjusted_mode;
> >  	u32 temp;
> >  	u32 enable_bits = SDVO_ENABLE;
> >  
> > +	intel_hdmi->set_infoframes(&encoder->base, adjusted_mode);
> > +
> >  	if (intel_hdmi->has_audio)
> >  		enable_bits |= SDVO_AUDIO_ENABLE;
> 
> That kind of change tends to freak out Paulo, our master of infoframes. Do
> doecs really state that this is how stuff should work in general, or is
> this just a gm45/vlv thing? Or vlv only?
> 
> /me remembers how often we've burnt our hands here

Hey infoframe emission was totally broken for awhile due to a generic
change, and we didn't notice that right away. :)

But yeah I'd prefer to test this on multiple platforms first, but don't
have that capability.  It does pass on BYT though, and the logic should
be similar to IBX, so this change ought to be safe.  It's easy to
revert too and make platform specific if we get regression reports, but
I expect it to fix weird issues instead of introducing new ones, based
on the infoframe analyzer results we have from BYT.

-- 
Jesse Barnes, Intel Open Source Technology Center