search

almindor / mipidsi

MIPI Display Serial Interface unified driver
MIT License
108 stars 46 forks source link

Add ILI9341 model #45

Closed rfuest closed 1 year ago

rfuest commented 1 year ago

This PR adds the ILI9341 model, that was previously added as part of #39. I've cleaned up the code some more and extracted the common code for the ILI3941 and ILI9342C controllers into a separate module.

One remaining issue is the color order: Before #36 the RGB/BGR bit for the ILI9342C was inverted. According to the datasheet this shouldn't be necessary, but without the inversion I get incorrect colors on my display. I'm not sure if the datasheet is wrong or what is going on, but I've checked that my display is indeed using the RGB subpixel order by using a magnifying glass.

rfuest commented 1 year ago

Forget what I wrote about the color order, my test program was broken.

rfuest commented 1 year ago

@georgik Could you test if this PR works as well as #39 for your display? Or if you need to set the color order to BGR to get it working?

georgik commented 1 year ago

@rfuest Thank you for the update.

I've run the tests and it seems to suffer the same problem like my PR when I copied the code from ILI9342C. Blue and red chnnel seems to be swapped. The version on PR #39 works.

Incorrect version

image

Correct version

rfuest commented 1 year ago

Thanks for testing the PR. It would be interesting to know if your display is actually using the BGR subpixel order, which would explain the difference in the colors. You could check this by drawing a white pixel in the top left corner of the display and use a magnifying glass or some other way of magnification to check if the subpixel on the edge of the display is red or green. If you try this it's important that no display rotation is enabled.

But if you just want to fix your display it should be enough to add .with_color_order(ColorOrder::Bgr) to the display initialization.

georgik commented 1 year ago

@rfuest Thank you very much for the suggestion. Using explicit with_color_order seems to be good option, because there are different displays with different color channel order. E.g. Kaluga is using RGB while M5Stack is using BGR.

rfuest commented 1 year ago

@georgik Good to hear that it works for you. @almindor The PR is ready to review/merge now.

almindor commented 1 year ago

Thank you