LCDIF and DCNANO performance improvements for LVGL #57421

danieldegrasse · 2023-05-01T01:27:36Z

This PR includes the following updates:

Updates Zephyr's revision of LVGL to include performance tuning options for the following settings:
- Enable forcing LVGL to perform a full display refresh on every update
- Enable custom alignment and linker section location of LVGL framebuffers.
Updates MCUX ECLDIF and DCNANO drivers to pass the framebuffer in the display_write call directly to hardware when possible (ie when the buffer is the same size as the display). This enables the calling application to avoid the expensive memory copies associated with partial display updates by always calling display_write with a buffer the same size as the connected display
Updates LVGL configuration for the RT595, RT1060, and RT1170 EVK with the following settings:
- Allocate dual LVGL rendering buffers equal in size to the connected display
- Always refresh the entire display (so that the hardware can use the LVGL rendering buffer directly)
- For the RT595 EVK, relocate the LVGL rendering buffers to external PSRAM since they will not fit in internal RAM

This results in the following performance improvements in the weighted FPS of the LVGL benchmark sample:

RT595:
3 FPS -> 7 FPS

RT1170:
3 FPS -> 34 FPS

RT1060:
18 FPS -> 65 FPS

zephyrbot · 2023-05-01T01:28:36Z

The following west manifest projects have been modified in this Pull Request:

Name	Old Revision	New Revision	Diff
lvgl	zephyrproject-rtos/lvgl@`1557cb3`	zephyrproject-rtos/lvgl@`7102083` (zephyr)	zephyrproject-rtos/lvgl@1557cb3e..7102083f

Note: This message is automatically posted and updated by the Manifest GitHub Action.

drivers/display/display_mcux_elcdif.c

pdgendt · 2023-05-08T06:59:09Z

drivers/display/display_mcux_elcdif.c

+	/* Wait for frame send to complete */
+	k_sem_take(&dev_data->sem, K_FOREVER);


Putting it here at the end will force the thread to always suspend, right? Shouldn't we keep this at the start to let, for example LVGL, continue running?

EDIT: I guess this is needed if there's only a single buffer, which we can't use until the display has synced.

Yes, and this should still work with a multi-buffer setup. Do you know if LVGL will be blocked by this? Based on the lv_disp_flush_ready call required when display flushes complete, I assumed that LVGL would not call display_write from the rendering thread

I think display_write will be called from the thread that runs lv_task_handler. A single thread should be used with LVGL as it explicitly states that it isn't thread-safe.

I think display_write will be called from the thread that runs lv_task_handler

That is a bit concerning- the reason this code currently blocks where it does is that when I did not LVGL would often start writing to the current display buffer while it was still being sent to the display- even in a double buffering situation.

Seems like what LVGL might expect is the user to call lv_disp_flush_ready from an ISR context, when the frame render completes? I'm not sure how else LVGL could get a notification that the framebuffer is now ready, and not block within the display write callback. I suppose from the Zephyr port, we could submit display render requests to a message queue, and have a separate thread manage those.

Ok, I was bothered by this so I took a look into it- it seems that LVGL may be designed to work like so:

LVGL calls flush_cb, which writes to the display

At this point, the flushing field of the draw buffer is set to true (Calling lv_disp_flush_ready will clear this field)

When LVGL refreshes again, if flushing is still set, then LVGL will use a callback called wait_cb to wait for the flushing field to be clear

We would probably need a background thread to call display_write, then call lv_disp_flush_ready. The background thread could then give to a semaphore. Our wait_cb would then wait on this semaphore, so that if LVGL attempted to refresh before the buffer was available, it would be blocked.

I think this is probably worth adding to our LVGL display port, and I will likely look at it when I get bandwidth to do so. For the time being though, I think this PR needs to block display_write the way it currently does.

Sounds good, maybe create an enhancement issue to keep track of this?

carlescufi · 2023-05-22T09:12:26Z

@danieldegrasse module PR merged, please update revision with SHA

Update LVGL revision with performance tuning options, including the following: - Enable forcing LVGL to perform a full display refresh on every update - Enable custom alignment and linker section location of LVGL framebuffers. Signed-off-by: Daniel DeGrasse <daniel.degrasse@nxp.com>

jfischer-no · 2023-05-25T08:11:42Z

boards/arm/mimxrt595_evk/board.c

+
+#ifdef CONFIG_LV_Z_VBD_CUSTOM_SECTION
+extern char __flexspi2_start[];
+extern char __flexspi2_end[];
+
+static int init_lvgl_bufs(void)
+{
+	/*
+	 * LVGL rendering buffers will be stored in FlexSPI2 linker section.
+	 * Zero out BSS section.
+	 */
+	memset(__flexspi2_start, 0, __flexspi2_end - __flexspi2_start);
+	return 0;
+}
+
+#endif /* CONFIG_LV_Z_VBD_CUSTOM_SECTION */
+
 #if CONFIG_REGULATOR
 /* PMIC setup is dependent on the regulator API */
 SYS_INIT(board_config_pmic, POST_KERNEL, CONFIG_APPLICATION_INIT_PRIORITY);
 #endif

+#ifdef CONFIG_LV_Z_VBD_CUSTOM_SECTION
+/* LVGL buffers should be setup after PSRAM is initialized but before LVGL init */
+SYS_INIT(init_lvgl_bufs, POST_KERNEL, CONFIG_APPLICATION_INIT_PRIORITY);
+#endif
+


That does not look right to me. There should be no third-party project-specific code in the boards/soc area. It could be dc_ram/dc_ram.ld, which would be used by any display subsystem.

Ok, I have renamed the linker file to dc_ram.ld, and the function call to init_psram_framebufs

Enable the ELCDIF driver to directly write the framebuffer using hardware, when an entire framebuffer update is requested. This will enable better performance for applications that avoid partial display updates. Signed-off-by: Daniel DeGrasse <daniel.degrasse@nxp.com>

Update LVGL settings to improve performance with ELCDIF. This update requires changes to the LVGL module, to introduce Kconfig settings for enabling full display refresh, and setting display buffer alignment. Signed-off-by: Daniel DeGrasse <daniel.degrasse@nxp.com>

Update LVGL settings to optimize RT1170 performance with LCDIF. The display hardware will perform best when LVGL always uses full refreshes, as this avoids memory copies required with partial display refreshes. Signed-off-by: Daniel DeGrasse <daniel.degrasse@nxp.com>

Enable direct framebuffer rendering using DCNANO LCDIF, to improve performance when the call to display_write is attempting to refresh the full display. Signed-off-by: Daniel DeGrasse <daniel.degrasse@nxp.com>

Add FLEXSPI1 and FLEXSPI2 memory sections for RT5xx SOC. These sections can be used by the user's application as part of a custom linker script, if the application needs to relocate a buffer to external SRAM connected to FLEXSPI1 or FLEXSPI2 Signed-off-by: Daniel DeGrasse <daniel.degrasse@nxp.com>

Configure LVGL to improve performance on the RT595 EVK, with the following changes: - Allocate two rendering buffers for LVGL, both the entire size of the display - Force LVGL full refresh bit - Locate LVGL rendering buffers in external PSRAM (since they will not fit on onboard SRAM) Signed-off-by: Daniel DeGrasse <daniel.degrasse@nxp.com>

gmarull

a couple of nitpicks, otherwise lgtm

gmarull · 2023-05-26T12:22:59Z

drivers/display/display_mcux_dcnano_lcdif.c

 	}

 #if defined(CONFIG_HAS_MCUX_CACHE) && defined(CONFIG_MCUX_DCNANO_LCDIF_MAINTAIN_CACHE)
-	CACHE64_InvalidateCacheByRange((uint32_t) data->fb[next_fb_idx],
+	CACHE64_CleanCacheByRange((uint32_t) data->active_fb,


note: can't cache.h API be used here?

In the future it should be, yes. Currently CONFIG_CACHE_MANAGEMENT is not enabled for the RT5xx platform this IP block is present on, as we do not have a cache API driver for the CACHE64 peripheral

ok, I think we should standardize this part in the near future as we now have APIs, cc: @carlocaione

I agree- I'm going to create an issue to highlight this, because this is something that we've also discussed internally and the change needs to be made

Edit: created as an enhancement: #58324. Can always be moved to a bug if this is something that we think needs to be addressed before the release

gmarull · 2023-05-26T12:23:27Z

drivers/display/display_mcux_elcdif.c

+	ELCDIF_EnableInterrupts(config->base,
+				kELCDIF_CurFrameDoneInterruptEnable);


this needs to use Zephyr IRQ API

IRQ enable is called down here: https://github.com/nxp-zephyr/zephyr/blob/feature/lcdif_performance/drivers/display/display_mcux_elcdif.c#L341. This call enables the peripheral itself to generate the interrupt signal for the NVIC, and has no Zephyr IRQ API equivalent.

ok thanks for the details

main concerns answered

danieldegrasse requested review from mmahadevan108 and dleach02 as code owners May 1, 2023 01:27

zephyrbot added platform: NXP NXP area: Display labels May 1, 2023

zephyrbot requested review from decsny, DerekSnell, EmilioCBen, jfischer-no, manuargue and yvanderv May 1, 2023 01:28

zephyrbot assigned dleach02 May 1, 2023

zephyrbot added manifest manifest-lvgl DNM This PR should not be merged (Do Not Merge) labels May 1, 2023

danieldegrasse force-pushed the feature/lcdif_performance branch 2 times, most recently from ab0dcba to 387fb0a Compare May 1, 2023 20:33

pdgendt previously approved these changes May 8, 2023

View reviewed changes

danieldegrasse dismissed pdgendt’s stale review via 27f0685 May 22, 2023 13:59

danieldegrasse force-pushed the feature/lcdif_performance branch from 387fb0a to 27f0685 Compare May 22, 2023 13:59

zephyrbot requested review from dbaluta and iuliana-prodan May 22, 2023 13:59

zephyrbot removed the DNM This PR should not be merged (Do Not Merge) label May 22, 2023

pdgendt previously approved these changes May 23, 2023

View reviewed changes

danieldegrasse dismissed pdgendt’s stale review via 0394a21 May 23, 2023 22:54

danieldegrasse force-pushed the feature/lcdif_performance branch from 27f0685 to 0394a21 Compare May 23, 2023 22:54

pdgendt previously approved these changes May 24, 2023

View reviewed changes

jfischer-no requested changes May 25, 2023

View reviewed changes

danieldegrasse added 6 commits May 25, 2023 09:31

drivers: display: enable direct framebuffer rendering with DCNANO LCDIF

5779d80

Enable direct framebuffer rendering using DCNANO LCDIF, to improve performance when the call to display_write is attempting to refresh the full display. Signed-off-by: Daniel DeGrasse <daniel.degrasse@nxp.com>

danieldegrasse dismissed pdgendt’s stale review via 16e97ad May 25, 2023 14:34

danieldegrasse force-pushed the feature/lcdif_performance branch from 0394a21 to 16e97ad Compare May 25, 2023 14:34

danieldegrasse requested a review from jfischer-no May 25, 2023 20:21

jfischer-no approved these changes May 26, 2023

View reviewed changes

jfischer-no requested review from pdgendt and gmarull May 26, 2023 11:18

pdgendt approved these changes May 26, 2023

View reviewed changes

gmarull previously requested changes May 26, 2023

View reviewed changes

danieldegrasse requested a review from gmarull May 26, 2023 14:06

mmahadevan108 merged commit 3c3132b into zephyrproject-rtos:main May 26, 2023
32 checks passed

danieldegrasse deleted the feature/lcdif_performance branch May 26, 2023 17:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

LCDIF and DCNANO performance improvements for LVGL #57421

LCDIF and DCNANO performance improvements for LVGL #57421

danieldegrasse commented May 1, 2023 •

edited

zephyrbot commented May 1, 2023 •

edited

pdgendt May 8, 2023 •

edited

danieldegrasse May 9, 2023

pdgendt May 9, 2023 •

edited

danieldegrasse May 9, 2023 •

edited

danieldegrasse May 9, 2023

pdgendt May 9, 2023

carlescufi commented May 22, 2023

jfischer-no May 25, 2023

danieldegrasse May 25, 2023

gmarull left a comment

gmarull May 26, 2023

danieldegrasse May 26, 2023

gmarull May 26, 2023

danieldegrasse May 26, 2023 •

edited

gmarull May 26, 2023

danieldegrasse May 26, 2023

gmarull May 26, 2023

		/* Wait for frame send to complete */
		k_sem_take(&dev_data->sem, K_FOREVER);

		ELCDIF_EnableInterrupts(config->base,
		kELCDIF_CurFrameDoneInterruptEnable);

LCDIF and DCNANO performance improvements for LVGL #57421

LCDIF and DCNANO performance improvements for LVGL #57421

Conversation

danieldegrasse commented May 1, 2023 • edited

zephyrbot commented May 1, 2023 • edited

pdgendt May 8, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pdgendt May 9, 2023 • edited

Choose a reason for hiding this comment

danieldegrasse May 9, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carlescufi commented May 22, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gmarull left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danieldegrasse May 26, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danieldegrasse commented May 1, 2023 •

edited

zephyrbot commented May 1, 2023 •

edited

pdgendt May 8, 2023 •

edited

pdgendt May 9, 2023 •

edited

danieldegrasse May 9, 2023 •

edited

danieldegrasse May 26, 2023 •

edited