Re: [PATCH v2 3/6] drm: move tinydrm_xrgb8888_to_rgb565() to drm_fb_helper.c
Den 04.04.2019 17.24, skrev Gerd Hoffmann:
> Also rename to drm_fb_xrgb_to_rgb565().
> Pure code motion, no functional change.
>
> Signed-off-by: Gerd Hoffmann
> ---
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index fabeb408dce6..087e49741094 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -3380,3 +3380,48 @@ void drm_fb_memcpy(void *dst, void *vaddr, struct
> drm_framebuffer *fb,
> }
> }
> EXPORT_SYMBOL(drm_fb_memcpy);
> +
> +/**
> + * drm_fb_xrgb_to_rgb565 - Convert XRGB to RGB565 clip buffer
> + * @dst: RGB565 destination buffer
> + * @vaddr: XRGB source buffer
> + * @fb: DRM framebuffer
> + * @clip: Clip rectangle area to copy
> + * @swap: Swap bytes
> + *
> + * Drivers can use this function for RGB565 devices that don't natively
> + * support XRGB.
> + */
> +void drm_fb_xrgb_to_rgb565(u16 *dst, void *vaddr,
> +struct drm_framebuffer *fb,
> +struct drm_rect *clip, bool swap)
> +{
> + size_t len = (clip->x2 - clip->x1) * sizeof(u32);
> + unsigned int x, y;
> + u32 *src, *buf;
> + u16 val16;
> +
> + buf = kmalloc(len, GFP_KERNEL);
It's not obvious what purpose this buffer serves and there's no help
from the surrounding functions either (tinydrm_swab16() has a comment).
I think we need a comment, something along these lines:
/*
* Write-combined buffers have uncached reads.
* Speed up by fetching one line at a time.
*/
Noralf.
> + if (!buf)
> + return;
> +
> + for (y = clip->y1; y < clip->y2; y++) {
> + src = vaddr + (y * fb->pitches[0]);
> + src += clip->x1;
> + memcpy(buf, src, len);
> + src = buf;
> + for (x = clip->x1; x < clip->x2; x++) {
> + val16 = ((*src & 0x00F8) >> 8) |
> + ((*src & 0xFC00) >> 5) |
> + ((*src & 0x00F8) >> 3);
> + src++;
> + if (swap)
> + *dst++ = swab16(val16);
> + else
> + *dst++ = val16;
> + }
> + }
> +
> + kfree(buf);
> +}
> +EXPORT_SYMBOL(drm_fb_xrgb_to_rgb565);
> diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
> b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
> index f5c6f35e6161..f4b84b15da75 100644
> --- a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
> +++ b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
> @@ -61,51 +61,6 @@ void tinydrm_swab16(u16 *dst, void *vaddr, struct
> drm_framebuffer *fb,
> }
> EXPORT_SYMBOL(tinydrm_swab16);
>
> -/**
> - * tinydrm_xrgb_to_rgb565 - Convert XRGB to RGB565 clip buffer
> - * @dst: RGB565 destination buffer
> - * @vaddr: XRGB source buffer
> - * @fb: DRM framebuffer
> - * @clip: Clip rectangle area to copy
> - * @swap: Swap bytes
> - *
> - * Drivers can use this function for RGB565 devices that don't natively
> - * support XRGB.
> - */
> -void tinydrm_xrgb_to_rgb565(u16 *dst, void *vaddr,
> - struct drm_framebuffer *fb,
> - struct drm_rect *clip, bool swap)
> -{
> - size_t len = (clip->x2 - clip->x1) * sizeof(u32);
> - unsigned int x, y;
> - u32 *src, *buf;
> - u16 val16;
> -
> - buf = kmalloc(len, GFP_KERNEL);
> - if (!buf)
> - return;
> -
> - for (y = clip->y1; y < clip->y2; y++) {
> - src = vaddr + (y * fb->pitches[0]);
> - src += clip->x1;
> - memcpy(buf, src, len);
> - src = buf;
> - for (x = clip->x1; x < clip->x2; x++) {
> - val16 = ((*src & 0x00F8) >> 8) |
> - ((*src & 0xFC00) >> 5) |
> - ((*src & 0x00F8) >> 3);
> - src++;
> - if (swap)
> - *dst++ = swab16(val16);
> - else
> - *dst++ = val16;
> - }
> - }
> -
> - kfree(buf);
> -}
> -EXPORT_SYMBOL(tinydrm_xrgb_to_rgb565);
> -
> /**
> * tinydrm_xrgb_to_gray8 - Convert XRGB to grayscale
> * @dst: 8-bit grayscale destination buffer
> diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c
> b/drivers/gpu/drm/tinydrm/mipi-dbi.c
> index e26fd61360a3..cf8df5fb7494 100644
> --- a/drivers/gpu/drm/tinydrm/mipi-dbi.c
> +++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c
> @@ -224,7 +224,7 @@ int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer
> *fb,
> drm_fb_memcpy(dst, src, fb, clip, false);
> break;
> case DRM_FORMAT_XRGB:
> - tinydrm_xrgb_to_rgb565(dst, src, fb, clip, swap);
> + drm_fb_xrgb_to_rgb565(dst, src, fb, clip, swap);
> break;
>
Re: [PATCH v2 3/6] drm: move tinydrm_xrgb8888_to_rgb565() to drm_fb_helper.c
Hi Gerd. On Thu, Apr 04, 2019 at 05:24:27PM +0200, Gerd Hoffmann wrote: > Also rename to drm_fb_xrgb_to_rgb565(). > Pure code motion, no functional change. > > Signed-off-by: Gerd Hoffmann Reviewed-by: Sam Ravnborg ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 3/6] drm: move tinydrm_xrgb8888_to_rgb565() to drm_fb_helper.c
Also rename to drm_fb_xrgb_to_rgb565().
Pure code motion, no functional change.
Signed-off-by: Gerd Hoffmann
---
include/drm/drm_fb_helper.h | 3 ++
include/drm/tinydrm/tinydrm-helpers.h | 3 --
drivers/gpu/drm/drm_fb_helper.c | 45 +++
.../gpu/drm/tinydrm/core/tinydrm-helpers.c| 45 ---
drivers/gpu/drm/tinydrm/mipi-dbi.c| 2 +-
5 files changed, 49 insertions(+), 49 deletions(-)
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index a57420f40643..0350c7e9d6ba 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -644,5 +644,8 @@ drm_fb_helper_remove_conflicting_pci_framebuffers(struct
pci_dev *pdev,
void drm_fb_memcpy(void *dst, void *vaddr, struct drm_framebuffer *fb,
struct drm_rect *clip, bool dstclip);
+void drm_fb_xrgb_to_rgb565(u16 *dst, void *vaddr,
+ struct drm_framebuffer *fb,
+ struct drm_rect *clip, bool swap);
#endif
diff --git a/include/drm/tinydrm/tinydrm-helpers.h
b/include/drm/tinydrm/tinydrm-helpers.h
index 591ca83330c4..220677107e0c 100644
--- a/include/drm/tinydrm/tinydrm-helpers.h
+++ b/include/drm/tinydrm/tinydrm-helpers.h
@@ -48,9 +48,6 @@ int tinydrm_display_pipe_init(struct drm_device *drm,
void tinydrm_swab16(u16 *dst, void *vaddr, struct drm_framebuffer *fb,
struct drm_rect *clip);
-void tinydrm_xrgb_to_rgb565(u16 *dst, void *vaddr,
- struct drm_framebuffer *fb,
- struct drm_rect *clip, bool swap);
void tinydrm_xrgb_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer
*fb,
struct drm_rect *clip);
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index fabeb408dce6..087e49741094 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -3380,3 +3380,48 @@ void drm_fb_memcpy(void *dst, void *vaddr, struct
drm_framebuffer *fb,
}
}
EXPORT_SYMBOL(drm_fb_memcpy);
+
+/**
+ * drm_fb_xrgb_to_rgb565 - Convert XRGB to RGB565 clip buffer
+ * @dst: RGB565 destination buffer
+ * @vaddr: XRGB source buffer
+ * @fb: DRM framebuffer
+ * @clip: Clip rectangle area to copy
+ * @swap: Swap bytes
+ *
+ * Drivers can use this function for RGB565 devices that don't natively
+ * support XRGB.
+ */
+void drm_fb_xrgb_to_rgb565(u16 *dst, void *vaddr,
+ struct drm_framebuffer *fb,
+ struct drm_rect *clip, bool swap)
+{
+ size_t len = (clip->x2 - clip->x1) * sizeof(u32);
+ unsigned int x, y;
+ u32 *src, *buf;
+ u16 val16;
+
+ buf = kmalloc(len, GFP_KERNEL);
+ if (!buf)
+ return;
+
+ for (y = clip->y1; y < clip->y2; y++) {
+ src = vaddr + (y * fb->pitches[0]);
+ src += clip->x1;
+ memcpy(buf, src, len);
+ src = buf;
+ for (x = clip->x1; x < clip->x2; x++) {
+ val16 = ((*src & 0x00F8) >> 8) |
+ ((*src & 0xFC00) >> 5) |
+ ((*src & 0x00F8) >> 3);
+ src++;
+ if (swap)
+ *dst++ = swab16(val16);
+ else
+ *dst++ = val16;
+ }
+ }
+
+ kfree(buf);
+}
+EXPORT_SYMBOL(drm_fb_xrgb_to_rgb565);
diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
index f5c6f35e6161..f4b84b15da75 100644
--- a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
+++ b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
@@ -61,51 +61,6 @@ void tinydrm_swab16(u16 *dst, void *vaddr, struct
drm_framebuffer *fb,
}
EXPORT_SYMBOL(tinydrm_swab16);
-/**
- * tinydrm_xrgb_to_rgb565 - Convert XRGB to RGB565 clip buffer
- * @dst: RGB565 destination buffer
- * @vaddr: XRGB source buffer
- * @fb: DRM framebuffer
- * @clip: Clip rectangle area to copy
- * @swap: Swap bytes
- *
- * Drivers can use this function for RGB565 devices that don't natively
- * support XRGB.
- */
-void tinydrm_xrgb_to_rgb565(u16 *dst, void *vaddr,
- struct drm_framebuffer *fb,
- struct drm_rect *clip, bool swap)
-{
- size_t len = (clip->x2 - clip->x1) * sizeof(u32);
- unsigned int x, y;
- u32 *src, *buf;
- u16 val16;
-
- buf = kmalloc(len, GFP_KERNEL);
- if (!buf)
- return;
-
- for (y = clip->y1; y < clip->y2; y++) {
- src = vaddr + (y * fb->pitches[0]);
- src += clip->x1;
- memcpy(buf, src, len);
- src = buf;
- for (x = clip->x1; x < clip->x2; x++) {
-
