Hello Geert,
Thanks for your feedback.
On 2/12/22 16:54, Geert Uytterhoeven wrote:
[snip]
for (i = start; i < end; i++) {unsigned int x = xb * 8 + i;byte >>= 1;if (src[x] >> 7)byte |= BIT(7);}x = xb * 8; for (i = start; i < end; i++) { byte >>= 1; byte |= src[x + i] & BIT(7); }
I think the original version from Noralf is easier to read and understand.
It makes explicit that the bit is set if the grayscale value is >= 128.
[snip]
+void drm_fb_xrgb8888_to_mono_reversed(void *dst, unsigned int dst_pitch, const void *vaddr,
const struct drm_framebuffer *fb, const struct drm_rect *clip)+{
[snip]
u8 *mono = dst, *gray8;u32 *src32;
[snip]
** Allocate a buffer to be used for both copying from the cma* memory and to store the intermediate grayscale line pixels.*/src32 = kmalloc(len_src32 + linepixels, GFP_KERNEL);if (!src32)return;gray8 = (u8 *)src32 + len_src32;The cast can be removed if src32 is changed to "void *". For symmetry, gray8 and mono can be changed to "void *", too.
Yes, but these being "u32 *" and "u8 *" also express that the source buffer contains 32-bit XRGB8888 pixels, the intermediate buffer a 8-bit grayscale pixel format and the destination buffer a 8-bit packed reversed monochrome.
Using "void *" for these would make that less clear when reading the code IMO.
Best regards,