linux-kernel - Re: [PATCH] drm/i915: Fix invalid backpanel values for GEN3 or older chips

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 17 Nov 2011 18:14:20 +0100
From:	Daniel Mack <zonque@...il.com>
To:	Takashi Iwai <tiwai@...e.de>
Cc:	Keith Packard <keithp@...thp.com>,
	Chris Wilson <chris@...is-wilson.co.uk>,
	Jesse Barnes <jbarnes@...tuousgeek.org>, harald@...hat.com,
	intel-gfx@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drm/i915: Fix invalid backpanel values for GEN3 or older chips

On Thu, Nov 17, 2011 at 5:33 PM, Takashi Iwai <tiwai@...e.de> wrote:
> At Wed, 16 Nov 2011 22:15:41 -0800,
> Keith Packard wrote:
>>
>> On Wed, 16 Nov 2011 18:14:55 +0100, Takashi Iwai <tiwai@...e.de> wrote:
>>
>> > While refactoring of backlight control code in commit [a95735569:
>> > drm/i915: Refactor panel backlight controls], the handling of the bit
>> > 0 of duty-cycle was gone except for pineview.  This resulted in invalid
>> > register values for old chips like 915GM.  When the bit 0 is set, the
>> > backlight is turned off suddenly.
>>
>> I'm looking at the mentioned patch and I don't see how that managed to
>> correctly handle bit 0; is this the patch that managed to break this?
>
> Hrm, then I must have been confused.  The gen < 4 check (formerly
> !IS_I965G()) was firstly introduced in this refactoring.  I thought
> this was done before, but apparently not.
>
> Looking more carefully again, actually the IS_PINEVIEW() part (former
> IS_GND()) was introduced in the commit [078a033f: drm/i915: fix
> opregion backlight chip detect and range].  Thus the same bug must
> have been present even in the earlier version, but didn't come up by
> some reason.
>
> So, I'll have to correct the patch commit log.  Sorry for that.
>
> Now, a question is whether the condition (gen < 4) is really correct.
> I *guess* the original code in the refactoring was intended to cover
> it:
>        if (!IS_I965G(dev))
>                max &= ~1;
>
> But this rule wasn't applied to set_backlight().
> The bit-0-clear above implies that it's a similar behavior like
> pineview.  And indeed Daniel's machine with 915GM hits this problem.
> If the above rule should be applied to all places, it'd make more
> sense to handle both in the same way like pineview.
>
> Unfortunately I have no old machines to test this, and the only
> material to judge was Daniel's case.  (Is Harald's machine also
> 915GM?)

I can test more patches if you want me to, no problem. Just let me know.


Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/