Jump to content

a bug in the ILI9488 driver?


kapacuk

Recommended Posts

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?

 

 

 

 

Link to comment
Share on other sites

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);
        }

 

Link to comment
Share on other sites

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?

Link to comment
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
×
×
  • Create New...