[PATCH 0/3] drm/sysfb: Integrate blit helper

[PATCH 0/3] drm/sysfb: Integrate blit helper

[Thomas Zimmermann]

The sysfb-helper library is the only remaining user of drm_fb_blit().
Merge the code into the module and remove the helper. Cleans up the
interface of the format-helper library. Also improves error detection
and performance of the sysfb code.

Tested on vesadrm with format conversion.

Thomas Zimmermann (3):
  drm/sysfb: Add custom plane state
  drm/sysfb: Lookup blit function during atomic check
  drm/format-helper: Remove drm_fb_blit()

 drivers/gpu/drm/drm_format_helper.c       |  91 -------------
 drivers/gpu/drm/sysfb/drm_sysfb_helper.h  |  34 ++++-
 drivers/gpu/drm/sysfb/drm_sysfb_modeset.c | 152 +++++++++++++++++++++-
 include/drm/drm_format_helper.h           |   4 -
 4 files changed, 180 insertions(+), 101 deletions(-)

-- 
2.51.0

[PATCH 1/3] drm/sysfb: Add custom plane state

[Thomas Zimmermann]

The plane-state type struct drm_sysfb_plane_state will store the
helper for blitting to the scanout buffer.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/sysfb/drm_sysfb_helper.h  | 20 ++++++++-
 drivers/gpu/drm/sysfb/drm_sysfb_modeset.c | 51 ++++++++++++++++++++++-
 2 files changed, 69 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/sysfb/drm_sysfb_helper.h b/drivers/gpu/drm/sysfb/drm_sysfb_helper.h
index 89633e30ca62..875dd6594760 100644
--- a/drivers/gpu/drm/sysfb/drm_sysfb_helper.h
+++ b/drivers/gpu/drm/sysfb/drm_sysfb_helper.h
@@ -10,6 +10,7 @@
 
 #include <drm/drm_crtc.h>
 #include <drm/drm_device.h>
+#include <drm/drm_gem_atomic_helper.h>
 #include <drm/drm_modes.h>
 
 struct drm_format_info;
@@ -93,6 +94,16 @@ static inline struct drm_sysfb_device *to_drm_sysfb_device(struct drm_device *de
  * Plane
  */
 
+struct drm_sysfb_plane_state {
+	struct drm_shadow_plane_state base;
+};
+
+static inline struct drm_sysfb_plane_state *
+to_drm_sysfb_plane_state(struct drm_plane_state *base)
+{
+	return container_of(to_drm_shadow_plane_state(base), struct drm_sysfb_plane_state, base);
+}
+
 size_t drm_sysfb_build_fourcc_list(struct drm_device *dev,
 				   const u32 *native_fourccs, size_t native_nfourccs,
 				   u32 *fourccs_out, size_t nfourccs_out);
@@ -120,10 +131,17 @@ int drm_sysfb_plane_helper_get_scanout_buffer(struct drm_plane *plane,
 	.atomic_disable = drm_sysfb_plane_helper_atomic_disable, \
 	.get_scanout_buffer = drm_sysfb_plane_helper_get_scanout_buffer
 
+void drm_sysfb_plane_reset(struct drm_plane *plane);
+struct drm_plane_state *drm_sysfb_plane_atomic_duplicate_state(struct drm_plane *plane);
+void drm_sysfb_plane_atomic_destroy_state(struct drm_plane *plane,
+					  struct drm_plane_state *plane_state);
+
 #define DRM_SYSFB_PLANE_FUNCS \
+	.reset = drm_sysfb_plane_reset, \
 	.update_plane = drm_atomic_helper_update_plane, \
 	.disable_plane = drm_atomic_helper_disable_plane, \
-	DRM_GEM_SHADOW_PLANE_FUNCS
+	.atomic_duplicate_state = drm_sysfb_plane_atomic_duplicate_state, \
+	.atomic_destroy_state = drm_sysfb_plane_atomic_destroy_state
 
 /*
  * CRTC
diff --git a/drivers/gpu/drm/sysfb/drm_sysfb_modeset.c b/drivers/gpu/drm/sysfb/drm_sysfb_modeset.c
index ddb4a7523ee6..a602bd633f9b 100644
--- a/drivers/gpu/drm/sysfb/drm_sysfb_modeset.c
+++ b/drivers/gpu/drm/sysfb/drm_sysfb_modeset.c
@@ -11,7 +11,6 @@
 #include <drm/drm_edid.h>
 #include <drm/drm_fourcc.h>
 #include <drm/drm_framebuffer.h>
-#include <drm/drm_gem_atomic_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_panic.h>
 #include <drm/drm_print.h>
@@ -185,6 +184,13 @@ size_t drm_sysfb_build_fourcc_list(struct drm_device *dev,
 }
 EXPORT_SYMBOL(drm_sysfb_build_fourcc_list);
 
+static void drm_sysfb_plane_state_destroy(struct drm_sysfb_plane_state *sysfb_plane_state)
+{
+	__drm_gem_destroy_shadow_plane_state(&sysfb_plane_state->base);
+
+	kfree(sysfb_plane_state);
+}
+
 int drm_sysfb_plane_helper_atomic_check(struct drm_plane *plane,
 					struct drm_atomic_state *new_state)
 {
@@ -321,6 +327,49 @@ int drm_sysfb_plane_helper_get_scanout_buffer(struct drm_plane *plane,
 }
 EXPORT_SYMBOL(drm_sysfb_plane_helper_get_scanout_buffer);
 
+void drm_sysfb_plane_reset(struct drm_plane *plane)
+{
+	struct drm_sysfb_plane_state *sysfb_plane_state;
+
+	if (plane->state)
+		drm_sysfb_plane_state_destroy(to_drm_sysfb_plane_state(plane->state));
+
+	sysfb_plane_state = kzalloc(sizeof(*sysfb_plane_state), GFP_KERNEL);
+	if (sysfb_plane_state)
+		__drm_gem_reset_shadow_plane(plane, &sysfb_plane_state->base);
+	else
+		__drm_gem_reset_shadow_plane(plane, NULL);
+}
+EXPORT_SYMBOL(drm_sysfb_plane_reset);
+
+struct drm_plane_state *drm_sysfb_plane_atomic_duplicate_state(struct drm_plane *plane)
+{
+	struct drm_device *dev = plane->dev;
+	struct drm_plane_state *plane_state = plane->state;
+	struct drm_sysfb_plane_state *new_sysfb_plane_state;
+
+	if (drm_WARN_ON(dev, !plane_state))
+		return NULL;
+
+	new_sysfb_plane_state = kzalloc(sizeof(*new_sysfb_plane_state), GFP_KERNEL);
+	if (!new_sysfb_plane_state)
+		return NULL;
+
+	sysfb_plane_state = to_drm_sysfb_plane_state(plane_state);
+
+	__drm_gem_duplicate_shadow_plane_state(plane, &new_sysfb_plane_state->base);
+
+	return &new_sysfb_plane_state->base.base;
+}
+EXPORT_SYMBOL(drm_sysfb_plane_atomic_duplicate_state);
+
+void drm_sysfb_plane_atomic_destroy_state(struct drm_plane *plane,
+					  struct drm_plane_state *plane_state)
+{
+	drm_sysfb_plane_state_destroy(to_drm_sysfb_plane_state(plane_state));
+}
+EXPORT_SYMBOL(drm_sysfb_plane_atomic_destroy_state);
+
 /*
  * CRTC
  */
-- 
2.51.0

[PATCH 2/3] drm/sysfb: Lookup blit function during atomic check

[Thomas Zimmermann]

Some configurations of sysfb outputs require format conversion from
framebuffer to scanout buffer. It is a driver bug if the conversion
helper is missing, yet it might happen on odd scanout formats. The old
code, based on drm_fb_blit(), only detects this situation during the
commit's hardware update, which is too late to abort the update.

Lookup the correct blit helper as part of the check phase. Then store
it in the sysfb plane state. Allows for detection of a missing helper
before the commit ihas started. Also avoids drm_fb_blit()'s large switch
statement on each updated scanline. Only a single lookup has to be done.

The lookup is in drm_sysfb_get_blit_func(), which only tracks formats
supported by sysfb drivers.

The lookup happens in sysfb's begin_fb_access helper instead of its
atomic_check helper. This allows vesadrm, and possibly other drivers,
to implement their own atomic_check without interfering with blit
lookups. Vesadrm implements XRGB8888 on top of R8 formats with the
help of the atomic_check. Doing the blit lookup in begin_fb_access then
always uses the correct CRTC format on all drivers.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/sysfb/drm_sysfb_helper.h  |  14 ++-
 drivers/gpu/drm/sysfb/drm_sysfb_modeset.c | 101 +++++++++++++++++++++-
 2 files changed, 111 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/sysfb/drm_sysfb_helper.h b/drivers/gpu/drm/sysfb/drm_sysfb_helper.h
index 875dd6594760..da670d7eeb2e 100644
--- a/drivers/gpu/drm/sysfb/drm_sysfb_helper.h
+++ b/drivers/gpu/drm/sysfb/drm_sysfb_helper.h
@@ -17,6 +17,12 @@ struct drm_format_info;
 struct drm_scanout_buffer;
 struct screen_info;
 
+typedef void (*drm_sysfb_blit_func)(struct iosys_map *, const unsigned int *,
+				    const struct iosys_map *,
+				    const struct drm_framebuffer *,
+				    const struct drm_rect *,
+				    struct drm_format_conv_state *);
+
 /*
  * Input parsing
  */
@@ -96,6 +102,9 @@ static inline struct drm_sysfb_device *to_drm_sysfb_device(struct drm_device *de
 
 struct drm_sysfb_plane_state {
 	struct drm_shadow_plane_state base;
+
+	/* transfers framebuffer data to scanout buffer in CRTC format */
+	drm_sysfb_blit_func blit_to_crtc;
 };
 
 static inline struct drm_sysfb_plane_state *
@@ -108,6 +117,8 @@ size_t drm_sysfb_build_fourcc_list(struct drm_device *dev,
 				   const u32 *native_fourccs, size_t native_nfourccs,
 				   u32 *fourccs_out, size_t nfourccs_out);
 
+int drm_sysfb_plane_helper_begin_fb_access(struct drm_plane *plane,
+					   struct drm_plane_state *plane_state);
 int drm_sysfb_plane_helper_atomic_check(struct drm_plane *plane,
 					struct drm_atomic_state *new_state);
 void drm_sysfb_plane_helper_atomic_update(struct drm_plane *plane,
@@ -125,7 +136,8 @@ int drm_sysfb_plane_helper_get_scanout_buffer(struct drm_plane *plane,
 	DRM_FORMAT_MOD_INVALID
 
 #define DRM_SYSFB_PLANE_HELPER_FUNCS \
-	DRM_GEM_SHADOW_PLANE_HELPER_FUNCS, \
+	.begin_fb_access = drm_sysfb_plane_helper_begin_fb_access, \
+	.end_fb_access = drm_gem_end_shadow_fb_access, \
 	.atomic_check = drm_sysfb_plane_helper_atomic_check, \
 	.atomic_update = drm_sysfb_plane_helper_atomic_update, \
 	.atomic_disable = drm_sysfb_plane_helper_atomic_disable, \
diff --git a/drivers/gpu/drm/sysfb/drm_sysfb_modeset.c b/drivers/gpu/drm/sysfb/drm_sysfb_modeset.c
index a602bd633f9b..9e82b4039003 100644
--- a/drivers/gpu/drm/sysfb/drm_sysfb_modeset.c
+++ b/drivers/gpu/drm/sysfb/drm_sysfb_modeset.c
@@ -191,6 +191,97 @@ static void drm_sysfb_plane_state_destroy(struct drm_sysfb_plane_state *sysfb_pl
 	kfree(sysfb_plane_state);
 }
 
+static void drm_sysfb_memcpy(struct iosys_map *dst, const unsigned int *dst_pitch,
+			     const struct iosys_map *src, const struct drm_framebuffer *fb,
+			     const struct drm_rect *clip, struct drm_format_conv_state *state)
+{
+	drm_fb_memcpy(dst, dst_pitch, src, fb, clip);
+}
+
+static drm_sysfb_blit_func drm_sysfb_get_blit_func(u32 dst_format, u32 src_format)
+{
+	if (src_format == dst_format) {
+		return drm_sysfb_memcpy;
+	} else if (src_format == DRM_FORMAT_XRGB8888) {
+		switch (dst_format) {
+		case DRM_FORMAT_RGB565:
+			return drm_fb_xrgb8888_to_rgb565;
+		case DRM_FORMAT_RGB565 | DRM_FORMAT_BIG_ENDIAN:
+			return drm_fb_xrgb8888_to_rgb565be;
+		case DRM_FORMAT_XRGB1555:
+			return drm_fb_xrgb8888_to_xrgb1555;
+		case DRM_FORMAT_ARGB1555:
+			return drm_fb_xrgb8888_to_argb1555;
+		case DRM_FORMAT_RGBA5551:
+			return drm_fb_xrgb8888_to_rgba5551;
+		case DRM_FORMAT_RGB888:
+			return drm_fb_xrgb8888_to_rgb888;
+		case DRM_FORMAT_BGR888:
+			return drm_fb_xrgb8888_to_bgr888;
+		case DRM_FORMAT_ARGB8888:
+			return drm_fb_xrgb8888_to_argb8888;
+		case DRM_FORMAT_XBGR8888:
+			return drm_fb_xrgb8888_to_xbgr8888;
+		case DRM_FORMAT_ABGR8888:
+			return drm_fb_xrgb8888_to_abgr8888;
+		case DRM_FORMAT_XRGB2101010:
+			return drm_fb_xrgb8888_to_xrgb2101010;
+		case DRM_FORMAT_ARGB2101010:
+			return drm_fb_xrgb8888_to_argb2101010;
+		case DRM_FORMAT_BGRX8888:
+			return drm_fb_xrgb8888_to_bgrx8888;
+		case DRM_FORMAT_RGB332:
+			return drm_fb_xrgb8888_to_rgb332;
+		}
+	}
+
+	return NULL;
+}
+
+int drm_sysfb_plane_helper_begin_fb_access(struct drm_plane *plane,
+					   struct drm_plane_state *plane_state)
+{
+	struct drm_device *dev = plane->dev;
+	struct drm_sysfb_plane_state *sysfb_plane_state = to_drm_sysfb_plane_state(plane_state);
+	struct drm_framebuffer *fb = plane_state->fb;
+	struct drm_crtc_state *crtc_state;
+	struct drm_sysfb_crtc_state *sysfb_crtc_state;
+	drm_sysfb_blit_func blit_to_crtc;
+	int ret;
+
+	ret = drm_gem_begin_shadow_fb_access(plane, plane_state);
+	if (ret)
+		return ret;
+
+	if (!fb)
+		return 0;
+
+	ret = -EINVAL;
+
+	crtc_state = drm_atomic_get_crtc_state(plane_state->state, plane_state->crtc);
+	if (drm_WARN_ON_ONCE(dev, !crtc_state))
+		goto err_drm_gem_end_shadow_fb_access;
+	sysfb_crtc_state = to_drm_sysfb_crtc_state(crtc_state);
+
+	if (drm_WARN_ON_ONCE(dev, !sysfb_crtc_state->format))
+		goto err_drm_gem_end_shadow_fb_access;
+	blit_to_crtc = drm_sysfb_get_blit_func(sysfb_crtc_state->format->format,
+					       fb->format->format);
+	if (!blit_to_crtc) {
+		drm_warn_once(dev, "No blit helper from %p4cc to %p4cc found.\n",
+			      &fb->format->format, &sysfb_crtc_state->format->format);
+		goto err_drm_gem_end_shadow_fb_access;
+	}
+	sysfb_plane_state->blit_to_crtc = blit_to_crtc;
+
+	return 0;
+
+err_drm_gem_end_shadow_fb_access:
+	drm_gem_end_shadow_fb_access(plane, plane_state);
+	return ret;
+}
+EXPORT_SYMBOL(drm_sysfb_plane_helper_begin_fb_access);
+
 int drm_sysfb_plane_helper_atomic_check(struct drm_plane *plane,
 					struct drm_atomic_state *new_state)
 {
@@ -241,12 +332,14 @@ void drm_sysfb_plane_helper_atomic_update(struct drm_plane *plane, struct drm_at
 	struct drm_sysfb_device *sysfb = to_drm_sysfb_device(dev);
 	struct drm_plane_state *plane_state = drm_atomic_get_new_plane_state(state, plane);
 	struct drm_plane_state *old_plane_state = drm_atomic_get_old_plane_state(state, plane);
-	struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(plane_state);
+	struct drm_sysfb_plane_state *sysfb_plane_state = to_drm_sysfb_plane_state(plane_state);
+	struct drm_shadow_plane_state *shadow_plane_state = &sysfb_plane_state->base;
 	struct drm_framebuffer *fb = plane_state->fb;
 	unsigned int dst_pitch = sysfb->fb_pitch;
 	struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, plane_state->crtc);
 	struct drm_sysfb_crtc_state *sysfb_crtc_state = to_drm_sysfb_crtc_state(crtc_state);
 	const struct drm_format_info *dst_format = sysfb_crtc_state->format;
+	drm_sysfb_blit_func blit_to_crtc = sysfb_plane_state->blit_to_crtc;
 	struct drm_atomic_helper_damage_iter iter;
 	struct drm_rect damage;
 	int ret, idx;
@@ -267,8 +360,8 @@ void drm_sysfb_plane_helper_atomic_update(struct drm_plane *plane, struct drm_at
 			continue;
 
 		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, &shadow_plane_state->fmtcnv_state);
+		blit_to_crtc(&dst, &dst_pitch, shadow_plane_state->data, fb, &damage,
+			     &shadow_plane_state->fmtcnv_state);
 	}
 
 	drm_dev_exit(idx);
@@ -347,6 +440,7 @@ struct drm_plane_state *drm_sysfb_plane_atomic_duplicate_state(struct drm_plane
 	struct drm_device *dev = plane->dev;
 	struct drm_plane_state *plane_state = plane->state;
 	struct drm_sysfb_plane_state *new_sysfb_plane_state;
+	struct drm_sysfb_plane_state *sysfb_plane_state;
 
 	if (drm_WARN_ON(dev, !plane_state))
 		return NULL;
@@ -358,6 +452,7 @@ struct drm_plane_state *drm_sysfb_plane_atomic_duplicate_state(struct drm_plane
 	sysfb_plane_state = to_drm_sysfb_plane_state(plane_state);
 
 	__drm_gem_duplicate_shadow_plane_state(plane, &new_sysfb_plane_state->base);
+	new_sysfb_plane_state->blit_to_crtc = sysfb_plane_state->blit_to_crtc;
 
 	return &new_sysfb_plane_state->base.base;
 }
-- 
2.51.0

[PATCH 3/3] drm/format-helper: Remove drm_fb_blit()

[Thomas Zimmermann]

The function is unused; remove it.

Instead of relying on a general blit helper, drivers should pick a blit
function by themselves from their list of supported color formats.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_format_helper.c | 91 -----------------------------
 include/drm/drm_format_helper.h     |  4 --
 2 files changed, 95 deletions(-)

diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
index 006836554cc2..6cddf05c493b 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -1165,97 +1165,6 @@ void drm_fb_argb8888_to_argb4444(struct iosys_map *dst, const unsigned int *dst_
 }
 EXPORT_SYMBOL(drm_fb_argb8888_to_argb4444);
 
-/**
- * drm_fb_blit - Copy parts of a framebuffer to display memory
- * @dst:	Array of display-memory addresses to copy to
- * @dst_pitch: Array of numbers of bytes between the start of two consecutive scanlines
- *             within @dst; can be NULL if scanlines are stored next to each other.
- * @dst_format:	FOURCC code of the display's color format
- * @src:	The framebuffer memory to copy from
- * @fb:		The framebuffer to copy from
- * @clip:	Clip rectangle area to copy
- * @state: 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
- * will attempt to convert between them during the process. The parameters @dst,
- * @dst_pitch and @src refer to arrays. Each array must have at least as many
- * entries as there are planes in @dst_format's format. Each entry stores the
- * value for the format's respective color plane at the same index.
- *
- * This function does not apply clipping on @dst (i.e. the destination is at the
- * top-left corner).
- *
- * Returns:
- * 0 on success, or
- * -EINVAL if the color-format conversion failed, or
- * a negative error code otherwise.
- */
-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, struct drm_format_conv_state *state)
-{
-	uint32_t fb_format = fb->format->format;
-
-	if (fb_format == dst_format) {
-		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, state);
-		return 0;
-	} else if (fb_format == (dst_format & ~DRM_FORMAT_BIG_ENDIAN)) {
-		drm_fb_swab(dst, dst_pitch, src, fb, clip, false, state);
-		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, state);
-			return 0;
-		} else if (dst_format == DRM_FORMAT_XRGB1555) {
-			drm_fb_xrgb8888_to_xrgb1555(dst, dst_pitch, src, fb, clip, state);
-			return 0;
-		} else if (dst_format == DRM_FORMAT_ARGB1555) {
-			drm_fb_xrgb8888_to_argb1555(dst, dst_pitch, src, fb, clip, state);
-			return 0;
-		} else if (dst_format == DRM_FORMAT_RGBA5551) {
-			drm_fb_xrgb8888_to_rgba5551(dst, dst_pitch, src, fb, clip, state);
-			return 0;
-		} else if (dst_format == DRM_FORMAT_RGB888) {
-			drm_fb_xrgb8888_to_rgb888(dst, dst_pitch, src, fb, clip, state);
-			return 0;
-		} else if (dst_format == DRM_FORMAT_BGR888) {
-			drm_fb_xrgb8888_to_bgr888(dst, dst_pitch, src, fb, clip, state);
-			return 0;
-		} else if (dst_format == DRM_FORMAT_ARGB8888) {
-			drm_fb_xrgb8888_to_argb8888(dst, dst_pitch, src, fb, clip, state);
-			return 0;
-		} else if (dst_format == DRM_FORMAT_XBGR8888) {
-			drm_fb_xrgb8888_to_xbgr8888(dst, dst_pitch, src, fb, clip, state);
-			return 0;
-		} else if (dst_format == DRM_FORMAT_ABGR8888) {
-			drm_fb_xrgb8888_to_abgr8888(dst, dst_pitch, src, fb, clip, state);
-			return 0;
-		} else if (dst_format == DRM_FORMAT_XRGB2101010) {
-			drm_fb_xrgb8888_to_xrgb2101010(dst, dst_pitch, src, fb, clip, state);
-			return 0;
-		} else if (dst_format == DRM_FORMAT_ARGB2101010) {
-			drm_fb_xrgb8888_to_argb2101010(dst, dst_pitch, src, fb, clip, state);
-			return 0;
-		} else if (dst_format == DRM_FORMAT_BGRX8888) {
-			drm_fb_swab(dst, dst_pitch, src, fb, clip, false, state);
-			return 0;
-		} else if (dst_format == DRM_FORMAT_RGB332) {
-			drm_fb_xrgb8888_to_rgb332(dst, dst_pitch, src, fb, clip, state);
-			return 0;
-		}
-	}
-
-	drm_warn_once(fb->dev, "No conversion helper from %p4cc to %p4cc found.\n",
-		      &fb_format, &dst_format);
-
-	return -EINVAL;
-}
-EXPORT_SYMBOL(drm_fb_blit);
-
 static void drm_fb_gray8_to_gray2_line(void *dbuf, const void *sbuf, unsigned int pixels)
 {
 	u8 *dbuf8 = dbuf;
diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
index 32d57d6c5327..2b5c1aef80b0 100644
--- a/include/drm/drm_format_helper.h
+++ b/include/drm/drm_format_helper.h
@@ -128,10 +128,6 @@ void drm_fb_argb8888_to_argb4444(struct iosys_map *dst, const unsigned int *dst_
 				 const struct iosys_map *src, const struct drm_framebuffer *fb,
 				 const struct drm_rect *clip, struct drm_format_conv_state *state);
 
-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, struct drm_format_conv_state *state);
-
 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, struct drm_format_conv_state *state);
-- 
2.51.0

Re: [PATCH 0/3] drm/sysfb: Integrate blit helper

[Thomas Zimmermann]

ping for review

Am 08.09.25 um 14:19 schrieb Thomas Zimmermann:
> The sysfb-helper library is the only remaining user of drm_fb_blit().
> Merge the code into the module and remove the helper. Cleans up the
> interface of the format-helper library. Also improves error detection
> and performance of the sysfb code.
>
> Tested on vesadrm with format conversion.
>
> Thomas Zimmermann (3):
>    drm/sysfb: Add custom plane state
>    drm/sysfb: Lookup blit function during atomic check
>    drm/format-helper: Remove drm_fb_blit()
>
>   drivers/gpu/drm/drm_format_helper.c       |  91 -------------
>   drivers/gpu/drm/sysfb/drm_sysfb_helper.h  |  34 ++++-
>   drivers/gpu/drm/sysfb/drm_sysfb_modeset.c | 152 +++++++++++++++++++++-
>   include/drm/drm_format_helper.h           |   4 -
>   4 files changed, 180 insertions(+), 101 deletions(-)
>

-- 
--
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)

Re: [PATCH 1/3] drm/sysfb: Add custom plane state

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann@suse.de> writes:

Hello Thomas,

> The plane-state type struct drm_sysfb_plane_state will store the
> helper for blitting to the scanout buffer.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---

Looks good to me. I've just a minor comment, but feel free to ignore it.

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

> +
> +struct drm_plane_state *drm_sysfb_plane_atomic_duplicate_state(struct drm_plane *plane)
> +{
> +	struct drm_device *dev = plane->dev;
> +	struct drm_plane_state *plane_state = plane->state;
> +	struct drm_sysfb_plane_state *new_sysfb_plane_state;

IMO the code is easier to read if you add an additonal variable, e.g:

        struct drm_shadow_plane_state *new_shadow_plane_state;
> +
> +	if (drm_WARN_ON(dev, !plane_state))
> +		return NULL;
> +
> +	new_sysfb_plane_state = kzalloc(sizeof(*new_sysfb_plane_state), GFP_KERNEL);
> +	if (!new_sysfb_plane_state)
> +		return NULL;
> +
> +	sysfb_plane_state = to_drm_sysfb_plane_state(plane_state);
        new_shadow_plane_state = &new_sysfb_plane_state->base;

	__drm_gem_duplicate_shadow_plane_state(plane, new_shadow_plane_state);

        return &new_shadow_plane_state->base;

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 2/3] drm/sysfb: Lookup blit function during atomic check

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann@suse.de> writes:

> Some configurations of sysfb outputs require format conversion from
> framebuffer to scanout buffer. It is a driver bug if the conversion
> helper is missing, yet it might happen on odd scanout formats. The old
> code, based on drm_fb_blit(), only detects this situation during the
> commit's hardware update, which is too late to abort the update.
>
> Lookup the correct blit helper as part of the check phase. Then store
> it in the sysfb plane state. Allows for detection of a missing helper
> before the commit ihas started. Also avoids drm_fb_blit()'s large switch

"commit has"  

> statement on each updated scanline. Only a single lookup has to be done.
>
> The lookup is in drm_sysfb_get_blit_func(), which only tracks formats
> supported by sysfb drivers.
>
> The lookup happens in sysfb's begin_fb_access helper instead of its
> atomic_check helper. This allows vesadrm, and possibly other drivers,
> to implement their own atomic_check without interfering with blit
> lookups. Vesadrm implements XRGB8888 on top of R8 formats with the
> help of the atomic_check. Doing the blit lookup in begin_fb_access then
> always uses the correct CRTC format on all drivers.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---

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

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 3/3] drm/format-helper: Remove drm_fb_blit()

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann@suse.de> writes:

> The function is unused; remove it.
>
> Instead of relying on a general blit helper, drivers should pick a blit
> function by themselves from their list of supported color formats.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---

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

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat