adafruit / Adafruit_ILI9341

Library for Adafruit ILI9341 displays
406 stars 281 forks source link

driver slow on (at least) ESP32 due to SPIClass::transfer() #19

Closed elFallino closed 7 years ago

elFallino commented 7 years ago

This driver works on ESP32 in software SPI and also hardware SPI mode, but is very slow.

This drivers method "spiwrite" calls SPI.transfer(uint8_t) witch is declared within the SPI driver. SPIClass::transfer() writes this byte to the SPI bus and reads it afterwards for returning. Since the returned result of re-reading the previously written byte is not used, it can be optimized away and speeds up things a lot.

Also the SPI driver of (at least) the ESP32 does also support sending 16 or 32 bit at once, there should be more optimizing possible.

ladyada commented 7 years ago

plz use bitbang SPI, its very fast! the library has to be rewritten significantly to avoid the mutex delay on esp32

elFallino commented 7 years ago

HAL mutexes can be disabled. See this commit: https://github.com/espressif/arduino-esp32/commit/c30012ab78c9a3e25110e0b868cdc39fd0b62419 I am not sure what would be the good way to do so but I chose the most wrongest way to do so by adding the neccessary C macro definition to esp32_hal_spi.c (#define CONFIG_DISABLE_HAL_LOCKS 1)

One additional tweak (I guess) can be to replace SPI.transfer(c); with SPI.write(c); in function spiwrite() as your driver does not do anything with the return value of transfer(). I did so and it should reduce timings about another 50% (didn't compare it).

In order to get HW Spi working I had to change some minor things. I am sure they can be done (they really need to be) better: commented out //SPI.beginTransaction(SPISettings(24000000, MSBFIRST, SPI_MODE0)); commented out // SPI.endTransaction();

added SPI.setFrequency(40000000); to constructor

replaced SPI.begin(); with SPI.begin(14, 12, 13, 32); (not sure if neccessary if I would have used the "hardware SPI" reset pin but might be good to pass the pin definition from constructor here.

Here the results: driver using Software SPI Benchmark Time (microseconds) Screen fill 3703660 Text 193965 Lines 1853981 Horiz/Vert Lines 301497 Rectangles (outline) 191813 Rectangles (filled) 4275616062 Circles (filled) 1082975 Circles (outline) 808785 Triangles (outline) 589072 Triangles (filled) 2497047 Rounded rects (outline) 379886 Rounded rects (filled) 8367935 Done!

anything unchanged but forcing hardware SPI Benchmark Time (microseconds) Screen fill 16546095 Text 856636 Lines 8284810 Horiz/Vert Lines 1353905 Rectangles (outline) 4268987500 Rectangles (filled) 7506881 Circles (filled) 4899979 Circles (outline) 3622163 Triangles (outline) 2627602 Triangles (filled) 11218099 Rounded rects (outline) 4269814054 Rounded rects (filled) 10559514 Done!

HAL locks disabled Benchmark Time (microseconds) Screen fill 662772 Text 51971 Lines 511414 Horiz/Vert Lines 55201 Rectangles (outline) 35831 Rectangles (filled) 1376127 Circles (filled) 235643 Circles (outline) 223025 Triangles (outline) 162235 Triangles (filled) 470071 Rounded rects (outline) 91688 Rounded rects (filled) 1513809 Done!

I hope you can understand my poor description. If not, please let me know and I will try my best to do it better ;)

marcmerlin commented 7 years ago

https://github.com/espressif/Adafruit_ILI9341 and https://github.com/espressif/Adafruit-GFX-Library are now blazing fast (good job @me-no-dev) Benchmark Time (microseconds) Screen fill 195235 (4x faster) Text 23768 (3x faster) Lines 214770 (2.8x faster) Horiz/Vert Lines 17631 (4.5x faster) Rectangles (outline) 11486 (20x faster?) Rectangles (filled) 405619 (2x faster) Circles (filled) 89422 (4x faster) Circles (outline) 94894 (2.5x faster) Triangles (outline) 47575 (5x faster) Triangles (filled) 150413 (4.5x faster) Rounded rects (outline) 35869 (4x faster) Rounded rects (filled) 452130

I hope this can be merged in for all to benefit

ladyada commented 7 years ago

ok - it will need to be tested on a large number of platforms but if they submit a pull req i can start the process! :)

me-no-dev commented 7 years ago

@ladyada the purpose of the forks are to me PRed to you :) We do not need another fork to confuse people. I have tested both libs on the platforms that I have here and tested in all cross combinations with your libs (your GFX + out ILI so on). Main difference and what is "improved" in GFX is implementing transactions based on draw rather than pixel, which coupled with some pin change optimizations (change DC only when writing CMD, CS once per transaction) makes up for much less time.

Here will follow some graphs that I made from the results.

Probably the most proving one: Original libs vs new libs on AVR SPI screen shot 2017-02-16 at 3 37 22 pm

ESP32 SPI Old vs New: screen shot 2017-02-16 at 2 48 44 pm

Some platforms tested: screen shot 2017-02-16 at 3 49 00 pm

ladyada commented 7 years ago

will start testing today using feathers, so will probably be able to merge soon!

me-no-dev commented 7 years ago

@ladyada I would like to ask a question about character drawing. Is it better on some screens to draw rectangles instead of multiple pixels? I am asking because I have written a method for printing chars to the screen that write pixels instead of rectangles when size is > 1, and at least on ILI9341 it saves many calls to setAddressWindow, instead it's called just once

void Adafruit_GFX::drawChar(int16_t x, int16_t y, unsigned char c,
        uint16_t color, uint16_t bg, uint8_t size){
    uint8_t row, column, vs, hs, ve;
    uint16_t col;

    if(!size){
        return;
    }

    startWrite();
    setAddrWindow(x, y, 6*size, 8*size);
    for(row=0;row<8;row++){
        for(vs=0;vs<size;vs++){
            // 5 columns of char data
            for(column=0;column<5;column++){
                col = (pgm_read_byte(font+(c*5)+column)&(1<<row))?color:bg;
                for(hs=0;hs<size;hs++){
                    writePixel(col);
                }
            }
            // 6th column is space
            for(ve=0;ve<size;ve++){
                writePixel(bg);
            }
        }
    }
    endWrite();
}

I have not compared speeds though.

me-no-dev commented 7 years ago

moving drawChar to virtual will let the ili driver overwrite the method.

ladyada commented 7 years ago

its been a while but you have to set the address window for pixels anyways?

btw, i will wait to test until you have a PR, in case there are other changes!

me-no-dev commented 7 years ago

with the method above, you basically write the color values to SPI one after another, after setting the address window for the whole character. In the original method, pixels/rectangles are written at locations, which triggers setAddressWindow for each. It can make sense on some screens that the GFX lib supports, so I rather make it virtual and overwrite it in the ILI library. Will test and report. If all good, will PR as I see nothing else to add.

me-no-dev commented 7 years ago

https://github.com/adafruit/Adafruit_ILI9341/pull/25 https://github.com/adafruit/Adafruit-GFX-Library/pull/107 PRs made :) best if you look at the resulting code as git diffs do not look very nice and at least on the ILI driver, changes are massive.

me-no-dev commented 7 years ago

btw my drawChar thing could not work with the API because it can not "skip" pixels (if color == bg)

marcmerlin commented 7 years ago

This is now fixed by https://github.com/adafruit/Adafruit_ILI9341/commit/7703b623235b3e63aac3e00b2a1655ffbca28580 Correct? If so, can you close this issue?

ladyada commented 7 years ago

oh yah