From patchwork Mon Jul 30 08:32:47 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: joeyli X-Patchwork-Id: 1257951 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 205E7DFAF2 for ; Tue, 31 Jul 2012 05:24:53 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id CB4389E9F0 for ; Mon, 30 Jul 2012 22:24:52 -0700 (PDT) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from smtp.nue.novell.com (smtp.nue.novell.com [195.135.221.5]) by gabe.freedesktop.org (Postfix) with ESMTP id 8322B9F6A5 for ; Mon, 30 Jul 2012 01:35:32 -0700 (PDT) Received: from [147.2.211.131] (syd-fw-asa1.apac.novell.com [130.57.30.250]) by smtp.nue.novell.com with ESMTP (TLS encrypted); Mon, 30 Jul 2012 10:35:27 +0200 Subject: Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code From: joeyli To: Luca Tettamanti In-Reply-To: <20120729131058.GB12378@growl> References: <1343237889-5220-1-git-send-email-alexdeucher@gmail.com> <20120726125838.GA28853@growl> <20120726193346.GA11288@growl> <20120728145626.GA6304@growl> <1343533908.6341.6.camel@linux-s257.site> <20120729131058.GB12378@growl> Date: Mon, 30 Jul 2012 16:32:47 +0800 Message-ID: <1343637167.6341.76.camel@linux-s257.site> Mime-Version: 1.0 X-Mailer: Evolution 2.28.2 X-Mailman-Approved-At: Mon, 30 Jul 2012 22:19:18 -0700 Cc: Alex Deucher , dri-devel@lists.freedesktop.org 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 ? ??2012-07-29 ? 15:10 +0200?Luca Tettamanti ??? > On Sun, Jul 29, 2012 at 11:51:48AM +0800, joeyli wrote: > > Hi Luca, > > > > ? ??2012-07-28 ? 16:56 +0200?Luca Tettamanti ??? > > > I just found the first problem (probably a BIOS bug): > > > ATIF_FUNCTION_GET_SYSTEM_PARAMETERS is implemented in the DSDT, but the > > > corresponding bit ATIF_GET_SYSTEM_PARAMETERS_SUPPORTED is not set :( > > > I intended to use the method to set up the notification handler but now > > > my BIOS says that it's not there even if it is... > > > Can I assume some default values (e.g. notifications are enabled and will > > > use 0x81 unless ATIF_FUNCTION_GET_SYSTEM_PARAMETERS says something > > > different)? > > > > > > > Did you check your DSDT for there have some "Notify (VGA, 0x81)" > > statement in AFN0..AFN15? > > If YES, I think that means your machine in case the 0x81 is for ATI used > > by default. > > Yes, my point is that the nofication is there, but since > GET_SYSTEM_PARAMETERS is not announced I have not way to check it. > IOW, what is implemented in the DSDT does not match what is announced by > VERIFY_INTERFACE. > For reference this is the DSDT: http://pastebin.com/KKS7ZsTt > > Luca > Yes, saw the problem in your DSDT: Method (AF00, 0, NotSerialized) { CreateWordField (ATIB, Zero, SSZE) ... Store (0x80, NMSK) Store (0x02, SFUN) <=== 10b, bit 0 is 0 Return (ATIB) } But, AF01 still supported in ATIF on this machine, maybe we should still try GET_SYSTEM_PARAMETERS even the function vector set to 0? No idea... On the other hand, My patch to avoid 0x81 event conflict with acpi/video driver like below. This patch woks on my notebook. Your patches do much more things then mine, so I think my patch just for reference. There have a problem is: If we want use acpi_notifier_call_chain to check does event consume by any notifier in acpi's blocking notifier chain, then we need return NOTIFY_BAD in radeon_acpi but not NOTIFY_OK. So, I suggest radeon_acpi should register to acpi notifier chain by itself but not append on radeon_acpi_event in radeon_pm. And, suggest also check the device class is ACPI_VIDEO_CLASS like following: +static int radeon_acpi_video_event(struct notifier_block *nb, ... + if (strcmp(event->device_class, ACPI_VIDEO_CLASS) != 0) + return NOTIFY_DONE; + Thanks a lot! Joey Lee >From 9c0c42ab8f37dafd3b512ca395b64bf39819d26b Mon Sep 17 00:00:00 2001 From: Lee, Chun-Yi Date: Mon, 30 Jul 2012 16:17:05 +0800 Subject: [PATCH] drm/radeon avoid 0x81 acpi event conflict with acpi video driver drm/radeon avoid 0x81 acpi event conflict with acpi video driver Signed-off-by: Lee, Chun-Yi --- drivers/acpi/video.c | 6 +- drivers/gpu/drm/radeon/radeon_acpi.c | 150 ++++++++++++++++++++++++++++++---- 2 files changed, 139 insertions(+), 17 deletions(-) diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c index 1e0a9e1..e32492d 100644 --- a/drivers/acpi/video.c +++ b/drivers/acpi/video.c @@ -1457,7 +1457,8 @@ 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; + if (!acpi_notifier_call_chain(device, event, 0)) + keycode = KEY_SWITCHVIDEOMODE; break; case ACPI_VIDEO_NOTIFY_CYCLE: /* Cycle Display output hotkey pressed. */ @@ -1479,7 +1480,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 5b32e49..37ed5e1 100644 --- a/drivers/gpu/drm/radeon/radeon_acpi.c +++ b/drivers/gpu/drm/radeon/radeon_acpi.c @@ -3,6 +3,7 @@ #include #include #include +#include #include "drmP.h" #include "drm.h" @@ -13,36 +14,150 @@ #include +struct atif_verify_output { + u16 size; /* structure size in bytes (includes size field) */ + u16 version; /* version */ + u32 notif_mask; /* supported notifications mask */ + u32 func_bits; /* supported functions bit vector */ +} __attribute__((packed)); + +static struct atif_verify_output *verify_output; + +struct atif_get_sysparams_output { + u16 size; /* structure size in bytes (includes size field) */ + u32 valid_flags_mask; /* valid flags mask */ + u32 flags; /* flags */ + u8 notify_code; /* notify command code */ +} __attribute__((packed)); + +static u8 notify_code; + /* Call the ATIF method * * Note: currently we discard the output */ -static int radeon_atif_call(acpi_handle handle) +static int radeon_atif_call(acpi_handle handle, int function, struct acpi_buffer *params, struct acpi_buffer *output) { acpi_status status; union acpi_object atif_arg_elements[2]; struct acpi_object_list atif_arg; - struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL}; + union acpi_object *obj; - atif_arg.count = 2; + atif_arg.count = 1; atif_arg.pointer = &atif_arg_elements[0]; atif_arg_elements[0].type = ACPI_TYPE_INTEGER; - atif_arg_elements[0].integer.value = ATIF_FUNCTION_VERIFY_INTERFACE; - atif_arg_elements[1].type = ACPI_TYPE_INTEGER; - atif_arg_elements[1].integer.value = 0; + atif_arg_elements[0].integer.value = function; - status = acpi_evaluate_object(handle, "ATIF", &atif_arg, &buffer); + if (params) { + atif_arg.count = 2; + atif_arg_elements[1].type = ACPI_TYPE_BUFFER; + atif_arg_elements[1].buffer.length = params->length; + atif_arg_elements[1].buffer.pointer = params->pointer; + } - /* Fail only if calling the method fails and ATIF is supported */ - if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) { - DRM_DEBUG_DRIVER("failed to evaluate ATIF got %s\n", - acpi_format_exception(status)); - kfree(buffer.pointer); + status = acpi_evaluate_object(handle, "ATIF", &atif_arg, output); + if (ACPI_FAILURE(status)) return 1; + + obj = (union acpi_object *) output->pointer; + if (!obj) + return 1; + else if (obj->type != ACPI_TYPE_BUFFER) { + kfree(obj); + return 1; + } else if (obj->buffer.length != 256) { + DRM_DEBUG_DRIVER("Unknown ATIF output buffer length %d\n", obj->buffer.length); + kfree(obj); + return 1; + } + + return 0; +} + +static int radeon_atif_verify(acpi_handle handle) +{ + struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL }; + struct atif_verify_output *voutput; + union acpi_object *obj; + int ret; + + /* evaluate the ATIF verify function */ + ret = radeon_atif_call(handle, ATIF_FUNCTION_VERIFY_INTERFACE, NULL, &output); + if (ret) { + kfree(output.pointer); + return ret; + } + + obj = (union acpi_object *) output.pointer; + + voutput = (struct atif_verify_output *) obj->buffer.pointer; + verify_output = kzalloc(sizeof(struct atif_verify_output), GFP_KERNEL); /* TODO: kfree */ + verify_output->size = voutput->size; + verify_output->version = voutput->version; + verify_output->notif_mask = voutput->notif_mask; + verify_output->func_bits = voutput->func_bits; + + kfree(output.pointer); + return 0; +} + +static int radeon_acpi_video_event(struct notifier_block *nb, + unsigned long val, void *data) +{ + /* The only video events relevant to opregion are 0x81 or n. These indicate + either a docking event, lid switch or display switch request. In + Linux, these are handled by the dock, button and video drivers. + */ + + struct acpi_bus_event *event = data; + int ret = NOTIFY_BAD; /* Question: for fill acpi's blocking notifier chains */ + + if (strcmp(event->device_class, ACPI_VIDEO_CLASS) != 0) + return NOTIFY_DONE; + + if (event->type != notify_code) + return NOTIFY_DONE; + + /* TODO: run anything should take care by radeon */ + + return ret; +} + +static struct notifier_block radeon_acpi_notifier = { + .notifier_call = radeon_acpi_video_event, +}; + +static int radeon_atif_get_system_param(acpi_handle handle) +{ + struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL }; + struct atif_get_sysparams_output *params_output; + union acpi_object *obj; + u32 flags; + int ret; + + /* evaluate the ATIF get system parameters function */ + ret = radeon_atif_call(handle, ATIF_FUNCTION_GET_SYSTEM_PARAMETERS, NULL, &output); + if (ret) { + kfree(output.pointer); + return ret; } - kfree(buffer.pointer); + obj = (union acpi_object *) output.pointer; + + params_output = (struct atif_get_sysparams_output *) obj->buffer.pointer; + flags = params_output->flags; + + if (flags) { + if (flags == 1) + notify_code = 0x81; + else if (flags == 2) + notify_code = params_output->notify_code; + + register_acpi_notifier(&radeon_acpi_notifier); + } + + kfree(output.pointer); return 0; } @@ -59,11 +174,16 @@ int radeon_acpi_init(struct radeon_device *rdev) if (!ASIC_IS_AVIVO(rdev) || !rdev->bios || !handle) return 0; - /* Call the ATIF method */ - ret = radeon_atif_call(handle); + /* Call the ATIF verify function */ + ret = radeon_atif_verify(handle); if (ret) return ret; + if (verify_output->func_bits & ATIF_GET_SYSTEM_PARAMETERS_SUPPORTED) + ret = radeon_atif_get_system_param(handle); + if (ret) + return ret; + return 0; }