lkml.org 
[lkml]   [2011]   [Nov]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] drm/i915: Fix invalid backpanel values for GEN3 or older chips
    At Fri, 18 Nov 2011 11:25:48 -0800,
    Keith Packard wrote:
    >
    > On Thu, 17 Nov 2011 17:33:50 +0100, Takashi Iwai <tiwai@suse.de> wrote:
    >
    > > 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...
    >
    > I read through the hardware docs yesterday and figured out what was
    > going on. On all pre-gen4 hardware, the maximum backlight value has the
    > lowest bit (of 16) hard-wired to zero. This means that the possible
    > backlight duty cycle values are limited to 0 .. 0xfffe.
    >
    > On older hardware, this was managed by reporting this true range. This
    > meant that the set_backlight path didn't need any special code; simply
    > setting the values as provided should have worked just fine.
    >
    > On Pineview, this was changed (for reasons unknown) to use only 15 bits
    > for backlight levels. The range of possible values is then
    > 0 .. 0x7fff. In this case, values have to be shifted by one to convert
    > between the advertised range of 0 .. 0x7fff and the hardware range of
    > 0 .. 0xfffe.
    >
    > Exposing the range of 0 .. 0xfffe introduces a potential problem --
    > write a value of 0xffff and the hardware gets mightily confused,
    > and probably ends up turning the backlight off. Using the range of
    > 0 .. 0x7fff avoids this issue completely.
    >
    > Using the narrower range does limit the precision of the backlight level
    > setting, but it seems like 32767 possible values is more than sufficient...
    >
    > Here's a patch which just uses the pineview version everywhere (and
    > cleans that up at the same time).

    Thanks! It's pretty similar with my patch in the end, so in case
    needed:
    Reviewed-by: Takashi Iwai <tiwai@suse.de>

    Maybe it'd be better to mention that actually setting bit-0 caused a
    blank screen on some machines.


    Takashi

    > From e06789f688dc7ab1331f5f15ad3d60326b5139b4 Mon Sep 17 00:00:00 2001
    > From: Keith Packard <keithp@keithp.com>
    > Date: Fri, 18 Nov 2011 11:09:24 -0800
    > Subject: [PATCH] drm/i915: Treat pre-gen4 backlight duty cycle value
    > consistently
    >
    > For i945 and earlier chips, the backlight frequency value had the low
    > bit (of 16) fixed to zero. The Pineview code path handled this by just
    > exposing the backlight range as 15 bits while other chips had the
    > backlight range limited to 0 .. 0xfffe.
    >
    > This patch makes everyone take the pineview code path, providing 15
    > bits of backlight duty cycle range which seems more than sufficient to me.
    >
    > Signed-off-by: Keith Packard <keithp@keithp.com>
    > ---
    > drivers/gpu/drm/i915/intel_panel.c | 16 +++++-----------
    > 1 files changed, 5 insertions(+), 11 deletions(-)
    >
    > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
    > index 21f60b7..04d79fd 100644
    > --- a/drivers/gpu/drm/i915/intel_panel.c
    > +++ b/drivers/gpu/drm/i915/intel_panel.c
    > @@ -178,13 +178,10 @@ u32 intel_panel_get_max_backlight(struct drm_device *dev)
    > if (HAS_PCH_SPLIT(dev)) {
    > max >>= 16;
    > } else {
    > - if (IS_PINEVIEW(dev)) {
    > + if (INTEL_INFO(dev)->gen < 4)
    > max >>= 17;
    > - } else {
    > + else
    > max >>= 16;
    > - if (INTEL_INFO(dev)->gen < 4)
    > - max &= ~1;
    > - }
    >
    > if (is_backlight_combination_mode(dev))
    > max *= 0xff;
    > @@ -203,13 +200,12 @@ u32 intel_panel_get_backlight(struct drm_device *dev)
    > val = I915_READ(BLC_PWM_CPU_CTL) & BACKLIGHT_DUTY_CYCLE_MASK;
    > } else {
    > val = I915_READ(BLC_PWM_CTL) & BACKLIGHT_DUTY_CYCLE_MASK;
    > - if (IS_PINEVIEW(dev))
    > + if (INTEL_INFO(dev)->gen < 4)
    > val >>= 1;
    >
    > if (is_backlight_combination_mode(dev)) {
    > u8 lbpc;
    >
    > - val &= ~1;
    > pci_read_config_byte(dev->pdev, PCI_LBPC, &lbpc);
    > val *= lbpc;
    > }
    > @@ -246,11 +242,9 @@ static void intel_panel_actually_set_backlight(struct drm_device *dev, u32 level
    > }
    >
    > tmp = I915_READ(BLC_PWM_CTL);
    > - if (IS_PINEVIEW(dev)) {
    > - tmp &= ~(BACKLIGHT_DUTY_CYCLE_MASK - 1);
    > + if (INTEL_INFO(dev)->gen < 4)
    > level <<= 1;
    > - } else
    > - tmp &= ~BACKLIGHT_DUTY_CYCLE_MASK;
    > + tmp &= ~BACKLIGHT_DUTY_CYCLE_MASK;
    > I915_WRITE(BLC_PWM_CTL, tmp | level);
    > }
    >
    > --
    > 1.7.7.3
    >
    >
    >
    > --
    > keith.packard@intel.com


    \
     
     \ /
      Last update: 2011-11-19 11:07    [W:0.026 / U:29.920 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site