On Wed, Nov 13, 2013 at 10:27:38AM +0200, Jani Nikula wrote:
> On Wed, 13 Nov 2013, Imre Deak <imre.d...@intel.com> wrote:
> > On Fri, 2013-11-08 at 16:48 +0200, Jani Nikula wrote:
> >> Signed-off-by: Jani Nikula <jani.nik...@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/intel_panel.c |   10 +++++++---
> >>  1 file changed, 7 insertions(+), 3 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/i915/intel_panel.c 
> >> b/drivers/gpu/drm/i915/intel_panel.c
> >> index a821949..e82b2dd 100644
> >> --- a/drivers/gpu/drm/i915/intel_panel.c
> >> +++ b/drivers/gpu/drm/i915/intel_panel.c
> >> @@ -555,7 +555,7 @@ static void i9xx_set_backlight(struct intel_connector 
> >> *connector, u32 level)
> >>  {
> >>    struct drm_device *dev = connector->base.dev;
> >>    struct drm_i915_private *dev_priv = dev->dev_private;
> >> -  u32 tmp;
> >> +  u32 tmp, mask;
> >>  
> >>    if (is_backlight_combination_mode(dev)) {
> >>            u32 max = intel_panel_get_max_backlight(connector);
> >> @@ -570,10 +570,14 @@ static void i9xx_set_backlight(struct 
> >> intel_connector *connector, u32 level)
> >>            pci_write_config_byte(dev->pdev, PCI_LBPC, lbpc);
> >>    }
> >>  
> >> -  if (INTEL_INFO(dev)->gen < 4)
> >> +  if (IS_GEN4(dev)) {
> >> +          mask = BACKLIGHT_DUTY_CYCLE_MASK;
> >> +  } else {
> >>            level <<= 1;
> >> +          mask = BACKLIGHT_DUTY_CYCLE_MASK_PNV;
> >> +  }
> >
> > According to the gen2/3 bspec I have, the correct mask is
> > BACKLIGHT_DUTY_CYCLE_MASK_PNV only in case of IS_PINEVIEW(dev), for
> > everything else it's BACKLIGHT_DUTY_CYCLE_MASK.
> 
> What you say is correct, but we've treated all gen2/3 similar to PNV
> since
> 
> commit ca88479c1c3b7b1a9f94320745f5331e1de77f80
> Author: Keith Packard <kei...@keithp.com>
> Date:   Fri Nov 18 11:09:24 2011 -0800
> 
>     drm/i915: Treat pre-gen4 backlight duty cycle value consistently
> 
> i.e. we only use the high 15 bits for all gen2/3. For non-PNV this just
> means the lowest bit is always zero. For PNV the lowest bit has a
> different meaning in both the PWM freq and duty cycle fields.
> 
> I don't want to take any chances by changing this behaviour. I realize
> there's zero comments about this in the code; would you like me to add
> some?

Yeah, I think some comment here would be good. Or maybe a follow-up patch
to differentiate between pnv and everything else on gen2/3.
-Daniel

> 
> 
> BR,
> Jani.
> 
> 
> >
> > --Imre
> >
> >>  
> >> -  tmp = I915_READ(BLC_PWM_CTL) & ~BACKLIGHT_DUTY_CYCLE_MASK;
> >> +  tmp = I915_READ(BLC_PWM_CTL) & ~mask;
> >>    I915_WRITE(BLC_PWM_CTL, tmp | level);
> >>  }
> >>  
> >
> >
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to