Re: [PATCH 1/3] Add a core driver for SI476x MFD

2012-09-21 Thread Hans Verkuil
On Fri September 21 2012 03:05:41 andrey.smir...@convergeddevices.net wrote:
 On 09/13/2012 11:44 PM, Hans Verkuil wrote:
  Hi Andrey!
 
  Thanks for posting this driver. One request for the future: please split 
  this
  patch up in smaller pieces: one for each c source for example. That makes it
  easier to review.
 
 Will do for next version.
 
  +
  +/**
  + * __core_send_command() - sends a command to si476x and waits its
  + * response
  + * @core:si476x_device structure for the device we are
  + *communicating with
  + * @command:  command id
  + * @args: command arguments we are sending
  + * @argn: actual size of @args
  + * @response: buffer to place the expected response from the device
  + * @respn:actual size of @response
  + * @usecs:amount of time to wait before reading the response (in
  + *usecs)
  + *
  + * Function returns 0 on succsess and negative error code on
  + * failure
  + */
  +static int __core_send_command(struct si476x_core *core,
  +   const u8 command,
  +   const u8 args[],
  +   const int argn,
  +   u8 resp[],
  +   const int respn,
  +   const int usecs)
  +{
  +   struct i2c_client *client = core-client;
  +   int err;
  +   u8  data[CMD_MAX_ARGS_COUNT + 1];
  +
  +   if (argn  CMD_MAX_ARGS_COUNT) {
  +   err = -ENOMEM;
  +   goto exit;
  Why goto exit? There is no clean up after the exit label, so just return
  immediately. Ditto for all the other goto exit's in this function.
 
 To have only just on point of exit from the function that's just
 personal coding style preference.
 There are no technical reasons behind that, I can change that.
 
 
  +  }
  +
  +  if (!client-adapter) {
  +  err = -ENODEV;
  +  goto exit;
  +  }
  +
  +  /* First send the command and its arguments */
  +  data[0] = command;
  +  memcpy(data[1], args, argn);
  +  DBG_BUFFER(client-dev, Command:\n, data, argn + 1);
  +
  +  err = si476x_i2c_xfer(core, SI476X_I2C_SEND, (char *) data, argn + 1);
  +  if (err != argn + 1) {
  +  dev_err(core-client-dev,
  +  Error while sending command 0x%02x\n,
  +  command);
  +  err = (err = 0) ? -EIO : err;
  +  goto exit;
  +  }
  +  /* Set CTS to zero only after the command is send to avoid
  +   * possible racing conditions when working in polling mode */
  +  atomic_set(core-cts, 0);
  +
  +  if (!wait_event_timeout(core-command,
  +  atomic_read(core-cts),
  +  usecs_to_jiffies(usecs) + 1))
  +  dev_warn(core-client-dev,
  +   (%s) [CMD 0x%02x] Device took too much time to 
  answer.\n,
  +   __func__, command);
  +
  +  /*
  +When working in polling mode, for some reason the tuner will
  +report CTS bit as being set in the first status byte read,
  +but all the consequtive ones will return zros until the
  +tuner is actually completed the POWER_UP command. To
  +workaround that we wait for second CTS to be reported
  +   */
  +  if (unlikely(!core-client-irq  command == CMD_POWER_UP)) {
  +  if (!wait_event_timeout(core-command,
  +  atomic_read(core-cts),
  +  usecs_to_jiffies(usecs) + 1))
  +  dev_warn(core-client-dev,
  +   (%s) Power up took too much time.\n,
  +   __func__);
  +  }
  +
  +  /* Then get the response */
  +  err = si476x_i2c_xfer(core, SI476X_I2C_RECV, resp, respn);
  +  if (err != respn) {
  +  dev_err(core-client-dev,
  +  Error while reading response for command 0x%02x\n,
  +  command);
  +  err = (err = 0) ? -EIO : err;
  +  goto exit;
  +  }
  +  DBG_BUFFER(client-dev, Response:\n, resp, respn);
  +
  +  err = 0;
  +
  +  if (resp[0]  SI476X_ERR) {
  +  dev_err(core-client-dev, Chip set error flag\n);
  +  err = si476x_core_parse_and_nag_about_error(core);
  +  goto exit;
  +  }
  +
  +  if (!(resp[0]  SI476X_CTS))
  +  err = -EBUSY;
  +exit:
  +  return err;
  +}
  +
  +#define CORE_SEND_COMMAND(core, cmd, args, resp, timeout) \
  +  __core_send_command(core, cmd, args,\
  +  ARRAY_SIZE(args),   \
  +  resp, ARRAY_SIZE(resp), \
  +  timeout)
  +
  +
  +static int __cmd_tune_seek_freq(struct si476x_core *core,
  +  uint8_t cmd,
  +  const uint8_t args[], size_t argn,
  +  uint8_t *resp, size_t respn,
  +  int (*clear_stcint) (struct si476x_core *core))
  +{
  +  int err;
  

Re: [alsa-devel] [PATCH v2 00/34] i.MX multi-platform support

2012-09-21 Thread Shawn Guo
On Thu, Sep 20, 2012 at 03:56:56PM +, Arnd Bergmann wrote:
 Ok, fair enough. I think we can put it in arm-soc/for-next as a staging
 branch anyway to give it some exposure to linux-next, and then we can
 decide whether a rebase is necessary before sending it to Linus.
 
I just saw the announcement from Olof - no more major merge for 3.7
will be accepted from now on.  Can this be an exception or should I
plan this for 3.8?

Shawn
--
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


Re: [alsa-devel] [PATCH v2 00/34] i.MX multi-platform support

2012-09-21 Thread Olof Johansson
On Fri, Sep 21, 2012 at 1:01 AM, Shawn Guo shawn@linaro.org wrote:
 On Thu, Sep 20, 2012 at 03:56:56PM +, Arnd Bergmann wrote:
 Ok, fair enough. I think we can put it in arm-soc/for-next as a staging
 branch anyway to give it some exposure to linux-next, and then we can
 decide whether a rebase is necessary before sending it to Linus.

 I just saw the announcement from Olof - no more major merge for 3.7
 will be accepted from now on.  Can this be an exception or should I
 plan this for 3.8?


I'll take a look at merging it tomorrow after I've dealt with smp_ops;
if it looks reasonably conflict-free I'll pull it in. We need the
sound dependency sorted out (or agreed upon) first though.


-Olof
--
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


Re: [RFC] Timestamps and V4L2

2012-09-21 Thread Christian Gmeiner
2012/9/20 Sakari Ailus sakari.ai...@iki.fi:
 Hi all,


 This RFC intends to summarise and further the recent discussion on
 linux-media regarding the proposed changes of timestamping V4L2 buffers.


 The problem
 ===

 The V4L2 has long used realtime timestamps (such as
 clock_gettime(CLOCK_REALTIME, ...)) to stamp the video buffers before
 handing them over to the user. This has been found problematic in
 associating the video buffers with data from other sources: realtime clock
 may jump around due to daylight saving time, for example, and ALSA
 (audio-video synchronisation is a common use case) user space API does not
 provide the user with realtime timestamps, but instead uses monotonic time
 (i.e. clock_gettime(CLOCK_MONOTONIC, ...)).

 This is especially an issue in embedded systems where video recording is a
 common use case. Drivers typically used in such systems have silently
 switched to use monotonic timestamps. While against the spec, this is
 necessary for those systems to operate properly.

 In general, realtime timestamps are seen of little use in other than
 debugging purposes, but monotonic timestamps are fine for that as well. It's
 still possible that an application I'm not aware of uses them in a peculiar
 way that would be adversely affected by changing to monotonic timestamps.
 Nevertheless, we're not supposed to break the API (or ABI). It'd be also
 very important for the application to know what kind of timestamps are
 provided by the device.


 Requirements, wishes and constraints
 

 Now that it seems to be about the time to fix these issues, it's worth
 looking a little bit to the future to anticipate the coming changes to be
 able to accommodate them better later on.

 - The new default should be monotonic. As the monotonic timestamps are seen
 to be the most useful, they should be made the default.

 - timeval vs. timespec. The two structs can be used to store timestamp
 information. They are not compatible with each other. It's a little bit
 uncertain what's the case with all the architectures but it looks like the
 timespec fits into the space of timeval in all cases. If timespec is
 considered to be used somewhere the compatibility must be ensured. Timespec
 is better than timeval since timespec has more precision and it's the same
 struct that's used everywhere else in the V4L2 API: timespec does not need
 conversion to timespec in the user space.

 struct timespec {
 __kernel_time_t tv_sec; /* seconds */
 longtv_nsec;/* nanoseconds */
 };

 struct timeval {
 __kernel_time_t tv_sec; /* seconds */
 __kernel_suseconds_ttv_usec;/* microseconds */
 };

 To be able to use timespec, the user would have to most likely explicitly
 choose to do that.

 - Users should know what kind of timestamps the device produces. This
 includes existing and future kernels. What should be considered are
 uninformed porting drivers back and forth across kernel versions and
 out-of-date kernel header files.

 - Device-dependent timestamps. Some devices such as the uvcvideo ones
 produce device-dependent timestamps for synchronising video and audio, both
 produced by the same physical hardware device. For uvcvideo these timestamps
 are unsigned 32-bit integers.


What about pure output devices like old-school mpeg2 cards? The timestamps for
audio and video are pure PTS values.


 - There's also another clock, Linux-specific raw monotonic clock (as in
 clock_gettime(CLOCK_RAW_MONOTONIC, ...)) that could be better in some use
 cases than the regular monotonic clock. The difference is that the raw
 monotonic clock is free from the NTP adjustments. It would be nice for the
 user to be able to choose the clock used for timestamps. This is especially
 important for device-dependent timestamps: not all applications can be
 expected to be able to use them.

 - The field adjacent to timestamp, timecode, is 128 bits wide, and not used
 by a single driver. This field could be re-used.


 Possible solutions
 ==

 Not all of the solutions below that have been proposed are mutually
 exclusive. That's also what's making the choice difficult: the ultimate
 solution to the issue of timestamping may involve several of these --- or
 possibly something better that's not on the list.


 Use of timespec
 ---

 If we can conclude timespec will always fit into the size of timeval (or
 timecode) we could use timespec instead. The solution should still make
 the use of timespec explicit to the user space. This seems to conflict with
 the idea of making monotonic timestamps the default: the default can't be
 anything incompatible with timeval, and at the same time it's the most
 important that the monotonic timestamps are timespec.


 Kernel version as indicator of timestamp
 

 Conversion of drivers to use 

[PATCH 1/2] m5mols: Remove unneeded control ops assignments

2012-09-21 Thread Sylwester Nawrocki
Since all host drivers using this subdev are already using
the control framework these compatibility ops can be removed.

Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
 drivers/media/video/m5mols/m5mols_core.c | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/drivers/media/video/m5mols/m5mols_core.c 
b/drivers/media/video/m5mols/m5mols_core.c
index ac7d28b..cbb6381 100644
--- a/drivers/media/video/m5mols/m5mols_core.c
+++ b/drivers/media/video/m5mols/m5mols_core.c
@@ -822,13 +822,6 @@ static int m5mols_log_status(struct v4l2_subdev *sd)
 
 static const struct v4l2_subdev_core_ops m5mols_core_ops = {
.s_power= m5mols_s_power,
-   .g_ctrl = v4l2_subdev_g_ctrl,
-   .s_ctrl = v4l2_subdev_s_ctrl,
-   .queryctrl  = v4l2_subdev_queryctrl,
-   .querymenu  = v4l2_subdev_querymenu,
-   .g_ext_ctrls= v4l2_subdev_g_ext_ctrls,
-   .try_ext_ctrls  = v4l2_subdev_try_ext_ctrls,
-   .s_ext_ctrls= v4l2_subdev_s_ext_ctrls,
.log_status = m5mols_log_status,
 };
 
-- 
1.7.11.3

--
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


[PATCH 2/2] m5mols: Protect driver data with a mutex

2012-09-21 Thread Sylwester Nawrocki
Without the locking the driver's data could get corrupted when the subdev
is accessed from user space and from host driver by multiple processes.

Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
 drivers/media/video/m5mols/m5mols.h  | 18 +---
 drivers/media/video/m5mols/m5mols_core.c | 77 +---
 2 files changed, 62 insertions(+), 33 deletions(-)

diff --git a/drivers/media/video/m5mols/m5mols.h 
b/drivers/media/video/m5mols/m5mols.h
index bb58991..15d3a4f 100644
--- a/drivers/media/video/m5mols/m5mols.h
+++ b/drivers/media/video/m5mols/m5mols.h
@@ -155,8 +155,6 @@ struct m5mols_version {
  * @pdata: platform data
  * @sd: v4l-subdev instance
  * @pad: media pad
- * @ffmt: current fmt according to resolution type
- * @res_type: current resolution type
  * @irq_waitq: waitqueue for the capture
  * @irq_done: set to 1 in the interrupt handler
  * @handle: control handler
@@ -174,6 +172,10 @@ struct m5mols_version {
  * @wdr: wide dynamic range control
  * @stabilization: image stabilization control
  * @jpeg_quality: JPEG compression quality control
+ * @set_power: optional power callback to the board code
+ * @lock: mutex protecting the structure fields below
+ * @ffmt: current fmt according to resolution type
+ * @res_type: current resolution type
  * @ver: information of the version
  * @cap: the capture mode attributes
  * @isp_ready: 1 when the ISP controller has completed booting
@@ -181,14 +183,11 @@ struct m5mols_version {
  * @ctrl_sync: 1 when the control handler state is restored in H/W
  * @resolution:register value for current resolution
  * @mode: register value for current operation mode
- * @set_power: optional power callback to the board code
  */
 struct m5mols_info {
const struct m5mols_platform_data *pdata;
struct v4l2_subdev sd;
struct media_pad pad;
-   struct v4l2_mbus_framefmt ffmt[M5MOLS_RESTYPE_MAX];
-   int res_type;
 
wait_queue_head_t irq_waitq;
atomic_t irq_done;
@@ -216,6 +215,13 @@ struct m5mols_info {
struct v4l2_ctrl *stabilization;
struct v4l2_ctrl *jpeg_quality;
 
+   int (*set_power)(struct device *dev, int on);
+
+   struct mutex lock;
+
+   struct v4l2_mbus_framefmt ffmt[M5MOLS_RESTYPE_MAX];
+   int res_type;
+
struct m5mols_version ver;
struct m5mols_capture cap;
 
@@ -225,8 +231,6 @@ struct m5mols_info {
 
u8 resolution;
u8 mode;
-
-   int (*set_power)(struct device *dev, int on);
 };
 
 #define is_available_af(__info)(__info-ver.af)
diff --git a/drivers/media/video/m5mols/m5mols_core.c 
b/drivers/media/video/m5mols/m5mols_core.c
index cbb6381..5d108fe 100644
--- a/drivers/media/video/m5mols/m5mols_core.c
+++ b/drivers/media/video/m5mols/m5mols_core.c
@@ -551,13 +551,18 @@ static int m5mols_get_fmt(struct v4l2_subdev *sd, struct 
v4l2_subdev_fh *fh,
 {
struct m5mols_info *info = to_m5mols(sd);
struct v4l2_mbus_framefmt *format;
+   int ret = 0;
+
+   mutex_lock(info-lock);
 
format = __find_format(info, fh, fmt-which, info-res_type);
if (!format)
-   return -EINVAL;
+   fmt-format = *format;
+   else
+   ret = -EINVAL;
 
-   fmt-format = *format;
-   return 0;
+   mutex_unlock(info-lock);
+   return ret;
 }
 
 static int m5mols_set_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh,
@@ -578,6 +583,7 @@ static int m5mols_set_fmt(struct v4l2_subdev *sd, struct 
v4l2_subdev_fh *fh,
if (!sfmt)
return 0;
 
+   mutex_lock(info-lock);
 
format-code = m5mols_default_ffmt[type].code;
format-colorspace = V4L2_COLORSPACE_JPEG;
@@ -589,7 +595,8 @@ static int m5mols_set_fmt(struct v4l2_subdev *sd, struct 
v4l2_subdev_fh *fh,
info-res_type = type;
}
 
-   return 0;
+   mutex_unlock(info-lock);
+   return ret;
 }
 
 static int m5mols_enum_mbus_code(struct v4l2_subdev *sd,
@@ -661,20 +668,25 @@ static int m5mols_start_monitor(struct m5mols_info *info)
 static int m5mols_s_stream(struct v4l2_subdev *sd, int enable)
 {
struct m5mols_info *info = to_m5mols(sd);
-   u32 code = info-ffmt[info-res_type].code;
+   u32 code;
+   int ret;
 
-   if (enable) {
-   int ret = -EINVAL;
+   mutex_lock(info-lock);
+   code = info-ffmt[info-res_type].code;
 
+   if (enable) {
if (is_code(code, M5MOLS_RESTYPE_MONITOR))
ret = m5mols_start_monitor(info);
if (is_code(code, M5MOLS_RESTYPE_CAPTURE))
ret = m5mols_start_capture(info);
-
-   return ret;
+   else
+   ret = -EINVAL;
+   } else {
+   ret = m5mols_set_mode(info, REG_PARAMETER);
}
 
-   return m5mols_set_mode(info, REG_PARAMETER);
+   

[PATCH v2 3/4] sta2x11_vip: convert to videobuf2 and control framework

2012-09-21 Thread federico . vaga
From: Federico Vaga federico.v...@gmail.com

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 | 1235 ++-
 2 file modificati, 411 inserzioni(+), 826 rimozioni(-)

diff --git a/drivers/media/pci/sta2x11/Kconfig 
b/drivers/media/pci/sta2x11/Kconfig
index 6749f67..654339f 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_STREAMING
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 4c10205..f423039 100644
--- a/drivers/media/pci/sta2x11/sta2x11_vip.c
+++ b/drivers/media/pci/sta2x11/sta2x11_vip.c
@@ -1,6 +1,7 @@
 /*
  * This is the driver for the STA2x11 Video Input Port.
  *
+ * Copyright (C) 2012   ST Microelectronics
  * Copyright (C) 2010   WindRiver Systems, Inc.
  *
  * This program is free software; you can redistribute it and/or modify it
@@ -19,36 +20,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-streaming.h
 
 #include sta2x11_vip.h
 
-#define DRV_NAME sta2x11_vip
 #define DRV_VERSION 1.3
 
 #ifndef PCI_DEVICE_ID_STMICRO_VIP
@@ -63,8 +58,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,44 +79,24 @@
 
 #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_fh {
+   struct v4l2_fh fh;
+};
 struct sta2x11_vip {
struct v4l2_device v4l2_dev;
struct video_device *video_dev;
@@ -129,21 +104,27 @@ struct sta2x11_vip {
struct i2c_adapter *adapter;
unsigned int 

[PATCH 1/4] v4l: vb2: add prepare/finish callbacks to allocators

2012-09-21 Thread Federico Vaga
This patch adds support for prepare/finish callbacks in VB2 allocators.
These callback are used for buffer flushing.

Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
Acked-by: Federico Vaga federico.v...@gmail.com
---
 drivers/media/v4l2-core/videobuf2-core.c | 11 +++
 include/media/videobuf2-core.h   |  7 +++
 2 file modificati, 18 inserzioni(+)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
b/drivers/media/v4l2-core/videobuf2-core.c
index 4da3df6..079fa79 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -790,6 +790,7 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum 
vb2_buffer_state state)
 {
struct vb2_queue *q = vb-vb2_queue;
unsigned long flags;
+   unsigned int plane;
 
if (vb-state != VB2_BUF_STATE_ACTIVE)
return;
@@ -800,6 +801,10 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum 
vb2_buffer_state state)
dprintk(4, Done processing on buffer %d, state: %d\n,
vb-v4l2_buf.index, vb-state);
 
+   /* sync buffers */
+   for (plane = 0; plane  vb-num_planes; ++plane)
+   call_memop(q, finish, vb-planes[plane].mem_priv);
+
/* Add the buffer to the done buffers list */
spin_lock_irqsave(q-done_lock, flags);
vb-state = state;
@@ -975,9 +980,15 @@ static int __qbuf_mmap(struct vb2_buffer *vb, const struct 
v4l2_buffer *b)
 static void __enqueue_in_driver(struct vb2_buffer *vb)
 {
struct vb2_queue *q = vb-vb2_queue;
+   unsigned int plane;
 
vb-state = VB2_BUF_STATE_ACTIVE;
atomic_inc(q-queued_count);
+
+   /* sync buffers */
+   for (plane = 0; plane  vb-num_planes; ++plane)
+   call_memop(q, prepare, vb-planes[plane].mem_priv);
+
q-ops-buf_queue(vb);
 }
 
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 8dd9b6c..f374f99 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -41,6 +41,10 @@ struct vb2_fileio_data;
  *  argument to other ops in this structure
  * @put_userptr: inform the allocator that a USERPTR buffer will no longer
  *  be used
+ * @prepare:   called everytime the buffer is passed from userspace to the
+ * driver, usefull for cache synchronisation, optional
+ * @finish:called everytime the buffer is passed back from the driver
+ * to the userspace, also optional
  * @vaddr: return a kernel virtual address to a given memory buffer
  * associated with the passed private structure or NULL if no
  * such mapping exists
@@ -65,6 +69,9 @@ struct vb2_mem_ops {
unsigned long size, int write);
void(*put_userptr)(void *buf_priv);
 
+   void(*prepare)(void *buf_priv);
+   void(*finish)(void *buf_priv);
+
void*(*vaddr)(void *buf_priv);
void*(*cookie)(void *buf_priv);
 
-- 
1.7.11.4

--
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


[PATCH v2 2/4] videobuf2-dma-streaming: new videobuf2 memory allocator

2012-09-21 Thread Federico Vaga
The DMA streaming allocator is similar to the DMA contig but it use the
DMA streaming interface (dma_map_single, dma_unmap_single). The
allocator allocates buffers and immediately map the memory for DMA
transfer. For each buffer prepare/finish it does a DMA synchronization.

Signed-off-by: Federico Vaga federico.v...@gmail.com
---
 drivers/media/v4l2-core/Kconfig   |   5 +
 drivers/media/v4l2-core/Makefile  |   1 +
 drivers/media/v4l2-core/videobuf2-dma-streaming.c | 205 ++
 include/media/videobuf2-dma-streaming.h   |  32 
 4 file modificati, 243 inserzioni(+)
 create mode 100644 drivers/media/v4l2-core/videobuf2-dma-streaming.c
 create mode 100644 include/media/videobuf2-dma-streaming.h

diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig
index 0c54e19..60548a7 100644
--- a/drivers/media/v4l2-core/Kconfig
+++ b/drivers/media/v4l2-core/Kconfig
@@ -79,3 +79,8 @@ config VIDEOBUF2_DMA_SG
#depends on HAS_DMA
select VIDEOBUF2_CORE
select VIDEOBUF2_MEMOPS
+
+config VIDEOBUF2_DMA_STREAMING
+   select VIDEOBUF2_CORE
+   select VIDEOBUF2_MEMOPS
+   tristate
diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
index c2d61d4..0b2756f 100644
--- a/drivers/media/v4l2-core/Makefile
+++ b/drivers/media/v4l2-core/Makefile
@@ -28,6 +28,7 @@ obj-$(CONFIG_VIDEOBUF2_MEMOPS) += videobuf2-memops.o
 obj-$(CONFIG_VIDEOBUF2_VMALLOC) += videobuf2-vmalloc.o
 obj-$(CONFIG_VIDEOBUF2_DMA_CONTIG) += videobuf2-dma-contig.o
 obj-$(CONFIG_VIDEOBUF2_DMA_SG) += videobuf2-dma-sg.o
+obj-$(CONFIG_VIDEOBUF2_DMA_STREAMING) += videobuf2-dma-streaming.o
 
 ccflags-y += -I$(srctree)/drivers/media/dvb-core
 ccflags-y += -I$(srctree)/drivers/media/dvb-frontends
diff --git a/drivers/media/v4l2-core/videobuf2-dma-streaming.c 
b/drivers/media/v4l2-core/videobuf2-dma-streaming.c
new file mode 100644
index 000..16d5569
--- /dev/null
+++ b/drivers/media/v4l2-core/videobuf2-dma-streaming.c
@@ -0,0 +1,205 @@
+/*
+ * videobuf2-dma-streaming.c - DMA streaming memory allocator for videobuf2
+ *
+ * Copyright (C) 2012 Federico Vaga federico.v...@gmail.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include linux/module.h
+#include linux/slab.h
+#include linux/pagemap.h
+#include linux/dma-mapping.h
+
+#include media/videobuf2-core.h
+#include media/videobuf2-dma-streaming.h
+#include media/videobuf2-memops.h
+
+struct vb2_streaming_conf {
+   struct device   *dev;
+};
+struct vb2_streaming_buf {
+   struct vb2_streaming_conf   *conf;
+   void*vaddr;
+
+   dma_addr_t  dma_handle;
+
+   unsigned long   size;
+   struct vm_area_struct   *vma;
+
+   atomic_trefcount;
+   struct vb2_vmarea_handler   handler;
+};
+
+static void vb2_dma_streaming_put(void *buf_priv)
+{
+   struct vb2_streaming_buf *buf = buf_priv;
+
+   if (atomic_dec_and_test(buf-refcount)) {
+   dma_unmap_single(buf-conf-dev, buf-dma_handle, buf-size,
+DMA_FROM_DEVICE);
+   free_pages_exact(buf-vaddr, buf-size);
+   kfree(buf);
+   }
+
+}
+
+static void *vb2_dma_streaming_alloc(void *alloc_ctx, unsigned long size)
+{
+   struct vb2_streaming_conf *conf = alloc_ctx;
+   struct vb2_streaming_buf *buf;
+   int err;
+
+   buf = kzalloc(sizeof *buf, GFP_KERNEL);
+   if (!buf)
+   return ERR_PTR(-ENOMEM);
+   buf-vaddr = alloc_pages_exact(size, GFP_KERNEL | GFP_DMA);
+   if (!buf-vaddr) {
+   err = -ENOMEM;
+   goto out;
+   }
+   buf-dma_handle = dma_map_single(conf-dev, buf-vaddr, size,
+DMA_FROM_DEVICE);
+   err = dma_mapping_error(conf-dev, buf-dma_handle);
+   if (err) {
+   dev_err(conf-dev, dma_map_single failed\n);
+
+   free_pages_exact(buf-vaddr, size);
+   buf-vaddr = NULL;
+   goto out_pages;
+   }
+   buf-conf = conf;
+   buf-size = size;
+   buf-handler.refcount = buf-refcount;
+   buf-handler.put = vb2_dma_streaming_put;
+   buf-handler.arg = buf;
+
+   atomic_inc(buf-refcount);
+   return buf;
+
+out_pages:
+   free_pages_exact(buf-vaddr, buf-size);
+out:
+   kfree(buf);
+   return ERR_PTR(err);
+}
+
+static void *vb2_dma_streaming_cookie(void *buf_priv)
+{
+   struct vb2_streaming_buf *buf = buf_priv;
+
+   return buf-dma_handle;
+}
+
+static void *vb2_dma_streaming_vaddr(void *buf_priv)
+{
+   struct vb2_streaming_buf *buf = buf_priv;
+
+   if (!buf)
+   return NULL;
+   return buf-vaddr;
+}
+
+static unsigned 

[PATCH v2 3/4] sta2x11_vip: convert to videobuf2 and control framework

2012-09-21 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 | 1235 ++-
 2 file modificati, 411 inserzioni(+), 826 rimozioni(-)

diff --git a/drivers/media/pci/sta2x11/Kconfig 
b/drivers/media/pci/sta2x11/Kconfig
index 6749f67..654339f 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_STREAMING
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 4c10205..f423039 100644
--- a/drivers/media/pci/sta2x11/sta2x11_vip.c
+++ b/drivers/media/pci/sta2x11/sta2x11_vip.c
@@ -1,6 +1,7 @@
 /*
  * This is the driver for the STA2x11 Video Input Port.
  *
+ * Copyright (C) 2012   ST Microelectronics
  * Copyright (C) 2010   WindRiver Systems, Inc.
  *
  * This program is free software; you can redistribute it and/or modify it
@@ -19,36 +20,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-streaming.h
 
 #include sta2x11_vip.h
 
-#define DRV_NAME sta2x11_vip
 #define DRV_VERSION 1.3
 
 #ifndef PCI_DEVICE_ID_STMICRO_VIP
@@ -63,8 +58,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,44 +79,24 @@
 
 #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_fh {
+   struct v4l2_fh fh;
+};
 struct sta2x11_vip {
struct v4l2_device v4l2_dev;
struct video_device *video_dev;
@@ -129,21 +104,27 @@ struct sta2x11_vip {
struct i2c_adapter *adapter;
unsigned int register_save_area[IRQ_COUNT + SAVE_COUNT + AUX_COUNT];

[PATCH v2 4/4] adv7180: remove {query/g_/s_}ctrl

2012-09-21 Thread Federico Vaga
All drivers which use this subdevice use also the control framework.
The v4l2_subdev_core_ops operations {query/g_/s_}ctrl are useless because
device drivers will inherit the controls from this subdevice.

Signed-off-by: Federico Vaga federico.v...@gmail.com
---
 drivers/media/i2c/adv7180.c | 3 ---
 1 file modificato, 3 rimozioni(-)

diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index 45ecf8d..43bc2b9 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -402,9 +402,6 @@ static const struct v4l2_subdev_video_ops adv7180_video_ops 
= {
 static const struct v4l2_subdev_core_ops adv7180_core_ops = {
.g_chip_ident = adv7180_g_chip_ident,
.s_std = adv7180_s_std,
-   .queryctrl = v4l2_subdev_queryctrl,
-   .g_ctrl = v4l2_subdev_g_ctrl,
-   .s_ctrl = v4l2_subdev_s_ctrl,
 };
 
 static const struct v4l2_subdev_ops adv7180_ops = {
-- 
1.7.11.4

--
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


Re: [PATCH 3/3] mt9v022: set y_skip_top field to zero

2012-09-21 Thread Anatolij Gustschin
Hi Guennadi,

On Tue, 11 Sep 2012 10:55:31 +0200 (CEST)
Guennadi Liakhovetski g.liakhovet...@gmx.de wrote:
...
   On what systems have you checked this?
  
  On camera systems from ifm, both using mt9v022.
 
 Ok, I agree, this was a hack in the beginning, and, probably, there was a 
 reason for the problem, that we've seen, that we didn't find a proper 
 solution to, but I wouldn't like to punish those systems now. The 
 y_skip_top field is only taken into account by the pxa driver, and there 
 is only one pxa270 system, using mt9v022: pcm990-baseboard.c. Could you, 
 please, add platform data to mt9v022 with only one parameter to initialise 
 y_skip_top, use 0 as default and set it to 1 on pcm990-baseboard.c?

Yes, I've reworked this patch as suggested and will resubmit.

Thanks,
Anatolij
--
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


Re: [PATCH 2/3] mt9v022: fix the V4L2_CID_EXPOSURE control

2012-09-21 Thread Anatolij Gustschin
On Fri, 24 Aug 2012 16:32:57 +0200 (CEST)
Guennadi Liakhovetski g.liakhovet...@gmx.de wrote:
...
   But why do we have to write it here at all then? Autoexposure can be off 
   only if the user has set exposure manually, using V4L2_CID_EXPOSURE_AUTO. 
   In this case MT9V022_TOTAL_SHUTTER_WIDTH already contains the correct 
   value. Why do we have to set it again? Maybe just adding a comment, 
   explaining the above, would suffice?
  
  Actually we do not have to write it here, yes. Should I remove the shutter
  register setting here entirely? And add a comment explaining, why?
 
 Remove it from the else clause, yes, please. And a comment would be 
 good!

Ok, I'll resubmit a reworked patch.

Thanks,
Anatolij
--
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


Re: [RFC] Timestamps and V4L2

2012-09-21 Thread Hans Verkuil
On Thu September 20 2012 22:21:22 Sakari Ailus wrote:
 Hi all,
 
 
 This RFC intends to summarise and further the recent discussion on
 linux-media regarding the proposed changes of timestamping V4L2 buffers.
 
 
 The problem
 ===
 
 The V4L2 has long used realtime timestamps (such as
 clock_gettime(CLOCK_REALTIME, ...)) to stamp the video buffers before
 handing them over to the user. This has been found problematic in
 associating the video buffers with data from other sources: realtime clock
 may jump around due to daylight saving time, for example, and ALSA
 (audio-video synchronisation is a common use case) user space API does not
 provide the user with realtime timestamps, but instead uses monotonic time
 (i.e. clock_gettime(CLOCK_MONOTONIC, ...)).
 
 This is especially an issue in embedded systems where video recording is a
 common use case. Drivers typically used in such systems have silently
 switched to use monotonic timestamps. While against the spec, this is
 necessary for those systems to operate properly.
 
 In general, realtime timestamps are seen of little use in other than
 debugging purposes, but monotonic timestamps are fine for that as well. It's
 still possible that an application I'm not aware of uses them in a peculiar
 way that would be adversely affected by changing to monotonic timestamps.
 Nevertheless, we're not supposed to break the API (or ABI). It'd be also
 very important for the application to know what kind of timestamps are
 provided by the device.
 
 
 Requirements, wishes and constraints
 
 
 Now that it seems to be about the time to fix these issues, it's worth
 looking a little bit to the future to anticipate the coming changes to be
 able to accommodate them better later on.
 
 - The new default should be monotonic. As the monotonic timestamps are seen
 to be the most useful, they should be made the default.
 
 - timeval vs. timespec. The two structs can be used to store timestamp
 information. They are not compatible with each other. It's a little bit
 uncertain what's the case with all the architectures but it looks like the
 timespec fits into the space of timeval in all cases. If timespec is
 considered to be used somewhere the compatibility must be ensured. Timespec
 is better than timeval since timespec has more precision and it's the same
 struct that's used everywhere else in the V4L2 API: timespec does not need
 conversion to timespec in the user space.
 
 struct timespec {
 __kernel_time_t tv_sec; /* seconds */
 longtv_nsec;/* nanoseconds */
 };
 
 struct timeval {
 __kernel_time_t tv_sec; /* seconds */
 __kernel_suseconds_ttv_usec;/* microseconds */
 };
 
 To be able to use timespec, the user would have to most likely explicitly
 choose to do that.
 
 - Users should know what kind of timestamps the device produces. This
 includes existing and future kernels. What should be considered are
 uninformed porting drivers back and forth across kernel versions and
 out-of-date kernel header files.
 
 - Device-dependent timestamps. Some devices such as the uvcvideo ones
 produce device-dependent timestamps for synchronising video and audio, both
 produced by the same physical hardware device. For uvcvideo these timestamps
 are unsigned 32-bit integers.
 
 - There's also another clock, Linux-specific raw monotonic clock (as in
 clock_gettime(CLOCK_RAW_MONOTONIC, ...)) that could be better in some use
 cases than the regular monotonic clock. The difference is that the raw
 monotonic clock is free from the NTP adjustments. It would be nice for the
 user to be able to choose the clock used for timestamps. This is especially
 important for device-dependent timestamps: not all applications can be
 expected to be able to use them.
 
 - The field adjacent to timestamp, timecode, is 128 bits wide, and not used
 by a single driver. This field could be re-used.
 
 
 Possible solutions
 ==
 
 Not all of the solutions below that have been proposed are mutually
 exclusive. That's also what's making the choice difficult: the ultimate
 solution to the issue of timestamping may involve several of these --- or
 possibly something better that's not on the list.
 
 
 Use of timespec
 ---
 
 If we can conclude timespec will always fit into the size of timeval (or
 timecode) we could use timespec instead. The solution should still make
 the use of timespec explicit to the user space. This seems to conflict with
 the idea of making monotonic timestamps the default: the default can't be
 anything incompatible with timeval, and at the same time it's the most
 important that the monotonic timestamps are timespec.

We have to keep timeval. Changing this will break the ABI. I see absolutely
no reason to use timespec for video. At 60 Hz a frame takes 16.67 ms, and that's
far, far removed from ns precisions. Should we 

Re: [PATCH 1/4] v4l: vb2: add prepare/finish callbacks to allocators

2012-09-21 Thread Hans Verkuil
On Fri September 21 2012 11:21:35 Federico Vaga wrote:
 This patch adds support for prepare/finish callbacks in VB2 allocators.
 These callback are used for buffer flushing.
 
 Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
 Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
 Acked-by: Federico Vaga federico.v...@gmail.com
 ---
  drivers/media/v4l2-core/videobuf2-core.c | 11 +++
  include/media/videobuf2-core.h   |  7 +++
  2 file modificati, 18 inserzioni(+)
 
 diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
 b/drivers/media/v4l2-core/videobuf2-core.c
 index 4da3df6..079fa79 100644
 --- a/drivers/media/v4l2-core/videobuf2-core.c
 +++ b/drivers/media/v4l2-core/videobuf2-core.c
 @@ -790,6 +790,7 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum 
 vb2_buffer_state state)
  {
   struct vb2_queue *q = vb-vb2_queue;
   unsigned long flags;
 + unsigned int plane;
  
   if (vb-state != VB2_BUF_STATE_ACTIVE)
   return;
 @@ -800,6 +801,10 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum 
 vb2_buffer_state state)
   dprintk(4, Done processing on buffer %d, state: %d\n,
   vb-v4l2_buf.index, vb-state);
  
 + /* sync buffers */
 + for (plane = 0; plane  vb-num_planes; ++plane)
 + call_memop(q, finish, vb-planes[plane].mem_priv);
 +
   /* Add the buffer to the done buffers list */
   spin_lock_irqsave(q-done_lock, flags);
   vb-state = state;
 @@ -975,9 +980,15 @@ static int __qbuf_mmap(struct vb2_buffer *vb, const 
 struct v4l2_buffer *b)
  static void __enqueue_in_driver(struct vb2_buffer *vb)
  {
   struct vb2_queue *q = vb-vb2_queue;
 + unsigned int plane;
  
   vb-state = VB2_BUF_STATE_ACTIVE;
   atomic_inc(q-queued_count);
 +
 + /* sync buffers */
 + for (plane = 0; plane  vb-num_planes; ++plane)
 + call_memop(q, prepare, vb-planes[plane].mem_priv);
 +
   q-ops-buf_queue(vb);
  }
  
 diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
 index 8dd9b6c..f374f99 100644
 --- a/include/media/videobuf2-core.h
 +++ b/include/media/videobuf2-core.h
 @@ -41,6 +41,10 @@ struct vb2_fileio_data;
   *argument to other ops in this structure
   * @put_userptr: inform the allocator that a USERPTR buffer will no longer
   *be used
 + * @prepare: called everytime the buffer is passed from userspace to the

nitpick: everytime - every time

 + *   driver, usefull for cache synchronisation, optional
 + * @finish:  called everytime the buffer is passed back from the driver

ditto.

 + *   to the userspace, also optional
   * @vaddr:   return a kernel virtual address to a given memory buffer
   *   associated with the passed private structure or NULL if no
   *   such mapping exists
 @@ -65,6 +69,9 @@ struct vb2_mem_ops {
   unsigned long size, int write);
   void(*put_userptr)(void *buf_priv);
  
 + void(*prepare)(void *buf_priv);
 + void(*finish)(void *buf_priv);
 +
   void*(*vaddr)(void *buf_priv);
   void*(*cookie)(void *buf_priv);
  
 
--
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


Re: [PATCH v2 3/4] sta2x11_vip: convert to videobuf2 and control framework

2012-09-21 Thread Hans Verkuil
On Fri September 21 2012 11:21:37 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)

Some more comments below, nothing major.

 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 | 1235 
 ++-
  2 file modificati, 411 inserzioni(+), 826 rimozioni(-)
 
 diff --git a/drivers/media/pci/sta2x11/Kconfig 
 b/drivers/media/pci/sta2x11/Kconfig
 index 6749f67..654339f 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_STREAMING
   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 4c10205..f423039 100644
 --- a/drivers/media/pci/sta2x11/sta2x11_vip.c
 +++ b/drivers/media/pci/sta2x11/sta2x11_vip.c
 @@ -1,6 +1,7 @@
  /*
   * This is the driver for the STA2x11 Video Input Port.
   *
 + * Copyright (C) 2012   ST Microelectronics
   * Copyright (C) 2010   WindRiver Systems, Inc.
   *
   * This program is free software; you can redistribute it and/or modify it
 @@ -19,36 +20,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-streaming.h
  
  #include sta2x11_vip.h
  
 -#define DRV_NAME sta2x11_vip
  #define DRV_VERSION 1.3
  
  #ifndef PCI_DEVICE_ID_STMICRO_VIP
 @@ -63,8 +58,8 @@
  #define DVP_TFS  0x08
  #define DVP_BFO  0x0C
  #define DVP_BFS  0x10
 -#define DVP_VTP 0x14
 -#define DVP_VBP 0x18
 +#define DVP_VTP  0x14
 +#define DVP_VBP  0x18
  #define DVP_VMP  0x1C
  #define DVP_ITM  0x98
  #define DVP_ITS  0x9C
 @@ -84,44 +79,24 @@
  
  #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_fh {
 + struct v4l2_fh fh;
 +};

No need to make a sta2x11_vip_fh struct, just use v4l2_fh directly. It 

Re: [PATCH 1/4] v4l: vb2: add prepare/finish callbacks to allocators

2012-09-21 Thread Federico Vaga
  
  + * @prepare:   called everytime the buffer is passed from userspace
  to the
 nitpick: everytime - every time
 
  + * driver, usefull for cache synchronisation, optional
  + * @finish:called everytime the buffer is passed back from the
  driver
 ditto.
 

This patch come from here: https://patchwork.kernel.org/patch/1323411/

I send it with my patch set because my work require this patch but it is 
not in the next tree. I think it is convenient to fix the original 
patch, probably it will be integrated in the kernel before this one; so 
this patch will be useless.

Anyway, I will apply this comment fix.

-- 
Federico Vaga
--
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


Re: [RFCv1 PATCH 4/6] videobuf2-core: fill in length field for multiplanar buffers.

2012-09-21 Thread Hans Verkuil
On Wed September 19 2012 16:37:38 Hans Verkuil wrote:
 From: Hans Verkuil hans.verk...@cisco.com
 
 length should be set to num_planes in __fill_v4l2_buffer(). That way the
 caller knows how many planes there are in the buffer.

Can someone review this? I'd like to know whether it will cause problems
for existing applications to set the length back to the actual number of
planes, and whether it makes sense at all to do so. I believe it does, but
I don't know if anyone is using the current behavior.

Note that the documentation currently doesn't specify what will happen with
length.

Since the only drivers implementing multiplanar support are Samsung drivers,
I assume that Samsung will know best whether this change might cause problems.

Regards,

Hans

 
 Signed-off-by: Hans Verkuil hans.verk...@cisco.com
 ---
  drivers/media/v4l2-core/videobuf2-core.c |1 +
  1 file changed, 1 insertion(+)
 
 diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
 b/drivers/media/v4l2-core/videobuf2-core.c
 index 929cc99..bbfe022 100644
 --- a/drivers/media/v4l2-core/videobuf2-core.c
 +++ b/drivers/media/v4l2-core/videobuf2-core.c
 @@ -348,6 +348,7 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb, 
 struct v4l2_buffer *b)
* Fill in plane-related data if userspace provided an array
* for it. The memory and size is verified above.
*/
 + b-length = q-num_planes;
   memcpy(b-m.planes, vb-v4l2_planes,
   b-length * sizeof(struct v4l2_plane));
   } else {
 
--
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


s5p-tv/mixer_video.c weirdness

2012-09-21 Thread Hans Verkuil
Hi Marek, Sylwester,

I've been investigating how multiplanar is used in various drivers, and I
came across this driver that is a bit weird.

querycap sets both single and multiple planar output caps:

cap-capabilities = V4L2_CAP_STREAMING |
V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_VIDEO_OUTPUT_MPLANE;

This suggests that both the single and multiplanar APIs are supported.

But mxr_ioctl_ops only implements these:

/* format handling */
.vidioc_enum_fmt_vid_out = mxr_enum_fmt,
.vidioc_s_fmt_vid_out_mplane = mxr_s_fmt,
.vidioc_g_fmt_vid_out_mplane = mxr_g_fmt,

Mixing single planar enum_fmt with multiplanar s/g_fmt makes little sense.

I suspect everything should be multiplanar.

BTW, I recommend running v4l2-compliance over your s5p drivers. I saw several
things it would fail on.

Regards,

Hans
--
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


RFC: single+multiplanar API in one driver: possible or not?

2012-09-21 Thread Hans Verkuil
Hi all,

I've been looking into multiplanar support recently, and I ran into some API
ambiguities.

In the examples below I stick to the capture case, but the same applies to
output and m2m.

There are two capabilities: V4L2_CAP_VIDEO_CAPTURE and 
V4L2_CAP_VIDEO_CAPTURE_MPLANE.
These caps tell the application whether the single and/or multiplanar API is
implemented by the driver.

If the hardware only supports single planar formats, then only 
V4L2_CAP_VIDEO_CAPTURE
is present. If the hardware only supports multiplanar formats, then only
V4L2_CAP_VIDEO_CAPTURE_MPLANE is present. The problems occurs when the hardware
supports both single and multiplanar formats.

The first question is if we want to allow drivers to implement both. The
advantages of that are:

- easy to implement: if the hardware supports one or more multiplanar formats,
  then the driver must implement only the multiplanar API. Applications will
  only see V4L2_CAP_VIDEO_CAPTURE or V4L2_CAP_VIDEO_CAPTURE_MPLANE and never
  both.
- no confusion: what should be done if a multiplanar format is set up
  and an application asks for the current single planar format? Return a
  fake format? Some error? This is currently undefined.

The disadvantages are:

- it won't work with most/all existing applications since they only understand
  single planar at the moment. However, all multiplanar drivers are for Samsung
  embedded SoCs, so is this a real problem?

If we would want to allow mixing the two, then we need to solve two problems:

- Determine the behavior when calling G_FMT for a single planar buffer type
  when the current format is a multiplanar format.
- We probably want to make a bunch of helper functions that do the job of
  handling the single planar case without requiring the driver to actually
  implement both.

The first is actually a major problem. Returning an error here is completely
unexpected behavior. The only reasonable solution I see is to remember the last
single planar format and return that. But then G_FMT for a single or a 
multiplanar
format will return different things.

The second problem is also difficult, in particular when dealing with the
streaming I/O ioctls. It's doable, but a fair amount of work.

A conversion from multiplanar to singleplanar might be something that can be
done in libv4l2. But that too is a substantial amount of work.

I am inclined to disallow mixing of single and multiplanar APIs in a driver.
Let's keep things simple.

Comments?

Hans
--
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


[RFC PATCH 0/3] In non-blocking mode return EAGAIN in hwseek

2012-09-21 Thread Hans Verkuil
This patch series resolves a problem with S_HW_FREQ_SEEK when called in
non-blocking mode. Currently this would actually block during the seek.

This is not a good idea. This patch changes the spec and the drivers to
return -EAGAIN when called in non-blocking mode.

In the future actual support for non-blocking mode might be added to
selected drivers, but that will require a new event (SEEK_READY or something
like that), and I am not convinced it is worth the effort anyway.

Regards,

Hans

--
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


[RFC PATCH 2/3] DocBook: in non-blocking mode return EAGAIN in hwseek

2012-09-21 Thread Hans Verkuil
From: Hans Verkuil hans.verk...@cisco.com

VIDIOC_S_HW_FREQ_SEEK should return EAGAIN when called in non-blocking
mode. This might change in the future if we add support for this in the
future, but right now this is not supported.

Signed-off-by: Hans Verkuil hans.verk...@cisco.com
---
 Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml |   10 ++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml 
b/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml
index 3dd1bec..5b379e7 100644
--- a/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml
@@ -75,6 +75,9 @@ seek is started./para
 
 paraThis ioctl is supported if the 
constantV4L2_CAP_HW_FREQ_SEEK/constant capability is set./para
 
+paraIf this ioctl is called from a non-blocking filehandle, then 
EAGAIN; is
+returned and no seek takes place./para
+
 table pgwide=1 frame=none id=v4l2-hw-freq-seek
   titlestruct structnamev4l2_hw_freq_seek/structname/title
   tgroup cols=3
@@ -158,6 +161,13 @@ fields is wrong./para
/listitem
   /varlistentry
   varlistentry
+   termerrorcodeEAGAIN/errorcode/term
+   listitem
+ paraAttempted to call constantVIDIOC_S_HW_FREQ_SEEK/constant
+ with the filehandle in non-blocking mode./para
+   /listitem
+  /varlistentry
+  varlistentry
termerrorcodeENODATA/errorcode/term
listitem
  paraThe hardware seek found no channels./para
-- 
1.7.10.4

--
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


[RFC PATCH 1/3] DocBook: EAGAIN == EWOULDBLOCK

2012-09-21 Thread Hans Verkuil
From: Hans Verkuil hans.verk...@cisco.com

Merge the two entries since they are one and the same error.

Signed-off-by: Hans Verkuil hans.verk...@cisco.com
---
 Documentation/DocBook/media/v4l/gen-errors.xml |9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/gen-errors.xml 
b/Documentation/DocBook/media/v4l/gen-errors.xml
index 737ecaa..7e29a4e 100644
--- a/Documentation/DocBook/media/v4l/gen-errors.xml
+++ b/Documentation/DocBook/media/v4l/gen-errors.xml
@@ -7,10 +7,12 @@
 tbody valign=top
!-- Keep it ordered alphabetically --
   row
-   entryEAGAIN/entry
+   entryEAGAIN (aka EWOULDBLOCK)/entry
entryThe ioctl can't be handled because the device is in state where
   it can't perform it. This could happen for example in case where
   device is sleeping and ioctl is performed to query statistics.
+  It is also returned when the ioctl would need to wait
+  for an event, but the device was opened in non-blocking mode.
/entry
   /row
   row
@@ -63,11 +65,6 @@
permission, or some special capabilities is needed
(e. g. root)/entry
   /row
-  row
-   entryEWOULDBLOCK/entry
-   entryOperation would block. Used when the ioctl would need to wait
-  for an event, but the device was opened in non-blocking 
mode./entry
-  /row
 /tbody
   /tgroup
 /table
-- 
1.7.10.4

--
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


[RFC PATCH 3/3] radio drivers: in non-blocking mode return EAGAIN in hwseek

2012-09-21 Thread Hans Verkuil
From: Hans Verkuil hans.verk...@cisco.com

VIDIOC_S_HW_FREQ_SEEK should return EAGAIN when called in non-blocking
mode. This might change in the future if we add support for this in the
future, but right now this is not supported.

Signed-off-by: Hans Verkuil hans.verk...@cisco.com
---
 drivers/media/radio/radio-mr800.c|3 +++
 drivers/media/radio/radio-tea5777.c  |3 +++
 drivers/media/radio/radio-wl1273.c   |3 +++
 drivers/media/radio/si470x/radio-si470x-common.c |3 +++
 drivers/media/radio/wl128x/fmdrv_v4l2.c  |3 +++
 5 files changed, 15 insertions(+)

diff --git a/drivers/media/radio/radio-mr800.c 
b/drivers/media/radio/radio-mr800.c
index 3182b26..ec7acfa 100644
--- a/drivers/media/radio/radio-mr800.c
+++ b/drivers/media/radio/radio-mr800.c
@@ -360,6 +360,9 @@ static int vidioc_s_hw_freq_seek(struct file *file, void 
*priv,
if (seek-tuner != 0 || !seek-wrap_around)
return -EINVAL;
 
+   if (file-f_flags  O_NONBLOCK)
+   return -EWOULDBLOCK;
+
retval = amradio_send_cmd(radio,
AMRADIO_SET_SEARCH_LVL, 0, buf, 8, false);
if (retval)
diff --git a/drivers/media/radio/radio-tea5777.c 
b/drivers/media/radio/radio-tea5777.c
index ef82898..50935d5 100644
--- a/drivers/media/radio/radio-tea5777.c
+++ b/drivers/media/radio/radio-tea5777.c
@@ -395,6 +395,9 @@ static int vidioc_s_hw_freq_seek(struct file *file, void 
*fh,
if (a-tuner || a-wrap_around)
return -EINVAL;
 
+   if (file-f_flags  O_NONBLOCK)
+   return -EWOULDBLOCK;
+
if (a-rangelow || a-rangehigh) {
for (i = 0; i  ARRAY_SIZE(bands); i++) {
if (i == BAND_AM  !tea-has_am)
diff --git a/drivers/media/radio/radio-wl1273.c 
b/drivers/media/radio/radio-wl1273.c
index a22ad1c..7ff2b3de 100644
--- a/drivers/media/radio/radio-wl1273.c
+++ b/drivers/media/radio/radio-wl1273.c
@@ -1693,6 +1693,9 @@ static int wl1273_fm_vidioc_s_hw_freq_seek(struct file 
*file, void *priv,
if (seek-tuner != 0 || seek-type != V4L2_TUNER_RADIO)
return -EINVAL;
 
+   if (file-f_flags  O_NONBLOCK)
+   return -EWOULDBLOCK;
+
if (mutex_lock_interruptible(core-lock))
return -EINTR;
 
diff --git a/drivers/media/radio/si470x/radio-si470x-common.c 
b/drivers/media/radio/si470x/radio-si470x-common.c
index 9bb65e1..fd74f5c 100644
--- a/drivers/media/radio/si470x/radio-si470x-common.c
+++ b/drivers/media/radio/si470x/radio-si470x-common.c
@@ -708,6 +708,9 @@ static int si470x_vidioc_s_hw_freq_seek(struct file *file, 
void *priv,
if (seek-tuner != 0)
return -EINVAL;
 
+   if (file-f_flags  O_NONBLOCK)
+   return -EWOULDBLOCK;
+
return si470x_set_seek(radio, seek);
 }
 
diff --git a/drivers/media/radio/wl128x/fmdrv_v4l2.c 
b/drivers/media/radio/wl128x/fmdrv_v4l2.c
index db2248e..8d1c094 100644
--- a/drivers/media/radio/wl128x/fmdrv_v4l2.c
+++ b/drivers/media/radio/wl128x/fmdrv_v4l2.c
@@ -408,6 +408,9 @@ static int fm_v4l2_vidioc_s_hw_freq_seek(struct file *file, 
void *priv,
struct fmdev *fmdev = video_drvdata(file);
int ret;
 
+   if (file-f_flags  O_NONBLOCK)
+   return -EWOULDBLOCK;
+
if (fmdev-curr_fmmode != FM_MODE_RX) {
ret = fmc_set_mode(fmdev, FM_MODE_RX);
if (ret != 0) {
-- 
1.7.10.4

--
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


Re: [RFC] Processing context in the V4L2 subdev and V4L2 controls API ?

2012-09-21 Thread Hans Verkuil
On Tue September 18 2012 17:06:54 Sylwester Nawrocki wrote:
 Hi All,
 
 I'm trying to fulfil following requirements with V4L2 API that are specific
 to most of Samsung camera sensors with embedded SoC ISP and also for local 
 SoC camera ISPs:
 
  - separate pixel format and pixel resolution needs to be configured
in a device for camera preview and capture;
 
  - there is a need to set capture or preview mode in a device explicitly
as it makes various adjustments (in firmware) in each operation mode
and controls external devices accordingly (e.g. camera Flash);
 
  - some devices have more than two use case specific contexts that a user
needs to choose from, e.g. video preview, video capture, still preview, 
still capture; for each of these modes there are separate settings, 
especially pixel resolution and others corresponding to existing v4l2 
controls;
 
  - some devices can have two processing contexts enabled simultaneously,
e.g. a sensor emitting YUYV and JPEG streams simultaneously (please see 
discussion [1]).
 
 This makes me considering making the v4l2 subdev (and maybe v4l2 controls)
 API processing (capture) context aware.
 
 If I remember correctly introducing processing context, as the per file 
 handle device contexts in case of mem-to-mem devices was considered bad
 idea in past discussions.

I don't remember this. Controls can already be per-filehandle for m2m devices,
so for m2m devices I see no problem. For other devices it is a different matter,
though. The current V4L2 API does not allow per-filehandle contexts there.

 But this was more about v4ll2 video nodes.
 
 And I was considering adding context only to v4l2 subdev API, and possibly
 to the (extended) control API. The idea is to extend the subdev (and 
 controls ?) ioctls so it is possible to preconfigure sets of parameters 
 on subdevs, while V4L2 video node parameters would be switched manually
 by applications to match a selected subdevs contest. There would also be
 needed an API to select specific context (e.g. a control), or maybe 
 multiple contexts like in case of a sensor from discussion [1].

We discussed the context idea before. The problem is how to implement it
in a way that still keeps things from becoming overly complex.

What I do not want to see is an API with large structs that contain the whole
context. That's a nightmare to maintain in the long run. So you want to be
able to use the existing API as much as possible and build up the context
bit by bit.

I don't think using a control to select contexts is a good idea. I think this
warrants one or more new ioctls.

What contexts would you need? What context operations do you need?

I would probably define a default or baseline context that all drivers have,
then create a CLONE_CONTEXT ioctl (cloning an existing context into a new one)
and an EDIT_CONTEXT ioctl (to edit an existing context) and any subsequent
ioctls will apply to that context. After the FINISH_CONTEXT ioctl the context
is finalized and any subsequent ioctls will apply again to the baseline context.
With APPLY_CONTEXT you apply a context to the baseline context and activate it.

Whether this context information is stored in the file handle (making it fh
specific) or globally is an interesting question to which I don't have an
answer.

This is just a quick brainstorm, but I think something like this might be
feasible.

 I've seen various hacks in some v4l2 drivers trying to fulfil above
 requirements, e.g. abusing struct v4l2_mbus_framefmt::colorspace field
 to select between capture/preview in a device or using 32-bit integer
 control where upper 16-bits are used for pixel width and lower 16 for
 pixel height.

Where is that? And what do you mean with pixel width and height? It this
used to define a pixel aspect ratio? Is this really related to context?

 This may suggest there something missing at the API.
 
 Any suggestions, critics, please ?... :)

Regards,

Hans
--
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


Re: [RFC PATCH 0/3] In non-blocking mode return EAGAIN in hwseek

2012-09-21 Thread Hans de Goede

Hi,

Looks good, but for patch 3/3 you're missing the same changes to
sound/i2c/other/tea575x-tuner.c

Regards,

Hans


On 09/21/2012 01:44 PM, Hans Verkuil wrote:

This patch series resolves a problem with S_HW_FREQ_SEEK when called in
non-blocking mode. Currently this would actually block during the seek.

This is not a good idea. This patch changes the spec and the drivers to
return -EAGAIN when called in non-blocking mode.

In the future actual support for non-blocking mode might be added to
selected drivers, but that will require a new event (SEEK_READY or something
like that), and I am not convinced it is worth the effort anyway.

Regards,

Hans


--
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


Re: [RFC PATCH 0/3] In non-blocking mode return EAGAIN in hwseek

2012-09-21 Thread Hans Verkuil
On Fri September 21 2012 14:29:03 Hans de Goede wrote:
 Hi,
 
 Looks good, but for patch 3/3 you're missing the same changes to
 sound/i2c/other/tea575x-tuner.c

Thanks! I'll add that as another patch.

I keep forgetting about that one :-)

Regards,

Hans

 On 09/21/2012 01:44 PM, Hans Verkuil wrote:
  This patch series resolves a problem with S_HW_FREQ_SEEK when called in
  non-blocking mode. Currently this would actually block during the seek.
 
  This is not a good idea. This patch changes the spec and the drivers to
  return -EAGAIN when called in non-blocking mode.
 
  In the future actual support for non-blocking mode might be added to
  selected drivers, but that will require a new event (SEEK_READY or something
  like that), and I am not convinced it is worth the effort anyway.
 
  Regards,
 
  Hans
 
 
--
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


Re: [PATCH v4] of: Add videomode helper

2012-09-21 Thread Steffen Trumtrar


On Thu, Sep 20, 2012 at 11:29:42PM +0200, Laurent Pinchart wrote:
 Hi,
 
 (CC'ing the linux-media mailing list, as video modes are of interest there as 
 well)
 
 On Wednesday 19 September 2012 12:19:22 Tomi Valkeinen wrote:
  On Wed, 2012-09-19 at 10:20 +0200, Steffen Trumtrar wrote:
   This patch adds a helper function for parsing videomodes from the
   devicetree. The videomode can be either converted to a struct
   drm_display_mode or a struct fb_videomode.
   
   Signed-off-by: Sascha Hauer s.ha...@pengutronix.de
   Signed-off-by: Steffen Trumtrar s.trumt...@pengutronix.de
   ---
   
   Hi!
   
   changes since v3:
 - print error messages
 - free alloced memory
 - general cleanup
   
   Regards
   Steffen
   
.../devicetree/bindings/video/displaymode  |   74 +
drivers/of/Kconfig |5 +
drivers/of/Makefile|1 +
drivers/of/of_videomode.c  |  283 +++
include/linux/of_videomode.h   |   56 
5 files changed, 419 insertions(+)
create mode 100644 Documentation/devicetree/bindings/video/displaymode
create mode 100644 drivers/of/of_videomode.c
create mode 100644 include/linux/of_videomode.h
   
   diff --git a/Documentation/devicetree/bindings/video/displaymode
   b/Documentation/devicetree/bindings/video/displaymode new file mode
   100644
   index 000..990ca52
   --- /dev/null
   +++ b/Documentation/devicetree/bindings/video/displaymode
   @@ -0,0 +1,74 @@
   +videomode bindings
   +==
   +
   +Required properties:
   + - hactive, vactive: Display resolution
   + - hfront-porch, hback-porch, hsync-len: Horizontal Display timing
   parameters
   +   in pixels
   +   vfront-porch, vback-porch, vsync-len: Vertical display timing
   parameters in
   +   lines
   + - clock: displayclock in Hz
   +
   +Optional properties:
   + - width-mm, height-mm: Display dimensions in mm
   + - hsync-active-high (bool): Hsync pulse is active high
   + - vsync-active-high (bool): Vsync pulse is active high
   + - interlaced (bool): This is an interlaced mode
   + - doublescan (bool): This is a doublescan mode
   +
   +There are different ways of describing a display mode. The devicetree
   representation
   +corresponds to the one commonly found in datasheets for displays.
   +The description of the display and its mode is split in two parts: first
   the display
   +properties like size in mm and (optionally) multiple subnodes with the
   supported modes.
   +
   +Example:
   +
   + display@0 {
   + width-mm = 800;
   + height-mm = 480;
   + modes {
   + mode0: mode@0 {
   + /* 1920x1080p24 */
   + clock = 5200;
   + hactive = 1920;
   + vactive = 1080;
   + hfront-porch = 25;
   + hback-porch = 25;
   + hsync-len = 25;
   + vback-porch = 2;
   + vfront-porch = 2;
   + vsync-len = 2;
   + hsync-active-high;
   + };
   + };
   + };
   +
   +Every property also supports the use of ranges, so the commonly used
   datasheet +description with min typ max-tuples can be used.
   +
   +Example:
   +
   + mode1: mode@1 {
   + /* 1920x1080p24 */
   + clock = 14850;
   + hactive = 1920;
   + vactive = 1080;
   + hsync-len = 0 44 60;
   + hfront-porch = 80 88 95;
   + hback-porch = 100 148 160;
   + vfront-porch = 0 4 6;
   + vback-porch = 0 36 50;
   + vsync-len = 0 5 6;
   + };
   +
   +The videomode can be linked to a connector via phandles. The connector
   has to
   +support the display- and default-mode-property to link to and select a
   mode.
   +
   +Example:
   +
   + hdmi@0012 {
   + status = okay;
   + display = benq;
   + default-mode = mode1;
   + };
   +
   diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
   index dfba3e6..a3acaa3 100644
   --- a/drivers/of/Kconfig
   +++ b/drivers/of/Kconfig
   @@ -83,4 +83,9 @@ config OF_MTD
   
 depends on MTD
 def_bool y
   
   +config OF_VIDEOMODE
   + def_bool y
   + help
   +   helper to parse videomodes from the devicetree
   +
   
endmenu # OF
   
   diff --git a/drivers/of/Makefile b/drivers/of/Makefile
   index e027f44..80e6db3 100644
   --- a/drivers/of/Makefile
   +++ b/drivers/of/Makefile
   @@ -11,3 +11,4 @@ obj-$(CONFIG_OF_MDIO)   += of_mdio.o
   
obj-$(CONFIG_OF_PCI) += of_pci.o
obj-$(CONFIG_OF_PCI_IRQ)  += of_pci_irq.o
obj-$(CONFIG_OF_MTD) += of_mtd.o
   
   +obj-$(CONFIG_OF_VIDEOMODE)   += of_videomode.o
   diff --git a/drivers/of/of_videomode.c b/drivers/of/of_videomode.c
   new file mode 

Re: [PATCH] Mygica X8507 audio for YPbPr, AV and S-Video

2012-09-21 Thread Alfredo Jesús Delaiti
Hi 

El 19/09/12 13:28, Mauro Carvalho Chehab escribió:
 Em 19-09-2012 10:31, Alfredo Jesús Delaiti escreveu:
 El 18/09/12 13:16, Mauro Carvalho Chehab escribió:
 Em 03-09-2012 17:14, Alfredo Jesús Delaiti escreveu:
 Hi

 This patch add audio support for input YPbPr, AV and S-Video for Mygica 
 X8507 card.
 I tried it with the 3.4 and 3.5 kernel

 Remains to be done: IR, FM and ISDBT

 Sorry if I sent the patch improperly.

 Signed-off-by: Alfredo J. Delaiti alfredodela...@netscape.net



 I resubmit the patch with the advice given.
 I apologize, but git and diff, still  aren't my friends

 Thanks


 Signed-off-by: Alfredo J. Delaiti alfredodela...@netscape.net

 diff --git a/drivers/media/pci/cx23885/cx23885-cards.c 
 b/drivers/media/pci/cx23885/cx23885-cards.c
 index d889bd2..cb5f847 100644
 --- a/drivers/media/pci/cx23885/cx23885-cards.c
 +++ b/drivers/media/pci/cx23885/cx23885-cards.c
 @@ -530,42 +530,45 @@ struct cx23885_board cx23885_boards[] = {
 [CX23885_BOARD_MYGICA_X8507] = {
 .name   = Mygica X8507,
 .tuner_type = TUNER_XC5000,
 .tuner_addr = 0x61,
 .tuner_bus  = 1,
 .porta  = CX23885_ANALOG_VIDEO,
 .input  = {
 {
 .type   = CX23885_VMUX_TELEVISION,
 .vmux   = CX25840_COMPOSITE2,
 .amux   = CX25840_AUDIO8,
 },
 {
 .type   = CX23885_VMUX_COMPOSITE1,
 .vmux   = CX25840_COMPOSITE8,
 +   .amux   = CX25840_AUDIO7,
 },
 {
 .type   = CX23885_VMUX_SVIDEO,
 .vmux   = CX25840_SVIDEO_LUMA3 |
 CX25840_SVIDEO_CHROMA4,
 
 It seems that your emailer is not your friend too - it broke your patch ;)

Grrr. You are right, mail setup is wron
I applied this change:/usr/src/linux/Documentation/email-clients.txt., again
I proved before send email and is ok.

 
 Please, be sure to not let your email break long lines like that, or the
 patch won't apply.
 
 IMO, the better is to submit patches with git send-email. There are some 
 examples
 at git-send-email manpage on how to use it with an smart host, if your 
 sendmail
 is not configured for that.
 
 Regards,
 Mauro.
 

diff --git a/drivers/media/pci/cx23885/cx23885-cards.c 
b/drivers/media/pci/cx23885/cx23885-cards.c
index d889bd2..cb5f847 100644
--- a/drivers/media/pci/cx23885/cx23885-cards.c
+++ b/drivers/media/pci/cx23885/cx23885-cards.c
@@ -530,42 +530,45 @@ struct cx23885_board cx23885_boards[] = {
[CX23885_BOARD_MYGICA_X8507] = {
.name   = Mygica X8507,
.tuner_type = TUNER_XC5000,
.tuner_addr = 0x61,
.tuner_bus  = 1,
.porta  = CX23885_ANALOG_VIDEO,
.input  = {
{
.type   = CX23885_VMUX_TELEVISION,
.vmux   = CX25840_COMPOSITE2,
.amux   = CX25840_AUDIO8,
},
{
.type   = CX23885_VMUX_COMPOSITE1,
.vmux   = CX25840_COMPOSITE8,
+   .amux   = CX25840_AUDIO7,
},
{
.type   = CX23885_VMUX_SVIDEO,
.vmux   = CX25840_SVIDEO_LUMA3 |
CX25840_SVIDEO_CHROMA4,
+   .amux   = CX25840_AUDIO7,
},
{
.type   = CX23885_VMUX_COMPONENT,
.vmux   = CX25840_COMPONENT_ON |
CX25840_VIN1_CH1 |
CX25840_VIN6_CH2 |
CX25840_VIN7_CH3,
+   .amux   = CX25840_AUDIO7,
},
},
},
[CX23885_BOARD_TERRATEC_CINERGY_T_PCIE_DUAL] = {
.name   = TerraTec Cinergy T PCIe Dual,
.portb  = CX23885_MPEG_DVB,
.portc  = CX23885_MPEG_DVB,
},
[CX23885_BOARD_TEVII_S471] = {
.name   = TeVii S471,
.portb  = CX23885_MPEG_DVB,
}
 };
 const unsigned int cx23885_bcount = ARRAY_SIZE(cx23885_boards);
 
diff --git a/drivers/media/pci/cx23885/cx23885-video.c 
b/drivers/media/pci/cx23885/cx23885-video.c
index 22f8e7f..fcb3f22 100644
--- a/drivers/media/pci/cx23885/cx23885-video.c
+++ 

Re: tda8290 regression fix

2012-09-21 Thread Mauro Carvalho Chehab
Em 21-09-2012 10:59, Anders Eriksson escreveu:
 
 
 On Wed, Sep 19, 2012 at 9:25 PM, Anders Thomson aerikss...@gmail.com 
 mailto:aerikss...@gmail.com wrote:
 
 diff --git a/drivers/media/common/tuners/tda8290.c 
 b/drivers/media/common/tuners/tda8290.c
 index 064d14c..498cc7b 100644
 --- a/drivers/media/common/tuners/__tda8290.c
 +++ b/drivers/media/common/tuners/__tda8290.c
 @@ -635,7 +635,11 @@ static int tda829x_find_tuner(struct dvb_frontend 
 *fe)
 
 
 dvb_attach(tda827x_attach, fe, priv-tda827x_addr,
priv-i2c_props.adap, priv-cfg);
 +   tuner_info(ANDERS: setting switch_addr. was 0x%02x, new 
 0x%02x\n,priv-cfg.switch___addr,priv-i2c_props.addr);
 priv-cfg.switch_addr = priv-i2c_props.addr;
 +   priv-cfg.switch_addr = 0xc2 / 2;
 +   tuner_info(ANDERS: new 
 0x%02x\n,priv-cfg.switch___addr);
 +
 }
 if (fe-ops.tuner_ops.init)
 fe-ops.tuner_ops.init(fe);
 It needs to be filled with 0xc2 / 2. I'm not sure where I got that 
 expression from, but it is the sum of my efforts tracing code changes around 
 2.6.26.
 
 
   Whereas to work, I need:
   anders@tv /usr/src/linux $ grep ANDERS /3.3.8-d.patched
   [6.565254] tda829x 5-004b: ANDERS: setting switch_addr. was 
 0x00, new 0x4b
 
 What looks weird here is that the device number changed from 4 to 5.
 
 
 If you're in doubt, you could add an extra printk at the 
 initialization code,
 in order to see what's happening there.
 
 Not sure I follow here. Which code should set the ox61 address? I'd be 
 more than happy to add printks. Where? I recall getting lost in how this 
 stuff uses the i2c code in the past.
 
 
 Hi,
 Dusting off my memory, I realized that there was a thread on this way back 
 when I discovered the breakage:
 You can find the archived thread here:
 http://www.mail-archive.com/linux-media@vger.kernel.org/msg05332.html
 
 Still puzzled as to how I can pursue this further.

Are you using the active antena that came with this device[1]?


[1] http://www.mail-archive.com/linux-media@vger.kernel.org/msg07234.html

 
 -Anders

--
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


RE: led flash driver for lm3554/lm3556

2012-09-21 Thread Jeong, Daniel

Hi Andy and Sakari. 

On Fri, Sep 21, 2012 at 01:55:15PM +0300, Andy Shevchenko wrote:
 On Fri, 2012-09-21 at 19:16 +0900, gshark wrote:
 
 [cut previous stories]
 
  My development enviroments are Unbuntu and Android on Exynos and 
  OMAP4 Processor.
  There is a function Assistive Light in the Android mobile phone 
  and it turns on/off the Torch regardless Camera. It is controlled by SW.
  And the indicator function in LM3554/6 can be controlled by SW. One 
  of our customers controls the indicator using SW.
 Fortunately our driver is dedicated for Android as well.
 
  I didn't know you did create the as3645a driver.
  But TI has similar products such as LM3554, 3555, 3556, 3559.. I 
  think we don't need to create drivers for each product.
  Now I'm doing put these products into one driver file. (lm355x.c )
 Actually accordingly to the specs lm3554 and lm3555 is quite different 
 by hw configuration. It's better to keep them separate from my p.o.v.

I agree with that. These chis are so simple it's easy to complicate the matter 
by trying to fit everything into the same driver. It'd be different if some of 
them would be just super or subsets of another chip.

Is it really quite different? 
   Input pins
--- 
 SCL/SDA  STROBE   TORCHHWENTX  
  
LM3554   O   O   O   O   O  
LM3555   O   O   O   O   X  
LM3556   O   O   O   O   O  
LM3559   O   O   O   O   O  

  Output pins
  -   
  LED(1) LED2  LEDI/NTC
 
LM3554  O  O  O
LM3555  O  X  X
LM3556  O  X  X
LM3559  O  O  O


   Operation Mode(ENABLE) bits size
 
LM3554  2bits
LM3555  2bits
LM3556  2bits
LM3559  2bits

   Torch Current Control bits size
 
LM3554  3bits
LM3555  3bits
LM3556  3bits
LM3559  3bits

   Flash Current Control bits size
 
LM3554  4bits
LM3555  4bits
LM3556  4bits
LM3559  4bits


   Indicator Current Control bits size
 
LM3554  2bits
LM3555  2bits
LM3556  2bits
LM3559  3bits

I'm sure that only register numbers are different and it can be handled in one 
driver.  

  [cut previous stories]
  I agree with you that we shouldn't have multiple drivers for the same 
  hardware.
  So I think the best way to avoid duplicated work is to change current 
  lm355x file to like below.
  kernel
  ../drivers/mfd/lm355x-core.c//  control  i2c-accss etc..
/media/video/lm355x-flash.c   //  control flash and torch.
/leds/leds-lm355x.c// control indicator 
  and torch.
 It doesn't require to split them. If you want to provide led framework 
 for that chip we could do the generic glue driver. And I would like to 
 do it.
 
 P.S. Any objections to go to the public mailing list with this 
 discussion? I mean linux-media@, for example.

I'm all for that. What's the relevant list for the LED framework?

  On Fri, 2012-09-21 at 15:06 +0300, Sakari Ailus wrote: 
  P.S. Any objections to go to the public mailing list with this 
  discussion? I mean linux-media@, for example.
 
 I'm all for that. What's the relevant list for the LED framework?
linux-l...@vger.kernel.org, surprise, surprise!

These chips are not only for Camera but also for Audio amp etc. 
You guys are focusing Camera but Audio can use this chip, actually some 
manufacturer's audio module uses this indicator. IMO these LED driver chips 
have multi-function, Flash, Torch and Indicator. Up to now Flash(Strobe) 
function has been dedicated to Camera but Torch and Indicator functions are 
used by the others, Audio etc. I think it is relevant to LED framework and it 
should split into three part. And lm3555 don't need to merged with lm355x since 
there is as3645a driver. 

Daniel. 
Regards. 
Texas Instruments. 

--
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


Re: [RFCv1 PATCH 4/6] videobuf2-core: fill in length field for multiplanar buffers.

2012-09-21 Thread Sylwester Nawrocki
Hi Hans,

On 09/19/2012 04:37 PM, Hans Verkuil wrote:
 From: Hans Verkuil hans.verk...@cisco.com
 
 length should be set to num_planes in __fill_v4l2_buffer(). That way the
 caller knows how many planes there are in the buffer.
 
 Signed-off-by: Hans Verkuil hans.verk...@cisco.com

I think this would break VIDIOC_CREATE_BUFS. We need per buffer num_planes.
Consider a use case where device is streaming with 2-planar pixel format
and we invoke VIDIOC_CREATE_BUFS with single-planar format. On a single 
queue there will be buffers with different number of planes. The number of 
planes information must be attached to a buffer, otherwise VIDIOC_QUERYBUF 
won't work.

 ---
  drivers/media/v4l2-core/videobuf2-core.c |1 +
  1 file changed, 1 insertion(+)
 
 diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
 b/drivers/media/v4l2-core/videobuf2-core.c
 index 929cc99..bbfe022 100644
 --- a/drivers/media/v4l2-core/videobuf2-core.c
 +++ b/drivers/media/v4l2-core/videobuf2-core.c
 @@ -348,6 +348,7 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb, 
 struct v4l2_buffer *b)
* Fill in plane-related data if userspace provided an array
* for it. The memory and size is verified above.
*/
 + b-length = q-num_planes;
   memcpy(b-m.planes, vb-v4l2_planes,
   b-length * sizeof(struct v4l2_plane));
   } else {

Regards,
Sylwester
--
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


Re: [RFCv1 PATCH 4/6] videobuf2-core: fill in length field for multiplanar buffers.

2012-09-21 Thread Hans Verkuil
On Fri September 21 2012 18:13:20 Sylwester Nawrocki wrote:
 Hi Hans,
 
 On 09/19/2012 04:37 PM, Hans Verkuil wrote:
  From: Hans Verkuil hans.verk...@cisco.com
  
  length should be set to num_planes in __fill_v4l2_buffer(). That way the
  caller knows how many planes there are in the buffer.
  
  Signed-off-by: Hans Verkuil hans.verk...@cisco.com
 
 I think this would break VIDIOC_CREATE_BUFS. We need per buffer num_planes.
 Consider a use case where device is streaming with 2-planar pixel format
 and we invoke VIDIOC_CREATE_BUFS with single-planar format. On a single 
 queue there will be buffers with different number of planes. The number of 
 planes information must be attached to a buffer, otherwise VIDIOC_QUERYBUF 
 won't work.

That's a very good point and one I need to meditate on.

However, your comment applies to patch 1/6, not to this one.
This patch is about whether or not the length field of v4l2_buffer should
be filled in with the actual number of planes used by that buffer or not.

Regards,

Hans

 
  ---
   drivers/media/v4l2-core/videobuf2-core.c |1 +
   1 file changed, 1 insertion(+)
  
  diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
  b/drivers/media/v4l2-core/videobuf2-core.c
  index 929cc99..bbfe022 100644
  --- a/drivers/media/v4l2-core/videobuf2-core.c
  +++ b/drivers/media/v4l2-core/videobuf2-core.c
  @@ -348,6 +348,7 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb, 
  struct v4l2_buffer *b)
   * Fill in plane-related data if userspace provided an array
   * for it. The memory and size is verified above.
   */
  +   b-length = q-num_planes;
  memcpy(b-m.planes, vb-v4l2_planes,
  b-length * sizeof(struct v4l2_plane));
  } else {
 
 Regards,
 Sylwester
 --
 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
 
--
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


media_build directory error after patch dvb_frontend: implement suspend / resume

2012-09-21 Thread Jan Hoogenraad
I try to compile the media_build on an old Ubuntu Lucid system.
2.6.32-42-generic-pae #96-Ubuntu SMP Wed Aug 15 19:12:17 UTC 2012 i686
GNU/Linux

The make job stops with

/home/jhh/dvb/media_build/v4l/dvb_usb_core.c: In function
'dvb_usb_data_complete_raw':
/home/jhh/dvb/media_build/v4l/dvb_usb_core.c:224: error: implicit
declaration of function 'dvb_dmx_swfilter_raw'
/home/jhh/dvb/media_build/v4l/dvb_usb_core.c: In function
'dvb_usbv2_suspend':
/home/jhh/dvb/media_build/v4l/dvb_usb_core.c:975: error: implicit
declaration of function 'dvb_frontend_suspend'
/home/jhh/dvb/media_build/v4l/dvb_usb_core.c: In function
'dvb_usbv2_resume_common':
/home/jhh/dvb/media_build/v4l/dvb_usb_core.c:994: error: implicit
declaration of function 'dvb_frontend_resume'

This probably started after
http://article.gmane.org/gmane.linux.drivers.video-input-infrastructure/52430/match=dvb_frontend_resume

The media_build tree is confused with too many files with the same name:

find linux -name \*front\* -ls
54788964   24 -rw-r--r--   1 jhh  jhh 12642 Aug 16 05:45
linux/drivers/media/dvb-core/dvb_frontend.h
54788973   80 -rw-r--r--   1 jhh  jhh 71787 Sep 14 05:45
linux/drivers/media/dvb-core/dvb_frontend.c
54397078   20 -rw-r--r--   1 jhh  jhh 12056 Sep 16 20:20
linux/drivers/media/dvb/dvb-core/dvb_frontend.h
54397083   68 -rw-r--r--   1 jhh  jhh 60558 Jun  8  2011
linux/drivers/media/dvb/dvb-core/dvb_frontend.c
5439   24 -rw-r--r--   1 jhh  jhh 13012 Aug 14 05:45
linux/include/linux/dvb/frontend.h

The frontend.h and .c chosen in the build are older ones
 find v4l -name \*front\* -ls

54135379   76 -rw-r--r--   1 jhh  jhh 67636 Sep 21 17:47
v4l/.dvb_frontend.o.cmd
541339200 lrwxrwxrwx   1 jhh  jhh50 Sep 21 18:13
v4l/dvb_frontend.h - ../linux/drivers/media/dvb/dvb-core/dvb_frontend.h
541339270 lrwxrwxrwx   1 jhh  jhh50 Sep 21 18:13
v4l/dvb_frontend.c - ../linux/drivers/media/dvb/dvb-core/dvb_frontend.c
54136025  184 -rw-r--r--   1 jhh  jhh179558 Sep 21 17:47
v4l/dvb_frontend.o

Do you know what to do about this situation ?
--
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


Re: [PATCH 1/3] Add a core driver for SI476x MFD

2012-09-21 Thread andrey.smir...@convergeddevices.net
On 09/21/2012 12:31 AM, Hans Verkuil wrote:
 On Fri September 21 2012 03:05:41 andrey.smir...@convergeddevices.net wrote:
 On 09/13/2012 11:44 PM, Hans Verkuil wrote:
 Hi Andrey!

 Thanks for posting this driver. One request for the future: please split 
 this
 patch up in smaller pieces: one for each c source for example. That makes it
 easier to review.
 Will do for next version.

 +
 +/**
 + * __core_send_command() - sends a command to si476x and waits its
 + * response
 + * @core:si476x_device structure for the device we are
 + *communicating with
 + * @command:  command id
 + * @args: command arguments we are sending
 + * @argn: actual size of @args
 + * @response: buffer to place the expected response from the device
 + * @respn:actual size of @response
 + * @usecs:amount of time to wait before reading the response (in
 + *usecs)
 + *
 + * Function returns 0 on succsess and negative error code on
 + * failure
 + */
 +static int __core_send_command(struct si476x_core *core,
 +   const u8 command,
 +   const u8 args[],
 +   const int argn,
 +   u8 resp[],
 +   const int respn,
 +   const int usecs)
 +{
 +   struct i2c_client *client = core-client;
 +   int err;
 +   u8  data[CMD_MAX_ARGS_COUNT + 1];
 +
 +   if (argn  CMD_MAX_ARGS_COUNT) {
 +   err = -ENOMEM;
 +   goto exit;
 Why goto exit? There is no clean up after the exit label, so just return
 immediately. Ditto for all the other goto exit's in this function.
 To have only just on point of exit from the function that's just
 personal coding style preference.
 There are no technical reasons behind that, I can change that.

 +  }
 +
 +  if (!client-adapter) {
 +  err = -ENODEV;
 +  goto exit;
 +  }
 +
 +  /* First send the command and its arguments */
 +  data[0] = command;
 +  memcpy(data[1], args, argn);
 +  DBG_BUFFER(client-dev, Command:\n, data, argn + 1);
 +
 +  err = si476x_i2c_xfer(core, SI476X_I2C_SEND, (char *) data, argn + 1);
 +  if (err != argn + 1) {
 +  dev_err(core-client-dev,
 +  Error while sending command 0x%02x\n,
 +  command);
 +  err = (err = 0) ? -EIO : err;
 +  goto exit;
 +  }
 +  /* Set CTS to zero only after the command is send to avoid
 +   * possible racing conditions when working in polling mode */
 +  atomic_set(core-cts, 0);
 +
 +  if (!wait_event_timeout(core-command,
 +  atomic_read(core-cts),
 +  usecs_to_jiffies(usecs) + 1))
 +  dev_warn(core-client-dev,
 +   (%s) [CMD 0x%02x] Device took too much time to 
 answer.\n,
 +   __func__, command);
 +
 +  /*
 +When working in polling mode, for some reason the tuner will
 +report CTS bit as being set in the first status byte read,
 +but all the consequtive ones will return zros until the
 +tuner is actually completed the POWER_UP command. To
 +workaround that we wait for second CTS to be reported
 +   */
 +  if (unlikely(!core-client-irq  command == CMD_POWER_UP)) {
 +  if (!wait_event_timeout(core-command,
 +  atomic_read(core-cts),
 +  usecs_to_jiffies(usecs) + 1))
 +  dev_warn(core-client-dev,
 +   (%s) Power up took too much time.\n,
 +   __func__);
 +  }
 +
 +  /* Then get the response */
 +  err = si476x_i2c_xfer(core, SI476X_I2C_RECV, resp, respn);
 +  if (err != respn) {
 +  dev_err(core-client-dev,
 +  Error while reading response for command 0x%02x\n,
 +  command);
 +  err = (err = 0) ? -EIO : err;
 +  goto exit;
 +  }
 +  DBG_BUFFER(client-dev, Response:\n, resp, respn);
 +
 +  err = 0;
 +
 +  if (resp[0]  SI476X_ERR) {
 +  dev_err(core-client-dev, Chip set error flag\n);
 +  err = si476x_core_parse_and_nag_about_error(core);
 +  goto exit;
 +  }
 +
 +  if (!(resp[0]  SI476X_CTS))
 +  err = -EBUSY;
 +exit:
 +  return err;
 +}
 +
 +#define CORE_SEND_COMMAND(core, cmd, args, resp, timeout) \
 +  __core_send_command(core, cmd, args,\
 +  ARRAY_SIZE(args),   \
 +  resp, ARRAY_SIZE(resp), \
 +  timeout)
 +
 +
 +static int __cmd_tune_seek_freq(struct si476x_core *core,
 +  uint8_t cmd,
 +  const uint8_t args[], size_t argn,
 +  uint8_t *resp, size_t respn,
 +  int (*clear_stcint) (struct si476x_core *core))
 +{
 +  int err;
 +
 +  atomic_set(core-stc, 0);
 +  err = __core_send_command(core, cmd, args, argn,
 +  

Re: [RFCv1 PATCH 6/6] DocBook: various updates w.r.t. v4l2_buffer and multiplanar.

2012-09-21 Thread Sylwester Nawrocki
On 09/19/2012 04:37 PM, Hans Verkuil wrote:
 From: Hans Verkuil hans.verk...@cisco.com
 
 Clarify the behavior of v4l2_buffer in the multiplanar case,
 including fixing a false statement: you can't set m.planes to NULL
 when calling DQBUF.
 
 Signed-off-by: Hans Verkuil hans.verk...@cisco.com

I'm not sure what was main argument for not requiring applications to 
pass the planes array in to VIDIOC_DQBUF. Maybe, to avoid copying the 
planes array so performance is improved when applications don't need 
all information from struct v4l2_plane after each DQBUF. So I'm not 
sure about DQBUF. But improving querybuf sounds like a reasonable thing 
to do. So far num_planes have had to be passed in by applications, as 
returned from S_FMT/G_FMT. There seems to be no way to verify how many
planes are valid based on VIDIOC_QUERYBUF output.

Anyway, both changes shouldn't be a big deal for existing applications. 
They should just make sure the pointer to the planes array is passed, 
which most probably already do. I'm OK with this change, it shouldn't 
be a big issue for applications using s5p drivers. 

 ---
  Documentation/DocBook/media/v4l/io.xml  |6 --
  Documentation/DocBook/media/v4l/vidioc-qbuf.xml |3 +--
  Documentation/DocBook/media/v4l/vidioc-querybuf.xml |   11 +++
  3 files changed, 12 insertions(+), 8 deletions(-)
 
 diff --git a/Documentation/DocBook/media/v4l/io.xml 
 b/Documentation/DocBook/media/v4l/io.xml
 index 1885cc0..c6d39fe 100644
 --- a/Documentation/DocBook/media/v4l/io.xml
 +++ b/Documentation/DocBook/media/v4l/io.xml
 @@ -677,8 +677,10 @@ memory, set by the application. See xref 
 linkend=userp / for details.
   entrystructfieldlength/structfield/entry
   entry/entry
   entrySize of the buffer (not the payload) in bytes for the
 - single-planar API. For the multi-planar API should contain the
 - number of elements in the structfieldplanes/structfield array.
 + single-planar API. For the multi-planar API the application sets
 + this to the number of elements in the 
 structfieldplanes/structfield
 + array. The driver will fill in the actual number of valid elements 
 in
 + that array.
   /entry
 /row
 row
 diff --git a/Documentation/DocBook/media/v4l/vidioc-qbuf.xml 
 b/Documentation/DocBook/media/v4l/vidioc-qbuf.xml
 index 6a821a6..2d37abe 100644
 --- a/Documentation/DocBook/media/v4l/vidioc-qbuf.xml
 +++ b/Documentation/DocBook/media/v4l/vidioc-qbuf.xml
 @@ -121,8 +121,7 @@ remaining fields or returns an error code. The driver may 
 also set
  field. It indicates a non-critical (recoverable) streaming error. In such 
 case
  the application may continue as normal, but should be aware that data in the
  dequeued buffer might be corrupted. When using the multi-planar API, the
 -planes array does not have to be passed; the 
 structfieldm.planes/structfield
 -member must be set to NULL in that case./para
 +planes array must be passed in as well./para
  
  paraBy default constantVIDIOC_DQBUF/constant blocks when no
  buffer is in the outgoing queue. When the
 diff --git a/Documentation/DocBook/media/v4l/vidioc-querybuf.xml 
 b/Documentation/DocBook/media/v4l/vidioc-querybuf.xml
 index 6e414d7..a597155 100644
 --- a/Documentation/DocBook/media/v4l/vidioc-querybuf.xml
 +++ b/Documentation/DocBook/media/v4l/vidioc-querybuf.xml
 @@ -48,8 +48,8 @@
refsect1
  titleDescription/title
  
 -paraThis ioctl is part of the link linkend=mmapmemory
 -mapping/link I/O method. It can be used to query the status of a
 +paraThis ioctl is part of the link linkend=mmapstreaming
 +/link I/O method. It can be used to query the status of a
  buffer at any time after buffers have been allocated with the
  VIDIOC-REQBUFS; ioctl./para
  
 @@ -71,6 +71,7 @@ the structure./para
  
  paraIn the structfieldflags/structfield field the
  constantV4L2_BUF_FLAG_MAPPED/constant,
 +constantV4L2_BUF_FLAG_PREPARED/constant,
  constantV4L2_BUF_FLAG_QUEUED/constant and
  constantV4L2_BUF_FLAG_DONE/constant flags will be valid. The
  structfieldmemory/structfield field will be set to the current
 @@ -79,8 +80,10 @@ contains the offset of the buffer from the start of the 
 device memory,
  the structfieldlength/structfield field its size. For the multi-planar 
 API,
  fields structfieldm.mem_offset/structfield and
  structfieldlength/structfield in the structfieldm.planes/structfield
 -array elements will be used instead. The driver may or may not set the 
 remaining
 -fields and flags, they are meaningless in this context./para
 +array elements will be used instead and the structfieldlength/structfield
 +field of v4l2-buffer; is set to the number of filled-in array elements.
 +The driver may or may not set the remaining fields and flags, they are
 +meaningless in this context./para
  
  paraThe structnamev4l2_buffer/structname structure is
  specified in xref linkend=buffer 

Re: [PATCH 1/3] Add a core driver for SI476x MFD

2012-09-21 Thread Hans Verkuil
On Fri September 21 2012 18:33:45 andrey.smir...@convergeddevices.net wrote:
 On 09/21/2012 12:31 AM, Hans Verkuil wrote:
  On Fri September 21 2012 03:05:41 andrey.smir...@convergeddevices.net wrote:
  On 09/13/2012 11:44 PM, Hans Verkuil wrote:
  Hi Andrey!
 
  Thanks for posting this driver. One request for the future: please split 
  this
  patch up in smaller pieces: one for each c source for example. That makes 
  it
  easier to review.
  Will do for next version.
 
  +
  +/**
  + * __core_send_command() - sends a command to si476x and waits its
  + * response
  + * @core:si476x_device structure for the device we are
  + *communicating with
  + * @command:  command id
  + * @args: command arguments we are sending
  + * @argn: actual size of @args
  + * @response: buffer to place the expected response from the device
  + * @respn:actual size of @response
  + * @usecs:amount of time to wait before reading the response (in
  + *usecs)
  + *
  + * Function returns 0 on succsess and negative error code on
  + * failure
  + */
  +static int __core_send_command(struct si476x_core *core,
  + const u8 command,
  + const u8 args[],
  + const int argn,
  + u8 resp[],
  + const int respn,
  + const int usecs)
  +{
  + struct i2c_client *client = core-client;
  + int err;
  + u8  data[CMD_MAX_ARGS_COUNT + 1];
  +
  + if (argn  CMD_MAX_ARGS_COUNT) {
  + err = -ENOMEM;
  + goto exit;
  Why goto exit? There is no clean up after the exit label, so just return
  immediately. Ditto for all the other goto exit's in this function.
  To have only just on point of exit from the function that's just
  personal coding style preference.
  There are no technical reasons behind that, I can change that.
 
  +}
  +
  +if (!client-adapter) {
  +err = -ENODEV;
  +goto exit;
  +}
  +
  +/* First send the command and its arguments */
  +data[0] = command;
  +memcpy(data[1], args, argn);
  +DBG_BUFFER(client-dev, Command:\n, data, argn + 1);
  +
  +err = si476x_i2c_xfer(core, SI476X_I2C_SEND, (char *) data, 
  argn + 1);
  +if (err != argn + 1) {
  +dev_err(core-client-dev,
  +Error while sending command 0x%02x\n,
  +command);
  +err = (err = 0) ? -EIO : err;
  +goto exit;
  +}
  +/* Set CTS to zero only after the command is send to avoid
  + * possible racing conditions when working in polling mode */
  +atomic_set(core-cts, 0);
  +
  +if (!wait_event_timeout(core-command,
  +atomic_read(core-cts),
  +usecs_to_jiffies(usecs) + 1))
  +dev_warn(core-client-dev,
  + (%s) [CMD 0x%02x] Device took too much time 
  to answer.\n,
  + __func__, command);
  +
  +/*
  +  When working in polling mode, for some reason the tuner will
  +  report CTS bit as being set in the first status byte read,
  +  but all the consequtive ones will return zros until the
  +  tuner is actually completed the POWER_UP command. To
  +  workaround that we wait for second CTS to be reported
  + */
  +if (unlikely(!core-client-irq  command == CMD_POWER_UP)) {
  +if (!wait_event_timeout(core-command,
  +atomic_read(core-cts),
  +usecs_to_jiffies(usecs) + 1))
  +dev_warn(core-client-dev,
  + (%s) Power up took too much time.\n,
  + __func__);
  +}
  +
  +/* Then get the response */
  +err = si476x_i2c_xfer(core, SI476X_I2C_RECV, resp, respn);
  +if (err != respn) {
  +dev_err(core-client-dev,
  +Error while reading response for command 
  0x%02x\n,
  +command);
  +err = (err = 0) ? -EIO : err;
  +goto exit;
  +}
  +DBG_BUFFER(client-dev, Response:\n, resp, respn);
  +
  +err = 0;
  +
  +if (resp[0]  SI476X_ERR) {
  +dev_err(core-client-dev, Chip set error flag\n);
  +err = si476x_core_parse_and_nag_about_error(core);
  +goto exit;
  +}
  +
  +if (!(resp[0]  SI476X_CTS))
  +err = -EBUSY;
  +exit:
  +return err;
  +}
  +
  +#define CORE_SEND_COMMAND(core, cmd, args, resp, timeout)   
  \
  +__core_send_command(core, cmd, args,
  \
  +   

Re: [alsa-devel] [PATCH v2 00/34] i.MX multi-platform support

2012-09-21 Thread Shawn Guo
Hi Olof,

On Fri, Sep 21, 2012 at 01:26:43AM -0700, Olof Johansson wrote:
 I'll take a look at merging it tomorrow after I've dealt with smp_ops;
 if it looks reasonably conflict-free I'll pull it in. We need the
 sound dependency sorted out (or agreed upon) first though.
 
I just published the branch below with this series rebased on top of
the necessary dependant branches.

  git://git.linaro.org/people/shawnguo/linux-2.6.git staging/imx-multiplatform

The dependant branches include:

* arm-soc/multiplatform/platform-data

* arm-soc/multiplatform/smp_ops

* git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-3.7

  It contains dependant patch ASoC: mx27vis: retrieve gpio numbers
  from platform_data

* git://git.infradead.org/mtd-2.6.git master

  The series is based on this tree to solve some non-trivial conflicts
  on mxc_nand driver.  Because mtd tree completely missed 3.6 merge
  window, having the series base on 3.6-rc actually means 3.5 code base
  in term of mtd support.  There are currently two cycles changes
  accumulated on mtd, and we need to base the series on it to sort out
  the conflicts.

* git://linuxtv.org/mchehab/media-next.git master

  The media tree renames mx2/mx3 camera drivers twice.  I'm not sure
  if git merge can detect them, so I just rebased the series on media
  tree to solve that.  The bonus point is that a number of trivial
  conflicts with imx27-coda support on media tree gets solved as well.

I'm not requesting you to pull the branch into arm-soc as a stable
branch but staging one, because the external dependencies which might
not be stable.  I attempt to use it for exposing the series on
linux-next, so that we can send it to Linus for 3.7 if there is chance
for us to (e.g. all the dependant branches hit mainline early during
3.7 merge window).

Shawn
--
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


Re: [RFCv1 PATCH 4/6] videobuf2-core: fill in length field for multiplanar buffers.

2012-09-21 Thread Sylwester Nawrocki
On 09/21/2012 06:23 PM, Hans Verkuil wrote:
 On Fri September 21 2012 18:13:20 Sylwester Nawrocki wrote:
 Hi Hans,

 On 09/19/2012 04:37 PM, Hans Verkuil wrote:
 From: Hans Verkuil hans.verk...@cisco.com

 length should be set to num_planes in __fill_v4l2_buffer(). That way the
 caller knows how many planes there are in the buffer.

 Signed-off-by: Hans Verkuil hans.verk...@cisco.com

 I think this would break VIDIOC_CREATE_BUFS. We need per buffer num_planes.
 Consider a use case where device is streaming with 2-planar pixel format
 and we invoke VIDIOC_CREATE_BUFS with single-planar format. On a single 
 queue there will be buffers with different number of planes. The number of 
 planes information must be attached to a buffer, otherwise VIDIOC_QUERYBUF 
 won't work.
 
 That's a very good point and one I need to meditate on.
 
 However, your comment applies to patch 1/6, not to this one.
 This patch is about whether or not the length field of v4l2_buffer should
 be filled in with the actual number of planes used by that buffer or not.

Yes, right. Sorry, I was editing response to multiple patches from this
series and have mixed things a bit. I agree that it is logical and expected
to update struct v4l2_buffer for user space.

I have spent some time on this series, and even prepared a patch for s5p-mfc,
as it relies on num_planes being in struct vb2_buffer. But then a realized
there could be buffers with distinct number of planes an a single queue.

Regards,
Sylwester

--
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


Re: [alsa-devel] [PATCH v2 00/34] i.MX multi-platform support

2012-09-21 Thread Shawn Guo
On Sat, Sep 22, 2012 at 12:46:26AM +0800, Shawn Guo wrote:
 I just published the branch below with this series rebased on top of
 the necessary dependant branches.
 
   git://git.linaro.org/people/shawnguo/linux-2.6.git staging/imx-multiplatform
 
 The dependant branches include:
 

Forgot the base:

  * arm-soc/next/multiplatform

Shawn

 * arm-soc/multiplatform/platform-data
 
 * arm-soc/multiplatform/smp_ops
 
 * git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-3.7
 
   It contains dependant patch ASoC: mx27vis: retrieve gpio numbers
   from platform_data
 
 * git://git.infradead.org/mtd-2.6.git master
 
   The series is based on this tree to solve some non-trivial conflicts
   on mxc_nand driver.  Because mtd tree completely missed 3.6 merge
   window, having the series base on 3.6-rc actually means 3.5 code base
   in term of mtd support.  There are currently two cycles changes
   accumulated on mtd, and we need to base the series on it to sort out
   the conflicts.
 
 * git://linuxtv.org/mchehab/media-next.git master
 
   The media tree renames mx2/mx3 camera drivers twice.  I'm not sure
   if git merge can detect them, so I just rebased the series on media
   tree to solve that.  The bonus point is that a number of trivial
   conflicts with imx27-coda support on media tree gets solved as well.
 
 I'm not requesting you to pull the branch into arm-soc as a stable
 branch but staging one, because the external dependencies which might
 not be stable.  I attempt to use it for exposing the series on
 linux-next, so that we can send it to Linus for 3.7 if there is chance
 for us to (e.g. all the dependant branches hit mainline early during
 3.7 merge window).
--
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


Re: [RFCv1 PATCH 4/6] videobuf2-core: fill in length field for multiplanar buffers.

2012-09-21 Thread Hans Verkuil
On Fri September 21 2012 18:47:54 Sylwester Nawrocki wrote:
 On 09/21/2012 06:23 PM, Hans Verkuil wrote:
  On Fri September 21 2012 18:13:20 Sylwester Nawrocki wrote:
  Hi Hans,
 
  On 09/19/2012 04:37 PM, Hans Verkuil wrote:
  From: Hans Verkuil hans.verk...@cisco.com
 
  length should be set to num_planes in __fill_v4l2_buffer(). That way the
  caller knows how many planes there are in the buffer.
 
  Signed-off-by: Hans Verkuil hans.verk...@cisco.com
 
  I think this would break VIDIOC_CREATE_BUFS. We need per buffer num_planes.
  Consider a use case where device is streaming with 2-planar pixel format
  and we invoke VIDIOC_CREATE_BUFS with single-planar format. On a single 
  queue there will be buffers with different number of planes. The number of 
  planes information must be attached to a buffer, otherwise VIDIOC_QUERYBUF 
  won't work.
  
  That's a very good point and one I need to meditate on.
  
  However, your comment applies to patch 1/6, not to this one.
  This patch is about whether or not the length field of v4l2_buffer should
  be filled in with the actual number of planes used by that buffer or not.
 
 Yes, right. Sorry, I was editing response to multiple patches from this
 series and have mixed things a bit. I agree that it is logical and expected
 to update struct v4l2_buffer for user space.

OK, great. That's was actually the main reason for working on this as this
unexpected behavior bit me when writing mplane streaming support for v4l2-ctl.

 I have spent some time on this series, and even prepared a patch for s5p-mfc,
 as it relies on num_planes being in struct vb2_buffer. But then a realized
 there could be buffers with distinct number of planes an a single queue.

I'll get back to you about this, probably on Monday. I need to think about the
implications of this.

Regards,

Hans
--
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


Re: media_build directory error after patch dvb_frontend: implement suspend / resume

2012-09-21 Thread Antti Palosaari

On 09/21/2012 07:19 PM, Jan Hoogenraad wrote:

I try to compile the media_build on an old Ubuntu Lucid system.
2.6.32-42-generic-pae #96-Ubuntu SMP Wed Aug 15 19:12:17 UTC 2012 i686
GNU/Linux

The make job stops with

/home/jhh/dvb/media_build/v4l/dvb_usb_core.c: In function
'dvb_usb_data_complete_raw':
/home/jhh/dvb/media_build/v4l/dvb_usb_core.c:224: error: implicit
declaration of function 'dvb_dmx_swfilter_raw'
/home/jhh/dvb/media_build/v4l/dvb_usb_core.c: In function
'dvb_usbv2_suspend':
/home/jhh/dvb/media_build/v4l/dvb_usb_core.c:975: error: implicit
declaration of function 'dvb_frontend_suspend'
/home/jhh/dvb/media_build/v4l/dvb_usb_core.c: In function
'dvb_usbv2_resume_common':
/home/jhh/dvb/media_build/v4l/dvb_usb_core.c:994: error: implicit
declaration of function 'dvb_frontend_resume'

This probably started after
http://article.gmane.org/gmane.linux.drivers.video-input-infrastructure/52430/match=dvb_frontend_resume

The media_build tree is confused with too many files with the same name:

find linux -name \*front\* -ls
54788964   24 -rw-r--r--   1 jhh  jhh 12642 Aug 16 05:45
linux/drivers/media/dvb-core/dvb_frontend.h
54788973   80 -rw-r--r--   1 jhh  jhh 71787 Sep 14 05:45
linux/drivers/media/dvb-core/dvb_frontend.c
54397078   20 -rw-r--r--   1 jhh  jhh 12056 Sep 16 20:20
linux/drivers/media/dvb/dvb-core/dvb_frontend.h
54397083   68 -rw-r--r--   1 jhh  jhh 60558 Jun  8  2011
linux/drivers/media/dvb/dvb-core/dvb_frontend.c
5439   24 -rw-r--r--   1 jhh  jhh 13012 Aug 14 05:45
linux/include/linux/dvb/frontend.h

The frontend.h and .c chosen in the build are older ones
  find v4l -name \*front\* -ls

54135379   76 -rw-r--r--   1 jhh  jhh 67636 Sep 21 17:47
v4l/.dvb_frontend.o.cmd
541339200 lrwxrwxrwx   1 jhh  jhh50 Sep 21 18:13
v4l/dvb_frontend.h - ../linux/drivers/media/dvb/dvb-core/dvb_frontend.h
541339270 lrwxrwxrwx   1 jhh  jhh50 Sep 21 18:13
v4l/dvb_frontend.c - ../linux/drivers/media/dvb/dvb-core/dvb_frontend.c
54136025  184 -rw-r--r--   1 jhh  jhh179558 Sep 21 17:47
v4l/dvb_frontend.o

Do you know what to do about this situation ?


All that is due to drivers/media directory structure re-organization. 
Could you use clean directory for media_build.git? If you want use 
exiting media_build.git you should remove manually those old directories.


regards
Antti

--
http://palosaari.fi/
--
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


Re: media_build directory error after patch dvb_frontend: implement suspend / resume

2012-09-21 Thread Jan Hoogenraad
Thanks Antti:

Maybe the media_build could clean out the linux subdirectories (e.g.
drivers   include  sound) before the tar x command.

Your rtl28xxu driver immediately works, even on this old system !
Thanks for that as well.

I will remove the very outdated info I put onto the Wiki pages:
http://linuxtv.org/wiki/index.php/Realtek_RTL2831U

http://linuxtv.org/wiki/index.php/DVB-T_USB_Devices
http://linuxtv.org/wiki/index.php/DVB-T_USB_Devices#Freecom_rev_4_DVB-T_USB_2.0_tuner


Antti Palosaari wrote:
 On 09/21/2012 07:19 PM, Jan Hoogenraad wrote:
 I try to compile the media_build on an old Ubuntu Lucid system.
 2.6.32-42-generic-pae #96-Ubuntu SMP Wed Aug 15 19:12:17 UTC 2012 i686
 GNU/Linux

 The make job stops with

 /home/jhh/dvb/media_build/v4l/dvb_usb_core.c: In function
 'dvb_usb_data_complete_raw':
 /home/jhh/dvb/media_build/v4l/dvb_usb_core.c:224: error: implicit
 declaration of function 'dvb_dmx_swfilter_raw'
 /home/jhh/dvb/media_build/v4l/dvb_usb_core.c: In function
 'dvb_usbv2_suspend':
 /home/jhh/dvb/media_build/v4l/dvb_usb_core.c:975: error: implicit
 declaration of function 'dvb_frontend_suspend'
 /home/jhh/dvb/media_build/v4l/dvb_usb_core.c: In function
 'dvb_usbv2_resume_common':
 /home/jhh/dvb/media_build/v4l/dvb_usb_core.c:994: error: implicit
 declaration of function 'dvb_frontend_resume'

 This probably started after
 http://article.gmane.org/gmane.linux.drivers.video-input-infrastructure/52430/match=dvb_frontend_resume


 The media_build tree is confused with too many files with the same name:

 find linux -name \*front\* -ls
 54788964   24 -rw-r--r--   1 jhh  jhh 12642 Aug 16 05:45
 linux/drivers/media/dvb-core/dvb_frontend.h
 54788973   80 -rw-r--r--   1 jhh  jhh 71787 Sep 14 05:45
 linux/drivers/media/dvb-core/dvb_frontend.c
 54397078   20 -rw-r--r--   1 jhh  jhh 12056 Sep 16 20:20
 linux/drivers/media/dvb/dvb-core/dvb_frontend.h
 54397083   68 -rw-r--r--   1 jhh  jhh 60558 Jun  8  2011
 linux/drivers/media/dvb/dvb-core/dvb_frontend.c
 5439   24 -rw-r--r--   1 jhh  jhh 13012 Aug 14 05:45
 linux/include/linux/dvb/frontend.h

 The frontend.h and .c chosen in the build are older ones
   find v4l -name \*front\* -ls

 54135379   76 -rw-r--r--   1 jhh  jhh 67636 Sep 21 17:47
 v4l/.dvb_frontend.o.cmd
 541339200 lrwxrwxrwx   1 jhh  jhh50 Sep 21 18:13
 v4l/dvb_frontend.h - ../linux/drivers/media/dvb/dvb-core/dvb_frontend.h
 541339270 lrwxrwxrwx   1 jhh  jhh50 Sep 21 18:13
 v4l/dvb_frontend.c - ../linux/drivers/media/dvb/dvb-core/dvb_frontend.c
 54136025  184 -rw-r--r--   1 jhh  jhh179558 Sep 21 17:47
 v4l/dvb_frontend.o

 Do you know what to do about this situation ?
 
 All that is due to drivers/media directory structure re-organization.
 Could you use clean directory for media_build.git? If you want use
 exiting media_build.git you should remove manually those old directories.
 
 regards
 Antti
 


-- 
Jan Hoogenraad
Hoogenraad Interface Services
Postbus 2717
3500 GS Utrecht
--
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


cron job: media_tree daily build: ERRORS

2012-09-21 Thread Hans Verkuil
This message is generated daily by a cron job that builds media_tree for
the kernels and architectures in the list below.

Results of the daily build of media_tree:

date:Fri Sep 21 19:00:18 CEST 2012
git hash:4313902ebe33155209472215c62d2f29d117be29
gcc version:  i686-linux-gcc (GCC) 4.7.1
host hardware:x86_64
host os:  3.4.07-marune

linux-git-arm-eabi-davinci: WARNINGS
linux-git-arm-eabi-exynos: OK
linux-git-arm-eabi-omap: WARNINGS
linux-git-i686: WARNINGS
linux-git-m32r: WARNINGS
linux-git-mips: WARNINGS
linux-git-powerpc64: OK
linux-git-sh: ERRORS
linux-git-x86_64: WARNINGS
linux-2.6.31.12-x86_64: ERRORS
linux-2.6.32.6-x86_64: ERRORS
linux-2.6.33-x86_64: ERRORS
linux-2.6.34-x86_64: ERRORS
linux-2.6.35.3-x86_64: ERRORS
linux-2.6.36-x86_64: ERRORS
linux-2.6.37-x86_64: WARNINGS
linux-2.6.38.2-x86_64: WARNINGS
linux-2.6.39.1-x86_64: WARNINGS
linux-3.0-x86_64: WARNINGS
linux-3.1-x86_64: WARNINGS
linux-3.2.1-x86_64: WARNINGS
linux-3.3-x86_64: WARNINGS
linux-3.4-x86_64: WARNINGS
linux-3.5-x86_64: WARNINGS
linux-3.6-rc2-x86_64: WARNINGS
linux-2.6.31.12-i686: ERRORS
linux-2.6.32.6-i686: ERRORS
linux-2.6.33-i686: ERRORS
linux-2.6.34-i686: ERRORS
linux-2.6.35.3-i686: ERRORS
linux-2.6.36-i686: ERRORS
linux-2.6.37-i686: WARNINGS
linux-2.6.38.2-i686: WARNINGS
linux-2.6.39.1-i686: WARNINGS
linux-3.0-i686: WARNINGS
linux-3.1-i686: WARNINGS
linux-3.2.1-i686: WARNINGS
linux-3.3-i686: WARNINGS
linux-3.4-i686: WARNINGS
linux-3.5-i686: WARNINGS
linux-3.6-rc2-i686: WARNINGS
apps: WARNINGS
spec-git: WARNINGS
sparse: ERRORS

Detailed results are available here:

http://www.xs4all.nl/~hverkuil/logs/Friday.log

Full logs are available here:

http://www.xs4all.nl/~hverkuil/logs/Friday.tar.bz2

The V4L-DVB specification from this daily build is here:

http://www.xs4all.nl/~hverkuil/spec/media.html
--
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


media_build error on header file not present in old linux in ivtv-alsa-pcm.c

2012-09-21 Thread Jan Hoogenraad
I try to compile the media_build on an old Ubuntu Lucid system.
2.6.32-42-generic-pae #96-Ubuntu SMP Wed Aug 15 19:12:17 UTC 2012 i686
GNU/Linux

The make job stops with

/home/jhh/dvb/media_build/v4l/ivtv-alsa-pcm.c:29:26: error:
linux/printk.h: No such file or directory
make[3]: *** [/home/jhh/dvb/media_build/v4l/ivtv-alsa-pcm.o] Error 1

Apparently, this header file was not yet present in this version of
linux. It is the only driver requesting this header file.
Removing line 29

#include linux/printk.h

fixes the problem. All compiles well then.



-- 
Jan Hoogenraad
Hoogenraad Interface Services
Postbus 2717
3500 GS Utrecht
--
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


Re: media_build error on header file not present in old linux in ivtv-alsa-pcm.c

2012-09-21 Thread Andy Walls
Jan Hoogenraad jan-conceptro...@hoogenraad.net wrote:

I try to compile the media_build on an old Ubuntu Lucid system.
2.6.32-42-generic-pae #96-Ubuntu SMP Wed Aug 15 19:12:17 UTC 2012 i686
GNU/Linux

The make job stops with

/home/jhh/dvb/media_build/v4l/ivtv-alsa-pcm.c:29:26: error:
linux/printk.h: No such file or directory
make[3]: *** [/home/jhh/dvb/media_build/v4l/ivtv-alsa-pcm.o] Error 1

Apparently, this header file was not yet present in this version of
linux. It is the only driver requesting this header file.
Removing line 29

#include linux/printk.h

fixes the problem. All compiles well then.



-- 
Jan Hoogenraad
Hoogenraad Interface Services
Postbus 2717
3500 GS Utrecht
--
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

Hi Jan,

Hans Verkuil already noticed this and submitted a patch to remove the include.

Regards,
Andy
--
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


[PATCH 1/2] cypress_firmware: use Kernel dev_foo() logging

2012-09-21 Thread Antti Palosaari
Signed-off-by: Antti Palosaari cr...@iki.fi
---
 drivers/media/usb/dvb-usb-v2/cypress_firmware.c | 21 -
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/media/usb/dvb-usb-v2/cypress_firmware.c 
b/drivers/media/usb/dvb-usb-v2/cypress_firmware.c
index 9f7c970..bb21eee 100644
--- a/drivers/media/usb/dvb-usb-v2/cypress_firmware.c
+++ b/drivers/media/usb/dvb-usb-v2/cypress_firmware.c
@@ -30,6 +30,9 @@ static const struct usb_cypress_controller cypress[] = {
 static int usb_cypress_writemem(struct usb_device *udev, u16 addr, u8 *data,
u8 len)
 {
+   dvb_usb_dbg_usb_control_msg(udev,
+   0xa0, USB_TYPE_VENDOR, addr, 0x00, data, len);
+
return usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
0xa0, USB_TYPE_VENDOR, addr, 0x00, data, len, 5000);
 }
@@ -45,24 +48,24 @@ int usbv2_cypress_load_firmware(struct usb_device *udev,
reset = 1;
ret = usb_cypress_writemem(udev, cypress[type].cs_reg, reset, 1);
if (ret != 1)
-   pr_err(%s: could not stop the USB controller CPU,
+   dev_err(udev-dev,
+   %s: could not stop the USB controller CPU\n,
KBUILD_MODNAME);
 
while ((ret = dvb_usbv2_get_hexline(fw, hx, pos))  0) {
-   pr_debug(%s: writing to address %04x (buffer: %02x %02x)\n,
-   __func__, hx.addr, hx.len, hx.chk);
-
ret = usb_cypress_writemem(udev, hx.addr, hx.data, hx.len);
if (ret != hx.len) {
-   pr_err(%s: error while transferring firmware  \
-   (transferred size=%d, block size=%d),
+   dev_err(udev-dev, %s: error while transferring  \
+   firmware (transferred size=%d,  \
+   block size=%d)\n,
KBUILD_MODNAME, ret, hx.len);
ret = -EINVAL;
break;
}
}
if (ret  0) {
-   pr_err(%s: firmware download failed at %d with %d,
+   dev_err(udev-dev,
+   %s: firmware download failed at %d with %d\n,
KBUILD_MODNAME, pos, ret);
return ret;
}
@@ -72,8 +75,8 @@ int usbv2_cypress_load_firmware(struct usb_device *udev,
reset = 0;
if (ret || usb_cypress_writemem(
udev, cypress[type].cs_reg, reset, 1) != 1) {
-   pr_err(%s: could not restart the USB controller CPU,
-   KBUILD_MODNAME);
+   dev_err(udev-dev, %s: could not restart the USB  \
+   controller CPU\n, KBUILD_MODNAME);
ret = -EINVAL;
}
} else
-- 
1.7.11.4

--
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


[PATCH 2/2] cypress_firmware: refactor firmware downloading

2012-09-21 Thread Antti Palosaari
Refactor firmware download function. It also should fix one bug
coming from usb_control_msg() message buffers. Taking buffers from
the stack for usb_control_msg() is not allowed as it does not work
on all architectures. Allocate buffers using kmalloc().

Signed-off-by: Antti Palosaari cr...@iki.fi
---
 drivers/media/usb/dvb-usb-v2/cypress_firmware.c | 80 +
 1 file changed, 43 insertions(+), 37 deletions(-)

diff --git a/drivers/media/usb/dvb-usb-v2/cypress_firmware.c 
b/drivers/media/usb/dvb-usb-v2/cypress_firmware.c
index bb21eee..211df54 100644
--- a/drivers/media/usb/dvb-usb-v2/cypress_firmware.c
+++ b/drivers/media/usb/dvb-usb-v2/cypress_firmware.c
@@ -40,48 +40,59 @@ static int usb_cypress_writemem(struct usb_device *udev, 
u16 addr, u8 *data,
 int usbv2_cypress_load_firmware(struct usb_device *udev,
const struct firmware *fw, int type)
 {
-   struct hexline hx;
-   u8 reset;
+   struct hexline *hx;
int ret, pos = 0;
 
+   hx = kmalloc(sizeof(struct hexline), GFP_KERNEL);
+   if (!hx) {
+   dev_err(udev-dev, %s: kmalloc() failed\n, KBUILD_MODNAME);
+   return -ENOMEM;
+   }
+
/* stop the CPU */
-   reset = 1;
-   ret = usb_cypress_writemem(udev, cypress[type].cs_reg, reset, 1);
-   if (ret != 1)
-   dev_err(udev-dev,
-   %s: could not stop the USB controller CPU\n,
-   KBUILD_MODNAME);
-
-   while ((ret = dvb_usbv2_get_hexline(fw, hx, pos))  0) {
-   ret = usb_cypress_writemem(udev, hx.addr, hx.data, hx.len);
-   if (ret != hx.len) {
+   hx-data[0] = 1;
+   ret = usb_cypress_writemem(udev, cypress[type].cs_reg, hx-data, 1);
+   if (ret != 1) {
+   dev_err(udev-dev, %s: CPU stop failed=%d\n,
+   KBUILD_MODNAME, ret);
+   ret = -EIO;
+   goto err_kfree;
+   }
+
+   /* write firmware to memory */
+   for (;;) {
+   ret = dvb_usbv2_get_hexline(fw, hx, pos);
+   if (ret  0)
+   goto err_kfree;
+   else if (ret == 0)
+   break;
+
+   ret = usb_cypress_writemem(udev, hx-addr, hx-data, hx-len);
+   if (ret  0) {
+   goto err_kfree;
+   } else if (ret != hx-len) {
dev_err(udev-dev, %s: error while transferring  \
firmware (transferred size=%d,  \
block size=%d)\n,
-   KBUILD_MODNAME, ret, hx.len);
-   ret = -EINVAL;
-   break;
+   KBUILD_MODNAME, ret, hx-len);
+   ret = -EIO;
+   goto err_kfree;
}
}
-   if (ret  0) {
-   dev_err(udev-dev,
-   %s: firmware download failed at %d with %d\n,
-   KBUILD_MODNAME, pos, ret);
-   return ret;
-   }
 
-   if (ret == 0) {
-   /* restart the CPU */
-   reset = 0;
-   if (ret || usb_cypress_writemem(
-   udev, cypress[type].cs_reg, reset, 1) != 1) {
-   dev_err(udev-dev, %s: could not restart the USB  \
-   controller CPU\n, KBUILD_MODNAME);
-   ret = -EINVAL;
-   }
-   } else
+   /* start the CPU */
+   hx-data[0] = 0;
+   ret = usb_cypress_writemem(udev, cypress[type].cs_reg, hx-data, 1);
+   if (ret != 1) {
+   dev_err(udev-dev, %s: CPU start failed=%d\n,
+   KBUILD_MODNAME, ret);
ret = -EIO;
+   goto err_kfree;
+   }
 
+   ret = 0;
+err_kfree:
+   kfree(hx);
return ret;
 }
 EXPORT_SYMBOL(usbv2_cypress_load_firmware);
@@ -96,7 +107,6 @@ int dvb_usbv2_get_hexline(const struct firmware *fw, struct 
hexline *hx,
return 0;
 
memset(hx, 0, sizeof(struct hexline));
-
hx-len = b[0];
 
if ((*pos + hx-len + 4) = fw-size)
@@ -109,14 +119,10 @@ int dvb_usbv2_get_hexline(const struct firmware *fw, 
struct hexline *hx,
/* b[4] and b[5] are the Extended linear address record data
 * field */
hx-addr |= (b[4]  24) | (b[5]  16);
-   /*
-   hx-len -= 2;
-   data_offs += 2;
-   */
}
+
memcpy(hx-data, b[data_offs], hx-len);
hx-chk = b[hx-len + data_offs];
-
*pos += hx-len + 5;
 
return *pos;
-- 
1.7.11.4

--
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  

Re: [PATCH 01/14] davinci: vpfe: add dm3xx IPIPEIF hardware support module

2012-09-21 Thread Prabhakar Lad
Hi Laurent,

Thanks for the review.

On Thu, Sep 20, 2012 at 3:31 AM, Laurent Pinchart
laurent.pinch...@ideasonboard.com wrote:
 Hi Prabhakar,

 Thanks for the patch.

 On Friday 14 September 2012 18:16:31 Prabhakar Lad wrote:
 From: Manjunath Hadli manjunath.ha...@ti.com

 add support for dm3xx IPIPEIF hardware setup. This is the
 lowest software layer for the dm3x vpfe driver which directly
 accesses hardware. Add support for features like default
 pixel correction, dark frame substraction and hardware setup.

 Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com
 Signed-off-by: Lad, Prabhakar prabhakar@ti.com
 ---
  drivers/media/platform/davinci/dm3xx_ipipeif.c |  318 +
  drivers/media/platform/davinci/dm3xx_ipipeif.h |  262 +++
  include/linux/dm3xx_ipipeif.h  |   62 +


  3 files changed, 642 insertions(+), 0 deletions(-)
  create mode 100644 drivers/media/platform/davinci/dm3xx_ipipeif.c
  create mode 100644 drivers/media/platform/davinci/dm3xx_ipipeif.h
  create mode 100644 include/linux/dm3xx_ipipeif.h

 diff --git a/drivers/media/platform/davinci/dm3xx_ipipeif.c
 b/drivers/media/platform/davinci/dm3xx_ipipeif.c new file mode 100644
 index 000..7961a74
 --- /dev/null
 +++ b/drivers/media/platform/davinci/dm3xx_ipipeif.c

 [snip]

 +#include linux/io.h
 +#include linux/module.h
 +#include linux/kernel.h
 +#include linux/uaccess.h
 +#include linux/v4l2-mediabus.h
 +#include linux/platform_device.h

 Just a side note, I usually sort headers alphabetically, but feel free to use
 whatever convention you like.

Ok I'll sort to.

 +#include dm3xx_ipipeif.h
 +
 +static void *__iomem ipipeif_base_addr;

 That's not good. You shouldn't have global constants like that. The value
 should instead be stored in your device structure, that you will need to pass
 around to all functions.

Ok.

 +static inline u32 regr_if(u32 offset)
 +{
 + return readl(ipipeif_base_addr + offset);
 +}
 +
 +static inline void regw_if(u32 val, u32 offset)
 +{
 + writel(val, ipipeif_base_addr + offset);
 +}

 Maybe ipipeif_read() and ipipeif_write() ?

 +void ipipeif_set_enable()
 +{
 + regw_if(1, IPIPEIF_ENABLE);
 +}

 Please define constants in a header file for register values, masks and shifts
 instead of hardcoding the raw numbers.

Ok.

 [snip]

 +static int __devinit dm3xx_ipipeif_probe(struct platform_device *pdev)
 +{
 + static resource_size_t  res_len;
 + struct resource *res;
 + int status;
 +
 + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 + if (!res)
 + return -ENOENT;
 +
 + res_len = resource_size(res);
 +
 + res = request_mem_region(res-start, res_len, res-name);
 + if (!res)
 + return -EBUSY;
 +
 + ipipeif_base_addr = ioremap_nocache(res-start, res_len);
 + if (!ipipeif_base_addr) {
 + status = -EBUSY;
 + goto fail;
 + }
 + return 0;
 +
 +fail:
 + release_mem_region(res-start, res_len);
 +
 + return status;
 +}
 +
 +static int dm3xx_ipipeif_remove(struct platform_device *pdev)
 +{
 + struct resource *res;
 +
 + iounmap(ipipeif_base_addr);
 + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 + if (res)
 + release_mem_region(res-start, resource_size(res));
 + return 0;
 +}
 +
 +static struct platform_driver dm3xx_ipipeif_driver = {
 + .driver = {
 + .name   = dm3xx_ipipeif,
 + .owner = THIS_MODULE,
 + },
 + .remove = __devexit_p(dm3xx_ipipeif_remove),
 + .probe = dm3xx_ipipeif_probe,
 +};
 +
 +static int dm3xx_ipipeif_init(void)
 +{
 + return platform_driver_register(dm3xx_ipipeif_driver);
 +}
 +
 +static void dm3xx_ipipeif_exit(void)
 +{
 + platform_driver_unregister(dm3xx_ipipeif_driver);
 +}
 +
 +module_init(dm3xx_ipipeif_init);
 +module_exit(dm3xx_ipipeif_exit);

 I'm not sure to like this. You're registering a module for a device that
 essentially sits there without doing anything, except providing functions that
 can be called by other modules.

 I somehow feel that the way the code is split amongst the different layers
 isn't optimal. Could you briefly explain the rationale behind the current
 architecture ?

 (BTW, please use the module_platform_driver() macro instead of
 module_init/module_exit)

As discussed over the IRC, I am working on new design, hopefully
you will be happy this time :)

 [snip]

 diff --git a/include/linux/dm3xx_ipipeif.h b/include/linux/dm3xx_ipipeif.h
 new file mode 100644
 index 000..1c1a830
 --- /dev/null
 +++ b/include/linux/dm3xx_ipipeif.h

 [snip]

 +#include media/davinci/vpfe_types.h
 +#include media/davinci/vpfe.h

 This header file defines part of the userspace API, but includes media/
 headers that are not exported to userspace.

 Header files should be split between 3 directories:

 - Definitions required by platform data used to go to media/ but the new
 include/linux/platform_data/ directory might