[Sound-open-firmware] [PATCH] DMIC: configure DMIC with parameters from ipc instead of hardcoded params
This patch makes the following changes to configure DMIC from ipc params:
1. remove redundant hdr member item from struct sof_ipc_dai_dmic_params 2. Rename number_of_pdm_controllers member in the above structure to num_pdm_active to be more representative of the active pdm count. 3. Add an "id" member to struct sof_ipc_dai_dmic_pdm_ctrl 4. Remove hardcoded config params from DMIC set_config function and use ipc params instead.
Signed-off-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com ---
Notes: Tested with:
APL based Up squared board:
SOF Kernel: https://github.com/ranj063/sound branch: topic/dmic SOF: master SOFT: https://github.com/ranj063/soft.git branch: topic/dmic
src/drivers/dmic.c | 97 +++++++++++++++++++++--------------------- src/include/uapi/ipc.h | 4 +- 2 files changed, 50 insertions(+), 51 deletions(-)
diff --git a/src/drivers/dmic.c b/src/drivers/dmic.c index 5c8591c..b66769c 100644 --- a/src/drivers/dmic.c +++ b/src/drivers/dmic.c @@ -49,11 +49,6 @@ */ #define DMIC_FIR_PIPELINE_OVERHEAD 5
-/* Force a 48 kHz config while kernel driver IPC is under construction. Remove - * or undefine when it's available. - */ -#define DMIC_FORCE_CONFIG - struct decim_modes { int16_t clkdiv[DMIC_MAX_MODES]; int16_t mcic[DMIC_MAX_MODES]; @@ -648,7 +643,7 @@ static inline void ipm_helper(int *ipm, int stereo[], int swap[], * left (A) or mono right (B) mode. Mono right mode is setup as channel * swapped mono left. */ - for (i = 0; i < dmic->number_of_pdm_controllers; i++) { + for (i = 0; i < dmic->num_pdm_active; i++) { cnt = 0; if (dmic->pdm[i].enable_mic_a > 0) cnt++; @@ -693,7 +688,7 @@ static inline void source_ipm_helper(int source[], int *ipm, int stereo[], * swapped mono left. The function returns also in array source[] the * indice of enabled pdm controllers to be used for IPM configuration. */ - for (i = 0; i < dmic->number_of_pdm_controllers; i++) { + for (i = 0; i < dmic->num_pdm_active; i++) { if (dmic->pdm[i].enable_mic_a > 0 || dmic->pdm[i].enable_mic_b > 0) { pdm[i] = 1; @@ -719,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->number_of_pdm_controllers; i++) + for (i = 0; i < dmic->num_pdm_active; i++) *ipm += pdm[i]; } #endif @@ -782,7 +777,7 @@ static int configure_registers(struct dai *dai, struct dmic_configuration *cfg, }
/* Sanity checks */ - if (dmic->number_of_pdm_controllers > DMIC_HW_CONTROLLERS) { + if (dmic->num_pdm_active > DMIC_HW_CONTROLLERS) { trace_dmic_error("num"); return -EINVAL; } @@ -1016,45 +1011,40 @@ static int configure_registers(struct dai *dai, struct dmic_configuration *cfg,
static int dmic_set_config(struct dai *dai, struct sof_ipc_dai_config *config) { - struct decim_modes modes_a; - struct decim_modes modes_b; + struct dmic_pdata *dmic = dai_get_drvdata(dai); + struct sof_ipc_dai_dmic_params *prm; struct matched_modes modes_ab; struct dmic_configuration cfg; - int ret; - struct sof_ipc_dai_dmic_params *prm = &config->dmic; - struct dmic_pdata *dmic = dai_get_drvdata(dai); + struct decim_modes modes_a; + struct decim_modes modes_b; + int num_pdm_active = config->dmic.num_pdm_active; + size_t size; + int i, j, ret = 0;
trace_dmic("dsc");
-#if defined DMIC_FORCE_CONFIG - /* This is a temporary workaound to set parameters while - * there is no driver and topology scripts support to - * set these. Also the PCM sample format(s) and sample rate(s) - * setting would be better to be common with other DAI types. + /* + * "config" might contain pdm controller params for only + * the active controllers + * "prm" is initialized with default params for all HW controllers */ - prm->driver_ipc_version = 1; - prm->pdmclk_min = 500000; /* Min 500 kHz */ - prm->pdmclk_max = 4800000; /* Max 4.80 MHz */ - prm->fifo_fs_a = 48000; - prm->fifo_fs_b = 0; - prm->fifo_bits_a = 32; - prm->fifo_bits_b = 32; - prm->duty_min = 40; /* Min. 40% */ - prm->duty_max = 60; /* Max. 60% */ - prm->number_of_pdm_controllers = 2; - prm->pdm[0].clk_edge = 0; - prm->pdm[0].enable_mic_a = 1; /* Left */ - prm->pdm[0].enable_mic_b = 1; /* Right */ - prm->pdm[0].polarity_mic_a = 0; - prm->pdm[0].polarity_mic_b = 0; - prm->pdm[0].skew = 0; - prm->pdm[1].clk_edge = 0; - prm->pdm[1].enable_mic_a = 0; /* Left */ - prm->pdm[1].enable_mic_b = 0; /* Right */ - prm->pdm[1].polarity_mic_a = 0; - prm->pdm[1].polarity_mic_b = 0; - prm->pdm[1].skew = 0; -#endif + size = sizeof(*prm) + DMIC_HW_CONTROLLERS + * sizeof(struct sof_ipc_dai_dmic_pdm_ctrl); + prm = rzalloc(RZONE_SYS, SOF_MEM_CAPS_RAM, size); + + /* copy the command DAI config params and DMIC params */ + memcpy(prm, &config->dmic, sizeof(struct sof_ipc_dai_config)); + + /* copy the pdm controller params from ipc */ + for (i = 0; i < DMIC_HW_CONTROLLERS; i++) { + prm->pdm[i].id = i; + for (j = 0; j < num_pdm_active; j++) { + /* copy the pdm controller params id the id's match */ + if (prm->pdm[i].id == config->dmic.pdm[j].id) + memcpy(&prm->pdm[i], &config->dmic.pdm[j], + sizeof(prm->pdm[i])); + } + }
trace_value(prm->driver_ipc_version); trace_value(prm->pdmclk_min); @@ -1065,11 +1055,12 @@ static int dmic_set_config(struct dai *dai, struct sof_ipc_dai_config *config) trace_value(prm->fifo_bits_b); trace_value(prm->duty_min); trace_value(prm->duty_max); - trace_value(prm->number_of_pdm_controllers); + trace_value(prm->num_pdm_active);
if (prm->driver_ipc_version != DMIC_IPC_VERSION) { trace_dmic_error("ver"); - return -EINVAL; + ret = -EINVAL; + goto finish; }
/* Match and select optimal decimators configuration for FIFOs A and B @@ -1081,20 +1072,23 @@ static int dmic_set_config(struct dai *dai, struct sof_ipc_dai_config *config) find_modes(&modes_a, prm, prm->fifo_fs_a); if (modes_a.num_of_modes == 0 && prm->fifo_fs_a > 0) { trace_dmic_error("amo"); - return -EINVAL; + ret = -EINVAL; + goto finish; }
find_modes(&modes_b, prm, prm->fifo_fs_b); if (modes_b.num_of_modes == 0 && prm->fifo_fs_b > 0) { trace_dmic_error("bmo"); - return -EINVAL; + ret = -EINVAL; + goto finish; }
match_modes(&modes_ab, &modes_a, &modes_b); ret = select_mode(&cfg, &modes_ab); if (ret < 0) { trace_dmic_error("smo"); - return -EINVAL; + ret = -EINVAL; + goto finish; }
trace_dmic("cfg"); @@ -1115,12 +1109,17 @@ static int dmic_set_config(struct dai *dai, struct sof_ipc_dai_config *config) ret = configure_registers(dai, &cfg, prm); if (ret < 0) { trace_dmic_error("cor"); - return -EINVAL; + ret = -EINVAL; + goto finish; }
dmic->state = COMP_STATE_PREPARE;
- return 0; +finish: + /* free config params */ + rfree(prm); + + return ret; }
/* start the DMIC for capture */ diff --git a/src/include/uapi/ipc.h b/src/include/uapi/ipc.h index b32376d..9b1063f 100644 --- a/src/include/uapi/ipc.h +++ b/src/include/uapi/ipc.h @@ -292,6 +292,7 @@ struct sof_ipc_dai_hda_params { * data integrity problems. */ struct sof_ipc_dai_dmic_pdm_ctrl { + uint16_t id; /* PDM controller ID */ uint16_t enable_mic_a; /* Use A (left) channel mic (0 or 1)*/ uint16_t enable_mic_b; /* Use B (right) channel mic (0 or 1)*/ uint16_t polarity_mic_a; /* Optionally invert mic A signal (0 or 1) */ @@ -322,7 +323,6 @@ struct sof_ipc_dai_dmic_pdm_ctrl { * somewhat. */ struct sof_ipc_dai_dmic_params { - struct sof_ipc_hdr hdr; uint32_t driver_ipc_version; /* Version (1..N) */ uint32_t pdmclk_min; /* Minimum microphone clock in Hz (100000..N) */ uint32_t pdmclk_max; /* Maximum microphone clock in Hz (min...N) */ @@ -333,7 +333,7 @@ struct sof_ipc_dai_dmic_params { uint16_t fifo_bits_b; /* FIFO B word length (16 or 24) */ uint16_t duty_min; /* Min. mic clock duty cycle in % (20..80) */ uint16_t duty_max; /* Max. mic clock duty cycle in % (min..80) */ - uint32_t number_of_pdm_controllers; /* Number 2ch controllers (2) */ + uint32_t num_pdm_active; /* Number of active controllers */ struct sof_ipc_dai_dmic_pdm_ctrl pdm[]; } __attribute__((packed));
a gentle reminder about this one so it doesnt get forgotten.
Seppo/Liam, do you have any comments about this one?
On Sat, 2018-05-26 at 23:22 -0700, Ranjani Sridharan wrote:
This patch makes the following changes to configure DMIC from ipc params:
- remove redundant hdr member item from struct
sof_ipc_dai_dmic_params 2. Rename number_of_pdm_controllers member in the above structure to num_pdm_active to be more representative of the active pdm count. 3. Add an "id" member to struct sof_ipc_dai_dmic_pdm_ctrl 4. Remove hardcoded config params from DMIC set_config function and use ipc params instead.
Signed-off-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com
Notes: Tested with:
APL based Up squared board: SOF Kernel: https://github.com/ranj063/sound branch: topic/dmic SOF: master SOFT: https://github.com/ranj063/soft.git branch: topic/dmic
src/drivers/dmic.c | 97 +++++++++++++++++++++-------------------
src/include/uapi/ipc.h | 4 +- 2 files changed, 50 insertions(+), 51 deletions(-)
diff --git a/src/drivers/dmic.c b/src/drivers/dmic.c index 5c8591c..b66769c 100644 --- a/src/drivers/dmic.c +++ b/src/drivers/dmic.c @@ -49,11 +49,6 @@ */ #define DMIC_FIR_PIPELINE_OVERHEAD 5
-/* Force a 48 kHz config while kernel driver IPC is under construction. Remove
- or undefine when it's available.
- */
-#define DMIC_FORCE_CONFIG
struct decim_modes { int16_t clkdiv[DMIC_MAX_MODES]; int16_t mcic[DMIC_MAX_MODES]; @@ -648,7 +643,7 @@ static inline void ipm_helper(int *ipm, int stereo[], int swap[], * left (A) or mono right (B) mode. Mono right mode is setup as channel * swapped mono left. */
- for (i = 0; i < dmic->number_of_pdm_controllers; i++) {
- for (i = 0; i < dmic->num_pdm_active; i++) { cnt = 0; if (dmic->pdm[i].enable_mic_a > 0) cnt++;
@@ -693,7 +688,7 @@ static inline void source_ipm_helper(int source[], int *ipm, int stereo[], * swapped mono left. The function returns also in array source[] the * indice of enabled pdm controllers to be used for IPM configuration. */
- for (i = 0; i < dmic->number_of_pdm_controllers; i++) {
- for (i = 0; i < dmic->num_pdm_active; i++) { if (dmic->pdm[i].enable_mic_a > 0 || dmic->pdm[i].enable_mic_b > 0) { pdm[i] = 1;
@@ -719,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->number_of_pdm_controllers; i++)
- for (i = 0; i < dmic->num_pdm_active; i++) *ipm += pdm[i];
} #endif @@ -782,7 +777,7 @@ static int configure_registers(struct dai *dai, struct dmic_configuration *cfg, }
/* Sanity checks */
- if (dmic->number_of_pdm_controllers > DMIC_HW_CONTROLLERS) {
- if (dmic->num_pdm_active > DMIC_HW_CONTROLLERS) { trace_dmic_error("num"); return -EINVAL; }
@@ -1016,45 +1011,40 @@ static int configure_registers(struct dai *dai, struct dmic_configuration *cfg,
static int dmic_set_config(struct dai *dai, struct sof_ipc_dai_config *config) {
- struct decim_modes modes_a;
- struct decim_modes modes_b;
- struct dmic_pdata *dmic = dai_get_drvdata(dai);
- struct sof_ipc_dai_dmic_params *prm; struct matched_modes modes_ab; struct dmic_configuration cfg;
- int ret;
- struct sof_ipc_dai_dmic_params *prm = &config->dmic;
- struct dmic_pdata *dmic = dai_get_drvdata(dai);
struct decim_modes modes_a;
struct decim_modes modes_b;
int num_pdm_active = config->dmic.num_pdm_active;
size_t size;
int i, j, ret = 0;
trace_dmic("dsc");
-#if defined DMIC_FORCE_CONFIG
- /* This is a temporary workaound to set parameters while
* there is no driver and topology scripts support to
* set these. Also the PCM sample format(s) and sample
rate(s)
* setting would be better to be common with other DAI
types.
- /*
* "config" might contain pdm controller params for only
* the active controllers
* "prm" is initialized with default params for all HW
controllers */
- prm->driver_ipc_version = 1;
- prm->pdmclk_min = 500000; /* Min 500 kHz */
- prm->pdmclk_max = 4800000; /* Max 4.80 MHz */
- prm->fifo_fs_a = 48000;
- prm->fifo_fs_b = 0;
- prm->fifo_bits_a = 32;
- prm->fifo_bits_b = 32;
- prm->duty_min = 40; /* Min. 40% */
- prm->duty_max = 60; /* Max. 60% */
- prm->number_of_pdm_controllers = 2;
- prm->pdm[0].clk_edge = 0;
- prm->pdm[0].enable_mic_a = 1; /* Left */
- prm->pdm[0].enable_mic_b = 1; /* Right */
- prm->pdm[0].polarity_mic_a = 0;
- prm->pdm[0].polarity_mic_b = 0;
- prm->pdm[0].skew = 0;
- prm->pdm[1].clk_edge = 0;
- prm->pdm[1].enable_mic_a = 0; /* Left */
- prm->pdm[1].enable_mic_b = 0; /* Right */
- prm->pdm[1].polarity_mic_a = 0;
- prm->pdm[1].polarity_mic_b = 0;
- prm->pdm[1].skew = 0;
-#endif
- size = sizeof(*prm) + DMIC_HW_CONTROLLERS
* sizeof(struct sof_ipc_dai_dmic_pdm_ctrl);
- prm = rzalloc(RZONE_SYS, SOF_MEM_CAPS_RAM, size);
- /* copy the command DAI config params and DMIC params */
- memcpy(prm, &config->dmic, sizeof(struct
sof_ipc_dai_config));
- /* copy the pdm controller params from ipc */
- for (i = 0; i < DMIC_HW_CONTROLLERS; i++) {
prm->pdm[i].id = i;
for (j = 0; j < num_pdm_active; j++) {
/* copy the pdm controller params id the
id's match */
if (prm->pdm[i].id == config-
dmic.pdm[j].id)
memcpy(&prm->pdm[i], &config-
dmic.pdm[j],
sizeof(prm->pdm[i]));
}
}
trace_value(prm->driver_ipc_version); trace_value(prm->pdmclk_min);
@@ -1065,11 +1055,12 @@ static int dmic_set_config(struct dai *dai, struct sof_ipc_dai_config *config) trace_value(prm->fifo_bits_b); trace_value(prm->duty_min); trace_value(prm->duty_max);
- trace_value(prm->number_of_pdm_controllers);
trace_value(prm->num_pdm_active);
if (prm->driver_ipc_version != DMIC_IPC_VERSION) { trace_dmic_error("ver");
return -EINVAL;
ret = -EINVAL;
goto finish;
}
/* Match and select optimal decimators configuration for
FIFOs A and B @@ -1081,20 +1072,23 @@ static int dmic_set_config(struct dai *dai, struct sof_ipc_dai_config *config) find_modes(&modes_a, prm, prm->fifo_fs_a); if (modes_a.num_of_modes == 0 && prm->fifo_fs_a > 0) { trace_dmic_error("amo");
return -EINVAL;
ret = -EINVAL;
goto finish;
}
find_modes(&modes_b, prm, prm->fifo_fs_b); if (modes_b.num_of_modes == 0 && prm->fifo_fs_b > 0) { trace_dmic_error("bmo");
return -EINVAL;
ret = -EINVAL;
goto finish;
}
match_modes(&modes_ab, &modes_a, &modes_b); ret = select_mode(&cfg, &modes_ab); if (ret < 0) { trace_dmic_error("smo");
return -EINVAL;
ret = -EINVAL;
goto finish;
}
trace_dmic("cfg");
@@ -1115,12 +1109,17 @@ static int dmic_set_config(struct dai *dai, struct sof_ipc_dai_config *config) ret = configure_registers(dai, &cfg, prm); if (ret < 0) { trace_dmic_error("cor");
return -EINVAL;
ret = -EINVAL;
goto finish;
}
dmic->state = COMP_STATE_PREPARE;
- return 0;
+finish:
- /* free config params */
- rfree(prm);
- return ret;
}
/* start the DMIC for capture */ diff --git a/src/include/uapi/ipc.h b/src/include/uapi/ipc.h index b32376d..9b1063f 100644 --- a/src/include/uapi/ipc.h +++ b/src/include/uapi/ipc.h @@ -292,6 +292,7 @@ struct sof_ipc_dai_hda_params {
- data integrity problems.
*/ struct sof_ipc_dai_dmic_pdm_ctrl {
- uint16_t id; /* PDM controller ID */ uint16_t enable_mic_a; /* Use A (left) channel mic (0 or
1)*/ uint16_t enable_mic_b; /* Use B (right) channel mic (0 or 1)*/ uint16_t polarity_mic_a; /* Optionally invert mic A signal (0 or 1) */ @@ -322,7 +323,6 @@ struct sof_ipc_dai_dmic_pdm_ctrl {
- somewhat.
*/ struct sof_ipc_dai_dmic_params {
- struct sof_ipc_hdr hdr; uint32_t driver_ipc_version; /* Version (1..N) */ uint32_t pdmclk_min; /* Minimum microphone clock in Hz
(100000..N) */ uint32_t pdmclk_max; /* Maximum microphone clock in Hz (min...N) */ @@ -333,7 +333,7 @@ struct sof_ipc_dai_dmic_params { uint16_t fifo_bits_b; /* FIFO B word length (16 or 24) */ uint16_t duty_min; /* Min. mic clock duty cycle in % (20..80) */ uint16_t duty_max; /* Max. mic clock duty cycle in % (min..80) */
- uint32_t number_of_pdm_controllers; /* Number 2ch
controllers (2) */
- uint32_t num_pdm_active; /* Number of active controllers */ struct sof_ipc_dai_dmic_pdm_ctrl pdm[];
} __attribute__((packed));
On Thu, 2018-05-31 at 10:46 -0700, Ranjani Sridharan wrote:
a gentle reminder about this one so it doesnt get forgotten.
Please don't top post :)
Seppo/Liam, do you have any comments about this one?
I'm good if Seppo is ?
On Sat, 2018-05-26 at 23:22 -0700, Ranjani Sridharan wrote:
This patch makes the following changes to configure DMIC from ipc params:
- remove redundant hdr member item from struct
sof_ipc_dai_dmic_params 2. Rename number_of_pdm_controllers member in the above structure to num_pdm_active to be more representative of the active pdm count. 3. Add an "id" member to struct sof_ipc_dai_dmic_pdm_ctrl 4. Remove hardcoded config params from DMIC set_config function and use ipc params instead.
Signed-off-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com
Liam
On 31.05.2018 23:36, Liam Girdwood wrote:
I'm good if Seppo is ?
Yes, it's OK!
Sorry, I though I had already sent my ack & review note for this work. I had only the question about how to increase channels count in the generated test topologies and it was explained so everything looks good to me.
Thanks, Seppo
On Sat, 2018-05-26 at 23:22 -0700, Ranjani Sridharan wrote:
This patch makes the following changes to configure DMIC from ipc params:
- remove redundant hdr member item from struct sof_ipc_dai_dmic_params
- Rename number_of_pdm_controllers member in the above structure
to num_pdm_active to be more representative of the active pdm count. 3. Add an "id" member to struct sof_ipc_dai_dmic_pdm_ctrl 4. Remove hardcoded config params from DMIC set_config function and use ipc params instead.
Signed-off-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com
Applied.
Thanks
Liam
participants (3)
-
Liam Girdwood
-
Ranjani Sridharan
-
Seppo Ingalsuo