With 2.6.37-rc2+, closing and opening my laptop lid causes the screen to be dimmed by a few notches when it comes back up. The process can be repeated until the backlight is _completely_ off. Touching the backlight controls on the keyboard causes the backlight to go back to normal. Happens both in X and at the console. This is a regression from 2.6.36. I'm using a ThinkPad T500 laptop with a GM45. Bisection implicates the following. Unfortunately, it doesn't revert cleanly. e9e331a8abeece1565d383510ed985945132ffe3 is the first bad commit commit e9e331a8abeece1565d383510ed985945132ffe3 Author: Chris Wilson <chris@chris-wilson.co.uk> Date: Mon Sep 13 01:16:10 2010 +0100 drm/i915/lvds: Ensure panel is unlocked for Ironlake or the panel fitter Commit 77d07fd9d73ef28689737c0952dbd5d6a5017743 introduced a regression where by not waiting for the panel to be turned off, left the panel and PLL registers locked across the modeset. Thus the panel remaining blank. As pointed out by Daniel Vetter, when testing LVDS it helps to open the laptop and look at the actual panel you are purporting to test. A second issue with the patch was that in order to modify the panel fitter before gen5, the pipe and the panel must have be completely powered down. So we wait. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> :040000 040000 292f504943b059e03adbf0ec744a4a77f72dba5a 5d8c29b4581d37de8f188b8f1817847b63ce155d M drivers git bisect start 'drivers/gpu' # bad: [0adcbd7033be103482fd965496c8be5dd8bcd95e] Revert "ipv6: addrconf: don't remove address state on ifdown if the address is being kept" git bisect bad 0adcbd7033be103482fd965496c8be5dd8bcd95e # good: [f6f94e2ab1b33f0082ac22d71f66385a60d8157f] Linux 2.6.36 git bisect good f6f94e2ab1b33f0082ac22d71f66385a60d8157f # bad: [a424d761a00c0233cb7734a8cd572ecd6d0362aa] drm/nouveau/kms: Avoid a hang entering KDB with VT accel on. git bisect bad a424d761a00c0233cb7734a8cd572ecd6d0362aa # bad: [e65d9305f528d4f354378690135ee8d1c60ec0a5] drm/i915: Initialize intel_crtc->active git bisect bad e65d9305f528d4f354378690135ee8d1c60ec0a5 # good: [106dadacbeeea92f61a2c32f3651ee31c1b34e31] drm/i915/overlay: Workaround i830 overlay activation bug. git bisect good 106dadacbeeea92f61a2c32f3651ee31c1b34e31 # good: [f875c15a4fbf37534dda30771d8bde8604fbbf09] drm/i915: Use the direct mapping of pipe->crtc git bisect good f875c15a4fbf37534dda30771d8bde8604fbbf09 # good: [ea056c14a269be393468fe3734f6c2319eb23a3f] drm/i915: enable thermal reporting for IPS git bisect good ea056c14a269be393468fe3734f6c2319eb23a3f # good: [ec5da01e23eec303dd313aa62b8ed4712c488437] drm/i915: Use msleep instead of mdelay during wait_vblank_off git bisect good ec5da01e23eec303dd313aa62b8ed4712c488437 # bad: [8aadf70bd72c8f15994e68503af8f6722cd5c813] drm/i915/lvds: Remove incorrect mode locking git bisect bad 8aadf70bd72c8f15994e68503af8f6722cd5c813 # good: [6edc3242e35f03990e362e7c115e722717f0f7a7] drm/i915/bios: Prevent NULL dereference after allocation failure git bisect good 6edc3242e35f03990e362e7c115e722717f0f7a7 # bad: [e9e331a8abeece1565d383510ed985945132ffe3] drm/i915/lvds: Ensure panel is unlocked for Ironlake or the panel fitter git bisect bad e9e331a8abeece1565d383510ed985945132ffe3
How's the backlight faring with Keith's fixes to both -intel and drm.ko?
The issue persists with 2.6.37-rc5 (which includes Keith's patches) and lastest git xf86-video-intel (although note that it also occurs before X is even started).
Still present in latest git (-rc8 with the subsequent i915 pull).
I believe this is the fix you are looking for: commit 47356eb67285014527a5ab87543ba1fae3d1e10a Author: Chris Wilson <chris@chris-wilson.co.uk> Date: Tue Jan 11 17:06:04 2011 +0000 drm/i915/panel: Only record the backlight level when it is enabled By tracking the current status of the backlight we can prevent recording the value of the current backlight when we have disabled it. And so prevent restoring it to 'off' after an unbalanced sequence of intel_lvds_disable/enable. Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=22672 Tested-by: Alex Riesen <raa.lkml@gmail.com> Tested-by: Larry Finger <Larry.Finger@lwfinger.net> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: stable@kernel.org pushed to -fixes and pull request sent.
Unfortunately, that commit doesn't seem to have changed anything. I tested both 2.6.37 with that commit cherry-picked, and also Linus' master with your drm-intel-fixes branch merged in: in either case, the backlight is still dimmed when the lid is opened. Something I didn't really notice until recently is that there is a delay before the backlight gets set to the wrong value: when the lid is first opened, the backlight comes on at the (presumably) correct brightness; after about half a second or so, it is set to the dimmer value.
(In reply to comment #5) > Unfortunately, that commit doesn't seem to have changed anything. I spoke a little too soon: it did change _something_. With this commit, the backlight cannot be dimmed repeatedly by opening and closing the lid multiple times as described on the original report -- if I close and open the lid a second time, the backlight will remain at the previous (dim) level. If I touch one of the brightness keys, everything is reset and closing/opening the lid will again dim the backlight (once).
Nick, do you mind instrumenting intel_panel_enable_backlight() and intel_panel_disable_backlight() to check when they are being called and recording the backlight level before turning off? What it sounds like is there is a conflict between the value we are reading and the value ACPI is using.
So this seems interesting: with Linus' master, I added printks to intel_panel_{enable,disable}_backlight and booted with drm.debug=0x2 to see what's going on. Here's the redacted and annotated summary of what gets printed. Early in boot (before fbcon is started), I can see the printk I added in disable: [drm:intel_panel_get_backlight], get backlight PWM = 1204110 intel_panel_disable_backlight: level was 1204110 (so the level 1204110 got stored in the private structure. There is another call to disable_backlight which skips saving the value due to the earlier patch). Later, after fbcon is initialized, we see the backlight being set to the proper level (via ACPI?) -- it remains at this level until the lid is closed. Console: switching to colour frame buffer device 210x65 fb0: inteldrmfb frame buffer device drm: registered panic notifier [drm:intel_opregion_asle_intr], non asle set request?? [drm:intel_panel_get_max_backlight], max backlight PWM = 2408475 [drm:intel_panel_set_backlight], set backlight PWM = 2408475 [drm:intel_panel_get_max_backlight], max backlight PWM = 2408475 [drm:intel_opregion_asle_intr], non asle set request?? [drm:intel_opregion_asle_intr], non asle set request?? [drm:intel_panel_get_max_backlight], max backlight PWM = 2408475 After boot, when the lid is closed and opened, the following is printed (note that disable_backlight is *not* called): intel_panel_enable_backlight: level set to 1204110 [drm:intel_panel_set_backlight], set backlight PWM = 1204110 [drm:intel_panel_get_max_backlight], max backlight PWM = 2408475 [drm:intel_opregion_asle_intr], non asle set request?? So enable_backlight restored the value saved at early boot, which is presumably the dim value that I'm seeing. (If I use the keyboard controls to drop the backlight as far as it'll go, the effect is that the screen gets brighter when the lid is closed and opened). When I press the backlight up key, I see requests to set the backlight to the maximum value: [drm:intel_opregion_asle_intr], non asle set request?? [drm:intel_panel_get_max_backlight], max backlight PWM = 2408475 [drm:intel_panel_set_backlight], set backlight PWM = 2408475 [drm:intel_panel_get_max_backlight], max backlight PWM = 2408475 [drm:intel_opregion_asle_intr], non asle set request?? Finally, I decided to look at what gets printed when I do 'xset dpms force standby' -- this case was uninteresting to me before because it doesn't cause any problems with the backlight. But with the debugging info, it looks really interesting. First, on standby (while the screen is still blank): [drm:intel_panel_get_backlight], get backlight PWM = 1204110 intel_panel_disable_backlight: level was 1204110 [drm:intel_panel_set_backlight], set backlight PWM = 0 [drm:intel_panel_get_max_backlight], max backlight PWM = 2408475 [drm:intel_opregion_asle_intr], non asle set request?? [drm:intel_opregion_asle_intr], non asle set request?? [drm:intel_panel_get_max_backlight], max backlight PWM = 2408475 [drm:intel_panel_set_backlight], set backlight PWM = 217235 [drm:intel_panel_get_max_backlight], max backlight PWM = 2408475 [drm:intel_opregion_asle_intr], non asle set request?? intel_panel_disable_backlight: level was 4294967295 [drm:intel_panel_set_backlight], set backlight PWM = 0 [drm:intel_panel_get_max_backlight], max backlight PWM = 2408475 [drm:intel_opregion_asle_intr], non asle set request?? [drm:intel_opregion_asle_intr], non asle set request?? [drm:intel_panel_get_max_backlight], max backlight PWM = 2408475 [drm:intel_panel_set_backlight], set backlight PWM = 217235 [drm:intel_panel_get_max_backlight], max backlight PWM = 2408475 [drm:intel_opregion_asle_intr], non asle set request?? It seems that 1204110 got saved, despite the backlight being set to 2408475. (the value 4294967295 was the one I used if nothing got saved due to the double call to disable_backlight). Finally, when the srceen is brought out of standby, we se the following: intel_panel_enable_backlight: level set to 1204110 [drm:intel_panel_set_backlight], set backlight PWM = 1204110 [drm:intel_panel_get_max_backlight], max backlight PWM = 2408475 [drm:intel_opregion_asle_intr], non asle set request?? [drm:intel_opregion_asle_intr], non asle set request?? [drm:intel_panel_get_max_backlight], max backlight PWM = 2408475 [drm:intel_panel_set_backlight], set backlight PWM = 2408475 [drm:intel_panel_get_max_backlight], max backlight PWM = 2408475 [drm:intel_opregion_asle_intr], non asle set request?? So it seems that in the DPMS case, the backlight was restored to the wrong level after all, but then something (ACPI?) sets it to the right one. Let me know if you need any more info.
Ho hum. Could it just be as simple as that the "get >>= 1" is completely wrong?
Created attachment 41959 [details] [review] Fix backlight combination mode
Created attachment 41960 [details] [review] Fix backlight combination mode Works better if we don't overflow the u8.
I won't be able to test anything until this evening, but I tried the first patch earlier -- with it, the system restores the backlight to full brightness every time (probably because disable_backlight does not seem to be called when I close the lid). So if I lower the backlight using the keyboard controls, the issue still occurs (but in reverse). I decided to try out 2.6.36 again, as well, and I noticed something else with this kernel (and I think all others before it) which may be related: When the panel is opened, first the backlight is on (at the correct level), then the screen goes completely black, then the screen turns on again (at the correct backlight level). The time where the screen goes black in 2.6.36 is the same as where 2.6.37 sets the backlight to the wrong level -- the difference is that 2.6.36 seems to correct the issue after a moment but 2.6.37 does not.
Created attachment 41989 [details] [review] Stub out backlight enable/disable/setup functions. (In reply to comment #11) > Created an attachment (id=41960) [details] > Fix backlight combination mode > > Works better if we don't overflow the u8. The second patch completely breaks the keyboard backlight controls; with it applied, I can only set the backlight to either maximum brightness or completely off. Specifically, every setting except the brightest results in the backlight turning off (e.g., if I press backlight-down 8 times, I have to press backlight-up 8 times for the panel to turn back on). The first patch also caused some issues with the keyboard controls, but none so severe as the second. Anyway, I decided to try an experiment with the attached patch. I'm probably not understanding all the issues, but my backlight seems to work perfectly with the enable/disable/setup functions stubbed out!
Yeah, on your machine we have two drivers fighting over control of the backlight. Admittedly one of those is broken... One last experiment: diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_pan index 71ba046..c8bcc4c 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -282,6 +282,9 @@ void intel_panel_setup_backlight(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; + I915_WRITE(BLC_PWM_CTL2, + I915_READ(BLC_PWM_CTL2) &~ BLM_COMBINATION_MODE) + dev_priv->backlight_level = intel_panel_get_backlight(dev); dev_priv->backlight_enabled = dev_priv->backlight_level != 0; }
(In reply to comment #14) > Yeah, on your machine we have two drivers fighting over control of the > backlight. Admittedly one of those is broken... > > One last experiment: The effects of this patch are the same as described in comment #12 (first paragraph). That is: the backlight seems to be restored correctly but the current level is not saved when I close the lid (it is saved on dpms off, however). Furthermore, the keyboard controls still work correctly with this one.
commit 951f3512dba5bd44cda3e5ee22b4b522e4bb09fb Author: Indan Zupancic <indan@nul.nu> Date: Thu Feb 17 02:41:49 2011 +0100 drm/i915: Do not handle backlight combination mode specially
(In reply to comment #16) > commit 951f3512dba5bd44cda3e5ee22b4b522e4bb09fb > Author: Indan Zupancic <indan@nul.nu> > Date: Thu Feb 17 02:41:49 2011 +0100 > > drm/i915: Do not handle backlight combination mode specially Unfortunately, this hasn't fixed the issue completely: the effects are the same as described in comment #15. To clarify: the backlight is restored to whatever level it was at during the last DPMS off (or bootup, whichever is more recent). So if I boot up with the backlight at full brightness, then set the backlight as dim as it'll go, then close/open the lid, the backlight will come back at full brightness again. As before, once I touch the backlight controls, it will suddenly become fully dim. Similarly, if I set the backlight fully dim, then dpms off/on, then set the backlight fully bright, then close/open the lid, the backlight will come back filly dim. Once I touch the backlight controls in this case, it will suddenly be fully bright again. For the record, my patch which deletes the backlight-setting code still works.
Daniel is working on some patches to fix the backlight controller and is looking for victims^Wtesters.
Ok, I've created these patches, available at http://cgit.freedesktop.org/~danvet/drm/log/?h=backlight-confusion Git link is git://people.freedesktop.org/~danvet/drm backlight-confusion Please test them, thanks.
Reporter seems to have disappeared, but 3.6 has all the backlight changes. Presumably the bug is now fixed, please reopen if this is still an issue.
(In reply to comment #20) > Reporter seems to have disappeared, but 3.6 has all the backlight changes. > Presumably the bug is now fixed, please reopen if this is still an issue. The regression as originally reported has actually been fixed for some time now (i.e., the behaviour is the same as it was with Linux 2.6.36). So yes, this bug can be closed as fixed. As it was with Linux 2.6.36, the screen turns black for a very short time about 1 second after opening the lid. It is clearly possible to work better than this, since there was no such flicker (and the backlight worked fine) way back when I tested the patch in comment #13. That patch no longer works. Anyway, that flicker is a separate issue not related to the original report, and is not a regression. I may file another bug about it if it still happens with 3.6.
That flickering when opening the lid is most likely a "feature" added by Linus Torvalds in: commit c9354c85c1c7bac788ce57d3c17f2016c1c45b1d Author: Linus Torvalds <torvalds@linux-foundation.org> Date: Mon Nov 2 09:29:55 2009 -0800 i915: fix intel graphics suspend breakage due to resume/lid event confusion That commit adds a superflous (sometimes) modeset at lid open. We're looking into it, unfortunately this area is a regression minefield, so don't expect anything anytime soon :(
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.