ILI9341 driver and MSP2401/MSP2402 displays

[adeuring]

Playing with an ESP32 module, micro-gui and a Chinese ILI9341 module as documented here: http://www.lcdwiki.com/2.4inch_SPI_Module_ILI9341_SKU:MSP2402 , I got quite distorted images on the display. For the values of constructor parameters of class ILI9341 width=320, height=240, usd=False, the display shows this when the "aclock" example is running:

With width=320, height=240, usd=True:

With width=240, height=320, usd=False:

With width=240, height=320, usd=True:

The "skew" of the circle in the third and forth images let me suspect that the "column address" setting, ILI9341 command 0x2A, and/or "page address" (as called in the data sheet – should be "row address" or "row number range" in my understanding...), ILI9341 commad 0x2B, could be off by 1, so I tried this patch:

+++ b/drivers/ili93xx/ili9341.py
@@ -126,8 +126,8 @@ class ILI9341(framebuf.FrameBuffer):
         if self._spi_init:  # A callback was passed
             self._spi_init(self._spi)  # Bus may be shared
         # Commands needed to start data write 
-        self._wcd(b'\x2a', int.to_bytes(self.width, 4, 'big'))  # SET_COLUMN
-        self._wcd(b'\x2b', int.to_bytes(ht, 4, 'big'))  # SET_PAGE
+        self._wcd(b'\x2a', int.to_bytes(self.width-1, 4, 'big'))  # SET_COLUMN
+        self._wcd(b'\x2b', int.to_bytes(ht-1, 4, 'big'))  # SET_PAGE
         self._wcmd(b'\x2c')  # WRITE_RAM
         self._dc(1)
         self._cs(0)
@@ -147,8 +147,8 @@ class ILI9341(framebuf.FrameBuffer):
             lb = self._linebuf
             buf = self._mvb
             # Commands needed to start data write 
-            self._wcd(b'\x2a', int.to_bytes(self.width, 4, 'big'))  # SET_COLUMN
-            self._wcd(b'\x2b', int.to_bytes(ht, 4, 'big'))  # SET_PAGE
+            self._wcd(b'\x2a', int.to_bytes(self.width-1, 4, 'big'))  # SET_COLUMN
+            self._wcd(b'\x2b', int.to_bytes(ht-1, 4, 'big'))  # SET_PAGE
             self._wcmd(b'\x2c')  # WRITE_RAM
             self._dc(1)
             line = 0

This patch fixes the "skew". Example for the parameters width=240, height=320, usd=False:

The "noisy parts" of the display still remain, and the proper selection of landscape/portrait orientation still does not work. This can be fixed by changing the parameters for the ILI9341 command 0x36, "Memory Access Control":

+++ b/drivers/ili93xx/ili9341.py
@@ -80,9 +80,9 @@ class ILI9341(framebuf.FrameBuffer):
         self._wcd(b'\xc7', b'\x86')  # VMCTR2 VCOM ctrl 2
         # (b'\x88', b'\xe8', b'\x48', b'\x28')[rotation // 90]
         if self.height > self.width:
-            self._wcd(b'\x36', b'\x48' if usd else b'\x88')  # MADCTL: RGB portrait mode
+            self._wcd(b'\x36', b'\xe8' if usd else b'\x28')  # MADCTL: RGB portrait mode
         else:
-            self._wcd(b'\x36', b'\x28' if usd else b'\xe8')  # MADCTL: RGB landscape mode
+            self._wcd(b'\x36', b'\x88' if usd else b'\x48')  # MADCTL: RGB landscape mode
         self._wcd(b'\x37', b'\x00')  # VSCRSADD Vertical scrolling start address
         self._wcd(b'\x3a', b'\x55')  # PIXFMT COLMOD: Pixel format 16 bits (MCU & interface)
         self._wcd(b'\xb1', b'\x00\x18')  # FRMCTR1 Frame rate ctrl

The display now looks as expected. Parameters width=320, height=240 usd=False:

width=320, height=240 usd=True:

width=240, height=320 usd=False:

width=240, height=320 usd=True:

I do not (yet) submit a pull request for these simple changes since I assume that the unmodified code works well for the Adafruit display. Would it make sense to add another parameter to ILI9341.init() like "module_type" that could be either "Adafruit" or "MSP240x" that would select between the original parameters and my variants?

While trying to find the right values for the command 0x36, I noticed that it is quite easy to display mirrored images; Just flip bit 7. I don't have any personal use case for a mirrored display but just in case somebody can need it: What about adding another parameter "mirrored=False" to ILI9341.init()?

[beetlegigg]

Quote " I assume that the unmodified code works well for the Adafruit display "

Just a note to say my Chinese module purchased on Amazon - CNBTR Red 2.8"" SPI TFT LCD Touch Panel Serial Port Module With PBC ILI9341 Support Serial SPI Mode -
works perfectly, so its not just the Adafruit displays the current code is working well on.

So if any mods to the code to select a model type should not just be Adafruit or MSP240z as that would confuse other ILI9341 users like me :-)

[adeuring]

So if any mods to the code to select a model type should not just be Adafruit or MSP240z as that would confuse other ILI9341 users like me :-)

Good point. So what about ILI9341.__init__(..., module_type=None), the other allowed value would be the string 'msp240x'.

[peterhinch]

The problem with display drivers is that often there are different ways of connecting the driver chip to the display, with a need for matching code differences. FWIW I have used the driver with Chinese modules - but evidently sources differ.

Please could you try the ILI9486 driver docs and code. This is a very minimal driver which does the rotation in code rather than in the chip. This driver works with my Chinese ILI9341 displays. I would be interested to hear the outcome.

[adeuring]

Please could you try the ILI9486 driver docs and code. This is a very minimal driver which does the rotation in code rather than in the chip. This driver works with my Chinese ILI9341 displays. I would be interested to hear the outcome.

The driver works – but the displayed image is mirrored, both in landscape and portrait orientation and for usd=True and usd=False. With the following patch the displayed image looks fine:

--- a/drivers/ili94xx/ili9486.py
+++ b/drivers/ili94xx/ili9486.py
@@ -66,7 +66,7 @@ class ILI9486(framebuf.FrameBuffer):
         return cls.COLOR_INVERT ^ ((r & 0xF8) | (g & 0xE0) >> 5 | (g & 0x1C) << 11 | (b & 0xF8) << 5)
 
     # Transpose width & height for landscape mode
-    def __init__(self, spi, cs, dc, rst, height=320, width=480, usd=False, init_spi=False):
+    def __init__(self, spi, cs, dc, rst, height=320, width=480, usd=False, init_spi=False, mirror=False):
         self._spi = spi
         self._cs = cs
         self._dc = dc
@@ -107,7 +107,10 @@ class ILI9486(framebuf.FrameBuffer):
         # Default page address start == 0 end == 0x1DF (479)
         if self._long != 480:
             self._wcd(b"\x2b", int.to_bytes(self._long - 1, 4, "big"))  # SET_PAGE ht
-        self._wcd(b"\x36", b"\x48" if usd else b"\x88")  # MADCTL: RGB portrait mode
+        madctl = 0x48 if usd else 0x88
+        if mirror:
+            madctl ^= 0x80
+        self._wcd(b"\x36", madctl.to_bytes(1, 'big'))  # MADCTL: RGB portrait mode
         self._wcmd(b"\x11")  # sleep out
         self._wcmd(b"\x29")  # display on

[peterhinch]

Thank you for testing. I have pushed updates to micro-gui and nano-gui. The only change is to order the constructor args so that mirror is adjacent to usd, which seems a logical pairing.

DRIVERS.md is amended to point users to this solution and to credit you with the patch.