Hello Matt DeVillier, Angel Pons, Patrick Rudolph,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/40728
to review the following change.
Change subject: soc/intel/gma: Move DDI-A 4-lane config to common code ......................................................................
soc/intel/gma: Move DDI-A 4-lane config to common code
Change-Id: I0572dbbfb61e5e0129fe6a3a1b5894145d74fd0d Signed-off-by: Nico Huber nico.h@gmx.de --- M src/soc/intel/cannonlake/Kconfig M src/soc/intel/cannonlake/graphics.c M src/soc/intel/common/block/graphics/Kconfig M src/soc/intel/common/block/graphics/graphics.c M src/soc/intel/skylake/Kconfig M src/soc/intel/skylake/graphics.c 6 files changed, 18 insertions(+), 36 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/28/40728/1
diff --git a/src/soc/intel/cannonlake/Kconfig b/src/soc/intel/cannonlake/Kconfig index 9bd57a3..c8602da 100644 --- a/src/soc/intel/cannonlake/Kconfig +++ b/src/soc/intel/cannonlake/Kconfig @@ -25,6 +25,7 @@ select SOC_INTEL_CANNONLAKE_BASE select FSP_USES_CB_STACK select HAVE_INTEL_FSP_REPO + select SOC_INTEL_CONFIGURE_DDI_A_4_LANES help Intel Coffeelake support
@@ -33,6 +34,7 @@ select SOC_INTEL_CANNONLAKE_BASE select FSP_USES_CB_STACK select HAVE_INTEL_FSP_REPO + select SOC_INTEL_CONFIGURE_DDI_A_4_LANES help Intel Whiskeylake support
@@ -42,6 +44,7 @@ select MICROCODE_BLOB_UNDISCLOSED select FSP_USES_CB_STACK select HAVE_INTEL_FSP_REPO + select SOC_INTEL_CONFIGURE_DDI_A_4_LANES help Intel Cometlake support
diff --git a/src/soc/intel/cannonlake/graphics.c b/src/soc/intel/cannonlake/graphics.c index 1ca5b76..cdc4a6a 100644 --- a/src/soc/intel/cannonlake/graphics.c +++ b/src/soc/intel/cannonlake/graphics.c @@ -24,20 +24,3 @@ { return graphics_get_memory_base(); } - -void graphics_soc_init(struct device *dev) -{ - uint32_t ddi_buf_ctl; - - /* - * Enable DDI-A (eDP) 4-lane operation if the link is not up yet. - * This will allow the kernel to use 4-lane eDP links properly - * if the VBIOS or GOP driver do not execute. - */ - ddi_buf_ctl = graphics_gtt_read(DDI_BUF_CTL_A); - if (!acpi_is_wakeup_s3() && !(ddi_buf_ctl & DDI_BUF_CTL_ENABLE)) { - ddi_buf_ctl |= (DDI_A_4_LANES | DDI_INIT_DISPLAY_DETECTED | - DDI_BUF_IS_IDLE); - graphics_gtt_write(DDI_BUF_CTL_A, ddi_buf_ctl); - } -} diff --git a/src/soc/intel/common/block/graphics/Kconfig b/src/soc/intel/common/block/graphics/Kconfig index 4ab9200..e632cb9 100644 --- a/src/soc/intel/common/block/graphics/Kconfig +++ b/src/soc/intel/common/block/graphics/Kconfig @@ -2,3 +2,8 @@ bool help Intel Processor common Graphics support + +config SOC_INTEL_CONFIGURE_DDI_A_4_LANES + bool + help + Selected by platforms that require DDI-A bifurcation setup. diff --git a/src/soc/intel/common/block/graphics/graphics.c b/src/soc/intel/common/block/graphics/graphics.c index 7d7a49d..206810f 100644 --- a/src/soc/intel/common/block/graphics/graphics.c +++ b/src/soc/intel/common/block/graphics/graphics.c @@ -13,6 +13,7 @@ * GNU General Public License for more details. */
+#include <arch/acpi.h> #include <assert.h> #include <bootmode.h> #include <console/console.h> @@ -45,6 +46,13 @@ /* SoC specific configuration. */ graphics_soc_init(dev);
+ if (CONFIG(SOC_INTEL_CONFIGURE_DDI_A_4_LANES) && !acpi_is_wakeup_s3()) { + const u32 ddi_buf_ctl = graphics_gtt_read(DDI_BUF_CTL_A); + /* Only program if the buffer is not enabled yet. */ + if (!(ddi_buf_ctl & DDI_BUF_CTL_ENABLE)) + graphics_gtt_write(DDI_BUF_CTL_A, ddi_buf_ctl | DDI_A_4_LANES); + } + if (!CONFIG(NO_GFX_INIT)) pci_or_config16(dev, PCI_COMMAND, PCI_COMMAND_MASTER);
diff --git a/src/soc/intel/skylake/Kconfig b/src/soc/intel/skylake/Kconfig index 0a5daea..0d5ea44 100644 --- a/src/soc/intel/skylake/Kconfig +++ b/src/soc/intel/skylake/Kconfig @@ -73,6 +73,7 @@ select SOC_INTEL_COMMON_PCH_BASE select SOC_INTEL_COMMON_NHLT select SOC_INTEL_COMMON_RESET + select SOC_INTEL_CONFIGURE_DDI_A_4_LANES select SSE2 select SUPPORT_CPU_UCODE_IN_CBFS select TSC_MONOTONIC_TIMER diff --git a/src/soc/intel/skylake/graphics.c b/src/soc/intel/skylake/graphics.c index 97cdc44..8285ad9 100644 --- a/src/soc/intel/skylake/graphics.c +++ b/src/soc/intel/skylake/graphics.c @@ -16,7 +16,7 @@ return graphics_get_memory_base(); }
-static void graphics_setup_panel(struct device *dev) +void graphics_soc_init(struct device *dev) { struct soc_intel_skylake_config *conf = config_of(dev); struct resource *mmio_res; @@ -76,24 +76,6 @@ } }
-void graphics_soc_init(struct device *dev) -{ - u32 ddi_buf_ctl; - - graphics_setup_panel(dev); - - /* - * Enable DDI-A (eDP) 4-lane operation if the link is not up yet. - * This will allow the kernel to use 4-lane eDP links properly - * if the VBIOS or GOP driver does not execute. - */ - ddi_buf_ctl = graphics_gtt_read(DDI_BUF_CTL_A); - if (!acpi_is_wakeup_s3() && !(ddi_buf_ctl & DDI_BUF_CTL_ENABLE)) { - ddi_buf_ctl |= DDI_A_4_LANES; - graphics_gtt_write(DDI_BUF_CTL_A, ddi_buf_ctl); - } -} - const struct i915_gpu_controller_info * intel_igd_get_controller_info(struct device *device) {
Hello Matt DeVillier, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40728
to look at the new patch set (#2).
Change subject: soc/intel/gma: Move DDI-A 4-lane config to common code ......................................................................
soc/intel/gma: Move DDI-A 4-lane config to common code
Change-Id: I0572dbbfb61e5e0129fe6a3a1b5894145d74fd0d Signed-off-by: Nico Huber nico.h@gmx.de --- M src/soc/intel/cannonlake/Kconfig M src/soc/intel/cannonlake/graphics.c M src/soc/intel/common/block/graphics/Kconfig M src/soc/intel/common/block/graphics/graphics.c M src/soc/intel/skylake/Kconfig M src/soc/intel/skylake/graphics.c 6 files changed, 18 insertions(+), 37 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/28/40728/2
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40728 )
Change subject: soc/intel/gma: Move DDI-A 4-lane config to common code ......................................................................
Patch Set 2: Code-Review+1
tested ok on google/chell (SKL) with HiDPI display requiring 4 lanes (3200x1800)
Hello build bot (Jenkins), Matt DeVillier, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40728
to look at the new patch set (#3).
Change subject: soc/intel/gma: Move DDI-A 4-lane config to common code ......................................................................
soc/intel/gma: Move DDI-A 4-lane config to common code
Change-Id: I0572dbbfb61e5e0129fe6a3a1b5894145d74fd0d Signed-off-by: Nico Huber nico.h@gmx.de --- M src/soc/intel/cannonlake/Kconfig M src/soc/intel/cannonlake/graphics.c M src/soc/intel/common/block/graphics/Kconfig M src/soc/intel/common/block/graphics/graphics.c M src/soc/intel/skylake/Kconfig M src/soc/intel/skylake/graphics.c 6 files changed, 18 insertions(+), 37 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/28/40728/3
Hello build bot (Jenkins), Matt DeVillier, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40728
to look at the new patch set (#4).
Change subject: soc/intel/gma: Move DDI-A 4-lane config to common code ......................................................................
soc/intel/gma: Move DDI-A 4-lane config to common code
Change-Id: I0572dbbfb61e5e0129fe6a3a1b5894145d74fd0d Signed-off-by: Nico Huber nico.h@gmx.de --- M src/soc/intel/cannonlake/Kconfig M src/soc/intel/cannonlake/graphics.c M src/soc/intel/common/block/graphics/Kconfig M src/soc/intel/common/block/graphics/graphics.c M src/soc/intel/skylake/Kconfig M src/soc/intel/skylake/graphics.c 6 files changed, 18 insertions(+), 37 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/28/40728/4
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40728 )
Change subject: soc/intel/gma: Move DDI-A 4-lane config to common code ......................................................................
Patch Set 4: Code-Review+2
Hello build bot (Jenkins), Matt DeVillier, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40728
to look at the new patch set (#5).
Change subject: soc/intel/gma: Move DDI-A 4-lane config to common code ......................................................................
soc/intel/gma: Move DDI-A 4-lane config to common code
Change-Id: I0572dbbfb61e5e0129fe6a3a1b5894145d74fd0d Signed-off-by: Nico Huber nico.h@gmx.de --- M src/soc/intel/cannonlake/Kconfig M src/soc/intel/cannonlake/graphics.c M src/soc/intel/common/block/graphics/Kconfig M src/soc/intel/common/block/graphics/graphics.c M src/soc/intel/skylake/Kconfig M src/soc/intel/skylake/graphics.c 6 files changed, 18 insertions(+), 37 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/28/40728/5
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40728 )
Change subject: soc/intel/gma: Move DDI-A 4-lane config to common code ......................................................................
Patch Set 6: Code-Review+2
Nico Huber has submitted this change. ( https://review.coreboot.org/c/coreboot/+/40728 )
Change subject: soc/intel/gma: Move DDI-A 4-lane config to common code ......................................................................
soc/intel/gma: Move DDI-A 4-lane config to common code
Change-Id: I0572dbbfb61e5e0129fe6a3a1b5894145d74fd0d Signed-off-by: Nico Huber nico.h@gmx.de Reviewed-on: https://review.coreboot.org/c/coreboot/+/40728 Reviewed-by: Angel Pons th3fanbus@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/cannonlake/Kconfig M src/soc/intel/cannonlake/graphics.c M src/soc/intel/common/block/graphics/Kconfig M src/soc/intel/common/block/graphics/graphics.c M src/soc/intel/skylake/Kconfig M src/soc/intel/skylake/graphics.c 6 files changed, 18 insertions(+), 37 deletions(-)
Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved
diff --git a/src/soc/intel/cannonlake/Kconfig b/src/soc/intel/cannonlake/Kconfig index 7a56d0d..f06d84b 100644 --- a/src/soc/intel/cannonlake/Kconfig +++ b/src/soc/intel/cannonlake/Kconfig @@ -25,6 +25,7 @@ select SOC_INTEL_CANNONLAKE_BASE select FSP_USES_CB_STACK select HAVE_INTEL_FSP_REPO + select SOC_INTEL_CONFIGURE_DDI_A_4_LANES help Intel Coffeelake support
@@ -33,6 +34,7 @@ select SOC_INTEL_CANNONLAKE_BASE select FSP_USES_CB_STACK select HAVE_INTEL_FSP_REPO + select SOC_INTEL_CONFIGURE_DDI_A_4_LANES help Intel Whiskeylake support
@@ -41,6 +43,7 @@ select SOC_INTEL_CANNONLAKE_BASE select FSP_USES_CB_STACK select HAVE_INTEL_FSP_REPO + select SOC_INTEL_CONFIGURE_DDI_A_4_LANES help Intel Cometlake support
diff --git a/src/soc/intel/cannonlake/graphics.c b/src/soc/intel/cannonlake/graphics.c index cd5e773..5fbe0d5 100644 --- a/src/soc/intel/cannonlake/graphics.c +++ b/src/soc/intel/cannonlake/graphics.c @@ -11,20 +11,3 @@ { return graphics_get_memory_base(); } - -void graphics_soc_init(struct device *dev) -{ - uint32_t ddi_buf_ctl; - - /* - * Enable DDI-A (eDP) 4-lane operation if the link is not up yet. - * This will allow the kernel to use 4-lane eDP links properly - * if the VBIOS or GOP driver do not execute. - */ - ddi_buf_ctl = graphics_gtt_read(DDI_BUF_CTL_A); - if (!acpi_is_wakeup_s3() && !(ddi_buf_ctl & DDI_BUF_CTL_ENABLE)) { - ddi_buf_ctl |= (DDI_A_4_LANES | DDI_INIT_DISPLAY_DETECTED | - DDI_BUF_IS_IDLE); - graphics_gtt_write(DDI_BUF_CTL_A, ddi_buf_ctl); - } -} diff --git a/src/soc/intel/common/block/graphics/Kconfig b/src/soc/intel/common/block/graphics/Kconfig index 4ab9200..e632cb9 100644 --- a/src/soc/intel/common/block/graphics/Kconfig +++ b/src/soc/intel/common/block/graphics/Kconfig @@ -2,3 +2,8 @@ bool help Intel Processor common Graphics support + +config SOC_INTEL_CONFIGURE_DDI_A_4_LANES + bool + help + Selected by platforms that require DDI-A bifurcation setup. diff --git a/src/soc/intel/common/block/graphics/graphics.c b/src/soc/intel/common/block/graphics/graphics.c index ba4bc85..e2c9060 100644 --- a/src/soc/intel/common/block/graphics/graphics.c +++ b/src/soc/intel/common/block/graphics/graphics.c @@ -1,5 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-or-later */
+#include <acpi/acpi.h> #include <assert.h> #include <bootmode.h> #include <console/console.h> @@ -34,6 +35,13 @@ /* SoC specific configuration. */ graphics_soc_init(dev);
+ if (CONFIG(SOC_INTEL_CONFIGURE_DDI_A_4_LANES) && !acpi_is_wakeup_s3()) { + const u32 ddi_buf_ctl = graphics_gtt_read(DDI_BUF_CTL_A); + /* Only program if the buffer is not enabled yet. */ + if (!(ddi_buf_ctl & DDI_BUF_CTL_ENABLE)) + graphics_gtt_write(DDI_BUF_CTL_A, ddi_buf_ctl | DDI_A_4_LANES); + } + /* * GFX PEIM module inside FSP binary is taking care of graphics * initialization based on RUN_FSP_GOP Kconfig option and input diff --git a/src/soc/intel/skylake/Kconfig b/src/soc/intel/skylake/Kconfig index 55437f3..27c1084 100644 --- a/src/soc/intel/skylake/Kconfig +++ b/src/soc/intel/skylake/Kconfig @@ -74,6 +74,7 @@ select SOC_INTEL_COMMON_NHLT select SOC_INTEL_COMMON_RESET select SOC_INTEL_COMMON_BLOCK_POWER_LIMIT + select SOC_INTEL_CONFIGURE_DDI_A_4_LANES select SSE2 select SUPPORT_CPU_UCODE_IN_CBFS select TSC_MONOTONIC_TIMER diff --git a/src/soc/intel/skylake/graphics.c b/src/soc/intel/skylake/graphics.c index d887097..27cb874 100644 --- a/src/soc/intel/skylake/graphics.c +++ b/src/soc/intel/skylake/graphics.c @@ -1,6 +1,5 @@ /* SPDX-License-Identifier: GPL-2.0-only */
-#include <acpi/acpi.h> #include <commonlib/helpers.h> #include <console/console.h> #include <device/mmio.h> @@ -16,7 +15,7 @@ return graphics_get_memory_base(); }
-static void graphics_setup_panel(struct device *dev) +void graphics_soc_init(struct device *dev) { struct soc_intel_skylake_config *conf = config_of(dev); struct resource *mmio_res; @@ -76,24 +75,6 @@ } }
-void graphics_soc_init(struct device *dev) -{ - u32 ddi_buf_ctl; - - graphics_setup_panel(dev); - - /* - * Enable DDI-A (eDP) 4-lane operation if the link is not up yet. - * This will allow the kernel to use 4-lane eDP links properly - * if the VBIOS or GOP driver does not execute. - */ - ddi_buf_ctl = graphics_gtt_read(DDI_BUF_CTL_A); - if (!acpi_is_wakeup_s3() && !(ddi_buf_ctl & DDI_BUF_CTL_ENABLE)) { - ddi_buf_ctl |= DDI_A_4_LANES; - graphics_gtt_write(DDI_BUF_CTL_A, ddi_buf_ctl); - } -} - const struct i915_gpu_controller_info * intel_igd_get_controller_info(const struct device *device) {
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40728 )
Change subject: soc/intel/gma: Move DDI-A 4-lane config to common code ......................................................................
Patch Set 7:
Automatic boot test returned (PASS/FAIL/TOTAL): 4/0/4 Emulation targets: "QEMU x86 q35/ich9" using payload TianoCore : SUCCESS : https://lava.9esec.io/r/4238 "QEMU x86 q35/ich9" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/4237 "QEMU x86 i440fx/piix4" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/4236 "QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/4235
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)
-
Nico Huber (Code Review)