| Message ID | 1379076986-10446-7-git-send-email-s.nawrocki@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Sylwester, On Fri, Sep 13, 2013 at 6:26 PM, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote: > Simplify the driver by using the m2m ioctl and vb2 helpers. > > TODO: Add setting of default initial format. > > Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > Signed-off-by: Kyugmin Park <kyungmin.park@samsung.com> > --- > drivers/media/platform/exynos-gsc/gsc-core.h | 12 --- > drivers/media/platform/exynos-gsc/gsc-m2m.c | 109 ++++---------------------- > 2 files changed, 16 insertions(+), 105 deletions(-) > > diff --git a/drivers/media/platform/exynos-gsc/gsc-core.h b/drivers/media/platform/exynos-gsc/gsc-core.h > index cc19bba..1afad32 100644 > --- a/drivers/media/platform/exynos-gsc/gsc-core.h > +++ b/drivers/media/platform/exynos-gsc/gsc-core.h > @@ -464,18 +464,6 @@ static inline void gsc_hw_clear_irq(struct gsc_dev *dev, int irq) > writel(cfg, dev->regs + GSC_IRQ); > } > > -static inline void gsc_lock(struct vb2_queue *vq) > -{ > - struct gsc_ctx *ctx = vb2_get_drv_priv(vq); > - mutex_lock(&ctx->gsc_dev->lock); > -} > - > -static inline void gsc_unlock(struct vb2_queue *vq) > -{ > - struct gsc_ctx *ctx = vb2_get_drv_priv(vq); > - mutex_unlock(&ctx->gsc_dev->lock); > -} > - > static inline bool gsc_ctx_state_is_set(u32 mask, struct gsc_ctx *ctx) > { > unsigned long flags; > diff --git a/drivers/media/platform/exynos-gsc/gsc-m2m.c b/drivers/media/platform/exynos-gsc/gsc-m2m.c > index 40a73f7..4f5d6cb 100644 > --- a/drivers/media/platform/exynos-gsc/gsc-m2m.c > +++ b/drivers/media/platform/exynos-gsc/gsc-m2m.c > @@ -262,8 +262,8 @@ static struct vb2_ops gsc_m2m_qops = { > .queue_setup = gsc_m2m_queue_setup, > .buf_prepare = gsc_m2m_buf_prepare, > .buf_queue = gsc_m2m_buf_queue, > - .wait_prepare = gsc_unlock, > - .wait_finish = gsc_lock, > + .wait_prepare = vb2_ops_wait_prepare, > + .wait_finish = vb2_ops_wait_finish, > .stop_streaming = gsc_m2m_stop_streaming, > .start_streaming = gsc_m2m_start_streaming, > }; > @@ -376,57 +376,6 @@ static int gsc_m2m_reqbufs(struct file *file, void *fh, > return v4l2_m2m_reqbufs(file, ctx->m2m_ctx, reqbufs); > } > > -static int gsc_m2m_expbuf(struct file *file, void *fh, > - struct v4l2_exportbuffer *eb) > -{ > - struct gsc_ctx *ctx = fh_to_ctx(fh); > - return v4l2_m2m_expbuf(file, ctx->m2m_ctx, eb); > -} > - > -static int gsc_m2m_querybuf(struct file *file, void *fh, > - struct v4l2_buffer *buf) > -{ > - struct gsc_ctx *ctx = fh_to_ctx(fh); > - return v4l2_m2m_querybuf(file, ctx->m2m_ctx, buf); > -} > - > -static int gsc_m2m_qbuf(struct file *file, void *fh, > - struct v4l2_buffer *buf) > -{ > - struct gsc_ctx *ctx = fh_to_ctx(fh); > - return v4l2_m2m_qbuf(file, ctx->m2m_ctx, buf); > -} > - > -static int gsc_m2m_dqbuf(struct file *file, void *fh, > - struct v4l2_buffer *buf) > -{ > - struct gsc_ctx *ctx = fh_to_ctx(fh); > - return v4l2_m2m_dqbuf(file, ctx->m2m_ctx, buf); > -} > - > -static int gsc_m2m_streamon(struct file *file, void *fh, > - enum v4l2_buf_type type) > -{ > - struct gsc_ctx *ctx = fh_to_ctx(fh); > - > - /* The source and target color format need to be set */ > - if (V4L2_TYPE_IS_OUTPUT(type)) { > - if (!gsc_ctx_state_is_set(GSC_SRC_FMT, ctx)) > - return -EINVAL; > - } else if (!gsc_ctx_state_is_set(GSC_DST_FMT, ctx)) { > - return -EINVAL; > - } > - > - return v4l2_m2m_streamon(file, ctx->m2m_ctx, type); > -} > - > -static int gsc_m2m_streamoff(struct file *file, void *fh, > - enum v4l2_buf_type type) > -{ > - struct gsc_ctx *ctx = fh_to_ctx(fh); > - return v4l2_m2m_streamoff(file, ctx->m2m_ctx, type); > -} > - > /* Return 1 if rectangle a is enclosed in rectangle b, or 0 otherwise. */ > static int is_rectangle_enclosed(struct v4l2_rect *a, struct v4l2_rect *b) > { > @@ -563,13 +512,15 @@ static const struct v4l2_ioctl_ops gsc_m2m_ioctl_ops = { > .vidioc_try_fmt_vid_out_mplane = gsc_m2m_try_fmt_mplane, > .vidioc_s_fmt_vid_cap_mplane = gsc_m2m_s_fmt_mplane, > .vidioc_s_fmt_vid_out_mplane = gsc_m2m_s_fmt_mplane, > - .vidioc_reqbufs = gsc_m2m_reqbufs, > - .vidioc_expbuf = gsc_m2m_expbuf, > - .vidioc_querybuf = gsc_m2m_querybuf, > - .vidioc_qbuf = gsc_m2m_qbuf, > - .vidioc_dqbuf = gsc_m2m_dqbuf, > - .vidioc_streamon = gsc_m2m_streamon, > - .vidioc_streamoff = gsc_m2m_streamoff, > + > + .vidioc_reqbufs = v4l2_m2m_ioctl_reqbufs, I think your intention was not to replace gsc_m2m_reqbufs() with v4l2_m2m_ioctl_reqbufs(). you didn't remove the gsc_m2m_reqbufs() function :) On top of that, gsc_m2m_reqbufs() has some buffer count related checks. Regards, Shaik Ameer Basha > + .vidioc_querybuf = v4l2_m2m_ioctl_querybuf, > + .vidioc_expbuf = v4l2_m2m_ioctl_expbuf, > + .vidioc_qbuf = v4l2_m2m_ioctl_qbuf, > + .vidioc_dqbuf = v4l2_m2m_ioctl_dqbuf, > + > + .vidioc_streamon = v4l2_m2m_ioctl_streamon, > + .vidioc_streamoff = v4l2_m2m_ioctl_streamoff, > .vidioc_g_selection = gsc_m2m_g_selection, > .vidioc_s_selection = gsc_m2m_s_selection > }; > @@ -588,6 +539,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, > src_vq->mem_ops = &vb2_dma_contig_memops; > src_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer); > src_vq->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY; > + src_vq->lock = &ctx->gsc_dev->lock; > > ret = vb2_queue_init(src_vq); > if (ret) > @@ -601,6 +553,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, > dst_vq->mem_ops = &vb2_dma_contig_memops; > dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer); > dst_vq->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY; > + dst_vq->lock = &ctx->gsc_dev->lock; > > return vb2_queue_init(dst_vq); > } > @@ -648,6 +601,7 @@ static int gsc_m2m_open(struct file *file) > ret = PTR_ERR(ctx->m2m_ctx); > goto error_ctrls; > } > + ctx->fh.m2m_ctx = ctx->m2m_ctx; > > if (gsc->m2m.refcnt++ == 0) > set_bit(ST_M2M_OPEN, &gsc->state); > @@ -691,44 +645,13 @@ static int gsc_m2m_release(struct file *file) > return 0; > } > > -static unsigned int gsc_m2m_poll(struct file *file, > - struct poll_table_struct *wait) > -{ > - struct gsc_ctx *ctx = fh_to_ctx(file->private_data); > - struct gsc_dev *gsc = ctx->gsc_dev; > - int ret; > - > - if (mutex_lock_interruptible(&gsc->lock)) > - return -ERESTARTSYS; > - > - ret = v4l2_m2m_poll(file, ctx->m2m_ctx, wait); > - mutex_unlock(&gsc->lock); > - > - return ret; > -} > - > -static int gsc_m2m_mmap(struct file *file, struct vm_area_struct *vma) > -{ > - struct gsc_ctx *ctx = fh_to_ctx(file->private_data); > - struct gsc_dev *gsc = ctx->gsc_dev; > - int ret; > - > - if (mutex_lock_interruptible(&gsc->lock)) > - return -ERESTARTSYS; > - > - ret = v4l2_m2m_mmap(file, ctx->m2m_ctx, vma); > - mutex_unlock(&gsc->lock); > - > - return ret; > -} > - > static const struct v4l2_file_operations gsc_m2m_fops = { > .owner = THIS_MODULE, > .open = gsc_m2m_open, > .release = gsc_m2m_release, > - .poll = gsc_m2m_poll, > + .poll = v4l2_m2m_fop_poll, > .unlocked_ioctl = video_ioctl2, > - .mmap = gsc_m2m_mmap, > + .mmap = v4l2_m2m_fop_mmap, > }; > > static struct v4l2_m2m_ops gsc_m2m_ops = { > -- > 1.7.9.5 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Shaik, On 09/13/2013 04:12 PM, Shaik Ameer Basha wrote: [...] >> -static int gsc_m2m_streamon(struct file *file, void *fh, >> - enum v4l2_buf_type type) >> -{ >> - struct gsc_ctx *ctx = fh_to_ctx(fh); >> - >> - /* The source and target color format need to be set */ >> - if (V4L2_TYPE_IS_OUTPUT(type)) { >> - if (!gsc_ctx_state_is_set(GSC_SRC_FMT, ctx)) >> - return -EINVAL; >> - } else if (!gsc_ctx_state_is_set(GSC_DST_FMT, ctx)) { >> - return -EINVAL; >> - } >> - >> - return v4l2_m2m_streamon(file, ctx->m2m_ctx, type); >> -} >> - >> -static int gsc_m2m_streamoff(struct file *file, void *fh, >> - enum v4l2_buf_type type) >> -{ >> - struct gsc_ctx *ctx = fh_to_ctx(fh); >> - return v4l2_m2m_streamoff(file, ctx->m2m_ctx, type); >> -} >> - >> /* Return 1 if rectangle a is enclosed in rectangle b, or 0 otherwise. */ >> static int is_rectangle_enclosed(struct v4l2_rect *a, struct v4l2_rect *b) >> { >> @@ -563,13 +512,15 @@ static const struct v4l2_ioctl_ops gsc_m2m_ioctl_ops = { >> .vidioc_try_fmt_vid_out_mplane = gsc_m2m_try_fmt_mplane, >> .vidioc_s_fmt_vid_cap_mplane = gsc_m2m_s_fmt_mplane, >> .vidioc_s_fmt_vid_out_mplane = gsc_m2m_s_fmt_mplane, >> - .vidioc_reqbufs = gsc_m2m_reqbufs, >> - .vidioc_expbuf = gsc_m2m_expbuf, >> - .vidioc_querybuf = gsc_m2m_querybuf, >> - .vidioc_qbuf = gsc_m2m_qbuf, >> - .vidioc_dqbuf = gsc_m2m_dqbuf, >> - .vidioc_streamon = gsc_m2m_streamon, >> - .vidioc_streamoff = gsc_m2m_streamoff, >> + >> + .vidioc_reqbufs = v4l2_m2m_ioctl_reqbufs, > > I think your intention was not to replace gsc_m2m_reqbufs() with > v4l2_m2m_ioctl_reqbufs(). > you didn't remove the gsc_m2m_reqbufs() function :) > > On top of that, gsc_m2m_reqbufs() has some buffer count related checks. Thanks for the review. Sorry, I actually left this patch halfway done. There is some clean up required before we can actually benefit from those m2m helpers. First of all the driver should have set valid default format right when the video device is opened. Then the hack with *{SRC, DST}_FMT flags should be removed. The fact that the selection API on mem-to-mem video nodes and its interaction with VIDIOC_S_FMT is not well defined doesn't of course help here. I thought I'd drop exynos-gsc from this series but I had a look at it and it didn't take much time to make those cleanups, so there will be two more patches for exynos-gsc, unfortunately not tested yet. Regarding gsc_m2m_reqbufs(), it currently behaves incorrectly. It should adjust reqbufs->count to a supported value, rather than returning EINVAL. Moreover, the buffer count limit is currently 32 for both CAPTURE and OUTPUT queue. I don't know when this number comes from, the driver always uses DMA buffer descriptor 0 for all transactions (GSC_M2M_BUF_NUM). Maybe this code was inherited from the initial BSP gsc-capture driver, where the buffer masking feature was actually used. Besides that, the number of requested buffer per vb2 buffer queue is always being limited to VIDEO_MAX_FRAME in videobuf2, which is also 32. So I think gsc_m2m_reqbufs() can be pretty much optimized, including removal of an unused 'frame' variable, and we can safely replace it with v4l2_m2m_ioctl_reqbufs(). -- Regards, Sylwester -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/13/2013 02:56 PM, Sylwester Nawrocki wrote: > Simplify the driver by using the m2m ioctl and vb2 helpers. > > TODO: Add setting of default initial format. So this patch can't be applied yet. Other than that it looks good, but I won't ack it since it introduces a regression as long as the TODO isn't addressed. Regards, Hans > > Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > Signed-off-by: Kyugmin Park <kyungmin.park@samsung.com> > --- > drivers/media/platform/exynos-gsc/gsc-core.h | 12 --- > drivers/media/platform/exynos-gsc/gsc-m2m.c | 109 ++++---------------------- > 2 files changed, 16 insertions(+), 105 deletions(-) > > diff --git a/drivers/media/platform/exynos-gsc/gsc-core.h b/drivers/media/platform/exynos-gsc/gsc-core.h > index cc19bba..1afad32 100644 > --- a/drivers/media/platform/exynos-gsc/gsc-core.h > +++ b/drivers/media/platform/exynos-gsc/gsc-core.h > @@ -464,18 +464,6 @@ static inline void gsc_hw_clear_irq(struct gsc_dev *dev, int irq) > writel(cfg, dev->regs + GSC_IRQ); > } > > -static inline void gsc_lock(struct vb2_queue *vq) > -{ > - struct gsc_ctx *ctx = vb2_get_drv_priv(vq); > - mutex_lock(&ctx->gsc_dev->lock); > -} > - > -static inline void gsc_unlock(struct vb2_queue *vq) > -{ > - struct gsc_ctx *ctx = vb2_get_drv_priv(vq); > - mutex_unlock(&ctx->gsc_dev->lock); > -} > - > static inline bool gsc_ctx_state_is_set(u32 mask, struct gsc_ctx *ctx) > { > unsigned long flags; > diff --git a/drivers/media/platform/exynos-gsc/gsc-m2m.c b/drivers/media/platform/exynos-gsc/gsc-m2m.c > index 40a73f7..4f5d6cb 100644 > --- a/drivers/media/platform/exynos-gsc/gsc-m2m.c > +++ b/drivers/media/platform/exynos-gsc/gsc-m2m.c > @@ -262,8 +262,8 @@ static struct vb2_ops gsc_m2m_qops = { > .queue_setup = gsc_m2m_queue_setup, > .buf_prepare = gsc_m2m_buf_prepare, > .buf_queue = gsc_m2m_buf_queue, > - .wait_prepare = gsc_unlock, > - .wait_finish = gsc_lock, > + .wait_prepare = vb2_ops_wait_prepare, > + .wait_finish = vb2_ops_wait_finish, > .stop_streaming = gsc_m2m_stop_streaming, > .start_streaming = gsc_m2m_start_streaming, > }; > @@ -376,57 +376,6 @@ static int gsc_m2m_reqbufs(struct file *file, void *fh, > return v4l2_m2m_reqbufs(file, ctx->m2m_ctx, reqbufs); > } > > -static int gsc_m2m_expbuf(struct file *file, void *fh, > - struct v4l2_exportbuffer *eb) > -{ > - struct gsc_ctx *ctx = fh_to_ctx(fh); > - return v4l2_m2m_expbuf(file, ctx->m2m_ctx, eb); > -} > - > -static int gsc_m2m_querybuf(struct file *file, void *fh, > - struct v4l2_buffer *buf) > -{ > - struct gsc_ctx *ctx = fh_to_ctx(fh); > - return v4l2_m2m_querybuf(file, ctx->m2m_ctx, buf); > -} > - > -static int gsc_m2m_qbuf(struct file *file, void *fh, > - struct v4l2_buffer *buf) > -{ > - struct gsc_ctx *ctx = fh_to_ctx(fh); > - return v4l2_m2m_qbuf(file, ctx->m2m_ctx, buf); > -} > - > -static int gsc_m2m_dqbuf(struct file *file, void *fh, > - struct v4l2_buffer *buf) > -{ > - struct gsc_ctx *ctx = fh_to_ctx(fh); > - return v4l2_m2m_dqbuf(file, ctx->m2m_ctx, buf); > -} > - > -static int gsc_m2m_streamon(struct file *file, void *fh, > - enum v4l2_buf_type type) > -{ > - struct gsc_ctx *ctx = fh_to_ctx(fh); > - > - /* The source and target color format need to be set */ > - if (V4L2_TYPE_IS_OUTPUT(type)) { > - if (!gsc_ctx_state_is_set(GSC_SRC_FMT, ctx)) > - return -EINVAL; > - } else if (!gsc_ctx_state_is_set(GSC_DST_FMT, ctx)) { > - return -EINVAL; > - } > - > - return v4l2_m2m_streamon(file, ctx->m2m_ctx, type); > -} > - > -static int gsc_m2m_streamoff(struct file *file, void *fh, > - enum v4l2_buf_type type) > -{ > - struct gsc_ctx *ctx = fh_to_ctx(fh); > - return v4l2_m2m_streamoff(file, ctx->m2m_ctx, type); > -} > - > /* Return 1 if rectangle a is enclosed in rectangle b, or 0 otherwise. */ > static int is_rectangle_enclosed(struct v4l2_rect *a, struct v4l2_rect *b) > { > @@ -563,13 +512,15 @@ static const struct v4l2_ioctl_ops gsc_m2m_ioctl_ops = { > .vidioc_try_fmt_vid_out_mplane = gsc_m2m_try_fmt_mplane, > .vidioc_s_fmt_vid_cap_mplane = gsc_m2m_s_fmt_mplane, > .vidioc_s_fmt_vid_out_mplane = gsc_m2m_s_fmt_mplane, > - .vidioc_reqbufs = gsc_m2m_reqbufs, > - .vidioc_expbuf = gsc_m2m_expbuf, > - .vidioc_querybuf = gsc_m2m_querybuf, > - .vidioc_qbuf = gsc_m2m_qbuf, > - .vidioc_dqbuf = gsc_m2m_dqbuf, > - .vidioc_streamon = gsc_m2m_streamon, > - .vidioc_streamoff = gsc_m2m_streamoff, > + > + .vidioc_reqbufs = v4l2_m2m_ioctl_reqbufs, > + .vidioc_querybuf = v4l2_m2m_ioctl_querybuf, > + .vidioc_expbuf = v4l2_m2m_ioctl_expbuf, > + .vidioc_qbuf = v4l2_m2m_ioctl_qbuf, > + .vidioc_dqbuf = v4l2_m2m_ioctl_dqbuf, > + > + .vidioc_streamon = v4l2_m2m_ioctl_streamon, > + .vidioc_streamoff = v4l2_m2m_ioctl_streamoff, > .vidioc_g_selection = gsc_m2m_g_selection, > .vidioc_s_selection = gsc_m2m_s_selection > }; > @@ -588,6 +539,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, > src_vq->mem_ops = &vb2_dma_contig_memops; > src_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer); > src_vq->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY; > + src_vq->lock = &ctx->gsc_dev->lock; > > ret = vb2_queue_init(src_vq); > if (ret) > @@ -601,6 +553,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, > dst_vq->mem_ops = &vb2_dma_contig_memops; > dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer); > dst_vq->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY; > + dst_vq->lock = &ctx->gsc_dev->lock; > > return vb2_queue_init(dst_vq); > } > @@ -648,6 +601,7 @@ static int gsc_m2m_open(struct file *file) > ret = PTR_ERR(ctx->m2m_ctx); > goto error_ctrls; > } > + ctx->fh.m2m_ctx = ctx->m2m_ctx; > > if (gsc->m2m.refcnt++ == 0) > set_bit(ST_M2M_OPEN, &gsc->state); > @@ -691,44 +645,13 @@ static int gsc_m2m_release(struct file *file) > return 0; > } > > -static unsigned int gsc_m2m_poll(struct file *file, > - struct poll_table_struct *wait) > -{ > - struct gsc_ctx *ctx = fh_to_ctx(file->private_data); > - struct gsc_dev *gsc = ctx->gsc_dev; > - int ret; > - > - if (mutex_lock_interruptible(&gsc->lock)) > - return -ERESTARTSYS; > - > - ret = v4l2_m2m_poll(file, ctx->m2m_ctx, wait); > - mutex_unlock(&gsc->lock); > - > - return ret; > -} > - > -static int gsc_m2m_mmap(struct file *file, struct vm_area_struct *vma) > -{ > - struct gsc_ctx *ctx = fh_to_ctx(file->private_data); > - struct gsc_dev *gsc = ctx->gsc_dev; > - int ret; > - > - if (mutex_lock_interruptible(&gsc->lock)) > - return -ERESTARTSYS; > - > - ret = v4l2_m2m_mmap(file, ctx->m2m_ctx, vma); > - mutex_unlock(&gsc->lock); > - > - return ret; > -} > - > static const struct v4l2_file_operations gsc_m2m_fops = { > .owner = THIS_MODULE, > .open = gsc_m2m_open, > .release = gsc_m2m_release, > - .poll = gsc_m2m_poll, > + .poll = v4l2_m2m_fop_poll, > .unlocked_ioctl = video_ioctl2, > - .mmap = gsc_m2m_mmap, > + .mmap = v4l2_m2m_fop_mmap, > }; > > static struct v4l2_m2m_ops gsc_m2m_ops = { > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/30/2013 11:50 AM, Hans Verkuil wrote: > On 09/13/2013 02:56 PM, Sylwester Nawrocki wrote: >> > Simplify the driver by using the m2m ioctl and vb2 helpers. >> > >> > TODO: Add setting of default initial format. > > So this patch can't be applied yet. Indeed, it's just an RFC series, I wanted it to give an overview of how much code can be eliminated with those helpers and posted this series regardless of availability of the pre-requisite cleanups, which I might or might not have time to prepare. Anyway I've already written those missing patches and these may be included in the final series, as soon as people Ack and test the patches where I couldn't test myself. > Other than that it looks good, but I won't ack it since it introduces a regression > as long as the TODO isn't addressed. Thanks, whole series updated to follow. -- Regards, Sylwester -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/media/platform/exynos-gsc/gsc-core.h b/drivers/media/platform/exynos-gsc/gsc-core.h index cc19bba..1afad32 100644 --- a/drivers/media/platform/exynos-gsc/gsc-core.h +++ b/drivers/media/platform/exynos-gsc/gsc-core.h @@ -464,18 +464,6 @@ static inline void gsc_hw_clear_irq(struct gsc_dev *dev, int irq) writel(cfg, dev->regs + GSC_IRQ); } -static inline void gsc_lock(struct vb2_queue *vq) -{ - struct gsc_ctx *ctx = vb2_get_drv_priv(vq); - mutex_lock(&ctx->gsc_dev->lock); -} - -static inline void gsc_unlock(struct vb2_queue *vq) -{ - struct gsc_ctx *ctx = vb2_get_drv_priv(vq); - mutex_unlock(&ctx->gsc_dev->lock); -} - static inline bool gsc_ctx_state_is_set(u32 mask, struct gsc_ctx *ctx) { unsigned long flags; diff --git a/drivers/media/platform/exynos-gsc/gsc-m2m.c b/drivers/media/platform/exynos-gsc/gsc-m2m.c index 40a73f7..4f5d6cb 100644 --- a/drivers/media/platform/exynos-gsc/gsc-m2m.c +++ b/drivers/media/platform/exynos-gsc/gsc-m2m.c @@ -262,8 +262,8 @@ static struct vb2_ops gsc_m2m_qops = { .queue_setup = gsc_m2m_queue_setup, .buf_prepare = gsc_m2m_buf_prepare, .buf_queue = gsc_m2m_buf_queue, - .wait_prepare = gsc_unlock, - .wait_finish = gsc_lock, + .wait_prepare = vb2_ops_wait_prepare, + .wait_finish = vb2_ops_wait_finish, .stop_streaming = gsc_m2m_stop_streaming, .start_streaming = gsc_m2m_start_streaming, }; @@ -376,57 +376,6 @@ static int gsc_m2m_reqbufs(struct file *file, void *fh, return v4l2_m2m_reqbufs(file, ctx->m2m_ctx, reqbufs); } -static int gsc_m2m_expbuf(struct file *file, void *fh, - struct v4l2_exportbuffer *eb) -{ - struct gsc_ctx *ctx = fh_to_ctx(fh); - return v4l2_m2m_expbuf(file, ctx->m2m_ctx, eb); -} - -static int gsc_m2m_querybuf(struct file *file, void *fh, - struct v4l2_buffer *buf) -{ - struct gsc_ctx *ctx = fh_to_ctx(fh); - return v4l2_m2m_querybuf(file, ctx->m2m_ctx, buf); -} - -static int gsc_m2m_qbuf(struct file *file, void *fh, - struct v4l2_buffer *buf) -{ - struct gsc_ctx *ctx = fh_to_ctx(fh); - return v4l2_m2m_qbuf(file, ctx->m2m_ctx, buf); -} - -static int gsc_m2m_dqbuf(struct file *file, void *fh, - struct v4l2_buffer *buf) -{ - struct gsc_ctx *ctx = fh_to_ctx(fh); - return v4l2_m2m_dqbuf(file, ctx->m2m_ctx, buf); -} - -static int gsc_m2m_streamon(struct file *file, void *fh, - enum v4l2_buf_type type) -{ - struct gsc_ctx *ctx = fh_to_ctx(fh); - - /* The source and target color format need to be set */ - if (V4L2_TYPE_IS_OUTPUT(type)) { - if (!gsc_ctx_state_is_set(GSC_SRC_FMT, ctx)) - return -EINVAL; - } else if (!gsc_ctx_state_is_set(GSC_DST_FMT, ctx)) { - return -EINVAL; - } - - return v4l2_m2m_streamon(file, ctx->m2m_ctx, type); -} - -static int gsc_m2m_streamoff(struct file *file, void *fh, - enum v4l2_buf_type type) -{ - struct gsc_ctx *ctx = fh_to_ctx(fh); - return v4l2_m2m_streamoff(file, ctx->m2m_ctx, type); -} - /* Return 1 if rectangle a is enclosed in rectangle b, or 0 otherwise. */ static int is_rectangle_enclosed(struct v4l2_rect *a, struct v4l2_rect *b) { @@ -563,13 +512,15 @@ static const struct v4l2_ioctl_ops gsc_m2m_ioctl_ops = { .vidioc_try_fmt_vid_out_mplane = gsc_m2m_try_fmt_mplane, .vidioc_s_fmt_vid_cap_mplane = gsc_m2m_s_fmt_mplane, .vidioc_s_fmt_vid_out_mplane = gsc_m2m_s_fmt_mplane, - .vidioc_reqbufs = gsc_m2m_reqbufs, - .vidioc_expbuf = gsc_m2m_expbuf, - .vidioc_querybuf = gsc_m2m_querybuf, - .vidioc_qbuf = gsc_m2m_qbuf, - .vidioc_dqbuf = gsc_m2m_dqbuf, - .vidioc_streamon = gsc_m2m_streamon, - .vidioc_streamoff = gsc_m2m_streamoff, + + .vidioc_reqbufs = v4l2_m2m_ioctl_reqbufs, + .vidioc_querybuf = v4l2_m2m_ioctl_querybuf, + .vidioc_expbuf = v4l2_m2m_ioctl_expbuf, + .vidioc_qbuf = v4l2_m2m_ioctl_qbuf, + .vidioc_dqbuf = v4l2_m2m_ioctl_dqbuf, + + .vidioc_streamon = v4l2_m2m_ioctl_streamon, + .vidioc_streamoff = v4l2_m2m_ioctl_streamoff, .vidioc_g_selection = gsc_m2m_g_selection, .vidioc_s_selection = gsc_m2m_s_selection }; @@ -588,6 +539,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, src_vq->mem_ops = &vb2_dma_contig_memops; src_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer); src_vq->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY; + src_vq->lock = &ctx->gsc_dev->lock; ret = vb2_queue_init(src_vq); if (ret) @@ -601,6 +553,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, dst_vq->mem_ops = &vb2_dma_contig_memops; dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer); dst_vq->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY; + dst_vq->lock = &ctx->gsc_dev->lock; return vb2_queue_init(dst_vq); } @@ -648,6 +601,7 @@ static int gsc_m2m_open(struct file *file) ret = PTR_ERR(ctx->m2m_ctx); goto error_ctrls; } + ctx->fh.m2m_ctx = ctx->m2m_ctx; if (gsc->m2m.refcnt++ == 0) set_bit(ST_M2M_OPEN, &gsc->state); @@ -691,44 +645,13 @@ static int gsc_m2m_release(struct file *file) return 0; } -static unsigned int gsc_m2m_poll(struct file *file, - struct poll_table_struct *wait) -{ - struct gsc_ctx *ctx = fh_to_ctx(file->private_data); - struct gsc_dev *gsc = ctx->gsc_dev; - int ret; - - if (mutex_lock_interruptible(&gsc->lock)) - return -ERESTARTSYS; - - ret = v4l2_m2m_poll(file, ctx->m2m_ctx, wait); - mutex_unlock(&gsc->lock); - - return ret; -} - -static int gsc_m2m_mmap(struct file *file, struct vm_area_struct *vma) -{ - struct gsc_ctx *ctx = fh_to_ctx(file->private_data); - struct gsc_dev *gsc = ctx->gsc_dev; - int ret; - - if (mutex_lock_interruptible(&gsc->lock)) - return -ERESTARTSYS; - - ret = v4l2_m2m_mmap(file, ctx->m2m_ctx, vma); - mutex_unlock(&gsc->lock); - - return ret; -} - static const struct v4l2_file_operations gsc_m2m_fops = { .owner = THIS_MODULE, .open = gsc_m2m_open, .release = gsc_m2m_release, - .poll = gsc_m2m_poll, + .poll = v4l2_m2m_fop_poll, .unlocked_ioctl = video_ioctl2, - .mmap = gsc_m2m_mmap, + .mmap = v4l2_m2m_fop_mmap, }; static struct v4l2_m2m_ops gsc_m2m_ops = {