[PATCH 0/9] drm/vesadrm: Support 8-bit palettes

[PATCH 0/9] drm/vesadrm: Support 8-bit palettes

[Thomas Zimmermann]

VESA provides a wide range ot 8-bit palette modes. Add support to
vesadrm. For compatibility with common userspace the driver also
provides XRGB8888 on top of the palettized output.

Patches 1 to 3 prepare screen_info and struct pixel_format for
indexed formats. This cleans up the code for other drivers as well.
The pixel-format compare functions will be helpful in various places.

Patches 4 to 6 prepare for using RGB332 as intermediate format
between XRGB8888 and C8. This requires support for format conversion
and RGB332 hardware palettes.

Patches 7 to 9 update vesadrm. Patches 7 and 8 prepare the driver
without adding new functionality. Patch 9 adds support for the color
formats C8 and XRGB888. It adjusts output color formats in the plane's
atomic_check helper as needed. Palette setup happens in atomic_flush
as usual.

Tested on VGA hardware. Besides supporting odd use cases, this feature
can also help with testing support for low-end displays. Such displays
often have similar limitations.

Thomas Zimmermann (9):
  video: pixel_format: Add compare helpers
  video: screen_info: Add pixel-format helper for linear framebuffers
  drm/sysfb: Find screen_info format with helpers
  drm/sysfb: Blit to CRTC destination format
  drm/color-mgmt: Prepare for RGB332 palettes
  drm/format-helper: Add XRGB8888-to-RGB332 to drm_fb_blit()
  drm/vesadrm: Rename vesadrm_set_gamma_lut() to vesadrm_set_color_lut()
  drm/vesadrm: Prepare color management for palette-based framebuffers
  drm/vesadrm: Support DRM_FORMAT_C8

 drivers/gpu/drm/drm_color_mgmt.c              |  32 ++++
 drivers/gpu/drm/drm_format_helper.c           |   3 +
 drivers/gpu/drm/sysfb/drm_sysfb_helper.h      |   2 +-
 drivers/gpu/drm/sysfb/drm_sysfb_modeset.c     |  29 ++--
 drivers/gpu/drm/sysfb/drm_sysfb_screen_info.c |  21 ++-
 drivers/gpu/drm/sysfb/vesadrm.c               | 153 ++++++++++++++++--
 drivers/video/screen_info_generic.c           |  55 +++++++
 include/drm/drm_color_mgmt.h                  |   1 +
 include/linux/screen_info.h                   |   2 +
 include/video/pixel_format.h                  |  61 +++++++
 10 files changed, 319 insertions(+), 40 deletions(-)

-- 
2.49.0

[PATCH 1/9] video: pixel_format: Add compare helpers

[Thomas Zimmermann]

Add helpers that compare two pixel-format descriptions against
each other.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 include/video/pixel_format.h | 58 ++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/include/video/pixel_format.h b/include/video/pixel_format.h
index b5104b2a3a13..c57019cd6ea8 100644
--- a/include/video/pixel_format.h
+++ b/include/video/pixel_format.h
@@ -38,4 +38,62 @@ struct pixel_format {
 #define PIXEL_FORMAT_XRGB2101010 \
 	{ 32, false, { .alpha = {0, 0}, .red = {20, 10}, .green = {10, 10}, .blue = {0, 10} } }
 
+#define __pixel_format_cmp_field(lhs, rhs, name) \
+	{ \
+		int ret = ((lhs)->name) - ((rhs)->name); \
+		if (ret) \
+			return ret; \
+	}
+
+#define __pixel_format_cmp_bitfield(lhs, rhs, name) \
+	{ \
+		__pixel_format_cmp_field(lhs, rhs, name.offset); \
+		__pixel_format_cmp_field(lhs, rhs, name.length); \
+	}
+
+/**
+ * pixel_format_cmp - Compares two pixel-format descriptions
+ *
+ * @lhs: a pixel-format description
+ * @rhs: a pixel-format description
+ *
+ * Compares two pixel-format descriptions for their order. The semantics
+ * are equivalent to memcmp().
+ *
+ * Returns:
+ * 0 if both arguments describe the same pixel format, less-than-zero if lhs < rhs,
+ * or greater-than-zero if lhs > rhs.
+ */
+static inline int pixel_format_cmp(const struct pixel_format *lhs, const struct pixel_format *rhs)
+{
+	__pixel_format_cmp_field(lhs, rhs, bits_per_pixel);
+	__pixel_format_cmp_field(lhs, rhs, indexed);
+
+	if (lhs->indexed) {
+		__pixel_format_cmp_bitfield(lhs, rhs, index);
+	} else {
+		__pixel_format_cmp_bitfield(lhs, rhs, alpha);
+		__pixel_format_cmp_bitfield(lhs, rhs, red);
+		__pixel_format_cmp_bitfield(lhs, rhs, green);
+		__pixel_format_cmp_bitfield(lhs, rhs, blue);
+	}
+
+	return 0;
+}
+
+/**
+ * pixel_format_equal - Compares two pixel-format descriptions for equality
+ *
+ * @lhs: a pixel-format description
+ * @rhs: a pixel-format description
+ *
+ * Returns:
+ * True if both arguments describe the same pixel format, or false otherwise.
+ */
+static inline bool pixel_format_equal(const struct pixel_format *lhs,
+				      const struct pixel_format *rhs)
+{
+	return !pixel_format_cmp(lhs, rhs);
+}
+
 #endif
-- 
2.49.0

[PATCH 2/9] video: screen_info: Add pixel-format helper for linear framebuffers

[Thomas Zimmermann]

Add screen_info_pixel_format(), which converts a screen_info's
information about the color format to struct pixel_format. The encoding
within the screen_info structure is complex and therefore prone to
errors. Later patches will convert callers to use the pixel format.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/screen_info_generic.c | 55 +++++++++++++++++++++++++++++
 include/linux/screen_info.h         |  2 ++
 2 files changed, 57 insertions(+)

diff --git a/drivers/video/screen_info_generic.c b/drivers/video/screen_info_generic.c
index 900e9386eceb..763adbba71cb 100644
--- a/drivers/video/screen_info_generic.c
+++ b/drivers/video/screen_info_generic.c
@@ -5,6 +5,8 @@
 #include <linux/screen_info.h>
 #include <linux/string.h>
 
+#include <video/pixel_format.h>
+
 static void resource_init_named(struct resource *r,
 				resource_size_t start, resource_size_t size,
 				const char *name, unsigned int flags)
@@ -180,3 +182,56 @@ u32 __screen_info_lfb_bits_per_pixel(const struct screen_info *si)
 	return bits_per_pixel;
 }
 EXPORT_SYMBOL(__screen_info_lfb_bits_per_pixel);
+
+static int __screen_info_lfb_pixel_format(const struct screen_info *si, struct pixel_format *f)
+{
+	u32 bits_per_pixel = __screen_info_lfb_bits_per_pixel(si);
+
+	if (bits_per_pixel > U8_MAX)
+		return -EINVAL;
+
+	f->bits_per_pixel = bits_per_pixel;
+
+	if (si->lfb_depth > 8) {
+		f->indexed = false;
+		f->alpha.offset = 0;
+		f->alpha.length = 0;
+		f->red.offset = si->red_pos;
+		f->red.length = si->red_size;
+		f->green.offset = si->green_pos;
+		f->green.length = si->green_size;
+		f->blue.offset = si->blue_pos;
+		f->blue.length = si->blue_size;
+	} else {
+		f->indexed = true;
+		f->index.offset = 0;
+		f->index.length = si->lfb_depth;
+	}
+
+	return 0;
+}
+
+/**
+ * screen_info_pixel_format - Returns the screen-info format as pixel-format description
+ *
+ * @si: the screen_info
+ * @f: pointer to return pixel-format description
+ *
+ * Returns:
+ * 0 on success, or a negative errno code otherwise.
+ */
+int screen_info_pixel_format(const struct screen_info *si, struct pixel_format *f)
+{
+	unsigned int type = screen_info_video_type(si);
+
+	/* TODO: Add support for additional types as needed. */
+	switch (type) {
+	case VIDEO_TYPE_VLFB:
+	case VIDEO_TYPE_EFI:
+		return __screen_info_lfb_pixel_format(si, f);
+	}
+
+	/* not supported */
+	return -EINVAL;
+}
+EXPORT_SYMBOL(screen_info_pixel_format);
diff --git a/include/linux/screen_info.h b/include/linux/screen_info.h
index 923d68e07679..1690706206e8 100644
--- a/include/linux/screen_info.h
+++ b/include/linux/screen_info.h
@@ -12,6 +12,7 @@
 #define SCREEN_INFO_MAX_RESOURCES	3
 
 struct pci_dev;
+struct pixel_format;
 struct resource;
 
 static inline bool __screen_info_has_lfb(unsigned int type)
@@ -136,6 +137,7 @@ static inline u32 __screen_info_vesapm_info_base(const struct screen_info *si)
 ssize_t screen_info_resources(const struct screen_info *si, struct resource *r, size_t num);
 
 u32 __screen_info_lfb_bits_per_pixel(const struct screen_info *si);
+int screen_info_pixel_format(const struct screen_info *si, struct pixel_format *f);
 
 #if defined(CONFIG_PCI)
 void screen_info_apply_fixups(void);
-- 
2.49.0

[PATCH 3/9] drm/sysfb: Find screen_info format with helpers

[Thomas Zimmermann]

Convert drm_sysfb_get_format_si() to lookup the screen_info color
format as struct pixel_format with screen_info_pixel_format(). Then
search the list of given formats for the screen_info format with
pixel_format_equal().

Replaces custom code with helpers. The pixel-compare helper
pixel_format_equal() also handles indexed color formats. Prepares
for sysfb drivers to support color palettes.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/sysfb/drm_sysfb_screen_info.c | 21 ++++++++-----------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/sysfb/drm_sysfb_screen_info.c b/drivers/gpu/drm/sysfb/drm_sysfb_screen_info.c
index 0b3fb874a51f..885864168c54 100644
--- a/drivers/gpu/drm/sysfb/drm_sysfb_screen_info.c
+++ b/drivers/gpu/drm/sysfb/drm_sysfb_screen_info.c
@@ -79,22 +79,19 @@ const struct drm_format_info *drm_sysfb_get_format_si(struct drm_device *dev,
 						      const struct screen_info *si)
 {
 	const struct drm_format_info *format = NULL;
-	u32 bits_per_pixel;
+	struct pixel_format pixel;
 	size_t i;
+	int ret;
 
-	bits_per_pixel = __screen_info_lfb_bits_per_pixel(si);
+	ret = screen_info_pixel_format(si, &pixel);
+	if (ret)
+		return NULL;
 
 	for (i = 0; i < nformats; ++i) {
-		const struct pixel_format *f = &formats[i].pixel;
-
-		if (bits_per_pixel == f->bits_per_pixel &&
-		    si->red_size == f->red.length &&
-		    si->red_pos == f->red.offset &&
-		    si->green_size == f->green.length &&
-		    si->green_pos == f->green.offset &&
-		    si->blue_size == f->blue.length &&
-		    si->blue_pos == f->blue.offset) {
-			format = drm_format_info(formats[i].fourcc);
+		const struct drm_sysfb_format *f = &formats[i];
+
+		if (pixel_format_equal(&pixel, &f->pixel)) {
+			format = drm_format_info(f->fourcc);
 			break;
 		}
 	}
-- 
2.49.0

[PATCH 4/9] drm/sysfb: Blit to CRTC destination format

[Thomas Zimmermann]

Use the color format stored in struct drm_sysfb_crtc_state for
color-format conversion instead of the scanout-buffer format
announced by firmware. Currently, both values are identical.

This will allow drivers to modify the CRTC's output format to a
certain extend. Specifically, vesadrm will be able to display RGB
framebuffers when the scanout buffer is of C8 format. With color-
format conversion to RGB332 and correct setup of the C8 palette,
output of XRGB8888-based buffers can be achieved.

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

diff --git a/drivers/gpu/drm/sysfb/drm_sysfb_helper.h b/drivers/gpu/drm/sysfb/drm_sysfb_helper.h
index cb08a88242cc..39704953d2d4 100644
--- a/drivers/gpu/drm/sysfb/drm_sysfb_helper.h
+++ b/drivers/gpu/drm/sysfb/drm_sysfb_helper.h
@@ -128,7 +128,7 @@ int drm_sysfb_plane_helper_get_scanout_buffer(struct drm_plane *plane,
 struct drm_sysfb_crtc_state {
 	struct drm_crtc_state base;
 
-	/* Primary-plane format; required for color mgmt. */
+	/* CRTC output color format; required for color mgmt. */
 	const struct drm_format_info *format;
 };
 
diff --git a/drivers/gpu/drm/sysfb/drm_sysfb_modeset.c b/drivers/gpu/drm/sysfb/drm_sysfb_modeset.c
index ffaa2522ab96..d79ac285ac43 100644
--- a/drivers/gpu/drm/sysfb/drm_sysfb_modeset.c
+++ b/drivers/gpu/drm/sysfb/drm_sysfb_modeset.c
@@ -72,7 +72,12 @@ int drm_sysfb_plane_helper_atomic_check(struct drm_plane *plane,
 	else if (!new_plane_state->visible)
 		return 0;
 
-	if (new_fb->format != sysfb->fb_format) {
+	new_crtc_state = drm_atomic_get_new_crtc_state(new_state, new_plane_state->crtc);
+
+	new_sysfb_crtc_state = to_drm_sysfb_crtc_state(new_crtc_state);
+	new_sysfb_crtc_state->format = sysfb->fb_format;
+
+	if (new_fb->format != new_sysfb_crtc_state->format) {
 		void *buf;
 
 		/* format conversion necessary; reserve buffer */
@@ -82,11 +87,6 @@ int drm_sysfb_plane_helper_atomic_check(struct drm_plane *plane,
 			return -ENOMEM;
 	}
 
-	new_crtc_state = drm_atomic_get_new_crtc_state(new_state, new_plane_state->crtc);
-
-	new_sysfb_crtc_state = to_drm_sysfb_crtc_state(new_crtc_state);
-	new_sysfb_crtc_state->format = new_fb->format;
-
 	return 0;
 }
 EXPORT_SYMBOL(drm_sysfb_plane_helper_atomic_check);
@@ -100,7 +100,10 @@ void drm_sysfb_plane_helper_atomic_update(struct drm_plane *plane, struct drm_at
 	struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(plane_state);
 	struct drm_framebuffer *fb = plane_state->fb;
 	unsigned int dst_pitch = sysfb->fb_pitch;
-	const struct drm_format_info *dst_format = sysfb->fb_format;
+	struct drm_crtc_state *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;
 	struct drm_atomic_helper_damage_iter iter;
 	struct drm_rect damage;
 	int ret, idx;
@@ -140,7 +143,10 @@ void drm_sysfb_plane_helper_atomic_disable(struct drm_plane *plane,
 	struct drm_plane_state *plane_state = drm_atomic_get_new_plane_state(state, plane);
 	void __iomem *dst_vmap = dst.vaddr_iomem; /* TODO: Use mapping abstraction */
 	unsigned int dst_pitch = sysfb->fb_pitch;
-	const struct drm_format_info *dst_format = sysfb->fb_format;
+	struct drm_crtc_state *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;
 	struct drm_rect dst_clip;
 	unsigned long lines, linepixels, i;
 	int idx;
@@ -232,16 +238,19 @@ EXPORT_SYMBOL(drm_sysfb_crtc_helper_atomic_check);
 
 void drm_sysfb_crtc_reset(struct drm_crtc *crtc)
 {
+	struct drm_sysfb_device *sysfb = to_drm_sysfb_device(crtc->dev);
 	struct drm_sysfb_crtc_state *sysfb_crtc_state;
 
 	if (crtc->state)
 		drm_sysfb_crtc_state_destroy(to_drm_sysfb_crtc_state(crtc->state));
 
 	sysfb_crtc_state = kzalloc(sizeof(*sysfb_crtc_state), GFP_KERNEL);
-	if (sysfb_crtc_state)
+	if (sysfb_crtc_state) {
+		sysfb_crtc_state->format = sysfb->fb_format;
 		__drm_atomic_helper_crtc_reset(crtc, &sysfb_crtc_state->base);
-	else
+	} else {
 		__drm_atomic_helper_crtc_reset(crtc, NULL);
+	}
 }
 EXPORT_SYMBOL(drm_sysfb_crtc_reset);
 
-- 
2.49.0

[PATCH 5/9] drm/color-mgmt: Prepare for RGB332 palettes

[Thomas Zimmermann]

Add helper drm_crtc_fill_palette_332(), which fills palettes with
RGB332 color data. Each color in RGB332 format serves as an index
into an 8-bit palette that stores the corresponding component-based
colors.

Vesadrm will use the new helper to emulate RGB formats on top of
framebuffers in C8 format.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_color_mgmt.c | 32 ++++++++++++++++++++++++++++++++
 include/drm/drm_color_mgmt.h     |  1 +
 2 files changed, 33 insertions(+)

diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index 37a3270bc3c2..7ef214848313 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -817,6 +817,38 @@ void drm_crtc_load_palette_8(struct drm_crtc *crtc, const struct drm_color_lut *
 }
 EXPORT_SYMBOL(drm_crtc_load_palette_8);
 
+static void fill_palette_332(struct drm_crtc *crtc, u16 r, u16 g, u16 b,
+			     drm_crtc_set_lut_func set_palette)
+{
+	unsigned int i = (r << 5) | (g << 2) | b;
+
+	r = (r << 13) | (r << 10) | (r << 7) | (r << 4) | (r << 1) | (r >> 2);
+	g = (g << 13) | (g << 10) | (g << 7) | (g << 4) | (g << 1) | (g >> 2);
+	b = (b << 14) | (b << 12) | (b << 10) | (b << 8) | (b << 6) | (b << 4) | (b << 2) | b;
+
+	set_palette(crtc, i, r, g, b);
+}
+
+/**
+ * drm_crtc_fill_palette_332 - Programs a default palette for R332-like formats
+ * @crtc: The displaying CRTC
+ * @set_palette: Callback for programming the hardware gamma LUT
+ *
+ * Programs an RGB332 palette to hardware.
+ */
+void drm_crtc_fill_palette_332(struct drm_crtc *crtc, drm_crtc_set_lut_func set_palette)
+{
+	unsigned int r, g, b;
+
+	for (r = 0; r < 8; ++r) {
+		for (g = 0; g < 8; ++g) {
+			for (b = 0; b < 4; ++b)
+				fill_palette_332(crtc, r, g, b, set_palette);
+		}
+	}
+}
+EXPORT_SYMBOL(drm_crtc_fill_palette_332);
+
 static void fill_palette_8(struct drm_crtc *crtc, unsigned int i,
 			   drm_crtc_set_lut_func set_palette)
 {
diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
index 6cb577f6dba6..eccb71ab335a 100644
--- a/include/drm/drm_color_mgmt.h
+++ b/include/drm/drm_color_mgmt.h
@@ -143,6 +143,7 @@ void drm_crtc_fill_gamma_555(struct drm_crtc *crtc, drm_crtc_set_lut_func set_ga
 void drm_crtc_load_palette_8(struct drm_crtc *crtc, const struct drm_color_lut *lut,
 			     drm_crtc_set_lut_func set_palette);
 
+void drm_crtc_fill_palette_332(struct drm_crtc *crtc, drm_crtc_set_lut_func set_palette);
 void drm_crtc_fill_palette_8(struct drm_crtc *crtc, drm_crtc_set_lut_func set_palette);
 
 #endif
-- 
2.49.0

[PATCH 6/9] drm/format-helper: Add XRGB8888-to-RGB332 to drm_fb_blit()

[Thomas Zimmermann]

List the conversion from XRGB8888-to-RGB332 in drm_fb_blit(), so that
drivers based on sysfb-helpers can use it.

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

diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
index 0c04247ef702..17c4f511a9c1 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -1135,6 +1135,9 @@ int drm_fb_blit(struct iosys_map *dst, const unsigned int *dst_pitch, uint32_t d
 		} 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;
 		}
 	}
 
-- 
2.49.0

[PATCH 7/9] drm/vesadrm: Rename vesadrm_set_gamma_lut() to vesadrm_set_color_lut()

[Thomas Zimmermann]

Rename vesadrm's gamma helpers in preparation of the upcoming support
for color palettes. Gamma correction and color palettes share the same
hardware features, but the driver's old naming only indicated support
for gamma LUTs.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/sysfb/vesadrm.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/sysfb/vesadrm.c b/drivers/gpu/drm/sysfb/vesadrm.c
index 074d2bae9db3..ea5ca8745c27 100644
--- a/drivers/gpu/drm/sysfb/vesadrm.c
+++ b/drivers/gpu/drm/sysfb/vesadrm.c
@@ -82,7 +82,7 @@ static struct vesadrm_device *to_vesadrm_device(struct drm_device *dev)
 }
 
 /*
- * Palette
+ * Color LUT
  */
 
 static void vesadrm_vga_cmap_write(struct vesadrm_device *vesa, unsigned int index,
@@ -128,7 +128,7 @@ static void vesadrm_pmi_cmap_write(struct vesadrm_device *vesa, unsigned int ind
 }
 #endif
 
-static void vesadrm_set_gamma_lut(struct drm_crtc *crtc, unsigned int index,
+static void vesadrm_set_color_lut(struct drm_crtc *crtc, unsigned int index,
 				  u16 red, u16 green, u16 blue)
 {
 	struct drm_device *dev = crtc->dev;
@@ -149,15 +149,15 @@ static void vesadrm_fill_gamma_lut(struct vesadrm_device *vesa,
 
 	switch (format->format) {
 	case DRM_FORMAT_XRGB1555:
-		drm_crtc_fill_gamma_555(crtc, vesadrm_set_gamma_lut);
+		drm_crtc_fill_gamma_555(crtc, vesadrm_set_color_lut);
 		break;
 	case DRM_FORMAT_RGB565:
-		drm_crtc_fill_gamma_565(crtc, vesadrm_set_gamma_lut);
+		drm_crtc_fill_gamma_565(crtc, vesadrm_set_color_lut);
 		break;
 	case DRM_FORMAT_RGB888:
 	case DRM_FORMAT_XRGB8888:
 	case DRM_FORMAT_BGRX8888:
-		drm_crtc_fill_gamma_888(crtc, vesadrm_set_gamma_lut);
+		drm_crtc_fill_gamma_888(crtc, vesadrm_set_color_lut);
 		break;
 	default:
 		drm_warn_once(dev, "Unsupported format %p4cc for gamma correction\n",
@@ -175,15 +175,15 @@ static void vesadrm_load_gamma_lut(struct vesadrm_device *vesa,
 
 	switch (format->format) {
 	case DRM_FORMAT_XRGB1555:
-		drm_crtc_load_gamma_555_from_888(crtc, lut, vesadrm_set_gamma_lut);
+		drm_crtc_load_gamma_555_from_888(crtc, lut, vesadrm_set_color_lut);
 		break;
 	case DRM_FORMAT_RGB565:
-		drm_crtc_load_gamma_565_from_888(crtc, lut, vesadrm_set_gamma_lut);
+		drm_crtc_load_gamma_565_from_888(crtc, lut, vesadrm_set_color_lut);
 		break;
 	case DRM_FORMAT_RGB888:
 	case DRM_FORMAT_XRGB8888:
 	case DRM_FORMAT_BGRX8888:
-		drm_crtc_load_gamma_888(crtc, lut, vesadrm_set_gamma_lut);
+		drm_crtc_load_gamma_888(crtc, lut, vesadrm_set_color_lut);
 		break;
 	default:
 		drm_warn_once(dev, "Unsupported format %p4cc for gamma correction\n",
-- 
2.49.0

[PATCH 8/9] drm/vesadrm: Prepare color management for palette-based framebuffers

[Thomas Zimmermann]

Distiguish between component-based formats and 'the rest' in vesadrm's
color management. Scanout buffers with component-based format allow
for gamma correction. Palette-based formats (i.e., 'the rest') require
palette setup.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/sysfb/vesadrm.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/sysfb/vesadrm.c b/drivers/gpu/drm/sysfb/vesadrm.c
index ea5ca8745c27..559650a00857 100644
--- a/drivers/gpu/drm/sysfb/vesadrm.c
+++ b/drivers/gpu/drm/sysfb/vesadrm.c
@@ -223,15 +223,22 @@ static void vesadrm_crtc_helper_atomic_flush(struct drm_crtc *crtc,
 	 * plane's color format.
 	 */
 	if (crtc_state->enable && crtc_state->color_mgmt_changed) {
-		if (sysfb_crtc_state->format == sysfb->fb_format) {
-			if (crtc_state->gamma_lut)
-				vesadrm_load_gamma_lut(vesa,
-						       sysfb_crtc_state->format,
-						       crtc_state->gamma_lut->data);
-			else
+		switch (sysfb->fb_format->format) {
+		/*
+		 * Component formats
+		 */
+		default:
+			if (sysfb_crtc_state->format == sysfb->fb_format) {
+				if (crtc_state->gamma_lut)
+					vesadrm_load_gamma_lut(vesa,
+							       sysfb_crtc_state->format,
+							       crtc_state->gamma_lut->data);
+				else
+					vesadrm_fill_gamma_lut(vesa, sysfb_crtc_state->format);
+			} else {
 				vesadrm_fill_gamma_lut(vesa, sysfb_crtc_state->format);
-		} else {
-			vesadrm_fill_gamma_lut(vesa, sysfb_crtc_state->format);
+			}
+			break;
 		}
 	}
 }
-- 
2.49.0

[PATCH 9/9] drm/vesadrm: Support DRM_FORMAT_C8

[Thomas Zimmermann]

Add support for DRM_FORMAT_C8 to vesadrm. The new pixel-format
description PIXEL_FORMAT_C8 describes the layout. Vesadrm's helpers
vesadrm_fill_palette_lut() and vesadrm_load_palette_lut() set the
hardware palette according to the CRTC's output format.

The driver emulates XRGB8888 by converting the source buffer to
RGB332 and using the resulting 256 colors as index into the hardware
palette. The hardware palette converts back to RGB during scanout.
This has no overhead compared to other format conversion, but allows
common userspace, such as Wayland compositors, to operate on the
display.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/sysfb/vesadrm.c | 114 +++++++++++++++++++++++++++++++-
 include/video/pixel_format.h    |   3 +
 2 files changed, 116 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/sysfb/vesadrm.c b/drivers/gpu/drm/sysfb/vesadrm.c
index 559650a00857..a1429c562fc5 100644
--- a/drivers/gpu/drm/sysfb/vesadrm.c
+++ b/drivers/gpu/drm/sysfb/vesadrm.c
@@ -46,6 +46,7 @@ static const struct drm_format_info *vesadrm_get_format_si(struct drm_device *de
 		{ PIXEL_FORMAT_RGB888, DRM_FORMAT_RGB888, },
 		{ PIXEL_FORMAT_XRGB8888, DRM_FORMAT_XRGB8888, },
 		{ PIXEL_FORMAT_XBGR8888, DRM_FORMAT_XBGR8888, },
+		{ PIXEL_FORMAT_C8, DRM_FORMAT_C8, },
 	};
 
 	return drm_sysfb_get_format_si(dev, formats, ARRAY_SIZE(formats), si);
@@ -192,6 +193,44 @@ static void vesadrm_load_gamma_lut(struct vesadrm_device *vesa,
 	}
 }
 
+static void vesadrm_fill_palette_lut(struct vesadrm_device *vesa,
+				     const struct drm_format_info *format)
+{
+	struct drm_device *dev = &vesa->sysfb.dev;
+	struct drm_crtc *crtc = &vesa->crtc;
+
+	switch (format->format) {
+	case DRM_FORMAT_C8:
+		drm_crtc_fill_palette_8(crtc, vesadrm_set_color_lut);
+		break;
+	case DRM_FORMAT_RGB332:
+		drm_crtc_fill_palette_332(crtc, vesadrm_set_color_lut);
+		break;
+	default:
+		drm_warn_once(dev, "Unsupported format %p4cc for palette\n",
+			      &format->format);
+		break;
+	}
+}
+
+static void vesadrm_load_palette_lut(struct vesadrm_device *vesa,
+				     const struct drm_format_info *format,
+				     struct drm_color_lut *lut)
+{
+	struct drm_device *dev = &vesa->sysfb.dev;
+	struct drm_crtc *crtc = &vesa->crtc;
+
+	switch (format->format) {
+	case DRM_FORMAT_C8:
+		drm_crtc_load_palette_8(crtc, lut, vesadrm_set_color_lut);
+		break;
+	default:
+		drm_warn_once(dev, "Unsupported format %p4cc for gamma correction\n",
+			      &format->format);
+		break;
+	}
+}
+
 /*
  * Modesetting
  */
@@ -200,8 +239,67 @@ static const u64 vesadrm_primary_plane_format_modifiers[] = {
 	DRM_SYSFB_PLANE_FORMAT_MODIFIERS,
 };
 
+static int vesadrm_primary_plane_helper_atomic_check(struct drm_plane *plane,
+						     struct drm_atomic_state *new_state)
+{
+	struct drm_sysfb_device *sysfb = to_drm_sysfb_device(plane->dev);
+	struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(new_state, plane);
+	struct drm_framebuffer *new_fb = new_plane_state->fb;
+	struct drm_crtc_state *new_crtc_state;
+	struct drm_sysfb_crtc_state *new_sysfb_crtc_state;
+	int ret;
+
+	ret = drm_sysfb_plane_helper_atomic_check(plane, new_state);
+	if (ret)
+		return ret;
+	else if (!new_plane_state->visible)
+		return 0;
+
+	/*
+	 * Fix up format conversion for specific cases
+	 */
+
+	switch (sysfb->fb_format->format) {
+	case DRM_FORMAT_C8:
+		new_crtc_state = drm_atomic_get_new_crtc_state(new_state, new_plane_state->crtc);
+		new_sysfb_crtc_state = to_drm_sysfb_crtc_state(new_crtc_state);
+
+		switch (new_fb->format->format) {
+		case DRM_FORMAT_XRGB8888:
+			/*
+			 * Reduce XRGB8888 to RGB332. Each resulting pixel is an index
+			 * into the C8 hardware palette, which stores RGB332 colors.
+			 */
+			if (new_sysfb_crtc_state->format->format != DRM_FORMAT_RGB332) {
+				new_sysfb_crtc_state->format =
+					drm_format_info(DRM_FORMAT_RGB332);
+				new_crtc_state->color_mgmt_changed = true;
+			}
+			break;
+		case DRM_FORMAT_C8:
+			/*
+			 * Restore original output. Emulation of XRGB8888 set RBG332
+			 * output format and hardware palette. This needs to be undone
+			 * when we switch back to DRM_FORMAT_C8.
+			 */
+			if (new_sysfb_crtc_state->format->format == DRM_FORMAT_RGB332) {
+				new_sysfb_crtc_state->format = sysfb->fb_format;
+				new_crtc_state->color_mgmt_changed = true;
+			}
+			break;
+		}
+		break;
+	};
+
+	return 0;
+}
+
 static const struct drm_plane_helper_funcs vesadrm_primary_plane_helper_funcs = {
-	DRM_SYSFB_PLANE_HELPER_FUNCS,
+	DRM_GEM_SHADOW_PLANE_HELPER_FUNCS,
+	.atomic_check = vesadrm_primary_plane_helper_atomic_check,
+	.atomic_update = drm_sysfb_plane_helper_atomic_update,
+	.atomic_disable = drm_sysfb_plane_helper_atomic_disable,
+	.get_scanout_buffer = drm_sysfb_plane_helper_get_scanout_buffer,
 };
 
 static const struct drm_plane_funcs vesadrm_primary_plane_funcs = {
@@ -224,6 +322,20 @@ static void vesadrm_crtc_helper_atomic_flush(struct drm_crtc *crtc,
 	 */
 	if (crtc_state->enable && crtc_state->color_mgmt_changed) {
 		switch (sysfb->fb_format->format) {
+		/*
+		 * Index formats
+		 */
+		case DRM_FORMAT_C8:
+			if (sysfb_crtc_state->format->format == DRM_FORMAT_RGB332) {
+				vesadrm_fill_palette_lut(vesa, sysfb_crtc_state->format);
+			} else if (crtc->state->gamma_lut) {
+				vesadrm_load_palette_lut(vesa,
+							 sysfb_crtc_state->format,
+							 crtc_state->gamma_lut->data);
+			} else {
+				vesadrm_fill_palette_lut(vesa, sysfb_crtc_state->format);
+			}
+			break;
 		/*
 		 * Component formats
 		 */
diff --git a/include/video/pixel_format.h b/include/video/pixel_format.h
index c57019cd6ea8..6874754b0474 100644
--- a/include/video/pixel_format.h
+++ b/include/video/pixel_format.h
@@ -20,6 +20,9 @@ struct pixel_format {
 	};
 };
 
+#define PIXEL_FORMAT_C8 \
+	{ 8, true, { .index = {0, 8}, } }
+
 #define PIXEL_FORMAT_XRGB1555 \
 	{ 16, false, { .alpha = {0, 0}, .red = {10, 5}, .green = {5, 5}, .blue = {0, 5} } }
 
-- 
2.49.0

Re: [PATCH 1/9] video: pixel_format: Add compare helpers

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann@suse.de> writes:

Hello Thomas,

> Add helpers that compare two pixel-format descriptions against
> each other.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  include/video/pixel_format.h | 58 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 58 insertions(+)
>

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

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 2/9] video: screen_info: Add pixel-format helper for linear framebuffers

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann@suse.de> writes:

> Add screen_info_pixel_format(), which converts a screen_info's
> information about the color format to struct pixel_format. The encoding
> within the screen_info structure is complex and therefore prone to
> errors. Later patches will convert callers to use the pixel format.
>
> 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/9] drm/sysfb: Find screen_info format with helpers

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann@suse.de> writes:

> Convert drm_sysfb_get_format_si() to lookup the screen_info color
> format as struct pixel_format with screen_info_pixel_format(). Then
> search the list of given formats for the screen_info format with
> pixel_format_equal().
>
> Replaces custom code with helpers. The pixel-compare helper
> pixel_format_equal() also handles indexed color formats. Prepares
> for sysfb drivers to support color palettes.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/gpu/drm/sysfb/drm_sysfb_screen_info.c | 21 ++++++++-----------
>  1 file changed, 9 insertions(+), 12 deletions(-)
>

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

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 4/9] drm/sysfb: Blit to CRTC destination format

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann@suse.de> writes:

> Use the color format stored in struct drm_sysfb_crtc_state for
> color-format conversion instead of the scanout-buffer format
> announced by firmware. Currently, both values are identical.
>
> This will allow drivers to modify the CRTC's output format to a
> certain extend. Specifically, vesadrm will be able to display RGB
> framebuffers when the scanout buffer is of C8 format. With color-
> format conversion to RGB332 and correct setup of the C8 palette,
> output of XRGB8888-based buffers can be achieved.
>
> 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 5/9] drm/color-mgmt: Prepare for RGB332 palettes

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann@suse.de> writes:

> Add helper drm_crtc_fill_palette_332(), which fills palettes with
> RGB332 color data. Each color in RGB332 format serves as an index
> into an 8-bit palette that stores the corresponding component-based
> colors.
>
> Vesadrm will use the new helper to emulate RGB formats on top of
> framebuffers in C8 format.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/gpu/drm/drm_color_mgmt.c | 32 ++++++++++++++++++++++++++++++++
>  include/drm/drm_color_mgmt.h     |  1 +
>  2 files changed, 33 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> index 37a3270bc3c2..7ef214848313 100644
> --- a/drivers/gpu/drm/drm_color_mgmt.c
> +++ b/drivers/gpu/drm/drm_color_mgmt.c
> @@ -817,6 +817,38 @@ void drm_crtc_load_palette_8(struct drm_crtc *crtc, const struct drm_color_lut *
>  }
>  EXPORT_SYMBOL(drm_crtc_load_palette_8);
>  
> +static void fill_palette_332(struct drm_crtc *crtc, u16 r, u16 g, u16 b,
> +			     drm_crtc_set_lut_func set_palette)
> +{
> +	unsigned int i = (r << 5) | (g << 2) | b;
> +
> +	r = (r << 13) | (r << 10) | (r << 7) | (r << 4) | (r << 1) | (r >> 2);
> +	g = (g << 13) | (g << 10) | (g << 7) | (g << 4) | (g << 1) | (g >> 2);
> +	b = (b << 14) | (b << 12) | (b << 10) | (b << 8) | (b << 6) | (b << 4) | (b << 2) | b;
> +
> +	set_palette(crtc, i, r, g, b);
> +}

I think this helper can benefit of having a kernel-doc or some code
comments, e.g:

          /* Calculate the 8-bit palette index from the color components */
          unsigned int i = (r << 5) | (g << 2) | b;

          /* Expand R (3-bit) G (3-bit) and B (2-bit) values to 16-bit depth colors */
          r = (r << 13) | (r << 10) | (r << 7) | (r << 4) | (r << 1) | (r >> 2);
          g = (g << 13) | (g << 10) | (g << 7) | (g << 4) | (g << 1) | (g >> 2);
          b = (b << 14) | (b << 12) | (b << 10) | (b << 8) | (b << 6) | (b << 4) | (b << 2) | b;

          /* Call the drivers' specific callback to program the hardware LUT */
          set_palette(crtc, i, r, g, b);

It might be evident to you, but I don't think it will be for others looking
at the code later.

The code itself looks good to me, if I understood it correctly :)

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

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 6/9] drm/format-helper: Add XRGB8888-to-RGB332 to drm_fb_blit()

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann@suse.de> writes:

> List the conversion from XRGB8888-to-RGB332 in drm_fb_blit(), so that
> drivers based on sysfb-helpers can use it.
>
> 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 7/9] drm/vesadrm: Rename vesadrm_set_gamma_lut() to vesadrm_set_color_lut()

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann@suse.de> writes:

> Rename vesadrm's gamma helpers in preparation of the upcoming support
> for color palettes. Gamma correction and color palettes share the same
> hardware features, but the driver's old naming only indicated support
> for gamma LUTs.
>
> 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 8/9] drm/vesadrm: Prepare color management for palette-based framebuffers

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann@suse.de> writes:

> Distiguish between component-based formats and 'the rest' in vesadrm's
> color management. Scanout buffers with component-based format allow
> for gamma correction. Palette-based formats (i.e., 'the rest') require
> palette setup.
>
> 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 5/9] drm/color-mgmt: Prepare for RGB332 palettes

[Thomas Zimmermann]

Hi

Am 11.07.25 um 14:24 schrieb Javier Martinez Canillas:
> Thomas Zimmermann <tzimmermann@suse.de> writes:
>
>> Add helper drm_crtc_fill_palette_332(), which fills palettes with
>> RGB332 color data. Each color in RGB332 format serves as an index
>> into an 8-bit palette that stores the corresponding component-based
>> colors.
>>
>> Vesadrm will use the new helper to emulate RGB formats on top of
>> framebuffers in C8 format.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>   drivers/gpu/drm/drm_color_mgmt.c | 32 ++++++++++++++++++++++++++++++++
>>   include/drm/drm_color_mgmt.h     |  1 +
>>   2 files changed, 33 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
>> index 37a3270bc3c2..7ef214848313 100644
>> --- a/drivers/gpu/drm/drm_color_mgmt.c
>> +++ b/drivers/gpu/drm/drm_color_mgmt.c
>> @@ -817,6 +817,38 @@ void drm_crtc_load_palette_8(struct drm_crtc *crtc, const struct drm_color_lut *
>>   }
>>   EXPORT_SYMBOL(drm_crtc_load_palette_8);
>>   
>> +static void fill_palette_332(struct drm_crtc *crtc, u16 r, u16 g, u16 b,
>> +			     drm_crtc_set_lut_func set_palette)
>> +{
>> +	unsigned int i = (r << 5) | (g << 2) | b;
>> +
>> +	r = (r << 13) | (r << 10) | (r << 7) | (r << 4) | (r << 1) | (r >> 2);
>> +	g = (g << 13) | (g << 10) | (g << 7) | (g << 4) | (g << 1) | (g >> 2);
>> +	b = (b << 14) | (b << 12) | (b << 10) | (b << 8) | (b << 6) | (b << 4) | (b << 2) | b;
>> +
>> +	set_palette(crtc, i, r, g, b);
>> +}
> I think this helper can benefit of having a kernel-doc or some code
> comments, e.g:
>
>            /* Calculate the 8-bit palette index from the color components */
>            unsigned int i = (r << 5) | (g << 2) | b;
>
>            /* Expand R (3-bit) G (3-bit) and B (2-bit) values to 16-bit depth colors */
>            r = (r << 13) | (r << 10) | (r << 7) | (r << 4) | (r << 1) | (r >> 2);
>            g = (g << 13) | (g << 10) | (g << 7) | (g << 4) | (g << 1) | (g >> 2);
>            b = (b << 14) | (b << 12) | (b << 10) | (b << 8) | (b << 6) | (b << 4) | (b << 2) | b;
>
>            /* Call the drivers' specific callback to program the hardware LUT */
>            set_palette(crtc, i, r, g, b);
>
> It might be evident to you, but I don't think it will be for others looking
> at the code later.
>
> The code itself looks good to me, if I understood it correctly :)

Thanks for reviewing. Commenting this code makes sense.  I just looked 
again at the r-g-b for loop and the limits are incorrect. Needs to by 
3-3-2 instead of 8-8-4. IDK how that happened, as the screenshot from 
the test looks correct.

https://imgur.com/a/vesadrm-day4-z2XQ4wA

Anyway that is another fix here.

Best regards
Thomas

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

Re: [PATCH 9/9] drm/vesadrm: Support DRM_FORMAT_C8

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann@suse.de> writes:

> Add support for DRM_FORMAT_C8 to vesadrm. The new pixel-format
> description PIXEL_FORMAT_C8 describes the layout. Vesadrm's helpers
> vesadrm_fill_palette_lut() and vesadrm_load_palette_lut() set the
> hardware palette according to the CRTC's output format.
>
> The driver emulates XRGB8888 by converting the source buffer to
> RGB332 and using the resulting 256 colors as index into the hardware
> palette. The hardware palette converts back to RGB during scanout.
> This has no overhead compared to other format conversion, but allows
> common userspace, such as Wayland compositors, to operate on the
> display.
>
> 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 5/9] drm/color-mgmt: Prepare for RGB332 palettes

[Thomas Zimmermann]

Am 11.07.25 um 14:30 schrieb Thomas Zimmermann:
> Hi
>
> Am 11.07.25 um 14:24 schrieb Javier Martinez Canillas:
>> Thomas Zimmermann <tzimmermann@suse.de> writes:
>>
>>> Add helper drm_crtc_fill_palette_332(), which fills palettes with
>>> RGB332 color data. Each color in RGB332 format serves as an index
>>> into an 8-bit palette that stores the corresponding component-based
>>> colors.
>>>
>>> Vesadrm will use the new helper to emulate RGB formats on top of
>>> framebuffers in C8 format.
>>>
>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>> ---
>>>   drivers/gpu/drm/drm_color_mgmt.c | 32 
>>> ++++++++++++++++++++++++++++++++
>>>   include/drm/drm_color_mgmt.h     |  1 +
>>>   2 files changed, 33 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/drm_color_mgmt.c 
>>> b/drivers/gpu/drm/drm_color_mgmt.c
>>> index 37a3270bc3c2..7ef214848313 100644
>>> --- a/drivers/gpu/drm/drm_color_mgmt.c
>>> +++ b/drivers/gpu/drm/drm_color_mgmt.c
>>> @@ -817,6 +817,38 @@ void drm_crtc_load_palette_8(struct drm_crtc 
>>> *crtc, const struct drm_color_lut *
>>>   }
>>>   EXPORT_SYMBOL(drm_crtc_load_palette_8);
>>>   +static void fill_palette_332(struct drm_crtc *crtc, u16 r, u16 g, 
>>> u16 b,
>>> +                 drm_crtc_set_lut_func set_palette)
>>> +{
>>> +    unsigned int i = (r << 5) | (g << 2) | b;
>>> +
>>> +    r = (r << 13) | (r << 10) | (r << 7) | (r << 4) | (r << 1) | (r 
>>> >> 2);
>>> +    g = (g << 13) | (g << 10) | (g << 7) | (g << 4) | (g << 1) | (g 
>>> >> 2);
>>> +    b = (b << 14) | (b << 12) | (b << 10) | (b << 8) | (b << 6) | 
>>> (b << 4) | (b << 2) | b;
>>> +
>>> +    set_palette(crtc, i, r, g, b);
>>> +}
>> I think this helper can benefit of having a kernel-doc or some code
>> comments, e.g:
>>
>>            /* Calculate the 8-bit palette index from the color 
>> components */
>>            unsigned int i = (r << 5) | (g << 2) | b;
>>
>>            /* Expand R (3-bit) G (3-bit) and B (2-bit) values to 
>> 16-bit depth colors */
>>            r = (r << 13) | (r << 10) | (r << 7) | (r << 4) | (r << 1) 
>> | (r >> 2);
>>            g = (g << 13) | (g << 10) | (g << 7) | (g << 4) | (g << 1) 
>> | (g >> 2);
>>            b = (b << 14) | (b << 12) | (b << 10) | (b << 8) | (b << 
>> 6) | (b << 4) | (b << 2) | b;
>>
>>            /* Call the drivers' specific callback to program the 
>> hardware LUT */
>>            set_palette(crtc, i, r, g, b);
>>
>> It might be evident to you, but I don't think it will be for others 
>> looking
>> at the code later.
>>
>> The code itself looks good to me, if I understood it correctly :)
>
> Thanks for reviewing. Commenting this code makes sense.  I just looked 
> again at the r-g-b for loop and the limits are incorrect. Needs to by 
> 3-3-2 instead of 8-8-4. IDK how that happened, as the screenshot from 
> the test looks correct.
>
> https://imgur.com/a/vesadrm-day4-z2XQ4wA
>
> Anyway that is another fix here.

And looking at it again, 8-8-4 is actually correct. It's just 
non-obvious even to the code's author. Hence some docs/changes are needed.

>
> Best regards
> Thomas
>
>>
>> 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)