[PATCH v4 6/6] ALSA: led control - add sysfs kcontrol LED marking layer

[Jaroslav Kysela]

Dne 23. 03. 21 v 10:49 Takashi Iwai napsal(a):
> On Tue, 23 Mar 2021 10:38:46 +0100,
> Takashi Iwai wrote:
>>
>> On Mon, 22 Mar 2021 15:16:30 +0100,
>> Jaroslav Kysela wrote:
>>>
>>> Dne 20. 03. 21 v 10:48 Takashi Iwai napsal(a):
>>>
>>>>> With other OS you mean e.g. Android?  Android has device-specific
>>>>> init-scripts which can either call alsactl or directly do the
>>>>> echo-s.
>>>>
>>>> Also ChromeOS.  I'd like to get a general consensus before moving
>>>> forward.
>>>
>>> Where are ChromeOS people? They could join to the discussion which is floating
>>> few months now. Perhaps, the gmail's spam filter does not allow them to
>>> communicate with us ;-)
>>
>> Also adding Dylan and Mark to Cc.
>>
>> FYI, the patch set is:
>>   https://lore.kernel.org/alsa-devel/20210317172945.842280-1-perex@perex.cz/
> 
> ... and now back to the topic.
> 
> So the primary question is whether we want the sysfs entries to allow
> user-space defining the mute-LED vs control binding externally.  With
> this, the mute LED is supposed to be set up via udev rules that
> triggers some alsactl stuff, and the rest is handled in an extension
> in UCM profile.  If this approach is acceptable on all platforms, we
> can go for it.  That was the question to other platforms like Android
> and ChromeOS.
> 
> 
> And, now looking into the details, I have a few more questions:
> 
> - The binding with SNDRV_CTL_ELEM_* bit flag is handy for some drivers
>   but not for everything; e.g. if we want to add the binding in ASoC
>   machine driver, an API like
>     snd_ctl_bind_mute_led(card, elem_id, inverted);
>   would be easier.  It'd be essentially an internal call of the sysfs
>   binding.

I would probably create more universal helper for the access field. It may be
handy to update other flags like INACTIVE or so. Something like:

  snd_ctl_update_access(card, elem_id, access_mask, access_bits);

If we decide to move this information out of access field, we can replace
those calls with another function.

For ASoC codecs, it may be difficult to do such calls in the init phase,
because the card is not bound to the component. But yes, I agree that this
setting should be handled in the upper layer (machine) than the component layer.

>  (I haven't checked, but might this be also more
>   straightforward conversion for HD-audio case, too?)

I don't think that it brings a simplification. The id composition is more
complex than 'if (codec->led_flag) access |= LED_GROUP'.

> - The binding in the kernel could (should?) be shown in the sysfs
>   output.  Currently it seems handled differently?

It isn't. The LED group is stored in the access field and my implementation
tracks those bits per elements. So, the sysfs LED code updates those bits,
too. The settings is preserved even if you reload the ctl-led module.

> - Specifying the numid may the code simpler in kernel side?
>   alsactl has already the string parser.

Yes, but it's not so handy for scripting / UCM. I can add find-ctl-numid
lookup to UCM, of course, but what about standard shell scripting?

> - Do we have to deal with binding with multiple controls to a single
>   mute LED?  Might a single exclusive binding make things easier?
>   Then we don't have to create sysfs entries per card, and it'll be
>   something like
>      echo 1:10 > /sys/devices/virtual/sound/ctl-led/mic/bind
>   which is equivalent with the API call above.
>   If multiple bindings are attempted, it can simply give an error.
>   In the driver side, it catches the unexpected binding, too.

AMD ACP digital + HDA analog headset microphone. If we follow the standard HDA
behaviour, both inputs should trigger the mic LED. Two cards are in the game.

					Jaroslav

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