[RFC,0/3] meida: uvcvideo: reimplement privacy gpio as a separate subdevice

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,
  

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>
  

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
  

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
  

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>