Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33737 )
Change subject: device/oprom: Add vbe return status support as per VBE spec 3.0 ......................................................................
Patch Set 8:
(1 comment)
Patch Set 8:
Sorry for the late review, I've little time at the moment. And this code for legacy concepts and binary blobs is really not what I like to work on. If you would write modern open-source code for your graphics cards instead, you would have my full attention :D
@Nico: i don't understand what you mean by "your graphics card" ? i was explaining what i would like to achieve here. I've not added any new piece of code of oprom here. I'm just saving developers time to debug issue when coreboot Kconfig forces to set one mode which is not supported by opRom card.
It took me some time to figure out that the code you edit in the two commits ahead (yabel) and this (realmode) code are mutually exclusive. You are going through a lot of trouble to make the existing `vbe_info_t` compatible to your new code here. It would be much easier to declare a new struct. Or instead of squeezing the yabel code into a new shape, you could make its vbe_info code (vbe_info() and vbe_get_info()) common to both implementations. The existing yabel code looks more comprehen- sive than the mode listing here.
yable and realmode mode both are referring from same vbe spec hence i don't it would matter. But as i said, existing yabel code itself has bug, mentioned here: https://review.coreboot.org/c/coreboot/+/33782/1/src/include/vbe.h#48
https://review.coreboot.org/c/coreboot/+/33737/8/src/device/oprom/realmode/x... File src/device/oprom/realmode/x86.c:
https://review.coreboot.org/c/coreboot/+/33737/8/src/device/oprom/realmode/x... PS8, Line 246: mode = *video_mode_ptr++;
I'm confused by this dereference. Isn't this a segment:offset […]
we have already copied it into buffer right ?