> -----Original Message----- > From: Intel-gfx [mailto:intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf Of > Lin, Mengdong > Sent: Wednesday, October 12, 2016 10:46 AM > To: Nikula, Jani <jani.nikula@xxxxxxxxx>; intel-gfx@xxxxxxxxxxxxxxxxxxxxx > Cc: Nikula, Jani <jani.nikula@xxxxxxxxx>; libin.yang@xxxxxxxxxxxxxxx; > Pandiyan, Dhinakaran <dhinakaran.pandiyan@xxxxxxxxx> > Subject: Re: [PATCH RESEND 9/9] drm/i915: set proper N/M in > modeset > > > > > -----Original Message----- > > From: Intel-gfx [mailto:intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx] On > > Behalf Of Jani Nikula > > Sent: Monday, October 10, 2016 11:04 PM > > To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > Cc: Nikula, Jani <jani.nikula@xxxxxxxxx>; libin.yang@xxxxxxxxxxxxxxx; > > Pandiyan, Dhinakaran <dhinakaran.pandiyan@xxxxxxxxx> > > Subject: [PATCH RESEND 9/9] drm/i915: set proper N/M in > > modeset > > > > 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. Otherwise > > the first several seconds may be silent in audio playback. > > > > 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. > > > > Signed-off-by: Libin Yang <libin.yang@xxxxxxxxxxxxxxx> > > Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx> > > --- > > drivers/gpu/drm/i915/i915_reg.h | 7 +++ > > drivers/gpu/drm/i915/intel_audio.c | 100 > > ++++++++++++++++++++++++++++++++++++- > > 2 files changed, 105 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > b/drivers/gpu/drm/i915/i915_reg.h index 595d196f753f..8d9dbc7d5b32 > > 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -7359,6 +7359,13 @@ enum { > > #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) > > > > +#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 AUD_CONFIG_M_MASK 0xfffff > > The last line cause misalignment after applying the patch. > > > + > > #define _HSW_AUD_DIP_ELD_CTRL_ST_A 0x650b4 > > #define _HSW_AUD_DIP_ELD_CTRL_ST_B 0x651b4 > > #define HSW_AUD_DIP_ELD_CTRL(pipe) _MMIO_PIPE(pipe, > > _HSW_AUD_DIP_ELD_CTRL_ST_A, _HSW_AUD_DIP_ELD_CTRL_ST_B) diff -- > git > > a/drivers/gpu/drm/i915/intel_audio.c > > b/drivers/gpu/drm/i915/intel_audio.c > > index 81df29ca4947..0bc2701b6c9c 100644 > > --- a/drivers/gpu/drm/i915/intel_audio.c > > +++ b/drivers/gpu/drm/i915/intel_audio.c > > @@ -57,6 +57,70 @@ > > * struct &i915_audio_component_audio_ops @audio_ops is called from > > i915 driver. > > */ > > > > +/* DP N/M table */ > > +#define LC_540M 540000 > > +#define LC_270M 270000 > > +#define LC_162M 162000 > > + > > +struct dp_aud_n_m { > > + int sample_rate; > > + int clock; > > + u16 n; > > + u16 m; > > +}; > > + > > +static const struct dp_aud_n_m dp_aud_n_m[] = { > > + { 192000, LC_540M, 5625, 1024 }, > > + { 176400, LC_540M, 9375, 1568 }, > > + { 96000, LC_540M, 5625, 512 }, > > + { 88200, LC_540M, 9375, 784 }, > > + { 48000, LC_540M, 5625, 256 }, > > + { 44100, LC_540M, 9375, 392 }, > > + { 32000, LC_540M, 16875, 512 }, > > + { 192000, LC_270M, 5625, 2048 }, > > + { 176400, LC_270M, 9375, 3136 }, > > + { 96000, LC_270M, 5625, 1024 }, > > + { 88200, LC_270M, 9375, 1568 }, > > + { 48000, LC_270M, 5625, 512 }, > > + { 44100, LC_270M, 9375, 784 }, > > + { 32000, LC_270M, 16875, 1024 }, > > + { 192000, LC_162M, 3375, 2048 }, > > + { 176400, LC_162M, 5625, 3136 }, > > + { 96000, LC_162M, 3375, 1024 }, > > + { 88200, LC_162M, 5625, 1568 }, > > + { 48000, LC_162M, 3375, 512 }, > > + { 44100, LC_162M, 5625, 784 }, > > + { 32000, LC_162M, 10125, 1024 }, > > +}; > > + > > +static const struct dp_aud_n_m * > > +audio_config_dp_get_n_m(struct intel_crtc *intel_crtc, int rate) { > > + int i; > > + > > + for (i = 0; i < ARRAY_SIZE(dp_aud_n_m); i++) { > > + if (rate == dp_aud_n_m[i].sample_rate && > > + intel_crtc->config->port_clock == dp_aud_n_m[i].clock) > > + return &dp_aud_n_m[i]; > > + } > > + > > + return NULL; > > +} > > + > > +static int audio_config_dp_get_m(struct intel_crtc *intel_crtc, int > > +rate) { > > + const struct dp_aud_n_m *nm = > > audio_config_dp_get_n_m(intel_crtc, > > +rate); > > + > > + return nm ? nm->m : 0; > > +} > > + > > +static int audio_config_dp_get_n(struct intel_crtc *intel_crtc, int > > +rate) { > > + const struct dp_aud_n_m *nm = > > audio_config_dp_get_n_m(intel_crtc, > > +rate); > > + > > + return nm ? nm->n : 0; > > +} > > + > > static const struct { > > int clock; > > u32 config; > > @@ -225,8 +289,10 @@ hsw_dp_audio_config_update(struct intel_crtc > > *intel_crtc, enum port port, > > const struct drm_display_mode *adjusted_mode) { > > struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev); > > + struct i915_audio_component *acomp = dev_priv- > > >audio_component; > > + int rate = acomp ? acomp->aud_sample_rate[port] : 0; > > enum pipe pipe = intel_crtc->pipe; > > - u32 tmp; > > + u32 tmp, n, m; > > > > tmp = I915_READ(HSW_AUD_CFG(pipe)); > > tmp &= ~AUD_CONFIG_N_VALUE_INDEX; > > @@ -234,7 +300,30 @@ hsw_dp_audio_config_update(struct intel_crtc > > *intel_crtc, enum port port, > > tmp &= ~AUD_CONFIG_N_PROG_ENABLE; > > tmp |= AUD_CONFIG_N_VALUE_INDEX; > > > > + if (intel_crtc->config->port_clock == LC_540M || > > + intel_crtc->config->port_clock == LC_270M || > > + intel_crtc->config->port_clock == LC_162M) { > > + n = audio_config_dp_get_n(intel_crtc, rate); > > + if (n != 0) { > > + tmp &= ~AUD_CONFIG_N_MASK; > > + tmp |= AUD_CONFIG_N(n); > > + tmp |= AUD_CONFIG_N_PROG_ENABLE; > > + } else { > > + DRM_DEBUG_KMS("no suitable N value is found\n"); > > Could some comments be added here why we don't return if no valid N value > is found? This is because although we don't need set N value here, but there are some other bits to be set. So you can see we call I915_WRITE(HSW_AUD_CFG(pipe), tmp); Later. > Do we still need to program write the register to let HW calculate N by itself? Ideally, all the frequency should be in the table. But if the frequency is not in our table, we will let HW calculate N by itself. > > > + } > > + } > > + > > I915_WRITE(HSW_AUD_CFG(pipe), tmp); > > And if no valid N value is found, need we go ahead to check M value? If there is no valid N value, audio_config_dp_get_m(intel_crtc, rate); will also return 0 and skip the m setting. Regards, Libin > > > + > > + m = audio_config_dp_get_m(intel_crtc, rate); > > + if (m) { > > + tmp = I915_READ(HSW_AUD_M_CTS_ENABLE(pipe)); > > + tmp &= ~AUD_CONFIG_M_MASK; > > + tmp |= m; > > + tmp |= AUD_M_CTS_M_VALUE_INDEX; > > + tmp |= AUD_M_CTS_M_PROG_ENABLE; > > + I915_WRITE(HSW_AUD_M_CTS_ENABLE(pipe), tmp); > > + } > > } > > Thanks > Mengdong > > > static void > > @@ -267,6 +356,12 @@ hsw_hdmi_audio_config_update(struct intel_crtc > > *intel_crtc, enum port port, > > } > > > > I915_WRITE(HSW_AUD_CFG(pipe), tmp); > > + > > + tmp = I915_READ(HSW_AUD_M_CTS_ENABLE(pipe)); > > + tmp &= ~AUD_CONFIG_M_MASK; > > + tmp &= ~AUD_M_CTS_M_VALUE_INDEX; > > + tmp |= AUD_M_CTS_M_PROG_ENABLE; > > + I915_WRITE(HSW_AUD_M_CTS_ENABLE(pipe), tmp); > > } > > > > static void > > @@ -687,7 +782,8 @@ static int > > i915_audio_component_sync_audio_rate(struct device *kdev, int port, > > /* 1. get the pipe */ > > intel_encoder = get_saved_enc(dev_priv, port, pipe); > > if (!intel_encoder || !intel_encoder->base.crtc || > > - intel_encoder->type != INTEL_OUTPUT_HDMI) { > > + (intel_encoder->type != INTEL_OUTPUT_HDMI && > > + intel_encoder->type != INTEL_OUTPUT_DP)) { > > DRM_DEBUG_KMS("Not valid for port %c\n", > port_name(port)); > > err = -ENODEV; > > goto unlock; > > -- > > 2.1.4 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx