Skip to content

Update u8g_ll_api.c#24

Closed
kgy2002 wants to merge 2 commits intoMarlinFirmware:masterfrom
kgy2002:master
Closed

Update u8g_ll_api.c#24
kgy2002 wants to merge 2 commits intoMarlinFirmware:masterfrom
kgy2002:master

Conversation

@kgy2002
Copy link
Copy Markdown

@kgy2002 kgy2002 commented Mar 23, 2021

Replace u8g_Begin with u8g_UpdateDimension per comment to prevent 2 calls to u8g_Begin:

  /* On the Arduino Environment this will lead to two calls to u8g_Begin(), the following line will be called first (by U8glib constructors) */
  /* if - in future releases - this is removed, then still call u8g_UpdateDimension() */
  /* if Arduino call u8g_UpdateDimension else u8g_Begin */
  /* issue 146 */

This fixes pin 0 config when u8g_Begin is called twice.
May affect other inits but only tested this change on my board.

See Marlin Issue MarlinFirmware/Marlin#21277 and PR MarlinFirmware/Marlin#21417 for more info.

Replace u8g_Begin with u8g_UpdateDimension per comment
@thinkyhead
Copy link
Copy Markdown
Member

Well isn't it nice that a comment is already there to point the way…!

@kgy2002
Copy link
Copy Markdown
Author

kgy2002 commented Mar 24, 2021

Looks good to me. Everything still working here.

@rhapsodyv
Copy link
Copy Markdown
Member

rhapsodyv commented Mar 31, 2021

We are having similar issue with SMT32....

The issue is: Marlin keeps a global uglib instance, that makes it init in the __libc_init_array, before any system initialisation... and uglib try to do hardware things on init... this is messing up everything...

(gdb) bt
#0  WWDG_IRQHandler () at /Users/keith/.platformio/packages/framework-arduinoststm32/system/Drivers/CMSIS/Device/ST/STM32F1xx/Source/Templates/gcc/startup_stm32f103xe.s:114
#1  <signal handler called>
#2  pinMode (ulPin=ulPin@entry=23, ulMode=ulMode@entry=1) at /Users/keith/.platformio/packages/framework-arduinoststm32/cores/arduino/wiring_digital.c:31
#3  0x08013b10 in u8g_dev_rrd_st7920_128x64_fn (u8g=0x200009e4 <u8g+8>, dev=0x2000006c <u8g_dev_st7920_128x64_rrd_sw_spi>, msg=<optimized out>, arg=0x0) at Marlin/src/lcd/dogm/ultralcd_st7920_u8glib_rrd_AVR.cpp:123
#4  0x0802240a in u8g_InitLL (u8g=u8g@entry=0x200009e4 <u8g+8>, dev=0x2000006c <u8g_dev_st7920_128x64_rrd_sw_spi>) at .pio/libdeps/STM32F103RC_btt_stm32/U8glib-HAL/src/clib/u8g_ll_api.c:53
#5  0x080224fc in u8g_Begin (u8g=u8g@entry=0x200009e4 <u8g+8>) at .pio/libdeps/STM32F103RC_btt_stm32/U8glib-HAL/src/clib/u8g_ll_api.c:191
#6  0x08022544 in u8g_InitSPI (u8g=u8g@entry=0x200009e4 <u8g+8>, dev=<optimized out>, sck=<optimized out>, mosi=<optimized out>, cs=24 '\030', a0=255 '\377', reset=255 '\377') at .pio/libdeps/STM32F103RC_btt_stm32/U8glib-HAL/src/clib/u8g_ll_api.c:255
#7  0x08021f50 in U8GLIB::initSPI (this=this@entry=0x200009dc <u8g>, dev=<optimized out>, sck=sck@entry=25 '\031', mosi=mosi@entry=23 '\027', cs=<optimized out>, cs@entry=24 '\030', a0=<optimized out>, a0@entry=255 '\377', reset=<optimized out>, reset@entry=255 '\377')
    at .pio/libdeps/STM32F103RC_btt_stm32/U8glib-HAL/src/U8glib.cpp:45
#8  0x08012b5e in U8GLIB::U8GLIB (reset=255 '\377', a0=255 '\377', cs=24 '\030', mosi=23 '\027', sck=25 '\031', dev=<optimized out>, this=0x200009dc <u8g>) at .pio/libdeps/STM32F103RC_btt_stm32/U8glib-HAL/src/U8glib.h:75
#9  U8GLIB_ST7920_128X64_RRD::U8GLIB_ST7920_128X64_RRD (reset=255, cs=24, mosi=23, sck=25, this=0x200009dc <u8g>) at Marlin/src/lcd/dogm/HAL_LCD_class_defines.h:63
#10 __static_initialization_and_destruction_0 (__initialize_p=1, __priority=65535) at Marlin/src/lcd/dogm/marlinui_DOGM.cpp:79
#11 _GLOBAL__sub_I_u8g () at Marlin/src/lcd/dogm/marlinui_DOGM.cpp:472
#12 0x08022c30 in __libc_init_array ()
#13 0x08009266 in Reset_Handler () at /Users/keith/.platformio/packages/framework-arduinoststm32/system/Drivers/CMSIS/Device/ST/STM32F1xx/Source/Templates/gcc/startup_stm32f103xe.s:97
Backtrace stopped: previous frame identical to this frame (corrupt stack?)

The correct fix here is:

  1. don't have a u8glib global var
  2. make u8glib global var a pointer, and init it on the Marlin UI init
  3. u8glib don't try init hardware on constructor, having a "init" method to be called on setup

@rhapsodyv
Copy link
Copy Markdown
Member

I just send the fix I proposed to Marlin. It fixes the u8g lib issue we are having on STM32.

@kgy2002 can you test if my fix work for your case too?

MarlinFirmware/Marlin#21496

@kgy2002
Copy link
Copy Markdown
Author

kgy2002 commented Apr 2, 2021

@rhapsodyv
I tested your latest (2nd) commit to the PR and it doesn't fix my issue. In the end you are still setting up the constructor which calls u8g->begin() once then u8g->firstPage() which calls u8g->begin() a second time. The second call is what's breaking the first call's I/O setting.

@rhapsodyv
Copy link
Copy Markdown
Member

Ok, don't worry. We can have two issues and two fixes. :-)

We just need do one more check. Some Hal on marlin do some sort of hack to handle these multiple u8g init calls. They use a static local counter and only init the io when u8g lib is called more than once.

I don't know if this fix will affect this. But if it affect, we need to update the marlin code too.

https://github.com/MarlinFirmware/Marlin/blob/e3a12c3c28c2b6c4882eaa6549c6053b4d38953e/Marlin/src/lcd/dogm/u8g_dev_tft_upscale_from_128x64.cpp#L432 to line 437

This code was inherited by the old upscale dogm code.

I will test it with this changes. And seek for another places with the same hack.

@thinkyhead
Copy link
Copy Markdown
Member

Please test Marlin as it stands now, since I went ahead and implemented delayed initialization directly in this library. If it is working well, then this can be closed.

@kgy2002
Copy link
Copy Markdown
Author

kgy2002 commented Apr 15, 2021

Tested the current version of this library with bugfix-2.0.x pulled today. Everything works fine.

@kgy2002 kgy2002 closed this Apr 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants