Re: Syntek webcams and out-of-tree driver

2013-08-07 Thread Ondrej Zary
On Tuesday 06 August 2013 08:10:38 Hans de Goede wrote:
> Hi,
>
> On 08/05/2013 11:19 PM, Ondrej Zary wrote:
> > Hello,
> > the in-kernel stkwebcam driver (by Jaime Velasco Juan and Nicolas VIVIEN)
> > supports only two webcam types (USB IDs 0x174f:0xa311 and 0x05e1:0x0501).
> > There are many other Syntek webcam types that are not supported by this
> > driver (such as 0x174f:0x6a31 in Asus F5RL laptop).
> >
> > There is an out-of-tree GPL driver called stk11xx (by Martin Roos and
> > also Nicolas VIVIEN) at http://sourceforge.net/projects/syntekdriver/
> > which supports more webcams. It can be even compiled for the latest
> > kernels using the patch below and seems to work somehow (slow and buggy
> > but better than nothing) with the Asus F5RL.
>
> I took a quick look and there are a number of issues with this driver:
>
> 1) It conflicts usb-id wise with the new stk1160 driver (which supports
> usb-id 05e1:0408) so support for that usb-id, and any code only used for
> that id will need to be removed
>
> 2) "seems to work somehow (slow and buggy)" is not really the quality
> we aim for with in kernel drivers. We definitely will want to remove
> any usb-ids, and any code only used for those ids, where there is overlap
> with the existing stkwebcam driver, to avoid regressions
>
> 3) It does in kernel bayer decoding, this is not acceptable, it needs to
> be modified to produce buffers with raw bayer data (libv4l will take care
> of the bater decoding in userspace).
>
> 4) It is not using any of the new kernel infrastructure we have been adding
> over time, like the control-framework, videobuf2, etc. It would be best
> to convert this to a gspca sub driver (of which there are many already,
> which can serve as examples), so that it will use all the existing
> framework code.

Here's a proof of concept that this can be done. HW dependent code copied from
stk11xx and modified a bit. Userspace bayer decoding works fine - I get
correct image in cheese (or mplayer with LD_PRELOAD)!

I've also found the STK1135 datasheet!
http://wenku.baidu.com/view/028c3459be23482fb4da4c5d
https://anonfiles.com/file/3b813f8e4c848ed26aaec804e0afa092

So I can clean up the driver now.

---
 drivers/media/usb/gspca/Kconfig   |9 +
 drivers/media/usb/gspca/Makefile  |2 +
 drivers/media/usb/gspca/stk1135.c |  968 +
 3 files changed, 979 insertions(+), 0 deletions(-)
 create mode 100644 drivers/media/usb/gspca/stk1135.c

diff --git a/drivers/media/usb/gspca/Kconfig b/drivers/media/usb/gspca/Kconfig
index 6345f93..4f0c6d5 100644
--- a/drivers/media/usb/gspca/Kconfig
+++ b/drivers/media/usb/gspca/Kconfig
@@ -338,6 +338,15 @@ config USB_GSPCA_STK014
  To compile this driver as a module, choose M here: the
  module will be called gspca_stk014.
 
+config USB_GSPCA_STK1135
+   tristate "Syntek STK1135 USB Camera Driver"
+   depends on VIDEO_V4L2 && USB_GSPCA
+   help
+ Say Y here if you want support for cameras based on the STK1135 chip.
+
+ To compile this driver as a module, choose M here: the
+ module will be called gspca_stk1135.
+
 config USB_GSPCA_STV0680
tristate "STV0680 USB Camera Driver"
depends on VIDEO_V4L2 && USB_GSPCA
diff --git a/drivers/media/usb/gspca/Makefile b/drivers/media/usb/gspca/Makefile
index c901da0..5855131 100644
--- a/drivers/media/usb/gspca/Makefile
+++ b/drivers/media/usb/gspca/Makefile
@@ -34,6 +34,7 @@ obj-$(CONFIG_USB_GSPCA_SQ905C)   += gspca_sq905c.o
 obj-$(CONFIG_USB_GSPCA_SQ930X)   += gspca_sq930x.o
 obj-$(CONFIG_USB_GSPCA_SUNPLUS)  += gspca_sunplus.o
 obj-$(CONFIG_USB_GSPCA_STK014)   += gspca_stk014.o
+obj-$(CONFIG_USB_GSPCA_STK1135)  += gspca_stk1135.o
 obj-$(CONFIG_USB_GSPCA_STV0680)  += gspca_stv0680.o
 obj-$(CONFIG_USB_GSPCA_T613) += gspca_t613.o
 obj-$(CONFIG_USB_GSPCA_TOPRO)+= gspca_topro.o
@@ -78,6 +79,7 @@ gspca_sq905-objs:= sq905.o
 gspca_sq905c-objs   := sq905c.o
 gspca_sq930x-objs   := sq930x.o
 gspca_stk014-objs   := stk014.o
+gspca_stk1135-objs  := stk1135.o
 gspca_stv0680-objs  := stv0680.o
 gspca_sunplus-objs  := sunplus.o
 gspca_t613-objs := t613.o
diff --git a/drivers/media/usb/gspca/stk1135.c 
b/drivers/media/usb/gspca/stk1135.c
new file mode 100644
index 000..ce17202
--- /dev/null
+++ b/drivers/media/usb/gspca/stk1135.c
@@ -0,0 +1,968 @@
+/*
+ * Syntek STK1135 subdriver
+ *
+ * Copyright (c) 2013 Ondrej Zary
+ *
+ * Based on Syntekdriver by Nicolas VIVIEN:
+ *   http://syntekdriver.sourceforge.net
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Publi

Re: Syntek webcams and out-of-tree driver

2013-08-06 Thread Ondrej Zary
On Tuesday 06 August 2013, Hans de Goede wrote:
> Hi,
>
> On 08/05/2013 11:19 PM, Ondrej Zary wrote:
> > Hello,
> > the in-kernel stkwebcam driver (by Jaime Velasco Juan and Nicolas VIVIEN)
> > supports only two webcam types (USB IDs 0x174f:0xa311 and 0x05e1:0x0501).
> > There are many other Syntek webcam types that are not supported by this
> > driver (such as 0x174f:0x6a31 in Asus F5RL laptop).
> >
> > There is an out-of-tree GPL driver called stk11xx (by Martin Roos and
> > also Nicolas VIVIEN) at http://sourceforge.net/projects/syntekdriver/
> > which supports more webcams. It can be even compiled for the latest
> > kernels using the patch below and seems to work somehow (slow and buggy
> > but better than nothing) with the Asus F5RL.
>
> I took a quick look and there are a number of issues with this driver:
>
> 1) It conflicts usb-id wise with the new stk1160 driver (which supports
> usb-id 05e1:0408) so support for that usb-id, and any code only used for
> that id will need to be removed
>
> 2) "seems to work somehow (slow and buggy)" is not really the quality
> we aim for with in kernel drivers. We definitely will want to remove
> any usb-ids, and any code only used for those ids, where there is overlap
> with the existing stkwebcam driver, to avoid regressions
>
> 3) It does in kernel bayer decoding, this is not acceptable, it needs to
> be modified to produce buffers with raw bayer data (libv4l will take care
> of the bater decoding in userspace).
>
> 4) It is not using any of the new kernel infrastructure we have been adding
> over time, like the control-framework, videobuf2, etc. It would be best
> to convert this to a gspca sub driver (of which there are many already,
> which can serve as examples), so that it will use all the existing
> framework code.

Yes, this would be the best way - only extract the HW-dependent parts.

> As a minimum issues 1-3 needs to be addressed before this can be merged. An
> alternative /  better approach might be to simply only lift the code for
> your camera, and add a new gspca driver supporting only your camera.
>
> Either way since non of the v4l developers have a laptop which such a
> camera, you will need to do most of the work yourself, as we cannot test.
>
> So congratulations, you've just become a v4l kernel developer :)

Unfortunately the laptop isn't mine. I'll have it only for a while but will 
try to do something.

> Regards,
>
> Hans
>
> > Is there any possibility that this driver could be merged into the
> > kernel? The code could probably be simplified a lot and integrated into
> > gspca.
> >
> >
> > diff -urp syntekdriver-code-107-trunk-orig/driver/stk11xx.h
> > syntekdriver-code-107-trunk//driver/stk11xx.h ---
> > syntekdriver-code-107-trunk-orig/driver/stk11xx.h   2012-03-10
> > 10:03:12.0 +0100 +++
> > syntekdriver-code-107-trunk//driver/stk11xx.h   2013-08-05
> > 22:50:00.0 +0200 @@ -33,6 +33,7 @@
> >
> >   #ifndef STK11XX_H
> >   #define STK11XX_H
> > +#include 
> >
> >   #define DRIVER_NAME   "stk11xx"   
> > /**< Name of this driver */
> >   #define DRIVER_VERSION"v3.0.0"
> > /**< Version of this driver */
> > @@ -316,6 +317,7 @@ struct stk11xx_video {
> >* @struct usb_stk11xx
> >*/
> >   struct usb_stk11xx {
> > +   struct v4l2_device v4l2_dev;
> > struct video_device *vdev;  /**< Pointer on a V4L2 
> > video device */
> > struct usb_device *udev;/**< Pointer on a USB 
> > device */
> > struct usb_interface *interface;/**< Pointer on a USB interface 
> > */
> > diff -urp syntekdriver-code-107-trunk-orig/driver/stk11xx-v4l.c
> > syntekdriver-code-107-trunk//driver/stk11xx-v4l.c ---
> > syntekdriver-code-107-trunk-orig/driver/stk11xx-v4l.c   2012-03-10
> > 09:54:57.0 +0100 +++
> > syntekdriver-code-107-trunk//driver/stk11xx-v4l.c   2013-08-05
> > 22:51:12.0 +0200 @@ -1498,9 +1498,17 @@ int
> > v4l_stk11xx_register_video_device(st
> >   {
> > int err;
> >
> > +   err = v4l2_device_register(&dev->interface->dev, &dev->v4l2_dev);
> > +   if (err < 0) {
> > +   STK_ERROR("couldn't register v4l2_device\n");
> > +   kfree(dev);
> > +   return err;
> > +   }
> > +
> > strcpy(dev->vdev->name, DRIVER_DESC);
> >
> > -   dev->vdev->parent = &dev->interface->dev;
> > +// dev->vdev->parent = &dev->interface->dev;
> > +   dev->vdev->v4l2_dev = &dev->v4l2_dev;
> > dev->vdev->fops = &v4l_stk11xx_fops;
> > dev->vdev->release = video_device_release;
> > dev->vdev->minor = -1;
> > @@ -1533,6 +1541,7 @@ int v4l_stk11xx_unregister_video_device(
> >
> > video_set_drvdata(dev->vdev, NULL);
> > video_unregister_device(dev->vdev);
> > +   v4l2_device_unregister(&dev->v4l2_dev);
> >
> > return 0;
> >   }



-- 
Ondrej Zary
--
To unsubscribe from this list: send the 

Re: Syntek webcams and out-of-tree driver

2013-08-05 Thread Hans de Goede

Hi,

On 08/05/2013 11:19 PM, Ondrej Zary wrote:

Hello,
the in-kernel stkwebcam driver (by Jaime Velasco Juan and Nicolas VIVIEN)
supports only two webcam types (USB IDs 0x174f:0xa311 and 0x05e1:0x0501).
There are many other Syntek webcam types that are not supported by this
driver (such as 0x174f:0x6a31 in Asus F5RL laptop).

There is an out-of-tree GPL driver called stk11xx (by Martin Roos and also
Nicolas VIVIEN) at http://sourceforge.net/projects/syntekdriver/ which
supports more webcams. It can be even compiled for the latest kernels using
the patch below and seems to work somehow (slow and buggy but better than
nothing) with the Asus F5RL.


I took a quick look and there are a number of issues with this driver:

1) It conflicts usb-id wise with the new stk1160 driver (which supports
usb-id 05e1:0408) so support for that usb-id, and any code only used for
that id will need to be removed

2) "seems to work somehow (slow and buggy)" is not really the quality
we aim for with in kernel drivers. We definitely will want to remove
any usb-ids, and any code only used for those ids, where there is overlap
with the existing stkwebcam driver, to avoid regressions

3) It does in kernel bayer decoding, this is not acceptable, it needs to
be modified to produce buffers with raw bayer data (libv4l will take care
of the bater decoding in userspace).

4) It is not using any of the new kernel infrastructure we have been adding
over time, like the control-framework, videobuf2, etc. It would be best
to convert this to a gspca sub driver (of which there are many already,
which can serve as examples), so that it will use all the existing framework
code.

As a minimum issues 1-3 needs to be addressed before this can be merged. An
alternative /  better approach might be to simply only lift the code for your
camera, and add a new gspca driver supporting only your camera.

Either way since non of the v4l developers have a laptop which such a camera,
you will need to do most of the work yourself, as we cannot test.

So congratulations, you've just become a v4l kernel developer :)

Regards,

Hans




Is there any possibility that this driver could be merged into the kernel?
The code could probably be simplified a lot and integrated into gspca.


diff -urp syntekdriver-code-107-trunk-orig/driver/stk11xx.h 
syntekdriver-code-107-trunk//driver/stk11xx.h
--- syntekdriver-code-107-trunk-orig/driver/stk11xx.h   2012-03-10 
10:03:12.0 +0100
+++ syntekdriver-code-107-trunk//driver/stk11xx.h   2013-08-05 
22:50:00.0 +0200
@@ -33,6 +33,7 @@

  #ifndef STK11XX_H
  #define STK11XX_H
+#include 

  #define DRIVER_NAME   "stk11xx"  
   /**< Name of this driver */
  #define DRIVER_VERSION"v3.0.0"   
   /**< Version of this driver */
@@ -316,6 +317,7 @@ struct stk11xx_video {
   * @struct usb_stk11xx
   */
  struct usb_stk11xx {
+   struct v4l2_device v4l2_dev;
struct video_device *vdev;  /**< Pointer on a V4L2 
video device */
struct usb_device *udev;/**< Pointer on a USB 
device */
struct usb_interface *interface;/**< Pointer on a USB interface 
*/
diff -urp syntekdriver-code-107-trunk-orig/driver/stk11xx-v4l.c 
syntekdriver-code-107-trunk//driver/stk11xx-v4l.c
--- syntekdriver-code-107-trunk-orig/driver/stk11xx-v4l.c   2012-03-10 
09:54:57.0 +0100
+++ syntekdriver-code-107-trunk//driver/stk11xx-v4l.c   2013-08-05 
22:51:12.0 +0200
@@ -1498,9 +1498,17 @@ int v4l_stk11xx_register_video_device(st
  {
int err;

+   err = v4l2_device_register(&dev->interface->dev, &dev->v4l2_dev);
+   if (err < 0) {
+   STK_ERROR("couldn't register v4l2_device\n");
+   kfree(dev);
+   return err;
+   }
+
strcpy(dev->vdev->name, DRIVER_DESC);

-   dev->vdev->parent = &dev->interface->dev;
+// dev->vdev->parent = &dev->interface->dev;
+   dev->vdev->v4l2_dev = &dev->v4l2_dev;
dev->vdev->fops = &v4l_stk11xx_fops;
dev->vdev->release = video_device_release;
dev->vdev->minor = -1;
@@ -1533,6 +1541,7 @@ int v4l_stk11xx_unregister_video_device(

video_set_drvdata(dev->vdev, NULL);
video_unregister_device(dev->vdev);
+   v4l2_device_unregister(&dev->v4l2_dev);

return 0;
  }




--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Syntek webcams and out-of-tree driver

2013-08-05 Thread Ondrej Zary
Hello,
the in-kernel stkwebcam driver (by Jaime Velasco Juan and Nicolas VIVIEN)
supports only two webcam types (USB IDs 0x174f:0xa311 and 0x05e1:0x0501).
There are many other Syntek webcam types that are not supported by this
driver (such as 0x174f:0x6a31 in Asus F5RL laptop).

There is an out-of-tree GPL driver called stk11xx (by Martin Roos and also
Nicolas VIVIEN) at http://sourceforge.net/projects/syntekdriver/ which
supports more webcams. It can be even compiled for the latest kernels using
the patch below and seems to work somehow (slow and buggy but better than
nothing) with the Asus F5RL.

Is there any possibility that this driver could be merged into the kernel?
The code could probably be simplified a lot and integrated into gspca.


diff -urp syntekdriver-code-107-trunk-orig/driver/stk11xx.h 
syntekdriver-code-107-trunk//driver/stk11xx.h
--- syntekdriver-code-107-trunk-orig/driver/stk11xx.h   2012-03-10 
10:03:12.0 +0100
+++ syntekdriver-code-107-trunk//driver/stk11xx.h   2013-08-05 
22:50:00.0 +0200
@@ -33,6 +33,7 @@
 
 #ifndef STK11XX_H
 #define STK11XX_H
+#include 
 
 #define DRIVER_NAME"stk11xx"   
/**< Name of this driver */
 #define DRIVER_VERSION "v3.0.0"
/**< Version of this driver */
@@ -316,6 +317,7 @@ struct stk11xx_video {
  * @struct usb_stk11xx
  */
 struct usb_stk11xx {
+   struct v4l2_device v4l2_dev;
struct video_device *vdev;  /**< Pointer on a V4L2 
video device */
struct usb_device *udev;/**< Pointer on a USB 
device */
struct usb_interface *interface;/**< Pointer on a USB interface 
*/
diff -urp syntekdriver-code-107-trunk-orig/driver/stk11xx-v4l.c 
syntekdriver-code-107-trunk//driver/stk11xx-v4l.c
--- syntekdriver-code-107-trunk-orig/driver/stk11xx-v4l.c   2012-03-10 
09:54:57.0 +0100
+++ syntekdriver-code-107-trunk//driver/stk11xx-v4l.c   2013-08-05 
22:51:12.0 +0200
@@ -1498,9 +1498,17 @@ int v4l_stk11xx_register_video_device(st
 {
int err;
 
+   err = v4l2_device_register(&dev->interface->dev, &dev->v4l2_dev);
+   if (err < 0) {
+   STK_ERROR("couldn't register v4l2_device\n");
+   kfree(dev);
+   return err;
+   }
+
strcpy(dev->vdev->name, DRIVER_DESC);
 
-   dev->vdev->parent = &dev->interface->dev;
+// dev->vdev->parent = &dev->interface->dev;
+   dev->vdev->v4l2_dev = &dev->v4l2_dev;
dev->vdev->fops = &v4l_stk11xx_fops;
dev->vdev->release = video_device_release;
dev->vdev->minor = -1;
@@ -1533,6 +1541,7 @@ int v4l_stk11xx_unregister_video_device(
 
video_set_drvdata(dev->vdev, NULL);
video_unregister_device(dev->vdev);
+   v4l2_device_unregister(&dev->v4l2_dev);
 
return 0;
 }



-- 
Ondrej Zary
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html