Re: [PATCH] zr36067: Debugging cleanups (updated again)

Hi Trent,

On May 23, 2007, at 7:57 PM, Trent Piepho wrote:
[... explanation about zoran buffers vs. zoran_fh buffers ...]
> Thanks for the explaination.  Is this also used so that S_FMT can  
> be called
> to change the fh copy of the settings, but not have them actually take
> effect until STREAMON?

Yes (see S_FMT in zoran_driver.c).

> 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.

> wrt to v4l uncompressed capture, the fields in zr that need the  
> spinlock to
> be read (R) or written (W) from outside the isr are:
>
> zr->v4l_settings.height 	W
> zr->v4l_settings.bytesperline	W
> zr->codec_mode			W
> zr->v4l_memgrab_active		W
> zr->v4l_pend[x]			W
>   where x>=v4l_pend_tail && x<v4l_pend_head
> zr->v4l_pend_head		W
> zr->v4l_pend_tail		RW
> zr->v4l_grab_frame		RW
> zr->v4l_grab_seq		RW
> zr->v4l_buffers.buffer[x]	RW
>   where x>=v4l_pend[v4l_pend_tail] && x<=v4l_pend[v4l_pend_head-1]
>   should be the same as saying when
>   buffer[x].state == PEND || buffer[x].state = DMA it needs the
>   spinlock as the isr won't touch buffers in USER or DONE state
>
> So this means the poll() function needs to get the spinlock, since  
> it wants to
> check for a buffer (pointed to by v4l_sync_head) going from PEND| 
> DMA state to
> DONE state.

Yes, that looks ok to me also.

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. Setting them to 644 implies that a user can change those at  
runtime, which is not true. I would recommend to leave them a 444 to  
imply that. 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 essentially says x contains the y bit and it  
contains nothing which is not the y bit). Could you elaborate?

Ronald


-------------------------------------------------------------------------
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/