[RFC V2 PATCH 10/11] ALSA: mixer_audient: add control value caching

[Christian Ruppert <arc@gmx.li>]

In preparation of id14 support, add control value caching.

Signed-off-by: Christian Ruppert <arc@gmx.li>
Co-developed-by: Will Porter <mrwillporter@gmail.com>
Signed-off-by: Will Porter <mrwillporter@gmail.com>
Assisted-by: Claude:claude-opus-5-5
---
 sound/usb/mixer_audient.c | 186 ++++++++++++++++++++++++++++++--------
 1 file changed, 150 insertions(+), 36 deletions(-)

diff --git a/sound/usb/mixer_audient.c b/sound/usb/mixer_audient.c
index f61f9fe61345..856fb669af3f 100644
--- a/sound/usb/mixer_audient.c
+++ b/sound/usb/mixer_audient.c
@@ -40,6 +40,7 @@ static const DECLARE_TLV_DB_MINMAX_MUTE(snd_audient_tlv_mixer, -12800, 600);
 #define SND_AUDIENT_CTYPE_FLAGS_SIGNED		BIT(0)
 #define SND_AUDIENT_CTYPE_FLAGS_INIT_HW		BIT(1)
 #define SND_AUDIENT_CTYPE_FLAGS_FORCE_SET	BIT(2)
+#define SND_AUDIENT_CTYPE_FLAGS_CACHE		BIT(3)
 
 static const struct snd_audient_ctrl_type {
 	snd_ctl_elem_type_t type;
@@ -51,7 +52,7 @@ static const struct snd_audient_ctrl_type {
 	union {
 		long integer;
 	} max;		/* for booleans holds physical int value for true */
-	u32 init;
+	u32 init;	/* hardware and cache are initialized to this value */
 	const unsigned int *tlv;
 	u8 unit;
 	u8 sel;
@@ -91,7 +92,8 @@ static const struct snd_audient_ctrl_type {
 	},
 	[SND_AUDIENT_TYPE_MIXER] = {
 		.type = SNDRV_CTL_ELEM_TYPE_INTEGER,
-		.flags = SND_AUDIENT_CTYPE_FLAGS_SIGNED,
+		.flags = SND_AUDIENT_CTYPE_FLAGS_SIGNED
+			 | SND_AUDIENT_CTYPE_FLAGS_CACHE,
 		.len = 2,
 		.min.integer = -128 * 256,
 		.max.integer = 6 * 256,
@@ -238,6 +240,26 @@ static inline u16 snd_audient_kctl_channels(struct snd_kcontrol *kctl)
 	return info->channels;
 }
 
+/*
+ * A cached value is valid once a transfer confirmed it: a read, or a write
+ * that succeeded. A probe transfer that fails leaves it invalid. CACHE |
+ * FORCE_SET without INIT_HW keeps the earlier behaviour: the init value is
+ * assumed, without a transfer.
+ */
+struct snd_audient_cache_entry {
+	u32 value;
+	bool valid;
+};
+
+/* Cache entry of element channel i, or NULL for an uncached control. */
+static inline struct snd_audient_cache_entry *
+snd_audient_cache_slot(struct usb_mixer_elem_info *elem, unsigned int i)
+{
+	struct snd_audient_cache_entry *const cache = elem->private_data;
+
+	return cache ? cache + i : NULL;
+}
+
 
 #define AUDIENT_REQTYPE_SET (USB_DIR_OUT | USB_TYPE_CLASS \
 					 | USB_RECIP_INTERFACE)
@@ -287,12 +309,26 @@ static inline int snd_audient_set_cur(struct usb_mixer_interface *mixer,
 
 static int snd_audient_raw_get_one(struct usb_mixer_interface *const mixer,
 			const struct snd_audient_ctrl_type *const ctype,
+			struct snd_audient_cache_entry *const slot,
 			const u8 cidx, u32 *const value)
 {
 	int ret;
 	__le32 buf = 0;
 	s32 val;
 
+	if (slot && slot->valid) {
+		*value = slot->value;
+
+		return ctype->len;
+	}
+
+	/*
+	 * Cached FORCE_SET controls are treated as write-only: an invalid
+	 * cache entry does not trigger a GET.
+	 */
+	if (slot && (ctype->flags & SND_AUDIENT_CTYPE_FLAGS_FORCE_SET))
+		return -ENODATA;
+
 	ret = snd_audient_get_cur(mixer, ctype, cidx, &buf);
 	if (ret < 0)
 		return ret;
@@ -304,43 +340,58 @@ static int snd_audient_raw_get_one(struct usb_mixer_interface *const mixer,
 
 	*value = val;
 
+	if (slot) {
+		slot->value = val;
+		slot->valid = true;
+	}
+
 	return ret;
 }
 
 static int snd_audient_raw_set_one(struct usb_mixer_interface *const mixer,
 			const struct snd_audient_ctrl_type *const ctype,
-			const u8 cidx, const u32 value)
+			struct snd_audient_cache_entry *const slot,
+			const u8 cidx, const u32 value,
+			bool force)
 {
 	int ret;
 	u32 val;
 
-	if (ctype->flags & SND_AUDIENT_CTYPE_FLAGS_FORCE_SET) {
+	if (force) {
 		val = !value;		/* just make it different */
 	} else {
-		ret = snd_audient_raw_get_one(mixer, ctype, cidx, &val);
-		if (ret < 0)
+		ret = snd_audient_raw_get_one(mixer, ctype, slot, cidx, &val);
+		if (ret == -ENODATA)	/* FORCED_SET and not cached yet */
+			val = !value;	/* just make it different */
+		else if (ret < 0)
 			return ret;
 	}
 
-	if (val != value) {
+	force = force || (ctype->flags & SND_AUDIENT_CTYPE_FLAGS_FORCE_SET);
+
+	if (force || (val != value)) {
 		__le32 buf = cpu_to_le32(value);
 
 		ret = snd_audient_set_cur(mixer, ctype, cidx, &buf);
 		if (ret < 0)
 			return ret;
 
-		return 1;
+		if (slot) {
+			slot->value = value;
+			slot->valid = true;
+		}
+
 	}
 
-	return 0;
+	return val != value;
 }
 
 
 static int snd_ctl_audient_boolean_get(struct snd_kcontrol *kctl,
 				       struct snd_ctl_elem_value *ctl_val)
 {
-	struct usb_mixer_elem_list *const list = snd_kcontrol_chip(kctl);
-	struct usb_mixer_interface *const mixer = list->mixer;
+	struct usb_mixer_elem_info *const elem = snd_kcontrol_chip(kctl);
+	struct usb_mixer_interface *const mixer = elem->head.mixer;
 
 	const unsigned int nchan = snd_audient_kctl_channels(kctl);
 	const unsigned int hwchan = snd_audient_kctl_hwchannels(kctl);
@@ -353,13 +404,19 @@ static int snd_ctl_audient_boolean_get(struct snd_kcontrol *kctl,
 	CLASS(snd_usb_lock, pm)(mixer->chip);
 	if (pm.err < 0)
 		return pm.err;
+	/* serializes the transfers and the cache, also between gets */
+	guard(mutex)(&mixer->lock);
 
 	for (i = 0; i < hwchan; i++) {
 		u32 buf;
 		int ret;
 
-		ret = snd_audient_raw_get_one(mixer, ctype, base + i, &buf);
-		if (ret < 0)
+		ret = snd_audient_raw_get_one(mixer, ctype,
+					      snd_audient_cache_slot(elem, i),
+					      base + i, &buf);
+		if (ret == -ENODATA)	/* unknown write-only value: off */
+			buf = ctype->min.integer;
+		else if (ret < 0)
 			return ret;
 
 		ctl_val->value.integer.value[i] = (buf == ctype->min.integer)
@@ -375,8 +432,8 @@ static int snd_ctl_audient_boolean_get(struct snd_kcontrol *kctl,
 static int snd_ctl_audient_boolean_set(struct snd_kcontrol *kctl,
 				       struct snd_ctl_elem_value *ctl_val)
 {
-	struct usb_mixer_elem_list *list = snd_kcontrol_chip(kctl);
-	struct usb_mixer_interface *mixer = list->mixer;
+	struct usb_mixer_elem_info *const elem = snd_kcontrol_chip(kctl);
+	struct usb_mixer_interface *const mixer = elem->head.mixer;
 
 	const unsigned int hwchan = snd_audient_kctl_hwchannels(kctl);
 	const struct snd_audient_ctrl_type *const ctype
@@ -389,13 +446,17 @@ static int snd_ctl_audient_boolean_set(struct snd_kcontrol *kctl,
 	CLASS(snd_usb_lock, pm)(mixer->chip);
 	if (pm.err < 0)
 		return pm.err;
+	/* serializes the transfers and the cache, also between gets */
+	guard(mutex)(&mixer->lock);
 
 	for (i = 0; i < hwchan; i++) {
 		u32 buf = ctl_val->value.integer.value[i] ? ctype->max.integer
 							  : ctype->min.integer;
 		int ret;
 
-		ret = snd_audient_raw_set_one(mixer, ctype, base + i, buf);
+		ret = snd_audient_raw_set_one(mixer, ctype,
+					      snd_audient_cache_slot(elem, i),
+					      base + i, buf, false);
 		if (ret < 0)
 			return ret;
 
@@ -409,8 +470,8 @@ static int snd_ctl_audient_boolean_set(struct snd_kcontrol *kctl,
 static int snd_ctl_audient_integer_get(struct snd_kcontrol *kctl,
 				   struct snd_ctl_elem_value *ctl_val)
 {
-	struct usb_mixer_elem_list *const list = snd_kcontrol_chip(kctl);
-	struct usb_mixer_interface *const mixer = list->mixer;
+	struct usb_mixer_elem_info *const elem = snd_kcontrol_chip(kctl);
+	struct usb_mixer_interface *const mixer = elem->head.mixer;
 
 	const unsigned int nchan = snd_audient_kctl_channels(kctl);
 	const unsigned int hwchan = snd_audient_kctl_hwchannels(kctl);
@@ -423,13 +484,19 @@ static int snd_ctl_audient_integer_get(struct snd_kcontrol *kctl,
 	CLASS(snd_usb_lock, pm)(mixer->chip);
 	if (pm.err < 0)
 		return pm.err;
+	/* serializes the transfers and the cache, also between gets */
+	guard(mutex)(&mixer->lock);
 
 	for (i = 0; i < hwchan; i++) {
 		s32 val;
 		int ret;
 
-		ret = snd_audient_raw_get_one(mixer, ctype, base + i, &val);
-		if (ret < 0)
+		ret = snd_audient_raw_get_one(mixer, ctype,
+					      snd_audient_cache_slot(elem, i),
+					      base + i, &val);
+		if (ret == -ENODATA)	/* unknown write-only value: minimum */
+			val = ctype->min.integer;
+		else if (ret < 0)
 			return ret;
 
 		if (val < ctype->min.integer)
@@ -450,8 +517,8 @@ static int snd_ctl_audient_integer_get(struct snd_kcontrol *kctl,
 static int snd_ctl_audient_integer_set(struct snd_kcontrol *kctl,
 				   struct snd_ctl_elem_value *ctl_val)
 {
-	struct usb_mixer_elem_list *list = snd_kcontrol_chip(kctl);
-	struct usb_mixer_interface *mixer = list->mixer;
+	struct usb_mixer_elem_info *const elem = snd_kcontrol_chip(kctl);
+	struct usb_mixer_interface *mixer = elem->head.mixer;
 
 	const unsigned int hwchan = snd_audient_kctl_hwchannels(kctl);
 	const struct snd_audient_ctrl_type *const ctype
@@ -471,12 +538,16 @@ static int snd_ctl_audient_integer_set(struct snd_kcontrol *kctl,
 	CLASS(snd_usb_lock, pm)(mixer->chip);
 	if (pm.err < 0)
 		return pm.err;
+	/* serializes the transfers and the cache, also between gets */
+	guard(mutex)(&mixer->lock);
 
 	for (i = 0; i < hwchan; i++) {
 		s32 val = ctl_val->value.integer.value[i];
 		int ret;
 
-		ret = snd_audient_raw_set_one(mixer, ctype, base + i, val);
+		ret = snd_audient_raw_set_one(mixer, ctype,
+					      snd_audient_cache_slot(elem, i),
+					      base + i, val, false);
 		if (ret < 0)
 			return ret;
 
@@ -517,6 +588,8 @@ static int snd_audient_add_ctrl(struct usb_mixer_interface *mixer,
 	struct snd_kcontrol *kctl;
 	struct usb_mixer_elem_info *elem;
 
+	int ret;
+
 	knew.iface = SNDRV_CTL_ELEM_IFACE_MIXER;
 	knew.name = ctrl->name;
 	knew.info = snd_ctl_audient_info;
@@ -524,7 +597,9 @@ static int snd_audient_add_ctrl(struct usb_mixer_interface *mixer,
 						   ctrl->hwchannels);
 	knew.tlv.p = ctype->tlv;
 	knew.access = SNDRV_CTL_ELEM_ACCESS_READWRITE
-			| SNDRV_CTL_ELEM_ACCESS_VOLATILE
+			| ((ctype->flags & SND_AUDIENT_CTYPE_FLAGS_CACHE)
+			   ? 0
+			   : SNDRV_CTL_ELEM_ACCESS_VOLATILE)
 			| (ctype->tlv ? SNDRV_CTL_ELEM_ACCESS_TLV_READ : 0);
 
 	switch (ctype->type) {
@@ -540,36 +615,75 @@ static int snd_audient_add_ctrl(struct usb_mixer_interface *mixer,
 		return -EINVAL;
 	}
 
+	elem = kzalloc(sizeof(*elem)
+		       + ((ctype->flags & SND_AUDIENT_CTYPE_FLAGS_CACHE)
+			  ? (sizeof(struct snd_audient_cache_entry)
+			     * ctrl->hwchannels)
+			  : 0),
+		       GFP_KERNEL);
+	if (!elem)
+		return -ENOMEM;
+
+	elem->head.mixer = mixer;
+	elem->channels = ctrl->channels;
+
+	if (ctype->flags & SND_AUDIENT_CTYPE_FLAGS_CACHE)
+		elem->private_data = elem + 1;
+
+	/*
+	 * A failed probe transfer must not fail the card: warn and keep the
+	 * control. Its cache entry stays invalid, so a readable control reads
+	 * the device again on its next get, and a write-only control shows an
+	 * unknown value until a put succeeds.
+	 */
 	if (ctype->flags & SND_AUDIENT_CTYPE_FLAGS_INIT_HW) {
 		int i;
 
 		for (i = 0; i < ctrl->hwchannels; i++) {
-			int ret = snd_audient_raw_set_one(mixer, ctype,
-							  ctrl->val_offs + i,
-							  ctype->init);
+			ret = snd_audient_raw_set_one(mixer, ctype,
+						      snd_audient_cache_slot(elem, i),
+						      ctrl->val_offs + i,
+						      ctype->init, true);
 			if (ret < 0)
 				usb_audio_warn(mixer->chip,
 					       "%s: cannot set channel %d (%d)\n",
 					       ctrl->name, i, ret);
 		}
-	}
-
-	elem = kzalloc_obj(*elem);
-	if (!elem)
-		return -ENOMEM;
+	} else if (elem->private_data) {
+		struct snd_audient_cache_entry *const cache = elem->private_data;
+		int i;
 
-	elem->head.mixer = mixer;
-	elem->channels = ctrl->channels;
+		for (i = 0; i < ctrl->hwchannels; i++) {
+			if (ctype->flags & SND_AUDIENT_CTYPE_FLAGS_FORCE_SET) {
+				cache[i].value = ctype->init;
+				cache[i].valid = true;
+				continue;
+			}
+
+			ret = snd_audient_raw_get_one(mixer, ctype, cache + i,
+						      ctrl->val_offs + i,
+						      &cache[i].value);
+			if (ret < 0)
+				usb_audio_warn(mixer->chip,
+					       "%s: cannot read channel %d (%d)\n",
+					       ctrl->name, i, ret);
+		}
+	}
 
 	kctl = snd_ctl_new1(&knew, elem);
 	if (!kctl) {
-		kfree(elem);
-		return -ENOMEM;
+		ret = -ENOMEM;
+		goto free_elem;
 	}
 
 	kctl->private_free = snd_usb_mixer_elem_free;
 
 	return snd_usb_mixer_add_control(&elem->head, kctl);
+
+free_elem:
+	kfree(elem);
+
+	return ret;
 }
 
 
-- 
2.56.0