Hi Hans, On 11/21/2016 05:04 PM, Hans Verkuil wrote: > On 18/11/16 10:11, Stanimir Varbanov wrote: >> Hi Hans, >> >>>>> + >>>>> +static int >>>>> +vdec_reqbufs(struct file *file, void *fh, struct >>>>> v4l2_requestbuffers *b) >>>>> +{ >>>>> + struct vb2_queue *queue = to_vb2q(file, b->type); >>>>> + >>>>> + if (!queue) >>>>> + return -EINVAL; >>>>> + >>>>> + return vb2_reqbufs(queue, b); >>>>> +} >>>> >>>> Is there any reason why the v4l2_m2m_ioctl_reqbufs et al helper >>>> functions >>>> can't be used? I strongly recommend that, unless there is a specific >>>> reason >>>> why that won't work. >>> >>> So that means I need to completely rewrite the v4l2 part and adopt it >>> for mem2mem device APIs. >>> >>> If that is what you meant I can invest some time to make a estimation >>> what would be the changes and time needed. After that we can decide what >>> to do - take the driver as is and port it to mem2mem device APIs later >>> on or wait for the this transition to happen before merging. >>> >> >> I made an attempt to adopt v4l2 part of the venus driver to m2m API's >> and the result was ~300 less lines of code, but with the price of few >> extensions in m2m APIs (and I still have issues with running >> simultaneously multiple instances). >> >> I have to add few functions/macros to iterate over a list (list_for_each >> and friends). This is used to find the returned from decoder buffers by >> address and associate them to vb2_buffer, because the decoder can change >> the order of the output buffers. >> >> The main problem I have is registering of the capture buffers before >> session_start. This is requirement (disadvantage) of the firmware >> implementation i.e. I need to announce capture buffers (address and size >> of the buffer) to the firmware before start buffer interaction by >> session_start. >> >> So having that I think I will need one more week to stabilize the driver >> to the state that it was before this m2m transition. >> >> Thoughts? >> > > It sounds like this it worth doing, since if you need these extensions, > then > it is likely someone else will need it as well. Meanwhile I have found bigger obstacle - I cannot run multiple instances simultaneously. By m2m design it can execute only one job (m2m context) at a time per m2m device. Can you confirm that my observation is correct? If so one solution could be on every fops::open I can create m2m_dev and init m2m_cxt. > > Can you mail me a preliminary patch with the core m2m changes? That will be > helpful for me to look at. Something like below diffs: diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c b/drivers/media/v4l2-core/v4l2-mem2mem.c index 61d56c940f80..52e22ec0f67b 100644 --- a/drivers/media/v4l2-core/v4l2-mem2mem.c +++ b/drivers/media/v4l2-core/v4l2-mem2mem.c @@ -136,6 +136,28 @@ void *v4l2_m2m_buf_remove(struct v4l2_m2m_queue_ctx *q_ctx) } EXPORT_SYMBOL_GPL(v4l2_m2m_buf_remove); +struct vb2_v4l2_buffer * +v4l2_m2m_buf_remove_match(struct v4l2_m2m_queue_ctx *q_ctx, void *priv, + int (*match)(void *priv, struct vb2_v4l2_buffer *vb)) +{ + struct v4l2_m2m_buffer *b, *tmp; + struct vb2_v4l2_buffer *ret = NULL; + unsigned long flags; + + spin_lock_irqsave(&q_ctx->rdy_spinlock, flags); + list_for_each_entry_safe(b, tmp, &q_ctx->rdy_queue, list) { + if (match(priv, &b->vb)) { + list_del(&b->list); + ret = &b->vb; + break; + } + } + spin_unlock_irqrestore(&q_ctx->rdy_spinlock, flags); + + return ret; +} +EXPORT_SYMBOL_GPL(v4l2_m2m_buf_remove_match); + /* * Scheduling handlers */ diff --git a/include/media/v4l2-mem2mem.h b/include/media/v4l2-mem2mem.h index 64e1819ea66d..e943609209ba 100644 --- a/include/media/v4l2-mem2mem.h +++ b/include/media/v4l2-mem2mem.h @@ -263,6 +263,24 @@ static inline void *v4l2_m2m_dst_buf_remove(struct v4l2_m2m_ctx *m2m_ctx) return v4l2_m2m_buf_remove(&m2m_ctx->cap_q_ctx); } +struct vb2_v4l2_buffer * +v4l2_m2m_buf_remove_match(struct v4l2_m2m_queue_ctx *q_ctx, void *priv, + int (*match)(void *priv, struct vb2_v4l2_buffer *vb)); + +static inline struct vb2_v4l2_buffer * +v4l2_m2m_src_buf_remove_match(struct v4l2_m2m_ctx *m2m_ctx, void *priv, + int (*match)(void *priv, struct vb2_v4l2_buffer *vb)) +{ + return v4l2_m2m_buf_remove_match(&m2m_ctx->out_q_ctx, priv, match); +} + +static inline struct vb2_v4l2_buffer * +v4l2_m2m_dst_buf_remove_match(struct v4l2_m2m_ctx *m2m_ctx, void *priv, + int (*match)(void *priv, struct vb2_v4l2_buffer *vb)) +{ + return v4l2_m2m_buf_remove_match(&m2m_ctx->cap_q_ctx, priv, match); +} diff --git a/include/media/v4l2-mem2mem.h b/include/media/v4l2-mem2mem.h index 5a9597dd1ee0..64e1819ea66d 100644 --- a/include/media/v4l2-mem2mem.h +++ b/include/media/v4l2-mem2mem.h @@ -211,6 +211,12 @@ static inline void *v4l2_m2m_next_dst_buf(struct v4l2_m2m_ctx *m2m_ctx) return v4l2_m2m_next_buf(&m2m_ctx->cap_q_ctx); } +#define v4l2_m2m_for_each_dst_buf(q_ctx, b) \ + list_for_each_entry(b, &q_ctx->cap_q_ctx.rdy_queue, list) + +#define v4l2_m2m_for_each_src_buf(q_ctx, b) \ + list_for_each_entry(b, &q_ctx->out_q_ctx.rdy_queue, list) + /** * v4l2_m2m_get_src_vq() - return vb2_queue for source buffers * -- regards, Stan -- 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