Laurent,
I need to re-visit following comments as I found something different
after going through the videobuf-core.c
>[snip]
>
>> > > +static void vpfe_videobuf_queue(struct videobuf_queue *vq,
>> > > + struct videobuf_buffer *vb)
>> > > +{
>> > > + /* Get the file handle object and channel object */
>> > > + struct vpfe_fh *fh = vq->priv_data;
>> > > + struct channel_obj *channel = fh->channel;
>> > > + struct common_obj *common = NULL;
>> > > +
>> > > + v4l2_dbg(1, debug, vpfe_dev->driver, "<vpfe_buffer_queue>\n");
>> > > + common = &(channel->common[VPFE_VIDEO_INDEX]);
>> > > +
>> > > + /* add the buffer to the DMA queue */
>> > > + list_add_tail(&vb->queue, &common->dma_queue);
>> >
>> > This needs to be protected by a mutex
>>
>> [MK] I think in the videobuf-core.c this is already protected
>> by mutex lock using vb_lock of videobuf_queue structure.
>
>Actually it needs to be protected by a spinlock. common->dma_queue is
>accessed
>in IRQ handlers.
>
[MK] in videobuf-core.c, all calls to buffer_queue() is called inside
spin_lock_irqsave()/spin_lock_restore() which means irq is disabled.
So this is not required to be locked in the bridge driver code.
>[snip]
>
>> > > +static void vpfe_videobuf_release(struct videobuf_queue *vq,
>> > > + struct videobuf_buffer *vb)
>> > > +{
>> > > + v4l2_dbg(1, debug, vpfe_dev->driver, "vpfe_videobuf_release\n");
>> > > + videobuf_dma_contig_free(vq, vb);
>> > > + vb->state = VIDEOBUF_NEEDS_INIT;
>> >
>> > I'm not sure about this, but aren't you supposed to remove the buffer
>from
>> > the dma_queue here ?
>>
>> [MK] I believe when REQUEST_BUF is called the second time, INIT_LIST_HEAD
>> will re-initialize the queue.
>
>The buf_release callback is not necessarily followed by a VIDIOC_REQBUFS.
>For
>instance, if the user calls VIDIOC_STREAMOFF, vpfe_videobuf_release will be
>called. It would be perfectly valid to call VIDIOC_STREAMON after that with
>any VIDIOC_REQBUFS in between.
[MK] also here when freeing the buffer, driver could use the same irqlock
passed to the core from bridge driver to lock/unlock since when calling this
function, core is not using the same. Do you agree ?
_______________________________________________
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
