gspca second isoc endpoint / kinect depth

gspca second isoc endpoint / kinect depth

[Alexander Sosna]

Hi,

I took drivers/media/usb/gspca/kinect.c as skeleton to build a depth
driver for the kinect camera.

I needed to implement this feature because libfreenect performs so badly
on the raspberry pi that you can't get a single frame.

The kinecet has two isoc endpoints but gspca only uses the first.
To get it running I made a dirty hack to drivers/media/usb/gspca/gspca.c
I changed usb_host_endpoint *alt_xfer(...) so that it always returns the
second endpoint, which is not really good for everyone.


My driver is not ready for upstream now, it can not coexist with the
current gspca_kinect so you have to decide if you want to load the video
or the depth driver. Would be better to have one driver to do it all.

But in the meantime I would like to ask for ideas about a more clean
solution to get other isoc endpoints.

There was already a little discussion about this when kinect.c was
written by Antonio Ospite:
http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/26194

http://article.gmane.org/gmane.linux.drivers.video-input-infrastructure/26213

Has something changed?
Is there a point against making multiple endpoints available?
Better solution?

I am new to device drivers so I hope you can help me with this.




Regards,

Alexander


btw: Thanks for the gspca framework, I started writing a standalone
driver from scratch but the isoc handling f***** me up a bit.
Sorry for my poor English!

Re: gspca second isoc endpoint / kinect depth

[Hans de Goede]

Hi,

On 04/08/2014 08:13 PM, Alexander Sosna wrote:
> Hi,
> 
> I took drivers/media/usb/gspca/kinect.c as skeleton to build a depth
> driver for the kinect camera.
> 
> I needed to implement this feature because libfreenect performs so badly
> on the raspberry pi that you can't get a single frame.
> 
> The kinecet has two isoc endpoints but gspca only uses the first.
> To get it running I made a dirty hack to drivers/media/usb/gspca/gspca.c
> I changed usb_host_endpoint *alt_xfer(...) so that it always returns the
> second endpoint, which is not really good for everyone.
> 
> 
> My driver is not ready for upstream now, it can not coexist with the
> current gspca_kinect so you have to decide if you want to load the video
> or the depth driver. Would be better to have one driver to do it all.
> 
> But in the meantime I would like to ask for ideas about a more clean
> solution to get other isoc endpoints.
> 
> There was already a little discussion about this when kinect.c was
> written by Antonio Ospite:
> http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/26194
> 
> http://article.gmane.org/gmane.linux.drivers.video-input-infrastructure/26213
> 
> Has something changed?

No.

> Is there a point against making multiple endpoints available?

No.

> Better solution?

Not that I know of.

Regards,

Hans

[RFC 0/2] gspca_kinect: add support for the depth stream

[Antonio Ospite]

Hi,

Alexander needed to get the depth stream out of a Kinect without using
libusb because he experienced problem with that on the RaspberryPi, and
using v4l2 proved to be useful to him (BTW Alexander any actual
benchmark?).

So with the following patches I want to explore how to support the depth
stream in the mainline driver.

The first patch is about supporting data streams on endpoints other than
the first one in gspca.

The second patch adds support for the depth data to the kinect subdriver.

Some more specific comments are annotated per-patch.

Ciao ciao,
   Antonio

Antonio Ospite (2):
  gspca: provide a mechanism to select a specific transfer endpoint
  gspca_kinect: add support for the depth stream

 drivers/media/usb/gspca/gspca.c  | 20 ++++++---
 drivers/media/usb/gspca/gspca.h  |  1 +
 drivers/media/usb/gspca/kinect.c | 97 +++++++++++++++++++++++++++++++++++-----
 3 files changed, 101 insertions(+), 17 deletions(-)

-- 
Antonio Ospite
http://ao2.it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

[RFC 1/2] gspca: provide a mechanism to select a specific transfer endpoint

[Antonio Ospite]

Add a xfer_ep_index field to struct gspca_dev, and change alt_xfer() so
that it accepts a parameter which represents a specific endpoint to look
for.

If a subdriver wants to specify a value for gspca_dev->xfer_ep_index it
can do that in its sd_config() callback.

Signed-off-by: Antonio Ospite <ao2@ao2.it>
---

I am not sure if it is OK to specify an endpoint _index_ or if it would be
better to specify the endpoint address directly (in Kinect 0x81 is for video
data and 0x82 is for depth data).

Hans, any comment on that?

Thanks,
   Antonio

 drivers/media/usb/gspca/gspca.c | 20 ++++++++++++++------
 drivers/media/usb/gspca/gspca.h |  1 +
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/media/usb/gspca/gspca.c b/drivers/media/usb/gspca/gspca.c
index f3a7ace..7e5226c 100644
--- a/drivers/media/usb/gspca/gspca.c
+++ b/drivers/media/usb/gspca/gspca.c
@@ -603,10 +603,13 @@ static void gspca_stream_off(struct gspca_dev *gspca_dev)
 }
 
 /*
- * look for an input transfer endpoint in an alternate setting
+ * look for an input transfer endpoint in an alternate setting.
+ *
+ * If xfer_ep_index is negative, return the first valid one found, otherwise
+ * look for exactly the one in position xfer_ep.
  */
 static struct usb_host_endpoint *alt_xfer(struct usb_host_interface *alt,
-					  int xfer)
+					  int xfer, int xfer_ep_index)
 {
 	struct usb_host_endpoint *ep;
 	int i, attr;
@@ -616,8 +619,10 @@ static struct usb_host_endpoint *alt_xfer(struct usb_host_interface *alt,
 		attr = ep->desc.bmAttributes & USB_ENDPOINT_XFERTYPE_MASK;
 		if (attr == xfer
 		    && ep->desc.wMaxPacketSize != 0
-		    && usb_endpoint_dir_in(&ep->desc))
+		    && usb_endpoint_dir_in(&ep->desc)
+		    && (xfer_ep_index < 0 || i == xfer_ep_index)) {
 			return ep;
+		}
 	}
 	return NULL;
 }
@@ -689,7 +694,7 @@ static int build_isoc_ep_tb(struct gspca_dev *gspca_dev,
 		found = 0;
 		for (j = 0; j < nbalt; j++) {
 			ep = alt_xfer(&intf->altsetting[j],
-				      USB_ENDPOINT_XFER_ISOC);
+				      USB_ENDPOINT_XFER_ISOC, gspca_dev->xfer_ep_index);
 			if (ep == NULL)
 				continue;
 			if (ep->desc.bInterval == 0) {
@@ -862,7 +867,8 @@ static int gspca_init_transfer(struct gspca_dev *gspca_dev)
 	/* if bulk or the subdriver forced an altsetting, get the endpoint */
 	if (gspca_dev->alt != 0) {
 		gspca_dev->alt--;	/* (previous version compatibility) */
-		ep = alt_xfer(&intf->altsetting[gspca_dev->alt], xfer);
+		ep = alt_xfer(&intf->altsetting[gspca_dev->alt], xfer,
+			      gspca_dev->xfer_ep_index);
 		if (ep == NULL) {
 			pr_err("bad altsetting %d\n", gspca_dev->alt);
 			return -EIO;
@@ -904,7 +910,8 @@ static int gspca_init_transfer(struct gspca_dev *gspca_dev)
 		if (!gspca_dev->cam.no_urb_create) {
 			PDEBUG(D_STREAM, "init transfer alt %d", alt);
 			ret = create_urbs(gspca_dev,
-				alt_xfer(&intf->altsetting[alt], xfer));
+				alt_xfer(&intf->altsetting[alt], xfer,
+					 gspca_dev->xfer_ep_index));
 			if (ret < 0) {
 				destroy_urbs(gspca_dev);
 				goto out;
@@ -2030,6 +2037,7 @@ int gspca_dev_probe2(struct usb_interface *intf,
 	}
 	gspca_dev->dev = dev;
 	gspca_dev->iface = intf->cur_altsetting->desc.bInterfaceNumber;
+	gspca_dev->xfer_ep_index = -1;
 
 	/* check if any audio device */
 	if (dev->actconfig->desc.bNumInterfaces != 1) {
diff --git a/drivers/media/usb/gspca/gspca.h b/drivers/media/usb/gspca/gspca.h
index 300642d..92317af 100644
--- a/drivers/media/usb/gspca/gspca.h
+++ b/drivers/media/usb/gspca/gspca.h
@@ -205,6 +205,7 @@ struct gspca_dev {
 	char memory;			/* memory type (V4L2_MEMORY_xxx) */
 	__u8 iface;			/* USB interface number */
 	__u8 alt;			/* USB alternate setting */
+	int xfer_ep_index;		/* index of the USB transfer endpoint */
 	u8 audio;			/* presence of audio device */
 
 	/* (*) These variables are proteced by both usb_lock and queue_lock,
-- 
2.0.0

[RFC 2/2] gspca_kinect: add support for the depth stream

[Antonio Ospite]

Add support for the depth mode at 10bpp, use a command line parameter to
switch mode.

NOTE: this is just a proof-of-concept, the final implementation will
have to expose two v4l2 devices, one for the video stream and one for
the depth stream.

Signed-off-by: Alexander Sosna <alexander@xxor.de>
Signed-off-by: Antonio Ospite <ao2@ao2.it>
---

For now a command line parameter called "depth_mode" is used to select which
mode to activate when loading the driver, this is necessary because gspca is
not quite ready to have a subdriver call gspca_dev_probe() multiple times.

The problem seems to be that gspca assumes one video device per USB
_interface_, and so it stores a pointer to gspca_dev as the interface
private data: see usb_set_intfdata(intf, gspca_dev) in
gspca_dev_probe2().

If anyone feels brave (do a backup first, etc. etc.), hack the sd_probe()
below to register both the devices: you will get the two v4l nodes and both
streams will work OK, but the kernel will halt when you disconnect the device,
i.e. some problem arises in gspca_disconnect() after the usb_get_intfdata(intf)
call.

I am still figuring out the details of the failure sequence, and I'll try to
imagine a way to support the use case "multiple v4l devices on one USB
interface", but this will take some more time.

Thanks,
   Antonio

 drivers/media/usb/gspca/kinect.c | 97 +++++++++++++++++++++++++++++++++++-----
 1 file changed, 86 insertions(+), 11 deletions(-)

diff --git a/drivers/media/usb/gspca/kinect.c b/drivers/media/usb/gspca/kinect.c
index 081f051..ce688d8 100644
--- a/drivers/media/usb/gspca/kinect.c
+++ b/drivers/media/usb/gspca/kinect.c
@@ -36,6 +36,8 @@ MODULE_AUTHOR("Antonio Ospite <ospite@studenti.unina.it>");
 MODULE_DESCRIPTION("GSPCA/Kinect Sensor Device USB Camera Driver");
 MODULE_LICENSE("GPL");
 
+static bool depth_mode;
+
 struct pkt_hdr {
 	uint8_t magic[2];
 	uint8_t pad;
@@ -73,6 +75,14 @@ struct sd {
 
 #define FPS_HIGH       0x0100
 
+static const struct v4l2_pix_format depth_camera_mode[] = {
+	{640, 480, V4L2_PIX_FMT_Y10BPACK, V4L2_FIELD_NONE,
+	 .bytesperline = 640 * 10 / 8,
+	 .sizeimage =  640 * 480 * 10 / 8,
+	 .colorspace = V4L2_COLORSPACE_SRGB,
+	 .priv = MODE_640x488 | FORMAT_Y10B},
+};
+
 static const struct v4l2_pix_format video_camera_mode[] = {
 	{640, 480, V4L2_PIX_FMT_SGRBG8, V4L2_FIELD_NONE,
 	 .bytesperline = 640,
@@ -219,7 +229,7 @@ static int write_register(struct gspca_dev *gspca_dev, uint16_t reg,
 }
 
 /* this function is called at probe time */
-static int sd_config(struct gspca_dev *gspca_dev,
+static int sd_config_video(struct gspca_dev *gspca_dev,
 		     const struct usb_device_id *id)
 {
 	struct sd *sd = (struct sd *) gspca_dev;
@@ -227,8 +237,7 @@ static int sd_config(struct gspca_dev *gspca_dev,
 
 	sd->cam_tag = 0;
 
-	/* Only video stream is supported for now,
-	 * which has stream flag = 0x80 */
+	/* video has stream flag = 0x80 */
 	sd->stream_flag = 0x80;
 
 	cam = &gspca_dev->cam;
@@ -245,6 +254,26 @@ static int sd_config(struct gspca_dev *gspca_dev,
 	return 0;
 }
 
+static int sd_config_depth(struct gspca_dev *gspca_dev,
+		     const struct usb_device_id *id)
+{
+	struct sd *sd = (struct sd *) gspca_dev;
+	struct cam *cam;
+
+	sd->cam_tag = 0;
+
+	cam = &gspca_dev->cam;
+
+	/* depth has stream flag = 0x70 */
+	sd->stream_flag = 0x70;
+	cam->cam_mode = depth_camera_mode;
+	cam->nmodes = ARRAY_SIZE(depth_camera_mode);
+
+	gspca_dev->xfer_ep_index = 1;
+
+	return 0;
+}
+
 /* this function is called at probe and resume time */
 static int sd_init(struct gspca_dev *gspca_dev)
 {
@@ -253,7 +282,7 @@ static int sd_init(struct gspca_dev *gspca_dev)
 	return 0;
 }
 
-static int sd_start(struct gspca_dev *gspca_dev)
+static int sd_start_video(struct gspca_dev *gspca_dev)
 {
 	int mode;
 	uint8_t fmt_reg, fmt_val;
@@ -325,12 +354,39 @@ static int sd_start(struct gspca_dev *gspca_dev)
 	return 0;
 }
 
-static void sd_stopN(struct gspca_dev *gspca_dev)
+static int sd_start_depth(struct gspca_dev *gspca_dev)
+{
+	/* turn off IR-reset function */
+	write_register(gspca_dev, 0x105, 0x00);
+
+	/* reset depth stream */
+	write_register(gspca_dev, 0x06, 0x00);
+	/* Depth Stream Format 0x03: 11 bit stream | 0x02: 10 bit */
+	write_register(gspca_dev, 0x12, 0x02);
+	/* Depth Stream Resolution 1: standard (640x480) */
+	write_register(gspca_dev, 0x13, 0x01);
+	/* Depth Framerate / 0x1e (30): 30 fps */
+	write_register(gspca_dev, 0x14, 0x1e);
+	/* Depth Stream Control  / 2: Open Depth Stream */
+	write_register(gspca_dev, 0x06, 0x02);
+	/* disable depth hflip / LSB = 0: Smoothing Disabled */
+	write_register(gspca_dev, 0x17, 0x00);
+
+	return 0;
+}
+
+static void sd_stopN_video(struct gspca_dev *gspca_dev)
 {
 	/* reset video stream */
 	write_register(gspca_dev, 0x05, 0x00);
 }
 
+static void sd_stopN_depth(struct gspca_dev *gspca_dev)
+{
+	/* reset depth stream */
+	write_register(gspca_dev, 0x06, 0x00);
+}
+
 static void sd_pkt_scan(struct gspca_dev *gspca_dev, u8 *__data, int len)
 {
 	struct sd *sd = (struct sd *) gspca_dev;
@@ -366,12 +422,24 @@ static void sd_pkt_scan(struct gspca_dev *gspca_dev, u8 *__data, int len)
 }
 
 /* sub-driver description */
-static const struct sd_desc sd_desc = {
+static const struct sd_desc sd_desc_video = {
 	.name      = MODULE_NAME,
-	.config    = sd_config,
+	.config    = sd_config_video,
 	.init      = sd_init,
-	.start     = sd_start,
-	.stopN     = sd_stopN,
+	.start     = sd_start_video,
+	.stopN     = sd_stopN_video,
+	.pkt_scan  = sd_pkt_scan,
+	/*
+	.get_streamparm = sd_get_streamparm,
+	.set_streamparm = sd_set_streamparm,
+	*/
+};
+static const struct sd_desc sd_desc_depth = {
+	.name      = MODULE_NAME,
+	.config    = sd_config_depth,
+	.init      = sd_init,
+	.start     = sd_start_depth,
+	.stopN     = sd_stopN_depth,
 	.pkt_scan  = sd_pkt_scan,
 	/*
 	.get_streamparm = sd_get_streamparm,
@@ -391,8 +459,12 @@ MODULE_DEVICE_TABLE(usb, device_table);
 /* -- device connect -- */
 static int sd_probe(struct usb_interface *intf, const struct usb_device_id *id)
 {
-	return gspca_dev_probe(intf, id, &sd_desc, sizeof(struct sd),
-				THIS_MODULE);
+	if (depth_mode)
+		return gspca_dev_probe(intf, id, &sd_desc_depth,
+				       sizeof(struct sd), THIS_MODULE);
+	else
+		return gspca_dev_probe(intf, id, &sd_desc_video,
+				       sizeof(struct sd), THIS_MODULE);
 }
 
 static struct usb_driver sd_driver = {
@@ -408,3 +480,6 @@ static struct usb_driver sd_driver = {
 };
 
 module_usb_driver(sd_driver);
+
+module_param(depth_mode, bool, 0644);
+MODULE_PARM_DESC(depth_mode, "0=rgb 1=depth");
-- 
2.0.0

Re: [RFC 1/2] gspca: provide a mechanism to select a specific transfer endpoint

[Hans de Goede]

Hi Antonio,

Thanks for working on this.

On 06/04/2014 10:24 PM, Antonio Ospite wrote:
> Add a xfer_ep_index field to struct gspca_dev, and change alt_xfer() so
> that it accepts a parameter which represents a specific endpoint to look
> for.
> 
> If a subdriver wants to specify a value for gspca_dev->xfer_ep_index it
> can do that in its sd_config() callback.
> 
> Signed-off-by: Antonio Ospite <ao2@ao2.it>
> ---
> 
> I am not sure if it is OK to specify an endpoint _index_ or if it would be
> better to specify the endpoint address directly (in Kinect 0x81 is for video
> data and 0x82 is for depth data).
>
> Hans, any comment on that?

I think it would be better to use the endpoint address directly for this,
relying on the order in which the endpoints are listed in the descriptor
feels wrong to me.

Other then that this patch looks good.

Regards,

Hans


> 
> Thanks,
>    Antonio
> 
>  drivers/media/usb/gspca/gspca.c | 20 ++++++++++++++------
>  drivers/media/usb/gspca/gspca.h |  1 +
>  2 files changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/usb/gspca/gspca.c b/drivers/media/usb/gspca/gspca.c
> index f3a7ace..7e5226c 100644
> --- a/drivers/media/usb/gspca/gspca.c
> +++ b/drivers/media/usb/gspca/gspca.c
> @@ -603,10 +603,13 @@ static void gspca_stream_off(struct gspca_dev *gspca_dev)
>  }
>  
>  /*
> - * look for an input transfer endpoint in an alternate setting
> + * look for an input transfer endpoint in an alternate setting.
> + *
> + * If xfer_ep_index is negative, return the first valid one found, otherwise
> + * look for exactly the one in position xfer_ep.
>   */
>  static struct usb_host_endpoint *alt_xfer(struct usb_host_interface *alt,
> -					  int xfer)
> +					  int xfer, int xfer_ep_index)
>  {
>  	struct usb_host_endpoint *ep;
>  	int i, attr;
> @@ -616,8 +619,10 @@ static struct usb_host_endpoint *alt_xfer(struct usb_host_interface *alt,
>  		attr = ep->desc.bmAttributes & USB_ENDPOINT_XFERTYPE_MASK;
>  		if (attr == xfer
>  		    && ep->desc.wMaxPacketSize != 0
> -		    && usb_endpoint_dir_in(&ep->desc))
> +		    && usb_endpoint_dir_in(&ep->desc)
> +		    && (xfer_ep_index < 0 || i == xfer_ep_index)) {
>  			return ep;
> +		}
>  	}
>  	return NULL;
>  }
> @@ -689,7 +694,7 @@ static int build_isoc_ep_tb(struct gspca_dev *gspca_dev,
>  		found = 0;
>  		for (j = 0; j < nbalt; j++) {
>  			ep = alt_xfer(&intf->altsetting[j],
> -				      USB_ENDPOINT_XFER_ISOC);
> +				      USB_ENDPOINT_XFER_ISOC, gspca_dev->xfer_ep_index);
>  			if (ep == NULL)
>  				continue;
>  			if (ep->desc.bInterval == 0) {
> @@ -862,7 +867,8 @@ static int gspca_init_transfer(struct gspca_dev *gspca_dev)
>  	/* if bulk or the subdriver forced an altsetting, get the endpoint */
>  	if (gspca_dev->alt != 0) {
>  		gspca_dev->alt--;	/* (previous version compatibility) */
> -		ep = alt_xfer(&intf->altsetting[gspca_dev->alt], xfer);
> +		ep = alt_xfer(&intf->altsetting[gspca_dev->alt], xfer,
> +			      gspca_dev->xfer_ep_index);
>  		if (ep == NULL) {
>  			pr_err("bad altsetting %d\n", gspca_dev->alt);
>  			return -EIO;
> @@ -904,7 +910,8 @@ static int gspca_init_transfer(struct gspca_dev *gspca_dev)
>  		if (!gspca_dev->cam.no_urb_create) {
>  			PDEBUG(D_STREAM, "init transfer alt %d", alt);
>  			ret = create_urbs(gspca_dev,
> -				alt_xfer(&intf->altsetting[alt], xfer));
> +				alt_xfer(&intf->altsetting[alt], xfer,
> +					 gspca_dev->xfer_ep_index));
>  			if (ret < 0) {
>  				destroy_urbs(gspca_dev);
>  				goto out;
> @@ -2030,6 +2037,7 @@ int gspca_dev_probe2(struct usb_interface *intf,
>  	}
>  	gspca_dev->dev = dev;
>  	gspca_dev->iface = intf->cur_altsetting->desc.bInterfaceNumber;
> +	gspca_dev->xfer_ep_index = -1;
>  
>  	/* check if any audio device */
>  	if (dev->actconfig->desc.bNumInterfaces != 1) {
> diff --git a/drivers/media/usb/gspca/gspca.h b/drivers/media/usb/gspca/gspca.h
> index 300642d..92317af 100644
> --- a/drivers/media/usb/gspca/gspca.h
> +++ b/drivers/media/usb/gspca/gspca.h
> @@ -205,6 +205,7 @@ struct gspca_dev {
>  	char memory;			/* memory type (V4L2_MEMORY_xxx) */
>  	__u8 iface;			/* USB interface number */
>  	__u8 alt;			/* USB alternate setting */
> +	int xfer_ep_index;		/* index of the USB transfer endpoint */
>  	u8 audio;			/* presence of audio device */
>  
>  	/* (*) These variables are proteced by both usb_lock and queue_lock,
> 

Re: [RFC 2/2] gspca_kinect: add support for the depth stream

[Hans de Goede]

Hi,

On 06/04/2014 10:24 PM, Antonio Ospite wrote:
> Add support for the depth mode at 10bpp, use a command line parameter to
> switch mode.
> 
> NOTE: this is just a proof-of-concept, the final implementation will
> have to expose two v4l2 devices, one for the video stream and one for
> the depth stream.

Thanks for the patch. If this is useful for some people I'm willing to
merge this until we've a better fix.

> Signed-off-by: Alexander Sosna <alexander@xxor.de>
> Signed-off-by: Antonio Ospite <ao2@ao2.it>
> ---
> 
> For now a command line parameter called "depth_mode" is used to select which
> mode to activate when loading the driver, this is necessary because gspca is
> not quite ready to have a subdriver call gspca_dev_probe() multiple times.
> 
> The problem seems to be that gspca assumes one video device per USB
> _interface_, and so it stores a pointer to gspca_dev as the interface
> private data: see usb_set_intfdata(intf, gspca_dev) in
> gspca_dev_probe2().
> 
> If anyone feels brave (do a backup first, etc. etc.), hack the sd_probe()
> below to register both the devices: you will get the two v4l nodes and both
> streams will work OK, but the kernel will halt when you disconnect the device,
> i.e. some problem arises in gspca_disconnect() after the usb_get_intfdata(intf)
> call.
> 
> I am still figuring out the details of the failure sequence, and I'll try to
> imagine a way to support the use case "multiple v4l devices on one USB
> interface", but this will take some more time.

I believe that support 2 devices would require separating the per video node /
stream data and global data into separate structs, and then refactoring everything
so that we can have 2 streams on one gspca_dev. If you do this please make it
a patch-set with many small patches, rather then 1 or 2 very large patches.

And then in things like disconnect, loop over the streams and stop both, unregister
both nodes, etc.

If you ever decide to add support for controls you will also need to think about what
to do with those, but for now I guess you can just register all the controls on the
first video-node/stream (which will be the only one for all devices except kinect
devices, and the kinect code currently does not have controls.

Regards,

Hans



> 
> Thanks,
>    Antonio
> 
>  drivers/media/usb/gspca/kinect.c | 97 +++++++++++++++++++++++++++++++++++-----
>  1 file changed, 86 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/media/usb/gspca/kinect.c b/drivers/media/usb/gspca/kinect.c
> index 081f051..ce688d8 100644
> --- a/drivers/media/usb/gspca/kinect.c
> +++ b/drivers/media/usb/gspca/kinect.c
> @@ -36,6 +36,8 @@ MODULE_AUTHOR("Antonio Ospite <ospite@studenti.unina.it>");
>  MODULE_DESCRIPTION("GSPCA/Kinect Sensor Device USB Camera Driver");
>  MODULE_LICENSE("GPL");
>  
> +static bool depth_mode;
> +
>  struct pkt_hdr {
>  	uint8_t magic[2];
>  	uint8_t pad;
> @@ -73,6 +75,14 @@ struct sd {
>  
>  #define FPS_HIGH       0x0100
>  
> +static const struct v4l2_pix_format depth_camera_mode[] = {
> +	{640, 480, V4L2_PIX_FMT_Y10BPACK, V4L2_FIELD_NONE,
> +	 .bytesperline = 640 * 10 / 8,
> +	 .sizeimage =  640 * 480 * 10 / 8,
> +	 .colorspace = V4L2_COLORSPACE_SRGB,
> +	 .priv = MODE_640x488 | FORMAT_Y10B},
> +};
> +
>  static const struct v4l2_pix_format video_camera_mode[] = {
>  	{640, 480, V4L2_PIX_FMT_SGRBG8, V4L2_FIELD_NONE,
>  	 .bytesperline = 640,
> @@ -219,7 +229,7 @@ static int write_register(struct gspca_dev *gspca_dev, uint16_t reg,
>  }
>  
>  /* this function is called at probe time */
> -static int sd_config(struct gspca_dev *gspca_dev,
> +static int sd_config_video(struct gspca_dev *gspca_dev,
>  		     const struct usb_device_id *id)
>  {
>  	struct sd *sd = (struct sd *) gspca_dev;
> @@ -227,8 +237,7 @@ static int sd_config(struct gspca_dev *gspca_dev,
>  
>  	sd->cam_tag = 0;
>  
> -	/* Only video stream is supported for now,
> -	 * which has stream flag = 0x80 */
> +	/* video has stream flag = 0x80 */
>  	sd->stream_flag = 0x80;
>  
>  	cam = &gspca_dev->cam;
> @@ -245,6 +254,26 @@ static int sd_config(struct gspca_dev *gspca_dev,
>  	return 0;
>  }
>  
> +static int sd_config_depth(struct gspca_dev *gspca_dev,
> +		     const struct usb_device_id *id)
> +{
> +	struct sd *sd = (struct sd *) gspca_dev;
> +	struct cam *cam;
> +
> +	sd->cam_tag = 0;
> +
> +	cam = &gspca_dev->cam;
> +
> +	/* depth has stream flag = 0x70 */
> +	sd->stream_flag = 0x70;
> +	cam->cam_mode = depth_camera_mode;
> +	cam->nmodes = ARRAY_SIZE(depth_camera_mode);
> +
> +	gspca_dev->xfer_ep_index = 1;
> +
> +	return 0;
> +}
> +
>  /* this function is called at probe and resume time */
>  static int sd_init(struct gspca_dev *gspca_dev)
>  {
> @@ -253,7 +282,7 @@ static int sd_init(struct gspca_dev *gspca_dev)
>  	return 0;
>  }
>  
> -static int sd_start(struct gspca_dev *gspca_dev)
> +static int sd_start_video(struct gspca_dev *gspca_dev)
>  {
>  	int mode;
>  	uint8_t fmt_reg, fmt_val;
> @@ -325,12 +354,39 @@ static int sd_start(struct gspca_dev *gspca_dev)
>  	return 0;
>  }
>  
> -static void sd_stopN(struct gspca_dev *gspca_dev)
> +static int sd_start_depth(struct gspca_dev *gspca_dev)
> +{
> +	/* turn off IR-reset function */
> +	write_register(gspca_dev, 0x105, 0x00);
> +
> +	/* reset depth stream */
> +	write_register(gspca_dev, 0x06, 0x00);
> +	/* Depth Stream Format 0x03: 11 bit stream | 0x02: 10 bit */
> +	write_register(gspca_dev, 0x12, 0x02);
> +	/* Depth Stream Resolution 1: standard (640x480) */
> +	write_register(gspca_dev, 0x13, 0x01);
> +	/* Depth Framerate / 0x1e (30): 30 fps */
> +	write_register(gspca_dev, 0x14, 0x1e);
> +	/* Depth Stream Control  / 2: Open Depth Stream */
> +	write_register(gspca_dev, 0x06, 0x02);
> +	/* disable depth hflip / LSB = 0: Smoothing Disabled */
> +	write_register(gspca_dev, 0x17, 0x00);
> +
> +	return 0;
> +}
> +
> +static void sd_stopN_video(struct gspca_dev *gspca_dev)
>  {
>  	/* reset video stream */
>  	write_register(gspca_dev, 0x05, 0x00);
>  }
>  
> +static void sd_stopN_depth(struct gspca_dev *gspca_dev)
> +{
> +	/* reset depth stream */
> +	write_register(gspca_dev, 0x06, 0x00);
> +}
> +
>  static void sd_pkt_scan(struct gspca_dev *gspca_dev, u8 *__data, int len)
>  {
>  	struct sd *sd = (struct sd *) gspca_dev;
> @@ -366,12 +422,24 @@ static void sd_pkt_scan(struct gspca_dev *gspca_dev, u8 *__data, int len)
>  }
>  
>  /* sub-driver description */
> -static const struct sd_desc sd_desc = {
> +static const struct sd_desc sd_desc_video = {
>  	.name      = MODULE_NAME,
> -	.config    = sd_config,
> +	.config    = sd_config_video,
>  	.init      = sd_init,
> -	.start     = sd_start,
> -	.stopN     = sd_stopN,
> +	.start     = sd_start_video,
> +	.stopN     = sd_stopN_video,
> +	.pkt_scan  = sd_pkt_scan,
> +	/*
> +	.get_streamparm = sd_get_streamparm,
> +	.set_streamparm = sd_set_streamparm,
> +	*/
> +};
> +static const struct sd_desc sd_desc_depth = {
> +	.name      = MODULE_NAME,
> +	.config    = sd_config_depth,
> +	.init      = sd_init,
> +	.start     = sd_start_depth,
> +	.stopN     = sd_stopN_depth,
>  	.pkt_scan  = sd_pkt_scan,
>  	/*
>  	.get_streamparm = sd_get_streamparm,
> @@ -391,8 +459,12 @@ MODULE_DEVICE_TABLE(usb, device_table);
>  /* -- device connect -- */
>  static int sd_probe(struct usb_interface *intf, const struct usb_device_id *id)
>  {
> -	return gspca_dev_probe(intf, id, &sd_desc, sizeof(struct sd),
> -				THIS_MODULE);
> +	if (depth_mode)
> +		return gspca_dev_probe(intf, id, &sd_desc_depth,
> +				       sizeof(struct sd), THIS_MODULE);
> +	else
> +		return gspca_dev_probe(intf, id, &sd_desc_video,
> +				       sizeof(struct sd), THIS_MODULE);
>  }
>  
>  static struct usb_driver sd_driver = {
> @@ -408,3 +480,6 @@ static struct usb_driver sd_driver = {
>  };
>  
>  module_usb_driver(sd_driver);
> +
> +module_param(depth_mode, bool, 0644);
> +MODULE_PARM_DESC(depth_mode, "0=rgb 1=depth");
> 

Re: [RFC 1/2] gspca: provide a mechanism to select a specific transfer endpoint

[Antonio Ospite]

On Thu, 19 Jun 2014 16:27:59 +0200
Hans de Goede <hdegoede@redhat.com> wrote:

> Hi Antonio,
> 
> Thanks for working on this.
> 
> On 06/04/2014 10:24 PM, Antonio Ospite wrote:
> > Add a xfer_ep_index field to struct gspca_dev, and change alt_xfer() so
> > that it accepts a parameter which represents a specific endpoint to look
> > for.
> > 
> > If a subdriver wants to specify a value for gspca_dev->xfer_ep_index it
> > can do that in its sd_config() callback.
> > 
> > Signed-off-by: Antonio Ospite <ao2@ao2.it>
> > ---
> > 
> > I am not sure if it is OK to specify an endpoint _index_ or if it would be
> > better to specify the endpoint address directly (in Kinect 0x81 is for video
> > data and 0x82 is for depth data).
> >
> > Hans, any comment on that?
> 
> I think it would be better to use the endpoint address directly for this,
> relying on the order in which the endpoints are listed in the descriptor
> feels wrong to me.
>

I see.

If I declare the new field as __u8 (same type of a bEndpointAddress), I
could mark an invalid ep address with ~(USB_ENDPOINT_DIR_MASK | 
USB_ENDPOINT_NUMBER_MASK) in gspca_dev_probe2(), instead of using an
int set to -1; how does that sound?

> Other then that this patch looks good.
> 
> Regards,
> 
> Hans
> 
> 
> > 
> > Thanks,
> >    Antonio
> > 
> >  drivers/media/usb/gspca/gspca.c | 20 ++++++++++++++------
> >  drivers/media/usb/gspca/gspca.h |  1 +
> >  2 files changed, 15 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/media/usb/gspca/gspca.c b/drivers/media/usb/gspca/gspca.c
> > index f3a7ace..7e5226c 100644
> > --- a/drivers/media/usb/gspca/gspca.c
> > +++ b/drivers/media/usb/gspca/gspca.c
> > @@ -603,10 +603,13 @@ static void gspca_stream_off(struct gspca_dev *gspca_dev)
> >  }
> >  
> >  /*
> > - * look for an input transfer endpoint in an alternate setting
> > + * look for an input transfer endpoint in an alternate setting.
> > + *
> > + * If xfer_ep_index is negative, return the first valid one found, otherwise
> > + * look for exactly the one in position xfer_ep.
> >   */
> >  static struct usb_host_endpoint *alt_xfer(struct usb_host_interface *alt,
> > -					  int xfer)
> > +					  int xfer, int xfer_ep_index)
> >  {
> >  	struct usb_host_endpoint *ep;
> >  	int i, attr;
> > @@ -616,8 +619,10 @@ static struct usb_host_endpoint *alt_xfer(struct usb_host_interface *alt,
> >  		attr = ep->desc.bmAttributes & USB_ENDPOINT_XFERTYPE_MASK;
> >  		if (attr == xfer
> >  		    && ep->desc.wMaxPacketSize != 0
> > -		    && usb_endpoint_dir_in(&ep->desc))
> > +		    && usb_endpoint_dir_in(&ep->desc)
> > +		    && (xfer_ep_index < 0 || i == xfer_ep_index)) {
> >  			return ep;
> > +		}
> >  	}
> >  	return NULL;
> >  }
> > @@ -689,7 +694,7 @@ static int build_isoc_ep_tb(struct gspca_dev *gspca_dev,
> >  		found = 0;
> >  		for (j = 0; j < nbalt; j++) {
> >  			ep = alt_xfer(&intf->altsetting[j],
> > -				      USB_ENDPOINT_XFER_ISOC);
> > +				      USB_ENDPOINT_XFER_ISOC, gspca_dev->xfer_ep_index);
> >  			if (ep == NULL)
> >  				continue;
> >  			if (ep->desc.bInterval == 0) {
> > @@ -862,7 +867,8 @@ static int gspca_init_transfer(struct gspca_dev *gspca_dev)
> >  	/* if bulk or the subdriver forced an altsetting, get the endpoint */
> >  	if (gspca_dev->alt != 0) {
> >  		gspca_dev->alt--;	/* (previous version compatibility) */
> > -		ep = alt_xfer(&intf->altsetting[gspca_dev->alt], xfer);
> > +		ep = alt_xfer(&intf->altsetting[gspca_dev->alt], xfer,
> > +			      gspca_dev->xfer_ep_index);
> >  		if (ep == NULL) {
> >  			pr_err("bad altsetting %d\n", gspca_dev->alt);
> >  			return -EIO;
> > @@ -904,7 +910,8 @@ static int gspca_init_transfer(struct gspca_dev *gspca_dev)
> >  		if (!gspca_dev->cam.no_urb_create) {
> >  			PDEBUG(D_STREAM, "init transfer alt %d", alt);
> >  			ret = create_urbs(gspca_dev,
> > -				alt_xfer(&intf->altsetting[alt], xfer));
> > +				alt_xfer(&intf->altsetting[alt], xfer,
> > +					 gspca_dev->xfer_ep_index));
> >  			if (ret < 0) {
> >  				destroy_urbs(gspca_dev);
> >  				goto out;
> > @@ -2030,6 +2037,7 @@ int gspca_dev_probe2(struct usb_interface *intf,
> >  	}
> >  	gspca_dev->dev = dev;
> >  	gspca_dev->iface = intf->cur_altsetting->desc.bInterfaceNumber;
> > +	gspca_dev->xfer_ep_index = -1;
> >  
> >  	/* check if any audio device */
> >  	if (dev->actconfig->desc.bNumInterfaces != 1) {
> > diff --git a/drivers/media/usb/gspca/gspca.h b/drivers/media/usb/gspca/gspca.h
> > index 300642d..92317af 100644
> > --- a/drivers/media/usb/gspca/gspca.h
> > +++ b/drivers/media/usb/gspca/gspca.h
> > @@ -205,6 +205,7 @@ struct gspca_dev {
> >  	char memory;			/* memory type (V4L2_MEMORY_xxx) */
> >  	__u8 iface;			/* USB interface number */
> >  	__u8 alt;			/* USB alternate setting */
> > +	int xfer_ep_index;		/* index of the USB transfer endpoint */
> >  	u8 audio;			/* presence of audio device */
> >  
> >  	/* (*) These variables are proteced by both usb_lock and queue_lock,
> > 


-- 
Antonio Ospite
http://ao2.it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

Re: [RFC 1/2] gspca: provide a mechanism to select a specific transfer endpoint

[Hans de Goede]

Hi,

On 06/24/2014 03:35 PM, Antonio Ospite wrote:
> On Thu, 19 Jun 2014 16:27:59 +0200
> Hans de Goede <hdegoede@redhat.com> wrote:
> 
>> Hi Antonio,
>>
>> Thanks for working on this.
>>
>> On 06/04/2014 10:24 PM, Antonio Ospite wrote:
>>> Add a xfer_ep_index field to struct gspca_dev, and change alt_xfer() so
>>> that it accepts a parameter which represents a specific endpoint to look
>>> for.
>>>
>>> If a subdriver wants to specify a value for gspca_dev->xfer_ep_index it
>>> can do that in its sd_config() callback.
>>>
>>> Signed-off-by: Antonio Ospite <ao2@ao2.it>
>>> ---
>>>
>>> I am not sure if it is OK to specify an endpoint _index_ or if it would be
>>> better to specify the endpoint address directly (in Kinect 0x81 is for video
>>> data and 0x82 is for depth data).
>>>
>>> Hans, any comment on that?
>>
>> I think it would be better to use the endpoint address directly for this,
>> relying on the order in which the endpoints are listed in the descriptor
>> feels wrong to me.
>>
> 
> I see.
> 
> If I declare the new field as __u8 (same type of a bEndpointAddress), I
> could mark an invalid ep address with ~(USB_ENDPOINT_DIR_MASK | 
> USB_ENDPOINT_NUMBER_MASK) in gspca_dev_probe2(), instead of using an
> int set to -1; how does that sound?

I would prefer an int with a simple -1 value of invalid.

Regards,

Hans

Re: [RFC 2/2] gspca_kinect: add support for the depth stream

[Antonio Ospite]

On Thu, 19 Jun 2014 16:35:17 +0200
Hans de Goede <hdegoede@redhat.com> wrote:

> Hi,
> 
> On 06/04/2014 10:24 PM, Antonio Ospite wrote:
> > Add support for the depth mode at 10bpp, use a command line parameter to
> > switch mode.
> > 
> > NOTE: this is just a proof-of-concept, the final implementation will
> > have to expose two v4l2 devices, one for the video stream and one for
> > the depth stream.
> 
> Thanks for the patch. If this is useful for some people I'm willing to
> merge this until we've a better fix.
>

I guess adding a command line parameter (depth_mode) and then removing
it in the future, when a far better alternative is available, is
acceptable in this case; we also did something like that before for the
PS3 Eye driver for instance.

So I am going to submit this patch set for inclusion.

> > Signed-off-by: Alexander Sosna <alexander@xxor.de>
> > Signed-off-by: Antonio Ospite <ao2@ao2.it>
> > ---
> > 
> > For now a command line parameter called "depth_mode" is used to select which
> > mode to activate when loading the driver, this is necessary because gspca is
> > not quite ready to have a subdriver call gspca_dev_probe() multiple times.
> > 
> > The problem seems to be that gspca assumes one video device per USB
> > _interface_, and so it stores a pointer to gspca_dev as the interface
> > private data: see usb_set_intfdata(intf, gspca_dev) in
> > gspca_dev_probe2().
> > 
> > If anyone feels brave (do a backup first, etc. etc.), hack the sd_probe()
> > below to register both the devices: you will get the two v4l nodes and both
> > streams will work OK, but the kernel will halt when you disconnect the device,
> > i.e. some problem arises in gspca_disconnect() after the usb_get_intfdata(intf)
> > call.
> > 
> > I am still figuring out the details of the failure sequence, and I'll try to
> > imagine a way to support the use case "multiple v4l devices on one USB
> > interface", but this will take some more time.
> 
> I believe that support 2 devices would require separating the per video node /
> stream data and global data into separate structs, and then refactoring everything
> so that we can have 2 streams on one gspca_dev. If you do this please make it
> a patch-set with many small patches, rather then 1 or 2 very large patches.
>
> And then in things like disconnect, loop over the streams and stop both, unregister
> both nodes, etc.
> 
> If you ever decide to add support for controls you will also need to think about what
> to do with those, but for now I guess you can just register all the controls on the
> first video-node/stream (which will be the only one for all devices except kinect
> devices, and the kinect code currently does not have controls.
>

Very useful hints, as always.

Thanks,
   Antonio

-- 
Antonio Ospite
http://ao2.it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

[PATCH 0/2] gspca, gspca_kinect: add support for the depth stream

[Antonio Ospite]

Hi,

here are the patches to make gspca able to deal with the Kinect depth
stream at 10bpp.

If anyone is really interested in the 11bpp data too, ping me.

Alexander, please let us know if you can test these anytime soon.

Thanks,
   Antonio

Antonio Ospite (2):
  gspca: provide a mechanism to select a specific transfer endpoint
  gspca_kinect: add support for the depth stream

 drivers/media/usb/gspca/gspca.c  | 20 +++++---
 drivers/media/usb/gspca/gspca.h  |  1 +
 drivers/media/usb/gspca/kinect.c | 98 +++++++++++++++++++++++++++++++++++-----
 3 files changed, 102 insertions(+), 17 deletions(-)

-- 
Antonio Ospite
http://ao2.it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

[PATCH 1/2] gspca: provide a mechanism to select a specific transfer endpoint

[Antonio Ospite]

Currently gspca selects the first ISOC input endpoint as the input
transfer endpoint, however some devices can provide streams on endpoints
different then the first one, so some subdrivers (e.g. gspca_kinect) may
want to select a specific endpoint to use as a transfer endpoint.

Add an xfer_ep field to struct gspca_dev, and change alt_xfer() so that
it accepts a parameter which represents a specific endpoint address to
look for.

If a subdriver wants to specify a value for gspca_dev->xfer_ep it can do
that in its sd_config() callback.

Signed-off-by: Antonio Ospite <ao2@ao2.it>
---
 drivers/media/usb/gspca/gspca.c | 20 ++++++++++++++------
 drivers/media/usb/gspca/gspca.h |  1 +
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/media/usb/gspca/gspca.c b/drivers/media/usb/gspca/gspca.c
index f3a7ace..f9a75ad 100644
--- a/drivers/media/usb/gspca/gspca.c
+++ b/drivers/media/usb/gspca/gspca.c
@@ -603,10 +603,13 @@ static void gspca_stream_off(struct gspca_dev *gspca_dev)
 }
 
 /*
- * look for an input transfer endpoint in an alternate setting
+ * look for an input transfer endpoint in an alternate setting.
+ *
+ * If xfer_ep is invalid, return the first valid ep found, otherwise
+ * look for exactly the ep with address equal to xfer_ep.
  */
 static struct usb_host_endpoint *alt_xfer(struct usb_host_interface *alt,
-					  int xfer)
+					  int xfer, int xfer_ep)
 {
 	struct usb_host_endpoint *ep;
 	int i, attr;
@@ -616,7 +619,8 @@ static struct usb_host_endpoint *alt_xfer(struct usb_host_interface *alt,
 		attr = ep->desc.bmAttributes & USB_ENDPOINT_XFERTYPE_MASK;
 		if (attr == xfer
 		    && ep->desc.wMaxPacketSize != 0
-		    && usb_endpoint_dir_in(&ep->desc))
+		    && usb_endpoint_dir_in(&ep->desc)
+		    && (xfer_ep < 0 || ep->desc.bEndpointAddress == xfer_ep))
 			return ep;
 	}
 	return NULL;
@@ -689,7 +693,8 @@ static int build_isoc_ep_tb(struct gspca_dev *gspca_dev,
 		found = 0;
 		for (j = 0; j < nbalt; j++) {
 			ep = alt_xfer(&intf->altsetting[j],
-				      USB_ENDPOINT_XFER_ISOC);
+				      USB_ENDPOINT_XFER_ISOC,
+				      gspca_dev->xfer_ep);
 			if (ep == NULL)
 				continue;
 			if (ep->desc.bInterval == 0) {
@@ -862,7 +867,8 @@ static int gspca_init_transfer(struct gspca_dev *gspca_dev)
 	/* if bulk or the subdriver forced an altsetting, get the endpoint */
 	if (gspca_dev->alt != 0) {
 		gspca_dev->alt--;	/* (previous version compatibility) */
-		ep = alt_xfer(&intf->altsetting[gspca_dev->alt], xfer);
+		ep = alt_xfer(&intf->altsetting[gspca_dev->alt], xfer,
+			      gspca_dev->xfer_ep);
 		if (ep == NULL) {
 			pr_err("bad altsetting %d\n", gspca_dev->alt);
 			return -EIO;
@@ -904,7 +910,8 @@ static int gspca_init_transfer(struct gspca_dev *gspca_dev)
 		if (!gspca_dev->cam.no_urb_create) {
 			PDEBUG(D_STREAM, "init transfer alt %d", alt);
 			ret = create_urbs(gspca_dev,
-				alt_xfer(&intf->altsetting[alt], xfer));
+				alt_xfer(&intf->altsetting[alt], xfer,
+					 gspca_dev->xfer_ep));
 			if (ret < 0) {
 				destroy_urbs(gspca_dev);
 				goto out;
@@ -2030,6 +2037,7 @@ int gspca_dev_probe2(struct usb_interface *intf,
 	}
 	gspca_dev->dev = dev;
 	gspca_dev->iface = intf->cur_altsetting->desc.bInterfaceNumber;
+	gspca_dev->xfer_ep = -1;
 
 	/* check if any audio device */
 	if (dev->actconfig->desc.bNumInterfaces != 1) {
diff --git a/drivers/media/usb/gspca/gspca.h b/drivers/media/usb/gspca/gspca.h
index 300642d..f06253c 100644
--- a/drivers/media/usb/gspca/gspca.h
+++ b/drivers/media/usb/gspca/gspca.h
@@ -205,6 +205,7 @@ struct gspca_dev {
 	char memory;			/* memory type (V4L2_MEMORY_xxx) */
 	__u8 iface;			/* USB interface number */
 	__u8 alt;			/* USB alternate setting */
+	int xfer_ep;			/* USB transfer endpoint address */
 	u8 audio;			/* presence of audio device */
 
 	/* (*) These variables are proteced by both usb_lock and queue_lock,
-- 
2.0.0

[PATCH 2/2] gspca_kinect: add support for the depth stream

[Antonio Ospite]

Add support for the depth stream at 10bpp, for now use a 'depth_mode'
command line parameter to switch between video and depth mode.

Signed-off-by: Alexander Sosna <alexander@xxor.de>
Signed-off-by: Antonio Ospite <ao2@ao2.it>
---
 drivers/media/usb/gspca/kinect.c | 98 +++++++++++++++++++++++++++++++++++-----
 1 file changed, 87 insertions(+), 11 deletions(-)

diff --git a/drivers/media/usb/gspca/kinect.c b/drivers/media/usb/gspca/kinect.c
index 081f051..45bc1f5 100644
--- a/drivers/media/usb/gspca/kinect.c
+++ b/drivers/media/usb/gspca/kinect.c
@@ -36,6 +36,8 @@ MODULE_AUTHOR("Antonio Ospite <ospite@studenti.unina.it>");
 MODULE_DESCRIPTION("GSPCA/Kinect Sensor Device USB Camera Driver");
 MODULE_LICENSE("GPL");
 
+static bool depth_mode;
+
 struct pkt_hdr {
 	uint8_t magic[2];
 	uint8_t pad;
@@ -73,6 +75,14 @@ struct sd {
 
 #define FPS_HIGH       0x0100
 
+static const struct v4l2_pix_format depth_camera_mode[] = {
+	{640, 480, V4L2_PIX_FMT_Y10BPACK, V4L2_FIELD_NONE,
+	 .bytesperline = 640 * 10 / 8,
+	 .sizeimage =  640 * 480 * 10 / 8,
+	 .colorspace = V4L2_COLORSPACE_SRGB,
+	 .priv = MODE_640x488 | FORMAT_Y10B},
+};
+
 static const struct v4l2_pix_format video_camera_mode[] = {
 	{640, 480, V4L2_PIX_FMT_SGRBG8, V4L2_FIELD_NONE,
 	 .bytesperline = 640,
@@ -219,7 +229,7 @@ static int write_register(struct gspca_dev *gspca_dev, uint16_t reg,
 }
 
 /* this function is called at probe time */
-static int sd_config(struct gspca_dev *gspca_dev,
+static int sd_config_video(struct gspca_dev *gspca_dev,
 		     const struct usb_device_id *id)
 {
 	struct sd *sd = (struct sd *) gspca_dev;
@@ -227,8 +237,6 @@ static int sd_config(struct gspca_dev *gspca_dev,
 
 	sd->cam_tag = 0;
 
-	/* Only video stream is supported for now,
-	 * which has stream flag = 0x80 */
 	sd->stream_flag = 0x80;
 
 	cam = &gspca_dev->cam;
@@ -236,6 +244,8 @@ static int sd_config(struct gspca_dev *gspca_dev,
 	cam->cam_mode = video_camera_mode;
 	cam->nmodes = ARRAY_SIZE(video_camera_mode);
 
+	gspca_dev->xfer_ep = 0x81;
+
 #if 0
 	/* Setting those values is not needed for video stream */
 	cam->npkt = 15;
@@ -245,6 +255,26 @@ static int sd_config(struct gspca_dev *gspca_dev,
 	return 0;
 }
 
+static int sd_config_depth(struct gspca_dev *gspca_dev,
+		     const struct usb_device_id *id)
+{
+	struct sd *sd = (struct sd *) gspca_dev;
+	struct cam *cam;
+
+	sd->cam_tag = 0;
+
+	sd->stream_flag = 0x70;
+
+	cam = &gspca_dev->cam;
+
+	cam->cam_mode = depth_camera_mode;
+	cam->nmodes = ARRAY_SIZE(depth_camera_mode);
+
+	gspca_dev->xfer_ep = 0x82;
+
+	return 0;
+}
+
 /* this function is called at probe and resume time */
 static int sd_init(struct gspca_dev *gspca_dev)
 {
@@ -253,7 +283,7 @@ static int sd_init(struct gspca_dev *gspca_dev)
 	return 0;
 }
 
-static int sd_start(struct gspca_dev *gspca_dev)
+static int sd_start_video(struct gspca_dev *gspca_dev)
 {
 	int mode;
 	uint8_t fmt_reg, fmt_val;
@@ -325,12 +355,39 @@ static int sd_start(struct gspca_dev *gspca_dev)
 	return 0;
 }
 
-static void sd_stopN(struct gspca_dev *gspca_dev)
+static int sd_start_depth(struct gspca_dev *gspca_dev)
+{
+	/* turn off IR-reset function */
+	write_register(gspca_dev, 0x105, 0x00);
+
+	/* reset depth stream */
+	write_register(gspca_dev, 0x06, 0x00);
+	/* Depth Stream Format 0x03: 11 bit stream | 0x02: 10 bit */
+	write_register(gspca_dev, 0x12, 0x02);
+	/* Depth Stream Resolution 1: standard (640x480) */
+	write_register(gspca_dev, 0x13, 0x01);
+	/* Depth Framerate / 0x1e (30): 30 fps */
+	write_register(gspca_dev, 0x14, 0x1e);
+	/* Depth Stream Control  / 2: Open Depth Stream */
+	write_register(gspca_dev, 0x06, 0x02);
+	/* disable depth hflip / LSB = 0: Smoothing Disabled */
+	write_register(gspca_dev, 0x17, 0x00);
+
+	return 0;
+}
+
+static void sd_stopN_video(struct gspca_dev *gspca_dev)
 {
 	/* reset video stream */
 	write_register(gspca_dev, 0x05, 0x00);
 }
 
+static void sd_stopN_depth(struct gspca_dev *gspca_dev)
+{
+	/* reset depth stream */
+	write_register(gspca_dev, 0x06, 0x00);
+}
+
 static void sd_pkt_scan(struct gspca_dev *gspca_dev, u8 *__data, int len)
 {
 	struct sd *sd = (struct sd *) gspca_dev;
@@ -366,12 +423,24 @@ static void sd_pkt_scan(struct gspca_dev *gspca_dev, u8 *__data, int len)
 }
 
 /* sub-driver description */
-static const struct sd_desc sd_desc = {
+static const struct sd_desc sd_desc_video = {
 	.name      = MODULE_NAME,
-	.config    = sd_config,
+	.config    = sd_config_video,
 	.init      = sd_init,
-	.start     = sd_start,
-	.stopN     = sd_stopN,
+	.start     = sd_start_video,
+	.stopN     = sd_stopN_video,
+	.pkt_scan  = sd_pkt_scan,
+	/*
+	.get_streamparm = sd_get_streamparm,
+	.set_streamparm = sd_set_streamparm,
+	*/
+};
+static const struct sd_desc sd_desc_depth = {
+	.name      = MODULE_NAME,
+	.config    = sd_config_depth,
+	.init      = sd_init,
+	.start     = sd_start_depth,
+	.stopN     = sd_stopN_depth,
 	.pkt_scan  = sd_pkt_scan,
 	/*
 	.get_streamparm = sd_get_streamparm,
@@ -391,8 +460,12 @@ MODULE_DEVICE_TABLE(usb, device_table);
 /* -- device connect -- */
 static int sd_probe(struct usb_interface *intf, const struct usb_device_id *id)
 {
-	return gspca_dev_probe(intf, id, &sd_desc, sizeof(struct sd),
-				THIS_MODULE);
+	if (depth_mode)
+		return gspca_dev_probe(intf, id, &sd_desc_depth,
+				       sizeof(struct sd), THIS_MODULE);
+	else
+		return gspca_dev_probe(intf, id, &sd_desc_video,
+				       sizeof(struct sd), THIS_MODULE);
 }
 
 static struct usb_driver sd_driver = {
@@ -408,3 +481,6 @@ static struct usb_driver sd_driver = {
 };
 
 module_usb_driver(sd_driver);
+
+module_param(depth_mode, bool, 0644);
+MODULE_PARM_DESC(depth_mode, "0=video 1=depth");
-- 
2.0.0