2019-07-28 19:27:10

by Daniel Baluta

[permalink] [raw]
Subject: [PATCH v2 0/7] Add support for new SAI IP version

So far SAI IPs integrated with imx6 only supported one data line.
Starting with imx7 and imx8 SAI integration support up to 8 data
lines.

New SAI IP version introduces two new registers (Version and Parmeter
registers) which are placed at the beginning of register address space.
For this reason we need to fix the register's address.

Changes since v1:
- removed patches from Lucas as they were already accepted
- addressed comments from Lucas and Nicolin regarding
device tree property naming
- removed comment saying that "datalines" must be always
consecutively enabled (this is not true, checked with IP owner)
- added new patch to document newly introduced compatbile
strings
- removed patch introducing combined mode as I will still need
some time to figure out how to properly allow users to set it.

Nicolin,

Unfortunately I couldn't find any clean solution on handling registers
address shifts. As mentioned in patch 5/7 Tx/Rx data registers and
Tx/Rx FIFO registers keep their addresses while others are shifted
by 8 bytes.

Even if I could create two regmaps as suggested I will still need
to update each call of regmap_functions.

Daniel Baluta (7):
ASoC: fsl_sai: Add registers definition for multiple datalines
ASoC: fsl_sai: Update Tx/Rx channel enable mask
ASoC: fsl_sai: Add support to enable multiple data lines
ASoC: dt-bindings: Document dl-mask property
ASoC: fsl_sai: Add support for SAI new version
ASoC: fsl_sai: Add support for imx7ulp/imx8mq
ASoC: dt-bindings: Introduce compatible strings for 7ULP and 8MQ

.../devicetree/bindings/sound/fsl-sai.txt | 10 +-
sound/soc/fsl/fsl_sai.c | 331 ++++++++++++------
sound/soc/fsl/fsl_sai.h | 82 +++--
3 files changed, 293 insertions(+), 130 deletions(-)

--
2.17.1



2019-07-28 19:27:40

by Daniel Baluta

[permalink] [raw]
Subject: [PATCH v2 3/7] ASoC: fsl_sai: Add support to enable multiple data lines

SAI supports up to 8 Rx/Tx data lines which can be enabled
using TCE/RCE bits of TCR3/RCR3 registers.

Data lines to be enabled are read from DT fsl,dl-mask property.
By default (if no DT entry is provided) only data line 0 is enabled.

Signed-off-by: Daniel Baluta <[email protected]>
---
sound/soc/fsl/fsl_sai.c | 11 ++++++++++-
sound/soc/fsl/fsl_sai.h | 4 +++-
2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index 637b1d12a575..5e7cb7fd29f5 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -601,7 +601,7 @@ static int fsl_sai_startup(struct snd_pcm_substream *substream,

regmap_update_bits(sai->regmap, FSL_SAI_xCR3(tx),
FSL_SAI_CR3_TRCE_MASK,
- FSL_SAI_CR3_TRCE);
+ FSL_SAI_CR3_TRCE(sai->soc_data->dl_mask[tx]);

ret = snd_pcm_hw_constraint_list(substream->runtime, 0,
SNDRV_PCM_HW_PARAM_RATE, &fsl_sai_rate_constraints);
@@ -888,6 +888,15 @@ static int fsl_sai_probe(struct platform_device *pdev)
}
}

+ /*
+ * active data lines mask for TX/RX, defaults to 1 (only the first
+ * data line is enabled
+ */
+ sai->dl_mask[RX] = 1;
+ sai->dl_mask[TX] = 1;
+ of_property_read_u32_index(np, "fsl,dl-mask", RX, &sai->dl_mask[RX]);
+ of_property_read_u32_index(np, "fsl,dl-mask", TX, &sai->dl_mask[TX]);
+
irq = platform_get_irq(pdev, 0);
if (irq < 0) {
dev_err(&pdev->dev, "no irq for node %s\n", pdev->name);
diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h
index 20c5b9b1e8bc..6d32f0950ec5 100644
--- a/sound/soc/fsl/fsl_sai.h
+++ b/sound/soc/fsl/fsl_sai.h
@@ -109,7 +109,7 @@
#define FSL_SAI_CR2_DIV_MASK 0xff

/* SAI Transmit and Receive Configuration 3 Register */
-#define FSL_SAI_CR3_TRCE BIT(16)
+#define FSL_SAI_CR3_TRCE(x) ((x) << 16)
#define FSL_SAI_CR3_TRCE_MASK GENMASK(23, 16)
#define FSL_SAI_CR3_WDFL(x) (x)
#define FSL_SAI_CR3_WDFL_MASK 0x1f
@@ -176,6 +176,8 @@ struct fsl_sai {
unsigned int slots;
unsigned int slot_width;

+ unsigned int dl_mask[2];
+
const struct fsl_sai_soc_data *soc_data;
struct snd_dmaengine_dai_dma_data dma_params_rx;
struct snd_dmaengine_dai_dma_data dma_params_tx;
--
2.17.1


2019-07-28 19:28:00

by Daniel Baluta

[permalink] [raw]
Subject: [PATCH v2 1/7] ASoC: fsl_sai: Add registers definition for multiple datalines

SAI IP supports up to 8 data lines. The configuration of
supported number of data lines is decided at SoC integration
time.

This patch adds definitions for all related data TX/RX registers:
* TDR0..7, Transmit data register
* TFR0..7, Transmit FIFO register
* RDR0..7, Receive data register
* RFR0..7, Receive FIFO register

Signed-off-by: Daniel Baluta <[email protected]>
---
sound/soc/fsl/fsl_sai.c | 76 +++++++++++++++++++++++++++++++++++------
sound/soc/fsl/fsl_sai.h | 36 ++++++++++++++++---
2 files changed, 98 insertions(+), 14 deletions(-)

diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index 6d3c6c8d50ce..17b0aff4ee8b 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -685,7 +685,14 @@ static struct reg_default fsl_sai_reg_defaults[] = {
{FSL_SAI_TCR3, 0},
{FSL_SAI_TCR4, 0},
{FSL_SAI_TCR5, 0},
- {FSL_SAI_TDR, 0},
+ {FSL_SAI_TDR0, 0},
+ {FSL_SAI_TDR1, 0},
+ {FSL_SAI_TDR2, 0},
+ {FSL_SAI_TDR3, 0},
+ {FSL_SAI_TDR4, 0},
+ {FSL_SAI_TDR5, 0},
+ {FSL_SAI_TDR6, 0},
+ {FSL_SAI_TDR7, 0},
{FSL_SAI_TMR, 0},
{FSL_SAI_RCR1, 0},
{FSL_SAI_RCR2, 0},
@@ -704,7 +711,14 @@ static bool fsl_sai_readable_reg(struct device *dev, unsigned int reg)
case FSL_SAI_TCR3:
case FSL_SAI_TCR4:
case FSL_SAI_TCR5:
- case FSL_SAI_TFR:
+ case FSL_SAI_TFR0:
+ case FSL_SAI_TFR1:
+ case FSL_SAI_TFR2:
+ case FSL_SAI_TFR3:
+ case FSL_SAI_TFR4:
+ case FSL_SAI_TFR5:
+ case FSL_SAI_TFR6:
+ case FSL_SAI_TFR7:
case FSL_SAI_TMR:
case FSL_SAI_RCSR:
case FSL_SAI_RCR1:
@@ -712,8 +726,22 @@ static bool fsl_sai_readable_reg(struct device *dev, unsigned int reg)
case FSL_SAI_RCR3:
case FSL_SAI_RCR4:
case FSL_SAI_RCR5:
- case FSL_SAI_RDR:
- case FSL_SAI_RFR:
+ case FSL_SAI_RDR0:
+ case FSL_SAI_RDR1:
+ case FSL_SAI_RDR2:
+ case FSL_SAI_RDR3:
+ case FSL_SAI_RDR4:
+ case FSL_SAI_RDR5:
+ case FSL_SAI_RDR6:
+ case FSL_SAI_RDR7:
+ case FSL_SAI_RFR0:
+ case FSL_SAI_RFR1:
+ case FSL_SAI_RFR2:
+ case FSL_SAI_RFR3:
+ case FSL_SAI_RFR4:
+ case FSL_SAI_RFR5:
+ case FSL_SAI_RFR6:
+ case FSL_SAI_RFR7:
case FSL_SAI_RMR:
return true;
default:
@@ -726,9 +754,30 @@ static bool fsl_sai_volatile_reg(struct device *dev, unsigned int reg)
switch (reg) {
case FSL_SAI_TCSR:
case FSL_SAI_RCSR:
- case FSL_SAI_TFR:
- case FSL_SAI_RFR:
- case FSL_SAI_RDR:
+ case FSL_SAI_TFR0:
+ case FSL_SAI_TFR1:
+ case FSL_SAI_TFR2:
+ case FSL_SAI_TFR3:
+ case FSL_SAI_TFR4:
+ case FSL_SAI_TFR5:
+ case FSL_SAI_TFR6:
+ case FSL_SAI_TFR7:
+ case FSL_SAI_RFR0:
+ case FSL_SAI_RFR1:
+ case FSL_SAI_RFR2:
+ case FSL_SAI_RFR3:
+ case FSL_SAI_RFR4:
+ case FSL_SAI_RFR5:
+ case FSL_SAI_RFR6:
+ case FSL_SAI_RFR7:
+ case FSL_SAI_RDR0:
+ case FSL_SAI_RDR1:
+ case FSL_SAI_RDR2:
+ case FSL_SAI_RDR3:
+ case FSL_SAI_RDR4:
+ case FSL_SAI_RDR5:
+ case FSL_SAI_RDR6:
+ case FSL_SAI_RDR7:
return true;
default:
return false;
@@ -744,7 +793,14 @@ static bool fsl_sai_writeable_reg(struct device *dev, unsigned int reg)
case FSL_SAI_TCR3:
case FSL_SAI_TCR4:
case FSL_SAI_TCR5:
- case FSL_SAI_TDR:
+ case FSL_SAI_TDR0:
+ case FSL_SAI_TDR1:
+ case FSL_SAI_TDR2:
+ case FSL_SAI_TDR3:
+ case FSL_SAI_TDR4:
+ case FSL_SAI_TDR5:
+ case FSL_SAI_TDR6:
+ case FSL_SAI_TDR7:
case FSL_SAI_TMR:
case FSL_SAI_RCSR:
case FSL_SAI_RCR1:
@@ -885,8 +941,8 @@ static int fsl_sai_probe(struct platform_device *pdev)
MCLK_DIR(index));
}

- sai->dma_params_rx.addr = res->start + FSL_SAI_RDR;
- sai->dma_params_tx.addr = res->start + FSL_SAI_TDR;
+ sai->dma_params_rx.addr = res->start + FSL_SAI_RDR0;
+ sai->dma_params_tx.addr = res->start + FSL_SAI_TDR0;
sai->dma_params_rx.maxburst = FSL_SAI_MAXBURST_RX;
sai->dma_params_tx.maxburst = FSL_SAI_MAXBURST_TX;

diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h
index 7c1ef671da28..4bb478041d67 100644
--- a/sound/soc/fsl/fsl_sai.h
+++ b/sound/soc/fsl/fsl_sai.h
@@ -20,8 +20,22 @@
#define FSL_SAI_TCR3 0x0c /* SAI Transmit Configuration 3 */
#define FSL_SAI_TCR4 0x10 /* SAI Transmit Configuration 4 */
#define FSL_SAI_TCR5 0x14 /* SAI Transmit Configuration 5 */
-#define FSL_SAI_TDR 0x20 /* SAI Transmit Data */
-#define FSL_SAI_TFR 0x40 /* SAI Transmit FIFO */
+#define FSL_SAI_TDR0 0x20 /* SAI Transmit Data 0 */
+#define FSL_SAI_TDR1 0x24 /* SAI Transmit Data 1 */
+#define FSL_SAI_TDR2 0x28 /* SAI Transmit Data 2 */
+#define FSL_SAI_TDR3 0x2C /* SAI Transmit Data 3 */
+#define FSL_SAI_TDR4 0x30 /* SAI Transmit Data 4 */
+#define FSL_SAI_TDR5 0x34 /* SAI Transmit Data 5 */
+#define FSL_SAI_TDR6 0x38 /* SAI Transmit Data 6 */
+#define FSL_SAI_TDR7 0x3C /* SAI Transmit Data 7 */
+#define FSL_SAI_TFR0 0x40 /* SAI Transmit FIFO 0 */
+#define FSL_SAI_TFR1 0x44 /* SAI Transmit FIFO 1 */
+#define FSL_SAI_TFR2 0x48 /* SAI Transmit FIFO 2 */
+#define FSL_SAI_TFR3 0x4C /* SAI Transmit FIFO 3 */
+#define FSL_SAI_TFR4 0x50 /* SAI Transmit FIFO 4 */
+#define FSL_SAI_TFR5 0x54 /* SAI Transmit FIFO 5 */
+#define FSL_SAI_TFR6 0x58 /* SAI Transmit FIFO 6 */
+#define FSL_SAI_TFR7 0x5C /* SAI Transmit FIFO 7 */
#define FSL_SAI_TMR 0x60 /* SAI Transmit Mask */
#define FSL_SAI_RCSR 0x80 /* SAI Receive Control */
#define FSL_SAI_RCR1 0x84 /* SAI Receive Configuration 1 */
@@ -29,8 +43,22 @@
#define FSL_SAI_RCR3 0x8c /* SAI Receive Configuration 3 */
#define FSL_SAI_RCR4 0x90 /* SAI Receive Configuration 4 */
#define FSL_SAI_RCR5 0x94 /* SAI Receive Configuration 5 */
-#define FSL_SAI_RDR 0xa0 /* SAI Receive Data */
-#define FSL_SAI_RFR 0xc0 /* SAI Receive FIFO */
+#define FSL_SAI_RDR0 0xa0 /* SAI Receive Data 0 */
+#define FSL_SAI_RDR1 0xa4 /* SAI Receive Data 1 */
+#define FSL_SAI_RDR2 0xa8 /* SAI Receive Data 2 */
+#define FSL_SAI_RDR3 0xac /* SAI Receive Data 3 */
+#define FSL_SAI_RDR4 0xb0 /* SAI Receive Data 4 */
+#define FSL_SAI_RDR5 0xb4 /* SAI Receive Data 5 */
+#define FSL_SAI_RDR6 0xb8 /* SAI Receive Data 6 */
+#define FSL_SAI_RDR7 0xbc /* SAI Receive Data 7 */
+#define FSL_SAI_RFR0 0xc0 /* SAI Receive FIFO 0 */
+#define FSL_SAI_RFR1 0xc4 /* SAI Receive FIFO 1 */
+#define FSL_SAI_RFR2 0xc8 /* SAI Receive FIFO 2 */
+#define FSL_SAI_RFR3 0xcc /* SAI Receive FIFO 3 */
+#define FSL_SAI_RFR4 0xd0 /* SAI Receive FIFO 4 */
+#define FSL_SAI_RFR5 0xd4 /* SAI Receive FIFO 5 */
+#define FSL_SAI_RFR6 0xd8 /* SAI Receive FIFO 6 */
+#define FSL_SAI_RFR7 0xdc /* SAI Receive FIFO 7 */
#define FSL_SAI_RMR 0xe0 /* SAI Receive Mask */

#define FSL_SAI_xCSR(tx) (tx ? FSL_SAI_TCSR : FSL_SAI_RCSR)
--
2.17.1


2019-07-30 01:51:29

by Nicolin Chen

[permalink] [raw]
Subject: Re: [PATCH v2 1/7] ASoC: fsl_sai: Add registers definition for multiple datalines

On Sun, Jul 28, 2019 at 10:24:23PM +0300, Daniel Baluta wrote:
> SAI IP supports up to 8 data lines. The configuration of
> supported number of data lines is decided at SoC integration
> time.
>
> This patch adds definitions for all related data TX/RX registers:
> * TDR0..7, Transmit data register
> * TFR0..7, Transmit FIFO register
> * RDR0..7, Receive data register
> * RFR0..7, Receive FIFO register
>
> Signed-off-by: Daniel Baluta <[email protected]>
> ---
> sound/soc/fsl/fsl_sai.c | 76 +++++++++++++++++++++++++++++++++++------
> sound/soc/fsl/fsl_sai.h | 36 ++++++++++++++++---
> 2 files changed, 98 insertions(+), 14 deletions(-)
>
> diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
> index 6d3c6c8d50ce..17b0aff4ee8b 100644
> --- a/sound/soc/fsl/fsl_sai.c
> +++ b/sound/soc/fsl/fsl_sai.c

> @@ -704,7 +711,14 @@ static bool fsl_sai_readable_reg(struct device *dev, unsigned int reg)
> case FSL_SAI_TCR3:
> case FSL_SAI_TCR4:
> case FSL_SAI_TCR5:
> - case FSL_SAI_TFR:
> + case FSL_SAI_TFR0:
> + case FSL_SAI_TFR1:
> + case FSL_SAI_TFR2:
> + case FSL_SAI_TFR3:
> + case FSL_SAI_TFR4:
> + case FSL_SAI_TFR5:
> + case FSL_SAI_TFR6:
> + case FSL_SAI_TFR7:
> case FSL_SAI_TMR:
> case FSL_SAI_RCSR:
> case FSL_SAI_RCR1:

A tricky thing here is that those SAI instances on older SoC don't
support multi data lines physically, while seemly having registers
pre-defined. So your change doesn't sound doing anything wrong to
them at all, I am still wondering if it is necessary to apply them
to newer compatible only though, as for older compatibles of SAI,
these registers would be useless and confusing if being exposed.

What do you think?

2019-07-30 07:16:20

by Nicolin Chen

[permalink] [raw]
Subject: Re: [PATCH v2 3/7] ASoC: fsl_sai: Add support to enable multiple data lines

On Sun, Jul 28, 2019 at 10:24:25PM +0300, Daniel Baluta wrote:
> SAI supports up to 8 Rx/Tx data lines which can be enabled
> using TCE/RCE bits of TCR3/RCR3 registers.
>
> Data lines to be enabled are read from DT fsl,dl-mask property.
> By default (if no DT entry is provided) only data line 0 is enabled.
>
> Signed-off-by: Daniel Baluta <[email protected]>
> ---
> sound/soc/fsl/fsl_sai.c | 11 ++++++++++-
> sound/soc/fsl/fsl_sai.h | 4 +++-
> 2 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
> index 637b1d12a575..5e7cb7fd29f5 100644
> --- a/sound/soc/fsl/fsl_sai.c
> +++ b/sound/soc/fsl/fsl_sai.c
> @@ -601,7 +601,7 @@ static int fsl_sai_startup(struct snd_pcm_substream *substream,
>
> regmap_update_bits(sai->regmap, FSL_SAI_xCR3(tx),
> FSL_SAI_CR3_TRCE_MASK,
> - FSL_SAI_CR3_TRCE);
> + FSL_SAI_CR3_TRCE(sai->soc_data->dl_mask[tx]);
>
> ret = snd_pcm_hw_constraint_list(substream->runtime, 0,
> SNDRV_PCM_HW_PARAM_RATE, &fsl_sai_rate_constraints);
> @@ -888,6 +888,15 @@ static int fsl_sai_probe(struct platform_device *pdev)
> }
> }
>
> + /*
> + * active data lines mask for TX/RX, defaults to 1 (only the first
> + * data line is enabled
> + */
> + sai->dl_mask[RX] = 1;
> + sai->dl_mask[TX] = 1;
> + of_property_read_u32_index(np, "fsl,dl-mask", RX, &sai->dl_mask[RX]);
> + of_property_read_u32_index(np, "fsl,dl-mask", TX, &sai->dl_mask[TX]);

Just curious what if we enable 8 data lines through DT bindings
while an audio file only has 1 or 2 channels. Will TRCE bits be
okay to stay with 8 data channels configurations? Btw, how does
DMA work for the data registers? ESAI has one entry at a fixed
address for all data channels while SAI seems to have different
data registers.

2019-08-06 15:30:45

by Daniel Baluta

[permalink] [raw]
Subject: Re: [PATCH v2 3/7] ASoC: fsl_sai: Add support to enable multiple data lines

On Mon, Jul 29, 2019 at 11:22 PM Nicolin Chen <[email protected]> wrote:
>
> On Sun, Jul 28, 2019 at 10:24:25PM +0300, Daniel Baluta wrote:
> > SAI supports up to 8 Rx/Tx data lines which can be enabled
> > using TCE/RCE bits of TCR3/RCR3 registers.
> >
> > Data lines to be enabled are read from DT fsl,dl-mask property.
> > By default (if no DT entry is provided) only data line 0 is enabled.
> >
> > Signed-off-by: Daniel Baluta <[email protected]>
> > ---
> > sound/soc/fsl/fsl_sai.c | 11 ++++++++++-
> > sound/soc/fsl/fsl_sai.h | 4 +++-
> > 2 files changed, 13 insertions(+), 2 deletions(-)
> >
> > diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
> > index 637b1d12a575..5e7cb7fd29f5 100644
> > --- a/sound/soc/fsl/fsl_sai.c
> > +++ b/sound/soc/fsl/fsl_sai.c
> > @@ -601,7 +601,7 @@ static int fsl_sai_startup(struct snd_pcm_substream *substream,
> >
> > regmap_update_bits(sai->regmap, FSL_SAI_xCR3(tx),
> > FSL_SAI_CR3_TRCE_MASK,
> > - FSL_SAI_CR3_TRCE);
> > + FSL_SAI_CR3_TRCE(sai->soc_data->dl_mask[tx]);
> >
> > ret = snd_pcm_hw_constraint_list(substream->runtime, 0,
> > SNDRV_PCM_HW_PARAM_RATE, &fsl_sai_rate_constraints);
> > @@ -888,6 +888,15 @@ static int fsl_sai_probe(struct platform_device *pdev)
> > }
> > }
> >
> > + /*
> > + * active data lines mask for TX/RX, defaults to 1 (only the first
> > + * data line is enabled
> > + */
> > + sai->dl_mask[RX] = 1;
> > + sai->dl_mask[TX] = 1;
> > + of_property_read_u32_index(np, "fsl,dl-mask", RX, &sai->dl_mask[RX]);
> > + of_property_read_u32_index(np, "fsl,dl-mask", TX, &sai->dl_mask[TX]);
>
> Just curious what if we enable 8 data lines through DT bindings
> while an audio file only has 1 or 2 channels. Will TRCE bits be
> okay to stay with 8 data channels configurations? Btw, how does
> DMA work for the data registers? ESAI has one entry at a fixed
> address for all data channels while SAI seems to have different
> data registers.

Hi Nicolin,

I have sent v3 and removed this patch from the series because we
need to find a better solution.

I think we should enable TCE based on the number of available datalines
and the number of active channels. Will come with a RFC patch later.

Pasting here the reply of SAI Audio IP owner regarding to your question above,
just for anyone to have more info of our private discussion:

If all 8 datalines are enabled using TCE then the transmit FIFO for
all 8 datalines need to be serviced, otherwise a FIFO underrun will be
generated.
Each dataline has a separate transmit FIFO with a separate register to
service the FIFO, so each dataline can be serviced separately. Note
that configuring FCOMB=2 would make it look like ESAI with a common
address for all FIFOs.
When performing DMA transfers to multiple datalines, there are a
couple of options:
* Use 1 DMA channel to copy first slot for each dataline to each
FIFO and then update the destination address back to the first
register.
* Configure separate DMA channel for each dataline and trigger the
first one by the DMA request and the subsequent channels by DMA
linking or scatter/gather.
* Configure FCOMB=2 and treat it the same as the ESAI. This is
almost the same as 1, but don’t need to update the destination
address.

Thanks,
Daniel.

2019-08-07 01:14:48

by Nicolin Chen

[permalink] [raw]
Subject: Re: [PATCH v2 3/7] ASoC: fsl_sai: Add support to enable multiple data lines

On Tue, Aug 06, 2019 at 06:23:27PM +0300, Daniel Baluta wrote:
> On Mon, Jul 29, 2019 at 11:22 PM Nicolin Chen <[email protected]> wrote:
> >
> > On Sun, Jul 28, 2019 at 10:24:25PM +0300, Daniel Baluta wrote:
> > > SAI supports up to 8 Rx/Tx data lines which can be enabled
> > > using TCE/RCE bits of TCR3/RCR3 registers.
> > >
> > > Data lines to be enabled are read from DT fsl,dl-mask property.
> > > By default (if no DT entry is provided) only data line 0 is enabled.
> > >
> > > Signed-off-by: Daniel Baluta <[email protected]>
> > > ---
> > > sound/soc/fsl/fsl_sai.c | 11 ++++++++++-
> > > sound/soc/fsl/fsl_sai.h | 4 +++-
> > > 2 files changed, 13 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
> > > index 637b1d12a575..5e7cb7fd29f5 100644
> > > --- a/sound/soc/fsl/fsl_sai.c
> > > +++ b/sound/soc/fsl/fsl_sai.c
> > > @@ -601,7 +601,7 @@ static int fsl_sai_startup(struct snd_pcm_substream *substream,
> > >
> > > regmap_update_bits(sai->regmap, FSL_SAI_xCR3(tx),
> > > FSL_SAI_CR3_TRCE_MASK,
> > > - FSL_SAI_CR3_TRCE);
> > > + FSL_SAI_CR3_TRCE(sai->soc_data->dl_mask[tx]);
> > >
> > > ret = snd_pcm_hw_constraint_list(substream->runtime, 0,
> > > SNDRV_PCM_HW_PARAM_RATE, &fsl_sai_rate_constraints);
> > > @@ -888,6 +888,15 @@ static int fsl_sai_probe(struct platform_device *pdev)
> > > }
> > > }
> > >
> > > + /*
> > > + * active data lines mask for TX/RX, defaults to 1 (only the first
> > > + * data line is enabled
> > > + */
> > > + sai->dl_mask[RX] = 1;
> > > + sai->dl_mask[TX] = 1;
> > > + of_property_read_u32_index(np, "fsl,dl-mask", RX, &sai->dl_mask[RX]);
> > > + of_property_read_u32_index(np, "fsl,dl-mask", TX, &sai->dl_mask[TX]);
> >
> > Just curious what if we enable 8 data lines through DT bindings
> > while an audio file only has 1 or 2 channels. Will TRCE bits be
> > okay to stay with 8 data channels configurations? Btw, how does
> > DMA work for the data registers? ESAI has one entry at a fixed
> > address for all data channels while SAI seems to have different
> > data registers.
>
> Hi Nicolin,
>
> I have sent v3 and removed this patch from the series because we
> need to find a better solution.

Ack. I was in that private mail thread. So it's totally fine.

>
> I think we should enable TCE based on the number of available datalines
> and the number of active channels. Will come with a RFC patch later.

Yea, that's exactly what I suspected during patch review and
what I suggested previously too. Look forward to your patch.

> Pasting here the reply of SAI Audio IP owner regarding to your question above,
> just for anyone to have more info of our private discussion:
>
> If all 8 datalines are enabled using TCE then the transmit FIFO for
> all 8 datalines need to be serviced, otherwise a FIFO underrun will be
> generated.
> Each dataline has a separate transmit FIFO with a separate register to
> service the FIFO, so each dataline can be serviced separately. Note
> that configuring FCOMB=2 would make it look like ESAI with a common
> address for all FIFOs.
> When performing DMA transfers to multiple datalines, there are a
> couple of options:
> * Use 1 DMA channel to copy first slot for each dataline to each
> FIFO and then update the destination address back to the first
> register.
> * Configure separate DMA channel for each dataline and trigger the
> first one by the DMA request and the subsequent channels by DMA
> linking or scatter/gather.
> * Configure FCOMB=2 and treat it the same as the ESAI. This is
> almost the same as 1, but don’t need to update the destination
> address.
>
> Thanks,
> Daniel.