RE: [PATCH] [RFC] ALSA: control - add generic LED API to sound core routines

["Yuan, Perry" <Perry.Yuan@dell.com>]

Hi Jaroslav
> -----Original Message-----
> From: Jaroslav Kysela <perex@perex.cz>
> Sent: 2021年2月8日 19:42
> To: Hans de Goede; ALSA development
> Cc: Takashi Iwai; Yuan, Perry
> Subject: Re: [PATCH] [RFC] ALSA: control - add generic LED API to sound core
> routines
> 
> 
> [EXTERNAL EMAIL]
> 
> Dne 07. 02. 21 v 21:52 Hans de Goede napsal(a):
> > Hi Jaroslav,
> >
> > On 2/7/21 9:11 PM, Jaroslav Kysela wrote:
> >> [DO NOT MERGE!]
> >> [This is just an early proposal for comments] [The code is not tested
> >> / debugged]
> >>
> >> The recent laptops have usually two LEDs assigned to reflect the
> >> speaker and microphone mute state. This implementation adds a tiny
> >> layer on top of the control API which calculates the state for those
> >> LEDs using the driver callbacks.
> >>
> >> Two new access flags are introduced to describe the controls which
> >> affects the audio path settings (an easy path for drivers).
> >>
> >> The LED resource can be shared with multiple sound cards with this
> >> code. The user space controls may be added to the state chain, too.
> >>
> >> This code should replace the LED code in the HDA driver and add a
> >> possibility to easy extend the other drivers (ASoC codecs etc.).
> >>
> >> Note: The snd_ctl_notify_one() changes should be moved to a separate
> >> patch. I will do this, when this proposal is agreed.
> >>
> >> Signed-off-by: Jaroslav Kysela <perex@perex.cz>
> >> Cc: Perry Yuan <Perry.Yuan@dell.com>
> >> Cc: Hans de Goede <hdegoede@redhat.com>
> >
> > Nice I think that having this handled in a generic manner would be a
> > great improvement.
> >
> > I have 2 HP x2 models which are Bay Trail resp. Cherry Trail based,
> > which means they use Intel's LPE (Low Power Engine) for audio which
> > uses ASoC.
> >
> > These come with detachable USB keyboard-docks which have a speaker
> > mute LED. I have already added LED-class support to the HID driver for
> > these, but I never got around to hooking up a trigger from the ASoC
> > code.
> >
> > So if I understand things correctly, then if I add this patch to a
> > kernel and in the ASoC codec driver add
> SNDRV_CTL_ELEM_ACCESS_SPK_LED
> > to one or more of the controls, and add .default_trigger =
> > "audio-mute" to the led_class_dev for the LED, then if I toggle the
> > control on / off in alsamixer this should then control the LED ?
> 
> Yes, the ALSA control code should do just the LED trigger.
> The LED class driver is a separate thing.
> 
> > If I got that right I will certainly give this a try.
> >
> > Any advice for which control would be the best to use?
> > Looking at the code, it assumes that if a control is enabled that then
> > there will be a route, which makes sense if there are e.g. headphones
> > and speaker and lineout controls. But what about chained controls,
> > e.g. separate volume + mute controls or multiple volume controls
> > chained. With ASoC this is not unheard of. I guess the answer is to
> > pick the control which we will also want to use for hw volume-control
> > if/when UCM profiles grow hw volume-control support ?
> 
> The proposed implementation just goes through all marked controls and if
> any of the marked controls is set to value greater than the minimal value, then
> the path is evaluated as "on" (thus the mute LED should be set to off).
> 
> The controls tied to the PCM stream (DAC / DMA) should be marked. The
> physical inputs behind a multiplexer without a "global" volume / switch
> controls will require a bit different handling. I would just start with something
> simple and we can add the more complex things on demand.
> 
> >
> > Regards,
> >
> > Hans
> >
> >
> >
> >
> > p.s. This would only / at least add proper support for these LEDs at
> > the kernel level. ATM these devices which use UCM profiles don't
> > actually use any kind of hw volume-control. So we would also need to
> > fix that (in userspace) to get these LEDs to really work when an user
> > hits the mute key, or lowers the volume to NIL.
> 
> There's already the hw volume support in UCM and in PA (I added this to PA
> 14.0). Some drivers like SOF HDA / SoundWire already set the
> PlaybackMixerElem / CaptureMixerElem values. It's true that the hw volume
> controls are missing for the Intel SST hardware.
> 
> The similar situation is for the SoundWire codecs (missing LED controls,
> missing hw volume configuration in UCM).
> 
> 					Jaroslav
> 
> --
> Jaroslav Kysela <perex@perex.cz>
> Linux Sound Maintainer; ALSA Project; Red Hat, Inc.

Will this new audio led control implementation be supporting SOF/Soundwire subsystem ?
If so, I can help to do some testing rebase  my privacy driver which needs the audio led control.

Perry