lib/edid_fill_fb: Support multiple framebuffers

Currently it's not possible to add multiple graphics driver into
one coreboot image. This patch series will fix this issue by providing
a single API that multiple graphics driver can use.

This is required for platforms that have two graphic cards, but
different graphic drivers, like Intel and Aspeed on server platforms or
Intel and Nvidia on consumer notebooks.

The goals are to remove duplicated fill_fb_framebuffer(), to advertise
multiple independent framebuffers in coreboot tables, and better
runtime/build time graphic configuration options.

Add an implementation in edid_fill_fb that supports registering
multiple framebuffers, each with its own configuration.

As the current code is only compiled for a single graphics driver
there's no change in functionality.

Change-Id: I7264c2ea2f72f36adfd26f26b00e3ce172133621
Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/39002
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Nico Huber <nico.h@gmx.de>

src/include/edid.h

@@ -4,6 +4,7 @@
 #define EDID_H
 
 #include <stdint.h>
+#include <framebuffer_info.h>
 #include "commonlib/coreboot_tables.h"
 
 enum edid_modes {
@@ -95,7 +96,7 @@ enum edid_status {
 int decode_edid(unsigned char *edid, int size, struct edid *out);
 void edid_set_framebuffer_bits_per_pixel(struct edid *edid, int fb_bpp,
 					 int row_byte_alignment);
-void set_vbe_mode_info_valid(const struct edid *edid, uintptr_t fb_addr);
+struct fb_info *set_vbe_mode_info_valid(const struct edid *edid, uintptr_t fb_addr);
 void set_vbe_framebuffer_orientation(enum lb_fb_orientation orientation);
 int set_display_mode(struct edid *edid, enum edid_modes mode);
 

src/include/framebuffer_info.h

@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#ifndef __FRAMEBUFFER_INFO_H_
+#define __FRAMEBUFFER_INFO_H_
+
+#include <stdint.h>
+#include <stdbool.h>
+#include <commonlib/coreboot_tables.h>
+
+struct fb_info;
+
+struct fb_info *
+fb_add_framebuffer_info_ex(const struct lb_framebuffer *fb);
+
+struct fb_info *fb_add_framebuffer_info(uintptr_t fb_addr, uint32_t x_resolution,
+					uint32_t y_resolution, uint32_t bytes_per_line,
+					uint8_t bits_per_pixel);
+
+void fb_set_orientation(struct fb_info *info,
+			enum lb_fb_orientation orientation);
+
+#endif /* __FRAMEBUFFER_INFO_H_ */

src/lib/edid_fill_fb.c

@@ -3,75 +3,170 @@
 #include <console/console.h>
 #include <edid.h>
 #include <boot/coreboot_tables.h>
+#include <framebuffer_info.h>
+#include <string.h>
+#include <stdlib.h>
+#include <bootsplash.h>
+#include <list.h>
 
-static int fb_valid;
+struct fb_info {
-static struct lb_framebuffer edid_fb;
+	struct list_node node;
+	struct lb_framebuffer fb;
+};
+static struct list_node list;
 
 /*
- * Take an edid, and create a framebuffer. Set fb_valid to 1.
+ * Allocate a new framebuffer info struct on heap.
+ * Returns NULL on error.
  */
-void set_vbe_mode_info_valid(const struct edid *edid, uintptr_t fb_addr)
+static struct fb_info *fb_new_framebuffer_info(void)
 {
-	edid_fb.physical_address = fb_addr;
+	struct fb_info *ret;
-	edid_fb.x_resolution = edid->x_resolution;
+	ret = malloc(sizeof(struct fb_info));
-	edid_fb.y_resolution = edid->y_resolution;
+	if (ret)
-	edid_fb.bytes_per_line = edid->bytes_per_line;
+		memset(ret, 0, sizeof(struct fb_info));
-	/* In the case of (e.g.) 24 framebuffer bits per pixel, the convention
-	 * nowadays seems to be to round it up to the nearest reasonable
-	 * boundary, because otherwise the byte-packing is hideous.
-	 * So, for example, in RGB with no alpha, the bytes are still
-	 * packed into 32-bit words, the so-called 32bpp-no-alpha mode.
-	 * Or, in 5:6:5 mode, the bytes are also packed into 32-bit words,
-	 * and in 4:4:4 mode, they are packed into 16-bit words.
-	 * Good call on the hardware guys part.
-	 * It's not clear we're covering all cases here, but
-	 * I'm not sure with grahpics you ever can.
-	 */
-	edid_fb.bits_per_pixel = edid->framebuffer_bits_per_pixel;
-	edid_fb.reserved_mask_pos = 0;
-	edid_fb.reserved_mask_size = 0;
-	switch (edid->framebuffer_bits_per_pixel) {
-	case 32:
-	case 24:
-		/* packed into 4-byte words */
-		edid_fb.reserved_mask_pos = 24;
-		edid_fb.reserved_mask_size = 8;
-		edid_fb.red_mask_pos = 16;
-		edid_fb.red_mask_size = 8;
-		edid_fb.green_mask_pos = 8;
-		edid_fb.green_mask_size = 8;
-		edid_fb.blue_mask_pos = 0;
-		edid_fb.blue_mask_size = 8;
-		break;
-	case 16:
-		/* packed into 2-byte words */
-		edid_fb.red_mask_pos = 11;
-		edid_fb.red_mask_size = 5;
-		edid_fb.green_mask_pos = 5;
-		edid_fb.green_mask_size = 6;
-		edid_fb.blue_mask_pos = 0;
-		edid_fb.blue_mask_size = 5;
-		break;
-	default:
-		printk(BIOS_SPEW, "%s: unsupported BPP %d\n", __func__,
-		       edid->framebuffer_bits_per_pixel);
-		return;
-	}
 
-	fb_valid = 1;
+	return ret;
 }
 
-void set_vbe_framebuffer_orientation(enum lb_fb_orientation orientation)
+/*
+ * Fills a provided framebuffer info struct and adds it to the internal list if it's
+ * valid. Returns NULL on error.
+ */
+struct fb_info *
+fb_add_framebuffer_info_ex(const struct lb_framebuffer *fb)
 {
-	edid_fb.orientation = orientation;
+	struct fb_info *info;
+	uint8_t bpp_mask;
+
+	/* Validate input */
+	if (!fb || !fb->x_resolution || !fb->y_resolution || !fb->bytes_per_line ||
+	    !fb->bits_per_pixel) {
+		printk(BIOS_ERR, "%s: Invalid framebuffer data provided\n", __func__);
+		return NULL;
+	}
+
+	bpp_mask = fb->blue_mask_size + fb->green_mask_size + fb->red_mask_size +
+		fb->reserved_mask_size;
+	if (fb->bits_per_pixel != bpp_mask) {
+		printk(BIOS_ERR, "%s: BPP=%d and channel bit mask=%d doesn't match."
+		       " This is a driver bug.\n", __func__, fb->bits_per_pixel, bpp_mask);
+		return NULL;
+	}
+
+	info = fb_new_framebuffer_info();
+	if (!info)
+		return NULL;
+
+	printk(BIOS_INFO, "framebuffer_info: bytes_per_line: %d, bits_per_pixel: %d\n "
+			  "                  x_res x y_res: %d x %d, size: %d at 0x%llx\n",
+			fb->bytes_per_line, fb->bits_per_pixel, fb->x_resolution,
+			fb->y_resolution, (fb->bytes_per_line * fb->y_resolution),
+			fb->physical_address);
+
+	/* Update */
+	info->fb = *fb;
+
+	list_insert_after(&info->node, &list);
+
+	return info;
+}
+
+/*
+ * Allocates a new framebuffer info struct and fills it for 32/24/16bpp framebuffers.
+ * Intended for drivers that only support reporting the current information or have a single
+ * modeset invocation.
+ *
+ * Complex drivers should use fb_add_framebuffer_info_ex() instead.
+ */
+struct fb_info *
+fb_add_framebuffer_info(uintptr_t fb_addr, uint32_t x_resolution,
+			uint32_t y_resolution, uint32_t bytes_per_line,
+			uint8_t bits_per_pixel)
+{
+	struct fb_info *info = NULL;
+
+	switch (bits_per_pixel) {
+	case 32:
+	case 24: {
+		/* FIXME: 24 BPP might be RGB8 or XRGB8 */
+		/* packed into 4-byte words */
+
+		const struct lb_framebuffer fb = {
+			.physical_address    = fb_addr,
+			.x_resolution        = x_resolution,
+			.y_resolution        = y_resolution,
+			.bytes_per_line      = bytes_per_line,
+			.bits_per_pixel      = bits_per_pixel,
+			.red_mask_pos        = 16,
+			.red_mask_size       = 8,
+			.green_mask_pos      = 8,
+			.green_mask_size     = 8,
+			.blue_mask_pos       = 0,
+			.blue_mask_size      = 8,
+			.reserved_mask_pos   = 24,
+			.reserved_mask_size  = 8,
+			.orientation         = LB_FB_ORIENTATION_NORMAL,
+		};
+
+		info = fb_add_framebuffer_info_ex(&fb);
+		break;
+	}
+	case 16: {
+		/* packed into 2-byte words */
+		const struct lb_framebuffer fb = {
+			.physical_address   = fb_addr,
+			.x_resolution       = x_resolution,
+			.y_resolution       = y_resolution,
+			.bytes_per_line     = bytes_per_line,
+			.bits_per_pixel     = 16,
+			.red_mask_pos       = 11,
+			.red_mask_size      = 5,
+			.green_mask_pos     = 5,
+			.green_mask_size    = 6,
+			.blue_mask_pos      = 0,
+			.blue_mask_size     = 5,
+			.reserved_mask_pos  = 0,
+			.reserved_mask_size = 0,
+			.orientation        = LB_FB_ORIENTATION_NORMAL,
+		};
+		info = fb_add_framebuffer_info_ex(&fb);
+		break;
+	}
+	default:
+		printk(BIOS_ERR, "%s: unsupported BPP %d\n", __func__, bits_per_pixel);
+	}
+	if (!info)
+		printk(BIOS_ERR, "%s: failed to add framebuffer info\n", __func__);
+
+	return info;
+}
+
+void fb_set_orientation(struct fb_info *info, enum lb_fb_orientation orientation)
+{
+	if (!info)
+		return;
+
+	info->fb.orientation = orientation;
+}
+
+/*
+ * Take an edid, and create a framebuffer.
+ */
+struct fb_info *set_vbe_mode_info_valid(const struct edid *edid, uintptr_t fb_addr)
+{
+	return fb_add_framebuffer_info(fb_addr, edid->x_resolution, edid->y_resolution,
+		edid->bytes_per_line, edid->framebuffer_bits_per_pixel);
 }
 
 int fill_lb_framebuffer(struct lb_framebuffer *framebuffer)
 {
-	if (!fb_valid)
+	struct fb_info *i;
-		return -1;
 
-	*framebuffer = edid_fb;
+	list_for_each(i, list, node) {
-
+		//TODO: Add support for advertising all framebuffers in this list
-	return 0;
+		*framebuffer = i->fb;
+		return 0;
+	}
+	return -1;
 }

src/mainboard/google/kukui/mainboard.c

@@ -10,6 +10,7 @@
 #include <device/device.h>
 #include <ec/google/chromeec/ec.h>
 #include <edid.h>
+#include <framebuffer_info.h>
 #include <gpio.h>
 #include <soc/ddp.h>
 #include <soc/dsi.h>
@@ -168,8 +169,10 @@ static bool configure_display(void)
 		return false;
 	}
 	mtk_ddp_mode_set(edid);
-	set_vbe_mode_info_valid(edid, 0);
+	struct fb_info *info = set_vbe_mode_info_valid(edid, 0);
-	set_vbe_framebuffer_orientation(panel->s->orientation);
+	if (info)
+		fb_set_orientation(info, panel->s->orientation);
+
 	return true;
 }