New patch to review for coreboot: edid: Add helper function to calculate bits-per-pixel dependent values
Julius Werner (jwerner@chromium.org) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/14158
-gerrit
commit 56a6d3a48a64a97e6057f4c8e91a54760423e98c Author: Julius Werner jwerner@chromium.org Date: Mon Mar 14 17:29:55 2016 -0700
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 --- src/drivers/emulation/qemu/bochs.c | 5 +---- src/drivers/emulation/qemu/cirrus.c | 5 +---- src/include/edid.h | 1 + src/lib/edid.c | 45 +++++++++++++++++-------------------- src/soc/nvidia/tegra124/display.c | 9 ++++---- src/soc/nvidia/tegra132/dc.c | 12 +++++----- src/soc/nvidia/tegra210/dc.c | 12 +++++----- src/soc/rockchip/rk3288/display.c | 6 ++--- 8 files changed, 40 insertions(+), 55 deletions(-)
diff --git a/src/drivers/emulation/qemu/bochs.c b/src/drivers/emulation/qemu/bochs.c index ae2975d..7166378 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 42cc120..0b5316e 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 d3cab17..f46c761 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 3aebc65..52898fb 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 ac1b950..3108a74 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 2b1687b..9ab278c 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 5dad92e..7d44349 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 56eea07..6083363 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);
-
Julius Werner