[PATCH 0/5] drm: Provide helpers for programming gamma ramps and palettes

[PATCH 0/5] drm: Provide helpers for programming gamma ramps and palettes

[Thomas Zimmermann]

We have a number of drivers that offer simple gamma correction and
palette modes. Depending on their hardware, the drivers process the
provided data in similar ways. Unify the functionality in several
DRM color-management helpers and update the drivers. The new helpers
can load provided data or generate default data to load.

With the drivers; ast, mgag200 ofdrm and vesadrm; gamma ramps are
always 8 bit wide. For 24-bit color depth, 8-bit gamma ramps are being
loaded to hardware as provided. For lower color depths the hardware
often requires the gamma ramp to be reduced to the number of bits
per pixel component, which the new helpers can do automatically. The
exception is ast's hardware, which always uses 8-bit gamma ramps.

The default gamma ramp uses a factor of 1.0 (as has been the case in
existing the per-driver implementations). A later update could change
this to the common value of 2.2 or a system-specific value.

Helpers for palettes either load an 8-bit palette or generate a default
palette with increasing luminance. The goal for the default is to keep
the display content visible with black at index 0. A later update could
possibly load a system-specific default palette.

Thomas Zimmermann (5):
  drm: Add helpers for programming hardware gamma LUTs
  drm/ast: Use helpers for programming gamma ramps and palettes
  drm/mgag200: Use helpers for programming gamma ramps
  drm/ofdrm: Use helpers for programming gamma ramps
  drm/vesadrm: Use helpers for programming gamma ramps

 drivers/gpu/drm/ast/ast_mode.c           |  69 +++++---
 drivers/gpu/drm/drm_color_mgmt.c         | 206 +++++++++++++++++++++++
 drivers/gpu/drm/mgag200/mgag200_drv.h    |   4 +-
 drivers/gpu/drm/mgag200/mgag200_g200er.c |   4 +-
 drivers/gpu/drm/mgag200/mgag200_g200ev.c |   4 +-
 drivers/gpu/drm/mgag200/mgag200_g200se.c |   4 +-
 drivers/gpu/drm/mgag200/mgag200_mode.c   |  78 ++++-----
 drivers/gpu/drm/sysfb/ofdrm.c            |  78 ++++-----
 drivers/gpu/drm/sysfb/vesadrm.c          | 100 ++++-------
 include/drm/drm_color_mgmt.h             |  27 +++
 10 files changed, 380 insertions(+), 194 deletions(-)


base-commit: 842c3c276c106040f9b96d72b9df35ed6aed9ae9
-- 
2.49.0

[PATCH 1/5] drm: Add helpers for programming hardware gamma LUTs

[Thomas Zimmermann]

Provide helpers that program hardware gamma LUTs. Tha gamma ramp is
either provided by the driver or generated by the helper.

The DRM driver exports the GAMMA_LUT property with a fixed number of
entries per color component, such as 256 on 8-bit-wide components. The
entries describe the gamma ramp of each individual component. The new
helper drm_crtc_load_gamma_888() loads such gamma ramp to hardware. The
hardware uses each displayed pixel's individial components as indeces
into the hardware gamma table.

For color modes with less than 8 bits per color component, the helpers
drm_crtc_load_gamma_565_from() and drm_crtc_load_gamma_555_from_888()
interpolate the provided gamma ramp to reduce it to the correct number
of entries; 5/6/5 for RGB565-like formats and 5/5/5 for RGB1555-like
formats.

If no gamma ramp has been provided, drivers can use the new helper
drm_crtc_fill_gamma_888() to load a default gamma ramp with 256 entries
per color component. For color modes with less bits, the new helpers
drm_crtc_fill_gamma_565() and drm_crtc_fill_gamma_555() are available.
The default gamma ramp uses a gamma factor of 1. Later patches can
change this. For PCs, a gamma factor of 2.2 is common.

For color modes with palette, drm_crtc_load_palette_8() load an 8-bit
palette into the hardware. If no palette has been specified,
drm_crtc_fill_palette_8() load a system-specific default palette. This
is currently only a grey-scale palette with increasing luminance, but
later patches can change this. For PCs, a VGA default palette could
be used.

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

diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index 3969dc548cff..dd3e06605180 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -630,3 +630,209 @@ int drm_color_lut_check(const struct drm_property_blob *lut, u32 tests)
 	return 0;
 }
 EXPORT_SYMBOL(drm_color_lut_check);
+
+/*
+ * Gamma-LUT programming
+ */
+
+/**
+ * drm_crtc_load_gamma_888 - Programs gamma ramp for RGB888-like formats
+ * @crtc: The displaying CRTC
+ * @lut: The gamma ramp to program
+ * @set_gamma: Callback for programming the hardware gamma LUT
+ *
+ * Programs the gamma ramp specified in @lut to hardware. The input gamma
+ * ramp must have 256 entries per color component.
+ */
+void drm_crtc_load_gamma_888(struct drm_crtc *crtc, const struct drm_color_lut *lut,
+			     drm_crtc_set_lut_func set_gamma)
+{
+	unsigned int i;
+
+	for (i = 0; i < 256; ++i)
+		set_gamma(crtc, i, lut[i].red, lut[i].green, lut[i].blue);
+}
+EXPORT_SYMBOL(drm_crtc_load_gamma_888);
+
+/**
+ * drm_crtc_load_gamma_565_from_888 - Programs gamma ramp for RGB565-like formats
+ * @crtc: The displaying CRTC
+ * @lut: The gamma ramp to program
+ * @set_gamma: Callback for programming the hardware gamma LUT
+ *
+ * Programs the gamma ramp specified in @lut to hardware. The input gamma
+ * ramp must have 256 entries per color component. The helper interpolates
+ * the individual color components to reduce the number of entries to 5/6/5.
+ */
+void drm_crtc_load_gamma_565_from_888(struct drm_crtc *crtc, const struct drm_color_lut *lut,
+				      drm_crtc_set_lut_func set_gamma)
+{
+	unsigned int i;
+	u16 r, g, b;
+
+	for (i = 0; i < 32; ++i) {
+		r = lut[i * 8 + i / 4].red;
+		g = lut[i * 4 + i / 16].green;
+		b = lut[i * 8 + i / 4].blue;
+		set_gamma(crtc, i, r, g, b);
+	}
+	/* Green has one more bit, so add padding with 0 for red and blue. */
+	for (i = 32; i < 64; ++i) {
+		g = lut[i * 4 + i / 16].green;
+		set_gamma(crtc, i, 0, g, 0);
+	}
+}
+EXPORT_SYMBOL(drm_crtc_load_gamma_565_from_888);
+
+/**
+ * drm_crtc_load_gamma_555_from_888 - Programs gamma ramp for RGB555-like formats
+ * @crtc: The displaying CRTC
+ * @lut: The gamma ramp to program
+ * @set_gamma: Callback for programming the hardware gamma LUT
+ *
+ * Programs the gamma ramp specified in @lut to hardware. The input gamma
+ * ramp must have 256 entries per color component. The helper interpolates
+ * the individual color components to reduce the number of entries to 5/5/5.
+ */
+void drm_crtc_load_gamma_555_from_888(struct drm_crtc *crtc, const struct drm_color_lut *lut,
+				      drm_crtc_set_lut_func set_gamma)
+{
+	unsigned int i;
+	u16 r, g, b;
+
+	for (i = 0; i < 32; ++i) {
+		r = lut[i * 8 + i / 4].red;
+		g = lut[i * 8 + i / 4].green;
+		b = lut[i * 8 + i / 4].blue;
+		set_gamma(crtc, i, r, g, b);
+	}
+}
+EXPORT_SYMBOL(drm_crtc_load_gamma_555_from_888);
+
+static void fill_gamma_888(struct drm_crtc *crtc, unsigned int i, u16 r, u16 g, u16 b,
+			   drm_crtc_set_lut_func set_gamma)
+{
+	r = (r << 8) | r;
+	g = (g << 8) | g;
+	b = (b << 8) | b;
+
+	set_gamma(crtc, i, r, g, b);
+}
+
+/**
+ * drm_crtc_fill_gamma_888 - Programs a default gamma ramp for RGB888-like formats
+ * @crtc: The displaying CRTC
+ * @set_gamma: Callback for programming the hardware gamma LUT
+ *
+ * Programs a default gamma ramp to hardware.
+ */
+void drm_crtc_fill_gamma_888(struct drm_crtc *crtc, drm_crtc_set_lut_func set_gamma)
+{
+	unsigned int i;
+
+	for (i = 0; i < 256; ++i)
+		fill_gamma_888(crtc, i, i, i, i, set_gamma);
+}
+EXPORT_SYMBOL(drm_crtc_fill_gamma_888);
+
+static void fill_gamma_565(struct drm_crtc *crtc, unsigned int i, u16 r, u16 g, u16 b,
+			   drm_crtc_set_lut_func set_gamma)
+{
+	r = (r << 11) | (r << 6) | (r << 1) | (r >> 4);
+	g = (g << 10) | (g << 4) | (g >> 2);
+	b = (b << 11) | (b << 6) | (b << 1) | (b >> 4);
+
+	set_gamma(crtc, i, r, g, b);
+}
+
+/**
+ * drm_crtc_fill_gamma_565 - Programs a default gamma ramp for RGB565-like formats
+ * @crtc: The displaying CRTC
+ * @set_gamma: Callback for programming the hardware gamma LUT
+ *
+ * Programs a default gamma ramp to hardware.
+ */
+void drm_crtc_fill_gamma_565(struct drm_crtc *crtc, drm_crtc_set_lut_func set_gamma)
+{
+	unsigned int i;
+
+	for (i = 0; i < 32; ++i)
+		fill_gamma_565(crtc, i, i, i, i, set_gamma);
+	/* Green has one more bit, so add padding with 0 for red and blue. */
+	for (i = 32; i < 64; ++i)
+		fill_gamma_565(crtc, i, 0, i, 0, set_gamma);
+}
+EXPORT_SYMBOL(drm_crtc_fill_gamma_565);
+
+static void fill_gamma_555(struct drm_crtc *crtc, unsigned int i, u16 r, u16 g, u16 b,
+			   drm_crtc_set_lut_func set_gamma)
+{
+	r = (r << 11) | (r << 6) | (r << 1) | (r >> 4);
+	g = (g << 11) | (g << 6) | (g << 1) | (g >> 4);
+	b = (b << 11) | (b << 6) | (b << 1) | (r >> 4);
+
+	set_gamma(crtc, i, r, g, b);
+}
+
+/**
+ * drm_crtc_fill_gamma_555 - Programs a default gamma ramp for RGB555-like formats
+ * @crtc: The displaying CRTC
+ * @set_gamma: Callback for programming the hardware gamma LUT
+ *
+ * Programs a default gamma ramp to hardware.
+ */
+void drm_crtc_fill_gamma_555(struct drm_crtc *crtc, drm_crtc_set_lut_func set_gamma)
+{
+	unsigned int i;
+
+	for (i = 0; i < 32; ++i)
+		fill_gamma_555(crtc, i, i, i, i, set_gamma);
+}
+EXPORT_SYMBOL(drm_crtc_fill_gamma_555);
+
+/*
+ * Color-LUT programming
+ */
+
+/**
+ * drm_crtc_load_palette_8 - Programs palette for C8-like formats
+ * @crtc: The displaying CRTC
+ * @lut: The palette to program
+ * @set_palette: Callback for programming the hardware palette
+ *
+ * Programs the palette specified in @lut to hardware. The input palette
+ * must have 256 entries per color component.
+ */
+void drm_crtc_load_palette_8(struct drm_crtc *crtc, const struct drm_color_lut *lut,
+			     drm_crtc_set_lut_func set_palette)
+{
+	unsigned int i;
+
+	for (i = 0; i < 256; ++i)
+		set_palette(crtc, i, lut[i].red, lut[i].green, lut[i].blue);
+}
+EXPORT_SYMBOL(drm_crtc_load_palette_8);
+
+static void fill_palette_8(struct drm_crtc *crtc, unsigned int i,
+			   drm_crtc_set_lut_func set_palette)
+{
+	u16 Y = (i << 8) | i; // relative luminance
+
+	set_palette(crtc, i, Y, Y, Y);
+}
+
+/**
+ * drm_crtc_fill_palette_8 - Programs a default palette for C8-like formats
+ * @crtc: The displaying CRTC
+ * @set_palette: Callback for programming the hardware gamma LUT
+ *
+ * Programs a default palette to hardware.
+ */
+void drm_crtc_fill_palette_8(struct drm_crtc *crtc, drm_crtc_set_lut_func set_palette)
+{
+	unsigned int i;
+
+	for (i = 0; i < 256; ++i)
+		fill_palette_8(crtc, i, set_palette);
+}
+EXPORT_SYMBOL(drm_crtc_fill_palette_8);
diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
index ed81741036d7..6cb577f6dba6 100644
--- a/include/drm/drm_color_mgmt.h
+++ b/include/drm/drm_color_mgmt.h
@@ -118,4 +118,31 @@ enum drm_color_lut_tests {
 };
 
 int drm_color_lut_check(const struct drm_property_blob *lut, u32 tests);
+
+/*
+ * Gamma-LUT programming
+ */
+
+typedef void (*drm_crtc_set_lut_func)(struct drm_crtc *, unsigned int, u16, u16, u16);
+
+void drm_crtc_load_gamma_888(struct drm_crtc *crtc, const struct drm_color_lut *lut,
+			     drm_crtc_set_lut_func set_gamma);
+void drm_crtc_load_gamma_565_from_888(struct drm_crtc *crtc, const struct drm_color_lut *lut,
+				      drm_crtc_set_lut_func set_gamma);
+void drm_crtc_load_gamma_555_from_888(struct drm_crtc *crtc, const struct drm_color_lut *lut,
+				      drm_crtc_set_lut_func set_gamma);
+
+void drm_crtc_fill_gamma_888(struct drm_crtc *crtc, drm_crtc_set_lut_func set_gamma);
+void drm_crtc_fill_gamma_565(struct drm_crtc *crtc, drm_crtc_set_lut_func set_gamma);
+void drm_crtc_fill_gamma_555(struct drm_crtc *crtc, drm_crtc_set_lut_func set_gamma);
+
+/*
+ * Color-LUT programming
+ */
+
+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_8(struct drm_crtc *crtc, drm_crtc_set_lut_func set_palette);
+
 #endif
-- 
2.49.0

[PATCH 2/5] drm/ast: Use helpers for programming gamma ramps and palettes

[Thomas Zimmermann]

Replace ast's code for programming the hardware gamma/palette LUT
with DRM helpers. Either load provided data or program a default.
Set the individual entries with a callback.

Each gamma/palette value is given as 3 individual 16-bit values
for red, green and blue. The driver reduces them to 8 bit to make
them fit into hardware registers.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_mode.c | 69 +++++++++++++++++++++-------------
 1 file changed, 42 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 1de832964e92..7908087bcb5a 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -34,6 +34,7 @@
 
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
+#include <drm/drm_color_mgmt.h>
 #include <drm/drm_crtc.h>
 #include <drm/drm_damage_helper.h>
 #include <drm/drm_format_helper.h>
@@ -71,31 +72,44 @@ static unsigned long ast_fb_vram_size(struct ast_device *ast)
 	return cursor_offset - offset;
 }
 
-static inline void ast_load_palette_index(struct ast_device *ast,
-				     u8 index, u8 red, u8 green,
-				     u8 blue)
+static void ast_set_gamma_lut(struct drm_crtc *crtc, unsigned int index,
+			      u16 red, u16 green, u16 blue)
 {
-	ast_io_write8(ast, AST_IO_VGADWR, index);
+	struct drm_device *dev = crtc->dev;
+	struct ast_device *ast = to_ast_device(dev);
+	u8 i8 = index & 0xff;
+	u8 r8 = red >> 8;
+	u8 g8 = green >> 8;
+	u8 b8 = blue >> 8;
+
+	if (drm_WARN_ON_ONCE(dev, index != i8))
+		return; /* driver bug */
+
+	ast_io_write8(ast, AST_IO_VGADWR, i8);
 	ast_io_read8(ast, AST_IO_VGASRI);
-	ast_io_write8(ast, AST_IO_VGAPDR, red);
+	ast_io_write8(ast, AST_IO_VGAPDR, r8);
 	ast_io_read8(ast, AST_IO_VGASRI);
-	ast_io_write8(ast, AST_IO_VGAPDR, green);
+	ast_io_write8(ast, AST_IO_VGAPDR, g8);
 	ast_io_read8(ast, AST_IO_VGASRI);
-	ast_io_write8(ast, AST_IO_VGAPDR, blue);
+	ast_io_write8(ast, AST_IO_VGAPDR, b8);
 	ast_io_read8(ast, AST_IO_VGASRI);
 }
 
-static void ast_crtc_set_gamma_linear(struct ast_device *ast,
-				      const struct drm_format_info *format)
+static void ast_crtc_fill_gamma(struct ast_device *ast,
+				const struct drm_format_info *format)
 {
-	int i;
+	struct drm_crtc *crtc = &ast->crtc;
 
 	switch (format->format) {
-	case DRM_FORMAT_C8: /* In this case, gamma table is used as color palette */
+	case DRM_FORMAT_C8:
+		/* gamma table is used as color palette */
+		drm_crtc_fill_palette_8(crtc, ast_set_gamma_lut);
+		break;
 	case DRM_FORMAT_RGB565:
+		/* also uses 8-bit gamma ramp on low-color modes */
+		fallthrough;
 	case DRM_FORMAT_XRGB8888:
-		for (i = 0; i < AST_LUT_SIZE; i++)
-			ast_load_palette_index(ast, i, i, i, i);
+		drm_crtc_fill_gamma_888(crtc, ast_set_gamma_lut);
 		break;
 	default:
 		drm_warn_once(&ast->base, "Unsupported format %p4cc for gamma correction\n",
@@ -104,21 +118,22 @@ static void ast_crtc_set_gamma_linear(struct ast_device *ast,
 	}
 }
 
-static void ast_crtc_set_gamma(struct ast_device *ast,
-			       const struct drm_format_info *format,
-			       struct drm_color_lut *lut)
+static void ast_crtc_load_gamma(struct ast_device *ast,
+				const struct drm_format_info *format,
+				struct drm_color_lut *lut)
 {
-	int i;
+	struct drm_crtc *crtc = &ast->crtc;
 
 	switch (format->format) {
-	case DRM_FORMAT_C8: /* In this case, gamma table is used as color palette */
+	case DRM_FORMAT_C8:
+		/* gamma table is used as color palette */
+		drm_crtc_load_palette_8(crtc, lut, ast_set_gamma_lut);
+		break;
 	case DRM_FORMAT_RGB565:
+		/* also uses 8-bit gamma ramp on low-color modes */
+		fallthrough;
 	case DRM_FORMAT_XRGB8888:
-		for (i = 0; i < AST_LUT_SIZE; i++)
-			ast_load_palette_index(ast, i,
-					       lut[i].red >> 8,
-					       lut[i].green >> 8,
-					       lut[i].blue >> 8);
+		drm_crtc_load_gamma_888(crtc, lut, ast_set_gamma_lut);
 		break;
 	default:
 		drm_warn_once(&ast->base, "Unsupported format %p4cc for gamma correction\n",
@@ -811,11 +826,11 @@ ast_crtc_helper_atomic_flush(struct drm_crtc *crtc,
 	 */
 	if (crtc_state->enable && crtc_state->color_mgmt_changed) {
 		if (crtc_state->gamma_lut)
-			ast_crtc_set_gamma(ast,
-					   ast_crtc_state->format,
-					   crtc_state->gamma_lut->data);
+			ast_crtc_load_gamma(ast,
+					    ast_crtc_state->format,
+					    crtc_state->gamma_lut->data);
 		else
-			ast_crtc_set_gamma_linear(ast, ast_crtc_state->format);
+			ast_crtc_fill_gamma(ast, ast_crtc_state->format);
 	}
 }
 
-- 
2.49.0

[PATCH 3/5] drm/mgag200: Use helpers for programming gamma ramps

[Thomas Zimmermann]

Replace mgag200's code for programming the hardware gamma LUT with
DRM helpers. Either load a provided gamma ramp or program a default.
Set the individual entries with a callback.

Each gamma value is given as 3 individual 16-bit values for red,
green and blue. The driver reduces them to 8 bit to make them fit
into hardware registers.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/mgag200/mgag200_drv.h    |  4 +-
 drivers/gpu/drm/mgag200/mgag200_g200er.c |  4 +-
 drivers/gpu/drm/mgag200/mgag200_g200ev.c |  4 +-
 drivers/gpu/drm/mgag200/mgag200_g200se.c |  4 +-
 drivers/gpu/drm/mgag200/mgag200_mode.c   | 78 ++++++++++--------------
 5 files changed, 40 insertions(+), 54 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h
index 819a7e9381e3..7d481e11f9d6 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.h
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
@@ -382,8 +382,8 @@ int mgag200_primary_plane_helper_get_scanout_buffer(struct drm_plane *plane,
 	.destroy = drm_plane_cleanup, \
 	DRM_GEM_SHADOW_PLANE_FUNCS
 
-void mgag200_crtc_set_gamma_linear(struct mga_device *mdev, const struct drm_format_info *format);
-void mgag200_crtc_set_gamma(struct mga_device *mdev,
+void mgag200_crtc_fill_gamma(struct mga_device *mdev, const struct drm_format_info *format);
+void mgag200_crtc_load_gamma(struct mga_device *mdev,
 			    const struct drm_format_info *format,
 			    struct drm_color_lut *lut);
 
diff --git a/drivers/gpu/drm/mgag200/mgag200_g200er.c b/drivers/gpu/drm/mgag200/mgag200_g200er.c
index c20ed0ab50ec..23debc70dc54 100644
--- a/drivers/gpu/drm/mgag200/mgag200_g200er.c
+++ b/drivers/gpu/drm/mgag200/mgag200_g200er.c
@@ -200,9 +200,9 @@ static void mgag200_g200er_crtc_helper_atomic_enable(struct drm_crtc *crtc,
 	mgag200_g200er_reset_tagfifo(mdev);
 
 	if (crtc_state->gamma_lut)
-		mgag200_crtc_set_gamma(mdev, format, crtc_state->gamma_lut->data);
+		mgag200_crtc_load_gamma(mdev, format, crtc_state->gamma_lut->data);
 	else
-		mgag200_crtc_set_gamma_linear(mdev, format);
+		mgag200_crtc_fill_gamma(mdev, format);
 
 	mgag200_enable_display(mdev);
 }
diff --git a/drivers/gpu/drm/mgag200/mgag200_g200ev.c b/drivers/gpu/drm/mgag200/mgag200_g200ev.c
index 78be964eb97c..f8796e2b7a0f 100644
--- a/drivers/gpu/drm/mgag200/mgag200_g200ev.c
+++ b/drivers/gpu/drm/mgag200/mgag200_g200ev.c
@@ -201,9 +201,9 @@ static void mgag200_g200ev_crtc_helper_atomic_enable(struct drm_crtc *crtc,
 	mgag200_g200ev_set_hiprilvl(mdev);
 
 	if (crtc_state->gamma_lut)
-		mgag200_crtc_set_gamma(mdev, format, crtc_state->gamma_lut->data);
+		mgag200_crtc_load_gamma(mdev, format, crtc_state->gamma_lut->data);
 	else
-		mgag200_crtc_set_gamma_linear(mdev, format);
+		mgag200_crtc_fill_gamma(mdev, format);
 
 	mgag200_enable_display(mdev);
 }
diff --git a/drivers/gpu/drm/mgag200/mgag200_g200se.c b/drivers/gpu/drm/mgag200/mgag200_g200se.c
index 7a32d3b1d226..e80da12ba1fe 100644
--- a/drivers/gpu/drm/mgag200/mgag200_g200se.c
+++ b/drivers/gpu/drm/mgag200/mgag200_g200se.c
@@ -332,9 +332,9 @@ static void mgag200_g200se_crtc_helper_atomic_enable(struct drm_crtc *crtc,
 	mgag200_g200se_set_hiprilvl(mdev, adjusted_mode, format);
 
 	if (crtc_state->gamma_lut)
-		mgag200_crtc_set_gamma(mdev, format, crtc_state->gamma_lut->data);
+		mgag200_crtc_load_gamma(mdev, format, crtc_state->gamma_lut->data);
 	else
-		mgag200_crtc_set_gamma_linear(mdev, format);
+		mgag200_crtc_fill_gamma(mdev, format);
 
 	mgag200_enable_display(mdev);
 }
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 6067d08aeee3..39bfb9f4b205 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -13,6 +13,7 @@
 
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
+#include <drm/drm_color_mgmt.h>
 #include <drm/drm_damage_helper.h>
 #include <drm/drm_edid.h>
 #include <drm/drm_format_helper.h>
@@ -30,35 +31,37 @@
  * This file contains setup code for the CRTC.
  */
 
-void mgag200_crtc_set_gamma_linear(struct mga_device *mdev,
-				   const struct drm_format_info *format)
+static void mgag200_set_gamma_lut(struct drm_crtc *crtc, unsigned int index,
+				  u16 red, u16 green, u16 blue)
 {
-	int i;
+	struct drm_device *dev = crtc->dev;
+	struct mga_device *mdev = to_mga_device(dev);
+	u8 i8 = index & 0xff;
+	u8 r8 = red >> 8;
+	u8 g8 = green >> 8;
+	u8 b8 = blue >> 8;
+
+	if (drm_WARN_ON_ONCE(dev, index != i8))
+		return; /* driver bug */
+
+	WREG8(DAC_INDEX + MGA1064_INDEX, i8);
+	WREG8(DAC_INDEX + MGA1064_COL_PAL, r8);
+	WREG8(DAC_INDEX + MGA1064_COL_PAL, g8);
+	WREG8(DAC_INDEX + MGA1064_COL_PAL, b8);
+}
 
-	WREG8(DAC_INDEX + MGA1064_INDEX, 0);
+void mgag200_crtc_fill_gamma(struct mga_device *mdev,
+			     const struct drm_format_info *format)
+{
+	struct drm_crtc *crtc = &mdev->crtc;
 
 	switch (format->format) {
 	case DRM_FORMAT_RGB565:
-		/* Use better interpolation, to take 32 values from 0 to 255 */
-		for (i = 0; i < MGAG200_LUT_SIZE / 8; i++) {
-			WREG8(DAC_INDEX + MGA1064_COL_PAL, i * 8 + i / 4);
-			WREG8(DAC_INDEX + MGA1064_COL_PAL, i * 4 + i / 16);
-			WREG8(DAC_INDEX + MGA1064_COL_PAL, i * 8 + i / 4);
-		}
-		/* Green has one more bit, so add padding with 0 for red and blue. */
-		for (i = MGAG200_LUT_SIZE / 8; i < MGAG200_LUT_SIZE / 4; i++) {
-			WREG8(DAC_INDEX + MGA1064_COL_PAL, 0);
-			WREG8(DAC_INDEX + MGA1064_COL_PAL, i * 4 + i / 16);
-			WREG8(DAC_INDEX + MGA1064_COL_PAL, 0);
-		}
+		drm_crtc_fill_gamma_565(crtc, mgag200_set_gamma_lut);
 		break;
 	case DRM_FORMAT_RGB888:
 	case DRM_FORMAT_XRGB8888:
-		for (i = 0; i < MGAG200_LUT_SIZE; i++) {
-			WREG8(DAC_INDEX + MGA1064_COL_PAL, i);
-			WREG8(DAC_INDEX + MGA1064_COL_PAL, i);
-			WREG8(DAC_INDEX + MGA1064_COL_PAL, i);
-		}
+		drm_crtc_fill_gamma_888(crtc, mgag200_set_gamma_lut);
 		break;
 	default:
 		drm_warn_once(&mdev->base, "Unsupported format %p4cc for gamma correction\n",
@@ -67,36 +70,19 @@ void mgag200_crtc_set_gamma_linear(struct mga_device *mdev,
 	}
 }
 
-void mgag200_crtc_set_gamma(struct mga_device *mdev,
+void mgag200_crtc_load_gamma(struct mga_device *mdev,
 			    const struct drm_format_info *format,
 			    struct drm_color_lut *lut)
 {
-	int i;
-
-	WREG8(DAC_INDEX + MGA1064_INDEX, 0);
+	struct drm_crtc *crtc = &mdev->crtc;
 
 	switch (format->format) {
 	case DRM_FORMAT_RGB565:
-		/* Use better interpolation, to take 32 values from lut[0] to lut[255] */
-		for (i = 0; i < MGAG200_LUT_SIZE / 8; i++) {
-			WREG8(DAC_INDEX + MGA1064_COL_PAL, lut[i * 8 + i / 4].red >> 8);
-			WREG8(DAC_INDEX + MGA1064_COL_PAL, lut[i * 4 + i / 16].green >> 8);
-			WREG8(DAC_INDEX + MGA1064_COL_PAL, lut[i * 8 + i / 4].blue >> 8);
-		}
-		/* Green has one more bit, so add padding with 0 for red and blue. */
-		for (i = MGAG200_LUT_SIZE / 8; i < MGAG200_LUT_SIZE / 4; i++) {
-			WREG8(DAC_INDEX + MGA1064_COL_PAL, 0);
-			WREG8(DAC_INDEX + MGA1064_COL_PAL, lut[i * 4 + i / 16].green >> 8);
-			WREG8(DAC_INDEX + MGA1064_COL_PAL, 0);
-		}
+		drm_crtc_load_gamma_565_from_888(crtc, lut, mgag200_set_gamma_lut);
 		break;
 	case DRM_FORMAT_RGB888:
 	case DRM_FORMAT_XRGB8888:
-		for (i = 0; i < MGAG200_LUT_SIZE; i++) {
-			WREG8(DAC_INDEX + MGA1064_COL_PAL, lut[i].red >> 8);
-			WREG8(DAC_INDEX + MGA1064_COL_PAL, lut[i].green >> 8);
-			WREG8(DAC_INDEX + MGA1064_COL_PAL, lut[i].blue >> 8);
-		}
+		drm_crtc_load_gamma_888(crtc, lut, mgag200_set_gamma_lut);
 		break;
 	default:
 		drm_warn_once(&mdev->base, "Unsupported format %p4cc for gamma correction\n",
@@ -642,9 +628,9 @@ void mgag200_crtc_helper_atomic_flush(struct drm_crtc *crtc, struct drm_atomic_s
 		const struct drm_format_info *format = mgag200_crtc_state->format;
 
 		if (crtc_state->gamma_lut)
-			mgag200_crtc_set_gamma(mdev, format, crtc_state->gamma_lut->data);
+			mgag200_crtc_load_gamma(mdev, format, crtc_state->gamma_lut->data);
 		else
-			mgag200_crtc_set_gamma_linear(mdev, format);
+			mgag200_crtc_fill_gamma(mdev, format);
 	}
 }
 
@@ -665,9 +651,9 @@ void mgag200_crtc_helper_atomic_enable(struct drm_crtc *crtc, struct drm_atomic_
 		funcs->pixpllc_atomic_update(crtc, old_state);
 
 	if (crtc_state->gamma_lut)
-		mgag200_crtc_set_gamma(mdev, format, crtc_state->gamma_lut->data);
+		mgag200_crtc_load_gamma(mdev, format, crtc_state->gamma_lut->data);
 	else
-		mgag200_crtc_set_gamma_linear(mdev, format);
+		mgag200_crtc_fill_gamma(mdev, format);
 
 	mgag200_enable_display(mdev);
 }
-- 
2.49.0

[PATCH 4/5] drm/ofdrm: Use helpers for programming gamma ramps

[Thomas Zimmermann]

Replace ofdrm's code for programming the hardware gamma LUT with
DRM helpers. Either load a provided gamma ramp or program a default.
Set the individual entries with a callback.

Each gamma value is given as 3 individual 16-bit values for red,
green and blue. The driver reduces them to 8 bit to make them fit
into hardware registers.

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

diff --git a/drivers/gpu/drm/sysfb/ofdrm.c b/drivers/gpu/drm/sysfb/ofdrm.c
index fddfe8bea9f7..96fafdc8dd5e 100644
--- a/drivers/gpu/drm/sysfb/ofdrm.c
+++ b/drivers/gpu/drm/sysfb/ofdrm.c
@@ -8,6 +8,7 @@
 #include <drm/clients/drm_client_setup.h>
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_state_helper.h>
+#include <drm/drm_color_mgmt.h>
 #include <drm/drm_connector.h>
 #include <drm/drm_damage_helper.h>
 #include <drm/drm_device.h>
@@ -644,36 +645,36 @@ static void ofdrm_qemu_cmap_write(struct ofdrm_device *odev, unsigned char index
 	writeb(b, data);
 }
 
-static void ofdrm_device_set_gamma_linear(struct ofdrm_device *odev,
+static void ofdrm_set_gamma_lut(struct drm_crtc *crtc, unsigned int index,
+				u16 red, u16 green, u16 blue)
+{
+	struct drm_device *dev = crtc->dev;
+	struct ofdrm_device *odev = ofdrm_device_of_dev(dev);
+	u8 i8 = index & 0xff;
+	u8 r8 = red >> 8;
+	u8 g8 = green >> 8;
+	u8 b8 = blue >> 8;
+
+	if (drm_WARN_ON_ONCE(dev, index != i8))
+		return; /* driver bug */
+
+	odev->funcs->cmap_write(odev, i8, r8, g8, b8);
+}
+
+static void ofdrm_device_fill_gamma(struct ofdrm_device *odev,
 					  const struct drm_format_info *format)
 {
 	struct drm_device *dev = &odev->sysfb.dev;
-	int i;
+	struct drm_crtc *crtc = &odev->crtc;
 
 	switch (format->format) {
 	case DRM_FORMAT_RGB565:
 	case DRM_FORMAT_RGB565 | DRM_FORMAT_BIG_ENDIAN:
-		/* Use better interpolation, to take 32 values from 0 to 255 */
-		for (i = 0; i < OFDRM_GAMMA_LUT_SIZE / 8; i++) {
-			unsigned char r = i * 8 + i / 4;
-			unsigned char g = i * 4 + i / 16;
-			unsigned char b = i * 8 + i / 4;
-
-			odev->funcs->cmap_write(odev, i, r, g, b);
-		}
-		/* Green has one more bit, so add padding with 0 for red and blue. */
-		for (i = OFDRM_GAMMA_LUT_SIZE / 8; i < OFDRM_GAMMA_LUT_SIZE / 4; i++) {
-			unsigned char r = 0;
-			unsigned char g = i * 4 + i / 16;
-			unsigned char b = 0;
-
-			odev->funcs->cmap_write(odev, i, r, g, b);
-		}
+		drm_crtc_fill_gamma_565(crtc, ofdrm_set_gamma_lut);
 		break;
 	case DRM_FORMAT_XRGB8888:
 	case DRM_FORMAT_BGRX8888:
-		for (i = 0; i < OFDRM_GAMMA_LUT_SIZE; i++)
-			odev->funcs->cmap_write(odev, i, i, i, i);
+		drm_crtc_fill_gamma_888(crtc, ofdrm_set_gamma_lut);
 		break;
 	default:
 		drm_warn_once(dev, "Unsupported format %p4cc for gamma correction\n",
@@ -682,42 +683,21 @@ static void ofdrm_device_set_gamma_linear(struct ofdrm_device *odev,
 	}
 }
 
-static void ofdrm_device_set_gamma(struct ofdrm_device *odev,
-				   const struct drm_format_info *format,
-				   struct drm_color_lut *lut)
+static void ofdrm_device_load_gamma(struct ofdrm_device *odev,
+				    const struct drm_format_info *format,
+				    struct drm_color_lut *lut)
 {
 	struct drm_device *dev = &odev->sysfb.dev;
-	int i;
+	struct drm_crtc *crtc = &odev->crtc;
 
 	switch (format->format) {
 	case DRM_FORMAT_RGB565:
 	case DRM_FORMAT_RGB565 | DRM_FORMAT_BIG_ENDIAN:
-		/* Use better interpolation, to take 32 values from lut[0] to lut[255] */
-		for (i = 0; i < OFDRM_GAMMA_LUT_SIZE / 8; i++) {
-			unsigned char r = lut[i * 8 + i / 4].red >> 8;
-			unsigned char g = lut[i * 4 + i / 16].green >> 8;
-			unsigned char b = lut[i * 8 + i / 4].blue >> 8;
-
-			odev->funcs->cmap_write(odev, i, r, g, b);
-		}
-		/* Green has one more bit, so add padding with 0 for red and blue. */
-		for (i = OFDRM_GAMMA_LUT_SIZE / 8; i < OFDRM_GAMMA_LUT_SIZE / 4; i++) {
-			unsigned char r = 0;
-			unsigned char g = lut[i * 4 + i / 16].green >> 8;
-			unsigned char b = 0;
-
-			odev->funcs->cmap_write(odev, i, r, g, b);
-		}
+		drm_crtc_load_gamma_565_from_888(crtc, lut, ofdrm_set_gamma_lut);
 		break;
 	case DRM_FORMAT_XRGB8888:
 	case DRM_FORMAT_BGRX8888:
-		for (i = 0; i < OFDRM_GAMMA_LUT_SIZE; i++) {
-			unsigned char r = lut[i].red >> 8;
-			unsigned char g = lut[i].green >> 8;
-			unsigned char b = lut[i].blue >> 8;
-
-			odev->funcs->cmap_write(odev, i, r, g, b);
-		}
+		drm_crtc_load_gamma_888(crtc, lut, ofdrm_set_gamma_lut);
 		break;
 	default:
 		drm_warn_once(dev, "Unsupported format %p4cc for gamma correction\n",
@@ -753,9 +733,9 @@ static void ofdrm_crtc_helper_atomic_flush(struct drm_crtc *crtc, struct drm_ato
 		const struct drm_format_info *format = sysfb_crtc_state->format;
 
 		if (crtc_state->gamma_lut)
-			ofdrm_device_set_gamma(odev, format, crtc_state->gamma_lut->data);
+			ofdrm_device_load_gamma(odev, format, crtc_state->gamma_lut->data);
 		else
-			ofdrm_device_set_gamma_linear(odev, format);
+			ofdrm_device_fill_gamma(odev, format);
 	}
 }
 
-- 
2.49.0

[PATCH 5/5] drm/vesadrm: Use helpers for programming gamma ramps

[Thomas Zimmermann]

Replace vesadrm's code for programming the hardware gamma LUT with
DRM helpers. Either load a provided gamma ramp or program a default.
Set the individual entries with a callback.

Each gamma value is given as 3 individual 16-bit values for red,
green and blue. The driver reduces them to 8 bit to make them fit
into hardware registers.

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

diff --git a/drivers/gpu/drm/sysfb/vesadrm.c b/drivers/gpu/drm/sysfb/vesadrm.c
index 4d62c78e7d1e..7945544ba73e 100644
--- a/drivers/gpu/drm/sysfb/vesadrm.c
+++ b/drivers/gpu/drm/sysfb/vesadrm.c
@@ -9,6 +9,7 @@
 #include <drm/clients/drm_client_setup.h>
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_state_helper.h>
+#include <drm/drm_color_mgmt.h>
 #include <drm/drm_connector.h>
 #include <drm/drm_damage_helper.h>
 #include <drm/drm_device.h>
@@ -87,15 +88,10 @@ static struct vesadrm_device *to_vesadrm_device(struct drm_device *dev)
 static void vesadrm_vga_cmap_write(struct vesadrm_device *vesa, unsigned int index,
 				   u16 red, u16 green, u16 blue)
 {
-	u8 i8, r8, g8, b8;
-
-	if (index > 255)
-		return;
-
-	i8 = index;
-	r8 = red >> 8;
-	g8 = green >> 8;
-	b8 = blue >> 8;
+	u8 i8 = index;
+	u8 r8 = red >> 8;
+	u8 g8 = green >> 8;
+	u8 b8 = blue >> 8;
 
 	outb_p(i8, VGA_PEL_IW);
 	outb_p(r8, VGA_PEL_D);
@@ -120,9 +116,6 @@ static void vesadrm_pmi_cmap_write(struct vesadrm_device *vesa, unsigned int ind
 		0x00,
 	};
 
-	if (index > 255)
-		return;
-
 	__asm__ __volatile__ (
 		"call *(%%esi)"
 		: /* no return value */
@@ -135,43 +128,36 @@ static void vesadrm_pmi_cmap_write(struct vesadrm_device *vesa, unsigned int ind
 }
 #endif
 
-static void vesadrm_set_gamma_linear(struct vesadrm_device *vesa,
-				     const struct drm_format_info *format)
+static void vesadrm_set_gamma_lut(struct drm_crtc *crtc, unsigned int index,
+				  u16 red, u16 green, u16 blue)
+{
+	struct drm_device *dev = crtc->dev;
+	struct vesadrm_device *vesa = to_vesadrm_device(dev);
+	u8 i8 = index & 0xff;
+
+	if (drm_WARN_ON_ONCE(dev, index != i8))
+		return; /* driver bug */
+
+	vesa->cmap_write(vesa, i8, red, green, blue);
+}
+
+static void vesadrm_fill_gamma_lut(struct vesadrm_device *vesa,
+				   const struct drm_format_info *format)
 {
 	struct drm_device *dev = &vesa->sysfb.dev;
-	size_t i;
-	u16 r16, g16, b16;
+	struct drm_crtc *crtc = &vesa->crtc;
 
 	switch (format->format) {
 	case DRM_FORMAT_XRGB1555:
-		for (i = 0; i < 32; ++i) {
-			r16 = i * 8 + i / 4;
-			r16 |= (r16 << 8) | r16;
-			vesa->cmap_write(vesa, i, r16, r16, r16);
-		}
+		drm_crtc_fill_gamma_555(crtc, vesadrm_set_gamma_lut);
 		break;
 	case DRM_FORMAT_RGB565:
-		for (i = 0; i < 32; ++i) {
-			r16 = i * 8 + i / 4;
-			r16 |= (r16 << 8) | r16;
-			g16 = i * 4 + i / 16;
-			g16 |= (g16 << 8) | g16;
-			b16 = r16;
-			vesa->cmap_write(vesa, i, r16, g16, b16);
-		}
-		for (i = 32; i < 64; ++i) {
-			g16 = i * 4 + i / 16;
-			g16 |= (g16 << 8) | g16;
-			vesa->cmap_write(vesa, i, 0, g16, 0);
-		}
+		drm_crtc_fill_gamma_565(crtc, vesadrm_set_gamma_lut);
 		break;
 	case DRM_FORMAT_RGB888:
 	case DRM_FORMAT_XRGB8888:
 	case DRM_FORMAT_BGRX8888:
-		for (i = 0; i < 256; ++i) {
-			r16 = (i << 8) | i;
-			vesa->cmap_write(vesa, i, r16, r16, r16);
-		}
+		drm_crtc_fill_gamma_888(crtc, vesadrm_set_gamma_lut);
 		break;
 	default:
 		drm_warn_once(dev, "Unsupported format %p4cc for gamma correction\n",
@@ -180,38 +166,24 @@ static void vesadrm_set_gamma_linear(struct vesadrm_device *vesa,
 	}
 }
 
-static void vesadrm_set_gamma_lut(struct vesadrm_device *vesa,
-				  const struct drm_format_info *format,
-				  struct drm_color_lut *lut)
+static void vesadrm_load_gamma_lut(struct vesadrm_device *vesa,
+				   const struct drm_format_info *format,
+				   struct drm_color_lut *lut)
 {
 	struct drm_device *dev = &vesa->sysfb.dev;
-	size_t i;
-	u16 r16, g16, b16;
+	struct drm_crtc *crtc = &vesa->crtc;
 
 	switch (format->format) {
 	case DRM_FORMAT_XRGB1555:
-		for (i = 0; i < 32; ++i) {
-			r16 = lut[i * 8 + i / 4].red;
-			g16 = lut[i * 8 + i / 4].green;
-			b16 = lut[i * 8 + i / 4].blue;
-			vesa->cmap_write(vesa, i, r16, g16, b16);
-		}
+		drm_crtc_load_gamma_555_from_888(crtc, lut, vesadrm_set_gamma_lut);
 		break;
 	case DRM_FORMAT_RGB565:
-		for (i = 0; i < 32; ++i) {
-			r16 = lut[i * 8 + i / 4].red;
-			g16 = lut[i * 4 + i / 16].green;
-			b16 = lut[i * 8 + i / 4].blue;
-			vesa->cmap_write(vesa, i, r16, g16, b16);
-		}
-		for (i = 32; i < 64; ++i)
-			vesa->cmap_write(vesa, i, 0, lut[i * 4 + i / 16].green, 0);
+		drm_crtc_load_gamma_565_from_888(crtc, lut, vesadrm_set_gamma_lut);
 		break;
 	case DRM_FORMAT_RGB888:
 	case DRM_FORMAT_XRGB8888:
 	case DRM_FORMAT_BGRX8888:
-		for (i = 0; i < 256; ++i)
-			vesa->cmap_write(vesa, i, lut[i].red, lut[i].green, lut[i].blue);
+		drm_crtc_load_gamma_888(crtc, lut, vesadrm_set_gamma_lut);
 		break;
 	default:
 		drm_warn_once(dev, "Unsupported format %p4cc for gamma correction\n",
@@ -253,13 +225,13 @@ static void vesadrm_crtc_helper_atomic_flush(struct drm_crtc *crtc,
 	if (crtc_state->enable && crtc_state->color_mgmt_changed) {
 		if (sysfb_crtc_state->format == sysfb->fb_format) {
 			if (crtc_state->gamma_lut)
-				vesadrm_set_gamma_lut(vesa,
-						      sysfb_crtc_state->format,
-						      crtc_state->gamma_lut->data);
+				vesadrm_load_gamma_lut(vesa,
+						       sysfb_crtc_state->format,
+						       crtc_state->gamma_lut->data);
 			else
-				vesadrm_set_gamma_linear(vesa, sysfb_crtc_state->format);
+				vesadrm_fill_gamma_lut(vesa, sysfb_crtc_state->format);
 		} else {
-			vesadrm_set_gamma_linear(vesa, sysfb_crtc_state->format);
+			vesadrm_fill_gamma_lut(vesa, sysfb_crtc_state->format);
 		}
 	}
 }
-- 
2.49.0

Re: [PATCH 0/5] drm: Provide helpers for programming gamma ramps and palettes

[Thomas Zimmermann]

Am 09.05.25 um 10:23 schrieb Thomas Zimmermann:
> We have a number of drivers that offer simple gamma correction and
> palette modes. Depending on their hardware, the drivers process the
> provided data in similar ways. Unify the functionality in several
> DRM color-management helpers and update the drivers. The new helpers
> can load provided data or generate default data to load.
>
> With the drivers; ast, mgag200 ofdrm and vesadrm; gamma ramps are
> always 8 bit wide. For 24-bit color depth, 8-bit gamma ramps are being
> loaded to hardware as provided. For lower color depths the hardware
> often requires the gamma ramp to be reduced to the number of bits
> per pixel component, which the new helpers can do automatically. The
> exception is ast's hardware, which always uses 8-bit gamma ramps.

> The default gamma ramp uses a factor of 1.0 (as has been the case in
> existing the per-driver implementations). A later update could change
> this to the common value of 2.2 or a system-specific value.

That might not be such a great idea. See 
https://people.freedesktop.org/~cbrill/dri-log/?channel=dri-devel&highlight_names=&date=2025-05-12&show_html=true

>
> Helpers for palettes either load an 8-bit palette or generate a default
> palette with increasing luminance. The goal for the default is to keep
> the display content visible with black at index 0. A later update could
> possibly load a system-specific default palette.
>
> Thomas Zimmermann (5):
>    drm: Add helpers for programming hardware gamma LUTs
>    drm/ast: Use helpers for programming gamma ramps and palettes
>    drm/mgag200: Use helpers for programming gamma ramps
>    drm/ofdrm: Use helpers for programming gamma ramps
>    drm/vesadrm: Use helpers for programming gamma ramps
>
>   drivers/gpu/drm/ast/ast_mode.c           |  69 +++++---
>   drivers/gpu/drm/drm_color_mgmt.c         | 206 +++++++++++++++++++++++
>   drivers/gpu/drm/mgag200/mgag200_drv.h    |   4 +-
>   drivers/gpu/drm/mgag200/mgag200_g200er.c |   4 +-
>   drivers/gpu/drm/mgag200/mgag200_g200ev.c |   4 +-
>   drivers/gpu/drm/mgag200/mgag200_g200se.c |   4 +-
>   drivers/gpu/drm/mgag200/mgag200_mode.c   |  78 ++++-----
>   drivers/gpu/drm/sysfb/ofdrm.c            |  78 ++++-----
>   drivers/gpu/drm/sysfb/vesadrm.c          | 100 ++++-------
>   include/drm/drm_color_mgmt.h             |  27 +++
>   10 files changed, 380 insertions(+), 194 deletions(-)
>
>
> base-commit: 842c3c276c106040f9b96d72b9df35ed6aed9ae9

-- 
--
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/5] drm: Add helpers for programming hardware gamma LUTs

[Jocelyn Falempe]

On 09/05/2025 10:23, Thomas Zimmermann wrote:
> Provide helpers that program hardware gamma LUTs. Tha gamma ramp is
> either provided by the driver or generated by the helper.
> 
> The DRM driver exports the GAMMA_LUT property with a fixed number of
> entries per color component, such as 256 on 8-bit-wide components. The
> entries describe the gamma ramp of each individual component. The new
> helper drm_crtc_load_gamma_888() loads such gamma ramp to hardware. The
> hardware uses each displayed pixel's individial components as indeces
> into the hardware gamma table.
> 
> For color modes with less than 8 bits per color component, the helpers
> drm_crtc_load_gamma_565_from() and drm_crtc_load_gamma_555_from_888()
> interpolate the provided gamma ramp to reduce it to the correct number
> of entries; 5/6/5 for RGB565-like formats and 5/5/5 for RGB1555-like
> formats.
> 
> If no gamma ramp has been provided, drivers can use the new helper
> drm_crtc_fill_gamma_888() to load a default gamma ramp with 256 entries
> per color component. For color modes with less bits, the new helpers
> drm_crtc_fill_gamma_565() and drm_crtc_fill_gamma_555() are available.
> The default gamma ramp uses a gamma factor of 1. Later patches can
> change this. For PCs, a gamma factor of 2.2 is common.
> 
> For color modes with palette, drm_crtc_load_palette_8() load an 8-bit
> palette into the hardware. If no palette has been specified,
> drm_crtc_fill_palette_8() load a system-specific default palette. This
> is currently only a grey-scale palette with increasing luminance, but
> later patches can change this. For PCs, a VGA default palette could
> be used.

Thanks for this work. I've one comment below regarding the 
drm_crtc_set_lut_func prototype.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>   drivers/gpu/drm/drm_color_mgmt.c | 206 +++++++++++++++++++++++++++++++
>   include/drm/drm_color_mgmt.h     |  27 ++++
>   2 files changed, 233 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> index 3969dc548cff..dd3e06605180 100644
> --- a/drivers/gpu/drm/drm_color_mgmt.c
> +++ b/drivers/gpu/drm/drm_color_mgmt.c
> @@ -630,3 +630,209 @@ int drm_color_lut_check(const struct drm_property_blob *lut, u32 tests)
>   	return 0;
>   }
>   EXPORT_SYMBOL(drm_color_lut_check);
> +
> +/*
> + * Gamma-LUT programming
> + */
> +
> +/**
> + * drm_crtc_load_gamma_888 - Programs gamma ramp for RGB888-like formats
> + * @crtc: The displaying CRTC
> + * @lut: The gamma ramp to program
> + * @set_gamma: Callback for programming the hardware gamma LUT
> + *
> + * Programs the gamma ramp specified in @lut to hardware. The input gamma
> + * ramp must have 256 entries per color component.
> + */
> +void drm_crtc_load_gamma_888(struct drm_crtc *crtc, const struct drm_color_lut *lut,
> +			     drm_crtc_set_lut_func set_gamma)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < 256; ++i)
> +		set_gamma(crtc, i, lut[i].red, lut[i].green, lut[i].blue);
> +}
> +EXPORT_SYMBOL(drm_crtc_load_gamma_888);
> +
> +/**
> + * drm_crtc_load_gamma_565_from_888 - Programs gamma ramp for RGB565-like formats
> + * @crtc: The displaying CRTC
> + * @lut: The gamma ramp to program
> + * @set_gamma: Callback for programming the hardware gamma LUT
> + *
> + * Programs the gamma ramp specified in @lut to hardware. The input gamma
> + * ramp must have 256 entries per color component. The helper interpolates
> + * the individual color components to reduce the number of entries to 5/6/5.
> + */
> +void drm_crtc_load_gamma_565_from_888(struct drm_crtc *crtc, const struct drm_color_lut *lut,
> +				      drm_crtc_set_lut_func set_gamma)
> +{
> +	unsigned int i;
> +	u16 r, g, b;
> +
> +	for (i = 0; i < 32; ++i) {
> +		r = lut[i * 8 + i / 4].red;
> +		g = lut[i * 4 + i / 16].green;
> +		b = lut[i * 8 + i / 4].blue;
> +		set_gamma(crtc, i, r, g, b);
> +	}
> +	/* Green has one more bit, so add padding with 0 for red and blue. */
> +	for (i = 32; i < 64; ++i) {
> +		g = lut[i * 4 + i / 16].green;
> +		set_gamma(crtc, i, 0, g, 0);
> +	}
> +}
> +EXPORT_SYMBOL(drm_crtc_load_gamma_565_from_888);
> +
> +/**
> + * drm_crtc_load_gamma_555_from_888 - Programs gamma ramp for RGB555-like formats
> + * @crtc: The displaying CRTC
> + * @lut: The gamma ramp to program
> + * @set_gamma: Callback for programming the hardware gamma LUT
> + *
> + * Programs the gamma ramp specified in @lut to hardware. The input gamma
> + * ramp must have 256 entries per color component. The helper interpolates
> + * the individual color components to reduce the number of entries to 5/5/5.
> + */
> +void drm_crtc_load_gamma_555_from_888(struct drm_crtc *crtc, const struct drm_color_lut *lut,
> +				      drm_crtc_set_lut_func set_gamma)
> +{
> +	unsigned int i;
> +	u16 r, g, b;
> +
> +	for (i = 0; i < 32; ++i) {
> +		r = lut[i * 8 + i / 4].red;
> +		g = lut[i * 8 + i / 4].green;
> +		b = lut[i * 8 + i / 4].blue;
> +		set_gamma(crtc, i, r, g, b);
> +	}
> +}
> +EXPORT_SYMBOL(drm_crtc_load_gamma_555_from_888);
> +
> +static void fill_gamma_888(struct drm_crtc *crtc, unsigned int i, u16 r, u16 g, u16 b,
> +			   drm_crtc_set_lut_func set_gamma)
> +{
> +	r = (r << 8) | r;
> +	g = (g << 8) | g;
> +	b = (b << 8) | b;
> +
> +	set_gamma(crtc, i, r, g, b);
> +}
> +
> +/**
> + * drm_crtc_fill_gamma_888 - Programs a default gamma ramp for RGB888-like formats
> + * @crtc: The displaying CRTC
> + * @set_gamma: Callback for programming the hardware gamma LUT
> + *
> + * Programs a default gamma ramp to hardware.
> + */
> +void drm_crtc_fill_gamma_888(struct drm_crtc *crtc, drm_crtc_set_lut_func set_gamma)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < 256; ++i)
> +		fill_gamma_888(crtc, i, i, i, i, set_gamma);
> +}
> +EXPORT_SYMBOL(drm_crtc_fill_gamma_888);
> +
> +static void fill_gamma_565(struct drm_crtc *crtc, unsigned int i, u16 r, u16 g, u16 b,
> +			   drm_crtc_set_lut_func set_gamma)
> +{
> +	r = (r << 11) | (r << 6) | (r << 1) | (r >> 4);
> +	g = (g << 10) | (g << 4) | (g >> 2);
> +	b = (b << 11) | (b << 6) | (b << 1) | (b >> 4);
> +
> +	set_gamma(crtc, i, r, g, b);
> +}
> +
> +/**
> + * drm_crtc_fill_gamma_565 - Programs a default gamma ramp for RGB565-like formats
> + * @crtc: The displaying CRTC
> + * @set_gamma: Callback for programming the hardware gamma LUT
> + *
> + * Programs a default gamma ramp to hardware.
> + */
> +void drm_crtc_fill_gamma_565(struct drm_crtc *crtc, drm_crtc_set_lut_func set_gamma)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < 32; ++i)
> +		fill_gamma_565(crtc, i, i, i, i, set_gamma);
> +	/* Green has one more bit, so add padding with 0 for red and blue. */
> +	for (i = 32; i < 64; ++i)
> +		fill_gamma_565(crtc, i, 0, i, 0, set_gamma);
> +}
> +EXPORT_SYMBOL(drm_crtc_fill_gamma_565);
> +
> +static void fill_gamma_555(struct drm_crtc *crtc, unsigned int i, u16 r, u16 g, u16 b,
> +			   drm_crtc_set_lut_func set_gamma)
> +{
> +	r = (r << 11) | (r << 6) | (r << 1) | (r >> 4);
> +	g = (g << 11) | (g << 6) | (g << 1) | (g >> 4);
> +	b = (b << 11) | (b << 6) | (b << 1) | (r >> 4);
> +
> +	set_gamma(crtc, i, r, g, b);
> +}
> +
> +/**
> + * drm_crtc_fill_gamma_555 - Programs a default gamma ramp for RGB555-like formats
> + * @crtc: The displaying CRTC
> + * @set_gamma: Callback for programming the hardware gamma LUT
> + *
> + * Programs a default gamma ramp to hardware.
> + */
> +void drm_crtc_fill_gamma_555(struct drm_crtc *crtc, drm_crtc_set_lut_func set_gamma)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < 32; ++i)
> +		fill_gamma_555(crtc, i, i, i, i, set_gamma);
> +}
> +EXPORT_SYMBOL(drm_crtc_fill_gamma_555);
> +
> +/*
> + * Color-LUT programming
> + */
> +
> +/**
> + * drm_crtc_load_palette_8 - Programs palette for C8-like formats
> + * @crtc: The displaying CRTC
> + * @lut: The palette to program
> + * @set_palette: Callback for programming the hardware palette
> + *
> + * Programs the palette specified in @lut to hardware. The input palette
> + * must have 256 entries per color component.
> + */
> +void drm_crtc_load_palette_8(struct drm_crtc *crtc, const struct drm_color_lut *lut,
> +			     drm_crtc_set_lut_func set_palette)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < 256; ++i)
> +		set_palette(crtc, i, lut[i].red, lut[i].green, lut[i].blue);
> +}
> +EXPORT_SYMBOL(drm_crtc_load_palette_8);
> +
> +static void fill_palette_8(struct drm_crtc *crtc, unsigned int i,
> +			   drm_crtc_set_lut_func set_palette)
> +{
> +	u16 Y = (i << 8) | i; // relative luminance
> +
> +	set_palette(crtc, i, Y, Y, Y);
> +}
> +
> +/**
> + * drm_crtc_fill_palette_8 - Programs a default palette for C8-like formats
> + * @crtc: The displaying CRTC
> + * @set_palette: Callback for programming the hardware gamma LUT
> + *
> + * Programs a default palette to hardware.
> + */
> +void drm_crtc_fill_palette_8(struct drm_crtc *crtc, drm_crtc_set_lut_func set_palette)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < 256; ++i)
> +		fill_palette_8(crtc, i, set_palette);
> +}
> +EXPORT_SYMBOL(drm_crtc_fill_palette_8);
> diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> index ed81741036d7..6cb577f6dba6 100644
> --- a/include/drm/drm_color_mgmt.h
> +++ b/include/drm/drm_color_mgmt.h
> @@ -118,4 +118,31 @@ enum drm_color_lut_tests {
>   };
>   
>   int drm_color_lut_check(const struct drm_property_blob *lut, u32 tests);
> +
> +/*
> + * Gamma-LUT programming
> + */
> +
> +typedef void (*drm_crtc_set_lut_func)(struct drm_crtc *, unsigned int, u16, u16, u16);

All drivers using this code, have 256 LUT size, and use 8bits value.
So I would prefer to have it declared like this:
typedef void (*drm_crtc_set_lut_func)(struct drm_crtc *, u8, u8, u8, u8);

This will avoid to upcast from u8 to u16, and then from u16 to u8 in 
each driver.

> +
> +void drm_crtc_load_gamma_888(struct drm_crtc *crtc, const struct drm_color_lut *lut,
> +			     drm_crtc_set_lut_func set_gamma);
> +void drm_crtc_load_gamma_565_from_888(struct drm_crtc *crtc, const struct drm_color_lut *lut,
> +				      drm_crtc_set_lut_func set_gamma);
> +void drm_crtc_load_gamma_555_from_888(struct drm_crtc *crtc, const struct drm_color_lut *lut,
> +				      drm_crtc_set_lut_func set_gamma);
> +
> +void drm_crtc_fill_gamma_888(struct drm_crtc *crtc, drm_crtc_set_lut_func set_gamma);
> +void drm_crtc_fill_gamma_565(struct drm_crtc *crtc, drm_crtc_set_lut_func set_gamma);
> +void drm_crtc_fill_gamma_555(struct drm_crtc *crtc, drm_crtc_set_lut_func set_gamma);
> +
> +/*
> + * Color-LUT programming
> + */
> +
> +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_8(struct drm_crtc *crtc, drm_crtc_set_lut_func set_palette);
> +
>   #endif


Best regards,

-- 

Jocelyn

Re: [PATCH 1/5] drm: Add helpers for programming hardware gamma LUTs

[Thomas Zimmermann]

Hi

Am 12.05.25 um 15:31 schrieb Jocelyn Falempe:
> On 09/05/2025 10:23, Thomas Zimmermann wrote:
>> Provide helpers that program hardware gamma LUTs. Tha gamma ramp is
>> either provided by the driver or generated by the helper.
>>
>> The DRM driver exports the GAMMA_LUT property with a fixed number of
>> entries per color component, such as 256 on 8-bit-wide components. The
>> entries describe the gamma ramp of each individual component. The new
>> helper drm_crtc_load_gamma_888() loads such gamma ramp to hardware. The
>> hardware uses each displayed pixel's individial components as indeces
>> into the hardware gamma table.
>>
>> For color modes with less than 8 bits per color component, the helpers
>> drm_crtc_load_gamma_565_from() and drm_crtc_load_gamma_555_from_888()
>> interpolate the provided gamma ramp to reduce it to the correct number
>> of entries; 5/6/5 for RGB565-like formats and 5/5/5 for RGB1555-like
>> formats.
>>
>> If no gamma ramp has been provided, drivers can use the new helper
>> drm_crtc_fill_gamma_888() to load a default gamma ramp with 256 entries
>> per color component. For color modes with less bits, the new helpers
>> drm_crtc_fill_gamma_565() and drm_crtc_fill_gamma_555() are available.
>> The default gamma ramp uses a gamma factor of 1. Later patches can
>> change this. For PCs, a gamma factor of 2.2 is common.
>>
>> For color modes with palette, drm_crtc_load_palette_8() load an 8-bit
>> palette into the hardware. If no palette has been specified,
>> drm_crtc_fill_palette_8() load a system-specific default palette. This
>> is currently only a grey-scale palette with increasing luminance, but
>> later patches can change this. For PCs, a VGA default palette could
>> be used.
>
> Thanks for this work. I've one comment below regarding the 
> drm_crtc_set_lut_func prototype.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>   drivers/gpu/drm/drm_color_mgmt.c | 206 +++++++++++++++++++++++++++++++
>>   include/drm/drm_color_mgmt.h     |  27 ++++
>>   2 files changed, 233 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_color_mgmt.c 
>> b/drivers/gpu/drm/drm_color_mgmt.c
>> index 3969dc548cff..dd3e06605180 100644
>> --- a/drivers/gpu/drm/drm_color_mgmt.c
>> +++ b/drivers/gpu/drm/drm_color_mgmt.c
>> @@ -630,3 +630,209 @@ int drm_color_lut_check(const struct 
>> drm_property_blob *lut, u32 tests)
>>       return 0;
>>   }
>>   EXPORT_SYMBOL(drm_color_lut_check);
>> +
>> +/*
>> + * Gamma-LUT programming
>> + */
>> +
>> +/**
>> + * drm_crtc_load_gamma_888 - Programs gamma ramp for RGB888-like 
>> formats
>> + * @crtc: The displaying CRTC
>> + * @lut: The gamma ramp to program
>> + * @set_gamma: Callback for programming the hardware gamma LUT
>> + *
>> + * Programs the gamma ramp specified in @lut to hardware. The input 
>> gamma
>> + * ramp must have 256 entries per color component.
>> + */
>> +void drm_crtc_load_gamma_888(struct drm_crtc *crtc, const struct 
>> drm_color_lut *lut,
>> +                 drm_crtc_set_lut_func set_gamma)
>> +{
>> +    unsigned int i;
>> +
>> +    for (i = 0; i < 256; ++i)
>> +        set_gamma(crtc, i, lut[i].red, lut[i].green, lut[i].blue);
>> +}
>> +EXPORT_SYMBOL(drm_crtc_load_gamma_888);
>> +
>> +/**
>> + * drm_crtc_load_gamma_565_from_888 - Programs gamma ramp for 
>> RGB565-like formats
>> + * @crtc: The displaying CRTC
>> + * @lut: The gamma ramp to program
>> + * @set_gamma: Callback for programming the hardware gamma LUT
>> + *
>> + * Programs the gamma ramp specified in @lut to hardware. The input 
>> gamma
>> + * ramp must have 256 entries per color component. The helper 
>> interpolates
>> + * the individual color components to reduce the number of entries 
>> to 5/6/5.
>> + */
>> +void drm_crtc_load_gamma_565_from_888(struct drm_crtc *crtc, const 
>> struct drm_color_lut *lut,
>> +                      drm_crtc_set_lut_func set_gamma)
>> +{
>> +    unsigned int i;
>> +    u16 r, g, b;
>> +
>> +    for (i = 0; i < 32; ++i) {
>> +        r = lut[i * 8 + i / 4].red;
>> +        g = lut[i * 4 + i / 16].green;
>> +        b = lut[i * 8 + i / 4].blue;
>> +        set_gamma(crtc, i, r, g, b);
>> +    }
>> +    /* Green has one more bit, so add padding with 0 for red and 
>> blue. */
>> +    for (i = 32; i < 64; ++i) {
>> +        g = lut[i * 4 + i / 16].green;
>> +        set_gamma(crtc, i, 0, g, 0);
>> +    }
>> +}
>> +EXPORT_SYMBOL(drm_crtc_load_gamma_565_from_888);
>> +
>> +/**
>> + * drm_crtc_load_gamma_555_from_888 - Programs gamma ramp for 
>> RGB555-like formats
>> + * @crtc: The displaying CRTC
>> + * @lut: The gamma ramp to program
>> + * @set_gamma: Callback for programming the hardware gamma LUT
>> + *
>> + * Programs the gamma ramp specified in @lut to hardware. The input 
>> gamma
>> + * ramp must have 256 entries per color component. The helper 
>> interpolates
>> + * the individual color components to reduce the number of entries 
>> to 5/5/5.
>> + */
>> +void drm_crtc_load_gamma_555_from_888(struct drm_crtc *crtc, const 
>> struct drm_color_lut *lut,
>> +                      drm_crtc_set_lut_func set_gamma)
>> +{
>> +    unsigned int i;
>> +    u16 r, g, b;
>> +
>> +    for (i = 0; i < 32; ++i) {
>> +        r = lut[i * 8 + i / 4].red;
>> +        g = lut[i * 8 + i / 4].green;
>> +        b = lut[i * 8 + i / 4].blue;
>> +        set_gamma(crtc, i, r, g, b);
>> +    }
>> +}
>> +EXPORT_SYMBOL(drm_crtc_load_gamma_555_from_888);
>> +
>> +static void fill_gamma_888(struct drm_crtc *crtc, unsigned int i, 
>> u16 r, u16 g, u16 b,
>> +               drm_crtc_set_lut_func set_gamma)
>> +{
>> +    r = (r << 8) | r;
>> +    g = (g << 8) | g;
>> +    b = (b << 8) | b;
>> +
>> +    set_gamma(crtc, i, r, g, b);
>> +}
>> +
>> +/**
>> + * drm_crtc_fill_gamma_888 - Programs a default gamma ramp for 
>> RGB888-like formats
>> + * @crtc: The displaying CRTC
>> + * @set_gamma: Callback for programming the hardware gamma LUT
>> + *
>> + * Programs a default gamma ramp to hardware.
>> + */
>> +void drm_crtc_fill_gamma_888(struct drm_crtc *crtc, 
>> drm_crtc_set_lut_func set_gamma)
>> +{
>> +    unsigned int i;
>> +
>> +    for (i = 0; i < 256; ++i)
>> +        fill_gamma_888(crtc, i, i, i, i, set_gamma);
>> +}
>> +EXPORT_SYMBOL(drm_crtc_fill_gamma_888);
>> +
>> +static void fill_gamma_565(struct drm_crtc *crtc, unsigned int i, 
>> u16 r, u16 g, u16 b,
>> +               drm_crtc_set_lut_func set_gamma)
>> +{
>> +    r = (r << 11) | (r << 6) | (r << 1) | (r >> 4);
>> +    g = (g << 10) | (g << 4) | (g >> 2);
>> +    b = (b << 11) | (b << 6) | (b << 1) | (b >> 4);
>> +
>> +    set_gamma(crtc, i, r, g, b);
>> +}
>> +
>> +/**
>> + * drm_crtc_fill_gamma_565 - Programs a default gamma ramp for 
>> RGB565-like formats
>> + * @crtc: The displaying CRTC
>> + * @set_gamma: Callback for programming the hardware gamma LUT
>> + *
>> + * Programs a default gamma ramp to hardware.
>> + */
>> +void drm_crtc_fill_gamma_565(struct drm_crtc *crtc, 
>> drm_crtc_set_lut_func set_gamma)
>> +{
>> +    unsigned int i;
>> +
>> +    for (i = 0; i < 32; ++i)
>> +        fill_gamma_565(crtc, i, i, i, i, set_gamma);
>> +    /* Green has one more bit, so add padding with 0 for red and 
>> blue. */
>> +    for (i = 32; i < 64; ++i)
>> +        fill_gamma_565(crtc, i, 0, i, 0, set_gamma);
>> +}
>> +EXPORT_SYMBOL(drm_crtc_fill_gamma_565);
>> +
>> +static void fill_gamma_555(struct drm_crtc *crtc, unsigned int i, 
>> u16 r, u16 g, u16 b,
>> +               drm_crtc_set_lut_func set_gamma)
>> +{
>> +    r = (r << 11) | (r << 6) | (r << 1) | (r >> 4);
>> +    g = (g << 11) | (g << 6) | (g << 1) | (g >> 4);
>> +    b = (b << 11) | (b << 6) | (b << 1) | (r >> 4);
>> +
>> +    set_gamma(crtc, i, r, g, b);
>> +}
>> +
>> +/**
>> + * drm_crtc_fill_gamma_555 - Programs a default gamma ramp for 
>> RGB555-like formats
>> + * @crtc: The displaying CRTC
>> + * @set_gamma: Callback for programming the hardware gamma LUT
>> + *
>> + * Programs a default gamma ramp to hardware.
>> + */
>> +void drm_crtc_fill_gamma_555(struct drm_crtc *crtc, 
>> drm_crtc_set_lut_func set_gamma)
>> +{
>> +    unsigned int i;
>> +
>> +    for (i = 0; i < 32; ++i)
>> +        fill_gamma_555(crtc, i, i, i, i, set_gamma);
>> +}
>> +EXPORT_SYMBOL(drm_crtc_fill_gamma_555);
>> +
>> +/*
>> + * Color-LUT programming
>> + */
>> +
>> +/**
>> + * drm_crtc_load_palette_8 - Programs palette for C8-like formats
>> + * @crtc: The displaying CRTC
>> + * @lut: The palette to program
>> + * @set_palette: Callback for programming the hardware palette
>> + *
>> + * Programs the palette specified in @lut to hardware. The input 
>> palette
>> + * must have 256 entries per color component.
>> + */
>> +void drm_crtc_load_palette_8(struct drm_crtc *crtc, const struct 
>> drm_color_lut *lut,
>> +                 drm_crtc_set_lut_func set_palette)
>> +{
>> +    unsigned int i;
>> +
>> +    for (i = 0; i < 256; ++i)
>> +        set_palette(crtc, i, lut[i].red, lut[i].green, lut[i].blue);
>> +}
>> +EXPORT_SYMBOL(drm_crtc_load_palette_8);
>> +
>> +static void fill_palette_8(struct drm_crtc *crtc, unsigned int i,
>> +               drm_crtc_set_lut_func set_palette)
>> +{
>> +    u16 Y = (i << 8) | i; // relative luminance
>> +
>> +    set_palette(crtc, i, Y, Y, Y);
>> +}
>> +
>> +/**
>> + * drm_crtc_fill_palette_8 - Programs a default palette for C8-like 
>> formats
>> + * @crtc: The displaying CRTC
>> + * @set_palette: Callback for programming the hardware gamma LUT
>> + *
>> + * Programs a default palette to hardware.
>> + */
>> +void drm_crtc_fill_palette_8(struct drm_crtc *crtc, 
>> drm_crtc_set_lut_func set_palette)
>> +{
>> +    unsigned int i;
>> +
>> +    for (i = 0; i < 256; ++i)
>> +        fill_palette_8(crtc, i, set_palette);
>> +}
>> +EXPORT_SYMBOL(drm_crtc_fill_palette_8);
>> diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
>> index ed81741036d7..6cb577f6dba6 100644
>> --- a/include/drm/drm_color_mgmt.h
>> +++ b/include/drm/drm_color_mgmt.h
>> @@ -118,4 +118,31 @@ enum drm_color_lut_tests {
>>   };
>>     int drm_color_lut_check(const struct drm_property_blob *lut, u32 
>> tests);
>> +
>> +/*
>> + * Gamma-LUT programming
>> + */
>> +
>> +typedef void (*drm_crtc_set_lut_func)(struct drm_crtc *, unsigned 
>> int, u16, u16, u16);
>
> All drivers using this code, have 256 LUT size, and use 8bits value.
> So I would prefer to have it declared like this:
> typedef void (*drm_crtc_set_lut_func)(struct drm_crtc *, u8, u8, u8, u8);
>
> This will avoid to upcast from u8 to u16, and then from u16 to u8 in 
> each driver.

I don't want to change that, because that second shift really depends on 
the hardware:

- some of the VESA color modes affect bit shifts [1] (although I was 
unable to setup one so far)
- anything VGA-compatible would use 6 bits per component
- other drivers use 10 bits per component, for example amdgpu at [2].

To build reusable helpers, we need to take this into account.

Best regards
Thomas

[1] 
https://elixir.bootlin.com/linux/v6.14.6/source/drivers/video/fbdev/vesafb.c#L93
[2] 
https://elixir.bootlin.com/linux/v6.14.6/source/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c#L2071

>
>> +
>> +void drm_crtc_load_gamma_888(struct drm_crtc *crtc, const struct 
>> drm_color_lut *lut,
>> +                 drm_crtc_set_lut_func set_gamma);
>> +void drm_crtc_load_gamma_565_from_888(struct drm_crtc *crtc, const 
>> struct drm_color_lut *lut,
>> +                      drm_crtc_set_lut_func set_gamma);
>> +void drm_crtc_load_gamma_555_from_888(struct drm_crtc *crtc, const 
>> struct drm_color_lut *lut,
>> +                      drm_crtc_set_lut_func set_gamma);
>> +
>> +void drm_crtc_fill_gamma_888(struct drm_crtc *crtc, 
>> drm_crtc_set_lut_func set_gamma);
>> +void drm_crtc_fill_gamma_565(struct drm_crtc *crtc, 
>> drm_crtc_set_lut_func set_gamma);
>> +void drm_crtc_fill_gamma_555(struct drm_crtc *crtc, 
>> drm_crtc_set_lut_func set_gamma);
>> +
>> +/*
>> + * Color-LUT programming
>> + */
>> +
>> +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_8(struct drm_crtc *crtc, 
>> drm_crtc_set_lut_func set_palette);
>> +
>>   #endif
>
>
> Best regards,
>

-- 
--
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/5] drm: Add helpers for programming hardware gamma LUTs

[Jocelyn Falempe]

On 09/05/2025 10:23, Thomas Zimmermann wrote:
> Provide helpers that program hardware gamma LUTs. Tha gamma ramp is
> either provided by the driver or generated by the helper.
> 
> The DRM driver exports the GAMMA_LUT property with a fixed number of
> entries per color component, such as 256 on 8-bit-wide components. The
> entries describe the gamma ramp of each individual component. The new
> helper drm_crtc_load_gamma_888() loads such gamma ramp to hardware. The
> hardware uses each displayed pixel's individial components as indeces
> into the hardware gamma table.
> 
> For color modes with less than 8 bits per color component, the helpers
> drm_crtc_load_gamma_565_from() and drm_crtc_load_gamma_555_from_888()
> interpolate the provided gamma ramp to reduce it to the correct number
> of entries; 5/6/5 for RGB565-like formats and 5/5/5 for RGB1555-like
> formats.
> 
> If no gamma ramp has been provided, drivers can use the new helper
> drm_crtc_fill_gamma_888() to load a default gamma ramp with 256 entries
> per color component. For color modes with less bits, the new helpers
> drm_crtc_fill_gamma_565() and drm_crtc_fill_gamma_555() are available.
> The default gamma ramp uses a gamma factor of 1. Later patches can
> change this. For PCs, a gamma factor of 2.2 is common.
> 
> For color modes with palette, drm_crtc_load_palette_8() load an 8-bit
> palette into the hardware. If no palette has been specified,
> drm_crtc_fill_palette_8() load a system-specific default palette. This
> is currently only a grey-scale palette with increasing luminance, but
> later patches can change this. For PCs, a VGA default palette could
> be used.

If we plan to use it for driver with 10bits gamma LUT, then it's fine 
for me.

Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>

> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>   drivers/gpu/drm/drm_color_mgmt.c | 206 +++++++++++++++++++++++++++++++
>   include/drm/drm_color_mgmt.h     |  27 ++++
>   2 files changed, 233 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> index 3969dc548cff..dd3e06605180 100644
> --- a/drivers/gpu/drm/drm_color_mgmt.c
> +++ b/drivers/gpu/drm/drm_color_mgmt.c
> @@ -630,3 +630,209 @@ int drm_color_lut_check(const struct drm_property_blob *lut, u32 tests)
>   	return 0;
>   }
>   EXPORT_SYMBOL(drm_color_lut_check);
> +
> +/*
> + * Gamma-LUT programming
> + */
> +
> +/**
> + * drm_crtc_load_gamma_888 - Programs gamma ramp for RGB888-like formats
> + * @crtc: The displaying CRTC
> + * @lut: The gamma ramp to program
> + * @set_gamma: Callback for programming the hardware gamma LUT
> + *
> + * Programs the gamma ramp specified in @lut to hardware. The input gamma
> + * ramp must have 256 entries per color component.
> + */
> +void drm_crtc_load_gamma_888(struct drm_crtc *crtc, const struct drm_color_lut *lut,
> +			     drm_crtc_set_lut_func set_gamma)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < 256; ++i)
> +		set_gamma(crtc, i, lut[i].red, lut[i].green, lut[i].blue);
> +}
> +EXPORT_SYMBOL(drm_crtc_load_gamma_888);
> +
> +/**
> + * drm_crtc_load_gamma_565_from_888 - Programs gamma ramp for RGB565-like formats
> + * @crtc: The displaying CRTC
> + * @lut: The gamma ramp to program
> + * @set_gamma: Callback for programming the hardware gamma LUT
> + *
> + * Programs the gamma ramp specified in @lut to hardware. The input gamma
> + * ramp must have 256 entries per color component. The helper interpolates
> + * the individual color components to reduce the number of entries to 5/6/5.
> + */
> +void drm_crtc_load_gamma_565_from_888(struct drm_crtc *crtc, const struct drm_color_lut *lut,
> +				      drm_crtc_set_lut_func set_gamma)
> +{
> +	unsigned int i;
> +	u16 r, g, b;
> +
> +	for (i = 0; i < 32; ++i) {
> +		r = lut[i * 8 + i / 4].red;
> +		g = lut[i * 4 + i / 16].green;
> +		b = lut[i * 8 + i / 4].blue;
> +		set_gamma(crtc, i, r, g, b);
> +	}
> +	/* Green has one more bit, so add padding with 0 for red and blue. */
> +	for (i = 32; i < 64; ++i) {
> +		g = lut[i * 4 + i / 16].green;
> +		set_gamma(crtc, i, 0, g, 0);
> +	}
> +}
> +EXPORT_SYMBOL(drm_crtc_load_gamma_565_from_888);
> +
> +/**
> + * drm_crtc_load_gamma_555_from_888 - Programs gamma ramp for RGB555-like formats
> + * @crtc: The displaying CRTC
> + * @lut: The gamma ramp to program
> + * @set_gamma: Callback for programming the hardware gamma LUT
> + *
> + * Programs the gamma ramp specified in @lut to hardware. The input gamma
> + * ramp must have 256 entries per color component. The helper interpolates
> + * the individual color components to reduce the number of entries to 5/5/5.
> + */
> +void drm_crtc_load_gamma_555_from_888(struct drm_crtc *crtc, const struct drm_color_lut *lut,
> +				      drm_crtc_set_lut_func set_gamma)
> +{
> +	unsigned int i;
> +	u16 r, g, b;
> +
> +	for (i = 0; i < 32; ++i) {
> +		r = lut[i * 8 + i / 4].red;
> +		g = lut[i * 8 + i / 4].green;
> +		b = lut[i * 8 + i / 4].blue;
> +		set_gamma(crtc, i, r, g, b);
> +	}
> +}
> +EXPORT_SYMBOL(drm_crtc_load_gamma_555_from_888);
> +
> +static void fill_gamma_888(struct drm_crtc *crtc, unsigned int i, u16 r, u16 g, u16 b,
> +			   drm_crtc_set_lut_func set_gamma)
> +{
> +	r = (r << 8) | r;
> +	g = (g << 8) | g;
> +	b = (b << 8) | b;
> +
> +	set_gamma(crtc, i, r, g, b);
> +}
> +
> +/**
> + * drm_crtc_fill_gamma_888 - Programs a default gamma ramp for RGB888-like formats
> + * @crtc: The displaying CRTC
> + * @set_gamma: Callback for programming the hardware gamma LUT
> + *
> + * Programs a default gamma ramp to hardware.
> + */
> +void drm_crtc_fill_gamma_888(struct drm_crtc *crtc, drm_crtc_set_lut_func set_gamma)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < 256; ++i)
> +		fill_gamma_888(crtc, i, i, i, i, set_gamma);
> +}
> +EXPORT_SYMBOL(drm_crtc_fill_gamma_888);
> +
> +static void fill_gamma_565(struct drm_crtc *crtc, unsigned int i, u16 r, u16 g, u16 b,
> +			   drm_crtc_set_lut_func set_gamma)
> +{
> +	r = (r << 11) | (r << 6) | (r << 1) | (r >> 4);
> +	g = (g << 10) | (g << 4) | (g >> 2);
> +	b = (b << 11) | (b << 6) | (b << 1) | (b >> 4);
> +
> +	set_gamma(crtc, i, r, g, b);
> +}
> +
> +/**
> + * drm_crtc_fill_gamma_565 - Programs a default gamma ramp for RGB565-like formats
> + * @crtc: The displaying CRTC
> + * @set_gamma: Callback for programming the hardware gamma LUT
> + *
> + * Programs a default gamma ramp to hardware.
> + */
> +void drm_crtc_fill_gamma_565(struct drm_crtc *crtc, drm_crtc_set_lut_func set_gamma)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < 32; ++i)
> +		fill_gamma_565(crtc, i, i, i, i, set_gamma);
> +	/* Green has one more bit, so add padding with 0 for red and blue. */
> +	for (i = 32; i < 64; ++i)
> +		fill_gamma_565(crtc, i, 0, i, 0, set_gamma);
> +}
> +EXPORT_SYMBOL(drm_crtc_fill_gamma_565);
> +
> +static void fill_gamma_555(struct drm_crtc *crtc, unsigned int i, u16 r, u16 g, u16 b,
> +			   drm_crtc_set_lut_func set_gamma)
> +{
> +	r = (r << 11) | (r << 6) | (r << 1) | (r >> 4);
> +	g = (g << 11) | (g << 6) | (g << 1) | (g >> 4);
> +	b = (b << 11) | (b << 6) | (b << 1) | (r >> 4);
> +
> +	set_gamma(crtc, i, r, g, b);
> +}
> +
> +/**
> + * drm_crtc_fill_gamma_555 - Programs a default gamma ramp for RGB555-like formats
> + * @crtc: The displaying CRTC
> + * @set_gamma: Callback for programming the hardware gamma LUT
> + *
> + * Programs a default gamma ramp to hardware.
> + */
> +void drm_crtc_fill_gamma_555(struct drm_crtc *crtc, drm_crtc_set_lut_func set_gamma)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < 32; ++i)
> +		fill_gamma_555(crtc, i, i, i, i, set_gamma);
> +}
> +EXPORT_SYMBOL(drm_crtc_fill_gamma_555);
> +
> +/*
> + * Color-LUT programming
> + */
> +
> +/**
> + * drm_crtc_load_palette_8 - Programs palette for C8-like formats
> + * @crtc: The displaying CRTC
> + * @lut: The palette to program
> + * @set_palette: Callback for programming the hardware palette
> + *
> + * Programs the palette specified in @lut to hardware. The input palette
> + * must have 256 entries per color component.
> + */
> +void drm_crtc_load_palette_8(struct drm_crtc *crtc, const struct drm_color_lut *lut,
> +			     drm_crtc_set_lut_func set_palette)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < 256; ++i)
> +		set_palette(crtc, i, lut[i].red, lut[i].green, lut[i].blue);
> +}
> +EXPORT_SYMBOL(drm_crtc_load_palette_8);
> +
> +static void fill_palette_8(struct drm_crtc *crtc, unsigned int i,
> +			   drm_crtc_set_lut_func set_palette)
> +{
> +	u16 Y = (i << 8) | i; // relative luminance
> +
> +	set_palette(crtc, i, Y, Y, Y);
> +}
> +
> +/**
> + * drm_crtc_fill_palette_8 - Programs a default palette for C8-like formats
> + * @crtc: The displaying CRTC
> + * @set_palette: Callback for programming the hardware gamma LUT
> + *
> + * Programs a default palette to hardware.
> + */
> +void drm_crtc_fill_palette_8(struct drm_crtc *crtc, drm_crtc_set_lut_func set_palette)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < 256; ++i)
> +		fill_palette_8(crtc, i, set_palette);
> +}
> +EXPORT_SYMBOL(drm_crtc_fill_palette_8);
> diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> index ed81741036d7..6cb577f6dba6 100644
> --- a/include/drm/drm_color_mgmt.h
> +++ b/include/drm/drm_color_mgmt.h
> @@ -118,4 +118,31 @@ enum drm_color_lut_tests {
>   };
>   
>   int drm_color_lut_check(const struct drm_property_blob *lut, u32 tests);
> +
> +/*
> + * Gamma-LUT programming
> + */
> +
> +typedef void (*drm_crtc_set_lut_func)(struct drm_crtc *, unsigned int, u16, u16, u16);
> +
> +void drm_crtc_load_gamma_888(struct drm_crtc *crtc, const struct drm_color_lut *lut,
> +			     drm_crtc_set_lut_func set_gamma);
> +void drm_crtc_load_gamma_565_from_888(struct drm_crtc *crtc, const struct drm_color_lut *lut,
> +				      drm_crtc_set_lut_func set_gamma);
> +void drm_crtc_load_gamma_555_from_888(struct drm_crtc *crtc, const struct drm_color_lut *lut,
> +				      drm_crtc_set_lut_func set_gamma);
> +
> +void drm_crtc_fill_gamma_888(struct drm_crtc *crtc, drm_crtc_set_lut_func set_gamma);
> +void drm_crtc_fill_gamma_565(struct drm_crtc *crtc, drm_crtc_set_lut_func set_gamma);
> +void drm_crtc_fill_gamma_555(struct drm_crtc *crtc, drm_crtc_set_lut_func set_gamma);
> +
> +/*
> + * Color-LUT programming
> + */
> +
> +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_8(struct drm_crtc *crtc, drm_crtc_set_lut_func set_palette);
> +
>   #endif

Re: [PATCH 2/5] drm/ast: Use helpers for programming gamma ramps and palettes

[Jocelyn Falempe]

On 09/05/2025 10:23, Thomas Zimmermann wrote:
> Replace ast's code for programming the hardware gamma/palette LUT
> with DRM helpers. Either load provided data or program a default.
> Set the individual entries with a callback.
> 
> Each gamma/palette value is given as 3 individual 16-bit values
> for red, green and blue. The driver reduces them to 8 bit to make
> them fit into hardware registers.

Thanks, it looks good to me.

Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>   drivers/gpu/drm/ast/ast_mode.c | 69 +++++++++++++++++++++-------------
>   1 file changed, 42 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index 1de832964e92..7908087bcb5a 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -34,6 +34,7 @@
>   
>   #include <drm/drm_atomic.h>
>   #include <drm/drm_atomic_helper.h>
> +#include <drm/drm_color_mgmt.h>
>   #include <drm/drm_crtc.h>
>   #include <drm/drm_damage_helper.h>
>   #include <drm/drm_format_helper.h>
> @@ -71,31 +72,44 @@ static unsigned long ast_fb_vram_size(struct ast_device *ast)
>   	return cursor_offset - offset;
>   }
>   
> -static inline void ast_load_palette_index(struct ast_device *ast,
> -				     u8 index, u8 red, u8 green,
> -				     u8 blue)
> +static void ast_set_gamma_lut(struct drm_crtc *crtc, unsigned int index,
> +			      u16 red, u16 green, u16 blue)
>   {
> -	ast_io_write8(ast, AST_IO_VGADWR, index);
> +	struct drm_device *dev = crtc->dev;
> +	struct ast_device *ast = to_ast_device(dev);
> +	u8 i8 = index & 0xff;
> +	u8 r8 = red >> 8;
> +	u8 g8 = green >> 8;
> +	u8 b8 = blue >> 8;
> +
> +	if (drm_WARN_ON_ONCE(dev, index != i8))
> +		return; /* driver bug */
> +
> +	ast_io_write8(ast, AST_IO_VGADWR, i8);
>   	ast_io_read8(ast, AST_IO_VGASRI);
> -	ast_io_write8(ast, AST_IO_VGAPDR, red);
> +	ast_io_write8(ast, AST_IO_VGAPDR, r8);
>   	ast_io_read8(ast, AST_IO_VGASRI);
> -	ast_io_write8(ast, AST_IO_VGAPDR, green);
> +	ast_io_write8(ast, AST_IO_VGAPDR, g8);
>   	ast_io_read8(ast, AST_IO_VGASRI);
> -	ast_io_write8(ast, AST_IO_VGAPDR, blue);
> +	ast_io_write8(ast, AST_IO_VGAPDR, b8);
>   	ast_io_read8(ast, AST_IO_VGASRI);
>   }
>   
> -static void ast_crtc_set_gamma_linear(struct ast_device *ast,
> -				      const struct drm_format_info *format)
> +static void ast_crtc_fill_gamma(struct ast_device *ast,
> +				const struct drm_format_info *format)
>   {
> -	int i;
> +	struct drm_crtc *crtc = &ast->crtc;
>   
>   	switch (format->format) {
> -	case DRM_FORMAT_C8: /* In this case, gamma table is used as color palette */
> +	case DRM_FORMAT_C8:
> +		/* gamma table is used as color palette */
> +		drm_crtc_fill_palette_8(crtc, ast_set_gamma_lut);
> +		break;
>   	case DRM_FORMAT_RGB565:
> +		/* also uses 8-bit gamma ramp on low-color modes */
> +		fallthrough;
>   	case DRM_FORMAT_XRGB8888:
> -		for (i = 0; i < AST_LUT_SIZE; i++)
> -			ast_load_palette_index(ast, i, i, i, i);
> +		drm_crtc_fill_gamma_888(crtc, ast_set_gamma_lut);
>   		break;
>   	default:
>   		drm_warn_once(&ast->base, "Unsupported format %p4cc for gamma correction\n",
> @@ -104,21 +118,22 @@ static void ast_crtc_set_gamma_linear(struct ast_device *ast,
>   	}
>   }
>   
> -static void ast_crtc_set_gamma(struct ast_device *ast,
> -			       const struct drm_format_info *format,
> -			       struct drm_color_lut *lut)
> +static void ast_crtc_load_gamma(struct ast_device *ast,
> +				const struct drm_format_info *format,
> +				struct drm_color_lut *lut)
>   {
> -	int i;
> +	struct drm_crtc *crtc = &ast->crtc;
>   
>   	switch (format->format) {
> -	case DRM_FORMAT_C8: /* In this case, gamma table is used as color palette */
> +	case DRM_FORMAT_C8:
> +		/* gamma table is used as color palette */
> +		drm_crtc_load_palette_8(crtc, lut, ast_set_gamma_lut);
> +		break;
>   	case DRM_FORMAT_RGB565:
> +		/* also uses 8-bit gamma ramp on low-color modes */
> +		fallthrough;
>   	case DRM_FORMAT_XRGB8888:
> -		for (i = 0; i < AST_LUT_SIZE; i++)
> -			ast_load_palette_index(ast, i,
> -					       lut[i].red >> 8,
> -					       lut[i].green >> 8,
> -					       lut[i].blue >> 8);
> +		drm_crtc_load_gamma_888(crtc, lut, ast_set_gamma_lut);
>   		break;
>   	default:
>   		drm_warn_once(&ast->base, "Unsupported format %p4cc for gamma correction\n",
> @@ -811,11 +826,11 @@ ast_crtc_helper_atomic_flush(struct drm_crtc *crtc,
>   	 */
>   	if (crtc_state->enable && crtc_state->color_mgmt_changed) {
>   		if (crtc_state->gamma_lut)
> -			ast_crtc_set_gamma(ast,
> -					   ast_crtc_state->format,
> -					   crtc_state->gamma_lut->data);
> +			ast_crtc_load_gamma(ast,
> +					    ast_crtc_state->format,
> +					    crtc_state->gamma_lut->data);
>   		else
> -			ast_crtc_set_gamma_linear(ast, ast_crtc_state->format);
> +			ast_crtc_fill_gamma(ast, ast_crtc_state->format);
>   	}
>   }
>   

Re: [PATCH 3/5] drm/mgag200: Use helpers for programming gamma ramps

[Jocelyn Falempe]

On 09/05/2025 10:23, Thomas Zimmermann wrote:
> Replace mgag200's code for programming the hardware gamma LUT with
> DRM helpers. Either load a provided gamma ramp or program a default.
> Set the individual entries with a callback.
> 
> Each gamma value is given as 3 individual 16-bit values for red,
> green and blue. The driver reduces them to 8 bit to make them fit
> into hardware registers.

Thanks, it looks good to me.

Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>   drivers/gpu/drm/mgag200/mgag200_drv.h    |  4 +-
>   drivers/gpu/drm/mgag200/mgag200_g200er.c |  4 +-
>   drivers/gpu/drm/mgag200/mgag200_g200ev.c |  4 +-
>   drivers/gpu/drm/mgag200/mgag200_g200se.c |  4 +-
>   drivers/gpu/drm/mgag200/mgag200_mode.c   | 78 ++++++++++--------------
>   5 files changed, 40 insertions(+), 54 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h
> index 819a7e9381e3..7d481e11f9d6 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_drv.h
> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
> @@ -382,8 +382,8 @@ int mgag200_primary_plane_helper_get_scanout_buffer(struct drm_plane *plane,
>   	.destroy = drm_plane_cleanup, \
>   	DRM_GEM_SHADOW_PLANE_FUNCS
>   
> -void mgag200_crtc_set_gamma_linear(struct mga_device *mdev, const struct drm_format_info *format);
> -void mgag200_crtc_set_gamma(struct mga_device *mdev,
> +void mgag200_crtc_fill_gamma(struct mga_device *mdev, const struct drm_format_info *format);
> +void mgag200_crtc_load_gamma(struct mga_device *mdev,
>   			    const struct drm_format_info *format,
>   			    struct drm_color_lut *lut);
>   
> diff --git a/drivers/gpu/drm/mgag200/mgag200_g200er.c b/drivers/gpu/drm/mgag200/mgag200_g200er.c
> index c20ed0ab50ec..23debc70dc54 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_g200er.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_g200er.c
> @@ -200,9 +200,9 @@ static void mgag200_g200er_crtc_helper_atomic_enable(struct drm_crtc *crtc,
>   	mgag200_g200er_reset_tagfifo(mdev);
>   
>   	if (crtc_state->gamma_lut)
> -		mgag200_crtc_set_gamma(mdev, format, crtc_state->gamma_lut->data);
> +		mgag200_crtc_load_gamma(mdev, format, crtc_state->gamma_lut->data);
>   	else
> -		mgag200_crtc_set_gamma_linear(mdev, format);
> +		mgag200_crtc_fill_gamma(mdev, format);
>   
>   	mgag200_enable_display(mdev);
>   }
> diff --git a/drivers/gpu/drm/mgag200/mgag200_g200ev.c b/drivers/gpu/drm/mgag200/mgag200_g200ev.c
> index 78be964eb97c..f8796e2b7a0f 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_g200ev.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_g200ev.c
> @@ -201,9 +201,9 @@ static void mgag200_g200ev_crtc_helper_atomic_enable(struct drm_crtc *crtc,
>   	mgag200_g200ev_set_hiprilvl(mdev);
>   
>   	if (crtc_state->gamma_lut)
> -		mgag200_crtc_set_gamma(mdev, format, crtc_state->gamma_lut->data);
> +		mgag200_crtc_load_gamma(mdev, format, crtc_state->gamma_lut->data);
>   	else
> -		mgag200_crtc_set_gamma_linear(mdev, format);
> +		mgag200_crtc_fill_gamma(mdev, format);
>   
>   	mgag200_enable_display(mdev);
>   }
> diff --git a/drivers/gpu/drm/mgag200/mgag200_g200se.c b/drivers/gpu/drm/mgag200/mgag200_g200se.c
> index 7a32d3b1d226..e80da12ba1fe 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_g200se.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_g200se.c
> @@ -332,9 +332,9 @@ static void mgag200_g200se_crtc_helper_atomic_enable(struct drm_crtc *crtc,
>   	mgag200_g200se_set_hiprilvl(mdev, adjusted_mode, format);
>   
>   	if (crtc_state->gamma_lut)
> -		mgag200_crtc_set_gamma(mdev, format, crtc_state->gamma_lut->data);
> +		mgag200_crtc_load_gamma(mdev, format, crtc_state->gamma_lut->data);
>   	else
> -		mgag200_crtc_set_gamma_linear(mdev, format);
> +		mgag200_crtc_fill_gamma(mdev, format);
>   
>   	mgag200_enable_display(mdev);
>   }
> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
> index 6067d08aeee3..39bfb9f4b205 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
> @@ -13,6 +13,7 @@
>   
>   #include <drm/drm_atomic.h>
>   #include <drm/drm_atomic_helper.h>
> +#include <drm/drm_color_mgmt.h>
>   #include <drm/drm_damage_helper.h>
>   #include <drm/drm_edid.h>
>   #include <drm/drm_format_helper.h>
> @@ -30,35 +31,37 @@
>    * This file contains setup code for the CRTC.
>    */
>   
> -void mgag200_crtc_set_gamma_linear(struct mga_device *mdev,
> -				   const struct drm_format_info *format)
> +static void mgag200_set_gamma_lut(struct drm_crtc *crtc, unsigned int index,
> +				  u16 red, u16 green, u16 blue)
>   {
> -	int i;
> +	struct drm_device *dev = crtc->dev;
> +	struct mga_device *mdev = to_mga_device(dev);
> +	u8 i8 = index & 0xff;
> +	u8 r8 = red >> 8;
> +	u8 g8 = green >> 8;
> +	u8 b8 = blue >> 8;
> +
> +	if (drm_WARN_ON_ONCE(dev, index != i8))
> +		return; /* driver bug */
> +
> +	WREG8(DAC_INDEX + MGA1064_INDEX, i8);
> +	WREG8(DAC_INDEX + MGA1064_COL_PAL, r8);
> +	WREG8(DAC_INDEX + MGA1064_COL_PAL, g8);
> +	WREG8(DAC_INDEX + MGA1064_COL_PAL, b8);
> +}
>   
> -	WREG8(DAC_INDEX + MGA1064_INDEX, 0);
> +void mgag200_crtc_fill_gamma(struct mga_device *mdev,
> +			     const struct drm_format_info *format)
> +{
> +	struct drm_crtc *crtc = &mdev->crtc;
>   
>   	switch (format->format) {
>   	case DRM_FORMAT_RGB565:
> -		/* Use better interpolation, to take 32 values from 0 to 255 */
> -		for (i = 0; i < MGAG200_LUT_SIZE / 8; i++) {
> -			WREG8(DAC_INDEX + MGA1064_COL_PAL, i * 8 + i / 4);
> -			WREG8(DAC_INDEX + MGA1064_COL_PAL, i * 4 + i / 16);
> -			WREG8(DAC_INDEX + MGA1064_COL_PAL, i * 8 + i / 4);
> -		}
> -		/* Green has one more bit, so add padding with 0 for red and blue. */
> -		for (i = MGAG200_LUT_SIZE / 8; i < MGAG200_LUT_SIZE / 4; i++) {
> -			WREG8(DAC_INDEX + MGA1064_COL_PAL, 0);
> -			WREG8(DAC_INDEX + MGA1064_COL_PAL, i * 4 + i / 16);
> -			WREG8(DAC_INDEX + MGA1064_COL_PAL, 0);
> -		}
> +		drm_crtc_fill_gamma_565(crtc, mgag200_set_gamma_lut);
>   		break;
>   	case DRM_FORMAT_RGB888:
>   	case DRM_FORMAT_XRGB8888:
> -		for (i = 0; i < MGAG200_LUT_SIZE; i++) {
> -			WREG8(DAC_INDEX + MGA1064_COL_PAL, i);
> -			WREG8(DAC_INDEX + MGA1064_COL_PAL, i);
> -			WREG8(DAC_INDEX + MGA1064_COL_PAL, i);
> -		}
> +		drm_crtc_fill_gamma_888(crtc, mgag200_set_gamma_lut);
>   		break;
>   	default:
>   		drm_warn_once(&mdev->base, "Unsupported format %p4cc for gamma correction\n",
> @@ -67,36 +70,19 @@ void mgag200_crtc_set_gamma_linear(struct mga_device *mdev,
>   	}
>   }
>   
> -void mgag200_crtc_set_gamma(struct mga_device *mdev,
> +void mgag200_crtc_load_gamma(struct mga_device *mdev,
>   			    const struct drm_format_info *format,
>   			    struct drm_color_lut *lut)
>   {
> -	int i;
> -
> -	WREG8(DAC_INDEX + MGA1064_INDEX, 0);
> +	struct drm_crtc *crtc = &mdev->crtc;
>   
>   	switch (format->format) {
>   	case DRM_FORMAT_RGB565:
> -		/* Use better interpolation, to take 32 values from lut[0] to lut[255] */
> -		for (i = 0; i < MGAG200_LUT_SIZE / 8; i++) {
> -			WREG8(DAC_INDEX + MGA1064_COL_PAL, lut[i * 8 + i / 4].red >> 8);
> -			WREG8(DAC_INDEX + MGA1064_COL_PAL, lut[i * 4 + i / 16].green >> 8);
> -			WREG8(DAC_INDEX + MGA1064_COL_PAL, lut[i * 8 + i / 4].blue >> 8);
> -		}
> -		/* Green has one more bit, so add padding with 0 for red and blue. */
> -		for (i = MGAG200_LUT_SIZE / 8; i < MGAG200_LUT_SIZE / 4; i++) {
> -			WREG8(DAC_INDEX + MGA1064_COL_PAL, 0);
> -			WREG8(DAC_INDEX + MGA1064_COL_PAL, lut[i * 4 + i / 16].green >> 8);
> -			WREG8(DAC_INDEX + MGA1064_COL_PAL, 0);
> -		}
> +		drm_crtc_load_gamma_565_from_888(crtc, lut, mgag200_set_gamma_lut);
>   		break;
>   	case DRM_FORMAT_RGB888:
>   	case DRM_FORMAT_XRGB8888:
> -		for (i = 0; i < MGAG200_LUT_SIZE; i++) {
> -			WREG8(DAC_INDEX + MGA1064_COL_PAL, lut[i].red >> 8);
> -			WREG8(DAC_INDEX + MGA1064_COL_PAL, lut[i].green >> 8);
> -			WREG8(DAC_INDEX + MGA1064_COL_PAL, lut[i].blue >> 8);
> -		}
> +		drm_crtc_load_gamma_888(crtc, lut, mgag200_set_gamma_lut);
>   		break;
>   	default:
>   		drm_warn_once(&mdev->base, "Unsupported format %p4cc for gamma correction\n",
> @@ -642,9 +628,9 @@ void mgag200_crtc_helper_atomic_flush(struct drm_crtc *crtc, struct drm_atomic_s
>   		const struct drm_format_info *format = mgag200_crtc_state->format;
>   
>   		if (crtc_state->gamma_lut)
> -			mgag200_crtc_set_gamma(mdev, format, crtc_state->gamma_lut->data);
> +			mgag200_crtc_load_gamma(mdev, format, crtc_state->gamma_lut->data);
>   		else
> -			mgag200_crtc_set_gamma_linear(mdev, format);
> +			mgag200_crtc_fill_gamma(mdev, format);
>   	}
>   }
>   
> @@ -665,9 +651,9 @@ void mgag200_crtc_helper_atomic_enable(struct drm_crtc *crtc, struct drm_atomic_
>   		funcs->pixpllc_atomic_update(crtc, old_state);
>   
>   	if (crtc_state->gamma_lut)
> -		mgag200_crtc_set_gamma(mdev, format, crtc_state->gamma_lut->data);
> +		mgag200_crtc_load_gamma(mdev, format, crtc_state->gamma_lut->data);
>   	else
> -		mgag200_crtc_set_gamma_linear(mdev, format);
> +		mgag200_crtc_fill_gamma(mdev, format);
>   
>   	mgag200_enable_display(mdev);
>   }

Re: [PATCH 1/5] drm: Add helpers for programming hardware gamma LUTs

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann@suse.de> writes:

> Provide helpers that program hardware gamma LUTs. Tha gamma ramp is
> either provided by the driver or generated by the helper.
>
> The DRM driver exports the GAMMA_LUT property with a fixed number of
> entries per color component, such as 256 on 8-bit-wide components. The
> entries describe the gamma ramp of each individual component. The new
> helper drm_crtc_load_gamma_888() loads such gamma ramp to hardware. The
> hardware uses each displayed pixel's individial components as indeces

I think the correct plural form is either "indexes" and "indices".

> into the hardware gamma table.
>
> For color modes with less than 8 bits per color component, the helpers
> drm_crtc_load_gamma_565_from() and drm_crtc_load_gamma_555_from_888()
> interpolate the provided gamma ramp to reduce it to the correct number
> of entries; 5/6/5 for RGB565-like formats and 5/5/5 for RGB1555-like
> formats.
>
> If no gamma ramp has been provided, drivers can use the new helper
> drm_crtc_fill_gamma_888() to load a default gamma ramp with 256 entries
> per color component. For color modes with less bits, the new helpers
> drm_crtc_fill_gamma_565() and drm_crtc_fill_gamma_555() are available.
> The default gamma ramp uses a gamma factor of 1. Later patches can
> change this. For PCs, a gamma factor of 2.2 is common.
>
> For color modes with palette, drm_crtc_load_palette_8() load an 8-bit
> palette into the hardware. If no palette has been specified,
> drm_crtc_fill_palette_8() load a system-specific default palette. This
> is currently only a grey-scale palette with increasing luminance, but
> later patches can change this. For PCs, a VGA default palette could
> be used.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/gpu/drm/drm_color_mgmt.c | 206 +++++++++++++++++++++++++++++++
>  include/drm/drm_color_mgmt.h     |  27 ++++
>  2 files changed, 233 insertions(+)
>

Thanks a lot for adding all these helpers. I'm not an expert on gamma LUTs
but the patch looks good to me.

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

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 4/5] drm/ofdrm: Use helpers for programming gamma ramps

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann@suse.de> writes:

> Replace ofdrm's code for programming the hardware gamma LUT with
> DRM helpers. Either load a provided gamma ramp or program a default.
> Set the individual entries with a callback.
>
> Each gamma value is given as 3 individual 16-bit values for red,
> green and blue. The driver reduces them to 8 bit to make them fit
> into hardware registers.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/gpu/drm/sysfb/ofdrm.c | 78 +++++++++++++----------------------
>  1 file changed, 29 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/gpu/drm/sysfb/ofdrm.c b/drivers/gpu/drm/sysfb/ofdrm.c
> index fddfe8bea9f7..96fafdc8dd5e 100644
> --- a/drivers/gpu/drm/sysfb/ofdrm.c
> +++ b/drivers/gpu/drm/sysfb/ofdrm.c
> @@ -8,6 +8,7 @@

Looks good to me.

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

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 5/5] drm/vesadrm: Use helpers for programming gamma ramps

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann@suse.de> writes:

> Replace vesadrm's code for programming the hardware gamma LUT with
> DRM helpers. Either load a provided gamma ramp or program a default.
> Set the individual entries with a callback.
>
> Each gamma value is given as 3 individual 16-bit values for red,
> green and blue. The driver reduces them to 8 bit to make them fit
> into hardware registers.
>
> 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