[Sound-open-firmware] [PATCH 1/4] [RFC]DMIC: Add PDM microphones (DMIC) support to DAI

[Pierre-Louis Bossart]

On 4/30/18 11:04 AM, Seppo Ingalsuo wrote:
> This patch adds the DMIC audio capture driver for SOF DAI component use.
> The DMIC feature allows to directly attach one to (typically) four PDM
> digital microphones into Intel SoC without a separate codec IC. This is
> supported by APL and most successor platforms.
> 
> Corresponding patches are needed for kernel driver and topology to enable
> this feature.
> 
> Tested in
> APL UP squared board without connected microphones
> SOF git master 3ad69eb715a09de9a0b91c56c9cca8a79ead00a9
> 
> Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo at linux.intel.com>
> ---
>   configure.ac            |    6 +
>   src/audio/dai.c         |    5 +
>   src/drivers/Makefile.am |    6 +-
>   src/drivers/dmic.c      | 1331 +++++++++++++++++++++++++++++++++++++++++++++++

this is a big one...

>   src/include/sof/dmic.h  |  318 +++++++++++
>   src/include/sof/io.h    |    8 +
>   src/include/sof/trace.h |    3 +-
>   src/include/uapi/ipc.h  |   61 ++-

and all those files could really be in separate patches since they 
prepare the work for dmic enablement.

>   8 files changed, 1734 insertions(+), 4 deletions(-)
>   create mode 100644 src/drivers/dmic.c
>   create mode 100644 src/include/sof/dmic.h
> 

[...]

> +/* 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
> + * microphone clock min/max and duty cycle requirements need be checked from
> + * used microphone component datasheet.
> + */
> +static void find_modes(struct decim_modes *modes,
> +	struct sof_ipc_dai_dmic_params *prm, uint32_t fs)
> +{
> +	int clkdiv_min;
> +	int clkdiv_max;
> +	int clkdiv;
> +	int c1;
> +	int c2;
> +	int du_min;
> +	int du_max;
> +	int pdmclk;
> +	int osr;
> +	int mfir;
> +	int mcic;
> +	int ioclk_test;
> +	int osr_min = DMIC_MIN_OSR;
> +	int i = 0;
> +
> +	/* Defaults, empty result */
> +	modes->num_of_modes = 0;
> +
> +	/* The FIFO is not requested if sample rate is set to zero. Just
> +	 * return in such case with num_of_modes as zero.
> +	 */
> +	if (fs == 0)
> +		return;
> +
> +	/* Override DMIC_MIN_OSR for very high sample rates, use as minimum
> +	 * the nominal clock for the high rates.
> +	 */
> +	if (fs >= DMIC_HIGH_RATE_MIN_FS)
> +		osr_min = DMIC_HIGH_RATE_OSR_MIN;
> +
> +	/* Check for sane pdm clock, min 100 kHz, max ioclk/2 */
> +	if (prm->pdmclk_max < DMIC_HW_PDM_CLK_MIN ||
> +		prm->pdmclk_max > DMIC_HW_IOCLK / 2) {
> +		trace_dmic_error("pmx");
> +		return;
> +	}
> +	if (prm->pdmclk_min < DMIC_HW_PDM_CLK_MIN ||
> +		prm->pdmclk_min > prm->pdmclk_max) {
> +		trace_dmic_error("pmn");
> +		return;
> +	}
> +
> +	/* Check for sane duty cycle */
> +	if (prm->duty_min > prm->duty_max) {
> +		trace_dmic_error("pdu");
> +		return;
> +	}
> +	if (prm->duty_min < DMIC_HW_DUTY_MIN ||
> +		prm->duty_min > DMIC_HW_DUTY_MAX) {
> +		trace_dmic_error("pdn");
> +		return;
> +	}
> +	if (prm->duty_max < DMIC_HW_DUTY_MIN ||
> +		prm->duty_max > DMIC_HW_DUTY_MAX) {
> +		trace_dmic_error("pdx");
> +		return;
> +	}
> +
> +	/* Min and max clock dividers */
> +	clkdiv_min = ceil_divide(DMIC_HW_IOCLK, prm->pdmclk_max);
> +	clkdiv_min = MAX(clkdiv_min, DMIC_HW_CIC_DECIM_MIN);
> +	clkdiv_max = DMIC_HW_IOCLK / prm->pdmclk_min;
> +
> +	/* Loop possible clock dividers and check based on resulting
> +	 * oversampling ratio that CIC and FIR decimation ratios are
> +	 * feasible. The ratios need to be integers. Also the mic clock
> +	 * duty cycle need to be within limits.
> +	 */
> +	for (clkdiv = clkdiv_min; clkdiv <= clkdiv_max; clkdiv++) {
> +		c1 = clkdiv >> 1;
> +		c2 = clkdiv - c1;
> +		du_min = 100 * c1 / clkdiv;

du_min = 100 * (clkdiv/2)/clkdiv  -> 50?

> +		du_max = 100 * c2 / clkdiv;

c2 = clkdiv - c1 = clkdiv/2, so du_min == du_max ?
what am I missing?

> +		pdmclk = DMIC_HW_IOCLK / clkdiv;
> +		osr = pdmclk / fs;
> +
> +		/* Check that OSR constraints is met and clock duty cycle does
> +		 * not exceed microphone specification. If exceed proceed to
> +		 * next clkdiv.
> +		 */
> +		if (osr < osr_min || du_min < prm->duty_min ||
> +			du_max > prm->duty_max)
> +			continue;
> +
> +		/* Loop FIR decimation factors candidates. If the
> +		 * integer divided decimation factors and clock dividers
> +		 * as multiplied with sample rate match the IO clock
> +		 * rate the division was exact and such decimation mode
> +		 * is possible. Then check that CIC decimation constraints
> +		 * are met. The passed decimation modes are added to array.
> +		 */
> +		for (mfir = DMIC_HW_FIR_DECIM_MIN;
> +			mfir <= DMIC_HW_FIR_DECIM_MAX; mfir++) {
> +			mcic = osr / mfir;
> +			ioclk_test = fs * mfir * mcic * clkdiv;
> +
> +			if (ioclk_test == DMIC_HW_IOCLK &&
> +				mcic >= DMIC_HW_CIC_DECIM_MIN &&
> +				mcic <= DMIC_HW_CIC_DECIM_MAX &&
> +				i < DMIC_MAX_MODES) {
> +				modes->clkdiv[i] = clkdiv;
> +				modes->mcic[i] = mcic;
> +				modes->mfir[i] = mfir;
> +				i++;
> +				modes->num_of_modes = i;
> +			}
> +		}
> +	}
> +}
> +
> +/* The previous raw modes list contains sane configuration possibilities. When
> + * there is request for both FIFOs A and B operation this function returns
> + * list of compatible settings.
> + */
> +static void match_modes(struct matched_modes *c, struct decim_modes *a,
> +	struct decim_modes *b)
> +{
> +	int idx[DMIC_MAX_MODES];
> +	int idx_length;
> +	int i;
> +	int n;
> +	int m;
> +
> +	/* Check if previous search got results. */
> +	c->num_of_modes = 0;
> +	if (a->num_of_modes == 0 && b->num_of_modes == 0) {
> +		/* Nothing to do */
> +		return;
> +	}
> +
> +	/* Check for request only for FIFO A or B. In such case pass list for
> +	 * A or B as such.
> +	 */
> +	if (b->num_of_modes == 0) {
> +		c->num_of_modes = a->num_of_modes;
> +		for (i = 0; i < a->num_of_modes; i++) {
> +			c->clkdiv[i] = a->clkdiv[i];
> +			c->mcic[i] = a->mcic[i];
> +			c->mfir_a[i] = a->mfir[i];
> +			c->mfir_b[i] = 0;

is this initialization to zero needed?

> +		}
> +		return;
> +	}
> +
> +	if (a->num_of_modes == 0) {
> +		c->num_of_modes = b->num_of_modes;
> +		for (i = 0; i < b->num_of_modes; i++) {
> +			c->clkdiv[i] = b->clkdiv[i];
> +			c->mcic[i] = b->mcic[i];
> +			c->mfir_b[i] = b->mfir[i];
> +			c->mfir_a[i] = 0;

same here, is this initialization needed?

> +		}
> +		return;
> +	}
> +
> +	/* Merge a list of compatible modes */
> +	i = 0;
> +	for (n = 0; n < a->num_of_modes; n++) {
> +		/* Find all indices of values a->clkdiv[n] in b->clkdiv[] */
> +		idx_length = find_equal(idx, b->clkdiv, a->clkdiv[n],
> +			b->num_of_modes, 0);
> +		for (m = 0; m < idx_length; m++) {
> +			if (b->mcic[idx[m]] == a->mcic[n]) {
> +				c->clkdiv[i] = a->clkdiv[n];
> +				c->mcic[i] = a->mcic[n];
> +				c->mfir_a[i] = a->mfir[n];
> +				c->mfir_b[i] = b->mfir[idx[m]];
> +				i++;
> +			}
> +		}
> +		c->num_of_modes = i;
> +	}
> +}
> +
> +/* Finds a suitable FIR decimation filter from the included set */
> +static struct pdm_decim *get_fir(struct dmic_configuration *cfg, int mfir)
> +{
> +	int i;
> +	int fs;
> +	int cic_fs;
> +	int fir_max_length;
> +	struct pdm_decim *fir = NULL;
> +
> +	if (mfir <= 0)
> +		return fir;
> +
> +	cic_fs = DMIC_HW_IOCLK / cfg->clkdiv / cfg->mcic;
> +	fs = cic_fs / mfir;
> +	/* FIR max. length depends on available cycles and coef RAM
> +	 * length. Exceeding this length sets HW overrun status and
> +	 * overwrite of other register.
> +	 */
> +	fir_max_length = MIN(DMIC_HW_FIR_LENGTH_MAX,
> +		DMIC_HW_IOCLK / fs / 2 - DMIC_FIR_PIPELINE_OVERHEAD);
> +
> +	for (i = 0; i < DMIC_FIR_LIST_LENGTH; i++) {
> +		if (fir_list[i]->decim_factor == mfir &&
> +			fir_list[i]->length <= fir_max_length) {
> +			/* Store pointer, break from loop to avoid a
> +			 * Possible other mode with lower FIR length.
> +			 */
> +			fir = fir_list[i];
> +			break;
> +		}
> +	}
> +
> +	return fir;
> +}
> +
> +/* Calculate scale and shift to use for FIR coefficients. Scale is applied
> + * before write to HW coef RAM. Shift will be programmed to HW register.
> + */
> +static int fir_coef_scale(int32_t *fir_scale, int *fir_shift, int add_shift,
> +	const int32_t coef[], int coef_length, int32_t gain)
> +{
> +	int32_t amax;
> +	int32_t new_amax;
> +	int32_t new_scale;
> +	int32_t fir_gain;
> +	int shift;
> +	const int32_t coef_max_val = Q_CONVERT_FLOAT(0.9999, 20); /* Q1.20 */
> +
> +	/* Multiply gain passed from CIC with output full scale, result Q4.20 */
> +	fir_gain = Q_MULTSR_32X32((int64_t)gain, DMIC_HW_SENS_Q23, 20, 23, 20);
> +
> +	/* Find the largest FIR coefficient value */
> +	amax = find_max_abs_int32((int32_t *)coef, coef_length);
> +
> +	/* Scale with FIR gain */
> +	new_amax = Q_MULTSR_32X32((int64_t)amax, fir_gain, 31, 20, 20);
> +	if (new_amax <= 0)
> +		return -EINVAL;
> +
> +	/* Needed scale for FIR taps, target is 0.9999 max */
> +	new_scale = (int32_t)((((int64_t)coef_max_val << 20)) / new_amax);
> +
> +	/* Find shift value */
> +	if (new_scale == 0)
> +		return -EINVAL;
> +
> +	shift = 0;
> +	while ((new_scale << shift) < (1 << 20))
> +		shift++;

don't we have some sort of macro for this (find number of leading zeroes)?

> +
> +	/* Add to shift in storate Q31 format and store to configuration */
> +	*fir_shift = shift + add_shift;
> +
> +	/* Compensate shift value to scale and store to configuration
> +	 * as Q1.31. Also apply raw 32 bit to actual HW FIR coef
> +	 * precision.
> +	 */
> +	*fir_scale = (fir_gain >> shift);
> +	return 0;
> +}
> +
> +/* This function selects with a simple criteria one mode to set up the
> + * decimator. For the settings chosen for FIFOs A and B output a lookup
> + * is done for FIR coefficients from the included coefficients tables.
> + * For some decimation factors there may be several length coefficient sets.
> + * It is due to possible restruction of decimation engine cycles per given
> + * sample rate. If the coefficients length is exceeded the lookup continues.
> + * Therefore the list of coefficient set must present the filters for a
> + * decimation factor in decreasing length order.
> + *
> + * Note: If there is no filter available an error is returned. The parameters
> + * should be reviewed for such case. If still a filter is missing it should be
> + * added into the included set. FIR decimation with a high factor usually
> + * needs compromizes into specifications and is not desirable.
> + */
> +static int select_mode(struct dmic_configuration *cfg,
> +	struct matched_modes *modes)
> +{
> +	int32_t g_cic;
> +	int32_t g_tmp;
> +	int32_t fir_in_max;
> +	int32_t cic_out_max;
> +	int32_t gain_to_fir;
> +	int idx[DMIC_MAX_MODES];
> +	int n = 1;
> +	int mmin;
> +	int count;
> +	int *mfir;
> +	int mcic;
> +	int bits_cic;
> +	int ret;
> +
> +	/* If there are more than one possibilities select a mode with lowest
> +	 * FIR decimation factor. If there are several select mode with highest
> +	 * ioclk divider to minimize microphone power consumption. The highest
> +	 * clock divisors are in the end of list so select the last of list.
> +	 * The minimum OSR criteria used in previous ensures that quality in
> +	 * the candidates should be sufficient.
> +	 */
> +	if (modes->num_of_modes == 0) {
> +		trace_dmic_error("nom");
> +		return -EINVAL;
> +	}
> +
> +	if (modes->mfir_a[0] > 0)
> +		mfir = modes->mfir_a;
> +	else
> +		mfir = modes->mfir_b;

does look like the initialization done above has a side effect here...
worth a comment maybe?
> +
> +	mmin = find_min(mfir, modes->num_of_modes);
> +	count = find_equal(idx, mfir, mmin, modes->num_of_modes, 0);
> +	n = idx[count - 1];
> +
> +	/* Get microphone clock and decimation parameters for used mode from
> +	 * the list.
> +	 */
> +	cfg->clkdiv = modes->clkdiv[n];
> +	cfg->mfir_a = modes->mfir_a[n];
> +	cfg->mfir_b = modes->mfir_b[n];
> +	cfg->mcic = modes->mcic[n];
> +	cfg->fir_a = NULL;
> +	cfg->fir_b = NULL;
> +
> +	/* Find raw FIR coefficients to match the decimation foctors of FIR

factors

> +	 * A and B.
> +	 */
> +	if (cfg->mfir_a > 0) {
> +		cfg->fir_a = get_fir(cfg, cfg->mfir_a);
> +		if (!cfg->fir_a) {
> +			trace_dmic_error("fam");
> +			trace_value(cfg->mfir_a);
> +			return -EINVAL;
> +		}
> +	}
> +
> +	if (cfg->mfir_b > 0) {
> +		cfg->fir_b = get_fir(cfg, cfg->mfir_b);
> +		if (!cfg->fir_b) {
> +			trace_dmic_error("fbm");
> +			trace_value(cfg->mfir_b);
> +			return -EINVAL;
> +		}
> +	}
> +
> +	/* Calculate CIC shift from the decimation factor specific gain.
> +	 */
> +	mcic = cfg->mcic;
> +	g_cic = mcic * mcic * mcic * mcic * mcic;

yikes, what is this mcic^5 ?


> +	g_tmp = g_cic;
> +	bits_cic = 1;
> +	if (g_tmp < 0) {
> +		/* Erroneus decimation factor and CIC gain */

erroneous?

> +		trace_dmic_error("gci");
> +		return -EINVAL;
> +	}
> +	while (g_tmp > 0) {
> +		g_tmp = g_tmp >> 1;
> +		bits_cic++;
> +	}

this again looks like finding the first non-zero bit

> +	cfg->cic_shift = bits_cic - DMIC_HW_BITS_FIR_INPUT;
> +
> +	/* Calculate remaining gain to FIR as Q4.20. */
> +	fir_in_max = (1 << (DMIC_HW_BITS_FIR_INPUT - 1));
> +	if (cfg->cic_shift >= 0)
> +		cic_out_max = g_cic >> cfg->cic_shift;
> +	else
> +		cic_out_max = g_cic << -cfg->cic_shift;
> +
> +	gain_to_fir = (int32_t)((((int64_t) fir_in_max) << 20) / cic_out_max);
> +
> +	/* Calculate FIR scale and shift */
> +	if (cfg->mfir_a > 0) {
> +		cfg->fir_a_length = cfg->fir_a->length;
> +		ret = fir_coef_scale(&cfg->fir_a_scale, &cfg->fir_a_shift,
> +			cfg->fir_a->shift, cfg->fir_a->coef, cfg->fir_a->length,
> +			gain_to_fir);
> +		if (ret < 0) {
> +			/* Invalid coefficient set found, should not happen. */
> +			trace_dmic_error("ina");
> +			return -EINVAL;
> +		}
> +	} else {
> +		cfg->fir_a_scale = 0;
> +		cfg->fir_a_shift = 0;
> +		cfg->fir_a_length = 0;
> +	}
> +
> +	if (cfg->mfir_b > 0) {
> +		cfg->fir_b_length = cfg->fir_b->length;
> +		ret = fir_coef_scale(&cfg->fir_b_scale, &cfg->fir_b_shift,
> +			cfg->fir_b->shift, cfg->fir_b->coef, cfg->fir_b->length,
> +			gain_to_fir);
> +		if (ret < 0) {
> +			/* Invalid coefficient set found, should not happen. */
> +			trace_dmic_error("inb");
> +			return -EINVAL;
> +		}
> +	} else {
> +		cfg->fir_b_scale = 0;
> +		cfg->fir_b_shift = 0;
> +		cfg->fir_b_length = 0;
> +	}
> +
> +	return 0;
> +}
> +
> +/* The FIFO input packer mode (IPM) settings are somewhat different in
> + * 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[],
> +	struct sof_ipc_dai_dmic_params *dmic)
> +{
> +	int pdm[DMIC_HW_CONTROLLERS];
> +	int i;
> +
> +	/* Loop number of PDM controllers in the configuration. If mic A
> +	 * or B is enabled then a pdm controller is marked as active. Also it
> +	 * is checked whether the controller should operate as stereo or mono
> +	 * 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++) {
> +		if (dmic->pdm[i].enable_mic_a > 0 ||
> +			dmic->pdm[i].enable_mic_b > 0)
> +			pdm[i] = 1;
> +		else
> +			pdm[i] = 0;
> +
> +		if (dmic->pdm[i].enable_mic_a > 0 &&
> +			dmic->pdm[i].enable_mic_b > 0) {
> +			stereo[i] = 1;
> +			swap[i] = 0;
> +		} else {
> +			stereo[i] = 0;
> +			if (dmic->pdm[i].enable_mic_a == 0)
> +				swap[i] = 1;
> +			else
> +				swap[i] = 0;
> +		}

the logic is weird here.

if (a || b)
	...
else 	
	...

if (a && b)
	...
else
	// this could be a || b or !a && !b??

> +	}
> +
> +	/* IPM indicates active pdm controllers. */
> +	*ipm = 0;
> +
> +	if (pdm[0] == 0 && pdm[1] > 0)
> +		*ipm = 1;
> +
> +	if (pdm[0] > 0 && pdm[1] > 0)
> +		*ipm = 2;
> +}
> +#endif
> +
> +#if DMIC_HW_VERSION == 2
> +
> +static inline void source_ipm_helper(int source[], int *ipm, int stereo[],
> +	int swap[], struct sof_ipc_dai_dmic_params *dmic)
> +{
> +	int pdm[DMIC_HW_CONTROLLERS];
> +	int i;
> +	int n = 0;
> +
> +	/* Loop number of PDM controllers in the configuration. If mic A
> +	 * or B is enabled then a pdm controller is marked as active. Also it
> +	 * is checked whether the controller should operate as stereo or mono
> +	 * left (A) or mono right (B) mode. Mono right mode is setup as channel
> +	 * 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++) {
> +		if (dmic->pdm[i].enable_mic_a > 0 ||
> +			dmic->pdm[i].enable_mic_b > 0) {
> +			pdm[i] = 1;
> +			source[n] = i;
> +			n++;
> +		} else {
> +			pdm[i] = 0;
> +			swap[i] = 0;
> +		}
> +
> +		if (dmic->pdm[i].enable_mic_a > 0 &&
> +			dmic->pdm[i].enable_mic_b > 0) {
> +			stereo[i] = 1;
> +			swap[i] = 0;
> +		} else {
> +			stereo[i] = 0;
> +			if (dmic->pdm[i].enable_mic_a == 0)
> +				swap[i] = 1;
> +			else
> +				swap[i] = 0;
> +		}
> +	}
> +
> +	/* IPM bit field is set to count of active pdm controllers. */
> +	*ipm = pdm[0];
> +	for (i = 0; i < dmic->number_of_pdm_controllers; i++)
> +		*ipm += pdm[i];
> +}
> +#endif
> +
> +static int configure_registers(struct dai *dai, struct dmic_configuration *cfg,
> +	struct sof_ipc_dai_dmic_params *dmic)
> +{
> +	int stereo[DMIC_HW_CONTROLLERS];
> +	int swap[DMIC_HW_CONTROLLERS];
> +	uint32_t val;
> +	int32_t ci;
> +	uint32_t cu;
> +	int ipm;
> +	int of0;
> +	int of1;
> +	int fir_decim;
> +	int fir_length;
> +	int length;
> +	int edge;
> +	int dccomp;
> +	int cic_start_a;
> +	int cic_start_b;
> +	int fir_start_a;
> +	int fir_start_b;
> +	int soft_reset;
> +	int i;
> +	int j;
> +
> +	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 bfth = 1; /* Should be 3 for 8 entries, 1 is 2 entries */
> +	int th = 0; /* Used with TIE=1 */
> +
> +	/* Normal start sequence */
> +	dccomp = 1;
> +	soft_reset = 1;
> +	cic_start_a = 0;
> +	cic_start_b = 0;
> +	fir_start_a = 0;
> +	fir_start_b = 0;
> +
> +#if DMIC_HW_VERSION == 2
> +	int source[4] = {0, 0, 0, 0};
> +#endif
> +
> +	/* pdata is set by dmic_probe(), error if it has not been set */
> +	if (!pdata) {
> +		trace_dmic_error("cfr");
> +		return -EINVAL;
> +	}
> +
> +	/* Sanity checks */
> +	if (dmic->number_of_pdm_controllers > DMIC_HW_CONTROLLERS) {
> +		trace_dmic_error("num");
> +		return -EINVAL;
> +	}
> +
> +	/* OUTCONTROL0 and OUTCONTROL1 */
> +	trace_dmic("reg");
> +	of0 = (dmic->fifo_bits_a == 32) ? 2 : 0;
> +	of1 = (dmic->fifo_bits_b == 32) ? 2 : 0;
> +
> +#if DMIC_HW_VERSION == 1
> +	ipm_helper(&ipm, stereo, swap, dmic);
> +	val = OUTCONTROL0_TIE(0) |
> +		OUTCONTROL0_SIP(0) |
> +		OUTCONTROL0_FINIT(1) |
> +		OUTCONTROL0_FCI(0) |
> +		OUTCONTROL0_BFTH(bfth) |
> +		OUTCONTROL0_OF(of0) |
> +		OUTCONTROL0_IPM(ipm) |
> +		OUTCONTROL0_TH(th);
> +	dmic_write(dai, OUTCONTROL0, val);
> +	trace_value(val);
> +
> +	val = OUTCONTROL1_TIE(0) |
> +		OUTCONTROL1_SIP(0) |
> +		OUTCONTROL1_FINIT(1) |
> +		OUTCONTROL1_FCI(0) |
> +		OUTCONTROL1_BFTH(bfth) |
> +		OUTCONTROL1_OF(of1) |
> +		OUTCONTROL1_IPM(ipm) |
> +		OUTCONTROL1_TH(th);
> +	dmic_write(dai, OUTCONTROL1, val);
> +	trace_value(val);
> +#endif
> +
> +#if DMIC_HW_VERSION == 2
> +	source_ipm_helper(source, &ipm, stereo, swap, dmic);
> +	val = OUTCONTROL0_TIE(0) |
> +		OUTCONTROL0_SIP(0) |
> +		OUTCONTROL0_FINIT(1) |
> +		OUTCONTROL0_FCI(0) |
> +		OUTCONTROL0_BFTH(3) |
> +		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);
> +	dmic_write(dai, OUTCONTROL0, val);
> +	trace_value(val);
> +
> +	val = OUTCONTROL1_TIE(0) |
> +		OUTCONTROL1_SIP(0) |
> +		OUTCONTROL1_FINIT(1) |
> +		OUTCONTROL1_FCI(0) |
> +		OUTCONTROL1_BFTH(3) |
> +		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);
> +	dmic_write(dai, OUTCONTROL1, val);
> +	trace_value(val);
> +#endif
> +
> +	/* Mark enabled microphones into private data to be later used
> +	 * for starting correct parts of the HW.
> +	 */
> +	for (i = 0; i < DMIC_HW_CONTROLLERS; i++) {
> +		pdata->enable[i] = (dmic->pdm[i].enable_mic_b << 1) |
> +			dmic->pdm[i].enable_mic_a;
> +	}
> +
> +	for (i = 0; i < DMIC_HW_CONTROLLERS; i++) {
> +		/* CIC */
> +		val = CIC_CONTROL_SOFT_RESET(soft_reset) |
> +			CIC_CONTROL_CIC_START_B(cic_start_b) |
> +			CIC_CONTROL_CIC_START_A(cic_start_a) |
> +		CIC_CONTROL_MIC_B_POLARITY(dmic->pdm[i].polarity_mic_a) |
> +		CIC_CONTROL_MIC_A_POLARITY(dmic->pdm[i].polarity_mic_b) |
> +			CIC_CONTROL_MIC_MUTE(cic_mute) |
> +			CIC_CONTROL_STEREO_MODE(stereo[i]);
> +		dmic_write(dai, base[i] + CIC_CONTROL, val);
> +		trace_value(val);
> +
> +		val = CIC_CONFIG_CIC_SHIFT(cfg->cic_shift + 8) |
> +			CIC_CONFIG_COMB_COUNT(cfg->mcic - 1);
> +		dmic_write(dai, base[i] + CIC_CONFIG, val);
> +		trace_value(val);
> +
> +		/* Mono right channel mic usage requires swap of PDM channels
> +		 * since the mono decimation is done with only left channel
> +		 * processing active.
> +		 */
> +		edge = dmic->pdm[i].clk_edge;
> +		if (swap[i])
> +			edge = edge ^ 1;

! edge?

> +
> +		val = MIC_CONTROL_PDM_CLKDIV(cfg->clkdiv - 2) |
> +			MIC_CONTROL_PDM_SKEW(dmic->pdm[i].skew) |
> +			MIC_CONTROL_CLK_EDGE(edge) |
> +			MIC_CONTROL_PDM_EN_B(cic_start_b) |
> +			MIC_CONTROL_PDM_EN_A(cic_start_a);
> +		dmic_write(dai, base[i] + MIC_CONTROL, val);
> +		trace_value(val);
> +
> +		/* FIR A */
> +		fir_decim = MAX(cfg->mfir_a - 1, 0);
> +		fir_length = MAX(cfg->fir_a_length - 1, 0);
> +		val = FIR_CONTROL_A_START(fir_start_a) |
> +			FIR_CONTROL_A_ARRAY_START_EN(array_a) |
> +			FIR_CONTROL_A_DCCOMP(dccomp) |
> +			FIR_CONTROL_A_MUTE(fir_mute) |
> +			FIR_CONTROL_A_STEREO(stereo[i]);
> +		dmic_write(dai, base[i] + FIR_CONTROL_A, val);
> +		trace_value(val);
> +
> +		val = FIR_CONFIG_A_FIR_DECIMATION(fir_decim) |
> +			FIR_CONFIG_A_FIR_SHIFT(cfg->fir_a_shift) |
> +			FIR_CONFIG_A_FIR_LENGTH(fir_length);
> +		dmic_write(dai, base[i] + FIR_CONFIG_A, val);
> +		trace_value(val);
> +
> +		val = DC_OFFSET_LEFT_A_DC_OFFS(DCCOMP_TC0);
> +		dmic_write(dai, base[i] + DC_OFFSET_LEFT_A, val);
> +		trace_value(val);
> +
> +		val = DC_OFFSET_RIGHT_A_DC_OFFS(DCCOMP_TC0);
> +		dmic_write(dai, base[i] + DC_OFFSET_RIGHT_A, val);
> +		trace_value(val);
> +
> +		val = OUT_GAIN_LEFT_A_GAIN(0);
> +		dmic_write(dai, base[i] + OUT_GAIN_LEFT_A, val);
> +		trace_value(val);
> +
> +		val = OUT_GAIN_RIGHT_A_GAIN(0);
> +		dmic_write(dai, base[i] + OUT_GAIN_RIGHT_A, val);
> +		trace_value(val);
> +
> +		/* FIR B */
> +		fir_decim = MAX(cfg->mfir_b - 1, 0);
> +		fir_length = MAX(cfg->fir_b_length - 1, 0);
> +		val = FIR_CONTROL_B_START(fir_start_b) |
> +			FIR_CONTROL_B_ARRAY_START_EN(array_b) |
> +			FIR_CONTROL_B_DCCOMP(dccomp) |
> +			FIR_CONTROL_B_MUTE(fir_mute) |
> +			FIR_CONTROL_B_STEREO(stereo[i]);
> +		dmic_write(dai, base[i] + FIR_CONTROL_B, val);
> +		trace_value(val);
> +
> +		val = FIR_CONFIG_B_FIR_DECIMATION(fir_decim) |
> +			FIR_CONFIG_B_FIR_SHIFT(cfg->fir_b_shift) |
> +			FIR_CONFIG_B_FIR_LENGTH(fir_length);
> +		dmic_write(dai, base[i] + FIR_CONFIG_B, val);
> +		trace_value(val);
> +
> +		val = DC_OFFSET_LEFT_B_DC_OFFS(DCCOMP_TC0);
> +		dmic_write(dai, base[i] + DC_OFFSET_LEFT_B, val);
> +		trace_value(val);
> +		trace_value(val);
> +
> +		val = DC_OFFSET_RIGHT_B_DC_OFFS(DCCOMP_TC0);
> +		dmic_write(dai, base[i] + DC_OFFSET_RIGHT_B, val);
> +		trace_value(val);
> +
> +		val = OUT_GAIN_LEFT_B_GAIN(0);
> +		dmic_write(dai, base[i] + OUT_GAIN_LEFT_B, val);
> +		trace_value(val);
> +
> +		val = OUT_GAIN_RIGHT_B_GAIN(0);
> +		dmic_write(dai, base[i] + OUT_GAIN_RIGHT_B, val);
> +		trace_value(val);
> +
> +		/* Write coef RAM A with scaled coefficient in reverse order */
> +		length = cfg->fir_a_length;
> +		for (j = 0; j < length; j++) {
> +			ci = (int32_t)Q_MULTSR_32X32(
> +				(int64_t)cfg->fir_a->coef[j],
> +				cfg->fir_a_scale, 31, 20, DMIC_HW_FIR_COEF_Q);
> +			cu = FIR_COEF_A(ci);
> +			dmic_write(dai, coef_base_a[i]
> +				+ ((length - j - 1) << 2), cu);
> +		}
> +
> +		/* Write coef RAM B with scaled coefficient in reverse order */
> +		length = cfg->fir_b_length;
> +		for (j = 0; j < length; j++) {
> +			ci = (int32_t)Q_MULTSR_32X32(
> +				(int64_t)cfg->fir_b->coef[j],
> +				cfg->fir_b_scale, 31, 20, DMIC_HW_FIR_COEF_Q);
> +			cu = FIR_COEF_B(ci);
> +			dmic_write(dai, coef_base_b[i]
> +				+ ((length - j - 1) << 2), cu);
> +		}
> +	}
> +
> +	/* Function dmic_start() uses these to start the used FIFOs */
> +	if (cfg->mfir_a > 0)
> +		pdata->fifo_a = 1;
> +	else
> +		pdata->fifo_a = 0;
> +
> +	if (cfg->mfir_b > 0)
> +		pdata->fifo_b = 1;
> +	else
> +		pdata->fifo_b = 0;
> +
> +	return 0;
> +}
> +
> +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 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);
> +
> +	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.
> +	 */
> +	prm->driver_ipc_version = 1;
> +	prm->pdmclk_min = 768000; /* Min 768 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 = 0;
> +	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
> +
> +	trace_value(prm->driver_ipc_version);
> +	trace_value(prm->pdmclk_min);
> +	trace_value(prm->pdmclk_max);
> +	trace_value(prm->fifo_fs_a);
> +	trace_value(prm->fifo_fs_b);
> +	trace_value(prm->fifo_bits_a);
> +	trace_value(prm->fifo_bits_b);
> +	trace_value(prm->duty_min);
> +	trace_value(prm->duty_max);
> +	trace_value(prm->number_of_pdm_controllers);
> +
> +	if (prm->driver_ipc_version != DMIC_IPC_VERSION) {
> +		trace_dmic_error("ver");
> +		return -EINVAL;
> +	}
> +
> +	/* Match and select optimal decimators configuration for FIFOs A and B
> +	 * paths. This setup phase is still abstract. Successful completion
> +	 * points struct cfg to FIR coefficients and contains the scale value
> +	 * to use for FIR coefficient RAM write as well as the CIC and FIR
> +	 * shift values.
> +	 */
> +	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;
> +	}
> +
> +	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;
> +	}
> +
> +	match_modes(&modes_ab, &modes_a, &modes_b);
> +	ret = select_mode(&cfg, &modes_ab);
> +	if (ret < 0) {
> +		trace_dmic_error("smo");
> +		return -EINVAL;
> +	}
> +
> +	trace_dmic("cfg");
> +	trace_value(cfg.clkdiv);
> +	trace_value(cfg.mcic);
> +	trace_value(cfg.mfir_a);
> +	trace_value(cfg.mfir_b);
> +	trace_value(cfg.fir_a_length);
> +	trace_value(cfg.fir_b_length);
> +	trace_value(cfg.cic_shift);
> +	trace_value(cfg.fir_a_shift);
> +	trace_value(cfg.fir_b_shift);
> +
> +	/* Struct reg contains a mirror of actual HW registers. Determine
> +	 * register bits configuration from decimator configuration and the
> +	 * requested parameters.
> +	 */
> +	ret = configure_registers(dai, &cfg, prm);
> +	if (ret < 0) {
> +		trace_dmic_error("cor");
> +		return -EINVAL;
> +	}
> +
> +	dmic->state = COMP_STATE_PREPARE;
> +
> +	return 0;
> +}
> +
> +/* start the DMIC for capture */
> +static void dmic_start(struct dai *dai, int direction)
> +{
> +	struct dmic_pdata *dmic = dai_get_drvdata(dai);
> +	int i;
> +	int mic_a;
> +	int mic_b;
> +	int fir_a;
> +	int fir_b;
> +
> +	if (direction != DAI_DIR_CAPTURE)
> +		return;
> +
> +	/* enable port */
> +	spin_lock(&dmic->lock);
> +	trace_dmic("sta");
> +	dmic->state = COMP_STATE_ACTIVE;
> +
> +	if (dmic->fifo_a) {
> +		trace_dmic("ffa");
> +		/*  Clear FIFO A initialize, Enable interrupts to DSP,
> +		 *  Start FIFO A packer.
> +		 */
> +		dmic_update_bits(dai, OUTCONTROL0,
> +			OUTCONTROL0_FINIT_BIT | OUTCONTROL0_SIP_BIT,
> +			OUTCONTROL0_SIP_BIT);
> +	}
> +	if (dmic->fifo_b) {
> +		trace_dmic("ffb");
> +		/*  Clear FIFO B initialize, Enable interrupts to DSP,
> +		 *  Start FIFO B packer.
> +		 */
> +		dmic_update_bits(dai, OUTCONTROL1,
> +			OUTCONTROL1_FINIT_BIT | OUTCONTROL1_SIP_BIT,
> +			OUTCONTROL1_SIP_BIT);
> +	}
> +
> +	for (i = 0; i < DMIC_HW_CONTROLLERS; i++) {
> +		mic_a = dmic->enable[i] & 1;
> +		mic_b = (dmic->enable[i] & 2) >> 1;
> +		if (dmic->fifo_a)
> +			fir_a = (dmic->enable[i] > 0) ? 1 : 0;
> +		else
> +			fir_a = 0;
> +		if (dmic->fifo_b)
> +			fir_b = (dmic->enable[i] > 0) ? 1 : 0;
> +		else
> +			fir_b = 0;
> +
> +		trace_dmic("mfn");
> +		trace_value(mic_a);
> +		trace_value(mic_b);
> +		trace_value(fir_a);
> +		trace_value(fir_b);
> +
> +		dmic_update_bits(dai, base[i] + CIC_CONTROL,
> +			CIC_CONTROL_CIC_START_A_BIT |
> +			CIC_CONTROL_CIC_START_B_BIT,
> +			CIC_CONTROL_CIC_START_A(mic_a) |
> +			CIC_CONTROL_CIC_START_B(mic_b));
> +		dmic_update_bits(dai, base[i] + MIC_CONTROL,
> +			MIC_CONTROL_PDM_EN_A_BIT |
> +			MIC_CONTROL_PDM_EN_B_BIT,
> +			MIC_CONTROL_PDM_EN_A(mic_a) |
> +			MIC_CONTROL_PDM_EN_B(mic_b));
> +
> +		dmic_update_bits(dai, base[i] + FIR_CONTROL_A,
> +			FIR_CONTROL_A_START_BIT, FIR_CONTROL_A_START(fir_a));
> +		dmic_update_bits(dai, base[i] + FIR_CONTROL_B,
> +			FIR_CONTROL_B_START_BIT, FIR_CONTROL_B_START(fir_b));
> +	}
> +
> +	/* Clear soft reset for all/used PDM controllers. This should
> +	 * start capture in sync.
> +	 */
> +	trace_dmic("unr");
> +	for (i = 0; i < DMIC_HW_CONTROLLERS; i++) {
> +		dmic_update_bits(dai, base[i] + CIC_CONTROL,
> +			CIC_CONTROL_SOFT_RESET_BIT, 0);
> +	}
> +
> +	spin_unlock(&dmic->lock);
> +
> +	/* Currently there's no DMIC HW internal mutings and wait times
> +	 * applied into this start sequence. It can be implemented here if
> +	 * start of audio capture would contain clicks and/or noise and it
> +	 * is not suppressed by gain ramp somewhere in the capture pipe.
> +	 */
> +	trace_dmic("run");
> +}
> +
> +/* stop the DMIC for capture */
> +static void dmic_stop(struct dai *dai)
> +{
> +	struct dmic_pdata *dmic = dai_get_drvdata(dai);
> +	int i;
> +
> +	trace_dmic("sto")
> +
> +	spin_lock(&dmic->lock);
> +
> +	/* stop Rx if we are not capturing */
> +	if (dmic->state != COMP_STATE_ACTIVE) {
> +		/* Set every PDM controller to soft reset */
> +		trace_dmic("sre");
> +		for (i = 0; i < DMIC_HW_CONTROLLERS; i++) {
> +			dmic_update_bits(dai, base[i] + CIC_CONTROL,
> +				CIC_CONTROL_SOFT_RESET_BIT, 0);
> +		}
> +
> +		/* Stop FIFO packers and set FIFO initialize bits */
> +		dmic_update_bits(dai, OUTCONTROL0,
> +			OUTCONTROL0_SIP_BIT | OUTCONTROL0_FINIT_BIT,
> +			OUTCONTROL0_FINIT_BIT);
> +		dmic_update_bits(dai, OUTCONTROL1,
> +			OUTCONTROL1_SIP_BIT | OUTCONTROL1_FINIT_BIT,
> +			OUTCONTROL1_FINIT_BIT);
> +
> +		dmic->state = COMP_STATE_PREPARE;
> +	}
> +
> +	/* Set soft reset 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);
> +	}
> +
> +	spin_unlock(&dmic->lock);
> +}
> +
> +/* save DMIC context prior to entering D3 */
> +static int dmic_context_store(struct dai *dai)
> +{
> +	/* TODO: Nothing stored at the moment. Could read the registers into
> +	 * persisten memory if needed but the large coef RAM is not desirable
> +	 * to copy. It would be better to store selected mode parametesr from
> +	 * previous configuration request and re-program registers from
> +	 * scratch.
> +	 */
> +	return 0;
> +}
> +
> +/* restore DMIC context after leaving D3 */
> +static int dmic_context_restore(struct dai *dai)
> +{
> +	/* Nothing restored at the moment. */
> +	return 0;
> +}
> +
> +static int dmic_trigger(struct dai *dai, int cmd, int direction)
> +{
> +	struct dmic_pdata *dmic = dai_get_drvdata(dai);
> +
> +	trace_dmic("tri");
> +
> +	/* dai private is set in dmic_probe(), error if not set */
> +	if (!dmic) {
> +		trace_dmic_error("trn");
> +		return -EINVAL;
> +	}
> +
> +	if (direction != DAI_DIR_CAPTURE) {
> +		trace_dmic_error("cap");
> +		return -EINVAL;
> +	}
> +
> +	switch (cmd) {
> +	case COMP_TRIGGER_START:
> +		if (dmic->state == COMP_STATE_PREPARE ||
> +			dmic->state == COMP_STATE_PAUSED) {
> +			dmic_start(dai, direction);
> +		} else {
> +			trace_dmic_error("cst");
> +			trace_value(dmic->state);
> +		}
> +		break;
> +	case COMP_TRIGGER_RELEASE:
> +		if (dmic->state == COMP_STATE_PREPARE) {
> +			dmic_start(dai, direction);
> +		} else {
> +			trace_dmic_error("crl");
> +			trace_value(dmic->state);
> +		}
> +		break;
> +	case COMP_TRIGGER_STOP:
> +	case COMP_TRIGGER_PAUSE:
> +		dmic->state = COMP_STATE_PAUSED;
> +		dmic_stop(dai);

shouldn't the state assignment be protected by a spinlock (as done for 
the start case)?
Also is this really PAUSED?

> +		break;
> +	case COMP_TRIGGER_RESUME:
> +		dmic_context_restore(dai);
> +		break;
> +	case COMP_TRIGGER_SUSPEND:
> +		dmic_context_store(dai);
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	return 0;
> +}
> +
> +/* TODO: No idea what should be done here. Currently only trace the
> + * status register that contains a few status and error bit fields.
> + */
> +static void dmic_irq_handler(void *data)
> +{
> +	struct dai *dai = data;
> +	uint32_t val;
> +
> +	/* Trace OUTSTAT0 register */
> +	val = dmic_read(dai, OUTSTAT0);
> +	trace_dmic("irq");
> +
> +	if (val & OUTSTAT0_ROR_BIT)
> +		trace_dmic_error("eor"); /* Full fifo or PDM overrrun */
> +
> +	trace_value(dmic_read(dai, OUTSTAT0));
> +
> +	/* clear IRQ */
> +	platform_interrupt_clear(dmic_irq(dai), 1);
> +}
> +
> +static int dmic_probe(struct dai *dai)
> +{
> +	struct dmic_pdata *dmic;
> +
> +	trace_dmic("pro");
> +
> +	/* allocate private data */
> +	dmic = rzalloc(RZONE_SYS, SOF_MEM_CAPS_RAM, sizeof(*dmic));
> +	dai_set_drvdata(dai, dmic);
> +
> +	spinlock_init(&dmic->lock);
> +
> +	/* Set state, note there is no playback direction support */
> +	dmic->state = COMP_STATE_READY;
> +
> +	/* register our IRQ handler */
> +	interrupt_register(dmic_irq(dai), dmic_irq_handler, dai);
> +
> +	platform_interrupt_unmask(dmic_irq(dai), 1);
> +	interrupt_enable(dmic_irq(dai));
> +
> +	return 0;
> +}
> +
> +/* DMIC has no loopback support */
> +static inline int dmic_set_loopback_mode(struct dai *dai, uint32_t lbm)
> +{
> +	return -EINVAL;
> +}
> +
> +const struct dai_ops dmic_ops = {
> +	.trigger = dmic_trigger,
> +	.set_config = dmic_set_config,
> +	.pm_context_store = dmic_context_store,
> +	.pm_context_restore = dmic_context_restore,
> +	.probe = dmic_probe,
> +	.set_loopback_mode = dmic_set_loopback_mode,
> +};

[cut here]