Hi Ville, > -----Original Message----- > From: Ville Syrjälä [mailto:[email protected]] > Sent: Friday, July 29, 2016 5:47 PM > To: Yang, Libin <[email protected]> > Cc: [email protected]; [email protected]; > [email protected]; Vetter, Daniel <[email protected]>; > [email protected] > Subject: Re: [PATCH] drm/i915: set proper N/M in modeset > > On Fri, Jul 29, 2016 at 05:54:23AM +0000, Yang, Libin wrote: > > Hi Ville, > > > > > -----Original Message----- > > > From: Ville Syrjälä [mailto:[email protected]] > > > Sent: Thursday, July 28, 2016 3:42 PM > > > To: [email protected] > > > Cc: [email protected]; [email protected]; > > > Vetter, Daniel <[email protected]>; [email protected]; Yang, Libin > > > <[email protected]> > > > Subject: Re: [PATCH] drm/i915: set proper N/M in modeset > > > > > > On Thu, Jul 14, 2016 at 03:06:21PM +0800, [email protected] > wrote: > > > > From: Libin Yang <[email protected]> > > > > > > > > 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. > > > > > > > > Also, the patch applies > > > > commit 7e8275c2f2bb ("drm/i915: set proper N/CTS in modeset") to > > > > APL platform. > > > > > > > > Signed-off-by: Libin Yang <[email protected]> > > > > --- > > > > drivers/gpu/drm/i915/i915_reg.h | 6 ++ > > > > drivers/gpu/drm/i915/intel_audio.c | 116 > > > > ++++++++++++++++++++++++++++++++----- > > > > 2 files changed, 108 insertions(+), 14 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..e2e4d4b 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_533M 533250 > > > > +#define LC_148M 148500 > > > > +static const struct { > > > > + int sample_rate; > > > > + int clock; > > > > + int n; > > > > + int m; > > > > +} aud_nm[] = { > > > > + {48000, LC_533M, 88875, 4096}, > > > > + {44100, LC_533M, 148125, 6272}, > > > > + {32000, LC_533M, 266625, 8192}, > > > > + {48000, LC_148M, 12375, 2048}, > > > > + {44100, LC_148M, 20625, 3136}, > > > > + {32000, LC_148M, 37125, 4096}, > > > > +}; > > > > + > > > > /* get AUD_CONFIG_PIXEL_CLOCK_HDMI_* value for mode */ static > > > > u32 audio_config_hdmi_pixel_clock(const struct drm_display_mode > > > > *adjusted_mode) { @@ -121,20 +137,50 @@ static u32 > > > > audio_config_hdmi_pixel_clock(const struct drm_display_mode > *adjusted > > > > return hdmi_audio_clock[i].config; } > > > > > > > > -static int audio_config_get_n(const struct drm_display_mode > > > > *mode, int rate) > > > > +static int audio_config_get_n(struct intel_crtc *crtc, > > > > + const struct drm_display_mode *mode, int > > > > rate) { > > > > + int i; > > > > + > > > > + if (intel_crtc_has_type(crtc->config, INTEL_OUTPUT_HDMI)) { > > > > + for (i = 0; i < ARRAY_SIZE(aud_ncts); i++) { > > > > + if ((rate == aud_ncts[i].sample_rate) && > > > > + (mode->clock == aud_ncts[i].clock)) { > > > > + return aud_ncts[i].n; > > > > + } > > > > + } > > > > + } > > > > + > > > > + if (intel_crtc_has_type(crtc->config, INTEL_OUTPUT_DP)) { > > > > + for (i = 0; i < ARRAY_SIZE(aud_nm); i++) { > > > > + if ((rate == aud_nm[i].sample_rate) && > > > > + (mode->clock == aud_nm[i].clock)) { > > > > > > So the spec says we should have "Maud / Naud = 512 * fs / f_LS_Clk", > > > where fs is the audio sample rate, and f_LS_CLK is the link symbol clock. > > > With that in mind this should not be looking at mode->clock but > > > crtc->config->port_clock. > > > > > > So I don't actually understand why you say LS_CLK has "special" values. > > > It shouldn't. It's always either 162, 270, or 540 MHz. > > > > Thanks for the correction. I will use crtc->config->port_clock. > > But why do you need to do it at all? The hardware can't deal with one of the > standard link rates on its own?
Yes, we found that the HW can't set the N/M automatically. This will cause there is several seconds of silence at the beginning of audio playback. > > > > > > > > > Actually even the HDMI case is wrong in the code, it should be > > > looking at > > > mode->crtc_clock instead of mode->clock. Or perhaps even port_clock, > > > mode->if my > > > reading of the HDMI spec is correct, but I never got any sane answer > > > from any hw folks to my questions about this :( > > > > I will try mode->crtc_clock later for HDMI. Thanks. > > While you're at it, can you pls do s/mode/adjusted_mode/ ? A while back I > unified our naming convention for these things, but looks like the audio code > has since gone back to the inconsisten old way. Yes, I will do it, replace mode with adjusted_mode. Regards, Libin > > > > > Regards, > > Libin > > > > > _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
