| Message ID | 1439880714-40931-4-git-send-email-libin.yang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Aug 18, 2015 at 02:51:54PM +0800, libin.yang@intel.com wrote: > From: Libin Yang <libin.yang@intel.com> > > When modeset occurs and the TMDS frequency is set to some > speical values, the N/CTS need to be set manually if audio > is playing. > > Signed-off-by: Libin Yang <libin.yang@intel.com> > --- > drivers/gpu/drm/i915/i915_reg.h | 8 ++++++++ > drivers/gpu/drm/i915/intel_audio.c | 40 +++++++++++++++++++++++++++++++++++++- > 2 files changed, 47 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 6786e94..122b5bd 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -7035,6 +7035,8 @@ enum skl_disp_power_wells { > _HSW_AUD_MISC_CTRL_A, \ > _HSW_AUD_MISC_CTRL_B) > > +#define HSW_AUD_PIPE_CONN_SEL_CTRL 0x650ac > + > #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) _PIPE(pipe, \ > @@ -7049,6 +7051,12 @@ enum skl_disp_power_wells { > _HSW_AUD_DIG_CNVT_2) > #define DIP_PORT_SEL_MASK 0x3 > > +#define _HSW_AUD_STR_DESC_1 0x65084 > +#define _HSW_AUD_STR_DESC_2 0x65184 > +#define AUD_STR_DESC(pipe) _PIPE(pipe, \ > + _HSW_AUD_STR_DESC_1, \ > + _HSW_AUD_STR_DESC_2) > + > #define _HSW_AUD_EDID_DATA_A 0x65050 > #define _HSW_AUD_EDID_DATA_B 0x65150 > #define HSW_AUD_EDID_DATA(pipe) _PIPE(pipe, \ > diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c > index 96b97be..0dfdc77 100644 > --- a/drivers/gpu/drm/i915/intel_audio.c > +++ b/drivers/gpu/drm/i915/intel_audio.c > @@ -140,6 +140,27 @@ static bool audio_rate_need_prog(struct intel_crtc *crtc, > return false; > } > > +static int audio_config_get_rate(struct drm_i915_private *dev_priv, > + enum pipe pipe) > +{ > + uint32_t tmp; > + int cvt_idx; > + int base_rate, mul, div, rate; > + > + tmp = I915_READ(HSW_AUD_PIPE_CONN_SEL_CTRL); > + cvt_idx = (tmp >> (pipe * 8)) & 0xff; > + tmp = I915_READ(AUD_STR_DESC(cvt_idx)); > + base_rate = tmp & (1 << 14); > + if (base_rate == 0) > + rate = 48000; > + else > + rate = 44100; > + mul = (tmp & (0x7 << 11)) + 1; > + div = (tmp & (0x7 << 8)) + 1; > + rate = rate * mul / div; > + return rate; > +} Given your new .sync_audio_rate() callback, the audio driver should have already told us what the current sample rate is, and so we shouldn't have to go dig it up from registers. > + > static bool intel_eld_uptodate(struct drm_connector *connector, > int reg_eldv, uint32_t bits_eldv, > int reg_elda, uint32_t bits_elda, > @@ -261,6 +282,8 @@ static void hsw_audio_codec_enable(struct drm_connector *connector, > const uint8_t *eld = connector->eld; > uint32_t tmp; > int len, i; > + int n_low, n_up, n; > + int rate; > > DRM_DEBUG_KMS("Enable audio codec on pipe %c, %u bytes ELD\n", > pipe_name(pipe), drm_eld_size(eld)); > @@ -296,12 +319,27 @@ static void hsw_audio_codec_enable(struct drm_connector *connector, > /* Enable timestamps */ > tmp = I915_READ(HSW_AUD_CFG(pipe)); > tmp &= ~AUD_CONFIG_N_VALUE_INDEX; > - tmp &= ~AUD_CONFIG_N_PROG_ENABLE; > tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK; > if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT)) > tmp |= AUD_CONFIG_N_VALUE_INDEX; > else > tmp |= audio_config_hdmi_pixel_clock(mode); > + > + tmp &= ~AUD_CONFIG_N_PROG_ENABLE; > + if (audio_rate_eed_prog(intel_crtc, mode)) { > + rate = audio_config_get_rate(dev_priv, pipe); > + n = audio_config_get_n(mode, rate); > + if (n != 0) { > + n_low = n & 0xfff; > + n_up = (n >> 12) & 0xff; > + tmp &= ~(AUD_CONFIG_UPPER_N_MASK | > + AUD_CONFIG_LOWER_N_MASK); > + tmp |= ((n_up << AUD_CONFIG_UPPER_N_SHIFT) | > + (n_low << AUD_CONFIG_LOWER_N_SHIFT) | > + AUD_CONFIG_N_PROG_ENABLE); > + } > + } We should share the code to set this up with .sync_audio_rate() rather than having two copies of essentically the same code. > + > I915_WRITE(HSW_AUD_CFG(pipe), tmp); > } > > -- > 1.9.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> -----Original Message----- > From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] > Sent: Friday, August 21, 2015 11:26 PM > To: Yang, Libin > Cc: alsa-devel@alsa-project.org; tiwai@suse.de; intel- > gfx@lists.freedesktop.org; daniel.vetter@ffwll.ch; > jani.nikula@linux.intel.com > Subject: Re: [Intel-gfx] [PATCH v5 4/4] drm/i915: set proper N/CTS in > modeset > > On Tue, Aug 18, 2015 at 02:51:54PM +0800, libin.yang@intel.com > wrote: > > From: Libin Yang <libin.yang@intel.com> > > > > When modeset occurs and the TMDS frequency is set to some > > speical values, the N/CTS need to be set manually if audio > > is playing. > > > > Signed-off-by: Libin Yang <libin.yang@intel.com> > > --- > > drivers/gpu/drm/i915/i915_reg.h | 8 ++++++++ > > drivers/gpu/drm/i915/intel_audio.c | 40 > +++++++++++++++++++++++++++++++++++++- > > 2 files changed, 47 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > b/drivers/gpu/drm/i915/i915_reg.h > > index 6786e94..122b5bd 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -7035,6 +7035,8 @@ enum skl_disp_power_wells { > > _HSW_AUD_MISC_CTRL_A, \ > > _HSW_AUD_MISC_CTRL_B) > > > > +#define HSW_AUD_PIPE_CONN_SEL_CTRL 0x650ac > > + > > #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) _PIPE(pipe, \ > > @@ -7049,6 +7051,12 @@ enum skl_disp_power_wells { > > _HSW_AUD_DIG_CNVT_2) > > #define DIP_PORT_SEL_MASK 0x3 > > > > +#define _HSW_AUD_STR_DESC_1 0x65084 > > +#define _HSW_AUD_STR_DESC_2 0x65184 > > +#define AUD_STR_DESC(pipe) _PIPE(pipe, \ > > + _HSW_AUD_STR_DESC_1, > \ > > + _HSW_AUD_STR_DESC_2) > > + > > #define _HSW_AUD_EDID_DATA_A 0x65050 > > #define _HSW_AUD_EDID_DATA_B 0x65150 > > #define HSW_AUD_EDID_DATA(pipe) _PIPE(pipe, \ > > diff --git a/drivers/gpu/drm/i915/intel_audio.c > b/drivers/gpu/drm/i915/intel_audio.c > > index 96b97be..0dfdc77 100644 > > --- a/drivers/gpu/drm/i915/intel_audio.c > > +++ b/drivers/gpu/drm/i915/intel_audio.c > > @@ -140,6 +140,27 @@ static bool audio_rate_need_prog(struct > intel_crtc *crtc, > > return false; > > } > > > > +static int audio_config_get_rate(struct drm_i915_private *dev_priv, > > + enum pipe pipe) > > +{ > > + uint32_t tmp; > > + int cvt_idx; > > + int base_rate, mul, div, rate; > > + > > + tmp = I915_READ(HSW_AUD_PIPE_CONN_SEL_CTRL); > > + cvt_idx = (tmp >> (pipe * 8)) & 0xff; > > + tmp = I915_READ(AUD_STR_DESC(cvt_idx)); > > + base_rate = tmp & (1 << 14); > > + if (base_rate == 0) > > + rate = 48000; > > + else > > + rate = 44100; > > + mul = (tmp & (0x7 << 11)) + 1; > > + div = (tmp & (0x7 << 8)) + 1; > > + rate = rate * mul / div; > > + return rate; > > +} > > Given your new .sync_audio_rate() callback, the audio driver should > have > already told us what the current sample rate is, and so we shouldn't > have to go dig it up from registers. This patch is not calling sync_audio_rate() from audio driver. It happens when gfx doing the modeset. > > > + > > static bool intel_eld_uptodate(struct drm_connector *connector, > > int reg_eldv, uint32_t bits_eldv, > > int reg_elda, uint32_t bits_elda, > > @@ -261,6 +282,8 @@ static void hsw_audio_codec_enable(struct > drm_connector *connector, > > const uint8_t *eld = connector->eld; > > uint32_t tmp; > > int len, i; > > + int n_low, n_up, n; > > + int rate; > > > > DRM_DEBUG_KMS("Enable audio codec on pipe %c, %u bytes > ELD\n", > > pipe_name(pipe), drm_eld_size(eld)); > > @@ -296,12 +319,27 @@ static void > hsw_audio_codec_enable(struct drm_connector *connector, > > /* Enable timestamps */ > > tmp = I915_READ(HSW_AUD_CFG(pipe)); > > tmp &= ~AUD_CONFIG_N_VALUE_INDEX; > > - tmp &= ~AUD_CONFIG_N_PROG_ENABLE; > > tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK; > > if (intel_pipe_has_type(intel_crtc, > INTEL_OUTPUT_DISPLAYPORT)) > > tmp |= AUD_CONFIG_N_VALUE_INDEX; > > else > > tmp |= audio_config_hdmi_pixel_clock(mode); > > + > > + tmp &= ~AUD_CONFIG_N_PROG_ENABLE; > > + if (audio_rate_eed_prog(intel_crtc, mode)) { > > + rate = audio_config_get_rate(dev_priv, pipe); > > + n = audio_config_get_n(mode, rate); > > + if (n != 0) { > > + n_low = n & 0xfff; > > + n_up = (n >> 12) & 0xff; > > + tmp &= ~(AUD_CONFIG_UPPER_N_MASK | > > + > AUD_CONFIG_LOWER_N_MASK); > > + tmp |= ((n_up << > AUD_CONFIG_UPPER_N_SHIFT) | > > + (n_low << > AUD_CONFIG_LOWER_N_SHIFT) | > > + > AUD_CONFIG_N_PROG_ENABLE); > > + } > > + } > > We should share the code to set this up with .sync_audio_rate() rather > than having two copies of essentically the same code. It's only setting a variable tmp, not sure we really need a separate function to do the setting? > > > + > > I915_WRITE(HSW_AUD_CFG(pipe), tmp); > > } > > > > -- > > 1.9.1 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel OTC
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 6786e94..122b5bd 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -7035,6 +7035,8 @@ enum skl_disp_power_wells { _HSW_AUD_MISC_CTRL_A, \ _HSW_AUD_MISC_CTRL_B) +#define HSW_AUD_PIPE_CONN_SEL_CTRL 0x650ac + #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) _PIPE(pipe, \ @@ -7049,6 +7051,12 @@ enum skl_disp_power_wells { _HSW_AUD_DIG_CNVT_2) #define DIP_PORT_SEL_MASK 0x3 +#define _HSW_AUD_STR_DESC_1 0x65084 +#define _HSW_AUD_STR_DESC_2 0x65184 +#define AUD_STR_DESC(pipe) _PIPE(pipe, \ + _HSW_AUD_STR_DESC_1, \ + _HSW_AUD_STR_DESC_2) + #define _HSW_AUD_EDID_DATA_A 0x65050 #define _HSW_AUD_EDID_DATA_B 0x65150 #define HSW_AUD_EDID_DATA(pipe) _PIPE(pipe, \ diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index 96b97be..0dfdc77 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -140,6 +140,27 @@ static bool audio_rate_need_prog(struct intel_crtc *crtc, return false; } +static int audio_config_get_rate(struct drm_i915_private *dev_priv, + enum pipe pipe) +{ + uint32_t tmp; + int cvt_idx; + int base_rate, mul, div, rate; + + tmp = I915_READ(HSW_AUD_PIPE_CONN_SEL_CTRL); + cvt_idx = (tmp >> (pipe * 8)) & 0xff; + tmp = I915_READ(AUD_STR_DESC(cvt_idx)); + base_rate = tmp & (1 << 14); + if (base_rate == 0) + rate = 48000; + else + rate = 44100; + mul = (tmp & (0x7 << 11)) + 1; + div = (tmp & (0x7 << 8)) + 1; + rate = rate * mul / div; + return rate; +} + static bool intel_eld_uptodate(struct drm_connector *connector, int reg_eldv, uint32_t bits_eldv, int reg_elda, uint32_t bits_elda, @@ -261,6 +282,8 @@ static void hsw_audio_codec_enable(struct drm_connector *connector, const uint8_t *eld = connector->eld; uint32_t tmp; int len, i; + int n_low, n_up, n; + int rate; DRM_DEBUG_KMS("Enable audio codec on pipe %c, %u bytes ELD\n", pipe_name(pipe), drm_eld_size(eld)); @@ -296,12 +319,27 @@ static void hsw_audio_codec_enable(struct drm_connector *connector, /* Enable timestamps */ tmp = I915_READ(HSW_AUD_CFG(pipe)); tmp &= ~AUD_CONFIG_N_VALUE_INDEX; - tmp &= ~AUD_CONFIG_N_PROG_ENABLE; tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK; if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT)) tmp |= AUD_CONFIG_N_VALUE_INDEX; else tmp |= audio_config_hdmi_pixel_clock(mode); + + tmp &= ~AUD_CONFIG_N_PROG_ENABLE; + if (audio_rate_need_prog(intel_crtc, mode)) { + rate = audio_config_get_rate(dev_priv, pipe); + n = audio_config_get_n(mode, rate); + if (n != 0) { + n_low = n & 0xfff; + n_up = (n >> 12) & 0xff; + tmp &= ~(AUD_CONFIG_UPPER_N_MASK | + AUD_CONFIG_LOWER_N_MASK); + tmp |= ((n_up << AUD_CONFIG_UPPER_N_SHIFT) | + (n_low << AUD_CONFIG_LOWER_N_SHIFT) | + AUD_CONFIG_N_PROG_ENABLE); + } + } + I915_WRITE(HSW_AUD_CFG(pipe), tmp); }