Renesas: Smartbond: Add Display Driver Support #67649

ioannis-karachalios · 2024-01-16T06:36:39Z

No description provided.

zephyrbot · 2024-01-16T06:37:53Z

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

Name	Old Revision	New Revision	Diff

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

ioannis-karachalios · 2024-03-05T21:23:19Z

@danieldegrasse is there any further feedback? Could you remove the platform: ITE label added mistakenly after the last rebase?

danieldegrasse · 2024-03-05T21:06:51Z

boards/renesas/da1469x_dk_pro/Kconfig.defconfig

+if LVGL
+
+# LCDC imposes display buffer be word aligned
+config LV_Z_VDB_ALIGN


This is the default value, and can be dropped.

danieldegrasse · 2024-03-05T21:12:31Z

drivers/display/display_renesas_lcdc.c

+	return ret;
+}
+
+static int display_smartbond_set_orientation(const struct device *dev,


Is this implementation needed? The display API no longer requires all functions be implemented, it will return -ENOSYS if one isn't present.

danieldegrasse · 2024-03-05T21:17:05Z

drivers/display/display_renesas_lcdc.c

+	 * directly by LCDC and is allocated statically during device initialization. The color
+	 * format is defined based on the pixel-format property dictated by lcd-controller
+	 * bindings.
+	 */


Please be consistent here as far as multi-instance versus single instance code. For example, if you wanted to make this block entirely multi instance, it would look something like this:

capabilities->supported_pixel_formats = config->pixel_format capabilities->current_orientation = DISPLAY_ORIENTATION_NORMAL; capabilities->current_pixel_format = config->pixel_format capabilities->x_resolution = config->x_res; capabilities->y_resolution = config->y_res;

Alternatively, an entirely single instance version would look like this:

capabilities->supported_pixel_formats = DT_INST_PROP(0, pixel_format); capabilities->current_orientation = DISPLAY_ORIENTATION_NORMAL; capabilities->current_pixel_format = DT_INST_PROP(0, pixel_format); capabilities->x_resolution = DT_INST_PROP(inst, width); capabilities->y_resolution = DT_INST_PROP(inst, height);

Personally, I would strongly prefer the driver is implemented as multi instance, since that is more scalable in the future.

This is something we've already discussed - I'd prefer to keep this a single instance driver and revisit it in due course when there is necessity to do so.

danieldegrasse · 2024-03-05T21:22:32Z

drivers/display/display_renesas_lcdc.c

+				const uint16_t x, const uint16_t y,
+				const struct display_buffer_descriptor *desc,
+				const void *buf)
+{


I'm not familiar with this LCD controller, but something that has been implemented for the ELCDIF (which is designed to stream a buffer in memory directly to the display) is to switch to the user provided buffer directly if the buffer is the same size as the output display.

zephyr/drivers/display/display_mcux_elcdif.c

Line 94 in eaf140e

full_fb = true;

I'm not sure if this is something your controller supports, but it might be worth investigating- it improved performance quite a bit using LVGL with the ELCDIF

No sure if this is going to work in our case. The main reasons is that LCDC imposes that pixel buffers be word aligned as well as the stride size (number of bytes per row). If you see, when I allocate the internal buffer I am making sure the necessary padding is added, if necessary. In addition, the copy from application to the internal buffer is done via the DMA accelerator mainly due to the stride alignment requirement as well as time efficiency. In fact, even if there was not the stride requirement, using memcpy should result in making the device unresponsive when partial updates take place all the time. What is also not clear to me is, if LCDC employs application buffer directly then any rending performed during frame update shouldn't result in visual artifacts?

What is also not clear to me is, if LCDC employs application buffer directly then any rending performed during frame update shouldn't result in visual artifacts?

The trick here would be to queue the application buffer somehow, and not load it into the LCDC hardware until the last frame was done streaming. Again, I am not familiar with the hardware here- so this should be taken as an idea, nothing more. No need to implement it if it is not possible or not worth looking into

I see - such an architectural scheme should not work for us. However, I will spend some time on the underlying implementation :)

danieldegrasse · 2024-03-05T21:22:59Z

samples/drivers/display/boards/da1469x_dk_pro_lcdc.overlay

+
+	/*
+	 * Panel settings for the NHD-4.3-480272EF-ASXP-CTP
+     * display panel model which integrates the SC7283


nit- fix indentation

Indentation still looks wrong here

danieldegrasse · 2024-03-05T21:27:15Z

samples/modules/lvgl/demos/boards/da1469x_dk_pro_lcdc.overlay

I think if we are going to add overlays for each display supported with this board, it really might be valuable to consider moving these to the board dts folder. There are other display samples in tree (and I suspect some customers will develop using the development kit and this display initially), so putting the overlays in the board's dts folder will simplify supporting other display samples or customer applications

There should be parts that are not common for all sample codes/applications e.g. LVGL and input pointer. Then, one should create another overlay under application's board folder to add the extra parts, isn't it? In addition, any overlay should be explicitly invoked during west build since multiple overlays are expected per LCD interface type i.e. da1469x_dk_pro_lcdc.overlay, da1469x_dk_pro_mipi_dbi.overlay. This is something we discussed in the MIPI DBI PR since the same LCDC IP is used for different driver classes.

Sure- but defining the LVGL/input pointer in an application that does not use CONFIG_LVGL=y will have no effect. I think you could probably simplify this to two overlays, one for the LCDC IP using the MIPI DBI driver class, and one for the LCDC IP using the display driver class.

OK - I was a bit of unsure whether defining blocks that are not used would be acceptable or not.

Exhibit Renesas LCD controller's driver implementation. The driver is intended to employ the controller in the continuous mode so it can drive display panels in the parallel RGB mode. Signed-off-by: Ioannis Karachalios <ioannis.karachalios.px@renesas.com>

danieldegrasse · 2024-03-07T17:50:03Z

dts/arm/renesas/smartbond/da1469x.dtsi

@@ -332,6 +332,13 @@
 			interrupts = <9 0>;
 		};

+		display: lcdc@30030000 {


Instead of defining the same IP block twice with different compatibles, you should override the compatible of the device in the overlay file, something like so:

da1469x.dtsi:

lcdc: lcdc@30030000 { compatible = "renesas,smartbond-display"; reg = <0x30030000 0x18C>; interrupts = <32 0>; status = "disabled"; };

da1469x_dk_pro_mipi_dbi.overlay

&lcdc { status = "okay"; compatible = "renesas,smartbond-mipi-dbi"; };

Thanks for the hint. I followed your suggestion but I am now unable to build. In specific, when I try to build for the display compatible via
west build -p always -b da1469x_dk_pro samples/drivers/display/ -- -DDTC_OVERLAY_FILE="da1469x_dk_pro_lcdc.overlay" I am getting errors on missing properties of the MIPI DBI compatible. The same goes if I build for the MIPI DBI compatible. Could you please check?

Forget my previous comment; I messed up the compatible properties between the display and MIPI DBI overlays :/

No worries! Thanks for getting this changed :)

Define a single node that reflects the LCDC IP. Instead of defining the same IP block twice with different compatibles (mipi dbi, display) we define a single node for the default display interface and other interfaces like the MIPI DBI should override the compatible entry with the appropriate one within its DTS overlay file. Signed-off-by: Ioannis Karachalios <ioannis.karachalios.px@renesas.com>

Update board's DTS configurations to support the Renesas LCD controller. Signed-off-by: Ioannis Karachalios <ioannis.karachalios.px@renesas.com>

In order to avoid defining almost the same overlays for different sample codes and/or applications a common overlay file per display interface is defined under the boards dts folder. In doing so, an application/sample code will only have to define another overlay explicitly under application's board folder if more blocks are to be enabled. In either case, users should explicitly invoke the requested overfiles at 'west build' invokation by using the DTC_OVERLAY_FILE system variable. Signed-off-by: Ioannis Karachalios <ioannis.karachalios.px@renesas.com>

ioannis-karachalios · 2024-03-07T21:23:37Z

@danieldegrasse many thanks for your contribution to this PR.

zephyrbot added area: Display area: Devicetree Binding PR modifies or adds a Device Tree binding platform: Renesas SmartBond Renesas Electronics Corporation, SmartBond area: Samples Samples labels Jan 16, 2024

zephyrbot requested review from andrzej-kaczmarek, galak, jfischer-no, kartben and nashif January 16, 2024 06:37

zephyrbot assigned kartben Jan 16, 2024

zephyrbot added manifest manifest-hal_renesas DNM This PR should not be merged (Do Not Merge) labels Jan 16, 2024

ioannis-karachalios force-pushed the da1469x-display branch 6 times, most recently from dff150c to 4d83ce0 Compare January 19, 2024 14:43

zephyrbot removed the DNM This PR should not be merged (Do Not Merge) label Jan 19, 2024

ioannis-karachalios force-pushed the da1469x-display branch 4 times, most recently from cef35f3 to f2f9beb Compare January 25, 2024 12:17

zephyrbot added the DNM This PR should not be merged (Do Not Merge) label Jan 25, 2024

ioannis-karachalios force-pushed the da1469x-display branch 3 times, most recently from a5df263 to 9a75397 Compare January 30, 2024 11:30

kartben requested a review from faxe1008 January 30, 2024 11:34

zephyrbot requested review from jackrosenthal, keith-zephyr and sjg20 March 5, 2024 21:06

ioannis-karachalios force-pushed the da1469x-display branch 2 times, most recently from 83a4dec to 62c8ff9 Compare March 5, 2024 21:16

danieldegrasse reviewed Mar 5, 2024

View reviewed changes

ioannis-karachalios force-pushed the da1469x-display branch from 62c8ff9 to ec6003b Compare March 6, 2024 10:14

ioannis-karachalios force-pushed the da1469x-display branch 2 times, most recently from ec61756 to 6a8ebc2 Compare March 6, 2024 10:57

ioannis-karachalios requested a review from danieldegrasse March 6, 2024 11:12

ioannis-karachalios force-pushed the da1469x-display branch 2 times, most recently from c97a6f4 to 93b3348 Compare March 6, 2024 16:45

danieldegrasse reviewed Mar 7, 2024

View reviewed changes

ioannis-karachalios force-pushed the da1469x-display branch from 93b3348 to 006b8d0 Compare March 7, 2024 19:18

boards: renesas: smartbond: Add support for the display driver class.

516275c

Update board's DTS configurations to support the Renesas LCD controller. Signed-off-by: Ioannis Karachalios <ioannis.karachalios.px@renesas.com>

ioannis-karachalios force-pushed the da1469x-display branch 3 times, most recently from 90fa015 to 5482701 Compare March 7, 2024 19:31

ioannis-karachalios requested a review from danieldegrasse March 7, 2024 19:45

ioannis-karachalios force-pushed the da1469x-display branch from 5482701 to 6dda7c0 Compare March 7, 2024 19:51

danieldegrasse approved these changes Mar 7, 2024

View reviewed changes

andrzej-kaczmarek approved these changes Mar 8, 2024

View reviewed changes

fabiobaltieri approved these changes Mar 8, 2024

View reviewed changes

fabiobaltieri assigned danieldegrasse Mar 8, 2024

fabiobaltieri merged commit 6afea7c into zephyrproject-rtos:main Mar 8, 2024
22 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Renesas: Smartbond: Add Display Driver Support #67649

Renesas: Smartbond: Add Display Driver Support #67649

ioannis-karachalios commented Jan 16, 2024

zephyrbot commented Jan 16, 2024 •

edited

ioannis-karachalios commented Mar 5, 2024

danieldegrasse Mar 5, 2024

danieldegrasse Mar 5, 2024

danieldegrasse Mar 5, 2024

ioannis-karachalios Mar 6, 2024

danieldegrasse Mar 5, 2024

ioannis-karachalios Mar 6, 2024 •

edited

danieldegrasse Mar 6, 2024

ioannis-karachalios Mar 6, 2024

danieldegrasse Mar 5, 2024

danieldegrasse Mar 6, 2024

danieldegrasse Mar 5, 2024

ioannis-karachalios Mar 6, 2024 •

edited

danieldegrasse Mar 6, 2024

ioannis-karachalios Mar 6, 2024 •

edited

danieldegrasse Mar 7, 2024

ioannis-karachalios Mar 7, 2024

ioannis-karachalios Mar 7, 2024

danieldegrasse Mar 7, 2024

ioannis-karachalios commented Mar 7, 2024

Renesas: Smartbond: Add Display Driver Support #67649

Renesas: Smartbond: Add Display Driver Support #67649

Conversation

ioannis-karachalios commented Jan 16, 2024

zephyrbot commented Jan 16, 2024 • edited

ioannis-karachalios commented Mar 5, 2024

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

Choose a reason for hiding this comment

ioannis-karachalios Mar 6, 2024 • 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ioannis-karachalios Mar 6, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ioannis-karachalios Mar 6, 2024 • 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

Choose a reason for hiding this comment

ioannis-karachalios commented Mar 7, 2024

zephyrbot commented Jan 16, 2024 •

edited

ioannis-karachalios Mar 6, 2024 •

edited

ioannis-karachalios Mar 6, 2024 •

edited

ioannis-karachalios Mar 6, 2024 •

edited