Re: [PATCH 3/3] media: stk1160: use dma_alloc_noncontiguous API

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 11.01.22 14:38, Ezequiel Garcia wrote:
Hi Dafna,

Very nice work.

I specially like all the testing that you mentioned in the cover-letter.

Back in the day, there were a few users trying to use STK1160 (Easycap)
with Beaglebone boards, without success due to lack of USB throughput.

Hi, what do you mean by "USB throughput" ?
I see that the FPS does not change with this patchset and stands on about 25 frames/sec
So this patchset only improve cpu time.


If anything else, I think it's good to see additional noncontiguous API users.

On Tue, Jan 11, 2022 at 08:55:05AM +0200, Dafna Hirschfeld wrote:
Replace the urb buffers allocation to
use the noncontiguous API. This improve performance
on Arm.
The noncontiguous API require handling
synchronization.
This commit is similar to the one sent to
uvc: [1]

[1] https://lkml.org/lkml/2021/3/12/1506


This commit description needs lots of attention. In particular,
please add the test results here.

Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@xxxxxxxxxxxxx>
---
  drivers/media/usb/stk1160/stk1160-v4l.c   |   3 +
  drivers/media/usb/stk1160/stk1160-video.c | 109 +++++++++++++---------
  drivers/media/usb/stk1160/stk1160.h       |  10 ++
  3 files changed, 79 insertions(+), 43 deletions(-)

[..]
@@ -501,7 +524,7 @@ int stk1160_alloc_isoc(struct stk1160 *dev)
free_i_bufs:
  	/* Save the allocated buffers so far, so we can properly free them */
-	dev->isoc_ctl.num_bufs = i+1;
+	dev->isoc_ctl.num_bufs = i;

This looks like a separate fix, similar to 1/3 ?

  	stk1160_free_isoc(dev);
  	return -ENOMEM;
  }
diff --git a/drivers/media/usb/stk1160/stk1160.h b/drivers/media/usb/stk1160/stk1160.h
index 1ffca1343d88..52bea7815ae5 100644
--- a/drivers/media/usb/stk1160/stk1160.h
+++ b/drivers/media/usb/stk1160/stk1160.h
@@ -16,6 +16,8 @@
  #include <media/videobuf2-v4l2.h>
  #include <media/v4l2-device.h>
  #include <media/v4l2-ctrls.h>
+#include <linux/usb.h>
+#include <linux/usb/hcd.h>
#define STK1160_VERSION "0.9.5"
  #define STK1160_VERSION_NUM	0x000905
@@ -87,6 +89,9 @@ struct stk1160_buffer {
  struct stk1160_urb {
  	struct urb *urb;
  	char *transfer_buffer;
+	struct sg_table *sgt;
+	struct stk1160 *dev;
+	dma_addr_t dma;
  };
struct stk1160_isoc_ctl {
@@ -190,3 +195,8 @@ void stk1160_select_input(struct stk1160 *dev);
/* Provided by stk1160-ac97.c */
  void stk1160_ac97_setup(struct stk1160 *dev);
+
+static inline struct device *stk1160_get_dmadev(struct stk1160 *dev)
+{
+	return bus_to_hcd(dev->udev->bus)->self.sysdev;

This function looks truly horrible, is there no other way to get the device ?

I suppose this function return something different than (struct stk1160*)dev->dev ?

I remember I tried using this device and got an error that it is not cable of doing dma
allocations or something like that. The function "stk1160_get_dmadev" is a copy-paste from the
uvc equivalent:

static inline struct device *uvc_stream_to_dmadev(struct uvc_streaming *stream)
{
        return bus_to_hcd(stream->dev->udev->bus)->self.sysdev;
}

Thanks,
Dafna


Thanks,
Ezequiel




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux