Bug 111475

Summary: Brightness control inversion from VBT table is not taken into account in Ubuntu 18.10
Product: DRI Reporter: Szymon <szymon.balcerak>
Component: DRM/IntelAssignee: Intel GFX Bugs mailing list <intel-gfx-bugs>
Status: RESOLVED MOVED QA Contact: Intel GFX Bugs mailing list <intel-gfx-bugs>
Severity: normal    
Priority: not set CC: intel-gfx-bugs, jani.nikula
Version: unspecified   
Hardware: x86 (IA32)   
OS: Linux (All)   
Whiteboard: Triaged
i915 platform: CFL i915 features:
Attachments:
Description Flags
dmseg acquired while PWM inverted logic is used.
none
VBT acquired while PWM inverted logic is used. none

Description Szymon 2019-08-23 09:03:51 UTC
I’m currently struggling with LVDS panel brightness control on CoffeLake PCH H310 board with Ubuntu 18.10.

Our setup is as follows:
1. The board is equipped with Realtek RTD2136R DP to LVDS converter.
2. LVDS backlight PWM signal comes from PCH EDP_BKLTCTL (GPP_F21) output pin.
3. Board has BIOS which allows user to change LVDS PWM logic (normal/inverted) within SETUP.
4. Once user chooses “Inverted” setting, BIOS changes VBT table provided to OS. By changing VBT I mean that BIOS changes active_low_pwm bit to 1 for LVDS panel, so consequently we expect dev_priv->vbt.backlight.active_low_pwm from intel_bios.c to be set to 1.

Result:
Ubuntu does NOT take PWM inversion into consideration and brightness control behaves as in normal mode.

Expected result: 
Once PWM normal is set, LVDS brightness control should set minimum brightness for position 0 of OS brightness slider and maximum brightness for position 100 of slider. 
Once PWM inverted is set, LVDS brightness control should set maximum brightness for position 0 of OS brightness slider and minimum brightness for position 100 of slider.

Additional notes:
• Once kernel boot parameter acpi_backlight=vendor i915.invert_brightness=1 is set Ubuntu properly handles PWM inversion.
• Exactly the same BIOS & board with Windows 10 behaves correctly  -> inversion is taken into account. 
• I’m mostly familiar with Windows, not with Linux.
• I personally did not try any other Linux distribution however I assume all of them use similar graphic driver code base.
• Although problem is visible on H310 PCH I assume it's NOT PCH specific (however I haven't tried with different CFL PCH as only this H310 board is equipped with LVDS).  
• I can alter BIOS behavior if requested/needed as I'm board's BIOS developer.

Questions:
1. Could you please verify if VBT information is properly handled in Linux driver?
2. Is my understanding correct that Linux graphic driver tries to write 0xC8250 MMIO register with bit 29 once PWM inversion is set within VBT table?
3. Could you tell which Intel's documentation specify CoffeLake PCH register (0xC8250 -> i915_reg.h) to modify PWM behavior? 
4. Is modification of VBT table the right way to achieve expected results or there should be some other way applied by BIOS. 

Thanks,
Szymon
Comment 1 Jani Nikula 2019-08-23 09:58:54 UTC
We don't look at the VBT active low bit, instead we read the backlight registers and check what the BIOS has set up. This is due to historical reasons, we always preferred what was actually set up by the BIOS instead of looking at the VBT. Arguably this is a bug; if the BIOS does not set up the registers, we don't get it right.

Does your BIOS enable the display?

Please set drm.debug=14 module parameter, and attach dmesg output all the way from boot.

Please attach /sys/kernel/debug/dri/0/i915_vbt.
Comment 2 Szymon 2019-08-23 10:38:23 UTC
Created attachment 145143 [details]
dmseg acquired while PWM inverted logic is used.
Comment 3 Szymon 2019-08-23 10:39:10 UTC
Created attachment 145144 [details]
VBT acquired while PWM inverted logic is used.
Comment 4 Szymon 2019-08-23 10:43:26 UTC
I'm not sure what do you exactly mean by "BIOS enable the display". For sure LVDS display is switched on and I can see the screen. I assume BIOS enabled it.

I provided requested files as attachments.

Can you point Intel's document number which describes the content of register which should be set by BIOS?

P.S. I'm running Ubuntu from USB stick (without installation).
Comment 5 Szymon 2019-08-26 09:52:25 UTC
Today I performed another tests a here are the results:

1. Confirmed that Linux does not take VBT backlight active low bit into consideration. If this is the bug that is to be decided by Linux graphic driver developers. NOTE: Windows graphic driver takes this setting into consideration.

2. Once BIOS writes 0xC8250 MMIO register with bit 29, backlight logic is inverted correctly. However 10% minimum brightness level defined in VBT table is NOT taken into account in this case, i.e. Linux brightness slider set at maximum produces complete darkness instead of 10% brightness (as set as minimum).

3. In comparison to point 2: Once backlight logic is inverted by kernel boot parameter acpi_backlight=vendor i915.invert_brightness=1, slider at max still respects minimum brightness level 10% defined in VBT.
Comment 6 Szymon 2019-09-16 10:14:50 UTC
Is there maybe any update regarding the topic?
Comment 7 Lakshmi 2019-09-16 15:18:32 UTC
(In reply to Szymon from comment #4)
> I'm not sure what do you exactly mean by "BIOS enable the display". For sure
> LVDS display is switched on and I can see the screen. I assume BIOS enabled
> it.
> 
> I provided requested files as attachments.
> 
> Can you point Intel's document number which describes the content of
> register which should be set by BIOS?
> 
> P.S. I'm running Ubuntu from USB stick (without installation).

(In reply to Szymon from comment #5)
> Today I performed another tests a here are the results:
> 
> 1. Confirmed that Linux does not take VBT backlight active low bit into
> consideration. If this is the bug that is to be decided by Linux graphic
> driver developers. NOTE: Windows graphic driver takes this setting into
> consideration.
> 
> 2. Once BIOS writes 0xC8250 MMIO register with bit 29, backlight logic is
> inverted correctly. However 10% minimum brightness level defined in VBT
> table is NOT taken into account in this case, i.e. Linux brightness slider
> set at maximum produces complete darkness instead of 10% brightness (as set
> as minimum).
> 
> 3. In comparison to point 2: Once backlight logic is inverted by kernel boot
> parameter acpi_backlight=vendor i915.invert_brightness=1, slider at max
> still respects minimum brightness level 10% defined in VBT.


Jani, any further comments?
Comment 8 Jani Nikula 2019-09-16 18:44:21 UTC
(In reply to Szymon from comment #0)
> • I can alter BIOS behavior if requested/needed as I'm board's BIOS
> developer.

Please try to set bit 29 of register 0xc8250 in bios.
Comment 9 Szymon 2019-09-17 05:27:16 UTC
(In reply to Jani Nikula from comment #8)
> (In reply to Szymon from comment #0)
> > • I can alter BIOS behavior if requested/needed as I'm board's BIOS
> > developer.
> 
> Please try to set bit 29 of register 0xc8250 in bios.


I already did it but there is still the problem with minimum brightness. See point 2 of comment 5.
Comment 10 Jani Nikula 2019-10-04 12:35:03 UTC
(In reply to Szymon from comment #5)
> 2. Once BIOS writes 0xC8250 MMIO register with bit 29, backlight logic is
> inverted correctly. However 10% minimum brightness level defined in VBT
> table is NOT taken into account in this case, i.e. Linux brightness slider
> set at maximum produces complete darkness instead of 10% brightness (as set
> as minimum).
> 
> 3. In comparison to point 2: Once backlight logic is inverted by kernel boot
> parameter acpi_backlight=vendor i915.invert_brightness=1, slider at max
> still respects minimum brightness level 10% defined in VBT.

Err, what? Slider set at max should produce max brightness instead of darkness, right? Slider set at min should produce min brightness.

I'm at a loss with what you're trying to accomplish.

Perhaps we should take the GUI sliders out of the equation, and use /sys/class/backlight/intel_backlight/ directly.

This should set the backlight to minimum brightness:

# echo 0 > brightness

This should set the backlight to maximum brightness:

# echo $(cat max_brightness) > brightness
Comment 11 Szymon 2019-10-07 11:50:10 UTC
(In reply to Jani Nikula from comment #10)
> Err, what? Slider set at max should produce max brightness instead of
> darkness, right? Slider set at min should produce min brightness.
> 
> I'm at a loss with what you're trying to accomplish.
> 
> Perhaps we should take the GUI sliders out of the equation, and use
> /sys/class/backlight/intel_backlight/ directly.
> 
> This should set the backlight to minimum brightness:
> 
> # echo 0 > brightness
> 
> This should set the backlight to maximum brightness:
> 
> # echo $(cat max_brightness) > brightness

Sorry if I confused you by my description. Let me explain once again:
1. "Slider set at max should produce max brightness instead of darkness. Slider set at min should produce min brightness." - this general rule is of course true and expected.
2. However our board has a possibility to attach different LVDS panels. By different I mean especially different brightness polarity. I.e. 100% PWM might mean max brightness for LVDS with "normal" polarity and 100%PWM might mean min brightness for LVDS with 'inverted' polarity.
3. To still satisfy general rule from point 1, we introduced BIOS SETUP setting which allows user to change LVDS PWM logic (normal/inverted).
4. So now user having LVDS with "normal" polarity should use "normal" setting and user with LVDS with "inverted" polarity should use "normal" setting.

To perform my testes with Linux I have LVDS with normal polarity, but set "inverted" setting in SETUP (BIOS wrote 0xC8250 MMIO register with bit 29), that is why I received less brightness with higher slider position and more brightness with lower slider position. I hope now it's clearer to you.

Having this setup I received problems as in comment 5.
Comment 12 Martin Peres 2019-11-29 19:24:44 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/382.

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.