From: "Indan Zupancic" <indan@nul.nu> To: "Linus Torvalds" <torvalds@linux-foundation.org> Cc: "Alex Riesen" <raa.lkml@gmail.com>, "Jesse Barnes" <jbarnes@virtuousgeek.org>, "DRI mailing list" <dri-devel@lists.freedesktop.org>, "Chris Wilson" <chris@chris-wilson.co.uk>, "Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>, "Tino Keitel" <tino.keitel@tikei.de>, stable@kernel.org, indan@nul.nu, "Takashi Iwai" <tiwai@suse.de> Subject: Re: [PATCH] fix backlight brightness on intel LVDS panel after reopening lid Date: Thu, 10 Mar 2011 06:50:09 +0100 (CET) [thread overview] Message-ID: <1a06e2711df021a802d609ad1a75db17.squirrel@webmail.greenhost.nl> (raw) In-Reply-To: <AANLkTimyQ30ELw9Sm57NXgetiXnzJMHby2BE3GsynJLs@mail.gmail.com> 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@nul.nu> 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@nul.nu> --- 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);
WARNING: multiple messages have this Message-ID (diff)
From: "Indan Zupancic" <indan@nul.nu> To: Linus Torvalds <torvalds@linux-foundation.org> Cc: Takashi Iwai <tiwai@suse.de>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, DRI mailing list <dri-devel@lists.freedesktop.org>, Alex Riesen <raa.lkml@gmail.com>, Tino Keitel <tino.keitel@tikei.de>, indan@nul.nu, stable@kernel.org Subject: Re: [PATCH] fix backlight brightness on intel LVDS panel after reopening lid Date: Thu, 10 Mar 2011 06:50:09 +0100 (CET) [thread overview] Message-ID: <1a06e2711df021a802d609ad1a75db17.squirrel@webmail.greenhost.nl> (raw) In-Reply-To: <AANLkTimyQ30ELw9Sm57NXgetiXnzJMHby2BE3GsynJLs@mail.gmail.com> 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@nul.nu> 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@nul.nu> --- 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);
next prev parent reply other threads:[~2011-03-10 5:50 UTC|newest] Thread overview: 65+ messages / expand[flat|nested] mbox.gz Atom feed top 2011-02-16 4:16 Linux 2.6.38-rc5 Linus Torvalds 2011-02-16 11:14 ` Eric Dumazet 2011-02-16 13:55 ` Eric Dumazet 2011-02-16 15:46 ` Linus Torvalds 2011-02-16 16:06 ` Al Viro 2011-02-16 16:19 ` Al Viro 2011-02-16 16:33 ` Linus Torvalds 2011-02-16 16:39 ` Al Viro 2011-02-16 16:47 ` Eric Dumazet 2011-02-16 16:22 ` Eric Dumazet 2011-02-16 19:26 ` [PATCH] fix backlight brightness on intel LVDS panel after reopening lid Alex Riesen 2011-02-16 19:46 ` Alex Riesen 2011-02-16 19:54 ` Jesse Barnes 2011-02-16 19:59 ` Alex Riesen 2011-02-16 20:05 ` Jesse Barnes 2011-02-16 20:28 ` Alex Riesen 2011-02-17 1:41 ` [PATCH] drm/i915: Do not handle backlight combination mode specially Indan Zupancic 2011-02-17 1:41 ` Indan Zupancic 2011-02-17 22:13 ` [PATCH] fix backlight brightness on intel LVDS panel after reopening lid Tino Keitel 2011-02-18 4:57 ` Indan Zupancic 2011-02-19 12:11 ` Alex Riesen 2011-02-19 12:26 ` Alex Riesen 2011-02-19 23:07 ` Linus Torvalds 2011-02-19 23:07 ` Linus Torvalds 2011-02-22 21:04 ` Jesse Barnes 2011-02-22 21:04 ` Jesse Barnes 2011-02-22 22:31 ` Tino Keitel 2011-02-23 1:09 ` Linus Torvalds 2011-02-23 1:09 ` Linus Torvalds 2011-03-04 6:53 ` Indan Zupancic 2011-03-04 6:53 ` Indan Zupancic 2011-03-04 18:47 ` Linus Torvalds 2011-03-04 23:32 ` Indan Zupancic 2011-03-04 23:32 ` Indan Zupancic 2011-03-06 17:40 ` Alex Riesen 2011-03-10 5:50 ` Indan Zupancic [this message] 2011-03-10 5:50 ` Indan Zupancic 2011-03-10 6:00 ` Indan Zupancic 2011-03-10 6:00 ` Indan Zupancic 2011-03-10 7:49 ` Takashi Iwai 2011-03-10 7:49 ` Takashi Iwai 2011-03-10 8:25 ` Takashi Iwai 2011-03-10 10:06 ` Indan Zupancic 2011-03-10 10:06 ` Indan Zupancic 2011-03-10 12:59 ` Takashi Iwai 2011-03-10 13:02 ` [PATCH] drm/i915: Revive combination mode for backlight control Takashi Iwai 2011-03-10 19:36 ` Keith Packard 2011-03-10 19:36 ` Keith Packard 2011-03-11 1:30 ` Indan Zupancic 2011-03-11 1:23 ` Indan Zupancic 2011-03-11 1:28 ` Linus Torvalds 2011-03-11 1:28 ` Linus Torvalds 2011-03-11 7:26 ` Takashi Iwai 2011-03-11 9:08 ` Indan Zupancic 2011-03-11 9:08 ` Indan Zupancic 2011-03-11 7:34 ` Keith Packard 2011-03-11 7:34 ` Keith Packard 2011-03-10 8:45 ` [PATCH] fix backlight brightness on intel LVDS panel after reopening lid Indan Zupancic 2011-03-10 12:51 ` Takashi Iwai 2011-03-05 0:26 ` Peter Stuge 2011-03-05 0:26 ` Peter Stuge 2011-02-23 1:32 ` Indan Zupancic 2011-02-23 1:32 ` Indan Zupancic 2011-02-18 5:45 ` Tino Keitel 2011-02-20 14:03 ` Linux 2.6.38-rc5 Paul Rolland
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=1a06e2711df021a802d609ad1a75db17.squirrel@webmail.greenhost.nl \ --to=indan@nul.nu \ --cc=chris@chris-wilson.co.uk \ --cc=dri-devel@lists.freedesktop.org \ --cc=jbarnes@virtuousgeek.org \ --cc=linux-kernel@vger.kernel.org \ --cc=raa.lkml@gmail.com \ --cc=stable@kernel.org \ --cc=tino.keitel@tikei.de \ --cc=tiwai@suse.de \ --cc=torvalds@linux-foundation.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.