drivers: display: introduce G1120B0MIPI watch display #57578

danieldegrasse · 2023-05-05T04:25:45Z

This PR introduces the G1120B0MIPI watch display. It includes the following updates:

display driver for RM67162 display controller on the G1120B0 display
updates to the FT5336 input driver to handle the reset GPIO present on some focaltech chips, and to clarify the driver supports the FT3267 IC
updates to both MCUX DSI drivers to fix incorrect handling of DCS_LONG_WRITE commands, as well as to respect the MODE_VIDEO flag, which indicates a display uses constant refreshing.

This display has been verified to function with the RT1170 and RT595 EVKs.

~~Note that this PR currently depends on #53907 and #55493, and contains commits from both. The commits will be removed as those PRs merge.~~

Signed-off-by: Daniel DeGrasse daniel.degrasse@nxp.com

drivers/mipi_dsi/dsi_mcux_2l.c

dts/arm/nxp/nxp_rt5xx_common.dtsi

drivers/display/display_rm67162.c

boards/shields/g1120b0mipi/g1120b0mipi.overlay

fabiobaltieri · 2023-07-17T16:35:54Z

drivers/mipi_dsi/dsi_mcux_2l.c

+#define MCUX_DSI_DPI_CONFIG(id)									\
+	IF_ENABLED(DT_NODE_HAS_PROP(DT_DRV_INST(id), nxp_lcdif),				\
+	(.dpi_config = {									\
+		.dpiColorCoding = DT_INST_ENUM_IDX(id, dpi_color_coding),			\


How about having the macro at this level? Like

#define MCUX_DSI_DPI_CONFIG_INIT(id) { \ .dpiColorCoding = ...

That's what I had in mind when I suggested to bring it out (sorry I was unclear), thinking that doing that would get rid of another tab and then you'd actually be able to keep more stuff inline.

The kernel structure have these all over, example: https://github.com/zephyrproject-rtos/zephyr/blob/main/include/zephyr/kernel.h#L695-L710

Sorry I misunderstood- thanks for clarifying. Should be fixed to this format in latest push

Thanks, I feel a bit bad that after making you redo this twice most lines still don't fit without wrapping by just one character but it's probably good to have it split out anyway :-)

No worries- still looks cleaner overall

gmarull

some minor nitpicks

gmarull · 2023-07-19T11:22:07Z

boards/shields/g1120b0mipi/Kconfig.defconfig

+if DISPLAY
+
+# Enable MIPI DSI, as this display controller requires it.
+
+config MIPI_DSI
+	default y
+
+endif # DISPLAY


note: I think we can safely follow the select MIPI_DSI pattern here.

gmarull · 2023-07-19T11:23:19Z

boards/shields/g1120b0mipi/g1120b0mipi.overlay

+		 * Note- the actual controller present on this IC is a FT3267,
+		 * but the FT35336 driver in Zephyr supports this IC.
+		 */
+		compatible = "focaltech,ft5336";


Suggested change

compatible = "focaltech,ft5336";

compatible = "focaltech,ft3267";

and adjust driver to support multiple compatibles?

Oh, just noticed a typo in the comment too: "FT35336"

The input driver here already supports multiple devices (although only the one compatible). Should we instead rename the compatible to focaltech,ftxxxx? We can keep the focaltech,ft5336 compatible so that downsteam devicetrees aren't affected.

Renaming it is a bit of a nightmare, it'd mess up downstream applications. I think we should avoid that for now since we don't have a well defined format for handling driver with multiple compatibles. I'd say do the def-undef trick (like https://github.com/zephyrproject-rtos/zephyr/blob/main/drivers/sensor/tmp108/tmp108.c#L406 but you can use the same init macro), though that does not scale that well. @gmarull how about having two compatibles in it? Unsure what's less worse, wish we had a proper framework for this case were we could add specific compatibles without causing regressions downstream.

I agree, about the renaming being an issue- I don't want to mess up applications downstream here. Maybe we use the def-undef trick to cover the following compatibles:

focaltech,ft5336

focaltech,ft5xx6

focaltech,ft6xx6

focaltech,ft3267

I'd prefer to leave the Kconfig itself untouched, as that would also affect downstream applications

I'm also siding for leaving this as is, counting on the ADI folks to come up with something more convenient down the road to handle driver families.

It definitely can be fixed later, it's not a blocker to me, just noting this needs to be improved/fixed.

gmarull · 2023-07-19T11:23:39Z

drivers/display/Kconfig.rm67162

+config RM67162
+	bool "RM67162 display driver"
+	default y
+	depends on MIPI_DSI


see above, select

gmarull · 2023-07-19T11:24:12Z

drivers/display/display_rm67162.c

+		gpio_pin_set_dt(&config->reset_gpio, 0);
+		/* Per datasheet, reset low pulse width should be at least 10usec */
+		k_sleep(K_USEC(30));
+		gpio_pin_set_dt(&config->reset_gpio, 1);


missing error handling here

gmarull · 2023-07-19T11:24:50Z

drivers/input/input_ft5336.c

+		/* Delay 5 ms to reset controller */
+		k_sleep(K_MSEC(5));


where are these 5ms documented?

I've updated the comment based on the datasheet. It appears that the minimum reset time is 1ms, but I would rather keep the longer delay to be safe (as the reset time needs to be extended if the controller is just powering on)

Add DSI video mode flag to MIPI configuration, to indicate to MIPI drivers that this display uses video mode and must be refreshed constantly. Signed-off-by: Daniel DeGrasse <daniel.degrasse@nxp.com>

Make DPI mode an optional configuration for the DSI MCUX 2L driver. DPI mode will only be enabled when the MIPI is attached in video mode, since this is when DPI formatted packets are expected. This will enable the DSI driver to also support DBI/command mode, for displays that use this format. Signed-off-by: Daniel DeGrasse <daniel.degrasse@nxp.com>

Fixup support for DCS_LONG_WRITE command in DSI MCUX 2L driver. Since long DCS commands may benefit from nonblocking I/O, add support for non blocking transfers to the DSI driver. This commit also corrects the interrupt number for the RT595, which uses the DSI_MCUX_2L IP block. Signed-off-by: Daniel DeGrasse <daniel.degrasse@nxp.com>

Only setup DPI input from LCDIF if MODE_VIDEO is set, as this is the the only case where input from the LCDIF would be required to drive the display. Do not populate the dpi_config structure unless a reference the the NXP LCDIF device is provided, since this is the output device providing DPI data. Signed-off-by: Daniel DeGrasse <daniel.degrasse@nxp.com>

Fix support for DCS long write command in DSI mcux driver, to enable use with displays that require this command for full support. Signed-off-by: Daniel DeGrasse <daniel.degrasse@nxp.com>

Only set LV_Z_VDB_CUSTOM_SECTION for the RK055HDMIPI4M shield, as it is only required when using displays that require a large framebuffer. Signed-off-by: Daniel DeGrasse <daniel.degrasse@nxp.com>

Add support for RM67162 MIPI display controller. This controller is configured to run in MIPI command/DBI mode, driving a 400x392 OLED display. Signed-off-by: Daniel DeGrasse <daniel.degrasse@nxp.com>

Add support for resetting controller at boot, and update FT5336 documentation to indicate that the FT3267 IC is also supported by this driver. Signed-off-by: Daniel DeGrasse <daniel.degrasse@nxp.com>

Add g1120b0mipi watch display to shields. This shield is currently supported with the RT595 and RT1170 EVK. The shield contains a 400x392 circular OLED watch display, with an integrated RM67162 display controller Note that the touchscreen IC can be driven by the FT5336 driver, with the following changes: - the FT3267 reports touch data as inverted from the FT5336, so LVGL must be made aware of this via CONFIG_LV_Z_POINTER_KSCAN_INVERT_Y - the FT3267 reports the X and Y coordinates as swapped from the FT5336. To resolve this, the RM67162 driver reports the display as rotated by 90 degrees. Signed-off-by: Daniel DeGrasse <daniel.degrasse@nxp.com>

danieldegrasse · 2023-07-24T14:03:50Z

@gmarull can you verify that your comments have been addressed?

danieldegrasse force-pushed the feature/rm67162_display branch from dd3e157 to 6770484 Compare May 5, 2023 14:34

danieldegrasse mentioned this pull request May 5, 2023

drivers: dma: add SMARTDMA driver, and enable on G1120B0 display #57616

Merged

github-actions bot added the Stale label Jul 5, 2023

zephyrproject-rtos deleted a comment from github-actions bot Jul 5, 2023

danieldegrasse removed the Stale label Jul 5, 2023

danieldegrasse force-pushed the feature/rm67162_display branch 2 times, most recently from 8295130 to b889404 Compare July 12, 2023 15:57

danieldegrasse marked this pull request as ready for review July 12, 2023 18:37

danieldegrasse requested review from nashif, mmahadevan108, dleach02, galak and erwango as code owners July 12, 2023 18:37

zephyrbot added area: Shields Shields (add-on boards) area: Display area: Input Input Subsystem and Drivers platform: NXP NXP area: Devicetree Binding PR modifies or adds a Device Tree binding labels Jul 12, 2023

zephyrbot requested review from avisconti, dbaluta, decsny, DerekSnell, EmilioCBen, fabiobaltieri, iuliana-prodan, jfischer-no, manuargue and yvanderv July 12, 2023 18:38

zephyrbot assigned dleach02 Jul 12, 2023

fabiobaltieri reviewed Jul 14, 2023

View reviewed changes

danieldegrasse force-pushed the feature/rm67162_display branch 2 times, most recently from a93a930 to 9fb0296 Compare July 17, 2023 15:39

fabiobaltieri reviewed Jul 17, 2023

View reviewed changes

danieldegrasse force-pushed the feature/rm67162_display branch 2 times, most recently from 4c27347 to d567341 Compare July 18, 2023 18:22

fabiobaltieri previously approved these changes Jul 18, 2023

View reviewed changes

jfischer-no previously approved these changes Jul 19, 2023

View reviewed changes

gmarull reviewed Jul 19, 2023

View reviewed changes

danieldegrasse dismissed stale reviews from jfischer-no and fabiobaltieri via 7f7d4c2 July 20, 2023 14:10

danieldegrasse force-pushed the feature/rm67162_display branch from d567341 to 7f7d4c2 Compare July 20, 2023 14:10

danieldegrasse added 9 commits July 20, 2023 09:13

drivers: display: display_rm68200: add DSI video mode flag

6e55a43

Add DSI video mode flag to MIPI configuration, to indicate to MIPI drivers that this display uses video mode and must be refreshed constantly. Signed-off-by: Daniel DeGrasse <daniel.degrasse@nxp.com>

drivers: mipi_dsi: dsi_mcux: fix support for DCS_LONG_WRITE command

1f96316

Fix support for DCS long write command in DSI mcux driver, to enable use with displays that require this command for full support. Signed-off-by: Daniel DeGrasse <daniel.degrasse@nxp.com>

boards: mimxrt595_evk_cm33: only set LV_Z_VDB_CUSTOM_SECTION for shield

7b98d9d

Only set LV_Z_VDB_CUSTOM_SECTION for the RK055HDMIPI4M shield, as it is only required when using displays that require a large framebuffer. Signed-off-by: Daniel DeGrasse <daniel.degrasse@nxp.com>

drivers: display: add support for RM67162 controller

49fa159

Add support for RM67162 MIPI display controller. This controller is configured to run in MIPI command/DBI mode, driving a 400x392 OLED display. Signed-off-by: Daniel DeGrasse <daniel.degrasse@nxp.com>

drivers: input: ft5336: Add support for reset GPIO and FT3267 IC

26edf01

Add support for resetting controller at boot, and update FT5336 documentation to indicate that the FT3267 IC is also supported by this driver. Signed-off-by: Daniel DeGrasse <daniel.degrasse@nxp.com>

danieldegrasse force-pushed the feature/rm67162_display branch from 7f7d4c2 to 33fca87 Compare July 20, 2023 14:17

fabiobaltieri approved these changes Jul 20, 2023

View reviewed changes

erwango approved these changes Jul 24, 2023

View reviewed changes

jfischer-no approved these changes Jul 24, 2023

View reviewed changes

gmarull approved these changes Jul 24, 2023

View reviewed changes

carlescufi merged commit f907f95 into zephyrproject-rtos:main Jul 25, 2023
18 checks passed

danieldegrasse deleted the feature/rm67162_display branch July 28, 2023 21:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

drivers: display: introduce G1120B0MIPI watch display #57578

drivers: display: introduce G1120B0MIPI watch display #57578

danieldegrasse commented May 5, 2023 •

edited

fabiobaltieri Jul 17, 2023 •

edited

danieldegrasse Jul 18, 2023

fabiobaltieri Jul 18, 2023

danieldegrasse Jul 18, 2023

gmarull left a comment

gmarull Jul 19, 2023

gmarull Jul 19, 2023

fabiobaltieri Jul 19, 2023

danieldegrasse Jul 19, 2023 •

edited

fabiobaltieri Jul 19, 2023

danieldegrasse Jul 19, 2023

fabiobaltieri Jul 20, 2023

gmarull Jul 20, 2023

gmarull Jul 19, 2023

gmarull Jul 19, 2023

gmarull Jul 19, 2023

danieldegrasse Jul 20, 2023

danieldegrasse commented Jul 24, 2023

	compatible = "focaltech,ft5336";
	compatible = "focaltech,ft3267";

drivers: display: introduce G1120B0MIPI watch display #57578

drivers: display: introduce G1120B0MIPI watch display #57578

Conversation

danieldegrasse commented May 5, 2023 • edited

fabiobaltieri Jul 17, 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

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 Jul 19, 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

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

danieldegrasse commented Jul 24, 2023

danieldegrasse commented May 5, 2023 •

edited

fabiobaltieri Jul 17, 2023 •

edited

danieldegrasse Jul 19, 2023 •

edited