Re: [PATCH 0/5] ALSA: control - add generic LED trigger code

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, 14 Feb 2021 19:55:21 +0100,
Jaroslav Kysela wrote:
> 
> Dne 12. 02. 21 v 13:28 Takashi Iwai napsal(a):
> > On Fri, 12 Feb 2021 11:32:38 +0100,
> > Jaroslav Kysela wrote:
> >>
> >> Dne 12. 02. 21 v 10:23 Takashi Iwai napsal(a):
> >>> On Thu, 11 Feb 2021 18:53:20 +0100,
> >>> Jaroslav Kysela wrote:
> >>>>
> >>>> Dne 11. 02. 21 v 18:15 Takashi Iwai napsal(a):
> >>>>
> >>>>>> Jaroslav Kysela (5):
> >>>>>>   ALSA: control - introduce snd_ctl_notify_one() helper
> >>>>>>   ALSA: control - add layer registration routines
> >>>>>>   ALSA: control - add generic LED trigger module as the new control
> >>>>>>     layer
> >>>>>>   ALSA: HDA - remove the custom implementation for the audio LED trigger
> >>>>>>   ALSA: control - add sysfs support to the LED trigger module
> >>>>
> >>>>> One thing I still miss from the picture is how to deal with the case
> >>>>> like AMD ACP.  It has no mixer control to bundle with the LED trigger.
> >>>>> Your idea is to make a (dummy) user element and tie the LED trigger
> >>>>> with it?
> >>>>
> >>>> Yes, the user-space code which guarantee the silence stream should create an
> >>>> user space control with the appropriate LED group access bits. The alsa-lib's
> >>>> softvol PCM plugin can do this silencing for example.
> >>>
> >>> What control would it create?  In the case of softvol, it's a volume
> >>> control that really changes the volume.  For the mute LED, it's a
> >>> control turn on/off the mute?  If so, I wonder what makes better than
> >>> creating it from the kernel driver.  (Of course, we can list up like
> >>> "flexibility", etc, but it has a flip side of "complexity" and
> >>> "fragility"...)
> >>
> >> The current code handles both switch / volume for the marked control (assuming
> >> that the minimal value - usually zero - is full mute). And actually, there are
> >> snd_pcm_areas_silence() calls in the softvol plugin, so we know that the PCM
> >> stream is not passed to the application at this point.
> >>
> >> Condition:
> >>
> >>       if (info.type == SNDRV_CTL_ELEM_TYPE_BOOLEAN ||
> >>           info.type == SNDRV_CTL_ELEM_TYPE_INTEGER)
> >>           ... value.value.integer.value[i] != info.value.integer.min
> >>
> >> The softvol plugin may be extended to add the mute switch control, of course.
> > 
> > Well, my question was what kind of mixer control will be added at all,
> > although the chip does neither volume nor mute function.  Would we add
> > a fake volume/switch like softvol, or would we add rather a control
> > that is directly tied with the LED state?
> 
> I don't understand your question. If the user space marks the own vol/sw
> control with the LED group, then it's tied with the LED state. I believe that
> the control should be created in the code which make sure that the PCM stream
> is silenced (like alsa-lib's softvol plugin).

The softvol (or similar effect) is to be ignored by PA, as it's not
suitable with the timer-scheduled type of PCM streaming.  So the
control shouldn't have any actual effect of PCM stream itself but
merely for controlling the LED state.  If that's the case, it
shouldn't be named like "XXX Switch" or "XXX Volume", but it's a
control like "Mic Mute LED Status" -- and ironically, that's a kind of
thing we didn't want to take in the kernel side implementation...

That said, I see the value of a generic code for the LED control that
is tied with the existing volume/switch; there is already such an
implementation in HD-audio, and applying to the other drivers makes
sense.  OTOH, for the case for AMD ACP, I'm still not sure what's the
best way.


thanks,

Takashi




Takashi

> 
> 						Jaroslav
> 
> -- 
> Jaroslav Kysela <perex@xxxxxxxx>
> Linux Sound Maintainer; ALSA Project; Red Hat, Inc.
> 



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux