RE: [PATCH] drm/i915/audio: Fix audio time stamp programming for DP
> -Original Message-
> From: Borah, Chaitanya Kumar
> Sent: Tuesday, April 30, 2024 2:48 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Vehmanen, Kai ; Saarinen, Jani
> ; ville.syrj...@linux.intel.com;
> jani.nik...@linux.intel.com; Yang, Libin ; Shankar, Uma
>
> Subject: [PATCH] drm/i915/audio: Fix audio time stamp programming for DP
>
> Intel hardware is capable of programming the Maud/Naud SDPs on its own based
> on real-time clocks. While doing so, it takes care of any deviations from the
> theoretical values. Programming the registers explicitly with static values
> can
> interfere with this logic. Therefore, let the HW decide the Maud and Naud SDPs
> on it's own.
Based on internal discussions with hardware team and validation on various
platforms,
Maud/Naud programming can be dropped. Looks ok to me.
Reviewed-by: Uma Shankar
> Cc: sta...@vger.kernel.org # v5.17
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/8097
> Co-developed-by: Kai Vehmanen
> Signed-off-by: Kai Vehmanen
> Signed-off-by: Chaitanya Kumar Borah
> ---
> drivers/gpu/drm/i915/display/intel_audio.c | 113 ++---
> 1 file changed, 8 insertions(+), 105 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_audio.c
> b/drivers/gpu/drm/i915/display/intel_audio.c
> index 07e0c73204f3..ed81e1466c4b 100644
> --- a/drivers/gpu/drm/i915/display/intel_audio.c
> +++ b/drivers/gpu/drm/i915/display/intel_audio.c
> @@ -76,19 +76,6 @@ struct intel_audio_funcs {
> struct intel_crtc_state *crtc_state); };
>
> -/* DP N/M table */
> -#define LC_810M 81
> -#define LC_540M 54
> -#define LC_270M 27
> -#define LC_162M 162000
> -
> -struct dp_aud_n_m {
> - int sample_rate;
> - int clock;
> - u16 m;
> - u16 n;
> -};
> -
> struct hdmi_aud_ncts {
> int sample_rate;
> int clock;
> @@ -96,60 +83,6 @@ struct hdmi_aud_ncts {
> int cts;
> };
>
> -/* Values according to DP 1.4 Table 2-104 */ -static const struct dp_aud_n_m
> dp_aud_n_m[] = {
> - { 32000, LC_162M, 1024, 10125 },
> - { 44100, LC_162M, 784, 5625 },
> - { 48000, LC_162M, 512, 3375 },
> - { 64000, LC_162M, 2048, 10125 },
> - { 88200, LC_162M, 1568, 5625 },
> - { 96000, LC_162M, 1024, 3375 },
> - { 128000, LC_162M, 4096, 10125 },
> - { 176400, LC_162M, 3136, 5625 },
> - { 192000, LC_162M, 2048, 3375 },
> - { 32000, LC_270M, 1024, 16875 },
> - { 44100, LC_270M, 784, 9375 },
> - { 48000, LC_270M, 512, 5625 },
> - { 64000, LC_270M, 2048, 16875 },
> - { 88200, LC_270M, 1568, 9375 },
> - { 96000, LC_270M, 1024, 5625 },
> - { 128000, LC_270M, 4096, 16875 },
> - { 176400, LC_270M, 3136, 9375 },
> - { 192000, LC_270M, 2048, 5625 },
> - { 32000, LC_540M, 1024, 33750 },
> - { 44100, LC_540M, 784, 18750 },
> - { 48000, LC_540M, 512, 11250 },
> - { 64000, LC_540M, 2048, 33750 },
> - { 88200, LC_540M, 1568, 18750 },
> - { 96000, LC_540M, 1024, 11250 },
> - { 128000, LC_540M, 4096, 33750 },
> - { 176400, LC_540M, 3136, 18750 },
> - { 192000, LC_540M, 2048, 11250 },
> - { 32000, LC_810M, 1024, 50625 },
> - { 44100, LC_810M, 784, 28125 },
> - { 48000, LC_810M, 512, 16875 },
> - { 64000, LC_810M, 2048, 50625 },
> - { 88200, LC_810M, 1568, 28125 },
> - { 96000, LC_810M, 1024, 16875 },
> - { 128000, LC_810M, 4096, 50625 },
> - { 176400, LC_810M, 3136, 28125 },
> - { 192000, LC_810M, 2048, 16875 },
> -};
> -
> -static const struct dp_aud_n_m *
> -audio_config_dp_get_n_m(const struct intel_crtc_state *crtc_state, int rate)
> -{
> - int i;
> -
> - for (i = 0; i < ARRAY_SIZE(dp_aud_n_m); i++) {
> - if (rate == dp_aud_n_m[i].sample_rate &&
> - crtc_state->port_clock == dp_aud_n_m[i].clock)
> - return _aud_n_m[i];
> - }
> -
> - return NULL;
> -}
> -
> static const struct {
> int clock;
> u32 config;
> @@ -387,47 +320,17 @@ hsw_dp_audio_config_update(struct intel_encoder
> *encoder,
> const struct intel_crtc_state *crtc_state) {
> struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> - struct i915_audio_component *acomp = i915->display.audio.component;
> enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> - enum port port = encoder->port;
> - const struct dp_aud_n_m *nm;
> - int rate;
> - u32 tmp;
> -
> - rate = acomp ? acomp->aud_sample_rate[port] : 0;
> - nm = audio_config_dp_get_n_m(crtc_state, rate);
> - if (nm)
> - drm_dbg_kms(>drm, "using Maud %u, Naud %u\n", nm-
> >m,
> - nm->n);
> - else
> - drm_dbg_kms(>drm, "using automatic Maud, Naud\n");
> -
> - tmp = intel_de_read(i915, HSW_AUD_CFG(cpu_transcoder));
> - tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
> - tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
> -
RE: [PATCH] drm/i915/audio: Fix audio time stamp programming for DP
Hello all,
Please ignore this version. I accidentally sent a version I was experimenting
with. I will correct it in the next revision.
Sorry for the spam.
> -Original Message-
> From: Intel-gfx On Behalf Of
> Chaitanya Kumar Borah
> Sent: Tuesday, April 30, 2024 2:04 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Vehmanen, Kai ; Saarinen, Jani
> ; ville.syrj...@linux.intel.com;
> jani.nik...@linux.intel.com; Yang, Libin ; Shankar, Uma
>
> Subject: [PATCH] drm/i915/audio: Fix audio time stamp programming for DP
>
> Intel hardware is capable of programming the Maud/Naud SDPs on its own
> based on real-time clocks. While doing so, it takes care of any deviations
> from
> the theoretical values. Programming the registers explicitly with static
> values
> can interfere with this logic. Therefore, let the HW decide the Maud and Naud
> SDPs on it's own.
>
> Cc: sta...@vger.kernel.org # v5.17
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/8097
> Co-developed-by: Kai Vehmanen
> Signed-off-by: Kai Vehmanen
> Signed-off-by: Chaitanya Kumar Borah
> ---
> drivers/gpu/drm/i915/display/intel_audio.c | 111 ++---
> 1 file changed, 6 insertions(+), 105 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_audio.c
> b/drivers/gpu/drm/i915/display/intel_audio.c
> index 07e0c73204f3..12e2ba462077 100644
> --- a/drivers/gpu/drm/i915/display/intel_audio.c
> +++ b/drivers/gpu/drm/i915/display/intel_audio.c
> @@ -76,19 +76,6 @@ struct intel_audio_funcs {
> struct intel_crtc_state *crtc_state); };
>
> -/* DP N/M table */
> -#define LC_810M 81
> -#define LC_540M 54
> -#define LC_270M 27
> -#define LC_162M 162000
> -
> -struct dp_aud_n_m {
> - int sample_rate;
> - int clock;
> - u16 m;
> - u16 n;
> -};
> -
> struct hdmi_aud_ncts {
> int sample_rate;
> int clock;
> @@ -96,60 +83,6 @@ struct hdmi_aud_ncts {
> int cts;
> };
>
> -/* Values according to DP 1.4 Table 2-104 */ -static const struct dp_aud_n_m
> dp_aud_n_m[] = {
> - { 32000, LC_162M, 1024, 10125 },
> - { 44100, LC_162M, 784, 5625 },
> - { 48000, LC_162M, 512, 3375 },
> - { 64000, LC_162M, 2048, 10125 },
> - { 88200, LC_162M, 1568, 5625 },
> - { 96000, LC_162M, 1024, 3375 },
> - { 128000, LC_162M, 4096, 10125 },
> - { 176400, LC_162M, 3136, 5625 },
> - { 192000, LC_162M, 2048, 3375 },
> - { 32000, LC_270M, 1024, 16875 },
> - { 44100, LC_270M, 784, 9375 },
> - { 48000, LC_270M, 512, 5625 },
> - { 64000, LC_270M, 2048, 16875 },
> - { 88200, LC_270M, 1568, 9375 },
> - { 96000, LC_270M, 1024, 5625 },
> - { 128000, LC_270M, 4096, 16875 },
> - { 176400, LC_270M, 3136, 9375 },
> - { 192000, LC_270M, 2048, 5625 },
> - { 32000, LC_540M, 1024, 33750 },
> - { 44100, LC_540M, 784, 18750 },
> - { 48000, LC_540M, 512, 11250 },
> - { 64000, LC_540M, 2048, 33750 },
> - { 88200, LC_540M, 1568, 18750 },
> - { 96000, LC_540M, 1024, 11250 },
> - { 128000, LC_540M, 4096, 33750 },
> - { 176400, LC_540M, 3136, 18750 },
> - { 192000, LC_540M, 2048, 11250 },
> - { 32000, LC_810M, 1024, 50625 },
> - { 44100, LC_810M, 784, 28125 },
> - { 48000, LC_810M, 512, 16875 },
> - { 64000, LC_810M, 2048, 50625 },
> - { 88200, LC_810M, 1568, 28125 },
> - { 96000, LC_810M, 1024, 16875 },
> - { 128000, LC_810M, 4096, 50625 },
> - { 176400, LC_810M, 3136, 28125 },
> - { 192000, LC_810M, 2048, 16875 },
> -};
> -
> -static const struct dp_aud_n_m *
> -audio_config_dp_get_n_m(const struct intel_crtc_state *crtc_state, int rate)
> -
> {
> - int i;
> -
> - for (i = 0; i < ARRAY_SIZE(dp_aud_n_m); i++) {
> - if (rate == dp_aud_n_m[i].sample_rate &&
> - crtc_state->port_clock == dp_aud_n_m[i].clock)
> - return _aud_n_m[i];
> - }
> -
> - return NULL;
> -}
> -
> static const struct {
> int clock;
> u32 config;
> @@ -387,47 +320,15 @@ hsw_dp_audio_config_update(struct
> intel_encoder *encoder,
> const struct intel_crtc_state *crtc_state) {
> struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> - struct i915_audio_component *acomp = i915-
> >display.audio.component;
> enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> - enum port port = encoder->port;
> - const struct dp_aud_n_m *nm;
> - int rate;
> - u32 tmp;
>
> - rate = acomp ? acomp->aud_sample_rate[port] : 0;
> - nm = audio_config_dp_get_n_m(crtc_state, rate);
> - if (nm)
> - drm_dbg_kms(>drm, "using Maud %u, Naud %u\n",
> nm->m,
> - nm->n);
> - else
> - drm_dbg_kms(>drm, "using automatic Maud,
> Naud\n");
> -
> - tmp = intel_de_read(i915, HSW_AUD_CFG(cpu_transcoder));
> - tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
> - tmp &=
Re: [PATCH] drm/i915/audio: Fix audio time stamp programming for DP
On Thu, 18 Apr 2024, Chaitanya Kumar Borah
wrote:
> Intel hardware is capable of programming the Maud/Naud SDPs on its
> own based on real-time clocks. While doing so, it takes care
> of any deviations from the theoretical values. Programming the registers
> explicitly with static values can interfere with this logic. Therefore,
> let the HW decide the Maud and Naud SDPs on it's own.
Hmm. I thought this was added due to some platforms *not* being able to
do this for some DP link or audio rates.
BR,
Jani.
>
> Fixes: 6014ac122ed0 ("drm/i915/audio: set proper N/M in modeset")
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/8097
> Co-developed-by: Kai Vehmanen
> Signed-off-by: Kai Vehmanen
> Signed-off-by: Chaitanya Kumar Borah
> ---
> drivers/gpu/drm/i915/display/intel_audio.c | 113 ++---
> 1 file changed, 8 insertions(+), 105 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_audio.c
> b/drivers/gpu/drm/i915/display/intel_audio.c
> index 07e0c73204f3..ed81e1466c4b 100644
> --- a/drivers/gpu/drm/i915/display/intel_audio.c
> +++ b/drivers/gpu/drm/i915/display/intel_audio.c
> @@ -76,19 +76,6 @@ struct intel_audio_funcs {
> struct intel_crtc_state *crtc_state);
> };
>
> -/* DP N/M table */
> -#define LC_810M 81
> -#define LC_540M 54
> -#define LC_270M 27
> -#define LC_162M 162000
> -
> -struct dp_aud_n_m {
> - int sample_rate;
> - int clock;
> - u16 m;
> - u16 n;
> -};
> -
> struct hdmi_aud_ncts {
> int sample_rate;
> int clock;
> @@ -96,60 +83,6 @@ struct hdmi_aud_ncts {
> int cts;
> };
>
> -/* Values according to DP 1.4 Table 2-104 */
> -static const struct dp_aud_n_m dp_aud_n_m[] = {
> - { 32000, LC_162M, 1024, 10125 },
> - { 44100, LC_162M, 784, 5625 },
> - { 48000, LC_162M, 512, 3375 },
> - { 64000, LC_162M, 2048, 10125 },
> - { 88200, LC_162M, 1568, 5625 },
> - { 96000, LC_162M, 1024, 3375 },
> - { 128000, LC_162M, 4096, 10125 },
> - { 176400, LC_162M, 3136, 5625 },
> - { 192000, LC_162M, 2048, 3375 },
> - { 32000, LC_270M, 1024, 16875 },
> - { 44100, LC_270M, 784, 9375 },
> - { 48000, LC_270M, 512, 5625 },
> - { 64000, LC_270M, 2048, 16875 },
> - { 88200, LC_270M, 1568, 9375 },
> - { 96000, LC_270M, 1024, 5625 },
> - { 128000, LC_270M, 4096, 16875 },
> - { 176400, LC_270M, 3136, 9375 },
> - { 192000, LC_270M, 2048, 5625 },
> - { 32000, LC_540M, 1024, 33750 },
> - { 44100, LC_540M, 784, 18750 },
> - { 48000, LC_540M, 512, 11250 },
> - { 64000, LC_540M, 2048, 33750 },
> - { 88200, LC_540M, 1568, 18750 },
> - { 96000, LC_540M, 1024, 11250 },
> - { 128000, LC_540M, 4096, 33750 },
> - { 176400, LC_540M, 3136, 18750 },
> - { 192000, LC_540M, 2048, 11250 },
> - { 32000, LC_810M, 1024, 50625 },
> - { 44100, LC_810M, 784, 28125 },
> - { 48000, LC_810M, 512, 16875 },
> - { 64000, LC_810M, 2048, 50625 },
> - { 88200, LC_810M, 1568, 28125 },
> - { 96000, LC_810M, 1024, 16875 },
> - { 128000, LC_810M, 4096, 50625 },
> - { 176400, LC_810M, 3136, 28125 },
> - { 192000, LC_810M, 2048, 16875 },
> -};
> -
> -static const struct dp_aud_n_m *
> -audio_config_dp_get_n_m(const struct intel_crtc_state *crtc_state, int rate)
> -{
> - int i;
> -
> - for (i = 0; i < ARRAY_SIZE(dp_aud_n_m); i++) {
> - if (rate == dp_aud_n_m[i].sample_rate &&
> - crtc_state->port_clock == dp_aud_n_m[i].clock)
> - return _aud_n_m[i];
> - }
> -
> - return NULL;
> -}
> -
> static const struct {
> int clock;
> u32 config;
> @@ -387,47 +320,17 @@ hsw_dp_audio_config_update(struct intel_encoder
> *encoder,
> const struct intel_crtc_state *crtc_state)
> {
> struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> - struct i915_audio_component *acomp = i915->display.audio.component;
> enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> - enum port port = encoder->port;
> - const struct dp_aud_n_m *nm;
> - int rate;
> - u32 tmp;
> -
> - rate = acomp ? acomp->aud_sample_rate[port] : 0;
> - nm = audio_config_dp_get_n_m(crtc_state, rate);
> - if (nm)
> - drm_dbg_kms(>drm, "using Maud %u, Naud %u\n", nm->m,
> - nm->n);
> - else
> - drm_dbg_kms(>drm, "using automatic Maud, Naud\n");
> -
> - tmp = intel_de_read(i915, HSW_AUD_CFG(cpu_transcoder));
> - 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;
>
> - if (nm) {
> - tmp &= ~AUD_CONFIG_N_MASK;
> - tmp |= AUD_CONFIG_N(nm->n);
> - tmp |= AUD_CONFIG_N_PROG_ENABLE;
> - }
> -
> - intel_de_write(i915, HSW_AUD_CFG(cpu_transcoder),
