[alsa-devel] [Sound-open-firmware] Loading coefficients for Audio IP
Pierre-Louis Bossart
pierre-louis.bossart at linux.intel.com
Wed Dec 19 17:26:36 CET 2018
Hi Daniel,
> What is the standard ALSA way of loading tables of coefficients for
> various audio IPs?
>
> We have IPs like PDM or ASRC which need large table of coefficients
> depending on configuration. This tables can reach up to 120K in size.
>
> Creating this kind of tables as arrays of integers in kernel doesn't seem
> to be a common used solution and we will end up with large files that
> contains only numbers.
>
> Another approach that we are thinking of is to create and compile this
> tables in userspace as raw binary files and then loading them in kernel
> using the firmware interface.
I think there are two separate parts in your question (not trying to
pontificate but split variables).
1. how do we download binary tables that are related to hardware
interfaces or firmware modules?
In that case the "coefficients" don't depend on use cases but are set in
stone either at build time or when the device is flashed. They never
vary once the boot starts. You absolutely do not want these coefficients
in the kernel, and I can think of two ways of dealing with this case
1.a. request_firmware(). If you are using the topology framework, there
may be a way of adding tokens so that the file names for the coefficient
doesn't have to be a constant string in the kernel.
1.b adding binary data as part of the topology file. I vaguely recall
discussing this with Liam, not sure if it's feasible.
1.c a new custom interface to add modules to the firmware. Liam worked
on that part to add new code, but it could be used for coefficients
since the symbol address resolution would be identical.
2. how do we download binary data related to processing/algorithms,
which can vary depending on use cases and possibly user interaction?
That part is more complicated and I don't have a turn-key solution, I
only started looking into this last week and you'll find below all my
raw notes (all based on publicly-available information).
The main issue here is that ALSA BYTE controls are limited to 512 bytes,
which doesn't cut it for most applications. There were two successive
changes, one to add "extended controls" and a second to use TLVs. I
personally missed that second step and wasn't aware that they were used
by Intel in the Skylake driver. After looking in more details at all the
online information I am really not hot on this direction. There are a
couple of issues with TLVs
2.a The API was abused big time. The "T" in TLV should have been a
standard one, but it turns out everyone just used whatever they wanted.
2.b. the API isn't a good fit for the type of data being handled. the
"L" in TLV is supposed to indicate the length of the binary data, but
the API can also pass a length information. I find this really
confusing. Such binary data should be handled in an 'atomic' way, all or
none.
2.c and fundamentally I don't see what TLVs bring over regular extended
controls. If the driver ignores the length passed by the application and
only uses the encoded length, then it'd just be equivalent to use a
custom encoding and an extended control.
In the notes from the last two or three ALSA miniconfs there are
mentions of looking into other directions such as hwdep:. I must admit I
am not familiar with this interface so there's some learning needed on
my side. Patrick Lai from Qualcomm also mentioned a number of times the
need to support 'calibration' files and I wonder what the need really is.
I am also wondering if there is a need for reads of large binary data. I
can see cases where you could extract state information, but this could
be abused if people used it for probes or data logging, which is really
streaming and can be handled e.g. with the compressed API. And you would
probably need a 'freeze/snapshot' command in the first place otherwise
the state could be inconsistent between the initial part of the read and
the last.
At any rate, for SOF I don't feel we have to comply with any legacy, but
rather we should try and help the ALSA community do the right thing with
a solution that is agreed upon and will be acceptable for a number of
years. Restarting with controversial concepts doesn't seem like a good
direction to me.
Most likely something that will happen in 2019 though. Happy Holidays y'all.
Regards
-Pierre
Raw notes on ALSA binary control support:
2014-05-02
d98812082c87
ASoC: add SND_SOC_BYTES_EXT
we need _EXT version for SND_SOC_BYTES so that DSPs can use this to
pass data
for DSP modules
Signed-off-by: Vinod Koul <vinod.koul at intel.com>
2014-07-15
7523a271682fc
ASoC: core: add a helper for extended byte controls using TLV
ALSA supports arbitrary length TLVs for each kcontrol that can be used
to pass metadata about the control (e.g. volumes, enum
information). The
same transport mechanism is now used for arbitrary length data by
defining a new helper.
Signed-off-by: Omair Mohammed Abdullah <omair.m.abdullah at intel.com>
Signed-off-by: Vinod Koul <vinod.koul at intel.com>
2016-11-23
50a4f98d3452
ASoC: core: mark SND_SOC_BYTES_EXT as deprecated
Since we have SND_SOC_BYTES_TLV control to lets devices have
larger size data sent, we do not need SND_SOC_BYTES_EXT with 512
byte limitation so mark it deprecated
Signed-off-by: Vinod Koul <vinod.koul at intel.com>
[PLB] I have no idea why TLVs were marked as a better solution
2015-09-15
[alsa-devel] [PATCH] Revert "ASoC: core: mark SND_SOC_BYTES_EXT as
deprecated"
SND_SOC_BYTES_TLV brings confusions to user land because it doesn't follow
to a protocol of ctl and tlv operation. At least, this macro and related
kernel APIs include two misunderstandings:
- 'struct snd_ctl_elem_info.count' can also represent the length of TLV
packet paylaod, snd_soc_bytes_info_ext() performs in this way.
- 'struct snd_ctl_tlv.tlv' can include arbitrary data regardless of TLV
packet structure, snd_soc_bytes_tlv_callback() performs in this way.
In a policy of kernel land development, it's quite worse to break protocols
for applications. Therefore, developers are discouraged to use these kernel
APIs.
In the first place, SND_SOC_BYTES_TLV was added to satisfy a request of
developers who need to add control elements which transfer data larger than
the size which 'struct snd_ctl_elem_value' can represent; e.g. over 512
bytes. However, as long as the size is less than the size; e.g. 512 bytes,
SND_SOC_BYTES_EXT is still available. Although there is actually the
limitation of maximum data size, it's better to use this API for stable
application interface till better alternative ways are implemented in
future.
For these reasons, this commit reverts the previous commit which lead
developers to the worse behaviour.
Signed-off-by: Takashi Sakamoto <o-takashi at sakamocchi.jp>
[PLB] Sakamoto-san was very vocal on this and provided even more details at
https://patchwork.kernel.org/patch/9304649/
While this thread is difficult to follow (turn off social media and get
a large mug of coffee), it does go over all the known TLV problems. One
of the main issues with TLV seems to be that if the application passes a
large buffer of data, only the parts that can be handled at the ASoC
level will be handled. The application has no means to know how much was
written nor how much it gets back on a read. Maybe this could be
improved by first querying the control size and asking the application
to respect that max size. But to some extent letting the application
muck with the length isn't quite right in the first place, the data
should be used as is in an atomic way.
In latest code:
git grep SND_SOC_BYTES_EXT
include/sound/soc.h: * SND_SOC_BYTES_EXT is deprecated, please USE
SND_SOC_BYTES_TLV instead
include/sound/soc.h:#define SND_SOC_BYTES_EXT(xname, xcount,
xhandler_get, xhandler_put) \
sound/soc/codecs/da7218.c: SND_SOC_BYTES_EXT("Sidetone BiQuad
Coefficients",
sound/soc/codecs/da7218.c: SND_SOC_BYTES_EXT("BiQuad Coefficients",
sound/soc/codecs/nau8810.c: SND_SOC_BYTES_EXT("EQ Parameters", 10,
sound/soc/codecs/nau8822.c: SND_SOC_BYTES_EXT("EQ Parameters", 10,
sound/soc/codecs/nau8825.c: SND_SOC_BYTES_EXT("BIQ Coefficients", 20,
sound/soc/codecs/wm5102.c:SND_SOC_BYTES_EXT("Output Compensation
Coefficient", 2,
sound/soc/intel/haswell/sst-haswell-pcm.c: SND_SOC_BYTES_EXT("Waves Set
Param", WAVES_PARAM_COUNT,
git grep SND_SOC_BYTES_TLV
include/sound/soc.h: * SND_SOC_BYTES_EXT is deprecated, please USE
SND_SOC_BYTES_TLV instead
include/sound/soc.h:#define SND_SOC_BYTES_TLV(xname, xcount,
xhandler_get, xhandler_put) \
[PLB]: There is no clear mention in the kernel code that TLVs are
actually used
git grep SKL_CONTROL_TYPE_BYTE_TLV
include/uapi/sound/skl-tplg-interface.h:#define
SKL_CONTROL_TYPE_BYTE_TLV 0x100
sound/soc/intel/skylake/skl-topology.c: {SKL_CONTROL_TYPE_BYTE_TLV,
skl_tplg_tlv_control_get,
static int skl_tplg_tlv_control_get(struct snd_kcontrol *kcontrol,
unsigned int __user *data, unsigned int size)
{
struct soc_bytes_ext *sb =
(struct soc_bytes_ext *)kcontrol->private_value;
struct skl_algo_data *bc = (struct skl_algo_data *)sb->dobj.private;
struct snd_soc_dapm_widget *w = snd_soc_dapm_kcontrol_widget(kcontrol);
struct skl_module_cfg *mconfig = w->priv;
struct skl *skl = get_skl_ctx(w->dapm->dev);
if (w->power)
skl_get_module_params(skl->skl_sst, (u32 *)bc->params,
bc->size, bc->param_id, mconfig);
/* decrement size for TLV header */
size -= 2 * sizeof(u32);
/* check size as we don't want to send kernel data */
if (size > bc->max)
size = bc->max;
if (bc->params) {
if (copy_to_user(data, &bc->param_id, sizeof(u32)))
return -EFAULT;
if (copy_to_user(data + 1, &size, sizeof(u32)))
return -EFAULT;
if (copy_to_user(data + 2, bc->params, size))
return -EFAULT;
}
return 0;
}
static int skl_tplg_tlv_control_set(struct snd_kcontrol *kcontrol,
const unsigned int __user *data, unsigned int size)
{
struct snd_soc_dapm_widget *w = snd_soc_dapm_kcontrol_widget(kcontrol);
struct skl_module_cfg *mconfig = w->priv;
struct soc_bytes_ext *sb =
(struct soc_bytes_ext *)kcontrol->private_value;
struct skl_algo_data *ac = (struct skl_algo_data *)sb->dobj.private;
struct skl *skl = get_skl_ctx(w->dapm->dev);
if (ac->params) {
if (size > ac->max)
return -EINVAL;
ac->size = size;
/*
* if the param_is is of type Vendor, firmware expects actual
* parameter id and size from the control.
*/
if (ac->param_id == SKL_PARAM_VENDOR_ID) {
if (copy_from_user(ac->params, data, size))
return -EFAULT;
} else {
if (copy_from_user(ac->params,
data + 2, size))
return -EFAULT;
}
if (w->power)
return skl_set_module_params(skl->skl_sst,
(u32 *)ac->params, ac->size,
ac->param_id, mconfig);
}
return 0;
}
[PLB] So Skylake does appear to use an implicit TLV, but the TLV
information is not propagated as is to firmware. Custom methods are used
to pass the data in chunks.
https://www.alsa-project.org/main/index.php/Miniconf_2017
TLV API for binary controls
Picking up the discussion from last year.
Problem: Can't figure out what kind of TLV control it is
It's a hack to use TLV for binary controls, probably worth reconsidering.
Only users are Cirrus and Intel. Only for controls with more than 512
bytes for Cirrus.
Tidy up existing one? Add a flag saying that it's binary nonsense -
disagreement.
Add TLV headers in the data? Probably breaks Intel who have headers that
look like they mean something.
One goal for using TLV was to get data through existing API clients.
Takashi would prefer to add a new API for this.
Indirect pointers sound complicated.
Maybe use file based interface (ioctl takes a fd).
Sounds like new ioctls, Charles volunteers to implement.
https://www.alsa-project.org/main/index.php/Miniconf_2018
Byte controls
Already covered, Sakamoto-san’s new elem API probably can have no
limit if desired
Vinod: it’s easier for usespace to handle a single userspace interface
Charles: Cirrus has few large controls
Having to extend every single UCM implementation is a pain
tiwai: controls need to be readable at any time
Calibration loading
QC currently has a calibration mechanism parallel to standard
ALSA one. QC code available on codeaurora. Want to re-do this with
upstream constructs.
2 methods aware of upstream currently
Pass calibration through byte control from user space
Go through request firmware
Discussion converging towards hwdep
Vinod: fudge hwdep in alsa-lib
Charles: happy to investigate what the hwdep solution would
look like from cirrus PoV
Patrick: need to provide different calibration data depending
on sample rate.
UCM: Problems with Android but otherwise good
More information about the Alsa-devel
mailing list