From 5f0225a7b57b837517fb6e0b3cfd3bccb1fc4a8a Mon Sep 17 00:00:00 2001 From: Subrata Banik Date: Thu, 28 Aug 2025 16:06:15 +0530 Subject: [PATCH] drivers/intel/fsp2_0: Refactor for earlier graphics memory WC MTRR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Moves the MTRR setup for graphics memory (GMADR) from the `soc_load_logo_by_coreboot()` function to `do_silicon_init()`. This refactors the logic into a new helper function, `soc_mark_gfx_memory()`, which acquires a temporary Write-Combine (WC) MTRR. The MTRR is now configured earlier in the silicon initialization process, making the setup and cleanup independent of the `soc_load_logo_by_coreboot()` function itself. This improves FSP-S performance and ensures the MTRR is correctly managed within the silicon initialization flow which was earlier missed when platform selects `USE_COREBOOT_FOR_BMP_RENDERING` aka rendering the BMP logo using coreboot driver and not using FSP driver logic. The cleanup of the MTRR is also moved to `do_silicon_init()` to pair with the earlier setup. TEST=Successfully boot to OS on google/fatcat using coreboot for logo rendering. w/o this patch ``` 963:returning from FspMultiPhaseSiInit 1,164,839 (123,244) ``` w/ this patch ``` 963:returning from FspMultiPhaseSiInit 1,143,974 (115,443) ``` Change-Id: I5da3178c622f5fd6cb3d7f3f574e59f9ed5a5b3d Signed-off-by: Subrata Banik Reviewed-on: https://review.coreboot.org/c/coreboot/+/88982 Reviewed-by: Jérémy Compostella Reviewed-by: Pranava Y N Tested-by: build bot (Jenkins) --- src/drivers/intel/fsp2_0/cb_logo.c | 30 ++++++++++++++-------- src/drivers/intel/fsp2_0/include/fsp/api.h | 2 ++ src/drivers/intel/fsp2_0/silicon_init.c | 9 +++++++ 3 files changed, 30 insertions(+), 11 deletions(-) diff --git a/src/drivers/intel/fsp2_0/cb_logo.c b/src/drivers/intel/fsp2_0/cb_logo.c index c77e55eeed..33563d5271 100644 --- a/src/drivers/intel/fsp2_0/cb_logo.c +++ b/src/drivers/intel/fsp2_0/cb_logo.c @@ -39,11 +39,29 @@ static void program_igd_lmembar(uint32_t base) pci_or_config16(SA_DEV_IGD, PCI_COMMAND, enable_mask); } +/* + * Marks the GMADR range as Write-Combine (WC) memory. + * + * This function acquires and configures a temporary MTRR for the GMADR range. + * + * @return valid MTRR index on success, -1 on failure. + */ +int soc_mark_gfx_memory(void) +{ + /* Set up a temporary Write Combine (WC) MTRR for the GMADR range */ + int temp_mtrr_index = acquire_and_configure_mtrr(GMADR_BASE, GMADR_SIZE, MTRR_TYPE_WRCOMB); + if (temp_mtrr_index < 0) { + printk(BIOS_ERR, "Failed to configure WC MTRR for GMADR.\n"); + return -1; + } + + return temp_mtrr_index; +} + void soc_load_logo_by_coreboot(void) { const struct hob_graphics_info *ginfo; struct soc_intel_common_config *config = chip_get_common_soc_structure(); - int temp_mtrr_index = -1; struct logo_config logo_cfg; size_t size; @@ -57,13 +75,6 @@ void soc_load_logo_by_coreboot(void) /* Program the IGD LMEMBAR */ program_igd_lmembar(GMADR_BASE); - /* Set up a temporary Write Combine (WC) MTRR for the GMADR range */ - temp_mtrr_index = acquire_and_configure_mtrr(GMADR_BASE, GMADR_SIZE, MTRR_TYPE_WRCOMB); - if (temp_mtrr_index < 0) { - printk(BIOS_ERR, "Failed to configure WC MTRR for GMADR.\n"); - return; - } - /* * Adjusts panel orientation for external display when the lid is closed. * @@ -88,7 +99,4 @@ void soc_load_logo_by_coreboot(void) logo_cfg.logo_bottom_margin = config->logo_bottom_margin; render_logo_to_framebuffer(&logo_cfg); - - /* Clear temporary Write Combine (WC) MTRR */ - clear_var_mtrr(temp_mtrr_index); } diff --git a/src/drivers/intel/fsp2_0/include/fsp/api.h b/src/drivers/intel/fsp2_0/include/fsp/api.h index 9ff32be6fc..84edb640b2 100644 --- a/src/drivers/intel/fsp2_0/include/fsp/api.h +++ b/src/drivers/intel/fsp2_0/include/fsp/api.h @@ -108,8 +108,10 @@ void soc_load_logo_by_fsp(FSPS_UPD *supd); */ #if CONFIG(USE_COREBOOT_FOR_BMP_RENDERING) void soc_load_logo_by_coreboot(void); +int soc_mark_gfx_memory(void); #else static inline void soc_load_logo_by_coreboot(void) { /* nop */ } +static inline int soc_mark_gfx_memory(void) { return -1; } #endif /* Update the SOC specific memory config param for mma. */ diff --git a/src/drivers/intel/fsp2_0/silicon_init.c b/src/drivers/intel/fsp2_0/silicon_init.c index 6de71c9202..ea179bd945 100644 --- a/src/drivers/intel/fsp2_0/silicon_init.c +++ b/src/drivers/intel/fsp2_0/silicon_init.c @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -96,6 +97,7 @@ static void do_silicon_init(struct fsp_header *hdr) fsp_multi_phase_init_fn multi_phase_si_init; struct fsp_multi_phase_params multi_phase_params; struct fsp_multi_phase_get_number_of_phases_params multi_phase_get_number; + int temp_mtrr_index = -1; supd = (FSPS_UPD *)(uintptr_t)(hdr->cfg_region_offset + hdr->image_base); @@ -123,6 +125,9 @@ static void do_silicon_init(struct fsp_header *hdr) /* Give SoC/mainboard a chance to populate entries */ platform_fsp_silicon_init_params_cb(upd); + if (CONFIG(BMP_LOGO) && CONFIG(USE_COREBOOT_FOR_BMP_RENDERING)) + temp_mtrr_index = soc_mark_gfx_memory(); + /* * Populate UPD entries for the logo if the platform utilizes * the FSP's capability for rendering bitmap (BMP) images. @@ -229,6 +234,10 @@ static void do_silicon_init(struct fsp_header *hdr) * the rendering. */ timestamp_add_now(TS_FIRMWARE_SPLASH_RENDERED); + + /* Clear temporary Write Combine (WC) MTRR */ + if (temp_mtrr_index >= 0) + clear_var_mtrr(temp_mtrr_index); } /* Reinitialize CPUs if FSP-S has done MP Init */