Den 09.04.2019 13.59, skrev Gerd Hoffmann: > Also add two conversion functions for the swab / not swab cases. > That way we have to check the swab flag only once. > > Signed-off-by: Gerd Hoffmann <kra...@redhat.com> > --- > drivers/gpu/drm/drm_format_helper.c | 117 +++++++++++++++------------- > 1 file changed, 62 insertions(+), 55 deletions(-) > > diff --git a/drivers/gpu/drm/drm_format_helper.c > b/drivers/gpu/drm/drm_format_helper.c > index f32e0173600c..b8d17cd0a9f8 100644 > --- a/drivers/gpu/drm/drm_format_helper.c > +++ b/drivers/gpu/drm/drm_format_helper.c > @@ -38,6 +38,46 @@ static struct drm_format_convert convert_swab16 = { > .func = convert_swab16_fn, > }; > > +static void convert_xrgb8888_to_rgb565_fn(void *dst, void *src, u32 pixels) > +{ > + u32 *sbuf = src; > + u16 *dbuf = dst; > + u32 x; > + > + for (x = 0; x < pixels; x++) { > + dbuf[x] = ((sbuf[x] & 0x00F80000) >> 8) | > + ((sbuf[x] & 0x0000FC00) >> 5) | > + ((sbuf[x] & 0x000000F8) >> 3); > + } > +} > + > +static struct drm_format_convert convert_xrgb8888_to_rgb565 = { > + .dst_cpp = 2, > + .src_cpp = 4, > + .func = convert_xrgb8888_to_rgb565_fn, > +}; > + > +static void convert_xrgb8888_to_rgb565_swab_fn(void *dst, void *src, u32 > pixels) > +{ > + u32 *sbuf = src; > + u16 *dbuf = dst; > + u16 val16; > + u32 x; > + > + for (x = 0; x < pixels; x++) { > + val16 = ((sbuf[x] & 0x00F80000) >> 8) | > + ((sbuf[x] & 0x0000FC00) >> 5) | > + ((sbuf[x] & 0x000000F8) >> 3); > + dbuf[x] = swab16(val16); > + } > +} > + > +static struct drm_format_convert convert_xrgb8888_to_rgb565_swap = { > + .dst_cpp = 2, > + .src_cpp = 4, > + .func = convert_xrgb8888_to_rgb565_swab_fn, > +}; > +
I find that drm_format_convert makes the code more difficult to read. The only shared code is the xrgb8888 to rgb565 conversion. Can we do something like this instead: static void drm_fb_xrgb8888_to_rgb565_line(u16 *dst, u32 *src, unsigned int width, bool swap) { unsigned int x; for (x = 0; x < width; x++) { u16 val = ((*src & 0x00F80000) >> 8) | ((*src & 0x0000FC00) >> 5) | ((*src & 0x000000F8) >> 3); src++; if (swap) *dst++ = swab16(val); else *dst++ = val; } } void drm_fb_xrgb8888_to_rgb565(void *dst, void *vaddr, struct drm_framebuffer *fb, struct drm_rect *clip, bool swap) { void *src = vaddr + clip_offset(clip, fb->pitches[0], 4); unsigned width = clip->x2 - clip->x1; unsigned height = clip->y2 - clip->y1; size_t src_clip_pitch = width * 4; size_t dst_clip_pitch = width * 2; unsigned int y; void *sbuf; /* * The cma memory is write-combined so reads are uncached. * Speed up by fetching one line at a time. */ sbuf = kmalloc(src_clip_pitch, GFP_KERNEL); if (!sbuf) return; for (y = 0; y < height; y++) { memcpy(sbuf, src, src_clip_pitch); drm_fb_xrgb8888_to_rgb565_line(dst, sbuf, width, swap); src += fb->pitches[0]; dst += dst_clip_pitch; } kfree(sbuf); } EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb565); void drm_fb_xrgb8888_to_rgb565_dstclip(void __iomem *dst, unsigned int dst_pitch, void *vaddr, struct drm_framebuffer *fb, struct drm_rect *clip, bool swap) { void *src = vaddr + clip_offset(clip, fb->pitches[0], 4); unsigned width = clip->x2 - clip->x1; unsigned height = clip->y2 - clip->y1; size_t dst_clip_pitch = width * 2; unsigned int y; void *dbuf; dst += clip_offset(clip, dst_pitch, 2); dbuf = kmalloc(dst_clip_pitch, GFP_KERNEL); if (!dbuf) return; for (y = 0; y < height; y++) { drm_fb_xrgb8888_to_rgb565_line(dbuf, src, width, swap); memcpy_toio(dst, dbuf, dst_clip_pitch); src += fb->pitches[0]; dst += dst_pitch; } kfree(dbuf); } EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb565_dstclip); Other things I noticed: drm_fb_memcpy() is just a wrapper around drm_fb_memcpy_lines() and drm_fb_memcpy_dstclip() around drm_fb_memcpy_lines_toio(). They can be merged. checkpatch gave this: WARNING: Use #include <linux/io.h> instead of <asm/io.h> #50: FILE: drivers/gpu/drm/drm_format_helper.c:14: +#include <asm/io.h> Noralf. > static void convert_lines(void *dst, unsigned int dst_pitch, > void *src, unsigned int src_pitch, > unsigned int pixels, > @@ -152,44 +192,6 @@ void drm_fb_swab16(u16 *dst, void *vaddr, struct > drm_framebuffer *fb, > } > EXPORT_SYMBOL(drm_fb_swab16); > > -static void drm_fb_xrgb8888_to_rgb565_lines(void *dst, unsigned int > dst_pitch, > - void *src, unsigned int src_pitch, > - unsigned int src_linelength, > - unsigned int lines, > - bool swap) > -{ > - unsigned int linepixels = src_linelength / sizeof(u32); > - unsigned int x, y; > - u32 *sbuf; > - u16 *dbuf, val16; > - > - /* > - * The cma memory is write-combined so reads are uncached. > - * Speed up by fetching one line at a time. > - */ > - sbuf = kmalloc(src_linelength, GFP_KERNEL); > - if (!sbuf) > - return; > - > - for (y = 0; y < lines; y++) { > - memcpy(sbuf, src, src_linelength); > - dbuf = dst; > - for (x = 0; x < linepixels; x++) { > - val16 = ((sbuf[x] & 0x00F80000) >> 8) | > - ((sbuf[x] & 0x0000FC00) >> 5) | > - ((sbuf[x] & 0x000000F8) >> 3); > - if (swap) > - *dbuf++ = swab16(val16); > - else > - *dbuf++ = val16; > - } > - src += src_pitch; > - dst += dst_pitch; > - } > - > - kfree(sbuf); > -} > - > /** > * drm_fb_xrgb8888_to_rgb565 - Convert XRGB8888 to RGB565 clip buffer > * @dst: RGB565 destination buffer > @@ -208,15 +210,17 @@ void drm_fb_xrgb8888_to_rgb565(void *dst, void *vaddr, > struct drm_framebuffer *fb, > struct drm_rect *clip, bool swap) > { > - unsigned int src_offset = (clip->y1 * fb->pitches[0]) > - + (clip->x1 * sizeof(u32)); > - size_t src_len = (clip->x2 - clip->x1) * sizeof(u32); > - size_t dst_len = (clip->x2 - clip->x1) * sizeof(u16); > + struct drm_format_convert *conv = swap > + ? &convert_xrgb8888_to_rgb565_swap > + : &convert_xrgb8888_to_rgb565; > + unsigned int src_offset = > + clip_offset(clip, fb->pitches[0], conv->src_cpp); > + size_t pixels = (clip->x2 - clip->x1); > + size_t lines = (clip->y2 - clip->y1); > > - drm_fb_xrgb8888_to_rgb565_lines(dst, dst_len, > - vaddr + src_offset, fb->pitches[0], > - src_len, clip->y2 - clip->y1, > - swap); > + convert_lines(dst, pixels * conv->dst_cpp, > + vaddr + src_offset, fb->pitches[0], > + pixels, lines, conv); > } > EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb565); > > @@ -239,16 +243,19 @@ void drm_fb_xrgb8888_to_rgb565_dstclip(void *dst, > unsigned int dst_pitch, > void *vaddr, struct drm_framebuffer *fb, > struct drm_rect *clip, bool swap) > { > - unsigned int src_offset = (clip->y1 * fb->pitches[0]) > - + (clip->x1 * sizeof(u32)); > - unsigned int dst_offset = (clip->y1 * dst_pitch) > - + (clip->x1 * sizeof(u16)); > - size_t src_len = (clip->x2 - clip->x1) * sizeof(u32); > + struct drm_format_convert *conv = swap > + ? &convert_xrgb8888_to_rgb565_swap > + : &convert_xrgb8888_to_rgb565; > + unsigned int src_offset = > + clip_offset(clip, fb->pitches[0], conv->src_cpp); > + unsigned int dst_offset = > + clip_offset(clip, dst_pitch, conv->dst_cpp); > + size_t pixels = (clip->x2 - clip->x1); > + size_t lines = (clip->y2 - clip->y1); > > - drm_fb_xrgb8888_to_rgb565_lines(dst + dst_offset, dst_pitch, > - vaddr + src_offset, fb->pitches[0], > - src_len, clip->y2 - clip->y1, > - swap); > + convert_lines(dst + dst_offset, dst_pitch, > + vaddr + src_offset, fb->pitches[0], > + pixels, lines, conv); > } > EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb565_dstclip); > > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel