[RFC] Demux blob #2594

juimonen · 2020-03-20T10:47:59Z

This is a RFC for making mux/demux routing matrix configuration easier as the routing matrix is now "visualized" in m4.

juimonen · 2020-03-20T10:51:41Z

see also this: thesofproject/sof-docs#203

@akloniex FYI, would appreciate your comments.

@lgirdwood Thinking and trying out different ways of setting the parameters in mux demux, I'm hesitant to use the enum controls, as at least for me they seems very confusing and we loose some of the demux/mux functionality. So I would go for more "visualized" approach in topology and still try to make the command line generator better...

xiulipan

Nice patch. Some suggestion to make it more generic.

tools/topology/sof/pipe-volume-demux-playback.m4

akloniex · 2020-03-20T15:29:44Z

@juimonen I'm currently working on few mux/demux optimizations.
Few things up front:

frame format in configuration blob does not have to be valid ATM - pipeline params propagation takes care of that just fine
number of channels per stream will not need to be vaild after my PR (WIP, just need to split it into logical commits) - same as above, pipeline params propagation does the trick

I'll send the PR today, and if we wish, we might go one step further and remove these fields from the interface altogether (for now I'll leave them marked as reserved). That would obviously cause backward incompatibility, but would cleanup the interface - I will not judge which way to go.

lgirdwood

So this will build the initial configuration of the mux/demux ? We still need to build the enum kcontrols.

tools/topology/m4/utils.m4

tools/topology/m4/muxdemux.m4

akloniex · 2020-03-20T19:07:12Z

@juimonen Please take a look at #2600
As I've mentioned before, few fields from the interface are obsolete, which would greatly simplify configuration of MUX/DEMUX. Now the configuration boils down to the routing table and pipe ID per stream configured.

lgirdwood · 2020-03-23T09:34:14Z

@juimonen Please take a look at #2600
As I've mentioned before, few fields from the interface are obsolete, which would greatly simplify configuration of MUX/DEMUX. Now the configuration boils down to the routing table and pipe ID per stream configured.

@akloniex @juimonen can we make these fields "reserved" in the IPC if they have never been used. If they have been used in the past then please add a "_deprecated" suffix and a cooment stating last supported ABI version.

plbossart

Comments for the rest of us humans that barely understand M4 would be nice :-)

tools/topology/m4/muxdemux.m4

plbossart · 2020-03-23T15:22:37Z

tools/topology/sof/pipe-volume-demux-playback.m4

can you explain what those matrices do? not use if this is a passthrough or a splitter?

thesofproject/sof-docs#203 hope this helps, would be good if you have time to browse it through at some point...

tools/topology/sof/pipe-volume-demux-playback.m4

juimonen · 2020-03-24T18:40:58Z

@xiulipan @lgirdwood @plbossart @akloniex updated.

cut the m4 macros to 80 chars (or at least shorter than before), there are some issues always as I try to make the "bytes" blob look nice in .conf
more comments to blob generation, 0 bit fields are mostly for reserved members in structs
move blob generation to top level, as it is otherwise really hard to see where the pipeline_id's are coming, hope we get rid of this in Artur's PR. This helps also with differentiating the configuration
remove unused default demux blob and pipeline
ABI_VER macros should now work with CI, new define SOFABIPATH needs to be used for command line demux topology hacking.

juimonen · 2020-03-24T18:42:23Z

And finally, this is PR is made now so that it should be applied only after #2600, so it takes into account the changed demux structs.

lgirdwood

Can you add more comment on how the matrices configure the mux/demux routing.

lgirdwood · 2020-03-24T19:47:55Z

tools/topology/m4/utils.m4

Best to use existing convention and use SOF_ABI_PATH

Btw, this path could also be invoked via the command line -I switch if easier.

The path is given to command line system call egrep, so I don't know how to forward it from m4 include... anyway I changed the definition. As the SOF_ABI_VERSION is composed by cmake, at least CI/Docker will find it (when building tools). You can give it from command line now as -DSOF_ABI_FILE= if you want to compile individual demux topology file by hand. Otherwise the macro is empty.

tools/topology/m4/utils.m4

tools/topology/m4/muxdemux.m4

tools/topology/sof-icl-rt711-rt1308-rt715-hdmi.m4

bardliao · 2020-03-26T03:19:18Z

I tested it on CML-H with rt711+rt1308+rt715. But somehow there is no output from both speakers.

juimonen · 2020-03-26T07:43:51Z

@bardliao so this latest version would need #2600 as under it. So 2 things:

Can you make the device work with only muxdemux: Optimize and cleanup mux/demux internal and external interface. #2600 without this PR?
If you can, problem is in the blob in this PR, so could you then just make the conf file from topology with and without this PR so it would be easy to compare the bytes....?
if already muxdemux: Optimize and cleanup mux/demux internal and external interface. #2600 causes issues, that needs to be debugged first

bardliao · 2020-03-26T12:26:51Z

Can you make the device work with only muxdemux: Optimize and cleanup mux/demux internal and external interface. #2600 without this PR?

@juimonen No, I will see DSP Panic if I apply #2600. I don't know what did I miss. I will re-test it after #2600 is merged.

juimonen · 2020-03-30T15:14:14Z

@xiulipan @lgirdwood @plbossart @akloniex @bardliao updated.

commented byte macros
changed the abi version generation macro
added mux -prefix to mux hard coded value
added MONO ifdefs around icl topology
added comments what the matrices do and how they can be "read"

keyonjie · 2020-04-01T03:17:37Z

tools/topology/sof-apl-demux-pcm512x.m4

Hi @juimonen since we are aligned that we don't plan to support channel mix features with mux/demux component, how about simplify this like below:

define(matrix1, `ROUTE_MATRIX(1, 0, 1, 2, 3, 4, 5, 6, 7)')

And change the ROUTE_MATRIX macro definition(something like BIT($i) will be good enough)) to be with bit indices inputs(we can use '-1' if it doesn't exist for the corresponding channel).

This will simplify the top level topology files a lot.

@keyonjie are we aligned on that matter ? :)

I really would not want to castrate this component... I'm 99% sure we will need the mix feature very soon in some occasion.

@juimonen per my understanding yes. :)

If we are inheriting the MUX/DEMUX concepts from ALSA, which means N->1 or 1->N only, no mixing concept there. But if we want to innovate a more powerful component here (e.g. named shuffler? I actually proposed this to @plbossart and @lgirdwood in 2017), that is another topic.

The pros for the more powerful one is that it is more generic and we don't need to create many components like mixer/mux/channel_selector, just use this unified one with different configure blob.

The cons with it is that the component may run with lower efficiency(we need to check more bit masks and flags) and not so straightforward to be understood, especially the higher MCPS consuming make me nervous and that's why I voted to remove the mixing fancy feature from it.

@lgirdwood please help chime in this please, if mixing is here, then using mapping mechanism to simplify/optimize the mux component may become impossible since it is not 1chan->1chan mapping anymore, there could be channel accumulation(e.g. chan a = chan b + chan c).

@Keyon yes I know the alsa mux concept and as you said this component is much more.

This should then be converted as official ASoC mux component, which also forces enum controls to user space.

I actually tried to convert this to ASoC mux at some point but stopped on the way, because this was much more versatile. I also remember talking for example with @singalsuo cases that would need more complicated functioning than simple mux.

We could also do following: use same component as "process" component with full API, as here now.

Make Asoc mux interface for the same component (restricted usage)

@Keyon and agree with your observations about performance, the optimizations you mention should be done. However, if you don't know what you are doing, you can do very nasty things with all our components, build really strange pipelines that will consume the whole dsp resources.

Thinking of the interface, I really think the visual "routing matrix" is really the most informative with this kind of component. With enum we would have 2^64 different matrices and it is just an example of a component for which the configuration through ALSA controls is just awkward.

@Keyon and agree with your observations about performance, the optimizations you mention should be done. However, if you don't know what you are doing, you can do very nasty things with all our components, build really strange pipelines that will consume the whole dsp resources.

Thinking of the interface, I really think the visual "routing matrix" is really the most informative with this kind of component. With enum we would have 2^64 different matrices and it is just an example of a component for which the configuration through ALSA controls is just awkward.

Agree with that. If we do keep the current mux component design, it is unrealistic to use enum as @lgirdwood mentioned, and your "routing matrix" depicted here is more informative and topology user friendly, thank you for that.

@juimonen @keyonjie we should be aiming for enums (once mixing is removed). The max numbers of enums/options we are looking at are

demux - 8 source channels on 1 stream to map of 8 sink channels on 8 streams i.e. 8 enums of 64 options

mux - 8 source channels on 8 streams to 8 sink channels on 1 stream i.e. 8 enums of 64 options max.
In reality we should have less enums and options as 8x8 topologies will be rare.

bardliao · 2020-04-06T09:21:22Z

@juimonen Can you rebase to the latest code? There is a conflict on sof-icl-rt711-rt1308-rt715-hdmi.m4. Thanks.

Bit and byte manipulation macros are needed for building binary blobs, so add them. Add also macro to generate sof_abi_version. Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com>

Add macro for binary blob generation for muxdemux. With this macro the stream routing matrix will be easier to visualize and manipulate. Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com>

Add mux config to top level topology. Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com>

Remove default demux parameters and start using blob defined in top level topology. Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com>

Demux default coeffs has been replaced by top level demux configuration, so remove it. Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com>

pipe-demux-playback.m4 is not used by any topology, so remove it. Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com>

juimonen · 2020-04-06T09:56:57Z

@bardliao rebased.

lgirdwood · 2020-04-07T13:35:53Z

Jenkins known issues, Travis looks like a false positive.

juimonen requested review from dbaluta, jajanusz, lbetlej, lgirdwood, mmaka1, plbossart and tlauda as code owners March 20, 2020 10:47

juimonen mentioned this pull request Mar 20, 2020

[BUG]DEMUX comp not work normally with channel 4 to 2 with format conversion #2533

Closed

xiulipan suggested changes Mar 20, 2020

View reviewed changes

tools/topology/sof/pipe-volume-demux-playback.m4 Outdated Show resolved Hide resolved

tools/topology/sof/pipe-volume-demux-playback.m4 Outdated Show resolved Hide resolved

tools/topology/sof/pipe-volume-demux-playback.m4 Outdated Show resolved Hide resolved

lgirdwood requested changes Mar 20, 2020

View reviewed changes

tools/topology/m4/utils.m4 Outdated Show resolved Hide resolved

tools/topology/m4/muxdemux.m4 Outdated Show resolved Hide resolved

plbossart requested changes Mar 23, 2020

View reviewed changes

juimonen force-pushed the demux_blob branch from 6eb49a5 to 527894d Compare March 24, 2020 18:32

juimonen requested a review from ranj063 as a code owner March 24, 2020 18:32

lgirdwood requested changes Mar 24, 2020

View reviewed changes

bardliao reviewed Mar 26, 2020

View reviewed changes

tools/topology/sof-icl-rt711-rt1308-rt715-hdmi.m4 Outdated Show resolved Hide resolved

bardliao mentioned this pull request Mar 26, 2020

[RFC]: topology: demux_coef_default: Define demux pipeline id variable #2540

Closed

juimonen mentioned this pull request Mar 26, 2020

muxdemux: Optimize and cleanup mux/demux internal and external interface. #2600

Merged

bardliao mentioned this pull request Mar 30, 2020

[HACK]: topology: demux_coef_default: modify pipeline id #2663

Merged

juimonen force-pushed the demux_blob branch from 527894d to 3dad1a4 Compare March 30, 2020 15:04

juimonen force-pushed the demux_blob branch from 3dad1a4 to 7c9dd5e Compare March 30, 2020 17:43

mengdonglin mentioned this pull request Mar 31, 2020

[BUG][CML-H]" error: ipc error for 0x60010000 size 20" display when aplay via speaker0,2. #2633

Closed

lgirdwood approved these changes Mar 31, 2020

View reviewed changes

juimonen mentioned this pull request Mar 31, 2020

[RFC] add mux/demux documentation thesofproject/sof-docs#203

Merged

keyonjie reviewed Apr 1, 2020

View reviewed changes

Jaska Uimonen added 8 commits April 6, 2020 12:39

topology: add bit, byte and abi_version macros to utils

36692b3

Bit and byte manipulation macros are needed for building binary blobs, so add them. Add also macro to generate sof_abi_version. Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com>

topology: add binary blob macro for muxdemux

2d57fa8

Add macro for binary blob generation for muxdemux. With this macro the stream routing matrix will be easier to visualize and manipulate. Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com>

topology: add mux config to sof-apl-demux-pcm512x.m4

aa0bf5d

Add mux config to top level topology. Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com>

topology: add mux config to sof-cml-demux-rt5682.m4

064d985

Add mux config to top level topology. Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com>

topology: add mux config to sof-icl-rt711-rt1308-rt715-hdmi.m4

b28d96f

Add mux config to top level topology. Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com>

topology: remove default demux config from pipe-volume-demux-playback

31058e2

Remove default demux parameters and start using blob defined in top level topology. Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com>

topology: remove unused demux default coeffs

a26efeb

Demux default coeffs has been replaced by top level demux configuration, so remove it. Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com>

topology: remove unused demux pipeline

ede497d

pipe-demux-playback.m4 is not used by any topology, so remove it. Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com>

juimonen force-pushed the demux_blob branch from 7c9dd5e to ede497d Compare April 6, 2020 09:56

sinahuang mentioned this pull request Apr 7, 2020

[BUG][CML][tplg]"error: ipc error for 0x60010000 size 20" happened after booting up successfully. #2690

Closed

lgirdwood approved these changes Apr 7, 2020

View reviewed changes

lgirdwood merged commit 5672896 into thesofproject:master Apr 7, 2020

[RFC] Demux blob #2594

[RFC] Demux blob #2594

Uh oh!

Conversation

juimonen commented Mar 20, 2020

Uh oh!

juimonen commented Mar 20, 2020

Uh oh!

xiulipan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

akloniex commented Mar 20, 2020

Uh oh!

lgirdwood left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

akloniex commented Mar 20, 2020

Uh oh!

lgirdwood commented Mar 23, 2020

Uh oh!

plbossart left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

juimonen commented Mar 24, 2020

Uh oh!

juimonen commented Mar 24, 2020

Uh oh!

lgirdwood left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bardliao commented Mar 26, 2020

Uh oh!

juimonen commented Mar 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bardliao commented Mar 26, 2020

Uh oh!

juimonen commented Mar 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

keyonjie Apr 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

keyonjie Apr 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

keyonjie Apr 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

juimonen commented Mar 26, 2020 •

edited

Loading

juimonen commented Mar 30, 2020 •

edited

Loading

keyonjie Apr 1, 2020 •

edited

Loading

keyonjie Apr 1, 2020 •

edited

Loading

keyonjie Apr 1, 2020 •

edited

Loading