Message-ID: <87jzt6ja5s.wl-tiwai@suse.de>
Date:   Mon, 04 Sep 2023 16:16:47 +0200
From:   Takashi Iwai <tiwai@...e.de>
To:     Stefan Binding <sbinding@...nsource.cirrus.com>
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 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

Powered by blists - more mailing lists