Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30567 )
Change subject: nb/intel/sandybridge: Use understandable values for PWM backlight ......................................................................
Patch Set 5:
(6 comments)
https://review.coreboot.org/#/c/30567/5/src/mainboard/lenovo/t430s/variants/... File src/mainboard/lenovo/t430s/variants/t431s/overridetree.cb:
https://review.coreboot.org/#/c/30567/5/src/mainboard/lenovo/t430s/variants/... PS5, Line 14: pwm_freq_hz setting missing here?
https://review.coreboot.org/#/c/30567/5/src/northbridge/intel/sandybridge/gm... File src/northbridge/intel/sandybridge/gma.c:
https://review.coreboot.org/#/c/30567/5/src/northbridge/intel/sandybridge/gm... PS5, Line 596: 0xc8250 SBLC_BLM_CTL1
https://review.coreboot.org/#/c/30567/5/src/northbridge/intel/sandybridge/gm... PS5, Line 605: if (gtt_read(0xc8250) & (1 << 30)) from the datasheet "A value of zero will turn the backlight off." - is this true for both states of gtt_read(0xc8250) & (1 << 30)?
https://review.coreboot.org/#/c/30567/5/src/northbridge/intel/sandybridge/gm... PS5, Line 605: 0xc8250 SBLC_BLM_CTL1
https://review.coreboot.org/#/c/30567/5/src/northbridge/intel/sandybridge/gm... PS5, Line 605: (1 << 30) is this bit also present in ivy bridge? only saw that in the register description for sandy bridge. i also wonder where this bit gets set and if it gets set at all
https://review.coreboot.org/#/c/30567/5/src/northbridge/intel/sandybridge/gm... PS5, Line 608: 0xc8254 SBLC_BLM_CTL2