Hello Matt DeVillier, Angel Pons, Patrick Rudolph,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/40725
to review the following change.
Change subject: soc/intel/gma: Move display and opregion init to common code ......................................................................
soc/intel/gma: Move display and opregion init to common code
Change-Id: I359b529df44db7d63c5a7922cb1ebd8e130d0c43 Signed-off-by: Nico Huber nico.h@gmx.de --- M src/soc/intel/apollolake/graphics.c M src/soc/intel/cannonlake/graphics.c M src/soc/intel/common/block/graphics/graphics.c M src/soc/intel/icelake/graphics.c M src/soc/intel/jasperlake/graphics.c M src/soc/intel/skylake/graphics.c M src/soc/intel/tigerlake/graphics.c 7 files changed, 39 insertions(+), 194 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/25/40725/1
diff --git a/src/soc/intel/apollolake/graphics.c b/src/soc/intel/apollolake/graphics.c index 99c3c88..8deb548 100644 --- a/src/soc/intel/apollolake/graphics.c +++ b/src/soc/intel/apollolake/graphics.c @@ -13,42 +13,11 @@ * GNU General Public License for more details. */
-#include <stdint.h> -#include <arch/acpi.h> -#include <bootmode.h> -#include <console/console.h> #include <fsp/util.h> -#include <device/device.h> -#include <device/pci.h> -#include <device/pci_ops.h> #include <intelblocks/graphics.h> -#include <drivers/intel/gma/opregion.h> -#include <drivers/intel/gma/libgfxinit.h> #include <types.h>
uintptr_t fsp_soc_get_igd_bar(void) { return graphics_get_memory_base(); } - -void graphics_soc_init(struct device *const dev) -{ - uint32_t reg32 = pci_read_config32(dev, PCI_COMMAND); - reg32 |= PCI_COMMAND_MASTER; - pci_write_config32(dev, PCI_COMMAND, reg32); - - if (CONFIG(RUN_FSP_GOP)) { - /* nothing to do */ - } else if (CONFIG(MAINBOARD_USE_LIBGFXINIT)) { - if (!acpi_is_wakeup_s3() && display_init_required()) { - int lightup_ok; - gma_gfxinit(&lightup_ok); - gfx_set_init_done(lightup_ok); - } - } else { - /* Initialize PCI device, load/execute BIOS Option ROM */ - pci_dev_init(dev); - } - - intel_gma_init_igd_opregion(); -} diff --git a/src/soc/intel/cannonlake/graphics.c b/src/soc/intel/cannonlake/graphics.c index 9046119..1ca5b76 100644 --- a/src/soc/intel/cannonlake/graphics.c +++ b/src/soc/intel/cannonlake/graphics.c @@ -14,15 +14,9 @@ */
#include <arch/acpi.h> -#include <bootmode.h> -#include <console/console.h> #include <fsp/util.h> #include <device/device.h> -#include <device/pci.h> -#include <device/pci_ops.h> #include <drivers/intel/gma/i915_reg.h> -#include <drivers/intel/gma/libgfxinit.h> -#include <drivers/intel/gma/opregion.h> #include <intelblocks/graphics.h> #include <types.h>
@@ -46,32 +40,4 @@ DDI_BUF_IS_IDLE); graphics_gtt_write(DDI_BUF_CTL_A, ddi_buf_ctl); } - - /* IGD needs to Bus Master */ - uint32_t reg32 = pci_read_config32(dev, PCI_COMMAND); - reg32 |= PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY | PCI_COMMAND_IO; - pci_write_config32(dev, PCI_COMMAND, reg32); - - /* - * GFX PEIM module inside FSP binary is taking care of graphics - * initialization based on RUN_FSP_GOP Kconfig option and input - * VBT file. - * - * In case of non-FSP solution, SoC need to select another - * Kconfig to perform GFX initialization. - */ - if (CONFIG(RUN_FSP_GOP)) { - /* nothing to do */ - } else if (CONFIG(MAINBOARD_USE_LIBGFXINIT)) { - if (!acpi_is_wakeup_s3() && display_init_required()) { - int lightup_ok; - gma_gfxinit(&lightup_ok); - gfx_set_init_done(lightup_ok); - } - } else { - /* Initialize PCI device, load/execute BIOS Option ROM */ - pci_dev_init(dev); - } - - intel_gma_init_igd_opregion(); } diff --git a/src/soc/intel/common/block/graphics/graphics.c b/src/soc/intel/common/block/graphics/graphics.c index 0b2d635..533ae31 100644 --- a/src/soc/intel/common/block/graphics/graphics.c +++ b/src/soc/intel/common/block/graphics/graphics.c @@ -14,11 +14,14 @@ */
#include <assert.h> +#include <bootmode.h> #include <console/console.h> #include <device/mmio.h> #include <device/pci.h> #include <device/pci_ids.h> #include <drivers/intel/gma/i915.h> +#include <drivers/intel/gma/libgfxinit.h> +#include <drivers/intel/gma/opregion.h> #include <intelblocks/graphics.h> #include <soc/pci_devs.h>
@@ -28,9 +31,7 @@ /* * User needs to implement SoC override in case wishes * to perform certain specific graphics initialization - * along with pci_dev_init(dev) */ - pci_dev_init(dev); }
__weak const struct i915_gpu_controller_info * @@ -39,6 +40,40 @@ return NULL; }
+static void gma_init(struct device *const dev) +{ + /* SoC specific configuration. */ + graphics_soc_init(dev); + + /* IGD needs to Bus Master */ + u32 reg32 = pci_read_config32(dev, PCI_COMMAND); + reg32 |= PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY | PCI_COMMAND_IO; + pci_write_config32(dev, PCI_COMMAND, reg32); + + /* + * GFX PEIM module inside FSP binary is taking care of graphics + * initialization based on RUN_FSP_GOP Kconfig option and input + * VBT file. + * + * In case of non-FSP solution, SoC need to select another + * Kconfig to perform GFX initialization. + */ + if (CONFIG(RUN_FSP_GOP)) { + /* nothing to do */ + } else if (CONFIG(MAINBOARD_USE_LIBGFXINIT)) { + if (!acpi_is_wakeup_s3() && display_init_required()) { + int lightup_ok; + gma_gfxinit(&lightup_ok); + gfx_set_init_done(lightup_ok); + } + } else { + /* Initialize PCI device, load/execute BIOS Option ROM */ + pci_dev_init(dev); + } + + intel_gma_init_igd_opregion(); +} + static void gma_generate_ssdt(struct device *device) { const struct i915_gpu_controller_info *gfx = intel_igd_get_controller_info(device); @@ -129,7 +164,7 @@ .read_resources = pci_dev_read_resources, .set_resources = pci_dev_set_resources, .enable_resources = pci_dev_enable_resources, - .init = graphics_soc_init, + .init = gma_init, .ops_pci = &pci_dev_ops_pci, #if CONFIG(HAVE_ACPI_TABLES) .acpi_fill_ssdt = gma_generate_ssdt, diff --git a/src/soc/intel/icelake/graphics.c b/src/soc/intel/icelake/graphics.c index 79a1125..8deb548 100644 --- a/src/soc/intel/icelake/graphics.c +++ b/src/soc/intel/icelake/graphics.c @@ -13,12 +13,7 @@ * GNU General Public License for more details. */
-#include <console/console.h> #include <fsp/util.h> -#include <device/device.h> -#include <device/pci.h> -#include <device/pci_ops.h> -#include <drivers/intel/gma/opregion.h> #include <intelblocks/graphics.h> #include <types.h>
@@ -26,29 +21,3 @@ { return graphics_get_memory_base(); } - -void graphics_soc_init(struct device *dev) -{ - /* IGD needs to Bus Master */ - uint32_t reg32 = pci_read_config32(dev, PCI_COMMAND); - reg32 |= PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY | PCI_COMMAND_IO; - pci_write_config32(dev, PCI_COMMAND, reg32); - - /* - * GFX PEIM module inside FSP binary is taking care of graphics - * initialization based on RUN_FSP_GOP Kconfig - * option and input VBT file. Hence no need to load/execute legacy VGA - * OpROM in order to initialize GFX. - * - * In case of non-FSP solution, SoC need to select VGA_ROM_RUN - * Kconfig to perform GFX initialization through VGA OpRom. - */ - if (CONFIG(RUN_FSP_GOP)) { - /* nothing to do */ - } else { - /* Initialize PCI device, load/execute BIOS Option ROM */ - pci_dev_init(dev); - } - - intel_gma_init_igd_opregion(); -} diff --git a/src/soc/intel/jasperlake/graphics.c b/src/soc/intel/jasperlake/graphics.c index 79a1125..8deb548 100644 --- a/src/soc/intel/jasperlake/graphics.c +++ b/src/soc/intel/jasperlake/graphics.c @@ -13,12 +13,7 @@ * GNU General Public License for more details. */
-#include <console/console.h> #include <fsp/util.h> -#include <device/device.h> -#include <device/pci.h> -#include <device/pci_ops.h> -#include <drivers/intel/gma/opregion.h> #include <intelblocks/graphics.h> #include <types.h>
@@ -26,29 +21,3 @@ { return graphics_get_memory_base(); } - -void graphics_soc_init(struct device *dev) -{ - /* IGD needs to Bus Master */ - uint32_t reg32 = pci_read_config32(dev, PCI_COMMAND); - reg32 |= PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY | PCI_COMMAND_IO; - pci_write_config32(dev, PCI_COMMAND, reg32); - - /* - * GFX PEIM module inside FSP binary is taking care of graphics - * initialization based on RUN_FSP_GOP Kconfig - * option and input VBT file. Hence no need to load/execute legacy VGA - * OpROM in order to initialize GFX. - * - * In case of non-FSP solution, SoC need to select VGA_ROM_RUN - * Kconfig to perform GFX initialization through VGA OpRom. - */ - if (CONFIG(RUN_FSP_GOP)) { - /* nothing to do */ - } else { - /* Initialize PCI device, load/execute BIOS Option ROM */ - pci_dev_init(dev); - } - - intel_gma_init_igd_opregion(); -} diff --git a/src/soc/intel/skylake/graphics.c b/src/soc/intel/skylake/graphics.c index 5646522..97cdc44 100644 --- a/src/soc/intel/skylake/graphics.c +++ b/src/soc/intel/skylake/graphics.c @@ -1,17 +1,13 @@ /* SPDX-License-Identifier: GPL-2.0-only */ /* This file is part of the coreboot project. */
-#include <bootmode.h> #include <commonlib/helpers.h> #include <console/console.h> -#include <device/pci.h> -#include <device/pci_ops.h> +#include <device/mmio.h> #include <device/resource.h> #include <drivers/intel/gma/i915.h> #include <drivers/intel/gma/i915_reg.h> -#include <drivers/intel/gma/libgfxinit.h> #include <intelblocks/graphics.h> -#include <drivers/intel/gma/opregion.h> #include <soc/ramstage.h> #include <types.h>
@@ -96,34 +92,6 @@ ddi_buf_ctl |= DDI_A_4_LANES; graphics_gtt_write(DDI_BUF_CTL_A, ddi_buf_ctl); } - - /* IGD needs to Bus Master */ - u32 reg32 = pci_read_config32(dev, PCI_COMMAND); - reg32 |= PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY | PCI_COMMAND_IO; - pci_write_config32(dev, PCI_COMMAND, reg32); - - /* - * GFX PEIM module inside FSP binary is taking care of graphics - * initialization based on RUN_FSP_GOP Kconfig option and input - * VBT file. - * - * In case of non-FSP solution, SoC need to select another - * Kconfig to perform GFX initialization. - */ - if (CONFIG(RUN_FSP_GOP)) { - /* nothing to do */ - } else if (CONFIG(MAINBOARD_USE_LIBGFXINIT)) { - if (!acpi_is_wakeup_s3() && display_init_required()) { - int lightup_ok; - gma_gfxinit(&lightup_ok); - gfx_set_init_done(lightup_ok); - } - } else { - /* Initialize PCI device, load/execute BIOS Option ROM */ - pci_dev_init(dev); - } - - intel_gma_init_igd_opregion(); }
const struct i915_gpu_controller_info * diff --git a/src/soc/intel/tigerlake/graphics.c b/src/soc/intel/tigerlake/graphics.c index 979b4a0..90d3a02 100644 --- a/src/soc/intel/tigerlake/graphics.c +++ b/src/soc/intel/tigerlake/graphics.c @@ -19,12 +19,7 @@ * Chapter number: 4 */
-#include <console/console.h> #include <fsp/util.h> -#include <device/device.h> -#include <device/pci.h> -#include <device/pci_ops.h> -#include <drivers/intel/gma/opregion.h> #include <intelblocks/graphics.h> #include <types.h>
@@ -32,29 +27,3 @@ { return graphics_get_memory_base(); } - -void graphics_soc_init(struct device *dev) -{ - /* IGD needs to Bus Master */ - uint32_t reg32 = pci_read_config32(dev, PCI_COMMAND); - reg32 |= PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY | PCI_COMMAND_IO; - pci_write_config32(dev, PCI_COMMAND, reg32); - - /* - * GFX PEIM module inside FSP binary is taking care of graphics - * initialization based on RUN_FSP_GOP Kconfig - * option and input VBT file. Hence no need to load/execute legacy VGA - * OpROM in order to initialize GFX. - * - * In case of non-FSP solution, SoC need to select VGA_ROM_RUN - * Kconfig to perform GFX initialization through VGA OpRom. - */ - if (CONFIG(RUN_FSP_GOP)) { - /* nothing to do */ - } else { - /* Initialize PCI device, load/execute BIOS Option ROM */ - pci_dev_init(dev); - } - - intel_gma_init_igd_opregion(); -}