Re: [PATCH V4 2/3] sta2x11_vip: convert to videobuf2 and control framework

2013-01-07 Thread Hans Verkuil
Hi Frederico!

Just one comment, see below:

On Sun January 6 2013 18:29:02 Federico Vaga wrote:
> This patch re-write the driver and use the videobuf2
> interface instead of the old videobuf. Moreover, it uses also
> the control framework which allows the driver to inherit
> controls from its subdevice (ADV7180)
> 
> Signed-off-by: Federico Vaga 
> Acked-by: Giancarlo Asnaghi 
> ---
>  drivers/media/pci/sta2x11/Kconfig   |2 +-
>  drivers/media/pci/sta2x11/sta2x11_vip.c | 1244 
> ++-
>  2 file modificati, 414 inserzioni(+), 832 rimozioni(-)
> 
> diff --git a/drivers/media/pci/sta2x11/Kconfig 
> b/drivers/media/pci/sta2x11/Kconfig
> index 6749f67..a94ccad 100644
> --- a/drivers/media/pci/sta2x11/Kconfig
> +++ b/drivers/media/pci/sta2x11/Kconfig
> @@ -2,7 +2,7 @@ config STA2X11_VIP
>   tristate "STA2X11 VIP Video For Linux"
>   depends on STA2X11
>   select VIDEO_ADV7180 if MEDIA_SUBDRV_AUTOSELECT
> - select VIDEOBUF_DMA_CONTIG
> + select VIDEOBUF2_DMA_CONTIG
>   depends on PCI && VIDEO_V4L2 && VIRT_TO_BUS
>   help
> Say Y for support for STA2X11 VIP (Video Input Port) capture
> diff --git a/drivers/media/pci/sta2x11/sta2x11_vip.c 
> b/drivers/media/pci/sta2x11/sta2x11_vip.c
> index ed1337a..e379e03 100644
> --- a/drivers/media/pci/sta2x11/sta2x11_vip.c
> +++ b/drivers/media/pci/sta2x11/sta2x11_vip.c



> -/**
> - * vidioc_try_fmt_vid_cap - set video capture format
> - * @file: descriptor of device ( not used)
> - * @priv: points to current videodevice
> - * @f: new format
> - *
> - * new video format is set which includes width and
> - * field type. width is fixed to 720, no scaling.
> - * Only UYVY is supported by this hardware.
> - * the minimum height is 200, the maximum is 576 (PAL)
> - *
> - * return value: 0, no error
> - *
> - * -EINVAL, pixel or field format not supported
> - *
> - */
>  static int vidioc_try_fmt_vid_cap(struct file *file, void *priv,
> struct v4l2_format *f)
>  {
> - struct video_device *dev = priv;
> - struct sta2x11_vip *vip = video_get_drvdata(dev);
> + struct sta2x11_vip *vip = video_drvdata(file);
>   int interlace_lim;
>  
> - if (V4L2_PIX_FMT_UYVY != f->fmt.pix.pixelformat)
> - return -EINVAL;
> -

You should keep this check for now. See this discussion:

http://www.mail-archive.com/linux-media@vger.kernel.org/msg56550.html

I'm going to change v4l2-compliance to make this a warning instead of an
error for now.

>   if (V4L2_STD_525_60 & vip->std)
>   interlace_lim = 240;
>   else
>   interlace_lim = 288;
>  
>   switch (f->fmt.pix.field) {
> + default:
>   case V4L2_FIELD_ANY:
>   if (interlace_lim < f->fmt.pix.height)
>   f->fmt.pix.field = V4L2_FIELD_INTERLACED;

After updating v4l2-compliance (I've just made the change to v4l2-compliance)
can you also post the output of v4l2-compliance for this driver?

Thanks,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH V4 2/3] sta2x11_vip: convert to videobuf2 and control framework

2013-01-07 Thread Hans Verkuil
Hi Frederico!

Just one comment, see below:

On Sun January 6 2013 18:29:02 Federico Vaga wrote:
 This patch re-write the driver and use the videobuf2
 interface instead of the old videobuf. Moreover, it uses also
 the control framework which allows the driver to inherit
 controls from its subdevice (ADV7180)
 
 Signed-off-by: Federico Vaga federico.v...@gmail.com
 Acked-by: Giancarlo Asnaghi giancarlo.asna...@st.com
 ---
  drivers/media/pci/sta2x11/Kconfig   |2 +-
  drivers/media/pci/sta2x11/sta2x11_vip.c | 1244 
 ++-
  2 file modificati, 414 inserzioni(+), 832 rimozioni(-)
 
 diff --git a/drivers/media/pci/sta2x11/Kconfig 
 b/drivers/media/pci/sta2x11/Kconfig
 index 6749f67..a94ccad 100644
 --- a/drivers/media/pci/sta2x11/Kconfig
 +++ b/drivers/media/pci/sta2x11/Kconfig
 @@ -2,7 +2,7 @@ config STA2X11_VIP
   tristate STA2X11 VIP Video For Linux
   depends on STA2X11
   select VIDEO_ADV7180 if MEDIA_SUBDRV_AUTOSELECT
 - select VIDEOBUF_DMA_CONTIG
 + select VIDEOBUF2_DMA_CONTIG
   depends on PCI  VIDEO_V4L2  VIRT_TO_BUS
   help
 Say Y for support for STA2X11 VIP (Video Input Port) capture
 diff --git a/drivers/media/pci/sta2x11/sta2x11_vip.c 
 b/drivers/media/pci/sta2x11/sta2x11_vip.c
 index ed1337a..e379e03 100644
 --- a/drivers/media/pci/sta2x11/sta2x11_vip.c
 +++ b/drivers/media/pci/sta2x11/sta2x11_vip.c

snip

 -/**
 - * vidioc_try_fmt_vid_cap - set video capture format
 - * @file: descriptor of device ( not used)
 - * @priv: points to current videodevice
 - * @f: new format
 - *
 - * new video format is set which includes width and
 - * field type. width is fixed to 720, no scaling.
 - * Only UYVY is supported by this hardware.
 - * the minimum height is 200, the maximum is 576 (PAL)
 - *
 - * return value: 0, no error
 - *
 - * -EINVAL, pixel or field format not supported
 - *
 - */
  static int vidioc_try_fmt_vid_cap(struct file *file, void *priv,
 struct v4l2_format *f)
  {
 - struct video_device *dev = priv;
 - struct sta2x11_vip *vip = video_get_drvdata(dev);
 + struct sta2x11_vip *vip = video_drvdata(file);
   int interlace_lim;
  
 - if (V4L2_PIX_FMT_UYVY != f-fmt.pix.pixelformat)
 - return -EINVAL;
 -

You should keep this check for now. See this discussion:

http://www.mail-archive.com/linux-media@vger.kernel.org/msg56550.html

I'm going to change v4l2-compliance to make this a warning instead of an
error for now.

   if (V4L2_STD_525_60  vip-std)
   interlace_lim = 240;
   else
   interlace_lim = 288;
  
   switch (f-fmt.pix.field) {
 + default:
   case V4L2_FIELD_ANY:
   if (interlace_lim  f-fmt.pix.height)
   f-fmt.pix.field = V4L2_FIELD_INTERLACED;

After updating v4l2-compliance (I've just made the change to v4l2-compliance)
can you also post the output of v4l2-compliance for this driver?

Thanks,

Hans
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH V4 2/3] sta2x11_vip: convert to videobuf2 and control framework

2013-01-06 Thread Federico Vaga
This patch re-write the driver and use the videobuf2
interface instead of the old videobuf. Moreover, it uses also
the control framework which allows the driver to inherit
controls from its subdevice (ADV7180)

Signed-off-by: Federico Vaga 
Acked-by: Giancarlo Asnaghi 
---
 drivers/media/pci/sta2x11/Kconfig   |2 +-
 drivers/media/pci/sta2x11/sta2x11_vip.c | 1244 ++-
 2 file modificati, 414 inserzioni(+), 832 rimozioni(-)

diff --git a/drivers/media/pci/sta2x11/Kconfig 
b/drivers/media/pci/sta2x11/Kconfig
index 6749f67..a94ccad 100644
--- a/drivers/media/pci/sta2x11/Kconfig
+++ b/drivers/media/pci/sta2x11/Kconfig
@@ -2,7 +2,7 @@ config STA2X11_VIP
tristate "STA2X11 VIP Video For Linux"
depends on STA2X11
select VIDEO_ADV7180 if MEDIA_SUBDRV_AUTOSELECT
-   select VIDEOBUF_DMA_CONTIG
+   select VIDEOBUF2_DMA_CONTIG
depends on PCI && VIDEO_V4L2 && VIRT_TO_BUS
help
  Say Y for support for STA2X11 VIP (Video Input Port) capture
diff --git a/drivers/media/pci/sta2x11/sta2x11_vip.c 
b/drivers/media/pci/sta2x11/sta2x11_vip.c
index ed1337a..e379e03 100644
--- a/drivers/media/pci/sta2x11/sta2x11_vip.c
+++ b/drivers/media/pci/sta2x11/sta2x11_vip.c
@@ -1,7 +1,11 @@
 /*
  * This is the driver for the STA2x11 Video Input Port.
  *
+ * Copyright (C) 2012   ST Microelectronics
+ * author: Federico Vaga 
  * Copyright (C) 2010   WindRiver Systems, Inc.
+ * authors: Andreas Kies 
+ *  Vlad Lungu   
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms and conditions of the GNU General Public License,
@@ -19,36 +23,30 @@
  * The full GNU General Public License is included in this distribution in
  * the file called "COPYING".
  *
- * Author: Andreas Kies 
- * Vlad Lungu 
- *
  */
 
 #include 
 #include 
 #include 
 #include 
-#include 
-
 #include 
-
 #include 
-
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
-#include 
+#include 
+#include 
+#include 
 
 #include "sta2x11_vip.h"
 
-#define DRV_NAME "sta2x11_vip"
 #define DRV_VERSION "1.3"
 
 #ifndef PCI_DEVICE_ID_STMICRO_VIP
@@ -63,8 +61,8 @@
 #define DVP_TFS0x08
 #define DVP_BFO0x0C
 #define DVP_BFS0x10
-#define DVP_VTP 0x14
-#define DVP_VBP 0x18
+#define DVP_VTP0x14
+#define DVP_VBP0x18
 #define DVP_VMP0x1C
 #define DVP_ITM0x98
 #define DVP_ITS0x9C
@@ -84,43 +82,20 @@
 
 #define DVP_HLFLN_SD   0x0001
 
-#define REG_WRITE(vip, reg, value) iowrite32((value), (vip->iomem)+(reg))
-#define REG_READ(vip, reg) ioread32((vip->iomem)+(reg))
-
 #define SAVE_COUNT 8
 #define AUX_COUNT 3
 #define IRQ_COUNT 1
 
-/**
- * struct sta2x11_vip - All internal data for one instance of device
- * @v4l2_dev: device registered in v4l layer
- * @video_dev: properties of our device
- * @pdev: PCI device
- * @adapter: contains I2C adapter information
- * @register_save_area: All relevant register are saved here during suspend
- * @decoder: contains information about video DAC
- * @format: pixel format, fixed UYVY
- * @std: video standard (e.g. PAL/NTSC)
- * @input: input line for video signal ( 0 or 1 )
- * @users: Number of open of device ( max. 1 )
- * @disabled: Device is in power down state
- * @mutex: ensures exclusive opening of device
- * @slock: for excluse acces of registers
- * @vb_vidq: queue maintained by videobuf layer
- * @capture: linked list of capture buffer
- * @active: struct videobuf_buffer currently beingg filled
- * @started: device is ready to capture frame
- * @closing: device will be shut down
- * @tcount: Number of top frames
- * @bcount: Number of bottom frames
- * @overflow: Number of FIFO overflows
- * @mem_spare: small buffer of unused frame
- * @dma_spare: dma addres of mem_spare
- * @iomem: hardware base address
- * @config: I2C and gpio config from platform
- *
- * All non-local data is accessed via this structure.
- */
+
+struct vip_buffer {
+   struct vb2_buffer   vb;
+   struct list_headlist;
+   dma_addr_t  dma;
+};
+static inline struct vip_buffer *to_vip_buffer(struct vb2_buffer *vb2)
+{
+   return container_of(vb2, struct vip_buffer, vb);
+}
 
 struct sta2x11_vip {
struct v4l2_device v4l2_dev;
@@ -129,21 +104,27 @@ struct sta2x11_vip {
struct i2c_adapter *adapter;
unsigned int register_save_area[IRQ_COUNT + SAVE_COUNT + AUX_COUNT];
struct v4l2_subdev *decoder;
-   struct v4l2_pix_format format;
-   v4l2_std_id std;
-   unsigned int input;
-   int users;
-   int disabled;
-   struct mutex mutex; /* exclusive access during open */
-   spinlock_t slock;   /* spin lock for hardware and queue access */
-   struct videobuf_queue vb_vidq;
-   struct list_head 

[PATCH V4 2/3] sta2x11_vip: convert to videobuf2 and control framework

2013-01-06 Thread Federico Vaga
This patch re-write the driver and use the videobuf2
interface instead of the old videobuf. Moreover, it uses also
the control framework which allows the driver to inherit
controls from its subdevice (ADV7180)

Signed-off-by: Federico Vaga federico.v...@gmail.com
Acked-by: Giancarlo Asnaghi giancarlo.asna...@st.com
---
 drivers/media/pci/sta2x11/Kconfig   |2 +-
 drivers/media/pci/sta2x11/sta2x11_vip.c | 1244 ++-
 2 file modificati, 414 inserzioni(+), 832 rimozioni(-)

diff --git a/drivers/media/pci/sta2x11/Kconfig 
b/drivers/media/pci/sta2x11/Kconfig
index 6749f67..a94ccad 100644
--- a/drivers/media/pci/sta2x11/Kconfig
+++ b/drivers/media/pci/sta2x11/Kconfig
@@ -2,7 +2,7 @@ config STA2X11_VIP
tristate STA2X11 VIP Video For Linux
depends on STA2X11
select VIDEO_ADV7180 if MEDIA_SUBDRV_AUTOSELECT
-   select VIDEOBUF_DMA_CONTIG
+   select VIDEOBUF2_DMA_CONTIG
depends on PCI  VIDEO_V4L2  VIRT_TO_BUS
help
  Say Y for support for STA2X11 VIP (Video Input Port) capture
diff --git a/drivers/media/pci/sta2x11/sta2x11_vip.c 
b/drivers/media/pci/sta2x11/sta2x11_vip.c
index ed1337a..e379e03 100644
--- a/drivers/media/pci/sta2x11/sta2x11_vip.c
+++ b/drivers/media/pci/sta2x11/sta2x11_vip.c
@@ -1,7 +1,11 @@
 /*
  * This is the driver for the STA2x11 Video Input Port.
  *
+ * Copyright (C) 2012   ST Microelectronics
+ * author: Federico Vaga federico.v...@gmail.com
  * Copyright (C) 2010   WindRiver Systems, Inc.
+ * authors: Andreas Kies andreas.k...@windriver.com
+ *  Vlad Lungu   vlad.lu...@windriver.com
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms and conditions of the GNU General Public License,
@@ -19,36 +23,30 @@
  * The full GNU General Public License is included in this distribution in
  * the file called COPYING.
  *
- * Author: Andreas Kies andreas.k...@windriver.com
- * Vlad Lungu vlad.lu...@windriver.com
- *
  */
 
 #include linux/types.h
 #include linux/kernel.h
 #include linux/module.h
 #include linux/init.h
-#include linux/vmalloc.h
-
 #include linux/videodev2.h
-
 #include linux/kmod.h
-
 #include linux/pci.h
 #include linux/interrupt.h
-#include linux/mutex.h
 #include linux/io.h
 #include linux/gpio.h
 #include linux/i2c.h
 #include linux/delay.h
 #include media/v4l2-common.h
 #include media/v4l2-device.h
+#include media/v4l2-ctrls.h
 #include media/v4l2-ioctl.h
-#include media/videobuf-dma-contig.h
+#include media/v4l2-fh.h
+#include media/v4l2-event.h
+#include media/videobuf2-dma-contig.h
 
 #include sta2x11_vip.h
 
-#define DRV_NAME sta2x11_vip
 #define DRV_VERSION 1.3
 
 #ifndef PCI_DEVICE_ID_STMICRO_VIP
@@ -63,8 +61,8 @@
 #define DVP_TFS0x08
 #define DVP_BFO0x0C
 #define DVP_BFS0x10
-#define DVP_VTP 0x14
-#define DVP_VBP 0x18
+#define DVP_VTP0x14
+#define DVP_VBP0x18
 #define DVP_VMP0x1C
 #define DVP_ITM0x98
 #define DVP_ITS0x9C
@@ -84,43 +82,20 @@
 
 #define DVP_HLFLN_SD   0x0001
 
-#define REG_WRITE(vip, reg, value) iowrite32((value), (vip-iomem)+(reg))
-#define REG_READ(vip, reg) ioread32((vip-iomem)+(reg))
-
 #define SAVE_COUNT 8
 #define AUX_COUNT 3
 #define IRQ_COUNT 1
 
-/**
- * struct sta2x11_vip - All internal data for one instance of device
- * @v4l2_dev: device registered in v4l layer
- * @video_dev: properties of our device
- * @pdev: PCI device
- * @adapter: contains I2C adapter information
- * @register_save_area: All relevant register are saved here during suspend
- * @decoder: contains information about video DAC
- * @format: pixel format, fixed UYVY
- * @std: video standard (e.g. PAL/NTSC)
- * @input: input line for video signal ( 0 or 1 )
- * @users: Number of open of device ( max. 1 )
- * @disabled: Device is in power down state
- * @mutex: ensures exclusive opening of device
- * @slock: for excluse acces of registers
- * @vb_vidq: queue maintained by videobuf layer
- * @capture: linked list of capture buffer
- * @active: struct videobuf_buffer currently beingg filled
- * @started: device is ready to capture frame
- * @closing: device will be shut down
- * @tcount: Number of top frames
- * @bcount: Number of bottom frames
- * @overflow: Number of FIFO overflows
- * @mem_spare: small buffer of unused frame
- * @dma_spare: dma addres of mem_spare
- * @iomem: hardware base address
- * @config: I2C and gpio config from platform
- *
- * All non-local data is accessed via this structure.
- */
+
+struct vip_buffer {
+   struct vb2_buffer   vb;
+   struct list_headlist;
+   dma_addr_t  dma;
+};
+static inline struct vip_buffer *to_vip_buffer(struct vb2_buffer *vb2)
+{
+   return container_of(vb2, struct vip_buffer, vb);
+}
 
 struct sta2x11_vip {
struct v4l2_device v4l2_dev;
@@ -129,21 +104,27 @@ struct