6 comments:
File src/include/framebuffer_info.h:
Any reason this now slipped into this patch? Seems to me like it belongs in CB:39430.
Patch Set #11, Line 35: static
Is there a good reason this is an inline? It's not *that* small...
Patch Set #11, Line 37: uint16_t bpp)
Since the use of this is pretty specific to VESA OPROMs, maybe it would be better to just make it take a vbe_mode_info_t* (e.g. call it vbe_get_row_alignment() or something and put it in src/include/vbe.h)?
Patch Set #11, Line 39: DIV_ROUND_UP(bpp, 8) / 8
If bpp is 24 this will be 3 but we need it to be 4. In fact, I think this is also a mistake in CB:39430. Maybe it would help to have a bytes_per_pixel(int bits_per_pixel) helper function to do that translation (should basically be (1 << log2_ceil(bits_per_pixel)) to always get the next-larger power of two).
Patch Set #11, Line 43: /* find first set bit */
We have an __ffs() function for this... but I don't think this is the right way to do this anyway. I think you just want to find the next power of two that's larger than the difference:
row_alignment = 1 << log2_ceil(scanline_bytes - h_active);
Odd indentation?
To view, visit change 39002. To unsubscribe, or for help writing mail filters, visit settings.