RudolphRiedel / FT800-FT813

Multi-Platform C code Library for EVE graphics controllers from FTDI / Bridgetek (FT810, FT811, FT812, FT813, BT815, BT816, BT817, BT818)
MIT License
129 stars 58 forks source link

Double preprocessor variable for power pin #136

Open Jwf68 opened 4 weeks ago

Jwf68 commented 4 weeks ago

I am trying to use the libary for an STM32 MCU. After stepping the SPI initialization code, I discovered that there are two preprocessor variables for the power pin. EVE_PD_PORT_NUM=4U mening GPIOD EVE_PDN_PORT_NUM=4U mening GPIOD This appears to be wrong.

RudolphRiedel commented 4 weeks ago

Thank you, I just fixed it. That was supposed to be EVE_PD_PORT_NUM in EVE_target.c as well.

I left behind STM32 a bit frustrated back then, after I could not figure out how to to use the updated DMA unit of the STM32H7.

Jwf68 commented 4 weeks ago

A different problem that I have. STM32 example load image is stuck in a loop, EVE_busy() does not return zero. Would you consider helping via remote meeting with Teams? This is a commercial project, so we can pay for support.

RudolphRiedel commented 4 weeks ago

I am making it a (maybe bad) habbit of not getting paid for support here. And I have never used teams on my personal Windows machine, I am not even sure that it can be working since I am trying to avoid using a microsoft account. Apart from that, don't trust my STM32 skills, I never used this for anything beyond tinkering with SPI. :)

That said, which STM32 are we even talking about? There is a small chance, but maybe I have an eval board that at least is close enough.

Not returning from a load image sounds like an issue with the image though, if you can get up to that point everything else must be working already. What are the specs of that image, format and binary size?

RudolphRiedel commented 4 weeks ago

Ok, I just dusted off my STM32F407 Discovery and copied over the latest library files to the STM32CubeIDE project I was using. It still builds and it still runs, including one: EVE_cmd_loadimage(MEM_PIC1, EVE_OPT_NODL, pic, sizeof(pic));

But that image is a .jpg and only 3391 bytes in length.

Edit: I took Ducati_side_shadow.png from the EAB "Image Utility Test" folder. It is 75628 bytes and is 288x163 pixels. Putting this line first: EVE_cmd_loadimage(0, EVE_OPT_NODL, ducati, sizeof(ducati)); The image is loaded fine and I am displaying it right now.

And I put it first because cmd_loadimage is using the top 42kiB of memory to decode .png files.

Jwf68 commented 4 weeks ago

I am using a STM32F405RGT6 bare bone no operating system. Display with FT812 https://www.mouser.se/datasheet/2/880/DS_ME812A-WH50R-1154864.pdf I have an C++ program that does hal initialization but the SPI and power pin config is your code. I have got https://github.com/Bridgetek/EVE-MCU-BRT_AN_025 to run this way, so the hardware is ok.

I have modified your main.c from https://github.com/RudolphRiedel/FT800-FT813/tree/5.x/examples/EVE_Test_STM32_RiTFT50_PlatformIO/src so that I can call it from my C++ code.

In tft.c the program never returns from: EVE_cmd_loadimage(MEM_PIC1, EVE_OPT_NODL, pic, sizeof(pic));

In EVE_commands.c uint8_t EVE_busy(void) Never returns zero. Return variable ret is set to 13 in: ret = EVE_FIFO_HALF_EMPTY;

Any Idea what the problem is?

I can not use the preprocessor variable USE_HAL_DRIVER since my application does not compile, if it is defined.

Settings: Not possible gives compile errors USE_HAL_DRIVER STM32F4 EVE_ME812A EVE_SPI_NUM=3U EVE_SPI_PORT_NUM=3U EVE_SPI_GPIO_ALT_FUNCTION=GPIO_AF6_SPI3 EVE_SCK=GPIO_PIN_10 EVE_MISO=GPIO_PIN_11 EVE_MOSI=GPIO_PIN_12 EVE_SPI_PRESCALER=SPI_BAUDRATEPRESCALER_128 EVE_CS_PORT_NUM=1U EVE_CS=GPIO_PIN_15 EVE_PDN_PORT_NUM=4U EVE_PD_PORT_NUM=4U EVE_PD=GPIO_PIN_2 STM32F405xx EVE_DMA

RudolphRiedel commented 4 weeks ago

Ah interesting, the F407 that I am using is practically the same. And my settings are very similar, except for: EVE_SPI_PRESCALER=SPI_BAUDRATEPRESCALER_128

That seems to be rather low for the SPI clock. I am using EVE_SPI_PRESCALER=SPI_BAUDRATEPRESCALER_4

I renamed USE_HAL_DRIVER to USE_HAL_DRIVER! and it had no effect at all. I am not sure why this was set.

But, I am using an EVE3-50G right now, so a BT815. And somehow my EVE2-50G with FT813 is not working anymore. Using the BT815 as if it were a FT813 should also work - but it does not.

Looks like I might have broken FT81x support somehow.

RudolphRiedel commented 4 weeks ago

Try this in EVE_commands.c: void EVE_cmdWrite(uint8_t const command, uint8_t const parameter) { EVE_cs_set(); spi_transmit(command); spi_transmit(parameter); spi_transmit(DUMMY_BYTE); EVE_cs_clear(); EVE_cs_clear(); // <- extra line }

I have no idea yet why this happens, but the first sequence I am seeing without this line for FT813 is this: grafik

But these are two commands so this is supposed to look like this: grafik

I have no idea how this is possible. If this would be an issue with the controller optimizing something away, it also should not work with the BT815.

if defined (EVE_HAS_CRYSTAL)

EVE_cmdWrite(EVE_CLKEXT, 0U); /* setup EVE for external clock */

else

EVE_cmdWrite(EVE_CLKINT, 0U); /* setup EVE for internal clock */

endif

if EVE_GEN > 2

EVE_cmdWrite(EVE_CLKSEL, 0x86U); /* set clock to 72 MHz */

endif

EVE_cmdWrite(EVE_ACTIVE, 0U); /* start EVE */

So in the two cases I am looking at, it either is EVE_cmdWrite(EVE_CLKINT, 0U); / setup EVE for internal clock / EVE_cmdWrite(EVE_ACTIVE, 0U); / start EVE / or EVE_cmdWrite(EVE_CLKEXT, 0U); / setup EVE for external clock / EVE_cmdWrite(EVE_CLKSEL, 0x86U); / set clock to 72 MHz / EVE_cmdWrite(EVE_ACTIVE, 0U); / start EVE /

Why the heck is the first not working properly, but the second is?

And using EVE_cmdWrite(EVE_CLKINT, 0U); / setup EVE for internal clock / EVE_cmdWrite(EVE_ACTIVE, 0U); / start EVE / EVE_cmdWrite(EVE_ACTIVE, 0U); / start EVE / Also works: grafik

RudolphRiedel commented 4 weeks ago

Adding a NOP in EVE_cs_clear() also works: static inline void EVE_cs_clear(void) { while (LL_SPI_IsActiveFlag_BSY(EVE_SPI)) {} LL_GPIO_SetOutputPin(EVE_CS_PORT, EVE_CS); asm volatile ("nop"); // test! }

But why though? With the NOP and two commands CS is only high for 40ns. Without the NOP and three commands CS is high for 80ns.

Is this some weird cache issue with the STM32F40x?

Jwf68 commented 4 weeks ago

You have used a mix of two STM32 libarys, HAL library and LL library. Atached files only use the HAL library. EVE_MCU_STM32cube.zip Chip select NOP commented out in https://github.com/Bridgetek/EVE-MCU-BRT_AN_025/blob/main/ports/eve_arch_stm32/EVE_MCU_STM32.c

RudolphRrr commented 4 weeks ago

You have used a mix of two STM32 libarys, HAL library and LL library.

Yes, this is by design, the HAL library is significantly slower. Well, this applies to the SPI transfers and checking of the flags, I guess setting the I/Os might not be that bad.

That might also explain why you have issues with the HAL settings, I setup my project to use LL.

Atached files only use the HAL library.

Thank you, I'll play with this later.

Jwf68 commented 4 weeks ago

"That might also explain why you have issues with the HAL settings, I setup my project to use LL." In https://github.com/RudolphRiedel/FT800-FT813/blob/5.x/examples/EVE_Test_STM32_RiTFT50_PlatformIO/src/main.c I have found HAL_Init(). HAL_Init() is called in C++ app main method.
What do you do to init the LL library? All that I have done is copying stm32f4xx_ll_spi c and h. We are using GCC and Visual sudio with visualGDB. Cand you please chare your complete STM32CubeIDE project?

Jwf68 commented 4 weeks ago

From https://github.com/RudolphRiedel/FT800-FT813/blob/5.x/src/EVE_target.c / SPIx GPIO Configuration: / gpio_init.Pin = EVE_SCK|EVE_MOSI|EVE_MISO; gpio_init.Mode = GPIO_MODE_AF_PP; gpio_init.Pull = GPIO_NOPULL; gpio_init.Speed = GPIO_SPEED_FREQ_HIGH; gpio_init.Alternate = EVE_SPI_GPIO_ALT_FUNCTION; HAL_GPIO_Init(EVE_SPI_PORT, &gpio_init);

I generated code for the low leval hal using the STM32CubeMX:
GPIO_InitStruct.Pin = LL_GPIO_PIN_10|LL_GPIO_PIN_11|LL_GPIO_PIN_12; GPIO_InitStruct.Mode = LL_GPIO_MODE_ALTERNATE; GPIO_InitStruct.Speed = LL_GPIO_SPEED_FREQ_VERY_HIGH; GPIO_InitStruct.OutputType = LL_GPIO_OUTPUT_PUSHPULL; GPIO_InitStruct.Pull = LL_GPIO_PULL_NO; GPIO_InitStruct.Alternate = LL_GPIO_AF_6; LL_GPIO_Init(GPIOC, &GPIO_InitStruct);

You are using spi low level liabary, but init is done using HAL libary. Can this realy work?

RudolphRiedel commented 4 weeks ago

Ok, I tried this now: static inline void EVE_cs_clear(void) { while (LL_SPI_IsActiveFlag_BSY(EVE_SPI)) {} // LL_GPIO_SetOutputPin(EVE_CS_PORT, EVE_CS); HAL_GPIO_WritePin(EVE_CS_PORT, EVE_CS, GPIO_PIN_SET); }

static inline void EVE_cs_set(void) { // LL_GPIO_ResetOutputPin(EVE_CS_PORT, EVE_CS); HAL_GPIO_WritePin(EVE_CS_PORT, EVE_CS, GPIO_PIN_RESET); }

And the impact is measureable, but not at all significant. The host commands are a few % slower and all the other non-DMA commands execute a little slower as well.

For example, the three read commands I am using in my touch function now need 300ns to 400ns longer with HAL than with LL, that is not enough to even try and measure it more acurately.

Well, it works this way while using LL should be working, but somehow does not work reliably as if a call is optimized away but not always... As using LL is practically using direct register access and the register are volatile, this should never be "optimized" - yet here we are.

One might think that the issue could be that the core is so fast that the IO block has no time to actually carry out the level change before the register is written again. But that does not explain why calling the function two times does not work and when calling it three times it works.

You are using spi low level liabary, but init is done using HAL libary. Can this realy work?

Yes, absolutely. First of all I was wrong earlier, I did not "setup my project to use LL". There is nothing to set up there, LL does not require initialization, at least not of the library itself.

__STATIC_INLINE void LL_GPIO_SetOutputPin(GPIO_TypeDef *GPIOx, uint32_t PinMask) { WRITE_REG(GPIOx->BSRR, PinMask); }

This is just direct register access with a minimal abstraction so that it can work across most STM32 families as long as the correct include is loaded.

IOPorts are a bad example in this, these are too simple.

void HAL_GPIO_WritePin(GPIO_TypeDef GPIOx, uint16_t GPIO_Pin, GPIO_PinState PinState) { / Check the parameters */ assert_param(IS_GPIO_PIN(GPIO_Pin)); assert_param(IS_GPIO_PIN_ACTION(PinState));

if(PinState != GPIO_PIN_RESET) { GPIOx->BSRR = GPIO_Pin; } else { GPIOx->BSRR = (uint32_t)GPIO_Pin << 16U; } }

So also almost direct register access. But as a function that gets called and with an if statement, this needs a few extra clock cycles.

while (!LL_SPI_IsActiveFlag_TXE(EVE_SPI)) {}

This works across a lot of STM32 familes, wherever that bit actually is placed.

ST unfortunately broke with this for the STM32H7 for example: while (!LL_SPI_IsActiveFlag_TXP(EVE_SPI)) {}

Whatever the reason was behind renaming that bit for STM32H7 only...

Cand you please chare your complete STM32CubeIDE project?

Sure, here: EVE_Test_STM32F407.zip

Not that this is a real project, just something I tinkered with to implement STM32 support in my library. I am using the code generator for SPI and GPIO, but I am not even calling the resulting code, this is just for reference what it might should look like.

And I am using HAL for the initialization as this part is not time crititcal at all. It does not really matter how long HAL_SPI_Init() needs to jump thru whatever hoops deemed necessary, it only is called once.

But for example HAL_SPI_TransmitReceive() is 241 lines long for the STM32F4 and does a lot of things that really are not necessary to do over and over and over again. Like checking if the SPI unit is even active. Duh, I want to send several thousand bytes per minute over the SPI, not read three bytes from an ADC every now and then. :)

RudolphRiedel commented 4 weeks ago

grafik

This is the software, running on an EVE2-50G from Matrix Orbital, so the chip is a FT813.

Jwf68 commented 4 weeks ago

Thank you for the EVE_Test_STM32F407 projekt! I have an STM32F0DISCOVERY that I will try to modyfy and run the EVE_Test_STM32F407 projekt on. If that fails I will buy the same card as you have.

RudolphRiedel commented 4 weeks ago

Well, this is supposed to run on everything with any EVE display module, kind off. :-) The F405 you have is pratically the same, so much so that it is binary compatible if the extra modules of the F407 are not used. And the ME812A also is similar.

I do not have a ME812A, but the configuration is in EVE_config.h.

And if it is still not working I would like to see a logic analyzer trace.

Jwf68 commented 3 weeks ago

Hello Rudolph, My colleague modified EVE_target_STM32.h in order to get it working on a STM32F405RGT6 MCU, see attachments. diff_EVE_target_STM32.zip We do not use DMA at the moment. I have not tried your EVE_Test_STM32F407 project. I can't explain why it works on your EVE_Test_STM32F407, but not on our STM32F405RGT6. Except then possibly problems with mixing different libraries, and its related initialization.
Maybe STM32CubeIDE does something under the hood, but that's pure speculation.

Best regards Jonas Wahlfrid

RudolphRrr commented 3 weeks ago

Hi, do you share the SPI with other devices that use a different configuration?

Jwf68 commented 3 weeks ago

Hi, "do you share the SPI with other devices that use a different configuration?" No the only SPI is the FT812 chip.

RudolphRrr commented 3 weeks ago

Then I do not understand the issue as the low-level library does practically nothing beyond supplying an API for direct register access. The same registers with the exact same functionality that HAL is writing to / reading from with a lot more code.

Sure, there is nothing wrong with using HAL library, it only is slower as it executes not necessary code over and over again. I will later try your code and check what the impact is.

RudolphRiedel commented 3 weeks ago

Ok, I diffed your version, added the code, modified to make the variations swappable, then changed your version to remove the unnecessary lines and finally optimized spi_transmit_32() a bit further.

This is from my version, compiled and executed last: grafik grafik

This is from your version: grafik

And this is the slightly tweaked version of your variant: grafik

So even the tweaked version takes more than twice as long doing the same thing. And since the clockrate is the same, all the time can be seen in the periods with no activity on the SPI.

Is this still viable? Sure, if you are using DMA. The time for the non-DMA part of my example code went up from 18µs to 38µs, this is mostly the three SPI transfers.

Display refresh with DMA: 12µs Display refresh without DMA, my code: 196µs Display refresh without DMA, your code: 311µs

And the display list is very short, 260 bytes with DMA, about 32 bytes less without DMA due to less debug-output.

Oh yes, of course the whole initilization of the display also takes more time, including uploading assets to the display. But interestingly with the huge .png, the difference in time spend is almost the same.

This is my version: grafik

This is your variant: grafik

The huge block in the middle is the transfer of the 75kiB .png. The gaps in the chip-select that are way larger with your variant are the actually transfers in chunks of 3840 bytes. The rest in between is polling if the command co-processor is done with the data. So in this particular case my transfer time is shorter, but in return the wait is longer, interesting. :-)

Jwf68 commented 3 weeks ago

Hi Rudolph, For your information, our main and Clock. main.zip

/Jonas

Jwf68 commented 3 weeks ago

Hi Rudoiph! Could you please share your optimized version of my colleague's EVE_target_STM32?

/Jonas

RudolphRrr commented 3 weeks ago

Damn, I forgot to attach the archive that I prepared over all the fun with the logic analyzer.

Here, I edited it from scratch again, might not be 100% the same, but I have no access to the file right now. EVE_target_STM32.zip

Jwf68 commented 3 weeks ago

Hi Rudolph, Your optimized version of my colleague's EVE_target_STM32, works fine. Thank you! /Jonas

RudolphRrr commented 3 weeks ago

Well, "optimized", this is more clean up than optimization, using HAL for SPI transfers remains slow.

Jwf68 commented 2 weeks ago

Hi Rudolph, Minor details, I added: / use this to add a header file with your custom module configuration / / -DEVE_CUSTOM_MODULE_H='"..\mycfg.h"' /

if defined (EVE_CUSTOM_MODULE_H)

include EVE_CUSTOM_MODULE_H

endif

To EVE_target_STM32 and removed commented out code. EVE_target_STM32.zip

Jwf68 commented 2 weeks ago

Hi Rudolph! I have started to play with the EVE Screen Editor. I have noted that for example "CMD_BUTTON(76, 162, 120, 36, 27, 0, "Button")" need to be changed to "EVE_cmd_button(76, 162, 120, 36, 27, 0, "Button")". Why not just offer a method "CMDBUTTON(76, 162, 120, 36, 27, 0, "Button")" without EVE and all caps?

/Jonas

RudolphRrr commented 2 weeks ago

Hi,

Why not just offer a method "CMDBUTTON(76, 162, 120, 36, 27, 0, "Button")" without EVE and all caps?

Because the library is mostly for C, not C++ and there are no namespaces in C. Also all caps is usually used for macros only, so the EVE_ prefix is a compromise. And the macro CMD_BUTTON already is in use:

define CMD_BUTTON ((uint32_t) 0xFFFFFF0DUL)

And then, when you export it from ESE it becomes: Gpu_CoCmd_Button(phost, 293, 238, 120, 36, 27, 0, "Button"); (HAL 2.0)

And then there also is this: EVE_CMD_BUTTON() - EVE-MCU-BRT_AN_025 EVE_CoCmd_button() - EW2024 photobooth example

So the better question might be why there is no ESE exporter for my library and why ESE is not offering export for all of the Bridgtek formats, or also what the "official" C API is. :-)

And I started to tinker with an export script but never finished it.

Jwf68 commented 2 weeks ago

Good points!

RudolphRrr commented 2 weeks ago

I just went over my mails and just remembered what I wanted to write earlier.

Minor details, I added: / use this to add a header file with your custom module configuration / / -DEVE_CUSTOM_MODULE_H='"..\mycfg.h"' /

if defined (EVE_CUSTOM_MODULE_H)

include EVE_CUSTOM_MODULE_H

endif

To EVE_target_STM32

Why though? This already is in EVE_config.h and placed there to allow loading a custom set of display parameters without needing to modify EVE_config.h.

Jwf68 commented 2 weeks ago

If comment out the folowing in EVE_target_STM32.h: //#if defined (EVE_CUSTOM_MODULE_H) //#include EVE_CUSTOM_MODULE_H //#endif I get error: ../FT800-FT813-5/EVE_Test_STM32/src/../../src/EVE_target/EVE_target_STM32.h:143:2: error: #error "EVE_SPI_GPIO_ALT_FUNCTION must be defined in order to configure the SPI GPIO pins (e.g. -DEVE_SPI_GPIO_ALT_FUNCTION=GPIO_AF5_SPI4)"

RudolphRrr commented 2 weeks ago

So, then define it in your build environment? :-)

Ok, so you rather add an include file with the pin configuration than to add all the parameters to the build environment? Well, ok, why not, but EVE_CUSTOM_MODULE_H already is used by EVE_config.h.

Jwf68 commented 2 weeks ago

"Ok, so you rather add an include file with the pin configuration than to add all the parameters to the build environment?" Correct. "Well, ok, why not, but EVE_CUSTOM_MODULE_H already is used by EVE_config.h." I have one file, Defines.h where I put all the settings.

RudolphRrr commented 2 weeks ago

That is not my "issue", I suggest using a different macro to trigger including a file from the target headers, like EVE_CUSTOM_PINS_H for example. If your Defines.h has an include-guard you still could use one file, only triggered by two macros.

Jwf68 commented 2 weeks ago

Good idea!

RudolphRrr commented 2 weeks ago

Why are you using a custom module configuration though? You are not actually using a ME812A, it's configuration EVE_ME812A is not properly working or you needed to tweak the config?

Jwf68 commented 2 weeks ago

We have changed the touch calibration values for EVE_ME812A. We use the folowing in Defines.h:

define EVE_ME812A

define EVE_SPI_NUM 3U

define EVE_SPI_PORT_NUM 3U

define EVE_SPI_GPIO_ALT_FUNCTION GPIO_AF6_SPI3

define EVE_SCK GPIO_PIN_10

define EVE_MOSI GPIO_PIN_12

define EVE_MISO GPIO_PIN_11

define EVE_SPI_PRESCALER SPI_BAUDRATEPRESCALER_128

define EVE_CS_PORT_NUM 1U

define EVE_CS GPIO_PIN_15

define EVE_PD_PORT_NUM 4U

define EVE_PD GPIO_PIN_2

define EVE_DMA

define EVE_DMA_CHANNEL_NUM 0U

define EVE_DMA_STREAM_NUM 5U

define EVE_DMA_UNIT_NUM 1U

RudolphRrr commented 2 weeks ago

Ah ok, not a custom module, so EVE_CUSTOM_MODULE_H in EVE_config.h does not actually need to load this.

Jwf68 commented 2 weeks ago

My thought was that EVE_config.h should be informed that EVE_ME812A is defined. By including Defines.h via EVE_CUSTOM_MODULE_H. But I might have missed something.

RudolphRrr commented 2 weeks ago

And you are not wrong, EVE_config.h needs this. But on the application side you include "EVE.h" and it then includes EVE_target.h, EVE_config.h and EVE_commands.h.

Jwf68 commented 2 weeks ago

And EVE_target.h includes EVE_target_STM32. EVE_target_STM32 includes Defines.h via the use of EVE_CUSTOM_PINS_H. Excelent point!

Jwf68 commented 2 weeks ago

Hello Rudolph! My colleague thinks that this discussion should have been in the discussion section. He is of course right. I have appreciated the discussion. I hope you got something out of it all. I appreciate all the help, thank you! Best regards Jonas Wahlfrid

RudolphRiedel commented 2 weeks ago

Well, it started out with an issue. :-) I don't mind, I had a good reason to tinker with STM32 again and adopting EVE_CUSTOM_PINS_H seems to be a good idea, especially with the ton of macros I added to the STM32 targets. Oh yes, I also started to check out the ESE exporter again, but this is a bit obscure, perhaps an opportunity to brush up on Python.