Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34877 )
Change subject: mb/google/kukui: Move panel description to CBFS files ......................................................................
Patch Set 3:
(3 comments)
https://review.coreboot.org/c/coreboot/+/34877/3/src/mainboard/google/kukui/... File src/mainboard/google/kukui/mainboard.c:
https://review.coreboot.org/c/coreboot/+/34877/3/src/mainboard/google/kukui/... PS3, Line 147: /* edid->mode.name is a pointer and cannot be serialized. */ Not sure what that comment is doing here, I'd rather put it in panel.h or something...
https://review.coreboot.org/c/coreboot/+/34877/3/src/mainboard/google/kukui/... PS3, Line 151: edid->y_resolution ? edid->y_resolution : edid->mode.va, Why not just always print ha/va? That should always be the same for your case. (In fact, I'm not even sure why we have the x_resolution/y_resolution seperately...)
https://review.coreboot.org/c/coreboot/+/34877/3/src/mainboard/google/kukui/... File src/mainboard/google/kukui/panel_kukui.c:
https://review.coreboot.org/c/coreboot/+/34877/3/src/mainboard/google/kukui/... PS3, Line 51: if (desc) Hmmm... might be worth splitting the serializable part of struct panel_description out into sub-struct like this:
struct panel_serialization { struct edid edid; enum lb_fb_orientation orientation; u8 init[]; }
struct panel_description { char *filename; void (*power_on)(void); struct panel_serialization *s; }
Then you'd have something like
static struct panel_description * kukui_panels[] = { [0] = { .filename = "CMN_P097PFG_SSD2858", .power_on = power_on_ssd2858 }, }
here, and get_panel_from_cbfs() just fills in the 's' pointer.