Re: [RFC PATCH] thinkpad-acpi: Improve hardware volume controls

Re: [RFC PATCH] thinkpad-acpi: Improve hardware volume controls

[Andrew Lutomirski]

[Hi, Dmitry -- there's an input layer question in here for you]

On Thu, May 12, 2011 at 10:39 AM, Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> On Thu, May 12, 2011 at 10:24:10AM -0400, Andrew Lutomirski wrote:
>> On Thu, May 12, 2011 at 9:48 AM, Matthew Garrett <mjg59@srcf.ucam.org> wrote:
>> > It looks like SAUM was introduced with the *61 machines, and it's
>> > identical from then on.
>>
>> I wonder the machines with SAUM are the same as the machines on which
>> pressing mute generates an i8042 keystroke instead of an HKEY.  If so,
>> it looks like there's some code (or at least comments) in
>> thinkpad_acpi that mention doing the right thing when the HKEY mute
>> event in generated (e.g. the driver won't send KEY_MUTE to the input
>> layer), so something like my patch along with removing the _OSI(Linux)
>> hack for the newer models might make everything work right.
>
> I think so. The machines we have in the OSI blacklist all appear to have
> the SAUM method, so I think we can take your patch and drop the
> blacklist. Good work!
>
>> Still, someone who has an older laptop should test it, because all I
>> can do is pretend I carefully inspected all the possible code paths.
>
> I can't see any way this would cause problems, except in the case where
> the method exists but doesn't do anything. I'd be surprised if that's a
> real problem.
>
>> (Even if it works, don't apply this patch for 2.6.40 as it stands
>> because the ALSA change notification on KEY_MUTE is crap and should at
>> least ignore key release events.)
>
> Are you working with the ALSA people on that?

I don't think I need help from ALSA.  I just need to stop telling ALSA
that the volume changes twice every time that mute is pressed.  I'll
send v3 out in the next day or two with the fix.

I do, however, have a question for the input people.  Dmitry: Lenovo
makes laptops which are kind enough to tell us that the volume changed
by sending a keystroke over the atkbd-based keyboard.  (wtf!)  I've
modified the thinkpad-acpi driver to register an input handler to
catch those events coming from the keyboard and send them to ALSA
where they belong.  But if there's a keyboard grab, it won't work.
Would you accept a patch to the input layer to allow filters (or maybe
just filters that specifically request it) to run even if there's a
grab?

Without a change to the input layer, if someone grabs the keyboard
then the volume controls will start getting screwed up.  If X grabs
the keyboard it's even worse because pulseaudio will start getting
extra confused and preventing that is the whole point of this patch.

(Yes, it's gross, but the way around it I can see would be to inject a
custom ACPI method, and that still might not be enough to prevent
userspace from seeing a keystroke that it's really not supposed to
see.  And IMHO custom ACPI methods are even worse than input filters.)

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH] thinkpad-acpi: Improve hardware volume controls

[Dmitry Torokhov]

Hi Andrew,

On Thu, May 12, 2011 at 10:50:54AM -0400, Andrew Lutomirski wrote:
> 
> I do, however, have a question for the input people.  Dmitry: Lenovo
> makes laptops which are kind enough to tell us that the volume changed
> by sending a keystroke over the atkbd-based keyboard.  (wtf!)  I've
> modified the thinkpad-acpi driver to register an input handler to
> catch those events coming from the keyboard and send them to ALSA
> where they belong.  But if there's a keyboard grab, it won't work.
> Would you accept a patch to the input layer to allow filters (or maybe
> just filters that specifically request it) to run even if there's a
> grab?

There is a filter on i8042 level that was introduced specifically for
cases when events not having any relation to the input are routed via
KBC interface. It looks like this is the one you want to use. See
include/linux/i8042.h::i8042_install_filter(). It allows for such events
to completely bypass input layer.

Hope this helps.

-- 
Dmitry

------------------------------------------------------------------------------
Achieve unprecedented app performance and reliability
What every C/C++ and Fortran developer should know.
Learn how Intel has extended the reach of its next-generation tools
to help boost performance applications - inlcuding clusters.
http://p.sf.net/sfu/intel-dev2devmay

Re: [RFC PATCH] thinkpad-acpi: Improve hardware volume controls

[Andrew Lutomirski]

On Thu, May 12, 2011 at 11:39 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> Hi Andrew,
>
> On Thu, May 12, 2011 at 10:50:54AM -0400, Andrew Lutomirski wrote:
>>
>> I do, however, have a question for the input people.  Dmitry: Lenovo
>> makes laptops which are kind enough to tell us that the volume changed
>> by sending a keystroke over the atkbd-based keyboard.  (wtf!)  I've
>> modified the thinkpad-acpi driver to register an input handler to
>> catch those events coming from the keyboard and send them to ALSA
>> where they belong.  But if there's a keyboard grab, it won't work.
>> Would you accept a patch to the input layer to allow filters (or maybe
>> just filters that specifically request it) to run even if there's a
>> grab?
>
> There is a filter on i8042 level that was introduced specifically for
> cases when events not having any relation to the input are routed via
> KBC interface. It looks like this is the one you want to use. See
> include/linux/i8042.h::i8042_install_filter(). It allows for such events
> to completely bypass input layer.
>
> Hope this helps.

Sort of.

dell-laptop and msi-laptop are content to take some action on the keys
they see but still leave the keys in the input stream, so their job is
a bit easier.

I need to swallow one kind of extended key, detect and not swallow two
others, and ignore all the ones that are normal keys.  But that means
that I don't know whether I should filter out 0xe0 until it's too
late.

So either I'd need a function to feed an event back into i8042 or I
need to filter a little farther downstream when the keys are resolved
into keycodes (or scancodes -- I'm not really up on the terminology).

--andy

Re: [RFC PATCH] thinkpad-acpi: Improve hardware volume controls

[Dmitry Torokhov]

On Thu, May 12, 2011 at 03:33:54PM -0400, Andrew Lutomirski wrote:
> On Thu, May 12, 2011 at 11:39 AM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > Hi Andrew,
> >
> > On Thu, May 12, 2011 at 10:50:54AM -0400, Andrew Lutomirski wrote:
> >>
> >> I do, however, have a question for the input people.  Dmitry: Lenovo
> >> makes laptops which are kind enough to tell us that the volume changed
> >> by sending a keystroke over the atkbd-based keyboard.  (wtf!)  I've
> >> modified the thinkpad-acpi driver to register an input handler to
> >> catch those events coming from the keyboard and send them to ALSA
> >> where they belong.  But if there's a keyboard grab, it won't work.
> >> Would you accept a patch to the input layer to allow filters (or maybe
> >> just filters that specifically request it) to run even if there's a
> >> grab?
> >
> > There is a filter on i8042 level that was introduced specifically for
> > cases when events not having any relation to the input are routed via
> > KBC interface. It looks like this is the one you want to use. See
> > include/linux/i8042.h::i8042_install_filter(). It allows for such events
> > to completely bypass input layer.
> >
> > Hope this helps.
> 
> Sort of.
> 
> dell-laptop and msi-laptop are content to take some action on the keys
> they see but still leave the keys in the input stream, so their job is
> a bit easier.
> 
> I need to swallow one kind of extended key, detect and not swallow two
> others, and ignore all the ones that are normal keys.  But that means
> that I don't know whether I should filter out 0xe0 until it's too
> late.
> 
> So either I'd need a function to feed an event back into i8042 or I
> need to filter a little farther downstream when the keys are resolved
> into keycodes (or scancodes -- I'm not really up on the terminology).

You can use serio_interrupt() to inject additional bytes into serio data
stream.

Thanks.

-- 
Dmitry