Re: [PATCH 4/5] media: coda: Remove pic_run_work worker

[Ezequiel Garcia <ezequiel@collabora.com>]

On Fri, 2019-04-26 at 10:11 +0200, Philipp Zabel wrote:
> On Thu, 2019-04-25 at 15:35 -0300, Ezequiel Garcia wrote:
> > There isn't any reason to run the mem2mem job on a separate worker,
> > because the mem2mem framework guarantees that device_run will never
> > run in interrupt context.
> 
> The purpose of the workqueue is to serialize BIT processor commands,
> currently the PIC_RUN commands issued by the mem2mem framework (as well
> as SEQ_INIT, SET_FRAME_BUF, and ENCODE_HEADER) against the SEQ_END
> command issued directly from the STREAMOFF ioctl.

Right, but that's serialized by the coda_mutex, not by the worker, right?

> Further, to fully support the stateful decoder API we'll have to move
> SEQ_INIT out of the mem2mem device_run as well, since that should be
> called on queued OUTPUT buffers before the CAPTURE side is even
> streaming.
> 

Isn't that already done? I see SEQ_INIT being issued in start_streaming.
In what sense is this driver currently violating the decoder spec?

So, returning to the serialization. I believe commands are still
serialized after this change.

The pic_run_work worker is only queued from .device_run.
Only one job can run on a context at any given time,
but multiple contexts can run in parallel.

Inside the worker, the coda_mutex serializes the commands.
The worker waits until the command has finished execution.

So, with this commit, there is no longer a worker, but commands
are still serialized by the mutex and the fact that the command
is completed in .device_run.

That being said, the worker does makes the serialization
more clear, so I think dropping this patch is better.

Perhaps adding a small comment so the purpose of pic_run_work
is clear.

Thanks,
Eze