ILI9341_t3 readRect() / writeRect() bug

Yes, I tried your changes and it happens that readRect() was executed infinite. However, I think we should not handle the case when bytes were eaten because of too high clock rates or noise. If so, we should include a timeout.
 
Made another update to the github above. Wondering if maybe the input queue got full while I was waiting for EOQ flag to be set. So did a quick and dirty of checking for three bytes available in that loop and process in there as well.

You might give it a try.
 
There is no more lost pixel, however sometimes readPixel() hangs in infinite loop, even with regular Teensy 3.1 clock of 72 MHz (tested with 7.2MHz SPI clock). Please note, I can't guarantee that the signal to the display is clean. Last time I viewed the signal lines with an oscilloscope, the clock looks very horrible, almost not square wave like but with many many random frequencys like pure noise. This is why I ordered the Schmitt-Trigger buffers as I mentioned in previous post. I'll try them to find out if this helps to get cleaner signals. Never the less, this method should not hang in infinite loop.
 
We know the Teensy 3 is capable of generating clean clock signals. I assume this means that the noise must be coming from the display. For such an inexpensive display, one could hardly complain, but I wonder if the display module might be missing something to reduce the noise. I'm not an electrical engineer, so I'm a bit out of my depth. If the noise is indeed coming from the display, I'm not sure how an external buffer would improve matters.

On the other hand, the magnitude of the problem does seem to depend on both the Teensy clock frequency and the SPI clock frequency. I wonder if the best we can do is one of the following:

  • Repeat the readback operation for each pixel until two values in a row match or fail if a count is exceeded
  • Don't use the readback capability, but instead keep a second copy of the display image in RAM or in a memory external to the Teensy.

At the best frequencies, the readback errors seem to average under a dozen per screen image and I haven't observed very high variance, so temporal redundancy might not be so bad. Also, by limiting the number of retries, the worst case can be managed.
 
So, what is the conclusion to this issue? I want to use a ILI9341 to display transparent TGA files from the SD card. I already have that part sorted out. However, I have some problem reading the current colors on screen with readRect or readPixel. The colors returned are off. They are consistent with what they should be, but always off. I made a small debug program just to figure it out and get the following color:

I use a for loop to send pixels of 0 to 31 blue value to the display and I get the following results back:
What I send:
0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31
With readRect
0 0 1 1 2 2 3 3 4 4 5 5 6 6 7 7 24 24 25 25 26 26 27 27 28 28 29 29 30 30 31 31 .
With readPixel
0 0 1 1 2 2 3 3 4 4 5 5 6 6 7 7 24 24 25 25 26 26 27 27 28 28 29 29 30 30 31 31 .

This is a loop through the 32 adjacent pixels. Included is the program I used to get this result.

Any idea what is going on?


Thanks
 

Attachments

  • ReadRectDebug.ino
    1.7 KB · Views: 133
Last edited:
What I've observed is that the values read back are not deterministic and are sensitive to the clock speeds, so this would rule out 18-bit encoding as the sole culprit.
 
If I read the readRect and readPixel functions in the library, they expect 3 bytes (18 bits) from the ili9341 and then pass these bytes to the colors565 function before returning a uint16_t or and array of that. So, this seems to be taken in account by the functions themselves and anyway, as a "end-user", I should not have to bother about how the color values are returned by the lcd to the teensy; the library should cope with that and present me with a valid 565 color value.

It might be a timing issue. I reduced the CPU speed to 24MHz and the values returned by readRect were correct. The values returned by readPixel were wrong: as I put 0 to 31 to the drawPixel, I should only change the blue color and indeed, the pixels on the display are a gradient of blue. What I get back from the readPixel is:
0 64 128 192 256 320 384 448 512 576 640 704 768 832 896 960 1056 1120 18 1248 1312 1376 1440 1504 1568 1632 1696 1760 1824 1888 1952 2016
which is a gradient of green.

So, is it fixable?
 
My guess is yes, it is fixable by adding delays only where needed or by working around the unreliable/slow read back capability.

[This is wrong. Was thinking the display was smaller.]
The Teensy 3.1 has just enough RAM for a frame buffer, but for applications that can get by with indexed color, one byte per pixel instead of three would leave a decent amount of RAM for other things.

An alternative would be a frame buffer for a fraction of the display, say half or quarter. The drawing primitives could be extended to write to the frame buffer and maintain a display list. The code change would be a finger exercise, but the drawing library is pretty small. Definitely better to fix it than work around it, but certainly there are ways to make some read-back or redraw capability work.
 
Last edited:
I know at one point I posted a version that allowed you to easily change the read SPI speed to a slower speed which helped some chips work better... As pictographer mentioned, driver could be changed to use a memory buffer that all of the primitives write out through, and which you then add calls to tell when you wish for the display to be updated. I know some versions of these ILI... libraries have been made to do this, but don't remember which library they started with nor was it the Teensy or the Edison...
 
Ok, so no easy way to do transparency on the whole display. Framebuffer for the whole display would eat 153K (16 bpp). Quite disappointing as I had all the other parts of the puzzle in place:

I planned to use Font Awesome and Weathercons and then convert the sprites to PNG.
I created a patch for Imagemagick to enable it to create valid 16 bit TGA files with 1 bit transparency from these PNG.
The decoding part in the teensy was also done.
I would then be able to draw a small GUI and place all kind of interesting icons/images all over the place.

I guess I'll wait for any development for now and program some new functionalities instead.
 
I've made some progress on the bugs in ILI9341_t3::readPixel(). A couple of small changes appear to make it work correctly provided the Teensy 3.1 clock frequency is 72MHz or 24MHz. Other clock frequencies still produce errors.

readPixel() returns non-deterministic incorrect values a fraction of a percent of the time. The errors vary with the clock frequency but not in a way that's obvious to me.

My changes to ILI9341_t3.cpp are highlighted below.
Code:
// Read Pixel at x,y and get back 16-bit packed coloruint16_t ILI9341_t3::readPixel(int16_t x, int16_t y)
{
        uint8_t dummy __attribute__((unused));
        uint8_t r,g,b;


        SPI.beginTransaction(SPISettings(SPICLOCK, MSBFIRST, SPI_MODE0));


        setAddr(x, y, x, y);
        writecommand_cont(ILI9341_RAMRD); // read from RAM
        waitTransmitComplete();


        // Push 4 bytes over SPI
        KINETISK_SPI0.PUSHR = 0 | (pcs_data << 16) | SPI_PUSHR_CTAS(0)| SPI_PUSHR_CONT;
[COLOR=#0000ff]        [B]delayMicroseconds(4);[/B][/COLOR]
        waitFifoEmpty();    // wait for both queues to be empty.


        KINETISK_SPI0.PUSHR = 0 | (pcs_data << 16) | SPI_PUSHR_CTAS(0)| SPI_PUSHR_CONT;
        KINETISK_SPI0.PUSHR = 0 | (pcs_data << 16) | SPI_PUSHR_CTAS(0)| SPI_PUSHR_CONT;
        KINETISK_SPI0.PUSHR = 0 | (pcs_data << 16) | SPI_PUSHR_CTAS(0)| SPI_PUSHR_EOQ;


        // Wait for End of Queue
        while ((KINETISK_SPI0.SR & SPI_SR_EOQF) == 0) ;
        KINETISK_SPI0.SR = SPI_SR_EOQF;  // make sure it is clear


        // Read Pixel Data
[COLOR=#0000ff][B]//        dummy = KINETISK_SPI0.POPR;     // Read a DUMMY byte of GRAM
[/B][/COLOR]        r = KINETISK_SPI0.POPR;         // Read a RED byte of GRAM
        g = KINETISK_SPI0.POPR;         // Read a GREEN byte of GRAM
        b = KINETISK_SPI0.POPR;         // Read a BLUE byte of GRAM


        SPI.endTransaction();
        return color565(r,g,b);
}

I wrote a sketch to illustrate the problem.
Code:
// Illustration of ILI 9431 readback bug over SPI.// See notes at the bottom.


#include "SPI.h"
#include "ILI9341_t3.h"


ILI9341_t3 d = ILI9341_t3(10, 9);


uint16_t buf[80];


uint16_t good[512];
uint16_t bad[512];


uint8_t diag[5];


// http://en.wikipedia.org/wiki/Hamming_distance
int hamming_distance(unsigned x, unsigned y) {
  int       dist;
  unsigned  val;


  dist = 0;
  val = x ^ y;


  // Count the number of bits set
  while (val != 0) {
    // A bit is set, so increment the count and clear the bit
    dist++;
    val &= val - 1;
  }


  // Return the number of differing bits
  return dist;
}


// Read diagnostics. Only appears to work when Teensy CPU is 72 Mhz or 24 Mhz.
void readdiag() {
  diag[0] = d.readcommand8(ILI9341_RDMODE);
  diag[1] = d.readcommand8(ILI9341_RDMADCTL);
  diag[2] = d.readcommand8(ILI9341_RDPIXFMT);
  diag[3] = d.readcommand8(ILI9341_RDIMGFMT);
  diag[4] = d.readcommand8(ILI9341_RDSELFDIAG);
}


void printdiag() {
  d.setCursor(0, 4);
  d.print("Display Power Mode: 0x"); d.println(diag[0], HEX);
  d.print("MADCTL Mode: 0x");        d.println(diag[1], HEX);
  d.print("Pixel Format: 0x");       d.println(diag[2], HEX);
  d.print("Image Format: 0x");       d.println(diag[3], HEX);
  d.print("Self Diagnostic: 0x");    d.println(diag[4], HEX);
}


// Draw four rows. 
// Row 1 has the diagnostic codes. 
// Rows 2, 3 and 4 should match.
// Row 2 is a gradient created with drawFastVLine().
// Row 3 is a gradient created with writeRect().
// Row 4 is a gradient created with readPixel() and 
// writeRect().
void draw() {
  
  // Draw row labels.
  for (int i = 1; i <= 4; ++i) {
    int y = 60 * (i - 1) + 26;
    d.setCursor(288, y);
    d.print(i);
    d.drawFastHLine(256, 60 * i, 320 - 256, ILI9341_WHITE);
  }
  
  // Draw three gradients that should match.
  for (int x = 0; x < 256; ++x) {
    
    // Row 2: Draw one column of a gradient with drawFastVLine().
    uint16_t c = d.color565(x, x, x);
    d.drawFastVLine(x, 60, 60, c);


    // Row 3: Draw one column of a gradient with writeRect().
    for (int i = 0; i < 60; ++i) buf[i] = c;
    d.writeRect(x, 120, 1, 60, buf);
    
    // Row 4: Draw one column of a gradient with readPixel() and writeRect().


    // Attempting to copy from first row - readPixel() gets (usually) white trash
    // roughly 0.5% of the time.
    uint16_t r = d.readPixel(x, 61);


    //    // Instead take the majority of three reads.
    //    for (int i=0;i<2;++i) {
    //      uint16_t r2 = d.readPixel(x, 61);
    //      if (r == r2) break;
    //      r = r2;
    //    }


    for (int i = 0; i < 60; ++i) buf[i] = r;
    d.writeRect(x, 180, 1, 60, buf);


    // Save these to memory in case Serial interferes somehow.
    good[x] = c;
    bad[x] = r;
  }


  // Error log. There should't be any messages unless there are errors.
  for (int x = 0; x < 256; ++x) {
    if (good[x] != bad[x]) {
      Serial.print(x, HEX);
      Serial.print(" ");
      Serial.print(good[x], HEX);
      Serial.print(" ");
      Serial.print(d.readPixel(x, 0), HEX);
      Serial.print(" ");
      // These values should match the good column. They're not even deterministic!
      Serial.print(bad[x], HEX);
      Serial.print(" ");
      // The Hamming distance is usually 1 or 2, but occasionally as large as 8.
      Serial.println(hamming_distance(good[x], bad[x]));
    }
  }
}


void setup() {
  // Set a couple of outputs for flagging extraneous SPI::endTransaction() calls.
  pinMode(17, OUTPUT);
  pinMode(18, OUTPUT);
  digitalWriteFast(18, 0);
  digitalWriteFast(17, 0);
  
  // Set up the searial console for error logging.
  Serial.begin(9600);
  uint32_t count = 10000;
  while (!Serial && count--) delay(1);
  Serial.println("Read Pixel Test");
  
  d.begin();
  d.setRotation(1);
  readdiag();
  d.fillScreen(ILI9341_BLACK);
}


void loop() {
  draw();
  readdiag();
  printdiag();
}


// In ILI9341_t3::readRect, adding a 4us delay before waitFifoEmpty() and eliminating 
// the dummy read makes it function correctly 72Mhz and 24Mhz, optimized or not.
//    Display Power Mode: 0x9C
//    MADCTL Mode: 0x28
//    Pixel Format: 0x5
//    Image Format: 0x0
//    Self Diagnostic: 0x0
//
// However if we set the Teensy clock to another value, the display and the diagnostics get 
// messed up. The display shows random errors in row 4. The diagnostics report:
//    96MHz or 48MHz optimized or non-optimized
//    Display Power Mode: 0xDE
//    MADCTL Mode: 0x3C
//    Pixel Format: 0x7
//    Image Format: 0x0
//    Self Diagnostic: 0xE0


// No extra SPI.endTransaction() calls were detected. No extra SPI.beginTransaction() 
// calls were inferred because no deadlock was observed.
//
// Triplicating the readPixel() call significantly reduced the appearance of errors.
// Discarding the first read doesn't appear to help.

The reason I think this bug is important is that some sort of readback function would be very handy for implementing a mouse cursor and because the display should either work or be documented as not working at all supported CPU frequencies. I've seen the problems on the Teensy 3.0 as well, but haven't repeated all my recent tests on the 3.0. (The ILI9431_t3 library has not been ported to the LC or AVR processors.)

Here's a picture showing the bug. Note: despite the appearance, it's not a simple quantization error.
readpixel1.jpg

If we get root cause on this and extend the fix to readRect(), I'll be happy to submit a pull request or contribute to someone else's.
 
Back
Top