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

Refine coprocessor API #7218

Closed
4 tasks
tannewt opened this issue Nov 16, 2022 · 4 comments · Fixed by #7359
Closed
4 tasks

Refine coprocessor API #7218

tannewt opened this issue Nov 16, 2022 · 4 comments · Fixed by #7359
Assignees
Milestone

Comments

@tannewt
Copy link
Member

tannewt commented Nov 16, 2022

I think we should build on #6902 and refine the APIs to be more generic. I've been thinking about coprocessor and memory APIs within a few contexts that this API can apply to:

  1. An FPGA world where peripherals are dynamic and a "raw" memory API is needed to write drivers for them.
  2. An FPGA world where you can instantiate as many CPU cores as you like. Probably RISC-V ones. Controlling them would likely be done through the debug spec.
  3. A Stemma QT board with a separate microcontroller on it. CircuitPython would manage the code on it and the interaction back to it.

So I think there are two separate pieces:

  1. The "raw" memory API.
  2. The coprocessor control API.

We should also think about a world with more than one coprocessor (like Cortex-A chips or multiple Stemma QT coprocessors.) I doubt we want run and halt at the module level. Perhaps we want to make microcontroller.cpu[1+] be Coprocessor instances instead of microcontroller.Processor. We probably want to revamp Processor too. uid, temperature, reset_reason are actually system level. Processor would be the core that runs CircuitPython and the Coprocessors would be CircuitPython controlled and tracked like other peripherals for "in use."

I don't think we want to manage multiple programs (or tasks) on a single cpu. Instead, we keep along the lines of doing one thing on a core at a time and track "in use". Later, if we want to do multiple tasks across a set of cores, then the Coprocessors could be added to a task manager-ish thing. (The USB host code on the RP2040 will need to claim the coprocessor for example.)

Here are the things I'd definitely do before 8.0.0:

  • Move coproc.CoprocMemory to memoryio.Memory. The shared-bindings constructor should delegate to a port-specific construct method so that a port can raise exceptions for protected memory regions and track "in use". The goal is to make it harder to have two things using memory at once including native classes and memoryio concurrently. (A kwarg could be given to override this check.) Making this generic would allow us to write peripheral drivers in pure python, not just C.
  • Rename coproc to coprocessor for clarity. Can always import coprocessor as coproc to shorten it.
  • Rename coproc.Coproc to Program (it should look similar to PIO's programs) or skip this and use a ReadableBuffer for the program like the low level PIO API. If we add Program, then it could be generically used for both on-chip coprocessors and those off-chip (like Stemma Qt.) We may want to include architecture and entry address info on it.
  • Add a coprocessor.Coprocessor class for run and halt. The CoprocessorAlarm would also take a Coprocessor along with the Program.
@microdev1
Copy link
Collaborator

microdev1 commented Nov 18, 2022

I like the general idea that this portrays and agree with the suggested api changes.
This is a vast topic covering multiple platforms and use cases, it would be interesting to see how this evolves.

@tannewt tannewt self-assigned this Nov 29, 2022
@tannewt
Copy link
Member Author

tannewt commented Dec 9, 2022

Ok, I've spent a good chunk of time on this in https://github.com/tannewt/circuitpython/tree/rework_coproc_api. I've tried getting the ULP working on ESP32-S3 and have only managed to get it working after reset. It stops working after reloads. Furthermore, I've traced some S3 sleep issues to having the coprocessor enabled (but not used by CP) due to the ulp falsely triggering on fault.

So, I'd like to simply remove the coproc module and disable the coproc in sdkconfig. My branch can be a starting point for anyone who can get deep sleep working ok and the ulp restarting correctly.

@tannewt
Copy link
Member Author

tannewt commented Dec 9, 2022

Power usage should also be checked since the IDF fix for the S3 forces the ULP clock on. It may invalidate power savings without the ULP.

@tannewt
Copy link
Member Author

tannewt commented Dec 10, 2022

Ok, with @microdev1's help (thanks!) I've made good progress.

My program was failing because I set the stack pointer to the wrong address. 🤦 I was also setting the wrong bit in the gpio registers (there's an extra 10 bit shift.)

I'll need to test deep sleep with it next week. (To make sure #7300 is fixed.)

dhalbert pushed a commit that referenced this issue Dec 21, 2022
It is now a generic `memorymap` API and an ESP specific `espulp` module.

Fixes #7218. Fixes #3234. Fixes #7300.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants