Re: [PATCH] [media] davinci: vpfe: Add documentation

2012-09-04 Thread Manjunath Hadli
Hi Sakari,

On Saturday 01 September 2012 10:55 PM, Sakari Ailus wrote:
 Hi Manju,
 
 My apologies for the delayed answer.
 
 On Wed, Aug 22, 2012 at 02:26:50PM +0530, Manjunath Hadli wrote:
 On Thursday 16 August 2012 09:53 PM, Sakari Ailus wrote:
 On Thu, Aug 09, 2012 at 09:13:52AM +0530, Manjunath Hadli wrote:
 On Thursday 02 August 2012 05:37 AM, Sakari Ailus wrote:
 Hi Manju,

 Thanks for the patch.

 Please make sure these patches reach linux-media next time. If they do
 not,
 it severely limits the number of potential reviewers. I don't know
 why, but
 the original patch isn't on linux-media even if the list was cc'd.

 Dropping linux-kernel from cc.

 Manjunath Hadli wrote:
 Add documentation on the Davinci VPFE driver. Document the subdevs,
 and private IOTCLs the driver implements

 Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com
 Signed-off-by: Lad, Prabhakar prabhakar@ti.com
 ---
   Documentation/video4linux/davinci-vpfe-mc.txt |  263
 +
   1 files changed, 263 insertions(+), 0 deletions(-)
   create mode 100644 Documentation/video4linux/davinci-vpfe-mc.txt

 diff --git a/Documentation/video4linux/davinci-vpfe-mc.txt
 b/Documentation/video4linux/davinci-vpfe-mc.txt
 new file mode 100644
 index 000..968194f
 --- /dev/null
 +++ b/Documentation/video4linux/davinci-vpfe-mc.txt
 @@ -0,0 +1,263 @@
 +Davinci Video processing Front End (VPFE) driver
 +
 +Copyright (C) 2012 Texas Instruments Inc
 +
 +Contacts: Manjunath Hadli manjunath.ha...@ti.com
 +
 +Introduction
 +
 +
 +This file documents the Texas Instruments Davinci Video processing
 Front End
 +(VPFE) driver located under drivers/media/video/davinci. The
 original driver
 +exists for Davinci VPFE, which is now being changed to Media Controller
 +Framework.
 +
 +Currently the driver has been successfully used on the following
 version of Davinci:
 +
 +DM365/DM368
 +
 +The driver implements V4L2, Media controller and v4l2_subdev
 interfaces.
 +Sensor, lens and flash drivers using the v4l2_subdev interface in
 the kernel
 +are supported.
 +
 +
 +Split to subdevs
 +
 +
 +The Davinic VPFE is split into V4L2 subdevs, each of the blocks
 inside the VPFE
 +having one subdev to represent it. Each of the subdevs provide a
 V4L2 subdev
 +interface to userspace.
 +
 +DAVINCI CCDC
 +DAVINCI PREVIEWER
 +DAVINCI RESIZER
 +DAVINCI AEW
 +DAVINCI AF
 +
 +Each possible link in the VPFE is modeled by a link in the Media
 controller
 +interface. For an example program see [1].
 +
 +
 +Private IOCTLs
 +==
 +
 +The Davinci Video processing Front End (VPFE) driver supports
 standard V4L2
 +IOCTLs and controls where possible and practical. Much of the
 functions provided
 +by the VPFE, however, does not fall under the standard IOCTLs.
 +
 +In general, there is a private ioctl for configuring each of the blocks
 +containing hardware-dependent functions.
 +
 +The following private IOCTLs are supported:
 +
 +1: IOCTL: PREV_S_PARAM/PREV_G_PARAM
 +Description:
 +Sets/Gets the parameters required by the previewer module
 +Parameter:
 +/**
 + * struct prev_module_param- structure to configure preview modules
 + * @version: Version of the preview module
 + * @len: Length of the module config structure
 + * @module_id: Module id
 + * @param: pointer to module config parameter.
 + */
 +struct prev_module_param {
 +char version[IMP_MAX_NAME_SIZE];
 +unsigned short len;
 +unsigned short module_id;
 +void *param;
 +};

 In addition to what Laurent commented on this, could the version
 information be passed in struct media_entity_desc instead?
 I plan to leave out the version.

 As a general comment, it's a bad idea to design an API that allows
 passing
 blobs, especially when the expected size of the blobs isn't known. That
 really equals to asking for trouble.

 That said, I know this is an area where complete documentation is acarce,
 but I think that at least the memory layout of the current blob pointers
 should be visible in the struct definitions whenever possible. See
 e.g. the
 OMAP 3 ISP driver.
 I have proposed using a union of structures instead of the void  blob. 
 I also saw the OMAP implementation, and they are pointers (but not void). 
 To me the union approach looks better as it keeps the architecture
 intact and does not necessitate an
 explicit copy_from_user. Which of these ways do you suggest?

 I would suggest to use the approach taken in the OMAP 3 ISP driver as it has
 one obvious advantage over the union of pointers: it makes it possible to
 perform atomic changes to the configuration.

 However, changes to configuration done through controls and the private
 IOCTL can never be atomic as they're done using a different IOCTL. This is
 something that will require some work at the API level in the future.

 I'm fine with both union of struct pointers or a struct of struct pointers
 (as in 

Re: [RFC 0/5] Generic panel framework

2012-09-04 Thread Zhou Zhu
Hi Laurent,

Basically I agree that we need a common panel framework. I just have
some questions:
1.  I think we should add color format in videomode - if we use such
common video mode structure shared across subsystems.
In HDMI, colors are bind with timings tightly. We need a combined
videomode with timing and color format together.
2. I think we should add set_videomode interface. It helps HDMI
monitors to set EDIDs.

-- 
Thanks,
-Zhou
--
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 1/5] video: Add generic display panel core

2012-09-04 Thread Sascha Hauer
Hi Laurent,

On Fri, Aug 17, 2012 at 02:49:39AM +0200, Laurent Pinchart wrote:
 +/**
 + * panel_get_modes - Get video modes supported by the panel
 + * @panel: The panel
 + * @modes: Pointer to an array of modes
 + *
 + * Fill the modes argument with a pointer to an array of video modes. The 
 array
 + * is owned by the panel.
 + *
 + * Return the number of supported modes on success (including 0 if no mode is
 + * supported) or a negative error code otherwise.
 + */
 +int panel_get_modes(struct panel *panel, const struct fb_videomode **modes)
 +{
 + if (!panel-ops || !panel-ops-get_modes)
 + return 0;
 +
 + return panel-ops-get_modes(panel, modes);
 +}
 +EXPORT_SYMBOL_GPL(panel_get_modes);

You have seen my of videomode helper proposal. One result there was that
we want to have ranges for the margin/synclen fields. Does it make sense
to base this new panel framework on a more sophisticated internal
reprentation of the panel parameters? This could then be converted to
struct fb_videomode / struct drm_display_mode as needed. This would also
make it more suitable for drm drivers which are not interested in struct
fb_videomode.

Related to this I suggest to change the API to be able to iterate over
the different modes, like:

struct videomode *panel_get_mode(struct panel *panel, int num);

This was we do not have to have an array of modes in memory, which may
be a burden for some panel drivers.

Sascha


-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
--
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: use of timestamp/sequence in v4l2_buffer

2012-09-04 Thread Hans Verkuil
Hi all,

During the Media Workshop last week we discussed how the timestamp and sequence
fields in struct v4l2_buffer should be used.

While trying to document the exact behavior I realized that there are a few
missing pieces.

Open questions with regards to the sequence field:

1) Should the first frame that was captured or displayed after starting 
streaming
for the first time always start with sequence index 0? In my opinion it should.

2) Should the sequence counter be reset to 0 after a STREAMOFF? Or should it 
only
be reset to 0 after REQBUFS/CREATE_BUFS is called?

3) Should the sequence counter behave differently for mem2mem devices? With the
current definition both the capture and display sides of a mem2mem device just 
have
their own independent sequence counter.

4) If frames are skipped, should the sequence counter skip as well? In my 
opinion
it shouldn't.

5) Should the sequence counter always be monotonically increasing? I think it 
should.

Open questions with regards to the timestamp field:

6) For output devices the timestamp field can be used to determine when to 
transmit
the frame. In practice there are no output drivers that support this. It is also
unclear how this would work: if the timestamp is 1 hour into the future, should 
the
driver hold on to that frame for one hour? If another frame is queued with a 
timestamp
that's earlier than the previous frame, should that frame be output first?

I am inclined to drop this behavior from the spec. Should we get drivers that 
actually
implement this, then we need to clarify the spec and add a new capability flag 
somewhere
to tell userspace that you can actually use the timestamp for this purpose.

7) Should the timestamp field always be monotonically increasing? Or it is 
possible
to get timestamps that jump around? This makes sense for encoders that create 
B-frames
referring to frames captured earlier than an I-frame.

8) How should the timestamp field be handled for mem2mem devices? Setting the 
timestamp
is pointless for the display side of a mem2mem device (as discussed above as 
well).
One option is that the mem2mem driver sets the timestamp when it starts 
processing a
queued frame. This timestamp is returned when the buffer is dequeued. This 
timestamp
is also copied to the processed frame (on the capture side of the mem2mem 
device),
thus allowing one to relate the captured processed frame to the source frame.

Even if encoder devices rearrange the frames (as discussed in the previous 
point),
the timestamp can still be used to map the frames.

Comments are welcome!

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


[PATCH 2/3] drivers/media/rc/ati_remote.c: fix error return code

2012-09-04 Thread Peter Senna Tschudin
From: Peter Senna Tschudin peter.se...@gmail.com

Convert a nonnegative error return code to a negative one, as returned
elsewhere in the function.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// smpl
(
if@p1 (\(ret  0\|ret != 0\))
 { ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
when != ret
*if(...)
{
  ... when != ret = e2
  when forall
 return ret;
}

// /smpl

Signed-off-by: Peter Senna Tschudin peter.se...@gmail.com

---
 drivers/media/rc/ati_remote.c |4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/media/rc/ati_remote.c b/drivers/media/rc/ati_remote.c
index 08aede5..49bb356 100644
--- a/drivers/media/rc/ati_remote.c
+++ b/drivers/media/rc/ati_remote.c
@@ -937,8 +937,10 @@ static int ati_remote_probe(struct usb_interface 
*interface,
/* Set up and register mouse input device */
if (mouse) {
input_dev = input_allocate_device();
-   if (!input_dev)
+   if (!input_dev) {
+   err = -ENOMEM;
goto fail4;
+   }
 
ati_remote-idev = input_dev;
ati_remote_input_init(ati_remote);

--
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/3] drivers/media/rc/redrat3.c: fix error return code

2012-09-04 Thread Peter Senna Tschudin
From: Peter Senna Tschudin peter.se...@gmail.com

Convert a nonnegative error return code to a negative one, as returned
elsewhere in the function.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// smpl
(
if@p1 (\(ret  0\|ret != 0\))
 { ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
when != ret
*if(...)
{
  ... when != ret = e2
  when forall
 return ret;
}

// /smpl

Signed-off-by: Peter Senna Tschudin peter.se...@gmail.com

---
 drivers/media/rc/redrat3.c |5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/media/rc/redrat3.c b/drivers/media/rc/redrat3.c
index 2878b0e..49731b1 100644
--- a/drivers/media/rc/redrat3.c
+++ b/drivers/media/rc/redrat3.c
@@ -1217,9 +1217,10 @@ static int __devinit redrat3_dev_probe(struct 
usb_interface *intf,
rr3-carrier = 38000;
 
rr3-rc = redrat3_init_rc_dev(rr3);
-   if (!rr3-rc)
+   if (!rr3-rc) {
+   retval = -ENOMEM;
goto error;
-
+   }
setup_timer(rr3-rx_timeout, redrat3_rx_timeout, (unsigned long)rr3);
 
/* we can register the device now, as it is ready */

--
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 3/3] drivers/media/platform/davinci/vpfe_capture.c: fix error return code

2012-09-04 Thread Peter Senna Tschudin
From: Peter Senna Tschudin peter.se...@gmail.com

Convert a nonnegative error return code to a negative one, as returned
elsewhere in the function.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// smpl
(
if@p1 (\(ret  0\|ret != 0\))
 { ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
when != ret
*if(...)
{
  ... when != ret = e2
  when forall
 return ret;
}

// /smpl

Signed-off-by: Peter Senna Tschudin peter.se...@gmail.com

---
 drivers/media/platform/davinci/vpfe_capture.c |   15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/davinci/vpfe_capture.c 
b/drivers/media/platform/davinci/vpfe_capture.c
index 843b138..f99198c 100644
--- a/drivers/media/platform/davinci/vpfe_capture.c
+++ b/drivers/media/platform/davinci/vpfe_capture.c
@@ -1131,11 +1131,11 @@ static int vpfe_s_input(struct file *file, void *priv, 
unsigned int index)
ret = -EBUSY;
goto unlock_out;
}
-
-   if (vpfe_get_subdev_input_index(vpfe_dev,
-   subdev_index,
-   inp_index,
-   index)  0) {
+   ret = vpfe_get_subdev_input_index(vpfe_dev,
+ subdev_index,
+ inp_index,
+ index);
+   if (ret  0) {
v4l2_err(vpfe_dev-v4l2_dev, invalid input index\n);
goto unlock_out;
}
@@ -1748,8 +1748,9 @@ static long vpfe_param_handler(struct file *file, void 
*priv,
Error setting parameters in CCDC\n);
goto unlock_out;
}
-   if (vpfe_get_ccdc_image_format(vpfe_dev,
-  vpfe_dev-fmt)  0) {
+   ret = vpfe_get_ccdc_image_format(vpfe_dev,
+vpfe_dev-fmt);
+   if (ret  0) {
v4l2_dbg(1, debug, vpfe_dev-v4l2_dev,
Invalid image format at CCDC\n);
goto unlock_out;

--
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] [media] rc: filter out not allowed protocols when decoding

2012-09-04 Thread Sean Young
On Tue, Sep 04, 2012 at 11:06:07AM +0800, Changbin Du wrote:
 mutex_lock(ir_raw_handler_lock);
   - list_for_each_entry(handler, ir_raw_handler_list, list)
   - handler-decode(raw-dev, ev);
   + list_for_each_entry(handler, ir_raw_handler_list, list) {
   + /* use all protocol by default */
   + if (raw-dev-allowed_protos == RC_TYPE_UNKNOWN ||
   + raw-dev-allowed_protos  handler-protocols)
   + handler-decode(raw-dev, ev);
   + }
 
  Each IR protocol decoder already checks whether it is enabled or not;
  should it not be so that only allowed protocols can be enabled rather
  than checking both enabled_protocols and allowed_protocols?
 
  Just from reading store_protocols it looks like decoders which aren't
  in allowed_protocols can be enabled, which makes no sense. Also
  ir_raw_event_register all protocols are enabled rather than the
  allowed ones.
 
 
  Lastely I don't know why raw ir drivers should dictate which protocols
  can be enabled. Would it not be better to remove it entirely?
 
 
 I agree with you. I just thought that the only thing a decoder should care
 is its decoding logic, but not including decoder management. My idaea is:
  1) use enabled_protocols to select decoders in ir_raw.c, but not
 placed in decoders to do the judgement.
  2) remove  allowed_protocols or just use it to set the default
 decoder (also should rename allowed_protocols  to default_protocol).

The default decoder should be the one set by the rc keymap.

 I also have a question:
  Is there a requirement that one more decoders are enabled for a
 IR device at the same time?

Yes, you want to be able to multiple remotes on the IR device (which
you can do as long as the scancodes don't overlap, I think), and the 
lirc device is implemented as a decoder, so you might want to see the
raw IR as well as have it decoded.

 And if that will lead to a issue that each decoder may decode a
 same pulse sequence to different evnets since their protocol is
 different?

At the moment, no. David Hardeman has sent a patch for this:

http://patchwork.linuxtv.org/patch/11388/


Sean
--
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: use of timestamp/sequence in v4l2_buffer

2012-09-04 Thread Rémi Denis-Courmont
Le mardi 4 septembre 2012 13:38:06, Hans Verkuil a écrit :
 7) Should the timestamp field always be monotonically increasing? Or it is
 possible to get timestamps that jump around? This makes sense for encoders
 that create B-frames referring to frames captured earlier than an I-frame.

I would expect an encoder to output frames in DTS order rather than PTS and a 
decoder to output frames in PTS order. The timestamp field is the only 
indication of the PCR, so an application might not recover if the timestamp 
jumps backward.

If there is an ambiguity between PTS and DTS, I think it should be documented 
and specified per format what the timestamp is.

-- 
Rémi Denis-Courmont
http://www.remlab.net/
--
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: [PATCHv3 2/9] ir-rx51: Handle signals properly

2012-09-04 Thread Sean Young
On Mon, Sep 03, 2012 at 11:41:55PM +0200, David Härdeman wrote:
 Hej,
 
 On Mon, Sep 03, 2012 at 01:36:53PM +0100, Sean Young wrote:
 On Sun, Sep 02, 2012 at 11:08:20PM +0300, Timo Kokkonen wrote:
  I guess the assumption is to avoid
  breaking the transmission in the middle in case the process is signaled.
  And that's why we shouldn't use interruptible waits.
 
  However, if we allow simply breaking the transmitting in case the
  process is signaled any way during the transmission, then the handling
  would be trivial in the driver. That is, if someone for example kills or
  stops the lirc daemon process, then the IR code just wouldn't finish ever.
  
  Sean, do you have an opinion how this should or is allowed to work?
 
 You want to know when the hardware is done sending the IR. If you return
 EINTR to user space, how would user space know how much IR has been sent, 
 if any?
 
 This ABI is not particularily elegant so there are proposals for a better
 interface which would obsolete the lirc interface. David Hardeman has
 worked on this:
 
 http://patchwork.linuxtv.org/patch/11411/
 
 
 Yes, the first step is an asynchronous interface using a kfifo which is
 managed/fed using functionality in rc-core and drained by the drivers.
 
 The size of the kfifo() itself is the only limiting factor right now,
 but I do think we should eventually add some restrictions on the combined
 duration of the pulse/space timings that are in the queue at any given
 point.

While we're at it, it would be useful to check that there no 0s in 
the timings, I'm not sure how well the drivers deal with those.

 Say, for example, that any given pulse/space value is not allowed to be
 above 500ms and the total duration of the queue is not allowed to be
 above 1000ms. In case user-space wants (for whatever reason)...to write
 a 4000ms space, it would have to do so in 8 messages of 500ms each.
 
 Each message write() provides the opportunity for a interruptible wait
 (in the regular case) or returning EAGAIN (in the O_NONBLOCK case) -
 assuming that the kfifo already holds pulse/space timing totalling
 1000ms and/or is full.
 
 EINTR should only be returned if nothing has been written to the kfifo
 at all.

This interface is much better but it's also an ABI change. How should this
be handled? Should rc-core expose it's own /dev/rc[0-9] device with its
own ioctls?

 That way we would avoid policy in kernel while still making it possible
 to kill a misbehaving user-space process by forcing it to drip feed long
 TX sequences.

Is the purpose of this to prevent one user space program from writing
minutes worth of IR and then killing the process won't help since it's
already in the kfifo?

In that case I'd say that close() should purge the kfifo and user space
needs to do fsync to ensure that all IR has been sent.


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


[GIT PULL FOR v3.6] VIDIOC_OVERLAY regression fix

2012-09-04 Thread Hans Verkuil
Hi Mauro,

While working on something else I suddenly saw that the VIDIOC_OVERLAY support
was broken in 3.6. The vidioc_overlay op in v4l2-ioctls.h receives an unsigned 
int,
whereas the ioctl table in v4l2-ioctls.c assumed that it would be an unsigned 
int
pointer.

To fix this a small conversion function was made.

This patch applies to 3.7. By checking 'v4l2-core' to 'video' in the path of
the file this patch will also apply to 3.6.

All other ioctls that do this dereferencing are fine, it's just this one that's
failing.

Regards,

Hans

The following changes since commit 79e8c7bebb467bbc3f2514d75bba669a3f354324:

  Merge tag 'v3.6-rc3' into staging/for_v3.7 (2012-08-24 11:25:10 -0300)

are available in the git repository at:


  git://linuxtv.org/hverkuil/media_tree.git overlay

for you to fetch changes up to 22ffede236c77ad0d4bff19a9cffe4f27bd05819:

  v4l2-ioctl.c: fix overlay support. (2012-09-04 15:08:01 +0200)


Hans Verkuil (1):
  v4l2-ioctl.c: fix overlay support.

 drivers/media/v4l2-core/v4l2-ioctl.c |8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)
--
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] cx88: Fix reset delays

2012-09-04 Thread Alan Cox
From: Alan Cox a...@linux.intel.com

This was reported in March 2011 by Mirek Slugen, and a simple fix posted at the 
time then
never got fixed and applied. The bug is still present.

Resolves-bug: https://bugzilla.kernel.org/show_bug.cgi?id=37703
Signed-off-by: Alan Cox a...@linux.intel.com
---

 drivers/media/pci/cx88/cx88-cards.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/pci/cx88/cx88-cards.c 
b/drivers/media/pci/cx88/cx88-cards.c
index 4e9d4f7..0c25524 100644
--- a/drivers/media/pci/cx88/cx88-cards.c
+++ b/drivers/media/pci/cx88/cx88-cards.c
@@ -3028,9 +3028,9 @@ static int cx88_xc3028_winfast1800h_callback(struct 
cx88_core *core,
cx_set(MO_GP1_IO, 0x1010);
mdelay(50);
cx_clear(MO_GP1_IO, 0x10);
-   mdelay(50);
+   mdelay(75);
cx_set(MO_GP1_IO, 0x10);
-   mdelay(50);
+   mdelay(75);
return 0;
}
return -EINVAL;

--
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] tlg2300: fix missing check for audio creation

2012-09-04 Thread Alan Cox
From: Alan Cox a...@linux.intel.com

If we fail to set up the capture device we go through negative indexes and
badness happens. Add the missing test.

Resolves-bug: https://bugzilla.kernel.org/show_bug.cgi?id=44551
Signed-off-by: Alan Cox a...@linux.intel.com
---

 drivers/media/usb/tlg2300/pd-alsa.c |4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/media/usb/tlg2300/pd-alsa.c 
b/drivers/media/usb/tlg2300/pd-alsa.c
index 9f8b7da..0c77869 100644
--- a/drivers/media/usb/tlg2300/pd-alsa.c
+++ b/drivers/media/usb/tlg2300/pd-alsa.c
@@ -305,6 +305,10 @@ int poseidon_audio_init(struct poseidon *p)
return ret;
 
ret = snd_pcm_new(card, poseidon audio, 0, 0, 1, pcm);
+   if (ret  0) {
+   snd_free_card(card);
+   return ret;
+   }
snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_CAPTURE, pcm_capture_ops);
pcm-info_flags   = 0;
pcm-private_data = p;

--
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/5] drivers/media/platform/s5p-tv/sdo_drv.c: fix error return code

2012-09-04 Thread Peter Senna Tschudin
From: Peter Senna Tschudin peter.se...@gmail.com

Convert a nonnegative error return code to a negative one, as returned
elsewhere in the function.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// smpl
(
if@p1 (\(ret  0\|ret != 0\))
 { ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
when != ret
*if(...)
{
  ... when != ret = e2
  when forall
 return ret;
}

// /smpl

Signed-off-by: Peter Senna Tschudin peter.se...@gmail.com

---
 drivers/media/platform/s5p-tv/sdo_drv.c |3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/media/platform/s5p-tv/sdo_drv.c 
b/drivers/media/platform/s5p-tv/sdo_drv.c
index ad68bbe..58cf56d 100644
--- a/drivers/media/platform/s5p-tv/sdo_drv.c
+++ b/drivers/media/platform/s5p-tv/sdo_drv.c
@@ -369,6 +369,7 @@ static int __devinit sdo_probe(struct platform_device *pdev)
sdev-fout_vpll = clk_get(dev, fout_vpll);
if (IS_ERR_OR_NULL(sdev-fout_vpll)) {
dev_err(dev, failed to get clock 'fout_vpll'\n);
+   ret = -ENODEV;
goto fail_dacphy;
}
dev_info(dev, fout_vpll.rate = %lu\n, clk_get_rate(sclk_vpll));
@@ -377,11 +378,13 @@ static int __devinit sdo_probe(struct platform_device 
*pdev)
sdev-vdac = devm_regulator_get(dev, vdd33a_dac);
if (IS_ERR_OR_NULL(sdev-vdac)) {
dev_err(dev, failed to get regulator 'vdac'\n);
+   ret = -ENODEV;
goto fail_fout_vpll;
}
sdev-vdet = devm_regulator_get(dev, vdet);
if (IS_ERR_OR_NULL(sdev-vdet)) {
dev_err(dev, failed to get regulator 'vdet'\n);
+   ret = -ENODEV;
goto fail_fout_vpll;
}
 

--
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/5] drivers/media/platform/davinci/vpbe.c: fix error return code

2012-09-04 Thread Peter Senna Tschudin
From: Peter Senna Tschudin peter.se...@gmail.com

Convert a nonnegative error return code to a negative one, as returned
elsewhere in the function.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// smpl
(
if@p1 (\(ret  0\|ret != 0\))
 { ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
when != ret
*if(...)
{
  ... when != ret = e2
  when forall
 return ret;
}

// /smpl

Signed-off-by: Peter Senna Tschudin peter.se...@gmail.com

---
 drivers/media/platform/davinci/vpbe.c |   13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/davinci/vpbe.c 
b/drivers/media/platform/davinci/vpbe.c
index c4a82a1..2e4a0da 100644
--- a/drivers/media/platform/davinci/vpbe.c
+++ b/drivers/media/platform/davinci/vpbe.c
@@ -603,7 +603,6 @@ static int vpbe_initialize(struct device *dev, struct 
vpbe_device *vpbe_dev)
int output_index;
int num_encoders;
int ret = 0;
-   int err;
int i;
 
/*
@@ -646,10 +645,10 @@ static int vpbe_initialize(struct device *dev, struct 
vpbe_device *vpbe_dev)
}
v4l2_info(vpbe_dev-v4l2_dev, vpbe v4l2 device registered\n);
 
-   err = bus_for_each_dev(platform_bus_type, NULL, vpbe_dev,
+   ret = bus_for_each_dev(platform_bus_type, NULL, vpbe_dev,
   platform_device_get);
-   if (err  0)
-   return err;
+   if (ret  0)
+   return ret;
 
vpbe_dev-venc = venc_sub_dev_init(vpbe_dev-v4l2_dev,
   vpbe_dev-cfg-venc.module_name);
@@ -664,11 +663,11 @@ static int vpbe_initialize(struct device *dev, struct 
vpbe_device *vpbe_dev)
osd_device = vpbe_dev-osd_device;
 
if (NULL != osd_device-ops.initialize) {
-   err = osd_device-ops.initialize(osd_device);
-   if (err) {
+   ret = osd_device-ops.initialize(osd_device);
+   if (ret) {
v4l2_err(vpbe_dev-v4l2_dev,
 unable to initialize the OSD device);
-   err = -ENOMEM;
+   ret = -ENOMEM;
goto vpbe_fail_v4l2_device;
}
}

--
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 5/5] drivers/media/platform/omap3isp/isp.c: fix error return code

2012-09-04 Thread Peter Senna Tschudin
From: Peter Senna Tschudin peter.se...@gmail.com

Convert a nonnegative error return code to a negative one, as returned
elsewhere in the function.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// smpl
(
if@p1 (\(ret  0\|ret != 0\))
 { ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
when != ret
*if(...)
{
  ... when != ret = e2
  when forall
 return ret;
}

// /smpl

Signed-off-by: Peter Senna Tschudin peter.se...@gmail.com

---
 drivers/media/platform/omap3isp/isp.c |4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/omap3isp/isp.c 
b/drivers/media/platform/omap3isp/isp.c
index e0096e0..91fcaef 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -2102,8 +2102,10 @@ static int __devinit isp_probe(struct platform_device 
*pdev)
if (ret  0)
goto error;
 
-   if (__omap3isp_get(isp, false) == NULL)
+   if (__omap3isp_get(isp, false) == NULL) {
+   ret = -EBUSY; /* Not sure if EBUSY is best for here */
goto error;
+   }
 
ret = isp_reset(isp);
if (ret  0)

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


[PATCH 4/5] drivers/media/platform/vino.c: fix error return code

2012-09-04 Thread Peter Senna Tschudin
From: Peter Senna Tschudin peter.se...@gmail.com

Convert a nonnegative error return code to a negative one, as returned
elsewhere in the function.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// smpl
(
if@p1 (\(ret  0\|ret != 0\))
 { ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
when != ret
*if(...)
{
  ... when != ret = e2
  when forall
 return ret;
}

// /smpl

Signed-off-by: Peter Senna Tschudin peter.se...@gmail.com

---
 drivers/media/platform/vino.c |1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/platform/vino.c b/drivers/media/platform/vino.c
index cc9110c..6654a28 100644
--- a/drivers/media/platform/vino.c
+++ b/drivers/media/platform/vino.c
@@ -2061,6 +2061,7 @@ static int vino_capture_next(struct vino_channel_settings 
*vcs, int start)
}
if (incoming == 0) {
dprintk(vino_capture_next(): no buffers available\n);
+   err = -ENOMEM;
goto out;
}
 

--
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 3/5] drivers/media/platform/s5p-tv/mixer_video.c: fix error return code

2012-09-04 Thread Peter Senna Tschudin
From: Peter Senna Tschudin peter.se...@gmail.com

Convert a nonnegative error return code to a negative one, as returned
elsewhere in the function.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// smpl
(
if@p1 (\(ret  0\|ret != 0\))
 { ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
when != ret
*if(...)
{
  ... when != ret = e2
  when forall
 return ret;
}

// /smpl

Signed-off-by: Peter Senna Tschudin peter.se...@gmail.com

---
 drivers/media/platform/s5p-tv/mixer_video.c |5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/s5p-tv/mixer_video.c 
b/drivers/media/platform/s5p-tv/mixer_video.c
index a9c6be3..f139fed 100644
--- a/drivers/media/platform/s5p-tv/mixer_video.c
+++ b/drivers/media/platform/s5p-tv/mixer_video.c
@@ -83,6 +83,7 @@ int __devinit mxr_acquire_video(struct mxr_device *mdev,
mdev-alloc_ctx = vb2_dma_contig_init_ctx(mdev-dev);
if (IS_ERR_OR_NULL(mdev-alloc_ctx)) {
mxr_err(mdev, could not acquire vb2 allocator\n);
+   ret = -ENODEV;
goto fail_v4l2_dev;
}
 
@@ -764,8 +765,10 @@ static int mxr_video_open(struct file *file)
}
 
/* leaving if layer is already initialized */
-   if (!v4l2_fh_is_singular_file(file))
+   if (!v4l2_fh_is_singular_file(file)) {
+   ret = -EBUSY; /* Not sure if EBUSY is the best for here */
goto unlock;
+   }
 
/* FIXME: should power be enabled on open? */
ret = mxr_power_get(mdev);

--
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] media: v4l2-ctrls: add control for dpcm predictor

2012-09-04 Thread Sakari Ailus
Hi Prabhakar,

Thanks for the patch. I've got a few comments below.

On Tue, Sep 04, 2012 at 11:07:52AM +0530, Prabhakar Lad wrote:
 From: Lad, Prabhakar prabhakar@ti.com
 
 add V4L2_CID_DPCM_PREDICTOR control of type menu, which
 determines the dpcm predictor. The predictor can be either
 simple or advanced.
 
 Signed-off-by: Lad, Prabhakar prabhakar@ti.com
 Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com
 Acked-by: Hans Verkuil hans.verk...@cisco.com
 Reviewed-by: Sylwester Nawrocki s.nawro...@samsung.com
 Cc: Sakari Ailus sakari.ai...@iki.fi
 Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com
 Cc: Mauro Carvalho Chehab mche...@infradead.org
 Cc: Hans de Goede hdego...@redhat.com
 Cc: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Rob Landley r...@landley.net
 ---
 This patches has one checkpatch warning for line over
 80 characters altough it can be avoided I have kept it
 for consistency.
 
 Changes for v4:
 1: Aligned the description to fit appropriately in the
 para tag, pointed by Sylwester.
 
 Changes for v3:
 1: Added better explanation for DPCM, pointed by Hans.
 
 Changes for v2:
 1: Added documentaion in controls.xml pointed by Sylwester.
 2: Chnaged V4L2_DPCM_PREDICTOR_ADVANCE to V4L2_DPCM_PREDICTOR_ADVANCED
pointed by Sakari.
 
  Documentation/DocBook/media/v4l/controls.xml |   46 
 +-
  drivers/media/v4l2-core/v4l2-ctrls.c |9 +
  include/linux/videodev2.h|5 +++
  3 files changed, 59 insertions(+), 1 deletions(-)
 
 diff --git a/Documentation/DocBook/media/v4l/controls.xml 
 b/Documentation/DocBook/media/v4l/controls.xml
 index 93b9c68..ad873ea 100644
 --- a/Documentation/DocBook/media/v4l/controls.xml
 +++ b/Documentation/DocBook/media/v4l/controls.xml
 @@ -4267,7 +4267,51 @@ interface and may change in the future./para
   pixels / second.
   /entry
 /row
 -   rowentry/entry/row
 +   row
 + entry 
 spanname=idconstantV4L2_CID_DPCM_PREDICTOR/constant/entry
 + entrymenu/entry
 +   /row
 +   row id=v4l2-dpcm-predictor
 + entry spanname=descr Differential pulse-code modulation (DPCM) 
 is a signal
 + encoder that uses the baseline of pulse-code modulation (PCM) but 
 adds some
 + functionalities based on the prediction of the samples of the 
 signal. The input
 + can be an analog signal or a digital signal.
 +
 + paraIf the input is a continuous-time analog signal, it needs to 
 be sampled
 + first so that a discrete-time signal is the input to the DPCM 
 encoder./para
 +
 + paraSimple: take the values of two consecutive samples; if they 
 are analog
 + samples, quantize them; calculate the difference between the first 
 one and the
 + next; the output is the difference, and it can be further entropy 
 coded./para
 +
 + paraAdvanced: instead of taking a difference relative to a 
 previous input sample,
 + take the difference relative to the output of a local model of the 
 decoder process;
 + in this option, the difference can be quantized, which allows a 
 good way to
 + incorporate a controlled loss in the encoding./para

This is directly from Wikipedia, isn't it?

What comes to the content, DPCM in the context of V4L2 media bus codes, as a
digital interface, is always digital. So there's no need to document it.
Entropy coding is also out of the question: the samples of the currently
defined formats are equal in size.

Another thing what I'm not sure is the definition of the simple and advanced
encoders. I've seen sensors that allow you to choose which one to use, but
the documentation hasn't stated what the actual implementation is. Does TI
documentation do so?

In V4L2 documentation we should state what is common in the hardware
documentation, and that is mostly limited to simple and advanced. I
really don't know enough that I could say what the exact implamentation of
those two are in all of the cases.

I suggest we leave just a few words of the DPCM compression itself (roughly
the factual content of the first paragraph with the exception of the
reference to analogue signal) and a link to Wikipedia.

 + paraApplying one of these two processes, short-term redundancy 
 (positive correlation of
 + nearby values) of the signal is eliminated; compression ratios on 
 the order of 2 to 4
 + can be achieved if differences are subsequently entropy coded, 
 because the entropy of
 + the difference signal is much smaller than that of the original 
 discrete signal treated
 + as independent samples.For more information about DPCM see ulink
 + 
 url=http://en.wikipedia.org/wiki/Differential_pulse-code_modulation;Wikipedia/ulink./para
 + /entry
 +   /row
 +   row
 + entrytbl spanname=descr cols=2
 +   tbody valign=top
 + row
 +  

cron job: media_tree daily build: WARNINGS

2012-09-04 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:Tue Sep  4 19:00:18 CEST 2012
git hash:79e8c7bebb467bbc3f2514d75bba669a3f354324
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: WARNINGS
linux-git-x86_64: WARNINGS
linux-2.6.31.12-x86_64: WARNINGS
linux-2.6.32.6-x86_64: WARNINGS
linux-2.6.33-x86_64: WARNINGS
linux-2.6.34-x86_64: WARNINGS
linux-2.6.35.3-x86_64: WARNINGS
linux-2.6.36-x86_64: WARNINGS
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: WARNINGS
linux-2.6.32.6-i686: WARNINGS
linux-2.6.33-i686: WARNINGS
linux-2.6.34-i686: WARNINGS
linux-2.6.35.3-i686: WARNINGS
linux-2.6.36-i686: WARNINGS
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: OK
sparse: ERRORS

Detailed results are available here:

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

Full logs are available here:

http://www.xs4all.nl/~hverkuil/logs/Tuesday.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


SAA7231 chipset support

2012-09-04 Thread Wombachi
Hi,

Google tries to convince me that the Philips / NXP / Trident /
SigmaDesigns  SAA7231  chip is not (and will probably not be)
supported on Linux (mostly because of bad will from the hardware
companies).

Could you please confirm that this chip is not (and will probably not
be) supported [1]  ?

Thanks for your time


[1] as opposed to other SAA71x chips, as seen in 3.2 kernel headers
and on http://linuxtv.org/wiki/index.php/DVB-T_PCI_Cards


You'll find details below (chip details / history, partial output of
lspci -v / vv, hardware, uname -a)


1. Chip history:
- announced by NXP (2008-03):
   
http://www.nxp.com/news/press-releases/2008/03/nxp-launches-world-s-smallest-single-chip-triple-mode-pctv-solution.html
- some guy at a German company began devlopments but this seems discontinued
  http://osdir.com/ml/video4linux-list/2010-07/msg00016.html
  and the src code url is dead / 404:  http://www.jusst.de/hg/saa7231/
- Trident was the next owner. See TV  PC TV  SAAA link from the CX2450X page
  http://www.tridentmicro.com/producttree/stb/satellite-stb/cx2450x/
  but they refused to give any help to linux develpers -- then finally
filed for bankrupcy
  
http://forums.opensuse.org/english/get-technical-help-here/hardware/450034-saa7231-module-2.html
  
https://www.linux.com/community/forums/multimedia/installing-and-using-a-saa7231-tv-card/limit/20/offset/0
- the current owner seems to be Sigma Design for the SAA71x (and
perhaps the SAA72) chips
  http://www.sigmadesigns.com/uploads/library/press_releases/120402.pdf

2. Output of lspci -v

02:00.0 Multimedia controller: Philips Semiconductors SAA7231 (rev ca)
Subsystem: Yuan Yuan Enterprise Co., Ltd. Device 0762
Flags: bus master, fast devsel, latency 0, IRQ 11
Memory at dcc0 (64-bit, non-prefetchable) [size=4M]
Memory at dc80 (64-bit, non-prefetchable) [size=4M]
Capabilities: [40] MSI: Enable- Count=1/16 Maskable- 64bit+
Capabilities: [50] Express Endpoint, MSI 00
Capabilities: [74] Power Management version 3
Capabilities: [7c] Vendor Specific Information: Len=84 ?
Capabilities: [100] Vendor Specific Information: ID= Rev=0
Len=094 ?

and lspci -vv

02:00.0 Multimedia controller: Philips Semiconductors SAA7231 (rev ca)
Subsystem: Yuan Yuan Enterprise Co., Ltd. Device 0762
Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop-
ParErr- Stepping- SERR- FastB2B- DisINTx-
Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast TAbort-
TAbort- MAbort- SERR- PERR- INTx-
Latency: 0, Cache Line Size: 64 bytes
Interrupt: pin A routed to IRQ 11
Region 0: Memory at dcc0 (64-bit, non-prefetchable) [size=4M]
Region 2: Memory at dc80 (64-bit, non-prefetchable) [size=4M]
Capabilities: [40] MSI: Enable- Count=1/16 Maskable- 64bit+
Address:   Data: 
Capabilities: [50] Express (v1) Endpoint, MSI 00
DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s
256ns, L1 1us
ExtTag- AttnBtn- AttnInd- PwrInd- RBE+ FLReset-
DevCtl: Report errors: Correctable- Non-Fatal- Fatal-
Unsupported-
RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop-
MaxPayload 128 bytes, MaxReadReq 128 bytes
DevSta: CorrErr- UncorrErr- FatalErr- UnsuppReq-
AuxPwr+ TransPend-
LnkCap: Port #1, Speed 2.5GT/s, Width x1, ASPM L0s L1,
Latency L0 4us, L1 64us
ClockPM- Surprise- LLActRep- BwNot-
LnkCtl: ASPM Disabled; RCB 128 bytes Disabled- Retrain- CommClk-
ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
LnkSta: Speed 2.5GT/s, Width x1, TrErr- Train-
SlotClk- DLActive- BWMgmt- ABWMgmt-
Capabilities: [74] Power Management version 3
Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=55mA
PME(D0+,D1+,D2+,D3hot+,D3cold+)
Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
Capabilities: [7c] Vendor Specific Information: Len=84 ?
Capabilities: [100 v1] Vendor Specific Information: ID=
Rev=0 Len=094 ?

3. Other details:

Hardware: the laptop with this chipset is an Asus N73S.
Software: Linux Kubuntu. uname -a:
   Linux Newton 3.2.0-29-generic #46-Ubuntu SMP Fri Jul 27 17:03:23
UTC 2012 x86_64 x86_64 x86_64 GNU/Linux
--
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] media: v4l2-ctrls: add control for dpcm predictor

2012-09-04 Thread Prabhakar Lad
Hi Sakari,

Thanks for the review.

On Wednesday 05 September 2012 12:42 AM, Sakari Ailus wrote:
 Hi Prabhakar,
 
 Thanks for the patch. I've got a few comments below.
 
 On Tue, Sep 04, 2012 at 11:07:52AM +0530, Prabhakar Lad wrote:
 From: Lad, Prabhakar prabhakar@ti.com

 add V4L2_CID_DPCM_PREDICTOR control of type menu, which
 determines the dpcm predictor. The predictor can be either
 simple or advanced.

 Signed-off-by: Lad, Prabhakar prabhakar@ti.com
 Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com
 Acked-by: Hans Verkuil hans.verk...@cisco.com
 Reviewed-by: Sylwester Nawrocki s.nawro...@samsung.com
 Cc: Sakari Ailus sakari.ai...@iki.fi
 Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com
 Cc: Mauro Carvalho Chehab mche...@infradead.org
 Cc: Hans de Goede hdego...@redhat.com
 Cc: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Rob Landley r...@landley.net
 ---
 This patches has one checkpatch warning for line over
 80 characters altough it can be avoided I have kept it
 for consistency.

 Changes for v4:
 1: Aligned the description to fit appropriately in the
 para tag, pointed by Sylwester.

 Changes for v3:
 1: Added better explanation for DPCM, pointed by Hans.

 Changes for v2:
 1: Added documentaion in controls.xml pointed by Sylwester.
 2: Chnaged V4L2_DPCM_PREDICTOR_ADVANCE to V4L2_DPCM_PREDICTOR_ADVANCED
pointed by Sakari.

  Documentation/DocBook/media/v4l/controls.xml |   46 
 +-
  drivers/media/v4l2-core/v4l2-ctrls.c |9 +
  include/linux/videodev2.h|5 +++
  3 files changed, 59 insertions(+), 1 deletions(-)

 diff --git a/Documentation/DocBook/media/v4l/controls.xml 
 b/Documentation/DocBook/media/v4l/controls.xml
 index 93b9c68..ad873ea 100644
 --- a/Documentation/DocBook/media/v4l/controls.xml
 +++ b/Documentation/DocBook/media/v4l/controls.xml
 @@ -4267,7 +4267,51 @@ interface and may change in the future./para
  pixels / second.
  /entry
/row
 -  rowentry/entry/row
 +  row
 +entry 
 spanname=idconstantV4L2_CID_DPCM_PREDICTOR/constant/entry
 +entrymenu/entry
 +  /row
 +  row id=v4l2-dpcm-predictor
 +entry spanname=descr Differential pulse-code modulation (DPCM) 
 is a signal
 +encoder that uses the baseline of pulse-code modulation (PCM) but 
 adds some
 +functionalities based on the prediction of the samples of the 
 signal. The input
 +can be an analog signal or a digital signal.
 +
 +paraIf the input is a continuous-time analog signal, it needs to 
 be sampled
 +first so that a discrete-time signal is the input to the DPCM 
 encoder./para
 +
 +paraSimple: take the values of two consecutive samples; if they 
 are analog
 +samples, quantize them; calculate the difference between the first 
 one and the
 +next; the output is the difference, and it can be further entropy 
 coded./para
 +
 +paraAdvanced: instead of taking a difference relative to a 
 previous input sample,
 +take the difference relative to the output of a local model of the 
 decoder process;
 +in this option, the difference can be quantized, which allows a 
 good way to
 +incorporate a controlled loss in the encoding./para
 
 This is directly from Wikipedia, isn't it?
 
Yes.

 What comes to the content, DPCM in the context of V4L2 media bus codes, as a
 digital interface, is always digital. So there's no need to document it.
 Entropy coding is also out of the question: the samples of the currently
 defined formats are equal in size.
 
Ok.

 Another thing what I'm not sure is the definition of the simple and advanced
 encoders. I've seen sensors that allow you to choose which one to use, but
 the documentation hasn't stated what the actual implementation is. Does TI
 documentation do so?
 
Couldn't find much apart from this 'The DPCM compression system uses two
different predictors; one is simple and the other is complex. Predictor1
is very simple, so the processing power and the memory requirements are
reduced with it (when the image quality is already high enough).
Predictor2 gives a slightly better prediction for pixel value and the
image quality can be improved with it.'

 In V4L2 documentation we should state what is common in the hardware
 documentation, and that is mostly limited to simple and advanced. I
 really don't know enough that I could say what the exact implamentation of
 those two are in all of the cases.
 
 I suggest we leave just a few words of the DPCM compression itself (roughly
 the factual content of the first paragraph with the exception of the
 reference to analogue signal) and a link to Wikipedia.
 
Ok.

 +paraApplying one of these two processes, short-term redundancy 
 (positive correlation of
 +nearby values) of the signal is eliminated; compression ratios on 
 the order of 2 to 4
 +can be achieved if differences are subsequently