[U-Boot] sunxi: video: Add LCD output and A13-Olinuxino VGA output support

[U-Boot] sunxi: video: Add LCD output and A13-Olinuxino VGA output support

[Hans de Goede]

Hi All,

After a couple of days of work I'm very happy to send out the next sunxi
video improvement series. This series adds support for outputting video
through the sunxi lcd controller. This means we now have u-boot status and
console output on e.g. A23 tablets, reducing the need to solder a serial
console to their PCB-s :)

Besides this, this series also adds support for the special VGA output
found on A13-Olinuxino[-Micro] boards.

I've one more series (or hopefully a few small patches) planned to also add
support for the native vga dac found on most sunxi SoCs and actually routed
to a VGA connector on some boards. After that I'm done with the sunxi video
support work, and I hope to slowly start on A80 support.

As always please review.

Merry Christmas & Regards,

Hans

[U-Boot] [PATCH 01/17] videomodes: Add support for refresh and pclk_khz to video_get_params()

[Hans de Goede]

Add support to video_get_params() for setting the new refresh and pixclock_khz
struct ctfb_res_modes members.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/video/videomodes.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/video/videomodes.c b/drivers/video/videomodes.c
index 07d1a9f..3e88273 100644
--- a/drivers/video/videomodes.c
+++ b/drivers/video/videomodes.c
@@ -198,6 +198,7 @@ int video_get_params (struct ctfb_res_modes *pPar, char *penv)
 	while ((i = video_get_param_len (p, ',')) != 0) {
 		GET_OPTION ("x:", pPar->xres)
 			GET_OPTION ("y:", pPar->yres)
+			GET_OPTION ("refresh:", pPar->refresh)
 			GET_OPTION ("le:", pPar->left_margin)
 			GET_OPTION ("ri:", pPar->right_margin)
 			GET_OPTION ("up:", pPar->upper_margin)
@@ -207,6 +208,7 @@ int video_get_params (struct ctfb_res_modes *pPar, char *penv)
 			GET_OPTION ("sync:", pPar->sync)
 			GET_OPTION ("vmode:", pPar->vmode)
 			GET_OPTION ("pclk:", pPar->pixclock)
+			GET_OPTION ("pclk_khz:", pPar->pixclock_khz)
 			GET_OPTION ("depth:", bpp)
 			p += i;
 		if (*p != 0)
-- 
2.1.0

[U-Boot] [PATCH 01/17] videomodes: Add support for refresh and pclk_khz to video_get_params()

[Ian Campbell]

On Wed, 2014-12-24 at 20:06 +0100, Hans de Goede wrote:
> Add support to video_get_params() for setting the new refresh and pixclock_khz
> struct ctfb_res_modes members.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Ian Campbell <ijc@hellion.org.uk> (needs Anatolij's though I
think)

> ---
>  drivers/video/videomodes.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/video/videomodes.c b/drivers/video/videomodes.c
> index 07d1a9f..3e88273 100644
> --- a/drivers/video/videomodes.c
> +++ b/drivers/video/videomodes.c
> @@ -198,6 +198,7 @@ int video_get_params (struct ctfb_res_modes *pPar, char *penv)
>  	while ((i = video_get_param_len (p, ',')) != 0) {
>  		GET_OPTION ("x:", pPar->xres)
>  			GET_OPTION ("y:", pPar->yres)
> +			GET_OPTION ("refresh:", pPar->refresh)
>  			GET_OPTION ("le:", pPar->left_margin)
>  			GET_OPTION ("ri:", pPar->right_margin)
>  			GET_OPTION ("up:", pPar->upper_margin)
> @@ -207,6 +208,7 @@ int video_get_params (struct ctfb_res_modes *pPar, char *penv)
>  			GET_OPTION ("sync:", pPar->sync)
>  			GET_OPTION ("vmode:", pPar->vmode)
>  			GET_OPTION ("pclk:", pPar->pixclock)
> +			GET_OPTION ("pclk_khz:", pPar->pixclock_khz)
>  			GET_OPTION ("depth:", bpp)
>  			p += i;
>  		if (*p != 0)

[U-Boot] [PATCH 01/17] videomodes: Add support for refresh and pclk_khz to video_get_params()

[Anatolij Gustschin]

On Wed, 24 Dec 2014 20:06:13 +0100
Hans de Goede <hdegoede@redhat.com> wrote:

> Add support to video_get_params() for setting the new refresh and pixclock_khz
> struct ctfb_res_modes members.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Anatolij Gustschin <agust@denx.de>

[U-Boot] [PATCH 02/17] sunxi: gpio: Properly sort mux defines by port number

[Hans de Goede]

Move a few mux defines around so that all the mux defines are properly sorted
by port number.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 arch/arm/include/asm/arch-sunxi/gpio.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm/include/asm/arch-sunxi/gpio.h b/arch/arm/include/asm/arch-sunxi/gpio.h
index 6623f15..6730879 100644
--- a/arch/arm/include/asm/arch-sunxi/gpio.h
+++ b/arch/arm/include/asm/arch-sunxi/gpio.h
@@ -145,11 +145,6 @@ enum sunxi_gpio_number {
 #define SUN5I_GPB19_UART0_TX	2
 #define SUN5I_GPB20_UART0_RX	2
 
-#define SUN5I_GPG3_SDC1		2
-
-#define SUN5I_GPG3_UART1_TX	4
-#define SUN5I_GPG4_UART1_RX	4
-
 #define SUNXI_GPC6_SDC2		3
 
 #define SUNXI_GPF0_SDC0		2
@@ -166,6 +161,11 @@ enum sunxi_gpio_number {
 
 #define SUN4I_GPG0_SDC1		4
 
+#define SUN5I_GPG3_SDC1		2
+
+#define SUN5I_GPG3_UART1_TX	4
+#define SUN5I_GPG4_UART1_RX	4
+
 #define SUN4I_GPH22_SDC1	5
 
 #define SUN6I_GPH20_UART0_TX	2
-- 
2.1.0

[U-Boot] [PATCH 02/17] sunxi: gpio: Properly sort mux defines by port number

[Ian Campbell]

On Wed, 2014-12-24 at 20:06 +0100, Hans de Goede wrote:
> Move a few mux defines around so that all the mux defines are properly sorted
> by port number.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Ian Campbell <ijc@hellion.org.uk>

[U-Boot] [PATCH 03/17] sunxi: gpio: Add support for gpio pins on the AXP209 pmic

[Hans de Goede]

Some boards use GPIO-s on the pmic, one example of this is the A13-OLinuXino
board, which uses gpio0 of the axp209 for the lcd-power signal.

This commit adds support for gpio pins on the AXP209 pmic, the sunxi_gpio.c
changes are universal, adding gpio support for the other AXP pmics (when
necessary) should be a matter of adding the necessary axp_gpio_foo functions
to their resp. drivers, and add "#define AXP_GPIO" to their header file.

Note this commit only adds support for the non device-model version of the
gpio code, patches for adding support to the device-model version are very
welcome.

The string representation for these gpio-s is AXP0-#, the 0 in the AXP0 prefix
is there in case we need to support gpio-s on more then 1 pmic in the future.
At least A80 boards have 2 pmics, and we may end up needing to support gpio-s
on both.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 arch/arm/include/asm/arch-sunxi/gpio.h |  3 ++
 drivers/gpio/sunxi_gpio.c              | 30 ++++++++++++++
 drivers/power/axp209.c                 | 74 ++++++++++++++++++++++++++++++++++
 include/axp209.h                       |  7 ++++
 4 files changed, 114 insertions(+)

diff --git a/arch/arm/include/asm/arch-sunxi/gpio.h b/arch/arm/include/asm/arch-sunxi/gpio.h
index 6730879..32941cb 100644
--- a/arch/arm/include/asm/arch-sunxi/gpio.h
+++ b/arch/arm/include/asm/arch-sunxi/gpio.h
@@ -114,6 +114,7 @@ enum sunxi_gpio_number {
 	SUNXI_GPIO_I_START = SUNXI_GPIO_NEXT(SUNXI_GPIO_H),
 	SUNXI_GPIO_L_START = 352,
 	SUNXI_GPIO_M_START = SUNXI_GPIO_NEXT(SUNXI_GPIO_L),
+	SUNXI_GPIO_AXP0_START = 1024,
 };
 
 /* SUNXI GPIO number definitions */
@@ -129,6 +130,8 @@ enum sunxi_gpio_number {
 #define SUNXI_GPL(_nr)	(SUNXI_GPIO_L_START + (_nr))
 #define SUNXI_GPM(_nr)	(SUNXI_GPIO_M_START + (_nr))
 
+#define SUNXI_GPAXP0(_nr)	(SUNXI_GPIO_AXP0_START + (_nr))
+
 /* GPIO pin function config */
 #define SUNXI_GPIO_INPUT	0
 #define SUNXI_GPIO_OUTPUT	1
diff --git a/drivers/gpio/sunxi_gpio.c b/drivers/gpio/sunxi_gpio.c
index 2fa50f9..6296092 100644
--- a/drivers/gpio/sunxi_gpio.c
+++ b/drivers/gpio/sunxi_gpio.c
@@ -18,6 +18,9 @@
 #include <asm/io.h>
 #include <asm/gpio.h>
 #include <dm/device-internal.h>
+#ifdef CONFIG_AXP209_POWER
+#include <axp209.h>
+#endif
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -73,6 +76,10 @@ int gpio_free(unsigned gpio)
 
 int gpio_direction_input(unsigned gpio)
 {
+#ifdef AXP_GPIO
+	if (gpio >= SUNXI_GPIO_AXP0_START)
+		return axp_gpio_direction_input(gpio - SUNXI_GPIO_AXP0_START);
+#endif
 	sunxi_gpio_set_cfgpin(gpio, SUNXI_GPIO_INPUT);
 
 	return 0;
@@ -80,6 +87,11 @@ int gpio_direction_input(unsigned gpio)
 
 int gpio_direction_output(unsigned gpio, int value)
 {
+#ifdef AXP_GPIO
+	if (gpio >= SUNXI_GPIO_AXP0_START)
+		return axp_gpio_direction_output(gpio - SUNXI_GPIO_AXP0_START,
+						 value);
+#endif
 	sunxi_gpio_set_cfgpin(gpio, SUNXI_GPIO_OUTPUT);
 
 	return sunxi_gpio_output(gpio, value);
@@ -87,11 +99,19 @@ int gpio_direction_output(unsigned gpio, int value)
 
 int gpio_get_value(unsigned gpio)
 {
+#ifdef AXP_GPIO
+	if (gpio >= SUNXI_GPIO_AXP0_START)
+		return axp_gpio_get_value(gpio - SUNXI_GPIO_AXP0_START);
+#endif
 	return sunxi_gpio_input(gpio);
 }
 
 int gpio_set_value(unsigned gpio, int value)
 {
+#ifdef AXP_GPIO
+	if (gpio >= SUNXI_GPIO_AXP0_START)
+		return axp_gpio_set_value(gpio - SUNXI_GPIO_AXP0_START, value);
+#endif
 	return sunxi_gpio_output(gpio, value);
 }
 
@@ -101,6 +121,16 @@ int sunxi_name_to_gpio(const char *name)
 	int groupsize = 9 * 32;
 	long pin;
 	char *eptr;
+
+#ifdef AXP_GPIO
+	if (strncasecmp(name, "AXP0-", 5) == 0) {
+		name += 5;
+		pin = simple_strtol(name, &eptr, 10);
+		if (!*name || *eptr)
+			return -1;
+		return SUNXI_GPIO_AXP0_START + pin;
+	}
+#endif
 	if (*name == 'P' || *name == 'p')
 		name++;
 	if (*name >= 'A') {
diff --git a/drivers/power/axp209.c b/drivers/power/axp209.c
index 9798e5b..397963e 100644
--- a/drivers/power/axp209.c
+++ b/drivers/power/axp209.c
@@ -18,6 +18,11 @@ enum axp209_reg {
 	AXP209_LDO3_VOLTAGE = 0x29,
 	AXP209_IRQ_STATUS5 = 0x4c,
 	AXP209_SHUTDOWN = 0x32,
+	AXP209_GPIO0_CTRL = 0x90,
+	AXP209_GPIO1_CTRL = 0x92,
+	AXP209_GPIO2_CTRL = 0x93,
+	AXP209_GPIO_STATE = 0x94,
+	AXP209_GPIO3_CTRL = 0x95,
 };
 
 #define AXP209_POWER_STATUS_ON_BY_DC	(1 << 0)
@@ -27,6 +32,17 @@ enum axp209_reg {
 
 #define AXP209_POWEROFF			(1 << 7)
 
+#define AXP209_GPIO_OUTPUT_LOW		0x00
+#define AXP209_GPIO_OUTPUT_HIGH		0x01
+#define AXP209_GPIO_INPUT		0x02
+
+#define AXP209_GPIO_OUTPUT_LOW		0x00
+#define AXP209_GPIO_OUTPUT_HIGH		0x01
+
+#define AXP209_GPIO3_OUTPUT_LOW		0x00 /* Drive pin low, Output mode */
+#define AXP209_GPIO3_OUTPUT_HIGH	0x02 /* Float pin, Output mode */
+#define AXP209_GPIO3_INPUT		0x06 /* Float pin, Input mode */
+
 static int axp209_write(enum axp209_reg reg, u8 val)
 {
 	return i2c_write(0x34, reg, 1, &val, 1);
@@ -165,3 +181,61 @@ int axp209_power_button(void)
 
 	return v & AXP209_IRQ5_PEK_DOWN;
 }
+
+static u8 axp209_get_gpio_ctrl_reg(unsigned int pin)
+{
+	switch (pin) {
+	case 0: return AXP209_GPIO0_CTRL;
+	case 1: return AXP209_GPIO1_CTRL;
+	case 2: return AXP209_GPIO2_CTRL;
+	case 3: return AXP209_GPIO3_CTRL;
+	}
+	return 0;
+}
+
+int axp_gpio_direction_input(unsigned int pin)
+{
+	u8 reg = axp209_get_gpio_ctrl_reg(pin);
+	/* GPIO3 is "special" */
+	u8 val = (pin == 3) ? AXP209_GPIO3_INPUT : AXP209_GPIO_INPUT;
+
+	return axp209_write(reg, val);
+}
+
+int axp_gpio_direction_output(unsigned int pin, unsigned int val)
+{
+	u8 reg = axp209_get_gpio_ctrl_reg(pin);
+
+	if (val) {
+		val = (pin == 3) ? AXP209_GPIO3_OUTPUT_HIGH : 
+				   AXP209_GPIO_OUTPUT_HIGH;
+	} else {
+		val = (pin == 3) ? AXP209_GPIO3_OUTPUT_LOW : 
+				   AXP209_GPIO_OUTPUT_LOW;
+	}
+
+	return axp209_write(reg, val);
+}
+
+int axp_gpio_get_value(unsigned int pin)
+{
+	u8 val, mask;
+	int rc;
+
+	if (pin == 3) {
+		rc = axp209_read(AXP209_GPIO3_CTRL, &val);
+		mask = 1;
+	} else {
+		rc = axp209_read(AXP209_GPIO_STATE, &val);
+		mask = 1 << (pin + 4);
+	}
+	if (rc)
+		return rc;
+
+	return (val & mask) ? 1 : 0;
+}
+
+int axp_gpio_set_value(unsigned int pin, unsigned int val)
+{
+	return axp_gpio_direction_output(pin, val);
+}
diff --git a/include/axp209.h b/include/axp209.h
index 21efce6..0436249 100644
--- a/include/axp209.h
+++ b/include/axp209.h
@@ -4,6 +4,8 @@
  * SPDX-License-Identifier:	GPL-2.0+
  */
 
+#define AXP_GPIO
+
 extern int axp209_set_dcdc2(int mvolt);
 extern int axp209_set_dcdc3(int mvolt);
 extern int axp209_set_ldo2(int mvolt);
@@ -12,3 +14,8 @@ extern int axp209_set_ldo4(int mvolt);
 extern int axp209_init(void);
 extern int axp209_poweron_by_dc(void);
 extern int axp209_power_button(void);
+
+extern int axp_gpio_direction_input(unsigned int pin);
+extern int axp_gpio_direction_output(unsigned int pin, unsigned int val);
+extern int axp_gpio_get_value(unsigned int pin);
+extern int axp_gpio_set_value(unsigned int pin, unsigned int val);
-- 
2.1.0

[U-Boot] [PATCH 03/17] sunxi: gpio: Add support for gpio pins on the AXP209 pmic

[Ian Campbell]

On Wed, 2014-12-24 at 20:06 +0100, Hans de Goede wrote:
> @@ -27,6 +32,17 @@ enum axp209_reg {
>  
>  #define AXP209_POWEROFF			(1 << 7)
>  
> +#define AXP209_GPIO_OUTPUT_LOW		0x00
> +#define AXP209_GPIO_OUTPUT_HIGH		0x01
> +#define AXP209_GPIO_INPUT		0x02
> +
> +#define AXP209_GPIO_OUTPUT_LOW		0x00
> +#define AXP209_GPIO_OUTPUT_HIGH		0x01

Aren't these LOW+HIGH ones duplicated?

Also, they lack the helpful comments which you added to the GPIO3 ones.

I'd be included to add a /* GPIO3 is different from the others */ just
here too (also: "sigh" re h/w inconsistency...).

> +#define AXP209_GPIO3_OUTPUT_LOW		0x00 /* Drive pin low, Output mode */
> +#define AXP209_GPIO3_OUTPUT_HIGH	0x02 /* Float pin, Output mode */

Is a floating output really a thing or is this a cut-and-paste-o?

> +#define AXP209_GPIO3_INPUT		0x06 /* Float pin, Input mode */
> +
> +int axp_gpio_direction_output(unsigned int pin, unsigned int val)
> +{
> +	u8 reg = axp209_get_gpio_ctrl_reg(pin);
> +
> +	if (val) {
> +		val = (pin == 3) ? AXP209_GPIO3_OUTPUT_HIGH : 
> +				   AXP209_GPIO_OUTPUT_HIGH;
> +	} else {
> +		val = (pin == 3) ? AXP209_GPIO3_OUTPUT_LOW : 
> +				   AXP209_GPIO_OUTPUT_LOW;

Both OUTPUT_LOW values happen to be the same, although I could see why
you might want to do it this way for consistency.

Ian.

[U-Boot] [PATCH 03/17] sunxi: gpio: Add support for gpio pins on the AXP209 pmic

[Hans de Goede]

Hi,

On 28-12-14 10:34, Ian Campbell wrote:
> On Wed, 2014-12-24 at 20:06 +0100, Hans de Goede wrote:
>> @@ -27,6 +32,17 @@ enum axp209_reg {
>>
>>   #define AXP209_POWEROFF			(1 << 7)
>>
>> +#define AXP209_GPIO_OUTPUT_LOW		0x00
>> +#define AXP209_GPIO_OUTPUT_HIGH		0x01
>> +#define AXP209_GPIO_INPUT		0x02
>> +
>> +#define AXP209_GPIO_OUTPUT_LOW		0x00
>> +#define AXP209_GPIO_OUTPUT_HIGH		0x01
>
> Aren't these LOW+HIGH ones duplicated?

Oops, copy + paste fail.

>
> Also, they lack the helpful comments which you added to the GPIO3 ones.

That is because they are self explanatory, unlike gpio3 which is
interesting with decoupled control bits, gpio0-2 simply have
3 bits which code 0 - 7, with 3+ being not interesting to us, so
we only use 0-2 which are low / high / input. Still I'll add some
comments for you :)

> I'd be included to add a /* GPIO3 is different from the others */ just
> here too (also: "sigh" re h/w inconsistency...).

Will do.

>> +#define AXP209_GPIO3_OUTPUT_LOW		0x00 /* Drive pin low, Output mode */
>> +#define AXP209_GPIO3_OUTPUT_HIGH	0x02 /* Float pin, Output mode */
>
> Is a floating output really a thing or is this a cut-and-paste-o?

Rather then a push-pull driver, gpio3 seems to have an
open collector / open drain driver, in combination with the
output enable transistor which one would expect on a tristate
driver. So it seems there are 2 ways to float the pin, one by
not driving the output-enable transistor, but also by driving
the output-enable transister, but not pulling the output low,
at least the datasheet describes 2 separate bits one to select
output/input the other to select drive-low/float.

I've chosen to map drvie output, but do not pull output low as
AXP209_GPIO3_OUTPUT_HIGH, and that is what the comment tries
to convey. Note this is all my interpretation of the not so
helpful datasheet.

>> +#define AXP209_GPIO3_INPUT		0x06 /* Float pin, Input mode */
>> +
>> +int axp_gpio_direction_output(unsigned int pin, unsigned int val)
>> +{
>> +	u8 reg = axp209_get_gpio_ctrl_reg(pin);
>> +
>> +	if (val) {
>> +		val = (pin == 3) ? AXP209_GPIO3_OUTPUT_HIGH :
>> +				   AXP209_GPIO_OUTPUT_HIGH;
>> +	} else {
>> +		val = (pin == 3) ? AXP209_GPIO3_OUTPUT_LOW :
>> +				   AXP209_GPIO_OUTPUT_LOW;
>
> Both OUTPUT_LOW values happen to be the same, although I could see why
> you might want to do it this way for consistency.

I also notices the happen to be the same, but I indeed did things
this way for consistency.

Regards,

Hans

[U-Boot] [PATCH 04/17] sunxi: video: Drop disabling of backend / lcdc / hdmi encoder on modeset

[Hans de Goede]

The enable flags are initialized to 0 already by engines_init, so now that
we no longer retry the mode-set, the disabling of the blocks in
sunxi_mode_set() is a no-op.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/video/sunxi_display.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/video/sunxi_display.c b/drivers/video/sunxi_display.c
index a0a0613..ced5ce5 100644
--- a/drivers/video/sunxi_display.c
+++ b/drivers/video/sunxi_display.c
@@ -561,10 +561,6 @@ static void sunxi_mode_set(const struct ctfb_res_modes *mode, char *monitor,
 	int clk_div, clk_double;
 	bool hdmi_mode = strcmp(monitor, "hdmi") == 0;
 
-	clrbits_le32(&hdmi->video_ctrl, SUNXI_HDMI_VIDEO_CTRL_ENABLE);
-	clrbits_le32(&lcdc->ctrl, SUNXI_LCDC_CTRL_TCON_ENABLE);
-	clrbits_le32(&de_be->mode, SUNXI_DE_BE_MODE_START);
-
 	sunxi_composer_mode_set(mode, address);
 	sunxi_lcdc_mode_set(mode, &clk_div, &clk_double);
 	sunxi_hdmi_mode_set(mode, hdmi_mode, clk_div, clk_double);
-- 
2.1.0

[U-Boot] [PATCH 04/17] sunxi: video: Drop disabling of backend / lcdc / hdmi encoder on modeset

[Ian Campbell]

On Wed, 2014-12-24 at 20:06 +0100, Hans de Goede wrote:
> The enable flags are initialized to 0 already by engines_init, so now that
> we no longer retry the mode-set, the disabling of the blocks in
> sunxi_mode_set() is a no-op.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Ian Campbell <ijc@hellion.org.uk>

[U-Boot] [PATCH 05/17] sunxi: video: Improve monitor video-mode option handling

[Hans de Goede]

Add a sunxi_monitor enum and parse the monitor option string into this enum
once, rather then doing strcmp-s on it in various places. This also adds
checking for it being a valid value.

This also adds new "none" and "lcd" values in preparation for lcd support.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/video/sunxi_display.c | 51 +++++++++++++++++++++++++++++--------------
 1 file changed, 35 insertions(+), 16 deletions(-)

diff --git a/drivers/video/sunxi_display.c b/drivers/video/sunxi_display.c
index ced5ce5..83ee360 100644
--- a/drivers/video/sunxi_display.c
+++ b/drivers/video/sunxi_display.c
@@ -21,9 +21,18 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
+enum sunxi_monitor {
+	sunxi_monitor_none,
+	sunxi_monitor_dvi,
+	sunxi_monitor_hdmi,
+	sunxi_monitor_lcd,
+	sunxi_monitor_vga,
+};
+
 struct sunxi_display {
 	GraphicDevice graphic_device;
 	bool enabled;
+	enum sunxi_monitor monitor;
 } sunxi_display;
 
 /*
@@ -159,7 +168,7 @@ static int sunxi_hdmi_edid_get_block(int block, u8 *buf)
 	return r;
 }
 
-static int sunxi_hdmi_edid_get_mode(struct ctfb_res_modes *mode, char *monitor)
+static int sunxi_hdmi_edid_get_mode(struct ctfb_res_modes *mode)
 {
 	struct edid1_info edid1;
 	struct edid_cea861_info cea681[4];
@@ -241,14 +250,14 @@ static int sunxi_hdmi_edid_get_mode(struct ctfb_res_modes *mode, char *monitor)
 	}
 
 	/* Check for basic audio support, if found enable hdmi output */
-	strcpy(monitor, "dvi");
+	sunxi_display.monitor = sunxi_monitor_dvi;
 	for (i = 0; i < ext_blocks; i++) {
 		if (cea681[i].extension_tag != EDID_CEA861_EXTENSION_TAG ||
 		    cea681[i].revision < 2)
 			continue;
 
 		if (EDID_CEA861_SUPPORTS_BASIC_AUDIO(cea681[i]))
-			strcpy(monitor, "hdmi");
+			sunxi_display.monitor = sunxi_monitor_hdmi;
 	}
 
 	return 0;
@@ -489,7 +498,7 @@ static void sunxi_hdmi_setup_info_frames(const struct ctfb_res_modes *mode)
 }
 
 static void sunxi_hdmi_mode_set(const struct ctfb_res_modes *mode,
-				bool hdmi_mode, int clk_div, int clk_double)
+				int clk_div, int clk_double)
 {
 	struct sunxi_hdmi_reg * const hdmi =
 		(struct sunxi_hdmi_reg *)SUNXI_HDMI_BASE;
@@ -498,7 +507,7 @@ static void sunxi_hdmi_mode_set(const struct ctfb_res_modes *mode,
 	/* Write clear interrupt status bits */
 	writel(SUNXI_HDMI_IRQ_STATUS_BITS, &hdmi->irq);
 
-	if (hdmi_mode)
+	if (sunxi_display.monitor == sunxi_monitor_hdmi)
 		sunxi_hdmi_setup_info_frames(mode);
 
 	/* Set input sync enable */
@@ -549,7 +558,7 @@ static void sunxi_engines_init(void)
 #endif
 }
 
-static void sunxi_mode_set(const struct ctfb_res_modes *mode, char *monitor,
+static void sunxi_mode_set(const struct ctfb_res_modes *mode,
 			   unsigned int address)
 {
 	struct sunxi_de_be_reg * const de_be =
@@ -559,11 +568,10 @@ static void sunxi_mode_set(const struct ctfb_res_modes *mode, char *monitor,
 	struct sunxi_hdmi_reg * const hdmi =
 		(struct sunxi_hdmi_reg *)SUNXI_HDMI_BASE;
 	int clk_div, clk_double;
-	bool hdmi_mode = strcmp(monitor, "hdmi") == 0;
 
 	sunxi_composer_mode_set(mode, address);
 	sunxi_lcdc_mode_set(mode, &clk_div, &clk_double);
-	sunxi_hdmi_mode_set(mode, hdmi_mode, clk_div, clk_double);
+	sunxi_hdmi_mode_set(mode, clk_div, clk_double);
 
 	setbits_le32(&de_be->reg_ctrl, SUNXI_DE_BE_REG_CTRL_LOAD_REGS);
 	setbits_le32(&de_be->mode, SUNXI_DE_BE_MODE_START);
@@ -581,8 +589,9 @@ void *video_hw_init(void)
 	struct ctfb_res_modes edid_mode;
 	const char *options;
 	unsigned int depth;
-	int ret, hpd, edid;
-	char monitor[16];
+	int i, ret, hpd, edid;
+	char mon[16];
+	const char *mon_desc[] = { "none", "dvi", "hdmi", "lcd", "vga" };
 
 	memset(&sunxi_display, 0, sizeof(struct sunxi_display));
 
@@ -593,8 +602,18 @@ void *video_hw_init(void)
 	video_get_ctfb_res_modes(RES_MODE_1024x768, 24, &mode, &depth, &options);
 	hpd = video_get_option_int(options, "hpd", 1);
 	edid = video_get_option_int(options, "edid", 1);
-	video_get_option_string(options, "monitor", monitor, sizeof(monitor),
-				"dvi");
+	sunxi_display.monitor = sunxi_monitor_dvi;
+	video_get_option_string(options, "monitor", mon, sizeof(mon),
+				mon_desc[sunxi_display.monitor]);
+	for (i = 0; i < ARRAY_SIZE(mon_desc); i++) {
+		if (strcmp(mon, mon_desc[i]) == 0) {
+			sunxi_display.monitor = i;
+			break;
+		}
+	}
+	if (i == ARRAY_SIZE(mon_desc))
+		printf("Unknown monitor: '%s', falling back to '%s'\n",
+		       mon, mon_desc[sunxi_display.monitor]);
 
 	/* Always call hdp_detect, as it also enables various clocks, etc. */
 	ret = sunxi_hdmi_hpd_detect();
@@ -607,7 +626,7 @@ void *video_hw_init(void)
 
 	/* Check edid if requested and we've a cable plugged in */
 	if (edid && ret) {
-		if (sunxi_hdmi_edid_get_mode(&edid_mode, monitor) == 0)
+		if (sunxi_hdmi_edid_get_mode(&edid_mode) == 0)
 			mode = &edid_mode;
 	}
 
@@ -615,13 +634,13 @@ void *video_hw_init(void)
 		printf("Only non-interlaced modes supported, falling back to 1024x768\n");
 		mode = &res_mode_init[RES_MODE_1024x768];
 	} else {
-		printf("Setting up a %dx%d %s console\n",
-		       mode->xres, mode->yres, monitor);
+		printf("Setting up a %dx%d %s console\n", mode->xres,
+		       mode->yres, mon_desc[sunxi_display.monitor]);
 	}
 
 	sunxi_display.enabled = true;
 	sunxi_engines_init();
-	sunxi_mode_set(mode, monitor, gd->fb_base - CONFIG_SYS_SDRAM_BASE);
+	sunxi_mode_set(mode, gd->fb_base - CONFIG_SYS_SDRAM_BASE);
 
 	/*
 	 * These are the only members of this structure that are used. All the
-- 
2.1.0

[U-Boot] [PATCH 05/17] sunxi: video: Improve monitor video-mode option handling

[Ian Campbell]

On Wed, 2014-12-24 at 20:06 +0100, Hans de Goede wrote:

> +enum sunxi_monitor {
> +	sunxi_monitor_none,
> +	sunxi_monitor_dvi,
> +	sunxi_monitor_hdmi,
> +	sunxi_monitor_lcd,
> +	sunxi_monitor_vga,
> +};
[...]
> +	const char *mon_desc[] = { "none", "dvi", "hdmi", "lcd", "vga" };

These risk getting out of sync. I half expect the array will be punted
by the compiler into static storage anyway, so you could just make it a
global up near the enum (and perhaps use the [sunxi_monitor_none] =
"none" initialiser style).

Another option (which I think I prefer) would be a get_mon_desc helper
with a switch in it over the enum, returning the const char *. Then the
compiler will (hopefully) complain if a new enum is added without a
corresponding description.

The rest all looks fine to me.

[U-Boot] [PATCH 05/17] sunxi: video: Improve monitor video-mode option handling

[Hans de Goede]

Hi,

On 28-12-14 10:40, Ian Campbell wrote:
> On Wed, 2014-12-24 at 20:06 +0100, Hans de Goede wrote:
>
>> +enum sunxi_monitor {
>> +	sunxi_monitor_none,
>> +	sunxi_monitor_dvi,
>> +	sunxi_monitor_hdmi,
>> +	sunxi_monitor_lcd,
>> +	sunxi_monitor_vga,
>> +};
> [...]
>> +	const char *mon_desc[] = { "none", "dvi", "hdmi", "lcd", "vga" };
>
> These risk getting out of sync. I half expect the array will be punted
> by the compiler into static storage anyway, so you could just make it a
> global up near the enum (and perhaps use the [sunxi_monitor_none] =
> "none" initialiser style).
>
> Another option (which I think I prefer) would be a get_mon_desc helper
> with a switch in it over the enum, returning the const char *. Then the
> compiler will (hopefully) complain if a new enum is added without a
> corresponding description.

Yeah the compiler will complain then. I've gone with your suggestion
to add a get_mon_desc helper, note that I did need to also add a
SUNXI_MONITOR_LAST define to use in the loop to compare the user provided
string with the get_mon_desc strings. I've put this directly under the enum
to make it hard to overlook and skip it when updating the enum.

Regards,

Hans

[U-Boot] [PATCH 06/17] sunxi: video: Prepare for lcd support

[Hans de Goede]

Refactor sunxi_mode_set into a bunch of helpers, and make it do a switch
case on sunxi_display.monitor to decide what to do.

Also rename sunxi_lcdc_mode_set to sunxi_lcdc_tcon1_mode_set, as it sets the
timings for tcon1, and for lcd support we need a similar function operating
on tcon0.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/video/sunxi_display.c | 102 +++++++++++++++++++++++++++++-------------
 1 file changed, 70 insertions(+), 32 deletions(-)

diff --git a/drivers/video/sunxi_display.c b/drivers/video/sunxi_display.c
index 83ee360..1dfde11 100644
--- a/drivers/video/sunxi_display.c
+++ b/drivers/video/sunxi_display.c
@@ -310,6 +310,15 @@ static void sunxi_composer_mode_set(const struct ctfb_res_modes *mode,
 	setbits_le32(&de_be->mode, SUNXI_DE_BE_MODE_LAYER0_ENABLE);
 }
 
+static void sunxi_composer_enable(void)
+{
+	struct sunxi_de_be_reg * const de_be =
+		(struct sunxi_de_be_reg *)SUNXI_DE_BE0_BASE;
+
+	setbits_le32(&de_be->reg_ctrl, SUNXI_DE_BE_REG_CTRL_LOAD_REGS);
+	setbits_le32(&de_be->mode, SUNXI_DE_BE_MODE_START);
+}
+
 /*
  * LCDC, what allwinner calls a CRTC, so timing controller and serializer.
  */
@@ -400,8 +409,16 @@ static void sunxi_lcdc_init(void)
 	writel(0xffffffff, &lcdc->tcon1_io_tristate);
 }
 
-static void sunxi_lcdc_mode_set(const struct ctfb_res_modes *mode,
-				int *clk_div, int *clk_double)
+static void sunxi_lcdc_enable(void)
+{
+	struct sunxi_lcdc_reg * const lcdc =
+		(struct sunxi_lcdc_reg *)SUNXI_LCD0_BASE;
+
+	setbits_le32(&lcdc->ctrl, SUNXI_LCDC_CTRL_TCON_ENABLE);
+}
+
+static void sunxi_lcdc_tcon1_mode_set(const struct ctfb_res_modes *mode,
+				      int *clk_div, int *clk_double)
 {
 	struct sunxi_lcdc_reg * const lcdc =
 		(struct sunxi_lcdc_reg *)SUNXI_LCD0_BASE;
@@ -549,6 +566,15 @@ static void sunxi_hdmi_mode_set(const struct ctfb_res_modes *mode,
 		setbits_le32(&hdmi->video_polarity, SUNXI_HDMI_VIDEO_POL_VER);
 }
 
+static void sunxi_hdmi_enable(void)
+{
+	struct sunxi_hdmi_reg * const hdmi =
+		(struct sunxi_hdmi_reg *)SUNXI_HDMI_BASE;
+
+	udelay(100);
+	setbits_le32(&hdmi->video_ctrl, SUNXI_HDMI_VIDEO_CTRL_ENABLE);
+}
+
 static void sunxi_engines_init(void)
 {
 	sunxi_composer_init();
@@ -561,25 +587,26 @@ static void sunxi_engines_init(void)
 static void sunxi_mode_set(const struct ctfb_res_modes *mode,
 			   unsigned int address)
 {
-	struct sunxi_de_be_reg * const de_be =
-		(struct sunxi_de_be_reg *)SUNXI_DE_BE0_BASE;
-	struct sunxi_lcdc_reg * const lcdc =
-		(struct sunxi_lcdc_reg *)SUNXI_LCD0_BASE;
-	struct sunxi_hdmi_reg * const hdmi =
-		(struct sunxi_hdmi_reg *)SUNXI_HDMI_BASE;
-	int clk_div, clk_double;
-
-	sunxi_composer_mode_set(mode, address);
-	sunxi_lcdc_mode_set(mode, &clk_div, &clk_double);
-	sunxi_hdmi_mode_set(mode, clk_div, clk_double);
-
-	setbits_le32(&de_be->reg_ctrl, SUNXI_DE_BE_REG_CTRL_LOAD_REGS);
-	setbits_le32(&de_be->mode, SUNXI_DE_BE_MODE_START);
-	setbits_le32(&lcdc->ctrl, SUNXI_LCDC_CTRL_TCON_ENABLE);
-
-	udelay(100);
-
-	setbits_le32(&hdmi->video_ctrl, SUNXI_HDMI_VIDEO_CTRL_ENABLE);
+	switch (sunxi_display.monitor) {
+	case sunxi_monitor_none:
+		break;
+	case sunxi_monitor_dvi:
+	case sunxi_monitor_hdmi: {
+		int clk_div, clk_double;
+		sunxi_composer_mode_set(mode, address);
+		sunxi_lcdc_tcon1_mode_set(mode, &clk_div, &clk_double);
+		sunxi_hdmi_mode_set(mode, clk_div, clk_double);
+		sunxi_composer_enable();
+		sunxi_lcdc_enable();
+		sunxi_hdmi_enable();
+		}
+		break;
+	case sunxi_monitor_lcd:
+		/* TODO */
+		break;
+	case sunxi_monitor_vga:
+		break;
+	}
 }
 
 void *video_hw_init(void)
@@ -615,19 +642,30 @@ void *video_hw_init(void)
 		printf("Unknown monitor: '%s', falling back to '%s'\n",
 		       mon, mon_desc[sunxi_display.monitor]);
 
-	/* Always call hdp_detect, as it also enables various clocks, etc. */
-	ret = sunxi_hdmi_hpd_detect();
-	if (hpd && !ret) {
-		sunxi_hdmi_shutdown();
+	switch (sunxi_display.monitor) {
+	case sunxi_monitor_none:
 		return NULL;
-	}
-	if (ret)
-		printf("HDMI connected: ");
+	case sunxi_monitor_dvi:
+	case sunxi_monitor_hdmi:
+		/* Always call hdp_detect, as it also enables clocks, etc. */
+		ret = sunxi_hdmi_hpd_detect();
+		if (ret) {
+			printf("HDMI connected: ");
+			if (edid && sunxi_hdmi_edid_get_mode(&edid_mode) == 0)
+				mode = &edid_mode;
+			break;
+		}
+		if (!hpd)
+			break; /* User has requested to ignore hpd */
 
-	/* Check edid if requested and we've a cable plugged in */
-	if (edid && ret) {
-		if (sunxi_hdmi_edid_get_mode(&edid_mode) == 0)
-			mode = &edid_mode;
+		sunxi_hdmi_shutdown();
+		return NULL;
+	case sunxi_monitor_lcd:
+		printf("LCD not supported on this board\n");
+		return NULL;
+	case sunxi_monitor_vga:
+		printf("VGA not supported on this board\n");
+		return NULL;
 	}
 
 	if (mode->vmode != FB_VMODE_NONINTERLACED) {
-- 
2.1.0

[U-Boot] [PATCH 06/17] sunxi: video: Prepare for lcd support

[Ian Campbell]

On Wed, 2014-12-24 at 20:06 +0100, Hans de Goede wrote:
> Refactor sunxi_mode_set into a bunch of helpers, and make it do a switch
> case on sunxi_display.monitor to decide what to do.
> 
> Also rename sunxi_lcdc_mode_set to sunxi_lcdc_tcon1_mode_set, as it sets the
> timings for tcon1, and for lcd support we need a similar function operating
> on tcon0.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Ian Campbell <ijc@hellion.org.uk>

(I've timed out on review bandwidth for right now, I'll get to the rest
later, perhaps this afternoon but more likely in a day or two).

Ian

[U-Boot] [PATCH 07/17] sunxi: video: Modify sunxi_lcdc_pll_set to work with both tcon0 and tcon1

[Hans de Goede]

Modify sunxi_lcdc_pll_set to work with both tcon0 and tcon1, this is a
preparation patch for adding lcd support.

While at it also swap the divider search order, searching from low to
high, as the comment above the code says we should do. In cases where there
are multiple solutions this will result in picking a lower pll clock and
divider, which is more stable and saves power.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 arch/arm/include/asm/arch-sunxi/clock_sun6i.h |  2 ++
 drivers/video/sunxi_display.c                 | 31 +++++++++++++++++++++------
 2 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/arch/arm/include/asm/arch-sunxi/clock_sun6i.h b/arch/arm/include/asm/arch-sunxi/clock_sun6i.h
index 45a199c..f2f1a9b 100644
--- a/arch/arm/include/asm/arch-sunxi/clock_sun6i.h
+++ b/arch/arm/include/asm/arch-sunxi/clock_sun6i.h
@@ -255,6 +255,8 @@ struct sunxi_ccm_reg {
 #define CCM_LCD_CH0_CTRL_PLL3_2X	(2 << 24)
 #define CCM_LCD_CH0_CTRL_PLL7_2X	(3 << 24)
 #define CCM_LCD_CH0_CTRL_MIPI_PLL	(4 << 24)
+/* No reset bit in ch0_clk_cfg (reset is controlled through ahb_reset1) */
+#define CCM_LCD_CH0_CTRL_RST		0
 #define CCM_LCD_CH0_CTRL_GATE		(0x1 << 31)
 
 #define CCM_LCD_CH1_CTRL_M(n)		((((n) - 1) & 0xf) << 0)
diff --git a/drivers/video/sunxi_display.c b/drivers/video/sunxi_display.c
index 1dfde11..aed6ae6 100644
--- a/drivers/video/sunxi_display.c
+++ b/drivers/video/sunxi_display.c
@@ -322,20 +322,29 @@ static void sunxi_composer_enable(void)
 /*
  * LCDC, what allwinner calls a CRTC, so timing controller and serializer.
  */
-static void sunxi_lcdc_pll_set(int dotclock, int *clk_div, int *clk_double)
+static void sunxi_lcdc_pll_set(int tcon, int dotclock,
+			       int *clk_div, int *clk_double)
 {
 	struct sunxi_ccm_reg * const ccm =
 		(struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
-	int value, n, m, diff;
+	int value, n, m, min_m, max_m, diff;
 	int best_n = 0, best_m = 0, best_diff = 0x0FFFFFFF;
 	int best_double = 0;
 
+	if (tcon == 0) {
+		min_m = 6;
+		max_m = 127;
+	} else {
+		min_m = 1;
+		max_m = 15;
+	}
+
 	/*
 	 * Find the lowest divider resulting in a matching clock, if there
 	 * is no match, pick the closest lower clock, as monitors tend to
 	 * not sync to higher frequencies.
 	 */
-	for (m = 15; m > 0; m--) {
+	for (m = min_m; m <= max_m; m++) {
 		n = (m * dotclock) / 3000;
 
 		if ((n >= 9) && (n <= 127)) {
@@ -372,9 +381,17 @@ static void sunxi_lcdc_pll_set(int dotclock, int *clk_div, int *clk_double)
 
 	clock_set_pll3(best_n * 3000000);
 
-	writel(CCM_LCD_CH1_CTRL_GATE |
-	    (best_double ? CCM_LCD_CH1_CTRL_PLL3_2X : CCM_LCD_CH1_CTRL_PLL3) |
-	    CCM_LCD_CH1_CTRL_M(best_m), &ccm->lcd0_ch1_clk_cfg);
+	if (tcon == 0) {
+		writel(CCM_LCD_CH0_CTRL_GATE | CCM_LCD_CH0_CTRL_RST |
+		       (best_double ? CCM_LCD_CH0_CTRL_PLL3_2X :
+				      CCM_LCD_CH0_CTRL_PLL3),
+		       &ccm->lcd0_ch0_clk_cfg);
+	} else {
+		writel(CCM_LCD_CH1_CTRL_GATE |
+		       (best_double ? CCM_LCD_CH1_CTRL_PLL3_2X :
+				      CCM_LCD_CH1_CTRL_PLL3) |
+		       CCM_LCD_CH1_CTRL_M(best_m), &ccm->lcd0_ch1_clk_cfg);
+	}
 
 	*clk_div = best_m;
 	*clk_double = best_double;
@@ -452,7 +469,7 @@ static void sunxi_lcdc_tcon1_mode_set(const struct ctfb_res_modes *mode,
 	writel(SUNXI_LCDC_X(mode->hsync_len) | SUNXI_LCDC_Y(mode->vsync_len),
 	       &lcdc->tcon1_timing_sync);
 
-	sunxi_lcdc_pll_set(mode->pixclock_khz, clk_div, clk_double);
+	sunxi_lcdc_pll_set(1, mode->pixclock_khz, clk_div, clk_double);
 }
 
 #ifdef CONFIG_MACH_SUN6I
-- 
2.1.0

[U-Boot] [PATCH 07/17] sunxi: video: Modify sunxi_lcdc_pll_set to work with both tcon0 and tcon1

[Ian Campbell]

On Wed, 2014-12-24 at 20:06 +0100, Hans de Goede wrote:
> Modify sunxi_lcdc_pll_set to work with both tcon0 and tcon1, this is a
> preparation patch for adding lcd support.
> 
> While at it also swap the divider search order, searching from low to
> high, as the comment above the code says we should do. In cases where there
> are multiple solutions this will result in picking a lower pll clock and
> divider, which is more stable and saves power.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Ian Campbell <ijc@hellion.org.uk>

[U-Boot] [PATCH 08/17] sunxi: video: Move sunxi_drc_init

[Hans de Goede]

Move sunxi_drc_init to directly above sunxi_engines_init, to avoid unnecessary

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/video/sunxi_display.c | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/video/sunxi_display.c b/drivers/video/sunxi_display.c
index aed6ae6..ea7548b 100644
--- a/drivers/video/sunxi_display.c
+++ b/drivers/video/sunxi_display.c
@@ -472,18 +472,6 @@ static void sunxi_lcdc_tcon1_mode_set(const struct ctfb_res_modes *mode,
 	sunxi_lcdc_pll_set(1, mode->pixclock_khz, clk_div, clk_double);
 }
 
-#ifdef CONFIG_MACH_SUN6I
-static void sunxi_drc_init(void)
-{
-	struct sunxi_ccm_reg * const ccm =
-		(struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
-
-	/* On sun6i the drc must be clocked even when in pass-through mode */
-	setbits_le32(&ccm->ahb_reset1_cfg, 1 << AHB_RESET_OFFSET_DRC0);
-	clock_set_de_mod_clock(&ccm->iep_drc0_clk_cfg, 300000000);
-}
-#endif
-
 static void sunxi_hdmi_setup_info_frames(const struct ctfb_res_modes *mode)
 {
 	struct sunxi_hdmi_reg * const hdmi =
@@ -592,13 +580,23 @@ static void sunxi_hdmi_enable(void)
 	setbits_le32(&hdmi->video_ctrl, SUNXI_HDMI_VIDEO_CTRL_ENABLE);
 }
 
+static void sunxi_drc_init(void)
+{
+#ifdef CONFIG_MACH_SUN6I
+	struct sunxi_ccm_reg * const ccm =
+		(struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
+
+	/* On sun6i the drc must be clocked even when in pass-through mode */
+	setbits_le32(&ccm->ahb_reset1_cfg, 1 << AHB_RESET_OFFSET_DRC0);
+	clock_set_de_mod_clock(&ccm->iep_drc0_clk_cfg, 300000000);
+#endif
+}
+
 static void sunxi_engines_init(void)
 {
 	sunxi_composer_init();
 	sunxi_lcdc_init();
-#ifdef CONFIG_MACH_SUN6I
 	sunxi_drc_init();
-#endif
 }
 
 static void sunxi_mode_set(const struct ctfb_res_modes *mode,
-- 
2.1.0

[U-Boot] [PATCH 08/17] sunxi: video: Move sunxi_drc_init

[Chen-Yu Tsai]

On Thu, Dec 25, 2014 at 3:06 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> Move sunxi_drc_init to directly above sunxi_engines_init, to avoid unnecessary

Unfinished commit message?

ChenYu

> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/video/sunxi_display.c | 26 ++++++++++++--------------
>  1 file changed, 12 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/video/sunxi_display.c b/drivers/video/sunxi_display.c
> index aed6ae6..ea7548b 100644
> --- a/drivers/video/sunxi_display.c
> +++ b/drivers/video/sunxi_display.c
> @@ -472,18 +472,6 @@ static void sunxi_lcdc_tcon1_mode_set(const struct ctfb_res_modes *mode,
>         sunxi_lcdc_pll_set(1, mode->pixclock_khz, clk_div, clk_double);
>  }
>
> -#ifdef CONFIG_MACH_SUN6I
> -static void sunxi_drc_init(void)
> -{
> -       struct sunxi_ccm_reg * const ccm =
> -               (struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
> -
> -       /* On sun6i the drc must be clocked even when in pass-through mode */
> -       setbits_le32(&ccm->ahb_reset1_cfg, 1 << AHB_RESET_OFFSET_DRC0);
> -       clock_set_de_mod_clock(&ccm->iep_drc0_clk_cfg, 300000000);
> -}
> -#endif
> -
>  static void sunxi_hdmi_setup_info_frames(const struct ctfb_res_modes *mode)
>  {
>         struct sunxi_hdmi_reg * const hdmi =
> @@ -592,13 +580,23 @@ static void sunxi_hdmi_enable(void)
>         setbits_le32(&hdmi->video_ctrl, SUNXI_HDMI_VIDEO_CTRL_ENABLE);
>  }
>
> +static void sunxi_drc_init(void)
> +{
> +#ifdef CONFIG_MACH_SUN6I
> +       struct sunxi_ccm_reg * const ccm =
> +               (struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
> +
> +       /* On sun6i the drc must be clocked even when in pass-through mode */
> +       setbits_le32(&ccm->ahb_reset1_cfg, 1 << AHB_RESET_OFFSET_DRC0);
> +       clock_set_de_mod_clock(&ccm->iep_drc0_clk_cfg, 300000000);
> +#endif
> +}
> +
>  static void sunxi_engines_init(void)
>  {
>         sunxi_composer_init();
>         sunxi_lcdc_init();
> -#ifdef CONFIG_MACH_SUN6I
>         sunxi_drc_init();
> -#endif
>  }
>
>  static void sunxi_mode_set(const struct ctfb_res_modes *mode,
> --
> 2.1.0
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 08/17] sunxi: video: Move sunxi_drc_init

[Hans de Goede]

Hi,

On 25-12-14 10:08, Chen-Yu Tsai wrote:
> On Thu, Dec 25, 2014 at 3:06 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>> Move sunxi_drc_init to directly above sunxi_engines_init, to avoid unnecessary
>
> Unfinished commit message?

Sortof the next line read:

#ifdef-ery in later patches.

But git commit acted on the # and treated the line as a comment, and I did not
notice this until you pointed it out.

I'll amend the commit message in my personal tree.

Regards,

Hans

>
> ChenYu
>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   drivers/video/sunxi_display.c | 26 ++++++++++++--------------
>>   1 file changed, 12 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/video/sunxi_display.c b/drivers/video/sunxi_display.c
>> index aed6ae6..ea7548b 100644
>> --- a/drivers/video/sunxi_display.c
>> +++ b/drivers/video/sunxi_display.c
>> @@ -472,18 +472,6 @@ static void sunxi_lcdc_tcon1_mode_set(const struct ctfb_res_modes *mode,
>>          sunxi_lcdc_pll_set(1, mode->pixclock_khz, clk_div, clk_double);
>>   }
>>
>> -#ifdef CONFIG_MACH_SUN6I
>> -static void sunxi_drc_init(void)
>> -{
>> -       struct sunxi_ccm_reg * const ccm =
>> -               (struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
>> -
>> -       /* On sun6i the drc must be clocked even when in pass-through mode */
>> -       setbits_le32(&ccm->ahb_reset1_cfg, 1 << AHB_RESET_OFFSET_DRC0);
>> -       clock_set_de_mod_clock(&ccm->iep_drc0_clk_cfg, 300000000);
>> -}
>> -#endif
>> -
>>   static void sunxi_hdmi_setup_info_frames(const struct ctfb_res_modes *mode)
>>   {
>>          struct sunxi_hdmi_reg * const hdmi =
>> @@ -592,13 +580,23 @@ static void sunxi_hdmi_enable(void)
>>          setbits_le32(&hdmi->video_ctrl, SUNXI_HDMI_VIDEO_CTRL_ENABLE);
>>   }
>>
>> +static void sunxi_drc_init(void)
>> +{
>> +#ifdef CONFIG_MACH_SUN6I
>> +       struct sunxi_ccm_reg * const ccm =
>> +               (struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
>> +
>> +       /* On sun6i the drc must be clocked even when in pass-through mode */
>> +       setbits_le32(&ccm->ahb_reset1_cfg, 1 << AHB_RESET_OFFSET_DRC0);
>> +       clock_set_de_mod_clock(&ccm->iep_drc0_clk_cfg, 300000000);
>> +#endif
>> +}
>> +
>>   static void sunxi_engines_init(void)
>>   {
>>          sunxi_composer_init();
>>          sunxi_lcdc_init();
>> -#ifdef CONFIG_MACH_SUN6I
>>          sunxi_drc_init();
>> -#endif
>>   }
>>
>>   static void sunxi_mode_set(const struct ctfb_res_modes *mode,
>> --
>> 2.1.0
>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 08/17] sunxi: video: Move sunxi_drc_init

[Ian Campbell]

On Thu, 2014-12-25 at 11:22 +0100, Hans de Goede wrote:
> Hi,
> 
> On 25-12-14 10:08, Chen-Yu Tsai wrote:
> > On Thu, Dec 25, 2014 at 3:06 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> >> Move sunxi_drc_init to directly above sunxi_engines_init, to avoid unnecessary
> >
> > Unfinished commit message?
> 
> Sortof the next line read:
> 
> #ifdef-ery in later patches.
> 
> But git commit acted on the # and treated the line as a comment,

Awesome!

>  and I did not
> notice this until you pointed it out.
> 
> I'll amend the commit message in my personal tree.

That version:

Acked-by: Ian Campbell <ijc@hellion.org.uk>

[U-Boot] [PATCH 09/17] sunxi: video: Add lcd output support

[Hans de Goede]

Add lcd output support, see the new Kconfig entries and doc/README.video for
how to enable / configure this.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 arch/arm/include/asm/arch-sunxi/display.h |  25 ++++-
 arch/arm/include/asm/arch-sunxi/gpio.h    |   2 +
 board/sunxi/Kconfig                       |  43 ++++++-
 doc/README.video                          |  50 +++++++--
 drivers/video/sunxi_display.c             | 180 ++++++++++++++++++++++++++++--
 5 files changed, 274 insertions(+), 26 deletions(-)

diff --git a/arch/arm/include/asm/arch-sunxi/display.h b/arch/arm/include/asm/arch-sunxi/display.h
index 00e3466..dcb2fe4 100644
--- a/arch/arm/include/asm/arch-sunxi/display.h
+++ b/arch/arm/include/asm/arch-sunxi/display.h
@@ -57,14 +57,13 @@ struct sunxi_lcdc_reg {
 	u32 int0;			/* 0x04 */
 	u32 int1;			/* 0x08 */
 	u8 res0[0x04];			/* 0x0c */
-	u32 frame_ctrl;			/* 0x10 */
-	u8 res1[0x2c];			/* 0x14 */
+	u32 frame_ctrl[12];		/* 0x10 */
 	u32 tcon0_ctrl;			/* 0x40 */
 	u32 tcon0_dclk;			/* 0x44 */
-	u32 tcon0_basic_timing0;	/* 0x48 */
-	u32 tcon0_basic_timing1;	/* 0x4c */
-	u32 tcon0_basic_timing2;	/* 0x50 */
-	u32 tcon0_basic_timing3;	/* 0x54 */
+	u32 tcon0_timing_active;	/* 0x48 */
+	u32 tcon0_timing_h;		/* 0x4c */
+	u32 tcon0_timing_v;		/* 0x50 */
+	u32 tcon0_timing_sync;		/* 0x54 */
 	u32 tcon0_hv_intf;		/* 0x58 */
 	u8 res2[0x04];			/* 0x5c */
 	u32 tcon0_cpu_intf;		/* 0x60 */
@@ -179,7 +178,21 @@ struct sunxi_hdmi_reg {
 #define SUNXI_LCDC_CTRL_IO_MAP_TCON0		(0 << 0)
 #define SUNXI_LCDC_CTRL_IO_MAP_TCON1		(1 << 0)
 #define SUNXI_LCDC_CTRL_TCON_ENABLE		(1 << 31)
+#define SUNXI_LCDC_FRAME_CTRL0_RGB666		((1 << 31) | (0 << 4))
+#define SUNXI_LCDC_FRAME_CTRL0_RGB656		((1 << 31) | (5 << 4))
+#define SUNXI_LCDC_FRAME_CTRL_DITHER0		0x11111111
+#define SUNXI_LCDC_FRAME_CTRL_DITHER1		0x01010000
+#define SUNXI_LCDC_FRAME_CTRL_DITHER2		0x15151111
+#define SUNXI_LCDC_FRAME_CTRL_DITHER3		0x57575555
+#define SUNXI_LCDC_FRAME_CTRL_DITHER4		0x7f7f7777
+#define SUNXI_LCDC_TCON0_CTRL_CLK_DELAY(n)	(((n) & 0x1f) << 4)
+#define SUNXI_LCDC_TCON0_CTRL_ENABLE		(1 << 31)
+#define SUNXI_LCDC_TCON0_DCLK_DIV(n)		((n) << 0)
 #define SUNXI_LCDC_TCON0_DCLK_ENABLE		(0xf << 28)
+#define SUNXI_LCDC_TCON0_TIMING_H_BP(n)		(((n) - 1) << 0)
+#define SUNXI_LCDC_TCON0_TIMING_H_TOTAL(n)	(((n) - 1) << 16)
+#define SUNXI_LCDC_TCON0_TIMING_V_BP(n)		(((n) - 1) << 0)
+#define SUNXI_LCDC_TCON0_TIMING_V_TOTAL(n)	(((n) * 2) << 16)
 #define SUNXI_LCDC_TCON1_CTRL_CLK_DELAY(n)	(((n) & 0x1f) << 4)
 #define SUNXI_LCDC_TCON1_CTRL_ENABLE		(1 << 31)
 #define SUNXI_LCDC_TCON1_TIMING_H_BP(n)		(((n) - 1) << 0)
diff --git a/arch/arm/include/asm/arch-sunxi/gpio.h b/arch/arm/include/asm/arch-sunxi/gpio.h
index 32941cb..9438f5a 100644
--- a/arch/arm/include/asm/arch-sunxi/gpio.h
+++ b/arch/arm/include/asm/arch-sunxi/gpio.h
@@ -150,6 +150,8 @@ enum sunxi_gpio_number {
 
 #define SUNXI_GPC6_SDC2		3
 
+#define SUNXI_GPD0_LCD0		2
+
 #define SUNXI_GPF0_SDC0		2
 
 #define SUNXI_GPF2_SDC0		2
diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
index 5a88ba0..35c59e9 100644
--- a/board/sunxi/Kconfig
+++ b/board/sunxi/Kconfig
@@ -239,17 +239,52 @@ config USB2_VBUS_PIN
 	See USB1_VBUS_PIN help text.
 
 config VIDEO
-	boolean "Enable graphical uboot console on HDMI"
+	boolean "Enable graphical uboot console on HDMI, LCD or VGA"
 	default y
 	---help---
-	Say Y here to add support for using a cfb console on the HDMI output
-	found on most sunxi devices.
+	Say Y here to add support for using a cfb console on the HDMI, LCD
+	or VGA output found on most sunxi devices. See doc/README.video for
+	info on how to select the video output and mode.
+
+config VIDEO_LCD_MODE
+	string "LCD panel timing details"
+	depends on VIDEO
+	default ""
+	---help---
+	LCD panel timing details string, leave empty if there is no LCD panel.
+	This is in drivers/video/videomodes.c: video_get_params() format, e.g.
+	x:800,y:480,depth:18,pclk_khz:33000,le:16,ri:209,up:22,lo:22,hs:30,vs:1,sync:0,vmode:0
+
+config VIDEO_LCD_POWER
+	string "LCD panel power enable pin"
+	depends on VIDEO
+	default ""
+	---help---
+	Set the power enable pin for the LCD panel. This takes a string in the
+	format understood by sunxi_name_to_gpio, e.g. PH1 for pin 1 of port H.
+
+config VIDEO_LCD_BL_EN
+	string "LCD panel backlight enable pin"
+	depends on VIDEO
+	default ""
+	---help---
+	Set the backlight enable pin for the LCD panel. This takes a string in the
+	the format understood by sunxi_name_to_gpio, e.g. PH1 for pin 1 of
+	port H.
+
+config VIDEO_LCD_BL_PWM
+	string "LCD panel backlight pwm pin"
+	depends on VIDEO
+	default ""
+	---help---
+	Set the backlight pwm pin for the LCD panel. This takes a string in the
+	format understood by sunxi_name_to_gpio, e.g. PH1 for pin 1 of port H.
 
 config USB_KEYBOARD
 	boolean "Enable USB keyboard support"
 	default y
 	---help---
 	Say Y here to add support for using a USB keyboard (typically used
-	in combination with a graphical console on HDMI).
+	in combination with a graphical console).
 
 endif
diff --git a/doc/README.video b/doc/README.video
index dadbfcd..cfe6318 100644
--- a/doc/README.video
+++ b/doc/README.video
@@ -5,15 +5,8 @@
  * SPDX-License-Identifier:	GPL-2.0+
  */
 
-U-Boot MPC8xx video controller driver
-======================================
-
-The driver has been tested with the following configurations:
-
-- MPC823FADS with AD7176 on a PAL TV (YCbYCr)	- arsenio@tin.it
-
 "video-mode" environment variable
-===============================
+=================================
 
 The 'video-mode' environment variable can be used to enable and configure
 some video drivers.  The format matches the video= command-line option used
@@ -28,4 +21,45 @@ for Linux:
 	<freq>		The frequency (in Hz) to use.
 	<options>	A comma-separated list of device-specific options
 
+
+U-Boot MPC8xx video controller driver
+=====================================
+
+The driver has been tested with the following configurations:
+
+- MPC823FADS with AD7176 on a PAL TV (YCbYCr)	- arsenio at tin.it
+
 Example: video-mode=fslfb:1280x1024-32 at 60,monitor=dvi
+
+
+U-boot sunxi video controller driver
+====================================
+
+U-boot supports hdmi and lcd output on Allwinner sunxi SoCs, lcd output
+requires the CONFIG_VIDEO_LCD_MODE Kconfig value to be set.
+
+The sunxi u-boot driver supports the following video-mode options:
+
+- monitor=[none|dvi|hdmi|lcd] - Select the video output to use
+ none:     Disable video output.
+ dvi/hdmi: Selects output over the hdmi connector with dvi resp. hdmi output
+           format, if edid is used the format is automatically selected.
+ lcd:      Selects video output to a LCD screen.
+ vga:      Selects bideo output over the VGA connector.
+ Defaults to monitor=dvi.
+
+- hpd=[0|1] - Enable use of the hdmi HotPlug Detect feature
+ 0: Disabled. Configure dvi/hdmi output even if no cable is detected
+ 1: Enabled. If a LCD has been configured fallback to the LCD when no cable is
+    detected, if no LCD is configured, disable video ouput.
+ Defaults to hpd=1.
+
+- edid=[0|1] - Enable use of DDC + EDID to get monitor info
+ 0: Disabled.
+ 1: Enabled. If valid EDID info was read from the monitor the EDID info will
+    overrides the xres, yres and refresh from the video-mode env. variable.
+ Defaults to edid=1.
+
+For example to always use the hdmi connector, even if no cable is inserted,
+using edid info when available and otherwise initalizing it at 1024x768 at 60Hz,
+use: video-mode=sunxi:1024x768-24 at 60,monitor=dvi,hpd=0,edid=1 .
diff --git a/drivers/video/sunxi_display.c b/drivers/video/sunxi_display.c
index ea7548b..bfcfd3f 100644
--- a/drivers/video/sunxi_display.c
+++ b/drivers/video/sunxi_display.c
@@ -11,7 +11,9 @@
 
 #include <asm/arch/clock.h>
 #include <asm/arch/display.h>
+#include <asm/arch/gpio.h>
 #include <asm/global_data.h>
+#include <asm/gpio.h>
 #include <asm/io.h>
 #include <errno.h>
 #include <fdtdec.h>
@@ -33,6 +35,7 @@ struct sunxi_display {
 	GraphicDevice graphic_device;
 	bool enabled;
 	enum sunxi_monitor monitor;
+	unsigned int depth;
 } sunxi_display;
 
 /*
@@ -434,6 +437,136 @@ static void sunxi_lcdc_enable(void)
 	setbits_le32(&lcdc->ctrl, SUNXI_LCDC_CTRL_TCON_ENABLE);
 }
 
+static void sunxi_lcdc_panel_enable(void)
+{
+	int pin;
+
+	/*
+	 * Start with backlight disabled to avoid the screen flashing to
+	 * white while the lcd inits.
+	 */
+	pin = sunxi_name_to_gpio(CONFIG_VIDEO_LCD_BL_EN);
+	if (pin != -1) {
+		gpio_request(pin, "lcd_backlight_enable");
+		gpio_direction_output(pin, 0);
+	}
+
+	pin = sunxi_name_to_gpio(CONFIG_VIDEO_LCD_BL_PWM);
+	if (pin != -1) {
+		gpio_request(pin, "lcd_backlight_pwm");
+		/* backlight pwm is inverted, set to 1 to disable backlight */
+		gpio_direction_output(pin, 1);
+	}
+
+	/* Give the backlight some time to turn off and power up the panel. */
+	mdelay(40);
+	pin = sunxi_name_to_gpio(CONFIG_VIDEO_LCD_POWER);
+	if (pin != -1) {
+		gpio_request(pin, "lcd_power");
+		gpio_direction_output(pin, 1);
+	}
+}
+
+static void sunxi_lcdc_backlight_enable(void)
+{
+	int pin;
+
+	/*
+	 * We want to have scanned out atleast one frame before enabling the
+	 * backlight to avoid the screen flashing to white when we enable it.
+	 */
+	mdelay(40);
+
+	pin = sunxi_name_to_gpio(CONFIG_VIDEO_LCD_BL_EN);
+	if (pin != -1) {
+		gpio_request(pin, "lcd_backlight_enable");
+		gpio_direction_output(pin, 1);
+	}
+
+	pin = sunxi_name_to_gpio(CONFIG_VIDEO_LCD_BL_PWM);
+	if (pin != -1) {
+		gpio_request(pin, "lcd_backlight_pwm");
+		/* backlight pwm is inverted, set to 0 to enable backlight */
+		gpio_direction_output(pin, 0);
+	}
+}
+
+static int sunxi_lcdc_get_clk_delay(const struct ctfb_res_modes *mode)
+{
+	int delay;
+
+	delay = mode->lower_margin + mode->vsync_len + mode->upper_margin - 2;
+	return (delay > 30) ? 30 : delay;
+}
+
+static void sunxi_lcdc_tcon0_mode_set(const struct ctfb_res_modes *mode)
+{
+	struct sunxi_lcdc_reg * const lcdc =
+		(struct sunxi_lcdc_reg *)SUNXI_LCD0_BASE;
+	int bp, clk_delay, clk_div, clk_double, pin, total;
+
+	for (pin = SUNXI_GPD(0); pin <= SUNXI_GPD(27); pin++)
+		sunxi_gpio_set_cfgpin(pin, SUNXI_GPD0_LCD0);
+
+	sunxi_lcdc_pll_set(0, mode->pixclock_khz, &clk_div, &clk_double);
+
+	/* Use tcon0 */
+	clrsetbits_le32(&lcdc->ctrl, SUNXI_LCDC_CTRL_IO_MAP_MASK,
+			SUNXI_LCDC_CTRL_IO_MAP_TCON0);
+
+	clk_delay = sunxi_lcdc_get_clk_delay(mode);
+	writel(SUNXI_LCDC_TCON0_CTRL_ENABLE |
+	       SUNXI_LCDC_TCON0_CTRL_CLK_DELAY(clk_delay), &lcdc->tcon0_ctrl);
+
+	writel(SUNXI_LCDC_TCON0_DCLK_ENABLE |
+	       SUNXI_LCDC_TCON0_DCLK_DIV(clk_div), &lcdc->tcon0_dclk);
+
+	writel(SUNXI_LCDC_X(mode->xres) | SUNXI_LCDC_Y(mode->yres),
+	       &lcdc->tcon0_timing_active);
+
+	bp = mode->hsync_len + mode->left_margin;
+	total = mode->xres + mode->right_margin + bp;
+	writel(SUNXI_LCDC_TCON0_TIMING_H_TOTAL(total) |
+	       SUNXI_LCDC_TCON0_TIMING_H_BP(bp), &lcdc->tcon0_timing_h);
+
+	bp = mode->vsync_len + mode->upper_margin;
+	total = mode->yres + mode->lower_margin + bp;
+	writel(SUNXI_LCDC_TCON0_TIMING_V_TOTAL(total) |
+	       SUNXI_LCDC_TCON0_TIMING_V_BP(bp), &lcdc->tcon0_timing_v);
+
+	writel(SUNXI_LCDC_X(mode->hsync_len) | SUNXI_LCDC_Y(mode->vsync_len),
+	       &lcdc->tcon0_timing_sync);
+
+	/* We only support hv-sync parallel lcd-s for now */
+	writel(0, &lcdc->tcon0_hv_intf);
+	writel(0, &lcdc->tcon0_cpu_intf);
+
+	if (sunxi_display.depth == 18 || sunxi_display.depth == 17) {
+		writel(SUNXI_LCDC_FRAME_CTRL_DITHER0, &lcdc->frame_ctrl[1]);
+		writel(SUNXI_LCDC_FRAME_CTRL_DITHER0, &lcdc->frame_ctrl[2]);
+		writel(SUNXI_LCDC_FRAME_CTRL_DITHER0, &lcdc->frame_ctrl[3]);
+		writel(SUNXI_LCDC_FRAME_CTRL_DITHER0, &lcdc->frame_ctrl[4]);
+		writel(SUNXI_LCDC_FRAME_CTRL_DITHER0, &lcdc->frame_ctrl[5]);
+		writel(SUNXI_LCDC_FRAME_CTRL_DITHER0, &lcdc->frame_ctrl[6]);
+		writel(SUNXI_LCDC_FRAME_CTRL_DITHER1, &lcdc->frame_ctrl[7]);
+		writel(SUNXI_LCDC_FRAME_CTRL_DITHER2, &lcdc->frame_ctrl[8]);
+		writel(SUNXI_LCDC_FRAME_CTRL_DITHER3, &lcdc->frame_ctrl[9]);
+		writel(SUNXI_LCDC_FRAME_CTRL_DITHER4, &lcdc->frame_ctrl[10]);
+		writel(((sunxi_display.depth == 18) ?
+			SUNXI_LCDC_FRAME_CTRL0_RGB666 :
+			SUNXI_LCDC_FRAME_CTRL0_RGB656),
+		       &lcdc->frame_ctrl[0]);
+	}
+
+	/*
+	 * Bit 24 and 25 of tcon0_io_polarity can be used to invert hsync /
+	 * vsync polarity, but this leads to noise problems, so we always
+	 * keep the polarity positive.
+	 */
+	writel(0, &lcdc->tcon0_io_polarity);
+	writel(0, &lcdc->tcon0_io_tristate);
+}
+
 static void sunxi_lcdc_tcon1_mode_set(const struct ctfb_res_modes *mode,
 				      int *clk_div, int *clk_double)
 {
@@ -617,7 +750,12 @@ static void sunxi_mode_set(const struct ctfb_res_modes *mode,
 		}
 		break;
 	case sunxi_monitor_lcd:
-		/* TODO */
+		sunxi_lcdc_panel_enable();
+		sunxi_composer_mode_set(mode, address);
+		sunxi_lcdc_tcon0_mode_set(mode);
+		sunxi_composer_enable();
+		sunxi_lcdc_enable();
+		sunxi_lcdc_backlight_enable();
 		break;
 	case sunxi_monitor_vga:
 		break;
@@ -628,12 +766,12 @@ void *video_hw_init(void)
 {
 	static GraphicDevice *graphic_device = &sunxi_display.graphic_device;
 	const struct ctfb_res_modes *mode;
-	struct ctfb_res_modes edid_mode;
+	struct ctfb_res_modes custom;
 	const char *options;
-	unsigned int depth;
 	int i, ret, hpd, edid;
 	char mon[16];
 	const char *mon_desc[] = { "none", "dvi", "hdmi", "lcd", "vga" };
+	char *lcd_mode = CONFIG_VIDEO_LCD_MODE;
 
 	memset(&sunxi_display, 0, sizeof(struct sunxi_display));
 
@@ -641,7 +779,8 @@ void *video_hw_init(void)
 	       CONFIG_SUNXI_FB_SIZE >> 10);
 	gd->fb_base = gd->ram_top;
 
-	video_get_ctfb_res_modes(RES_MODE_1024x768, 24, &mode, &depth, &options);
+	video_get_ctfb_res_modes(RES_MODE_1024x768, 24, &mode,
+				 &sunxi_display.depth, &options);
 	hpd = video_get_option_int(options, "hpd", 1);
 	edid = video_get_option_int(options, "edid", 1);
 	sunxi_display.monitor = sunxi_monitor_dvi;
@@ -666,16 +805,26 @@ void *video_hw_init(void)
 		ret = sunxi_hdmi_hpd_detect();
 		if (ret) {
 			printf("HDMI connected: ");
-			if (edid && sunxi_hdmi_edid_get_mode(&edid_mode) == 0)
-				mode = &edid_mode;
+			if (edid && sunxi_hdmi_edid_get_mode(&custom) == 0)
+				mode = &custom;
 			break;
 		}
 		if (!hpd)
 			break; /* User has requested to ignore hpd */
 
 		sunxi_hdmi_shutdown();
-		return NULL;
+
+		if (lcd_mode[0] == 0)
+			return NULL; /* No LCD, bail */
+
+		/* Fall back / through to LCD */
+		sunxi_display.monitor = sunxi_monitor_lcd;
 	case sunxi_monitor_lcd:
+		if (lcd_mode[0]) {
+			sunxi_display.depth = video_get_params(&custom, lcd_mode);
+			mode = &custom;
+			break;
+		}
 		printf("LCD not supported on this board\n");
 		return NULL;
 	case sunxi_monitor_vga:
@@ -717,16 +866,31 @@ int sunxi_simplefb_setup(void *blob)
 {
 	static GraphicDevice *graphic_device = &sunxi_display.graphic_device;
 	int offset, ret;
+	const char *pipeline = NULL;
 
 	if (!sunxi_display.enabled)
 		return 0;
 
+	switch (sunxi_display.monitor) {
+	case sunxi_monitor_none:
+		return 0;
+	case sunxi_monitor_dvi:
+	case sunxi_monitor_hdmi:
+		pipeline = "de_be0-lcd0-hdmi";
+		break;
+	case sunxi_monitor_lcd:
+		pipeline = "de_be0-lcd0";
+		break;
+	case sunxi_monitor_vga:
+		break;
+	}
+
 	/* Find a framebuffer node, with pipeline == "de_be0-lcd0-hdmi" */
 	offset = fdt_node_offset_by_compatible(blob, -1,
 					       "allwinner,simple-framebuffer");
 	while (offset >= 0) {
 		ret = fdt_find_string(blob, offset, "allwinner,pipeline",
-				      "de_be0-lcd0-hdmi");
+				      pipeline);
 		if (ret == 0)
 			break;
 		offset = fdt_node_offset_by_compatible(blob, offset,
-- 
2.1.0

[U-Boot] [PATCH 09/17] sunxi: video: Add lcd output support

[Ian Campbell]

On Wed, 2014-12-24 at 20:06 +0100, Hans de Goede wrote:

> +static void sunxi_lcdc_panel_enable(void)
> +{
> +	int pin;
> +
> +	/*
> +	 * Start with backlight disabled to avoid the screen flashing to
> +	 * white while the lcd inits.
> +	 */
> +	pin = sunxi_name_to_gpio(CONFIG_VIDEO_LCD_BL_EN);
> +	if (pin != -1) {
> +		gpio_request(pin, "lcd_backlight_enable");

You request this and the backlight pwm again in backlight_enable. Are
such multiple requests OK?

> +		gpio_direction_output(pin, 0);
> +	}
> +
> +	pin = sunxi_name_to_gpio(CONFIG_VIDEO_LCD_BL_PWM);
> +	if (pin != -1) {
> +		gpio_request(pin, "lcd_backlight_pwm");
> +		/* backlight pwm is inverted, set to 1 to disable backlight */
> +		gpio_direction_output(pin, 1);
> +	}
> +
> +	/* Give the backlight some time to turn off and power up the panel. */
> +	mdelay(40);
> +	pin = sunxi_name_to_gpio(CONFIG_VIDEO_LCD_POWER);
> +	if (pin != -1) {
> +		gpio_request(pin, "lcd_power");
> +		gpio_direction_output(pin, 1);
> +	}
> +}
> +
> +static void sunxi_lcdc_backlight_enable(void)
> +{
> +	int pin;
> +
> +	/*
> +	 * We want to have scanned out atleast one frame before enabling the

"at least".

> +	 * backlight to avoid the screen flashing to white when we enable it.
> +	 */
> +	mdelay(40);

I take it there isn't a vsync status bit or something we can watch for?

> [...] 
> +	switch (sunxi_display.monitor) {
> +	case sunxi_monitor_none:
> +		return 0;
> +	case sunxi_monitor_dvi:
> +	case sunxi_monitor_hdmi:
> +		pipeline = "de_be0-lcd0-hdmi";
> +		break;
> +	case sunxi_monitor_lcd:
> +		pipeline = "de_be0-lcd0";
> +		break;
> +	case sunxi_monitor_vga:
> +		break;
> +	}
> +
>  	/* Find a framebuffer node, with pipeline == "de_be0-lcd0-hdmi" */

This comment is no longer accurate.

>  	offset = fdt_node_offset_by_compatible(blob, -1,
>  					       "allwinner,simple-framebuffer");
>  	while (offset >= 0) {
>  		ret = fdt_find_string(blob, offset, "allwinner,pipeline",
> -				      "de_be0-lcd0-hdmi");
> +				      pipeline);
>  		if (ret == 0)
>  			break;
>  		offset = fdt_node_offset_by_compatible(blob, offset,

Ian.

[U-Boot] [PATCH 09/17] sunxi: video: Add lcd output support

[Hans de Goede]

Hi,

On 29-12-14 14:43, Ian Campbell wrote:
> On Wed, 2014-12-24 at 20:06 +0100, Hans de Goede wrote:
>
>> +static void sunxi_lcdc_panel_enable(void)
>> +{
>> +	int pin;
>> +
>> +	/*
>> +	 * Start with backlight disabled to avoid the screen flashing to
>> +	 * white while the lcd inits.
>> +	 */
>> +	pin = sunxi_name_to_gpio(CONFIG_VIDEO_LCD_BL_EN);
>> +	if (pin != -1) {
>> +		gpio_request(pin, "lcd_backlight_enable");
>
> You request this and the backlight pwm again in backlight_enable. Are
> such multiple requests OK?

No, not when using the device_model, I've removed the 2nd gpio_request
call done for both from backlight_enable. I'll send a v2 with this fixed.

>> +		gpio_direction_output(pin, 0);
>> +	}
>> +
>> +	pin = sunxi_name_to_gpio(CONFIG_VIDEO_LCD_BL_PWM);
>> +	if (pin != -1) {
>> +		gpio_request(pin, "lcd_backlight_pwm");
>> +		/* backlight pwm is inverted, set to 1 to disable backlight */
>> +		gpio_direction_output(pin, 1);
>> +	}
>> +
>> +	/* Give the backlight some time to turn off and power up the panel. */
>> +	mdelay(40);
>> +	pin = sunxi_name_to_gpio(CONFIG_VIDEO_LCD_POWER);
>> +	if (pin != -1) {
>> +		gpio_request(pin, "lcd_power");
>> +		gpio_direction_output(pin, 1);
>> +	}
>> +}
>> +
>> +static void sunxi_lcdc_backlight_enable(void)
>> +{
>> +	int pin;
>> +
>> +	/*
>> +	 * We want to have scanned out atleast one frame before enabling the
>
> "at least".

Fixed for v2.

>> +	 * backlight to avoid the screen flashing to white when we enable it.
>> +	 */
>> +	mdelay(40);
>
> I take it there isn't a vsync status bit or something we can watch for?

There is a line interrupt, which we could theoretically use, but the docs are
rather lacking, so I believe this is the best solution for now.

>
>> [...]
>> +	switch (sunxi_display.monitor) {
>> +	case sunxi_monitor_none:
>> +		return 0;
>> +	case sunxi_monitor_dvi:
>> +	case sunxi_monitor_hdmi:
>> +		pipeline = "de_be0-lcd0-hdmi";
>> +		break;
>> +	case sunxi_monitor_lcd:
>> +		pipeline = "de_be0-lcd0";
>> +		break;
>> +	case sunxi_monitor_vga:
>> +		break;
>> +	}
>> +
>>   	/* Find a framebuffer node, with pipeline == "de_be0-lcd0-hdmi" */
>
> This comment is no longer accurate.

Fixed for v2.

>>   	offset = fdt_node_offset_by_compatible(blob, -1,
>>   					       "allwinner,simple-framebuffer");
>>   	while (offset >= 0) {
>>   		ret = fdt_find_string(blob, offset, "allwinner,pipeline",
>> -				      "de_be0-lcd0-hdmi");
>> +				      pipeline);
>>   		if (ret == 0)
>>   			break;
>>   		offset = fdt_node_offset_by_compatible(blob, offset,
>
> Ian.

Regards,

Hans

[U-Boot] [PATCH 10/17] sunxi: video: Add suppport SoCs without HDMI, e.g. the A13 and A23

[Hans de Goede]

Some SoCs, specifically the A13 (sun5i variant) and the A23 (sun8i) only have
lcd output support.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 board/sunxi/Kconfig           |  7 +++++++
 drivers/video/sunxi_display.c | 30 ++++++++++++++++++++++++++----
 2 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
index 35c59e9..7a8503b 100644
--- a/board/sunxi/Kconfig
+++ b/board/sunxi/Kconfig
@@ -246,6 +246,13 @@ config VIDEO
 	or VGA output found on most sunxi devices. See doc/README.video for
 	info on how to select the video output and mode.
 
+config VIDEO_HDMI
+	boolean "HDMI output support"
+	depends on VIDEO && !MACH_SUN8I
+	default y
+	---help---
+	Say Y here to add support for outputting video over HDMI.
+
 config VIDEO_LCD_MODE
 	string "LCD panel timing details"
 	depends on VIDEO
diff --git a/drivers/video/sunxi_display.c b/drivers/video/sunxi_display.c
index bfcfd3f..2822cc6 100644
--- a/drivers/video/sunxi_display.c
+++ b/drivers/video/sunxi_display.c
@@ -38,6 +38,8 @@ struct sunxi_display {
 	unsigned int depth;
 } sunxi_display;
 
+#ifdef CONFIG_VIDEO_HDMI
+
 /*
  * Wait up to 200ms for value to be set in given part of reg.
  */
@@ -266,6 +268,8 @@ static int sunxi_hdmi_edid_get_mode(struct ctfb_res_modes *mode)
 	return 0;
 }
 
+#endif /* CONFIG_VIDEO_HDMI */
+
 /*
  * This is the entity that mixes and matches the different layers and inputs.
  * Allwinner calls it the back-end, but i like composer better.
@@ -278,7 +282,7 @@ static void sunxi_composer_init(void)
 		(struct sunxi_de_be_reg *)SUNXI_DE_BE0_BASE;
 	int i;
 
-#ifdef CONFIG_MACH_SUN6I
+#if defined CONFIG_MACH_SUN6I || defined CONFIG_MACH_SUN8I
 	/* Reset off */
 	setbits_le32(&ccm->ahb_reset1_cfg, 1 << AHB_RESET_OFFSET_DE_BE0);
 #endif
@@ -408,7 +412,7 @@ static void sunxi_lcdc_init(void)
 		(struct sunxi_lcdc_reg *)SUNXI_LCD0_BASE;
 
 	/* Reset off */
-#ifdef CONFIG_MACH_SUN6I
+#if defined CONFIG_MACH_SUN6I || defined CONFIG_MACH_SUN8I
 	setbits_le32(&ccm->ahb_reset1_cfg, 1 << AHB_RESET_OFFSET_LCD0);
 #else
 	setbits_le32(&ccm->lcd0_ch0_clk_cfg, CCM_LCD_CH0_CTRL_RST);
@@ -567,6 +571,8 @@ static void sunxi_lcdc_tcon0_mode_set(const struct ctfb_res_modes *mode)
 	writel(0, &lcdc->tcon0_io_tristate);
 }
 
+#ifdef CONFIG_VIDEO_HDMI
+
 static void sunxi_lcdc_tcon1_mode_set(const struct ctfb_res_modes *mode,
 				      int *clk_div, int *clk_double)
 {
@@ -713,9 +719,11 @@ static void sunxi_hdmi_enable(void)
 	setbits_le32(&hdmi->video_ctrl, SUNXI_HDMI_VIDEO_CTRL_ENABLE);
 }
 
+#endif /* CONFIG_VIDEO_HDMI */
+
 static void sunxi_drc_init(void)
 {
-#ifdef CONFIG_MACH_SUN6I
+#if defined CONFIG_MACH_SUN6I || defined CONFIG_MACH_SUN8I
 	struct sunxi_ccm_reg * const ccm =
 		(struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
 
@@ -740,6 +748,7 @@ static void sunxi_mode_set(const struct ctfb_res_modes *mode,
 		break;
 	case sunxi_monitor_dvi:
 	case sunxi_monitor_hdmi: {
+#ifdef CONFIG_VIDEO_HDMI
 		int clk_div, clk_double;
 		sunxi_composer_mode_set(mode, address);
 		sunxi_lcdc_tcon1_mode_set(mode, &clk_div, &clk_double);
@@ -747,6 +756,7 @@ static void sunxi_mode_set(const struct ctfb_res_modes *mode,
 		sunxi_composer_enable();
 		sunxi_lcdc_enable();
 		sunxi_hdmi_enable();
+#endif
 		}
 		break;
 	case sunxi_monitor_lcd:
@@ -768,10 +778,13 @@ void *video_hw_init(void)
 	const struct ctfb_res_modes *mode;
 	struct ctfb_res_modes custom;
 	const char *options;
-	int i, ret, hpd, edid;
+#ifdef CONFIG_VIDEO_HDMI
+	int ret, hpd, edid;
+#endif
 	char mon[16];
 	const char *mon_desc[] = { "none", "dvi", "hdmi", "lcd", "vga" };
 	char *lcd_mode = CONFIG_VIDEO_LCD_MODE;
+	int i;
 
 	memset(&sunxi_display, 0, sizeof(struct sunxi_display));
 
@@ -781,9 +794,13 @@ void *video_hw_init(void)
 
 	video_get_ctfb_res_modes(RES_MODE_1024x768, 24, &mode,
 				 &sunxi_display.depth, &options);
+#ifdef CONFIG_VIDEO_HDMI
 	hpd = video_get_option_int(options, "hpd", 1);
 	edid = video_get_option_int(options, "edid", 1);
 	sunxi_display.monitor = sunxi_monitor_dvi;
+#else
+	sunxi_display.monitor = sunxi_monitor_lcd;
+#endif
 	video_get_option_string(options, "monitor", mon, sizeof(mon),
 				mon_desc[sunxi_display.monitor]);
 	for (i = 0; i < ARRAY_SIZE(mon_desc); i++) {
@@ -801,6 +818,10 @@ void *video_hw_init(void)
 		return NULL;
 	case sunxi_monitor_dvi:
 	case sunxi_monitor_hdmi:
+#ifndef CONFIG_VIDEO_HDMI
+		printf("HDMI/DVI not supported on this board\n");
+		return NULL;
+#else
 		/* Always call hdp_detect, as it also enables clocks, etc. */
 		ret = sunxi_hdmi_hpd_detect();
 		if (ret) {
@@ -819,6 +840,7 @@ void *video_hw_init(void)
 
 		/* Fall back / through to LCD */
 		sunxi_display.monitor = sunxi_monitor_lcd;
+#endif
 	case sunxi_monitor_lcd:
 		if (lcd_mode[0]) {
 			sunxi_display.depth = video_get_params(&custom, lcd_mode);
-- 
2.1.0

[U-Boot] [PATCH 10/17] sunxi: video: Add suppport SoCs without HDMI, e.g. the A13 and A23

[Ian Campbell]

On Wed, 2014-12-24 at 20:06 +0100, Hans de Goede wrote:

You've got three p's in suppport in the subject.

> Some SoCs, specifically the A13 (sun5i variant) and the A23 (sun8i) only have
> lcd output support.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  board/sunxi/Kconfig           |  7 +++++++
>  drivers/video/sunxi_display.c | 30 ++++++++++++++++++++++++++----
>  2 files changed, 33 insertions(+), 4 deletions(-)
> 
> diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
> index 35c59e9..7a8503b 100644
> --- a/board/sunxi/Kconfig
> +++ b/board/sunxi/Kconfig
> @@ -246,6 +246,13 @@ config VIDEO
>  	or VGA output found on most sunxi devices. See doc/README.video for
>  	info on how to select the video output and mode.
>  
> +config VIDEO_HDMI
> +	boolean "HDMI output support"
> +	depends on VIDEO && !MACH_SUN8I

There is no "&& !MACH_SUN5I" here because there are sun5i variants other
than the A13 which do have HDMI, correct?

> @@ -278,7 +282,7 @@ static void sunxi_composer_init(void)
>  		(struct sunxi_de_be_reg *)SUNXI_DE_BE0_BASE;
>  	int i;
>  
> -#ifdef CONFIG_MACH_SUN6I
> +#if defined CONFIG_MACH_SUN6I || defined CONFIG_MACH_SUN8I

These seem a bit odd. If the sun8i has no HDMI why are we messing with
the reset bits? (Or at least, why is such messing new in this particular
patch).

I think perhaps rather than "disabling the non-existent HDMI on sun8i"
which is how I read the patch description I should have read "enable
video on sun8i, which supports LCD but not HDMI"?

Ian.

[U-Boot] [PATCH 10/17] sunxi: video: Add suppport SoCs without HDMI, e.g. the A13 and A23

[Hans de Goede]

Hi,

On 29-12-14 14:50, Ian Campbell wrote:
> On Wed, 2014-12-24 at 20:06 +0100, Hans de Goede wrote:
>
> You've got three p's in suppport in the subject.
>
>> Some SoCs, specifically the A13 (sun5i variant) and the A23 (sun8i) only have
>> lcd output support.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   board/sunxi/Kconfig           |  7 +++++++
>>   drivers/video/sunxi_display.c | 30 ++++++++++++++++++++++++++----
>>   2 files changed, 33 insertions(+), 4 deletions(-)
>>
>> diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
>> index 35c59e9..7a8503b 100644
>> --- a/board/sunxi/Kconfig
>> +++ b/board/sunxi/Kconfig
>> @@ -246,6 +246,13 @@ config VIDEO
>>   	or VGA output found on most sunxi devices. See doc/README.video for
>>   	info on how to select the video output and mode.
>>
>> +config VIDEO_HDMI
>> +	boolean "HDMI output support"
>> +	depends on VIDEO && !MACH_SUN8I
>
> There is no "&& !MACH_SUN5I" here because there are sun5i variants other
> than the A13 which do have HDMI, correct?

Correct, the A10s has HDMI out support.

>> @@ -278,7 +282,7 @@ static void sunxi_composer_init(void)
>>   		(struct sunxi_de_be_reg *)SUNXI_DE_BE0_BASE;
>>   	int i;
>>
>> -#ifdef CONFIG_MACH_SUN6I
>> +#if defined CONFIG_MACH_SUN6I || defined CONFIG_MACH_SUN8I
>
> These seem a bit odd. If the sun8i has no HDMI why are we messing with
> the reset bits? (Or at least, why is such messing new in this particular
> patch).
>
> I think perhaps rather than "disabling the non-existent HDMI on sun8i"
> which is how I read the patch description I should have read "enable
> video on sun8i, which supports LCD but not HDMI"?

Yeah that is better, I'll do a v2 with the commit message fixed.

Regards,

Hans

[U-Boot] [PATCH 11/17] sunxi: video: Add support for VGA via external DACs connected to the LCD pins

[Hans de Goede]

Add support for external DACs connected to the parallel LCD interface driving
a VGA connector, such as found on the Olimex A13 boards.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 board/sunxi/Kconfig           |  9 +++++++++
 drivers/video/sunxi_display.c | 14 ++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
index 7a8503b..3e42866 100644
--- a/board/sunxi/Kconfig
+++ b/board/sunxi/Kconfig
@@ -253,6 +253,15 @@ config VIDEO_HDMI
 	---help---
 	Say Y here to add support for outputting video over HDMI.
 
+config VIDEO_VGA_VIA_LCD
+	boolean "VGA via LCD controller support"
+	depends on VIDEO
+	default n
+	---help---
+	Say Y here to add support for external DACs connected to the parallel
+	LCD interface driving a VGA connector, such as found on the
+	Olimex A13 boards.
+
 config VIDEO_LCD_MODE
 	string "LCD panel timing details"
 	depends on VIDEO
diff --git a/drivers/video/sunxi_display.c b/drivers/video/sunxi_display.c
index 2822cc6..624e5e0 100644
--- a/drivers/video/sunxi_display.c
+++ b/drivers/video/sunxi_display.c
@@ -768,6 +768,12 @@ static void sunxi_mode_set(const struct ctfb_res_modes *mode,
 		sunxi_lcdc_backlight_enable();
 		break;
 	case sunxi_monitor_vga:
+#ifdef CONFIG_VIDEO_VGA_VIA_LCD
+		sunxi_composer_mode_set(mode, address);
+		sunxi_lcdc_tcon0_mode_set(mode);
+		sunxi_composer_enable();
+		sunxi_lcdc_enable();
+#endif
 		break;
 	}
 }
@@ -798,6 +804,8 @@ void *video_hw_init(void)
 	hpd = video_get_option_int(options, "hpd", 1);
 	edid = video_get_option_int(options, "edid", 1);
 	sunxi_display.monitor = sunxi_monitor_dvi;
+#elif defined CONFIG_VIDEO_VGA_VIA_LCD
+	sunxi_display.monitor = sunxi_monitor_vga;
 #else
 	sunxi_display.monitor = sunxi_monitor_lcd;
 #endif
@@ -850,8 +858,13 @@ void *video_hw_init(void)
 		printf("LCD not supported on this board\n");
 		return NULL;
 	case sunxi_monitor_vga:
+#ifdef CONFIG_VIDEO_VGA_VIA_LCD
+		sunxi_display.depth = 18;
+		break;
+#else
 		printf("VGA not supported on this board\n");
 		return NULL;
+#endif
 	}
 
 	if (mode->vmode != FB_VMODE_NONINTERLACED) {
@@ -904,6 +917,7 @@ int sunxi_simplefb_setup(void *blob)
 		pipeline = "de_be0-lcd0";
 		break;
 	case sunxi_monitor_vga:
+		pipeline = "de_be0-lcd0";
 		break;
 	}
 
-- 
2.1.0

[U-Boot] [PATCH 11/17] sunxi: video: Add support for VGA via external DACs connected to the LCD pins

[Ian Campbell]

On Wed, 2014-12-24 at 20:06 +0100, Hans de Goede wrote:
> Add support for external DACs connected to the parallel LCD interface driving
> a VGA connector, such as found on the Olimex A13 boards.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Ian Campbell <ijc@hellion.org.uk>

> ---
>  board/sunxi/Kconfig           |  9 +++++++++
>  drivers/video/sunxi_display.c | 14 ++++++++++++++
>  2 files changed, 23 insertions(+)
> 
> diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
> index 7a8503b..3e42866 100644
> --- a/board/sunxi/Kconfig
> +++ b/board/sunxi/Kconfig
> @@ -253,6 +253,15 @@ config VIDEO_HDMI
>  	---help---
>  	Say Y here to add support for outputting video over HDMI.
>  
> +config VIDEO_VGA_VIA_LCD
> +	boolean "VGA via LCD controller support"
> +	depends on VIDEO
> +	default n
> +	---help---
> +	Say Y here to add support for external DACs connected to the parallel
> +	LCD interface driving a VGA connector, such as found on the
> +	Olimex A13 boards.

But not unique to those boards I take it? I'd be inclined to drop
everything from the "," to the end of the sentence.

Ian.

[U-Boot] [PATCH 11/17] sunxi: video: Add support for VGA via external DACs connected to the LCD pins

[Hans de Goede]

Hi,

On 29-12-14 14:51, Ian Campbell wrote:
> On Wed, 2014-12-24 at 20:06 +0100, Hans de Goede wrote:
>> Add support for external DACs connected to the parallel LCD interface driving
>> a VGA connector, such as found on the Olimex A13 boards.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>
> Acked-by: Ian Campbell <ijc@hellion.org.uk>
>
>> ---
>>   board/sunxi/Kconfig           |  9 +++++++++
>>   drivers/video/sunxi_display.c | 14 ++++++++++++++
>>   2 files changed, 23 insertions(+)
>>
>> diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
>> index 7a8503b..3e42866 100644
>> --- a/board/sunxi/Kconfig
>> +++ b/board/sunxi/Kconfig
>> @@ -253,6 +253,15 @@ config VIDEO_HDMI
>>   	---help---
>>   	Say Y here to add support for outputting video over HDMI.
>>
>> +config VIDEO_VGA_VIA_LCD
>> +	boolean "VGA via LCD controller support"
>> +	depends on VIDEO
>> +	default n
>> +	---help---
>> +	Say Y here to add support for external DACs connected to the parallel
>> +	LCD interface driving a VGA connector, such as found on the
>> +	Olimex A13 boards.
>
> But not unique to those boards I take it?

Actually sofar it is unique to those boards, both the normal and the Micro
variant (in theory we could see something similar on other boards one day).

Regards,

Hans

[U-Boot] [PATCH 11/17] sunxi: video: Add support for VGA via external DACs connected to the LCD pins

[Chen-Yu Tsai]

Hi Hans,

On Tue, Dec 30, 2014 at 3:25 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
>
> On 29-12-14 14:51, Ian Campbell wrote:
>>
>> On Wed, 2014-12-24 at 20:06 +0100, Hans de Goede wrote:
>>>
>>> Add support for external DACs connected to the parallel LCD interface
>>> driving
>>> a VGA connector, such as found on the Olimex A13 boards.
>>>
>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>
>>
>> Acked-by: Ian Campbell <ijc@hellion.org.uk>
>>
>>> ---
>>>   board/sunxi/Kconfig           |  9 +++++++++
>>>   drivers/video/sunxi_display.c | 14 ++++++++++++++
>>>   2 files changed, 23 insertions(+)
>>>
>>> diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
>>> index 7a8503b..3e42866 100644
>>> --- a/board/sunxi/Kconfig
>>> +++ b/board/sunxi/Kconfig
>>> @@ -253,6 +253,15 @@ config VIDEO_HDMI
>>>         ---help---
>>>         Say Y here to add support for outputting video over HDMI.
>>>
>>> +config VIDEO_VGA_VIA_LCD
>>> +       boolean "VGA via LCD controller support"
>>> +       depends on VIDEO
>>> +       default n
>>> +       ---help---
>>> +       Say Y here to add support for external DACs connected to the
>>> parallel
>>> +       LCD interface driving a VGA connector, such as found on the
>>> +       Olimex A13 boards.
>>
>>
>> But not unique to those boards I take it?
>
>
> Actually sofar it is unique to those boards, both the normal and the Micro
> variant (in theory we could see something similar on other boards one day).

The Hummingbird A31 has the same setup, using an external DAC for the
RGB signals, as it doesn't have the TV encoder. See:

    http://linux-sunxi.org/Merrii_Hummingbird_A31#VGA_out

ChenYu

[U-Boot] [PATCH 11/17] sunxi: video: Add support for VGA via external DACs connected to the LCD pins

[Hans de Goede]

Hi,

On 30-12-14 03:21, Chen-Yu Tsai wrote:
> Hi Hans,
>
> On Tue, Dec 30, 2014 at 3:25 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
>>
>>
>> On 29-12-14 14:51, Ian Campbell wrote:
>>>
>>> On Wed, 2014-12-24 at 20:06 +0100, Hans de Goede wrote:
>>>>
>>>> Add support for external DACs connected to the parallel LCD interface
>>>> driving
>>>> a VGA connector, such as found on the Olimex A13 boards.
>>>>
>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>
>>>
>>> Acked-by: Ian Campbell <ijc@hellion.org.uk>
>>>
>>>> ---
>>>>    board/sunxi/Kconfig           |  9 +++++++++
>>>>    drivers/video/sunxi_display.c | 14 ++++++++++++++
>>>>    2 files changed, 23 insertions(+)
>>>>
>>>> diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
>>>> index 7a8503b..3e42866 100644
>>>> --- a/board/sunxi/Kconfig
>>>> +++ b/board/sunxi/Kconfig
>>>> @@ -253,6 +253,15 @@ config VIDEO_HDMI
>>>>          ---help---
>>>>          Say Y here to add support for outputting video over HDMI.
>>>>
>>>> +config VIDEO_VGA_VIA_LCD
>>>> +       boolean "VGA via LCD controller support"
>>>> +       depends on VIDEO
>>>> +       default n
>>>> +       ---help---
>>>> +       Say Y here to add support for external DACs connected to the
>>>> parallel
>>>> +       LCD interface driving a VGA connector, such as found on the
>>>> +       Olimex A13 boards.
>>>
>>>
>>> But not unique to those boards I take it?
>>
>>
>> Actually sofar it is unique to those boards, both the normal and the Micro
>> variant (in theory we could see something similar on other boards one day).
>
> The Hummingbird A31 has the same setup, using an external DAC for the
> RGB signals, as it doesn't have the TV encoder. See:
>
>      http://linux-sunxi.org/Merrii_Hummingbird_A31#VGA_out

Interesting, it would be cool to see if this works with CONFIG_VIDEO_VGA_VIA_LCD=y
and something like "setenv video-mode sunxi:1024x768-24 at 60,monitor=vga" "saveenv"
with my sunxi-wip u-boot branch, I guess you need to add support for the enable gpio
for this to work, patches for this are welcome :)

Regards,

Hans

[U-Boot] [PATCH 12/17] sunxi: sunxi-common.h: Reduce bootm_size to take the framebuffer into account

[Hans de Goede]

Before video output support can be enabled on the A13-OLinuXinoM, bootm_size
must first be reduced to take into account that the framebuffer is shaved of
the top of the DRAM. For other boards this is not an issue since bootm was set
to 256M and all boards have at least 512M except for the A13-OLinuXinoM.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 include/configs/sunxi-common.h | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
index 30b7ee4..809e700 100644
--- a/include/configs/sunxi-common.h
+++ b/include/configs/sunxi-common.h
@@ -269,10 +269,13 @@
 #ifndef CONFIG_SPL_BUILD
 #include <config_distro_defaults.h>
 
-/* 256M RAM (minimum), 32M uncompressed kernel, 16M compressed kernel, 1M fdt,
- * 1M script, 1M pxe and the ramdisk at the end */
+/*
+ * 240M RAM (256M minimum minus space for the framebuffer),
+ * 32M uncompressed kernel, 16M compressed kernel, 1M fdt,
+ * 1M script, 1M pxe and the ramdisk at the end.
+ */
 #define MEM_LAYOUT_ENV_SETTINGS \
-	"bootm_size=0x10000000\0" \
+	"bootm_size=0xf000000\0" \
 	"kernel_addr_r=0x42000000\0" \
 	"fdt_addr_r=0x43000000\0" \
 	"scriptaddr=0x43100000\0" \
-- 
2.1.0

[U-Boot] [PATCH 12/17] sunxi: sunxi-common.h: Reduce bootm_size to take the framebuffer into account

[Ian Campbell]

On Wed, 2014-12-24 at 20:06 +0100, Hans de Goede wrote:
> Before video output support can be enabled on the A13-OLinuXinoM, bootm_size
> must first be reduced to take into account that the framebuffer is shaved of
> the top of the DRAM. For other boards this is not an issue since bootm was set
> to 256M and all boards have at least 512M except for the A13-OLinuXinoM.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Ian Campbell <ijc@hellion.org.uk>

[U-Boot] [PATCH 13/17] sunxi: A13-OLinuXino defconfigs: Enable VGA output, add lcd-mode for 7" LCD

[Hans de Goede]

Enable VGA output on the A13-OLinuXino and A13-OLinuXinoM now that we've
support for it.

Also add LCD timing and gpio info for the Olimex 7" LCD module. We can safely
put this in the default config on this boards, since by default we will always
use VGA, and the LCD timing info will only get used if the user explicitly
sets monitor=lcd in the video-mode env. variable.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 configs/A13-OLinuXinoM_defconfig | 9 +++++++--
 configs/A13-OLinuXino_defconfig  | 9 +++++++--
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/configs/A13-OLinuXinoM_defconfig b/configs/A13-OLinuXinoM_defconfig
index be8652b..7790460 100644
--- a/configs/A13-OLinuXinoM_defconfig
+++ b/configs/A13-OLinuXinoM_defconfig
@@ -2,8 +2,13 @@ CONFIG_SPL=y
 CONFIG_SYS_EXTRA_OPTIONS="CONS_INDEX=2,USB_EHCI"
 CONFIG_FDTFILE="sun5i-a13-olinuxino-micro.dtb"
 CONFIG_USB1_VBUS_PIN="PG11"
-CONFIG_VIDEO=n
-CONFIG_USB_KEYBOARD=n
+CONFIG_VIDEO_HDMI=n
+CONFIG_VIDEO_VGA_VIA_LCD=y
+# For use with the Olimex 7" LCD module, adjust timings for other displays
+# Set video-mode=sunxi:800x600-24 at 60,monitor=lcd in the env. to enable
+CONFIG_VIDEO_LCD_MODE="x:800,y:480,depth:18,pclk_khz:33000,le:16,ri:209,up:22,lo:22,hs:30,vs:1,sync:0,vmode:0"
+CONFIG_VIDEO_LCD_POWER="PB10"
+CONFIG_VIDEO_LCD_BL_PWM="PB2"
 +S:CONFIG_ARM=y
 +S:CONFIG_ARCH_SUNXI=y
 +S:CONFIG_MACH_SUN5I=y
diff --git a/configs/A13-OLinuXino_defconfig b/configs/A13-OLinuXino_defconfig
index 654e12a..944f2e5 100644
--- a/configs/A13-OLinuXino_defconfig
+++ b/configs/A13-OLinuXino_defconfig
@@ -2,8 +2,13 @@ CONFIG_SPL=y
 CONFIG_SYS_EXTRA_OPTIONS="CONS_INDEX=2,AXP209_POWER,USB_EHCI"
 CONFIG_FDTFILE="sun5i-a13-olinuxino.dtb"
 CONFIG_USB1_VBUS_PIN="PG11"
-CONFIG_VIDEO=n
-CONFIG_USB_KEYBOARD=n
+CONFIG_VIDEO_HDMI=n
+CONFIG_VIDEO_VGA_VIA_LCD=y
+# For use with the Olimex 7" LCD module, adjust timings for other displays
+# Set video-mode=sunxi:800x600-24 at 60,monitor=lcd in the env. to enable
+CONFIG_VIDEO_LCD_MODE="x:800,y:480,depth:18,pclk_khz:33000,le:16,ri:209,up:22,lo:22,hs:30,vs:1,sync:0,vmode:0"
+CONFIG_VIDEO_LCD_POWER="AXP0-0"
+CONFIG_VIDEO_LCD_BL_PWM="PB2"
 +S:CONFIG_ARM=y
 +S:CONFIG_ARCH_SUNXI=y
 +S:CONFIG_MACH_SUN5I=y
-- 
2.1.0

[U-Boot] [PATCH 13/17] sunxi: A13-OLinuXino defconfigs: Enable VGA output, add lcd-mode for 7" LCD

[Ian Campbell]

On Wed, 2014-12-24 at 20:06 +0100, Hans de Goede wrote:
> Enable VGA output on the A13-OLinuXino and A13-OLinuXinoM now that we've
> support for it.
> 
> Also add LCD timing and gpio info for the Olimex 7" LCD module. We can safely
> put this in the default config on this boards, since by default we will always
> use VGA, and the LCD timing info will only get used if the user explicitly
> sets monitor=lcd in the video-mode env. variable.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Acked-by: Ian Campbell <ijc@hellion.org.uk>

[U-Boot] [PATCH 14/17] sunxi: Add 2 defconfigs for using the Olimex 7" lcd with olinuxino boards

[Hans de Goede]

Add defconfigs for using the Olimex 7" lcd with the A10s-OLinuXino-M and
the A20-OLinuXino_MICRO boards.

Note this cannot be added to the regular defconfigs, like with the A13 boards,
because the video code will automatically fallback to driving the LCD if
no HDMI cable is connected (this is the desired behavior on e.g. tablets),
but the LCD connector on the Olimex dev boards may be used for something
completely different, so we do not want to automatically enable it.

Note I do not intend to add defconfigs for all possible permutation of Olimex
boards and LCD displays, these 2 serve mostly as examples. In the long run
this should be solved by moving all the LCD info into dts files and using
devicetree overlays for add-on modules like the LCD displays.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 configs/A10s-OLinuXino-M-lcd7_defconfig    | 14 ++++++++++++++
 configs/A20-OLinuXino_MICRO-lcd7_defconfig | 13 +++++++++++++
 2 files changed, 27 insertions(+)
 create mode 100644 configs/A10s-OLinuXino-M-lcd7_defconfig
 create mode 100644 configs/A20-OLinuXino_MICRO-lcd7_defconfig

diff --git a/configs/A10s-OLinuXino-M-lcd7_defconfig b/configs/A10s-OLinuXino-M-lcd7_defconfig
new file mode 100644
index 0000000..eafa974
--- /dev/null
+++ b/configs/A10s-OLinuXino-M-lcd7_defconfig
@@ -0,0 +1,14 @@
+CONFIG_SPL=y
+CONFIG_SYS_EXTRA_OPTIONS="AXP152_POWER,SUNXI_EMAC,USB_EHCI"
+CONFIG_FDTFILE="sun5i-a10s-olinuxino-micro.dtb"
+CONFIG_MMC_SUNXI_SLOT_EXTRA=1
+CONFIG_USB1_VBUS_PIN="PB10"
+CONFIG_VIDEO_LCD_MODE="x:800,y:480,depth:18,pclk_khz:33000,le:16,ri:209,up:22,lo:22,hs:30,vs:1,sync:0,vmode:0"
+CONFIG_VIDEO_LCD_POWER="PB9"
+CONFIG_VIDEO_LCD_BL_PWM="PB2"
++S:CONFIG_MMC0_CD_PIN="PG1"
++S:CONFIG_MMC1_CD_PIN="PG13"
++S:CONFIG_ARM=y
++S:CONFIG_ARCH_SUNXI=y
++S:CONFIG_MACH_SUN5I=y
++S:CONFIG_TARGET_A10S_OLINUXINO_M=y
diff --git a/configs/A20-OLinuXino_MICRO-lcd7_defconfig b/configs/A20-OLinuXino_MICRO-lcd7_defconfig
new file mode 100644
index 0000000..05674d7
--- /dev/null
+++ b/configs/A20-OLinuXino_MICRO-lcd7_defconfig
@@ -0,0 +1,13 @@
+CONFIG_SPL=y
+CONFIG_SYS_EXTRA_OPTIONS="AXP209_POWER,SUNXI_GMAC,AHCI,SATAPWR=SUNXI_GPB(8),USB_EHCI"
+CONFIG_FDTFILE="sun7i-a20-olinuxino-micro.dtb"
+CONFIG_MMC_SUNXI_SLOT_EXTRA=3
+CONFIG_VIDEO_LCD_MODE="x:800,y:480,depth:18,pclk_khz:33000,le:16,ri:209,up:22,lo:22,hs:30,vs:1,sync:0,vmode:0"
+CONFIG_VIDEO_LCD_POWER="PH8"
+CONFIG_VIDEO_LCD_BL_PWM="PB2"
++S:CONFIG_MMC0_CD_PIN="PH1"
++S:CONFIG_MMC3_CD_PIN="PH11"
++S:CONFIG_ARM=y
++S:CONFIG_ARCH_SUNXI=y
++S:CONFIG_MACH_SUN7I=y
++S:CONFIG_TARGET_A20_OLINUXINO_M=y
-- 
2.1.0

[U-Boot] [PATCH 14/17] sunxi: Add 2 defconfigs for using the Olimex 7" lcd with olinuxino boards

[Ian Campbell]

On Wed, 2014-12-24 at 20:06 +0100, Hans de Goede wrote:
> Add defconfigs for using the Olimex 7" lcd with the A10s-OLinuXino-M and
> the A20-OLinuXino_MICRO boards.
> 
> Note this cannot be added to the regular defconfigs, like with the A13 boards,
> because the video code will automatically fallback to driving the LCD if
> no HDMI cable is connected (this is the desired behavior on e.g. tablets),
> but the LCD connector on the Olimex dev boards may be used for something
> completely different, so we do not want to automatically enable it.
> 
> Note I do not intend to add defconfigs for all possible permutation of Olimex
> boards and LCD displays, these 2 serve mostly as examples.

Does the A13 one not serve that function already without the downside of
duplicating up the defconfig?

>  In the long run
> this should be solved by moving all the LCD info into dts files and using
> devicetree overlays for add-on modules like the LCD displays.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  configs/A10s-OLinuXino-M-lcd7_defconfig    | 14 ++++++++++++++
>  configs/A20-OLinuXino_MICRO-lcd7_defconfig | 13 +++++++++++++
>  2 files changed, 27 insertions(+)
>  create mode 100644 configs/A10s-OLinuXino-M-lcd7_defconfig
>  create mode 100644 configs/A20-OLinuXino_MICRO-lcd7_defconfig
> 
> diff --git a/configs/A10s-OLinuXino-M-lcd7_defconfig b/configs/A10s-OLinuXino-M-lcd7_defconfig
> new file mode 100644
> index 0000000..eafa974
> --- /dev/null
> +++ b/configs/A10s-OLinuXino-M-lcd7_defconfig
> @@ -0,0 +1,14 @@
> +CONFIG_SPL=y
> +CONFIG_SYS_EXTRA_OPTIONS="AXP152_POWER,SUNXI_EMAC,USB_EHCI"
> +CONFIG_FDTFILE="sun5i-a10s-olinuxino-micro.dtb"
> +CONFIG_MMC_SUNXI_SLOT_EXTRA=1
> +CONFIG_USB1_VBUS_PIN="PB10"
> +CONFIG_VIDEO_LCD_MODE="x:800,y:480,depth:18,pclk_khz:33000,le:16,ri:209,up:22,lo:22,hs:30,vs:1,sync:0,vmode:0"
> +CONFIG_VIDEO_LCD_POWER="PB9"
> +CONFIG_VIDEO_LCD_BL_PWM="PB2"
> ++S:CONFIG_MMC0_CD_PIN="PG1"
> ++S:CONFIG_MMC1_CD_PIN="PG13"
> ++S:CONFIG_ARM=y
> ++S:CONFIG_ARCH_SUNXI=y
> ++S:CONFIG_MACH_SUN5I=y
> ++S:CONFIG_TARGET_A10S_OLINUXINO_M=y
> diff --git a/configs/A20-OLinuXino_MICRO-lcd7_defconfig b/configs/A20-OLinuXino_MICRO-lcd7_defconfig
> new file mode 100644
> index 0000000..05674d7
> --- /dev/null
> +++ b/configs/A20-OLinuXino_MICRO-lcd7_defconfig
> @@ -0,0 +1,13 @@
> +CONFIG_SPL=y
> +CONFIG_SYS_EXTRA_OPTIONS="AXP209_POWER,SUNXI_GMAC,AHCI,SATAPWR=SUNXI_GPB(8),USB_EHCI"
> +CONFIG_FDTFILE="sun7i-a20-olinuxino-micro.dtb"
> +CONFIG_MMC_SUNXI_SLOT_EXTRA=3
> +CONFIG_VIDEO_LCD_MODE="x:800,y:480,depth:18,pclk_khz:33000,le:16,ri:209,up:22,lo:22,hs:30,vs:1,sync:0,vmode:0"
> +CONFIG_VIDEO_LCD_POWER="PH8"
> +CONFIG_VIDEO_LCD_BL_PWM="PB2"
> ++S:CONFIG_MMC0_CD_PIN="PH1"
> ++S:CONFIG_MMC3_CD_PIN="PH11"
> ++S:CONFIG_ARM=y
> ++S:CONFIG_ARCH_SUNXI=y
> ++S:CONFIG_MACH_SUN7I=y
> ++S:CONFIG_TARGET_A20_OLINUXINO_M=y

[U-Boot] [PATCH 14/17] sunxi: Add 2 defconfigs for using the Olimex 7" lcd with olinuxino boards

[Hans de Goede]

Hi,

On 29-12-14 14:55, Ian Campbell wrote:
> On Wed, 2014-12-24 at 20:06 +0100, Hans de Goede wrote:
>> Add defconfigs for using the Olimex 7" lcd with the A10s-OLinuXino-M and
>> the A20-OLinuXino_MICRO boards.
>>
>> Note this cannot be added to the regular defconfigs, like with the A13 boards,
>> because the video code will automatically fallback to driving the LCD if
>> no HDMI cable is connected (this is the desired behavior on e.g. tablets),
>> but the LCD connector on the Olimex dev boards may be used for something
>> completely different, so we do not want to automatically enable it.
>>
>> Note I do not intend to add defconfigs for all possible permutation of Olimex
>> boards and LCD displays, these 2 serve mostly as examples.
>
> Does the A13 one not serve that function already without the downside of
> duplicating up the defconfig?

True, I wrote these before doing the A13 stuff, then ammended the commit message
to explain why these where different from the A13 case later.

But you're right the A13 configs do more or less serve the same purpose so we
could drop these.

I take it that dropping these has your preference ?

Regards,

Hans

[U-Boot] [PATCH 14/17] sunxi: Add 2 defconfigs for using the Olimex 7" lcd with olinuxino boards

[Ian Campbell]

On Mon, 2014-12-29 at 20:27 +0100, Hans de Goede wrote:
> Hi,
> 
> On 29-12-14 14:55, Ian Campbell wrote:
> > On Wed, 2014-12-24 at 20:06 +0100, Hans de Goede wrote:
> >> Add defconfigs for using the Olimex 7" lcd with the A10s-OLinuXino-M and
> >> the A20-OLinuXino_MICRO boards.
> >>
> >> Note this cannot be added to the regular defconfigs, like with the A13 boards,
> >> because the video code will automatically fallback to driving the LCD if
> >> no HDMI cable is connected (this is the desired behavior on e.g. tablets),
> >> but the LCD connector on the Olimex dev boards may be used for something
> >> completely different, so we do not want to automatically enable it.
> >>
> >> Note I do not intend to add defconfigs for all possible permutation of Olimex
> >> boards and LCD displays, these 2 serve mostly as examples.
> >
> > Does the A13 one not serve that function already without the downside of
> > duplicating up the defconfig?
> 
> True, I wrote these before doing the A13 stuff, then ammended the commit message
> to explain why these where different from the A13 case later.
> 
> But you're right the A13 configs do more or less serve the same purpose so we
> could drop these.
> 
> I take it that dropping these has your preference ?

That's right.

Ian.

[U-Boot] [PATCH 15/17] sunxi: Ippo_q8h defconfigs: Enable the LCD panel found on these tablets.

[Hans de Goede]

Enable the new LCD support on Ippo_q8h tablets.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 configs/Ippo_q8h_v1_2_defconfig | 5 ++++-
 configs/Ippo_q8h_v5_defconfig   | 5 ++++-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/configs/Ippo_q8h_v1_2_defconfig b/configs/Ippo_q8h_v1_2_defconfig
index fefed32..c773f5f 100644
--- a/configs/Ippo_q8h_v1_2_defconfig
+++ b/configs/Ippo_q8h_v1_2_defconfig
@@ -1,7 +1,10 @@
 CONFIG_SPL=y
 CONFIG_SYS_EXTRA_OPTIONS="CONS_INDEX=5"
 CONFIG_FDTFILE="sun8i-a23-ippo-q8h-v1.2.dtb"
-CONFIG_VIDEO=n
+CONFIG_VIDEO_LCD_MODE="x:800,y:480,depth:18,pclk_khz:33000,le:16,ri:209,up:22,lo:22,hs:30,vs:1,sync:0,vmode:0"
+CONFIG_VIDEO_LCD_POWER="PH7"
+CONFIG_VIDEO_LCD_BL_EN="PH6"
+CONFIG_VIDEO_LCD_BL_PWM="PH0"
 CONFIG_USB_KEYBOARD=n
 +S:CONFIG_ARM=y
 +S:CONFIG_ARCH_SUNXI=y
diff --git a/configs/Ippo_q8h_v5_defconfig b/configs/Ippo_q8h_v5_defconfig
index b8d3afe..ce4f0b8 100644
--- a/configs/Ippo_q8h_v5_defconfig
+++ b/configs/Ippo_q8h_v5_defconfig
@@ -1,7 +1,10 @@
 CONFIG_SPL=y
 CONFIG_SYS_EXTRA_OPTIONS="CONS_INDEX=5"
 CONFIG_FDTFILE="sun8i-a23-ippo-q8h-v5.dtb"
-CONFIG_VIDEO=n
+CONFIG_VIDEO_LCD_MODE="x:800,y:480,depth:18,pclk_khz:33000,le:16,ri:209,up:22,lo:22,hs:30,vs:1,sync:0,vmode:0"
+CONFIG_VIDEO_LCD_POWER="PH7"
+CONFIG_VIDEO_LCD_BL_EN="PH6"
+CONFIG_VIDEO_LCD_BL_PWM="PH0"
 CONFIG_USB_KEYBOARD=n
 +S:CONFIG_ARM=y
 +S:CONFIG_ARCH_SUNXI=y
-- 
2.1.0

[U-Boot] [PATCH 15/17] sunxi: Ippo_q8h defconfigs: Enable the LCD panel found on these tablets.

[Chen-Yu Tsai]

Hi,

On Thu, Dec 25, 2014 at 3:06 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> Enable the new LCD support on Ippo_q8h tablets.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  configs/Ippo_q8h_v1_2_defconfig | 5 ++++-
>  configs/Ippo_q8h_v5_defconfig   | 5 ++++-
>  2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/configs/Ippo_q8h_v1_2_defconfig b/configs/Ippo_q8h_v1_2_defconfig
> index fefed32..c773f5f 100644
> --- a/configs/Ippo_q8h_v1_2_defconfig
> +++ b/configs/Ippo_q8h_v1_2_defconfig
> @@ -1,7 +1,10 @@
>  CONFIG_SPL=y
>  CONFIG_SYS_EXTRA_OPTIONS="CONS_INDEX=5"
>  CONFIG_FDTFILE="sun8i-a23-ippo-q8h-v1.2.dtb"
> -CONFIG_VIDEO=n
> +CONFIG_VIDEO_LCD_MODE="x:800,y:480,depth:18,pclk_khz:33000,le:16,ri:209,up:22,lo:22,hs:30,vs:1,sync:0,vmode:0"
> +CONFIG_VIDEO_LCD_POWER="PH7"
> +CONFIG_VIDEO_LCD_BL_EN="PH6"
> +CONFIG_VIDEO_LCD_BL_PWM="PH0"
>  CONFIG_USB_KEYBOARD=n
>  +S:CONFIG_ARM=y
>  +S:CONFIG_ARCH_SUNXI=y
> diff --git a/configs/Ippo_q8h_v5_defconfig b/configs/Ippo_q8h_v5_defconfig
> index b8d3afe..ce4f0b8 100644
> --- a/configs/Ippo_q8h_v5_defconfig
> +++ b/configs/Ippo_q8h_v5_defconfig
> @@ -1,7 +1,10 @@
>  CONFIG_SPL=y
>  CONFIG_SYS_EXTRA_OPTIONS="CONS_INDEX=5"
>  CONFIG_FDTFILE="sun8i-a23-ippo-q8h-v5.dtb"
> -CONFIG_VIDEO=n
> +CONFIG_VIDEO_LCD_MODE="x:800,y:480,depth:18,pclk_khz:33000,le:16,ri:209,up:22,lo:22,hs:30,vs:1,sync:0,vmode:0"

The display on my Q8H is a bit off to the left. With the simplefb
bindings from your kernel sunxi-wip branch, I get a nice console.
Though I've no way to type, at least I can tell my tablet is on. :)

Could you briefly explain how to convert the values in the fex
file to the mode line here? It could also help others with
enabling display on their tablets.

Thanks
ChenYu

> +CONFIG_VIDEO_LCD_POWER="PH7"
> +CONFIG_VIDEO_LCD_BL_EN="PH6"
> +CONFIG_VIDEO_LCD_BL_PWM="PH0"
>  CONFIG_USB_KEYBOARD=n
>  +S:CONFIG_ARM=y
>  +S:CONFIG_ARCH_SUNXI=y
> --
> 2.1.0
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 15/17] sunxi: Ippo_q8h defconfigs: Enable the LCD panel found on these tablets.

[Hans de Goede]

Hi,

On 25-12-14 11:00, Chen-Yu Tsai wrote:
> Hi,
>
> On Thu, Dec 25, 2014 at 3:06 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>> Enable the new LCD support on Ippo_q8h tablets.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   configs/Ippo_q8h_v1_2_defconfig | 5 ++++-
>>   configs/Ippo_q8h_v5_defconfig   | 5 ++++-
>>   2 files changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/configs/Ippo_q8h_v1_2_defconfig b/configs/Ippo_q8h_v1_2_defconfig
>> index fefed32..c773f5f 100644
>> --- a/configs/Ippo_q8h_v1_2_defconfig
>> +++ b/configs/Ippo_q8h_v1_2_defconfig
>> @@ -1,7 +1,10 @@
>>   CONFIG_SPL=y
>>   CONFIG_SYS_EXTRA_OPTIONS="CONS_INDEX=5"
>>   CONFIG_FDTFILE="sun8i-a23-ippo-q8h-v1.2.dtb"
>> -CONFIG_VIDEO=n
>> +CONFIG_VIDEO_LCD_MODE="x:800,y:480,depth:18,pclk_khz:33000,le:16,ri:209,up:22,lo:22,hs:30,vs:1,sync:0,vmode:0"
>> +CONFIG_VIDEO_LCD_POWER="PH7"
>> +CONFIG_VIDEO_LCD_BL_EN="PH6"
>> +CONFIG_VIDEO_LCD_BL_PWM="PH0"
>>   CONFIG_USB_KEYBOARD=n
>>   +S:CONFIG_ARM=y
>>   +S:CONFIG_ARCH_SUNXI=y
>> diff --git a/configs/Ippo_q8h_v5_defconfig b/configs/Ippo_q8h_v5_defconfig
>> index b8d3afe..ce4f0b8 100644
>> --- a/configs/Ippo_q8h_v5_defconfig
>> +++ b/configs/Ippo_q8h_v5_defconfig
>> @@ -1,7 +1,10 @@
>>   CONFIG_SPL=y
>>   CONFIG_SYS_EXTRA_OPTIONS="CONS_INDEX=5"
>>   CONFIG_FDTFILE="sun8i-a23-ippo-q8h-v5.dtb"
>> -CONFIG_VIDEO=n
>> +CONFIG_VIDEO_LCD_MODE="x:800,y:480,depth:18,pclk_khz:33000,le:16,ri:209,up:22,lo:22,hs:30,vs:1,sync:0,vmode:0"
>
> The display on my Q8H is a bit off to the left. With the simplefb
> bindings from your kernel sunxi-wip branch, I get a nice console.
> Though I've no way to type, at least I can tell my tablet is on. :)
>
> Could you briefly explain how to convert the values in the fex
> file to the mode line here? It could also help others with
> enabling display on their tablets.

Ah yes, I used the slightly different timings from the olimex 7" lcd
panel for olinuxino boards, and since those worked fine on my a23
tablet I never adjusted things. Here is a translation table:


CONFIG_VIDEO_LCD_MODE		fex value(s)

x				lcd_x
y				lcd_y
depth:18			lcd_frm = 1
pclk_khz			lcd_dclk_freq * 1000
hs				lcd_hv_hspw (with a minimum of 1)
vs				lcd_hv_vspw (with a minimum of 1)
le				lcd_hbp - hs
ri				lcd_ht - lcd_x - lcd_hbp
up				lcd_vbp - vs

On sun4i/sun5i/sun7i:
lo				(lcd_vt / 2) - lcd_y - lcd_vbp
On sun8i:
lo				lcd_vt - lcd_y - lcd_vbp

sync				0
mode				0

I notice that the Ippo_q8h_v5 fex uses 0 for lcd_hv_hspw and lcd_hv_vspw, which
is not a valid value as the register value contains hspw - 1, so the minimum is 1,
and looking at a register dump under android with my A23 tablet the value indeed
should be 1.

So if I'm not mistaken for the Ippo_q8h_v5 the timings should be:

CONFIG_VIDEO_LCD_MODE="x:800,y:480,depth:18,pclk_khz:33000,le:87,ri:168,up:31,lo:13,hs:1,vs:1,sync:0,vmode:0"

Regards,

Hans

[U-Boot] [PATCH 15/17] sunxi: Ippo_q8h defconfigs: Enable the LCD panel found on these tablets.

[Chen-Yu Tsai]

Hi,

On Thu, Dec 25, 2014 at 6:59 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
>
> On 25-12-14 11:00, Chen-Yu Tsai wrote:
>>
>> Hi,
>>
>> On Thu, Dec 25, 2014 at 3:06 AM, Hans de Goede <hdegoede@redhat.com>
>> wrote:
>>>
>>> Enable the new LCD support on Ippo_q8h tablets.
>>>
>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>> ---
>>>   configs/Ippo_q8h_v1_2_defconfig | 5 ++++-
>>>   configs/Ippo_q8h_v5_defconfig   | 5 ++++-
>>>   2 files changed, 8 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/configs/Ippo_q8h_v1_2_defconfig
>>> b/configs/Ippo_q8h_v1_2_defconfig
>>> index fefed32..c773f5f 100644
>>> --- a/configs/Ippo_q8h_v1_2_defconfig
>>> +++ b/configs/Ippo_q8h_v1_2_defconfig
>>> @@ -1,7 +1,10 @@
>>>   CONFIG_SPL=y
>>>   CONFIG_SYS_EXTRA_OPTIONS="CONS_INDEX=5"
>>>   CONFIG_FDTFILE="sun8i-a23-ippo-q8h-v1.2.dtb"
>>> -CONFIG_VIDEO=n
>>>
>>> +CONFIG_VIDEO_LCD_MODE="x:800,y:480,depth:18,pclk_khz:33000,le:16,ri:209,up:22,lo:22,hs:30,vs:1,sync:0,vmode:0"
>>> +CONFIG_VIDEO_LCD_POWER="PH7"
>>> +CONFIG_VIDEO_LCD_BL_EN="PH6"
>>> +CONFIG_VIDEO_LCD_BL_PWM="PH0"
>>>   CONFIG_USB_KEYBOARD=n
>>>   +S:CONFIG_ARM=y
>>>   +S:CONFIG_ARCH_SUNXI=y
>>> diff --git a/configs/Ippo_q8h_v5_defconfig
>>> b/configs/Ippo_q8h_v5_defconfig
>>> index b8d3afe..ce4f0b8 100644
>>> --- a/configs/Ippo_q8h_v5_defconfig
>>> +++ b/configs/Ippo_q8h_v5_defconfig
>>> @@ -1,7 +1,10 @@
>>>   CONFIG_SPL=y
>>>   CONFIG_SYS_EXTRA_OPTIONS="CONS_INDEX=5"
>>>   CONFIG_FDTFILE="sun8i-a23-ippo-q8h-v5.dtb"
>>> -CONFIG_VIDEO=n
>>>
>>> +CONFIG_VIDEO_LCD_MODE="x:800,y:480,depth:18,pclk_khz:33000,le:16,ri:209,up:22,lo:22,hs:30,vs:1,sync:0,vmode:0"
>>
>>
>> The display on my Q8H is a bit off to the left. With the simplefb
>> bindings from your kernel sunxi-wip branch, I get a nice console.
>> Though I've no way to type, at least I can tell my tablet is on. :)
>>
>> Could you briefly explain how to convert the values in the fex
>> file to the mode line here? It could also help others with
>> enabling display on their tablets.
>
>
> Ah yes, I used the slightly different timings from the olimex 7" lcd
> panel for olinuxino boards, and since those worked fine on my a23
> tablet I never adjusted things. Here is a translation table:
>
>
> CONFIG_VIDEO_LCD_MODE           fex value(s)
>
> x                               lcd_x
> y                               lcd_y
> depth:18                        lcd_frm = 1
> pclk_khz                        lcd_dclk_freq * 1000
> hs                              lcd_hv_hspw (with a minimum of 1)
> vs                              lcd_hv_vspw (with a minimum of 1)
> le                              lcd_hbp - hs
> ri                              lcd_ht - lcd_x - lcd_hbp
> up                              lcd_vbp - vs
>
> On sun4i/sun5i/sun7i:
> lo                              (lcd_vt / 2) - lcd_y - lcd_vbp
> On sun8i:
> lo                              lcd_vt - lcd_y - lcd_vbp
>
> sync                            0
> mode                            0
>
> I notice that the Ippo_q8h_v5 fex uses 0 for lcd_hv_hspw and lcd_hv_vspw,
> which
> is not a valid value as the register value contains hspw - 1, so the minimum
> is 1,
> and looking at a register dump under android with my A23 tablet the value
> indeed
> should be 1.
>
> So if I'm not mistaken for the Ippo_q8h_v5 the timings should be:
>
> CONFIG_VIDEO_LCD_MODE="x:800,y:480,depth:18,pclk_khz:33000,le:87,ri:168,up:31,lo:13,hs:1,vs:1,sync:0,vmode:0"


The new values look better. I haven't tested displaying anything other
than a framebuffer console, so I can't say if the other margins are
correct.

I've created a simple wiki page to put the translation table:
http://linux-sunxi.org/LCD

Thanks!
ChenYu

[U-Boot] [PATCH 15/17] sunxi: Ippo_q8h defconfigs: Enable the LCD panel found on these tablets.

[Hans de Goede]

Hi,

On 26-12-14 07:44, Chen-Yu Tsai wrote:
> Hi,
>
> On Thu, Dec 25, 2014 at 6:59 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
>>
>>
>> On 25-12-14 11:00, Chen-Yu Tsai wrote:
>>>
>>> Hi,
>>>
>>> On Thu, Dec 25, 2014 at 3:06 AM, Hans de Goede <hdegoede@redhat.com>
>>> wrote:
>>>>
>>>> Enable the new LCD support on Ippo_q8h tablets.
>>>>
>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>> ---
>>>>    configs/Ippo_q8h_v1_2_defconfig | 5 ++++-
>>>>    configs/Ippo_q8h_v5_defconfig   | 5 ++++-
>>>>    2 files changed, 8 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/configs/Ippo_q8h_v1_2_defconfig
>>>> b/configs/Ippo_q8h_v1_2_defconfig
>>>> index fefed32..c773f5f 100644
>>>> --- a/configs/Ippo_q8h_v1_2_defconfig
>>>> +++ b/configs/Ippo_q8h_v1_2_defconfig
>>>> @@ -1,7 +1,10 @@
>>>>    CONFIG_SPL=y
>>>>    CONFIG_SYS_EXTRA_OPTIONS="CONS_INDEX=5"
>>>>    CONFIG_FDTFILE="sun8i-a23-ippo-q8h-v1.2.dtb"
>>>> -CONFIG_VIDEO=n
>>>>
>>>> +CONFIG_VIDEO_LCD_MODE="x:800,y:480,depth:18,pclk_khz:33000,le:16,ri:209,up:22,lo:22,hs:30,vs:1,sync:0,vmode:0"
>>>> +CONFIG_VIDEO_LCD_POWER="PH7"
>>>> +CONFIG_VIDEO_LCD_BL_EN="PH6"
>>>> +CONFIG_VIDEO_LCD_BL_PWM="PH0"
>>>>    CONFIG_USB_KEYBOARD=n
>>>>    +S:CONFIG_ARM=y
>>>>    +S:CONFIG_ARCH_SUNXI=y
>>>> diff --git a/configs/Ippo_q8h_v5_defconfig
>>>> b/configs/Ippo_q8h_v5_defconfig
>>>> index b8d3afe..ce4f0b8 100644
>>>> --- a/configs/Ippo_q8h_v5_defconfig
>>>> +++ b/configs/Ippo_q8h_v5_defconfig
>>>> @@ -1,7 +1,10 @@
>>>>    CONFIG_SPL=y
>>>>    CONFIG_SYS_EXTRA_OPTIONS="CONS_INDEX=5"
>>>>    CONFIG_FDTFILE="sun8i-a23-ippo-q8h-v5.dtb"
>>>> -CONFIG_VIDEO=n
>>>>
>>>> +CONFIG_VIDEO_LCD_MODE="x:800,y:480,depth:18,pclk_khz:33000,le:16,ri:209,up:22,lo:22,hs:30,vs:1,sync:0,vmode:0"
>>>
>>>
>>> The display on my Q8H is a bit off to the left. With the simplefb
>>> bindings from your kernel sunxi-wip branch, I get a nice console.
>>> Though I've no way to type, at least I can tell my tablet is on. :)
>>>
>>> Could you briefly explain how to convert the values in the fex
>>> file to the mode line here? It could also help others with
>>> enabling display on their tablets.
>>
>>
>> Ah yes, I used the slightly different timings from the olimex 7" lcd
>> panel for olinuxino boards, and since those worked fine on my a23
>> tablet I never adjusted things. Here is a translation table:
>>
>>
>> CONFIG_VIDEO_LCD_MODE           fex value(s)
>>
>> x                               lcd_x
>> y                               lcd_y
>> depth:18                        lcd_frm = 1
>> pclk_khz                        lcd_dclk_freq * 1000
>> hs                              lcd_hv_hspw (with a minimum of 1)
>> vs                              lcd_hv_vspw (with a minimum of 1)
>> le                              lcd_hbp - hs
>> ri                              lcd_ht - lcd_x - lcd_hbp
>> up                              lcd_vbp - vs
>>
>> On sun4i/sun5i/sun7i:
>> lo                              (lcd_vt / 2) - lcd_y - lcd_vbp
>> On sun8i:
>> lo                              lcd_vt - lcd_y - lcd_vbp
>>
>> sync                            0
>> mode                            0
>>
>> I notice that the Ippo_q8h_v5 fex uses 0 for lcd_hv_hspw and lcd_hv_vspw,
>> which
>> is not a valid value as the register value contains hspw - 1, so the minimum
>> is 1,
>> and looking at a register dump under android with my A23 tablet the value
>> indeed
>> should be 1.
>>
>> So if I'm not mistaken for the Ippo_q8h_v5 the timings should be:
>>
>> CONFIG_VIDEO_LCD_MODE="x:800,y:480,depth:18,pclk_khz:33000,le:87,ri:168,up:31,lo:13,hs:1,vs:1,sync:0,vmode:0"
>
>
> The new values look better.

Good.

> I haven't tested displaying anything other
> than a framebuffer console, so I can't say if the other margins are
> correct.
>
> I've created a simple wiki page to put the translation table:
> http://linux-sunxi.org/LCD

Thanks for doing that.

Regards,

Hans

[U-Boot] [PATCH 15/17] sunxi: Ippo_q8h defconfigs: Enable the LCD panel found on these tablets.

[Ian Campbell]

On Fri, 2014-12-26 at 14:44 +0800, Chen-Yu Tsai wrote:
> > So if I'm not mistaken for the Ippo_q8h_v5 the timings should be:
> >
> > CONFIG_VIDEO_LCD_MODE="x:800,y:480,depth:18,pclk_khz:33000,le:87,ri:168,up:31,lo:13,hs:1,vs:1,sync:0,vmode:0"
> 
> 
> The new values look better.

Thanks. I've not seen a v2 of this patch, but assuming a patch with
these new values were to be posted I would say:
    Acked-by: Ian Campbell <ijc@hellion.org.uk>

I think we should add your Tested-by too.

Ian.

[U-Boot] [PATCH 15/17] sunxi: Ippo_q8h defconfigs: Enable the LCD panel found on these tablets.

[Chen-Yu Tsai]

On Mon, Dec 29, 2014 at 9:57 PM, Ian Campbell <ijc@hellion.org.uk> wrote:
> On Fri, 2014-12-26 at 14:44 +0800, Chen-Yu Tsai wrote:
>> > So if I'm not mistaken for the Ippo_q8h_v5 the timings should be:
>> >
>> > CONFIG_VIDEO_LCD_MODE="x:800,y:480,depth:18,pclk_khz:33000,le:87,ri:168,up:31,lo:13,hs:1,vs:1,sync:0,vmode:0"
>>
>>
>> The new values look better.
>
> Thanks. I've not seen a v2 of this patch, but assuming a patch with
> these new values were to be posted I would say:
>     Acked-by: Ian Campbell <ijc@hellion.org.uk>
>
> I think we should add your Tested-by too.

Tested-by: Chen-Yu Tsai <wens@csie.org>

for the new values.

[U-Boot] [PATCH 15/17] sunxi: Ippo_q8h defconfigs: Enable the LCD panel found on these tablets.

[Hans de Goede]

Hi,

On 29-12-14 16:56, Chen-Yu Tsai wrote:
> On Mon, Dec 29, 2014 at 9:57 PM, Ian Campbell <ijc@hellion.org.uk> wrote:
>> On Fri, 2014-12-26 at 14:44 +0800, Chen-Yu Tsai wrote:
>>>> So if I'm not mistaken for the Ippo_q8h_v5 the timings should be:
>>>>
>>>> CONFIG_VIDEO_LCD_MODE="x:800,y:480,depth:18,pclk_khz:33000,le:87,ri:168,up:31,lo:13,hs:1,vs:1,sync:0,vmode:0"
>>>
>>>
>>> The new values look better.
>>
>> Thanks. I've not seen a v2 of this patch, but assuming a patch with
>> these new values were to be posted I would say:
>>      Acked-by: Ian Campbell <ijc@hellion.org.uk>
>>
>> I think we should add your Tested-by too.
>
> Tested-by: Chen-Yu Tsai <wens@csie.org>

Thanks, I've added both the Acked-by  and the Tested-by to the commit in
my personal tree, which has the new values.

Regards,

Hans

[U-Boot] [PATCH 15/17] sunxi: Ippo_q8h defconfigs: Enable the LCD panel found on these tablets.

[Siarhei Siamashka]

On Thu, 25 Dec 2014 11:59:55 +0100
Hans de Goede <hdegoede@redhat.com> wrote:

> Hi,
> 
> On 25-12-14 11:00, Chen-Yu Tsai wrote:
> > Hi,
> >
> > On Thu, Dec 25, 2014 at 3:06 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> >> Enable the new LCD support on Ippo_q8h tablets.
> >>
> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >> ---
> >>   configs/Ippo_q8h_v1_2_defconfig | 5 ++++-
> >>   configs/Ippo_q8h_v5_defconfig   | 5 ++++-
> >>   2 files changed, 8 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/configs/Ippo_q8h_v1_2_defconfig b/configs/Ippo_q8h_v1_2_defconfig
> >> index fefed32..c773f5f 100644
> >> --- a/configs/Ippo_q8h_v1_2_defconfig
> >> +++ b/configs/Ippo_q8h_v1_2_defconfig
> >> @@ -1,7 +1,10 @@
> >>   CONFIG_SPL=y
> >>   CONFIG_SYS_EXTRA_OPTIONS="CONS_INDEX=5"
> >>   CONFIG_FDTFILE="sun8i-a23-ippo-q8h-v1.2.dtb"
> >> -CONFIG_VIDEO=n
> >> +CONFIG_VIDEO_LCD_MODE="x:800,y:480,depth:18,pclk_khz:33000,le:16,ri:209,up:22,lo:22,hs:30,vs:1,sync:0,vmode:0"
> >> +CONFIG_VIDEO_LCD_POWER="PH7"
> >> +CONFIG_VIDEO_LCD_BL_EN="PH6"
> >> +CONFIG_VIDEO_LCD_BL_PWM="PH0"
> >>   CONFIG_USB_KEYBOARD=n
> >>   +S:CONFIG_ARM=y
> >>   +S:CONFIG_ARCH_SUNXI=y
> >> diff --git a/configs/Ippo_q8h_v5_defconfig b/configs/Ippo_q8h_v5_defconfig
> >> index b8d3afe..ce4f0b8 100644
> >> --- a/configs/Ippo_q8h_v5_defconfig
> >> +++ b/configs/Ippo_q8h_v5_defconfig
> >> @@ -1,7 +1,10 @@
> >>   CONFIG_SPL=y
> >>   CONFIG_SYS_EXTRA_OPTIONS="CONS_INDEX=5"
> >>   CONFIG_FDTFILE="sun8i-a23-ippo-q8h-v5.dtb"
> >> -CONFIG_VIDEO=n
> >> +CONFIG_VIDEO_LCD_MODE="x:800,y:480,depth:18,pclk_khz:33000,le:16,ri:209,up:22,lo:22,hs:30,vs:1,sync:0,vmode:0"
> >
> > The display on my Q8H is a bit off to the left. With the simplefb
> > bindings from your kernel sunxi-wip branch, I get a nice console.
> > Though I've no way to type, at least I can tell my tablet is on. :)
> >
> > Could you briefly explain how to convert the values in the fex
> > file to the mode line here? It could also help others with
> > enabling display on their tablets.
> 
> Ah yes, I used the slightly different timings from the olimex 7" lcd
> panel for olinuxino boards, and since those worked fine on my a23
> tablet I never adjusted things. Here is a translation table:
> 
> 
> CONFIG_VIDEO_LCD_MODE		fex value(s)
> 
> x				lcd_x
> y				lcd_y
> depth:18			lcd_frm = 1
> pclk_khz			lcd_dclk_freq * 1000
> hs				lcd_hv_hspw (with a minimum of 1)
> vs				lcd_hv_vspw (with a minimum of 1)
> le				lcd_hbp - hs
> ri				lcd_ht - lcd_x - lcd_hbp
> up				lcd_vbp - vs
> 
> On sun4i/sun5i/sun7i:
> lo				(lcd_vt / 2) - lcd_y - lcd_vbp
> On sun8i:
> lo				lcd_vt - lcd_y - lcd_vbp
> 
> sync				0
> mode				0
> 
> I notice that the Ippo_q8h_v5 fex uses 0 for lcd_hv_hspw and lcd_hv_vspw, which
> is not a valid value as the register value contains hspw - 1, so the minimum is 1,
> and looking at a register dump under android with my A23 tablet the value indeed
> should be 1.

That's interesting. What would be the correct general formula for the
hs/vs values then? "max(lcd_hv_hspw, 1)" or maybe "lcd_hv_hspw + 1"?

BTW, I have done a preliminary automatic conversion for all FEX
files from sunxi-boards, which enable lcd0 in fex. The results are
now available at the all the same http://linux-sunxi.org/LCD wiki page.
If "hs = lcd_hv_hspw + 1" is a better choice, then the whole table
probably needs to be re-generated.

Also additional explanations about GPIO related options (what would be
the exact rules to interpret FEX?) and more details about "lcd_frm" and
"lcd_if" would help a lot to get a better understanding about what
still needs to be done to get LCD displays supported on all devices.

If I understand it correctly, the kernel sources from the Allwinner SDK
contain the relevant code for handling the information from FEX, and
this code is the best reference. And it's more reliable to refer to
A23 SDK for interpreting the FEX files originally snatched from A23
devices, and likewise A31 SDK for A31 devices. For example, it is not
uncommon to see both 'lcd_pwm_used' and 'lcd_pwm_not_used' variables
defined in FEX. And sometimes the values of these variables even
contradict each other. So the fine details about the relative
priorities of these variables and other similar things might need
to be discovered for perfectly correct conversion.

-- 
Best regards,
Siarhei Siamashka

[U-Boot] [PATCH 15/17] sunxi: Ippo_q8h defconfigs: Enable the LCD panel found on these tablets.

[Hans de Goede]

Hi,

On 30-12-14 11:18, Siarhei Siamashka wrote:
> On Thu, 25 Dec 2014 11:59:55 +0100
> Hans de Goede <hdegoede@redhat.com> wrote:
>
>> Hi,
>>
>> On 25-12-14 11:00, Chen-Yu Tsai wrote:
>>> Hi,
>>>
>>> On Thu, Dec 25, 2014 at 3:06 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>>> Enable the new LCD support on Ippo_q8h tablets.
>>>>
>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>> ---
>>>>    configs/Ippo_q8h_v1_2_defconfig | 5 ++++-
>>>>    configs/Ippo_q8h_v5_defconfig   | 5 ++++-
>>>>    2 files changed, 8 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/configs/Ippo_q8h_v1_2_defconfig b/configs/Ippo_q8h_v1_2_defconfig
>>>> index fefed32..c773f5f 100644
>>>> --- a/configs/Ippo_q8h_v1_2_defconfig
>>>> +++ b/configs/Ippo_q8h_v1_2_defconfig
>>>> @@ -1,7 +1,10 @@
>>>>    CONFIG_SPL=y
>>>>    CONFIG_SYS_EXTRA_OPTIONS="CONS_INDEX=5"
>>>>    CONFIG_FDTFILE="sun8i-a23-ippo-q8h-v1.2.dtb"
>>>> -CONFIG_VIDEO=n
>>>> +CONFIG_VIDEO_LCD_MODE="x:800,y:480,depth:18,pclk_khz:33000,le:16,ri:209,up:22,lo:22,hs:30,vs:1,sync:0,vmode:0"
>>>> +CONFIG_VIDEO_LCD_POWER="PH7"
>>>> +CONFIG_VIDEO_LCD_BL_EN="PH6"
>>>> +CONFIG_VIDEO_LCD_BL_PWM="PH0"
>>>>    CONFIG_USB_KEYBOARD=n
>>>>    +S:CONFIG_ARM=y
>>>>    +S:CONFIG_ARCH_SUNXI=y
>>>> diff --git a/configs/Ippo_q8h_v5_defconfig b/configs/Ippo_q8h_v5_defconfig
>>>> index b8d3afe..ce4f0b8 100644
>>>> --- a/configs/Ippo_q8h_v5_defconfig
>>>> +++ b/configs/Ippo_q8h_v5_defconfig
>>>> @@ -1,7 +1,10 @@
>>>>    CONFIG_SPL=y
>>>>    CONFIG_SYS_EXTRA_OPTIONS="CONS_INDEX=5"
>>>>    CONFIG_FDTFILE="sun8i-a23-ippo-q8h-v5.dtb"
>>>> -CONFIG_VIDEO=n
>>>> +CONFIG_VIDEO_LCD_MODE="x:800,y:480,depth:18,pclk_khz:33000,le:16,ri:209,up:22,lo:22,hs:30,vs:1,sync:0,vmode:0"
>>>
>>> The display on my Q8H is a bit off to the left. With the simplefb
>>> bindings from your kernel sunxi-wip branch, I get a nice console.
>>> Though I've no way to type, at least I can tell my tablet is on. :)
>>>
>>> Could you briefly explain how to convert the values in the fex
>>> file to the mode line here? It could also help others with
>>> enabling display on their tablets.
>>
>> Ah yes, I used the slightly different timings from the olimex 7" lcd
>> panel for olinuxino boards, and since those worked fine on my a23
>> tablet I never adjusted things. Here is a translation table:
>>
>>
>> CONFIG_VIDEO_LCD_MODE		fex value(s)
>>
>> x				lcd_x
>> y				lcd_y
>> depth:18			lcd_frm = 1
>> pclk_khz			lcd_dclk_freq * 1000
>> hs				lcd_hv_hspw (with a minimum of 1)
>> vs				lcd_hv_vspw (with a minimum of 1)
>> le				lcd_hbp - hs
>> ri				lcd_ht - lcd_x - lcd_hbp
>> up				lcd_vbp - vs
>>
>> On sun4i/sun5i/sun7i:
>> lo				(lcd_vt / 2) - lcd_y - lcd_vbp
>> On sun8i:
>> lo				lcd_vt - lcd_y - lcd_vbp
>>
>> sync				0
>> mode				0
>>
>> I notice that the Ippo_q8h_v5 fex uses 0 for lcd_hv_hspw and lcd_hv_vspw, which
>> is not a valid value as the register value contains hspw - 1, so the minimum is 1,
>> and looking at a register dump under android with my A23 tablet the value indeed
>> should be 1.
>
> That's interesting. What would be the correct general formula for the
> hs/vs values then? "max(lcd_hv_hspw, 1)" or maybe "lcd_hv_hspw + 1"?

Looking at the register values set by android vs the fex file, the correct
formula is "max(lcd_hv_hspw, 1)".

> BTW, I have done a preliminary automatic conversion for all FEX
> files from sunxi-boards, which enable lcd0 in fex. The results are
> now available at the all the same http://linux-sunxi.org/LCD wiki page.

Cool, thanks for doing this!

> If "hs = lcd_hv_hspw + 1" is a better choice, then the whole table
> probably needs to be re-generated.
>
> Also additional explanations about GPIO related options (what would be
> the exact rules to interpret FEX?) and more details about "lcd_frm" and
> "lcd_if" would help a lot to get a better understanding about what
> still needs to be done to get LCD displays supported on all devices.

Currently basically only lcd_if = 0 and lcd_frm = 1 are supported, it
should be possible to add support for other lcd_frm = x values easily,
so if you encounter those let me know, lcd_if != 0 is going to be much
harder to support and currently is not on my schedule.

> If I understand it correctly, the kernel sources from the Allwinner SDK
> contain the relevant code for handling the information from FEX, and
> this code is the best reference. And it's more reliable to refer to
> A23 SDK for interpreting the FEX files originally snatched from A23
> devices, and likewise A31 SDK for A31 devices. For example, it is not
> uncommon to see both 'lcd_pwm_used' and 'lcd_pwm_not_used' variables
> defined in FEX. And sometimes the values of these variables even
> contradict each other. So the fine details about the relative
> priorities of these variables and other similar things might need
> to be discovered for perfectly correct conversion.

All I can say here is that I agree with the above, I'm afraid I'm not
familiar enough with the (quite large) sunxi display code in the SDK
kernels to provide answers here.

Regards,

Hans

[U-Boot] [PATCH 15/17] sunxi: Ippo_q8h defconfigs: Enable the LCD panel found on these tablets.

[Hans de Goede]

Hi,

On 30-12-14 11:26, Hans de Goede wrote:
> Hi,
>
> On 30-12-14 11:18, Siarhei Siamashka wrote:

<snip>

>> BTW, I have done a preliminary automatic conversion for all FEX
>> files from sunxi-boards, which enable lcd0 in fex. The results are
>> now available at the all the same http://linux-sunxi.org/LCD wiki page.
>
> Cool, thanks for doing this!

I've just taken a look, looks good, as for the yellow entries with:

# warning: could not decode 'lcd_power' (port:power0<1><0><default><1>)

Those should be translated to:

CONFIG_VIDEO_LCD_FOO="AXP0-0" for power0
CONFIG_VIDEO_LCD_FOO="AXP0-1" for power1

And I think you can also drop the:

"# warning: 'lcd_pwm' gpio extracted from 'pwm0_para' section"

That seems to be the right thing todo for A23 at least.

Regards,

Hans

[U-Boot] [PATCH 15/17] sunxi: Ippo_q8h defconfigs: Enable the LCD panel found on these tablets.

[Siarhei Siamashka]

On Tue, 30 Dec 2014 11:36:23 +0100
Hans de Goede <hdegoede@redhat.com> wrote:

> Hi,
> 
> On 30-12-14 11:26, Hans de Goede wrote:
> > Hi,
> >
> > On 30-12-14 11:18, Siarhei Siamashka wrote:
> 
> <snip>
> 
> >> BTW, I have done a preliminary automatic conversion for all FEX
> >> files from sunxi-boards, which enable lcd0 in fex. The results are
> >> now available at the all the same http://linux-sunxi.org/LCD wiki page.
> >
> > Cool, thanks for doing this!
> 
> I've just taken a look, looks good, as for the yellow entries with:
> 
> # warning: could not decode 'lcd_power' (port:power0<1><0><default><1>)
> 
> Those should be translated to:
> 
> CONFIG_VIDEO_LCD_FOO="AXP0-0" for power0
> CONFIG_VIDEO_LCD_FOO="AXP0-1" for power1

Is this supported only for AXP209 so far? What about the devices with
a different PMIC?

> And I think you can also drop the:
> 
> "# warning: 'lcd_pwm' gpio extracted from 'pwm0_para' section"
> 
> That seems to be the right thing todo for A23 at least.

OK. Right now it looks up the "pwmX_para" section, where X is the
number from "lcd_pwm_ch" found in the "lcd0_para" section.

Thanks for your comments and clarifications.

-- 
Best regards,
Siarhei Siamashka

[U-Boot] [PATCH 15/17] sunxi: Ippo_q8h defconfigs: Enable the LCD panel found on these tablets.

[Hans de Goede]

Hi,

On 30-12-14 12:25, Siarhei Siamashka wrote:
> On Tue, 30 Dec 2014 11:36:23 +0100
> Hans de Goede <hdegoede@redhat.com> wrote:
>
>> Hi,
>>
>> On 30-12-14 11:26, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 30-12-14 11:18, Siarhei Siamashka wrote:
>>
>> <snip>
>>
>>>> BTW, I have done a preliminary automatic conversion for all FEX
>>>> files from sunxi-boards, which enable lcd0 in fex. The results are
>>>> now available at the all the same http://linux-sunxi.org/LCD wiki page.
>>>
>>> Cool, thanks for doing this!
>>
>> I've just taken a look, looks good, as for the yellow entries with:
>>
>> # warning: could not decode 'lcd_power' (port:power0<1><0><default><1>)
>>
>> Those should be translated to:
>>
>> CONFIG_VIDEO_LCD_FOO="AXP0-0" for power0
>> CONFIG_VIDEO_LCD_FOO="AXP0-1" for power1
>
> Is this supported only for AXP209 so far?

Yes.

> What about the devices with a different PMIC?

Adding support for those should be easy to do on an as needed basis,
and will use the same string notation. The string parsing code is
generic and not isolated to axp209.c .

>
>> And I think you can also drop the:
>>
>> "# warning: 'lcd_pwm' gpio extracted from 'pwm0_para' section"
>>
>> That seems to be the right thing todo for A23 at least.
>
> OK. Right now it looks up the "pwmX_para" section, where X is the
> number from "lcd_pwm_ch" found in the "lcd0_para" section.
>
> Thanks for your comments and clarifications.

You're welcome, thanks for your work on this.

Regards,

Hans

[U-Boot] [PATCH 15/17] sunxi: Ippo_q8h defconfigs: Enable the LCD panel found on these tablets.

[Siarhei Siamashka]

On Tue, 30 Dec 2014 11:26:51 +0100
Hans de Goede <hdegoede@redhat.com> wrote:

> Hi,
> 
> On 30-12-14 11:18, Siarhei Siamashka wrote:
> > On Thu, 25 Dec 2014 11:59:55 +0100
> > Hans de Goede <hdegoede@redhat.com> wrote:
> >
> >> Ah yes, I used the slightly different timings from the olimex 7" lcd
> >> panel for olinuxino boards, and since those worked fine on my a23
> >> tablet I never adjusted things. Here is a translation table:
> >>
> >>
> >> CONFIG_VIDEO_LCD_MODE		fex value(s)
> >>
> >> x				lcd_x
> >> y				lcd_y
> >> depth:18			lcd_frm = 1
> >> pclk_khz			lcd_dclk_freq * 1000
> >> hs				lcd_hv_hspw (with a minimum of 1)
> >> vs				lcd_hv_vspw (with a minimum of 1)
> >> le				lcd_hbp - hs
> >> ri				lcd_ht - lcd_x - lcd_hbp
> >> up				lcd_vbp - vs
> >>
> >> On sun4i/sun5i/sun7i:
> >> lo				(lcd_vt / 2) - lcd_y - lcd_vbp
> >> On sun8i:
> >> lo				lcd_vt - lcd_y - lcd_vbp
> >>
> >> sync				0
> >> mode				0
> >>
> >> I notice that the Ippo_q8h_v5 fex uses 0 for lcd_hv_hspw and lcd_hv_vspw, which
> >> is not a valid value as the register value contains hspw - 1, so the minimum is 1,
> >> and looking at a register dump under android with my A23 tablet the value indeed
> >> should be 1.
> >
> > That's interesting. What would be the correct general formula for the
> > hs/vs values then? "max(lcd_hv_hspw, 1)" or maybe "lcd_hv_hspw + 1"?
> 
> Looking at the register values set by android vs the fex file, the correct
> formula is "max(lcd_hv_hspw, 1)".

How can this be verified? Which hardware register needs to be read?

I can use Android to test this on Primo73 tablet, where the
hs/vs values are originally non-zero in fex.

> > BTW, I have done a preliminary automatic conversion for all FEX
> > files from sunxi-boards, which enable lcd0 in fex. The results are
> > now available at the all the same http://linux-sunxi.org/LCD wiki page.
> 
> Cool, thanks for doing this!
> 
> > If "hs = lcd_hv_hspw + 1" is a better choice, then the whole table
> > probably needs to be re-generated.
> >
> > Also additional explanations about GPIO related options (what would be
> > the exact rules to interpret FEX?) and more details about "lcd_frm" and
> > "lcd_if" would help a lot to get a better understanding about what
> > still needs to be done to get LCD displays supported on all devices.
> 
> Currently basically only lcd_if = 0 and lcd_frm = 1 are supported, it
> should be possible to add support for other lcd_frm = x values easily,
> so if you encounter those let me know, lcd_if != 0 is going to be much
> harder to support and currently is not on my schedule.

It's all in the orange part of the table at the bottom. The lcd_frm = 0
seems to be relatively common. The links to FEX files for each device
are also there in the table and can be used to confirm the details.

The http://linux-sunxi.org/Wexler_TAB_7200 tablet with its fex file
https://github.com/linux-sunxi/sunxi-boards/blob/master/sys_config/a20/wexler_tab_7200.fex
is one of the examples.

> > If I understand it correctly, the kernel sources from the Allwinner SDK
> > contain the relevant code for handling the information from FEX, and
> > this code is the best reference. And it's more reliable to refer to
> > A23 SDK for interpreting the FEX files originally snatched from A23
> > devices, and likewise A31 SDK for A31 devices. For example, it is not
> > uncommon to see both 'lcd_pwm_used' and 'lcd_pwm_not_used' variables
> > defined in FEX. And sometimes the values of these variables even
> > contradict each other. So the fine details about the relative
> > priorities of these variables and other similar things might need
> > to be discovered for perfectly correct conversion.
> 
> All I can say here is that I agree with the above, I'm afraid I'm not
> familiar enough with the (quite large) sunxi display code in the SDK
> kernels to provide answers here.

I'm not familiar with that code either. I have just started looking at
these sources, searching for some answers. For example, that's where
I found the information about the 'pwm0_para' section and some other
things. SDK is not useful for anything other than the FEX interpretation
details. For implementing the actual code we do have the hardware
documentation.

Just right now it's a good idea to figure out some basic FEX conversion
rules and probably document them in the wiki. If you can share the
rest of the information that you know about this LCD code, it would
be surely a great help for this documentation and conversion code.

I also compared the config settings from your patches with the
automatically generated records and noticed some inconsistencies.

For example, Ippo_q8h_v1_2_defconfig :

=== A part of your patch:

+CONFIG_VIDEO_LCD_POWER="PH7"
+CONFIG_VIDEO_LCD_BL_EN="PH6"
+CONFIG_VIDEO_LCD_BL_PWM="PH0"

=== My script:

# warning: could not decode 'lcd_power' (port:power2<1><0><default><1>)
CONFIG_VIDEO_LCD_BL_EN="PH6"
# warning: 'lcd_pwm' gpio extracted from 'pwm0_para' section
CONFIG_VIDEO_LCD_BL_PWM="PH0"
# warning: 'lcd_gpio_0' = 'port:PH07<1><0><default><1>'

===

In both cases we have references to PH0/PH6/PH7 pins, however my
script would pick "AXP0-2" for CONFIG_VIDEO_LCD_POWER (taking your
advise), while the role of 'lcd_gpio_0' is not quite clear.
And this is Allwinner A23, which does not use even use AXP209 PMIC.

What is actually going on with the AXP GPIO and power routing? Is AXP
GPIO really doing what we expect it to be doing?

-- 
Best regards,
Siarhei Siamashka

[U-Boot] [PATCH 15/17] sunxi: Ippo_q8h defconfigs: Enable the LCD panel found on these tablets.

[Hans de Goede]

Hi,

On 30-12-14 13:17, Siarhei Siamashka wrote:
> On Tue, 30 Dec 2014 11:26:51 +0100
> Hans de Goede <hdegoede@redhat.com> wrote:
>
>> Hi,
>>
>> On 30-12-14 11:18, Siarhei Siamashka wrote:
>>> On Thu, 25 Dec 2014 11:59:55 +0100
>>> Hans de Goede <hdegoede@redhat.com> wrote:
>>>
>>>> Ah yes, I used the slightly different timings from the olimex 7" lcd
>>>> panel for olinuxino boards, and since those worked fine on my a23
>>>> tablet I never adjusted things. Here is a translation table:
>>>>
>>>>
>>>> CONFIG_VIDEO_LCD_MODE		fex value(s)
>>>>
>>>> x				lcd_x
>>>> y				lcd_y
>>>> depth:18			lcd_frm = 1
>>>> pclk_khz			lcd_dclk_freq * 1000
>>>> hs				lcd_hv_hspw (with a minimum of 1)
>>>> vs				lcd_hv_vspw (with a minimum of 1)
>>>> le				lcd_hbp - hs
>>>> ri				lcd_ht - lcd_x - lcd_hbp
>>>> up				lcd_vbp - vs
>>>>
>>>> On sun4i/sun5i/sun7i:
>>>> lo				(lcd_vt / 2) - lcd_y - lcd_vbp
>>>> On sun8i:
>>>> lo				lcd_vt - lcd_y - lcd_vbp
>>>>
>>>> sync				0
>>>> mode				0
>>>>
>>>> I notice that the Ippo_q8h_v5 fex uses 0 for lcd_hv_hspw and lcd_hv_vspw, which
>>>> is not a valid value as the register value contains hspw - 1, so the minimum is 1,
>>>> and looking at a register dump under android with my A23 tablet the value indeed
>>>> should be 1.
>>>
>>> That's interesting. What would be the correct general formula for the
>>> hs/vs values then? "max(lcd_hv_hspw, 1)" or maybe "lcd_hv_hspw + 1"?
>>
>> Looking at the register values set by android vs the fex file, the correct
>> formula is "max(lcd_hv_hspw, 1)".
>
> How can this be verified? Which hardware register needs to be read?

Register 0x01C0C054 "TCON0_BASIC3_REG", low 16 bits contain VSPW with 0-x
meaning a vspw value of 1 - (x + 1), high 16 bits contain HSPW in the same
format.


>
> I can use Android to test this on Primo73 tablet, where the
> hs/vs values are originally non-zero in fex.


>
>>> BTW, I have done a preliminary automatic conversion for all FEX
>>> files from sunxi-boards, which enable lcd0 in fex. The results are
>>> now available at the all the same http://linux-sunxi.org/LCD wiki page.
>>
>> Cool, thanks for doing this!
>>
>>> If "hs = lcd_hv_hspw + 1" is a better choice, then the whole table
>>> probably needs to be re-generated.
>>>
>>> Also additional explanations about GPIO related options (what would be
>>> the exact rules to interpret FEX?) and more details about "lcd_frm" and
>>> "lcd_if" would help a lot to get a better understanding about what
>>> still needs to be done to get LCD displays supported on all devices.
>>
>> Currently basically only lcd_if = 0 and lcd_frm = 1 are supported, it
>> should be possible to add support for other lcd_frm = x values easily,
>> so if you encounter those let me know, lcd_if != 0 is going to be much
>> harder to support and currently is not on my schedule.
>
> It's all in the orange part of the table at the bottom. The lcd_frm = 0
> seems to be relatively common. The links to FEX files for each device
> are also there in the table and can be used to confirm the details.
>
> The http://linux-sunxi.org/Wexler_TAB_7200 tablet with its fex file
> https://github.com/linux-sunxi/sunxi-boards/blob/master/sys_config/a20/wexler_tab_7200.fex
> is one of the examples.

Ok, so I've looked this up in the linux-sunxi code again to freshen my
memory, and grepping that code gives this:

drivers/video/sunxi/disp/ebios_lcdc_tve.h
51:     LCDC_FRM_RGB888 = 0,
52:     LCDC_FRM_RGB666 = 1,
53:     LCDC_FRM_RGB656 = 2,

All 3 of which are already supported (but other then LCDC_FRM_RGB666
untested) in the u-boot lcd code :

     LCDC_FRM_RGB888 -> depth:24
     LCDC_FRM_RGB666 -> depth:18
     LCDC_FRM_RGB656 -> depth:17

So this results in the following translation:

     lcd_frm = 0  -> depth:24
     lcd_frm = 1  -> depth:18
     lcd_frm = 2  -> depth:17

>>> If I understand it correctly, the kernel sources from the Allwinner SDK
>>> contain the relevant code for handling the information from FEX, and
>>> this code is the best reference. And it's more reliable to refer to
>>> A23 SDK for interpreting the FEX files originally snatched from A23
>>> devices, and likewise A31 SDK for A31 devices. For example, it is not
>>> uncommon to see both 'lcd_pwm_used' and 'lcd_pwm_not_used' variables
>>> defined in FEX. And sometimes the values of these variables even
>>> contradict each other. So the fine details about the relative
>>> priorities of these variables and other similar things might need
>>> to be discovered for perfectly correct conversion.
>>
>> All I can say here is that I agree with the above, I'm afraid I'm not
>> familiar enough with the (quite large) sunxi display code in the SDK
>> kernels to provide answers here.
>
> I'm not familiar with that code either. I have just started looking at
> these sources, searching for some answers. For example, that's where
> I found the information about the 'pwm0_para' section and some other
> things. SDK is not useful for anything other than the FEX interpretation
> details. For implementing the actual code we do have the hardware
> documentation.
>
> Just right now it's a good idea to figure out some basic FEX conversion
> rules and probably document them in the wiki. If you can share the
> rest of the information that you know about this LCD code, it would
> be surely a great help for this documentation and conversion code.

See above for what I know about lcd_frm, if you've any more questions
feel free to ask. Making a brain dump is sort of hard to do :)

> I also compared the config settings from your patches with the
> automatically generated records and noticed some inconsistencies.
>
> For example, Ippo_q8h_v1_2_defconfig :
>
> === A part of your patch:
>
> +CONFIG_VIDEO_LCD_POWER="PH7"
> +CONFIG_VIDEO_LCD_BL_EN="PH6"
> +CONFIG_VIDEO_LCD_BL_PWM="PH0"
>
> === My script:
>
> # warning: could not decode 'lcd_power' (port:power2<1><0><default><1>)
> CONFIG_VIDEO_LCD_BL_EN="PH6"
> # warning: 'lcd_pwm' gpio extracted from 'pwm0_para' section
> CONFIG_VIDEO_LCD_BL_PWM="PH0"
> # warning: 'lcd_gpio_0' = 'port:PH07<1><0><default><1>'
>
> ===
>
> In both cases we have references to PH0/PH6/PH7 pins, however my
> script would pick "AXP0-2" for CONFIG_VIDEO_LCD_POWER (taking your
> advise), while the role of 'lcd_gpio_0' is not quite clear.
> And this is Allwinner A23, which does not use even use AXP209 PMIC.
>
> What is actually going on with the AXP GPIO and power routing? Is AXP
> GPIO really doing what we expect it to be doing?

What is likely going on here is that we do not need PH7 at all, when I
first wrote the LCD support for the Ippo_q8h_v1_2 I did not have any
axp-gpio support at all, so I decided to just ignore the axp gpio listed
in the fex and see if things would work without it, and they did, so it
seems that either the LCD does not have a power/enable pin for the lcd
itself, or at least it is ok to leave the pin floating (which is the axp
gpio default).

As for the lcd_gpio_# entries in the fex, looking at the linux-sunxi code,
it seems that they are initialized to the value specified in the fex once,
which for the Ippo_q8h_v1_2 means setting the gpio to output high once,
which we do effectively be assigning PH7 to CONFIG_VIDEO_LCD_POWER (and
I need to test if this is really necessary).

I agree that this is confusing, and that we should probably add something
akin to lcd_gpio_0 to u-boot so that we've a 1:1 translation.

A quick grep:

[hans at shalem u-boot]$ grep -R -h lcd_gpio_0 ../sunxi-boards/sys_config | sort | uniq
;lcd_gpio_0          = port:PA23<0><0><default><default>
;lcd_gpio_0          = port:PH10<1><0><2><1>
Binary file ../sunxi-boards/sys_config/a20/script.bin matches
lcd_gpio_0      =
lcd_gpio_0          =
lcd_gpio_0 =
lcd_gpio_0 = ""
lcd_gpio_0 = port:PA06<1><0><default><1>
lcd_gpio_0 = port:PH07<1><0><default><1>
lcd_gpio_0 = port:PH10<1><0><2><1>

Shows that the port is always put in output high mode, grepping for gpio_1 returns
2 boards which use gpio_1 (and higher):

sys_config/a20/icou_fatty_i.fex
sys_config/a31s/msi_primo81.fex

Both of which have a non supported cpu_if setting of 4 resp 6.

So it seems for now we can simply add a CONFIG_VIDEO_LCD_GPIO0 and always
drive the pin specified there (if any) high, and then we're good to go.

Let me know if you agree with going this route and then I'll whip up a
patch for this.

Regards,

Hans

[U-Boot] [PATCH 15/17] sunxi: Ippo_q8h defconfigs: Enable the LCD panel found on these tablets.

[Chen-Yu Tsai]

Hi Hans,

On Wed, Dec 31, 2014 at 7:22 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
>
> On 30-12-14 13:17, Siarhei Siamashka wrote:
>>
>> On Tue, 30 Dec 2014 11:26:51 +0100
>> Hans de Goede <hdegoede@redhat.com> wrote:
>>
>>> Hi,
>>>
>>> On 30-12-14 11:18, Siarhei Siamashka wrote:
>>>>
>>>> On Thu, 25 Dec 2014 11:59:55 +0100
>>>> Hans de Goede <hdegoede@redhat.com> wrote:
>>>>
>>>>> Ah yes, I used the slightly different timings from the olimex 7" lcd
>>>>> panel for olinuxino boards, and since those worked fine on my a23
>>>>> tablet I never adjusted things. Here is a translation table:
>>>>>
>>>>>
>>>>> CONFIG_VIDEO_LCD_MODE           fex value(s)
>>>>>
>>>>> x                               lcd_x
>>>>> y                               lcd_y
>>>>> depth:18                        lcd_frm = 1
>>>>> pclk_khz                        lcd_dclk_freq * 1000
>>>>> hs                              lcd_hv_hspw (with a minimum of 1)
>>>>> vs                              lcd_hv_vspw (with a minimum of 1)
>>>>> le                              lcd_hbp - hs
>>>>> ri                              lcd_ht - lcd_x - lcd_hbp
>>>>> up                              lcd_vbp - vs
>>>>>
>>>>> On sun4i/sun5i/sun7i:
>>>>> lo                              (lcd_vt / 2) - lcd_y - lcd_vbp
>>>>> On sun8i:
>>>>> lo                              lcd_vt - lcd_y - lcd_vbp
>>>>>
>>>>> sync                            0
>>>>> mode                            0
>>>>>
>>>>> I notice that the Ippo_q8h_v5 fex uses 0 for lcd_hv_hspw and
>>>>> lcd_hv_vspw, which
>>>>> is not a valid value as the register value contains hspw - 1, so the
>>>>> minimum is 1,
>>>>> and looking at a register dump under android with my A23 tablet the
>>>>> value indeed
>>>>> should be 1.
>>>>
>>>>
>>>> That's interesting. What would be the correct general formula for the
>>>> hs/vs values then? "max(lcd_hv_hspw, 1)" or maybe "lcd_hv_hspw + 1"?
>>>
>>>
>>> Looking at the register values set by android vs the fex file, the
>>> correct
>>> formula is "max(lcd_hv_hspw, 1)".
>>
>>
>> How can this be verified? Which hardware register needs to be read?
>
>
> Register 0x01C0C054 "TCON0_BASIC3_REG", low 16 bits contain VSPW with 0-x
> meaning a vspw value of 1 - (x + 1), high 16 bits contain HSPW in the same
> format.
>
>
>>
>> I can use Android to test this on Primo73 tablet, where the
>> hs/vs values are originally non-zero in fex.
>
>
>
>>
>>>> BTW, I have done a preliminary automatic conversion for all FEX
>>>> files from sunxi-boards, which enable lcd0 in fex. The results are
>>>> now available at the all the same http://linux-sunxi.org/LCD wiki page.
>>>
>>>
>>> Cool, thanks for doing this!
>>>
>>>> If "hs = lcd_hv_hspw + 1" is a better choice, then the whole table
>>>> probably needs to be re-generated.
>>>>
>>>> Also additional explanations about GPIO related options (what would be
>>>> the exact rules to interpret FEX?) and more details about "lcd_frm" and
>>>> "lcd_if" would help a lot to get a better understanding about what
>>>> still needs to be done to get LCD displays supported on all devices.
>>>
>>>
>>> Currently basically only lcd_if = 0 and lcd_frm = 1 are supported, it
>>> should be possible to add support for other lcd_frm = x values easily,
>>> so if you encounter those let me know, lcd_if != 0 is going to be much
>>> harder to support and currently is not on my schedule.
>>
>>
>> It's all in the orange part of the table at the bottom. The lcd_frm = 0
>> seems to be relatively common. The links to FEX files for each device
>> are also there in the table and can be used to confirm the details.
>>
>> The http://linux-sunxi.org/Wexler_TAB_7200 tablet with its fex file
>>
>> https://github.com/linux-sunxi/sunxi-boards/blob/master/sys_config/a20/wexler_tab_7200.fex
>> is one of the examples.
>
>
> Ok, so I've looked this up in the linux-sunxi code again to freshen my
> memory, and grepping that code gives this:
>
> drivers/video/sunxi/disp/ebios_lcdc_tve.h
> 51:     LCDC_FRM_RGB888 = 0,
> 52:     LCDC_FRM_RGB666 = 1,
> 53:     LCDC_FRM_RGB656 = 2,
>
> All 3 of which are already supported (but other then LCDC_FRM_RGB666
> untested) in the u-boot lcd code :
>
>     LCDC_FRM_RGB888 -> depth:24
>     LCDC_FRM_RGB666 -> depth:18
>     LCDC_FRM_RGB656 -> depth:17
>
> So this results in the following translation:
>
>     lcd_frm = 0  -> depth:24
>     lcd_frm = 1  -> depth:18
>     lcd_frm = 2  -> depth:17
>
>>>> If I understand it correctly, the kernel sources from the Allwinner SDK
>>>> contain the relevant code for handling the information from FEX, and
>>>> this code is the best reference. And it's more reliable to refer to
>>>> A23 SDK for interpreting the FEX files originally snatched from A23
>>>> devices, and likewise A31 SDK for A31 devices. For example, it is not
>>>> uncommon to see both 'lcd_pwm_used' and 'lcd_pwm_not_used' variables
>>>> defined in FEX. And sometimes the values of these variables even
>>>> contradict each other. So the fine details about the relative
>>>> priorities of these variables and other similar things might need
>>>> to be discovered for perfectly correct conversion.
>>>
>>>
>>> All I can say here is that I agree with the above, I'm afraid I'm not
>>> familiar enough with the (quite large) sunxi display code in the SDK
>>> kernels to provide answers here.
>>
>>
>> I'm not familiar with that code either. I have just started looking at
>> these sources, searching for some answers. For example, that's where
>> I found the information about the 'pwm0_para' section and some other
>> things. SDK is not useful for anything other than the FEX interpretation
>> details. For implementing the actual code we do have the hardware
>> documentation.
>>
>> Just right now it's a good idea to figure out some basic FEX conversion
>> rules and probably document them in the wiki. If you can share the
>> rest of the information that you know about this LCD code, it would
>> be surely a great help for this documentation and conversion code.
>
>
> See above for what I know about lcd_frm, if you've any more questions
> feel free to ask. Making a brain dump is sort of hard to do :)
>
>> I also compared the config settings from your patches with the
>> automatically generated records and noticed some inconsistencies.
>>
>> For example, Ippo_q8h_v1_2_defconfig :
>>
>> === A part of your patch:
>>
>> +CONFIG_VIDEO_LCD_POWER="PH7"
>> +CONFIG_VIDEO_LCD_BL_EN="PH6"
>> +CONFIG_VIDEO_LCD_BL_PWM="PH0"
>>
>> === My script:
>>
>> # warning: could not decode 'lcd_power' (port:power2<1><0><default><1>)
>> CONFIG_VIDEO_LCD_BL_EN="PH6"
>> # warning: 'lcd_pwm' gpio extracted from 'pwm0_para' section
>> CONFIG_VIDEO_LCD_BL_PWM="PH0"
>> # warning: 'lcd_gpio_0' = 'port:PH07<1><0><default><1>'
>>
>> ===
>>
>> In both cases we have references to PH0/PH6/PH7 pins, however my
>> script would pick "AXP0-2" for CONFIG_VIDEO_LCD_POWER (taking your
>> advise), while the role of 'lcd_gpio_0' is not quite clear.
>> And this is Allwinner A23, which does not use even use AXP209 PMIC.
>>
>> What is actually going on with the AXP GPIO and power routing? Is AXP
>> GPIO really doing what we expect it to be doing?
>
>
> What is likely going on here is that we do not need PH7 at all, when I
> first wrote the LCD support for the Ippo_q8h_v1_2 I did not have any
> axp-gpio support at all, so I decided to just ignore the axp gpio listed
> in the fex and see if things would work without it, and they did, so it
> seems that either the LCD does not have a power/enable pin for the lcd
> itself, or at least it is ok to leave the pin floating (which is the axp
> gpio default).

On the A23 reference design, LCD power is only controlled by enabling
the AXP LDO output. Downstream just uses the LDO voltage level, rather
than a GPIO, to enable the discrete regulators. Since the AXP is already
software controllable, it makes sense. But it's possible on some boards
we may need it, when the LCD power is derived from a common 5V or 3.3V
source.

>
> As for the lcd_gpio_# entries in the fex, looking at the linux-sunxi code,
> it seems that they are initialized to the value specified in the fex once,
> which for the Ippo_q8h_v1_2 means setting the gpio to output high once,
> which we do effectively be assigning PH7 to CONFIG_VIDEO_LCD_POWER (and
> I need to test if this is really necessary).

PH7 in the reference design drives LCD_RST. I guess leaving it floating
doesn't assert reset in the LCD driver. IMO, LCD_RESET would be a better
name. It's what schematics use.

> I agree that this is confusing, and that we should probably add something
> akin to lcd_gpio_0 to u-boot so that we've a 1:1 translation.
>
> A quick grep:
>
> [hans at shalem u-boot]$ grep -R -h lcd_gpio_0 ../sunxi-boards/sys_config |
> sort | uniq
> ;lcd_gpio_0          = port:PA23<0><0><default><default>
> ;lcd_gpio_0          = port:PH10<1><0><2><1>
> Binary file ../sunxi-boards/sys_config/a20/script.bin matches
> lcd_gpio_0      =
> lcd_gpio_0          =
> lcd_gpio_0 =
> lcd_gpio_0 = ""
> lcd_gpio_0 = port:PA06<1><0><default><1>
> lcd_gpio_0 = port:PH07<1><0><default><1>
> lcd_gpio_0 = port:PH10<1><0><2><1>
>
> Shows that the port is always put in output high mode, grepping for gpio_1
> returns
> 2 boards which use gpio_1 (and higher):
>
> sys_config/a20/icou_fatty_i.fex
> sys_config/a31s/msi_primo81.fex
>
> Both of which have a non supported cpu_if setting of 4 resp 6.
>
> So it seems for now we can simply add a CONFIG_VIDEO_LCD_GPIO0 and always
> drive the pin specified there (if any) high, and then we're good to go.

See the previous paragraph about the name.

Cheers
ChenYu

> Let me know if you agree with going this route and then I'll whip up a
> patch for this.
>
> Regards,
>
> Hans

[U-Boot] [PATCH 15/17] sunxi: Ippo_q8h defconfigs: Enable the LCD panel found on these tablets.

[Hans de Goede]

Hi,

On 01-01-15 03:35, Chen-Yu Tsai wrote:
> Hi Hans,
>
> On Wed, Dec 31, 2014 at 7:22 PM, Hans de Goede <hdegoede@redhat.com> wrote:

<snip>

>>> I also compared the config settings from your patches with the
>>> automatically generated records and noticed some inconsistencies.
>>>
>>> For example, Ippo_q8h_v1_2_defconfig :
>>>
>>> === A part of your patch:
>>>
>>> +CONFIG_VIDEO_LCD_POWER="PH7"
>>> +CONFIG_VIDEO_LCD_BL_EN="PH6"
>>> +CONFIG_VIDEO_LCD_BL_PWM="PH0"
>>>
>>> === My script:
>>>
>>> # warning: could not decode 'lcd_power' (port:power2<1><0><default><1>)
>>> CONFIG_VIDEO_LCD_BL_EN="PH6"
>>> # warning: 'lcd_pwm' gpio extracted from 'pwm0_para' section
>>> CONFIG_VIDEO_LCD_BL_PWM="PH0"
>>> # warning: 'lcd_gpio_0' = 'port:PH07<1><0><default><1>'
>>>
>>> ===
>>>
>>> In both cases we have references to PH0/PH6/PH7 pins, however my
>>> script would pick "AXP0-2" for CONFIG_VIDEO_LCD_POWER (taking your
>>> advise), while the role of 'lcd_gpio_0' is not quite clear.
>>> And this is Allwinner A23, which does not use even use AXP209 PMIC.
>>>
>>> What is actually going on with the AXP GPIO and power routing? Is AXP
>>> GPIO really doing what we expect it to be doing?
>>
>>
>> What is likely going on here is that we do not need PH7 at all, when I
>> first wrote the LCD support for the Ippo_q8h_v1_2 I did not have any
>> axp-gpio support at all, so I decided to just ignore the axp gpio listed
>> in the fex and see if things would work without it, and they did, so it
>> seems that either the LCD does not have a power/enable pin for the lcd
>> itself, or at least it is ok to leave the pin floating (which is the axp
>> gpio default).
>
> On the A23 reference design, LCD power is only controlled by enabling
> the AXP LDO output. Downstream just uses the LDO voltage level, rather
> than a GPIO, to enable the discrete regulators. Since the AXP is already
> software controllable, it makes sense. But it's possible on some boards
> we may need it, when the LCD power is derived from a common 5V or 3.3V
> source.

Ok.

>> As for the lcd_gpio_# entries in the fex, looking at the linux-sunxi code,
>> it seems that they are initialized to the value specified in the fex once,
>> which for the Ippo_q8h_v1_2 means setting the gpio to output high once,
>> which we do effectively be assigning PH7 to CONFIG_VIDEO_LCD_POWER (and
>> I need to test if this is really necessary).
>
> PH7 in the reference design drives LCD_RST. I guess leaving it floating
> doesn't assert reset in the LCD driver. IMO, LCD_RESET would be a better
> name. It's what schematics use.

Ok, note that the current defconfig for the q8h is driving it, but is using
CONFIG_VIDEO_LCD_POWER to drive it, which certainly seems the wrong name for
it.

>> I agree that this is confusing, and that we should probably add something
>> akin to lcd_gpio_0 to u-boot so that we've a 1:1 translation.
>>
>> A quick grep:
>>
>> [hans at shalem u-boot]$ grep -R -h lcd_gpio_0 ../sunxi-boards/sys_config |
>> sort | uniq
>> ;lcd_gpio_0          = port:PA23<0><0><default><default>
>> ;lcd_gpio_0          = port:PH10<1><0><2><1>
>> Binary file ../sunxi-boards/sys_config/a20/script.bin matches
>> lcd_gpio_0      =
>> lcd_gpio_0          =
>> lcd_gpio_0 =
>> lcd_gpio_0 = ""
>> lcd_gpio_0 = port:PA06<1><0><default><1>
>> lcd_gpio_0 = port:PH07<1><0><default><1>
>> lcd_gpio_0 = port:PH10<1><0><2><1>
>>
>> Shows that the port is always put in output high mode, grepping for gpio_1
>> returns
>> 2 boards which use gpio_1 (and higher):
>>
>> sys_config/a20/icou_fatty_i.fex
>> sys_config/a31s/msi_primo81.fex
>>
>> Both of which have a non supported cpu_if setting of 4 resp 6.
>>
>> So it seems for now we can simply add a CONFIG_VIDEO_LCD_GPIO0 and always
>> drive the pin specified there (if any) high, and then we're good to go.
>
> See the previous paragraph about the name.

Although I think that it is great that you've schematics for the q8h, we do not
have schematics for the majority of the boards we want to support, so I think it
is best to stick with the CONFIG_VIDEO_LCD_GPIO# name, and add a commment to the
defconfig what it really is in the cases where we know what it is used for.

Regards,

Hans

[U-Boot] [PATCH 15/17] sunxi: Ippo_q8h defconfigs: Enable the LCD panel found on these tablets.

[Siarhei Siamashka]

On Thu, 01 Jan 2015 13:36:11 +0100
Hans de Goede <hdegoede@redhat.com> wrote:

> Hi,
> 
> On 01-01-15 03:35, Chen-Yu Tsai wrote:
> > Hi Hans,
> >
> > On Wed, Dec 31, 2014 at 7:22 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> 
> <snip>
> 
> >>> I also compared the config settings from your patches with the
> >>> automatically generated records and noticed some inconsistencies.
> >>>
> >>> For example, Ippo_q8h_v1_2_defconfig :
> >>>
> >>> === A part of your patch:
> >>>
> >>> +CONFIG_VIDEO_LCD_POWER="PH7"
> >>> +CONFIG_VIDEO_LCD_BL_EN="PH6"
> >>> +CONFIG_VIDEO_LCD_BL_PWM="PH0"
> >>>
> >>> === My script:
> >>>
> >>> # warning: could not decode 'lcd_power' (port:power2<1><0><default><1>)
> >>> CONFIG_VIDEO_LCD_BL_EN="PH6"
> >>> # warning: 'lcd_pwm' gpio extracted from 'pwm0_para' section
> >>> CONFIG_VIDEO_LCD_BL_PWM="PH0"
> >>> # warning: 'lcd_gpio_0' = 'port:PH07<1><0><default><1>'
> >>>
> >>> ===
> >>>
> >>> In both cases we have references to PH0/PH6/PH7 pins, however my
> >>> script would pick "AXP0-2" for CONFIG_VIDEO_LCD_POWER (taking your
> >>> advise), while the role of 'lcd_gpio_0' is not quite clear.
> >>> And this is Allwinner A23, which does not use even use AXP209 PMIC.
> >>>
> >>> What is actually going on with the AXP GPIO and power routing? Is AXP
> >>> GPIO really doing what we expect it to be doing?
> >>
> >>
> >> What is likely going on here is that we do not need PH7 at all, when I
> >> first wrote the LCD support for the Ippo_q8h_v1_2 I did not have any
> >> axp-gpio support at all, so I decided to just ignore the axp gpio listed
> >> in the fex and see if things would work without it, and they did, so it
> >> seems that either the LCD does not have a power/enable pin for the lcd
> >> itself, or at least it is ok to leave the pin floating (which is the axp
> >> gpio default).
> >
> > On the A23 reference design,

Thanks for the hint. I looked through the A13, A20, A23 and A31
reference design schematics and got a lot of useful information.

> > LCD power is only controlled by enabling the AXP LDO output. Downstream just
> > uses the LDO voltage level, rather than a GPIO, to enable the discrete
> > regulators. Since the AXP is already software controllable, it makes
> > sense. But it's possible on some boards we may need it, when the LCD
> > power is derived from a common 5V or 3.3V
> > source.

Do you mean 3.0V on DCDC1SW or something else?

Now I also got LCD display in my MSI Primo81 tablet working. Because
Allwinner A31s does not have a native MIPI DSI interface, it uses
an extra SSD2828 bridge chip, which converts parallel LCD interface
into MIPI DSI. MIPI DSI interface is needed for the B079XAN01/LP079X01
7.85" IPS LCD display. Supposedly SSD2828 chip needs 1.2V on ELDO3 for
power (based on checking the kernel sources in A31 SDK), but everything
seems to work even without it. And the fex file also specifies
"lcd_power" as "port:power2<1><0><default><1>" similar to A23,
however everything power-related just happens to magically work fine
without doing anything special. This feels very suspicious and
abnormal :-)

> Ok.
> 
> >> As for the lcd_gpio_# entries in the fex, looking at the linux-sunxi code,
> >> it seems that they are initialized to the value specified in the fex once,
> >> which for the Ippo_q8h_v1_2 means setting the gpio to output high once,
> >> which we do effectively be assigning PH7 to CONFIG_VIDEO_LCD_POWER (and
> >> I need to test if this is really necessary).
> >
> > PH7 in the reference design drives LCD_RST. I guess leaving it floating
> > doesn't assert reset in the LCD driver. IMO, LCD_RESET would be a better
> > name. It's what schematics use.
> 
> Ok, note that the current defconfig for the q8h is driving it, but is using
> CONFIG_VIDEO_LCD_POWER to drive it, which certainly seems the wrong name for
> it.
> 
> >> I agree that this is confusing, and that we should probably add something
> >> akin to lcd_gpio_0 to u-boot so that we've a 1:1 translation.
> >>
> >> A quick grep:
> >>
> >> [hans at shalem u-boot]$ grep -R -h lcd_gpio_0 ../sunxi-boards/sys_config |
> >> sort | uniq
> >> ;lcd_gpio_0          = port:PA23<0><0><default><default>
> >> ;lcd_gpio_0          = port:PH10<1><0><2><1>
> >> Binary file ../sunxi-boards/sys_config/a20/script.bin matches
> >> lcd_gpio_0      =
> >> lcd_gpio_0          =
> >> lcd_gpio_0 =
> >> lcd_gpio_0 = ""
> >> lcd_gpio_0 = port:PA06<1><0><default><1>
> >> lcd_gpio_0 = port:PH07<1><0><default><1>
> >> lcd_gpio_0 = port:PH10<1><0><2><1>
> >>
> >> Shows that the port is always put in output high mode, grepping for gpio_1
> >> returns
> >> 2 boards which use gpio_1 (and higher):
> >>
> >> sys_config/a20/icou_fatty_i.fex
> >> sys_config/a31s/msi_primo81.fex
> >>
> >> Both of which have a non supported cpu_if setting of 4 resp 6.
> >>
> >> So it seems for now we can simply add a CONFIG_VIDEO_LCD_GPIO0 and always
> >> drive the pin specified there (if any) high, and then we're good to go.
> >
> > See the previous paragraph about the name.
> 
> Although I think that it is great that you've schematics for the q8h, we do not
> have schematics for the majority of the boards we want to support, so I think it
> is best to stick with the CONFIG_VIDEO_LCD_GPIO# name, and add a commment to the
> defconfig what it really is in the cases where we know what it is used for.

BTW, the MSI Primo81 tablet (lcd_if=6) uses a set of four pins in the
'lcd_gpio_*' fex config. Three of them are used for SPI communication
to configure the SSD2828 chip (write only). And one more pin is reset.
The reset pin is not "lcd_gpio_0", but "lcd_gpio_2". So yes, the exact
pin naming may depend on the "lcd_if" config variable and also maybe
even change between different Allwinner SDK versions.

I'll submit the lcd_if=6 (LCD_IF_EXT_DSI) support code soon, but the
original Allwinner's SPI bit-banging code needs to be reworked to get
rid of magic constants. Given that the SSD2828 datasheet exists, this
should be doable.

Do we really want to keep the magic CONFIG_VIDEO_LCD_GPIO# names?

-- 
Best regards,
Siarhei Siamashka

[U-Boot] [PATCH 15/17] sunxi: Ippo_q8h defconfigs: Enable the LCD panel found on these tablets.

[Hans de Goede]

Hi,

On 02-01-15 12:02, Siarhei Siamashka wrote:

<snip>

> Now I also got LCD display in my MSI Primo81 tablet working. Because
> Allwinner A31s does not have a native MIPI DSI interface, it uses
> an extra SSD2828 bridge chip, which converts parallel LCD interface
> into MIPI DSI.

Cool!

> MIPI DSI interface is needed for the B079XAN01/LP079X01
> 7.85" IPS LCD display. Supposedly SSD2828 chip needs 1.2V on ELDO3 for
> power (based on checking the kernel sources in A31 SDK), but everything
> seems to work even without it. And the fex file also specifies
> "lcd_power" as "port:power2<1><0><default><1>" similar to A23,
> however everything power-related just happens to magically work fine
> without doing anything special. This feels very suspicious and
> abnormal :-)
>
>> Ok.
>>
>>>> As for the lcd_gpio_# entries in the fex, looking at the linux-sunxi code,
>>>> it seems that they are initialized to the value specified in the fex once,
>>>> which for the Ippo_q8h_v1_2 means setting the gpio to output high once,
>>>> which we do effectively be assigning PH7 to CONFIG_VIDEO_LCD_POWER (and
>>>> I need to test if this is really necessary).
>>>
>>> PH7 in the reference design drives LCD_RST. I guess leaving it floating
>>> doesn't assert reset in the LCD driver. IMO, LCD_RESET would be a better
>>> name. It's what schematics use.
>>
>> Ok, note that the current defconfig for the q8h is driving it, but is using
>> CONFIG_VIDEO_LCD_POWER to drive it, which certainly seems the wrong name for
>> it.
>>
>>>> I agree that this is confusing, and that we should probably add something
>>>> akin to lcd_gpio_0 to u-boot so that we've a 1:1 translation.
>>>>
>>>> A quick grep:
>>>>
>>>> [hans at shalem u-boot]$ grep -R -h lcd_gpio_0 ../sunxi-boards/sys_config |
>>>> sort | uniq
>>>> ;lcd_gpio_0          = port:PA23<0><0><default><default>
>>>> ;lcd_gpio_0          = port:PH10<1><0><2><1>
>>>> Binary file ../sunxi-boards/sys_config/a20/script.bin matches
>>>> lcd_gpio_0      =
>>>> lcd_gpio_0          =
>>>> lcd_gpio_0 =
>>>> lcd_gpio_0 = ""
>>>> lcd_gpio_0 = port:PA06<1><0><default><1>
>>>> lcd_gpio_0 = port:PH07<1><0><default><1>
>>>> lcd_gpio_0 = port:PH10<1><0><2><1>
>>>>
>>>> Shows that the port is always put in output high mode, grepping for gpio_1
>>>> returns
>>>> 2 boards which use gpio_1 (and higher):
>>>>
>>>> sys_config/a20/icou_fatty_i.fex
>>>> sys_config/a31s/msi_primo81.fex
>>>>
>>>> Both of which have a non supported cpu_if setting of 4 resp 6.
>>>>
>>>> So it seems for now we can simply add a CONFIG_VIDEO_LCD_GPIO0 and always
>>>> drive the pin specified there (if any) high, and then we're good to go.
>>>
>>> See the previous paragraph about the name.
>>
>> Although I think that it is great that you've schematics for the q8h, we do not
>> have schematics for the majority of the boards we want to support, so I think it
>> is best to stick with the CONFIG_VIDEO_LCD_GPIO# name, and add a commment to the
>> defconfig what it really is in the cases where we know what it is used for.
>
> BTW, the MSI Primo81 tablet (lcd_if=6) uses a set of four pins in the
> 'lcd_gpio_*' fex config. Three of them are used for SPI communication
> to configure the SSD2828 chip (write only). And one more pin is reset.
> The reset pin is not "lcd_gpio_0", but "lcd_gpio_2". So yes, the exact
> pin naming may depend on the "lcd_if" config variable and also maybe
> even change between different Allwinner SDK versions.

Ah interesting, I had some trouble getting the theoretically easy addition
of lvds going on my A10 tablet because the hitachi lcd used in it has a
lcd controller which needs some poking over SPI for it to power up.

> I'll submit the lcd_if=6 (LCD_IF_EXT_DSI) support code soon, but the
> original Allwinner's SPI bit-banging code needs to be reworked to get
> rid of magic constants. Given that the SSD2828 datasheet exists, this
> should be doable.

Sounds good, my own support for the hitachi lcd needed some cleanup,
which I've just completed. I've just posted the series to the mailinglist,
and it is available in my sunxi-wip branch. As you can see I've taken into
account that we also need support for the SSD2828 and made the VIDEO_LCD_PANEL
Kconfig option a choice, so adding one more special panel should be easy,
just slot it in in the Kconfig and add it to drivers/video/sunxi_lcd_panel.c

I've found a few other boards using the same hitachi panel, but they all use
the same gpios for the spi bit-banging, which seems to be a different set
from what is being used in the primo for the SSD2828, I think it is easiest to
just hardcode the gpio-s needed for the spi inside drivers/video/sunxi_lcd_panel.c
for now, based on the lcd panel type, if we ever need to make things more
flexible because some new board comes along which uses an already supported
lcd panel / conversion chip, but with a different set of gpio-s then we can
easily make things more flexible then.

> Do we really want to keep the magic CONFIG_VIDEO_LCD_GPIO# names?

Given that in your case they are being (ab)used to code spi pins, no. I'll
do a patch one of the coming days to introduce a CONFIG_VIDEO_LCD_RESET Kconfig
option, I think it would be be safe for your fex conversion script to fill that
with gpio0 for boards which only set gpio0, and just complain in case there is
more then one gpio and not do anything with the gpio-s.

Talking about your script, lcd_if = 3 is supported with my LVDS patches, this
should select CONFIG_VIDEO_LCD_PANEL_LVDS, lcd_lvds_ch, lcd_lvds_mode and
lcd_lvds_io_cross should all be 0, otherwise we're dealing with an unsupported
config. lcd_lvds_bitwidth = 0 means depth:24, lcd_lvds_bitwidth = 1 means depth:18,
for lvds this takes precedence over whatever lcd_frm sets. Note some lvds using
fex files set lcd_lvds_bitwidth = 1 without enabling dithering through lcd_frm,
this makes no sense, and the SDK kernel code even complains about it, so I've
no intention of supporting 18 bits over lvds without dithering.

Regards,

Hans

[U-Boot] [PATCH 15/17] sunxi: Ippo_q8h defconfigs: Enable the LCD panel found on these tablets.

[Siarhei Siamashka]

On Wed, 31 Dec 2014 12:22:17 +0100
Hans de Goede <hdegoede@redhat.com> wrote:

> On 30-12-14 13:17, Siarhei Siamashka wrote:
> > On Tue, 30 Dec 2014 11:26:51 +0100
> > Hans de Goede <hdegoede@redhat.com> wrote:
> >
> >> Currently basically only lcd_if = 0 and lcd_frm = 1 are supported, it
> >> should be possible to add support for other lcd_frm = x values easily,
> >> so if you encounter those let me know, lcd_if != 0 is going to be much
> >> harder to support and currently is not on my schedule.
> >
> > It's all in the orange part of the table at the bottom. The lcd_frm = 0
> > seems to be relatively common. The links to FEX files for each device
> > are also there in the table and can be used to confirm the details.
> >
> > The http://linux-sunxi.org/Wexler_TAB_7200 tablet with its fex file
> > https://github.com/linux-sunxi/sunxi-boards/blob/master/sys_config/a20/wexler_tab_7200.fex
> > is one of the examples.
> 
> Ok, so I've looked this up in the linux-sunxi code again to freshen my
> memory, and grepping that code gives this:
> 
> drivers/video/sunxi/disp/ebios_lcdc_tve.h
> 51:     LCDC_FRM_RGB888 = 0,
> 52:     LCDC_FRM_RGB666 = 1,
> 53:     LCDC_FRM_RGB656 = 2,
> 
> All 3 of which are already supported (but other then LCDC_FRM_RGB666
> untested) in the u-boot lcd code :
> 
>      LCDC_FRM_RGB888 -> depth:24
>      LCDC_FRM_RGB666 -> depth:18
>      LCDC_FRM_RGB656 -> depth:17
> 
> So this results in the following translation:
> 
>      lcd_frm = 0  -> depth:24
>      lcd_frm = 1  -> depth:18
>      lcd_frm = 2  -> depth:17

In fact the 'lcd_frm' FEX option only controls dithering. The color
depth is controlled by the 'lcd_cpu_if' FEX option. These color
depth settings end up in the TCON0_CPU_IF_REG register, and your
LCD patches currently unconditionally set it to 18-bit depth
anyway.

There is also a wiki page, describing FEX settings with a lot of
useful information there:
    http://linux-sunxi.org/Fex_Guide#lcd.5B0.2F1.5D_configuration

As for the dithering settings, you can try to compare the results of
having "depth:0" vs. "depth:18" in CONFIG_VIDEO_LCD_MODE and using
the following simple test program.

With "depth:0", the picture looks correct, but blocky (6 bits per
color component is not great for gradients without dithering).
With "depth:18", the picture looks smooth.

/******** Display a gradient picture ************/
#include <stdint.h>
#include <stdio.h>
#include <fcntl.h>
#include <linux/fb.h>
#include <sys/ioctl.h>
#include <sys/mman.h>

int main()
{
    int fd, x, y;
    uint32_t *fb;
    struct fb_fix_screeninfo finfo;
    struct fb_var_screeninfo vinfo;

    if ((fd = open("/dev/fb0", O_RDWR)) == -1) {
        printf("Can't open /dev/fb0\n");
        return 1;
    }

    if (ioctl(fd, FBIOGET_FSCREENINFO, &finfo)) {
        printf("FBIOGET_FSCREENINFO failed\n");
        return 1;
    }

    if (ioctl(fd, FBIOGET_VSCREENINFO, &vinfo)) {
        printf("FBIOGET_VSCREENINFO failed\n");
        return 1;
    }

    if (vinfo.bits_per_pixel != 32) {
        printf("Only 32bpp framebuffer is supported\n");
        return 1;
    }

    fb = mmap(0, finfo.smem_len, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
    if (fb == (void *)-1) {
        printf("mmap failed\n");
        return 1;
    }

    for (y = 0; y < vinfo.yres; y++)
        for (x = 0; x < vinfo.xres; x++)
            fb[y * vinfo.xres + x] = (255 * x / vinfo.xres) * 0x000100 +
                                     (255 * y / vinfo.yres) * 0x010001;

    return 0;
}
/************************************************/

So "lcd_frm = 0" is not expected to cause any problems and is
already supported. But naming and meaning of the options in the
CONFIG_VIDEO_LCD_MODE string could be improved.

I just wonder why there are so many FEX files with lcd_frm = 0. It
does not seem to be a great idea, unless there is some good reason.

-- 
Best regards,
Siarhei Siamashka

[U-Boot] [PATCH 15/17] sunxi: Ippo_q8h defconfigs: Enable the LCD panel found on these tablets.

[Hans de Goede]

Hi,

On 01-01-15 21:03, Siarhei Siamashka wrote:
> On Wed, 31 Dec 2014 12:22:17 +0100
> Hans de Goede <hdegoede@redhat.com> wrote:
>
>> On 30-12-14 13:17, Siarhei Siamashka wrote:
>>> On Tue, 30 Dec 2014 11:26:51 +0100
>>> Hans de Goede <hdegoede@redhat.com> wrote:
>>>
>>>> Currently basically only lcd_if = 0 and lcd_frm = 1 are supported, it
>>>> should be possible to add support for other lcd_frm = x values easily,
>>>> so if you encounter those let me know, lcd_if != 0 is going to be much
>>>> harder to support and currently is not on my schedule.
>>>
>>> It's all in the orange part of the table at the bottom. The lcd_frm = 0
>>> seems to be relatively common. The links to FEX files for each device
>>> are also there in the table and can be used to confirm the details.
>>>
>>> The http://linux-sunxi.org/Wexler_TAB_7200 tablet with its fex file
>>> https://github.com/linux-sunxi/sunxi-boards/blob/master/sys_config/a20/wexler_tab_7200.fex
>>> is one of the examples.
>>
>> Ok, so I've looked this up in the linux-sunxi code again to freshen my
>> memory, and grepping that code gives this:
>>
>> drivers/video/sunxi/disp/ebios_lcdc_tve.h
>> 51:     LCDC_FRM_RGB888 = 0,
>> 52:     LCDC_FRM_RGB666 = 1,
>> 53:     LCDC_FRM_RGB656 = 2,
>>
>> All 3 of which are already supported (but other then LCDC_FRM_RGB666
>> untested) in the u-boot lcd code :
>>
>>       LCDC_FRM_RGB888 -> depth:24
>>       LCDC_FRM_RGB666 -> depth:18
>>       LCDC_FRM_RGB656 -> depth:17
>>
>> So this results in the following translation:
>>
>>       lcd_frm = 0  -> depth:24
>>       lcd_frm = 1  -> depth:18
>>       lcd_frm = 2  -> depth:17
>
> In fact the 'lcd_frm' FEX option only controls dithering. The color
> depth is controlled by the 'lcd_cpu_if' FEX option. These color
> depth settings end up in the TCON0_CPU_IF_REG register, and your
> LCD patches currently unconditionally set it to 18-bit depth
> anyway.

Erm, no lcd_cpu_if only gets used if lcd_if == 1, likewise the
lcd_ttl settings only get used if the lcd_if == 2.

Which reminds me your script should check that lcd_hv_if == 0,
because we do not support the other variants atm.

> There is also a wiki page, describing FEX settings with a lot of
> useful information there:
>      http://linux-sunxi.org/Fex_Guide#lcd.5B0.2F1.5D_configuration
>
> As for the dithering settings, you can try to compare the results of
> having "depth:0" vs. "depth:18" in CONFIG_VIDEO_LCD_MODE and using
> the following simple test program.
>
> With "depth:0", the picture looks correct, but blocky (6 bits per
> color component is not great for gradients without dithering).
> With "depth:18", the picture looks smooth.
>
> /******** Display a gradient picture ************/
> #include <stdint.h>
> #include <stdio.h>
> #include <fcntl.h>
> #include <linux/fb.h>
> #include <sys/ioctl.h>
> #include <sys/mman.h>
>
> int main()
> {
>      int fd, x, y;
>      uint32_t *fb;
>      struct fb_fix_screeninfo finfo;
>      struct fb_var_screeninfo vinfo;
>
>      if ((fd = open("/dev/fb0", O_RDWR)) == -1) {
>          printf("Can't open /dev/fb0\n");
>          return 1;
>      }
>
>      if (ioctl(fd, FBIOGET_FSCREENINFO, &finfo)) {
>          printf("FBIOGET_FSCREENINFO failed\n");
>          return 1;
>      }
>
>      if (ioctl(fd, FBIOGET_VSCREENINFO, &vinfo)) {
>          printf("FBIOGET_VSCREENINFO failed\n");
>          return 1;
>      }
>
>      if (vinfo.bits_per_pixel != 32) {
>          printf("Only 32bpp framebuffer is supported\n");
>          return 1;
>      }
>
>      fb = mmap(0, finfo.smem_len, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
>      if (fb == (void *)-1) {
>          printf("mmap failed\n");
>          return 1;
>      }
>
>      for (y = 0; y < vinfo.yres; y++)
>          for (x = 0; x < vinfo.xres; x++)
>              fb[y * vinfo.xres + x] = (255 * x / vinfo.xres) * 0x000100 +
>                                       (255 * y / vinfo.yres) * 0x010001;
>
>      return 0;
> }
> /************************************************/
>
> So "lcd_frm = 0" is not expected to cause any problems and is
> already supported. But naming and meaning of the options in the
> CONFIG_VIDEO_LCD_MODE string could be improved.

That string uses a standard u-boot parsing function, so we cannot easily
change it.

> I just wonder why there are so many FEX files with lcd_frm = 0. It
> does not seem to be a great idea, unless there is some good reason.

The parallel lcd interface which is used in most cases, and which we
support has 8 data pins for each color, which is why using 24 bit mode
also works with non 24 bit displays, they simply ignore the lower bits
likely displays which use lcd_frm = 0 actually use all 24 bits.

Regards,

Hans

p.s.

1) I'm working on support for lvds displays as I have an A10 tablet
lying around which I've never worked on until now (it was confiscated
by my children) and it actually turns out to have an lvds display,
so cpu_if = 3 should be supported soonish.

2) I'm going afk for 3 days, so my next reply in this thread will be
a bit slower then usual.

[U-Boot] [PATCH 15/17] sunxi: Ippo_q8h defconfigs: Enable the LCD panel found on these tablets.

[Siarhei Siamashka]

On Thu, 01 Jan 2015 21:15:58 +0100
Hans de Goede <hdegoede@redhat.com> wrote:

> Hi,
> 
> On 01-01-15 21:03, Siarhei Siamashka wrote:
> > On Wed, 31 Dec 2014 12:22:17 +0100
> > Hans de Goede <hdegoede@redhat.com> wrote:
> >
> >> On 30-12-14 13:17, Siarhei Siamashka wrote:
> >>> On Tue, 30 Dec 2014 11:26:51 +0100
> >>> Hans de Goede <hdegoede@redhat.com> wrote:
> >>>
> >>>> Currently basically only lcd_if = 0 and lcd_frm = 1 are supported, it
> >>>> should be possible to add support for other lcd_frm = x values easily,
> >>>> so if you encounter those let me know, lcd_if != 0 is going to be much
> >>>> harder to support and currently is not on my schedule.
> >>>
> >>> It's all in the orange part of the table at the bottom. The lcd_frm = 0
> >>> seems to be relatively common. The links to FEX files for each device
> >>> are also there in the table and can be used to confirm the details.
> >>>
> >>> The http://linux-sunxi.org/Wexler_TAB_7200 tablet with its fex file
> >>> https://github.com/linux-sunxi/sunxi-boards/blob/master/sys_config/a20/wexler_tab_7200.fex
> >>> is one of the examples.
> >>
> >> Ok, so I've looked this up in the linux-sunxi code again to freshen my
> >> memory, and grepping that code gives this:
> >>
> >> drivers/video/sunxi/disp/ebios_lcdc_tve.h
> >> 51:     LCDC_FRM_RGB888 = 0,
> >> 52:     LCDC_FRM_RGB666 = 1,
> >> 53:     LCDC_FRM_RGB656 = 2,
> >>
> >> All 3 of which are already supported (but other then LCDC_FRM_RGB666
> >> untested) in the u-boot lcd code :
> >>
> >>       LCDC_FRM_RGB888 -> depth:24
> >>       LCDC_FRM_RGB666 -> depth:18
> >>       LCDC_FRM_RGB656 -> depth:17
> >>
> >> So this results in the following translation:
> >>
> >>       lcd_frm = 0  -> depth:24
> >>       lcd_frm = 1  -> depth:18
> >>       lcd_frm = 2  -> depth:17
> >
> > In fact the 'lcd_frm' FEX option only controls dithering. The color
> > depth is controlled by the 'lcd_cpu_if' FEX option. These color
> > depth settings end up in the TCON0_CPU_IF_REG register, and your
> > LCD patches currently unconditionally set it to 18-bit depth
> > anyway.
> 
> Erm, no lcd_cpu_if only gets used if lcd_if == 1, likewise the
> lcd_ttl settings only get used if the lcd_if == 2.

Appears that the wiki page is not entirely correct and just needs to
be updated. Happens all the time.

> Which reminds me your script should check that lcd_hv_if == 0,
> because we do not support the other variants atm.

OK.

> > There is also a wiki page, describing FEX settings with a lot of
> > useful information there:
> >      http://linux-sunxi.org/Fex_Guide#lcd.5B0.2F1.5D_configuration
> >
> > As for the dithering settings, you can try to compare the results of
> > having "depth:0" vs. "depth:18" in CONFIG_VIDEO_LCD_MODE and using
> > the following simple test program.
> >
> > With "depth:0", the picture looks correct, but blocky (6 bits per
> > color component is not great for gradients without dithering).
> > With "depth:18", the picture looks smooth.
> >
> > /******** Display a gradient picture ************/
> > #include <stdint.h>
> > #include <stdio.h>
> > #include <fcntl.h>
> > #include <linux/fb.h>
> > #include <sys/ioctl.h>
> > #include <sys/mman.h>
> >
> > int main()
> > {
> >      int fd, x, y;
> >      uint32_t *fb;
> >      struct fb_fix_screeninfo finfo;
> >      struct fb_var_screeninfo vinfo;
> >
> >      if ((fd = open("/dev/fb0", O_RDWR)) == -1) {
> >          printf("Can't open /dev/fb0\n");
> >          return 1;
> >      }
> >
> >      if (ioctl(fd, FBIOGET_FSCREENINFO, &finfo)) {
> >          printf("FBIOGET_FSCREENINFO failed\n");
> >          return 1;
> >      }
> >
> >      if (ioctl(fd, FBIOGET_VSCREENINFO, &vinfo)) {
> >          printf("FBIOGET_VSCREENINFO failed\n");
> >          return 1;
> >      }
> >
> >      if (vinfo.bits_per_pixel != 32) {
> >          printf("Only 32bpp framebuffer is supported\n");
> >          return 1;
> >      }
> >
> >      fb = mmap(0, finfo.smem_len, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
> >      if (fb == (void *)-1) {
> >          printf("mmap failed\n");
> >          return 1;
> >      }
> >
> >      for (y = 0; y < vinfo.yres; y++)
> >          for (x = 0; x < vinfo.xres; x++)
> >              fb[y * vinfo.xres + x] = (255 * x / vinfo.xres) * 0x000100 +
> >                                       (255 * y / vinfo.yres) * 0x010001;
> >
> >      return 0;
> > }
> > /************************************************/
> >
> > So "lcd_frm = 0" is not expected to cause any problems and is
> > already supported. But naming and meaning of the options in the
> > CONFIG_VIDEO_LCD_MODE string could be improved.
> 
> That string uses a standard u-boot parsing function, so we cannot easily
> change it.

We could use other options. If we had to. But appears that we don't :-)

> > I just wonder why there are so many FEX files with lcd_frm = 0. It
> > does not seem to be a great idea, unless there is some good reason.
> 
> The parallel lcd interface which is used in most cases, and which we
> support has 8 data pins for each color, which is why using 24 bit mode
> also works with non 24 bit displays, they simply ignore the lower bits
> likely displays which use lcd_frm = 0 actually use all 24 bits.

Right. So the color depth does not strictly need to be set correctly
for the parallel lcd interface. And the only user visible effect is
proper or wrong dithering.

The users of devices with "lcd_frm = 0" in FEX can just run the gradient
test program and check if the color transitions are blocky or smooth.

Anyway, your u-boot LCD code already supports "lcd_frm = 0" and it
indeed seems to be reasonable to convert it to "depth:24" in
CONFIG_VIDEO_LCD_MODE.

Thanks.

> Regards,
> 
> Hans
> 
> p.s.
> 
> 1) I'm working on support for lvds displays as I have an A10 tablet
> lying around which I've never worked on until now (it was confiscated
> by my children) and it actually turns out to have an lvds display,
> so cpu_if = 3 should be supported soonish.

That's great.

> 2) I'm going afk for 3 days, so my next reply in this thread will be
> a bit slower then usual.

-- 
Best regards,
Siarhei Siamashka

[U-Boot] [PATCH 16/17] sunxi: video: Remove sunxi_display.enabled variable

[Hans de Goede]

Having both a sunxi_display.enabled variable and
sunxi_display.monitor == sunxi_monitor_none duplicates state, use
sunxi_display.monitor = sunxi_monitor_none when ever we do not have a display.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/video/sunxi_display.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/video/sunxi_display.c b/drivers/video/sunxi_display.c
index 624e5e0..a169d78 100644
--- a/drivers/video/sunxi_display.c
+++ b/drivers/video/sunxi_display.c
@@ -33,7 +33,6 @@ enum sunxi_monitor {
 
 struct sunxi_display {
 	GraphicDevice graphic_device;
-	bool enabled;
 	enum sunxi_monitor monitor;
 	unsigned int depth;
 } sunxi_display;
@@ -828,6 +827,7 @@ void *video_hw_init(void)
 	case sunxi_monitor_hdmi:
 #ifndef CONFIG_VIDEO_HDMI
 		printf("HDMI/DVI not supported on this board\n");
+		sunxi_display.monitor = sunxi_monitor_none;
 		return NULL;
 #else
 		/* Always call hdp_detect, as it also enables clocks, etc. */
@@ -843,8 +843,10 @@ void *video_hw_init(void)
 
 		sunxi_hdmi_shutdown();
 
-		if (lcd_mode[0] == 0)
+		if (lcd_mode[0] == 0) {
+			sunxi_display.monitor = sunxi_monitor_none;
 			return NULL; /* No LCD, bail */
+		}
 
 		/* Fall back / through to LCD */
 		sunxi_display.monitor = sunxi_monitor_lcd;
@@ -856,6 +858,7 @@ void *video_hw_init(void)
 			break;
 		}
 		printf("LCD not supported on this board\n");
+		sunxi_display.monitor = sunxi_monitor_none;
 		return NULL;
 	case sunxi_monitor_vga:
 #ifdef CONFIG_VIDEO_VGA_VIA_LCD
@@ -863,6 +866,7 @@ void *video_hw_init(void)
 		break;
 #else
 		printf("VGA not supported on this board\n");
+		sunxi_display.monitor = sunxi_monitor_none;
 		return NULL;
 #endif
 	}
@@ -875,7 +879,6 @@ void *video_hw_init(void)
 		       mode->yres, mon_desc[sunxi_display.monitor]);
 	}
 
-	sunxi_display.enabled = true;
 	sunxi_engines_init();
 	sunxi_mode_set(mode, gd->fb_base - CONFIG_SYS_SDRAM_BASE);
 
@@ -903,9 +906,6 @@ int sunxi_simplefb_setup(void *blob)
 	int offset, ret;
 	const char *pipeline = NULL;
 
-	if (!sunxi_display.enabled)
-		return 0;
-
 	switch (sunxi_display.monitor) {
 	case sunxi_monitor_none:
 		return 0;
-- 
2.1.0

[U-Boot] [PATCH 16/17] sunxi: video: Remove sunxi_display.enabled variable

[Ian Campbell]

On Wed, 2014-12-24 at 20:06 +0100, Hans de Goede wrote:
> Having both a sunxi_display.enabled variable and
> sunxi_display.monitor == sunxi_monitor_none duplicates state, use
> sunxi_display.monitor = sunxi_monitor_none when ever we do not have a display.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Ian Campbell <ijc@hellion.org.uk>

[U-Boot] [PATCH 17/17] sunxi: video: Use sunxi_lcdc_get_clk_delay to calculate tcon1 delay

[Hans de Goede]

Use sunxi_lcdc_get_clk_delay to calculate tcon1 delay instead of hardcoding
it to 30. We will still end up using 30 for most modes, but for e.g. 800x600
this makes a (small) difference.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/video/sunxi_display.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/video/sunxi_display.c b/drivers/video/sunxi_display.c
index a169d78..a20ff57 100644
--- a/drivers/video/sunxi_display.c
+++ b/drivers/video/sunxi_display.c
@@ -577,15 +577,15 @@ static void sunxi_lcdc_tcon1_mode_set(const struct ctfb_res_modes *mode,
 {
 	struct sunxi_lcdc_reg * const lcdc =
 		(struct sunxi_lcdc_reg *)SUNXI_LCD0_BASE;
-	int bp, total;
+	int bp, clk_delay, total;
 
 	/* Use tcon1 */
 	clrsetbits_le32(&lcdc->ctrl, SUNXI_LCDC_CTRL_IO_MAP_MASK,
 			SUNXI_LCDC_CTRL_IO_MAP_TCON1);
 
-	/* Enabled, 0x1e start delay */
+	clk_delay = sunxi_lcdc_get_clk_delay(mode);
 	writel(SUNXI_LCDC_TCON1_CTRL_ENABLE |
-	       SUNXI_LCDC_TCON1_CTRL_CLK_DELAY(0x1e), &lcdc->tcon1_ctrl);
+	       SUNXI_LCDC_TCON1_CTRL_CLK_DELAY(clk_delay), &lcdc->tcon1_ctrl);
 
 	writel(SUNXI_LCDC_X(mode->xres) | SUNXI_LCDC_Y(mode->yres),
 	       &lcdc->tcon1_timing_source);
-- 
2.1.0

[U-Boot] [PATCH 17/17] sunxi: video: Use sunxi_lcdc_get_clk_delay to calculate tcon1 delay

[Ian Campbell]

On Wed, 2014-12-24 at 20:06 +0100, Hans de Goede wrote:
> Use sunxi_lcdc_get_clk_delay to calculate tcon1 delay instead of hardcoding
> it to 30. We will still end up using 30 for most modes, but for e.g. 800x600
> this makes a (small) difference.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Ian Campbell <ijc@hellion.org.uk>