Bug 47941

Summary: [snb] Backlight control broken between 3.6.0-rc1 and 3.6.0-rc4
Product: Drivers Reporter: Dan Carpenter (error27)
Component: Video(DRI - Intel)Assignee: intel-gfx-bugs (intel-gfx-bugs)
Status: RESOLVED OBSOLETE    
Severity: normal CC: alan, apsu, bugs+kernel, bugs, colin, daniel, EGriffith92, el, emailgrant, florian, h, infernix, intel-gfx-bugs, jfceklosky, jlee, kamal, kernelbugs, leoni.massimiliano1, ludovico.cavedon, mezin.alexander, mike, mikko.cal, nalimilan, tiwai, ulf, v-kernel, vincent.debian
Priority: P1    
Hardware: All   
OS: Linux   
Kernel Version: 3.6.0-rc4 Subsystem:
Regression: Yes Bisected commit-id:
Bug Depends on:    
Bug Blocks: 47411    
Attachments: 3.6.0-rc7 kernel config
dmesg output in 3.6.0-rc7
intel_reg_dumper with the backlight working
intel_reg_dumper with the backlight not working
drm/i915: set PCH backlight enable independent of CPU backlight enable
A test patch for fixing backlight regression
A test fix patch for regression of regression fix
Set backlight in delayed work
drm/i915: quirk no PCH_PWM_ENABLE for Dell XPS13 backlight
drm/i915: enable/modify backlight only if needed
drm/i915: quirk no PCH_PWM_ENABLE for Dell XPS13 backlight
possible fix for XPS13+UEFI+Legacy
i915.disable_pch_pwm boot param
get edp dpcd info
get edp info results
Updated version of i915.disable_pch_pwm command line patch for v3.12

Description Dan Carpenter 2012-09-25 11:37:15 UTC
Subject    : Backlight control broken between 3.6.0-rc1 and 3.6.0-rc4
Submitter  : Grant <emailgrant@gmail.com>
Date       : Sun, 16 Sep 2012 04:25:10 -0700
Message-ID : <CAN0CFw1GiAZxLcRGaEOxYcGhOfF2hjDuoF+e0T8r_ZryTFzw5Q@mail.gmail.com>
References : https://lkml.org/lkml/2012/9/16/26

I have a Dell XPS 13 Ultrabook laptop.  Backlight control used to be
broken, it works in 3.6.0-rc1, and it is broken again in 3.6.0-rc4.

- Grant
Comment 1 emailgrant 2012-09-29 07:46:19 UTC
Created attachment 81441 [details]
3.6.0-rc7 kernel config
Comment 2 emailgrant 2012-09-29 07:47:27 UTC
Created attachment 81451 [details]
dmesg output in 3.6.0-rc7
Comment 3 emailgrant 2012-09-29 07:48:48 UTC
It is also not working in 3.6.0-rc7.  Please let me know if you need anything else.  lsmod is empty:

# lsmod
Module                  Size  Used by
#
Comment 4 emailgrant 2012-10-02 08:01:09 UTC
Backlight control still does not work in the 3.6.0 release.
Comment 5 emailgrant 2012-11-09 00:25:17 UTC
Backlight control still does not work in 3.7-rc3.
Comment 6 Alan 2012-11-09 10:46:33 UTC
HD Graphics 3000
Comment 7 Daniel Vetter 2012-11-09 19:54:54 UTC
Can you please do a bisect of where exactly the backlight control broke? Also, please describe the symptoms more precisely: Do the backlight keys no longer work? Or the backlight controllers in /sys/class/backlight/? If the latter, which one?
Comment 8 emailgrant 2012-11-09 20:28:19 UTC
The backlight keys no longer work.

The 3.6.0-rc* packages are no longer available from Gentoo.  Can you tell me how to download 3.6.0-rc2 and 3.6.0-rc3 so I can test them?  If via git, please let me know the exact git commands as I don't know how to use git.  Thank you.
Comment 9 Daniel Vetter 2012-11-09 20:38:47 UTC
http://www.reactivated.net/weblog/archives/2006/01/using-git-bisect-to-find-buggy-kernel-patches/

is imo an excellent kernel bisect howto using git, you only have to replace the kernel versions with your starting points. You can make sure that you can properly reproduce the regression with the git kernels by checking out a specific version with

$ git checkout v3.6-rc2

before you start the bisect (just to make sure that nothing in the gentoo kernels changes the behavior).
Comment 10 emailgrant 2012-11-09 22:36:01 UTC
Can anyone wager a guess as to how much downloading this will require?  I'm on a metered connection right now and I want to be sure I can afford it.
Comment 11 Daniel Vetter 2012-11-09 22:40:16 UTC
The current kernel git tree is around 800M. A shallow clone might reduce this a bit, but for bisecting it's a big fuzz to set up.
Comment 12 emailgrant 2012-11-09 23:08:12 UTC
OK, I'm on the road and moving around a lot.  I might end up in a place with free internet soon.  If not, I'll probably just spring for it.  Thanks for your help thus far.
Comment 13 Daniel Vetter 2012-11-13 12:18:17 UTC
(In reply to comment #8)
> The backlight keys no longer work.

I've totally missed this one here. If the backlight control via GUI or in /sys/class/backlights still works, then this is not a i915.ko bug, but much more likely an issue with the input side of things. Bisect would still be very useful though for the input guys.

Please confirm quickly, thanks.
Comment 14 emailgrant 2012-11-13 20:46:37 UTC
I have more info:

In 3.7.0-rc3, I have two brightness interfaces:

/sys/class/backlight/acpi_video0/brightness
/sys/class/backlight/intel_backlight/brightness

acpi_video0/brightness starts at 15 and intel_backlight/brightness starts at 0.  If I change acpi_video0/brightness, nothing happens.  If I change intel_backlight/brightness to a value above 0, the backlight flickers violently.  However, if I change intel_backlight/brightness to a value above 0 and then back to 0, changing acpi_video0/brightness works as it should.

I tried adding acpi_backlight=vendor to grub and /sys/class/backlight/acpi_video0/brightness disappeared, but changing intel_backlight/brightness still resulted in violent flickering.
Comment 15 Daniel Vetter 2012-11-13 21:07:39 UTC
Ok, sounds like something's amiss in the i915 backlight code then. And I guess we need the bisect result to move forward with this ...
Comment 16 emailgrant 2012-11-16 20:04:45 UTC
I will have gloriously free internet starting on the 30th and I will bisect then.
Comment 17 emailgrant 2012-12-03 05:42:44 UTC
I bisected and came up with this:

770c12312ad617172b1a65b911d3e6564fc5aca8 is the first bad commit
commit 770c12312ad617172b1a65b911d3e6564fc5aca8
Author: Takashi Iwai <tiwai@suse.de>
Date:   Sat Aug 11 08:56:42 2012 +0200

    drm/i915: Fix blank panel at reopening lid
    
    When you reopen the lid on a laptop with PCH, the panel suddenly goes
    blank sometimes.  It seems because BLC_PWM_CPU_CTL register is cleared
    to zero when BLC_PWM_CPU_CTL2 and BLC_PWM_PCH_CTL1 registers are
    enabled.
    
    This patch fixes the problem by moving the call of the function setting
    BLC_PWM_CPU_CTL after enabling other two registers.
    
    Reported-and-tested-by: Hugh Dickins <hughd@google.com>
    Signed-off-by: Takashi Iwai <tiwai@suse.de>
    Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

:040000 040000 77788b57cbec30de320931ff5a5b8d439d211875 9f2da69a41da655948e963bc49638cd140f22f5d M	drivers

I also noticed that if I run 'xset dpms force off', the backlight stops working on the good kernel (3.6.0-rc1).
Comment 18 Daniel Vetter 2012-12-04 17:50:19 UTC
Adding Takashi ...
Comment 19 Takashi Iwai 2012-12-05 07:52:08 UTC
Does reverting the commit above fix your problem?
Comment 20 emailgrant 2012-12-08 02:17:20 UTC
I'm sorry for the delay.  Yes, I just confirmed that reverting the commit does fix the backlight.  'xset dpms force off' then breaks it as it does with 3.6.0-rc1.
Comment 21 Takashi Iwai 2012-12-08 08:08:03 UTC
Hm, the commit above just shuffles the order of three register writes.
What happens if you add the lines back like the patch below, i.e. calling intel_panel_actually_set_backlight() twice, before and after setting two registers?

diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index e2aacd3..7e82b0b 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -320,6 +320,9 @@ void intel_panel_enable_backlight(struct drm_device *dev,
 	if (dev_priv->backlight_level == 0)
 		dev_priv->backlight_level = intel_panel_get_max_backlight(dev);
 
+	dev_priv->backlight_enabled = true;
+	intel_panel_actually_set_backlight(dev, dev_priv->backlight_level);
+
 	if (INTEL_INFO(dev)->gen >= 4) {
 		uint32_t reg, tmp;
Comment 22 emailgrant 2012-12-08 21:02:58 UTC
I'm confused.  Reverting patch 770c12312ad617172b1a65b911d3e6564fc5aca8 means adding those lines, right?

http://kernel.opensuse.org/cgit/kernel/commit/?id=770c12312ad617172b1a65b911d3e6564fc5aca8

I already did that.  Please straighten me out if I'm doing this incorrectly.
Comment 23 Takashi Iwai 2012-12-10 07:10:20 UTC
No, reverting the patch *moves* the code to the place before two register writes.
I asked to *add* the code again at that place -- i.e. intl_panel_actually_set_backlight() will be called twice.
Comment 24 emailgrant 2012-12-10 22:25:19 UTC
I see now, you wanted me to apply the patch in Comment #21 before reverting 770c12312ad617172b1a65b911d3e6564fc5aca8.

I did that and the backlight does not work.
Comment 25 Takashi Iwai 2012-12-10 23:07:14 UTC
Ok, then something gets broken by the second call of intel_actually*().

Could you get intel_reg_dumper output and compare between working and nonworking cases?
Comment 26 emailgrant 2012-12-11 03:15:50 UTC
Created attachment 88871 [details]
intel_reg_dumper with the backlight working
Comment 27 emailgrant 2012-12-11 03:17:12 UTC
Created attachment 88881 [details]
intel_reg_dumper with the backlight not working
Comment 28 Daniel Vetter 2012-12-11 09:47:36 UTC
Maybe-related: https://bugs.freedesktop.org/show_bug.cgi?id=43577
Comment 29 Jani Nikula 2012-12-11 09:57:11 UTC
Created attachment 88911 [details]
drm/i915: set PCH backlight enable independent of CPU backlight enable

You might want to try the patch attached in the fdo bug referred to by Daniel, and also this one. Which is also a long shot...
Comment 30 emailgrant 2012-12-11 21:20:43 UTC
I tried both of those patches but they do not fix the backlight.
Comment 31 Takashi Iwai 2012-12-12 13:21:04 UTC
Then how about the patch below?

It's not tested for the machines that need the original fix, though.  Unfortunately, I don't remember on which machine the problem hit.  (There are way too many test machines here...)
Maybe Hugh can check whether this patch breaks on his machine or not.
Comment 32 Takashi Iwai 2012-12-12 13:21:32 UTC
Created attachment 88981 [details]
A test patch for fixing backlight regression
Comment 33 emailgrant 2012-12-12 20:27:27 UTC
I'm happy to say that patch does fix it.

'xset dpms force off' breaks it again though, as it did with 3.6.0-rc1.  Should I file a separate bug for that?
Comment 34 emailgrant 2012-12-16 21:52:09 UTC
Will that patch be integrated into the kernel release?
Comment 35 emailgrant 2012-12-29 16:33:11 UTC
Can anyone tell me the status of this?  Why is it NEEDINFO?
Comment 36 Daniel Vetter 2013-02-06 10:20:28 UTC
Sorry for the delay, I've just worked Takashi's quick hack into a real patch and submitted it for inclusion (with cc: stable added). Should move to a kernel release near you soon, but probably will land only in 3.9 and then get backport (Linus is rather strict already with fixes for 3.8).
Comment 37 Daniel Vetter 2013-02-06 10:25:52 UTC
Patch: https://patchwork.kernel.org/patch/2102971/
Comment 38 emailgrant 2013-02-08 06:17:01 UTC
Thank you, I tested it and it does work until I issue 'xset dpms force off'.  I will file another bug for that.
Comment 39 Daniel Vetter 2013-02-08 08:34:04 UTC
Patch is now merged and on track to land into 3.9 (and get backported from there):

commit 6e1c8a382a938a228b2c6dc575862aaa0d1042a7
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Wed Feb 6 11:24:41 2013 +0100

    drm/i915: write backlight harder
Comment 40 Florian Mickler 2013-03-04 21:27:44 UTC
A patch referencing this bug report has been merged in Linux v3.9-rc1:

commit cf0a6584aa6d382f802f2c3cacac23ccbccde0cd
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Wed Feb 6 11:24:41 2013 +0100

    drm/i915: write backlight harder
Comment 41 Takashi Iwai 2013-03-04 21:48:12 UTC
My patch seems breaking again the machines that my first patch had fixed, unfortunately, as I was afraid of.
So it has to be reverted again or we need a safer fix (e.g. fixup per DMI id)...
Comment 42 Milan Bouchet-Valat 2013-03-05 09:31:52 UTC
*** Bug 54771 has been marked as a duplicate of this bug. ***
Comment 43 Daniel Vetter 2013-03-05 10:37:46 UTC
I think we need to reopen this can of worms ...
Comment 44 Aleksandr Mezin 2013-03-15 08:15:52 UTC
The "write backlight harder" patch causes black screen after suspend on my HP Pavilion dv6-6179er. If I revert it, everything works fine. Also, setting backlight level twice, but without "if (!intel_panel_get_backlight(dev))" works fine too. I even inserted a printk before this if, and it reports:
[   68.335727] i915 backlight: 4882
which is max backlight level.
Comment 45 Aleksandr Mezin 2013-03-15 08:55:00 UTC
Sorry, it may be not clear: intel_panel_get_backlight(dev) returns 4882 but screen is turned off. But calling "intel_panel_actually_set_backlight" unconditionally fixes the problem.

Also, I don't understand, how backlight control even works on machines where "setting the backlight level _after_ enabling it seems
to reset it somehow". Maybe the problem with these machines isn't there?
Comment 46 Takashi Iwai 2013-03-15 11:20:07 UTC
Yeah, my patch is really broken.  I can confirm that a new HP test machine is broken on 3.9-rc2, and I had to revert the commit.

While comparing the reg dump with the one on Dell, I noticed the difference of BLC_PWM_CPU_CTL.  Hopefully the patch below works for both Dell and HP.

It works on a HP machine here.  Alexander, please check whether it works for you, too.

More importantly, Grant, could you check whether it doesn't break yours again?
Comment 47 Takashi Iwai 2013-03-15 11:21:30 UTC
Created attachment 95481 [details]
A test fix patch for regression of regression fix
Comment 48 Aleksandr Mezin 2013-03-15 11:56:29 UTC
No, it doesn't work. Also, now screen turns off on boot, and turns on only when Xorg starts. I inserted a printk again, and:
on boot val is 0
on resume val is 4882, so val >> 16 is 0
Comment 49 Takashi Iwai 2013-03-15 12:32:25 UTC
Hrm, OK, then scratch it.

I'm afraid that the only way for fixing it is to have a blacklist...
Comment 50 Aleksandr Mezin 2013-03-15 12:53:08 UTC
Does backlight control work for those Dell machines later? If so, maybe add a delay before setting backlight level second time, but do it unconditionally?
Comment 51 Tony Houghton 2013-03-15 23:02:25 UTC
Since upgrading to kernel 3.8 the display on my Samsung NP900X3C won't resume from sleep. I can enable it again by restarting gdm3 with ssh, but I can't find a way to recover without having another PC handy. FWIW this model is notorious for broken ACPI behaviour, and in earlier kernels closing the lid often fails to make it sleep, and it never resumes just by opening the lid, you have to press the power button.

I came here via git bisect:

cf0a6584aa6d382f802f2c3cacac23ccbccde0cd is the first bad commit
commit cf0a6584aa6d382f802f2c3cacac23ccbccde0cd
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Wed Feb 6 11:24:41 2013 +0100

    drm/i915: write backlight harder
    
    770c12312ad617172b1a65b911d3e6564fc5aca8 is the first bad commit
    commit 770c12312ad617172b1a65b911d3e6564fc5aca8
    Author: Takashi Iwai <tiwai@suse.de>
    Date:   Sat Aug 11 08:56:42 2012 +0200
    
        drm/i915: Fix blank panel at reopening lid
    
    changed the register write sequence for restoring the backlight, which
    helped prevent non-working backlights on some machines. Turns out that
    the original sequence was the right thing to do for a different set of
    machines. Worse, setting the backlight level _after_ enabling it seems
    to reset it somehow. So we need to make that one conditional upon the
    backlight having been reset to zero, and add the old one back.
    
    Cargo-culting at it's best, but it seems to work.
    
    Cc: stable@vger.kernel.org
    Cc: Takashi Iwai <tiwai@suse.de>
    Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=47941
    Reviewed-by: Jani Nikula <jani.nikula@intel.com>
    Acked-by: Takashi Iwai <tiwai@suse.de>
    Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

:040000 040000 3a65de0b75b23cc30f2d8d0fe90304ca2302fbfa ec984a40be2c47943aa202094b7ce224ff7e8ef7 M	drivers
Comment 52 Tony Houghton 2013-03-16 15:01:41 UTC
Patch id 95481 makes things worse for me too, the light turns off in boot, but unlike in comment #48 mine doesn't turn back on when X starts. Samsung NP900X3C.
Comment 53 Aleksandr Mezin 2013-03-16 15:27:06 UTC
Does backlight control work for you?
Comment 54 Tony Houghton 2013-03-16 17:00:52 UTC
I'm not really sure what you mean. I can control the brightness with 3.7 and earlier, but I had to add and change some udev files to make the Fn keys work. I think I tried those to try to recover from suspend, but they didn't work. But I don't think they work in the login and lock screens anyway.

Do you need me to test with the problem kernels?
Comment 55 Aleksandr Mezin 2013-03-16 17:13:10 UTC
Created attachment 95571 [details]
Set backlight in delayed work

Maybe something like this will work on machines where immediate intel_panel_actually_set_backlight causes problems...
Please test.
Comment 56 Tony Houghton 2013-03-16 19:14:30 UTC
Patch 95571 works for me. The screen goes blank a couple of times in start-up but it comes on again OK. It also resumes successfully from suspend.
Comment 57 Daniel Vetter 2013-03-18 09:50:14 UTC
Another shot worth would be:

https://patchwork.kernel.org/patch/2254211/

and

https://patchwork.kernel.org/patch/2254221/

If that doesn't help I suspect that the utter lack of locking for the backlight code could be the culprit.

But the right course of action might be to revert both the above commit and the original fix from Takashi (since oldest regressions win in this game here).
Comment 58 Aleksandr Mezin 2013-03-18 10:19:54 UTC
These patches doesn't work over vanilla 3.8.3 kernel
These patches work if I revert the "write backlight harder" patch. However, just reverting the "write backlight harder" works for me too.
Comment 59 Elvis Pranskevichus 2013-03-23 03:50:23 UTC
The "drm/i915: write backlight harder" commit broke backlight after resume on my Samsung NP900X4C as well.  Patches suggested in #57 don't help.  Reverting the commit works and backlight functions perfectly after that.
Comment 60 Joe Ceklosky 2013-03-23 04:55:48 UTC
The "drm/i915: write backlight harder" commit broke backlight after resume on
my Asus K55A as well (HD 4000 video).  Reverting the commit works and backlight functions perfectly after that.
Comment 61 Joe Ceklosky 2013-03-23 04:59:30 UTC
I am interested in trying the delayed option suggested id=95571 which I will report on as well.
Comment 62 Aleksandr Mezin 2013-03-23 05:08:38 UTC
If reverting the "write backlight harder" patch works for you, then setting backlight in a delayed work will do the job too.

Someone, for whom backlight currently works, and for whom reverting the patch breaks it, should test "delayed" patch.
Comment 63 Joe Ceklosky 2013-03-23 05:19:12 UTC
I will patch in the delayed code to 3.8.4.  

Does anyone have any idea as to what is really the issue?  I am willing to add debug prints on various settings throughout the code to help fix this for all.
Comment 64 Takashi Iwai 2013-03-23 09:09:48 UTC
Actually, the only interesting case for the delayed work patch is the machines that were _fixed_ by the affecting commit "write backlight harder" (e.g. original reporter's Dell machine).  We need to know whether these machines still work with the delayed work patch.  Honestly speaking, I doubt it.

The machines that got broken by the "write backlight harder" commit are basically not so interesting for now.  It's known that writing BLC_PWM_CPU_CTL register again recovers the backlight at any moment on such machines.
Comment 65 Daniel Vetter 2013-03-23 11:33:50 UTC
Revert merged to drm-intel-fixes:

commit 2a4d5cac39fe8c5b9a485d717477ed2a459ec194
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Fri Mar 22 15:44:46 2013 +0100

    Revert "drm/i915: write backlight harder"
Comment 66 Vincent Blut 2013-03-25 15:25:45 UTC
I had the same issue on my Asus UX31A (http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=703640). Reverting this commit fix the issue but in my case setting acpi_osi="!Windows 2012" on the kernel command line seems to inhibit the issue too.
Comment 67 emailgrant 2013-03-25 21:07:38 UTC
Unfortunately the backlight on my Dell XPS 13 does not work with patch 95571.
Comment 68 Joe Ceklosky 2013-03-26 01:27:27 UTC
Sounds like the Dell is the odd case and will need some type of quirk to enable this behavior using a a kernel parm or a DMI string match to enable the backlight harder functionality.
Comment 69 Kamal Mostafa 2013-04-08 21:41:17 UTC
Created attachment 97751 [details]
drm/i915: quirk no PCH_PWM_ENABLE for Dell XPS13 backlight

The commit "cf0a658 drm/i915: write backlight harder" enabled mostly* working brightness control for the Dell XPS 13 (both Sandy Bridge and Ivy Bridge).  But since that has now been reverted (b128937) those machines are back to non-functional brightness control again.

The attached patch provides a DMI quirk fix which fixes brightness controls for the Dell XPS 13 family, slightly better* than "write backlight harder".

Feedback is invited: Does this fix your XPS 13 brightness control?

*The "write backlight harder" fix stops working after screensaver idle-out per https://bugs.launchpad.net/bugs/1162026 ; this fix continues working.
Comment 70 Daniel Vetter 2013-04-08 21:55:48 UTC
Pasting private mail from Kamal (please just add this to the bug next time around, never send a mail just to me personally, it might get lost):

You've set the status of this to [RESOLVED CODE_FIX] ...

https://bugzilla.kernel.org/show_activity.cgi?id=47941
[snb] Backlight control broken between 3.6.0-rc1 and 3.6.0-rc4

... but the revert of "write backlight harder" was actually the cause of
this bug (for the XPS13), so shouldn't the status be REOPENED still?


Please also note my post and DMI quirk patch which fixes it quite well
for both the Sandy and Ivy XPS 13's:
https://bugzilla.kernel.org/show_bug.cgi?id=47941#c69
(I'll be happy to repost that patch to the intel-gfx list, pending
reaction to the bugzilla post).

Thanks, as always, for your work.
Comment 71 emailgrant 2013-04-20 07:53:10 UTC
Attachment ID 97751 fixes both this bug and this bug:

https://bugzilla.kernel.org/show_bug.cgi?id=53491

Thank you!
Comment 72 Jani Nikula 2013-04-30 07:16:08 UTC
We had some races in the backlight code, which may have caused some of the issues you're seeing. We now have locking, and some other fixes, in place at the drm-intel-nightly branch of git://people.freedesktop.org/~danvet/drm-intel. Please try that and report back.
Comment 73 emailgrant 2013-04-30 18:58:18 UTC
I would be happy to test that but I'm not sure how to do so.  Can you give me a sequence of commands to patch or overwrite the necessary files in my kernel source directory?
Comment 74 Daniel Vetter 2013-04-30 21:42:56 UTC
Probably simplest to grab the sources from git with git clone git://people.freedesktop.org/~danvet/drm-intel -b drm-intel-nightly

That should give you the latest drm/i915 bits with everything.
Comment 75 emailgrant 2013-05-01 03:56:30 UTC
I'm sorry to say that with drm-intel-nightly the backlight does not work on my Dell XPS 13.
Comment 76 Jani Nikula 2013-05-02 11:35:33 UTC
Created attachment 100441 [details]
drm/i915: enable/modify backlight only if needed

(In reply to comment #75)
> I'm sorry to say that with drm-intel-nightly the backlight does not work on
> my
> Dell XPS 13.

Please try the attached patch on top of drm-intel-nightly, and see if it helps. Thanks.
Comment 77 emailgrant 2013-05-02 21:07:49 UTC
No change with that patch unfortunately.
Comment 78 Jani Nikula 2013-05-13 07:10:14 UTC
*** Bug 57151 has been marked as a duplicate of this bug. ***
Comment 79 Jani Nikula 2013-05-13 07:13:36 UTC
Hmm, I seem to have overlooked comment #66. Does acpi_osi="!Windows 2012" on the kernel command line help?
Comment 80 emailgrant 2013-05-13 17:22:17 UTC
Adding acpi_osi="!Windows 2012" to the kernel command line without any patches seems to have no affect.  I think I tried that a while back.
Comment 81 Kamal Mostafa 2013-05-13 18:52:55 UTC
Created attachment 101331 [details]
drm/i915: quirk no PCH_PWM_ENABLE for Dell XPS13 backlight

This updated version of the XPS13 backlight quirk patch restricts its effect to the SandyBridge model only (the quirk breaks the IvyBridge, which didn't need it after all).  For reference, Ubuntu Raring's 3.8 kernel now includes this patch.
Comment 82 Martin 2013-05-15 10:05:23 UTC
As a fresh owner of a XPS 13 (fhd version) I stumbled accross the backlight problems and found this bug. I'm sorry to say that patch in attachment 1011331 applied to a vanilla 3.9.2 (succeeds with some offsets) does not fix the problem for me.

The only working solution for me is "echo 0 > /sys/class/backlight/intel_backlight/brightness" after boot or resume from suspend.

Some research learns that I seem to own the Ivy Bridge version (i7-3537U) so it's to be expected this patch shouldn't work, but contradicts the statement "which didn't need it after all".
Comment 83 Jani Nikula 2013-05-15 10:52:08 UTC
(In reply to comment #82)
> As a fresh owner of a XPS 13 (fhd version) I stumbled accross the backlight
> problems and found this bug. I'm sorry to say that patch in attachment
> 1011331
> applied to a vanilla 3.9.2 (succeeds with some offsets) does not fix the
> problem for me.
> 
> The only working solution for me is "echo 0 >
> /sys/class/backlight/intel_backlight/brightness" after boot or resume from
> suspend.
> 
> Some research learns that I seem to own the Ivy Bridge version (i7-3537U) so
> it's to be expected this patch shouldn't work, but contradicts the statement
> "which didn't need it after all".

How about with the old version of the patch that should cover IVB too?
https://bugzilla.kernel.org/attachment.cgi?id=97751
Comment 84 Martin 2013-05-15 11:08:20 UTC
Yes, the patch in attachment.cgi?id=97751 does the trick for me. Preliminary tests showed I now have functional backlight both from power-up and standby conditions, without the "echo 0".
Comment 85 Jan De Luyck 2013-05-20 13:40:01 UTC
(In reply to comment #83)
> How about with the old version of the patch that should cover IVB too?
> https://bugzilla.kernel.org/attachment.cgi?id=97751

I can confirm this fixes brightness control on my XPS13 (Ivy bridge version), against 3.10-rc1.
Comment 86 Eric Griffith 2013-05-21 15:30:58 UTC
Kamal, when can we expect to see this hardware quirk mainlined, or whats the easiest way to apply it? Could it make it into rc3? Since its fixing a bug that was supposed to be fixed last cycle and given that its PCI-checked to prevent accidental breakage.

Arch x64, Sandy Bridge XPS13z
Comment 87 Martin 2013-07-01 08:25:28 UTC
Hmm... too bad this patch didn't make it into 3.10.0?
Comment 88 Eric Griffith 2013-07-01 20:34:18 UTC
Agreed. hopefully its able to be backported to 3.10.x rather than just 3.11
Comment 89 Daniel Vetter 2013-07-16 06:24:29 UTC
Hm, somehow this patch got lost, and it never showed up on intel-gfx@lists.fd.o

Kamal, can you please submit the patch so that I can merge it? The list is open to non-subscribers, so just submitting the patch with git send-email --to 'intel-gfx@lists.freedesktop.org' should get the job done.
Comment 90 Eric Griffith 2013-07-16 18:19:34 UTC
Kamal, not sure if you already submitted the patch as I don't follow the intel-gfx mailing list. If you have you can ignore this comment. 

I know this bug was marked as NEEDINFO-- not sure if that was just a 'no better option' or if you were actually waiting on info. I've got one of the Dell XPS13z Sandy Bridge ultrabooks, so if you need me to test anything I can. 

The latest patch: https://bugzilla.kernel.org/attachment.cgi?id=97751

Correctly fixes backlight controls on Sandy Bridge, and does NOT break backlight on my other test laptop (Asus with a perfectly functional backlight control). I cannot test on Ivy Bridge as I dont have one laying around.
Comment 91 Kamal Mostafa 2013-07-16 18:35:16 UTC
The XPS13 quirk patch got stalled because I received reports that it broke and/or failed to fix brightness control on the XPS13 "FHD" model.   I didn't have one of those to work on, but I have now received one from Dell.  Eric, I also have a non-FHD SandyBridge model, but of course I'll appreciate your test feedback as well.

Daniel, I'll work on a revised patch for intel-gfx this week.
Comment 92 Eric Griffith 2013-07-16 19:02:32 UTC
Much appreciated Kamal. Happy to test any patches you want for this bug. 

Reading the comments above, it says the Sandy Bridge is the only one with a quirk? Ivy Bridge was actually fine all along / suffering from a different bug. I'm looking at your comment in #81, though maybe im reading that improperly.
Comment 93 Kamal Mostafa 2013-07-19 22:02:26 UTC
My previous comments "the quirk breaks the IvyBridge, which didn't need it after all" should be disregarded.  Further testing with all three XPS13 models shows that all three do need the backlight quirk fix (attachment 97751 [details]) and it doesn't break any of them (for 3.8+ at least).

So, I have just submitted the quirk patch to intel-gfx:
  Subject: [PATCH] drm/i915: quirk no PCH_PWM_ENABLE for Dell XPS13 backlight

Thanks in advance for merging it, Daniel.
Comment 94 Martin 2013-08-05 10:38:35 UTC
Completely fixed without needing to patch 3.10.5, for me (actually the patch brakes the incremental 3.10.4-5 patch) which was to be expected so I compiled a clean 3.10.5 and the problem is gone.

Thx!
Comment 95 leoni.massimiliano1 2013-08-07 13:48:06 UTC
Wonderful! Release 3.10.5 fixed the bug on my Archlinux.
Comment 96 Evan Callicoat 2013-08-08 15:25:34 UTC
The attached patch ( https://bugzilla.kernel.org/attachment.cgi?id=97751 ) doesn't exactly match the actual patch that was applied in 3.10.5. The actual patch splits the model matches in the intel_quirks struct into two model entries each with their own comment, and the comment texts are more detailed, so attempting to revert from the attachment doesn't work.

That said, this patch absolutely breaks the backlight entirely on my Dell XPS 13 Developer Edition FHD (IvyBridge, with the latest BIOS revision A09). I believe Kamal did not test the IVB he received thoroughly enough, which is not surprising since there's some very counter-intuitive behavior based on the BIOS settings and whether you're booting in UEFI or BIOS mode.

There are three basic scenarios:

1) Booting in Legacy (BIOS) mode
2) Booting in UEFI mode with the Legacy Option Rom
3) Booting in UEFI mode without the Legacy Option ROM

The first two didn't have backlight issues before the patch and don't have any after the patch. They operate the GPU differently somehow. Booting with drm.debug=0xe I haven't been able to clearly identify the exact differences yet.

However, the third configuration (which I use) works great without the patch, and the backlight is completely inoperable with the patch. I am well-aware of ACPI OSI strings and have tried various combinations and omitting them. I am also aware of /sys/class/backlight/{acpi_video0,intel_backlight}, and acpi_backlight=legacy/vendor/[omitted].

No combination of these things will make the backlight work on this specific machine booted in mode 3 with this patch.

Please revisit this!
Comment 97 Martin 2013-08-08 15:38:57 UTC
I confirm that I boot my laptop in Legacy BIOS mode, but have to negate the assumption that my IvyBridge XPS13 didn't need the patch in the first place. My backlight was broken, but fixed using either the first patch or vanilla 3.10.5.
Comment 98 Evan Callicoat 2013-08-08 15:41:59 UTC
@Martin perhaps I was too zealous in that assessment; I last tested the Legacy BIOS mode on 3.10.2 or .3 and it worked for me (as did the other modes). Since this is my daily driver I only tested 3.10.5 vanilla (with the patch). I will defer to your test for that mode inbetween .2 and .4
Comment 99 Daniel Vetter 2013-08-08 19:17:27 UTC
So do we need to revert the patch again? I'm completely confused what's going on here ...
Comment 100 Evan Callicoat 2013-08-08 19:37:16 UTC
@Daniel I'm not sure what the right answer is necessarily. It's easy to say "Boot in Legacy mode!" as the fix, but on an XPS 13 IVB FHD, Secure Boot requires the pure UEFI mode; it's mutually exclusive with the Legacy Option ROM/Legacy BIOS mode.

So, aside from the off-topic discussion of whether anyone cares about Secure Boot, and given that this *used* to work in pure UEFI mode until a kernel revision or two ago, I'm not sure the patch is right yet. The solution probably requires gathering drm.debug information in both modes and comparing to find out what the legacy ROM is telling the driver differently.
Comment 101 Kamal Mostafa 2013-08-08 19:43:07 UTC
> So do we need to revert the patch again?

In my opinion: We should definitely _not_ revert the patch (e85843b "drm/i915: quirk no PCH_PWM_ENABLE for Dell XPS13 backlight").

If I understand Evan Callicoat correctly, he's reporting that it breaks backlight control only for a single non-default configuration (UEFI without Legacy).  But it fixes backlight control for all other configurations, most importantly the default Legacy BIOS configuration which is what the majority of XPS13 users are actually using.

Evan's report warrants further investigation and maybe an additional patch, but lets not break the many many XPS13 users who've just gotten working backlight controls, please!
Comment 102 Evan Callicoat 2013-08-08 19:49:30 UTC
@Kamal I'm not necessarily suggesting you revert what just helped lots of folks. 

However, as I mentioned, the pure UEFI mode is the only mode available when Secure Boot is enabled, and I imagine quite a few Dell XPS13 users (all the non-DE purchasers at least) are replacing Windows 8 with Linux, meaning their BIOS will have Secure Boot on by default. Therefore I disagree with your statement that the majority aren't in the same boat as how I'd like to boot my XPS.

There's another bug affecting the pure UEFI boot since 3.10.x, https://bugzilla.kernel.org/show_bug.cgi?id=59841, which causes constant display corruption upon KMS engaging, with it's own patch that's yet to go upstream. As you can see towards the end of that bug report, some folks are now getting hit by *this* patch, with a dark screen unable to be made brighter.

I submit that we need to have a better solution to discovering what the Legacy ROM is providing that these patches (or lack thereof) are working with yet breaking without.
Comment 103 Eric Griffith 2013-08-08 21:46:16 UTC
Daniel, this ONLY breaks on IVY BRIDGE Dell XPS 13z's-- Sandy Bridge is working just fine now on 3.10.5. So if you do revert this, don't just tear it out. Just remove the PCI-checks to enable the quirk on Ivy Bridge, keep it in place for Sandy Bridge.

Now, that being said I don't have a clue what IS happening on Ivy Bridge since I don't have a Gen2 Dell XPS 13z laying around. Maybe Kamal and Evan can figure out whats going on with this one :-/ Was initially a little upset that I didn't snag an Ivy Bridge but with this maybe its a good thing.
Comment 104 Jan De Luyck 2013-08-09 05:32:14 UTC
I've been using the patch at https://bugzilla.kernel.org/attachment.cgi?id=97751 to make the backlight work on my IvyBridge XPS13, which is booting in UEFI + Legacy Rom mode.
Comment 105 VK 2013-08-19 16:31:17 UTC
My IvyBridge XPS-13 is unusable due to the backlight being turned off as of 3.10.5 (actually Fedora 19 with 3.10.5-201.fc19.x86_64 ) with UEFI plus Legacy BIOS. Backlight control has worked on this system since I got it.

If an immediate fix is not ready, can the patch be reverted until a better implementation is ready? IMHO, in the absence of a working solution, leaving users with an unusable display is more severe than waiting for a more mature implementation for brightness to work on older systems.
Comment 106 Kamal Mostafa 2013-08-19 20:49:10 UTC
Created attachment 107249 [details]
possible fix for XPS13+UEFI+Legacy

For the record: Note that some XPS13-IvyBridge+UEFI+Legacy users (Martin and others) report that the committed "quirk no PCH_PWM_ENABLE" patch fixes their previously broken backlight control, whereas other users with the same configuration (Evan Callicoat and VK) report that it breaks their backlight on startup.

I'm attaching an additional test patch here (applies on top of "drm/i915: quirk no PCH_PWM_ENABLE for Dell XPS13 backlight" already in 3.11 and 3.10).

Evan and VK, please advise whether or not this additional patch helps on your UEFI+Legacy machines.  (Other XPS13 testers may also wish to confirm that it still fixes backlight control on non-UEFI XPS13's -- my testing indicates that it does).
Comment 107 VK 2013-08-26 03:14:05 UTC
I finally got around to testing my machine (XPS13/IvyBridge/Fedora 19). I incorrectly reported  "UEFI plus Legacy BIOS". It's actually UEFI without Legacy ROM (scenario 3 in comment #96). Sorry about that.

I built 3.10.9 with the patch in attachment 107249 [details] ( comment #106 ) and it did NOT fix things. Backlight is still off after grub2 8-(

Back to 3.10.4 for me ...
Comment 108 Kamal Mostafa 2013-08-28 22:11:45 UTC
Created attachment 107347 [details]
i915.disable_pch_pwm boot param

VK, thanks anyway for testing my "possible fix" patch (so much for that idea).

Now I've attached a new patch which adds a boot parameter you can use to override the dmi table match and inhibit the quirk.  XPS13/UEFI/Legacy people experiencing the black screen at boot, please apply this patch (on top of v3.11-rc7 or similar) and try booting with "i915.disable_pch_pwm=0".  Does that fix the problem?

For reference. with this patch i915.disable_pch_pwm can be set to
 -1: auto (default; allows the disabling of pch_pwm by dmi quirk table match)
  0: inhibits the disabling of pch_pwm (overrides dmi quirk table match)
  1: forces the disabling of pch_pwm
Comment 109 VK 2013-08-30 05:19:07 UTC
Success! Used patch from comment #108 and i915.disable_pch_pwm=0 with kernel 3.11-rc7 on my Dell XPS-13/IvyBridge + UEFI.

Backlight now works for me, i.e. doesn't turn off after grub2 loads kernel (and backlight keys work fine to adjust brightness in X, as they did in 3.10.4 and prior)

Thanks!
Comment 110 Gerben Meijer 2013-09-02 15:14:27 UTC
I can also confirm that the patch in comment 108 fixes backlight on my XPS-13 FHD Ivy Bridge + UEFI with i915.disable_pch_pwm=0 (have not tested -1 or 1). Additionally, on my XPS 13 I needed patches from https://bugzilla.kernel.org/show_bug.cgi?id=59841 to resolve graphical corruption. Those together result in a working display.
Comment 111 Kamal Mostafa 2013-09-03 17:44:36 UTC
I've submitted the comment #108 patch ("drm/i915: i915.disable_pch_pwm overrides PCH_PWM_ENABLE quirk") to the intel-gfx and linux-kernel mailing lists.

I'll submit backports to stable kernels which picked up the previous XPS13 quirk patch (3.2 through 3.10) after it lands in mainline.

Thanks for the testing help folks, and thanks in advance for applying it Daniel.
Comment 112 Jani Nikula 2013-09-19 13:10:59 UTC
Created attachment 108901 [details]
get edp dpcd info

Kamal, care to run the attached patch on an affected machine, and provide dmesg with drm.debug=0xe please? Thanks.
Comment 113 Kamal Mostafa 2013-09-19 18:39:37 UTC
Created attachment 108941 [details]
get edp info results

Jani, attached is the full drm.debug output from an XPS13 FHD IvyBridge with your "get edp info" patch applied atop current post-3.12-rc1 mainline.  The new EDPx dump may be less enlightening than you'd hoped though :-) ...

 [drm:intel_dp_get_dpcd], DPCD: 11 0a 02 00 00 00 00 00 00 00 00 00 00 00 00 
 [drm:intel_dp_get_dpcd], EDP1: 00 00 00 00 
 [drm:intel_dp_get_dpcd], EDP2: 00 00 00 00 00 00 00 00 00 
 [drm:intel_dp_get_dpcd], EDP3: 00 00 00 00 00 00 00 00 00 00 

(Fwiw, I get the same EDP-all-zeros result on a v3.8-stable kernel too).
Comment 114 VK 2013-10-16 15:05:29 UTC
Just checking where this is in terms of upstream adoption?

The workaround helps me with a custom kernel I've patched (see comment #109), but I would love to see this upstream. I'm consuming Fedora 19 kernels.

I also have one request: until a permanent solution is found, can the default for whatever is submitted upstream be i915.disable_pch_pwm=0 or something similar? (FYI, I've also tried explicitly setting the default of -1 and it doesn't work)

Why? Without a setting of 0, the consequence is an unusable screen for a subset of users such as myself, whereas for those who are merely inconvenienced by not being able to use brightness keys, it seems reasonable to have to add a kernel arg to get it working.

At the moment, I'm only able to my laptop because I have an existing install and older kernels lying around. If I had to do a fresh install with a new upstream kernel, I'd be in trouble.
Comment 115 Ulf Winkelvos 2013-11-02 01:28:13 UTC
This patch should pulled upstream as the quirk still renders the display unusable on recent builds of linuz' git tree.
Comment 116 Ulf Winkelvos 2013-11-02 01:30:19 UTC
(In reply to Ulf Winkelvos from comment #115)
> This patch should pulled upstream as the quirk still renders the display
> unusable on recent builds of linuz' git tree.
(in uefi mode w/o csm)
Comment 117 Colin Guthrie 2013-11-04 16:24:58 UTC
Created attachment 113291 [details]
Updated version of i915.disable_pch_pwm command line patch for v3.12

I second Ulf's comment. v3.12 is the first kernel I've been able to boot since 3.9.8 on my XPS13+UEFI, but the backlight is busted due to the quirk.

Applying this patch and setting i915.disable_pch_pwm=0 on the command line gives me a working system!

Are there any working patches to do this automatically in the pipeline that I may have missed or can we get this quirk control patch applied in the short term?
Comment 118 Jani Nikula 2013-11-08 14:30:39 UTC
Please try backlight-rework branch of [1] on both the machines needing the current quirk and machines that the quirk breaks.

[1] git://gitorious.org/jani/drm.git
Comment 119 Ulf Winkelvos 2013-11-08 20:27:04 UTC
(In reply to Jani Nikula from comment #118)
> Please try backlight-rework branch of [1] on both the machines needing the
> current quirk and machines that the quirk breaks.
> 
> [1] git://gitorious.org/jani/drm.git
works fine on XPS 13 FHD UEFI
Comment 120 Ulf Winkelvos 2013-11-08 20:29:42 UTC
boots up in CSM too, but brightness control does not work.
Comment 121 Jani Nikula 2013-11-11 12:59:51 UTC
(In reply to Ulf Winkelvos from comment #120)
> boots up in CSM too, but brightness control does not work.

There are *so* many ways to control brightness... did you try /sys/class/backlight/intel_backlight?
Comment 122 Michael Shigorin 2013-11-11 14:16:30 UTC
(In reply to Jani Nikula from comment #121)
> There are *so* many ways to control brightness... did you try
> /sys/class/backlight/intel_backlight?
Yeah, it's most likely a separate problem; Ulf, I've finally posted https://bugs.freedesktop.org/71493, please have a look at that one.
Comment 123 Ulf Winkelvos 2013-11-12 00:34:54 UTC
In csm mode I can control brightness via intel_backlight, when I echo zero to the acpi_backlight before, but then the backlight is flickering at lower levels. Without your patch, so with the quirk mode introduced in kernel 3.10 (?) up to 3.12, I can control brighntess via acpi_backlight just fine. But in no csm setup can I control the brightness via both interfaces. In uefi mode on the contrary with those patches the brightness is controllable via both interface and setting the one proportionally chages the other.
Comment 124 Jani Nikula 2013-11-14 10:51:55 UTC
(In reply to Ulf Winkelvos from comment #123)
> In csm mode I can control brightness via intel_backlight, when I echo zero
> to the acpi_backlight before, but then the backlight is flickering at lower
> levels. Without your patch, so with the quirk mode introduced in kernel 3.10
> (?) up to 3.12, I can control brighntess via acpi_backlight just fine. But
> in no csm setup can I control the brightness via both interfaces. In uefi
> mode on the contrary with those patches the brightness is controllable via
> both interface and setting the one proportionally chages the other.

Please try this on top of linux-next branch of [1] with video.use_native_backlight=1 module parameter.

[1] http://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git
Comment 125 Ulf Winkelvos 2013-11-15 19:02:53 UTC
(In reply to Jani Nikula from comment #124)ortionally chages the other.
> 
> Please try this on top of linux-next branch of [1] with
> video.use_native_backlight=1 module parameter.
> 
> [1] http://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git

Then there is only intel_backlight, but its not working at all.
Comment 126 Ulf Winkelvos 2013-11-28 00:32:45 UTC
(In reply to Jani Nikula from comment #118)
> Please try backlight-rework branch of [1] on both the machines needing the
> current quirk and machines that the quirk breaks.
> 
> [1] git://gitorious.org/jani/drm.git
Is this going to be merged upstream by any chance?
Comment 127 Daniel Vetter 2013-11-28 13:57:54 UTC
Jani's patches are completely merged into the drm-intel-nightly branch and heading in for 3.14:

http://cgit.freedesktop.org/~danvet/drm-intel/
Comment 128 Jani Nikula 2013-12-12 18:02:21 UTC
As Daniel says, the overhauled backlight code has been merged to drm-intel-nightly. The PCH enable quirks have been removed. Any tested-by's would be appreciated.

I'm inclined to close this bug and encourage filing new bugs for any remaining issues.
Comment 129 Ulf Winkelvos 2013-12-14 00:59:40 UTC
Retestet this with drm-intel-nightly and it still works nice in uefi mode, with perfectly smooth brightness control, but brightness control in csm is acting funny. See http://www.youtube.com/watch?v=Updq6uojyCA Should i file a seperate bug for that, or wait until this hits mainline, if it still occurs then?
Comment 130 Jani Nikula 2013-12-20 12:47:18 UTC
(In reply to Ulf Winkelvos from comment #129)
> Retestet this with drm-intel-nightly and it still works nice in uefi mode,
> with perfectly smooth brightness control, but brightness control in csm is
> acting funny. See http://www.youtube.com/watch?v=Updq6uojyCA Should i file a
> seperate bug for that, or wait until this hits mainline, if it still occurs
> then?

I am not happy with how this bug ended up being handled. It has become convoluted, and we've since pretty much rewritten the code in order to be able to address issues like this in a more sane and hopefully timely manner.

I'm resolving the bug as "obsolete" but what I really mean is "let's start over with a clean sheet".

Please do file new bugs as you discover issues. We appreciate bug reports on drm-intel-nightly too, so we have a chance to fix stuff before hitting mainline.

Everyone, thanks for your patience.