Add support for 3-wire SPI displays. Addresses #26, #27 and #40

CMakeLists.txt

@@ -71,9 +71,12 @@ if (ARMV8A)
 endif()
 
 set(GPIO_TFT_DATA_CONTROL 0 CACHE STRING "Explicitly specify the Data/Control GPIO pin (sometimes also called Register Select)")
-if (GPIO_TFT_DATA_CONTROL)
- message(STATUS "Using GPIO pin ${GPIO_TFT_DATA_CONTROL} for Data/Control line")
- set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DGPIO_TFT_DATA_CONTROL=${GPIO_TFT_DATA_CONTROL}")
+if (GPIO_TFT_DATA_CONTROL GREATER 0)
+ message(STATUS "Using 4-wire SPI mode of communication, with GPIO pin ${GPIO_TFT_DATA_CONTROL} for Data/Control line")
+ set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DGPIO_TFT_DATA_CONTROL=${GPIO_TFT_DATA_CONTROL} -DSPI_4WIRE=1")
+elseif (GPIO_TFT_DATA_CONTROL LESS 0)
+ message(STATUS "Using 3-wire SPI mode of communication, i.e. a display that does not havea a Data/Control line")
+ set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DSPI_3WIRE=1")
 endif()
 
 set(GPIO_TFT_RESET_PIN 0 CACHE STRING "Explicitly specify the Reset GPIO pin (leave out if there is no Reset line)")

README.md

@@ -130,7 +130,7 @@ If you connected wires directly on the Pi instead of using a Hat from the above
 
 And additionally, pass the following to customize the GPIO pin assignments you used:
 
-- `-DGPIO_TFT_DATA_CONTROL=number`: Specifies/overrides which GPIO pin to use for the Data/Control (DC) line on the 4-wire SPI communication. This pin number is specified in BCM pin numbers.
+- `-DGPIO_TFT_DATA_CONTROL=number`: Specifies/overrides which GPIO pin to use for the Data/Control (DC) line on the 4-wire SPI communication. This pin number is specified in BCM pin numbers. If you have a 3-wire SPI display that does not have a Data/Control line, **set this value to -1**, i.e. `-DGPIO_TFT_DATA_CONTROL=-1` to tell fbcp-ili9341 to target 3-wire ("9-bit") SPI communication.
 - `-DGPIO_TFT_RESET_PIN=number`: Specifies/overrides which GPIO pin to use for the display Reset line. This pin number is specified in BCM pin numbers. If omitted, it is assumed that the display does not have a Reset pin, and is always on.
 - `-DGPIO_TFT_BACKLIGHT=number`: Specifies/overrides which GPIO pin to use for the display backlight line. This pin number is specified in BCM pin numbers. If omitted, it is assumed that the display does not have a GPIO-controlled backlight pin, and is always on. If setting this, also see the `#define BACKLIGHT_CONTROL` option in `config.h`.
 
@@ -395,7 +395,13 @@ If fbcp-ili9341 does not support your display controller, you will have to write
 
 #### Does fbcp-ili9341 work with 3-wire SPI displays?
 
-No, only 4-wire SPI displays work. Make sure the display has a Data/Control (DC) GPIO pin to connect.
+Yes! This is a more recent experimental feature that may not be as stable, and there are some limitations, but 3-wire ("9-bit") SPI display support is now available. If you have a 3-wire SPI display, i.e. one that does not have a Data/Control (DC) GPIO pin to connect, configure it via CMake with directive `-D-DGPIO_TFT_DATA_CONTROL=-1` to tell fbcp-ili9341 that it should be driving the display with 3-wire protocol.
+
+Current limitations of 3-wire communication are:
+ - The performance option `ALL_TASKS_SHOULD_DMA` is currently not supported, there is an issue with DMA chaining that prevents this from being enabled. As result, CPU usage on 3-wire displays will be slightly higher than on 4-wire displays.
+ - The performance option `OFFLOAD_PIXEL_COPY_TO_DMA_CPP` is currently not supported. As a result, 3-wire displays may not work that well on single core Pis like Pi Zero.
+ - This has only been tested on my Adafruit SSD1351 128x96 RGB OLED display, which can be soldered to operate in 3-wire SPI mode, so testing has not been particularly extensive.
+ - Displays that have a 16-bit wide command word, such as ILI9486, do not currently work in 3-wire ("17-bit") mode. (But ILI9486L has 8-bit command word, so that does work)
 
 #### Does fbcp-ili9341 work with I2C, DPI, MIPI DSI or USB connected displays?
 
@@ -433,7 +439,7 @@ Unfortunately there are a number of things to go wrong that all result in a whit
 - double check that the display controller is really what you expected. Trying to drive with the display with wrong initialization code usually results in the display not reacting, and the screen stays white,
 - shut down and physically power off the Pi and the display in between multiple tests. Driving a display with a wrong initialization routine may put it in a bad state that needs a physical power off for it to reset,
 - if there is a reset pin on the display, make sure to pass it in CMake line. Or alternatively, try driving fbcp-ili9341 without specifying the reset pin,
-- make sure the display is configured to run 4-wire SPI mode, and not in parallel mode or 3-wire SPI mode. You may need to solder or desolder some connections or set a jumper to configure the specific driving mode.
+- make sure the display is configured to run 4-wire SPI mode, and not in parallel mode or 3-wire SPI mode. You may need to solder or desolder some connections or set a jumper to configure the specific driving mode. Support for 3-wire SPI displays does exist, but it is more limited and a bit experimental.
 
 #### The display stays blank at boot without lighting up
 
@@ -481,7 +487,7 @@ You can also try looking through the commit history to find changes related to y
 
 #### Which SPI display should I buy to make sure it works best with fbcp-ili9341?
 
-First, make sure the display is a 4-wire SPI and not a 3-wire one. fbcp-ili9341 does not currently support 3-wire SPI. A display is 4-wire SPI if it has a Data/Control (DC) GPIO line that needs connecting.
+First, make sure the display is a 4-wire SPI and not a 3-wire one. A display is 4-wire SPI if it has a Data/Control (DC) GPIO line that needs connecting. Sometimes the D/C pin is labeled RS (Register Select). Support for 3-wire SPI displays does exist, but it is experimental and not nearly as well tested as 4-wire displays.
 
 Second is the consideration about display speed. Below is a performance chart of the different displays I have tested. Note that these are sample sizes of one, I don't know how much sample variance there exists. Also I don't know if it is likely that there exists big differences between displays with same controller from different manufacturers. At least the different ILI9341 displays that I have are all quite consistent on performance, whether they are from Adafruit or WaveShare or from BuyDisplay.com.
 

config.h

@@ -110,7 +110,7 @@
 // requires that ALL_TASKS_SHOULD_DMA is also enabled.
 // #define UPDATE_FRAMES_WITHOUT_DIFFING
 
-#if defined(SINGLE_CORE_BOARD) && defined(USE_DMA_TRANSFERS)
+#if defined(SINGLE_CORE_BOARD) && defined(USE_DMA_TRANSFERS) && !defined(SPI_3WIRE) // TODO: 3-wire SPI displays are not yet compatible with ALL_TASKS_SHOULD_DMA option.
 // These are prerequisites for good performance on Pi Zero
 #ifndef ALL_TASKS_SHOULD_DMA
 #define ALL_TASKS_SHOULD_DMA

display.h

@@ -87,9 +87,10 @@
 #define SPI_BYTESPERPIXEL 2
 #endif
 
-#if (DISPLAY_DRAWABLE_WIDTH % 16 == 0) && defined(ALL_TASKS_SHOULD_DMA) &&!defined(USE_SPI_THREAD) && defined(USE_GPU_VSYNC) && !defined(DISPLAY_COLOR_FORMAT_R6X2G6X2B6X2)
+#if (DISPLAY_DRAWABLE_WIDTH % 16 == 0) && defined(ALL_TASKS_SHOULD_DMA) &&!defined(USE_SPI_THREAD) && defined(USE_GPU_VSYNC) && !defined(DISPLAY_COLOR_FORMAT_R6X2G6X2B6X2) && !defined(SPI_3WIRE)
 // If conditions are suitable, defer moving pixels until the very last moment in dma.cpp when we are about
 // to kick off DMA tasks.
+// TODO: 3-wire SPI displays are not yet compatible with this path. Implement support for this to optimize performance of 3-wire SPI displays on Pi Zero. (Pi 3B does not care that much)
 #define OFFLOAD_PIXEL_COPY_TO_DMA_CPP
 #endif
 
@@ -106,6 +107,6 @@ void DeinitSPIDisplay(void);
 #error Please define -DSPI_BUS_CLOCK_DIVISOR=<some even number> on the CMake command line! This parameter along with core_freq=xxx in /boot/config.txt defines the SPI display speed. (spi speed = core_freq / SPI_BUS_CLOCK_DIVISOR)
 #endif
 
-#if !defined(GPIO_TFT_DATA_CONTROL)
+#if !defined(GPIO_TFT_DATA_CONTROL) && !defined(SPI_3WIRE)
 #error Please reconfigure CMake with -DGPIO_TFT_DATA_CONTROL=<int> specifying which pin your display is using for the Data/Control line!
 #endif

dma.cpp

@@ -488,16 +488,27 @@ static void memcpy_to_dma_and_prev_framebuffer_in_c(uint16_t *dstDma, uint16_t *
  *dstPrevFramebuffer = prevData;
 }
 
+#if defined(ALL_TASKS_SHOULD_DMA) && defined(SPI_3WIRE)
+// Bug: there is something about the chained DMA transfer mechanism that makes write window coordinate set commands not go through properly
+// on 3-wire displays, but do not yet know what. (Remove this #error statement to debug)
+#error ALL_TASKS_SHOULD_DMA and SPI_3WIRE are currently not mutually compatible!
+#endif
+
+#if defined(OFFLOAD_PIXEL_COPY_TO_DMA_CPP) && defined(SPI_3WIRE)
+// We would have to convert 8-bit tasks to 9-bit tasks immediately after offloaded memcpy has been done below to implement this.
+#error OFFLOAD_PIXEL_COPY_TO_DMA_CPP and SPI_3WIRE are not mutually compatible!
+#endif
+
 void SPIDMATransfer(SPITask *task)
 {
 // There is a limit to how many bytes can be sent in one DMA-based SPI task, so if the task
 // is larger than this, we'll split the send into multiple individual DMA SPI transfers
 // and chain them together. This should be a multiple of 32 bytes to keep tasks cache aligned on ARMv6.
 #define MAX_DMA_SPI_TASK_SIZE 65504
 
- const int numDMASendTasks = (task->size + MAX_DMA_SPI_TASK_SIZE - 1) / MAX_DMA_SPI_TASK_SIZE;
+ const int numDMASendTasks = (task->PayloadSize() + MAX_DMA_SPI_TASK_SIZE - 1) / MAX_DMA_SPI_TASK_SIZE;
 
- volatile uint32_t *dmaData = (volatile uint32_t *)GrabFreeDMASourceBytes(4*(numDMASendTasks-1)+4*numDMASendTasks+task->size);
+ volatile uint32_t *dmaData = (volatile uint32_t *)GrabFreeDMASourceBytes(4*(numDMASendTasks-1)+4*numDMASendTasks+task->PayloadSize());
  volatile uint32_t *setDMATxAddressData = dmaData;
  volatile uint32_t *txData = dmaData+numDMASendTasks-1;
 
@@ -510,12 +521,12 @@ void SPIDMATransfer(SPITask *task)
 #ifdef OFFLOAD_PIXEL_COPY_TO_DMA_CPP
  uint8_t *data = task->fb;
  uint8_t *prevData = task->prevFb;
- const bool taskAndFramebufferSizesCompatibleWithTightMemcpy = (task->size % 32 == 0) && (task->width % 16 == 0);
+ const bool taskAndFramebufferSizesCompatibleWithTightMemcpy = (task->PayloadSize() % 32 == 0) && (task->width % 16 == 0);
 #else
- uint8_t *data = task->data;
+ uint8_t *data = task->PayloadStart();
 #endif
 
- int bytesLeft = task->size;
+ int bytesLeft = task->PayloadSize();
  int taskStartX = 0;
 
  while(bytesLeft > 0)
@@ -629,10 +640,12 @@ void SPIDMATransfer(SPITask *task)
  }
  if (!programRunning) return;
 
- pendingTaskBytes = task->size;
+ pendingTaskBytes = task->PayloadSize();
 
  // First send the SPI command byte in Polled SPI mode
  spi->cs = BCM2835_SPI0_CS_TA | BCM2835_SPI0_CS_CLEAR | DISPLAY_SPI_DRIVE_SETTINGS;
+
+#ifdef SPI_4WIRE
  CLEAR_GPIO(GPIO_TFT_DATA_CONTROL);
 #ifdef DISPLAY_SPI_BUS_IS_16BITS_WIDE
  spi->fifo = 0;
@@ -647,6 +660,8 @@ void SPIDMATransfer(SPITask *task)
 #endif
 
  SET_GPIO(GPIO_TFT_DATA_CONTROL);
+#endif
+
  spi->cs = BCM2835_SPI0_CS_DMAEN | BCM2835_SPI0_CS_CLEAR | DISPLAY_SPI_DRIVE_SETTINGS;
 
  dmaTx->cbAddr = VIRT_TO_BUS(dmaCb, tx0);
@@ -663,20 +678,21 @@ void SPIDMATransfer(SPITask *task)
 {
  // Transition the SPI peripheral to enable the use of DMA
  spi->cs = BCM2835_SPI0_CS_DMAEN | BCM2835_SPI0_CS_CLEAR | DISPLAY_SPI_DRIVE_SETTINGS;
- task->dmaSpiHeader = BCM2835_SPI0_CS_TA | DISPLAY_SPI_DRIVE_SETTINGS | (task->size << 16); // The first four bytes written to the SPI data register control the DLEN and CS,CPOL,CPHA settings.
+ uint32_t *headerAddr = task->DmaSpiHeaderAddress();
+ *headerAddr = BCM2835_SPI0_CS_TA | DISPLAY_SPI_DRIVE_SETTINGS | (task->PayloadSize() << 16); // The first four bytes written to the SPI data register control the DLEN and CS,CPOL,CPHA settings.
 
  // TODO: Ideally we would be able to directly perform the DMA from the SPI ring buffer from 'task' pointer. However
  // that pointer is shared to userland, and it is proving troublesome to make it both userland-writable as well as cache-bypassing DMA coherent.
  // Therefore these two memory areas are separate for now, and we memcpy() from SPI ring buffer to an intermediate 'dmaSourceMemory' memory area to perform
  // the DMA transfer. Is there a way to avoid this intermediate buffer? That would improve performance a bit.
- memcpy(dmaSourceBuffer.virtualAddr, (void*)&task->dmaSpiHeader, task->size + 4);
+ memcpy(dmaSourceBuffer.virtualAddr, headerAddr, task->PayloadSize() + 4);
 
  volatile DMAControlBlock *cb = (volatile DMAControlBlock *)dmaCb.virtualAddr;
  volatile DMAControlBlock *txcb = &cb[0];
  txcb->ti = BCM2835_DMA_TI_PERMAP(BCM2835_DMA_TI_PERMAP_SPI_TX) | BCM2835_DMA_TI_DEST_DREQ | BCM2835_DMA_TI_SRC_INC | BCM2835_DMA_TI_WAIT_RESP;
  txcb->src = dmaSourceBuffer.busAddress;
  txcb->dst = DMA_SPI_FIFO_PHYS_ADDRESS; // Write out to the SPI peripheral 
- txcb->len = task->size + 4;
+ txcb->len = task->PayloadSize() + 4;
  txcb->stride = 0;
  txcb->next = 0;
  txcb->debug = 0;
@@ -687,7 +703,7 @@ void SPIDMATransfer(SPITask *task)
  rxcb->ti = BCM2835_DMA_TI_PERMAP(BCM2835_DMA_TI_PERMAP_SPI_RX) | BCM2835_DMA_TI_SRC_DREQ | BCM2835_DMA_TI_DEST_IGNORE;
  rxcb->src = DMA_SPI_FIFO_PHYS_ADDRESS;
  rxcb->dst = 0;
- rxcb->len = task->size;
+ rxcb->len = task->PayloadSize();
  rxcb->stride = 0;
  rxcb->next = 0;
  rxcb->debug = 0;
@@ -699,7 +715,7 @@ void SPIDMATransfer(SPITask *task)
  dmaRx->cs = BCM2835_DMA_CS_ACTIVE;
  __sync_synchronize();
 
- double pendingTaskUSecs = task->size * spiUsecsPerByte;
+ double pendingTaskUSecs = task->PayloadSize() * spiUsecsPerByte;
  if (pendingTaskUSecs > 70)
  usleep(pendingTaskUSecs-70);
 

fbcp-ili9341.cpp

@@ -463,7 +463,7 @@ int main()
  SPITask *task = AllocTask(i->size*SPI_BYTESPERPIXEL);
  task->cmd = DISPLAY_WRITE_PIXELS;
 
- bytesTransferred += task->size+1;
+ bytesTransferred += task->PayloadSize()+1;
  uint16_t *scanline = framebuffer[0] + i->y * (gpuFramebufferScanlineStrideBytes>>1);
  uint16_t *prevScanline = framebuffer[1] + i->y * (gpuFramebufferScanlineStrideBytes>>1);