Message-ID: <34c0e515-bc69-0d67-eac5-581875179ce6@opensource.cirrus.com>
Date:   Mon, 4 Sep 2023 15:44:02 +0100
From:   Stefan Binding <sbinding@...nsource.cirrus.com>
To:     Takashi Iwai <tiwai@...e.de>
CC:     Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>,
        <alsa-devel@...a-project.org>, <linux-kernel@...r.kernel.org>,
        <patches@...nsource.cirrus.com>,
        Vitaly Rodionov <vitalyr@...nsource.cirrus.com>
Subject: Re: [PATCH v1] ALSA: hda: cs35l41: Support mute notifications for
 CS35L41 HDA


On 04/09/2023 15:16, Takashi Iwai wrote:
> On Mon, 04 Sep 2023 16:05:59 +0200,
> Stefan Binding wrote:
>>
>> On 04/09/2023 14:55, Takashi Iwai wrote:
>>> On Mon, 04 Sep 2023 15:47:49 +0200,
>>> Stefan Binding wrote:
>>>> On 04/09/2023 13:29, Takashi Iwai wrote:
>>>>> On Mon, 04 Sep 2023 14:00:20 +0200,
>>>>> Stefan Binding wrote:
>>>>>> On 29/08/2023 15:23, Takashi Iwai wrote:
>>>>>>> On Tue, 29 Aug 2023 16:18:12 +0200,
>>>>>>> Stefan Binding wrote:
>>>>>>>> On 25/08/2023 13:13, Takashi Iwai wrote:
>>>>>>>>> On Fri, 25 Aug 2023 14:05:25 +0200,
>>>>>>>>> Stefan Binding wrote:
>>>>>>>>>> From: Vitaly Rodionov <vitalyr@...nsource.cirrus.com>
>>>>>>>>>>
>>>>>>>>>> Some laptops require a hardware based mute system, where when a hotkey
>>>>>>>>>> is pressed, it forces the amp to be muted.
>>>>>>>>>>
>>>>>>>>>> For CS35L41, when the hotkey is pressed, an acpi notification is sent
>>>>>>>>>> to the CS35L41 Device Node. The driver needs to handle this notification
>>>>>>>>>> and call a _DSM function to retrieve the mute state.
>>>>>>>>>>
>>>>>>>>>> Since the amp is only muted during playback, the driver will only mute
>>>>>>>>>> or unmute if playback is occurring, otherwise it will save the mute
>>>>>>>>>> state for when playback starts.
>>>>>>>>>>
>>>>>>>>>> Only one handler can be registered for the acpi notification, but all
>>>>>>>>>> amps need to receive that notification, we can register a single handler
>>>>>>>>>> inside the Realtek HDA driver, so that it can then notify through the
>>>>>>>>>> component framework.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Vitaly Rodionov <vitalyr@...nsource.cirrus.com>
>>>>>>>>>> Signed-off-by: Stefan Binding <sbinding@...nsource.cirrus.com>
>>>>>>>>> We don't do normally in this way.  The ACPI hot key handling is done
>>>>>>>>> via user-space, and user-space daemon triggers the mute of the
>>>>>>>>> system.
>>>>>>>>>
>>>>>>>>> Can't the ACPI notify the key event on those machines?
>>>>>>>> This feature is not the "normal" mute button on a keyboard, it is a
>>>>>>>> custom request
>>>>>>>> from a manufacturer which only mutes the audio on the speakers.
>>>>>>>> On previous generations, this was achieved using a GPIO controlled by
>>>>>>>> the BIOS/EC.
>>>>>>>> However, since CS35L41 does not have such GPIO, we must control it by
>>>>>>>> other means.
>>>>>>>>
>>>>>>>> Our solution, which we have to share with the Windows driver, it to use ACPI
>>>>>>>> notifications to tell the driver to mute the amps when the shortcut is
>>>>>>>> pressed.
>>>>>>>>
>>>>>>>> Does this seem like a valid exception to the typical approach?
>>>>>>> It's still the question whether we have to do this inevitably in the
>>>>>>> kernel in a way like that.  It sounds quite unusual.  Why this must be
>>>>>>> handled directly?  IOW, what's the difference from the "normal" mute
>>>>>>> button?
>>>>>>>
>>>>>>> And, even if we take this approach, it leaves the device muted without
>>>>>>> exposing it to user-space.  Then user wouldn't know what happens.
>>>>>>>
>>>>>>>
>>>>>>> thanks,
>>>>>>>
>>>>>>> Takashi
>>>>>> We spoke to the ODM for this system to get a more detailed explanation
>>>>>> of this feature.
>>>>>> The keyboard shortcut enables something called "Unobtrusive
>>>>>> Mode". According to their explanation:
>>>>>>
>>>>>> - Unobtrusive mode is distinct to normal mute, as it only mutes the speakers
>>>>>> - There is no requirement to update the volume controls, as the screen
>>>>>> backlight will be off anyway in this mode
>>>>>> - All other unobtrusive mode functions are enabled without user-space
>>>>>> dependencies, and they would prefer not to make speaker mute an
>>>>>> exception
>>>>> Thanks, it gives a bit better clue.
>>>>> The remaining question is rather the exact behavior of this
>>>>> "unobtrusive mode".  How is it triggered, and what's the exact
>>>>> expectation?  e.g. It must secretly mute the speaker?  That is, it
>>>>> must not  expose the mixer state change to user-space?  Or is it tied
>>>>> with the normal mixer state and user may unmute again?
>>>>>
>>>>>
>>>>> Takashi
>>>>   From what we understand, unobtrusive mode, which is activated by a
>>>> keyboard shortcut (not a single key), performs several operations,
>>>> such as:
>>>> - muting the speaker (headphones remain unmuted)
>>>> - dimming/shutting down the LCD backlight
>>>> - turning off keyboard backlight and any keyboard LEDs
>>>> Apart from muting the speaker, all of these operations are done in
>>>> hardware, as the keyboard shortcut still works in the BIOS.
>>>> Previous laptops with this feature appear to use a GPIO to mute the
>>>> speaker, and we are informed that on those laptops userspace was not
>>>> informed of the mute.
>>>> Since CS35L41 does not have a GPIO mute, we had to use a different
>>>> solution, involving ACPI notifications, which request the driver to
>>>> mute.
>>>> The same mechanism is used in Windows.
>>>> Our understanding is that it is not intended for the mute to be
>>>> overridden by userspace.
>>>> Similarly, on previous laptops, userspace could not override this
>>>> mute, since it was not informed of it.
>>> OK, thanks for explanation.
>>>
>>> I still don't like the idea to hide this completely, though.  The mode
>>> should be somehow exposed even if the mute isn't controllable via
>>> mixer, but currently there is no indication at all.
>>>
>>>
>>> Takashi
>> We could create and expose a read-only ALSA control which would
>> display the mute status of the amp.
>> This way its possible to see the status of the amp, without breaking
>> the mechanism.
>> Would this be acceptable?
> Yeah, that's a compromise.
>
> BTW, the acpi notification handling is enabled for all devices?  I
> don't see the conditional enablement.
>
>
> thanks,
>
> Takashi

Thanks, I re-do this patch and add the ALSA control.
Whilst I dont think having the notification handler installed for all 
devices causes any issues, it is unneccesary for most models, so I'll 
add a conditional check for this.

Thanks,

Stefan

Powered by blists - more mailing lists