| Message ID | 20240827164255.314432-1-hdegoede@redhat.com |
|---|---|
| Headers | show |
| Series |
|
| Related | show |
Hi Hans, I've reviewed the series and had minor comments only. I was considering addressing them myself and merging the patches, but I'd like to get your ack on the proposed changes in 3/3. If you're fine with them and can submit a v3, I'll merge that. On Tue, Aug 27, 2024 at 06:42:52PM +0200, Hans de Goede wrote: > Hi all, > > The uvcvideo pipeline handler always keeps the uvcvideo /dev/video# device > for a pipeline open after enumerating the camera. > > This is a problem for uvcvideo, as keeping the /dev/video# node open stops > the underlying USB device and the USB bus controller from being able to > enter runtime-suspend causing significant unnecessary power-usage. > > Here is v2 of my series making the uvcvideo pipeline handler open > /dev/video# on acquire and close it on release to fix this. > > Changes in v2: > - Drop the first 2 patches these have already been merged > - Add a note to both the doxygen documentation as well as to the commit > messages that opening/closing /dev/video# from acquire()/release() > as done by the uvcvideo pipeline handler is an exception and that this > behavior should not be copied by other pipeline handlers > - Other doxygen doc fixes / improvements > - Only unlock media devices on acquireDevice() failure if useCount_ == 0 > - Drop PipelineHandler::lock_, update "\context" in doxygen docs > > I've pushed this to the software-isp gitlab repo for CI: > https://gitlab.freedesktop.org/camera/libcamera-softisp/-/commits/pipeline-acquireDevice-v2 > > and it has passed CI. > > Regards, > > Hans > > > Hans de Goede (3): > pipeline_handler: Add acquireDevice() method to mirror existing > releaseDevice() > camera: Use invokeMethod() for pipe_->acquire() and pipe_->release() > uvcvideo: Implement acquireDevice() + releaseDevice() > > include/libcamera/internal/pipeline_handler.h | 8 +-- > src/libcamera/camera.cpp | 6 +- > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 45 ++++++++++++++ > src/libcamera/pipeline_handler.cpp | 60 ++++++++++++++----- > 4 files changed, 98 insertions(+), 21 deletions(-)
Hi all, The uvcvideo pipeline handler always keeps the uvcvideo /dev/video# device for a pipeline open after enumerating the camera. This is a problem for uvcvideo, as keeping the /dev/video# node open stops the underlying USB device and the USB bus controller from being able to enter runtime-suspend causing significant unnecessary power-usage. Here is v2 of my series making the uvcvideo pipeline handler open /dev/video# on acquire and close it on release to fix this. Changes in v2: - Drop the first 2 patches these have already been merged - Add a note to both the doxygen documentation as well as to the commit messages that opening/closing /dev/video# from acquire()/release() as done by the uvcvideo pipeline handler is an exception and that this behavior should not be copied by other pipeline handlers - Other doxygen doc fixes / improvements - Only unlock media devices on acquireDevice() failure if useCount_ == 0 - Drop PipelineHandler::lock_, update "\context" in doxygen docs I've pushed this to the software-isp gitlab repo for CI: https://gitlab.freedesktop.org/camera/libcamera-softisp/-/commits/pipeline-acquireDevice-v2 and it has passed CI. Regards, Hans Hans de Goede (3): pipeline_handler: Add acquireDevice() method to mirror existing releaseDevice() camera: Use invokeMethod() for pipe_->acquire() and pipe_->release() uvcvideo: Implement acquireDevice() + releaseDevice() include/libcamera/internal/pipeline_handler.h | 8 +-- src/libcamera/camera.cpp | 6 +- src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 45 ++++++++++++++ src/libcamera/pipeline_handler.cpp | 60 ++++++++++++++----- 4 files changed, 98 insertions(+), 21 deletions(-)