Re: [PATCH] zr36067: Debugging cleanups (updated again)
Trent Piepho <[email protected]>
| Newsgroups | gmane.comp.video.mjpeg.devel |
|---|---|
| Message-ID | <[email protected]> |
On Wed, 23 May 2007, Ronald S. Bultje wrote: > > One of the bugs I fixed to get mplayer and tvtime to work was that > > poll() > > would never return. It was looking in fh->v4l_buffers, which never > > change > > from the PEND state to the DONE state. When the isr finishes > > capturing a > > buffer, it only updates the state in zr->v4l_buffers. Since poll() is > > looking in fh->v4l_buffers, it never thinks any frames are ready. > > > > I changed poll() to look at zr->v4l_buffers, I take it that was the > > correct > > solution? > > Yes, I got a patch for that (from you?) a while ago, it looks correct > to me. Must have been someone else. I spent quite a bit of time finding out that one of the reasons mplayer didn't work was because of poll(). I've attached my patch to this email. > As for the patches, the 0x644 modes look weird to me, since most of them > are copied to the zoran structs on startup and never looked at again. Many of them are like that, but not all. I checked how each parameter was used, and only set 0644 for those which can be changed and take effect. For instance lock_norm is checked every time S_STD is called, v4l_bufsize is used on every open, pass_through takes effect on every device close, etc. Is there any option in paticular you think is wrong? > I don't really understand the other patch (zr_norm), since you're not > actually changing anything (x & y && !(x &~ y) is the same as x == y, it x = V4L2_STD_PAL_BG; y = V4L2_STD_PAL_BG|V4L2_STD_PAL_DK|V4L2_STD_PAL_I|V4L2_STD_PAL_H; x == y is false, but (x&y) && !(x&~y) is true. > essentially says x contains the y bit and it contains nothing which is > not the y bit). Could you elaborate? Not exactly, it says "x contains a bit in the set y and contains no bits which are not in the set y." You're assuming y is a single bit, which is not the case. For example, V4L2_STD_PAL is not a single bit, it is a mask of multiple bits for all the different PAL standards. The same for NTSC and SECAM. tvtime will request the standard NTSC-M and the zr36067 driver will reject it. ------------------------------------------------------------------------- This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/ _______________________________________________ Mjpeg-developer mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/mjpeg-developer
zr_poll.patch
(text/plain, 2.7 KB)
From: Trent Piepho <[email protected]> zr36067: Fix poll() operation From: Trent Piepho <[email protected]> The poll() function was looking the wrong frame. It was using the frame the driver was going to capture into next (pend_tail), when it should have been looking at the next frame to be de-queued with DQBUF/SYNC (sync_tail). It also wasn't looking in the right spot. It was looking at the file handle's copy of the buffer status, rather than the driver core copy. The interrupt routine marks frames as done in the driver core copy, the file handle copy isn't updated. So even if poll() looked at the right frame, it would never see it transition to done and return POLLIN. Signed-off-by: Trent Piepho <[email protected]> diff --git a/linux/drivers/media/video/zoran_driver.c b/linux/drivers/media/video/zoran_driver.c --- a/linux/drivers/media/video/zoran_driver.c +++ b/linux/drivers/media/video/zoran_driver.c @@ -4271,6 +4271,7 @@ zoran_poll (struct file *file, struct zoran *zr = fh->zr; wait_queue_head_t *queue = NULL; int res = 0, frame; + unsigned long flags; /* we should check whether buffers are ready to be synced on * (w/o waits - O_NONBLOCK) here @@ -4284,19 +4285,33 @@ zoran_poll (struct file *file, switch (fh->map_mode) { case ZORAN_MAP_MODE_RAW: - if (fh->v4l_buffers.active == ZORAN_FREE || - zr->v4l_pend_head == zr->v4l_pend_tail) { + frame = zr->v4l_pend[zr->v4l_sync_tail & V4L_MASK_FRAME]; + + spin_lock_irqsave(&zr->spinlock, flags); + dprintk(3, + KERN_DEBUG + "%s: %s() - active=%c, sync_tail=%lu/%c, pend_tail=%lu, pend_head=%lu\n", + ZR_DEVNAME(zr), __FUNCTION__, + "FAL"[fh->v4l_buffers.active], zr->v4l_sync_tail, + "UPMD"[zr->v4l_buffers.buffer[frame].state], + zr->v4l_pend_tail, zr->v4l_pend_head); + /* Buffer ready to DQBUF? */ + if (zr->v4l_buffers.buffer[frame].state == BUZ_STATE_DONE) + res = POLLIN | POLLRDNORM; + /* No? Are there going to be any? */ + else if (fh->v4l_buffers.active == ZORAN_FREE || + zr->v4l_pend_head == zr->v4l_pend_tail) + res = POLLNVAL; /* POLLHUP? */ + spin_unlock_irqrestore(&zr->spinlock, flags); + + if (res != POLLNVAL) + poll_wait(file, &zr->v4l_capq, wait); + else { dprintk(1, + KERN_WARNING "%s: zoran_poll() - no buffers queued\n", ZR_DEVNAME(zr)); - res = POLLNVAL; - goto poll_unlock_and_return; - } - queue = &zr->v4l_capq; - frame = zr->v4l_pend[zr->v4l_pend_tail & V4L_MASK_FRAME]; - poll_wait(file, queue, wait); - if (fh->v4l_buffers.buffer[frame].state == BUZ_STATE_DONE) - res = POLLIN | POLLRDNORM; + } break; case ZORAN_MAP_MODE_JPG_REC: