Re: [PATCH v5 7/7] ASoC: fsl_easrc: Add EASRC ASoC CPU DAI drivers
A few small comments -- trying to improve readability.
On Mon, Mar 09, 2020 at 11:58:34AM +0800, Shengjiu Wang wrote:
> EASRC (Enhanced Asynchronous Sample Rate Converter) is a new IP module
> found on i.MX8MN. It is different with old ASRC module.
>
> The primary features for the EASRC are as follows:
> - 4 Contexts - groups of channels with an independent time base
> - Fully independent and concurrent context control
> - Simultaneous processing of up to 32 audio channels
> - Programmable filter charachteristics for each context
> - 32, 24, 20, and 16-bit fixed point audio sample support
> - 32-bit floating point audio sample support
> - 8kHz to 384kHz sample rate
> - 1/16 to 8x sample rate conversion ratio
>
> Signed-off-by: Shengjiu Wang
> Signed-off-by: Cosmin-Gabriel Samoila
> ---
> sound/soc/fsl/Kconfig | 11 +
> sound/soc/fsl/Makefile|2 +
> sound/soc/fsl/fsl_easrc.c | 2111 +
> sound/soc/fsl/fsl_easrc.h | 651
> 4 files changed, 2775 insertions(+)
> create mode 100644 sound/soc/fsl/fsl_easrc.c
> create mode 100644 sound/soc/fsl/fsl_easrc.h
> diff --git a/sound/soc/fsl/fsl_easrc.c b/sound/soc/fsl/fsl_easrc.c
> +static int fsl_easrc_resampler_config(struct fsl_asrc *easrc)
> +{
> + struct device *dev = &easrc->pdev->dev;
> + struct fsl_easrc_priv *easrc_priv = easrc->private;
> + struct asrc_firmware_hdr *hdr = easrc_priv->firmware_hdr;
> + struct interp_params *interp = easrc_priv->interp;
> + struct interp_params *selected_interp = NULL;
> + unsigned int num_coeff;
> + unsigned int i;
> + u64 *arr;
> + u32 *r;
> + int ret;
> +
> + if (!hdr) {
> + dev_err(dev, "firmware not loaded!\n");
> + return -ENODEV;
> + }
> +
> + for (i = 0; i < hdr->interp_scen; i++) {
> + if ((interp[i].num_taps - 1) ==
> + bits_taps_to_val(easrc_priv->rs_num_taps)) {
Could fit everything under 80 characters:
+ if ((interp[i].num_taps - 1) !=
+ bits_taps_to_val(easrc_priv->rs_num_taps))
+ continue;
+
+ arr = interp[i].coeff;
+ selected_interp = &interp[i];
+ dev_dbg(dev, "Selected interp_filter: %u taps - %u phases\n",
+ selected_interp->num_taps,
+ selected_interp->num_phases);
+ break;
> +static int fsl_easrc_normalize_filter(struct fsl_asrc *easrc,
> + u64 *infilter,
> + u64 *outfilter,
> + int shift)
> +{
> + struct device *dev = &easrc->pdev->dev;
> + u64 coef = *infilter;
> + s64 exp = (coef & 0x7ff0ll) >> 52;
Hmm...by masking 0x7ff0ll, MSB (sign bit) is gone.
Would the result still possibly be a negative value?
> + /*
> + * If exponent is zero (value == 0), or 7ff (value == NaNs)
> + * dont touch the content
> + */
> + if (((coef & 0x7ff0ll) == 0) ||
> + ((coef & 0x7ff0ll) == ((u64)0x7ff << 52))) {
You have extracted "exp" already:
+ if (exp == 0 || (u64)exp & 0x7ff == 0x7ff)
> + *outfilter = coef;
Could simply a bit by returning here:
+ return 0;
+ }
Then:
+ /* coef * 2^shift == exp + shift */
+ exp += shift;
+
+ if ((shift > 0 && exp >= 2047) || (shift < 0 && exp <= 0)) {
+ dev_err(dev, "coef out of range\n");
+ return -EINVAL;
+ }
+
+ outcoef = (u64)(coef & 0x800Fll) + ((u64)exp << 52);
+ *outfilter = outcoef;
> +static int fsl_easrc_write_pf_coeff_mem(struct fsl_asrc *easrc, int ctx_id,
> + u64 *arr, int n_taps, int shift)
> +{
> + if (!arr) {
> + dev_err(dev, "NULL buffer\n");
Could it be slightly more specific?
> +static int fsl_easrc_prefilter_config(struct fsl_asrc *easrc,
> + unsigned int ctx_id)
> +{
> + ctx_priv->in_filled_sample = bits_taps_to_val(easrc_priv->rs_num_taps)
> / 2;
> + ctx_priv->out_missed_sample = ctx_priv->in_filled_sample *
> + ctx_priv->out_params.sample_rate /
> + ctx_priv->in_params.sample_rate;
There are quite a few references to sample_rate and sample_format
here, so we could use some local variables to cache them:
+ in_s_rate = ctx_priv->in_params.sample_rate;
+ out_s_rate = ctx_priv->out_params.sample_rate;
+ in_s_fmt = ctx_priv->in_params.sample_format;
+ out_s_fmt = ctx_priv->out_params.sample_format;
> +static int fsl_easrc_config_slot(struct fsl_asrc *easrc, unsigned int ctx_id)
> +{
> + struct fsl_easrc_priv *easrc_priv = easrc->private;
> + struct fsl_asrc_pair *ctx = easrc->pair[ctx_id];
> + int req_channels = ctx-
[PATCH v5 7/7] ASoC: fsl_easrc: Add EASRC ASoC CPU DAI drivers
EASRC (Enhanced Asynchronous Sample Rate Converter) is a new IP module
found on i.MX8MN. It is different with old ASRC module.
The primary features for the EASRC are as follows:
- 4 Contexts - groups of channels with an independent time base
- Fully independent and concurrent context control
- Simultaneous processing of up to 32 audio channels
- Programmable filter charachteristics for each context
- 32, 24, 20, and 16-bit fixed point audio sample support
- 32-bit floating point audio sample support
- 8kHz to 384kHz sample rate
- 1/16 to 8x sample rate conversion ratio
Signed-off-by: Shengjiu Wang
Signed-off-by: Cosmin-Gabriel Samoila
---
sound/soc/fsl/Kconfig | 11 +
sound/soc/fsl/Makefile|2 +
sound/soc/fsl/fsl_easrc.c | 2111 +
sound/soc/fsl/fsl_easrc.h | 651
4 files changed, 2775 insertions(+)
create mode 100644 sound/soc/fsl/fsl_easrc.c
create mode 100644 sound/soc/fsl/fsl_easrc.h
diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig
index 65e8cd4be930..ea7b4787a8af 100644
--- a/sound/soc/fsl/Kconfig
+++ b/sound/soc/fsl/Kconfig
@@ -84,6 +84,17 @@ config SND_SOC_FSL_MICFIL
Say Y if you want to add Pulse Density Modulation microphone
interface (MICFIL) support for NXP.
+config SND_SOC_FSL_EASRC
+ tristate "Enhanced Asynchronous Sample Rate Converter (EASRC) module
support"
+ depends on SND_SOC_FSL_ASRC
+ select REGMAP_MMIO
+ select SND_SOC_GENERIC_DMAENGINE_PCM
+ help
+ Say Y if you want to add Enhanced ASRC support for NXP. The ASRC is
+ a digital module that converts audio from a source sample rate to a
+ destination sample rate. It is a new design module compare with the
+ old ASRC.
+
config SND_SOC_FSL_UTILS
tristate
diff --git a/sound/soc/fsl/Makefile b/sound/soc/fsl/Makefile
index 8cde88c72d93..b835eebf8825 100644
--- a/sound/soc/fsl/Makefile
+++ b/sound/soc/fsl/Makefile
@@ -24,6 +24,7 @@ snd-soc-fsl-micfil-objs := fsl_micfil.o
snd-soc-fsl-utils-objs := fsl_utils.o
snd-soc-fsl-dma-objs := fsl_dma.o
snd-soc-fsl-mqs-objs := fsl_mqs.o
+snd-soc-fsl-easrc-objs := fsl_easrc.o
obj-$(CONFIG_SND_SOC_FSL_AUDMIX) += snd-soc-fsl-audmix.o
obj-$(CONFIG_SND_SOC_FSL_ASOC_CARD) += snd-soc-fsl-asoc-card.o
@@ -35,6 +36,7 @@ obj-$(CONFIG_SND_SOC_FSL_ESAI) += snd-soc-fsl-esai.o
obj-$(CONFIG_SND_SOC_FSL_MICFIL) += snd-soc-fsl-micfil.o
obj-$(CONFIG_SND_SOC_FSL_UTILS) += snd-soc-fsl-utils.o
obj-$(CONFIG_SND_SOC_FSL_MQS) += snd-soc-fsl-mqs.o
+obj-$(CONFIG_SND_SOC_FSL_EASRC) += snd-soc-fsl-easrc.o
obj-$(CONFIG_SND_SOC_POWERPC_DMA) += snd-soc-fsl-dma.o
# MPC5200 Platform Support
diff --git a/sound/soc/fsl/fsl_easrc.c b/sound/soc/fsl/fsl_easrc.c
new file mode 100644
index ..52c76760de0e
--- /dev/null
+++ b/sound/soc/fsl/fsl_easrc.c
@@ -0,0 +1,2111 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright 2019 NXP
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "fsl_easrc.h"
+#include "imx-pcm.h"
+
+#define FSL_EASRC_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | \
+SNDRV_PCM_FMTBIT_U16_LE | \
+SNDRV_PCM_FMTBIT_S24_LE | \
+SNDRV_PCM_FMTBIT_S24_3LE | \
+SNDRV_PCM_FMTBIT_U24_LE | \
+SNDRV_PCM_FMTBIT_U24_3LE | \
+SNDRV_PCM_FMTBIT_S32_LE | \
+SNDRV_PCM_FMTBIT_U32_LE | \
+SNDRV_PCM_FMTBIT_S20_3LE | \
+SNDRV_PCM_FMTBIT_U20_3LE | \
+SNDRV_PCM_FMTBIT_FLOAT_LE)
+
+static int fsl_easrc_iec958_put_bits(struct snd_kcontrol *kcontrol,
+struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_soc_component *comp = snd_kcontrol_chip(kcontrol);
+ struct fsl_asrc *easrc = snd_soc_component_get_drvdata(comp);
+ struct fsl_easrc_priv *easrc_priv = easrc->private;
+ struct soc_mreg_control *mc =
+ (struct soc_mreg_control *)kcontrol->private_value;
+ unsigned int regval = ucontrol->value.integer.value[0];
+
+ easrc_priv->bps_iec958[mc->regbase] = regval;
+
+ return 0;
+}
+
+static int fsl_easrc_iec958_get_bits(struct snd_kcontrol *kcontrol,
+struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_soc_component *comp = snd_kcontrol_chip(kcontrol);
+ struct fsl_asrc *easrc = snd_soc_component_get_drvdata(comp);
+ struct fsl_easrc_priv *easrc_priv = easrc->private;
+ struct soc_mreg_control *mc =
+ (struct soc_mreg_control *)kcontrol->private_va
