[coreboot-gerrit] Change in coreboot[master]: lib/edid: Split out fill_lb_framebuffer()

Martin Roth (Code Review) gerrit at coreboot.org
Tue May 30 18:32:50 CEST 2017


Martin Roth has submitted this change and it was merged. ( https://review.coreboot.org/19764 )

Change subject: lib/edid: Split out fill_lb_framebuffer()
......................................................................


lib/edid: Split out fill_lb_framebuffer()

Place it into new edid_fill_fb.c, and invert the logic of the Kconfig
guard (NATIVE_VGA_INIT_USE_EDID is now !NO_EDID_FILL_FB). It has to be
selected by all drivers that use MAINBOARD_DO_NATIVE_VGA_INIT but pro-
vide their own fill_lb_framebuffer() implementation.

Change-Id: I90634b835bd8e2d150b1c714328a5b2774d891bd
Signed-off-by: Nico Huber <nico.huber at secunet.com>
Reviewed-on: https://review.coreboot.org/19764
Tested-by: build bot (Jenkins) <no-reply at coreboot.org>
Reviewed-by: Aaron Durbin <adurbin at chromium.org>
Reviewed-by: Julius Werner <jwerner at chromium.org>
---
M src/Kconfig
M src/device/Kconfig
M src/drivers/xgi/common/Kconfig
M src/drivers/xgi/z9s/z9s.c
M src/include/edid.h
A src/lib/Kconfig
M src/lib/Makefile.inc
M src/lib/edid.c
A src/lib/edid_fill_fb.c
9 files changed, 109 insertions(+), 78 deletions(-)

Approvals:
  Aaron Durbin: Looks good to me, approved
  Julius Werner: Looks good to me, but someone else must approve
  build bot (Jenkins): Verified



diff --git a/src/Kconfig b/src/Kconfig
index 4091a61..50a054a 100644
--- a/src/Kconfig
+++ b/src/Kconfig
@@ -1043,6 +1043,9 @@
 ###############################################################################
 # Set variables with no prompt - these can be set anywhere, and putting at
 # the end of this file gives the most flexibility.
+
+source "src/lib/Kconfig"
+
 config ENABLE_APIC_EXT_ID
 	bool
 	default n
diff --git a/src/device/Kconfig b/src/device/Kconfig
index 3dfc507..778ab88 100644
--- a/src/device/Kconfig
+++ b/src/device/Kconfig
@@ -21,13 +21,6 @@
 	bool
 	default n
 
-# FIXME Ugly hack to allow Z9s driver native framebuffer configuration
-config NATIVE_VGA_INIT_USE_EDID
-	bool
-	depends on MAINBOARD_DO_NATIVE_VGA_INIT
-	default n if DRIVERS_XGI_Z9S || MAINBOARD_USE_LIBGFXINIT
-	default y if !DRIVERS_XGI_Z9S
-
 config MAINBOARD_HAS_NATIVE_VGA_INIT_TEXTMODECFG
 	bool
 	default n
@@ -57,6 +50,7 @@
 	select MAINBOARD_HAS_NATIVE_VGA_INIT_TEXTMODECFG
 	select RAMSTAGE_LIBHWBASE
 	select VGA if !FRAMEBUFFER_KEEP_VESA_MODE
+	select NO_EDID_FILL_FB
 	default n
 	help
 	  Use the SPARK library `libgfxinit` for the native graphics
diff --git a/src/drivers/xgi/common/Kconfig b/src/drivers/xgi/common/Kconfig
index a0b6fed..203a123 100644
--- a/src/drivers/xgi/common/Kconfig
+++ b/src/drivers/xgi/common/Kconfig
@@ -1,3 +1,4 @@
 config DRIVERS_XGI_Z79_COMMON
 	bool
 	select VGA
+	select NO_EDID_FILL_FB
diff --git a/src/drivers/xgi/z9s/z9s.c b/src/drivers/xgi/z9s/z9s.c
index 8c1c97c..86808fc 100644
--- a/src/drivers/xgi/z9s/z9s.c
+++ b/src/drivers/xgi/z9s/z9s.c
@@ -16,7 +16,6 @@
 #include <stdlib.h>
 #include <string.h>
 #include <arch/io.h>
-#include <edid.h>
 
 #include <console/console.h>
 #include <device/device.h>
diff --git a/src/include/edid.h b/src/include/edid.h
index 1eb8c4d..d567115 100644
--- a/src/include/edid.h
+++ b/src/include/edid.h
@@ -16,6 +16,8 @@
 #ifndef EDID_H
 #define EDID_H
 
+#include <stdint.h>
+
 enum edid_modes {
 	EDID_MODE_640x480_60Hz,
 	EDID_MODE_720x480_60Hz,
diff --git a/src/lib/Kconfig b/src/lib/Kconfig
new file mode 100644
index 0000000..6d5f034
--- /dev/null
+++ b/src/lib/Kconfig
@@ -0,0 +1,7 @@
+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.
diff --git a/src/lib/Makefile.inc b/src/lib/Makefile.inc
index 55f5960..c4d4f6a 100644
--- a/src/lib/Makefile.inc
+++ b/src/lib/Makefile.inc
@@ -131,6 +131,9 @@
 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
diff --git a/src/lib/edid.c b/src/lib/edid.c
index 2216690..7ad4136 100644
--- a/src/lib/edid.c
+++ b/src/lib/edid.c
@@ -94,9 +94,6 @@
 
 static struct edid tmp_edid;
 
-static int vbe_valid;
-static struct lb_framebuffer edid_fb;
-
 static char *manufacturer_name(unsigned char *x)
 {
 	extra_info.manuf_name[0] = ((x[0] & 0x7C) >> 2) + '@';
@@ -1693,70 +1690,3 @@
 	edid->x_resolution = edid->bytes_per_line / (fb_bpp / 8);
 	edid->y_resolution = edid->mode.va;
 }
-
-/*
- * Take an edid, and create a framebuffer. Set vbe_valid to 1.
- */
-
-void set_vbe_mode_info_valid(const struct edid *edid, uintptr_t fb_addr)
-{
-	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;
-	}
-
-	vbe_valid = 1;
-}
-
-#if IS_ENABLED(CONFIG_NATIVE_VGA_INIT_USE_EDID)
-int fill_lb_framebuffer(struct lb_framebuffer *framebuffer)
-{
-	if (!vbe_valid)
-		return -1;
-
-	*framebuffer = edid_fb;
-
-	return 0;
-}
-#endif
diff --git a/src/lib/edid_fill_fb.c b/src/lib/edid_fill_fb.c
new file mode 100644
index 0000000..210c727
--- /dev/null
+++ b/src/lib/edid_fill_fb.c
@@ -0,0 +1,92 @@
+/*
+ * Copyright 2013 Google Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * on the rights to use, copy, modify, merge, publish, distribute, sub
+ * license, and/or sell copies of the Software, and to permit persons to whom
+ * the Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER
+ * IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+#include <console/console.h>
+#include <edid.h>
+#include <boot/coreboot_tables.h>
+
+static int fb_valid;
+static struct lb_framebuffer edid_fb;
+
+/*
+ * Take an edid, and create a framebuffer. Set fb_valid to 1.
+ */
+void set_vbe_mode_info_valid(const struct edid *edid, uintptr_t fb_addr)
+{
+	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;
+	}
+
+	fb_valid = 1;
+}
+
+int fill_lb_framebuffer(struct lb_framebuffer *framebuffer)
+{
+	if (!fb_valid)
+		return -1;
+
+	*framebuffer = edid_fb;
+
+	return 0;
+}

-- 
To view, visit https://review.coreboot.org/19764
To unsubscribe, visit https://review.coreboot.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I90634b835bd8e2d150b1c714328a5b2774d891bd
Gerrit-PatchSet: 7
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Nico Huber <nico.h at gmx.de>
Gerrit-Reviewer: Aaron Durbin <adurbin at chromium.org>
Gerrit-Reviewer: Arthur Heymans <arthur at aheymans.xyz>
Gerrit-Reviewer: Julius Werner <jwerner at chromium.org>
Gerrit-Reviewer: Julius Werner <jwerner at google.com>
Gerrit-Reviewer: Martin Roth <martinroth at google.com>
Gerrit-Reviewer: Nico Huber <nico.h at gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter at users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply at coreboot.org>



More information about the coreboot-gerrit mailing list