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

Add option to put LEDC into IRAM (IDFGH-10293) #11554

Closed
klew opened this issue May 31, 2023 · 2 comments
Closed

Add option to put LEDC into IRAM (IDFGH-10293) #11554

klew opened this issue May 31, 2023 · 2 comments
Assignees
Labels
Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally Type: Feature Request Feature request for IDF

Comments

@klew
Copy link
Contributor

klew commented May 31, 2023

Is your feature request related to a problem?

I'm working on LED matrix display in which I can only select LEDs from single row at a time, so I have to switch display between rows frequently - I use it GPTimer in ISR context.
On the other hand, display brightness is controlled by PWM conntected to one GPIO. I use LEDC driver to control it.

I had to put function that display a row at LED matrix in ISR context, so it won't glitch i.e. when I perform flash write operations.

However I had to disable PWM when I configure display for another row. Otherwise, I see dimmed LEDs in one row below current new line. This is related to HW design of this LED matrix, which updates column index of enabled LED and in last operation, it switches selected row.

Working solution for my problem is to put LEDC functions (at least ledc_stop and ledc_update_duty) in IRAM (otherwise it can crash, when cache is disabled i.e. during flash write).
I did it by manually editing linker.lf and by adding there "ledc (noflash)".

Currently there is no option to put LEDC into IRAM via configuration. I also didn't find any way how to force linker to do it without modification of esp-idf itself.

Describe the solution you'd like.

Add option to put LEDC into IRAM (at least ledc_stop and ledc_update_duty functions).

Describe alternatives you've considered.

No response

Additional context.

No response

@klew klew added the Type: Feature Request Feature request for IDF label May 31, 2023
@espressif-bot espressif-bot added the Status: Opened Issue is new label May 31, 2023
@github-actions github-actions bot changed the title Add option to put LEDC into IRAM Add option to put LEDC into IRAM (IDFGH-10293) May 31, 2023
@espressif-bot espressif-bot added Status: Selected for Development Issue is selected for development and removed Status: Opened Issue is new labels May 31, 2023
@igrr
Copy link
Member

igrr commented May 31, 2023

I did it by manually editing linker.lf and by adding there "ledc (noflash)".
I also didn't find any way how to force linker to do it without modification of esp-idf itself.

You can add a linker script rule not only in the component where the object file is located, but also from a component in your application. See, for example, how the GPIO driver is moved into IRAM in this example's main component:

[mapping:ext_driver]
archive: libdriver.a
entries:
# gpio_set_level, gpio_get_level, gpio_context, _gpio_hal, etc...
gpio (noflash)
[mapping:ext_soc]
archive: libhal.a
entries:
gpio_hal (noflash)

@klew
Copy link
Contributor Author

klew commented May 31, 2023

You can add a linker script rule not only in the component where the object file is located, but also from a component in your application. See, for example, how the GPIO driver is moved into IRAM in this example's main component:

Thanks for hint, it worked!
I was thinking about it earlier, but I found some forum post where it was stated that there was some error reported when duplicated archive name is found (it was related to esp-idf v3.x). Then I read https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-guides/linker-script-generation.html which says that "placement is specified at component level" and "In cases where multiple fragments of the same type and name are encountered, an exception is thrown", so I assumed that I can't override default placement in other place then "driver" component itself.

So basically, this solves my case, however I still think that having configuration option to place LEDC (or part of it) in IRAM may be beneficial for other users as well. Feel free to keep this issue open and add such option, or to close it with mentioned above solution.

@espressif-bot espressif-bot added Status: In Progress Work is in progress Status: Reviewing Issue is being reviewed Status: Done Issue is done internally Resolution: NA Issue resolution is unavailable and removed Status: Selected for Development Issue is selected for development Status: In Progress Work is in progress Status: Reviewing Issue is being reviewed labels Jun 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally Type: Feature Request Feature request for IDF
Projects
None yet
Development

No branches or pull requests

4 participants