Repository navigation
Codec Adapter: Initial setup data problem #4838
Description
I'm hoping someone here will be able to advise.... I am currently using the SOF codec adapter mechanism as a way to integrate my company's technoloy for use in SOF. So far everything has gone fairly smoothly, so thanks to everyone for that!
The last issue I have, which I'm a little stuck on is passing my initial setup configuration to my codec adapter using the pipeline .m4 file that I have written. I've pasted a working version of my .m4 at the end of this message.
The problem I have is that I need to include ~900 bytes in CA_SETUP_CONTROLBYTES. Through trial and error, the 'limit' I see to hit is around 220 bytes. By contrast, I'm able to send the same 900 byte configuration 'blob' to my runtime control via ALSA (using snd_ctl_elem_tlv_write in a similar vein to sof-ctl) with no problems at all - all the data I expect to arrive at my codec adapter arrives sucessfully and I'm able to update the settings in my underlying software with no issue.
When I run my topology (on an UP! Xtreme dev board) with the larger, non-working, CA_SETUP_CONTROLBYTES, I see the following in dmesg output.
[ 7.391056] sof-audio-pci-intel-cnl 0000:00:1f.3: widget PCM0P setup complete
[ 7.391060] sof-audio-pci-intel-cnl 0000:00:1f.3: ipc tx: 0x30200000
[ 7.391145] sof-audio-pci-intel-cnl 0000:00:1f.3: ipc tx succeeded: 0x30200000
[ 7.391146] sof-audio-pci-intel-cnl 0000:00:1f.3: widget BUF1.0 setup complete
[ 7.391154] sof-audio-pci-intel-cnl 0000:00:1f.3: ipc tx: 0x30010000
[ 7.391263] sof-audio-pci-intel-cnl 0000:00:1f.3: ipc tx succeeded: 0x30010000
[ 7.391274] sof-audio-pci-intel-cnl 0000:00:1f.3: ipc tx: 0x50030000
[ 7.391646] sof-audio-pci-intel-cnl 0000:00:1f.3: ipc tx succeeded: 0x50030000
[ 7.391648] sof-audio-pci-intel-cnl 0000:00:1f.3: widget EQIIR1.0 setup complete
[ 7.391651] sof-audio-pci-intel-cnl 0000:00:1f.3: ipc tx: 0x30200000
[ 7.391739] sof-audio-pci-intel-cnl 0000:00:1f.3: ipc tx succeeded: 0x30200000
[ 7.391742] sof-audio-pci-intel-cnl 0000:00:1f.3: widget BUF1.1 setup complete
[ 7.391766] sof-audio-pci-intel-cnl 0000:00:1f.3: ipc tx: 0x30010000
[ 7.391864] sof-audio-pci-intel-cnl 0000:00:1f.3: ipc tx error for 0x30010000 (msg/reply size: 116/20): -22
[ 7.391869] sof-audio-pci-intel-cnl 0000:00:1f.3: error: failed to load widget CODEC_ADAPTER1.0
[ 7.391872] sof-audio-pci-intel-cnl 0000:00:1f.3: ipc tx: 0x30020000
[ 7.392131] sof-audio-pci-intel-cnl 0000:00:1f.3: ipc tx succeeded: 0x30020000
[ 7.392134] sof-audio-pci-intel-cnl 0000:00:1f.3: widget PCM0P freed
[ 7.392137] sof-audio-pci-intel-cnl 0000:00:1f.3: ipc tx: 0x30210000
[ 7.392201] sof-audio-pci-intel-cnl 0000:00:1f.3: ipc tx succeeded: 0x30210000
[ 7.392203] sof-audio-pci-intel-cnl 0000:00:1f.3: widget BUF1.0 freed
[ 7.392205] sof-audio-pci-intel-cnl 0000:00:1f.3: ipc tx: 0x30020000
[ 7.392410] sof-audio-pci-intel-cnl 0000:00:1f.3: ipc tx succeeded: 0x30020000
[ 7.392411] sof-audio-pci-intel-cnl 0000:00:1f.3: widget EQIIR1.0 freed
[ 7.392414] sof-audio-pci-intel-cnl 0000:00:1f.3: ipc tx: 0x30210000
[ 7.392491] sof-audio-pci-intel-cnl 0000:00:1f.3: ipc tx succeeded: 0x30210000
[ 7.392494] sof-audio-pci-intel-cnl 0000:00:1f.3: widget BUF1.1 freed
[ 7.392496] sof-audio-pci-intel-cnl 0000:00:1f.3: ipc tx: 0x30110000
[ 7.393071] sof-audio-pci-intel-cnl 0000:00:1f.3: ipc tx succeeded: 0x30110000
[ 7.393074] sof-audio-pci-intel-cnl 0000:00:1f.3: widget PIPELINE.1.HDA0.OUT freed
[ 7.393076] sof-audio-pci-intel-cnl 0000:00:1f.3: error: failed widget list set up for pcm 0 dir 0
[ 7.393080] sof-audio-pci-intel-cnl 0000:00:1f.3: ASoC: error at snd_soc_pcm_component_hw_params on 0000:00:1f.3: -22
[ 7.393085] HDA Analog: ASoC: soc_pcm_hw_params() failed (-22)
Here's the contents of my m4 pipeline file in the state that works. Once the system is initialised with this file, I am able to send my 900 byte runtime config data blobs to control with no issue.
# Low Latency Passthrough with My SDK codec Pipeline and PCM
#
# Pipeline Endpoints for connection are :-
#
# host PCM_P --> B0 --> EQ 0 --> B1 --> My SDK codec --> B2 --> sink DAI0
# My SDK codec setup config
define(`CA_SETUP_CONTROLBYTES',
`` bytes "0x53,0x4f,0x46,0x00,0x00,0x00,0x00,0x00,'
` 0x20,0x00,0x00,0x00,0x01,0x20,0x01,0x03,'
` 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,'
` 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,'
` 0x00,0x01,0x41,0x57,0x00,0x00,0x00,0x00,'
` 0x80,0xbb,0x00,0x00,0x18,0x00,0x00,0x00,'
` 0x02,0x00,0x00,0x00,0x01,0x00,0x00,0x00,'
` 0x0c,0x00,0x00,0x00,0x00,0x00,0x00,0x00"''
)
define(`CA_SETUP_CONTROLBYTES_MAX', 8192)
define(`CA_SETUP_CONTROLBYTES_NAME', `MyCompany' `ENDPOINT_NAME' `Setup ')
define(`CA_RUNTIME_CONTROLBYTES',
`` bytes "0x53,0x4f,0x46,0x00,0x01,0x00,0x00,0x00,'
` 0x00,0x00,0x00,0x00,0x01,0x20,0x01,0x03,'
` 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,'
` 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00"''
)
define(`CA_RUNTIME_CONTROLBYTES_MAX', 8192)
define(`CA_RUNTIME_CONTROLBYTES_NAME', `MyCompany' `ENDPOINT_NAME' `Runtime ')
define(`CA_SCHEDULE_CORE', 0)
DECLARE_SOF_RT_UUID("My codec", my_sdk_uuid, 0xd95fc34f, 0x370f, 0x4ac7, 0xbc, 0x86, 0xbf, 0xdc, 0x5b, 0xe2, 0x41, 0xe6);
define(`CA_UUID', my_sdk_uuid)
# Include codec adapter playback topology
include(`sof/pipe-eq-iir-codec-adapter-playback.m4')
However, if I specify the following CA_SETUP_CONTROLBYTES, which is the actual setup data I want to use to initialise my software, then I get the failure message in dmesg as above:
# My SDK codec setup config
define(`CA_SETUP_CONTROLBYTES',
`` bytes "0x53,0x4f,0x46,0x00,0x00,0x00,0x00,0x00,'
` 0xd4,0x02,0x00,0x00,0x01,0x20,0x01,0x03,'
` 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,'
` 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,'
` 0x00,0x01,0x41,0x57,0x00,0x00,0x00,0x00,'
` 0x80,0xbb,0x00,0x00,0x10,0x00,0x00,0x00,'
` 0x02,0x00,0x00,0x00,0x01,0x00,0x00,0x00,'
` 0xc0,0x02,0x00,0x00,0x20,0x00,0x00,0x00,'
` 0x64,0x02,0x00,0x00,0x0a,0x6c,0x4e,0x00,'
` 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,'
` 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,'
` 0x00,0x00,0x00,0x00,0x0a,0x6c,0x4e,0x00,'
` 0x00,0x00,0x20,0x00,0x00,0x00,0x20,0x00,'
` 0x00,0x00,0x00,0x00,0x40,0x00,0x00,0x00,'
[SNIP] Omitting ~800 bytes here for brevity
` 0x00,0x00,0x00,0x20,0x08,0x00,0x00,0x00,'
` 0x00,0x00,0x00,0x40,0x1e,0xc7,0xfe,0x12,'
` 0xbc,0x38,0x21,0x2f,0x00,0x00,0x00,0x40,'
` 0x57,0x24,0xfd,0x07,0xe2,0xfa,0xd8,0x00,'
` 0xe0,0x42,0x0a,0x39,0x53,0xd3,0x15,0x00,'
` 0x53,0xd3,0x15,0x00,0x00,0x00,0x00,0x00,'
` 0x33,0xbc,0x06,0x11,0x00,0x00,0x00,0x00,'
` 0x14,0x9e,0xca,0x7e"''
)
Thanks in advance,
Mark
Activity
dbaluta commentedon Oct 4, 2021
@markbartonxperi what is the output you see with sof-logger?
https://thesofproject.github.io/latest/developer_guides/debugability/logger/index.html
markbartonxperi commentedon Oct 4, 2021
@dbaluta Nothing very useful, unfortunately.
When I run with the 'working' topology from my original message, and play audio content then I see all the log messages that I would expect to see, including those coming from my own codec adapter.
markbartonxperi commentedon Oct 4, 2021
Sorry - I should probably add some other more useful diagnostics about my current target system:
SOF Firmware: I'm currently building commit a938cec, plus my own changes to integrate my company's technology. My target platform is cnl.
Up! Xtreme linux kernel: I rebuilt the kernel on Friday from commit 138c1c5da4b5 in the sof/linux repository just in case there had been any upstream changes that addressed my problem (in particular I saw that there was some activity around IPC4, which felt like it might be relevant...)
On my Up! board, I currently see:
markbartonxperi commentedon Oct 4, 2021
@dbaluta I rebuilt the kernel for my Up board with some of the helpful sounding extra debug options enabled. I think setting
SND_SOC_SOF_DEBUG_RETAIN_DSP_CONTEXTmeans that the log data is now persisting. Attaching the output from sof-logger after a restart.Looks like the config has zero size when it reaches the firmware. I will start triple checking my data packing - I've checked this many times already, of course, but maybe I missed something and there is an error there....!
sof-logger-output.txt
juimonen commentedon Oct 5, 2021
@markbartonxperi can you compare the .conf topology files with the working/non-working case. So you have the m4 -> which is converted to .conf -> which alsatplg converts to .tplg binary. Or can you perhaps attach here both? Just to first check that there's nothing strange...
dbaluta commentedon Oct 5, 2021
@markbartonxperi there is definitely something wrong with the tplg file. From the log:
Maybe start adding some prints in comp_new() (ipc/ipc3/helper.c) and understand why ipc_config_specific.size = 0. Check the comp_specific_builder() function.
markbartonxperi commentedon Oct 5, 2021
@juimonen
Hi - I've attached the .conf files for you to have a look at
Thanks!!
sof-hda-generic-non-working.conf.txt
sof-hda-generic-working.conf.txt
juimonen commentedon Oct 5, 2021
@markbartonxperi I could not immediately spot anything wrong in conf files...
@dbaluta am I reading it correctly that size is never set in codec_adapter_set_params, if msg_index is something else than 0?
lgirdwood commentedon Oct 5, 2021
@singalsu can you advise how we are doing this with other components. I dont think there are any size limitations.
@markbartonxperi could you add more debug around
The size will be extracted from an IPC structure or IPC tuple. Btw, M4 does no macro argument validation or verification (this is fixed in topology 2) but please also check your macro argument ordering and count agianst a working version that does the same thing.
dbaluta commentedon Oct 5, 2021
Looking at the code this looks correct.
juimonen commentedon Oct 5, 2021
@dbaluta ok I missed the "static" in the declaration of "size".
If I interpret the dmesg correctly we should be at the 1st large ipc tx. If I recall correctly for large messages we should see tx in dmesg for all parts of the chopped message -> so if the size would be 0, and we would be in first ipc, we should bail out earlier in the codec_adapter_set_params... which makes me think something overwrites the static variable in the middle?
@lgirdwood could we possibly be overwriting the "static size" with mem alloc and memset? or I'm just totally lost here..
markbartonxperi commentedon Oct 5, 2021
@dbaluta @juimonen @lgirdwood
Evening all
I had some time this afternoon to try adding some extra logging on the firmware side.... I'll be honest and say my log message locations felt a bit random given that I don't know the codebase so well. And also don't completely understand the expected behaviour.
Here's a fragment from sof-logger with a few additional prints (note I'm using the error trace macro in favour of working out how to enable the debug trace macro!)
Once I got to this point I noticed that the size logged from src/ipc/ipc3/helper.c:100 (116 bytes) matched the log message from the kernel that the failed IPC tx size was also 116 bytes, which made me wonder if actually my data is being read from the topology incorrectly.... so I switched my efforts over to the kernel side of things.
In my latest run I see this in dmesg:
..which suggests my configuration is being loaded (total size is 756 including ABI header, so 724 bytes when that's subtracted)
Later on in dmesg I see the same as before:
I will keep digging, but would definitely welcome any pointers or corrections :) In particular, is it expected that my 724 bytes should later be logged with a message size of 116 bytes? I feel like the 116 bytes is just the size of a header, and that there's probably some other struct member I should query for the payload size....
Thanks
Mark
juimonen commentedon Oct 5, 2021
@markbartonxperi really nice that you can get logs! I would suggest to put these trace messages all over to codec_adapter_set_params to see that the "static size" is set correctly. like all of the:
"size = cdata->num_elems + cdata->elems_remaining;" you could print out to see if there are some funny values. And even just put prints to clearly see which branches are taken.
I'm yet not clear are we setting the size incorrectly on the kernel side, are we losing it on the way with ipc, or are we losing it inside fw processing...
juimonen commentedon Oct 6, 2021
@markbartonxperi the codec_adapter doesn't behave nicely if the control data size is over MAX_IPC_MSG_SIZE=384...
long story: driver only supports "large" messages with control data. When the widget is loaded -> data is dug out from the related control and added to the widget -> but if the total size is > 384 the control data (initial config) is discarded, as it can't be partly added.
Anyway, you should get the data little bit later when the control is loaded (for e.g. when streaming starts). The component just needs to handle the situation, that is doesn't get the init data in comp_new, but later. This is how for e.g. the eq works.
comp_init_data_blob function is just happy with 0 data and doesn't error out.
So, codec_adapter should not bail out with "0" init data, it needs to be changed on the firmware code. @mrajwa FYI.
9 remaining items
juimonen commentedon Oct 7, 2021
@markbartonxperi somehow we need to fix in codec adapter, if you want to set these "big" parameters in the topology. It doesn't help here to move them around between runtime and setup controls. 1 way to set these is put some smaller parameters in these controls in topology and send the big parameters through alsa control interface in user space.
I think the google hotword works like that. So the big params are in alsa UCM user space config -> when you start streaming, particular use case in UCM is loaded -> it loads the big blob through alsa interface before streaming.
So parameters are not used in the dsp loading, but set separately. so not defined in topology. OTH this sets up dependency for user space to load those -> but in chrome that is existing mechanism (alsa UCM).
But let me still have couple of discussions with the folks how to solve this. We'll get the params there one way or another :)
juimonen commentedon Oct 7, 2021
@markbartonxperi so to mimic the "UCM way" in testing is to setup the component with some smaller parameters. In your case they could be just bogus or whatever. Then before you start to aplay you set the big params with sof-ctl (found in sof codebase). If you set them once they will be there "forever". We use this method with eq all the time.
The unfortunate thing here is that we support big params currently only with kcontrols -> and in initial topology loading the kcontrol values are not set. When component is initially loaded we use a hack where the config is dug out of the kcontrol associated with it -> but this has an issue that the component loading doesn't support big params.
Why this 0-size change above works is because when the dsp wakes up from power save (when you start to stream) it will restore all components and all kcontrols (and send the values to match the state before power save). These kcontrol values are not send in initial loading as the dsp is not in a state to receive these controls (it is still loading the topology).
If we would not go to power save, this thing would break for sure. 99% we are going for power save, but to really make sure your parameters are set -> it needs to be set explicitly from user space.
markbartonxperi commentedon Oct 7, 2021
@juimonen Thanks for the reply - all understood!
I'll leave things in your hands, then. As before, feel free to let me know here if there's anything you want me to test.
Thanks again!
lgirdwood commentedon Oct 7, 2021
@markbartonxperi can you work around this atm ? Hopefully it wont block you in the integration work, this should give some time for clarity on the way ahead.
markbartonxperi commentedon Oct 7, 2021
@lgirdwood Up until yesterday, my workaround had been to hardcode my initial configuration data as a big static array in my codec :)
I can certainly continue to do that for the short term. However, our internal plan is to aim for a PR of our work in the next 7-10 days, so it sounds like I should aim to make use of the ALSA UCM mechanism @juimonen was mentioning in lieu of a change to the codec adapter.
lgirdwood commentedon Oct 13, 2021
@markbartonxperi I'm back from vacation, did you make any progress with UCM ?
Fwiw, @juimonen and I are looking at a big improvement here in topology2 whereby binary data can be inserted into topology using a command line tool i.e. the topology or UCM files can invoke a binary cmd line tool that would insert the data into the topology file or kcontrol. This cmd line tool can be open or closed source and its arguments would be supplied from the topology or UCM data. This would mean we can remove all the
bytessections from topology and replace with tool invocations.markbartonxperi commentedon Oct 13, 2021
@lgirdwood Hi - welcome back - hope you had a good break!
I made a little bit of progress with UCM - mostly getting to grips with the
alsaucmcommand as well as the format of the.conffiles. There was also a little work required on my tooling-side to generate suitable binary files. At that point I hit a bit of roadblock as I managed to runalsaucmin some way that killed the audio output of my Up Xtreme....So along with recovering the UP board, and a pre-PR refactor of our code, I haven't quite got to the point where I've successfully applied settings via UCM.... however, I feel confident that this approach will work for us in the shorter term. Longer term, it sounds like topology2 will be the way to go.
plbossart commentedon Oct 13, 2021
@markbartonxperi topology and UCM are different things. With UCM you can add SKU-specific changes and use-case/endpoint-specific settings. The topology is good only for settings that don't change as a result of user/use-case changes.
juimonen commentedon Oct 13, 2021
@markbartonxperi this hasn't been forgotten, I'll have a meeting with the architects and other technical folks soon as we need to sort couple of things out with the codec adapter design:
Just little bit slow to get everybody into same meeting :)
markbartonxperi commentedon Oct 18, 2021
@lgirdwood @juimonen A quick update from me.
I've now had a bit more time to look into using UCM to do what I was previously trying to do via the configuration bytes in the topology and this is going to work well for us - to be honest, this was almost certainly the way I should've been doing things in the first place!
@plbossart I probably wasn't very clear in my earlier answer.... my interpretation of what @lgirdwood was saying was that as a part of topology2, I will have build-time tools available to me that let me customise aspects of the topology at build-time based on configuration options I provide. Whilst UCM gives me a good solution, it feels a bit fragile that the data driving the settings for my processing lives outside the topology and only gets sent if everything UCM-land is configured correctly. If I had the option when building for e.g. ChromeBook xyz, to inject the configuration bytes specific to that machine into my .m4 pipeline at build time so that they are then 'baked' into the topology itself, then that feels a bit more robust in terms of knowing my code will always get the correct settings for the specific machine and won't be at the mercy of any user-fiddling.
plbossart commentedon Oct 18, 2021
@markbartonxperi like I said it depends on the usage. If you have the same pipeline in the topology that may be shared in headphone and speaker mode, you will need UCM to provide different settings. the topology is self-contained but doesn't have any sense of usage, i.e. the topology does not know what physical endpoint is used and what changes of routing need to be accounted for.
I don't disagree with the fact that UCM and topology can be misaligned, that's one of the main problems I have with the separate releases of packages without clear dependency requirements.
lgirdwood commentedon Oct 18, 2021
@markbartonxperi fwiw @juimonen should be able to add more detail this week on supporting configuration data. This will appear in the local ALSA branch at first until we can upstream into ALSA main. @juimonen can you make sure @markbartonxperi is added as a reviewer. Thanks !
lgirdwood commentedon Dec 1, 2021
@markbartonxperi all good now, can we close ?
markbartonxperi commentedon Dec 2, 2021
@lgirdwood Afternoon. Yes - all good now! Thanks for your help!