Attention is currently required from: Subrata Banik, Nico Huber, Wonkyu Kim, Ethan Tsao, Ravishankar Sarawadi, Tim Wawrzynczak, Paul Menzel, Raj Astekar, Patrick Rudolph. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61389 )
Change subject: soc/intel/graphics: Create Kconfig for shifting graphic memory base ......................................................................
Patch Set 13:
(4 comments)
File src/soc/intel/common/block/graphics/graphics.c:
https://review.coreboot.org/c/coreboot/+/61389/comment/db6162f3_8456f07f PS11, Line 122: printk(BIOS_INFO, "gfx memory bar(0x18) = 0x%lx GFX_MEMBASE_OFFSET = 0x%x\n", : memory_base, CONFIG_SOC_INTEL_GFX_MEMBASE_OFFSET);
The purpose of this message is to print gfx basic information without adding debug message and re-build while there is gfx issue.
I don't think one need to relying on coreboot debug msg to know the GMADR address, PCI config read is sufficient.
Also, if you have `zero` value in GMADR then the line #125 can able to catch such failure as well.
File src/soc/intel/common/block/graphics/graphics.c:
https://review.coreboot.org/c/coreboot/+/61389/comment/a81f45d8_419bdb37 PS13, Line 128: memory_base = memory_base + CONFIG_SOC_INTEL_GFX_MEMBASE_OFFSET; you have marked done but appear not done yet.
memory_base += CONFIG_SOC_INTEL_GFX_MEMBASE_OFFSET;
Also offline discussion with Will suggests me that this logic is
memory_base += (GTT size + CONFIG_SOC_INTEL_GFX_MEMBASE_OFFSET);
Are we ignoring GTT size or I misunderstood @Will ?
https://review.coreboot.org/c/coreboot/+/61389/comment/af01a419_be8f0e5b PS13, Line 129: after shift what is shift ?
https://review.coreboot.org/c/coreboot/+/61389/comment/3a775ad3_77711951 PS13, Line 129: printk(BIOS_INFO, "gfx memory base(after shift) = 0x%lx\n", memory_base); same as last comment, you don't need this.