[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 at 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 at 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