gdb, video, r61524: gdb visual feedback + start video intf on CG

- Define a draft of the video interface
- Implement that dragt for CG for a single mode
  * Includes stub of brightness setting from disassembling 3.60
- Use the video interface to show visual feedback on GDB on CG

Using the video interface avoids directly linking into a driver, which
will serve modularity in gint 3.

CMakeLists.txt

@@ -243,10 +243,12 @@ set(SOURCES
   src/usb/string.c
   src/usb/usb.c
   src/usb/write4.S
+  # Video driver interface
+  src/video/video.c
 )
 
-set(ASSETS_FX src/font5x7.png)
+set(ASSETS_FX src/font5x7.png src/gdb/icons-i1msb.png)
-set(ASSETS_CG src/font8x9.png)
+set(ASSETS_CG src/font8x9.png src/gdb/icons-rgb565.png)
 fxconv_declare_assets(${ASSETS_FX} ${ASSETS_CG})
 
 include_directories(

TODO

@@ -53,3 +53,14 @@ R61524:
   mode 1 :   i3???   396x224  @? Hz
   mode 2 :   i1msb   128x64   @? Hz # <- for g1a emulation :)
   mode 3 :   2i1msb  128x64   @? Hz # <- for g1a emulation :)
+
+enum {
+    /* Indexed 1-bit, 0 is white, 1 is black. Row-major, left-to-right, each
+       row a set of 4-byte values with leftmost pixel on the MSB. */
+    IMAGE_FORMAT_I1MSB,
+    /* Indexed 2-bit: white, light gray, dark gray, black. Represented as a
+       pair of I1MSB buffers, the first being bit #0, the second bit #1. */
+    IMAGE_FORMAT_2I1MSB,
+    /* 16-bit RGB565. Row-major, left-to-right. */
+    IMAGE_FORMAT_RGB565,
+};

include/gint/image.h

@@ -35,8 +35,6 @@ extern "C" {
 #endif
 
 #include <gint/config.h>
-#if GINT_RENDER_RGB
-
 #include <gint/defs/attributes.h>
 #include <gint/defs/types.h>
 
@@ -168,6 +166,8 @@ enum {
 // Image creation and destruction
 //---
 
+#if GINT_RENDER_RGB
+
 /* image_alloc(): Create a new (uninitialized) image
 
    This function allocates a new image of the specified dimensions and format.

include/gint/video.h

@@ -0,0 +1,91 @@
+//---
+// gint:video - Generic video interface
+//
+// This header defines the interface for video (display) drivers. It allows
+// high-level code to manipulate the display independently of the underlying
+// hardware.
+//---
+
+#ifndef GINT_VIDEO
+#define GINT_VIDEO
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include <gint/defs/types.h>
+#include <gint/drivers.h>
+#include <gint/image.h>
+
+/* Video mode offered by a driver for rendering. */
+typedef struct {
+    /* Mode size */
+    uint16_t width;
+    uint16_t height;
+    /* Pixel format */
+    int16_t format;
+    /* Refresh frequency, -1 if unknown */
+    int16_t freq;
+} video_mode_t;
+
+/* Flags for the update function of the interface. */
+#define VIDEO_UPDATE_ENABLE_DMA  0x01
+#define VIDEO_UPDATE_ATOMIC      0x02
+
+/* Video driver interface. */
+typedef struct {
+    /* Associated driver (NULL if there's none). */
+    gint_driver_t const *driver;
+
+    /* List of modes (terminated by an all-0 element). The first mode is the
+       default mode and it should always be available. */
+    video_mode_t const *modes;
+    /* Get current video mode. */
+    uint (*mode_get)(void);
+    /* Set a video mode. */
+    bool (*mode_set)(uint id);
+
+    /* Minimum and maximum brightness settings. */
+    int brightness_min;
+    int brightness_max;
+    /* Set a brightness setting. */
+    bool (*brightness_set)(int setting);
+
+    /* Implements video_update(); bounds are checked befoer calling. */
+    bool (*update)(int x, int y, image_t const *framebuffer, int flags);
+
+} video_interface_t;
+
+/* Get the video interface currently in use. This can be NULL if the program is
+   being linked without a display driver. */
+video_interface_t const *video_get_current_interface(void);
+
+/* Get the index of the current video mode. This can be -1 if there is no
+   interface; however, if there is an interface, this is always >= 0. */
+int video_get_current_mode_index(void);
+
+/* Get the a pointer to the current video mode's definition. */
+video_mode_t const *video_get_current_mode(void);
+
+/* Update the contents of the display from a framebuffer image. A combination
+   of `VIDEO_UPDATE_*` flags can be specified to select the update method:
+   - `ENABLE_DMA` allows the driver to copy using DMA, when applicable;
+   - `ATOMIC` requires the driver to use an interrupt-less method.
+   Returns true on success.
+
+   Update flags will be ignored if not applicable (e.g. `ENABLE_DMA` for a
+   video interface that doesn't support DMA) but will result in an error if
+   applicable and the specified method fails (e.g. DMA transfer error).
+
+   This function is usually called with (x,y) = (0,0) and a contiguous
+   framebuffer whose size is the video mode size. Specifying images with other
+   sizes, positions and strides is allowed only when they result in data
+   transfers that are byte-aligned. DMA support is only guaranteed for
+   contiguous input and output. The implied rectangle must be in-bounds. */
+bool video_update(int x, int y, image_t const *fb, int flags);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* GINT_VIDEO */

src/gdb/fxconv-metadata.txt

@@ -0,0 +1,9 @@
+icons-i1msb.png:
+  type: bopti-image
+  profile: mono
+  name: gint_gdb_icons_i1msb
+
+icons-rgb565.png:
+  type: bopti-image
+  profile: rgb565
+  name: gint_gdb_icons_rgb565

src/gdb/gdb.c

@@ -4,12 +4,40 @@
 #include <gint/ubc.h>
 #include <gint/usb-ff-bulk.h>
 #include <gint/usb.h>
+#include <gint/video.h>
 
 #include <ctype.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 
+#define GDB_VISUAL_FEEDBACK 1
+
+#if GDB_VISUAL_FEEDBACK
+
+enum { ICON_WORKING, ICON_ERROR, ICON_COMM, ICON_IDLE };
+
+static void gdb_show_stub_status(int icon)
+{
+	video_mode_t const *M = video_get_current_mode();
+	if(!M)
+		return;
+
+	// extern image_t gint_gdb_icons_i1msb;
+	extern image_t gint_gdb_icons_rgb565;
+
+	if(M->format == IMAGE_RGB565) {
+		image_t sub;
+		image_sub(&gint_gdb_icons_rgb565, 6*icon, 0, 7, 7, &sub);
+		if(!video_update(M->width-7, 0, &sub, 0))
+			abort();
+	}
+}
+
+#else
+# define gdb_show_stub_status(...) ((void)0)
+#endif
+
 static void gdb_hexlify(char* output_string, const uint8_t* input_buffer, size_t input_size)
 {
 	const char* hex = "0123456789ABCDEF";
@@ -531,11 +559,11 @@ static void gdb_handle_single_step(uint32_t pc, ubc_break_mode_t break_mode)
 void gdb_main(gdb_cpu_state_t* cpu_state)
 {
 	if (!gdb_started && gdb_start()) {
-		// TODO: Visual signal "GDB stub error"
+		gdb_show_stub_status(ICON_ERROR);
 		return;
 	}
 
-	// TODO: Visual signal "GDB stub idle"
+	gdb_show_stub_status(ICON_IDLE);
 
 	if (gdb_single_step_backup.single_stepped) {
 		if (gdb_single_step_backup.channel0_used) {
@@ -557,7 +585,7 @@ void gdb_main(gdb_cpu_state_t* cpu_state)
 	}
 
 	while (1) {
-		// TODO: Visual signal "GDB stub communicating"
+		gdb_show_stub_status(ICON_COMM);
 
 		char packet_buffer[256];
 		ssize_t packet_size = gdb_recv_packet(packet_buffer, sizeof(packet_buffer));
@@ -566,7 +594,7 @@ void gdb_main(gdb_cpu_state_t* cpu_state)
 			continue;
 		}
 
-		// TODO: Visual signal "GDB stub working"
+		gdb_show_stub_status(ICON_WORKING);
 
 		switch (packet_buffer[0]) {
 			case '?': // Halt reason
@@ -620,7 +648,7 @@ void gdb_main(gdb_cpu_state_t* cpu_state)
 				break;
 		}
 
-		// TODO: Visual signal "GDB stub idle"
+		gdb_show_stub_status(ICON_IDLE);
 	}
 
 ret:

src/r61524/r61524.c

@@ -9,96 +9,59 @@
 #include <gint/drivers/states.h>
 #include <gint/dma.h>
 #include <gint/drivers/r61524.h>
+#include <gint/video.h>
+#include <gint/image.h>
 #include <gint/config.h>
 
 #if GINT_HW_CG
 
-#define DMA SH7305_DMA
+/* Registers */
-#define POWER SH7305_POWER
+#define REG_DEVICE_CODE_READ    0x000
-
+#define REG_DRIVER_OUTPUT_CTL   0x001
-//---
+#define REG_ENTRY_MODE          0x003
-//	Device specification sheet
+#define REG_DISPLAY_CTL2        0x008
-//---
+#define REG_LOW_POWER_CTL       0x00b
-
+#define REG_HADDR               0x200
-/* Registers and operations */
+#define REG_VADDR               0x201
-enum {
+#define REG_DATA                0x202
-	device_code_read		= 0x000,
+#define REG_HSTART              0x210
-	driver_output_control		= 0x001,
+#define REG_HEND                0x211
-	entry_mode			= 0x003,
+#define REG_VSTART              0x212
-	display_control_2		= 0x008,
+#define REG_VEND                0x213
-	low_power_control		= 0x00b,
-
-	ram_address_horizontal		= 0x200,
-	ram_address_vertical		= 0x201,
-	write_data			= 0x202,
-
-	horizontal_ram_start		= 0x210,
-	horizontal_ram_end		= 0x211,
-	vertical_ram_start		= 0x212,
-	vertical_ram_end		= 0x213,
-};
-
-typedef word_union(entry_mode_t,
-	uint TRI	:1;
-	uint DFM	:1;
-	uint		:1;
-	uint BGR	:1;
-	uint		:2;
-	uint HWM	:1;
-	uint		:1;
-	uint ORG	:1;
-	uint		:1;
-	uint ID		:2;
-	uint AM		:1;
-	uint		:1;
-	uint EPF	:2;
-);
-
-//---
-//	Device communication primitives
-//---
 
 /* Interface with the controller */
-static volatile uint16_t *intf = (void *)0xb4000000;
+static volatile uint16_t *DISPLAY = (void *)0xb4000000;
 /* Bit 4 of Port R controls the RS bit of the display driver */
 static volatile uint8_t *PRDR = (void *)0xa405013c;
 
+/* Select a register */
 GINLINE static void select(uint16_t reg)
 {
 	/* Clear RS and write the register number */
 	*PRDR &= ~0x10;
 	synco();
-	*intf = reg;
+	*DISPLAY = reg;
 	synco();
-
+	/* Set RS=1 to allow consecutive reads/writes after a select() */
-	/* Set RS back. We don't do this in read()/write() because the display
-	   driver is optimized for consecutive GRAM access. LCD-transfers will
-	   be faster when executing select() followed by several calls to
-	   write(). (Although most applications should use the DMA instead.) */
 	*PRDR |= 0x10;
 	synco();
 }
 
-GINLINE static uint16_t read(void)
-{
-	return *intf;
-}
-
 GINLINE static void write(uint16_t data)
 {
-	*intf = data;
+	*DISPLAY = data;
 }
 
-uint16_t r61524_get(int ID)
+GINLINE uint16_t r61524_get(int ID)
 {
 	select(ID);
-	return read();
+	return *DISPLAY;
 }
 
-void r61524_set(int ID, uint16_t value)
+GINLINE void r61524_set(int ID, uint16_t value)
 {
 	select(ID);
-	write(value);
+	*DISPLAY = value;
 }
 
 //---
@@ -107,28 +70,54 @@ void r61524_set(int ID, uint16_t value)
 
 void r61524_win_get(uint16_t *HSA, uint16_t *HEA, uint16_t *VSA, uint16_t *VEA)
 {
-	select(horizontal_ram_start);
+	*HSA = r61524_get(REG_HSTART);
-	*HSA = read();
+	*HEA = r61524_get(REG_HEND);
-	select(horizontal_ram_end);
+	*VSA = r61524_get(REG_VSTART);
-	*HEA = read();
+	*VEA = r61524_get(REG_VEND);
-
-	select(vertical_ram_start);
-	*VSA = read();
-	select(vertical_ram_end);
-	*VEA = read();
 }
 
 void r61524_win_set(uint16_t HSA, uint16_t HEA, uint16_t VSA, uint16_t VEA)
 {
-	select(horizontal_ram_start);
+	r61524_set(REG_HSTART, HSA);
-	write(HSA);
+	r61524_set(REG_HEND, HEA);
-	select(horizontal_ram_end);
+	r61524_set(REG_VSTART, VSA);
-	write(HEA);
+	r61524_set(REG_VEND, VEA);
+}
 
-	select(vertical_ram_start);
+//---
-	write(VSA);
+// Backlight management
-	select(vertical_ram_end);
+//---
-	write(VEA);
+
+void r61525_brightness_set(int level)
+{
+	bool GLOBAL_backlight_high_bit[7] = { /* at 0x80399530 */
+	    false, false, false, false,
+	    true, true, true,
+	};
+	uint8_t GLOBAL_backlight_table[7] = { /* at 0x80399537 */
+	    0x14, 0x4b, 0x78, 0xd2,
+	    0x6e, 0xa0, 0xc8,
+	};
+	uint8_t GLOBAL_0[10] = { /* at 0x8039953e */
+	    0x00, 0x01, 0x02, 0x03, 0x07,
+	    0x0f, 0x1f, 0x3f, 0x7f, 0xff,
+	};
+
+	if(level < 1)
+		level = 1;
+	if(level > 5)
+		level = 5;
+
+	int8_t volatile *PNDR = (void *)0xa4050138;
+	if(GLOBAL_backlight_high_bit[level])
+		*PNDR |= 0x10;
+	else
+		*PNDR &= 0xef;
+
+	synco();
+
+	r61524_set(0x5a2, GLOBAL_0[(level < 2) ? 9 : 5]);
+	r61524_set(0x5a1, GLOBAL_backlight_table[level]);
 }
 
 //---
@@ -141,13 +130,11 @@ void r61524_start_frame(int xmin, int xmax, int ymin, int ymax)
 {
 	/* Move the window to the desired region, then select address 0 */
 	r61524_win_set(395-xmax, 395-xmin, ymin, ymax);
-	select(ram_address_horizontal);
+	r61524_set(REG_HADDR, 0);
-	write(0);
+	r61524_set(REG_VADDR, 0);
-	select(ram_address_vertical);
-	write(0);
 
 	/* Bind address 0xb4000000 to the data write command */
-	select(write_data);
+	select(REG_DATA);
 }
 
 void r61524_display(uint16_t *vram, int start, int height, int method)
@@ -281,6 +268,45 @@ void r61524_display_gray_128x64(uint32_t *light, uint32_t *dark)
 		write(border);
 }
 
+static bool r61524_update(int x, int y, image_t const *fb, int flags)
+{
+	// TODO: r61524_update: Handle the mono cases
+	if(fb->format != IMAGE_RGB565)
+		return false;
+
+	uint w = fb->width;
+	uint h = fb->height;
+
+	dma_transfer_wait(0);
+	r61524_start_frame(x, x+w-1, y, y+h-1);
+
+	/* DMA if enabled */
+	bool dma_possible = (!x && w == 396 && fb->stride == 396*2 && !(h%4));
+	if((flags & VIDEO_UPDATE_ENABLE_DMA) && dma_possible) {
+		void *src = fb->data;
+		void *dst = (void *)DISPLAY;
+		int blocks = 99 * (h / 4);
+
+		if(flags & VIDEO_UPDATE_ATOMIC)
+			dma_transfer_atomic(0, DMA_32B, blocks, src, DMA_INC,
+				dst, DMA_FIXED);
+		else
+			dma_transfer_async(0, DMA_32B, blocks, src, DMA_INC,
+				dst, DMA_FIXED, GINT_CALL_NULL);
+		return true;
+	}
+
+	uint16_t *pixels = fb->data;
+
+	for(int y = 0; y < fb->height; y++) {
+		for(int x = 0; x < fb->width; x++)
+			write(pixels[x]);
+		pixels = (void *)pixels + fb->stride;
+	}
+
+	return true;
+}
+
 //---
 // State and driver metadata
 //---
@@ -303,4 +329,33 @@ gint_driver_t drv_r61524 = {
 };
 GINT_DECLARE_DRIVER(26, drv_r61524);
 
+//---
+// Video driver interface
+//---
+
+static video_mode_t r61524_modes[] = {
+	/* Standard full-screen full-color mode */
+	{ 396, 224, IMAGE_RGB565, -1 },
+#if 0
+	/* R61524 8-color mode with lower power consumption */
+	{ 396, 224, IMAGE_P8_RGB565, -1 }, // TODO: actually P3, that's closest
+	/* T6K11-emulation black-and-white mode */
+	{ 128, 64, IMAGE_I1MSB, -1 },
+	/* T6K11-emulation gray mode */
+	{ 128, 64, IMAGE_2I1MSB, -1 },
+#endif
+	{ 0 }
+};
+
+video_interface_t r61524_video = {
+	.driver          = &drv_r61524,
+	.modes           = r61524_modes,
+	.mode_get        = NULL, // TODO
+	.mode_set        = NULL, // TODO
+	.brightness_min  = 0, // TODO
+	.brightness_max  = 0, // TODO
+	.brightness_set  = NULL,
+	.update          = r61524_update,
+};
+
 #endif /* GINT_HW_CG */

src/video/video.c

@@ -0,0 +1,39 @@
+#include <gint/video.h>
+#include <gint/config.h>
+
+// TODO: video: Have interface set by gint pre-main call instead
+extern video_interface_t t6k11_video, r61524_video;
+static video_interface_t const *current_intf =
+    GINT_HW_SWITCH(NULL, &r61524_video);
+static int current_mode_index = GINT_HW_SWITCH(-1, 0);
+
+video_interface_t const *video_get_current_interface(void)
+{
+    return current_intf;
+}
+
+int video_get_current_mode_index(void)
+{
+    return current_mode_index;
+}
+
+video_mode_t const *video_get_current_mode(void)
+{
+    return (current_intf && current_mode_index >= 0)
+           ? &current_intf->modes[current_mode_index]
+           : NULL;
+}
+
+bool video_update(int x, int y, image_t const *fb, int flags)
+{
+    video_mode_t const *M = video_get_current_mode();
+
+    if(!M || !current_intf->update || fb->format != M->format)
+        return false;
+    if(x < 0 || y < 0)
+        return false;
+    if(x + fb->width > M->width || y + fb->height > M->height)
+        return false;
+
+    return current_intf->update(x, y, fb, flags);
+}