[PATCH v2 0/5] drm: Reuse temporary memory for format conversion

[PATCH v2 0/5] drm: Reuse temporary memory for format conversion

[Thomas Zimmermann]

DRM's format-conversion helpers require temporary memory. Pass the
buffer from the caller and keep it allocated over several calls. Allow
the caller to preallocate the buffer memory.

The motivation for this patchset is the recent work on a DRM panic
handler. The panic handler requires format conversion to display an
error to the screen. But allocating memory during kernel panics is
fragile. The changes in this patchset enable the DRM panic handler to
preallocate buffer storage before the panic occurs.

As an additonal benefit, drivers can now keep the temporary storage
across multiple display updates. Avoiding memory allocation reduces
the CPU overhead of the format helpers.

Patch 1 adds struct drm_xfrm_buf, a simple interface to pass around
the buffer storage. Patch 2 moves the memory management from the format
helpers into their callers. Drivers release the temporary storage at
the end of their display-update functions.

Patches 3 to 8 update three drivers to keep the allocated memory for
all of a device's lifetime. Managed cleanup releases the buffer as part
of releaseing the device. As additional benefit, buffer allocation now
happens in atomic_check helpers. The driver thus detects OOM errors
before the display update begins.

Tested with simpledrm.

v2:
	* reserve storage during probing in the drivers

Thomas Zimmermann (5):
  drm/format-helper: Add struct drm_xfrm_buf to cache format conversion
  drm/format-helper: Pass xfrm buffer to format-conversion helpers
  drm/simpledrm: Store xfrm buffer in device instance
  drm/ofdrm: Store xfrm buffer in device instance
  drm/ssd130x: Store xfrm buffer in device instance

 drivers/gpu/drm/drm_format_helper.c           | 204 +++++++++++++-----
 drivers/gpu/drm/drm_mipi_dbi.c                |   7 +-
 drivers/gpu/drm/gud/gud_pipe.c                |  21 +-
 drivers/gpu/drm/solomon/ssd130x.c             |  16 +-
 drivers/gpu/drm/solomon/ssd130x.h             |   3 +
 .../gpu/drm/tests/drm_format_helper_test.c    |  33 +--
 drivers/gpu/drm/tiny/cirrus.c                 |   5 +-
 drivers/gpu/drm/tiny/ofdrm.c                  |  11 +-
 drivers/gpu/drm/tiny/repaper.c                |   5 +-
 drivers/gpu/drm/tiny/simpledrm.c              |  11 +-
 drivers/gpu/drm/tiny/st7586.c                 |   5 +-
 include/drm/drm_format_helper.h               |  74 +++++--
 12 files changed, 300 insertions(+), 95 deletions(-)

-- 
2.42.0

[PATCH v2 1/5] drm/format-helper: Add struct drm_xfrm_buf to cache format conversion

[Thomas Zimmermann]

Hold temporary memory for format conversion in an instance of struct
drm_xfrm_buf. Update internal helpers of DRM's format-conversion code
accordingly. Drivers will later be able to keep this cache across
display updates.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_format_helper.c | 111 +++++++++++++++++++++++++---
 include/drm/drm_format_helper.h     |  46 ++++++++++++
 2 files changed, 146 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
index f93a4efcee909..029ca7893260a 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -17,9 +17,93 @@
 #include <drm/drm_format_helper.h>
 #include <drm/drm_framebuffer.h>
 #include <drm/drm_fourcc.h>
+#include <drm/drm_managed.h>
 #include <drm/drm_print.h>
 #include <drm/drm_rect.h>
 
+static void drm_xfrm_buf_init_release(struct drm_device *dev, void *res)
+{
+	struct drm_xfrm_buf *buf = res;
+
+	drm_xfrm_buf_release(buf);
+}
+
+/**
+ * drmm_xfrm_buf_init - Initialize xfrm buffer with managed cleanup
+ * @dev: The DRM device
+ * @buf: The xfrm buffer to initialize
+ *
+ * Clears all fields in struct drm_xfrm_buf and installs a DRM release
+ * action for the buffer. The buffer will be empty with no preallocated
+ * resources.
+ *
+ * Returns:
+ * 0 on success, or a negative errno code otherwise.
+ */
+int drmm_xfrm_buf_init(struct drm_device *dev, struct drm_xfrm_buf *buf)
+{
+	buf->mem = NULL;
+	buf->size = 0;
+	buf->preallocated = false;
+
+	return drmm_add_action_or_reset(dev, drm_xfrm_buf_init_release, buf);
+}
+EXPORT_SYMBOL(drmm_xfrm_buf_init);
+
+/**
+ * drm_xfrm_buf_reserve - Allocates storage in an xfrm buffer
+ * @buf: The xfrm buffer
+ * @new_size: The minimum allocation size
+ * @flags: Flags for kmalloc()
+ *
+ * Allocates at least @new_size bytes and returns a pointer to the memory
+ * range. After calling this function, previously returned memory blocks
+ * are invalid. It's best to collect all memory requirements of a format
+ * conversion and call this function once to allocate the range.
+ *
+ * Returns:
+ * A pointer to the allocated memory range, or a pointer-encoded errno code otherwise.
+ */
+void *drm_xfrm_buf_reserve(struct drm_xfrm_buf *buf, size_t new_size, gfp_t flags)
+{
+	void *mem;
+
+	if (new_size <= buf->size)
+		goto out;
+	else if (buf->preallocated)
+		return NULL;
+
+	mem = krealloc(buf->mem, new_size, flags);
+	if (!mem)
+		return NULL;
+
+	buf->mem = mem;
+	buf->size = new_size;
+
+out:
+	return buf->mem;
+}
+EXPORT_SYMBOL(drm_xfrm_buf_reserve);
+
+/**
+ * drm_xfrm_buf_release - Releases an xfrm buffer's storage
+ * @buf: The xfrm buffer
+ *
+ * Releases the memory range references by the xfrm buffer. After
+ * this call, all pointers to the memory are invalid. Prefer
+ * drmm_xfrm_buffer_init() for cleaning up and unloading a driver.
+ */
+void drm_xfrm_buf_release(struct drm_xfrm_buf *buf)
+{
+	if (buf->preallocated)
+		return;
+
+	kfree(buf->mem);
+	buf->mem = NULL;
+	buf->size = 0;
+}
+EXPORT_SYMBOL(drm_xfrm_buf_release);
+
 static unsigned int clip_offset(const struct drm_rect *clip, unsigned int pitch, unsigned int cpp)
 {
 	return clip->y1 * pitch + clip->x1 * cpp;
@@ -45,6 +129,7 @@ EXPORT_SYMBOL(drm_fb_clip_offset);
 static int __drm_fb_xfrm(void *dst, unsigned long dst_pitch, unsigned long dst_pixsize,
 			 const void *vaddr, const struct drm_framebuffer *fb,
 			 const struct drm_rect *clip, bool vaddr_cached_hint,
+			 struct drm_xfrm_buf *xfrm,
 			 void (*xfrm_line)(void *dbuf, const void *sbuf, unsigned int npixels))
 {
 	unsigned long linepixels = drm_rect_width(clip);
@@ -60,7 +145,7 @@ static int __drm_fb_xfrm(void *dst, unsigned long dst_pitch, unsigned long dst_p
 	 * one line at a time.
 	 */
 	if (!vaddr_cached_hint) {
-		stmp = kmalloc(sbuf_len, GFP_KERNEL);
+		stmp = drm_xfrm_buf_reserve(xfrm, sbuf_len, GFP_KERNEL);
 		if (!stmp)
 			return -ENOMEM;
 	}
@@ -79,8 +164,6 @@ static int __drm_fb_xfrm(void *dst, unsigned long dst_pitch, unsigned long dst_p
 		dst += dst_pitch;
 	}
 
-	kfree(stmp);
-
 	return 0;
 }
 
@@ -88,6 +171,7 @@ static int __drm_fb_xfrm(void *dst, unsigned long dst_pitch, unsigned long dst_p
 static int __drm_fb_xfrm_toio(void __iomem *dst, unsigned long dst_pitch, unsigned long dst_pixsize,
 			      const void *vaddr, const struct drm_framebuffer *fb,
 			      const struct drm_rect *clip, bool vaddr_cached_hint,
+			      struct drm_xfrm_buf *xfrm,
 			      void (*xfrm_line)(void *dbuf, const void *sbuf, unsigned int npixels))
 {
 	unsigned long linepixels = drm_rect_width(clip);
@@ -101,9 +185,9 @@ static int __drm_fb_xfrm_toio(void __iomem *dst, unsigned long dst_pitch, unsign
 	void *dbuf;
 
 	if (vaddr_cached_hint) {
-		dbuf = kmalloc(dbuf_len, GFP_KERNEL);
+		dbuf = drm_xfrm_buf_reserve(xfrm, dbuf_len, GFP_KERNEL);
 	} else {
-		dbuf = kmalloc(stmp_off + sbuf_len, GFP_KERNEL);
+		dbuf = drm_xfrm_buf_reserve(xfrm, stmp_off + sbuf_len, GFP_KERNEL);
 		stmp = dbuf + stmp_off;
 	}
 	if (!dbuf)
@@ -124,8 +208,6 @@ static int __drm_fb_xfrm_toio(void __iomem *dst, unsigned long dst_pitch, unsign
 		dst += dst_pitch;
 	}
 
-	kfree(dbuf);
-
 	return 0;
 }
 
@@ -139,17 +221,24 @@ static int drm_fb_xfrm(struct iosys_map *dst,
 	static const unsigned int default_dst_pitch[DRM_FORMAT_MAX_PLANES] = {
 		0, 0, 0, 0
 	};
+	struct drm_xfrm_buf tmp = DRM_XFRM_BUF_INIT;
+	int ret;
 
 	if (!dst_pitch)
 		dst_pitch = default_dst_pitch;
 
 	/* TODO: handle src in I/O memory here */
 	if (dst[0].is_iomem)
-		return __drm_fb_xfrm_toio(dst[0].vaddr_iomem, dst_pitch[0], dst_pixsize[0],
-					  src[0].vaddr, fb, clip, vaddr_cached_hint, xfrm_line);
+		ret = __drm_fb_xfrm_toio(dst[0].vaddr_iomem, dst_pitch[0], dst_pixsize[0],
+					 src[0].vaddr, fb, clip, vaddr_cached_hint, &tmp,
+					 xfrm_line);
 	else
-		return __drm_fb_xfrm(dst[0].vaddr, dst_pitch[0], dst_pixsize[0],
-				     src[0].vaddr, fb, clip, vaddr_cached_hint, xfrm_line);
+		ret = __drm_fb_xfrm(dst[0].vaddr, dst_pitch[0], dst_pixsize[0],
+				    src[0].vaddr, fb, clip, vaddr_cached_hint, &tmp,
+				    xfrm_line);
+	drm_xfrm_buf_release(&tmp);
+
+	return ret;
 }
 
 /**
diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
index 291deb09475bb..245a5edc4735a 100644
--- a/include/drm/drm_format_helper.h
+++ b/include/drm/drm_format_helper.h
@@ -15,6 +15,52 @@ struct drm_rect;
 
 struct iosys_map;
 
+/**
+ * struct drm_xfrm_buf - Stores transformation and conversion state
+ *
+ * DRM helpers for format conversion store temporary state in
+ * struct drm_xfrm_buf. The buffer's resources can be reused
+ * among multiple conversion operations.
+ *
+ * All fields are considered private.
+ */
+struct drm_xfrm_buf {
+	void *mem;
+	size_t size;
+	bool preallocated;
+};
+
+/**
+ * DRM_XFRM_BUF_INIT - Initializer for struct drm_xfrm_buf
+ *
+ * Initializes an instance of struct drm_xfrm_buf to default
+ * values.
+ */
+#define DRM_XFRM_BUF_INIT { \
+		.mem = NULL, \
+		.size = 0, \
+		.preallocated = false, \
+	}
+
+/**
+ * DRM_XFRM_BUF_INIT_PREALLOCATED - Initializer for struct drm_xfrm_buf
+ * @_mem: The preallocated memory area
+ * @_size: The number of bytes in _mem
+ *
+ * Initializes an instance of struct drm_xfrm_buf to preallocated
+ * storage. The caller is responsible for a releases the provided
+ * memory range.
+ */
+#define DRM_XFRM_BUF_INIT_PREALLOCATED(_mem, _size) { \
+		.mem = (_mem), \
+		.size = (_size), \
+		.preallocated = true, \
+	}
+
+int drmm_xfrm_buf_init(struct drm_device *dev, struct drm_xfrm_buf *buf);
+void *drm_xfrm_buf_reserve(struct drm_xfrm_buf *buf, size_t new_size, gfp_t flags);
+void drm_xfrm_buf_release(struct drm_xfrm_buf *buf);
+
 unsigned int drm_fb_clip_offset(unsigned int pitch, const struct drm_format_info *format,
 				const struct drm_rect *clip);
 
-- 
2.42.0

[PATCH v2 2/5] drm/format-helper: Pass xfrm buffer to format-conversion helpers

[Thomas Zimmermann]

Pass an instance of struct drm_xfrm_buf to DRM's format conversion
helpers. Update all callers. Drivers will later be able to keep this
cache across display updates.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Noralf Trønnes <noralf@tronnes.org>
Cc: Javier Martinez Canillas <javierm@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: David Lechner <david@lechnology.com>
---
 drivers/gpu/drm/drm_format_helper.c           | 119 ++++++++++--------
 drivers/gpu/drm/drm_mipi_dbi.c                |   7 +-
 drivers/gpu/drm/gud/gud_pipe.c                |  21 ++--
 drivers/gpu/drm/solomon/ssd130x.c             |   5 +-
 .../gpu/drm/tests/drm_format_helper_test.c    |  33 ++---
 drivers/gpu/drm/tiny/cirrus.c                 |   5 +-
 drivers/gpu/drm/tiny/ofdrm.c                  |   4 +-
 drivers/gpu/drm/tiny/repaper.c                |   5 +-
 drivers/gpu/drm/tiny/simpledrm.c              |   4 +-
 drivers/gpu/drm/tiny/st7586.c                 |   5 +-
 include/drm/drm_format_helper.h               |  28 +++--
 11 files changed, 139 insertions(+), 97 deletions(-)

diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
index 029ca7893260a..70a717ee79d03 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -216,29 +216,25 @@ static int drm_fb_xfrm(struct iosys_map *dst,
 		       const unsigned int *dst_pitch, const u8 *dst_pixsize,
 		       const struct iosys_map *src, const struct drm_framebuffer *fb,
 		       const struct drm_rect *clip, bool vaddr_cached_hint,
+		       struct drm_xfrm_buf *xfrm,
 		       void (*xfrm_line)(void *dbuf, const void *sbuf, unsigned int npixels))
 {
 	static const unsigned int default_dst_pitch[DRM_FORMAT_MAX_PLANES] = {
 		0, 0, 0, 0
 	};
-	struct drm_xfrm_buf tmp = DRM_XFRM_BUF_INIT;
-	int ret;
 
 	if (!dst_pitch)
 		dst_pitch = default_dst_pitch;
 
 	/* TODO: handle src in I/O memory here */
 	if (dst[0].is_iomem)
-		ret = __drm_fb_xfrm_toio(dst[0].vaddr_iomem, dst_pitch[0], dst_pixsize[0],
-					 src[0].vaddr, fb, clip, vaddr_cached_hint, &tmp,
-					 xfrm_line);
+		return __drm_fb_xfrm_toio(dst[0].vaddr_iomem, dst_pitch[0], dst_pixsize[0],
+					  src[0].vaddr, fb, clip, vaddr_cached_hint, xfrm,
+					  xfrm_line);
 	else
-		ret = __drm_fb_xfrm(dst[0].vaddr, dst_pitch[0], dst_pixsize[0],
-				    src[0].vaddr, fb, clip, vaddr_cached_hint, &tmp,
-				    xfrm_line);
-	drm_xfrm_buf_release(&tmp);
-
-	return ret;
+		return __drm_fb_xfrm(dst[0].vaddr, dst_pitch[0], dst_pixsize[0],
+				     src[0].vaddr, fb, clip, vaddr_cached_hint, xfrm,
+				     xfrm_line);
 }
 
 /**
@@ -324,6 +320,7 @@ static void drm_fb_swab32_line(void *dbuf, const void *sbuf, unsigned int pixels
  * @fb: DRM framebuffer
  * @clip: Clip rectangle area to copy
  * @cached: Source buffer is mapped cached (eg. not write-combined)
+ * @xfrm: Transform and conversion state
  *
  * This function copies parts of a framebuffer to display memory and swaps per-pixel
  * bytes during the process. Destination and framebuffer formats must match. The
@@ -338,7 +335,8 @@ static void drm_fb_swab32_line(void *dbuf, const void *sbuf, unsigned int pixels
  */
 void drm_fb_swab(struct iosys_map *dst, const unsigned int *dst_pitch,
 		 const struct iosys_map *src, const struct drm_framebuffer *fb,
-		 const struct drm_rect *clip, bool cached)
+		 const struct drm_rect *clip, bool cached,
+		 struct drm_xfrm_buf *xfrm)
 {
 	const struct drm_format_info *format = fb->format;
 	u8 cpp = DIV_ROUND_UP(drm_format_info_bpp(format, 0), 8);
@@ -357,7 +355,7 @@ void drm_fb_swab(struct iosys_map *dst, const unsigned int *dst_pitch,
 		return;
 	}
 
-	drm_fb_xfrm(dst, dst_pitch, &cpp, src, fb, clip, cached, swab_line);
+	drm_fb_xfrm(dst, dst_pitch, &cpp, src, fb, clip, cached, xfrm, swab_line);
 }
 EXPORT_SYMBOL(drm_fb_swab);
 
@@ -384,6 +382,7 @@ static void drm_fb_xrgb8888_to_rgb332_line(void *dbuf, const void *sbuf, unsigne
  * @src: Array of XRGB8888 source buffers
  * @fb: DRM framebuffer
  * @clip: Clip rectangle area to copy
+ * @xfrm: Transform and conversion state
  *
  * This function copies parts of a framebuffer to display memory and converts the
  * color format during the process. Destination and framebuffer formats must match. The
@@ -398,13 +397,13 @@ static void drm_fb_xrgb8888_to_rgb332_line(void *dbuf, const void *sbuf, unsigne
  */
 void drm_fb_xrgb8888_to_rgb332(struct iosys_map *dst, const unsigned int *dst_pitch,
 			       const struct iosys_map *src, const struct drm_framebuffer *fb,
-			       const struct drm_rect *clip)
+			       const struct drm_rect *clip, struct drm_xfrm_buf *xfrm)
 {
 	static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
 		1,
 	};
 
-	drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false,
+	drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false, xfrm,
 		    drm_fb_xrgb8888_to_rgb332_line);
 }
 EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb332);
@@ -453,6 +452,7 @@ static void drm_fb_xrgb8888_to_rgb565_swab_line(void *dbuf, const void *sbuf,
  * @src: Array of XRGB8888 source buffer
  * @fb: DRM framebuffer
  * @clip: Clip rectangle area to copy
+ * @xfrm: Transform and conversion state
  * @swab: Swap bytes
  *
  * This function copies parts of a framebuffer to display memory and converts the
@@ -468,7 +468,8 @@ static void drm_fb_xrgb8888_to_rgb565_swab_line(void *dbuf, const void *sbuf,
  */
 void drm_fb_xrgb8888_to_rgb565(struct iosys_map *dst, const unsigned int *dst_pitch,
 			       const struct iosys_map *src, const struct drm_framebuffer *fb,
-			       const struct drm_rect *clip, bool swab)
+			       const struct drm_rect *clip, struct drm_xfrm_buf *xfrm,
+			       bool swab)
 {
 	static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
 		2,
@@ -481,7 +482,7 @@ void drm_fb_xrgb8888_to_rgb565(struct iosys_map *dst, const unsigned int *dst_pi
 	else
 		xfrm_line = drm_fb_xrgb8888_to_rgb565_line;
 
-	drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false, xfrm_line);
+	drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false, xfrm, xfrm_line);
 }
 EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb565);
 
@@ -510,6 +511,7 @@ static void drm_fb_xrgb8888_to_xrgb1555_line(void *dbuf, const void *sbuf, unsig
  * @src: Array of XRGB8888 source buffer
  * @fb: DRM framebuffer
  * @clip: Clip rectangle area to copy
+ * @xfrm: Transform and conversion state
  *
  * This function copies parts of a framebuffer to display memory and converts
  * the color format during the process. The parameters @dst, @dst_pitch and
@@ -525,13 +527,13 @@ static void drm_fb_xrgb8888_to_xrgb1555_line(void *dbuf, const void *sbuf, unsig
  */
 void drm_fb_xrgb8888_to_xrgb1555(struct iosys_map *dst, const unsigned int *dst_pitch,
 				 const struct iosys_map *src, const struct drm_framebuffer *fb,
-				 const struct drm_rect *clip)
+				 const struct drm_rect *clip, struct drm_xfrm_buf *xfrm)
 {
 	static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
 		2,
 	};
 
-	drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false,
+	drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false, xfrm,
 		    drm_fb_xrgb8888_to_xrgb1555_line);
 }
 EXPORT_SYMBOL(drm_fb_xrgb8888_to_xrgb1555);
@@ -562,6 +564,7 @@ static void drm_fb_xrgb8888_to_argb1555_line(void *dbuf, const void *sbuf, unsig
  * @src: Array of XRGB8888 source buffer
  * @fb: DRM framebuffer
  * @clip: Clip rectangle area to copy
+ * @xfrm: Transform and conversion state
  *
  * This function copies parts of a framebuffer to display memory and converts
  * the color format during the process. The parameters @dst, @dst_pitch and
@@ -577,13 +580,13 @@ static void drm_fb_xrgb8888_to_argb1555_line(void *dbuf, const void *sbuf, unsig
  */
 void drm_fb_xrgb8888_to_argb1555(struct iosys_map *dst, const unsigned int *dst_pitch,
 				 const struct iosys_map *src, const struct drm_framebuffer *fb,
-				 const struct drm_rect *clip)
+				 const struct drm_rect *clip, struct drm_xfrm_buf *xfrm)
 {
 	static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
 		2,
 	};
 
-	drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false,
+	drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false, xfrm,
 		    drm_fb_xrgb8888_to_argb1555_line);
 }
 EXPORT_SYMBOL(drm_fb_xrgb8888_to_argb1555);
@@ -614,6 +617,7 @@ static void drm_fb_xrgb8888_to_rgba5551_line(void *dbuf, const void *sbuf, unsig
  * @src: Array of XRGB8888 source buffer
  * @fb: DRM framebuffer
  * @clip: Clip rectangle area to copy
+ * @xfrm: Transform and conversion state
  *
  * This function copies parts of a framebuffer to display memory and converts
  * the color format during the process. The parameters @dst, @dst_pitch and
@@ -629,13 +633,13 @@ static void drm_fb_xrgb8888_to_rgba5551_line(void *dbuf, const void *sbuf, unsig
  */
 void drm_fb_xrgb8888_to_rgba5551(struct iosys_map *dst, const unsigned int *dst_pitch,
 				 const struct iosys_map *src, const struct drm_framebuffer *fb,
-				 const struct drm_rect *clip)
+				 const struct drm_rect *clip, struct drm_xfrm_buf *xfrm)
 {
 	static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
 		2,
 	};
 
-	drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false,
+	drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false, xfrm,
 		    drm_fb_xrgb8888_to_rgba5551_line);
 }
 EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgba5551);
@@ -664,6 +668,7 @@ static void drm_fb_xrgb8888_to_rgb888_line(void *dbuf, const void *sbuf, unsigne
  * @src: Array of XRGB8888 source buffers
  * @fb: DRM framebuffer
  * @clip: Clip rectangle area to copy
+ * @xfrm: Transform and conversion state
  *
  * This function copies parts of a framebuffer to display memory and converts the
  * color format during the process. Destination and framebuffer formats must match. The
@@ -679,13 +684,13 @@ static void drm_fb_xrgb8888_to_rgb888_line(void *dbuf, const void *sbuf, unsigne
  */
 void drm_fb_xrgb8888_to_rgb888(struct iosys_map *dst, const unsigned int *dst_pitch,
 			       const struct iosys_map *src, const struct drm_framebuffer *fb,
-			       const struct drm_rect *clip)
+			       const struct drm_rect *clip, struct drm_xfrm_buf *xfrm)
 {
 	static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
 		3,
 	};
 
-	drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false,
+	drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false, xfrm,
 		    drm_fb_xrgb8888_to_rgb888_line);
 }
 EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb888);
@@ -712,6 +717,7 @@ static void drm_fb_xrgb8888_to_argb8888_line(void *dbuf, const void *sbuf, unsig
  * @src: Array of XRGB8888 source buffer
  * @fb: DRM framebuffer
  * @clip: Clip rectangle area to copy
+ * @xfrm: Transform and conversion state
  *
  * This function copies parts of a framebuffer to display memory and converts the
  * color format during the process. The parameters @dst, @dst_pitch and @src refer
@@ -727,13 +733,13 @@ static void drm_fb_xrgb8888_to_argb8888_line(void *dbuf, const void *sbuf, unsig
  */
 void drm_fb_xrgb8888_to_argb8888(struct iosys_map *dst, const unsigned int *dst_pitch,
 				 const struct iosys_map *src, const struct drm_framebuffer *fb,
-				 const struct drm_rect *clip)
+				 const struct drm_rect *clip, struct drm_xfrm_buf *xfrm)
 {
 	static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
 		4,
 	};
 
-	drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false,
+	drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false, xfrm,
 		    drm_fb_xrgb8888_to_argb8888_line);
 }
 EXPORT_SYMBOL(drm_fb_xrgb8888_to_argb8888);
@@ -758,13 +764,13 @@ static void drm_fb_xrgb8888_to_abgr8888_line(void *dbuf, const void *sbuf, unsig
 static void drm_fb_xrgb8888_to_abgr8888(struct iosys_map *dst, const unsigned int *dst_pitch,
 					const struct iosys_map *src,
 					const struct drm_framebuffer *fb,
-					const struct drm_rect *clip)
+					const struct drm_rect *clip, struct drm_xfrm_buf *xfrm)
 {
 	static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
 		4,
 	};
 
-	drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false,
+	drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false, xfrm,
 		    drm_fb_xrgb8888_to_abgr8888_line);
 }
 
@@ -788,13 +794,13 @@ static void drm_fb_xrgb8888_to_xbgr8888_line(void *dbuf, const void *sbuf, unsig
 static void drm_fb_xrgb8888_to_xbgr8888(struct iosys_map *dst, const unsigned int *dst_pitch,
 					const struct iosys_map *src,
 					const struct drm_framebuffer *fb,
-					const struct drm_rect *clip)
+					const struct drm_rect *clip, struct drm_xfrm_buf *xfrm)
 {
 	static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
 		4,
 	};
 
-	drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false,
+	drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false, xfrm,
 		    drm_fb_xrgb8888_to_xbgr8888_line);
 }
 
@@ -824,6 +830,7 @@ static void drm_fb_xrgb8888_to_xrgb2101010_line(void *dbuf, const void *sbuf, un
  * @src: Array of XRGB8888 source buffers
  * @fb: DRM framebuffer
  * @clip: Clip rectangle area to copy
+ * @xfrm: Transform and conversion state
  *
  * This function copies parts of a framebuffer to display memory and converts the
  * color format during the process. Destination and framebuffer formats must match. The
@@ -839,13 +846,13 @@ static void drm_fb_xrgb8888_to_xrgb2101010_line(void *dbuf, const void *sbuf, un
  */
 void drm_fb_xrgb8888_to_xrgb2101010(struct iosys_map *dst, const unsigned int *dst_pitch,
 				    const struct iosys_map *src, const struct drm_framebuffer *fb,
-				    const struct drm_rect *clip)
+				    const struct drm_rect *clip, struct drm_xfrm_buf *xfrm)
 {
 	static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
 		4,
 	};
 
-	drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false,
+	drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false, xfrm,
 		    drm_fb_xrgb8888_to_xrgb2101010_line);
 }
 EXPORT_SYMBOL(drm_fb_xrgb8888_to_xrgb2101010);
@@ -877,6 +884,7 @@ static void drm_fb_xrgb8888_to_argb2101010_line(void *dbuf, const void *sbuf, un
  * @src: Array of XRGB8888 source buffers
  * @fb: DRM framebuffer
  * @clip: Clip rectangle area to copy
+ * @xfrm: Transform and conversion state
  *
  * This function copies parts of a framebuffer to display memory and converts
  * the color format during the process. The parameters @dst, @dst_pitch and
@@ -892,13 +900,13 @@ static void drm_fb_xrgb8888_to_argb2101010_line(void *dbuf, const void *sbuf, un
  */
 void drm_fb_xrgb8888_to_argb2101010(struct iosys_map *dst, const unsigned int *dst_pitch,
 				    const struct iosys_map *src, const struct drm_framebuffer *fb,
-				    const struct drm_rect *clip)
+				    const struct drm_rect *clip, struct drm_xfrm_buf *xfrm)
 {
 	static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
 		4,
 	};
 
-	drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false,
+	drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false, xfrm,
 		    drm_fb_xrgb8888_to_argb2101010_line);
 }
 EXPORT_SYMBOL(drm_fb_xrgb8888_to_argb2101010);
@@ -928,6 +936,7 @@ static void drm_fb_xrgb8888_to_gray8_line(void *dbuf, const void *sbuf, unsigned
  * @src: Array of XRGB8888 source buffers
  * @fb: DRM framebuffer
  * @clip: Clip rectangle area to copy
+ * @xfrm: Transform and conversion state
  *
  * This function copies parts of a framebuffer to display memory and converts the
  * color format during the process. Destination and framebuffer formats must match. The
@@ -947,13 +956,13 @@ static void drm_fb_xrgb8888_to_gray8_line(void *dbuf, const void *sbuf, unsigned
  */
 void drm_fb_xrgb8888_to_gray8(struct iosys_map *dst, const unsigned int *dst_pitch,
 			      const struct iosys_map *src, const struct drm_framebuffer *fb,
-			      const struct drm_rect *clip)
+			      const struct drm_rect *clip, struct drm_xfrm_buf *xfrm)
 {
 	static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
 		1,
 	};
 
-	drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false,
+	drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false, xfrm,
 		    drm_fb_xrgb8888_to_gray8_line);
 }
 EXPORT_SYMBOL(drm_fb_xrgb8888_to_gray8);
@@ -967,6 +976,7 @@ EXPORT_SYMBOL(drm_fb_xrgb8888_to_gray8);
  * @src:	The framebuffer memory to copy from
  * @fb:		The framebuffer to copy from
  * @clip:	Clip rectangle area to copy
+ * @xfrm: Transform and conversion state
  *
  * This function copies parts of a framebuffer to display memory. If the
  * formats of the display and the framebuffer mismatch, the blit function
@@ -985,7 +995,7 @@ EXPORT_SYMBOL(drm_fb_xrgb8888_to_gray8);
  */
 int drm_fb_blit(struct iosys_map *dst, const unsigned int *dst_pitch, uint32_t dst_format,
 		const struct iosys_map *src, const struct drm_framebuffer *fb,
-		const struct drm_rect *clip)
+		const struct drm_rect *clip, struct drm_xfrm_buf *xfrm)
 {
 	uint32_t fb_format = fb->format->format;
 
@@ -993,44 +1003,44 @@ int drm_fb_blit(struct iosys_map *dst, const unsigned int *dst_pitch, uint32_t d
 		drm_fb_memcpy(dst, dst_pitch, src, fb, clip);
 		return 0;
 	} else if (fb_format == (dst_format | DRM_FORMAT_BIG_ENDIAN)) {
-		drm_fb_swab(dst, dst_pitch, src, fb, clip, false);
+		drm_fb_swab(dst, dst_pitch, src, fb, clip, false, xfrm);
 		return 0;
 	} else if (fb_format == (dst_format & ~DRM_FORMAT_BIG_ENDIAN)) {
-		drm_fb_swab(dst, dst_pitch, src, fb, clip, false);
+		drm_fb_swab(dst, dst_pitch, src, fb, clip, false, xfrm);
 		return 0;
 	} else if (fb_format == DRM_FORMAT_XRGB8888) {
 		if (dst_format == DRM_FORMAT_RGB565) {
-			drm_fb_xrgb8888_to_rgb565(dst, dst_pitch, src, fb, clip, false);
+			drm_fb_xrgb8888_to_rgb565(dst, dst_pitch, src, fb, clip, xfrm, false);
 			return 0;
 		} else if (dst_format == DRM_FORMAT_XRGB1555) {
-			drm_fb_xrgb8888_to_xrgb1555(dst, dst_pitch, src, fb, clip);
+			drm_fb_xrgb8888_to_xrgb1555(dst, dst_pitch, src, fb, clip, xfrm);
 			return 0;
 		} else if (dst_format == DRM_FORMAT_ARGB1555) {
-			drm_fb_xrgb8888_to_argb1555(dst, dst_pitch, src, fb, clip);
+			drm_fb_xrgb8888_to_argb1555(dst, dst_pitch, src, fb, clip, xfrm);
 			return 0;
 		} else if (dst_format == DRM_FORMAT_RGBA5551) {
-			drm_fb_xrgb8888_to_rgba5551(dst, dst_pitch, src, fb, clip);
+			drm_fb_xrgb8888_to_rgba5551(dst, dst_pitch, src, fb, clip, xfrm);
 			return 0;
 		} else if (dst_format == DRM_FORMAT_RGB888) {
-			drm_fb_xrgb8888_to_rgb888(dst, dst_pitch, src, fb, clip);
+			drm_fb_xrgb8888_to_rgb888(dst, dst_pitch, src, fb, clip, xfrm);
 			return 0;
 		} else if (dst_format == DRM_FORMAT_ARGB8888) {
-			drm_fb_xrgb8888_to_argb8888(dst, dst_pitch, src, fb, clip);
+			drm_fb_xrgb8888_to_argb8888(dst, dst_pitch, src, fb, clip, xfrm);
 			return 0;
 		} else if (dst_format == DRM_FORMAT_XBGR8888) {
-			drm_fb_xrgb8888_to_xbgr8888(dst, dst_pitch, src, fb, clip);
+			drm_fb_xrgb8888_to_xbgr8888(dst, dst_pitch, src, fb, clip, xfrm);
 			return 0;
 		} else if (dst_format == DRM_FORMAT_ABGR8888) {
-			drm_fb_xrgb8888_to_abgr8888(dst, dst_pitch, src, fb, clip);
+			drm_fb_xrgb8888_to_abgr8888(dst, dst_pitch, src, fb, clip, xfrm);
 			return 0;
 		} else if (dst_format == DRM_FORMAT_XRGB2101010) {
-			drm_fb_xrgb8888_to_xrgb2101010(dst, dst_pitch, src, fb, clip);
+			drm_fb_xrgb8888_to_xrgb2101010(dst, dst_pitch, src, fb, clip, xfrm);
 			return 0;
 		} else if (dst_format == DRM_FORMAT_ARGB2101010) {
-			drm_fb_xrgb8888_to_argb2101010(dst, dst_pitch, src, fb, clip);
+			drm_fb_xrgb8888_to_argb2101010(dst, dst_pitch, src, fb, clip, xfrm);
 			return 0;
 		} else if (dst_format == DRM_FORMAT_BGRX8888) {
-			drm_fb_swab(dst, dst_pitch, src, fb, clip, false);
+			drm_fb_swab(dst, dst_pitch, src, fb, clip, false, xfrm);
 			return 0;
 		}
 	}
@@ -1067,6 +1077,7 @@ static void drm_fb_gray8_to_mono_line(void *dbuf, const void *sbuf, unsigned int
  * @src: Array of XRGB8888 source buffers
  * @fb: DRM framebuffer
  * @clip: Clip rectangle area to copy
+ * @xfrm: Transform and conversion state
  *
  * This function copies parts of a framebuffer to display memory and converts the
  * color format during the process. Destination and framebuffer formats must match. The
@@ -1091,7 +1102,7 @@ static void drm_fb_gray8_to_mono_line(void *dbuf, const void *sbuf, unsigned int
  */
 void drm_fb_xrgb8888_to_mono(struct iosys_map *dst, const unsigned int *dst_pitch,
 			     const struct iosys_map *src, const struct drm_framebuffer *fb,
-			     const struct drm_rect *clip)
+			     const struct drm_rect *clip, struct drm_xfrm_buf *xfrm)
 {
 	static const unsigned int default_dst_pitch[DRM_FORMAT_MAX_PLANES] = {
 		0, 0, 0, 0
@@ -1131,7 +1142,7 @@ void drm_fb_xrgb8888_to_mono(struct iosys_map *dst, const unsigned int *dst_pitc
 	 * Allocate a buffer to be used for both copying from the cma
 	 * memory and to store the intermediate grayscale line pixels.
 	 */
-	src32 = kmalloc(len_src32 + linepixels, GFP_KERNEL);
+	src32 = drm_xfrm_buf_reserve(xfrm, len_src32 + linepixels, GFP_KERNEL);
 	if (!src32)
 		return;
 
@@ -1145,8 +1156,6 @@ void drm_fb_xrgb8888_to_mono(struct iosys_map *dst, const unsigned int *dst_pitc
 		vaddr += fb->pitches[0];
 		mono += dst_pitch_0;
 	}
-
-	kfree(src32);
 }
 EXPORT_SYMBOL(drm_fb_xrgb8888_to_mono);
 
diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
index e90f0bf895b33..b5f9ab9de1d08 100644
--- a/drivers/gpu/drm/drm_mipi_dbi.c
+++ b/drivers/gpu/drm/drm_mipi_dbi.c
@@ -206,6 +206,7 @@ int mipi_dbi_buf_copy(void *dst, struct iosys_map *src, struct drm_framebuffer *
 {
 	struct drm_gem_object *gem = drm_gem_fb_get_obj(fb, 0);
 	struct iosys_map dst_map = IOSYS_MAP_INIT_VADDR(dst);
+	struct drm_xfrm_buf xfrm = DRM_XFRM_BUF_INIT;
 	int ret;
 
 	ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
@@ -215,12 +216,12 @@ int mipi_dbi_buf_copy(void *dst, struct iosys_map *src, struct drm_framebuffer *
 	switch (fb->format->format) {
 	case DRM_FORMAT_RGB565:
 		if (swap)
-			drm_fb_swab(&dst_map, NULL, src, fb, clip, !gem->import_attach);
+			drm_fb_swab(&dst_map, NULL, src, fb, clip, !gem->import_attach, &xfrm);
 		else
 			drm_fb_memcpy(&dst_map, NULL, src, fb, clip);
 		break;
 	case DRM_FORMAT_XRGB8888:
-		drm_fb_xrgb8888_to_rgb565(&dst_map, NULL, src, fb, clip, swap);
+		drm_fb_xrgb8888_to_rgb565(&dst_map, NULL, src, fb, clip, &xfrm, swap);
 		break;
 	default:
 		drm_err_once(fb->dev, "Format is not supported: %p4cc\n",
@@ -230,6 +231,8 @@ int mipi_dbi_buf_copy(void *dst, struct iosys_map *src, struct drm_framebuffer *
 
 	drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
 
+	drm_xfrm_buf_release(&xfrm);
+
 	return ret;
 }
 EXPORT_SYMBOL(mipi_dbi_buf_copy);
diff --git a/drivers/gpu/drm/gud/gud_pipe.c b/drivers/gpu/drm/gud/gud_pipe.c
index d2f199ea3c111..38d4a65c0204c 100644
--- a/drivers/gpu/drm/gud/gud_pipe.c
+++ b/drivers/gpu/drm/gud/gud_pipe.c
@@ -57,6 +57,7 @@ static size_t gud_xrgb8888_to_r124(u8 *dst, const struct drm_format_info *format
 	unsigned int bits_per_pixel = 8 / block_width;
 	unsigned int x, y, width, height;
 	u8 pix, *pix8, *block = dst; /* Assign to silence compiler warning */
+	struct drm_xfrm_buf xfrm = DRM_XFRM_BUF_INIT;
 	struct iosys_map dst_map, vmap;
 	size_t len;
 	void *buf;
@@ -75,7 +76,7 @@ static size_t gud_xrgb8888_to_r124(u8 *dst, const struct drm_format_info *format
 
 	iosys_map_set_vaddr(&dst_map, buf);
 	iosys_map_set_vaddr(&vmap, src);
-	drm_fb_xrgb8888_to_gray8(&dst_map, NULL, &vmap, fb, rect);
+	drm_fb_xrgb8888_to_gray8(&dst_map, NULL, &vmap, fb, rect, &xfrm);
 	pix8 = buf;
 
 	for (y = 0; y < height; y++) {
@@ -94,6 +95,7 @@ static size_t gud_xrgb8888_to_r124(u8 *dst, const struct drm_format_info *format
 	}
 
 	kfree(buf);
+	drm_xfrm_buf_release(&xfrm);
 
 	return len;
 }
@@ -154,6 +156,7 @@ static int gud_prep_flush(struct gud_device *gdrm, struct drm_framebuffer *fb,
 			  const struct drm_format_info *format, struct drm_rect *rect,
 			  struct gud_set_buffer_req *req)
 {
+	struct drm_xfrm_buf xfrm = DRM_XFRM_BUF_INIT;
 	u8 compression = gdrm->compression;
 	struct iosys_map dst;
 	void *vaddr, *buf;
@@ -179,22 +182,24 @@ static int gud_prep_flush(struct gud_device *gdrm, struct drm_framebuffer *fb,
 	if (format != fb->format) {
 		if (format->format == GUD_DRM_FORMAT_R1) {
 			len = gud_xrgb8888_to_r124(buf, format, vaddr, fb, rect);
-			if (!len)
+			if (!len) {
+				drm_xfrm_buf_release(&xfrm);
 				return -ENOMEM;
+			}
 		} else if (format->format == DRM_FORMAT_R8) {
-			drm_fb_xrgb8888_to_gray8(&dst, NULL, src, fb, rect);
+			drm_fb_xrgb8888_to_gray8(&dst, NULL, src, fb, rect, &xfrm);
 		} else if (format->format == DRM_FORMAT_RGB332) {
-			drm_fb_xrgb8888_to_rgb332(&dst, NULL, src, fb, rect);
+			drm_fb_xrgb8888_to_rgb332(&dst, NULL, src, fb, rect, &xfrm);
 		} else if (format->format == DRM_FORMAT_RGB565) {
-			drm_fb_xrgb8888_to_rgb565(&dst, NULL, src, fb, rect,
+			drm_fb_xrgb8888_to_rgb565(&dst, NULL, src, fb, rect, &xfrm,
 						  gud_is_big_endian());
 		} else if (format->format == DRM_FORMAT_RGB888) {
-			drm_fb_xrgb8888_to_rgb888(&dst, NULL, src, fb, rect);
+			drm_fb_xrgb8888_to_rgb888(&dst, NULL, src, fb, rect, &xfrm);
 		} else {
 			len = gud_xrgb8888_to_color(buf, format, vaddr, fb, rect);
 		}
 	} else if (gud_is_big_endian() && format->cpp[0] > 1) {
-		drm_fb_swab(&dst, NULL, src, fb, rect, cached_reads);
+		drm_fb_swab(&dst, NULL, src, fb, rect, cached_reads, &xfrm);
 	} else if (compression && cached_reads && pitch == fb->pitches[0]) {
 		/* can compress directly from the framebuffer */
 		buf = vaddr + rect->y1 * pitch;
@@ -222,6 +227,8 @@ static int gud_prep_flush(struct gud_device *gdrm, struct drm_framebuffer *fb,
 		req->compressed_length = cpu_to_le32(complen);
 	}
 
+	drm_xfrm_buf_release(&xfrm);
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/solomon/ssd130x.c b/drivers/gpu/drm/solomon/ssd130x.c
index 5a80b228d18ca..d11079733bc0e 100644
--- a/drivers/gpu/drm/solomon/ssd130x.c
+++ b/drivers/gpu/drm/solomon/ssd130x.c
@@ -571,6 +571,7 @@ static int ssd130x_fb_blit_rect(struct drm_plane_state *state,
 	struct ssd130x_device *ssd130x = drm_to_ssd130x(fb->dev);
 	unsigned int page_height = ssd130x->device_info->page_height;
 	struct ssd130x_plane_state *ssd130x_state = to_ssd130x_plane_state(state);
+	struct drm_xfrm_buf xfrm = DRM_XFRM_BUF_INIT;
 	u8 *buf = ssd130x_state->buffer;
 	struct iosys_map dst;
 	unsigned int dst_pitch;
@@ -587,12 +588,14 @@ static int ssd130x_fb_blit_rect(struct drm_plane_state *state,
 		return ret;
 
 	iosys_map_set_vaddr(&dst, buf);
-	drm_fb_xrgb8888_to_mono(&dst, &dst_pitch, vmap, fb, rect);
+	drm_fb_xrgb8888_to_mono(&dst, &dst_pitch, vmap, fb, rect, &xfrm);
 
 	drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
 
 	ssd130x_update_rect(ssd130x, ssd130x_state, rect);
 
+	drm_xfrm_buf_release(&xfrm);
+
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c b/drivers/gpu/drm/tests/drm_format_helper_test.c
index 1a6bd291345de..c6deabb6c64e5 100644
--- a/drivers/gpu/drm/tests/drm_format_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_format_helper_test.c
@@ -20,6 +20,10 @@
 
 #define TEST_USE_DEFAULT_PITCH 0
 
+static unsigned char conversion_buf_mem[PAGE_SIZE];
+static struct drm_xfrm_buf xfrm =
+	DRM_XFRM_BUF_INIT_PREALLOCATED(conversion_buf_mem, ARRAY_SIZE(conversion_buf_mem));
+
 struct convert_to_gray8_result {
 	unsigned int dst_pitch;
 	const u8 expected[TEST_BUF_SIZE];
@@ -568,8 +572,7 @@ static void drm_test_fb_xrgb8888_to_gray8(struct kunit *test)
 	const unsigned int *dst_pitch = (result->dst_pitch == TEST_USE_DEFAULT_PITCH) ?
 		NULL : &result->dst_pitch;
 
-	drm_fb_xrgb8888_to_gray8(&dst, dst_pitch, &src, &fb, &params->clip);
-
+	drm_fb_xrgb8888_to_gray8(&dst, dst_pitch, &src, &fb, &params->clip, &xfrm);
 	KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size);
 }
 
@@ -602,7 +605,7 @@ static void drm_test_fb_xrgb8888_to_rgb332(struct kunit *test)
 	const unsigned int *dst_pitch = (result->dst_pitch == TEST_USE_DEFAULT_PITCH) ?
 		NULL : &result->dst_pitch;
 
-	drm_fb_xrgb8888_to_rgb332(&dst, dst_pitch, &src, &fb, &params->clip);
+	drm_fb_xrgb8888_to_rgb332(&dst, dst_pitch, &src, &fb, &params->clip, &xfrm);
 	KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size);
 }
 
@@ -635,12 +638,14 @@ static void drm_test_fb_xrgb8888_to_rgb565(struct kunit *test)
 	const unsigned int *dst_pitch = (result->dst_pitch == TEST_USE_DEFAULT_PITCH) ?
 		NULL : &result->dst_pitch;
 
-	drm_fb_xrgb8888_to_rgb565(&dst, dst_pitch, &src, &fb, &params->clip, false);
+	drm_fb_xrgb8888_to_rgb565(&dst, dst_pitch, &src, &fb, &params->clip,
+				  &xfrm, false);
 	buf = le16buf_to_cpu(test, (__force const __le16 *)buf, dst_size / sizeof(__le16));
 	KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size);
 
 	buf = dst.vaddr; /* restore original value of buf */
-	drm_fb_xrgb8888_to_rgb565(&dst, &result->dst_pitch, &src, &fb, &params->clip, true);
+	drm_fb_xrgb8888_to_rgb565(&dst, &result->dst_pitch, &src, &fb, &params->clip,
+				  &xfrm, true);
 	buf = le16buf_to_cpu(test, (__force const __le16 *)buf, dst_size / sizeof(__le16));
 	KUNIT_EXPECT_MEMEQ(test, buf, result->expected_swab, dst_size);
 }
@@ -674,7 +679,7 @@ static void drm_test_fb_xrgb8888_to_xrgb1555(struct kunit *test)
 	const unsigned int *dst_pitch = (result->dst_pitch == TEST_USE_DEFAULT_PITCH) ?
 		NULL : &result->dst_pitch;
 
-	drm_fb_xrgb8888_to_xrgb1555(&dst, dst_pitch, &src, &fb, &params->clip);
+	drm_fb_xrgb8888_to_xrgb1555(&dst, dst_pitch, &src, &fb, &params->clip, &xfrm);
 	buf = le16buf_to_cpu(test, (__force const __le16 *)buf, dst_size / sizeof(__le16));
 	KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size);
 }
@@ -708,7 +713,7 @@ static void drm_test_fb_xrgb8888_to_argb1555(struct kunit *test)
 	const unsigned int *dst_pitch = (result->dst_pitch == TEST_USE_DEFAULT_PITCH) ?
 		NULL : &result->dst_pitch;
 
-	drm_fb_xrgb8888_to_argb1555(&dst, dst_pitch, &src, &fb, &params->clip);
+	drm_fb_xrgb8888_to_argb1555(&dst, dst_pitch, &src, &fb, &params->clip, &xfrm);
 	buf = le16buf_to_cpu(test, (__force const __le16 *)buf, dst_size / sizeof(__le16));
 	KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size);
 }
@@ -742,7 +747,7 @@ static void drm_test_fb_xrgb8888_to_rgba5551(struct kunit *test)
 	const unsigned int *dst_pitch = (result->dst_pitch == TEST_USE_DEFAULT_PITCH) ?
 		NULL : &result->dst_pitch;
 
-	drm_fb_xrgb8888_to_rgba5551(&dst, dst_pitch, &src, &fb, &params->clip);
+	drm_fb_xrgb8888_to_rgba5551(&dst, dst_pitch, &src, &fb, &params->clip, &xfrm);
 	buf = le16buf_to_cpu(test, (__force const __le16 *)buf, dst_size / sizeof(__le16));
 	KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size);
 }
@@ -780,7 +785,7 @@ static void drm_test_fb_xrgb8888_to_rgb888(struct kunit *test)
 	const unsigned int *dst_pitch = (result->dst_pitch == TEST_USE_DEFAULT_PITCH) ?
 		NULL : &result->dst_pitch;
 
-	drm_fb_xrgb8888_to_rgb888(&dst, dst_pitch, &src, &fb, &params->clip);
+	drm_fb_xrgb8888_to_rgb888(&dst, dst_pitch, &src, &fb, &params->clip, &xfrm);
 	KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size);
 }
 
@@ -813,7 +818,7 @@ static void drm_test_fb_xrgb8888_to_argb8888(struct kunit *test)
 	const unsigned int *dst_pitch = (result->dst_pitch == TEST_USE_DEFAULT_PITCH) ?
 		NULL : &result->dst_pitch;
 
-	drm_fb_xrgb8888_to_argb8888(&dst, dst_pitch, &src, &fb, &params->clip);
+	drm_fb_xrgb8888_to_argb8888(&dst, dst_pitch, &src, &fb, &params->clip, &xfrm);
 	buf = le32buf_to_cpu(test, (__force const __le32 *)buf, dst_size / sizeof(u32));
 	KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size);
 }
@@ -847,7 +852,7 @@ static void drm_test_fb_xrgb8888_to_xrgb2101010(struct kunit *test)
 	const unsigned int *dst_pitch = (result->dst_pitch == TEST_USE_DEFAULT_PITCH) ?
 		NULL : &result->dst_pitch;
 
-	drm_fb_xrgb8888_to_xrgb2101010(&dst, dst_pitch, &src, &fb, &params->clip);
+	drm_fb_xrgb8888_to_xrgb2101010(&dst, dst_pitch, &src, &fb, &params->clip, &xfrm);
 	buf = le32buf_to_cpu(test, buf, dst_size / sizeof(u32));
 	KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size);
 }
@@ -881,7 +886,7 @@ static void drm_test_fb_xrgb8888_to_argb2101010(struct kunit *test)
 	const unsigned int *dst_pitch = (result->dst_pitch == TEST_USE_DEFAULT_PITCH) ?
 		NULL : &result->dst_pitch;
 
-	drm_fb_xrgb8888_to_argb2101010(&dst, dst_pitch, &src, &fb, &params->clip);
+	drm_fb_xrgb8888_to_argb2101010(&dst, dst_pitch, &src, &fb, &params->clip, &xfrm);
 	buf = le32buf_to_cpu(test, (__force const __le32 *)buf, dst_size / sizeof(u32));
 	KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size);
 }
@@ -915,7 +920,7 @@ static void drm_test_fb_xrgb8888_to_mono(struct kunit *test)
 	const unsigned int *dst_pitch = (result->dst_pitch == TEST_USE_DEFAULT_PITCH) ?
 		NULL : &result->dst_pitch;
 
-	drm_fb_xrgb8888_to_mono(&dst, dst_pitch, &src, &fb, &params->clip);
+	drm_fb_xrgb8888_to_mono(&dst, dst_pitch, &src, &fb, &params->clip, &xfrm);
 	KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size);
 }
 
@@ -948,7 +953,7 @@ static void drm_test_fb_swab(struct kunit *test)
 	const unsigned int *dst_pitch = (result->dst_pitch == TEST_USE_DEFAULT_PITCH) ?
 		NULL : &result->dst_pitch;
 
-	drm_fb_swab(&dst, dst_pitch, &src, &fb, &params->clip, false);
+	drm_fb_swab(&dst, dst_pitch, &src, &fb, &params->clip, false, &xfrm);
 	buf = le32buf_to_cpu(test, (__force const __le32 *)buf, dst_size / sizeof(u32));
 	KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size);
 }
diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
index 594bc472862fe..74821d688daf5 100644
--- a/drivers/gpu/drm/tiny/cirrus.c
+++ b/drivers/gpu/drm/tiny/cirrus.c
@@ -391,6 +391,7 @@ static void cirrus_primary_plane_helper_atomic_update(struct drm_plane *plane,
 	struct cirrus_primary_plane_state *old_primary_plane_state =
 		to_cirrus_primary_plane_state(old_plane_state);
 	struct iosys_map vaddr = IOSYS_MAP_INIT_VADDR_IOMEM(cirrus->vram);
+	struct drm_xfrm_buf xfrm = DRM_XFRM_BUF_INIT;
 	struct drm_atomic_helper_damage_iter iter;
 	struct drm_rect damage;
 	int idx;
@@ -411,10 +412,12 @@ static void cirrus_primary_plane_helper_atomic_update(struct drm_plane *plane,
 		unsigned int offset = drm_fb_clip_offset(pitch, format, &damage);
 		struct iosys_map dst = IOSYS_MAP_INIT_OFFSET(&vaddr, offset);
 
-		drm_fb_blit(&dst, &pitch, format->format, shadow_plane_state->data, fb, &damage);
+		drm_fb_blit(&dst, &pitch, format->format, shadow_plane_state->data, fb,
+			    &damage, &xfrm);
 	}
 
 	drm_dev_exit(idx);
+	drm_xfrm_buf_release(&xfrm);
 }
 
 static const struct drm_plane_helper_funcs cirrus_primary_plane_helper_funcs = {
diff --git a/drivers/gpu/drm/tiny/ofdrm.c b/drivers/gpu/drm/tiny/ofdrm.c
index 2d999a0facdee..1add55c872670 100644
--- a/drivers/gpu/drm/tiny/ofdrm.c
+++ b/drivers/gpu/drm/tiny/ofdrm.c
@@ -796,6 +796,7 @@ static void ofdrm_primary_plane_helper_atomic_update(struct drm_plane *plane,
 	struct drm_framebuffer *fb = plane_state->fb;
 	unsigned int dst_pitch = odev->pitch;
 	const struct drm_format_info *dst_format = odev->format;
+	struct drm_xfrm_buf xfrm = DRM_XFRM_BUF_INIT;
 	struct drm_atomic_helper_damage_iter iter;
 	struct drm_rect damage;
 	int ret, idx;
@@ -817,12 +818,13 @@ static void ofdrm_primary_plane_helper_atomic_update(struct drm_plane *plane,
 
 		iosys_map_incr(&dst, drm_fb_clip_offset(dst_pitch, dst_format, &dst_clip));
 		drm_fb_blit(&dst, &dst_pitch, dst_format->format, shadow_plane_state->data, fb,
-			    &damage);
+			    &damage, &xfrm);
 	}
 
 	drm_dev_exit(idx);
 out_drm_gem_fb_end_cpu_access:
 	drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
+	drm_xfrm_buf_release(&xfrm);
 }
 
 static void ofdrm_primary_plane_helper_atomic_disable(struct drm_plane *plane,
diff --git a/drivers/gpu/drm/tiny/repaper.c b/drivers/gpu/drm/tiny/repaper.c
index 73dd4f4289c20..fddcaab78b92c 100644
--- a/drivers/gpu/drm/tiny/repaper.c
+++ b/drivers/gpu/drm/tiny/repaper.c
@@ -514,6 +514,7 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb)
 	struct drm_gem_dma_object *dma_obj = drm_fb_dma_get_gem_obj(fb, 0);
 	struct repaper_epd *epd = drm_to_epd(fb->dev);
 	unsigned int dst_pitch = 0;
+	struct drm_xfrm_buf xfrm = DRM_XFRM_BUF_INIT;
 	struct iosys_map dst, vmap;
 	struct drm_rect clip;
 	int idx, ret = 0;
@@ -545,7 +546,7 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb)
 
 	iosys_map_set_vaddr(&dst, buf);
 	iosys_map_set_vaddr(&vmap, dma_obj->vaddr);
-	drm_fb_xrgb8888_to_mono(&dst, &dst_pitch, &vmap, fb, &clip);
+	drm_fb_xrgb8888_to_mono(&dst, &dst_pitch, &vmap, fb, &clip, &xfrm);
 
 	drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
 
@@ -601,6 +602,8 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb)
 out_exit:
 	drm_dev_exit(idx);
 
+	drm_xfrm_buf_release(&xfrm);
+
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
index ff86ba1ae1b8b..8aceb7d378dea 100644
--- a/drivers/gpu/drm/tiny/simpledrm.c
+++ b/drivers/gpu/drm/tiny/simpledrm.c
@@ -486,6 +486,7 @@ static void simpledrm_primary_plane_helper_atomic_update(struct drm_plane *plane
 	struct drm_framebuffer *fb = plane_state->fb;
 	struct drm_device *dev = plane->dev;
 	struct simpledrm_device *sdev = simpledrm_device_of_dev(dev);
+	struct drm_xfrm_buf xfrm = DRM_XFRM_BUF_INIT;
 	struct drm_atomic_helper_damage_iter iter;
 	struct drm_rect damage;
 	int ret, idx;
@@ -507,12 +508,13 @@ static void simpledrm_primary_plane_helper_atomic_update(struct drm_plane *plane
 
 		iosys_map_incr(&dst, drm_fb_clip_offset(sdev->pitch, sdev->format, &dst_clip));
 		drm_fb_blit(&dst, &sdev->pitch, sdev->format->format, shadow_plane_state->data,
-			    fb, &damage);
+			    fb, &damage, &xfrm);
 	}
 
 	drm_dev_exit(idx);
 out_drm_gem_fb_end_cpu_access:
 	drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
+	drm_xfrm_buf_release(&xfrm);
 }
 
 static void simpledrm_primary_plane_helper_atomic_disable(struct drm_plane *plane,
diff --git a/drivers/gpu/drm/tiny/st7586.c b/drivers/gpu/drm/tiny/st7586.c
index 3cf4eec16a813..e4d1e071aae56 100644
--- a/drivers/gpu/drm/tiny/st7586.c
+++ b/drivers/gpu/drm/tiny/st7586.c
@@ -67,6 +67,7 @@ static void st7586_xrgb8888_to_gray332(u8 *dst, void *vaddr,
 				       struct drm_rect *clip)
 {
 	size_t len = (clip->x2 - clip->x1) * (clip->y2 - clip->y1);
+	struct drm_xfrm_buf xfrm = DRM_XFRM_BUF_INIT;
 	unsigned int x, y;
 	u8 *src, *buf, val;
 	struct iosys_map dst_map, vmap;
@@ -77,7 +78,7 @@ static void st7586_xrgb8888_to_gray332(u8 *dst, void *vaddr,
 
 	iosys_map_set_vaddr(&dst_map, buf);
 	iosys_map_set_vaddr(&vmap, vaddr);
-	drm_fb_xrgb8888_to_gray8(&dst_map, NULL, &vmap, fb, clip);
+	drm_fb_xrgb8888_to_gray8(&dst_map, NULL, &vmap, fb, clip, &xfrm);
 	src = buf;
 
 	for (y = clip->y1; y < clip->y2; y++) {
@@ -90,6 +91,8 @@ static void st7586_xrgb8888_to_gray332(u8 *dst, void *vaddr,
 	}
 
 	kfree(buf);
+
+	drm_xfrm_buf_release(&xfrm);
 }
 
 static int st7586_buf_copy(void *dst, struct iosys_map *src, struct drm_framebuffer *fb,
diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
index 245a5edc4735a..11bab88114e2d 100644
--- a/include/drm/drm_format_helper.h
+++ b/include/drm/drm_format_helper.h
@@ -69,45 +69,47 @@ void drm_fb_memcpy(struct iosys_map *dst, const unsigned int *dst_pitch,
 		   const struct drm_rect *clip);
 void drm_fb_swab(struct iosys_map *dst, const unsigned int *dst_pitch,
 		 const struct iosys_map *src, const struct drm_framebuffer *fb,
-		 const struct drm_rect *clip, bool cached);
+		 const struct drm_rect *clip, bool cached,
+		 struct drm_xfrm_buf *xfrm);
 void drm_fb_xrgb8888_to_rgb332(struct iosys_map *dst, const unsigned int *dst_pitch,
 			       const struct iosys_map *src, const struct drm_framebuffer *fb,
-			       const struct drm_rect *clip);
+			       const struct drm_rect *clip, struct drm_xfrm_buf *xfrm);
 void drm_fb_xrgb8888_to_rgb565(struct iosys_map *dst, const unsigned int *dst_pitch,
 			       const struct iosys_map *src, const struct drm_framebuffer *fb,
-			       const struct drm_rect *clip, bool swab);
+			       const struct drm_rect *clip, struct drm_xfrm_buf *xfrm,
+			       bool swab);
 void drm_fb_xrgb8888_to_xrgb1555(struct iosys_map *dst, const unsigned int *dst_pitch,
 				 const struct iosys_map *src, const struct drm_framebuffer *fb,
-				 const struct drm_rect *clip);
+				 const struct drm_rect *clip, struct drm_xfrm_buf *xfrm);
 void drm_fb_xrgb8888_to_argb1555(struct iosys_map *dst, const unsigned int *dst_pitch,
 				 const struct iosys_map *src, const struct drm_framebuffer *fb,
-				 const struct drm_rect *clip);
+				 const struct drm_rect *clip, struct drm_xfrm_buf *xfrm);
 void drm_fb_xrgb8888_to_rgba5551(struct iosys_map *dst, const unsigned int *dst_pitch,
 				 const struct iosys_map *src, const struct drm_framebuffer *fb,
-				 const struct drm_rect *clip);
+				 const struct drm_rect *clip, struct drm_xfrm_buf *xfrm);
 void drm_fb_xrgb8888_to_rgb888(struct iosys_map *dst, const unsigned int *dst_pitch,
 			       const struct iosys_map *src, const struct drm_framebuffer *fb,
-			       const struct drm_rect *clip);
+			       const struct drm_rect *clip, struct drm_xfrm_buf *xfrm);
 void drm_fb_xrgb8888_to_argb8888(struct iosys_map *dst, const unsigned int *dst_pitch,
 				 const struct iosys_map *src, const struct drm_framebuffer *fb,
-				 const struct drm_rect *clip);
+				 const struct drm_rect *clip, struct drm_xfrm_buf *xfrm);
 void drm_fb_xrgb8888_to_xrgb2101010(struct iosys_map *dst, const unsigned int *dst_pitch,
 				    const struct iosys_map *src, const struct drm_framebuffer *fb,
-				    const struct drm_rect *clip);
+				    const struct drm_rect *clip, struct drm_xfrm_buf *xfrm);
 void drm_fb_xrgb8888_to_argb2101010(struct iosys_map *dst, const unsigned int *dst_pitch,
 				    const struct iosys_map *src, const struct drm_framebuffer *fb,
-				    const struct drm_rect *clip);
+				    const struct drm_rect *clip, struct drm_xfrm_buf *xfrm);
 void drm_fb_xrgb8888_to_gray8(struct iosys_map *dst, const unsigned int *dst_pitch,
 			      const struct iosys_map *src, const struct drm_framebuffer *fb,
-			      const struct drm_rect *clip);
+			      const struct drm_rect *clip, struct drm_xfrm_buf *xfrm);
 
 int drm_fb_blit(struct iosys_map *dst, const unsigned int *dst_pitch, uint32_t dst_format,
 		const struct iosys_map *src, const struct drm_framebuffer *fb,
-		const struct drm_rect *rect);
+		const struct drm_rect *clip, struct drm_xfrm_buf *xfrm);
 
 void drm_fb_xrgb8888_to_mono(struct iosys_map *dst, const unsigned int *dst_pitch,
 			     const struct iosys_map *src, const struct drm_framebuffer *fb,
-			     const struct drm_rect *clip);
+			     const struct drm_rect *clip, struct drm_xfrm_buf *xfrm);
 
 size_t drm_fb_build_fourcc_list(struct drm_device *dev,
 				const u32 *native_fourccs, size_t native_nfourccs,
-- 
2.42.0

[PATCH v2 3/5] drm/simpledrm: Store xfrm buffer in device instance

[Thomas Zimmermann]

Store and instance of struct drm_xfrm_buf in struct simpledrm_device
and keep the allocated memory allocated across display updates. Avoid
possibly reallocating temporary memory on each display update. Instead
preallocate temporary memory during initialization. Releasing the DRM
device also releases the xfrm buffer.

v2:
	* reserve storage during probe

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/tiny/simpledrm.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
index 8aceb7d378dea..a3d8a956a4c4e 100644
--- a/drivers/gpu/drm/tiny/simpledrm.c
+++ b/drivers/gpu/drm/tiny/simpledrm.c
@@ -232,6 +232,7 @@ struct simpledrm_device {
 	struct drm_display_mode mode;
 	const struct drm_format_info *format;
 	unsigned int pitch;
+	struct drm_xfrm_buf xfrm;
 
 	/* memory management */
 	struct iosys_map screen_base;
@@ -486,7 +487,6 @@ static void simpledrm_primary_plane_helper_atomic_update(struct drm_plane *plane
 	struct drm_framebuffer *fb = plane_state->fb;
 	struct drm_device *dev = plane->dev;
 	struct simpledrm_device *sdev = simpledrm_device_of_dev(dev);
-	struct drm_xfrm_buf xfrm = DRM_XFRM_BUF_INIT;
 	struct drm_atomic_helper_damage_iter iter;
 	struct drm_rect damage;
 	int ret, idx;
@@ -508,13 +508,12 @@ static void simpledrm_primary_plane_helper_atomic_update(struct drm_plane *plane
 
 		iosys_map_incr(&dst, drm_fb_clip_offset(sdev->pitch, sdev->format, &dst_clip));
 		drm_fb_blit(&dst, &sdev->pitch, sdev->format->format, shadow_plane_state->data,
-			    fb, &damage, &xfrm);
+			    fb, &damage, &sdev->xfrm);
 	}
 
 	drm_dev_exit(idx);
 out_drm_gem_fb_end_cpu_access:
 	drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
-	drm_xfrm_buf_release(&xfrm);
 }
 
 static void simpledrm_primary_plane_helper_atomic_disable(struct drm_plane *plane,
@@ -637,6 +636,7 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
 	struct drm_connector *connector;
 	unsigned long max_width, max_height;
 	size_t nformats;
+	void *buf;
 	int ret;
 
 	sdev = devm_drm_dev_alloc(&pdev->dev, drv, struct simpledrm_device, dev);
@@ -718,6 +718,13 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
 	drm_dbg(dev, "framebuffer format=%p4cc, size=%dx%d, stride=%d byte\n",
 		&format->format, width, height, stride);
 
+	ret = drmm_xfrm_buf_init(dev, &sdev->xfrm);
+	if (ret)
+		return ERR_PTR(ret);
+	buf = drm_xfrm_buf_reserve(&sdev->xfrm, sdev->pitch, GFP_KERNEL);
+	if (!buf)
+		return ERR_PTR(-ENOMEM);
+
 	/*
 	 * Memory management
 	 */
-- 
2.42.0

[PATCH v2 4/5] drm/ofdrm: Store xfrm buffer in device instance

[Thomas Zimmermann]

Store and instance of struct drm_xfrm_buf in struct ofdrm_device and
keep the allocated memory allocated across display updates. Avoid
possibly reallocating temporary memory on each display update. Instead
preallocate temporary memory during initialization. Releasing the DRM
device also releases the xfrm buffer.

v2:
	* reserve storage during probe

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/tiny/ofdrm.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/tiny/ofdrm.c b/drivers/gpu/drm/tiny/ofdrm.c
index 1add55c872670..ede7b168bd9d0 100644
--- a/drivers/gpu/drm/tiny/ofdrm.c
+++ b/drivers/gpu/drm/tiny/ofdrm.c
@@ -301,6 +301,7 @@ struct ofdrm_device {
 	struct drm_display_mode mode;
 	const struct drm_format_info *format;
 	unsigned int pitch;
+	struct drm_xfrm_buf xfrm;
 
 	/* colormap */
 	void __iomem *cmap_base;
@@ -796,7 +797,6 @@ static void ofdrm_primary_plane_helper_atomic_update(struct drm_plane *plane,
 	struct drm_framebuffer *fb = plane_state->fb;
 	unsigned int dst_pitch = odev->pitch;
 	const struct drm_format_info *dst_format = odev->format;
-	struct drm_xfrm_buf xfrm = DRM_XFRM_BUF_INIT;
 	struct drm_atomic_helper_damage_iter iter;
 	struct drm_rect damage;
 	int ret, idx;
@@ -818,13 +818,12 @@ static void ofdrm_primary_plane_helper_atomic_update(struct drm_plane *plane,
 
 		iosys_map_incr(&dst, drm_fb_clip_offset(dst_pitch, dst_format, &dst_clip));
 		drm_fb_blit(&dst, &dst_pitch, dst_format->format, shadow_plane_state->data, fb,
-			    &damage, &xfrm);
+			    &damage, &odev->xfrm);
 	}
 
 	drm_dev_exit(idx);
 out_drm_gem_fb_end_cpu_access:
 	drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
-	drm_xfrm_buf_release(&xfrm);
 }
 
 static void ofdrm_primary_plane_helper_atomic_disable(struct drm_plane *plane,
@@ -1096,6 +1095,7 @@ static struct ofdrm_device *ofdrm_device_create(struct drm_driver *drv,
 	struct drm_connector *connector;
 	unsigned long max_width, max_height;
 	size_t nformats;
+	void *buf;
 	int ret;
 
 	odev = devm_drm_dev_alloc(&pdev->dev, drv, struct ofdrm_device, dev);
@@ -1248,6 +1248,13 @@ static struct ofdrm_device *ofdrm_device_create(struct drm_driver *drv,
 	drm_dbg(dev, "framebuffer format=%p4cc, size=%dx%d, linebytes=%d byte\n",
 		&format->format, width, height, linebytes);
 
+	ret = drmm_xfrm_buf_init(dev, &odev->xfrm);
+	if (ret)
+		return ERR_PTR(ret);
+	buf = drm_xfrm_buf_reserve(&odev->xfrm, odev->pitch, GFP_KERNEL);
+	if (!buf)
+		return ERR_PTR(-ENOMEM);
+
 	/*
 	 * Mode-setting pipeline
 	 */
-- 
2.42.0

[PATCH v2 5/5] drm/ssd130x: Store xfrm buffer in device instance

[Thomas Zimmermann]

Store and instance of struct drm_xfrm_buf in struct ssd130x_device and
keep the allocated memory allocated across display updates. Avoid
possibly reallocating temporary memory on each display update. Instead
preallocate temporary memory during initialization. Releasing the DRM
device also releases the xfrm buffer.

v2:
	* reserve storage during probe

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/solomon/ssd130x.c | 19 +++++++++++++++----
 drivers/gpu/drm/solomon/ssd130x.h |  3 +++
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/solomon/ssd130x.c b/drivers/gpu/drm/solomon/ssd130x.c
index d11079733bc0e..93a5df31d0d9a 100644
--- a/drivers/gpu/drm/solomon/ssd130x.c
+++ b/drivers/gpu/drm/solomon/ssd130x.c
@@ -571,7 +571,6 @@ static int ssd130x_fb_blit_rect(struct drm_plane_state *state,
 	struct ssd130x_device *ssd130x = drm_to_ssd130x(fb->dev);
 	unsigned int page_height = ssd130x->device_info->page_height;
 	struct ssd130x_plane_state *ssd130x_state = to_ssd130x_plane_state(state);
-	struct drm_xfrm_buf xfrm = DRM_XFRM_BUF_INIT;
 	u8 *buf = ssd130x_state->buffer;
 	struct iosys_map dst;
 	unsigned int dst_pitch;
@@ -588,14 +587,12 @@ static int ssd130x_fb_blit_rect(struct drm_plane_state *state,
 		return ret;
 
 	iosys_map_set_vaddr(&dst, buf);
-	drm_fb_xrgb8888_to_mono(&dst, &dst_pitch, vmap, fb, rect, &xfrm);
+	drm_fb_xrgb8888_to_mono(&dst, &dst_pitch, vmap, fb, rect, &ssd130x->xfrm);
 
 	drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
 
 	ssd130x_update_rect(ssd130x, ssd130x_state, rect);
 
-	drm_xfrm_buf_release(&xfrm);
-
 	return ret;
 }
 
@@ -1084,6 +1081,8 @@ struct ssd130x_device *ssd130x_probe(struct device *dev, struct regmap *regmap)
 	struct ssd130x_device *ssd130x;
 	struct backlight_device *bl;
 	struct drm_device *drm;
+	const struct drm_format_info *fi;
+	void *buf;
 	int ret;
 
 	ssd130x = devm_drm_dev_alloc(dev, &ssd130x_drm_driver,
@@ -1117,6 +1116,18 @@ struct ssd130x_device *ssd130x_probe(struct device *dev, struct regmap *regmap)
 	bl->props.max_brightness = MAX_CONTRAST;
 	ssd130x->bl_dev = bl;
 
+	ret = drmm_xfrm_buf_init(drm, &ssd130x->xfrm);
+	if (ret)
+		return ERR_PTR(ret);
+	fi = drm_format_info(DRM_FORMAT_R1);
+	if (!fi)
+		return ERR_PTR(-EINVAL);
+	buf = drm_xfrm_buf_reserve(&ssd130x->xfrm,
+				   drm_format_info_min_pitch(fi, 0, ssd130x->width),
+				   GFP_KERNEL);
+	if (!buf)
+		return ERR_PTR(-ENOMEM);
+
 	ret = ssd130x_init_modeset(ssd130x);
 	if (ret)
 		return ERR_PTR(ret);
diff --git a/drivers/gpu/drm/solomon/ssd130x.h b/drivers/gpu/drm/solomon/ssd130x.h
index 87968b3e7fb82..f4e525feb2188 100644
--- a/drivers/gpu/drm/solomon/ssd130x.h
+++ b/drivers/gpu/drm/solomon/ssd130x.h
@@ -17,6 +17,7 @@
 #include <drm/drm_crtc.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_encoder.h>
+#include <drm/drm_format_helper.h>
 #include <drm/drm_plane_helper.h>
 
 #include <linux/regmap.h>
@@ -55,6 +56,8 @@ struct ssd130x_device {
 	struct drm_connector connector;
 	struct i2c_client *client;
 
+	struct drm_xfrm_buf xfrm;
+
 	struct regmap *regmap;
 
 	const struct ssd130x_deviceinfo *device_info;
-- 
2.42.0

Re: [PATCH v2 0/5] drm: Reuse temporary memory for format conversion

[Jocelyn Falempe]

On 20/09/2023 16:24, Thomas Zimmermann wrote:
> DRM's format-conversion helpers require temporary memory. Pass the
> buffer from the caller and keep it allocated over several calls. Allow
> the caller to preallocate the buffer memory.
> 
> The motivation for this patchset is the recent work on a DRM panic
> handler. The panic handler requires format conversion to display an
> error to the screen. But allocating memory during kernel panics is
> fragile. The changes in this patchset enable the DRM panic handler to
> preallocate buffer storage before the panic occurs.
> 
> As an additonal benefit, drivers can now keep the temporary storage
> across multiple display updates. Avoiding memory allocation reduces
> the CPU overhead of the format helpers.
> 
> Patch 1 adds struct drm_xfrm_buf, a simple interface to pass around
> the buffer storage. Patch 2 moves the memory management from the format
> helpers into their callers. Drivers release the temporary storage at
> the end of their display-update functions.
> 
> Patches 3 to 8 update three drivers to keep the allocated memory for
> all of a device's lifetime. Managed cleanup releases the buffer as part
> of releaseing the device. As additional benefit, buffer allocation now
> happens in atomic_check helpers. The driver thus detects OOM errors
> before the display update begins.

Thanks for the patches.
I'm still experimenting with drm_panic, and it's not clear if it will 
use that or not yet.
But it's already useful for other drivers, avoiding alloc/free for each 
frame, is a good thing.


Best regards,

-- 

Jocelyn


> 
> Tested with simpledrm.
> 
> v2:
> 	* reserve storage during probing in the drivers
> 
> Thomas Zimmermann (5):
>    drm/format-helper: Add struct drm_xfrm_buf to cache format conversion
>    drm/format-helper: Pass xfrm buffer to format-conversion helpers
>    drm/simpledrm: Store xfrm buffer in device instance
>    drm/ofdrm: Store xfrm buffer in device instance
>    drm/ssd130x: Store xfrm buffer in device instance
> 
>   drivers/gpu/drm/drm_format_helper.c           | 204 +++++++++++++-----
>   drivers/gpu/drm/drm_mipi_dbi.c                |   7 +-
>   drivers/gpu/drm/gud/gud_pipe.c                |  21 +-
>   drivers/gpu/drm/solomon/ssd130x.c             |  16 +-
>   drivers/gpu/drm/solomon/ssd130x.h             |   3 +
>   .../gpu/drm/tests/drm_format_helper_test.c    |  33 +--
>   drivers/gpu/drm/tiny/cirrus.c                 |   5 +-
>   drivers/gpu/drm/tiny/ofdrm.c                  |  11 +-
>   drivers/gpu/drm/tiny/repaper.c                |   5 +-
>   drivers/gpu/drm/tiny/simpledrm.c              |  11 +-
>   drivers/gpu/drm/tiny/st7586.c                 |   5 +-
>   include/drm/drm_format_helper.h               |  74 +++++--
>   12 files changed, 300 insertions(+), 95 deletions(-)
> 

Re: [PATCH v2 3/5] drm/simpledrm: Store xfrm buffer in device instance

[Jocelyn Falempe]

On 20/09/2023 16:24, Thomas Zimmermann wrote:
> Store and instance of struct drm_xfrm_buf in struct simpledrm_device
> and keep the allocated memory allocated across display updates. Avoid
> possibly reallocating temporary memory on each display update. Instead
> preallocate temporary memory during initialization. Releasing the DRM
> device also releases the xfrm buffer.
> 
> v2:
> 	* reserve storage during probe
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>   drivers/gpu/drm/tiny/simpledrm.c | 13 ++++++++++---
>   1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
> index 8aceb7d378dea..a3d8a956a4c4e 100644
> --- a/drivers/gpu/drm/tiny/simpledrm.c
> +++ b/drivers/gpu/drm/tiny/simpledrm.c
> @@ -232,6 +232,7 @@ struct simpledrm_device {
>   	struct drm_display_mode mode;
>   	const struct drm_format_info *format;
>   	unsigned int pitch;
> +	struct drm_xfrm_buf xfrm;
>   
>   	/* memory management */
>   	struct iosys_map screen_base;
> @@ -486,7 +487,6 @@ static void simpledrm_primary_plane_helper_atomic_update(struct drm_plane *plane
>   	struct drm_framebuffer *fb = plane_state->fb;
>   	struct drm_device *dev = plane->dev;
>   	struct simpledrm_device *sdev = simpledrm_device_of_dev(dev);
> -	struct drm_xfrm_buf xfrm = DRM_XFRM_BUF_INIT;
>   	struct drm_atomic_helper_damage_iter iter;
>   	struct drm_rect damage;
>   	int ret, idx;
> @@ -508,13 +508,12 @@ static void simpledrm_primary_plane_helper_atomic_update(struct drm_plane *plane
>   
>   		iosys_map_incr(&dst, drm_fb_clip_offset(sdev->pitch, sdev->format, &dst_clip));
>   		drm_fb_blit(&dst, &sdev->pitch, sdev->format->format, shadow_plane_state->data,
> -			    fb, &damage, &xfrm);
> +			    fb, &damage, &sdev->xfrm);
>   	}
>   
>   	drm_dev_exit(idx);
>   out_drm_gem_fb_end_cpu_access:
>   	drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
> -	drm_xfrm_buf_release(&xfrm);
>   }
>   
>   static void simpledrm_primary_plane_helper_atomic_disable(struct drm_plane *plane,
> @@ -637,6 +636,7 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
>   	struct drm_connector *connector;
>   	unsigned long max_width, max_height;
>   	size_t nformats;
> +	void *buf;
>   	int ret;
>   
>   	sdev = devm_drm_dev_alloc(&pdev->dev, drv, struct simpledrm_device, dev);
> @@ -718,6 +718,13 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
>   	drm_dbg(dev, "framebuffer format=%p4cc, size=%dx%d, stride=%d byte\n",
>   		&format->format, width, height, stride);
>   
> +	ret = drmm_xfrm_buf_init(dev, &sdev->xfrm);
> +	if (ret)
> +		return ERR_PTR(ret);
> +	buf = drm_xfrm_buf_reserve(&sdev->xfrm, sdev->pitch, GFP_KERNEL);
> +	if (!buf)
> +		return ERR_PTR(-ENOMEM);
> +

I think it would be nice to have a "init_and_reserve()" function, to 
simplify the callers ?

>   	/*
>   	 * Memory management
>   	 */

Re: [PATCH v2 3/5] drm/simpledrm: Store xfrm buffer in device instance

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 3919 bytes --]

Hi

Am 26.09.23 um 09:31 schrieb Jocelyn Falempe:
> On 20/09/2023 16:24, Thomas Zimmermann wrote:
>> Store and instance of struct drm_xfrm_buf in struct simpledrm_device
>> and keep the allocated memory allocated across display updates. Avoid
>> possibly reallocating temporary memory on each display update. Instead
>> preallocate temporary memory during initialization. Releasing the DRM
>> device also releases the xfrm buffer.
>>
>> v2:
>>     * reserve storage during probe
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>   drivers/gpu/drm/tiny/simpledrm.c | 13 ++++++++++---
>>   1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/tiny/simpledrm.c 
>> b/drivers/gpu/drm/tiny/simpledrm.c
>> index 8aceb7d378dea..a3d8a956a4c4e 100644
>> --- a/drivers/gpu/drm/tiny/simpledrm.c
>> +++ b/drivers/gpu/drm/tiny/simpledrm.c
>> @@ -232,6 +232,7 @@ struct simpledrm_device {
>>       struct drm_display_mode mode;
>>       const struct drm_format_info *format;
>>       unsigned int pitch;
>> +    struct drm_xfrm_buf xfrm;
>>       /* memory management */
>>       struct iosys_map screen_base;
>> @@ -486,7 +487,6 @@ static void 
>> simpledrm_primary_plane_helper_atomic_update(struct drm_plane *plane
>>       struct drm_framebuffer *fb = plane_state->fb;
>>       struct drm_device *dev = plane->dev;
>>       struct simpledrm_device *sdev = simpledrm_device_of_dev(dev);
>> -    struct drm_xfrm_buf xfrm = DRM_XFRM_BUF_INIT;
>>       struct drm_atomic_helper_damage_iter iter;
>>       struct drm_rect damage;
>>       int ret, idx;
>> @@ -508,13 +508,12 @@ static void 
>> simpledrm_primary_plane_helper_atomic_update(struct drm_plane *plane
>>           iosys_map_incr(&dst, drm_fb_clip_offset(sdev->pitch, 
>> sdev->format, &dst_clip));
>>           drm_fb_blit(&dst, &sdev->pitch, sdev->format->format, 
>> shadow_plane_state->data,
>> -                fb, &damage, &xfrm);
>> +                fb, &damage, &sdev->xfrm);
>>       }
>>       drm_dev_exit(idx);
>>   out_drm_gem_fb_end_cpu_access:
>>       drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
>> -    drm_xfrm_buf_release(&xfrm);
>>   }
>>   static void simpledrm_primary_plane_helper_atomic_disable(struct 
>> drm_plane *plane,
>> @@ -637,6 +636,7 @@ static struct simpledrm_device 
>> *simpledrm_device_create(struct drm_driver *drv,
>>       struct drm_connector *connector;
>>       unsigned long max_width, max_height;
>>       size_t nformats;
>> +    void *buf;
>>       int ret;
>>       sdev = devm_drm_dev_alloc(&pdev->dev, drv, struct 
>> simpledrm_device, dev);
>> @@ -718,6 +718,13 @@ static struct simpledrm_device 
>> *simpledrm_device_create(struct drm_driver *drv,
>>       drm_dbg(dev, "framebuffer format=%p4cc, size=%dx%d, stride=%d 
>> byte\n",
>>           &format->format, width, height, stride);
>> +    ret = drmm_xfrm_buf_init(dev, &sdev->xfrm);
>> +    if (ret)
>> +        return ERR_PTR(ret);
>> +    buf = drm_xfrm_buf_reserve(&sdev->xfrm, sdev->pitch, GFP_KERNEL);
>> +    if (!buf)
>> +        return ERR_PTR(-ENOMEM);
>> +
> 
> I think it would be nice to have a "init_and_reserve()" function, to 
> simplify the callers ?

I think I can add the reserve parameters directly to the _init() 
function. Reserving '0' will then not reserve memory.

Best regards
Thomas

> 
>>       /*
>>        * Memory management
>>        */
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH v2 1/5] drm/format-helper: Add struct drm_xfrm_buf to cache format conversion

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann@suse.de> writes:

> Hold temporary memory for format conversion in an instance of struct
> drm_xfrm_buf. Update internal helpers of DRM's format-conversion code
> accordingly. Drivers will later be able to keep this cache across
> display updates.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---

[...]

> +int drmm_xfrm_buf_init(struct drm_device *dev, struct drm_xfrm_buf *buf)
> +{
> +	buf->mem = NULL;
> +	buf->size = 0;
> +	buf->preallocated = false;
> +
> +	return drmm_add_action_or_reset(dev, drm_xfrm_buf_init_release, buf);
> +}
> +EXPORT_SYMBOL(drmm_xfrm_buf_init);
> +

Can we find a better name than xfrm? I know that this is what's used in
the internal drm_format_helper.c helpers but if we are exposing this to
drivers, then I think that the naming is not self explanatory.

> +/**
> + * drm_xfrm_buf_reserve - Allocates storage in an xfrm buffer
> + * @buf: The xfrm buffer

At least in the kernel-doc we can say "The buffer used for pixel format
conversion" or something along those lines.

[...]

> +/**
> + * struct drm_xfrm_buf - Stores transformation and conversion state
> + *
> + * DRM helpers for format conversion store temporary state in
> + * struct drm_xfrm_buf. The buffer's resources can be reused

And same here. Maybe struct drm_fmt_conversion_buf ?

Other than this nit, the patch looks good to me.

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v2 2/5] drm/format-helper: Pass xfrm buffer to format-conversion helpers

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann@suse.de> writes:

Hello Thomas,

> Pass an instance of struct drm_xfrm_buf to DRM's format conversion
> helpers. Update all callers. Drivers will later be able to keep this
> cache across display updates.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Noralf Trønnes <noralf@tronnes.org>
> Cc: Javier Martinez Canillas <javierm@redhat.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: David Lechner <david@lechnology.com>
> ---

[...]

> diff --git a/drivers/gpu/drm/solomon/ssd130x.c b/drivers/gpu/drm/solomon/ssd130x.c
> index 5a80b228d18ca..d11079733bc0e 100644
> --- a/drivers/gpu/drm/solomon/ssd130x.c
> +++ b/drivers/gpu/drm/solomon/ssd130x.c
> @@ -571,6 +571,7 @@ static int ssd130x_fb_blit_rect(struct drm_plane_state *state,
>  	struct ssd130x_device *ssd130x = drm_to_ssd130x(fb->dev);
>  	unsigned int page_height = ssd130x->device_info->page_height;
>  	struct ssd130x_plane_state *ssd130x_state = to_ssd130x_plane_state(state);
> +	struct drm_xfrm_buf xfrm = DRM_XFRM_BUF_INIT;

I would prefer if this structure is a field of struct ssd130x_plane_state.

Since ssd130x_primary_plane_helper_atomic_check() zero allocates that, it
will have the same initial values as set by DRM_XFRM_BUF_INIT.

Alternatively you can do a drmm_xfrm_buf_init() + drm_xfrm_buf_reserve()
in ssd130x_primary_plane_helper_atomic_check().

>  	u8 *buf = ssd130x_state->buffer;

and struct drm_xfrm_buf *xfrm = &ssd130x_state->xfrm;

>  	struct iosys_map dst;
>  	unsigned int dst_pitch;
> @@ -587,12 +588,14 @@ static int ssd130x_fb_blit_rect(struct drm_plane_state *state,
>  		return ret;
>  
>  	iosys_map_set_vaddr(&dst, buf);
> -	drm_fb_xrgb8888_to_mono(&dst, &dst_pitch, vmap, fb, rect);
> +	drm_fb_xrgb8888_to_mono(&dst, &dst_pitch, vmap, fb, rect, &xfrm);
>  
>  	drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
>  
>  	ssd130x_update_rect(ssd130x, ssd130x_state, rect);
>  
> +	drm_xfrm_buf_release(&xfrm);
> +

and you can release it in ssd130x_primary_plane_destroy_state().

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v2 5/5] drm/ssd130x: Store xfrm buffer in device instance

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann@suse.de> writes:

> Store and instance of struct drm_xfrm_buf in struct ssd130x_device and
> keep the allocated memory allocated across display updates. Avoid
> possibly reallocating temporary memory on each display update. Instead
> preallocate temporary memory during initialization. Releasing the DRM
> device also releases the xfrm buffer.
>
> v2:
> 	* reserve storage during probe
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---

[...]

> @@ -1084,6 +1081,8 @@ struct ssd130x_device *ssd130x_probe(struct device *dev, struct regmap *regmap)
>  	struct ssd130x_device *ssd130x;
>  	struct backlight_device *bl;
>  	struct drm_device *drm;
> +	const struct drm_format_info *fi;
> +	void *buf;
>  	int ret;
>  
>  	ssd130x = devm_drm_dev_alloc(dev, &ssd130x_drm_driver,
> @@ -1117,6 +1116,18 @@ struct ssd130x_device *ssd130x_probe(struct device *dev, struct regmap *regmap)
>  	bl->props.max_brightness = MAX_CONTRAST;
>  	ssd130x->bl_dev = bl;
>  
> +	ret = drmm_xfrm_buf_init(drm, &ssd130x->xfrm);
> +	if (ret)
> +		return ERR_PTR(ret);
> +	fi = drm_format_info(DRM_FORMAT_R1);
> +	if (!fi)
> +		return ERR_PTR(-EINVAL);
> +	buf = drm_xfrm_buf_reserve(&ssd130x->xfrm,
> +				   drm_format_info_min_pitch(fi, 0, ssd130x->width),
> +				   GFP_KERNEL);
> +	if (!buf)
> +		return ERR_PTR(-ENOMEM);
> +
  
I think this is OK but then I wonder if we should not just allocate all
the buffers in the probe function. Right now, what the driver does is to
have two structures to keep the driver-private atomic state:

1) struct ssd130x_crtc_state that has a .data_array to store the pixels
   in the HW format (e.g: R1) and written to the panel.

2) struct ssd130x_plane_state that has a .buffer to store the pixels that
   are converted from the emulated XRGB8888 used by the shadow-plane, to
   the HW pixel format.

The (2) will be optional once Geert's R1 support lands. Now we are adding
a third buffer so I wonder if should be part of one of these private state
or not.

I said that should be a field of struct ssd130x_plane_state in a previous
email, but on a second thought it makes more sense if is a field of the
struct ssd130x_crtc_state.

That way the allocation will only be in ssd130x_crtc_atomic_check() and
the release in the ssd130x_crtc_destroy_state(). If you do that on patch
#2, then this patch #5 could be dropped.

The reason why I added those private state structures is twofold: because
the buffers are tied to the CRTC and planes and to show how a driver can
maintain their own private atomic state.

After all, one of my goals of this driver is to be used for educational
purposes and provide a simple driver that people can use as a reference.

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v2 0/5] drm: Reuse temporary memory for format conversion

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2744 bytes --]

On Wed, Sep 20, 2023 at 04:24:26PM +0200, Thomas Zimmermann wrote:
> DRM's format-conversion helpers require temporary memory. Pass the
> buffer from the caller and keep it allocated over several calls. Allow
> the caller to preallocate the buffer memory.
> 
> The motivation for this patchset is the recent work on a DRM panic
> handler. The panic handler requires format conversion to display an
> error to the screen. But allocating memory during kernel panics is
> fragile. The changes in this patchset enable the DRM panic handler to
> preallocate buffer storage before the panic occurs.
> 
> As an additonal benefit, drivers can now keep the temporary storage
> across multiple display updates. Avoiding memory allocation reduces
> the CPU overhead of the format helpers.

This argument is getting a bit tiring. The main reason is that it isn't
one, and:

  - we allocate something in the 10-20 range objects at a given commit,
    so another small one is not insignificant.

  - If it was, it would be trivial to produce a benchmark, but no-one
    ever actually showed a workload and numbers where there's actually
    any difference.

  - Also, the CPU overhead is indeed (even if marginally) decreased, but
    the memory overhead is increased. I don't know whether that's a good
    trade-off or not, see the point above.

It really sounds like an empty statement to me: "But just think of the
CPU!".

That being said, I also have more fundamental doubts about this series.

The first one is that storing the buffer pointer in the device instead
of the state makes it harder to reason about. When you have a state, the
framework provides the guarantee at commit time that there's only going
to be one at a given time. And since the buffer is stored in that state
object, you can't access it by mistake. The buffer size also depends on
the state, so that all makes sense from a logical PoV.

If we store the buffer into the device, then suddenly you have to think
about whether there's multiple CRTCs or not (since commits aren't
serialised if they affect different CRTCs), whether the buffer size you
allocated is large enough now for your current resolution and format,
etc. It adds a decent chunk of complexity on something that was quite
complex already.

I understand that the motivation is for drm_panic to have a buffer ready
when it has to kick in. But why don't we just allocate (and check) a
spare commit at probe time so we just have to commit it when we panic.

That would fall nicely into the rest of the atomic modesetting code, and
since we pretty much require not to allocate anything during
atomic_commit, we have that constraints already figured out.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 0/5] drm: Reuse temporary memory for format conversion

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 3894 bytes --]

Hi

Am 29.09.23 um 14:11 schrieb Maxime Ripard:
> On Wed, Sep 20, 2023 at 04:24:26PM +0200, Thomas Zimmermann wrote:
>> DRM's format-conversion helpers require temporary memory. Pass the
>> buffer from the caller and keep it allocated over several calls. Allow
>> the caller to preallocate the buffer memory.
>>
>> The motivation for this patchset is the recent work on a DRM panic
>> handler. The panic handler requires format conversion to display an
>> error to the screen. But allocating memory during kernel panics is
>> fragile. The changes in this patchset enable the DRM panic handler to
>> preallocate buffer storage before the panic occurs.
>>
>> As an additonal benefit, drivers can now keep the temporary storage
>> across multiple display updates. Avoiding memory allocation reduces
>> the CPU overhead of the format helpers.
> 
> This argument is getting a bit tiring. The main reason is that it isn't
> one, and:

CPU overhead isn't the driver behind this patchset, but if it is 
affected, why not say that in the commit message? There's a alloc/free 
pair for each updated scanline. For a full-screen updates, that's quite 
a bit.

> 
>    - we allocate something in the 10-20 range objects at a given commit,
>      so another small one is not insignificant.
> 
>    - If it was, it would be trivial to produce a benchmark, but no-one
>      ever actually showed a workload and numbers where there's actually
>      any difference.
> 
>    - Also, the CPU overhead is indeed (even if marginally) decreased, but
>      the memory overhead is increased. I don't know whether that's a good
>      trade-off or not, see the point above.
> 
> It really sounds like an empty statement to me: "But just think of the
> CPU!".
> 
> That being said, I also have more fundamental doubts about this series.
> 
> The first one is that storing the buffer pointer in the device instead
> of the state makes it harder to reason about. When you have a state, the
> framework provides the guarantee at commit time that there's only going
> to be one at a given time. And since the buffer is stored in that state
> object, you can't access it by mistake. The buffer size also depends on
> the state, so that all makes sense from a logical PoV.

Yes. I discussed this with Javier already. Putting this into the state 
is the clean solution.

> 
> If we store the buffer into the device, then suddenly you have to think
> about whether there's multiple CRTCs or not (since commits aren't
> serialised if they affect different CRTCs), whether the buffer size you
> allocated is large enough now for your current resolution and format,
> etc. It adds a decent chunk of complexity on something that was quite
> complex already.

It's in the device because it's good enough for these simple drivers. 
The next best place would be a dedicated plane structure in each driver. 
The per-plane cache would then be clearly attributed to a single plane.

> 
> I understand that the motivation is for drm_panic to have a buffer ready
> when it has to kick in. But why don't we just allocate (and check) a
> spare commit at probe time so we just have to commit it when we panic.

DRM panic doesn't commit anything. It picks up whatever the current 
scanout buffer is and draws into that. If the DRM driver cannot provide 
a scanout buffer, DRM panic does nothing.

Best regards
Thomas

> 
> That would fall nicely into the rest of the atomic modesetting code, and
> since we pretty much require not to allocate anything during
> atomic_commit, we have that constraints already figured out.
> 
> Maxime

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH v2 0/5] drm: Reuse temporary memory for format conversion

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 4354 bytes --]

On Fri, Sep 29, 2023 at 04:58:30PM +0200, Thomas Zimmermann wrote:
> Am 29.09.23 um 14:11 schrieb Maxime Ripard:
> > On Wed, Sep 20, 2023 at 04:24:26PM +0200, Thomas Zimmermann wrote:
> > > DRM's format-conversion helpers require temporary memory. Pass the
> > > buffer from the caller and keep it allocated over several calls. Allow
> > > the caller to preallocate the buffer memory.
> > > 
> > > The motivation for this patchset is the recent work on a DRM panic
> > > handler. The panic handler requires format conversion to display an
> > > error to the screen. But allocating memory during kernel panics is
> > > fragile. The changes in this patchset enable the DRM panic handler to
> > > preallocate buffer storage before the panic occurs.
> > > 
> > > As an additonal benefit, drivers can now keep the temporary storage
> > > across multiple display updates. Avoiding memory allocation reduces
> > > the CPU overhead of the format helpers.
> > 
> > This argument is getting a bit tiring. The main reason is that it isn't
> > one, and:
> 
> CPU overhead isn't the driver behind this patchset, but if it is affected,
> why not say that in the commit message?

Any patch affects the CPU overhead then, one way or the other.

> There's a alloc/free pair for each updated scanline. For a full-screen
> updates, that's quite a bit.
>
> > 
> >    - we allocate something in the 10-20 range objects at a given commit,
> >      so another small one is not insignificant.
> > 
> >    - If it was, it would be trivial to produce a benchmark, but no-one
> >      ever actually showed a workload and numbers where there's actually
> >      any difference.
> > 
> >    - Also, the CPU overhead is indeed (even if marginally) decreased, but
> >      the memory overhead is increased. I don't know whether that's a good
> >      trade-off or not, see the point above.
> > 
> > It really sounds like an empty statement to me: "But just think of the
> > CPU!".
> > 
> > That being said, I also have more fundamental doubts about this series.
> > 
> > The first one is that storing the buffer pointer in the device instead
> > of the state makes it harder to reason about. When you have a state, the
> > framework provides the guarantee at commit time that there's only going
> > to be one at a given time. And since the buffer is stored in that state
> > object, you can't access it by mistake. The buffer size also depends on
> > the state, so that all makes sense from a logical PoV.
> 
> Yes. I discussed this with Javier already. Putting this into the state is
> the clean solution.
> 
> > 
> > If we store the buffer into the device, then suddenly you have to think
> > about whether there's multiple CRTCs or not (since commits aren't
> > serialised if they affect different CRTCs), whether the buffer size you
> > allocated is large enough now for your current resolution and format,
> > etc. It adds a decent chunk of complexity on something that was quite
> > complex already.
> 
> It's in the device because it's good enough for these simple drivers. The
> next best place would be a dedicated plane structure in each driver. The
> per-plane cache would then be clearly attributed to a single plane.

Right, but you still need to think about all that before figuring it
out. And we now have simple drivers very likely to be used as an example
unsafe-but-actually-aren't. When copied and pasted into a different
context, it might not be safe anymore. And since those copy/pasting that
code are super experienced, they won't know about it.

> > I understand that the motivation is for drm_panic to have a buffer ready
> > when it has to kick in. But why don't we just allocate (and check) a
> > spare commit at probe time so we just have to commit it when we panic.
> 
> DRM panic doesn't commit anything. It picks up whatever the current scanout
> buffer is and draws into that. If the DRM driver cannot provide a scanout
> buffer, DRM panic does nothing.

My point is that it could do a commit if we have prepared everything. Or
we could steal the current buffer. Or we could pre-allocate a dumb
buffer for every device. You'll have to interact the state anyway for
any proper driver, and I don't think just allocating it here like you do
is safe and enough.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 1/5] drm/format-helper: Add struct drm_xfrm_buf to cache format conversion

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 2446 bytes --]

Hi Javier

Am 29.09.23 um 10:27 schrieb Javier Martinez Canillas:
> Thomas Zimmermann <tzimmermann@suse.de> writes:
> 
>> Hold temporary memory for format conversion in an instance of struct
>> drm_xfrm_buf. Update internal helpers of DRM's format-conversion code
>> accordingly. Drivers will later be able to keep this cache across
>> display updates.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
> 
> [...]
> 
>> +int drmm_xfrm_buf_init(struct drm_device *dev, struct drm_xfrm_buf *buf)
>> +{
>> +	buf->mem = NULL;
>> +	buf->size = 0;
>> +	buf->preallocated = false;
>> +
>> +	return drmm_add_action_or_reset(dev, drm_xfrm_buf_init_release, buf);
>> +}
>> +EXPORT_SYMBOL(drmm_xfrm_buf_init);
>> +
> 
> Can we find a better name than xfrm? I know that this is what's used in
> the internal drm_format_helper.c helpers but if we are exposing this to
> drivers, then I think that the naming is not self explanatory.
> 
>> +/**
>> + * drm_xfrm_buf_reserve - Allocates storage in an xfrm buffer
>> + * @buf: The xfrm buffer
> 
> At least in the kernel-doc we can say "The buffer used for pixel format
> conversion" or something along those lines.
> 
> [...]
> 
>> +/**
>> + * struct drm_xfrm_buf - Stores transformation and conversion state
>> + *
>> + * DRM helpers for format conversion store temporary state in
>> + * struct drm_xfrm_buf. The buffer's resources can be reused
> 
> And same here. Maybe struct drm_fmt_conversion_buf ?

I find this name to be unpleasant to read. Can we use 
drm_format_conv_state or drm_fmtcnv_state?

In the discussion about the panic handler, I mentioned that the struct 
can be used to store more inforamtion, such as palette entries or fg/bg 
colors. That would enable support for converting indexed formats, hence 
the _state postfix.

In the longer term, I'd also like to replace the drm_framebuffer from 
the API and then rename the functions to something like 
drm_fmtcnv_<x>_to_<y>(). The framebuffer really doesn't make much sense 
any longer.

Best regards
Thomas

> 
> Other than this nit, the patch looks good to me.
> 
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH v2 1/5] drm/format-helper: Add struct drm_xfrm_buf to cache format conversion

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann@suse.de> writes:

Hello Thomas,

> Hi Javier
>
> Am 29.09.23 um 10:27 schrieb Javier Martinez Canillas:

[...]

>>> +/**
>>> + * struct drm_xfrm_buf - Stores transformation and conversion state
>>> + *
>>> + * DRM helpers for format conversion store temporary state in
>>> + * struct drm_xfrm_buf. The buffer's resources can be reused
>> 
>> And same here. Maybe struct drm_fmt_conversion_buf ?
>
> I find this name to be unpleasant to read. Can we use 
> drm_format_conv_state or drm_fmtcnv_state?
>

Sure, it was just a suggestion. Anything than xfrm_buf works for me :)

From your options I prefer the former, which is easier to understand.

> In the discussion about the panic handler, I mentioned that the struct 
> can be used to store more inforamtion, such as palette entries or fg/bg 
> colors. That would enable support for converting indexed formats, hence 
> the _state postfix.
>

Got it. Makes sense.

> In the longer term, I'd also like to replace the drm_framebuffer from 
> the API and then rename the functions to something like 
> drm_fmtcnv_<x>_to_<y>(). The framebuffer really doesn't make much sense 
> any longer.
>

Agreed.

> Best regards
> Thomas
>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat