alsa-devel.alsa-project.org archive mirror
 help / color / mirror / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: Jaroslav Kysela <perex@perex.cz>
Cc: Hans de Goede <hdegoede@redhat.com>,
	ALSA development <alsa-devel@alsa-project.org>,
	Perry Yuan <Perry.Yuan@dell.com>
Subject: Re: [PATCH 0/5] ALSA: control - add generic LED trigger code
Date: Fri, 12 Feb 2021 10:23:52 +0100	[thread overview]
Message-ID: <s5hblcpej13.wl-tiwai@suse.de> (raw)
In-Reply-To: <3c84c275-0c62-d2f4-38ad-be6accb3b159@perex.cz>

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"...)

> > Another slight concern is the possible regression: by moving the
> > mute-LED mode enum stuff into the sysfs, user will get
> > incompatibilities after the kernel update.  And it's not that trivial
> > to change the sysfs entry as default for each user.
> > It needs some detailed documentation or some temporary workaround
> > (e.g. keep providing the controls for now but warns if the value is
> > changed from the default value via the controls).
> 
> I don't think that we have a user space application which is using those
> controls (Pulseaudio or so..) in an abstract way. I think that it's really
> minor issue. We should probably concentrate for the main designed purpose
> (notify about the mute / silent state) and handle those add-on features as an
> experimental stuff.

I'm sure that there are users of the reverse mic-mute LED ("follow
capture" mode); the feature was added because of the explicit request
from my colleague, and this mode works no matter whether ALSA native
or PA is used.  Not sure about "on" and "off" mode; maybe there can be
some users who want to disable the LED.

But, yes, this is a minor issue and should be in a lower priority.
It's just as a reminder.


thanks,

Takashi

  reply	other threads:[~2021-02-12  9:25 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-11 11:13 [PATCH 0/5] ALSA: control - add generic LED trigger code Jaroslav Kysela
2021-02-11 11:13 ` [PATCH 1/5] ALSA: control - introduce snd_ctl_notify_one() helper Jaroslav Kysela
2021-02-11 11:13 ` [PATCH 2/5] ALSA: control - add layer registration routines Jaroslav Kysela
2021-02-11 11:13 ` [PATCH 3/5] ALSA: control - add generic LED trigger module as the new control layer Jaroslav Kysela
2021-02-11 11:13 ` [PATCH 4/5] ALSA: HDA - remove the custom implementation for the audio LED trigger Jaroslav Kysela
2021-02-11 12:19   ` kernel test robot
2021-02-11 13:03     ` Jaroslav Kysela
2021-02-22  7:28   ` [ALSA] a7fc56df5a: WARNING:possible_circular_locking_dependency_detected kernel test robot
2021-02-11 11:14 ` [PATCH 5/5] ALSA: control - add sysfs support to the LED trigger module Jaroslav Kysela
2021-02-11 12:43   ` kernel test robot
2021-02-11 17:15 ` [PATCH 0/5] ALSA: control - add generic LED trigger code Takashi Iwai
2021-02-11 17:53   ` Jaroslav Kysela
2021-02-12  9:23     ` Takashi Iwai [this message]
2021-02-12 10:32       ` Jaroslav Kysela
2021-02-12 12:28         ` Takashi Iwai
2021-02-14 18:55           ` Jaroslav Kysela
2021-02-15  7:50             ` Takashi Iwai
2021-02-15 17:49               ` Jaroslav Kysela
2021-02-12 20:31 ` Hans de Goede
2021-02-15 12:02   ` Hans de Goede
2021-02-15 12:35     ` Jaroslav Kysela

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=s5hblcpej13.wl-tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=Perry.Yuan@dell.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=hdegoede@redhat.com \
    --cc=perex@perex.cz \
    /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;
as well as URLs for NNTP newsgroup(s).