From patchwork Thu Aug 2 00:45:30 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Zhang Rui X-Patchwork-Id: 1266181 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by patchwork2.kernel.org (Postfix) with ESMTP id 273B1DF223 for ; Thu, 2 Aug 2012 04:56:53 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 063E8A0A4B for ; Wed, 1 Aug 2012 21:56:53 -0700 (PDT) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mga03.intel.com (mga03.intel.com [143.182.124.21]) by gabe.freedesktop.org (Postfix) with ESMTP id 8E99D9EB24 for ; Wed, 1 Aug 2012 17:44:17 -0700 (PDT) Received: from azsmga001.ch.intel.com ([10.2.17.19]) by azsmga101.ch.intel.com with ESMTP; 01 Aug 2012 17:44:17 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.71,315,1320652800"; d="scan'208";a="175959464" Received: from rui.sh.intel.com (HELO [10.239.36.18]) ([10.239.36.18]) by azsmga001.ch.intel.com with ESMTP; 01 Aug 2012 17:44:15 -0700 Message-ID: <1343868330.1682.502.camel@rui.sh.intel.com> Subject: Re: [PATCH/RFC] drm/radeon: ACPI: veto the keypress on ATIF events From: Zhang Rui To: Luca Tettamanti Date: Thu, 02 Aug 2012 08:45:30 +0800 In-Reply-To: <20120801134900.GA7909@growl> References: <20120728145626.GA6304@growl> <20120729130644.GA12378@growl> <20120730202449.GA5600@growl> <20120731200520.GA5425@growl> <20120801134900.GA7909@growl> X-Mailer: Evolution 3.2.2 (3.2.2-1.fc16) Mime-Version: 1.0 X-Mailman-Approved-At: Wed, 01 Aug 2012 21:52:43 -0700 Cc: linux-acpi@vger.kernel.org, dri-devel@lists.freedesktop.org, joeyli , Alex Deucher , Len Brown X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org Errors-To: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org 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? > 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 */ 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);