ILI9341 TFT driver (SPI) #153

sago35 · 2020-05-20T12:34:58Z

This PR adds SPI support for the ILI9341.

sago35 · 2020-05-20T12:37:05Z

ILI9341 TFT driver
- ILI9341 TFT driver #115
Seeed WioTerminal support
- Seeed WioTerminal support tinygo#1124

sago35 · 2020-05-20T12:40:38Z

Below is a commit that adds basic support, but it's too slow.

b8efa55

With pyportal_boing I had the following FPS

WioTerminal with spiDriver b8efa55
- 10fps
PyPortal with parallelDriver
- 51 fps

sago35 · 2020-05-20T12:57:42Z

Using 32bit-extension made it a little faster.

WioTerminal with spiDriver b8efa55
- 10 fps
WioTerminal with spiDriver + 32bit extension
- 36 fps
  - Initially, it slows down at 42 fps and then converges to about 36 fps.

note:
tinygo-org/tinygo@658e08b

sago35 · 2020-05-20T13:00:56Z

The reason CI is failing is that there is no function for 32bit extensions that I added below.

b8efa55.

aykevl · 2020-05-20T22:02:33Z

ili9341/spi_atsamd51.go

+func (pd *spiDriver) configure(config *Config) {
+}
+
+//go:inline


Do all these //go:inline annotations really improve performance?

In general they should only be added as a fix for performance. Also, before doing that please test with -opt=2 first because that will likely make the //go:inline pragmas unnecessary (and you should add -opt=2 if speed is important).

go:inline is something I copied from parallel_atsamd51.go.
I wasn't particularly aware of it.

Removing go:inline didn't change the speed.
So, I'll remove it.

The FPS didn't change when I set -opt=2.

sago35 · 2020-05-21T11:26:35Z

By using INTEFLAG.DRE, I was able to improve performance without using 32bit-extensions.
ILI9341's pyportal_boing is now 43 fps instead of 36 fps.

There is one area that bothers me.
In the SPI source, the processing speed gets slower and slower with each process.
However, it appears to be converging at 43 fps.

The pyportal parallel_atsamd51.go is similarly slow, but converges from 51 fps to 49 fps.

// pyportal with parallel_atsamd51.go
width, height == 240 320
51  fps
51  fps
51  fps
51  fps
51  fps
49  fps
49  fps
49  fps
50  fps
50  fps
49  fps
49  fps
49  fps
49  fps
49  fps
49  fps
49  fps
49  fps
49  fps

// wioterminal with spi_atsamd51.go
width, height == 240 320
51  fps
46  fps
45  fps
44  fps
44  fps
45  fps
44  fps
44  fps
44  fps
44  fps
44  fps
44  fps
44  fps
44  fps
44  fps
44  fps
43  fps
43  fps
44  fps
44  fps

sago35 · 2020-05-21T11:32:41Z

I don't understand why CircleCI failed.
examples/ili9341/basic has pyportal.go and backlight and display should be set, but it's an error.

tinygo build -size short -o ./build/test.hex -target=pyportal ./examples/ili9341/basic/main.go
# ./examples/ili9341/basic/main.go
examples/ili9341/basic/main.go:21:2: undeclared name: backlight
examples/ili9341/basic/main.go:23:2: undeclared name: display
examples/ili9341/basic/main.go:24:19: undeclared name: display
examples/ili9341/basic/main.go:26:2: undeclared name: display
examples/ili9341/basic/main.go:27:2: undeclared name: backlight
examples/ili9341/basic/main.go:29:2: undeclared name: display
examples/ili9341/basic/main.go:30:2: undeclared name: display
examples/ili9341/basic/main.go:31:2: undeclared name: display
examples/ili9341/basic/main.go:32:2: undeclared name: display
examples/ili9341/basic/main.go:33:2: undeclared name: display
make: *** [Makefile:60: smoke-test] Error 1

sago35 · 2020-05-24T02:47:07Z

~~TODO: change pin names~~
Changed.

deadprogram · 2020-05-26T17:51:44Z

Looks like the build problem is this:

tinygo build -size short -o ./build/test.hex -target=pyportal ./examples/ili9341/basic/main.go
# ./examples/ili9341/basic/main.go
examples/ili9341/basic/main.go:21:2: undeclared name: backlight
examples/ili9341/basic/main.go:23:2: undeclared name: display
examples/ili9341/basic/main.go:24:19: undeclared name: display
examples/ili9341/basic/main.go:26:2: undeclared name: display
examples/ili9341/basic/main.go:27:2: undeclared name: backlight
examples/ili9341/basic/main.go:29:2: undeclared name: display
examples/ili9341/basic/main.go:30:2: undeclared name: display
examples/ili9341/basic/main.go:31:2: undeclared name: display
examples/ili9341/basic/main.go:32:2: undeclared name: display
examples/ili9341/basic/main.go:33:2: undeclared name: display
make: *** [Makefile:60: smoke-test] Error 1

sago35 · 2020-05-27T14:39:14Z

Now I know why the CI test was failing.
This is because I specified main.go in go build.
So, it's fixed.
I added a test for pyportal_boing as a side note.

But this time, I got a different error.
Where I made the error is not where I made the change.

#!/bin/bash -eo pipefail
make smoke-test

tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/adt7410/main.go
# tinygo.org/x/drivers/adt7410
examples/adt7410/main.go:8:2: cannot find package "." in:
	/usr/local/go/src/vendor/tinygo.org/x/drivers/adt7410
make: *** [Makefile:12: smoke-test] Error 1

Exited with code exit status 2

sago35 · 2020-05-28T14:26:53Z

@deadprogram
The change does not appear to be related to CI error.
Do you have anything to think about?

diff: 2ac3d61

ci before: https://circleci.com/gh/tinygo-org/drivers/468
ci now: https://circleci.com/gh/tinygo-org/drivers/471?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

deadprogram · 2020-05-28T17:25:42Z

This change is now needed in order to run tinygo build on CircleCI: 7967e82

@aykevl might this be related to us now using Go modules? I am OK with making this change for the build, but I think we might want to understand if it might be something to look into further.

sago35 · 2020-06-01T22:24:19Z

rebased.
smoke-test is now OK.

aykevl · 2020-06-04T22:53:18Z

I think the CI issue was that we were checking out in GOROOT, while it should have been checking out in GOPATH. But now with Go modules, the location doesn't matter much (apart from that it shouldn't be in GOROOT, apparently). See: #157

sago35 · 2020-06-06T10:53:43Z

rebased.

deadprogram · 2020-06-06T14:23:28Z

I have tested on PyPortal and it is noticeably faster. Good work @sago35 thank you!

@aykevl @bgould @conejoninja any last feedback before I squash/merge this?

sago35 · 2020-06-06T22:46:34Z

wioterminal.go did not need power control, so I removed it.
Other than that, I haven't changed it.

sago35 · 2020-06-09T11:16:11Z

Removed unnecessary pins in struct.
I think everything else is fine.

sago35 · 2020-06-09T11:20:47Z

I'm working on another driver for spi with DMA, but I'll be creating a separate PR for it.

pyportal / parallel : 49fps
wioterminal / spi : 44 fps
wioterminal / spi + DMA : 64 fps

The source code is below, but refactoring is required.
sago35@71349ad

deadprogram · 2020-06-12T10:49:33Z

Any feedback on this @conejoninja or @bgould ?

sago35

Updated. (5cd2e05)
Since the actual values were the same, the behavior remains the same.

	SERCOM_SPIS_CTRLB_RXEN        = 0x20000 // Bit RXEN.
	SERCOM_SPIS_INTFLAG_DRE       = 0x1  // Bit DRE.
	SERCOM_SPIS_SYNCBUSY_CTRLB    = 0x4  // Bit CTRLB.


	SERCOM_SPIM_CTRLB_RXEN        = 0x20000 // Bit RXEN.
	SERCOM_SPIM_INTFLAG_DRE       = 0x1  // Bit DRE.
	SERCOM_SPIM_SYNCBUSY_CTRLB    = 0x4  // Bit CTRLB.

sago35 · 2020-06-13T03:10:01Z

ili9341/spi_atsamd51.go

+}
+
+func (pd *spiDriver) write8(b byte) {
+	pd.bus.Bus.CTRLB.ClearBits(sam.SERCOM_SPIS_CTRLB_RXEN)


It's SPIM, not SPIS, so it needs to be fixed

bgould · 2020-06-14T13:52:20Z

@deadprogram How are you testing this on pyportal? Did you have to cut and short the traces on the back to switch it into SPI mode, or some other way? The ILI9341 board I have is my pyportal so just curious if I need to do that (I want to try out the DMA improvement when its ready so I don't mind making that change if it is necessary)

deadprogram · 2020-06-14T14:16:25Z

No I did not modify my PyPortal. I do have a WioTerminal now that I am using for testing.

deadprogram · 2020-06-14T14:18:04Z

@sago35 are you going to add your DMA code to this PR?

conejoninja · 2020-06-14T15:16:35Z

Did not test it, but the code looks good to me. Maybe the driver interface should be in a separate file, or at the beginning of ili6341.go (I like to put on top var declarations, structs, and interfaces, not sure if that follows the go guidelines)

sago35 · 2020-06-15T03:14:40Z

are you going to add your DMA code to this PR?
It is better to keep PR separate.

For DMA support, a wait is required after DMA transfer and before SPI transmission (or CS/DC, etc.).
It may be troublesome to coexist with parallel if the current method of ili9341.go is used.

The following is a great library, and it would be easier to make a good library if you focus on SPI + DMA only, as shown below.
https://github.com/lovyan03/LovyanGFX

sago35 · 2020-06-15T03:15:58Z

Using DMA in the following way will not change the speed.
You must have another job to do while you are waiting for DMA to complete.

func (pd *spiDriver) write16sl(data []uint16) {
	pd.DmaSend16(data)
	for !sam.DMAC.CHANNEL[pd.dmaChannel].CHINTFLAG.HasBits(sam.DMAC_CHANNEL_CHINTFLAG_TCMPL) {
	}
	sam.DMAC.CHANNEL[pd.dmaChannel].CHINTFLAG.SetBits(sam.DMAC_CHANNEL_CHINTFLAG_TCMPL)
}

bgould · 2020-06-15T04:57:16Z

Not really pertinent to this PR, but pretty sure DMA is possible with the parallel interface also - https://www.youtube.com/watch?v=ClabKcWVxT8 - there is C code demonstrating that in the Adafruit ILI9341 driver for Arduino, it is a little hard to read though IMO especially because it is buried inside #ifdefs.

I'm not going modify my pyportal at this time to test, the code looks good to me though. Probably DMA could be handled separately; I imagine that possibly could require a new driver interface - I believe @aykevl might have some ideas about that and also the current work to make blocking operations in TinyGo work properly with interrupts could come into play as well.

Thats all to say that the SPI driver looks good to me and DMA support could be integrated separately IMO. Nice work @sago35 :)

deadprogram · 2020-06-15T16:45:39Z

I am going to squash/merge this PR now. Thank you very much @sago35 for another awesome contribution!

sago35 · 2020-06-16T02:54:21Z

https://www.youtube.com/watch?v=ClabKcWVxT8

@bgould
If you know where the source code is, please let me know.
I would like to read it.

My latest try was 136 fps, but adafruit's video is a bit faster than that.

pyportal_boing 136 fps with TinyGo + Wio Terminal
https://www.youtube.com/watch?v=YZ9RTJB9Ac4

sago35 · 2020-06-16T02:56:02Z

Since DMA is not supposed to be able to do DMA on ports, I guess it's like inverting wr with TC while rewriting RAM with DMA.

sago35 · 2020-06-16T03:10:43Z

Did you have to cut and short the traces on the back to switch it into SPI mode, or some other way?

I also tried the SPI by shorting out the jumper on the PyPortal board, but the screen did not appear.

It is important to note that TFT_RS and SCK are shorted by the jumper.
I think it is not good to switch the jumper while the parallel driver program is running.
I may have broken the board.

The feather-m4 and the external SPI ILI9341 worked correctly, so I think something is wrong with the settings.

aykevl · 2020-06-17T13:58:15Z

I believe @aykevl might have some ideas about that and also the current work to make blocking operations in TinyGo work properly with interrupts could come into play as well.

My intention is the following:

Make regular spi.Tx functions use DMA transparently when available (in a blocking fashion). This can be useful to do the send in one goroutine while the other renders the next frame. This depends on scheduler support that Jaden is working on.
In addition to that, to avoid scheduler overhead it may be necessary to also define a "start transaction" function and a way to wait until the last one is sent. That would allow a single goroutine to use DMA to improve performance. But it also complicates things so ideally I'd want to see how bad the first option is before doing this one.

sago35 · 2020-06-19T10:23:50Z

I tried again, and the SPI driver works on the PyPortal.
As it says on the adafruit page, you need to make two changes.

It is best not to use ILI9341, such as blinky, before working with it.
TFT_RS and SCK will be shorted out.

https://www.youtube.com/watch?v=VKO-poNS8wU

sago35 · 2020-06-19T11:12:38Z

examples/ili9341/pyportal_boing/pyportal.go

// +build pyportal

package main

import (
	"machine"

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

var (
	display = ili9341.NewSpi(
		machine.SPI0,
		machine.TFT_WR, // In SPI mode, WR is used instead of DC.
		machine.TFT_CS,
		//machine.NoPin,
		machine.TFT_RESET,
	)

	backlight = machine.TFT_BACKLIGHT
)

func init() {
	machine.SPI0.Configure(machine.SPIConfig{
		SCK:       machine.SPI0_SCK_PIN,
		MOSI:      machine.SPI0_MOSI_PIN,
		MISO:      machine.SPI0_MISO_PIN,
		LSBFirst:  false,
		Mode:      machine.Mode0,
		Frequency: 40000000,
	})

	machine.D34.Configure(machine.PinConfig{Mode: machine.PinOutput})
	machine.D34.Low()
	machine.D35.Configure(machine.PinConfig{Mode: machine.PinOutput})
	machine.D35.Low()
	machine.D36.Configure(machine.PinConfig{Mode: machine.PinOutput})
	machine.D36.Low()
	machine.D37.Configure(machine.PinConfig{Mode: machine.PinOutput})
	machine.D37.Low()
	machine.D38.Configure(machine.PinConfig{Mode: machine.PinOutput})
	machine.D38.Low()
	machine.D39.Configure(machine.PinConfig{Mode: machine.PinOutput})
	machine.D39.Low()
	machine.D40.Configure(machine.PinConfig{Mode: machine.PinOutput})
	machine.D40.Low()
	machine.D41.Configure(machine.PinConfig{Mode: machine.PinOutput})
	machine.D41.Low()

	machine.TFT_RD.Configure(machine.PinConfig{Mode: machine.PinOutput})
	machine.TFT_RD.High()

	machine.TFT_DC.Configure(machine.PinConfig{Mode: machine.PinInput})
}

* ili9341: Add spidriver

sago35 changed the title ~~ILI9341 TFT driver (SPI)~~ [WIP] ILI9341 TFT driver (SPI) May 20, 2020

sago35 changed the title ~~[WIP] ILI9341 TFT driver (SPI)~~ ILI9341 TFT driver (SPI) May 20, 2020

aykevl reviewed May 20, 2020

View reviewed changes

aykevl mentioned this pull request May 22, 2020

Seeed WioTerminal support tinygo-org/tinygo#1124

Merged

5 tasks

sago35 force-pushed the ili9341-spidriver branch from 60250dc to 2ac3d61 Compare May 27, 2020 14:36

sago35 force-pushed the ili9341-spidriver branch from 2ac3d61 to e630c00 Compare June 1, 2020 22:16

sago35 added 8 commits June 6, 2020 19:39

Add ili9341 spidriver (atsamd51)

1b41d0a

Add suport samd5x's SPI 32-bit extension

3b77a1b

Add wioterminal examples for ILI9341

18f5f22

Delete //go:inline annotations

e5ce7b4

Improve SPI performance

86805a6

Update pin names

a86a22d

Update pin names

71e5cc3

Fix tests

33c4d3a

sago35 force-pushed the ili9341-spidriver branch from e630c00 to 33c4d3a Compare June 6, 2020 10:40

Remove unnecessary power control

88fcc21

Remove unnecessary pins

eca6f9e

Fix register name

5cd2e05

sago35 commented Jun 14, 2020

View reviewed changes

deadprogram merged commit 941ea4e into tinygo-org:dev Jun 15, 2020

sago35 mentioned this pull request Jun 18, 2020

machine: add DMA support to spi tx tinygo-org/tinygo#1176

Closed

sago35 added a commit to sago35/drivers that referenced this pull request Jul 13, 2020

ILI9341 TFT driver (SPI) (tinygo-org#153)

5b67b09

* ili9341: Add spidriver

sago35 mentioned this pull request Sep 15, 2020

ILI9341: add SPI driver for STM32F4 #196

Draft

ILI9341 TFT driver (SPI) #153

ILI9341 TFT driver (SPI) #153

Conversation

sago35 commented May 20, 2020

sago35 commented May 20, 2020

sago35 commented May 20, 2020 • edited Loading

sago35 commented May 20, 2020 • edited Loading

sago35 commented May 20, 2020

aykevl May 20, 2020

Choose a reason for hiding this comment

sago35 May 20, 2020

Choose a reason for hiding this comment

sago35 May 20, 2020

Choose a reason for hiding this comment

sago35 commented May 21, 2020 • edited Loading

sago35 commented May 21, 2020

sago35 commented May 24, 2020 • edited Loading

deadprogram commented May 26, 2020

sago35 commented May 27, 2020

sago35 commented May 28, 2020

deadprogram commented May 28, 2020

sago35 commented Jun 1, 2020

aykevl commented Jun 4, 2020

sago35 commented Jun 6, 2020

deadprogram commented Jun 6, 2020

sago35 commented Jun 6, 2020

sago35 commented Jun 9, 2020

sago35 commented Jun 9, 2020 • edited Loading

deadprogram commented Jun 12, 2020

sago35 left a comment

Choose a reason for hiding this comment

sago35 Jun 13, 2020

Choose a reason for hiding this comment

bgould commented Jun 14, 2020

deadprogram commented Jun 14, 2020

deadprogram commented Jun 14, 2020

conejoninja commented Jun 14, 2020

sago35 commented Jun 15, 2020

sago35 commented Jun 15, 2020

bgould commented Jun 15, 2020

deadprogram commented Jun 15, 2020

sago35 commented Jun 16, 2020

sago35 commented Jun 16, 2020

sago35 commented Jun 16, 2020

aykevl commented Jun 17, 2020

sago35 commented Jun 19, 2020

sago35 commented Jun 19, 2020

sago35 commented May 20, 2020 •

edited

Loading

sago35 commented May 20, 2020 •

edited

Loading

sago35 commented May 21, 2020 •

edited

Loading

sago35 commented May 24, 2020 •

edited

Loading

sago35 commented Jun 9, 2020 •

edited

Loading