Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions examples/ili9341/pyportal_boing/feather-stm32f405.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// +build feather_stm32f405

package main

import (
"machine"

"tinygo.org/x/drivers/ili9341"
)

var (
csPin = machine.D12
dcPin = machine.D10
display = ili9341.NewSPI(
machine.SPI0,
dcPin,
csPin,
machine.D8, // if wired to 3.3V, pick an unused pin
)

// ILI9341's LED pin. set this to an unused pin (but not NoPin!) if
// wired via resistor straight to 3.3V. the boing example tries to
// set this pin and will panic if NoPin is used.
backlight = machine.D13
Comment on lines +21 to +24
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

)

func init() {
machine.SPI0.Configure(machine.SPIConfig{
SCK: machine.SPI0_SCK_PIN,
SDO: machine.SPI0_SDO_PIN,
SDI: machine.SPI0_SDI_PIN,
Frequency: 40000000,
})
}
1 change: 1 addition & 0 deletions examples/ili9341/pyportal_boing/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ func main() {
backlight.High()

display.SetRotation(ili9341.Rotation270)
time.Sleep(50 * time.Millisecond)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reasons for the addition?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

DrawBackground()

startTime = time.Now().UnixNano()
Expand Down
142 changes: 142 additions & 0 deletions ili9341/spi_stm32f4.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
// +build stm32f4

package ili9341

import (
"device/stm32"
"machine"
)

type spiDriver struct {
bus machine.SPI
}

func NewSPI(bus machine.SPI, dc, cs, rst machine.Pin) *Device {
return &Device{
dc: dc,
cs: cs,
rst: rst,
rd: machine.NoPin,
driver: &spiDriver{
bus: bus,
},
}
}

func (pd *spiDriver) configure(config *Config) {
}

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)
}

Comment on lines +29 to +38
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
}

Copy link
Contributor Author

@ardnew ardnew Sep 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

@ardnew ardnew Sep 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

func (pd *spiDriver) write8n(b byte, n int) {
if !pd.bus.Bus.CR1.HasBits(stm32.SPI_CR1_SPE) {
pd.bus.Bus.CR1.SetBits(stm32.SPI_CR1_SPE)
}

for i := 0; i < n; i++ {
pd.setWord(b, i == 0, i+1 == n)
}

pd.bus.Bus.CR1.ClearBits(stm32.SPI_CR1_SPE)
}

func (pd *spiDriver) write8sl(b []byte) {
if !pd.bus.Bus.CR1.HasBits(stm32.SPI_CR1_SPE) {
pd.bus.Bus.CR1.SetBits(stm32.SPI_CR1_SPE)
}

for i, w := range b {
pd.setWord(w, i == 0, i+1 == len(b))
}

pd.bus.Bus.CR1.ClearBits(stm32.SPI_CR1_SPE)
}

func (pd *spiDriver) write16(data uint16) {
if !pd.bus.Bus.CR1.HasBits(stm32.SPI_CR1_SPE) {
pd.bus.Bus.CR1.SetBits(stm32.SPI_CR1_SPE)
}

pd.setWord(uint8(data>>8), true, false)
pd.setWord(uint8(data), false, true)

pd.bus.Bus.CR1.ClearBits(stm32.SPI_CR1_SPE)
}

func (pd *spiDriver) write16n(data uint16, n int) {
if !pd.bus.Bus.CR1.HasBits(stm32.SPI_CR1_SPE) {
pd.bus.Bus.CR1.SetBits(stm32.SPI_CR1_SPE)
}

for i := 0; i < n; i++ {
pd.setWord(uint8(data>>8), i == 0, false)
pd.setWord(uint8(data), false, i+1 == n)
}

pd.bus.Bus.CR1.ClearBits(stm32.SPI_CR1_SPE)
}

func (pd *spiDriver) write16sl(data []uint16) {
if !pd.bus.Bus.CR1.HasBits(stm32.SPI_CR1_SPE) {
pd.bus.Bus.CR1.SetBits(stm32.SPI_CR1_SPE)
}

for i, w := range data {
pd.setWord(uint8(w>>8), i == 0, false)
pd.setWord(uint8(w), false, i+1 == len(data))
}

pd.bus.Bus.CR1.ClearBits(stm32.SPI_CR1_SPE)
}

// puts a single 8-bit word in the SPI data register (DR).
// if first (first word being transmitted) is false, waits for the SPI transmit
// buffer empty bit (TXE) is set before putting the word in DR.
// if last (last word being transmitted) is true, waits for the SPI transmit
// buffer empty bit (TXE) is set and SPI bus busy bit (BSY) is clear before
// returning.
// for all wait operations, a fixed number of wait iterations (const tryMax) are
// performed before a timeout is assumed.
// if timeout occurs, returns false. otherwise, returns true.
func (pd *spiDriver) setWord(word uint8, first bool, last bool) bool {

const tryMax = 10000

canWrite := first
for i := 0; (!canWrite) && (i < tryMax); i++ {
canWrite = pd.bus.Bus.SR.HasBits(stm32.SPI_SR_TXE)
}
if !canWrite {
return false // timeout
}

pd.bus.Bus.DR.Set(uint32(word))

if last {
canReturn := false
for i := 0; (!canReturn) && (i < tryMax); i++ {
canReturn = pd.bus.Bus.SR.HasBits(stm32.SPI_SR_TXE)
}
if !canReturn {
return false // timeout
}

canReturn = false
for i := 0; (!canReturn) && (i < tryMax); i++ {
canReturn = !pd.bus.Bus.SR.HasBits(stm32.SPI_SR_BSY)
}
if !canReturn {
return false // timeout
}
}

return true
}