[RFC 2/2] ASoC: rt5670: Add LED trigger support

[Jaroslav Kysela]

Dne 26. 02. 21 v 9:41 Takashi Iwai napsal(a):
> On Thu, 25 Feb 2021 19:09:44 +0100,
> Jaroslav Kysela wrote:
>>
>> Dne 25. 02. 21 v 12:00 Takashi Iwai napsal(a):
>>> On Wed, 24 Feb 2021 18:57:42 +0100,
>>> Jaroslav Kysela wrote:
>>>>
>>>> Dne 24. 02. 21 v 13:42 Takashi Iwai napsal(a):
>>>>> On Wed, 24 Feb 2021 13:08:55 +0100,
>>>>> Jaroslav Kysela wrote:
>>>>>>
>>>>>> Dne 24. 02. 21 v 12:43 Takashi Iwai napsal(a):
>>>>>>
>>>>>>>>> So far, a user control is merely storing the value, let read/write via
>>>>>>>>> the control API.  That's all, and nothing wrong can happen just by
>>>>>>>>> that.  Now if it interacts with other subsystem...
>>>>>>>>>
>>>>>>>>> A more serious concern is rather the fragility of the setup; for
>>>>>>>>> enabling the mute LED control, you'd have to create a new user-space
>>>>>>>>> control, the function of the control has to be ignored by some
>>>>>>>>> application and some not, etc.  This has to be done on each machine
>>>>>>>>
>>>>>>>> You're using "ignore", but as I explained before, the user space switch will
>>>>>>>> be used in the whole chain:
>>>>>>>>
>>>>>>>> capture stream ->
>>>>>>>>   alsa-lib mute switch / silence PCM stream ->
>>>>>>>>   PA mute switch / silence PCM stream
>>>>>>>>
>>>>>>>> So PA can use this switch like the traditional hardware mute switch.
>>>>>>>
>>>>>>> Does it mean PA would work as of now without any change?  Or does it
>>>>>>> need patching?
>>>>>>
>>>>>> Yes, no PA modifications are required with my mechanism. The PA will just see
>>>>>> the new user space control - mute switch - created in alsa-lib - which will be
>>>>>> synced the internal PA path mute state like for the hardware mute
>>>>>> switch.
>>>>>
>>>>> OK, but how would we create and manage the user control element?  And
>>>>> why it has to be user control?
>>>>
>>>> The softvol or alsactl can create the user control element. The alsa-lib
>>>> softvol plugin and PA can silence stream according the state.
>>>
>>> And that's tricky if it's only with PA, as PA won't open a softvol PCM
>>> stream...
>>
>> The protection is in alsa-lib, so we can skip to check this hint flag for this
>> particular case like:
>>
>> https://github.com/alsa-project/alsa-lib/pull/121/commits/1acc1c7eccab0359996b25de54a6b6e0aa1e0c17
>>
>> So it may depend on the softvol config not PA itself.
> 
> Thanks, that's what I missed from the big picture.
> 
>> Even for the solution bellow, we need to modify softvol to handle the kernel
>> control elements, too. Actually softvol is not active, when the specified
>> control element is found and this element is not from the user space.
> 
> Hm, that would certainly work, but as we discussed before, it enforces
> the softvol PCM process for PA.  That isn't too bad for the capture,
> fortunately, but not ideal, OTOH.

PA can work without softvol in this case, but in my opinion, it may be better
to force the silence in UCM (do not depend only on the implementation of the
UCM application - like PA - more security).

As I wrote before, the silencing can be done in alsa-lib AND in PA
(simultaneously). If we use only mute switch control configuration (on/off -
not volume) for sofvol, then the code in alsa-lib is really effective without
any performance penalty.

> And, I'm not sure how PA can take any control as its main capture mute
> switch, if it's named differently.  Wouldn't we need to change mixer
> path in PA as well?  And, if we may change PA side, it sounds more
> natural to change a control directly in PA's mixer path.  The softvol
> doesn't fit well with PA, after all.

The ACP microphone is handled via UCM, so we can add the new control switch
identifier to UCM. No PA modifications should be required. And the LED state
can be handled via the phantom control (AMD ACP) + legacy HDA capture switch.
So two controls from two different sound cards may be marked with the LED
microphone group, if it's the correct way for users.

					Jaroslav

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