Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

drivers/lcd: add MCU-driven low-level parallel interface #19941

Merged
merged 5 commits into from
Oct 5, 2023

Conversation

gschorcht
Copy link
Contributor

@gschorcht gschorcht commented Sep 22, 2023

Contribution description

The PR extends the LCD driver by a low-level interface for MCU-driven implementations of the MCU 8080 16-/8-bit parallel interface, allowing the MCU to use special peripherals for the interface, such as the FMC for STM32 MCUs, which is significantly faster than the integrated GPIO-driven parallel interface implementation of the LCD driver.

Testing procedure

Once PR #19938 and PR #19939 are merged, a PRs for these board can be pushed that allow to test this PR.

Use either PR #19943 or PR #19944 on top of this PR to test, e.g. with PR #19943:

BOARD=stm32f723e-disco make -j8 -C tests/drivers/st77xx flash

Issues/PRs references

@github-actions github-actions bot added Area: tests Area: tests and testing framework Area: drivers Area: Device drivers Area: Kconfig Area: Kconfig integration labels Sep 22, 2023
@gschorcht gschorcht added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Sep 22, 2023
@riot-ci
Copy link

riot-ci commented Sep 22, 2023

Murdock results

✔️ PASSED

e0a76c5 drivers/lcd: add MCU-driven low-level parallel interface

Success Failures Total Runtime
7937 0 7937 14m:58s

Artifacts

@gschorcht gschorcht added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Sep 22, 2023
@gschorcht gschorcht force-pushed the drivers/lcd_ll_mcu branch 2 times, most recently from 8e41e49 to abfbfa4 Compare September 22, 2023 12:53
Comment on lines 384 to 389
if (lcd_ll_par_driver.init) {
lcd_ll_par_driver.init(dev);
}
else {
lcd_ll_par_gpio_init(dev);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we have a lcd_parallel_gpio or lcd_parallel_periph pseudo-module so we can have this check compile-time const if only one of the two is used - and don't have to include all the bit-banging code when a periph implementation is used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense.

Copy link
Contributor

Choose a reason for hiding this comment

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

actually, why have the switch at all instead of

lcd_ll_par_driver.init = lcd_ll_par_gpio_init;

for the bit-banging case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is not necessary that the MCU implements all low-level functions. For example, the ESP32 implementation doesn't define the init and cmd_start driver functions. If the MCU-driven low-level implementation doesn't define them, the corresponding lcd_ll_par_gpio_* functions are used instead.

Alternatively, we could set those members of the driver structure to the lcd_ll_par_gpio_* functions in the MCU-driven low-level implementation.

I'm still not really happy having only one low-level driver instance. This avoids to use multiple displays with parallel interface but different low-level implementations. Ideally, each display device should point to it's own low-level implementation. But the configuration via the constant LCD display parameter set seems to be too complicated. Doing it in RAM during the initialization of the device would cost a lot of RAM.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On the other hand, the question arises whether there will ever be a board with two or more displays and whether multiple displays with different low-level implementations would then be used. Parallel interfaces occupy soooooo many GPIOs that we can probably assume that if we used the parallel interface, all displays would use the same interface but with different select signals. If this assumption is true, only one low-level driver instance is sufficient.

@gschorcht gschorcht force-pushed the drivers/lcd_ll_mcu branch 2 times, most recently from c1c98a9 to 0851952 Compare September 23, 2023 05:48
@MrKevinWeiss
Copy link
Contributor

Anything blocking this?

@gschorcht
Copy link
Contributor Author

Anything blocking this?

Not from point view.

Copy link
Contributor

@benpicco benpicco 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 from my side too, sorry for the delay

@gschorcht gschorcht added the CI: ready for merge train 🚃 PR is ready to be merged and awaiting the next merge train label Oct 4, 2023
@benpicco
Copy link
Contributor

benpicco commented Oct 5, 2023

bors merge

@bors
Copy link
Contributor

bors bot commented Oct 5, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit dd62f41 into RIOT-OS:master Oct 5, 2023
@gschorcht
Copy link
Contributor Author

Thanks

bors bot added a commit that referenced this pull request Oct 6, 2023
19943: cpu/stm32: FMC used for low-level LCD parallel interface r=aabadie a=gschorcht

### Contribution description

This PR provides the implementation of the LCD low-level MCU 8080 parallel interface using the FMC peripheral.

### Testing procedure

```
BOARD=stm32f723e-disco make -C tests/drivers/st77xx flash
```
and
```
BOARD=stm32l496g-disco make -C tests/drivers/st77xx flash
```
should work on top of PR #19941. Drawing operations should be much faster.

### Issues/PRs references

Depends on PR #19941


Co-authored-by: Gunar Schorcht <gunar@schorcht.net>
bors bot added a commit that referenced this pull request Oct 13, 2023
19943: cpu/stm32: FMC used for low-level LCD parallel interface r=aabadie a=gschorcht

### Contribution description

This PR provides the implementation of the LCD low-level MCU 8080 parallel interface using the FMC peripheral.

### Testing procedure

```
BOARD=stm32f723e-disco make -C tests/drivers/st77xx flash
```
and
```
BOARD=stm32l496g-disco make -C tests/drivers/st77xx flash
```
should work on top of PR #19941. Drawing operations should be much faster.

### Issues/PRs references

Depends on PR #19941


Co-authored-by: Gunar Schorcht <gunar@schorcht.net>
bors bot added a commit that referenced this pull request Oct 16, 2023
19943: cpu/stm32: FMC used for low-level LCD parallel interface r=MrKevinWeiss a=gschorcht

### Contribution description

This PR provides the implementation of the LCD low-level MCU 8080 parallel interface using the FMC peripheral.

### Testing procedure

```
BOARD=stm32f723e-disco make -C tests/drivers/st77xx flash
```
and
```
BOARD=stm32l496g-disco make -C tests/drivers/st77xx flash
```
should work on top of PR #19941. Drawing operations should be much faster.

### Issues/PRs references

Depends on PR #19941


Co-authored-by: Gunar Schorcht <gunar@schorcht.net>
bors bot added a commit that referenced this pull request Oct 16, 2023
19943: cpu/stm32: FMC used for low-level LCD parallel interface r=maribu a=gschorcht

### Contribution description

This PR provides the implementation of the LCD low-level MCU 8080 parallel interface using the FMC peripheral.

### Testing procedure

```
BOARD=stm32f723e-disco make -C tests/drivers/st77xx flash
```
and
```
BOARD=stm32l496g-disco make -C tests/drivers/st77xx flash
```
should work on top of PR #19941. Drawing operations should be much faster.

### Issues/PRs references

Depends on PR #19941


19978: treewide: fix typos to make codespell happy r=maribu a=maribu

### Contribution description

- fixes typos in comments and docs (no generated firmware changes expected)
- fixes a typo in a string in a GUI of a utility program
- add some false positives to the ignore list

### Testing procedure

- No generated binaries (except for the GUI version of the utility program to flash the MSB-A2) should change
- The diff should not look too scary

### Issues/PRs references

None

Co-authored-by: Gunar Schorcht <gunar@schorcht.net>
Co-authored-by: Marian Buschsieweke <marian.buschsieweke@posteo.net>
bors bot added a commit that referenced this pull request Nov 4, 2023
19546: Enable compile_and_test_for_board to skip if nothing changed r=benpicco a=MrKevinWeiss

### Contribution description

The overall goal of this PR is to be able to keep running the `compile_and_test_for_board.py` script without destroying ones boards.  Useful if you are a board owner that wants to keep testing on master (say with nightlies, say in a CI).

For example, I could now just run and rerun the following:
```
./dist/tools/compile_and_test_for_board/compile_and_test_for_board.py . <MY_BOARD> -c
```
and monitor the results, even after updating a branch.

This is because the hashes now get stored with the results (thanks to the `RIOT_TEST_HASH_DIR` var) which means cleaning will not wipe them out.  Specifically in (`<results_base>/<board>/hashes/<app_dir>/test-input-hash.sha1`)

I tried to do as much as I could with make and only alter the python script when needed.

### Testing procedure

Clear results folder and run:
```
./dist/tools/compile_and_test_for_board/compile_and_test_for_board.py . native --applications tests/shell -c
```
<details>

```
INFO:native:Saving toolchain
INFO:native.tests/shell:Board supported: True
INFO:native.tests/shell:Board has enough memory: True
INFO:native.tests/shell:Application has test: True
INFO:native.tests/shell:Run compilation
**INFO:native.tests/shell:Hashes match: False**
INFO:native.tests/shell:Run test
INFO:native.tests/shell:Run test.flash
INFO:native.tests/shell:Success
INFO:native:Tests successful
```
</details>

it should execute the test...

Then the same command again should skip it.
<details>

```
INFO:native:Saving toolchain
INFO:native.tests/shell:Board supported: True
INFO:native.tests/shell:Board has enough memory: True
INFO:native.tests/shell:Application has test: True
INFO:native.tests/shell:Run compilation
**INFO:native.tests/shell:Hashes match: True**
INFO:native.tests/shell:Success
INFO:native:Tests successful
```
</details>

Try changing something (say the `tests/shell/01-run.py`) and rerun, it should trigger the test again.
<details>

```
INFO:native:Saving toolchain
INFO:native.tests/shell:Board supported: True
INFO:native.tests/shell:Board has enough memory: True
INFO:native.tests/shell:Application has test: True
INFO:native.tests/shell:Run compilation
**INFO:native.tests/shell:Hashes match: False**
INFO:native.tests/shell:Run test
INFO:native.tests/shell:Run test.flash
INFO:native.tests/shell:Success
INFO:native:Tests successful
```
</details>

...finally, running without the changes check should run the test as usual, even if nothing changes:
```
./dist/tools/compile_and_test_for_board/compile_and_test_for_board.py . native --applications tests/shell
```
<details>

```
INFO:native:Saving toolchain
INFO:native.tests/shell:Board supported: True
INFO:native.tests/shell:Board has enough memory: True
INFO:native.tests/shell:Application has test: True
INFO:native.tests/shell:Run compilation
INFO:native.tests/shell:Run test
INFO:native.tests/shell:Run test.flash
INFO:native.tests/shell:Success
INFO:native:Tests successful
```
</details>

### Issues/PRs references

Might help the surprises we got with #19469


19944: cpu/esp32: add low-level LCD parallel interface using LCD peripheral r=benpicco a=gschorcht

### Contribution description

This PR the implementation of the LCD low-level MCU 8080 parallel interface using the LCD peripheral for ESP32, ESP32-S2 and ESP32-S3 or `periph_gpio_ll` for the ESP32-C3.

### Testing procedure

```
BOARD=esp32s3-wt32-sc01-plus make -C tests/drivers/st77xx flash
```
should work on top of PR #19941. Drawing operations should be much faster.

### Issues/PRs references

Co-authored-by: MrKevinWeiss <weiss.kevin604@gmail.com>
Co-authored-by: Gunar Schorcht <gunar@schorcht.net>
bors bot added a commit that referenced this pull request Nov 4, 2023
19944: cpu/esp32: add low-level LCD parallel interface using LCD peripheral r=benpicco a=gschorcht

### Contribution description

This PR the implementation of the LCD low-level MCU 8080 parallel interface using the LCD peripheral for ESP32, ESP32-S2 and ESP32-S3 or `periph_gpio_ll` for the ESP32-C3.

### Testing procedure

```
BOARD=esp32s3-wt32-sc01-plus make -C tests/drivers/st77xx flash
```
should work on top of PR #19941. Drawing operations should be much faster.

### Issues/PRs references

Co-authored-by: Gunar Schorcht <gunar@schorcht.net>
@MrKevinWeiss MrKevinWeiss added this to the Release 2023.10 milestone Nov 17, 2023
@gschorcht gschorcht deleted the drivers/lcd_ll_mcu branch November 22, 2024 06:46
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Area: drivers Area: Device drivers Area: Kconfig Area: Kconfig integration Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: ready for merge train 🚃 PR is ready to be merged and awaiting the next merge train Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants