Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 27 May 1997 16:00:27 GMT
From:      Luigi Rizzo <luigi@iet.unipi.it>
To:        multimedia@freebsd.org
Subject:   bt848 status, comments and diffs
Message-ID:  <199705271600.QAA00791@info.iet.unipi.it>

next in thread | raw e-mail | index | archive | help
Guys,

I was looking at solving my problems (missing odd frames etc.) with
the YUV* modes, and I noticed that there are some inconsistencies
in the handling of SYNC instructions at the end of each frame etc,
especially for what regards IRQ generation. This would explain why
continuous capture modes work whereas single capture mode dont (and
perhaps signals would not work either, although tv does not use
them).

To clean up things, and before investigating further on the problem,
I defined a few macros (OP_IRQ, OP_RESYNC) for commands which are
generally used in instructions. I also defined a macro, I2(a,b)
which I used for all occurrence of 2-word instructions. This makes
the code shorter and especially risc instructions easier to read.

Before going on I would like to know (from Amancio perhaps):

- what is the meaning of the 'interlace' variable ? What corresponds
  to the various values (1,2,3) which it can assume ?

- is it ok to generate an IRQ on every SYNC instruction,
  and let the handler ignore it if not required ? This seems to me
  the safest option (and, even if for continuous capture to the frame
  buffer it might not be necessary to generate 60 irq/s, the overhead
  should not be so terrible considering the amount of bus traffic
  this capture generates).

- why, in some SYNC instructions, you use 0xC << 24 ? Those
  are reserved bits, but probably are SOL/EOL. Are you following
  some application note, or trying to circumvent problems, or what ?

- in modes which require full frames, I think it would be worthwile
  to make frames always start on the same field (probably odd), i.e.
  include an OP_SYNC | BKTR_VRO instruction at the beginning. This
  would make the output more predictable and probably also remove
  some artifacts which people are seeing and which might be related to
  some frames starting with the odd field, some with the even field.

  In the current code you start with OP_SYNC | BKTR_FM1 (or FM3) which
  does not let you know which frame are you starting with.

I could derive the above info from the sources, but because of some
inconsistencies I would prefer a word on how things should be, rather
than look at what they are.

Once the above things have been sorted out, it will probably be
relatively easy to fix the driver (and maybe also make it shorter).

The diffs follow (relative to today's version from amancio).

	Cheers
	Luigi


--- brooktree848.c.amancio	Tue May 27 14:05:03 1997
+++ brooktree848.c	Tue May 27 15:38:43 1997
@@ -2179,7 +2179,7 @@
 #define BKTR_FM3      0xe
 #define BKTR_VRE      0x4
 #define BKTR_VRO      0xC
-#define BKTR_PXV      0x0
+#define BKTR_PXV      0x0	/* never used */
 #define BKTR_EOL      0x1
 #define BKTR_SOL      0x2
 
@@ -2193,6 +2193,11 @@
 #define OP_SOL	      (1 << 27)
 #define OP_EOL	      (1 << 26)
 
+#define	OP_IRQ	      (1 << 24)
+#define	OP_RESYNC     (1 << 15)
+
+#define	I2(a, b)    { *dma_prog++ = a ; *dma_prog++ = b ; }
+
 bool_t notclipped (bktr_reg_t * bktr, int x, int width) {
     int i;
     bktr_clip_t * clip_node;
@@ -2369,11 +2374,9 @@
 	buffer = target_buffer;
 	if (interlace == 2 && rows < 320 ) target_buffer += pitch; 
 
-	/* contruct sync : for video packet format */
-	*dma_prog++ = OP_SYNC  | 1 << 15 | BKTR_FM1;
+	/* contruct sync : for video packed format */
+	I2( OP_SYNC  | OP_RESYNC | BKTR_FM1, 0 );
 
-	/* sync, mode indicator packed data */
-	*dma_prog++ = 0;  /* NULL WORD */
 	width = cols;
 	for (i = 0; i < (rows/interlace); i++) {
 	    target = target_buffer;
@@ -2406,28 +2409,20 @@
 	switch (i_flag) {
 	case 1:
 		/* sync vre */
-		*dma_prog++ = OP_SYNC | 0xC << 24 | 1 << 24 | BKTR_VRE;
-		*dma_prog++ = 0;  /* NULL WORD */
-
-		*dma_prog++ = OP_JUMP	| 0xC << 24;
-		*dma_prog++ = (u_long ) vtophys(bktr->dma_prog);
+		I2( OP_SYNC | 0xC << 24 | OP_IRQ | BKTR_VRE, 0 );
+		I2( OP_JUMP | 0xC << 24, (u_long ) vtophys(bktr->dma_prog) );
 		return;
 
 	case 2:
 		/* sync vro */
-		*dma_prog++ = OP_SYNC | 1 << 24 | 1 << 20 | BKTR_VRO;
-		*dma_prog++ = 0;  /* NULL WORD */
-
-		*dma_prog++ = OP_JUMP;
-		*dma_prog++ = (u_long ) vtophys(bktr->dma_prog);
+		I2( OP_SYNC | OP_IRQ | 1 << 20 | BKTR_VRO, 0 );
+		I2( OP_JUMP, (u_long ) vtophys(bktr->dma_prog) );
 		return;
 
 	case 3:
 		/* sync vro */
-		*dma_prog++ = OP_SYNC | 1 << 24 | 1 << 15 | BKTR_VRO;
-		*dma_prog++ = 0;  /* NULL WORD */
-		*dma_prog++ = OP_JUMP | 0xc << 24 ;
-		*dma_prog = (u_long ) vtophys(bktr->odd_dma_prog);
+		I2(OP_SYNC | OP_IRQ | OP_RESYNC | BKTR_VRO, 0 );
+		I2(OP_JUMP | 0xc << 24, (u_long) vtophys(bktr->odd_dma_prog));
 		break;
 	}
 
@@ -2441,8 +2436,7 @@
 
 
 		/* sync vre IRQ bit */
-		*dma_prog++ = OP_SYNC |  1 << 15 | BKTR_FM1;
-		*dma_prog++ = 0;  /* NULL WORD */
+		I2( OP_SYNC |  OP_RESYNC | BKTR_FM1, 0 );
                 width = cols;
 		for (i = 0; i < (rows/interlace); i++) {
 		    target = target_buffer;
@@ -2474,10 +2468,8 @@
 	}
 
 	/* sync vre IRQ bit */
-	*dma_prog++ = OP_SYNC |  1 << 24 | 1 << 15 | BKTR_VRE;
-	*dma_prog++ = 0;  /* NULL WORD */
-	*dma_prog++ = OP_JUMP ;
-	*dma_prog++ = (u_long ) vtophys(bktr->dma_prog) ;
+	I2( OP_SYNC |  OP_IRQ | OP_RESYNC | BKTR_VRE, 0 );
+	I2( OP_JUMP , (u_long ) vtophys(bktr->dma_prog) );
 	*dma_prog++ = 0;  /* NULL WORD */
 }
 
@@ -2531,43 +2523,34 @@
 
 	/* contruct sync : for video packet format */
 	/* sync, mode indicator packed data */
-	*dma_prog++ = OP_SYNC | 1 << 15 | BKTR_FM1;
-	*dma_prog++ = 0;  /* NULL WORD */
+	I2( OP_SYNC | OP_RESYNC | BKTR_FM1, 0 );
 
 	b = cols;
 
 	for (i = 0; i < (rows/interlace); i++) {
-		*dma_prog++ = inst;
-		*dma_prog++ = target_buffer;
-		*dma_prog++ = inst3;
-		*dma_prog++ = target_buffer + b; 
+	/* XXX I don't understand why 2 instructions since b = cols  */
+		I2( inst, target_buffer );
+		I2( inst3, target_buffer + b );
 		target_buffer += interlace*(cols * 2);
 	}
 
 	switch (i_flag) {
 	case 1:
 		/* sync vre */
-		*dma_prog++ = OP_SYNC  | 0xC << 24  | BKTR_VRE;
-		*dma_prog++ = 0;  /* NULL WORD */
-
-		*dma_prog++ = OP_JUMP;
-		*dma_prog++ = (u_long ) vtophys(bktr->dma_prog);
+		I2( OP_SYNC  | 0xC << 24  | BKTR_VRE, 0 );
+		I2( OP_JUMP, (u_long ) vtophys(bktr->dma_prog) );
 		return;
 
 	case 2:
 		/* sync vro */
-		*dma_prog++ = OP_SYNC  | 0xC << 24 | BKTR_VRO;
-		*dma_prog++ = 0;  /* NULL WORD */
-		*dma_prog++ = OP_JUMP;
-		*dma_prog++ = (u_long ) vtophys(bktr->dma_prog);
+		I2( OP_SYNC  | 0xC << 24 | BKTR_VRO, 0 );
+		I2( OP_JUMP, (u_long ) vtophys(bktr->dma_prog) );
 		return;
 
 	case 3:
 		/* sync vre */
-		*dma_prog++ = OP_SYNC	 | BKTR_VRE;
-		*dma_prog++ = 0;  /* NULL WORD */
-		*dma_prog++ = OP_JUMP  ;
-		*dma_prog = (u_long ) vtophys(bktr->odd_dma_prog);
+		I2( OP_SYNC | BKTR_VRE, 0 );
+		I2( OP_JUMP,(u_long ) vtophys(bktr->odd_dma_prog) );
 		break;
 	}
 
@@ -2578,26 +2561,20 @@
 		dma_prog = (u_long * ) bktr->odd_dma_prog;
 
 		/* sync vre */
-		*dma_prog++ = OP_SYNC | 1 << 24 |  1 << 15 | BKTR_FM1;
-		*dma_prog++ = 0;  /* NULL WORD */
+		I2( OP_SYNC | OP_IRQ |  OP_RESYNC | BKTR_FM1, 0 );
 
 		for (i = 0; i < (rows/interlace) ; i++) {
-			*dma_prog++ = inst;
-			*dma_prog++ = target_buffer;
-			*dma_prog++ = inst3;
-			*dma_prog++ = target_buffer + b;
+			I2( inst, target_buffer );
+			I2( inst3, target_buffer + b );
 			target_buffer += interlace * ( cols*2);
 		}
 	}
 
 	/* sync vro IRQ bit */
-	*dma_prog++ = OP_SYNC   |  1 << 24  |  BKTR_VRE;
-	*dma_prog++ = 0;  /* NULL WORD */
-	*dma_prog++ = OP_JUMP ;
-	*dma_prog++ = (u_long ) vtophys(bktr->dma_prog);
+	I2( OP_SYNC   |  OP_IRQ  |  BKTR_VRE, 0 );
+	I2( OP_JUMP , (u_long ) vtophys(bktr->dma_prog) );
 
-	*dma_prog++ = OP_JUMP;
-	*dma_prog++ = (u_long ) vtophys(bktr->dma_prog);
+	I2( OP_JUMP, (u_long ) vtophys(bktr->dma_prog) ); /* XXX ??? */
 	*dma_prog++ = 0;  /* NULL WORD */
 }
 
@@ -2655,8 +2632,7 @@
 	t1 = target_buffer;
 
 	/* contruct sync : for video packet format */
-	*dma_prog++ = OP_SYNC  | 1 << 15 |	BKTR_FM3; /*sync, mode indicator packed data*/
-	*dma_prog++ = 0;  /* NULL WORD */
+	I2( OP_SYNC | OP_RESYNC | BKTR_FM3, 0 );
 
 	for (i = 0; i < (rows/interlace ) - 1; i++) {
 		*dma_prog++ = inst;
@@ -2669,27 +2645,18 @@
 
 	switch (i_flag) {
 	case 1:
-		*dma_prog++ = OP_SYNC  | 0xC << 24 | 1 << 24 |	BKTR_VRE;  /*sync vre*/
-		*dma_prog++ = 0;  /* NULL WORD */
-
-		*dma_prog++ = OP_JUMP | 0xc << 24;
-		*dma_prog++ = (u_long ) vtophys(bktr->dma_prog);
+		I2( OP_SYNC | 0xC << 24 | OP_IRQ | BKTR_VRE, 0);  /*sync vre*/
+		I2( OP_JUMP | 0xc << 24, (u_long ) vtophys(bktr->dma_prog) );
 		return;
 
 	case 2:
-		*dma_prog++ = OP_SYNC  | 0xC << 24 | 1 << 24 |	BKTR_VRO;  /*sync vre*/
-		*dma_prog++ = 0;  /* NULL WORD */
-
-		*dma_prog++ = OP_JUMP;
-		*dma_prog++ = (u_long ) vtophys(bktr->dma_prog);
+		I2( OP_SYNC | 0xC << 24 | OP_IRQ | BKTR_VRO, 0);  /*sync vre*/
+		I2( OP_JUMP, (u_long ) vtophys(bktr->dma_prog) );
 		return;
 
 	case 3:
-		*dma_prog++ = OP_SYNC	| 1 << 15 |   BKTR_VRO; 
-		*dma_prog++ = 0;  /* NULL WORD */
-
-		*dma_prog++ = OP_JUMP  ;
-		*dma_prog = (u_long ) vtophys(bktr->odd_dma_prog);
+		I2( OP_SYNC | OP_RESYNC |   BKTR_VRO, 0 );
+		I2( OP_JUMP, (u_long ) vtophys(bktr->odd_dma_prog) );
 		break;
 	}
 
@@ -2699,8 +2666,7 @@
 
 		target_buffer  = (u_long) buffer + cols;
 		t1 = target_buffer + cols/2;
-		*dma_prog++ = OP_SYNC	|   1 << 15 | BKTR_FM3; 
-		*dma_prog++ = 0;  /* NULL WORD */
+		I2( OP_SYNC	|   OP_RESYNC | BKTR_FM3, 0 );
 
 		for (i = 0; i < (rows/interlace )  - 1; i++) {
 			*dma_prog++ = inst;
@@ -2712,10 +2678,8 @@
 		}
 	}
     
-	*dma_prog++ = OP_SYNC  | 1 << 24 |   BKTR_VRE; 
-	*dma_prog++ = 0;  /* NULL WORD */
-	*dma_prog++ = OP_JUMP ;
-	*dma_prog++ = (u_long ) vtophys(bktr->dma_prog) ;
+	I2( OP_SYNC  | OP_IRQ |   BKTR_VRE, 0 ); 
+	I2( OP_JUMP , (u_long ) vtophys(bktr->dma_prog) ) ;
 	*dma_prog++ = 0;  /* NULL WORD */
 }
 



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199705271600.QAA00791>