* [PATCH RFC 0/3] meida: uvcvideo: reimplement privacy gpio as a separate subdevice
@ 2023-01-11 8:52 Yunke Cao
2023-01-11 8:52 ` [PATCH RFC 1/3] media: v4l2-ctrls: Expose v4l2_ctrl_fill_event() Yunke Cao
` (4 more replies)
0 siblings, 5 replies; 11+ messages in thread
From: Yunke Cao @ 2023-01-11 8:52 UTC (permalink / raw)
To: Laurent Pinchart, Mauro Carvalho Chehab, Ricardo Ribalda, Hans Verkuil
Cc: Yunke Cao, linux-kernel, linux-media, Sakari Ailus, Ricardo Ribalda
privacy_gpio in uvc were added as V4L2_CID_PRIVACY in uvc video node in
https://lore.kernel.org/all/20201223133528.55014-1-ribalda@chromium.org/
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 <ribalda@chromium.org>
Signed-off-by: Yunke Cao <yunkec@chromium.org>
---
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 <yunkec@chromium.org>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH RFC 1/3] media: v4l2-ctrls: Expose v4l2_ctrl_fill_event()
2023-01-11 8:52 [PATCH RFC 0/3] meida: uvcvideo: reimplement privacy gpio as a separate subdevice Yunke Cao
@ 2023-01-11 8:52 ` Yunke Cao
2023-01-11 8:52 ` [PATCH RFC 2/3] media: uvcvideo: remove entity privacy control in the uvc video node Yunke Cao
` (3 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Yunke Cao @ 2023-01-11 8:52 UTC (permalink / raw)
To: Laurent Pinchart, Mauro Carvalho Chehab, Ricardo Ribalda, Hans Verkuil
Cc: Yunke Cao, linux-kernel, linux-media, Sakari Ailus, Ricardo Ribalda
Rename fill_event() to v4l2_ctrl_fill_event() and expose it to drivers.
Signed-off-by: Yunke Cao <yunkec@chromium.org>
---
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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH RFC 2/3] media: uvcvideo: remove entity privacy control in the uvc video node
2023-01-11 8:52 [PATCH RFC 0/3] meida: uvcvideo: reimplement privacy gpio as a separate subdevice Yunke Cao
2023-01-11 8:52 ` [PATCH RFC 1/3] media: v4l2-ctrls: Expose v4l2_ctrl_fill_event() Yunke Cao
@ 2023-01-11 8:52 ` Yunke Cao
2023-01-11 10:33 ` Andrzej Pietrasiewicz
2023-01-11 8:52 ` [PATCH RFC 3/3] media: uvcvideo: reimplement privacy GPIO as a separate subdevice Yunke Cao
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Yunke Cao @ 2023-01-11 8:52 UTC (permalink / raw)
To: Laurent Pinchart, Mauro Carvalho Chehab, Ricardo Ribalda, Hans Verkuil
Cc: Yunke Cao, linux-kernel, linux-media, Sakari Ailus, Ricardo Ribalda
For privacy_gpio, do not expose V4L2_CID_PRIVACY to userspace as a control
of the video node.
Signed-off-by: Yunke Cao <yunkec@chromium.org>
---
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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH RFC 3/3] media: uvcvideo: reimplement privacy GPIO as a separate subdevice
2023-01-11 8:52 [PATCH RFC 0/3] meida: uvcvideo: reimplement privacy gpio as a separate subdevice Yunke Cao
2023-01-11 8:52 ` [PATCH RFC 1/3] media: v4l2-ctrls: Expose v4l2_ctrl_fill_event() Yunke Cao
2023-01-11 8:52 ` [PATCH RFC 2/3] media: uvcvideo: remove entity privacy control in the uvc video node Yunke Cao
@ 2023-01-11 8:52 ` Yunke Cao
2023-11-21 20:46 ` Hans Verkuil
2023-01-13 8:25 ` [PATCH RFC 0/3] meida: uvcvideo: reimplement privacy gpio " Ricardo Ribalda
2023-11-21 20:05 ` Ricardo Ribalda
4 siblings, 1 reply; 11+ messages in thread
From: Yunke Cao @ 2023-01-11 8:52 UTC (permalink / raw)
To: Laurent Pinchart, Mauro Carvalho Chehab, Ricardo Ribalda, Hans Verkuil
Cc: Yunke Cao, linux-kernel, linux-media, Sakari Ailus, Ricardo Ribalda
Reimplement privacy GPIO as a v4l2 subdev with a volatile privacy control.
A v4l2 control event is sent in irq when privacy control value changes.
The behavior matches the original implementation, except that the
control is of a separate subdevice.
V4L2 control kAPI is used for simplicity.
Signed-off-by: Yunke Cao <yunkec@chromium.org>
---
drivers/media/usb/uvc/uvc_driver.c | 44 +++-------------------
drivers/media/usb/uvc/uvc_entity.c | 76 ++++++++++++++++++++++++++++++++++++++
drivers/media/usb/uvc/uvcvideo.h | 19 +++++++---
3 files changed, 94 insertions(+), 45 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index e4bcb5011360..25848f4dbce0 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -1209,43 +1209,6 @@ static int uvc_parse_control(struct uvc_device *dev)
* Privacy GPIO
*/
-static void uvc_gpio_event(struct uvc_device *dev)
-{
- struct uvc_entity *unit = dev->gpio_unit;
- struct uvc_video_chain *chain;
- u8 new_val;
-
- if (!unit)
- return;
-
- new_val = gpiod_get_value_cansleep(unit->gpio.gpio_privacy);
-
- /* GPIO entities are always on the first chain. */
- chain = list_first_entry(&dev->chains, struct uvc_video_chain, list);
- uvc_ctrl_status_event(chain, unit->controls, &new_val);
-}
-
-static int uvc_gpio_get_cur(struct uvc_device *dev, struct uvc_entity *entity,
- u8 cs, void *data, u16 size)
-{
- if (cs != UVC_CT_PRIVACY_CONTROL || size < 1)
- return -EINVAL;
-
- *(u8 *)data = gpiod_get_value_cansleep(entity->gpio.gpio_privacy);
-
- return 0;
-}
-
-static int uvc_gpio_get_info(struct uvc_device *dev, struct uvc_entity *entity,
- u8 cs, u8 *caps)
-{
- if (cs != UVC_CT_PRIVACY_CONTROL)
- return -EINVAL;
-
- *caps = UVC_CONTROL_CAP_GET | UVC_CONTROL_CAP_AUTOUPDATE;
- return 0;
-}
-
static irqreturn_t uvc_gpio_irq(int irq, void *data)
{
struct uvc_device *dev = data;
@@ -1279,8 +1242,6 @@ static int uvc_gpio_parse(struct uvc_device *dev)
unit->gpio.bControlSize = 1;
unit->gpio.bmControls = (u8 *)unit + sizeof(*unit);
unit->gpio.bmControls[0] = 1;
- unit->get_cur = uvc_gpio_get_cur;
- unit->get_info = uvc_gpio_get_info;
strscpy(unit->name, "GPIO", sizeof(unit->name));
list_add_tail(&unit->list, &dev->entities);
@@ -2202,6 +2163,11 @@ static int uvc_probe(struct usb_interface *intf,
if (media_device_register(&dev->mdev) < 0)
goto error;
#endif
+
+ /* Expose all subdev's nodes*/
+ if (v4l2_device_register_subdev_nodes(&dev->vdev) < 0)
+ goto error;
+
/* Save our data pointer in the interface data. */
usb_set_intfdata(intf, dev);
diff --git a/drivers/media/usb/uvc/uvc_entity.c b/drivers/media/usb/uvc/uvc_entity.c
index 7c4d2f93d351..c8e41b42ffd8 100644
--- a/drivers/media/usb/uvc/uvc_entity.c
+++ b/drivers/media/usb/uvc/uvc_entity.c
@@ -56,17 +56,90 @@ static int uvc_mc_create_links(struct uvc_video_chain *chain,
return 0;
}
+static const struct v4l2_subdev_core_ops uvc_subdev_core_ops = {
+ .subscribe_event = v4l2_ctrl_subdev_subscribe_event,
+ .unsubscribe_event = v4l2_event_subdev_unsubscribe,
+};
+
static const struct v4l2_subdev_ops uvc_subdev_ops = {
+ .core = &uvc_subdev_core_ops,
};
void uvc_mc_cleanup_entity(struct uvc_entity *entity)
{
+ if (UVC_ENTITY_TYPE(entity) == UVC_EXT_GPIO_UNIT)
+ v4l2_ctrl_handler_free(&entity->gpio.hdl);
+
if (UVC_ENTITY_TYPE(entity) != UVC_TT_STREAMING)
media_entity_cleanup(&entity->subdev.entity);
else if (entity->vdev != NULL)
media_entity_cleanup(&entity->vdev->entity);
}
+static int uvc_gpio_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
+{
+ struct uvc_gpio *gpio =
+ container_of(ctrl->handler, struct uvc_gpio, hdl);
+
+ ctrl->cur.val = gpiod_get_value_cansleep(gpio->gpio_privacy);
+
+ return 0;
+}
+
+static const struct v4l2_ctrl_ops uvc_gpio_ctrl_ops = {
+ .g_volatile_ctrl = uvc_gpio_g_volatile_ctrl,
+};
+
+void uvc_gpio_event(struct uvc_device *dev)
+{
+ struct uvc_entity *unit = dev->gpio_unit;
+ struct v4l2_ctrl *ctrl;
+ struct v4l2_event ev;
+ s32 old_val;
+ int ret;
+
+ if (!unit)
+ return;
+
+ ctrl = unit->gpio.privacy_ctrl;
+ old_val = ctrl->cur.val;
+
+ v4l2_ctrl_lock(ctrl);
+
+ ret = uvc_gpio_g_volatile_ctrl(ctrl);
+ if (ret < 0 || old_val == ctrl->cur.val) {
+ v4l2_ctrl_unlock(ctrl);
+ return;
+ }
+
+ v4l2_ctrl_fill_event(&ev, ctrl, V4L2_EVENT_CTRL_CH_VALUE);
+ v4l2_ctrl_unlock(ctrl);
+
+ v4l2_event_queue(unit->subdev.devnode, &ev);
+}
+
+static void uvc_gpio_init_ctrl(struct uvc_entity *entity)
+{
+ struct v4l2_ctrl *ctrl;
+ struct v4l2_ctrl_handler *hdl = &entity->gpio.hdl;
+
+ entity->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE |
+ V4L2_SUBDEV_FL_HAS_EVENTS;
+
+ v4l2_ctrl_handler_init(hdl, 1);
+ entity->subdev.ctrl_handler = hdl;
+ ctrl = v4l2_ctrl_new_std(hdl, &uvc_gpio_ctrl_ops, V4L2_CID_PRIVACY,
+ 0, 1, 1, 0);
+ if (ctrl)
+ ctrl->flags |= V4L2_CTRL_FLAG_VOLATILE |
+ V4L2_CTRL_FLAG_READ_ONLY;
+
+ /* Read GPIO value to initialize the control. */
+ uvc_gpio_g_volatile_ctrl(ctrl);
+
+ entity->gpio.privacy_ctrl = ctrl;
+}
+
static int uvc_mc_init_entity(struct uvc_video_chain *chain,
struct uvc_entity *entity)
{
@@ -113,6 +186,9 @@ static int uvc_mc_init_entity(struct uvc_video_chain *chain,
entity->subdev.entity.function = function;
+ if (UVC_ENTITY_TYPE(entity) == UVC_EXT_GPIO_UNIT)
+ uvc_gpio_init_ctrl(entity);
+
ret = media_entity_pads_init(&entity->subdev.entity,
entity->num_pads, entity->pads);
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index df93db259312..19ca2896c398 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -15,6 +15,7 @@
#include <linux/videodev2.h>
#include <linux/workqueue.h>
#include <media/media-device.h>
+#include <media/v4l2-ctrls.h>
#include <media/v4l2-device.h>
#include <media/v4l2-event.h>
#include <media/v4l2-fh.h>
@@ -163,6 +164,15 @@ struct uvc_control {
#define UVC_ENTITY_FLAG_DEFAULT (1 << 0)
+struct uvc_gpio {
+ u8 bControlSize;
+ u8 *bmControls;
+ struct gpio_desc *gpio_privacy;
+ int irq;
+ struct v4l2_ctrl_handler hdl;
+ struct v4l2_ctrl *privacy_ctrl;
+};
+
struct uvc_entity {
struct list_head list; /* Entity as part of a UVC device. */
struct list_head chain; /* Entity as part of a video device chain. */
@@ -221,12 +231,7 @@ struct uvc_entity {
u8 *bmControlsType;
} extension;
- struct {
- u8 bControlSize;
- u8 *bmControls;
- struct gpio_desc *gpio_privacy;
- int irq;
- } gpio;
+ struct uvc_gpio gpio;
};
u8 bNrInPins;
@@ -766,6 +771,8 @@ int uvc_ctrl_is_accessible(struct uvc_video_chain *chain, u32 v4l2_id,
int uvc_xu_ctrl_query(struct uvc_video_chain *chain,
struct uvc_xu_control_query *xqry);
+void uvc_gpio_event(struct uvc_device *dev);
+
/* Utility functions */
struct usb_host_endpoint *uvc_find_endpoint(struct usb_host_interface *alts,
u8 epaddr);
--
b4 0.11.0-dev-4d321
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH RFC 2/3] media: uvcvideo: remove entity privacy control in the uvc video node
2023-01-11 8:52 ` [PATCH RFC 2/3] media: uvcvideo: remove entity privacy control in the uvc video node Yunke Cao
@ 2023-01-11 10:33 ` Andrzej Pietrasiewicz
2023-01-12 1:40 ` Yunke Cao
0 siblings, 1 reply; 11+ messages in thread
From: Andrzej Pietrasiewicz @ 2023-01-11 10:33 UTC (permalink / raw)
To: Yunke Cao, Laurent Pinchart, Mauro Carvalho Chehab,
Ricardo Ribalda, Hans Verkuil
Cc: linux-kernel, linux-media, Sakari Ailus
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 <yunkec@chromium.org>
> ---
> 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[] = {
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH RFC 2/3] media: uvcvideo: remove entity privacy control in the uvc video node
2023-01-11 10:33 ` Andrzej Pietrasiewicz
@ 2023-01-12 1:40 ` Yunke Cao
0 siblings, 0 replies; 11+ messages in thread
From: Yunke Cao @ 2023-01-12 1:40 UTC (permalink / raw)
To: Andrzej Pietrasiewicz
Cc: Laurent Pinchart, Mauro Carvalho Chehab, Ricardo Ribalda,
Hans Verkuil, linux-kernel, linux-media, Sakari Ailus
Hi Andrej,
On Wed, Jan 11, 2023 at 7:33 PM Andrzej Pietrasiewicz
<andrzej.p@collabora.com> 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 <yunkec@chromium.org>
> > ---
> > 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[] = {
> >
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH RFC 0/3] meida: uvcvideo: reimplement privacy gpio as a separate subdevice
2023-01-11 8:52 [PATCH RFC 0/3] meida: uvcvideo: reimplement privacy gpio as a separate subdevice Yunke Cao
` (2 preceding siblings ...)
2023-01-11 8:52 ` [PATCH RFC 3/3] media: uvcvideo: reimplement privacy GPIO as a separate subdevice Yunke Cao
@ 2023-01-13 8:25 ` Ricardo Ribalda
2023-02-14 5:45 ` Yunke Cao
2023-11-21 20:05 ` Ricardo Ribalda
4 siblings, 1 reply; 11+ messages in thread
From: Ricardo Ribalda @ 2023-01-13 8:25 UTC (permalink / raw)
To: Yunke Cao
Cc: Laurent Pinchart, Mauro Carvalho Chehab, Hans Verkuil,
linux-kernel, linux-media, Sakari Ailus
Hi Yunke
Thank you very much for the patchset :)
On Wed, 11 Jan 2023 at 09:52, Yunke Cao <yunkec@chromium.org> wrote:
>
> privacy_gpio in uvc were added as V4L2_CID_PRIVACY in uvc video node in
> https://lore.kernel.org/all/20201223133528.55014-1-ribalda@chromium.org/
>
> 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 <ribalda@chromium.org>
> Signed-off-by: Yunke Cao <yunkec@chromium.org>
> ---
> 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 <yunkec@chromium.org>
--
Ricardo Ribalda
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH RFC 0/3] meida: uvcvideo: reimplement privacy gpio as a separate subdevice
2023-01-13 8:25 ` [PATCH RFC 0/3] meida: uvcvideo: reimplement privacy gpio " Ricardo Ribalda
@ 2023-02-14 5:45 ` Yunke Cao
2023-03-08 22:57 ` Ricardo Ribalda
0 siblings, 1 reply; 11+ messages in thread
From: Yunke Cao @ 2023-02-14 5:45 UTC (permalink / raw)
To: Ricardo Ribalda
Cc: Laurent Pinchart, Mauro Carvalho Chehab, Hans Verkuil,
linux-kernel, linux-media, Sakari Ailus
Hi!
On Fri, Jan 13, 2023 at 5:26 PM Ricardo Ribalda <ribalda@chromium.org> wrote:
>
> Hi Yunke
>
> Thank you very much for the patchset :)
>
> On Wed, 11 Jan 2023 at 09:52, Yunke Cao <yunkec@chromium.org> wrote:
> >
> > privacy_gpio in uvc were added as V4L2_CID_PRIVACY in uvc video node in
> > https://lore.kernel.org/all/20201223133528.55014-1-ribalda@chromium.org/
> >
> > 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 <ribalda@chromium.org>
> > Signed-off-by: Yunke Cao <yunkec@chromium.org>
> > ---
> > 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 <yunkec@chromium.org>
>
>
>
> --
> Ricardo Ribalda
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH RFC 0/3] meida: uvcvideo: reimplement privacy gpio as a separate subdevice
2023-02-14 5:45 ` Yunke Cao
@ 2023-03-08 22:57 ` Ricardo Ribalda
0 siblings, 0 replies; 11+ messages in thread
From: Ricardo Ribalda @ 2023-03-08 22:57 UTC (permalink / raw)
To: Yunke Cao, Sakari Ailus, Hans Verkuil, Laurent Pinchart
Cc: Mauro Carvalho Chehab, linux-kernel, linux-media
On Tue, 14 Feb 2023 at 06:46, Yunke Cao <yunkec@chromium.org> wrote:
>
> Hi!
>
> On Fri, Jan 13, 2023 at 5:26 PM Ricardo Ribalda <ribalda@chromium.org> wrote:
> >
> > Hi Yunke
> >
> > Thank you very much for the patchset :)
> >
> > On Wed, 11 Jan 2023 at 09:52, Yunke Cao <yunkec@chromium.org> wrote:
> > >
> > > privacy_gpio in uvc were added as V4L2_CID_PRIVACY in uvc video node in
> > > https://lore.kernel.org/all/20201223133528.55014-1-ribalda@chromium.org/
> > >
> > > 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 <ribalda@chromium.org>
> > > Signed-off-by: Yunke Cao <yunkec@chromium.org>
> > > ---
> > > 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 <yunkec@chromium.org>
> >
> >
> >
> > --
> > Ricardo Ribalda
--
Ricardo Ribalda
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH RFC 0/3] meida: uvcvideo: reimplement privacy gpio as a separate subdevice
2023-01-11 8:52 [PATCH RFC 0/3] meida: uvcvideo: reimplement privacy gpio as a separate subdevice Yunke Cao
` (3 preceding siblings ...)
2023-01-13 8:25 ` [PATCH RFC 0/3] meida: uvcvideo: reimplement privacy gpio " Ricardo Ribalda
@ 2023-11-21 20:05 ` Ricardo Ribalda
4 siblings, 0 replies; 11+ messages in thread
From: Ricardo Ribalda @ 2023-11-21 20:05 UTC (permalink / raw)
To: Yunke Cao, Sakari Ailus
Cc: Laurent Pinchart, Mauro Carvalho Chehab, Hans Verkuil,
linux-kernel, linux-media
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 <yunkec@chromium.org> wrote:
>
> privacy_gpio in uvc were added as V4L2_CID_PRIVACY in uvc video node in
> https://lore.kernel.org/all/20201223133528.55014-1-ribalda@chromium.org/
>
> 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 <ribalda@chromium.org>
> Signed-off-by: Yunke Cao <yunkec@chromium.org>
> ---
> 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 <yunkec@chromium.org>
--
Ricardo Ribalda
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH RFC 3/3] media: uvcvideo: reimplement privacy GPIO as a separate subdevice
2023-01-11 8:52 ` [PATCH RFC 3/3] media: uvcvideo: reimplement privacy GPIO as a separate subdevice Yunke Cao
@ 2023-11-21 20:46 ` Hans Verkuil
0 siblings, 0 replies; 11+ messages in thread
From: Hans Verkuil @ 2023-11-21 20:46 UTC (permalink / raw)
To: Yunke Cao, Laurent Pinchart, Mauro Carvalho Chehab, Ricardo Ribalda
Cc: linux-kernel, linux-media, Sakari Ailus
Hi Yunke,
A comment below:
On 11/01/2023 09:52, Yunke Cao wrote:
> Reimplement privacy GPIO as a v4l2 subdev with a volatile privacy control.
> A v4l2 control event is sent in irq when privacy control value changes.
>
> The behavior matches the original implementation, except that the
> control is of a separate subdevice.
>
> V4L2 control kAPI is used for simplicity.
>
> Signed-off-by: Yunke Cao <yunkec@chromium.org>
> ---
> drivers/media/usb/uvc/uvc_driver.c | 44 +++-------------------
> drivers/media/usb/uvc/uvc_entity.c | 76 ++++++++++++++++++++++++++++++++++++++
> drivers/media/usb/uvc/uvcvideo.h | 19 +++++++---
> 3 files changed, 94 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> index e4bcb5011360..25848f4dbce0 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -1209,43 +1209,6 @@ static int uvc_parse_control(struct uvc_device *dev)
> * Privacy GPIO
> */
>
> -static void uvc_gpio_event(struct uvc_device *dev)
> -{
> - struct uvc_entity *unit = dev->gpio_unit;
> - struct uvc_video_chain *chain;
> - u8 new_val;
> -
> - if (!unit)
> - return;
> -
> - new_val = gpiod_get_value_cansleep(unit->gpio.gpio_privacy);
> -
> - /* GPIO entities are always on the first chain. */
> - chain = list_first_entry(&dev->chains, struct uvc_video_chain, list);
> - uvc_ctrl_status_event(chain, unit->controls, &new_val);
> -}
> -
> -static int uvc_gpio_get_cur(struct uvc_device *dev, struct uvc_entity *entity,
> - u8 cs, void *data, u16 size)
> -{
> - if (cs != UVC_CT_PRIVACY_CONTROL || size < 1)
> - return -EINVAL;
> -
> - *(u8 *)data = gpiod_get_value_cansleep(entity->gpio.gpio_privacy);
> -
> - return 0;
> -}
> -
> -static int uvc_gpio_get_info(struct uvc_device *dev, struct uvc_entity *entity,
> - u8 cs, u8 *caps)
> -{
> - if (cs != UVC_CT_PRIVACY_CONTROL)
> - return -EINVAL;
> -
> - *caps = UVC_CONTROL_CAP_GET | UVC_CONTROL_CAP_AUTOUPDATE;
> - return 0;
> -}
> -
> static irqreturn_t uvc_gpio_irq(int irq, void *data)
> {
> struct uvc_device *dev = data;
> @@ -1279,8 +1242,6 @@ static int uvc_gpio_parse(struct uvc_device *dev)
> unit->gpio.bControlSize = 1;
> unit->gpio.bmControls = (u8 *)unit + sizeof(*unit);
> unit->gpio.bmControls[0] = 1;
> - unit->get_cur = uvc_gpio_get_cur;
> - unit->get_info = uvc_gpio_get_info;
> strscpy(unit->name, "GPIO", sizeof(unit->name));
>
> list_add_tail(&unit->list, &dev->entities);
> @@ -2202,6 +2163,11 @@ static int uvc_probe(struct usb_interface *intf,
> if (media_device_register(&dev->mdev) < 0)
> goto error;
> #endif
> +
> + /* Expose all subdev's nodes*/
> + if (v4l2_device_register_subdev_nodes(&dev->vdev) < 0)
> + goto error;
> +
> /* Save our data pointer in the interface data. */
> usb_set_intfdata(intf, dev);
>
> diff --git a/drivers/media/usb/uvc/uvc_entity.c b/drivers/media/usb/uvc/uvc_entity.c
> index 7c4d2f93d351..c8e41b42ffd8 100644
> --- a/drivers/media/usb/uvc/uvc_entity.c
> +++ b/drivers/media/usb/uvc/uvc_entity.c
> @@ -56,17 +56,90 @@ static int uvc_mc_create_links(struct uvc_video_chain *chain,
> return 0;
> }
>
> +static const struct v4l2_subdev_core_ops uvc_subdev_core_ops = {
> + .subscribe_event = v4l2_ctrl_subdev_subscribe_event,
> + .unsubscribe_event = v4l2_event_subdev_unsubscribe,
> +};
> +
> static const struct v4l2_subdev_ops uvc_subdev_ops = {
> + .core = &uvc_subdev_core_ops,
> };
>
> void uvc_mc_cleanup_entity(struct uvc_entity *entity)
> {
> + if (UVC_ENTITY_TYPE(entity) == UVC_EXT_GPIO_UNIT)
> + v4l2_ctrl_handler_free(&entity->gpio.hdl);
> +
> if (UVC_ENTITY_TYPE(entity) != UVC_TT_STREAMING)
> media_entity_cleanup(&entity->subdev.entity);
> else if (entity->vdev != NULL)
> media_entity_cleanup(&entity->vdev->entity);
> }
>
> +static int uvc_gpio_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
> +{
> + struct uvc_gpio *gpio =
> + container_of(ctrl->handler, struct uvc_gpio, hdl);
> +
> + ctrl->cur.val = gpiod_get_value_cansleep(gpio->gpio_privacy);
> +
> + return 0;
> +}
> +
> +static const struct v4l2_ctrl_ops uvc_gpio_ctrl_ops = {
> + .g_volatile_ctrl = uvc_gpio_g_volatile_ctrl,
> +};
> +
> +void uvc_gpio_event(struct uvc_device *dev)
> +{
> + struct uvc_entity *unit = dev->gpio_unit;
> + struct v4l2_ctrl *ctrl;
> + struct v4l2_event ev;
> + s32 old_val;
> + int ret;
> +
> + if (!unit)
> + return;
> +
> + ctrl = unit->gpio.privacy_ctrl;
> + old_val = ctrl->cur.val;
> +
> + v4l2_ctrl_lock(ctrl);
> +
> + ret = uvc_gpio_g_volatile_ctrl(ctrl);
> + if (ret < 0 || old_val == ctrl->cur.val) {
> + v4l2_ctrl_unlock(ctrl);
> + return;
> + }
> +
> + v4l2_ctrl_fill_event(&ev, ctrl, V4L2_EVENT_CTRL_CH_VALUE);
> + v4l2_ctrl_unlock(ctrl);
> +
> + v4l2_event_queue(unit->subdev.devnode, &ev);
> +}
> +
> +static void uvc_gpio_init_ctrl(struct uvc_entity *entity)
> +{
> + struct v4l2_ctrl *ctrl;
> + struct v4l2_ctrl_handler *hdl = &entity->gpio.hdl;
> +
> + entity->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE |
> + V4L2_SUBDEV_FL_HAS_EVENTS;
> +
> + v4l2_ctrl_handler_init(hdl, 1);
> + entity->subdev.ctrl_handler = hdl;
> + ctrl = v4l2_ctrl_new_std(hdl, &uvc_gpio_ctrl_ops, V4L2_CID_PRIVACY,
> + 0, 1, 1, 0);
> + if (ctrl)
> + ctrl->flags |= V4L2_CTRL_FLAG_VOLATILE |
> + V4L2_CTRL_FLAG_READ_ONLY;
This is not a volatile control. You know exactly what the value is
and when it changes since a gpio interrupt is issued.
So this is just a regular read-only control, and when uvc_gpio_event
is called you can just call v4l2_ctrl_s_ctrl with the new value
from gpiod_get_value_cansleep().
No need to expose v4l2_ctrl_fill_event either since the call above
will take care of the control event.
Regards,
Hans
> +
> + /* Read GPIO value to initialize the control. */
> + uvc_gpio_g_volatile_ctrl(ctrl);
> +
> + entity->gpio.privacy_ctrl = ctrl;
> +}
> +
> static int uvc_mc_init_entity(struct uvc_video_chain *chain,
> struct uvc_entity *entity)
> {
> @@ -113,6 +186,9 @@ static int uvc_mc_init_entity(struct uvc_video_chain *chain,
>
> entity->subdev.entity.function = function;
>
> + if (UVC_ENTITY_TYPE(entity) == UVC_EXT_GPIO_UNIT)
> + uvc_gpio_init_ctrl(entity);
> +
> ret = media_entity_pads_init(&entity->subdev.entity,
> entity->num_pads, entity->pads);
>
> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> index df93db259312..19ca2896c398 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -15,6 +15,7 @@
> #include <linux/videodev2.h>
> #include <linux/workqueue.h>
> #include <media/media-device.h>
> +#include <media/v4l2-ctrls.h>
> #include <media/v4l2-device.h>
> #include <media/v4l2-event.h>
> #include <media/v4l2-fh.h>
> @@ -163,6 +164,15 @@ struct uvc_control {
>
> #define UVC_ENTITY_FLAG_DEFAULT (1 << 0)
>
> +struct uvc_gpio {
> + u8 bControlSize;
> + u8 *bmControls;
> + struct gpio_desc *gpio_privacy;
> + int irq;
> + struct v4l2_ctrl_handler hdl;
> + struct v4l2_ctrl *privacy_ctrl;
> +};
> +
> struct uvc_entity {
> struct list_head list; /* Entity as part of a UVC device. */
> struct list_head chain; /* Entity as part of a video device chain. */
> @@ -221,12 +231,7 @@ struct uvc_entity {
> u8 *bmControlsType;
> } extension;
>
> - struct {
> - u8 bControlSize;
> - u8 *bmControls;
> - struct gpio_desc *gpio_privacy;
> - int irq;
> - } gpio;
> + struct uvc_gpio gpio;
> };
>
> u8 bNrInPins;
> @@ -766,6 +771,8 @@ int uvc_ctrl_is_accessible(struct uvc_video_chain *chain, u32 v4l2_id,
> int uvc_xu_ctrl_query(struct uvc_video_chain *chain,
> struct uvc_xu_control_query *xqry);
>
> +void uvc_gpio_event(struct uvc_device *dev);
> +
> /* Utility functions */
> struct usb_host_endpoint *uvc_find_endpoint(struct usb_host_interface *alts,
> u8 epaddr);
>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-11-21 20:46 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-11 8:52 [PATCH RFC 0/3] meida: uvcvideo: reimplement privacy gpio as a separate subdevice Yunke Cao
2023-01-11 8:52 ` [PATCH RFC 1/3] media: v4l2-ctrls: Expose v4l2_ctrl_fill_event() Yunke Cao
2023-01-11 8:52 ` [PATCH RFC 2/3] media: uvcvideo: remove entity privacy control in the uvc video node Yunke Cao
2023-01-11 10:33 ` Andrzej Pietrasiewicz
2023-01-12 1:40 ` Yunke Cao
2023-01-11 8:52 ` [PATCH RFC 3/3] media: uvcvideo: reimplement privacy GPIO as a separate subdevice Yunke Cao
2023-11-21 20:46 ` Hans Verkuil
2023-01-13 8:25 ` [PATCH RFC 0/3] meida: uvcvideo: reimplement privacy gpio " Ricardo Ribalda
2023-02-14 5:45 ` Yunke Cao
2023-03-08 22:57 ` Ricardo Ribalda
2023-11-21 20:05 ` Ricardo Ribalda
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).