almindor / st7789

Rust library for displays using the ST7735 driver
https://docs.rs/st7735-lcd
MIT License
47 stars 24 forks source link

16 bit parallel mode incorrectly implemented #9

Closed therealprof closed 4 years ago

therealprof commented 4 years ago

The driver is currently using write_word to send associated command data, however this is implemented as

    // Writes a data word to the display.
    fn write_word(&mut self, value: u16) -> Result<(), Error<PinE>> {
        use display_interface::DataFormat::U16;
        self.di
            .send_data(U16(&[value.to_be()]))
            .map_err(|_| Error::DisplayError)
    }

which means it'll try to send data as 16 bit value, however the only display commands accepting 16 bit values are RAMWR and RAMWRC, i.e. the ones used to send pixel data. Everything else must be sent as multiple 8bit values, even in 16bit parallel mode.

almindor commented 4 years ago

I might be wrong but I think this is actually OK. write_word is only used for RAMWR and CASET/RASET but those expect "16 bit" values too...

therealprof commented 4 years ago

No, because they always need to be sent in two separate 8 bit values but if you specifiy a 16 bit Datasize, a 16 bit DI impl will not split it up but send it as a single 16 Bit value.

therealprof commented 4 years ago
Screen Shot 2020-05-18 at 09 38 51

You can clearly see that the column D8-15 is empty and there're 4 rows with 8 bit transfers.

almindor commented 4 years ago

Ah yes you're right. I'll switch it to U8

almindor commented 4 years ago

Fixed

therealprof commented 4 years ago

Can we get a release? ;)

almindor commented 4 years ago

We did, v0.4.2