Hi Javier,
Am Montag, den 03.09.2012, 14:18 +0200 schrieb javier Martin:
> On 31 August 2012 10:11, Philipp Zabel <p.zabel@xxxxxxxxxxxxxx> wrote:
> > Add a 1 second timeout for each PIC_RUN command to the CODA. In
> > case it locks up, stop all queues and dequeue remaining buffers.
> >
> > Signed-off-by: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>
> > ---
> > Changes since v2:
> > - Call cancel_delayed_work in coda_stop_streaming instead of coda_irq_handler.
> > ---
> > drivers/media/platform/coda.c | 21 +++++++++++++++++++++
> > 1 file changed, 21 insertions(+)
> >
> > diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c
> > index 7bc2d87..6e3f026 100644
> > --- a/drivers/media/platform/coda.c
> > +++ b/drivers/media/platform/coda.c
> > @@ -137,6 +137,7 @@ struct coda_dev {
> > struct vb2_alloc_ctx *alloc_ctx;
> > struct list_head instances;
> > unsigned long instance_mask;
> > + struct delayed_work timeout;
> > };
> >
> > struct coda_params {
> > @@ -723,6 +724,9 @@ static void coda_device_run(void *m2m_priv)
> > CODA7_REG_BIT_AXI_SRAM_USE);
> > }
> >
> > + /* 1 second timeout in case CODA locks up */
> > + schedule_delayed_work(&dev->timeout, HZ);
> > +
> > coda_command_async(ctx, CODA_COMMAND_PIC_RUN);
> > }
> >
> > @@ -1221,6 +1225,8 @@ static int coda_stop_streaming(struct vb2_queue *q)
> > }
> >
> > if (!ctx->rawstreamon && !ctx->compstreamon) {
> > + cancel_delayed_work(&dev->timeout);
> > +
>
> Since 'schedule_delayed_work()' is called for each frame and
> 'cancel_delayed_work()' is called only when stopping the streaming,
> the timeout will always trigger after 1 second.
> I can confirm this due some tests where I always get this message
> after one second: coda coda-imx27.0: CODA PIC_RUN timeout, stopping
> all streams
Yes, removing the __cancel_delayed_work in coda_irq_handler was a
mistake.
> Please find some of my doubts below:
>
> Do we really need this patch? Couldn't you just use
> 'coda_command_sync()' for CODA_COMMAND_PIC_RUN? Why did you changed
> 'cancel_delayed_work()' from the IRQ to stop streaming? I think the
> former was correct, wasn't it?
Ideally, the CODA shouldn't hang. I've seen it happen, though - whether
because of firmware or driver bugs I couldn't say.
Using coda_command_sync would mean spinning in coda_wait_timeout all the
time. I guess we could put device_run to sleep with
wait_for_completion_interruptible_timeout and just complete the
completion in the irq handler.
regards
Philipp
--
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
