[Sound-open-firmware] [PATCH 1/5] DMIC: Bug fix for IPM value calculation for HW version 2
The IPM bit field should be set to the number of used stereo controllers. This patch fixes the issue that pdm[0] got counted twice.
Signed-off-by: Sathish K. Kuttan sathish.k.kuttan@intel.com --- src/drivers/dmic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/drivers/dmic.c b/src/drivers/dmic.c index 97ca8b2..0f55474 100644 --- a/src/drivers/dmic.c +++ b/src/drivers/dmic.c @@ -714,7 +714,7 @@ static inline void source_ipm_helper(int source[], int *ipm, int stereo[],
/* IPM bit field is set to count of active pdm controllers. */ *ipm = pdm[0]; - for (i = 0; i < dmic->num_pdm_active; i++) + for (i = 1; i < dmic->num_pdm_active; i++) *ipm += pdm[i]; } #endif
This aligns the HW versions 1 and 2 code and avoids a build error about unused variables.
Signed-off-by: Seppo Ingalsuo seppo.ingalsuo@linux.intel.com --- src/drivers/dmic.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/drivers/dmic.c b/src/drivers/dmic.c index 0f55474..70942a2 100644 --- a/src/drivers/dmic.c +++ b/src/drivers/dmic.c @@ -818,14 +818,14 @@ static int configure_registers(struct dai *dai, struct dmic_configuration *cfg, OUTCONTROL0_SIP(0) | OUTCONTROL0_FINIT(1) | OUTCONTROL0_FCI(0) | - OUTCONTROL0_BFTH(3) | + OUTCONTROL0_BFTH(bfth) | OUTCONTROL0_OF(of0) | OUTCONTROL0_NUMBER_OF_DECIMATORS(ipm) | OUTCONTROL0_IPM_SOURCE_1(source[0]) | OUTCONTROL0_IPM_SOURCE_2(source[1]) | OUTCONTROL0_IPM_SOURCE_3(source[2]) | OUTCONTROL0_IPM_SOURCE_4(source[3]) | - OUTCONTROL0_TH(3); + OUTCONTROL0_TH(th); dmic_write(dai, OUTCONTROL0, val); trace_value(val);
@@ -833,14 +833,14 @@ static int configure_registers(struct dai *dai, struct dmic_configuration *cfg, OUTCONTROL1_SIP(0) | OUTCONTROL1_FINIT(1) | OUTCONTROL1_FCI(0) | - OUTCONTROL1_BFTH(3) | + OUTCONTROL1_BFTH(bfth) | OUTCONTROL1_OF(of1) | OUTCONTROL1_NUMBER_OF_DECIMATORS(ipm) | OUTCONTROL1_IPM_SOURCE_1(source[0]) | OUTCONTROL1_IPM_SOURCE_2(source[1]) | OUTCONTROL1_IPM_SOURCE_3(source[2]) | OUTCONTROL1_IPM_SOURCE_4(source[3]) | - OUTCONTROL1_TH(3); + OUTCONTROL1_TH(th); dmic_write(dai, OUTCONTROL1, val); trace_value(val); #endif
Signed-off-by: Seppo Ingalsuo seppo.ingalsuo@linux.intel.com --- src/drivers/dmic.c | 4 ++-- src/include/sof/dmic.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/drivers/dmic.c b/src/drivers/dmic.c index 70942a2..cbc714f 100644 --- a/src/drivers/dmic.c +++ b/src/drivers/dmic.c @@ -820,7 +820,7 @@ static int configure_registers(struct dai *dai, struct dmic_configuration *cfg, OUTCONTROL0_FCI(0) | OUTCONTROL0_BFTH(bfth) | OUTCONTROL0_OF(of0) | - OUTCONTROL0_NUMBER_OF_DECIMATORS(ipm) | + OUTCONTROL0_IPM(ipm) | OUTCONTROL0_IPM_SOURCE_1(source[0]) | OUTCONTROL0_IPM_SOURCE_2(source[1]) | OUTCONTROL0_IPM_SOURCE_3(source[2]) | @@ -835,7 +835,7 @@ static int configure_registers(struct dai *dai, struct dmic_configuration *cfg, OUTCONTROL1_FCI(0) | OUTCONTROL1_BFTH(bfth) | OUTCONTROL1_OF(of1) | - OUTCONTROL1_NUMBER_OF_DECIMATORS(ipm) | + OUTCONTROL1_IPM(ipm) | OUTCONTROL1_IPM_SOURCE_1(source[0]) | OUTCONTROL1_IPM_SOURCE_2(source[1]) | OUTCONTROL1_IPM_SOURCE_3(source[2]) | diff --git a/src/include/sof/dmic.h b/src/include/sof/dmic.h index 290da9f..afdd0b5 100644 --- a/src/include/sof/dmic.h +++ b/src/include/sof/dmic.h @@ -160,7 +160,7 @@ #define OUTCONTROL0_FCI(x) SET_BIT(24, x) #define OUTCONTROL0_BFTH(x) SET_BITS(23, 20, x) #define OUTCONTROL0_OF(x) SET_BITS(19, 18, x) -#define OUTCONTROL0_NUMBER_OF_DECIMATORS(x) SET_BITS(17, 15, x) +#define OUTCONTROL0_IPM(x) SET_BITS(17, 15, x) #define OUTCONTROL0_IPM_SOURCE_1(x) SET_BITS(14, 13, x) #define OUTCONTROL0_IPM_SOURCE_2(x) SET_BITS(12, 11, x) #define OUTCONTROL0_IPM_SOURCE_3(x) SET_BITS(10, 9, x) @@ -178,7 +178,7 @@ #define OUTCONTROL1_FCI(x) SET_BIT(24, x) #define OUTCONTROL1_BFTH(x) SET_BITS(23, 20, x) #define OUTCONTROL1_OF(x) SET_BITS(19, 18, x) -#define OUTCONTROL1_NUMBER_OF_DECIMATORS(x) SET_BITS(17, 15, x) +#define OUTCONTROL1_IPM(x) SET_BITS(17, 15, x) #define OUTCONTROL1_IPM_SOURCE_1(x) SET_BITS(14, 13, x) #define OUTCONTROL1_IPM_SOURCE_2(x) SET_BITS(12, 11, x) #define OUTCONTROL1_IPM_SOURCE_3(x) SET_BITS(10, 9, x)
Depending on HW configuration for two PDM controllers or less the IPM bit field value calculation need to be done like for HW version 1. The new style IPM bit field value calculation is used for three or four PDM controllers.
Signed-off-by: Seppo Ingalsuo seppo.ingalsuo@linux.intel.com --- src/drivers/dmic.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/src/drivers/dmic.c b/src/drivers/dmic.c index cbc714f..9e6998c 100644 --- a/src/drivers/dmic.c +++ b/src/drivers/dmic.c @@ -628,9 +628,8 @@ static int select_mode(struct dmic_configuration *cfg, * HW versions. This helper function returns a suitable IPM bit field * value to use. */ -#if DMIC_HW_VERSION == 1
-static inline void ipm_helper(int *ipm, int stereo[], int swap[], +static inline void ipm_helper1(int *ipm, int stereo[], int swap[], struct sof_ipc_dai_dmic_params *dmic) { int pdm[DMIC_HW_CONTROLLERS] = {0}; @@ -672,9 +671,9 @@ static inline void ipm_helper(int *ipm, int stereo[], int swap[], *ipm = 2; }
-#elif DMIC_HW_VERSION == 2 +#if DMIC_HW_VERSION == 2
-static inline void source_ipm_helper(int source[], int *ipm, int stereo[], +static inline void ipm_helper2(int source[], int *ipm, int stereo[], int swap[], struct sof_ipc_dai_dmic_params *dmic) { int pdm[DMIC_HW_CONTROLLERS]; @@ -788,7 +787,7 @@ static int configure_registers(struct dai *dai, struct dmic_configuration *cfg, of1 = (dmic->fifo_bits_b == 32) ? 2 : 0;
#if DMIC_HW_VERSION == 1 - ipm_helper(&ipm, stereo, swap, dmic); + ipm_helper1(&ipm, stereo, swap, dmic); val = OUTCONTROL0_TIE(0) | OUTCONTROL0_SIP(0) | OUTCONTROL0_FINIT(1) | @@ -813,7 +812,11 @@ static int configure_registers(struct dai *dai, struct dmic_configuration *cfg, #endif
#if DMIC_HW_VERSION == 2 - source_ipm_helper(source, &ipm, stereo, swap, dmic); +#if DMIC_HW_CONTROLLERS > 2 + ipm_helper2(source, &ipm, stereo, swap, dmic); +#else + ipm_helper1(&ipm, stereo, swap, dmic); +#endif val = OUTCONTROL0_TIE(0) | OUTCONTROL0_SIP(0) | OUTCONTROL0_FINIT(1) |
The loud DC thump sound plus the impulse like noises in the beginning of capture can be mitigated with the gain ramp and muted filters start. The gain ramp rate and CIC and FIR unmute delays parameters can be adjusted from dmic.h header file.
Note: The logarithmic ramp length is now hardwired to 300 us for the 1 ms update rate via macro LOGRAMP_GM in dmic.c. A larger value will shorten the ramp. It will be replaced later by topology passed ramp characteristic.
Signed-off-by: Seppo Ingalsuo seppo.ingalsuo@linux.intel.com --- src/drivers/dmic.c | 106 +++++++++++++++++++++++++++++++++++++++++++++++-- src/include/sof/dmic.h | 11 +++++ 2 files changed, 113 insertions(+), 4 deletions(-)
diff --git a/src/drivers/dmic.c b/src/drivers/dmic.c index 9e6998c..58d5e20 100644 --- a/src/drivers/dmic.c +++ b/src/drivers/dmic.c @@ -119,6 +119,19 @@ struct pdm_controllers_configuration { /* Internal precision in gains computation, e.g. Q4.28 in int32_t */ #define DMIC_FIR_SCALE_Q 28
+/* Used in unmute ramp values calculation */ +#define DMIC_HW_FIR_GAIN_MAX ((1 << (DMIC_HW_BITS_FIR_GAIN - 1)) - 1) + +/* Hardwired log ramp parameters. The first value is the initial logarithmic + * gain and the second value is the multiplier for gain to achieve the linear + * decibels change over time. Currently the coefficient GM needs to be + * calculated manually. The 300 ms ramp should ensure clean sounding start with + * any microphone. However it is likely unnecessarily long for machine hearing. + * TODO: Add ramp characteristic passing via topology. + */ +#define LOGRAMP_GI 33954 /* -90 dB, Q2.30*/ +#define LOGRAMP_GM 16959 /* Gives 300 ms ramp for -90..0 dB, Q2.14 */ + /* tracing */ #define trace_dmic(__e) trace_event(TRACE_CLASS_DMIC, __e) #define trace_dmic_error(__e) trace_error(TRACE_CLASS_DMIC, __e) @@ -184,6 +197,74 @@ static void dmic_update_bits(struct dai *dai, uint32_t reg, uint32_t mask, } #endif
+/* this ramps volume changes over time */ +static uint64_t dmic_work(void *data, uint64_t delay) +{ + struct dai *dai = (struct dai *)data; + struct dmic_pdata *dmic = dai_get_drvdata(dai); + int32_t gval; + uint32_t val; + int i; + + tracev_dmic("wrk"); + spin_lock(&dmic->lock); + + /* Increment gain with logaritmic step. + * Gain is Q2.30 and gain modifier is Q2.14. + */ + dmic->startcount++; + dmic->gain = Q_MULTSR_32X32((int64_t)dmic->gain, + LOGRAMP_GM, 30, 14, 30); + + /* Gain is stored as Q2.30, while HW register is Q1.19 so shift + * the value right by 11. + */ + gval = dmic->gain >> 11; + + /* Note that DMIC gain value zero has a special purpose. Value zero + * sets gain bypass mode in HW. Zero value will be applied after ramp + * is complete. It is because exact 1.0 gain is not possible with Q1.19. + */ + if (gval > DMIC_HW_FIR_GAIN_MAX) + gval = 0; + + /* Write gain to registers */ + for (i = 0; i < DMIC_HW_CONTROLLERS; i++) { + if (!dmic->enable[i]) + continue; + + if (dmic->startcount == DMIC_UNMUTE_CIC) + dmic_update_bits(dai, base[i] + CIC_CONTROL, + CIC_CONTROL_MIC_MUTE_BIT, 0); + + if (dmic->startcount == DMIC_UNMUTE_FIR) { + if (dmic->fifo_a) + dmic_update_bits(dai, base[i] + FIR_CONTROL_A, + FIR_CONTROL_A_MUTE_BIT, 0); + + if (dmic->fifo_b) + dmic_update_bits(dai, base[i] + FIR_CONTROL_B, + FIR_CONTROL_B_MUTE_BIT, 0); + } + if (dmic->fifo_a) { + val = OUT_GAIN_LEFT_A_GAIN(gval); + dmic_write(dai, base[i] + OUT_GAIN_LEFT_A, val); + dmic_write(dai, base[i] + OUT_GAIN_RIGHT_A, val); + } + if (dmic->fifo_b) { + val = OUT_GAIN_LEFT_B_GAIN(gval); + dmic_write(dai, base[i] + OUT_GAIN_LEFT_B, val); + dmic_write(dai, base[i] + OUT_GAIN_RIGHT_B, val); + } + } + spin_unlock(&dmic->lock); + + if (gval) + return DMIC_UNMUTE_RAMP_US; + else + return 0; +} + /* This function returns a raw list of potential microphone clock and decimation * modes for achieving requested sample rates. The search is constrained by * decimation HW capabililies and setup parameters. The parameters such as @@ -745,8 +826,8 @@ static int configure_registers(struct dai *dai, struct dmic_configuration *cfg, struct dmic_pdata *pdata = dai_get_drvdata(dai); int array_a = 0; int array_b = 0; - int cic_mute = 0; - int fir_mute = 0; + int cic_mute = 1; + int fir_mute = 1; int bfth = 1; /* Should be 3 for 8 entries, 1 is 2 entries */ int th = 0; /* Used with TIE=1 */
@@ -1026,6 +1107,9 @@ static int dmic_set_config(struct dai *dai, struct sof_ipc_dai_config *config)
trace_dmic("dsc");
+ /* Initialize start sequence handler */ + work_init(&dmic->dmicwork, dmic_work, dai, WORK_ASYNC); + /* * "config" might contain pdm controller params for only * the active controllers @@ -1139,6 +1223,8 @@ static void dmic_start(struct dai *dai) spin_lock(&dmic->lock); trace_dmic("sta"); dmic->state = COMP_STATE_ACTIVE; + dmic->startcount = 0; + dmic->gain = LOGRAMP_GI; /* Initial gain value */
if (dmic->fifo_a) { trace_dmic("ffa"); @@ -1210,6 +1296,9 @@ static void dmic_start(struct dai *dai) * start of audio capture would contain clicks and/or noise and it * is not suppressed by gain ramp somewhere in the capture pipe. */ + + work_schedule_default(&dmic->dmicwork, DMIC_UNMUTE_RAMP_US); + trace_dmic("run"); } /* stop the DMIC for capture */ @@ -1230,12 +1319,21 @@ static void dmic_stop(struct dai *dai) OUTCONTROL1_SIP_BIT | OUTCONTROL1_FINIT_BIT, OUTCONTROL1_FINIT_BIT);
- /* Set soft reset for all PDM controllers. + /* Set soft reset and mute on for all PDM controllers. */ trace_dmic("sre"); for (i = 0; i < DMIC_HW_CONTROLLERS; i++) { dmic_update_bits(dai, base[i] + CIC_CONTROL, - CIC_CONTROL_SOFT_RESET_BIT, CIC_CONTROL_SOFT_RESET_BIT); + CIC_CONTROL_SOFT_RESET_BIT | + CIC_CONTROL_MIC_MUTE_BIT, + CIC_CONTROL_SOFT_RESET_BIT | + CIC_CONTROL_MIC_MUTE_BIT); + dmic_update_bits(dai, base[i] + FIR_CONTROL_A, + FIR_CONTROL_A_MUTE_BIT, + FIR_CONTROL_A_MUTE_BIT); + dmic_update_bits(dai, base[i] + FIR_CONTROL_B, + FIR_CONTROL_B_MUTE_BIT, + FIR_CONTROL_B_MUTE_BIT); }
spin_unlock(&dmic->lock); diff --git a/src/include/sof/dmic.h b/src/include/sof/dmic.h index afdd0b5..4cd0fab 100644 --- a/src/include/sof/dmic.h +++ b/src/include/sof/dmic.h @@ -37,6 +37,14 @@
#if defined CONFIG_DMIC
+/* The microphones create a low frequecy thump sound when clock is enabled. + * The unmute linear gain ramp chacteristic is defined here. + * NOTE: Do not set any of these to 0. + */ +#define DMIC_UNMUTE_RAMP_US 1000 /* 1 ms (in microseconds) */ +#define DMIC_UNMUTE_CIC 1 /* Unmute CIC at 1 ms */ +#define DMIC_UNMUTE_FIR 2 /* Unmute FIR at 2 ms */ + #if defined CONFIG_APOLLOLAKE #define DMIC_HW_VERSION 1 #define DMIC_HW_CONTROLLERS 2 @@ -311,6 +319,9 @@ struct dmic_pdata { completion_t drain_complete; struct sof_ipc_dai_config config; struct sof_ipc_dai_dmic_params params; + struct work dmicwork; + int32_t startcount; + int32_t gain; };
extern const struct dai_ops dmic_ops;
On Tue, 2018-06-19 at 18:14 +0300, Seppo Ingalsuo wrote:
The IPM bit field should be set to the number of used stereo controllers. This patch fixes the issue that pdm[0] got counted twice.
Signed-off-by: Sathish K. Kuttan sathish.k.kuttan@intel.com
src/drivers/dmic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/drivers/dmic.c b/src/drivers/dmic.c index 97ca8b2..0f55474 100644 --- a/src/drivers/dmic.c +++ b/src/drivers/dmic.c @@ -714,7 +714,7 @@ static inline void source_ipm_helper(int source[], int *ipm, int stereo[],
/* IPM bit field is set to count of active pdm controllers. */ *ipm = pdm[0];
- for (i = 0; i < dmic->num_pdm_active; i++)
- for (i = 1; i < dmic->num_pdm_active; i++) *ipm += pdm[i];
} #endif
All applied.
Thanks
Liam
participants (2)
-
Liam Girdwood
-
Seppo Ingalsuo