On Fri, Sep 02, 2016 at 08:18:04PM +0900, Takashi Sakamoto wrote:
Clemens,
On Aug 31 2016 20:54, Clemens Ladisch wrote:
Takashi Iwai wrote:
TLV has never been and (will never be) an API to handle a generic binary stream.
It would be possible to define something like SNDRV_CTL_TLVT_HWDEP_BLOB or _COEFFICIENTS, or to reserve a range of TLVT values for driver- defined types. (But we cannot change soc-wm-adsp to support only proper TLV data, because this would introduce a regression.)
For our information, I wrote a patch including your idea.
Thanks for your bright comment ;)
Takashi Sakamoto
---- 8< ----
From d1a0eabf1b33fae5de7cedf7aee23778f5dcf46c Mon Sep 17 00:00:00 2001
From: Takashi Sakamoto o-takashi@sakamocchi.jp Date: Fri, 2 Sep 2016 00:12:20 +0900 Subject: [PATCH] ALSA: control: coefficient support
include/uapi/sound/asound.h | 5 +++++ include/uapi/sound/tlv.h | 1 + sound/soc/codecs/wm_adsp.c | 3 ++- sound/soc/soc-ops.c | 37 ++++++++++++++++++++++++++++++++++--- sound/soc/soc-topology.c | 3 ++- 5 files changed, 44 insertions(+), 5 deletions(-)
diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h index 609cadb..69c0585 100644 --- a/include/uapi/sound/asound.h +++ b/include/uapi/sound/asound.h @@ -848,6 +848,11 @@ 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 */ +/*
- Arbitrary data is accepted for coefficients, instead of pure
threshold level
- information.
- */
+#define SNDRV_CTL_ELEM_ACCESS_TLV_COEFF (1<<7)
Yeah the addition of a new flag here would very much get my vote. The only way at the moment to tell if you are dealing with this type of control from user-space is to look for a control of type SNDRV_CTL_ELEM_TYPE_BYTES that doesn't have SNDRV_CTL_ELEM_ACCESS_READ or SNDRV_CTL_ELEM_ACCESS_WRITE. Which is far from ideal.
#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/include/uapi/sound/tlv.h b/include/uapi/sound/tlv.h index ffc4f20..fd1c867 100644 --- a/include/uapi/sound/tlv.h +++ b/include/uapi/sound/tlv.h @@ -19,6 +19,7 @@ #define SNDRV_CTL_TLVT_DB_RANGE 3 /* dB range container */ #define SNDRV_CTL_TLVT_DB_MINMAX 4 /* dB scale with min/max */ #define SNDRV_CTL_TLVT_DB_MINMAX_MUTE 5 /* dB scale with min/max with mute */ +#define SNDRV_CTL_TLVT_COEFF 6 /* Arbitrary data */
/*
- channel-mapping TLV items
diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c index 21fbe7d..525d70b 100644 --- a/sound/soc/codecs/wm_adsp.c +++ b/sound/soc/codecs/wm_adsp.c @@ -930,7 +930,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 |
} else { rd = SNDRV_CTL_ELEM_ACCESS_READ; wr = SNDRV_CTL_ELEM_ACCESS_WRITE;SNDRV_CTL_ELEM_ACCESS_TLV_COEFF;
diff --git a/sound/soc/soc-ops.c b/sound/soc/soc-ops.c index a513a34..14cdd59 100644 --- a/sound/soc/soc-ops.c +++ b/sound/soc/soc-ops.c @@ -31,6 +31,7 @@ #include <sound/soc.h> #include <sound/soc-dpcm.h> #include <sound/initval.h> +#include <sound/tlv.h>
/**
- snd_soc_info_enum_double - enumerated double mixer info callback
@@ -773,19 +774,49 @@ int snd_soc_bytes_tlv_callback(struct snd_kcontrol *kcontrol, int op_flag, unsigned int size, unsigned int __user *tlv) { struct soc_bytes_ext *params = (void *)kcontrol->private_value;
- unsigned int count = size < params->max ? size : params->max;
unsigned int count;
unsigned int type; int ret = -ENXIO;
/*
* The TLV packet can transfer numerical ID for one control element.
* But ALSA control core don't tell it to each implementation of
* TLV callback. Here, instead, use the first volatile data to
* check access information.
*/
if (!(kcontrol->vd[0].access & SNDRV_CTL_ELEM_ACCESS_TLV_COEFF))
return -ENXIO;
/* The data should be constructed according to TLV protocol. */
if (copy_from_user(&type, tlv, sizeof(unsigned int)))
return -EFAULT;
/* The type should be an arbitrary data. */
if (type != SNDRV_CTL_TLVT_COEFF)
return -ENXIO;
I agree it is probably a bit late to add this now.
Thanks, Charles