On 06/13/2013 11:45 AM, Kamil Debski wrote: > Pawel and Marek, I would really like to hear your opinions on this patch. > I remember that the assumption of the mem2mem framework (not to be confused > with > mem2mem type of devices) was that it was for simple devices where one output > buffer > was equivalent to one capture buffer. More complicated devices were supposed > to use videobuf2 directly. > > Please state your opinions. > > Best wishes, > -- > Kamil Debski > Linux Kernel Developer > Samsung R&D Institute Poland > >> > -----Original Message----- >> > From: Philipp Zabel [mailto:p.zabel@xxxxxxxxxxxxxx] >> > Sent: Monday, June 03, 2013 10:24 AM >> > To: linux-media@xxxxxxxxxxxxxxx >> > Cc: Sylwester Nawrocki; Mauro Carvalho Chehab; Pawel Osciak; John Sheu; >> > Hans Verkuil; Kamil Debski; Andrzej Hajda; Philipp Zabel >> > Subject: [PATCH v4] [media] mem2mem: add support for hardware buffered >> > queue >> > >> > On mem2mem decoders with a hardware bitstream ringbuffer, to drain the >> > buffer at the end of the stream, remaining frames might need to be >> > decoded from the bitstream buffer without additional input buffers >> > being provided. >> > To achieve this, allow a queue to be marked as buffered by the driver, >> > and allow scheduling of device_runs when buffered ready queues are >> > empty. >> > >> > This also allows a driver to copy input buffers into their bitstream >> > ringbuffer and immediately mark them as done to be dequeued. >> > >> > The motivation for this patch is hardware assisted h.264 reordering >> > support in the coda driver. For high profile streams, the coda can hold >> > back out-of-order frames, causing a few mem2mem device runs in the >> > beginning, that don't produce any decompressed buffer at the v4l2 >> > capture side. At the same time, the last few frames can be decoded from >> > the bitstream with mem2mem device runs that don't need a new input >> > buffer at the v4l2 output side. The decoder command ioctl can be used >> > to put the decoder into the ringbuffer draining end-of-stream mode. >> > >> > Signed-off-by: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx> As far as I'm concerned this change looks like something useful upstream, it's really a simple modification and it makes the in-kernel m2m framework more universal and useful for more drivers. But that's just my personal opinion. :-) Thanks, Sylwester >> > --- >> > Changes since v3: >> > - Split queue_set_buffered into set_src_buffered and set_dst_buffered, >> > which >> > take a v4l2_m2m_ctx pointer instead of a vb2_queue (which isn't >> > guaranteed >> > to be embedded in a v4l2_m2m_queue_ctx). >> > - Make them static inline. >> > --- >> > drivers/media/v4l2-core/v4l2-mem2mem.c | 10 ++++++++-- >> > include/media/v4l2-mem2mem.h | 13 +++++++++++++ >> > 2 files changed, 21 insertions(+), 2 deletions(-) >> > >> > diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c >> > b/drivers/media/v4l2-core/v4l2-mem2mem.c >> > index 66f599f..1007e60 100644 >> > --- a/drivers/media/v4l2-core/v4l2-mem2mem.c >> > +++ b/drivers/media/v4l2-core/v4l2-mem2mem.c >> > @@ -196,6 +196,10 @@ static void v4l2_m2m_try_run(struct v4l2_m2m_dev >> > *m2m_dev) >> > * 2) at least one destination buffer has to be queued, >> > * 3) streaming has to be on. >> > * >> > + * If a queue is buffered (for example a decoder hardware ringbuffer >> > + that has >> > + * to be drained before doing streamoff), allow scheduling without >> > v4l2 >> > + buffers >> > + * on that queue. >> > + * >> > * There may also be additional, custom requirements. In such case the >> > driver >> > * should supply a custom callback (job_ready in v4l2_m2m_ops) that >> > should >> > * return 1 if the instance is ready. >> > @@ -224,14 +228,16 @@ static void v4l2_m2m_try_schedule(struct >> > v4l2_m2m_ctx *m2m_ctx) >> > } >> > >> > spin_lock_irqsave(&m2m_ctx->out_q_ctx.rdy_spinlock, flags); >> > - if (list_empty(&m2m_ctx->out_q_ctx.rdy_queue)) { >> > + if (list_empty(&m2m_ctx->out_q_ctx.rdy_queue) >> > + && !m2m_ctx->out_q_ctx.buffered) { >> > spin_unlock_irqrestore(&m2m_ctx->out_q_ctx.rdy_spinlock, >> > flags); >> > spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags_job); >> > dprintk("No input buffers available\n"); >> > return; >> > } >> > spin_lock_irqsave(&m2m_ctx->cap_q_ctx.rdy_spinlock, flags); >> > - if (list_empty(&m2m_ctx->cap_q_ctx.rdy_queue)) { >> > + if (list_empty(&m2m_ctx->cap_q_ctx.rdy_queue) >> > + && !m2m_ctx->cap_q_ctx.buffered) { >> > spin_unlock_irqrestore(&m2m_ctx->cap_q_ctx.rdy_spinlock, >> > flags); >> > spin_unlock_irqrestore(&m2m_ctx->out_q_ctx.rdy_spinlock, >> > flags); >> > spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags_job); >> > diff --git a/include/media/v4l2-mem2mem.h b/include/media/v4l2- >> > mem2mem.h index d3eef01..a8e1bb3 100644 >> > --- a/include/media/v4l2-mem2mem.h >> > +++ b/include/media/v4l2-mem2mem.h >> > @@ -60,6 +60,7 @@ struct v4l2_m2m_queue_ctx { >> > struct list_head rdy_queue; >> > spinlock_t rdy_spinlock; >> > u8 num_rdy; >> > + bool buffered; >> > }; >> > >> > struct v4l2_m2m_ctx { >> > @@ -132,6 +133,18 @@ struct v4l2_m2m_ctx *v4l2_m2m_ctx_init(struct >> > v4l2_m2m_dev *m2m_dev, >> > void *drv_priv, >> > int (*queue_init)(void *priv, struct vb2_queue *src_vq, >> > struct vb2_queue *dst_vq)); >> > >> > +static inline void v4l2_m2m_set_src_buffered(struct v4l2_m2m_ctx >> > *m2m_ctx, >> > + bool buffered) >> > +{ >> > + m2m_ctx->out_q_ctx.buffered = buffered; } >> > + >> > +static inline void v4l2_m2m_set_dst_buffered(struct v4l2_m2m_ctx >> > *m2m_ctx, >> > + bool buffered) >> > +{ >> > + m2m_ctx->cap_q_ctx.buffered = buffered; } >> > + >> > void v4l2_m2m_ctx_release(struct v4l2_m2m_ctx *m2m_ctx); >> > >> > void v4l2_m2m_buf_queue(struct v4l2_m2m_ctx *m2m_ctx, struct >> > vb2_buffer *vb); >> > -- >> > 1.8.2.rc2 -- Sylwester Nawrocki Samsung R&D Institute Poland Samsung Electronics -- 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
