Angel Pons submitted this change.

View Change

Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved Angel Pons: Looks good to me, approved
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>
---
M src/include/edid.h
A src/include/framebuffer_info.h
M src/lib/edid_fill_fb.c
M src/mainboard/google/kukui/mainboard.c
4 files changed, 181 insertions(+), 60 deletions(-)

diff --git a/src/include/edid.h b/src/include/edid.h
index 7536a66..3644e6a 100644
--- a/src/include/edid.h
+++ b/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 @@
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);

diff --git a/src/include/framebuffer_info.h b/src/include/framebuffer_info.h
new file mode 100644
index 0000000..5251f29
--- /dev/null
+++ b/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_ */
diff --git a/src/lib/edid_fill_fb.c b/src/lib/edid_fill_fb.c
index 422feaf..1e8cc5a 100644
--- a/src/lib/edid_fill_fb.c
+++ b/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;
-static struct lb_framebuffer edid_fb;
+struct fb_info {
+ 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;
- edid_fb.x_resolution = edid->x_resolution;
- edid_fb.y_resolution = edid->y_resolution;
- edid_fb.bytes_per_line = edid->bytes_per_line;
- /* 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;
- }
+ struct fb_info *ret;
+ ret = malloc(sizeof(struct fb_info));
+ if (ret)
+ memset(ret, 0, sizeof(struct fb_info));

- 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)
- return -1;
+ struct fb_info *i;

- *framebuffer = edid_fb;
-
- return 0;
+ list_for_each(i, list, node) {
+ //TODO: Add support for advertising all framebuffers in this list
+ *framebuffer = i->fb;
+ return 0;
+ }
+ return -1;
}
diff --git a/src/mainboard/google/kukui/mainboard.c b/src/mainboard/google/kukui/mainboard.c
index 4220810d..216b6f4 100644
--- a/src/mainboard/google/kukui/mainboard.c
+++ b/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 @@
return false;
}
mtk_ddp_mode_set(edid);
- set_vbe_mode_info_valid(edid, 0);
- set_vbe_framebuffer_orientation(panel->s->orientation);
+ struct fb_info *info = set_vbe_mode_info_valid(edid, 0);
+ if (info)
+ fb_set_orientation(info, panel->s->orientation);
+
return true;
}


To view, visit change 39002. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7264c2ea2f72f36adfd26f26b00e3ce172133621
Gerrit-Change-Number: 39002
Gerrit-PatchSet: 27
Gerrit-Owner: Patrick Rudolph <patrick.rudolph@9elements.com>
Gerrit-Reviewer: Alexandru Gagniuc <alexandrux.gagniuc@intel.com>
Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz>
Gerrit-Reviewer: Christian Walter <christian.walter@9elements.com>
Gerrit-Reviewer: Frans Hendriks <fhendriks@eltan.com>
Gerrit-Reviewer: Julius Werner <jwerner@chromium.org>
Gerrit-Reviewer: Lee Leahy <leroy.p.leahy@intel.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier@gmail.com>
Gerrit-Reviewer: Michael Niewöhner <foss@mniewoehner.de>
Gerrit-Reviewer: Naresh Solanki <naresh.solanki@intel.com>
Gerrit-Reviewer: Nico Huber <nico.h@gmx.de>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph@9elements.com>
Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter@users.sourceforge.net>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki@gmail.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com>
Gerrit-Reviewer: Wim Vervoorn <wvervoorn@eltan.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-CC: Felix Singer <felixsinger@posteo.net>
Gerrit-MessageType: merged