[PATCH v1 1/1] ASoC: rsnd: ssi: Fix issue in dma data address assignment

[PATCH v1 1/1] ASoC: rsnd: ssi: Fix issue in dma data address assignment

[jiada_wang]

From: Jiada Wang <jiada_wang@mentor.com>

Same SSI device may be used in different dai links,
by only having one dma struct in rsnd_ssi, after the first
instance's dma config be initilized, the following instances
can no longer configure dma, this causes issue, when their
dma data address are different from the first instance.

This patch by introduces two dma struct in rdai, each SSI
instance in a dai link is assigned with two dma struct,
to store dma configuration for playback and capture.

Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
---
 sound/soc/sh/rcar/rsnd.h |    2 ++
 sound/soc/sh/rcar/ssi.c  |    6 +++---
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/sound/soc/sh/rcar/rsnd.h b/sound/soc/sh/rcar/rsnd.h
index 57cd2bc..a26156c 100644
--- a/sound/soc/sh/rcar/rsnd.h
+++ b/sound/soc/sh/rcar/rsnd.h
@@ -459,6 +459,8 @@ struct rsnd_dai {
 	unsigned int frm_clk_inv:1;
 	unsigned int sys_delay:1;
 	unsigned int data_alignment:1;
+
+	struct rsnd_mod *dma[2];
 };
 
 #define rsnd_rdai_nr(priv) ((priv)->rdai_nr)
diff --git a/sound/soc/sh/rcar/ssi.c b/sound/soc/sh/rcar/ssi.c
index fece1e5f..4e97065 100644
--- a/sound/soc/sh/rcar/ssi.c
+++ b/sound/soc/sh/rcar/ssi.c
@@ -66,7 +66,6 @@
 
 struct rsnd_ssi {
 	struct rsnd_mod mod;
-	struct rsnd_mod *dma;
 
 	u32 flags;
 	u32 cr_own;
@@ -861,7 +860,8 @@ static int rsnd_ssi_dma_probe(struct rsnd_mod *mod,
 			      struct rsnd_dai_stream *io,
 			      struct rsnd_priv *priv)
 {
-	struct rsnd_ssi *ssi = rsnd_mod_to_ssi(mod);
+	struct rsnd_dai *rdai = rsnd_io_to_rdai(io);
+	int is_play = rsnd_io_is_play(io);
 	int ret;
 
 	/*
@@ -876,7 +876,7 @@ static int rsnd_ssi_dma_probe(struct rsnd_mod *mod,
 		return ret;
 
 	/* SSI probe might be called many times in MUX multi path */
-	ret = rsnd_dma_attach(io, mod, &ssi->dma);
+	ret = rsnd_dma_attach(io, mod, &rdai->dma[is_play]);
 
 	return ret;
 }
-- 
1.7.9.5

Re: [PATCH v1 1/1] ASoC: rsnd: ssi: Fix issue in dma data address assignment

[Kuninori Morimoto]

Hi Jiada

Thank you for your patch

> Same SSI device may be used in different dai links,
> by only having one dma struct in rsnd_ssi, after the first
> instance's dma config be initilized, the following instances
> can no longer configure dma, this causes issue, when their
> dma data address are different from the first instance.
> 
> This patch by introduces two dma struct in rdai, each SSI
> instance in a dai link is assigned with two dma struct,
> to store dma configuration for playback and capture.
> 
> Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
> ---
(snip)
> @@ -876,7 +876,7 @@ static int rsnd_ssi_dma_probe(struct rsnd_mod *mod,
>  		return ret;
>  
>  	/* SSI probe might be called many times in MUX multi path */
> -	ret = rsnd_dma_attach(io, mod, &ssi->dma);
> +	ret = rsnd_dma_attach(io, mod, &rdai->dma[is_play]);
>  
>  	return ret;
>  }

Some cases, same SSI might be used on different dai links.
In my understanding, it happen if you uses MIXer.
But, are you using same SSI for both playback and capture ??

Best regards
---
Kuninori Morimoto

Re: [PATCH v1 1/1] ASoC: rsnd: ssi: Fix issue in dma data address assignment

[Jiada Wang]

Hi Morimoto-san

On 12/20/2017 10:42 PM, Kuninori Morimoto wrote:
> Hi Jiada
>
> Thank you for your patch
>
>> Same SSI device may be used in different dai links,
>> by only having one dma struct in rsnd_ssi, after the first
>> instance's dma config be initilized, the following instances
>> can no longer configure dma, this causes issue, when their
>> dma data address are different from the first instance.
>>
>> This patch by introduces two dma struct in rdai, each SSI
>> instance in a dai link is assigned with two dma struct,
>> to store dma configuration for playback and capture.
>>
>> Signed-off-by: Jiada Wang<jiada_wang@mentor.com>
>> ---
> (snip)
>> @@ -876,7 +876,7 @@ static int rsnd_ssi_dma_probe(struct rsnd_mod *mod,
>>   		return ret;
>>
>>   	/* SSI probe might be called many times in MUX multi path */
>> -	ret = rsnd_dma_attach(io, mod,&ssi->dma);
>> +	ret = rsnd_dma_attach(io, mod,&rdai->dma[is_play]);
>>
>>   	return ret;
>>   }
> Some cases, same SSI might be used on different dai links.
> In my understanding, it happen if you uses MIXer.
> But, are you using same SSI for both playback and capture ??
No, I am not using same SSI in both playback and capture of same dai-link,
without this patch, I am seeing issues when rcar sound is working in 
multi DAI mode,
for example with the following configuration

diff --git a/arch/arm64/boot/dts/renesas/salvator-common.dtsi 
b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
index a298df7..16f3214 100644
--- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
+++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
@@ -94,14 +94,24 @@
         };

         rsnd_ak4613: sound {
-               compatible = "simple-audio-card";
+               compatible = "simple-scu-audio-card";

                 simple-audio-card,format = "left_j";
                 simple-audio-card,bitclock-master = <&sndcpu>;
                 simple-audio-card,frame-master = <&sndcpu>;

-               sndcpu: simple-audio-card,cpu {
-                       sound-dai = <&rcar_sound>;
+               simple-audio-card,prefix = "ak4613";
+                simple-audio-card,routing =
+                        "ak4613 Playback", "DAI0 Playback",
+                        "DAI0 Capture", "ak4613 Capture",
+                        "ak4613 Playback", "DAI1 Playback";
+
+               sndcpu: simple-audio-card,cpu@0 {
+                       sound-dai = <&rcar_sound 0>;
+               };
+
+               simple-audio-card,cpu@1 {
+                       sound-dai = <&rcar_sound 1>;
                 };

                 sndcodec: simple-audio-card,codec {
@@ -517,7 +527,7 @@
         pinctrl-names = "default";

         /* Single DAI */
-       #sound-dai-cells = <0>;
+       #sound-dai-cells = <1>;

         /* audio_clkout0/1/2/3 */
  #clock-cells = <1>;
@@ -549,6 +559,9 @@
                         playback = <&ssi0 &src0 &dvc0>;
                         capture  = <&ssi1 &src1 &dvc1>;
                 };
+               dai1 {
+                       playback = <&ssi0>;
+               };
         };
  };

playing with dai1 will have issue.

Thanks,
Jiada

> Best regards
> ---
> Kuninori Morimoto

Re: [PATCH v1 1/1] ASoC: rsnd: ssi: Fix issue in dma data address assignment

[Kuninori Morimoto]

Hi Jiada

> diff --git a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> index a298df7..16f3214 100644
> --- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> +++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> @@ -94,14 +94,24 @@
>         };
> 
>         rsnd_ak4613: sound {
> -               compatible = "simple-audio-card";
> +               compatible = "simple-scu-audio-card";
> 
>                 simple-audio-card,format = "left_j";
>                 simple-audio-card,bitclock-master = <&sndcpu>;
>                 simple-audio-card,frame-master = <&sndcpu>;
> 
> -               sndcpu: simple-audio-card,cpu {
> -                       sound-dai = <&rcar_sound>;
> +               simple-audio-card,prefix = "ak4613";
> +                simple-audio-card,routing =
> +                        "ak4613 Playback", "DAI0 Playback",
> +                        "DAI0 Capture", "ak4613 Capture",
> +                        "ak4613 Playback", "DAI1 Playback";
> +
> +               sndcpu: simple-audio-card,cpu@0 {
> +                       sound-dai = <&rcar_sound 0>;
> +               };
> +
> +               simple-audio-card,cpu@1 {
> +                       sound-dai = <&rcar_sound 1>;
>                 };
> 
>                 sndcodec: simple-audio-card,codec {
> @@ -517,7 +527,7 @@
>         pinctrl-names = "default";
> 
>         /* Single DAI */
> -       #sound-dai-cells = <0>;
> +       #sound-dai-cells = <1>;
> 
>         /* audio_clkout0/1/2/3 */
>  #clock-cells = <1>;
> @@ -549,6 +559,9 @@
>                         playback = <&ssi0 &src0 &dvc0>;
>                         capture  = <&ssi1 &src1 &dvc1>;
>                 };
> +               dai1 {
> +                       playback = <&ssi0>;
> +               };
>         };
>  };
> 
> playing with dai1 will have issue.

I guess so because you are using strange settings...
What do you want to do ?

Best regards
---
Kuninori Morimoto

Re: [PATCH v1 1/1] ASoC: rsnd: ssi: Fix issue in dma data address assignment

[Jiada Wang]

Hi Morimoto-san

On 12/20/2017 11:39 PM, Kuninori Morimoto wrote:
> Hi Jiada
>
>> diff --git a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
>> b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
>> index a298df7..16f3214 100644
>> --- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
>> +++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
>> @@ -94,14 +94,24 @@
>>          };
>>
>>          rsnd_ak4613: sound {
>> -               compatible = "simple-audio-card";
>> +               compatible = "simple-scu-audio-card";
>>
>>                  simple-audio-card,format = "left_j";
>>                  simple-audio-card,bitclock-master =<&sndcpu>;
>>                  simple-audio-card,frame-master =<&sndcpu>;
>>
>> -               sndcpu: simple-audio-card,cpu {
>> -                       sound-dai =<&rcar_sound>;
>> +               simple-audio-card,prefix = "ak4613";
>> +                simple-audio-card,routing =
>> +                        "ak4613 Playback", "DAI0 Playback",
>> +                        "DAI0 Capture", "ak4613 Capture",
>> +                        "ak4613 Playback", "DAI1 Playback";
>> +
>> +               sndcpu: simple-audio-card,cpu@0 {
>> +                       sound-dai =<&rcar_sound 0>;
>> +               };
>> +
>> +               simple-audio-card,cpu@1 {
>> +                       sound-dai =<&rcar_sound 1>;
>>                  };
>>
>>                  sndcodec: simple-audio-card,codec {
>> @@ -517,7 +527,7 @@
>>          pinctrl-names = "default";
>>
>>          /* Single DAI */
>> -       #sound-dai-cells =<0>;
>> +       #sound-dai-cells =<1>;
>>
>>          /* audio_clkout0/1/2/3 */
>>   #clock-cells =<1>;
>> @@ -549,6 +559,9 @@
>>                          playback =<&ssi0&src0&dvc0>;
>>                          capture  =<&ssi1&src1&dvc1>;
>>                  };
>> +               dai1 {
>> +                       playback =<&ssi0>;
>> +               };
>>          };
>>   };
>>
>> playing with dai1 will have issue.
> I guess so because you are using strange settings...
> What do you want to do ?
this is just an example setting to reproduce the issue with current 
mainline kernel,

We have enabled TDM Split and Ex-Split mode in our kernel,
and SSI(U)'s dma address diffs based on the BUSIF it is using,
so have a single dma data struct per rsnd_ssi will cause issue when
SSI isn't working with BUSIF0.

Do you have any suggestion to address this issue?


Thanks,
Jiada
> Best regards
> ---
> Kuninori Morimoto

Re: [PATCH v1 1/1] ASoC: rsnd: ssi: Fix issue in dma data address assignment

[Kuninori Morimoto]

Hi Jiada

Thank you for your feedback
I understand your situation

> We have enabled TDM Split and Ex-Split mode in our kernel,
> and SSI(U)'s dma address diffs based on the BUSIF it is using,
> so have a single dma data struct per rsnd_ssi will cause issue when
> SSI isn't working with BUSIF0.

First of all, "TDM (Ex) Split mode" is not yet supported.
And unfortunately, your patch is not enough for it.
I guess you enabled it with many local patches,
and posted one of them ?

It is very advanced feature, we need to consider about
channel/sampling rate/data width/settings/address etc etc etc...
Lots of things we need to solve/care !
DMA pointer is one of them.

If we focus only to DMA, your patch is still wrong I think.
"Playback/Capture direction" is not related to this topic.
1 DMA on 1 DAI is enough ?
And we need to update rsnd_gen2_dma_addr() too for DMA address.

> Do you have any suggestion to address this issue?

I have no idea at this point.
Missing part for TDM (Ex) Split mode is not only DMA pointer.

Why do you want to use it ?
If you want to do is only "use 2 DAIs for playback",
how about to use MIXer ? It is already supported on upstream.

Best regards
---
Kuninori Morimoto

Re: [PATCH v1 1/1] ASoC: rsnd: ssi: Fix issue in dma data address assignment

[Jiada Wang]

Hi Morimoto-san

On 12/21/2017 04:43 PM, Kuninori Morimoto wrote:
> Hi Jiada
>
> Thank you for your feedback
> I understand your situation
>
>> We have enabled TDM Split and Ex-Split mode in our kernel,
>> and SSI(U)'s dma address diffs based on the BUSIF it is using,
>> so have a single dma data struct per rsnd_ssi will cause issue when
>> SSI isn't working with BUSIF0.
> First of all, "TDM (Ex) Split mode" is not yet supported.
> And unfortunately, your patch is not enough for it.
> I guess you enabled it with many local patches,
> and posted one of them ?
>
> It is very advanced feature, we need to consider about
> channel/sampling rate/data width/settings/address etc etc etc...
> Lots of things we need to solve/care !
> DMA pointer is one of them.
>
> If we focus only to DMA, your patch is still wrong I think.
> "Playback/Capture direction" is not related to this topic.
> 1 DMA on 1 DAI is enough ?
> And we need to update rsnd_gen2_dma_addr() too for DMA address.
Yes, this patch is only one of a serial patch set to enable TDM (Ex) 
Split mode,
I submitted it alone because think it is independent,
but you are right, to make the background more clear,
I will submit the whole patch set together with this single patch in v2,
(this probably will take some time)

Thanks,
Jiada
>> Do you have any suggestion to address this issue?
> I have no idea at this point.
> Missing part for TDM (Ex) Split mode is not only DMA pointer.
>
> Why do you want to use it ?
> If you want to do is only "use 2 DAIs for playback",
> how about to use MIXer ? It is already supported on upstream.
> Best regards
> ---
> Kuninori Morimoto