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

[Yang, Libin]

> -----Original Message-----
> From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com]
> Sent: Friday, August 5, 2016 6:54 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 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.

Get it. I was thinking you are saying setting the CTS.

Regards,
Libin

> 
> --
> Ville Syrjälä
> Intel OTC