Color lookup support for the atmel-hlcdc driver

Peter Rosin peda at axentia.se
Thu Jun 15 09:54:29 UTC 2017


On 2017-06-13 17:30, Boris Brezillon wrote:
> Hi Peter,
> 
> On Tue, 13 Jun 2017 16:34:25 +0200
> Peter Rosin <peda at axentia.se> wrote:
> 
>> Hi!
>>
>> I need color lookup support for the atmel-hlcdc driver, and had a peek
>> at the code. I also looked at the drivers/gpu/drm/stm driver and came
>> up with the below diff. It compiles, but I have not booted it for the
>> simple reason that I can't imagine it will work.
>>
>> Sure, the code fills the clut registers in the .load_lut helper function
>> and I think it might even do the right thing when setting up the mode.
>> I'm less sure DMA will work correctly? It might, but I haven't looked at
>> it for many seconds...
>>
>> The big question is how the color info gets into the new clut array
>> I created in atmel_hlcdc_crtc? When I look at the stm driver, which does
>> it just like this AFAICT I just don't see it. So, what I'm I missing?
> 
> You should look at drm_atomic_helper_legacy_gamma_set() and its users.

Ok, thanks. I had a long look and could not get it to work at all. So,
I did as below instead. However, there are a few glaring problems...

Something, somewhere, sets up default entries for the 16 first entries
of the palette and seem to expect them to stay as some kind of safe
basic palette, and my applications don't handle it too well. When they
want to draw black, they get a poisonous cyan/green instead etc. But
it's pretty close. Can anyone provide some clue as to how that is
supposed to be handled?

Also, I had to change the second argument of the drm_fbdev_cma_init...
call at the end of atmel_hlcdc_dc.c:atmel_hlcdc_dc_load() from 24 to 8
to make it possible to set 8-modes. However, doing so fucks up 24-bit
modes. Which made me suspect that no non-24-bit mode work w/o my patch.
And I could indeed only get 24-bit modes to work, unless I changed this
value to 16. Then RGB565 works like a charm, but RGB888 don't. What's
up with that? Seems like a rather silly limitation, but it's perhaps
just a bug?

Cheers,
peda


diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
index 5348985..0de1047 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
@@ -61,6 +61,7 @@ struct atmel_hlcdc_crtc {
 	struct atmel_hlcdc_dc *dc;
 	struct drm_pending_vblank_event *event;
 	int id;
+	u32 clut[256];
 };
 
 static inline struct atmel_hlcdc_crtc *
@@ -140,6 +141,47 @@ 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;
+	unsigned int color;
+	int layer;
+
+	for (layer = 0; layer < ATMEL_HLCDC_MAX_LAYERS; layer++) {
+		if (!dc->layers[layer])
+			continue;
+		for (color = 0; color < 256; color++)
+			atmel_hlcdc_layer_write_clut(dc->layers[layer],
+						     color, crtc->clut[color]);
+	}
+}
+
+void atmel_hlcdc_gamma_set(struct drm_crtc *c,
+			   u16 r, u16 g, u16 b, int idx)
+{
+	struct atmel_hlcdc_crtc *crtc = drm_crtc_to_atmel_hlcdc_crtc(c);
+
+	if (idx < 0 || idx > 255)
+		return;
+
+	crtc->clut[idx] = ((r << 8) & 0xff0000) | (g & 0xff00) | (b >> 8);
+}
+
+void atmel_hlcdc_gamma_get(struct drm_crtc *c,
+			   u16 *r, u16 *g, u16 *b, int idx)
+{
+	struct atmel_hlcdc_crtc *crtc = drm_crtc_to_atmel_hlcdc_crtc(c);
+
+	if (idx < 0 || idx > 255)
+		return;
+
+	*r = (crtc->clut[idx] >> 8) & 0xff00;
+	*g = crtc->clut[idx] & 0xff00;
+	*b = (crtc->clut[idx] << 8) & 0xff00;
+}
+
 static enum drm_mode_status
 atmel_hlcdc_crtc_mode_valid(struct drm_crtc *c,
 			    const struct drm_display_mode *mode)
@@ -319,6 +361,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 30dbffd..bec15f8 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,
 	},
 };
 
@@ -588,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);
@@ -651,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, 8,
+			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 b0596a8..2a4f874 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>
@@ -88,6 +89,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)
@@ -259,6 +265,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 +421,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)
@@ -432,6 +447,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);
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
index 1124200..5537843 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;
diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c
index 53f9bdf..112feab 100644
--- a/drivers/gpu/drm/drm_fb_cma_helper.c
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -426,7 +426,7 @@ static void drm_fbdev_cma_defio_fini(struct fb_info *fbi)
 	kfree(fbi->fbops);
 }
 
-static int
+int
 drm_fbdev_cma_create(struct drm_fb_helper *helper,
 	struct drm_fb_helper_surface_size *sizes)
 {
@@ -507,23 +507,16 @@ 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
- * @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
- *
- * 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 +527,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 +567,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
+ * @funcs: fb helper 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 *funcs)
+{
+	return drm_fbdev_cma_init_with_funcs2(dev, preferred_bpp,
+					      max_conn_count,
+					      funcs, NULL);
+}
 EXPORT_SYMBOL_GPL(drm_fbdev_cma_init_with_funcs);
 
 /**
@@ -581,9 +599,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..ea15e20a 100644
--- a/include/drm/drm_fb_cma_helper.h
+++ b/include/drm/drm_fb_cma_helper.h
@@ -15,6 +15,10 @@ 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);
@@ -22,6 +26,8 @@ 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);


More information about the dri-devel mailing list