privacy_gpio in uvc were added as V4L2_CID_PRIVACY in uvc video node in
https://lore.kernel.org/all/[email protected]/
Userspace applications often require to constantly poll privacy control.
Currently, polling privacy control requires keeping the video node open,
which prevents the camera from autosuspending.
This patchset adds a separate v4l2 subdevice. Userspace access the gpio
via V4L2_CID_PRIVACY in the new subdevice. Applications can poll the
privacy control status without opening the video node and activate the
camera.
The non-gpio V4L2_CID_PRIVACY in uvc is not affected.
Suggested-by: Ricardo Ribalda <[email protected]>
Signed-off-by: Yunke Cao <[email protected]>
---
Yunke Cao (3):
media: v4l2-ctrls: Expose v4l2_ctrl_fill_event()
media: uvcvideo: remove entity privacy control in the uvc video node
media: uvcvideo: reimplement privacy GPIO as a separate subdevice
drivers/media/usb/uvc/uvc_ctrl.c | 17 -------
drivers/media/usb/uvc/uvc_driver.c | 44 ++----------------
drivers/media/usb/uvc/uvc_entity.c | 76 +++++++++++++++++++++++++++++++
drivers/media/usb/uvc/uvcvideo.h | 19 +++++---
drivers/media/v4l2-core/v4l2-ctrls-core.c | 9 ++--
include/media/v4l2-ctrls.h | 12 +++++
6 files changed, 111 insertions(+), 66 deletions(-)
---
base-commit: 7dd4b804e08041ff56c88bdd8da742d14b17ed25
change-id: 20230111-uvc_privacy_subdev-1e7a167e86eb
Best regards,
--
Yunke Cao <[email protected]>
Rename fill_event() to v4l2_ctrl_fill_event() and expose it to drivers.
Signed-off-by: Yunke Cao <[email protected]>
---
drivers/media/v4l2-core/v4l2-ctrls-core.c | 9 +++++----
include/media/v4l2-ctrls.h | 12 ++++++++++++
2 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-ctrls-core.c b/drivers/media/v4l2-core/v4l2-ctrls-core.c
index 29169170880a..184e03d032d9 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls-core.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls-core.c
@@ -16,8 +16,8 @@
static const union v4l2_ctrl_ptr ptr_null;
-static void fill_event(struct v4l2_event *ev, struct v4l2_ctrl *ctrl,
- u32 changes)
+void v4l2_ctrl_fill_event(struct v4l2_event *ev, struct v4l2_ctrl *ctrl,
+ u32 changes)
{
memset(ev, 0, sizeof(*ev));
ev->type = V4L2_EVENT_CTRL;
@@ -38,6 +38,7 @@ static void fill_event(struct v4l2_event *ev, struct v4l2_ctrl *ctrl,
ev->u.ctrl.step = ctrl->step;
ev->u.ctrl.default_value = ctrl->default_value;
}
+EXPORT_SYMBOL(v4l2_ctrl_fill_event);
void send_initial_event(struct v4l2_fh *fh, struct v4l2_ctrl *ctrl)
{
@@ -46,7 +47,7 @@ void send_initial_event(struct v4l2_fh *fh, struct v4l2_ctrl *ctrl)
if (!(ctrl->flags & V4L2_CTRL_FLAG_WRITE_ONLY))
changes |= V4L2_EVENT_CTRL_CH_VALUE;
- fill_event(&ev, ctrl, changes);
+ v4l2_ctrl_fill_event(&ev, ctrl, changes);
v4l2_event_queue_fh(fh, &ev);
}
@@ -57,7 +58,7 @@ void send_event(struct v4l2_fh *fh, struct v4l2_ctrl *ctrl, u32 changes)
if (list_empty(&ctrl->ev_subs))
return;
- fill_event(&ev, ctrl, changes);
+ v4l2_ctrl_fill_event(&ev, ctrl, changes);
list_for_each_entry(sev, &ctrl->ev_subs, node)
if (sev->fh != fh ||
diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
index e59d9a234631..52b2f366cdb6 100644
--- a/include/media/v4l2-ctrls.h
+++ b/include/media/v4l2-ctrls.h
@@ -847,6 +847,18 @@ void v4l2_ctrl_auto_cluster(unsigned int ncontrols,
*/
struct v4l2_ctrl *v4l2_ctrl_find(struct v4l2_ctrl_handler *hdl, u32 id);
+/**
+ * v4l2_ctrl_fill_event() - Fill the v4l2 event for a control.
+ *
+ * @ev: The event to fill.
+ * @ctrl: The struct v4l2_ctrl for the control event.
+ * @changes: A bitmask that tells what has changed.
+ *
+ * This function assumes that the control handler is locked.
+ */
+void v4l2_ctrl_fill_event(struct v4l2_event *ev, struct v4l2_ctrl *ctrl,
+ u32 changes);
+
/**
* v4l2_ctrl_activate() - Make the control active or inactive.
* @ctrl: The control to (de)activate.
--
b4 0.11.0-dev-4d321
For privacy_gpio, do not expose V4L2_CID_PRIVACY to userspace as a control
of the video node.
Signed-off-by: Yunke Cao <[email protected]>
---
drivers/media/usb/uvc/uvc_ctrl.c | 17 -----------------
1 file changed, 17 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index c95a2229f4fa..77c5ff19add8 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -348,14 +348,6 @@ static const struct uvc_control_info uvc_ctrls[] = {
| UVC_CTRL_FLAG_RESTORE
| UVC_CTRL_FLAG_AUTO_UPDATE,
},
- {
- .entity = UVC_GUID_EXT_GPIO_CONTROLLER,
- .selector = UVC_CT_PRIVACY_CONTROL,
- .index = 0,
- .size = 1,
- .flags = UVC_CTRL_FLAG_GET_CUR
- | UVC_CTRL_FLAG_AUTO_UPDATE,
- },
};
static const u32 uvc_control_classes[] = {
@@ -710,15 +702,6 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
.v4l2_type = V4L2_CTRL_TYPE_BOOLEAN,
.data_type = UVC_CTRL_DATA_TYPE_BOOLEAN,
},
- {
- .id = V4L2_CID_PRIVACY,
- .entity = UVC_GUID_EXT_GPIO_CONTROLLER,
- .selector = UVC_CT_PRIVACY_CONTROL,
- .size = 1,
- .offset = 0,
- .v4l2_type = V4L2_CTRL_TYPE_BOOLEAN,
- .data_type = UVC_CTRL_DATA_TYPE_BOOLEAN,
- },
};
static const struct uvc_control_mapping uvc_ctrl_mappings_uvc11[] = {
--
b4 0.11.0-dev-4d321
Hello,
W dniu 11.01.2023 o 09:52, Yunke Cao pisze:
> For privacy_gpio, do not expose V4L2_CID_PRIVACY to userspace as a control
> of the video node.
>
I know it is an RFC, so maybe you distribute the changes into 3 patches
on purpose. But, after applying this patch V4L2_CID_PRIVACY is lost
until it is re-implemented later, isn't it? Because of that It seems to me
patches 2/3 and 3/3 should be combined into one.
Regards,
Andrzej
> Signed-off-by: Yunke Cao <[email protected]>
> ---
> drivers/media/usb/uvc/uvc_ctrl.c | 17 -----------------
> 1 file changed, 17 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index c95a2229f4fa..77c5ff19add8 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -348,14 +348,6 @@ static const struct uvc_control_info uvc_ctrls[] = {
> | UVC_CTRL_FLAG_RESTORE
> | UVC_CTRL_FLAG_AUTO_UPDATE,
> },
> - {
> - .entity = UVC_GUID_EXT_GPIO_CONTROLLER,
> - .selector = UVC_CT_PRIVACY_CONTROL,
> - .index = 0,
> - .size = 1,
> - .flags = UVC_CTRL_FLAG_GET_CUR
> - | UVC_CTRL_FLAG_AUTO_UPDATE,
> - },
> };
>
> static const u32 uvc_control_classes[] = {
> @@ -710,15 +702,6 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> .v4l2_type = V4L2_CTRL_TYPE_BOOLEAN,
> .data_type = UVC_CTRL_DATA_TYPE_BOOLEAN,
> },
> - {
> - .id = V4L2_CID_PRIVACY,
> - .entity = UVC_GUID_EXT_GPIO_CONTROLLER,
> - .selector = UVC_CT_PRIVACY_CONTROL,
> - .size = 1,
> - .offset = 0,
> - .v4l2_type = V4L2_CTRL_TYPE_BOOLEAN,
> - .data_type = UVC_CTRL_DATA_TYPE_BOOLEAN,
> - },
> };
>
> static const struct uvc_control_mapping uvc_ctrl_mappings_uvc11[] = {
>
Hi Andrej,
On Wed, Jan 11, 2023 at 7:33 PM Andrzej Pietrasiewicz
<[email protected]> wrote:
>
> Hello,
>
> W dniu 11.01.2023 o 09:52, Yunke Cao pisze:
> > For privacy_gpio, do not expose V4L2_CID_PRIVACY to userspace as a control
> > of the video node.
> >
>
> I know it is an RFC, so maybe you distribute the changes into 3 patches
> on purpose. But, after applying this patch V4L2_CID_PRIVACY is lost
> until it is re-implemented later, isn't it? Because of that It seems to me
> patches 2/3 and 3/3 should be combined into one.
>
Yes, that's correct. Now that I look at it, it makes little sense to split them.
Thanks!
Yunke
> Regards,
>
> Andrzej
>
> > Signed-off-by: Yunke Cao <[email protected]>
> > ---
> > drivers/media/usb/uvc/uvc_ctrl.c | 17 -----------------
> > 1 file changed, 17 deletions(-)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> > index c95a2229f4fa..77c5ff19add8 100644
> > --- a/drivers/media/usb/uvc/uvc_ctrl.c
> > +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> > @@ -348,14 +348,6 @@ static const struct uvc_control_info uvc_ctrls[] = {
> > | UVC_CTRL_FLAG_RESTORE
> > | UVC_CTRL_FLAG_AUTO_UPDATE,
> > },
> > - {
> > - .entity = UVC_GUID_EXT_GPIO_CONTROLLER,
> > - .selector = UVC_CT_PRIVACY_CONTROL,
> > - .index = 0,
> > - .size = 1,
> > - .flags = UVC_CTRL_FLAG_GET_CUR
> > - | UVC_CTRL_FLAG_AUTO_UPDATE,
> > - },
> > };
> >
> > static const u32 uvc_control_classes[] = {
> > @@ -710,15 +702,6 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> > .v4l2_type = V4L2_CTRL_TYPE_BOOLEAN,
> > .data_type = UVC_CTRL_DATA_TYPE_BOOLEAN,
> > },
> > - {
> > - .id = V4L2_CID_PRIVACY,
> > - .entity = UVC_GUID_EXT_GPIO_CONTROLLER,
> > - .selector = UVC_CT_PRIVACY_CONTROL,
> > - .size = 1,
> > - .offset = 0,
> > - .v4l2_type = V4L2_CTRL_TYPE_BOOLEAN,
> > - .data_type = UVC_CTRL_DATA_TYPE_BOOLEAN,
> > - },
> > };
> >
> > static const struct uvc_control_mapping uvc_ctrl_mappings_uvc11[] = {
> >
>
Hi Yunke
Thank you very much for the patchset :)
On Wed, 11 Jan 2023 at 09:52, Yunke Cao <[email protected]> wrote:
>
> privacy_gpio in uvc were added as V4L2_CID_PRIVACY in uvc video node in
> https://lore.kernel.org/all/[email protected]/
>
> Userspace applications often require to constantly poll privacy control.
> Currently, polling privacy control requires keeping the video node open,
> which prevents the camera from autosuspending.
>
> This patchset adds a separate v4l2 subdevice. Userspace access the gpio
> via V4L2_CID_PRIVACY in the new subdevice. Applications can poll the
> privacy control status without opening the video node and activate the
> camera.
>
> The non-gpio V4L2_CID_PRIVACY in uvc is not affected.
Since this is a RFC, lets focus on the idea and not on the code itself.
- I am missing a reference to the subdevice from the media device. How
will a user figure out that /dev/v4l-subdev0 is the privacy gpio of
/dev/media0 and not /dev/media1?. Thake a look to the "ancillary
links"
- We have already exposed the control as part of the main video
device, that means that we need to keep that API. The control on
/dev/v4l-subdev0 should "mirror" the control on /dev/video0
- There is no need to v4l2_ctrl_fill_event(), if you modify the
control with a set controll function, the media controller should take
care of everything
@Sakari Ailus @Hans Verkuil : Assuming a correct implementation, how
would you feel about exposing a privacy gpio as a subdevice?
Thanks!!!
>
> Suggested-by: Ricardo Ribalda <[email protected]>
> Signed-off-by: Yunke Cao <[email protected]>
> ---
> Yunke Cao (3):
> media: v4l2-ctrls: Expose v4l2_ctrl_fill_event()
> media: uvcvideo: remove entity privacy control in the uvc video node
> media: uvcvideo: reimplement privacy GPIO as a separate subdevice
>
> drivers/media/usb/uvc/uvc_ctrl.c | 17 -------
> drivers/media/usb/uvc/uvc_driver.c | 44 ++----------------
> drivers/media/usb/uvc/uvc_entity.c | 76 +++++++++++++++++++++++++++++++
> drivers/media/usb/uvc/uvcvideo.h | 19 +++++---
> drivers/media/v4l2-core/v4l2-ctrls-core.c | 9 ++--
> include/media/v4l2-ctrls.h | 12 +++++
> 6 files changed, 111 insertions(+), 66 deletions(-)
> ---
> base-commit: 7dd4b804e08041ff56c88bdd8da742d14b17ed25
> change-id: 20230111-uvc_privacy_subdev-1e7a167e86eb
>
> Best regards,
> --
> Yunke Cao <[email protected]>
--
Ricardo Ribalda
Hi!
On Fri, Jan 13, 2023 at 5:26 PM Ricardo Ribalda <[email protected]> wrote:
>
> Hi Yunke
>
> Thank you very much for the patchset :)
>
> On Wed, 11 Jan 2023 at 09:52, Yunke Cao <[email protected]> wrote:
> >
> > privacy_gpio in uvc were added as V4L2_CID_PRIVACY in uvc video node in
> > https://lore.kernel.org/all/[email protected]/
> >
> > Userspace applications often require to constantly poll privacy control.
> > Currently, polling privacy control requires keeping the video node open,
> > which prevents the camera from autosuspending.
> >
> > This patchset adds a separate v4l2 subdevice. Userspace access the gpio
> > via V4L2_CID_PRIVACY in the new subdevice. Applications can poll the
> > privacy control status without opening the video node and activate the
> > camera.
> >
> > The non-gpio V4L2_CID_PRIVACY in uvc is not affected.
>
> Since this is a RFC, lets focus on the idea and not on the code itself.
>
> - I am missing a reference to the subdevice from the media device. How
> will a user figure out that /dev/v4l-subdev0 is the privacy gpio of
> /dev/media0 and not /dev/media1?. Thake a look to the "ancillary
> links"
> - We have already exposed the control as part of the main video
> device, that means that we need to keep that API. The control on
> /dev/v4l-subdev0 should "mirror" the control on /dev/video0
> - There is no need to v4l2_ctrl_fill_event(), if you modify the
> control with a set controll function, the media controller should take
> care of everything
Thanks! I will fix these in the next version if we decide to proceed.
>
> @Sakari Ailus @Hans Verkuil : Assuming a correct implementation, how
> would you feel about exposing a privacy gpio as a subdevice?
>
Sakari, Hans, do you think this idea makes sense?
Best,
Yunke
>
> Thanks!!!
>
>
> >
> > Suggested-by: Ricardo Ribalda <[email protected]>
> > Signed-off-by: Yunke Cao <[email protected]>
> > ---
> > Yunke Cao (3):
> > media: v4l2-ctrls: Expose v4l2_ctrl_fill_event()
> > media: uvcvideo: remove entity privacy control in the uvc video node
> > media: uvcvideo: reimplement privacy GPIO as a separate subdevice
> >
> > drivers/media/usb/uvc/uvc_ctrl.c | 17 -------
> > drivers/media/usb/uvc/uvc_driver.c | 44 ++----------------
> > drivers/media/usb/uvc/uvc_entity.c | 76 +++++++++++++++++++++++++++++++
> > drivers/media/usb/uvc/uvcvideo.h | 19 +++++---
> > drivers/media/v4l2-core/v4l2-ctrls-core.c | 9 ++--
> > include/media/v4l2-ctrls.h | 12 +++++
> > 6 files changed, 111 insertions(+), 66 deletions(-)
> > ---
> > base-commit: 7dd4b804e08041ff56c88bdd8da742d14b17ed25
> > change-id: 20230111-uvc_privacy_subdev-1e7a167e86eb
> >
> > Best regards,
> > --
> > Yunke Cao <[email protected]>
>
>
>
> --
> Ricardo Ribalda
On Tue, 14 Feb 2023 at 06:46, Yunke Cao <[email protected]> wrote:
>
> Hi!
>
> On Fri, Jan 13, 2023 at 5:26 PM Ricardo Ribalda <[email protected]> wrote:
> >
> > Hi Yunke
> >
> > Thank you very much for the patchset :)
> >
> > On Wed, 11 Jan 2023 at 09:52, Yunke Cao <[email protected]> wrote:
> > >
> > > privacy_gpio in uvc were added as V4L2_CID_PRIVACY in uvc video node in
> > > https://lore.kernel.org/all/[email protected]/
> > >
> > > Userspace applications often require to constantly poll privacy control.
> > > Currently, polling privacy control requires keeping the video node open,
> > > which prevents the camera from autosuspending.
> > >
> > > This patchset adds a separate v4l2 subdevice. Userspace access the gpio
> > > via V4L2_CID_PRIVACY in the new subdevice. Applications can poll the
> > > privacy control status without opening the video node and activate the
> > > camera.
> > >
> > > The non-gpio V4L2_CID_PRIVACY in uvc is not affected.
> >
> > Since this is a RFC, lets focus on the idea and not on the code itself.
> >
> > - I am missing a reference to the subdevice from the media device. How
> > will a user figure out that /dev/v4l-subdev0 is the privacy gpio of
> > /dev/media0 and not /dev/media1?. Thake a look to the "ancillary
> > links"
> > - We have already exposed the control as part of the main video
> > device, that means that we need to keep that API. The control on
> > /dev/v4l-subdev0 should "mirror" the control on /dev/video0
> > - There is no need to v4l2_ctrl_fill_event(), if you modify the
> > control with a set controll function, the media controller should take
> > care of everything
>
> Thanks! I will fix these in the next version if we decide to proceed.
>
> >
> > @Sakari Ailus @Hans Verkuil : Assuming a correct implementation, how
> > would you feel about exposing a privacy gpio as a subdevice?
> >
>
> Sakari, Hans, do you think this idea makes sense?
Friendly ping
>
> Best,
> Yunke
>
> >
> > Thanks!!!
> >
> >
> > >
> > > Suggested-by: Ricardo Ribalda <[email protected]>
> > > Signed-off-by: Yunke Cao <[email protected]>
> > > ---
> > > Yunke Cao (3):
> > > media: v4l2-ctrls: Expose v4l2_ctrl_fill_event()
> > > media: uvcvideo: remove entity privacy control in the uvc video node
> > > media: uvcvideo: reimplement privacy GPIO as a separate subdevice
> > >
> > > drivers/media/usb/uvc/uvc_ctrl.c | 17 -------
> > > drivers/media/usb/uvc/uvc_driver.c | 44 ++----------------
> > > drivers/media/usb/uvc/uvc_entity.c | 76 +++++++++++++++++++++++++++++++
> > > drivers/media/usb/uvc/uvcvideo.h | 19 +++++---
> > > drivers/media/v4l2-core/v4l2-ctrls-core.c | 9 ++--
> > > include/media/v4l2-ctrls.h | 12 +++++
> > > 6 files changed, 111 insertions(+), 66 deletions(-)
> > > ---
> > > base-commit: 7dd4b804e08041ff56c88bdd8da742d14b17ed25
> > > change-id: 20230111-uvc_privacy_subdev-1e7a167e86eb
> > >
> > > Best regards,
> > > --
> > > Yunke Cao <[email protected]>
> >
> >
> >
> > --
> > Ricardo Ribalda
--
Ricardo Ribalda
Hi Sakari
Could you take a look at this RFC? Would be great to have your opinion
from a subdevice point of view.
Thanks!
On Wed, 11 Jan 2023 at 09:52, Yunke Cao <[email protected]> wrote:
>
> privacy_gpio in uvc were added as V4L2_CID_PRIVACY in uvc video node in
> https://lore.kernel.org/all/[email protected]/
>
> Userspace applications often require to constantly poll privacy control.
> Currently, polling privacy control requires keeping the video node open,
> which prevents the camera from autosuspending.
>
> This patchset adds a separate v4l2 subdevice. Userspace access the gpio
> via V4L2_CID_PRIVACY in the new subdevice. Applications can poll the
> privacy control status without opening the video node and activate the
> camera.
>
> The non-gpio V4L2_CID_PRIVACY in uvc is not affected.
>
> Suggested-by: Ricardo Ribalda <[email protected]>
> Signed-off-by: Yunke Cao <[email protected]>
> ---
> Yunke Cao (3):
> media: v4l2-ctrls: Expose v4l2_ctrl_fill_event()
> media: uvcvideo: remove entity privacy control in the uvc video node
> media: uvcvideo: reimplement privacy GPIO as a separate subdevice
>
> drivers/media/usb/uvc/uvc_ctrl.c | 17 -------
> drivers/media/usb/uvc/uvc_driver.c | 44 ++----------------
> drivers/media/usb/uvc/uvc_entity.c | 76 +++++++++++++++++++++++++++++++
> drivers/media/usb/uvc/uvcvideo.h | 19 +++++---
> drivers/media/v4l2-core/v4l2-ctrls-core.c | 9 ++--
> include/media/v4l2-ctrls.h | 12 +++++
> 6 files changed, 111 insertions(+), 66 deletions(-)
> ---
> base-commit: 7dd4b804e08041ff56c88bdd8da742d14b17ed25
> change-id: 20230111-uvc_privacy_subdev-1e7a167e86eb
>
> Best regards,
> --
> Yunke Cao <[email protected]>
--
Ricardo Ribalda