[PATCH 0/5] sunxi: video: Add V3S simple-framebuffer

[PATCH 0/5] sunxi: video: Add V3S simple-framebuffer

[Martin Cerveny]

There are patches to enable "allwinner,simple-framebuffer"
with "mixer0-lcd0" pipeline for V3s.

First patch corrects "2x pll" loop for pixelclock search.
Second patch enable PWM0 (for backlight support).
Third patch is the main extension to code and build:
- V3s has 2x VI and 1x UI channels (use UI channel), so use third channel.
- V3s uses PLL3 (PLL_VIDEO) for both DE2 and TCON0 pixelclock.
  PLL10 is used for DDR2.
- V3s supports resolution upto 1024x1024. Also scale down memory 
  requirements for V3s (64MB embedded DDR2)
- V3s does not support HDMI.
Fourth patch extends u-boot DTS (linux DTS patch coming soon)
Fifth patch enable PWM drivers and corrects CONFIG_VIDEO_LCD_DCLK_PHASE.

Tested with "Lichee Zero" V3s and 480x272 LCD
(not configured DRM DE2/TCON0/panel in linux DTS).
- https://github.com/mcerveny/u-boot/tree/simplefb_v3s_v1

Martin Cerveny (5):
  sunxi: video: No double clock on DE2
  pwm: sunxi: v3s: Enable PWM0
  sunxi: video: v3s: Enable LCD support
  arm: dts: v3s: Add support for simple-framebuffer with DE2/TCON
  sunxi: add drivers and configs needed for LCD display

 arch/arm/dts/sun8i-v3s.dtsi                   | 128 +++++++++++++++++-
 arch/arm/include/asm/arch-sunxi/clock_sun6i.h |   5 +-
 arch/arm/include/asm/arch-sunxi/gpio.h        |   2 +
 arch/arm/mach-sunxi/Kconfig                   |   1 +
 configs/LicheePi_Zero_defconfig               |   4 +
 drivers/pwm/sunxi_pwm.c                       |   3 +
 drivers/video/sunxi/lcdc.c                    |   7 +-
 drivers/video/sunxi/sunxi_de2.c               |  25 +++-
 drivers/video/sunxi/sunxi_dw_hdmi.c           |   2 +
 drivers/video/sunxi/sunxi_lcd.c               |   9 +-
 10 files changed, 172 insertions(+), 14 deletions(-)

-- 
2.17.1

[PATCH 1/5] sunxi: video: No double clock on DE2

[Martin Cerveny]

Weird code or comment. This is variant is tested on V3s.

Signed-off-by: Martin Cerveny <m.cerveny@computer.org>
---
 drivers/video/sunxi/lcdc.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/video/sunxi/lcdc.c b/drivers/video/sunxi/lcdc.c
index 73033c3b85..b772947a6b 100644
--- a/drivers/video/sunxi/lcdc.c
+++ b/drivers/video/sunxi/lcdc.c
@@ -244,7 +244,6 @@ void lcdc_pll_set(struct sunxi_ccm_reg *ccm, int tcon, int dotclock,
 	 * not sync to higher frequencies.
 	 */
 	for (m = min_m; m <= max_m; m++) {
-#ifndef CONFIG_SUNXI_DE2
 		n = (m * dotclock) / step;
 
 		if ((n >= 9) && (n <= 127)) {
@@ -261,8 +260,8 @@ void lcdc_pll_set(struct sunxi_ccm_reg *ccm, int tcon, int dotclock,
 		/* These are just duplicates */
 		if (!(m & 1))
 			continue;
-#endif
 
+#ifndef CONFIG_SUNXI_DE2
 		/* No double clock on DE2 */
 		n = (m * dotclock) / (step * 2);
 		if ((n >= 9) && (n <= 127)) {
@@ -275,6 +274,7 @@ void lcdc_pll_set(struct sunxi_ccm_reg *ccm, int tcon, int dotclock,
 				best_double = 1;
 			}
 		}
+#endif
 	}
 
 #ifdef CONFIG_MACH_SUN6I
@@ -315,8 +315,7 @@ void lcdc_pll_set(struct sunxi_ccm_reg *ccm, int tcon, int dotclock,
 		writel(CCM_LCD_CH0_CTRL_GATE | CCM_LCD_CH0_CTRL_RST | pll,
 		       &ccm->lcd0_ch0_clk_cfg);
 #else
-		writel(CCM_LCD_CH0_CTRL_GATE | CCM_LCD_CH0_CTRL_RST | pll,
-		       &ccm->lcd0_clk_cfg);
+		writel(CCM_LCD0_CTRL_GATE | pll, &ccm->lcd0_clk_cfg);
 #endif
 	}
 #ifndef CONFIG_SUNXI_DE2
-- 
2.17.1

[PATCH 2/5] pwm: sunxi: v3s: Enable PWM0

[Martin Cerveny]

Enable PWM0 for V3S (for backlight LCD support).

Signed-off-by: Martin Cerveny <m.cerveny@computer.org>
---
 arch/arm/include/asm/arch-sunxi/gpio.h | 1 +
 drivers/pwm/sunxi_pwm.c                | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/arch/arm/include/asm/arch-sunxi/gpio.h b/arch/arm/include/asm/arch-sunxi/gpio.h
index a646ea6a3c..d83dfdf605 100644
--- a/arch/arm/include/asm/arch-sunxi/gpio.h
+++ b/arch/arm/include/asm/arch-sunxi/gpio.h
@@ -164,6 +164,7 @@ enum sunxi_gpio_number {
 #define SUN8I_A33_GPB_UART0	3
 #define SUN8I_A83T_GPB_UART0	2
 #define SUN8I_V3S_GPB_UART0	3
+#define SUN8I_V3S_GPB_PWM0	2
 #define SUN50I_GPB_UART0	4
 
 #define SUNXI_GPC_NAND		2
diff --git a/drivers/pwm/sunxi_pwm.c b/drivers/pwm/sunxi_pwm.c
index e2ae1a8009..136e7c63d3 100644
--- a/drivers/pwm/sunxi_pwm.c
+++ b/drivers/pwm/sunxi_pwm.c
@@ -48,6 +48,8 @@ static int sunxi_pwm_config_pinmux(void)
 {
 #ifdef CONFIG_MACH_SUN50I
 	sunxi_gpio_set_cfgpin(SUNXI_GPD(22), SUNXI_GPD_PWM);
+#elif defined CONFIG_MACH_SUN8I_V3S
+	sunxi_gpio_set_cfgpin(SUNXI_GPB(4), SUN8I_V3S_GPB_PWM0);
 #endif
 	return 0;
 }
@@ -171,6 +173,7 @@ static const struct pwm_ops sunxi_pwm_ops = {
 static const struct udevice_id sunxi_pwm_ids[] = {
 	{ .compatible = "allwinner,sun5i-a13-pwm" },
 	{ .compatible = "allwinner,sun50i-a64-pwm" },
+	{ .compatible = "allwinner,sun7i-a20-pwm" },
 	{ }
 };
 
-- 
2.17.1

[PATCH 3/5] sunxi: video: v3s: Enable LCD support

[Martin Cerveny]

Enable support for V3s LCD display.

V3s has 2x VI and 1x UI channels (use UI channel).
V3s uses PLL3 (PLL_VIDEO) for both DE2 and TCON0 pixelclock.
V3s supports resolution upto 1024x1024.
V3s does not support HDMI.

Signed-off-by: Martin Cerveny <m.cerveny@computer.org>
---
 arch/arm/include/asm/arch-sunxi/clock_sun6i.h |  5 ++--
 arch/arm/include/asm/arch-sunxi/gpio.h        |  1 +
 arch/arm/mach-sunxi/Kconfig                   |  1 +
 drivers/video/sunxi/sunxi_de2.c               | 25 ++++++++++++++++---
 drivers/video/sunxi/sunxi_dw_hdmi.c           |  2 ++
 drivers/video/sunxi/sunxi_lcd.c               |  9 ++++++-
 6 files changed, 37 insertions(+), 6 deletions(-)

diff --git a/arch/arm/include/asm/arch-sunxi/clock_sun6i.h b/arch/arm/include/asm/arch-sunxi/clock_sun6i.h
index ee387127f3..737c3a77fc 100644
--- a/arch/arm/include/asm/arch-sunxi/clock_sun6i.h
+++ b/arch/arm/include/asm/arch-sunxi/clock_sun6i.h
@@ -329,7 +329,7 @@ struct sunxi_ccm_reg {
 #define AHB_GATE_OFFSET_DE		12
 #define AHB_GATE_OFFSET_HDMI		11
 #define AHB_GATE_OFFSET_TVE		9
-#ifndef CONFIG_SUNXI_DE2
+#if !defined CONFIG_SUNXI_DE2 || defined CONFIG_MACH_SUN8I_V3S
 #define AHB_GATE_OFFSET_LCD1		5
 #define AHB_GATE_OFFSET_LCD0		4
 #else
@@ -476,7 +476,7 @@ struct sunxi_ccm_reg {
 #define AHB_RESET_OFFSET_HDMI		11
 #define AHB_RESET_OFFSET_HDMI2		10
 #define AHB_RESET_OFFSET_TVE		9
-#ifndef CONFIG_SUNXI_DE2
+#if !defined CONFIG_SUNXI_DE2 || defined CONFIG_MACH_SUN8I_V3S
 #define AHB_RESET_OFFSET_LCD1		5
 #define AHB_RESET_OFFSET_LCD0		4
 #else
@@ -510,6 +510,7 @@ struct sunxi_ccm_reg {
 #define CCM_DE2_CTRL_PLL_MASK		(3 << 24)
 #define CCM_DE2_CTRL_PLL6_2X		(0 << 24)
 #define CCM_DE2_CTRL_PLL10		(1 << 24)
+#define CCM_DE2_CTRL_PLL3_V3S		(0 << 24)
 #define CCM_DE2_CTRL_GATE		(0x1 << 31)
 
 /* CCU security switch, H3 only */
diff --git a/arch/arm/include/asm/arch-sunxi/gpio.h b/arch/arm/include/asm/arch-sunxi/gpio.h
index d83dfdf605..9b580fbe26 100644
--- a/arch/arm/include/asm/arch-sunxi/gpio.h
+++ b/arch/arm/include/asm/arch-sunxi/gpio.h
@@ -181,6 +181,7 @@ enum sunxi_gpio_number {
 #define SUN5I_GPE_SDC2		3
 #define SUN8I_GPE_TWI2		3
 #define SUN50I_GPE_TWI2		3
+#define SUN8I_V3S_GPE_LCD0	3
 
 #define SUNXI_GPF_SDC0		2
 #define SUNXI_GPF_UART0		4
diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
index be0822bfb7..dc0ee2cdef 100644
--- a/arch/arm/mach-sunxi/Kconfig
+++ b/arch/arm/mach-sunxi/Kconfig
@@ -258,6 +258,7 @@ config MACH_SUN8I_V3S
 	select CPU_V7_HAS_NONSEC
 	select CPU_V7_HAS_VIRT
 	select ARCH_SUPPORT_PSCI
+	select SUNXI_DE2
 	select SUNXI_GEN_SUN6I
 	select SUNXI_DRAM_DW
 	select SUNXI_DRAM_DW_16BIT
diff --git a/drivers/video/sunxi/sunxi_de2.c b/drivers/video/sunxi/sunxi_de2.c
index b657e163f0..49d41eb243 100644
--- a/drivers/video/sunxi/sunxi_de2.c
+++ b/drivers/video/sunxi/sunxi_de2.c
@@ -26,12 +26,21 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
+#ifdef CONFIG_MACH_SUN8I_V3S
+enum {
+	/* Maximum LCD size we support */
+	LCD_MAX_WIDTH		= 1024,
+	LCD_MAX_HEIGHT		= 1024,
+	LCD_MAX_LOG2_BPP	= VIDEO_BPP32,
+};
+#else
 enum {
 	/* Maximum LCD size we support */
 	LCD_MAX_WIDTH		= 3840,
 	LCD_MAX_HEIGHT		= 2160,
 	LCD_MAX_LOG2_BPP	= VIDEO_BPP32,
 };
+#endif
 
 static void sunxi_de2_composer_init(void)
 {
@@ -47,11 +56,19 @@ static void sunxi_de2_composer_init(void)
 	writel(reg_value, SUNXI_SRAMC_BASE + 0x04);
 #endif
 
+#ifdef CONFIG_MACH_SUN8I_V3S
+	clock_set_pll3(50000000);
+	/* pll3 is also used for pixelclock and speed will be recomputed */
+	/* Set DE parent to pll3 */
+	clrsetbits_le32(&ccm->de_clk_cfg, CCM_DE2_CTRL_PLL_MASK,
+			CCM_DE2_CTRL_PLL3_V3S);
+#else
 	clock_set_pll10(432000000);
 
 	/* Set DE parent to pll10 */
 	clrsetbits_le32(&ccm->de_clk_cfg, CCM_DE2_CTRL_PLL_MASK,
 			CCM_DE2_CTRL_PLL10);
+#endif
 
 	/* Set ahb gating to pass */
 	setbits_le32(&ccm->ahb_reset1_cfg, 1 << AHB_RESET_OFFSET_DE);
@@ -77,7 +94,8 @@ static void sunxi_de2_mode_set(int mux, const struct display_timing *mode,
 	struct de_ui * const de_ui_regs =
 		(struct de_ui *)(de_mux_base +
 				 SUNXI_DE2_MUX_CHAN_REGS +
-				 SUNXI_DE2_MUX_CHAN_SZ * 1);
+				 SUNXI_DE2_MUX_CHAN_SZ *
+				 (IS_ENABLED(CONFIG_MACH_SUN8I_V3S) ? 2 : 1));
 	struct de_csc * const de_csc_regs =
 		(struct de_csc *)(de_mux_base +
 				  SUNXI_DE2_MUX_DCSC_REGS);
@@ -104,14 +122,15 @@ static void sunxi_de2_mode_set(int mux, const struct display_timing *mode,
 	for (channel = 0; channel < 4; channel++) {
 		void *ch = (void *)(de_mux_base + SUNXI_DE2_MUX_CHAN_REGS +
 				    SUNXI_DE2_MUX_CHAN_SZ * channel);
-		memset(ch, 0, (channel == 0) ?
+		memset(ch, 0, (channel == 0 ||
+			       (IS_ENABLED(CONFIG_MACH_SUN8I_V3S) && channel == 1)) ?
 			sizeof(struct de_vi) : sizeof(struct de_ui));
 	}
 	memset(de_bld_regs, 0, sizeof(struct de_bld));
 
 	writel(0x00000101, &de_bld_regs->fcolor_ctl);
 
-	writel(1, &de_bld_regs->route);
+	writel(IS_ENABLED(CONFIG_MACH_SUN8I_V3S) ? 2 : 1, &de_bld_regs->route);
 
 	writel(0, &de_bld_regs->premultiply);
 	writel(0xff000000, &de_bld_regs->bkcolor);
diff --git a/drivers/video/sunxi/sunxi_dw_hdmi.c b/drivers/video/sunxi/sunxi_dw_hdmi.c
index 01d4b7a11c..899fbe2f7f 100644
--- a/drivers/video/sunxi/sunxi_dw_hdmi.c
+++ b/drivers/video/sunxi/sunxi_dw_hdmi.c
@@ -398,6 +398,8 @@ U_BOOT_DRIVER(sunxi_dw_hdmi) = {
 	.priv_auto_alloc_size = sizeof(struct sunxi_dw_hdmi_priv),
 };
 
+#ifndef CONFIG_MACH_SUN8I_V3S
 U_BOOT_DEVICE(sunxi_dw_hdmi) = {
 	.name = "sunxi_dw_hdmi"
 };
+#endif
diff --git a/drivers/video/sunxi/sunxi_lcd.c b/drivers/video/sunxi/sunxi_lcd.c
index 49bf083a8d..132e970e7d 100644
--- a/drivers/video/sunxi/sunxi_lcd.c
+++ b/drivers/video/sunxi/sunxi_lcd.c
@@ -32,6 +32,13 @@ static void sunxi_lcdc_config_pinmux(void)
 		sunxi_gpio_set_cfgpin(pin, SUNXI_GPD_LCD0);
 		sunxi_gpio_set_drv(pin, 3);
 	}
+#elif defined CONFIG_MACH_SUN8I_V3S
+	int pin;
+
+	for (pin = SUNXI_GPE(0); pin <= SUNXI_GPE(19); pin++)
+		sunxi_gpio_set_cfgpin(pin, SUN8I_V3S_GPE_LCD0);
+	sunxi_gpio_set_cfgpin(SUNXI_GPE(23), SUN8I_V3S_GPE_LCD0);
+	sunxi_gpio_set_cfgpin(SUNXI_GPE(24), SUN8I_V3S_GPE_LCD0);
 #endif
 }
 
@@ -145,7 +152,7 @@ U_BOOT_DRIVER(sunxi_lcd) = {
 	.priv_auto_alloc_size = sizeof(struct sunxi_lcd_priv),
 };
 
-#ifdef CONFIG_MACH_SUN50I
+#if defined CONFIG_MACH_SUN50I || defined CONFIG_MACH_SUN8I_V3S
 U_BOOT_DEVICE(sunxi_lcd) = {
 	.name = "sunxi_lcd"
 };
-- 
2.17.1

[PATCH 4/5] arm: dts: v3s: Add support for simple-framebuffer with DE2/TCON

[Martin Cerveny]

Add support for DE2 and TCON connected LCD display.
Add support for export as "allwinner,simple-framebuffer"
with "mixer0-lcd0" pipeline.

Signed-off-by: Martin Cerveny <m.cerveny@computer.org>
---
 arch/arm/dts/sun8i-v3s.dtsi | 128 ++++++++++++++++++++++++++++++++++--
 1 file changed, 124 insertions(+), 4 deletions(-)

diff --git a/arch/arm/dts/sun8i-v3s.dtsi b/arch/arm/dts/sun8i-v3s.dtsi
index ebefc0fefe..59ad5945a1 100644
--- a/arch/arm/dts/sun8i-v3s.dtsi
+++ b/arch/arm/dts/sun8i-v3s.dtsi
@@ -44,12 +44,28 @@
 #include <dt-bindings/reset/sun8i-v3s-ccu.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/pinctrl/sun4i-a10.h>
+#include <dt-bindings/clock/sun8i-de2.h>
 
 / {
 	#address-cells = <1>;
 	#size-cells = <1>;
 	interrupt-parent = <&gic>;
 
+	chosen {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+
+		framebuffer-lcd {
+			compatible = "allwinner,simple-framebuffer",
+				     "simple-framebuffer";
+			allwinner,pipeline = "mixer0-lcd0";
+			clocks = <&display_clocks CLK_MIXER0>,
+				 <&ccu CLK_TCON0>;
+			status = "disabled";
+		};
+	};
+
 	cpus {
 		#address-cells = <1>;
 		#size-cells = <0>;
@@ -90,12 +106,90 @@
 		};
 	};
 
+	de: display-engine {
+		compatible = "allwinner,sun8i-v3s-display-engine";
+		allwinner,pipelines = <&mixer0>;
+		status = "disabled";
+	};
+
 	soc {
 		compatible = "simple-bus";
 		#address-cells = <1>;
 		#size-cells = <1>;
 		ranges;
 
+		display_clocks: clock at 1000000 {
+			compatible = "allwinner,sun8i-v3s-de2-clk";
+			reg = <0x01000000 0x10000>;
+			clocks = <&ccu CLK_BUS_DE>,
+				 <&ccu CLK_DE>;
+			clock-names = "bus",
+				      "mod";
+			resets = <&ccu RST_BUS_DE>;
+			#clock-cells = <1>;
+			#reset-cells = <1>;
+		};
+
+		mixer0: mixer at 1100000 {
+			compatible = "allwinner,sun8i-v3s-de2-mixer";
+			reg = <0x01100000 0x100000>;
+			clocks = <&display_clocks 0>,
+				 <&display_clocks 6>;
+			clock-names = "bus",
+				      "mod";
+			resets = <&display_clocks 0>;
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				mixer0_out: port at 1 {
+					reg = <1>;
+
+					mixer0_out_tcon0: endpoint {
+						remote-endpoint = <&tcon0_in_mixer0>;
+					};
+				};
+			};
+		};
+
+		tcon0: lcd-controller at 1c0c000 {
+			compatible = "allwinner,sun8i-v3s-tcon";
+			reg = <0x01c0c000 0x1000>;
+			interrupts = <GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&ccu CLK_BUS_TCON0>,
+				 <&ccu CLK_TCON0>;
+			clock-names = "ahb",
+				      "tcon-ch0";
+			clock-output-names = "tcon-pixel-clock";
+			#clock-cells = <0>;
+			resets = <&ccu RST_BUS_TCON0>;
+			reset-names = "lcd";
+			status = "disabled";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				tcon0_in: port at 0 {
+					reg = <0>;
+
+					tcon0_in_mixer0: endpoint {
+						remote-endpoint = <&mixer0_out_tcon0>;
+					};
+				};
+
+				tcon0_out: port at 1 {
+					#address-cells = <1>;
+					#size-cells = <0>;
+					reg = <1>;
+					tcon0_out_lcd: endpoint at 0 {
+						reg = <0>;
+					};
+				};
+			};
+		};
+
 		mmc0: mmc at 01c0f000 {
 			compatible = "allwinner,sun7i-a20-mmc";
 			reg = <0x01c0f000 0x1000>;
@@ -208,10 +302,12 @@
 			interrupt-controller;
 			#interrupt-cells = <3>;
 
-			uart0_pins_a: uart0 at 0 {
-				pins = "PB8", "PB9";
-				function = "uart0";
-				bias-pull-up;
+			lcd_pins: lcd-pins {
+				pins = "PE0", "PE1", "PE2", "PE3", "PE4", "PE5",
+				       "PE6", "PE7", "PE8", "PE9", "PE10", "PE11",
+				       "PE12", "PE13", "PE14", "PE15", "PE16", "PE17",
+				       "PE18", "PE19", "PE23", "PE24";
+				function = "lcd";
 			};
 
 			mmc0_pins_a: mmc0 at 0 {
@@ -221,6 +317,22 @@
 				drive-strength = <30>;
 				bias-pull-up;
 			};
+
+			pwm0_pins: pwm0-pins {
+				pins = "PB4";
+				function = "pwm0";
+			};
+
+			pwm1_pins: pwm1-pins {
+				pins = "PB5";
+				function = "pwm1";
+			};
+
+			uart0_pins_a: uart0 at 0 {
+				pins = "PB8", "PB9";
+				function = "uart0";
+				bias-pull-up;
+			};
 		};
 
 		timer at 01c20c00 {
@@ -280,5 +392,13 @@
 			#interrupt-cells = <3>;
 			interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
 		};
+
+		pwm: pwm at 1c21400 {
+			compatible = "allwinner,sun7i-a20-pwm";
+			reg = <0x01c21400 0x400>;
+			clocks = <&osc24M>;
+			#pwm-cells = <3>;
+			status = "disabled";
+		};
 	};
 };
-- 
2.17.1

[PATCH 5/5] sunxi: add drivers and configs needed for LCD display

[Martin Cerveny]

Add PWM and dummy power regulator support.
Modify data timings for LCD displays.

Signed-off-by: Martin Cerveny <m.cerveny@computer.org>
---
 configs/LicheePi_Zero_defconfig | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/configs/LicheePi_Zero_defconfig b/configs/LicheePi_Zero_defconfig
index 04d7b64504..ba1c085ec3 100644
--- a/configs/LicheePi_Zero_defconfig
+++ b/configs/LicheePi_Zero_defconfig
@@ -3,5 +3,9 @@ CONFIG_ARCH_SUNXI=y
 CONFIG_SPL=y
 CONFIG_MACH_SUN8I_V3S=y
 CONFIG_DRAM_CLK=360
+CONFIG_VIDEO_LCD_DCLK_PHASE=0
 CONFIG_DEFAULT_DEVICE_TREE="sun8i-v3s-licheepi-zero"
 # CONFIG_NETDEVICES is not set
+CONFIG_DM_REGULATOR=y
+CONFIG_DM_PWM=y
+CONFIG_PWM_SUNXI=y
-- 
2.17.1

[PATCH 1/5] sunxi: video: No double clock on DE2

[Maxime Ripard]

On Wed, Sep 16, 2020 at 04:10:48PM +0200, Martin Cerveny wrote:
> Weird code or comment. This is variant is tested on V3s.
> 
> Signed-off-by: Martin Cerveny <m.cerveny@computer.org>

Generally speaking your commit logs are fairly concise, but it really
becomes an issue when you're allegedly fixing a bug.

There's a bunch of questions here that are completely up in the air:

  - What issue are you actually trying to fix, how can one reproduce it
  - You claim that there's no double clock on the DE2, according to
    what?
  - DE2 is used on way more SoCs than just the V3s, did you check/test
    on those SoCs as well?

In general, a good commit log should not explain what you're doing but
*why* you're doing it. The what can be quite easily figured out from the
patch content, the why can't today, and it will be even harder in a
year's time.

Maxime
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200916/57638848/attachment.sig>

[PATCH 4/5] arm: dts: v3s: Add support for simple-framebuffer with DE2/TCON

[Maxime Ripard]

On Wed, Sep 16, 2020 at 04:10:51PM +0200, Martin Cerveny wrote:
> Add support for DE2 and TCON connected LCD display.
> Add support for export as "allwinner,simple-framebuffer"
> with "mixer0-lcd0" pipeline.
> 
> Signed-off-by: Martin Cerveny <m.cerveny@computer.org>

You shouldn't do multiple things at once. If you need to, then split it into several patches.

Maxime
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200916/16d2ee9f/attachment.sig>

[PATCH 5/5] sunxi: add drivers and configs needed for LCD display

[Maxime Ripard]

On Wed, Sep 16, 2020 at 04:10:52PM +0200, Martin Cerveny wrote:
> Add PWM and dummy power regulator support.
> Modify data timings for LCD displays.
> 
> Signed-off-by: Martin Cerveny <m.cerveny@computer.org>
> ---
>  configs/LicheePi_Zero_defconfig | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/configs/LicheePi_Zero_defconfig b/configs/LicheePi_Zero_defconfig
> index 04d7b64504..ba1c085ec3 100644
> --- a/configs/LicheePi_Zero_defconfig
> +++ b/configs/LicheePi_Zero_defconfig
> @@ -3,5 +3,9 @@ CONFIG_ARCH_SUNXI=y
>  CONFIG_SPL=y
>  CONFIG_MACH_SUN8I_V3S=y
>  CONFIG_DRAM_CLK=360
> +CONFIG_VIDEO_LCD_DCLK_PHASE=0
>  CONFIG_DEFAULT_DEVICE_TREE="sun8i-v3s-licheepi-zero"
>  # CONFIG_NETDEVICES is not set
> +CONFIG_DM_REGULATOR=y
> +CONFIG_DM_PWM=y
> +CONFIG_PWM_SUNXI=y

The LicheePi Zero doesn't have any display by default. Also you're
mentionning in your commit log that you "modify data timings", but I
don't see any timings in there?

Maxime
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200916/9411ab26/attachment.sig>

[PATCH 5/5] sunxi: add drivers and configs needed for LCD display

[Martin Cerveny]

On Wed, 16 Sep 2020, Maxime Ripard wrote:
> On Wed, Sep 16, 2020 at 04:10:52PM +0200, Martin Cerveny wrote:
>> Add PWM and dummy power regulator support.
>> Modify data timings for LCD displays.
>>
>> Signed-off-by: Martin Cerveny <m.cerveny@computer.org>
>> ---
>>  configs/LicheePi_Zero_defconfig | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/configs/LicheePi_Zero_defconfig b/configs/LicheePi_Zero_defconfig
>> index 04d7b64504..ba1c085ec3 100644
>> --- a/configs/LicheePi_Zero_defconfig
>> +++ b/configs/LicheePi_Zero_defconfig
>> @@ -3,5 +3,9 @@ CONFIG_ARCH_SUNXI=y
>>  CONFIG_SPL=y
>>  CONFIG_MACH_SUN8I_V3S=y
>>  CONFIG_DRAM_CLK=360
>> +CONFIG_VIDEO_LCD_DCLK_PHASE=0
>>  CONFIG_DEFAULT_DEVICE_TREE="sun8i-v3s-licheepi-zero"
>>  # CONFIG_NETDEVICES is not set
>> +CONFIG_DM_REGULATOR=y
>> +CONFIG_DM_PWM=y
>> +CONFIG_PWM_SUNXI=y
>
> The LicheePi Zero doesn't have any display by default. Also you're
> mentionning in your commit log that you "modify data timings", but I
> don't see any timings in there?

CONFIG_VIDEO_LCD_DCLK_PHASE is LCD data clock phase,
standard "1" corrupts output on display,
see
- register "TCON0_IO_POL_REG" in V3S manual
- arch/arm/mach-sunxi/Kconfig

Regards.

[PATCH 4/5] arm: dts: v3s: Add support for simple-framebuffer with DE2/TCON

[Martin Cerveny]

On Wed, 16 Sep 2020, Maxime Ripard wrote:
> On Wed, Sep 16, 2020 at 04:10:51PM +0200, Martin Cerveny wrote:
>> Add support for DE2 and TCON connected LCD display.
>> Add support for export as "allwinner,simple-framebuffer"
>> with "mixer0-lcd0" pipeline.
>>
>> Signed-off-by: Martin Cerveny <m.cerveny@computer.org>
>
> You shouldn't do multiple things at once. If you need to, then split it into several patches.
>
> Maxime

How to split - is three sufficient ?
- pwm pins & registration
- LCD pins & DE/TCOM registration
- choosen framebuffer-lcd

Regards.

[PATCH 1/5] sunxi: video: No double clock on DE2

[Martin Cerveny]

On Wed, 16 Sep 2020, Maxime Ripard wrote:

> On Wed, Sep 16, 2020 at 04:10:48PM +0200, Martin Cerveny wrote:
>> Weird code or comment. This is variant is tested on V3s.
>>
>> Signed-off-by: Martin Cerveny <m.cerveny@computer.org>
>
> Generally speaking your commit logs are fairly concise, but it really
> becomes an issue when you're allegedly fixing a bug.
>
> There's a bunch of questions here that are completely up in the air:
>
>  - What issue are you actually trying to fix, how can one reproduce it
>  - You claim that there's no double clock on the DE2, according to
>    what?
>  - DE2 is used on way more SoCs than just the V3s, did you check/test
>    on those SoCs as well?
>
> In general, a good commit log should not explain what you're doing but
> *why* you're doing it. The what can be quite easily figured out from the
> patch content, the why can't today, and it will be even harder in a
> year's time.
>
> Maxime
>

I cannot issue more information on the problem:
- check original code - the "1x pll" was commented by #ifndef
   (only "2x pll" is computed)
- but original author added comment "/* No double clock on DE2 */"
- V3s does not support "2x pll" in TCON_CLK_REG

So I wrote where I tested this patch.
It should be discussed with original author.

Regards.

CC: Vasily Khoruzhick <anarsoul@gmail.com>

[PATCH 1/5] sunxi: video: No double clock on DE2

[Vasily Khoruzhick]

On Wed, Sep 16, 2020 at 9:26 AM Martin Cerveny <martin@c-home.cz> wrote:
>
>
>
> On Wed, 16 Sep 2020, Maxime Ripard wrote:
>
> > On Wed, Sep 16, 2020 at 04:10:48PM +0200, Martin Cerveny wrote:
> >> Weird code or comment. This is variant is tested on V3s.
> >>
> >> Signed-off-by: Martin Cerveny <m.cerveny@computer.org>
> >
> > Generally speaking your commit logs are fairly concise, but it really
> > becomes an issue when you're allegedly fixing a bug.
> >
> > There's a bunch of questions here that are completely up in the air:
> >
> >  - What issue are you actually trying to fix, how can one reproduce it
> >  - You claim that there's no double clock on the DE2, according to
> >    what?
> >  - DE2 is used on way more SoCs than just the V3s, did you check/test
> >    on those SoCs as well?
> >
> > In general, a good commit log should not explain what you're doing but
> > *why* you're doing it. The what can be quite easily figured out from the
> > patch content, the why can't today, and it will be even harder in a
> > year's time.
> >
> > Maxime
> >
>
> I cannot issue more information on the problem:
> - check original code - the "1x pll" was commented by #ifndef
>    (only "2x pll" is computed)
> - but original author added comment "/* No double clock on DE2 */"
> - V3s does not support "2x pll" in TCON_CLK_REG
>
> So I wrote where I tested this patch.
> It should be discussed with original author.

Sorry, I don't remember the details on why the driver uses
pll-video0-2x, that's the code from 2017

> Regards.
>
> CC: Vasily Khoruzhick <anarsoul@gmail.com>

[PATCH 5/5] sunxi: add drivers and configs needed for LCD display

[Maxime Ripard]

On Wed, Sep 16, 2020 at 05:44:58PM +0200, Martin Cerveny wrote:
> 
> 
> On Wed, 16 Sep 2020, Maxime Ripard wrote:
> > On Wed, Sep 16, 2020 at 04:10:52PM +0200, Martin Cerveny wrote:
> > > Add PWM and dummy power regulator support.
> > > Modify data timings for LCD displays.
> > > 
> > > Signed-off-by: Martin Cerveny <m.cerveny@computer.org>
> > > ---
> > >  configs/LicheePi_Zero_defconfig | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/configs/LicheePi_Zero_defconfig b/configs/LicheePi_Zero_defconfig
> > > index 04d7b64504..ba1c085ec3 100644
> > > --- a/configs/LicheePi_Zero_defconfig
> > > +++ b/configs/LicheePi_Zero_defconfig
> > > @@ -3,5 +3,9 @@ CONFIG_ARCH_SUNXI=y
> > >  CONFIG_SPL=y
> > >  CONFIG_MACH_SUN8I_V3S=y
> > >  CONFIG_DRAM_CLK=360
> > > +CONFIG_VIDEO_LCD_DCLK_PHASE=0
> > >  CONFIG_DEFAULT_DEVICE_TREE="sun8i-v3s-licheepi-zero"
> > >  # CONFIG_NETDEVICES is not set
> > > +CONFIG_DM_REGULATOR=y
> > > +CONFIG_DM_PWM=y
> > > +CONFIG_PWM_SUNXI=y
> > 
> > The LicheePi Zero doesn't have any display by default. Also you're
> > mentionning in your commit log that you "modify data timings", but I
> > don't see any timings in there?
> 
> CONFIG_VIDEO_LCD_DCLK_PHASE is LCD data clock phase,
> standard "1" corrupts output on display,
> see
> - register "TCON0_IO_POL_REG" in V3S manual
> - arch/arm/mach-sunxi/Kconfig

So just call the phase of the data signal. Timings for LCD displays have
a very different meaning.

Maxime

[PATCH 4/5] arm: dts: v3s: Add support for simple-framebuffer with DE2/TCON

[Maxime Ripard]

On Wed, Sep 16, 2020 at 05:53:35PM +0200, Martin Cerveny wrote:
> On Wed, 16 Sep 2020, Maxime Ripard wrote:
> > On Wed, Sep 16, 2020 at 04:10:51PM +0200, Martin Cerveny wrote:
> > > Add support for DE2 and TCON connected LCD display.
> > > Add support for export as "allwinner,simple-framebuffer"
> > > with "mixer0-lcd0" pipeline.
> > > 
> > > Signed-off-by: Martin Cerveny <m.cerveny@computer.org>
> > 
> > You shouldn't do multiple things at once. If you need to, then split it into several patches.
> > 
> > Maxime
> 
> How to split - is three sufficient ?
> - pwm pins & registration
> - LCD pins & DE/TCOM registration
> - choosen framebuffer-lcd

Yep, that sounds reasonable.

For the last one though, we generally favor just sync'ing the Linux
device tree

Maxime