[Intel-gfx] [PATCH v3 1/3] drm/i915: set proper N/M in modeset

[Ville Syrjälä]

On Fri, Aug 05, 2016 at 08:55:27AM +0000, Yang, Libin wrote:
> 
> > -----Original Message-----
> > From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com]
> > Sent: Friday, August 5, 2016 4:45 PM
> > To: Yang, Libin <libin.yang at intel.com>
> > Cc: libin.yang at linux.intel.com; intel-gfx at lists.freedesktop.org;
> > jani.nikula at linux.intel.com; Vetter, Daniel <daniel.vetter at intel.com>;
> > tiwai at suse.de
> > Subject: Re: [PATCH v3 1/3] drm/i915: set proper N/M in modeset
> > 
> > On Fri, Aug 05, 2016 at 07:24:14AM +0000, Yang, Libin wrote:
> > >
> > > > -----Original Message-----
> > > > From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com]
> > > > Sent: Friday, August 5, 2016 3:17 PM
> > > > To: Yang, Libin <libin.yang at intel.com>
> > > > Cc: libin.yang at linux.intel.com; intel-gfx at lists.freedesktop.org;
> > > > jani.nikula at linux.intel.com; Vetter, Daniel
> > > > <daniel.vetter at intel.com>; tiwai at suse.de
> > > > Subject: Re: [PATCH v3 1/3] drm/i915: set proper N/M in modeset
> > > >
> > > > > > > > > > > +		m = audio_config_get_m(intel_crtc, rate);
> > > > > > > > > > > +		if (m != 0) {
> > > > > > > > > > > +			tmp =
> > > > > > I915_READ(HSW_AUD_M_CTS_ENABLE(pipe));
> > > > > > > > > > > +			tmp =
> > audio_config_setup_m_reg(intel_crtc,
> > > > > > m, tmp);
> > > > > > > > > > > +
> > 	I915_WRITE(HSW_AUD_M_CTS_ENABLE(pipe),
> > > > > > tmp);
> > > > > > > > > >
> > > > > > > > > > We should program this register even for HDMI, so that
> > > > > > > > > > we don't leak invalid register values eg. when changing from DP-
> > >HDMI.
> > > > > > > > >
> > > > > > > > > HDMI doesn't need set these values based on our test. It
> > > > > > > > > seems silicon can handle smoothly for HDMI.
> > > > > > > >
> > > > > > > > Yes, but we nee to make sure we clear whatever we programmed
> > > > > > > > in for DP previously.
> > > > > > >
> > > > > > > The silicon seems to be able to handle the situation of DP ->
> > HDMI > > > > > and HDMI -> DP.
> > > > > >
> > > > > > Did you make sure eg. that the power well didn't get toggled in
> > between?
> > > > > > That would reset the register anyway.
> > > > >
> > > > > We have done the test for this case. So far it is OK.
> > > >
> > > > Test what? Checking that the register gets reset w/o any power well
> > > > toggling and the like? So what event does reset that register?
> > >
> > > The register is used to set m/cts, which will impacts the audio clock
> > > sync. We didn't check the register reset or not. But the HDMI audio
> > > and DP will both works smoothly. And for your consideration, it is for
> > > HDMI. Let's make another patch for this issue if we really met the
> > > issue that hdmi can't sync the clock. What do you think?
> > 
> > How about we just always write the register to make sure we won't get any
> > stupid bugs because of this.
> 
> OK. Do you think it is OK I will write a separate patch for it, not included
> in the patch series?
> 
> This is for HDMI N/CTS setting and this needs a lot of  test on HDMI platforms.

I don't mean you should write the CTS value there, that would change
the behaviour. Just make sure the manual programming bit is cleared
etc. like it was before this DP patch. Ie. probably just overwrite the
register with 0, or something.

-- 
Ville Syrjälä
Intel OTC