| Message ID | 1443144972-81982-1-git-send-email-libin.yang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 25 Sep 2015, 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> Reviewed-by: Jani Nikula <jani.nikula@intel.com> > --- > drivers/gpu/drm/i915/intel_audio.c | 57 ++++++++++++++++++++++++++++++++------ > include/drm/i915_component.h | 10 +++++++ > 2 files changed, 58 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c > index e3c32ce..642dfbd 100644 > --- a/drivers/gpu/drm/i915/intel_audio.c > +++ b/drivers/gpu/drm/i915/intel_audio.c > @@ -128,6 +128,20 @@ static int audio_config_get_n(const struct drm_display_mode *mode, int rate) > return 0; > } > > +static uint32_t audio_config_setup_n_reg(int n, uint32_t val) > +{ > + int n_low, n_up; > + uint32_t tmp = val; > + > + 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); > + return tmp; > +} > + > /* check whether N/CTS/M need be set manually */ > static bool audio_rate_need_prog(struct intel_crtc *crtc, > struct drm_display_mode *mode) > @@ -262,9 +276,14 @@ static void hsw_audio_codec_enable(struct drm_connector *connector, > struct drm_i915_private *dev_priv = connector->dev->dev_private; > struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc); > enum pipe pipe = intel_crtc->pipe; > + struct i915_audio_component *acomp = dev_priv->audio_component; > const uint8_t *eld = connector->eld; > + struct intel_digital_port *intel_dig_port = > + enc_to_dig_port(&encoder->base); > + enum port port = intel_dig_port->port; > uint32_t tmp; > int len, i; > + int n, rate; > > DRM_DEBUG_KMS("Enable audio codec on pipe %c, %u bytes ELD\n", > pipe_name(pipe), drm_eld_size(eld)); > @@ -302,12 +321,29 @@ 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)) { > + if (!acomp) > + rate = 0; > + else if (port >= PORT_A && port <= PORT_E) > + rate = acomp->aud_sample_rate[port]; > + else { > + DRM_ERROR("invalid port: %d\n", port); > + rate = 0; > + } > + n = audio_config_get_n(mode, rate); > + if (n != 0) > + tmp = audio_config_setup_n_reg(n, tmp); > + else > + DRM_DEBUG_KMS("no suitable N value is found\n"); > + } > + > I915_WRITE(HSW_AUD_CFG(pipe), tmp); > > mutex_unlock(&dev_priv->av_mutex); > @@ -594,9 +630,10 @@ static int i915_audio_component_sync_audio_rate(struct device *dev, > struct intel_digital_port *intel_dig_port; > struct intel_crtc *crtc; > struct drm_display_mode *mode; > + struct i915_audio_component *acomp = dev_priv->audio_component; > enum pipe pipe = -1; > u32 tmp; > - int n_low, n_up, n; > + int n; > > /* HSW, BDW SKL need this fix */ > if (!IS_SKYLAKE(dev_priv) && > @@ -630,6 +667,9 @@ static int i915_audio_component_sync_audio_rate(struct device *dev, > pipe_name(pipe), port_name(port)); > mode = &crtc->config->base.adjusted_mode; > > + /* port must be valid now, otherwise the pipe will be invalid */ > + acomp->aud_sample_rate[port] = rate; > + > /* 2. check whether to set the N/CTS/M manually or not */ > if (!audio_rate_need_prog(crtc, mode)) { > tmp = I915_READ(HSW_AUD_CFG(pipe)); > @@ -649,15 +689,10 @@ static int i915_audio_component_sync_audio_rate(struct device *dev, > mutex_unlock(&dev_priv->av_mutex); > return 0; > } > - n_low = n & 0xfff; > - n_up = (n >> 12) & 0xff; > > - /* 4. set the N/CTS/M */ > + /* 3. set the N/CTS/M */ > tmp = I915_READ(HSW_AUD_CFG(pipe)); > - 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); > + tmp = audio_config_setup_n_reg(n, tmp); > I915_WRITE(HSW_AUD_CFG(pipe), tmp); > > mutex_unlock(&dev_priv->av_mutex); > @@ -678,6 +713,7 @@ static int i915_audio_component_bind(struct device *i915_dev, > { > struct i915_audio_component *acomp = data; > struct drm_i915_private *dev_priv = dev_to_i915(i915_dev); > + int i; > > if (WARN_ON(acomp->ops || acomp->dev)) > return -EEXIST; > @@ -685,6 +721,9 @@ static int i915_audio_component_bind(struct device *i915_dev, > drm_modeset_lock_all(dev_priv->dev); > acomp->ops = &i915_audio_component_ops; > acomp->dev = i915_dev; > + BUILD_BUG_ON(MAX_PORTS != I915_MAX_PORTS); > + for (i = 0; i < ARRAY_SIZE(acomp->aud_sample_rate); i++) > + acomp->aud_sample_rate[i] = 0; > dev_priv->audio_component = acomp; > drm_modeset_unlock_all(dev_priv->dev); > > diff --git a/include/drm/i915_component.h b/include/drm/i915_component.h > index e6d35d7..89dc7d6 100644 > --- a/include/drm/i915_component.h > +++ b/include/drm/i915_component.h > @@ -24,8 +24,18 @@ > #ifndef _I915_COMPONENT_H_ > #define _I915_COMPONENT_H_ > > +/* MAX_PORT is the number of port > + * It must be sync with I915_MAX_PORTS defined i915_drv.h > + * 5 should be enough as only HSW, BDW, SKL need such fix. > + */ > +#define MAX_PORTS 5 > + > struct i915_audio_component { > struct device *dev; > + /** > + * @aud_sample_rate: the array of audio sample rate per port > + */ > + int aud_sample_rate[MAX_PORTS]; > > const struct i915_audio_component_ops { > struct module *owner; > -- > 1.9.1 >
On Fri, 25 Sep 2015 10:01:48 +0200, Jani Nikula wrote: > > On Fri, 25 Sep 2015, 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> > > Reviewed-by: Jani Nikula <jani.nikula@intel.com> Now all 4 patches are applied to topic/drm-sync-audio-rate. git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git topic/drm-sync-audio-rate It was merged back to sound for-next branch, too. Feel free to merge this branch into drm tree. thanks, Takashi > > > > --- > > drivers/gpu/drm/i915/intel_audio.c | 57 ++++++++++++++++++++++++++++++++------ > > include/drm/i915_component.h | 10 +++++++ > > 2 files changed, 58 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c > > index e3c32ce..642dfbd 100644 > > --- a/drivers/gpu/drm/i915/intel_audio.c > > +++ b/drivers/gpu/drm/i915/intel_audio.c > > @@ -128,6 +128,20 @@ static int audio_config_get_n(const struct drm_display_mode *mode, int rate) > > return 0; > > } > > > > +static uint32_t audio_config_setup_n_reg(int n, uint32_t val) > > +{ > > + int n_low, n_up; > > + uint32_t tmp = val; > > + > > + 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); > > + return tmp; > > +} > > + > > /* check whether N/CTS/M need be set manually */ > > static bool audio_rate_need_prog(struct intel_crtc *crtc, > > struct drm_display_mode *mode) > > @@ -262,9 +276,14 @@ static void hsw_audio_codec_enable(struct drm_connector *connector, > > struct drm_i915_private *dev_priv = connector->dev->dev_private; > > struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc); > > enum pipe pipe = intel_crtc->pipe; > > + struct i915_audio_component *acomp = dev_priv->audio_component; > > const uint8_t *eld = connector->eld; > > + struct intel_digital_port *intel_dig_port = > > + enc_to_dig_port(&encoder->base); > > + enum port port = intel_dig_port->port; > > uint32_t tmp; > > int len, i; > > + int n, rate; > > > > DRM_DEBUG_KMS("Enable audio codec on pipe %c, %u bytes ELD\n", > > pipe_name(pipe), drm_eld_size(eld)); > > @@ -302,12 +321,29 @@ 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)) { > > + if (!acomp) > > + rate = 0; > > + else if (port >= PORT_A && port <= PORT_E) > > + rate = acomp->aud_sample_rate[port]; > > + else { > > + DRM_ERROR("invalid port: %d\n", port); > > + rate = 0; > > + } > > + n = audio_config_get_n(mode, rate); > > + if (n != 0) > > + tmp = audio_config_setup_n_reg(n, tmp); > > + else > > + DRM_DEBUG_KMS("no suitable N value is found\n"); > > + } > > + > > I915_WRITE(HSW_AUD_CFG(pipe), tmp); > > > > mutex_unlock(&dev_priv->av_mutex); > > @@ -594,9 +630,10 @@ static int i915_audio_component_sync_audio_rate(struct device *dev, > > struct intel_digital_port *intel_dig_port; > > struct intel_crtc *crtc; > > struct drm_display_mode *mode; > > + struct i915_audio_component *acomp = dev_priv->audio_component; > > enum pipe pipe = -1; > > u32 tmp; > > - int n_low, n_up, n; > > + int n; > > > > /* HSW, BDW SKL need this fix */ > > if (!IS_SKYLAKE(dev_priv) && > > @@ -630,6 +667,9 @@ static int i915_audio_component_sync_audio_rate(struct device *dev, > > pipe_name(pipe), port_name(port)); > > mode = &crtc->config->base.adjusted_mode; > > > > + /* port must be valid now, otherwise the pipe will be invalid */ > > + acomp->aud_sample_rate[port] = rate; > > + > > /* 2. check whether to set the N/CTS/M manually or not */ > > if (!audio_rate_need_prog(crtc, mode)) { > > tmp = I915_READ(HSW_AUD_CFG(pipe)); > > @@ -649,15 +689,10 @@ static int i915_audio_component_sync_audio_rate(struct device *dev, > > mutex_unlock(&dev_priv->av_mutex); > > return 0; > > } > > - n_low = n & 0xfff; > > - n_up = (n >> 12) & 0xff; > > > > - /* 4. set the N/CTS/M */ > > + /* 3. set the N/CTS/M */ > > tmp = I915_READ(HSW_AUD_CFG(pipe)); > > - 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); > > + tmp = audio_config_setup_n_reg(n, tmp); > > I915_WRITE(HSW_AUD_CFG(pipe), tmp); > > > > mutex_unlock(&dev_priv->av_mutex); > > @@ -678,6 +713,7 @@ static int i915_audio_component_bind(struct device *i915_dev, > > { > > struct i915_audio_component *acomp = data; > > struct drm_i915_private *dev_priv = dev_to_i915(i915_dev); > > + int i; > > > > if (WARN_ON(acomp->ops || acomp->dev)) > > return -EEXIST; > > @@ -685,6 +721,9 @@ static int i915_audio_component_bind(struct device *i915_dev, > > drm_modeset_lock_all(dev_priv->dev); > > acomp->ops = &i915_audio_component_ops; > > acomp->dev = i915_dev; > > + BUILD_BUG_ON(MAX_PORTS != I915_MAX_PORTS); > > + for (i = 0; i < ARRAY_SIZE(acomp->aud_sample_rate); i++) > > + acomp->aud_sample_rate[i] = 0; > > dev_priv->audio_component = acomp; > > drm_modeset_unlock_all(dev_priv->dev); > > > > diff --git a/include/drm/i915_component.h b/include/drm/i915_component.h > > index e6d35d7..89dc7d6 100644 > > --- a/include/drm/i915_component.h > > +++ b/include/drm/i915_component.h > > @@ -24,8 +24,18 @@ > > #ifndef _I915_COMPONENT_H_ > > #define _I915_COMPONENT_H_ > > > > +/* MAX_PORT is the number of port > > + * It must be sync with I915_MAX_PORTS defined i915_drv.h > > + * 5 should be enough as only HSW, BDW, SKL need such fix. > > + */ > > +#define MAX_PORTS 5 > > + > > struct i915_audio_component { > > struct device *dev; > > + /** > > + * @aud_sample_rate: the array of audio sample rate per port > > + */ > > + int aud_sample_rate[MAX_PORTS]; > > > > const struct i915_audio_component_ops { > > struct module *owner; > > -- > > 1.9.1 > > > > -- > Jani Nikula, Intel Open Source Technology Center >
On Fri, Sep 25, 2015 at 10:11 AM, Takashi Iwai <tiwai@suse.de> wrote: > On Fri, 25 Sep 2015 10:01:48 +0200, > Jani Nikula wrote: >> >> On Fri, 25 Sep 2015, 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> >> >> Reviewed-by: Jani Nikula <jani.nikula@intel.com> > > Now all 4 patches are applied to topic/drm-sync-audio-rate. > git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git topic/drm-sync-audio-rate > > It was merged back to sound for-next branch, too. > > Feel free to merge this branch into drm tree. I've merged your branch with now 5 patches into drm-intel-next-queued. There's a bit of conflicts with ongoing i915 work, so for pull request ordering it's probably better when sound goes in before drm (or otherwise Linus will end up with a mess). Adding Dave about that one as fyi. -Daniel
On 2015-09-25 03:36, libin.yang@intel.com wrote: > @@ -24,8 +24,18 @@ > #ifndef _I915_COMPONENT_H_ > #define _I915_COMPONENT_H_ > > +/* MAX_PORT is the number of port > + * It must be sync with I915_MAX_PORTS defined i915_drv.h > + * 5 should be enough as only HSW, BDW, SKL need such fix. > + */ > +#define MAX_PORTS 5 > + > struct i915_audio_component { > struct device *dev; > + /** > + * @aud_sample_rate: the array of audio sample rate per port > + */ > + int aud_sample_rate[MAX_PORTS]; Just a quick question as I was trying to understand the code: aud_sample_rate seems only set and get on the i915 side, never on the hda side. In short, why is this variable here, and not in e g, as a single integer in the intel_digital_port struct? That way we also avoid the possibility of MAX_PORTS becoming out of sync with the i915 side.
+ gfx driver team. Hi David, > -----Original Message----- > From: David Henningsson [mailto:david.henningsson@canonical.com] > Sent: Tuesday, October 13, 2015 9:43 PM > To: alsa-devel@alsa-project.org; Yang, Libin > Subject: Re: [alsa-devel] [PATCH v2] drm/i915: set proper N/CTS in > modeset > > > > On 2015-09-25 03:36, libin.yang@intel.com wrote: > > @@ -24,8 +24,18 @@ > > #ifndef _I915_COMPONENT_H_ > > #define _I915_COMPONENT_H_ > > > > +/* MAX_PORT is the number of port > > + * It must be sync with I915_MAX_PORTS defined i915_drv.h > > + * 5 should be enough as only HSW, BDW, SKL need such fix. > > + */ > > +#define MAX_PORTS 5 > > + > > struct i915_audio_component { > > struct device *dev; > > + /** > > + * @aud_sample_rate: the array of audio sample rate per port > > + */ > > + int aud_sample_rate[MAX_PORTS]; > > Just a quick question as I was trying to understand the code: > > aud_sample_rate seems only set and get on the i915 side, never on the > hda side. > > In short, why is this variable here, and not in e g, as a single integer > in the intel_digital_port struct? > > That way we also avoid the possibility of MAX_PORTS becoming out of > sync > with the i915 side. Put it in struct i915_audio_component just because it is audio related and easy for management. It seems to put it in intel_digital_port will be easier. Let's hear the gfx team's comments. Regards, Libin > > -- > David Henningsson, Canonical Ltd. > https://launchpad.net/~diwic
On Tue, Oct 13, 2015 at 02:21:31PM +0000, Yang, Libin wrote: > + gfx driver team. > > Hi David, > > > -----Original Message----- > > From: David Henningsson [mailto:david.henningsson@canonical.com] > > Sent: Tuesday, October 13, 2015 9:43 PM > > To: alsa-devel@alsa-project.org; Yang, Libin > > Subject: Re: [alsa-devel] [PATCH v2] drm/i915: set proper N/CTS in > > modeset > > > > > > > > On 2015-09-25 03:36, libin.yang@intel.com wrote: > > > @@ -24,8 +24,18 @@ > > > #ifndef _I915_COMPONENT_H_ > > > #define _I915_COMPONENT_H_ > > > > > > +/* MAX_PORT is the number of port > > > + * It must be sync with I915_MAX_PORTS defined i915_drv.h > > > + * 5 should be enough as only HSW, BDW, SKL need such fix. > > > + */ > > > +#define MAX_PORTS 5 > > > + > > > struct i915_audio_component { > > > struct device *dev; > > > + /** > > > + * @aud_sample_rate: the array of audio sample rate per port > > > + */ > > > + int aud_sample_rate[MAX_PORTS]; > > > > Just a quick question as I was trying to understand the code: > > > > aud_sample_rate seems only set and get on the i915 side, never on the > > hda side. > > > > In short, why is this variable here, and not in e g, as a single integer > > in the intel_digital_port struct? > > > > That way we also avoid the possibility of MAX_PORTS becoming out of > > sync > > with the i915 side. > > Put it in struct i915_audio_component just because it is audio related and > easy for management. > > It seems to put it in intel_digital_port will be easier. IIRC that's more or less how I pseudo-coded it in one of my replies in one of the threads discussing this... Yeah here http://lists.freedesktop.org/archives/intel-gfx/2015-August/074267.html
On Tue, 13 Oct 2015, "Yang, Libin" <libin.yang@intel.com> wrote: > + gfx driver team. > > Hi David, > >> -----Original Message----- >> From: David Henningsson [mailto:david.henningsson@canonical.com] >> Sent: Tuesday, October 13, 2015 9:43 PM >> To: alsa-devel@alsa-project.org; Yang, Libin >> Subject: Re: [alsa-devel] [PATCH v2] drm/i915: set proper N/CTS in >> modeset >> >> >> >> On 2015-09-25 03:36, libin.yang@intel.com wrote: >> > @@ -24,8 +24,18 @@ >> > #ifndef _I915_COMPONENT_H_ >> > #define _I915_COMPONENT_H_ >> > >> > +/* MAX_PORT is the number of port >> > + * It must be sync with I915_MAX_PORTS defined i915_drv.h >> > + * 5 should be enough as only HSW, BDW, SKL need such fix. >> > + */ >> > +#define MAX_PORTS 5 >> > + >> > struct i915_audio_component { >> > struct device *dev; >> > + /** >> > + * @aud_sample_rate: the array of audio sample rate per port >> > + */ >> > + int aud_sample_rate[MAX_PORTS]; >> >> Just a quick question as I was trying to understand the code: >> >> aud_sample_rate seems only set and get on the i915 side, never on the >> hda side. >> >> In short, why is this variable here, and not in e g, as a single integer >> in the intel_digital_port struct? >> >> That way we also avoid the possibility of MAX_PORTS becoming out of >> sync >> with the i915 side. > > Put it in struct i915_audio_component just because it is audio related and > easy for management. > > It seems to put it in intel_digital_port will be easier. > > Let's hear the gfx team's comments. Ack on moving it into struct intel_digital_port. We had a bit of a hurry to get the original patches in, didn't want to start bikeshedding at each step of the way... BR, Jani. > > Regards, > Libin > >> >> -- >> David Henningsson, Canonical Ltd. >> https://launchpad.net/~diwic
diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index e3c32ce..642dfbd 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -128,6 +128,20 @@ static int audio_config_get_n(const struct drm_display_mode *mode, int rate) return 0; } +static uint32_t audio_config_setup_n_reg(int n, uint32_t val) +{ + int n_low, n_up; + uint32_t tmp = val; + + 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); + return tmp; +} + /* check whether N/CTS/M need be set manually */ static bool audio_rate_need_prog(struct intel_crtc *crtc, struct drm_display_mode *mode) @@ -262,9 +276,14 @@ static void hsw_audio_codec_enable(struct drm_connector *connector, struct drm_i915_private *dev_priv = connector->dev->dev_private; struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc); enum pipe pipe = intel_crtc->pipe; + struct i915_audio_component *acomp = dev_priv->audio_component; const uint8_t *eld = connector->eld; + struct intel_digital_port *intel_dig_port = + enc_to_dig_port(&encoder->base); + enum port port = intel_dig_port->port; uint32_t tmp; int len, i; + int n, rate; DRM_DEBUG_KMS("Enable audio codec on pipe %c, %u bytes ELD\n", pipe_name(pipe), drm_eld_size(eld)); @@ -302,12 +321,29 @@ 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)) { + if (!acomp) + rate = 0; + else if (port >= PORT_A && port <= PORT_E) + rate = acomp->aud_sample_rate[port]; + else { + DRM_ERROR("invalid port: %d\n", port); + rate = 0; + } + n = audio_config_get_n(mode, rate); + if (n != 0) + tmp = audio_config_setup_n_reg(n, tmp); + else + DRM_DEBUG_KMS("no suitable N value is found\n"); + } + I915_WRITE(HSW_AUD_CFG(pipe), tmp); mutex_unlock(&dev_priv->av_mutex); @@ -594,9 +630,10 @@ static int i915_audio_component_sync_audio_rate(struct device *dev, struct intel_digital_port *intel_dig_port; struct intel_crtc *crtc; struct drm_display_mode *mode; + struct i915_audio_component *acomp = dev_priv->audio_component; enum pipe pipe = -1; u32 tmp; - int n_low, n_up, n; + int n; /* HSW, BDW SKL need this fix */ if (!IS_SKYLAKE(dev_priv) && @@ -630,6 +667,9 @@ static int i915_audio_component_sync_audio_rate(struct device *dev, pipe_name(pipe), port_name(port)); mode = &crtc->config->base.adjusted_mode; + /* port must be valid now, otherwise the pipe will be invalid */ + acomp->aud_sample_rate[port] = rate; + /* 2. check whether to set the N/CTS/M manually or not */ if (!audio_rate_need_prog(crtc, mode)) { tmp = I915_READ(HSW_AUD_CFG(pipe)); @@ -649,15 +689,10 @@ static int i915_audio_component_sync_audio_rate(struct device *dev, mutex_unlock(&dev_priv->av_mutex); return 0; } - n_low = n & 0xfff; - n_up = (n >> 12) & 0xff; - /* 4. set the N/CTS/M */ + /* 3. set the N/CTS/M */ tmp = I915_READ(HSW_AUD_CFG(pipe)); - 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); + tmp = audio_config_setup_n_reg(n, tmp); I915_WRITE(HSW_AUD_CFG(pipe), tmp); mutex_unlock(&dev_priv->av_mutex); @@ -678,6 +713,7 @@ static int i915_audio_component_bind(struct device *i915_dev, { struct i915_audio_component *acomp = data; struct drm_i915_private *dev_priv = dev_to_i915(i915_dev); + int i; if (WARN_ON(acomp->ops || acomp->dev)) return -EEXIST; @@ -685,6 +721,9 @@ static int i915_audio_component_bind(struct device *i915_dev, drm_modeset_lock_all(dev_priv->dev); acomp->ops = &i915_audio_component_ops; acomp->dev = i915_dev; + BUILD_BUG_ON(MAX_PORTS != I915_MAX_PORTS); + for (i = 0; i < ARRAY_SIZE(acomp->aud_sample_rate); i++) + acomp->aud_sample_rate[i] = 0; dev_priv->audio_component = acomp; drm_modeset_unlock_all(dev_priv->dev); diff --git a/include/drm/i915_component.h b/include/drm/i915_component.h index e6d35d7..89dc7d6 100644 --- a/include/drm/i915_component.h +++ b/include/drm/i915_component.h @@ -24,8 +24,18 @@ #ifndef _I915_COMPONENT_H_ #define _I915_COMPONENT_H_ +/* MAX_PORT is the number of port + * It must be sync with I915_MAX_PORTS defined i915_drv.h + * 5 should be enough as only HSW, BDW, SKL need such fix. + */ +#define MAX_PORTS 5 + struct i915_audio_component { struct device *dev; + /** + * @aud_sample_rate: the array of audio sample rate per port + */ + int aud_sample_rate[MAX_PORTS]; const struct i915_audio_component_ops { struct module *owner;