Attention is currently required from: shkim, Paul Menzel, SH Kim, Julius Werner, Yu-Ping Wu, Karthik Ramasubramanian. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57327 )
Change subject: lib/edid_fill_fb: Make framebuffer orientation to be configurable ......................................................................
Patch Set 4:
(1 comment)
Patchset:
PS2: I agree the existing API was better. I think we can have two mechanisms (the direct, sane, existing API) and the weak override function side by side or better modify the existing fb_info like Julius suggests. That you had to add a lot of code to Kukui just to keep the existing functionality shows that the existing API makes sense.
I think the real problem here is that the FSP graphics driver is too far removed from the mainboard and doesn't allow to directly pass anything in or out. The cleanest solution for that would be to fix it so that the mainboard init code actually calls something to trigger display initialization and then gets something back (e.g. a struct fb_info pointer).
It's basically impossible without properly designing FSP first. I have my reasons when I keep repeating that FSP is not coreboot compatible. But it's also not necessary. It's much easier to write open-source code (there are no secrets in display modesetting on x86, or at least they are abstracted away (AMD's ATOMBIOS)).
If we don't want to do that, because it's a bit too big of a change maybe, you could also do something like have the FSP graphics driver store the fb_info pointer in a global and provide a function for the mainboard code to retrieve it and add additional info to it.
That's a nice thought. We could give the mainboard code access to the registered fb_info structs. I suppose mainboard code would know that it's only a single entry, so to simplify things an access function for the first list entry might be enough, e.g.:
struct lb_framebufer *lb_get_first_framebuffer(void) { if (list.next) return &list.next->fb; return NULL; }
The mainboard code could then override whatever it likes and we wouldn't need any specific override mechanism:
struct lb_framebuffer *const fb = lb_get_first_framebuffer(); if (fb) fb.orientation = ...
The only obstacle might be to find the right time in coreboot's flow to do this. I guess we can generally expect graphics init to be ready after the device initialization phase.