Patrick Rudolph has uploaded this change for review.

View Change

[WIP]drivers: Replace multiple fill_fb_framebuffer with single instance

Replace all duplications of fill_fb_framebuffer and provide a single one.
Should not change the current behaviour.

TODO: Libgfxinit seems to expose one,too

Change-Id: Ife507f7e7beaf59854e533551b4b87ea6980c1f4
Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
---
M src/device/oprom/realmode/x86.c
M src/device/oprom/yabel/vbe.c
M src/drivers/intel/fsp1_1/fsp_gop.c
M src/drivers/intel/fsp2_0/graphics.c
M src/drivers/intel/fsp2_0/include/fsp/util.h
M src/drivers/xgi/common/xgi_coreboot.c
M src/lib/Kconfig
M src/lib/Makefile.inc
8 files changed, 111 insertions(+), 185 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/03/39003/1
diff --git a/src/device/oprom/realmode/x86.c b/src/device/oprom/realmode/x86.c
index 8ba0241..4c11e6f 100644
--- a/src/device/oprom/realmode/x86.c
+++ b/src/device/oprom/realmode/x86.c
@@ -23,6 +23,7 @@
#include <pc80/i8254.h>
#include <string.h>
#include <vbe.h>
+#include <framebuffer_info.h>

/* we use x86emu's register file representation */
#include <x86emu/regs.h>
@@ -218,6 +219,7 @@
#if CONFIG(FRAMEBUFFER_SET_VESA_MODE)
static vbe_mode_info_t mode_info;
static int mode_info_valid;
+static struct edid_fb_info *fb_info;

static int vbe_mode_info_valid(void)
{
@@ -362,6 +364,28 @@
}

vbe_set_mode(&mode_info);
+
+ if (!vbe_mode_info_valid())
+ return;
+
+ if (!fb_info) {
+ fb_info = fb_new_framebuffer_info();
+ }
+ if (fb_info) {
+ fb_fill_framebuffer_info_ex(fb_info, mode_info.vesa.phys_base_ptr,
+ le16_to_cpu(mode_info.vesa.x_resolution),
+ le16_to_cpu(mode_info.vesa.y_resolution),
+ le16_to_cpu(mode_info.vesa.bytes_per_scanline),
+ mode_info.vesa.bits_per_pixel,
+ mode_info.vesa.reserved_mask_pos,
+ mode_info.vesa.reserved_mask_size,
+ mode_info.vesa.red_mask_pos,
+ mode_info.vesa.red_mask_size,
+ mode_info.vesa.green_mask_pos,
+ mode_info.vesa.green_mask_size,
+ mode_info.vesa.blue_mask_pos,
+ mode_info.vesa.blue_mask_size);
+ }
}

void vbe_textmode_console(void)
@@ -373,34 +397,6 @@
die("\nError: In %s function\n", __func__);
}

-int fill_lb_framebuffer(struct lb_framebuffer *framebuffer)
-{
- if (!vbe_mode_info_valid())
- return -1;
-
- framebuffer->physical_address = mode_info.vesa.phys_base_ptr;
-
- framebuffer->x_resolution = le16_to_cpu(mode_info.vesa.x_resolution);
- framebuffer->y_resolution = le16_to_cpu(mode_info.vesa.y_resolution);
- framebuffer->bytes_per_line =
- le16_to_cpu(mode_info.vesa.bytes_per_scanline);
- framebuffer->bits_per_pixel = mode_info.vesa.bits_per_pixel;
-
- framebuffer->red_mask_pos = mode_info.vesa.red_mask_pos;
- framebuffer->red_mask_size = mode_info.vesa.red_mask_size;
-
- framebuffer->green_mask_pos = mode_info.vesa.green_mask_pos;
- framebuffer->green_mask_size = mode_info.vesa.green_mask_size;
-
- framebuffer->blue_mask_pos = mode_info.vesa.blue_mask_pos;
- framebuffer->blue_mask_size = mode_info.vesa.blue_mask_size;
-
- framebuffer->reserved_mask_pos = mode_info.vesa.reserved_mask_pos;
- framebuffer->reserved_mask_size = mode_info.vesa.reserved_mask_size;
-
- return 0;
-}
-
#endif

void run_bios(struct device *dev, unsigned long addr)
diff --git a/src/device/oprom/yabel/vbe.c b/src/device/oprom/yabel/vbe.c
index a3d736f..2c05338 100644
--- a/src/device/oprom/yabel/vbe.c
+++ b/src/device/oprom/yabel/vbe.c
@@ -162,6 +162,7 @@
}

static int mode_info_valid;
+static struct edid_fb_info *fb_info;

static int vbe_mode_info_valid(void)
{
@@ -747,33 +748,28 @@
mode_info.video_mode = (1 << 14) | CONFIG_FRAMEBUFFER_VESA_MODE;
vbe_get_mode_info(&mode_info);
vbe_set_mode(&mode_info);
-}

-int fill_lb_framebuffer(struct lb_framebuffer *framebuffer)
-{
if (!vbe_mode_info_valid())
- return -1;
+ return;

- framebuffer->physical_address = le32_to_cpu(mode_info.vesa.phys_base_ptr);
-
- framebuffer->x_resolution = le16_to_cpu(mode_info.vesa.x_resolution);
- framebuffer->y_resolution = le16_to_cpu(mode_info.vesa.y_resolution);
- framebuffer->bytes_per_line = le16_to_cpu(mode_info.vesa.bytes_per_scanline);
- framebuffer->bits_per_pixel = mode_info.vesa.bits_per_pixel;
-
- framebuffer->red_mask_pos = mode_info.vesa.red_mask_pos;
- framebuffer->red_mask_size = mode_info.vesa.red_mask_size;
-
- framebuffer->green_mask_pos = mode_info.vesa.green_mask_pos;
- framebuffer->green_mask_size = mode_info.vesa.green_mask_size;
-
- framebuffer->blue_mask_pos = mode_info.vesa.blue_mask_pos;
- framebuffer->blue_mask_size = mode_info.vesa.blue_mask_size;
-
- framebuffer->reserved_mask_pos = mode_info.vesa.reserved_mask_pos;
- framebuffer->reserved_mask_size = mode_info.vesa.reserved_mask_size;
-
- return 0;
+ if (!fb_info) {
+ fb_info = fb_new_framebuffer_info();
+ }
+ if (fb_info) {
+ fb_fill_framebuffer_info_ex(fb_info, mode_info.vesa.phys_base_ptr,
+ le16_to_cpu(mode_info.vesa.x_resolution),
+ le16_to_cpu(mode_info.vesa.y_resolution),
+ le16_to_cpu(mode_info.vesa.bytes_per_scanline),
+ mode_info.vesa.bits_per_pixel,
+ mode_info.vesa.reserved_mask_pos,
+ mode_info.vesa.reserved_mask_size,
+ mode_info.vesa.red_mask_pos,
+ mode_info.vesa.red_mask_size,
+ mode_info.vesa.green_mask_pos,
+ mode_info.vesa.green_mask_size,
+ mode_info.vesa.blue_mask_pos,
+ mode_info.vesa.blue_mask_size);
+ }
}

void vbe_textmode_console(void)
diff --git a/src/drivers/intel/fsp1_1/fsp_gop.c b/src/drivers/intel/fsp1_1/fsp_gop.c
index eb64151..9b555e6 100644
--- a/src/drivers/intel/fsp1_1/fsp_gop.c
+++ b/src/drivers/intel/fsp1_1/fsp_gop.c
@@ -12,10 +12,12 @@
*/

#include <boot/coreboot_tables.h>
+#include <bootstate.h>
#include <console/console.h>
+#include <framebuffer_info.h>
#include <fsp/util.h>

-int fill_lb_framebuffer(struct lb_framebuffer *framebuffer)
+static void fill_framebuffer_info(void *unused)
{
VOID *hob_list_ptr;
hob_list_ptr = get_hob_list();
@@ -30,20 +32,15 @@
printk(BIOS_DEBUG, "FSP_DEBUG: Graphics Data HOB present\n");
vbt_gop = GET_GUID_HOB_DATA(vbt_hob);

- framebuffer->physical_address = vbt_gop->FrameBufferBase;
- framebuffer->x_resolution = vbt_gop->GraphicsMode.HorizontalResolution;
- framebuffer->y_resolution = vbt_gop->GraphicsMode.VerticalResolution;
- framebuffer->bytes_per_line = vbt_gop->GraphicsMode.PixelsPerScanLine
- * 4;
- framebuffer->bits_per_pixel = 32;
- framebuffer->red_mask_pos = 16;
- framebuffer->red_mask_size = 8;
- framebuffer->green_mask_pos = 8;
- framebuffer->green_mask_size = 8;
- framebuffer->blue_mask_pos = 0;
- framebuffer->blue_mask_size = 8;
- framebuffer->reserved_mask_pos = 24;
- framebuffer->reserved_mask_size = 8;
+ struct edid_fb_info *info = fb_new_framebuffer_info();
+ if (!info)
+ return;

- return 0;
+ fb_fill_framebuffer_info(info, vbt_gop->FrameBufferBase,
+ vbt_gop->GraphicsMode.HorizontalResolution,
+ vbt_gop->GraphicsMode.VerticalResolution,
+ vbt_gop->GraphicsMode.PixelsPerScanLine * 4,
+ 32);
}
+
+BOOT_STATE_INIT_ENTRY(BS_DEV_INIT, BS_ON_EXIT, fill_framebuffer_info, NULL);
diff --git a/src/drivers/intel/fsp2_0/graphics.c b/src/drivers/intel/fsp2_0/graphics.c
index be7afdb..f35556f 100644
--- a/src/drivers/intel/fsp2_0/graphics.c
+++ b/src/drivers/intel/fsp2_0/graphics.c
@@ -17,6 +17,7 @@
#include <fsp/util.h>
#include <soc/intel/common/vbt.h>
#include <types.h>
+#include <framebuffer_info.h>

enum pixel_format {
pixel_rgbx_8bpc = 0,
@@ -58,48 +59,18 @@
[pixel_bgrx_8bpc] = { {16, 8}, {8, 8}, {0, 8}, {24, 8} },
};

-enum cb_err fsp_fill_lb_framebuffer(struct lb_framebuffer *framebuffer)
+__weak uintptr_t fsp_soc_get_igd_bar(void)
{
- size_t size;
- const struct hob_graphics_info *ginfo;
- const struct fsp_framebuffer *fbinfo;
-
- ginfo = fsp_find_extension_hob_by_guid(fsp_graphics_info_guid, &size);
-
- if (!ginfo) {
- printk(BIOS_ALERT, "Graphics hand-off block not found\n");
- return CB_ERR;
- }
-
- if (ginfo->pixel_format >= ARRAY_SIZE(fsp_framebuffer_format_map)) {
- printk(BIOS_ALERT, "FSP set unknown framebuffer format: %d\n",
- ginfo->pixel_format);
- return CB_ERR;
- }
-
- fbinfo = fsp_framebuffer_format_map + ginfo->pixel_format;
-
- framebuffer->physical_address = ginfo->framebuffer_base;
- framebuffer->x_resolution = ginfo->horizontal_resolution;
- framebuffer->y_resolution = ginfo->vertical_resolution;
- framebuffer->bytes_per_line = ginfo->pixels_per_scanline * 4;
- framebuffer->bits_per_pixel = 32;
- framebuffer->red_mask_pos = fbinfo->red.pos;
- framebuffer->red_mask_size = fbinfo->red.size;
- framebuffer->green_mask_pos = fbinfo->green.pos;
- framebuffer->green_mask_size = fbinfo->green.size;
- framebuffer->blue_mask_pos = fbinfo->blue.pos;
- framebuffer->blue_mask_size = fbinfo->blue.size;
- framebuffer->reserved_mask_pos = fbinfo->rsvd.pos;
- framebuffer->reserved_mask_size = fbinfo->rsvd.pos;
-
- return CB_SUCCESS;
+ return 0;
}

-int fill_lb_framebuffer(struct lb_framebuffer *framebuffer)
+static void fill_framebuffer_info(void *unused)
{
+ size_t size;
enum cb_err ret;
uintptr_t framebuffer_bar;
+ const struct hob_graphics_info *ginfo;
+ const struct fsp_framebuffer *fbinfo;

/* Pci enumeration happens after silicon init.
* After enumeration graphic framebuffer base may be relocated.
@@ -109,24 +80,44 @@

if (!framebuffer_bar) {
printk(BIOS_ALERT, "Framebuffer BAR invalid\n");
- return -1;
- }
-
- ret = fsp_fill_lb_framebuffer(framebuffer);
- if (ret != CB_SUCCESS) {
- printk(BIOS_ALERT, "FSP did not return a valid framebuffer\n");
- return -1;
+ return;
}

/* Resource allocator can move the BAR around after FSP configures it */
- framebuffer->physical_address = framebuffer_bar;
- printk(BIOS_DEBUG, "Graphics framebuffer located at 0x%llx\n",
- framebuffer->physical_address);
+ printk(BIOS_DEBUG, "Graphics framebuffer located at 0x%llx\n", framebuffer_bar);

- return 0;
+ ginfo = fsp_find_extension_hob_by_guid(fsp_graphics_info_guid, &size);
+
+ if (!ginfo) {
+ printk(BIOS_ALERT, "Graphics hand-off block not found\n");
+ return;
+ }
+
+ if (ginfo->pixel_format >= ARRAY_SIZE(fsp_framebuffer_format_map)) {
+ printk(BIOS_ALERT, "FSP set unknown framebuffer format: %d\n",
+ ginfo->pixel_format);
+ return;
+ }
+
+ fbinfo = fsp_framebuffer_format_map + ginfo->pixel_format;
+
+ struct edid_fb_info *info = fb_new_framebuffer_info();
+ if (!info)
+ return;
+
+ fb_fill_framebuffer_info_ex(fb_info, ginfo->framebuffer_base,
+ ginfo->horizontal_resolution,
+ ginfo->vertical_resolution,
+ ginfo->pixels_per_scanline * 4,
+ 32,
+ fbinfo->rsvd.pos,
+ fbinfo->rsvd.size,
+ fbinfo->red.pos,
+ fbinfo->red.size,
+ fbinfo->green.pos,
+ fbinfo->green.size,
+ fbinfo->blue.pos,
+ fbinfo->blue.size);
}

-__weak uintptr_t fsp_soc_get_igd_bar(void)
-{
- return 0;
-}
+BOOT_STATE_INIT_ENTRY(BS_DEV_INIT, BS_ON_EXIT, fill_framebuffer_info, NULL);
diff --git a/src/drivers/intel/fsp2_0/include/fsp/util.h b/src/drivers/intel/fsp2_0/include/fsp/util.h
index 303bafe..15e7390 100644
--- a/src/drivers/intel/fsp2_0/include/fsp/util.h
+++ b/src/drivers/intel/fsp2_0/include/fsp/util.h
@@ -76,7 +76,6 @@
void *fsp_get_hob_list_ptr(void);
const void *fsp_find_extension_hob_by_guid(const uint8_t *guid, size_t *size);
const void *fsp_find_nv_storage_data(size_t *size);
-enum cb_err fsp_fill_lb_framebuffer(struct lb_framebuffer *framebuffer);
int fsp_find_range_hob(struct range_entry *re, const uint8_t guid[16]);
void fsp_display_fvi_version_hob(void);
void fsp_find_reserved_memory(struct range_entry *re);
diff --git a/src/drivers/xgi/common/xgi_coreboot.c b/src/drivers/xgi/common/xgi_coreboot.c
index d65e007..235cbe2 100644
--- a/src/drivers/xgi/common/xgi_coreboot.c
+++ b/src/drivers/xgi/common/xgi_coreboot.c
@@ -21,6 +21,7 @@
#include <device/pci_ids.h>
#include <device/pci_ops.h>
#include <pc80/vga.h>
+#include <framebuffer_info.h>

#include "xgi_coreboot.h"
#include "vstruct.h"
@@ -31,8 +32,7 @@
#include "vb_setmode.h"
#include "XGI_main.c"

-static int xgi_vbe_valid;
-static struct lb_framebuffer xgi_fb;
+static struct edid_fb_info *fb_info;

int xgifb_probe(struct pci_dev *pdev, struct xgifb_video_info *xgifb_info)
{
@@ -359,43 +359,16 @@
XGIbios_mode[xgifb_info->mode_idx].bpp,
xgifb_info->refresh_rate);

- /* Set LinuxBIOS framebuffer information */
- xgi_vbe_valid = 1;
- xgi_fb.physical_address = xgifb_info->video_base;
- xgi_fb.x_resolution = xgifb_info->video_width;
- xgi_fb.y_resolution = xgifb_info->video_height;
- xgi_fb.bytes_per_line =
- xgifb_info->video_width * xgifb_info->video_bpp;
- xgi_fb.bits_per_pixel = xgifb_info->video_bpp;
-
- xgi_fb.reserved_mask_pos = 0;
- xgi_fb.reserved_mask_size = 0;
- switch (xgifb_info->video_bpp) {
- case 32:
- case 24:
- /* packed into 4-byte words */
- xgi_fb.reserved_mask_pos = 24;
- xgi_fb.reserved_mask_size = 8;
- xgi_fb.red_mask_pos = 16;
- xgi_fb.red_mask_size = 8;
- xgi_fb.green_mask_pos = 8;
- xgi_fb.green_mask_size = 8;
- xgi_fb.blue_mask_pos = 0;
- xgi_fb.blue_mask_size = 8;
- break;
- case 16:
- /* packed into 2-byte words */
- xgi_fb.red_mask_pos = 11;
- xgi_fb.red_mask_size = 5;
- xgi_fb.green_mask_pos = 5;
- xgi_fb.green_mask_size = 6;
- xgi_fb.blue_mask_pos = 0;
- xgi_fb.blue_mask_size = 5;
- break;
- default:
- printk(BIOS_SPEW, "%s: unsupported BPP %d\n", __func__,
- xgifb_info->video_bpp);
- xgi_vbe_valid = 0;
+ /* Set framebuffer information */
+ if (!fb_info) {
+ fb_info = fb_new_framebuffer_info();
+ }
+ if (fb_info) {
+ fb_fill_framebuffer_info(fb_info, xgifb_info->video_base,
+ xgifb_info->video_width,
+ xgifb_info->video_height,
+ xgifb_info->video_width * xgifb_info->video_bpp,
+ xgifb_info->video_bpp);
}
} else {
/*
@@ -415,22 +388,6 @@

return 0;
}
-
-static int vbe_mode_info_valid(void)
-{
- return xgi_vbe_valid;
-}
-
-int fill_lb_framebuffer(struct lb_framebuffer *framebuffer)
-{
- if (!vbe_mode_info_valid())
- return -1;
-
- *framebuffer = xgi_fb;
-
- return 0;
-}
-
struct xgifb_video_info *xgifb_video_info_ptr;

struct xgifb_video_info *pci_get_drvdata(struct pci_dev *pdev) {
diff --git a/src/lib/Kconfig b/src/lib/Kconfig
index dd9974a..d831975 100644
--- a/src/lib/Kconfig
+++ b/src/lib/Kconfig
@@ -5,14 +5,6 @@
implementation. This activates a stub that logs the missing
board reset and halts execution.

-config NO_EDID_FILL_FB
- bool
- default y if !MAINBOARD_DO_NATIVE_VGA_INIT
- help
- Don't include default fill_lb_framebuffer() implementation. Select
- this if your drivers uses MAINBOARD_DO_NATIVE_VGA_INIT but provides
- its own fill_lb_framebuffer() implementation.
-
config RAMSTAGE_ADA
bool
help
diff --git a/src/lib/Makefile.inc b/src/lib/Makefile.inc
index 2333f64..a1fb8c2 100644
--- a/src/lib/Makefile.inc
+++ b/src/lib/Makefile.inc
@@ -134,9 +134,7 @@
ramstage-$(CONFIG_COLLECT_TIMESTAMPS) += timestamp.c
ramstage-$(CONFIG_COVERAGE) += libgcov.c
ramstage-y += edid.c
-ifneq ($(CONFIG_NO_EDID_FILL_FB),y)
ramstage-y += edid_fill_fb.c
-endif
ramstage-y += memrange.c
ramstage-$(CONFIG_COOP_MULTITASKING) += thread.c
ramstage-$(CONFIG_TIMER_QUEUE) += timer_queue.c

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

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ife507f7e7beaf59854e533551b4b87ea6980c1f4
Gerrit-Change-Number: 39003
Gerrit-PatchSet: 1
Gerrit-Owner: Patrick Rudolph <patrick.rudolph@9elements.com>
Gerrit-Reviewer: Huang Jin <huang.jin@intel.com>
Gerrit-Reviewer: Lee Leahy <leroy.p.leahy@intel.com>
Gerrit-Reviewer: Martin Roth <martinroth@google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph@9elements.com>
Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org>
Gerrit-MessageType: newchange