[PATCH v4 0/8] Add support for XLCDC to sam9x7 SoC family.

[PATCH v4 0/8] Add support for XLCDC to sam9x7 SoC family.

[Manikandan Muralidharan]

This patch series aims to add support for XLCDC IP of sam9x7 SoC family
to the DRM subsystem.XLCDC IP has additional registers and new
configuration bits compared to the existing register set of HLCDC IP.
The new compatible string "microchip,sam9x75-xlcdc" is defined for sam9x75
variant of the sam9x7 SoC family.The is_xlcdc flag under driver data helps
to differentiate the XLCDC and existing HLCDC code within the same driver.

changes in v4:
* fixed kernel warnings reported by kernel test robot.

changes in v3:
* Removed de-referencing the value of is_xlcdc flag multiple times in
a single function.
* Removed cpu_relax() call when using regmap_read_poll_timeout.
* Updated xfactor and yfactor equations using shift operators
* Defined CSC co-efficients in an array for code readability.

changes in v2:
* Change the driver compatible name from "microchip,sam9x7-xlcdc" to
"microchip,sam9x75-xlcdc".
* Move is_xlcdc flag to driver data.
* Remove unsed Macro definitions.
* Add co-developed-bys tags
* Replace regmap_read() with regmap_read_poll_timeout() call
* Split code into two helpers for code readablitity.

Durai Manickam KR (1):
  drm: atmel-hlcdc: Define SAM9X7 SoC XLCDC specific registers

Manikandan Muralidharan (7):
  mfd: atmel-hlcdc: Add compatible for sam9x75 XLCD controller
  drm: atmel-hlcdc: add flag to differentiate XLCDC and HLCDC IP
  drm: atmel-hlcdc: add LCD controller layer definition for sam9x75
  drm: atmel_hlcdc: Add support for XLCDC in atmel LCD driver
  drm: atmel-hlcdc: add DPI mode support for XLCDC
  drm: atmel-hlcdc: add vertical and horizontal scaling support for
    XLCDC
  drm: atmel-hlcdc: add support for DSI output formats

 .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c    | 168 +++++++--
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c  |  99 +++++
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h  |  48 +++
 .../gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c   | 353 +++++++++++++++---
 drivers/mfd/atmel-hlcdc.c                     |   1 +
 include/linux/mfd/atmel-hlcdc.h               |  10 +
 6 files changed, 584 insertions(+), 95 deletions(-)

-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 1/8] mfd: atmel-hlcdc: Add compatible for sam9x75 XLCD controller

[Manikandan Muralidharan]

Add compatible for sam9x75 XLCD controller.

Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
---
 drivers/mfd/atmel-hlcdc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mfd/atmel-hlcdc.c b/drivers/mfd/atmel-hlcdc.c
index 3c2414ba4b01..1daa7410468a 100644
--- a/drivers/mfd/atmel-hlcdc.c
+++ b/drivers/mfd/atmel-hlcdc.c
@@ -141,6 +141,7 @@ static const struct of_device_id atmel_hlcdc_match[] = {
 	{ .compatible = "atmel,sama5d3-hlcdc" },
 	{ .compatible = "atmel,sama5d4-hlcdc" },
 	{ .compatible = "microchip,sam9x60-hlcdc" },
+	{ .compatible = "microchip,sam9x75-xlcdc" },
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, atmel_hlcdc_match);
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 2/8] drm: atmel-hlcdc: add flag to differentiate XLCDC and HLCDC IP

[Manikandan Muralidharan]

Add is_xlcdc flag in driver data to differentiate XLCDC and HLCDC code
within the atmel-hlcdc driver files.

Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
index 5b5c774e0edf..d68c79a6eae7 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
@@ -304,6 +304,7 @@ atmel_hlcdc_layer_to_plane(struct atmel_hlcdc_layer *layer)
  * @conflicting_output_formats: true if RGBXXX output formats conflict with
  *				each other.
  * @fixed_clksrc: true if clock source is fixed
+ * @is_xlcdc: true if XLCDC IP is supported
  * @layers: a layer description table describing available layers
  * @nlayers: layer description table size
  */
@@ -317,6 +318,7 @@ struct atmel_hlcdc_dc_desc {
 	int max_hpw;
 	bool conflicting_output_formats;
 	bool fixed_clksrc;
+	bool is_xlcdc;
 	const struct atmel_hlcdc_layer_desc *layers;
 	int nlayers;
 };
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 3/8] drm: atmel-hlcdc: add LCD controller layer definition for sam9x75

[Manikandan Muralidharan]

Add the LCD controller layer definition and descriptor structure for
sam9x75 for the following layers,
- Base Layer
- Overlay1 Layer
- Overlay2 Layer
- High End Overlay

Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 97 ++++++++++++++++++++
 1 file changed, 97 insertions(+)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
index fa0f9a93d50d..d30aec174aa2 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
@@ -462,6 +462,99 @@ static const struct atmel_hlcdc_dc_desc atmel_hlcdc_dc_sam9x60 = {
 	.layers = atmel_hlcdc_sam9x60_layers,
 };
 
+static const struct atmel_hlcdc_layer_desc atmel_xlcdc_sam9x75_layers[] = {
+	{
+		.name = "base",
+		.formats = &atmel_hlcdc_plane_rgb_formats,
+		.regs_offset = 0x60,
+		.id = 0,
+		.type = ATMEL_HLCDC_BASE_LAYER,
+		.cfgs_offset = 0x1c,
+		.layout = {
+			.xstride = { 2 },
+			.default_color = 3,
+			.general_config = 4,
+			.disc_pos = 5,
+			.disc_size = 6,
+		},
+		.clut_offset = 0x700,
+	},
+	{
+		.name = "overlay1",
+		.formats = &atmel_hlcdc_plane_rgb_formats,
+		.regs_offset = 0x160,
+		.id = 1,
+		.type = ATMEL_HLCDC_OVERLAY_LAYER,
+		.cfgs_offset = 0x1c,
+		.layout = {
+			.pos = 2,
+			.size = 3,
+			.xstride = { 4 },
+			.pstride = { 5 },
+			.default_color = 6,
+			.chroma_key = 7,
+			.chroma_key_mask = 8,
+			.general_config = 9,
+		},
+		.clut_offset = 0xb00,
+	},
+	{
+		.name = "overlay2",
+		.formats = &atmel_hlcdc_plane_rgb_formats,
+		.regs_offset = 0x260,
+		.id = 2,
+		.type = ATMEL_HLCDC_OVERLAY_LAYER,
+		.cfgs_offset = 0x1c,
+		.layout = {
+			.pos = 2,
+			.size = 3,
+			.xstride = { 4 },
+			.pstride = { 5 },
+			.default_color = 6,
+			.chroma_key = 7,
+			.chroma_key_mask = 8,
+			.general_config = 9,
+		},
+		.clut_offset = 0xf00,
+	},
+	{
+		.name = "high-end-overlay",
+		.formats = &atmel_hlcdc_plane_rgb_and_yuv_formats,
+		.regs_offset = 0x360,
+		.id = 3,
+		.type = ATMEL_HLCDC_OVERLAY_LAYER,
+		.cfgs_offset = 0x30,
+		.layout = {
+			.pos = 2,
+			.size = 3,
+			.memsize = 4,
+			.xstride = { 5, 7 },
+			.pstride = { 6, 8 },
+			.default_color = 9,
+			.chroma_key = 10,
+			.chroma_key_mask = 11,
+			.general_config = 12,
+			.csc = 16,
+			.scaler_config = 23,
+		},
+		.clut_offset = 0x1300,
+	},
+};
+
+static const struct atmel_hlcdc_dc_desc atmel_xlcdc_dc_sam9x75 = {
+	.min_width = 0,
+	.min_height = 0,
+	.max_width = 2048,
+	.max_height = 2048,
+	.max_spw = 0xff,
+	.max_vpw = 0xff,
+	.max_hpw = 0x3ff,
+	.fixed_clksrc = true,
+	.is_xlcdc = true,
+	.nlayers = ARRAY_SIZE(atmel_xlcdc_sam9x75_layers),
+	.layers = atmel_xlcdc_sam9x75_layers,
+};
+
 static const struct of_device_id atmel_hlcdc_of_match[] = {
 	{
 		.compatible = "atmel,at91sam9n12-hlcdc",
@@ -487,6 +580,10 @@ static const struct of_device_id atmel_hlcdc_of_match[] = {
 		.compatible = "microchip,sam9x60-hlcdc",
 		.data = &atmel_hlcdc_dc_sam9x60,
 	},
+	{
+		.compatible = "microchip,sam9x75-xlcdc",
+		.data = &atmel_xlcdc_dc_sam9x75,
+	},
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, atmel_hlcdc_of_match);
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 4/8] drm: atmel-hlcdc: Define SAM9X7 SoC XLCDC specific registers

[Manikandan Muralidharan]

From: Durai Manickam KR <durai.manickamkr@microchip.com>

The register address of the XLCDC IP used in SAM9X7 SoC family
are different from the previous HLCDC.Defining those address
space with valid macros.

Signed-off-by: Durai Manickam KR <durai.manickamkr@microchip.com>
[manikandan.m@microchip.com: Remove unused macro definitions]
Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 42 ++++++++++++++++++++
 include/linux/mfd/atmel-hlcdc.h              | 10 +++++
 2 files changed, 52 insertions(+)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
index d68c79a6eae7..8b05a54b5fd0 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
@@ -15,6 +15,7 @@
 
 #include <drm/drm_plane.h>
 
+/* LCD controller common registers */
 #define ATMEL_HLCDC_LAYER_CHER			0x0
 #define ATMEL_HLCDC_LAYER_CHDR			0x4
 #define ATMEL_HLCDC_LAYER_CHSR			0x8
@@ -128,6 +129,47 @@
 
 #define ATMEL_HLCDC_MAX_LAYERS			6
 
+/* XLCDC controller specific registers */
+#define ATMEL_XLCDC_LAYER_ENR			0x10
+#define ATMEL_XLCDC_LAYER_EN			BIT(0)
+
+#define ATMEL_XLCDC_LAYER_IER			0x0
+#define ATMEL_XLCDC_LAYER_IDR			0x4
+#define ATMEL_XLCDC_LAYER_ISR			0xc
+#define ATMEL_XLCDC_LAYER_OVR_IRQ(p)		BIT(2 + (8 * (p)))
+
+#define ATMEL_XLCDC_LAYER_PLANE_ADDR(p)		(((p) * 0x4) + 0x18)
+
+#define ATMEL_XLCDC_LAYER_DMA_CFG		0
+
+#define ATMEL_XLCDC_LAYER_DMA			BIT(0)
+#define ATMEL_XLCDC_LAYER_REP			BIT(1)
+#define ATMEL_XLCDC_LAYER_DISCEN                BIT(4)
+
+#define ATMEL_XLCDC_LAYER_SFACTC_A0_MULT_AS	(4 << 6)
+#define ATMEL_XLCDC_LAYER_SFACTA_ONE		BIT(9)
+#define ATMEL_XLCDC_LAYER_DFACTC_M_A0_MULT_AS	(6 << 11)
+#define ATMEL_XLCDC_LAYER_DFACTA_ONE		BIT(14)
+
+#define ATMEL_XLCDC_LAYER_A0_SHIFT		16
+#define ATMEL_XLCDC_LAYER_A0(x)			\
+	((x) << ATMEL_XLCDC_LAYER_A0_SHIFT)
+
+#define ATMEL_XLCDC_LAYER_VSCALER_LUMA_ENABLE		BIT(0)
+#define ATMEL_XLCDC_LAYER_VSCALER_CHROMA_ENABLE		BIT(1)
+#define ATMEL_XLCDC_LAYER_HSCALER_LUMA_ENABLE		BIT(4)
+#define ATMEL_XLCDC_LAYER_HSCALER_CHROMA_ENABLE		BIT(5)
+
+#define ATMEL_XLCDC_LAYER_VXSYCFG_ONE		BIT(0)
+#define ATMEL_XLCDC_LAYER_VXSYTAP2_ENABLE	BIT(4)
+#define ATMEL_XLCDC_LAYER_VXSCCFG_ONE		BIT(16)
+#define ATMEL_XLCDC_LAYER_VXSCTAP2_ENABLE	BIT(20)
+
+#define ATMEL_XLCDC_LAYER_HXSYCFG_ONE		BIT(0)
+#define ATMEL_XLCDC_LAYER_HXSYTAP2_ENABLE	BIT(4)
+#define ATMEL_XLCDC_LAYER_HXSCCFG_ONE		BIT(16)
+#define ATMEL_XLCDC_LAYER_HXSCTAP2_ENABLE	BIT(20)
+
 /**
  * Atmel HLCDC Layer registers layout structure
  *
diff --git a/include/linux/mfd/atmel-hlcdc.h b/include/linux/mfd/atmel-hlcdc.h
index a186119a49b5..80d675a03b39 100644
--- a/include/linux/mfd/atmel-hlcdc.h
+++ b/include/linux/mfd/atmel-hlcdc.h
@@ -22,6 +22,8 @@
 #define ATMEL_HLCDC_DITHER		BIT(6)
 #define ATMEL_HLCDC_DISPDLY		BIT(7)
 #define ATMEL_HLCDC_MODE_MASK		GENMASK(9, 8)
+#define ATMEL_XLCDC_MODE_MASK		GENMASK(10, 8)
+#define ATMEL_XLCDC_DPI			BIT(11)
 #define ATMEL_HLCDC_PP			BIT(10)
 #define ATMEL_HLCDC_VSPSU		BIT(12)
 #define ATMEL_HLCDC_VSPHO		BIT(13)
@@ -34,6 +36,12 @@
 #define ATMEL_HLCDC_IDR			0x30
 #define ATMEL_HLCDC_IMR			0x34
 #define ATMEL_HLCDC_ISR			0x38
+#define ATMEL_XLCDC_ATTRE		0x3c
+
+#define ATMEL_XLCDC_BASE_UPDATE		BIT(0)
+#define ATMEL_XLCDC_OVR1_UPDATE		BIT(1)
+#define ATMEL_XLCDC_OVR3_UPDATE		BIT(2)
+#define ATMEL_XLCDC_HEO_UPDATE		BIT(3)
 
 #define ATMEL_HLCDC_CLKPOL		BIT(0)
 #define ATMEL_HLCDC_CLKSEL		BIT(2)
@@ -48,6 +56,8 @@
 #define ATMEL_HLCDC_DISP		BIT(2)
 #define ATMEL_HLCDC_PWM			BIT(3)
 #define ATMEL_HLCDC_SIP			BIT(4)
+#define ATMEL_XLCDC_SD			BIT(5)
+#define ATMEL_XLCDC_CM			BIT(6)
 
 #define ATMEL_HLCDC_SOF			BIT(0)
 #define ATMEL_HLCDC_SYNCDIS		BIT(1)
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 4/8] drm: atmel-hlcdc: Define SAM9X7 SoC XLCDC specific registers

[claudiu beznea]

On 8/25/23 15:54, Manikandan Muralidharan wrote:
> From: Durai Manickam KR <durai.manickamkr@microchip.com>
> 
> The register address of the XLCDC IP used in SAM9X7 SoC family
> are different from the previous HLCDC.Defining those address
> space with valid macros.
> 
> Signed-off-by: Durai Manickam KR <durai.manickamkr@microchip.com>
> [manikandan.m@microchip.com: Remove unused macro definitions]
> Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
> ---
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 42 ++++++++++++++++++++
>  include/linux/mfd/atmel-hlcdc.h              | 10 +++++
>  2 files changed, 52 insertions(+)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> index d68c79a6eae7..8b05a54b5fd0 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> @@ -15,6 +15,7 @@
>  
>  #include <drm/drm_plane.h>
>  
> +/* LCD controller common registers */
>  #define ATMEL_HLCDC_LAYER_CHER			0x0
>  #define ATMEL_HLCDC_LAYER_CHDR			0x4
>  #define ATMEL_HLCDC_LAYER_CHSR			0x8
> @@ -128,6 +129,47 @@
>  
>  #define ATMEL_HLCDC_MAX_LAYERS			6
>  
> +/* XLCDC controller specific registers */
> +#define ATMEL_XLCDC_LAYER_ENR			0x10
> +#define ATMEL_XLCDC_LAYER_EN			BIT(0)
> +
> +#define ATMEL_XLCDC_LAYER_IER			0x0
> +#define ATMEL_XLCDC_LAYER_IDR			0x4
> +#define ATMEL_XLCDC_LAYER_ISR			0xc
> +#define ATMEL_XLCDC_LAYER_OVR_IRQ(p)		BIT(2 + (8 * (p)))
> +
> +#define ATMEL_XLCDC_LAYER_PLANE_ADDR(p)		(((p) * 0x4) + 0x18)
> +
> +#define ATMEL_XLCDC_LAYER_DMA_CFG		0
> +
> +#define ATMEL_XLCDC_LAYER_DMA			BIT(0)
> +#define ATMEL_XLCDC_LAYER_REP			BIT(1)
> +#define ATMEL_XLCDC_LAYER_DISCEN                BIT(4)

You have spaces after macro name

> +
> +#define ATMEL_XLCDC_LAYER_SFACTC_A0_MULT_AS	(4 << 6)
> +#define ATMEL_XLCDC_LAYER_SFACTA_ONE		BIT(9)
> +#define ATMEL_XLCDC_LAYER_DFACTC_M_A0_MULT_AS	(6 << 11)
> +#define ATMEL_XLCDC_LAYER_DFACTA_ONE		BIT(14)
> +
> +#define ATMEL_XLCDC_LAYER_A0_SHIFT		16
> +#define ATMEL_XLCDC_LAYER_A0(x)			\
> +	((x) << ATMEL_XLCDC_LAYER_A0_SHIFT)
> +
> +#define ATMEL_XLCDC_LAYER_VSCALER_LUMA_ENABLE		BIT(0)
> +#define ATMEL_XLCDC_LAYER_VSCALER_CHROMA_ENABLE		BIT(1)
> +#define ATMEL_XLCDC_LAYER_HSCALER_LUMA_ENABLE		BIT(4)
> +#define ATMEL_XLCDC_LAYER_HSCALER_CHROMA_ENABLE		BIT(5)
> +
> +#define ATMEL_XLCDC_LAYER_VXSYCFG_ONE		BIT(0)
> +#define ATMEL_XLCDC_LAYER_VXSYTAP2_ENABLE	BIT(4)
> +#define ATMEL_XLCDC_LAYER_VXSCCFG_ONE		BIT(16)
> +#define ATMEL_XLCDC_LAYER_VXSCTAP2_ENABLE	BIT(20)
> +
> +#define ATMEL_XLCDC_LAYER_HXSYCFG_ONE		BIT(0)
> +#define ATMEL_XLCDC_LAYER_HXSYTAP2_ENABLE	BIT(4)
> +#define ATMEL_XLCDC_LAYER_HXSCCFG_ONE		BIT(16)
> +#define ATMEL_XLCDC_LAYER_HXSCTAP2_ENABLE	BIT(20)
> +
>  /**
>   * Atmel HLCDC Layer registers layout structure
>   *
> diff --git a/include/linux/mfd/atmel-hlcdc.h b/include/linux/mfd/atmel-hlcdc.h
> index a186119a49b5..80d675a03b39 100644
> --- a/include/linux/mfd/atmel-hlcdc.h
> +++ b/include/linux/mfd/atmel-hlcdc.h
> @@ -22,6 +22,8 @@
>  #define ATMEL_HLCDC_DITHER		BIT(6)
>  #define ATMEL_HLCDC_DISPDLY		BIT(7)
>  #define ATMEL_HLCDC_MODE_MASK		GENMASK(9, 8)
> +#define ATMEL_XLCDC_MODE_MASK		GENMASK(10, 8)
> +#define ATMEL_XLCDC_DPI			BIT(11)
>  #define ATMEL_HLCDC_PP			BIT(10)
>  #define ATMEL_HLCDC_VSPSU		BIT(12)
>  #define ATMEL_HLCDC_VSPHO		BIT(13)
> @@ -34,6 +36,12 @@
>  #define ATMEL_HLCDC_IDR			0x30
>  #define ATMEL_HLCDC_IMR			0x34
>  #define ATMEL_HLCDC_ISR			0x38
> +#define ATMEL_XLCDC_ATTRE		0x3c
> +
> +#define ATMEL_XLCDC_BASE_UPDATE		BIT(0)
> +#define ATMEL_XLCDC_OVR1_UPDATE		BIT(1)
> +#define ATMEL_XLCDC_OVR3_UPDATE		BIT(2)
> +#define ATMEL_XLCDC_HEO_UPDATE		BIT(3)
>  
>  #define ATMEL_HLCDC_CLKPOL		BIT(0)
>  #define ATMEL_HLCDC_CLKSEL		BIT(2)
> @@ -48,6 +56,8 @@
>  #define ATMEL_HLCDC_DISP		BIT(2)
>  #define ATMEL_HLCDC_PWM			BIT(3)
>  #define ATMEL_HLCDC_SIP			BIT(4)
> +#define ATMEL_XLCDC_SD			BIT(5)
> +#define ATMEL_XLCDC_CM			BIT(6)
>  
>  #define ATMEL_HLCDC_SOF			BIT(0)
>  #define ATMEL_HLCDC_SYNCDIS		BIT(1)

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 5/8] drm: atmel_hlcdc: Add support for XLCDC in atmel LCD driver

[Manikandan Muralidharan]

- XLCDC in SAM9X7 has different sets of registers and additional
configuration bits when compared to previous HLCDC IP. Read/write
operation on the controller registers is now separated using the
XLCDC status flag.
	- HEO scaling, window resampling, Alpha blending, YUV-to-RGB
conversion in XLCDC is derived and handled using additional
configuration bits and registers.
	- Writing one to the Enable fields of each layer in LCD_ATTRE
is required to reflect the values set in Configuration, FBA, Enable
registers of each layer

Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
Co-developed-by: Hari Prasath Gujulan Elango <Hari.PrasathGE@microchip.com>
Signed-off-by: Hari Prasath Gujulan Elango <Hari.PrasathGE@microchip.com>
Co-developed-by: Durai Manickam KR <durai.manickamkr@microchip.com>
Signed-off-by: Durai Manickam KR <durai.manickamkr@microchip.com>
---
 .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c    |  25 +-
 .../gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c   | 333 +++++++++++++++---
 2 files changed, 299 insertions(+), 59 deletions(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
index cc5cf4c2faf7..4b11a1de8af4 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
@@ -79,6 +79,7 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c)
 	unsigned int mask = ATMEL_HLCDC_CLKDIV_MASK | ATMEL_HLCDC_CLKPOL;
 	unsigned int cfg = 0;
 	int div, ret;
+	bool is_xlcdc = crtc->dc->desc->is_xlcdc;
 
 	/* get encoder from crtc */
 	drm_for_each_encoder(en_iter, ddev) {
@@ -164,10 +165,10 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c)
 	state = drm_crtc_state_to_atmel_hlcdc_crtc_state(c->state);
 	cfg = state->output_mode << 8;
 
-	if (adj->flags & DRM_MODE_FLAG_NVSYNC)
+	if (!is_xlcdc && (adj->flags & DRM_MODE_FLAG_NVSYNC))
 		cfg |= ATMEL_HLCDC_VSPOL;
 
-	if (adj->flags & DRM_MODE_FLAG_NHSYNC)
+	if (!is_xlcdc && (adj->flags & DRM_MODE_FLAG_NHSYNC))
 		cfg |= ATMEL_HLCDC_HSPOL;
 
 	regmap_update_bits(regmap, ATMEL_HLCDC_CFG(5),
@@ -202,6 +203,16 @@ static void atmel_hlcdc_crtc_atomic_disable(struct drm_crtc *c,
 
 	pm_runtime_get_sync(dev->dev);
 
+	if (crtc->dc->desc->is_xlcdc) {
+		regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_XLCDC_CM);
+		regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
+					 !(status & ATMEL_XLCDC_CM), 10, 0);
+
+		regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_XLCDC_SD);
+		regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
+					 status & ATMEL_XLCDC_SD, 10, 0);
+	}
+
 	regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_HLCDC_DISP);
 	while (!regmap_read(regmap, ATMEL_HLCDC_SR, &status) &&
 	       (status & ATMEL_HLCDC_DISP))
@@ -256,6 +267,16 @@ static void atmel_hlcdc_crtc_atomic_enable(struct drm_crtc *c,
 	       !(status & ATMEL_HLCDC_DISP))
 		cpu_relax();
 
+	if (crtc->dc->desc->is_xlcdc) {
+		regmap_write(regmap, ATMEL_HLCDC_EN, ATMEL_XLCDC_CM);
+		regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
+					 status & ATMEL_XLCDC_CM, 10, 0);
+
+		regmap_write(regmap, ATMEL_HLCDC_EN, ATMEL_XLCDC_SD);
+		regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
+					 !(status & ATMEL_XLCDC_SD), 10, 0);
+	}
+
 	pm_runtime_put_sync(dev->dev);
 
 }
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
index daa508504f47..26caf4cddfa4 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
@@ -330,11 +330,59 @@ static void atmel_hlcdc_plane_setup_scaler(struct atmel_hlcdc_plane *plane,
 								     yfactor));
 }
 
+static void atmel_xlcdc_plane_setup_scaler(struct atmel_hlcdc_plane *plane,
+					   struct atmel_hlcdc_plane_state *state)
+{
+	const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
+	u32 xfactor, yfactor;
+
+	if (!desc->layout.scaler_config)
+		return;
+
+	if (state->crtc_w == state->src_w && state->crtc_h == state->src_h) {
+		atmel_hlcdc_layer_write_cfg(&plane->layer,
+					    desc->layout.scaler_config, 0);
+		return;
+	}
+
+	/* xfactor = round[(2^20 * XMEMSIZE)/XSIZE)] */
+	xfactor = (u32)(((1 << 20) * state->src_w) / state->crtc_w);
+
+	/* yfactor = round[(2^20 * YMEMSIZE)/YSIZE)] */
+	yfactor = (u32)(((1 << 20) * state->src_h) / state->crtc_h);
+
+	atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config,
+				    ATMEL_XLCDC_LAYER_VSCALER_LUMA_ENABLE |
+				    ATMEL_XLCDC_LAYER_VSCALER_CHROMA_ENABLE |
+				    ATMEL_XLCDC_LAYER_HSCALER_LUMA_ENABLE |
+				    ATMEL_XLCDC_LAYER_HSCALER_CHROMA_ENABLE);
+
+	atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 1,
+				    yfactor);
+	atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 3,
+				    xfactor);
+
+	/* As per YCbCr window resampling configuration */
+	if (state->base.fb->format->format == DRM_FORMAT_YUV420) {
+		atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 2,
+					    yfactor / 2);
+		atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 4,
+					    xfactor / 2);
+	} else {
+		/* As per ARGB window resampling configuration */
+		atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 2,
+					    yfactor);
+		atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 4,
+					    xfactor);
+	}
+}
+
 static void
 atmel_hlcdc_plane_update_pos_and_size(struct atmel_hlcdc_plane *plane,
 				      struct atmel_hlcdc_plane_state *state)
 {
 	const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
+	struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
 
 	if (desc->layout.size)
 		atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.size,
@@ -352,7 +400,10 @@ atmel_hlcdc_plane_update_pos_and_size(struct atmel_hlcdc_plane *plane,
 					ATMEL_HLCDC_LAYER_POS(state->crtc_x,
 							      state->crtc_y));
 
-	atmel_hlcdc_plane_setup_scaler(plane, state);
+	if (dc->desc->is_xlcdc)
+		atmel_xlcdc_plane_setup_scaler(plane, state);
+	else
+		atmel_hlcdc_plane_setup_scaler(plane, state);
 }
 
 static void
@@ -393,6 +444,40 @@ atmel_hlcdc_plane_update_general_settings(struct atmel_hlcdc_plane *plane,
 				    cfg);
 }
 
+static void
+atmel_xlcdc_plane_update_general_settings(struct atmel_hlcdc_plane *plane,
+					  struct atmel_hlcdc_plane_state *state)
+{
+	unsigned int cfg;
+	const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
+	const struct drm_format_info *format = state->base.fb->format;
+
+	atmel_hlcdc_layer_write_cfg(&plane->layer, ATMEL_XLCDC_LAYER_DMA_CFG,
+				    ATMEL_HLCDC_LAYER_DMA_BLEN_INCR16 | state->ahb_id);
+
+	cfg = ATMEL_XLCDC_LAYER_DMA | ATMEL_XLCDC_LAYER_REP;
+
+	if (plane->base.type != DRM_PLANE_TYPE_PRIMARY) {
+		/*
+		 * Alpha Blending bits specific to SAM9X7 SoC
+		 */
+		cfg |= ATMEL_XLCDC_LAYER_SFACTC_A0_MULT_AS |
+		       ATMEL_XLCDC_LAYER_SFACTA_ONE |
+		       ATMEL_XLCDC_LAYER_DFACTC_M_A0_MULT_AS |
+		       ATMEL_XLCDC_LAYER_DFACTA_ONE;
+		if (format->has_alpha)
+			cfg |= ATMEL_XLCDC_LAYER_A0(0xff);
+		else
+			cfg |= ATMEL_XLCDC_LAYER_A0(state->base.alpha);
+	}
+
+	if (state->disc_h && state->disc_w)
+		cfg |= ATMEL_XLCDC_LAYER_DISCEN;
+
+	atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.general_config,
+				    cfg);
+}
+
 static void atmel_hlcdc_plane_update_format(struct atmel_hlcdc_plane *plane,
 					struct atmel_hlcdc_plane_state *state)
 {
@@ -437,36 +522,55 @@ static void atmel_hlcdc_plane_update_clut(struct atmel_hlcdc_plane *plane,
 	}
 }
 
+static void update_hlcdc_buffers(struct atmel_hlcdc_plane *plane,
+				 struct atmel_hlcdc_plane_state *state, u32 sr, int i)
+{
+	atmel_hlcdc_layer_write_reg(&plane->layer,
+				    ATMEL_HLCDC_LAYER_PLANE_HEAD(i),
+				    state->dscrs[i]->self);
+
+	if (!(sr & ATMEL_HLCDC_LAYER_EN)) {
+		atmel_hlcdc_layer_write_reg(&plane->layer,
+					    ATMEL_HLCDC_LAYER_PLANE_ADDR(i),
+					    state->dscrs[i]->addr);
+		atmel_hlcdc_layer_write_reg(&plane->layer,
+					    ATMEL_HLCDC_LAYER_PLANE_CTRL(i),
+					    state->dscrs[i]->ctrl);
+		atmel_hlcdc_layer_write_reg(&plane->layer,
+					    ATMEL_HLCDC_LAYER_PLANE_NEXT(i),
+					    state->dscrs[i]->self);
+	}
+}
+
+static void update_xlcdc_buffers(struct atmel_hlcdc_plane *plane,
+				 struct atmel_hlcdc_plane_state *state, int i)
+{
+	atmel_hlcdc_layer_write_reg(&plane->layer,
+				    ATMEL_XLCDC_LAYER_PLANE_ADDR(i),
+				    state->dscrs[i]->addr);
+}
+
 static void atmel_hlcdc_plane_update_buffers(struct atmel_hlcdc_plane *plane,
-					struct atmel_hlcdc_plane_state *state)
+					     struct atmel_hlcdc_plane_state *state)
 {
 	const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
+	struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
 	struct drm_framebuffer *fb = state->base.fb;
 	u32 sr;
 	int i;
 
-	sr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHSR);
+	if (!dc->desc->is_xlcdc)
+		sr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHSR);
 
 	for (i = 0; i < state->nplanes; i++) {
 		struct drm_gem_dma_object *gem = drm_fb_dma_get_gem_obj(fb, i);
 
 		state->dscrs[i]->addr = gem->dma_addr + state->offsets[i];
 
-		atmel_hlcdc_layer_write_reg(&plane->layer,
-					    ATMEL_HLCDC_LAYER_PLANE_HEAD(i),
-					    state->dscrs[i]->self);
-
-		if (!(sr & ATMEL_HLCDC_LAYER_EN)) {
-			atmel_hlcdc_layer_write_reg(&plane->layer,
-					ATMEL_HLCDC_LAYER_PLANE_ADDR(i),
-					state->dscrs[i]->addr);
-			atmel_hlcdc_layer_write_reg(&plane->layer,
-					ATMEL_HLCDC_LAYER_PLANE_CTRL(i),
-					state->dscrs[i]->ctrl);
-			atmel_hlcdc_layer_write_reg(&plane->layer,
-					ATMEL_HLCDC_LAYER_PLANE_NEXT(i),
-					state->dscrs[i]->self);
-		}
+		if (dc->desc->is_xlcdc)
+			update_xlcdc_buffers(plane, state, i);
+		else
+			update_hlcdc_buffers(plane, state, sr, i);
 
 		if (desc->layout.xstride[i])
 			atmel_hlcdc_layer_write_cfg(&plane->layer,
@@ -712,11 +816,8 @@ static int atmel_hlcdc_plane_atomic_check(struct drm_plane *p,
 	return 0;
 }
 
-static void atmel_hlcdc_plane_atomic_disable(struct drm_plane *p,
-					     struct drm_atomic_state *state)
+static void hlcdc_atomic_disable(struct atmel_hlcdc_plane *plane)
 {
-	struct atmel_hlcdc_plane *plane = drm_plane_to_atmel_hlcdc_plane(p);
-
 	/* Disable interrupts */
 	atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_IDR,
 				    0xffffffff);
@@ -731,6 +832,72 @@ static void atmel_hlcdc_plane_atomic_disable(struct drm_plane *p,
 	atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_ISR);
 }
 
+static void xlcdc_atomic_disable(struct atmel_hlcdc_plane *plane)
+{
+	/* Disable interrupts */
+	atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_XLCDC_LAYER_IDR,
+				    0xffffffff);
+
+	/* Disable the layer */
+	atmel_hlcdc_layer_write_reg(&plane->layer,
+				    ATMEL_XLCDC_LAYER_ENR, 0);
+
+	/* Clear all pending interrupts */
+	atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_XLCDC_LAYER_ISR);
+}
+
+static void atmel_hlcdc_plane_atomic_disable(struct drm_plane *p,
+					     struct drm_atomic_state *state)
+{
+	struct atmel_hlcdc_plane *plane = drm_plane_to_atmel_hlcdc_plane(p);
+	struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
+
+	if (dc->desc->is_xlcdc)
+		xlcdc_atomic_disable(plane);
+	else
+		hlcdc_atomic_disable(plane);
+}
+
+static void hlcdc_atomic_update(struct atmel_hlcdc_plane *plane)
+{
+	u32 sr;
+
+	/* Enable the overrun interrupts. */
+	atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_IER,
+				    ATMEL_HLCDC_LAYER_OVR_IRQ(0) |
+				    ATMEL_HLCDC_LAYER_OVR_IRQ(1) |
+				    ATMEL_HLCDC_LAYER_OVR_IRQ(2));
+
+	/* Apply the new config at the next SOF event. */
+	sr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHSR);
+	atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHER,
+				    ATMEL_HLCDC_LAYER_UPDATE |
+				    (sr & ATMEL_HLCDC_LAYER_EN ?
+				    ATMEL_HLCDC_LAYER_A2Q : ATMEL_HLCDC_LAYER_EN));
+}
+
+static void xlcdc_atomic_update(struct atmel_hlcdc_plane *plane,
+				struct atmel_hlcdc_dc *dc)
+{
+	/* Enable the overrun interrupts. */
+	atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_XLCDC_LAYER_IER,
+				    ATMEL_XLCDC_LAYER_OVR_IRQ(0) |
+				    ATMEL_XLCDC_LAYER_OVR_IRQ(1) |
+				    ATMEL_XLCDC_LAYER_OVR_IRQ(2));
+
+	atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_XLCDC_LAYER_ENR,
+				    ATMEL_XLCDC_LAYER_EN);
+
+	/*
+	 * Updating XLCDC_xxxCFGx, XLCDC_xxxFBA and XLCDC_xxxEN,
+	 * (where xxx indicates each layer) requires writing one to the
+	 * Update Attribute field for each layer in LCDC_ATTRE register for SAM9X7.
+	 */
+	regmap_write(dc->hlcdc->regmap, ATMEL_XLCDC_ATTRE, ATMEL_XLCDC_BASE_UPDATE |
+		     ATMEL_XLCDC_OVR1_UPDATE | ATMEL_XLCDC_OVR3_UPDATE |
+		     ATMEL_XLCDC_HEO_UPDATE);
+}
+
 static void atmel_hlcdc_plane_atomic_update(struct drm_plane *p,
 					    struct drm_atomic_state *state)
 {
@@ -739,7 +906,7 @@ static void atmel_hlcdc_plane_atomic_update(struct drm_plane *p,
 	struct atmel_hlcdc_plane *plane = drm_plane_to_atmel_hlcdc_plane(p);
 	struct atmel_hlcdc_plane_state *hstate =
 			drm_plane_state_to_atmel_hlcdc_plane_state(new_s);
-	u32 sr;
+	struct atmel_hlcdc_dc *dc = p->dev->dev_private;
 
 	if (!new_s->crtc || !new_s->fb)
 		return;
@@ -750,29 +917,67 @@ static void atmel_hlcdc_plane_atomic_update(struct drm_plane *p,
 	}
 
 	atmel_hlcdc_plane_update_pos_and_size(plane, hstate);
-	atmel_hlcdc_plane_update_general_settings(plane, hstate);
+	if (dc->desc->is_xlcdc)
+		atmel_xlcdc_plane_update_general_settings(plane, hstate);
+	else
+		atmel_hlcdc_plane_update_general_settings(plane, hstate);
 	atmel_hlcdc_plane_update_format(plane, hstate);
 	atmel_hlcdc_plane_update_clut(plane, hstate);
 	atmel_hlcdc_plane_update_buffers(plane, hstate);
 	atmel_hlcdc_plane_update_disc_area(plane, hstate);
 
-	/* Enable the overrun interrupts. */
-	atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_IER,
-				    ATMEL_HLCDC_LAYER_OVR_IRQ(0) |
-				    ATMEL_HLCDC_LAYER_OVR_IRQ(1) |
-				    ATMEL_HLCDC_LAYER_OVR_IRQ(2));
+	if (dc->desc->is_xlcdc)
+		xlcdc_atomic_update(plane, dc);
+	else
+		hlcdc_atomic_update(plane);
+}
 
-	/* Apply the new config at the next SOF event. */
-	sr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHSR);
-	atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHER,
-			ATMEL_HLCDC_LAYER_UPDATE |
-			(sr & ATMEL_HLCDC_LAYER_EN ?
-			 ATMEL_HLCDC_LAYER_A2Q : ATMEL_HLCDC_LAYER_EN));
+u32 hlcdc_csc_coeffs[] = {
+	0x4c900091,
+	0x7a5f5090,
+	0x40040890
+};
+
+u32 xlcdc_csc_coeffs[] = {
+	0x00000488,
+	0x00000648,
+	0x1EA00480,
+	0x00001D28,
+	0x08100480,
+	0x00000000,
+	0x00000007
+};
+
+static void hlcdc_csc_init(struct atmel_hlcdc_plane *plane,
+			   const struct atmel_hlcdc_layer_desc *desc)
+{
+	/*
+	 * TODO: declare a "yuv-to-rgb-conv-factors" property to let
+	 * userspace modify these factors (using a BLOB property ?).
+	 */
+	for (int i = 0; i < ARRAY_SIZE(hlcdc_csc_coeffs); i++)
+		atmel_hlcdc_layer_write_cfg(&plane->layer,
+					    desc->layout.csc + i,
+					    hlcdc_csc_coeffs[i]);
+}
+
+static void xlcdc_csc_init(struct atmel_hlcdc_plane *plane,
+			   const struct atmel_hlcdc_layer_desc *desc)
+{
+	/*
+	 * yuv-to-rgb-conv-factors are now defined from LCDC_HEOCFG16 to
+	 * LCDC_HEOCFG21 registers in SAM9X7.
+	 */
+	for (int i = 0; i < ARRAY_SIZE(xlcdc_csc_coeffs); i++)
+		atmel_hlcdc_layer_write_cfg(&plane->layer,
+					    desc->layout.csc + i,
+					    xlcdc_csc_coeffs[i]);
 }
 
 static int atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane)
 {
 	const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
+	struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
 
 	if (desc->type == ATMEL_HLCDC_OVERLAY_LAYER ||
 	    desc->type == ATMEL_HLCDC_CURSOR_LAYER) {
@@ -796,31 +1001,19 @@ static int atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane)
 			return ret;
 	}
 
-	if (desc->layout.csc) {
-		/*
-		 * TODO: decare a "yuv-to-rgb-conv-factors" property to let
-		 * userspace modify these factors (using a BLOB property ?).
-		 */
-		atmel_hlcdc_layer_write_cfg(&plane->layer,
-					    desc->layout.csc,
-					    0x4c900091);
-		atmel_hlcdc_layer_write_cfg(&plane->layer,
-					    desc->layout.csc + 1,
-					    0x7a5f5090);
-		atmel_hlcdc_layer_write_cfg(&plane->layer,
-					    desc->layout.csc + 2,
-					    0x40040890);
-	}
+	if (dc->desc->is_xlcdc && desc->layout.csc)
+		xlcdc_csc_init(plane, desc);
+	else
+		if (desc->layout.csc)
+			hlcdc_csc_init(plane, desc);
 
 	return 0;
 }
 
-void atmel_hlcdc_plane_irq(struct atmel_hlcdc_plane *plane)
+static void hlcdc_irq_dbg(struct atmel_hlcdc_plane *plane,
+			  const struct atmel_hlcdc_layer_desc *desc)
 {
-	const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
-	u32 isr;
-
-	isr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_ISR);
+	u32 isr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_ISR);
 
 	/*
 	 * There's not much we can do in case of overrun except informing
@@ -830,8 +1023,34 @@ void atmel_hlcdc_plane_irq(struct atmel_hlcdc_plane *plane)
 	if (isr &
 	    (ATMEL_HLCDC_LAYER_OVR_IRQ(0) | ATMEL_HLCDC_LAYER_OVR_IRQ(1) |
 	     ATMEL_HLCDC_LAYER_OVR_IRQ(2)))
-		dev_dbg(plane->base.dev->dev, "overrun on plane %s\n",
-			desc->name);
+		pr_warn("%s: overrun on plane %s\n", __func__, desc->name);
+}
+
+static void xlcdc_irq_dbg(struct atmel_hlcdc_plane *plane,
+			  const struct atmel_hlcdc_layer_desc *desc)
+{
+	u32 isr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_XLCDC_LAYER_ISR);
+
+	/*
+	 * There's not much we can do in case of overrun except informing
+	 * the user. However, we are in interrupt context here, hence the
+	 * use of dev_dbg().
+	 */
+	if (isr &
+	    (ATMEL_XLCDC_LAYER_OVR_IRQ(0) | ATMEL_XLCDC_LAYER_OVR_IRQ(1) |
+	     ATMEL_XLCDC_LAYER_OVR_IRQ(2)))
+		pr_warn("%s: overrun on plane %s\n", __func__, desc->name);
+}
+
+void atmel_hlcdc_plane_irq(struct atmel_hlcdc_plane *plane)
+{
+	const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
+	struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
+
+	if (dc->desc->is_xlcdc)
+		xlcdc_irq_dbg(plane, desc);
+	else
+		hlcdc_irq_dbg(plane, desc);
 }
 
 static const struct drm_plane_helper_funcs atmel_hlcdc_layer_plane_helper_funcs = {
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 5/8] drm: atmel_hlcdc: Add support for XLCDC in atmel LCD driver

[claudiu beznea]

Hi, Manikandan,

On 8/25/23 15:54, Manikandan Muralidharan wrote:
> - XLCDC in SAM9X7 has different sets of registers and additional
> configuration bits when compared to previous HLCDC IP. Read/write
> operation on the controller registers is now separated using the
> XLCDC status flag.
> 	- HEO scaling, window resampling, Alpha blending, YUV-to-RGB
> conversion in XLCDC is derived and handled using additional
> configuration bits and registers.
> 	- Writing one to the Enable fields of each layer in LCD_ATTRE
> is required to reflect the values set in Configuration, FBA, Enable
> registers of each layer
> 
> Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
> Co-developed-by: Hari Prasath Gujulan Elango <Hari.PrasathGE@microchip.com>
> Signed-off-by: Hari Prasath Gujulan Elango <Hari.PrasathGE@microchip.com>
> Co-developed-by: Durai Manickam KR <durai.manickamkr@microchip.com>
> Signed-off-by: Durai Manickam KR <durai.manickamkr@microchip.com>
> ---
>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c    |  25 +-
>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c   | 333 +++++++++++++++---
>  2 files changed, 299 insertions(+), 59 deletions(-)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> index cc5cf4c2faf7..4b11a1de8af4 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> @@ -79,6 +79,7 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c)
>  	unsigned int mask = ATMEL_HLCDC_CLKDIV_MASK | ATMEL_HLCDC_CLKPOL;
>  	unsigned int cfg = 0;
>  	int div, ret;
> +	bool is_xlcdc = crtc->dc->desc->is_xlcdc;
>  
>  	/* get encoder from crtc */
>  	drm_for_each_encoder(en_iter, ddev) {
> @@ -164,10 +165,10 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c)
>  	state = drm_crtc_state_to_atmel_hlcdc_crtc_state(c->state);
>  	cfg = state->output_mode << 8;
>  
> -	if (adj->flags & DRM_MODE_FLAG_NVSYNC)
> +	if (!is_xlcdc && (adj->flags & DRM_MODE_FLAG_NVSYNC))
>  		cfg |= ATMEL_HLCDC_VSPOL;
>  
> -	if (adj->flags & DRM_MODE_FLAG_NHSYNC)
> +	if (!is_xlcdc && (adj->flags & DRM_MODE_FLAG_NHSYNC))
>  		cfg |= ATMEL_HLCDC_HSPOL;
>  
>  	regmap_update_bits(regmap, ATMEL_HLCDC_CFG(5),
> @@ -202,6 +203,16 @@ static void atmel_hlcdc_crtc_atomic_disable(struct drm_crtc *c,
>  
>  	pm_runtime_get_sync(dev->dev);
>  
> +	if (crtc->dc->desc->is_xlcdc) {
> +		regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_XLCDC_CM);
> +		regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
> +					 !(status & ATMEL_XLCDC_CM), 10, 0);

You may want to check the return value of regmap_read_poll_timeout().
Otherwise your setup may fail.

Also, regmap_read_poll_timeout() may sleep, the other settings in this
functions are done with bussy looping, is there a reason for that?

> +
> +		regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_XLCDC_SD);
> +		regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
> +					 status & ATMEL_XLCDC_SD, 10, 0);

Same here.

> +	}
> +
>  	regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_HLCDC_DISP);
>  	while (!regmap_read(regmap, ATMEL_HLCDC_SR, &status) &&
>  	       (status & ATMEL_HLCDC_DISP))
> @@ -256,6 +267,16 @@ static void atmel_hlcdc_crtc_atomic_enable(struct drm_crtc *c,
>  	       !(status & ATMEL_HLCDC_DISP))
>  		cpu_relax();
>  
> +	if (crtc->dc->desc->is_xlcdc) {
> +		regmap_write(regmap, ATMEL_HLCDC_EN, ATMEL_XLCDC_CM);
> +		regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
> +					 status & ATMEL_XLCDC_CM, 10, 0);
> +
> +		regmap_write(regmap, ATMEL_HLCDC_EN, ATMEL_XLCDC_SD);
> +		regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
> +					 !(status & ATMEL_XLCDC_SD), 10, 0);
> +	}
> +
>  	pm_runtime_put_sync(dev->dev);
>  
>  }
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> index daa508504f47..26caf4cddfa4 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> @@ -330,11 +330,59 @@ static void atmel_hlcdc_plane_setup_scaler(struct atmel_hlcdc_plane *plane,
>  								     yfactor));
>  }
>  
> +static void atmel_xlcdc_plane_setup_scaler(struct atmel_hlcdc_plane *plane,
> +					   struct atmel_hlcdc_plane_state *state)
> +{
> +	const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
> +	u32 xfactor, yfactor;
> +
> +	if (!desc->layout.scaler_config)
> +		return;
> +
> +	if (state->crtc_w == state->src_w && state->crtc_h == state->src_h) {
> +		atmel_hlcdc_layer_write_cfg(&plane->layer,
> +					    desc->layout.scaler_config, 0);
> +		return;
> +	}
> +
> +	/* xfactor = round[(2^20 * XMEMSIZE)/XSIZE)] */
> +	xfactor = (u32)(((1 << 20) * state->src_w) / state->crtc_w);

Could ((1 << 20) * state->src_w) overflow?

> +
> +	/* yfactor = round[(2^20 * YMEMSIZE)/YSIZE)] */
> +	yfactor = (u32)(((1 << 20) * state->src_h) / state->crtc_h);

Same here.

> +
> +	atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config,
> +				    ATMEL_XLCDC_LAYER_VSCALER_LUMA_ENABLE |
> +				    ATMEL_XLCDC_LAYER_VSCALER_CHROMA_ENABLE |
> +				    ATMEL_XLCDC_LAYER_HSCALER_LUMA_ENABLE |
> +				    ATMEL_XLCDC_LAYER_HSCALER_CHROMA_ENABLE);
> +
> +	atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 1,
> +				    yfactor);
> +	atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 3,
> +				    xfactor);
> +
> +	/* As per YCbCr window resampling configuration */
> +	if (state->base.fb->format->format == DRM_FORMAT_YUV420) {
> +		atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 2,
> +					    yfactor / 2);
> +		atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 4,
> +					    xfactor / 2);
> +	} else {
> +		/* As per ARGB window resampling configuration */
> +		atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 2,
> +					    yfactor);
> +		atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 4,
> +					    xfactor);
> +	}

This can be written as follows:
if (state->base.fb->format->format == DRM_FORMAT_YUV420) {
	yfactor /= 2);
	xfactor /= 2);
}
atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 2,
			    yfactor);
atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 4,
			    xfactor);



> +}
> +
>  static void
>  atmel_hlcdc_plane_update_pos_and_size(struct atmel_hlcdc_plane *plane,
>  				      struct atmel_hlcdc_plane_state *state)
>  {
>  	const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
> +	struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
>  
>  	if (desc->layout.size)
>  		atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.size,
> @@ -352,7 +400,10 @@ atmel_hlcdc_plane_update_pos_and_size(struct atmel_hlcdc_plane *plane,
>  					ATMEL_HLCDC_LAYER_POS(state->crtc_x,
>  							      state->crtc_y));
>  
> -	atmel_hlcdc_plane_setup_scaler(plane, state);
> +	if (dc->desc->is_xlcdc)
> +		atmel_xlcdc_plane_setup_scaler(plane, state);
> +	else
> +		atmel_hlcdc_plane_setup_scaler(plane, state);

What if you embedd the plane_setup_scaller function in struct
atmel_hlcdc_dc_desc and define per lcdc variant the proper function you can
have here something like:
	dc->desc->plane_setup_scaler();

This is valid for other places in this patch (see below).

>  }
>  
>  static void
> @@ -393,6 +444,40 @@ atmel_hlcdc_plane_update_general_settings(struct atmel_hlcdc_plane *plane,
>  				    cfg);
>  }
>  
> +static void
> +atmel_xlcdc_plane_update_general_settings(struct atmel_hlcdc_plane *plane,
> +					  struct atmel_hlcdc_plane_state *state)
> +{
> +	unsigned int cfg;
> +	const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
> +	const struct drm_format_info *format = state->base.fb->format;
> +
> +	atmel_hlcdc_layer_write_cfg(&plane->layer, ATMEL_XLCDC_LAYER_DMA_CFG,
> +				    ATMEL_HLCDC_LAYER_DMA_BLEN_INCR16 | state->ahb_id);
> +
> +	cfg = ATMEL_XLCDC_LAYER_DMA | ATMEL_XLCDC_LAYER_REP;
> +
> +	if (plane->base.type != DRM_PLANE_TYPE_PRIMARY) {
> +		/*
> +		 * Alpha Blending bits specific to SAM9X7 SoC
> +		 */
> +		cfg |= ATMEL_XLCDC_LAYER_SFACTC_A0_MULT_AS |
> +		       ATMEL_XLCDC_LAYER_SFACTA_ONE |
> +		       ATMEL_XLCDC_LAYER_DFACTC_M_A0_MULT_AS |
> +		       ATMEL_XLCDC_LAYER_DFACTA_ONE;
> +		if (format->has_alpha)
> +			cfg |= ATMEL_XLCDC_LAYER_A0(0xff);
> +		else
> +			cfg |= ATMEL_XLCDC_LAYER_A0(state->base.alpha);
> +	}
> +
> +	if (state->disc_h && state->disc_w)
> +		cfg |= ATMEL_XLCDC_LAYER_DISCEN;
> +
> +	atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.general_config,
> +				    cfg);
> +}
> +
>  static void atmel_hlcdc_plane_update_format(struct atmel_hlcdc_plane *plane,
>  					struct atmel_hlcdc_plane_state *state)
>  {
> @@ -437,36 +522,55 @@ static void atmel_hlcdc_plane_update_clut(struct atmel_hlcdc_plane *plane,
>  	}
>  }
>  
> +static void update_hlcdc_buffers(struct atmel_hlcdc_plane *plane,
> +				 struct atmel_hlcdc_plane_state *state, u32 sr, int i)
> +{
> +	atmel_hlcdc_layer_write_reg(&plane->layer,
> +				    ATMEL_HLCDC_LAYER_PLANE_HEAD(i),
> +				    state->dscrs[i]->self);
> +
> +	if (!(sr & ATMEL_HLCDC_LAYER_EN)) {

To avoid extra indenting:
	if (sr & ATMEL_HLCDC_LAYER_EN)
		return;

> +		atmel_hlcdc_layer_write_reg(&plane->layer,
> +					    ATMEL_HLCDC_LAYER_PLANE_ADDR(i),
> +					    state->dscrs[i]->addr);
> +		atmel_hlcdc_layer_write_reg(&plane->layer,
> +					    ATMEL_HLCDC_LAYER_PLANE_CTRL(i),
> +					    state->dscrs[i]->ctrl);
> +		atmel_hlcdc_layer_write_reg(&plane->layer,
> +					    ATMEL_HLCDC_LAYER_PLANE_NEXT(i),
> +					    state->dscrs[i]->self);
> +	}
> +}
> +
> +static void update_xlcdc_buffers(struct atmel_hlcdc_plane *plane,
> +				 struct atmel_hlcdc_plane_state *state, int i)
> +{
> +	atmel_hlcdc_layer_write_reg(&plane->layer,
> +				    ATMEL_XLCDC_LAYER_PLANE_ADDR(i),
> +				    state->dscrs[i]->addr);
> +}
> +
>  static void atmel_hlcdc_plane_update_buffers(struct atmel_hlcdc_plane *plane,
> -					struct atmel_hlcdc_plane_state *state)
> +					     struct atmel_hlcdc_plane_state *state)

update_hlcdc_buffers() and update_xlcdc_buffers() could also be members in
the struct atmel_hlcdc_dc_desc and called accordingly where needed w/o
checking is_xlcdc.

>  {
>  	const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
> +	struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
>  	struct drm_framebuffer *fb = state->base.fb;
>  	u32 sr;
>  	int i;
>  
> -	sr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHSR);
> +	if (!dc->desc->is_xlcdc)
> +		sr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHSR);
>  
>  	for (i = 0; i < state->nplanes; i++) {
>  		struct drm_gem_dma_object *gem = drm_fb_dma_get_gem_obj(fb, i);
>  
>  		state->dscrs[i]->addr = gem->dma_addr + state->offsets[i];
>  
> -		atmel_hlcdc_layer_write_reg(&plane->layer,
> -					    ATMEL_HLCDC_LAYER_PLANE_HEAD(i),
> -					    state->dscrs[i]->self);
> -
> -		if (!(sr & ATMEL_HLCDC_LAYER_EN)) {
> -			atmel_hlcdc_layer_write_reg(&plane->layer,
> -					ATMEL_HLCDC_LAYER_PLANE_ADDR(i),
> -					state->dscrs[i]->addr);
> -			atmel_hlcdc_layer_write_reg(&plane->layer,
> -					ATMEL_HLCDC_LAYER_PLANE_CTRL(i),
> -					state->dscrs[i]->ctrl);
> -			atmel_hlcdc_layer_write_reg(&plane->layer,
> -					ATMEL_HLCDC_LAYER_PLANE_NEXT(i),
> -					state->dscrs[i]->self);
> -		}
> +		if (dc->desc->is_xlcdc)
> +			update_xlcdc_buffers(plane, state, i);
> +		else
> +			update_hlcdc_buffers(plane, state, sr, i);

And here you can have something like:
		dc->desc->update_lcdc_buffers();
>  
>  		if (desc->layout.xstride[i])
>  			atmel_hlcdc_layer_write_cfg(&plane->layer,
> @@ -712,11 +816,8 @@ static int atmel_hlcdc_plane_atomic_check(struct drm_plane *p,
>  	return 0;
>  }
>  
> -static void atmel_hlcdc_plane_atomic_disable(struct drm_plane *p,
> -					     struct drm_atomic_state *state)
> +static void hlcdc_atomic_disable(struct atmel_hlcdc_plane *plane)
>  {
> -	struct atmel_hlcdc_plane *plane = drm_plane_to_atmel_hlcdc_plane(p);
> -
>  	/* Disable interrupts */
>  	atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_IDR,
>  				    0xffffffff);
> @@ -731,6 +832,72 @@ static void atmel_hlcdc_plane_atomic_disable(struct drm_plane *p,
>  	atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_ISR);
>  }
>  
> +static void xlcdc_atomic_disable(struct atmel_hlcdc_plane *plane)
> +{
> +	/* Disable interrupts */
> +	atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_XLCDC_LAYER_IDR,
> +				    0xffffffff);
> +
> +	/* Disable the layer */
> +	atmel_hlcdc_layer_write_reg(&plane->layer,
> +				    ATMEL_XLCDC_LAYER_ENR, 0);
> +
> +	/* Clear all pending interrupts */
> +	atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_XLCDC_LAYER_ISR);
> +}
> +
> +static void atmel_hlcdc_plane_atomic_disable(struct drm_plane *p,
> +					     struct drm_atomic_state *state)
> +{
> +	struct atmel_hlcdc_plane *plane = drm_plane_to_atmel_hlcdc_plane(p);
> +	struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
> +
> +	if (dc->desc->is_xlcdc)
> +		xlcdc_atomic_disable(plane);
> +	else
> +		hlcdc_atomic_disable(plane);

	dc->desc->lcdc_atomic_disable();

> +}
> +
> +static void hlcdc_atomic_update(struct atmel_hlcdc_plane *plane)
> +{
> +	u32 sr;
> +
> +	/* Enable the overrun interrupts. */
> +	atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_IER,
> +				    ATMEL_HLCDC_LAYER_OVR_IRQ(0) |
> +				    ATMEL_HLCDC_LAYER_OVR_IRQ(1) |
> +				    ATMEL_HLCDC_LAYER_OVR_IRQ(2));
> +
> +	/* Apply the new config at the next SOF event. */
> +	sr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHSR);
> +	atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHER,
> +				    ATMEL_HLCDC_LAYER_UPDATE |
> +				    (sr & ATMEL_HLCDC_LAYER_EN ?
> +				    ATMEL_HLCDC_LAYER_A2Q : ATMEL_HLCDC_LAYER_EN));
> +}
> +
> +static void xlcdc_atomic_update(struct atmel_hlcdc_plane *plane,
> +				struct atmel_hlcdc_dc *dc)
> +{
> +	/* Enable the overrun interrupts. */
> +	atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_XLCDC_LAYER_IER,
> +				    ATMEL_XLCDC_LAYER_OVR_IRQ(0) |
> +				    ATMEL_XLCDC_LAYER_OVR_IRQ(1) |
> +				    ATMEL_XLCDC_LAYER_OVR_IRQ(2));
> +
> +	atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_XLCDC_LAYER_ENR,
> +				    ATMEL_XLCDC_LAYER_EN);
> +
> +	/*
> +	 * Updating XLCDC_xxxCFGx, XLCDC_xxxFBA and XLCDC_xxxEN,
> +	 * (where xxx indicates each layer) requires writing one to the
> +	 * Update Attribute field for each layer in LCDC_ATTRE register for SAM9X7.
> +	 */
> +	regmap_write(dc->hlcdc->regmap, ATMEL_XLCDC_ATTRE, ATMEL_XLCDC_BASE_UPDATE |
> +		     ATMEL_XLCDC_OVR1_UPDATE | ATMEL_XLCDC_OVR3_UPDATE |
> +		     ATMEL_XLCDC_HEO_UPDATE);
> +}
> +
>  static void atmel_hlcdc_plane_atomic_update(struct drm_plane *p,
>  					    struct drm_atomic_state *state)
>  {
> @@ -739,7 +906,7 @@ static void atmel_hlcdc_plane_atomic_update(struct drm_plane *p,
>  	struct atmel_hlcdc_plane *plane = drm_plane_to_atmel_hlcdc_plane(p);
>  	struct atmel_hlcdc_plane_state *hstate =
>  			drm_plane_state_to_atmel_hlcdc_plane_state(new_s);
> -	u32 sr;
> +	struct atmel_hlcdc_dc *dc = p->dev->dev_private;
>  
>  	if (!new_s->crtc || !new_s->fb)
>  		return;
> @@ -750,29 +917,67 @@ static void atmel_hlcdc_plane_atomic_update(struct drm_plane *p,
>  	}
>  
>  	atmel_hlcdc_plane_update_pos_and_size(plane, hstate);
> -	atmel_hlcdc_plane_update_general_settings(plane, hstate);
> +	if (dc->desc->is_xlcdc)
> +		atmel_xlcdc_plane_update_general_settings(plane, hstate);
> +	else
> +		atmel_hlcdc_plane_update_general_settings(plane, hstate);

	dc->desc->lcdc_plane_update_general_settigns();

>  	atmel_hlcdc_plane_update_format(plane, hstate);
>  	atmel_hlcdc_plane_update_clut(plane, hstate);
>  	atmel_hlcdc_plane_update_buffers(plane, hstate);
>  	atmel_hlcdc_plane_update_disc_area(plane, hstate);
>  
> -	/* Enable the overrun interrupts. */
> -	atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_IER,
> -				    ATMEL_HLCDC_LAYER_OVR_IRQ(0) |
> -				    ATMEL_HLCDC_LAYER_OVR_IRQ(1) |
> -				    ATMEL_HLCDC_LAYER_OVR_IRQ(2));
> +	if (dc->desc->is_xlcdc)
> +		xlcdc_atomic_update(plane, dc);
> +	else
> +		hlcdc_atomic_update(plane);

	dc->desc->lcdc_atomic_update();

> +}
>  
> -	/* Apply the new config at the next SOF event. */
> -	sr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHSR);
> -	atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHER,
> -			ATMEL_HLCDC_LAYER_UPDATE |
> -			(sr & ATMEL_HLCDC_LAYER_EN ?
> -			 ATMEL_HLCDC_LAYER_A2Q : ATMEL_HLCDC_LAYER_EN));
> +u32 hlcdc_csc_coeffs[] = {
> +	0x4c900091,
> +	0x7a5f5090,
> +	0x40040890
> +};

You can move this in function where it is used and declare it static.

> +
> +u32 xlcdc_csc_coeffs[] = {
> +	0x00000488,
> +	0x00000648,
> +	0x1EA00480,
> +	0x00001D28,
> +	0x08100480,
> +	0x00000000,
> +	0x00000007
> +};

Same here.

> +
> +static void hlcdc_csc_init(struct atmel_hlcdc_plane *plane,
> +			   const struct atmel_hlcdc_layer_desc *desc)
> +{
> +	/*
> +	 * TODO: declare a "yuv-to-rgb-conv-factors" property to let
> +	 * userspace modify these factors (using a BLOB property ?).
> +	 */
> +	for (int i = 0; i < ARRAY_SIZE(hlcdc_csc_coeffs); i++)

{ at the end of for statement.

> +		atmel_hlcdc_layer_write_cfg(&plane->layer,
> +					    desc->layout.csc + i,
> +					    hlcdc_csc_coeffs[i]);

} here

> +}
> +
> +static void xlcdc_csc_init(struct atmel_hlcdc_plane *plane,
> +			   const struct atmel_hlcdc_layer_desc *desc)
> +{
> +	/*
> +	 * yuv-to-rgb-conv-factors are now defined from LCDC_HEOCFG16 to
> +	 * LCDC_HEOCFG21 registers in SAM9X7.
> +	 */
> +	for (int i = 0; i < ARRAY_SIZE(xlcdc_csc_coeffs); i++)
> +		atmel_hlcdc_layer_write_cfg(&plane->layer,
> +					    desc->layout.csc + i,
> +					    xlcdc_csc_coeffs[i]);

Ditto

>  }
>  
>  static int atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane)
>  {
>  	const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
> +	struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
>  
>  	if (desc->type == ATMEL_HLCDC_OVERLAY_LAYER ||
>  	    desc->type == ATMEL_HLCDC_CURSOR_LAYER) {
> @@ -796,31 +1001,19 @@ static int atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane)
>  			return ret;
>  	}
>  
> -	if (desc->layout.csc) {
> -		/*
> -		 * TODO: decare a "yuv-to-rgb-conv-factors" property to let
> -		 * userspace modify these factors (using a BLOB property ?).
> -		 */
> -		atmel_hlcdc_layer_write_cfg(&plane->layer,
> -					    desc->layout.csc,
> -					    0x4c900091);
> -		atmel_hlcdc_layer_write_cfg(&plane->layer,
> -					    desc->layout.csc + 1,
> -					    0x7a5f5090);
> -		atmel_hlcdc_layer_write_cfg(&plane->layer,
> -					    desc->layout.csc + 2,
> -					    0x40040890);
> -	}
> +	if (dc->desc->is_xlcdc && desc->layout.csc)
> +		xlcdc_csc_init(plane, desc);
> +	else
> +		if (desc->layout.csc)
> +			hlcdc_csc_init(plane, desc);

	if (desc->layout.csc)
		dc->desc->lcdc_csc_init();

>  
>  	return 0;
>  }
>  
> -void atmel_hlcdc_plane_irq(struct atmel_hlcdc_plane *plane)
> +static void hlcdc_irq_dbg(struct atmel_hlcdc_plane *plane,
> +			  const struct atmel_hlcdc_layer_desc *desc)
>  {
> -	const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
> -	u32 isr;
> -
> -	isr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_ISR);
> +	u32 isr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_ISR);
>  
>  	/*
>  	 * There's not much we can do in case of overrun except informing
> @@ -830,8 +1023,34 @@ void atmel_hlcdc_plane_irq(struct atmel_hlcdc_plane *plane)
>  	if (isr &
>  	    (ATMEL_HLCDC_LAYER_OVR_IRQ(0) | ATMEL_HLCDC_LAYER_OVR_IRQ(1) |
>  	     ATMEL_HLCDC_LAYER_OVR_IRQ(2)))
> -		dev_dbg(plane->base.dev->dev, "overrun on plane %s\n",
> -			desc->name);
> +		pr_warn("%s: overrun on plane %s\n", __func__, desc->name);

Why changing to pr_warn? why not dev_warn, if any?

> +}
> +
> +static void xlcdc_irq_dbg(struct atmel_hlcdc_plane *plane,
> +			  const struct atmel_hlcdc_layer_desc *desc)
> +{
> +	u32 isr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_XLCDC_LAYER_ISR);
> +
> +	/*
> +	 * There's not much we can do in case of overrun except informing
> +	 * the user. However, we are in interrupt context here, hence the
> +	 * use of dev_dbg().
> +	 */
> +	if (isr &
> +	    (ATMEL_XLCDC_LAYER_OVR_IRQ(0) | ATMEL_XLCDC_LAYER_OVR_IRQ(1) |
> +	     ATMEL_XLCDC_LAYER_OVR_IRQ(2)))
> +		pr_warn("%s: overrun on plane %s\n", __func__, desc->name);

dev_warn() ?

> +}
> +
> +void atmel_hlcdc_plane_irq(struct atmel_hlcdc_plane *plane)
> +{
> +	const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
> +	struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
> +
> +	if (dc->desc->is_xlcdc)
> +		xlcdc_irq_dbg(plane, desc);
> +	else
> +		hlcdc_irq_dbg(plane, desc);

	dc->desc->lcdc_irq_dbg() ?

>  }
>  
>  static const struct drm_plane_helper_funcs atmel_hlcdc_layer_plane_helper_funcs = {

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 5/8] drm: atmel_hlcdc: Add support for XLCDC in atmel LCD driver

[Manikandan.M]

On 09/09/23 9:50 pm, claudiu beznea wrote:
> [You don't often get email from claudiu.beznea@tuxon.dev. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi, Manikandan,
> 
> On 8/25/23 15:54, Manikandan Muralidharan wrote:
>> - XLCDC in SAM9X7 has different sets of registers and additional
>> configuration bits when compared to previous HLCDC IP. Read/write
>> operation on the controller registers is now separated using the
>> XLCDC status flag.
>>        - HEO scaling, window resampling, Alpha blending, YUV-to-RGB
>> conversion in XLCDC is derived and handled using additional
>> configuration bits and registers.
>>        - Writing one to the Enable fields of each layer in LCD_ATTRE
>> is required to reflect the values set in Configuration, FBA, Enable
>> registers of each layer
>>
>> Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
>> Co-developed-by: Hari Prasath Gujulan Elango <Hari.PrasathGE@microchip.com>
>> Signed-off-by: Hari Prasath Gujulan Elango <Hari.PrasathGE@microchip.com>
>> Co-developed-by: Durai Manickam KR <durai.manickamkr@microchip.com>
>> Signed-off-by: Durai Manickam KR <durai.manickamkr@microchip.com>
>> ---
>>   .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c    |  25 +-
>>   .../gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c   | 333 +++++++++++++++---
>>   2 files changed, 299 insertions(+), 59 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
>> index cc5cf4c2faf7..4b11a1de8af4 100644
>> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
>> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
>> @@ -79,6 +79,7 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c)
>>        unsigned int mask = ATMEL_HLCDC_CLKDIV_MASK | ATMEL_HLCDC_CLKPOL;
>>        unsigned int cfg = 0;
>>        int div, ret;
>> +     bool is_xlcdc = crtc->dc->desc->is_xlcdc;
>>
>>        /* get encoder from crtc */
>>        drm_for_each_encoder(en_iter, ddev) {
>> @@ -164,10 +165,10 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c)
>>        state = drm_crtc_state_to_atmel_hlcdc_crtc_state(c->state);
>>        cfg = state->output_mode << 8;
>>
>> -     if (adj->flags & DRM_MODE_FLAG_NVSYNC)
>> +     if (!is_xlcdc && (adj->flags & DRM_MODE_FLAG_NVSYNC))
>>                cfg |= ATMEL_HLCDC_VSPOL;
>>
>> -     if (adj->flags & DRM_MODE_FLAG_NHSYNC)
>> +     if (!is_xlcdc && (adj->flags & DRM_MODE_FLAG_NHSYNC))
>>                cfg |= ATMEL_HLCDC_HSPOL;
>>
>>        regmap_update_bits(regmap, ATMEL_HLCDC_CFG(5),
>> @@ -202,6 +203,16 @@ static void atmel_hlcdc_crtc_atomic_disable(struct drm_crtc *c,
>>
>>        pm_runtime_get_sync(dev->dev);
>>
>> +     if (crtc->dc->desc->is_xlcdc) {
>> +             regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_XLCDC_CM);
>> +             regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
>> +                                      !(status & ATMEL_XLCDC_CM), 10, 0);
> 
> You may want to check the return value of regmap_read_poll_timeout().
> Otherwise your setup may fail.
> 
> Also, regmap_read_poll_timeout() may sleep, the other settings in this
> functions are done with bussy looping, is there a reason for that?
>Hi Claudiu

Not sure if a non-zero timeout_us coud be sufficient for this operation, 
considering the next power-up and power-down sequence following the 
current step.
Any suggestion on the value of timeout_us is appreciable.
>> +
>> +             regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_XLCDC_SD);
>> +             regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
>> +                                      status & ATMEL_XLCDC_SD, 10, 0);
> 
> Same here.
> 
>> +     }
>> +
>>        regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_HLCDC_DISP);
>>        while (!regmap_read(regmap, ATMEL_HLCDC_SR, &status) &&
>>               (status & ATMEL_HLCDC_DISP))
>> @@ -256,6 +267,16 @@ static void atmel_hlcdc_crtc_atomic_enable(struct drm_crtc *c,
>>               !(status & ATMEL_HLCDC_DISP))
>>                cpu_relax();
>>
>> +     if (crtc->dc->desc->is_xlcdc) {
>> +             regmap_write(regmap, ATMEL_HLCDC_EN, ATMEL_XLCDC_CM);
>> +             regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
>> +                                      status & ATMEL_XLCDC_CM, 10, 0);
>> +
>> +             regmap_write(regmap, ATMEL_HLCDC_EN, ATMEL_XLCDC_SD);
>> +             regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
>> +                                      !(status & ATMEL_XLCDC_SD), 10, 0);
>> +     }
>> +
>>        pm_runtime_put_sync(dev->dev);
>>
>>   }
>> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
>> index daa508504f47..26caf4cddfa4 100644
>> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
>> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
>> @@ -330,11 +330,59 @@ static void atmel_hlcdc_plane_setup_scaler(struct atmel_hlcdc_plane *plane,
>>                                                                     yfactor));
>>   }
>>
>> +static void atmel_xlcdc_plane_setup_scaler(struct atmel_hlcdc_plane *plane,
>> +                                        struct atmel_hlcdc_plane_state *state)
>> +{
>> +     const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
>> +     u32 xfactor, yfactor;
>> +
>> +     if (!desc->layout.scaler_config)
>> +             return;
>> +
>> +     if (state->crtc_w == state->src_w && state->crtc_h == state->src_h) {
>> +             atmel_hlcdc_layer_write_cfg(&plane->layer,
>> +                                         desc->layout.scaler_config, 0);
>> +             return;
>> +     }
>> +
>> +     /* xfactor = round[(2^20 * XMEMSIZE)/XSIZE)] */
>> +     xfactor = (u32)(((1 << 20) * state->src_w) / state->crtc_w);
> 
> Could ((1 << 20) * state->src_w) overflow?
It is within the limits, state->src_w will contain a 11bit reg value at max.
> 
>> +
>> +     /* yfactor = round[(2^20 * YMEMSIZE)/YSIZE)] */
>> +     yfactor = (u32)(((1 << 20) * state->src_h) / state->crtc_h);
> 
> Same here.
> 
>> +
>> +     atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config,
>> +                                 ATMEL_XLCDC_LAYER_VSCALER_LUMA_ENABLE |
>> +                                 ATMEL_XLCDC_LAYER_VSCALER_CHROMA_ENABLE |
>> +                                 ATMEL_XLCDC_LAYER_HSCALER_LUMA_ENABLE |
>> +                                 ATMEL_XLCDC_LAYER_HSCALER_CHROMA_ENABLE);
>> +
>> +     atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 1,
>> +                                 yfactor);
>> +     atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 3,
>> +                                 xfactor);
>> +
>> +     /* As per YCbCr window resampling configuration */
>> +     if (state->base.fb->format->format == DRM_FORMAT_YUV420) {
>> +             atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 2,
>> +                                         yfactor / 2);
>> +             atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 4,
>> +                                         xfactor / 2);
>> +     } else {
>> +             /* As per ARGB window resampling configuration */
>> +             atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 2,
>> +                                         yfactor);
>> +             atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 4,
>> +                                         xfactor);
>> +     }
> 
> This can be written as follows:
> if (state->base.fb->format->format == DRM_FORMAT_YUV420) {
>          yfactor /= 2);
>          xfactor /= 2);
> }
> atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 2,
>                              yfactor);
> atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 4,
>                              xfactor);
> 
> 
> 
>> +}
>> +
>>   static void
>>   atmel_hlcdc_plane_update_pos_and_size(struct atmel_hlcdc_plane *plane,
>>                                      struct atmel_hlcdc_plane_state *state)
>>   {
>>        const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
>> +     struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
>>
>>        if (desc->layout.size)
>>                atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.size,
>> @@ -352,7 +400,10 @@ atmel_hlcdc_plane_update_pos_and_size(struct atmel_hlcdc_plane *plane,
>>                                        ATMEL_HLCDC_LAYER_POS(state->crtc_x,
>>                                                              state->crtc_y));
>>
>> -     atmel_hlcdc_plane_setup_scaler(plane, state);
>> +     if (dc->desc->is_xlcdc)
>> +             atmel_xlcdc_plane_setup_scaler(plane, state);
>> +     else
>> +             atmel_hlcdc_plane_setup_scaler(plane, state);
> 
> What if you embedd the plane_setup_scaller function in struct
> atmel_hlcdc_dc_desc and define per lcdc variant the proper function you can
> have here something like:
>          dc->desc->plane_setup_scaler();
> 
> This is valid for other places in this patch (see below).
Sure, I will try to incoperate this change.
> 
>>   }
>>
>>   static void
>> @@ -393,6 +444,40 @@ atmel_hlcdc_plane_update_general_settings(struct atmel_hlcdc_plane *plane,
>>                                    cfg);
>>   }
>>
>> +static void
>> +atmel_xlcdc_plane_update_general_settings(struct atmel_hlcdc_plane *plane,
>> +                                       struct atmel_hlcdc_plane_state *state)
>> +{
>> +     unsigned int cfg;
>> +     const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
>> +     const struct drm_format_info *format = state->base.fb->format;
>> +
>> +     atmel_hlcdc_layer_write_cfg(&plane->layer, ATMEL_XLCDC_LAYER_DMA_CFG,
>> +                                 ATMEL_HLCDC_LAYER_DMA_BLEN_INCR16 | state->ahb_id);
>> +
>> +     cfg = ATMEL_XLCDC_LAYER_DMA | ATMEL_XLCDC_LAYER_REP;
>> +
>> +     if (plane->base.type != DRM_PLANE_TYPE_PRIMARY) {
>> +             /*
>> +              * Alpha Blending bits specific to SAM9X7 SoC
>> +              */
>> +             cfg |= ATMEL_XLCDC_LAYER_SFACTC_A0_MULT_AS |
>> +                    ATMEL_XLCDC_LAYER_SFACTA_ONE |
>> +                    ATMEL_XLCDC_LAYER_DFACTC_M_A0_MULT_AS |
>> +                    ATMEL_XLCDC_LAYER_DFACTA_ONE;
>> +             if (format->has_alpha)
>> +                     cfg |= ATMEL_XLCDC_LAYER_A0(0xff);
>> +             else
>> +                     cfg |= ATMEL_XLCDC_LAYER_A0(state->base.alpha);
>> +     }
>> +
>> +     if (state->disc_h && state->disc_w)
>> +             cfg |= ATMEL_XLCDC_LAYER_DISCEN;
>> +
>> +     atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.general_config,
>> +                                 cfg);
>> +}
>> +
>>   static void atmel_hlcdc_plane_update_format(struct atmel_hlcdc_plane *plane,
>>                                        struct atmel_hlcdc_plane_state *state)
>>   {
>> @@ -437,36 +522,55 @@ static void atmel_hlcdc_plane_update_clut(struct atmel_hlcdc_plane *plane,
>>        }
>>   }
>>
>> +static void update_hlcdc_buffers(struct atmel_hlcdc_plane *plane,
>> +                              struct atmel_hlcdc_plane_state *state, u32 sr, int i)
>> +{
>> +     atmel_hlcdc_layer_write_reg(&plane->layer,
>> +                                 ATMEL_HLCDC_LAYER_PLANE_HEAD(i),
>> +                                 state->dscrs[i]->self);
>> +
>> +     if (!(sr & ATMEL_HLCDC_LAYER_EN)) {
> 
> To avoid extra indenting:
>          if (sr & ATMEL_HLCDC_LAYER_EN)
>                  return;
> 
>> +             atmel_hlcdc_layer_write_reg(&plane->layer,
>> +                                         ATMEL_HLCDC_LAYER_PLANE_ADDR(i),
>> +                                         state->dscrs[i]->addr);
>> +             atmel_hlcdc_layer_write_reg(&plane->layer,
>> +                                         ATMEL_HLCDC_LAYER_PLANE_CTRL(i),
>> +                                         state->dscrs[i]->ctrl);
>> +             atmel_hlcdc_layer_write_reg(&plane->layer,
>> +                                         ATMEL_HLCDC_LAYER_PLANE_NEXT(i),
>> +                                         state->dscrs[i]->self);
>> +     }
>> +}
>> +
>> +static void update_xlcdc_buffers(struct atmel_hlcdc_plane *plane,
>> +                              struct atmel_hlcdc_plane_state *state, int i)
>> +{
>> +     atmel_hlcdc_layer_write_reg(&plane->layer,
>> +                                 ATMEL_XLCDC_LAYER_PLANE_ADDR(i),
>> +                                 state->dscrs[i]->addr);
>> +}
>> +
>>   static void atmel_hlcdc_plane_update_buffers(struct atmel_hlcdc_plane *plane,
>> -                                     struct atmel_hlcdc_plane_state *state)
>> +                                          struct atmel_hlcdc_plane_state *state)
> 
> update_hlcdc_buffers() and update_xlcdc_buffers() could also be members in
> the struct atmel_hlcdc_dc_desc and called accordingly where needed w/o
> checking is_xlcdc.
> 
>>   {
>>        const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
>> +     struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
>>        struct drm_framebuffer *fb = state->base.fb;
>>        u32 sr;
>>        int i;
>>
>> -     sr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHSR);
>> +     if (!dc->desc->is_xlcdc)
>> +             sr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHSR);
>>
>>        for (i = 0; i < state->nplanes; i++) {
>>                struct drm_gem_dma_object *gem = drm_fb_dma_get_gem_obj(fb, i);
>>
>>                state->dscrs[i]->addr = gem->dma_addr + state->offsets[i];
>>
>> -             atmel_hlcdc_layer_write_reg(&plane->layer,
>> -                                         ATMEL_HLCDC_LAYER_PLANE_HEAD(i),
>> -                                         state->dscrs[i]->self);
>> -
>> -             if (!(sr & ATMEL_HLCDC_LAYER_EN)) {
>> -                     atmel_hlcdc_layer_write_reg(&plane->layer,
>> -                                     ATMEL_HLCDC_LAYER_PLANE_ADDR(i),
>> -                                     state->dscrs[i]->addr);
>> -                     atmel_hlcdc_layer_write_reg(&plane->layer,
>> -                                     ATMEL_HLCDC_LAYER_PLANE_CTRL(i),
>> -                                     state->dscrs[i]->ctrl);
>> -                     atmel_hlcdc_layer_write_reg(&plane->layer,
>> -                                     ATMEL_HLCDC_LAYER_PLANE_NEXT(i),
>> -                                     state->dscrs[i]->self);
>> -             }
>> +             if (dc->desc->is_xlcdc)
>> +                     update_xlcdc_buffers(plane, state, i);
>> +             else
>> +                     update_hlcdc_buffers(plane, state, sr, i);
> 
> And here you can have something like:
>                  dc->desc->update_lcdc_buffers();
>>
>>                if (desc->layout.xstride[i])
>>                        atmel_hlcdc_layer_write_cfg(&plane->layer,
>> @@ -712,11 +816,8 @@ static int atmel_hlcdc_plane_atomic_check(struct drm_plane *p,
>>        return 0;
>>   }
>>
>> -static void atmel_hlcdc_plane_atomic_disable(struct drm_plane *p,
>> -                                          struct drm_atomic_state *state)
>> +static void hlcdc_atomic_disable(struct atmel_hlcdc_plane *plane)
>>   {
>> -     struct atmel_hlcdc_plane *plane = drm_plane_to_atmel_hlcdc_plane(p);
>> -
>>        /* Disable interrupts */
>>        atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_IDR,
>>                                    0xffffffff);
>> @@ -731,6 +832,72 @@ static void atmel_hlcdc_plane_atomic_disable(struct drm_plane *p,
>>        atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_ISR);
>>   }
>>
>> +static void xlcdc_atomic_disable(struct atmel_hlcdc_plane *plane)
>> +{
>> +     /* Disable interrupts */
>> +     atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_XLCDC_LAYER_IDR,
>> +                                 0xffffffff);
>> +
>> +     /* Disable the layer */
>> +     atmel_hlcdc_layer_write_reg(&plane->layer,
>> +                                 ATMEL_XLCDC_LAYER_ENR, 0);
>> +
>> +     /* Clear all pending interrupts */
>> +     atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_XLCDC_LAYER_ISR);
>> +}
>> +
>> +static void atmel_hlcdc_plane_atomic_disable(struct drm_plane *p,
>> +                                          struct drm_atomic_state *state)
>> +{
>> +     struct atmel_hlcdc_plane *plane = drm_plane_to_atmel_hlcdc_plane(p);
>> +     struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
>> +
>> +     if (dc->desc->is_xlcdc)
>> +             xlcdc_atomic_disable(plane);
>> +     else
>> +             hlcdc_atomic_disable(plane);
> 
>          dc->desc->lcdc_atomic_disable();
> 
>> +}
>> +
>> +static void hlcdc_atomic_update(struct atmel_hlcdc_plane *plane)
>> +{
>> +     u32 sr;
>> +
>> +     /* Enable the overrun interrupts. */
>> +     atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_IER,
>> +                                 ATMEL_HLCDC_LAYER_OVR_IRQ(0) |
>> +                                 ATMEL_HLCDC_LAYER_OVR_IRQ(1) |
>> +                                 ATMEL_HLCDC_LAYER_OVR_IRQ(2));
>> +
>> +     /* Apply the new config at the next SOF event. */
>> +     sr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHSR);
>> +     atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHER,
>> +                                 ATMEL_HLCDC_LAYER_UPDATE |
>> +                                 (sr & ATMEL_HLCDC_LAYER_EN ?
>> +                                 ATMEL_HLCDC_LAYER_A2Q : ATMEL_HLCDC_LAYER_EN));
>> +}
>> +
>> +static void xlcdc_atomic_update(struct atmel_hlcdc_plane *plane,
>> +                             struct atmel_hlcdc_dc *dc)
>> +{
>> +     /* Enable the overrun interrupts. */
>> +     atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_XLCDC_LAYER_IER,
>> +                                 ATMEL_XLCDC_LAYER_OVR_IRQ(0) |
>> +                                 ATMEL_XLCDC_LAYER_OVR_IRQ(1) |
>> +                                 ATMEL_XLCDC_LAYER_OVR_IRQ(2));
>> +
>> +     atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_XLCDC_LAYER_ENR,
>> +                                 ATMEL_XLCDC_LAYER_EN);
>> +
>> +     /*
>> +      * Updating XLCDC_xxxCFGx, XLCDC_xxxFBA and XLCDC_xxxEN,
>> +      * (where xxx indicates each layer) requires writing one to the
>> +      * Update Attribute field for each layer in LCDC_ATTRE register for SAM9X7.
>> +      */
>> +     regmap_write(dc->hlcdc->regmap, ATMEL_XLCDC_ATTRE, ATMEL_XLCDC_BASE_UPDATE |
>> +                  ATMEL_XLCDC_OVR1_UPDATE | ATMEL_XLCDC_OVR3_UPDATE |
>> +                  ATMEL_XLCDC_HEO_UPDATE);
>> +}
>> +
>>   static void atmel_hlcdc_plane_atomic_update(struct drm_plane *p,
>>                                            struct drm_atomic_state *state)
>>   {
>> @@ -739,7 +906,7 @@ static void atmel_hlcdc_plane_atomic_update(struct drm_plane *p,
>>        struct atmel_hlcdc_plane *plane = drm_plane_to_atmel_hlcdc_plane(p);
>>        struct atmel_hlcdc_plane_state *hstate =
>>                        drm_plane_state_to_atmel_hlcdc_plane_state(new_s);
>> -     u32 sr;
>> +     struct atmel_hlcdc_dc *dc = p->dev->dev_private;
>>
>>        if (!new_s->crtc || !new_s->fb)
>>                return;
>> @@ -750,29 +917,67 @@ static void atmel_hlcdc_plane_atomic_update(struct drm_plane *p,
>>        }
>>
>>        atmel_hlcdc_plane_update_pos_and_size(plane, hstate);
>> -     atmel_hlcdc_plane_update_general_settings(plane, hstate);
>> +     if (dc->desc->is_xlcdc)
>> +             atmel_xlcdc_plane_update_general_settings(plane, hstate);
>> +     else
>> +             atmel_hlcdc_plane_update_general_settings(plane, hstate);
> 
>          dc->desc->lcdc_plane_update_general_settigns();
> 
>>        atmel_hlcdc_plane_update_format(plane, hstate);
>>        atmel_hlcdc_plane_update_clut(plane, hstate);
>>        atmel_hlcdc_plane_update_buffers(plane, hstate);
>>        atmel_hlcdc_plane_update_disc_area(plane, hstate);
>>
>> -     /* Enable the overrun interrupts. */
>> -     atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_IER,
>> -                                 ATMEL_HLCDC_LAYER_OVR_IRQ(0) |
>> -                                 ATMEL_HLCDC_LAYER_OVR_IRQ(1) |
>> -                                 ATMEL_HLCDC_LAYER_OVR_IRQ(2));
>> +     if (dc->desc->is_xlcdc)
>> +             xlcdc_atomic_update(plane, dc);
>> +     else
>> +             hlcdc_atomic_update(plane);
> 
>          dc->desc->lcdc_atomic_update();
> 
>> +}
>>
>> -     /* Apply the new config at the next SOF event. */
>> -     sr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHSR);
>> -     atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHER,
>> -                     ATMEL_HLCDC_LAYER_UPDATE |
>> -                     (sr & ATMEL_HLCDC_LAYER_EN ?
>> -                      ATMEL_HLCDC_LAYER_A2Q : ATMEL_HLCDC_LAYER_EN));
>> +u32 hlcdc_csc_coeffs[] = {
>> +     0x4c900091,
>> +     0x7a5f5090,
>> +     0x40040890
>> +};
> 
> You can move this in function where it is used and declare it static.
> 
>> +
>> +u32 xlcdc_csc_coeffs[] = {
>> +     0x00000488,
>> +     0x00000648,
>> +     0x1EA00480,
>> +     0x00001D28,
>> +     0x08100480,
>> +     0x00000000,
>> +     0x00000007
>> +};
> 
> Same here.
> 
>> +
>> +static void hlcdc_csc_init(struct atmel_hlcdc_plane *plane,
>> +                        const struct atmel_hlcdc_layer_desc *desc)
>> +{
>> +     /*
>> +      * TODO: declare a "yuv-to-rgb-conv-factors" property to let
>> +      * userspace modify these factors (using a BLOB property ?).
>> +      */
>> +     for (int i = 0; i < ARRAY_SIZE(hlcdc_csc_coeffs); i++)
> 
> { at the end of for statement.
> 
>> +             atmel_hlcdc_layer_write_cfg(&plane->layer,
>> +                                         desc->layout.csc + i,
>> +                                         hlcdc_csc_coeffs[i]);
> 
> } here
> 
>> +}
>> +
>> +static void xlcdc_csc_init(struct atmel_hlcdc_plane *plane,
>> +                        const struct atmel_hlcdc_layer_desc *desc)
>> +{
>> +     /*
>> +      * yuv-to-rgb-conv-factors are now defined from LCDC_HEOCFG16 to
>> +      * LCDC_HEOCFG21 registers in SAM9X7.
>> +      */
>> +     for (int i = 0; i < ARRAY_SIZE(xlcdc_csc_coeffs); i++)
>> +             atmel_hlcdc_layer_write_cfg(&plane->layer,
>> +                                         desc->layout.csc + i,
>> +                                         xlcdc_csc_coeffs[i]);
> 
> Ditto
> 
>>   }
>>
>>   static int atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane)
>>   {
>>        const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
>> +     struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
>>
>>        if (desc->type == ATMEL_HLCDC_OVERLAY_LAYER ||
>>            desc->type == ATMEL_HLCDC_CURSOR_LAYER) {
>> @@ -796,31 +1001,19 @@ static int atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane)
>>                        return ret;
>>        }
>>
>> -     if (desc->layout.csc) {
>> -             /*
>> -              * TODO: decare a "yuv-to-rgb-conv-factors" property to let
>> -              * userspace modify these factors (using a BLOB property ?).
>> -              */
>> -             atmel_hlcdc_layer_write_cfg(&plane->layer,
>> -                                         desc->layout.csc,
>> -                                         0x4c900091);
>> -             atmel_hlcdc_layer_write_cfg(&plane->layer,
>> -                                         desc->layout.csc + 1,
>> -                                         0x7a5f5090);
>> -             atmel_hlcdc_layer_write_cfg(&plane->layer,
>> -                                         desc->layout.csc + 2,
>> -                                         0x40040890);
>> -     }
>> +     if (dc->desc->is_xlcdc && desc->layout.csc)
>> +             xlcdc_csc_init(plane, desc);
>> +     else
>> +             if (desc->layout.csc)
>> +                     hlcdc_csc_init(plane, desc);
> 
>          if (desc->layout.csc)
>                  dc->desc->lcdc_csc_init();
> 
>>
>>        return 0;
>>   }
>>
>> -void atmel_hlcdc_plane_irq(struct atmel_hlcdc_plane *plane)
>> +static void hlcdc_irq_dbg(struct atmel_hlcdc_plane *plane,
>> +                       const struct atmel_hlcdc_layer_desc *desc)
>>   {
>> -     const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
>> -     u32 isr;
>> -
>> -     isr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_ISR);
>> +     u32 isr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_ISR);
>>
>>        /*
>>         * There's not much we can do in case of overrun except informing
>> @@ -830,8 +1023,34 @@ void atmel_hlcdc_plane_irq(struct atmel_hlcdc_plane *plane)
>>        if (isr &
>>            (ATMEL_HLCDC_LAYER_OVR_IRQ(0) | ATMEL_HLCDC_LAYER_OVR_IRQ(1) |
>>             ATMEL_HLCDC_LAYER_OVR_IRQ(2)))
>> -             dev_dbg(plane->base.dev->dev, "overrun on plane %s\n",
>> -                     desc->name);
>> +             pr_warn("%s: overrun on plane %s\n", __func__, desc->name);
> 
> Why changing to pr_warn? why not dev_warn, if any?
> 
>> +}
>> +
>> +static void xlcdc_irq_dbg(struct atmel_hlcdc_plane *plane,
>> +                       const struct atmel_hlcdc_layer_desc *desc)
>> +{
>> +     u32 isr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_XLCDC_LAYER_ISR);
>> +
>> +     /*
>> +      * There's not much we can do in case of overrun except informing
>> +      * the user. However, we are in interrupt context here, hence the
>> +      * use of dev_dbg().
>> +      */
>> +     if (isr &
>> +         (ATMEL_XLCDC_LAYER_OVR_IRQ(0) | ATMEL_XLCDC_LAYER_OVR_IRQ(1) |
>> +          ATMEL_XLCDC_LAYER_OVR_IRQ(2)))
>> +             pr_warn("%s: overrun on plane %s\n", __func__, desc->name);
> 
> dev_warn() ?
> 
>> +}
>> +
>> +void atmel_hlcdc_plane_irq(struct atmel_hlcdc_plane *plane)
>> +{
>> +     const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
>> +     struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
>> +
>> +     if (dc->desc->is_xlcdc)
>> +             xlcdc_irq_dbg(plane, desc);
>> +     else
>> +             hlcdc_irq_dbg(plane, desc);
> 
>          dc->desc->lcdc_irq_dbg() ?
> 
>>   }
>>
>>   static const struct drm_plane_helper_funcs atmel_hlcdc_layer_plane_helper_funcs = {

-- 
Thanks and Regards,
Manikandan M.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 5/8] drm: atmel_hlcdc: Add support for XLCDC in atmel LCD driver

[claudiu beznea]

On 12.09.2023 13:44, Manikandan.M@microchip.com wrote:
> On 09/09/23 9:50 pm, claudiu beznea wrote:
>> [You don't often get email from claudiu.beznea@tuxon.dev. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>>
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> Hi, Manikandan,
>>
>> On 8/25/23 15:54, Manikandan Muralidharan wrote:
>>> - XLCDC in SAM9X7 has different sets of registers and additional
>>> configuration bits when compared to previous HLCDC IP. Read/write
>>> operation on the controller registers is now separated using the
>>> XLCDC status flag.
>>>        - HEO scaling, window resampling, Alpha blending, YUV-to-RGB
>>> conversion in XLCDC is derived and handled using additional
>>> configuration bits and registers.
>>>        - Writing one to the Enable fields of each layer in LCD_ATTRE
>>> is required to reflect the values set in Configuration, FBA, Enable
>>> registers of each layer
>>>
>>> Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
>>> Co-developed-by: Hari Prasath Gujulan Elango <Hari.PrasathGE@microchip.com>
>>> Signed-off-by: Hari Prasath Gujulan Elango <Hari.PrasathGE@microchip.com>
>>> Co-developed-by: Durai Manickam KR <durai.manickamkr@microchip.com>
>>> Signed-off-by: Durai Manickam KR <durai.manickamkr@microchip.com>
>>> ---
>>>   .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c    |  25 +-
>>>   .../gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c   | 333 +++++++++++++++---
>>>   2 files changed, 299 insertions(+), 59 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
>>> index cc5cf4c2faf7..4b11a1de8af4 100644
>>> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
>>> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
>>> @@ -79,6 +79,7 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c)
>>>        unsigned int mask = ATMEL_HLCDC_CLKDIV_MASK | ATMEL_HLCDC_CLKPOL;
>>>        unsigned int cfg = 0;
>>>        int div, ret;
>>> +     bool is_xlcdc = crtc->dc->desc->is_xlcdc;
>>>
>>>        /* get encoder from crtc */
>>>        drm_for_each_encoder(en_iter, ddev) {
>>> @@ -164,10 +165,10 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c)
>>>        state = drm_crtc_state_to_atmel_hlcdc_crtc_state(c->state);
>>>        cfg = state->output_mode << 8;
>>>
>>> -     if (adj->flags & DRM_MODE_FLAG_NVSYNC)
>>> +     if (!is_xlcdc && (adj->flags & DRM_MODE_FLAG_NVSYNC))
>>>                cfg |= ATMEL_HLCDC_VSPOL;
>>>
>>> -     if (adj->flags & DRM_MODE_FLAG_NHSYNC)
>>> +     if (!is_xlcdc && (adj->flags & DRM_MODE_FLAG_NHSYNC))
>>>                cfg |= ATMEL_HLCDC_HSPOL;
>>>
>>>        regmap_update_bits(regmap, ATMEL_HLCDC_CFG(5),
>>> @@ -202,6 +203,16 @@ static void atmel_hlcdc_crtc_atomic_disable(struct drm_crtc *c,
>>>
>>>        pm_runtime_get_sync(dev->dev);
>>>
>>> +     if (crtc->dc->desc->is_xlcdc) {
>>> +             regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_XLCDC_CM);
>>> +             regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
>>> +                                      !(status & ATMEL_XLCDC_CM), 10, 0);
>>
>> You may want to check the return value of regmap_read_poll_timeout().
>> Otherwise your setup may fail.
>>
>> Also, regmap_read_poll_timeout() may sleep, the other settings in this
>> functions are done with bussy looping, is there a reason for that?
>> Hi Claudiu
> 
> Not sure if a non-zero timeout_us coud be sufficient for this operation, 
> considering the next power-up and power-down sequence following the 
> current step.
> Any suggestion on the value of timeout_us is appreciable.

Ok, haven't noticed that the timeout is zero.

>>> +
>>> +             regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_XLCDC_SD);
>>> +             regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
>>> +                                      status & ATMEL_XLCDC_SD, 10, 0);
>>
>> Same here.
>>
>>> +     }
>>> +
>>>        regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_HLCDC_DISP);
>>>        while (!regmap_read(regmap, ATMEL_HLCDC_SR, &status) &&
>>>               (status & ATMEL_HLCDC_DISP))
>>> @@ -256,6 +267,16 @@ static void atmel_hlcdc_crtc_atomic_enable(struct drm_crtc *c,
>>>               !(status & ATMEL_HLCDC_DISP))
>>>                cpu_relax();
>>>
>>> +     if (crtc->dc->desc->is_xlcdc) {
>>> +             regmap_write(regmap, ATMEL_HLCDC_EN, ATMEL_XLCDC_CM);
>>> +             regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
>>> +                                      status & ATMEL_XLCDC_CM, 10, 0);
>>> +
>>> +             regmap_write(regmap, ATMEL_HLCDC_EN, ATMEL_XLCDC_SD);
>>> +             regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
>>> +                                      !(status & ATMEL_XLCDC_SD), 10, 0);
>>> +     }
>>> +
>>>        pm_runtime_put_sync(dev->dev);
>>>
>>>   }
>>> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
>>> index daa508504f47..26caf4cddfa4 100644
>>> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
>>> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
>>> @@ -330,11 +330,59 @@ static void atmel_hlcdc_plane_setup_scaler(struct atmel_hlcdc_plane *plane,
>>>                                                                     yfactor));
>>>   }
>>>
>>> +static void atmel_xlcdc_plane_setup_scaler(struct atmel_hlcdc_plane *plane,
>>> +                                        struct atmel_hlcdc_plane_state *state)
>>> +{
>>> +     const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
>>> +     u32 xfactor, yfactor;
>>> +
>>> +     if (!desc->layout.scaler_config)
>>> +             return;
>>> +
>>> +     if (state->crtc_w == state->src_w && state->crtc_h == state->src_h) {
>>> +             atmel_hlcdc_layer_write_cfg(&plane->layer,
>>> +                                         desc->layout.scaler_config, 0);
>>> +             return;
>>> +     }
>>> +
>>> +     /* xfactor = round[(2^20 * XMEMSIZE)/XSIZE)] */
>>> +     xfactor = (u32)(((1 << 20) * state->src_w) / state->crtc_w);
>>
>> Could ((1 << 20) * state->src_w) overflow?
> It is within the limits, state->src_w will contain a 11bit reg value at max.
>>
>>> +
>>> +     /* yfactor = round[(2^20 * YMEMSIZE)/YSIZE)] */
>>> +     yfactor = (u32)(((1 << 20) * state->src_h) / state->crtc_h);
>>
>> Same here.
>>
>>> +
>>> +     atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config,
>>> +                                 ATMEL_XLCDC_LAYER_VSCALER_LUMA_ENABLE |
>>> +                                 ATMEL_XLCDC_LAYER_VSCALER_CHROMA_ENABLE |
>>> +                                 ATMEL_XLCDC_LAYER_HSCALER_LUMA_ENABLE |
>>> +                                 ATMEL_XLCDC_LAYER_HSCALER_CHROMA_ENABLE);
>>> +
>>> +     atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 1,
>>> +                                 yfactor);
>>> +     atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 3,
>>> +                                 xfactor);
>>> +
>>> +     /* As per YCbCr window resampling configuration */
>>> +     if (state->base.fb->format->format == DRM_FORMAT_YUV420) {
>>> +             atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 2,
>>> +                                         yfactor / 2);
>>> +             atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 4,
>>> +                                         xfactor / 2);
>>> +     } else {
>>> +             /* As per ARGB window resampling configuration */
>>> +             atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 2,
>>> +                                         yfactor);
>>> +             atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 4,
>>> +                                         xfactor);
>>> +     }
>>
>> This can be written as follows:
>> if (state->base.fb->format->format == DRM_FORMAT_YUV420) {
>>          yfactor /= 2);
>>          xfactor /= 2);
>> }
>> atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 2,
>>                              yfactor);
>> atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 4,
>>                              xfactor);
>>
>>
>>
>>> +}
>>> +
>>>   static void
>>>   atmel_hlcdc_plane_update_pos_and_size(struct atmel_hlcdc_plane *plane,
>>>                                      struct atmel_hlcdc_plane_state *state)
>>>   {
>>>        const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
>>> +     struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
>>>
>>>        if (desc->layout.size)
>>>                atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.size,
>>> @@ -352,7 +400,10 @@ atmel_hlcdc_plane_update_pos_and_size(struct atmel_hlcdc_plane *plane,
>>>                                        ATMEL_HLCDC_LAYER_POS(state->crtc_x,
>>>                                                              state->crtc_y));
>>>
>>> -     atmel_hlcdc_plane_setup_scaler(plane, state);
>>> +     if (dc->desc->is_xlcdc)
>>> +             atmel_xlcdc_plane_setup_scaler(plane, state);
>>> +     else
>>> +             atmel_hlcdc_plane_setup_scaler(plane, state);
>>
>> What if you embedd the plane_setup_scaller function in struct
>> atmel_hlcdc_dc_desc and define per lcdc variant the proper function you can
>> have here something like:
>>          dc->desc->plane_setup_scaler();
>>
>> This is valid for other places in this patch (see below).
> Sure, I will try to incoperate this change.
>>
>>>   }
>>>
>>>   static void
>>> @@ -393,6 +444,40 @@ atmel_hlcdc_plane_update_general_settings(struct atmel_hlcdc_plane *plane,
>>>                                    cfg);
>>>   }
>>>
>>> +static void
>>> +atmel_xlcdc_plane_update_general_settings(struct atmel_hlcdc_plane *plane,
>>> +                                       struct atmel_hlcdc_plane_state *state)
>>> +{
>>> +     unsigned int cfg;
>>> +     const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
>>> +     const struct drm_format_info *format = state->base.fb->format;
>>> +
>>> +     atmel_hlcdc_layer_write_cfg(&plane->layer, ATMEL_XLCDC_LAYER_DMA_CFG,
>>> +                                 ATMEL_HLCDC_LAYER_DMA_BLEN_INCR16 | state->ahb_id);
>>> +
>>> +     cfg = ATMEL_XLCDC_LAYER_DMA | ATMEL_XLCDC_LAYER_REP;
>>> +
>>> +     if (plane->base.type != DRM_PLANE_TYPE_PRIMARY) {
>>> +             /*
>>> +              * Alpha Blending bits specific to SAM9X7 SoC
>>> +              */
>>> +             cfg |= ATMEL_XLCDC_LAYER_SFACTC_A0_MULT_AS |
>>> +                    ATMEL_XLCDC_LAYER_SFACTA_ONE |
>>> +                    ATMEL_XLCDC_LAYER_DFACTC_M_A0_MULT_AS |
>>> +                    ATMEL_XLCDC_LAYER_DFACTA_ONE;
>>> +             if (format->has_alpha)
>>> +                     cfg |= ATMEL_XLCDC_LAYER_A0(0xff);
>>> +             else
>>> +                     cfg |= ATMEL_XLCDC_LAYER_A0(state->base.alpha);
>>> +     }
>>> +
>>> +     if (state->disc_h && state->disc_w)
>>> +             cfg |= ATMEL_XLCDC_LAYER_DISCEN;
>>> +
>>> +     atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.general_config,
>>> +                                 cfg);
>>> +}
>>> +
>>>   static void atmel_hlcdc_plane_update_format(struct atmel_hlcdc_plane *plane,
>>>                                        struct atmel_hlcdc_plane_state *state)
>>>   {
>>> @@ -437,36 +522,55 @@ static void atmel_hlcdc_plane_update_clut(struct atmel_hlcdc_plane *plane,
>>>        }
>>>   }
>>>
>>> +static void update_hlcdc_buffers(struct atmel_hlcdc_plane *plane,
>>> +                              struct atmel_hlcdc_plane_state *state, u32 sr, int i)
>>> +{
>>> +     atmel_hlcdc_layer_write_reg(&plane->layer,
>>> +                                 ATMEL_HLCDC_LAYER_PLANE_HEAD(i),
>>> +                                 state->dscrs[i]->self);
>>> +
>>> +     if (!(sr & ATMEL_HLCDC_LAYER_EN)) {
>>
>> To avoid extra indenting:
>>          if (sr & ATMEL_HLCDC_LAYER_EN)
>>                  return;
>>
>>> +             atmel_hlcdc_layer_write_reg(&plane->layer,
>>> +                                         ATMEL_HLCDC_LAYER_PLANE_ADDR(i),
>>> +                                         state->dscrs[i]->addr);
>>> +             atmel_hlcdc_layer_write_reg(&plane->layer,
>>> +                                         ATMEL_HLCDC_LAYER_PLANE_CTRL(i),
>>> +                                         state->dscrs[i]->ctrl);
>>> +             atmel_hlcdc_layer_write_reg(&plane->layer,
>>> +                                         ATMEL_HLCDC_LAYER_PLANE_NEXT(i),
>>> +                                         state->dscrs[i]->self);
>>> +     }
>>> +}
>>> +
>>> +static void update_xlcdc_buffers(struct atmel_hlcdc_plane *plane,
>>> +                              struct atmel_hlcdc_plane_state *state, int i)
>>> +{
>>> +     atmel_hlcdc_layer_write_reg(&plane->layer,
>>> +                                 ATMEL_XLCDC_LAYER_PLANE_ADDR(i),
>>> +                                 state->dscrs[i]->addr);
>>> +}
>>> +
>>>   static void atmel_hlcdc_plane_update_buffers(struct atmel_hlcdc_plane *plane,
>>> -                                     struct atmel_hlcdc_plane_state *state)
>>> +                                          struct atmel_hlcdc_plane_state *state)
>>
>> update_hlcdc_buffers() and update_xlcdc_buffers() could also be members in
>> the struct atmel_hlcdc_dc_desc and called accordingly where needed w/o
>> checking is_xlcdc.
>>
>>>   {
>>>        const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
>>> +     struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
>>>        struct drm_framebuffer *fb = state->base.fb;
>>>        u32 sr;
>>>        int i;
>>>
>>> -     sr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHSR);
>>> +     if (!dc->desc->is_xlcdc)
>>> +             sr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHSR);
>>>
>>>        for (i = 0; i < state->nplanes; i++) {
>>>                struct drm_gem_dma_object *gem = drm_fb_dma_get_gem_obj(fb, i);
>>>
>>>                state->dscrs[i]->addr = gem->dma_addr + state->offsets[i];
>>>
>>> -             atmel_hlcdc_layer_write_reg(&plane->layer,
>>> -                                         ATMEL_HLCDC_LAYER_PLANE_HEAD(i),
>>> -                                         state->dscrs[i]->self);
>>> -
>>> -             if (!(sr & ATMEL_HLCDC_LAYER_EN)) {
>>> -                     atmel_hlcdc_layer_write_reg(&plane->layer,
>>> -                                     ATMEL_HLCDC_LAYER_PLANE_ADDR(i),
>>> -                                     state->dscrs[i]->addr);
>>> -                     atmel_hlcdc_layer_write_reg(&plane->layer,
>>> -                                     ATMEL_HLCDC_LAYER_PLANE_CTRL(i),
>>> -                                     state->dscrs[i]->ctrl);
>>> -                     atmel_hlcdc_layer_write_reg(&plane->layer,
>>> -                                     ATMEL_HLCDC_LAYER_PLANE_NEXT(i),
>>> -                                     state->dscrs[i]->self);
>>> -             }
>>> +             if (dc->desc->is_xlcdc)
>>> +                     update_xlcdc_buffers(plane, state, i);
>>> +             else
>>> +                     update_hlcdc_buffers(plane, state, sr, i);
>>
>> And here you can have something like:
>>                  dc->desc->update_lcdc_buffers();
>>>
>>>                if (desc->layout.xstride[i])
>>>                        atmel_hlcdc_layer_write_cfg(&plane->layer,
>>> @@ -712,11 +816,8 @@ static int atmel_hlcdc_plane_atomic_check(struct drm_plane *p,
>>>        return 0;
>>>   }
>>>
>>> -static void atmel_hlcdc_plane_atomic_disable(struct drm_plane *p,
>>> -                                          struct drm_atomic_state *state)
>>> +static void hlcdc_atomic_disable(struct atmel_hlcdc_plane *plane)
>>>   {
>>> -     struct atmel_hlcdc_plane *plane = drm_plane_to_atmel_hlcdc_plane(p);
>>> -
>>>        /* Disable interrupts */
>>>        atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_IDR,
>>>                                    0xffffffff);
>>> @@ -731,6 +832,72 @@ static void atmel_hlcdc_plane_atomic_disable(struct drm_plane *p,
>>>        atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_ISR);
>>>   }
>>>
>>> +static void xlcdc_atomic_disable(struct atmel_hlcdc_plane *plane)
>>> +{
>>> +     /* Disable interrupts */
>>> +     atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_XLCDC_LAYER_IDR,
>>> +                                 0xffffffff);
>>> +
>>> +     /* Disable the layer */
>>> +     atmel_hlcdc_layer_write_reg(&plane->layer,
>>> +                                 ATMEL_XLCDC_LAYER_ENR, 0);
>>> +
>>> +     /* Clear all pending interrupts */
>>> +     atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_XLCDC_LAYER_ISR);
>>> +}
>>> +
>>> +static void atmel_hlcdc_plane_atomic_disable(struct drm_plane *p,
>>> +                                          struct drm_atomic_state *state)
>>> +{
>>> +     struct atmel_hlcdc_plane *plane = drm_plane_to_atmel_hlcdc_plane(p);
>>> +     struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
>>> +
>>> +     if (dc->desc->is_xlcdc)
>>> +             xlcdc_atomic_disable(plane);
>>> +     else
>>> +             hlcdc_atomic_disable(plane);
>>
>>          dc->desc->lcdc_atomic_disable();
>>
>>> +}
>>> +
>>> +static void hlcdc_atomic_update(struct atmel_hlcdc_plane *plane)
>>> +{
>>> +     u32 sr;
>>> +
>>> +     /* Enable the overrun interrupts. */
>>> +     atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_IER,
>>> +                                 ATMEL_HLCDC_LAYER_OVR_IRQ(0) |
>>> +                                 ATMEL_HLCDC_LAYER_OVR_IRQ(1) |
>>> +                                 ATMEL_HLCDC_LAYER_OVR_IRQ(2));
>>> +
>>> +     /* Apply the new config at the next SOF event. */
>>> +     sr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHSR);
>>> +     atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHER,
>>> +                                 ATMEL_HLCDC_LAYER_UPDATE |
>>> +                                 (sr & ATMEL_HLCDC_LAYER_EN ?
>>> +                                 ATMEL_HLCDC_LAYER_A2Q : ATMEL_HLCDC_LAYER_EN));
>>> +}
>>> +
>>> +static void xlcdc_atomic_update(struct atmel_hlcdc_plane *plane,
>>> +                             struct atmel_hlcdc_dc *dc)
>>> +{
>>> +     /* Enable the overrun interrupts. */
>>> +     atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_XLCDC_LAYER_IER,
>>> +                                 ATMEL_XLCDC_LAYER_OVR_IRQ(0) |
>>> +                                 ATMEL_XLCDC_LAYER_OVR_IRQ(1) |
>>> +                                 ATMEL_XLCDC_LAYER_OVR_IRQ(2));
>>> +
>>> +     atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_XLCDC_LAYER_ENR,
>>> +                                 ATMEL_XLCDC_LAYER_EN);
>>> +
>>> +     /*
>>> +      * Updating XLCDC_xxxCFGx, XLCDC_xxxFBA and XLCDC_xxxEN,
>>> +      * (where xxx indicates each layer) requires writing one to the
>>> +      * Update Attribute field for each layer in LCDC_ATTRE register for SAM9X7.
>>> +      */
>>> +     regmap_write(dc->hlcdc->regmap, ATMEL_XLCDC_ATTRE, ATMEL_XLCDC_BASE_UPDATE |
>>> +                  ATMEL_XLCDC_OVR1_UPDATE | ATMEL_XLCDC_OVR3_UPDATE |
>>> +                  ATMEL_XLCDC_HEO_UPDATE);
>>> +}
>>> +
>>>   static void atmel_hlcdc_plane_atomic_update(struct drm_plane *p,
>>>                                            struct drm_atomic_state *state)
>>>   {
>>> @@ -739,7 +906,7 @@ static void atmel_hlcdc_plane_atomic_update(struct drm_plane *p,
>>>        struct atmel_hlcdc_plane *plane = drm_plane_to_atmel_hlcdc_plane(p);
>>>        struct atmel_hlcdc_plane_state *hstate =
>>>                        drm_plane_state_to_atmel_hlcdc_plane_state(new_s);
>>> -     u32 sr;
>>> +     struct atmel_hlcdc_dc *dc = p->dev->dev_private;
>>>
>>>        if (!new_s->crtc || !new_s->fb)
>>>                return;
>>> @@ -750,29 +917,67 @@ static void atmel_hlcdc_plane_atomic_update(struct drm_plane *p,
>>>        }
>>>
>>>        atmel_hlcdc_plane_update_pos_and_size(plane, hstate);
>>> -     atmel_hlcdc_plane_update_general_settings(plane, hstate);
>>> +     if (dc->desc->is_xlcdc)
>>> +             atmel_xlcdc_plane_update_general_settings(plane, hstate);
>>> +     else
>>> +             atmel_hlcdc_plane_update_general_settings(plane, hstate);
>>
>>          dc->desc->lcdc_plane_update_general_settigns();
>>
>>>        atmel_hlcdc_plane_update_format(plane, hstate);
>>>        atmel_hlcdc_plane_update_clut(plane, hstate);
>>>        atmel_hlcdc_plane_update_buffers(plane, hstate);
>>>        atmel_hlcdc_plane_update_disc_area(plane, hstate);
>>>
>>> -     /* Enable the overrun interrupts. */
>>> -     atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_IER,
>>> -                                 ATMEL_HLCDC_LAYER_OVR_IRQ(0) |
>>> -                                 ATMEL_HLCDC_LAYER_OVR_IRQ(1) |
>>> -                                 ATMEL_HLCDC_LAYER_OVR_IRQ(2));
>>> +     if (dc->desc->is_xlcdc)
>>> +             xlcdc_atomic_update(plane, dc);
>>> +     else
>>> +             hlcdc_atomic_update(plane);
>>
>>          dc->desc->lcdc_atomic_update();
>>
>>> +}
>>>
>>> -     /* Apply the new config at the next SOF event. */
>>> -     sr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHSR);
>>> -     atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHER,
>>> -                     ATMEL_HLCDC_LAYER_UPDATE |
>>> -                     (sr & ATMEL_HLCDC_LAYER_EN ?
>>> -                      ATMEL_HLCDC_LAYER_A2Q : ATMEL_HLCDC_LAYER_EN));
>>> +u32 hlcdc_csc_coeffs[] = {
>>> +     0x4c900091,
>>> +     0x7a5f5090,
>>> +     0x40040890
>>> +};
>>
>> You can move this in function where it is used and declare it static.
>>
>>> +
>>> +u32 xlcdc_csc_coeffs[] = {
>>> +     0x00000488,
>>> +     0x00000648,
>>> +     0x1EA00480,
>>> +     0x00001D28,
>>> +     0x08100480,
>>> +     0x00000000,
>>> +     0x00000007
>>> +};
>>
>> Same here.
>>
>>> +
>>> +static void hlcdc_csc_init(struct atmel_hlcdc_plane *plane,
>>> +                        const struct atmel_hlcdc_layer_desc *desc)
>>> +{
>>> +     /*
>>> +      * TODO: declare a "yuv-to-rgb-conv-factors" property to let
>>> +      * userspace modify these factors (using a BLOB property ?).
>>> +      */
>>> +     for (int i = 0; i < ARRAY_SIZE(hlcdc_csc_coeffs); i++)
>>
>> { at the end of for statement.
>>
>>> +             atmel_hlcdc_layer_write_cfg(&plane->layer,
>>> +                                         desc->layout.csc + i,
>>> +                                         hlcdc_csc_coeffs[i]);
>>
>> } here
>>
>>> +}
>>> +
>>> +static void xlcdc_csc_init(struct atmel_hlcdc_plane *plane,
>>> +                        const struct atmel_hlcdc_layer_desc *desc)
>>> +{
>>> +     /*
>>> +      * yuv-to-rgb-conv-factors are now defined from LCDC_HEOCFG16 to
>>> +      * LCDC_HEOCFG21 registers in SAM9X7.
>>> +      */
>>> +     for (int i = 0; i < ARRAY_SIZE(xlcdc_csc_coeffs); i++)
>>> +             atmel_hlcdc_layer_write_cfg(&plane->layer,
>>> +                                         desc->layout.csc + i,
>>> +                                         xlcdc_csc_coeffs[i]);
>>
>> Ditto
>>
>>>   }
>>>
>>>   static int atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane)
>>>   {
>>>        const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
>>> +     struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
>>>
>>>        if (desc->type == ATMEL_HLCDC_OVERLAY_LAYER ||
>>>            desc->type == ATMEL_HLCDC_CURSOR_LAYER) {
>>> @@ -796,31 +1001,19 @@ static int atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane)
>>>                        return ret;
>>>        }
>>>
>>> -     if (desc->layout.csc) {
>>> -             /*
>>> -              * TODO: decare a "yuv-to-rgb-conv-factors" property to let
>>> -              * userspace modify these factors (using a BLOB property ?).
>>> -              */
>>> -             atmel_hlcdc_layer_write_cfg(&plane->layer,
>>> -                                         desc->layout.csc,
>>> -                                         0x4c900091);
>>> -             atmel_hlcdc_layer_write_cfg(&plane->layer,
>>> -                                         desc->layout.csc + 1,
>>> -                                         0x7a5f5090);
>>> -             atmel_hlcdc_layer_write_cfg(&plane->layer,
>>> -                                         desc->layout.csc + 2,
>>> -                                         0x40040890);
>>> -     }
>>> +     if (dc->desc->is_xlcdc && desc->layout.csc)
>>> +             xlcdc_csc_init(plane, desc);
>>> +     else
>>> +             if (desc->layout.csc)
>>> +                     hlcdc_csc_init(plane, desc);
>>
>>          if (desc->layout.csc)
>>                  dc->desc->lcdc_csc_init();
>>
>>>
>>>        return 0;
>>>   }
>>>
>>> -void atmel_hlcdc_plane_irq(struct atmel_hlcdc_plane *plane)
>>> +static void hlcdc_irq_dbg(struct atmel_hlcdc_plane *plane,
>>> +                       const struct atmel_hlcdc_layer_desc *desc)
>>>   {
>>> -     const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
>>> -     u32 isr;
>>> -
>>> -     isr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_ISR);
>>> +     u32 isr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_ISR);
>>>
>>>        /*
>>>         * There's not much we can do in case of overrun except informing
>>> @@ -830,8 +1023,34 @@ void atmel_hlcdc_plane_irq(struct atmel_hlcdc_plane *plane)
>>>        if (isr &
>>>            (ATMEL_HLCDC_LAYER_OVR_IRQ(0) | ATMEL_HLCDC_LAYER_OVR_IRQ(1) |
>>>             ATMEL_HLCDC_LAYER_OVR_IRQ(2)))
>>> -             dev_dbg(plane->base.dev->dev, "overrun on plane %s\n",
>>> -                     desc->name);
>>> +             pr_warn("%s: overrun on plane %s\n", __func__, desc->name);
>>
>> Why changing to pr_warn? why not dev_warn, if any?
>>
>>> +}
>>> +
>>> +static void xlcdc_irq_dbg(struct atmel_hlcdc_plane *plane,
>>> +                       const struct atmel_hlcdc_layer_desc *desc)
>>> +{
>>> +     u32 isr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_XLCDC_LAYER_ISR);
>>> +
>>> +     /*
>>> +      * There's not much we can do in case of overrun except informing
>>> +      * the user. However, we are in interrupt context here, hence the
>>> +      * use of dev_dbg().
>>> +      */
>>> +     if (isr &
>>> +         (ATMEL_XLCDC_LAYER_OVR_IRQ(0) | ATMEL_XLCDC_LAYER_OVR_IRQ(1) |
>>> +          ATMEL_XLCDC_LAYER_OVR_IRQ(2)))
>>> +             pr_warn("%s: overrun on plane %s\n", __func__, desc->name);
>>
>> dev_warn() ?
>>
>>> +}
>>> +
>>> +void atmel_hlcdc_plane_irq(struct atmel_hlcdc_plane *plane)
>>> +{
>>> +     const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
>>> +     struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
>>> +
>>> +     if (dc->desc->is_xlcdc)
>>> +             xlcdc_irq_dbg(plane, desc);
>>> +     else
>>> +             hlcdc_irq_dbg(plane, desc);
>>
>>          dc->desc->lcdc_irq_dbg() ?
>>
>>>   }
>>>
>>>   static const struct drm_plane_helper_funcs atmel_hlcdc_layer_plane_helper_funcs = {
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 6/8] drm: atmel-hlcdc: add DPI mode support for XLCDC

[Manikandan Muralidharan]

Add support for Display Pixel Interface (DPI) Compatible Mode
support in atmel-hlcdc driver for XLCDC IP along with legacy
pixel mapping.DPI mode BIT is configured in LCDC_CFG5 register.

Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
[durai.manickamkr@microchip.com: update DPI mode bit using is_xlcdc flag]
Signed-off-by: Durai Manickam KR <durai.manickamkr@microchip.com>
---
 .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c    | 22 ++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
index 4b11a1de8af4..c3d0c60ba419 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
@@ -30,10 +30,12 @@
  *
  * @base: base CRTC state
  * @output_mode: RGBXXX output mode
+ * @dpi: output DPI mode
  */
 struct atmel_hlcdc_crtc_state {
 	struct drm_crtc_state base;
 	unsigned int output_mode;
+	bool dpi;
 };
 
 static inline struct atmel_hlcdc_crtc_state *
@@ -164,6 +166,8 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c)
 
 	state = drm_crtc_state_to_atmel_hlcdc_crtc_state(c->state);
 	cfg = state->output_mode << 8;
+	if (is_xlcdc)
+		cfg |= state->dpi << 11;
 
 	if (!is_xlcdc && (adj->flags & DRM_MODE_FLAG_NVSYNC))
 		cfg |= ATMEL_HLCDC_VSPOL;
@@ -176,7 +180,9 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c)
 			   ATMEL_HLCDC_VSPDLYS | ATMEL_HLCDC_VSPDLYE |
 			   ATMEL_HLCDC_DISPPOL | ATMEL_HLCDC_DISPDLY |
 			   ATMEL_HLCDC_VSPSU | ATMEL_HLCDC_VSPHO |
-			   ATMEL_HLCDC_GUARDTIME_MASK | ATMEL_HLCDC_MODE_MASK,
+			   ATMEL_HLCDC_GUARDTIME_MASK |
+			   (is_xlcdc ? ATMEL_XLCDC_MODE_MASK |
+			   ATMEL_XLCDC_DPI : ATMEL_HLCDC_MODE_MASK),
 			   cfg);
 
 	clk_disable_unprepare(crtc->dc->hlcdc->sys_clk);
@@ -366,7 +372,15 @@ static int atmel_hlcdc_crtc_select_output_mode(struct drm_crtc_state *state)
 
 	hstate = drm_crtc_state_to_atmel_hlcdc_crtc_state(state);
 	hstate->output_mode = fls(output_fmts) - 1;
-
+	if (crtc->dc->desc->is_xlcdc) {
+		/* check if MIPI DPI bit needs to be set */
+		if (fls(output_fmts) > 3) {
+			hstate->output_mode -= 4;
+			hstate->dpi = true;
+		} else {
+			hstate->dpi = false;
+		}
+	}
 	return 0;
 }
 
@@ -470,7 +484,7 @@ static struct drm_crtc_state *
 atmel_hlcdc_crtc_duplicate_state(struct drm_crtc *crtc)
 {
 	struct atmel_hlcdc_crtc_state *state, *cur;
-
+	struct atmel_hlcdc_crtc *c = drm_crtc_to_atmel_hlcdc_crtc(crtc);
 	if (WARN_ON(!crtc->state))
 		return NULL;
 
@@ -481,6 +495,8 @@ atmel_hlcdc_crtc_duplicate_state(struct drm_crtc *crtc)
 
 	cur = drm_crtc_state_to_atmel_hlcdc_crtc_state(crtc->state);
 	state->output_mode = cur->output_mode;
+	if (c->dc->desc->is_xlcdc)
+		state->dpi = cur->dpi;
 
 	return &state->base;
 }
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 6/8] drm: atmel-hlcdc: add DPI mode support for XLCDC

[claudiu beznea]

On 8/25/23 15:54, Manikandan Muralidharan wrote:
> Add support for Display Pixel Interface (DPI) Compatible Mode
> support in atmel-hlcdc driver for XLCDC IP along with legacy
> pixel mapping.DPI mode BIT is configured in LCDC_CFG5 register.
> 
> Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
> [durai.manickamkr@microchip.com: update DPI mode bit using is_xlcdc flag]
> Signed-off-by: Durai Manickam KR <durai.manickamkr@microchip.com>
> ---
>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c    | 22 ++++++++++++++++---
>  1 file changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> index 4b11a1de8af4..c3d0c60ba419 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> @@ -30,10 +30,12 @@
>   *
>   * @base: base CRTC state
>   * @output_mode: RGBXXX output mode
> + * @dpi: output DPI mode
>   */
>  struct atmel_hlcdc_crtc_state {
>  	struct drm_crtc_state base;
>  	unsigned int output_mode;
> +	bool dpi;

To avoid confusion, better use u8 to avoid shifting a boolean value in
configuration phase.

>  };
>  
>  static inline struct atmel_hlcdc_crtc_state *
> @@ -164,6 +166,8 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c)
>  
>  	state = drm_crtc_state_to_atmel_hlcdc_crtc_state(c->state);
>  	cfg = state->output_mode << 8;
> +	if (is_xlcdc)
> +		cfg |= state->dpi << 11;
>  
>  	if (!is_xlcdc && (adj->flags & DRM_MODE_FLAG_NVSYNC))
>  		cfg |= ATMEL_HLCDC_VSPOL;
> @@ -176,7 +180,9 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c)
>  			   ATMEL_HLCDC_VSPDLYS | ATMEL_HLCDC_VSPDLYE |
>  			   ATMEL_HLCDC_DISPPOL | ATMEL_HLCDC_DISPDLY |
>  			   ATMEL_HLCDC_VSPSU | ATMEL_HLCDC_VSPHO |
> -			   ATMEL_HLCDC_GUARDTIME_MASK | ATMEL_HLCDC_MODE_MASK,
> +			   ATMEL_HLCDC_GUARDTIME_MASK |
> +			   (is_xlcdc ? ATMEL_XLCDC_MODE_MASK |
> +			   ATMEL_XLCDC_DPI : ATMEL_HLCDC_MODE_MASK),
>  			   cfg);
>  
>  	clk_disable_unprepare(crtc->dc->hlcdc->sys_clk);
> @@ -366,7 +372,15 @@ static int atmel_hlcdc_crtc_select_output_mode(struct drm_crtc_state *state)
>  
>  	hstate = drm_crtc_state_to_atmel_hlcdc_crtc_state(state);
>  	hstate->output_mode = fls(output_fmts) - 1;
> -
> +	if (crtc->dc->desc->is_xlcdc) {
> +		/* check if MIPI DPI bit needs to be set */
> +		if (fls(output_fmts) > 3) {
> +			hstate->output_mode -= 4;
> +			hstate->dpi = true;
> +		} else {
> +			hstate->dpi = false;
> +		}
> +	}
>  	return 0;
>  }
>  
> @@ -470,7 +484,7 @@ static struct drm_crtc_state *
>  atmel_hlcdc_crtc_duplicate_state(struct drm_crtc *crtc)
>  {
>  	struct atmel_hlcdc_crtc_state *state, *cur;
> -
> +	struct atmel_hlcdc_crtc *c = drm_crtc_to_atmel_hlcdc_crtc(crtc);
>  	if (WARN_ON(!crtc->state))
>  		return NULL;
>  
> @@ -481,6 +495,8 @@ atmel_hlcdc_crtc_duplicate_state(struct drm_crtc *crtc)
>  
>  	cur = drm_crtc_state_to_atmel_hlcdc_crtc_state(crtc->state);
>  	state->output_mode = cur->output_mode;
> +	if (c->dc->desc->is_xlcdc)
> +		state->dpi = cur->dpi;
>  
>  	return &state->base;
>  }

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 7/8] drm: atmel-hlcdc: add vertical and horizontal scaling support for XLCDC

[Manikandan Muralidharan]

update the LCDC_HEOCFG30 and LCDC_HEOCFG31 registers of XLCDC IP which
supports vertical and horizontal scaling with Bilinear and Bicubic
co-efficients taps for Chroma and Luma componenets of the Pixel.

Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c  |  2 ++
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h  |  4 ++++
 .../gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c   | 20 +++++++++++++++++++
 3 files changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
index d30aec174aa2..ae3e1a813482 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
@@ -536,6 +536,8 @@ static const struct atmel_hlcdc_layer_desc atmel_xlcdc_sam9x75_layers[] = {
 			.general_config = 12,
 			.csc = 16,
 			.scaler_config = 23,
+			.vxs_config = 30,
+			.hxs_config = 31,
 		},
 		.clut_offset = 0x1300,
 	},
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
index 8b05a54b5fd0..27074a4c5aec 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
@@ -198,6 +198,8 @@
  * @disc_pos: discard area position register
  * @disc_size: discard area size register
  * @csc: color space conversion register
+ * @vxs_config: vertical scalar filter taps control register
+ * @hxs_config: horizontal scalar filter taps control register
  */
 struct atmel_hlcdc_layer_cfg_layout {
 	int xstride[ATMEL_HLCDC_LAYER_MAX_PLANES];
@@ -217,6 +219,8 @@ struct atmel_hlcdc_layer_cfg_layout {
 	int disc_pos;
 	int disc_size;
 	int csc;
+	int vxs_config;
+	int hxs_config;
 };
 
 /**
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
index 26caf4cddfa4..a06ae2dc5909 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
@@ -972,6 +972,26 @@ static void xlcdc_csc_init(struct atmel_hlcdc_plane *plane,
 		atmel_hlcdc_layer_write_cfg(&plane->layer,
 					    desc->layout.csc + i,
 					    xlcdc_csc_coeffs[i]);
+
+	if (desc->layout.vxs_config && desc->layout.hxs_config) {
+		/*
+		 * Updating vxs.config and hxs.config fixes the
+		 * Green Color Issue in SAM9X7 EGT Video Player App
+		 */
+		atmel_hlcdc_layer_write_cfg(&plane->layer,
+					    desc->layout.vxs_config,
+					    ATMEL_XLCDC_LAYER_VXSYCFG_ONE |
+					    ATMEL_XLCDC_LAYER_VXSYTAP2_ENABLE |
+					    ATMEL_XLCDC_LAYER_VXSCCFG_ONE |
+					    ATMEL_XLCDC_LAYER_VXSCTAP2_ENABLE);
+
+		atmel_hlcdc_layer_write_cfg(&plane->layer,
+					    desc->layout.hxs_config,
+					    ATMEL_XLCDC_LAYER_HXSYCFG_ONE |
+					    ATMEL_XLCDC_LAYER_HXSYTAP2_ENABLE |
+					    ATMEL_XLCDC_LAYER_HXSCCFG_ONE |
+					    ATMEL_XLCDC_LAYER_HXSCTAP2_ENABLE);
+	}
 }
 
 static int atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane)
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 7/8] drm: atmel-hlcdc: add vertical and horizontal scaling support for XLCDC

[claudiu beznea]

On 8/25/23 15:54, Manikandan Muralidharan wrote:
> update the LCDC_HEOCFG30 and LCDC_HEOCFG31 registers of XLCDC IP which

s/update/Update

> supports vertical and horizontal scaling with Bilinear and Bicubic
> co-efficients taps for Chroma and Luma componenets of the Pixel.
> 
> Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
> ---
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c  |  2 ++
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h  |  4 ++++
>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c   | 20 +++++++++++++++++++
>  3 files changed, 26 insertions(+)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> index d30aec174aa2..ae3e1a813482 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> @@ -536,6 +536,8 @@ static const struct atmel_hlcdc_layer_desc atmel_xlcdc_sam9x75_layers[] = {
>  			.general_config = 12,
>  			.csc = 16,
>  			.scaler_config = 23,
> +			.vxs_config = 30,
> +			.hxs_config = 31,
>  		},
>  		.clut_offset = 0x1300,
>  	},
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> index 8b05a54b5fd0..27074a4c5aec 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> @@ -198,6 +198,8 @@
>   * @disc_pos: discard area position register
>   * @disc_size: discard area size register
>   * @csc: color space conversion register
> + * @vxs_config: vertical scalar filter taps control register
> + * @hxs_config: horizontal scalar filter taps control register
>   */
>  struct atmel_hlcdc_layer_cfg_layout {
>  	int xstride[ATMEL_HLCDC_LAYER_MAX_PLANES];
> @@ -217,6 +219,8 @@ struct atmel_hlcdc_layer_cfg_layout {
>  	int disc_pos;
>  	int disc_size;
>  	int csc;
> +	int vxs_config;
> +	int hxs_config;
>  };
>  
>  /**
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> index 26caf4cddfa4..a06ae2dc5909 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> @@ -972,6 +972,26 @@ static void xlcdc_csc_init(struct atmel_hlcdc_plane *plane,
>  		atmel_hlcdc_layer_write_cfg(&plane->layer,
>  					    desc->layout.csc + i,
>  					    xlcdc_csc_coeffs[i]);
> +
> +	if (desc->layout.vxs_config && desc->layout.hxs_config) {
> +		/*
> +		 * Updating vxs.config and hxs.config fixes the
> +		 * Green Color Issue in SAM9X7 EGT Video Player App
> +		 */
> +		atmel_hlcdc_layer_write_cfg(&plane->layer,
> +					    desc->layout.vxs_config,
> +					    ATMEL_XLCDC_LAYER_VXSYCFG_ONE |
> +					    ATMEL_XLCDC_LAYER_VXSYTAP2_ENABLE |
> +					    ATMEL_XLCDC_LAYER_VXSCCFG_ONE |
> +					    ATMEL_XLCDC_LAYER_VXSCTAP2_ENABLE);
> +
> +		atmel_hlcdc_layer_write_cfg(&plane->layer,
> +					    desc->layout.hxs_config,
> +					    ATMEL_XLCDC_LAYER_HXSYCFG_ONE |
> +					    ATMEL_XLCDC_LAYER_HXSYTAP2_ENABLE |
> +					    ATMEL_XLCDC_LAYER_HXSCCFG_ONE |
> +					    ATMEL_XLCDC_LAYER_HXSCTAP2_ENABLE);
> +	}
>  }
>  
>  static int atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane)

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 8/8] drm: atmel-hlcdc: add support for DSI output formats

[Manikandan Muralidharan]

Add support for the following DPI mode if the encoder type
is DSI as per the XLCDC IP datasheet:
- 16BPPCFG1
- 16BPPCFG2
- 16BPPCFG3
- 18BPPCFG1
- 18BPPCFG2
- 24BPP

Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
[durai.manickamkr@microchip.com: update output format using is_xlcdc flag]
Signed-off-by: Durai Manickam KR <durai.manickamkr@microchip.com>
---
 .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c    | 123 +++++++++++++-----
 1 file changed, 89 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
index c3d0c60ba419..0d10f84c82d8 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
@@ -287,11 +287,18 @@ static void atmel_hlcdc_crtc_atomic_enable(struct drm_crtc *c,
 
 }
 
-#define ATMEL_HLCDC_RGB444_OUTPUT	BIT(0)
-#define ATMEL_HLCDC_RGB565_OUTPUT	BIT(1)
-#define ATMEL_HLCDC_RGB666_OUTPUT	BIT(2)
-#define ATMEL_HLCDC_RGB888_OUTPUT	BIT(3)
-#define ATMEL_HLCDC_OUTPUT_MODE_MASK	GENMASK(3, 0)
+#define ATMEL_HLCDC_RGB444_OUTPUT		BIT(0)
+#define ATMEL_HLCDC_RGB565_OUTPUT		BIT(1)
+#define ATMEL_HLCDC_RGB666_OUTPUT		BIT(2)
+#define ATMEL_HLCDC_RGB888_OUTPUT		BIT(3)
+#define ATMEL_HLCDC_DPI_RGB565C1_OUTPUT		BIT(4)
+#define ATMEL_HLCDC_DPI_RGB565C2_OUTPUT		BIT(5)
+#define ATMEL_HLCDC_DPI_RGB565C3_OUTPUT		BIT(6)
+#define ATMEL_HLCDC_DPI_RGB666C1_OUTPUT		BIT(7)
+#define ATMEL_HLCDC_DPI_RGB666C2_OUTPUT		BIT(8)
+#define ATMEL_HLCDC_DPI_RGB888_OUTPUT		BIT(9)
+#define ATMEL_HLCDC_OUTPUT_MODE_MASK		GENMASK(3, 0)
+#define ATMEL_XLCDC_OUTPUT_MODE_MASK		GENMASK(9, 0)
 
 static int atmel_hlcdc_connector_output_mode(struct drm_connector_state *state)
 {
@@ -305,37 +312,83 @@ static int atmel_hlcdc_connector_output_mode(struct drm_connector_state *state)
 	if (!encoder)
 		encoder = connector->encoder;
 
-	switch (atmel_hlcdc_encoder_get_bus_fmt(encoder)) {
-	case 0:
-		break;
-	case MEDIA_BUS_FMT_RGB444_1X12:
-		return ATMEL_HLCDC_RGB444_OUTPUT;
-	case MEDIA_BUS_FMT_RGB565_1X16:
-		return ATMEL_HLCDC_RGB565_OUTPUT;
-	case MEDIA_BUS_FMT_RGB666_1X18:
-		return ATMEL_HLCDC_RGB666_OUTPUT;
-	case MEDIA_BUS_FMT_RGB888_1X24:
-		return ATMEL_HLCDC_RGB888_OUTPUT;
-	default:
-		return -EINVAL;
-	}
-
-	for (j = 0; j < info->num_bus_formats; j++) {
-		switch (info->bus_formats[j]) {
-		case MEDIA_BUS_FMT_RGB444_1X12:
-			supported_fmts |= ATMEL_HLCDC_RGB444_OUTPUT;
+	if (encoder->encoder_type == DRM_MODE_ENCODER_DSI) {
+		/*
+		 * atmel-hlcdc to support DSI formats with DSI video pipeline
+		 * when DRM_MODE_ENCODER_DSI type is set by
+		 * connector driver component.
+		 */
+		switch (atmel_hlcdc_encoder_get_bus_fmt(encoder)) {
+		case 0:
 			break;
 		case MEDIA_BUS_FMT_RGB565_1X16:
-			supported_fmts |= ATMEL_HLCDC_RGB565_OUTPUT;
-			break;
+			return ATMEL_HLCDC_DPI_RGB565C1_OUTPUT;
 		case MEDIA_BUS_FMT_RGB666_1X18:
-			supported_fmts |= ATMEL_HLCDC_RGB666_OUTPUT;
-			break;
+			return ATMEL_HLCDC_DPI_RGB666C1_OUTPUT;
+		case MEDIA_BUS_FMT_RGB666_1X24_CPADHI:
+			return ATMEL_HLCDC_DPI_RGB666C2_OUTPUT;
 		case MEDIA_BUS_FMT_RGB888_1X24:
-			supported_fmts |= ATMEL_HLCDC_RGB888_OUTPUT;
-			break;
+			return ATMEL_HLCDC_DPI_RGB888_OUTPUT;
 		default:
+			return -EINVAL;
+		}
+
+		for (j = 0; j < info->num_bus_formats; j++) {
+			switch (info->bus_formats[j]) {
+			case MEDIA_BUS_FMT_RGB565_1X16:
+				supported_fmts |=
+					ATMEL_HLCDC_DPI_RGB565C1_OUTPUT;
+				break;
+			case MEDIA_BUS_FMT_RGB666_1X18:
+				supported_fmts |=
+					ATMEL_HLCDC_DPI_RGB666C1_OUTPUT;
+				break;
+			case MEDIA_BUS_FMT_RGB666_1X24_CPADHI:
+				supported_fmts |=
+					ATMEL_HLCDC_DPI_RGB666C2_OUTPUT;
+				break;
+			case MEDIA_BUS_FMT_RGB888_1X24:
+				supported_fmts |=
+					ATMEL_HLCDC_DPI_RGB888_OUTPUT;
+				break;
+			default:
+				break;
+			}
+		}
+
+	} else {
+		switch (atmel_hlcdc_encoder_get_bus_fmt(encoder)) {
+		case 0:
 			break;
+		case MEDIA_BUS_FMT_RGB444_1X12:
+			return ATMEL_HLCDC_RGB444_OUTPUT;
+		case MEDIA_BUS_FMT_RGB565_1X16:
+			return ATMEL_HLCDC_RGB565_OUTPUT;
+		case MEDIA_BUS_FMT_RGB666_1X18:
+			return ATMEL_HLCDC_RGB666_OUTPUT;
+		case MEDIA_BUS_FMT_RGB888_1X24:
+			return ATMEL_HLCDC_RGB888_OUTPUT;
+		default:
+			return -EINVAL;
+		}
+
+		for (j = 0; j < info->num_bus_formats; j++) {
+			switch (info->bus_formats[j]) {
+			case MEDIA_BUS_FMT_RGB444_1X12:
+				supported_fmts |= ATMEL_HLCDC_RGB444_OUTPUT;
+				break;
+			case MEDIA_BUS_FMT_RGB565_1X16:
+				supported_fmts |= ATMEL_HLCDC_RGB565_OUTPUT;
+				break;
+			case MEDIA_BUS_FMT_RGB666_1X18:
+				supported_fmts |= ATMEL_HLCDC_RGB666_OUTPUT;
+				break;
+			case MEDIA_BUS_FMT_RGB888_1X24:
+				supported_fmts |= ATMEL_HLCDC_RGB888_OUTPUT;
+				break;
+			default:
+				break;
+			}
 		}
 	}
 
@@ -344,14 +397,16 @@ static int atmel_hlcdc_connector_output_mode(struct drm_connector_state *state)
 
 static int atmel_hlcdc_crtc_select_output_mode(struct drm_crtc_state *state)
 {
-	unsigned int output_fmts = ATMEL_HLCDC_OUTPUT_MODE_MASK;
+	unsigned int output_fmts;
 	struct atmel_hlcdc_crtc_state *hstate;
 	struct drm_connector_state *cstate;
 	struct drm_connector *connector;
-	struct atmel_hlcdc_crtc *crtc;
+	struct atmel_hlcdc_crtc *crtc = drm_crtc_to_atmel_hlcdc_crtc(state->crtc);
 	int i;
+	bool is_xlcdc = crtc->dc->desc->is_xlcdc;
 
-	crtc = drm_crtc_to_atmel_hlcdc_crtc(state->crtc);
+	output_fmts = is_xlcdc ? ATMEL_XLCDC_OUTPUT_MODE_MASK :
+		      ATMEL_HLCDC_OUTPUT_MODE_MASK;
 
 	for_each_new_connector_in_state(state->state, connector, cstate, i) {
 		unsigned int supported_fmts = 0;
@@ -372,7 +427,7 @@ static int atmel_hlcdc_crtc_select_output_mode(struct drm_crtc_state *state)
 
 	hstate = drm_crtc_state_to_atmel_hlcdc_crtc_state(state);
 	hstate->output_mode = fls(output_fmts) - 1;
-	if (crtc->dc->desc->is_xlcdc) {
+	if (is_xlcdc) {
 		/* check if MIPI DPI bit needs to be set */
 		if (fls(output_fmts) > 3) {
 			hstate->output_mode -= 4;
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 8/8] drm: atmel-hlcdc: add support for DSI output formats

[claudiu beznea]

On 8/25/23 15:54, Manikandan Muralidharan wrote:
> Add support for the following DPI mode if the encoder type
> is DSI as per the XLCDC IP datasheet:
> - 16BPPCFG1
> - 16BPPCFG2
> - 16BPPCFG3
> - 18BPPCFG1
> - 18BPPCFG2
> - 24BPP
> 
> Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
> [durai.manickamkr@microchip.com: update output format using is_xlcdc flag]
> Signed-off-by: Durai Manickam KR <durai.manickamkr@microchip.com>
> ---
>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c    | 123 +++++++++++++-----
>  1 file changed, 89 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> index c3d0c60ba419..0d10f84c82d8 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> @@ -287,11 +287,18 @@ static void atmel_hlcdc_crtc_atomic_enable(struct drm_crtc *c,
>  
>  }
>  
> -#define ATMEL_HLCDC_RGB444_OUTPUT	BIT(0)
> -#define ATMEL_HLCDC_RGB565_OUTPUT	BIT(1)
> -#define ATMEL_HLCDC_RGB666_OUTPUT	BIT(2)
> -#define ATMEL_HLCDC_RGB888_OUTPUT	BIT(3)
> -#define ATMEL_HLCDC_OUTPUT_MODE_MASK	GENMASK(3, 0)
> +#define ATMEL_HLCDC_RGB444_OUTPUT		BIT(0)
> +#define ATMEL_HLCDC_RGB565_OUTPUT		BIT(1)
> +#define ATMEL_HLCDC_RGB666_OUTPUT		BIT(2)
> +#define ATMEL_HLCDC_RGB888_OUTPUT		BIT(3)
> +#define ATMEL_HLCDC_DPI_RGB565C1_OUTPUT		BIT(4)
> +#define ATMEL_HLCDC_DPI_RGB565C2_OUTPUT		BIT(5)
> +#define ATMEL_HLCDC_DPI_RGB565C3_OUTPUT		BIT(6)
> +#define ATMEL_HLCDC_DPI_RGB666C1_OUTPUT		BIT(7)
> +#define ATMEL_HLCDC_DPI_RGB666C2_OUTPUT		BIT(8)
> +#define ATMEL_HLCDC_DPI_RGB888_OUTPUT		BIT(9)
> +#define ATMEL_HLCDC_OUTPUT_MODE_MASK		GENMASK(3, 0)
> +#define ATMEL_XLCDC_OUTPUT_MODE_MASK		GENMASK(9, 0)
>  
>  static int atmel_hlcdc_connector_output_mode(struct drm_connector_state *state)
>  {
> @@ -305,37 +312,83 @@ static int atmel_hlcdc_connector_output_mode(struct drm_connector_state *state)
>  	if (!encoder)
>  		encoder = connector->encoder;
>  
> -	switch (atmel_hlcdc_encoder_get_bus_fmt(encoder)) {
> -	case 0:
> -		break;
> -	case MEDIA_BUS_FMT_RGB444_1X12:
> -		return ATMEL_HLCDC_RGB444_OUTPUT;
> -	case MEDIA_BUS_FMT_RGB565_1X16:
> -		return ATMEL_HLCDC_RGB565_OUTPUT;
> -	case MEDIA_BUS_FMT_RGB666_1X18:
> -		return ATMEL_HLCDC_RGB666_OUTPUT;
> -	case MEDIA_BUS_FMT_RGB888_1X24:
> -		return ATMEL_HLCDC_RGB888_OUTPUT;
> -	default:
> -		return -EINVAL;
> -	}
> -
> -	for (j = 0; j < info->num_bus_formats; j++) {
> -		switch (info->bus_formats[j]) {
> -		case MEDIA_BUS_FMT_RGB444_1X12:
> -			supported_fmts |= ATMEL_HLCDC_RGB444_OUTPUT;
> +	if (encoder->encoder_type == DRM_MODE_ENCODER_DSI) {
> +		/*
> +		 * atmel-hlcdc to support DSI formats with DSI video pipeline
> +		 * when DRM_MODE_ENCODER_DSI type is set by
> +		 * connector driver component.
> +		 */
> +		switch (atmel_hlcdc_encoder_get_bus_fmt(encoder)) {
> +		case 0:
>  			break;
>  		case MEDIA_BUS_FMT_RGB565_1X16:
> -			supported_fmts |= ATMEL_HLCDC_RGB565_OUTPUT;
> -			break;
> +			return ATMEL_HLCDC_DPI_RGB565C1_OUTPUT;
>  		case MEDIA_BUS_FMT_RGB666_1X18:
> -			supported_fmts |= ATMEL_HLCDC_RGB666_OUTPUT;
> -			break;
> +			return ATMEL_HLCDC_DPI_RGB666C1_OUTPUT;
> +		case MEDIA_BUS_FMT_RGB666_1X24_CPADHI:
> +			return ATMEL_HLCDC_DPI_RGB666C2_OUTPUT;
>  		case MEDIA_BUS_FMT_RGB888_1X24:
> -			supported_fmts |= ATMEL_HLCDC_RGB888_OUTPUT;
> -			break;
> +			return ATMEL_HLCDC_DPI_RGB888_OUTPUT;
>  		default:
> +			return -EINVAL;
> +		}
> +
> +		for (j = 0; j < info->num_bus_formats; j++) {
> +			switch (info->bus_formats[j]) {
> +			case MEDIA_BUS_FMT_RGB565_1X16:
> +				supported_fmts |=
> +					ATMEL_HLCDC_DPI_RGB565C1_OUTPUT;
> +				break;
> +			case MEDIA_BUS_FMT_RGB666_1X18:
> +				supported_fmts |=
> +					ATMEL_HLCDC_DPI_RGB666C1_OUTPUT;
> +				break;
> +			case MEDIA_BUS_FMT_RGB666_1X24_CPADHI:
> +				supported_fmts |=
> +					ATMEL_HLCDC_DPI_RGB666C2_OUTPUT;
> +				break;
> +			case MEDIA_BUS_FMT_RGB888_1X24:
> +				supported_fmts |=
> +					ATMEL_HLCDC_DPI_RGB888_OUTPUT;
> +				break;
> +			default:
> +				break;
> +			}
> +		}
> +

blank line here.

> +	} else {
> +		switch (atmel_hlcdc_encoder_get_bus_fmt(encoder)) {
> +		case 0:
>  			break;
> +		case MEDIA_BUS_FMT_RGB444_1X12:
> +			return ATMEL_HLCDC_RGB444_OUTPUT;
> +		case MEDIA_BUS_FMT_RGB565_1X16:
> +			return ATMEL_HLCDC_RGB565_OUTPUT;
> +		case MEDIA_BUS_FMT_RGB666_1X18:
> +			return ATMEL_HLCDC_RGB666_OUTPUT;
> +		case MEDIA_BUS_FMT_RGB888_1X24:
> +			return ATMEL_HLCDC_RGB888_OUTPUT;
> +		default:
> +			return -EINVAL;
> +		}
> +
> +		for (j = 0; j < info->num_bus_formats; j++) {
> +			switch (info->bus_formats[j]) {
> +			case MEDIA_BUS_FMT_RGB444_1X12:
> +				supported_fmts |= ATMEL_HLCDC_RGB444_OUTPUT;
> +				break;
> +			case MEDIA_BUS_FMT_RGB565_1X16:
> +				supported_fmts |= ATMEL_HLCDC_RGB565_OUTPUT;
> +				break;
> +			case MEDIA_BUS_FMT_RGB666_1X18:
> +				supported_fmts |= ATMEL_HLCDC_RGB666_OUTPUT;
> +				break;
> +			case MEDIA_BUS_FMT_RGB888_1X24:
> +				supported_fmts |= ATMEL_HLCDC_RGB888_OUTPUT;
> +				break;
> +			default:
> +				break;
> +			}
>  		}
>  	}
>  
> @@ -344,14 +397,16 @@ static int atmel_hlcdc_connector_output_mode(struct drm_connector_state *state)
>  
>  static int atmel_hlcdc_crtc_select_output_mode(struct drm_crtc_state *state)
>  {
> -	unsigned int output_fmts = ATMEL_HLCDC_OUTPUT_MODE_MASK;
> +	unsigned int output_fmts;
>  	struct atmel_hlcdc_crtc_state *hstate;
>  	struct drm_connector_state *cstate;
>  	struct drm_connector *connector;
> -	struct atmel_hlcdc_crtc *crtc;
> +	struct atmel_hlcdc_crtc *crtc = drm_crtc_to_atmel_hlcdc_crtc(state->crtc);
>  	int i;
> +	bool is_xlcdc = crtc->dc->desc->is_xlcdc;
>  
> -	crtc = drm_crtc_to_atmel_hlcdc_crtc(state->crtc);
> +	output_fmts = is_xlcdc ? ATMEL_XLCDC_OUTPUT_MODE_MASK :
> +		      ATMEL_HLCDC_OUTPUT_MODE_MASK;
>  
>  	for_each_new_connector_in_state(state->state, connector, cstate, i) {
>  		unsigned int supported_fmts = 0;
> @@ -372,7 +427,7 @@ static int atmel_hlcdc_crtc_select_output_mode(struct drm_crtc_state *state)
>  
>  	hstate = drm_crtc_state_to_atmel_hlcdc_crtc_state(state);
>  	hstate->output_mode = fls(output_fmts) - 1;
> -	if (crtc->dc->desc->is_xlcdc) {
> +	if (is_xlcdc) {
>  		/* check if MIPI DPI bit needs to be set */
>  		if (fls(output_fmts) > 3) {
>  			hstate->output_mode -= 4;

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel