From f3ca63ccb89160a8ebf51466928ef7fcc673a17b Mon Sep 17 00:00:00 2001 From: Kai Vehmanen Date: Wed, 2 Jun 2021 21:14:48 +0300 Subject: [PATCH 01/13] dmic: fix pause/release error leading to invalid dmic_active_fifos The 'in_active' parameter of dmic_stop() is used to control whether the fifo should be released or not. In case of COMP_TRIGGER_PAUSE, this should be false and the 'dmic_active_fifos' count should not be modified. Without this fix: PAUSED -> RELEASE -> dmic_start() -> no change on dmic_active_fifos ACTIVE -> PAUSE -> dmic_stop() -> dmic_active_fifos-- If a test case repeatedly pauses and releases, 'dmic_active_fifos' will go out-of-sync. Fix the issue that dmic_stop for PAUSE does not release the fifo reference. Fixes: 22731744cb04 ("dmic: don't decrement active FIFO count below 0") Signed-off-by: Kai Vehmanen --- src/drivers/intel/dmic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/drivers/intel/dmic.c b/src/drivers/intel/dmic.c index fff265cf9ac0..ae6b131e5f26 100644 --- a/src/drivers/intel/dmic.c +++ b/src/drivers/intel/dmic.c @@ -1527,7 +1527,7 @@ static int dmic_trigger(struct dai *dai, int cmd, int direction) break; case COMP_TRIGGER_PAUSE: dmic->state = COMP_STATE_PAUSED; - dmic_stop(dai, true); + dmic_stop(dai, false); break; case COMP_TRIGGER_RESUME: dmic_context_restore(dai); From 4d38fdbea469379a9d8a039b705d7e3ce874bec4 Mon Sep 17 00:00:00 2001 From: Kai Vehmanen Date: Fri, 4 Jun 2021 13:53:42 +0300 Subject: [PATCH 02/13] drivers: dmic: fix multi-fifo logic in interrupt handler When the dmic driver is instantiated multiple times (e.g. for fifo-A and fifo-B), the interrupt gets registered also twice. While supported usage of interrupt interface, there is no guarantee that the interrupt context data is for the expected dai instance. It is thus not safe to modify the dai state directly or call dai_stop(). Modify the interrupt handler not to make any assumptions on which dai instance is passed as 'data' matches a specific fifo instance. Signed-off-by: Kai Vehmanen --- src/drivers/intel/dmic.c | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/src/drivers/intel/dmic.c b/src/drivers/intel/dmic.c index ae6b131e5f26..408b2903c11a 100644 --- a/src/drivers/intel/dmic.c +++ b/src/drivers/intel/dmic.c @@ -1420,18 +1420,10 @@ static void dmic_start(struct dai *dai) *uncached_dmic_active_fifos); } -/* stop the DMIC for capture */ -static void dmic_stop(struct dai *dai, bool in_active) +static void dmic_stop_fifo_packers(struct dai *dai, int fifo_index) { - struct dmic_pdata *dmic = dai_get_drvdata(dai); - int *uncached_dmic_active_fifos = cache_to_uncache(&dmic_active_fifos); - int i; - - dai_dbg(dai, "dmic_stop()"); - spin_lock(&dai->lock); - /* Stop FIFO packers and set FIFO initialize bits */ - switch (dai->index) { + switch (fifo_index) { case 0: dai_update_bits(dai, OUTCONTROL0, OUTCONTROL0_SIP_BIT | OUTCONTROL0_FINIT_BIT, @@ -1443,6 +1435,19 @@ static void dmic_stop(struct dai *dai, bool in_active) OUTCONTROL1_FINIT_BIT); break; } +} + +/* stop the DMIC for capture */ +static void dmic_stop(struct dai *dai, bool in_active) +{ + struct dmic_pdata *dmic = dai_get_drvdata(dai); + int *uncached_dmic_active_fifos = cache_to_uncache(&dmic_active_fifos); + int i; + + dai_dbg(dai, "dmic_stop()"); + spin_lock(&dai->lock); + + dmic_stop_fifo_packers(dai, dai->index); /* Set soft reset and mute on for all PDM controllers. */ @@ -1549,7 +1554,6 @@ static int dmic_trigger(struct dai *dai, int cmd, int direction) static void dmic_irq_handler(void *data) { struct dai *dai = data; - struct dmic_pdata *dmic = dai_get_drvdata(dai); uint32_t val0; uint32_t val1; @@ -1562,15 +1566,13 @@ static void dmic_irq_handler(void *data) if (val0 & OUTSTAT0_ROR_BIT) { dai_err(dai, "dmic_irq_handler(): full fifo A or PDM overrun"); dai_write(dai, OUTSTAT0, val0); - dmic_stop(dai, dmic->state == COMP_STATE_ACTIVE); - dmic->state = COMP_STATE_PREPARE; + dmic_stop_fifo_packers(dai, 0); } if (val1 & OUTSTAT1_ROR_BIT) { dai_err(dai, "dmic_irq_handler(): full fifo B or PDM overrun"); dai_write(dai, OUTSTAT1, val1); - dmic_stop(dai, dmic->state == COMP_STATE_ACTIVE); - dmic->state = COMP_STATE_PREPARE; + dmic_stop_fifo_packers(dai, 1); } } From f4454260f024d6991e00e969cbe413a45f31d420 Mon Sep 17 00:00:00 2001 From: Seppo Ingalsuo Date: Thu, 3 Jun 2021 20:01:57 +0300 Subject: [PATCH 03/13] Drivers: Intel: DMIC: Track fifos activity with mask bits The counter dmic_active_fifos has been a source of many issues. The use of bits to track active FIFOs is simpler and avoids the risk of counting to negative or over the actual FIFOs count in various control scenarios. The incorrect counter value has caused resources allocate and free and start/stop sequences fails. Signed-off-by: Seppo Ingalsuo --- src/drivers/intel/dmic.c | 39 +++++++++++++++++++++------------------ 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/src/drivers/intel/dmic.c b/src/drivers/intel/dmic.c index 408b2903c11a..03bc355ecf54 100644 --- a/src/drivers/intel/dmic.c +++ b/src/drivers/intel/dmic.c @@ -136,7 +136,7 @@ static const uint32_t coef_base_b[4] = {PDM0_COEFFICIENT_B, PDM1_COEFFICIENT_B, /* Global configuration request for DMIC, need to use uncached address to access them */ static SHARED_DATA struct sof_ipc_dai_dmic_params *dmic_prm[DMIC_HW_FIFOS]; -static SHARED_DATA int dmic_active_fifos; +static SHARED_DATA int dmic_active_fifos_mask; /* this ramps volume changes over time */ static enum task_state dmic_work(void *data) @@ -780,7 +780,7 @@ static int configure_registers(struct dai *dai, struct dmic_configuration *cfg) { struct sof_ipc_dai_dmic_params **prm_t = cache_to_uncache(&dmic_prm[0]); - int uncached_dmic_active_fifos = *cache_to_uncache(&dmic_active_fifos); + int uncached_dmic_active_fifos_mask = *cache_to_uncache(&dmic_active_fifos_mask); int stereo[DMIC_HW_CONTROLLERS]; int swap[DMIC_HW_CONTROLLERS]; uint32_t val; @@ -914,7 +914,7 @@ static int configure_registers(struct dai *dai, } for (i = 0; i < DMIC_HW_CONTROLLERS; i++) { - if (uncached_dmic_active_fifos == 0) { + if (uncached_dmic_active_fifos_mask == 0) { /* CIC */ val = CIC_CONTROL_SOFT_RESET(soft_reset) | CIC_CONTROL_CIC_START_B(0) | @@ -1290,7 +1290,7 @@ static int dmic_set_config(struct dai *dai, struct sof_ipc_dai_config *config) static void dmic_start(struct dai *dai) { struct sof_ipc_dai_dmic_params **uncached_dmic_prm = cache_to_uncache(&dmic_prm[0]); - int *uncached_dmic_active_fifos = cache_to_uncache(&dmic_active_fifos); + int *uncached_dmic_active_fifos_mask = cache_to_uncache(&dmic_active_fifos_mask); struct dmic_pdata *dmic = dai_get_drvdata(dai); int i; int mic_a; @@ -1399,11 +1399,11 @@ static void dmic_start(struct dai *dai) CIC_CONTROL_SOFT_RESET_BIT, 0); } + /* Set bit dai->index */ if (dmic->state == COMP_STATE_PREPARE) - (*uncached_dmic_active_fifos)++; + *uncached_dmic_active_fifos_mask |= BIT(dai->index); dmic->state = COMP_STATE_ACTIVE; - spin_unlock(&dai->lock); /* Currently there's no DMIC HW internal mutings and wait times @@ -1416,8 +1416,8 @@ static void dmic_start(struct dai *dai) DMIC_UNMUTE_RAMP_US); - dai_info(dai, "dmic_start(), done active_fifos = %d", - *uncached_dmic_active_fifos); + dai_info(dai, "dmic_start(), dmic_active_fifos_mask = 0x%x", + *uncached_dmic_active_fifos_mask); } static void dmic_stop_fifo_packers(struct dai *dai, int fifo_index) @@ -1441,7 +1441,7 @@ static void dmic_stop_fifo_packers(struct dai *dai, int fifo_index) static void dmic_stop(struct dai *dai, bool in_active) { struct dmic_pdata *dmic = dai_get_drvdata(dai); - int *uncached_dmic_active_fifos = cache_to_uncache(&dmic_active_fifos); + int *uncached_dmic_active_fifos_mask = cache_to_uncache(&dmic_active_fifos_mask); int i; dai_dbg(dai, "dmic_stop()"); @@ -1451,12 +1451,16 @@ static void dmic_stop(struct dai *dai, bool in_active) /* Set soft reset and mute on for all PDM controllers. */ - dai_info(dai, "dmic_stop(), dmic_active_fifos = %d", - *uncached_dmic_active_fifos); + dai_info(dai, "dmic_stop(), dmic_active_fifos_mask = 0x%x", + *uncached_dmic_active_fifos_mask); + + /* Clear bit dai->index */ + if (in_active) + *uncached_dmic_active_fifos_mask &= ~BIT(dai->index); for (i = 0; i < DMIC_HW_CONTROLLERS; i++) { - /* Don't stop CIC yet if both FIFOs were active */ - if (*uncached_dmic_active_fifos == 1) { + /* Don't stop CIC yet if one FIFO remains active */ + if (*uncached_dmic_active_fifos_mask == 0) { dai_update_bits(dai, base[i] + CIC_CONTROL, CIC_CONTROL_SOFT_RESET_BIT | CIC_CONTROL_MIC_MUTE_BIT, @@ -1477,9 +1481,6 @@ static void dmic_stop(struct dai *dai, bool in_active) } } - if (in_active) - (*uncached_dmic_active_fifos)--; - schedule_task_cancel(&dmic->dmicwork); spin_unlock(&dai->lock); } @@ -1632,7 +1633,7 @@ static int dmic_probe(struct dai *dai) static int dmic_remove(struct dai *dai) { struct sof_ipc_dai_dmic_params **uncached_dmic_prm = cache_to_uncache(&dmic_prm[0]); - int uncached_dmic_active_fifos = *cache_to_uncache(&dmic_active_fifos); + int uncached_dmic_active_fifos_mask = *cache_to_uncache(&dmic_active_fifos_mask); struct dmic_pdata *dmic = dai_get_drvdata(dai); int i; @@ -1648,7 +1649,9 @@ static int dmic_remove(struct dai *dai) interrupt_unregister(dmic->irq, dai); /* The next end tasks must be passed if another DAI FIFO still runs */ - if (uncached_dmic_active_fifos) + dai_info(dai, "dmic_remove(), dmic_active_fifos_mask = 0x%x", + uncached_dmic_active_fifos_mask); + if (uncached_dmic_active_fifos_mask) return 0; pm_runtime_put_sync(DMIC_CLK, dai->index); From ccfc9e6cf54c2e54e63b3e2821a4ea3347630a3c Mon Sep 17 00:00:00 2001 From: Seppo Ingalsuo Date: Fri, 4 Jun 2021 14:49:28 +0300 Subject: [PATCH 04/13] Driver: Intel: DMIC: Remove state check from active FIFO mask update The mask does not over/undercount events so no need to protect from multiple start() or stop() calls. Signed-off-by: Seppo Ingalsuo --- src/drivers/intel/dmic.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/drivers/intel/dmic.c b/src/drivers/intel/dmic.c index 03bc355ecf54..d3b9f0a412e8 100644 --- a/src/drivers/intel/dmic.c +++ b/src/drivers/intel/dmic.c @@ -1400,8 +1400,7 @@ static void dmic_start(struct dai *dai) } /* Set bit dai->index */ - if (dmic->state == COMP_STATE_PREPARE) - *uncached_dmic_active_fifos_mask |= BIT(dai->index); + *uncached_dmic_active_fifos_mask |= BIT(dai->index); dmic->state = COMP_STATE_ACTIVE; spin_unlock(&dai->lock); @@ -1455,8 +1454,7 @@ static void dmic_stop(struct dai *dai, bool in_active) *uncached_dmic_active_fifos_mask); /* Clear bit dai->index */ - if (in_active) - *uncached_dmic_active_fifos_mask &= ~BIT(dai->index); + *uncached_dmic_active_fifos_mask &= ~BIT(dai->index); for (i = 0; i < DMIC_HW_CONTROLLERS; i++) { /* Don't stop CIC yet if one FIFO remains active */ From 8eed33d6a7e81456d3893d0af75ebdd4a94a2382 Mon Sep 17 00:00:00 2001 From: Seppo Ingalsuo Date: Fri, 4 Jun 2021 15:47:41 +0300 Subject: [PATCH 05/13] Drivers: Intel: DMIC: Remove in_active parameter from dmic_stop() Cleanup to trigger() since the parameter is not needed to protect from errors with bit mask active FIFOs tracking. Signed-off-by: Seppo Ingalsuo --- src/drivers/intel/dmic.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/drivers/intel/dmic.c b/src/drivers/intel/dmic.c index d3b9f0a412e8..8f20a0a4a0e1 100644 --- a/src/drivers/intel/dmic.c +++ b/src/drivers/intel/dmic.c @@ -1437,7 +1437,7 @@ static void dmic_stop_fifo_packers(struct dai *dai, int fifo_index) } /* stop the DMIC for capture */ -static void dmic_stop(struct dai *dai, bool in_active) +static void dmic_stop(struct dai *dai) { struct dmic_pdata *dmic = dai_get_drvdata(dai); int *uncached_dmic_active_fifos_mask = cache_to_uncache(&dmic_active_fifos_mask); @@ -1527,11 +1527,11 @@ static int dmic_trigger(struct dai *dai, int cmd, int direction) break; case COMP_TRIGGER_STOP: dmic->state = COMP_STATE_PREPARE; - dmic_stop(dai, true); + dmic_stop(dai); break; case COMP_TRIGGER_PAUSE: dmic->state = COMP_STATE_PAUSED; - dmic_stop(dai, false); + dmic_stop(dai); break; case COMP_TRIGGER_RESUME: dmic_context_restore(dai); From 2a87538c425f3e95c373cd5a90ed6762476a0959 Mon Sep 17 00:00:00 2001 From: Seppo Ingalsuo Date: Tue, 8 Jun 2021 14:02:05 +0300 Subject: [PATCH 06/13] Drivers: Intel: DMIC: Change active_fifos_mask to uint32_t This patch does not change any functionality. The mask is no more signed value and cache_to_uncache() needs a multiple of 32 bit type. Signed-off-by: Seppo Ingalsuo --- src/drivers/intel/dmic.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/drivers/intel/dmic.c b/src/drivers/intel/dmic.c index 8f20a0a4a0e1..84ba54357fbc 100644 --- a/src/drivers/intel/dmic.c +++ b/src/drivers/intel/dmic.c @@ -136,7 +136,7 @@ static const uint32_t coef_base_b[4] = {PDM0_COEFFICIENT_B, PDM1_COEFFICIENT_B, /* Global configuration request for DMIC, need to use uncached address to access them */ static SHARED_DATA struct sof_ipc_dai_dmic_params *dmic_prm[DMIC_HW_FIFOS]; -static SHARED_DATA int dmic_active_fifos_mask; +static SHARED_DATA uint32_t dmic_active_fifos_mask; /* this ramps volume changes over time */ static enum task_state dmic_work(void *data) @@ -1290,7 +1290,7 @@ static int dmic_set_config(struct dai *dai, struct sof_ipc_dai_config *config) static void dmic_start(struct dai *dai) { struct sof_ipc_dai_dmic_params **uncached_dmic_prm = cache_to_uncache(&dmic_prm[0]); - int *uncached_dmic_active_fifos_mask = cache_to_uncache(&dmic_active_fifos_mask); + uint32_t *uncached_dmic_active_fifos_mask = cache_to_uncache(&dmic_active_fifos_mask); struct dmic_pdata *dmic = dai_get_drvdata(dai); int i; int mic_a; @@ -1440,7 +1440,7 @@ static void dmic_stop_fifo_packers(struct dai *dai, int fifo_index) static void dmic_stop(struct dai *dai) { struct dmic_pdata *dmic = dai_get_drvdata(dai); - int *uncached_dmic_active_fifos_mask = cache_to_uncache(&dmic_active_fifos_mask); + uint32_t *uncached_dmic_active_fifos_mask = cache_to_uncache(&dmic_active_fifos_mask); int i; dai_dbg(dai, "dmic_stop()"); @@ -1631,7 +1631,7 @@ static int dmic_probe(struct dai *dai) static int dmic_remove(struct dai *dai) { struct sof_ipc_dai_dmic_params **uncached_dmic_prm = cache_to_uncache(&dmic_prm[0]); - int uncached_dmic_active_fifos_mask = *cache_to_uncache(&dmic_active_fifos_mask); + uint32_t uncached_dmic_active_fifos_mask = *cache_to_uncache(&dmic_active_fifos_mask); struct dmic_pdata *dmic = dai_get_drvdata(dai); int i; From 3fabb11ee6ebe73c6d58cc299e8b8757dd389716 Mon Sep 17 00:00:00 2001 From: Seppo Ingalsuo Date: Tue, 8 Jun 2021 14:13:21 +0300 Subject: [PATCH 07/13] Drivers: Intel: DMIC: Fix FW panic caused by too eager dmic_remove() This patch fixes an FW panic that could be triggered by multiple DMIC DAI capture when pausing, stopping, and resuming. A sequence like this crashed in last operation start capture on dmic0, dmic1 pause 0 stop 1 resume 0 It was caused by dmic_remove() for dmic1 that issued pm_runtime_put_sync() for DMIC clock and power, and freed the dmic_prm[] that resume for 0 needed. commit 282fe224d83b ("dmic: fix pause/release error leading to invalid dmic_active_fifos") solved the panic but left the HW start/stop sequence details incorrect. commit 73e6a6fab19e ("Driver: Intel: DMIC: Remove state check from active FIFO mask update") re-introduced the FW panic issue in multiple DAI capture. This should be a proper quick fix for pause handling. A successive patch will clean up the DMIC component data and do this in a nicer looking way. Signed-off-by: Seppo Ingalsuo --- src/drivers/intel/dmic.c | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/src/drivers/intel/dmic.c b/src/drivers/intel/dmic.c index 84ba54357fbc..20c5d96de91f 100644 --- a/src/drivers/intel/dmic.c +++ b/src/drivers/intel/dmic.c @@ -137,6 +137,7 @@ static const uint32_t coef_base_b[4] = {PDM0_COEFFICIENT_B, PDM1_COEFFICIENT_B, /* Global configuration request for DMIC, need to use uncached address to access them */ static SHARED_DATA struct sof_ipc_dai_dmic_params *dmic_prm[DMIC_HW_FIFOS]; static SHARED_DATA uint32_t dmic_active_fifos_mask; +static SHARED_DATA uint32_t dmic_pause_mask; /* this ramps volume changes over time */ static enum task_state dmic_work(void *data) @@ -1291,6 +1292,7 @@ static void dmic_start(struct dai *dai) { struct sof_ipc_dai_dmic_params **uncached_dmic_prm = cache_to_uncache(&dmic_prm[0]); uint32_t *uncached_dmic_active_fifos_mask = cache_to_uncache(&dmic_active_fifos_mask); + uint32_t *uncached_dmic_pause_mask = cache_to_uncache(&dmic_pause_mask); struct dmic_pdata *dmic = dai_get_drvdata(dai); int i; int mic_a; @@ -1399,8 +1401,9 @@ static void dmic_start(struct dai *dai) CIC_CONTROL_SOFT_RESET_BIT, 0); } - /* Set bit dai->index */ + /* Set bit dai->index for active FIFO, and clear pause bit */ *uncached_dmic_active_fifos_mask |= BIT(dai->index); + *uncached_dmic_pause_mask &= ~BIT(dai->index); dmic->state = COMP_STATE_ACTIVE; spin_unlock(&dai->lock); @@ -1437,10 +1440,11 @@ static void dmic_stop_fifo_packers(struct dai *dai, int fifo_index) } /* stop the DMIC for capture */ -static void dmic_stop(struct dai *dai) +static void dmic_stop(struct dai *dai, bool stop_is_pause) { struct dmic_pdata *dmic = dai_get_drvdata(dai); uint32_t *uncached_dmic_active_fifos_mask = cache_to_uncache(&dmic_active_fifos_mask); + uint32_t *uncached_dmic_pause_mask = cache_to_uncache(&dmic_pause_mask); int i; dai_dbg(dai, "dmic_stop()"); @@ -1453,8 +1457,14 @@ static void dmic_stop(struct dai *dai) dai_info(dai, "dmic_stop(), dmic_active_fifos_mask = 0x%x", *uncached_dmic_active_fifos_mask); - /* Clear bit dai->index */ + /* Clear bit dai->index for active FIFO. If stop for pause, set pause mask bit. + * If stop is not for pausing, it is safe to clear the pause bit. + */ *uncached_dmic_active_fifos_mask &= ~BIT(dai->index); + if (stop_is_pause) + *uncached_dmic_pause_mask |= BIT(dai->index); + else + *uncached_dmic_pause_mask &= ~BIT(dai->index); for (i = 0; i < DMIC_HW_CONTROLLERS; i++) { /* Don't stop CIC yet if one FIFO remains active */ @@ -1527,11 +1537,11 @@ static int dmic_trigger(struct dai *dai, int cmd, int direction) break; case COMP_TRIGGER_STOP: dmic->state = COMP_STATE_PREPARE; - dmic_stop(dai); + dmic_stop(dai, false); break; case COMP_TRIGGER_PAUSE: dmic->state = COMP_STATE_PAUSED; - dmic_stop(dai); + dmic_stop(dai, true); break; case COMP_TRIGGER_RESUME: dmic_context_restore(dai); @@ -1632,6 +1642,7 @@ static int dmic_remove(struct dai *dai) { struct sof_ipc_dai_dmic_params **uncached_dmic_prm = cache_to_uncache(&dmic_prm[0]); uint32_t uncached_dmic_active_fifos_mask = *cache_to_uncache(&dmic_active_fifos_mask); + uint32_t uncached_dmic_pause_mask = *cache_to_uncache(&dmic_pause_mask); struct dmic_pdata *dmic = dai_get_drvdata(dai); int i; @@ -1647,9 +1658,9 @@ static int dmic_remove(struct dai *dai) interrupt_unregister(dmic->irq, dai); /* The next end tasks must be passed if another DAI FIFO still runs */ - dai_info(dai, "dmic_remove(), dmic_active_fifos_mask = 0x%x", - uncached_dmic_active_fifos_mask); - if (uncached_dmic_active_fifos_mask) + dai_info(dai, "dmic_remove(), dmic_active_fifos_mask = 0x%x, dmic_pause_mask = 0x%x", + uncached_dmic_active_fifos_mask, uncached_dmic_pause_mask); + if (uncached_dmic_active_fifos_mask || uncached_dmic_pause_mask) return 0; pm_runtime_put_sync(DMIC_CLK, dai->index); From 3551febdb2604c017f19e831b2f69ea29c39b229 Mon Sep 17 00:00:00 2001 From: Seppo Ingalsuo Date: Tue, 8 Jun 2021 19:40:05 +0300 Subject: [PATCH 08/13] Drivers: Intel: DMIC: Spinlocks notes add This patch adds comments to code parts about spinlocks applied elsewhere to avoid confusion. Signed-off-by: Seppo Ingalsuo --- src/drivers/intel/dmic.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/drivers/intel/dmic.c b/src/drivers/intel/dmic.c index 20c5d96de91f..92619763741c 100644 --- a/src/drivers/intel/dmic.c +++ b/src/drivers/intel/dmic.c @@ -914,6 +914,9 @@ static int configure_registers(struct dai *dai, return ret; } + /* Note about accessing dmic_active_fifos_mask: the dai spinlock has been set in + * the calling function dmic_set_config(). + */ for (i = 0; i < DMIC_HW_CONTROLLERS; i++) { if (uncached_dmic_active_fifos_mask == 0) { /* CIC */ @@ -1657,7 +1660,10 @@ static int dmic_remove(struct dai *dai) interrupt_disable(dmic->irq, dai); interrupt_unregister(dmic->irq, dai); - /* The next end tasks must be passed if another DAI FIFO still runs */ + /* The next end tasks must be passed if another DAI FIFO still runs. + * Note: dai_put() function that calls remove() applies the spinlock + * so it is not needed here to protect access to mask bits. + */ dai_info(dai, "dmic_remove(), dmic_active_fifos_mask = 0x%x, dmic_pause_mask = 0x%x", uncached_dmic_active_fifos_mask, uncached_dmic_pause_mask); if (uncached_dmic_active_fifos_mask || uncached_dmic_pause_mask) From b4f7438113898550f5a0132bbd8f4eef63b8de8e Mon Sep 17 00:00:00 2001 From: Seppo Ingalsuo Date: Fri, 4 Jun 2021 17:27:57 +0300 Subject: [PATCH 09/13] Drivers: Intel: DMIC: Re-arrange DMIC DAI component data This patch collects the global DMIC driver parameters into single struct. The pointer global in every DMIC DAI instance points into the same data. All access from functions happens via the pointer that simplifies the next step to add NHLT binary data configuration for the driver. The previous dmic_prm[] is no more allocated dynamically since the single data structure is fixed size and not large. It simplifies the driver code that handles the multiple ownership. Signed-off-by: Seppo Ingalsuo --- src/drivers/intel/dmic.c | 227 ++++++++++++++------------------- src/include/sof/drivers/dmic.h | 23 +++- 2 files changed, 112 insertions(+), 138 deletions(-) diff --git a/src/drivers/intel/dmic.c b/src/drivers/intel/dmic.c index 92619763741c..108dcfcb049f 100644 --- a/src/drivers/intel/dmic.c +++ b/src/drivers/intel/dmic.c @@ -134,10 +134,8 @@ static const uint32_t coef_base_a[4] = {PDM0_COEFFICIENT_A, PDM1_COEFFICIENT_A, static const uint32_t coef_base_b[4] = {PDM0_COEFFICIENT_B, PDM1_COEFFICIENT_B, PDM2_COEFFICIENT_B, PDM3_COEFFICIENT_B}; -/* Global configuration request for DMIC, need to use uncached address to access them */ -static SHARED_DATA struct sof_ipc_dai_dmic_params *dmic_prm[DMIC_HW_FIFOS]; -static SHARED_DATA uint32_t dmic_active_fifos_mask; -static SHARED_DATA uint32_t dmic_pause_mask; +/* Global configuration request and state for DMIC */ +static SHARED_DATA struct dmic_global_shared dmic_global; /* this ramps volume changes over time */ static enum task_state dmic_work(void *data) @@ -226,7 +224,7 @@ static enum task_state dmic_work(void *data) static void find_modes(struct dai *dai, struct decim_modes *modes, uint32_t fs, int di) { - struct sof_ipc_dai_dmic_params **uncached_dmic_prm = cache_to_uncache(&dmic_prm[0]); + struct dmic_pdata *dmic = dai_get_drvdata(dai); int clkdiv_min; int clkdiv_max; int clkdiv; @@ -258,37 +256,37 @@ static void find_modes(struct dai *dai, osr_min = DMIC_HIGH_RATE_OSR_MIN; /* Check for sane pdm clock, min 100 kHz, max ioclk/2 */ - if (uncached_dmic_prm[di]->pdmclk_max < DMIC_HW_PDM_CLK_MIN || - uncached_dmic_prm[di]->pdmclk_max > DMIC_HW_IOCLK / 2) { + if (dmic->global->prm[di].pdmclk_max < DMIC_HW_PDM_CLK_MIN || + dmic->global->prm[di].pdmclk_max > DMIC_HW_IOCLK / 2) { dai_err(dai, "find_modes(): pdm clock max not in range"); return; } - if (uncached_dmic_prm[di]->pdmclk_min < DMIC_HW_PDM_CLK_MIN || - uncached_dmic_prm[di]->pdmclk_min > uncached_dmic_prm[di]->pdmclk_max) { + if (dmic->global->prm[di].pdmclk_min < DMIC_HW_PDM_CLK_MIN || + dmic->global->prm[di].pdmclk_min > dmic->global->prm[di].pdmclk_max) { dai_err(dai, "find_modes(): pdm clock min not in range"); return; } /* Check for sane duty cycle */ - if (uncached_dmic_prm[di]->duty_min > uncached_dmic_prm[di]->duty_max) { + if (dmic->global->prm[di].duty_min > dmic->global->prm[di].duty_max) { dai_err(dai, "find_modes(): duty cycle min > max"); return; } - if (uncached_dmic_prm[di]->duty_min < DMIC_HW_DUTY_MIN || - uncached_dmic_prm[di]->duty_min > DMIC_HW_DUTY_MAX) { + if (dmic->global->prm[di].duty_min < DMIC_HW_DUTY_MIN || + dmic->global->prm[di].duty_min > DMIC_HW_DUTY_MAX) { dai_err(dai, "find_modes(): pdm clock min not in range"); return; } - if (uncached_dmic_prm[di]->duty_max < DMIC_HW_DUTY_MIN || - uncached_dmic_prm[di]->duty_max > DMIC_HW_DUTY_MAX) { + if (dmic->global->prm[di].duty_max < DMIC_HW_DUTY_MIN || + dmic->global->prm[di].duty_max > DMIC_HW_DUTY_MAX) { dai_err(dai, "find_modes(): pdm clock max not in range"); return; } /* Min and max clock dividers */ - clkdiv_min = ceil_divide(DMIC_HW_IOCLK, uncached_dmic_prm[di]->pdmclk_max); + clkdiv_min = ceil_divide(DMIC_HW_IOCLK, dmic->global->prm[di].pdmclk_max); clkdiv_min = MAX(clkdiv_min, DMIC_HW_CIC_DECIM_MIN); - clkdiv_max = DMIC_HW_IOCLK / uncached_dmic_prm[di]->pdmclk_min; + clkdiv_max = DMIC_HW_IOCLK / dmic->global->prm[di].pdmclk_min; /* Loop possible clock dividers and check based on resulting * oversampling ratio that CIC and FIR decimation ratios are @@ -311,8 +309,8 @@ static void find_modes(struct dai *dai, * not exceed microphone specification. If exceed proceed to * next clkdiv. */ - if (osr < osr_min || du_min < uncached_dmic_prm[di]->duty_min || - du_max > uncached_dmic_prm[di]->duty_max) + if (osr < osr_min || du_min < dmic->global->prm[di].duty_min || + du_max > dmic->global->prm[di].duty_max) continue; /* Loop FIR decimation factors candidates. If the @@ -673,9 +671,8 @@ static int select_mode(struct dai *dai, * value to use. */ -static inline void ipm_helper1(int *ipm, int di) +static inline void ipm_helper1(struct dmic_pdata *dmic, int *ipm, int di) { - struct sof_ipc_dai_dmic_params **uncached_dmic_prm = cache_to_uncache(&dmic_prm[0]); int pdm[DMIC_HW_CONTROLLERS]; int i; @@ -684,8 +681,8 @@ static inline void ipm_helper1(int *ipm, int di) * this DAI. */ for (i = 0; i < DMIC_HW_CONTROLLERS; i++) { - if (uncached_dmic_prm[di]->pdm[i].enable_mic_a || - uncached_dmic_prm[di]->pdm[i].enable_mic_b) + if (dmic->global->prm[di].pdm[i].enable_mic_a || + dmic->global->prm[di].pdm[i].enable_mic_b) pdm[i] = 1; else pdm[i] = 0; @@ -703,9 +700,8 @@ static inline void ipm_helper1(int *ipm, int di) #if DMIC_HW_VERSION >= 2 -static inline void ipm_helper2(int source[], int *ipm, int di) +static inline void ipm_helper2(struct dmic_pdata *dmic, int source[], int *ipm, int di) { - struct sof_ipc_dai_dmic_params **uncached_dmic_prm = cache_to_uncache(&dmic_prm[0]); int pdm[DMIC_HW_CONTROLLERS]; int i; int n = 0; @@ -719,8 +715,8 @@ static inline void ipm_helper2(int source[], int *ipm, int di) * pdm controllers to be used for IPM configuration. */ for (i = 0; i < DMIC_HW_CONTROLLERS; i++) { - if (uncached_dmic_prm[di]->pdm[i].enable_mic_a || - uncached_dmic_prm[di]->pdm[i].enable_mic_b) { + if (dmic->global->prm[di].pdm[i].enable_mic_a || + dmic->global->prm[di].pdm[i].enable_mic_b) { pdm[i] = 1; source[n] = i; n++; @@ -741,9 +737,8 @@ static inline void ipm_helper2(int source[], int *ipm, int di) * or mono right (B) mode. Mono right mode is setup as channel * swapped mono left. */ -static int stereo_helper(int stereo[], int swap[]) +static int stereo_helper(struct dmic_pdata *dmic, int stereo[], int swap[]) { - struct sof_ipc_dai_dmic_params **uncached_dmic_prm = cache_to_uncache(&dmic_prm[0]); int cnt; int i; int swap_check; @@ -751,12 +746,12 @@ static int stereo_helper(int stereo[], int swap[]) for (i = 0; i < DMIC_HW_CONTROLLERS; i++) { cnt = 0; - if (uncached_dmic_prm[0]->pdm[i].enable_mic_a || - uncached_dmic_prm[1]->pdm[i].enable_mic_a) + if (dmic->global->prm[0].pdm[i].enable_mic_a || + dmic->global->prm[1].pdm[i].enable_mic_a) cnt++; - if (uncached_dmic_prm[0]->pdm[i].enable_mic_b || - uncached_dmic_prm[1]->pdm[i].enable_mic_b) + if (dmic->global->prm[0].pdm[i].enable_mic_b || + dmic->global->prm[1].pdm[i].enable_mic_b) cnt++; /* Set stereo mode if both mic A anc B are enabled. */ @@ -764,12 +759,12 @@ static int stereo_helper(int stereo[], int swap[]) stereo[i] = cnt; /* Swap channels if only mic B is used for mono processing. */ - swap[i] = (uncached_dmic_prm[0]->pdm[i].enable_mic_b || - uncached_dmic_prm[1]->pdm[i].enable_mic_b) && !cnt; + swap[i] = (dmic->global->prm[0].pdm[i].enable_mic_b || + dmic->global->prm[1].pdm[i].enable_mic_b) && !cnt; /* Check that swap does not conflict with other DAI request */ - swap_check = uncached_dmic_prm[1]->pdm[i].enable_mic_a || - uncached_dmic_prm[0]->pdm[i].enable_mic_a; + swap_check = dmic->global->prm[1].pdm[i].enable_mic_a || + dmic->global->prm[0].pdm[i].enable_mic_a; if (swap_check && swap[i]) ret = -EINVAL; @@ -780,8 +775,7 @@ static int stereo_helper(int stereo[], int swap[]) static int configure_registers(struct dai *dai, struct dmic_configuration *cfg) { - struct sof_ipc_dai_dmic_params **prm_t = cache_to_uncache(&dmic_prm[0]); - int uncached_dmic_active_fifos_mask = *cache_to_uncache(&dmic_active_fifos_mask); + struct dmic_pdata *dmic = dai_get_drvdata(dai); int stereo[DMIC_HW_CONTROLLERS]; int swap[DMIC_HW_CONTROLLERS]; uint32_t val; @@ -801,8 +795,9 @@ static int configure_registers(struct dai *dai, int i; int j; int ret; + int pol_a; + int pol_b; int di = dai->index; - struct dmic_pdata *pdata = dai_get_drvdata(dai); int dccomp = 1; int array_a = 0; int array_b = 0; @@ -819,7 +814,7 @@ static int configure_registers(struct dai *dai, #endif /* pdata is set by dmic_probe(), error if it has not been set */ - if (!pdata) { + if (!dmic) { dai_err(dai, "configure_registers(): pdata not set"); return -EINVAL; } @@ -827,17 +822,17 @@ static int configure_registers(struct dai *dai, dai_info(dai, "configuring registers"); /* OUTCONTROL0 and OUTCONTROL1 */ - of0 = (prm_t[0]->fifo_bits == 32) ? 2 : 0; + of0 = (dmic->global->prm[0].fifo_bits == 32) ? 2 : 0; #if DMIC_HW_FIFOS > 1 - of1 = (prm_t[1]->fifo_bits == 32) ? 2 : 0; + of1 = (dmic->global->prm[1].fifo_bits == 32) ? 2 : 0; #else of1 = 0; #endif #if DMIC_HW_VERSION == 1 || (DMIC_HW_VERSION == 2 && DMIC_HW_CONTROLLERS <= 2) if (di == 0) { - ipm_helper1(&ipm, 0); + ipm_helper1(dmic, &ipm, 0); val = OUTCONTROL0_TIE(0) | OUTCONTROL0_SIP(0) | OUTCONTROL0_FINIT(1) | @@ -849,7 +844,7 @@ static int configure_registers(struct dai *dai, dai_write(dai, OUTCONTROL0, val); dai_dbg(dai, "configure_registers(), OUTCONTROL0 = %08x", val); } else { - ipm_helper1(&ipm, 1); + ipm_helper1(dmic, &ipm, 1); val = OUTCONTROL1_TIE(0) | OUTCONTROL1_SIP(0) | OUTCONTROL1_FINIT(1) | @@ -865,7 +860,7 @@ static int configure_registers(struct dai *dai, #if DMIC_HW_VERSION == 3 || (DMIC_HW_VERSION == 2 && DMIC_HW_CONTROLLERS > 2) if (di == 0) { - ipm_helper2(source, &ipm, 0); + ipm_helper2(dmic, source, &ipm, 0); val = OUTCONTROL0_TIE(0) | OUTCONTROL0_SIP(0) | OUTCONTROL0_FINIT(1) | @@ -881,7 +876,7 @@ static int configure_registers(struct dai *dai, dai_write(dai, OUTCONTROL0, val); dai_dbg(dai, "configure_registers(), OUTCONTROL0 = %08x", val); } else { - ipm_helper2(source, &ipm, 1); + ipm_helper2(dmic, source, &ipm, 1); val = OUTCONTROL1_TIE(0) | OUTCONTROL1_SIP(0) | OUTCONTROL1_FINIT(1) | @@ -903,12 +898,12 @@ static int configure_registers(struct dai *dai, * for starting correct parts of the HW. */ for (i = 0; i < DMIC_HW_CONTROLLERS; i++) { - pdata->enable[i] = (prm_t[di]->pdm[i].enable_mic_b << 1) | - prm_t[di]->pdm[i].enable_mic_a; + dmic->enable[i] = (dmic->global->prm[di].pdm[i].enable_mic_b << 1) | + dmic->global->prm[di].pdm[i].enable_mic_a; } - ret = stereo_helper(stereo, swap); + ret = stereo_helper(dmic, stereo, swap); if (ret < 0) { dai_err(dai, "configure_registers(): enable conflict"); return ret; @@ -918,13 +913,15 @@ static int configure_registers(struct dai *dai, * the calling function dmic_set_config(). */ for (i = 0; i < DMIC_HW_CONTROLLERS; i++) { - if (uncached_dmic_active_fifos_mask == 0) { + if (dmic->global->active_fifos_mask == 0) { /* CIC */ + pol_a = dmic->global->prm[di].pdm[i].polarity_mic_a; + pol_b = dmic->global->prm[di].pdm[i].polarity_mic_b; val = CIC_CONTROL_SOFT_RESET(soft_reset) | CIC_CONTROL_CIC_START_B(0) | CIC_CONTROL_CIC_START_A(0) | - CIC_CONTROL_MIC_B_POLARITY(prm_t[di]->pdm[i].polarity_mic_a) | - CIC_CONTROL_MIC_A_POLARITY(prm_t[di]->pdm[i].polarity_mic_b) | + CIC_CONTROL_MIC_B_POLARITY(pol_b) | + CIC_CONTROL_MIC_A_POLARITY(pol_a) | CIC_CONTROL_MIC_MUTE(cic_mute) | CIC_CONTROL_STEREO_MODE(stereo[i]); dai_write(dai, base[i] + CIC_CONTROL, val); @@ -939,12 +936,12 @@ static int configure_registers(struct dai *dai, * since the mono decimation is done with only left channel * processing active. */ - edge = prm_t[di]->pdm[i].clk_edge; + edge = dmic->global->prm[di].pdm[i].clk_edge; if (swap[i]) edge = !edge; val = MIC_CONTROL_PDM_CLKDIV(cfg->clkdiv - 2) | - MIC_CONTROL_PDM_SKEW(prm_t[di]->pdm[i].skew) | + MIC_CONTROL_PDM_SKEW(dmic->global->prm[di].pdm[i].skew) | MIC_CONTROL_CLK_EDGE(edge) | MIC_CONTROL_PDM_EN_B(0) | MIC_CONTROL_PDM_EN_A(0); @@ -1070,18 +1067,17 @@ static int configure_registers(struct dai *dai, static int dmic_get_hw_params(struct dai *dai, struct sof_ipc_stream_params *params, int dir) { - struct sof_ipc_dai_dmic_params **uncached_dmic_prm = cache_to_uncache(&dmic_prm[0]); + struct dmic_pdata *dmic = dai_get_drvdata(dai); int di = dai->index; - if (!uncached_dmic_prm[di]) { - dai_err(dai, "dmic_get_hw_params(): dai %d not configured! &dmic_prm[di] 0x%p", - di, &uncached_dmic_prm[di]); + if (!dmic) { + dai_err(dai, "dmic_get_hw_params(): dai %d not configured!", di); return -EINVAL; } - params->rate = uncached_dmic_prm[di]->fifo_fs; + params->rate = dmic->global->prm[di].fifo_fs; params->buffer_fmt = 0; - switch (uncached_dmic_prm[di]->num_pdm_active) { + switch (dmic->global->prm[di].num_pdm_active) { case 1: params->channels = 2; break; @@ -1089,11 +1085,12 @@ static int dmic_get_hw_params(struct dai *dai, params->channels = 4; break; default: - dai_info(dai, "dmic_get_hw_params(): not supported channels amount"); + dai_info(dai, "dmic_get_hw_params(): not supported PDM active count %d", + dmic->global->prm[di].num_pdm_active); return -EINVAL; } - switch (uncached_dmic_prm[di]->fifo_bits) { + switch (dmic->global->prm[di].fifo_bits) { case 16: params->frame_fmt = SOF_IPC_FRAME_S16_LE; break; @@ -1110,7 +1107,6 @@ static int dmic_get_hw_params(struct dai *dai, static int dmic_set_config(struct dai *dai, struct sof_ipc_dai_config *config) { - struct sof_ipc_dai_dmic_params **uncached_dmic_prm = cache_to_uncache(&dmic_prm[0]); struct dmic_pdata *dmic = dai_get_drvdata(dai); struct matched_modes modes_ab; struct dmic_configuration cfg; @@ -1118,7 +1114,6 @@ static int dmic_set_config(struct dai *dai, struct sof_ipc_dai_config *config) struct decim_modes modes_b; int32_t unmute_ramp_time_ms; int32_t step_db; - size_t size; int i, j, ret = 0; int di = dai->index; @@ -1169,44 +1164,25 @@ static int dmic_set_config(struct dai *dai, struct sof_ipc_dai_config *config) * the active controllers * "prm" is initialized with default params for all HW controllers */ - if (!uncached_dmic_prm[0]) { - size = sizeof(struct sof_ipc_dai_dmic_params); - dmic_prm[0] = rzalloc(SOF_MEM_ZONE_RUNTIME_SHARED, - 0, SOF_MEM_CAPS_RAM, - DMIC_HW_FIFOS * size); - /* write it back */ - uncached_dmic_prm[0] = dmic_prm[0]; - if (!uncached_dmic_prm[0]) { - dai_err(dai, "dmic_set_config(): prm not initialized"); - ret = -ENOMEM; - goto out; - } - for (i = 1; i < DMIC_HW_FIFOS; i++) { - dmic_prm[i] = (struct sof_ipc_dai_dmic_params *) - ((uint8_t *)uncached_dmic_prm[i - 1] + size); - /* write it back */ - uncached_dmic_prm[i] = dmic_prm[i]; - } - } + assert(dmic); /* Copy the new DMIC params header (all but not pdm[]) to persistent. * The last arrived request determines the parameters. */ - ret = memcpy_s(uncached_dmic_prm[di], sizeof(*uncached_dmic_prm[di]), &config->dmic, + ret = memcpy_s(&dmic->global->prm[di], sizeof(dmic->global->prm[di]), &config->dmic, offsetof(struct sof_ipc_dai_dmic_params, pdm)); assert(!ret); /* copy the pdm controller params from ipc */ for (i = 0; i < DMIC_HW_CONTROLLERS; i++) { - uncached_dmic_prm[di]->pdm[i].id = i; + dmic->global->prm[di].pdm[i].id = i; for (j = 0; j < config->dmic.num_pdm_active; j++) { /* copy the pdm controller params id the id's match */ - if (uncached_dmic_prm[di]->pdm[i].id == config->dmic.pdm[j].id) { - ret = memcpy_s(&uncached_dmic_prm[di]->pdm[i], - sizeof(uncached_dmic_prm[di]->pdm[i]), + if (dmic->global->prm[di].pdm[i].id == config->dmic.pdm[j].id) { + ret = memcpy_s(&dmic->global->prm[di].pdm[i], + sizeof(dmic->global->prm[di].pdm[i]), &config->dmic.pdm[j], - sizeof( - struct sof_ipc_dai_dmic_pdm_ctrl)); + sizeof(struct sof_ipc_dai_dmic_pdm_ctrl)); assert(!ret); } } @@ -1215,13 +1191,13 @@ static int dmic_set_config(struct dai *dai, struct sof_ipc_dai_config *config) dai_info(dai, "dmic_set_config(), prm config->dmic.num_pdm_active = %u", config->dmic.num_pdm_active); dai_info(dai, "dmic_set_config(), prm pdmclk_min = %u, pdmclk_max = %u", - uncached_dmic_prm[di]->pdmclk_min, uncached_dmic_prm[di]->pdmclk_max); + dmic->global->prm[di].pdmclk_min, dmic->global->prm[di].pdmclk_max); dai_info(dai, "dmic_set_config(), prm duty_min = %u, duty_max = %u", - uncached_dmic_prm[di]->duty_min, uncached_dmic_prm[di]->duty_max); + dmic->global->prm[di].duty_min, dmic->global->prm[di].duty_max); dai_info(dai, "dmic_set_config(), prm fifo_fs = %u, fifo_bits = %u", - uncached_dmic_prm[di]->fifo_fs, uncached_dmic_prm[di]->fifo_bits); + dmic->global->prm[di].fifo_fs, dmic->global->prm[di].fifo_bits); - switch (uncached_dmic_prm[di]->fifo_bits) { + switch (dmic->global->prm[di].fifo_bits) { case 0: case 16: case 32: @@ -1238,15 +1214,15 @@ static int dmic_set_config(struct dai *dai, struct sof_ipc_dai_config *config) * to use for FIR coefficient RAM write as well as the CIC and FIR * shift values. */ - find_modes(dai, &modes_a, uncached_dmic_prm[0]->fifo_fs, di); - if (modes_a.num_of_modes == 0 && uncached_dmic_prm[0]->fifo_fs > 0) { + find_modes(dai, &modes_a, dmic->global->prm[0].fifo_fs, di); + if (modes_a.num_of_modes == 0 && dmic->global->prm[0].fifo_fs > 0) { dai_err(dai, "dmic_set_config(): No modes found found for FIFO A"); ret = -EINVAL; goto out; } - find_modes(dai, &modes_b, uncached_dmic_prm[1]->fifo_fs, di); - if (modes_b.num_of_modes == 0 && uncached_dmic_prm[1]->fifo_fs > 0) { + find_modes(dai, &modes_b, dmic->global->prm[1].fifo_fs, di); + if (modes_b.num_of_modes == 0 && dmic->global->prm[1].fifo_fs > 0) { dai_err(dai, "dmic_set_config(): No modes found for FIFO B"); ret = -EINVAL; goto out; @@ -1293,9 +1269,6 @@ static int dmic_set_config(struct dai *dai, struct sof_ipc_dai_config *config) /* start the DMIC for capture */ static void dmic_start(struct dai *dai) { - struct sof_ipc_dai_dmic_params **uncached_dmic_prm = cache_to_uncache(&dmic_prm[0]); - uint32_t *uncached_dmic_active_fifos_mask = cache_to_uncache(&dmic_active_fifos_mask); - uint32_t *uncached_dmic_pause_mask = cache_to_uncache(&dmic_pause_mask); struct dmic_pdata *dmic = dai_get_drvdata(dai); int i; int mic_a; @@ -1334,13 +1307,13 @@ static void dmic_start(struct dai *dai) for (i = 0; i < DMIC_HW_CONTROLLERS; i++) { mic_a = dmic->enable[i] & 1; mic_b = (dmic->enable[i] & 2) >> 1; - if (uncached_dmic_prm[0]->fifo_fs > 0) + if (dmic->global->prm[0].fifo_fs > 0) fir_a = (dmic->enable[i] > 0) ? 1 : 0; else fir_a = 0; #if DMIC_HW_FIFOS > 1 - if (uncached_dmic_prm[1]->fifo_fs > 0) + if (dmic->global->prm[1].fifo_fs > 0) fir_b = (dmic->enable[i] > 0) ? 1 : 0; else fir_b = 0; @@ -1404,9 +1377,9 @@ static void dmic_start(struct dai *dai) CIC_CONTROL_SOFT_RESET_BIT, 0); } - /* Set bit dai->index for active FIFO, and clear pause bit */ - *uncached_dmic_active_fifos_mask |= BIT(dai->index); - *uncached_dmic_pause_mask &= ~BIT(dai->index); + /* Set bit dai->index */ + dmic->global->active_fifos_mask |= BIT(dai->index); + dmic->global->pause_mask &= ~BIT(dai->index); dmic->state = COMP_STATE_ACTIVE; spin_unlock(&dai->lock); @@ -1422,7 +1395,7 @@ static void dmic_start(struct dai *dai) dai_info(dai, "dmic_start(), dmic_active_fifos_mask = 0x%x", - *uncached_dmic_active_fifos_mask); + dmic->global->active_fifos_mask); } static void dmic_stop_fifo_packers(struct dai *dai, int fifo_index) @@ -1446,8 +1419,6 @@ static void dmic_stop_fifo_packers(struct dai *dai, int fifo_index) static void dmic_stop(struct dai *dai, bool stop_is_pause) { struct dmic_pdata *dmic = dai_get_drvdata(dai); - uint32_t *uncached_dmic_active_fifos_mask = cache_to_uncache(&dmic_active_fifos_mask); - uint32_t *uncached_dmic_pause_mask = cache_to_uncache(&dmic_pause_mask); int i; dai_dbg(dai, "dmic_stop()"); @@ -1458,20 +1429,20 @@ static void dmic_stop(struct dai *dai, bool stop_is_pause) /* Set soft reset and mute on for all PDM controllers. */ dai_info(dai, "dmic_stop(), dmic_active_fifos_mask = 0x%x", - *uncached_dmic_active_fifos_mask); + dmic->global->active_fifos_mask); /* Clear bit dai->index for active FIFO. If stop for pause, set pause mask bit. * If stop is not for pausing, it is safe to clear the pause bit. */ - *uncached_dmic_active_fifos_mask &= ~BIT(dai->index); + dmic->global->active_fifos_mask &= ~BIT(dai->index); if (stop_is_pause) - *uncached_dmic_pause_mask |= BIT(dai->index); + dmic->global->pause_mask |= BIT(dai->index); else - *uncached_dmic_pause_mask &= ~BIT(dai->index); + dmic->global->pause_mask &= ~BIT(dai->index); for (i = 0; i < DMIC_HW_CONTROLLERS; i++) { /* Don't stop CIC yet if one FIFO remains active */ - if (*uncached_dmic_active_fifos_mask == 0) { + if (dmic->global->active_fifos_mask == 0) { dai_update_bits(dai, base[i] + CIC_CONTROL, CIC_CONTROL_SOFT_RESET_BIT | CIC_CONTROL_MIC_MUTE_BIT, @@ -1599,7 +1570,6 @@ static int dmic_probe(struct dai *dai) if (dai_get_drvdata(dai)) return -EEXIST; /* already created */ - /* allocate private data */ dmic = rzalloc(SOF_MEM_ZONE_RUNTIME_SHARED, 0, SOF_MEM_CAPS_RAM, sizeof(*dmic)); if (!dmic) { dai_err(dai, "dmic_probe(): alloc failed"); @@ -1607,6 +1577,9 @@ static int dmic_probe(struct dai *dai) } dai_set_drvdata(dai, dmic); + /* Common shared data for all DMIC DAI instances */ + dmic->global = platform_shared_get(&dmic_global, sizeof(dmic_global)); + /* Set state, note there is no playback direction support */ dmic->state = COMP_STATE_READY; @@ -1643,11 +1616,9 @@ static int dmic_probe(struct dai *dai) static int dmic_remove(struct dai *dai) { - struct sof_ipc_dai_dmic_params **uncached_dmic_prm = cache_to_uncache(&dmic_prm[0]); - uint32_t uncached_dmic_active_fifos_mask = *cache_to_uncache(&dmic_active_fifos_mask); - uint32_t uncached_dmic_pause_mask = *cache_to_uncache(&dmic_pause_mask); struct dmic_pdata *dmic = dai_get_drvdata(dai); - int i; + uint32_t active_fifos_mask = dmic->global->active_fifos_mask; + uint32_t pause_mask = dmic->global->pause_mask; dai_info(dai, "dmic_remove()"); @@ -1660,26 +1631,20 @@ static int dmic_remove(struct dai *dai) interrupt_disable(dmic->irq, dai); interrupt_unregister(dmic->irq, dai); + dai_info(dai, "dmic_remove(), dmic_active_fifos_mask = 0x%x, dmic_pause_mask = 0x%x", + active_fifos_mask, pause_mask); + rfree(dmic); + /* The next end tasks must be passed if another DAI FIFO still runs. * Note: dai_put() function that calls remove() applies the spinlock * so it is not needed here to protect access to mask bits. */ - dai_info(dai, "dmic_remove(), dmic_active_fifos_mask = 0x%x, dmic_pause_mask = 0x%x", - uncached_dmic_active_fifos_mask, uncached_dmic_pause_mask); - if (uncached_dmic_active_fifos_mask || uncached_dmic_pause_mask) + if (active_fifos_mask || pause_mask) return 0; + /* Disable DMIC clock and power */ pm_runtime_put_sync(DMIC_CLK, dai->index); - /* Disable DMIC power */ pm_runtime_put_sync(DMIC_POW, dai->index); - - rfree(uncached_dmic_prm[0]); - for (i = 0; i < DMIC_HW_FIFOS; i++) { - dmic_prm[i] = NULL; - /* write it back */ - uncached_dmic_prm[i] = dmic_prm[i]; - } - return 0; } diff --git a/src/include/sof/drivers/dmic.h b/src/include/sof/drivers/dmic.h index 3c3448c1cbce..c0640ddd8e63 100644 --- a/src/include/sof/drivers/dmic.h +++ b/src/include/sof/drivers/dmic.h @@ -50,6 +50,7 @@ #if DMIC_HW_VERSION +#include #include #include #include @@ -315,15 +316,23 @@ #define dmic_irq(dmic) dmic->plat_data.irq #define dmic_irq_name(dmic) dmic->plat_data.irq_name +struct dmic_global_shared { + struct sof_ipc_dai_dmic_params prm[DMIC_HW_FIFOS]; /* Configuration requests */ + uint32_t active_fifos_mask; /* Bits (dai->index) are set to indicate active FIFO */ + uint32_t pause_mask; /* Bits (dai->index) are set to indicate driver pause */ +}; + /* DMIC private data */ struct dmic_pdata { - uint16_t enable[DMIC_HW_CONTROLLERS]; - uint32_t state; - struct task dmicwork; - int32_t startcount; - int32_t gain; - int32_t gain_coef; - int irq; + struct dmic_global_shared *global; /* Common data for all DMIC DAI instances */ + struct task dmicwork; /* HW gain ramp update task */ + uint16_t enable[DMIC_HW_CONTROLLERS]; /* Mic 0 and 1 enable bits array for PDMx */ + uint32_t state; /* Driver component state */ + int32_t startcount; /* Counter in dmicwork that controls HW unmute */ + int32_t gain_coef; /* Gain update constant */ + int32_t gain; /* Gain value to be applied to HW */ + int irq; /* Interrupt number used */ + }; extern const struct dai_driver dmic_driver; From 1cfee18bd701e58b42080329dfd98013905fbc47 Mon Sep 17 00:00:00 2001 From: Seppo Ingalsuo Date: Wed, 16 Jun 2021 13:15:00 +0300 Subject: [PATCH 10/13] Drivers: Intel: DMIC: Run dmic_work() in the same core as other driver Previously the gain ramp updates were executed in core 0 only even if rest of driver was executed in other core. It caused issues with driver state variables in cached memory. Signed-off-by: Seppo Ingalsuo --- src/drivers/intel/dmic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/drivers/intel/dmic.c b/src/drivers/intel/dmic.c index 108dcfcb049f..f71deb45b4a8 100644 --- a/src/drivers/intel/dmic.c +++ b/src/drivers/intel/dmic.c @@ -1601,7 +1601,7 @@ static int dmic_probe(struct dai *dai) /* Initialize start sequence handler */ schedule_task_init_ll(&dmic->dmicwork, SOF_UUID(dmic_work_task_uuid), SOF_SCHEDULE_LL_TIMER, - SOF_TASK_PRI_MED, dmic_work, dai, 0, 0); + SOF_TASK_PRI_MED, dmic_work, dai, cpu_get_id(), 0); /* Enable DMIC power */ pm_runtime_get_sync(DMIC_POW, dai->index); From f47fe82721f453786d189235bf1376afcb4c3b6b Mon Sep 17 00:00:00 2001 From: Seppo Ingalsuo Date: Wed, 16 Jun 2021 13:17:10 +0300 Subject: [PATCH 11/13] Drivers: Intel: DMIC: Code lines cleanup This patch only removes some unnecessary line feeds for better readability. Signed-off-by: Seppo Ingalsuo --- src/drivers/intel/dmic.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/drivers/intel/dmic.c b/src/drivers/intel/dmic.c index f71deb45b4a8..1dbd3f3f9bde 100644 --- a/src/drivers/intel/dmic.c +++ b/src/drivers/intel/dmic.c @@ -159,8 +159,7 @@ static enum task_state dmic_work(void *data) * Gain is Q2.30 and gain modifier is Q12.20. */ dmic->startcount++; - dmic->gain = q_multsr_sat_32x32(dmic->gain, dmic->gain_coef, - Q_SHIFT_GAIN_X_GAIN_COEF); + dmic->gain = q_multsr_sat_32x32(dmic->gain, dmic->gain_coef, Q_SHIFT_GAIN_X_GAIN_COEF); /* Gain is stored as Q2.30, while HW register is Q1.19 so shift * the value right by 11. @@ -1607,10 +1606,7 @@ static int dmic_probe(struct dai *dai) pm_runtime_get_sync(DMIC_POW, dai->index); /* Disable dynamic clock gating for dmic before touching any reg */ pm_runtime_get_sync(DMIC_CLK, dai->index); - interrupt_enable(dmic->irq, dai); - - return 0; } From 1f71b502df4bf4ef6eb13636338780433adf0435 Mon Sep 17 00:00:00 2001 From: Ranjani Sridharan Date: Sun, 27 Jun 2021 15:38:45 -0700 Subject: [PATCH 12/13] drivers: intel: dmic: fix double free dmic_remove() Disable dmic interrupts first followed freeing by the dmic task and finally, free the dmic pdata. Signed-off-by: Ranjani Sridharan --- src/drivers/intel/dmic.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/drivers/intel/dmic.c b/src/drivers/intel/dmic.c index 1dbd3f3f9bde..e30034ad4161 100644 --- a/src/drivers/intel/dmic.c +++ b/src/drivers/intel/dmic.c @@ -1618,17 +1618,15 @@ static int dmic_remove(struct dai *dai) dai_info(dai, "dmic_remove()"); - /* remove scheduling */ - schedule_task_free(&dmic->dmicwork); - - rfree(dai_get_drvdata(dai)); - dai_set_drvdata(dai, NULL); - interrupt_disable(dmic->irq, dai); interrupt_unregister(dmic->irq, dai); + /* remove scheduling */ + schedule_task_free(&dmic->dmicwork); + dai_info(dai, "dmic_remove(), dmic_active_fifos_mask = 0x%x, dmic_pause_mask = 0x%x", active_fifos_mask, pause_mask); + dai_set_drvdata(dai, NULL); rfree(dmic); /* The next end tasks must be passed if another DAI FIFO still runs. From c33e8f7a6b7dd9a5c10e0f243a3f9068cd4f42a8 Mon Sep 17 00:00:00 2001 From: Seppo Ingalsuo Date: Mon, 12 Apr 2021 15:54:29 +0300 Subject: [PATCH 13/13] Drivers: Intel: DMIC: Move driver code to subdirectory dmic In preparation for more functionality the module is moved to own directory similarly as other DAI drivers. Signed-off-by: Seppo Ingalsuo --- src/drivers/intel/CMakeLists.txt | 2 +- src/drivers/intel/dmic/CMakeLists.txt | 3 +++ src/drivers/intel/{ => dmic}/dmic.c | 0 zephyr/CMakeLists.txt | 8 ++++---- 4 files changed, 8 insertions(+), 5 deletions(-) create mode 100644 src/drivers/intel/dmic/CMakeLists.txt rename src/drivers/intel/{ => dmic}/dmic.c (100%) diff --git a/src/drivers/intel/CMakeLists.txt b/src/drivers/intel/CMakeLists.txt index aed842ce4b7f..265369dbed72 100644 --- a/src/drivers/intel/CMakeLists.txt +++ b/src/drivers/intel/CMakeLists.txt @@ -22,5 +22,5 @@ if(CONFIG_INTEL_ALH) endif() if(CONFIG_INTEL_DMIC) - add_local_sources(sof dmic.c) + add_subdirectory(dmic) endif() diff --git a/src/drivers/intel/dmic/CMakeLists.txt b/src/drivers/intel/dmic/CMakeLists.txt new file mode 100644 index 000000000000..14b33ab5d6b7 --- /dev/null +++ b/src/drivers/intel/dmic/CMakeLists.txt @@ -0,0 +1,3 @@ +# SPDX-License-Identifier: BSD-3-Clause + +add_local_sources(sof dmic.c) diff --git a/src/drivers/intel/dmic.c b/src/drivers/intel/dmic/dmic.c similarity index 100% rename from src/drivers/intel/dmic.c rename to src/drivers/intel/dmic/dmic.c diff --git a/zephyr/CMakeLists.txt b/zephyr/CMakeLists.txt index 7ff591b26abb..b824985087a8 100644 --- a/zephyr/CMakeLists.txt +++ b/zephyr/CMakeLists.txt @@ -132,7 +132,7 @@ if (CONFIG_SOC_SERIES_INTEL_CAVS_V15) ) zephyr_library_sources_ifdef(CONFIG_INTEL_DMIC - ${SOF_DRIVERS_PATH}/intel/dmic.c + ${SOF_DRIVERS_PATH}/intel/dmic/dmic.c ) # Platform sources @@ -185,7 +185,7 @@ if (CONFIG_SOC_SERIES_INTEL_CAVS_V18) ) zephyr_library_sources_ifdef(CONFIG_INTEL_DMIC - ${SOF_DRIVERS_PATH}/intel/dmic.c + ${SOF_DRIVERS_PATH}/intel/dmic/dmic.c ) # Platform sources @@ -240,7 +240,7 @@ if (CONFIG_SOC_SERIES_INTEL_CAVS_V20) ) zephyr_library_sources_ifdef(CONFIG_INTEL_DMIC - ${SOF_DRIVERS_PATH}/intel/dmic.c + ${SOF_DRIVERS_PATH}/intel/dmic/dmic.c ) # Platform sources @@ -297,7 +297,7 @@ if (CONFIG_SOC_SERIES_INTEL_CAVS_V25) ) zephyr_library_sources_ifdef(CONFIG_INTEL_DMIC - ${SOF_DRIVERS_PATH}/intel/dmic.c + ${SOF_DRIVERS_PATH}/intel/dmic/dmic.c ) # Platform sources