Re: [PATCH 5/9] drm/tinydrm: Add helper functions

[Daniel Vetter]

On Sun, Jan 22, 2017 at 07:11:13PM +0100, Noralf Trønnes wrote:
> Add common functionality needed by many tinydrm drivers.
> 
> Signed-off-by: Noralf Trønnes 

Bunch of comments below, all optional.
-Daniel

> ---
>  Documentation/gpu/drm-kms-helpers.rst  |   6 +
>  drivers/gpu/drm/tinydrm/core/Makefile  |   2 +-
>  drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c | 490 
> +
>  include/drm/tinydrm/tinydrm-helpers.h  | 100 +
>  4 files changed, 597 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
>  create mode 100644 include/drm/tinydrm/tinydrm-helpers.h
> 
> diff --git a/Documentation/gpu/drm-kms-helpers.rst 
> b/Documentation/gpu/drm-kms-helpers.rst
> index a86bd7f..be07e76 100644
> --- a/Documentation/gpu/drm-kms-helpers.rst
> +++ b/Documentation/gpu/drm-kms-helpers.rst
> @@ -287,3 +287,9 @@ tinydrm Helper Reference
>  
>  .. kernel-doc:: drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c
> :export:
> +
> +.. kernel-doc:: include/drm/tinydrm/tinydrm-helpers.h
> +   :internal:
> +
> +.. kernel-doc:: drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
> +   :export:
> diff --git a/drivers/gpu/drm/tinydrm/core/Makefile 
> b/drivers/gpu/drm/tinydrm/core/Makefile
> index 4f14a0f..fb221e6 100644
> --- a/drivers/gpu/drm/tinydrm/core/Makefile
> +++ b/drivers/gpu/drm/tinydrm/core/Makefile
> @@ -1,3 +1,3 @@
> -tinydrm-y := tinydrm-core.o tinydrm-pipe.o
> +tinydrm-y := tinydrm-core.o tinydrm-pipe.o tinydrm-helpers.o
>  
>  obj-$(CONFIG_DRM_TINYDRM) += tinydrm.o
> diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c 
> b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
> new file mode 100644
> index 000..fc02e01
> --- /dev/null
> +++ b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
> @@ -0,0 +1,490 @@
> +/*
> + * Copyright (C) 2016 Noralf Trønnes
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +static unsigned int spi_max;
> +module_param(spi_max, uint, 0400);
> +MODULE_PARM_DESC(spi_max, "Set a lower SPI max transfer size");
> +
> +/**
> + * tinydrm_merge_clips - Merge clip rectangles
> + * @dst: Destination clip rectangle
> + * @src: Source clip rectangle(s)
> + * @num_clips: Number of @src clip rectangles
> + * @flags: Dirty fb ioctl flags
> + * @max_width: Maximum width of @dst
> + * @max_height: Maximum height of @dst
> + *
> + * This function merges @src clip rectangle(s) into @dst. If @src is NULL,
> + * @max_width and @min_width is used to set a full @dst clip rectangle.
> + *
> + * Returns:
> + * true if it's a full clip, false otherwise
> + */
> +bool tinydrm_merge_clips(struct drm_clip_rect *dst,
> +  struct drm_clip_rect *src, unsigned int num_clips,
> +  unsigned int flags, u32 max_width, u32 max_height)
> +{
> + unsigned int i;
> +
> + if (!src || !num_clips) {
> + dst->x1 = 0;
> + dst->x2 = max_width;
> + dst->y1 = 0;
> + dst->y2 = max_height;
> + return true;
> + }
> +
> + dst->x1 = ~0;
> + dst->y1 = ~0;
> + dst->x2 = 0;
> + dst->y2 = 0;
> +
> + for (i = 0; i < num_clips; i++) {
> + if (flags & DRM_MODE_FB_DIRTY_ANNOTATE_COPY)
> + i++;
> + dst->x1 = min(dst->x1, src[i].x1);
> + dst->x2 = max(dst->x2, src[i].x2);
> + dst->y1 = min(dst->y1, src[i].y1);
> + dst->y2 = max(dst->y2, src[i].y2);
> + }
> +
> + if (dst->x2 > max_width || dst->y2 > max_height ||
> + dst->x1 >= dst->x2 || dst->y1 >= dst->y2) {
> + DRM_DEBUG_KMS("Illegal clip: x1=%u, x2=%u, y1=%u, y2=%u\n",
> +   dst->x1, dst->x2, dst->y1, dst->y2);
> + dst->x1 = 0;
> + dst->y1 = 0;
> + dst->x2 = max_width;
> + dst->y2 = max_height;
> + }
> +
> + return (dst->x2 - dst->x1) == max_width &&
> +(dst->y2 - dst->y1) == max_height;
> +}
> +EXPORT_SYMBOL(tinydrm_merge_clips);

Argh, the drm_clip_rect vs. drm_rect confusion strikes again :(

> +
> +/**
> + * tinydrm_memcpy - Copy clip buffer
> + * @dst: Destination buffer
> + * @vaddr: Source buffer
> + * @fb: DRM framebuffer
> + * @clip: Clip rectangle area to copy
> + */
> +void tinydrm_memcpy(void *dst, void *vaddr, struct drm_framebuffer *fb,
> + struct drm_clip_rect *clip)
> +{
> + unsigned int cpp = drm_format_plane_cpp(fb->format->format, 0);
> + unsigned int pitch = fb->pitches[0];
> + void *src = vaddr + (clip->y1 * pitch) + (clip->x1 * cpp);
> + size_t len = (clip->x2 - clip->x1) * cpp;
> + unsigned 

[PATCH 5/9] drm/tinydrm: Add helper functions

[Noralf Trønnes]

Add common functionality needed by many tinydrm drivers.

Signed-off-by: Noralf Trønnes 
---
 Documentation/gpu/drm-kms-helpers.rst  |   6 +
 drivers/gpu/drm/tinydrm/core/Makefile  |   2 +-
 drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c | 490 +
 include/drm/tinydrm/tinydrm-helpers.h  | 100 +
 4 files changed, 597 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
 create mode 100644 include/drm/tinydrm/tinydrm-helpers.h

diff --git a/Documentation/gpu/drm-kms-helpers.rst 
b/Documentation/gpu/drm-kms-helpers.rst
index a86bd7f..be07e76 100644
--- a/Documentation/gpu/drm-kms-helpers.rst
+++ b/Documentation/gpu/drm-kms-helpers.rst
@@ -287,3 +287,9 @@ tinydrm Helper Reference
 
 .. kernel-doc:: drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c
:export:
+
+.. kernel-doc:: include/drm/tinydrm/tinydrm-helpers.h
+   :internal:
+
+.. kernel-doc:: drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
+   :export:
diff --git a/drivers/gpu/drm/tinydrm/core/Makefile 
b/drivers/gpu/drm/tinydrm/core/Makefile
index 4f14a0f..fb221e6 100644
--- a/drivers/gpu/drm/tinydrm/core/Makefile
+++ b/drivers/gpu/drm/tinydrm/core/Makefile
@@ -1,3 +1,3 @@
-tinydrm-y := tinydrm-core.o tinydrm-pipe.o
+tinydrm-y := tinydrm-core.o tinydrm-pipe.o tinydrm-helpers.o
 
 obj-$(CONFIG_DRM_TINYDRM) += tinydrm.o
diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c 
b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
new file mode 100644
index 000..fc02e01
--- /dev/null
+++ b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
@@ -0,0 +1,490 @@
+/*
+ * Copyright (C) 2016 Noralf Trønnes
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+static unsigned int spi_max;
+module_param(spi_max, uint, 0400);
+MODULE_PARM_DESC(spi_max, "Set a lower SPI max transfer size");
+
+/**
+ * tinydrm_merge_clips - Merge clip rectangles
+ * @dst: Destination clip rectangle
+ * @src: Source clip rectangle(s)
+ * @num_clips: Number of @src clip rectangles
+ * @flags: Dirty fb ioctl flags
+ * @max_width: Maximum width of @dst
+ * @max_height: Maximum height of @dst
+ *
+ * This function merges @src clip rectangle(s) into @dst. If @src is NULL,
+ * @max_width and @min_width is used to set a full @dst clip rectangle.
+ *
+ * Returns:
+ * true if it's a full clip, false otherwise
+ */
+bool tinydrm_merge_clips(struct drm_clip_rect *dst,
+struct drm_clip_rect *src, unsigned int num_clips,
+unsigned int flags, u32 max_width, u32 max_height)
+{
+   unsigned int i;
+
+   if (!src || !num_clips) {
+   dst->x1 = 0;
+   dst->x2 = max_width;
+   dst->y1 = 0;
+   dst->y2 = max_height;
+   return true;
+   }
+
+   dst->x1 = ~0;
+   dst->y1 = ~0;
+   dst->x2 = 0;
+   dst->y2 = 0;
+
+   for (i = 0; i < num_clips; i++) {
+   if (flags & DRM_MODE_FB_DIRTY_ANNOTATE_COPY)
+   i++;
+   dst->x1 = min(dst->x1, src[i].x1);
+   dst->x2 = max(dst->x2, src[i].x2);
+   dst->y1 = min(dst->y1, src[i].y1);
+   dst->y2 = max(dst->y2, src[i].y2);
+   }
+
+   if (dst->x2 > max_width || dst->y2 > max_height ||
+   dst->x1 >= dst->x2 || dst->y1 >= dst->y2) {
+   DRM_DEBUG_KMS("Illegal clip: x1=%u, x2=%u, y1=%u, y2=%u\n",
+ dst->x1, dst->x2, dst->y1, dst->y2);
+   dst->x1 = 0;
+   dst->y1 = 0;
+   dst->x2 = max_width;
+   dst->y2 = max_height;
+   }
+
+   return (dst->x2 - dst->x1) == max_width &&
+  (dst->y2 - dst->y1) == max_height;
+}
+EXPORT_SYMBOL(tinydrm_merge_clips);
+
+/**
+ * tinydrm_memcpy - Copy clip buffer
+ * @dst: Destination buffer
+ * @vaddr: Source buffer
+ * @fb: DRM framebuffer
+ * @clip: Clip rectangle area to copy
+ */
+void tinydrm_memcpy(void *dst, void *vaddr, struct drm_framebuffer *fb,
+   struct drm_clip_rect *clip)
+{
+   unsigned int cpp = drm_format_plane_cpp(fb->format->format, 0);
+   unsigned int pitch = fb->pitches[0];
+   void *src = vaddr + (clip->y1 * pitch) + (clip->x1 * cpp);
+   size_t len = (clip->x2 - clip->x1) * cpp;
+   unsigned int y;
+
+   for (y = clip->y1; y < clip->y2; y++) {
+   memcpy(dst, src, len);
+   src += pitch;
+   dst += len;
+   }
+}
+EXPORT_SYMBOL(tinydrm_memcpy);
+
+/**
+ * tinydrm_swab16 - Swap bytes into clip buffer
+ * @dst: RGB565 destination buffer
+ * @vaddr: RGB565 source buffer
+ * @fb: DRM framebuffer
+ * @clip: Clip rectangle area