ILI9341: add SPI driver for STM32F4

[ardnew]

This PR adds an SPI-based ILI9341 driver for STM32F4 targets such as TinyGo boards stm32f4disco and feather-stm32f405.

Also added is the pyportal_boing example configured for the feather-stm32f405.

Note the feather-stm32f405 board is still a WIP in the dev branch of tinygo. Adding this PR as a draft until that board and its SPI implementation are merged to make sure I'm submitting this PR correctly

[aykevl]

What does this code do? Is it using a standard SPI connection or is it doing something special?

[ardnew]

What does this code do? Is it using a standard SPI connection or is it doing something special?

Which code are you referring to? If you're asking about all of it, then yes it's just using a standard SPI connection, along with the STM32F4-specific peripheral interface registers.

Similar drivers exist for SPI on SAMD21 and SAMD51, which this was largely inspired from. See:

https://github.com/tinygo-org/drivers/blob/dev/ili9341/spi_atsamd21.go

https://github.com/tinygo-org/drivers/blob/dev/ili9341/spi_atsamd51.go

[sago35]

@ardnew
I would like you to make a PR for the SPI branch of stm32f405.
I have a feather-stm32f405 and SPI-ILI9341 and I'm going to review this PR.

[ardnew]

@sago35 just added the PR here: tinygo-org/tinygo#1377

[sago35]

The processing for SPI_CR1_SPE does not seem to be necessary, and removing it will increase the speed from 68 fps to 88 fps.
Other functions are the same.

Any reason to use SPI_CR1_SPE ?

Suggested change

	func (pd *spiDriver) write8(b byte) {
	if !pd.bus.Bus.CR1.HasBits(stm32.SPI_CR1_SPE) {
	pd.bus.Bus.CR1.SetBits(stm32.SPI_CR1_SPE)
	}
	pd.setWord(b, true, true)
	pd.bus.Bus.CR1.ClearBits(stm32.SPI_CR1_SPE)
	}
	func (pd *spiDriver) write8(b byte) {
	pd.setWord(b, true, true)
	}

[ardnew]

I wasn't sure about that either, perhaps it clears certain flags (e.g., overrun/underrun?) in the SPI peripheral once the transaction has completed? You wouldn't want them to persist across transactions.

But for good connections with a well-behaving slave, you can probably get by without doing it.

Note that the motivation for this disabling and reenabling the peripheral comes from STM32CubeIDE-generated driver code, which also performs this set/clear on CR1 SPE in its Transmit/Receive functions - both their HAL driver and their LL driver do this.

For reference, here is my STM32CubeIDE project configured for the STM32F405 Feather. It has UART, I2C, and SPI interfaces all configured on their correct pins, and the system clock is configured with the 12 MHz HSE crystal onboard the feather.

@sago35 https://github.com/ardnew/ardnew/tree/master/share/tinygo/sago35

There are two different copies of the exact same project, one using the generated HAL drivers and one using the generated LL (low-level) drivers. You will find the relevant driver code for both in a subdirectory. SPI, for example:

HAL: feather-stm32f405.HAL/feather-stm32f405/Drivers/STM32F4xx_HAL_Driver/stm32f4xx_hal_spi.c
LL: feather-stm32f405.LL/feather-stm32f405/Drivers/STM32F4xx_HAL_Driver/stm32f4xx_ll_spi.c

[ardnew]

I suggest removing this logic from the ILI9341 driver as you're implying.

This logic would be better suited already exists in (SPI).Transfer where robustness and bus stability is more important than performance.

Do you agree? Or would you remove it from both locations?

[sago35]

In my opinion, it should be removed from ILI9341.
And as you say, I think we should leave it in SPI.Transfer().

https://github.com/tinygo-org/tinygo/blob/7fc5095b6b768a6e441752223724b27adaf2f014/src/machine/machine_stm32_spi.go#L150

[sago35]

Please add the following examples as well.
I've confirmed that it works in my environment.

• examples/ili9341/basic
• examples/ili9341/scroll

[sago35]

I hadn't noticed.
It is better to have the following changes made.

// examples/pyportal_boing/main.go - main()
	if backlight != machine.NoPin {
		// configure backlight
		backlight.Configure(machine.PinConfig{machine.PinOutput})
		backlight.High()
	}

I think the target source code is as follows.

• examples/ili9341/basic/main.go
• examples/ili9341/pyportal_boing/main.go
• examples/ili9341/scroll/main.go

[sago35]

Any reasons for the addition?

[ardnew]

Unfortunately I don't have any documentation to justify this, only a use case:

Before the boing demo enters its main loop (drawing the ball frames), it first draws the entire 1-bit background across the screen.

With my Feather, the top ~0.5cm of this background (including the top of the grid) was not being drawn, and would remain either white or have a scrambled gray coloring -- it seemed like the first SPI transactions were starting too quickly and the screen wasn't able to draw the data in time. The rest of the graphics continued normally without any problem.

After adding this short delay, I never saw the problem again.

I'll try to recreate and take a screenshot.

[aykevl]

What does this code do? Is it using a standard SPI connection or is it doing something special?

Which code are you referring to? If you're asking about all of it, then yes it's just using a standard SPI connection, along with the STM32F4-specific peripheral interface registers.

The thing is, if it can work with standard SPI (no STM32 specific registers) then that would be much better as it would support any chip with SPI support. That said, it would be great to add the necessary interfaces directly to the machine package so that chips don't have to be coupled to the drivers.

[ardnew]

Oh I see. I thought this chip coupling was convention, since there exists SAMD21 and SAMD51 specific drivers in this package already. Your comment applies to those as well then.

That being said, I highly doubt we can achieve the SPI efficiency required for the pyportal_boing demo using the (SPI).Transfer method alone. These chip-specific drivers take advantage of the ILI9341's API, eliminating overhead performed with usual SPI transfers (e.g., not waiting for TXE/BSY, or toggling SPE, for every byte clocked out). @sago35 may be able to confirm or refute this from experience.

[ardnew]

it would be great to add the necessary interfaces directly to the machine package so that chips don't have to be coupled to the drivers.

Indeed, I can't find a single other driver that includes chip-specific code. I would have to defer to others on how to expose this functionality in the machine package.

[Amblyopius]

I think I can chip in a bit as I've been playing around with the ILI9341 drivers and have a local version that also reads (ATSAMD51 only currently). For ATSAMD51 (and ATSAMD21) SPI there's separate routines in order to have a send only option rather than using default SPI transfer which always reads too. I would assume this was mainly done for speed (though I have not tested yet if it really makes a difference). Essentially it puts the ATSAMD51/21 SPI in write only mode by disabling the receiver (using CTRLB.RXEN). It is true that having the receiver on is pointless in most cases as ILI9341 is not returning anything useful.

Also abstracting it in write8, write8n ... for ILI9341 driver was likely done because ILI9341 isn't always connected by SPI so an intermediate layer between the actual driver routines and the hardware layer might be needed. That said it should be trivial to have a generic SPI driver that works board independent and is simply based on generic SPI transfer function (which is exactly what I did to have reads). Having board specific SPI routines should provide some sort of benefit to be justified. If the other connection options provide an identical transfer function you could probably do away with a lot of code.

[sago35]

Oh I see. I thought this chip coupling was convention, since there exists SAMD21 and SAMD51 specific drivers in this package already. Your comment applies to those as well then.

That being said, I highly doubt we can achieve the SPI efficiency required for the pyportal_boing demo using the (SPI).Transfer method alone. These chip-specific drivers take advantage of the ILI9341's API, eliminating overhead performed with usual SPI transfers (e.g., not waiting for TXE/BSY, or toggling SPE, for every byte clocked out). @sago35 may be able to confirm or refute this from experience.

Waiting for TXE will result in several times worse performance.
For example, in the samd41 example, the performance goes from 40 fps to 10 fps.
Therefore, I think the ILI9341 driver needs to manipulate registers independently instead of using SPI.

#153 (comment)
#153 (comment)

[aykevl]

If disabling the SPI receiver is the only difference, then it should be relatively straightforward to implement in the machine package. In fact, some chips already have support for this.

See: https://github.com/tinygo-org/tinygo/blob/dev/src/machine/machine_nrf.go#L438-L452
In this case the TXD byte is double buffered, which ensures the SPI bus is in constant use (no pause between sent bytes).
Other chips probably work differently.

Essentially, you can implement a custom SPI.Tx function in the machine package that switches to write-only behavior when the rx byte slice is empty. This has the big advantage that it improves SPI for all drivers, not just this ili9341 driver.

[ardnew]

Essentially, you can implement a custom SPI.Tx function in the machine package that switches to write-only behavior when the rx byte slice is empty. This has the big advantage that it improves SPI for all drivers, not just this ili9341 driver.

Clever! I'd really like to attempt implementing this, but would be a separate effort from the current PR for SPI on the STM32F405.

This ILI9341 driver PR is still in draft, so if I can get that working on the machine-side, we can remove 90% of the additions in this PR.