NetBSD-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

kern/38058: bktr(4) not signalling properly



>Number:         38058
>Category:       kern
>Synopsis:       bktr(4) not signalling properly
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Feb 18 21:15:01 +0000 2008
>Originator:     Matthew Mondor
>Release:        NetBSD 4.0_STABLE
>Organization:
>Environment:
System: NetBSD ginseng.xisop 4.0_STABLE NetBSD 4.0_STABLE (GENERIC_MM.IPV6) #0: 
Mon Dec 24 18:26:33 EST 2007 
root%hal.xisop@localhost:/usr/obj/sys/arch/i386/compile/GENERIC_MM.IPV6 i386
Architecture: i386
Machine: i386
>Description:
        When using the bktr(4) signalling facility (METEORSSIGNAL ioctl)
        as well as METEORSFPS ioctl to set a frame rate, the frames are
        indeed updated at specified FPS, but the complete frame signal
        continues to be sent at a very high rate.  Thus the application
        has to use setitimer(2) or another such facility, plus the bktr
        signal and match them to grab frames at a wanted rate, which is
        less precise and adds unnecessary overhead.  A custom timer alone
        cannot be used because frames need to only be processed when they
        fully were updated to avoid sync problems artifacts.  Thus, the
        bktr(4) signal is important, and ideally an application would only
        need to use that.

        I did see that on FreeBSD there was a filed PR about this,
        including a diff:
        http://www.freebsd.org/cgi/query-pr.cgi?pr=35289&cat=

        I adapted the diff to NetBSD 4 code (my -current box is a laptop
        so I can't test my card on it).  I tested it, and the frequency
        of signals indeed was reduced.  However, it still doesn't work as
        intended and any review or suggestion would be appreciated.

        At 4fps using YUV_PACKED mode and 320x240 the signal occurs at a
        rate of 8fps.  At 8fps the signal rate seemed about 10fps, and
        at 16fps the signal rate was about 20fps.

        Since the device is able to capture frames at the exact specified
        rate, my guess is that signalling should also be able to be fixed
        to report at the exact rate.
>How-To-Repeat:
        
>Fix:
        This is not a proper fix yet, although it did try to address the
        problem and somewhat made things better:


Index: bktr_core.c
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/bktr/bktr_core.c,v
retrieving revision 1.40.2.1
diff -u -r1.40.2.1 bktr_core.c
--- bktr_core.c 21 Jan 2008 20:40:15 -0000      1.40.2.1
+++ bktr_core.c 18 Feb 2008 20:44:15 -0000
@@ -692,9 +692,7 @@
        bktr_ptr_t              bktr;
        u_int                   bktr_status;
        u_char                  dstatus;
-       u_int                  field;
-       u_int                  w_field;
-       u_int                  req_field;
+       u_int                   field;
 
        bktr = (bktr_ptr_t) arg;
 
@@ -753,21 +751,6 @@
                OUTB(bktr, BKTR_TDEC, 0);
                OUTB(bktr, BKTR_TDEC, tdec_save);
 
-               /*  Reset to no-fields captured state  */
-               if (bktr->flags & (METEOR_CONTIN | METEOR_SYNCAP)) {
-                       switch(bktr->flags & METEOR_ONLY_FIELDS_MASK) {
-                       case METEOR_ONLY_ODD_FIELDS:
-                               bktr->flags |= METEOR_WANT_ODD;
-                               break;
-                       case METEOR_ONLY_EVEN_FIELDS:
-                               bktr->flags |= METEOR_WANT_EVEN;
-                               break;
-                       default:
-                               bktr->flags |= METEOR_WANT_MASK;
-                               break;
-                       }
-               }
-
                OUTL(bktr, BKTR_RISC_STRT_ADD,
                    bktr->dm_prog->dm_segs[0].ds_addr);
                OUTW(bktr, BKTR_GPIO_DMA_CTL, FIFO_ENABLED);
@@ -804,6 +787,11 @@
        /* Determine which field generated this interrupt */
        field = (bktr_status & BT848_INT_FIELD) ? EVEN_F : ODD_F;
 
+       if (field == ODD_F)
+               bktr->sync_fields++;
+       else
+               bktr->sync_fields += 0x10;
+
 
        /*
         * Process the VBI data if it is being captured. We do this once
@@ -828,57 +816,17 @@
                }
        }
 
-       /*
-        *  Register the completed field
-        *    (For dual-field mode, require fields from the same frame)
-        */
-       switch (bktr->flags & METEOR_WANT_MASK) {
-               case METEOR_WANT_ODD  : w_field = ODD_F         ;  break;
-               case METEOR_WANT_EVEN : w_field = EVEN_F        ;  break;
-               default               : w_field = (ODD_F|EVEN_F);  break;
-       }
-       switch (bktr->flags & METEOR_ONLY_FIELDS_MASK) {
-               case METEOR_ONLY_ODD_FIELDS  : req_field = ODD_F  ;  break;
-               case METEOR_ONLY_EVEN_FIELDS : req_field = EVEN_F ;  break;
-               default                      : req_field = (ODD_F|EVEN_F);
-                                              break;
-       }
-
-       if ((field == EVEN_F) && (w_field == EVEN_F))
-               bktr->flags &= ~METEOR_WANT_EVEN;
-       else if ((field == ODD_F) && (req_field == ODD_F) &&
-                (w_field == ODD_F))
-               bktr->flags &= ~METEOR_WANT_ODD;
-       else if ((field == ODD_F) && (req_field == (ODD_F|EVEN_F)) &&
-                (w_field == (ODD_F|EVEN_F)))
-               bktr->flags &= ~METEOR_WANT_ODD;
-       else if ((field == ODD_F) && (req_field == (ODD_F|EVEN_F)) &&
-                (w_field == ODD_F)) {
-               bktr->flags &= ~METEOR_WANT_ODD;
-               bktr->flags |=  METEOR_WANT_EVEN;
-       }
-       else {
-               /*  We're out of sync.  Start over.  */
-               if (bktr->flags & (METEOR_CONTIN | METEOR_SYNCAP)) {
-                       switch(bktr->flags & METEOR_ONLY_FIELDS_MASK) {
-                       case METEOR_ONLY_ODD_FIELDS:
-                               bktr->flags |= METEOR_WANT_ODD;
-                               break;
-                       case METEOR_ONLY_EVEN_FIELDS:
-                               bktr->flags |= METEOR_WANT_EVEN;
-                               break;
-                       default:
-                               bktr->flags |= METEOR_WANT_MASK;
-                               break;
-                       }
-               }
+       if (bktr->sync_fields > bktr->sync_match) {
+               /* Out of sync, restart */
+               bktr->sync_fields = 0;
                return 1;
        }
 
        /*
         * If we have a complete frame.
         */
-       if (!(bktr->flags & METEOR_WANT_MASK)) {
+       if ((bktr->sync_fields & bktr->sync_match) == bktr->sync_match) {
+               bktr->sync_fields = 0;
                bktr->frames_captured++;
                /*
                 * post the completion time.
@@ -1047,6 +995,8 @@
        bktr->frames_captured = 0;
        bktr->even_fields_captured = 0;
        bktr->odd_fields_captured = 0;
+       bktr->sync_fields = 0;
+       bktr->sync_match = SYNC_RGB;
        bktr->proc = NULL;
        set_fps(bktr, frame_rate);
        bktr->video.addr = 0;
@@ -1809,17 +1759,21 @@
                        case 0:                 /* default */
                        case METEOR_GEO_RGB16:
                                    bktr->format = METEOR_GEO_RGB16;
+                                   bktr->sync_match = SYNC_RGB;
                                    break;
                        case METEOR_GEO_RGB24:
                                    bktr->format = METEOR_GEO_RGB24;
+                                   bktr->sync_match = SYNC_RGB;
                                    break;
                        case METEOR_GEO_YUV_422:
                                    bktr->format = METEOR_GEO_YUV_422;
                                     if (geo->oformat & METEOR_GEO_YUV_12)
                                        bktr->format = METEOR_GEO_YUV_12;
+                                   bktr->sync_match = SYNC_YUV;
                                    break;
                        case METEOR_GEO_YUV_PACKED:
                                    bktr->format = METEOR_GEO_YUV_PACKED;
+                                   bktr->sync_match = SYNC_YUV;
                                    break;
                        }
                        bktr->pixfmt = oformat_meteor_to_bt(bktr->format);
@@ -1849,6 +1803,12 @@
                                                    BT848_INT_FMTCHG);
                        }
                }
+
+               if (geo->oformat & METEOR_GEO_ODD_ONLY)
+                       bktr->sync_match &= SYNC_ODD;
+               if (geo->oformat & METEOR_GEO_EVEN_ONLY)
+                       bktr->sync_match &= SYNC_EVEN;
+
                break;
        /* end of METEORSETGEO */
 
Index: bktr_reg.h
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/bktr/bktr_reg.h,v
retrieving revision 1.16.24.1
diff -u -r1.16.24.1 bktr_reg.h
--- bktr_reg.h  21 Jan 2008 20:40:14 -0000      1.16.24.1
+++ bktr_reg.h  18 Feb 2008 20:44:15 -0000
@@ -636,6 +636,12 @@
     int                frame_size;     /* number of bytes in a frame */
     u_int      fifo_errors;    /* number of fifo capture errors since open */
     u_int      dma_errors;     /* number of DMA capture errors since open */
+#define SYNC_YUV               0x00000012
+#define SYNC_RGB               0x00000011
+#define SYNC_ODD               0x0000000F
+#define SYNC_EVEN              0x000000F0
+    u_int      sync_match;
+    u_int      sync_fields;
     u_int      frames_captured;/* number of frames captured since open */
     u_int      even_fields_captured; /* number of even fields captured */
     u_int      odd_fields_captured; /* number of odd fields captured */



Home | Main Index | Thread Index | Old Index