Switched to Bgr to fix Red/Blue colours being swapped #13
Conversation
src/models/st7735s.rs
Outdated
| @@ -52,7 +52,7 @@ impl Model for ST7735s { | |||
| write_command(di, Instruction::PGC, &[0x10, 0x0E, 0x02, 0x03, 0x0E, 0x07, 0x02, 0x07, 0x0A, 0x12, 0x27, 0x37, 0x00, 0x0D, 0x0E, 0x10])?; // set GAMMA +Polarity characteristics | |||
| write_command(di, Instruction::NGC, &[0x10, 0x0E, 0x03, 0x03, 0x0F, 0x06, 0x02, 0x08, 0x0A, 0x13, 0x26, 0x36, 0x00, 0x0D, 0x0E, 0x10])?; // set GAMMA -Polarity characteristics | |||
| write_command(di, Instruction::COLMOD, &[0b0101_0101])?; // set interface pixel format, 16bit pixel into frame memory | |||
| write_command(di, Instruction::MADCTL, &[0b1010_1000])?; // set memory data access control, Top -> Bottom, RGB, Left -> Right | |||
| write_command(di, Instruction::MADCTL, &[0b1010_0000])?; // set memory data access control, Top -> Bottom, RGB, Left -> Right | |||
There was a problem hiding this comment.
This bit changed is MH, "Horizontal refresh order control". Probably don't need to change this in order to fix the colours.
There was a problem hiding this comment.
I just double checked Datasheet 1.4 and D3 is RGB.
Do you have a link to the Datasheet you have available?
There was a problem hiding this comment.
Sorry, I was out by 1 bit, reading the docs. Your are right. Trying to understand the need to change the ColorFormat, would it be sufficient just to change the MADCTL value?
There was a problem hiding this comment.
Setting the MADCTL bit had no effect without also switching to BGR.
Just confirming that this is indeed BE C code?
static void LCD_0IN96_SendData_16Bit(UWORD Data)
{
DEV_Digital_Write(LCD_DC_PIN, 1);
DEV_Digital_Write(LCD_CS_PIN, 0);
DEV_SPI_WriteByte((Data >> 8) & 0xFF);
DEV_SPI_WriteByte(Data);
DEV_Digital_Write(LCD_CS_PIN, 1);
}There was a problem hiding this comment.
I might be getting confused here. To me it sounds like setting the MADCTL RGB bit has no effect. I just tried setting this bit on my ILI99488 device, which supposedly has the same MADCTL RGB bit. Absolutely no change that I can see, all colours are the exactly same. Odd. Wonder what the point of this config option is when it does nothing. Maybe hardware dependant.
I think you swap the colours by switching the colour model, and that means when you call c.into_storage() the colours get serialised in the alternate order:
https://github.com/almindor/mipidsi/blob/master/src/models/st7789.rs#L63
I believe there might be two ways to make this configurable:
-
At compile time. Make
ColorFormata generic parameter to theST7789struct. I think this should be possible... -
Do the serialisation "by hand". e.g. have a look at https://github.com/almindor/mipidsi/blob/master/src/models/ili9486.rs#L98-L103 - ideally for efficiency probably only want to do this in such a way that you test config once, not once for every individual colour.
There was a problem hiding this comment.
I have switched to serialising manually after finding this in the C drivers clear method
Color = ((Color<<8)&0xff00)|(Color>>8);although this just swaps the MSB and the LSB I found that the G range of colours would be wrong doing it this way.
|
The datasheets that I have referenced stipulate RGB565. Checking the C code showed that Big Endian was used to write the pixel data. I briefly tried using Little Endian to write the data instead. This solved the red/blue problem but then created a problem with the 6 bits for green. Switching the MADCTL bit and the ColourFormat solves red/green/blue for this particular device. I do not have additional st7735s based devices to verify if this will work for all iterations. Making it configurable would be a good solution. At this point in time I do not have enough experience to do this. |
…dditional BGR changes.
…und in the C driver.
Fixes #10 for the st7735s.