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

[Zhang Rui <rui.zhang@intel.com>]

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?

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 */


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