From: Christian Ruppert <arc@gmx.li>
To: Will Porter <mrwillporter@gmail.com>
Cc: Takashi Iwai <tiwai@suse.com>, Jaroslav Kysela <perex@perex.cz>,
linux-sound@vger.kernel.org, linux-kernel@vger.kernel.org,
Neil Andrews <neil@androos.io>
Subject: Re: [PATCH 0/5] ALSA: usb-audio: Audient iD14 MkI vendor controls
Date: Wed, 28 Oct 2026 00:21:51 +0100 [thread overview]
Message-ID: <auEyD23PT_kpJsrJ@khmer.arcnet> (raw)
In-Reply-To: <CAMZ8TExJThTMZVvbDMVdtXSB-edGAsAnmKYtAtM1CCw0pDvNGw@mail.gmail.com>
Hi Will,
On Mon, Oct 26, 2026 at 04:44:50PM -0500, Will Porter wrote:
> On Mon, Oct 26, 2026 at 07:36:24PM +0100, Christian Ruppert wrote:
> > I took a look at this question and it seems like we can share the majority
> > of code in the EVO driver by complementing it with the following features:
> > - variable width read/write
> > - caching of write-only values
> > - enum data type
>
> Thanks for looking into this so closely, and for the fixes. I like
> the idea of one driver, and I'd like to find out whether the iD line fits.
>
> Your TLV in patch 4/4 is the same one I use for the iD14 matrix: same
> unit 0x3C, s16 in 1/256 dB, -128 to +6 dB. That made me wonder whether
> the iD14 matrix could use your MIXER type almost as it is.
Agreed. If my understanding is correct, this type could be reused without
any modifications. The others would have to be newly defined due to
different unit IDs/control selectors. BTW: the mixer matrix seems to be
identical also for the EVO8 so chances are it's the same for most audient
devices and we could simply use a width and height parameter to generate
it in a loop.
> Its channel
> number is input * 4 + output, so Main L/R and Cue L/R of each input
> are adjacent, like your stereo pairs. Could we expose it as 28
> controls with two values each, rather than my 56 single-value ones?
> An input's level and pan in a mix set its L and R values together,
> so a pair seems a natural fit. Does that look right to you?
This is exactly what the stereo pairs do for the EVO. For example, mic 1
shows up as one stereo pair which can be either recorded on both channels
(paned between L and R) or exclusively on the left one if it's connected
to a stereo mic. Same for mic 2 (which can be used as the right channel
of a stereo mic or an independently paned mic). The same principle applies
for the L and R outputs of the monitor and USB main out where it seems a
bit more quirky but is coherent with the Mics. That said, the existing
table system would also allow for individual channels or grouping them in
"mini-matrices of four" but neither seemed right when looking at them in
alsamixer and so I stuck with one stereo mixer for each (mono) input
channel, similar to most mixing consoles.
> > The only part which seems more problematic is probably FU12 which looks
> > like it requires some custom logic and would still need actual code.
>
> Agreed. I'd keep FU 12 as its own code.
>
> A few questions about the rest:
>
> 1. Read before write. As I understand it, the put handlers in
> mixer_evo.c read the value first to see if it changed. On the iD14,
> GET stalls for mic boost, input polarity, optical mode and the iD
> key function, so a put that reads first would fail there. Talkback
> source always reads 0, so a read would hide real changes. Could
> these write-only controls skip GET in both get and put, with get
> returning the cached value? And could every put still send the
> value, even when it matches the cache, so that "alsactl restore"
> reaches a device that was reset?
My view of the WO implementation would be to add three flags: cache, init
and force_write:
- If cache is set, the values will be cached and the channel not marked as
volatile. No need to read back actual values from USB before a write in
this case. This could also be used in the EVO for the mixer matrix which
doesn't have physical controls that can change values while the driver
doesn't look. Didn't implement it to not complicate the driver with
unnecessary caching but if we need caching anyway we should use it.
- If init is set, the driver will initialize the control at probe. The EVO
mixer matrix' initial read returns a wrong value so initialization would
be great. Also not implemented today because there's no need to complicate
the driver for a control which must be fiddled with anyway before it is
useful but if we have the feature we should use it.
- If force_write is set, the driver will always write the value, no matter
if a cached (or previously read) value is identical. IMHO this is mainly
required for broken or quirky controls but I'm a bit undecided if it
would really hurt to simply make this behaviour the default for
everything.
> 2. The cache. My driver writes a known value at probe (off or none,
> and for optical mode its measured power-on mode, S/PDIF) and
> updates the cache only after a full-length write. If the probe
> write fails, an enum reads "Unknown" and a switch reads off. Would
> that approach work for you?
This is very much in line with what I was thinking, see above.
> 3. Control entries. The iD14 table uses 16 unit/selector pairs and
> explicit control names, such as "Main Out L Source Playback Enum".
> It also needs volumes in 1 dB steps (0..128, stored as s16
> 1/256 dB), a boost switch that writes 0x0a00 for on, and enums
> that map items to device values, plus an "Other" item that only
> get reports, for values outside the list. Would it make sense to
> move the unit, selector and an optional full name into each control
> entry, and add the new value formats (1 dB volumes, boost, enum)
> as ctypes? Or would you structure it differently?
This is several questions in one, let me try not to forget an answer ;)
- The intent of the control types was to associate unit/selector pairs
with data types. I would add a new ctype for each unit/selector pair
but am open to move unit/selector to the controls and only leave the
data type definition in ctype (which will then become rather a dtype)
if it significantly reduces repetition. I'm afraid it might just shift
repetition to the controls table, though. Btw: I like the unit/selector
macros, will introduce this also for the EVO.
- I don't have a preference between explicit control names and the
basename/suffix system the EVO driver currently uses. If you prefer we
can simply replace the basename with a full name and remove the flags
field in the controls.
- The boost switch writing 0x0 for 0dB gain and 0x0a00 for 10dB gain looks
suspiciously like a volume control between 0dB and 10dB * 256steps/dB.
Does this control only accept these two values or can we simply
implement it as a standard volume type controlling a mic pre-amp? If
it's only 0xa00 or 0x0 I would suggest to slightly redefine the boolean
type to use the max value for true and the min value for false which
would also be an elegant way to remove the inversion flag.
- We definitely need a new snd_evo_type entry and get/set functions for
enums, up to you to define those in the most suitable manner for the
id14.
> 4. Checks. My helper returns -EIO on a short read or write, before
> it decodes a value or updates the cache. My matrix put also rejects
> values outside -128..+6 dB, and get clamps what it reads back.
> Could the shared code keep these checks?
Sure, error management is always good to have. I never encountered any
short reads/writes or out of range reads with the EVO so I didn't add any
logic for this until now. Also, for the controls I checked, the EVO
cleanly rejects out-of-range writes but if the id14 doesn't we should add
those checks as well.
> I don't have a firm view yet on which way is better. If the iD14
> fits the table without many special cases, one driver seems clearly
> worth it. If it needs a lot of per-device branches, two smaller files
> might be easier to read and review. What do you think, now that you
> have seen what the iD14 needs?
I think we can put everything except for FU12 into the tables and the EVO
driver would even benefit from the added features (see above).
> If we do combine them, the order I imagined is: your four fixes first,
> then the rename to Audient, then the generic changes, then the iD14
> tables on top as v2 of my series.
Agreed. Renaming the file first might turn out to be annoying because it's
the one thing which might create rebase conflicts we'll have to deal with
while we work on this but it's probably preferable to keep as much of the
commit history as possible after the rename.
> In that case, would you prefer to do the rename and the generic part
> yourself?
I suppose we can cut one testing loop if I do the part of the generic part
that touches the EVO myself so I can do that, no problem.
> The rename would also touch MAINTAINERS; how would you like
> to handle review of the iD14 parts?
I would definitely prefer to list both of us as maintainers. Practically
all code will be shared and I can't speak for the id14 in any way.
> I can write the iD14 parts and
> test them on my MkI. I don't have an EVO 4, so would you be able to
> test that side? And which base should I work against, sound for-next
> once your fixes are in?
It shouldn't make a huge difference because we're rather independent on
this file. I'll probably prepare everything on 7.3 and then rebase it onto
sound/master to test it there and submit (that's what I did until now, but
for-next is also fine). I suggest I submit an RFC with the generic part
that you can apply locally to work against (and potentially adjust) before
you submit everything in a final series which, if everything goes well, I
only need to retest and tag.
> I'm also not sure how a combined change
> should reach Takashi: as one series, or as two with a stated
> dependency.
next prev parent reply other threads:[~2026-10-27 23:22 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-19 23:01 Will Porter
2026-10-19 23:01 ` [PATCH 1/5] ALSA: usb-audio: Give the Audient iD14 MkI its own mixer map Will Porter
2026-10-19 23:01 ` [PATCH 2/5] ALSA: usb-audio: Add Audient iD14 MkI monitor and routing controls Will Porter
2026-10-19 23:01 ` [PATCH 3/5] ALSA: usb-audio: Add Audient iD14 MkI mixer matrix Will Porter
2026-10-19 23:01 ` [PATCH 4/5] ALSA: usb-audio: Add Audient iD14 MkI write-only controls Will Porter
2026-10-19 23:01 ` [PATCH 5/5] ALSA: usb-audio: Replace the Audient iD14 MkI FU 12 control Will Porter
2026-10-20 13:10 ` [PATCH 0/5] ALSA: usb-audio: Audient iD14 MkI vendor controls Takashi Iwai
2026-10-26 18:36 ` Christian Ruppert
2026-10-26 21:44 ` Will Porter
2026-10-27 6:35 ` Takashi Iwai
2026-10-27 23:21 ` Christian Ruppert [this message]
2026-10-28 2:36 ` Will Porter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=auEyD23PT_kpJsrJ@khmer.arcnet \
--to=arc@gmx.li \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-sound@vger.kernel.org \
--cc=mrwillporter@gmail.com \
--cc=neil@androos.io \
--cc=perex@perex.cz \
--cc=tiwai@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®