From: Chris Wilson <chris@chris-wilson.co.uk>
To: Aaron Lu <aaron.lu@intel.com>
Cc: Matthew Garrett <mjg59@srcf.ucam.org>,
Daniel Vetter <daniel.vetter@ffwll.ch>,
"intel-gfx@lists.freedesktop.org"
<intel-gfx@lists.freedesktop.org>,
"Rafael J. Wysocki" <rjw@rjwysocki.net>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Oleksij Rempel <linux@rempel-privat.de>,
ACPI Devel Mailing List <linux-acpi@vger.kernel.org>
Subject: Re: [PATCH] drm/i915/opregion: work around buggy firmware that provides 8+ output devices
Date: Thu, 13 Feb 2014 10:08:14 +0000 [thread overview]
Message-ID: <20140213100814.GM14909@nuc-i3427.alporthouse.com> (raw)
In-Reply-To: <52FC8C01.1040002@intel.com>
On Thu, Feb 13, 2014 at 05:10:25PM +0800, Aaron Lu wrote:
> On 02/12/2014 06:31 PM, Chris Wilson wrote:
> > On Wed, Feb 12, 2014 at 11:05:40AM +0800, Aaron Lu wrote:
> >> The ACPI table on ASUS UX302LA has more than 8 output devices under the
> >> graphics controller device node. The problem is, the real active output
> >> device, the LCD panel, is listed the last. The result is, the LCD's
> >> device id doesn't get recorded in the active device list CADL array and
> >> when the _DCS control method for the LCD device is executed, it returns
> >> 0x1d, meaning it is not active. This affects the hotkey delivery ASL
> >> code that will not deliver a notification if the output device is not
> >> active on backlight hotkey press.
> >>
> >> I don't see a clean way to solve this problem since the operation region
> >> spec doesn't allow more than 8 output devices so we have no way of
> >> storing all these output devices. The fact that output devices that have
> >> _BCM control method usually means they have a higher possibility of being
> >> used than those who don't made me choose a simple way to work around
> >> the buggy firmware by replacing the last entry in CADL array with the one
> >> that has _BCM control method. There is no specific reason why the last
> >> entry is picked instead of others.
> >
> > Another possibility is that the connector list is in rough priority
> > order so might be useful for sorting the CADL array.
> >
> > Since the CADL should only be a list of currently active devices, we
> > could just bite the bullet and repopulate it correctly after every
> > setcrtc.
>
> Thanks for the suggestion. As a first step, does the following un-tested
> patch look OK?
Yes. Maybe worth putting together the similar routines for blind
setting the didl and the cadl, or at least for computing the value from
the connector. For instance, the didl logic disagrees with the value of
index - is that relevant? I have a suspicion that the CADL entry should
match the DIDL entry for the connector, but that is not actually
mentioned in the opregion spec afaict.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
WARNING: multiple messages have this Message-ID (diff)
From: Chris Wilson <chris@chris-wilson.co.uk>
To: Aaron Lu <aaron.lu@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>,
Matthew Garrett <mjg59@srcf.ucam.org>,
Jani Nikula <jani.nikula@linux.intel.com>,
"Rafael J. Wysocki" <rjw@rjwysocki.net>,
Oleksij Rempel <linux@rempel-privat.de>,
"intel-gfx@lists.freedesktop.org"
<intel-gfx@lists.freedesktop.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
ACPI Devel Mailing List <linux-acpi@vger.kernel.org>
Subject: Re: [PATCH] drm/i915/opregion: work around buggy firmware that provides 8+ output devices
Date: Thu, 13 Feb 2014 10:08:14 +0000 [thread overview]
Message-ID: <20140213100814.GM14909@nuc-i3427.alporthouse.com> (raw)
In-Reply-To: <52FC8C01.1040002@intel.com>
On Thu, Feb 13, 2014 at 05:10:25PM +0800, Aaron Lu wrote:
> On 02/12/2014 06:31 PM, Chris Wilson wrote:
> > On Wed, Feb 12, 2014 at 11:05:40AM +0800, Aaron Lu wrote:
> >> The ACPI table on ASUS UX302LA has more than 8 output devices under the
> >> graphics controller device node. The problem is, the real active output
> >> device, the LCD panel, is listed the last. The result is, the LCD's
> >> device id doesn't get recorded in the active device list CADL array and
> >> when the _DCS control method for the LCD device is executed, it returns
> >> 0x1d, meaning it is not active. This affects the hotkey delivery ASL
> >> code that will not deliver a notification if the output device is not
> >> active on backlight hotkey press.
> >>
> >> I don't see a clean way to solve this problem since the operation region
> >> spec doesn't allow more than 8 output devices so we have no way of
> >> storing all these output devices. The fact that output devices that have
> >> _BCM control method usually means they have a higher possibility of being
> >> used than those who don't made me choose a simple way to work around
> >> the buggy firmware by replacing the last entry in CADL array with the one
> >> that has _BCM control method. There is no specific reason why the last
> >> entry is picked instead of others.
> >
> > Another possibility is that the connector list is in rough priority
> > order so might be useful for sorting the CADL array.
> >
> > Since the CADL should only be a list of currently active devices, we
> > could just bite the bullet and repopulate it correctly after every
> > setcrtc.
>
> Thanks for the suggestion. As a first step, does the following un-tested
> patch look OK?
Yes. Maybe worth putting together the similar routines for blind
setting the didl and the cadl, or at least for computing the value from
the connector. For instance, the didl logic disagrees with the value of
index - is that relevant? I have a suspicion that the CADL entry should
match the DIDL entry for the connector, but that is not actually
mentioned in the opregion spec afaict.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
next prev parent reply other threads:[~2014-02-13 10:08 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-02-12 3:05 [PATCH] drm/i915/opregion: work around buggy firmware that provides 8+ output devices Aaron Lu
2014-02-12 10:31 ` Chris Wilson
2014-02-12 10:31 ` Chris Wilson
2014-02-12 10:52 ` Jani Nikula
2014-02-12 10:52 ` Jani Nikula
2014-02-13 9:10 ` Aaron Lu
2014-02-13 9:10 ` Aaron Lu
2014-02-13 10:08 ` Chris Wilson [this message]
2014-02-13 10:08 ` Chris Wilson
2014-02-13 12:03 ` Daniel Vetter
2014-02-19 7:31 ` Aaron Lu
2014-02-19 7:33 ` Matthew Garrett
2014-02-19 8:59 ` Aaron Lu
2014-02-19 8:59 ` Aaron Lu
2014-03-04 14:45 ` Daniel Vetter
2014-12-08 1:58 ` Aaron Lu
2014-12-08 1:58 ` Aaron Lu
2014-12-08 11:00 ` Jani Nikula
2014-12-08 11:00 ` Jani Nikula
2014-12-08 11:04 ` Jani Nikula
2014-12-08 11:04 ` [Intel-gfx] " Jani Nikula
2014-12-09 9:15 ` Aaron Lu
2014-12-09 9:15 ` [Intel-gfx] " Aaron Lu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140213100814.GM14909@nuc-i3427.alporthouse.com \
--to=chris@chris-wilson.co.uk \
--cc=aaron.lu@intel.com \
--cc=daniel.vetter@ffwll.ch \
--cc=intel-gfx@lists.freedesktop.org \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@rempel-privat.de \
--cc=mjg59@srcf.ucam.org \
--cc=rjw@rjwysocki.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.