Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39004 )
Change subject: drivers: Replace a few set_vbe_mode_info_valid ......................................................................
Patch Set 7:
(2 comments)
https://review.coreboot.org/c/coreboot/+/39004/7/src/soc/nvidia/tegra210/dc.... File src/soc/nvidia/tegra210/dc.c:
https://review.coreboot.org/c/coreboot/+/39004/7/src/soc/nvidia/tegra210/dc.... PS7, Line 230: DIV_ROUND_UP(config->framebuffer_bits_per_pixel, 8), 64); I would rather keep this calculation centralized. bytes_per_line is really not a property of the EDID (i.e the display itself) anyway, but rather one of the framebuffer, so it's probably time to clean that up. How about getting rid of that field from struct edid and of edid_set_framebuffer_bits_per_pixel(), and instead add a row_alignment parameter to fb_set_framebuffer_info() and set_vbe_mode_info_valid(), so that the framebuffer code can correctly calculate it internally?
https://review.coreboot.org/c/coreboot/+/39004/7/src/soc/nvidia/tegra210/dc.... PS7, Line 231: //FIXME: Why not config->framebuffer_base? For Arm64 we switched to the payload allocating the framebuffer itself (which became necessary due to MMU changes). config->framebuffer_base is probably a leftover from when this SoC was first forked off the older Arm32 chip, and was never used.