[RFC PATCH v2 0/3] drm: atmel-hlcdc: clut support

[RFC PATCH v2 0/3] drm: atmel-hlcdc: clut support

[Peter Rosin]

Hi!

This series adds support for an 8-bit clut mode in the atmel-hlcdc
driver.

I have only tested it with the fbdev interface, and am therefore not
really sure if it works w/o patch 2 and 3. One significant reason for
that is that the modetest test program in libdrm does not support clut
modes like C8.

Also, when using it with the fbdev emulation layer, something seems
to allocate the first 16 clut entries, which throws my users of
ioctl(fb, FBIOPUTCMAP, ...) when index 0 in the given clut is
really pixel value 16. If anyone can shed some light on that, I'd
be most greatful!

Changes since v1:

- Move the clut update from atmel_hlcdc_crtc_mode_valid to
  atmel_hlcdc_plane_atomic_update.
- Add default .gamma_set helper (drm_atomic_helper_legacy_gamma_set).
- Don't keep a spare copy of the clut, reuse gamma_store instead.
- Don't try to synchronize the legacy fb clut with the drm clut.

I'm obvously new to this part of the kernel, but I'm beginning to
think that perhaps the .gamma_set and .gamma_get implementations
that I add in patch 3/3 are generic enough to just be default
helper implementations? Then, all the spilt guts in patch 2/3
could be kept internal. But, I don't know if those are generic
enough implementations to be suitable as defaults...

Oh right, I forgot to mention this in v1, but I have not added any
.clut_offset to the overlay2 layer of sama5d4, since the chip does
not appear to have that layer. I didn't do that to make it easier
to work with the two patches, but I suspect bad things may happen
to sama5d4 users if they do not have that layer removed.

Cheers,
peda

Peter Rosin (3):
  drm: atmel-hlcdc: add support for 8-bit color lookup table mode
  drm/fb-cma-helper: expose more of fb cma guts
  drm: atmel-hlcdc: add clut support for legacy fbdev

 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c  | 58 +++++++++++++++++++++++++
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c    | 25 ++++++++++-
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h    | 20 +++++++++
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 29 +++++++++++++
 drivers/gpu/drm/drm_fb_cma_helper.c             | 55 ++++++++++++++++++-----
 include/drm/drm_fb_cma_helper.h                 |  8 +++-
 6 files changed, 182 insertions(+), 13 deletions(-)

-- 
2.1.4

[RFC PATCH v2 1/3] drm: atmel-hlcdc: add support for 8-bit color lookup table mode

[Peter Rosin]

All layers of all supported chips support this, the only variable is the
base address of the lookup table in the register map.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c  |  5 +++++
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c    | 13 +++++++++++
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h    | 16 ++++++++++++++
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 29 +++++++++++++++++++++++++
 4 files changed, 63 insertions(+)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
index 5348985..694adcc 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
@@ -429,6 +429,8 @@ static const struct drm_crtc_funcs atmel_hlcdc_crtc_funcs = {
 	.atomic_destroy_state = atmel_hlcdc_crtc_destroy_state,
 	.enable_vblank = atmel_hlcdc_crtc_enable_vblank,
 	.disable_vblank = atmel_hlcdc_crtc_disable_vblank,
+	.set_property = drm_atomic_helper_crtc_set_property,
+	.gamma_set = drm_atomic_helper_legacy_gamma_set,
 };
 
 int atmel_hlcdc_crtc_create(struct drm_device *dev)
@@ -484,6 +486,9 @@ int atmel_hlcdc_crtc_create(struct drm_device *dev)
 	drm_crtc_helper_add(&crtc->base, &lcdc_crtc_helper_funcs);
 	drm_crtc_vblank_reset(&crtc->base);
 
+	drm_mode_crtc_set_gamma_size(&crtc->base, ATMEL_HLCDC_CLUT_SIZE);
+	drm_crtc_enable_color_mgmt(&crtc->base, 0, false, ATMEL_HLCDC_CLUT_SIZE);
+
 	dc->crtc = &crtc->base;
 
 	return 0;
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
index 30dbffd..4f6ef07 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
@@ -42,6 +42,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_at91sam9n12_layers[] = {
 			.default_color = 3,
 			.general_config = 4,
 		},
+		.clut_offset = 0x400,
 	},
 };
 
@@ -73,6 +74,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_at91sam9x5_layers[] = {
 			.disc_pos = 5,
 			.disc_size = 6,
 		},
+		.clut_offset = 0x400,
 	},
 	{
 		.name = "overlay1",
@@ -91,6 +93,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_at91sam9x5_layers[] = {
 			.chroma_key_mask = 8,
 			.general_config = 9,
 		},
+		.clut_offset = 0x800,
 	},
 	{
 		.name = "high-end-overlay",
@@ -112,6 +115,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_at91sam9x5_layers[] = {
 			.scaler_config = 13,
 			.csc = 14,
 		},
+		.clut_offset = 0x1000,
 	},
 	{
 		.name = "cursor",
@@ -131,6 +135,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_at91sam9x5_layers[] = {
 			.chroma_key_mask = 8,
 			.general_config = 9,
 		},
+		.clut_offset = 0x1400,
 	},
 };
 
@@ -162,6 +167,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d3_layers[] = {
 			.disc_pos = 5,
 			.disc_size = 6,
 		},
+		.clut_offset = 0x600,
 	},
 	{
 		.name = "overlay1",
@@ -180,6 +186,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d3_layers[] = {
 			.chroma_key_mask = 8,
 			.general_config = 9,
 		},
+		.clut_offset = 0xa00,
 	},
 	{
 		.name = "overlay2",
@@ -198,6 +205,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d3_layers[] = {
 			.chroma_key_mask = 8,
 			.general_config = 9,
 		},
+		.clut_offset = 0xe00,
 	},
 	{
 		.name = "high-end-overlay",
@@ -223,6 +231,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d3_layers[] = {
 			},
 			.csc = 14,
 		},
+		.clut_offset = 0x1200,
 	},
 	{
 		.name = "cursor",
@@ -244,6 +253,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d3_layers[] = {
 			.general_config = 9,
 			.scaler_config = 13,
 		},
+		.clut_offset = 0x1600,
 	},
 };
 
@@ -275,6 +285,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d4_layers[] = {
 			.disc_pos = 5,
 			.disc_size = 6,
 		},
+		.clut_offset = 0x600,
 	},
 	{
 		.name = "overlay1",
@@ -293,6 +304,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d4_layers[] = {
 			.chroma_key_mask = 8,
 			.general_config = 9,
 		},
+		.clut_offset = 0xa00,
 	},
 	{
 		.name = "overlay2",
@@ -336,6 +348,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d4_layers[] = {
 			},
 			.csc = 14,
 		},
+		.clut_offset = 0x1200,
 	},
 };
 
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
index b0596a8..709f7b9 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
@@ -88,6 +88,11 @@
 #define ATMEL_HLCDC_YUV422SWP			BIT(17)
 #define ATMEL_HLCDC_DSCALEOPT			BIT(20)
 
+#define ATMEL_HLCDC_C1_MODE			ATMEL_HLCDC_CLUT_MODE(0)
+#define ATMEL_HLCDC_C2_MODE			ATMEL_HLCDC_CLUT_MODE(1)
+#define ATMEL_HLCDC_C4_MODE			ATMEL_HLCDC_CLUT_MODE(2)
+#define ATMEL_HLCDC_C8_MODE			ATMEL_HLCDC_CLUT_MODE(3)
+
 #define ATMEL_HLCDC_XRGB4444_MODE		ATMEL_HLCDC_RGB_MODE(0)
 #define ATMEL_HLCDC_ARGB4444_MODE		ATMEL_HLCDC_RGB_MODE(1)
 #define ATMEL_HLCDC_RGBA4444_MODE		ATMEL_HLCDC_RGB_MODE(2)
@@ -142,6 +147,8 @@
 #define ATMEL_HLCDC_DMA_CHANNEL_DSCR_DONE	BIT(2)
 #define ATMEL_HLCDC_DMA_CHANNEL_DSCR_OVERRUN	BIT(3)
 
+#define ATMEL_HLCDC_CLUT_SIZE			256
+
 #define ATMEL_HLCDC_MAX_LAYERS			6
 
 /**
@@ -259,6 +266,7 @@ struct atmel_hlcdc_layer_desc {
 	int id;
 	int regs_offset;
 	int cfgs_offset;
+	int clut_offset;
 	struct atmel_hlcdc_formats *formats;
 	struct atmel_hlcdc_layer_cfg_layout layout;
 	int max_width;
@@ -414,6 +422,14 @@ static inline u32 atmel_hlcdc_layer_read_cfg(struct atmel_hlcdc_layer *layer,
 					  (cfgid * sizeof(u32)));
 }
 
+static inline void atmel_hlcdc_layer_write_clut(struct atmel_hlcdc_layer *layer,
+						unsigned int c, u32 val)
+{
+	atmel_hlcdc_layer_write_reg(layer,
+				    layer->desc->clut_offset + c * sizeof(u32),
+				    val);
+}
+
 static inline void atmel_hlcdc_layer_init(struct atmel_hlcdc_layer *layer,
 				const struct atmel_hlcdc_layer_desc *desc,
 				struct regmap *regmap)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
index 1124200..b5bd9b0 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
@@ -83,6 +83,7 @@ drm_plane_state_to_atmel_hlcdc_plane_state(struct drm_plane_state *s)
 #define SUBPIXEL_MASK			0xffff
 
 static uint32_t rgb_formats[] = {
+	DRM_FORMAT_C8,
 	DRM_FORMAT_XRGB4444,
 	DRM_FORMAT_ARGB4444,
 	DRM_FORMAT_RGBA4444,
@@ -100,6 +101,7 @@ struct atmel_hlcdc_formats atmel_hlcdc_plane_rgb_formats = {
 };
 
 static uint32_t rgb_and_yuv_formats[] = {
+	DRM_FORMAT_C8,
 	DRM_FORMAT_XRGB4444,
 	DRM_FORMAT_ARGB4444,
 	DRM_FORMAT_RGBA4444,
@@ -128,6 +130,9 @@ struct atmel_hlcdc_formats atmel_hlcdc_plane_rgb_and_yuv_formats = {
 static int atmel_hlcdc_format_to_plane_mode(u32 format, u32 *mode)
 {
 	switch (format) {
+	case DRM_FORMAT_C8:
+		*mode = ATMEL_HLCDC_C8_MODE;
+		break;
 	case DRM_FORMAT_XRGB4444:
 		*mode = ATMEL_HLCDC_XRGB4444_MODE;
 		break;
@@ -424,6 +429,29 @@ static void atmel_hlcdc_plane_update_format(struct atmel_hlcdc_plane *plane,
 				    ATMEL_HLCDC_LAYER_FORMAT_CFG, cfg);
 }
 
+static void atmel_hlcdc_plane_update_clut(struct atmel_hlcdc_plane *plane)
+{
+	struct drm_crtc *crtc = plane->base.crtc;
+	struct drm_color_lut *lut;
+	int idx;
+
+	if (!crtc || !crtc->state)
+		return;
+
+	if (!crtc->state->color_mgmt_changed || !crtc->state->gamma_lut)
+		return;
+
+	lut = (struct drm_color_lut *)crtc->state->gamma_lut->data;
+
+	for (idx = 0; idx < ATMEL_HLCDC_CLUT_SIZE; idx++, lut++) {
+		u32 val = ((lut->red << 8) & 0xff0000) |
+			(lut->green & 0xff00) |
+			(lut->blue >> 8);
+
+		atmel_hlcdc_layer_write_clut(&plane->layer, idx, val);
+	}
+}
+
 static void atmel_hlcdc_plane_update_buffers(struct atmel_hlcdc_plane *plane,
 					struct atmel_hlcdc_plane_state *state)
 {
@@ -768,6 +796,7 @@ static void atmel_hlcdc_plane_atomic_update(struct drm_plane *p,
 	atmel_hlcdc_plane_update_pos_and_size(plane, state);
 	atmel_hlcdc_plane_update_general_settings(plane, state);
 	atmel_hlcdc_plane_update_format(plane, state);
+	atmel_hlcdc_plane_update_clut(plane);
 	atmel_hlcdc_plane_update_buffers(plane, state);
 	atmel_hlcdc_plane_update_disc_area(plane, state);
 
-- 
2.1.4

[RFC PATCH v2 2/3] drm/fb-cma-helper: expose more of fb cma guts

[Peter Rosin]

DRM drivers supporting clut may want a convenient way to only use
non-default .gamma_set and .gamma_get ops in the drm_fb_helper_funcs
in order to avoid the following

	/*
	 * The driver really shouldn't advertise pseudo/directcolor
	 * visuals if it can't deal with the palette.
	 */
	if (WARN_ON(!fb_helper->funcs->gamma_set ||
		    !fb_helper->funcs->gamma_get))
		return -EINVAL;

warning in drm_fb_helper.c:setcolreg().

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/gpu/drm/drm_fb_cma_helper.c | 55 ++++++++++++++++++++++++++++++-------
 include/drm/drm_fb_cma_helper.h     |  8 +++++-
 2 files changed, 52 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c
index 53f9bdf..ef96227 100644
--- a/drivers/gpu/drm/drm_fb_cma_helper.c
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -426,7 +426,12 @@ static void drm_fbdev_cma_defio_fini(struct fb_info *fbi)
 	kfree(fbi->fbops);
 }
 
-static int
+/**
+ * drm_fbdev_cma_create() - Default fb_probe() function for fb_cma_helper_funcs
+ * @helper: The fb_helper to create a cma for
+ * @sizes: The fbdev sizes
+ */
+int
 drm_fbdev_cma_create(struct drm_fb_helper *helper,
 	struct drm_fb_helper_surface_size *sizes)
 {
@@ -507,23 +512,28 @@ drm_fbdev_cma_create(struct drm_fb_helper *helper,
 	drm_gem_object_put_unlocked(&obj->base);
 	return ret;
 }
+EXPORT_SYMBOL_GPL(drm_fbdev_cma_create);
 
 static const struct drm_fb_helper_funcs drm_fb_cma_helper_funcs = {
 	.fb_probe = drm_fbdev_cma_create,
 };
 
 /**
- * drm_fbdev_cma_init_with_funcs() - Allocate and initializes a drm_fbdev_cma struct
+ * drm_fbdev_cma_init_with_funcs2() - Allocate and initializes a drm_fbdev_cma struct
  * @dev: DRM device
  * @preferred_bpp: Preferred bits per pixel for the device
  * @max_conn_count: Maximum number of connectors
- * @funcs: fb helper functions, in particular a custom dirty() callback
+ * @framebuffer_funcs: framebuffer functions, in particular a custom dirty() callback
+ * @fb_helper_funcs: fb helper functions, in particular custom gamma_set() and gamma_get() callbacks
+ *
+ * If framebuffer_funcs or fb_helper_funcs are NULL, default functions are used.
  *
  * Returns a newly allocated drm_fbdev_cma struct or a ERR_PTR.
  */
-struct drm_fbdev_cma *drm_fbdev_cma_init_with_funcs(struct drm_device *dev,
+struct drm_fbdev_cma *drm_fbdev_cma_init_with_funcs2(struct drm_device *dev,
 	unsigned int preferred_bpp, unsigned int max_conn_count,
-	const struct drm_framebuffer_funcs *funcs)
+	const struct drm_framebuffer_funcs *framebuffer_funcs,
+	const struct drm_fb_helper_funcs *fb_helper_funcs)
 {
 	struct drm_fbdev_cma *fbdev_cma;
 	struct drm_fb_helper *helper;
@@ -534,11 +544,17 @@ struct drm_fbdev_cma *drm_fbdev_cma_init_with_funcs(struct drm_device *dev,
 		dev_err(dev->dev, "Failed to allocate drm fbdev.\n");
 		return ERR_PTR(-ENOMEM);
 	}
-	fbdev_cma->fb_funcs = funcs;
+
+	if (!framebuffer_funcs)
+		framebuffer_funcs = &drm_fb_cma_funcs;
+	if (!fb_helper_funcs)
+		fb_helper_funcs = &drm_fb_cma_helper_funcs;
+
+	fbdev_cma->fb_funcs = framebuffer_funcs;
 
 	helper = &fbdev_cma->fb_helper;
 
-	drm_fb_helper_prepare(dev, helper, &drm_fb_cma_helper_funcs);
+	drm_fb_helper_prepare(dev, helper, fb_helper_funcs);
 
 	ret = drm_fb_helper_init(dev, helper, max_conn_count);
 	if (ret < 0) {
@@ -568,6 +584,25 @@ struct drm_fbdev_cma *drm_fbdev_cma_init_with_funcs(struct drm_device *dev,
 
 	return ERR_PTR(ret);
 }
+EXPORT_SYMBOL_GPL(drm_fbdev_cma_init_with_funcs2);
+
+/**
+ * drm_fbdev_cma_init_with_funcs() - Allocate and initializes a drm_fbdev_cma struct
+ * @dev: DRM device
+ * @preferred_bpp: Preferred bits per pixel for the device
+ * @max_conn_count: Maximum number of connectors
+ * @framebuffer_funcs: framebuffer functions, in particular a custom dirty() callback
+ *
+ * Returns a newly allocated drm_fbdev_cma struct or a ERR_PTR.
+ */
+struct drm_fbdev_cma *drm_fbdev_cma_init_with_funcs(struct drm_device *dev,
+	unsigned int preferred_bpp, unsigned int max_conn_count,
+	const struct drm_framebuffer_funcs *framebuffer_funcs)
+{
+	return drm_fbdev_cma_init_with_funcs2(dev, preferred_bpp,
+					      max_conn_count,
+					      framebuffer_funcs, NULL);
+}
 EXPORT_SYMBOL_GPL(drm_fbdev_cma_init_with_funcs);
 
 /**
@@ -581,9 +616,9 @@ EXPORT_SYMBOL_GPL(drm_fbdev_cma_init_with_funcs);
 struct drm_fbdev_cma *drm_fbdev_cma_init(struct drm_device *dev,
 	unsigned int preferred_bpp, unsigned int max_conn_count)
 {
-	return drm_fbdev_cma_init_with_funcs(dev, preferred_bpp,
-					     max_conn_count,
-					     &drm_fb_cma_funcs);
+	return drm_fbdev_cma_init_with_funcs2(dev, preferred_bpp,
+					      max_conn_count,
+					      NULL, NULL);
 }
 EXPORT_SYMBOL_GPL(drm_fbdev_cma_init);
 
diff --git a/include/drm/drm_fb_cma_helper.h b/include/drm/drm_fb_cma_helper.h
index 199a63f..280ec2b 100644
--- a/include/drm/drm_fb_cma_helper.h
+++ b/include/drm/drm_fb_cma_helper.h
@@ -15,13 +15,19 @@ struct drm_mode_fb_cmd2;
 struct drm_plane;
 struct drm_plane_state;
 
+struct drm_fbdev_cma *drm_fbdev_cma_init_with_funcs2(struct drm_device *dev,
+	unsigned int preferred_bpp, unsigned int max_conn_count,
+	const struct drm_framebuffer_funcs *framebuffer_funcs,
+	const struct drm_fb_helper_funcs *fb_helper_funcs);
 struct drm_fbdev_cma *drm_fbdev_cma_init_with_funcs(struct drm_device *dev,
 	unsigned int preferred_bpp, unsigned int max_conn_count,
-	const struct drm_framebuffer_funcs *funcs);
+	const struct drm_framebuffer_funcs *framebuffer_funcs);
 struct drm_fbdev_cma *drm_fbdev_cma_init(struct drm_device *dev,
 	unsigned int preferred_bpp, unsigned int max_conn_count);
 void drm_fbdev_cma_fini(struct drm_fbdev_cma *fbdev_cma);
 
+int drm_fbdev_cma_create(struct drm_fb_helper *helper,
+	struct drm_fb_helper_surface_size *sizes);
 void drm_fbdev_cma_restore_mode(struct drm_fbdev_cma *fbdev_cma);
 void drm_fbdev_cma_hotplug_event(struct drm_fbdev_cma *fbdev_cma);
 void drm_fbdev_cma_set_suspend(struct drm_fbdev_cma *fbdev_cma, int state);
-- 
2.1.4

[RFC PATCH v2 3/3] drm: atmel-hlcdc: add clut support for legacy fbdev

[Peter Rosin]

The clut is not synchronized with the drm gamma_lut state.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 53 ++++++++++++++++++++++++++
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c   | 12 +++++-
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h   |  4 ++
 3 files changed, 67 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
index 694adcc..4bee26e 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
@@ -140,6 +140,58 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c)
 			   cfg);
 }
 
+static void
+atmel_hlcdc_crtc_load_lut(struct drm_crtc *c)
+{
+	struct atmel_hlcdc_crtc *crtc = drm_crtc_to_atmel_hlcdc_crtc(c);
+	struct atmel_hlcdc_dc *dc = crtc->dc;
+	uint16_t *red = c->gamma_store;
+	uint16_t *green = red + c->gamma_size;
+	uint16_t *blue = green + c->gamma_size;
+	int layer;
+	int idx;
+
+	for (layer = 0; layer < ATMEL_HLCDC_MAX_LAYERS; layer++) {
+		if (!dc->layers[layer])
+			continue;
+
+		for (idx = 0; idx < ATMEL_HLCDC_CLUT_SIZE; idx++) {
+			u32 val = ((red[idx] << 8) & 0xff0000) |
+				(green[idx] & 0xff00) |
+				(blue[idx] >> 8);
+
+			atmel_hlcdc_layer_write_clut(dc->layers[layer],
+						     idx, val);
+		}
+	}
+}
+
+void atmel_hlcdc_gamma_set(struct drm_crtc *c,
+			   u16 r, u16 g, u16 b, int idx)
+{
+	if (idx < 0 || idx >= c->gamma_size)
+		return;
+
+	c->gamma_store[idx] = r;
+	idx += c->gamma_size;
+	c->gamma_store[idx] = g;
+	idx += c->gamma_size;
+	c->gamma_store[idx] = b;
+}
+
+void atmel_hlcdc_gamma_get(struct drm_crtc *c,
+			   u16 *r, u16 *g, u16 *b, int idx)
+{
+	if (idx < 0 || idx >= c->gamma_size)
+		return;
+
+	*r = c->gamma_store[idx];
+	idx += c->gamma_size;
+	*g = c->gamma_store[idx];
+	idx += c->gamma_size;
+	*b = c->gamma_store[idx];
+}
+
 static enum drm_mode_status
 atmel_hlcdc_crtc_mode_valid(struct drm_crtc *c,
 			    const struct drm_display_mode *mode)
@@ -319,6 +371,7 @@ static const struct drm_crtc_helper_funcs lcdc_crtc_helper_funcs = {
 	.mode_set = drm_helper_crtc_mode_set,
 	.mode_set_nofb = atmel_hlcdc_crtc_mode_set_nofb,
 	.mode_set_base = drm_helper_crtc_mode_set_base,
+	.load_lut = atmel_hlcdc_crtc_load_lut,
 	.disable = atmel_hlcdc_crtc_disable,
 	.enable = atmel_hlcdc_crtc_enable,
 	.atomic_check = atmel_hlcdc_crtc_atomic_check,
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
index 4f6ef07..9a09c73 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
@@ -601,6 +601,12 @@ static int atmel_hlcdc_dc_modeset_init(struct drm_device *dev)
 	return 0;
 }
 
+static const struct drm_fb_helper_funcs atmel_hlcdc_fb_cma_helper_funcs = {
+	.gamma_set	= atmel_hlcdc_gamma_set,
+	.gamma_get	= atmel_hlcdc_gamma_get,
+	.fb_probe	= drm_fbdev_cma_create,
+};
+
 static int atmel_hlcdc_dc_load(struct drm_device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev->dev);
@@ -664,8 +670,10 @@ static int atmel_hlcdc_dc_load(struct drm_device *dev)
 
 	platform_set_drvdata(pdev, dev);
 
-	dc->fbdev = drm_fbdev_cma_init(dev, 24,
-			dev->mode_config.num_connector);
+	dc->fbdev = drm_fbdev_cma_init_with_funcs2(dev, 24,
+			dev->mode_config.num_connector,
+			NULL,
+			&atmel_hlcdc_fb_cma_helper_funcs);
 	if (IS_ERR(dc->fbdev))
 		dc->fbdev = NULL;
 
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
index 709f7b9..1b13224 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
@@ -32,6 +32,7 @@
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_crtc.h>
 #include <drm/drm_crtc_helper.h>
+#include <drm/drm_fb_helper.h>
 #include <drm/drm_fb_cma_helper.h>
 #include <drm/drm_gem_cma_helper.h>
 #include <drm/drm_panel.h>
@@ -448,6 +449,9 @@ void atmel_hlcdc_plane_irq(struct atmel_hlcdc_plane *plane);
 int atmel_hlcdc_plane_prepare_disc_area(struct drm_crtc_state *c_state);
 int atmel_hlcdc_plane_prepare_ahb_routing(struct drm_crtc_state *c_state);
 
+void atmel_hlcdc_gamma_set(struct drm_crtc *c, u16 r, u16 g, u16 b, int idx);
+void atmel_hlcdc_gamma_get(struct drm_crtc *c, u16 *r, u16 *g, u16 *b, int idx);
+
 void atmel_hlcdc_crtc_irq(struct drm_crtc *c);
 
 int atmel_hlcdc_crtc_create(struct drm_device *dev);
-- 
2.1.4

Re: [RFC PATCH v2 3/3] drm: atmel-hlcdc: add clut support for legacy fbdev

[Daniel Vetter]

On Sat, Jun 17, 2017 at 07:48:04PM +0200, Peter Rosin wrote:
> The clut is not synchronized with the drm gamma_lut state.
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>

This needs to be done in the fbdev helper, not like this. Yes it's an old
issue, but forcing every driver to duplicate code like this isn't cool.

Thanks, Daniel

> ---
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 53 ++++++++++++++++++++++++++
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c   | 12 +++++-
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h   |  4 ++
>  3 files changed, 67 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> index 694adcc..4bee26e 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> @@ -140,6 +140,58 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c)
>  			   cfg);
>  }
>  
> +static void
> +atmel_hlcdc_crtc_load_lut(struct drm_crtc *c)
> +{
> +	struct atmel_hlcdc_crtc *crtc = drm_crtc_to_atmel_hlcdc_crtc(c);
> +	struct atmel_hlcdc_dc *dc = crtc->dc;
> +	uint16_t *red = c->gamma_store;
> +	uint16_t *green = red + c->gamma_size;
> +	uint16_t *blue = green + c->gamma_size;
> +	int layer;
> +	int idx;
> +
> +	for (layer = 0; layer < ATMEL_HLCDC_MAX_LAYERS; layer++) {
> +		if (!dc->layers[layer])
> +			continue;
> +
> +		for (idx = 0; idx < ATMEL_HLCDC_CLUT_SIZE; idx++) {
> +			u32 val = ((red[idx] << 8) & 0xff0000) |
> +				(green[idx] & 0xff00) |
> +				(blue[idx] >> 8);
> +
> +			atmel_hlcdc_layer_write_clut(dc->layers[layer],
> +						     idx, val);
> +		}
> +	}
> +}
> +
> +void atmel_hlcdc_gamma_set(struct drm_crtc *c,
> +			   u16 r, u16 g, u16 b, int idx)
> +{
> +	if (idx < 0 || idx >= c->gamma_size)
> +		return;
> +
> +	c->gamma_store[idx] = r;
> +	idx += c->gamma_size;
> +	c->gamma_store[idx] = g;
> +	idx += c->gamma_size;
> +	c->gamma_store[idx] = b;
> +}
> +
> +void atmel_hlcdc_gamma_get(struct drm_crtc *c,
> +			   u16 *r, u16 *g, u16 *b, int idx)
> +{
> +	if (idx < 0 || idx >= c->gamma_size)
> +		return;
> +
> +	*r = c->gamma_store[idx];
> +	idx += c->gamma_size;
> +	*g = c->gamma_store[idx];
> +	idx += c->gamma_size;
> +	*b = c->gamma_store[idx];
> +}
> +
>  static enum drm_mode_status
>  atmel_hlcdc_crtc_mode_valid(struct drm_crtc *c,
>  			    const struct drm_display_mode *mode)
> @@ -319,6 +371,7 @@ static const struct drm_crtc_helper_funcs lcdc_crtc_helper_funcs = {
>  	.mode_set = drm_helper_crtc_mode_set,
>  	.mode_set_nofb = atmel_hlcdc_crtc_mode_set_nofb,
>  	.mode_set_base = drm_helper_crtc_mode_set_base,
> +	.load_lut = atmel_hlcdc_crtc_load_lut,
>  	.disable = atmel_hlcdc_crtc_disable,
>  	.enable = atmel_hlcdc_crtc_enable,
>  	.atomic_check = atmel_hlcdc_crtc_atomic_check,
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> index 4f6ef07..9a09c73 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> @@ -601,6 +601,12 @@ static int atmel_hlcdc_dc_modeset_init(struct drm_device *dev)
>  	return 0;
>  }
>  
> +static const struct drm_fb_helper_funcs atmel_hlcdc_fb_cma_helper_funcs = {
> +	.gamma_set	= atmel_hlcdc_gamma_set,
> +	.gamma_get	= atmel_hlcdc_gamma_get,
> +	.fb_probe	= drm_fbdev_cma_create,
> +};
> +
>  static int atmel_hlcdc_dc_load(struct drm_device *dev)
>  {
>  	struct platform_device *pdev = to_platform_device(dev->dev);
> @@ -664,8 +670,10 @@ static int atmel_hlcdc_dc_load(struct drm_device *dev)
>  
>  	platform_set_drvdata(pdev, dev);
>  
> -	dc->fbdev = drm_fbdev_cma_init(dev, 24,
> -			dev->mode_config.num_connector);
> +	dc->fbdev = drm_fbdev_cma_init_with_funcs2(dev, 24,
> +			dev->mode_config.num_connector,
> +			NULL,
> +			&atmel_hlcdc_fb_cma_helper_funcs);
>  	if (IS_ERR(dc->fbdev))
>  		dc->fbdev = NULL;
>  
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> index 709f7b9..1b13224 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> @@ -32,6 +32,7 @@
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_crtc.h>
>  #include <drm/drm_crtc_helper.h>
> +#include <drm/drm_fb_helper.h>
>  #include <drm/drm_fb_cma_helper.h>
>  #include <drm/drm_gem_cma_helper.h>
>  #include <drm/drm_panel.h>
> @@ -448,6 +449,9 @@ void atmel_hlcdc_plane_irq(struct atmel_hlcdc_plane *plane);
>  int atmel_hlcdc_plane_prepare_disc_area(struct drm_crtc_state *c_state);
>  int atmel_hlcdc_plane_prepare_ahb_routing(struct drm_crtc_state *c_state);
>  
> +void atmel_hlcdc_gamma_set(struct drm_crtc *c, u16 r, u16 g, u16 b, int idx);
> +void atmel_hlcdc_gamma_get(struct drm_crtc *c, u16 *r, u16 *g, u16 *b, int idx);
> +
>  void atmel_hlcdc_crtc_irq(struct drm_crtc *c);
>  
>  int atmel_hlcdc_crtc_create(struct drm_device *dev);
> -- 
> 2.1.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [RFC PATCH v2 3/3] drm: atmel-hlcdc: add clut support for legacy fbdev

[Daniel Vetter]

On Sat, Jun 17, 2017 at 07:48:04PM +0200, Peter Rosin wrote:
> The clut is not synchronized with the drm gamma_lut state.
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>

This needs to be done in the fbdev helper, not like this. Yes it's an old
issue, but forcing every driver to duplicate code like this isn't cool.

Thanks, Daniel

> ---
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 53 ++++++++++++++++++++++++++
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c   | 12 +++++-
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h   |  4 ++
>  3 files changed, 67 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> index 694adcc..4bee26e 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> @@ -140,6 +140,58 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c)
>  			   cfg);
>  }
>  
> +static void
> +atmel_hlcdc_crtc_load_lut(struct drm_crtc *c)
> +{
> +	struct atmel_hlcdc_crtc *crtc = drm_crtc_to_atmel_hlcdc_crtc(c);
> +	struct atmel_hlcdc_dc *dc = crtc->dc;
> +	uint16_t *red = c->gamma_store;
> +	uint16_t *green = red + c->gamma_size;
> +	uint16_t *blue = green + c->gamma_size;
> +	int layer;
> +	int idx;
> +
> +	for (layer = 0; layer < ATMEL_HLCDC_MAX_LAYERS; layer++) {
> +		if (!dc->layers[layer])
> +			continue;
> +
> +		for (idx = 0; idx < ATMEL_HLCDC_CLUT_SIZE; idx++) {
> +			u32 val = ((red[idx] << 8) & 0xff0000) |
> +				(green[idx] & 0xff00) |
> +				(blue[idx] >> 8);
> +
> +			atmel_hlcdc_layer_write_clut(dc->layers[layer],
> +						     idx, val);
> +		}
> +	}
> +}
> +
> +void atmel_hlcdc_gamma_set(struct drm_crtc *c,
> +			   u16 r, u16 g, u16 b, int idx)
> +{
> +	if (idx < 0 || idx >= c->gamma_size)
> +		return;
> +
> +	c->gamma_store[idx] = r;
> +	idx += c->gamma_size;
> +	c->gamma_store[idx] = g;
> +	idx += c->gamma_size;
> +	c->gamma_store[idx] = b;
> +}
> +
> +void atmel_hlcdc_gamma_get(struct drm_crtc *c,
> +			   u16 *r, u16 *g, u16 *b, int idx)
> +{
> +	if (idx < 0 || idx >= c->gamma_size)
> +		return;
> +
> +	*r = c->gamma_store[idx];
> +	idx += c->gamma_size;
> +	*g = c->gamma_store[idx];
> +	idx += c->gamma_size;
> +	*b = c->gamma_store[idx];
> +}
> +
>  static enum drm_mode_status
>  atmel_hlcdc_crtc_mode_valid(struct drm_crtc *c,
>  			    const struct drm_display_mode *mode)
> @@ -319,6 +371,7 @@ static const struct drm_crtc_helper_funcs lcdc_crtc_helper_funcs = {
>  	.mode_set = drm_helper_crtc_mode_set,
>  	.mode_set_nofb = atmel_hlcdc_crtc_mode_set_nofb,
>  	.mode_set_base = drm_helper_crtc_mode_set_base,
> +	.load_lut = atmel_hlcdc_crtc_load_lut,
>  	.disable = atmel_hlcdc_crtc_disable,
>  	.enable = atmel_hlcdc_crtc_enable,
>  	.atomic_check = atmel_hlcdc_crtc_atomic_check,
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> index 4f6ef07..9a09c73 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> @@ -601,6 +601,12 @@ static int atmel_hlcdc_dc_modeset_init(struct drm_device *dev)
>  	return 0;
>  }
>  
> +static const struct drm_fb_helper_funcs atmel_hlcdc_fb_cma_helper_funcs = {
> +	.gamma_set	= atmel_hlcdc_gamma_set,
> +	.gamma_get	= atmel_hlcdc_gamma_get,
> +	.fb_probe	= drm_fbdev_cma_create,
> +};
> +
>  static int atmel_hlcdc_dc_load(struct drm_device *dev)
>  {
>  	struct platform_device *pdev = to_platform_device(dev->dev);
> @@ -664,8 +670,10 @@ static int atmel_hlcdc_dc_load(struct drm_device *dev)
>  
>  	platform_set_drvdata(pdev, dev);
>  
> -	dc->fbdev = drm_fbdev_cma_init(dev, 24,
> -			dev->mode_config.num_connector);
> +	dc->fbdev = drm_fbdev_cma_init_with_funcs2(dev, 24,
> +			dev->mode_config.num_connector,
> +			NULL,
> +			&atmel_hlcdc_fb_cma_helper_funcs);
>  	if (IS_ERR(dc->fbdev))
>  		dc->fbdev = NULL;
>  
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> index 709f7b9..1b13224 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> @@ -32,6 +32,7 @@
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_crtc.h>
>  #include <drm/drm_crtc_helper.h>
> +#include <drm/drm_fb_helper.h>
>  #include <drm/drm_fb_cma_helper.h>
>  #include <drm/drm_gem_cma_helper.h>
>  #include <drm/drm_panel.h>
> @@ -448,6 +449,9 @@ void atmel_hlcdc_plane_irq(struct atmel_hlcdc_plane *plane);
>  int atmel_hlcdc_plane_prepare_disc_area(struct drm_crtc_state *c_state);
>  int atmel_hlcdc_plane_prepare_ahb_routing(struct drm_crtc_state *c_state);
>  
> +void atmel_hlcdc_gamma_set(struct drm_crtc *c, u16 r, u16 g, u16 b, int idx);
> +void atmel_hlcdc_gamma_get(struct drm_crtc *c, u16 *r, u16 *g, u16 *b, int idx);
> +
>  void atmel_hlcdc_crtc_irq(struct drm_crtc *c);
>  
>  int atmel_hlcdc_crtc_create(struct drm_device *dev);
> -- 
> 2.1.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCH v2 1/3] drm: atmel-hlcdc: add support for 8-bit color lookup table mode

[Daniel Vetter]

On Sat, Jun 17, 2017 at 07:48:02PM +0200, Peter Rosin wrote:
> All layers of all supported chips support this, the only variable is the
> base address of the lookup table in the register map.
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>

As Boris said, pls use the new color manager stuff for atomic drivers, and
then use the helper to implement the legacy kms LUT support on top of it,
it's drm_atomic_helper_legacy_gamma_set().
-Daniel

> ---
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c  |  5 +++++
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c    | 13 +++++++++++
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h    | 16 ++++++++++++++
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 29 +++++++++++++++++++++++++
>  4 files changed, 63 insertions(+)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> index 5348985..694adcc 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> @@ -429,6 +429,8 @@ static const struct drm_crtc_funcs atmel_hlcdc_crtc_funcs = {
>  	.atomic_destroy_state = atmel_hlcdc_crtc_destroy_state,
>  	.enable_vblank = atmel_hlcdc_crtc_enable_vblank,
>  	.disable_vblank = atmel_hlcdc_crtc_disable_vblank,
> +	.set_property = drm_atomic_helper_crtc_set_property,
> +	.gamma_set = drm_atomic_helper_legacy_gamma_set,
>  };
>  
>  int atmel_hlcdc_crtc_create(struct drm_device *dev)
> @@ -484,6 +486,9 @@ int atmel_hlcdc_crtc_create(struct drm_device *dev)
>  	drm_crtc_helper_add(&crtc->base, &lcdc_crtc_helper_funcs);
>  	drm_crtc_vblank_reset(&crtc->base);
>  
> +	drm_mode_crtc_set_gamma_size(&crtc->base, ATMEL_HLCDC_CLUT_SIZE);
> +	drm_crtc_enable_color_mgmt(&crtc->base, 0, false, ATMEL_HLCDC_CLUT_SIZE);
> +
>  	dc->crtc = &crtc->base;
>  
>  	return 0;
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> index 30dbffd..4f6ef07 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> @@ -42,6 +42,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_at91sam9n12_layers[] = {
>  			.default_color = 3,
>  			.general_config = 4,
>  		},
> +		.clut_offset = 0x400,
>  	},
>  };
>  
> @@ -73,6 +74,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_at91sam9x5_layers[] = {
>  			.disc_pos = 5,
>  			.disc_size = 6,
>  		},
> +		.clut_offset = 0x400,
>  	},
>  	{
>  		.name = "overlay1",
> @@ -91,6 +93,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_at91sam9x5_layers[] = {
>  			.chroma_key_mask = 8,
>  			.general_config = 9,
>  		},
> +		.clut_offset = 0x800,
>  	},
>  	{
>  		.name = "high-end-overlay",
> @@ -112,6 +115,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_at91sam9x5_layers[] = {
>  			.scaler_config = 13,
>  			.csc = 14,
>  		},
> +		.clut_offset = 0x1000,
>  	},
>  	{
>  		.name = "cursor",
> @@ -131,6 +135,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_at91sam9x5_layers[] = {
>  			.chroma_key_mask = 8,
>  			.general_config = 9,
>  		},
> +		.clut_offset = 0x1400,
>  	},
>  };
>  
> @@ -162,6 +167,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d3_layers[] = {
>  			.disc_pos = 5,
>  			.disc_size = 6,
>  		},
> +		.clut_offset = 0x600,
>  	},
>  	{
>  		.name = "overlay1",
> @@ -180,6 +186,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d3_layers[] = {
>  			.chroma_key_mask = 8,
>  			.general_config = 9,
>  		},
> +		.clut_offset = 0xa00,
>  	},
>  	{
>  		.name = "overlay2",
> @@ -198,6 +205,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d3_layers[] = {
>  			.chroma_key_mask = 8,
>  			.general_config = 9,
>  		},
> +		.clut_offset = 0xe00,
>  	},
>  	{
>  		.name = "high-end-overlay",
> @@ -223,6 +231,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d3_layers[] = {
>  			},
>  			.csc = 14,
>  		},
> +		.clut_offset = 0x1200,
>  	},
>  	{
>  		.name = "cursor",
> @@ -244,6 +253,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d3_layers[] = {
>  			.general_config = 9,
>  			.scaler_config = 13,
>  		},
> +		.clut_offset = 0x1600,
>  	},
>  };
>  
> @@ -275,6 +285,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d4_layers[] = {
>  			.disc_pos = 5,
>  			.disc_size = 6,
>  		},
> +		.clut_offset = 0x600,
>  	},
>  	{
>  		.name = "overlay1",
> @@ -293,6 +304,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d4_layers[] = {
>  			.chroma_key_mask = 8,
>  			.general_config = 9,
>  		},
> +		.clut_offset = 0xa00,
>  	},
>  	{
>  		.name = "overlay2",
> @@ -336,6 +348,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d4_layers[] = {
>  			},
>  			.csc = 14,
>  		},
> +		.clut_offset = 0x1200,
>  	},
>  };
>  
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> index b0596a8..709f7b9 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> @@ -88,6 +88,11 @@
>  #define ATMEL_HLCDC_YUV422SWP			BIT(17)
>  #define ATMEL_HLCDC_DSCALEOPT			BIT(20)
>  
> +#define ATMEL_HLCDC_C1_MODE			ATMEL_HLCDC_CLUT_MODE(0)
> +#define ATMEL_HLCDC_C2_MODE			ATMEL_HLCDC_CLUT_MODE(1)
> +#define ATMEL_HLCDC_C4_MODE			ATMEL_HLCDC_CLUT_MODE(2)
> +#define ATMEL_HLCDC_C8_MODE			ATMEL_HLCDC_CLUT_MODE(3)
> +
>  #define ATMEL_HLCDC_XRGB4444_MODE		ATMEL_HLCDC_RGB_MODE(0)
>  #define ATMEL_HLCDC_ARGB4444_MODE		ATMEL_HLCDC_RGB_MODE(1)
>  #define ATMEL_HLCDC_RGBA4444_MODE		ATMEL_HLCDC_RGB_MODE(2)
> @@ -142,6 +147,8 @@
>  #define ATMEL_HLCDC_DMA_CHANNEL_DSCR_DONE	BIT(2)
>  #define ATMEL_HLCDC_DMA_CHANNEL_DSCR_OVERRUN	BIT(3)
>  
> +#define ATMEL_HLCDC_CLUT_SIZE			256
> +
>  #define ATMEL_HLCDC_MAX_LAYERS			6
>  
>  /**
> @@ -259,6 +266,7 @@ struct atmel_hlcdc_layer_desc {
>  	int id;
>  	int regs_offset;
>  	int cfgs_offset;
> +	int clut_offset;
>  	struct atmel_hlcdc_formats *formats;
>  	struct atmel_hlcdc_layer_cfg_layout layout;
>  	int max_width;
> @@ -414,6 +422,14 @@ static inline u32 atmel_hlcdc_layer_read_cfg(struct atmel_hlcdc_layer *layer,
>  					  (cfgid * sizeof(u32)));
>  }
>  
> +static inline void atmel_hlcdc_layer_write_clut(struct atmel_hlcdc_layer *layer,
> +						unsigned int c, u32 val)
> +{
> +	atmel_hlcdc_layer_write_reg(layer,
> +				    layer->desc->clut_offset + c * sizeof(u32),
> +				    val);
> +}
> +
>  static inline void atmel_hlcdc_layer_init(struct atmel_hlcdc_layer *layer,
>  				const struct atmel_hlcdc_layer_desc *desc,
>  				struct regmap *regmap)
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> index 1124200..b5bd9b0 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> @@ -83,6 +83,7 @@ drm_plane_state_to_atmel_hlcdc_plane_state(struct drm_plane_state *s)
>  #define SUBPIXEL_MASK			0xffff
>  
>  static uint32_t rgb_formats[] = {
> +	DRM_FORMAT_C8,
>  	DRM_FORMAT_XRGB4444,
>  	DRM_FORMAT_ARGB4444,
>  	DRM_FORMAT_RGBA4444,
> @@ -100,6 +101,7 @@ struct atmel_hlcdc_formats atmel_hlcdc_plane_rgb_formats = {
>  };
>  
>  static uint32_t rgb_and_yuv_formats[] = {
> +	DRM_FORMAT_C8,
>  	DRM_FORMAT_XRGB4444,
>  	DRM_FORMAT_ARGB4444,
>  	DRM_FORMAT_RGBA4444,
> @@ -128,6 +130,9 @@ struct atmel_hlcdc_formats atmel_hlcdc_plane_rgb_and_yuv_formats = {
>  static int atmel_hlcdc_format_to_plane_mode(u32 format, u32 *mode)
>  {
>  	switch (format) {
> +	case DRM_FORMAT_C8:
> +		*mode = ATMEL_HLCDC_C8_MODE;
> +		break;
>  	case DRM_FORMAT_XRGB4444:
>  		*mode = ATMEL_HLCDC_XRGB4444_MODE;
>  		break;
> @@ -424,6 +429,29 @@ static void atmel_hlcdc_plane_update_format(struct atmel_hlcdc_plane *plane,
>  				    ATMEL_HLCDC_LAYER_FORMAT_CFG, cfg);
>  }
>  
> +static void atmel_hlcdc_plane_update_clut(struct atmel_hlcdc_plane *plane)
> +{
> +	struct drm_crtc *crtc = plane->base.crtc;
> +	struct drm_color_lut *lut;
> +	int idx;
> +
> +	if (!crtc || !crtc->state)
> +		return;
> +
> +	if (!crtc->state->color_mgmt_changed || !crtc->state->gamma_lut)
> +		return;
> +
> +	lut = (struct drm_color_lut *)crtc->state->gamma_lut->data;
> +
> +	for (idx = 0; idx < ATMEL_HLCDC_CLUT_SIZE; idx++, lut++) {
> +		u32 val = ((lut->red << 8) & 0xff0000) |
> +			(lut->green & 0xff00) |
> +			(lut->blue >> 8);
> +
> +		atmel_hlcdc_layer_write_clut(&plane->layer, idx, val);
> +	}
> +}
> +
>  static void atmel_hlcdc_plane_update_buffers(struct atmel_hlcdc_plane *plane,
>  					struct atmel_hlcdc_plane_state *state)
>  {
> @@ -768,6 +796,7 @@ static void atmel_hlcdc_plane_atomic_update(struct drm_plane *p,
>  	atmel_hlcdc_plane_update_pos_and_size(plane, state);
>  	atmel_hlcdc_plane_update_general_settings(plane, state);
>  	atmel_hlcdc_plane_update_format(plane, state);
> +	atmel_hlcdc_plane_update_clut(plane);
>  	atmel_hlcdc_plane_update_buffers(plane, state);
>  	atmel_hlcdc_plane_update_disc_area(plane, state);
>  
> -- 
> 2.1.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [RFC PATCH v2 1/3] drm: atmel-hlcdc: add support for 8-bit color lookup table mode

[Daniel Vetter]

On Sat, Jun 17, 2017 at 07:48:02PM +0200, Peter Rosin wrote:
> All layers of all supported chips support this, the only variable is the
> base address of the lookup table in the register map.
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>

As Boris said, pls use the new color manager stuff for atomic drivers, and
then use the helper to implement the legacy kms LUT support on top of it,
it's drm_atomic_helper_legacy_gamma_set().
-Daniel

> ---
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c  |  5 +++++
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c    | 13 +++++++++++
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h    | 16 ++++++++++++++
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 29 +++++++++++++++++++++++++
>  4 files changed, 63 insertions(+)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> index 5348985..694adcc 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> @@ -429,6 +429,8 @@ static const struct drm_crtc_funcs atmel_hlcdc_crtc_funcs = {
>  	.atomic_destroy_state = atmel_hlcdc_crtc_destroy_state,
>  	.enable_vblank = atmel_hlcdc_crtc_enable_vblank,
>  	.disable_vblank = atmel_hlcdc_crtc_disable_vblank,
> +	.set_property = drm_atomic_helper_crtc_set_property,
> +	.gamma_set = drm_atomic_helper_legacy_gamma_set,
>  };
>  
>  int atmel_hlcdc_crtc_create(struct drm_device *dev)
> @@ -484,6 +486,9 @@ int atmel_hlcdc_crtc_create(struct drm_device *dev)
>  	drm_crtc_helper_add(&crtc->base, &lcdc_crtc_helper_funcs);
>  	drm_crtc_vblank_reset(&crtc->base);
>  
> +	drm_mode_crtc_set_gamma_size(&crtc->base, ATMEL_HLCDC_CLUT_SIZE);
> +	drm_crtc_enable_color_mgmt(&crtc->base, 0, false, ATMEL_HLCDC_CLUT_SIZE);
> +
>  	dc->crtc = &crtc->base;
>  
>  	return 0;
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> index 30dbffd..4f6ef07 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> @@ -42,6 +42,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_at91sam9n12_layers[] = {
>  			.default_color = 3,
>  			.general_config = 4,
>  		},
> +		.clut_offset = 0x400,
>  	},
>  };
>  
> @@ -73,6 +74,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_at91sam9x5_layers[] = {
>  			.disc_pos = 5,
>  			.disc_size = 6,
>  		},
> +		.clut_offset = 0x400,
>  	},
>  	{
>  		.name = "overlay1",
> @@ -91,6 +93,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_at91sam9x5_layers[] = {
>  			.chroma_key_mask = 8,
>  			.general_config = 9,
>  		},
> +		.clut_offset = 0x800,
>  	},
>  	{
>  		.name = "high-end-overlay",
> @@ -112,6 +115,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_at91sam9x5_layers[] = {
>  			.scaler_config = 13,
>  			.csc = 14,
>  		},
> +		.clut_offset = 0x1000,
>  	},
>  	{
>  		.name = "cursor",
> @@ -131,6 +135,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_at91sam9x5_layers[] = {
>  			.chroma_key_mask = 8,
>  			.general_config = 9,
>  		},
> +		.clut_offset = 0x1400,
>  	},
>  };
>  
> @@ -162,6 +167,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d3_layers[] = {
>  			.disc_pos = 5,
>  			.disc_size = 6,
>  		},
> +		.clut_offset = 0x600,
>  	},
>  	{
>  		.name = "overlay1",
> @@ -180,6 +186,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d3_layers[] = {
>  			.chroma_key_mask = 8,
>  			.general_config = 9,
>  		},
> +		.clut_offset = 0xa00,
>  	},
>  	{
>  		.name = "overlay2",
> @@ -198,6 +205,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d3_layers[] = {
>  			.chroma_key_mask = 8,
>  			.general_config = 9,
>  		},
> +		.clut_offset = 0xe00,
>  	},
>  	{
>  		.name = "high-end-overlay",
> @@ -223,6 +231,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d3_layers[] = {
>  			},
>  			.csc = 14,
>  		},
> +		.clut_offset = 0x1200,
>  	},
>  	{
>  		.name = "cursor",
> @@ -244,6 +253,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d3_layers[] = {
>  			.general_config = 9,
>  			.scaler_config = 13,
>  		},
> +		.clut_offset = 0x1600,
>  	},
>  };
>  
> @@ -275,6 +285,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d4_layers[] = {
>  			.disc_pos = 5,
>  			.disc_size = 6,
>  		},
> +		.clut_offset = 0x600,
>  	},
>  	{
>  		.name = "overlay1",
> @@ -293,6 +304,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d4_layers[] = {
>  			.chroma_key_mask = 8,
>  			.general_config = 9,
>  		},
> +		.clut_offset = 0xa00,
>  	},
>  	{
>  		.name = "overlay2",
> @@ -336,6 +348,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d4_layers[] = {
>  			},
>  			.csc = 14,
>  		},
> +		.clut_offset = 0x1200,
>  	},
>  };
>  
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> index b0596a8..709f7b9 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> @@ -88,6 +88,11 @@
>  #define ATMEL_HLCDC_YUV422SWP			BIT(17)
>  #define ATMEL_HLCDC_DSCALEOPT			BIT(20)
>  
> +#define ATMEL_HLCDC_C1_MODE			ATMEL_HLCDC_CLUT_MODE(0)
> +#define ATMEL_HLCDC_C2_MODE			ATMEL_HLCDC_CLUT_MODE(1)
> +#define ATMEL_HLCDC_C4_MODE			ATMEL_HLCDC_CLUT_MODE(2)
> +#define ATMEL_HLCDC_C8_MODE			ATMEL_HLCDC_CLUT_MODE(3)
> +
>  #define ATMEL_HLCDC_XRGB4444_MODE		ATMEL_HLCDC_RGB_MODE(0)
>  #define ATMEL_HLCDC_ARGB4444_MODE		ATMEL_HLCDC_RGB_MODE(1)
>  #define ATMEL_HLCDC_RGBA4444_MODE		ATMEL_HLCDC_RGB_MODE(2)
> @@ -142,6 +147,8 @@
>  #define ATMEL_HLCDC_DMA_CHANNEL_DSCR_DONE	BIT(2)
>  #define ATMEL_HLCDC_DMA_CHANNEL_DSCR_OVERRUN	BIT(3)
>  
> +#define ATMEL_HLCDC_CLUT_SIZE			256
> +
>  #define ATMEL_HLCDC_MAX_LAYERS			6
>  
>  /**
> @@ -259,6 +266,7 @@ struct atmel_hlcdc_layer_desc {
>  	int id;
>  	int regs_offset;
>  	int cfgs_offset;
> +	int clut_offset;
>  	struct atmel_hlcdc_formats *formats;
>  	struct atmel_hlcdc_layer_cfg_layout layout;
>  	int max_width;
> @@ -414,6 +422,14 @@ static inline u32 atmel_hlcdc_layer_read_cfg(struct atmel_hlcdc_layer *layer,
>  					  (cfgid * sizeof(u32)));
>  }
>  
> +static inline void atmel_hlcdc_layer_write_clut(struct atmel_hlcdc_layer *layer,
> +						unsigned int c, u32 val)
> +{
> +	atmel_hlcdc_layer_write_reg(layer,
> +				    layer->desc->clut_offset + c * sizeof(u32),
> +				    val);
> +}
> +
>  static inline void atmel_hlcdc_layer_init(struct atmel_hlcdc_layer *layer,
>  				const struct atmel_hlcdc_layer_desc *desc,
>  				struct regmap *regmap)
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> index 1124200..b5bd9b0 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> @@ -83,6 +83,7 @@ drm_plane_state_to_atmel_hlcdc_plane_state(struct drm_plane_state *s)
>  #define SUBPIXEL_MASK			0xffff
>  
>  static uint32_t rgb_formats[] = {
> +	DRM_FORMAT_C8,
>  	DRM_FORMAT_XRGB4444,
>  	DRM_FORMAT_ARGB4444,
>  	DRM_FORMAT_RGBA4444,
> @@ -100,6 +101,7 @@ struct atmel_hlcdc_formats atmel_hlcdc_plane_rgb_formats = {
>  };
>  
>  static uint32_t rgb_and_yuv_formats[] = {
> +	DRM_FORMAT_C8,
>  	DRM_FORMAT_XRGB4444,
>  	DRM_FORMAT_ARGB4444,
>  	DRM_FORMAT_RGBA4444,
> @@ -128,6 +130,9 @@ struct atmel_hlcdc_formats atmel_hlcdc_plane_rgb_and_yuv_formats = {
>  static int atmel_hlcdc_format_to_plane_mode(u32 format, u32 *mode)
>  {
>  	switch (format) {
> +	case DRM_FORMAT_C8:
> +		*mode = ATMEL_HLCDC_C8_MODE;
> +		break;
>  	case DRM_FORMAT_XRGB4444:
>  		*mode = ATMEL_HLCDC_XRGB4444_MODE;
>  		break;
> @@ -424,6 +429,29 @@ static void atmel_hlcdc_plane_update_format(struct atmel_hlcdc_plane *plane,
>  				    ATMEL_HLCDC_LAYER_FORMAT_CFG, cfg);
>  }
>  
> +static void atmel_hlcdc_plane_update_clut(struct atmel_hlcdc_plane *plane)
> +{
> +	struct drm_crtc *crtc = plane->base.crtc;
> +	struct drm_color_lut *lut;
> +	int idx;
> +
> +	if (!crtc || !crtc->state)
> +		return;
> +
> +	if (!crtc->state->color_mgmt_changed || !crtc->state->gamma_lut)
> +		return;
> +
> +	lut = (struct drm_color_lut *)crtc->state->gamma_lut->data;
> +
> +	for (idx = 0; idx < ATMEL_HLCDC_CLUT_SIZE; idx++, lut++) {
> +		u32 val = ((lut->red << 8) & 0xff0000) |
> +			(lut->green & 0xff00) |
> +			(lut->blue >> 8);
> +
> +		atmel_hlcdc_layer_write_clut(&plane->layer, idx, val);
> +	}
> +}
> +
>  static void atmel_hlcdc_plane_update_buffers(struct atmel_hlcdc_plane *plane,
>  					struct atmel_hlcdc_plane_state *state)
>  {
> @@ -768,6 +796,7 @@ static void atmel_hlcdc_plane_atomic_update(struct drm_plane *p,
>  	atmel_hlcdc_plane_update_pos_and_size(plane, state);
>  	atmel_hlcdc_plane_update_general_settings(plane, state);
>  	atmel_hlcdc_plane_update_format(plane, state);
> +	atmel_hlcdc_plane_update_clut(plane);
>  	atmel_hlcdc_plane_update_buffers(plane, state);
>  	atmel_hlcdc_plane_update_disc_area(plane, state);
>  
> -- 
> 2.1.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCH v2 1/3] drm: atmel-hlcdc: add support for 8-bit color lookup table mode

[Peter Rosin]

On 2017-06-20 11:40, Daniel Vetter wrote:
> On Sat, Jun 17, 2017 at 07:48:02PM +0200, Peter Rosin wrote:
>> All layers of all supported chips support this, the only variable is the
>> base address of the lookup table in the register map.
>>
>> Signed-off-by: Peter Rosin <peda@axentia.se>
> 
> As Boris said, pls use the new color manager stuff for atomic drivers, and
> then use the helper to implement the legacy kms LUT support on top of it,
> it's drm_atomic_helper_legacy_gamma_set().
> -Daniel

Hmm, I don't see how this comment applies to this patch? Isn't this
patch good as is, because I thought I already did wire this up with
the drm_atomic_helper_legacy_gamma_set helper as requested?
(sure, v3 has a register offset tweak, but that's just a detail)

I totally agree that 2/3 and 3/3 are ugly and I am working on that, but
if 1/3 is also not in the right direction I'd like to know ASAP. Thanks!

Cheers,
peda

> 
>> ---
>>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c  |  5 +++++
>>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c    | 13 +++++++++++
>>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h    | 16 ++++++++++++++
>>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 29 +++++++++++++++++++++++++
>>  4 files changed, 63 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
>> index 5348985..694adcc 100644
>> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
>> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
>> @@ -429,6 +429,8 @@ static const struct drm_crtc_funcs atmel_hlcdc_crtc_funcs = {
>>  	.atomic_destroy_state = atmel_hlcdc_crtc_destroy_state,
>>  	.enable_vblank = atmel_hlcdc_crtc_enable_vblank,
>>  	.disable_vblank = atmel_hlcdc_crtc_disable_vblank,
>> +	.set_property = drm_atomic_helper_crtc_set_property,
>> +	.gamma_set = drm_atomic_helper_legacy_gamma_set,
>>  };
>>  
>>  int atmel_hlcdc_crtc_create(struct drm_device *dev)
>> @@ -484,6 +486,9 @@ int atmel_hlcdc_crtc_create(struct drm_device *dev)
>>  	drm_crtc_helper_add(&crtc->base, &lcdc_crtc_helper_funcs);
>>  	drm_crtc_vblank_reset(&crtc->base);
>>  
>> +	drm_mode_crtc_set_gamma_size(&crtc->base, ATMEL_HLCDC_CLUT_SIZE);
>> +	drm_crtc_enable_color_mgmt(&crtc->base, 0, false, ATMEL_HLCDC_CLUT_SIZE);
>> +
>>  	dc->crtc = &crtc->base;
>>  
>>  	return 0;
>> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
>> index 30dbffd..4f6ef07 100644
>> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
>> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
>> @@ -42,6 +42,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_at91sam9n12_layers[] = {
>>  			.default_color = 3,
>>  			.general_config = 4,
>>  		},
>> +		.clut_offset = 0x400,
>>  	},
>>  };
>>  
>> @@ -73,6 +74,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_at91sam9x5_layers[] = {
>>  			.disc_pos = 5,
>>  			.disc_size = 6,
>>  		},
>> +		.clut_offset = 0x400,
>>  	},
>>  	{
>>  		.name = "overlay1",
>> @@ -91,6 +93,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_at91sam9x5_layers[] = {
>>  			.chroma_key_mask = 8,
>>  			.general_config = 9,
>>  		},
>> +		.clut_offset = 0x800,
>>  	},
>>  	{
>>  		.name = "high-end-overlay",
>> @@ -112,6 +115,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_at91sam9x5_layers[] = {
>>  			.scaler_config = 13,
>>  			.csc = 14,
>>  		},
>> +		.clut_offset = 0x1000,
>>  	},
>>  	{
>>  		.name = "cursor",
>> @@ -131,6 +135,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_at91sam9x5_layers[] = {
>>  			.chroma_key_mask = 8,
>>  			.general_config = 9,
>>  		},
>> +		.clut_offset = 0x1400,
>>  	},
>>  };
>>  
>> @@ -162,6 +167,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d3_layers[] = {
>>  			.disc_pos = 5,
>>  			.disc_size = 6,
>>  		},
>> +		.clut_offset = 0x600,
>>  	},
>>  	{
>>  		.name = "overlay1",
>> @@ -180,6 +186,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d3_layers[] = {
>>  			.chroma_key_mask = 8,
>>  			.general_config = 9,
>>  		},
>> +		.clut_offset = 0xa00,
>>  	},
>>  	{
>>  		.name = "overlay2",
>> @@ -198,6 +205,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d3_layers[] = {
>>  			.chroma_key_mask = 8,
>>  			.general_config = 9,
>>  		},
>> +		.clut_offset = 0xe00,
>>  	},
>>  	{
>>  		.name = "high-end-overlay",
>> @@ -223,6 +231,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d3_layers[] = {
>>  			},
>>  			.csc = 14,
>>  		},
>> +		.clut_offset = 0x1200,
>>  	},
>>  	{
>>  		.name = "cursor",
>> @@ -244,6 +253,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d3_layers[] = {
>>  			.general_config = 9,
>>  			.scaler_config = 13,
>>  		},
>> +		.clut_offset = 0x1600,
>>  	},
>>  };
>>  
>> @@ -275,6 +285,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d4_layers[] = {
>>  			.disc_pos = 5,
>>  			.disc_size = 6,
>>  		},
>> +		.clut_offset = 0x600,
>>  	},
>>  	{
>>  		.name = "overlay1",
>> @@ -293,6 +304,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d4_layers[] = {
>>  			.chroma_key_mask = 8,
>>  			.general_config = 9,
>>  		},
>> +		.clut_offset = 0xa00,
>>  	},
>>  	{
>>  		.name = "overlay2",
>> @@ -336,6 +348,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d4_layers[] = {
>>  			},
>>  			.csc = 14,
>>  		},
>> +		.clut_offset = 0x1200,
>>  	},
>>  };
>>  
>> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
>> index b0596a8..709f7b9 100644
>> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
>> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
>> @@ -88,6 +88,11 @@
>>  #define ATMEL_HLCDC_YUV422SWP			BIT(17)
>>  #define ATMEL_HLCDC_DSCALEOPT			BIT(20)
>>  
>> +#define ATMEL_HLCDC_C1_MODE			ATMEL_HLCDC_CLUT_MODE(0)
>> +#define ATMEL_HLCDC_C2_MODE			ATMEL_HLCDC_CLUT_MODE(1)
>> +#define ATMEL_HLCDC_C4_MODE			ATMEL_HLCDC_CLUT_MODE(2)
>> +#define ATMEL_HLCDC_C8_MODE			ATMEL_HLCDC_CLUT_MODE(3)
>> +
>>  #define ATMEL_HLCDC_XRGB4444_MODE		ATMEL_HLCDC_RGB_MODE(0)
>>  #define ATMEL_HLCDC_ARGB4444_MODE		ATMEL_HLCDC_RGB_MODE(1)
>>  #define ATMEL_HLCDC_RGBA4444_MODE		ATMEL_HLCDC_RGB_MODE(2)
>> @@ -142,6 +147,8 @@
>>  #define ATMEL_HLCDC_DMA_CHANNEL_DSCR_DONE	BIT(2)
>>  #define ATMEL_HLCDC_DMA_CHANNEL_DSCR_OVERRUN	BIT(3)
>>  
>> +#define ATMEL_HLCDC_CLUT_SIZE			256
>> +
>>  #define ATMEL_HLCDC_MAX_LAYERS			6
>>  
>>  /**
>> @@ -259,6 +266,7 @@ struct atmel_hlcdc_layer_desc {
>>  	int id;
>>  	int regs_offset;
>>  	int cfgs_offset;
>> +	int clut_offset;
>>  	struct atmel_hlcdc_formats *formats;
>>  	struct atmel_hlcdc_layer_cfg_layout layout;
>>  	int max_width;
>> @@ -414,6 +422,14 @@ static inline u32 atmel_hlcdc_layer_read_cfg(struct atmel_hlcdc_layer *layer,
>>  					  (cfgid * sizeof(u32)));
>>  }
>>  
>> +static inline void atmel_hlcdc_layer_write_clut(struct atmel_hlcdc_layer *layer,
>> +						unsigned int c, u32 val)
>> +{
>> +	atmel_hlcdc_layer_write_reg(layer,
>> +				    layer->desc->clut_offset + c * sizeof(u32),
>> +				    val);
>> +}
>> +
>>  static inline void atmel_hlcdc_layer_init(struct atmel_hlcdc_layer *layer,
>>  				const struct atmel_hlcdc_layer_desc *desc,
>>  				struct regmap *regmap)
>> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
>> index 1124200..b5bd9b0 100644
>> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
>> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
>> @@ -83,6 +83,7 @@ drm_plane_state_to_atmel_hlcdc_plane_state(struct drm_plane_state *s)
>>  #define SUBPIXEL_MASK			0xffff
>>  
>>  static uint32_t rgb_formats[] = {
>> +	DRM_FORMAT_C8,
>>  	DRM_FORMAT_XRGB4444,
>>  	DRM_FORMAT_ARGB4444,
>>  	DRM_FORMAT_RGBA4444,
>> @@ -100,6 +101,7 @@ struct atmel_hlcdc_formats atmel_hlcdc_plane_rgb_formats = {
>>  };
>>  
>>  static uint32_t rgb_and_yuv_formats[] = {
>> +	DRM_FORMAT_C8,
>>  	DRM_FORMAT_XRGB4444,
>>  	DRM_FORMAT_ARGB4444,
>>  	DRM_FORMAT_RGBA4444,
>> @@ -128,6 +130,9 @@ struct atmel_hlcdc_formats atmel_hlcdc_plane_rgb_and_yuv_formats = {
>>  static int atmel_hlcdc_format_to_plane_mode(u32 format, u32 *mode)
>>  {
>>  	switch (format) {
>> +	case DRM_FORMAT_C8:
>> +		*mode = ATMEL_HLCDC_C8_MODE;
>> +		break;
>>  	case DRM_FORMAT_XRGB4444:
>>  		*mode = ATMEL_HLCDC_XRGB4444_MODE;
>>  		break;
>> @@ -424,6 +429,29 @@ static void atmel_hlcdc_plane_update_format(struct atmel_hlcdc_plane *plane,
>>  				    ATMEL_HLCDC_LAYER_FORMAT_CFG, cfg);
>>  }
>>  
>> +static void atmel_hlcdc_plane_update_clut(struct atmel_hlcdc_plane *plane)
>> +{
>> +	struct drm_crtc *crtc = plane->base.crtc;
>> +	struct drm_color_lut *lut;
>> +	int idx;
>> +
>> +	if (!crtc || !crtc->state)
>> +		return;
>> +
>> +	if (!crtc->state->color_mgmt_changed || !crtc->state->gamma_lut)
>> +		return;
>> +
>> +	lut = (struct drm_color_lut *)crtc->state->gamma_lut->data;
>> +
>> +	for (idx = 0; idx < ATMEL_HLCDC_CLUT_SIZE; idx++, lut++) {
>> +		u32 val = ((lut->red << 8) & 0xff0000) |
>> +			(lut->green & 0xff00) |
>> +			(lut->blue >> 8);
>> +
>> +		atmel_hlcdc_layer_write_clut(&plane->layer, idx, val);
>> +	}
>> +}
>> +
>>  static void atmel_hlcdc_plane_update_buffers(struct atmel_hlcdc_plane *plane,
>>  					struct atmel_hlcdc_plane_state *state)
>>  {
>> @@ -768,6 +796,7 @@ static void atmel_hlcdc_plane_atomic_update(struct drm_plane *p,
>>  	atmel_hlcdc_plane_update_pos_and_size(plane, state);
>>  	atmel_hlcdc_plane_update_general_settings(plane, state);
>>  	atmel_hlcdc_plane_update_format(plane, state);
>> +	atmel_hlcdc_plane_update_clut(plane);
>>  	atmel_hlcdc_plane_update_buffers(plane, state);
>>  	atmel_hlcdc_plane_update_disc_area(plane, state);
>>  
>> -- 
>> 2.1.4
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

Re: [RFC PATCH v2 1/3] drm: atmel-hlcdc: add support for 8-bit color lookup table mode

[Daniel Vetter]

On Tue, Jun 20, 2017 at 11:55:01AM +0200, Peter Rosin wrote:
> On 2017-06-20 11:40, Daniel Vetter wrote:
> > On Sat, Jun 17, 2017 at 07:48:02PM +0200, Peter Rosin wrote:
> >> All layers of all supported chips support this, the only variable is the
> >> base address of the lookup table in the register map.
> >>
> >> Signed-off-by: Peter Rosin <peda@axentia.se>
> > 
> > As Boris said, pls use the new color manager stuff for atomic drivers, and
> > then use the helper to implement the legacy kms LUT support on top of it,
> > it's drm_atomic_helper_legacy_gamma_set().
> > -Daniel
> 
> Hmm, I don't see how this comment applies to this patch? Isn't this
> patch good as is, because I thought I already did wire this up with
> the drm_atomic_helper_legacy_gamma_set helper as requested?
> (sure, v3 has a register offset tweak, but that's just a detail)
> 
> I totally agree that 2/3 and 3/3 are ugly and I am working on that, but
> if 1/3 is also not in the right direction I'd like to know ASAP. Thanks!

Sorry, I must have mixed up your patches. This one here indeed looks like
it does things properly. For this one here:

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Cheers, Daniel

> 
> Cheers,
> peda
> 
> > 
> >> ---
> >>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c  |  5 +++++
> >>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c    | 13 +++++++++++
> >>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h    | 16 ++++++++++++++
> >>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 29 +++++++++++++++++++++++++
> >>  4 files changed, 63 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> >> index 5348985..694adcc 100644
> >> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> >> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> >> @@ -429,6 +429,8 @@ static const struct drm_crtc_funcs atmel_hlcdc_crtc_funcs = {
> >>  	.atomic_destroy_state = atmel_hlcdc_crtc_destroy_state,
> >>  	.enable_vblank = atmel_hlcdc_crtc_enable_vblank,
> >>  	.disable_vblank = atmel_hlcdc_crtc_disable_vblank,
> >> +	.set_property = drm_atomic_helper_crtc_set_property,
> >> +	.gamma_set = drm_atomic_helper_legacy_gamma_set,
> >>  };
> >>  
> >>  int atmel_hlcdc_crtc_create(struct drm_device *dev)
> >> @@ -484,6 +486,9 @@ int atmel_hlcdc_crtc_create(struct drm_device *dev)
> >>  	drm_crtc_helper_add(&crtc->base, &lcdc_crtc_helper_funcs);
> >>  	drm_crtc_vblank_reset(&crtc->base);
> >>  
> >> +	drm_mode_crtc_set_gamma_size(&crtc->base, ATMEL_HLCDC_CLUT_SIZE);
> >> +	drm_crtc_enable_color_mgmt(&crtc->base, 0, false, ATMEL_HLCDC_CLUT_SIZE);
> >> +
> >>  	dc->crtc = &crtc->base;
> >>  
> >>  	return 0;
> >> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> >> index 30dbffd..4f6ef07 100644
> >> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> >> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> >> @@ -42,6 +42,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_at91sam9n12_layers[] = {
> >>  			.default_color = 3,
> >>  			.general_config = 4,
> >>  		},
> >> +		.clut_offset = 0x400,
> >>  	},
> >>  };
> >>  
> >> @@ -73,6 +74,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_at91sam9x5_layers[] = {
> >>  			.disc_pos = 5,
> >>  			.disc_size = 6,
> >>  		},
> >> +		.clut_offset = 0x400,
> >>  	},
> >>  	{
> >>  		.name = "overlay1",
> >> @@ -91,6 +93,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_at91sam9x5_layers[] = {
> >>  			.chroma_key_mask = 8,
> >>  			.general_config = 9,
> >>  		},
> >> +		.clut_offset = 0x800,
> >>  	},
> >>  	{
> >>  		.name = "high-end-overlay",
> >> @@ -112,6 +115,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_at91sam9x5_layers[] = {
> >>  			.scaler_config = 13,
> >>  			.csc = 14,
> >>  		},
> >> +		.clut_offset = 0x1000,
> >>  	},
> >>  	{
> >>  		.name = "cursor",
> >> @@ -131,6 +135,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_at91sam9x5_layers[] = {
> >>  			.chroma_key_mask = 8,
> >>  			.general_config = 9,
> >>  		},
> >> +		.clut_offset = 0x1400,
> >>  	},
> >>  };
> >>  
> >> @@ -162,6 +167,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d3_layers[] = {
> >>  			.disc_pos = 5,
> >>  			.disc_size = 6,
> >>  		},
> >> +		.clut_offset = 0x600,
> >>  	},
> >>  	{
> >>  		.name = "overlay1",
> >> @@ -180,6 +186,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d3_layers[] = {
> >>  			.chroma_key_mask = 8,
> >>  			.general_config = 9,
> >>  		},
> >> +		.clut_offset = 0xa00,
> >>  	},
> >>  	{
> >>  		.name = "overlay2",
> >> @@ -198,6 +205,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d3_layers[] = {
> >>  			.chroma_key_mask = 8,
> >>  			.general_config = 9,
> >>  		},
> >> +		.clut_offset = 0xe00,
> >>  	},
> >>  	{
> >>  		.name = "high-end-overlay",
> >> @@ -223,6 +231,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d3_layers[] = {
> >>  			},
> >>  			.csc = 14,
> >>  		},
> >> +		.clut_offset = 0x1200,
> >>  	},
> >>  	{
> >>  		.name = "cursor",
> >> @@ -244,6 +253,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d3_layers[] = {
> >>  			.general_config = 9,
> >>  			.scaler_config = 13,
> >>  		},
> >> +		.clut_offset = 0x1600,
> >>  	},
> >>  };
> >>  
> >> @@ -275,6 +285,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d4_layers[] = {
> >>  			.disc_pos = 5,
> >>  			.disc_size = 6,
> >>  		},
> >> +		.clut_offset = 0x600,
> >>  	},
> >>  	{
> >>  		.name = "overlay1",
> >> @@ -293,6 +304,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d4_layers[] = {
> >>  			.chroma_key_mask = 8,
> >>  			.general_config = 9,
> >>  		},
> >> +		.clut_offset = 0xa00,
> >>  	},
> >>  	{
> >>  		.name = "overlay2",
> >> @@ -336,6 +348,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d4_layers[] = {
> >>  			},
> >>  			.csc = 14,
> >>  		},
> >> +		.clut_offset = 0x1200,
> >>  	},
> >>  };
> >>  
> >> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> >> index b0596a8..709f7b9 100644
> >> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> >> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> >> @@ -88,6 +88,11 @@
> >>  #define ATMEL_HLCDC_YUV422SWP			BIT(17)
> >>  #define ATMEL_HLCDC_DSCALEOPT			BIT(20)
> >>  
> >> +#define ATMEL_HLCDC_C1_MODE			ATMEL_HLCDC_CLUT_MODE(0)
> >> +#define ATMEL_HLCDC_C2_MODE			ATMEL_HLCDC_CLUT_MODE(1)
> >> +#define ATMEL_HLCDC_C4_MODE			ATMEL_HLCDC_CLUT_MODE(2)
> >> +#define ATMEL_HLCDC_C8_MODE			ATMEL_HLCDC_CLUT_MODE(3)
> >> +
> >>  #define ATMEL_HLCDC_XRGB4444_MODE		ATMEL_HLCDC_RGB_MODE(0)
> >>  #define ATMEL_HLCDC_ARGB4444_MODE		ATMEL_HLCDC_RGB_MODE(1)
> >>  #define ATMEL_HLCDC_RGBA4444_MODE		ATMEL_HLCDC_RGB_MODE(2)
> >> @@ -142,6 +147,8 @@
> >>  #define ATMEL_HLCDC_DMA_CHANNEL_DSCR_DONE	BIT(2)
> >>  #define ATMEL_HLCDC_DMA_CHANNEL_DSCR_OVERRUN	BIT(3)
> >>  
> >> +#define ATMEL_HLCDC_CLUT_SIZE			256
> >> +
> >>  #define ATMEL_HLCDC_MAX_LAYERS			6
> >>  
> >>  /**
> >> @@ -259,6 +266,7 @@ struct atmel_hlcdc_layer_desc {
> >>  	int id;
> >>  	int regs_offset;
> >>  	int cfgs_offset;
> >> +	int clut_offset;
> >>  	struct atmel_hlcdc_formats *formats;
> >>  	struct atmel_hlcdc_layer_cfg_layout layout;
> >>  	int max_width;
> >> @@ -414,6 +422,14 @@ static inline u32 atmel_hlcdc_layer_read_cfg(struct atmel_hlcdc_layer *layer,
> >>  					  (cfgid * sizeof(u32)));
> >>  }
> >>  
> >> +static inline void atmel_hlcdc_layer_write_clut(struct atmel_hlcdc_layer *layer,
> >> +						unsigned int c, u32 val)
> >> +{
> >> +	atmel_hlcdc_layer_write_reg(layer,
> >> +				    layer->desc->clut_offset + c * sizeof(u32),
> >> +				    val);
> >> +}
> >> +
> >>  static inline void atmel_hlcdc_layer_init(struct atmel_hlcdc_layer *layer,
> >>  				const struct atmel_hlcdc_layer_desc *desc,
> >>  				struct regmap *regmap)
> >> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> >> index 1124200..b5bd9b0 100644
> >> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> >> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> >> @@ -83,6 +83,7 @@ drm_plane_state_to_atmel_hlcdc_plane_state(struct drm_plane_state *s)
> >>  #define SUBPIXEL_MASK			0xffff
> >>  
> >>  static uint32_t rgb_formats[] = {
> >> +	DRM_FORMAT_C8,
> >>  	DRM_FORMAT_XRGB4444,
> >>  	DRM_FORMAT_ARGB4444,
> >>  	DRM_FORMAT_RGBA4444,
> >> @@ -100,6 +101,7 @@ struct atmel_hlcdc_formats atmel_hlcdc_plane_rgb_formats = {
> >>  };
> >>  
> >>  static uint32_t rgb_and_yuv_formats[] = {
> >> +	DRM_FORMAT_C8,
> >>  	DRM_FORMAT_XRGB4444,
> >>  	DRM_FORMAT_ARGB4444,
> >>  	DRM_FORMAT_RGBA4444,
> >> @@ -128,6 +130,9 @@ struct atmel_hlcdc_formats atmel_hlcdc_plane_rgb_and_yuv_formats = {
> >>  static int atmel_hlcdc_format_to_plane_mode(u32 format, u32 *mode)
> >>  {
> >>  	switch (format) {
> >> +	case DRM_FORMAT_C8:
> >> +		*mode = ATMEL_HLCDC_C8_MODE;
> >> +		break;
> >>  	case DRM_FORMAT_XRGB4444:
> >>  		*mode = ATMEL_HLCDC_XRGB4444_MODE;
> >>  		break;
> >> @@ -424,6 +429,29 @@ static void atmel_hlcdc_plane_update_format(struct atmel_hlcdc_plane *plane,
> >>  				    ATMEL_HLCDC_LAYER_FORMAT_CFG, cfg);
> >>  }
> >>  
> >> +static void atmel_hlcdc_plane_update_clut(struct atmel_hlcdc_plane *plane)
> >> +{
> >> +	struct drm_crtc *crtc = plane->base.crtc;
> >> +	struct drm_color_lut *lut;
> >> +	int idx;
> >> +
> >> +	if (!crtc || !crtc->state)
> >> +		return;
> >> +
> >> +	if (!crtc->state->color_mgmt_changed || !crtc->state->gamma_lut)
> >> +		return;
> >> +
> >> +	lut = (struct drm_color_lut *)crtc->state->gamma_lut->data;
> >> +
> >> +	for (idx = 0; idx < ATMEL_HLCDC_CLUT_SIZE; idx++, lut++) {
> >> +		u32 val = ((lut->red << 8) & 0xff0000) |
> >> +			(lut->green & 0xff00) |
> >> +			(lut->blue >> 8);
> >> +
> >> +		atmel_hlcdc_layer_write_clut(&plane->layer, idx, val);
> >> +	}
> >> +}
> >> +
> >>  static void atmel_hlcdc_plane_update_buffers(struct atmel_hlcdc_plane *plane,
> >>  					struct atmel_hlcdc_plane_state *state)
> >>  {
> >> @@ -768,6 +796,7 @@ static void atmel_hlcdc_plane_atomic_update(struct drm_plane *p,
> >>  	atmel_hlcdc_plane_update_pos_and_size(plane, state);
> >>  	atmel_hlcdc_plane_update_general_settings(plane, state);
> >>  	atmel_hlcdc_plane_update_format(plane, state);
> >> +	atmel_hlcdc_plane_update_clut(plane);
> >>  	atmel_hlcdc_plane_update_buffers(plane, state);
> >>  	atmel_hlcdc_plane_update_disc_area(plane, state);
> >>  
> >> -- 
> >> 2.1.4
> >>
> >> _______________________________________________
> >> dri-devel mailing list
> >> dri-devel@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [RFC PATCH v2 1/3] drm: atmel-hlcdc: add support for 8-bit color lookup table mode

[Daniel Vetter]

On Tue, Jun 20, 2017 at 11:55:01AM +0200, Peter Rosin wrote:
> On 2017-06-20 11:40, Daniel Vetter wrote:
> > On Sat, Jun 17, 2017 at 07:48:02PM +0200, Peter Rosin wrote:
> >> All layers of all supported chips support this, the only variable is the
> >> base address of the lookup table in the register map.
> >>
> >> Signed-off-by: Peter Rosin <peda@axentia.se>
> > 
> > As Boris said, pls use the new color manager stuff for atomic drivers, and
> > then use the helper to implement the legacy kms LUT support on top of it,
> > it's drm_atomic_helper_legacy_gamma_set().
> > -Daniel
> 
> Hmm, I don't see how this comment applies to this patch? Isn't this
> patch good as is, because I thought I already did wire this up with
> the drm_atomic_helper_legacy_gamma_set helper as requested?
> (sure, v3 has a register offset tweak, but that's just a detail)
> 
> I totally agree that 2/3 and 3/3 are ugly and I am working on that, but
> if 1/3 is also not in the right direction I'd like to know ASAP. Thanks!

Sorry, I must have mixed up your patches. This one here indeed looks like
it does things properly. For this one here:

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Cheers, Daniel

> 
> Cheers,
> peda
> 
> > 
> >> ---
> >>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c  |  5 +++++
> >>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c    | 13 +++++++++++
> >>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h    | 16 ++++++++++++++
> >>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 29 +++++++++++++++++++++++++
> >>  4 files changed, 63 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> >> index 5348985..694adcc 100644
> >> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> >> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> >> @@ -429,6 +429,8 @@ static const struct drm_crtc_funcs atmel_hlcdc_crtc_funcs = {
> >>  	.atomic_destroy_state = atmel_hlcdc_crtc_destroy_state,
> >>  	.enable_vblank = atmel_hlcdc_crtc_enable_vblank,
> >>  	.disable_vblank = atmel_hlcdc_crtc_disable_vblank,
> >> +	.set_property = drm_atomic_helper_crtc_set_property,
> >> +	.gamma_set = drm_atomic_helper_legacy_gamma_set,
> >>  };
> >>  
> >>  int atmel_hlcdc_crtc_create(struct drm_device *dev)
> >> @@ -484,6 +486,9 @@ int atmel_hlcdc_crtc_create(struct drm_device *dev)
> >>  	drm_crtc_helper_add(&crtc->base, &lcdc_crtc_helper_funcs);
> >>  	drm_crtc_vblank_reset(&crtc->base);
> >>  
> >> +	drm_mode_crtc_set_gamma_size(&crtc->base, ATMEL_HLCDC_CLUT_SIZE);
> >> +	drm_crtc_enable_color_mgmt(&crtc->base, 0, false, ATMEL_HLCDC_CLUT_SIZE);
> >> +
> >>  	dc->crtc = &crtc->base;
> >>  
> >>  	return 0;
> >> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> >> index 30dbffd..4f6ef07 100644
> >> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> >> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> >> @@ -42,6 +42,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_at91sam9n12_layers[] = {
> >>  			.default_color = 3,
> >>  			.general_config = 4,
> >>  		},
> >> +		.clut_offset = 0x400,
> >>  	},
> >>  };
> >>  
> >> @@ -73,6 +74,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_at91sam9x5_layers[] = {
> >>  			.disc_pos = 5,
> >>  			.disc_size = 6,
> >>  		},
> >> +		.clut_offset = 0x400,
> >>  	},
> >>  	{
> >>  		.name = "overlay1",
> >> @@ -91,6 +93,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_at91sam9x5_layers[] = {
> >>  			.chroma_key_mask = 8,
> >>  			.general_config = 9,
> >>  		},
> >> +		.clut_offset = 0x800,
> >>  	},
> >>  	{
> >>  		.name = "high-end-overlay",
> >> @@ -112,6 +115,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_at91sam9x5_layers[] = {
> >>  			.scaler_config = 13,
> >>  			.csc = 14,
> >>  		},
> >> +		.clut_offset = 0x1000,
> >>  	},
> >>  	{
> >>  		.name = "cursor",
> >> @@ -131,6 +135,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_at91sam9x5_layers[] = {
> >>  			.chroma_key_mask = 8,
> >>  			.general_config = 9,
> >>  		},
> >> +		.clut_offset = 0x1400,
> >>  	},
> >>  };
> >>  
> >> @@ -162,6 +167,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d3_layers[] = {
> >>  			.disc_pos = 5,
> >>  			.disc_size = 6,
> >>  		},
> >> +		.clut_offset = 0x600,
> >>  	},
> >>  	{
> >>  		.name = "overlay1",
> >> @@ -180,6 +186,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d3_layers[] = {
> >>  			.chroma_key_mask = 8,
> >>  			.general_config = 9,
> >>  		},
> >> +		.clut_offset = 0xa00,
> >>  	},
> >>  	{
> >>  		.name = "overlay2",
> >> @@ -198,6 +205,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d3_layers[] = {
> >>  			.chroma_key_mask = 8,
> >>  			.general_config = 9,
> >>  		},
> >> +		.clut_offset = 0xe00,
> >>  	},
> >>  	{
> >>  		.name = "high-end-overlay",
> >> @@ -223,6 +231,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d3_layers[] = {
> >>  			},
> >>  			.csc = 14,
> >>  		},
> >> +		.clut_offset = 0x1200,
> >>  	},
> >>  	{
> >>  		.name = "cursor",
> >> @@ -244,6 +253,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d3_layers[] = {
> >>  			.general_config = 9,
> >>  			.scaler_config = 13,
> >>  		},
> >> +		.clut_offset = 0x1600,
> >>  	},
> >>  };
> >>  
> >> @@ -275,6 +285,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d4_layers[] = {
> >>  			.disc_pos = 5,
> >>  			.disc_size = 6,
> >>  		},
> >> +		.clut_offset = 0x600,
> >>  	},
> >>  	{
> >>  		.name = "overlay1",
> >> @@ -293,6 +304,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d4_layers[] = {
> >>  			.chroma_key_mask = 8,
> >>  			.general_config = 9,
> >>  		},
> >> +		.clut_offset = 0xa00,
> >>  	},
> >>  	{
> >>  		.name = "overlay2",
> >> @@ -336,6 +348,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d4_layers[] = {
> >>  			},
> >>  			.csc = 14,
> >>  		},
> >> +		.clut_offset = 0x1200,
> >>  	},
> >>  };
> >>  
> >> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> >> index b0596a8..709f7b9 100644
> >> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> >> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> >> @@ -88,6 +88,11 @@
> >>  #define ATMEL_HLCDC_YUV422SWP			BIT(17)
> >>  #define ATMEL_HLCDC_DSCALEOPT			BIT(20)
> >>  
> >> +#define ATMEL_HLCDC_C1_MODE			ATMEL_HLCDC_CLUT_MODE(0)
> >> +#define ATMEL_HLCDC_C2_MODE			ATMEL_HLCDC_CLUT_MODE(1)
> >> +#define ATMEL_HLCDC_C4_MODE			ATMEL_HLCDC_CLUT_MODE(2)
> >> +#define ATMEL_HLCDC_C8_MODE			ATMEL_HLCDC_CLUT_MODE(3)
> >> +
> >>  #define ATMEL_HLCDC_XRGB4444_MODE		ATMEL_HLCDC_RGB_MODE(0)
> >>  #define ATMEL_HLCDC_ARGB4444_MODE		ATMEL_HLCDC_RGB_MODE(1)
> >>  #define ATMEL_HLCDC_RGBA4444_MODE		ATMEL_HLCDC_RGB_MODE(2)
> >> @@ -142,6 +147,8 @@
> >>  #define ATMEL_HLCDC_DMA_CHANNEL_DSCR_DONE	BIT(2)
> >>  #define ATMEL_HLCDC_DMA_CHANNEL_DSCR_OVERRUN	BIT(3)
> >>  
> >> +#define ATMEL_HLCDC_CLUT_SIZE			256
> >> +
> >>  #define ATMEL_HLCDC_MAX_LAYERS			6
> >>  
> >>  /**
> >> @@ -259,6 +266,7 @@ struct atmel_hlcdc_layer_desc {
> >>  	int id;
> >>  	int regs_offset;
> >>  	int cfgs_offset;
> >> +	int clut_offset;
> >>  	struct atmel_hlcdc_formats *formats;
> >>  	struct atmel_hlcdc_layer_cfg_layout layout;
> >>  	int max_width;
> >> @@ -414,6 +422,14 @@ static inline u32 atmel_hlcdc_layer_read_cfg(struct atmel_hlcdc_layer *layer,
> >>  					  (cfgid * sizeof(u32)));
> >>  }
> >>  
> >> +static inline void atmel_hlcdc_layer_write_clut(struct atmel_hlcdc_layer *layer,
> >> +						unsigned int c, u32 val)
> >> +{
> >> +	atmel_hlcdc_layer_write_reg(layer,
> >> +				    layer->desc->clut_offset + c * sizeof(u32),
> >> +				    val);
> >> +}
> >> +
> >>  static inline void atmel_hlcdc_layer_init(struct atmel_hlcdc_layer *layer,
> >>  				const struct atmel_hlcdc_layer_desc *desc,
> >>  				struct regmap *regmap)
> >> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> >> index 1124200..b5bd9b0 100644
> >> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> >> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> >> @@ -83,6 +83,7 @@ drm_plane_state_to_atmel_hlcdc_plane_state(struct drm_plane_state *s)
> >>  #define SUBPIXEL_MASK			0xffff
> >>  
> >>  static uint32_t rgb_formats[] = {
> >> +	DRM_FORMAT_C8,
> >>  	DRM_FORMAT_XRGB4444,
> >>  	DRM_FORMAT_ARGB4444,
> >>  	DRM_FORMAT_RGBA4444,
> >> @@ -100,6 +101,7 @@ struct atmel_hlcdc_formats atmel_hlcdc_plane_rgb_formats = {
> >>  };
> >>  
> >>  static uint32_t rgb_and_yuv_formats[] = {
> >> +	DRM_FORMAT_C8,
> >>  	DRM_FORMAT_XRGB4444,
> >>  	DRM_FORMAT_ARGB4444,
> >>  	DRM_FORMAT_RGBA4444,
> >> @@ -128,6 +130,9 @@ struct atmel_hlcdc_formats atmel_hlcdc_plane_rgb_and_yuv_formats = {
> >>  static int atmel_hlcdc_format_to_plane_mode(u32 format, u32 *mode)
> >>  {
> >>  	switch (format) {
> >> +	case DRM_FORMAT_C8:
> >> +		*mode = ATMEL_HLCDC_C8_MODE;
> >> +		break;
> >>  	case DRM_FORMAT_XRGB4444:
> >>  		*mode = ATMEL_HLCDC_XRGB4444_MODE;
> >>  		break;
> >> @@ -424,6 +429,29 @@ static void atmel_hlcdc_plane_update_format(struct atmel_hlcdc_plane *plane,
> >>  				    ATMEL_HLCDC_LAYER_FORMAT_CFG, cfg);
> >>  }
> >>  
> >> +static void atmel_hlcdc_plane_update_clut(struct atmel_hlcdc_plane *plane)
> >> +{
> >> +	struct drm_crtc *crtc = plane->base.crtc;
> >> +	struct drm_color_lut *lut;
> >> +	int idx;
> >> +
> >> +	if (!crtc || !crtc->state)
> >> +		return;
> >> +
> >> +	if (!crtc->state->color_mgmt_changed || !crtc->state->gamma_lut)
> >> +		return;
> >> +
> >> +	lut = (struct drm_color_lut *)crtc->state->gamma_lut->data;
> >> +
> >> +	for (idx = 0; idx < ATMEL_HLCDC_CLUT_SIZE; idx++, lut++) {
> >> +		u32 val = ((lut->red << 8) & 0xff0000) |
> >> +			(lut->green & 0xff00) |
> >> +			(lut->blue >> 8);
> >> +
> >> +		atmel_hlcdc_layer_write_clut(&plane->layer, idx, val);
> >> +	}
> >> +}
> >> +
> >>  static void atmel_hlcdc_plane_update_buffers(struct atmel_hlcdc_plane *plane,
> >>  					struct atmel_hlcdc_plane_state *state)
> >>  {
> >> @@ -768,6 +796,7 @@ static void atmel_hlcdc_plane_atomic_update(struct drm_plane *p,
> >>  	atmel_hlcdc_plane_update_pos_and_size(plane, state);
> >>  	atmel_hlcdc_plane_update_general_settings(plane, state);
> >>  	atmel_hlcdc_plane_update_format(plane, state);
> >> +	atmel_hlcdc_plane_update_clut(plane);
> >>  	atmel_hlcdc_plane_update_buffers(plane, state);
> >>  	atmel_hlcdc_plane_update_disc_area(plane, state);
> >>  
> >> -- 
> >> 2.1.4
> >>
> >> _______________________________________________
> >> dri-devel mailing list
> >> dri-devel@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel