| Message ID | 20240901211834.145186-2-hdegoede@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | media: v4l: Call s_stream() on VCM when it is called on the associated sensor | expand |
Hi Hans, Thank you for the patch. On Sun, Sep 01, 2024 at 11:18:33PM +0200, Hans de Goede wrote: > Currently VCM drivers power-up the VCM as soon as the VCM's /dev node > is opened and through the runtime-pm device-link to the sensor this > also powers up the sensor. > > Powering up the VCM and sensor when the /dev node is opened is undesirable, > without a VCM sensors delay powering up until s_stream(1) is called. This > allows querying / negotiating capabilities without powering things up. > > Sometimes a long running daemon like pipewire may keep the /dev node open > all the time. The kernel should still be able to powerdown the VCM + sensor > in this scenario. > > VCM drivers should be able to do the same as sensor drivers and only > power-up the VCM when s_stream(1) is called on the VCM subdev, but this > requires that s_stream() is actually called on the VCM when the sensor > starts / stops streaming. .s_stream() doesn't conceptually make sense for VCMs. Furthermore, .s_stream() is being replaced with .enable_streams() and .disable_streams(), which will make even less sense. We need a different API. > The s_stream() call on sensor subdevs is done by CSI-receiver/ISP drivers. > To avoid needing to change all these call sites to also call s_stream() > on the VCM (if there is one) handle the VCM in the v4l2-core similar to how > the core takes care of turning on/off the privacy LED. This needs to come with a design rationale, documented in Documentation/. The design needs to explain the use cases. Lens motion may take time, which I expect will influence how we will need to handle power management. I'm not very comfortable handling this in v4l2-subdev.c, it seems that we'll hardcode use cases. Without a clear and detailed designed rationale, this patch feels we'll paint ourselves in a corner. We have enough badly designed (or not designed at all) APIs for cameras, it's time to do better. > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > drivers/media/v4l2-core/v4l2-async.c | 20 +++++++++++++++++++ > drivers/media/v4l2-core/v4l2-subdev.c | 28 +++++++++++++++++++++------ > include/media/v4l2-subdev.h | 2 ++ > 3 files changed, 44 insertions(+), 6 deletions(-) > > diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c > index ee884a8221fb..9b854f1d1051 100644 > --- a/drivers/media/v4l2-core/v4l2-async.c > +++ b/drivers/media/v4l2-core/v4l2-async.c > @@ -330,6 +330,11 @@ static int v4l2_async_create_ancillary_links(struct v4l2_async_notifier *n, > return 0; > } > > + if (sd->entity.function == MEDIA_ENT_F_LENS) { > + dev_dbg(n->sd->dev, "Using %s VCM\n", dev_name(sd->dev)); > + n->sd->vcm = sd; > + } > + > link = media_create_ancillary_link(&n->sd->entity, &sd->entity); > > return IS_ERR(link) ? PTR_ERR(link) : 0; > @@ -871,6 +876,21 @@ void v4l2_async_unregister_subdev(struct v4l2_subdev *sd) > if (!sd->async_list.next) > return; > > +#if defined(CONFIG_MEDIA_CONTROLLER) > + if (sd->entity.function == MEDIA_ENT_F_LENS && sd->v4l2_dev && sd->v4l2_dev->mdev) { > + struct media_entity *entity; > + > + media_device_for_each_entity(entity, sd->v4l2_dev->mdev) { > + struct v4l2_subdev *it = media_entity_to_v4l2_subdev(entity); > + > + if (it->vcm == sd) { > + dev_dbg(it->dev, "Clearing VCM\n"); > + it->vcm = NULL; > + } > + } > + } > +#endif > + > v4l2_subdev_put_privacy_led(sd); > > mutex_lock(&list_lock); > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c > index 7c5812d55315..24a68d90f686 100644 > --- a/drivers/media/v4l2-core/v4l2-subdev.c > +++ b/drivers/media/v4l2-core/v4l2-subdev.c > @@ -148,17 +148,33 @@ static int subdev_close(struct file *file) > } > #endif /* CONFIG_VIDEO_V4L2_SUBDEV_API */ > > -static void v4l2_subdev_enable_privacy_led(struct v4l2_subdev *sd) > +static void v4l2_subdev_enable_privacy_led_and_vcm(struct v4l2_subdev *sd) > { > #if IS_REACHABLE(CONFIG_LEDS_CLASS) > if (!IS_ERR_OR_NULL(sd->privacy_led)) > led_set_brightness(sd->privacy_led, > sd->privacy_led->max_brightness); > #endif > + > + if (sd->vcm && !sd->vcm_enabled && > + v4l2_subdev_has_op(sd->vcm, video, s_stream)) { > + int ret; > + > + ret = v4l2_subdev_call(sd->vcm, video, s_stream, 1); > + if (ret) > + dev_err(sd->vcm->dev, "Error powering on VCM: %d\n", ret); > + else > + sd->vcm_enabled = true; > + } > } > > -static void v4l2_subdev_disable_privacy_led(struct v4l2_subdev *sd) > +static void v4l2_subdev_disable_privacy_led_and_vcm(struct v4l2_subdev *sd) > { > + if (sd->vcm && sd->vcm_enabled) { > + v4l2_subdev_call(sd->vcm, video, s_stream, 0); > + sd->vcm_enabled = false; > + } > + > #if IS_REACHABLE(CONFIG_LEDS_CLASS) > if (!IS_ERR_OR_NULL(sd->privacy_led)) > led_set_brightness(sd->privacy_led, 0); > @@ -466,9 +482,9 @@ static int call_s_stream(struct v4l2_subdev *sd, int enable) > sd->s_stream_enabled = enable; > > if (enable) > - v4l2_subdev_enable_privacy_led(sd); > + v4l2_subdev_enable_privacy_led_and_vcm(sd); > else > - v4l2_subdev_disable_privacy_led(sd); > + v4l2_subdev_disable_privacy_led_and_vcm(sd); > } > > return ret; > @@ -2289,7 +2305,7 @@ int v4l2_subdev_enable_streams(struct v4l2_subdev *sd, u32 pad, > * for all cases. > */ > if (!use_s_stream && !already_streaming) > - v4l2_subdev_enable_privacy_led(sd); > + v4l2_subdev_enable_privacy_led_and_vcm(sd); > > done: > if (!use_s_stream) > @@ -2382,7 +2398,7 @@ int v4l2_subdev_disable_streams(struct v4l2_subdev *sd, u32 pad, > done: > if (!use_s_stream) { > if (!v4l2_subdev_is_streaming(sd)) > - v4l2_subdev_disable_privacy_led(sd); > + v4l2_subdev_disable_privacy_led_and_vcm(sd); > > v4l2_subdev_unlock_state(state); > } > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h > index bd235d325ff9..6568a0cc070b 100644 > --- a/include/media/v4l2-subdev.h > +++ b/include/media/v4l2-subdev.h > @@ -1094,6 +1094,7 @@ struct v4l2_subdev { > */ > > struct led_classdev *privacy_led; > + struct v4l2_subdev *vcm; > > /* > * TODO: active_state should most likely be changed from a pointer to an > @@ -1104,6 +1105,7 @@ struct v4l2_subdev { > struct v4l2_subdev_state *active_state; > u64 enabled_pads; > bool s_stream_enabled; > + bool vcm_enabled; > }; > >
p.s. On 23-Oct-24 2:48 PM, Hans de Goede wrote: > Hi Laurent, > > On 1-Sep-24 11:28 PM, Laurent Pinchart wrote: >> Hi Hans, >> >> Thank you for the patch. >> >> On Sun, Sep 01, 2024 at 11:18:33PM +0200, Hans de Goede wrote: >>> Currently VCM drivers power-up the VCM as soon as the VCM's /dev node >>> is opened and through the runtime-pm device-link to the sensor this >>> also powers up the sensor. >>> >>> Powering up the VCM and sensor when the /dev node is opened is undesirable, >>> without a VCM sensors delay powering up until s_stream(1) is called. This >>> allows querying / negotiating capabilities without powering things up. >>> >>> Sometimes a long running daemon like pipewire may keep the /dev node open >>> all the time. The kernel should still be able to powerdown the VCM + sensor >>> in this scenario. >>> >>> VCM drivers should be able to do the same as sensor drivers and only >>> power-up the VCM when s_stream(1) is called on the VCM subdev, but this >>> requires that s_stream() is actually called on the VCM when the sensor >>> starts / stops streaming. >> >> .s_stream() doesn't conceptually make sense for VCMs. Furthermore, >> .s_stream() is being replaced with .enable_streams() and >> .disable_streams(), which will make even less sense. We need a different >> API. > > We can discuss how to call the subdev ops for this once we know > what the overal design for this is going to be. So lets postpone > the discussion about naming the subdev ops for this till later. > >>> The s_stream() call on sensor subdevs is done by CSI-receiver/ISP drivers. >>> To avoid needing to change all these call sites to also call s_stream() >>> on the VCM (if there is one) handle the VCM in the v4l2-core similar to how >>> the core takes care of turning on/off the privacy LED. >> >> This needs to come with a design rationale, documented in >> Documentation/. The design needs to explain the use cases. Lens motion >> may take time, which I expect will influence how we will need to handle >> power management. >> >> I'm not very comfortable handling this in v4l2-subdev.c, it seems that >> we'll hardcode use cases. Without a clear and detailed designed >> rationale, this patch feels we'll paint ourselves in a corner. We have >> enough badly designed (or not designed at all) APIs for cameras, it's >> time to do better. > > Design wise I roughly see 3 options: > > a. Stick with the current design of powering on on open of /dev/v4l-subdev > > b. Add explicit calls userspace can make to power-on the device > > c. My current proposal to automatically power-on when the sensor associated > with the VCM. > > Since this patch series implements 3, I'm going to discuss that option > here now. But if that hits a dead end we can look at 2. too. > > Talking about use-cases, lets simplify things by looking at use-cases > solely from a VCM power on/off point of view. Looking at it that way > I really only see 2 orthogonal groupings for all use-cases (so 4 > possible combinations of groupings, but I think we can discuss each > of the 2 axis separately): > > Grouping i: amount of sensor stream start/stops in a single "use" > > 1. The sensor starts streaming once and then the stream stops for > a significant period of time; vs > > 2. The stream is sometimes stopped to change settings and then > restarted within a single use-case. > > For 1. automatically powering on the VCM on stream start is fine. > For 2. we do not want the VCM to turn off and then back on again > while changing settings. We can use auto-suspend with a long enough > auto-suspend delay (we could default to 1s) for this. There might > be some special cases where we need to change the auto-suspend delay. > We could add some V4L2 API (ctrl?) for this, or use the existing > sysfs API through e.g. a udev rule. > > To me using autosuspend-delays here seems workable and having > the v4l2-subdev code manage the power has the advantage that it does > not break any userspace APIs. IOW things will just work while > saving power for existing users and things will also just work > for new use-cases without requiring userspace to have to do > extra work for this. Note that apparently even with the existing power-on on open() there are issues where the VCM powers-down when change modes and people are already submitting patch to work around this, e.g. : https://patchwork.linuxtv.org/project/linux-media/patch/20240831055328.22482-1-zhi.mao@mediatek.com/ > Grouping ii: Does the VCM need to be turned on before the stream > starts: > > 3. A small delay at startup for the power-on (on the first start > stream, not when switching modes) is fine / having the first few > frames being out of focus while the lens moves is fine, they typically > will be anyways until the autofocus algorithm has locked; vs > > 4. A small delay at startup for the power-on is undesirable / > the focus value is known beforehand and the lens must be in position > before capturing the first frame. > > 4. will clearly not work well when the v4l2-subdev code manage > the power. But this seems like a rather corner case scenario; and > one which we could still make work by simply disabling runtime-pm > for the VCM through sysfs in this case. > > In conclusion to me it seems that having the v4l2-subdev code > manage the power seems like it would work well. The only really > problematic scenario which I can come up with is 4. and that seems > a bit of a corner case. If this ever becomes a problem we could > can make it work by either disabling runtime-pm through sysfs or > by offering some API to prime the VCM which turns it on before > the first stream-start (and it will still get automatically turned > off after stream-stop + timeout). > > We could even mix option b) and c) from the "Design wise I roughly > see 3 options" options above and offer explicit power-management > for apps which want that; while still automatically increasing > the power-on count on sensor streaming, with any power-on > count > 1 turning the VCM on. > > This way apps/use-cases which want to make sure the VCM is on > early or stays on during mode switching can use the explicit APIs > while simpler use-cases (and existing use-cases since we cannot > break userspace API) can rely on the automatic powering on done > by the v4l2-subdev core. > > The only downside which I can see of using a power-on counter > and combining explicit + automatic control is that it will be > impossible to turn the VCM off while streaming. > > Regards, > > Hans > > > > > > >> >>> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >>> --- >>> drivers/media/v4l2-core/v4l2-async.c | 20 +++++++++++++++++++ >>> drivers/media/v4l2-core/v4l2-subdev.c | 28 +++++++++++++++++++++------ >>> include/media/v4l2-subdev.h | 2 ++ >>> 3 files changed, 44 insertions(+), 6 deletions(-) >>> >>> diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c >>> index ee884a8221fb..9b854f1d1051 100644 >>> --- a/drivers/media/v4l2-core/v4l2-async.c >>> +++ b/drivers/media/v4l2-core/v4l2-async.c >>> @@ -330,6 +330,11 @@ static int v4l2_async_create_ancillary_links(struct v4l2_async_notifier *n, >>> return 0; >>> } >>> >>> + if (sd->entity.function == MEDIA_ENT_F_LENS) { >>> + dev_dbg(n->sd->dev, "Using %s VCM\n", dev_name(sd->dev)); >>> + n->sd->vcm = sd; >>> + } >>> + >>> link = media_create_ancillary_link(&n->sd->entity, &sd->entity); >>> >>> return IS_ERR(link) ? PTR_ERR(link) : 0; >>> @@ -871,6 +876,21 @@ void v4l2_async_unregister_subdev(struct v4l2_subdev *sd) >>> if (!sd->async_list.next) >>> return; >>> >>> +#if defined(CONFIG_MEDIA_CONTROLLER) >>> + if (sd->entity.function == MEDIA_ENT_F_LENS && sd->v4l2_dev && sd->v4l2_dev->mdev) { >>> + struct media_entity *entity; >>> + >>> + media_device_for_each_entity(entity, sd->v4l2_dev->mdev) { >>> + struct v4l2_subdev *it = media_entity_to_v4l2_subdev(entity); >>> + >>> + if (it->vcm == sd) { >>> + dev_dbg(it->dev, "Clearing VCM\n"); >>> + it->vcm = NULL; >>> + } >>> + } >>> + } >>> +#endif >>> + >>> v4l2_subdev_put_privacy_led(sd); >>> >>> mutex_lock(&list_lock); >>> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c >>> index 7c5812d55315..24a68d90f686 100644 >>> --- a/drivers/media/v4l2-core/v4l2-subdev.c >>> +++ b/drivers/media/v4l2-core/v4l2-subdev.c >>> @@ -148,17 +148,33 @@ static int subdev_close(struct file *file) >>> } >>> #endif /* CONFIG_VIDEO_V4L2_SUBDEV_API */ >>> >>> -static void v4l2_subdev_enable_privacy_led(struct v4l2_subdev *sd) >>> +static void v4l2_subdev_enable_privacy_led_and_vcm(struct v4l2_subdev *sd) >>> { >>> #if IS_REACHABLE(CONFIG_LEDS_CLASS) >>> if (!IS_ERR_OR_NULL(sd->privacy_led)) >>> led_set_brightness(sd->privacy_led, >>> sd->privacy_led->max_brightness); >>> #endif >>> + >>> + if (sd->vcm && !sd->vcm_enabled && >>> + v4l2_subdev_has_op(sd->vcm, video, s_stream)) { >>> + int ret; >>> + >>> + ret = v4l2_subdev_call(sd->vcm, video, s_stream, 1); >>> + if (ret) >>> + dev_err(sd->vcm->dev, "Error powering on VCM: %d\n", ret); >>> + else >>> + sd->vcm_enabled = true; >>> + } >>> } >>> >>> -static void v4l2_subdev_disable_privacy_led(struct v4l2_subdev *sd) >>> +static void v4l2_subdev_disable_privacy_led_and_vcm(struct v4l2_subdev *sd) >>> { >>> + if (sd->vcm && sd->vcm_enabled) { >>> + v4l2_subdev_call(sd->vcm, video, s_stream, 0); >>> + sd->vcm_enabled = false; >>> + } >>> + >>> #if IS_REACHABLE(CONFIG_LEDS_CLASS) >>> if (!IS_ERR_OR_NULL(sd->privacy_led)) >>> led_set_brightness(sd->privacy_led, 0); >>> @@ -466,9 +482,9 @@ static int call_s_stream(struct v4l2_subdev *sd, int enable) >>> sd->s_stream_enabled = enable; >>> >>> if (enable) >>> - v4l2_subdev_enable_privacy_led(sd); >>> + v4l2_subdev_enable_privacy_led_and_vcm(sd); >>> else >>> - v4l2_subdev_disable_privacy_led(sd); >>> + v4l2_subdev_disable_privacy_led_and_vcm(sd); >>> } >>> >>> return ret; >>> @@ -2289,7 +2305,7 @@ int v4l2_subdev_enable_streams(struct v4l2_subdev *sd, u32 pad, >>> * for all cases. >>> */ >>> if (!use_s_stream && !already_streaming) >>> - v4l2_subdev_enable_privacy_led(sd); >>> + v4l2_subdev_enable_privacy_led_and_vcm(sd); >>> >>> done: >>> if (!use_s_stream) >>> @@ -2382,7 +2398,7 @@ int v4l2_subdev_disable_streams(struct v4l2_subdev *sd, u32 pad, >>> done: >>> if (!use_s_stream) { >>> if (!v4l2_subdev_is_streaming(sd)) >>> - v4l2_subdev_disable_privacy_led(sd); >>> + v4l2_subdev_disable_privacy_led_and_vcm(sd); >>> >>> v4l2_subdev_unlock_state(state); >>> } >>> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h >>> index bd235d325ff9..6568a0cc070b 100644 >>> --- a/include/media/v4l2-subdev.h >>> +++ b/include/media/v4l2-subdev.h >>> @@ -1094,6 +1094,7 @@ struct v4l2_subdev { >>> */ >>> >>> struct led_classdev *privacy_led; >>> + struct v4l2_subdev *vcm; >>> >>> /* >>> * TODO: active_state should most likely be changed from a pointer to an >>> @@ -1104,6 +1105,7 @@ struct v4l2_subdev { >>> struct v4l2_subdev_state *active_state; >>> u64 enabled_pads; >>> bool s_stream_enabled; >>> + bool vcm_enabled; >>> }; >>> >>> >> >
diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c index ee884a8221fb..9b854f1d1051 100644 --- a/drivers/media/v4l2-core/v4l2-async.c +++ b/drivers/media/v4l2-core/v4l2-async.c @@ -330,6 +330,11 @@ static int v4l2_async_create_ancillary_links(struct v4l2_async_notifier *n, return 0; } + if (sd->entity.function == MEDIA_ENT_F_LENS) { + dev_dbg(n->sd->dev, "Using %s VCM\n", dev_name(sd->dev)); + n->sd->vcm = sd; + } + link = media_create_ancillary_link(&n->sd->entity, &sd->entity); return IS_ERR(link) ? PTR_ERR(link) : 0; @@ -871,6 +876,21 @@ void v4l2_async_unregister_subdev(struct v4l2_subdev *sd) if (!sd->async_list.next) return; +#if defined(CONFIG_MEDIA_CONTROLLER) + if (sd->entity.function == MEDIA_ENT_F_LENS && sd->v4l2_dev && sd->v4l2_dev->mdev) { + struct media_entity *entity; + + media_device_for_each_entity(entity, sd->v4l2_dev->mdev) { + struct v4l2_subdev *it = media_entity_to_v4l2_subdev(entity); + + if (it->vcm == sd) { + dev_dbg(it->dev, "Clearing VCM\n"); + it->vcm = NULL; + } + } + } +#endif + v4l2_subdev_put_privacy_led(sd); mutex_lock(&list_lock); diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c index 7c5812d55315..24a68d90f686 100644 --- a/drivers/media/v4l2-core/v4l2-subdev.c +++ b/drivers/media/v4l2-core/v4l2-subdev.c @@ -148,17 +148,33 @@ static int subdev_close(struct file *file) } #endif /* CONFIG_VIDEO_V4L2_SUBDEV_API */ -static void v4l2_subdev_enable_privacy_led(struct v4l2_subdev *sd) +static void v4l2_subdev_enable_privacy_led_and_vcm(struct v4l2_subdev *sd) { #if IS_REACHABLE(CONFIG_LEDS_CLASS) if (!IS_ERR_OR_NULL(sd->privacy_led)) led_set_brightness(sd->privacy_led, sd->privacy_led->max_brightness); #endif + + if (sd->vcm && !sd->vcm_enabled && + v4l2_subdev_has_op(sd->vcm, video, s_stream)) { + int ret; + + ret = v4l2_subdev_call(sd->vcm, video, s_stream, 1); + if (ret) + dev_err(sd->vcm->dev, "Error powering on VCM: %d\n", ret); + else + sd->vcm_enabled = true; + } } -static void v4l2_subdev_disable_privacy_led(struct v4l2_subdev *sd) +static void v4l2_subdev_disable_privacy_led_and_vcm(struct v4l2_subdev *sd) { + if (sd->vcm && sd->vcm_enabled) { + v4l2_subdev_call(sd->vcm, video, s_stream, 0); + sd->vcm_enabled = false; + } + #if IS_REACHABLE(CONFIG_LEDS_CLASS) if (!IS_ERR_OR_NULL(sd->privacy_led)) led_set_brightness(sd->privacy_led, 0); @@ -466,9 +482,9 @@ static int call_s_stream(struct v4l2_subdev *sd, int enable) sd->s_stream_enabled = enable; if (enable) - v4l2_subdev_enable_privacy_led(sd); + v4l2_subdev_enable_privacy_led_and_vcm(sd); else - v4l2_subdev_disable_privacy_led(sd); + v4l2_subdev_disable_privacy_led_and_vcm(sd); } return ret; @@ -2289,7 +2305,7 @@ int v4l2_subdev_enable_streams(struct v4l2_subdev *sd, u32 pad, * for all cases. */ if (!use_s_stream && !already_streaming) - v4l2_subdev_enable_privacy_led(sd); + v4l2_subdev_enable_privacy_led_and_vcm(sd); done: if (!use_s_stream) @@ -2382,7 +2398,7 @@ int v4l2_subdev_disable_streams(struct v4l2_subdev *sd, u32 pad, done: if (!use_s_stream) { if (!v4l2_subdev_is_streaming(sd)) - v4l2_subdev_disable_privacy_led(sd); + v4l2_subdev_disable_privacy_led_and_vcm(sd); v4l2_subdev_unlock_state(state); } diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h index bd235d325ff9..6568a0cc070b 100644 --- a/include/media/v4l2-subdev.h +++ b/include/media/v4l2-subdev.h @@ -1094,6 +1094,7 @@ struct v4l2_subdev { */ struct led_classdev *privacy_led; + struct v4l2_subdev *vcm; /* * TODO: active_state should most likely be changed from a pointer to an @@ -1104,6 +1105,7 @@ struct v4l2_subdev { struct v4l2_subdev_state *active_state; u64 enabled_pads; bool s_stream_enabled; + bool vcm_enabled; };
Currently VCM drivers power-up the VCM as soon as the VCM's /dev node is opened and through the runtime-pm device-link to the sensor this also powers up the sensor. Powering up the VCM and sensor when the /dev node is opened is undesirable, without a VCM sensors delay powering up until s_stream(1) is called. This allows querying / negotiating capabilities without powering things up. Sometimes a long running daemon like pipewire may keep the /dev node open all the time. The kernel should still be able to powerdown the VCM + sensor in this scenario. VCM drivers should be able to do the same as sensor drivers and only power-up the VCM when s_stream(1) is called on the VCM subdev, but this requires that s_stream() is actually called on the VCM when the sensor starts / stops streaming. The s_stream() call on sensor subdevs is done by CSI-receiver/ISP drivers. To avoid needing to change all these call sites to also call s_stream() on the VCM (if there is one) handle the VCM in the v4l2-core similar to how the core takes care of turning on/off the privacy LED. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/media/v4l2-core/v4l2-async.c | 20 +++++++++++++++++++ drivers/media/v4l2-core/v4l2-subdev.c | 28 +++++++++++++++++++++------ include/media/v4l2-subdev.h | 2 ++ 3 files changed, 44 insertions(+), 6 deletions(-)