[Sound-open-firmware] [PATCH 01/15] [RFC][v2]DMIC: Add PDM microphones (DMIC) support to DAI

[Liam Girdwood]

On Fri, 2018-05-04 at 18:01 +0300, 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>
> ---
>  src/drivers/dmic.c | 1383 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 1383 insertions(+)
>  create mode 100644 src/drivers/dmic.c
> 
> 

I've applied 2,5,7,8,9,10,11,14 and 15. The rest are good but needs  


> +
> +#if defined MODULE_TEST
> +#include <stdio.h>
> +#define QTOF(x, q) ((float)(x) / ((int64_t)1 << (q)))
> +#endif

Does this need to go in a maths header ?

+
+#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);


Best to use #elif



+/* 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;


Regarding the spinlock, was this to be used to guarantee that both mic A and B
would use a compatible configuration at start ? 

> 
> +	if (direction != DAI_DIR_CAPTURE) {
> +		trace_dmic_error("cap");
> +		return -EINVAL;
> +	}


You need to do this here :-

	ret = comp_set_state(dev, cmd);
	if (ret < 0)
		return ret;

This does all state checking/setting so no need to do it below.

> +
> +	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);
> +		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.
> + */

The intention here is to emit any DMIC errors via trace errors to help with
debug.

> +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);
> +}
> 

Liam