Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32549 )
Change subject: soc/intel/common: Hook up GMA ACPI brightness controls ......................................................................
Patch Set 7:
(7 comments)
https://review.coreboot.org/c/coreboot/+/32549/5/src/soc/intel/common/block/... File src/soc/intel/common/block/graphics/graphics.c:
https://review.coreboot.org/c/coreboot/+/32549/5/src/soc/intel/common/block/... PS5, Line 38: }
The only platform specific part seems to be the pointer in […]
Done
https://review.coreboot.org/c/coreboot/+/32549/5/src/soc/intel/skylake/acpi/... File src/soc/intel/skylake/acpi/globalnvs.asl:
https://review.coreboot.org/c/coreboot/+/32549/5/src/soc/intel/skylake/acpi/... PS5, Line 73: BRTL, 32, // 0x54 - 0x57 Brightness Level
I don't see this used anywhere.
Ack
https://review.coreboot.org/c/coreboot/+/32549/5/src/soc/intel/skylake/acpi/... PS5, Line 74:
Put a tab here instead of spaces.
Ack
https://review.coreboot.org/c/coreboot/+/32549/5/src/soc/intel/skylake/acpi/... PS5, Line 75:
same as above.
Ack
https://review.coreboot.org/c/coreboot/+/32549/5/src/soc/intel/skylake/acpi/... PS5, Line 74: CSTE, 16, // 0x58 - 0x59 Current display state : NSTE, 16, // 0x5a - 0x5b Next display state
Why add this to GNVS? It seems they are only used in […]
Done
https://review.coreboot.org/c/coreboot/+/32549/5/src/soc/intel/skylake/graph... File src/soc/intel/skylake/graphics.c:
https://review.coreboot.org/c/coreboot/+/32549/5/src/soc/intel/skylake/graph... PS5, Line 128: 0x2
Device 2 on the System Agent is iGFX. Perhaps "SA_DEV_IGD" from pci_devs. […]
Ack
https://review.coreboot.org/c/coreboot/+/32549/5/src/soc/intel/skylake/graph... PS5, Line 130: return NULL;
...and it should only be called when the device is enabled.
Ack