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

Siarhei Siamashka
2015-01-01T21:05:27Z

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

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

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

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

OK.

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

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

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

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

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

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

Thanks.

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

That's great.

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

--
Best regards,
Siarhei Siamashka