On Wed, Sep 14, 2016 at 8:11 PM, Anton Lindqvist
<anton.lindqv...@gmail.com> wrote:
> On Tue, Sep 13, 2016 at 10:48:26AM -0600, Theo de Raadt wrote:
>> > Anton Lindqvist wrote:
>> > > I'm trying to fix a minor annoyance on my x240: the speaker mute key
>> > > LED-state is not respected at boot. Pressing the mute key will mute the
>> > > speaker while the expected behavior is to unmute. The LED-state will
>> > > remain out-of-sync until I run `mixerctl -t outputs.master.mute`.
>> > >
>> > > I've managed to determine if the speaker is muted in the acpithinkpad(4)
>> > > attach function by reading from the embedded controller. However,
>> > > calling wskbd_set_mixervolume at this stage returns ENODEV. I assume the
>> > > audio device has not been attached yet. I'm new to kernel development
>> > > and therefore wonder if this approach makes sense. If true, is it
>> > > possible to postpone a task to run once a certain device has attached?
>> >
>> > The function you're looking for is startuphook_establish.
>> >
>> > But this requires crazy amounts of testing, since many thinkpads will be
>> > different. It's well known that the mute button, hardware state, and 
>> > software
>> > state can became desynced, but the specifics vary by model.
>> >
>> Actually, I think Anton might be onto something here.  The desyncronization
>> may be due to the audio driver + BIOS not recognizing the other's state.
> By using startuphook_establish, as proposed by tedu@, I managed to get
> it working. Booting with the volume muted before reboot and the volume
> not muted before reboot both behaves correctly.
> A few comments regarding the patch:
> - At first, I tried sticking all code in the startuphook function
>   because it felt like a good approach to keep all the logic in one
>   place and reduce the number of ifdef-conditionals. However, that
>   caused my kernel to halt since the EC is busy at this point and the
>   assertion in acpiec.c:337 failed. The only other usage of acpiec_read
>   I can find is related to polling the sensors. I therefore kept volume
>   reading from the EC inside the attached function and moved it prior
>   attaching the sensors. The kernel then continued successfully, but I'm
>   still worried a that potential race still can occur. Or is the action
>   taken enough to make the volume EC read atomic?
> - I didn't bother checking the return value of startuphook_establish
>   since I couldn't find any other invocation in the kernel doing so.
> - The VOLUME_MUTE_MASK is taken from the FreeBSD ThinkPad ACPI driver.
> If the patch looks good, then I would like to add some logging prior
> asking people to try it out.

Works as intended on my X220.


Reply via email to