Patrick Rudolph 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 16:
(5 comments)
https://review.coreboot.org/c/coreboot/+/39004/1/src/drivers/aspeed/common/a... File src/drivers/aspeed/common/ast_mode_corebootfb.c:
https://review.coreboot.org/c/coreboot/+/39004/1/src/drivers/aspeed/common/a... PS1, Line 256: }
Log the failure case?
reworked the code, this no longer applies
https://review.coreboot.org/c/coreboot/+/39004/4/src/soc/nvidia/tegra210/dc.... File src/soc/nvidia/tegra210/dc.c:
https://review.coreboot.org/c/coreboot/+/39004/4/src/soc/nvidia/tegra210/dc.... PS4, Line 231: config->framebuffer_base?
It looks like copy-paste residue. The config value is nowhere used, not […]
Done
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. […]
Done
https://review.coreboot.org/c/coreboot/+/39004/7/src/soc/nvidia/tegra210/dc.... PS7, Line 231: //FIXME: Why not config->framebuffer_base?
Well, we definitely need all the other information stuff (height, width, bpp, etc. […]
Done
https://review.coreboot.org/c/coreboot/+/39004/8/src/soc/nvidia/tegra210/dc.... File src/soc/nvidia/tegra210/dc.c:
https://review.coreboot.org/c/coreboot/+/39004/8/src/soc/nvidia/tegra210/dc.... PS8, Line 233: bytes_per_line, config->framebuffer_bits_per_pixel);
Isn't this missing the row alignment field?
Done