Matt DeVillier has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/40712 )
Change subject: drivers/intel/gma: drop gma_set_gnvs_aslb() ......................................................................
drivers/intel/gma: drop gma_set_gnvs_aslb()
At one point this was called in opregion.c, but now only exists as duplicated code in several platforms/SoCs that for most turns a single line of code into a 4-line function, and for others is simply unused. Drop it and clean up the mess.
Change-Id: Ida23ca5ecf1821e019ed850d1eec2a858bd2e50f Signed-off-by: Matt DeVillier matt.devillier@gmail.com --- M src/drivers/intel/gma/opregion.h M src/northbridge/intel/gm45/gma.c M src/northbridge/intel/haswell/gma.c M src/northbridge/intel/i945/gma.c M src/northbridge/intel/ironlake/gma.c M src/northbridge/intel/pineview/gma.c M src/northbridge/intel/sandybridge/gma.c M src/northbridge/intel/x4x/gma.c M src/soc/intel/baytrail/gfx.c M src/soc/intel/braswell/gfx.c M src/soc/intel/broadwell/igd.c M src/soc/intel/skylake/graphics.c 12 files changed, 9 insertions(+), 87 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/40712/1
diff --git a/src/drivers/intel/gma/opregion.h b/src/drivers/intel/gma/opregion.h index 4967691..d49424d 100644 --- a/src/drivers/intel/gma/opregion.h +++ b/src/drivers/intel/gma/opregion.h @@ -246,7 +246,6 @@ void intel_gma_opregion_register(uintptr_t opregion); void intel_gma_restore_opregion(void); uintptr_t gma_get_gnvs_aslb(const void *gnvs); -void gma_set_gnvs_aslb(void *gnvs, uintptr_t aslb); enum cb_err intel_gma_init_igd_opregion(igd_opregion_t *opregion);
/* diff --git a/src/northbridge/intel/gm45/gma.c b/src/northbridge/intel/gm45/gma.c index 25f7518..4a6ea63 100644 --- a/src/northbridge/intel/gm45/gma.c +++ b/src/northbridge/intel/gm45/gma.c @@ -38,13 +38,6 @@ return (uintptr_t)(gnvs_ptr ? gnvs_ptr->aslb : 0); }
-void gma_set_gnvs_aslb(void *gnvs, uintptr_t aslb) -{ - global_nvs_t *gnvs_ptr = gnvs; - if (gnvs_ptr) - gnvs_ptr->aslb = aslb; -} - static u32 get_cdclk(struct device *const dev) { const u16 cdclk_sel = @@ -232,7 +225,7 @@ gnvs = cbmem_find(CBMEM_ID_ACPI_GNVS); if (gnvs) { /* IGD OpRegion Base Address */ - gma_set_gnvs_aslb(gnvs, (uintptr_t)opregion); + gnvs->aslb = (uintptr_t)opregion); } else { printk(BIOS_ERR, "Error: GNVS table not found.\n"); } diff --git a/src/northbridge/intel/haswell/gma.c b/src/northbridge/intel/haswell/gma.c index c6b8fab..f325109 100644 --- a/src/northbridge/intel/haswell/gma.c +++ b/src/northbridge/intel/haswell/gma.c @@ -216,13 +216,6 @@ return (uintptr_t)(gnvs_ptr ? gnvs_ptr->aslb : 0); }
-void gma_set_gnvs_aslb(void *gnvs, uintptr_t aslb) -{ - global_nvs_t *gnvs_ptr = gnvs; - if (gnvs_ptr) - gnvs_ptr->aslb = aslb; -} - static void power_well_enable(void) { gtt_write(HSW_PWR_WELL_CTL1, HSW_PWR_WELL_ENABLE); @@ -535,7 +528,7 @@ gnvs = cbmem_find(CBMEM_ID_ACPI_GNVS); if (gnvs) { /* IGD OpRegion Base Address */ - gma_set_gnvs_aslb(gnvs, (uintptr_t)opregion); + gnvs->aslb = (uintptr_t)opregion); } else { printk(BIOS_ERR, "Error: GNVS table not found.\n"); } diff --git a/src/northbridge/intel/i945/gma.c b/src/northbridge/intel/i945/gma.c index 8a19b3e..c9105a1 100644 --- a/src/northbridge/intel/i945/gma.c +++ b/src/northbridge/intel/i945/gma.c @@ -50,13 +50,6 @@ return (uintptr_t)(gnvs_ptr ? gnvs_ptr->aslb : 0); }
-void gma_set_gnvs_aslb(void *gnvs, uintptr_t aslb) -{ - global_nvs_t *gnvs_ptr = gnvs; - if (gnvs_ptr) - gnvs_ptr->aslb = aslb; -} - static int gtt_setup(u8 *mmiobase) { unsigned long PGETBL_save; @@ -781,7 +774,7 @@ gnvs = cbmem_find(CBMEM_ID_ACPI_GNVS); if (gnvs) { /* IGD OpRegion Base Address */ - gma_set_gnvs_aslb(gnvs, (uintptr_t)opregion); + gnvs->aslb = (uintptr_t)opregion); } else { printk(BIOS_ERR, "Error: GNVS table not found.\n"); } diff --git a/src/northbridge/intel/ironlake/gma.c b/src/northbridge/intel/ironlake/gma.c index c8bbbfd..f2680a5 100644 --- a/src/northbridge/intel/ironlake/gma.c +++ b/src/northbridge/intel/ironlake/gma.c @@ -71,13 +71,6 @@ return (uintptr_t)(gnvs_ptr ? gnvs_ptr->aslb : 0); }
-void gma_set_gnvs_aslb(void *gnvs, uintptr_t aslb) -{ - global_nvs_t *gnvs_ptr = gnvs; - if (gnvs_ptr) - gnvs_ptr->aslb = aslb; -} - static void gma_pm_init_post_vbios(struct device *dev) { struct northbridge_intel_ironlake_config *conf = dev->chip_info; @@ -227,7 +220,7 @@ gnvs = cbmem_find(CBMEM_ID_ACPI_GNVS); if (gnvs) { /* IGD OpRegion Base Address */ - gma_set_gnvs_aslb(gnvs, (uintptr_t)opregion); + gnvs->aslb = (uintptr_t)opregion); } else { printk(BIOS_ERR, "Error: GNVS table not found.\n"); } diff --git a/src/northbridge/intel/pineview/gma.c b/src/northbridge/intel/pineview/gma.c index d398b55..5e541c0 100644 --- a/src/northbridge/intel/pineview/gma.c +++ b/src/northbridge/intel/pineview/gma.c @@ -49,13 +49,6 @@ return (uintptr_t)(gnvs_ptr ? gnvs_ptr->aslb : 0); }
-void gma_set_gnvs_aslb(void *gnvs, uintptr_t aslb) -{ - global_nvs_t *gnvs_ptr = gnvs; - if (gnvs_ptr) - gnvs_ptr->aslb = aslb; -} - static int gtt_setup(u8 *mmiobase) { u32 gttbase; @@ -293,7 +286,7 @@ gnvs = cbmem_find(CBMEM_ID_ACPI_GNVS); if (gnvs) { /* IGD OpRegion Base Address */ - gma_set_gnvs_aslb(gnvs, (uintptr_t)opregion); + gnvs->aslb = (uintptr_t)opregion); } else { printk(BIOS_ERR, "Error: GNVS table not found.\n"); } diff --git a/src/northbridge/intel/sandybridge/gma.c b/src/northbridge/intel/sandybridge/gma.c index 5c4f548..aafb193 100644 --- a/src/northbridge/intel/sandybridge/gma.c +++ b/src/northbridge/intel/sandybridge/gma.c @@ -309,13 +309,6 @@ return (uintptr_t)(gnvs_ptr ? gnvs_ptr->aslb : 0); }
-void gma_set_gnvs_aslb(void *gnvs, uintptr_t aslb) -{ - global_nvs_t *gnvs_ptr = gnvs; - if (gnvs_ptr) - gnvs_ptr->aslb = aslb; -} - static void gma_pm_init_pre_vbios(struct device *dev) { u32 reg32; @@ -662,7 +655,7 @@ gnvs = cbmem_find(CBMEM_ID_ACPI_GNVS); if (gnvs) { /* IGD OpRegion Base Address */ - gma_set_gnvs_aslb(gnvs, (uintptr_t)opregion); + gnvs->aslb = (uintptr_t)opregion); } else { printk(BIOS_ERR, "Error: GNVS table not found.\n"); } diff --git a/src/northbridge/intel/x4x/gma.c b/src/northbridge/intel/x4x/gma.c index f5335ec..6496e0e 100644 --- a/src/northbridge/intel/x4x/gma.c +++ b/src/northbridge/intel/x4x/gma.c @@ -34,13 +34,6 @@ return (uintptr_t)(gnvs_ptr ? gnvs_ptr->aslb : 0); }
-void gma_set_gnvs_aslb(void *gnvs, uintptr_t aslb) -{ - global_nvs_t *gnvs_ptr = gnvs; - if (gnvs_ptr) - gnvs_ptr->aslb = aslb; -} - static void gma_func0_init(struct device *dev) { u32 reg32; @@ -104,7 +97,7 @@ gnvs = cbmem_find(CBMEM_ID_ACPI_GNVS); if (gnvs) { /* IGD OpRegion Base Address */ - gma_set_gnvs_aslb(gnvs, (uintptr_t)opregion); + gnvs->aslb = (uintptr_t)opregion); } else { printk(BIOS_ERR, "Error: GNVS table not found.\n"); } diff --git a/src/soc/intel/baytrail/gfx.c b/src/soc/intel/baytrail/gfx.c index 64c9061..e0f25bf 100644 --- a/src/soc/intel/baytrail/gfx.c +++ b/src/soc/intel/baytrail/gfx.c @@ -359,13 +359,6 @@ return (uintptr_t)(gnvs_ptr ? gnvs_ptr->aslb : 0); }
-void gma_set_gnvs_aslb(void *gnvs, uintptr_t aslb) -{ - global_nvs_t *gnvs_ptr = gnvs; - if (gnvs_ptr) - gnvs_ptr->aslb = aslb; -} - static void gfx_init(struct device *dev) { /* Pre VBIOS Init */ @@ -410,7 +403,7 @@ gnvs = cbmem_find(CBMEM_ID_ACPI_GNVS); if (gnvs) { /* IGD OpRegion Base Address */ - gma_set_gnvs_aslb(gnvs, (uintptr_t)opregion); + gnvs->aslb = (uintptr_t)opregion); } else { printk(BIOS_ERR, "Error: GNVS table not found.\n"); } diff --git a/src/soc/intel/braswell/gfx.c b/src/soc/intel/braswell/gfx.c index d6671f6..046dad2 100644 --- a/src/soc/intel/braswell/gfx.c +++ b/src/soc/intel/braswell/gfx.c @@ -70,13 +70,6 @@ return (uintptr_t)(gnvs_ptr ? gnvs_ptr->aslb : 0); }
-void gma_set_gnvs_aslb(void *gnvs, uintptr_t aslb) -{ - global_nvs_t *gnvs_ptr = gnvs; - if (gnvs_ptr) - gnvs_ptr->aslb = aslb; -} - static void gma_generate_ssdt(struct device *dev) { const struct soc_intel_braswell_config *chip = dev->chip_info; diff --git a/src/soc/intel/broadwell/igd.c b/src/soc/intel/broadwell/igd.c index dbb4205..b0ee605 100644 --- a/src/soc/intel/broadwell/igd.c +++ b/src/soc/intel/broadwell/igd.c @@ -498,13 +498,6 @@ return (uintptr_t)(gnvs_ptr ? gnvs_ptr->aslb : 0); }
-void gma_set_gnvs_aslb(void *gnvs, uintptr_t aslb) -{ - global_nvs_t *gnvs_ptr = gnvs; - if (gnvs_ptr) - gnvs_ptr->aslb = aslb; -} - static void igd_init(struct device *dev) { int is_broadwell = !!(cpu_family_model() == BROADWELL_FAMILY_ULT); @@ -607,7 +600,7 @@ gnvs = cbmem_find(CBMEM_ID_ACPI_GNVS); if (gnvs) { /* IGD OpRegion Base Address */ - gma_set_gnvs_aslb(gnvs, (uintptr_t)opregion); + gnvs->aslb = (uintptr_t)opregion); } else { printk(BIOS_ERR, "Error: GNVS table not found.\n"); } diff --git a/src/soc/intel/skylake/graphics.c b/src/soc/intel/skylake/graphics.c index c338a67..b7db45c 100644 --- a/src/soc/intel/skylake/graphics.c +++ b/src/soc/intel/skylake/graphics.c @@ -134,13 +134,6 @@ return (uintptr_t)(gnvs_ptr ? gnvs_ptr->aslb : 0); }
-void gma_set_gnvs_aslb(void *gnvs, uintptr_t aslb) -{ - global_nvs_t *gnvs_ptr = gnvs; - if (gnvs_ptr) - gnvs_ptr->aslb = aslb; -} - /* Initialize IGD OpRegion, called from ACPI code */ static void update_igd_opregion(igd_opregion_t *opregion) {
Hello build bot (Jenkins), Damien Zammit, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40712
to look at the new patch set (#2).
Change subject: drivers/intel/gma: drop gma_set_gnvs_aslb() ......................................................................
drivers/intel/gma: drop gma_set_gnvs_aslb()
At one point this was called in opregion.c, but now only exists as duplicated code in several platforms/SoCs that for most turns a single line of code into a 4-line function, and for others is simply unused. Drop it and clean up the mess.
Change-Id: Ida23ca5ecf1821e019ed850d1eec2a858bd2e50f Signed-off-by: Matt DeVillier matt.devillier@gmail.com --- M src/drivers/intel/gma/opregion.h M src/northbridge/intel/gm45/gma.c M src/northbridge/intel/haswell/gma.c M src/northbridge/intel/i945/gma.c M src/northbridge/intel/ironlake/gma.c M src/northbridge/intel/pineview/gma.c M src/northbridge/intel/sandybridge/gma.c M src/northbridge/intel/x4x/gma.c M src/soc/intel/baytrail/gfx.c M src/soc/intel/braswell/gfx.c M src/soc/intel/broadwell/igd.c M src/soc/intel/skylake/graphics.c 12 files changed, 9 insertions(+), 87 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/40712/2
Hello build bot (Jenkins), Damien Zammit, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40712
to look at the new patch set (#3).
Change subject: drivers/intel/gma: drop gma_set_gnvs_aslb() ......................................................................
drivers/intel/gma: drop gma_set_gnvs_aslb()
At one point this was called in opregion.c, but now only exists as duplicated code in several platforms/SoCs that for most turns a single line of code into a 4-line function, and for others is simply unused. Drop it and clean up the mess.
Change-Id: Ida23ca5ecf1821e019ed850d1eec2a858bd2e50f Signed-off-by: Matt DeVillier matt.devillier@gmail.com --- M src/drivers/intel/gma/opregion.h M src/northbridge/intel/gm45/gma.c M src/northbridge/intel/haswell/gma.c M src/northbridge/intel/i945/gma.c M src/northbridge/intel/ironlake/gma.c M src/northbridge/intel/pineview/gma.c M src/northbridge/intel/sandybridge/gma.c M src/northbridge/intel/x4x/gma.c M src/soc/intel/baytrail/gfx.c M src/soc/intel/braswell/gfx.c M src/soc/intel/broadwell/igd.c M src/soc/intel/skylake/graphics.c 12 files changed, 9 insertions(+), 87 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/40712/3
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40712 )
Change subject: drivers/intel/gma: drop gma_set_gnvs_aslb() ......................................................................
Patch Set 3: Code-Review+1
I guess the idea was to drop the duplicated soc code at some time by using this interface, but this never happend.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40712 )
Change subject: drivers/intel/gma: drop gma_set_gnvs_aslb() ......................................................................
Patch Set 3:
I guess the idea was to drop the duplicated soc code at some time by using this interface, but this never happend.
I'll try to move things to CBMEM. If that works, we won't need GNVS anymore. If it doesn't, we can still keep these functions and move the `.write_acpi_tables` implementation into the common driver.
Please postpone this change until we know which road to take.
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40712 )
Change subject: drivers/intel/gma: drop gma_set_gnvs_aslb() ......................................................................
Patch Set 3: Code-Review-1
Patch Set 3:
Please postpone this change until we know which road to take.
can do
Matt DeVillier has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/40712 )
Change subject: drivers/intel/gma: drop gma_set_gnvs_aslb() ......................................................................
Abandoned
superseded by CB:40724
-
Matt DeVillier (Code Review) -
Nico Huber (Code Review)
-
Patrick Rudolph (Code Review)