8000 Fix giga display config. by iabdalkader · Pull Request #117 · arduino/ArduinoCore-zephyr · GitHub
[go: up one dir, main page]
Skip to content

Fix giga display config. #117

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 4 commits into
base: arduino
Choose a base branch
from
Open

Fix giga display config. #117

wants to merge 4 commits into from

Conversation

iabdalkader
Copy link

The SMH will not be enabled for the shield by default, so we need to enable it here. There's also a new option to enable SMH for the display (and configure its attributes) which is used here. This should be merged when zephyrproject-rtos/zephyr#89246 is merged and after an update.

Note this dc22eb9 can probably be reverted. The whole SDRAM can be used for SMH, unless some other drivers use SDRAM outside of SMH (in this case they should also be converted to use SMH).

@iabdalkader
loader/fixup: Fix SMH init priority.
5951714 
Probably a copy&paste error.
@iabdalkader
variants/arduino_giga_r1_stm32h747xx_m7: Enable SMH for display.
6d09592 
The LTDC will have its own SMH option, and the upstream shield
config will Not enable this by default.
@KurtE
Copy link
KurtE commented Apr 30, 2025

@iabdalkader - I tried bringing this into my current build and it fails to build the variant:

/home/kurte/git/ArduinoCore-zephyr/loader/../variants/arduino_giga_r1_stm32h747xx_m7/arduino_giga_r1_stm32h747xx_m7.conf:50: warning: attempt to assign the value '2' to the undefined symbol STM32_LTDC_FB_SMH_ATTRIBUTE
Parsing /home/kurte/git/zephyr/Kconfig

Now that I look at it, looks like it failed on the same line in the code checks. as well.

Is it maybe supposed to be: CONFIG_STM32_LTDC_FB_NUM ?
And allocating two frame buffers?

@iabdalkader
Copy link
Author

This should be merged when zephyrproject-rtos/zephyr#89246 is merged and after an update.

Also this PR may change.

@KurtE
Copy link
KurtE commented May 1, 2025

This should be merged when zephyrproject-rtos/zephyr#89246 is merged and after an update.

Also this PR may change.

Thanks.
Sorry slightly off topic. But do you know of any example zephyr sketch and/or ArduinoCore-zephyr sketch to experiment with the GIGA with the display shield, that does both display and touch?

I have tried to build a few different Zephyr ones like:

(.venv) D:\zephyrproject\zephyr>west build -p -b  arduino_giga_r1//m7 --shield giga_display_shield  samples\modules\lvgl\demos

But it fails to build.

D:/zephyrproject/zephyr/drivers/display/display_stm32_ltdc.c:422:25: error: 'CONFIG_VIDEO_BUFFER_SMH_ATTRIBUTE' undeclared (first use in this function)
  422 |                         CONFIG_VIDEO_BUFFER_SMH_ATTRIBUTE,
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Note: My zephyr branch I am using is one that I have changes to add the Teensy Micromod board, which I rebased a few days ago.

Thanks!

@iabdalkader
Copy link
Author

I don't know of any samples that do that, but I haven't tried many samples.

But it fails to build.

Yes, that's what I'm fixing in zephyrproject-rtos/zephyr#89246

@iabdalkader
Copy link
Author

I have tried to build a few different Zephyr ones like:

@KurtE zephyrproject-rtos/zephyr#89246 has been merged, so you should be able to build the upstream samples.

@KurtE
Copy link
KurtE commented May 7, 2025

@KurtE zephyrproject-rtos/zephyr#89246 has been merged, so you should be able to build the upstream samples.

Thanks I will try it.

Note: On THIS pr I believe you will also need to update boards.txt in the root directory.
It currently has for GIGA:

giga.build.zephyr_target=arduino_giga_r1//m7
giga.build.zephyr_args=--shield giga_display_shield

And I believe that last line needs to change to:
giga.build.zephyr_args=--shield arduino_giga_display_shield

@KurtE
Copy link
KurtE commented May 7, 2025

@KurtE zephyrproject-rtos/zephyr#89246 has been merged, so you should be able to build the upstream samples.

Thanks I will try it.

I tried it and now the build fails with the code not fitting...
west build -p -b arduino_giga_r1//m7 --shield arduino_giga_display_shield samples\modules\lvgl\demos
Error message:

[522/527] Linking C executable zephyr\zephyr_pre0.elf
FAILED: zephyr/zephyr_pre0.elf zephyr/zephyr_pre0.map D:/zephyrproject/zephyr/build/zephyr/zephyr_pre0.map
C:\WINDOWS\system32\cmd.exe /C "cd . && ccache D:\Users\kurte\Downloads\zephyr-sdk-0.16.8\arm-zephyr-eabi\bin\arm-zephyr-eabi-gcc.exe  -gdwarf-4 zephyr/CMakeFiles/zephyr_pre0.dir/misc/empty_file.c.obj -o zephyr\zephyr_pre0.elf  zephyr/CMakeFiles/offsets.dir/./D_/Users/kurte/zephyrproject/zephyr/arch/arm/core/offsets/offsets.c.obj  -T  zephyr/linker_zephyr_pre0.cmd  -Wl,-Map=D:/zephyrproject/zephyr/build/zephyr/zephyr_pre0.map  -Wl,--whole-archive  app/libapp.a  zephyr/libzephyr.a  zephyr/arch/common/libarch__common.a  zephyr/arch/arch/arm/core/lib..__..__Users__kurte__zephyrproject__zephyr__arch__arm__core.a  zephyr/arch/arch/arm/core/cortex_m/lib..__..__Users__kurte__zephyrproject__zephyr__arch__arm__core__cortex_m.a  zephyr/arch/arch/arm/core/mpu/lib..__..__Users__kurte__zephyrproject__zephyr__arch__arm__core__mpu.a  zephyr/lib/libc/picolibc/liblib__libc__picolibc.a  zephyr/lib/libc/common/liblib__libc__common.a  zephyr/lib/posix/options/liblib__posix__options.a  zephyr/subsys/input/libsubsys__input.a  zephyr/drivers/interrupt_controller/libdrivers__interrupt_controller.a  zephyr/drivers/clock_control/libdrivers__clock_control.a  zephyr/drivers/console/libdrivers__console.a  zephyr/drivers/display/libdrivers__display.a  zephyr/drivers/gpio/libdrivers__gpio.a  zephyr/drivers/i2c/libdrivers__i2c.a  zephyr/drivers/input/libdrivers__input.a  zephyr/drivers/memc/libdrivers__memc.a  zephyr/drivers/pinctrl/libdrivers__pinctrl.a  zephyr/drivers/reset/libdrivers__reset.a  zephyr/drivers/serial/libdrivers__serial.a  zephyr/drivers/timer/libdrivers__timer.a  modules/hal_stm32/stm32cube/lib..__..__Users__kurte__zephyrproject__modules__hal__stm32__stm32cube.a  modules/lvgl/libmodules__lvgl.a  -Wl,--no-whole-archive  zephyr/kernel/libkernel.a  -LD:/zephyrproject/zephyr/build/zephyr  zephyr/arch/common/libisr_tables.a  -mcpu=cortex-m7  -mthumb  -mabi=aapcs  -mfp16-format=ieee  -mtp=soft  -fuse-ld=bfd  -Wl,--gc-sections  -Wl,--build-id=none  -Wl,--sort-common=descending  -Wl,--sort-section=alignment  -Wl,-u,_OffsetAbsSyms  -Wl,-u,_ConfigAbsSyms  -nostdlib  -static  -Wl,-X  -Wl,-N  -Wl,--orphan-handling=warn  -Wl,-no-pie  -specs=picolibc.specs  -DPICOLIBC_LONG_LONG_PRINTF_SCANF -L"d:/users/kurte/downloads/zephyr-sdk-0.16.8/arm-zephyr-eabi/bin/../lib/gcc/arm-zephyr-eabi/12.2.0/thumb/v7e-m/nofp" -lc -lgcc && C:\WINDOWS\system32\cmd.exe /C "cd /D D:\zephyrproject\zephyr\build\zephyr && "C:\Program Files\CMake\bin\cmake.exe" -E true""
d:/users/kurte/downloads/zephyr-sdk-0.16.8/arm-zephyr-eabi/bin/../lib/gcc/arm-zephyr-eabi/12.2.0/../../../../arm-zephyr-eabi/bin/ld.bfd.exe: zephyr\zephyr_pre0.elf section `rodata' will not fit in region `FLASH'
d:/users/kurte/downloads/zephyr-sdk-0.16.8/arm-zephyr-eabi/bin/../lib/gcc/arm-zephyr-eabi/12.2.0/../../../../arm-zephyr-eabi/bin/ld.bfd.exe: region `FLASH' overflowed by 176920 bytes
collect2.exe: error: ld returned 1 exit status
ninja: build stopped: subcommand failed.
FATAL ERROR: command exited with status 1: 'C:\Program Files\CMake\bin\cmake.EXE' --build 'D:\zephyrproject\zephyr\build'

@iabdalkader
Copy link
Author

Maybe there's not enough flash for that demo/sample, but this now buil 8000 ds:

west build -b arduino_giga_r1//m7 -p --shield arduino_giga_display_shield samples/subsys/display/lvgl

@iabdalkader
Copy link
Author

Note: On THIS pr I believe you will also need to update boards.txt in the root directory.

Fixed, thanks!

@KurtE
Copy link
KurtE commented May 7, 2025

Maybe there's not enough flash for that demo/sample, but this now builds:

west build -b arduino_giga_r1//m7 -p --shield arduino_giga_display_shield samples/subsys/display/lvgl

Thanks, that does build and run.

It shows hello world and a little text at bottom with what looks like loop count...
Will play more later. Hoping to see if it handle touch at all.

Thanks again

@mjs513
Copy link
mjs513 commented May 15, 2025

@iabdalkader - @facchinm - @KurtE
Just as a FYI.

I went ahead and modified out display_gfx library to now use the display buffer:

#ifdef CONFIG_SHARED_MULTI_HEAP
    void* ptrFB = this->display->getFrameBuffer();
    if (ptrFB == nullptr){
      Serial.println("Memory not allocated successfully." );
      while(1){}
    }
    // Cast the void pointer to an int pointer to use it
    buffer = static_cast<uint16_t*>(ptrFB);

and works - also resolved another issue I was seeing with bus faulting.

@mjs513 mjs513 mentioned this pull request May 15, 2025
Open
@KurtE
Copy link
KurtE commented May 31, 2025

@iabdalkader @facchinm @mjs513 and all...

Sorry, not sure if this is the best place to ask/discuss this:

Currently I am trying out the Pull Request #130 which brings in a newer version of zephyr, which I believe has
your mentioned zephyr PR pulled in. So I manually added your changes into my fork:

git pull upstream pull/130/head:pr_130
git checkout pr_130

I then modified the two files in this pr in and I am trying some of the examples @mjs513 and I have been plying with, and running into some issues, which I am trying to resolve. Hopefully before a new beta is released that once some of these PR's are pulled in.

With your changes, it appears like you are now having the driver allocate two frame buffers, and I am wondering how they are intended to work. That is if you look at the file display_stm32_ltdc.c at the stm32_ltdc_write function:

Writes no full size buffer:
If I am understanding this code correctly: if the buffer you pass in, is not the same size as the display, it will
alternate between the two buffers, and it will memcpy the currently active buffer into the other buffer, than memcpy
each row of the new image into the frame buffer... Then it will setup to have the interrupt code swap the buffer
when it finishes the frame...

Notes/questions:

a) There is no validating or clipping of these ranges), that is if X+width > frame_width it wrap data to next line.
likewise for y and Height, which may corrupt memory...

b) Wondering about the calls to sys_cache_data_flush_range in the for loop for each row, but not in initial copy?

Full Size Buffer
The code simply sets the front_buf to the new buffer passed in to this function, which then makes you wonder when are
the 2 frame buffers that are allocated to the display used?

Are you expecting that the calling code will do something like: display_get_framebuffer(dev);
when it is getting ready to do an update to the screen? And then somehow know if this is the first or second buffer
and compute the other buffer. Then do all of it's updates to this buffer, and then call the write function with it?

Do you suspect that we will need to flush the cache before calling write?
know/avoid: updating the display again, until the last buffer you passed to write is now the current buffer...

Or do you see us, allocating additional buffers and using them in a round robin list...

Thanks
Kurt

@KurtE
Copy link
KurtE commented May 31, 2025

Quick update on test sketches now showing stuff.

I fixed a few of the test sketch issues. I added an example of using the two buffers like I mentioned, which works.

#include "Arduino_GigaDisplay.h"
Display display;

uint16_t *initialFB;

void setup() {
  while(!Serial && millis()< 5000){}
  Serial.begin(9600);
  Serial.println("Zephyr simple Giga Display Test!");

  display.begin();
  display.setFrameDesc(480, 800, 480, 480*800*2);
  initialFB = (uint16_t*)display.getFrameBuffer();
    pinMode(74, OUTPUT);
    Serial.print("Pin74: ");
    Serial.println(digitalRead(74), DEC);
    digitalWrite(74, HIGH);

}

void fillScreen(uint16_t color) {
  uint16_t *fbCur = (uint16_t*)display.getFrameBuffer();
  uint16_t *fb = (fbCur == initialFB)? &initialFB[480*800] : initialFB;

  for (int i = 0; i < 480*800; i++) fb[i] = color;
  display.write8(0, 0, fb);

}

void loop(void) {
  fillScreen(RGB565_WHITE);
  delay(500);
  fillScreen(RGB565_RED);
  delay(500);
  fillScreen(RGB565_GREEN);
  delay(500);
  fillScreen(RGB565_BLUE);
  delay(500);
  fillScreen(RGB565_BLACK);
  delay(500);
}

This one uses the two buffers part of the object ... Which worked.

Nothing showed up on the screen until I turned on the backligth...
I did it in the sketch like:

    pinMode(74, OUTPUT);
    digitalWrite(74, HIGH);

Probably not the prescribed way? But at least I have stuff showing up again on the screen.

@KurtE
Copy link
KurtE commented Jun 1, 2025

Nothing showed up on the screen until I turned on the backlight... I did it in the sketch like:

    pinMode(74, OUTPUT);
    digitalWrite(74, HIGH);

Probably not the prescribed way? But at least I have stuff showing up again on the screen.
I believe probably the correct way to do this is with a call like:

    // turn on the display backlight
    display->setBlanking(false);

At least it uses the defines within the display object and image shows up.

Side note: I now appear to have some touch code somewhat working. At least on my machines it does not crash and
it is giving me coordinates. One issue I believe I am seeing between my two GIGAs with displays. The orientation of
the touch screen versus the display screen is different between my two setups. maybe turned 180 degrees?
Will post more on the Touch issue.

KurtE added a commit to KurtE/ArduinoCore-zephyr that referenced this pull request Jun 1, 2025
@KurtE
Starting touch support.
20a4caf 
Note this branch is based off of commit 130 and hand modified the changes from arduino#117

it has a callback added from the touch controller which saves the last touch point
and a call function, that allows you to retrieve the last notification
@KurtE KurtE mentioned this pull request Jun 1, 2025
Open
@KurtE
Copy link
KurtE commented Jun 2, 2025

Quick notes: I was playing with, extracting the semi-libraries in our test sketches and update some of the Arduino libraries
to differentiate between MBED and zephyr... Like the touch library the GFX library and the GIGA display.
The LEDS part looks like it works without changing.

One part I have ported is the GigaDisplayBacklight, which I will play with. As I mentioned in previous post I turned on the
backlight first using the digitalWrite code, and later I saw that setBlanking does it using the define within the display class for
the blanking pin parameter.

What I am wondering is with the Backlight class code, should this be contained within the Arduino Library and/or should
this somehow be part of the zephyr display class? If part of the display class, it might want to know if the IO
pin is a PWM pin or not. If PWM, it would be nice to just be able to set the duty... Unfortunately it appears that
the GIGA pin chosen for this does not support PWM. So will probably need to be some form of timer. Preferably one
that we can change the duration each time. That it would be nice to not have to interrupt every 2ms for a 100ms
cycle (50 times per on/off...)

Thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@iabdalkader @KurtE @mjs513 @facchinm
0