Re: [PATCH v2 3/3] ASoC: fsl_easrc: Add EASRC ASoC CPU DAI and platform drivers
On Wed, Feb 26, 2020 at 09:51:39AM +0800, Shengjiu Wang wrote:
> > > > > +static const struct regmap_config fsl_easrc_regmap_config = {
> > > > > + .readable_reg = fsl_easrc_readable_reg,
> > > > > + .volatile_reg = fsl_easrc_volatile_reg,
> > > > > + .writeable_reg = fsl_easrc_writeable_reg,
> > > >
> > > > Can we use regmap_range and regmap_access_table?
> > > >
> > >
> > > Can the regmap_range support discontinuous registers? The
> > > reg_stride = 4.
> >
> > I think it does. Giving an example here:
> > https://github.com/torvalds/linux/blob/master/drivers/mfd/da9063-i2c.c
>
> The register in this i2c driver are continuous, from 0x00, 0x01, 0x02...
>
> But our case is 0x00, 0x04, 0x08, does it work?
Ah...I see your point now. I am not very sure -- have only used
in I2C drivers. You can ignore if it doesn't likely work for us.
Re: [PATCH v2 3/3] ASoC: fsl_easrc: Add EASRC ASoC CPU DAI and platform drivers
On Tue, Feb 25, 2020 at 4:05 PM Nicolin Chen wrote:
>
> On Mon, Feb 24, 2020 at 08:53:25AM +, S.j. Wang wrote:
> > Hi
> >
> > > >
> > > > Signed-off-by: Shengjiu Wang
> > > > ---
> > > > sound/soc/fsl/Kconfig | 10 +
> > > > sound/soc/fsl/Makefile |2 +
> > > > sound/soc/fsl/fsl_asrc_common.h |1 +
> > > > sound/soc/fsl/fsl_easrc.c | 2265 +++
> > > > sound/soc/fsl/fsl_easrc.h | 668 +
> > > > sound/soc/fsl/fsl_easrc_dma.c | 440 ++
> > >
> > > I see a 90% similarity between fsl_asrc_dma and fsl_easrc_dma files.
> > > Would it be possible reuse the existing code? Could share structures from
> > > my point of view, just like it reuses "enum asrc_pair_index", I know
> > > differentiating "pair" and "context" is a big point here though.
> > >
> > > A possible quick solution for that, off the top of my head, could be:
> > >
> > > 1) in fsl_asrc_common.h
> > >
> > > struct fsl_asrc {
> > >
> > > };
> > >
> > > struct fsl_asrc_pair {
> > >
> > > };
> > >
> > > 2) in fsl_easrc.h
> > >
> > > /* Renaming shared structures */
> > > #define fsl_easrc fsl_asrc
> > > #define fsl_easrc_context fsl_asrc_pair
> > >
> > > May be a good idea to see if others have some opinion too.
> > >
> >
> > We need to modify the fsl_asrc and fsl_asrc_pair, let them
> > To be used by both driver, also we need to put the specific
> > Definition for each module to same struct, right?
>
> Yea. A merged structure if that doesn't look that bad. I see most
> of the fields in struct fsl_asrc are being reused by in fsl_easrc.
>
> > >
> > > > +static const struct regmap_config fsl_easrc_regmap_config = {
> > > > + .readable_reg = fsl_easrc_readable_reg,
> > > > + .volatile_reg = fsl_easrc_volatile_reg,
> > > > + .writeable_reg = fsl_easrc_writeable_reg,
> > >
> > > Can we use regmap_range and regmap_access_table?
> > >
> >
> > Can the regmap_range support discontinuous registers? The
> > reg_stride = 4.
>
> I think it does. Giving an example here:
> https://github.com/torvalds/linux/blob/master/drivers/mfd/da9063-i2c.c
The register in this i2c driver are continuous, from 0x00, 0x01, 0x02...
But our case is 0x00, 0x04, 0x08, does it work?
best regards
wang shengjiu
Re: [PATCH v2 3/3] ASoC: fsl_easrc: Add EASRC ASoC CPU DAI and platform drivers
On Mon, Feb 24, 2020 at 08:53:25AM +, S.j. Wang wrote:
> Hi
>
> > >
> > > Signed-off-by: Shengjiu Wang
> > > ---
> > > sound/soc/fsl/Kconfig | 10 +
> > > sound/soc/fsl/Makefile |2 +
> > > sound/soc/fsl/fsl_asrc_common.h |1 +
> > > sound/soc/fsl/fsl_easrc.c | 2265 +++
> > > sound/soc/fsl/fsl_easrc.h | 668 +
> > > sound/soc/fsl/fsl_easrc_dma.c | 440 ++
> >
> > I see a 90% similarity between fsl_asrc_dma and fsl_easrc_dma files.
> > Would it be possible reuse the existing code? Could share structures from
> > my point of view, just like it reuses "enum asrc_pair_index", I know
> > differentiating "pair" and "context" is a big point here though.
> >
> > A possible quick solution for that, off the top of my head, could be:
> >
> > 1) in fsl_asrc_common.h
> >
> > struct fsl_asrc {
> >
> > };
> >
> > struct fsl_asrc_pair {
> >
> > };
> >
> > 2) in fsl_easrc.h
> >
> > /* Renaming shared structures */
> > #define fsl_easrc fsl_asrc
> > #define fsl_easrc_context fsl_asrc_pair
> >
> > May be a good idea to see if others have some opinion too.
> >
>
> We need to modify the fsl_asrc and fsl_asrc_pair, let them
> To be used by both driver, also we need to put the specific
> Definition for each module to same struct, right?
Yea. A merged structure if that doesn't look that bad. I see most
of the fields in struct fsl_asrc are being reused by in fsl_easrc.
> >
> > > +static const struct regmap_config fsl_easrc_regmap_config = {
> > > + .readable_reg = fsl_easrc_readable_reg,
> > > + .volatile_reg = fsl_easrc_volatile_reg,
> > > + .writeable_reg = fsl_easrc_writeable_reg,
> >
> > Can we use regmap_range and regmap_access_table?
> >
>
> Can the regmap_range support discontinuous registers? The
> reg_stride = 4.
I think it does. Giving an example here:
https://github.com/torvalds/linux/blob/master/drivers/mfd/da9063-i2c.c
Re: [PATCH v2 3/3] ASoC: fsl_easrc: Add EASRC ASoC CPU DAI and platform drivers
Hi
> >
> > Signed-off-by: Shengjiu Wang
> > ---
> > sound/soc/fsl/Kconfig | 10 +
> > sound/soc/fsl/Makefile |2 +
> > sound/soc/fsl/fsl_asrc_common.h |1 +
> > sound/soc/fsl/fsl_easrc.c | 2265 +++
> > sound/soc/fsl/fsl_easrc.h | 668 +
> > sound/soc/fsl/fsl_easrc_dma.c | 440 ++
>
> I see a 90% similarity between fsl_asrc_dma and fsl_easrc_dma files.
> Would it be possible reuse the existing code? Could share structures from
> my point of view, just like it reuses "enum asrc_pair_index", I know
> differentiating "pair" and "context" is a big point here though.
>
> A possible quick solution for that, off the top of my head, could be:
>
> 1) in fsl_asrc_common.h
>
> struct fsl_asrc {
>
> };
>
> struct fsl_asrc_pair {
>
> };
>
> 2) in fsl_easrc.h
>
> /* Renaming shared structures */
> #define fsl_easrc fsl_asrc
> #define fsl_easrc_context fsl_asrc_pair
>
> May be a good idea to see if others have some opinion too.
>
We need to modify the fsl_asrc and fsl_asrc_pair, let them
To be used by both driver, also we need to put the specific
Definition for each module to same struct, right?
>
> > +static const struct regmap_config fsl_easrc_regmap_config = {
> > + .readable_reg = fsl_easrc_readable_reg,
> > + .volatile_reg = fsl_easrc_volatile_reg,
> > + .writeable_reg = fsl_easrc_writeable_reg,
>
> Can we use regmap_range and regmap_access_table?
>
Can the regmap_range support discontinuous registers? The
reg_stride = 4.
Best regards
Wang shengjiu
Re: [PATCH v2 3/3] ASoC: fsl_easrc: Add EASRC ASoC CPU DAI and platform drivers
On Tue, Feb 18, 2020 at 02:39:37PM +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
> ---
> sound/soc/fsl/Kconfig | 10 +
> sound/soc/fsl/Makefile |2 +
> sound/soc/fsl/fsl_asrc_common.h |1 +
> sound/soc/fsl/fsl_easrc.c | 2265 +++
> sound/soc/fsl/fsl_easrc.h | 668 +
> sound/soc/fsl/fsl_easrc_dma.c | 440 ++
I see a 90% similarity between fsl_asrc_dma and fsl_easrc_dma files.
Would it be possible reuse the existing code? Could share structures
from my point of view, just like it reuses "enum asrc_pair_index", I
know differentiating "pair" and "context" is a big point here though.
A possible quick solution for that, off the top of my head, could be:
1) in fsl_asrc_common.h
struct fsl_asrc {
};
struct fsl_asrc_pair {
};
2) in fsl_easrc.h
/* Renaming shared structures */
#define fsl_easrc fsl_asrc
#define fsl_easrc_context fsl_asrc_pair
May be a good idea to see if others have some opinion too.
> diff --git a/sound/soc/fsl/fsl_easrc.c b/sound/soc/fsl/fsl_easrc.c
> new file mode 100644
> index ..6fe2953317f2
> --- /dev/null
> +++ b/sound/soc/fsl/fsl_easrc.c
> +
> +/* set_rs_ratio
> + *
> + * According to the resample taps, calculate the resample ratio
> + */
> +static int set_rs_ratio(struct fsl_easrc_context *ctx)
"fsl_easrc_" prefix? Would be nice to have a formula in the comments.
> +/* resets the pointer of the coeff memory pointers */
> +static int fsl_coeff_mem_ptr_reset(struct fsl_easrc *easrc,
> +unsigned int ctx_id, int mem_type)
> +{
> + /* To reset the write pointer back to zero, the register field
> + * ASRC_CTX_CTRL_EXT1x[PF_COEFF_MEM_RST] can be toggled from
> + * 0x0 to 0x1 to 0x0.
> + */
Please use the style:
/*
* xxx
*/
> +static int fsl_easrc_resampler_config(struct fsl_easrc *easrc)
> +{
> + for (i = 0; i < hdr->interp_scen; i++) {
> + if ((interp[i].num_taps - 1) ==
> + bits_taps_to_val(easrc->rs_num_taps)) {
Could do below to save some indentations from the rest of the routine:
+ if ((interp[i].num_taps - 1) !=
+ bits_taps_to_val(easrc->rs_num_taps))
+ continue;
> + arr = interp[i].coeff;
> + selected_interp = [i];
> + dev_dbg(dev, "Selected interp_filter: %u taps - %u
> phases\n",
> + selected_interp->num_taps,
> + selected_interp->num_phases);
> + break;
> +/*
> + * Scale filter coefficients (64 bits float)
> + * For input float32 normalized range (1.0,-1.0) -> output int[16,24,32]:
> + * scale it by multiplying filter coefficients by 2^31
> + * For input int[16, 24, 32] -> output float32
> + * scale it by multiplying filter coefficients by 2^-15, 2^-23, 2^-31
> + * input:
> + * easrc: Structure pointer of fsl_easrc
> + * infilter : Pointer to non-scaled input filter
> + * shift: The multiply factor
> + * output:
> + * outfilter: scaled filter
> +
> */
> +static int NormalizedFilterForFloat32InIntOut(struct fsl_easrc *easrc,
> + u64 *infilter,
> + u64 *outfilter,
> + int shift)
Coding style looks very different, at comments and function naming.
> +{
> + struct device *dev = >pdev->dev;
> + u64 coef = *infilter;
> + s64 exp = (coef & 0x7ff0ll) >> 52;
> + u64 outcoef;
> +
> + /*
> + * If exponent is zero (value == 0), or 7ff (value == NaNs)
> + * dont touch the content
> + */
> + if (((coef & 0x7ff0ll) == 0) ||
> + ((coef & 0x7ff0ll) == ((u64)0x7ff << 52))) {
> + *outfilter = coef;
> + } else {
> + if ((shift > 0 && (shift + exp) >= 2047) ||
> + (shift < 0 && (exp + shift) <= 0)) {
> + dev_err(dev, "coef
[PATCH v2 3/3] ASoC: fsl_easrc: Add EASRC ASoC CPU DAI and platform 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
---
sound/soc/fsl/Kconfig | 10 +
sound/soc/fsl/Makefile |2 +
sound/soc/fsl/fsl_asrc_common.h |1 +
sound/soc/fsl/fsl_easrc.c | 2265 +++
sound/soc/fsl/fsl_easrc.h | 668 +
sound/soc/fsl/fsl_easrc_dma.c | 440 ++
6 files changed, 3386 insertions(+)
create mode 100644 sound/soc/fsl/fsl_easrc.c
create mode 100644 sound/soc/fsl/fsl_easrc.h
create mode 100644 sound/soc/fsl/fsl_easrc_dma.c
diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig
index 65e8cd4be930..21ba7642b5d0 100644
--- a/sound/soc/fsl/Kconfig
+++ b/sound/soc/fsl/Kconfig
@@ -84,6 +84,16 @@ 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"
+ 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..a5a2d08930f4 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 fsl_easrc_dma.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_asrc_common.h b/sound/soc/fsl/fsl_asrc_common.h
index 8acc55778ff2..c2056b661f15 100644
--- a/sound/soc/fsl/fsl_asrc_common.h
+++ b/sound/soc/fsl/fsl_asrc_common.h
@@ -16,6 +16,7 @@ enum asrc_pair_index {
ASRC_PAIR_A = 0,
ASRC_PAIR_B = 1,
ASRC_PAIR_C = 2,
+ ASRC_PAIR_D = 3,
};
#endif /* _FSL_ASRC_COMMON_H */
diff --git a/sound/soc/fsl/fsl_easrc.c b/sound/soc/fsl/fsl_easrc.c
new file mode 100644
index ..6fe2953317f2
--- /dev/null
+++ b/sound/soc/fsl/fsl_easrc.c
@@ -0,0 +1,2265 @@
+// 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_easrc *easrc = snd_soc_component_get_drvdata(comp);
+ struct soc_mreg_control *mc =
+ (struct soc_mreg_control *)kcontrol->private_value;
+ unsigned int regval = ucontrol->value.integer.value[0];
+
+ easrc->bps_iec958[mc->regbase] = regval;
+
+ return 0;
+}
+
+static int
