blackmagic-debug / blackmagic

In application debugger for ARM Cortex microcontrollers.
GNU General Public License v3.0
3.3k stars 775 forks source link

Feature: Alternate Manchester SWO implementation using DMA #1958

Closed ssimek closed 1 month ago

ssimek commented 1 month ago

Fast DMA-based TRACESWO Manchester decoding

Detailed description

The current interrupt-based TRACESWO decoding suffers (IMO) from several issues

This PR contains a full reimplementation of the TRACESWO link decoder using DMA that makes capture a lot more efficient:

  1. all edge times of the signal are captured using a timer
  2. DMA is used to record the timings into a circular buffer
  3. the buffer is periodically processed in batches, transformig the edge stream into a byte stream for sending in another circular buffer, resulting in effective processing time per sample on the order of several clock cycles
  4. the output buffer is processed in a lower-priority ISR as time permits

The result is the ability to process SWO signal up to ~3 MHz with the probe being more resilient against higher frequency signals (it just fails to decode it properly). Continuous streaming at these speeds obviously makes USB a bottleneck (1.5 Mbit = ~180 kB/s, with the USB controller seemingly being able to handle up to about 70 kB/s without double buffering). But for reasonably intermittent flows, one big benefit is the lightened load on the target.

Your checklist for this pull request

Closing issues

None that I'm aware of

dragonmux commented 1 month ago

Before we review this, please adjust all the nomenclature etc over to the current state of things - the pin is technically called TRACESWO in exactly one place in the CoreSight TRM, however it is referred to as SWO everywhere else including all the Architecture Reference Manuals so BMD takes the opinion that we should be consistent and use SWO only to refer to it to make sure nobody gets confused with parallel Trace.

Additionally, the file should be named for which encoding mode it implements - so, in this case.. swo_manchester_dma.c. This code likewise seems to take the old SWO init interface, please use and obey the current API given this also needs a way to be deinitialised and the pin returned back to use as TDO on most probes. The SWO_PIN and SWO_PORT macros exist for switching the pin over to the right mode for the decoder.

Also note that trace_buf_drain was entirely replaced with a routine that is interrupt safe, and that all the buffering for this was lifted out of the backends, simplifying them, the API, etc.

Once you have adjusted things for the current API, please run clang-format on your contribution as it contains many formatting errors.

Finally, we are retargeting this to main as this change will not be landing on the v1.10 firmware branch. That branch exists for bugfixes not features.

ssimek commented 1 month ago

Making this PR against 1.10 was fully intentional since main has diverged significantly and seems to be unfortunately quite unstable and under heavy development. If there's not going to be another 1.x release (the nature and cadency of changes in main has a very 2.0-ish vibe to it), I'm ok with maintaining this in my own fork for my own use for now. Once there is a stable release based on current main, I'll look into adjusting the code to the new standards.

dragonmux commented 1 month ago

main should be stable, and if it is not for your target(s), we need to know please - open bug reports, tell us, that's how we get to know that something broke. The issue with Flash size will be addressed in the next couple of weeks as the firmware build is going to be split into target profiles. It is at this point seeing minor features only and bugfixes pending a v2.0 release candidate.

The SWO engine was entirely overhauled as it was entirely broken on GD32F1-based BMPs, and the buffer management rewritten so it does something more useful for both SWO modes + so the pin could be properly returned back to use for JTAG after as this is a bug in the v1.x firmware series. The new code should be considered stable pending any show-stopping bugs that require any more rework.

It's not so much that there won't be another v1.x release, so much as that branch is specifically for bugfixes backported from main, so unless a major bug were found in v1.10 there is unlikely to be another v1.10 release and if there was it would only be for a bugfix, not a feature. The release branches are always specifically for this purpose only.

ssimek commented 1 month ago

I understand. I'll try to find the time to give 2.0 another chance - it may have been just bad luck, but the main I originally checked out about a week ago had issues with SWO, buffering, etc. (various artifacts appearing in the stream even in low speed scenarios) so I chose to base my work on the stable branch. In case it is stable enough for daily use, I'll port the changes and reopen this PR.

ssimek commented 1 month ago

Created #1960 for the 2.0 implementation