Re: [PATCH v2 6/6] uvcvideo: Add VIDIOC_[GS]_PRIORITY support

2012-11-23 Thread Hans Verkuil
Hi Laurent,

You were right about your remark about setting USE_FH_PRIO: you do need to do
that here.

Thanks for reposting with more context, now I can see where the prio checks are
added :-)

I have just one small remark:

On Fri November 23 2012 13:32:05 Laurent Pinchart wrote:
> Signed-off-by: Laurent Pinchart 
> ---
>  drivers/media/usb/uvc/uvc_driver.c |3 ++
>  drivers/media/usb/uvc/uvc_v4l2.c   |   45 
> 
>  drivers/media/usb/uvc/uvcvideo.h   |1 +
>  3 files changed, 49 insertions(+), 0 deletions(-)
> 
> Resent with larger contexts to make review easier.
> 
> diff --git a/drivers/media/usb/uvc/uvc_driver.c 
> b/drivers/media/usb/uvc/uvc_driver.c
> index ae24f7d..22f14d2 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -651,10 +668,14 @@ static long uvc_v4l2_do_ioctl(struct file *file, 
> unsigned int cmd, void *arg)
>   ret = uvc_ctrl_rollback(handle);
>   break;
>   }
>  
>   case VIDIOC_S_EXT_CTRLS:
> + ret = v4l2_prio_check(vdev->prio, handle->vfh.prio);
> + if (ret < 0)
> + return ret;

Please add a /* fall through */ comment here.

> +
>   case VIDIOC_TRY_EXT_CTRLS:
>   {
>   struct v4l2_ext_controls *ctrls = arg;
>   struct v4l2_ext_control *ctrl = ctrls->controls;
>   unsigned int i;

After making that change you can add my Acked-by:

Acked-by: Hans Verkuil 

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 6/6] uvcvideo: Add VIDIOC_[GS]_PRIORITY support

2012-11-23 Thread Laurent Pinchart
Signed-off-by: Laurent Pinchart 
---
 drivers/media/usb/uvc/uvc_driver.c |3 ++
 drivers/media/usb/uvc/uvc_v4l2.c   |   45 
 drivers/media/usb/uvc/uvcvideo.h   |1 +
 3 files changed, 49 insertions(+), 0 deletions(-)

Resent with larger contexts to make review easier.

diff --git a/drivers/media/usb/uvc/uvc_driver.c 
b/drivers/media/usb/uvc/uvc_driver.c
index ae24f7d..22f14d2 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -1560,10 +1560,11 @@ static int uvc_scan_device(struct uvc_device *dev)
return -ENOMEM;
 
INIT_LIST_HEAD(&chain->entities);
mutex_init(&chain->ctrl_mutex);
chain->dev = dev;
+   v4l2_prio_init(&chain->prio);
 
if (uvc_scan_chain(chain, term) < 0) {
kfree(chain);
continue;
}
@@ -1720,10 +1721,12 @@ static int uvc_register_video(struct uvc_device *dev,
 * get another one.
 */
vdev->v4l2_dev = &dev->vdev;
vdev->fops = &uvc_fops;
vdev->release = uvc_release;
+   vdev->prio = &stream->chain->prio;
+   set_bit(V4L2_FL_USE_FH_PRIO, &vdev->flags);
if (stream->type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
vdev->vfl_dir = VFL_DIR_TX;
strlcpy(vdev->name, dev->name, sizeof vdev->name);
 
/* Set the driver data before calling video_register_device, otherwise
diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
index bf9d073..d6aa402 100644
--- a/drivers/media/usb/uvc/uvc_v4l2.c
+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -574,10 +574,23 @@ static long uvc_v4l2_do_ioctl(struct file *file, unsigned 
int cmd, void *arg)
cap->device_caps = V4L2_CAP_VIDEO_OUTPUT
 | V4L2_CAP_STREAMING;
break;
}
 
+   /* Priority */
+   case VIDIOC_G_PRIORITY:
+   *(u32 *)arg = v4l2_prio_max(vdev->prio);
+   break;
+
+   case VIDIOC_S_PRIORITY:
+   ret = v4l2_prio_check(vdev->prio, handle->vfh.prio);
+   if (ret < 0)
+   return ret;
+
+   return v4l2_prio_change(vdev->prio, &handle->vfh.prio,
+   *(u32 *)arg);
+
/* Get, Set & Query control */
case VIDIOC_QUERYCTRL:
return uvc_query_v4l2_ctrl(chain, arg);
 
case VIDIOC_G_CTRL:
@@ -604,10 +617,14 @@ static long uvc_v4l2_do_ioctl(struct file *file, unsigned 
int cmd, void *arg)
case VIDIOC_S_CTRL:
{
struct v4l2_control *ctrl = arg;
struct v4l2_ext_control xctrl;
 
+   ret = v4l2_prio_check(vdev->prio, handle->vfh.prio);
+   if (ret < 0)
+   return ret;
+
memset(&xctrl, 0, sizeof xctrl);
xctrl.id = ctrl->id;
xctrl.value = ctrl->value;
 
ret = uvc_ctrl_begin(chain);
@@ -651,10 +668,14 @@ static long uvc_v4l2_do_ioctl(struct file *file, unsigned 
int cmd, void *arg)
ret = uvc_ctrl_rollback(handle);
break;
}
 
case VIDIOC_S_EXT_CTRLS:
+   ret = v4l2_prio_check(vdev->prio, handle->vfh.prio);
+   if (ret < 0)
+   return ret;
+
case VIDIOC_TRY_EXT_CTRLS:
{
struct v4l2_ext_controls *ctrls = arg;
struct v4l2_ext_control *ctrl = ctrls->controls;
unsigned int i;
@@ -745,10 +766,14 @@ static long uvc_v4l2_do_ioctl(struct file *file, unsigned 
int cmd, void *arg)
 
case VIDIOC_S_INPUT:
{
u32 input = *(u32 *)arg + 1;
 
+   ret = v4l2_prio_check(vdev->prio, handle->vfh.prio);
+   if (ret < 0)
+   return ret;
+
if ((ret = uvc_acquire_privileges(handle)) < 0)
return ret;
 
if (chain->selector == NULL ||
(chain->dev->quirks & UVC_QUIRK_IGNORE_SELECTOR_UNIT)) {
@@ -798,10 +823,14 @@ static long uvc_v4l2_do_ioctl(struct file *file, unsigned 
int cmd, void *arg)
 
return uvc_v4l2_try_format(stream, arg, &probe, NULL, NULL);
}
 
case VIDIOC_S_FMT:
+   ret = v4l2_prio_check(vdev->prio, handle->vfh.prio);
+   if (ret < 0)
+   return ret;
+
if ((ret = uvc_acquire_privileges(handle)) < 0)
return ret;
 
return uvc_v4l2_set_format(stream, arg);
 
@@ -900,10 +929,14 @@ static long uvc_v4l2_do_ioctl(struct file *file, unsigned 
int cmd, void *arg)
/* Get & Set streaming parameters */
case VIDIOC_G_PARM:
return uvc_v4l2_get_streamparm(stream, arg);
 
case VIDIOC_S_PARM:
+