Attention is currently required from: Nico Huber, Krishna Prabhakaran, Furquan Shaikh, Maulik V Vaghela, Selma Bensaid, Subrata Banik, Meera Ravindranath, Patrick Rudolph. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52758 )
Change subject: drivers/intel/gma: Support IGD Opregion 2.1 ......................................................................
Patch Set 19:
(3 comments)
File src/drivers/intel/gma/opregion.h:
https://review.coreboot.org/c/coreboot/+/52758/comment/0a309aac_80b12a31 PS19, Line 150: * Physical (2.0) or relative from opregion (2.1+) Alignment seems to be off (last tab before the `*` should be a space)
File src/drivers/intel/gma/opregion.c:
https://review.coreboot.org/c/coreboot/+/52758/comment/bbb58e65_d334e1cc PS14, Line 271: if (CONFIG(INTEL_GMA_OPREGION_2_1)) : return 0x201; : : return 0x200;
Sorry about the back n forth on this. […]
Another approach is to return a struct, but one needs to expose the struct type:
struct __packed opregion_version { u8 rsvd; u8 revision; u8 minor; u8 major; };
One would then declare `over` in the mailbox 0 struct as follows:
struct opregion_version over;
And the resulting function would then be:
static struct opregion_version opregion_version(void) { if (CONFIG(INTEL_GMA_OPREGION_2_1)) return (struct opregion_version) { .major = 2, .minor = 1 };
return (struct opregion_version) { .major = 2, .minor = 0 }; }
This function would then be used as follows:
opregion->header.over = opregion_version();
https://review.coreboot.org/c/coreboot/+/52758/comment/638aa9bc_68c7f87c PS14, Line 290: CONFIG(INTEL_GMA_OPREGION_2_1)
With the above change to set version, you don't need to check the config again here. […]
Hmm, Opregion 3.0 { .major = 3, .minor = 0 } won't work.
I'd just use Kconfig, but check for `!CONFIG(INTEL_GMA_OPREGION_2_0)` instead.
-
Angel Pons (Code Review)