Nico Huber has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34165 )
Change subject: soc/intel/cnl/graphics: Hook up libgfxinit ......................................................................
soc/intel/cnl/graphics: Hook up libgfxinit
Change-Id: Ic038adad6cf76867cd4a8626d4c49e17018389fd Signed-off-by: Nico Huber nico.huber@secunet.com --- M src/soc/intel/cannonlake/graphics.c 1 file changed, 22 insertions(+), 14 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/34165/1
diff --git a/src/soc/intel/cannonlake/graphics.c b/src/soc/intel/cannonlake/graphics.c index 2acfecc..ebe8b0b 100644 --- a/src/soc/intel/cannonlake/graphics.c +++ b/src/soc/intel/cannonlake/graphics.c @@ -15,12 +15,14 @@ */
#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,25 +48,31 @@ graphics_gtt_write(DDI_BUF_CTL_A, ddi_buf_ctl); }
- /* - * GFX PEIM module inside FSP binary is taking care of graphics - * initialization based on INTEL_GMA_ADD_VBT 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(INTEL_GMA_ADD_VBT)) - return; - /* 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);
- /* Initialize PCI device, load/execute BIOS Option ROM */ - pci_dev_init(dev); + /* + * 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); + } }
uintptr_t graphics_soc_write_acpi_opregion(struct device *device,
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34165 )
Change subject: soc/intel/cnl/graphics: Hook up libgfxinit ......................................................................
Patch Set 1: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/34165/1/src/soc/intel/cannonlake/gr... File src/soc/intel/cannonlake/graphics.c:
https://review.coreboot.org/c/coreboot/+/34165/1/src/soc/intel/cannonlake/gr... PS1, Line 75: } unrelated, but should this platform call intel_gma_restore_opregion() here?
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34165 )
Change subject: soc/intel/cnl/graphics: Hook up libgfxinit ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34165/1/src/soc/intel/cannonlake/gr... File src/soc/intel/cannonlake/graphics.c:
https://review.coreboot.org/c/coreboot/+/34165/1/src/soc/intel/cannonlake/gr... PS1, Line 75: }
unrelated, but should this platform call intel_gma_restore_opregion() here?
I guess, yes. afaict, this file is not platform specific anyway. Just copy-first bug-later practice.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34165 )
Change subject: soc/intel/cnl/graphics: Hook up libgfxinit ......................................................................
Patch Set 1: Code-Review+1
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34165 )
Change subject: soc/intel/cnl/graphics: Hook up libgfxinit ......................................................................
Patch Set 1: Code-Review+1
(2 comments)
https://review.coreboot.org/c/coreboot/+/34165/1/src/soc/intel/cannonlake/gr... File src/soc/intel/cannonlake/graphics.c:
https://review.coreboot.org/c/coreboot/+/34165/1/src/soc/intel/cannonlake/gr... PS1, Line 58: and input : * VBT file. Is this still true?
https://review.coreboot.org/c/coreboot/+/34165/1/src/soc/intel/cannonlake/gr... PS1, Line 62: . Minor: I think you can add 'e.g. libgfxinit' at the end (just enough space for it)
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34165 )
Change subject: soc/intel/cnl/graphics: Hook up libgfxinit ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34165/1/src/soc/intel/cannonlake/gr... File src/soc/intel/cannonlake/graphics.c:
https://review.coreboot.org/c/coreboot/+/34165/1/src/soc/intel/cannonlake/gr... PS1, Line 58: and input : * VBT file.
Is this still true?
AFAIK, what could differ?
https://review.coreboot.org/c/coreboot/+/34165/1/src/soc/intel/cannonlake/gr... PS1, Line 62: .
Minor: I think you can add 'e.g. […]
I really don't care enough about this comment, this is not the place to document Kconfig options. I merely realigned the code with its original copy, afair.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34165 )
Change subject: soc/intel/cnl/graphics: Hook up libgfxinit ......................................................................
Patch Set 2:
(3 comments)
https://review.coreboot.org/c/coreboot/+/34165/1/src/soc/intel/cannonlake/gr... File src/soc/intel/cannonlake/graphics.c:
https://review.coreboot.org/c/coreboot/+/34165/1/src/soc/intel/cannonlake/gr... PS1, Line 58: and input : * VBT file.
AFAIK, what could differ?
Done, I guess
https://review.coreboot.org/c/coreboot/+/34165/1/src/soc/intel/cannonlake/gr... PS1, Line 62: .
I really don't care enough about this comment, this is not the […]
Done, I guess
https://review.coreboot.org/c/coreboot/+/34165/1/src/soc/intel/cannonlake/gr... PS1, Line 75: }
I guess, yes. afaict, this file is not platform specific anyway. Just […]
Done
Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34165 )
Change subject: soc/intel/cnl/graphics: Hook up libgfxinit ......................................................................
soc/intel/cnl/graphics: Hook up libgfxinit
Change-Id: Ic038adad6cf76867cd4a8626d4c49e17018389fd Signed-off-by: Nico Huber nico.huber@secunet.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/34165 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Patrick Rudolph siro@das-labor.org Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net Reviewed-by: Angel Pons th3fanbus@gmail.com --- M src/soc/intel/cannonlake/graphics.c 1 file changed, 22 insertions(+), 14 deletions(-)
Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Patrick Rudolph: Looks good to me, approved Angel Pons: Looks good to me, but someone else must approve
diff --git a/src/soc/intel/cannonlake/graphics.c b/src/soc/intel/cannonlake/graphics.c index 2acfecc..ebe8b0b 100644 --- a/src/soc/intel/cannonlake/graphics.c +++ b/src/soc/intel/cannonlake/graphics.c @@ -15,12 +15,14 @@ */
#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,25 +48,31 @@ graphics_gtt_write(DDI_BUF_CTL_A, ddi_buf_ctl); }
- /* - * GFX PEIM module inside FSP binary is taking care of graphics - * initialization based on INTEL_GMA_ADD_VBT 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(INTEL_GMA_ADD_VBT)) - return; - /* 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);
- /* Initialize PCI device, load/execute BIOS Option ROM */ - pci_dev_init(dev); + /* + * 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); + } }
uintptr_t graphics_soc_write_acpi_opregion(struct device *device,
-
Angel Pons (Code Review) -
Nico Huber (Code Review)
-
Patrick Rudolph (Code Review)
-
Paul Menzel (Code Review)