Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code

From: Alex Deucher <alexdeucher@gmail.com>
To: Luca Tettamanti <kronos.it@gmail.com>
Cc: Alex Deucher <alexander.deucher@amd.com>, joeyli <jlee@suse.com>,
	dri-devel@lists.freedesktop.org
Subject: Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code
Date: Mon, 30 Jul 2012 16:45:49 -0400	[thread overview]
Message-ID: <CADnq5_NP9Rw8DhmS+Q5G1PaUqcHQtVA6KoCGH5MME6DEd-Lw7Q@mail.gmail.com> (raw)
In-Reply-To: <20120730202449.GA5600@growl>

On Mon, Jul 30, 2012 at 4:24 PM, Luca Tettamanti <kronos.it@gmail.com> wrote:
> On Mon, Jul 30, 2012 at 10:20:15AM -0400, Alex Deucher wrote:
>> On Sun, Jul 29, 2012 at 9:06 AM, Luca Tettamanti <kronos.it@gmail.com> 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 <kronos.it@gmail.com> 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.

Regarding patches 3 and 4, it might be easier to just store a pointer
to the relevant encoder when you verify ATIF rather than walking the
encoder list every time.

Alex

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