Hello,
On Wednesday, June 29, 2011 1:15 PM Hans Verkuil wrote:
> On Wednesday, June 29, 2011 11:49:06 Marek Szyprowski wrote:
> > This patch introduces VB2_STREAMON_WITHOUT_BUFFERS io flag and changes
> > the order of operations during stream on operation. Now the buffer are
> > first queued to the driver and then the start_streaming method is called.
> > This resolves the most common case when the driver needs to know buffer
> > addresses to enable dma engine and start streaming. For drivers that can
> > handle start_streaming without queued buffers (mem2mem and 'one shot'
> > capture case) a new VB2_STREAMON_WITHOUT_BUFFERS io flag has been
> > introduced. Driver can set it to let videobuf2 know that it support this
> > mode.
> >
> > This patch also updates videobuf2 clients (s5p-fimc, mem2mem_testdev and
> > vivi) to work properly with the changed order of operations and enables
> > use of the newly introduced flag.
> >
> > Signed-off-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
> > Signed-off-by: Sylwester Nawrocki <s.nawrocki@xxxxxxxxxxx>
> > Signed-off-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx>
> > CC: Pawel Osciak <pawel@xxxxxxxxxx>
> > ---
> >
> > drivers/media/video/mem2mem_testdev.c | 4 +-
> > drivers/media/video/s5p-fimc/fimc-capture.c | 65
> ++++++++++++++++----------
> > drivers/media/video/s5p-fimc/fimc-core.c | 4 +-
> > drivers/media/video/videobuf2-core.c | 21 ++++-----
> > drivers/media/video/vivi.c | 2 +-
> > include/media/videobuf2-core.h | 11 +++--
> > 6 files changed, 62 insertions(+), 45 deletions(-)
> >
> >
> > ---
> >
> > Hello,
> >
> > This patch introduces significant changes in the vb2 streamon operation,
> > so all vb2 clients need to be checked and updated. Right now I didn't
> > update mx3_camera and sh_mobile_ceu_camera drivers. Once we agree that
> > this patch can be merged, I will update it to include all the required
> > changes to these two drivers as well.
> >
> > Best regards
> > --
> > Marek Szyprowski
> > Samsung Poland R&D Center
> >
>
> <snip>
>
> > diff --git a/drivers/media/video/videobuf2-core.c
> b/drivers/media/video/videobuf2-core.c
> > index 5517913..911e2eb 100644
> > --- a/drivers/media/video/videobuf2-core.c
> > +++ b/drivers/media/video/videobuf2-core.c
> > @@ -1136,17 +1136,23 @@ int vb2_streamon(struct vb2_queue *q, enum
> v4l2_buf_type type)
> > }
> >
> > /*
> > - * Cannot start streaming on an OUTPUT device if no buffers have
> > - * been queued yet.
> > + * Cannot start streaming if driver requires queued buffers.
> > */
> > - if (V4L2_TYPE_IS_OUTPUT(q->type)) {
> > + if (!(q->io_flags & VB2_STREAMON_WITHOUT_BUFFERS)) {
> > if (list_empty(&q->queued_list)) {
> > - dprintk(1, "streamon: no output buffers queued\n");
> > + dprintk(1, "streamon: no buffers queued\n");
> > return -EINVAL;
> > }
> > }
> >
> > /*
> > + * If any buffers were queued before streamon,
> > + * we can now pass them to driver for processing.
> > + */
> > + list_for_each_entry(vb, &q->queued_list, queued_entry)
> > + __enqueue_in_driver(vb);
> > +
> > + /*
> > * Let driver notice that streaming state has been enabled.
> > */
> > ret = call_qop(q, start_streaming, q);
>
> Am I missing something? What is the purpose of this flag? Why not let the
> driver check in start_streaming whether any buffers have been queued and
> return -EINVAL if there aren't any? Between setting a flag or just doing
> the test in start_streaming I would prefer just doing the test.
>
> To make it even easier you can perhaps add an extra argument to
> start_streaming with the number of buffers that are already queued.
Ok, this sounds reasonable, it looks that I over-engineered it again...
> I can't help thinking that this is made more difficult than it really is.
Thanks for the idea!
Best regards
--
Marek Szyprowski
Samsung Poland R&D Center
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
