almindor / mipidsi

MIPI Display Serial Interface unified driver
MIT License
108 stars 47 forks source link

The ST7789 device displays abnormalities when using the master branch. #129

Open Song-aff opened 2 months ago

Song-aff commented 2 months ago

first,I use the master branch code.

cargo.toml

[package]
name = "esp32-mipidsi-test"
version = "0.1.0"
authors = ["song <281218023@qq.com>"]
edition = "2021"
license = "MIT OR Apache-2.0"
[dependencies]
embedded-hal = "1.0.0"
hal = { package = "esp-hal", version = "0.16.1" ,features = [ "esp32c3","eh1" ] }
esp-backtrace = { version = "0.11.1", features = ["esp32c3", "panic-handler", "exception-handler", "println"] }
esp-println = { version = "0.9.1", features = ["esp32c3"] }
embedded-graphics = "0.8.0"
display-interface-spi = "0.5.0"
mipidsi ={git="https://github.com/almindor/mipidsi",branch = "master"}
fugit = "0.3.7"
embedded-hal-bus = "0.1.0"

[features]
ILI9341 = []
ILI9342 = []
ILI9386 = []
ST7735s = []
ST7789 = []
ST7796 = []
[profile.dev]
# Rust debug is too slow. 
# For debug builds always builds with some optimization
opt-level = "s"
[profile.release]
codegen-units = 1 # LLVM can perform better optimizations using a single thread
debug = 2
debug-assertions = false
incremental = false
lto = 'fat'
opt-level = 's'
overflow-checks = false

main.rs

#![no_std]
#![no_main]

/* --- Needed by ESP32-c3 --- */
use esp_backtrace as _;
use hal::{
    clock::ClockControl,
    peripherals::Peripherals,
    prelude::*,
    spi::{master::Spi, SpiMode},
    gpio::{NO_PIN,IO},
    Delay
};
/* -------------------------- */

use embedded_graphics::{
    pixelcolor::Rgb565,
    prelude::*,
    primitives::{Circle, Primitive, PrimitiveStyle, Rectangle, Triangle},
};

// Provides the parallel port and display interface builders
use display_interface_spi::SPIInterface;

// Provides the Display builder
use mipidsi::Builder;

use fugit::RateExtU32;
use esp_println::println;

#[entry]
fn main() -> ! {

    let peripherals = Peripherals::take();
    let system = peripherals.SYSTEM.split();
    let clocks = ClockControl::max(system.clock_control).freeze();
    let mut delay = Delay::new(&clocks);
    let io = IO::new(peripherals.GPIO, peripherals.IO_MUX);

    let dc = io.pins.gpio7.into_push_pull_output();
    let mut rst = io.pins.gpio8.into_push_pull_output();
    rst.set_high().unwrap();

    // Define the SPI pins and create the SPI interface
    let sck = io.pins.gpio5;
    // let miso = io.pins.gpio4;
    let mosi = io.pins.gpio6;
    let cs = io.pins.gpio10.into_push_pull_output();

    let spi = Spi::new(peripherals.SPI2, 60u32.MHz(), SpiMode::Mode0, &clocks).with_pins(
        Some(sck),
        Some(mosi),
        NO_PIN,
        NO_PIN,
    );

    let spi_device = embedded_hal_bus::spi::ExclusiveDevice::new(spi, cs, delay);
    let di = SPIInterface::new(spi_device, dc);

    let display_modal = mipidsi::models::ST7735s;

    #[cfg(feature = "ST7735s")]
    let display_modal = mipidsi::models::ST7735s;
    #[cfg(feature = "ST7789")]
    let display_modal = mipidsi::models::ST7789;
    #[cfg(feature = "ST7796")]
    let display_modal = mipidsi::models::ST7796;
    #[cfg(feature = "ILI9341")]
    let display_modal = mipidsi::models::ILI9341Rgb565;
    #[cfg(feature = "ILI9342")]
    let display_modal = mipidsi::models::ILI9342CRgb565;
    #[cfg(feature = "ILI9386")]
    let display_modal = mipidsi::models::ILI9486Rgb565;

    let mut display = mipidsi::Builder::new(display_modal, di)
    // .display_size(160, 200)
    .reset_pin(rst)
    .init(&mut delay)
    .unwrap();

    // Make the display all black
    display.clear(Rgb565::BLACK).unwrap();
    // Draw a smiley face with white eyes and a red mouth
    // draw_smiley(&mut display).unwrap();
    demo(&mut display).unwrap();
    loop {
        // Do nothing
    }
}

fn draw_smiley<T: DrawTarget<Color = Rgb565>>(display: &mut T) -> Result<(), T::Error> {
    // Draw the left eye as a circle located at (50, 100), with a diameter of 40, filled with white
    Circle::new(Point::new(50, 100), 40)
        .into_styled(PrimitiveStyle::with_fill(Rgb565::WHITE))
        .draw(display)?;

    // Draw the right eye as a circle located at (50, 200), with a diameter of 40, filled with white
    Circle::new(Point::new(50, 200), 40)
        .into_styled(PrimitiveStyle::with_fill(Rgb565::WHITE))
        .draw(display)?;

    // Draw an upside down red triangle to represent a smiling mouth
    Triangle::new(
        Point::new(130, 140),
        Point::new(130, 200),
        Point::new(160, 170),
    )
    .into_styled(PrimitiveStyle::with_fill(Rgb565::RED))
    .draw(display)?;

    // Cover the top part of the mouth with a black triangle so it looks closed instead of open
    Triangle::new(
        Point::new(130, 150),
        Point::new(130, 190),
        Point::new(150, 170),
    )
    .into_styled(PrimitiveStyle::with_fill(Rgb565::BLACK))
    .draw(display)?;
    Ok(())
}

fn demo<T: DrawTarget<Color = Rgb565>>(display: &mut T) -> Result<(), T::Error> {
        // Draw the left eye as a circle located at (50, 100), with a diameter of 40, filled with white
        Circle::new(Point::new(0, 0), 40)
        .into_styled(PrimitiveStyle::with_fill(Rgb565::WHITE))
        .draw(display)?;
    Ok(())
}
The ST7789 device performance in the following models (x for white screen) Model ILI9341 ILI9342 ILI9386 ST7735 ST7789 ST7796
Display Mirror Mirror x x x x

ST7735 and ST7796 devices are both normal.

Link: https://github.com/Song-aff/esp32-mipidsi-test

When using mipidsi = "0.7.1"

    let mut display = Builder::st7789(di).init(&mut delay, Some(rst)).unwrap();

The display is normal.

rfuest commented 2 months ago

What do you mean by "white screen"? Is the display inverted or is nothing visible at all?

Song-aff commented 2 months ago

What do you mean by "white screen"? Is the display inverted or is nothing visible at all?

The entire screen is white, with only the backlight on, resembling an uninitialized state.

rfuest commented 2 months ago

That's strange we didn't really change the initialization of the ST7789 between 0.7.1 and the latest GIT revision. I only have access to a cheap ST7789 without a CS pin, which did work with the master branch. It might be an issue with how the CS pin is handled by the new embedded-hal and display-interface versions. Can you capture the signals on the SPI bus with an oscilloscope or logic analyzer?

You could also try to run the display without using the CS pin. This isn't a great solution, but it might help to narrow down the issue:

/// Noop impl of OutputPin.
struct NoCs;

impl OutputPin for NoCs {
    fn set_low(&mut self) -> Result<(), Self::Error> {
        Ok(())
    }

    fn set_high(&mut self) -> Result<(), Self::Error> {
        Ok(())
    }
}

impl embedded_hal::digital::ErrorType for NoCs {
    type Error = core::convert::Infallible;
}

// replace this:
    let cs = io.pins.gpio10.into_push_pull_output();

    let spi = Spi::new(peripherals.SPI2, 60u32.MHz(), SpiMode::Mode0, &clocks).with_pins(
        Some(sck),
        Some(mosi),
        NO_PIN,
        NO_PIN,
    );

    let spi_device = embedded_hal_bus::spi::ExclusiveDevice::new(spi, cs, delay);
    let di = SPIInterface::new(spi_device, dc);
// with:
    let cs = io.pins.gpio10.into_push_pull_output();
    cs.set_low().unwrap();

    let spi = Spi::new(peripherals.SPI2, 10u32.MHz(), SpiMode::Mode3, &clocks).with_pins(
        Some(sck),
        Some(mosi),
        NO_PIN,
        NO_PIN,
    );

    let spi_device = embedded_hal_bus::spi::ExclusiveDevice::new_no_delay(spi, NoCS);
    let di = SPIInterface::new(spi_device, dc);
Song-aff commented 2 months ago

That's strange we didn't really change the initialization of the ST7789 between 0.7.1 and the latest GIT revision. I only have access to a cheap ST7789 without a CS pin, which did work with the master branch. It might be an issue with how the CS pin is handled by the new embedded-hal and display-interface versions. Can you capture the signals on the SPI bus with an oscilloscope or logic analyzer?

You could also try to run the display without using the CS pin. This isn't a great solution, but it might help to narrow down the issue:

/// Noop impl of OutputPin.
struct NoCs;

impl OutputPin for NoCs {
    fn set_low(&mut self) -> Result<(), Self::Error> {
        Ok(())
    }

    fn set_high(&mut self) -> Result<(), Self::Error> {
        Ok(())
    }
}

impl embedded_hal::digital::ErrorType for NoCs {
    type Error = core::convert::Infallible;
}

// replace this:
    let cs = io.pins.gpio10.into_push_pull_output();

    let spi = Spi::new(peripherals.SPI2, 60u32.MHz(), SpiMode::Mode0, &clocks).with_pins(
        Some(sck),
        Some(mosi),
        NO_PIN,
        NO_PIN,
    );

    let spi_device = embedded_hal_bus::spi::ExclusiveDevice::new(spi, cs, delay);
    let di = SPIInterface::new(spi_device, dc);
// with:
    let cs = io.pins.gpio10.into_push_pull_output();
    cs.set_low().unwrap();

    let spi = Spi::new(peripherals.SPI2, 10u32.MHz(), SpiMode::Mode3, &clocks).with_pins(
        Some(sck),
        Some(mosi),
        NO_PIN,
        NO_PIN,
    );

    let spi_device = embedded_hal_bus::spi::ExclusiveDevice::new_no_delay(spi, NoCS);
    let di = SPIInterface::new(spi_device, dc);

According to your modifications, it can now function, but it seems to be mirrored.

Song-aff commented 2 months ago

I am an amateur player and do not have these devices at hand

rfuest commented 2 months ago

According to your modifications, it can now function, but it seems to be mirrored.

Do you mean that the coordinates are mirrored or are the colors mirrored? If the colors are inverted (e.g. black is displayed as white), add .with_invert_colors(mipidsi::options::ColorInversion::Inverted) to the builder call (see the troubleshooting guide for more information about incorrect colors). If the coordinates are inverted add something like .with_orientation(Orientation::default().flip_horizontal()).

I am an amateur player and do not have these devices at hand

No worries, thanks for reporting this issue and testing the workaround. We'll try to reproduce the issue.

@almindor: Do you have a ESP-C3 to test the difference between the master version and 0.7.1?.

almindor commented 2 months ago

@almindor: Do you have a ESP-C3 to test the difference between the master version and 0.7.1?.

I'm afraid not, I'm actually stuck on a single Red-V (RISC-V) MCU atm, and am the only party responsible for updating the HAL to v1.0 on it as well. My testing is essentially blocked due to this. I do hope to get that resolved in a week or so, but I have no other functioning MCUs atm.

rfuest commented 2 months ago

OK, I'll try to reproduce the issue.

Song-aff commented 2 months ago
[package]
name = "esp32-mipidsi-test"
version = "0.1.0"
authors = ["song <281218023@qq.com>"]
edition = "2021"
license = "MIT OR Apache-2.0"

[dependencies]
hal = { package = "esp32c3-hal", version = "0.10.0" }
esp-backtrace = { version = "0.7.0", features = [
    "esp32c3",
    "panic-handler",
    "exception-handler",
    "print-uart",
] }
esp-println = { version = "0.5.0", features = ["esp32c3"] }
embedded-graphics = "0.8.0"
display-interface-spi = "0.4.1"
mipidsi = "0.7.1"
fugit = "0.3.7"

[profile.dev]
# Rust debug is too slow. 
# For debug builds always builds with some optimization
opt-level = "s"

[profile.release]
codegen-units = 1        # LLVM can perform better optimizations using a single thread
debug = 2
debug-assertions = false
incremental = false
lto = 'fat'
opt-level = 's'
overflow-checks = false
#![no_std]
#![no_main]

/* --- Needed by ESP32-c3 --- */
use esp_backtrace as _;
use hal::{
    clock::ClockControl,
    peripherals::Peripherals,
    prelude::*,
    spi::{Spi, SpiMode},
    timer::TimerGroup,
    Delay, Rtc, IO,
};
/* -------------------------- */

use embedded_graphics::{
    pixelcolor::Rgb565,
    prelude::*,
    primitives::{Circle, Primitive, PrimitiveStyle, Rectangle, Triangle},
};

// Provides the parallel port and display interface builders
use display_interface_spi::SPIInterfaceNoCS;

// Provides the Display builder
use mipidsi::Builder;

use fugit::RateExtU32;

#[entry]
fn main() -> ! {
    let peripherals = Peripherals::take();
    let mut system = peripherals.SYSTEM.split();
    let clocks = ClockControl::boot_defaults(system.clock_control).freeze();
    let io = IO::new(peripherals.GPIO, peripherals.IO_MUX);

    // Disable the RTC and TIMG watchdog timers
    let mut rtc = Rtc::new(peripherals.RTC_CNTL);
    let timer_group0 = TimerGroup::new(
        peripherals.TIMG0,
        &clocks,
        &mut system.peripheral_clock_control,
    );
    let mut wdt0 = timer_group0.wdt;
    let timer_group1 = TimerGroup::new(
        peripherals.TIMG1,
        &clocks,
        &mut system.peripheral_clock_control,
    );
    let mut wdt1 = timer_group1.wdt;
    rtc.swd.disable();
    rtc.rwdt.disable();
    wdt0.disable();
    wdt1.disable();

    // Define the delay struct, needed for the display driver
    let mut delay = Delay::new(&clocks);

    // Define the Data/Command select pin as a digital output
    let dc = io.pins.gpio7.into_push_pull_output();
    // Define the reset pin as digital outputs and make it high
    let mut rst = io.pins.gpio8.into_push_pull_output();
    rst.set_high().unwrap();

    // Define the SPI pins and create the SPI interface
    let sck = io.pins.gpio5;
    let miso = io.pins.gpio4;
    let mosi = io.pins.gpio6;
    let cs = io.pins.gpio10;
    let spi = Spi::new(
        peripherals.SPI2,
        sck,
        mosi,
        miso,
        cs,
        60_u32.MHz(),
        SpiMode::Mode0,
        &mut system.peripheral_clock_control,
        &clocks,
    );

    // Define the display interface with no chip select
    let di = SPIInterfaceNoCS::new(spi, dc);

    // Define the display from the display interface and initialize it
    let mut display = Builder::st7789(di).init(&mut delay, Some(rst)).unwrap();

    // Make the display all black
    display.clear(Rgb565::BLACK).unwrap();

    // Draw a smiley face with white eyes and a red mouth
    draw_smiley(&mut display).unwrap();

    loop {
        // Do nothing
    }
}

fn draw_smiley<T: DrawTarget<Color = Rgb565>>(display: &mut T) -> Result<(), T::Error> {
    // Draw the left eye as a circle located at (50, 100), with a diameter of 40, filled with white
    Circle::new(Point::new(50, 100), 40)
        .into_styled(PrimitiveStyle::with_fill(Rgb565::WHITE))
        .draw(display)?;

    // Draw the right eye as a circle located at (50, 200), with a diameter of 40, filled with white
    Circle::new(Point::new(50, 200), 40)
        .into_styled(PrimitiveStyle::with_fill(Rgb565::WHITE))
        .draw(display)?;

    // Draw an upside down red triangle to represent a smiling mouth
    Triangle::new(
        Point::new(130, 140),
        Point::new(130, 200),
        Point::new(160, 170),
    )
    .into_styled(PrimitiveStyle::with_fill(Rgb565::RED))
    .draw(display)?;

    // Cover the top part of the mouth with a black triangle so it looks closed instead of open
    Triangle::new(
        Point::new(130, 150),
        Point::new(130, 190),
        Point::new(150, 170),
    )
    .into_styled(PrimitiveStyle::with_fill(Rgb565::BLACK))
    .draw(display)?;

    Ok(())
}

In this situation, it is working normally.

Song-aff commented 2 months ago

According to your modifications, it can now function, but it seems to be mirrored.

Do you mean that the coordinates are mirrored or are the colors mirrored? If the colors are inverted (e.g. black is displayed as white), add to the builder call (see the troubleshooting guide for more information about incorrect colors). If the coordinates are inverted add something like ..with_invert_colors(mipidsi::options::ColorInversion::Inverted)``.with_orientation(Orientation::default().flip_horizontal())

I am an amateur player and do not have these devices at hand

No worries, thanks for reporting this issue and testing the workaround. We'll try to reproduce the issue.

@almindor: Do you have a ESP-C3 to test the difference between the version and ?.master``0.7.1

coordinates are mirrored

rfuest commented 2 months ago

coordinates are mirrored

In version 0.7.1 you'll need to add something like .with_orientation(Orientation::Portrait(true)) to your builder call to mirror the display, where true means that the display will be mirrored. You might need to experiment with other orientations to get the desired effect: https://docs.rs/mipidsi/latest/mipidsi/options/enum.Orientation.html.

rfuest commented 2 months ago

I've just tried the closest configuration I have at the moment, which is a ESP32-S3 with a ST7789 display attached via a parallel interface. This still works as expected with the master branch. I'll do further testing with a SPI display when my ESP32-C3 arrives.

rfuest commented 2 months ago

My ESP32-C3 arrived and after some surgery on my cheap ST7789 module to add a CS connection I was able to reproduce the problem.

The problem is caused by an invalid initial state of the CS line. It is low after .into_push_pull_output() is called and ExclusiveDevice::new doesn't set a valid initial state for CS. This way the first command gets corrupted and the controller doesn't get out of sleep mode. Adding this code in front of the ExclusiveDevice::new call fixes the issue for me:

let mut cs = cs.into_push_pull_output();
cs.set_high();

Ping @bugadani: Do you think this is a bug in ExclusiveDevice? I'm asking you because I noticed your name in the ExclusiveDevice GIT history and as the (de facto?) maintainer of display-interface-spi.

bugadani commented 2 months ago

Thanks for the ping. As I'm not an embedded-hal team member, I don't feel comfortable to just propose an API change like this, so I posted the issue as a discussion point for the next Rust Embedded meeting.

rfuest commented 2 months ago

Nice, thanks for taking care of that.

bugadani commented 2 months ago

embedded-hal-bus 0.2 has been released recently. The new constructor now sets the CS high regardless of its initial value.

rfuest commented 2 months ago

@Song-aff: Can you verify that updating embedded-hal-bus to 0.2 fixes your issue with the GIT version of mipidsi?