a bug in the ILI9488 driver?

[kapacuk]

I bought an ILI9488 module recently, and it took me a few hours to make it work with uGFX. I think I found a bug in the ILI9488 driver. The driver uses this command the set the RGB565 mode:

	write_index(g, 0x3A);
	write_data(g, 0x55);

This mode requires two bytes per pixel, but the gdisp_lld_write_color() function calls write_data() only once. Everywhere else the driver expects write_data() to send only the lower byte to the controller, as in the example above. However, gdisp_lld_write_color() assumes that write_data() would transmit both bytes of the 16-bit data.

There is no way to implement write_data() so that it would send either one byte or two bytes, depending on the context. Well, there might be a way, but it would be ugly.

I would expect gdisp_lld_write_color to call write_data16(), as ILI9341 driver does. Ideally, I'd prefer to be able to implement write_data16() myself in the board file, as it could be done a bit more efficiently than just calling write_data() twice. Or, even better, to remove the assumption about the color mode, the board file could implement these two functions:
 

static GFXINLINE void write_byte(GDisplay *g, gU8 data);
static GFXINLINE void write_pixel(GDisplay *g, gColor pixel);

This would allow support for 18bits/pixel and 24bits/pixel as well.

What do you think?

 

 

 

 

[Guest Good Samaritan]

I had this exact same issue. The solution I have is this...

(I did not use the drivers provided by the uGFX repository, so this code may not be directly usable...)

    #if GDISP_HARDWARE_STREAM_WRITE
        static uint8 ColorWriteState = 0;
        static uint16 ColorFirst = 0;
        static uint8 ColorNext = 0;
        
        LLDSPEC    void gdisp_lld_write_start(GDisplay *g)
        {
            acquire_bus(g);
            
            set_viewport(g);
            write_index(g, MEMORY_WRITE);
        }
        
        LLDSPEC    void gdisp_lld_write_color(GDisplay *g)
        {
            if (ColorWriteState == 0)
            {
                /*Save B and G for writing next time*/
                ColorFirst = (uint16)((BLUE_OF(g->p.color) << 8) | GREEN_OF(g->p.color));
                /*Save R for the next time*/
                ColorNext = RED_OF(g->p.color);
                /*Switch state*/
                ColorWriteState = !ColorWriteState;
                return;
            }
            else
            {
                /*Write first B and G*/
                write_data(g, ColorFirst);
                /*Write first R and second B*/
                write_data(g, (uint16)((ColorNext << 8) | BLUE_OF(g->p.color)));
                /*Write second G and R*/
                write_data(g, (uint16)((GREEN_OF(g->p.color) << 8) | RED_OF(g->p.color)));
                
                ColorWriteState = !ColorWriteState;
            }
        }
        
        LLDSPEC    void gdisp_lld_write_stop(GDisplay *g)
        {
            
            if (ColorWriteState)
            {
                /*Write first B and G*/
                write_data(g, ColorFirst);
                /*Write first R and second B*/
                write_data(g, (uint16)(ColorNext << 8));
                ColorWriteState = 0;
            }
            release_bus(g);
        }

 

[kapacuk]

I was wondering if this driver ever worked for anybody at all. I think I understand now. It turned out that ILI9488 can be configured to use either SPI or the parallel interface (8 bit or 16-bit). I'm using the 4-line SPI interface, but I guess that the driver was designed for 16-bit parallel interface only, which is part of the problem. The driver should not make any assumptions about the connection type, that's the job of the board file.

Also, depending on the connection type, some colour modes may be not available. In particular, the RGB565 mode used by the driver is only allowed for parallel connections. For the SPI connection the choice is between RGB111 (1 byte) or RGB666 (2 bytes). The logical conclusion is that the colour mode should also be defined in the board file.

I'd like to submit a patch for the driver, but I don't see any way to fix it without breaking the backward compatibility. I can fork it under a different name (any suggestions?) and fix the new one. Is any of the maintainers reading this? Would you accept it?

[inmarket]

 

Please post the patch files or better please put in a pull request for your changes. If it breaks other code (board files etc) that code will need to be appropriately fixed.

[kapacuk]

I've submitted a pull request (https://git.ugfx.io/uGFX/uGFX/pulls/41). I tested it with the SPI connection, but I don't have a module with 16-bit parallel connection. I don't see why it would not work, but it would be nice if somebody could test it on a parallel module.

Edited November 29, 2019 by kapacuk