drivers: display: Add 3wire mode support in st7735 controller

[ozersa]

st7735 display controller can driver display both over 3wire or 4wire mode.
Existing st7725r.c display controller only supports 4wire mode. This commit add 3wire mode support in the controller.

Both 3wire and 4wire mode tested with MAX32672EVKIT which has CFAF128128B1-0145T display on board.

• 3-wire, 9-bit SPI mode. The D/C pin is not used. The 9th SPI bit selects the data or command register.
• 4-wire, 8-bit SPI mode. The D/C pin is used to select the data or command register.

[github-actions]

Hello @ozersa, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

[MaureenHelm]

Some additional tab added in some lines to make next commit more clear.

Don't do this. Each commit should be able to pass code style checks

[ozersa]

Thanks Maureen for feedback.
Sure.

[MaureenHelm]

This property should be moved to dts/bindings/spi/spi-device.yaml and renamed to spi-3wire to be consistent with the Linux kernel.

[ozersa]

Today I see st7789v driver, in this driver this option implicitly handled by gpio command.
And MIPI DIB API also does not need additional parameter to select 3/4 wire mode. Please see: mipi_dbi/Kconfig.spi

So it might be better if we totally remove this additional parameter.

Anyway if you think it will be better please let me know.

[danieldegrasse]

This can be implemented using the MIPI DBI API, which already implements support for SPI 3 wire mode: https://docs.zephyrproject.org/latest/hardware/peripherals/mipi_dbi.html.

Instead of implementing 3 wire support in the driver, can you instead update this driver to use the MIPI DBI API? That should enable 3 wire support to be added relatively easily

Just a note- although the code the 3 wire support is present in the MIPI DBI SPI mode driver, I don't have a 3 wire mode display to test support with. If you run into issues with the implementation please let me know, I'd be happy to help get this moved over

[ozersa]

This can be implemented using the MIPI DBI API, which already implements support for SPI 3 wire mode: https://docs.zephyrproject.org/latest/hardware/peripherals/mipi_dbi.html.

Instead of implementing 3 wire support in the driver, can you instead update this driver to use the MIPI DBI API? That should enable 3 wire support to be added relatively easily

Just a note- although the code the 3 wire support is present in the MIPI DBI SPI mode driver, I don't have a 3 wire mode display to test support with. If you run into issues with the implementation please let me know, I'd be happy to help get this moved over

Thanks for the feedback, I just get a chance to revisit this
It seems MIPI DBI driver requires some updates, I have update it to work with MIPI API but st7735r.c driver includes some spi_release_dt(...) call which not easily be replaced with MIPI driver update. I have not found a appropriate way for them yet.

Please see my updated here: 2e754db

Also today I see st7789v driver which handle 3/4 wire mode in the driver, please see: https://github.com/zephyrproject-rtos/zephyr/blob/main/drivers/display/display_st7789v.c#L71

Does it make sense we follow st7789v.c format and then in future convert them to MIPI DBI API?

[ozersa]

@danieldegrasse @MaureenHelm does it make sense if we follow st7789v.c format and in future update both st7735 and st7789v to use mipi dib driver?

[danieldegrasse]

Thanks for the feedback, I just get a chance to revisit this
It seems MIPI DBI driver requires some updates, I have update it to work with MIPI API but st7735r.c driver includes some spi_release_dt(...) call which not easily be replaced with MIPI driver update. I have not found a appropriate way for them yet.

I'll be honest- this is exactly why I want to convert drivers like this. I unfortunately don't have a 3 wire display to test with, but we need to prove out support for the MIPI DBI 3 wire mode. If we need to make changes to the MIPI DBI driver implementation to get things working with 3 wire mode, PRs like this will catch that. If I converted this driver to use the MIPI DBI API, would you be willing to help test 3 wire support with your display?

[ozersa]

Thanks for the feedback, I just get a chance to revisit this
It seems MIPI DBI driver requires some updates, I have update it to work with MIPI API but st7735r.c driver includes some spi_release_dt(...) call which not easily be replaced with MIPI driver update. I have not found a appropriate way for them yet.

I'll be honest- this is exactly why I want to convert drivers like this. I unfortunately don't have a 3 wire display to test with, but we need to prove out support for the MIPI DBI 3 wire mode. If we need to make changes to the MIPI DBI driver implementation to get things working with 3 wire mode, PRs like this will catch that. If I converted this driver to use the MIPI DBI API, would you be willing to help test 3 wire support with your display?

As I mentioned, I have already converted driver to mipi dbi after your first comment, it works with 8bit mode (4wire). but not work for 3wrire mode, seems stop clocking after some bytes, I do not know how should handle spi_release_dt(...) function which exist in old version of st7735r.c driver. I believe this is the reason of the failure for 3wire mode. I am going to check SPI line with logic analyzer.
Now this PR includes MIPI DIB conversion, please take a look, and if you are not happy with the conversion to MIPI DBI driver i am fine, please feel free to convert it, incase of need I can apply test on my board.

[danieldegrasse]

As I mentioned, I have already converted driver to mipi dbi after your first comment, it works with 8bit mode (4wire). but not work for 3wrire mode, seems stop clocking after some bytes, I do not know how should handle spi_release_dt(...) function which exist in old version of st7735r.c driver. I believe this is the reason of the failure for 3wire mode. I am going to check SPI line with logic analyzer.
Now this PR includes MIPI DIB conversion, please take a look, and if you are not happy with the conversion to MIPI DBI driver i am fine, please feel free to convert it, incase of need I can apply test on my board.

Thank you- I really appreciate you looking at this. I took a look at converting this display as well: #70583. I've added the ability to specify the mode in devicetree with that PR- perhaps you could try using 3 wire mode with that change? I added some code to handle spi_release_dt, but I unfortunately can't test it as I don't have this display.

[ozersa]

Thanks for the update, today I tried to test it, I cherry picked your commits from nxp fork, but encounter confliction between multiple zephyr forks,
We have an internal fork (it is behind the upstream), you are working on nxp fork, So that it will be better for me check it after we fetch upstream in next weeks.

[ozersa]

Superseded by #70583