-
Notifications
You must be signed in to change notification settings - Fork 8k
Enable LTDC/DSI on stm32f469i-disco using PLLSAI #95954
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
base: main
Are you sure you want to change the base?
Conversation
4f47d94
to
87579b6
Compare
You should clarify in the description that the PR depends on #95200, and maybe make it a draft until the latter is merged. |
If possible, please also fix I tested it on the |
Actually, this PR fully depends on #95200. It is what i wanted to mention in the beginning of the description. |
@phildefer The problem with including commits from another PR and marking the PR as ready-to-review, is that many reviewers are pinged unnecessarily. |
I misread the board. This is f4. I should reply to another PR, but the other PR is waiting to be merged. |
@zhang-wenchao If you need something to be fixed, please open a dedicated PR and avoid noise and scope creep in other PRs. |
@JarmouniA : display is fully integrated onto the stm32f469i-disco and it is not the mb1166 shield as on some other STM32 boards. That is the reason why changes has been made directly in board dts file and not using st_b_lcd40_dsi1_mb1166 overlay. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @phildefer.
Indeed, while this is very close (is it really exactly same ?) to the MB1166 (either normal or a0 variant), this is directly connected to the board via a different 25pins connector different from the usual 30pins DSI/I2C connector.
I have been wondering if it would be possible to directly attach (by default) the shield st_b_lcd40_dsi1_mb1166 to this board (kind of select SHIELD_ST_B_LCD40_DSI1_MB1166 with this board but that doesn't seem possible.
Benefit of it would have been to avoid duplication.
A few comments apart from that.
61bfdeb
to
d47ddc4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @phildefer for the update.
A few comments added, mainly due to other ongoing PRs which are also impacting this one.
Apart from that, this looks good to me.
rebased, integrating changes from #96322 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @phildefer for the update.
LGTM, just a very minor point regarding an already commented out line in the defconfig to be removed (MEM_POOL_SIZE) and maybe if you make a new version of the PR you might want as well to correct the ordering in the dts files (status property at the end).
Thanks for your patience.
|
config LV_Z_FULL_REFRESH | ||
default y | ||
|
||
config INPUT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not specific to LVGL.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, it is not specific to LVGL, however in a sense it often comes with LVGL and I feel that having this LVGL part within the Kconfig.defconfig allows to avoid having to add a board specifc conf in each LVGL sample apps.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, just needs to be outside if LVGL... endif
, and inside if DISPLAY... endif
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part is common with settings present in M1166 display shield
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having checked a bit more, I am having 2nd thought about this config INPUT in fact.
- LVGL can work without having an input available (at least it sounds like it)
- While there is a touchscreen on this display, an application running might not actually want to have the touchscreen initialized / running
- LVGL samples (at least the ones in samples/modules/lvgl) ALSO set CONFIG_INPUT=y themself.
- It should also be possible to have INPUT enabled even if there is no LVGL used
So, for all those reasons, I actually think that we just shouldn't have config INPUT at all in this Kconfig.defconfig, this is a matter of the project to say if it wants to have the INPUT available or not based on its needs.
Does this make sense ?
While this is also done in several other Kconfig.defconfig around for other STM32 boards/shield, I think this should not be put in here and should actually be removed from the existing places.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@phildefer, if you agree with my analysis, I let you remove this config INPUT from the Kconfig.default and I will shortly push a PR to avoid enforcing INPUT=y in the STM32 board / shield.
Describe mipi_dsi block available on stm32f469 & above Allow to display data on DSI panels taking output of LTDC after serializing data. Signed-off-by: Philippe Peurichard <[email protected]>
Extension of support of pllsai for display configuration of stm32f469 discovery board. Enable Display panel through LTDC & DSI-HOST blocks. Enable Touch screen. Enable FMC/SDRAM for Framebuffer. Signed-off-by: Philippe Peurichard <[email protected]>
Add stm32f749i_disco conf file to support samples display driver applications . Increase the amount of HEAP to ensure k_malloc allocation goes well. Signed-off-by: Philippe Peurichard <[email protected]>
To complement Pull Request #95200, this PR adds configuration of PLLSAI for stm32f469i-disco.
It enables configurations of LTDC & DSI-HOST blocks to use display and touchscreen on this board.
To use framebuffer required for ltdc & dsi drivers, it enables FMC/SDRAM also on this board.
This PR has been successfully tested using samples/drivers/display, samples/subsys/display/lvgl &
samples/subsys/inputs_draw_events applications