Hello, On Fri, March 4, 2011 19:47, Linus Torvalds wrote: > Alex, can you confirm that the revert of 951f3512dba5 plus the > one-liner patch from Takashi that Indan quoted also works for you? > > Linus > > On Thu, Mar 3, 2011 at 10:53 PM, Indan Zupancic <indan@xxxxxx> wrote: >> >> So please revert my patch and apply Takashi Iwai's, which fixes the >> most immediate bug without changing anything else. This should go >> in stable too. > I found another backlight bug: When suspending intel_panel_disable_backlight() is never called, but intel_panel_enable_backlight() is called at resume. With the effect that if the brightness was ever changed after screen blanking, the wrong brightness gets restored. This explains the weird behaviour I've seen. I didn't see it with combination mode, because then the brightness is always the same (zero or the maximum, the BIOS only uses LBPC on my system.) I'll send a patch in a moment. Alternative for reverting the combination mode removal (I can also redo the patch against the revert and Takashi's patch, if that's preferred): -- drm/i915: Do handle backlight combination mode specially Add back the combination mode check, but with slightly cleaner code and the weirdness removed: No val >>= 1, but also no val &= ~1. The old code probably confused bit 0 with BLM_LEGACY_MODE, which is bit 16. The other change is clearer calculations: Just check for zero level explicitly instead of avoiding the divide-by-zero. Signed-off-by: Indan Zupancic <indan@xxxxxx> --- diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index d860abe..b05631a 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -30,6 +30,10 @@ #include "intel_drv.h" +#define PCI_LBPC 0xf4 /* legacy/combination backlight modes */ +#define BLM_COMBINATION_MODE (1 << 30) +#define BLM_LEGACY_MODE (1 << 16) + void intel_fixed_panel_mode(struct drm_display_mode *fixed_mode, struct drm_display_mode *adjusted_mode) @@ -110,6 +114,22 @@ done: dev_priv->pch_pf_size = (width << 16) | height; } +/* + * What about gen 3? If there are no gen 3 systems with ASLE, + * then it doesn't matter, as we don't need to change the + * brightness. But then the gen 2 check can be removed too. + */ +static int is_backlight_combination_mode(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + + if (INTEL_INFO(dev)->gen >= 4) + return I915_READ(BLC_PWM_CTL2) & BLM_COMBINATION_MODE; + if (IS_GEN2(dev)) + return I915_READ(BLC_PWM_CTL) & BLM_LEGACY_MODE; + return 0; +} + static u32 i915_read_blc_pwm_ctl(struct drm_i915_private *dev_priv) { u32 val; @@ -163,9 +183,12 @@ u32 intel_panel_get_max_backlight(struct drm_device *dev) max >>= 17; } else { max >>= 16; + /* Ignore BLM_LEGACY_MODE bit */ if (INTEL_INFO(dev)->gen < 4) max &= ~1; } + if (is_backlight_combination_mode(dev)) + max *= 0xff; } DRM_DEBUG_DRIVER("max backlight PWM = %d\n", max); @@ -183,6 +206,12 @@ u32 intel_panel_get_backlight(struct drm_device *dev) val = I915_READ(BLC_PWM_CTL) & BACKLIGHT_DUTY_CYCLE_MASK; if (IS_PINEVIEW(dev)) val >>= 1; + if (is_backlight_combination_mode(dev)){ + u8 lbpc; + + pci_read_config_byte(dev->pdev, PCI_LBPC, &lbpc); + val *= lbpc; + } } DRM_DEBUG_DRIVER("get backlight PWM = %d\n", val); @@ -205,6 +234,15 @@ void intel_panel_set_backlight(struct drm_device *dev, u32 level) if (HAS_PCH_SPLIT(dev)) return intel_pch_panel_set_backlight(dev, level); + + if (level && is_backlight_combination_mode(dev)){ + u32 max = intel_panel_get_max_backlight(dev); + u8 lpbc; + + lpbc = level * 0xff / max; + level /= lpbc; + pci_write_config_byte(dev->pdev, PCI_LBPC, lpbc); + } tmp = I915_READ(BLC_PWM_CTL); if (IS_PINEVIEW(dev)) { tmp &= ~(BACKLIGHT_DUTY_CYCLE_MASK - 1); _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel