[v3,0/3] Fix uvcvideo pipelinehandler keeping /dev/video# open

21075 mbox series

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 v3 of my series making the uvcvideo pipeline handler open
/dev/video# on acquire and close it on release to fix this.

Changes in v3:
- s/method/function/ in commit msg
- Put MutexLocker locker(data_->openLock_); in a { } context
  to reduce the time the lock is held to the minimum time necessary
- Add Reviewed-by-s

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

*** BLURB HERE ***

Hans de Goede (3):
  pipeline_handler: Add acquireDevice() function 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  | 54 ++++++++++++++++-
 src/libcamera/pipeline_handler.cpp            | 60 ++++++++++++++-----
 4 files changed, 104 insertions(+), 24 deletions(-)

Hi Hans,

Applied and pushed. Thank you for fixing this longstanding issue.

On Fri, Aug 30, 2024 at 01:12:04PM +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 v3 of my series making the uvcvideo pipeline handler open
> /dev/video# on acquire and close it on release to fix this.
> 
> Changes in v3:
> - s/method/function/ in commit msg
> - Put MutexLocker locker(data_->openLock_); in a { } context
>   to reduce the time the lock is held to the minimum time necessary
> - Add Reviewed-by-s
> 
> 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
> 
> *** BLURB HERE ***
> 
> Hans de Goede (3):
>   pipeline_handler: Add acquireDevice() function 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  | 54 ++++++++++++++++-
>  src/libcamera/pipeline_handler.cpp            | 60 ++++++++++++++-----
>  4 files changed, 104 insertions(+), 24 deletions(-)