Hi Michal,
Thank you for the patch, and sorry for the late reply.
On Friday 06 September 2013 00:18:54 Michael Grzeschik wrote:
> This patch lets the kernel handle setup and data events of
> the uvc gadget. It is generating its response data by the
> prepared usb uvc control descriptores.
>
> For now only simple probe and commit events are generated and
> stored inside the uvc_device structure.
While I like to idea of handling requests in the kernel without userspace
intervention, the current implementation is too limited. All requests but
streaming control requests are ignored. The unsupported requests should be
forwarded to userspace instead (there's no way to handle controls in the
kernel).
The implementation of streaming control requests is in my opinion also
problematic. UVC_GET_MIN and UVC_GET_DEF return the same values and don't
support use cases where the default value should be different than the minimum
value. The compression quality isn't handled, and should be forwarded to
userspace. Setting the streaming commit control during streaming isn't
disallowed.
To summarize my point, this is an interesting idea, but the implementation
hasn't reached a point where it could be mainlined.
> Signed-off-by: Michael Grzeschik <m.grzeschik@xxxxxxxxxxxxxx>
> ---
> drivers/usb/gadget/f_uvc.c | 273 ++++++++++++++++++++++++++++++++++++---
> drivers/usb/gadget/uvc.h | 7 ++
> drivers/usb/gadget/uvc_v4l2.c | 12 --
> 3 files changed, 265 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/usb/gadget/f_uvc.c b/drivers/usb/gadget/f_uvc.c
> index e2a1f50..73fdf90 100644
> --- a/drivers/usb/gadget/f_uvc.c
> +++ b/drivers/usb/gadget/f_uvc.c
> @@ -213,20 +213,257 @@ static const struct usb_descriptor_header * const
> uvc_ss_streaming[] = { */
>
> static void
> +uvc_get_format(struct uvc_device *uvc, struct uvc_streaming_control *ctrl,
> struct v4l2_format *fmt)
> +{
> + unsigned int nframes = 1, pformat = 1;
> + struct usb_composite_dev *cdev = uvc->func.config->cdev;
> + const struct uvc_descriptor_header * const *uvc_streaming_cls;
> + const struct uvc_descriptor_header *format, *frame;
> + struct uvc_input_header_descriptor *input;
> + unsigned int absdiff, count;
> +
> + /* main descriptor */
> + uvc_streaming_cls = uvc->desc.fs_streaming;
> + if (gadget_is_dualspeed(cdev->gadget))
> + uvc_streaming_cls = uvc->desc.hs_streaming;
> + if (gadget_is_superspeed(cdev->gadget))
> + uvc_streaming_cls = uvc->desc.ss_streaming;
> +
> + /* input header */
> + input = (struct uvc_input_header_descriptor *) *uvc_streaming_cls;
> + ctrl->bFormatIndex =
> + clamp((unsigned int)ctrl->bFormatIndex, 1U, (unsigned
> int)input->bNumFormats);
> +
> + /* format header */
> + do {
> + uvc_streaming_cls += nframes; /* skip to the next format header */
> + format = *uvc_streaming_cls;
> + if (format->bDescriptorSubType == UVC_VS_FORMAT_UNCOMPRESSED) {
> + struct uvc_format_uncompressed *yuyv =
> + (struct uvc_format_uncompressed *)format;
> + nframes = yuyv->bNumFrameDescriptors;
> + fmt->fmt.pix.pixelformat = V4L2_PIX_FMT_YUYV;
> + } else if (format->bDescriptorSubType == UVC_VS_FORMAT_MJPEG) {
> + struct uvc_format_mjpeg *mjpeg =
> + (struct uvc_format_mjpeg *)format;
> + nframes = mjpeg->bNumFrameDescriptors;
> + fmt->fmt.pix.pixelformat = V4L2_PIX_FMT_MJPEG;
> + }
> + pformat++;
> + } while (pformat < ctrl->bFormatIndex);
> +
> + ctrl->bFrameIndex = clamp((unsigned int)ctrl->bFrameIndex, 1U, nframes);
> +
> + /* frame header */
> + uvc_streaming_cls += ctrl->bFrameIndex;
> + frame = *uvc_streaming_cls;
> + if (format->bDescriptorSubType == UVC_VS_FORMAT_UNCOMPRESSED) {
> + struct uvc_frame_uncompressed *yuyv =
> + (struct uvc_frame_uncompressed *)frame;
> + fmt->fmt.pix.width = yuyv->wWidth;
> + fmt->fmt.pix.height = yuyv->wHeight;
> + fmt->fmt.pix.sizeimage = fmt->fmt.pix.width * fmt->fmt.pix.height *
2;
> + fmt->fmt.pix.priv = yuyv->dwDefaultFrameInterval;
> + if (ctrl->dwFrameInterval) {
> + absdiff = ctrl->dwFrameInterval - yuyv->dwFrameInterval[0];
> + for (count = 1; count < yuyv->bFrameIntervalType; count++) {
> + unsigned int tmpdiff = ctrl->dwFrameInterval -
> yuyv->dwFrameInterval[count];
> + if (tmpdiff < absdiff) {
> + absdiff = tmpdiff;
> + fmt->fmt.pix.priv = yuyv->dwFrameInterval[count];
> + }
> + }
> + }
> + } else if (format->bDescriptorSubType == UVC_VS_FORMAT_MJPEG) {
> + struct uvc_frame_mjpeg *mjpeg =
> + (struct uvc_frame_mjpeg *)frame;
> + fmt->fmt.pix.width = mjpeg->wWidth;
> + fmt->fmt.pix.height = mjpeg->wHeight;
> + fmt->fmt.pix.sizeimage = fmt->fmt.pix.width * fmt->fmt.pix.height;
> + fmt->fmt.pix.priv = mjpeg->dwDefaultFrameInterval;
> + if (ctrl->dwFrameInterval) {
> + absdiff = ctrl->dwFrameInterval - mjpeg->dwFrameInterval[0];
> + for (count = 1; count < mjpeg->bFrameIntervalType; count++) {
> + unsigned int tmpdiff = ctrl->dwFrameInterval -
> mjpeg->dwFrameInterval[count];
> + if (tmpdiff < absdiff) {
> + absdiff = tmpdiff;
> + fmt->fmt.pix.priv = mjpeg->dwFrameInterval[count];
> + }
> + }
> + }
> + }
> +}
> +
> +static void
> +uvc_fill_streaming_control(struct uvc_device *uvc,
> + struct uvc_streaming_control *ctrl,
> + int iframe, int iformat)
> +{
> + struct v4l2_format fmt;
> +
> + memset(&fmt, 0, sizeof(fmt));
> + memset(ctrl, 0, sizeof(*ctrl));
> +
> + ctrl->bmHint = 1;
> + ctrl->bFormatIndex = iformat;
> + ctrl->bFrameIndex = iframe;
> + ctrl->bmFramingInfo = 3;
> + ctrl->bPreferedVersion = 1;
> + ctrl->bMaxVersion = 1;
> + ctrl->dwMaxPayloadTransferSize = uvc->video.ep->maxpacket;
> +
> + uvc_get_format(uvc, ctrl, &fmt);
> +
> + ctrl->dwFrameInterval = fmt.fmt.pix.priv;
> + ctrl->dwMaxVideoFrameSize = fmt.fmt.pix.sizeimage;
> +}
> +
> +void
> +uvc_events_process_data(struct uvc_device *uvc, struct usb_request *req)
> +{
> + struct uvc_streaming_control *target;
> + struct uvc_streaming_control *ctrl;
> + struct v4l2_format fmt;
> + int ret;
> +
> + memset(&fmt, 0, sizeof(struct v4l2_format));
> + fmt.type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
> + fmt.fmt.pix.field = V4L2_FIELD_NONE;
> +
> + ctrl = (struct uvc_streaming_control *)req->buf;
> +
> + switch (uvc->control) {
> + case UVC_VS_PROBE_CONTROL:
> + printk(KERN_DEBUG "setting probe control, length = %d\n", req-
>actual);
> + target = &uvc->probe;
> + break;
> +
> + case UVC_VS_COMMIT_CONTROL:
> + printk(KERN_DEBUG "setting commit control, length = %d\n", req-
>actual);
> + target = &uvc->commit;
> + break;
> +
> + default:
> + printk(KERN_DEBUG "setting unknown control, length = %d\n", req-
>actual);
> + return;
> + }
> +
> + uvc_get_format(uvc, ctrl, &fmt);
> +
> + target->bFrameIndex = ctrl->bFrameIndex;
> + target->bFormatIndex = ctrl->bFormatIndex;
> + target->dwFrameInterval = fmt.fmt.pix.priv;
> + target->dwMaxVideoFrameSize = fmt.fmt.pix.sizeimage;
> +
> + if (uvc->control == UVC_VS_COMMIT_CONTROL) {
> + memcpy(&uvc->current_fmt, &fmt, sizeof(struct v4l2_format));
> + printk(KERN_DEBUG "Setting format to 0x%08x %ux%u\n",
> + fmt.fmt.pix.pixelformat, fmt.fmt.pix.width, fmt.fmt.pix.height);
> +
> + ret = uvc_v4l2_set_format(&uvc->video, &uvc->current_fmt);
> + if (ret < 0)
> + printk(KERN_ERR "Unable to set format.\n");
> + }
> +}
> +
> +static void
> +uvc_events_process_streaming(struct uvc_device *uvc, uint8_t req, uint8_t
> cs,
> + struct uvc_request_data *resp)
> +{
> + struct uvc_streaming_control *ctrl;
> +
> + printk(KERN_DEBUG "streaming request (req %02x cs %02x)\n", req, cs);
> +
> + if (cs != UVC_VS_PROBE_CONTROL && cs != UVC_VS_COMMIT_CONTROL)
> + return;
> +
> + ctrl = (struct uvc_streaming_control *)&resp->data;
> + resp->length = sizeof(*ctrl);
> +
> + switch (req) {
> + case UVC_SET_CUR:
> + uvc->control = cs;
> + resp->length = 34;
> + break;
> +
> + case UVC_GET_CUR:
> + if (cs == UVC_VS_PROBE_CONTROL)
> + memcpy(ctrl, &uvc->probe, sizeof(*ctrl));
> + else
> + memcpy(ctrl, &uvc->commit, sizeof(*ctrl));
> + break;
> +
> + case UVC_GET_MIN:
> + case UVC_GET_MAX:
> + case UVC_GET_DEF:
> + uvc_fill_streaming_control(uvc, ctrl, req == UVC_GET_MAX ? -1 : 0,
> + req == UVC_GET_MAX ? -1 : 0);
> + break;
> +
> + case UVC_GET_RES:
> + memset(ctrl, 0, sizeof(*ctrl));
> + break;
> +
> + case UVC_GET_LEN:
> + resp->data[0] = 0x00;
> + resp->data[1] = 0x22;
> + resp->length = 2;
> + break;
> +
> + case UVC_GET_INFO:
> + resp->data[0] = 0x03;
> + resp->length = 1;
> + break;
> + }
> +}
> +
> +static void
> +uvc_events_process_class(struct uvc_device *uvc, const struct
> usb_ctrlrequest *ctrl,
> + struct uvc_request_data *resp)
> +{
> + if ((ctrl->bRequestType & USB_RECIP_MASK) != USB_RECIP_INTERFACE)
> + return;
> +
> + switch (ctrl->wIndex & 0xff) {
> + case UVC_INTF_STREAMING:
> + uvc_events_process_streaming(uvc, ctrl->bRequest, ctrl->wValue >> 8,
> resp);
> + break;
> +
> + case UVC_INTF_CONTROL:
> + default:
> + break;
> + }
> +}
> +
> +void
> +uvc_events_process_setup(struct uvc_device *uvc, const struct
> usb_ctrlrequest *ctrl,
> + struct uvc_request_data *resp)
> +{
> + uvc->control = 0;
> +
> + printk(KERN_DEBUG "bRequestType %02x bRequest %02x wValue %04x wIndex
> %04x"
> + "wLength %04x\n", ctrl->bRequestType, ctrl->bRequest,
> + ctrl->wValue, ctrl->wIndex, ctrl->wLength);
> +
> + switch (ctrl->bRequestType & USB_TYPE_MASK) {
> + case USB_TYPE_CLASS:
> + uvc_events_process_class(uvc, ctrl, resp);
> + break;
> +
> + case USB_TYPE_STANDARD:
> + default:
> + break;
> + }
> +}
> +
> +static void
> uvc_function_ep0_complete(struct usb_ep *ep, struct usb_request *req)
> {
> struct uvc_device *uvc = req->context;
> - struct v4l2_event v4l2_event;
> - struct uvc_event *uvc_event = (void *)&v4l2_event.u.data;
>
> if (uvc->event_setup_out) {
> uvc->event_setup_out = 0;
> -
> - memset(&v4l2_event, 0, sizeof(v4l2_event));
> - v4l2_event.type = UVC_EVENT_DATA;
> - uvc_event->data.length = req->actual;
> - memcpy(&uvc_event->data.data, req->buf, req->actual);
> - v4l2_event_queue(uvc->vdev, &v4l2_event);
> + uvc_events_process_data(uvc, req);
> }
> }
>
> @@ -234,8 +471,8 @@ static int
> uvc_function_setup(struct usb_function *f, const struct usb_ctrlrequest
> *ctrl) {
> struct uvc_device *uvc = to_uvc(f);
> - struct v4l2_event v4l2_event;
> - struct uvc_event *uvc_event = (void *)&v4l2_event.u.data;
> + struct uvc_request_data resp;
> + int ret = 0;
>
> /* printk(KERN_INFO "setup request %02x %02x value %04x index %04x
> %04x\n",
> * ctrl->bRequestType, ctrl->bRequest, le16_to_cpu(ctrl->wValue),
> @@ -251,12 +488,15 @@ uvc_function_setup(struct usb_function *f, const
> struct usb_ctrlrequest *ctrl)
> if (le16_to_cpu(ctrl->wLength) > UVC_MAX_REQUEST_SIZE)
> return -EINVAL;
>
> - memset(&v4l2_event, 0, sizeof(v4l2_event));
> - v4l2_event.type = UVC_EVENT_SETUP;
> - memcpy(&uvc_event->req, ctrl, sizeof(uvc_event->req));
> - v4l2_event_queue(uvc->vdev, &v4l2_event);
> + uvc->event_setup_out =
> + !(ctrl->bRequestType & USB_DIR_IN);
>
> - return 0;
> + uvc->event_length = ctrl->wLength;
> +
> + uvc_events_process_setup(uvc, ctrl, &resp);
> + ret = uvc_send_response(uvc, &resp);
> +
> + return ret;
> }
>
> void uvc_function_setup_continue(struct uvc_device *uvc)
> @@ -708,6 +948,9 @@ uvc_function_bind(struct usb_configuration *c, struct
> usb_function *f) goto error;
> }
>
> + uvc_fill_streaming_control(uvc, &uvc->probe, 0, 0);
> + uvc_fill_streaming_control(uvc, &uvc->commit, 0, 0);
> +
> return 0;
>
> error:
> diff --git a/drivers/usb/gadget/uvc.h b/drivers/usb/gadget/uvc.h
> index 7a9111d..ebbabc1 100644
> --- a/drivers/usb/gadget/uvc.h
> +++ b/drivers/usb/gadget/uvc.h
> @@ -170,6 +170,13 @@ struct uvc_device
> /* Events */
> unsigned int event_length;
> unsigned int event_setup_out : 1;
> +
> + struct uvc_streaming_control probe;
> + struct uvc_streaming_control commit;
> +
> + int control;
> +
> + struct v4l2_format current_fmt;
> };
>
> static inline struct uvc_device *to_uvc(struct usb_function *f)
> diff --git a/drivers/usb/gadget/uvc_v4l2.c b/drivers/usb/gadget/uvc_v4l2.c
> index ad48e81..b4ab764 100644
> --- a/drivers/usb/gadget/uvc_v4l2.c
> +++ b/drivers/usb/gadget/uvc_v4l2.c
> @@ -276,18 +276,6 @@ uvc_v4l2_do_ioctl(struct file *file, unsigned int cmd,
> void *arg)
>
> ret = v4l2_event_dequeue(&handle->vfh, event,
> file->f_flags & O_NONBLOCK);
> - if (ret == 0 && event->type == UVC_EVENT_SETUP) {
> - struct uvc_event *uvc_event = (void *)&event->u.data;
> -
> - /* Tell the complete callback to generate an event for
> - * the next request that will be enqueued by
> - * uvc_event_write.
> - */
> - uvc->event_setup_out =
> - !(uvc_event->req.bRequestType & USB_DIR_IN);
> - uvc->event_length = uvc_event->req.wLength;
> - }
> -
> return ret;
> }
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
