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

[Daniel Mack]

On Thu, Nov 17, 2011 at 5:33 PM, Takashi Iwai <tiwai@xxxxxxx> 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@xxxxxxx> 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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/