Bug 112014 - i915: intel_dp_aux_backlight max_brightness set to incorrect value on Thinkpad P1 Gen2 AMOLED
Summary: i915: intel_dp_aux_backlight max_brightness set to incorrect value on Thinkpa...
Status: RESOLVED MOVED
Alias: None
Product: DRI
Classification: Unclassified
Component: DRM/Intel (show other bugs)
Version: DRI git
Hardware: x86-64 (AMD64) Linux (All)
: not set normal
Assignee: Intel GFX Bugs mailing list
QA Contact: Intel GFX Bugs mailing list
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-10-16 01:17 UTC by Josh Farwell
Modified: 2019-11-29 19:40 UTC (History)
3 users (show)

See Also:
i915 platform:
i915 features: display/backlight


Attachments

Description Josh Farwell 2019-10-16 01:17:46 UTC
I found an interesting issue concerning the backlight on my new laptop. I am running Fedora 30 and replicated the issue with 5.2.17 and 5.3.5 kernels. Hardware is as follows:

Thinkpad P1 Gen2
- i9-9880k
- Nvidia Optimus with Quadro T2000
- 4K AMOLED Touch display with eDP display connector

I would not be surprised at all if the Thinkpad X1 Extreme Gen 2 with AMOLED has the same problem, they're pretty much exactly the same hardware apparently.

This laptop's display uses AMOLED and does not have a traditional backlight. We get backlight control by turning on the enable_dpcd_backlight parameter in the i915 module and using PWM based backlight control. I dropped this config in /etc/modprobe.d/i915.conf to enable this feature:

```
[josh@lappy-mk5 linux-stable]$ cat /etc/modprobe.d/i915.conf 
options i915 enable_dpcd_backlight
```

When I turned it on, I discovered that the brightness controls were very wonky. As I triggered the "brightness up" ACPI event, the backlight was switching between low brightness and high brightness with every key press, and the brightness level seemed random. There were no errors found in dmesg.

The backlight control is found in /sys/devices/pci0000:00/0000:00:02.0/drm/card0/card0-eDP-1/intel_backlight on my system. max_brightness is set to 65535, which i've seen on other systems. I discovered through some tinkering that if we pass integers 0 - 2047 to intel_backlight/brightness, the brightness scales linearly. Once we pass 2048 to the brightness function, the brightness drops back down to the same level as it was at 0. This happens again at 4097 where it is at maximum brightness, and 4098 brings the brightness level back to zero again. This sequence repeats 30 more times until we are at maximum brightness at 65535.

I traced this issue to this function in drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c (line 228-245 on 5.3.5 stable):

```
static int intel_dp_aux_setup_backlight(struct intel_connector *connector,
                                        enum pipe pipe)
{
        struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base);
        struct intel_panel *panel = &connector->panel;

        if (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_BYTE_COUNT)
                panel->backlight.max = 0xFFFF;
        else
                panel->backlight.max = 0xFF;

        panel->backlight.min = 0;
        panel->backlight.level = intel_dp_aux_get_backlight(connector);

        panel->backlight.enabled = panel->backlight.level != 0;

        return 0;
}
```

When I hack this function and replace 0xFFFF with 0x7FF, my backlight works perfectly with no problems.

It seems like this function needs a third branch in that conditional that knows something specific about this laptop's display and can set the max_brightness to 2047. I don't know enough about the rest of the driver to write a patch for this, but I am happy to help by collecting debug information and testing patches. Just let me know what I should provide next.
Comment 1 scachemaille 2019-11-06 13:53:04 UTC
I confirm the same problem on my Thinkpad X1 extrem Gen2. with a 4k OLED display
Comment 2 Lakshmi 2019-11-07 10:38:19 UTC
(In reply to Josh Farwell from comment #0)
> I found an interesting issue concerning the backlight on my new laptop. I am
> running Fedora 30 and replicated the issue with 5.2.17 and 5.3.5 kernels.
> Hardware is as follows:
> 
> Thinkpad P1 Gen2
> - i9-9880k
> - Nvidia Optimus with Quadro T2000
> - 4K AMOLED Touch display with eDP display connector
> 
> I would not be surprised at all if the Thinkpad X1 Extreme Gen 2 with AMOLED
> has the same problem, they're pretty much exactly the same hardware
> apparently.
> 
> This laptop's display uses AMOLED and does not have a traditional backlight.
> We get backlight control by turning on the enable_dpcd_backlight parameter
> in the i915 module and using PWM based backlight control. I dropped this
> config in /etc/modprobe.d/i915.conf to enable this feature:
> 
> ```
> [josh@lappy-mk5 linux-stable]$ cat /etc/modprobe.d/i915.conf 
> options i915 enable_dpcd_backlight
> ```
> 
> When I turned it on, I discovered that the brightness controls were very
> wonky. As I triggered the "brightness up" ACPI event, the backlight was
> switching between low brightness and high brightness with every key press,
> and the brightness level seemed random. There were no errors found in dmesg.
> 
> The backlight control is found in
> /sys/devices/pci0000:00/0000:00:02.0/drm/card0/card0-eDP-1/intel_backlight
> on my system. max_brightness is set to 65535, which i've seen on other
> systems. I discovered through some tinkering that if we pass integers 0 -
> 2047 to intel_backlight/brightness, the brightness scales linearly. Once we
> pass 2048 to the brightness function, the brightness drops back down to the
> same level as it was at 0. This happens again at 4097 where it is at maximum
> brightness, and 4098 brings the brightness level back to zero again. This
> sequence repeats 30 more times until we are at maximum brightness at 65535.
> 
> I traced this issue to this function in
> drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c (line 228-245 on 5.3.5
> stable):
> 
> ```
> static int intel_dp_aux_setup_backlight(struct intel_connector *connector,
>                                         enum pipe pipe)
> {
>         struct intel_dp *intel_dp =
> enc_to_intel_dp(&connector->encoder->base);
>         struct intel_panel *panel = &connector->panel;
> 
>         if (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_BYTE_COUNT)
>                 panel->backlight.max = 0xFFFF;
>         else
>                 panel->backlight.max = 0xFF;
> 
>         panel->backlight.min = 0;
>         panel->backlight.level = intel_dp_aux_get_backlight(connector);
> 
>         panel->backlight.enabled = panel->backlight.level != 0;
> 
>         return 0;
> }
> ```
> 
> When I hack this function and replace 0xFFFF with 0x7FF, my backlight works
> perfectly with no problems.
> 
> It seems like this function needs a third branch in that conditional that
> knows something specific about this laptop's display and can set the
> max_brightness to 2047. I don't know enough about the rest of the driver to
> write a patch for this, but I am happy to help by collecting debug
> information and testing patches. Just let me know what I should provide next.

@Josh, Which platform is this?

CC'ing Jani.
Comment 3 scachemaille 2019-11-07 13:32:26 UTC
in my case this is my system:

System:    Host: myThinkPad Kernel: 5.3.8-3-MANJARO x86_64 bits: 64 compiler: gcc v: 9.2.0 Desktop: KDE Plasma 5.17.2 
           tk: Qt 5.13.2 wm: kwin_x11 dm: SDDM Distro: Manjaro Linux 
Machine:   Type: Laptop System: LENOVO product: 20QVCTO1WW v: ThinkPad X1 Extreme 2nd serial: <filter> Chassis: type: 10 
           serial: <filter> 
           Mobo: LENOVO model: 20QVCTO1WW v: SDK0T08861 WIN serial: <filter> UEFI: LENOVO v: N2OET39W (1.26 ) date: 09/19/2019 
Battery:   ID-1: BAT0 charge: 81.1 Wh condition: 81.1/80.4 Wh (101%) volts: 17.4/15.4 model: Celxpert 5B10V98091 
           serial: <filter> status: Full 
CPU:       Topology: 8-Core model: Intel Core i9-9880H bits: 64 type: MT MCP arch: Kaby Lake rev: D L2 cache: 16.0 MiB 
           flags: avx avx2 lm nx pae sse sse2 sse3 sse4_1 sse4_2 ssse3 vmx bogomips: 73625 
           Speed: 2300 MHz min/max: 800/2300 MHz Core speeds (MHz): 1: 2300 2: 2300 3: 2300 4: 2300 5: 2300 6: 2300 7: 2300 
           8: 2300 9: 2300 10: 2300 11: 2300 12: 2300 13: 2300 14: 2300 15: 2300 16: 2300 
Graphics:  Device-1: Intel UHD Graphics 630 vendor: Lenovo driver: i915 v: kernel bus ID: 00:02.0 chip ID: 8086:3e9b 
           Device-2: NVIDIA vendor: Lenovo driver: nvidia v: 440.26 bus ID: 01:00.0 chip ID: 10de:1f91 
           Display: x11 server: X.Org 1.20.5 driver: modesetting,nvidia compositor: kwin_x11 tty: N/A 
           OpenGL: renderer: Mesa DRI Intel UHD Graphics 630 (Coffeelake 3x8 GT2) v: 4.5 Mesa 19.2.2 compat-v: 3.0 
           direct render: Yes
Comment 5 scachemaille 2019-11-12 11:11:40 UTC
(In reply to Jani Nikula from comment #4)
> Does this help?
> 
> http://patchwork.freedesktop.org/patch/msgid/20190618062628.133783-1-
> furquan@google.com

Helllo,
I just tested the above patch on my Thinkpad X1 extreme Gen 2. on manjaro with the kernel 5.3.10 (the specs of my computer in my previous post)

And It seems to works fine on my system

Now the max brightness reported in /sys/devices/pci0000:00/0000:00:02.0/drm/card0/card0-eDP-1/intel_backlight/max_brightness is 2047 and the brightness control works as inteded on my plasma desktop and my 4k OLED display.

if you need more informations let me now.
Comment 6 scachemaille 2019-11-12 11:33:37 UTC
(In reply to Jani Nikula from comment #4)
> Does this help?
> 
> http://patchwork.freedesktop.org/patch/msgid/20190618062628.133783-1-
> furquan@google.com

just to be clear.. 
I 'm not used about the patch review.. and I'm not sure the version of the patch I tested.. I saw later there was different version.

I tested the patch directly accessible with the link above.

But I don't think it makes much difference.
Comment 7 Lyude Paul 2019-11-18 22:43:26 UTC
Hi! Jani Nakula pointed me towards this when I started asking around about the AMOLED backlight issues I'm seeing on an X1 Extreme 2nd Gen laptop that I got for testing purposes. While the patch that they linked here isn't enough, I've done a bit of experimenting and I managed to hack together something that actually works on my X1 extreme 2nd gen. There seem to be three different underlying issues here:

- The first, as the reporter here pointed out, is that we're not setting the max/min backlight correctly
- The second is that i915 with these patches appears to program DP_EDP_PWMGEN_BIT_COUNT at the start of the driver load, but never again. This doesn't seem to be correct since the panel forgets this value once it's been switched into D3 mode, so it needs to be programmed when enabling the backlight after the panel was previously shut off as well
- The third is that there appears to be a mandatory delay time that occurs after the initial link training process, before we're able to properly program the backlight registers over DPCD. Not sure what this is (msleep(500) seems to work..l.), but I'm assuming it's probably the same as the normal panel power cycle delays.

I'll try writing up some patches for this in a short bit and link to them here
Comment 8 Lyude Paul 2019-11-22 23:22:43 UTC
(In reply to scachemaille from comment #6)
> (In reply to Jani Nikula from comment #4)
> > Does this help?
> > 
> > http://patchwork.freedesktop.org/patch/msgid/20190618062628.133783-1-
> > furquan@google.com
> 
> just to be clear.. 
> I 'm not used about the patch review.. and I'm not sure the version of the
> patch I tested.. I saw later there was different version.
> 
> I tested the patch directly accessible with the link above.
> 
> But I don't think it makes much difference.

Hi! I posted the fixes I was talking about here:

https://patchwork.freedesktop.org/series/69914/

Would you mind testing them and letting me know if this fixes your issues?
Comment 9 scachemaille 2019-11-23 00:12:30 UTC
(In reply to Lyude Paul from comment #8)
> (In reply to scachemaille from comment #6)
> > (In reply to Jani Nikula from comment #4)
> > > Does this help?
> > > 
> > > http://patchwork.freedesktop.org/patch/msgid/20190618062628.133783-1-
> > > furquan@google.com
> > 
> > just to be clear.. 
> > I 'm not used about the patch review.. and I'm not sure the version of the
> > patch I tested.. I saw later there was different version.
> > 
> > I tested the patch directly accessible with the link above.
> > 
> > But I don't think it makes much difference.
> 
> Hi! I posted the fixes I was talking about here:
> 
> https://patchwork.freedesktop.org/series/69914/
> 
> Would you mind testing them and letting me know if this fixes your issues?

I will test as soon as possible.
I just have a question: 
There is the patch: "Force DPCD backlight mode on X1 Extreme 2nd Gen 4K AMOLED panel" and "Auto detect DPCD backlight support by default" 

Does that mean if should work without the need to set the "enable_dpcd_backlight"  in the module option. it should be automatic?
Comment 10 scachemaille 2019-11-23 21:03:46 UTC
(In reply to Lyude Paul from comment #8)
> (In reply to scachemaille from comment #6)
> > (In reply to Jani Nikula from comment #4)
> > > Does this help?
> > > 
> > > http://patchwork.freedesktop.org/patch/msgid/20190618062628.133783-1-
> > > furquan@google.com
> > 
> > just to be clear.. 
> > I 'm not used about the patch review.. and I'm not sure the version of the
> > patch I tested.. I saw later there was different version.
> > 
> > I tested the patch directly accessible with the link above.
> > 
> > But I don't think it makes much difference.
> 
> Hi! I posted the fixes I was talking about here:
> 
> https://patchwork.freedesktop.org/series/69914/
> 
> Would you mind testing them and letting me know if this fixes your issues?

Hello,

I tested your patches with the Manjaro kernel 5.4rc8.d1117.gaf42d34-1 (I would have to modify the patches for kernel 5.3.12)
I removed the setting for the options "enable_dpcd_backlight".

rebooted, and the brightness works as intended out of the box. 
I put my laptop in suspend mode, and all works when back to normal mode.
I power off and on my laptop and the brightness value was like as when I turned it off.

All seems to works.

If you need specifics test. or specific information in return, let me know.

best regards
Comment 11 Martin Peres 2019-11-29 19:40:30 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/drm/intel/issues/510.


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.