Matt DeVillier has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39948 )
Change subject: nb/intel/haswell: Simplify GMA SSDT generator ......................................................................
nb/intel/haswell: Simplify GMA SSDT generator
Simplify generation of GMA SSDT, using updated naming convention. If acpi_fill_ssdt_generator is being invoked, then we know the IGD device is present and enabled, so we can skip those checks. Only check we need is if a given board has supplied the i915 controller info to populate the SSDT.
Change-Id: Icd9caf622dd4c46b13589ebb772138b25888752f Signed-off-by: Matt DeVillier matt.devillier@gmail.com --- M src/northbridge/intel/haswell/gma.c 1 file changed, 6 insertions(+), 17 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/39948/1
diff --git a/src/northbridge/intel/haswell/gma.c b/src/northbridge/intel/haswell/gma.c index 65c5cf3..afb495d 100644 --- a/src/northbridge/intel/haswell/gma.c +++ b/src/northbridge/intel/haswell/gma.c @@ -524,24 +524,13 @@ intel_gma_restore_opregion(); }
-const struct i915_gpu_controller_info *intel_gma_get_controller_info(void) +static void gma_generate_ssdt(struct device *device) { - struct device *dev = pcidev_on_root(2, 0); - if (!dev) { - return NULL; - } - struct northbridge_intel_haswell_config *chip = dev->chip_info; - return &chip->gfx; -} + const struct northbridge_intel_haswell_config *chip = device->chip_info; + const struct i915_gpu_controller_info *gfx = &chip->gfx;
-static void gma_ssdt(struct device *device) -{ - const struct i915_gpu_controller_info *gfx = intel_gma_get_controller_info(); - if (!gfx) { - return; - } - - drivers_intel_gma_displays_ssdt_generate(gfx); + if (gfx) + drivers_intel_gma_displays_ssdt_generate(gfx); }
static unsigned long gma_write_acpi_tables(struct device *const dev, unsigned long current, @@ -577,7 +566,7 @@ .set_resources = pci_dev_set_resources, .enable_resources = pci_dev_enable_resources, .init = gma_func0_init, - .acpi_fill_ssdt_generator = gma_ssdt, + .acpi_fill_ssdt_generator = gma_generate_ssdt, .scan_bus = NULL, .enable = NULL, .ops_pci = &gma_pci_ops,
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39948 )
Change subject: nb/intel/haswell: Simplify GMA SSDT generator ......................................................................
Patch Set 4: Code-Review+1
(2 comments)
https://review.coreboot.org/c/coreboot/+/39948/4/src/northbridge/intel/haswe... File src/northbridge/intel/haswell/gma.c:
https://review.coreboot.org/c/coreboot/+/39948/4/src/northbridge/intel/haswe... PS4, Line 527: device dev
https://review.coreboot.org/c/coreboot/+/39948/4/src/northbridge/intel/haswe... PS4, Line 530: chip check if this is null instead?
Hello build bot (Jenkins), Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39948
to look at the new patch set (#5).
Change subject: nb/intel/haswell: Simplify GMA SSDT generator ......................................................................
nb/intel/haswell: Simplify GMA SSDT generator
Simplify generation of GMA SSDT, using updated naming convention. If acpi_fill_ssdt_generator is being invoked, then we know the IGD device is present and enabled, so we can skip those checks. Only check we need is if a given board has supplied the i915 controller info to populate the SSDT.
Change-Id: Icd9caf622dd4c46b13589ebb772138b25888752f Signed-off-by: Matt DeVillier matt.devillier@gmail.com --- M src/northbridge/intel/haswell/gma.c 1 file changed, 5 insertions(+), 17 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/39948/5
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39948 )
Change subject: nb/intel/haswell: Simplify GMA SSDT generator ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/c/coreboot/+/39948/4/src/northbridge/intel/haswe... File src/northbridge/intel/haswell/gma.c:
https://review.coreboot.org/c/coreboot/+/39948/4/src/northbridge/intel/haswe... PS4, Line 527: device
dev
Done
https://review.coreboot.org/c/coreboot/+/39948/4/src/northbridge/intel/haswe... PS4, Line 530: chip
check if this is null instead?
Done
Hello build bot (Jenkins), Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39948
to look at the new patch set (#6).
Change subject: nb/intel/haswell: Simplify GMA SSDT generator ......................................................................
nb/intel/haswell: Simplify GMA SSDT generator
Simplify generation of GMA SSDT, using updated naming convention. If acpi_fill_ssdt is being invoked, then we know the IGD device is present and enabled, so we can skip those checks. And the SSDT generator now checks that the gfx struct is populated, so we can skip that too.
Change-Id: Icd9caf622dd4c46b13589ebb772138b25888752f Signed-off-by: Matt DeVillier matt.devillier@gmail.com --- M src/northbridge/intel/haswell/gma.c 1 file changed, 12 insertions(+), 25 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/39948/6
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39948 )
Change subject: nb/intel/haswell: Simplify GMA SSDT generator ......................................................................
Patch Set 6: Code-Review+2
Matt DeVillier has submitted this change. ( https://review.coreboot.org/c/coreboot/+/39948 )
Change subject: nb/intel/haswell: Simplify GMA SSDT generator ......................................................................
nb/intel/haswell: Simplify GMA SSDT generator
Simplify generation of GMA SSDT, using updated naming convention. If acpi_fill_ssdt is being invoked, then we know the IGD device is present and enabled, so we can skip those checks. And the SSDT generator now checks that the gfx struct is populated, so we can skip that too.
Change-Id: Icd9caf622dd4c46b13589ebb772138b25888752f Signed-off-by: Matt DeVillier matt.devillier@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/39948 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Angel Pons th3fanbus@gmail.com --- M src/northbridge/intel/haswell/gma.c 1 file changed, 12 insertions(+), 25 deletions(-)
Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved
diff --git a/src/northbridge/intel/haswell/gma.c b/src/northbridge/intel/haswell/gma.c index c0eaf6d..4c11bd1 100644 --- a/src/northbridge/intel/haswell/gma.c +++ b/src/northbridge/intel/haswell/gma.c @@ -524,24 +524,11 @@ intel_gma_restore_opregion(); }
-const struct i915_gpu_controller_info *intel_gma_get_controller_info(void) +static void gma_generate_ssdt(struct device *dev) { - struct device *dev = pcidev_on_root(2, 0); - if (!dev) { - return NULL; - } - struct northbridge_intel_haswell_config *chip = dev->chip_info; - return &chip->gfx; -} + const struct northbridge_intel_haswell_config *chip = dev->chip_info;
-static void gma_ssdt(struct device *device) -{ - const struct i915_gpu_controller_info *gfx = intel_gma_get_controller_info(); - if (!gfx) { - return; - } - - drivers_intel_gma_displays_ssdt_generate(gfx); + drivers_intel_gma_displays_ssdt_generate(&chip->gfx); }
static unsigned long gma_write_acpi_tables(struct device *const dev, unsigned long current, @@ -573,15 +560,15 @@ };
static struct device_operations gma_func0_ops = { - .read_resources = pci_dev_read_resources, - .set_resources = pci_dev_set_resources, - .enable_resources = pci_dev_enable_resources, - .init = gma_func0_init, - .acpi_fill_ssdt = gma_ssdt, - .scan_bus = NULL, - .enable = NULL, - .ops_pci = &gma_pci_ops, - .write_acpi_tables = gma_write_acpi_tables, + .read_resources = pci_dev_read_resources, + .set_resources = pci_dev_set_resources, + .enable_resources = pci_dev_enable_resources, + .init = gma_func0_init, + .acpi_fill_ssdt = gma_generate_ssdt, + .scan_bus = NULL, + .enable = NULL, + .ops_pci = &gma_pci_ops, + .write_acpi_tables = gma_write_acpi_tables, };
static const unsigned short pci_device_ids[] = {
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39948 )
Change subject: nb/intel/haswell: Simplify GMA SSDT generator ......................................................................
Patch Set 7:
Automatic boot test returned (PASS/FAIL/TOTAL): 3/0/3 Emulation targets: EMULATION_QEMU_X86_Q35 using payload TianoCore : SUCCESS : https://lava.9esec.io/r/2007 EMULATION_QEMU_X86_Q35 using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/2006 EMULATION_QEMU_X86_I440FX using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/2005
Please note: This test is under development and might not be accurate at all!
-
9elements QA (Code Review) -
Angel Pons (Code Review)
-
Matt DeVillier (Code Review)