* [PATCH RESEND 0/9] drm/i915/audio: audio cleanups, 4k fixes
@ 2016-10-10 15:03 Jani Nikula
2016-10-10 15:04 ` [PATCH RESEND 1/9] drm/i915/audio: abstract audio config update Jani Nikula
` (9 more replies)
0 siblings, 10 replies; 29+ messages in thread
From: Jani Nikula @ 2016-10-10 15:03 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, libin.yang, Dhinakaran Pandiyan
Resend of [1] due to no review. Patches 1-8 should be easy stuff to
review. *wink*.
BR,
Jani.
[1] https://patchwork.freedesktop.org/series/12754/
Jani Nikula (7):
drm/i915/audio: abstract audio config update
drm/i915/audio: port is going to be just fine, simplify checks
drm/i915/audio: use the same code for updating audio config
drm/i915/audio: split dp and hdmi audio config update
drm/i915/audio: add register macros for audio config N value
drm/i915/audio: rename N value getter to emphasize it's for hdmi
drm/i915: set proper N/M in modeset
Libin Yang (2):
drm/i915/audio: set proper N/MCTS on more platforms
drm/i915/audio: HDMI audio gets the TMDS clock by crtc_clock
drivers/gpu/drm/i915/i915_reg.h | 11 ++
drivers/gpu/drm/i915/intel_audio.c | 260 ++++++++++++++++++++++++-------------
2 files changed, 179 insertions(+), 92 deletions(-)
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH RESEND 1/9] drm/i915/audio: abstract audio config update
2016-10-10 15:03 [PATCH RESEND 0/9] drm/i915/audio: audio cleanups, 4k fixes Jani Nikula
@ 2016-10-10 15:04 ` Jani Nikula
2016-10-11 1:59 ` Yang, Libin
2016-10-10 15:04 ` [PATCH RESEND 2/9] drm/i915/audio: port is going to be just fine, simplify checks Jani Nikula
` (8 subsequent siblings)
9 siblings, 1 reply; 29+ messages in thread
From: Jani Nikula @ 2016-10-10 15:04 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, libin.yang, Dhinakaran Pandiyan
Prepare for using the same code for updating HSW_AUD_CFG register. No
functional changes.
Cc: Libin Yang <libin.yang@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/i915/intel_audio.c | 68 ++++++++++++++++++++++----------------
1 file changed, 40 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index 9583f432e02e..0a54f7cdce37 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -245,6 +245,45 @@ static void g4x_audio_codec_enable(struct drm_connector *connector,
I915_WRITE(G4X_AUD_CNTL_ST, tmp);
}
+static void hsw_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;
+ enum pipe pipe = intel_crtc->pipe;
+ int n, rate;
+ u32 tmp;
+
+ tmp = I915_READ(HSW_AUD_CFG(pipe));
+ tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
+ tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
+ if (intel_crtc_has_dp_encoder(intel_crtc->config))
+ tmp |= AUD_CONFIG_N_VALUE_INDEX;
+ else
+ tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
+
+ tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
+ if (audio_rate_need_prog(intel_crtc, adjusted_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(adjusted_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);
+}
+
static void hsw_audio_codec_disable(struct intel_encoder *encoder)
{
struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
@@ -283,11 +322,9 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
struct intel_crtc *intel_crtc = to_intel_crtc(intel_encoder->base.crtc);
enum pipe pipe = intel_crtc->pipe;
enum port port = intel_encoder->port;
- struct i915_audio_component *acomp = dev_priv->audio_component;
const uint8_t *eld = connector->eld;
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));
@@ -323,32 +360,7 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp);
/* Enable timestamps */
- tmp = I915_READ(HSW_AUD_CFG(pipe));
- tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
- tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
- if (intel_crtc_has_dp_encoder(intel_crtc->config))
- tmp |= AUD_CONFIG_N_VALUE_INDEX;
- else
- tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
-
- tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
- if (audio_rate_need_prog(intel_crtc, adjusted_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(adjusted_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);
+ hsw_audio_config_update(intel_crtc, port, adjusted_mode);
mutex_unlock(&dev_priv->av_mutex);
}
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH RESEND 2/9] drm/i915/audio: port is going to be just fine, simplify checks
2016-10-10 15:03 [PATCH RESEND 0/9] drm/i915/audio: audio cleanups, 4k fixes Jani Nikula
2016-10-10 15:04 ` [PATCH RESEND 1/9] drm/i915/audio: abstract audio config update Jani Nikula
@ 2016-10-10 15:04 ` Jani Nikula
2016-10-11 2:37 ` Yang, Libin
2016-10-10 15:04 ` [PATCH RESEND 3/9] drm/i915/audio: use the same code for updating audio config Jani Nikula
` (7 subsequent siblings)
9 siblings, 1 reply; 29+ messages in thread
From: Jani Nikula @ 2016-10-10 15:04 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, libin.yang, Dhinakaran Pandiyan
If it was wrong, we'd be screwed already.
Cc: Libin Yang <libin.yang@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/i915/intel_audio.c | 12 ++----------
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index 0a54f7cdce37..5d0bd07afa51 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -251,8 +251,9 @@ static void hsw_audio_config_update(struct intel_crtc *intel_crtc,
{
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;
- int n, rate;
+ int n;
u32 tmp;
tmp = I915_READ(HSW_AUD_CFG(pipe));
@@ -265,15 +266,6 @@ static void hsw_audio_config_update(struct intel_crtc *intel_crtc,
tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
if (audio_rate_need_prog(intel_crtc, adjusted_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(adjusted_mode, rate);
if (n != 0)
tmp = audio_config_setup_n_reg(n, tmp);
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH RESEND 3/9] drm/i915/audio: use the same code for updating audio config
2016-10-10 15:03 [PATCH RESEND 0/9] drm/i915/audio: audio cleanups, 4k fixes Jani Nikula
2016-10-10 15:04 ` [PATCH RESEND 1/9] drm/i915/audio: abstract audio config update Jani Nikula
2016-10-10 15:04 ` [PATCH RESEND 2/9] drm/i915/audio: port is going to be just fine, simplify checks Jani Nikula
@ 2016-10-10 15:04 ` Jani Nikula
2016-10-11 5:25 ` Yang, Libin
2016-10-10 15:04 ` [PATCH RESEND 4/9] drm/i915/audio: split dp and hdmi audio config update Jani Nikula
` (6 subsequent siblings)
9 siblings, 1 reply; 29+ messages in thread
From: Jani Nikula @ 2016-10-10 15:04 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, libin.yang, Dhinakaran Pandiyan
It gets fragile to duplicate the code for updating HSW_AUD_CFG. The only
change should be that the hdmi pixel clock is also updated in
i915_audio_component_sync_audio_rate(), but it should not be any
different.
Cc: Libin Yang <libin.yang@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/i915/intel_audio.c | 29 +++--------------------------
1 file changed, 3 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index 5d0bd07afa51..4d62b3e8ac19 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -671,10 +671,8 @@ static int i915_audio_component_sync_audio_rate(struct device *kdev, int port,
struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
struct intel_encoder *intel_encoder;
struct intel_crtc *crtc;
- struct drm_display_mode *mode;
+ struct drm_display_mode *adjusted_mode;
struct i915_audio_component *acomp = dev_priv->audio_component;
- u32 tmp;
- int n;
int err = 0;
/* HSW, BDW, SKL, KBL need this fix */
@@ -700,33 +698,12 @@ static int i915_audio_component_sync_audio_rate(struct device *kdev, int port,
crtc = to_intel_crtc(intel_encoder->base.crtc);
pipe = crtc->pipe;
- mode = &crtc->config->base.adjusted_mode;
+ adjusted_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));
- tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
- I915_WRITE(HSW_AUD_CFG(pipe), tmp);
- goto unlock;
- }
-
- n = audio_config_get_n(mode, rate);
- if (n == 0) {
- DRM_DEBUG_KMS("Using automatic mode for N value on port %c\n",
- port_name(port));
- tmp = I915_READ(HSW_AUD_CFG(pipe));
- tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
- I915_WRITE(HSW_AUD_CFG(pipe), tmp);
- goto unlock;
- }
-
- /* 3. set the N/CTS/M */
- tmp = I915_READ(HSW_AUD_CFG(pipe));
- tmp = audio_config_setup_n_reg(n, tmp);
- I915_WRITE(HSW_AUD_CFG(pipe), tmp);
+ hsw_audio_config_update(crtc, port, adjusted_mode);
unlock:
mutex_unlock(&dev_priv->av_mutex);
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH RESEND 4/9] drm/i915/audio: split dp and hdmi audio config update
2016-10-10 15:03 [PATCH RESEND 0/9] drm/i915/audio: audio cleanups, 4k fixes Jani Nikula
` (2 preceding siblings ...)
2016-10-10 15:04 ` [PATCH RESEND 3/9] drm/i915/audio: use the same code for updating audio config Jani Nikula
@ 2016-10-10 15:04 ` Jani Nikula
2016-10-11 5:42 ` Yang, Libin
2016-10-10 15:04 ` [PATCH RESEND 5/9] drm/i915/audio: set proper N/MCTS on more platforms Jani Nikula
` (5 subsequent siblings)
9 siblings, 1 reply; 29+ messages in thread
From: Jani Nikula @ 2016-10-10 15:04 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, libin.yang, Dhinakaran Pandiyan
The code for dp and hdmi are already different, and they're about to
diverge even more. Split them for clarity in future work. No functional
changes.
Cc: Libin Yang <libin.yang@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/i915/intel_audio.c | 55 +++++++++++++++++++++++---------------
1 file changed, 34 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index 4d62b3e8ac19..db7fd6d8333f 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -148,18 +148,6 @@ static uint32_t audio_config_setup_n_reg(int n, uint32_t val)
return tmp;
}
-/* check whether N/CTS/M need be set manually */
-static bool audio_rate_need_prog(struct intel_crtc *crtc,
- const struct drm_display_mode *mode)
-{
- if (((mode->clock == TMDS_297M) ||
- (mode->clock == TMDS_296M)) &&
- intel_crtc_has_type(crtc->config, INTEL_OUTPUT_HDMI))
- return true;
- else
- return false;
-}
-
static bool intel_eld_uptodate(struct drm_connector *connector,
i915_reg_t reg_eldv, uint32_t bits_eldv,
i915_reg_t reg_elda, uint32_t bits_elda,
@@ -245,9 +233,26 @@ static void g4x_audio_codec_enable(struct drm_connector *connector,
I915_WRITE(G4X_AUD_CNTL_ST, tmp);
}
-static void hsw_audio_config_update(struct intel_crtc *intel_crtc,
- enum port port,
- const struct drm_display_mode *adjusted_mode)
+static void
+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);
+ enum pipe pipe = intel_crtc->pipe;
+ u32 tmp;
+
+ tmp = I915_READ(HSW_AUD_CFG(pipe));
+ tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
+ tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
+ tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
+ tmp |= AUD_CONFIG_N_VALUE_INDEX;
+
+ I915_WRITE(HSW_AUD_CFG(pipe), tmp);
+}
+
+static void
+hsw_hdmi_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;
@@ -259,13 +264,11 @@ static void hsw_audio_config_update(struct intel_crtc *intel_crtc,
tmp = I915_READ(HSW_AUD_CFG(pipe));
tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
- if (intel_crtc_has_dp_encoder(intel_crtc->config))
- tmp |= AUD_CONFIG_N_VALUE_INDEX;
- else
- tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
-
tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
- if (audio_rate_need_prog(intel_crtc, adjusted_mode)) {
+ tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
+
+ if (adjusted_mode->clock == TMDS_296M ||
+ adjusted_mode->clock == TMDS_297M) {
n = audio_config_get_n(adjusted_mode, rate);
if (n != 0)
tmp = audio_config_setup_n_reg(n, tmp);
@@ -276,6 +279,16 @@ static void hsw_audio_config_update(struct intel_crtc *intel_crtc,
I915_WRITE(HSW_AUD_CFG(pipe), tmp);
}
+static void
+hsw_audio_config_update(struct intel_crtc *intel_crtc, enum port port,
+ const struct drm_display_mode *adjusted_mode)
+{
+ if (intel_crtc_has_dp_encoder(intel_crtc->config))
+ hsw_dp_audio_config_update(intel_crtc, port, adjusted_mode);
+ else
+ hsw_hdmi_audio_config_update(intel_crtc, port, adjusted_mode);
+}
+
static void hsw_audio_codec_disable(struct intel_encoder *encoder)
{
struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH RESEND 5/9] drm/i915/audio: set proper N/MCTS on more platforms
2016-10-10 15:03 [PATCH RESEND 0/9] drm/i915/audio: audio cleanups, 4k fixes Jani Nikula
` (3 preceding siblings ...)
2016-10-10 15:04 ` [PATCH RESEND 4/9] drm/i915/audio: split dp and hdmi audio config update Jani Nikula
@ 2016-10-10 15:04 ` Jani Nikula
2016-10-10 15:04 ` [PATCH RESEND 6/9] drm/i915/audio: HDMI audio gets the TMDS clock by crtc_clock Jani Nikula
` (4 subsequent siblings)
9 siblings, 0 replies; 29+ messages in thread
From: Jani Nikula @ 2016-10-10 15:04 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, libin.yang, Dhinakaran Pandiyan
From: Libin Yang <libin.yang@linux.intel.com>
This patch applies setting proper N/M, N/CTS on more platforms.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/i915/intel_audio.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index db7fd6d8333f..bce3ad2eb86d 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -688,11 +688,7 @@ static int i915_audio_component_sync_audio_rate(struct device *kdev, int port,
struct i915_audio_component *acomp = dev_priv->audio_component;
int err = 0;
- /* HSW, BDW, SKL, KBL need this fix */
- if (!IS_SKYLAKE(dev_priv) &&
- !IS_KABYLAKE(dev_priv) &&
- !IS_BROADWELL(dev_priv) &&
- !IS_HASWELL(dev_priv))
+ if (!HAS_DDI(dev_priv))
return 0;
i915_audio_component_get_power(kdev);
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH RESEND 6/9] drm/i915/audio: HDMI audio gets the TMDS clock by crtc_clock
2016-10-10 15:03 [PATCH RESEND 0/9] drm/i915/audio: audio cleanups, 4k fixes Jani Nikula
` (4 preceding siblings ...)
2016-10-10 15:04 ` [PATCH RESEND 5/9] drm/i915/audio: set proper N/MCTS on more platforms Jani Nikula
@ 2016-10-10 15:04 ` Jani Nikula
2016-10-10 15:04 ` [PATCH RESEND 7/9] drm/i915/audio: add register macros for audio config N value Jani Nikula
` (3 subsequent siblings)
9 siblings, 0 replies; 29+ messages in thread
From: Jani Nikula @ 2016-10-10 15:04 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, libin.yang, Dhinakaran Pandiyan
From: Libin Yang <libin.yang@linux.intel.com>
HDMI audio should use crtc_clock to get the TMDS clock.
This patch renames mode to adjusted_mode to unify the name.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/i915/intel_audio.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index bce3ad2eb86d..6aa619a84439 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -121,13 +121,14 @@ 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(const struct drm_display_mode *adjusted_mode,
+ int rate)
{
int i;
for (i = 0; i < ARRAY_SIZE(aud_ncts); i++) {
if ((rate == aud_ncts[i].sample_rate) &&
- (mode->clock == aud_ncts[i].clock)) {
+ (adjusted_mode->crtc_clock == aud_ncts[i].clock)) {
return aud_ncts[i].n;
}
}
@@ -267,8 +268,8 @@ hsw_hdmi_audio_config_update(struct intel_crtc *intel_crtc, enum port port,
tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
- if (adjusted_mode->clock == TMDS_296M ||
- adjusted_mode->clock == TMDS_297M) {
+ if (adjusted_mode->crtc_clock == TMDS_296M ||
+ adjusted_mode->crtc_clock == TMDS_297M) {
n = audio_config_get_n(adjusted_mode, rate);
if (n != 0)
tmp = audio_config_setup_n_reg(n, tmp);
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH RESEND 7/9] drm/i915/audio: add register macros for audio config N value
2016-10-10 15:03 [PATCH RESEND 0/9] drm/i915/audio: audio cleanups, 4k fixes Jani Nikula
` (5 preceding siblings ...)
2016-10-10 15:04 ` [PATCH RESEND 6/9] drm/i915/audio: HDMI audio gets the TMDS clock by crtc_clock Jani Nikula
@ 2016-10-10 15:04 ` Jani Nikula
2016-10-11 5:52 ` Yang, Libin
2016-10-10 15:04 ` [PATCH RESEND 8/9] drm/i915/audio: rename N value getter to emphasize it's for hdmi Jani Nikula
` (2 subsequent siblings)
9 siblings, 1 reply; 29+ messages in thread
From: Jani Nikula @ 2016-10-10 15:04 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, libin.yang, Dhinakaran Pandiyan
Have generic macros in line with the rest of the register bit definition
macros instead of a dedicated function in intel_audio.c, and use them.
No functional changes.
Cc: Libin Yang <libin.yang@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 4 ++++
drivers/gpu/drm/i915/intel_audio.c | 23 ++++++-----------------
2 files changed, 10 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index acc767a52d8e..595d196f753f 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7332,6 +7332,10 @@ enum {
#define AUD_CONFIG_UPPER_N_MASK (0xff << 20)
#define AUD_CONFIG_LOWER_N_SHIFT 4
#define AUD_CONFIG_LOWER_N_MASK (0xfff << 4)
+#define AUD_CONFIG_N_MASK (AUD_CONFIG_UPPER_N_MASK | AUD_CONFIG_LOWER_N_MASK)
+#define AUD_CONFIG_N(n) \
+ (((((n) >> 12) & 0xff) << AUD_CONFIG_UPPER_N_SHIFT) | \
+ (((n) & 0xfff) << AUD_CONFIG_LOWER_N_SHIFT))
#define AUD_CONFIG_PIXEL_CLOCK_HDMI_SHIFT 16
#define AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK (0xf << 16)
#define AUD_CONFIG_PIXEL_CLOCK_HDMI_25175 (0 << 16)
diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index 6aa619a84439..d2c6227f72b8 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -135,20 +135,6 @@ static int audio_config_get_n(const struct drm_display_mode *adjusted_mode,
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;
-}
-
static bool intel_eld_uptodate(struct drm_connector *connector,
i915_reg_t reg_eldv, uint32_t bits_eldv,
i915_reg_t reg_elda, uint32_t bits_elda,
@@ -271,10 +257,13 @@ hsw_hdmi_audio_config_update(struct intel_crtc *intel_crtc, enum port port,
if (adjusted_mode->crtc_clock == TMDS_296M ||
adjusted_mode->crtc_clock == TMDS_297M) {
n = audio_config_get_n(adjusted_mode, rate);
- if (n != 0)
- tmp = audio_config_setup_n_reg(n, tmp);
- else
+ 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");
+ }
}
I915_WRITE(HSW_AUD_CFG(pipe), tmp);
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH RESEND 8/9] drm/i915/audio: rename N value getter to emphasize it's for hdmi
2016-10-10 15:03 [PATCH RESEND 0/9] drm/i915/audio: audio cleanups, 4k fixes Jani Nikula
` (6 preceding siblings ...)
2016-10-10 15:04 ` [PATCH RESEND 7/9] drm/i915/audio: add register macros for audio config N value Jani Nikula
@ 2016-10-10 15:04 ` Jani Nikula
2016-10-11 5:55 ` Yang, Libin
2016-10-10 15:04 ` [PATCH RESEND 9/9] drm/i915: set proper N/M in modeset Jani Nikula
2016-10-10 18:19 ` ✗ Fi.CI.BAT: warning for drm/i915/audio: audio cleanups, 4k fixes (rev3) Patchwork
9 siblings, 1 reply; 29+ messages in thread
From: Jani Nikula @ 2016-10-10 15:04 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, libin.yang, Dhinakaran Pandiyan
We'll be getting a function and a table for dp parameters soon enough,
so rename the function and table for hdmi. No functional changes.
Cc: Libin Yang <libin.yang@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/i915/intel_audio.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index d2c6227f72b8..81df29ca4947 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -81,7 +81,7 @@ static const struct {
int clock;
int n;
int cts;
-} aud_ncts[] = {
+} hdmi_aud_ncts[] = {
{ 44100, TMDS_296M, 4459, 234375 },
{ 44100, TMDS_297M, 4704, 247500 },
{ 48000, TMDS_296M, 5824, 281250 },
@@ -121,15 +121,15 @@ 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 *adjusted_mode,
- int rate)
+static int audio_config_hdmi_get_n(const struct drm_display_mode *adjusted_mode,
+ int rate)
{
int i;
- for (i = 0; i < ARRAY_SIZE(aud_ncts); i++) {
- if ((rate == aud_ncts[i].sample_rate) &&
- (adjusted_mode->crtc_clock == aud_ncts[i].clock)) {
- return aud_ncts[i].n;
+ for (i = 0; i < ARRAY_SIZE(hdmi_aud_ncts); i++) {
+ if (rate == hdmi_aud_ncts[i].sample_rate &&
+ adjusted_mode->crtc_clock == hdmi_aud_ncts[i].clock) {
+ return hdmi_aud_ncts[i].n;
}
}
return 0;
@@ -256,7 +256,7 @@ hsw_hdmi_audio_config_update(struct intel_crtc *intel_crtc, enum port port,
if (adjusted_mode->crtc_clock == TMDS_296M ||
adjusted_mode->crtc_clock == TMDS_297M) {
- n = audio_config_get_n(adjusted_mode, rate);
+ n = audio_config_hdmi_get_n(adjusted_mode, rate);
if (n != 0) {
tmp &= ~AUD_CONFIG_N_MASK;
tmp |= AUD_CONFIG_N(n);
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH RESEND 9/9] drm/i915: set proper N/M in modeset
2016-10-10 15:03 [PATCH RESEND 0/9] drm/i915/audio: audio cleanups, 4k fixes Jani Nikula
` (7 preceding siblings ...)
2016-10-10 15:04 ` [PATCH RESEND 8/9] drm/i915/audio: rename N value getter to emphasize it's for hdmi Jani Nikula
@ 2016-10-10 15:04 ` Jani Nikula
2016-10-12 2:45 ` Lin, Mengdong
2016-10-12 6:41 ` Zhang, Keqiao
2016-10-10 18:19 ` ✗ Fi.CI.BAT: warning for drm/i915/audio: audio cleanups, 4k fixes (rev3) Patchwork
9 siblings, 2 replies; 29+ messages in thread
From: Jani Nikula @ 2016-10-10 15:04 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, libin.yang, Dhinakaran Pandiyan
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@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
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
+
#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");
+ }
+ }
+
I915_WRITE(HSW_AUD_CFG(pipe), tmp);
+
+ 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);
+ }
}
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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 29+ messages in thread
* ✗ Fi.CI.BAT: warning for drm/i915/audio: audio cleanups, 4k fixes (rev3)
2016-10-10 15:03 [PATCH RESEND 0/9] drm/i915/audio: audio cleanups, 4k fixes Jani Nikula
` (8 preceding siblings ...)
2016-10-10 15:04 ` [PATCH RESEND 9/9] drm/i915: set proper N/M in modeset Jani Nikula
@ 2016-10-10 18:19 ` Patchwork
9 siblings, 0 replies; 29+ messages in thread
From: Patchwork @ 2016-10-10 18:19 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/audio: audio cleanups, 4k fixes (rev3)
URL : https://patchwork.freedesktop.org/series/12754/
State : warning
== Summary ==
Series 12754v3 drm/i915/audio: audio cleanups, 4k fixes
https://patchwork.freedesktop.org/api/1.0/series/12754/revisions/3/mbox/
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-c:
pass -> DMESG-WARN (fi-skl-6700k)
Test kms_psr_sink_crc:
Subgroup psr_basic:
pass -> DMESG-WARN (fi-skl-6700hq)
Test pm_rpm:
Subgroup basic-pci-d3-state:
pass -> DMESG-WARN (fi-skl-6770hq)
Test vgem_basic:
Subgroup unload:
skip -> PASS (fi-bsw-n3050)
skip -> PASS (fi-hsw-4770)
pass -> SKIP (fi-ilk-650)
fi-bdw-5557u total:248 pass:231 dwarn:0 dfail:0 fail:0 skip:17
fi-bsw-n3050 total:248 pass:205 dwarn:0 dfail:0 fail:0 skip:43
fi-bxt-t5700 total:248 pass:217 dwarn:0 dfail:0 fail:0 skip:31
fi-byt-j1900 total:248 pass:214 dwarn:1 dfail:0 fail:1 skip:32
fi-hsw-4770 total:248 pass:225 dwarn:0 dfail:0 fail:0 skip:23
fi-hsw-4770r total:248 pass:224 dwarn:0 dfail:0 fail:0 skip:24
fi-ilk-650 total:248 pass:184 dwarn:0 dfail:0 fail:2 skip:62
fi-ivb-3520m total:248 pass:221 dwarn:0 dfail:0 fail:0 skip:27
fi-ivb-3770 total:248 pass:207 dwarn:0 dfail:0 fail:0 skip:41
fi-kbl-7200u total:248 pass:222 dwarn:0 dfail:0 fail:0 skip:26
fi-skl-6260u total:248 pass:232 dwarn:0 dfail:0 fail:0 skip:16
fi-skl-6700hq total:248 pass:223 dwarn:1 dfail:0 fail:0 skip:24
fi-skl-6700k total:248 pass:220 dwarn:2 dfail:0 fail:0 skip:26
fi-skl-6770hq total:248 pass:230 dwarn:2 dfail:0 fail:1 skip:15
fi-snb-2520m total:248 pass:211 dwarn:0 dfail:0 fail:0 skip:37
fi-snb-2600 total:248 pass:209 dwarn:0 dfail:0 fail:0 skip:39
Results at /archive/results/CI_IGT_test/Patchwork_2663/
e37a15c8d775e79dddc8345a0f6afdcfe1f607d9 drm-intel-nightly: 2016y-10m-10d-14h-33m-29s UTC integration manifest
ceae7cb drm/i915: set proper N/M in modeset
831ac5b drm/i915/audio: rename N value getter to emphasize it's for hdmi
a12ba34 drm/i915/audio: add register macros for audio config N value
d887d42 drm/i915/audio: HDMI audio gets the TMDS clock by crtc_clock
ba64534 drm/i915/audio: set proper N/MCTS on more platforms
cc137fd drm/i915/audio: split dp and hdmi audio config update
293d738 drm/i915/audio: use the same code for updating audio config
f04978a drm/i915/audio: port is going to be just fine, simplify checks
5c152ee drm/i915/audio: abstract audio config update
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RESEND 1/9] drm/i915/audio: abstract audio config update
2016-10-10 15:04 ` [PATCH RESEND 1/9] drm/i915/audio: abstract audio config update Jani Nikula
@ 2016-10-11 1:59 ` Yang, Libin
0 siblings, 0 replies; 29+ messages in thread
From: Yang, Libin @ 2016-10-11 1:59 UTC (permalink / raw)
To: intel-gfx; +Cc: Nikula, Jani, libin.yang, Pandiyan, Dhinakaran
Reviewed-by: Libin Yang <libin.yang@intel.com>
Regards,
Libin
> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
> Jani Nikula
> Sent: Monday, October 10, 2016 11:04 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Nikula, Jani <jani.nikula@intel.com>; libin.yang@linux.intel.com;
> Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>
> Subject: [Intel-gfx] [PATCH RESEND 1/9] drm/i915/audio: abstract audio
> config update
>
> Prepare for using the same code for updating HSW_AUD_CFG register. No
> functional changes.
>
> Cc: Libin Yang <libin.yang@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/i915/intel_audio.c | 68 ++++++++++++++++++++++------------
> ----
> 1 file changed, 40 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_audio.c
> b/drivers/gpu/drm/i915/intel_audio.c
> index 9583f432e02e..0a54f7cdce37 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -245,6 +245,45 @@ static void g4x_audio_codec_enable(struct
> drm_connector *connector,
> I915_WRITE(G4X_AUD_CNTL_ST, tmp);
> }
>
> +static void hsw_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;
> + enum pipe pipe = intel_crtc->pipe;
> + int n, rate;
> + u32 tmp;
> +
> + tmp = I915_READ(HSW_AUD_CFG(pipe));
> + tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
> + tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
> + if (intel_crtc_has_dp_encoder(intel_crtc->config))
> + tmp |= AUD_CONFIG_N_VALUE_INDEX;
> + else
> + tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
> +
> + tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
> + if (audio_rate_need_prog(intel_crtc, adjusted_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(adjusted_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);
> +}
> +
> static void hsw_audio_codec_disable(struct intel_encoder *encoder) {
> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> @@ -283,11 +322,9 @@ static void hsw_audio_codec_enable(struct
> drm_connector *connector,
> struct intel_crtc *intel_crtc = to_intel_crtc(intel_encoder->base.crtc);
> enum pipe pipe = intel_crtc->pipe;
> enum port port = intel_encoder->port;
> - struct i915_audio_component *acomp = dev_priv-
> >audio_component;
> const uint8_t *eld = connector->eld;
> 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)); @@ -323,32 +360,7
> @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
> I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp);
>
> /* Enable timestamps */
> - tmp = I915_READ(HSW_AUD_CFG(pipe));
> - tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
> - tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
> - if (intel_crtc_has_dp_encoder(intel_crtc->config))
> - tmp |= AUD_CONFIG_N_VALUE_INDEX;
> - else
> - tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
> -
> - tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
> - if (audio_rate_need_prog(intel_crtc, adjusted_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(adjusted_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);
> + hsw_audio_config_update(intel_crtc, port, adjusted_mode);
>
> mutex_unlock(&dev_priv->av_mutex);
> }
> --
> 2.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RESEND 2/9] drm/i915/audio: port is going to be just fine, simplify checks
2016-10-10 15:04 ` [PATCH RESEND 2/9] drm/i915/audio: port is going to be just fine, simplify checks Jani Nikula
@ 2016-10-11 2:37 ` Yang, Libin
0 siblings, 0 replies; 29+ messages in thread
From: Yang, Libin @ 2016-10-11 2:37 UTC (permalink / raw)
To: intel-gfx; +Cc: Nikula, Jani, libin.yang, Pandiyan, Dhinakaran
Reviewed-by: Libin Yang <libin.yang@intel.com>
Regards,
Libin
> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
> Jani Nikula
> Sent: Monday, October 10, 2016 11:04 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Nikula, Jani <jani.nikula@intel.com>; libin.yang@linux.intel.com;
> Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>
> Subject: [Intel-gfx] [PATCH RESEND 2/9] drm/i915/audio: port is going to be
> just fine, simplify checks
>
> If it was wrong, we'd be screwed already.
>
> Cc: Libin Yang <libin.yang@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/i915/intel_audio.c | 12 ++----------
> 1 file changed, 2 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_audio.c
> b/drivers/gpu/drm/i915/intel_audio.c
> index 0a54f7cdce37..5d0bd07afa51 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -251,8 +251,9 @@ static void hsw_audio_config_update(struct intel_crtc
> *intel_crtc, {
> 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;
> - int n, rate;
> + int n;
> u32 tmp;
>
> tmp = I915_READ(HSW_AUD_CFG(pipe));
> @@ -265,15 +266,6 @@ static void hsw_audio_config_update(struct
> intel_crtc *intel_crtc,
>
> tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
> if (audio_rate_need_prog(intel_crtc, adjusted_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(adjusted_mode, rate);
> if (n != 0)
> tmp = audio_config_setup_n_reg(n, tmp);
> --
> 2.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RESEND 3/9] drm/i915/audio: use the same code for updating audio config
2016-10-10 15:04 ` [PATCH RESEND 3/9] drm/i915/audio: use the same code for updating audio config Jani Nikula
@ 2016-10-11 5:25 ` Yang, Libin
0 siblings, 0 replies; 29+ messages in thread
From: Yang, Libin @ 2016-10-11 5:25 UTC (permalink / raw)
To: intel-gfx; +Cc: Nikula, Jani, libin.yang, Pandiyan, Dhinakaran
Reviewed-by: Libin Yang <libin.yang@intel.com>
Regards,
Libin
> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
> Jani Nikula
> Sent: Monday, October 10, 2016 11:04 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Nikula, Jani <jani.nikula@intel.com>; libin.yang@linux.intel.com;
> Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>
> Subject: [Intel-gfx] [PATCH RESEND 3/9] drm/i915/audio: use the same code
> for updating audio config
>
> It gets fragile to duplicate the code for updating HSW_AUD_CFG. The only
> change should be that the hdmi pixel clock is also updated in
> i915_audio_component_sync_audio_rate(), but it should not be any
> different.
>
> Cc: Libin Yang <libin.yang@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/i915/intel_audio.c | 29 +++--------------------------
> 1 file changed, 3 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_audio.c
> b/drivers/gpu/drm/i915/intel_audio.c
> index 5d0bd07afa51..4d62b3e8ac19 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -671,10 +671,8 @@ static int
> i915_audio_component_sync_audio_rate(struct device *kdev, int port,
> struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
> struct intel_encoder *intel_encoder;
> struct intel_crtc *crtc;
> - struct drm_display_mode *mode;
> + struct drm_display_mode *adjusted_mode;
> struct i915_audio_component *acomp = dev_priv-
> >audio_component;
> - u32 tmp;
> - int n;
> int err = 0;
>
> /* HSW, BDW, SKL, KBL need this fix */ @@ -700,33 +698,12 @@
> static int i915_audio_component_sync_audio_rate(struct device *kdev, int
> port,
> crtc = to_intel_crtc(intel_encoder->base.crtc);
> pipe = crtc->pipe;
>
> - mode = &crtc->config->base.adjusted_mode;
> + adjusted_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));
> - tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
> - I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> - goto unlock;
> - }
> -
> - n = audio_config_get_n(mode, rate);
> - if (n == 0) {
> - DRM_DEBUG_KMS("Using automatic mode for N value on
> port %c\n",
> - port_name(port));
> - tmp = I915_READ(HSW_AUD_CFG(pipe));
> - tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
> - I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> - goto unlock;
> - }
> -
> - /* 3. set the N/CTS/M */
> - tmp = I915_READ(HSW_AUD_CFG(pipe));
> - tmp = audio_config_setup_n_reg(n, tmp);
> - I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> + hsw_audio_config_update(crtc, port, adjusted_mode);
>
> unlock:
> mutex_unlock(&dev_priv->av_mutex);
> --
> 2.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RESEND 4/9] drm/i915/audio: split dp and hdmi audio config update
2016-10-10 15:04 ` [PATCH RESEND 4/9] drm/i915/audio: split dp and hdmi audio config update Jani Nikula
@ 2016-10-11 5:42 ` Yang, Libin
0 siblings, 0 replies; 29+ messages in thread
From: Yang, Libin @ 2016-10-11 5:42 UTC (permalink / raw)
To: intel-gfx; +Cc: Nikula, Jani, libin.yang, Pandiyan, Dhinakaran
Reviewed-by: Libin Yang <libin.yang@intel.com>
Regards,
Libin
> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
> Jani Nikula
> Sent: Monday, October 10, 2016 11:04 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Nikula, Jani <jani.nikula@intel.com>; libin.yang@linux.intel.com;
> Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>
> Subject: [Intel-gfx] [PATCH RESEND 4/9] drm/i915/audio: split dp and hdmi
> audio config update
>
> The code for dp and hdmi are already different, and they're about to diverge
> even more. Split them for clarity in future work. No functional changes.
>
> Cc: Libin Yang <libin.yang@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/i915/intel_audio.c | 55 +++++++++++++++++++++++----------
> -----
> 1 file changed, 34 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_audio.c
> b/drivers/gpu/drm/i915/intel_audio.c
> index 4d62b3e8ac19..db7fd6d8333f 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -148,18 +148,6 @@ static uint32_t audio_config_setup_n_reg(int n,
> uint32_t val)
> return tmp;
> }
>
> -/* check whether N/CTS/M need be set manually */ -static bool
> audio_rate_need_prog(struct intel_crtc *crtc,
> - const struct drm_display_mode *mode)
> -{
> - if (((mode->clock == TMDS_297M) ||
> - (mode->clock == TMDS_296M)) &&
> - intel_crtc_has_type(crtc->config, INTEL_OUTPUT_HDMI))
> - return true;
> - else
> - return false;
> -}
> -
> static bool intel_eld_uptodate(struct drm_connector *connector,
> i915_reg_t reg_eldv, uint32_t bits_eldv,
> i915_reg_t reg_elda, uint32_t bits_elda, @@ -
> 245,9 +233,26 @@ static void g4x_audio_codec_enable(struct
> drm_connector *connector,
> I915_WRITE(G4X_AUD_CNTL_ST, tmp);
> }
>
> -static void hsw_audio_config_update(struct intel_crtc *intel_crtc,
> - enum port port,
> - const struct drm_display_mode
> *adjusted_mode)
> +static void
> +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);
> + enum pipe pipe = intel_crtc->pipe;
> + u32 tmp;
> +
> + tmp = I915_READ(HSW_AUD_CFG(pipe));
> + tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
> + tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
> + tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
> + tmp |= AUD_CONFIG_N_VALUE_INDEX;
> +
> + I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> +}
> +
> +static void
> +hsw_hdmi_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; @@ -259,13 +264,11 @@ static void
> hsw_audio_config_update(struct intel_crtc *intel_crtc,
> tmp = I915_READ(HSW_AUD_CFG(pipe));
> tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
> tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
> - if (intel_crtc_has_dp_encoder(intel_crtc->config))
> - tmp |= AUD_CONFIG_N_VALUE_INDEX;
> - else
> - tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
> -
> tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
> - if (audio_rate_need_prog(intel_crtc, adjusted_mode)) {
> + tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
> +
> + if (adjusted_mode->clock == TMDS_296M ||
> + adjusted_mode->clock == TMDS_297M) {
> n = audio_config_get_n(adjusted_mode, rate);
> if (n != 0)
> tmp = audio_config_setup_n_reg(n, tmp); @@ -276,6
> +279,16 @@ static void hsw_audio_config_update(struct intel_crtc
> *intel_crtc,
> I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> }
>
> +static void
> +hsw_audio_config_update(struct intel_crtc *intel_crtc, enum port port,
> + const struct drm_display_mode *adjusted_mode) {
> + if (intel_crtc_has_dp_encoder(intel_crtc->config))
> + hsw_dp_audio_config_update(intel_crtc, port,
> adjusted_mode);
> + else
> + hsw_hdmi_audio_config_update(intel_crtc, port,
> adjusted_mode); }
> +
> static void hsw_audio_codec_disable(struct intel_encoder *encoder) {
> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> --
> 2.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RESEND 7/9] drm/i915/audio: add register macros for audio config N value
2016-10-10 15:04 ` [PATCH RESEND 7/9] drm/i915/audio: add register macros for audio config N value Jani Nikula
@ 2016-10-11 5:52 ` Yang, Libin
0 siblings, 0 replies; 29+ messages in thread
From: Yang, Libin @ 2016-10-11 5:52 UTC (permalink / raw)
To: intel-gfx; +Cc: Nikula, Jani, libin.yang, Pandiyan, Dhinakaran
Reviewed-by: Libin Yang <libin.yang@intel.com>
Regards,
Libin
> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
> Jani Nikula
> Sent: Monday, October 10, 2016 11:04 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Nikula, Jani <jani.nikula@intel.com>; libin.yang@linux.intel.com;
> Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>
> Subject: [Intel-gfx] [PATCH RESEND 7/9] drm/i915/audio: add register macros
> for audio config N value
>
> Have generic macros in line with the rest of the register bit definition macros
> instead of a dedicated function in intel_audio.c, and use them.
> No functional changes.
>
> Cc: Libin Yang <libin.yang@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 4 ++++
> drivers/gpu/drm/i915/intel_audio.c | 23 ++++++-----------------
> 2 files changed, 10 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h index acc767a52d8e..595d196f753f
> 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7332,6 +7332,10 @@ enum {
> #define AUD_CONFIG_UPPER_N_MASK (0xff << 20)
> #define AUD_CONFIG_LOWER_N_SHIFT 4
> #define AUD_CONFIG_LOWER_N_MASK (0xfff << 4)
> +#define AUD_CONFIG_N_MASK
> (AUD_CONFIG_UPPER_N_MASK | AUD_CONFIG_LOWER_N_MASK)
> +#define AUD_CONFIG_N(n) \
> + (((((n) >> 12) & 0xff) << AUD_CONFIG_UPPER_N_SHIFT) | \
> + (((n) & 0xfff) << AUD_CONFIG_LOWER_N_SHIFT))
> #define AUD_CONFIG_PIXEL_CLOCK_HDMI_SHIFT 16
> #define AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK (0xf << 16)
> #define AUD_CONFIG_PIXEL_CLOCK_HDMI_25175 (0 << 16)
> diff --git a/drivers/gpu/drm/i915/intel_audio.c
> b/drivers/gpu/drm/i915/intel_audio.c
> index 6aa619a84439..d2c6227f72b8 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -135,20 +135,6 @@ static int audio_config_get_n(const struct
> drm_display_mode *adjusted_mode,
> 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;
> -}
> -
> static bool intel_eld_uptodate(struct drm_connector *connector,
> i915_reg_t reg_eldv, uint32_t bits_eldv,
> i915_reg_t reg_elda, uint32_t bits_elda, @@ -
> 271,10 +257,13 @@ hsw_hdmi_audio_config_update(struct intel_crtc
> *intel_crtc, enum port port,
> if (adjusted_mode->crtc_clock == TMDS_296M ||
> adjusted_mode->crtc_clock == TMDS_297M) {
> n = audio_config_get_n(adjusted_mode, rate);
> - if (n != 0)
> - tmp = audio_config_setup_n_reg(n, tmp);
> - else
> + 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");
> + }
> }
>
> I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> --
> 2.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RESEND 8/9] drm/i915/audio: rename N value getter to emphasize it's for hdmi
2016-10-10 15:04 ` [PATCH RESEND 8/9] drm/i915/audio: rename N value getter to emphasize it's for hdmi Jani Nikula
@ 2016-10-11 5:55 ` Yang, Libin
2016-10-11 14:25 ` Jani Nikula
0 siblings, 1 reply; 29+ messages in thread
From: Yang, Libin @ 2016-10-11 5:55 UTC (permalink / raw)
To: intel-gfx; +Cc: Nikula, Jani, libin.yang, Pandiyan, Dhinakaran
Reviewed-by: Libin Yang <libin.yang@intel.com>
Regards,
Libin
> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
> Jani Nikula
> Sent: Monday, October 10, 2016 11:04 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Nikula, Jani <jani.nikula@intel.com>; libin.yang@linux.intel.com;
> Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>
> Subject: [Intel-gfx] [PATCH RESEND 8/9] drm/i915/audio: rename N value
> getter to emphasize it's for hdmi
>
> We'll be getting a function and a table for dp parameters soon enough, so
> rename the function and table for hdmi. No functional changes.
>
> Cc: Libin Yang <libin.yang@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/i915/intel_audio.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_audio.c
> b/drivers/gpu/drm/i915/intel_audio.c
> index d2c6227f72b8..81df29ca4947 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -81,7 +81,7 @@ static const struct {
> int clock;
> int n;
> int cts;
> -} aud_ncts[] = {
> +} hdmi_aud_ncts[] = {
> { 44100, TMDS_296M, 4459, 234375 },
> { 44100, TMDS_297M, 4704, 247500 },
> { 48000, TMDS_296M, 5824, 281250 },
> @@ -121,15 +121,15 @@ 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
> *adjusted_mode,
> - int rate)
> +static int audio_config_hdmi_get_n(const struct drm_display_mode
> *adjusted_mode,
> + int rate)
> {
> int i;
>
> - for (i = 0; i < ARRAY_SIZE(aud_ncts); i++) {
> - if ((rate == aud_ncts[i].sample_rate) &&
> - (adjusted_mode->crtc_clock == aud_ncts[i].clock)) {
> - return aud_ncts[i].n;
> + for (i = 0; i < ARRAY_SIZE(hdmi_aud_ncts); i++) {
> + if (rate == hdmi_aud_ncts[i].sample_rate &&
> + adjusted_mode->crtc_clock == hdmi_aud_ncts[i].clock) {
> + return hdmi_aud_ncts[i].n;
> }
> }
> return 0;
> @@ -256,7 +256,7 @@ hsw_hdmi_audio_config_update(struct intel_crtc
> *intel_crtc, enum port port,
>
> if (adjusted_mode->crtc_clock == TMDS_296M ||
> adjusted_mode->crtc_clock == TMDS_297M) {
> - n = audio_config_get_n(adjusted_mode, rate);
> + n = audio_config_hdmi_get_n(adjusted_mode, rate);
> if (n != 0) {
> tmp &= ~AUD_CONFIG_N_MASK;
> tmp |= AUD_CONFIG_N(n);
> --
> 2.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RESEND 8/9] drm/i915/audio: rename N value getter to emphasize it's for hdmi
2016-10-11 5:55 ` Yang, Libin
@ 2016-10-11 14:25 ` Jani Nikula
0 siblings, 0 replies; 29+ messages in thread
From: Jani Nikula @ 2016-10-11 14:25 UTC (permalink / raw)
To: Yang, Libin, intel-gfx; +Cc: libin.yang, Pandiyan, Dhinakaran
On Tue, 11 Oct 2016, "Yang, Libin" <libin.yang@intel.com> wrote:
> Reviewed-by: Libin Yang <libin.yang@intel.com>
Pushed patches 1-8 to drm-intel-next-queued, thanks for the review.
BR,
Jani.
>
> Regards,
> Libin
>
>
>> -----Original Message-----
>> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
>> Jani Nikula
>> Sent: Monday, October 10, 2016 11:04 PM
>> To: intel-gfx@lists.freedesktop.org
>> Cc: Nikula, Jani <jani.nikula@intel.com>; libin.yang@linux.intel.com;
>> Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>
>> Subject: [Intel-gfx] [PATCH RESEND 8/9] drm/i915/audio: rename N value
>> getter to emphasize it's for hdmi
>>
>> We'll be getting a function and a table for dp parameters soon enough, so
>> rename the function and table for hdmi. No functional changes.
>>
>> Cc: Libin Yang <libin.yang@linux.intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>> drivers/gpu/drm/i915/intel_audio.c | 16 ++++++++--------
>> 1 file changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_audio.c
>> b/drivers/gpu/drm/i915/intel_audio.c
>> index d2c6227f72b8..81df29ca4947 100644
>> --- a/drivers/gpu/drm/i915/intel_audio.c
>> +++ b/drivers/gpu/drm/i915/intel_audio.c
>> @@ -81,7 +81,7 @@ static const struct {
>> int clock;
>> int n;
>> int cts;
>> -} aud_ncts[] = {
>> +} hdmi_aud_ncts[] = {
>> { 44100, TMDS_296M, 4459, 234375 },
>> { 44100, TMDS_297M, 4704, 247500 },
>> { 48000, TMDS_296M, 5824, 281250 },
>> @@ -121,15 +121,15 @@ 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
>> *adjusted_mode,
>> - int rate)
>> +static int audio_config_hdmi_get_n(const struct drm_display_mode
>> *adjusted_mode,
>> + int rate)
>> {
>> int i;
>>
>> - for (i = 0; i < ARRAY_SIZE(aud_ncts); i++) {
>> - if ((rate == aud_ncts[i].sample_rate) &&
>> - (adjusted_mode->crtc_clock == aud_ncts[i].clock)) {
>> - return aud_ncts[i].n;
>> + for (i = 0; i < ARRAY_SIZE(hdmi_aud_ncts); i++) {
>> + if (rate == hdmi_aud_ncts[i].sample_rate &&
>> + adjusted_mode->crtc_clock == hdmi_aud_ncts[i].clock) {
>> + return hdmi_aud_ncts[i].n;
>> }
>> }
>> return 0;
>> @@ -256,7 +256,7 @@ hsw_hdmi_audio_config_update(struct intel_crtc
>> *intel_crtc, enum port port,
>>
>> if (adjusted_mode->crtc_clock == TMDS_296M ||
>> adjusted_mode->crtc_clock == TMDS_297M) {
>> - n = audio_config_get_n(adjusted_mode, rate);
>> + n = audio_config_hdmi_get_n(adjusted_mode, rate);
>> if (n != 0) {
>> tmp &= ~AUD_CONFIG_N_MASK;
>> tmp |= AUD_CONFIG_N(n);
>> --
>> 2.1.4
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RESEND 9/9] drm/i915: set proper N/M in modeset
2016-10-10 15:04 ` [PATCH RESEND 9/9] drm/i915: set proper N/M in modeset Jani Nikula
@ 2016-10-12 2:45 ` Lin, Mengdong
2016-10-12 6:29 ` Yang, Libin
2016-10-12 6:41 ` Zhang, Keqiao
1 sibling, 1 reply; 29+ messages in thread
From: Lin, Mengdong @ 2016-10-12 2:45 UTC (permalink / raw)
To: intel-gfx; +Cc: Nikula, Jani, libin.yang, Pandiyan, Dhinakaran
> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
> Jani Nikula
> Sent: Monday, October 10, 2016 11:04 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Nikula, Jani <jani.nikula@intel.com>; libin.yang@linux.intel.com;
> Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>
> Subject: [Intel-gfx] [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@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
> 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?
Do we still need to program write the register to 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?
> +
> + 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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RESEND 9/9] drm/i915: set proper N/M in modeset
2016-10-12 2:45 ` Lin, Mengdong
@ 2016-10-12 6:29 ` Yang, Libin
2016-10-19 15:08 ` Jani Nikula
0 siblings, 1 reply; 29+ messages in thread
From: Yang, Libin @ 2016-10-12 6:29 UTC (permalink / raw)
To: Lin, Mengdong, intel-gfx; +Cc: Nikula, Jani, libin.yang, Pandiyan, Dhinakaran
> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
> Lin, Mengdong
> Sent: Wednesday, October 12, 2016 10:46 AM
> To: Nikula, Jani <jani.nikula@intel.com>; intel-gfx@lists.freedesktop.org
> Cc: Nikula, Jani <jani.nikula@intel.com>; libin.yang@linux.intel.com;
> Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>
> Subject: Re: [Intel-gfx] [PATCH RESEND 9/9] drm/i915: set proper N/M in
> modeset
>
>
>
> > -----Original Message-----
> > From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On
> > Behalf Of Jani Nikula
> > Sent: Monday, October 10, 2016 11:04 PM
> > To: intel-gfx@lists.freedesktop.org
> > Cc: Nikula, Jani <jani.nikula@intel.com>; libin.yang@linux.intel.com;
> > Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>
> > Subject: [Intel-gfx] [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@linux.intel.com>
> > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> > ---
> > 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@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RESEND 9/9] drm/i915: set proper N/M in modeset
2016-10-10 15:04 ` [PATCH RESEND 9/9] drm/i915: set proper N/M in modeset Jani Nikula
2016-10-12 2:45 ` Lin, Mengdong
@ 2016-10-12 6:41 ` Zhang, Keqiao
1 sibling, 0 replies; 29+ messages in thread
From: Zhang, Keqiao @ 2016-10-12 6:41 UTC (permalink / raw)
To: Nikula, Jani, intel-gfx; +Cc: libin.yang, Pandiyan, Dhinakaran
Tested by: Keqiao, Zhang <keqiao.zhang@intel.com>
-----Original Message-----
From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Nikula, Jani
Sent: Monday, October 10, 2016 11:04 PM
To: intel-gfx@lists.freedesktop.org
Cc: Nikula, Jani <jani.nikula@intel.com>; libin.yang@linux.intel.com; Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>
Subject: [Intel-gfx] [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@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
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
+
#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");
+ }
+ }
+
I915_WRITE(HSW_AUD_CFG(pipe), tmp);
+
+ 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);
+ }
}
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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH RESEND 9/9] drm/i915: set proper N/M in modeset
2016-10-12 6:29 ` Yang, Libin
@ 2016-10-19 15:08 ` Jani Nikula
2016-10-20 2:00 ` Yang, Libin
0 siblings, 1 reply; 29+ messages in thread
From: Jani Nikula @ 2016-10-19 15:08 UTC (permalink / raw)
To: Yang, Libin, Lin, Mengdong, intel-gfx
Cc: Zhang, Keqiao, libin.yang, Pandiyan, Dhinakaran, Zhao, Juan J
Sorry it's taken me forever to get back to this. Some comments inline.
BR,
Jani.
On Wed, 12 Oct 2016, "Yang, Libin" <libin.yang@intel.com> wrote:
>> -----Original Message-----
>> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
>> Lin, Mengdong
>> Sent: Wednesday, October 12, 2016 10:46 AM
>> To: Nikula, Jani <jani.nikula@intel.com>; intel-gfx@lists.freedesktop.org
>> Cc: Nikula, Jani <jani.nikula@intel.com>; libin.yang@linux.intel.com;
>> Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>
>> Subject: Re: [Intel-gfx] [PATCH RESEND 9/9] drm/i915: set proper N/M in
>> modeset
>>
>>
>>
>> > -----Original Message-----
>> > From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On
>> > Behalf Of Jani Nikula
>> > Sent: Monday, October 10, 2016 11:04 PM
>> > To: intel-gfx@lists.freedesktop.org
>> > Cc: Nikula, Jani <jani.nikula@intel.com>; libin.yang@linux.intel.com;
>> > Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>
>> > Subject: [Intel-gfx] [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@linux.intel.com>
>> > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> > ---
>> > 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 },
Any particular reason these M/N values are half of what they're in table
2-104 of DP 1.4 spec? (Admittedly the table is an informative example.)
>> > + { 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 },
Do we not support 128 or 64 kHz audio?
>> > +};
>> > +
>> > +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) {
Actually, this check could be removed. audio_config_dp_get_n_m will
always return 0 if this doesn't hold. But not a big deal.
>> > + 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.
If we can't find M/N values, the HSW_AUD_CFG register is set to use
automatic N value. However, the HSW_AUD_M_CTS_ENABLE register is not
updated at all, and could potentially have stale stuff. Do we need to
clear the AUD_M_CTS_M_VALUE_INDEX and/or AUD_M_CTS_M_PROG_ENABLE bits?
We only do this stuff in hsw_audio_codec_enable now. Do we need to do
something in hsw_audio_codec_disable too?
BR,
Jani.
>
> 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@lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RESEND 9/9] drm/i915: set proper N/M in modeset
2016-10-19 15:08 ` Jani Nikula
@ 2016-10-20 2:00 ` Yang, Libin
2016-10-20 8:33 ` Jani Nikula
0 siblings, 1 reply; 29+ messages in thread
From: Yang, Libin @ 2016-10-20 2:00 UTC (permalink / raw)
To: Nikula, Jani, Lin, Mengdong, intel-gfx
Cc: Zhang, Keqiao, libin.yang, Pandiyan, Dhinakaran, Zhao, Juan J
> -----Original Message-----
> From: Nikula, Jani
> Sent: Wednesday, October 19, 2016 11:09 PM
> To: Yang, Libin <libin.yang@intel.com>; Lin, Mengdong
> <mengdong.lin@intel.com>; intel-gfx@lists.freedesktop.org
> Cc: libin.yang@linux.intel.com; Pandiyan, Dhinakaran
> <dhinakaran.pandiyan@intel.com>; Zhang, Keqiao
> <keqiao.zhang@intel.com>; Zhao, Juan J <juan.j.zhao@intel.com>
> Subject: RE: [Intel-gfx] [PATCH RESEND 9/9] drm/i915: set proper N/M in
> modeset
>
>
> Sorry it's taken me forever to get back to this. Some comments inline.
>
> BR,
> Jani.
>
> On Wed, 12 Oct 2016, "Yang, Libin" <libin.yang@intel.com> wrote:
> >> -----Original Message-----
> >> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On
> >> Behalf Of Lin, Mengdong
> >> Sent: Wednesday, October 12, 2016 10:46 AM
> >> To: Nikula, Jani <jani.nikula@intel.com>;
> >> intel-gfx@lists.freedesktop.org
> >> Cc: Nikula, Jani <jani.nikula@intel.com>; libin.yang@linux.intel.com;
> >> Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>
> >> Subject: Re: [Intel-gfx] [PATCH RESEND 9/9] drm/i915: set proper N/M
> >> in modeset
> >>
> >>
> >>
> >> > -----Original Message-----
> >> > From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On
> >> > Behalf Of Jani Nikula
> >> > Sent: Monday, October 10, 2016 11:04 PM
> >> > To: intel-gfx@lists.freedesktop.org
> >> > Cc: Nikula, Jani <jani.nikula@intel.com>;
> >> > libin.yang@linux.intel.com; Pandiyan, Dhinakaran
> >> > <dhinakaran.pandiyan@intel.com>
> >> > Subject: [Intel-gfx] [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@linux.intel.com>
> >> > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >> > ---
> >> > 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 },
>
> Any particular reason these M/N values are half of what they're in table
> 2-104 of DP 1.4 spec? (Admittedly the table is an informative example.)
For HDMI, we found only set N is enough. HW then can handle the remaining.
>
> >> > + { 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 },
>
> Do we not support 128 or 64 kHz audio?
No, we don't support HBR audio.
>
> >> > +};
> >> > +
> >> > +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) {
>
> Actually, this check could be removed. audio_config_dp_get_n_m will always
> return 0 if this doesn't hold. But not a big deal.
Yes, we can remove it.
>
> >> > + 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.
>
> If we can't find M/N values, the HSW_AUD_CFG register is set to use
> automatic N value. However, the HSW_AUD_M_CTS_ENABLE register is not
> updated at all, and could potentially have stale stuff. Do we need to clear the
> AUD_M_CTS_M_VALUE_INDEX and/or AUD_M_CTS_M_PROG_ENABLE bits?
>
> We only do this stuff in hsw_audio_codec_enable now. Do we need to do
> something in hsw_audio_codec_disable too?
In _disable(), the audio doesn't work. So the value will not impact audio.
It will not be worse that we do not reset the value in _disable().
Without reset, the value is "cached". Next time, it may be a little quicker
for monitor response if we are using the same parameter, I think.
Regards
Libin
>
>
> BR,
> Jani.
>
>
> >
> > 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@lists.freedesktop.org
> >> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RESEND 9/9] drm/i915: set proper N/M in modeset
2016-10-20 2:00 ` Yang, Libin
@ 2016-10-20 8:33 ` Jani Nikula
2016-10-20 8:42 ` Yang, Libin
0 siblings, 1 reply; 29+ messages in thread
From: Jani Nikula @ 2016-10-20 8:33 UTC (permalink / raw)
To: Yang, Libin, Lin, Mengdong, intel-gfx
Cc: Zhang, Keqiao, libin.yang, Pandiyan, Dhinakaran, Zhao, Juan J
On Thu, 20 Oct 2016, "Yang, Libin" <libin.yang@intel.com> wrote:
>> -----Original Message-----
>> From: Nikula, Jani
>> Sent: Wednesday, October 19, 2016 11:09 PM
>> To: Yang, Libin <libin.yang@intel.com>; Lin, Mengdong
>> <mengdong.lin@intel.com>; intel-gfx@lists.freedesktop.org
>> Cc: libin.yang@linux.intel.com; Pandiyan, Dhinakaran
>> <dhinakaran.pandiyan@intel.com>; Zhang, Keqiao
>> <keqiao.zhang@intel.com>; Zhao, Juan J <juan.j.zhao@intel.com>
>> Subject: RE: [Intel-gfx] [PATCH RESEND 9/9] drm/i915: set proper N/M in
>> modeset
>>
>>
>> Sorry it's taken me forever to get back to this. Some comments inline.
>>
>> BR,
>> Jani.
>>
>> On Wed, 12 Oct 2016, "Yang, Libin" <libin.yang@intel.com> wrote:
>> >> -----Original Message-----
>> >> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On
>> >> Behalf Of Lin, Mengdong
>> >> Sent: Wednesday, October 12, 2016 10:46 AM
>> >> To: Nikula, Jani <jani.nikula@intel.com>;
>> >> intel-gfx@lists.freedesktop.org
>> >> Cc: Nikula, Jani <jani.nikula@intel.com>; libin.yang@linux.intel.com;
>> >> Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>
>> >> Subject: Re: [Intel-gfx] [PATCH RESEND 9/9] drm/i915: set proper N/M
>> >> in modeset
>> >>
>> >>
>> >>
>> >> > -----Original Message-----
>> >> > From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On
>> >> > Behalf Of Jani Nikula
>> >> > Sent: Monday, October 10, 2016 11:04 PM
>> >> > To: intel-gfx@lists.freedesktop.org
>> >> > Cc: Nikula, Jani <jani.nikula@intel.com>;
>> >> > libin.yang@linux.intel.com; Pandiyan, Dhinakaran
>> >> > <dhinakaran.pandiyan@intel.com>
>> >> > Subject: [Intel-gfx] [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@linux.intel.com>
>> >> > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> >> > ---
>> >> > 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 },
>>
>> Any particular reason these M/N values are half of what they're in table
>> 2-104 of DP 1.4 spec? (Admittedly the table is an informative example.)
>
> For HDMI, we found only set N is enough. HW then can handle the remaining.
I meant, the M and N values in this part of the dp_aud_n_m table are 1/2
of what they are in the DP spec table. Why?
Jani.
>
>>
>> >> > + { 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 },
>>
>> Do we not support 128 or 64 kHz audio?
>
> No, we don't support HBR audio.
>
>>
>> >> > +};
>> >> > +
>> >> > +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) {
>>
>> Actually, this check could be removed. audio_config_dp_get_n_m will always
>> return 0 if this doesn't hold. But not a big deal.
>
> Yes, we can remove it.
>
>>
>> >> > + 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.
>>
>> If we can't find M/N values, the HSW_AUD_CFG register is set to use
>> automatic N value. However, the HSW_AUD_M_CTS_ENABLE register is not
>> updated at all, and could potentially have stale stuff. Do we need to clear the
>> AUD_M_CTS_M_VALUE_INDEX and/or AUD_M_CTS_M_PROG_ENABLE bits?
>>
>> We only do this stuff in hsw_audio_codec_enable now. Do we need to do
>> something in hsw_audio_codec_disable too?
>
> In _disable(), the audio doesn't work. So the value will not impact audio.
> It will not be worse that we do not reset the value in _disable().
> Without reset, the value is "cached". Next time, it may be a little quicker
> for monitor response if we are using the same parameter, I think.
>
> Regards
> Libin
>
>>
>>
>> BR,
>> Jani.
>>
>>
>> >
>> > 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@lists.freedesktop.org
>> >> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> >> _______________________________________________
>> >> Intel-gfx mailing list
>> >> Intel-gfx@lists.freedesktop.org
>> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>> --
>> Jani Nikula, Intel Open Source Technology Center
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RESEND 9/9] drm/i915: set proper N/M in modeset
2016-10-20 8:33 ` Jani Nikula
@ 2016-10-20 8:42 ` Yang, Libin
2016-10-20 11:34 ` Jani Nikula
0 siblings, 1 reply; 29+ messages in thread
From: Yang, Libin @ 2016-10-20 8:42 UTC (permalink / raw)
To: Nikula, Jani, Lin, Mengdong, intel-gfx
Cc: Zhang, Keqiao, libin.yang, Pandiyan, Dhinakaran, Zhao, Juan J
> -----Original Message-----
> From: Nikula, Jani
> Sent: Thursday, October 20, 2016 4:34 PM
> To: Yang, Libin <libin.yang@intel.com>; Lin, Mengdong
> <mengdong.lin@intel.com>; intel-gfx@lists.freedesktop.org
> Cc: libin.yang@linux.intel.com; Pandiyan, Dhinakaran
> <dhinakaran.pandiyan@intel.com>; Zhang, Keqiao
> <keqiao.zhang@intel.com>; Zhao, Juan J <juan.j.zhao@intel.com>
> Subject: RE: [Intel-gfx] [PATCH RESEND 9/9] drm/i915: set proper N/M in
> modeset
>
> On Thu, 20 Oct 2016, "Yang, Libin" <libin.yang@intel.com> wrote:
> >> -----Original Message-----
> >> From: Nikula, Jani
> >> Sent: Wednesday, October 19, 2016 11:09 PM
> >> To: Yang, Libin <libin.yang@intel.com>; Lin, Mengdong
> >> <mengdong.lin@intel.com>; intel-gfx@lists.freedesktop.org
> >> Cc: libin.yang@linux.intel.com; Pandiyan, Dhinakaran
> >> <dhinakaran.pandiyan@intel.com>; Zhang, Keqiao
> >> <keqiao.zhang@intel.com>; Zhao, Juan J <juan.j.zhao@intel.com>
> >> Subject: RE: [Intel-gfx] [PATCH RESEND 9/9] drm/i915: set proper N/M
> >> in modeset
> >>
> >>
> >> Sorry it's taken me forever to get back to this. Some comments inline.
> >>
> >> BR,
> >> Jani.
> >>
> >> On Wed, 12 Oct 2016, "Yang, Libin" <libin.yang@intel.com> wrote:
> >> >> -----Original Message-----
> >> >> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org]
> >> >> On Behalf Of Lin, Mengdong
> >> >> Sent: Wednesday, October 12, 2016 10:46 AM
> >> >> To: Nikula, Jani <jani.nikula@intel.com>;
> >> >> intel-gfx@lists.freedesktop.org
> >> >> Cc: Nikula, Jani <jani.nikula@intel.com>;
> >> >> libin.yang@linux.intel.com; Pandiyan, Dhinakaran
> >> >> <dhinakaran.pandiyan@intel.com>
> >> >> Subject: Re: [Intel-gfx] [PATCH RESEND 9/9] drm/i915: set proper
> >> >> N/M in modeset
> >> >>
> >> >>
> >> >>
> >> >> > -----Original Message-----
> >> >> > From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org]
> >> >> > On Behalf Of Jani Nikula
> >> >> > Sent: Monday, October 10, 2016 11:04 PM
> >> >> > To: intel-gfx@lists.freedesktop.org
> >> >> > Cc: Nikula, Jani <jani.nikula@intel.com>;
> >> >> > libin.yang@linux.intel.com; Pandiyan, Dhinakaran
> >> >> > <dhinakaran.pandiyan@intel.com>
> >> >> > Subject: [Intel-gfx] [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@linux.intel.com>
> >> >> > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >> >> > ---
> >> >> > 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 },
> >>
> >> Any particular reason these M/N values are half of what they're in
> >> table
> >> 2-104 of DP 1.4 spec? (Admittedly the table is an informative
> >> example.)
> >
> > For HDMI, we found only set N is enough. HW then can handle the
> remaining.
>
> I meant, the M and N values in this part of the dp_aud_n_m table are 1/2 of
> what they are in the DP spec table. Why?
Which table are you meaning? I calculate the values myself. I didn't find the
full table in DP spec. I only find the table for 270MHz and 162MHz in
Table 2-50: Examples of Maud and Naud Values
Regards,
Libin
>
> Jani.
>
>
> >
> >>
> >> >> > + { 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 },
> >>
> >> Do we not support 128 or 64 kHz audio?
> >
> > No, we don't support HBR audio.
> >
> >>
> >> >> > +};
> >> >> > +
> >> >> > +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) {
> >>
> >> Actually, this check could be removed. audio_config_dp_get_n_m will
> >> always return 0 if this doesn't hold. But not a big deal.
> >
> > Yes, we can remove it.
> >
> >>
> >> >> > + 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.
> >>
> >> If we can't find M/N values, the HSW_AUD_CFG register is set to use
> >> automatic N value. However, the HSW_AUD_M_CTS_ENABLE register is
> not
> >> updated at all, and could potentially have stale stuff. Do we need to
> >> clear the AUD_M_CTS_M_VALUE_INDEX and/or
> AUD_M_CTS_M_PROG_ENABLE bits?
> >>
> >> We only do this stuff in hsw_audio_codec_enable now. Do we need to do
> >> something in hsw_audio_codec_disable too?
> >
> > In _disable(), the audio doesn't work. So the value will not impact audio.
> > It will not be worse that we do not reset the value in _disable().
> > Without reset, the value is "cached". Next time, it may be a little
> > quicker for monitor response if we are using the same parameter, I think.
> >
> > Regards
> > Libin
> >
> >>
> >>
> >> BR,
> >> Jani.
> >>
> >>
> >> >
> >> > 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@lists.freedesktop.org
> >> >> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >> >> _______________________________________________
> >> >> Intel-gfx mailing list
> >> >> Intel-gfx@lists.freedesktop.org
> >> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >>
> >> --
> >> Jani Nikula, Intel Open Source Technology Center
>
> --
> Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RESEND 9/9] drm/i915: set proper N/M in modeset
2016-10-20 8:42 ` Yang, Libin
@ 2016-10-20 11:34 ` Jani Nikula
2016-10-20 12:02 ` Ville Syrjälä
2016-10-21 6:21 ` Yang, Libin
0 siblings, 2 replies; 29+ messages in thread
From: Jani Nikula @ 2016-10-20 11:34 UTC (permalink / raw)
To: Yang, Libin, Lin, Mengdong, intel-gfx
Cc: Zhang, Keqiao, libin.yang, Pandiyan, Dhinakaran, Zhao, Juan J
On Thu, 20 Oct 2016, "Yang, Libin" <libin.yang@intel.com> wrote:
>> >> Any particular reason these M/N values are half of what they're in
>> >> table
>> >> 2-104 of DP 1.4 spec? (Admittedly the table is an informative
>> >> example.)
>> >
>> > For HDMI, we found only set N is enough. HW then can handle the
>> remaining.
>>
>> I meant, the M and N values in this part of the dp_aud_n_m table are 1/2 of
>> what they are in the DP spec table. Why?
>
> Which table are you meaning? I calculate the values myself. I didn't find the
> full table in DP spec. I only find the table for 270MHz and 162MHz in
> Table 2-50: Examples of Maud and Naud Values
Table 2-104 in DP 1.4. Maybe you're looking at an older version of the
spec.
BR,
Jani.
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RESEND 9/9] drm/i915: set proper N/M in modeset
2016-10-20 11:34 ` Jani Nikula
@ 2016-10-20 12:02 ` Ville Syrjälä
2016-10-21 6:47 ` Yang, Libin
2016-10-21 6:21 ` Yang, Libin
1 sibling, 1 reply; 29+ messages in thread
From: Ville Syrjälä @ 2016-10-20 12:02 UTC (permalink / raw)
To: Jani Nikula
Cc: Zhang, Keqiao, intel-gfx, Zhao, Juan J, Pandiyan, Dhinakaran, libin.yang
On Thu, Oct 20, 2016 at 02:34:34PM +0300, Jani Nikula wrote:
> On Thu, 20 Oct 2016, "Yang, Libin" <libin.yang@intel.com> wrote:
> >> >> Any particular reason these M/N values are half of what they're in
> >> >> table
> >> >> 2-104 of DP 1.4 spec? (Admittedly the table is an informative
> >> >> example.)
> >> >
> >> > For HDMI, we found only set N is enough. HW then can handle the
> >> remaining.
> >>
> >> I meant, the M and N values in this part of the dp_aud_n_m table are 1/2 of
> >> what they are in the DP spec table. Why?
> >
> > Which table are you meaning? I calculate the values myself. I didn't find the
> > full table in DP spec. I only find the table for 270MHz and 162MHz in
> > Table 2-50: Examples of Maud and Naud Values
>
> Table 2-104 in DP 1.4. Maybe you're looking at an older version of the
> spec.
So it looks like they used the same M value for all link frequencies
in that example, which for 540M ends up being double what the minimal
accurate value is. But as both M and N are doubled the ratio is still
exactly the same.
And there is indeed a 64k and 128k bitrates added as well. Those are
not classified under HBR audio. But as those can't be expressed via
short audio descriptors in the EDID, I don't think they would be
accepted by ALSA. Or am I wrong? I do wonder how they are supposed to be
used though since there must be some way for the sink to advertise
them, otherwise I can't see the point in adding them.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RESEND 9/9] drm/i915: set proper N/M in modeset
2016-10-20 11:34 ` Jani Nikula
2016-10-20 12:02 ` Ville Syrjälä
@ 2016-10-21 6:21 ` Yang, Libin
1 sibling, 0 replies; 29+ messages in thread
From: Yang, Libin @ 2016-10-21 6:21 UTC (permalink / raw)
To: Nikula, Jani, intel-gfx
Cc: Zhang, Keqiao, libin.yang, Pandiyan, Dhinakaran, Zhao, Juan J
Hi Jani,
> -----Original Message-----
> From: Nikula, Jani
> Sent: Thursday, October 20, 2016 7:35 PM
> To: Yang, Libin <libin.yang@intel.com>; Lin, Mengdong
> <mengdong.lin@intel.com>; intel-gfx@lists.freedesktop.org
> Cc: libin.yang@linux.intel.com; Pandiyan, Dhinakaran
> <dhinakaran.pandiyan@intel.com>; Zhang, Keqiao
> <keqiao.zhang@intel.com>; Zhao, Juan J <juan.j.zhao@intel.com>
> Subject: RE: [Intel-gfx] [PATCH RESEND 9/9] drm/i915: set proper N/M in
> modeset
>
> On Thu, 20 Oct 2016, "Yang, Libin" <libin.yang@intel.com> wrote:
> >> >> Any particular reason these M/N values are half of what they're in
> >> >> table
> >> >> 2-104 of DP 1.4 spec? (Admittedly the table is an informative
> >> >> example.)
> >> >
> >> > For HDMI, we found only set N is enough. HW then can handle the
> >> remaining.
> >>
> >> I meant, the M and N values in this part of the dp_aud_n_m table are
> >> 1/2 of what they are in the DP spec table. Why?
> >
> > Which table are you meaning? I calculate the values myself. I didn't
> > find the full table in DP spec. I only find the table for 270MHz and
> > 162MHz in Table 2-50: Examples of Maud and Naud Values
>
> Table 2-104 in DP 1.4. Maybe you're looking at an older version of the spec.
I was referring DP1.2. I don't have DP 1.4 spec on hand. But suppose
both table should be OK?
Regards,
Libin
>
> BR,
> Jani.
>
>
> --
> Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RESEND 9/9] drm/i915: set proper N/M in modeset
2016-10-20 12:02 ` Ville Syrjälä
@ 2016-10-21 6:47 ` Yang, Libin
0 siblings, 0 replies; 29+ messages in thread
From: Yang, Libin @ 2016-10-21 6:47 UTC (permalink / raw)
To: Ville Syrjälä, Nikula, Jani
Cc: Zhang, Keqiao, intel-gfx, Zhao, Juan J, Pandiyan, Dhinakaran, libin.yang
> -----Original Message-----
> From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> Sent: Thursday, October 20, 2016 8:03 PM
> To: Nikula, Jani <jani.nikula@intel.com>
> Cc: Yang, Libin <libin.yang@intel.com>; Lin, Mengdong
> <mengdong.lin@intel.com>; intel-gfx@lists.freedesktop.org; Zhang, Keqiao
> <keqiao.zhang@intel.com>; libin.yang@linux.intel.com; Pandiyan,
> Dhinakaran <dhinakaran.pandiyan@intel.com>; Zhao, Juan J
> <juan.j.zhao@intel.com>
> Subject: Re: [Intel-gfx] [PATCH RESEND 9/9] drm/i915: set proper N/M in
> modeset
>
> On Thu, Oct 20, 2016 at 02:34:34PM +0300, Jani Nikula wrote:
> > On Thu, 20 Oct 2016, "Yang, Libin" <libin.yang@intel.com> wrote:
> > >> >> Any particular reason these M/N values are half of what they're
> > >> >> in table
> > >> >> 2-104 of DP 1.4 spec? (Admittedly the table is an informative
> > >> >> example.)
> > >> >
> > >> > For HDMI, we found only set N is enough. HW then can handle the
> > >> remaining.
> > >>
> > >> I meant, the M and N values in this part of the dp_aud_n_m table
> > >> are 1/2 of what they are in the DP spec table. Why?
> > >
> > > Which table are you meaning? I calculate the values myself. I didn't
> > > find the full table in DP spec. I only find the table for 270MHz and
> > > 162MHz in Table 2-50: Examples of Maud and Naud Values
> >
> > Table 2-104 in DP 1.4. Maybe you're looking at an older version of the
> > spec.
>
> So it looks like they used the same M value for all link frequencies in that
> example, which for 540M ends up being double what the minimal accurate
> value is. But as both M and N are doubled the ratio is still exactly the same.
There is a formula in DP 1.2 spec:
Maud/Naud = 512 * fs / f_LS_Clk
Suppose all the values meeting the formula is acceptable.
>
> And there is indeed a 64k and 128k bitrates added as well. Those are not
> classified under HBR audio. But as those can't be expressed via short audio
> descriptors in the EDID, I don't think they would be accepted by ALSA. Or am
> I wrong? I do wonder how they are supposed to be used though since there
> must be some way for the sink to advertise them, otherwise I can't see the
> point in adding them.
Oh, sorry, I was thinking 128k is HBR. I didn't see 64k and 128k description
in DP 1.2 spec.
Regards,
Libin
>
> --
> Ville Syrjälä
> Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2016-10-21 6:47 UTC | newest]
Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-10 15:03 [PATCH RESEND 0/9] drm/i915/audio: audio cleanups, 4k fixes Jani Nikula
2016-10-10 15:04 ` [PATCH RESEND 1/9] drm/i915/audio: abstract audio config update Jani Nikula
2016-10-11 1:59 ` Yang, Libin
2016-10-10 15:04 ` [PATCH RESEND 2/9] drm/i915/audio: port is going to be just fine, simplify checks Jani Nikula
2016-10-11 2:37 ` Yang, Libin
2016-10-10 15:04 ` [PATCH RESEND 3/9] drm/i915/audio: use the same code for updating audio config Jani Nikula
2016-10-11 5:25 ` Yang, Libin
2016-10-10 15:04 ` [PATCH RESEND 4/9] drm/i915/audio: split dp and hdmi audio config update Jani Nikula
2016-10-11 5:42 ` Yang, Libin
2016-10-10 15:04 ` [PATCH RESEND 5/9] drm/i915/audio: set proper N/MCTS on more platforms Jani Nikula
2016-10-10 15:04 ` [PATCH RESEND 6/9] drm/i915/audio: HDMI audio gets the TMDS clock by crtc_clock Jani Nikula
2016-10-10 15:04 ` [PATCH RESEND 7/9] drm/i915/audio: add register macros for audio config N value Jani Nikula
2016-10-11 5:52 ` Yang, Libin
2016-10-10 15:04 ` [PATCH RESEND 8/9] drm/i915/audio: rename N value getter to emphasize it's for hdmi Jani Nikula
2016-10-11 5:55 ` Yang, Libin
2016-10-11 14:25 ` Jani Nikula
2016-10-10 15:04 ` [PATCH RESEND 9/9] drm/i915: set proper N/M in modeset Jani Nikula
2016-10-12 2:45 ` Lin, Mengdong
2016-10-12 6:29 ` Yang, Libin
2016-10-19 15:08 ` Jani Nikula
2016-10-20 2:00 ` Yang, Libin
2016-10-20 8:33 ` Jani Nikula
2016-10-20 8:42 ` Yang, Libin
2016-10-20 11:34 ` Jani Nikula
2016-10-20 12:02 ` Ville Syrjälä
2016-10-21 6:47 ` Yang, Libin
2016-10-21 6:21 ` Yang, Libin
2016-10-12 6:41 ` Zhang, Keqiao
2016-10-10 18:19 ` ✗ Fi.CI.BAT: warning for drm/i915/audio: audio cleanups, 4k fixes (rev3) Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.