thesofproject
ASoC: SOF: ipc4-topology: Use the single format specified by the dai copier if possible
Hi,
needs comment and commit message updates, just to show the idea I had regarding to #4867
I think there is one issue that this might have is if the DAI copier for DMIC have multiple bit depths then it is not going to force the 32bit, but forcing unconditionally 32bit blob w/o knowing if the copier have support for it in the first place is not correct. what if we have 32bit blob and the copier only supports 16bit bit depth?
I think if we load the 32bit blob forcibly then we need to change the ref_params so that we will be looking for 32bit cormat support on the dai copier...
@ranj063, any thoughts?
Changes since v1:
- fix i386 build ( sof_ipc4_prepare_copier_module() is a monster function... )
Changes since v1:
- Fix bug found by @singalsu : the sof_ipc4_update_hw_params() should be called with BIT(SNDRV_PCM_HW_PARAM_FORMAT) when fixing up the format of the params in case of single format
- Tested by @singalsu
TODO: commit message and comment updates Fix corner case when we want to pick 32bit format on DAI side but the copier declares multiple bit depths (atm we would go with the fe_params format).
Changes since v2:
- reduce the patch count to two
- Move the dai_copier preparation code out to a separate function
- The NHLT blob lookup now updates the params it receives in case different format is used to pick the blob (to find matching input settings)
- Comments updated, added
- Commit message proposal added
- Tested by @singalsu
I re-checked this version, works OK. Commit text of 1st is not explaining all changes included?
Let me check the commit message, thanks for testing it!
I can't figure out what the second commit is "replace, move the original here! "ASoC: SOF: ipc4-topology: Look for … …the 32-bit DMIC blob first"" ??
nor if it's better than https://github.com/thesofproject/linux/pull/4867
Shouldn't there be some discussion on alternatives and what the best option is?
I can't figure out what the second commit is "replace, move the original here! "ASoC: SOF: ipc4-topology: Look for … …the 32-bit DMIC blob first"" ??
That is a replacement patch for the one which requested 32-bit blob for DMIC. For upstream the two patch needs to be combined with an updated commit message as a new commit. Yeah, I could have just reverted the other in this PR...
nor if it's better than #4867
This does things in revers and it is not built on assumptions. You see: with #4867 we would unconditionally select 32bit blob for DMIC even if the copier does not support 32bit. I know that it is, but that is an assumption, right? This PR does things in reverse: it looks for a blob which matches with the single format supported by the copier, so indirectly we will have 32bit blob if the copier only supports 32bit, but this PR also handles the case when the copier have multiple formats, we will look for 32bit blob and if it is there we will look for the 32bit copier config and not a probably mismatching one from fe_params
Shouldn't there be some discussion on alternatives and what the best option is?
Certainly!
On a local laptop: On boot the usb dongle is not detected (usb-c -> usb-a -> gigabit). unplug / plug -> nothing in dmesg lsusb shows other devices Plug the dongle to other C port and it got detected and now in the original port it is working after boot.
alsabat failed on mtl-sdw but it looks like a false positive as #4908 have the alsabat fail on the same CI device.
Changes since v3:
- separate out the revert of #4840 patch
- Update the commit message for the second patch (was the first previously)
only alsa-bat issues in https://sof-ci.01.org/linuxpr/PR4899/build2589/devicetest/index.html, merging.