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

MIPI DSI Display HAL #473

Merged
merged 9 commits into from
Jan 27, 2024
Merged

MIPI DSI Display HAL #473

merged 9 commits into from
Jan 27, 2024

Conversation

romixlab
Copy link
Contributor

@romixlab romixlab commented Dec 1, 2023

Implement DSI driver with support for both Video and Adapted command modes. Bidirectional communication over DSI also works, which is usually used to configure display controllers.
Add examples for H747 Disco board (H747 Eval also works with minor modifications).
This reuses previous LTDC related work from this repo, the only difference is that RGB IO's are not initialised and instead DSI block takes over.

There is a small bug / discrepancy in the LTDC implementation that I have a dirty fix for, but for know I've left it untouched, which leads to every other frame being skipped in Adapted command mode. Video mode fully works.
There are three examples, two for Video mode and one for Command. I've spent many weeks troubleshooting tearing effects visible when fast moving objects are rendered, but finally was able to solve it by using portrait mode and correct timings, teartest example showcases it.

We've been using this code for a while at work and I finally got a permission to publish it.

There are links to my fork of embedded-hal in Cargo.toml, where I've added DSI related trait and some data structures common to this driver and glass controller crates. For now it's only OTM8009A (used on the EVK's display). Not sure if this is the best way though.

H747 Time Circuits

@olback
Copy link
Member

olback commented Dec 4, 2023

I have tested this on the Arduino H7 outputting over USB-C. I've not gotten it to work correctly, but it does indeed output video!

I had bunch of trouble getting the clock configuration correct, but that's another issue.

On the H7, there is a MIPI to DisplayPort converter, an ANX7625, which does not have a publicly available datasheet/rm which makes it incredibly hard to know if what I'm doing is correct, so I don't know what the problems I'm having is caused by.

@romixlab
Copy link
Contributor Author

romixlab commented Dec 4, 2023

I would imagine it won't work just like that, there are a lot of configuration register writes performed to properly initialize the display controller (located on the display glass itself). Aslo timings are most likely wrong. So best case you will see very wrong jumping image or nothing at all. That's a nice little board, having a USB-C output would be great, but without the datasheet it's really hard.

Configuration of external ICs has nothing to do with this crate though, for the EVK display I've extracted that part into a separate one (otm8009a).

@romixlab
Copy link
Contributor Author

romixlab commented Dec 4, 2023

Which board do you have, ABX00042 ABX00045 or ABX00046? I can probly get one and give it a try as well.

@olback
Copy link
Member

olback commented Dec 4, 2023

Setup:

Portenta H7 (ABX00042), Portenta Breakout (ASX00031), some no-name-brand USB-C dongle with HDMI and USB-C power in.

ANX7625 (MIPI-DSI to DP)

For the ANX7625 library, I'm using a modified version of the code found here (Arduino_H7_Video) and here (Linux kernel).
I have not re-written this in Rust, I'm using FFI to interract with it. This "Product Brief" is all i find.
https://github.com/olback/anx7625-2-rs

Clock config code

To get the desired LTDC clock, I had to configure PLL3 manually:

// HSE 25MHz
//
// 25MHz -> DIVM3 -> DIVN3 -- DIVP3 -> 144.5MHz
//          /25      x289  |  /2
//                         |
//                         |- DIVQ3 -> 41.285714MHz
//                         |  /7
//                         |
//                         | - DIVR3 -> 57.8MHz
//                             /5

let dp2 = pac::Peripherals::steal();
dp2.RCC.pllcfgr.modify(|_, w| w.divr3en().set_bit());
dp2.RCC.pll3fracr.write(|w| w.fracn3().bits(25));
dp2.RCC.pll3divr.write(|w| w.divp3().bits(2).divq3().bits(7).divr3().bits(5).divn3().bits(289));
dp2.RCC.cr.modify(|_, w| w.pll3on().on());
while dp2.RCC.cr.read().pll3rdy().is_not_ready() {}

Here's a link to what I've currently working with: https://github.com/olback/h7/tree/dsi-pr-working-2

If you want, I can make a simple getting-started project for the H7, there's some board setup needed to make things work. (PMIC, clock enable, magic write(?))

I just noticed that there seems to be at least two versions of the H7, one where the HSE is 25 MHz and one where it is 27MHz... The datasheet I downloaded a few years ago suggests that it's 27MHz, but almost all the code published by Arduino suggests 25MHz (older commits suggests 27...). The newest datasheet shows that both are connected on one sheet, but none on another. What a mess. 🙃

New:
20231204_215733

Old:
20231204_215844

Block diagram:
20231204_220111

This block diagram isn't quite right either, both the USB HS PHY and ethernet PHY are missing. The ethernet PHY is connected to the 25MHz clock according to another part of the schematic, and the USB PHY is connected to the 27MHz clock..

https://forum.arduino.cc/t/portenta-h7-schematics-wrong-mcu-xtal-is-25-mhz/1017205

This board drives me crazy

Others

  • AN4861

@richardeoin
Copy link
Member

Awesome - this is great work and I appreciate that you've taken the time and effort to share it!

There's a couple of loose ends that need to be tidied up before this can be merged. I would say:

  • Remove the git dependencies. The key one is the DSI related traits. I'd be happy to merge them into embedded-display-controller if that would be a good solution?
  • For the otm8009a and ft6236 crates, you could publish them on crates.io yourself (although there is already a ft6236 crate on there). Or it would also be technically possible to include the code in this PR in the utilities folder.
  • The extra files in the examples/utilities folder might be better in an examples/utilities_dsi folder or similar, so the selection of utilities available for the other examples is more minimal.

@romixlab
Copy link
Contributor Author

Thanks for looking into this! Sounds reasonable. Is it better to try to merge DSI related traits into embedded-hal crate or into embedded-display-controller?

Will publish otm8009a to crates. And ft6236 can be removed from examples, it's not related to display that much. Forked it because it uses newer I2C traits, while this HAL is using an older version, so had to downgrade.

Good point on utilities_dsi folder, it's a bit too much conditional logic with current setup.

@richardeoin
Copy link
Member

Is it better to try to merge DSI related traits into embedded-hal crate or into embedded-display-controller?

embedded-hal has a detailed review process because it's used by almost the whole embedded rust ecosystem. Afaict It's not really suitable for experimental traits. On the other hand embedded-display-controller is experimental, so the traits would be a good fit there.

@romixlab
Copy link
Contributor Author

That makes sense, let's do that. Created a pull request. When it's merged, I'll update the rest and publish otm8009 to crates.

@romixlab
Copy link
Contributor Author

Done, otm8009a published to crates, all git dependencies are gone. Checked that all 3 examples are still working on the real board.

@richardeoin
Copy link
Member

Awesome. It works for me too 🎉

H747 Disco board with display running display-dsi-video-stm32h747i-disco.rs

Cargo.toml Outdated Show resolved Hide resolved
@richardeoin
Copy link
Member

The Nightly rust examples CI is broken right now (fixed in 483) but the other 3 should be passing to merge this.

Quickly looking at the failures, it seems the new files in utilities aren't going to work with the basic examples where we want #![deny(unsafe_code)] enabled. I'd suggest moving the 4 new files to a new examples/utilities_dsi module that's only for the display- examples.

@romixlab
Copy link
Contributor Author

Moved to utilities_display as embedded-graphics example also uses the same BufferedDisplay and doesn't use dsi, hence the name. Though that's me who changed it to reduce code duplication, can roll it back as well.

@richardeoin
Copy link
Member

It's fine for the embedded-graphics example to use utilities_display

@@ -0,0 +1,77 @@
use cortex_m::peripheral::{MPU, SCB};

pub fn init_mpu(mpu: MPU, scb: &mut SCB, sdram_size: usize) {
Copy link
Member

@richardeoin richardeoin Jan 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pub fn init_mpu(mpu: MPU, scb: &mut SCB, sdram_size: usize) {
#[allow(unused, unsafe_code)]
pub fn init_mpu(mpu: MPU, scb: &mut SCB, sdram_size: usize) {

@richardeoin
Copy link
Member

There are a few more annoying minutiae to integrate this with the crate structure and the CI. I'll make a commit on top to take care of them

@richardeoin richardeoin merged commit 9bfd5b1 into stm32-rs:master Jan 27, 2024
19 of 22 checks passed
@romixlab
Copy link
Contributor Author

Awesome:) Thank you!

@romixlab romixlab deleted the dsi-display branch January 28, 2024 11:00
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

Successfully merging this pull request may close these issues.

3 participants