Fix issue #709. Keep SPI_WR_BYTE_ORDER and SPI_RD_BYTE_ORDER when upd…

[mtavenrath]

…ating the SPI_USER reg

[harry-boe]

good find thx.

The same is true for the SPI Clock Phase (CPHA) they go into the same register.
The idea of clearing the register was to create a controlled state.

Could you take a look if we have more overwrites on the SPI_USER register ?

[mtavenrath]

I've added SPI_CK_OUT_EDGE to the preserved bits. There're no other overwrites of the SPI_USER register.

[harry-boe]

I'm right now porting the SPI lib to RTOS

The proposed changes brakes the SDCard test cases -> so pls. give me some time to investigate what's going wrong here

[harry-boe]

Ok here is what i did

1st a save away the "initial" user-resiter value after applying the SPI settings in
void SPIClass::prepare(SPISettings mySettings)

    _SPISettings = mySettings;
    mySettings._user_regvalue = SPI_USER(SPI_NO);
    _init = true;

SPISettings is a friend class therefore i can write straight to the member vars

2nd

I restore the save setting before preparing the SPI transactions -> where you discovered the bug

        // restore inital SPI_USER register
        uint32_t regvalue = _SPISettings._user_regvalue;

This should do the trick. However as all my test cases use standard setting (i.e register value = 0) i cannot verify if it works for your application. Can you try to verify if it works for you.

/h

[mtavenrath]

Can you send me a diff of your change? Based on your description you rely on some bits set in SPI_USER at the time of beginTransaction.

Do you use the SPI lib for the SD card transfers? I'm wondering if this would be more accurate.

uint32 SPIClass::transfer32(uint32 data, uint8 bits) { uint32_t oldRegValue = READ_PERI_REG(SPI_USER(SPI_NO)); uint32_t regvalue = oldRegValue & (SPI_WR_BYTE_ORDER | SPI_RD_BYTE_ORDER | SPI_CK_OUT_EDGE); ... WRITE_PERI_REG(SPI_USER(SPI_NO), oldRegValue); }

I'm wondering if all the values needs to be set on each transfer call. Shouldn't it be enough to set them once in beginTransaction and restore the value in endTransaction?

[harry-boe]

i try to answer all your questions ..

• i don't have a diff right now but i just finished the port on SmingRTOS, Maybe you can take a look over there
• You are right about the begin transaction. From the Arduino API the idea is to support multiple SPI devices even if they require different byte orders, speed or SPI setting. So you should do a begin transaction with the required SPISettings before you do a transaction.
  However most libraries do not use transactions at all (in that case the lib uses the default speed and settings)
• About the SD card. The SD card is in fact my main test case. SD cards have a SPI slave controller build in and the interface is basically pure SPI (there are other modes as well). The SD card is currently the only Device within the Sming project that really uses bigger SPI data blocks. All the other devices do simple byte by byte transfers.
  (Comment: the initial SW SPI implementation was extracted from the SDCard implementation over here http://elm-chan.org/docs/mmc/mmc_e.html. it's a excellent lib and information sources about SPI)
• last one about the need to set the SPI_USER for each transaction.
  You are right. There is no need with the current feature set (the transfer and transfer32 methods). They use the same settings. The idea to have a controlled state at the begin of a transaction i more forward thinking. The api could be extended to add methods that make use of SPI commands, addresses or dummy bits. This is all supported by the HW and in that case it needs to be set correct - even than when a lib is not using transactions
• Hope this helps

[mtavenrath]

I'll take a look at the SmingRTOS changes.

I'm currently writing an driver for ST7735S devices which run pretty efficient on ESP8266 devices.

I.e. to fill the screen with a color this pattern is >10x faster than the adafruit library

beginUpstream(numberOfBits) setUpstreamBits(); // W0..Wn needs to be set only once when not fetching MISO data for (i = 0;i < count;ii+) upstream(); endUpstream();
In this case upstream() will wait for the SPI_USR bit to be unset and sets it again. endUpstream() will have to wait for SPI_USR to be unset too.

A separate upstream is required to ensure that the data is in W0...Wn does not get overwritten if it's required to fill a value. This was also the case where I discovered the MSB bug ;-)

In combination will double buffering, esp8266 supports splitting the registers into two halves), one can even compute data (i.e character blits) while uploading.

I've read that ESP8266 also has SPI interrupts. In theory one could utilize those to do async reading/writing giving the processor more time to do real work.

I'm also wondering if one could scanout a 1/2/4/8 bit paletted framebuffer at a 60hz frequency using the SPI interrupts. 40Mhz SPI requires 8ms per frame (measured), 80Mhz SPI might be able to do the same thing in 4ms.

It would be nice to have hw support for cmds and CS too to reduce CPU cost when sending large amounts of data. This might be useful for character blitting if there's no framebuffer on the CPU. ST7725 doesn't have a mode where you can read and send the framebuffer at the same time.So one would have to read one character, modify it and write it resulting in 3-6 commands and even more command/data toggles.

[harry-boe]

What is the difference between the ST7735 and the ST3755S (not sure if the FrameMemory is a ST7735S unique feature)

However - The Adafruit libraries are so tos peek "very pragmatic". They are not optimised for speed. Filling a screen means it sends pixel by pixel first the command as a SPI transaction, than the lower Byte and than the higher Byte -> so 3 SPI TX for a single byte multiplied by the number of pixels. That a lot of effort not one for the transmission but as well to prepare the TX in the library.
In short, there is a lot of room for performance tuning. Check out that blog over there about a tuned Arduino driver for the slightly bigger ILI9341 displays http://marekburiak.github.io/ILI9341_due/

On the ESP HW SPI, there are tons of options on how to configure and use the HW SPI bus. Including splitting registers (i.e memory segments) doing computations while transferring date etc.
Unfortunately a lot of those features are not really documented. So it's a lot of try and error with the Logic Analyzer (maybe you know my observation is made and documented on #618 ).
Most important finding is that you need to be careful with the Slave Signal especially when you do handovers from the HW control to the program logic.
There current HW implementation does not go back to the program logic until it has finished the transaction (it waits until all data got received - even if there was only a write instruction).
This mode is set by setting BIT= of the user register (btw: the SPI_DOUTIN definition is not anymore included in the api_registers.h shipped by Espressiv ?! (so much about documentation).

// from previous sdk's - no longer included in spi_register.h
#define SPI_DOUTDIN         (BIT(0))        // SPI_USER register

About the interrupts - looking at the Espressif IoT demo it looks like the interrupt are only supported when the SPI buss is in slave mode -> basically triggering a callback when data comes in. So i'm not sure for you display case if that is of any help.

Bottom line - you are very welcome to write a faster ST7735 display driver. Even better than some ideas can be ported to other TFT displays.
To be efficient you likely what to extend the HW SPI implementation to make use of commands and addresses. (the blond implementation from metalphreak is a really good read for that.)

I'm happy to support you where I can, but there is some very limited time I can spend on that.

Many THX, can't wait for a lightning fast TFT Display ;)

[harry-boe]

Last remark.

Please consider to move your efforts over to SmingRTOS.
Is still says bata but it's stable enough and now includes the same SPI implementation.

SmingNONOS might become legacy soon and new development is manly done for RTOS

[hreintke]

@harry-boe :
Is this PR OK to be merged ?

[mtavenrath]

@hreintke

I had no time yet to compare my change with the one from @harry-boe in SmingRTOS. I'd prefer to compare the changes first to ensure that nothing breaks.

[mtavenrath]

@harry-boe

With regards to the SPI interrupts I found this document:
https://espressif.com/sites/default/files/documentation/8j-esp8266_spi-wifi_passthrough_2-interrupt_mode_en_v1.0.pdf

It states:

When the master successfully reads data from or writes data into the slave, the interrupt will be triggered
This might mean that the interrupts can be triggered in master mode too.

[harry-boe]

@hreintke

I would go for the fix i did on RTOS. Using the preserved register values hold in SPISettings

any opinions before i prepare a PR

[hreintke]

@harry-boe :
I agree with using the same fix as in RTOS, keeps them as identical as possibe..
Just prepare the PR and I will merge.

[mtavenrath]

@harry-boe The fix you did in RTOS didn't work most likely because you've been using SPI_USER(SPI_NO) as value instead of READ_PERI_REG(SPI_USER(SPI_NO)); I haven't checked if fixing this will work, but I can do later.

I'm wondering shouldn't beginTransaction preserve the register value and endTransaction restore it? Shouldn't beginTransaction set the register value to a predefined value based on the SPISettings? Should the bits really change if begin/endTransaction is not being called?

[harry-boe]

Or that sounds reasonable. Unfortunately all my test devices use default settings (which basically set's the initial register to zero)

Can you prepare a PR with the register reserved in SPISettings as in RTOS and with the correct initialisation using READ_PERI_REG(SPI_USER(SPI_NO));

Many Thanks

[slaff]

@mtavenrath Is this PR still relevant / working as expected? If yes, we will merge it as part of the coming release.

[mtavenrath]

@slaff I don't have a lot of devices to test. It fixes the problems I had with ST7735 transfers when doing burst transfers of 32 to 512 bit.

[slaff]

@harry-boe Can you check this PR on your devices with the SDCard sample and tell me if it is working for you?

[harry-boe]

i'm still stuck with my toolchain after the OSX Sierra update. (can compile now but have issues with the linker scripts)
Maybe someone else can jump in here. Most of the LCD Display (e.g. the ST7735) have SD card readers attached to them. so testing should be easy.

On the other hand, the PR should not impact the SD card library from verifying the code. However i have not tested it!