[PATCH/RFC] drm/radeon: ACPI: veto the keypress on ATIF events

2012-08-03 Thread Zhang Rui 
On ?, 2012-08-02 at 21:45 -0400, Alex Deucher wrote:
> On Thu, Aug 2, 2012 at 9:40 PM, Zhang Rui  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 
> >> > > ---
> >> > > 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 
> >
> > 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

[PATCH/RFC] drm/radeon: ACPI: veto the keypress on ATIF events

2012-08-03 Thread Zhang Rui 
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 
> > > ---
> > > 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 

hmm, who should take these two patches?
and which tree the second patch is based on?

thanks,
rui

Re: [PATCH/RFC] drm/radeon: ACPI: veto the keypress on ATIF events

2012-08-03 Thread Zhang Rui 
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...@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.zh...@intel.com

hmm, who should take these two patches?
and which tree the second patch is based on?

thanks,
rui

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH/RFC] drm/radeon: ACPI: veto the keypress on ATIF events

2012-08-03 Thread Zhang Rui 
On 四, 2012-08-02 at 21:45 -0400, Alex Deucher wrote:
 On Thu, Aug 2, 2012 at 9:40 PM, Zhang Rui rui.zh...@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...@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.zh...@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

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH/RFC] drm/radeon: ACPI: veto the keypress on ATIF events

2012-08-02 Thread Alex Deucher 
On Thu, Aug 2, 2012 at 9:40 PM, Zhang Rui  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 
>> > > ---
>> > > 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 
>
> 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
>

[PATCH/RFC] drm/radeon: ACPI: veto the keypress on ATIF events

2012-08-02 Thread Luca Tettamanti 
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 
> > ---
> > 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.

BTW, I'm leaving for vacation in a few hours, I'll be offline till mid
August.

Luca
-- next part --
>From acce30c96b90d1bc550e82a9e7f19226fa194d5e Mon Sep 17 00:00:00 2001
From: Luca Tettamanti 
Date: Thu, 2 Aug 2012 15:30:27 +0200
Subject: [PATCH 1/2] ACPI video: allow events handlers to veto the keypress

The standard video events may be overloaded for device specific
purposes. For example AMD ACPI interface overloads
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 handlers the chance to examine the event and
block the keypress if the event is device specific.
v2: refactor as suggested by Zhang Rui 

Signed-off-by: Luca Tettamanti 
---
 drivers/acpi/video.c |8 
 1 file changed, 4 insertions(+), 4 deletions(-)

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);
-- 
1.7.10.4

-- next part --
>From def5119d8f617eef0fac2cd35d7bb18c176ff8f6 Mon Sep 17 00:00:00 2001
From: Luca Tettamanti 
Date: Thu, 2 Aug 2012 15:33:03 +0200
Subject: [PATCH 2/2] drm/radeon: block the keypress on ATIF events

The AMD ACPI interface may use ACPI_VIDEO_NOTIFY_PROBE to signal SBIOS
requests; block the keypress in this case since the user did not
actually press the mode switch key.

Signed-off-by: Luca Tettamanti 
---
 drivers/gpu/drm/radeon/radeon_acpi.c |7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/radeon/radeon_acpi.c 
b/drivers/gpu/drm/radeon/radeon_acpi.c
index 96de08d..ee0d29e 100644
--- a/drivers/gpu/drm/radeon/radeon_acpi.c
+++ b/drivers/gpu/drm/radeon/radeon_acpi.c
@@ -273,7 +273,12 @@ int radeon_atif_handler(struct radeon_device *rdev,
}
/* TODO: check other events */

-   return NOTIFY_OK;
+   /* We've handled the event, stop the notifier chain. The ACPI interface
+* overloads ACPI_VIDEO_NOTIFY_PROBE, we don't want to send that to
+* userspace if the event was generated only to signal a SBIOS
+* request.
+*/
+   return NOTIFY_BAD;
 }

 /* Call all ACPI methods here */
-- 
1.7.10.4

[PATCH/RFC] drm/radeon: ACPI: veto the keypress on ATIF events

2012-08-02 Thread Zhang Rui 
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 
> ---
> 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?

diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 1e0a9e1..8ad1f53 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,8 @@ 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))
+   keycode = 0;

if (keycode) {
input_report_key(input, keycode, 1);


>  drivers/acpi/video.c |   10 --
>  drivers/gpu/drm/radeon/radeon_acpi.c |7 ++-
>  2 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> index 1e0a9e1..a8592c4 100644
> --- a/drivers/acpi/video.c
> +++ b/drivers/acpi/video.c
> @@ -1457,7 +1457,12 @@ static void acpi_video_bus_notify(struct acpi_device 
> *device, u32 event)
>   acpi_video_device_enumerate(video);
>   acpi_video_device_rebind(video);
>   acpi_bus_generate_proc_event(device, event, 0);
> - keycode = KEY_SWITCHVIDEOMODE;
> + /* This event is also overloaded by AMD ACPI interface, don't
> +  * send the key press if the event has been handled elsewhere
> +  * (e.g. radeon DRM driver).
> +  */
> + if (!acpi_notifier_call_chain(device, event, 0))
> + keycode = KEY_SWITCHVIDEOMODE;
>   break;
>  
>   case ACPI_VIDEO_NOTIFY_CYCLE:   /* Cycle Display output hotkey pressed. 
> */
> @@ -1479,7 +1484,8 @@ static void acpi_video_bus_notify(struct acpi_device 
> *device, u32 event)
>   break;
>   }
>  
> - if (event != ACPI_VIDEO_NOTIFY_SWITCH)
> + if (event != ACPI_VIDEO_NOTIFY_SWITCH &&
> + event != ACPI_VIDEO_NOTIFY_PROBE)
>   acpi_notifier_call_chain(device, event, 0);
>  
>   if (keycode) {
> diff --git a/drivers/gpu/drm/radeon/radeon_acpi.c 
> b/drivers/gpu/drm/radeon/radeon_acpi.c
> index 96de08d..ee0d29e 100644
> --- a/drivers/gpu/drm/radeon/radeon_acpi.c
> +++ b/drivers/gpu/drm/radeon/radeon_acpi.c
> @@ -273,7 +273,12 @@ int radeon_atif_handler(struct radeon_device *rdev,
>   }
>   /* TODO: check other events */
>  
> - return NOTIFY_OK;
> + /* We've handled the event, stop the notifier chain. The ACPI interface
> +  * overloads ACPI_VIDEO_NOTIFY_PROBE, we don't want to send that to
> +  * userspace if the event was generated only to signal a SBIOS
> +  * request.
> +  */
> + return NOTIFY_BAD;
>  }
>  
>  /* Call all ACPI methods here */

Re: [PATCH/RFC] drm/radeon: ACPI: veto the keypress on ATIF events

2012-08-02 Thread Luca Tettamanti 
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...@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.

BTW, I'm leaving for vacation in a few hours, I'll be offline till mid
August.

Luca
From acce30c96b90d1bc550e82a9e7f19226fa194d5e Mon Sep 17 00:00:00 2001
From: Luca Tettamanti kronos...@gmail.com
Date: Thu, 2 Aug 2012 15:30:27 +0200
Subject: [PATCH 1/2] ACPI video: allow events handlers to veto the keypress

The standard video events may be overloaded for device specific
purposes. For example AMD ACPI interface overloads
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 handlers the chance to examine the event and
block the keypress if the event is device specific.
v2: refactor as suggested by Zhang Rui rui.zh...@intel.com

Signed-off-by: Luca Tettamanti kronos...@gmail.com
---
 drivers/acpi/video.c |8 
 1 file changed, 4 insertions(+), 4 deletions(-)

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);
-- 
1.7.10.4

From def5119d8f617eef0fac2cd35d7bb18c176ff8f6 Mon Sep 17 00:00:00 2001
From: Luca Tettamanti kronos...@gmail.com
Date: Thu, 2 Aug 2012 15:33:03 +0200
Subject: [PATCH 2/2] drm/radeon: block the keypress on ATIF events

The AMD ACPI interface may use ACPI_VIDEO_NOTIFY_PROBE to signal SBIOS
requests; block the keypress in this case since the user did not
actually press the mode switch key.

Signed-off-by: Luca Tettamanti kronos...@gmail.com
---
 drivers/gpu/drm/radeon/radeon_acpi.c |7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/radeon/radeon_acpi.c 
b/drivers/gpu/drm/radeon/radeon_acpi.c
index 96de08d..ee0d29e 100644
--- a/drivers/gpu/drm/radeon/radeon_acpi.c
+++ b/drivers/gpu/drm/radeon/radeon_acpi.c
@@ -273,7 +273,12 @@ int radeon_atif_handler(struct radeon_device *rdev,
}
/* TODO: check other events */
 
-   return NOTIFY_OK;
+   /* We've handled the event, stop the notifier chain. The ACPI interface
+* overloads ACPI_VIDEO_NOTIFY_PROBE, we don't want to send that to
+* userspace if the event was generated only to signal a SBIOS
+* request.
+*/
+   return NOTIFY_BAD;
 }
 
 /* Call all ACPI methods here */
-- 
1.7.10.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH/RFC] drm/radeon: ACPI: veto the keypress on ATIF events

2012-08-02 Thread Alex Deucher 
On Thu, Aug 2, 2012 at 9:40 PM, Zhang Rui rui.zh...@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...@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.zh...@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

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH/RFC] drm/radeon: ACPI: veto the keypress on ATIF events

2012-08-01 Thread joeyli 
? ??2012-08-01 ? 15:49 +0200?Luca Tettamanti ???
> 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 


Tested-by: Lee, Chun-Yi 


This patch works to me on my HP notebook to avoid (VGA, 0x81) notify
event issued when AC-power unpluged.


Thanks
Joey Lee

> ---
> Any comment from ACPI front?
> 
>  drivers/acpi/video.c |   10 --
>  drivers/gpu/drm/radeon/radeon_acpi.c |7 ++-
>  2 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> index 1e0a9e1..a8592c4 100644
> --- a/drivers/acpi/video.c
> +++ b/drivers/acpi/video.c
> @@ -1457,7 +1457,12 @@ static void acpi_video_bus_notify(struct acpi_device 
> *device, u32 event)
>   acpi_video_device_enumerate(video);
>   acpi_video_device_rebind(video);
>   acpi_bus_generate_proc_event(device, event, 0);
> - keycode = KEY_SWITCHVIDEOMODE;
> + /* This event is also overloaded by AMD ACPI interface, don't
> +  * send the key press if the event has been handled elsewhere
> +  * (e.g. radeon DRM driver).
> +  */
> + if (!acpi_notifier_call_chain(device, event, 0))
> + keycode = KEY_SWITCHVIDEOMODE;
>   break;
>  
>   case ACPI_VIDEO_NOTIFY_CYCLE:   /* Cycle Display output hotkey pressed. 
> */
> @@ -1479,7 +1484,8 @@ static void acpi_video_bus_notify(struct acpi_device 
> *device, u32 event)
>   break;
>   }
>  
> - if (event != ACPI_VIDEO_NOTIFY_SWITCH)
> + if (event != ACPI_VIDEO_NOTIFY_SWITCH &&
> + event != ACPI_VIDEO_NOTIFY_PROBE)
>   acpi_notifier_call_chain(device, event, 0);
>  
>   if (keycode) {
> diff --git a/drivers/gpu/drm/radeon/radeon_acpi.c 
> b/drivers/gpu/drm/radeon/radeon_acpi.c
> index 96de08d..ee0d29e 100644
> --- a/drivers/gpu/drm/radeon/radeon_acpi.c
> +++ b/drivers/gpu/drm/radeon/radeon_acpi.c
> @@ -273,7 +273,12 @@ int radeon_atif_handler(struct radeon_device *rdev,
>   }
>   /* TODO: check other events */
>  
> - return NOTIFY_OK;
> + /* We've handled the event, stop the notifier chain. The ACPI interface
> +  * overloads ACPI_VIDEO_NOTIFY_PROBE, we don't want to send that to
> +  * userspace if the event was generated only to signal a SBIOS
> +  * request.
> +  */
> + return NOTIFY_BAD;
>  }
>  
>  /* Call all ACPI methods here */

[PATCH/RFC] drm/radeon: ACPI: veto the keypress on ATIF events

2012-08-01 Thread Luca Tettamanti 
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 
---
Any comment from ACPI front?

 drivers/acpi/video.c |   10 --
 drivers/gpu/drm/radeon/radeon_acpi.c |7 ++-
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 1e0a9e1..a8592c4 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -1457,7 +1457,12 @@ static void acpi_video_bus_notify(struct acpi_device 
*device, u32 event)
acpi_video_device_enumerate(video);
acpi_video_device_rebind(video);
acpi_bus_generate_proc_event(device, event, 0);
-   keycode = KEY_SWITCHVIDEOMODE;
+   /* This event is also overloaded by AMD ACPI interface, don't
+* send the key press if the event has been handled elsewhere
+* (e.g. radeon DRM driver).
+*/
+   if (!acpi_notifier_call_chain(device, event, 0))
+   keycode = KEY_SWITCHVIDEOMODE;
break;

case ACPI_VIDEO_NOTIFY_CYCLE:   /* Cycle Display output hotkey pressed. 
*/
@@ -1479,7 +1484,8 @@ static void acpi_video_bus_notify(struct acpi_device 
*device, u32 event)
break;
}

-   if (event != ACPI_VIDEO_NOTIFY_SWITCH)
+   if (event != ACPI_VIDEO_NOTIFY_SWITCH &&
+   event != ACPI_VIDEO_NOTIFY_PROBE)
acpi_notifier_call_chain(device, event, 0);

if (keycode) {
diff --git a/drivers/gpu/drm/radeon/radeon_acpi.c 
b/drivers/gpu/drm/radeon/radeon_acpi.c
index 96de08d..ee0d29e 100644
--- a/drivers/gpu/drm/radeon/radeon_acpi.c
+++ b/drivers/gpu/drm/radeon/radeon_acpi.c
@@ -273,7 +273,12 @@ int radeon_atif_handler(struct radeon_device *rdev,
}
/* TODO: check other events */

-   return NOTIFY_OK;
+   /* We've handled the event, stop the notifier chain. The ACPI interface
+* overloads ACPI_VIDEO_NOTIFY_PROBE, we don't want to send that to
+* userspace if the event was generated only to signal a SBIOS
+* request.
+*/
+   return NOTIFY_BAD;
 }

 /* Call all ACPI methods here */
-- 
1.7.10.4

[PATCH/RFC] drm/radeon: ACPI: veto the keypress on ATIF events

2012-08-01 Thread Alex Deucher 
On Wed, Aug 1, 2012 at 9:49 AM, 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 

Looks good to me, but I'm certainly not an ACPI expert.

Acked-by: Alex Deucher

[PATCH/RFC] drm/radeon: ACPI: veto the keypress on ATIF events

2012-08-01 Thread Luca Tettamanti 
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...@gmail.com
---
Any comment from ACPI front?

 drivers/acpi/video.c |   10 --
 drivers/gpu/drm/radeon/radeon_acpi.c |7 ++-
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 1e0a9e1..a8592c4 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -1457,7 +1457,12 @@ static void acpi_video_bus_notify(struct acpi_device 
*device, u32 event)
acpi_video_device_enumerate(video);
acpi_video_device_rebind(video);
acpi_bus_generate_proc_event(device, event, 0);
-   keycode = KEY_SWITCHVIDEOMODE;
+   /* This event is also overloaded by AMD ACPI interface, don't
+* send the key press if the event has been handled elsewhere
+* (e.g. radeon DRM driver).
+*/
+   if (!acpi_notifier_call_chain(device, event, 0))
+   keycode = KEY_SWITCHVIDEOMODE;
break;
 
case ACPI_VIDEO_NOTIFY_CYCLE:   /* Cycle Display output hotkey pressed. 
*/
@@ -1479,7 +1484,8 @@ static void acpi_video_bus_notify(struct acpi_device 
*device, u32 event)
break;
}
 
-   if (event != ACPI_VIDEO_NOTIFY_SWITCH)
+   if (event != ACPI_VIDEO_NOTIFY_SWITCH 
+   event != ACPI_VIDEO_NOTIFY_PROBE)
acpi_notifier_call_chain(device, event, 0);
 
if (keycode) {
diff --git a/drivers/gpu/drm/radeon/radeon_acpi.c 
b/drivers/gpu/drm/radeon/radeon_acpi.c
index 96de08d..ee0d29e 100644
--- a/drivers/gpu/drm/radeon/radeon_acpi.c
+++ b/drivers/gpu/drm/radeon/radeon_acpi.c
@@ -273,7 +273,12 @@ int radeon_atif_handler(struct radeon_device *rdev,
}
/* TODO: check other events */
 
-   return NOTIFY_OK;
+   /* We've handled the event, stop the notifier chain. The ACPI interface
+* overloads ACPI_VIDEO_NOTIFY_PROBE, we don't want to send that to
+* userspace if the event was generated only to signal a SBIOS
+* request.
+*/
+   return NOTIFY_BAD;
 }
 
 /* Call all ACPI methods here */
-- 
1.7.10.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH/RFC] drm/radeon: ACPI: veto the keypress on ATIF events

2012-08-01 Thread Alex Deucher 
On Wed, Aug 1, 2012 at 9:49 AM, Luca Tettamanti kronos...@gmail.com 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...@gmail.com

Looks good to me, but I'm certainly not an ACPI expert.

Acked-by: Alex Deucher alexander.deuc...@amd.com
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH/RFC] drm/radeon: ACPI: veto the keypress on ATIF events

2012-08-01 Thread joeyli 
於 三，2012-08-01 於 15:49 +0200，Luca Tettamanti 提到：
 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...@gmail.com


Tested-by: Lee, Chun-Yi j...@suse.com


This patch works to me on my HP notebook to avoid (VGA, 0x81) notify
event issued when AC-power unpluged.


Thanks
Joey Lee

 ---
 Any comment from ACPI front?
 
  drivers/acpi/video.c |   10 --
  drivers/gpu/drm/radeon/radeon_acpi.c |7 ++-
  2 files changed, 14 insertions(+), 3 deletions(-)
 
 diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
 index 1e0a9e1..a8592c4 100644
 --- a/drivers/acpi/video.c
 +++ b/drivers/acpi/video.c
 @@ -1457,7 +1457,12 @@ static void acpi_video_bus_notify(struct acpi_device 
 *device, u32 event)
   acpi_video_device_enumerate(video);
   acpi_video_device_rebind(video);
   acpi_bus_generate_proc_event(device, event, 0);
 - keycode = KEY_SWITCHVIDEOMODE;
 + /* This event is also overloaded by AMD ACPI interface, don't
 +  * send the key press if the event has been handled elsewhere
 +  * (e.g. radeon DRM driver).
 +  */
 + if (!acpi_notifier_call_chain(device, event, 0))
 + keycode = KEY_SWITCHVIDEOMODE;
   break;
  
   case ACPI_VIDEO_NOTIFY_CYCLE:   /* Cycle Display output hotkey pressed. 
 */
 @@ -1479,7 +1484,8 @@ static void acpi_video_bus_notify(struct acpi_device 
 *device, u32 event)
   break;
   }
  
 - if (event != ACPI_VIDEO_NOTIFY_SWITCH)
 + if (event != ACPI_VIDEO_NOTIFY_SWITCH 
 + event != ACPI_VIDEO_NOTIFY_PROBE)
   acpi_notifier_call_chain(device, event, 0);
  
   if (keycode) {
 diff --git a/drivers/gpu/drm/radeon/radeon_acpi.c 
 b/drivers/gpu/drm/radeon/radeon_acpi.c
 index 96de08d..ee0d29e 100644
 --- a/drivers/gpu/drm/radeon/radeon_acpi.c
 +++ b/drivers/gpu/drm/radeon/radeon_acpi.c
 @@ -273,7 +273,12 @@ int radeon_atif_handler(struct radeon_device *rdev,
   }
   /* TODO: check other events */
  
 - return NOTIFY_OK;
 + /* We've handled the event, stop the notifier chain. The ACPI interface
 +  * overloads ACPI_VIDEO_NOTIFY_PROBE, we don't want to send that to
 +  * userspace if the event was generated only to signal a SBIOS
 +  * request.
 +  */
 + return NOTIFY_BAD;
  }
  
  /* Call all ACPI methods here */


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH/RFC] drm/radeon: ACPI: veto the keypress on ATIF events

2012-08-01 Thread Zhang Rui 
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...@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?

diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 1e0a9e1..8ad1f53 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,8 @@ 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))
+   keycode = 0;
 
if (keycode) {
input_report_key(input, keycode, 1);


  drivers/acpi/video.c |   10 --
  drivers/gpu/drm/radeon/radeon_acpi.c |7 ++-
  2 files changed, 14 insertions(+), 3 deletions(-)
 
 diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
 index 1e0a9e1..a8592c4 100644
 --- a/drivers/acpi/video.c
 +++ b/drivers/acpi/video.c
 @@ -1457,7 +1457,12 @@ static void acpi_video_bus_notify(struct acpi_device 
 *device, u32 event)
   acpi_video_device_enumerate(video);
   acpi_video_device_rebind(video);
   acpi_bus_generate_proc_event(device, event, 0);
 - keycode = KEY_SWITCHVIDEOMODE;
 + /* This event is also overloaded by AMD ACPI interface, don't
 +  * send the key press if the event has been handled elsewhere
 +  * (e.g. radeon DRM driver).
 +  */
 + if (!acpi_notifier_call_chain(device, event, 0))
 + keycode = KEY_SWITCHVIDEOMODE;
   break;
  
   case ACPI_VIDEO_NOTIFY_CYCLE:   /* Cycle Display output hotkey pressed. 
 */
 @@ -1479,7 +1484,8 @@ static void acpi_video_bus_notify(struct acpi_device 
 *device, u32 event)
   break;
   }
  
 - if (event != ACPI_VIDEO_NOTIFY_SWITCH)
 + if (event != ACPI_VIDEO_NOTIFY_SWITCH 
 + event != ACPI_VIDEO_NOTIFY_PROBE)
   acpi_notifier_call_chain(device, event, 0);
  
   if (keycode) {
 diff --git a/drivers/gpu/drm/radeon/radeon_acpi.c 
 b/drivers/gpu/drm/radeon/radeon_acpi.c
 index 96de08d..ee0d29e 100644
 --- a/drivers/gpu/drm/radeon/radeon_acpi.c
 +++ b/drivers/gpu/drm/radeon/radeon_acpi.c
 @@ -273,7 +273,12 @@ int radeon_atif_handler(struct radeon_device *rdev,
   }
   /* TODO: check other events */
  
 - return NOTIFY_OK;
 + /* We've handled the event, stop the notifier chain. The ACPI interface
 +  * overloads ACPI_VIDEO_NOTIFY_PROBE, we don't want to send that to
 +  * userspace if the event was generated only to signal a SBIOS
 +  * request.
 +  */
 + return NOTIFY_BAD;
  }
  
  /* Call all ACPI methods here */


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel