Attention is currently required from: Felix Singer, Paul Menzel, Iru Cai.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46630 )
Change subject: mb/hp: Add EliteBook 820 G2 ......................................................................
Patch Set 10: Code-Review+1
(5 comments)
File Documentation/mainboard/hp/elitebook_820_g2.md:
https://review.coreboot.org/c/coreboot/+/46630/comment/8c3ed101_e313470d PS8, Line 115: (needs a modified refcode)
I wrote it in CB:75418, is this OK to put it on coreboot docs?
Seems OK. Would be good to add a link here
File src/mainboard/hp/elitebook_820_g2/Kconfig:
https://review.coreboot.org/c/coreboot/+/46630/comment/242d211e_f4cd8a9e PS10, Line 23: default "8086,1616" Did youn test the VGA BIOS? If not, please remove. Besides, it's highly unlikely that this applies to all Elitebook 820 G2 out there: some have an i3 and others have an i7, with the latter having a better iGPU.
File src/mainboard/hp/elitebook_820_g2/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/46630/comment/2a503230_248616a0 PS10, Line 2: register "gfx" = "GMA_STATIC_DISPLAYS(0)" : register "gpu_dp_b_hotplug" = "4" : register "gpu_dp_c_hotplug" = "4" : register "gpu_dp_d_hotplug" = "0" : register "panel_cfg" = "{ : .up_delay_ms = 200, : .down_delay_ms = 50, : .cycle_delay_ms = 500, : .backlight_on_delay_ms = 1, : .backlight_off_delay_ms = 1, : .backlight_pwm_hz = 200, : }"
I missed this somehow. Please move into the graphics controller.
The other Broadwell boards have gfx stuff at the top of the devicetree, though.
https://review.coreboot.org/c/coreboot/+/46630/comment/c20635ee_ff4bb477 PS10, Line 56: # LPC bridge nit: align with other comments
File src/mainboard/hp/elitebook_820_g2/dsdt.asl:
https://review.coreboot.org/c/coreboot/+/46630/comment/6c01af87_d1eb3763 PS10, Line 28: #include <soc/intel/broadwell/pch/acpi/pch.asl> Don't you need some include for backlight stuff?
#include <drivers/intel/gma/acpi/default_brightness_levels.asl>