KurtE
commented
6 years ago Secondary Notes:
I thought I would try building the Adafruit_Ili9341 library for the OpenCR.
I first updated my installed version of Adafruits library. It did not compile as it was trying to define a port and mask to do quick pin IO and tries to define volatile RwReg *mosiport, ... Which does not exist. They already handle it to use digitalwrite for their ARDUINO_STM32_FEATHER case. So fixed this part by adding to their #define
#if defined (ARDUINO_STM32_FEATHER) || defined(__OPENCR__) // doesnt work on wiced feather or Robotis __OPENCR__
#undef USE_FAST_PINIO
#elif defined (__AVR__) || defined(TEENSYDUINO) || defined(ESP8266) || defined (ESP32) || defined(__arm__)
#define USE_FAST_PINIO
#endifI added the OPENCR test, which got it to mostly build. However it has an undefined export from GFX. This is because your version of GFX is something like 1.1.5, where current one is: 1.2.3.
It uses your version as you specified the architecture which guess overrides architecture=* in the library.properties file.
Couple obvious ways to fix... I just removed your copy of GFX and it builds. Other options, include update your copy to a more current version.
I have not tried running it yet. But if this is something interesting, probably should see if you can get Adafruit to take a simple Pull request.
Sorry I know at this scale does not show anything, except, looking at the sequence it looks like it has the proper output for a screen fill. Setting the horizontal/vertical ranges and then memory fill, followed by lots of writes of 0 (black)... Did not count the 320x240x2 bytes output... But it then properly shows that the CS is released, which is probably the last thing left before the functions should have returned from the fillScreen. Maybe memory corruption? maybe timing issue? to long in these outputs Will debug some more. Also maybe will do a little more editing of the Adafruit code to maybe say that we support 16 bit outputs... 
The SPI data is only taking up about 20% of the time... 
OpusK
Not sure if, best to do this as issue? Or discussion some place, but was wondering about the SPI support, so took a quick look through SPI implementation and have a few suggestions.
I might later try another Pull Request, but currently don't have enough information.
I noticed that you do have beginTransaction and endTransaction defined in the library but you have not told the world that you have it.
That is I believe most all SPI libraries that support transactions have:
You will find there are a reasonable number of libraries that test for this and then use the begin/end stuff.
Obviously would be easy to do simple PR to do this.
However, I believe that this should probably be enhanced some before doing so. That is currently your beginTransaction, is simply ALWAYS calling off to set everything.
Instead I believe it should at a minimum do something like: if new settings do not equal current settings, than do all of the init: Example Teensy
Again this checks the generated CTAR register value (Teensy hardware specific) and only when necessary update it... Could maybe do somethings similar for here, but I don't know your hardware... Is there hardware manual showing some of this?
There is also stuff about the clock handling: Right now you use the setClockDivider(settings.clockDiv);
So you currently take the desired clock rate, example 4000000 and in the function:
SPISettings::init_AlwaysInline you have code that divides the desired speed by some fixed dividers like 2, 4, 8, 16... and return some value like SPI_CLOCK_DIV4... (as 50000000/2 = 25000000 which > 2000000 Now as hopefully always inline will remove all of #if for constants being passed in... But maybe not. On Teensy the code checks for built in constants before doing the stuff always inline.
''' SPISettings(uint32_t clock, uint8_t bitOrder, uint8_t dataMode) { if (__builtin_constant_p(clock)) { init_AlwaysInline(clock, bitOrder, dataMode); } else { init_MightInline(clock, bitOrder, dataMode); } } '''
Then when you do the begin transaction the call to setClockDivider(SPI_CLOCK_DIV4) will then do case statement on the value and set... _hspi->Init.BaudRatePrescaler = SPI_BAUDRATEPRESCALER_4 Where #define SPI_BAUDRATEPRESCALER_4 ((uint32_t)0x00000008)
At a minimum I would believe that the SpiSettings object could/should hold onto the generated pre-scaler value and set it directly and not need the indirection. But next question might be are these the only valid values for SPI speed (25mhz, 12.5, 6.25...?)
Again I only took a quick look through here. If I actually were to try more of it out, I would probably have additional questions like: Is there anything speed wise between your normal SPI where you are in handshake mode for each byte/word transferred, That is void SPIClass::transfer(void *buf, size_t count) still just does a for loop transferring each item one at a time.
With many of the Teensy boards there is a SPI FIFO queue for RX and TX, so that you can keep the SPI busy. With the lower end (LC), it does not have FIFO but does support double buffering.
Does this processor support something like this?
DMA: I see there is a quick transfer, transferFast which I believe is only a write operation using DMA?
Again with the Teensy we added some more dma support... https://github.com/PaulStoffregen/SPI
Again not sure what level of support you would like here? But thought I would at least document a few things I noticed.
Maybe if I get a chance I might try doing a few changes and see about supporting some other display like the ili9341. It would be curious to see the speed. I would probably start with the Adafruit library, but it would be nice to get faster speed, like we have on the Teensy using the ili9341_t3 library and it's derivatives. I know there are a few DMA versions of this for some platforms.
Kurt