Derive the SPI divider base from the active GPSPI clock source - #260
Merged
Conversation
FreqToClockDiv could produce a CLKDIV_PRE value wider than the register field when the requested frequency was far below the base clock, spilling into adjacent fields. Clamp both divider components with the target register definitions so each SoC retains its supported range without corrupting neighboring bits.
Decode each supported ESP32 target's live GPSPI clock source and pre-divider instead of assuming an 80 MHz APB clock. Acquire the bus before reading that state because the driver may change it, allowing the existing cache to recalculate whenever another owner selects a different source. Unknown selectors use a safe upper bound so divider calculation does not accidentally overspeed the request.
On ESP32-C5, C61, C6, and P4 Arduino builds, temporarily select an 80 MHz GPSPI base only when it produces a strictly closer write clock without overspeeding either requested rate. Equal write results keep the current source; read throughput may decrease when write throughput improves. Save and restore only the owned clock fields inside the RCC atomic section, and clean up active ownership when a bus is released or destroyed. ESP-IDF transactions retain driver ownership, and the Arduino mutex does not serialize mixed API users.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
FreqToClockDiv()receives a base clock fromgetApbFrequency(), which reports a fixed 80 MHz on every target. That was true while the SPI peripheral was fed from APB, but the newer chips select a clock source and, on some of them, run it through pre-dividers first. Nothing configures that source unless the ESP-IDF driver acquires the bus, and Arduino builds take the Arduino bus mutex instead, so the source is left at whatever reset or the Arduino HAL happened to leave.The dividers were therefore computed against a base clock that is not the real one, and the resulting SCLK could be either far below or well above the requested value.
Measured on hardware
Requested vs. actual SCLK before these changes:
The overspeeding cases are the reason for the first two commits: a panel asked for 40 MHz and got twice that.
ESP32, ESP32-S2 and ESP32-C6 are unaffected and measured unchanged.
What the commits do
Clamp SPI clock dividers to their register widths —
FreqToClockDiv()could produce aCLKDIV_PREwider than its field, spilling into neighbouring bits; with a low enough request the spill reachedSPI_CLK_EQU_SYSCLKand drove the bus at full speed when a slow clock was asked for. The limits now come from the register definitions, so each target keeps its own field width.Use the active GPSPI source clock for divider calculations — the base clock is derived from the clock-source selector and pre-dividers that are actually programmed, instead of assuming 80 MHz. Bus acquisition moves ahead of the calculation because acquiring can change the source, and reading first would use a stale base for the first transaction. The crystal frequency is read at runtime rather than assumed, which matters on parts that ship with something other than 40 MHz.
Select faster GPSPI sources for Arduino transactions — where a better source exists, it is selected for the duration of a transaction and the saved fields are restored when it ends, so other users of the bus see the state they left. The takeover only runs while the bus lock is held, only when it produces a strictly closer write clock without overspeeding either requested rate, and is undone when the bus is released or destroyed.
Verification
One user-visible effect: an ESP32-C5 board whose panel is configured for 40 MHz was running at 24 MHz, and a full-screen fill went from 52.5 ms to 32.1 ms.