On Wed, 2018-07-18 at 12:21 +0200, Hans Verkuil wrote:
> On 12/07/18 17:43, Ezequiel Garcia wrote:
> > v4l2_m2m_job_finish() is typically called in interrupt context.
> >
> > Some implementation of .device_run might sleep, and so it's
> > desirable to avoid calling it directly from
> > v4l2_m2m_job_finish(), thus avoiding .device_run from running
> > in interrupt context.
> >
> > Implement a deferred context that gets scheduled by
> > v4l2_m2m_job_finish().
> >
> > The worker calls v4l2_m2m_try_schedule(), which makes sure
> > a single job is running at the same time, so it's safe to
> > call it from different executions context.
>
> I am not sure about this. I think that the only thing that needs to
> run in the work queue is the call to device_run. Everything else
> up to that point runs fine in interrupt context.
>
Yes, I think you are right. I originally went for the safer path
of running v4l2_m2m_try_schedule in the worker, but I think
it works to run only .device_run.
So there will be two paths to .device_run:
* qbuf/streamon which checks if a job is possible
for this or any other m2m context, and then
calls .device_run for the selected m2m context.
* job_finish will check if there is a job possible,
for this or any other m2m context, then
select a context by setting the current context
and setting TRANS_RUNNING, then schedule the worker.
Only one m2m context can be running at the same time, and
v4l2_m2m_cancel_job is called by v4l2_m2m_streamoff and
v4l2_m2m_ctx_release, guaranteeing we don't try to run
a job on a stopped/released m2m context.
> While we're on it: I see that v4l2_m2m_prepare_buf() also calls
> v4l2_m2m_try_schedule(): I don't think it should do that since
> prepare_buf does not actually queue a new buffer to the driver.
>
Yes, you are right about that. I'll remove it.
Thanks for reviewing,
Eze
