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

implement a parallel interface for esp32 using the i2s peripheral #2348

Merged
merged 29 commits into from
Oct 30, 2024

Conversation

liebman
Copy link
Contributor

@liebman liebman commented Oct 13, 2024

Thank you for your contribution!

We appreciate the time and effort you've put into this pull request.
To help us review it efficiently, please ensure you've gone through the following checklist:

Submission Checklist 📝

  • I have updated existing examples or added new ones (if applicable).
  • I have used cargo xtask fmt-packages command to ensure that all changed code is formatted correctly.
  • My changes were added to the CHANGELOG.md in the proper section.
  • I have added necessary changes to user code to the Migration Guide.
  • My changes are in accordance to the esp-rs API guidelines

Extra:

Pull Request Details 📖

Description

The ESP32 supports a "LCD" interface on the i2s peripheral. This PR implements that for TX. Note that there are some limitations on the byte/word order that I don't believe can be corrected with peripheral configuration:

  • 8bit: [0, 1, 2, 3] is output as [2, 3, 0, 1]
  • 16bit: [0, 1, 2, 3] is output as [1, 0, 3, 2]

Testing

  • Logic analyzer
  • HUB75 Display

@liebman liebman force-pushed the i2s-i8080 branch 3 times, most recently from ed9b4bc to 498d12a Compare October 18, 2024 15:22
@liebman liebman marked this pull request as ready for review October 19, 2024 19:06
@liebman
Copy link
Contributor Author

liebman commented Oct 19, 2024

Not sure why the i2s HIL test is failing - I made no changes there.

@Dominaezzz
Copy link
Collaborator

Not sure why the i2s HIL test is failing - I made no changes there.

They've always been flakey.

examples/src/bin/embassy_i2s_parallel.rs Outdated Show resolved Hide resolved
esp-hal/src/i2s_parallel.rs Outdated Show resolved Hide resolved
esp-hal/src/i2s_parallel.rs Outdated Show resolved Hide resolved
esp-hal/src/i2s_parallel.rs Show resolved Hide resolved
esp-hal/src/i2s_parallel.rs Show resolved Hide resolved
@liebman liebman force-pushed the i2s-i8080 branch 3 times, most recently from 80d61f9 to 1c86951 Compare October 27, 2024 20:15
Copy link
Contributor

@bjoernQ bjoernQ left a comment

Choose a reason for hiding this comment

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

Thanks - I was only able to test this with a logic-analyzer but that looked good

@bugadani
Copy link
Contributor

I'm hoping we can get away with minimal changes for the S2 - the TRM seems to mention LCD there as well. Thank you!

@bugadani bugadani enabled auto-merge October 30, 2024 18:41
@bugadani bugadani added this pull request to the merge queue Oct 30, 2024
Merged via the queue into esp-rs:main with commit 448ed9a Oct 30, 2024
28 checks passed
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.

4 participants