search

zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.75k stars 6.56k forks source link

Display: stm32_ltdc: using "display-controller" causes Device Initialization Priority Validation to fail #68527

Closed ajarmouni-st closed 1 month ago

ajarmouni-st commented 8 months ago

Describe the bug The phandle provided in the display-controller property causes Device Initialization Priority Validation to fail at compile time on stm32h747i_disco, as LTDC cannot be initialized before OTM8009A while it depends on it in DTS, & the latter needs to be initialized after both DSI & LTDC for the display to function correctly.

Please also mention any information which could help others to understand the problem you're facing:

To Reproduce Steps to reproduce the behavior:

  1. cd ~/zephyrproject/zephyr
  2. apply this patch 
    --- a/boards/shields/st_b_lcd40_dsi1_mb1166/boards/st_b_lcd40_dsi1_mb1166/stm32h747i_disco_stm32h747xx_m7.overlay
+++ b/boards/shields/st_b_lcd40_dsi1_mb1166/boards/st_b_lcd40_dsi1_mb1166/stm32h747i_disco_stm32h747xx_m7.overlay
@@ -4,6 +4,7 @@
* SPDX-License-Identifier: Apache-2.0
*/
&ltdc {
+       display-controller = <&otm8009a>;
    /* orisetech, otm8009a */
    display-timings {
            compatible = "zephyr,panel-timing";
  3. run west build -p always -b stm32h747i_disco/stm32h747xx/m7 samples/drivers/display -DSHIELD="st_b_lcd40_dsi1_mb1166"

Expected behavior Application built correctly

Impact New display-controller property cannot be used on this platform.

Logs and console output

ERROR: Device initialization priority validation failed, the sequence of initialization calls does not match the devicetree dependencies.
ERROR: /soc/display-controller@50001000 <stm32_ltdc_init> is initialized before its dependency /soc/dsihost@50000000/otm8009a@0 <otm8009a_init> (POST_KERNEL+4 < POST_KERNEL+6)

Environment (please complete the following information):

Additional context N/A

erwango commented 8 months ago

The phandle provided in the display-controller property causes Device Initialization Priority Validation to fail at compile time on stm32h747i_disco, as LTDC cannot be initialized before OTM8009A while it depends on it in DTS, & the latter needs to be initialized after both DSI & LTDC for the display to function correctly.

@danieldegrasse, @fabiobaltieri Any idea on how to solve this properly ? Would we be the firsts to face such issue ?

fabiobaltieri commented 8 months ago

Would we be the firsts to face such issue?

No, there's a _IGNORE_COMPATIBLES list for quirky cases, you could add the ltdc compatible there and it would not get validated anymore. Trying to understand though, can't LTDC initialize after the SPI screen driver?

ajarmouni-st commented 8 months ago

No, there's a _IGNORE_COMPATIBLES list for quirky cases, you could add the ltdc compatible there and it would not get validated anymore.

Hi @fabiobaltieri, thank you for your feedback.

Trying to understand though, can't LTDC initialize after the SPI screen driver?

On h747i_disco, no. I tried changing the initialization order but the display didn't work. Btw, the panel's controller driver uses MIPI DSI not SPI.

fabiobaltieri commented 8 months ago

On h747i_disco, no. I tried changing the initialization order but the display didn't work.

How does it break? What the devicetree is saying (and that check is enforcing) with

 &ltdc {
...
       display-controller = <&otm8009a>;

is that ltdc depends on otm8009a, hence otm8009a has to be initialized before ltdc and ltdc could be using it in its own init (which is not the case now, but it could be) and it'd have to be ready.

What prevents the display from working on your use case? I'd imagine the init sequence to be something like

And that makes sense to me.

By the way you can run west build -t initlevels to get the complete init sequence if that helps.

ajarmouni-st commented 8 months ago

How does it break? What the devicetree is saying (and that check is enforcing) with

 &ltdc {
...
       display-controller = <&otm8009a>;

is that ltdc depends on otm8009a, hence otm8009a has to be initialized before ltdc and ltdc could be using it in its own init (which is not the case now, but it could be) and it'd have to be ready.

What prevents the display from working on your use case? I'd imagine the init sequence to be something like

  • dsi controller
  • otm8009a (set the interface mode but do not unblank)
  • ltdc (initialize the data buffer and starts sending data to the display)
  • application, load the initial image and unblank from ltdc to otm8009a

And that makes sense to me.

You are right, sorry! The other day when I tested, I forgot to create a separate flag for LTDC priority, that must be why it didn't work. This patch solves the problem on h747i_disco.

--- a/boards/shields/st_b_lcd40_dsi1_mb1166/boards/stm32h747i_disco_m7.conf
+++ b/boards/shields/st_b_lcd40_dsi1_mb1166/boards/stm32h747i_disco_m7.conf
@@ -6,3 +6,4 @@ CONFIG_STM32_LTDC_RGB888=y
 CONFIG_HEAP_MEM_POOL_SIZE=65536
 # Initialize after LTDC and MIPI-DSI
 CONFIG_DISPLAY_OTM8009A_INIT_PRIORITY=87
+CONFIG_DISPLAY_LTDC_INIT_PRIORITY=87

--- a/drivers/display/Kconfig.stm32_ltdc
+++ b/drivers/display/Kconfig.stm32_ltdc
@@ -59,4 +59,10 @@ config STM32_LTDC_DISABLE_FMC_BANK1
          Disable FMC bank1 if not used to prevent speculative read accesses.
          Refer to AN4861 "4.6 Special recommendations for Cortex-M7 (STM32F7/H7)".

+config DISPLAY_LTDC_INIT_PRIORITY
+       int "Initialization priority"
+       default DISPLAY_INIT_PRIORITY
+       help
+         LTDC display driver initialization priority.
+
 endif # STM32_LTDC

--- a/drivers/display/display_stm32_ltdc.c
+++ b/drivers/display/display_stm32_ltdc.c
@@ -616,7 +616,7 @@ static const struct display_driver_api stm32_ltdc_display_api = {
                        &stm32_ltdc_data_##inst,                                                       \
                        &stm32_ltdc_config_##inst,                                                     \
                        POST_KERNEL,                                                                   \
-                       CONFIG_DISPLAY_INIT_PRIORITY,                                                  \
+                       CONFIG_DISPLAY_LTDC_INIT_PRIORITY,                                             \
                        &stm32_ltdc_display_api);

 DT_INST_FOREACH_STATUS_OKAY(STM32_LTDC_DEVICE)
ajarmouni-st commented 8 months ago

CONFIG_DISPLAY_LTDC_INIT_PRIORITY=88 gives the following error

[00:00:01.032,000] <err> dsi_stm32: Transfer failed! (3)          
[00:00:01.032,000] <err> otm8009a: Read panel ID failed! (-5)     
[00:00:01.032,000] <err> otm8009a: Panel ID check failed! (-5)
fabiobaltieri commented 8 months ago

CONFIG_DISPLAY_LTDC_INIT_PRIORITY=88 gives the following error

Ok we are getting somewhere :-) can you figure out why these are failing? Something in dsi_stm32 must be depending on something in ltdc, like a clock or reset or power or something. If that could be fixed then we are done.

About the priorities, ideally otm8009a and ltdc would have the same init priority number, then the devicetree ordinal magic would take care of keeping them in the correct order.

ajarmouni-st commented 8 months ago

About the priorities, ideally otm8009a and ltdc would have the same init priority number, then the devicetree ordinal magic would take care of keeping them in the correct order.

By default, they have the priority CONFIG_DISPLAY_INIT_PRIORITY (85).

fabiobaltieri commented 8 months ago

By default, they have the priority CONFIG_DISPLAY_INIT_PRIORITY (85).

Yeah if they both have it, then the sequence should be guaranteed to follow the devicetree dependencies.

ajarmouni-st commented 8 months ago

Yeah if they both have it, then the sequence should be guaranteed to follow the devicetree dependencies.

Yes, except in the case where the panel controller depends on DSI, which has default init priority 86.

fabiobaltieri commented 8 months ago

Ok let's keep the priority nightmare stuff out for now -- if you could troubleshoot that error that'd be cool.

github-actions[bot] commented 6 months ago

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

github-actions[bot] commented 4 months ago

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

github-actions[bot] commented 1 month ago

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.