libcamera: v4l2: Support changing poller thread for V4L2VideoDevice

21643 diff mbox series

From: Han-Lin Chen <hanlinchen@chromium.org>

Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org>
Co-developed-by: Yudhistira Erlandinata <yerlandinata@chromium.org>
Signed-off-by: Yudhistira Erlandinata <yerlandinata@chromium.org>
Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>
Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>
---
 include/libcamera/internal/v4l2_videodevice.h |  2 ++
 src/libcamera/v4l2_videodevice.cpp            | 13 +++++++++++++
 2 files changed, 15 insertions(+)

Quoting Harvey Yang (2024-10-16 14:46:33)
> From: Han-Lin Chen <hanlinchen@chromium.org>
> 

Could we get something more detailed in here?

What use case does this support ? Are there any restrictions on how/when
you move the thread? What's the default thread used to poll - and why
isn't this sufficient?

This seems like the sort of thing that deserves a test in
test/v4l2_videodevice.cpp to make sure the devices still operate after
the poll thread has been moved ?


> Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org>
> Co-developed-by: Yudhistira Erlandinata <yerlandinata@chromium.org>
> Signed-off-by: Yudhistira Erlandinata <yerlandinata@chromium.org>
> Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>
> Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>
> ---
>  include/libcamera/internal/v4l2_videodevice.h |  2 ++
>  src/libcamera/v4l2_videodevice.cpp            | 13 +++++++++++++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h
> index f021c2a01..f8d017b01 100644
> --- a/include/libcamera/internal/v4l2_videodevice.h
> +++ b/include/libcamera/internal/v4l2_videodevice.h
> @@ -232,6 +232,8 @@ public:
>  
>         V4L2PixelFormat toV4L2PixelFormat(const PixelFormat &pixelFormat) const;
>  
> +       void changePollerThread(Thread *thread);
> +
>  protected:
>         std::string logPrefix() const override;
>  
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index 14eba0561..fc6b88ec1 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -2120,6 +2120,19 @@ V4L2PixelFormat V4L2VideoDevice::toV4L2PixelFormat(const PixelFormat &pixelForma
>         return {};
>  }
>  
> +/**
> + * \brief Move the event notifier and timer to \a thread
> + * \param[in] thread The thread to poll on
> + */
> +void V4L2VideoDevice::changePollerThread(Thread *thread)
> +{
> +       if (!isOpen())
> +               return;
> +
> +       fdBufferNotifier_->moveToThread(thread);
> +       watchdog_.moveToThread(thread);
> +}
> +
>  /**
>   * \class V4L2M2MDevice
>   * \brief Memory-to-Memory video device
> -- 
> 2.47.0.rc1.288.g06298d1525-goog
>

Hi Kieran,

On Sun, Oct 20, 2024 at 6:15 AM Kieran Bingham
<kieran.bingham@ideasonboard.com> wrote:
>
> Quoting Harvey Yang (2024-10-16 14:46:33)
> > From: Han-Lin Chen <hanlinchen@chromium.org>
> >
>
> Could we get something more detailed in here?
>
> What use case does this support ? Are there any restrictions on how/when
> you move the thread? What's the default thread used to poll - and why
> isn't this sufficient?

True, mtkisp7 actually doesn't need to change the owner thread of
V4L2VideoDevice. [1]

Let's ignore this patch.

BR,
Harvey

[1]: https://chromium-review.googlesource.com/c/chromiumos/third_party/libcamera/+/5953203


>
> This seems like the sort of thing that deserves a test in
> test/v4l2_videodevice.cpp to make sure the devices still operate after
> the poll thread has been moved ?
>
>
> > Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org>
> > Co-developed-by: Yudhistira Erlandinata <yerlandinata@chromium.org>
> > Signed-off-by: Yudhistira Erlandinata <yerlandinata@chromium.org>
> > Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>
> > Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>
> > ---
> >  include/libcamera/internal/v4l2_videodevice.h |  2 ++
> >  src/libcamera/v4l2_videodevice.cpp            | 13 +++++++++++++
> >  2 files changed, 15 insertions(+)
> >
> > diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h
> > index f021c2a01..f8d017b01 100644
> > --- a/include/libcamera/internal/v4l2_videodevice.h
> > +++ b/include/libcamera/internal/v4l2_videodevice.h
> > @@ -232,6 +232,8 @@ public:
> >
> >         V4L2PixelFormat toV4L2PixelFormat(const PixelFormat &pixelFormat) const;
> >
> > +       void changePollerThread(Thread *thread);
> > +
> >  protected:
> >         std::string logPrefix() const override;
> >
> > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> > index 14eba0561..fc6b88ec1 100644
> > --- a/src/libcamera/v4l2_videodevice.cpp
> > +++ b/src/libcamera/v4l2_videodevice.cpp
> > @@ -2120,6 +2120,19 @@ V4L2PixelFormat V4L2VideoDevice::toV4L2PixelFormat(const PixelFormat &pixelForma
> >         return {};
> >  }
> >
> > +/**
> > + * \brief Move the event notifier and timer to \a thread
> > + * \param[in] thread The thread to poll on
> > + */
> > +void V4L2VideoDevice::changePollerThread(Thread *thread)
> > +{
> > +       if (!isOpen())
> > +               return;
> > +
> > +       fdBufferNotifier_->moveToThread(thread);
> > +       watchdog_.moveToThread(thread);
> > +}
> > +
> >  /**
> >   * \class V4L2M2MDevice
> >   * \brief Memory-to-Memory video device
> > --
> > 2.47.0.rc1.288.g06298d1525-goog
> >