[GIT PULL stable] fc2580: fix tuning failure on 32-bit arch

2014-04-11 Thread Antti Palosaari

The following changes since commit a83b93a7480441a47856dc9104bea970e84cda87:

  [media] em28xx-dvb: fix PCTV 461e tuner I2C binding (2014-03-31 
08:02:16 -0300)


are available in the git repository at:

  git://linuxtv.org/anttip/media_tree.git fc2580_32bit_calc_overflow

for you to fetch changes up to b9a87259efe859447ea6bf6dfa90cd70ee0b947f:

  fc2580: fix tuning failure on 32-bit arch (2014-04-11 22:56:55 +0300)


Antti Palosaari (1):
  fc2580: fix tuning failure on 32-bit arch

 drivers/media/tuners/fc2580.c  | 6 +++---
 drivers/media/tuners/fc2580_priv.h | 1 +
 2 files changed, 4 insertions(+), 3 deletions(-)

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


cron job: media_tree daily build: OK

2014-04-11 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:   Sat Apr 12 04:00:24 CEST 2014
git branch: test
git hash:   a83b93a7480441a47856dc9104bea970e84cda87
gcc version:i686-linux-gcc (GCC) 4.8.2
sparse version: v0.5.0-11-g38d1124
host hardware:  x86_64
host os:3.13-7.slh.1-amd64

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-exynos: OK
linux-git-arm-mx: OK
linux-git-arm-omap: OK
linux-git-arm-omap1: OK
linux-git-arm-pxa: OK
linux-git-blackfin: OK
linux-git-i686: OK
linux-git-m32r: OK
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
linux-2.6.31.14-i686: OK
linux-2.6.32.27-i686: OK
linux-2.6.33.7-i686: OK
linux-2.6.34.7-i686: OK
linux-2.6.35.9-i686: OK
linux-2.6.36.4-i686: OK
linux-2.6.37.6-i686: OK
linux-2.6.38.8-i686: OK
linux-2.6.39.4-i686: OK
linux-3.0.60-i686: OK
linux-3.1.10-i686: OK
linux-3.2.37-i686: OK
linux-3.3.8-i686: OK
linux-3.4.27-i686: OK
linux-3.5.7-i686: OK
linux-3.6.11-i686: OK
linux-3.7.4-i686: OK
linux-3.8-i686: OK
linux-3.9.2-i686: OK
linux-3.10.1-i686: OK
linux-3.11.1-i686: OK
linux-3.12-i686: OK
linux-3.13-i686: OK
linux-3.14-i686: OK
linux-2.6.31.14-x86_64: OK
linux-2.6.32.27-x86_64: OK
linux-2.6.33.7-x86_64: OK
linux-2.6.34.7-x86_64: OK
linux-2.6.35.9-x86_64: OK
linux-2.6.36.4-x86_64: OK
linux-2.6.37.6-x86_64: OK
linux-2.6.38.8-x86_64: OK
linux-2.6.39.4-x86_64: OK
linux-3.0.60-x86_64: OK
linux-3.1.10-x86_64: OK
linux-3.2.37-x86_64: OK
linux-3.3.8-x86_64: OK
linux-3.4.27-x86_64: OK
linux-3.5.7-x86_64: OK
linux-3.6.11-x86_64: OK
linux-3.7.4-x86_64: OK
linux-3.8-x86_64: OK
linux-3.9.2-x86_64: OK
linux-3.10.1-x86_64: OK
linux-3.11.1-x86_64: OK
linux-3.12-x86_64: OK
linux-3.13-x86_64: OK
linux-3.14-x86_64: OK
apps: OK
spec-git: OK
sparse version: v0.5.0-11-g38d1124
sparse: ERRORS

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API 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


[GIT PULL for v3.15-rc1] media fixes

2014-04-11 Thread Mauro Carvalho Chehab
Hi Linus,

Please pull from:
  git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media 
v4l_for_linus

For a series of bug fix patches for v3.15-rc1. Most are just driver fixes.
There are some changes at remote controller core level, fixing some 
definitions on a new API added for Kernel v3.15.

It also adds the missing include at include/uapi/linux/v4l2-common.h, 
to allow its compilation on userspace, as pointed by you.

Thanks,
Mauro

The following changes since commit 463b21fb27509061b3e97fb4fa69f26d089ddaf4:

  Merge branch 'topic/exynos' of 
git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media (2014-04-05 
13:10:00 -0700)

are available in the git repository at:


  git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media 
v4l_for_linus

for you to fetch changes up to 32654fba2fdb417390efb1af29f1b5693bc91397:

  [media] gpsca: remove the risk of a division by zero (2014-04-08 11:01:12 
-0300)


Antti Palosaari (5):
  [media] msi001: fix possible integer overflow
  [media] msi3101: remove unused variable assignment
  [media] msi3101: check I/O return values on stop streaming
  [media] xc2028: add missing break to switch
  [media] rtl28xxu: remove duplicate ID 0458:707f Genius TVGo DVB-T03

Archit Taneja (9):
  [media] v4l: ti-vpe: Make sure in job_ready that we have the needed 
number of dst_bufs
  [media] v4l: ti-vpe: Use video_device_release_empty
  [media] v4l: ti-vpe: Allow usage of smaller images
  [media] v4l: ti-vpe: report correct capabilities in querycap
  [media] v4l: ti-vpe: Use correct bus_info name for the device in querycap
  [media] v4l: ti-vpe: Fix initial configuration queue data
  [media] v4l: ti-vpe: zero out reserved fields in try_fmt
  [media] v4l: ti-vpe: Set correct field parameter for output and capture 
buffers
  [media] v4l: ti-vpe: retain v4l2_buffer flags for captured buffers

Benjamin Larsson (1):
  [media] r820t: fix size and init values

David Härdeman (3):
  [media] rc-core: do not change 32bit NEC scancode format for now
  [media] rc-core: split dev->s_filter
  [media] rc-core: remove generic scancode filter

Malcolm Priestley (1):
  [media] m88rs2000: fix sparse static warnings

Mauro Carvalho Chehab (3):
  [media] v4l2-common: fix warning when used on userpace
  [media] stk1160: warrant a NUL terminated string
  [media] gpsca: remove the risk of a division by zero

Paul Bolle (1):
  [media] drx-j: use customise option correctly

Shuah Khan (1):
  [media] lgdt3305: include sleep functionality in lgdt3304_ops

 drivers/media/dvb-frontends/drx39xyj/Kconfig |  2 +-
 drivers/media/dvb-frontends/lgdt3305.c   |  1 +
 drivers/media/dvb-frontends/m88rs2000.c  |  8 +--
 drivers/media/platform/ti-vpe/vpe.c  | 45 +
 drivers/media/rc/img-ir/img-ir-hw.c  | 15 -
 drivers/media/rc/img-ir/img-ir-nec.c | 27 
 drivers/media/rc/ir-nec-decoder.c|  5 +-
 drivers/media/rc/keymaps/rc-tivo.c   | 86 
 drivers/media/rc/rc-main.c   | 98 ++--
 drivers/media/tuners/r820t.c |  3 +-
 drivers/media/tuners/tuner-xc2028.c  |  1 +
 drivers/media/usb/dvb-usb-v2/rtl28xxu.c  |  2 -
 drivers/media/usb/gspca/jpeg.h   |  4 +-
 drivers/media/usb/stk1160/stk1160-ac97.c |  2 +-
 drivers/staging/media/msi3101/msi001.c   |  2 +-
 drivers/staging/media/msi3101/sdr-msi3101.c  | 15 +++--
 include/media/rc-core.h  |  8 ++-
 include/uapi/linux/v4l2-common.h |  2 +
 18 files changed, 200 insertions(+), 126 deletions(-)



-- 

Regards,
Mauro
--
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/2] managed token devres interfaces

2014-04-11 Thread Shuah Khan
Here is an example driver use-case for this token resource: This is not 
polished, but should give an idea on this new token resource is intended 
to be used:


- create token resources in em28xx_usb_probe() right before the required 
modules are loaded. It creates tuner, video and audio tokens to use as a 
large grain lock on tuner, video DMA engine, and audio DMA engine 
respectively.


- lock video token in em28xx_init_usb_xfer()
- unlock in em28xx_uninit_usb_xfer()

- destroy token in em28xx_release_resources()


I did some hotplug testing with this:

dmesg | grep token
[  771.613332] usb 8-1: devm_token_create(): created token: 
tuner:8-1-:00:10.1-0

[  771.613335] em2874 #0: devm_token_create() returned 0
[  771.613340] usb 8-1: devm_token_create(): created token: 
video:8-1-:00:10.1

[  851.202931] usb 8-1: devm_token_release(): release token
[  851.202936] usb 8-1: devm_token_release(): release token
[ 1775.052273] usb 8-2: devm_token_create(): created token: 
tuner:8-2-:00:10.1-0

[ 1775.052276] em2874 #0: devm_token_create() returned 0
[ 1775.052285] usb 8-2: devm_token_create(): created token: 
video:8-2-:00:10.1

shuah@anduin:~$ dmesg | grep token
[  771.613332] usb 8-1: devm_token_create(): created token: 
tuner:8-1-:00:10.1-0

[  771.613335] em2874 #0: devm_token_create() returned 0
[  771.613340] usb 8-1: devm_token_create(): created token: 
video:8-1-:00:10.1

[  851.202931] usb 8-1: devm_token_release(): release token
[  851.202936] usb 8-1: devm_token_release(): release token
[ 1775.052273] usb 8-2: devm_token_create(): created token: 
tuner:8-2-:00:10.1-0

[ 1775.052276] em2874 #0: devm_token_create() returned 0
[ 1775.052285] usb 8-2: devm_token_create(): created token: 
video:8-2-:00:10.1

[ 1799.610007] usb 8-2: devm_token_release(): release token
[ 1799.610013] usb 8-2: devm_token_release(): release token


diff --git a/drivers/media/usb/em28xx/em28xx-cards.c 
b/drivers/media/usb/em28xx/em28xx-cards.c

index 50aa5a5..3305250 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -2428,6 +2428,51 @@ static struct em28xx_hash_table em28xx_i2c_hash[] = {
 };
 /* NOTE: introduce a separate hash table for devices with 16 bit 
eeproms */


+/* interfaces to create and destroy token resources */
+static int em28xx_create_token_resources(struct em28xx *dev)
+{
+   int rc = 0;
+
+   /* create token devres for tuner */
+   snprintf(dev->tuner_tkn_id, sizeof(dev->tuner_tkn_id),
+   "tuner:%s-%s-%d",
+   dev_name(&dev->udev->dev),
+   dev->udev->bus->bus_name,
+   dev->tuner_addr);
+
+   rc = devm_token_create(&dev->udev->dev, dev->tuner_tkn_id);
+   em28xx_info("devm_token_create() returned %d\n", rc);
+
+   if (dev->has_video || dev->board.has_dvb) {
+   snprintf(dev->video_tkn_id, sizeof(dev->video_tkn_id),
+   "video:%s-%s",
+   dev_name(&dev->udev->dev),
+   dev->udev->bus->bus_name);
+   rc = devm_token_create(&dev->udev->dev, dev->video_tkn_id);
+   }
+   if (dev->audio_mode.has_audio) {
+   snprintf(dev->audio_tkn_id, sizeof(dev->audio_tkn_id),
+   "audio:%s-%s",
+   dev_name(&dev->udev->dev),
+   dev->udev->bus->bus_name);
+   rc = devm_token_create(&dev->udev->dev, dev->audio_tkn_id);
+   }
+   return rc;
+}
+
+static int em28xx_destroy_token_resources(struct em28xx *dev)
+{
+   int rc = 0;
+
+   devm_token_destroy(&dev->udev->dev, dev->tuner_tkn_id);
+   if (dev->has_video || dev->board.has_dvb)
+   rc = devm_token_destroy(&dev->udev->dev, dev->video_tkn_id);
+   if (dev->audio_mode.has_audio)
+   rc = devm_token_destroy(&dev->udev->dev, dev->audio_tkn_id);
+
+   return rc;
+}

 int em28xx_tuner_callback(void *ptr, int component, int command, int arg)
 {
struct em28xx_i2c_bus *i2c_bus = ptr;
@@ -2949,6 +2994,10 @@ static void em28xx_release_resources(struct 
em28xx *dev)

em28xx_i2c_unregister(dev, 1);
em28xx_i2c_unregister(dev, 0);

+   /* destroy token resources */
+   if (em28xx_destroy_token_resources(dev))
+   em28xx_info("Error destroying token resources\n");
+
usb_put_dev(dev->udev);

/* Mark device as unused */
@@ -3431,6 +3480,10 @@ static int em28xx_usb_probe(struct usb_interface 
*interface,


kref_init(&dev->ref);

+   /* create token resources before requesting for modules */
+   if (em28xx_create_token_resources(dev))
+   em28xx_info("Error creating token resources\n");
+
request_modules(dev);

/* Should be the last thing to do, to avoid newer udev's to
diff --git a/drivers/media/usb/em28xx/em28xx-core.c 
b/drivers/media/usb/em28xx/em28xx-core.c

index 523d7e9..8129a9c 100644
--- a/drivers

[PATCH] fc2580: fix tuning failure on 32-bit arch

2014-04-11 Thread Antti Palosaari
There was some frequency calculation overflows which caused tuning
failure on 32-bit architecture. Use 64-bit numbers where needed in
order to avoid calculation overflows.

Thanks for the Finnish person, who asked remain anonymous, reporting,
testing and suggesting the fix.

Cc: 
Signed-off-by: Antti Palosaari 
---
 drivers/media/tuners/fc2580.c  | 6 +++---
 drivers/media/tuners/fc2580_priv.h | 1 +
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/media/tuners/fc2580.c b/drivers/media/tuners/fc2580.c
index 3aecaf4..f0c9c42 100644
--- a/drivers/media/tuners/fc2580.c
+++ b/drivers/media/tuners/fc2580.c
@@ -195,7 +195,7 @@ static int fc2580_set_params(struct dvb_frontend *fe)
 
f_ref = 2UL * priv->cfg->clock / r_val;
n_val = div_u64_rem(f_vco, f_ref, &k_val);
-   k_val_reg = 1UL * k_val * (1 << 20) / f_ref;
+   k_val_reg = div_u64(1ULL * k_val * (1 << 20), f_ref);
 
ret = fc2580_wr_reg(priv, 0x18, r18_val | ((k_val_reg >> 16) & 0xff));
if (ret < 0)
@@ -348,8 +348,8 @@ static int fc2580_set_params(struct dvb_frontend *fe)
if (ret < 0)
goto err;
 
-   ret = fc2580_wr_reg(priv, 0x37, 1UL * priv->cfg->clock * \
-   fc2580_if_filter_lut[i].mul / 10);
+   ret = fc2580_wr_reg(priv, 0x37, div_u64(1ULL * priv->cfg->clock *
+   fc2580_if_filter_lut[i].mul, 10));
if (ret < 0)
goto err;
 
diff --git a/drivers/media/tuners/fc2580_priv.h 
b/drivers/media/tuners/fc2580_priv.h
index be38a9e..646c994 100644
--- a/drivers/media/tuners/fc2580_priv.h
+++ b/drivers/media/tuners/fc2580_priv.h
@@ -22,6 +22,7 @@
 #define FC2580_PRIV_H
 
 #include "fc2580.h"
+#include 
 
 struct fc2580_reg_val {
u8 reg;
-- 
1.9.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


Re: [PATCH 2/2] [RFC v2 with seqcount] reservation: add suppport for read-only access using rcu

2014-04-11 Thread Thomas Hellstrom
On 04/11/2014 08:09 PM, Maarten Lankhorst wrote:
> op 11-04-14 12:11, Thomas Hellstrom schreef:
>> On 04/11/2014 11:24 AM, Maarten Lankhorst wrote:
>>> op 11-04-14 10:38, Thomas Hellstrom schreef:
 Hi, Maarten.

 Here I believe we encounter a lot of locking inconsistencies.

 First, it seems you're use a number of pointers as RCU pointers
 without
 annotating them as such and use the correct rcu
 macros when assigning those pointers.

 Some pointers (like the pointers in the shared fence list) are both
 used
 as RCU pointers (in dma_buf_poll()) for example,
 or considered protected by the seqlock
 (reservation_object_get_fences_rcu()), which I believe is OK, but then
 the pointers must
 be assigned using the correct rcu macros. In the memcpy in
 reservation_object_get_fences_rcu() we might get away with an
 ugly typecast, but with a verbose comment that the pointers are
 considered protected by the seqlock at that location.

 So I've updated (attached) the headers with proper __rcu annotation
 and
 locking comments according to how they are being used in the various
 reading functions.
 I believe if we want to get rid of this we need to validate those
 pointers using the seqlock as well.
 This will generate a lot of sparse warnings in those places needing
 rcu_dereference()
 rcu_assign_pointer()
 rcu_dereference_protected()

 With this I think we can get rid of all ACCESS_ONCE macros: It's not
 needed when the rcu_x() macros are used, and
 it's never needed for the members protected by the seqlock, (provided
 that the seq is tested). The only place where I think that's
 *not* the case is at the krealloc in
 reservation_object_get_fences_rcu().

 Also I have some more comments in the
 reservation_object_get_fences_rcu() function below:
>>> I felt that the barriers needed for rcu were already provided by
>>> checking the seqcount lock.
>>> But looking at rcu_dereference makes it seem harmless to add it in
>>> more places, it handles
>>> the ACCESS_ONCE and barrier() for us.
>> And it makes the code more maintainable, and helps sparse doing a lot of
>> checking for us. I guess
>> we can tolerate a couple of extra barriers for that.
>>
>>> We could probably get away with using RCU_INIT_POINTER on the writer
>>> side,
>>> because the smp_wmb is already done by arranging seqcount updates
>>> correctly.
>> Hmm. yes, probably. At least in the replace function. I think if we do
>> it in other places, we should add comments as to where
>> the smp_wmb() is located, for future reference.
>>
>>
>> Also  I saw in a couple of places where you're checking the shared
>> pointers, you're not checking for NULL pointers, which I guess may
>> happen if shared_count and pointers are not in full sync?
>>
> No, because shared_count is protected with seqcount. I only allow
> appending to the array, so when
> shared_count is validated by seqcount it means that the
> [0...shared_count) indexes are valid and non-null.
> What could happen though is that the fence at a specific index is
> updated with another one from the same
> context, but that's harmless.
>

Hmm, doesn't attaching an exclusive fence clear all shared fence
pointers from under a reader?

/Thomas





> ~Maarten
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
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/2] [RFC v2 with seqcount] reservation: add suppport for read-only access using rcu

2014-04-11 Thread Thomas Hellstrom
Hi!

On 04/11/2014 08:09 PM, Maarten Lankhorst wrote:
> op 11-04-14 12:11, Thomas Hellstrom schreef:
>> On 04/11/2014 11:24 AM, Maarten Lankhorst wrote:
>>> op 11-04-14 10:38, Thomas Hellstrom schreef:
 Hi, Maarten.

 Here I believe we encounter a lot of locking inconsistencies.

 First, it seems you're use a number of pointers as RCU pointers
 without
 annotating them as such and use the correct rcu
 macros when assigning those pointers.

 Some pointers (like the pointers in the shared fence list) are both
 used
 as RCU pointers (in dma_buf_poll()) for example,
 or considered protected by the seqlock
 (reservation_object_get_fences_rcu()), which I believe is OK, but then
 the pointers must
 be assigned using the correct rcu macros. In the memcpy in
 reservation_object_get_fences_rcu() we might get away with an
 ugly typecast, but with a verbose comment that the pointers are
 considered protected by the seqlock at that location.

 So I've updated (attached) the headers with proper __rcu annotation
 and
 locking comments according to how they are being used in the various
 reading functions.
 I believe if we want to get rid of this we need to validate those
 pointers using the seqlock as well.
 This will generate a lot of sparse warnings in those places needing
 rcu_dereference()
 rcu_assign_pointer()
 rcu_dereference_protected()

 With this I think we can get rid of all ACCESS_ONCE macros: It's not
 needed when the rcu_x() macros are used, and
 it's never needed for the members protected by the seqlock, (provided
 that the seq is tested). The only place where I think that's
 *not* the case is at the krealloc in
 reservation_object_get_fences_rcu().

 Also I have some more comments in the
 reservation_object_get_fences_rcu() function below:
>>> I felt that the barriers needed for rcu were already provided by
>>> checking the seqcount lock.
>>> But looking at rcu_dereference makes it seem harmless to add it in
>>> more places, it handles
>>> the ACCESS_ONCE and barrier() for us.
>> And it makes the code more maintainable, and helps sparse doing a lot of
>> checking for us. I guess
>> we can tolerate a couple of extra barriers for that.
>>
>>> We could probably get away with using RCU_INIT_POINTER on the writer
>>> side,
>>> because the smp_wmb is already done by arranging seqcount updates
>>> correctly.
>> Hmm. yes, probably. At least in the replace function. I think if we do
>> it in other places, we should add comments as to where
>> the smp_wmb() is located, for future reference.
>>
>>
>> Also  I saw in a couple of places where you're checking the shared
>> pointers, you're not checking for NULL pointers, which I guess may
>> happen if shared_count and pointers are not in full sync?
>>
> No, because shared_count is protected with seqcount. I only allow
> appending to the array, so when
> shared_count is validated by seqcount it means that the
> [0...shared_count) indexes are valid and non-null.
> What could happen though is that the fence at a specific index is
> updated with another one from the same
> context, but that's harmless.

Hmm.
Shouldn't we have a way to clean signaled fences from reservation
objects? Perhaps when we attach a new fence, or after a wait with
ww_mutex held? Otherwise we'd have a lot of completely unused fence
objects hanging around for no reason. I don't think we need to be as
picky as TTM, but I think we should do something?

/Thomas



>
> ~Maarten
--
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/2] [RFC v2 with seqcount] reservation: add suppport for read-only access using rcu

2014-04-11 Thread Maarten Lankhorst

op 11-04-14 12:11, Thomas Hellstrom schreef:

On 04/11/2014 11:24 AM, Maarten Lankhorst wrote:

op 11-04-14 10:38, Thomas Hellstrom schreef:

Hi, Maarten.

Here I believe we encounter a lot of locking inconsistencies.

First, it seems you're use a number of pointers as RCU pointers without
annotating them as such and use the correct rcu
macros when assigning those pointers.

Some pointers (like the pointers in the shared fence list) are both used
as RCU pointers (in dma_buf_poll()) for example,
or considered protected by the seqlock
(reservation_object_get_fences_rcu()), which I believe is OK, but then
the pointers must
be assigned using the correct rcu macros. In the memcpy in
reservation_object_get_fences_rcu() we might get away with an
ugly typecast, but with a verbose comment that the pointers are
considered protected by the seqlock at that location.

So I've updated (attached) the headers with proper __rcu annotation and
locking comments according to how they are being used in the various
reading functions.
I believe if we want to get rid of this we need to validate those
pointers using the seqlock as well.
This will generate a lot of sparse warnings in those places needing
rcu_dereference()
rcu_assign_pointer()
rcu_dereference_protected()

With this I think we can get rid of all ACCESS_ONCE macros: It's not
needed when the rcu_x() macros are used, and
it's never needed for the members protected by the seqlock, (provided
that the seq is tested). The only place where I think that's
*not* the case is at the krealloc in
reservation_object_get_fences_rcu().

Also I have some more comments in the
reservation_object_get_fences_rcu() function below:

I felt that the barriers needed for rcu were already provided by
checking the seqcount lock.
But looking at rcu_dereference makes it seem harmless to add it in
more places, it handles
the ACCESS_ONCE and barrier() for us.

And it makes the code more maintainable, and helps sparse doing a lot of
checking for us. I guess
we can tolerate a couple of extra barriers for that.


We could probably get away with using RCU_INIT_POINTER on the writer
side,
because the smp_wmb is already done by arranging seqcount updates
correctly.

Hmm. yes, probably. At least in the replace function. I think if we do
it in other places, we should add comments as to where
the smp_wmb() is located, for future reference.


Also  I saw in a couple of places where you're checking the shared
pointers, you're not checking for NULL pointers, which I guess may
happen if shared_count and pointers are not in full sync?


No, because shared_count is protected with seqcount. I only allow appending to 
the array, so when
shared_count is validated by seqcount it means that the [0...shared_count) 
indexes are valid and non-null.
What could happen though is that the fence at a specific index is updated with 
another one from the same
context, but that's harmless.

~Maarten
--
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/RFC v3 0/5] LED / flash API integration

2014-04-11 Thread Jacek Anaszewski
This is is the third version of the patch series being a follow up
of the discussion on Media summit 2013-10-23, related to the
LED / flash API integration (the notes from the discussion were
enclosed in the message [1], paragraph 5).
The series is based on linux-next next-20140328

Description of the proposed modifications according to
the kernel components they are relevant to:
- LED subsystem modifications
* added led_flash module which, when enabled in the config,
  registers flash specific sysfs attributes:
- flash_brightness
- max_flash_brightness
- indicator_brightness
- max_indicator_brightness
- flash_timeout
- max_flash_timeout
- flash_strobe
- flash_fault
- external_strobe
and exposes kernel internal API
- led_set_flash_strobe
- led_get_flash_strobe
- led_set_indicator_brightness
- led_update_indicator_brightness
- led_set_flash_timeout
- led_get_flash_fault
- led_set_external_strobe
- led_sysfs_lock
- led_sysfs_unlock
- Addition of a V4L2 Flash sub-device registration helpers
* added v4l2-flash.c and v4l2-flash.h files with helper
  functions that facilitate registration/unregistration
  of a subdevice, which wrapps a LED subsystem device and
  exposes V4L2 Flash control interface
- Addition of a driver for the flash cell of the MAX77693 mfd
* the driver exploits the newly introduced mechanism
- Update of the max77693.txt DT bindings documentation


Changes since v2


- refactored the code so that it is possible to build
  led-core without led-flash module
- added v4l2-flash ops which slackens dependency from
  the led-flash module
- implemented led_clamp_align_val function and led_ctrl
  structure which allows to align led control values
  in the manner compatible with V4L2 Flash controls;
  the flash brightness and timeout units have been defined
  as microamperes and microseconds respectively to properly
  support devices which define current and time levels
  as fractions of 1/1000.
- added support for the flash privacy leds
- modified LED sysfs locking mechanism - now it locks/unlocks
  the interface on V4L2 Flash sub-device file open/close
- changed hw_triggered attribute name to external_strobe,
  which maps on the V4L2_FLASH_STROBE_SOURCE_EXTERNAL name 
  more intuitively
- made external_strobe and indicator related sysfs attributes
  created optionally only if related features are declared
  by the led device driver
- removed from the series patches modifying exynos4-is media
  controller - a proposal for "flash manager" which will take
  care of flash devices registration is due to be submitted
- removed modifications to the LED class devices documentation,
  it will be covered after the whole functionality is accepted

Thanks,
Jacek Anaszewski

[1] http://www.spinics.net/lists/linux-media/msg69253.html

Jacek Anaszewski (5):
  leds: Add sysfs and kernel internal API for flash LEDs
  leds: Improve and export led_update_brightness function
  leds: Add support for max77693 mfd flash cell
  DT: Add documentation for the mfd Maxim max77693 flash cell
  media: Add registration helpers for V4L2 flash sub-devices

 Documentation/devicetree/bindings/mfd/max77693.txt |   57 ++
 drivers/leds/Kconfig   |   18 +
 drivers/leds/Makefile  |2 +
 drivers/leds/led-class.c   |   42 +-
 drivers/leds/led-core.c|   16 +
 drivers/leds/led-flash.c   |  627 
 drivers/leds/led-triggers.c|   16 +-
 drivers/leds/leds-max77693.c   |  794 
 drivers/leds/leds.h|6 +
 drivers/media/v4l2-core/Kconfig|   10 +
 drivers/media/v4l2-core/Makefile   |2 +
 drivers/media/v4l2-core/v4l2-flash.c   |  393 ++
 drivers/mfd/max77693.c |2 +-
 include/linux/leds.h   |   60 +-
 include/linux/leds_flash.h |  252 +++
 include/linux/mfd/max77693.h   |   38 +
 include/media/v4l2-flash.h |  119 +++
 17 files changed, 2433 insertions(+), 21 deletions(-)
 create mode 100644 drivers/leds/led-flash.c
 create mode 100644 drivers/leds/leds-max77693.c
 create mode 100644 drivers/media/v4l2-core/v4l2-flash.c
 create mode 100644 include/linux/leds_flash.h
 create mode 100644 include/media/v4l2-flash.h

-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
th

[PATCH/RFC v3 2/5] leds: Improve and export led_update_brightness function

2014-04-11 Thread Jacek Anaszewski
led_update_brightness helper function used to be exploited
only locally in the led-class.c module, where its result was
being passed to the brightness_show sysfs callback. With the
introduction of v4l2-flash subdevice the same functionality
became required for reading current brightness from a LED
device. This patch adds checking brightness_get callback
error code and adds the function to the LED subsystem
public API.

Signed-off-by: Jacek Anaszewski 
Acked-by: Kyungmin Park 
Cc: Bryan Wu 
Cc: Richard Purdie 
---
 drivers/leds/led-class.c |6 --
 drivers/leds/led-core.c  |   16 
 include/linux/leds.h |   10 ++
 3 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 58f16c3..7f285d7 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -24,12 +24,6 @@
 
 static struct class *leds_class;
 
-static void led_update_brightness(struct led_classdev *led_cdev)
-{
-   if (led_cdev->brightness_get)
-   led_cdev->brightness = led_cdev->brightness_get(led_cdev);
-}
-
 static ssize_t brightness_show(struct device *dev,
struct device_attribute *attr, char *buf)
 {
diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index 71b40d3..376166c 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -126,3 +126,19 @@ void led_set_brightness(struct led_classdev *led_cdev,
__led_set_brightness(led_cdev, brightness);
 }
 EXPORT_SYMBOL(led_set_brightness);
+
+int led_update_brightness(struct led_classdev *led_cdev)
+{
+   int ret = 0;
+
+   if (led_cdev->brightness_get) {
+   ret = led_cdev->brightness_get(led_cdev);
+   if (ret >= 0) {
+   led_cdev->brightness = ret;
+   return 0;
+   }
+   }
+
+   return ret;
+}
+EXPORT_SYMBOL(led_update_brightness);
diff --git a/include/linux/leds.h b/include/linux/leds.h
index a794817..d085c21 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -174,6 +174,16 @@ extern void led_blink_set_oneshot(struct led_classdev 
*led_cdev,
  */
 extern void led_set_brightness(struct led_classdev *led_cdev,
   enum led_brightness brightness);
+/**
+ * led_update_brightness - update LED brightness
+ * @led_cdev: the LED to query
+ *
+ * Get an LED's current brightness and update led_cdev->brightness
+ * member with the obtained value.
+ *
+ * Returns: 0 on success or negative error value on failure
+ */
+extern int led_update_brightness(struct led_classdev *led_cdev);
 
 /**
  * led_sysfs_is_locked
-- 
1.7.9.5

--
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/RFC v3 3/5] leds: Add support for max77693 mfd flash cell

2014-04-11 Thread Jacek Anaszewski
This patch adds led-flash support to Maxim max77693 chipset.
A device can be exposed to user space through LED subsystem
sysfs interface or through V4L2 subdevice when the support
for V4L2 Flash sub-devices is enabled. Device supports up to
two leds which can work in flash and torch mode. Leds can
be triggered externally or by software.

Signed-off-by: Andrzej Hajda 
Signed-off-by: Jacek Anaszewski 
Acked-by: Kyungmin Park 
Cc: Bryan Wu 
Cc: Richard Purdie 
Cc: SangYoung Son 
Cc: Samuel Ortiz 
Cc: Lee Jones 
---
 drivers/leds/Kconfig |   10 +
 drivers/leds/Makefile|1 +
 drivers/leds/leds-max77693.c |  794 ++
 drivers/mfd/max77693.c   |2 +-
 include/linux/mfd/max77693.h |   38 ++
 5 files changed, 844 insertions(+), 1 deletion(-)
 create mode 100644 drivers/leds/leds-max77693.c

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 1e1c81f..b2152a6 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -462,6 +462,16 @@ config LEDS_TCA6507
  LED driver chips accessed via the I2C bus.
  Driver support brightness control and hardware-assisted blinking.
 
+config LEDS_MAX77693
+   tristate "LED support for MAX77693 Flash"
+   depends on LEDS_CLASS_FLASH
+   depends on MFD_MAX77693
+   depends on OF
+   help
+ This option enables support for the flash part of the MAX77693
+ multifunction device. It has build in control for two leds in flash
+ and torch mode.
+
 config LEDS_MAX8997
tristate "LED support for MAX8997 PMIC"
depends on LEDS_CLASS && MFD_MAX8997
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 8861b86..64f6234 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -52,6 +52,7 @@ obj-$(CONFIG_LEDS_MC13783)+= leds-mc13783.o
 obj-$(CONFIG_LEDS_NS2) += leds-ns2.o
 obj-$(CONFIG_LEDS_NETXBIG) += leds-netxbig.o
 obj-$(CONFIG_LEDS_ASIC3)   += leds-asic3.o
+obj-$(CONFIG_LEDS_MAX77693)+= leds-max77693.o
 obj-$(CONFIG_LEDS_MAX8997) += leds-max8997.o
 obj-$(CONFIG_LEDS_LM355x)  += leds-lm355x.o
 obj-$(CONFIG_LEDS_BLINKM)  += leds-blinkm.o
diff --git a/drivers/leds/leds-max77693.c b/drivers/leds/leds-max77693.c
new file mode 100644
index 000..979736c
--- /dev/null
+++ b/drivers/leds/leds-max77693.c
@@ -0,0 +1,794 @@
+/*
+ * Copyright (C) 2014, Samsung Electronics Co., Ltd.
+ *
+ * Authors: Andrzej Hajda 
+ *  Jacek Anaszewski 
+ *
+ * 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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define MAX77693_LED_NAME  "max77693-flash"
+
+#define MAX77693_TORCH_IOUT_BITS   4
+
+#define MAX77693_TORCH_NO_TIMER0x40
+#define MAX77693_FLASH_TIMER_LEVEL 0x80
+
+#define MAX77693_FLASH_EN_OFF  0
+#define MAX77693_FLASH_EN_FLASH1
+#define MAX77693_FLASH_EN_TORCH2
+#define MAX77693_FLASH_EN_ON   3
+
+#define MAX77693_FLASH_EN1_SHIFT   6
+#define MAX77693_FLASH_EN2_SHIFT   4
+#define MAX77693_TORCH_EN1_SHIFT   2
+#define MAX77693_TORCH_EN2_SHIFT   0
+
+#define MAX77693_FLASH_LOW_BATTERY_EN  0x80
+
+#define MAX77693_FLASH_BOOST_FIXED 0x04
+#define MAX77693_FLASH_BOOST_LEDNUM_2  0x80
+
+#define MAX77693_FLASH_TIMEOUT_MIN 62500
+#define MAX77693_FLASH_TIMEOUT_MAX 100
+#define MAX77693_FLASH_TIMEOUT_STEP62500
+
+#define MAX77693_TORCH_TIMEOUT_MIN 262000
+#define MAX77693_TORCH_TIMEOUT_MAX 15728000
+
+#define MAX77693_FLASH_IOUT_MIN15625
+#define MAX77693_FLASH_IOUT_MAX_1LED   100
+#define MAX77693_FLASH_IOUT_MAX_2LEDS  625000
+#define MAX77693_FLASH_IOUT_STEP   15625
+
+#define MAX77693_TORCH_IOUT_MIN15625
+#define MAX77693_TORCH_IOUT_MAX25
+#define MAX77693_TORCH_IOUT_STEP   15625
+
+#define MAX77693_FLASH_VSYS_MIN2400
+#define MAX77693_FLASH_VSYS_MAX3400
+#define MAX77693_FLASH_VSYS_STEP   33
+
+#define MAX77693_FLASH_VOUT_MIN3300
+#define MAX77693_FLASH_VOUT_MAX5500
+#define MAX77693_FLASH_VOUT_STEP   25
+#define MAX77693_FLASH_VOUT_RMIN   0x0c
+
+#define MAX77693_LED_STATUS_FLASH_ON   (1 << 3)
+#define MAX77693_LED_STATUS_TORCH_ON   (1 << 2)
+
+#define MAX77693_LED_FLASH_INT_FLED2_OPEN  (1 << 0)
+#define MAX77693_LED_FLASH_INT_FLED2_SHORT (1 << 1)
+#define MAX77693_LED_FLASH_INT_FLED1_OPEN  (1 << 2)
+#define MAX77693_LED_FLASH_INT_FLED1_SHORT (1 << 3)
+#define MAX77693_LED_FLASH_INT_OVER_CURRENT(1 << 4)
+
+#define MAX77693_MODE_OFF  0
+#define MAX77693_MODE_FLASH 

[PATCH/RFC v3 4/5] DT: Add documentation for the mfd Maxim max77693 flash cell

2014-04-11 Thread Jacek Anaszewski
This patch adds device tree binding documentation for
the flash cell of the Maxim max77693 multifunctional device.

Signed-off-by: Andrzej Hajda 
Acked-by: Kyungmin Park 
Cc: Rob Herring 
Cc: Pawel Moll 
Cc: Mark Rutland 
Cc: Ian Campbell 
Cc: Kumar Gala 
---
 Documentation/devicetree/bindings/mfd/max77693.txt |   57 
 1 file changed, 57 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/max77693.txt 
b/Documentation/devicetree/bindings/mfd/max77693.txt
index 11921cc..f58d192 100644
--- a/Documentation/devicetree/bindings/mfd/max77693.txt
+++ b/Documentation/devicetree/bindings/mfd/max77693.txt
@@ -27,6 +27,53 @@ Optional properties:
 
[*] refer Documentation/devicetree/bindings/regulator/regulator.txt
 
+Optional node:
+- led-flash : the LED submodule device node
+
+Required properties of "led-flash" node:
+- compatible : must be "maxim,max77693-flash"
+
+Optional properties of "led-flash" node:
+- maxim,iout : Array of four maximum intensities in microamperes of the current
+   in order: flash1, flash2, torch1, torch2.
+   Range:
+   flash - 15625 - 100 (max 625000 if boost mode
+is enabled for both outputs),
+   torch - 15625 - 25.
+- maxim,trigger : Array of flags indicating which trigger can activate given 
led
+   in order: flash1, flash2, torch1, torch2.
+   Possible flag values (can be combined):
+   1 - FLASH pin of the chip,
+   2 - TORCH pin of the chip,
+   4 - software via I2C command.
+- maxim,trigger-type : Array of trigger types in order: flash, torch.
+   Possible trigger types:
+   0 - Rising edge of the signal triggers the flash/torch,
+   1 - Signal level controls duration of the flash/torch.
+- maxim,timeout : Array of timeouts in microseconds after which leds are
+   turned off in order: flash, torch.
+   Range:
+   flash: 62500 - 100,
+   torch: 0 (no timeout) - 15728000.
+- maxim,boost-mode : Array of the flash boost modes in order: flash1, flash2.
+   If both current outputs are connected then the same non-zero value
+   has to be set for them. This setting influences also maximum
+   current value for torch and flash modes:
+   flash1 and flash2 set to 1 or 2:
+   - max flash current: 1250 mA (625 mA on each output)
+   - max torch current: 500 mA (250 mA on each output)
+   flash1 or flash2 set to 0:
+   - max flash current: 1000 mA
+   - max torch current: 250 mA
+   Possible values:
+   0 - no boost,
+   1 - adaptive mode,
+   2 - fixed mode.
+- maxim,boost-vout : Output voltage of the boost module in millivolts.
+- maxim,vsys-min : Low input voltage level in millivolts. Flash is not fired
+   if chip estimates that system voltage could drop below this level due
+   to flash power consumption.
+
 Example:
max77693@66 {
compatible = "maxim,max77693";
@@ -52,4 +99,14 @@ Example:
regulator-boot-on;
};
};
+   led_flash: led-flash {
+   compatible = "maxim,max77693-flash";
+   maxim,iout = <625000 625000 25 25>;
+   maxim,trigger = <5 5 6 6>;
+   maxim,trigger-type = <0 1>;
+   maxim,timeout = <50 0>;
+   maxim,boost-mode = <1 1>;
+   maxim,boost-vout = <5000>;
+   maxim,vsys-min = <2400>;
+   };
};
-- 
1.7.9.5

--
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/RFC v3 1/5] leds: Add sysfs and kernel internal API for flash LEDs

2014-04-11 Thread Jacek Anaszewski
Some LED devices support two operation modes - torch and
flash. This patch provides support for flash LED devices
in the LED subsystem by introducing new sysfs attributes
and kernel internal interface. The attributes being
introduced are: flash_brightness, flash_strobe, flash_timeout,
max_flash_timeout, max_flash_brightness, flash_fault and
optional external_strobe, indicator_brightness and
max_indicator_btightness. All the flash related features
are placed in a separate module.
The modifications aim to be compatible with V4L2 framework
requirements related to the flash devices management. The
design assumes that V4L2 sub-device can take of the LED class
device control and communicate with it through the kernel
internal interface. When V4L2 Flash sub-device file is
opened, the LED class device sysfs interface is made
unavailable.

Signed-off-by: Jacek Anaszewski 
Acked-by: Kyungmin Park 
Cc: Bryan Wu 
Cc: Richard Purdie 
---
 drivers/leds/Kconfig|8 +
 drivers/leds/Makefile   |1 +
 drivers/leds/led-class.c|   36 ++-
 drivers/leds/led-flash.c|  627 +++
 drivers/leds/led-triggers.c |   16 +-
 drivers/leds/leds.h |6 +
 include/linux/leds.h|   50 +++-
 include/linux/leds_flash.h  |  252 +
 8 files changed, 982 insertions(+), 14 deletions(-)
 create mode 100644 drivers/leds/led-flash.c
 create mode 100644 include/linux/leds_flash.h

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 2062682..1e1c81f 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -19,6 +19,14 @@ config LEDS_CLASS
  This option enables the led sysfs class in /sys/class/leds.  You'll
  need this to do anything useful with LEDs.  If unsure, say N.
 
+config LEDS_CLASS_FLASH
+   tristate "Flash LEDs Support"
+   depends on LEDS_CLASS
+   help
+ This option enables support for flash LED devices. Say Y if you
+ want to use flash specific features of a LED device, if they
+ are supported.
+
 comment "LED drivers"
 
 config LEDS_88PM860X
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 3cd76db..8861b86 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -2,6 +2,7 @@
 # LED Core
 obj-$(CONFIG_NEW_LEDS) += led-core.o
 obj-$(CONFIG_LEDS_CLASS)   += led-class.o
+obj-$(CONFIG_LEDS_CLASS_FLASH) += led-flash.o
 obj-$(CONFIG_LEDS_TRIGGERS)+= led-triggers.o
 
 # LED Platform Drivers
diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index f37d63c..58f16c3 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -9,15 +9,16 @@
  * published by the Free Software Foundation.
  */
 
-#include 
-#include 
+#include 
+#include 
+#include 
 #include 
+#include 
 #include 
+#include 
+#include 
 #include 
-#include 
 #include 
-#include 
-#include 
 #include 
 #include "leds.h"
 
@@ -45,28 +46,38 @@ static ssize_t brightness_store(struct device *dev,
 {
struct led_classdev *led_cdev = dev_get_drvdata(dev);
unsigned long state;
-   ssize_t ret = -EINVAL;
+   ssize_t ret;
+
+   mutex_lock(&led_cdev->led_lock);
+
+   if (led_sysfs_is_locked(led_cdev)) {
+   ret = -EBUSY;
+   goto unlock;
+   }
 
ret = kstrtoul(buf, 10, &state);
if (ret)
-   return ret;
+   goto unlock;
 
if (state == LED_OFF)
led_trigger_remove(led_cdev);
__led_set_brightness(led_cdev, state);
+   ret = size;
 
-   return size;
+unlock:
+   mutex_unlock(&led_cdev->led_lock);
+   return ret;
 }
 static DEVICE_ATTR_RW(brightness);
 
-static ssize_t led_max_brightness_show(struct device *dev,
+static ssize_t max_brightness_show(struct device *dev,
struct device_attribute *attr, char *buf)
 {
struct led_classdev *led_cdev = dev_get_drvdata(dev);
 
return sprintf(buf, "%u\n", led_cdev->max_brightness);
 }
-static DEVICE_ATTR(max_brightness, 0444, led_max_brightness_show, NULL);
+static DEVICE_ATTR_RO(max_brightness);
 
 #ifdef CONFIG_LEDS_TRIGGERS
 static DEVICE_ATTR(trigger, 0644, led_trigger_show, led_trigger_store);
@@ -174,6 +185,8 @@ EXPORT_SYMBOL_GPL(led_classdev_suspend);
 void led_classdev_resume(struct led_classdev *led_cdev)
 {
led_cdev->brightness_set(led_cdev, led_cdev->brightness);
+   if (led_cdev->flash_resume)
+   led_cdev->flash_resume(led_cdev);
led_cdev->flags &= ~LED_SUSPENDED;
 }
 EXPORT_SYMBOL_GPL(led_classdev_resume);
@@ -218,6 +231,7 @@ int led_classdev_register(struct device *parent, struct 
led_classdev *led_cdev)
 #ifdef CONFIG_LEDS_TRIGGERS
init_rwsem(&led_cdev->trigger_lock);
 #endif
+   mutex_init(&led_cdev->led_lock);
/* add to the list of leds */
down_write(&leds_list_lock);
list_add_tail(&led_cdev->node, &leds_list);
@@ -271,6 +285,8 @@ void led_clas

[PATCH/RFC v3 5/5] media: Add registration helpers for V4L2 flash sub-devices

2014-04-11 Thread Jacek Anaszewski
This patch adds helper functions for registering/unregistering
LED class flash devices as V4L2 subdevs. The functions should
be called from the LED subsystem device driver. In case the
support for V4L2 Flash sub-devices is disabled in the kernel
config the functions' empty versions will be used.

Signed-off-by: Jacek Anaszewski 
Acked-by: Kyungmin Park 
---
 drivers/media/v4l2-core/Kconfig  |   10 +
 drivers/media/v4l2-core/Makefile |2 +
 drivers/media/v4l2-core/v4l2-flash.c |  393 ++
 include/media/v4l2-flash.h   |  119 ++
 4 files changed, 524 insertions(+)
 create mode 100644 drivers/media/v4l2-core/v4l2-flash.c
 create mode 100644 include/media/v4l2-flash.h

diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig
index 2189bfb..1f8514d 100644
--- a/drivers/media/v4l2-core/Kconfig
+++ b/drivers/media/v4l2-core/Kconfig
@@ -35,6 +35,16 @@ config V4L2_MEM2MEM_DEV
 tristate
 depends on VIDEOBUF2_CORE
 
+# Used by LED subsystem flash drivers
+config V4L2_FLASH
+   bool "Enable support for Flash sub-devices"
+   depends on LEDS_CLASS_FLASH
+   ---help---
+ Say Y here to enable support for Flash sub-devices, which allow
+ to control LED class devices with use of V4L2 Flash controls.
+
+ When in doubt, say N.
+
 # Used by drivers that need Videobuf modules
 config VIDEOBUF_GEN
tristate
diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
index c6ae7ba..8e37ab4 100644
--- a/drivers/media/v4l2-core/Makefile
+++ b/drivers/media/v4l2-core/Makefile
@@ -22,6 +22,8 @@ obj-$(CONFIG_VIDEO_TUNER) += tuner.o
 
 obj-$(CONFIG_V4L2_MEM2MEM_DEV) += v4l2-mem2mem.o
 
+obj-$(CONFIG_V4L2_FLASH) += v4l2-flash.o
+
 obj-$(CONFIG_VIDEOBUF_GEN) += videobuf-core.o
 obj-$(CONFIG_VIDEOBUF_DMA_SG) += videobuf-dma-sg.o
 obj-$(CONFIG_VIDEOBUF_DMA_CONTIG) += videobuf-dma-contig.o
diff --git a/drivers/media/v4l2-core/v4l2-flash.c 
b/drivers/media/v4l2-core/v4l2-flash.c
new file mode 100644
index 000..f1be332
--- /dev/null
+++ b/drivers/media/v4l2-core/v4l2-flash.c
@@ -0,0 +1,393 @@
+/*
+ * V4L2 Flash LED sub-device registration helpers.
+ *
+ * Copyright (C) 2014 Samsung Electronics Co., Ltd
+ * Author: Jacek Anaszewski 
+ *
+ * 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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+static inline enum led_brightness v4l2_flash_intensity_to_led_brightness(
+   struct led_ctrl *config,
+   u32 intensity)
+{
+   return intensity / config->step;
+}
+
+static inline u32 v4l2_flash_led_brightness_to_intensity(
+   struct led_ctrl *config,
+   enum led_brightness brightness)
+{
+   return brightness * config->step;
+}
+
+static int v4l2_flash_g_volatile_ctrl(struct v4l2_ctrl *c)
+
+{
+   struct v4l2_flash *v4l2_flash = v4l2_ctrl_to_v4l2_flash(c);
+   struct led_classdev *led_cdev = v4l2_flash->led_cdev;
+   struct led_flash *flash = led_cdev->flash;
+   struct v4l2_flash_ctrl *ctrl = &v4l2_flash->ctrl;
+   u32 fault;
+   int ret;
+
+   switch (c->id) {
+   case V4L2_CID_FLASH_TORCH_INTENSITY:
+   if (ctrl->led_mode->val == V4L2_FLASH_LED_MODE_TORCH) {
+   ret = v4l2_call_flash_op(brightness_update, led_cdev);
+   if (ret < 0)
+   return ret;
+   ctrl->torch_intensity->val =
+   v4l2_flash_led_brightness_to_intensity(
+   &led_cdev->brightness_ctrl,
+   led_cdev->brightness);
+   }
+   return 0;
+   case V4L2_CID_FLASH_INTENSITY:
+   ret = v4l2_call_flash_op(flash_brightness_update, led_cdev);
+   if (ret < 0)
+   return ret;
+   /* no conversion is needed */
+   c->val = flash->brightness.val;
+   return 0;
+   case V4L2_CID_FLASH_INDICATOR_INTENSITY:
+   ret = v4l2_call_flash_op(indicator_brightness_update, led_cdev);
+   if (ret < 0)
+   return ret;
+   /* no conversion is needed */
+   c->val = flash->indicator_brightness->val;
+   return 0;
+   case V4L2_CID_FLASH_STROBE_STATUS:
+   ret = v4l2_call_flash_op(strobe_get, led_cdev);
+   if (ret < 0)
+   return ret;
+   c->val = !!ret;
+   return 0;
+   case V4L2_CID_FLASH_FAULT:
+   /* led faults map directly to V4L2 flash faults */
+   

Re: [REVIEWv2 PATCH 02/13] vb2: fix handling of data_offset and v4l2_plane.reserved[]

2014-04-11 Thread Hans Verkuil
On 04/11/2014 03:48 PM, Tomasz Stanislawski wrote:
> On 04/11/2014 03:03 PM, Hans Verkuil wrote:
>> On 04/11/2014 02:48 PM, Tomasz Stanislawski wrote:
>>> On 04/07/2014 03:11 PM, Hans Verkuil wrote:
 From: Hans Verkuil 

 The videobuf2-core did not zero the 'planes' array in __qbuf_userptr()
 and __qbuf_dmabuf(). That's now memset to 0. Without this the reserved
 array in struct v4l2_plane would be non-zero, causing v4l2-compliance
 errors.

 More serious is the fact that data_offset was not handled correctly:

 - for capture devices it was never zeroed, which meant that it was
   uninitialized. Unless the driver sets it it was a completely random
   number. With the memset above this is now fixed.

 - __qbuf_dmabuf had a completely incorrect length check that included
   data_offset.
>>>
>>> Hi Hans,
>>>
>>> I may understand it wrongly but IMO allowing non-zero data offset
>>> simplifies buffer sharing using dmabuf.
>>> I remember a problem that occurred when someone wanted to use
>>> a single dmabuf with multiplanar API.
>>>
>>> For example, MFC shares a buffer with DRM. Assume that DRM device
>>> forces the whole image to be located in one dmabuf.
>>>
>>> The MFC uses multiplanar API therefore application must use
>>> the same dmabuf to describe luma and chroma planes.
>>>
>>> It is intuitive to use the same dmabuf for both planes and
>>> data_offset=0 for luma plane and data_offset = luma_size
>>> for chroma offset.
>>>
>>> The check:
>>>
 -  if (planes[plane].length < planes[plane].data_offset +
 -  q->plane_sizes[plane]) {
>>>
>>> assured that the logical plane does not overflow the dmabuf.
>>>
>>> Am I wrong?
>>
>> Yes :-)
>>
>> For video capture the data_offset field is set by the *driver*, not the
>> application. In practice data_offset is the size of a header that is in
>> front of the actual image.
>>
>> You cannot use data_offset for the purpose you describe. To do that a new
>> offset field would have to be added (user_offset?). I'm not opposed to
>> that, I think it is a valid use-case for both dmabuf and userptr and
>> even mmap in combination with CREATE_BUFS.
> 
> Ok. What do you think about allowing the user to set this field?
> The driver would be allowed to adjust it.
> This is a slight change of semantics. As long as application
> was forced to treat data_offset as a reserved field (I mean zeroing)
> then this change would be backward compatible.

It's not quite that easy. Suppose the driver adds a 1024 bytes header
before the image (so data_offset is 1024) and the application sets
data_offset to 512. What does that mean? That the driver starts capturing
at start_of_buffer + 512 and then returns 1024+512 as data_offset? Or that
the driver replaces it by 1024 anyway?

Besides, there has never been a requirement that apps set it to 0 for
video capture. They only have to set it for video output. So this doesn't
work anyway.

> 
> Otherwise, a new field could be introduces as you mentioned.
> The name buffer_offset or simply offset might be more
> appropriate than user_offset.
> 
> I can prepare some RFC about this extension.

I would go with a new field.

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] [media] Prefer gspca_sonixb over sn9c102 for all devices

2014-04-11 Thread Greg Kroah-Hartman
On Fri, Apr 11, 2014 at 09:15:32AM +0200, Jean Delvare wrote:
> The sn9c102 driver is deprecated. It was moved to staging in
> anticipation of its removal in a future kernel version. However, USB
> devices 0C45:6024 and 0C45:6025 are still handled by sn9c102 when
> both sn9c102 and gspca_sonixb are enabled.
> 
> We must migrate all the users of these devices to the gspca_sonixb
> driver now, so that it gets sufficient testing before the sn9c102
> driver is finally phased out.
> 
> Signed-off-by: Jean Delvare 
> Cc: Hans de Goede 
> Cc: Mauro Carvalho Chehab 
> Cc: Luca Risolia 
> Cc: Greg Kroah-Hartman 
> ---
> I consider this a bug fix, I believe it should go upstream ASAP.
> 

Acked-by: Greg Kroah-Hartman 
--
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: [REVIEWv2 PATCH 02/13] vb2: fix handling of data_offset and v4l2_plane.reserved[]

2014-04-11 Thread Tomasz Stanislawski
On 04/11/2014 03:03 PM, Hans Verkuil wrote:
> On 04/11/2014 02:48 PM, Tomasz Stanislawski wrote:
>> On 04/07/2014 03:11 PM, Hans Verkuil wrote:
>>> From: Hans Verkuil 
>>>
>>> The videobuf2-core did not zero the 'planes' array in __qbuf_userptr()
>>> and __qbuf_dmabuf(). That's now memset to 0. Without this the reserved
>>> array in struct v4l2_plane would be non-zero, causing v4l2-compliance
>>> errors.
>>>
>>> More serious is the fact that data_offset was not handled correctly:
>>>
>>> - for capture devices it was never zeroed, which meant that it was
>>>   uninitialized. Unless the driver sets it it was a completely random
>>>   number. With the memset above this is now fixed.
>>>
>>> - __qbuf_dmabuf had a completely incorrect length check that included
>>>   data_offset.
>>
>> Hi Hans,
>>
>> I may understand it wrongly but IMO allowing non-zero data offset
>> simplifies buffer sharing using dmabuf.
>> I remember a problem that occurred when someone wanted to use
>> a single dmabuf with multiplanar API.
>>
>> For example, MFC shares a buffer with DRM. Assume that DRM device
>> forces the whole image to be located in one dmabuf.
>>
>> The MFC uses multiplanar API therefore application must use
>> the same dmabuf to describe luma and chroma planes.
>>
>> It is intuitive to use the same dmabuf for both planes and
>> data_offset=0 for luma plane and data_offset = luma_size
>> for chroma offset.
>>
>> The check:
>>
>>> -   if (planes[plane].length < planes[plane].data_offset +
>>> -   q->plane_sizes[plane]) {
>>
>> assured that the logical plane does not overflow the dmabuf.
>>
>> Am I wrong?
> 
> Yes :-)
> 
> For video capture the data_offset field is set by the *driver*, not the
> application. In practice data_offset is the size of a header that is in
> front of the actual image.
> 
> You cannot use data_offset for the purpose you describe. To do that a new
> offset field would have to be added (user_offset?). I'm not opposed to
> that, I think it is a valid use-case for both dmabuf and userptr and
> even mmap in combination with CREATE_BUFS.

Ok. What do you think about allowing the user to set this field?
The driver would be allowed to adjust it.
This is a slight change of semantics. As long as application
was forced to treat data_offset as a reserved field (I mean zeroing)
then this change would be backward compatible.

Otherwise, a new field could be introduces as you mentioned.
The name buffer_offset or simply offset might be more
appropriate than user_offset.

I can prepare some RFC about this extension.

Regards,
Tomasz Stanislawski

> 
> 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: [REVIEWv3 PATCH 09/13] vb2: add vb2_fileio_is_active and check it more often

2014-04-11 Thread Hans Verkuil
On 04/11/2014 03:05 PM, Tomasz Stanislawski wrote:
> Hi Hans,
> 
> On 04/11/2014 10:11 AM, Hans Verkuil wrote:
>> From: Hans Verkuil 
>>
>> Added a vb2_fileio_is_active inline function that returns true if fileio
>> is in progress. Check for this too in mmap() (you don't want apps mmap()ing
>> buffers used by fileio) and expbuf() (same reason).
> 
> Why? I expect that there is no sane use case for using
> mmap() and expbuf in read/write mode but why forbidding this.
> 
> Could you provide a reason?

The buffer management is completely internal to vb2 for read()/write().
I think that allowing expbuf/mmap is just plain weird. I don't think
it would do any harm other than increasing the memory refcount, but
I very much prefer to block this.

The only ioctl allowed is querybuf, and that primarily for debugging.
Frankly, I wouldn't mind if that is blocked off as well but since it
is guaranteed to have no side-effects and it actually has a use-case
(debugging) I've left that in.

Personally I think the question is not: "why block this?", it is:
"why would you allow it?".

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: [yavta PATCH 7/9] Print timestamp type and source for dequeued buffers

2014-04-11 Thread Laurent Pinchart
Hi Sakari,

On Friday 11 April 2014 01:36:03 Sakari Ailus wrote:
> Laurent Pinchart wrote:
> > On Thursday 10 April 2014 21:58:41 Sakari Ailus wrote:
> >> Laurent Pinchart wrote:
> >>> Hi Sakari,
> >>> 
> >>> Thank you for the patch.
> >>> 
> >>> Given that the timestamp type and source are not supposed to change
> >>> during streaming, do we really need to print them for every frame ?
> >> 
> >> When processing frames from memory to memory (COPY timestamp type), the
> >> it is entirely possible that the timestamp source changes as the flags
> >> are copied from the OUTPUT buffer to the CAPTURE buffer.
> > 
> > It's possible, but is it allowed by the V4L2 API ?
> 
> The spec states that:
> 
> "The V4L2_BUF_FLAG_TIMESTAMP_COPY timestamp type which is used by e.g. on
> mem-to-mem devices is an exception to the rule: the timestamp source flags
> are copied from the OUTPUT video buffer to the CAPTURE video buffer."
> 
> >> These patches do not support it but it is allowed.
> >> 
> >> One option would be to print the source on every frame only when the
> >> type is COPY. For a program like yavta this might be overly
> >> sophisticated IMO. :-)
> > 
> > My concern is that this makes the lines output by yavta pretty long.
> 
> True as well. I could remove "type/src " from the timestamp source
> information. That's mostly redundant anyway. Then we shouldn't exceed 80
> characters per line that easily anymore.

I think that would be better.
 
> Could this be the time to add a "verbose" option? :-)

Possibly, but then we'll need to discuss what information should be printed in 
verbose mode only :-)

-- 
Regards,

Laurent Pinchart

--
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: [REVIEWv3 PATCH 09/13] vb2: add vb2_fileio_is_active and check it more often

2014-04-11 Thread Tomasz Stanislawski
Hi Hans,

On 04/11/2014 10:11 AM, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> Added a vb2_fileio_is_active inline function that returns true if fileio
> is in progress. Check for this too in mmap() (you don't want apps mmap()ing
> buffers used by fileio) and expbuf() (same reason).

Why? I expect that there is no sane use case for using
mmap() and expbuf in read/write mode but why forbidding this.

Could you provide a reason?

Regard,
Tomasz Stanislawski

> 
> In addition drivers should be able to check for this in queue_setup() to
> return an error if an attempt is made to read() or write() with
> V4L2_FIELD_ALTERNATE being configured. This is illegal (there is no way
> to pass the TOP/BOTTOM information around using file I/O).
> 
> However, in order to be able to check for this the init_fileio function
> needs to set q->fileio early on, before the buffers are allocated. So switch
> to using internal functions (__reqbufs, vb2_internal_qbuf and
> vb2_internal_streamon) to skip the fileio check. Well, that's why the internal
> functions were created...
> 
> Signed-off-by: Hans Verkuil 
> Acked-by: Pawel Osciak 
> Acked-by: Sakari Ailus 

--
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: [REVIEWv2 PATCH 02/13] vb2: fix handling of data_offset and v4l2_plane.reserved[]

2014-04-11 Thread Hans Verkuil
On 04/11/2014 02:48 PM, Tomasz Stanislawski wrote:
> On 04/07/2014 03:11 PM, Hans Verkuil wrote:
>> From: Hans Verkuil 
>>
>> The videobuf2-core did not zero the 'planes' array in __qbuf_userptr()
>> and __qbuf_dmabuf(). That's now memset to 0. Without this the reserved
>> array in struct v4l2_plane would be non-zero, causing v4l2-compliance
>> errors.
>>
>> More serious is the fact that data_offset was not handled correctly:
>>
>> - for capture devices it was never zeroed, which meant that it was
>>   uninitialized. Unless the driver sets it it was a completely random
>>   number. With the memset above this is now fixed.
>>
>> - __qbuf_dmabuf had a completely incorrect length check that included
>>   data_offset.
> 
> Hi Hans,
> 
> I may understand it wrongly but IMO allowing non-zero data offset
> simplifies buffer sharing using dmabuf.
> I remember a problem that occurred when someone wanted to use
> a single dmabuf with multiplanar API.
> 
> For example, MFC shares a buffer with DRM. Assume that DRM device
> forces the whole image to be located in one dmabuf.
> 
> The MFC uses multiplanar API therefore application must use
> the same dmabuf to describe luma and chroma planes.
> 
> It is intuitive to use the same dmabuf for both planes and
> data_offset=0 for luma plane and data_offset = luma_size
> for chroma offset.
> 
> The check:
> 
>> -if (planes[plane].length < planes[plane].data_offset +
>> -q->plane_sizes[plane]) {
> 
> assured that the logical plane does not overflow the dmabuf.
> 
> Am I wrong?

Yes :-)

For video capture the data_offset field is set by the *driver*, not the
application. In practice data_offset is the size of a header that is in
front of the actual image.

You cannot use data_offset for the purpose you describe. To do that a new
offset field would have to be added (user_offset?). I'm not opposed to
that, I think it is a valid use-case for both dmabuf and userptr and
even mmap in combination with CREATE_BUFS.

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] coda: update CODA7541 to firmware 1.4.50

2014-04-11 Thread Fabio Estevam
Hi Dan/Philipp,

On Fri, Apr 11, 2014 at 8:08 AM, Dan Carpenter  wrote:
> What ever happened with this?
>
> regards,
> dan carpenter
>
> On Wed, Nov 06, 2013 at 07:13:43PM +0300, Dan Carpenter wrote:
>> Hello Philipp Zabel,
>>
>> This is a semi-automatic email about new static checker warnings.
>>
>> The patch 5677e3b04d3b: "[media] coda: update CODA7541 to firmware
>> 1.4.50" from Jun 21, 2013, leads to the following Smatch complaint:
>>
>> drivers/media/platform/coda.c:1530 coda_alloc_framebuffers()
>>error: we previously assumed 'ctx->codec' could be null (see line 
>> 1521)
>>
>> drivers/media/platform/coda.c
>>   1520
>>   1521if (ctx->codec && ctx->codec->src_fourcc == 
>> V4L2_PIX_FMT_H264)
>> ^^
>> Patch introduces a new NULL check.
>>
>>   1522height = round_up(height, 16);
>>   1523ysize = round_up(q_data->width, 8) * height;
>>   1524
>>   1525/* Allocate frame buffers */
>>   1526for (i = 0; i < ctx->num_internal_frames; i++) {
>>   1527size_t size;
>>   1528
>>   1529size = q_data->sizeimage;
>>   1530if (ctx->codec->src_fourcc == 
>> V4L2_PIX_FMT_H264 &&
>> ^^
>> Patch introduces a new unchecked dereference.
>>
>>   1531dev->devtype->product != CODA_DX6)
>>   1532ctx->internal_frames[i].size += 
>> ysize/4;

Would the fix below address this issue?

--- a/drivers/media/platform/coda.c
+++ b/drivers/media/platform/coda.c
@@ -1518,7 +1518,10 @@ static int coda_alloc_framebuffers(struct coda_ctx *ctx,
int ret;
int i;

-   if (ctx->codec && ctx->codec->src_fourcc == V4L2_PIX_FMT_H264)
+   if (!ctx->codec)
+   return -EINVAL;
+
+   if (ctx->codec->src_fourcc == V4L2_PIX_FMT_H264)
height = round_up(height, 16);
ysize = round_up(q_data->width, 8) * height;
--
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: [REVIEWv2 PATCH 02/13] vb2: fix handling of data_offset and v4l2_plane.reserved[]

2014-04-11 Thread Tomasz Stanislawski
On 04/07/2014 03:11 PM, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> The videobuf2-core did not zero the 'planes' array in __qbuf_userptr()
> and __qbuf_dmabuf(). That's now memset to 0. Without this the reserved
> array in struct v4l2_plane would be non-zero, causing v4l2-compliance
> errors.
> 
> More serious is the fact that data_offset was not handled correctly:
> 
> - for capture devices it was never zeroed, which meant that it was
>   uninitialized. Unless the driver sets it it was a completely random
>   number. With the memset above this is now fixed.
> 
> - __qbuf_dmabuf had a completely incorrect length check that included
>   data_offset.

Hi Hans,

I may understand it wrongly but IMO allowing non-zero data offset
simplifies buffer sharing using dmabuf.
I remember a problem that occurred when someone wanted to use
a single dmabuf with multiplanar API.

For example, MFC shares a buffer with DRM. Assume that DRM device
forces the whole image to be located in one dmabuf.

The MFC uses multiplanar API therefore application must use
the same dmabuf to describe luma and chroma planes.

It is intuitive to use the same dmabuf for both planes and
data_offset=0 for luma plane and data_offset = luma_size
for chroma offset.

The check:

> - if (planes[plane].length < planes[plane].data_offset +
> - q->plane_sizes[plane]) {

assured that the logical plane does not overflow the dmabuf.

Am I wrong?

Regards,
Tomasz Stanislawski

> 
> - in __fill_vb2_buffer in the DMABUF case the data_offset field was
>   unconditionally copied from v4l2_buffer to v4l2_plane when this
>   should only happen in the output case.
> 
> - in the single-planar case data_offset was never correctly set to 0.
>   The single-planar API doesn't support data_offset, so setting it
>   to 0 is the right thing to do. This too is now solved by the memset.
> 
> All these issues were found with v4l2-compliance.
> 
> Signed-off-by: Hans Verkuil 

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


[no subject]

2014-04-11 Thread Brinda Slauther


ARE YOU IN NEED OF A LOAN? WE OFFER ALL KINDS OF LOAN AT 2% ANNUAL INTEREST 
RATE, CONTACT US VIA EMAIL

--
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: Hauppauge ImpactVCB-e 01385

2014-04-11 Thread Steve Cookson - IT

Hi Hans,

On 11/04/14 11:35, Hans Verkuil wrote:

Actually, I would recommend that you try playing with the 'card=' option to
see if you can get something that works. I suspect that adding support for
this card isn't hard.
Well I did try the card= option before and it worked in a way.  The 
raster was compressed though and the image was not really acceptable.

Alternatively, you can wait. I've ordered this card myself (it's about time I
start cleaning up the cx23885 driver, so some extra hardware to test with won't
hurt) so once I have it I'll work on adding support for it to the cx23885
driver. The problem is that it has to be ordered, so it may take some time
before it arrives.

I'm happy to wait.

Is there something I can do in the mean time?  If I can download some 
code and start playing, I would do so if I knew where to look.


Tell me where to start and I will do.

Regards

Steve

PS  In fact I think I have 2 x ImpactVCB-e's, so if your card is very 
delayed, I could send you one.  If you're in the Netherlands it won't 
take long to get to you.

--
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 0/2] Add support for sii9234 chip

2014-04-11 Thread Tomasz Stanislawski
Hi everyone,
This patchset adds support for sii9234 HD Mobile Link Bridge.  The chip is used
to convert HDMI signal into MHL.  The driver enables HDMI output on Trats and
Trats2 boards.

The code is based on the driver [1] developed by:
   Adam Hampson 
   Erik Gilling 
with additional contributions from:
   Shankar Bandal 
   Dharam Kumar 

The drivers architecture was greatly simplified and transformed into a form
accepted (hopefully) by opensource community.  The main differences from
original code are:
* using single I2C client instead of 4 subclients
* remove all logic non-related to establishing HDMI link
* simplify error handling
* rewrite state machine in interrupt handler
* wakeup and discovery triggered by an extcon event
* integrate with Device Tree

For now, the driver is added to drivers/misc/ directory because it has neigher
userspace nor kernel interface.  The chip is capable of receiving and
processing CEC events, so the driver may export an input device in /dev/ in the
future.  However CEC could be also handled by HDMI driver.

I kindly ask for suggestions about the best location for this driver.

Both the chip and the driver are quite autonomic. The chip works as 'invisible
proxy' for HDMI signal, so there is no need to integrate it with HDMI drivers
by helper-subsystems like v4l2_subdev or drm_bridge.  If the driver is merged
then the driver from drivers/media/platform/s5p-tv/sii9234_drv.c could be safely
removed.

All comments are welcome.

Regards,
Tomasz Stanislawski


References:
[1] 
https://github.com/junpei0824/SC02C-linux/tree/master-jelly-cm-aokp/drivers/media/video/mhl


Tomasz Stanislawski (2):
  misc: add sii9234 driver
  arm: dts: trats2: add SiI9234 node

 Documentation/devicetree/bindings/sii9234.txt |   22 +
 arch/arm/boot/dts/exynos4412-trats2.dts   |   43 +
 drivers/misc/Kconfig  |8 +
 drivers/misc/Makefile |1 +
 drivers/misc/sii9234.c| 1081 +
 5 files changed, 1155 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sii9234.txt
 create mode 100644 drivers/misc/sii9234.c

-- 
1.7.9.5

--
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] arm: dts: trats2: add SiI9234 node

2014-04-11 Thread Tomasz Stanislawski
This patch adds configuration of SiI9234 bridge to Trats2 board.

Signed-off-by: Tomasz Stanislawski 
---
 arch/arm/boot/dts/exynos4412-trats2.dts |   43 +++
 1 file changed, 43 insertions(+)

diff --git a/arch/arm/boot/dts/exynos4412-trats2.dts 
b/arch/arm/boot/dts/exynos4412-trats2.dts
index 9583563d..65fd1d4 100644
--- a/arch/arm/boot/dts/exynos4412-trats2.dts
+++ b/arch/arm/boot/dts/exynos4412-trats2.dts
@@ -680,4 +680,47 @@
pulldown-ohm = <10>; /* 100K */
io-channels = <&adc 2>;  /* Battery temperature */
};
+
+   vsil: voltage-regulator-vsil {
+   compatible = "regulator-fixed";
+   regulator-name = "HDMI_5V";
+   regulator-min-microvolt = <500>;
+   regulator-max-microvolt = <500>;
+   gpio = <&gpl0 4 0>;
+   enable-active-high;
+   vin-supply = <&buck7_reg>;
+   };
+
+   i2c-mhl {
+   compatible = "i2c-gpio";
+   gpios = <&gpf0 4 0 &gpf0 6 0>;
+   i2c-gpio,delay-us = <100>;
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   pinctrl-0 = <&i2c_mhl_bus>;
+   pinctrl-names = "default";
+   status = "okay";
+
+   sii9234: sii9234@39 {
+   compatible = "sil,sii9234";
+   vcc-supply = <&vsil>;
+   gpio-reset = <&gpf3 4 0>;
+   gpio-int = <&gpf3 5 0>;
+   reg = <0x39>;
+   };
+   };
+};
+
+&pinctrl_0 {
+   mhl_int: mhl-int {
+   samsung,pins = "gpf3-5";
+   samsung,pin-pud = <0>;
+   };
+   i2c_mhl_bus: i2c-mhl-bus {
+   samsung,pins = "gpf0-4", "gpf0-6";
+   samsung,pin-function = <2>;
+   samsung,pin-pud = <1>;
+   samsung,pin-drv = <0>;
+   };
 };
-- 
1.7.9.5

--
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] misc: add sii9234 driver

2014-04-11 Thread Tomasz Stanislawski
Add driver for HDMI bridge using MHL connection.
Contains refactored code for MHL driver developed by:

Adam Hampson 
Erik Gilling 
Shankar Bandal 
Dharam Kumar 

Signed-off-by: Tomasz Stanislawski 
Signed-off-by: Kyungmin Park 
---
 Documentation/devicetree/bindings/sii9234.txt |   22 +
 drivers/misc/Kconfig  |8 +
 drivers/misc/Makefile |1 +
 drivers/misc/sii9234.c| 1081 +
 4 files changed, 1112 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sii9234.txt
 create mode 100644 drivers/misc/sii9234.c

diff --git a/Documentation/devicetree/bindings/sii9234.txt 
b/Documentation/devicetree/bindings/sii9234.txt
new file mode 100644
index 000..4431139
--- /dev/null
+++ b/Documentation/devicetree/bindings/sii9234.txt
@@ -0,0 +1,22 @@
+SiI9234 Mobile HD Link Transmitter
+
+Required properties:
+- compatible : "sil,sii9234".
+- reg : I2C address for TPI interface, use 0x39
+- vcc-supply : regulator that supplies the chip
+- gpio-reset : GPIO connected to RESET_N pin
+- gpio-int : GPIO connected to INT pin
+
+Optional properties:
+- extcon : phandle to extcon that wakes up the chip
+
+Additional compatible properties are also allowed.
+
+Example:
+   sii9234@39 {
+   compatible = "sil,sii9234";
+   reg = <0x39>;
+   vcc-supply = <&vsil>;
+   gpio-reset = <&gpf3 4 0>;
+   gpio-int = <&gpf3 5 0>;
+   };
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 1cb7408..3b7f266 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -515,6 +515,14 @@ config SRAM
  the genalloc API. It is supposed to be used for small on-chip SRAM
  areas found on many SoCs.
 
+config SII9234
+   tristate "Silicon Image SII9234 Driver"
+   depends on I2C
+   help
+ Say Y here if you want support for the MHL interface.
+ It is an I2C driver, that detects connection of MHL bridge
+ and starts encapsulation of HDMI signal.
+
 source "drivers/misc/c2port/Kconfig"
 source "drivers/misc/eeprom/Kconfig"
 source "drivers/misc/cb710/Kconfig"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 7eb4b69..145a47a 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -55,3 +55,4 @@ obj-$(CONFIG_SRAM)+= sram.o
 obj-y  += mic/
 obj-$(CONFIG_GENWQE)   += genwqe/
 obj-$(CONFIG_ECHO) += echo/
+obj-$(CONFIG_SII9234)  += sii9234.o
diff --git a/drivers/misc/sii9234.c b/drivers/misc/sii9234.c
new file mode 100644
index 000..60b7c66
--- /dev/null
+++ b/drivers/misc/sii9234.c
@@ -0,0 +1,1081 @@
+/*
+ * Copyright (C) 2014 Samsung Electronics
+ *
+ * Author: Tomasz Stanislawski 
+ *
+ * Based on sii9234 driver created by:
+ *Adam Hampson 
+ *Erik Gilling 
+ *Shankar Bandal 
+ *Dharam Kumar 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* MHL Tx registers and bits */
+#define MHL_TX_SRST 0x05
+#define MHL_TX_SYSSTAT_REG  0x09
+#define RSEN_STATUS (1<<2)
+#define MHL_TX_INTR1_REG0x71
+#define HPD_CHANGE_INT  (1<<6)
+#define RSEN_CHANGE_INT (1<<5)
+#define MHL_TX_INTR4_REG0x74
+#define RGND_READY_INT  (1<<6)
+#define VBUS_LOW_INT(1<<5)
+#define CBUS_LKOUT_INT  (1<<4)
+#define MHL_DISC_FAIL_INT   (1<<3)
+#define MHL_EST_INT (1<<2)
+#define MHL_TX_INTR1_ENABLE_REG 0x75
+#define HPD_CHANGE_INT_MASK (1<<6)
+#define RSEN_CHANGE_INT_MASK(1<<5)
+#define MHL_TX_INTR4_ENABLE_REG 0x78
+#define RGND_READY_MASK (1<<6)
+#define CBUS_LKOUT_MASK (1<<4)
+#define MHL_DISC_FAIL_MASK  (1<<3)
+#define MHL_EST_MASK(1<<2)
+#define MHL_TX_INT_CTRL_REG 0x79
+#define MHL_TX_TMDS_CCTRL   0x80
+#define MHL_TX_DISC_CTRL1_REG   0x90
+#define MHL_TX_DISC_CTRL2_REG   0x91
+#define SKIP_GND

Re: [media] coda: update CODA7541 to firmware 1.4.50

2014-04-11 Thread Dan Carpenter
What ever happened with this?

regards,
dan carpenter

On Wed, Nov 06, 2013 at 07:13:43PM +0300, Dan Carpenter wrote:
> Hello Philipp Zabel,
> 
> This is a semi-automatic email about new static checker warnings.
> 
> The patch 5677e3b04d3b: "[media] coda: update CODA7541 to firmware 
> 1.4.50" from Jun 21, 2013, leads to the following Smatch complaint:
> 
> drivers/media/platform/coda.c:1530 coda_alloc_framebuffers()
>error: we previously assumed 'ctx->codec' could be null (see line 1521)
> 
> drivers/media/platform/coda.c
>   1520
>   1521if (ctx->codec && ctx->codec->src_fourcc == 
> V4L2_PIX_FMT_H264)
> ^^
> Patch introduces a new NULL check.
> 
>   1522height = round_up(height, 16);
>   1523ysize = round_up(q_data->width, 8) * height;
>   1524
>   1525/* Allocate frame buffers */
>   1526for (i = 0; i < ctx->num_internal_frames; i++) {
>   1527size_t size;
>   1528
>   1529size = q_data->sizeimage;
>   1530if (ctx->codec->src_fourcc == V4L2_PIX_FMT_H264 
> &&
> ^^
> Patch introduces a new unchecked dereference.
> 
>   1531dev->devtype->product != CODA_DX6)
>   1532ctx->internal_frames[i].size += ysize/4;
> 
> regards,
> dan carpenter
--
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


SUSPECT: General web-mail maintenance

2014-04-11 Thread Semple, Kylie M
General web-mail maintenance
Dear Account Owner,
 THIS MESSAGE IS DIRECTED TO THOSE USING Office Outlook Web Access Web-mail 
(2007 version). We want to upgrade all Microsoft Exchange email account 
scheduled for today as part of our duty to strengthen security of your mailbox. 
CLICK HERE to upgrade your account to Outlook Web Apps 
2014. If your settings is not updated today, your account will be inactive and 
cannot send or receive message any longer.
Sincerely,
IT Helpdesk DEPT
--
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: Hauppauge ImpactVCB-e 01385

2014-04-11 Thread Hans Verkuil
On 04/11/2014 12:03 PM, Steve Cookson - IT wrote:
> Hi Hans,
> 
> Thanks for this, I'll do as you suggested.
> 
> On 11/04/14 10:45, Hans Verkuil wrote:
>> I have serious doubts whether this is actually supported. I see no mention of
>> that board in the cx23885 driver. I wonder if there is a mixup between the
>> ImpactVCB (which IS supported) and the ImpactVCB-e.
> Do you think there is another internal card out there that would do the 
> job of capturing raw SD video?
> 
> What would you recommend.

Actually, I would recommend that you try playing with the 'card=' option to
see if you can get something that works. I suspect that adding support for
this card isn't hard.

Alternatively, you can wait. I've ordered this card myself (it's about time I
start cleaning up the cx23885 driver, so some extra hardware to test with won't
hurt) so once I have it I'll work on adding support for it to the cx23885
driver. The problem is that it has to be ordered, so it may take some time
before it arrives.

There aren't all that many alternatives available for PCIe (PCI is easier). You
can look at the list of supported cards in cx23885-cards.c and see if you can
get any of those other cards.

Regards,

Hans

> 
> I'm currently using the Dazzle DVC100, stripping the case off and 
> installing it internally with cable ties.   It's just a bit basic. I'd 
> like something a bit more professional like a half-height PCIe board.  
> Which is why I selected the ImpactVCB-e, but really I have no attachment 
> to it.
> 
> Regards
> 
> Steve.
> --
> 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


Re: [PATCH 2/2] [RFC v2 with seqcount] reservation: add suppport for read-only access using rcu

2014-04-11 Thread Thomas Hellstrom
On 04/11/2014 11:24 AM, Maarten Lankhorst wrote:
> op 11-04-14 10:38, Thomas Hellstrom schreef:
>> Hi, Maarten.
>>
>> Here I believe we encounter a lot of locking inconsistencies.
>>
>> First, it seems you're use a number of pointers as RCU pointers without
>> annotating them as such and use the correct rcu
>> macros when assigning those pointers.
>>
>> Some pointers (like the pointers in the shared fence list) are both used
>> as RCU pointers (in dma_buf_poll()) for example,
>> or considered protected by the seqlock
>> (reservation_object_get_fences_rcu()), which I believe is OK, but then
>> the pointers must
>> be assigned using the correct rcu macros. In the memcpy in
>> reservation_object_get_fences_rcu() we might get away with an
>> ugly typecast, but with a verbose comment that the pointers are
>> considered protected by the seqlock at that location.
>>
>> So I've updated (attached) the headers with proper __rcu annotation and
>> locking comments according to how they are being used in the various
>> reading functions.
>> I believe if we want to get rid of this we need to validate those
>> pointers using the seqlock as well.
>> This will generate a lot of sparse warnings in those places needing
>> rcu_dereference()
>> rcu_assign_pointer()
>> rcu_dereference_protected()
>>
>> With this I think we can get rid of all ACCESS_ONCE macros: It's not
>> needed when the rcu_x() macros are used, and
>> it's never needed for the members protected by the seqlock, (provided
>> that the seq is tested). The only place where I think that's
>> *not* the case is at the krealloc in
>> reservation_object_get_fences_rcu().
>>
>> Also I have some more comments in the
>> reservation_object_get_fences_rcu() function below:
> I felt that the barriers needed for rcu were already provided by
> checking the seqcount lock.
> But looking at rcu_dereference makes it seem harmless to add it in
> more places, it handles
> the ACCESS_ONCE and barrier() for us.

And it makes the code more maintainable, and helps sparse doing a lot of
checking for us. I guess
we can tolerate a couple of extra barriers for that.

>
> We could probably get away with using RCU_INIT_POINTER on the writer
> side,
> because the smp_wmb is already done by arranging seqcount updates
> correctly.

Hmm. yes, probably. At least in the replace function. I think if we do
it in other places, we should add comments as to where
the smp_wmb() is located, for future reference.


Also  I saw in a couple of places where you're checking the shared
pointers, you're not checking for NULL pointers, which I guess may
happen if shared_count and pointers are not in full sync?

Thanks,
/Thomas


>
>> diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
>> index d89a98d2c37b..ca6ef0c4b358 100644
>> --- a/drivers/base/dma-buf.c
>> +++ b/drivers/base/dma-buf.c
>>
>> +int reservation_object_get_fences_rcu(struct reservation_object *obj,
>> +  struct fence **pfence_excl,
>> +  unsigned *pshared_count,
>> +  struct fence ***pshared)
>> +{
>> +unsigned shared_count = 0;
>> +unsigned retry = 1;
>> +struct fence **shared = NULL, *fence_excl = NULL;
>> +int ret = 0;
>> +
>> +while (retry) {
>> +struct reservation_object_list *fobj;
>> +unsigned seq, retry;
>> You're shadowing retry?
> Oops.
>>
>>> +
>>> +seq = read_seqcount_begin(&obj->seq);
>>> +
>>> +rcu_read_lock();
>>> +
>>> +fobj = ACCESS_ONCE(obj->fence);
>>> +if (fobj) {
>>> +struct fence **nshared;
>>> +
>>> +shared_count = ACCESS_ONCE(fobj->shared_count);
>>> +nshared = krealloc(shared, sizeof(*shared) *
>>> shared_count, GFP_KERNEL);
>> krealloc inside rcu_read_lock(). Better to put this first in the loop.
> Except that shared_count isn't known until the rcu_read_lock is taken.
>> Thanks,
>> Thomas
> ~Maarten
--
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: Hauppauge ImpactVCB-e 01385

2014-04-11 Thread Steve Cookson - IT

Hi Hans,

Thanks for this, I'll do as you suggested.

On 11/04/14 10:45, Hans Verkuil wrote:

I have serious doubts whether this is actually supported. I see no mention of
that board in the cx23885 driver. I wonder if there is a mixup between the
ImpactVCB (which IS supported) and the ImpactVCB-e.
Do you think there is another internal card out there that would do the 
job of capturing raw SD video?


What would you recommend.

I'm currently using the Dazzle DVC100, stripping the case off and 
installing it internally with cable ties.   It's just a bit basic. I'd 
like something a bit more professional like a half-height PCIe board.  
Which is why I selected the ImpactVCB-e, but really I have no attachment 
to it.


Regards

Steve.
--
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: Hauppauge ImpactVCB-e 01385

2014-04-11 Thread Hans Verkuil
On 04/11/2014 11:09 AM, Steve Cookson - IT wrote:
> So I'm back to the Hauppauge ImpactVCB-e 01385.
> 
> Apparently it's fully supported by the current Linux kernel:
> 
> Model Standard Interface Supported Comments
> ImpactVCB-e Video PCIe ✔ Yes No 
> tuners, only video-in. S-Video Capture works with kernel 3.5.0 (Ubuntu 
> 12.10).
> 
> http://linuxtv.org/wiki/index.php/Hauppauge.
> 
> So is this a typo or have I just encountered an install problem?

I have serious doubts whether this is actually supported. I see no mention of
that board in the cx23885 driver. I wonder if there is a mixup between the
ImpactVCB (which IS supported) and the ImpactVCB-e.

> 
>> When I plug in my 01385 I get the same old stuff in dmseg, ie:
>>
>> cx23885 driver version 0.0.3 loaded
>> [ 8.921390] cx23885[0]: Your board isn't known (yet) to the driver.
>> [ 8.921390] cx23885[0]: Try to pick one of the existing card configs via
>> [ 8.921390] cx23885[0]: card= insmod option. Updating to the latest
>> [ 8.921390] cx23885[0]: version might help as well.
>> [ 8.921393] cx23885[0]: Here is a list of valid choices for the 
>> card= insmod option:

You can try some of the existing cards: one  of 1, 2, 3, 6, 20, 24, 32 might
just work. Look in drivers/media/pci/cx23885/cx23885-cards.c.

Each card definition there defines the inputs that are supported by the card.
There is no perfect match, so you will have to change inputs to see which
input produces an image. You can also add a card definition yourself and
just fiddle around with the vmux/amux/gpio values to see which work. It is
probably something close to what is used by other Hauppauge cards.

>>
>> Etc.
> Would the daily build resolve this?  I haven't installed it on this test 
> system, but I'm never clear when I should install it or whether I should 
> just download a single driver from somewhere.

There is no point in using the daily build. The cx23885 driver hasn't been
updated in a long time.

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: [REVIEWv3 PATCH 00/13] vb2: various small fixes/improvements

2014-04-11 Thread Sakari Ailus
On Fri, Apr 11, 2014 at 10:11:06AM +0200, Hans Verkuil wrote:
> This is the third version of this review patch series.
> The previous can be found here:
> 
> http://www.spinics.net/lists/linux-media/msg75428.html
> 
> Changes since v2:
> 
> - Updated v4l2-pci-skeleton.c as well in patch 01/13
> - Dropped patch 10/13 as it is not needed
> - Added comment to patch 06/13 as suggested by Pawel
> - Added patch 13/13: fix HDTV interlaced handling in v4l2-pci-skeleton.c

For patches 10--13,

Acked-by: Sakari Ailus 

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
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: [v2] media: soc-camera: OF cameras

2014-04-11 Thread Ben Dooks

On 10/04/14 22:18, Guennadi Liakhovetski wrote:

Hi Bryan,

On Tue, 8 Apr 2014, Bryan Wu wrote:


Thanks Josh, I think I will take you point and rework my patch again.
But I need Guennadi's review firstly, Guennadi, could you please help
to review it?


Ok, let me double check the situation:

1. We've got this patch from you, aiming at adding OF probing support to
soc-camra

2. We've got an alternative patch from Ben to do the same, his last reply
to a comment to his patch was "Thanks, I will look into this."

3. We've got Ben's patches for rcar-vin, that presumably work with his
patch from (2) above

4. We've got Josh's patches to add OF / async probing to atmel-isi and
ov2640, that are not known to work with either (1) or (2) above, so, they
don't work at all, right?

So, to summarise, there is a core patch from Ben, that he possibly wants
to adjust, and that works with his rcar-vin OF, there is a patch from you
that isn't known to work with any driver, and there are patches from Josh,
that don't work, because there isn't a suitable patch available for them.
I will have a look at your and Ben's soc-camera OF patches to compare them
and compare them with my early code (hopefully this coming weekend), but
so far it looks like only Ben's solution has a complete working stack. Am
I missing something?


I am looking in to fix the comments from Josh to get the atmel to
work and hope to have them out this weekend.

--
Ben Dooks   http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
--
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/2] [RFC v2 with seqcount] reservation: add suppport for read-only access using rcu

2014-04-11 Thread Maarten Lankhorst

op 11-04-14 10:38, Thomas Hellstrom schreef:

Hi, Maarten.

Here I believe we encounter a lot of locking inconsistencies.

First, it seems you're use a number of pointers as RCU pointers without
annotating them as such and use the correct rcu
macros when assigning those pointers.

Some pointers (like the pointers in the shared fence list) are both used
as RCU pointers (in dma_buf_poll()) for example,
or considered protected by the seqlock
(reservation_object_get_fences_rcu()), which I believe is OK, but then
the pointers must
be assigned using the correct rcu macros. In the memcpy in
reservation_object_get_fences_rcu() we might get away with an
ugly typecast, but with a verbose comment that the pointers are
considered protected by the seqlock at that location.

So I've updated (attached) the headers with proper __rcu annotation and
locking comments according to how they are being used in the various
reading functions.
I believe if we want to get rid of this we need to validate those
pointers using the seqlock as well.
This will generate a lot of sparse warnings in those places needing
rcu_dereference()
rcu_assign_pointer()
rcu_dereference_protected()

With this I think we can get rid of all ACCESS_ONCE macros: It's not
needed when the rcu_x() macros are used, and
it's never needed for the members protected by the seqlock, (provided
that the seq is tested). The only place where I think that's
*not* the case is at the krealloc in reservation_object_get_fences_rcu().

Also I have some more comments in the
reservation_object_get_fences_rcu() function below:

I felt that the barriers needed for rcu were already provided by checking the 
seqcount lock.
But looking at rcu_dereference makes it seem harmless to add it in more places, 
it handles
the ACCESS_ONCE and barrier() for us.

We could probably get away with using RCU_INIT_POINTER on the writer side,
because the smp_wmb is already done by arranging seqcount updates correctly.


diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
index d89a98d2c37b..ca6ef0c4b358 100644
--- a/drivers/base/dma-buf.c
+++ b/drivers/base/dma-buf.c

+int reservation_object_get_fences_rcu(struct reservation_object *obj,
+  struct fence **pfence_excl,
+  unsigned *pshared_count,
+  struct fence ***pshared)
+{
+unsigned shared_count = 0;
+unsigned retry = 1;
+struct fence **shared = NULL, *fence_excl = NULL;
+int ret = 0;
+
+while (retry) {
+struct reservation_object_list *fobj;
+unsigned seq, retry;
You're shadowing retry?

Oops.



+
+seq = read_seqcount_begin(&obj->seq);
+
+rcu_read_lock();
+
+fobj = ACCESS_ONCE(obj->fence);
+if (fobj) {
+struct fence **nshared;
+
+shared_count = ACCESS_ONCE(fobj->shared_count);
+nshared = krealloc(shared, sizeof(*shared) *
shared_count, GFP_KERNEL);

krealloc inside rcu_read_lock(). Better to put this first in the loop.

Except that shared_count isn't known until the rcu_read_lock is taken.

Thanks,
Thomas

~Maarten
--
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: Hauppauge ImpactVCB-e 01385

2014-04-11 Thread Steve Cookson - IT

So I'm back to the Hauppauge ImpactVCB-e 01385.

Apparently it's fully supported by the current Linux kernel:

Model Standard Interface Supported Comments
ImpactVCB-e Video PCIe ✔ Yes No 
tuners, only video-in. S-Video Capture works with kernel 3.5.0 (Ubuntu 
12.10).


http://linuxtv.org/wiki/index.php/Hauppauge.

So is this a typo or have I just encountered an install problem?


When I plug in my 01385 I get the same old stuff in dmseg, ie:

cx23885 driver version 0.0.3 loaded
[ 8.921390] cx23885[0]: Your board isn't known (yet) to the driver.
[ 8.921390] cx23885[0]: Try to pick one of the existing card configs via
[ 8.921390] cx23885[0]: card= insmod option. Updating to the latest
[ 8.921390] cx23885[0]: version might help as well.
[ 8.921393] cx23885[0]: Here is a list of valid choices for the 
card= insmod option:


Etc.
Would the daily build resolve this?  I haven't installed it on this test 
system, but I'm never clear when I should install it or whether I should 
just download a single driver from somewhere.


Regards

Steve
--
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: [REVIEWv3 PATCH 07/13] vb2: reject output buffers with V4L2_FIELD_ALTERNATE

2014-04-11 Thread Archit Taneja

On Friday 11 April 2014 02:28 PM, Hans Verkuil wrote:

On 04/11/2014 10:42 AM, Archit Taneja wrote:

On Friday 11 April 2014 01:41 PM, Hans Verkuil wrote:

From: Hans Verkuil 

This is not allowed by the spec and does in fact not make any sense.
Return -EINVAL if this is the case.

Signed-off-by: Hans Verkuil 
Acked-by: Pawel Osciak 
Acked-by: Sakari Ailus 
---
   drivers/media/v4l2-core/videobuf2-core.c | 13 +
   1 file changed, 13 insertions(+)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
b/drivers/media/v4l2-core/videobuf2-core.c
index 6e05495..f8c0247 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1511,6 +1511,19 @@ static int __buf_prepare(struct vb2_buffer *vb, const 
struct v4l2_buffer *b)
dprintk(1, "plane parameters verification failed: %d\n", ret);
return ret;
}
+   if (b->field == V4L2_FIELD_ALTERNATE && V4L2_TYPE_IS_OUTPUT(q->type)) {
+   /*
+* If the format's field is ALTERNATE, then the buffer's field
+* should be either TOP or BOTTOM, not ALTERNATE since that
+* makes no sense. The driver has to know whether the
+* buffer represents a top or a bottom field in order to
+* program any DMA correctly. Using ALTERNATE is wrong, since
+* that just says that it is either a top or a bottom field,
+* but not which of the two it is.
+*/
+   dprintk(1, "the field is incorrectly set to ALTERNATE for an output 
buffer\n");
+   return -EINVAL;
+   }


If vb2_queue had a field parameter, which tells the format's field type.
We could have returned an error if the field format was ALTERNATE, and
the buffer field was not TOP or BOTTOM.

I don't know whether having a field parameter in vb2_queue is a good
idea or not.


The predecessor of vb2, videobuf, had that actually.

I am not sure myself if it is a good idea or not to do the same for vb2.
For now I think we should leave it as is. There are very few drivers that
support FIELD_ALTERNATE although this should become more common for
drivers supporting interlaced HDTV formats. When we see more drivers that
support this, then we can see if it makes sense to move part of the handling
to vb2.



Sure, queue_setup vb op might be a good place to populate it. But as you 
said, there aren't many drivers that use FIELD_ALTERNATE, and there 
doesn't seem to be any other advantage for having 'field' in vb2_queue. 
So, it can be left for later.


Archit

--
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: [REVIEWv3 PATCH 07/13] vb2: reject output buffers with V4L2_FIELD_ALTERNATE

2014-04-11 Thread Hans Verkuil
On 04/11/2014 10:42 AM, Archit Taneja wrote:
> On Friday 11 April 2014 01:41 PM, Hans Verkuil wrote:
>> From: Hans Verkuil 
>>
>> This is not allowed by the spec and does in fact not make any sense.
>> Return -EINVAL if this is the case.
>>
>> Signed-off-by: Hans Verkuil 
>> Acked-by: Pawel Osciak 
>> Acked-by: Sakari Ailus 
>> ---
>>   drivers/media/v4l2-core/videobuf2-core.c | 13 +
>>   1 file changed, 13 insertions(+)
>>
>> diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
>> b/drivers/media/v4l2-core/videobuf2-core.c
>> index 6e05495..f8c0247 100644
>> --- a/drivers/media/v4l2-core/videobuf2-core.c
>> +++ b/drivers/media/v4l2-core/videobuf2-core.c
>> @@ -1511,6 +1511,19 @@ static int __buf_prepare(struct vb2_buffer *vb, const 
>> struct v4l2_buffer *b)
>>  dprintk(1, "plane parameters verification failed: %d\n", ret);
>>  return ret;
>>  }
>> +if (b->field == V4L2_FIELD_ALTERNATE && V4L2_TYPE_IS_OUTPUT(q->type)) {
>> +/*
>> + * If the format's field is ALTERNATE, then the buffer's field
>> + * should be either TOP or BOTTOM, not ALTERNATE since that
>> + * makes no sense. The driver has to know whether the
>> + * buffer represents a top or a bottom field in order to
>> + * program any DMA correctly. Using ALTERNATE is wrong, since
>> + * that just says that it is either a top or a bottom field,
>> + * but not which of the two it is.
>> + */
>> +dprintk(1, "the field is incorrectly set to ALTERNATE for an 
>> output buffer\n");
>> +return -EINVAL;
>> +}
> 
> If vb2_queue had a field parameter, which tells the format's field type. 
> We could have returned an error if the field format was ALTERNATE, and 
> the buffer field was not TOP or BOTTOM.
> 
> I don't know whether having a field parameter in vb2_queue is a good 
> idea or not.

The predecessor of vb2, videobuf, had that actually.

I am not sure myself if it is a good idea or not to do the same for vb2.
For now I think we should leave it as is. There are very few drivers that
support FIELD_ALTERNATE although this should become more common for
drivers supporting interlaced HDTV formats. When we see more drivers that
support this, then we can see if it makes sense to move part of the handling
to vb2.

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: [PATCHv2 for v3.15] vb2: call finish() memop for prepared/queued buffers

2014-04-11 Thread Hans Verkuil
Please ignore this patch. This isn't valid for the current 3.15 tree. It will
become relevant later after additional patches have been applied.

I've rejected this patch in patchwork.

Regards,

Hans

On 03/28/2014 12:43 PM, Hans Verkuil wrote:
> This supersedes "[PATCH for v3.15] vb2: call __buf_finish_memory for
> prepared/queued buffers". I realized that that patch was for an internal
> git branch and didn't apply to the master branch. The fix is the same,
> though.
> 
> v4l2-compliance reports unbalanced prepare/finish memops in the case
> where buffers are queued, streamon is never called and then reqbufs()
> is called that has to cancel any queued buffers.
> 
> When canceling a queue the finish() memop should be called for all
> buffers in the state 'PREPARED' or 'QUEUED' to ensure the prepare() and
> finish() memops are balanced.
> 
> Signed-off-by: Hans Verkuil 
> ---
>  drivers/media/v4l2-core/videobuf2-core.c | 16 +---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
> b/drivers/media/v4l2-core/videobuf2-core.c
> index f9059bb..cc1bd5a 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -1063,6 +1063,15 @@ void *vb2_plane_cookie(struct vb2_buffer *vb, unsigned 
> int plane_no)
>  }
>  EXPORT_SYMBOL_GPL(vb2_plane_cookie);
>  
> +static void vb2_buffer_finish(struct vb2_buffer *vb)
> +{
> + unsigned plane;
> +
> + /* sync buffers */
> + for (plane = 0; plane < vb->num_planes; ++plane)
> + call_memop(vb, finish, vb->planes[plane].mem_priv);
> +}
> +
>  /**
>   * vb2_buffer_done() - inform videobuf that an operation on a buffer is 
> finished
>   * @vb:  vb2_buffer returned from the driver
> @@ -1086,7 +1095,6 @@ 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 (WARN_ON(vb->state != VB2_BUF_STATE_ACTIVE))
>   return;
> @@ -,8 +1119,7 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum 
> vb2_buffer_state state)
>   vb->v4l2_buf.index, state);
>  
>   /* sync buffers */
> - for (plane = 0; plane < vb->num_planes; ++plane)
> - call_memop(vb, finish, vb->planes[plane].mem_priv);
> + vb2_buffer_finish(vb);
>  
>   /* Add the buffer to the done buffers list */
>   spin_lock_irqsave(&q->done_lock, flags);
> @@ -2041,6 +2048,9 @@ static void __vb2_queue_cancel(struct vb2_queue *q)
>   struct vb2_buffer *vb = q->bufs[i];
>  
>   if (vb->state != VB2_BUF_STATE_DEQUEUED) {
> + if (vb->state == VB2_BUF_STATE_QUEUED ||
> + vb->state == VB2_BUF_STATE_PREPARED)
> + vb2_buffer_finish(vb);
>   vb->state = VB2_BUF_STATE_PREPARED;
>   call_vb_qop(vb, buf_finish, vb);
>   }
> 

--
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: [REVIEWv3 PATCH 07/13] vb2: reject output buffers with V4L2_FIELD_ALTERNATE

2014-04-11 Thread Archit Taneja

On Friday 11 April 2014 01:41 PM, Hans Verkuil wrote:

From: Hans Verkuil 

This is not allowed by the spec and does in fact not make any sense.
Return -EINVAL if this is the case.

Signed-off-by: Hans Verkuil 
Acked-by: Pawel Osciak 
Acked-by: Sakari Ailus 
---
  drivers/media/v4l2-core/videobuf2-core.c | 13 +
  1 file changed, 13 insertions(+)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
b/drivers/media/v4l2-core/videobuf2-core.c
index 6e05495..f8c0247 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1511,6 +1511,19 @@ static int __buf_prepare(struct vb2_buffer *vb, const 
struct v4l2_buffer *b)
dprintk(1, "plane parameters verification failed: %d\n", ret);
return ret;
}
+   if (b->field == V4L2_FIELD_ALTERNATE && V4L2_TYPE_IS_OUTPUT(q->type)) {
+   /*
+* If the format's field is ALTERNATE, then the buffer's field
+* should be either TOP or BOTTOM, not ALTERNATE since that
+* makes no sense. The driver has to know whether the
+* buffer represents a top or a bottom field in order to
+* program any DMA correctly. Using ALTERNATE is wrong, since
+* that just says that it is either a top or a bottom field,
+* but not which of the two it is.
+*/
+   dprintk(1, "the field is incorrectly set to ALTERNATE for an output 
buffer\n");
+   return -EINVAL;
+   }


If vb2_queue had a field parameter, which tells the format's field type. 
We could have returned an error if the field format was ALTERNATE, and 
the buffer field was not TOP or BOTTOM.


I don't know whether having a field parameter in vb2_queue is a good 
idea or not.


Thanks,
Archit

--
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/2] [RFC v2 with seqcount] reservation: add suppport for read-only access using rcu

2014-04-11 Thread Thomas Hellstrom
Hi, Maarten.

Here I believe we encounter a lot of locking inconsistencies.

First, it seems you're use a number of pointers as RCU pointers without
annotating them as such and use the correct rcu
macros when assigning those pointers.

Some pointers (like the pointers in the shared fence list) are both used
as RCU pointers (in dma_buf_poll()) for example,
or considered protected by the seqlock
(reservation_object_get_fences_rcu()), which I believe is OK, but then
the pointers must
be assigned using the correct rcu macros. In the memcpy in
reservation_object_get_fences_rcu() we might get away with an
ugly typecast, but with a verbose comment that the pointers are
considered protected by the seqlock at that location.

So I've updated (attached) the headers with proper __rcu annotation and
locking comments according to how they are being used in the various
reading functions.
I believe if we want to get rid of this we need to validate those
pointers using the seqlock as well.
This will generate a lot of sparse warnings in those places needing
rcu_dereference()
rcu_assign_pointer()
rcu_dereference_protected()

With this I think we can get rid of all ACCESS_ONCE macros: It's not
needed when the rcu_x() macros are used, and
it's never needed for the members protected by the seqlock, (provided
that the seq is tested). The only place where I think that's
*not* the case is at the krealloc in reservation_object_get_fences_rcu().

Also I have some more comments in the
reservation_object_get_fences_rcu() function below:


On 04/10/2014 05:00 PM, Maarten Lankhorst wrote:
> op 10-04-14 13:08, Thomas Hellstrom schreef:
>> On 04/10/2014 12:07 PM, Maarten Lankhorst wrote:
>>> Hey,
>>>
>>> op 10-04-14 10:46, Thomas Hellstrom schreef:
 Hi!

 Ugh. This became more complicated than I thought, but I'm OK with
 moving
 TTM over to fence while we sort out
 how / if we're going to use this.

 While reviewing, it struck me that this is kind of error-prone, and
 hard
 to follow since we're operating on a structure that may be
 continually updated under us, needing a lot of RCU-specific macros and
 barriers.
>>> Yeah, but with the exception of dma_buf_poll I don't think there is
>>> anything else
>>> outside drivers/base/reservation.c has to deal with rcu.
>>>
 Also the rcu wait appears to not complete until there are no busy
 fences
 left (new ones can be added while we wait) rather than
 waiting on a snapshot of busy fences.
>>> This has been by design, because 'wait for bo idle' type of functions
>>> only care
>>> if the bo is completely idle or not.
>> No, not when using RCU, because the bo may be busy again before the
>> function returns :)
>> Complete idleness can only be guaranteed if holding the reservation, or
>> otherwise making sure
>> that no new rendering is submitted to the buffer, so it's an overkill to
>> wait for complete idleness here.
> You're probably right, but it makes waiting a lot easier if I don't
> have to deal with memory allocations. :P
>>> It would be easy to make a snapshot even without seqlocks, just copy
>>> reservation_object_test_signaled_rcu to return a shared list if
>>> test_all is set, or return pointer to exclusive otherwise.
>>>
 I wonder if these issues can be addressed by having a function that
 provides a snapshot of all busy fences: This can be accomplished
 either by including the exclusive fence in the fence_list structure
 and
 allocate a new such structure each time it is updated. The RCU reader
 could then just make a copy of the current fence_list structure
 pointed
 to by &obj->fence, but I'm not sure we want to reallocate *each*
 time we
 update the fence pointer.
>>> No, the most common operation is updating fence pointers, which is why
>>> the current design makes that cheap. It's also why doing rcu reads is
>>> more expensive.
 The other approach uses a seqlock to obtain a consistent snapshot, and
 I've attached an incomplete outline, and I'm not 100% whether it's
 OK to
 combine RCU and seqlocks in this way...

 Both these approaches have the benefit of hiding the RCU
 snapshotting in
 a single function, that can then be used by any waiting
 or polling function.

>>> I think the middle way with using seqlocks to protect the fence_excl
>>> pointer and shared list combination,
>>> and using RCU to protect the refcounts for fences and the availability
>>> of the list could work for our usecase
>>> and might remove a bunch of memory barriers. But yeah that depends on
>>> layering rcu and seqlocks.
>>> No idea if that is allowed. But I suppose it is.
>>>
>>> Also, you're being overly paranoid with seqlock reading, we would only
>>> need something like this:
>>>
>>> rcu_read_lock()
>>>  preempt_disable()
>>>  seq = read_seqcount_begin()
>>>  read fence_excl, shared_count = ACCESS_ONCE(fence->shared_count)
>>>  copy shared to a 

Re: List objectives and interests.

2014-04-11 Thread Hans Verkuil
On 04/11/2014 10:20 AM, Steve Cookson - IT wrote:
> Hi Hans,
> 
> Thanks for your reply.
> 
> On 11/04/14 08:50, Hans Verkuil wrote:
>> HD is well supported for embedded systems
> What does embedded systems mean, you mean like the decklink proprietary 
> software?

No, I'm referring to SoC support, e.g. support for the HDTV capabilities of
a Samsung exynos SoC etc.

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: List objectives and interests.

2014-04-11 Thread Steve Cookson - IT

Hi Hans,

Thanks for your reply.

On 11/04/14 08:50, Hans Verkuil wrote:

HD is well supported for embedded systems
What does embedded systems mean, you mean like the decklink proprietary 
software?


Regards

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


[REVIEWv3 PATCH 08/13] vb2: simplify a confusing condition.

2014-04-11 Thread Hans Verkuil
From: Hans Verkuil 

q->start_streaming_called is always true, so the WARN_ON check against
it being false can be dropped.

Signed-off-by: Hans Verkuil 
Acked-by: Pawel Osciak 
Acked-by: Sakari Ailus 
---
 drivers/media/v4l2-core/videobuf2-core.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
b/drivers/media/v4l2-core/videobuf2-core.c
index f8c0247..3d2a003 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1094,9 +1094,8 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum 
vb2_buffer_state state)
if (!q->start_streaming_called) {
if (WARN_ON(state != VB2_BUF_STATE_QUEUED))
state = VB2_BUF_STATE_QUEUED;
-   } else if (!WARN_ON(!q->start_streaming_called)) {
-   if (WARN_ON(state != VB2_BUF_STATE_DONE &&
-   state != VB2_BUF_STATE_ERROR))
+   } else if (WARN_ON(state != VB2_BUF_STATE_DONE &&
+  state != VB2_BUF_STATE_ERROR)) {
state = VB2_BUF_STATE_ERROR;
}
 
-- 
1.9.1

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


[REVIEWv3 PATCH 06/13] vb2: set timestamp when using write()

2014-04-11 Thread Hans Verkuil
From: Hans Verkuil 

When using write() to write data to an output video node the vb2 core
should set timestamps if V4L2_BUF_FLAG_TIMESTAMP_COPY is set. Nobody
else is able to provide this information with the write() operation.

Signed-off-by: Hans Verkuil 
Acked-by: Pawel Osciak 
Acked-by: Sakari Ailus 
---
 drivers/media/v4l2-core/videobuf2-core.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
b/drivers/media/v4l2-core/videobuf2-core.c
index 2e448a7..6e05495 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -22,6 +22,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 static int debug;
@@ -2751,6 +2752,14 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, 
char __user *data, size_
 {
struct vb2_fileio_data *fileio;
struct vb2_fileio_buf *buf;
+   /*
+* When using write() to write data to an output video node the vb2 core
+* should set timestamps if V4L2_BUF_FLAG_TIMESTAMP_COPY is set. Nobody
+* else is able to provide this information with the write() operation.
+*/
+   bool set_timestamp = !read &&
+   (q->timestamp_flags & V4L2_BUF_FLAG_TIMESTAMP_MASK) ==
+   V4L2_BUF_FLAG_TIMESTAMP_COPY;
int ret, index;
 
dprintk(3, "mode %s, offset %ld, count %zd, %sblocking\n",
@@ -2852,6 +2861,8 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, 
char __user *data, size_
fileio->b.memory = q->memory;
fileio->b.index = index;
fileio->b.bytesused = buf->pos;
+   if (set_timestamp)
+   v4l2_get_timestamp(&fileio->b.timestamp);
ret = vb2_internal_qbuf(q, &fileio->b);
dprintk(5, "vb2_dbuf result: %d\n", ret);
if (ret)
-- 
1.9.1

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


[REVIEWv3 PATCH 12/13] DocBook media: update bytesused field description

2014-04-11 Thread Hans Verkuil
From: Hans Verkuil 

For output buffers the application has to set the bytesused field.
In reality applications often do not set this since drivers that
deal with fix image sizes just override it anyway.

The vb2 framework will replace this field with the length field if
bytesused was set to 0 by the application, which is what happens
in practice. Document this behavior.

Signed-off-by: Hans Verkuil 
---
 Documentation/DocBook/media/v4l/io.xml | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/io.xml 
b/Documentation/DocBook/media/v4l/io.xml
index 97a69bf..188e621 100644
--- a/Documentation/DocBook/media/v4l/io.xml
+++ b/Documentation/DocBook/media/v4l/io.xml
@@ -699,7 +699,12 @@ linkend="v4l2-buf-type" />
 buffer. It depends on the negotiated data format and may change with
 each buffer for compressed variable size data like JPEG images.
 Drivers must set this field when type
-refers to an input stream, applications when it refers to an output 
stream.
+refers to an input stream, applications when it refers to an output stream.
+If the application sets this to 0 for an output stream, then
+bytesused will be set to the size of the
+buffer (see the length field of this struct) by
+the driver. For multiplanar formats this field is ignored and the
+planes pointer is used instead.
  
  
__u32
@@ -861,7 +866,11 @@ should set this to 0.

The number of bytes occupied by data in the plane
  (its payload). Drivers must set this field when 
type
- refers to an input stream, applications when it refers to an 
output stream.
+ refers to an input stream, applications when it refers to an 
output stream.
+ If the application sets this to 0 for an output stream, then
+ bytesused will be set to the size of 
the
+ plane (see the length field of this 
struct)
+ by the driver.
  
  
__u32
-- 
1.9.1

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


[REVIEWv3 PATCH 09/13] vb2: add vb2_fileio_is_active and check it more often

2014-04-11 Thread Hans Verkuil
From: Hans Verkuil 

Added a vb2_fileio_is_active inline function that returns true if fileio
is in progress. Check for this too in mmap() (you don't want apps mmap()ing
buffers used by fileio) and expbuf() (same reason).

In addition drivers should be able to check for this in queue_setup() to
return an error if an attempt is made to read() or write() with
V4L2_FIELD_ALTERNATE being configured. This is illegal (there is no way
to pass the TOP/BOTTOM information around using file I/O).

However, in order to be able to check for this the init_fileio function
needs to set q->fileio early on, before the buffers are allocated. So switch
to using internal functions (__reqbufs, vb2_internal_qbuf and
vb2_internal_streamon) to skip the fileio check. Well, that's why the internal
functions were created...

Signed-off-by: Hans Verkuil 
Acked-by: Pawel Osciak 
Acked-by: Sakari Ailus 
---
 drivers/media/v4l2-core/videobuf2-core.c | 39 
 include/media/videobuf2-core.h   | 17 ++
 2 files changed, 41 insertions(+), 15 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
b/drivers/media/v4l2-core/videobuf2-core.c
index 3d2a003..acabf1f 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -755,7 +755,7 @@ static int __verify_memory_type(struct vb2_queue *q,
 * create_bufs is called with count == 0, but count == 0 should still
 * do the memory and type validation.
 */
-   if (q->fileio) {
+   if (vb2_fileio_is_active(q)) {
dprintk(1, "file io in progress\n");
return -EBUSY;
}
@@ -1617,7 +1617,7 @@ int vb2_prepare_buf(struct vb2_queue *q, struct 
v4l2_buffer *b)
struct vb2_buffer *vb;
int ret;
 
-   if (q->fileio) {
+   if (vb2_fileio_is_active(q)) {
dprintk(1, "file io in progress\n");
return -EBUSY;
}
@@ -1786,7 +1786,7 @@ static int vb2_internal_qbuf(struct vb2_queue *q, struct 
v4l2_buffer *b)
  */
 int vb2_qbuf(struct vb2_queue *q, struct v4l2_buffer *b)
 {
-   if (q->fileio) {
+   if (vb2_fileio_is_active(q)) {
dprintk(1, "file io in progress\n");
return -EBUSY;
}
@@ -2006,7 +2006,7 @@ static int vb2_internal_dqbuf(struct vb2_queue *q, struct 
v4l2_buffer *b, bool n
  */
 int vb2_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool nonblocking)
 {
-   if (q->fileio) {
+   if (vb2_fileio_is_active(q)) {
dprintk(1, "file io in progress\n");
return -EBUSY;
}
@@ -2136,7 +2136,7 @@ static int vb2_internal_streamon(struct vb2_queue *q, 
enum v4l2_buf_type type)
  */
 int vb2_streamon(struct vb2_queue *q, enum v4l2_buf_type type)
 {
-   if (q->fileio) {
+   if (vb2_fileio_is_active(q)) {
dprintk(1, "file io in progress\n");
return -EBUSY;
}
@@ -2183,7 +2183,7 @@ static int vb2_internal_streamoff(struct vb2_queue *q, 
enum v4l2_buf_type type)
  */
 int vb2_streamoff(struct vb2_queue *q, enum v4l2_buf_type type)
 {
-   if (q->fileio) {
+   if (vb2_fileio_is_active(q)) {
dprintk(1, "file io in progress\n");
return -EBUSY;
}
@@ -2268,6 +2268,11 @@ int vb2_expbuf(struct vb2_queue *q, struct 
v4l2_exportbuffer *eb)
return -EINVAL;
}
 
+   if (vb2_fileio_is_active(q)) {
+   dprintk(1, "expbuf: file io in progress\n");
+   return -EBUSY;
+   }
+
vb_plane = &vb->planes[eb->plane];
 
dbuf = call_memop(vb, get_dmabuf, vb_plane->mem_priv, eb->flags & 
O_ACCMODE);
@@ -2344,6 +2349,10 @@ int vb2_mmap(struct vb2_queue *q, struct vm_area_struct 
*vma)
return -EINVAL;
}
}
+   if (vb2_fileio_is_active(q)) {
+   dprintk(1, "mmap: file io in progress\n");
+   return -EBUSY;
+   }
 
/*
 * Find the plane corresponding to the offset passed by userspace.
@@ -2455,7 +2464,7 @@ unsigned int vb2_poll(struct vb2_queue *q, struct file 
*file, poll_table *wait)
/*
 * Start file I/O emulator only if streaming API has not been used yet.
 */
-   if (q->num_buffers == 0 && q->fileio == NULL) {
+   if (q->num_buffers == 0 && !vb2_fileio_is_active(q)) {
if (!V4L2_TYPE_IS_OUTPUT(q->type) && (q->io_modes & VB2_READ) &&
(req_events & (POLLIN | POLLRDNORM))) {
if (__vb2_init_fileio(q, 1))
@@ -2660,7 +2669,8 @@ static int __vb2_init_fileio(struct vb2_queue *q, int 
read)
fileio->req.count = count;
fileio->req.memory = V4L2_MEMORY_MMAP;
fileio->req.type = q->type;
-   ret = vb2_reqbufs(q, &fileio->req);
+   q->fileio = fileio;
+   ret = __reqbufs(q, &fileio->req);
if (ret)
goto err_kfree;
 
@@ -2698,7

[REVIEWv3 PATCH 11/13] vb2: start messages with a lower-case for consistency.

2014-04-11 Thread Hans Verkuil
From: Hans Verkuil 

The kernel debug messages produced by vb2 started either with a
lower or an upper case character. Switched all to use lower-case
which seemed to be what was used in the majority of the messages.

Signed-off-by: Hans Verkuil 
Acked-by: Pawel Osciak 
---
 drivers/media/v4l2-core/videobuf2-core.c | 58 
 1 file changed, 29 insertions(+), 29 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
b/drivers/media/v4l2-core/videobuf2-core.c
index 6d28f49..aa96997 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -151,7 +151,7 @@ static void __vb2_buf_mem_free(struct vb2_buffer *vb)
for (plane = 0; plane < vb->num_planes; ++plane) {
call_memop(vb, put, vb->planes[plane].mem_priv);
vb->planes[plane].mem_priv = NULL;
-   dprintk(3, "Freed plane %d of buffer %d\n", plane,
+   dprintk(3, "freed plane %d of buffer %d\n", plane,
vb->v4l2_buf.index);
}
 }
@@ -246,7 +246,7 @@ static void __setup_offsets(struct vb2_queue *q, unsigned 
int n)
for (plane = 0; plane < vb->num_planes; ++plane) {
vb->v4l2_planes[plane].m.mem_offset = off;
 
-   dprintk(3, "Buffer %d, plane %d offset 0x%08lx\n",
+   dprintk(3, "buffer %d, plane %d offset 0x%08lx\n",
buffer, plane, off);
 
off += vb->v4l2_planes[plane].length;
@@ -273,7 +273,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum 
v4l2_memory memory,
/* Allocate videobuf buffer structures */
vb = kzalloc(q->buf_struct_size, GFP_KERNEL);
if (!vb) {
-   dprintk(1, "Memory alloc for buffer struct failed\n");
+   dprintk(1, "memory alloc for buffer struct failed\n");
break;
}
 
@@ -292,7 +292,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum 
v4l2_memory memory,
if (memory == V4L2_MEMORY_MMAP) {
ret = __vb2_buf_mem_alloc(vb);
if (ret) {
-   dprintk(1, "Failed allocating memory for "
+   dprintk(1, "failed allocating memory for "
"buffer %d\n", buffer);
kfree(vb);
break;
@@ -304,7 +304,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum 
v4l2_memory memory,
 */
ret = call_vb_qop(vb, buf_init, vb);
if (ret) {
-   dprintk(1, "Buffer %d %p initialization"
+   dprintk(1, "buffer %d %p initialization"
" failed\n", buffer, vb);
fail_vb_qop(vb, buf_init);
__vb2_buf_mem_free(vb);
@@ -320,7 +320,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum 
v4l2_memory memory,
if (memory == V4L2_MEMORY_MMAP)
__setup_offsets(q, buffer);
 
-   dprintk(1, "Allocated %d buffers, %d plane(s) each\n",
+   dprintk(1, "allocated %d buffers, %d plane(s) each\n",
buffer, num_planes);
 
return buffer;
@@ -477,13 +477,13 @@ static int __verify_planes_array(struct vb2_buffer *vb, 
const struct v4l2_buffer
 
/* Is memory for copying plane information present? */
if (NULL == b->m.planes) {
-   dprintk(1, "Multi-planar buffer passed but "
+   dprintk(1, "multi-planar buffer passed but "
   "planes array not provided\n");
return -EINVAL;
}
 
if (b->length < vb->num_planes || b->length > VIDEO_MAX_PLANES) {
-   dprintk(1, "Incorrect planes array length, "
+   dprintk(1, "incorrect planes array length, "
   "expected %d, got %d\n", vb->num_planes, b->length);
return -EINVAL;
}
@@ -846,7 +846,7 @@ static int __reqbufs(struct vb2_queue *q, struct 
v4l2_requestbuffers *req)
/* Finally, allocate buffers and video memory */
allocated_buffers = __vb2_queue_alloc(q, req->memory, num_buffers, 
num_planes);
if (allocated_buffers == 0) {
-   dprintk(1, "Memory allocation failed\n");
+   dprintk(1, "memory allocation failed\n");
return -ENOMEM;
}
 
@@ -959,7 +959,7 @@ static int __create_bufs(struct vb2_queue *q, struct 
v4l2_create_buffers *create
allocated_buffers = __vb2_queue_alloc(q, create->memory, num_buffers,
num_planes);
if (allocated_buffers == 0) {
-   dprintk(1, "Memory allocation failed\n");
+   

[REVIEWv3 PATCH 10/13] vb2: allow read/write as long as the format is single planar

2014-04-11 Thread Hans Verkuil
From: Hans Verkuil 

It was impossible to read() or write() a frame if the queue type was 
multiplanar.
Even if the current format is single planar. Change this to just check whether
the number of planes is 1 or more.

Signed-off-by: Hans Verkuil 
---
 drivers/media/v4l2-core/videobuf2-core.c | 21 +
 1 file changed, 21 insertions(+)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
b/drivers/media/v4l2-core/videobuf2-core.c
index acabf1f..6d28f49 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -2609,6 +2609,7 @@ struct vb2_fileio_buf {
  */
 struct vb2_fileio_data {
struct v4l2_requestbuffers req;
+   struct v4l2_plane p;
struct v4l2_buffer b;
struct vb2_fileio_buf bufs[VIDEO_MAX_FRAME];
unsigned int cur_index;
@@ -2699,13 +2700,21 @@ static int __vb2_init_fileio(struct vb2_queue *q, int 
read)
 * Read mode requires pre queuing of all buffers.
 */
if (read) {
+   bool is_multiplanar = V4L2_TYPE_IS_MULTIPLANAR(q->type);
+
/*
 * Queue all buffers.
 */
for (i = 0; i < q->num_buffers; i++) {
struct v4l2_buffer *b = &fileio->b;
+
memset(b, 0, sizeof(*b));
b->type = q->type;
+   if (is_multiplanar) {
+   memset(&fileio->p, 0, sizeof(fileio->p));
+   b->m.planes = &fileio->p;
+   b->length = 1;
+   }
b->memory = q->memory;
b->index = i;
ret = vb2_internal_qbuf(q, b);
@@ -2773,6 +2782,7 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, 
char __user *data, size_
 {
struct vb2_fileio_data *fileio;
struct vb2_fileio_buf *buf;
+   bool is_multiplanar = V4L2_TYPE_IS_MULTIPLANAR(q->type);
/*
 * When using write() to write data to an output video node the vb2 core
 * should set timestamps if V4L2_BUF_FLAG_TIMESTAMP_COPY is set. Nobody
@@ -2812,6 +2822,11 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, 
char __user *data, size_
memset(&fileio->b, 0, sizeof(fileio->b));
fileio->b.type = q->type;
fileio->b.memory = q->memory;
+   if (is_multiplanar) {
+   memset(&fileio->p, 0, sizeof(fileio->p));
+   fileio->b.m.planes = &fileio->p;
+   fileio->b.length = 1;
+   }
ret = vb2_internal_dqbuf(q, &fileio->b, nonblock);
dprintk(5, "vb2_dqbuf result: %d\n", ret);
if (ret)
@@ -2882,6 +2897,12 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, 
char __user *data, size_
fileio->b.memory = q->memory;
fileio->b.index = index;
fileio->b.bytesused = buf->pos;
+   if (is_multiplanar) {
+   memset(&fileio->p, 0, sizeof(fileio->p));
+   fileio->p.bytesused = buf->pos;
+   fileio->b.m.planes = &fileio->p;
+   fileio->b.length = 1;
+   }
if (set_timestamp)
v4l2_get_timestamp(&fileio->b.timestamp);
ret = vb2_internal_qbuf(q, &fileio->b);
-- 
1.9.1

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


[REVIEWv3 PATCH 02/13] vb2: fix handling of data_offset and v4l2_plane.reserved[]

2014-04-11 Thread Hans Verkuil
From: Hans Verkuil 

The videobuf2-core did not zero the 'planes' array in __qbuf_userptr()
and __qbuf_dmabuf(). That's now memset to 0. Without this the reserved
array in struct v4l2_plane would be non-zero, causing v4l2-compliance
errors.

More serious is the fact that data_offset was not handled correctly:

- for capture devices it was never zeroed, which meant that it was
  uninitialized. Unless the driver sets it it was a completely random
  number. With the memset above this is now fixed.

- __qbuf_dmabuf had a completely incorrect length check that included
  data_offset.

- in __fill_vb2_buffer in the DMABUF case the data_offset field was
  unconditionally copied from v4l2_buffer to v4l2_plane when this
  should only happen in the output case.

- in the single-planar case data_offset was never correctly set to 0.
  The single-planar API doesn't support data_offset, so setting it
  to 0 is the right thing to do. This too is now solved by the memset.

All these issues were found with v4l2-compliance.

Signed-off-by: Hans Verkuil 
Acked-by: Pawel Osciak 
Acked-by: Sakari Ailus 
---
 drivers/media/v4l2-core/videobuf2-core.c | 13 -
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
b/drivers/media/v4l2-core/videobuf2-core.c
index f9059bb..596998e 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1169,8 +1169,6 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, 
const struct v4l2_buffer *b
b->m.planes[plane].m.fd;
v4l2_planes[plane].length =
b->m.planes[plane].length;
-   v4l2_planes[plane].data_offset =
-   b->m.planes[plane].data_offset;
}
}
} else {
@@ -1180,10 +1178,8 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, 
const struct v4l2_buffer *b
 * In videobuf we use our internal V4l2_planes struct for
 * single-planar buffers as well, for simplicity.
 */
-   if (V4L2_TYPE_IS_OUTPUT(b->type)) {
+   if (V4L2_TYPE_IS_OUTPUT(b->type))
v4l2_planes[0].bytesused = b->bytesused;
-   v4l2_planes[0].data_offset = 0;
-   }
 
if (b->memory == V4L2_MEMORY_USERPTR) {
v4l2_planes[0].m.userptr = b->m.userptr;
@@ -1193,9 +1189,7 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, 
const struct v4l2_buffer *b
if (b->memory == V4L2_MEMORY_DMABUF) {
v4l2_planes[0].m.fd = b->m.fd;
v4l2_planes[0].length = b->length;
-   v4l2_planes[0].data_offset = 0;
}
-
}
 
/* Zero flags that the vb2 core handles */
@@ -1238,6 +1232,7 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const 
struct v4l2_buffer *b)
int write = !V4L2_TYPE_IS_OUTPUT(q->type);
bool reacquired = vb->planes[0].mem_priv == NULL;
 
+   memset(planes, 0, sizeof(planes[0]) * vb->num_planes);
/* Copy relevant information provided by the userspace */
__fill_vb2_buffer(vb, b, planes);
 
@@ -1357,6 +1352,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const 
struct v4l2_buffer *b)
int write = !V4L2_TYPE_IS_OUTPUT(q->type);
bool reacquired = vb->planes[0].mem_priv == NULL;
 
+   memset(planes, 0, sizeof(planes[0]) * vb->num_planes);
/* Copy relevant information provided by the userspace */
__fill_vb2_buffer(vb, b, planes);
 
@@ -1374,8 +1370,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const 
struct v4l2_buffer *b)
if (planes[plane].length == 0)
planes[plane].length = dbuf->size;
 
-   if (planes[plane].length < planes[plane].data_offset +
-   q->plane_sizes[plane]) {
+   if (planes[plane].length < q->plane_sizes[plane]) {
dprintk(1, "qbuf: invalid dmabuf length for plane %d\n",
plane);
ret = -EINVAL;
-- 
1.9.1

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


[REVIEWv3 PATCH 03/13] vb2: if bytesused is 0, then fill with output buffer length

2014-04-11 Thread Hans Verkuil
From: Hans Verkuil 

The application should really always fill in bytesused for output
buffers, unfortunately the vb2 framework never checked for that.

So for single planar formats replace a bytesused of 0 by the length
of the buffer, and for multiplanar format do the same if bytesused is
0 for ALL planes.

This seems to be what the user really intended if v4l2_buffer was
just memset to 0.

I'm afraid that just checking for this and returning an error would
break too many applications. Quite a few drivers never check for bytesused
at all and just use the buffer length instead.

Signed-off-by: Hans Verkuil 
Acked-by: Pawel Osciak 
Acked-by: Sakari Ailus 
---
 drivers/media/v4l2-core/videobuf2-core.c | 31 ++-
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
b/drivers/media/v4l2-core/videobuf2-core.c
index 596998e..b2582cb 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1143,15 +1143,30 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, 
const struct v4l2_buffer *b
if (V4L2_TYPE_IS_MULTIPLANAR(b->type)) {
/* Fill in driver-provided information for OUTPUT types */
if (V4L2_TYPE_IS_OUTPUT(b->type)) {
+   bool bytesused_is_used;
+
+   /* Check if bytesused == 0 for all planes */
+   for (plane = 0; plane < vb->num_planes; ++plane)
+   if (b->m.planes[plane].bytesused)
+   break;
+   bytesused_is_used = plane < vb->num_planes;
+
/*
 * Will have to go up to b->length when API starts
 * accepting variable number of planes.
+*
+* If bytesused_is_used is false, then fall back to the
+* full buffer size. In that case userspace clearly
+* never bothered to set it and it's a safe assumption
+* that they really meant to use the full plane sizes.
 */
for (plane = 0; plane < vb->num_planes; ++plane) {
-   v4l2_planes[plane].bytesused =
-   b->m.planes[plane].bytesused;
-   v4l2_planes[plane].data_offset =
-   b->m.planes[plane].data_offset;
+   struct v4l2_plane *pdst = &v4l2_planes[plane];
+   struct v4l2_plane *psrc = &b->m.planes[plane];
+
+   pdst->bytesused = bytesused_is_used ?
+   psrc->bytesused : psrc->length;
+   pdst->data_offset = psrc->data_offset;
}
}
 
@@ -1177,9 +1192,15 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, 
const struct v4l2_buffer *b
 * so fill in relevant v4l2_buffer struct fields instead.
 * In videobuf we use our internal V4l2_planes struct for
 * single-planar buffers as well, for simplicity.
+*
+* If bytesused == 0, then fall back to the full buffer size
+* as that's a sensible default.
 */
if (V4L2_TYPE_IS_OUTPUT(b->type))
-   v4l2_planes[0].bytesused = b->bytesused;
+   v4l2_planes[0].bytesused =
+   b->bytesused ? b->bytesused : b->length;
+   else
+   v4l2_planes[0].bytesused = 0;
 
if (b->memory == V4L2_MEMORY_USERPTR) {
v4l2_planes[0].m.userptr = b->m.userptr;
-- 
1.9.1

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


[REVIEWv3 PATCH 05/13] vb2: move __qbuf_mmap before __qbuf_userptr

2014-04-11 Thread Hans Verkuil
From: Hans Verkuil 

__qbuf_mmap was sort of hidden in between the much larger __qbuf_userptr
and __qbuf_dmabuf functions. Move it before __qbuf_userptr which is
also conform the usual order these memory models are implemented: first
mmap, then userptr, then dmabuf.

Signed-off-by: Hans Verkuil 
Acked-by: Pawel Osciak 
Acked-by: Sakari Ailus 
---
 drivers/media/v4l2-core/videobuf2-core.c | 28 ++--
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
b/drivers/media/v4l2-core/videobuf2-core.c
index 1421075..2e448a7 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1240,6 +1240,20 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, 
const struct v4l2_buffer *b
 }
 
 /**
+ * __qbuf_mmap() - handle qbuf of an MMAP buffer
+ */
+static int __qbuf_mmap(struct vb2_buffer *vb, const struct v4l2_buffer *b)
+{
+   int ret;
+
+   __fill_vb2_buffer(vb, b, vb->v4l2_planes);
+   ret = call_vb_qop(vb, buf_prepare, vb);
+   if (ret)
+   fail_vb_qop(vb, buf_prepare);
+   return ret;
+}
+
+/**
  * __qbuf_userptr() - handle qbuf of a USERPTR buffer
  */
 static int __qbuf_userptr(struct vb2_buffer *vb, const struct v4l2_buffer *b)
@@ -1346,20 +1360,6 @@ err:
 }
 
 /**
- * __qbuf_mmap() - handle qbuf of an MMAP buffer
- */
-static int __qbuf_mmap(struct vb2_buffer *vb, const struct v4l2_buffer *b)
-{
-   int ret;
-
-   __fill_vb2_buffer(vb, b, vb->v4l2_planes);
-   ret = call_vb_qop(vb, buf_prepare, vb);
-   if (ret)
-   fail_vb_qop(vb, buf_prepare);
-   return ret;
-}
-
-/**
  * __qbuf_dmabuf() - handle qbuf of a DMABUF buffer
  */
 static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
-- 
1.9.1

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


[REVIEWv3 PATCH 07/13] vb2: reject output buffers with V4L2_FIELD_ALTERNATE

2014-04-11 Thread Hans Verkuil
From: Hans Verkuil 

This is not allowed by the spec and does in fact not make any sense.
Return -EINVAL if this is the case.

Signed-off-by: Hans Verkuil 
Acked-by: Pawel Osciak 
Acked-by: Sakari Ailus 
---
 drivers/media/v4l2-core/videobuf2-core.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
b/drivers/media/v4l2-core/videobuf2-core.c
index 6e05495..f8c0247 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1511,6 +1511,19 @@ static int __buf_prepare(struct vb2_buffer *vb, const 
struct v4l2_buffer *b)
dprintk(1, "plane parameters verification failed: %d\n", ret);
return ret;
}
+   if (b->field == V4L2_FIELD_ALTERNATE && V4L2_TYPE_IS_OUTPUT(q->type)) {
+   /*
+* If the format's field is ALTERNATE, then the buffer's field
+* should be either TOP or BOTTOM, not ALTERNATE since that
+* makes no sense. The driver has to know whether the
+* buffer represents a top or a bottom field in order to
+* program any DMA correctly. Using ALTERNATE is wrong, since
+* that just says that it is either a top or a bottom field,
+* but not which of the two it is.
+*/
+   dprintk(1, "the field is incorrectly set to ALTERNATE for an 
output buffer\n");
+   return -EINVAL;
+   }
 
vb->state = VB2_BUF_STATE_PREPARING;
vb->v4l2_buf.timestamp.tv_sec = 0;
-- 
1.9.1

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


[REVIEWv3 PATCH 01/13] vb2: stop_streaming should return void

2014-04-11 Thread Hans Verkuil
From: Hans Verkuil 

The vb2 core ignores any return code from the stop_streaming op.
And there really isn't anything it can do anyway in case of an error.
So change the return type to void and update any drivers that implement it.

The int return gave drivers the idea that this operation could actually
fail, but that's really not the case.

The pwc amd sdr-msi3101 drivers both had this construction:

if (mutex_lock_interruptible(&s->v4l2_lock))
return -ERESTARTSYS;

This has been updated to just call mutex_lock(). The stop_streaming op
expects this to really stop streaming and I very much doubt this will
work reliably if stop_streaming just returns without really stopping the
DMA.

Signed-off-by: Hans Verkuil 
Acked-by: Pawel Osciak 
Acked-by: Sakari Ailus 
---
 Documentation/video4linux/v4l2-pci-skeleton.c| 3 +--
 drivers/media/pci/sta2x11/sta2x11_vip.c  | 3 +--
 drivers/media/platform/blackfin/bfin_capture.c   | 6 +-
 drivers/media/platform/coda.c| 4 +---
 drivers/media/platform/davinci/vpbe_display.c| 6 ++
 drivers/media/platform/davinci/vpif_capture.c| 6 ++
 drivers/media/platform/davinci/vpif_display.c| 6 ++
 drivers/media/platform/exynos-gsc/gsc-m2m.c  | 4 +---
 drivers/media/platform/exynos4-is/fimc-capture.c | 6 +++---
 drivers/media/platform/exynos4-is/fimc-lite.c| 6 +++---
 drivers/media/platform/exynos4-is/fimc-m2m.c | 3 +--
 drivers/media/platform/marvell-ccic/mcam-core.c  | 7 +++
 drivers/media/platform/mem2mem_testdev.c | 5 ++---
 drivers/media/platform/s3c-camif/camif-capture.c | 4 ++--
 drivers/media/platform/s5p-jpeg/jpeg-core.c  | 4 +---
 drivers/media/platform/s5p-mfc/s5p_mfc_dec.c | 3 +--
 drivers/media/platform/s5p-mfc/s5p_mfc_enc.c | 3 +--
 drivers/media/platform/s5p-tv/mixer_video.c  | 3 +--
 drivers/media/platform/soc_camera/atmel-isi.c| 6 ++
 drivers/media/platform/soc_camera/mx2_camera.c   | 4 +---
 drivers/media/platform/soc_camera/mx3_camera.c   | 4 +---
 drivers/media/platform/soc_camera/rcar_vin.c | 4 +---
 drivers/media/platform/soc_camera/sh_mobile_ceu_camera.c | 4 ++--
 drivers/media/platform/vivi.c| 3 +--
 drivers/media/platform/vsp1/vsp1_video.c | 4 +---
 drivers/media/usb/em28xx/em28xx-v4l.h| 2 +-
 drivers/media/usb/em28xx/em28xx-video.c  | 8 ++--
 drivers/media/usb/pwc/pwc-if.c   | 7 ++-
 drivers/media/usb/s2255/s2255drv.c   | 5 ++---
 drivers/media/usb/stk1160/stk1160-v4l.c  | 4 ++--
 drivers/media/usb/usbtv/usbtv-video.c| 9 +++--
 drivers/staging/media/davinci_vpfe/vpfe_video.c  | 5 ++---
 drivers/staging/media/dt3155v4l/dt3155v4l.c  | 3 +--
 drivers/staging/media/go7007/go7007-v4l2.c   | 3 +--
 drivers/staging/media/msi3101/sdr-msi3101.c  | 7 ++-
 drivers/staging/media/rtl2832u_sdr/rtl2832_sdr.c | 7 ++-
 drivers/staging/media/solo6x10/solo6x10-v4l2-enc.c   | 3 +--
 drivers/staging/media/solo6x10/solo6x10-v4l2.c   | 3 +--
 include/media/videobuf2-core.h   | 2 +-
 39 files changed, 61 insertions(+), 118 deletions(-)

diff --git a/Documentation/video4linux/v4l2-pci-skeleton.c 
b/Documentation/video4linux/v4l2-pci-skeleton.c
index 3a1c0d2..54eca2b 100644
--- a/Documentation/video4linux/v4l2-pci-skeleton.c
+++ b/Documentation/video4linux/v4l2-pci-skeleton.c
@@ -254,7 +254,7 @@ static int start_streaming(struct vb2_queue *vq, unsigned 
int count)
  * Stop the DMA engine. Any remaining buffers in the DMA queue are dequeued
  * and passed on to the vb2 framework marked as STATE_ERROR.
  */
-static int stop_streaming(struct vb2_queue *vq)
+static void stop_streaming(struct vb2_queue *vq)
 {
struct skeleton *skel = vb2_get_drv_priv(vq);
 
@@ -262,7 +262,6 @@ static int stop_streaming(struct vb2_queue *vq)
 
/* Release all active buffers */
return_all_buffers(skel, VB2_BUF_STATE_ERROR);
-   return 0;
 }
 
 /*
diff --git a/drivers/media/pci/sta2x11/sta2x11_vip.c 
b/drivers/media/pci/sta2x11/sta2x11_vip.c
index bb11443..7559951 100644
--- a/drivers/media/pci/sta2x11/sta2x11_vip.c
+++ b/drivers/media/pci/sta2x11/sta2x11_vip.c
@@ -357,7 +357,7 @@ static int start_streaming(struct vb2_queue *vq, unsigned 
int count)
 }
 
 /* abort streaming and wait for last buffer */
-static int stop_streaming(struct vb2_queue *vq)
+static void stop_streaming(struct vb2_queue *vq)
 {
struct sta2x11_vip *vip = vb2_get_drv_priv(vq);
struct vip_buffer *vip_buf, *node;
@@ -374,7 +374,6 @@ static int stop_streaming(struct vb2_queue *vq)
list_del(&vip_buf->list);
}
  

[REVIEWv3 PATCH 13/13] v4l2-pci-skeleton.c: fix alternate field handling.

2014-04-11 Thread Hans Verkuil
From: Hans Verkuil 

For interlaced HDTV timings the correct field setting is FIELD_ALTERNATE,
not INTERLACED. Update this template driver accordingly:

- add check for the invalid combination of read() and FIELD_ALTERNATE.
- in the interrupt handler set v4l2_buffer field to alternating TOP and
  BOTTOM.

Signed-off-by: Hans Verkuil 
---
 Documentation/video4linux/v4l2-pci-skeleton.c | 39 +++
 1 file changed, 28 insertions(+), 11 deletions(-)

diff --git a/Documentation/video4linux/v4l2-pci-skeleton.c 
b/Documentation/video4linux/v4l2-pci-skeleton.c
index 54eca2b..53dd346 100644
--- a/Documentation/video4linux/v4l2-pci-skeleton.c
+++ b/Documentation/video4linux/v4l2-pci-skeleton.c
@@ -77,7 +77,8 @@ struct skeleton {
 
spinlock_t qlock;
struct list_head buf_list;
-   unsigned int sequence;
+   unsigned field;
+   unsigned sequence;
 };
 
 struct skel_buffer {
@@ -124,7 +125,7 @@ static const struct v4l2_dv_timings_cap skel_timings_cap = {
  * Interrupt handler: typically interrupts happen after a new frame has been
  * captured. It is the job of the handler to remove the new frame from the
  * internal list and give it back to the vb2 framework, updating the sequence
- * counter and timestamp at the same time.
+ * counter, field and timestamp at the same time.
  */
 static irqreturn_t skeleton_irq(int irq, void *dev_id)
 {
@@ -139,8 +140,15 @@ static irqreturn_t skeleton_irq(int irq, void *dev_id)
spin_lock(&skel->qlock);
list_del(&new_buf->list);
spin_unlock(&skel->qlock);
-   new_buf->vb.v4l2_buf.sequence = skel->sequence++;
v4l2_get_timestamp(&new_buf->vb.v4l2_buf.timestamp);
+   new_buf->vb.v4l2_buf.sequence = skel->sequence++;
+   new_buf->vb.v4l2_buf.field = skel->field;
+   if (skel->format.field == V4L2_FIELD_ALTERNATE) {
+   if (skel->field == V4L2_FIELD_BOTTOM)
+   skel->field = V4L2_FIELD_TOP;
+   else if (skel->field == V4L2_FIELD_TOP)
+   skel->field = V4L2_FIELD_BOTTOM;
+   }
vb2_buffer_done(&new_buf->vb, VB2_BUF_STATE_DONE);
}
 #endif
@@ -160,6 +168,17 @@ static int queue_setup(struct vb2_queue *vq, const struct 
v4l2_format *fmt,
 {
struct skeleton *skel = vb2_get_drv_priv(vq);
 
+   skel->field = skel->format.field;
+   if (skel->field == V4L2_FIELD_ALTERNATE) {
+   /*
+* You cannot use read() with FIELD_ALTERNATE since the field
+* information (TOP/BOTTOM) cannot be passed back to the user.
+*/
+   if (vb2_fileio_is_active(q))
+   return -EINVAL;
+   skel->field = V4L2_FIELD_TOP;
+   }
+
if (vq->num_buffers + *nbuffers < 3)
*nbuffers = 3 - vq->num_buffers;
 
@@ -173,10 +192,7 @@ static int queue_setup(struct vb2_queue *vq, const struct 
v4l2_format *fmt,
 
 /*
  * Prepare the buffer for queueing to the DMA engine: check and set the
- * payload size and fill in the field. Note: if the format's field is
- * V4L2_FIELD_ALTERNATE, then vb->v4l2_buf.field should be set in the
- * interrupt handler since that's usually where you know if the TOP or
- * BOTTOM field has been captured.
+ * payload size.
  */
 static int buffer_prepare(struct vb2_buffer *vb)
 {
@@ -190,7 +206,6 @@ static int buffer_prepare(struct vb2_buffer *vb)
}
 
vb2_set_plane_payload(vb, 0, size);
-   vb->v4l2_buf.field = skel->format.field;
return 0;
 }
 
@@ -318,10 +333,12 @@ static void skeleton_fill_pix_format(struct skeleton 
*skel,
/* HDMI input */
pix->width = skel->timings.bt.width;
pix->height = skel->timings.bt.height;
-   if (skel->timings.bt.interlaced)
-   pix->field = V4L2_FIELD_INTERLACED;
-   else
+   if (skel->timings.bt.interlaced) {
+   pix->field = V4L2_FIELD_ALTERNATE;
+   pix->height /= 2;
+   } else {
pix->field = V4L2_FIELD_NONE;
+   }
pix->colorspace = V4L2_COLORSPACE_REC709;
}
 
-- 
1.9.1

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


[REVIEWv3 PATCH 04/13] vb2: use correct prefix

2014-04-11 Thread Hans Verkuil
From: Hans Verkuil 

Many dprintk's in vb2 use a hardcoded prefix with the function name. In
many cases that is now outdated. To keep things consistent the dprintk
macro has been changed to print the function name in addition to the "vb2:"
prefix. Superfluous prefixes elsewhere in the code have been removed.

Signed-off-by: Hans Verkuil 
Acked-by: Pawel Osciak 
Acked-by: Sakari Ailus 
---
 drivers/media/v4l2-core/videobuf2-core.c | 133 +++
 1 file changed, 65 insertions(+), 68 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
b/drivers/media/v4l2-core/videobuf2-core.c
index b2582cb..1421075 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -27,10 +27,10 @@
 static int debug;
 module_param(debug, int, 0644);
 
-#define dprintk(level, fmt, arg...)\
-   do {\
-   if (debug >= level) \
-   printk(KERN_DEBUG "vb2: " fmt, ## arg); \
+#define dprintk(level, fmt, arg...)  \
+   do {  \
+   if (debug >= level)   \
+   pr_debug("vb2: %s: " fmt, __func__, ## arg); \
} while (0)
 
 #ifdef CONFIG_VIDEO_ADV_DEBUG
@@ -371,7 +371,7 @@ static int __vb2_queue_free(struct vb2_queue *q, unsigned 
int buffers)
if (q->bufs[buffer] == NULL)
continue;
if (q->bufs[buffer]->state == VB2_BUF_STATE_PREPARING) {
-   dprintk(1, "reqbufs: preparing buffers, cannot free\n");
+   dprintk(1, "preparing buffers, cannot free\n");
return -EAGAIN;
}
}
@@ -656,12 +656,12 @@ int vb2_querybuf(struct vb2_queue *q, struct v4l2_buffer 
*b)
int ret;
 
if (b->type != q->type) {
-   dprintk(1, "querybuf: wrong buffer type\n");
+   dprintk(1, "wrong buffer type\n");
return -EINVAL;
}
 
if (b->index >= q->num_buffers) {
-   dprintk(1, "querybuf: buffer index out of range\n");
+   dprintk(1, "buffer index out of range\n");
return -EINVAL;
}
vb = q->bufs[b->index];
@@ -721,12 +721,12 @@ static int __verify_memory_type(struct vb2_queue *q,
 {
if (memory != V4L2_MEMORY_MMAP && memory != V4L2_MEMORY_USERPTR &&
memory != V4L2_MEMORY_DMABUF) {
-   dprintk(1, "reqbufs: unsupported memory type\n");
+   dprintk(1, "unsupported memory type\n");
return -EINVAL;
}
 
if (type != q->type) {
-   dprintk(1, "reqbufs: requested type is incorrect\n");
+   dprintk(1, "requested type is incorrect\n");
return -EINVAL;
}
 
@@ -735,17 +735,17 @@ static int __verify_memory_type(struct vb2_queue *q,
 * are available.
 */
if (memory == V4L2_MEMORY_MMAP && __verify_mmap_ops(q)) {
-   dprintk(1, "reqbufs: MMAP for current setup unsupported\n");
+   dprintk(1, "MMAP for current setup unsupported\n");
return -EINVAL;
}
 
if (memory == V4L2_MEMORY_USERPTR && __verify_userptr_ops(q)) {
-   dprintk(1, "reqbufs: USERPTR for current setup unsupported\n");
+   dprintk(1, "USERPTR for current setup unsupported\n");
return -EINVAL;
}
 
if (memory == V4L2_MEMORY_DMABUF && __verify_dmabuf_ops(q)) {
-   dprintk(1, "reqbufs: DMABUF for current setup unsupported\n");
+   dprintk(1, "DMABUF for current setup unsupported\n");
return -EINVAL;
}
 
@@ -755,7 +755,7 @@ static int __verify_memory_type(struct vb2_queue *q,
 * do the memory and type validation.
 */
if (q->fileio) {
-   dprintk(1, "reqbufs: file io in progress\n");
+   dprintk(1, "file io in progress\n");
return -EBUSY;
}
return 0;
@@ -790,7 +790,7 @@ static int __reqbufs(struct vb2_queue *q, struct 
v4l2_requestbuffers *req)
int ret;
 
if (q->streaming) {
-   dprintk(1, "reqbufs: streaming active\n");
+   dprintk(1, "streaming active\n");
return -EBUSY;
}
 
@@ -800,7 +800,7 @@ static int __reqbufs(struct vb2_queue *q, struct 
v4l2_requestbuffers *req)
 * are not in use and can be freed.
 */
if (q->memory == V4L2_MEMORY_MMAP && __buffers_in_use(q)) {
-   dprintk(1, "reqbufs: memory in use, cannot free\n");
+   dprintk(1, "memory in use, cannot free\n");
return -EBU

[REVIEWv3 PATCH 00/13] vb2: various small fixes/improvements

2014-04-11 Thread Hans Verkuil
This is the third version of this review patch series.
The previous can be found here:

http://www.spinics.net/lists/linux-media/msg75428.html

Changes since v2:

- Updated v4l2-pci-skeleton.c as well in patch 01/13
- Dropped patch 10/13 as it is not needed
- Added comment to patch 06/13 as suggested by Pawel
- Added patch 13/13: fix HDTV interlaced handling in v4l2-pci-skeleton.c

If there are no more comments, then I plan on posting a pull request
on Monday.

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: List objectives and interests.

2014-04-11 Thread Hans Verkuil
Hi Steve,

On 04/11/2014 09:43 AM, Steve Cookson - IT wrote:
> Hi People,
> 
> Could I, please, I clarify what the purposes of this list are?
> 
> I am developing a system to collect medical raw video based on Linux.  I 
> have tested a number of SD and HD adaptors for this project some of 
> which have worked to a greater or lesser extent.
> 
> Is this the right place for peer-based support for video capture?
> 
> I know that SD is a bit old and maybe there is no longer much demand for 
> it, in which case I could move to an HD adaptor, but there also doesn't 
> seem to be much out there on Linux to capture raw HD either.
> 
> So is this list really about Linux TV rather than video capture more 
> generally?
> 
> Is there a more appropriate list to ask questions about raw video capture?

No, this is the right list for that. The linux-media mailinglist is for
developers and users of anything under drivers/media in the kernel tree.

With regards to HD: while HD is well supported for embedded systems, there
are really no V4L2 drivers for consumer HDTV capture boards. The closest is
an out-of-tree driver for a DVI capture board (Osprey 820e), but there is
nothing for HDMI capture.

Regards,

Hans

> Any help or pointers very gratefully received.
> 
> Regards
> 
> Steve.
> --
> 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


List objectives and interests.

2014-04-11 Thread Steve Cookson - IT

Hi People,

Could I, please, I clarify what the purposes of this list are?

I am developing a system to collect medical raw video based on Linux.  I 
have tested a number of SD and HD adaptors for this project some of 
which have worked to a greater or lesser extent.


Is this the right place for peer-based support for video capture?

I know that SD is a bit old and maybe there is no longer much demand for 
it, in which case I could move to an HD adaptor, but there also doesn't 
seem to be much out there on Linux to capture raw HD either.


So is this list really about Linux TV rather than video capture more 
generally?


Is there a more appropriate list to ask questions about raw video capture?

Any help or pointers very gratefully received.

Regards

Steve.
--
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: [REVIEW PATCH 10/11] vb2: set v4l2_buffer.bytesused to 0 for mp buffers

2014-04-11 Thread Hans Verkuil
On 04/09/2014 07:21 PM, Sakari Ailus wrote:
> Hi Hans,
> 
> Thanks for the set.
> 
> On Mon, Mar 10, 2014 at 10:20:57PM +0100, Hans Verkuil wrote:
>> From: Hans Verkuil 
>>
>> The bytesused field of struct v4l2_buffer is not used for multiplanar
>> formats, so just zero it to prevent it from having some random value.
>>
>> Signed-off-by: Hans Verkuil 
>> ---
>>  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 f68a60f..54a4150 100644
>> --- a/drivers/media/v4l2-core/videobuf2-core.c
>> +++ b/drivers/media/v4l2-core/videobuf2-core.c
>> @@ -583,6 +583,7 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb, 
>> struct v4l2_buffer *b)
>>   * for it. The caller has already verified memory and size.
>>   */
>>  b->length = vb->num_planes;
>> +b->bytesused = 0;
> 
> I wonder if I'm missing something, but doesn't the value of the field come
> from the v4l2_buf field of the vb2_buffer which is allocated using kzalloc()
> in __vb2_queue_alloc(), and never changed afterwards?

You are right, this isn't necessary. I've dropped this patch.

Thanks!

Hans

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


[PATCH] [media] Prefer gspca_sonixb over sn9c102 for all devices

2014-04-11 Thread Jean Delvare
The sn9c102 driver is deprecated. It was moved to staging in
anticipation of its removal in a future kernel version. However, USB
devices 0C45:6024 and 0C45:6025 are still handled by sn9c102 when
both sn9c102 and gspca_sonixb are enabled.

We must migrate all the users of these devices to the gspca_sonixb
driver now, so that it gets sufficient testing before the sn9c102
driver is finally phased out.

Signed-off-by: Jean Delvare 
Cc: Hans de Goede 
Cc: Mauro Carvalho Chehab 
Cc: Luca Risolia 
Cc: Greg Kroah-Hartman 
---
I consider this a bug fix, I believe it should go upstream ASAP.

 drivers/media/usb/gspca/sonixb.c |2 --
 drivers/staging/media/sn9c102/sn9c102_devtable.h |2 --
 2 files changed, 4 deletions(-)

--- linux-3.15-rc0.orig/drivers/media/usb/gspca/sonixb.c2014-04-11 
08:57:26.932408285 +0200
+++ linux-3.15-rc0/drivers/media/usb/gspca/sonixb.c 2014-04-11 
09:02:32.151943578 +0200
@@ -1430,10 +1430,8 @@ static const struct usb_device_id device
{USB_DEVICE(0x0c45, 0x600d), SB(PAS106, 101)},
{USB_DEVICE(0x0c45, 0x6011), SB(OV6650, 101)},
{USB_DEVICE(0x0c45, 0x6019), SB(OV7630, 101)},
-#if !IS_ENABLED(CONFIG_USB_SN9C102)
{USB_DEVICE(0x0c45, 0x6024), SB(TAS5130CXX, 102)},
{USB_DEVICE(0x0c45, 0x6025), SB(TAS5130CXX, 102)},
-#endif
{USB_DEVICE(0x0c45, 0x6027), SB(OV7630, 101)}, /* Genius Eye 310 */
{USB_DEVICE(0x0c45, 0x6028), SB(PAS202, 102)},
{USB_DEVICE(0x0c45, 0x6029), SB(PAS106, 102)},
--- linux-3.15-rc0.orig/drivers/staging/media/sn9c102/sn9c102_devtable.h
2014-04-11 08:57:26.932408285 +0200
+++ linux-3.15-rc0/drivers/staging/media/sn9c102/sn9c102_devtable.h 
2014-04-11 09:02:32.151943578 +0200
@@ -48,10 +48,8 @@ static const struct usb_device_id sn9c10
{ SN9C102_USB_DEVICE(0x0c45, 0x600d, BRIDGE_SN9C102), },
 /* { SN9C102_USB_DEVICE(0x0c45, 0x6011, BRIDGE_SN9C102), }, OV6650 */
{ SN9C102_USB_DEVICE(0x0c45, 0x6019, BRIDGE_SN9C102), },
-#endif
{ SN9C102_USB_DEVICE(0x0c45, 0x6024, BRIDGE_SN9C102), },
{ SN9C102_USB_DEVICE(0x0c45, 0x6025, BRIDGE_SN9C102), },
-#if !defined CONFIG_USB_GSPCA_SONIXB && !defined CONFIG_USB_GSPCA_SONIXB_MODULE
{ SN9C102_USB_DEVICE(0x0c45, 0x6028, BRIDGE_SN9C102), },
{ SN9C102_USB_DEVICE(0x0c45, 0x6029, BRIDGE_SN9C102), },
{ SN9C102_USB_DEVICE(0x0c45, 0x602a, BRIDGE_SN9C102), },


-- 
Jean Delvare
SUSE L3 Support
--
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] Helper to abstract vma handling in media layer

2014-04-11 Thread Hans Verkuil
On 04/11/2014 12:18 AM, Jan Kara wrote:
> On Thu 10-04-14 23:57:38, Jan Kara wrote:
>> On Thu 10-04-14 14:22:20, Hans Verkuil wrote:
>>> On 04/10/14 14:15, Jan Kara wrote:
 On Thu 10-04-14 13:07:42, Hans Verkuil wrote:
> On 04/10/14 12:32, Jan Kara wrote:
>>   Hello,
>>
>> On Thu 10-04-14 12:02:50, Marek Szyprowski wrote:
>>> On 2014-03-17 20:49, Jan Kara wrote:
   The following patch series is my first stab at abstracting vma 
 handling
>>> >from the various media drivers. After this patch set drivers have to 
>>> >know
 much less details about vmas, their types, and locking. My motivation 
 for
 the series is that I want to change get_user_pages() locking and I want
 to handle subtle locking details in as few places as possible.

 The core of the series is the new helper get_vaddr_pfns() which is 
 given a
 virtual address and it fills in PFNs into provided array. If PFNs 
 correspond to
 normal pages it also grabs references to these pages. The difference 
 from
 get_user_pages() is that this function can also deal with pfnmap, 
 mixed, and io
 mappings which is what the media drivers need.

 The patches are just compile tested (since I don't have any of the 
 hardware
 I'm afraid I won't be able to do any more testing anyway) so please 
 handle
 with care. I'm grateful for any comments.
>>>
>>> Thanks for posting this series! I will check if it works with our
>>> hardware soon.  This is something I wanted to introduce some time ago to
>>> simplify buffer handling in dma-buf, but I had no time to start working.
>>   Thanks for having a look in the series.
>>
>>> However I would like to go even further with integration of your pfn
>>> vector idea.  This structure looks like a best solution for a compact
>>> representation of the memory buffer, which should be considered by the
>>> hardware as contiguous (either contiguous in physical memory or mapped
>>> contiguously into dma address space by the respective iommu). As you
>>> already noticed it is widely used by graphics and video drivers.
>>>
>>> I would also like to add support for pfn vector directly to the
>>> dma-mapping subsystem. This can be done quite easily (even with a
>>> fallback for architectures which don't provide method for it). I will 
>>> try
>>> to prepare rfc soon.  This will finally remove the need for hacks in
>>> media/v4l2-core/videobuf2-dma-contig.c
>>   That would be a worthwhile thing to do. When I was reading the code 
>> this
>> seemed like something which could be done but I delibrately avoided doing
>> more unification than necessary for my purposes as I don't have any
>> hardware to test and don't know all the subtleties in the code... BTW, is
>> there some way to test the drivers without the physical video HW?
>
> You can use the vivi driver (drivers/media/platform/vivi) for this.
> However, while the vivi driver can import dma buffers it cannot export
> them. If you want that, then you have to use this tree:
>
> http://git.linuxtv.org/cgit.cgi/hverkuil/media_tree.git/log/?h=vb2-part4
   Thanks for the pointer that looks good. I've also found
 drivers/media/platform/mem2mem_testdev.c which seems to do even more
 testing of the area I made changes to. So now I have to find some userspace
 tool which can issue proper ioctls to setup and use the buffers and I can
 start testing what I wrote :)
>>>
>>> Get the v4l-utils.git repository 
>>> (http://git.linuxtv.org/cgit.cgi/v4l-utils.git/).
>>> You want the v4l2-ctl tool. Don't use the version supplied by your distro,
>>> that's often too old.
>>>
>>> 'v4l2-ctl --help-streaming' gives the available options for doing streaming.
>>>
>>> So simple capturing from vivi is 'v4l2-ctl --stream-mmap' or 
>>> '--stream-user'.
>>> You can't test dmabuf unless you switch to the vb2-part4 branch of my tree.
>>   Great, it seems to be doing something and it shows there's some bug in my
>> code. Thanks a lot for help.
>   OK, so after a small fix the basic functionality seems to be working. It
> doesn't seem there's a way to test multiplanar buffers with vivi, is there?

For that you need to switch to the vb2-part4 branch as well. That has support
for multiplanar.

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