Bug 625171

Summary: nouveau: MacBook Pro backlight too dim (/sys/class/backlight/nv_backlight/max_brightness is wrong)
Product: [Fedora] Fedora Reporter: Will Woods <wwoods>
Component: kernelAssignee: Ben Skeggs <bskeggs>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: 15CC: aeneby, alexpuntob, anton, benedikt.morbach+fedora, christopherheiny, crow, davewalker, dkovalsk, dougsland, gansalmon, itamar, jfeeney, jonathan, kernel-maint, madhu.chinakonda, marius.andreiana
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2012-06-04 11:21:34 EDT Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Attachments:
Description Flags
lspci -nn -vv and dmesg output
none
Video BIOS dump (from /sys/kernel/debug/dri/0/vbios.rom)
none
linux-2.6.35-NVAF-backlight.patch
none
lspci and demsg output (ThinkPad T410i)
none
acpi brightness vs nouveau actual brightness
none
ww-nvaf-backlight-printk.patch
none
ThinkPad T410i vbios none

Description Will Woods 2010-08-18 14:48:40 EDT
Created attachment 439469 [details]
lspci -nn -vv and dmesg output

So I have a fancy new MacBookPro7,1 with a new LED-backlit screen.

/sys/class/backlight/nv_backlight/max_brightness is hardcoded to 1025 in drivers/gpu/drm/nouveau/nouveau_backlight.c. But setting brightness to 1025 puts the screen brightness at something very dim.

Increasing max_brightness to 0xffff allows for (as far as I can tell) the full range of brightness. It seems like nouveau_backlight.c needs some way to actually determine max_brightness rather than hardcoding it.

I tried the obvious thing:

  unsigned int bl = 0xffffffff;
  nv_wr32(dev, NV50_PDISPLAY_SOR_BACKLIGHT, bl);
  printk("nv_backlight: wrote %x, read %x\n", bl, 
    nv_rd32(dev, NV50_PDISPLAY_SOR_BACKLIGHT));

0xffffffff hardlocks the system; 0x80ffffff yields 0x00ffffff (which is almost assuredly a lie). 

As for guessing the actual max value - there's a noticeable difference between 44000 and 50000, but no difference that I can perceive between 50000 and 65535. So the actual max value, wherever it's hidden, is probably somewhere between 44000 and 50000 - or the brightness scales nonlinearly as it approaches 0xffff.
Comment 1 Will Woods 2010-08-18 15:40:31 EDT
Created attachment 439483 [details]
Video BIOS dump (from /sys/kernel/debug/dri/0/vbios.rom)
Comment 2 Matthew Garrett 2010-08-19 09:26:43 EDT
Ben,

Any idea how we can identify the maximum value in the backlight register? One option would be to make it conditional on the GPU family, but just wanted to check whether there was a table anywhere that provided it.
Comment 3 Ben Skeggs 2010-08-19 18:34:58 EDT
The only way I think we can do is to make it conditional on card family.  I haven't seen anything in the vbios tables that looks relevant, though, there's still a number of tables we don't know anything about, so it *could* exist.
Comment 4 Dave Walker 2010-08-27 08:08:43 EDT
I would like to confirm that setting the value to 0xffff (MAX_UNSIGNED_SHORT) allows the full range of brightness, as Will stated.
Comment 5 Will Woods 2010-08-27 18:08:13 EDT
Created attachment 441623 [details]
linux-2.6.35-NVAF-backlight.patch

This patch sets max_brightness to 0xffff if the card chipset value is 0xaf.

Possibly the comparison should be different, e.g. dev_priv >= 0xaf, but I only have one piece of hardware and very limited knowledge about how the backlight works for any others. Ben/Matthew, would this patch be acceptable?
Comment 6 Aaron Sowry 2010-09-09 10:02:55 EDT
Created attachment 446254 [details]
lspci and demsg output (ThinkPad T410i)
Comment 7 Aaron Sowry 2010-09-09 10:07:14 EDT
I have the same problem on a ThinkPad T410i (see attachment in previous comment). I get useful brightness levels on boot with power connected, but as soon as I pull the power out I get plunged into darkness. Restoring power gives me about a second of normal brightness, but then a dark screen again.

Running 'echo 100 > /proc/acpi/video/VID1/LCD0/brightness' restores full brightness, using the deprecated /proc/acpi interface.
Comment 8 Matthew Garrett 2010-09-09 10:24:42 EDT
Aaron,

That sounds like an unrelated issue.
Comment 9 Aaron Sowry 2010-09-10 10:32:30 EDT
Created attachment 446523 [details]
acpi brightness vs nouveau actual brightness

Matthew,

Issue may not be totally unrelated (if it is, please let me know an appropriate place to post), as changing the max_brightness value to 0xffff as described above *almost* fixes the issue for me - I get a much more usable range of brightness levels, but still not as high as it should be.

Brightness levels can be properly manipulated via /sys/class/backlight/acpi_video0/brightness (1-15), and an interesting comparison is the relationship between this and nv_backlight/actual_brightness (see attachment).
Comment 10 Matthew Garrett 2010-09-10 10:41:34 EDT
Ah, ok. In your case we should always be using the acpi backlight, but what I suspect happens is that on resume the nouveau backlight also attempts to reprogram its value and blows away the acpi state. Looks like yours is using 0x1ffff - Will, any chance you can try that and see if your system has a difference between 0ffff and 1ffff?

Ben:

Looks like we do need to fix this...
Comment 11 Matthew Garrett 2010-09-10 10:41:57 EDT
Possibly the switch point is the gt21x?
Comment 12 Will Woods 2010-09-10 17:29:21 EDT
Created attachment 446598 [details]
ww-nvaf-backlight-printk.patch

No difference between 0xffff and 0x1ffff. 

I patched nouveau to show the initial state of the backlight register during init - it's reliably 0x4000b42d, regardless of whether the machine is plugged in, warmboot v. coldboot, etc. 

(You can also check this just by looking at /sys/class/backlight/nv_backlight/max_brightness right after boot, as long as the system is on AC power so g-p-m doesn't mess with the backlight)

Interestingly, 0xb42d is 46125 - which falls within the maximum brightness range I observed in the original bug description.

I also note that 0xb42d shows up exactly once in the video bios - at offset 0x6e78:
  0006e70 00b4 7a00 c084 0061 b42d c000 8553 6eff
but that's probably just a coincidence.

Anyway: The attached patch does two things:

1) prints out the initial value of the backlight register, and
2) on NVaf chips, sets the max_brightness to initreg & 0xfffffff

I'd be interested to know the contents of that part of vbios / initial register contents for the ThinkPad with this problem..
Comment 13 Aaron Sowry 2010-09-12 06:51:01 EDT
Created attachment 446737 [details]
ThinkPad T410i vbios

Unfortunately the latest testing update borked gdm somehow, so I want to figure out what's going on there before I test the above patch.

I managed to salvage a vbios dump in the meantime (/sys/devices/pci0000:00/0000:00:01.0/0000:01:00.0/rom), see attached.
Comment 14 Aaron Sowry 2010-09-12 13:18:42 EDT
After applying Will's printk patch (connected to AC power):

[aaron@jules ~]$ dmesg | grep nv50
nv50: initial backlight register: 40012ccd

[aaron@jules ~]$ cat /sys/class/backlight/nv_backlight/max_brightness 
1025

(Evidently not an NVaf chip)

[aaron@jules ~]$ printf "%x\n" `cat /sys/class/backlight/nv_backlight/actual_brightness`
40012ccd

[aaron@jules ~]$ cat /sys/class/backlight/acpi_video0/max_brightness 
15

[aaron@jules ~]$ cat /sys/class/backlight/acpi_video0/actual_brightness 
13

[aaron@jules ~]$ su -c 'echo 15 > /sys/class/backlight/acpi_video0/brightness'

(There is a noticeable increase in brightness at this point)

[aaron@jules ~]$ printf "%x\n" `cat /sys/class/backlight/nv_backlight/actual_brightness`
4001df67
Comment 15 Will Woods 2010-09-27 10:39:14 EDT
Interesting - on my system there's no alternate (e.g. acpi) backlight control, so nothing else is twiddling the brightness knobs. This could explain why mine starts out at full brightness and yours apparently doesn't.

(aside: I wonder if the 0x40000000 bit indicates "this brightness level set by BIOS" or similar?)

I'd bet that the brightness register would be 0x4001df67 (or thereabouts) if you read it before anything else had a chance to mess with it (e.g. if you didn't have the ACPI module available/inserted). Not sure how you'd test that, though.

But I'm not sure whether this is a reliable method for determining max_brightness - it works for me, but mjg said that the initial max_brightness might actually be set by the firmware/startup scripts sent to the card - which would mean we're setting this value somewhere earlier in the card initialization? Except if that's true, where did we get that particular number from, anyway?

I guess the next question is: How do we go about reverse-engineering how this gets done in Mac OS X or Windows?
Comment 16 Aaron Sowry 2010-09-29 02:04:10 EDT
Judging from the screen's apparent brightness during boot, it actually appears to be 0x40012ccd until GDM fires up, at which point g-p-m dims the display relative to nouveau's incorrect max_brightness value. At no point does the display seem to dim slightly during ACPI/nouveau initialisation, so I'm assuming that the initial value is 0x40012ccd and that reading this register is not a reliable method for determining max_brightness.

The thinkpad_acpi module determines that my hardware supports a standard ACPI backlight interface, and therefore leaves the handling up to the normal ACPI video driver. I have scoured this code looking for some kind of constant, but can't find anything.

I have also disasassembled and checked the ACPI tables - nothing too interesting, except that the _BCM definition used to set brightness references a method called 'BRNS', which appears to perform a whole bunch of funky operations on its argument before storing it. Maybe someone with a bit more ASL knowledge could tell me if this is worth looking into further or not.

The main question for me is why isn't g-p-m using ACPI to set brightness levels if ACPI is available?
Comment 17 Aaron Sowry 2010-10-04 02:21:03 EDT
(In reply to comment #5)
> Created attachment 441623 [details]
> linux-2.6.35-NVAF-backlight.patch
> 
> This patch sets max_brightness to 0xffff if the card chipset value is 0xaf.
> 
> Possibly the comparison should be different, e.g. dev_priv >= 0xaf, but I only
> have one piece of hardware and very limited knowledge about how the backlight
> works for any others. Ben/Matthew, would this patch be acceptable?

if (dev_priv->chipset >= 0xa8)
	props.max_brightness = 0x4001df67;
else
	props.max_brightness = 1025;

Will,

I'm assuming that the above patch set max_brightness way too high for your hardware? But it seems weird that they would lower this value again for later chipsets. Maybe it's an Apple-specific thing?
Comment 18 Marius Andreiana 2010-12-18 12:23:51 EST
ping, hoping this will make it to F15
Comment 19 David L. Crow 2010-12-29 22:10:04 EST
I'd like to push for this to be fixed sooner than F15.  I'm running a Thinkpad T510 (same display hardware as the T410i above) and having to place

  sudo sh -c "echo 100 > /proc/acpi/video/VID1/LCD0/brightness"

in an endless loop to allow my laptop to be usable when running on battery.

If there is anything I can test or additional information I can provide, please let me know.
Comment 20 Benedikt Morbach 2011-01-08 18:11:56 EST
Could someone who is experiencing this problem try out this patch:
http://cgit.freedesktop.org/nouveau/linux-2.6/patch/?id=fcd4800dd07f0e7923b91e8a9f745f3c2a0e1dda

It solved the issue for me. (See also https://bugzilla.redhat.com/show_bug.cgi?id=666462 )

If it works for everybody, maybe it can be rolled out as an update.

Is somebody running rawhide and can confirm that the problem doesn't exist there?
At least for me that's the case.
Comment 21 David L. Crow 2011-01-12 20:50:54 EST
I loaded the latest rawhide kernel and do not see the problem there!  As an FYI, here is the dmesg output for the nouveau driver:


[    3.037606] nouveau 0000:01:00.0: power state changed by ACPI to D0
[    3.037663] nouveau 0000:01:00.0: power state changed by ACPI to D0
[    3.037680] nouveau 0000:01:00.0: PCI INT A -> GSI 16 (level, low) -> IRQ 16
[    3.037691] nouveau 0000:01:00.0: setting latency timer to 64
[    3.044772] [drm] nouveau 0000:01:00.0: Detected an NV50 generation card (0x0a8600a2)
[    3.061775] [drm] nouveau 0000:01:00.0: Attempting to load BIOS image from PRAMIN
[    3.186502] [drm] nouveau 0000:01:00.0: ... appears to be valid
[    3.186509] [drm] nouveau 0000:01:00.0: BIT BIOS found
[    3.186514] [drm] nouveau 0000:01:00.0: Bios version 70.18.45.00
[    3.186518] [drm] nouveau 0000:01:00.0: Pointer to BIT loadval table invalid
[    3.186620] [drm] nouveau 0000:01:00.0: TMDS table version 2.0
[    3.186624] [drm] nouveau 0000:01:00.0: Found Display Configuration Block version 4.0
[    3.186629] [drm] nouveau 0000:01:00.0: Raw DCB entry 0: 01800323 00010034
[    3.186634] [drm] nouveau 0000:01:00.0: Raw DCB entry 1: 02811300 00000000
[    3.186638] [drm] nouveau 0000:01:00.0: Raw DCB entry 2: 028223a6 0f220010
[    3.186642] [drm] nouveau 0000:01:00.0: Raw DCB entry 3: 02822362 00020010
[    3.186646] [drm] nouveau 0000:01:00.0: Raw DCB entry 4: 048333b6 0f220010
[    3.186650] [drm] nouveau 0000:01:00.0: Raw DCB entry 5: 04833372 00020010
[    3.186654] [drm] nouveau 0000:01:00.0: Raw DCB entry 6: 088443c6 0f220010
[    3.186658] [drm] nouveau 0000:01:00.0: Raw DCB entry 7: 08844382 00020010
[    3.186661] [drm] nouveau 0000:01:00.0: Raw DCB entry 8: 0000000e 00000000
[    3.186667] [drm] nouveau 0000:01:00.0: DCB connector table: VHER 0x40 5 16 4
[    3.186672] [drm] nouveau 0000:01:00.0:   0: 0x00000040: type 0x40 idx 0 tag 0xff
[    3.186677] [drm] nouveau 0000:01:00.0:   1: 0x00000100: type 0x00 idx 1 tag 0xff
[    3.186682] [drm] nouveau 0000:01:00.0:   2: 0x00101246: type 0x46 idx 2 tag 0x07
[    3.186687] [drm] nouveau 0000:01:00.0:   3: 0x00202346: type 0x46 idx 3 tag 0x08
[    3.186692] [drm] nouveau 0000:01:00.0:   4: 0x00410446: type 0x46 idx 4 tag 0x51
[    3.186702] [drm] nouveau 0000:01:00.0: Parsing VBIOS init table 0 at offset 0xE3E1
[    3.248035] [drm] nouveau 0000:01:00.0: 0xE6AE: Condition still not met after 20ms, skipping following opcodes
[    3.269032] [drm] nouveau 0000:01:00.0: 0xE6B2: Condition still not met after 20ms, skipping following opcodes
[    3.290053] [drm] nouveau 0000:01:00.0: 0xE838: Condition still not met after 20ms, skipping following opcodes
[    3.290067] [drm] nouveau 0000:01:00.0: Parsing VBIOS init table 1 at offset 0xE865
[    3.301069] [drm] nouveau 0000:01:00.0: Parsing VBIOS init table 2 at offset 0xF399
[    3.301085] [drm] nouveau 0000:01:00.0: Parsing VBIOS init table 3 at offset 0xF3B2
[    3.344707] [drm] nouveau 0000:01:00.0: Parsing VBIOS init table 4 at offset 0xF499
[    3.344724] [drm] nouveau 0000:01:00.0: Parsing VBIOS init table at offset 0xF4FE
[    3.365039] [drm] nouveau 0000:01:00.0: 0xE255: Condition still not met after 20ms, skipping following opcodes
[    3.385805] [drm] nouveau 0000:01:00.0: 3 available performance level(s)
[    3.385813] [drm] nouveau 0000:01:00.0: 0: memory 135MHz core 135MHz shader 270MHz voltage 850mV
[    3.385819] [drm] nouveau 0000:01:00.0: 1: memory 405MHz core 405MHz shader 810MHz voltage 850mV
[    3.385826] [drm] nouveau 0000:01:00.0: 3: memory 790MHz core 606MHz shader 1468MHz voltage 1000mV
[    3.385852] [drm] nouveau 0000:01:00.0: c: memory 950MHz core 550MHz shader 200MHz
[    3.385926] [drm] nouveau 0000:01:00.0: Detected 512MiB VRAM
[    3.493823] [drm] nouveau 0000:01:00.0: 512 MiB GART (aperture)
[    4.550767] [drm] nouveau 0000:01:00.0: Allocating FIFO number 1
[    4.568177] [drm] nouveau 0000:01:00.0: nouveau_channel_alloc: initialised FIFO 1
[    4.568222] [drm] nouveau 0000:01:00.0: ACPI backlight interface available, not registering our own
[    4.881487] [drm] nouveau 0000:01:00.0: allocated 1920x1080 fb: 0x40260000, bo ffff88012b4fe400
[    4.881609] fbcon: nouveaufb (fb0) is primary device
[    4.894342] fb0: nouveaufb frame buffer device
[    4.894354] [drm] Initialized nouveau 0.0.16 20090420 for 0000:01:00.0 on minor 0
[   27.586195] [drm] nouveau 0000:01:00.0: Allocating FIFO number 2
[   27.599019] [drm] nouveau 0000:01:00.0: nouveau_channel_alloc: initialised FIFO 2
Comment 22 Christopher Heiny 2011-04-06 02:09:50 EDT
I'm experiencing this issue with Fedora 14 on a Thinkpad W510.  It'd be nice if we didn't have to wait till F15 for a fix.
Comment 23 Ben Skeggs 2011-04-06 18:53:20 EDT
*** Bug 666462 has been marked as a duplicate of this bug. ***
Comment 24 Marius Andreiana 2011-05-21 10:56:33 EDT
Still doesn't work in F15, brightness keys display the visual indicator, but have no effect.
Comment 25 Alessandro Bramucci 2011-05-23 04:05:01 EDT
Same problem in rawhide.
I noticed that in my previous installation of F15 alpha was working!

macbook_4.1
kernel-2.6.39-1.fc16.x86_64
Comment 26 Alessandro Bramucci 2011-06-09 13:34:03 EDT
I think my problem was different, in fact after I have checked the '/var/log/secure' I have found the following error:

tron pkexec[3969]: alex: The value for the SHELL variable was not found the /etc/shells file [USER=root] [TTY=unknown] [CWD=/home/alex] [COMMAND=/usr/sbin/gnome-power-backlight-helper --set-brightness 969]

so I have solved simply adding '/bin/bash' in the '/etc/shell' file.
Comment 27 Josh Boyer 2012-06-04 11:21:34 EDT
This should have been fixed in F16 or newer.