Spi implementation enhancements

[ianrrees]

Really just notes from a few hours spent with the SPI-driven WS2812 (neopixel) crate.

1. We currently implement embedded-hal 1.0's SpiBus only for Duplex SPIs, however it could be nice to provide implementations for Tx (and Rx?) SPIs as well, PyGamer provides two examples of peripherals that could use this feature: the WS2812 and the LCD. It's not always possible to provide a non-connected pin to the SPI ctor for an unused Rx or Tx, and a unidirectional implementation could be more efficient than a Duplex one. The impl of methods like SpiBus::read() for Tx SPI should probably panic with an appropriate message.
2. The implementation we've got now does not take advantage of the pipelines in the SERCOM, so in some cases it stalls between words and is considerably slower than it could be. For example, in the SpiBus::write() impl we have an implementation that doesn't send word N+1 OUT until N has shifted IN:

for word in words {
    let _ = self.transfer_word_in_place(*word)?;
}

but something a little more complicated can have considerably higher throughput, especially when the SPI clock is a higher fraction of the CPU clock (tinkering with a SAMD51, I'm seeing twice as fast in release mode with a 50MHz SPI):

let mut read_count = 0;
for word in words {
    self.block_on_flags(Flags::DRE)?;
    self.config.as_mut().regs.write_data(word.as_());

    if self.read_flags().contains(Flags::RXC) {
        read_count += 1;
        let _ = unsafe { self.read_data().as_() };
    }
}

while read_count < words.len() {
    self.block_on_flags(Flags::RXC)?;
    read_count += 1;
    let _ = unsafe { self.read_data().as_() };
}

3. For higher-bandwidth transfers, we could think about an implementation that uses DMA. This has an additional benefit that it should precisely control the timing between words, in a way that won't depend on the optimization settings of the compiler. That would help with peripherals like the WS2812, though I'm not sure it would be adequate to fix that particular implementation.

[ianrrees]

Thinking more about 1 above; since that behaviour might be a bit unexpected and wouldn't be caught at compile time, it might be better to wrap the unidirectional Spi<_, Tx> in a new PanicOnRead type which implements SpiBus. We could prototype that in a BSP, which might be good enough as it's rather niche.

[kyp44]

Hi @ianrrees I know it's not yet complete or released, but, for reasons, I am attempting to migrate from pygamer v0.9.0 to the latest in this repository (what will be v0.10.0) but my display completely stopped working after the switch (still initialized using pygamer::pins::Display::init). I am not getting a panic or anything, it's just that the embedded-graphics calls that were working perfectly before no longer have any visible affect on the display (it still just shows the bootloader screen or garbage, whatever was last in its buffer I it seems).

I noticed that the SPI types and objects have changed (they ostensibly seem like improvements, particularly the Pygamer-specific type aliases) so I am wondering if the display not working may be related to this issue.

I am interested in helping to solve the issue, but I'm not even really sure where to start in terms of what could be causing the display problem. I'm also still a bit new to embedded stuff and pretty much just know the basics of SPI so I'm not sure how useful I can be if this is a super low level issue.

Mainly just wanted to get your thoughts on this. I can create a MRE if needed.

[ianrrees]

Hi @kyp44 , sorry about my slow reply - I'm very busy with a new job away from home, and won't have SAMD hardware to work with until I'm back in mid-December.

It's hard to say what could be going wrong with your display - I ~recently made some updates to the PyGamer BSP and tested the examples that use the display, so I don't think the HAL is likely to be the cause of your problem so long as you're using one new enough to have #743 .

[kyp44]

@ianrrees Thanks for the response!

I've been working with @jbeaurivage on some things, and the display issue turned out to be something silly and not a real bug, so please disregard.