[alsa-devel] [PATCH v2] drm/i915: set proper N/CTS in modeset
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/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;
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);elseDRM_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;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);elseDRM_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;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
participants (7)
-
Daniel Vetter -
David Henningsson -
Jani Nikula -
libin.yang＠intel.com -
Takashi Iwai -
Ville Syrjälä -
Yang, Libin