diff mbox

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

Message ID 1419447989-21959-16-git-send-email-hdegoede@redhat.com
State Accepted
Delegated to: Ian Campbell
Headers show

Commit Message

Hans de Goede Dec. 24, 2014, 7:06 p.m. UTC
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(-)

Comments

Chen-Yu Tsai Dec. 25, 2014, 10 a.m. UTC | #1
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@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
Hans de Goede Dec. 25, 2014, 10:59 a.m. UTC | #2
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
Chen-Yu Tsai Dec. 26, 2014, 6:44 a.m. UTC | #3
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
Hans de Goede Dec. 26, 2014, 10:48 a.m. UTC | #4
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
Ian Campbell Dec. 29, 2014, 1:57 p.m. UTC | #5
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.
Chen-Yu Tsai Dec. 29, 2014, 3:56 p.m. UTC | #6
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.
Hans de Goede Dec. 29, 2014, 7:31 p.m. UTC | #7
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
Siarhei Siamashka Dec. 30, 2014, 10:18 a.m. UTC | #8
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.
Hans de Goede Dec. 30, 2014, 10:26 a.m. UTC | #9
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
Hans de Goede Dec. 30, 2014, 10:36 a.m. UTC | #10
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
Siarhei Siamashka Dec. 30, 2014, 11:25 a.m. UTC | #11
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.
Siarhei Siamashka Dec. 30, 2014, 12:17 p.m. UTC | #12
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?
Hans de Goede Dec. 31, 2014, 11:22 a.m. UTC | #13
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
Hans de Goede Dec. 31, 2014, 11:22 a.m. UTC | #14
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@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
Chen-Yu Tsai Jan. 1, 2015, 2:35 a.m. UTC | #15
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@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
Hans de Goede Jan. 1, 2015, 12:36 p.m. UTC | #16
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@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
Siarhei Siamashka Jan. 1, 2015, 8:03 p.m. UTC | #17
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.
Hans de Goede Jan. 1, 2015, 8:15 p.m. UTC | #18
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.
Siarhei Siamashka Jan. 1, 2015, 9:05 p.m. UTC | #19
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.
Siarhei Siamashka Jan. 2, 2015, 11:02 a.m. UTC | #20
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@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?
Hans de Goede Jan. 4, 2015, 8:22 p.m. UTC | #21
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@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
diff mbox

Patch

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