From patchwork Mon Jul 30 20:24:49 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Luca Tettamanti X-Patchwork-Id: 1256541 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 0746BDFFBF for ; Mon, 30 Jul 2012 20:25:18 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id C2CC99F38D for ; Mon, 30 Jul 2012 13:25:17 -0700 (PDT) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-wi0-f177.google.com (mail-wi0-f177.google.com [209.85.212.177]) by gabe.freedesktop.org (Postfix) with ESMTP id 95C239EC76 for ; Mon, 30 Jul 2012 13:24:58 -0700 (PDT) Received: by wibhm11 with SMTP id hm11so1666191wib.12 for ; Mon, 30 Jul 2012 13:24:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=DUQ6YXh8MPUG5PXnN17fCK78bW4F0AETBhFgj2pzIdM=; b=z91XnGaLCk57OjDPHXNPBua9wOjafhNngT3ykJiYuyg8J/QjTpulXZrvr4a3+82SdA ZMPy6tVjd7z/AoEtKQu0s2LLAIkIFfnr4KGNspZIP5NhLlDzjyJFYmPSQ7tZu3OZVNKj dqs8sba4jaGXTCx1ZGJf+YJJbPxLZlJN9pmFYE1TXjAluFQi9OvXYuKcoXaG4RJDB2Hw ACyhVC+FddCJcSOPVvra4Ck9mtyGdihkC924Lb1Ty95TGdIOc1tEDCFxPY2JqC/u5x+K ixx0nnXjGo8lmtghYAFPidAftaAqBuHTI+2NcB1jxwU8WoqsiWkvAae70RYObFtyw/en r7jg== Received: by 10.180.91.228 with SMTP id ch4mr792057wib.7.1343679897753; Mon, 30 Jul 2012 13:24:57 -0700 (PDT) Received: from growl (dynamic-adsl-78-14-228-32.clienti.tiscali.it. [78.14.228.32]) by mx.google.com with ESMTPS id fr4sm26789610wib.8.2012.07.30.13.24.54 (version=TLSv1/SSLv3 cipher=OTHER); Mon, 30 Jul 2012 13:24:56 -0700 (PDT) Date: Mon, 30 Jul 2012 22:24:49 +0200 From: Luca Tettamanti To: Alex Deucher Subject: Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code Message-ID: <20120730202449.GA5600@growl> References: <1343237889-5220-1-git-send-email-alexdeucher@gmail.com> <20120726125838.GA28853@growl> <20120726193346.GA11288@growl> <20120728145626.GA6304@growl> <20120729130644.GA12378@growl> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Cc: Alex Deucher , joeyli , 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 On Mon, Jul 30, 2012 at 10:20:15AM -0400, Alex Deucher wrote: > On Sun, Jul 29, 2012 at 9:06 AM, Luca Tettamanti wrote: > > On Sat, Jul 28, 2012 at 05:29:25PM -0400, Alex Deucher wrote: > >> On Sat, Jul 28, 2012 at 10:56 AM, Luca Tettamanti wrote: > >> > 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)? > >> > >> The spec says that the bits in the supported functions vector mean > >> that if bit n is set, function n+1 exists, > > > > Hum, I don't follow. The vector in my case is 0x2 (1 << 1), that would > > mean that ATIF_SELECT_ACTIVE_DISPLAYS_SUPPORTED (1 << 2) is supported? > > > > Maybe if the bit n is set then functions 0..n are available? That would > > (almost) match what I see... > > From the spec: > > Supported DWORD Bit vector providing supported functions > information. Each bit marks > Functions Bit support for one specific function of the > ATIF method. Bit n, if set, > Vector indicates that Function n+1 is supported. Sorry, I still don't understand it... what's "Function n+1" in this context? Does this mean that if bit n is set then the function defined as 1 << (n+1) is supported? > I don't know how wonky bioses in the wild are however. I can see what > our internal teams do in these sort of cases. That would be helpful :) Note that on this machine (Toshiba L855-10W) brightness control under windows does not work with the stock catalyst driver, it works only with the (oldish) driver supplied by Toshiba. Anyway, I'm attaching v2 of my patches, I've incorporated the suggestions and some bits of code from joeyli, and now brightness control is actually implemented. Still missing is the issue of event 0x81 and the conflict with video.ko; the handler should probably return NOTIFY_BAD to veto the keypress. Luca From f0f8699eabee0d47b93fba14f8126b821cc106a5 Mon Sep 17 00:00:00 2001 From: Luca Tettamanti Date: Sun, 29 Jul 2012 17:04:43 +0200 Subject: [PATCH 1/4] drm/radeon: refactor radeon_atif_call Don't hard-code function number, this will allow to reuse the function. v2: add support for the 2nd parameter (from Lee, Chun-Yi ). Signed-off-by: Luca Tettamanti --- drivers/gpu/drm/radeon/radeon_acpi.c | 38 ++++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon_acpi.c b/drivers/gpu/drm/radeon/radeon_acpi.c index 5b32e49..158e514 100644 --- a/drivers/gpu/drm/radeon/radeon_acpi.c +++ b/drivers/gpu/drm/radeon/radeon_acpi.c @@ -14,23 +14,30 @@ #include /* Call the ATIF method - * - * Note: currently we discard the output */ -static int radeon_atif_call(acpi_handle handle) +static union acpi_object *radeon_atif_call(acpi_handle handle, int function, + struct acpi_buffer *params) { acpi_status status; union acpi_object atif_arg_elements[2]; struct acpi_object_list atif_arg; - struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL}; + struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL }; atif_arg.count = 2; 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; + + if (params) { + atif_arg_elements[1].type = ACPI_TYPE_BUFFER; + atif_arg_elements[1].buffer.length = params->length; + atif_arg_elements[1].buffer.pointer = params->pointer; + } else { + /* We need a second fake parameter */ + atif_arg_elements[1].type = ACPI_TYPE_INTEGER; + atif_arg_elements[1].integer.value = 0; + } status = acpi_evaluate_object(handle, "ATIF", &atif_arg, &buffer); @@ -39,18 +46,18 @@ static int radeon_atif_call(acpi_handle handle) DRM_DEBUG_DRIVER("failed to evaluate ATIF got %s\n", acpi_format_exception(status)); kfree(buffer.pointer); - return 1; + return NULL; } - kfree(buffer.pointer); - return 0; + return buffer.pointer; } /* Call all ACPI methods here */ int radeon_acpi_init(struct radeon_device *rdev) { acpi_handle handle; - int ret; + union acpi_object *info; + int ret = 0; /* Get the device handle */ handle = DEVICE_ACPI_HANDLE(&rdev->pdev->dev); @@ -60,10 +67,11 @@ int radeon_acpi_init(struct radeon_device *rdev) return 0; /* Call the ATIF method */ - ret = radeon_atif_call(handle); - if (ret) - return ret; + info = radeon_atif_call(handle, ATIF_FUNCTION_VERIFY_INTERFACE, NULL); + if (!info) + ret = -EIO; - return 0; + kfree(info); + return ret; }