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

Thu Aug 4 06:06:10 UTC 2016

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. 
> 
> > 
> > > 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.

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

-- 
Ville Syrjälä
Intel OTC
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-drm-i915-Remove-useless-rate_to_index-usage.patch
Type: text/x-diff
Size: 1301 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/intel-gfx/attachments/20160804/f44925a0/attachment-0003.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-drm-i915-Allow-rate_to_index-to-return-non-exact-mat.patch
Type: text/x-diff
Size: 1415 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/intel-gfx/attachments/20160804/f44925a0/attachment-0004.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-drm-i915-Add-max_pipe_bpp-max_dot_clock-max_port_clo.patch
Type: text/x-diff
Size: 10807 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/intel-gfx/attachments/20160804/f44925a0/attachment-0005.patch>