test-screensavers app seems to mess up the color palette

[smcameron]

Specifically, the disp_asset_saver(). The normal screensaver (called from badge.c) doesn't seem to have this problem.

Symptoms: background goes white, or colors are messed up.

To reproduce: Select Settings/Screensaver/TEST, then use the left/right dpad to select different screensavers. Eventually, you'll notice the colors are all wrong, or maybe the screen goes completely white.

To recover, the badge can be rebooted, nothing else seems to completely clear the problem.

The problem only occurs on the actual badge, the simulator does not replicate the problem.

[smcameron]

If I call display_reset() from test-screensavers.c upon exiting the app, then things are restored to normal. This doesn't explain what goes wrong in the first place though, and I shouldn't need to reset the display like that. But it does seem to point to the problem being with something stomping on the setup of the display in some way as being the problem (already strongly hinted at because the simulator can't reproduce the problem.)

[smcameron]

Instead of calling display_reset(), calling display_init_device(), which is a subset of what display_reset() does, also brings things back to normality.

display_reset() does this:

void display_reset(void) {
    lcd_hardwareReset();
    lcd_softwareReset();
    display_init_device();
}

display_init_device does this:

void display_init_device(void) {

    static lcd_t lcd = {
        .width = 128,
        .height = 160,
        .width_offset = 0,
        .height_offset = 0,
        .pin_reset = BADGE_GPIO_DISPLAY_RESET,
        .interface_pixel_format = LCD_PIXEL_FORMAT_565,
        .dataMode_activeState = 1,
        .pin_communicationMode = BADGE_GPIO_DISPLAY_DC,
        .reset_activeState = 0,
    };

    lcd_setSettingsActive(&lcd);
    lcd_hardwareReset();
    lcd_initialize();
    lcd_setSleepMode(LCD_SLEEP_OUT);
    lcd_setMemoryAccessControl(LCD_MADCTL_BGR);
    lcd_setInterfacePixelFormat(LCD_PIXEL_FORMAT_565);
    lcd_setGammaPredefined(LCD_GAMMA_PREDEFINED_3);
    lcd_setDisplayInversion(LCD_INVERSION_OFF);
    lcd_setTearingEffectLine(LCD_TEARING_OFF);
    lcd_setDisplayMode(LCD_DISPLAY_ON);
}

So display_init_device() does another lcd_hardwareReset() but notably does not do lcd_softwareReset().

[smcameron]

I have now seen the badge go to an to all-white screen and/or screwed up colors with the normal screensaver, not only test-screensavers app.

[smcameron]

This commit: 9f40666 may help. some.

When the screensaver was active, it was still calling the badge-app callback function, and those tend to also draw to the screen. With this patch, when the screen saver is active, the badge-app callback is not called, and with this patch, the badge seems to be able to run for hours without getting the "white screen" syndrome, which is good. However, the test-screensavers app still cannot display the RVAsec 2023 logo without getting the colors screwed up.

[smcameron]

A few observations:

1. Currently, when switching between screensavers in the test-screensavers app, it resets the display between the screen savers, in the hope of restoring sanity in case things got screwed up.
2. When I set the screen saver to display the RVAsec 2023 logo, and it gets screwed up, then when I switch away to a different screensaver, momentarily, it shows the logo with correct colors immediately before switching to the next screensaver.
3. If I add a bunch of debug printfs into FbImage8bit() in framebuffer.c, and connect to the /dev/ttyACM0 on my laptop to see those printfs, then it uh, it works correctly! The colors don't get screwed up!

Are we hammering the display too fast in FbImage8bit() somehow and the USB serial port printfs slow things down enough that the display remains happy?

This is the patch that makes it "work", for no good reason:

diff --git a/source/display/framebuffer.c b/source/display/framebuffer.c
index d6d2a5ce..c9f2f639 100644
--- a/source/display/framebuffer.c
+++ b/source/display/framebuffer.c
@@ -1,5 +1,6 @@
 #include <stdlib.h>
 #include <string.h>
+#include <stdio.h>
 
 #include "framebuffer.h"
 #include "display.h"
@@ -179,6 +180,8 @@ void FbImage8bit(const struct asset* asset, unsigned char seqNum)
     for (y = G_Fb.pos.y; y < yEnd; y++) {
         pixdata = uCHAR(&(asset->pixdata[ (y - G_Fb.pos.y) * asset->x + seqNum * asset->x * asset->y]));
 
+       printf("pixdata = %p\n", (void *) pixdata);
+
         for (x = 0; x < asset->x; x++) {
             if ((x + G_Fb.pos.x) >= LCD_XSIZE) break; /* clip x */

[smcameron]

Additional data point:

If the test-screensaver app is displaying the RVA sec 2023 logo with the printf in there and I'm connected to the serial port on my laptop with minicom, it works correctly. However, the instant I exit minicom, the colors suddenly become incorrect. I'm guessing the USB serial driver notices the disconnect, and the printf becomes more or less a NOP, and the loop speeds up and triggers whatever the weird behavior is.

[smcameron]

Ok, this patch also "fixes" the problem:

diff --git a/source/display/framebuffer.c b/source/display/framebuffer.c
index d6d2a5ce..97de7f4f 100644
--- a/source/display/framebuffer.c
+++ b/source/display/framebuffer.c
@@ -6,6 +6,7 @@
 #include "assetList.h"
 #include "colors.h"
 #include "trig.h"
+#include "delay.h"
 
 #define uCHAR (unsigned char *)
 struct framebuffer_t G_Fb;
@@ -179,6 +180,8 @@ void FbImage8bit(const struct asset* asset, unsigned char seqNum)
     for (y = G_Fb.pos.y; y < yEnd; y++) {
         pixdata = uCHAR(&(asset->pixdata[ (y - G_Fb.pos.y) * asset->x + seqNum * asset->x * asset->y]));
 
+       sleep_us(1000); // This is a hack. Without this sleep, colors get messed up for unknown reasons.
+
         for (x = 0; x < asset->x; x++) {
             if ((x + G_Fb.pos.x) >= LCD_XSIZE) break; /* clip x */

I'm going to go ahead and commit this for now. Committed as: b6dc9fe

[warasilapm]

Committing a 1ms sleep in library code is generally uncouth.

The display controller is designed to be perfectly fine with data being streamed continuously, but that doesn't mean there isn't some timing parameter that we are missing. This could also be some kind of hardware bus issue, but at 16 MHz that seems unlikely. Hard to say; have you had an opportunity to reproduce this across multiple badges?

[smcameron]

Committing a 1ms sleep in library code is generally uncouth.

Yeah, no kidding. So is resetting the display all the time between screensavers.

I'm just trying things to characterize the problem, and to have a workaround if we can't figure out the real solution in time.

So the delay doesn't completely solve the problem anyway, because if I remove the display reset between screensavers, the problem re-appears.

A possible real solution is to convert the troublesome image to 4-bit color instead of 8-bit color. I think it is the only 8-bit color image we have, which is why I was ok with hacking in the delay, since it's only used in a context where the delay didn't matter.

[smcameron]

Another data point: Converting the image to 4-bit does not help, the problem still occurs.

[smcameron]

On a hunch, I tried resizing the image from 128x160 to 128x159 -- did not help.
Also tried cropping to 128x128, also did not help.
Also tried starting with a png instead of a webp file in case there was something weird about the webp conversion -- conversions produced identical output.