Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ESP32-S3 support | ILI9488 is working perfectly | esp-idf v4 and v5 compatible #227

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

VitorAlho
Copy link

Fix ILI9488 init function
Fix little ILI9488 display artifacts
Auto-dma by default for all esp devices
ESP32-S3 support added
Updated to be compatible with esp-idf =< 4 and >= 5 versions

@zbpan-sh
Copy link

zbpan-sh commented Sep 8, 2023

Tried your branch with my esp32s3 and ili9488 3.5inch display... but found the displaying color is not normal. Tried updating the gamma related driver init commands to the original ones, then the problem is fixed.

the init commands in your branch:
{ILI9488_CMD_POSITIVE_GAMMA_CORRECTION, {0x0F, 0x1F, 0x1C, 0x0C, 0x0F, 0x08, 0x48, 0x98, 0x37, 0x0A, 0x13, 0x04, 0x11, 0x0D, 0x00}, 15}, {ILI9488_CMD_NEGATIVE_GAMMA_CORRECTION, {0x0F, 0x32, 0x2E, 0x0B, 0x0D, 0x05, 0x47, 0x75, 0x37, 0x06, 0x10, 0x03, 0x24, 0x20, 0x00}, 15},

the original commands:
{ILI9488_CMD_POSITIVE_GAMMA_CORRECTION, {0x00, 0x03, 0x09, 0x08, 0x16, 0x0A, 0x3F, 0x78, 0x4C, 0x09, 0x0A, 0x08, 0x16, 0x1A, 0x0F}, 15}, {ILI9488_CMD_NEGATIVE_GAMMA_CORRECTION, {0x00, 0x16, 0x19, 0x03, 0x0F, 0x05, 0x32, 0x45, 0x46, 0x04, 0x0E, 0x0D, 0x35, 0x37, 0x0F}, 15},

@VitorAlho
Copy link
Author

Tks for the feedback.

It's interesting, my ili9488 display with original gamma parameters can only display red, green and blue colors correctly. Colors like purple, pink and other ones is always in gray scale. I'll try to send a picture.

@zbpan-sh
Copy link

zbpan-sh commented Sep 8, 2023

@VitorAlho , btw, this is what it looks like using your updated driver on my display.
ili9488_esp32s3_idf502

@VitorAlho
Copy link
Author

I`m thinking about use Kconfig to include 2 versions of gamma parameters and you choose one that fits your ili9488 display

@zbpan-sh
Copy link

zbpan-sh commented Sep 8, 2023

Found a similar issue about 9488, prenticedavid/MCUFRIEND_kbv#40
Maybe there are actually different kind of 9488 versions?

@VitorAlho
Copy link
Author

This is how original parameters looks like in my ili9488 display
image

Now with my branch parameters
image

@VitorAlho
Copy link
Author

Found a similar issue about 9488, prenticedavid/MCUFRIEND_kbv#40 Maybe there are actually different kind of 9488 versions?

The craziest thing is... How could we identify different versions?

@zbpan-sh
Copy link

zbpan-sh commented Sep 8, 2023

Maybe I can do some test next week if you can share your sdkconfig. @VitorAlho

@VitorAlho
Copy link
Author

Using squareline studio, I`ve created this screen test with different collors
image

Using default gamma parameters
image

Using my branch gamma parameters
image

@VitorAlho
Copy link
Author

I`m thinking about put it inside lvgl_tft/Kconfig if the selected display was ili9488 @zbpan-sh

@zbpan-sh
Copy link

zbpan-sh commented Sep 8, 2023

I`m thinking about put it inside lvgl_tft/Kconfig if the selected display was ili9488 @zbpan-sh

I'm not sure if doing that way fits the design philosophy of the lvgl_esp32_drivers project...

@VitorAlho
Copy link
Author

VitorAlho commented Sep 8, 2023

About sdk config, here it is. I need to rename with ".txt" because github needs.
sdkconfig.txt

Thank you for your help

@zbpan-sh
Copy link

Hi, @VitorAlho, I have tried to use your lvgl configurations in your sdkconfig on my device, it ends up with the same behavior as tested before. Now I think it makes sense for add an option for the ili9488 driver to support two kinds of "gamma correction" settings, since I found there are already similar solutions for other tft drivers.

Btw. There is another pull request #223 from @hiruna, working on supporting IDF5 and LVGL8.3. Hope you guys can work together to merge your changes.

#define DMA_MAX_BIT_LENGHT (1<<24) // according with SPI_LL_DMA_MAX_BIT_LEN in spi_ll.h
#endif

#define TFT_DISPLAY_BUFFER_SIZE_OVERFLOW_PROTECTION ((TFT_DISPLAY_BUFFER_SIZE_IN_BITS>DMA_MAX_BIT_LENGHT)? (((DMA_MAX_BIT_LENGHT-1000)/8)/3) :TFT_DISPLAY_BUFFER_SIZE)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like this approach here. We can always break one larger buffer into multiple DMA requests. Limit the DISP_BUF_SIZE is not the best option. In SPI code, we can break one request into multiple without every driver to handle it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants