RE: [PATCH] drm/i915/audio: Fix audio time stamp programming for DP

2024-05-01 Thread Shankar, Uma



> -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_c

[PATCH] drm/i915/audio: Fix audio time stamp programming for DP

2024-04-30 Thread Chaitanya Kumar Borah
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 | 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_810M81
-#define LC_540M54
-#define LC_270M27
-#define LC_162M162000
-
-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), tmp);
-
-   tmp = intel_de_read(i915, HSW_AUD_M_CTS_ENABLE(cpu_transcoder));
-   tmp &= ~AUD_CONFIG_M_MASK;
-   tmp &= ~AUD_M_CTS_M_VALUE_INDEX;
-   tmp &= ~AUD_M_CTS_M_PROG_ENABLE;
-
-   if (nm) {
-   tmp |= nm->m;
-   tmp |= AUD_M_CTS_M_VALUE_INDEX;
-   

RE: [PATCH] drm/i915/audio: Fix audio time stamp programming for DP

2024-04-30 Thread Borah, Chaitanya Kumar
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->

[PATCH] drm/i915/audio: Fix audio time stamp programming for DP

2024-04-30 Thread Chaitanya Kumar Borah
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_810M81
-#define LC_540M54
-#define LC_270M27
-#define LC_162M162000
-
-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 &= ~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), tmp);
-
-   tmp = intel_de_read(i915, HSW_AUD_M_CTS_ENABLE(cpu_transcoder));
-   tmp &= ~AUD_CONFIG_M_MASK;
-   tmp &= ~AUD_M_CTS_M_VALUE_INDEX;
-   tmp &= ~AUD_M_CTS_M_PROG_ENABLE;
-
-   if (nm) {
-   tmp |= nm->m;
-   tmp |= AUD_M_CTS_M_VALUE_INDEX;
-   

Re: [PATCH] drm/i915/audio: Fix audio time stamp programming for DP

2024-04-18 Thread Jani Nikula
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), 

[PATCH] drm/i915/audio: Fix audio time stamp programming for DP

2024-04-18 Thread Chaitanya Kumar Borah
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.

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_810M81
-#define LC_540M54
-#define LC_270M27
-#define LC_162M162000
-
-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), tmp);
-
-   tmp = intel_de_read(i915, HSW_AUD_M_CTS_ENABLE(cpu_transcoder));
-   tmp &= ~AUD_CONFIG_M_MASK;
-   tmp &= ~AUD_M_CTS_M_VALUE_INDEX;
-   tmp &= ~AUD_M_CTS_M_PROG_ENABLE;
-
-   if (nm) {
-   tmp |= nm->m;
-   tmp |=