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

Update dependency Embedded Graphics Core to 0.4.0 #58

Merged
merged 6 commits into from
May 24, 2023

Conversation

georgik
Copy link
Contributor

@georgik georgik commented May 18, 2023

There is a new release of Embedded Graphics 0.8.
It's necessary to update the dependency on the core to 0.4.0.

@almindor
Copy link
Owner

Very nice! @georgik did you also test with an actual device? Mine's currently unavailable so I want to make sure things didn't break.

@almindor
Copy link
Owner

Could you also please add a CHANGELOG entry?

@georgik
Copy link
Contributor Author

georgik commented May 19, 2023

@almindor Thank you for asking for further tests.
It seems that this commit 7ac38ac8ffe7da74502ab4cdc89e67ed6820551f after tagging 0.6 introduces a problem. The screen is always blank. Tested on several ESP targets.
I will investigate the issue further.

@georgik
Copy link
Contributor Author

georgik commented May 19, 2023

The strange thing was that simulation in Wokwi worked without problem.
After some investigation, there is an issue with the timing of commands which appears on real HW.
I've added a small delay as a workaround, and it solved the problem to init_common:
georgik@6e84871
This solved the problem for real HW. Do you have any opinions on this part?

@rfuest
Copy link
Collaborator

rfuest commented May 19, 2023

In section 15.4 of the datasheets of the ILI934x controllers the minimum time to wait between releasing of the reset signal and sending the first command is specified to be 5ms. I think we should raise the delay to at least 5ms to be sure that it works reliably.

But the same section also mentions that the Sleep Out command should be send at least 120 ms after the reset, which the current code also violates. Maybe we should shuffle the init around a bit, because we already have a 120 ms delay included in the init, but after all commands have been send.

@georgik
Copy link
Contributor Author

georgik commented May 22, 2023

Thank you @rfuest, for the details from the datasheet. These facts also explain why mipidsi version 0.6 was not working in some cases, often after booting the device for the first time.

@georgik
Copy link
Contributor Author

georgik commented May 22, 2023

@rfuest I've updated delay part. What's your opinion about the solution with delays.
I've run test on 11 ESP32 (including S2, S3, C3, C6) all seems to be working.

@rfuest
Copy link
Collaborator

rfuest commented May 22, 2023

I think this should work in most cases, but I've looked at the datasheet some more and I think the init sequence still needs some tweaks to work in all cases.

The "Sleep Out" command shouldn't be issued within 120ms of the reset, if the display was already in "Seep Out" mode before the reset. Section 13.2 Power Flow Chart shows the recommended delays.

The datasheet is not very clear, but I think this should be a sequence that works in all cases:

// 15.4:  It is necessary to wait 5msec after releasing RESX before sending commands.
// 8.2.2: It will be necessary to wait 5msec before sending new command following software reset.
delay.delay_us(5_000);

dcs.write_command(madctl)?;
dcs.write_raw(0xB4, &[0x0])?;
dcs.write_command(SetInvertMode(options.invert_colors))?;
dcs.write_command(SetPixelFormat::new(pixel_format))?;

dcs.write_command(EnterNormalMode)?;

// 8.2.12: It will be necessary to wait 120msec after sending Sleep In command (when in Sleep Out mode)
//          before Sleep Out command can be sent.
// The reset might have implicitly called the Sleep In command if the controller is reinitialized.
delay.delay_us(120_000);

dcs.write_command(ExitSleepMode)?;

// 8.2.12: It takes 120msec to become Sleep Out mode after SLPOUT command issued.
// 13.2 Power ON Sequence: Delay should be 60ms + 80ms
delay.delay_us(140_000);

dcs.write_command(SetDisplayOn)?;

@georgik
Copy link
Contributor Author

georgik commented May 23, 2023

Thank you @rfuest for the suggestion. I'll do the test with the updated code.

Meanwhile, I was speaking with ESP team who is responsible for LCD packages, and they pointed me to the following esp-bsp (Board Support Package) code: https://github.com/espressif/esp-bsp/tree/master/components/lcd

They will be adding new drivers in the future, so it might be a good source for checking implementation.

@georgik
Copy link
Contributor Author

georgik commented May 23, 2023

The change looks good. All boards are performing well, including M5 Stack.
I've run the test using the following project: https://github.com/georgik/esp32-spooky-maze-game

@almindor I've updated the CHANGELOG with topics that we've changed here

* ili934x with initialization delays

* doc: add infor about embedded-graphics-core 0.4.0 and initialization delays for ILI934x
@almindor
Copy link
Owner

Thanks everyone this was great work!

@almindor almindor merged commit 6f355ce into almindor:master May 24, 2023
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