[PATCH v5 0/2] drm: atmel-hlcdc: clut support

[PATCH v5 0/2] drm: atmel-hlcdc: clut support

[Peter Rosin]

Hi!

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

Changes since v4:

- Added .clut_offset for overlay2 at 0xe00 for sama5d4 (unconfirmed if 0xe00
  is the correct offset, but I'll eat my hat if it's not there). The sama5d4
  overlay2 is indeed there, it is just AWOL in the current datasheet.
- Added Acked-by from Daniel on patch 2/2.

Changes since v3:

- Dropped ugly code (patches 2/3 and 3/3) for legacy fbdev interaction.
- Slit out the .set_property change to a patch of its own.

Changes since v2:

- Fix mapping to the clut registers.

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.

Cheers,
peda

Peter Rosin (2):
  drm: atmel-hlcdc: add missing .set_property helper to the crtc
  drm: atmel-hlcdc: add support for 8-bit color lookup table mode

 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c  |  5 +++++
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c    | 14 ++++++++++++
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h    | 16 ++++++++++++++
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 29 +++++++++++++++++++++++++
 4 files changed, 64 insertions(+)

-- 
2.1.4

[PATCH v5 1/2] drm: atmel-hlcdc: add missing .set_property helper to the crtc

[Peter Rosin]

The default implementation should be used.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
index 5348985..cc00ce3 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
@@ -429,6 +429,7 @@ 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,
 };
 
 int atmel_hlcdc_crtc_create(struct drm_device *dev)
-- 
2.1.4

Re: [PATCH v5 1/2] drm: atmel-hlcdc: add missing .set_property helper to the crtc

[Daniel Vetter]

On Thu, Jun 22, 2017 at 07:03:10AM +0200, Peter Rosin wrote:
> The default implementation should be used.
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>

Purely optional refactor idea, since you're not the first one to stumble
over this, and in hindsight it makes sense to have these functions as the
defaults:

- Move all the set_property legacy2atomic helper functions into the core,
  renaming them to e.g. drm_atomic_default_crtc_set_property. But we'd do
  this for connectors and planes too ofc.

- Use them as the default handler if drm_drv_uses_atomic_modeset() is
  true.

- Nuke the default assignment from all drivers.

This way there's less boilerplate, less confusion (some people thought you
must have atomic userspace to use atomic properties, which isn't true as
long as this is wired up) and happier driver developers :-)

But since you've volunteered already to fix the fbdev clut stuff this
really is just an idea.
-Daniel

> ---
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> index 5348985..cc00ce3 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> @@ -429,6 +429,7 @@ 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,
>  };
>  
>  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

[PATCH v5 2/2] 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.

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c  |  4 ++++
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c    | 14 ++++++++++++
 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 cc00ce3..694adcc 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
@@ -430,6 +430,7 @@ static const struct drm_crtc_funcs atmel_hlcdc_crtc_funcs = {
 	.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)
@@ -485,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..6ff771c 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",
@@ -311,6 +323,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d4_layers[] = {
 			.chroma_key_mask = 8,
 			.general_config = 9,
 		},
+		.clut_offset = 0xe00,
 	},
 	{
 		.name = "high-end-overlay",
@@ -336,6 +349,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..4237b04 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)
+{
+	regmap_write(layer->regmap,
+		     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

Re: [PATCH v5 0/2] drm: atmel-hlcdc: clut support

[Nicolas Ferre]

On 22/06/2017 at 07:03, Peter Rosin wrote:
> Hi!
> 
> This series adds support for an 8-bit clut mode in the atmel-hlcdc
> driver.
> 
> Changes since v4:
> 
> - Added .clut_offset for overlay2 at 0xe00 for sama5d4 (unconfirmed if 0xe00
>   is the correct offset, but I'll eat my hat if it's not there).

You hat is safe and so you are: I confirm that overlay2 clut is at 0xe00
based on an older datasheet.

Regards,

>   The sama5d4
>   overlay2 is indeed there, it is just AWOL in the current datasheet.
> - Added Acked-by from Daniel on patch 2/2.
> 
> Changes since v3:
> 
> - Dropped ugly code (patches 2/3 and 3/3) for legacy fbdev interaction.
> - Slit out the .set_property change to a patch of its own.
> 
> Changes since v2:
> 
> - Fix mapping to the clut registers.
> 
> 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.
> 
> Cheers,
> peda
> 
> Peter Rosin (2):
>   drm: atmel-hlcdc: add missing .set_property helper to the crtc
>   drm: atmel-hlcdc: add support for 8-bit color lookup table mode
> 
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c  |  5 +++++
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c    | 14 ++++++++++++
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h    | 16 ++++++++++++++
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 29 +++++++++++++++++++++++++
>  4 files changed, 64 insertions(+)
> 


-- 
Nicolas Ferre

Re: [PATCH v5 0/2] drm: atmel-hlcdc: clut support

[Boris Brezillon]

On Thu, 22 Jun 2017 07:03:09 +0200
Peter Rosin <peda@axentia.se> wrote:

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

Applied to drm-misc-next.

Thanks,

Boris

> 
> Changes since v4:
> 
> - Added .clut_offset for overlay2 at 0xe00 for sama5d4 (unconfirmed if 0xe00
>   is the correct offset, but I'll eat my hat if it's not there). The sama5d4
>   overlay2 is indeed there, it is just AWOL in the current datasheet.
> - Added Acked-by from Daniel on patch 2/2.
> 
> Changes since v3:
> 
> - Dropped ugly code (patches 2/3 and 3/3) for legacy fbdev interaction.
> - Slit out the .set_property change to a patch of its own.
> 
> Changes since v2:
> 
> - Fix mapping to the clut registers.
> 
> 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.
> 
> Cheers,
> peda
> 
> Peter Rosin (2):
>   drm: atmel-hlcdc: add missing .set_property helper to the crtc
>   drm: atmel-hlcdc: add support for 8-bit color lookup table mode
> 
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c  |  5 +++++
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c    | 14 ++++++++++++
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h    | 16 ++++++++++++++
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 29 +++++++++++++++++++++++++
>  4 files changed, 64 insertions(+)
>