Skip to content

drivers: display: convert st7735r to MIPI DBI API #70583

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
May 21, 2024
Merged

drivers: display: convert st7735r to MIPI DBI API #70583

merged 4 commits into from
May 21, 2024

Conversation

@danieldegrasse
Copy link
Contributor

@danieldegrasse danieldegrasse commented Mar 21, 2024 •
edited
Loading

This PR converts the ST7735R to the MIPI DBI API. Boards using this display have also been updated.

Additionally, this PR adds support for a new API in the MIPI DBI driver, mipi_dbi_release. This API mirrors the functionality of spi_release, and is added because some displays depend on the SPI_HOLD_ON_CS functionality in order to work correctly- therefore, the MIPI DBI API needs to offer a similar ability to hold the CS signal low.

In draft until #71454 merges

@zephyrbot zephyrbot added area: Display Controller area: Display area: Shields Shields (add-on boards) Release Notes To be mentioned in the release notes area: Devicetree Binding PR modifies or adds a Device Tree binding labels Mar 21, 2024
@danieldegrasse
Copy link
Contributor Author

@jfischer-no I know you mentioned you have most in tree SPI displays- do you have this one, and if so would you be willing to test this change? I don't have hardware to verify this with

@danieldegrasse danieldegrasse mentioned this pull request Mar 22, 2024
Closed
@mmahadevan108 mmahadevan108 self-requested a review April 3, 2024 20:25
mmahadevan108
mmahadevan108 reviewed Apr 3, 2024
View reviewed changes
mmahadevan108
mmahadevan108 previously approved these changes Apr 5, 2024
View reviewed changes
@dleach02
Copy link
Member

@danieldegrasse, please address the merge conflict

@mmahadevan108 mmahadevan108 mentioned this pull request Apr 11, 2024
Merged
@mmahadevan108
Copy link
Contributor

@danieldegrasse, can we move the first 3 commits to a separate PR as they are related to updating the MIPI-DBI API and can be looked at separately.

@zephyrbot zephyrbot requested a review from decsny April 12, 2024 20:53
@danieldegrasse
Copy link
Contributor Author

@danieldegrasse, please address the merge conflict

Done, thanks for the reminder

@danieldegrasse, can we move the first 3 commits to a separate PR as they are related to updating the MIPI-DBI API and can be looked at separately.

Sure- I've created a PR here: #71454. Marking this PR as draft, since it now depends on that one

@danieldegrasse danieldegrasse marked this pull request as draft April 12, 2024 21:01
kartben
kartben requested changes Apr 15, 2024
View reviewed changes
@danieldegrasse danieldegrasse marked this pull request as ready for review April 19, 2024 21:09
@danieldegrasse
Copy link
Contributor Author

@kartben Requested fixes for migration guide should be applied to this PR. The other documentation changes you mentioned were part of #71454 (which is merged), so I opened #71715 to apply them

kartben
kartben previously approved these changes Apr 25, 2024
View reviewed changes
Copy link
Contributor

@kartben kartben left a comment

Choose a reason for hiding this comment

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

LGTM - thanks

@danieldegrasse
Copy link
Contributor Author

@ozersa would you be able to look over (and potentially test) this change?

@ozersa
Copy link
Contributor

ozersa commented Apr 29, 2024

@ozersa would you be able to look over (and potentially test) this change?

Unfortunately I do not have change to test it, As per of my review

For 3wire mode, 9bit SPI data sends, that requires below line being 2 instead of 1.

zephyr/drivers/mipi_dbi/mipi_dbi_spi.c

Line 78 in a1cdf27

buffer.len = 1;

ozersa
ozersa reviewed Apr 29, 2024
View reviewed changes
drivers/display/display_st7735r.c Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

SPI_WORD_SET(8)
It will be better if you set correct SPI data size depend on the selected mode (8 for 4wire, 9 for 3wire).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this makes sense. Doing so will also allow dropping the tmp_cfg within the MIPI DBI driver.

dleach02
dleach02 previously approved these changes Apr 29, 2024
View reviewed changes
@danieldegrasse danieldegrasse added the DNM This PR should not be merged (Do Not Merge) label Apr 30, 2024
@danieldegrasse
Copy link
Contributor Author

Marking [DNM] until I address @ozersa's review

@danieldegrasse danieldegrasse removed the DNM This PR should not be merged (Do Not Merge) label Apr 30, 2024
@danieldegrasse danieldegrasse dismissed stale reviews from dleach02 and kartben via 1229b0b April 30, 2024 16:00
ozersa
ozersa approved these changes May 7, 2024
View reviewed changes
Copy link
Contributor

@ozersa ozersa left a comment

Choose a reason for hiding this comment

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

Looks good.

dleach02
dleach02 previously approved these changes May 13, 2024
View reviewed changes
kartben
kartben previously approved these changes May 13, 2024
View reviewed changes
@danieldegrasse
Copy link
Contributor Author

@erwango friendly ping:)

erwango
erwango previously approved these changes May 14, 2024
View reviewed changes
@henrikbrixandersen
Copy link
Member

@danieldegrasse Please rebase on main.

@danieldegrasse
drivers: mipi_dbi: mipi_dbi_spi: require 3 wire mode use 9 bit SPI
2cbf35e 
Require that SPI config within the MIPI DBI API use 9 bit SPI mode, as 3
wire spi requires the command/data bit be packed into the start of the
SPI packet.

Signed-off-by: Daniel DeGrasse <[email protected]>
@danieldegrasse
drivers: display: st7735r: convert to MIPI DBI API
91e4928 
Convert the ST7735R display to use the MIPI DBI API. Boards and overlays
using this display are also updated.

Signed-off-by: Daniel DeGrasse <[email protected]>
@danieldegrasse
tests: drivers: build_all: add st7735r to test
e52967a 
Add entry for st7735r to display build all test

Signed-off-by: Daniel DeGrasse <[email protected]>
@danieldegrasse
docs: releases: migration-guide-3.7: add note about ST7735R migration
89a5baa 
Add note about migration of ST7735R displays to MIPI DBI API.

Signed-off-by: Daniel DeGrasse <[email protected]>
@danieldegrasse
Copy link
Contributor Author

@danieldegrasse Please rebase on main.

Done, thanks

@danieldegrasse danieldegrasse dismissed stale reviews from erwango, kartben, and dleach02 via 89a5baa May 14, 2024 14:19
@danieldegrasse danieldegrasse requested a review from erwango May 20, 2024 14:15
@nashif nashif merged commit 1ed9ef4 into zephyrproject-rtos:main May 21, 2024
@CharlesDias CharlesDias mentioned this pull request May 23, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fabiobaltieri fabiobaltieri fabiobaltieri approved these changes

@erwango erwango erwango approved these changes

@dleach02 dleach02 dleach02 approved these changes

@ozersa ozersa ozersa approved these changes

@kartben kartben kartben left review comments

@mmahadevan108 mmahadevan108 mmahadevan108 left review comments

@aescolar aescolar Awaiting requested review from aescolar

@avisconti avisconti Awaiting requested review from avisconti

@galak galak Awaiting requested review from galak

@jfischer-no jfischer-no Awaiting requested review from jfischer-no

@nashif nashif Awaiting requested review from nashif

@decsny decsny Awaiting requested review from decsny

Assignees

@erwango erwango

Labels

area: Devicetree Binding PR modifies or adds a Device Tree binding area: Display Controller area: Display area: Shields Shields (add-on boards) Release Notes To be mentioned in the release notes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

10 participants

@danieldegrasse @dleach02 @mmahadevan108 @ozersa @henrikbrixandersen @kartben @fabiobaltieri @erwango @nashif @zephyrbot