Hi Michael,
Thank you for the patch.
On Mon, Jun 14, 2021 at 07:35:58PM +0900, paul.elder@xxxxxxxxxxxxxxxx wrote:
> On Mon, May 31, 2021 at 12:30:41AM +0200, Michael Grzeschik wrote:
> > With usb3 we handle much more requests. It only enables the interrupt on
>
> s/much/many/
>
> > every quarter of the allocated requests. This patch decreases the
> > interrupt load.
>
> The last two sentences might be better combined, like:
>
> "Decrease the interrupt load by only enabling the interrupt every
> quarter of the allocated requests."
>
> > Signed-off-by: Michael Grzeschik <m.grzeschik@xxxxxxxxxxxxxx>
>
> Other than that, looks good to me.
>
> Reviewed-by: Paul Elder <paul.elder@xxxxxxxxxxxxxxxx>
>
> > ---
> > drivers/usb/gadget/function/uvc.h | 2 ++
> > drivers/usb/gadget/function/uvc_video.c | 12 ++++++++++++
> > 2 files changed, 14 insertions(+)
> >
> > diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h
> > index c1f06d9df5820..5a76e9351b530 100644
> > --- a/drivers/usb/gadget/function/uvc.h
> > +++ b/drivers/usb/gadget/function/uvc.h
> > @@ -101,6 +101,8 @@ struct uvc_video {
> > struct list_head req_free;
> > spinlock_t req_lock;
> >
> > + int req_int_count;
unsigned int.
> > +
> > void (*encode) (struct usb_request *req, struct uvc_video *video,
> > struct uvc_buffer *buf);
> >
> > diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c
> > index 240d361a45a44..66754687ce217 100644
> > --- a/drivers/usb/gadget/function/uvc_video.c
> > +++ b/drivers/usb/gadget/function/uvc_video.c
> > @@ -357,6 +357,16 @@ static void uvcg_video_pump(struct work_struct *work)
> >
> > video->encode(req, video, buf);
> >
A comment to explain the logic would be useful.
> > + if (list_empty(&video->req_free) ||
> > + (buf->state == UVC_BUF_STATE_DONE) ||
No need for parentheses here.
> > + (!(video->req_int_count %
> > + DIV_ROUND_UP(video->uvc_num_requests, 4)))) {
> > + video->req_int_count = 0;
> > + req->no_interrupt = 0;
> > + } else {
> > + req->no_interrupt = 1;
> > + }
> > +
> > /* Queue the USB request */
> > ret = uvcg_video_ep_queue(video, req);
> > spin_unlock_irqrestore(&queue->irqlock, flags);
> > @@ -365,6 +375,7 @@ static void uvcg_video_pump(struct work_struct *work)
> > uvcg_queue_cancel(queue, 0);
> > break;
> > }
> > + video->req_int_count++;
> > }
> >
> > spin_lock_irqsave(&video->req_lock, flags);
> > @@ -437,6 +448,7 @@ int uvcg_video_init(struct uvc_video *video, struct uvc_device *uvc)
> > video->width = 320;
> > video->height = 240;
> > video->imagesize = 320 * 240 * 2;
> > + video->req_int_count = 0;
Should this be initialized to 0 in uvcg_video_enable() instead of
uvcg_video_init(), to ensure that stop/start cycles will operate in a
predictable way ?
> >
> > /* Initialize the video buffers queue. */
> > uvcg_queue_init(uvc->v4l2_dev.dev, &video->queue, V4L2_BUF_TYPE_VIDEO_OUTPUT,
--
Regards,
Laurent Pinchart
