[PATCH libdrm v2 00/10] Add support for low-color frame buffer formats

[PATCH libdrm v2 00/10] Add support for low-color frame buffer formats

[Geert Uytterhoeven]

	Hi all,

A long outstanding issue with the DRM subsystem has been the lack of
support for low-color displays, as used typically on older desktop
systems, and on small embedded displays.

This patch series adds support for color-indexed frame buffer formats
with 2, 4, and 16 colors.  It has been tested on ARAnyM using a
work-in-progress Atari DRM driver.

Changes compared to v1:
  - SMPTE color LUT accuracy,
  - Factor out smpte color LUT,
  - Restructure patches,
  - Improve descriptions.
  - Store number of colors for indexed formats,
  - Add SMPTE pattern support for the C1 and C2 formats.

Please refer to [2] for related Linux DRM patches and background
information.

Thanks for your comments!

[1] "[PATCH libdrm 0/3] Add support for low-color frame buffer formats"
    https://lore.kernel.org/r/cover.1646683737.git.geert@linux-m68k.org
[2] "[PATCH v3 00/10] drm: Add support for low-color frame buffer formats"
    https://lore.kernel.org/r/cover.1657294931.git.geert@linux-m68k.org

Geert Uytterhoeven (10):
  util: Improve SMPTE color LUT accuracy
  util: Factor out and optimize C8 SMPTE color LUT
  [RFC] drm_fourcc: Add DRM_FORMAT_C[124]
  util: Add support for DRM_FORMAT_C[124]
  util: Store number of colors for indexed formats
  util: Add SMPTE pattern support for C4 format
  util: Add SMPTE pattern support for C1 format
  util: Add SMPTE pattern support for C2 format
  modetest: Add support for DRM_FORMAT_C[124]
  modetest: Add SMPTE pattern support for C4 format

 include/drm/drm_fourcc.h  |   5 +-
 tests/modetest/buffers.c  |  15 ++
 tests/modetest/modetest.c |   9 +-
 tests/util/format.c       |   5 +-
 tests/util/format.h       |   1 +
 tests/util/pattern.c      | 416 ++++++++++++++++++++++++++++++++++----
 tests/util/pattern.h      |   2 +-
 7 files changed, 409 insertions(+), 44 deletions(-)

-- 
2.25.1

[PATCH libdrm v2 01/10] util: Improve SMPTE color LUT accuracy

[Geert Uytterhoeven]

Fill in the LSB when converting color components from 8-bit to 16-bit.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
v2:
  - New.
---
 tests/util/pattern.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/util/pattern.c b/tests/util/pattern.c
index 178aee8341a38920..3753ebc1eeae6c9a 100644
--- a/tests/util/pattern.c
+++ b/tests/util/pattern.c
@@ -646,9 +646,9 @@ void util_smpte_c8_gamma(unsigned size, struct drm_color_lut *lut)
 	memset(lut, 0, size * sizeof(struct drm_color_lut));
 
 #define FILL_COLOR(idx, r, g, b) \
-	lut[idx].red = (r) << 8; \
-	lut[idx].green = (g) << 8; \
-	lut[idx].blue = (b) << 8
+	lut[idx].red = (r) * 0x101; \
+	lut[idx].green = (g) * 0x101; \
+	lut[idx].blue = (b) * 0x101
 
 	FILL_COLOR( 0, 192, 192, 192);	/* grey */
 	FILL_COLOR( 1, 192, 192, 0  );	/* yellow */
-- 
2.25.1

[PATCH libdrm v2 02/10] util: Factor out and optimize C8 SMPTE color LUT

[Geert Uytterhoeven]

The color LUT for the SMPTE pattern in indexed mode contains 22 entries,
although only 13 are non-unique.

Reduce the size of the color LUT by dropping duplicate entries, so it
can be reused for formats supporting e.g. 16 colors.  Rename
util_smpte_c8_gamma() to util_smpte_index_gamma() accordingly.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
v2:
  - Factor out smpte color LUT.
---
 tests/modetest/modetest.c |   2 +-
 tests/util/pattern.c      | 107 ++++++++++++++++++++++++++------------
 tests/util/pattern.h      |   2 +-
 3 files changed, 76 insertions(+), 35 deletions(-)

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 9ceb5688f7683ffc..11a39ada80579293 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -1135,7 +1135,7 @@ static void set_gamma(struct device *dev, unsigned crtc_id, unsigned fourcc)
 
 	if (fourcc == DRM_FORMAT_C8) {
 		/* TODO: Add C8 support for more patterns */
-		util_smpte_c8_gamma(256, gamma_lut);
+		util_smpte_index_gamma(256, gamma_lut);
 		drmModeCreatePropertyBlob(dev->fd, gamma_lut, sizeof(gamma_lut), &blob_id);
 	} else {
 		for (i = 0; i < 256; i++) {
diff --git a/tests/util/pattern.c b/tests/util/pattern.c
index 3753ebc1eeae6c9a..5a648b468e524fbb 100644
--- a/tests/util/pattern.c
+++ b/tests/util/pattern.c
@@ -605,6 +605,69 @@ static void fill_smpte_rgb16fp(const struct util_rgb_info *rgb, void *mem,
 	}
 }
 
+enum smpte_colors {
+	SMPTE_COLOR_GREY,
+	SMPTE_COLOR_YELLOW,
+	SMPTE_COLOR_CYAN,
+	SMPTE_COLOR_GREEN,
+	SMPTE_COLOR_MAGENTA,
+	SMPTE_COLOR_RED,
+	SMPTE_COLOR_BLUE,
+	SMPTE_COLOR_BLACK,
+	SMPTE_COLOR_IN_PHASE,
+	SMPTE_COLOR_SUPER_WHITE,
+	SMPTE_COLOR_QUADRATURE,
+	SMPTE_COLOR_3PC5,
+	SMPTE_COLOR_11PC5,
+};
+
+static const struct drm_color_lut smpte_color_lut[] = {
+	[SMPTE_COLOR_GREY] =        { 192 * 0x101, 192 * 0x101, 192 * 0x101 },
+	[SMPTE_COLOR_YELLOW] =      { 192 * 0x101, 192 * 0x101, 0   * 0x101 },
+	[SMPTE_COLOR_CYAN] =        { 0   * 0x101, 192 * 0x101, 192 * 0x101 },
+	[SMPTE_COLOR_GREEN] =       { 0   * 0x101, 192 * 0x101, 0   * 0x101 },
+	[SMPTE_COLOR_MAGENTA] =     { 192 * 0x101, 0   * 0x101, 192 * 0x101 },
+	[SMPTE_COLOR_RED] =         { 192 * 0x101, 0   * 0x101, 0   * 0x101 },
+	[SMPTE_COLOR_BLUE] =        { 0   * 0x101, 0   * 0x101, 192 * 0x101 },
+	[SMPTE_COLOR_BLACK] =       { 19  * 0x101, 19  * 0x101, 19  * 0x101 },
+	[SMPTE_COLOR_IN_PHASE] =    { 0   * 0x101, 33  * 0x101, 76  * 0x101 },
+	[SMPTE_COLOR_SUPER_WHITE] = { 255 * 0x101, 255 * 0x101, 255 * 0x101 },
+	[SMPTE_COLOR_QUADRATURE] =  { 50  * 0x101, 0   * 0x101, 106 * 0x101 },
+	[SMPTE_COLOR_3PC5] =        { 9   * 0x101, 9   * 0x101, 9   * 0x101 },
+	[SMPTE_COLOR_11PC5] =       { 29  * 0x101, 29  * 0x101, 29  * 0x101 },
+};
+
+static unsigned int smpte_top[7] = {
+	SMPTE_COLOR_GREY,
+	SMPTE_COLOR_YELLOW,
+	SMPTE_COLOR_CYAN,
+	SMPTE_COLOR_GREEN,
+	SMPTE_COLOR_MAGENTA,
+	SMPTE_COLOR_RED,
+	SMPTE_COLOR_BLUE,
+};
+
+static unsigned int smpte_middle[7] = {
+	SMPTE_COLOR_BLUE,
+	SMPTE_COLOR_BLACK,
+	SMPTE_COLOR_MAGENTA,
+	SMPTE_COLOR_BLACK,
+	SMPTE_COLOR_CYAN,
+	SMPTE_COLOR_BLACK,
+	SMPTE_COLOR_GREY,
+};
+
+static unsigned int smpte_bottom[8] = {
+	SMPTE_COLOR_IN_PHASE,
+	SMPTE_COLOR_SUPER_WHITE,
+	SMPTE_COLOR_QUADRATURE,
+	SMPTE_COLOR_BLACK,
+	SMPTE_COLOR_3PC5,
+	SMPTE_COLOR_BLACK,
+	SMPTE_COLOR_11PC5,
+	SMPTE_COLOR_BLACK,
+};
+
 static void fill_smpte_c8(void *mem, unsigned int width, unsigned int height,
 			  unsigned int stride)
 {
@@ -613,34 +676,35 @@ static void fill_smpte_c8(void *mem, unsigned int width, unsigned int height,
 
 	for (y = 0; y < height * 6 / 9; ++y) {
 		for (x = 0; x < width; ++x)
-			((uint8_t *)mem)[x] = x * 7 / width;
+			((uint8_t *)mem)[x] = smpte_top[x * 7 / width];
 		mem += stride;
 	}
 
 	for (; y < height * 7 / 9; ++y) {
 		for (x = 0; x < width; ++x)
-			((uint8_t *)mem)[x] = 7 + (x * 7 / width);
+			((uint8_t *)mem)[x] = smpte_middle[x * 7 / width];
 		mem += stride;
 	}
 
 	for (; y < height; ++y) {
 		for (x = 0; x < width * 5 / 7; ++x)
 			((uint8_t *)mem)[x] =
-				14 + (x * 4 / (width * 5 / 7));
+				smpte_bottom[x * 4 / (width * 5 / 7)];
 		for (; x < width * 6 / 7; ++x)
 			((uint8_t *)mem)[x] =
-				14 + ((x - width * 5 / 7) * 3
-					      / (width / 7) + 4);
+				smpte_bottom[(x - width * 5 / 7) * 3
+					     / (width / 7) + 4];
 		for (; x < width; ++x)
-			((uint8_t *)mem)[x] = 14 + 7;
+			((uint8_t *)mem)[x] = smpte_bottom[7];
 		mem += stride;
 	}
 }
 
-void util_smpte_c8_gamma(unsigned size, struct drm_color_lut *lut)
+void util_smpte_index_gamma(unsigned size, struct drm_color_lut *lut)
 {
-	if (size < 7 + 7 + 8) {
-		printf("Error: gamma too small: %d < %d\n", size, 7 + 7 + 8);
+	if (size < ARRAY_SIZE(smpte_color_lut)) {
+		printf("Error: gamma too small: %u < %zu\n", size,
+		       ARRAY_SIZE(smpte_color_lut));
 		return;
 	}
 	memset(lut, 0, size * sizeof(struct drm_color_lut));
@@ -650,30 +714,7 @@ void util_smpte_c8_gamma(unsigned size, struct drm_color_lut *lut)
 	lut[idx].green = (g) * 0x101; \
 	lut[idx].blue = (b) * 0x101
 
-	FILL_COLOR( 0, 192, 192, 192);	/* grey */
-	FILL_COLOR( 1, 192, 192, 0  );	/* yellow */
-	FILL_COLOR( 2, 0,   192, 192);	/* cyan */
-	FILL_COLOR( 3, 0,   192, 0  );	/* green */
-	FILL_COLOR( 4, 192, 0,   192);	/* magenta */
-	FILL_COLOR( 5, 192, 0,   0  );	/* red */
-	FILL_COLOR( 6, 0,   0,   192);	/* blue */
-
-	FILL_COLOR( 7, 0,   0,   192);	/* blue */
-	FILL_COLOR( 8, 19,  19,  19 );	/* black */
-	FILL_COLOR( 9, 192, 0,   192);	/* magenta */
-	FILL_COLOR(10, 19,  19,  19 );	/* black */
-	FILL_COLOR(11, 0,   192, 192);	/* cyan */
-	FILL_COLOR(12, 19,  19,  19 );	/* black */
-	FILL_COLOR(13, 192, 192, 192);	/* grey */
-
-	FILL_COLOR(14, 0,   33,  76);	/* in-phase */
-	FILL_COLOR(15, 255, 255, 255);	/* super white */
-	FILL_COLOR(16, 50,  0,   106);	/* quadrature */
-	FILL_COLOR(17, 19,  19,  19);	/* black */
-	FILL_COLOR(18, 9,   9,   9);	/* 3.5% */
-	FILL_COLOR(19, 19,  19,  19);	/* 7.5% */
-	FILL_COLOR(20, 29,  29,  29);	/* 11.5% */
-	FILL_COLOR(21, 19,  19,  19);	/* black */
+	memcpy(lut, smpte_color_lut, sizeof(smpte_color_lut));
 
 #undef FILL_COLOR
 }
diff --git a/tests/util/pattern.h b/tests/util/pattern.h
index ea38cafdcf27d811..d178bca69b227751 100644
--- a/tests/util/pattern.h
+++ b/tests/util/pattern.h
@@ -39,7 +39,7 @@ void util_fill_pattern(uint32_t format, enum util_fill_pattern pattern,
 		       void *planes[3], unsigned int width,
 		       unsigned int height, unsigned int stride);
 
-void util_smpte_c8_gamma(unsigned size, struct drm_color_lut *lut);
+void util_smpte_index_gamma(unsigned size, struct drm_color_lut *lut);
 
 enum util_fill_pattern util_pattern_enum(const char *name);
 
-- 
2.25.1

[PATCH/RFC libdrm v2 03/10] drm_fourcc: Add DRM_FORMAT_C[124]

[Geert Uytterhoeven]

Introduce fourcc codes for color-indexed frame buffer formats with two,
four, and sixteen colors.

The fill order (the order in which multiple pixels are packed in a byte)
is the same order as used for indexed-color (2, 4, and 16 colors) images
in the PNG specification, Version 1.2.
This order is also the recommended and default order (FillOrder = 1) for
palette-color (16 colors) images in the TIFF 6.0 Specification, and is
also used for 16-color Linux frame buffer logos.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
Marked RFC, as this patch should be replaced by a proper sync with
Linux' include/drm/drm_fourcc.h, once accepted.

https://gitlab.freedesktop.org/mesa/drm/-/blob/main/include/drm/README
section "When and how to update these files".

v2:
  - Split off changes to include/drm/drm_fourcc.h,
  - Improve description.
---
 include/drm/drm_fourcc.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
index c668ffb877f1b777..789c7a180cf559af 100644
--- a/include/drm/drm_fourcc.h
+++ b/include/drm/drm_fourcc.h
@@ -99,7 +99,10 @@ extern "C" {
 #define DRM_FORMAT_INVALID	0
 
 /* color index */
-#define DRM_FORMAT_C8		fourcc_code('C', '8', ' ', ' ') /* [7:0] C */
+#define DRM_FORMAT_C1		fourcc_code('C', '1', ' ', ' ') /* [7:0] C0:C1:C2:C3:C4:C5:C6:C7 1:1:1:1:1:1:1:1 eight pixels/byte */
+#define DRM_FORMAT_C2		fourcc_code('C', '2', ' ', ' ') /* [7:0] C0:C1:C2:C3 2:2:2:2 four pixels/byte */
+#define DRM_FORMAT_C4		fourcc_code('C', '4', ' ', ' ') /* [7:0] C0:C1 4:4 two pixels/byte */
+#define DRM_FORMAT_C8		fourcc_code('C', '8', ' ', ' ') /* [7:0] C 8 one pixel/byte */
 
 /* 8 bpp Red */
 #define DRM_FORMAT_R8		fourcc_code('R', '8', ' ', ' ') /* [7:0] R */
-- 
2.25.1

[PATCH libdrm v2 04/10] util: Add support for DRM_FORMAT_C[124]

[Geert Uytterhoeven]

Add support for creating buffers using the new color-indexed frame
buffer formats with two, four, and sixteen colors.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
v2:
  - Split off changes to tests/util/format.c.
---
 tests/util/format.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/util/format.c b/tests/util/format.c
index 1ca1b82ce947b2f4..4b984af9bce8ac6f 100644
--- a/tests/util/format.c
+++ b/tests/util/format.c
@@ -40,6 +40,9 @@
 
 static const struct util_format_info format_info[] = {
 	/* Indexed */
+	{ DRM_FORMAT_C1, "C1" },
+	{ DRM_FORMAT_C2, "C2" },
+	{ DRM_FORMAT_C4, "C4" },
 	{ DRM_FORMAT_C8, "C8" },
 	/* YUV packed */
 	{ DRM_FORMAT_UYVY, "UYVY", MAKE_YUV_INFO(YUV_YCbCr | YUV_CY, 2, 2, 2) },
-- 
2.25.1

[PATCH libdrm v2 05/10] util: Store number of colors for indexed formats

[Geert Uytterhoeven]

Store the number of available colors for color-indexed frame
buffer formats in the format_info[] array.  This avoids the need of test
code for having to use switch statements all the time to obtain the
number of colors, or to check if a mode is color-indexed or not.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
v2:
  - New.
---
 tests/util/format.c | 8 ++++----
 tests/util/format.h | 1 +
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/tests/util/format.c b/tests/util/format.c
index 4b984af9bce8ac6f..a5464de6fc1ac70f 100644
--- a/tests/util/format.c
+++ b/tests/util/format.c
@@ -40,10 +40,10 @@
 
 static const struct util_format_info format_info[] = {
 	/* Indexed */
-	{ DRM_FORMAT_C1, "C1" },
-	{ DRM_FORMAT_C2, "C2" },
-	{ DRM_FORMAT_C4, "C4" },
-	{ DRM_FORMAT_C8, "C8" },
+	{ DRM_FORMAT_C1, "C1", .ncolors = 2 },
+	{ DRM_FORMAT_C2, "C2", .ncolors = 4 },
+	{ DRM_FORMAT_C4, "C4", .ncolors = 16 },
+	{ DRM_FORMAT_C8, "C8", .ncolors = 256 },
 	/* YUV packed */
 	{ DRM_FORMAT_UYVY, "UYVY", MAKE_YUV_INFO(YUV_YCbCr | YUV_CY, 2, 2, 2) },
 	{ DRM_FORMAT_VYUY, "VYUY", MAKE_YUV_INFO(YUV_YCrCb | YUV_CY, 2, 2, 2) },
diff --git a/tests/util/format.h b/tests/util/format.h
index 2ce1c021fd78d51d..b847c9f2933b3cde 100644
--- a/tests/util/format.h
+++ b/tests/util/format.h
@@ -55,6 +55,7 @@ struct util_yuv_info {
 struct util_format_info {
 	uint32_t format;
 	const char *name;
+	unsigned int ncolors;
 	const struct util_rgb_info rgb;
 	const struct util_yuv_info yuv;
 };
-- 
2.25.1

[PATCH libdrm v2 06/10] util: Add SMPTE pattern support for C4 format

[Geert Uytterhoeven]

Add support for drawing the SMPTE pattern in a buffer using the C4
indexed format.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
v2:
  - Use new smpte_top[],
  - Split off changes to tests/util/pattern.c.
---
 tests/util/pattern.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/tests/util/pattern.c b/tests/util/pattern.c
index 5a648b468e524fbb..8ed5f8dd1a6282f9 100644
--- a/tests/util/pattern.c
+++ b/tests/util/pattern.c
@@ -668,6 +668,46 @@ static unsigned int smpte_bottom[8] = {
 	SMPTE_COLOR_BLACK,
 };
 
+static void write_pixel_4(uint8_t *mem, unsigned int x, unsigned int pixel)
+{
+	if (x & 1)
+		mem[x / 2] = (mem[x / 2] & 0xf0) | (pixel & 0x0f);
+	else
+		mem[x / 2] = (mem[x / 2] & 0x0f) | (pixel << 4);
+}
+
+static void fill_smpte_c4(void *mem, unsigned int width, unsigned int height,
+			  unsigned int stride)
+{
+	unsigned int x;
+	unsigned int y;
+
+	for (y = 0; y < height * 6 / 9; ++y) {
+		for (x = 0; x < width; ++x)
+			write_pixel_4(mem, x, smpte_top[x * 7 / width]);
+		mem += stride;
+	}
+
+	for (; y < height * 7 / 9; ++y) {
+		for (x = 0; x < width; ++x)
+			write_pixel_4(mem, x, smpte_middle[x * 7 / width]);
+		mem += stride;
+	}
+
+	for (; y < height; ++y) {
+		for (x = 0; x < width * 5 / 7; ++x)
+			write_pixel_4(mem, x,
+				      smpte_bottom[x * 4 / (width * 5 / 7)]);
+		for (; x < width * 6 / 7; ++x)
+			write_pixel_4(mem, x,
+				      smpte_bottom[(x - width * 5 / 7) * 3 /
+						   (width / 7) + 4]);
+		for (; x < width; ++x)
+			write_pixel_4(mem, x, smpte_bottom[7]);
+		mem += stride;
+	}
+}
+
 static void fill_smpte_c8(void *mem, unsigned int width, unsigned int height,
 			  unsigned int stride)
 {
@@ -726,6 +766,8 @@ static void fill_smpte(const struct util_format_info *info, void *planes[3],
 	unsigned char *u, *v;
 
 	switch (info->format) {
+	case DRM_FORMAT_C4:
+		return fill_smpte_c4(planes[0], width, height, stride);
 	case DRM_FORMAT_C8:
 		return fill_smpte_c8(planes[0], width, height, stride);
 	case DRM_FORMAT_UYVY:
-- 
2.25.1

[PATCH libdrm v2 07/10] util: Add SMPTE pattern support for C1 format

[Geert Uytterhoeven]

Add support for drawing the SMPTE pattern in a buffer using the C1
indexed format.

As only two colors are available, the pattern is drawn in black and
white, using Floyd-Steinberg dithering.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
v2:
  New.
---
 tests/util/pattern.c | 171 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 167 insertions(+), 4 deletions(-)

diff --git a/tests/util/pattern.c b/tests/util/pattern.c
index 8ed5f8dd1a6282f9..5ec7d66495209df4 100644
--- a/tests/util/pattern.c
+++ b/tests/util/pattern.c
@@ -668,6 +668,163 @@ static unsigned int smpte_bottom[8] = {
 	SMPTE_COLOR_BLACK,
 };
 
+/*
+ * Floyd-Steinberg dithering
+ */
+
+struct fsd {
+	unsigned int width;
+	unsigned int x;
+	unsigned int i;
+	int red;
+	int green;
+	int blue;
+	int error[];
+};
+
+static struct fsd *fsd_alloc(unsigned int width)
+{
+	unsigned int n = 3 * (width + 1);
+	struct fsd *fsd = malloc(sizeof(*fsd) + n * sizeof(fsd->error[0]));
+
+	fsd->width = width;
+	fsd->x = 0;
+	fsd->i = 0;
+	memset(fsd->error, 0, n * sizeof(fsd->error[0]));
+
+	return fsd;
+}
+
+static inline int clamp(int val, int min, int max)
+{
+	if (val < min)
+		return min;
+	if (val > max)
+		return max;
+	return val;
+}
+
+static void fsd_dither(struct fsd *fsd, struct drm_color_lut *color)
+{
+	unsigned int i = fsd->i;
+
+	fsd->red = (int)color->red + (fsd->error[3 * i] + 8) / 16;
+	fsd->green = (int)color->green + (fsd->error[3 * i + 1] + 8) / 16;
+	fsd->blue = (int)color->blue + (fsd->error[3 * i + 2] + 8) / 16;
+
+	color->red = clamp(fsd->red, 0, 65535);
+	color->green = clamp(fsd->green, 0, 65535);
+	color->blue = clamp(fsd->blue, 0, 65535);
+}
+
+static void fsd_update(struct fsd *fsd, const struct drm_color_lut *actual)
+{
+	int error_red = fsd->red - (int)actual->red;
+	int error_green = fsd->green - (int)actual->green;
+	int error_blue = fsd->blue - (int)actual->blue;
+	unsigned int width = fsd->width;
+	unsigned int i = fsd->i, j;
+	unsigned int n = width + 1;
+
+	/* Distribute errors over neighboring pixels */
+	if (fsd->x == width - 1) {
+		/* Last pixel on this scanline */
+		/* South East: initialize to zero */
+		fsd->error[3 * i] = 0;
+		fsd->error[3 * i + 1] = 0;
+		fsd->error[3 * i + 2] = 0;
+	} else {
+		/* East: accumulate error */
+		j = (i + 1) % n;
+		fsd->error[3 * j] += 7 * error_red;
+		fsd->error[3 * j + 1] += 7 * error_green;
+		fsd->error[3 * j + 2] += 7 * error_blue;
+
+		/* South East: initial error */
+		fsd->error[3 * i] = error_red;
+		fsd->error[3 * i + 1] = error_green;
+		fsd->error[3 * i + 2] = error_blue;
+	}
+	/* South West: accumulate error */
+	j = (i + width - 1) % n;
+	fsd->error[3 * j] += 3 * error_red;
+	fsd->error[3 * j + 1] += 3 * error_green;
+	fsd->error[3 * j + 2] += 3 * error_blue;
+
+	/* South: accumulate error */
+	j = (i + width) % n;
+	fsd->error[3 * j] += 5 * error_red;
+	fsd->error[3 * j + 1] += 5 * error_green;
+	fsd->error[3 * j + 2] += 5 * error_blue;
+
+	fsd->x = (fsd->x + 1) % width;
+	fsd->i = (fsd->i + 1) % n;
+}
+
+static void write_pixel_1(uint8_t *mem, unsigned int x, unsigned int pixel)
+{
+	unsigned int shift = 7 - (x & 7);
+	unsigned int mask = 1U << shift;
+
+	mem[x / 8] = (mem[x / 8] & ~mask) | ((pixel << shift) & mask);
+}
+
+static void write_color_1(struct fsd *fsd, uint8_t *mem, unsigned int x,
+			  unsigned int index)
+{
+	struct drm_color_lut color = smpte_color_lut[index];
+	unsigned int pixel;
+
+	fsd_dither(fsd, &color);
+
+	/* ITU BT.601: Y = 0.299 R + 0.587 G + 0.114 B */
+	if (3 * color.red + 6 * color.green + color.blue >= 10 * 32768) {
+		pixel = 1;
+		color.red = color.green = color.blue = 65535;
+	} else {
+		pixel = 0;
+		color.red = color.green = color.blue = 0;
+	}
+
+	fsd_update(fsd, &color);
+
+	write_pixel_1(mem, x, pixel);
+}
+
+static void fill_smpte_c1(void *mem, unsigned int width, unsigned int height,
+			  unsigned int stride)
+{
+	struct fsd *fsd = fsd_alloc(width);
+	unsigned int x;
+	unsigned int y;
+
+	for (y = 0; y < height * 6 / 9; ++y) {
+		for (x = 0; x < width; ++x)
+			write_color_1(fsd, mem, x, smpte_top[x * 7 / width]);
+		mem += stride;
+	}
+
+	for (; y < height * 7 / 9; ++y) {
+		for (x = 0; x < width; ++x)
+			write_color_1(fsd, mem, x, smpte_middle[x * 7 / width]);
+		mem += stride;
+	}
+
+	for (; y < height; ++y) {
+		for (x = 0; x < width * 5 / 7; ++x)
+			write_color_1(fsd, mem, x,
+				      smpte_bottom[x * 4 / (width * 5 / 7)]);
+		for (; x < width * 6 / 7; ++x)
+			write_color_1(fsd, mem, x,
+				      smpte_bottom[(x - width * 5 / 7) * 3 /
+						   (width / 7) + 4]);
+		for (; x < width; ++x)
+			write_color_1(fsd, mem, x, smpte_bottom[7]);
+		mem += stride;
+	}
+
+	free(fsd);
+}
 static void write_pixel_4(uint8_t *mem, unsigned int x, unsigned int pixel)
 {
 	if (x & 1)
@@ -742,9 +899,8 @@ static void fill_smpte_c8(void *mem, unsigned int width, unsigned int height,
 
 void util_smpte_index_gamma(unsigned size, struct drm_color_lut *lut)
 {
-	if (size < ARRAY_SIZE(smpte_color_lut)) {
-		printf("Error: gamma too small: %u < %zu\n", size,
-		       ARRAY_SIZE(smpte_color_lut));
+	if (size < 2) {
+		printf("Error: gamma too small: %u < 2\n", size);
 		return;
 	}
 	memset(lut, 0, size * sizeof(struct drm_color_lut));
@@ -754,7 +910,12 @@ void util_smpte_index_gamma(unsigned size, struct drm_color_lut *lut)
 	lut[idx].green = (g) * 0x101; \
 	lut[idx].blue = (b) * 0x101
 
-	memcpy(lut, smpte_color_lut, sizeof(smpte_color_lut));
+	if (size < ARRAY_SIZE(smpte_color_lut)) {
+		FILL_COLOR(0, 0,   0,   0  );	/* black */
+		FILL_COLOR(1, 255, 255, 255);	/* white */
+	} else {
+		memcpy(lut, smpte_color_lut, sizeof(smpte_color_lut));
+	}
 
 #undef FILL_COLOR
 }
@@ -766,6 +927,8 @@ static void fill_smpte(const struct util_format_info *info, void *planes[3],
 	unsigned char *u, *v;
 
 	switch (info->format) {
+	case DRM_FORMAT_C1:
+		return fill_smpte_c1(planes[0], width, height, stride);
 	case DRM_FORMAT_C4:
 		return fill_smpte_c4(planes[0], width, height, stride);
 	case DRM_FORMAT_C8:
-- 
2.25.1

[PATCH libdrm v2 08/10] util: Add SMPTE pattern support for C2 format

[Geert Uytterhoeven]

Add support for drawing the SMPTE pattern in a buffer using the C2
indexed format.

As only four colors are available, resolution is halved, and the pattern
is drawn in a PenTile RG-GB matrix, using Floyd-Steinberg dithering.
The magnitude of the green subpixels is reduced, as there are twice as
many green subpixels as red or blue subpixels.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
v2:
  - New.
---
 tests/util/pattern.c | 97 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 96 insertions(+), 1 deletion(-)

diff --git a/tests/util/pattern.c b/tests/util/pattern.c
index 5ec7d66495209df4..a5c4e31ad856709e 100644
--- a/tests/util/pattern.c
+++ b/tests/util/pattern.c
@@ -825,6 +825,93 @@ static void fill_smpte_c1(void *mem, unsigned int width, unsigned int height,
 
 	free(fsd);
 }
+
+static void write_pixel_2(uint8_t *mem, unsigned int x, unsigned int pixel)
+{
+	unsigned int shift = 6 - 2 * (x & 3);
+	unsigned int mask = 3U << shift;
+
+	mem[x / 4] = (mem[x / 4] & ~mask) | ((pixel << shift) & mask);
+}
+
+static void write_color_2(struct fsd *fsd, uint8_t *mem, unsigned int stride,
+			  unsigned int x, unsigned int index)
+{
+	struct drm_color_lut color = smpte_color_lut[index];
+	unsigned int r, g, b;
+
+	fsd_dither(fsd, &color);
+
+	if (color.red >= 32768) {
+		r = 1;
+		color.red = 65535;
+	} else {
+		r = 0;
+		color.red = 0;
+	}
+	if (color.green >= 32768) {
+		g = 2;
+		color.green = 65535;
+	} else {
+		g = 0;
+		color.green = 0;
+	}
+	if (color.blue >= 32768) {
+		b = 3;
+		color.blue = 65535;
+	} else {
+		b = 0;
+		color.blue = 0;
+	}
+
+	fsd_update(fsd, &color);
+
+	/* Use PenTile RG-GB */
+	write_pixel_2(mem, 2 * x, r);
+	write_pixel_2(mem, 2 * x + 1, g);
+	write_pixel_2(mem + stride, 2 * x, g);
+	write_pixel_2(mem + stride, 2 * x + 1, b);
+}
+
+static void fill_smpte_c2(void *mem, unsigned int width, unsigned int height,
+			  unsigned int stride)
+{
+	struct fsd *fsd = fsd_alloc(width);
+	unsigned int x;
+	unsigned int y;
+
+	/* Half resolution for PenTile RG-GB */
+	width /= 2;
+	height /= 2;
+
+	for (y = 0; y < height * 6 / 9; ++y) {
+		for (x = 0; x < width; ++x)
+			write_color_2(fsd, mem, stride, x, smpte_top[x * 7 / width]);
+		mem += 2 * stride;
+	}
+
+	for (; y < height * 7 / 9; ++y) {
+		for (x = 0; x < width; ++x)
+			write_color_2(fsd, mem, stride, x, smpte_middle[x * 7 / width]);
+		mem += 2 * stride;
+	}
+
+	for (; y < height; ++y) {
+		for (x = 0; x < width * 5 / 7; ++x)
+			write_color_2(fsd, mem, stride, x,
+				      smpte_bottom[x * 4 / (width * 5 / 7)]);
+		for (; x < width * 6 / 7; ++x)
+			write_color_2(fsd, mem, stride, x,
+				      smpte_bottom[(x - width * 5 / 7) * 3 /
+						   (width / 7) + 4]);
+		for (; x < width; ++x)
+			write_color_2(fsd, mem, stride, x, smpte_bottom[7]);
+		mem += 2 * stride;
+	}
+
+	free(fsd);
+}
+
 static void write_pixel_4(uint8_t *mem, unsigned int x, unsigned int pixel)
 {
 	if (x & 1)
@@ -910,9 +997,15 @@ void util_smpte_index_gamma(unsigned size, struct drm_color_lut *lut)
 	lut[idx].green = (g) * 0x101; \
 	lut[idx].blue = (b) * 0x101
 
-	if (size < ARRAY_SIZE(smpte_color_lut)) {
+	if (size < 4) {
 		FILL_COLOR(0, 0,   0,   0  );	/* black */
 		FILL_COLOR(1, 255, 255, 255);	/* white */
+	} else if (size < ARRAY_SIZE(smpte_color_lut)) {
+		/* PenTile RG-GB */
+		FILL_COLOR(0, 0,   0,   0  );	/* black */
+		FILL_COLOR(1, 255, 0,   0  );	/* red */
+		FILL_COLOR(2, 0,   207, 0  );	/* green */
+		FILL_COLOR(3, 0,   0,   255);	/* blue */
 	} else {
 		memcpy(lut, smpte_color_lut, sizeof(smpte_color_lut));
 	}
@@ -929,6 +1022,8 @@ static void fill_smpte(const struct util_format_info *info, void *planes[3],
 	switch (info->format) {
 	case DRM_FORMAT_C1:
 		return fill_smpte_c1(planes[0], width, height, stride);
+	case DRM_FORMAT_C2:
+		return fill_smpte_c2(planes[0], width, height, stride);
 	case DRM_FORMAT_C4:
 		return fill_smpte_c4(planes[0], width, height, stride);
 	case DRM_FORMAT_C8:
-- 
2.25.1

[PATCH libdrm v2 09/10] modetest: Add support for DRM_FORMAT_C[124]

[Geert Uytterhoeven]

Add support for creating buffers using the new color-indexed frame
buffer formats with two, four, and sixteen colors.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
v2:
  - Split off changes to tests/modetest/buffers.c.
---
 tests/modetest/buffers.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/tests/modetest/buffers.c b/tests/modetest/buffers.c
index 8a8d9e0143474378..af7f60b4fb4d09ad 100644
--- a/tests/modetest/buffers.c
+++ b/tests/modetest/buffers.c
@@ -135,6 +135,18 @@ bo_create(int fd, unsigned int format,
 	int ret;
 
 	switch (format) {
+	case DRM_FORMAT_C1:
+		bpp = 1;
+		break;
+
+	case DRM_FORMAT_C2:
+		bpp = 2;
+		break;
+
+	case DRM_FORMAT_C4:
+		bpp = 4;
+		break;
+
 	case DRM_FORMAT_C8:
 	case DRM_FORMAT_NV12:
 	case DRM_FORMAT_NV21:
@@ -283,6 +295,9 @@ bo_create(int fd, unsigned int format,
 		planes[2] = virtual + offsets[2];
 		break;
 
+	case DRM_FORMAT_C1:
+	case DRM_FORMAT_C2:
+	case DRM_FORMAT_C4:
 	case DRM_FORMAT_C8:
 	case DRM_FORMAT_ARGB4444:
 	case DRM_FORMAT_XRGB4444:
-- 
2.25.1

[PATCH libdrm v2 10/10] modetest: Add SMPTE pattern support for C4 format

[Geert Uytterhoeven]

Add support for drawing the SMPTE pattern in buffers using a
color-indexed frame buffer formats with two, four, or sixteen colors.

Note that this still uses 256 as the CLUT size, as
DRM_IOCTL_MODE_SETGAMMA enforces that the size matches against the
(fixed) gamma size, while the CLUT size depends on the format.

Move clearing the color LUT entries from util_smpte_index_gamma() to its
caller, as only the caller knows how many entries there really are
(currently DRM always assumes 256 entries).

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
v2:
  - Split off changes to tests/modetest/modetest.c,
  - Add C1 and C2 support.

The linuxdoc comments say userspace can query the gamma size:

 * drm_mode_gamma_set_ioctl - set the gamma table
 *
 * Set the gamma table of a CRTC to the one passed in by the user. Userspace can
 * inquire the required gamma table size through drm_mode_gamma_get_ioctl.

 * drm_mode_gamma_get_ioctl - get the gamma table
 *
 * Copy the current gamma table into the storage provided. This also provides
 * the gamma table size the driver expects, which can be used to size the
 * allocated storage.

but the code doesn't seem to support that in an easy way (like setting
red/green/blue to NULL on input, retrieving gamma_size on output), only
by providing big enough buffers for red/green/blue, and looping over
gamma_size until -EINVAL is no longer returned.
---
 tests/modetest/modetest.c | 9 ++++++---
 tests/util/pattern.c      | 1 -
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 11a39ada80579293..1d87046f0fdfaf24 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -1129,13 +1129,16 @@ static bool add_property_optional(struct device *dev, uint32_t obj_id,
 static void set_gamma(struct device *dev, unsigned crtc_id, unsigned fourcc)
 {
 	unsigned blob_id = 0;
+	const struct util_format_info *info;
 	/* TODO: support 1024-sized LUTs, when the use-case arises */
 	struct drm_color_lut gamma_lut[256];
 	int i, ret;
 
-	if (fourcc == DRM_FORMAT_C8) {
-		/* TODO: Add C8 support for more patterns */
-		util_smpte_index_gamma(256, gamma_lut);
+	info = util_format_info_find(fourcc);
+	if (info->ncolors) {
+		memset(gamma_lut, 0, sizeof(gamma_lut));
+		/* TODO: Add index support for more patterns */
+		util_smpte_index_gamma(info->ncolors, gamma_lut);
 		drmModeCreatePropertyBlob(dev->fd, gamma_lut, sizeof(gamma_lut), &blob_id);
 	} else {
 		for (i = 0; i < 256; i++) {
diff --git a/tests/util/pattern.c b/tests/util/pattern.c
index a5c4e31ad856709e..631114563fa011c2 100644
--- a/tests/util/pattern.c
+++ b/tests/util/pattern.c
@@ -990,7 +990,6 @@ void util_smpte_index_gamma(unsigned size, struct drm_color_lut *lut)
 		printf("Error: gamma too small: %u < 2\n", size);
 		return;
 	}
-	memset(lut, 0, size * sizeof(struct drm_color_lut));
 
 #define FILL_COLOR(idx, r, g, b) \
 	lut[idx].red = (r) * 0x101; \
-- 
2.25.1

Re: [PATCH libdrm v2 01/10] util: Improve SMPTE color LUT accuracy

[Sam Ravnborg]

Hi Geert,

On Fri, Jul 08, 2022 at 08:21:31PM +0200, Geert Uytterhoeven wrote:
> Fill in the LSB when converting color components from 8-bit to 16-bit.
> 
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
> v2:
>   - New.
> ---
>  tests/util/pattern.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/util/pattern.c b/tests/util/pattern.c
> index 178aee8341a38920..3753ebc1eeae6c9a 100644
> --- a/tests/util/pattern.c
> +++ b/tests/util/pattern.c
> @@ -646,9 +646,9 @@ void util_smpte_c8_gamma(unsigned size, struct drm_color_lut *lut)
>  	memset(lut, 0, size * sizeof(struct drm_color_lut));
>  
>  #define FILL_COLOR(idx, r, g, b) \
> -	lut[idx].red = (r) << 8; \
> -	lut[idx].green = (g) << 8; \
> -	lut[idx].blue = (b) << 8
> +	lut[idx].red = (r) * 0x101; \

	(lut[idx].red = (r) << 8) | 1;

had IMO been easier to read.

Patch is:
Acked-by: Sam Ravnborg <sam@ravnborg.org>

for both ways to do it.


> +	lut[idx].green = (g) * 0x101; \
> +	lut[idx].blue = (b) * 0x101
>  
>  	FILL_COLOR( 0, 192, 192, 192);	/* grey */
>  	FILL_COLOR( 1, 192, 192, 0  );	/* yellow */
> -- 
> 2.25.1

Re: [PATCH libdrm v2 00/10] Add support for low-color frame buffer formats

[Sam Ravnborg]

Hi Geert,

On Fri, Jul 08, 2022 at 08:21:30PM +0200, Geert Uytterhoeven wrote:
> 	Hi all,
> 
> A long outstanding issue with the DRM subsystem has been the lack of
> support for low-color displays, as used typically on older desktop
> systems, and on small embedded displays.
> 
> This patch series adds support for color-indexed frame buffer formats
> with 2, 4, and 16 colors.  It has been tested on ARAnyM using a
> work-in-progress Atari DRM driver.
> 
> Changes compared to v1:
>   - SMPTE color LUT accuracy,
>   - Factor out smpte color LUT,
>   - Restructure patches,
>   - Improve descriptions.
>   - Store number of colors for indexed formats,
>   - Add SMPTE pattern support for the C1 and C2 formats.
> 
> Please refer to [2] for related Linux DRM patches and background
> information.
> 
> Thanks for your comments!
> 
> [1] "[PATCH libdrm 0/3] Add support for low-color frame buffer formats"
>     https://lore.kernel.org/r/cover.1646683737.git.geert@linux-m68k.org
> [2] "[PATCH v3 00/10] drm: Add support for low-color frame buffer formats"
>     https://lore.kernel.org/r/cover.1657294931.git.geert@linux-m68k.org
> 
> Geert Uytterhoeven (10):
>   util: Improve SMPTE color LUT accuracy
>   util: Factor out and optimize C8 SMPTE color LUT
>   [RFC] drm_fourcc: Add DRM_FORMAT_C[124]
>   util: Add support for DRM_FORMAT_C[124]
>   util: Store number of colors for indexed formats
>   util: Add SMPTE pattern support for C4 format
>   util: Add SMPTE pattern support for C1 format
>   util: Add SMPTE pattern support for C2 format
>   modetest: Add support for DRM_FORMAT_C[124]
>   modetest: Add SMPTE pattern support for C4 format

I have browsed the patches - everything looked good.
The parts with Floyd-Steinberg dithering did get less
attention as I do not know it.
But everything looked good and the series is:

Acked-by: Sam Ravnborg <sam@ravnborg.org>

	Sam

Re: [PATCH libdrm v2 01/10] util: Improve SMPTE color LUT accuracy

[Geert Uytterhoeven]

Hi Sam,

On Sun, Jul 10, 2022 at 12:31 PM Sam Ravnborg <sam@ravnborg.org> wrote:
> On Fri, Jul 08, 2022 at 08:21:31PM +0200, Geert Uytterhoeven wrote:
> > Fill in the LSB when converting color components from 8-bit to 16-bit.
> >
> > Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> > ---
> > v2:
> >   - New.
> > ---
> >  tests/util/pattern.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/tests/util/pattern.c b/tests/util/pattern.c
> > index 178aee8341a38920..3753ebc1eeae6c9a 100644
> > --- a/tests/util/pattern.c
> > +++ b/tests/util/pattern.c
> > @@ -646,9 +646,9 @@ void util_smpte_c8_gamma(unsigned size, struct drm_color_lut *lut)
> >       memset(lut, 0, size * sizeof(struct drm_color_lut));
> >
> >  #define FILL_COLOR(idx, r, g, b) \
> > -     lut[idx].red = (r) << 8; \
> > -     lut[idx].green = (g) << 8; \
> > -     lut[idx].blue = (b) << 8
> > +     lut[idx].red = (r) * 0x101; \
>
>         (lut[idx].red = (r) << 8) | 1;
>
> had IMO been easier to read.

I guess you mean "| (r)" instead of "| 1"?

>
> Patch is:
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
>
> for both ways to do it.

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH libdrm v2 01/10] util: Improve SMPTE color LUT accuracy

[Sam Ravnborg]

Hi Geert,

On Sun, Jul 10, 2022 at 01:04:23PM +0200, Geert Uytterhoeven wrote:
> Hi Sam,
> 
> On Sun, Jul 10, 2022 at 12:31 PM Sam Ravnborg <sam@ravnborg.org> wrote:
> > On Fri, Jul 08, 2022 at 08:21:31PM +0200, Geert Uytterhoeven wrote:
> > > Fill in the LSB when converting color components from 8-bit to 16-bit.
> > >
> > > Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> > > ---
> > > v2:
> > >   - New.
> > > ---
> > >  tests/util/pattern.c | 6 +++---
> > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/tests/util/pattern.c b/tests/util/pattern.c
> > > index 178aee8341a38920..3753ebc1eeae6c9a 100644
> > > --- a/tests/util/pattern.c
> > > +++ b/tests/util/pattern.c
> > > @@ -646,9 +646,9 @@ void util_smpte_c8_gamma(unsigned size, struct drm_color_lut *lut)
> > >       memset(lut, 0, size * sizeof(struct drm_color_lut));
> > >
> > >  #define FILL_COLOR(idx, r, g, b) \
> > > -     lut[idx].red = (r) << 8; \
> > > -     lut[idx].green = (g) << 8; \
> > > -     lut[idx].blue = (b) << 8
> > > +     lut[idx].red = (r) * 0x101; \
> >
> >         (lut[idx].red = (r) << 8) | 1;
> >
> > had IMO been easier to read.
> 
> I guess you mean "| (r)" instead of "| 1"?
Well, I meant what I wrote but it is obviously wrong.

So yes
	lut[idx].red = (r) << 8 | (r);

is the equivalent of multiplying with 0x101.

Whatever works, but if you update this patch, then please update the
later patch where the multiply with 0x101 is also used.

	Sam

Re: [PATCH libdrm v2 00/10] Add support for low-color frame buffer formats

[Geert Uytterhoeven]

Hi Sam,

On Sun, Jul 10, 2022 at 12:40 PM Sam Ravnborg <sam@ravnborg.org> wrote:
> On Fri, Jul 08, 2022 at 08:21:30PM +0200, Geert Uytterhoeven wrote:
> > A long outstanding issue with the DRM subsystem has been the lack of
> > support for low-color displays, as used typically on older desktop
> > systems, and on small embedded displays.
> >
> > This patch series adds support for color-indexed frame buffer formats
> > with 2, 4, and 16 colors.  It has been tested on ARAnyM using a
> > work-in-progress Atari DRM driver.
> >
> > Changes compared to v1:
> >   - SMPTE color LUT accuracy,
> >   - Factor out smpte color LUT,
> >   - Restructure patches,
> >   - Improve descriptions.
> >   - Store number of colors for indexed formats,
> >   - Add SMPTE pattern support for the C1 and C2 formats.
> >
> > Please refer to [2] for related Linux DRM patches and background
> > information.
> >
> > Thanks for your comments!
> >
> > [1] "[PATCH libdrm 0/3] Add support for low-color frame buffer formats"
> >     https://lore.kernel.org/r/cover.1646683737.git.geert@linux-m68k.org
> > [2] "[PATCH v3 00/10] drm: Add support for low-color frame buffer formats"
> >     https://lore.kernel.org/r/cover.1657294931.git.geert@linux-m68k.org
> >
> > Geert Uytterhoeven (10):
> >   util: Improve SMPTE color LUT accuracy
> >   util: Factor out and optimize C8 SMPTE color LUT
> >   [RFC] drm_fourcc: Add DRM_FORMAT_C[124]
> >   util: Add support for DRM_FORMAT_C[124]
> >   util: Store number of colors for indexed formats
> >   util: Add SMPTE pattern support for C4 format
> >   util: Add SMPTE pattern support for C1 format
> >   util: Add SMPTE pattern support for C2 format
> >   modetest: Add support for DRM_FORMAT_C[124]
> >   modetest: Add SMPTE pattern support for C4 format
>
> I have browsed the patches - everything looked good.
> The parts with Floyd-Steinberg dithering did get less
> attention as I do not know it.

As a picture says more than 10000 words, I shared screenshots of
modetest showing the SMPTE pattern in C1 (monochrome FS-dithered
[1]), C2 (PenTile RG-GB FS-dithered [2]), and C4 ([3]).
And Wikipedia[4] is your friend for the algorithm ;-)

> But everything looked good and the series is:
>
> Acked-by: Sam Ravnborg <sam@ravnborg.org>

Thanks!

[1] https://drive.google.com/file/d/1waJczErrIaEKRhBCCU1ynxRG8agpo0Xx/view?usp=sharing
[2] https://drive.google.com/file/d/1g5O8XeacrjrC8rgaVENvR65YeI6QvmtO/view?usp=sharing
[3] https://drive.google.com/file/d/1VIMqz_OkjJbbFS2OCX1O8GBm4bag6y2n/view?usp=sharing
[4] https://en.wikipedia.org/wiki/Floyd%E2%80%93Steinberg_dithering

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH libdrm v2 01/10] util: Improve SMPTE color LUT accuracy

[Geert Uytterhoeven]

Hi Sam,

> On Sun, Jul 10, 2022 at 01:04:23PM +0200, Geert Uytterhoeven wrote:
> > On Sun, Jul 10, 2022 at 12:31 PM Sam Ravnborg <sam@ravnborg.org> wrote:
> > > On Fri, Jul 08, 2022 at 08:21:31PM +0200, Geert Uytterhoeven wrote:
> > > > Fill in the LSB when converting color components from 8-bit to 16-bit.
> > > >
> > > > Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> > > > --- a/tests/util/pattern.c
> > > > +++ b/tests/util/pattern.c
> > > > @@ -646,9 +646,9 @@ void util_smpte_c8_gamma(unsigned size, struct drm_color_lut *lut)
> > > >       memset(lut, 0, size * sizeof(struct drm_color_lut));
> > > >
> > > >  #define FILL_COLOR(idx, r, g, b) \
> > > > -     lut[idx].red = (r) << 8; \
> > > > -     lut[idx].green = (g) << 8; \
> > > > -     lut[idx].blue = (b) << 8
> > > > +     lut[idx].red = (r) * 0x101; \
> > >
> > >         (lut[idx].red = (r) << 8) | 1;
> > >
> > > had IMO been easier to read.
> >
> > I guess you mean "| (r)" instead of "| 1"?
>
> Well, I meant what I wrote but it is obviously wrong.

;-)

> So yes
>         lut[idx].red = (r) << 8 | (r);
>
> is the equivalent of multiplying with 0x101.
>
> Whatever works, but if you update this patch, then please update the
> later patch where the multiply with 0x101 is also used.

The advantage of the multiplication is that the macro's parameters
are used only once, hence no temporaries are needed to keep them safe.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds