On Thu, Sep 02, 2021 at 04:08:14PM +0200, Noralf Trønnes wrote:
>
>
> Den 31.08.2021 14.23, skrev Daniel Vetter:
> > On Mon, Aug 30, 2021 at 02:08:14PM +0200, Noralf Trønnes wrote:
> >>
> >>
> >> Den 17.08.2021 15.56, skrev Daniel Vetter:
> >>> On Tue, Aug 17, 2021 at 02:29:12PM +0200, Noralf Trønnes wrote:
> >>>> Add XRGB8888 emulation support for devices that can only do RGB332.
> >>>>
> >>>> Cc: Thomas Zimmermann <tzimmermann@xxxxxxx>
> >>>> Signed-off-by: Noralf Trønnes <noralf@xxxxxxxxxxx>
> >>>> ---
> >>>> drivers/gpu/drm/drm_format_helper.c | 47 +++++++++++++++++++++++++++++
> >>>> include/drm/drm_format_helper.h | 2 ++
> >>>> 2 files changed, 49 insertions(+)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
> >>>> index 5231104b1498..53b426da7467 100644
> >>>> --- a/drivers/gpu/drm/drm_format_helper.c
> >>>> +++ b/drivers/gpu/drm/drm_format_helper.c
> >>>> @@ -135,6 +135,53 @@ void drm_fb_swab(void *dst, void *src, struct drm_framebuffer *fb,
> >>>> }
> >>>> EXPORT_SYMBOL(drm_fb_swab);
> >>>>
> >>>> +static void drm_fb_xrgb8888_to_rgb332_line(u8 *dbuf, u32 *sbuf, unsigned int pixels)
> >>>> +{
> >>>> + unsigned int x;
> >>>> +
> >>>> + for (x = 0; x < pixels; x++)
> >>>> + dbuf[x] = ((sbuf[x] & 0x00e00000) >> 16) |
> >>>
> >>> I think for 2/3 bits correct rounding would be useful, not just masking.
> >>> i.e. before you shift add 0x00100000 here, and similar below.
> >>>
> >>
> >> Math isn't my strongest side and my brain failed to turn this into code.
> >
> > Essentially add half of the lowest bit before you mask, so
> >
> > ((sbuf[x] + 0x10) & 0xe0 )
> >
>
> But what if the value is 0xff, it overflows:
Your math is better than mine ...
> ((0xff + 0x10) & 0xe0 ) == 0x00
>
> Should it be OR?
>
> ((0xff | 0x10) & 0xe0 ) == 0xe0
Probably need a max on top to limit the overflow:
max(sbuf[x] + 0x10, 0xe)
But also maybe really not worth the head-scratching :-)
-Daniel
>
> Noralf.
>
> > I dropped the shift to make it clear what's going on. If you're mask is
> > 0xc0, then you simply add 0x20 before masking.
> >
> >>> Also I just realized we've totally ignored endianess on these, which is
> >>> not great, because strictly speaking all the drm_fourcc codes should be
> >>> little endian. But I'm also not sure that's worth fixing ...
> >>>
> >>
> >> Is it as simple as using le32_to_cpu()?
> >
> > I think so.
> >
> > Plus on any format that has u16 output we'd need a cpu_to_le16 wrapped
> > around the entire thing.
> > -Daniel
> >
> >> static void drm_fb_xrgb8888_to_rgb332_line(u8 *dbuf, __le32 *sbuf,
> >> unsigned int pixels)
> >> {
> >> unsigned int x;
> >> u32 pix;
> >>
> >> for (x = 0; x < pixels; x++) {
> >> pix = le32_to_cpu(sbuf[x]);
> >> dbuf[x] = ((pix & 0x00e00000) >> 16) |
> >> ((pix & 0x0000e000) >> 11) |
> >> ((pix & 0x000000c0) >> 6);
> >> }
> >> }
> >>
> >> Noralf.
> >>
> >>> Either way, lgtm:
> >>>
> >>> Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
> >>>
> >>>> + ((sbuf[x] & 0x0000e000) >> 11) |
> >>>> + ((sbuf[x] & 0x000000c0) >> 6);
> >>>> +}
> >>>> +
> >>>> +/**
> >>>> + * drm_fb_xrgb8888_to_rgb332 - Convert XRGB8888 to RGB332 clip buffer
> >>>> + * @dst: RGB332 destination buffer
> >>>> + * @src: XRGB8888 source buffer
> >>>> + * @fb: DRM framebuffer
> >>>> + * @clip: Clip rectangle area to copy
> >>>> + *
> >>>> + * Drivers can use this function for RGB332 devices that don't natively support XRGB8888.
> >>>> + *
> >>>> + * This function does not apply clipping on dst, i.e. the destination is a small buffer
> >>>> + * containing the clip rect only.
> >>>> + */
> >>>> +void drm_fb_xrgb8888_to_rgb332(void *dst, void *src, struct drm_framebuffer *fb,
> >>>> + struct drm_rect *clip)
> >>>> +{
> >>>> + size_t width = drm_rect_width(clip);
> >>>> + size_t src_len = width * sizeof(u32);
> >>>> + unsigned int y;
> >>>> + void *sbuf;
> >>>> +
> >>>> + /* Use a buffer to speed up access on buffers with uncached read mapping (i.e. WC) */
> >>>> + sbuf = kmalloc(src_len, GFP_KERNEL);
> >>>> + if (!sbuf)
> >>>> + return;
> >>>> +
> >>>> + src += clip_offset(clip, fb->pitches[0], sizeof(u32));
> >>>> + for (y = 0; y < drm_rect_height(clip); y++) {
> >>>> + memcpy(sbuf, src, src_len);
> >>>> + drm_fb_xrgb8888_to_rgb332_line(dst, sbuf, width);
> >>>> + src += fb->pitches[0];
> >>>> + dst += width;
> >>>> + }
> >>>> +
> >>>> + kfree(sbuf);
> >>>> +}
> >>>> +EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb332);
> >>>> +
> >>>> static void drm_fb_xrgb8888_to_rgb565_line(u16 *dbuf, u32 *sbuf,
> >>>> unsigned int pixels,
> >>>> bool swab)
> >>>> diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
> >>>> index 4e0258a61311..d0809aff5cf8 100644
> >>>> --- a/include/drm/drm_format_helper.h
> >>>> +++ b/include/drm/drm_format_helper.h
> >>>> @@ -16,6 +16,8 @@ void drm_fb_memcpy_dstclip(void __iomem *dst, unsigned int dst_pitch, void *vadd
> >>>> struct drm_rect *clip);
> >>>> void drm_fb_swab(void *dst, void *src, struct drm_framebuffer *fb,
> >>>> struct drm_rect *clip, bool cached);
> >>>> +void drm_fb_xrgb8888_to_rgb332(void *dst, void *vaddr, struct drm_framebuffer *fb,
> >>>> + struct drm_rect *clip);
> >>>> void drm_fb_xrgb8888_to_rgb565(void *dst, void *vaddr,
> >>>> struct drm_framebuffer *fb,
> >>>> struct drm_rect *clip, bool swab);
> >>>> --
> >>>> 2.32.0
> >>>>
> >>>
> >
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
