mux: using channel map as mux configuration

[abonislawski]

Previous sof_mux_config will be deleted along with global frame_format and num_channels for mux.
Mux will obtain format data from buffer params.

[paulstelian97]

So in short you just transmit which channels are to be muxed together using the channel map, then the actual config options come from the connected buffers themselves?

Should be more reliable if I understood this correctly.

[lgirdwood]

Can you comment this logic.

[abonislawski]

I think docs description (and examples) will be better than my words :)

https://github.com/mmaka1/sof-docs/blob/channel-mapping/api/uapi/channel-mapping.rst

[lgirdwood]

@abonislawski sorry I mean comment this code inline. i.e. briefly describe what it is doing at each block otherwise it's hard to follow.

[lgirdwood]

@abonislawski any eta for the comments, this can be merged soon. thanks.

[abonislawski]

@lgirdwood sorry, its done now but please note this cannot be merged before #2216

[abonislawski]

So in short you just transmit which channels are to be muxed together using the channel map, then the actual config options come from the connected buffers themselves?

Should be more reliable if I understood this correctly.

Yes but the most important part is to use channel map for configuration. Num channels and frame format is small thing which should be done anyway

[lgirdwood]

@abonislawski can you check ICL CI report.

[lgirdwood]

@abonislawski oh and these is some alignment issue picked up by checkpatch, can you fix.

[abonislawski]

@lgirdwood alignment fixed but not sure about CI because its only "Build not found." info for unit tests

[lgirdwood]

@zrombel CI has the build not found error, can you please check as it's blocking merge.

[zrombel]

Mux unit tests fail for all targets.

[abonislawski]

Tests updated with channel map generation, please review this part (more changes than expected) @lgirdwood @jajanusz @paulstelian97

[lgirdwood]

In general (not just this PR), I want to see comments added to any block of code that is non obvious, I'd like to see that here to to allow the reader easier understanding of the code.

[lgirdwood]

@abonislawski can you ping this PR once the dependency is merged. Thanks.

[lgirdwood]

@abonislawski any update ?

[abonislawski]

@lgirdwood dependency #2216 is ready to merge but not merged yet, kernel PR is ready to merge too

[zrombel]

Due to merge of PR#2303 this PR must be rebased or test results for KD tests with audio format 24b/32b ignored.

[juimonen]

@abonislawski you have the topology part for the configuration or are you doing with some new binary blo generator? I made #2345 to demonstrate the topology for demux, so I can modify it for this and we would have proper topology/configuration support.

We would really need some documentation for mux/demux usage, I've been at least 2 times searching for it with @singalsuo because of need to use mux/demux. I only had some pre-historic mail from @akloniex that I've lost on the way...

[lgirdwood]

@juimonen care to create feature request for the docs and write them, I think you are the new expert now !

[juimonen]

@abonislawski @mmaka1 how is the ch_coeffs array in sof_ipc_channel_map used?
so is it sparse, like if I have masks set in bit 3 and 5, do I index it like ch_coeffs[3] and ch_coeffs[5] or they are in ch_coeffs[0] and ch_coeffs[1] ?

[abonislawski]

@juimonen ch_coeffs is a flexible array to be as small as possible, so in your example you will have ch_coeffs[0] and ch_coeffs[1]

[juimonen]

@abonislawski ok thanks. this will be "pita" to parse from topology in kernel, but I'll see what I can do :)

[lgirdwood]

@juimonen why do we need to parse coeffs in the kernel ? We should just need to find them and pass them to FW at most ?

[juimonen]

@lgirdwood So I guess these coeffs would be coming from topology? And if the amount of coeffs varies between 0-32, so this kind of variable amount of tokens is just nasty. You can't easily copy it from tuples arrays to ipc structs. You can do it, but requires much more effort than simple fixed amount of tuples, that map almost directly to ipc.

[juimonen]

@lgirdwood and the problem with channel map is even worse as you might have many of those in one wodget. All can have different size. You can't even reserve correct amount of memory for ipc struct before parsing the whole area (to find out total variable size of all channel maps)

[lgirdwood]

@juimonen can you remind us of the structures we are passing here by pasting in reply.

[juimonen]

@lgirdwood I'm talking about the structs in src/include/ipc/channel_map.h

struct sof_ipc_channel_map {
        uint32_t ch_index;
        uint32_t ext_id;
        uint32_t ch_mask;
        uint32_t reserved;
        int32_t ch_coeffs[0];
} __packed;

and

struct sof_ipc_stream_map {
        struct sof_ipc_cmd_hdr hdr;
        uint32_t num_ch_map;
        uint32_t reserved[3];
        struct sof_ipc_channel_map ch_map[0];
} __packed;

So we should parse these from topology. stream_map contains variable amount of channel maps. channel_map contains variable amount of coeffs.

[lgirdwood]

@juimonen looks like we need a v2 for sof_ipc_stream_map and sof_ipc_channel_map since we have 2 variable length blobs in the array (making it impossible to easily index)

[jajanusz]

Depends on #2216

[lgirdwood]

Moving to v1.7

[mwasko]

The recommendation is to cancel this PR due to fact that there is no functional use-case for this PR and it is more a refactor how the configuration is passed which include ABI upgrade (do not want to do that if not really needed).