| Message ID | 20261027213240.140663-1-rylos78@gmail.com |
|---|---|
| Headers | show |
| Series |
|
| Related | show |
Hello On Tue, Oct 27, 2026 at 10:32:38PM +0100, Marco Ziliani wrote: > Lens drivers power the lens up when their subdev is opened, and on ACPI > platforms ipu-bridge links the lens to its sensor with a runtime PM > device link. CameraLens opens the lens subdev in init() and never I wonder if the device link in the ipu_bridge is the right thing to do. Keeping the sensor powered as long as the vcm is powered (or open() in this case) doesn't seem desirable ? > closes it, so with a long-lived CameraManager such as the WirePlumber > libcamera monitor the lens and the sensor stay powered all the time. Of course, even if we remove the device_link (or make it STATELESS to only guarantee suspend/resume ordering), you'll have the VCM powered on all the time, but not the sensor at least. > > On a Surface Pro 5 (IPU3) the rear ov8865 and its dw9719 lens are > active for as long as WirePlumber runs, while the front ov5693, which > has no lens, is runtime suspended when idle. > > Patch 1 closes the lens subdev at the end of CameraLens::init(), adds > open() and close(), and makes setFocusPosition() reopen the subdev on > demand, so pipeline handlers that are not converted keep working. > Patch 2 converts the IPU3 pipeline handler to open the lens on start() > and close it on stop. > > I'm sending this as an RFC because the topic has been discussed > before. Harvey Yang proposed open() and close() in CameraLens in > October 2024 [1], and Laurent preferred to address lens power > management in the kernel, where Hans de Goede had proposed powering > the VCM along with its sensor [2]. As far as I can tell neither > approach has landed. This series only uses the existing kernel > behaviour and fixes the idle power draw on IPU3 systems today. If the > preferred direction is still the kernel one, I'm happy to help test > that instead on the Surface Pro 5. > > Tested on a Surface Pro 5 with kernel 7.2 and libcamera 0.7.2 with the > same changes: > - idle with WirePlumber running: ov8865 and dw9719 runtime suspended > (before: both active); > - cam -c 1 --capture: both active, autofocus moves the lens > (focus_absolute 60 -> 268), 30 fps; > - about 1 s after the capture stops: both runtime suspended again; > - front camera (ov5693, no lens) unchanged. > On master the series builds with the ipu3 and rpi/vc4 pipelines; the > Raspberry Pi pipeline handler is not converted and was not tested on > hardware. > > [1] https://patchwork.libcamera.org/patch/21649/ > [2] https://lore.kernel.org/linux-media/20240901211834.145186-1-hdegoede@redhat.com/ Ok, both approaches clearly didn't progress. On the kernel side, while I agree that .enable/disable_streams doesn't really apply to vcm as a concept, extending the v4l2 core to call .enable_streams on vcm would allow you to power it up only when needed. The userspace approach you and Harvey proposed was refused on the ground that we need better kernel APIs, but as 2 years have passed and we still are consuming power for no reasons by keeping the sensor powered on, so something has to be done I guess. Let's maybe start discussing if the device_link with DL_FLAG_PM_RUNTIME in ipu_bridge is desirable or not. > > Marco Ziliani (2): > libcamera: camera_lens: Close the lens subdev when not in use > libcamera: pipeline: ipu3: Power the lens only while the camera runs > > include/libcamera/internal/camera_lens.h | 2 ++ > src/libcamera/camera_lens.cpp | 42 ++++++++++++++++++++++++ > src/libcamera/pipeline/ipu3/ipu3.cpp | 17 +++++++++- > 3 files changed, 60 insertions(+), 1 deletion(-) > > -- > 2.56.0 >
On Wed, Oct 28, 2026 at 10:11:51AM +0100, Jacopo Mondi wrote: > Hello > > On Tue, Oct 27, 2026 at 10:32:38PM +0100, Marco Ziliani wrote: > > Lens drivers power the lens up when their subdev is opened, and on ACPI > > platforms ipu-bridge links the lens to its sensor with a runtime PM > > device link. CameraLens opens the lens subdev in init() and never > > I wonder if the device link in the ipu_bridge is the right thing to > do. Keeping the sensor powered as long as the vcm is powered (or > open() in this case) doesn't seem desirable ? Ah well 8e3e916e23f5 ("media: ipu-bridge: Add a runtime-pm device-link between VCM and sensor") > > > closes it, so with a long-lived CameraManager such as the WirePlumber > > libcamera monitor the lens and the sensor stay powered all the time. > > Of course, even if we remove the device_link (or make it STATELESS to > only guarantee suspend/resume ordering), you'll have the VCM powered > on all the time, but not the sensor at least. > > > > > On a Surface Pro 5 (IPU3) the rear ov8865 and its dw9719 lens are > > active for as long as WirePlumber runs, while the front ov5693, which > > has no lens, is runtime suspended when idle. > > > > Patch 1 closes the lens subdev at the end of CameraLens::init(), adds > > open() and close(), and makes setFocusPosition() reopen the subdev on > > demand, so pipeline handlers that are not converted keep working. > > Patch 2 converts the IPU3 pipeline handler to open the lens on start() > > and close it on stop. > > > > I'm sending this as an RFC because the topic has been discussed > > before. Harvey Yang proposed open() and close() in CameraLens in > > October 2024 [1], and Laurent preferred to address lens power > > management in the kernel, where Hans de Goede had proposed powering > > the VCM along with its sensor [2]. As far as I can tell neither > > approach has landed. This series only uses the existing kernel > > behaviour and fixes the idle power draw on IPU3 systems today. If the > > preferred direction is still the kernel one, I'm happy to help test > > that instead on the Surface Pro 5. > > > > Tested on a Surface Pro 5 with kernel 7.2 and libcamera 0.7.2 with the > > same changes: > > - idle with WirePlumber running: ov8865 and dw9719 runtime suspended > > (before: both active); > > - cam -c 1 --capture: both active, autofocus moves the lens > > (focus_absolute 60 -> 268), 30 fps; > > - about 1 s after the capture stops: both runtime suspended again; > > - front camera (ov5693, no lens) unchanged. > > On master the series builds with the ipu3 and rpi/vc4 pipelines; the > > Raspberry Pi pipeline handler is not converted and was not tested on > > hardware. > > > > [1] https://patchwork.libcamera.org/patch/21649/ > > [2] https://lore.kernel.org/linux-media/20240901211834.145186-1-hdegoede@redhat.com/ > > Ok, both approaches clearly didn't progress. > > On the kernel side, while I agree that .enable/disable_streams doesn't > really apply to vcm as a concept, extending the v4l2 core to call > .enable_streams on vcm would allow you to power it up only when > needed. > > The userspace approach you and Harvey proposed was refused on the > ground that we need better kernel APIs, but as 2 years have passed and > we still are consuming power for no reasons by keeping the sensor > powered on, so something has to be done I guess. > > Let's maybe start discussing if the device_link with DL_FLAG_PM_RUNTIME > in ipu_bridge is desirable or not. > Apparently it is. As I don't see a new API materializing any time soon for VCM in Linux, I guess we should re-consider this open/close approach. Keeping a sensor powered for no reasons all the time is not acceptable imho. > > > > Marco Ziliani (2): > > libcamera: camera_lens: Close the lens subdev when not in use > > libcamera: pipeline: ipu3: Power the lens only while the camera runs > > > > include/libcamera/internal/camera_lens.h | 2 ++ > > src/libcamera/camera_lens.cpp | 42 ++++++++++++++++++++++++ > > src/libcamera/pipeline/ipu3/ipu3.cpp | 17 +++++++++- > > 3 files changed, 60 insertions(+), 1 deletion(-) > > > > -- > > 2.56.0 > >