Em Tue, 14 Aug 2018 09:28:42 +0200
Hans Verkuil <hverkuil@xxxxxxxxx> escreveu:
> On 13/08/18 17:05, Mauro Carvalho Chehab wrote:
> > Em Sat, 4 Aug 2018 14:45:22 +0200
> > Hans Verkuil <hverkuil@xxxxxxxxx> escreveu:
> >
> >> From: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> >>
> >> v4l2_ctrl uses mutexes, so we can't setup a ctrl_handler in
> >> interrupt context. Switch to a workqueue instead and drop the timer.
> >>
> >> Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> >
> > Reviewed-by: Mauro Carvalho Chehab <mchehab+samsung@xxxxxxxxxx>
> >
> > Shouldn't this come earlier at the series (before adding request API
> > support to m2m) in order to avoid regressions?
>
> ??? At this stage vim2m doesn't support the request API yet. It's the next
> patch that adds that (and that's when this patch is needed for it to work).
Ah, OK!
>
> Regards,
>
> Hans
>
> >
> >> ---
> >> drivers/media/platform/vim2m.c | 25 ++++++++++---------------
> >> 1 file changed, 10 insertions(+), 15 deletions(-)
> >>
> >> diff --git a/drivers/media/platform/vim2m.c b/drivers/media/platform/vim2m.c
> >> index 462099a141e4..6f87ef025ff1 100644
> >> --- a/drivers/media/platform/vim2m.c
> >> +++ b/drivers/media/platform/vim2m.c
> >> @@ -3,7 +3,8 @@
> >> *
> >> * This is a virtual device driver for testing mem-to-mem videobuf framework.
> >> * It simulates a device that uses memory buffers for both source and
> >> - * destination, processes the data and issues an "irq" (simulated by a timer).
> >> + * destination, processes the data and issues an "irq" (simulated by a delayed
> >> + * workqueue).
> >> * The device is capable of multi-instance, multi-buffer-per-transaction
> >> * operation (via the mem2mem framework).
> >> *
> >> @@ -19,7 +20,6 @@
> >> #include <linux/module.h>
> >> #include <linux/delay.h>
> >> #include <linux/fs.h>
> >> -#include <linux/timer.h>
> >> #include <linux/sched.h>
> >> #include <linux/slab.h>
> >>
> >> @@ -148,7 +148,7 @@ struct vim2m_dev {
> >> struct mutex dev_mutex;
> >> spinlock_t irqlock;
> >>
> >> - struct timer_list timer;
> >> + struct delayed_work work_run;
> >>
> >> struct v4l2_m2m_dev *m2m_dev;
> >> };
> >> @@ -336,12 +336,6 @@ static int device_process(struct vim2m_ctx *ctx,
> >> return 0;
> >> }
> >>
> >> -static void schedule_irq(struct vim2m_dev *dev, int msec_timeout)
> >> -{
> >> - dprintk(dev, "Scheduling a simulated irq\n");
> >> - mod_timer(&dev->timer, jiffies + msecs_to_jiffies(msec_timeout));
> >> -}
> >> -
> >> /*
> >> * mem2mem callbacks
> >> */
> >> @@ -387,13 +381,14 @@ static void device_run(void *priv)
> >>
> >> device_process(ctx, src_buf, dst_buf);
> >>
> >> - /* Run a timer, which simulates a hardware irq */
> >> - schedule_irq(dev, ctx->transtime);
> >> + /* Run delayed work, which simulates a hardware irq */
> >> + schedule_delayed_work(&dev->work_run, msecs_to_jiffies(ctx->transtime));
> >> }
> >>
> >> -static void device_isr(struct timer_list *t)
> >> +static void device_work(struct work_struct *w)
> >> {
> >> - struct vim2m_dev *vim2m_dev = from_timer(vim2m_dev, t, timer);
> >> + struct vim2m_dev *vim2m_dev =
> >> + container_of(w, struct vim2m_dev, work_run.work);
> >> struct vim2m_ctx *curr_ctx;
> >> struct vb2_v4l2_buffer *src_vb, *dst_vb;
> >> unsigned long flags;
> >> @@ -805,6 +800,7 @@ static void vim2m_stop_streaming(struct vb2_queue *q)
> >> struct vb2_v4l2_buffer *vbuf;
> >> unsigned long flags;
> >>
> >> + flush_scheduled_work();
> >> for (;;) {
> >> if (V4L2_TYPE_IS_OUTPUT(q->type))
> >> vbuf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
> >> @@ -1015,6 +1011,7 @@ static int vim2m_probe(struct platform_device *pdev)
> >> vfd = &dev->vfd;
> >> vfd->lock = &dev->dev_mutex;
> >> vfd->v4l2_dev = &dev->v4l2_dev;
> >> + INIT_DELAYED_WORK(&dev->work_run, device_work);
> >>
> >> ret = video_register_device(vfd, VFL_TYPE_GRABBER, 0);
> >> if (ret) {
> >> @@ -1026,7 +1023,6 @@ static int vim2m_probe(struct platform_device *pdev)
> >> v4l2_info(&dev->v4l2_dev,
> >> "Device registered as /dev/video%d\n", vfd->num);
> >>
> >> - timer_setup(&dev->timer, device_isr, 0);
> >> platform_set_drvdata(pdev, dev);
> >>
> >> dev->m2m_dev = v4l2_m2m_init(&m2m_ops);
> >> @@ -1083,7 +1079,6 @@ static int vim2m_remove(struct platform_device *pdev)
> >> media_device_cleanup(&dev->mdev);
> >> #endif
> >> v4l2_m2m_release(dev->m2m_dev);
> >> - del_timer_sync(&dev->timer);
> >> video_unregister_device(&dev->vfd);
> >> v4l2_device_unregister(&dev->v4l2_dev);
> >>
> >
> >
> >
> > Thanks,
> > Mauro
> >
>
Thanks,
Mauro
