[lkml]   [2011]   [Nov]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH] drm/i915: Fix invalid backpanel values for GEN3 or older chips
    At Wed, 16 Nov 2011 22:15:41 -0800,
    Keith Packard wrote:
    > On Wed, 16 Nov 2011 18:14:55 +0100, Takashi Iwai <> 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
    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

    If it's only with 915GM, we'll just need to change IS_PINEVEW() to
    IS_PINEVIEW() || IS_I915GM(). This might be a safer option at this
    moment unless we check all cases or specs...



     \ /
      Last update: 2011-11-17 17:35    [W:0.040 / U:105.468 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site