[U-Boot] [PATCH v2 0/2] at91: video: add driver for Atmel HLCD controller

[U-Boot] [PATCH v2 0/2] at91: video: add driver for Atmel HLCD controller

[Songjun Wu]

The i2c driver includes two parts.
1) Driver code to implement the HLCD function.
2) Device tree binding documentation, it describes how
   to add the HLCD in device tree.

Changes in v2:
- Due to the peripheral clock driver improvement, remove
  the unneccessary clock calling.

Songjun Wu (2):
  at91: video: Support driver-model for the HLCD driver
  at91: video: DT binding for HLCDC driver

 board/atmel/at91sam9n12ek/at91sam9n12ek.c       |   2 +
 board/atmel/at91sam9x5ek/at91sam9x5ek.c         |   2 +
 board/atmel/sama5d2_xplained/sama5d2_xplained.c |   2 +
 board/atmel/sama5d3xek/sama5d3xek.c             |   2 +
 board/atmel/sama5d4_xplained/sama5d4_xplained.c |   2 +
 board/atmel/sama5d4ek/sama5d4ek.c               |   2 +
 doc/device-tree-bindings/video/atme-hlcdc.txt   |  38 +++
 drivers/video/Kconfig                           |   9 +
 drivers/video/atmel_hlcdfb.c                    | 314 +++++++++++++++++++++++-
 include/configs/at91sam9n12ek.h                 |   4 +
 include/configs/at91sam9x5ek.h                  |   4 +
 include/configs/ma5d4evk.h                      |   4 +
 include/configs/sama5d2_xplained.h              |   4 +
 include/configs/sama5d3xek.h                    |   4 +
 include/configs/sama5d4_xplained.h              |   4 +
 include/configs/sama5d4ek.h                     |   4 +
 16 files changed, 398 insertions(+), 3 deletions(-)
 create mode 100644 doc/device-tree-bindings/video/atme-hlcdc.txt

-- 
2.11.0

[U-Boot] [PATCH v2 1/2] at91: video: Support driver-model for the HLCD driver

[Songjun Wu]

Add driver-model support to this driver.

Signed-off-by: Songjun Wu <songjun.wu@microchip.com>
---

Changes in v2:
- Due to the peripheral clock driver improvement, remove
  the unneccessary clock calling.

 board/atmel/at91sam9n12ek/at91sam9n12ek.c       |   2 +
 board/atmel/at91sam9x5ek/at91sam9x5ek.c         |   2 +
 board/atmel/sama5d2_xplained/sama5d2_xplained.c |   2 +
 board/atmel/sama5d3xek/sama5d3xek.c             |   2 +
 board/atmel/sama5d4_xplained/sama5d4_xplained.c |   2 +
 board/atmel/sama5d4ek/sama5d4ek.c               |   2 +
 drivers/video/Kconfig                           |   9 +
 drivers/video/atmel_hlcdfb.c                    | 314 +++++++++++++++++++++++-
 include/configs/at91sam9n12ek.h                 |   4 +
 include/configs/at91sam9x5ek.h                  |   4 +
 include/configs/ma5d4evk.h                      |   4 +
 include/configs/sama5d2_xplained.h              |   4 +
 include/configs/sama5d3xek.h                    |   4 +
 include/configs/sama5d4_xplained.h              |   4 +
 include/configs/sama5d4ek.h                     |   4 +
 15 files changed, 360 insertions(+), 3 deletions(-)

diff --git a/board/atmel/at91sam9n12ek/at91sam9n12ek.c b/board/atmel/at91sam9n12ek/at91sam9n12ek.c
index fc4f50d219..e70a929299 100644
--- a/board/atmel/at91sam9n12ek/at91sam9n12ek.c
+++ b/board/atmel/at91sam9n12ek/at91sam9n12ek.c
@@ -79,6 +79,7 @@ static void at91sam9n12ek_nand_hw_init(void)
 #endif
 
 #ifdef CONFIG_LCD
+#ifndef CONFIG_DM_VIDEO
 vidinfo_t panel_info = {
 	.vl_col = 480,
 	.vl_row = 272,
@@ -104,6 +105,7 @@ void lcd_disable(void)
 {
 	at91_set_pio_output(AT91_PIO_PORTC, 25, 1);	/* power down */
 }
+#endif
 
 #ifdef CONFIG_LCD_INFO
 void lcd_show_board_info(void)
diff --git a/board/atmel/at91sam9x5ek/at91sam9x5ek.c b/board/atmel/at91sam9x5ek/at91sam9x5ek.c
index b0d440d728..8504d604fa 100644
--- a/board/atmel/at91sam9x5ek/at91sam9x5ek.c
+++ b/board/atmel/at91sam9x5ek/at91sam9x5ek.c
@@ -108,6 +108,7 @@ int board_eth_init(bd_t *bis)
 }
 
 #ifdef CONFIG_LCD
+#ifndef CONFIG_DM_VIDEO
 vidinfo_t panel_info = {
 	.vl_col	= 800,
 	.vl_row = 480,
@@ -136,6 +137,7 @@ void lcd_disable(void)
 	if (has_lcdc())
 		at91_set_a_periph(AT91_PIO_PORTC, 29, 0);	/* power down */
 }
+#endif
 
 static void at91sam9x5ek_lcd_hw_init(void)
 {
diff --git a/board/atmel/sama5d2_xplained/sama5d2_xplained.c b/board/atmel/sama5d2_xplained/sama5d2_xplained.c
index c5337af4de..6e8851aaf6 100644
--- a/board/atmel/sama5d2_xplained/sama5d2_xplained.c
+++ b/board/atmel/sama5d2_xplained/sama5d2_xplained.c
@@ -34,6 +34,7 @@ static void board_usb_hw_init(void)
 }
 
 #ifdef CONFIG_LCD
+#ifndef CONFIG_DM_VIDEO
 vidinfo_t panel_info = {
 	.vl_col = 480,
 	.vl_row = 272,
@@ -57,6 +58,7 @@ unsigned int has_lcdc(void)
 {
 	return 1;
 }
+#endif
 
 static void board_lcd_hw_init(void)
 {
diff --git a/board/atmel/sama5d3xek/sama5d3xek.c b/board/atmel/sama5d3xek/sama5d3xek.c
index ce67478f0b..a83e96575f 100644
--- a/board/atmel/sama5d3xek/sama5d3xek.c
+++ b/board/atmel/sama5d3xek/sama5d3xek.c
@@ -142,6 +142,7 @@ static void sama5d3xek_mci_hw_init(void)
 #endif
 
 #ifdef CONFIG_LCD
+#ifndef CONFIG_DM_VIDEO
 vidinfo_t panel_info = {
 	.vl_col = 800,
 	.vl_row = 480,
@@ -164,6 +165,7 @@ void lcd_enable(void)
 void lcd_disable(void)
 {
 }
+#endif
 
 static void sama5d3xek_lcd_hw_init(void)
 {
diff --git a/board/atmel/sama5d4_xplained/sama5d4_xplained.c b/board/atmel/sama5d4_xplained/sama5d4_xplained.c
index 23ec274468..6512429ba7 100644
--- a/board/atmel/sama5d4_xplained/sama5d4_xplained.c
+++ b/board/atmel/sama5d4_xplained/sama5d4_xplained.c
@@ -108,6 +108,7 @@ static void sama5d4_xplained_usb_hw_init(void)
 #endif
 
 #ifdef CONFIG_LCD
+#ifndef CONFIG_DM_VIDEO
 vidinfo_t panel_info = {
 	.vl_col = 480,
 	.vl_row = 272,
@@ -131,6 +132,7 @@ unsigned int has_lcdc(void)
 {
 	return 1;
 }
+#endif
 
 static void sama5d4_xplained_lcd_hw_init(void)
 {
diff --git a/board/atmel/sama5d4ek/sama5d4ek.c b/board/atmel/sama5d4ek/sama5d4ek.c
index 72bad23087..82b8590102 100644
--- a/board/atmel/sama5d4ek/sama5d4ek.c
+++ b/board/atmel/sama5d4ek/sama5d4ek.c
@@ -109,6 +109,7 @@ static void sama5d4ek_usb_hw_init(void)
 #endif
 
 #ifdef CONFIG_LCD
+#ifndef CONFIG_DM_VIDEO
 vidinfo_t panel_info = {
 	.vl_col = 800,
 	.vl_row = 480,
@@ -132,6 +133,7 @@ unsigned int has_lcdc(void)
 {
 	return 1;
 }
+#endif
 
 static void sama5d4ek_lcd_hw_init(void)
 {
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 2069576958..44756ea18e 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -371,6 +371,15 @@ config DISPLAY
 	   The devices provide a simple interface to start up the display,
 	   read display information and enable it.
 
+config ATMEL_HLCD
+	bool "Enable ATMEL video support using HLCDC"
+	depends on DM_VIDEO
+	help
+	   HLCDC supports video output to an attached LCD panel as well as
+	   other options such as HDMI. At present only LCD is supported by
+	   U-Boot. This option enables this support which can be used on
+	   devices which have an LCD display connected.
+
 config VIDEO_BROADWELL_IGD
 	bool "Enable Intel Broadwell integrated graphics device"
 	depends on X86
diff --git a/drivers/video/atmel_hlcdfb.c b/drivers/video/atmel_hlcdfb.c
index 960b474b76..0bcb92b2cf 100644
--- a/drivers/video/atmel_hlcdfb.c
+++ b/drivers/video/atmel_hlcdfb.c
@@ -10,13 +10,24 @@
 #include <asm/io.h>
 #include <asm/arch/gpio.h>
 #include <asm/arch/clk.h>
+#include <clk.h>
+#include <dm.h>
+#include <fdtdec.h>
 #include <lcd.h>
+#include <video.h>
 #include <atmel_hlcdc.h>
 
 #if defined(CONFIG_LCD_LOGO)
 #include <bmp_logo.h>
 #endif
 
+DECLARE_GLOBAL_DATA_PTR;
+
+#define lcdc_readl(reg)		__raw_readl((reg))
+#define lcdc_writel(reg, val)	__raw_writel((val), (reg))
+
+#ifndef CONFIG_DM_VIDEO
+
 /* configurable parameters */
 #define ATMEL_LCDC_CVAL_DEFAULT		0xc8
 #define ATMEL_LCDC_DMA_BURST_LEN	8
@@ -26,9 +37,6 @@
 
 #define ATMEL_LCDC_FIFO_SIZE		512
 
-#define lcdc_readl(reg)		__raw_readl((reg))
-#define lcdc_writel(reg, val)	__raw_writel((val), (reg))
-
 /*
  * the CLUT register map as following
  * RCLUT(24 ~ 16), GCLUT(15 ~ 8), BCLUT(7 ~ 0)
@@ -218,3 +226,303 @@ void lcd_ctrl_init(void *lcdbase)
 	/* Enable flushing if we enabled dcache */
 	lcd_set_flush_dcache(1);
 }
+
+#else
+
+enum {
+	LCD_MAX_WIDTH		= 1024,
+	LCD_MAX_HEIGHT		= 768,
+	LCD_MAX_LOG2_BPP	= VIDEO_BPP16,
+};
+
+struct atmel_hlcdc_priv {
+	struct atmel_hlcd_regs *regs;
+	struct display_timing timing;
+	unsigned int vl_bpix;
+	unsigned int guard_time;
+	ulong clk_rate;
+};
+
+static int at91_hlcdc_enable_clk(struct udevice *dev)
+{
+	struct atmel_hlcdc_priv *priv = dev_get_priv(dev);
+	struct clk clk;
+	ulong clk_rate;
+	int ret;
+
+	ret = clk_get_by_index(dev, 0, &clk);
+	if (ret)
+		return -EINVAL;
+
+	ret = clk_enable(&clk);
+	if (ret)
+		return ret;
+
+	clk_rate = clk_get_rate(&clk);
+	if (!clk_rate)
+		return -ENODEV;
+
+	priv->clk_rate = clk_rate;
+
+	clk_free(&clk);
+
+	return 0;
+}
+
+static void atmel_hlcdc_init(struct udevice *dev)
+{
+	struct video_uc_platdata *uc_plat = dev_get_uclass_platdata(dev);
+	struct atmel_hlcdc_priv *priv = dev_get_priv(dev);
+	struct atmel_hlcd_regs *regs = priv->regs;
+	struct display_timing *timing = &priv->timing;
+	struct lcd_dma_desc *desc;
+	unsigned long value, vl_clk_pol;
+
+	/* Disable DISP signal */
+	lcdc_writel(&regs->lcdc_lcddis, LCDC_LCDDIS_DISPDIS);
+	while ((lcdc_readl(&regs->lcdc_lcdsr) & LCDC_LCDSR_DISPSTS))
+		udelay(1);
+	/* Disable synchronization */
+	lcdc_writel(&regs->lcdc_lcddis, LCDC_LCDDIS_SYNCDIS);
+	while ((lcdc_readl(&regs->lcdc_lcdsr) & LCDC_LCDSR_LCDSTS))
+		udelay(1);
+	/* Disable pixel clock */
+	lcdc_writel(&regs->lcdc_lcddis, LCDC_LCDDIS_CLKDIS);
+	while ((lcdc_readl(&regs->lcdc_lcdsr) & LCDC_LCDSR_CLKSTS))
+		udelay(1);
+	/* Disable PWM */
+	lcdc_writel(&regs->lcdc_lcddis, LCDC_LCDDIS_PWMDIS);
+	while ((lcdc_readl(&regs->lcdc_lcdsr) & LCDC_LCDSR_PWMSTS))
+		udelay(1);
+
+	/* Set pixel clock */
+	value = priv->clk_rate / timing->pixelclock.typ;
+	if (priv->clk_rate % timing->pixelclock.typ)
+		value++;
+
+	vl_clk_pol = 0;
+	if (timing->flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
+		vl_clk_pol = LCDC_LCDCFG0_CLKPOL;
+
+	if (value < 1) {
+		/* Using system clock as pixel clock */
+		lcdc_writel(&regs->lcdc_lcdcfg0,
+			    LCDC_LCDCFG0_CLKDIV(0)
+			    | LCDC_LCDCFG0_CGDISHCR
+			    | LCDC_LCDCFG0_CGDISHEO
+			    | LCDC_LCDCFG0_CGDISOVR1
+			    | LCDC_LCDCFG0_CGDISBASE
+			    | vl_clk_pol
+			    | LCDC_LCDCFG0_CLKSEL);
+
+	} else {
+		lcdc_writel(&regs->lcdc_lcdcfg0,
+			    LCDC_LCDCFG0_CLKDIV(value - 2)
+			    | LCDC_LCDCFG0_CGDISHCR
+			    | LCDC_LCDCFG0_CGDISHEO
+			    | LCDC_LCDCFG0_CGDISOVR1
+			    | LCDC_LCDCFG0_CGDISBASE
+			    | vl_clk_pol);
+	}
+
+	/* Initialize control register 5 */
+	value = 0;
+
+	if (!(timing->flags & DISPLAY_FLAGS_HSYNC_HIGH))
+		value |= LCDC_LCDCFG5_HSPOL;
+	if (!(timing->flags & DISPLAY_FLAGS_VSYNC_HIGH))
+		value |= LCDC_LCDCFG5_VSPOL;
+
+#ifndef LCD_OUTPUT_BPP
+	/* Output is 24bpp */
+	value |= LCDC_LCDCFG5_MODE_OUTPUT_24BPP;
+#else
+	switch (LCD_OUTPUT_BPP) {
+	case 12:
+		value |= LCDC_LCDCFG5_MODE_OUTPUT_12BPP;
+		break;
+	case 16:
+		value |= LCDC_LCDCFG5_MODE_OUTPUT_16BPP;
+		break;
+	case 18:
+		value |= LCDC_LCDCFG5_MODE_OUTPUT_18BPP;
+		break;
+	case 24:
+		value |= LCDC_LCDCFG5_MODE_OUTPUT_24BPP;
+		break;
+	default:
+		BUG();
+		break;
+	}
+#endif
+
+	value |= LCDC_LCDCFG5_GUARDTIME(priv->guard_time);
+	value |= (LCDC_LCDCFG5_DISPDLY | LCDC_LCDCFG5_VSPDLYS);
+	lcdc_writel(&regs->lcdc_lcdcfg5, value);
+
+	/* Vertical & Horizontal Timing */
+	value = LCDC_LCDCFG1_VSPW(timing->vsync_len.typ - 1);
+	value |= LCDC_LCDCFG1_HSPW(timing->hsync_len.typ - 1);
+	lcdc_writel(&regs->lcdc_lcdcfg1, value);
+
+	value = LCDC_LCDCFG2_VBPW(timing->vback_porch.typ);
+	value |= LCDC_LCDCFG2_VFPW(timing->vfront_porch.typ - 1);
+	lcdc_writel(&regs->lcdc_lcdcfg2, value);
+
+	value = LCDC_LCDCFG3_HBPW(timing->hback_porch.typ - 1);
+	value |= LCDC_LCDCFG3_HFPW(timing->hfront_porch.typ - 1);
+	lcdc_writel(&regs->lcdc_lcdcfg3, value);
+
+	/* Display size */
+	value = LCDC_LCDCFG4_RPF(timing->vactive.typ - 1);
+	value |= LCDC_LCDCFG4_PPL(timing->hactive.typ - 1);
+	lcdc_writel(&regs->lcdc_lcdcfg4, value);
+
+	lcdc_writel(&regs->lcdc_basecfg0,
+		    LCDC_BASECFG0_BLEN_AHB_INCR4 | LCDC_BASECFG0_DLBO);
+
+	switch (VNBITS(priv->vl_bpix)) {
+	case 16:
+		lcdc_writel(&regs->lcdc_basecfg1,
+			    LCDC_BASECFG1_RGBMODE_16BPP_RGB_565);
+		break;
+	case 32:
+		lcdc_writel(&regs->lcdc_basecfg1,
+			    LCDC_BASECFG1_RGBMODE_24BPP_RGB_888);
+		break;
+	default:
+		BUG();
+		break;
+	}
+
+	lcdc_writel(&regs->lcdc_basecfg2, LCDC_BASECFG2_XSTRIDE(0));
+	lcdc_writel(&regs->lcdc_basecfg3, 0);
+	lcdc_writel(&regs->lcdc_basecfg4, LCDC_BASECFG4_DMA);
+
+	/* Disable all interrupts */
+	lcdc_writel(&regs->lcdc_lcdidr, ~0UL);
+	lcdc_writel(&regs->lcdc_baseidr, ~0UL);
+
+	/* Setup the DMA descriptor, this descriptor will loop to itself */
+	desc = (struct lcd_dma_desc *)(uc_plat->base - 16);
+
+	desc->address = (u32)uc_plat->base;
+
+	/* Disable DMA transfer interrupt & descriptor loaded interrupt. */
+	desc->control = LCDC_BASECTRL_ADDIEN | LCDC_BASECTRL_DSCRIEN
+			| LCDC_BASECTRL_DMAIEN | LCDC_BASECTRL_DFETCH;
+	desc->next = (u32)desc;
+
+	/* Flush the DMA descriptor if we enabled dcache */
+	flush_dcache_range((u32)desc, (u32)desc + sizeof(*desc));
+
+	lcdc_writel(&regs->lcdc_baseaddr, desc->address);
+	lcdc_writel(&regs->lcdc_basectrl, desc->control);
+	lcdc_writel(&regs->lcdc_basenext, desc->next);
+	lcdc_writel(&regs->lcdc_basecher,
+		    LCDC_BASECHER_CHEN | LCDC_BASECHER_UPDATEEN);
+
+	/* Enable LCD */
+	value = lcdc_readl(&regs->lcdc_lcden);
+	lcdc_writel(&regs->lcdc_lcden, value | LCDC_LCDEN_CLKEN);
+	while (!(lcdc_readl(&regs->lcdc_lcdsr) & LCDC_LCDSR_CLKSTS))
+		udelay(1);
+	value = lcdc_readl(&regs->lcdc_lcden);
+	lcdc_writel(&regs->lcdc_lcden, value | LCDC_LCDEN_SYNCEN);
+	while (!(lcdc_readl(&regs->lcdc_lcdsr) & LCDC_LCDSR_LCDSTS))
+		udelay(1);
+	value = lcdc_readl(&regs->lcdc_lcden);
+	lcdc_writel(&regs->lcdc_lcden, value | LCDC_LCDEN_DISPEN);
+	while (!(lcdc_readl(&regs->lcdc_lcdsr) & LCDC_LCDSR_DISPSTS))
+		udelay(1);
+	value = lcdc_readl(&regs->lcdc_lcden);
+	lcdc_writel(&regs->lcdc_lcden, value | LCDC_LCDEN_PWMEN);
+	while (!(lcdc_readl(&regs->lcdc_lcdsr) & LCDC_LCDSR_PWMSTS))
+		udelay(1);
+}
+
+static int atmel_hlcdc_probe(struct udevice *dev)
+{
+	struct video_priv *uc_priv = dev_get_uclass_priv(dev);
+	struct atmel_hlcdc_priv *priv = dev_get_priv(dev);
+	int ret;
+
+	ret = at91_hlcdc_enable_clk(dev);
+	if (ret)
+		return ret;
+
+	atmel_hlcdc_init(dev);
+
+	uc_priv->xsize = priv->timing.hactive.typ;
+	uc_priv->ysize = priv->timing.vactive.typ;
+	uc_priv->bpix = priv->vl_bpix;
+
+	/* Enable flushing if we enabled dcache */
+	video_set_flush_dcache(dev, true);
+
+	return 0;
+}
+
+static int atmel_hlcdc_ofdata_to_platdata(struct udevice *dev)
+{
+	struct atmel_hlcdc_priv *priv = dev_get_priv(dev);
+	const void *blob = gd->fdt_blob;
+	int node = dev->of_offset;
+
+	priv->regs = (struct atmel_hlcd_regs *)dev_get_addr(dev);
+	if (!priv->regs) {
+		debug("%s: No display controller address\n", __func__);
+		return -EINVAL;
+	}
+
+	if (fdtdec_decode_display_timing(blob, dev->of_offset,
+					 0, &priv->timing)) {
+		debug("%s: Failed to decode display timing\n", __func__);
+		return -EINVAL;
+	}
+
+	if (priv->timing.hactive.typ > LCD_MAX_WIDTH)
+		priv->timing.hactive.typ = LCD_MAX_WIDTH;
+
+	if (priv->timing.vactive.typ > LCD_MAX_HEIGHT)
+		priv->timing.vactive.typ = LCD_MAX_HEIGHT;
+
+	priv->vl_bpix = fdtdec_get_int(blob, node, "atmel,vl-bpix", 0);
+	if (!priv->vl_bpix) {
+		debug("%s: Failed to get bits per pixel\n", __func__);
+		return -EINVAL;
+	}
+
+	priv->guard_time = fdtdec_get_int(blob, node, "atmel,guard-time", 1);
+
+	return 0;
+}
+
+static int atmel_hlcdc_bind(struct udevice *dev)
+{
+	struct video_uc_platdata *uc_plat = dev_get_uclass_platdata(dev);
+
+	uc_plat->size = LCD_MAX_WIDTH * LCD_MAX_HEIGHT *
+				(1 << LCD_MAX_LOG2_BPP) / 8;
+
+	debug("%s: Frame buffer size %x\n", __func__, uc_plat->size);
+
+	return 0;
+}
+
+static const struct udevice_id atmel_hlcdc_ids[] = {
+	{ .compatible = "atmel,sama5d2-hlcdc" },
+	{ }
+};
+
+U_BOOT_DRIVER(atmel_hlcdfb) = {
+	.name	= "atmel_hlcdfb",
+	.id	= UCLASS_VIDEO,
+	.of_match = atmel_hlcdc_ids,
+	.bind	= atmel_hlcdc_bind,
+	.probe	= atmel_hlcdc_probe,
+	.ofdata_to_platdata = atmel_hlcdc_ofdata_to_platdata,
+	.priv_auto_alloc_size = sizeof(struct atmel_hlcdc_priv),
+};
+
+#endif
diff --git a/include/configs/at91sam9n12ek.h b/include/configs/at91sam9n12ek.h
index 339f53f558..596d00bb20 100644
--- a/include/configs/at91sam9n12ek.h
+++ b/include/configs/at91sam9n12ek.h
@@ -44,7 +44,11 @@
 #define CONFIG_LCD_INFO
 #define CONFIG_LCD_INFO_BELOW_LOGO
 #define CONFIG_SYS_WHITE_ON_BLACK
+
+#ifndef CONFIG_DM_VIDEO
 #define CONFIG_ATMEL_HLCD
+#endif
+
 #define CONFIG_ATMEL_LCD_RGB565
 
 
diff --git a/include/configs/at91sam9x5ek.h b/include/configs/at91sam9x5ek.h
index 730b9807db..c0e87c7190 100644
--- a/include/configs/at91sam9x5ek.h
+++ b/include/configs/at91sam9x5ek.h
@@ -40,7 +40,11 @@
 #define CONFIG_LCD_INFO
 #define CONFIG_LCD_INFO_BELOW_LOGO
 #define CONFIG_SYS_WHITE_ON_BLACK
+
+#ifndef CONFIG_DM_VIDEO
 #define CONFIG_ATMEL_HLCD
+#endif
+
 #define CONFIG_ATMEL_LCD_RGB565
 
 
diff --git a/include/configs/ma5d4evk.h b/include/configs/ma5d4evk.h
index a5db11ce12..62ad187c8b 100644
--- a/include/configs/ma5d4evk.h
+++ b/include/configs/ma5d4evk.h
@@ -77,9 +77,13 @@
 #define CONFIG_BMP_32BPP
 #define LCD_BPP				LCD_COLOR16
 #define LCD_OUTPUT_BPP                  24
+
+#ifndef CONFIG_DM_VIDEO
 #define CONFIG_ATMEL_HLCD
 #endif
 
+#endif
+
 /*
  * SD/MMC
  */
diff --git a/include/configs/sama5d2_xplained.h b/include/configs/sama5d2_xplained.h
index ccbcc765c3..c504cc7aae 100644
--- a/include/configs/sama5d2_xplained.h
+++ b/include/configs/sama5d2_xplained.h
@@ -62,7 +62,11 @@
 #define CONFIG_LCD_INFO
 #define CONFIG_LCD_INFO_BELOW_LOGO
 #define CONFIG_SYS_WHITE_ON_BLACK
+
+#ifndef CONFIG_DM_VIDEO
 #define CONFIG_ATMEL_HLCD
+#endif
+
 #define CONFIG_ATMEL_LCD_RGB565
 #endif
 
diff --git a/include/configs/sama5d3xek.h b/include/configs/sama5d3xek.h
index 13790e7244..04ea23c461 100644
--- a/include/configs/sama5d3xek.h
+++ b/include/configs/sama5d3xek.h
@@ -40,7 +40,11 @@
 #define CONFIG_LCD_INFO
 #define CONFIG_LCD_INFO_BELOW_LOGO
 #define CONFIG_SYS_WHITE_ON_BLACK
+
+#ifndef CONFIG_DM_VIDEO
 #define CONFIG_ATMEL_HLCD
+#endif
+
 #define CONFIG_ATMEL_LCD_RGB565
 
 /* board specific (not enough SRAM) */
diff --git a/include/configs/sama5d4_xplained.h b/include/configs/sama5d4_xplained.h
index 55f10ad1f0..30f07f5331 100644
--- a/include/configs/sama5d4_xplained.h
+++ b/include/configs/sama5d4_xplained.h
@@ -92,7 +92,11 @@
 #define CONFIG_LCD_INFO
 #define CONFIG_LCD_INFO_BELOW_LOGO
 #define CONFIG_SYS_WHITE_ON_BLACK
+
+#ifndef CONFIG_DM_VIDEO
 #define CONFIG_ATMEL_HLCD
+#endif
+
 #define CONFIG_ATMEL_LCD_RGB565
 #endif
 
diff --git a/include/configs/sama5d4ek.h b/include/configs/sama5d4ek.h
index 680d5918d7..4d537b22aa 100644
--- a/include/configs/sama5d4ek.h
+++ b/include/configs/sama5d4ek.h
@@ -91,7 +91,11 @@
 #define CONFIG_LCD_INFO
 #define CONFIG_LCD_INFO_BELOW_LOGO
 #define CONFIG_SYS_WHITE_ON_BLACK
+
+#ifndef CONFIG_DM_VIDEO
 #define CONFIG_ATMEL_HLCD
+#endif
+
 #define CONFIG_ATMEL_LCD_RGB565
 
 #ifdef CONFIG_SYS_USE_SERIALFLASH
-- 
2.11.0

[U-Boot] [PATCH v2 2/2] at91: video: DT binding for HLCDC driver

[Songjun Wu]

DT binding documentation for atmel HLCDC driver.

Signed-off-by: Songjun Wu <songjun.wu@microchip.com>
---

Changes in v2: None

 doc/device-tree-bindings/video/atme-hlcdc.txt | 38 +++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)
 create mode 100644 doc/device-tree-bindings/video/atme-hlcdc.txt

diff --git a/doc/device-tree-bindings/video/atme-hlcdc.txt b/doc/device-tree-bindings/video/atme-hlcdc.txt
new file mode 100644
index 0000000000..aac0b842f0
--- /dev/null
+++ b/doc/device-tree-bindings/video/atme-hlcdc.txt
@@ -0,0 +1,38 @@
+Atmel HLCDC Framebuffer
+-----------------------------------------------------
+Required properties:
+- compatible :
+	"atmel,sama5d2-hlcdc".
+- reg: physical base address of the controller and length of memory mapped
+     region.
+- clocks: phandles to input clocks.
+- atmel,vl-bpix: Bits per pixel.
+- atmel,guard-time: lcd guard time (Delay in frame periods).
+- display-timings: please refer the displaymode.txt.
+
+Example:
+hlcdc: hlcdc at f0000000 {
+	u-boot,dm-pre-reloc;
+	compatible = "atmel,sama5d2-hlcdc";
+	reg = <0xf0000000 0x2000>;
+	clocks = <&lcdc_clk>;
+	atmel,vl-bpix = <4>;
+	atmel,guard-time = <1>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_lcd_base &pinctrl_lcd_pwm &pinctrl_lcd_rgb666>;
+	status = "okay";
+
+	display-timings {
+		480x272 {
+			clock-frequency = <9000000>;
+			hactive = <480>;
+			vactive = <272>;
+			hsync-len = <41>;
+			hfront-porch = <2>;
+			hback-porch = <2>;
+			vfront-porch = <2>;
+			vback-porch = <2>;
+			vsync-len = <11>;
+		};
+	};
+};
\ No newline at end of file
-- 
2.11.0

[U-Boot] [PATCH v2 1/2] at91: video: Support driver-model for the HLCD driver

[Marek Vasut]

On 03/01/2017 10:25 AM, Songjun Wu wrote:
> Add driver-model support to this driver.
> 
> Signed-off-by: Songjun Wu <songjun.wu@microchip.com>
> ---
> 
> Changes in v2:
> - Due to the peripheral clock driver improvement, remove
>   the unneccessary clock calling.
> 
>  board/atmel/at91sam9n12ek/at91sam9n12ek.c       |   2 +
>  board/atmel/at91sam9x5ek/at91sam9x5ek.c         |   2 +
>  board/atmel/sama5d2_xplained/sama5d2_xplained.c |   2 +
>  board/atmel/sama5d3xek/sama5d3xek.c             |   2 +
>  board/atmel/sama5d4_xplained/sama5d4_xplained.c |   2 +
>  board/atmel/sama5d4ek/sama5d4ek.c               |   2 +
>  drivers/video/Kconfig                           |   9 +
>  drivers/video/atmel_hlcdfb.c                    | 314 +++++++++++++++++++++++-

Split the driver from board changes.

>  include/configs/at91sam9n12ek.h                 |   4 +
>  include/configs/at91sam9x5ek.h                  |   4 +
>  include/configs/ma5d4evk.h                      |   4 +
>  include/configs/sama5d2_xplained.h              |   4 +
>  include/configs/sama5d3xek.h                    |   4 +
>  include/configs/sama5d4_xplained.h              |   4 +
>  include/configs/sama5d4ek.h                     |   4 +
>  15 files changed, 360 insertions(+), 3 deletions(-)

[...]

> diff --git a/drivers/video/atmel_hlcdfb.c b/drivers/video/atmel_hlcdfb.c
> index 960b474b76..0bcb92b2cf 100644
> --- a/drivers/video/atmel_hlcdfb.c
> +++ b/drivers/video/atmel_hlcdfb.c
> @@ -10,13 +10,24 @@
>  #include <asm/io.h>
>  #include <asm/arch/gpio.h>
>  #include <asm/arch/clk.h>
> +#include <clk.h>
> +#include <dm.h>
> +#include <fdtdec.h>
>  #include <lcd.h>
> +#include <video.h>
>  #include <atmel_hlcdc.h>
>  
>  #if defined(CONFIG_LCD_LOGO)
>  #include <bmp_logo.h>
>  #endif
>  
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +#define lcdc_readl(reg)		__raw_readl((reg))
> +#define lcdc_writel(reg, val)	__raw_writel((val), (reg))

Really ? Do we REALLY need more new accessors ? Just use readl/writel ...

> +#ifndef CONFIG_DM_VIDEO
> +
>  /* configurable parameters */
>  #define ATMEL_LCDC_CVAL_DEFAULT		0xc8
>  #define ATMEL_LCDC_DMA_BURST_LEN	8
> @@ -26,9 +37,6 @@
>  
>  #define ATMEL_LCDC_FIFO_SIZE		512
>  
> -#define lcdc_readl(reg)		__raw_readl((reg))
> -#define lcdc_writel(reg, val)	__raw_writel((val), (reg))
> -
>  /*
>   * the CLUT register map as following
>   * RCLUT(24 ~ 16), GCLUT(15 ~ 8), BCLUT(7 ~ 0)
> @@ -218,3 +226,303 @@ void lcd_ctrl_init(void *lcdbase)
>  	/* Enable flushing if we enabled dcache */
>  	lcd_set_flush_dcache(1);
>  }
> +
> +#else
> +
> +enum {
> +	LCD_MAX_WIDTH		= 1024,
> +	LCD_MAX_HEIGHT		= 768,
> +	LCD_MAX_LOG2_BPP	= VIDEO_BPP16,
> +};
> +
> +struct atmel_hlcdc_priv {
> +	struct atmel_hlcd_regs *regs;
> +	struct display_timing timing;
> +	unsigned int vl_bpix;
> +	unsigned int guard_time;
> +	ulong clk_rate;
> +};
> +
> +static int at91_hlcdc_enable_clk(struct udevice *dev)
> +{
> +	struct atmel_hlcdc_priv *priv = dev_get_priv(dev);
> +	struct clk clk;
> +	ulong clk_rate;
> +	int ret;
> +
> +	ret = clk_get_by_index(dev, 0, &clk);
> +	if (ret)
> +		return -EINVAL;
> +
> +	ret = clk_enable(&clk);
> +	if (ret)
> +		return ret;
> +
> +	clk_rate = clk_get_rate(&clk);
> +	if (!clk_rate)
> +		return -ENODEV;

Clock are still enabled if you fail here ...

> +	priv->clk_rate = clk_rate;
> +
> +	clk_free(&clk);
> +
> +	return 0;
> +}
> +
> +static void atmel_hlcdc_init(struct udevice *dev)
> +{
> +	struct video_uc_platdata *uc_plat = dev_get_uclass_platdata(dev);
> +	struct atmel_hlcdc_priv *priv = dev_get_priv(dev);
> +	struct atmel_hlcd_regs *regs = priv->regs;
> +	struct display_timing *timing = &priv->timing;
> +	struct lcd_dma_desc *desc;
> +	unsigned long value, vl_clk_pol;
> +
> +	/* Disable DISP signal */
> +	lcdc_writel(&regs->lcdc_lcddis, LCDC_LCDDIS_DISPDIS);
> +	while ((lcdc_readl(&regs->lcdc_lcdsr) & LCDC_LCDSR_DISPSTS))
> +		udelay(1);

wait_for_bit(), fix globally ... and don't use unbounded loops ...

> +	/* Disable synchronization */
> +	lcdc_writel(&regs->lcdc_lcddis, LCDC_LCDDIS_SYNCDIS);
> +	while ((lcdc_readl(&regs->lcdc_lcdsr) & LCDC_LCDSR_LCDSTS))
> +		udelay(1);
> +	/* Disable pixel clock */
> +	lcdc_writel(&regs->lcdc_lcddis, LCDC_LCDDIS_CLKDIS);
> +	while ((lcdc_readl(&regs->lcdc_lcdsr) & LCDC_LCDSR_CLKSTS))
> +		udelay(1);
> +	/* Disable PWM */
> +	lcdc_writel(&regs->lcdc_lcddis, LCDC_LCDDIS_PWMDIS);
> +	while ((lcdc_readl(&regs->lcdc_lcdsr) & LCDC_LCDSR_PWMSTS))
> +		udelay(1);
> +
> +	/* Set pixel clock */
> +	value = priv->clk_rate / timing->pixelclock.typ;
> +	if (priv->clk_rate % timing->pixelclock.typ)
> +		value++;
> +
> +	vl_clk_pol = 0;
> +	if (timing->flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
> +		vl_clk_pol = LCDC_LCDCFG0_CLKPOL;
> +
> +	if (value < 1) {

I guess I'd just introduce a variable and ORR it with either CLKSEL or
whatever bits based on the value to reduce this lengthy condition here.
ie u32 foo = LCDC_BAR | LCDC_BAZ;
if (value < 1)
  foo |= LCDC_QUUX;
else
  ...

writel(...);

> +		/* Using system clock as pixel clock */
> +		lcdc_writel(&regs->lcdc_lcdcfg0,
> +			    LCDC_LCDCFG0_CLKDIV(0)
> +			    | LCDC_LCDCFG0_CGDISHCR
> +			    | LCDC_LCDCFG0_CGDISHEO
> +			    | LCDC_LCDCFG0_CGDISOVR1
> +			    | LCDC_LCDCFG0_CGDISBASE
> +			    | vl_clk_pol
> +			    | LCDC_LCDCFG0_CLKSEL);
> +
> +	} else {
> +		lcdc_writel(&regs->lcdc_lcdcfg0,
> +			    LCDC_LCDCFG0_CLKDIV(value - 2)
> +			    | LCDC_LCDCFG0_CGDISHCR
> +			    | LCDC_LCDCFG0_CGDISHEO
> +			    | LCDC_LCDCFG0_CGDISOVR1
> +			    | LCDC_LCDCFG0_CGDISBASE
> +			    | vl_clk_pol);
> +	}
> +
> +	/* Initialize control register 5 */
> +	value = 0;
> +
> +	if (!(timing->flags & DISPLAY_FLAGS_HSYNC_HIGH))
> +		value |= LCDC_LCDCFG5_HSPOL;
> +	if (!(timing->flags & DISPLAY_FLAGS_VSYNC_HIGH))
> +		value |= LCDC_LCDCFG5_VSPOL;
> +
> +#ifndef LCD_OUTPUT_BPP

Just make sure that's always defined . If possible, get this from DT.

> +	/* Output is 24bpp */
> +	value |= LCDC_LCDCFG5_MODE_OUTPUT_24BPP;
> +#else
> +	switch (LCD_OUTPUT_BPP) {
> +	case 12:
> +		value |= LCDC_LCDCFG5_MODE_OUTPUT_12BPP;
> +		break;
> +	case 16:
> +		value |= LCDC_LCDCFG5_MODE_OUTPUT_16BPP;
> +		break;
> +	case 18:
> +		value |= LCDC_LCDCFG5_MODE_OUTPUT_18BPP;
> +		break;
> +	case 24:
> +		value |= LCDC_LCDCFG5_MODE_OUTPUT_24BPP;
> +		break;
> +	default:
> +		BUG();
> +		break;
> +	}
> +#endif
> +
> +	value |= LCDC_LCDCFG5_GUARDTIME(priv->guard_time);
> +	value |= (LCDC_LCDCFG5_DISPDLY | LCDC_LCDCFG5_VSPDLYS);
> +	lcdc_writel(&regs->lcdc_lcdcfg5, value);
> +
> +	/* Vertical & Horizontal Timing */
> +	value = LCDC_LCDCFG1_VSPW(timing->vsync_len.typ - 1);
> +	value |= LCDC_LCDCFG1_HSPW(timing->hsync_len.typ - 1);
> +	lcdc_writel(&regs->lcdc_lcdcfg1, value);
> +
> +	value = LCDC_LCDCFG2_VBPW(timing->vback_porch.typ);
> +	value |= LCDC_LCDCFG2_VFPW(timing->vfront_porch.typ - 1);
> +	lcdc_writel(&regs->lcdc_lcdcfg2, value);
> +
> +	value = LCDC_LCDCFG3_HBPW(timing->hback_porch.typ - 1);
> +	value |= LCDC_LCDCFG3_HFPW(timing->hfront_porch.typ - 1);
> +	lcdc_writel(&regs->lcdc_lcdcfg3, value);
> +
> +	/* Display size */
> +	value = LCDC_LCDCFG4_RPF(timing->vactive.typ - 1);
> +	value |= LCDC_LCDCFG4_PPL(timing->hactive.typ - 1);
> +	lcdc_writel(&regs->lcdc_lcdcfg4, value);
> +
> +	lcdc_writel(&regs->lcdc_basecfg0,
> +		    LCDC_BASECFG0_BLEN_AHB_INCR4 | LCDC_BASECFG0_DLBO);
> +
> +	switch (VNBITS(priv->vl_bpix)) {
> +	case 16:
> +		lcdc_writel(&regs->lcdc_basecfg1,
> +			    LCDC_BASECFG1_RGBMODE_16BPP_RGB_565);
> +		break;
> +	case 32:
> +		lcdc_writel(&regs->lcdc_basecfg1,
> +			    LCDC_BASECFG1_RGBMODE_24BPP_RGB_888);
> +		break;
> +	default:
> +		BUG();
> +		break;
> +	}
> +
> +	lcdc_writel(&regs->lcdc_basecfg2, LCDC_BASECFG2_XSTRIDE(0));
> +	lcdc_writel(&regs->lcdc_basecfg3, 0);
> +	lcdc_writel(&regs->lcdc_basecfg4, LCDC_BASECFG4_DMA);
> +
> +	/* Disable all interrupts */
> +	lcdc_writel(&regs->lcdc_lcdidr, ~0UL);
> +	lcdc_writel(&regs->lcdc_baseidr, ~0UL);
> +
> +	/* Setup the DMA descriptor, this descriptor will loop to itself */
> +	desc = (struct lcd_dma_desc *)(uc_plat->base - 16);
> +
> +	desc->address = (u32)uc_plat->base;
> +
> +	/* Disable DMA transfer interrupt & descriptor loaded interrupt. */
> +	desc->control = LCDC_BASECTRL_ADDIEN | LCDC_BASECTRL_DSCRIEN
> +			| LCDC_BASECTRL_DMAIEN | LCDC_BASECTRL_DFETCH;
> +	desc->next = (u32)desc;
> +
> +	/* Flush the DMA descriptor if we enabled dcache */
> +	flush_dcache_range((u32)desc, (u32)desc + sizeof(*desc));
> +
> +	lcdc_writel(&regs->lcdc_baseaddr, desc->address);
> +	lcdc_writel(&regs->lcdc_basectrl, desc->control);
> +	lcdc_writel(&regs->lcdc_basenext, desc->next);
> +	lcdc_writel(&regs->lcdc_basecher,
> +		    LCDC_BASECHER_CHEN | LCDC_BASECHER_UPDATEEN);
> +
> +	/* Enable LCD */
> +	value = lcdc_readl(&regs->lcdc_lcden);
> +	lcdc_writel(&regs->lcdc_lcden, value | LCDC_LCDEN_CLKEN);
> +	while (!(lcdc_readl(&regs->lcdc_lcdsr) & LCDC_LCDSR_CLKSTS))
> +		udelay(1);

wait_for_bit() ...

> +	value = lcdc_readl(&regs->lcdc_lcden);
> +	lcdc_writel(&regs->lcdc_lcden, value | LCDC_LCDEN_SYNCEN);
> +	while (!(lcdc_readl(&regs->lcdc_lcdsr) & LCDC_LCDSR_LCDSTS))
> +		udelay(1);
> +	value = lcdc_readl(&regs->lcdc_lcden);
> +	lcdc_writel(&regs->lcdc_lcden, value | LCDC_LCDEN_DISPEN);
> +	while (!(lcdc_readl(&regs->lcdc_lcdsr) & LCDC_LCDSR_DISPSTS))
> +		udelay(1);
> +	value = lcdc_readl(&regs->lcdc_lcden);
> +	lcdc_writel(&regs->lcdc_lcden, value | LCDC_LCDEN_PWMEN);
> +	while (!(lcdc_readl(&regs->lcdc_lcdsr) & LCDC_LCDSR_PWMSTS))
> +		udelay(1);
> +}
> +
> +static int atmel_hlcdc_probe(struct udevice *dev)
> +{
> +	struct video_priv *uc_priv = dev_get_uclass_priv(dev);
> +	struct atmel_hlcdc_priv *priv = dev_get_priv(dev);
> +	int ret;
> +
> +	ret = at91_hlcdc_enable_clk(dev);
> +	if (ret)
> +		return ret;
> +
> +	atmel_hlcdc_init(dev);
> +
> +	uc_priv->xsize = priv->timing.hactive.typ;
> +	uc_priv->ysize = priv->timing.vactive.typ;
> +	uc_priv->bpix = priv->vl_bpix;
> +
> +	/* Enable flushing if we enabled dcache */
> +	video_set_flush_dcache(dev, true);
> +
> +	return 0;
> +}
> +
> +static int atmel_hlcdc_ofdata_to_platdata(struct udevice *dev)
> +{
> +	struct atmel_hlcdc_priv *priv = dev_get_priv(dev);
> +	const void *blob = gd->fdt_blob;
> +	int node = dev->of_offset;
> +
> +	priv->regs = (struct atmel_hlcd_regs *)dev_get_addr(dev);
> +	if (!priv->regs) {
> +		debug("%s: No display controller address\n", __func__);
> +		return -EINVAL;
> +	}
> +
> +	if (fdtdec_decode_display_timing(blob, dev->of_offset,
> +					 0, &priv->timing)) {
> +		debug("%s: Failed to decode display timing\n", __func__);
> +		return -EINVAL;
> +	}
> +
> +	if (priv->timing.hactive.typ > LCD_MAX_WIDTH)
> +		priv->timing.hactive.typ = LCD_MAX_WIDTH;
> +
> +	if (priv->timing.vactive.typ > LCD_MAX_HEIGHT)
> +		priv->timing.vactive.typ = LCD_MAX_HEIGHT;
> +
> +	priv->vl_bpix = fdtdec_get_int(blob, node, "atmel,vl-bpix", 0);
> +	if (!priv->vl_bpix) {
> +		debug("%s: Failed to get bits per pixel\n", __func__);
> +		return -EINVAL;
> +	}
> +
> +	priv->guard_time = fdtdec_get_int(blob, node, "atmel,guard-time", 1);
> +
> +	return 0;
> +}
> +
> +static int atmel_hlcdc_bind(struct udevice *dev)
> +{
> +	struct video_uc_platdata *uc_plat = dev_get_uclass_platdata(dev);
> +
> +	uc_plat->size = LCD_MAX_WIDTH * LCD_MAX_HEIGHT *
> +				(1 << LCD_MAX_LOG2_BPP) / 8;
> +
> +	debug("%s: Frame buffer size %x\n", __func__, uc_plat->size);
> +
> +	return 0;
> +}
> +
> +static const struct udevice_id atmel_hlcdc_ids[] = {
> +	{ .compatible = "atmel,sama5d2-hlcdc" },
> +	{ }
> +};
> +
> +U_BOOT_DRIVER(atmel_hlcdfb) = {
> +	.name	= "atmel_hlcdfb",
> +	.id	= UCLASS_VIDEO,
> +	.of_match = atmel_hlcdc_ids,
> +	.bind	= atmel_hlcdc_bind,
> +	.probe	= atmel_hlcdc_probe,
> +	.ofdata_to_platdata = atmel_hlcdc_ofdata_to_platdata,
> +	.priv_auto_alloc_size = sizeof(struct atmel_hlcdc_priv),
> +};
> +
> +#endif

[...]

> diff --git a/include/configs/sama5d4ek.h b/include/configs/sama5d4ek.h
> index 680d5918d7..4d537b22aa 100644
> --- a/include/configs/sama5d4ek.h
> +++ b/include/configs/sama5d4ek.h
> @@ -91,7 +91,11 @@
>  #define CONFIG_LCD_INFO
>  #define CONFIG_LCD_INFO_BELOW_LOGO
>  #define CONFIG_SYS_WHITE_ON_BLACK
> +
> +#ifndef CONFIG_DM_VIDEO
>  #define CONFIG_ATMEL_HLCD

This should use Kconfig ...

> +#endif
> +
>  #define CONFIG_ATMEL_LCD_RGB565
>  
>  #ifdef CONFIG_SYS_USE_SERIALFLASH
> 


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v2 1/2] at91: video: Support driver-model for the HLCD driver

[Wu, Songjun]

Hi Marek,

Thank you for your comments.
I will fix these issues in next patch.

On 3/13/2017 13:05, Marek Vasut wrote:
> On 03/01/2017 10:25 AM, Songjun Wu wrote:
>> Add driver-model support to this driver.
>>
>> Signed-off-by: Songjun Wu <songjun.wu@microchip.com>
>> ---
>>
>> Changes in v2:
>> - Due to the peripheral clock driver improvement, remove
>>   the unneccessary clock calling.
>>
>>  board/atmel/at91sam9n12ek/at91sam9n12ek.c       |   2 +
>>  board/atmel/at91sam9x5ek/at91sam9x5ek.c         |   2 +
>>  board/atmel/sama5d2_xplained/sama5d2_xplained.c |   2 +
>>  board/atmel/sama5d3xek/sama5d3xek.c             |   2 +
>>  board/atmel/sama5d4_xplained/sama5d4_xplained.c |   2 +
>>  board/atmel/sama5d4ek/sama5d4ek.c               |   2 +
>>  drivers/video/Kconfig                           |   9 +
>>  drivers/video/atmel_hlcdfb.c                    | 314 +++++++++++++++++++++++-
>
> Split the driver from board changes.
>
>>  include/configs/at91sam9n12ek.h                 |   4 +
>>  include/configs/at91sam9x5ek.h                  |   4 +
>>  include/configs/ma5d4evk.h                      |   4 +
>>  include/configs/sama5d2_xplained.h              |   4 +
>>  include/configs/sama5d3xek.h                    |   4 +
>>  include/configs/sama5d4_xplained.h              |   4 +
>>  include/configs/sama5d4ek.h                     |   4 +
>>  15 files changed, 360 insertions(+), 3 deletions(-)
>
> [...]
>
>> diff --git a/drivers/video/atmel_hlcdfb.c b/drivers/video/atmel_hlcdfb.c
>> index 960b474b76..0bcb92b2cf 100644
>> --- a/drivers/video/atmel_hlcdfb.c
>> +++ b/drivers/video/atmel_hlcdfb.c
>> @@ -10,13 +10,24 @@
>>  #include <asm/io.h>
>>  #include <asm/arch/gpio.h>
>>  #include <asm/arch/clk.h>
>> +#include <clk.h>
>> +#include <dm.h>
>> +#include <fdtdec.h>
>>  #include <lcd.h>
>> +#include <video.h>
>>  #include <atmel_hlcdc.h>
>>
>>  #if defined(CONFIG_LCD_LOGO)
>>  #include <bmp_logo.h>
>>  #endif
>>
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> +#define lcdc_readl(reg)		__raw_readl((reg))
>> +#define lcdc_writel(reg, val)	__raw_writel((val), (reg))
>
> Really ? Do we REALLY need more new accessors ? Just use readl/writel ...
>
>> +#ifndef CONFIG_DM_VIDEO
>> +
>>  /* configurable parameters */
>>  #define ATMEL_LCDC_CVAL_DEFAULT		0xc8
>>  #define ATMEL_LCDC_DMA_BURST_LEN	8
>> @@ -26,9 +37,6 @@
>>
>>  #define ATMEL_LCDC_FIFO_SIZE		512
>>
>> -#define lcdc_readl(reg)		__raw_readl((reg))
>> -#define lcdc_writel(reg, val)	__raw_writel((val), (reg))
>> -
>>  /*
>>   * the CLUT register map as following
>>   * RCLUT(24 ~ 16), GCLUT(15 ~ 8), BCLUT(7 ~ 0)
>> @@ -218,3 +226,303 @@ void lcd_ctrl_init(void *lcdbase)
>>  	/* Enable flushing if we enabled dcache */
>>  	lcd_set_flush_dcache(1);
>>  }
>> +
>> +#else
>> +
>> +enum {
>> +	LCD_MAX_WIDTH		= 1024,
>> +	LCD_MAX_HEIGHT		= 768,
>> +	LCD_MAX_LOG2_BPP	= VIDEO_BPP16,
>> +};
>> +
>> +struct atmel_hlcdc_priv {
>> +	struct atmel_hlcd_regs *regs;
>> +	struct display_timing timing;
>> +	unsigned int vl_bpix;
>> +	unsigned int guard_time;
>> +	ulong clk_rate;
>> +};
>> +
>> +static int at91_hlcdc_enable_clk(struct udevice *dev)
>> +{
>> +	struct atmel_hlcdc_priv *priv = dev_get_priv(dev);
>> +	struct clk clk;
>> +	ulong clk_rate;
>> +	int ret;
>> +
>> +	ret = clk_get_by_index(dev, 0, &clk);
>> +	if (ret)
>> +		return -EINVAL;
>> +
>> +	ret = clk_enable(&clk);
>> +	if (ret)
>> +		return ret;
>> +
>> +	clk_rate = clk_get_rate(&clk);
>> +	if (!clk_rate)
>> +		return -ENODEV;
>
> Clock are still enabled if you fail here ...
>
>> +	priv->clk_rate = clk_rate;
>> +
>> +	clk_free(&clk);
>> +
>> +	return 0;
>> +}
>> +
>> +static void atmel_hlcdc_init(struct udevice *dev)
>> +{
>> +	struct video_uc_platdata *uc_plat = dev_get_uclass_platdata(dev);
>> +	struct atmel_hlcdc_priv *priv = dev_get_priv(dev);
>> +	struct atmel_hlcd_regs *regs = priv->regs;
>> +	struct display_timing *timing = &priv->timing;
>> +	struct lcd_dma_desc *desc;
>> +	unsigned long value, vl_clk_pol;
>> +
>> +	/* Disable DISP signal */
>> +	lcdc_writel(&regs->lcdc_lcddis, LCDC_LCDDIS_DISPDIS);
>> +	while ((lcdc_readl(&regs->lcdc_lcdsr) & LCDC_LCDSR_DISPSTS))
>> +		udelay(1);
>
> wait_for_bit(), fix globally ... and don't use unbounded loops ...
>
>> +	/* Disable synchronization */
>> +	lcdc_writel(&regs->lcdc_lcddis, LCDC_LCDDIS_SYNCDIS);
>> +	while ((lcdc_readl(&regs->lcdc_lcdsr) & LCDC_LCDSR_LCDSTS))
>> +		udelay(1);
>> +	/* Disable pixel clock */
>> +	lcdc_writel(&regs->lcdc_lcddis, LCDC_LCDDIS_CLKDIS);
>> +	while ((lcdc_readl(&regs->lcdc_lcdsr) & LCDC_LCDSR_CLKSTS))
>> +		udelay(1);
>> +	/* Disable PWM */
>> +	lcdc_writel(&regs->lcdc_lcddis, LCDC_LCDDIS_PWMDIS);
>> +	while ((lcdc_readl(&regs->lcdc_lcdsr) & LCDC_LCDSR_PWMSTS))
>> +		udelay(1);
>> +
>> +	/* Set pixel clock */
>> +	value = priv->clk_rate / timing->pixelclock.typ;
>> +	if (priv->clk_rate % timing->pixelclock.typ)
>> +		value++;
>> +
>> +	vl_clk_pol = 0;
>> +	if (timing->flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
>> +		vl_clk_pol = LCDC_LCDCFG0_CLKPOL;
>> +
>> +	if (value < 1) {
>
> I guess I'd just introduce a variable and ORR it with either CLKSEL or
> whatever bits based on the value to reduce this lengthy condition here.
> ie u32 foo = LCDC_BAR | LCDC_BAZ;
> if (value < 1)
>   foo |= LCDC_QUUX;
> else
>   ...
>
> writel(...);
>
>> +		/* Using system clock as pixel clock */
>> +		lcdc_writel(&regs->lcdc_lcdcfg0,
>> +			    LCDC_LCDCFG0_CLKDIV(0)
>> +			    | LCDC_LCDCFG0_CGDISHCR
>> +			    | LCDC_LCDCFG0_CGDISHEO
>> +			    | LCDC_LCDCFG0_CGDISOVR1
>> +			    | LCDC_LCDCFG0_CGDISBASE
>> +			    | vl_clk_pol
>> +			    | LCDC_LCDCFG0_CLKSEL);
>> +
>> +	} else {
>> +		lcdc_writel(&regs->lcdc_lcdcfg0,
>> +			    LCDC_LCDCFG0_CLKDIV(value - 2)
>> +			    | LCDC_LCDCFG0_CGDISHCR
>> +			    | LCDC_LCDCFG0_CGDISHEO
>> +			    | LCDC_LCDCFG0_CGDISOVR1
>> +			    | LCDC_LCDCFG0_CGDISBASE
>> +			    | vl_clk_pol);
>> +	}
>> +
>> +	/* Initialize control register 5 */
>> +	value = 0;
>> +
>> +	if (!(timing->flags & DISPLAY_FLAGS_HSYNC_HIGH))
>> +		value |= LCDC_LCDCFG5_HSPOL;
>> +	if (!(timing->flags & DISPLAY_FLAGS_VSYNC_HIGH))
>> +		value |= LCDC_LCDCFG5_VSPOL;
>> +
>> +#ifndef LCD_OUTPUT_BPP
>
> Just make sure that's always defined . If possible, get this from DT.
>
>> +	/* Output is 24bpp */
>> +	value |= LCDC_LCDCFG5_MODE_OUTPUT_24BPP;
>> +#else
>> +	switch (LCD_OUTPUT_BPP) {
>> +	case 12:
>> +		value |= LCDC_LCDCFG5_MODE_OUTPUT_12BPP;
>> +		break;
>> +	case 16:
>> +		value |= LCDC_LCDCFG5_MODE_OUTPUT_16BPP;
>> +		break;
>> +	case 18:
>> +		value |= LCDC_LCDCFG5_MODE_OUTPUT_18BPP;
>> +		break;
>> +	case 24:
>> +		value |= LCDC_LCDCFG5_MODE_OUTPUT_24BPP;
>> +		break;
>> +	default:
>> +		BUG();
>> +		break;
>> +	}
>> +#endif
>> +
>> +	value |= LCDC_LCDCFG5_GUARDTIME(priv->guard_time);
>> +	value |= (LCDC_LCDCFG5_DISPDLY | LCDC_LCDCFG5_VSPDLYS);
>> +	lcdc_writel(&regs->lcdc_lcdcfg5, value);
>> +
>> +	/* Vertical & Horizontal Timing */
>> +	value = LCDC_LCDCFG1_VSPW(timing->vsync_len.typ - 1);
>> +	value |= LCDC_LCDCFG1_HSPW(timing->hsync_len.typ - 1);
>> +	lcdc_writel(&regs->lcdc_lcdcfg1, value);
>> +
>> +	value = LCDC_LCDCFG2_VBPW(timing->vback_porch.typ);
>> +	value |= LCDC_LCDCFG2_VFPW(timing->vfront_porch.typ - 1);
>> +	lcdc_writel(&regs->lcdc_lcdcfg2, value);
>> +
>> +	value = LCDC_LCDCFG3_HBPW(timing->hback_porch.typ - 1);
>> +	value |= LCDC_LCDCFG3_HFPW(timing->hfront_porch.typ - 1);
>> +	lcdc_writel(&regs->lcdc_lcdcfg3, value);
>> +
>> +	/* Display size */
>> +	value = LCDC_LCDCFG4_RPF(timing->vactive.typ - 1);
>> +	value |= LCDC_LCDCFG4_PPL(timing->hactive.typ - 1);
>> +	lcdc_writel(&regs->lcdc_lcdcfg4, value);
>> +
>> +	lcdc_writel(&regs->lcdc_basecfg0,
>> +		    LCDC_BASECFG0_BLEN_AHB_INCR4 | LCDC_BASECFG0_DLBO);
>> +
>> +	switch (VNBITS(priv->vl_bpix)) {
>> +	case 16:
>> +		lcdc_writel(&regs->lcdc_basecfg1,
>> +			    LCDC_BASECFG1_RGBMODE_16BPP_RGB_565);
>> +		break;
>> +	case 32:
>> +		lcdc_writel(&regs->lcdc_basecfg1,
>> +			    LCDC_BASECFG1_RGBMODE_24BPP_RGB_888);
>> +		break;
>> +	default:
>> +		BUG();
>> +		break;
>> +	}
>> +
>> +	lcdc_writel(&regs->lcdc_basecfg2, LCDC_BASECFG2_XSTRIDE(0));
>> +	lcdc_writel(&regs->lcdc_basecfg3, 0);
>> +	lcdc_writel(&regs->lcdc_basecfg4, LCDC_BASECFG4_DMA);
>> +
>> +	/* Disable all interrupts */
>> +	lcdc_writel(&regs->lcdc_lcdidr, ~0UL);
>> +	lcdc_writel(&regs->lcdc_baseidr, ~0UL);
>> +
>> +	/* Setup the DMA descriptor, this descriptor will loop to itself */
>> +	desc = (struct lcd_dma_desc *)(uc_plat->base - 16);
>> +
>> +	desc->address = (u32)uc_plat->base;
>> +
>> +	/* Disable DMA transfer interrupt & descriptor loaded interrupt. */
>> +	desc->control = LCDC_BASECTRL_ADDIEN | LCDC_BASECTRL_DSCRIEN
>> +			| LCDC_BASECTRL_DMAIEN | LCDC_BASECTRL_DFETCH;
>> +	desc->next = (u32)desc;
>> +
>> +	/* Flush the DMA descriptor if we enabled dcache */
>> +	flush_dcache_range((u32)desc, (u32)desc + sizeof(*desc));
>> +
>> +	lcdc_writel(&regs->lcdc_baseaddr, desc->address);
>> +	lcdc_writel(&regs->lcdc_basectrl, desc->control);
>> +	lcdc_writel(&regs->lcdc_basenext, desc->next);
>> +	lcdc_writel(&regs->lcdc_basecher,
>> +		    LCDC_BASECHER_CHEN | LCDC_BASECHER_UPDATEEN);
>> +
>> +	/* Enable LCD */
>> +	value = lcdc_readl(&regs->lcdc_lcden);
>> +	lcdc_writel(&regs->lcdc_lcden, value | LCDC_LCDEN_CLKEN);
>> +	while (!(lcdc_readl(&regs->lcdc_lcdsr) & LCDC_LCDSR_CLKSTS))
>> +		udelay(1);
>
> wait_for_bit() ...
>
>> +	value = lcdc_readl(&regs->lcdc_lcden);
>> +	lcdc_writel(&regs->lcdc_lcden, value | LCDC_LCDEN_SYNCEN);
>> +	while (!(lcdc_readl(&regs->lcdc_lcdsr) & LCDC_LCDSR_LCDSTS))
>> +		udelay(1);
>> +	value = lcdc_readl(&regs->lcdc_lcden);
>> +	lcdc_writel(&regs->lcdc_lcden, value | LCDC_LCDEN_DISPEN);
>> +	while (!(lcdc_readl(&regs->lcdc_lcdsr) & LCDC_LCDSR_DISPSTS))
>> +		udelay(1);
>> +	value = lcdc_readl(&regs->lcdc_lcden);
>> +	lcdc_writel(&regs->lcdc_lcden, value | LCDC_LCDEN_PWMEN);
>> +	while (!(lcdc_readl(&regs->lcdc_lcdsr) & LCDC_LCDSR_PWMSTS))
>> +		udelay(1);
>> +}
>> +
>> +static int atmel_hlcdc_probe(struct udevice *dev)
>> +{
>> +	struct video_priv *uc_priv = dev_get_uclass_priv(dev);
>> +	struct atmel_hlcdc_priv *priv = dev_get_priv(dev);
>> +	int ret;
>> +
>> +	ret = at91_hlcdc_enable_clk(dev);
>> +	if (ret)
>> +		return ret;
>> +
>> +	atmel_hlcdc_init(dev);
>> +
>> +	uc_priv->xsize = priv->timing.hactive.typ;
>> +	uc_priv->ysize = priv->timing.vactive.typ;
>> +	uc_priv->bpix = priv->vl_bpix;
>> +
>> +	/* Enable flushing if we enabled dcache */
>> +	video_set_flush_dcache(dev, true);
>> +
>> +	return 0;
>> +}
>> +
>> +static int atmel_hlcdc_ofdata_to_platdata(struct udevice *dev)
>> +{
>> +	struct atmel_hlcdc_priv *priv = dev_get_priv(dev);
>> +	const void *blob = gd->fdt_blob;
>> +	int node = dev->of_offset;
>> +
>> +	priv->regs = (struct atmel_hlcd_regs *)dev_get_addr(dev);
>> +	if (!priv->regs) {
>> +		debug("%s: No display controller address\n", __func__);
>> +		return -EINVAL;
>> +	}
>> +
>> +	if (fdtdec_decode_display_timing(blob, dev->of_offset,
>> +					 0, &priv->timing)) {
>> +		debug("%s: Failed to decode display timing\n", __func__);
>> +		return -EINVAL;
>> +	}
>> +
>> +	if (priv->timing.hactive.typ > LCD_MAX_WIDTH)
>> +		priv->timing.hactive.typ = LCD_MAX_WIDTH;
>> +
>> +	if (priv->timing.vactive.typ > LCD_MAX_HEIGHT)
>> +		priv->timing.vactive.typ = LCD_MAX_HEIGHT;
>> +
>> +	priv->vl_bpix = fdtdec_get_int(blob, node, "atmel,vl-bpix", 0);
>> +	if (!priv->vl_bpix) {
>> +		debug("%s: Failed to get bits per pixel\n", __func__);
>> +		return -EINVAL;
>> +	}
>> +
>> +	priv->guard_time = fdtdec_get_int(blob, node, "atmel,guard-time", 1);
>> +
>> +	return 0;
>> +}
>> +
>> +static int atmel_hlcdc_bind(struct udevice *dev)
>> +{
>> +	struct video_uc_platdata *uc_plat = dev_get_uclass_platdata(dev);
>> +
>> +	uc_plat->size = LCD_MAX_WIDTH * LCD_MAX_HEIGHT *
>> +				(1 << LCD_MAX_LOG2_BPP) / 8;
>> +
>> +	debug("%s: Frame buffer size %x\n", __func__, uc_plat->size);
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct udevice_id atmel_hlcdc_ids[] = {
>> +	{ .compatible = "atmel,sama5d2-hlcdc" },
>> +	{ }
>> +};
>> +
>> +U_BOOT_DRIVER(atmel_hlcdfb) = {
>> +	.name	= "atmel_hlcdfb",
>> +	.id	= UCLASS_VIDEO,
>> +	.of_match = atmel_hlcdc_ids,
>> +	.bind	= atmel_hlcdc_bind,
>> +	.probe	= atmel_hlcdc_probe,
>> +	.ofdata_to_platdata = atmel_hlcdc_ofdata_to_platdata,
>> +	.priv_auto_alloc_size = sizeof(struct atmel_hlcdc_priv),
>> +};
>> +
>> +#endif
>
> [...]
>
>> diff --git a/include/configs/sama5d4ek.h b/include/configs/sama5d4ek.h
>> index 680d5918d7..4d537b22aa 100644
>> --- a/include/configs/sama5d4ek.h
>> +++ b/include/configs/sama5d4ek.h
>> @@ -91,7 +91,11 @@
>>  #define CONFIG_LCD_INFO
>>  #define CONFIG_LCD_INFO_BELOW_LOGO
>>  #define CONFIG_SYS_WHITE_ON_BLACK
>> +
>> +#ifndef CONFIG_DM_VIDEO
>>  #define CONFIG_ATMEL_HLCD
>
> This should use Kconfig ...
>
>> +#endif
>> +
>>  #define CONFIG_ATMEL_LCD_RGB565
>>
>>  #ifdef CONFIG_SYS_USE_SERIALFLASH
>>
>
>