Re: [hatari-devel] Most wanted debugger/profiler feature or convenience? (Blitter/LED)

[Eero Tamminen <oak@xxxxxxxxxxxxxx>]

Hi,

On torstai 11 huhtikuu 2013, Douglas Little wrote:
> > Hatari statusbar doesn't have space for multiple LEDs, so the current
> > LED needs to be shared.  Disk access is probably only thing that users
> > expect it to be show, so it should be configurable what the LED is
> > showing.
> 
> I'm not sure how you can share a LED for multiple things, as it loses its
> value then? Except as a flashy thing in the corner. Perhaps I'm missing
> something.

That's exactly what I meant.  If it would flash constantly because some
HW component is used, it would be useless (even if it would use different
colors for different components).

So, there would need to be an option with which user (developer) can specify
what HW activity is shown by the statusbar LED.


> TBH there will always be better ways to diagnose something than via a LED
> so it may not matter much.

Yes, I don't think it adds much value.  But there's patch for getting
Blitter to blink the statusbar LED (I haven't tried it):
http://listengine.tuxfamily.org/lists.tuxfamily.org/hatari-
devel/2012/06/msg00127.html


> > > Another useful thing for blitter problems - having the debugger
> > > notice/alert when the blitter HW registers (other than busy bit) are
> > > modified

Do you mean changes in any of the registers, or or just the control
register?

Easiest would be to take a copy of the register values when blitter
starts working and checking when blitter stops working whether any
of them had changed.  This would of course need to disregard any
regs that change while blitter works:
lines, words, src and dst adress and halftone bits in control register.


> > > while the blitter is running.

And "running" means the time from setting the "busy" bit, until blitter
clears it (after it has processed all lines)?


Attached is a patch to do that when blitter tracing is enabled.  It also
adds "info blitter" command as that was only couple of lines more code.

Apparently Paradox's Pacemaker demo changes blitter skew register while
Blitter is still working, both at the start and end of the whole demo.


	- Eero

diff -r 0227a8a7dd35 src/blitter.c
--- a/src/blitter.c	Wed Apr 10 18:32:56 2013 +0300
+++ b/src/blitter.c	Thu Apr 11 22:32:40 2013 +0300
@@ -29,6 +29,7 @@
 #include <SDL_types.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <stddef.h>
 
 #include "main.h"
 #include "blitter.h"
@@ -120,6 +121,9 @@
 /* Blitter logical op func */
 typedef Uint16 (*BLITTER_OP_FUNC)(void);
 
+#if ENABLE_TRACING
+static BLITTERREGS	BlitterRegsCopy;
+#endif
 static BLITTERREGS	BlitterRegs;
 static BLITTERVARS	BlitterVars;
 static BLITTERSTATE	BlitterState;
@@ -128,6 +132,30 @@
 static BLITTER_OP_FUNC Blitter_ComputeHOP;
 static BLITTER_OP_FUNC Blitter_ComputeLOP;
 
+
+/*-----------------------------------------------------------------------*/
+/**
+ * Show Blitter register values in given structure
+ */
+static void Blitter_ShowRegs(FILE *fp, BLITTERREGS *regs)
+{
+	fprintf(fp, "src addr:  0x%06x\n", regs->src_addr);
+	fprintf(fp, "dst addr:  0x%06x\n", regs->dst_addr);
+	fprintf(fp, "words:     %u\n", regs->words);
+	fprintf(fp, "lines:     %u\n", regs->lines);
+	fprintf(fp, "src X-inc: %hd\n", regs->src_x_incr);
+	fprintf(fp, "src Y-inc: %hd\n", regs->src_y_incr);
+	fprintf(fp, "dst X-inc: %hd\n", regs->dst_x_incr);
+	fprintf(fp, "dst Y-inc: %hd\n", regs->dst_y_incr);
+	fprintf(fp, "end mask1: 0x%04x\n", regs->end_mask_1);
+	fprintf(fp, "end mask2: 0x%04x\n", regs->end_mask_2);
+	fprintf(fp, "end mask3: 0x%04x\n", regs->end_mask_3);
+	fprintf(fp, "HOP:       0x%02x\n", regs->hop);
+	fprintf(fp, "LOP:       0x%02x\n", regs->lop);
+	fprintf(fp, "control:   0x%02x\n", regs->ctrl);
+	fprintf(fp, "skew:      0x%02x\n", regs->skew);
+}
+
 /*-----------------------------------------------------------------------*/
 /**
  * Count blitter cycles
@@ -525,8 +553,7 @@
 	/* setup vars */
 	BlitterVars.pass_cycles = 0;
 	BlitterVars.op_cycles = 0;
-	BlitterVars.src_words_reset = BlitterVars.dst_words_reset +
-									BlitterVars.fxsr - BlitterVars.nfsr;
+	BlitterVars.src_words_reset = BlitterVars.dst_words_reset + BlitterVars.fxsr - BlitterVars.nfsr;
 
 	/* bus arbitration */
 	BusMode = BUS_MODE_BLITTER;		/* bus is now owned by the blitter */
@@ -551,6 +578,21 @@
 
 	if (BlitterRegs.lines == 0)
 	{
+		if (LOG_TRACE_LEVEL(TRACE_BLITTER)) {
+			/* skip members (src & dst addr, words & lines)
+			 * and mask bits that change while blitter works
+			 */
+			const size_t size = sizeof(BLITTERREGS) - offsetof(BLITTERREGS, src_x_incr);
+			BlitterRegsCopy.ctrl = (BlitterRegsCopy.ctrl & 0xf0) | (BlitterRegs.ctrl & 0x0f);
+			/* and check & complain about the rest */
+			if (memcmp(&BlitterRegsCopy.src_x_incr, &BlitterRegs.src_x_incr, size) != 0) {
+				fputs("WARNING: Blitter registers changed while it was active!\n", TraceFile);
+				fputs("Original registers:\n", TraceFile);
+				Blitter_ShowRegs(TraceFile, &BlitterRegsCopy);
+				fputs("\nChanged registers:\n", TraceFile);
+				Blitter_ShowRegs(TraceFile, &BlitterRegs);
+			}
+		}
 		/* We're done, clear busy bit */
 		BlitterRegs.ctrl &= ~0x80;
 
@@ -938,6 +980,9 @@
 			/* Start blitting after some CPU cycles */
 			CycInt_AddRelativeInterrupt((CurrentInstrCycles+nWaitStateCycles)>>nCpuFreqShift,
 							 INT_CPU_CYCLE, INTERRUPT_BLITTER);
+#if ENABLE_TRACING
+			memcpy(&BlitterRegsCopy, &BlitterRegs, sizeof(BLITTERREGS));
+#endif
 		}
 	}
 }
@@ -980,3 +1025,12 @@
 	MemorySnapShot_Store(&BlitterVars, sizeof(BlitterVars));
 	MemorySnapShot_Store(&BlitterHalftone, sizeof(BlitterHalftone));
 }
+
+/*-----------------------------------------------------------------------*/
+/**
+ * Show Blitter register values.
+ */
+void Blitter_Info(Uint32 dummy)
+{
+	Blitter_ShowRegs(stderr, &BlitterRegs);
+}
diff -r 0227a8a7dd35 src/debug/debugInfo.c
--- a/src/debug/debugInfo.c	Wed Apr 10 18:32:56 2013 +0300
+++ b/src/debug/debugInfo.c	Thu Apr 11 22:32:40 2013 +0300
@@ -13,6 +13,7 @@
 #include <assert.h>
 #include "main.h"
 #include "bios.h"
+#include "blitter.h"
 #include "configuration.h"
 #include "debugInfo.h"
 #include "debugcpu.h"
@@ -865,6 +866,7 @@
 	{ false,"aes",       AES_Info,             NULL, "Show AES vector contents (with <value>, show opcodes)" },
 	{ false,"basepage",  DebugInfo_Basepage,   NULL, "Show program basepage info at given <address>" },
 	{ false,"bios",      Bios_Info,            NULL, "Show BIOS opcodes" },
+	{ false,"blitter",   Blitter_Info,         NULL, "Show Blitter register values" },
 	{ false,"cookiejar", DebugInfo_Cookiejar,  NULL, "Show TOS Cookiejar contents" },
 	{ false,"crossbar",  DebugInfo_Crossbar,   NULL, "Show Falcon crossbar HW register values" },
 	{ true, "default",   DebugInfo_Default,    NULL, "Show default debugger entry information" },
diff -r 0227a8a7dd35 src/includes/blitter.h
--- a/src/includes/blitter.h	Wed Apr 10 18:32:56 2013 +0300
+++ b/src/includes/blitter.h	Thu Apr 11 22:32:40 2013 +0300
@@ -76,5 +76,6 @@
 
 extern void Blitter_MemorySnapShot_Capture(bool bSave);
 extern void Blitter_InterruptHandler(void);
+extern void Blitter_Info(Uint32 arg);
 
 #endif /* BLITTER_H */