diff mbox series

[linux-next,v2,9/9] ASoC: rsnd: add busif property to dai stream

Message ID 20181003090136.4556-1-jiada_wang@mentor.com (mailing list archive)
State New, archived
Headers show
Series ASoC: rsnd: support to use different BUSIF for GEN3 | expand

Commit Message

Wang, Jiada Oct. 3, 2018, 9:01 a.m. UTC
From: Jiada Wang <jiada_wang@mentor.com>

in GEN3 SSI may use different BUSIF for data transfer,
this patch adds busif property to each dai stream,
to indicate the BUSIF used by playback/capture stream.

Also adds rsnd_ssi_select_busif() to automatically select
BUSIF (currently only BUSIF0 is selected)

Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
---
 sound/soc/sh/rcar/core.c |  3 +++
 sound/soc/sh/rcar/dma.c  | 31 +++++++++++++++++++++++++++++++
 sound/soc/sh/rcar/rsnd.h |  3 +++
 sound/soc/sh/rcar/ssi.c  | 30 +++++++++++++++++++++++++++++-
 4 files changed, 66 insertions(+), 1 deletion(-)

Comments

Kuninori Morimoto Oct. 4, 2018, 1:41 a.m. UTC | #1
Hi Jiada

Thank you for your patch

> in GEN3 SSI may use different BUSIF for data transfer,
> this patch adds busif property to each dai stream,
> to indicate the BUSIF used by playback/capture stream.
> 
> Also adds rsnd_ssi_select_busif() to automatically select
> BUSIF (currently only BUSIF0 is selected)
> 
> Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
> ---
>  sound/soc/sh/rcar/core.c |  3 +++
>  sound/soc/sh/rcar/dma.c  | 31 +++++++++++++++++++++++++++++++
>  sound/soc/sh/rcar/rsnd.h |  3 +++
>  sound/soc/sh/rcar/ssi.c  | 30 +++++++++++++++++++++++++++++-
>  4 files changed, 66 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/soc/sh/rcar/core.c b/sound/soc/sh/rcar/core.c
> index 40d7dc4f7839..5e3e6e65bcdf 100644
> --- a/sound/soc/sh/rcar/core.c
> +++ b/sound/soc/sh/rcar/core.c
> @@ -1158,6 +1158,9 @@ static int rsnd_hw_params(struct snd_pcm_substream *substream,
>  	struct rsnd_dai *rdai = rsnd_dai_to_rdai(dai);
>  	struct rsnd_dai_stream *io = rsnd_rdai_to_io(rdai, substream);
>  	int ret;
> +	int chan = rsnd_runtime_channel_for_ssi_with_params(io, hw_params);
> +
> +	rsnd_ssi_select_busif(io, chan);

If my understanding was correct, the chance to use BUSIFx is when TDM split mode.
And this patch selects it on runtime (= hw_param) ?
But, I think we can/should select it on probe timing from DT connection.
Am I misunderstanding ?

I'm not sure how to select, but adding new ssiuX0 - ssiuX7
is realistic idea (parse sound card is not realistic...) ?
If so, your rxu/txu DMA can be more simple ?
Wang, Jiada Oct. 4, 2018, 2:43 a.m. UTC | #2
Hi Morimoto-san


On 2018/10/04 10:41, Kuninori Morimoto wrote:
> Hi Jiada
>
> Thank you for your patch
>
>> in GEN3 SSI may use different BUSIF for data transfer,
>> this patch adds busif property to each dai stream,
>> to indicate the BUSIF used by playback/capture stream.
>>
>> Also adds rsnd_ssi_select_busif() to automatically select
>> BUSIF (currently only BUSIF0 is selected)
>>
>> Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
>> ---
>>   sound/soc/sh/rcar/core.c |  3 +++
>>   sound/soc/sh/rcar/dma.c  | 31 +++++++++++++++++++++++++++++++
>>   sound/soc/sh/rcar/rsnd.h |  3 +++
>>   sound/soc/sh/rcar/ssi.c  | 30 +++++++++++++++++++++++++++++-
>>   4 files changed, 66 insertions(+), 1 deletion(-)
>>
>> diff --git a/sound/soc/sh/rcar/core.c b/sound/soc/sh/rcar/core.c
>> index 40d7dc4f7839..5e3e6e65bcdf 100644
>> --- a/sound/soc/sh/rcar/core.c
>> +++ b/sound/soc/sh/rcar/core.c
>> @@ -1158,6 +1158,9 @@ static int rsnd_hw_params(struct snd_pcm_substream *substream,
>>   	struct rsnd_dai *rdai = rsnd_dai_to_rdai(dai);
>>   	struct rsnd_dai_stream *io = rsnd_rdai_to_io(rdai, substream);
>>   	int ret;
>> +	int chan = rsnd_runtime_channel_for_ssi_with_params(io, hw_params);
>> +
>> +	rsnd_ssi_select_busif(io, chan);
> If my understanding was correct, the chance to use BUSIFx is when TDM split mode.
Yes, only when SSI works in Split/Ex-Split mode, BUSIFx other than 0 is 
necessary
> And this patch selects it on runtime (= hw_param) ?
Because, in order to automatically determine BUSIF number,
information like SSI mode (non-Split/Split/Ex-Split), runtime channel, 
are required
(in our internal implementation, SSI mode is selected by kctrl)
because of this, in this patch, BUSIF is selected on runtime

> But, I think we can/should select it on probe timing from DT connection.
> Am I misunderstanding ?
with the above reasoning, BUSIF is selected on runtime.
what do you think?

Thanks,
Jiada
> I'm not sure how to select, but adding new ssiuX0 - ssiuX7
> is realistic idea (parse sound card is not realistic...) ?
> If so, your rxu/txu DMA can be more simple ?
Kuninori Morimoto Oct. 4, 2018, 3:43 a.m. UTC | #3
Hi Jiada

Thank you for your feedback

> >> in GEN3 SSI may use different BUSIF for data transfer,
> >> this patch adds busif property to each dai stream,
> >> to indicate the BUSIF used by playback/capture stream.
> >> 
> >> Also adds rsnd_ssi_select_busif() to automatically select
> >> BUSIF (currently only BUSIF0 is selected)
> >> 
> >> Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
> >> ---
(snip)
> > And this patch selects it on runtime (= hw_param) ?
> Because, in order to automatically determine BUSIF number,
> information like SSI mode (non-Split/Split/Ex-Split), runtime channel,
> are required
> (in our internal implementation, SSI mode is selected by kctrl)
> because of this, in this patch, BUSIF is selected on runtime
> 
> > But, I think we can/should select it on probe timing from DT connection.
> > Am I misunderstanding ?
> with the above reasoning, BUSIF is selected on runtime.
> what do you think?

I have no objection that you are customizing your kernel locally.
But, upstreaming kernel based on it is not acceptable for me.
I'm not sure detail of your local implementation, but I don't think we
need to select SSI mode by kctrl.
If my understanding was correct, it can also be selected automatically somehow.
Or, am I misunderstanding ?

I could understand what you want to do, and yes, I can agree that we want/need
to have it on upstream. Thank you very much to indicating it to me.
But we need to consider more how to implement it.
Especially, it is related to DT bindings.
As you already know, if it is implemented on upstream kernel, we need to keep
compatibility in the future, and it is very difficult.

So, my opinions for BUSIFn support are
	- SSI mode should be selected automatically
	- BUSIFn connection should be selected on DT
	  (I think we don't want random sound output position ?)
	  - To select it, we need to have new "ssiu" DT seetings,
	    or parse sound card. Maybe adding ssiu is realistic.


Best regards
---
Kuninori Morimoto
Wang, Jiada Oct. 4, 2018, 7 a.m. UTC | #4
Hi Morimoto-san


Thanks for your comments


On 2018/10/04 12:43, Kuninori Morimoto wrote:
> Hi Jiada
>
> Thank you for your feedback
>
>>>> in GEN3 SSI may use different BUSIF for data transfer,
>>>> this patch adds busif property to each dai stream,
>>>> to indicate the BUSIF used by playback/capture stream.
>>>>
>>>> Also adds rsnd_ssi_select_busif() to automatically select
>>>> BUSIF (currently only BUSIF0 is selected)
>>>>
>>>> Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
>>>> ---
> (snip)
>>> And this patch selects it on runtime (= hw_param) ?
>> Because, in order to automatically determine BUSIF number,
>> information like SSI mode (non-Split/Split/Ex-Split), runtime channel,
>> are required
>> (in our internal implementation, SSI mode is selected by kctrl)
>> because of this, in this patch, BUSIF is selected on runtime
>>
>>> But, I think we can/should select it on probe timing from DT connection.
>>> Am I misunderstanding ?
>> with the above reasoning, BUSIF is selected on runtime.
>> what do you think?
> I have no objection that you are customizing your kernel locally.
> But, upstreaming kernel based on it is not acceptable for me.
> I'm not sure detail of your local implementation, but I don't think we
> need to select SSI mode by kctrl.
> If my understanding was correct, it can also be selected automatically somehow.
> Or, am I misunderstanding ?
SSI can work in following modes
1. Basic Mode: (channel 1, 2, 4, 6, 8, 16)
2. TDM Extended Mode: (channel 6, 8)
3. TDM Split Mode: (channel 1, 2)
4. TDM Ex-Split mode: (Channel 2, 4, 6, 8, 10)

for example user asks dai-link0 to playback 2ch audio stream,
driver can't determine which mode to work, as it can be Basic mode, 
Split mode or Ex-Split mode.

> I could understand what you want to do, and yes, I can agree that we want/need
> to have it on upstream. Thank you very much to indicating it to me.
> But we need to consider more how to implement it.
> Especially, it is related to DT bindings.
> As you already know, if it is implemented on upstream kernel, we need to keep
> compatibility in the future, and it is very difficult.
Yes, I agree with you, upstream need to consider lots of things
>
> So, my opinions for BUSIFn support are
> 	- SSI mode should be selected automatically
can you give me your idea, how to automatically determine working mode,
when user plays 2 channel stream on playback dai-link
> 	- BUSIFn connection should be selected on DT
since which BUSIFx is used during audio data transfer, is not 
consideration of user,
I think your previous suggestion, (automatically select BUSIFx) makes 
more sense

Thanks,
Jiada
> 	  (I think we don't want random sound output position ?)
> 	  - To select it, we need to have new "ssiu" DT seetings,
> 	    or parse sound card. Maybe adding ssiu is realistic.
>
>
> Best regards
> ---
> Kuninori Morimoto
Kuninori Morimoto Oct. 9, 2018, 12:44 a.m. UTC | #5
Hi Jiada

> SSI can work in following modes
> 1. Basic Mode: (channel 1, 2, 4, 6, 8, 16)
> 2. TDM Extended Mode: (channel 6, 8)
> 3. TDM Split Mode: (channel 1, 2)
> 4. TDM Ex-Split mode: (Channel 2, 4, 6, 8, 10)
(snip)
> > So, my opinions for BUSIFn support are
> > 	- SSI mode should be selected automatically
> can you give me your idea, how to automatically determine working mode,
> when user plays 2 channel stream on playback dai-link

If my understanding was correct, we can do like this
If DT indicated sound card has dai-link x N, tdm-slots = <M>,
	If (N, M) = (1, 2) : Basic mode
	If (N, M) = (1, >2): TDM mode
	If (N, M) = (2, 4) : TDM Split mode
	If (N, M) = (2, >4): TDM Ex-Split mode
	If (N, M) = (>2, 8): TDM Split mode
	...

Maybe some combination was wrong, but we can do something like this ?

> for example user asks dai-link0 to playback 2ch audio stream,
> driver can't determine which mode to work, as it can be Basic mode,
> Split mode or Ex-Split mode.

Why do we need to use Basic mode if HW has TDM Split mode connection?
If user playbacks 2ch audio in such situation,
we can use TDM Split mode (= only 2ch has sound, other channel has no sound ?) 
user might start to playback for other channels.
I'm not sure how it works...

> > 	- BUSIFn connection should be selected on DT
> since which BUSIFx is used during audio data transfer, is not
> consideration of user,
> I think your previous suggestion, (automatically select BUSIFx) makes
> more sense

I'm not yet sure detail, but in your idea, does it mean,
BUSIFx connection might be exchanged runtime ?
I think BUSIFx connection shouldn't exchanged runtime IMO.
Otherwise, sound position can't be fixed, and user can't control
sound, I think...

Best regards
---
Kuninori Morimoto
Wang, Jiada Oct. 9, 2018, 7:09 a.m. UTC | #6
Hi Morimoto-san

Thanks for your comment

On 2018/10/09 9:44, Kuninori Morimoto wrote:
> Hi Jiada
>
>> SSI can work in following modes
>> 1. Basic Mode: (channel 1, 2, 4, 6, 8, 16)
>> 2. TDM Extended Mode: (channel 6, 8)
>> 3. TDM Split Mode: (channel 1, 2)
>> 4. TDM Ex-Split mode: (Channel 2, 4, 6, 8, 10)
> (snip)
>>> So, my opinions for BUSIFn support are
>>> 	- SSI mode should be selected automatically
>> can you give me your idea, how to automatically determine working mode,
>> when user plays 2 channel stream on playback dai-link
> If my understanding was correct, we can do like this
> If DT indicated sound card has dai-link x N, tdm-slots = <M>,
> 	If (N, M) = (1, 2) : Basic mode
> 	If (N, M) = (1, >2): TDM mode
> 	If (N, M) = (2, 4) : TDM Split mode
> 	If (N, M) = (2, >4): TDM Ex-Split mode
> 	If (N, M) = (>2, 8): TDM Split mode
> 	...
>
> Maybe some combination was wrong, but we can do something like this ?
The idea to consider tdm_slot when determine SSI mode makes sense to me,
by checking runtime channel and tdm_slots combination,
I think SSI mode can be automatically selected like following:

1ch:  (tdm_slots < 4) Basic mode, (tdm_slots >= 4) TDM Split mode
2ch: (2 <= tdm_slots < 8) Basic mode, (tdm_slots >= 8) TDM Ex-Split mode
4ch: (4 <= tdm_slots < 8) Basic mode, (tdm_slots >= 8) TDM Ex-Split mode
6ch: (6 <= tdm_slots < 8) Basic mode, (tdm_slots == 8) TDM Extended 
mode, (8 < tdm_slots) TDM Ex-Split mode
8ch: (6 <= tdm_slots < 8) TDM Extended mode, (8 <= tdm_slots < 16) Basic 
mode, (tdm_slots == 16) TDM Ex-Split
10ch: TDM Ex-Split mode
16ch: Basic Mode

>> for example user asks dai-link0 to playback 2ch audio stream,
>> driver can't determine which mode to work, as it can be Basic mode,
>> Split mode or Ex-Split mode.
> Why do we need to use Basic mode if HW has TDM Split mode connection?
> If user playbacks 2ch audio in such situation,
> we can use TDM Split mode (= only 2ch has sound, other channel has no sound ?)
> user might start to playback for other channels.
> I'm not sure how it works...
>>> 	- BUSIFn connection should be selected on DT
>> since which BUSIFx is used during audio data transfer, is not
>> consideration of user,
>> I think your previous suggestion, (automatically select BUSIFx) makes
>> more sense
> I'm not yet sure detail, but in your idea, does it mean,
> BUSIFx connection might be exchanged runtime ?
no BUSIFx shouldn't be changed during runtime, my idea is BUSIFx can be 
automatically selected
when corresponding dai-link is not active

The reason I added rsnd_ssi_select_busif(io, chan) in rsnd_hw_params() 
in patch ASoC: rsnd: add busif property to dai stream of v2 patch-set,
is because runtime channel is necessary information to determine which 
BUSIFx to select,
(which is mentioned in above)
and at this stage (rsnd_hw_params()), all other control settings 
(register setting, dma address calculation etc)
haven't been done, so corresponding dai-link can be considered to be not 
active at this timing
but maybe you have better suggestion when to automatically select BUSIFx

What is your opinion?

Thanks,
Jiada
> I think BUSIFx connection shouldn't exchanged runtime IMO.
> Otherwise, sound position can't be fixed, and user can't control
> sound, I think...

>
> Best regards
> ---
> Kuninori Morimoto
Kuninori Morimoto Oct. 9, 2018, 7:47 a.m. UTC | #7
Hi Jiada

Thanks for your feedback

> 1ch:  (tdm_slots < 4) Basic mode, (tdm_slots >= 4) TDM Split mode
> 2ch: (2 <= tdm_slots < 8) Basic mode, (tdm_slots >= 8) TDM Ex-Split mode
> 4ch: (4 <= tdm_slots < 8) Basic mode, (tdm_slots >= 8) TDM Ex-Split mode
> 6ch: (6 <= tdm_slots < 8) Basic mode, (tdm_slots == 8) TDM Extended
> mode, (8 < tdm_slots) TDM Ex-Split mode
> 8ch: (6 <= tdm_slots < 8) TDM Extended mode, (8 <= tdm_slots < 16)
> Basic mode, (tdm_slots == 16) TDM Ex-Split
> 10ch: TDM Ex-Split mode
> 16ch: Basic Mode

Sorry, but I couldn't understand what this table means ?
For example, what does "1ch" mean ?
It looks like "1ch playback by TDM"...

> The reason I added rsnd_ssi_select_busif(io, chan) in rsnd_hw_params()
> in patch ASoC: rsnd: add busif property to dai stream of v2 patch-set,
> is because runtime channel is necessary information to determine which
> BUSIFx to select,
> (which is mentioned in above)
> and at this stage (rsnd_hw_params()), all other control settings
> (register setting, dma address calculation etc)
> haven't been done, so corresponding dai-link can be considered to be
> not active at this timing
> but maybe you have better suggestion when to automatically select BUSIFx

My image is like this.

	sound {
		compatible = "simple-scu-audio-card or new card";
		...
		simple-audio-card,convert-channels = <8>;
		...
		busif0: simple-audio-card,cpu@0 { sound-dai = <&rcar_sound 0>; };
		busif1: simple-audio-card,cpu@1 { sound-dai = <&rcar_sound 1>; };
		busif2: simple-audio-card,cpu@2 { sound-dai = <&rcar_sound 2>; };
		busif3: simple-audio-card,cpu@3 { sound-dai = <&rcar_sound 3>; };
		        simple-audio-card,codec { sound-dai = <&xxx>;          };
	};

	rcar_sound {
		dai0 { playback = <&ssiu0 ssi0>; }
		dai1 { playback = <&ssiu1 ssi0>; }
		dai2 { playback = <&ssiu2 ssi0>; }
		dai3 { playback = <&ssiu3 ssi0>; }
	};

Best regards
---
Kuninori Morimoto
Wang, Jiada Oct. 9, 2018, 10:40 a.m. UTC | #8
Hi Morimoto-san

Thanks for your feedback

On 2018/10/09 16:47, Kuninori Morimoto wrote:
> Hi Jiada
>
> Thanks for your feedback
>
>> 1ch:  (tdm_slots < 4) Basic mode, (tdm_slots >= 4) TDM Split mode
>> 2ch: (2 <= tdm_slots < 8) Basic mode, (tdm_slots >= 8) TDM Ex-Split mode
>> 4ch: (4 <= tdm_slots < 8) Basic mode, (tdm_slots >= 8) TDM Ex-Split mode
>> 6ch: (6 <= tdm_slots < 8) Basic mode, (tdm_slots == 8) TDM Extended
>> mode, (8 < tdm_slots) TDM Ex-Split mode
>> 8ch: (6 <= tdm_slots < 8) TDM Extended mode, (8 <= tdm_slots < 16)
>> Basic mode, (tdm_slots == 16) TDM Ex-Split
>> 10ch: TDM Ex-Split mode
>> 16ch: Basic Mode
> Sorry, but I couldn't understand what this table means ?
> For example, what does "1ch" mean ?
> It looks like "1ch playback by TDM"...
sorry for vague explanation,
by "1ch" I mean runtime 1 channel playback
for example:
if user plays 1 channel stream, by checking tdm_slots value,
if it is < 4, then it can't be working in Split mode, so driver will
automatically set SSI to work in Basic mode, otherwise, SSI will
work in TDM Split mode.

>> The reason I added rsnd_ssi_select_busif(io, chan) in rsnd_hw_params()
>> in patch ASoC: rsnd: add busif property to dai stream of v2 patch-set,
>> is because runtime channel is necessary information to determine which
>> BUSIFx to select,
>> (which is mentioned in above)
>> and at this stage (rsnd_hw_params()), all other control settings
>> (register setting, dma address calculation etc)
>> haven't been done, so corresponding dai-link can be considered to be
>> not active at this timing
>> but maybe you have better suggestion when to automatically select BUSIFx
> My image is like this.
>
> 	sound {
> 		compatible = "simple-scu-audio-card or new card";
> 		...
> 		simple-audio-card,convert-channels = <8>;
> 		...
> 		busif0: simple-audio-card,cpu@0 { sound-dai = <&rcar_sound 0>; };
> 		busif1: simple-audio-card,cpu@1 { sound-dai = <&rcar_sound 1>; };
> 		busif2: simple-audio-card,cpu@2 { sound-dai = <&rcar_sound 2>; };
> 		busif3: simple-audio-card,cpu@3 { sound-dai = <&rcar_sound 3>; };
> 		        simple-audio-card,codec { sound-dai = <&xxx>;          };
> 	};
>
> 	rcar_sound {
> 		dai0 { playback = <&ssiu0 ssi0>; }
> 		dai1 { playback = <&ssiu1 ssi0>; }
> 		dai2 { playback = <&ssiu2 ssi0>; }
> 		dai3 { playback = <&ssiu3 ssi0>; }
> 	};
After re-think about BUSIFx selection, I agree with you,
it shouldn't be random, and select it via device-tree, as you have 
already demonstrated
is a good idea

Based on our discussion so far, I think we both agree on the following 
points
1. Driver select SSI mode automatically (by checking tdm_slots, runtime 
channel, etc)
2. Driver parse BUSIFx for each dai-link from device-tree

I will review my v2 patch-set, drop changes violate to above two points


Thanks,
Jiada
> Best regards
> ---
> Kuninori Morimoto
Kuninori Morimoto Oct. 10, 2018, 12:16 a.m. UTC | #9
Hi Jiada

> >> 1ch:  (tdm_slots < 4) Basic mode, (tdm_slots >= 4) TDM Split mode
> >> 2ch: (2 <= tdm_slots < 8) Basic mode, (tdm_slots >= 8) TDM Ex-Split mode
> >> 4ch: (4 <= tdm_slots < 8) Basic mode, (tdm_slots >= 8) TDM Ex-Split mode
> >> 6ch: (6 <= tdm_slots < 8) Basic mode, (tdm_slots == 8) TDM Extended
> >> mode, (8 < tdm_slots) TDM Ex-Split mode
> >> 8ch: (6 <= tdm_slots < 8) TDM Extended mode, (8 <= tdm_slots < 16)
> >> Basic mode, (tdm_slots == 16) TDM Ex-Split
> >> 10ch: TDM Ex-Split mode
> >> 16ch: Basic Mode
> > Sorry, but I couldn't understand what this table means ?
> > For example, what does "1ch" mean ?
> > It looks like "1ch playback by TDM"...
> sorry for vague explanation,
> by "1ch" I mean runtime 1 channel playback
> for example:
> if user plays 1 channel stream, by checking tdm_slots value,
> if it is < 4, then it can't be working in Split mode, so driver will
> automatically set SSI to work in Basic mode, otherwise, SSI will
> work in TDM Split mode.

Hmm... ??
Maybe, we are misunderstanding each other...

In my understanding, if platform can use TDM 8ch Split mode,
user interface will be for example dai0, dai1, dai2, dai3 (= for TDM 8ch).
Here, each daiX can handle stereo sound only.
Then, user can playback like this

	aplay -D plughw:0,0 xxx.wav (= will be 1ch, 2ch)
	aplay -D plughw:0,1 xxx.wav (= will be 3ch, 4ch)
	aplay -D plughw:0,2 xxx.wav (= will be 5ch, 6ch)
	aplay -D plughw:0,3 xxx.wav (= will be 7ch, 8ch)
	
1ch sound will be converted to 2ch by alsalib, and daiX will receive
converted 2ch sound.
SSI always playbacks it as part of TDM 8ch Split mode.
In this platform, it can handle stereo sound only on each daiX,
and always works as TDM Split mode, never works as Basic Mode / TDM Ex-Split mode.

If DAI was dai0, dai1 only, it will be TDM Ex-Split mode.
It depends on tdm-slots, I think.

if tdm-slots was 6ch...
	aplay -D plughw:0,0 xxx.wav (= will be 1ch, 2ch, 3ch, 4ch)
	aplay -D plughw:0,1 xxx.wav (= will be 5ch, 6ch)

if tdm-slots was 8ch...
	aplay -D plughw:0,0 xxx.wav (= will be 1ch, 2ch, 3ch, 4ch, 5ch, 6ch)
	aplay -D plughw:0,1 xxx.wav (= will be 7ch, 8ch)

something like this, is my understanding.


> > 	sound {
> > 		compatible = "simple-scu-audio-card or new card";
> > 		...
> > 		simple-audio-card,convert-channels = <8>;
> > 		...
> > 		busif0: simple-audio-card,cpu@0 { sound-dai = <&rcar_sound 0>; };
> > 		busif1: simple-audio-card,cpu@1 { sound-dai = <&rcar_sound 1>; };
> > 		busif2: simple-audio-card,cpu@2 { sound-dai = <&rcar_sound 2>; };
> > 		busif3: simple-audio-card,cpu@3 { sound-dai = <&rcar_sound 3>; };
> > 		        simple-audio-card,codec { sound-dai = <&xxx>;          };
> > 	};
> > 
> > 	rcar_sound {
> > 		dai0 { playback = <&ssiu0 ssi0>; }
> > 		dai1 { playback = <&ssiu1 ssi0>; }
> > 		dai2 { playback = <&ssiu2 ssi0>; }
> > 		dai3 { playback = <&ssiu3 ssi0>; }
> > 	};
> After re-think about BUSIFx selection, I agree with you,
> it shouldn't be random, and select it via device-tree, as you have
> already demonstrated
> is a good idea

Thank you for understanding my idea.
Nice to know


Best regards
---
Kuninori Morimoto
diff mbox series

Patch

diff --git a/sound/soc/sh/rcar/core.c b/sound/soc/sh/rcar/core.c
index 40d7dc4f7839..5e3e6e65bcdf 100644
--- a/sound/soc/sh/rcar/core.c
+++ b/sound/soc/sh/rcar/core.c
@@ -1158,6 +1158,9 @@  static int rsnd_hw_params(struct snd_pcm_substream *substream,
 	struct rsnd_dai *rdai = rsnd_dai_to_rdai(dai);
 	struct rsnd_dai_stream *io = rsnd_rdai_to_io(rdai, substream);
 	int ret;
+	int chan = rsnd_runtime_channel_for_ssi_with_params(io, hw_params);
+
+	rsnd_ssi_select_busif(io, chan);
 
 	ret = rsnd_dai_call(hw_params, io, substream, hw_params);
 	if (ret)
diff --git a/sound/soc/sh/rcar/dma.c b/sound/soc/sh/rcar/dma.c
index 6d1947515dc8..bf87939f79bb 100644
--- a/sound/soc/sh/rcar/dma.c
+++ b/sound/soc/sh/rcar/dma.c
@@ -792,6 +792,37 @@  int rsnd_dma_attach(struct rsnd_dai_stream *io, struct rsnd_mod *mod,
 	return rsnd_dai_connect(*dma_mod, io, (*dma_mod)->type);
 }
 
+void rsnd_dma_addr_update(struct rsnd_dai_stream *io)
+{
+	struct rsnd_mod *ssi_mod = rsnd_io_to_mod_ssi(io);
+	struct rsnd_mod *mod_from = NULL;
+	struct rsnd_mod *mod_to = NULL;
+	struct rsnd_priv *priv = rsnd_io_to_priv(io);
+	struct rsnd_dma_ctrl *dmac = rsnd_priv_to_dmac(priv);
+	struct device *dev = rsnd_priv_to_dev(priv);
+	int is_play = rsnd_io_is_play(io);
+	struct rsnd_dma *dma = rsnd_mod_to_dma(io->dma);
+
+	if (!dmac)
+		return;
+
+	rsnd_dma_of_path(ssi_mod, io, is_play, &mod_from, &mod_to);
+
+	dma->src_addr = rsnd_dma_addr(io, mod_from, is_play, 1);
+	dma->dst_addr = rsnd_dma_addr(io, mod_to,   is_play, 0);
+
+	if (mod_from && mod_to) {
+		struct rsnd_dmapp *dmapp = rsnd_dma_to_dmapp(dma);
+
+		dmapp->chcr = rsnd_dmapp_get_chcr(io, mod_from, mod_to) |
+						  PDMACHCR_DE;
+	}
+
+	dev_dbg(dev, "%s[%d] %pad -> %pad\n",
+		rsnd_mod_name(ssi_mod), rsnd_mod_id(ssi_mod),
+		&dma->src_addr, &dma->dst_addr);
+}
+
 int rsnd_dma_probe(struct rsnd_priv *priv)
 {
 	struct platform_device *pdev = rsnd_priv_to_pdev(priv);
diff --git a/sound/soc/sh/rcar/rsnd.h b/sound/soc/sh/rcar/rsnd.h
index 4464d1d0a042..7ce10267c87b 100644
--- a/sound/soc/sh/rcar/rsnd.h
+++ b/sound/soc/sh/rcar/rsnd.h
@@ -227,12 +227,14 @@  void rsnd_bset(struct rsnd_priv *priv, struct rsnd_mod *mod, enum rsnd_reg reg,
 u32 rsnd_get_adinr_bit(struct rsnd_mod *mod, struct rsnd_dai_stream *io);
 u32 rsnd_get_dalign(struct rsnd_mod *mod, struct rsnd_dai_stream *io);
 u32 rsnd_get_busif_shift(struct rsnd_dai_stream *io, struct rsnd_mod *mod);
+void rsnd_ssi_select_busif(struct rsnd_dai_stream *io, int chan);
 
 /*
  *	R-Car DMA
  */
 int rsnd_dma_attach(struct rsnd_dai_stream *io,
 		    struct rsnd_mod *mod, struct rsnd_mod **dma_mod);
+void rsnd_dma_addr_update(struct rsnd_dai_stream *io);
 int rsnd_dma_probe(struct rsnd_priv *priv);
 struct dma_chan *rsnd_dma_request_channel(struct device_node *of_node,
 					  struct rsnd_mod *mod, char *name);
@@ -457,6 +459,7 @@  struct rsnd_dai_stream {
 	struct rsnd_dai *rdai;
 	struct device *dmac_dev; /* for IPMMU */
 	u32 parent_ssi_status;
+	u32 busif;
 };
 #define rsnd_io_to_mod(io, i)	((i) < RSND_MOD_MAX ? (io)->mod[(i)] : NULL)
 #define rsnd_io_to_mod_ssi(io)	rsnd_io_to_mod((io), RSND_MOD_SSI)
diff --git a/sound/soc/sh/rcar/ssi.c b/sound/soc/sh/rcar/ssi.c
index 992aeac09e76..d515ee192276 100644
--- a/sound/soc/sh/rcar/ssi.c
+++ b/sound/soc/sh/rcar/ssi.c
@@ -154,7 +154,35 @@  int rsnd_ssi_use_busif(struct rsnd_dai_stream *io)
 
 int rsnd_ssi_get_busif(struct rsnd_dai_stream *io)
 {
-	return 0; /* BUSIF0 only for now */
+	struct rsnd_priv *priv = rsnd_io_to_priv(io);
+
+	if (!rsnd_is_gen3(priv))
+		return 0;
+
+	if (!rsnd_ssi_use_busif(io))
+		return 0;
+
+	return io->busif;
+}
+
+void rsnd_ssi_select_busif(struct rsnd_dai_stream *io, int chan)
+{
+	struct rsnd_priv *priv = rsnd_io_to_priv(io);
+	u32 busif = 0; /* BUSIF0 only for now */
+
+	if (!rsnd_is_gen3(priv))
+		return;
+
+	if (!rsnd_ssi_use_busif(io))
+		return;
+
+	/* FIXME: calcuate BUSIF based on SSI mode in future */
+	if (io->busif == busif)
+		return;
+
+	io->busif = busif;
+
+	rsnd_dma_addr_update(io);
 }
 
 static void rsnd_ssi_status_clear(struct rsnd_mod *mod)