Skip to content

Conversation

@LuskeyNoah
Copy link
Contributor

@LuskeyNoah LuskeyNoah commented Apr 29, 2024

Add support for 135x240, 240x320, 240x240 displays at 0, 90, 180, and 270 degree orientation.

closes #32286

Run clang-format on st7789v driver implementation

Signed-off-by: Noah Luskey <[email protected]>
Add support for communicating to 135x240, 240x320, 240x240 displays
at 0, 90, 180, and 270 degree orientation.

Signed-off-by: Noah Luskey <[email protected]>
@Snevzor
Copy link
Contributor

Snevzor commented May 25, 2024

Hello @LuskeyNoah, I'm also working on a similar PR for the ssd16xx driver (but focusing on setting the orientation in the DTS first in combination with correct LVGL init). I'm curious why you chose not to switch x/y_resolution in the get_capabilities function, wich is done here: https://github.com/zephyrproject-rtos/zephyr/blob/main/drivers/display/display_ili9xxx.c#L246.

I was traveling the ili9xxx road and got into an issue by switching those parameters in the get_capabilities function. LVGL init fails because this bit of glue code here: https://github.com/zephyrproject-rtos/zephyr/blob/main/modules/lvgl/lvgl.c#L107. The allocation function assumes X matches the display width and Y matches the the display height, which is not the case anymore when the display driver already has them switched. As a result, the lvgl_init fails for non-square resolutions.

I decided that the driver does not need to switch the resolutions (like with your implementation), but then LVGL needs to do it, so I think the glue code needs a "disp_drv.rotated = ..." (in lvgl_init()) based on the current orientation from the display capabilities. This is what I'm going for at the moment but I wonder if touching the glue code like that will break other display orientation implementations.

Also, based on my findings I actually think that ili9xxx + lvgl sample does not work currently when the orientation is 90/270 degrees (as initial orientation, using dts), but I cannot verify because I lack the necessary HW to test.

What are your thoughts on this?

@LuskeyNoah
Copy link
Contributor Author

I'm curious why you chose not to switch x/y_resolution in the get_capabilities function, wich is done here

@Snevzor - good catch. This is an oversight on my part. I'm pushing an update, as it looks like other drivers choose to take the approach of reporting the swapped dimensions.

@Snevzor
Copy link
Contributor

Snevzor commented Jun 1, 2024

@LuskeyNoah I'm still a bit confused about the correct approach on this. Please refer to my comment here: #73360 (comment).

@LuskeyNoah
Copy link
Contributor Author

LuskeyNoah commented Jun 9, 2024

I do not have the proper hardware to confidently/fully test this. Closing. @Snevzor please reopen and/or contribute to it if desired. I will reopen if I can fully test this.

@LuskeyNoah LuskeyNoah closed this Jun 9, 2024
@Snevzor
Copy link
Contributor

Snevzor commented Jun 9, 2024

@LuskeyNoah I unfortunately also do not have a st7789 based display laying around, I'm sorry.

@DBS06
Copy link
Contributor

DBS06 commented Feb 12, 2025

Hi, because I opened a PR for porting the Adafruit Feather ESP32-S2 (#85405) and I have a Adafruit Feather ESP32-S2 TFT which has a ST7789v and a "1.14" IPS TFT with 240x135 pixels" (actually the pixels are 135x240).
I tested the code with the samples/subsys/display/lvgl sample, the orientation change works in principal.

My first approach was this:

	int err = display_set_orientation(display_dev, DISPLAY_ORIENTATION_ROTATED_270);
	if (err == 0) {
		LOG_INF("Display orientation set to rotated 270 degrees");
	} else {
		LOG_ERR("Failed to set display orientation");
	}

The rotation works in principal but it gives me this result:
Screenshot 2025-02-12 130615

This looks like lvgl does not recognize a change in orientation from the display-driver-module. So I tried the following: set CONFIG_LV_Z_AUTO_INIT=n and called lvgl_init() after changing the orientation, but actually this had no effect at all.

After that I tried to just use lv_display_set_rotation(lv_display_get_default(), LV_DISPLAY_ROTATION_270); and had the following weird result:
Screenshot 2025-02-12 131358

By doing both display_set_orientation() and lv_display_set_rotation(), unset CONFIG_LV_Z_AUTO_INIT=n and not calling lvgl_init() manually, see code-snippet here:

	int err = display_set_orientation(display_dev, DISPLAY_ORIENTATION_ROTATED_270);
	if (err == 0) {
		LOG_INF("Display orientation set to rotated 270 degrees");
	} else {
		LOG_ERR("Failed to set display orientation");
	}
	lv_display_set_rotation(lv_display_get_default(), LV_DISPLAY_ROTATION_270);

It works as it should:
Screenshot 2025-02-12 131433

My branch can be viewed here:
https://github.com/DBS06/zephyr/tree/st7789v_implement_display_set_orientation_API

If this behavior is acceptable I would reopen this PR.

@Snevzor
Copy link
Contributor

Snevzor commented Feb 12, 2025

Hi @DBS06,

For this to work well with LVGL this is needed: #73556.
Also, in context to why this PR was abandoned, #82582 needs to be clarified fist. --> for this PR I think your best bet is to make sure the driver doesn't switch x/y resolution and then to test with the changes from #73556.

Maybe check with @danieldegrasse?

Br,
Sven

@DBS06
Copy link
Contributor

DBS06 commented Feb 12, 2025

@Snevzor Thanks for the fast response and clarification!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ST7789 driver enhancement to support display orientation

5 participants