Hello,
On Mon, Dec 06, 2021 at 08:34:17AM +0100, Greg KH wrote:
> 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.
We've moved memcpy()s to a work queue in the host UVC driver for
high-bandwidth devices, which brough massive performance improvements
(if I recall correctly, at least partly due to the parallelization of
memcpy operations). I'm not sure we have measured performances in the
gadget driver with the same level of accuracy and care though.
Michael, do you plan to make measurements ?
--
Regards,
Laurent Pinchart
