Skip to content

+ TFT attempts to bug-fix #7

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

+ TFT attempts to bug-fix #7

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1e527af
+ TFT inversion from menu
DnCraptor Oct 14, 2025
f884ff5
+ attempts to fix it
DnCraptor Oct 14, 2025
93f19c7
Merge branch 'xrip:main' into main
DnCraptor Oct 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 7 additions & 12 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@ endif()
# ====================================================================================
cmake_minimum_required(VERSION 3.13)
set(PICO_BOARD_HEADER_DIRS ${CMAKE_CURRENT_LIST_DIR}/boards )
set(PICO_PLATFORM "rp2350" CACHE STRING "Chip type")
#set(PICO_PLATFORM "rp2040" CACHE STRING "Chip type")
#set(PICO_BOARD "murmulator" CACHE STRING "Board type")
set(PICO_PLATFORM "rp2040" CACHE STRING "Chip type")
#set(PICO_PLATFORM "rp2350" CACHE STRING "Chip type")
set(PICO_BOARD "murmulator" CACHE STRING "Board type")
#set(PICO_BOARD "murmulator2" CACHE STRING "Board type")
#set(PICO_BOARD "rp2040pizero" CACHE STRING "Board type")
set(PICO_BOARD "rp2350pizero" CACHE STRING "Board type")
#set(PICO_BOARD "rp2350pizero" CACHE STRING "Board type")
#set(PICO_BOARD "olimex-pico-pc" CACHE STRING "Board type")

set(CMAKE_C_STANDARD 11)
Expand All @@ -38,7 +38,6 @@ option(I2S_CS4334 "Enable I2S CS4334 sound" OFF)
option(VGA "Enable VGA" OFF)
option(TFT "Enable TFT display" OFF)
option(ILI9341 "Enable TFT ILI9341 display" OFF)
option(INVERSION "Enable TFT display INVERSION" OFF)
option(HDMI "Enable HDMI display" OFF)
option(TV "Enable TV composite output" OFF)
option(SOFTTV "Enable TV soft composite output" OFF)
Expand All @@ -47,11 +46,10 @@ option(m1p2launcher "Enable m1p2-launcher support" OFF)
endif()

#set(m1p2launcher ON)
set(I2S ON)
#set(TFT ON)
#set(I2S ON)
set(TFT ON)
#set(ILI9341 ON)
#set(INVERSION ON)
set(HDMI ON)
#set(HDMI ON)
#set(TV ON)
#set(SOFTTV ON)

Expand Down Expand Up @@ -209,9 +207,6 @@ target_compile_definitions(${PROJECT_NAME} PRIVATE
IF(TFT)
target_link_libraries(${PROJECT_NAME} PRIVATE st7789)
target_compile_definitions(${PROJECT_NAME} PRIVATE TFT)
if (INVERSION)
target_compile_definitions(${PROJECT_NAME} PRIVATE INVERSION)
ENDIF()
SET(BUILD_NAME "${BUILD_NAME}-TFT")
IF(ILI9341)
SET(BUILD_NAME "${BUILD_NAME}-ILI9341")
Expand Down
4 changes: 3 additions & 1 deletion drivers/graphics/graphics.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ void hdmi_graphics_init();
#include "font6x8.h"
#include "font8x8.h"
#include "font8x16.h"
#define RGB888(r, g, b) ((r<<16) | (g << 8 ) | b )
#ifndef RGB888
#define RGB888(r, g, b) (((r) << 16) | ((g) << 8 ) | (b))
#endif
Comment on lines +31 to +33
Copy link

@gemini-code-assist gemini-code-assist bot Oct 14, 2025

Choose a reason for hiding this comment

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

high

While adding #ifndef prevents a compilation error, it introduces a more subtle issue. The RGB888 macro is also defined in drivers/st7789/st7789.h to perform a 24-bit to 16-bit color conversion. When TFT is enabled, graphics.h includes st7789.h, and due to this #ifndef, the 16-bit conversion macro from st7789.h is used everywhere RGB888 is called. This is problematic in files like src/main.cpp, where NesPalette888 is initialized with RGB888 and expects 24-bit color values, not 16-bit. This can lead to incorrect colors.

A better approach is to have uniquely named macros for different functionalities. I recommend the following changes:

  1. In drivers/st7789/st7789.h, rename RGB888 to something like RGB888_TO_565.
  2. In drivers/st7789/st7789.c, update the textmode_palette initialization to use the new macro name.
  3. In this file (drivers/graphics/graphics.h), you can then safely define RGB888 for 24-bit color without the #ifndef guard.

This will make the code clearer and prevent subtle color bugs.

enum graphics_mode_t {
TEXTMODE_DEFAULT,
GRAPHICSMODE_DEFAULT,
Expand Down
82 changes: 51 additions & 31 deletions drivers/st7789/st7789.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
#define MADCTL_ROW_COLUMN_EXCHANGE (1<<5)
#define MADCTL_COLUMN_ADDRESS_ORDER_SWAP (1<<6)

uint8_t INVERSION = 0;
Copy link

@gemini-code-assist gemini-code-assist bot Oct 14, 2025

Choose a reason for hiding this comment

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

medium

Using all-caps for a variable name like INVERSION can be misleading, as this convention is typically reserved for constants and macros. To improve clarity and adhere to common C/C++ style, consider renaming it to g_inversion to denote its global scope, or simply inversion. This would require updating its extern declaration and all usages across the project.


#define CHECK_BIT(var, pos) (((var)>>(pos)) & 1)

Expand All @@ -58,36 +59,6 @@ static int graphics_buffer_shift_y = 0;

enum graphics_mode_t graphics_mode = GRAPHICSMODE_DEFAULT;

static const uint8_t init_seq[] = {
1, 20, 0x01, // Software reset
1, 10, 0x11, // Exit sleep mode
2, 2, 0x3a, 0x55, // Set colour mode to 16 bit
#ifdef ILI9341
// ILI9341
#ifdef INVERSION
// ILI9341: переворот и инверсия цветов
2, 0, 0x36, MADCTL_MY | MADCTL_MX | MADCTL_ROW_COLUMN_EXCHANGE | MADCTL_BGR_PIXEL_ORDER, // flip + BGR
#else
2, 0, 0x36, MADCTL_ROW_COLUMN_EXCHANGE | MADCTL_BGR_PIXEL_ORDER, // Set MADCTL
#endif
#else
// ST7789
2, 0, 0x36, MADCTL_COLUMN_ADDRESS_ORDER_SWAP | MADCTL_ROW_COLUMN_EXCHANGE, // Set MADCTL
#endif
5, 0, 0x2a, 0x00, 0x00, SCREEN_WIDTH >> 8, SCREEN_WIDTH & 0xff, // CASET: column addresses
5, 0, 0x2b, 0x00, 0x00, SCREEN_HEIGHT >> 8, SCREEN_HEIGHT & 0xff, // RASET: row addresses
#ifdef INVERSION
1, 2, 0x21, // Inversion ON
#else
1, 2, 0x20, // Inversion OFF
#endif
1, 2, 0x13, // Normal display on, then 10 ms delay
1, 2, 0x29, // Main screen turn on, then wait 500 ms
0 // Terminate list
};
// Format: cmd length (including cmd byte), post delay in units of 5 ms, then cmd payload
// Note the delays have been shortened a little

static inline void lcd_set_dc_cs(const bool dc, const bool cs) {
sleep_us(5);
gpio_put_masked((1u << TFT_DC_PIN) | (1u << TFT_CS_PIN), !!dc << TFT_DC_PIN | !!cs << TFT_CS_PIN);
Expand Down Expand Up @@ -182,10 +153,39 @@ void graphics_init() {

gpio_put(TFT_CS_PIN, 1);
gpio_put(TFT_RST_PIN, 1);

const uint8_t init_seq[] = {
1, 20, 0x01, // Software reset
1, 10, 0x11, // Exit sleep mode
2, 2, 0x3a, 0x55, // Set colour mode to 16 bit
1, 2, 0x20 + INVERSION,
#ifdef ILI9341
// ILI9341
#ifdef INVERSION
// ILI9341: переворот и инверсия цветов
2, 0, 0x36, MADCTL_MY | MADCTL_MX | MADCTL_ROW_COLUMN_EXCHANGE | MADCTL_BGR_PIXEL_ORDER, // flip + BGR
#else
2, 0, 0x36, MADCTL_ROW_COLUMN_EXCHANGE | MADCTL_BGR_PIXEL_ORDER, // Set MADCTL
#endif
#else
Comment on lines +162 to +170
Copy link

@gemini-code-assist gemini-code-assist bot Oct 14, 2025

Choose a reason for hiding this comment

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

critical

The refactoring from a compile-time INVERSION macro to a runtime variable seems incomplete for the ILI9341 display. The #ifdef INVERSION block will now always take the #else path since the INVERSION macro is no longer defined. This means display flipping for ILI9341 is now disabled and cannot be enabled at runtime. The INVERSION variable is also not used to control the MADCTL register setting for ILI9341.

To fix this, you'll need to dynamically construct the init_seq array or parts of it based on the INVERSION variable's value. Since init_seq is a const array, you cannot modify it. One approach is to remove const, making it a mutable local array, and then set the MADCTL parameter for the 0x36 command based on the INVERSION variable before calling lcd_init().

// ST7789
// 2, 0, 0x36, MADCTL_COLUMN_ADDRESS_ORDER_SWAP | MADCTL_ROW_COLUMN_EXCHANGE, // Set MADCTL
2, 0, 0x36, MADCTL_COLUMN_ADDRESS_ORDER_SWAP | 32, // Set MADCTL
Copy link

@gemini-code-assist gemini-code-assist bot Oct 14, 2025

Choose a reason for hiding this comment

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

medium

Using the magic number 32 reduces code readability. It's better to use the MADCTL_ROW_COLUMN_EXCHANGE macro, which is defined in this file and represents the same value (1 << 5). This makes the code self-documenting and easier to maintain.

        2, 0, 0x36, MADCTL_COLUMN_ADDRESS_ORDER_SWAP | MADCTL_ROW_COLUMN_EXCHANGE, // Set MADCTL

#endif
5, 0, 0x2a, 0x00, 0x00, SCREEN_WIDTH >> 8, SCREEN_WIDTH & 0xff, // CASET: column addresses
5, 0, 0x2b, 0x00, 0x00, SCREEN_HEIGHT >> 8, SCREEN_HEIGHT & 0xff, // RASET: row addresses
1, 2, 0x13, // Normal display on, then 10 ms delay
1, 2, 0x29, // Main screen turn on, then wait 500 ms
0 // Terminate list
};
// Format: cmd length (including cmd byte), post delay in units of 5 ms, then cmd payload
// Note the delays have been shortened a little


lcd_init(init_seq);
gpio_put(TFT_LED_PIN, 1);

for (int i = 0; i < sizeof palette; i++) {
for (int i = 0; i < sizeof(palette) / sizeof(palette[0]); ++i) {
graphics_set_palette(i, 0x0000);
}
clrScr(0);
Expand Down Expand Up @@ -236,6 +236,26 @@ void st7789_dma_pixels(const uint16_t* pixels, const uint num_pixels) {
dma_channel_hw_addr(st7789_chan)->ctrl_trig = ctrl | DMA_CH0_CTRL_TRIG_INCR_READ_BITS;
}

static const uint16_t textmode_palette[16] = {
//R, G, B
RGB888(0x00,0x00, 0x00), //black
RGB888(0x00,0x00, 0xC4), //blue
RGB888(0x00,0xC4, 0x00), //green
RGB888(0x00,0xC4, 0xC4), //cyan
RGB888(0xC4,0x00, 0x00), //red
RGB888(0xC4,0x00, 0xC4), //magenta
RGB888(0xC4,0x7E, 0x00), //brown
RGB888(0xC4,0xC4, 0xC4), //light gray
RGB888(0x4E,0x4E, 0x4E), //dark gray
RGB888(0x4E,0x4E, 0xDC), //light blue
RGB888(0x4E,0xDC, 0x4E), //light green
RGB888(0x4E,0xF3, 0xF3), //light cyan
RGB888(0xDC,0x4E, 0x4E), //light red
RGB888(0xF3,0x4E, 0xF3), //light magenta
RGB888(0xF3,0xF3, 0x4E), //yellow
RGB888(0xFF,0xFF, 0xFF), //white
};

void __inline __scratch_y("refresh_lcd") refresh_lcd() {
switch (graphics_mode) {
case TEXTMODE_DEFAULT:
Expand Down
21 changes: 2 additions & 19 deletions drivers/st7789/st7789.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,26 +31,9 @@
#define TEXTMODE_COLS 53
#define TEXTMODE_ROWS 30

extern uint8_t INVERSION;

#define RGB888(r, g, b) ((((r) >> 3) << 11) | (((g) >> 2) << 5) | ((b) >> 3))
static const uint16_t textmode_palette[16] = {
//R, G, B
RGB888(0x00,0x00, 0x00), //black
RGB888(0x00,0x00, 0xC4), //blue
RGB888(0x00,0xC4, 0x00), //green
RGB888(0x00,0xC4, 0xC4), //cyan
RGB888(0xC4,0x00, 0x00), //red
RGB888(0xC4,0x00, 0xC4), //magenta
RGB888(0xC4,0x7E, 0x00), //brown
RGB888(0xC4,0xC4, 0xC4), //light gray
RGB888(0x4E,0x4E, 0x4E), //dark gray
RGB888(0x4E,0x4E, 0xDC), //light blue
RGB888(0x4E,0xDC, 0x4E), //light green
RGB888(0x4E,0xF3, 0xF3), //light cyan
RGB888(0xDC,0x4E, 0x4E), //light red
RGB888(0xF3,0x4E, 0xF3), //light magenta
RGB888(0xF3,0xF3, 0x4E), //yellow
RGB888(0xFF,0xFF, 0xFF), //white
};

inline static void graphics_set_bgcolor(uint32_t color888) {
// dummy
Expand Down
37 changes: 33 additions & 4 deletions src/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ SETTINGS settings = {
.player_2_input = GAMEPAD1,
.nes_palette = 0,
.swap_ab = false,
.inversion = false
};

static FATFS fs, fs1;
Expand Down Expand Up @@ -1010,6 +1011,10 @@ int InfoNES_Menu() {
return 1;
}

#ifdef TFT
extern "C" uint8_t INVERSION;
#endif

void load_config() {
char pathname[256];
sprintf(pathname, "%s\\pico-nes.cfg", HOME_DIR);
Expand All @@ -1021,6 +1026,9 @@ void load_config() {
UINT br;
f_read(&fd, &settings, sizeof(settings), &br);
f_close(&fd);
#ifdef TFT
INVERSION = settings.inversion;
#endif
}

void save_config() {
Expand Down Expand Up @@ -1099,6 +1107,10 @@ const MenuItem menu_items[] = {
{ "Flash frame: %s", ARRAY, &settings.flash_frame, nullptr,1, { "NO ", "YES" } },
{ "VGA Mode: %s", ARRAY, &settings.palette, nullptr,1, { "RGB333", "RGB222" } },
#endif
#if TFT
{ "" },
{ "Investion %s", ARRAY, &settings.inversion, nullptr, 1, { "NO ", "YES" } },
Copy link

@gemini-code-assist gemini-code-assist bot Oct 14, 2025

Choose a reason for hiding this comment

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

medium

There's a typo in the menu item text. 'Investion' should be 'Inversion'.

    { "Inversion %s", ARRAY, &settings.inversion, nullptr, 1, { "NO ", "YES" } },

#endif
#if SOFTTV
{ "" },
{ "TV system %s", ARRAY, &tv_out_mode.tv_system, nullptr, 1, { "PAL ", "NTSC" } },
Expand Down Expand Up @@ -1225,6 +1237,21 @@ int menu() {
}
draw_text(result, x, y, color, bg_color);
}
if (gamepad1_bits.b || (gamepad1_bits.select && !gamepad1_bits.start) ||
keyboard_bits.b || (keyboard_bits.select && !keyboard_bits.start)
) {
exit = true;
}
#ifdef TFT
if (INVERSION != settings.inversion) {
save_config();
f_unmount("");
watchdog_enable(10, true);
while(true) {
tight_loop_contents();
}
}
#endif
sleep_ms(100);
}
graphics_set_flashmode(settings.flash_line, settings.flash_frame);
Expand Down Expand Up @@ -1447,8 +1474,10 @@ int main() {
gpio_put(PICO_DEFAULT_LED_PIN, false);
}

#ifndef TFT
uint8_t link = testPins(VGA_BASE_PIN, VGA_BASE_PIN + 1);
SELECT_VGA = (link == 0) || (link == 0x1F);
#endif

// board_init();
tuh_init(BOARD_TUH_RHPORT);
Expand All @@ -1460,14 +1489,14 @@ int main() {
#if USE_NESPAD
nespad_begin(clock_get_hz(clk_sys) / 1000, NES_GPIO_CLK, NES_GPIO_DATA, NES_GPIO_LAT);
#endif
sem_init(&vga_start_semaphore, 0, 1);
multicore_launch_core1(render_core);
sem_release(&vga_start_semaphore);

f_mount(&fs, "", 1);
f_mkdir(HOME_DIR);
load_config();

sem_init(&vga_start_semaphore, 0, 1);
multicore_launch_core1(render_core);
sem_release(&vga_start_semaphore);

#ifndef BUILD_IN_GAMES
if (is_start_locked() || !parseROM(reinterpret_cast<const uint8_t *>(rom))) {
InfoNES_Menu();
Expand Down
1 change: 1 addition & 0 deletions src/main.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,5 @@ typedef struct __attribute__((__packed__)) {
INPUT player_2_input;
uint8_t nes_palette;
bool swap_ab;
bool inversion;
} SETTINGS;