Skip to content
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.

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

drivers: missing spi commands in mipi dbi driver #68815

Closed
everedero opened this issue Feb 9, 2024 · 19 comments · Fixed by #69988 or #70126
Closed

drivers: missing spi commands in mipi dbi driver #68815

everedero opened this issue Feb 9, 2024 · 19 comments · Fixed by #69988 or #70126
Assignees
Labels
area: Display bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug

Comments

@everedero
Copy link
Contributor

Describe the bug
LCD screen does not display images correctly since last ili9xxx drivers update.
There are some commands missing when probing the SPI bus.
Here is what is sent on the bus since the update:

ko_dbg1

With previous ili9xxx drivers:
ok_dbg1

Logs and console output
Debugger shows that the calls to send 0x2a and 0x2b are executed:

Breakpoint 1, mipi_dbi_spi_write_helper (dev=<optimized out>, dbi_config=0x800a9e4 <ili9xxx_config_0+8>, cmd_present=cmd_present@entry=true, cmd=<optimized out>, data_buf=0x200019ac <z_main_stack+876> "", len=len@entry=4) at /home/eve/zephyrproject/zephyr/drivers/mipi_dbi/mipi_dbi_spi.c:103                                                                                                                                                                                         
103                             ret = spi_write(config->spi_dev, &dbi_config->config,                                                                                                                                                         
(gdb) print buf_set->buffers->len                                                                                                                                                                                                             
$7 = 1                                                                                                                                                                                                                                        
(gdb) x/1bx buf_set->buffers->buf                                                                                                                                                                                                             
0x2000195f <z_main_stack+799>:  0x2b    

Environment (please complete the following information):

  • OS: GNU/Linux (LMDE 5)
  • Toolchain: Zephyr SDK
  • Zephyr branch #6fc6b30f (before MIPI DBI driver update) and #5f31c15b (after driver update)
  • Display sample application
  • Out-of-tree board: https://github.com/everedero/board_st_mb1283
  • SPI slowed down to 1MHz for probing
@everedero everedero added the bug The issue is a bug, or the PR is fixing a bug label Feb 9, 2024
@MaureenHelm MaureenHelm added the priority: medium Medium impact/importance bug label Feb 13, 2024
@danieldegrasse
Copy link
Collaborator

@everedero are you compiling with some form of optimizations enabled? Based on the probe you sent it seems that [ili9xxx_set_mem_area](https://github.com/zephyrproject-rtos/zephyr/blob/main/drivers/display/display_ili9xxx.c#L57 is simply not sending the command bytes. I can't reproduce this locally, compiling samples/drivers/display for the lpcxpresso55s69_cpu0 target with the command west build -p always -b lpcxpresso55s69_cpu0 samples/drivers/display/ -DSHIELD='adafruit_2_8_tft_touch_v2'

image

I'm testing on SHA 49c7f43, with the SPI clock slowed to 1.5MHz

@everedero
Copy link
Contributor Author

It might be a platform-specific issue, because ili9xxx_set_mem_area does jump into the SPI HAL, even for 0x2a and 0x2b commands:

#0  transceive (asynchronous=false, cb=0x0, userdata=0x0, rx_bufs=0x0, tx_bufs=<optimized out>, config=0x802d9c8 <ili9xxx_config_0+12>, dev=0x802d200 <__device_dts_ord_50>) at /home/eve/zephyrproject/zephyr/drivers/spi/spi_ll_stm32.c:773
#1  spi_stm32_transceive (dev=0x802d200 <__device_dts_ord_50>, config=0x802d9c8 <ili9xxx_config_0+12>, tx_bufs=<optimized out>, rx_bufs=0x0) at /home/eve/zephyrproject/zephyr/drivers/spi/spi_ll_stm32.c:1094
#2  0x0801e182 in z_impl_spi_transceive (rx_bufs=0x0, tx_bufs=0x2000dff8 <z_main_stack+1464>, config=0x802d9c8 <ili9xxx_config_0+12>, dev=<optimized out>) at /home/eve/zephyrproject/zephyr/include/zephyr/drivers/spi.h:761
#3  spi_transceive (rx_bufs=0x0, tx_bufs=0x2000dff8 <z_main_stack+1464>, config=0x802d9c8 <ili9xxx_config_0+12>, dev=<optimized out>) at /home/eve/Documents/zephyr_misc/zephyr_st25/build/zephyr/include/generated/syscalls/spi.h:38
#4  spi_write (tx_bufs=0x2000dff8 <z_main_stack+1464>, config=0x802d9c8 <ili9xxx_config_0+12>, dev=<optimized out>) at /home/eve/zephyrproject/zephyr/include/zephyr/drivers/spi.h:851
#5  mipi_dbi_spi_write_helper (dbi_config=0x802d9c4 <ili9xxx_config_0+8>, cmd_present=cmd_present@entry=true, cmd=<optimized out>, data_buf=0x2000e04c <z_main_stack+1548> "", len=len@entry=4, dev=<optimized out>)
    at /home/eve/zephyrproject/zephyr/drivers/mipi_dbi/mipi_dbi_spi.c:103
#6  0x0801e1e2 in mipi_dbi_spi_command_write (dev=<optimized out>, dbi_config=<optimized out>, cmd=<optimized out>, data_buf=<optimized out>, len=4) at /home/eve/zephyrproject/zephyr/drivers/mipi_dbi/mipi_dbi_spi.c:136
#7  0x0801e3ea in mipi_dbi_command_write (len=4, data=0x2000e00c <z_main_stack+1484> "\274\331\002\b7", cmd=42 '*', config=<optimized out>, dev=<optimized out>) at /home/eve/zephyrproject/zephyr/include/zephyr/drivers/mipi_dbi.h:167
#8  ili9xxx_transmit (dev=dev@entry=0x802d228 <__device_dts_ord_52>, cmd=cmd@entry=42 '*', tx_data=tx_data@entry=0x2000e04c <z_main_stack+1548>, tx_len=tx_len@entry=4)
    at /home/eve/zephyrproject/zephyr/drivers/display/display_ili9xxx.c:29
#9  0x0801e4d6 in ili9xxx_set_mem_area (h=36, w=<optimized out>, y=55, x=<optimized out>, dev=0x802d228 <__device_dts_ord_52>) at /home/eve/zephyrproject/zephyr/drivers/display/display_ili9xxx.c:66
#10 ili9xxx_write (dev=0x802d228 <__device_dts_ord_52>, x=<optimized out>, y=<optimized out>, desc=0x2000e0a0 <z_main_stack+1632>, buf=0x20005020 <buf0>) at /home/eve/zephyrproject/zephyr/drivers/display/display_ili9xxx.c:103
#11 0x0802c4da in display_write (buf=<optimized out>, desc=0x2000e0a0 <z_main_stack+1632>, y=<optimized out>, x=<optimized out>, dev=<optimized out>) at /home/eve/zephyrproject/zephyr/include/zephyr/drivers/display.h:250
#12 lvgl_flush_display (request=request@entry=0x2000e098 <z_main_stack+1624>) at /home/eve/zephyrproject/zephyr/modules/lvgl/lvgl_display.c:115
#13 0x0802c6de in lvgl_flush_cb_24bit (disp_drv=<optimized out>, area=<optimized out>, color_p=<optimized out>) at /home/eve/zephyrproject/zephyr/modules/lvgl/lvgl_display_24bit.c:26
#14 0x08022ff6 in call_flush_cb (drv=<optimized out>, area=<optimized out>, color_p=<optimized out>) at /home/eve/zephyrproject/modules/lib/gui/lvgl/src/core/lv_refr.c:1327
#15 0x0800845c in draw_buf_flush (disp=<optimized out>) at /home/eve/zephyrproject/modules/lib/gui/lvgl/src/core/lv_refr.c:1302
#16 refr_area_part (draw_ctx=draw_ctx@entry=0x20000bd4 <lvgl_heap_mem+596>) at /home/eve/zephyrproject/modules/lib/gui/lvgl/src/core/lv_refr.c:798
#17 0x08008c52 in refr_area (area_p=<optimized out>) at /home/eve/zephyrproject/modules/lib/gui/lvgl/src/core/lv_refr.c:680
#18 refr_invalid_areas () at /home/eve/zephyrproject/modules/lib/gui/lvgl/src/core/lv_refr.c:618
#19 _lv_disp_refr_timer (tmr=<optimized out>) at /home/eve/zephyrproject/modules/lib/gui/lvgl/src/core/lv_refr.c:325
#20 0x080142ea in lv_timer_exec (timer=0x20000c34 <lvgl_heap_mem+692>) at /home/eve/zephyrproject/modules/lib/gui/lvgl/src/misc/lv_timer.c:313
#21 lv_timer_handler () at /home/eve/zephyrproject/modules/lib/gui/lvgl/src/misc/lv_timer.c:109
#22 0x08000a58 in lv_task_handler () at /home/eve/zephyrproject/modules/lib/gui/lvgl/src/../src/lv_api_map.h:37
#23 main () at /home/eve/Documents/zephyr_misc/zephyr_st25/game/src/main.c:98

With:

(gdb) x/1bx tx_bufs->buffers->buf
0x2000195f <z_main_stack+799>:  0x2a

I am using debug optimizations (CONFIG_DEBUG_OPTIMIZATIONS=y). I tried to disable all the optimizations (CONFIG_NO_OPTIMIZATIONS=y... and CONFIG_MAIN_STACK_SIZE=4096 to avoid consequential stack overflow), but the issue persists.

@everedero
Copy link
Contributor Author

For reference, this is what the screen looks like with this 0x2a and 0x2b missing commands issue. Since it does not receive the coordinates correctly, the grey pixels from the display sample are stashed in a line on one side of the screen, instead of being in a square.

display_sample_with_issue

Working display sample:
display_sample_without_issue

@danieldegrasse
Copy link
Collaborator

danieldegrasse commented Feb 14, 2024

It might be a platform-specific issue, because ili9xxx_set_mem_area does jump into the SPI HAL, even for 0x2a and 0x2b commands

Maybe so- this one is really strange to me. I'm trying to think what is different about the CASET and PASET commands. I've tested on an ST board just to be sure, and my ILI9xxx display works fine there (with west build -p always -b nucleo_l433rc_p samples/drivers/display/ -DSHIELD='adafruit_2_8_tft_touch_v2' -DCONFIG_DEBUG_OPTIMIZATIONS=y). Are there any other commands that you see being missed when running the sample? Does it fail consistently?

@everedero
Copy link
Contributor Author

It does fail consistently, always the same 0x2a and 0x2b disappearing behaviour.
I tried to compare initialization commands in both cases, and only some commands are missing, never data:
ok_vs_ko.ods

@danieldegrasse
Copy link
Collaborator

danieldegrasse commented Feb 16, 2024

It does fail consistently, always the same 0x2a and 0x2b disappearing behaviour.
I tried to compare initialization commands in both cases, and only some commands are missing, never data:
ok_vs_ko.ods

Ok, so only the command is absent. I'm really not sure what would differ on your setup. You said the HAL is being called to send the command byte. A few questions:

@everedero
Copy link
Contributor Author

  • The delay does not change the behaviour:

delay_hs

  • I think nothing gets written to the hardware, it seems like it never gets to the SPIx->DR register modification (also, hardware breakpoints seem to crash my debugger sessions, so it is hard to know for sure)

  • So far I have tracked the issue to: not always jumping into spi_stm32_isr:

Breakpoint 7, transceive (dev=0x8009a04 <__device_dts_ord_47>, config=0x800a9e8 <ili9xxx_config_0+12>, tx_bufs=0x20001960 <z_main_stack+800>, rx_bufs=0x0, asynchronous=asynchronous@entry=false, cb=cb@entry=0x0, userdata=userdata@entry=0x0) at /home/eve/zephyrproject/zephyr/drivers/spi/spi_ll_stm32.c:842
842             ret = spi_context_wait_for_completion(&data->ctx);
(gdb) print tx_bufs->buffers->len
$94 = 1
(gdb) x /1bx tx_bufs->buffers->buf
0x2000195f <z_main_stack+799>:  0x2a

// Should reach breakpoint 11 in ISR here

Breakpoint 7, transceive (dev=0x8009a04 <__device_dts_ord_47>, config=0x800a9e8 <ili9xxx_config_0+12>, tx_bufs=0x20001960 <z_main_stack+800>, rx_bufs=0x0, asynchronous=asynchronous@entry=false, cb=cb@entry=0x0, userdata=userdata@entry=0x0) at /home/eve/zephyrproject/zephyr/drivers/spi/spi_ll_stm32.c:842
842             ret = spi_context_wait_for_completion(&data->ctx);
(gdb) print tx_bufs->buffers->len
$95 = 4
(gdb) x /4bx tx_bufs->buffers->buf
0x200019ac <z_main_stack+876>:  0x00    0x00    0x00    0x3b

Breakpoint 11, spi_stm32_isr (dev=0x8009a04 <__device_dts_ord_47>) at /home/eve/zephyrproject/zephyr/drivers/spi/spi_ll_stm32.c:526
526             if (spi_stm32_transfer_ongoing(data)) {

@everedero
Copy link
Contributor Author

The display works if I disable interrupt mode for SPI driver:

CONFIG_SPI_STM32_INTERRUPT=n

@danieldegrasse
Copy link
Collaborator

I took a look at your board, and I'm wondering if the duplex property is the issue- the MIPI DBI header file is currently handling this property incorrectly. Can you try running the display sample with #69200 and see if it works correctly?

@everedero
Copy link
Contributor Author

No, #69200 doesn't affect my issue

@danieldegrasse
Copy link
Collaborator

danieldegrasse commented Feb 20, 2024

No, #69200 doesn't affect my issue

Hmm, ok- can you try removing the duplex property from your display node? That is the only significant difference I see between your display devicetree and the ones in tree.

You might also want to try running the display sample with CONFIG_SPI_LOG_LEVEL_DBG=y, to see what this line prints: https://github.com/zephyrproject-rtos/zephyr/blob/main/drivers/spi/spi_ll_stm32.c#L675

@everedero
Copy link
Contributor Author

I tried with duplex removed, still with the fix, with no success.

Logging gives:
<dbg> spi_ll_stm32: spi_stm32_configure: Installed config 0x802ceec: freq 10500000Hz (div = 4), mode 0/0/0, slave 0

@danieldegrasse
Copy link
Collaborator

@erwango do you have any thoughts here? I'm not clear what changes would cause this to fail. I'm not sure this is a platform specific problem, but I have been unable to reproduce it on other boards thus far

@erwango
Copy link
Member

erwango commented Feb 21, 2024

@ajarmouni-st ^^

@ajarmouni-st
Copy link
Collaborator

@ajarmouni-st ^^

I was already following the discussion, & I don't really have an idea about what could be causing this

The display works if I disable interrupt mode for SPI driver:
CONFIG_SPI_STM32_INTERRUPT=n

(debugging IT issues is a nightmare).

@everedero
Copy link
Contributor Author

Maybe I could somehow try to record the exact calls the display driver is sending to the SPI driver, and compare the old and the new driver calls and timings? There has to be a tiny difference from the upper levels to trigger a different behaviour in lower levels, it cannot just be a curse from my desk, right? 🤔

@everedero
Copy link
Contributor Author

I tried tracing, it is not helping me much, but here it is.
The traces record a display_write call.
The 2 semaphores traced are ctx.lock and ctx.sync:

  • spi_stm32_dev_data_0.ctx.lock.wait_q.waitq <spi_stm32_dev_data_0+16>
  • spi_stm32_dev_data_0.ctx.sync.wait_q.waitq <spi_stm32_dev_data_0+36>

trace_not_working.txt
trace_working.txt

I also tried to print logs directly from the IRQ itself, it looks like it does not disrupt anything after all (the "." is a printk when entering IRQ):
log_not_working.txt
log_working.txt

I will add more detailed logging, until it either give me interesting information or make everything collapse 😃

@everedero
Copy link
Contributor Author

everedero commented Mar 5, 2024

Turns out everything works if I comment out the spinlock calls in mipi_dbi_spi_write_helper (drivers/mipi_dbi/mipi_dbi_spi.c:61 and 127).

k_spinlock_key_t spinlock_key = k_spin_lock(&data->lock);

The two missing interrupts popping between MIPI driver call and STM32 SPI LL driver call are back, for some reasons.

log_not_working_more_info.txt
log_working_without_sema.txt

trace_working_without_sema.txt

@danieldegrasse
Copy link
Collaborator

danieldegrasse commented Mar 5, 2024

Turns out everything works if I comment out the spinlock calls in mipi_dbi_spi_write_helper (drivers/mipi_dbi/mipi_dbi_spi.c:61 and 127).

k_spinlock_key_t spinlock_key = k_spin_lock(&data->lock);

The two missing interrupts popping between MIPI driver call and STM32 SPI LL driver call are back, for some reasons.

Wow. This is bad- looking at the implementation I did there, that should absolutely be a mutex. Blocking interrupts here doesn't make sense. Unless you'd like to, I'll send a PR (as well as a backport) to fix this

everedero pushed a commit to everedero/zephyr that referenced this issue Mar 10, 2024
Taking a spinlock will result in interrupts being blocked in the MIPI
DBI driver, which is not desired behavior while issuing SPI transfers,
since the driver may use interrupts to drive the transfer

Fixes zephyrproject-rtos#68815

Signed-off-by: Daniel DeGrasse <[email protected]>
everedero pushed a commit to everedero/zephyr that referenced this issue Mar 11, 2024
Taking a spinlock will result in interrupts being blocked in the MIPI
DBI driver, which is not desired behavior while issuing SPI transfers,
since the driver may use interrupts to drive the transfer

Fixes zephyrproject-rtos#68815

Signed-off-by: Daniel DeGrasse <[email protected]>
dleach02 pushed a commit that referenced this issue Mar 12, 2024
Taking a spinlock will result in interrupts being blocked in the MIPI
DBI driver, which is not desired behavior while issuing SPI transfers,
since the driver may use interrupts to drive the transfer

Fixes #68815

Signed-off-by: Daniel DeGrasse <[email protected]>
zephyrbot pushed a commit that referenced this issue Mar 12, 2024
Taking a spinlock will result in interrupts being blocked in the MIPI
DBI driver, which is not desired behavior while issuing SPI transfers,
since the driver may use interrupts to drive the transfer

Fixes #68815

Signed-off-by: Daniel DeGrasse <[email protected]>
(cherry picked from commit 5b6fadc)
henrikbrixandersen pushed a commit that referenced this issue Mar 18, 2024
Taking a spinlock will result in interrupts being blocked in the MIPI
DBI driver, which is not desired behavior while issuing SPI transfers,
since the driver may use interrupts to drive the transfer

Fixes #68815

Signed-off-by: Daniel DeGrasse <[email protected]>
(cherry picked from commit 5b6fadc)
ashiroji pushed a commit to ashiroji/zephyr that referenced this issue Mar 19, 2024
Taking a spinlock will result in interrupts being blocked in the MIPI
DBI driver, which is not desired behavior while issuing SPI transfers,
since the driver may use interrupts to drive the transfer

Fixes zephyrproject-rtos#68815

Signed-off-by: Daniel DeGrasse <[email protected]>
everedero pushed a commit to everedero/zephyr that referenced this issue May 1, 2024
Taking a spinlock will result in interrupts being blocked in the MIPI
DBI driver, which is not desired behavior while issuing SPI transfers,
since the driver may use interrupts to drive the transfer

Fixes zephyrproject-rtos#68815

Signed-off-by: Daniel DeGrasse <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Display bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug
Projects
None yet
6 participants