coreboot_tables: specify clear interface for lb_framebuffer()

For some reason the "interface" for adding framebuffer information
is sitting in src/include/vbe.h while also guarding the call to
fill_lb_framebuffer() with vbe_mode_info_valid() along with some
macro if CONFIG_* for good measure.

Move the fill_lb_framebuffer() declaration to coreboot_tables.h and
provide a comment about how it should be used. Also, now that
there's no need for the notion of a global vbe_mode_info_valid()
remove it from the conditional call path of fill_lb_framebuffer().

Change-Id: Ib3ade6314624091ae70424664527a02b279d0c9b
Signed-off-by: Aaron Durbin <adurbin@chromium.org>
Signed-off-by: Nico Huber <nico.huber@secunet.com>
Reviewed-on: https://review.coreboot.org/19729
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>

12 files changed, 80 insertions, 81 deletions

diff --git a/src/device/oprom/realmode/x86.c b/src/device/oprom/realmode/x86.c
index 83d068bcd6..0534d42db8 100644
--- a/src/device/oprom/realmode/x86.c
+++ b/src/device/oprom/realmode/x86.c
@@ -290,8 +290,11 @@ void vbe_textmode_console(void)
 				0x0000, 0x0000, 0x0000);
 }
 
-void fill_lb_framebuffer(struct lb_framebuffer *framebuffer)
+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);
@@ -311,19 +314,10 @@ void fill_lb_framebuffer(struct lb_framebuffer *framebuffer)
 
 	framebuffer->reserved_mask_pos = mode_info.vesa.reserved_mask_pos;
 	framebuffer->reserved_mask_size = mode_info.vesa.reserved_mask_size;
-}
 
-#else
-
-int vbe_mode_info_valid(void)
-{
 	return 0;
 }
 
-void fill_lb_framebuffer(struct lb_framebuffer *framebuffer)
-{
-}
-
 #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 219ef91496..913386458b 100644
--- a/src/device/oprom/yabel/vbe.c
+++ b/src/device/oprom/yabel/vbe.c
@@ -763,8 +763,11 @@ void vbe_set_graphics(void)
 #endif
 }
 
-void fill_lb_framebuffer(struct lb_framebuffer *framebuffer)
+int fill_lb_framebuffer(struct lb_framebuffer *framebuffer)
 {
+	if (!vbe_mode_info_valid())
+		return -1;
+
 	framebuffer->physical_address = le32_to_cpu(mode_info.vesa.phys_base_ptr);
 
 	framebuffer->x_resolution = le16_to_cpu(mode_info.vesa.x_resolution);
@@ -783,6 +786,8 @@ void fill_lb_framebuffer(struct lb_framebuffer *framebuffer)
 
 	framebuffer->reserved_mask_pos = mode_info.vesa.reserved_mask_pos;
 	framebuffer->reserved_mask_size = mode_info.vesa.reserved_mask_size;
+
+	return 0;
 }
 
 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 28ed06d763..dd190145e1 100644
--- a/src/drivers/intel/fsp1_1/fsp_gop.c
+++ b/src/drivers/intel/fsp1_1/fsp_gop.c
@@ -57,11 +57,8 @@ const optionrom_vbt_t *fsp_get_vbt(uint32_t *vbt_len)
 	return vbt.data;
 }
 
-void lb_framebuffer(struct lb_header *header)
+int fill_lb_framebuffer(struct lb_framebuffer *framebuffer)
 {
-	struct lb_framebuffer *framebuffer;
-	framebuffer = (struct lb_framebuffer *)lb_new_record(header);
-
 	VOID *hob_list_ptr;
 	hob_list_ptr = get_hob_list();
 	const EFI_GUID vbt_guid = EFI_PEI_GRAPHICS_INFO_HOB_GUID;
@@ -70,7 +67,7 @@ void lb_framebuffer(struct lb_header *header)
 	vbt_hob = get_next_guid_hob(&vbt_guid, hob_list_ptr);
 	if (vbt_hob == NULL) {
 		printk(BIOS_ERR, "FSP_ERR: Graphics Data HOB is not present\n");
-		return;
+		return -1;
 	}
 	printk(BIOS_DEBUG, "FSP_DEBUG: Graphics Data HOB present\n");
 	vbt_gop = GET_GUID_HOB_DATA(vbt_hob);
@@ -89,6 +86,6 @@ void lb_framebuffer(struct lb_header *header)
 	framebuffer->blue_mask_size = 8;
 	framebuffer->reserved_mask_pos = 24;
 	framebuffer->reserved_mask_size = 8;
-	framebuffer->tag = LB_TAG_FRAMEBUFFER;
-	framebuffer->size = sizeof(*framebuffer);
+
+	return 0;
 }
diff --git a/src/drivers/intel/fsp2_0/graphics.c b/src/drivers/intel/fsp2_0/graphics.c
index 97fffdc290..cfd3d16428 100644
--- a/src/drivers/intel/fsp2_0/graphics.c
+++ b/src/drivers/intel/fsp2_0/graphics.c
@@ -88,8 +88,7 @@ enum cb_err fsp_fill_lb_framebuffer(struct lb_framebuffer *framebuffer)
 	framebuffer->blue_mask_size = fbinfo->blue.size;
 	framebuffer->reserved_mask_pos = fbinfo->rsvd.pos;
 	framebuffer->reserved_mask_size = fbinfo->rsvd.pos;
-	framebuffer->tag = LB_TAG_FRAMEBUFFER;
-	framebuffer->size = sizeof(*framebuffer);
+
 	return CB_SUCCESS;
 }
 
@@ -104,10 +103,9 @@ uintptr_t fsp_load_vbt(void)
 	return (uintptr_t)vbt;
 }
 
-void lb_framebuffer(struct lb_header *header)
+int fill_lb_framebuffer(struct lb_framebuffer *framebuffer)
 {
 	enum cb_err ret;
-	struct lb_framebuffer *framebuffer;
 	uintptr_t framebuffer_bar;
 
 	/* Pci enumeration happens after silicon init.
@@ -118,20 +116,21 @@ void lb_framebuffer(struct lb_header *header)
 
 	if (!framebuffer_bar) {
 		printk(BIOS_ALERT, "Framebuffer BAR invalid\n");
-		return;
+		return -1;
 	}
 
-	framebuffer = (void *)lb_new_record(header);
 	ret = fsp_fill_lb_framebuffer(framebuffer);
 	if (ret != CB_SUCCESS) {
 		printk(BIOS_ALERT, "FSP did not return a valid framebuffer\n");
-		return;
+		return -1;
 	}
 
 	/* 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);
+
+	return 0;
 }
 
 __attribute__((weak)) uintptr_t fsp_soc_get_igd_bar(void)
diff --git a/src/drivers/intel/gma/gma.ads b/src/drivers/intel/gma/gma.ads
index d912df1635..157ec89fcc 100644
--- a/src/drivers/intel/gma/gma.ads
+++ b/src/drivers/intel/gma/gma.ads
@@ -15,9 +15,6 @@ is
 
    ----------------------------------------------------------------------------
 
-   function vbe_mode_info_valid return Interfaces.C.int;
-   pragma Export (C, vbe_mode_info_valid, "vbe_mode_info_valid");
-
    type lb_framebuffer is record
       tag                  : word32;
       size                 : word32;
@@ -37,7 +34,9 @@ is
       reserved_mask_size   : word8;
    end record;
 
-   procedure fill_lb_framebuffer (framebuffer : out lb_framebuffer);
+   function fill_lb_framebuffer
+     (framebuffer : in out lb_framebuffer)
+      return Interfaces.C.int;
    pragma Export (C, fill_lb_framebuffer, "fill_lb_framebuffer");
 
 end GMA;
diff --git a/src/drivers/intel/gma/hires_fb/gma.adb b/src/drivers/intel/gma/hires_fb/gma.adb
index 0ee4a73212..7fe4e1200d 100644
--- a/src/drivers/intel/gma/hires_fb/gma.adb
+++ b/src/drivers/intel/gma/hires_fb/gma.adb
@@ -12,38 +12,40 @@ with GMA.Mainboard;
 package body GMA
 is
 
-   vbe_valid : boolean := false;
+   fb_valid : boolean := false;
 
    linear_fb_addr : word64;
 
    fb : Framebuffer_Type;
 
-   function vbe_mode_info_valid return Interfaces.C.int
-   is
-   begin
-      return (if vbe_valid then 1 else 0);
-   end vbe_mode_info_valid;
-
-   procedure fill_lb_framebuffer (framebuffer : out lb_framebuffer)
+   function fill_lb_framebuffer
+     (framebuffer : in out lb_framebuffer)
+      return Interfaces.C.int
    is
       use type word32;
+      use type Interfaces.C.int;
    begin
-      framebuffer :=
-        (tag                  =>  0,
-         size                 =>  0,
-         physical_address     => linear_fb_addr,
-         x_resolution         => word32 (fb.Width),
-         y_resolution         => word32 (fb.Height),
-         bytes_per_line       => 4 * word32 (fb.Stride),
-         bits_per_pixel       => 32,
-         reserved_mask_pos    => 24,
-         reserved_mask_size   =>  8,
-         red_mask_pos         => 16,
-         red_mask_size        =>  8,
-         green_mask_pos       =>  8,
-         green_mask_size      =>  8,
-         blue_mask_pos        =>  0,
-         blue_mask_size       =>  8);
+      if fb_valid then
+         framebuffer :=
+           (tag                  =>  0,
+            size                 =>  0,
+            physical_address     => linear_fb_addr,
+            x_resolution         => word32 (fb.Width),
+            y_resolution         => word32 (fb.Height),
+            bytes_per_line       => 4 * word32 (fb.Stride),
+            bits_per_pixel       => 32,
+            reserved_mask_pos    => 24,
+            reserved_mask_size   =>  8,
+            red_mask_pos         => 16,
+            red_mask_size        =>  8,
+            green_mask_pos       =>  8,
+            green_mask_size      =>  8,
+            blue_mask_pos        =>  0,
+            blue_mask_size       =>  8);
+         return 0;
+      else
+         return -1;
+      end if;
    end fill_lb_framebuffer;
 
    ----------------------------------------------------------------------------
@@ -102,7 +104,7 @@ is
             HW.GFX.GMA.Update_Outputs (configs);
 
             linear_fb_addr := linear_fb;
-            vbe_valid := true;
+            fb_valid := true;
 
             lightup_ok := 1;
          end if;
diff --git a/src/drivers/intel/gma/text_fb/gma.adb b/src/drivers/intel/gma/text_fb/gma.adb
index 5df203b580..6453571d8c 100644
--- a/src/drivers/intel/gma/text_fb/gma.adb
+++ b/src/drivers/intel/gma/text_fb/gma.adb
@@ -11,16 +11,13 @@ with GMA.Mainboard;
 package body GMA
 is
 
-   function vbe_mode_info_valid return Interfaces.C.int
+   function fill_lb_framebuffer
+     (framebuffer : in out lb_framebuffer)
+      return Interfaces.C.int
    is
+      use type Interfaces.C.int;
    begin
-      return 0;
-   end vbe_mode_info_valid;
-
-   procedure fill_lb_framebuffer (framebuffer : out lb_framebuffer)
-   is
-   begin
-      null;
+      return -1;
    end fill_lb_framebuffer;
 
    ----------------------------------------------------------------------------
diff --git a/src/drivers/xgi/common/xgi_coreboot.c b/src/drivers/xgi/common/xgi_coreboot.c
index e78c7bba9d..4db06f9b6d 100644
--- a/src/drivers/xgi/common/xgi_coreboot.c
+++ b/src/drivers/xgi/common/xgi_coreboot.c
@@ -426,9 +426,14 @@ int vbe_mode_info_valid(void)
 	return xgi_vbe_valid;
 }
 
-void fill_lb_framebuffer(struct lb_framebuffer *framebuffer)
+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;
diff --git a/src/include/boot/coreboot_tables.h b/src/include/boot/coreboot_tables.h
index 5647629d76..5bebd4a623 100644
--- a/src/include/boot/coreboot_tables.h
+++ b/src/include/boot/coreboot_tables.h
@@ -22,8 +22,9 @@ void lb_add_console(uint16_t consoletype, void *data);
 /* Define this in mainboard.c to add board-specific table entries. */
 void lb_board(struct lb_header *header);
 
-/* Define this in soc or fsp driver to add specific table entries. */
-void lb_framebuffer(struct lb_header *header);
+/* Define this function to fill in the frame buffer returning 0 on success and
+   < 0 on error. */
+int fill_lb_framebuffer(struct lb_framebuffer *framebuffer);
 
 /* Allow arch to add records. */
 void lb_arch_add_records(struct lb_header *header);
diff --git a/src/include/vbe.h b/src/include/vbe.h
index 54dc560699..70c412490b 100644
--- a/src/include/vbe.h
+++ b/src/include/vbe.h
@@ -112,7 +112,6 @@ typedef struct {
 #define VESA_SET_MODE		0x4f02
 
 int vbe_mode_info_valid(void);
-void fill_lb_framebuffer(struct lb_framebuffer *framebuffer);
 void vbe_set_graphics(void);
 void vbe_textmode_console(void);
 
diff --git a/src/lib/coreboot_table.c b/src/lib/coreboot_table.c
index ecefba544c..f3cdb0bb14 100644
--- a/src/lib/coreboot_table.c
+++ b/src/lib/coreboot_table.c
@@ -135,22 +135,23 @@ void lb_add_console(uint16_t consoletype, void *data)
 	console->type = consoletype;
 }
 
-void __attribute__((weak)) lb_framebuffer(struct lb_header *header)
+int __attribute__((weak)) fill_lb_framebuffer(struct lb_framebuffer *fb)
 {
-#if CONFIG_FRAMEBUFFER_KEEP_VESA_MODE || CONFIG_MAINBOARD_DO_NATIVE_VGA_INIT
-	void fill_lb_framebuffer(struct lb_framebuffer *framebuffer);
-	int vbe_mode_info_valid(void);
+	return -1;
+}
 
-	// If there isn't any mode info to put in the table, don't ask for it
-	// to be filled with junk.
-	if (!vbe_mode_info_valid())
-		return;
+static void lb_framebuffer(struct lb_header *header)
+{
 	struct lb_framebuffer *framebuffer;
+	struct lb_framebuffer fb;
+
+	if (fill_lb_framebuffer(&fb))
+		return;
+
 	framebuffer = (struct lb_framebuffer *)lb_new_record(header);
-	fill_lb_framebuffer(framebuffer);
+	memcpy(framebuffer, &fb, sizeof(*framebuffer));
 	framebuffer->tag = LB_TAG_FRAMEBUFFER;
 	framebuffer->size = sizeof(*framebuffer);
-#endif
 }
 
 void lb_add_gpios(struct lb_gpios *gpios, const struct lb_gpio *gpio_table,
diff --git a/src/lib/edid.c b/src/lib/edid.c
index 405b3fdbc1..2216690963 100644
--- a/src/lib/edid.c
+++ b/src/lib/edid.c
@@ -1750,13 +1750,13 @@ void set_vbe_mode_info_valid(const struct edid *edid, uintptr_t fb_addr)
 }
 
 #if IS_ENABLED(CONFIG_NATIVE_VGA_INIT_USE_EDID)
-int vbe_mode_info_valid(void)
+int fill_lb_framebuffer(struct lb_framebuffer *framebuffer)
 {
-	return vbe_valid;
-}
+	if (!vbe_valid)
+		return -1;
 
-void fill_lb_framebuffer(struct lb_framebuffer *framebuffer)
-{
 	*framebuffer = edid_fb;
+
+	return 0;
 }
 #endif