drivers: display: support display driver on Renesas RA EK-RA8D1 #80694
Conversation
thenguyenyf
commented
Oct 31, 2024
thenguyenyf
commented
- Add support for MIPI-DSI driver
- Add support for Display driver
- Add support for rtkmipilcdb00000be display shield
- Add support for SDRAM on EK-RA8D1
zephyrbot
added
area: Samples
zephyrbot
requested review from
avisconti,
brgl,
danieldegrasse,
duynguyenxa,
erwango,
faxe1008,
henrikbrixandersen,
jfischer-no,
kartben,
KhiemNguyenT,
mnkp,
nashif,
pdgendt,
soburi and
thaoluonguw
|
The following west manifest projects have changed revision in this Pull Request:
✅ All manifest checks OK Note: This message is automatically posted and updated by the Manifest GitHub Action. |
zephyrbot
added
manifest
manifest-hal_renesas
DNM
thenguyenyf
force-pushed
the
support_renesas_ra_display
branch
from
31eafc6 to
537381c
Compare
There was a problem hiding this comment.
Commits need to be split up here to be more granular (this suggested list is ordered from oldest to newest)
- Hal update commit
- Commit adding SDRAM memc driver
- Commit adding SDRAM enablement changes at SOC level
- Commit enabling SDRAM driver on EK-RA8D1 board
- Commit enabling memc test for EK-RA8D1
- Commit adding renesas_ra display driver (and dts bindings)
- Commit adding SOC level definitions to support renesas_ra display driver
- Commit adding dsi_renesas_ra driver (and dts bindings)
- Commit adding SOC level definitions to support dsi_renesas_ra driver
- Commit adding ili9806e_dsi display driver (and dts bindings)
- Commit adding board support for renesas_ra, dsi_renesas, and ili9806_dsi display driver
- Commit adding rtkmipilcdb00000be shield
- Commit adding display test support for the EK-RA8D1
Note- the change request here is specifically about the use of CONFIG_RENESAS_RA_DISPLAY in generic code such as LVGL.
drivers/memc/memc_renesas_ra_sdram.c
Outdated
| LOG_ERR("pin function initial failed"); | ||
| return err; | ||
| } | ||
| sdram_init(); |
There was a problem hiding this comment.
It looks like the SDRAM initialization is defined here in the HAL: https://github.com/zephyrproject-rtos/hal_renesas/pull/45/files#diff-aebfe76749f0c33920f681bb5d983c3f86c6af05e9eca5af73030e941b239595R154
I am wondering- will this SDRAM initialization work for any SDRAM, or specifically the SDRAM part on the EK-RA8D1? I see that function sets some parameters like the CAS latency and write recovery time that I suspect could vary between SDRAM chips, right?
There was a problem hiding this comment.
Current implementation just supports for SDRAM on EK-RA8D1
There was a problem hiding this comment.
Ok- I would suggest naming the driver based on the SDRAM part number (IE renesas_ra_sdram_xxx) then. Otherwise, users may incorrectly assume that the driver will work for any SDRAM. Another (better) option would be to implement a MEMC driver capable of configuring the SDRAM controller based on settings in devicetree, so it can work for any SDRAM chip
There was a problem hiding this comment.
I'm still concerned about the naming of this driver. This implies the SDRAM memc driver will enable SDRAM support on any board with an RA SOC, right?
What will happen when a customer designs a board with a different SDRAM part, and tries to use this driver?
One option to explore would be making the SDRAM init part of a board specific C init file.
| @@ -25,5 +25,12 @@ | |||
| renesas,programming-enable; | |||
| }; | |||
| }; | |||
|
|
|||
| sdram: sdram@68000000 { | |||
There was a problem hiding this comment.
Again here- should this be at the SOC level? Seems like the SDRAM on the EK-RA8D1 is an external memory chip specific to that board. Perhaps the reg address can be set here, but the size probably needs to be 0, and can be overridden at the board level.
There was a problem hiding this comment.
I think the best option here is probably one of these two approaches:
- Define the entire SDRAM node at the board level
- Define a node for the SDRAM controller on the SOC at the SOC level, and define the SDRAM part itself at the board level. This is similar to how we define other external devices, like flash chips on a FlexSPI bus.
The best option would be the second one- if we do that though, we would need to support setting SDRAM settings like CAS latency from devicetree (or based on the compatible for the SRAM part number).
There was a problem hiding this comment.
Done. I have updated as first one. Update for SDRAM controller as second one will be added later. Thanks for your idea.
modules/lvgl/lvgl_display.c
Outdated
| @@ -85,9 +85,15 @@ int set_lvgl_rendering_cb(lv_disp_drv_t *disp_drv) | |||
| #endif | |||
| break; | |||
| case PIXEL_FORMAT_RGB_888: | |||
| #if defined(CONFIG_RENESAS_RA_DISPLAY) | |||
There was a problem hiding this comment.
Same story here. It seems like the display is using ARGB8888?
There was a problem hiding this comment.
This change has been reverted
First commit to add support for SDRAM controller on Renesas RA SoC Signed-off-by: The Nguyen <the.nguyen.yf@renesas.com>
Add device node to support SDRAM controller on Renesas RA8D1 SoC Signed-off-by: The Nguyen <the.nguyen.yf@renesas.com>
Add support for SDRAM on Renesas RA EK-RA8D1 Signed-off-by: The Nguyen <the.nguyen.yf@renesas.com>
First commit to add support for Graphics LCD Controller on Renesas RA Signed-off-by: The Nguyen <the.nguyen.yf@renesas.com>
Add device node to support Graphics LCD Controller on RA8D1 SoC Signed-off-by: The Nguyen <the.nguyen.yf@renesas.com>
First commit to add support for MIPI DSI driver on Renesas RA Signed-off-by: The Nguyen <the.nguyen.yf@renesas.com>
Add device node to support MIPI DSI driver on Renesas RA8D1 SoC Signed-off-by: The Nguyen <the.nguyen.yf@renesas.com>
First commit to add support for ili9806e_dsi driver Signed-off-by: The Nguyen <the.nguyen.yf@renesas.com>
First commit to support rtkmipilcdb00000be display shield Signed-off-by: The Nguyen <the.nguyen.yf@renesas.com>
thenguyenyf
dismissed stale reviews from KhiemNguyenT, uLipe, and duynguyenxa
via
90bdd17
thenguyenyf
force-pushed
the
support_renesas_ra_display
branch
from
a74f262 to
90bdd17
Compare
|
Hi @danieldegrasse . Could you please take a look? |
|
|
||
| config DISPLAY_BUFFER_USE_GENERIC_SECTION | ||
| bool "Place the display buffer in a specific memory section" | ||
| default n |
There was a problem hiding this comment.
Nit- redundant default n
There was a problem hiding this comment.
Fixed. Thanks for your support!
danieldegrasse
dismissed
their stale review
Block itself has been addressed, only nits remain
thenguyenyf
force-pushed
the
support_renesas_ra_display
branch
from
90bdd17 to
c418429
Compare
thenguyenyf
requested review from
danieldegrasse,
KhiemNguyenT,
duynguyenxa and
uLipe
This commit to add support to build tests/drivers/display on ek_ra8d1 Signed-off-by: The Nguyen <the.nguyen.yf@renesas.com>
Add support to build samples/drivers/display on ek_ra8d1 Signed-off-by: The Nguyen <the.nguyen.yf@renesas.com>
This commit to add support to build samples/subsys/display on ek_ra8d1 Signed-off-by: The Nguyen <the.nguyen.yf@renesas.com>
Add support for lvgl demo build with rtkmipilcdb00000be shield Signed-off-by: The Nguyen <the.nguyen.yf@renesas.com>
Add SDRAM controller test support Signed-off-by: The Nguyen <the.nguyen.yf@renesas.com>
thenguyenyf
force-pushed
the
support_renesas_ra_display
branch
from
c418429 to
3b390e4
Compare
