Separate tasks for frequency ramps and fan controller

[mutatrum]

Fixes #1210

This changes quite a lot. Two tasks are extracted from power management: a fan controller task and an asic management task for frequency and voltage changes. This stops frequency ramps - including the initial one at startup - to no longer block the power management task, including overheat checks.

The startup sequence is materially changed by this PR. The initial frequency ramp is done after the asic has been completely initialised and the fan controller is already running. This means that the full fan blast at startup is almost completely eliminated.

This PR also increases the frequency of fan controller to 10/sec and tuned the PID to be a lot less aggressive, and removing the startup sequence. These changes might be better done as a separate PR, depending on testing, but on my device the controller is oscillating a lot less.

🚨 This works on my Gamma, but needs a lot more testing, especially from a cold start. Proceed at your own risk! 🚨

Future improvements for this PR are to simplify the ASIC management task by adjusting a single frequency step on each tick, instead of delegating it to do_frequency_ramp.

It also needs more code cleanups, I'm not satisfied with the header file structure, especially the PowerManagementModule struct and how this is included throughout the application via global_state.h.

One open problem currently (also without this PR) is that changing the frequency through the BAP port will block the BAP handler as it's doing a blocking call to do_frequency_transition. The BAP handler should just set a new desired frequency, and the asic management calls the set_frequency function.

[github-actions]

Test Results

20 tests  ±0   20 ✅ ±0   0s ⏱️ ±0s
 1 suites ±0    0 💤 ±0 
 1 files   ±0    0 ❌ ±0 

Results for commit dc0d798. ± Comparison against base commit fa0409e.

♻️ This comment has been updated with latest results.

[mutatrum]

Stack size of a lot of tasks had to be reduced, we're running out of heap with that many tasks. Not sure how to make that bigger. It also needs to have some general error handling, the xTaskCreate fails silently if we run into this again.

[KillerInk]

that silent crash can get catched with a handler for every task, and then check after task create !handler

[mutatrum]

Good that you mentioned, I wanted to add this. I think it's a good thing do add anyways, as the results are very puzzling, depending on which tasks were actually started and which weren't.

I'm also looking at increasing the FreeRTOS heap space, as this is quite constrained with the number of tasks we have.

[mutatrum]

This is interesting reading material: espressif/esp-idf#11216 (comment)

TLDR: xTaskCreate always uses internal memory. This is by design. When a task does a NVS or SPI Flash write, it disables cache which makes the task switcher unable to execute anything else, rebooting the system. So, for those tasks, the stack can't be in SPI ram. All tasks that write to NVS or Flash need to be internal.

[KillerInk]

well that nvs stuff get used way to much. for that settings loading it would be enough to read it in main.c setup once and fill a struct with it or whatever. saving happen there only in the http_server.c except the highest diff. but that could get deligated to queue and saving run in a own singel run task

[WantClue]

There are some conflicts now

[mutatrum]

There are some conflicts now

Conflicts resolved, but I'm still testing. Sometimes it goes straight to overheat mode when booting cold. Not sure what's happening there.

And I also want to simplify the frequency controller, by adjusting a single step on each iteration instead of doing a loop within an eternal task.

[skot]

Sometimes it goes straight to overheat mode when booting cold. Not sure what's happening there.

FWIW the ASIC thermal diode will not read correctly until ASIC core voltage is on.

[mutatrum]

FWIW the ASIC thermal diode will not read correctly until ASIC core voltage is on.

Responsibilities are indeed not properly separated between the tasks, so there's some things happening out of order. I've set the PR to draft for now. Same with overheat mode, that's being overridden because of bad separation in the current code.

[KillerInk]

about heap

In general, external RAM will not be used as task stack memory. xTaskCreate() and similar functions will always allocate internal memory for stack and task TCBs.

The option CONFIG_FREERTOS_TASK_CREATE_ALLOW_EXT_MEM can be used to allow placing task stacks into external memory. In these cases xTaskCreateStatic() must be used to specify a task stack buffer allocated from external memory, otherwise task stacks will still be allocated from internal memory.

https://docs.espressif.com/projects/esp-idf/en/stable/esp32/api-guides/external-ram.html#external-ram-config-memory-map

[mutatrum]

There's also xTaskCreateWithCaps, but as stated above, you can't put the stack heap in PSRAM if that task writes to NVS, which unfortunately, a lot of tasks do.

Currently, NVS is used also as a message bus between tasks. It might be better to use GlobalState for that, and have a single NVS writer task. That way all the other tasks can leverage PSRAM. Only other exception would be the OTA flash task, as that one also has to be in internal memory.

[WantClue]

concept ack

[mutatrum]

As this adds more tasks and we're running out of internal memory, it's better to wait for #1245 and #1253. Furthermore, this needs another pass as the division between the tasks is not good yet.