code review and display driver changes

t6k11: use the gint array for variant detection
r61524: use true triple buffering by default
display: define DWIDTH and DHEIGHT
display: add C_RGB(r,g,b) (0 ≤ r,g,b ≤ 31) [fxcg50]

TODO

@@ -7,18 +7,24 @@ Crucial, missing things.
 
 Tests to run.
 * core:      run the alignment/size automated tests for memory functions
-* topti:     all charsets
+
+Issues.
+* #3         make drawing functions parameterized
+* #5         add decent random number generation (TinyMT)
+* #8         support fx-CG Manager
+* #9         add libimg
+* #10        support fx-CG 20
 
 Complementary elements on existing code.
 *            make fx9860g projects work out of the box on fxcg50
 * topti:     support unicode fonts
 * gray:      find good values for more models than the Graph 35+E II
 * dma:       maybe relax the 4-byte size constraint for dma_memset()
+* dma:       fx9860g support (need to switch it on)
 * core:      try to leave add-in without reset in case of panic
 * hardware:  fill in the HWMEM_FITTLB flag
 * keyboard:  think of extended functions
 * cpg:       spread spectrum on fxcg50
-* bopti:     blending modes for monochrome bitmaps (use topti assembler)
 * display:   use more of topti's assembler in drect()
 * core:      use cmp/str for memchr()
 * timer:     try putting addresses in <gint/mpu/tmu.h>
@@ -26,10 +32,12 @@ Complementary elements on existing code.
 * t6k11:     check if dupdate() can be done by the DMA
 
 Keep in mind.
+* build:     make the build system simpler (two targets are enough by default)
 * core:      run destructors when a task-switch results in leaving the app
+* core:      invoke main menu instead of returning after main() ends
 * prizm:     don't hardcode stack address in fxcg50.ld
 * prizm:     detect P1 static RAM (stack) in TLB
-* core:      prove and use qdiv10() instead of __sdivsi3
+* core rtc:  use qdiv10 to massively improve division performance
 * setjmp:    more registers may need to be saved
 * core:      free heap when a task-switch results in leaving the app
 * core:      save and restore interrupt masks

fx9860g.ld

@@ -190,8 +190,8 @@ SECTIONS
 
 	/*
 	**  RRAM sections
-	**  8800e000:4k  VBR space
+	**  8800e000:5k  VBR space
-	**  8800f000:4k  .gint.data and .gint.bss
+	**  8800f400:3k  .gint.data and .gint.bss
 	*/
 
 	/* VBR address: let's just start at the beginning of the RRAM area.

fxcg50.ld

@@ -16,10 +16,10 @@ MEMORY
 	rom   (rx):   o = 0x00300000, l = 220k
 	/* Static RAM; stack grows down from the end of this region.
 	   The first 0x2000 bytes are reserved by gint, see below */
-	ram   (rw):   o = 0x08102000, l = 512k
+	ram   (rw):   o = 0x08102000, l = 504k
-	/* gint's VBR space, mentioned here for completeness */
+	/* gint's VBR space, at the start of the user stack */
 	vbr   (rwx):  o = 0x8c160000, l = 5k
-	/* Some RAM region from P1 area; gint's data will reside here */
+	/* gint's data resides in these few kilobytes before the user area */
 	rram  (rwx):  o = 0x8c161400, l = 3k
 	/* On-chip IL memory */
 	ilram (rwx):  o = 0xe5200000, l = 4k
@@ -117,7 +117,7 @@ SECTIONS
 
 	_sbss = SIZEOF(.bss);
 
-	/* Read-write data sextions going to RAM (.data and .data.*) */
+	/* Read-write data sections going to RAM (.data and .data.*) */
 	.data ALIGN(4) : ALIGN(4) {
 		_ldata = LOADADDR(.data);
 		_rdata = . ;
@@ -175,8 +175,8 @@ SECTIONS
 
 	/*
 	**  gint-related sections
-	**  8c160000:4k  VBR space
+	**  8c160000:5k  VBR space
-	**  8c161000:4k  .gint.data and .gint.bss
+	**  8c161400:3k  .gint.data and .gint.bss
 	*/
 
 	/* VBR address: let's just start at the beginning of the RAM area.

include/display/cg.h

@@ -0,0 +1,12 @@
+//---
+//	display:cg - Internal definitions for the display module on fxcg50
+//---
+
+#ifndef DISPLAY_CG
+#define DISPLAY_CG
+
+/* dvram_switch() - triple buffering switch
+   Alternates VRAMs after a display update started. */
+void dvram_switch(void);
+
+#endif /* DISPLAY_CG */

include/gint/display-cg.h

@@ -15,6 +15,10 @@
 
 #include <gint/defs/types.h>
 
+/* Dimensions of the VRAM */
+#define DWIDTH 396
+#define DHEIGHT 224
+
 /* gint VRAM address. This value must always point to a 32-aligned bufer of
    size 177408. Any function can use it freely to perform rendering or store
    data when not drawing. Triple buffering is already implemented in gint, see
@@ -44,6 +48,10 @@ enum {
 	C_NONE = -1,
 };
 
+/* RGB color maker. Takes three channels of value 0..31 each (the extra bit of
+   green is not used). */
+#define C_RGB(r,g,b) (((r) << 11) | ((g) << 6) | (b))
+
 //---
 //	Image rendering (bopti)
 //---

include/gint/display-fx.h

@@ -13,6 +13,10 @@
 
 #include <gint/defs/types.h>
 
+/* Dimensions of the VRAM */
+#define DWIDTH 128
+#define DHEIGHT 64
+
 /* gint VRAM address. This value must always point to a 4-aligned buffer of
    size 1024. Any function can use it freely to:
    - Use another video ram area (triple buffering or more, gray engine);

include/gint/dma.h

@@ -5,6 +5,7 @@
 #ifndef GINT_DMA
 #define GINT_DMA
 
+/* TODO: Enable DMA on fx-9860G */
 #ifdef FXCG50
 
 #include <gint/defs/types.h>

include/gint/drivers.h

@@ -38,7 +38,7 @@
 typedef struct
 {
 	/* Driver name */
-	const char *name;
+	char const *name;
 
 	/* driver_sh3() - rectify driver initialization on SH3 platforms
 	   This function is called during driver initialization on SH3. It may
@@ -66,6 +66,10 @@ typedef struct
 	   macro of <defs/attributes.h> if it doesn't need to be initialized */
 	void *sys_ctx;
 
+	/* Stack context. This is a gint *internal* pointer used when switching
+	   in and out of the driver. Ignore it. */
+	void *_stack_ctx;
+
 	/* ctx_save() - save the driver's hardware support
 	   This function is provided by the driver to save the state of its
 	   hardware support (memory-mapped MPU registers, port state, etc).
@@ -84,7 +88,7 @@ typedef struct
 	   expected to be a short (max 21 bytes) string because only a few
 	   lines are available for all drivers.
 	   Returns a pointer to a string; a static buffer is suitable. */
-	const char * (*status)(void);
+	char const * (*status)(void);
 
 } GPACKED(4) gint_driver_t;
 

include/gint/drivers/iokbd.h

@@ -1,7 +1,7 @@
 //---
 //	gint:drivers:iokbd - I/O ports-driven keyboard scanner
 //
-//	This is for SH3 only. It reads key pressed from ports A/B/M.
+//	This is for SH3 only. It reads key presses from ports A/B/M.
 //---
 
 #ifndef GINT_DRIVERS_IOKBD

include/gint/exc.h

@@ -42,7 +42,7 @@ void gint_panic_set(GNORETURN void (*panic)(uint32_t code));
 
    Please be aware that many exceptions are of re-execution type. When
    execution restarts after such an exception is handled, the offending
-   instruction if re-executed. This can cause the exception handling mechanism
+   instruction is re-executed. This can cause the exception handling mechanism
    to loop. Use gint_exc_skip() to skip over the offending instruction when
    needed. Whether an exception is of re-execution type depends on the
    exception code. */

include/gint/gint.h

@@ -80,7 +80,7 @@ int gint_intlevel(int intid, int level);
    When an interrupt request is accepted, the hardware jumps to a specific
    interrupt handler at an address that depends on the interrupt source.
 
-   For safety, interrupt handlers should avoir referring to data from other
+   For safety, interrupt handlers should avoid referring to data from other
    blocks because the arrangement of blocks at runtime depends on event codes.
    The assembler program will assume that consecutive blocks in the source code
    will be consecutive in memory, which is not always true. Avoiding cross-
@@ -90,11 +90,11 @@ int gint_intlevel(int intid, int level);
    This function allows anyone to replace any interrupt handler so make sure
    you're not interfering with usual interrupt assignments.
 
-   The first parameter 'event_code' represents the event_code associated with
+   The first parameter event_code represents the event code associated with the
-   the interrupt. If it's not a multiple of 0x20 then you're doing something
+   interrupt. If it's not a multiple of 0x20 then you're doing something wrong.
-   wrong. The codes are normally platform-dependent, but gint always uses
+   The codes are normally platform-dependent, but gint always uses SH7305
-   SH7305 codes: SH3 platforms have a translation table. See the documentation
+   codes: SH3 platforms have a translation table. See the documentation for a
-   for a list of event codes and their associated interrupts.
+   list of event codes and their associated interrupts.
 
    The handler function must be an interrupt handler: it must not raise
    exceptions, must end with 'rte', and it will use the kernel register bank.

include/gint/hardware.h

@@ -124,7 +124,7 @@ void hw_detect(void);
 **  Direct Memory Access Controller
 */
 
-/* Nothing other than the HW_LOADED bit yet. Only valid on fxcg50 */
+/* Nothing other than the HW_LOADED bit yet */
 
 /*
 **  Timer Unit

include/gint/keyboard.h

@@ -237,7 +237,8 @@ key_event_t getkey_opt(int options, volatile int *timeout);
    perfectly regular, rather than approximating the requested frequency.
 
    The system default is (500 ms, 125 ms). With the 128 Hz setting, this
-   default is reached exactly without approximation.
+   default is reached exactly without approximation. gint's default is (400 ms,
+   40 ms) for more reactivity.
 
    @first  Delay between key press and first repeat (no more than one hour)
    @next   Delay between subsequent repeats (no more than one hour) */

include/gint/rtc.h

@@ -20,7 +20,7 @@ typedef struct
 	uint8_t month_day;	/* Day of month (1..31) */
 	uint8_t hours;		/* Hour (0..23) */
 	uint8_t minutes;	/* Minute (0..59) */
-	uint8_t seconds;	// Second (0..59) */
+	uint8_t seconds;	/* Second (0..59) */
 
 } rtc_time_t;
 

make/Makefile

@@ -46,10 +46,10 @@ src2obj = $(1:../src/%=src/%).o
 src2dep = $(1:../src/%=src/%).d
 
 # Source files
+# TODO: Enable the DMA on fx-9860G
 prune-fx := -name render-cg -prune -o -name dma  -prune -o -name r61524 -prune
 prune-cg := -name render-fx -prune -o -name gray -prune -o -name t6k11  -prune
-src      := $(shell find ../src \
+src      := $(shell find ../src $(prune-$(CONFIG.TARGET)) \
-                    $(prune-$(CONFIG.TARGET)) \
                     -o -name '*.[csS]' -print)
 src_obj  := $(foreach s,$(src),$(call src2obj,$s))
 
@@ -71,7 +71,6 @@ as	= $(CONFIG.TOOLCHAIN)-as
 ld	= $(CONFIG.TOOLCHAIN)-ld
 ar	= $(CONFIG.TOOLCHAIN)-ar
 objcopy	= $(CONFIG.TOOLCHAIN)-objcopy
-conv	= fxconv
 
 
 #
@@ -107,12 +106,12 @@ src/%.c.o: ../src/%.c src/%.c.d
 # Special files
 $(call src2obj,../src/font5x7.png): ../src/font5x7.png
 	@ mkdir -p $(dir $@)
-	$(call cmd_m,fxconv,font5x7.png)$(conv) -f $< -o $@ \
+	$(call cmd_m,fxconv,font5x7.png) fxconv -f $< -o $@ \
 		--fx --toolchain=$(CONFIG.TOOLCHAIN) name:gint_font5x7 \
 		charset:ascii grid.size:5x7 grid.padding:1 grid.border:0
 $(call src2obj,../src/font8x9.png): ../src/font8x9.png
 	@ mkdir -p $(dir $@)
-	$(call cmd_m,fxconv,font8x9.png)$(conv) -f $< -o $@ \
+	$(call cmd_m,fxconv,font8x9.png) fxconv -f $< -o $@ \
 		--cg --toolchain=$(CONFIG.TOOLCHAIN) name:gint_font8x9 \
 		charset:print grid.size:8x11 grid.padding:1 grid.border:0 \
 		proportional:true

src/core/exch.c

@@ -117,7 +117,7 @@ void gint_panic(uint32_t code)
 /* gint_panic_set(): Change the panic handler function */
 void gint_panic_set(GNORETURN void (*panic)(uint32_t code))
 {
-	gint_exc_panic = panic;
+	gint_exc_panic = panic ? panic : gint_default_panic;
 }
 
 /* gint_exc_catch(): Set a function to catch exceptions */

src/core/hardware.c

@@ -65,6 +65,20 @@ void hw_detect(void)
 		gint[HWCPUPR] = PRR;
 	}
 
+	gint[HWCALC] = HWCALC_FX9860G_SH4;
+	if(gint[HWMPU] == HWMPU_SH7337 || gint[HWMPU] == HWMPU_SH7355)
+	{
+		gint[HWCALC] = HWCALC_FX9860G_SH3;
+	}
+
+	/* Tell Graph 35+E II from OS version (this is accurate unless someone
+	   tweaks an OS file enough to de-correlate the version of the OS and
+	   the version of the display and storage memory drivers, which, let's
+	   be real, is enough for now.
+	   TODO: Try to detect Graph 35+E II from amount of ROM in BSC? */
+	char *version = (void *)0x80010020;
+	if(version[1] == '3') gint[HWCALC] = HWCALC_G35PE2;
+
 	/* Detect RAM by checking if 8804'0000 is the same as 8800'0000. */
 
 	volatile uint8_t *R4 = (void *)0x88040000;

src/core/start.c

@@ -163,7 +163,7 @@ int start(int isappli, int optnum)
 		if(drv->init) drv->init();
 
 		#ifdef GINT_BOOT_LOG
-		const char *status = drv->status ? drv->status() : NULL;
+		char const *status = drv->status ? drv->status() : NULL;
 		bootlog_driver(drv->name, status);
 		#endif
 	}

src/dma/dma.c

@@ -125,7 +125,7 @@ void dma_transfer_wait(int channel)
 
 	/* Wait for the channel to be disabled by the interrupt handler.
 	   When the source or the destination of the transfer is X, Y or IL
-	   memory, refrain from sleeping as this also stops the transfer. */
+	   memory, refrain from sleeping as this also stops the transfer! */
 	int onchip = 0;
 
 	if(ch->SAR >= 0xe5007000 && ch->SAR < 0xe5204000) onchip = 1;

src/r61524/r61524.c

@@ -223,11 +223,13 @@ void r61524_display(uint16_t *vram, int start, int height, int interrupts)
 	/* Now roll! */
 	if(interrupts)
 	{
+		/* If the previous transfer is still running, wait for it */
+		dma_transfer_wait(0);
 		/* Usa a normal, interrupt-based transfer */
 		dma_transfer(0, DMA_32B, blocks, src, DMA_INC, dst, DMA_FIXED);
-		/* Wait for it to finish */
+
-		/* TODO: Allow r61524_display() to return early */
+		/* Function returns early so that rendering can continue on
-		dma_transfer_wait(0);
+		   another VRAM while the transfer is still being done */
 	}
 	else dma_transfer_noint(0, DMA_32B, blocks, src,DMA_INC,dst,DMA_FIXED);
 }

src/render-cg/dupdate.c

@@ -1,10 +1,15 @@
 #include <gint/display.h>
+#include <display/cg.h>
 //#include <gint/drivers/r61524.h>
 
 /* dupdate() - Push the video RAM to the display driver */
 void dupdate(void)
 {
 	r61524_display(gint_vram, 0, 224, 1);
+
+	/* The DMA is still running, so we need to switch VRAMs to avoid
+	   overwriting the data which is about to be sent. */
+	dvram_switch();
 }
 
 /* dupdate_noint() - Push VRAM to the display without interrupts */

src/render-cg/dvram.c

@@ -7,9 +7,20 @@ static uint16_t *scnd = (void *)0xac11b500;
 /* Shared VRAM pointer, the one exposed by <gint/display.h> */
 uint16_t *gint_vram = (void *)0xac0f0000;
 
-/* dvram() - Control video RAM address and triple buffering */
+/* dvram() - control video RAM address and triple buffering */
 void dvram(uint16_t *new_main, uint16_t *new_secondary)
 {
+	if(gint_vram == main) gint_vram = new_main;
+	else if(gint_vram == scnd) gint_vram = new_secondary;
+
 	main = new_main;
 	scnd = new_secondary;
 }
+
+/* dvram_switch() - triple buffering switch
+   This function is not public, it is used only by dupdate(). */
+void dvram_switch(void)
+{
+	if(gint_vram == main) gint_vram = scnd;
+	else gint_vram = main;
+}

src/render/dprint.c

@@ -4,11 +4,11 @@
 /* dprint(): Display a formatted string */
 void dprint(int x, int y, int fg, int bg, char const *format, ...)
 {
-	char str[256];
+	char str[512];
 	va_list args;
 
 	va_start(args, format);
-	vsnprintf(str, 256, format, args);
+	vsnprintf(str, 512, format, args);
 	va_end(args);
 
 	dtext(x, y, str, fg, bg);

src/rtc/rtc.c

@@ -27,8 +27,7 @@ GBSS struct {
 //	Time management
 //---
 
-/* int8(), int16()
+/* int8(), int16() - convert BCD to integer */
-   Converts BCD values to integers */
 static int int8(uint8_t bcd)
 {
 	return (bcd & 0x0f) + 10 * (bcd >> 4);
@@ -39,9 +38,8 @@ static int int16(uint16_t bcd)
 		+ 1000 * (bcd >> 12);
 }
 
-/* bcd8(), bcd16()
+/* bcd8(), bcd16() - convert integer to BCD
-   Converts integer values to BCD
+   TODO: Use some kind of qdiv() for bcd8() and bcd16() */
-   TODO: Use some kind of qdiv() for bcd8() and bcd16()? */
 static uint8_t bcd8(int integer)
 {
 	integer %= 100;

src/std/stdio.c

@@ -383,9 +383,8 @@ static int digits_8(char *str, uint64_t n)
 
 static int64_t load_i(int size, va_list *args)
 {
-	/* All smaller types are promoeted to int so we can't read the
+	/* All smaller types are promoted to int wth sign extension, so we
-	   explicitly. They have been converted and sign-extended we don't need
+	   don't need to care about them. */
-	   to care what their size is, the result will remain the same */
 	if(size == 3) return va_arg(*args, long);
 	if(size == 4) return va_arg(*args, long long);
 	return va_arg(*args, int);

src/t6k11/t6k11.c

@@ -15,7 +15,13 @@
 //	Device specification sheet
 //---
 
-/* Screen registers. Registers 8..11 and 13..15 must not be used! */
+/* This version number is 1 for the old T6K11 everyone knows, and 2 for the
+   newer one found in the Graph 35+E II. Documentation is available only for
+   version 1. Dumps of Bdisp_PutDisp_DD() are used to driver version 2. */
+static int t6k11_version = 1;
+
+/* Screen registers on the original T6K11. Registers 8..11 and 13..15 are test
+   registers and must not be used! */
 enum {
 	reg_display	= 0,
 	reg_counter	= 1,
@@ -52,15 +58,6 @@ GDATA static volatile uint8_t *sel = (void *)0xb4000000;
 /* RS = 1: Command data or vram data */
 GDATA static volatile uint8_t *cmd = (void *)0xb4010000;
 
-/* variant() - check for the T6K11 variant that equips the Graph 35+E II
-   TODO: At least detect some piece of Graph 35+E II hardware. */
-static int variant(void)
-{
-	char os[11];
-	__os_version(os);
-	return (os[1] == '3');
-}
-
 /* command() - send a command to set the value of a register
    @reg   Register number
    @data  Value to set in reg */
@@ -80,8 +77,6 @@ GINLINE static uint8_t status(void)
    @buf  Buffer to take data from */
 GINLINE static void write_row(const uint8_t *buf)
 {
-	*sel = variant() ? 10 : reg_data;
-
 	/* Unroll the loop for more speed */
 	*cmd = *buf++;
 	*cmd = *buf++;
@@ -109,33 +104,40 @@ GINLINE static void write_row(const uint8_t *buf)
 //---
 
 /* t6k11_display() - send vram data to the LCD device */
-void t6k11_display(const void *vram, int y1, int y2, size_t stride)
+void t6k11_display_v1(const void *vram, int y1, int y2, size_t stride)
 {
 	for(int y = y1; y < y2; y++)
 	{
-		if(variant())
+		/* Set the X-address register for this row */
-		{
+		command(reg_xaddr, y | 0xc0);
-			command(8, y | 0x80);
+		/* Use Y-Up mode */
-			command(8, 4);
+		command(reg_counter, cnt_yup);
-		}
+		/* Start counting Y from 0 */
-
+		command(reg_yaddr, 0);
-		else
-		{
-			/* Set the X-address register for this row */
-			command(reg_xaddr, y | 0xc0);
-
-			/* Use Y-Up mode */
-			command(reg_counter, cnt_yup);
-
-			/* Start counting Y from 0 */
-			command(reg_yaddr, 0);
-		}
 
 		/* Send the row's data to the device */
+		*sel = reg_data;
 		write_row(vram);
 		vram += stride;
 	}
 }
+void t6k11_display_v2(const void *vram, int y1, int y2, size_t stride)
+{
+	for(int y = y1; y < y2; y++)
+	{
+		command(8, y | 0x80);
+		command(8, 4);
+
+		*sel = 10;
+		write_row(vram);
+		vram += stride;
+	}
+}
+void t6k11_display(const void *vram, int y1, int y2, size_t stride)
+{
+	if(t6k11_version == 1) t6k11_display_v1(vram, y1, y2, stride);
+	if(t6k11_version == 2) t6k11_display_v2(vram, y1, y2, stride);
+}
 
 /* t6k11_contrast() - change the contrast setting */
 void t6k11_contrast(int contrast)
@@ -199,7 +201,7 @@ GBSS static ctx_t sys_ctx;
 
 static void ctx_save(void *buf)
 {
-	if(variant()) return;
+	if(gint[HWCALC] == HWCALC_G35PE2) return;
 
 	ctx_t *ctx = buf;
 	ctx->strd = status();
@@ -207,7 +209,7 @@ static void ctx_save(void *buf)
 
 static void ctx_restore(void *buf)
 {
-	if(variant()) return;
+	if(gint[HWCALC] == HWCALC_G35PE2) return;
 
 	ctx_t *ctx = buf;
 
@@ -227,6 +229,8 @@ static void ctx_restore(void *buf)
 static void init(void)
 {
 	gint[HWDD] = HW_LOADED | HWDD_LIGHT;
+
+	if(gint[HWCALC] == HWCALC_G35PE2) t6k11_version = 2;
 }
 
 //---

src/tmu/tmu.c

@@ -250,8 +250,8 @@ static void driver_sh3(void)
 	timers[0].tmu = (void *)0xfffffe94;
 	timers[1].tmu = (void *)0xfffffea0;
 	timers[2].tmu = (void *)0xfffffeac;
-	/* We don't need to change the event code of ETMU0 since it's
+	/* We must not change the event code of ETMU0 since it's translated to
-	   translated to the SH4 code by the interrupt handler */
+	   its SH4 counterpart by the interrupt handler */
 	timers[3].tmu = (void *)0xa44c0030;
 
 	TSTR = (void *)0xfffffe92;
@@ -260,7 +260,7 @@ static void driver_sh3(void)
 
 static void init(void)
 {
-	/* Install the standard's TMU interrupt handlers */
+	/* Install the standard TMU's interrupt handlers */
 	void *h = gint_inthandler(0x400, inth_tmu, 128);
 
 	/* User information in interrupt handlers */