edid: Add helper function to calculate bits-per-pixel dependent values

Coreboot and most payloads support three basic pixel widths for the
framebuffer. It assumes 32 by default, but several chipsets need to
override that value with whatever else they're supporting. Our struct
edid contains multiple convenience values that are directly derived from
this (and other properties), so changing the bits per pixel always
requires recalculating all those dependents in the chipset code. This
patch provides a small convenience wrapper that can be used to
consistently update the whole struct edid with a new pixel width
instead, so we no longer need to duplicate those calculations
everywhere.

BUG=None
TEST=Booted Oak in all three pixel widths (which it conveniently all
supports), confirmed that images looked good.

Change-Id: I5376dd4e28cf107ac2fba1dc418f5e1c5a2e2de6
Signed-off-by: Julius Werner <jwerner@chromium.org>
Reviewed-on: https://review.coreboot.org/14158
Tested-by: build bot (Jenkins)
Reviewed-by: Patrick Georgi <pgeorgi@google.com>

8 files changed, 40 insertions, 55 deletions

diff --git a/src/drivers/emulation/qemu/bochs.c b/src/drivers/emulation/qemu/bochs.c
index ae2975d543..f32fee008c 100644
--- a/src/drivers/emulation/qemu/bochs.c
+++ b/src/drivers/emulation/qemu/bochs.c
@@ -111,12 +111,9 @@ static void bochs_init(struct device *dev)
 	/* setup coreboot framebuffer */
 	edid.mode.ha = width;
 	edid.mode.va = height;
-	edid.x_resolution = width;
-	edid.y_resolution = height;
-	edid.bytes_per_line = width * 4;
-	edid.framebuffer_bits_per_pixel = 32;
 	edid.panel_bits_per_color = 8;
 	edid.panel_bits_per_pixel = 24;
+	edid_set_framebuffer_bits_per_pixel(&edid, 32);
 	set_vbe_mode_info_valid(&edid, addr);
 #else
 	vga_misc_write(0x1);
diff --git a/src/drivers/emulation/qemu/cirrus.c b/src/drivers/emulation/qemu/cirrus.c
index 42cc120215..84c2958031 100644
--- a/src/drivers/emulation/qemu/cirrus.c
+++ b/src/drivers/emulation/qemu/cirrus.c
@@ -330,12 +330,9 @@ static void cirrus_init(struct device *dev)
 	struct edid edid;
 	edid.mode.ha = width;
 	edid.mode.va = height;
-	edid.x_resolution = width;
-	edid.y_resolution = height;
-	edid.bytes_per_line = width * 4;
-	edid.framebuffer_bits_per_pixel = 32;
 	edid.panel_bits_per_color = 8;
 	edid.panel_bits_per_pixel = 24;
+	edid_set_framebuffer_bits_per_pixel(&edid, 32);
 	set_vbe_mode_info_valid(&edid, addr);
 #else
 	vga_misc_write(0x1);
diff --git a/src/include/edid.h b/src/include/edid.h
index d3cab17f49..f46c761935 100644
--- a/src/include/edid.h
+++ b/src/include/edid.h
@@ -93,6 +93,7 @@ struct edid {
 
 /* Defined in src/lib/edid.c */
 int decode_edid(unsigned char *edid, int size, struct edid *out);
+void edid_set_framebuffer_bits_per_pixel(struct edid *edid, int fb_bpp);
 void set_vbe_mode_info_valid(struct edid *edid, uintptr_t fb_addr);
 int set_display_mode(struct edid *edid, enum edid_modes mode);
 
diff --git a/src/lib/edid.c b/src/lib/edid.c
index 3aebc65e00..52898fbfa6 100644
--- a/src/lib/edid.c
+++ b/src/lib/edid.c
@@ -28,6 +28,7 @@
  * at present.
  */
 
+#include <assert.h>
 #include <stddef.h>
 #include <console/console.h>
 #include <arch/io.h>
@@ -478,30 +479,12 @@ detailed_block(struct edid *result_edid, unsigned char *x, int in_extension,
 	out->mode.vso = ((x[10] >> 4) + ((x[11] & 0x0C) << 2));
 	out->mode.vspw = ((x[10] & 0x0F) + ((x[11] & 0x03) << 4));
 	out->mode.vborder = x[16];
-	/* set up some reasonable defaults for payloads.
-	 * We observe that most modern chipsets we work with
-	 * tend to support rgb888 without regard to the
-	 * panel bits per color or other settings. The rgb888
-	 * is a convenient layout for software because
-	 * it avoids the messy bit stuffing of rgb565 or rgb444.
-	 * It makes a reasonable trade of memory for speed.
-	 * So, set up the default for
-	 * 32 bits per pixel
-	 * rgb888 (i.e. no alpha, but pixels on 32-bit boundaries)
-	 * The mainboard can modify these if needed, though
-	 * we have yet to see a case where that will happen.
-	 * The existing ARM mainboards don't even call this function
-	 * so this will not affect them.
-	 */
-	out->framebuffer_bits_per_pixel = 32;
-
-	out->x_resolution = ALIGN(out->mode.ha *
-				  ((out->framebuffer_bits_per_pixel + 7) / 8),
-				  64) / (out->framebuffer_bits_per_pixel/8);
-	out->y_resolution = out->mode.va;
-	out->bytes_per_line = ALIGN(out->mode.ha *
-				    ((out->framebuffer_bits_per_pixel + 7)/8),
-				    64);
+
+	/* We assume rgb888 (32 bits per pixel) framebuffers by default.
+	 * Chipsets that want something else will need to override this
+	 * with another call to edid_set_framebuffer_bits_per_pixel(). */
+	edid_set_framebuffer_bits_per_pixel(out, 32);
+
 	switch ((x[17] & 0x18) >> 3) {
 	case 0x00:
 		extra_info.syncmethod = " analog composite";
@@ -1539,6 +1522,20 @@ int decode_edid(unsigned char *edid, int size, struct edid *out)
  * SPWG also says something strange about the LSB of detailed descriptor 1:
  * "LSB is set to "1" if panel is DE-timing only. H/V can be ignored."
  */
+
+/* Set the framebuffer bits-per-pixel, recalculating all dependent values. */
+void edid_set_framebuffer_bits_per_pixel(struct edid *edid, int fb_bpp)
+{
+	/* Caller should pass a supported value, everything else is BUG(). */
+	assert(fb_bpp == 32 || fb_bpp == 24 || fb_bpp == 16);
+
+	edid->framebuffer_bits_per_pixel = fb_bpp;
+	edid->bytes_per_line = ALIGN_UP(edid->mode.ha *
+					div_round_up(fb_bpp, 8), 64);
+	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.
  */
diff --git a/src/soc/nvidia/tegra124/display.c b/src/soc/nvidia/tegra124/display.c
index ac1b950b0b..74202bad23 100644
--- a/src/soc/nvidia/tegra124/display.c
+++ b/src/soc/nvidia/tegra124/display.c
@@ -331,10 +331,9 @@ void display_startup(device_t dev)
 	/* tell depthcharge ...
 	 */
 	struct edid edid;
-	edid.bytes_per_line = ((config->xres * config->framebuffer_bits_per_pixel / 8 + 31) /
-				32 * 32);
-	edid.x_resolution = edid.bytes_per_line / (config->framebuffer_bits_per_pixel / 8);
-	edid.y_resolution = config->yres;
-	edid.framebuffer_bits_per_pixel = config->framebuffer_bits_per_pixel;
+	edid.mode.va = config->yres;
+	edid.mode.ha = config->xres;
+	edid_set_framebuffer_bits_per_pixel(&edid,
+					    config->framebuffer_bits_per_pixel);
 	set_vbe_mode_info_valid(&edid, (uintptr_t)(framebuffer_base_mb*MiB));
 }
diff --git a/src/soc/nvidia/tegra132/dc.c b/src/soc/nvidia/tegra132/dc.c
index 2b1687b631..562061a1d3 100644
--- a/src/soc/nvidia/tegra132/dc.c
+++ b/src/soc/nvidia/tegra132/dc.c
@@ -226,13 +226,11 @@ void pass_mode_info_to_payload(
 			struct soc_nvidia_tegra132_config *config)
 {
 	struct edid edid;
-	/* Align bytes_per_line to 64 bytes as required by dc */
-	edid.bytes_per_line = ALIGN_UP((config->display_xres *
-				config->framebuffer_bits_per_pixel / 8), 64);
-	edid.x_resolution = edid.bytes_per_line /
-				(config->framebuffer_bits_per_pixel / 8);
-	edid.y_resolution = config->display_yres;
-	edid.framebuffer_bits_per_pixel = config->framebuffer_bits_per_pixel;
+
+	edid.mode.va = config->display_yres;
+	edid.mode.ha = config->display_xres;
+	edid_set_framebuffer_bits_per_pixel(&edid,
+					    config->framebuffer_bits_per_pixel);
 
 	printk(BIOS_INFO, "%s: bytes_per_line: %d, bits_per_pixel: %d\n "
 			"               x_res x y_res: %d x %d, size: %d\n",
diff --git a/src/soc/nvidia/tegra210/dc.c b/src/soc/nvidia/tegra210/dc.c
index 5dad92e6b8..72f1bcb3fa 100644
--- a/src/soc/nvidia/tegra210/dc.c
+++ b/src/soc/nvidia/tegra210/dc.c
@@ -226,13 +226,11 @@ void pass_mode_info_to_payload(
 			struct soc_nvidia_tegra210_config *config)
 {
 	struct edid edid;
-	/* Align bytes_per_line to 64 bytes as required by dc */
-	edid.bytes_per_line = ALIGN_UP((config->display_xres *
-				config->framebuffer_bits_per_pixel / 8), 64);
-	edid.x_resolution = edid.bytes_per_line /
-				(config->framebuffer_bits_per_pixel / 8);
-	edid.y_resolution = config->display_yres;
-	edid.framebuffer_bits_per_pixel = config->framebuffer_bits_per_pixel;
+
+	edid.mode.va = config->display_yres;
+	edid.mode.ha = config->display_xres;
+	edid_set_framebuffer_bits_per_pixel(&edid,
+					    config->framebuffer_bits_per_pixel);
 
 	printk(BIOS_INFO, "%s: bytes_per_line: %d, bits_per_pixel: %d\n "
 			"               x_res x y_res: %d x %d, size: %d\n",
diff --git a/src/soc/rockchip/rk3288/display.c b/src/soc/rockchip/rk3288/display.c
index 56eea076f8..66b2edc5b8 100644
--- a/src/soc/rockchip/rk3288/display.c
+++ b/src/soc/rockchip/rk3288/display.c
@@ -94,10 +94,8 @@ void rk_display_init(device_t dev, u32 lcdbase,
 		return;
 	}
 
-	edid.framebuffer_bits_per_pixel = conf->framebuffer_bits_per_pixel;
-	edid.bytes_per_line = edid.mode.ha * conf->framebuffer_bits_per_pixel / 8;
-	edid.x_resolution = edid.mode.ha;
-	edid.y_resolution = edid.mode.va;
+	edid_set_framebuffer_bits_per_pixel(&edid,
+					    conf->framebuffer_bits_per_pixel);
 	rkvop_mode_set(conf->vop_id, &edid, detected_mode);
 
 	rkvop_enable(conf->vop_id, lcdbase, &edid);