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

Thu Aug 4 06:12:42 UTC 2016

Hi Ville,

> -----Original Message-----
> From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com]
> Sent: Thursday, August 4, 2016 2:06 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] drm/i915: set proper N/M in modeset
> 
> On Thu, Aug 04, 2016 at 05:46:01AM +0000, Yang, Libin wrote:
> > Hi Ville,
> >
> > > -----Original Message-----
> > > From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com]
> > > Sent: Wednesday, August 3, 2016 12:59 AM
> > > 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] drm/i915: set proper N/M in modeset
> > >
> > > On Tue, Aug 02, 2016 at 01:58:51PM +0000, Yang, Libin wrote:
> > > > Hi Ville
> > > >
> > > > > -----Original Message-----
> > > > > From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com]
> > > > > Sent: Tuesday, August 2, 2016 6:47 PM
> > > > > To: libin.yang at linux.intel.com
> > > > > Cc: intel-gfx at lists.freedesktop.org;
> > > > > jani.nikula at linux.intel.com; Vetter, Daniel
> > > > > <daniel.vetter at intel.com>; tiwai at suse.de; Yang, Libin
> > > > > <libin.yang at intel.com>
> > > > > Subject: Re: [PATCH] drm/i915: set proper N/M in modeset
> > > > >
> > > > > On Tue, Aug 02, 2016 at 09:35:10AM +0800,
> > > > > libin.yang at linux.intel.com
> > > wrote:
> > > > > > From: Libin Yang <libin.yang at linux.intel.com>
> > > > > >
> > > > > > When modeset occurs and the LS_CLK is set to some special
> > > > > > values in DP mode, the N/M need to be set manually if audio is
> playing.
> > > > > >
> > > > > > The relationship of Maud and Naud is expressed in the
> > > > > > following
> > > > > > equation:
> > > > > > Maud/Naud = 512 * fs / f_LS_Clk
> > > > > >
> > > > > > Please refer VESA DisplayPort Standard spec for details.
> > > > > >
> > > > > > Also, the patch applies
> > > > > > commit 7e8275c2f2bb ("drm/i915: set proper N/CTS in modeset")
> > > > > > to APL platform.
> > > > > >
> > > > > > Signed-off-by: Libin Yang <libin.yang at linux.intel.com>
> > > > > > ---
> > > > > >  drivers/gpu/drm/i915/i915_reg.h    |   6 ++
> > > > > >  drivers/gpu/drm/i915/intel_audio.c | 122
> > > > > > +++++++++++++++++++++++++++++++------
> > > > > >  2 files changed, 111 insertions(+), 17 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > > > > b/drivers/gpu/drm/i915/i915_reg.h index 8bfde75..2f9d00e
> > > > > > 100644
> > > > > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > > > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > > > > @@ -7351,6 +7351,12 @@ enum {
> > > > > >  #define _HSW_AUD_CONFIG_B		0x65100
> > > > > >  #define HSW_AUD_CFG(pipe)		_MMIO_PIPE(pipe,
> > > > > _HSW_AUD_CONFIG_A, _HSW_AUD_CONFIG_B)
> > > > > >
> > > > > > +#define _HSW_AUD_M_CTS_ENABLE_A		0x65028
> > > > > > +#define _HSW_AUD_M_CTS_ENABLE_B		0x65128
> > > > > > +#define HSW_AUD_M_CTS_ENABLE(pipe)
> > > 	_MMIO_PIPE(pipe,
> > > > > _HSW_AUD_M_CTS_ENABLE_A, _HSW_AUD_M_CTS_ENABLE_B)
> > > > > > +#define   AUD_M_CTS_M_VALUE_INDEX	(1 << 21)
> > > > > > +#define   AUD_M_CTS_M_PROG_ENABLE	(1 << 20)
> > > > > > +
> > > > > >  #define _HSW_AUD_MISC_CTRL_A		0x65010
> > > > > >  #define _HSW_AUD_MISC_CTRL_B		0x65110
> > > > > >  #define HSW_AUD_MISC_CTRL(pipe)		_MMIO_PIPE(pipe,
> > > > > _HSW_AUD_MISC_CTRL_A, _HSW_AUD_MISC_CTRL_B)
> > > > > > diff --git a/drivers/gpu/drm/i915/intel_audio.c
> > > > > > b/drivers/gpu/drm/i915/intel_audio.c
> > > > > > index 6700a7b..de55ecf 100644
> > > > > > --- a/drivers/gpu/drm/i915/intel_audio.c
> > > > > > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > > > > > @@ -98,6 +98,22 @@ static const struct {
> > > > > >  	{ 192000, TMDS_297M, 20480, 247500 },  };
> > > > > >
> > > > > > +#define LC_540M 540000
> > > > > > +#define LC_162M 162000
> > > > >
> > > > > Do we have some explanation why 2.7 doesn't need M/N
> > > > > programming, but
> > > > > 1.62 and 5.4 do?
> > > >
> > > > I didn't use 2.7 because I can't find a mode using 2.7.
> > >
> > > Hmm. Maybe we should add some knobs to force a specific bpc/link
> > > rate/number of lanes to help with this kind of testing. Currently
> > > you just get what you get, which isn't so nice when you want to test all
> variations.
> > > ...
> > > OK, so I just went ahead and did that. Here's a branch:
> > >
> > > git://github.com/vsyrjala/linux.git modparam_clock_bpp_limit
> > >
> > > For your DP testing just setting
> > > i915.max_port_clock=162000 or i915.max_port_clock=270000 and then
> > > forcing a modeset should do the trick.
> >
> > Thanks for the new branch. It seems download is very slow, less than
> > 10Kib/s.
> 
> github that slow for you? Weird.
> 
> And I can't see a way to grab the raw patches from the github web
> interface :( so I've attached the patches to this mail in case you can't finish the
> git fetch in reasonable time.
> 
> > So I will submit the new patches firstly and then do the test.
> > Fortunately, I found there is recommended data for 340MHz in the spec.
> 
> 340 MHz? There's no such link rate for DP. I'm not sure what you're saying
> here...
> 
> > I copied the data to the patch and suppose the data should be accurate.

Thanks for the patch. Sorry, I mean 270MHz, not 340MHz.

> >
> > >
> > > > So I can't do the test.
> > > > 5.4 is for 4K and 1.62 is for 1080p.
> > > >
> > > > >
> > > > > And I see you're only doing this on HSW+. Earlier platforms don't need
> this?
> > > >
> > > > We are not supporting earlier platforms and I'm not sure whether
> > > > the old platforms supports 4K DP or not.
> > >
> > > SNB-IVB dotclock can go up to 360Mhz, ILK up to 405 Mhz. At least in
> theory.
> > > The DP link is limited to 4 x 2.7 for all. From the those the
> > > dotclock limit is the one you should hit first since DP can always
> > > fall back to 6bpc and that should be correspond to a dotclock of 480 MHz.
> > > Anyways, 360MHz is plenty for 4k at 30.
> > >
> > >
> > > The question really is why we need to do this in the first place.
> > > There's nothing in the spec telling is it's really required. All I
> > > can find in the DP spec is "Maud value is set to 2^15 (=32,768) when
> > > the audio clock is asynchronous to the LS_Clk.", and then
> >
> > We made the patch because we found the HW can't calculate the value
> > this will cause there is several seconds silence at the beginning of audio
> playback.
> > With this patch, the silence is much shorter than before and is acceptable.
> 
> I see. Must be sink specific since at least my current monitor (ASUS
> PB278) has no apparent delays with the current code. I guess some sinks are
> just slower in clock recovery than others, and starting out with better M/N
> values can speed it up a bit.

Yes, behaviors are different between different monitors.

> 
> Anyways, please specify this reason for the change in the commit message,
> otherwise no one can figure out why it's needed.

Get it. I will add it.

Regards,
Libin

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