[Sound-open-firmware] [PATCH] [RFC] topology: scripts: update for 4-channel feature.

[Pierre-Louis Bossart]

On 5/23/18 12:24 AM, Ranjani Sridharan wrote:
> On Wed, 2018-05-23 at 11:01 +0800, zhigangw wrote:
>>
>> On 2018年05月22日 20:27, Pierre-Louis Bossart wrote:
>>> On 5/22/18 4:48 AM, Wu Zhigang wrote:
>>>> the script should be updated for 4-channel feature.
>>>> if not, the volume of rear 2-channel cannot be adjusted.
>>>>
>>>> Signed-off-by: Wu Zhigang <zhigang.wu at linux.intel.com>
>>>>
>>>> ---
>>>> Test with:
>>>> APL-MRB with tdf8532 codec
>>>> Linux topic/sof-v4.14: 33807ca8baa20bab2f98e9cc8a23c51a0822ad9f
>>>> SOF master: e5dab2ada6ef98dbc6224863110cc86b5839ea01
>>>> Tool master: b4f3670870ba5a933c3ec58a51b59ff9ef8b3588
>>>>
>>>> The Sanity test is for nocodec case.
>>>> this change only affect the codec case.
>>>> so I did not do the sanity test.
>>>> ---
>>>>    topology/sof/pipe-volume-capture.m4  | 4 ++--
>>>>    topology/sof/pipe-volume-playback.m4 | 4 ++--
>>>>    2 files changed, 4 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/topology/sof/pipe-volume-capture.m4
>>>> b/topology/sof/pipe-volume-capture.m4
>>>> index b3d3206..3c03e68 100644
>>>> --- a/topology/sof/pipe-volume-capture.m4
>>>> +++ b/topology/sof/pipe-volume-capture.m4
>>>> @@ -22,8 +22,8 @@ C_CONTROLMIXER(Master Capture Volume,
>>>> PIPELINE_ID,
>>>>        CONTROLMIXER_MAX(, 32),
>>>>        false,
>>>>        CONTROLMIXER_TLV(TLV 32 steps from -90dB to +6dB for 3dB,
>>>> vtlv_m90s3),
>>>> -    Channel register and shift for Front Left/Right,
>>>> -    LIST(`    ', KCONTROL_CHANNEL(FL, 1, 0),
>>>> KCONTROL_CHANNEL(FR, 1,
>>>> 1)))
>>>> +    Channel register and shift for Front Left/Right and Rear
>>>> Left/Right,
>>>> +    LIST(`    ', KCONTROL_CHANNEL(FL, 1, 0),
>>>> KCONTROL_CHANNEL(FR, 1,
>>>> 1), KCONTROL_CHANNEL(RL, 2, 0), KCONTROL_CHANNEL(RR, 2, 1)))
>>>
>>> This doesn't scale. This should be a parameter defining which
>>> channels
>>> should be controlled. Hard-coding to two outputs or four outputs
>>> is
>>> not going to work for the next board if it has 6 channels...
>>>
>>
>> Maybe we could create different "pipe-volume-capture.m4" and
>> "pipe-volume-playback.m4" files for different channel number.
>> Normally there are not many kinds of channel combination in the real
>> case.
> 
> We dont need a separate m4 file. We should define the channels in the
> test-capture-ssp.m4 file that sets up the test topology parameters.
> 
> So to give you an example for the stereo case:
> I think it would be best to move this as an argument to
> PIPELINE_PCM_DAI_ADD:
> 
> PIPELINE_PCM_DAI_ADD(sof/pipe-TEST_PIPE_NAME-capture.m4,
> 	2, 0, 2, TEST_PIPE_FORMAT,
> 	48, 1000, 0, 0, 0, 1,
> 	TEST_DAI_TYPE, TEST_DAI_PORT, TEST_DAI_FORMAT, 2,
> 	LIST(`	', KCONTROL_CHANNEL(FL, 1, 0),
> KCONTROL_CHANNEL(FR, 1, 	1))))
> 
> then we can define out CHANNEL_LIST in the PIPELINE_PCM_DAI_ADD
> definition as:
> `define(`PIPELINE_CHANNELS_LIST', $16)'
> 
> and lastly in pipe-volume-capture.m4, we can replace the last line in
> C_CONTROLMIXER(....
> 	PIPELINE_CHANNELS_LIST)
> 
> Does this make sense?

Should those Kcontrols be listed as part of the volume definitions 
instead? They don't really apply to the pipeline proper but rather to 
the volume component that's inserted in a pipeline.

> 
>>
>> If this suggestion is not OK. then my question is:
>>
>> What is the maximum channel number we want to support?
>> What is the channel name for each we want to use?
>> do we have plan for this?
> 
> please take a look at this page for the supported control channel
> mapping in ALSA topology:
> https://www.alsa-project.org/alsa-doc/alsa-lib/group__topology.html
>>
>> for 5.1 channel normally like this.
>> chan1 -- FL
>> chan2 -- FR
>> chan3 -- RL
>> chan4 -- RR
>> chan5 -- FC
>> chan6 -- LFE
>>
>> for 7.1 channel normally like this.
>> chan1 -- FL
>> chan2 -- FR
>> chan3 -- Left
>> chan4 -- Right
>> chan5 -- RL
>> chan6 -- RR
>> chan7 -- FC
>> chan8 -- LFE
>>
>> If I am wrong, please point it out.
>> Actually each channel should be remapped according to the speaker's
>> location and type in the real case.
>> But we may not need to take it into account right now.
>>
>> thanks
>> br
>> ~zhigang
>>
>>>>      #
>>>>    # Components and Buffers
>>>> diff --git a/topology/sof/pipe-volume-playback.m4
>>>> b/topology/sof/pipe-volume-playback.m4
>>>> index 4b12d2a..65d7894 100644
>>>> --- a/topology/sof/pipe-volume-playback.m4
>>>> +++ b/topology/sof/pipe-volume-playback.m4
>>>> @@ -22,8 +22,8 @@ C_CONTROLMIXER(Master Playback Volume,
>>>> PIPELINE_ID,
>>>>        CONTROLMIXER_MAX(, 32),
>>>>        false,
>>>>        CONTROLMIXER_TLV(TLV 32 steps from -90dB to +6dB for 3dB,
>>>> vtlv_m90s3),
>>>> -    Channel register and shift for Front Left/Right,
>>>> -    LIST(`    ', KCONTROL_CHANNEL(FL, 1, 0),
>>>> KCONTROL_CHANNEL(FR, 1,
>>>> 1)))
>>>> +    Channel register and shift for Front Left/Right and Rear
>>>> Left/Right,
>>>> +    LIST(`    ', KCONTROL_CHANNEL(FL, 1, 0),
>>>> KCONTROL_CHANNEL(FR, 1,
>>>> 1), KCONTROL_CHANNEL(RL, 2, 0), KCONTROL_CHANNEL(RR, 2, 1)))
>>>>      #
>>>>    # Components and Buffers
>>>>
>>
>> _______________________________________________
>> Sound-open-firmware mailing list
>> Sound-open-firmware at alsa-project.org
>> http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware
> _______________________________________________
> Sound-open-firmware mailing list
> Sound-open-firmware at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware
>