On Sun, Dec 05, 2021 at 10:49:58PM +0100, Michael Grzeschik wrote:
> On Fri, Dec 03, 2021 at 01:55:01PM +0100, Greg KH wrote:
> > On Thu, Dec 02, 2021 at 01:58:52AM +0100, Michael Grzeschik wrote:
> > > Preparing the usb request is not very expensive, when using
> > > scatter gather. In that case we can even remove the overhead
> > > of the extra pump worker and call the pump function directly.
> > >
> > > Signed-off-by: Michael Grzeschik <m.grzeschik@xxxxxxxxxxxxxx>
> > > ---
> > > drivers/usb/gadget/function/uvc_v4l2.c | 8 +++++--
> > > drivers/usb/gadget/function/uvc_video.c | 28 +++++++++++++++++++------
> > > drivers/usb/gadget/function/uvc_video.h | 2 ++
> > > 3 files changed, 30 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/usb/gadget/function/uvc_v4l2.c b/drivers/usb/gadget/function/uvc_v4l2.c
> > > index a2c78690c5c288..020b4adc7840e0 100644
> > > --- a/drivers/usb/gadget/function/uvc_v4l2.c
> > > +++ b/drivers/usb/gadget/function/uvc_v4l2.c
> > > @@ -169,8 +169,12 @@ uvc_v4l2_qbuf(struct file *file, void *fh, struct v4l2_buffer *b)
> > > if (ret < 0)
> > > return ret;
> > >
> > > - if (uvc->state == UVC_STATE_STREAMING)
> > > - schedule_work(&video->pump);
> > > + if (uvc->state == UVC_STATE_STREAMING) {
> > > + if (!video->queue.use_sg)
> > > + schedule_work(&video->pump);
> > > + else
> > > + uvcg_video_pump(video);
> >
> > Wouldn't it be easier to understand this if you flip the if test around:
> > if (video->queue.use_sg)
> > uvcg_video_pump(video);
> > else
> > schedule_work(&video->pump);
> > ?
> >
> > Negagive logic is never fun to read...
>
> Yes, you are not wrong.
>
> > Also, are you sure that sg really is not expensive on all systems? What
> > did you test this on, and what was the results?
>
> I tested it on an zynqmp arm64 machine. I tried to test the sg case on
> an 32 bit IMX with chipidea, but the driver was complaining a lot about
> "not page aligned sg buffers". So if needed, I would first need to find
> a working machine to compare this with.
>
> However I would think that assigning some pointers on a scatterlist
> instead of doing memcpy of 1024 bytes should be less expensive.
Not true on many systems, memcpy can be _very_ fast, especially for
small amounts like 1024 bytes. So please, measure this to be sure.
thanks,
greg k-h
