| Message ID | 20120802134612.GA30802@growl (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On ?, 2012-08-02 at 15:46 +0200, Luca Tettamanti wrote: > On Thu, Aug 02, 2012 at 08:45:30AM +0800, Zhang Rui wrote: > > On ?, 2012-08-01 at 15:49 +0200, Luca Tettamanti wrote: > > > AMD ACPI interface may overload the standard event > > > ACPI_VIDEO_NOTIFY_PROBE (0x81) to signal AMD-specific events. In such > > > cases we don't want to send the keypress (KEY_SWITCHVIDEOMODE) to the > > > userspace because the user did not press the mode switch key (the > > > spurious keypress confuses the DE which usually changes the > > > display configuration and messes up a dual-screen setup). > > > This patch gives the radeon driver the chance to examine the event and > > > block the keypress if the event is an "AMD event". > > > > > > Signed-off-by: Luca Tettamanti <kronos.it@gmail.com> > > > --- > > > Any comment from ACPI front? > > > > > it looks good to me. > > But I'm wondering if we can use the following code for ACPI part, which > > looks cleaner. > > I know this may change the behavior of other events, but in theory, we > > should not send any input event if we know something wrong in kernel. > > > > what do you think? > > I like it, it's cleaner. > I've split the patch in two pieces (one for video, the other for > radeon) and adopted your suggestion. > Great. Acked-by: Zhang Rui <rui.zhang@intel.com> hmm, who should take these two patches? and which tree the second patch is based on? thanks, rui
On Thu, Aug 2, 2012 at 9:40 PM, Zhang Rui <rui.zhang@intel.com> wrote: > On ?, 2012-08-02 at 15:46 +0200, Luca Tettamanti wrote: >> On Thu, Aug 02, 2012 at 08:45:30AM +0800, Zhang Rui wrote: >> > On ?, 2012-08-01 at 15:49 +0200, Luca Tettamanti wrote: >> > > AMD ACPI interface may overload the standard event >> > > ACPI_VIDEO_NOTIFY_PROBE (0x81) to signal AMD-specific events. In such >> > > cases we don't want to send the keypress (KEY_SWITCHVIDEOMODE) to the >> > > userspace because the user did not press the mode switch key (the >> > > spurious keypress confuses the DE which usually changes the >> > > display configuration and messes up a dual-screen setup). >> > > This patch gives the radeon driver the chance to examine the event and >> > > block the keypress if the event is an "AMD event". >> > > >> > > Signed-off-by: Luca Tettamanti <kronos.it@gmail.com> >> > > --- >> > > Any comment from ACPI front? >> > > >> > it looks good to me. >> > But I'm wondering if we can use the following code for ACPI part, which >> > looks cleaner. >> > I know this may change the behavior of other events, but in theory, we >> > should not send any input event if we know something wrong in kernel. >> > >> > what do you think? >> >> I like it, it's cleaner. >> I've split the patch in two pieces (one for video, the other for >> radeon) and adopted your suggestion. >> > Great. > Acked-by: Zhang Rui <rui.zhang@intel.com> > > hmm, who should take these two patches? I'm happy to take the patches. > and which tree the second patch is based on? I've got a tree with all the radeon ACPI patches on the acpi_patches branches of my git tree: git://people.freedesktop.org/~agd5f/linux Alex > > thanks, > rui >
On ?, 2012-08-02 at 21:45 -0400, Alex Deucher wrote: > On Thu, Aug 2, 2012 at 9:40 PM, Zhang Rui <rui.zhang@intel.com> wrote: > > On ?, 2012-08-02 at 15:46 +0200, Luca Tettamanti wrote: > >> On Thu, Aug 02, 2012 at 08:45:30AM +0800, Zhang Rui wrote: > >> > On ?, 2012-08-01 at 15:49 +0200, Luca Tettamanti wrote: > >> > > AMD ACPI interface may overload the standard event > >> > > ACPI_VIDEO_NOTIFY_PROBE (0x81) to signal AMD-specific events. In such > >> > > cases we don't want to send the keypress (KEY_SWITCHVIDEOMODE) to the > >> > > userspace because the user did not press the mode switch key (the > >> > > spurious keypress confuses the DE which usually changes the > >> > > display configuration and messes up a dual-screen setup). > >> > > This patch gives the radeon driver the chance to examine the event and > >> > > block the keypress if the event is an "AMD event". > >> > > > >> > > Signed-off-by: Luca Tettamanti <kronos.it@gmail.com> > >> > > --- > >> > > Any comment from ACPI front? > >> > > > >> > it looks good to me. > >> > But I'm wondering if we can use the following code for ACPI part, which > >> > looks cleaner. > >> > I know this may change the behavior of other events, but in theory, we > >> > should not send any input event if we know something wrong in kernel. > >> > > >> > what do you think? > >> > >> I like it, it's cleaner. > >> I've split the patch in two pieces (one for video, the other for > >> radeon) and adopted your suggestion. > >> > > Great. > > Acked-by: Zhang Rui <rui.zhang@intel.com> > > > > hmm, who should take these two patches? > > I'm happy to take the patches. > > > and which tree the second patch is based on? > > I've got a tree with all the radeon ACPI patches on the acpi_patches > branches of my git tree: > git://people.freedesktop.org/~agd5f/linux > great. thanks, rui
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c index 1e0a9e1..d75642a 100644 --- a/drivers/acpi/video.c +++ b/drivers/acpi/video.c @@ -1448,8 +1448,7 @@ static void acpi_video_bus_notify(struct acpi_device *device, u32 event) case ACPI_VIDEO_NOTIFY_SWITCH: /* User requested a switch, * most likely via hotkey. */ acpi_bus_generate_proc_event(device, event, 0); - if (!acpi_notifier_call_chain(device, event, 0)) - keycode = KEY_SWITCHVIDEOMODE; + keycode = KEY_SWITCHVIDEOMODE; break; case ACPI_VIDEO_NOTIFY_PROBE: /* User plugged in or removed a video @@ -1479,8 +1478,9 @@ static void acpi_video_bus_notify(struct acpi_device *device, u32 event) break; } - if (event != ACPI_VIDEO_NOTIFY_SWITCH) - acpi_notifier_call_chain(device, event, 0); + if (acpi_notifier_call_chain(device, event, 0)) + /* Something vetoed the keypress. */ + keycode = 0; if (keycode) { input_report_key(input, keycode, 1);