diff mbox

drm/radeon: add new AMD ACPI header and update relevant code

Message ID CADnq5_Nr8S+kzdKVq9=VL9BVK+4eFeX0egs-rkiv882MAm3bow@mail.gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Alex Deucher July 26, 2012, 3:35 p.m. UTC 
On Thu, Jul 26, 2012 at 8:58 AM, Luca Tettamanti <kronos.it@gmail.com> wrote:
> On Wed, Jul 25, 2012 at 01:38:09PM -0400, alexdeucher@gmail.com wrote:
>> From: Alex Deucher <alexander.deucher@amd.com>
>>
>> Add a new header that defines the AMD ACPI interface used
>> for laptops, PowerXpress, and chipset specific functionality
>> and update the current code to use it.
>
> Great! Now my DSDT makes sense ;)
>
>> Todo:
>> - properly verify the ACPI interfaces
>> - hook up and handle ACPI notifications
>
> I see a problem here, I've hacked up a standalone test module, but:
>
>> +#define ATIF_FUNCTION_GET_SYSTEM_PARAMETERS                        0x1
> [...]
>> + * flags
>> + * bits 1:0:
>> + * 0 - Notify(VGA, 0x81) is not used for notification
>> + * 1 - Notify(VGA, 0x81) is used for notification
>
> My system has this bit set, and the brightness control method does send
> the notification.
> My module register itself with register_acpi_notifier and gets 2
> notifications, one with ACPI_VIDEO_NOTIFY_PROBE (0x81) and the other
> with ACPI_VIDEO_NOTIFY_INC_BRIGHTNESS (or DEC, depending on what I
> press).
>
> The standard acpi "video" module, however, in response to
> ACPI_VIDEO_NOTIFY_PROBE generates a key press sending
> KEY_SWITCHVIDEOMODE.
>
> This greatly confuses KDE which messes up my dual screen configuration;
> I guess that the spurious KEY_SWITCHVIDEOMODE may be problematic also
> with other DEs...
>
> In more detail what happens is the following:
> - I press the brightness hotkey, firmware generates a notification on
>   the relevant device (_SB.PCI0.PEG0.VGA.LCD)
> - ACPI video module gets the ACPI_VIDEO_NOTIFY_{DEC,INC}_BRIGHTNESS
>   notification and tries to adjust the brightness with
>   acpi_video_device_lcd_set_level, which in turns calls _BCM
> - _BCM sets the relevant bits in the AMD-specific structure and does a
>   Notify (VGA, 0x81)
> - again ACPI video module gets the nodification (in this case
>   ACPI_VIDEO_NOTIFY_PROBE), re-enumerated and send KEY_SWITCHVIDEOMODE
> - KDE seems this and muck with the screen configuration :(
> - meanwhile the brightness notification is propagated, the hypothetical
>   radeon driver does its magic to adjust the screen.
>
> My first idea would be to make ACPI_VIDEO_NOTIFY_PROBE also call to the
> acpi notifier chain, and allow the handlers to veto the key press (like
> it's done for ACPI_VIDEO_NOTIFY_SWITCH).
>
> Zhang Rui what do you think about this?
>
> The other missing bit is how to actually change the brightness... Alex,
> do you know what registers to poke?

You need to check if the GPU controls the backlight or the system
does.  I think the attached patches should point you in the right
direction.

Alex

>
> My card is a:
>
> 01:00.0 VGA compatible controller [0300]: Advanced Micro Devices [AMD] nee ATI Thames XT/GL [Radeon HD 600M Series] [1002:6840]
>
> on a Toshiba L855.
>
> Luca

Comments

Luca Tettamanti July 26, 2012, 7:33 p.m. UTC | #1 
On Thu, Jul 26, 2012 at 11:35:25AM -0400, Alex Deucher wrote:
> On Thu, Jul 26, 2012 at 8:58 AM, Luca Tettamanti <kronos.it@gmail.com> wrote:
> > The other missing bit is how to actually change the brightness... Alex,
> > do you know what registers to poke?
> 
> You need to check if the GPU controls the backlight or the system
> does.  I think the attached patches should point you in the right
> direction.

Yep :)

0050:  ATOM_FIRMWARE_CAPABILITY_ACCESS usFirmwareCapability :
  0050:  (union) ATOM_FIRMWARE_CAPABILITY sbfAccess          :
    USHORT GPUControlsBL:1                              = 0x0001     (1)

The panel is using the INTERNAL_UNIPHY encoder, and I see the
UNIPHYTransmitterControl command table.

Interaction with video.ko is still a bit messy...

Do you already have code for handling the notifications? I'll work on it
in the weekend otherwise ;)

Luca
--
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
Alex Deucher July 26, 2012, 7:42 p.m. UTC | #2 
On Thu, Jul 26, 2012 at 3:33 PM, Luca Tettamanti <kronos.it@gmail.com> wrote:
> On Thu, Jul 26, 2012 at 11:35:25AM -0400, Alex Deucher wrote:
>> On Thu, Jul 26, 2012 at 8:58 AM, Luca Tettamanti <kronos.it@gmail.com> wrote:
>> > The other missing bit is how to actually change the brightness... Alex,
>> > do you know what registers to poke?
>>
>> You need to check if the GPU controls the backlight or the system
>> does.  I think the attached patches should point you in the right
>> direction.
>
> Yep :)
>
> 0050:  ATOM_FIRMWARE_CAPABILITY_ACCESS usFirmwareCapability :
>   0050:  (union) ATOM_FIRMWARE_CAPABILITY sbfAccess          :
>     USHORT GPUControlsBL:1                              = 0x0001     (1)
>
> The panel is using the INTERNAL_UNIPHY encoder, and I see the
> UNIPHYTransmitterControl command table.
>
> Interaction with video.ko is still a bit messy...
>
> Do you already have code for handling the notifications? I'll work on it
> in the weekend otherwise ;)

I don't have patches for that.  Please feel free to work on it :)

Thanks,

Alex
--
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
Alex Deucher July 26, 2012, 7:58 p.m. UTC | #3 
On Thu, Jul 26, 2012 at 3:42 PM, Alex Deucher <alexdeucher@gmail.com> wrote:
> On Thu, Jul 26, 2012 at 3:33 PM, Luca Tettamanti <kronos.it@gmail.com> wrote:
>> On Thu, Jul 26, 2012 at 11:35:25AM -0400, Alex Deucher wrote:
>>> On Thu, Jul 26, 2012 at 8:58 AM, Luca Tettamanti <kronos.it@gmail.com> wrote:
>>> > The other missing bit is how to actually change the brightness... Alex,
>>> > do you know what registers to poke?
>>>
>>> You need to check if the GPU controls the backlight or the system
>>> does.  I think the attached patches should point you in the right
>>> direction.
>>
>> Yep :)
>>
>> 0050:  ATOM_FIRMWARE_CAPABILITY_ACCESS usFirmwareCapability :
>>   0050:  (union) ATOM_FIRMWARE_CAPABILITY sbfAccess          :
>>     USHORT GPUControlsBL:1                              = 0x0001     (1)
>>
>> The panel is using the INTERNAL_UNIPHY encoder, and I see the
>> UNIPHYTransmitterControl command table.
>>
>> Interaction with video.ko is still a bit messy...
>>
>> Do you already have code for handling the notifications? I'll work on it
>> in the weekend otherwise ;)
>
> I don't have patches for that.  Please feel free to work on it :)

One thing worth checking, the sbios may write the requested backlight
level to the bios scratch reg, in which case the driver only has to
execute the BL_BRIGHTNESS action rather than writing the requested
level to the register first.

Alex

>
> Thanks,
>
> Alex
--
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
Pali Rohár July 28, 2012, 2:39 p.m. UTC | #4 
Alex Deucher <alexdeucher <at> gmail.com> writes:
> 
> On Thu, Jul 26, 2012 at 8:58 AM, Luca Tettamanti wrote:
> > The other missing bit is how to actually change the brightness... Alex,
> > do you know what registers to poke?
> 
> You need to check if the GPU controls the backlight or the system
> does.  I think the attached patches should point you in the right
> direction.
> 
> Alex
> 

Hi, thank you very much for patches. With these patches now I'm able to adjust
brightness on my notebook HP EliteBook 8460p with Radeon 6470M card via file
/sys/class/backlight/radeon_bl/brightness

It has only one problem: When I enable Ambient Ligth Sensor via hp-wmi.ko
driver (echo 1 > /sys/devices/platform/hp-wmi/als which auto adjust some
brightness level based on sensor data on HW level) and I set radeon brightness
to 0 or 255 (min or max value) screen is turned off. Values 1..254 are OK.

What about to disable values 0 and 255 in radeon brightness driver?

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

Patch

From a56ac560e34032ca803e0909c8a92a9037046790 Mon Sep 17 00:00:00 2001
From: Alex Deucher <alexander.deucher@amd.com>
Date: Thu, 26 Jul 2012 09:50:57 -0400
Subject: [PATCH 1/3] drm/radeon: track whether the GPU controls the backlight

A table in the vbios tells us whether the GPU backlight controller
is used or not.  If the bit is set, the GPU backlight controller is
used; if it is not set, an off-chip backlight controller is used.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/radeon/radeon_atombios.c |    4 ++++
 drivers/gpu/drm/radeon/radeon_mode.h     |    2 ++
 2 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_atombios.c b/drivers/gpu/drm/radeon/radeon_atombios.c
index b1e3820..92267a8 100644
--- a/drivers/gpu/drm/radeon/radeon_atombios.c
+++ b/drivers/gpu/drm/radeon/radeon_atombios.c
@@ -1254,6 +1254,10 @@  bool radeon_atom_get_clock_info(struct drm_device *dev)
 		if (rdev->clock.max_pixel_clock == 0)
 			rdev->clock.max_pixel_clock = 40000;
 
+		/* not technically a clock, but... */
+		if (firmware_info->info.usFirmwareCapability.sbfAccess.GPUControlsBL)
+			rdev->mode_info.gpu_controls_bl = true;
+
 		return true;
 	}
 
diff --git a/drivers/gpu/drm/radeon/radeon_mode.h b/drivers/gpu/drm/radeon/radeon_mode.h
index f380d59..33cbf97 100644
--- a/drivers/gpu/drm/radeon/radeon_mode.h
+++ b/drivers/gpu/drm/radeon/radeon_mode.h
@@ -252,6 +252,8 @@  struct radeon_mode_info {
 
 	/* pointer to fbdev info structure */
 	struct radeon_fbdev *rfbdev;
+	/* GPU controls backlight */
+	bool gpu_controls_bl;
 };
 
 #define MAX_H_CODE_TIMING_LEN 32
-- 
1.7.7.5