[Sound-open-firmware] [PATCH] DMIC: configure DMIC with parameters from ipc instead of hardcoded params
Ranjani Sridharan
ranjani.sridharan at linux.intel.com
Thu May 31 19:46:36 CEST 2018
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:
>
> 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 at 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));
>
More information about the Sound-open-firmware
mailing list