juj / fbcp-ili9341

A blazing fast display driver for SPI-based LCD displays for Raspberry Pi A, B, 2, 3, 4 and Zero
MIT License
1.61k stars 268 forks source link

2.0" ST7789 display wraps right 1/4 around to the left #104

Open unphased opened 5 years ago

unphased commented 5 years ago

https://www.adafruit.com/product/4311

I noticed that this topic https://github.com/juj/fbcp-ili9341/issues/91 is also about a 320x240 display, but curiously if I change the code to set DISPLAY_NATIVE_WIDTH to 320 instead of the HEIGHT, what ends up being displayed is mirror flipped and unreadable.

So, here is the diff showing what I edited in the code to support this display:

diff --git a/st7735r.h b/st7735r.h
index 789b4b6..2801309 100644
--- a/st7735r.h
+++ b/st7735r.h
@@ -17,7 +17,7 @@

 #if defined(ST7789) || defined(ST7789VW)
 #define DISPLAY_NATIVE_WIDTH 240
-#define DISPLAY_NATIVE_HEIGHT 240
+#define DISPLAY_NATIVE_HEIGHT 320
 #elif defined(ST7735R)
 #define DISPLAY_NATIVE_WIDTH 128
 #define DISPLAY_NATIVE_HEIGHT 160

This yields a result of:

IMG_4463

As you can see if you look closely, the raspberries on the top should be aligned to the left. What appears to be happening is that the left-most (or top most?) 80 pixels are displaying the right-most 80 pixels of the buffer, it is wrapping...

I'd like to just inquire if anyone else has tried this display and encountered this or could comment on what i might be able to edit to cleanly resolve the issue.

unphased commented 5 years ago

I've gotten the offset fixed by having the code changed to:

diff --git a/st7735r.cpp b/st7735r.cpp
index ceac386..5b73c3f 100644
--- a/st7735r.cpp
+++ b/st7735r.cpp
@@ -92,7 +92,7 @@ void InitST7735R()
     // memory in row addresses Y = 319-(0...239) = 319...80 range. To view this range, we must scroll the view by +80 units in Y
     // direction so that contents of Y=80...319 is displayed instead of Y=0...239.
     if ((madctl & MADCTL_ROW_ADDRESS_ORDER_SWAP))
-      SPI_TRANSFER(0x37/*VSCSAD: Vertical Scroll Start Address of RAM*/, 0, 320 - DISPLAY_WIDTH);
+      SPI_TRANSFER(0x37/*VSCSAD: Vertical Scroll Start Address of RAM*/, 0, 0);
 #endif

     // TODO: The 0xB1 command is not Frame Rate Control for ST7789VW, 0xB3 is (add support to it)
diff --git a/st7735r.h b/st7735r.h
index 789b4b6..2801309 100644
--- a/st7735r.h
+++ b/st7735r.h
@@ -17,7 +17,7 @@

 #if defined(ST7789) || defined(ST7789VW)
 #define DISPLAY_NATIVE_WIDTH 240
-#define DISPLAY_NATIVE_HEIGHT 240
+#define DISPLAY_NATIVE_HEIGHT 320
 #elif defined(ST7735R)
 #define DISPLAY_NATIVE_WIDTH 128
 #define DISPLAY_NATIVE_HEIGHT 160

Unfortunately it looks like we need more #ifdefs...

juj commented 5 years ago

The intent is that display_native_width and _height are inherent to the display in question and are defined by the panel scan order. Try using the landscape/portrait rotation define in config.h primarily if that solves the issue?

unphased commented 5 years ago

In my testing, the only way to get this particular display to utilize all of its surface is to set DISPLAY_NATIVE_HEIGHT to 320, rather than DISPLAY_NATIVE_WIDTH to 320. This is regardless of whether the #define DISPLAY_OUTPUT_LANDSCAPE gets commented out.

Perhaps that is enough to reveal what the scanline direction is for the display. There are no datasheets to be found yet for this product.

it really does seem like for this product the 80 px "offset" just doesn't apply, because changing that math fixes the issues.

I realize that you do not have any display like this to test. Let me check with the other commenter to see if their code change was also on DISPLAY_NATIVE_HEIGHT.

s-marley commented 4 years ago

In case it's helpful to someone else having this problem, I made a video about how to make a 320x240 ST7789 display (in my case an Adafruit 2.0 320x240 Color IPS TFT) work with fbcp-ili9341 following the advice in this thread. The bit about what changes need to be made starts at 4:00.

juj commented 4 years ago

Fantastic video, your rhythm/pacing in the presentation is really good, see you put a good amount of time in production!

Perhaps there could be a -DST7789_240X320 target that would do config like this, not quite sure. The line

SPI_TRANSFER(0x37/*VSCSAD: Vertical Scroll Start Address of RAM*/, 0, 320 - DISPLAY_WIDTH);

does seem wrong, but not sure what the right statement would be that would preserve support for the other displays and all the orientations that people have been using. (perhaps

SPI_TRANSFER(0x37/*VSCSAD: Vertical Scroll Start Address of RAM*/, 0, 320 - DISPLAY_NATIVE_HEIGHT);

might be correct for all cases)

s-marley commented 4 years ago

Hi juj, thanks for your kind comments about the video, I'm just starting out so don't really know what I'm doing!

If you would like me to have a go with

SPI_TRANSFER(0x37/VSCSAD: Vertical Scroll Start Address of RAM/, 0, 320 - DISPLAY_NATIVE_HEIGHT)

using my display I'm happy to give that a go but it will have to be Monday when I have time. I'll let you know how it goes

Thanks again for writing the library, it's mega useful!

All the best,

Scott

On Sat, 25 Jan 2020, 8:06 am juj, notifications@github.com wrote:

Fantastic video, your rhythm/pacing in the presentation is really good, see you put a good amount of time in production!

Perhaps there could be a -DST7789_240X320 target that would do config like this, not quite sure. The line

SPI_TRANSFER(0x37/VSCSAD: Vertical Scroll Start Address of RAM/, 0, 320 - DISPLAY_WIDTH);

does seem wrong, but not sure what the right statement would be that would preserve support for the other displays and all the orientations that people have been using. (perhaps

SPI_TRANSFER(0x37/VSCSAD: Vertical Scroll Start Address of RAM/, 0, 320 - DISPLAY_NATIVE_HEIGHT);

might be correct for all cases)

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/juj/fbcp-ili9341/issues/104?email_source=notifications&email_token=ACSGWSNKSESX3HKP7AWTL5LQ7PXI3A5CNFSM4ILF6AZ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJ4XVWI#issuecomment-578386649, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACSGWSLC6GQVZRBQCPOPJJTQ7PXI3ANCNFSM4ILF6AZQ .

Redrocirt commented 4 years ago

I can confirm that the 320x240 pixel ST7789 (in my case this one from Adafruit) does work with the changes suggested in this issue.

As there are also ST7789-displays with 240x240 pixel I'd also vote for adding an option to select the pixel resolution for the ST7789 driver.

Darmur commented 3 years ago

I can also confirm it works fine, adding the -DST7789_240X320 variant would be great!

Gigahawk commented 3 years ago

Recently tested this on an Adafruit 2.0" 320x240 Color IPS TFT Display with microSD Card Breakout like others in this thread.

The display would just show a black screen with -DST7789=ON, and would only work with -DST7789VW=ON.

The only change I needed to make was to DISPLAY_NATIVE_HEIGHT to get the image to scale correctly, it seems to work fine with or without the offset change in st7735r.cpp.

I'm not familiar with what the difference is with the VW variant, but perhaps some batches of the display are using different controllers. The panel on mine says 2018/05/30.

The panel itself is a STP240320_0200B, seemingly only sold by this one vendor on aliexpress. Unfortunately the "datasheet" appears to just be a mechanical drawing and the pinout.

I've purchased a few from them directly and used them to assemble a breakout equivalent to the Adafruit one and they seem to have the same behavior. These directly purchased panels are dated 2020/06/24.