[RESEND][Quantal] [Precise SRU] drm/radeon: fix unintended display toggles

[Alex Hung]

On 02/25/2013 10:31 PM, Tim Gardner wrote:
> On 02/25/2013 04:00 AM, Alex Hung wrote:
>> SRU Justification:
>>
>> Impact:
>>    An additional KEY_SWITCHVIDEOMODE keycode is sent with when
>>    brightness up/down hotkey are pressed on some systems with AMD's
>>    VGA chips. It toggles displays unintendedly when multiple monitors
>>    are connected or causes LCD panel flickers with no external monitors.
>>
>> Fix:
>>    The standard video events may be overloaded for device specific
>>    purposes. For example AMD ACPI interface overloads
>>    ACPI_VIDEO_NOTIFY_PROBE (0x81) to signal AMD-specific events.
>>    This patch gives the handlers the chance to examine the event and
>>    block the keypress if the event is device specific.
>>
>>    The patches are included in upstream and are cherry-picked from Linus's repo.
>>
>> Test:
>>    I tested on ASUS 1015U, and it removed the additional keycode.
>>
>> Note:
>>
>> The following changes since commit 965b38cd16f964011172991c2e67b76562828b74:
>>
>>    UBUNTU: Ubuntu-lts-3.5.0-25.38
>>
>> are available in the git repository at:
>>    git://kernel.ubuntu.com/alexhung/ubuntu-precise.git quantal-ati
>>
>> for you to fetch upto bf241ddc8f9ee72fda14fe053e800b10e6c24b3c including the following patches:
>>
>>    drm/radeon: block the keypress on ATIF events
>>    ACPI video: allow events handlers to veto the keypress
>>    drm/radeon: re-organize the acpi notifier callback
>>    drm/radeon: implement handler for ACPI event
>>    drm/radeon: implement wrapper for GET_SYSTEM_PARAMS
>>    drm/radeon: implement radeon_atif_verify_interface
>>    drm/radeon: refactor radeon_atif_call
>>    drm/radeon: add backlight control for atom devices (v2)
>>    drm/radeon: rework legacy backlight control
>>    drm/radeon: track whether the GPU controls the backlight (v2)
>>    drm/radeon: add a license header to radeon_apci.c
>>    drm/radeon: add new AMD ACPI header and update relevant code (v2)
>>
>> Please note drm/radeon: implement radeon_atif_verify_interface (5213fc0f09) has resolved conflicts.
>>
>> BugLink: http://bugs.launchpad.net/bugs/1110219
>>
>
> Aside from the one Radeon based laptop that you've tried, what other
> regression testing has been done ? This is a giant patch set for what
> should be a relatively stable kernel. Is this truly the smallest patch
> you could come up with ?
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1110219/comments/4
> implies that perhaps you might be able to get by with a smaller backport.
>
> rtg
>

Hi,

The patches are like stacks and the later ones need the previous ones. 
Basically, the patches includes
1. Brightness functions,
2. ATIF (AMD's ACPI interfaces) that uses the new brightness functions,
3. The two patches that ignore 0x81 events in video module as ATIF 
(Radeon) handles it.

As a result, it is not easy to separate them nicely (except "add a 
license header to radeon_apci.c", but it is license head only and it 
won't affect anything functions stability).

Fortunately, the patches are not as big as they seem:
 >>    drm/radeon: add new AMD ACPI header and update relevant code (v2)
Add a header file + redefinition of strings (no function change)
 >>    drm/radeon: add a license header to radeon_apci.c
Add license header only (no function change)
 >>    drm/radeon: re-organize the acpi notifier callback
Refactor source code (no function change)
 >>    drm/radeon: track whether the GPU controls the backlight (v2)
Only set a flag used in "rework legacy backlight control" below

The real functions are here:
 >>    drm/radeon: add backlight control for atom devices (v2)
 >>    drm/radeon: rework legacy backlight control
Change brightness control function

 >>    drm/radeon: implement handler for ACPI event
 >>    drm/radeon: implement wrapper for GET_SYSTEM_PARAMS
 >>    drm/radeon: implement radeon_atif_verify_interface
 >>    drm/radeon: refactor radeon_atif_call
Implement ATIF functions, i.e. determines what events are from BIOS.

 >>    drm/radeon: block the keypress on ATIF events
 >>    ACPI video: allow events handlers to veto the keypress
Ignores 0x81 events

Specially, brightness control and ATIF are only triggered when BIOS 
issues ACPI events and only when ATIF is presents (it won't affect 
add-on Radeon cards to desktops).

I found four other Radeon laptops to run a number of tests as below:

OS: Ubuntu 12.04.02 (3.5.0-23) vs. updated kernel (3.5.0-25)
Tests:
	Brightness hotkey
	Display switching
	glmark2 (3D) x 5 cyclesonly
	Idle test
All machines have the same behaviors with and without the patches (all 
pass except one that always fails brightness hotkeys).

None of the machines I found work with S3 with 12.04.02, either 
with/without the patches.

In summary, I haven't found the patches break any other functions, and 
it only fixes the intended one.

List of machines I tested in addition to ASUS 1015U:
Dell Latitude 3330
HP Pavilion g6 Notebook PC
HP Pavilion Sleekbook 14 PC
HP Pavilion g4 Notebook PC

If any more tests are recommended, I am more than happy to verify them.

Cheers,
Alex Hung