Bug 45452

Summary:

Currently Active Display Devices List (CADL) is unimplemented, breaking brightness hotkeys

Product:

DRI

Reporter:

Peter Wu <peter>

Component:

DRM/Intel

Assignee:

Jesse Barnes <jbarnes>

Status:

CLOSED FIXED

QA Contact:

Severity:

normal

Priority:

medium

CC:

ben, chris, daniel, erappleman, florian, jbarnes, mavoga, peter, sonic_mu, xiong.y.zhang, ypwong

Version:

unspecified

Hardware:

Other

OS:

Linux (All)

Whiteboard:

i915 platform:

i915 features:

Attachments:

Description	Flags
acpidump of Clevo B7130	none
Clevo W150HRM acpidump	none
acpidump of Asus X401A	none
acpidump of Asus UX32VD	none

Description Peter Wu 2012-01-31 08:54:02 UTC

Created attachment 56399 [details]
acpidump of Clevo B7130

According to Section 3.1.6 on page 40 of the ACPI Integrated Graphics OpRegion Specification from http://intellinuxgraphics.org/ACPI_IGD_OpRegion_%20Spec.pdf, the driver is supposed to set the CADL field during boot and on every mode set process. An excerpt from the spec:

============================================================
Description
This field indicates which display devices (monitors) are currently
active. A maximum of eight monitors are assumed active on a given
platform. The IDs should be the same as the enumerated monitor or
connector IDs. The graphics driver determines active monitors
during mode set times and during boot.

Driver Access
Writes: The graphics driver writes to this field on every mode set
process and during boot.
Reads: The graphics driver can read this field to update NADL.
============================================================

The struct member "cadl" from drivers/gpu/drm/i915/intel_opregion.c is read/written nowhere. This has as implication that the field is left uninitialized and and code that relies on that field cannot work.

So far, the affected hardware I know of are the Clevo B7130 and Clevo H150HRM. The brightness hotkeys emit an ACPI event for the Embedded Controller that checks whether an monitor is connected or not. A part of method _Q11:
============================================================
If (LEqual (^^^GFX0.CDDS (0x0410), 0x1F)) {
    If (LEqual (OEM2, Zero)) {
        If (^^^^WMI.HKDR) {
            Store (0xE0, ^^^^WMI.EVNT)
            Notify (WMI, 0xD0)
        }
    } Else {
        Notify (^^^GFX0.LCD0, 0x87)
    }
}
============================================================

The CDDS functions checks the fields of CADL, CAL2, ..., CAL8 and returns 0x1F if an active display equals its first argument:
============================================================
Method (CDDS, 1, NotSerialized) {
    Store (And (Arg0, 0x0F0F), Local0)
    If (LEqual (Zero, Local0)) {
        Return (0x1D)
    }
    If (LEqual (And (CADL, 0x0F0F), Local0)) {
        Return (0x1F)
    }
============================================================

This bug also makes the return value of ACPI method _DCS useless.

Comment 1 Peter Wu 2012-01-31 09:01:15 UTC

Distribution: Kubuntu 11.10 AMD64
Kernel: 3.2.2 (from kernel.org)
Hardware: Intel i5 460M with HD graphics (+nvidia GT 425M, not important here)

When writing the output of the \_SB.PCI0.GFX0.LCD0._ADR value to the DIDL field, brightness keys works.

Comment 2 Peter Wu 2012-01-31 09:04:06 UTC

More machine information for an earlier bugreport related to brightness via hotkeys is available at: https://bugs.launchpad.net/linux/+bug/806032

Comment 3 Mau 2012-05-03 17:54:28 UTC

Created attachment 61006 [details]
Clevo W150HRM acpidump

Clevo W150HRM is equally affected: attaching acpidump (BIOS v1.09)

Comment 4 Peter Wu 2012-06-27 00:50:16 UTC

"Temporary" patch has been accepted, patch + acknowledgement:
http://lists.freedesktop.org/archives/dri-devel/2012-June/024488.html
http://lists.freedesktop.org/archives/dri-devel/2012-June/024529.html

Therefore I am marking this bug as resolved.

Comment 5 Daniel Vetter 2012-06-27 02:39:13 UTC

Merged to dinq as:

commit 3e52259264832dc8c8b94c91561f072ab1d192d8
Author: Lekensteyn <lekensteyn@gmail.com>
Date:   Tue Jun 26 00:36:24 2012 +0200

    i915: initialize CADL in opregion

Comment 6 Daniel Vetter 2012-06-29 07:50:41 UTC

Reopening because the patch had to be dropped - it caused regressions. The issues are now root-caused (it simply brought a pre-existing bug to light), but we first need to fix these bugs before I can merge the cadl patch again.

Please do not close this bug before the cadl patch has landed in drm-intel-next - I'll likely lose track of things otherwise ;-)

Comment 7 Peter Wu 2012-06-29 08:08:24 UTC

Okay, I'll let you handle the lock then :)
For the interested, a discussion is going on here: http://lists.freedesktop.org/archives/dri-devel/2012-June/024661.html

Have you already found out the root cause?

Comment 8 Daniel Vetter 2012-06-29 08:28:04 UTC

On Fri, Jun 29, 2012 at 5:08 PM,  <bugzilla-daemon@freedesktop.org> wrote:
> Have you already found out the root cause?

Yep, see Message-ID: <20120629144850.GB5463@phenom.ffwll.local> on
dri-devel and http://cgit.freedesktop.org/~danvet/drm/log/?h=cpu_edp-abomination
for the totally hackish patches ...

Comment 9 Anthony Wong 2012-07-17 15:09:03 UTC

Created attachment 64320 [details]
acpidump of Asus X401A

Hi Peter,

Your patch works great on the Asus X401A here, which also has the same problem, i.e. CADL is queried when brightness keys are pressed. Its CDDS method also looks almost the same as the one of Clevo B7130. I have uploaded the acpidump of the Asus X401A, so you can check if your patch can apply equally well to this machine. Thanks!

Comment 10 XiongZhang 2012-07-24 08:19:58 UTC

In the patch code, it treat CADL and DIDL as the same, but from spec, they are different: DIDL is Supported Display Devices ID List and CADL is Currently Attached Display Devices List.So only these IDs in DIDL with connect status can add to CADL, not all IDS in DIDL.

Comment 11 Daniel Vetter 2012-07-24 08:24:48 UTC

On Tue, Jul 24, 2012 at 10:19 AM,  <bugzilla-daemon@freedesktop.org> wrote:
> --- Comment #10 from XiongZhang <xiong.y.zhang@intel.com> 2012-07-24 08:19:58 UTC ---
> In the patch code, it treat CADL and DIDL as the same, but from spec, they are
> different: DIDL is Supported Display Devices ID List and CADL is Currently
> Attached Display Devices List.So only these IDs in DIDL with connect status can
> add to CADL, not all IDS in DIDL.

I know (I even suggested this approach), but this one here gets the
job done, so I plan to merge it until something more accurate shows
up. If something more accurate is required even.

Comment 12 Daniel Vetter 2012-08-22 10:34:31 UTC

*** Bug 52042 has been marked as a duplicate of this bug. ***

Comment 13 Anonymous Helper 2012-09-03 11:13:06 UTC

Created attachment 66537 [details]
acpidump of Asus UX32VD

Comment 14 Anonymous Helper 2012-09-03 11:14:54 UTC

Aahh, at last I found this entry. I was debugging the dsdt from my Asus UX32VD and I came to pretty much similar result. 
The DID2 field is never initialized, which lets CDDS(DID2) return 0x1d and makes _DCS useless. _DCS is used in _Q0E and _Q0F.
The lines in question: 
============================================================
Store (And (Arg0, 0x0F0F), Local0)
                    If (LEqual (Zero, Local0))
                    {
                        Return (0x1D)
                    } 
============================================================
used by Q0F:

============================================================
If (LNotEqual (^^^GFX0.LCDD._DCS (), 0x1F))
                    {       
                        Return (One)
                    }
============================================================

Since _DCS returns 0x1d, _Q0E and _Q0F returns silently one, instead of continuing until the Notify event is invoked.
Q0E/0F looks similar to your Q11, although _Q0E/F is a little bit more complicated, but the overall structure is the same.

Comment 15 Anonymous Helper 2012-09-03 12:17:12 UTC

Ooops, wasn't the did2 field. It was exactly the same as the others. Uninitialized cadl. Tried the patch. Works for the UX32VD.

Comment 16 Daniel Vetter 2012-09-13 21:58:11 UTC

Patch merged (again) to drm-intel-next-queued:

commit 036223401d50594c35fc5f7eafdbd2ddbe3ec921
Author: Lekensteyn <lekensteyn@gmail.com>
Date:   Tue Jun 26 00:36:24 2012 +0200

    i915: initialize CADL in opregion

Comment 17 Florian Mickler 2012-10-15 20:50:10 UTC

A patch referencing this bug report has been merged in Linux v3.7-rc1:

commit d627b62ff8d4d36761adbcd90ff143d79c94ab22
Author: Lekensteyn <lekensteyn@gmail.com>
Date:   Tue Jun 26 00:36:24 2012 +0200

    i915: initialize CADL in opregion

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.