Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33189 )
Change subject: soc/intel/common: Skip SoC GT programming based on CONFIG_SKIP_GRAPHICS_ENABLING ......................................................................
Patch Set 2: Code-Review-1
(2 comments)
https://review.coreboot.org/#/c/33189/1/src/soc/intel/icelake/graphics.c File src/soc/intel/icelake/graphics.c:
https://review.coreboot.org/#/c/33189/1/src/soc/intel/icelake/graphics.c@48 PS1, Line 48: DDI_A_4_LANES
yes, we have GPU enable skus now so we are okay now :-) […]
So it's just more code to copy-paste later. Probably without anyone ever checking if the code guarded by this makes sense for the new platform at all. It seems to me like a bad work- around for bad development practice.
If I understand you correctly, the display engine of your early samples works (you can see output on a display) but the graphics acceleration doesn't? But the code guarded here has absolutely nothing to do with graphics acceleration. Pushing this weird workaround with wrong explanations makes it very hard to work together in a community, I can't agree to it. But if you have to, go on.
https://review.coreboot.org/#/c/33189/2/src/soc/intel/icelake/graphics.c File src/soc/intel/icelake/graphics.c:
https://review.coreboot.org/#/c/33189/2/src/soc/intel/icelake/graphics.c@50 PS2, Line 50: graphics_gtt_write(DDI_BUF_CTL_A, ddi_buf_ctl); Subrata, Aamir, do you know any ICL documentation about this register? As this is the only thing effectively guarded by SKIP_GRAPHICS_ENABLING, I wonder if this copy-pasta is correct for ICL at all.