Message ID | 20241016134640.3648224-1-chenghaoyang@chromium.org |
---|---|
State | Rejected |
Headers | show |
Series |
|
Related | show |
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 > >
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