Messages in this thread | | | Subject | Re: [PATCH v2 2/3] drm/format-helper: Add drm_fb_xrgb8888_to_xrgb2101010_toio() | From | Hector Martin <> | Date | Tue, 7 Dec 2021 18:54:18 +0900 |
| |
Hi, thanks for the review!
On 07/12/2021 18.40, Thomas Zimmermann wrote: > Hi > > Am 07.12.21 um 08:29 schrieb Hector Martin: >> Add XRGB8888 emulation support for devices that can only do XRGB2101010. >> >> This is chiefly useful for simpledrm on Apple devices where the >> bootloader-provided framebuffer is 10-bit. >> >> Signed-off-by: Hector Martin <marcan@marcan.st> >> --- >> drivers/gpu/drm/drm_format_helper.c | 62 +++++++++++++++++++++++++++++ >> include/drm/drm_format_helper.h | 3 ++ >> 2 files changed, 65 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c >> index dbe3e830096e..edd611d3ab6a 100644 >> --- a/drivers/gpu/drm/drm_format_helper.c >> +++ b/drivers/gpu/drm/drm_format_helper.c >> @@ -409,6 +409,59 @@ void drm_fb_xrgb8888_to_rgb888_toio(void __iomem *dst, unsigned int dst_pitch, >> } >> EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb888_toio); >> >> +static void drm_fb_xrgb8888_to_xrgb2101010_line(u32 *dbuf, const u32 *sbuf, >> + unsigned int pixels) >> +{ >> + unsigned int x; >> + >> + for (x = 0; x < pixels; x++) { >> + *dbuf++ = ((sbuf[x] & 0x000000FF) << 2) | >> + ((sbuf[x] & 0x0000FF00) << 4) | >> + ((sbuf[x] & 0x00FF0000) << 6); > > This isn't quite right. The lowest two destination bits in each > component will always be zero. You have to do the shifting as above and > for each component the two highest source bits have to be OR'ed into the > two lowest destination bits. For example the source bits in a component > are numbered 7 to 0 > > | 7 6 5 4 3 2 1 0 | > > then the destination bits should be > > | 7 6 5 4 3 2 1 0 7 6 | >
I think both approaches have pros and cons. Leaving the two LSBs always at 0 yields a fully linear transfer curve with no discontinuities, but means the maximum brightness is slightly less than full. Setting them fully maps the brightness range, but creates 4 double wide steps in the transfer curve (also it's potentially slightly slower CPU-wise).
If you prefer the latter I'll do that for v2.
-- Hector Martin (marcan@marcan.st) Public Key: https://mrcn.st/pub
| |