Feature: Accelerate BMPremote SPI data phase by removing inter-byte gaps #1946

ALTracer commented 2024年10月01日 18:58:06 +02:00 (Migrated from github.com)

Detailed description

• This is a perf fix to an existing feature.
• The existing problem is significant gaps between SPI bytes in both read and write xfers as driven by bmpflash + blackpill-f411ce (and likely others).
• This PR solves it by providing a continuous block xfer primitive (no IRQ and no DMA involved)

Tested to increase bmpflash read -b int dump times from 35 to 31 seconds for a 8192 KiB w25q64 chip (using 12 MHz). The atomic section is used to block interrupts for 170 microseconds (256 byte read), otherwise my patch made the board hang reliably (no read timeouts). I may likely rewrite this once more using direct register manipulation as opposed to libopencm3 spi API usage. Short reads, like SFDP, indicate normal gaps between command bytes (I didn't change them) but no gaps in data page phase.
The acceleration is achieved by keeping a byte (actually 8/16-bit SPI word) in flight behind the DR shadow register, which is how it is intended to be used. DMA bindings are harder and may result in channel/stream conflicts.

Your checklist for this pull request

• I've read the Code of Conduct
• I've read the guidelines for contributing to this repository
• It builds for hardware native (see Building the firmware)
• It builds as BMDA (see Building the BMDA)
• I've tested it to the best of my ability
• My commit messages provide a useful short description of what the commits do

Closing issues

## Detailed description * This is a perf fix to an existing feature. * The existing problem is significant gaps between SPI bytes in both read and write xfers as driven by bmpflash + blackpill-f411ce (and likely others). * This PR solves it by providing a continuous block xfer primitive (no IRQ and no DMA involved) Tested to increase `bmpflash read -b int` dump times from 35 to 31 seconds for a 8192 KiB w25q64 chip (using 12 MHz). The atomic section is used to block interrupts for 170 microseconds (256 byte read), otherwise my patch made the board hang reliably (no read timeouts). I may likely rewrite this once more using direct register manipulation as opposed to libopencm3 spi API usage. Short reads, like SFDP, indicate normal gaps between command bytes (I didn't change them) but no gaps in data page phase. The acceleration is achieved by keeping a byte (actually 8/16-bit SPI word) in flight behind the DR shadow register, which is how it is intended to be used. DMA bindings are harder and may result in channel/stream conflicts. ## Your checklist for this pull request * [x] I've read the [Code of Conduct](https://github.com/blackmagic-debug/blackmagic/blob/main/CODE_OF_CONDUCT.md) * [x] I've read the [guidelines for contributing](https://github.com/blackmagic-debug/blackmagic/blob/main/CONTRIBUTING.md) to this repository * [x] It builds for hardware native (see [Building the firmware](https://github.com/blackmagic-debug/blackmagic?tab=readme-ov-file#building-black-magic-debug-firmware)) * [x] It builds as BMDA (see [Building the BMDA](https://github.com/blackmagic-debug/blackmagic?tab=readme-ov-file#building-black-magic-debug-app)) * [x] I've tested it to the best of my ability * [x] My commit messages provide a useful short description of what the commits do ## Closing issues

ALTracer commented 2024年10月26日 18:20:33 +02:00 (Migrated from github.com)

Rebased to main.
A one-line change in PR1968 triggers a warning from reusing tx buffer as a rx buffer now.
I can split them into distinct TX and RX buffers.
The IRQ-masking DR-polling version can deal with one of them possibly being NULL. I'd like to rely on TX buffer being non-null for simplicity, and containing zero-bytes which can be submitted to SPI DR.
Another venue is actually leveraging a DMA stream on F4 (or channel on F1) where platforms allow, and where it does not conflict with aux USART and SWO USART.
What is the API here -- submitting two unidirectional transfers, or submitting one transfer but specifying the Tx header length (and the rest is Rx)? I'm assuming full-duplex is time-divided into Tx then Rx. Block reads (and block writes) to 25-series flash are already implemented as a byte-wise Tx command then block-wise Rx.

Rebased to main. A one-line change in PR1968 triggers a warning from reusing tx buffer as a rx buffer now. I can split them into distinct TX and RX buffers. The IRQ-masking DR-polling version can deal with one of them possibly being NULL. I'd like to rely on TX buffer being non-null for simplicity, and containing zero-bytes which can be submitted to SPI DR. Another venue is actually leveraging a DMA stream on F4 (or channel on F1) where platforms allow, and where it does not conflict with aux USART and SWO USART. What is the API here -- submitting two unidirectional transfers, or submitting one transfer but specifying the Tx header length (and the rest is Rx)? I'm assuming full-duplex is time-divided into Tx then Rx. Block reads (and block writes) to 25-series flash are already implemented as a byte-wise Tx command then block-wise Rx.

This pull request doesn't have enough approvals yet. 0 of 1 approvals granted.

You are not authorized to merge this pull request.