Re: [RFC PATCH] thinkpad-acpi: Improve hardware volume controls
On Thu, 12 May 2011, Andrew Lutomirski wrote: > 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. Will do, will do. Please be patient. I will have thinkpad-acpi time this weekend :-) As you all noticed by now, you really must know what you're doing when messing with the thinkpad volume stuff. I will carefully review the patch, and we should be able to massage it into something that can be merged during the weekend and next week. -- "One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh -- To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] thinkpad-acpi: Improve hardware volume controls
On Thu, May 12, 2011 at 03:33:54PM -0400, Andrew Lutomirski wrote: > On Thu, May 12, 2011 at 11:39 AM, Dmitry Torokhov > 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 -- To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] thinkpad-acpi: Improve hardware volume controls
On Thu, May 12, 2011 at 11:39 AM, Dmitry Torokhov 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 -- To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] thinkpad-acpi: Improve hardware volume controls
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 -- To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] thinkpad-acpi: Improve hardware volume controls
[Hi, Dmitry -- there's an input layer question in here for you] On Thu, May 12, 2011 at 10:39 AM, Matthew Garrett 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 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 platform-driver-x86" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] thinkpad-acpi: Improve hardware volume controls
On Thu, May 12, 2011 at 10:24:10AM -0400, Andrew Lutomirski wrote: > On Thu, May 12, 2011 at 9:48 AM, Matthew Garrett 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? -- Matthew Garrett | [email protected] -- To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] thinkpad-acpi: Improve hardware volume controls
On Thu, May 12, 2011 at 9:48 AM, Matthew Garrett wrote: > On Mon, May 09, 2011 at 06:18:46PM -0400, Andy Lutomirski wrote: > >> Henrique, do you know of anywhere to find AML dumps from different models? >> It would be nice to see what SAUM looks like. > > 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. 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. (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.) --Andy > > -- > Matthew Garrett | [email protected] > -- To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] thinkpad-acpi: Improve hardware volume controls
On Mon, May 09, 2011 at 06:18:46PM -0400, Andy Lutomirski wrote: > Henrique, do you know of anywhere to find AML dumps from different models? > It would be nice to see what SAUM looks like. It looks like SAUM was introduced with the *61 machines, and it's identical from then on. -- Matthew Garrett | [email protected] -- To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] thinkpad-acpi: Improve hardware volume controls
On Mon, May 9, 2011 at 7:53 PM, Henrique de Moraes Holschuh wrote: > Interesting. Let me test it on my T43, it has hardware volume and seems to > cover a class of box you couldn't test. > > I like the idea, but I might have some questions to ask you about it. More > on this after I test the patch in the next few days. FWIW, this patch (other than having the wrong default on X220) makes Linux better than Windows :) Do you know how to read a field from the EC by name? I think I can pull out the default value from the HAUM field, but I don't know how to access it through acpica and installing a custom method just to read a register seems like a bad idea. (The register's in the same place on X200 and X220, though, so maybe it's stable enough to just read. But using the field listing in the DSDT would make me happy.) Also, do you know how to ask the kernel what the most recent acpi interrupt was? I'd like to find a better way to detect the mute button than watching for the keyboard event. --Andy > > -- > "One disk to rule them all, One disk to find them. One disk to bring > them all and in the darkness grind them. In the Land of Redmond > where the shadows lie." -- The Silicon Valley Tarot > Henrique Holschuh > -- To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] thinkpad-acpi: Improve hardware volume controls
Interesting. Let me test it on my T43, it has hardware volume and seems to cover a class of box you couldn't test. I like the idea, but I might have some questions to ask you about it. More on this after I test the patch in the next few days. -- "One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh -- To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
