Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Song-aff opened this issue Apr 18, 2024 · 18 comments
Open

Comments

@Song-aff
Copy link

first,I use the master branch code.

cargo.toml

[package]
name = "esp32-mipidsi-test"
version = "0.1.0"
authors = ["song <[email protected]>"]
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
Copy link
Collaborator

rfuest commented Apr 18, 2024

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

@Song-aff
Copy link
Author

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
Copy link
Collaborator

rfuest commented Apr 18, 2024

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
Copy link
Author

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
Copy link
Author

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

@rfuest
Copy link
Collaborator

rfuest commented Apr 19, 2024

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
Copy link
Owner

@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
Copy link
Collaborator

rfuest commented Apr 19, 2024

OK, I'll try to reproduce the issue.

@Song-aff
Copy link
Author

[package]
name = "esp32-mipidsi-test"
version = "0.1.0"
authors = ["song <[email protected]>"]
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
Copy link
Author

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
Copy link
Collaborator

rfuest commented Apr 19, 2024

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
Copy link
Collaborator

rfuest commented Apr 19, 2024

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
Copy link
Collaborator

rfuest commented Apr 20, 2024

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
Copy link

bugadani commented Apr 20, 2024

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
Copy link
Collaborator

rfuest commented Apr 20, 2024

Nice, thanks for taking care of that.

@bugadani
Copy link

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

@rfuest
Copy link
Collaborator

rfuest commented Apr 24, 2024

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

@almindor
Copy link
Owner

Could we close this issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants