Attention is currently required from: Furquan Shaikh, shkim, Paul Menzel, SH Kim, Julius Werner, Angel Pons, Andrey Petrov, Patrick Rudolph. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57199 )
Change subject: drivers/intel/fsp2_0: Make framebuffer orientation to be configurable ......................................................................
Patch Set 1:
(3 comments)
Patchset:
PS1:
I heard that the panel hardware in use for bugzzy is not able to rotate the framebuffer itself.
I meant the graphics hardware. For instance, Intel's display engine supports 90 degree rotation of the framebuffer since Skylake.
File src/drivers/intel/fsp2_0/graphics.c:
https://review.coreboot.org/c/coreboot/+/57199/comment/482d85b9_571f7c04 PS1, Line 103: enum lb_fb_orientation __weak fsp_get_framebuffer_orientation(void)
Not sure what you mean by "a common solution"? The `orientation` field in lb_framebuffer is the common solution that we already have, it's already used by several other boards. How the platform driver gets it there before calling fb_add_framebuffer is up to the platform driver, just like for all the other parameters.
I assumed it meant a more common place in the code. There seems to be no need to make this FSP specific.
- Actually I'm not familiar with open source graphics init in coreboot, but it seems we have some interfaces to override the orientation setting for each panel and each project in coreboot(lib/edid_fill_fb.c), but Yu-Ping Wu mentioned we couldn't use it for intel project using FSP graphics init.
There's a call to fb_add_framebuffer_info_ex() above. I have no idea why it couldn't be done inside that function or below.
https://review.coreboot.org/c/coreboot/+/57199/comment/a39e6ec1_98cdc0d3 PS1, Line 106: }
If we'd put a config for screen orientation, do you think we should use the config for FSP graphics […]
I don't see a difference if you do it here or there. What refactoring do you see necessary?
NB. `edid_fill_fb.c` is not considered graphics init. It's just some infrastructure code to report framebuffers. Each silicon vendor has their own graphics init so far. Most ARM SoCs have it below their respective src/soc/ dir. For Intel (up to 9.5th gen display engine), we have 3rdparty/libgfxinit. AMD is the only silicon vendor I know about that keeps their display engine closed.
Alas, libgfxinit is not developed much right now because Intel only updated their consumer products for some years and my employer doesn't care about those. And other coreboot-involved companies working with Intel don't care about open source. Their usual excuse that the hardware isn't documented and writing open-source code would be hard doesn't work for Intel graphics ;)