[libcamera-devel] [PATCH 2/3] vimc: Serialize vimc_streamer_s_stream()

[Dafna Hirschfeld]

Hi,

On Sat, May 18, 2019 at 4:14 AM Niklas Söderlund <
niklas.soderlund+renesas at ragnatech.se> wrote:

> Prepare for multiple video streams from the same sensor by serializing
> vimc_streamer_s_stream(). Multiple streams will allow for multiple
> concurrent calls to this function that could involve the same
> subdevices.
>
> If that happens the internal state of the involved subdevices could go
> out of sync as they are being started and stopped at the same time,
> prevent this by serializing starting and stopping of the vimc streamer.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  drivers/media/platform/vimc/vimc-streamer.c | 23 ++++++++++++++-------
>  1 file changed, 16 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/media/platform/vimc/vimc-streamer.c
> b/drivers/media/platform/vimc/vimc-streamer.c
> index 26b6742594890b16..514690dca3b1187b 100644
> --- a/drivers/media/platform/vimc/vimc-streamer.c
> +++ b/drivers/media/platform/vimc/vimc-streamer.c
> @@ -153,39 +153,48 @@ int vimc_streamer_s_stream(struct vimc_stream
> *stream,
>                            struct vimc_ent_device *ved,
>                            int enable)
>  {
> +       static DEFINE_MUTEX(vimc_streamer_lock);
>
This is unusual to have static global in a function, maybe better declare
it outside the function.


>         int ret;
>
>         if (!stream || !ved)
>                 return -EINVAL;
>
> +       ret = mutex_lock_interruptible(&vimc_streamer_lock);
> +       if (ret)
> +               return ret;
> +
>         if (enable) {
>                 if (stream->kthread)
> -                       return 0;
> +                       goto out;
>
>                 ret = vimc_streamer_pipeline_init(stream, ved);
>                 if (ret)
> -                       return ret;
> +                       goto out;
>
>                 stream->kthread = kthread_run(vimc_streamer_thread, stream,
>                                               "vimc-streamer thread");
>
> -               if (IS_ERR(stream->kthread))
> -                       return PTR_ERR(stream->kthread);
> +               if (IS_ERR(stream->kthread)) {
> +                       ret = PTR_ERR(stream->kthread);
> +                       goto out;
> +               }
>
>         } else {
>                 if (!stream->kthread)
> -                       return 0;
> +                       goto out;
>
>                 ret = kthread_stop(stream->kthread);
>                 if (ret)
> -                       return ret;
> +                       goto out;
>
>                 stream->kthread = NULL;
>
>                 vimc_streamer_pipeline_terminate(stream);
>         }
> +out:
> +       mutex_unlock(&vimc_streamer_lock);
>
> -       return 0;
> +       return ret;
>  }
>  EXPORT_SYMBOL_GPL(vimc_streamer_s_stream);
>
>
The frame allocated by each vimc entity  is the same frame for all
streaming threads,
isn't access to the frame should also be serialized?

Thanks,
Dafna

-- 
> 2.21.0
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20191202/65a923bb/attachment-0001.htm>