[alsa-devel] [PATCH] ALSA: core: Add new ACCESS_TLV flag for coefficient TLV controls
Charles Keepax
ckeepax at opensource.cirrus.com
Mon Oct 2 18:04:21 CEST 2017
ASoC can use the TLV mechanism to pass coefficient data for
controls that exceed the normal 512 byte limit. However, there
is no mechanism to inform user-space the control expects data
through the TLV mechanism. Currently amixer uses a combination
of the absense of non-TLV read/write permissions and the type
being BYTES. This is not ideal as it is not a clear signal and
prevents controls that might support both access mechanisms.
Add a specific flag SNDRV_CTL_ELEM_ACCESS_TLV_COEFF to let
user-space know when it is dealing with a TLV control that is
being used to pass actual coefficient data. Then add this flag
to the controls generated by the current users of these types of
controls.
Signed-off-by: Charles Keepax <ckeepax at opensource.cirrus.com>
---
Hi,
This builds on work done by Sakamoto-san [1] and the changes
mentioned to amixer in the commit message are found in [2]. I
think something like this should probably be added, what I am
less clear on is where we go from here?
Sakamoto-san's original patches also added a tag type
SNDRV_CTL_TVLT_COEFF, to keep the data matching the normal format
found in TLV controls. The trouble is neither of the two users
really follow this convention at the moment. The ADSP code just
takes the data as is with no additional headers and the Skylake
code does have a tag and a length as part of the data passed
through the TLV however the tag part looks like it is used for
some internal purpose, so I am far from clear it would support a
fixed value to identify the data without some changes as well.
Changing the ADSP code is probably acceptable since we have had
very few users in the field, and since this work has been done
improvements in the firmware tool chains mean most firmwares are
created with much smaller dedicated purpose controls rather than
the large block ones that were initially the reason for using
these controls on our side.
Finally, since [3] tinymix has been updated to add some TLV
information into control writes/reads which will break the ADSP
code once it trickles down to users. So I would be keen to
discuss this at this years conference and decide how we are going
to handle these controls? As Mark says [4] maybe we should see if
there is a better approach we should all migrate across to?
Thanks,
Charles
[1] https://patchwork.kernel.org/patch/9304649/
[2] https://patchwork.kernel.org/patch/8147001/
[3] https://github.com/tinyalsa/tinyalsa/commit/3f813e47784674a3909fe1277bc10b70d03791e2
[4] http://mailman.alsa-project.org/pipermail/alsa-devel/2017-September/125743.html
include/sound/soc.h | 3 ++-
include/uapi/sound/asound.h | 1 +
sound/core/control.c | 1 +
sound/soc/codecs/wm_adsp.c | 3 ++-
sound/soc/intel/skylake/skl-topology.c | 1 +
sound/soc/soc-ops.c | 3 +++
6 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/include/sound/soc.h b/include/sound/soc.h
index d776cdee30d7..d82297a95caf 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -323,7 +323,8 @@
#define SND_SOC_BYTES_TLV(xname, xcount, xhandler_get, xhandler_put) \
{ .iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname, \
.access = SNDRV_CTL_ELEM_ACCESS_TLV_READWRITE | \
- SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK, \
+ SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK | \
+ SNDRV_CTL_ELEM_ACCESS_TLV_COEFF, \
.tlv.c = (snd_soc_bytes_tlv_callback), \
.info = snd_soc_bytes_info_ext, \
.private_value = (unsigned long)&(struct soc_bytes_ext) \
diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h
index 1949923a40bf..4bf0b2f835e9 100644
--- a/include/uapi/sound/asound.h
+++ b/include/uapi/sound/asound.h
@@ -853,6 +853,7 @@ typedef int __bitwise snd_ctl_elem_iface_t;
#define SNDRV_CTL_ELEM_ACCESS_TLV_WRITE (1<<5) /* TLV write is possible */
#define SNDRV_CTL_ELEM_ACCESS_TLV_READWRITE (SNDRV_CTL_ELEM_ACCESS_TLV_READ|SNDRV_CTL_ELEM_ACCESS_TLV_WRITE)
#define SNDRV_CTL_ELEM_ACCESS_TLV_COMMAND (1<<6) /* TLV command is possible */
+#define SNDRV_CTL_ELEM_ACCESS_TLV_COEFF (1<<7) /* Coefficient data is accepted */
#define SNDRV_CTL_ELEM_ACCESS_INACTIVE (1<<8) /* control does actually nothing, but may be updated */
#define SNDRV_CTL_ELEM_ACCESS_LOCK (1<<9) /* write lock */
#define SNDRV_CTL_ELEM_ACCESS_OWNER (1<<10) /* write lock owner */
diff --git a/sound/core/control.c b/sound/core/control.c
index 56b3e2d49c82..e7a74d48633b 100644
--- a/sound/core/control.c
+++ b/sound/core/control.c
@@ -266,6 +266,7 @@ struct snd_kcontrol *snd_ctl_new1(const struct snd_kcontrol_new *ncontrol,
SNDRV_CTL_ELEM_ACCESS_INACTIVE |
SNDRV_CTL_ELEM_ACCESS_TLV_READWRITE |
SNDRV_CTL_ELEM_ACCESS_TLV_COMMAND |
+ SNDRV_CTL_ELEM_ACCESS_TLV_COEFF |
SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK);
err = snd_ctl_new(&kctl, count, access, NULL);
diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c
index 65c059b5ffd7..1d7fe0d8a18a 100644
--- a/sound/soc/codecs/wm_adsp.c
+++ b/sound/soc/codecs/wm_adsp.c
@@ -1156,7 +1156,8 @@ static unsigned int wmfw_convert_flags(unsigned int in, unsigned int len)
wr = SNDRV_CTL_ELEM_ACCESS_TLV_WRITE;
vol = SNDRV_CTL_ELEM_ACCESS_VOLATILE;
- out = SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK;
+ out = SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK |
+ SNDRV_CTL_ELEM_ACCESS_TLV_COEFF;
} else {
rd = SNDRV_CTL_ELEM_ACCESS_READ;
wr = SNDRV_CTL_ELEM_ACCESS_WRITE;
diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c
index 27bcb62568fb..01b01c6a2bf3 100644
--- a/sound/soc/intel/skylake/skl-topology.c
+++ b/sound/soc/intel/skylake/skl-topology.c
@@ -2870,6 +2870,7 @@ static int skl_tplg_control_load(struct snd_soc_component *cmpnt,
tplg_bc = container_of(hdr,
struct snd_soc_tplg_bytes_control, hdr);
if (kctl->access & SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK) {
+ kctl->access |= SNDRV_CTL_ELEM_ACCESS_TLV_COEFF;
sb = (struct soc_bytes_ext *)kctl->private_value;
if (tplg_bc->priv.size)
return skl_init_algo_data(
diff --git a/sound/soc/soc-ops.c b/sound/soc/soc-ops.c
index 500f98c730b9..5ac2afe12b5d 100644
--- a/sound/soc/soc-ops.c
+++ b/sound/soc/soc-ops.c
@@ -776,6 +776,9 @@ int snd_soc_bytes_tlv_callback(struct snd_kcontrol *kcontrol, int op_flag,
unsigned int count = size < params->max ? size : params->max;
int ret = -ENXIO;
+ if (!(kcontrol->vd[0].access & SNDRV_CTL_ELEM_ACCESS_TLV_COEFF))
+ return -ENXIO;
+
switch (op_flag) {
case SNDRV_CTL_TLV_OP_READ:
if (params->get)
--
2.11.0
More information about the Alsa-devel
mailing list