cron job: media_tree daily build: OK

[Hans Verkuil]

This message is generated daily by a cron job that builds media_tree for
the kernels and architectures in the list below.

Results of the daily build of media_tree:

date:   Tue Mar 15 04:00:22 CET 2016
git branch: test
git hash:   da470473c9cf9c4ebb40d046b306c76427b6df94
gcc version:i686-linux-gcc (GCC) 5.3.0
sparse version: v0.5.0-51-ga53cea2
smatch version: v0.5.0-3228-g5cf65ab
host hardware:  x86_64
host os:4.4.0-164

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-bf561: 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.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.23-i686: OK
linux-3.13.11-i686: OK
linux-3.14.9-i686: OK
linux-3.15.2-i686: OK
linux-3.16.7-i686: OK
linux-3.17.8-i686: OK
linux-3.18.7-i686: OK
linux-3.19-i686: OK
linux-4.0-i686: OK
linux-4.1.1-i686: OK
linux-4.2-i686: OK
linux-4.3-i686: OK
linux-4.4-i686: OK
linux-4.5-i686: 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.23-x86_64: OK
linux-3.13.11-x86_64: OK
linux-3.14.9-x86_64: OK
linux-3.15.2-x86_64: OK
linux-3.16.7-x86_64: OK
linux-3.17.8-x86_64: OK
linux-3.18.7-x86_64: OK
linux-3.19-x86_64: OK
linux-4.0-x86_64: OK
linux-4.1.1-x86_64: OK
linux-4.2-x86_64: OK
linux-4.3-x86_64: OK
linux-4.4-x86_64: OK
linux-4.5-x86_64: OK
apps: OK
spec-git: OK
sparse: WARNINGS
smatch: ERRORS

Detailed results are available here:

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

Full logs are available here:

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

The 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

Re: [PATCH v2] [media] media-device: Don't call notify callbacks holding a spinlock

[Shuah Khan]

On 03/14/2016 01:19 PM, Mauro Carvalho Chehab wrote:
> The notify routines may sleep. So, they can't be called in spinlock
> context. Also, they may want to call other media routines that might
> be spinning the spinlock, like creating a link.
> 
> This fixes the following bug:
> 
> [ 1839.510587] BUG: sleeping function called from invalid context at 
> mm/slub.c:1289
> [ 1839.510881] in_atomic(): 1, irqs_disabled(): 0, pid: 3479, name: modprobe
> [ 1839.511157] 4 locks held by modprobe/3479:
> [ 1839.511415]  #0:  (>mutex){..}, at: [] 
> __driver_attach+0xa3/0x160
> [ 1839.512381]  #1:  (>mutex){..}, at: [] 
> __driver_attach+0xb1/0x160
> [ 1839.512388]  #2:  (register_mutex#5){+.+.+.}, at: [] 
> usb_audio_probe+0x257/0x1c90 [snd_usb_audio]
> [ 1839.512401]  #3:  (&(>lock)->rlock){+.+.+.}, at: 
> [] media_device_register_entity+0x1cb/0x700 [media]
> [ 1839.512412] CPU: 2 PID: 3479 Comm: modprobe Not tainted 4.5.0-rc3+ #49
> [ 1839.512415] Hardware name:  /NUC5i7RYB, BIOS 
> RYBDWi35.86A.0350.2015.0812.1722 08/12/2015
> [ 1839.512417]   8803b3f6f288 81933901 
> 8803c4bae000
> [ 1839.512422]  8803c4bae5c8 8803b3f6f2b0 811c6af5 
> 8803c4bae000
> [ 1839.512427]  8285d7f6 0509 8803b3f6f2f0 
> 811c6ce5
> [ 1839.512432] Call Trace:
> [ 1839.512436]  [] dump_stack+0x85/0xc4
> [ 1839.512440]  [] ___might_sleep+0x245/0x3a0
> [ 1839.512443]  [] __might_sleep+0x95/0x1a0
> [ 1839.512446]  [] kmem_cache_alloc_trace+0x20e/0x300
> [ 1839.512451]  [] ? media_add_link+0x4d/0x140 [media]
> [ 1839.512455]  [] media_add_link+0x4d/0x140 [media]
> [ 1839.512459]  [] media_create_pad_link+0xa1/0x600 [media]
> [ 1839.512463]  [] au0828_media_graph_notify+0x173/0x360 
> [au0828]
> [ 1839.512467]  [] ? media_gobj_create+0x1ba/0x480 [media]
> [ 1839.512471]  [] media_device_register_entity+0x3ab/0x700 
> [media]
> 
> Tested with an HVR-950Q, under the following testcases:
> 
> 1) load au0828 driver first, and then snd-usb-audio;
> 2) load snd-usb-audio driver first, and then au0828;
> 3) loading both drivers, and then connecting the device.
> 
> Signed-off-by: Mauro Carvalho Chehab 

Looks good. Tested. Inserting device with au0828 blacklisted. Just the audio 
graph
gets created and then modprobed au0828. The complete graph looks good. Also 
tested
without blacklisting, so both drivers are probed together. Graph looks good.

Tested-by: Shuah Khan 

thanks,
-- Shuah
> ---
> 
> Please disconsider verison 1. It got amended with an experimental patch.
> 
>  drivers/media/media-device.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index 6ba6e8f982fc..fc3c199e5500 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -587,14 +587,15 @@ int __must_check media_device_register_entity(struct 
> media_device *mdev,
>   media_gobj_create(mdev, MEDIA_GRAPH_PAD,
>  >pads[i].graph_obj);
>  
> + spin_unlock(>lock);
> +
>   /* invoke entity_notify callbacks */
>   list_for_each_entry_safe(notify, next, >entity_notify, list) {
>   (notify)->notify(entity, notify->notify_data);
>   }
>  
> - spin_unlock(>lock);
> -
>   mutex_lock(>graph_mutex);
> +
>   if (mdev->entity_internal_idx_max
>   >= mdev->pm_count_walk.ent_enum.idx_max) {
>   struct media_entity_graph new = { .top = 0 };
> 


-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shua...@osg.samsung.com | (970) 217-8978
--
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 v4 0/2] media: soc_camera: rcar_vin: add fallback and r8a7792 bindings

[Simon Horman]

Hi,

this short series adds add fallback and r8a7792 bindings to rcar_vin.

Based on media-tree/master

Changes since v3:
* Add Acks
* Correct typo in changelog

Simon Horman (1):
  media: soc_camera: rcar_vin: add device tree support for r8a7792

Yoshihiro Kaneko (1):
  media: soc_camera: rcar_vin: add R-Car Gen 2 and 3 fallback
compatibility strings

 Documentation/devicetree/bindings/media/rcar_vin.txt | 12 ++--
 drivers/media/platform/soc_camera/rcar_vin.c |  2 ++
 2 files changed, 12 insertions(+), 2 deletions(-)

-- 
2.1.4

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

[PATCH v4 2/2] media: soc_camera: rcar_vin: add device tree support for r8a7792

[Simon Horman]

Simply document new compatibility string.
As a previous patch adds a generic R-Car Gen2 compatibility string
there appears to be no need for a driver updates.

By documenting this compat string it may be used in DTSs shipped, for
example as part of ROMs. It must be used in conjunction with the Gen2
fallback compat string. At this time there are no known differences between
the r8a7792 IP block and that implemented by the driver for the Gen2
fallback compat string. Thus there is no need to update the driver as the
use of the Gen2 fallback compat string will activate the correct code in
the current driver while leaving the option for r8a7792-specific driver
code to be activated in an updated driver should the need arise.

Signed-off-by: Simon Horman 
Acked-by: Geert Uytterhoeven 
---
v4
* s/sting/string/ in changelog
* Added Ack from Geert Uytterhoeven

v3
* New patch
---
 Documentation/devicetree/bindings/media/rcar_vin.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt 
b/Documentation/devicetree/bindings/media/rcar_vin.txt
index 4266123888ed..6a4e61cbe011 100644
--- a/Documentation/devicetree/bindings/media/rcar_vin.txt
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
@@ -9,6 +9,7 @@ channel which can be either RGB, YUYV or BT656.
- "renesas,vin-r8a7795" for the R8A7795 device
- "renesas,vin-r8a7794" for the R8A7794 device
- "renesas,vin-r8a7793" for the R8A7793 device
+   - "renesas,vin-r8a7792" for the R8A7792 device
- "renesas,vin-r8a7791" for the R8A7791 device
- "renesas,vin-r8a7790" for the R8A7790 device
- "renesas,vin-r8a7779" for the R8A7779 device
-- 
2.1.4

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

[PATCH v4 1/2] media: soc_camera: rcar_vin: add R-Car Gen 2 and 3 fallback compatibility strings

[Simon Horman]

From: Yoshihiro Kaneko 

Add fallback compatibility string for R-Car Gen 1 and 2.

In the case of Renesas R-Car hardware we know that there are generations of
SoCs, e.g. Gen 2 and 3. But beyond that its not clear what the relationship
between IP blocks might be. For example, I believe that r8a7790 is older
than r8a7791 but that doesn't imply that the latter is a descendant of the
former or vice versa.

We can, however, by examining the documentation and behaviour of the
hardware at run-time observe that the current driver implementation appears
to be compatible with the IP blocks on SoCs within a given generation.

For the above reasons and convenience when enabling new SoCs a
per-generation fallback compatibility string scheme being adopted for
drivers for Renesas SoCs.

Signed-off-by: Yoshihiro Kaneko 
Signed-off-by: Simon Horman 
Acked-by: Geert Uytterhoeven 
---
v4 [Simon Horman]
* Added Ack from Geert Uytterhoeven

v3 [Simon Horman]
* Reworked and expanded changelog.
* Minor documentation enhancements.

v2 [Yoshihiro Kaneko]
* As suggested by Geert Uytterhoeven
  drivers/media/platform/soc_camera/rcar_vin.c:
- The generic compatibility values are listed at the end of the
  rcar_vin_of_table[].

v1 [Yoshihiro Kaneko]
---
 Documentation/devicetree/bindings/media/rcar_vin.txt | 11 +--
 drivers/media/platform/soc_camera/rcar_vin.c |  2 ++
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt 
b/Documentation/devicetree/bindings/media/rcar_vin.txt
index 619193ccf7ff..4266123888ed 100644
--- a/Documentation/devicetree/bindings/media/rcar_vin.txt
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
@@ -5,7 +5,7 @@ The rcar_vin device provides video input capabilities for the 
Renesas R-Car
 family of devices. The current blocks are always slaves and suppot one input
 channel which can be either RGB, YUYV or BT656.
 
- - compatible: Must be one of the following
+ - compatible: Must be one or more of the following
- "renesas,vin-r8a7795" for the R8A7795 device
- "renesas,vin-r8a7794" for the R8A7794 device
- "renesas,vin-r8a7793" for the R8A7793 device
@@ -13,6 +13,13 @@ channel which can be either RGB, YUYV or BT656.
- "renesas,vin-r8a7790" for the R8A7790 device
- "renesas,vin-r8a7779" for the R8A7779 device
- "renesas,vin-r8a7778" for the R8A7778 device
+   - "renesas,rcar-gen2-vin" for a generic R-Car Gen2 compatible device.
+   - "renesas,rcar-gen3-vin" for a generic R-Car Gen3 compatible device.
+
+   When compatible with the generic version nodes must list the
+   SoC-specific version corresponding to the platform first
+   followed by the generic version.
+
  - reg: the register base and size for the device registers
  - interrupts: the interrupt for the device
  - clocks: Reference to the parent clock
@@ -37,7 +44,7 @@ Device node example
};
 
 vin0: vin@0xe6ef {
-compatible = "renesas,vin-r8a7790";
+compatible = "renesas,vin-r8a7790", "renesas,rcar-gen2-vin";
 clocks = <_clks R8A7790_CLK_VIN0>;
 reg = <0 0xe6ef 0 0x1000>;
 interrupts = <0 188 IRQ_TYPE_LEVEL_HIGH>;
diff --git a/drivers/media/platform/soc_camera/rcar_vin.c 
b/drivers/media/platform/soc_camera/rcar_vin.c
index 3b8edf458964..3f9c1b8456c3 100644
--- a/drivers/media/platform/soc_camera/rcar_vin.c
+++ b/drivers/media/platform/soc_camera/rcar_vin.c
@@ -1845,6 +1845,8 @@ static const struct of_device_id rcar_vin_of_table[] = {
{ .compatible = "renesas,vin-r8a7790", .data = (void *)RCAR_GEN2 },
{ .compatible = "renesas,vin-r8a7779", .data = (void *)RCAR_H1 },
{ .compatible = "renesas,vin-r8a7778", .data = (void *)RCAR_M1 },
+   { .compatible = "renesas,rcar-gen3-vin", .data = (void *)RCAR_GEN3 },
+   { .compatible = "renesas,rcar-gen2-vin", .data = (void *)RCAR_GEN2 },
{ },
 };
 MODULE_DEVICE_TABLE(of, rcar_vin_of_table);
-- 
2.1.4

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

[PATCH 3/3] [media] v4l2-mc: remove unused dtv_demod variable

[Arnd Bergmann]

A recent patch removed the only user of the 'dtv_demod' variable
in v4l2_mc_create_media_graph, but did not remove the declaration,
possibly as a result of an incorrect rebase:

drivers/media/v4l2-core/v4l2-mc.c: In function 'v4l2_mc_create_media_graph':
drivers/media/v4l2-core/v4l2-mc.c:37:55: error: unused variable 'dtv_demod' 
[-Werror=unused-variable]

This removes the unused variable as well.

Signed-off-by: Arnd Bergmann 
Fixes: 840f5b0572ea ("media: au0828 disable tuner to demod link in 
au0828_media_device_register()")
---
 drivers/media/v4l2-core/v4l2-mc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/v4l2-core/v4l2-mc.c 
b/drivers/media/v4l2-core/v4l2-mc.c
index 2a7b79bc90fd..2228cd3a846e 100644
--- a/drivers/media/v4l2-core/v4l2-mc.c
+++ b/drivers/media/v4l2-core/v4l2-mc.c
@@ -34,7 +34,7 @@ int v4l2_mc_create_media_graph(struct media_device *mdev)
 {
struct media_entity *entity;
struct media_entity *if_vid = NULL, *if_aud = NULL;
-   struct media_entity *tuner = NULL, *decoder = NULL, *dtv_demod = NULL;
+   struct media_entity *tuner = NULL, *decoder = NULL;
struct media_entity *io_v4l = NULL, *io_vbi = NULL, *io_swradio = NULL;
bool is_webcam = false;
u32 flags;
-- 
2.7.0

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

[PATCH 2/3] [media] am437x-vfpe: fix typo in vpfe_get_app_input_index

[Arnd Bergmann]

gcc-6 points out an obviously silly comparison in vpfe_get_app_input_index():

drivers/media/platform/am437x/am437x-vpfe.c: In function 
'vpfe_get_app_input_index':
drivers/media/platform/am437x/am437x-vpfe.c:1709:27: warning: self-comparison 
always evaluats to true [-Wtautological-compare]
   client->adapter->nr == client->adapter->nr) {
   ^~

This was introduced in a slighly incorrect conversion, and it's
clear that the comparison was meant to compare the iterator
to the current subdev instead, as we do in the line above.

Signed-off-by: Arnd Bergmann 
Fixes: d37232390fd4 ("[media] media: am437x-vpfe: match the OF node/i2c addr 
instead of name")
---
 drivers/media/platform/am437x/am437x-vpfe.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/am437x/am437x-vpfe.c 
b/drivers/media/platform/am437x/am437x-vpfe.c
index de32e3a3d4d1..e6a7bff4650c 100644
--- a/drivers/media/platform/am437x/am437x-vpfe.c
+++ b/drivers/media/platform/am437x/am437x-vpfe.c
@@ -1706,7 +1706,7 @@ static int vpfe_get_app_input_index(struct vpfe_device 
*vpfe,
sdinfo = >sub_devs[i];
client = v4l2_get_subdevdata(sdinfo->sd);
if (client->addr == curr_client->addr &&
-   client->adapter->nr == client->adapter->nr) {
+   client->adapter->nr == curr_client->adapter->nr) {
if (vpfe->current_input >= 1)
return -1;
*app_input_index = j + vpfe->current_input;
-- 
2.7.0

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

[PATCH 1/3] [media] cobalt: add MTD dependency

[Arnd Bergmann]

The cobalt driver fails to link when it is built-in and MTD is disabled or a
loadable module:

drivers/media/built-in.o: In function `cobalt_flash_probe':
:(.text+0xb8b46): undefined reference to `mtd_device_parse_register'
:(.text+0xb8b88): undefined reference to `do_map_probe'
drivers/media/built-in.o: In function `cobalt_flash_remove':
:(.text+0xb8bb4): undefined reference to `mtd_device_unregister'
:(.text+0xb8bbe): undefined reference to `map_destroy'

This adds a Kconfig dependency to ensure we can call the API.

Signed-off-by: Arnd Bergmann 
---
 drivers/media/pci/cobalt/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/pci/cobalt/Kconfig b/drivers/media/pci/cobalt/Kconfig
index a01f0cc745cc..70343829a125 100644
--- a/drivers/media/pci/cobalt/Kconfig
+++ b/drivers/media/pci/cobalt/Kconfig
@@ -4,6 +4,7 @@ config VIDEO_COBALT
depends on PCI_MSI && MTD_COMPLEX_MAPPINGS
depends on GPIOLIB || COMPILE_TEST
depends on SND
+   depends on MTD
select I2C_ALGOBIT
select VIDEO_ADV7604
select VIDEO_ADV7511
-- 
2.7.0

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

[PATCH] media: Update documentation for media_entity_notify

[Shuah Khan]

Update documentation for media_entity_notify to clearly state the usage
restrictions. This handler is intended for creating links between exiting
entities and should not used to create and register entities.

Signed-off-by: Shuah Khan 
---
 include/media/media-device.h | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/media/media-device.h b/include/media/media-device.h
index d9867ed..2ca9616 100644
--- a/include/media/media-device.h
+++ b/include/media/media-device.h
@@ -271,8 +271,10 @@ struct device;
  * @notify_data: Input data to invoke the callback
  * @notify: Callback function pointer
  *
- * Drivers may register a callback to take action when
- * new entities get registered with the media device.
+ * Drivers may register a callback to take action when new entities get
+ * registered with the media device. This handler is intended for creating
+ * links between existing entities and should not create entities and register
+ * them.
  */
 struct media_entity_notify {
struct list_head list;
-- 
2.5.0

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

[PATCH] include sys/sysmacros.h for major() & minor()

[Mike Frysinger]

Linux C libraries are looking to disentangle sysmacros.h from the
sys/types.h header to clean up namespace pollution.  Since these
macros are provided in glibc/etc... today, switch to pulling in
this header directly.

Signed-off-by: Mike Frysinger 
---
 contrib/test/mc_nextgen_test.c| 1 +
 lib/libv4lconvert/control/libv4lcontrol.c | 1 +
 utils/libmedia_dev/get_media_devices.c| 1 +
 utils/media-ctl/libmediactl.c | 1 +
 4 files changed, 4 insertions(+)

diff --git a/contrib/test/mc_nextgen_test.c b/contrib/test/mc_nextgen_test.c
index a62fd13..4ba37b0 100644
--- a/contrib/test/mc_nextgen_test.c
+++ b/contrib/test/mc_nextgen_test.c
@@ -28,6 +28,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
diff --git a/lib/libv4lconvert/control/libv4lcontrol.c 
b/lib/libv4lconvert/control/libv4lcontrol.c
index 3c8335c..59f28b1 100644
--- a/lib/libv4lconvert/control/libv4lcontrol.c
+++ b/lib/libv4lconvert/control/libv4lcontrol.c
@@ -20,6 +20,7 @@
  */
 
 #include 
+#include 
 #include 
 #include 
 #include 
diff --git a/utils/libmedia_dev/get_media_devices.c 
b/utils/libmedia_dev/get_media_devices.c
index e3a2200..edfeb41 100644
--- a/utils/libmedia_dev/get_media_devices.c
+++ b/utils/libmedia_dev/get_media_devices.c
@@ -20,6 +20,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
diff --git a/utils/media-ctl/libmediactl.c b/utils/media-ctl/libmediactl.c
index 4a82d24..16dddbe 100644
--- a/utils/media-ctl/libmediactl.c
+++ b/utils/media-ctl/libmediactl.c
@@ -24,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
-- 
2.6.2

--
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] media-device: Don't call notify callbacks holding a spinlock

[kbuild test robot]

Hi Mauro,

[auto build test WARNING on sailus-media/master]
[also build test WARNING on next-20160314]
[cannot apply to v4.5]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improving the system]

url:
https://github.com/0day-ci/linux/commits/Mauro-Carvalho-Chehab/media-device-Don-t-call-notify-callbacks-holding-a-spinlock/20160315-031954
base:   git://linuxtv.org/media_tree.git master
config: i386-randconfig-x001-201611 (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All warnings (new ones prefixed by >>):

   drivers/media/i2c/tvp5150.c: In function 'tvp5150_pad_init':
>> drivers/media/i2c/tvp5150.c:1214:6: warning: unused variable 'res' 
>> [-Wunused-variable]
 int res;
 ^
>> drivers/media/i2c/tvp5150.c:1213:18: warning: unused variable 'core' 
>> [-Wunused-variable]
 struct tvp5150 *core = to_tvp5150(sd);
 ^

vim +/res +1214 drivers/media/i2c/tvp5150.c

  1207  
  1208  return 0;
  1209  }
  1210  
  1211  static int __maybe_unused tvp5150_pad_init(struct v4l2_subdev *sd)
  1212  {
> 1213  struct tvp5150 *core = to_tvp5150(sd);
> 1214  int res;
  1215  
  1216  #if defined(CONFIG_MEDIA_CONTROLLER)
  1217  core->pads[DEMOD_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK;

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: Binary data

[PATCH v2] [media] media-device: Don't call notify callbacks holding a spinlock

[Mauro Carvalho Chehab]

The notify routines may sleep. So, they can't be called in spinlock
context. Also, they may want to call other media routines that might
be spinning the spinlock, like creating a link.

This fixes the following bug:

[ 1839.510587] BUG: sleeping function called from invalid context at 
mm/slub.c:1289
[ 1839.510881] in_atomic(): 1, irqs_disabled(): 0, pid: 3479, name: modprobe
[ 1839.511157] 4 locks held by modprobe/3479:
[ 1839.511415]  #0:  (>mutex){..}, at: [] 
__driver_attach+0xa3/0x160
[ 1839.512381]  #1:  (>mutex){..}, at: [] 
__driver_attach+0xb1/0x160
[ 1839.512388]  #2:  (register_mutex#5){+.+.+.}, at: [] 
usb_audio_probe+0x257/0x1c90 [snd_usb_audio]
[ 1839.512401]  #3:  (&(>lock)->rlock){+.+.+.}, at: [] 
media_device_register_entity+0x1cb/0x700 [media]
[ 1839.512412] CPU: 2 PID: 3479 Comm: modprobe Not tainted 4.5.0-rc3+ #49
[ 1839.512415] Hardware name:  /NUC5i7RYB, BIOS 
RYBDWi35.86A.0350.2015.0812.1722 08/12/2015
[ 1839.512417]   8803b3f6f288 81933901 
8803c4bae000
[ 1839.512422]  8803c4bae5c8 8803b3f6f2b0 811c6af5 
8803c4bae000
[ 1839.512427]  8285d7f6 0509 8803b3f6f2f0 
811c6ce5
[ 1839.512432] Call Trace:
[ 1839.512436]  [] dump_stack+0x85/0xc4
[ 1839.512440]  [] ___might_sleep+0x245/0x3a0
[ 1839.512443]  [] __might_sleep+0x95/0x1a0
[ 1839.512446]  [] kmem_cache_alloc_trace+0x20e/0x300
[ 1839.512451]  [] ? media_add_link+0x4d/0x140 [media]
[ 1839.512455]  [] media_add_link+0x4d/0x140 [media]
[ 1839.512459]  [] media_create_pad_link+0xa1/0x600 [media]
[ 1839.512463]  [] au0828_media_graph_notify+0x173/0x360 
[au0828]
[ 1839.512467]  [] ? media_gobj_create+0x1ba/0x480 [media]
[ 1839.512471]  [] media_device_register_entity+0x3ab/0x700 
[media]

Tested with an HVR-950Q, under the following testcases:

1) load au0828 driver first, and then snd-usb-audio;
2) load snd-usb-audio driver first, and then au0828;
3) loading both drivers, and then connecting the device.

Signed-off-by: Mauro Carvalho Chehab 
---

Please disconsider verison 1. It got amended with an experimental patch.

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

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 6ba6e8f982fc..fc3c199e5500 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -587,14 +587,15 @@ int __must_check media_device_register_entity(struct 
media_device *mdev,
media_gobj_create(mdev, MEDIA_GRAPH_PAD,
   >pads[i].graph_obj);
 
+   spin_unlock(>lock);
+
/* invoke entity_notify callbacks */
list_for_each_entry_safe(notify, next, >entity_notify, list) {
(notify)->notify(entity, notify->notify_data);
}
 
-   spin_unlock(>lock);
-
mutex_lock(>graph_mutex);
+
if (mdev->entity_internal_idx_max
>= mdev->pm_count_walk.ent_enum.idx_max) {
struct media_entity_graph new = { .top = 0 };
-- 
2.5.0


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

[PATCH] [media] media-device: Don't call notify callbacks holding a spinlock

[Mauro Carvalho Chehab]

The notify routines may sleep. So, they can't be called in spinlock
context. Also, they may want to call other media routines that might
be spinning the spinlock, like creating a link.

This fixes the following bug:

[ 1839.510587] BUG: sleeping function called from invalid context at 
mm/slub.c:1289
[ 1839.510881] in_atomic(): 1, irqs_disabled(): 0, pid: 3479, name: modprobe
[ 1839.511157] 4 locks held by modprobe/3479:
[ 1839.511415]  #0:  (>mutex){..}, at: [] 
__driver_attach+0xa3/0x160
[ 1839.512381]  #1:  (>mutex){..}, at: [] 
__driver_attach+0xb1/0x160
[ 1839.512388]  #2:  (register_mutex#5){+.+.+.}, at: [] 
usb_audio_probe+0x257/0x1c90 [snd_usb_audio]
[ 1839.512401]  #3:  (&(>lock)->rlock){+.+.+.}, at: [] 
media_device_register_entity+0x1cb/0x700 [media]
[ 1839.512412] CPU: 2 PID: 3479 Comm: modprobe Not tainted 4.5.0-rc3+ #49
[ 1839.512415] Hardware name:  /NUC5i7RYB, BIOS 
RYBDWi35.86A.0350.2015.0812.1722 08/12/2015
[ 1839.512417]   8803b3f6f288 81933901 
8803c4bae000
[ 1839.512422]  8803c4bae5c8 8803b3f6f2b0 811c6af5 
8803c4bae000
[ 1839.512427]  8285d7f6 0509 8803b3f6f2f0 
811c6ce5
[ 1839.512432] Call Trace:
[ 1839.512436]  [] dump_stack+0x85/0xc4
[ 1839.512440]  [] ___might_sleep+0x245/0x3a0
[ 1839.512443]  [] __might_sleep+0x95/0x1a0
[ 1839.512446]  [] kmem_cache_alloc_trace+0x20e/0x300
[ 1839.512451]  [] ? media_add_link+0x4d/0x140 [media]
[ 1839.512455]  [] media_add_link+0x4d/0x140 [media]
[ 1839.512459]  [] media_create_pad_link+0xa1/0x600 [media]
[ 1839.512463]  [] au0828_media_graph_notify+0x173/0x360 
[au0828]
[ 1839.512467]  [] ? media_gobj_create+0x1ba/0x480 [media]
[ 1839.512471]  [] media_device_register_entity+0x3ab/0x700 
[media]

Tested with an HVR-950Q, under the following testcases:

1) load au0828 driver first, and then snd-usb-audio;
2) load snd-usb-audio driver first, and then au0828;
3) loading both drivers, and then connecting the device.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/i2c/tvp5150.c   | 39 ++-
 drivers/media/media-device.c  |  5 +++--
 drivers/media/v4l2-core/v4l2-common.c |  8 +++
 include/media/v4l2-subdev.h   |  4 
 4 files changed, 40 insertions(+), 16 deletions(-)

diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index ff18444e19e4..14004fd7d0fb 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -1208,6 +1208,28 @@ static int tvp5150_registered_async(struct v4l2_subdev 
*sd)
return 0;
 }
 
+static int __maybe_unused tvp5150_pad_init(struct v4l2_subdev *sd)
+{
+   struct tvp5150 *core = to_tvp5150(sd);
+   int res;
+
+#if defined(CONFIG_MEDIA_CONTROLLER)
+   core->pads[DEMOD_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK;
+   core->pads[DEMOD_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE;
+   core->pads[DEMOD_PAD_VBI_OUT].flags = MEDIA_PAD_FL_SOURCE;
+
+   sd->entity.function = MEDIA_ENT_F_ATV_DECODER;
+
+   res = media_entity_pads_init(>entity, DEMOD_NUM_PADS, core->pads);
+   if (res < 0)
+   return res;
+
+   sd->entity.ops = _sd_media_ops;
+#endif
+   return 0;
+}
+
+
 /* --- */
 
 static const struct v4l2_ctrl_ops tvp5150_ctrl_ops = {
@@ -1246,6 +1268,9 @@ static const struct v4l2_subdev_vbi_ops tvp5150_vbi_ops = 
{
 };
 
 static const struct v4l2_subdev_pad_ops tvp5150_pad_ops = {
+#ifdef CONFIG_MEDIA_CONTROLLER
+   .pad_init = tvp5150_pad_init,
+#endif
.enum_mbus_code = tvp5150_enum_mbus_code,
.enum_frame_size = tvp5150_enum_frame_size,
.set_fmt = tvp5150_fill_fmt,
@@ -1475,20 +1500,6 @@ static int tvp5150_probe(struct i2c_client *c,
v4l2_i2c_subdev_init(sd, c, _ops);
sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
 
-#if defined(CONFIG_MEDIA_CONTROLLER)
-   core->pads[DEMOD_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK;
-   core->pads[DEMOD_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE;
-   core->pads[DEMOD_PAD_VBI_OUT].flags = MEDIA_PAD_FL_SOURCE;
-
-   sd->entity.function = MEDIA_ENT_F_ATV_DECODER;
-
-   res = media_entity_pads_init(>entity, DEMOD_NUM_PADS, core->pads);
-   if (res < 0)
-   return res;
-
-   sd->entity.ops = _sd_media_ops;
-#endif
-
res = tvp5150_detect_version(core);
if (res < 0)
return res;
diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 6ba6e8f982fc..fc3c199e5500 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -587,14 +587,15 @@ int __must_check media_device_register_entity(struct 
media_device *mdev,
media_gobj_create(mdev, MEDIA_GRAPH_PAD,
   >pads[i].graph_obj);
 
+   spin_unlock(>lock);
+
/* invoke entity_notify callbacks */

Re: [PATCH v3 3/9] [media] tvp5150: determine BT.656 or YUV 4:2:2 mode from device tree

[Javier Martinez Canillas]

Hello Lucas,

On Mon, Mar 14, 2016 at 12:23 PM, Lucas Stach  wrote:
> From: Philipp Zabel 
>
> By looking at the endpoint flags, it can be determined whether the link
> should be of V4L2_MBUS_PARALLEL or V4L2_MBUS_BT656 type. Disable the
> dedicated HSYNC/VSYNC outputs in BT.656 mode.
>
> For devices that are not instantiated through DT the current behavior
> is preserved.
>
> Signed-off-by: Philipp Zabel 
> Signed-off-by: Lucas Stach 
> ---

Similar to Mauro's comment on patch 2/9, the current driver already
supports configuring the interface output format using DT.

Best regards,
Javier
--
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: Any reason why media_entity_pads_init() isn't void?

[Mauro Carvalho Chehab]

Em Mon, 14 Mar 2016 10:05:01 -0300
Mauro Carvalho Chehab  escreveu:

> Em Mon, 14 Mar 2016 08:52:51 -0300
> Mauro Carvalho Chehab  escreveu:
> 
> > Em Mon, 14 Mar 2016 13:43:33 +0200
> > Sakari Ailus  escreveu:
> >   
> > > Hi Mauro,
> > > 
> > > On Mon, Mar 14, 2016 at 08:27:38AM -0300, Mauro Carvalho Chehab wrote:
> > > > Em Mon, 14 Mar 2016 12:36:44 +0200
> > > > Sakari Ailus  escreveu:
> > > >   
> > > > > Hi Hans,
> > > > > 
> > > > > On Mon, Mar 14, 2016 at 09:25:51AM +0100, Hans Verkuil wrote:  
> > > > > > I was fixing a sparse warning in media_entity_pads_init() and I 
> > > > > > noticed
> > > > > > that that function always returns 0. Any reason why this can't be 
> > > > > > changed
> > > > > > to a void function?
> > > > > 
> > > > > I was thinking of the same function but I had a different question: 
> > > > > why
> > > > > would one call this *after* entity->graph_obj.mdev is set? It is set 
> > > > > by
> > > > > media_device_register_entity(), but once mdev it's set, you're not 
> > > > > expected
> > > > > to call pads_init anymore...  
> > > > 
> > > > Ideally, drivers should *first* create mdev, and then create the
> > > > graph objects, as all objects should be registered at the mdev, in
> > > > order to get their object ID and to be registered at the mdev's object
> > > > lists.  
> > > 
> > > Right. I think it wouldn't hurt to have some comment hints in what's there
> > > for legacy use cases... I can submit patches for some of these.
> > 
> > Yeah, feel free to submit a patch for it. It could be good to add a
> > warn_once() that would hit for the legacy case too.
> >   
> > > 
> > > Currently what works is that you can register graph objects until the 
> > > media
> > > device node is exposed to the user.
> > 
> > Yes.
> >   
> > > We don't have proper serialisation in
> > > place to do that, do we? At least the framework functions leave it up to 
> > > the
> > > caller.
> > > 
> > > I think it wouldn't be a bad idea either to think about the serialisation
> > > model a bit. It's been unchanged from the day one basically.
> > 
> > Actually, the async logic does some sort of serialization, although it
> > doesn't enforce it.
> > 
> > Javier touched on some cases where the logic was not right, but he
> > didn't change everything. 
> > 
> > I agree with you here: it would be great to have this fixed in a better
> > way.
> > 
> > That's said, this affects only non-PCI/USB devices. On PCI/USB, the
> > main/bridge driver is always called first, and the subdev init only
> > happens after it registers the I2C bus.
> >   
> 
> Maybe we could do something like the patch below, to replace the I2C
> probe routines, and then create some logic at the async ops that would
> create the mdev and then call the SD-specific core.mc_probe() methods.
> 
> (patch doesn't compile yet, and it is not complete, but just to give
> an idea on how we could do it).

Still not tested, but, IMHO, in a better shape. See enclosed.

The idea behind it is to split the PAD init on all subdevs and let the
core call it.

For now, the core is calling it early, when v4l2_i2c_subdev_init() is
called, but, after making all subdevs use the new pad_init() callback,
we can call them later. I guess the best place for the code would be
to be called only at v4l2_device_register_subdev(), but we need to
double check if this would work on all cases.

-


diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index ff18444e19e4..14004fd7d0fb 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -1208,6 +1208,28 @@ static int tvp5150_registered_async(struct v4l2_subdev 
*sd)
return 0;
 }
 
+static int __maybe_unused tvp5150_pad_init(struct v4l2_subdev *sd)
+{
+   struct tvp5150 *core = to_tvp5150(sd);
+   int res;
+
+#if defined(CONFIG_MEDIA_CONTROLLER)
+   core->pads[DEMOD_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK;
+   core->pads[DEMOD_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE;
+   core->pads[DEMOD_PAD_VBI_OUT].flags = MEDIA_PAD_FL_SOURCE;
+
+   sd->entity.function = MEDIA_ENT_F_ATV_DECODER;
+
+   res = media_entity_pads_init(>entity, DEMOD_NUM_PADS, core->pads);
+   if (res < 0)
+   return res;
+
+   sd->entity.ops = _sd_media_ops;
+#endif
+   return 0;
+}
+
+
 /* --- */
 
 static const struct v4l2_ctrl_ops tvp5150_ctrl_ops = {
@@ -1246,6 +1268,9 @@ static const struct v4l2_subdev_vbi_ops tvp5150_vbi_ops = 
{
 };
 
 static const struct v4l2_subdev_pad_ops tvp5150_pad_ops = {
+#ifdef CONFIG_MEDIA_CONTROLLER
+   .pad_init = tvp5150_pad_init,
+#endif
.enum_mbus_code = tvp5150_enum_mbus_code,
.enum_frame_size = tvp5150_enum_frame_size,
.set_fmt = tvp5150_fill_fmt,
@@ -1475,20 +1500,6 @@ static int 

Re: [PATCH v3 2/9] [media] tvp5150: add userspace subdev API

[Mauro Carvalho Chehab]

Em Mon, 14 Mar 2016 16:23:30 +0100
Lucas Stach  escreveu:

> From: Philipp Zabel 
> 
> This patch adds userspace V4L2 subdevice API support.
> 
> Signed-off-by: Philipp Zabel 
> Signed-off-by: Lucas Stach 
> ---
>  drivers/media/i2c/tvp5150.c | 282 
> +++-
>  1 file changed, 223 insertions(+), 59 deletions(-)
> 
> diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
> index 6ba93a425640..67312c9d83c1 100644
> --- a/drivers/media/i2c/tvp5150.c
> +++ b/drivers/media/i2c/tvp5150.c
> @@ -36,7 +36,9 @@ MODULE_PARM_DESC(debug, "Debug level (0-2)");
>  
>  struct tvp5150 {
>   struct v4l2_subdev sd;
> + struct media_pad pad;

Huh? tvp5150 has already pads on it:

struct tvp5150 {
struct v4l2_subdev sd;
#ifdef CONFIG_MEDIA_CONTROLLER
struct media_pad pads[DEMOD_NUM_PADS];
struct media_entity input_ent[TVP5150_INPUT_NUM];
struct media_pad input_pad[TVP5150_INPUT_NUM];
#endif
struct v4l2_ctrl_handler hdl;
struct v4l2_rect rect;

v4l2_std_id norm;   /* Current set standard */
u32 input;
u32 output;
int enable;

u16 dev_id;
u16 rom_ver;

enum v4l2_mbus_type mbus_type;
};

It seems that your patchset is not based on the latest version of
the driver. Please rebase it on the top of the media tree:

https://git.linuxtv.org/media_tree.git/

Regards,
Mauro

>   struct v4l2_ctrl_handler hdl;
> + struct v4l2_mbus_framefmt format;
>   struct v4l2_rect rect;
>   struct regmap *regmap;
>  
> @@ -819,38 +821,68 @@ static int tvp5150_enum_mbus_code(struct v4l2_subdev 
> *sd,
>   return 0;
>  }
>  
> -static int tvp5150_fill_fmt(struct v4l2_subdev *sd,
> - struct v4l2_subdev_pad_config *cfg,
> - struct v4l2_subdev_format *format)
> +static void tvp5150_try_crop(struct tvp5150 *decoder, struct v4l2_rect *rect,
> +v4l2_std_id std)
>  {
> - struct v4l2_mbus_framefmt *f;
> - struct tvp5150 *decoder = to_tvp5150(sd);
> + unsigned int hmax;
>  
> - if (!format || format->pad)
> - return -EINVAL;
> + /* Clamp the crop rectangle boundaries to tvp5150 limits */
> + rect->left = clamp(rect->left, 0, TVP5150_MAX_CROP_LEFT);
> + rect->width = clamp(rect->width,
> + TVP5150_H_MAX - TVP5150_MAX_CROP_LEFT - rect->left,
> + TVP5150_H_MAX - rect->left);
> + rect->top = clamp(rect->top, 0, TVP5150_MAX_CROP_TOP);
>  
> - f = >format;
> + /* tvp5150 has some special limits */
> + rect->left = clamp(rect->left, 0, TVP5150_MAX_CROP_LEFT);
> + rect->width = clamp_t(unsigned int, rect->width,
> +   TVP5150_H_MAX - TVP5150_MAX_CROP_LEFT - 
> rect->left,
> +   TVP5150_H_MAX - rect->left);
> + rect->top = clamp(rect->top, 0, TVP5150_MAX_CROP_TOP);
> +
> + /* Calculate height based on current standard */
> + if (std & V4L2_STD_525_60)
> + hmax = TVP5150_V_MAX_525_60;
> + else
> + hmax = TVP5150_V_MAX_OTHERS;
>  
> - tvp5150_reset(sd, 0);
> + rect->height = clamp(rect->height,
> +  hmax - TVP5150_MAX_CROP_TOP - rect->top,
> +  hmax - rect->top);
> +}
>  
> - f->width = decoder->rect.width;
> - f->height = decoder->rect.height;
> +static void tvp5150_set_crop(struct tvp5150 *decoder, struct v4l2_rect *rect,
> +v4l2_std_id std)
> +{
> + struct regmap *map = decoder->regmap;
> + unsigned int hmax;
>  
> - f->code = MEDIA_BUS_FMT_UYVY8_2X8;
> - f->field = V4L2_FIELD_SEQ_TB;
> - f->colorspace = V4L2_COLORSPACE_SMPTE170M;
> + if (std & V4L2_STD_525_60)
> + hmax = TVP5150_V_MAX_525_60;
> + else
> + hmax = TVP5150_V_MAX_OTHERS;
>  
> - v4l2_dbg(1, debug, sd, "width = %d, height = %d\n", f->width,
> - f->height);
> - return 0;
> + regmap_write(map, TVP5150_VERT_BLANKING_START, rect->top);
> + regmap_write(map, TVP5150_VERT_BLANKING_STOP,
> +  rect->top + rect->height - hmax);
> + regmap_write(map, TVP5150_ACT_VD_CROP_ST_MSB,
> +  rect->left >> TVP5150_CROP_SHIFT);
> + regmap_write(map, TVP5150_ACT_VD_CROP_ST_LSB,
> +  rect->left | (1 << TVP5150_CROP_SHIFT));
> + regmap_write(map, TVP5150_ACT_VD_CROP_STP_MSB,
> +  (rect->left + rect->width - TVP5150_MAX_CROP_LEFT) >>
> +  TVP5150_CROP_SHIFT);
> + regmap_write(map, TVP5150_ACT_VD_CROP_STP_LSB,
> +  rect->left + rect->width - TVP5150_MAX_CROP_LEFT);
> +
> + decoder->rect = *rect;
>  }
>  
>  static int tvp5150_s_crop(struct v4l2_subdev *sd, const struct v4l2_crop *a)
>  {
> - struct 

Re: [PATCH 6/7] [media] gspca: fix a v4l2-compliance failure during VIDIOC_REQBUFS

[Hans de Goede]

Hi,

On 14-03-16 16:02, Antonio Ospite wrote:

On Thu, 10 Mar 2016 15:54:37 +0100
Hans de Goede  wrote:


Hi,

On 09-03-16 17:03, Antonio Ospite wrote:

When calling VIDIOC_REQBUFS v4l2-compliance fails with this message:

fail: v4l2-test-buffers.cpp(476): q.reqbufs(node, 1)
test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: FAIL

By looking at the v4l2-compliance code the failure happens when trying
to request V4L2_MEMORY_USERPTR buffers without freeing explicitly the
previously allocated V4L2_MEMORY_MMAP buffers.

This would suggest that when changing the memory field in struct
v4l2_requestbuffers the driver is supposed to free automatically any
previous allocated buffers, and looking for inspiration at the code in
drivers/media/v4l2-core/videobuf2-core.c::vb2_core_reqbufs() seems to
confirm this interpretation; however gspca is just returning -EBUSY in
this case.

Removing the special handling for the case of a different memory value
fixes the compliance failure.

Signed-off-by: Antonio Ospite 
---

This should be safe, but I'd really like a comment from someone with a more
global knowledge of v4l2.

If my interpretation about how drivers should behave when the value of the
memory field changes is correct, I could send also a documentation update for
Documentation/DocBook/media/v4l/vidioc-reqbufs.xml

Just let me know.

Thanks,
 Antonio


   drivers/media/usb/gspca/gspca.c | 7 ---
   1 file changed, 7 deletions(-)

diff --git a/drivers/media/usb/gspca/gspca.c b/drivers/media/usb/gspca/gspca.c
index 84b0d6a..915b6c7 100644
--- a/drivers/media/usb/gspca/gspca.c
+++ b/drivers/media/usb/gspca/gspca.c
@@ -1402,13 +1402,6 @@ static int vidioc_reqbufs(struct file *file, void *priv,
if (mutex_lock_interruptible(_dev->queue_lock))
return -ERESTARTSYS;

-   if (gspca_dev->memory != GSPCA_MEMORY_NO
-   && gspca_dev->memory != GSPCA_MEMORY_READ
-   && gspca_dev->memory != rb->memory) {
-   ret = -EBUSY;
-   goto out;
-   }
-


reqbufs is used internally and this change will allow changing
gspca_dev->memory from USERPTR / MMAP to GSPCA_MEMORY_READ
please replace this check with a check to only allow
rb->memory to be GSPCA_MEMORY_READ when coming from GSPCA_MEMORY_NO
or GSPCA_MEMORY_READ



OK, thanks, I'll take a look again later this week.

In the meantime, if patches from 1 to 5 are OK, can we have them merged
so I will just resubmit the last two in the set?


Not sure when I'll have time to do this, I would prefer to also take
v2 of patch 6 and 7 while at it. But I agree that there is no need
to pick op patches 1 - 5. I'll pick them up from patchwork when I've
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

[PATCH v3 4/9] [media] tvp5150: fix standard autodetection

[Lucas Stach]

From: Philipp Zabel 

Make sure to not overwrite decoder->norm when setting the standard
in hardware, but only when instructed by V4L2 API calls.

Signed-off-by: Philipp Zabel 
Signed-off-by: Lucas Stach 
---
 drivers/media/i2c/tvp5150.c | 56 +
 1 file changed, 31 insertions(+), 25 deletions(-)

diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index f6720d1d09ea..21d044b564ad 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -703,8 +703,6 @@ static int tvp5150_set_std(struct v4l2_subdev *sd, 
v4l2_std_id std)
struct tvp5150 *decoder = to_tvp5150(sd);
int fmt = 0;
 
-   decoder->norm = std;
-
/* First tests should be against specific std */
 
if (std == V4L2_STD_NTSC_443) {
@@ -741,13 +739,37 @@ static int tvp5150_s_std(struct v4l2_subdev *sd, 
v4l2_std_id std)
else
decoder->rect.height = TVP5150_V_MAX_OTHERS;
 
+   decoder->norm = std;
 
return tvp5150_set_std(sd, std);
 }
 
+static v4l2_std_id tvp5150_read_std(struct v4l2_subdev *sd)
+{
+   int val = tvp5150_read(sd, TVP5150_STATUS_REG_5);
+
+   switch (val & 0x0F) {
+   case 0x01:
+   return V4L2_STD_NTSC;
+   case 0x03:
+   return V4L2_STD_PAL;
+   case 0x05:
+   return V4L2_STD_PAL_M;
+   case 0x07:
+   return V4L2_STD_PAL_N | V4L2_STD_PAL_Nc;
+   case 0x09:
+   return V4L2_STD_NTSC_443;
+   case 0xb:
+   return V4L2_STD_SECAM;
+   default:
+   return V4L2_STD_UNKNOWN;
+   }
+}
+
 static int tvp5150_reset(struct v4l2_subdev *sd, u32 val)
 {
struct tvp5150 *decoder = to_tvp5150(sd);
+   v4l2_std_id std;
 
/* Initializes TVP5150 to its default values */
tvp5150_write_inittab(sd, tvp5150_init_default);
@@ -783,7 +805,13 @@ static int tvp5150_reset(struct v4l2_subdev *sd, u32 val)
/* Initialize image preferences */
v4l2_ctrl_handler_setup(>hdl);
 
-   tvp5150_set_std(sd, decoder->norm);
+   if (decoder->norm == V4L2_STD_ALL)
+   std = tvp5150_read_std(sd);
+   else
+   std = decoder->norm;
+
+   /* Disable autoswitch mode */
+   tvp5150_set_std(sd, std);
return 0;
 };
 
@@ -808,28 +836,6 @@ static int tvp5150_s_ctrl(struct v4l2_ctrl *ctrl)
return -EINVAL;
 }
 
-static v4l2_std_id tvp5150_read_std(struct v4l2_subdev *sd)
-{
-   int val = tvp5150_read(sd, TVP5150_STATUS_REG_5);
-
-   switch (val & 0x0F) {
-   case 0x01:
-   return V4L2_STD_NTSC;
-   case 0x03:
-   return V4L2_STD_PAL;
-   case 0x05:
-   return V4L2_STD_PAL_M;
-   case 0x07:
-   return V4L2_STD_PAL_N | V4L2_STD_PAL_Nc;
-   case 0x09:
-   return V4L2_STD_NTSC_443;
-   case 0xb:
-   return V4L2_STD_SECAM;
-   default:
-   return V4L2_STD_UNKNOWN;
-   }
-}
-
 static int tvp5150_enum_mbus_code(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_mbus_code_enum *code)
-- 
2.7.0

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

[PATCH v3 3/9] [media] tvp5150: determine BT.656 or YUV 4:2:2 mode from device tree

[Lucas Stach]

From: Philipp Zabel 

By looking at the endpoint flags, it can be determined whether the link
should be of V4L2_MBUS_PARALLEL or V4L2_MBUS_BT656 type. Disable the
dedicated HSYNC/VSYNC outputs in BT.656 mode.

For devices that are not instantiated through DT the current behavior
is preserved.

Signed-off-by: Philipp Zabel 
Signed-off-by: Lucas Stach 
---
 drivers/media/i2c/tvp5150.c | 34 --
 1 file changed, 32 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index 67312c9d83c1..f6720d1d09ea 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -11,10 +11,12 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
+#include 
 
 #include "tvp5150_reg.h"
 
@@ -38,6 +40,7 @@ struct tvp5150 {
struct v4l2_subdev sd;
struct media_pad pad;
struct v4l2_ctrl_handler hdl;
+   enum v4l2_mbus_type bus_type;
struct v4l2_mbus_framefmt format;
struct v4l2_rect rect;
struct regmap *regmap;
@@ -424,8 +427,6 @@ static const struct i2c_reg_value tvp5150_init_enable[] = {
TVP5150_MISC_CTL, 0x6f
},{ /* Activates video std autodetection for all standards */
TVP5150_AUTOSW_MSK, 0x0
-   },{ /* Default format: 0x47. For 4:2:2: 0x40 */
-   TVP5150_DATA_RATE_SEL, 0x47
},{
TVP5150_CHROMA_PROC_CTL_1, 0x0c
},{
@@ -760,6 +761,25 @@ static int tvp5150_reset(struct v4l2_subdev *sd, u32 val)
/* Initializes TVP5150 to stream enabled values */
tvp5150_write_inittab(sd, tvp5150_init_enable);
 
+   switch (decoder->bus_type) {
+   case V4L2_MBUS_BT656:
+   /* 8-bit ITU BT.656 */
+   regmap_update_bits(decoder->regmap, TVP5150_DATA_RATE_SEL,
+  0x7, 0x7);
+   /* disable HSYNC, VSYNC/PALI, AVID, and FID/GLCO */
+   regmap_update_bits(decoder->regmap, TVP5150_MISC_CTL, 0x4, 0x0);
+   break;
+   case V4L2_MBUS_PARALLEL:
+   /* 8-bit YUV 4:2:2 */
+   regmap_update_bits(decoder->regmap, TVP5150_DATA_RATE_SEL,
+  0x7, 0x0);
+   /* enable HSYNC, VSYNC/PALI, AVID, and FID/GLCO */
+   regmap_update_bits(decoder->regmap, TVP5150_MISC_CTL, 0x4, 0x4);
+   break;
+   default:
+   return -EINVAL;
+   }
+
/* Initialize image preferences */
v4l2_ctrl_handler_setup(>hdl);
 
@@ -1332,6 +1352,8 @@ static struct regmap_config tvp5150_config = {
 static int tvp5150_probe(struct i2c_client *c,
 const struct i2c_device_id *id)
 {
+   struct v4l2_of_endpoint bus_cfg;
+   struct device_node *endpoint;
struct tvp5150 *core;
struct v4l2_subdev *sd;
struct regmap *map;
@@ -1398,6 +1420,14 @@ static int tvp5150_probe(struct i2c_client *c,
}
}
 
+   endpoint = of_graph_get_next_endpoint(c->dev.of_node, NULL);
+   if (endpoint) {
+   v4l2_of_parse_endpoint(endpoint, _cfg);
+   core->bus_type = bus_cfg.bus_type;
+   } else {
+   core->bus_type = V4L2_MBUS_BT656;
+   }
+
core->norm = V4L2_STD_ALL;  /* Default is autodetect */
core->input = TVP5150_COMPOSITE1;
core->enable = 1;
-- 
2.7.0

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

[PATCH v3 1/9] [media] tvp5150: convert register access to regmap

[Lucas Stach]

From: Philipp Zabel 

Regmap provides built in debugging, caching and provides dedicated accessors
for bit manipulations in registers, which make the following changes a lot
simpler.

Signed-off-by: Philipp Zabel 
Signed-off-by: Lucas Stach 
---
 drivers/media/i2c/tvp5150.c | 194 ++--
 1 file changed, 133 insertions(+), 61 deletions(-)

diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index 6c3769d44b75..6ba93a425640 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -10,6 +10,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -37,6 +38,7 @@ struct tvp5150 {
struct v4l2_subdev sd;
struct v4l2_ctrl_handler hdl;
struct v4l2_rect rect;
+   struct regmap *regmap;
 
v4l2_std_id norm;   /* Current set standard */
u32 input;
@@ -56,30 +58,14 @@ static inline struct v4l2_subdev *to_sd(struct v4l2_ctrl 
*ctrl)
 
 static int tvp5150_read(struct v4l2_subdev *sd, unsigned char addr)
 {
-   struct i2c_client *c = v4l2_get_subdevdata(sd);
-   int rc;
-
-   rc = i2c_smbus_read_byte_data(c, addr);
-   if (rc < 0) {
-   v4l2_err(sd, "i2c i/o error: rc == %d\n", rc);
-   return rc;
-   }
-
-   v4l2_dbg(2, debug, sd, "tvp5150: read 0x%02x = 0x%02x\n", addr, rc);
-
-   return rc;
-}
+   struct tvp5150 *decoder = to_tvp5150(sd);
+   int ret, val;
 
-static inline void tvp5150_write(struct v4l2_subdev *sd, unsigned char addr,
-unsigned char value)
-{
-   struct i2c_client *c = v4l2_get_subdevdata(sd);
-   int rc;
+   ret = regmap_read(decoder->regmap, addr, );
+   if (ret < 0)
+   return ret;
 
-   v4l2_dbg(2, debug, sd, "tvp5150: writing 0x%02x 0x%02x\n", addr, value);
-   rc = i2c_smbus_write_byte_data(c, addr, value);
-   if (rc < 0)
-   v4l2_dbg(0, debug, sd, "i2c i/o error: rc == %d\n", rc);
+   return val;
 }
 
 static void dump_reg_range(struct v4l2_subdev *sd, char *s, u8 init,
@@ -266,8 +252,8 @@ static inline void tvp5150_selmux(struct v4l2_subdev *sd)
decoder->input, decoder->output,
input, opmode);
 
-   tvp5150_write(sd, TVP5150_OP_MODE_CTL, opmode);
-   tvp5150_write(sd, TVP5150_VD_IN_SRC_SEL_1, input);
+   regmap_write(decoder->regmap, TVP5150_OP_MODE_CTL, opmode);
+   regmap_write(decoder->regmap, TVP5150_VD_IN_SRC_SEL_1, input);
 
/* Svideo should enable YCrCb output and disable GPCL output
 * For Composite and TV, it should be the reverse
@@ -282,7 +268,7 @@ static inline void tvp5150_selmux(struct v4l2_subdev *sd)
val = (val & ~0x40) | 0x10;
else
val = (val & ~0x10) | 0x40;
-   tvp5150_write(sd, TVP5150_MISC_CTL, val);
+   regmap_write(decoder->regmap, TVP5150_MISC_CTL, val);
 };
 
 struct i2c_reg_value {
@@ -553,8 +539,10 @@ static struct i2c_vbi_ram_value vbi_ram_default[] =
 static int tvp5150_write_inittab(struct v4l2_subdev *sd,
const struct i2c_reg_value *regs)
 {
+   struct tvp5150 *decoder = to_tvp5150(sd);
+
while (regs->reg != 0xff) {
-   tvp5150_write(sd, regs->reg, regs->value);
+   regmap_write(decoder->regmap, regs->reg, regs->value);
regs++;
}
return 0;
@@ -563,22 +551,24 @@ static int tvp5150_write_inittab(struct v4l2_subdev *sd,
 static int tvp5150_vdp_init(struct v4l2_subdev *sd,
const struct i2c_vbi_ram_value *regs)
 {
+   struct tvp5150 *decoder = to_tvp5150(sd);
+   struct regmap *map = decoder->regmap;
unsigned int i;
 
/* Disable Full Field */
-   tvp5150_write(sd, TVP5150_FULL_FIELD_ENA, 0);
+   regmap_write(map, TVP5150_FULL_FIELD_ENA, 0);
 
/* Before programming, Line mode should be at 0xff */
for (i = TVP5150_LINE_MODE_INI; i <= TVP5150_LINE_MODE_END; i++)
-   tvp5150_write(sd, i, 0xff);
+   regmap_write(map, i, 0xff);
 
/* Load Ram Table */
while (regs->reg != (u16)-1) {
-   tvp5150_write(sd, TVP5150_CONF_RAM_ADDR_HIGH, regs->reg >> 8);
-   tvp5150_write(sd, TVP5150_CONF_RAM_ADDR_LOW, regs->reg);
+   regmap_write(map, TVP5150_CONF_RAM_ADDR_HIGH, regs->reg >> 8);
+   regmap_write(map, TVP5150_CONF_RAM_ADDR_LOW, regs->reg);
 
for (i = 0; i < 16; i++)
-   tvp5150_write(sd, TVP5150_VDP_CONF_RAM_DATA, 
regs->values[i]);
+   regmap_write(map, TVP5150_VDP_CONF_RAM_DATA, 
regs->values[i]);
 
regs++;
}
@@ -658,11 +648,11 @@ static int tvp5150_set_vbi(struct v4l2_subdev *sd,
reg=((line-6)<<1)+TVP5150_LINE_MODE_INI;
 
if 

[PATCH v3 2/9] [media] tvp5150: add userspace subdev API

[Lucas Stach]

From: Philipp Zabel 

This patch adds userspace V4L2 subdevice API support.

Signed-off-by: Philipp Zabel 
Signed-off-by: Lucas Stach 
---
 drivers/media/i2c/tvp5150.c | 282 +++-
 1 file changed, 223 insertions(+), 59 deletions(-)

diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index 6ba93a425640..67312c9d83c1 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -36,7 +36,9 @@ MODULE_PARM_DESC(debug, "Debug level (0-2)");
 
 struct tvp5150 {
struct v4l2_subdev sd;
+   struct media_pad pad;
struct v4l2_ctrl_handler hdl;
+   struct v4l2_mbus_framefmt format;
struct v4l2_rect rect;
struct regmap *regmap;
 
@@ -819,38 +821,68 @@ static int tvp5150_enum_mbus_code(struct v4l2_subdev *sd,
return 0;
 }
 
-static int tvp5150_fill_fmt(struct v4l2_subdev *sd,
-   struct v4l2_subdev_pad_config *cfg,
-   struct v4l2_subdev_format *format)
+static void tvp5150_try_crop(struct tvp5150 *decoder, struct v4l2_rect *rect,
+  v4l2_std_id std)
 {
-   struct v4l2_mbus_framefmt *f;
-   struct tvp5150 *decoder = to_tvp5150(sd);
+   unsigned int hmax;
 
-   if (!format || format->pad)
-   return -EINVAL;
+   /* Clamp the crop rectangle boundaries to tvp5150 limits */
+   rect->left = clamp(rect->left, 0, TVP5150_MAX_CROP_LEFT);
+   rect->width = clamp(rect->width,
+   TVP5150_H_MAX - TVP5150_MAX_CROP_LEFT - rect->left,
+   TVP5150_H_MAX - rect->left);
+   rect->top = clamp(rect->top, 0, TVP5150_MAX_CROP_TOP);
 
-   f = >format;
+   /* tvp5150 has some special limits */
+   rect->left = clamp(rect->left, 0, TVP5150_MAX_CROP_LEFT);
+   rect->width = clamp_t(unsigned int, rect->width,
+ TVP5150_H_MAX - TVP5150_MAX_CROP_LEFT - 
rect->left,
+ TVP5150_H_MAX - rect->left);
+   rect->top = clamp(rect->top, 0, TVP5150_MAX_CROP_TOP);
+
+   /* Calculate height based on current standard */
+   if (std & V4L2_STD_525_60)
+   hmax = TVP5150_V_MAX_525_60;
+   else
+   hmax = TVP5150_V_MAX_OTHERS;
 
-   tvp5150_reset(sd, 0);
+   rect->height = clamp(rect->height,
+hmax - TVP5150_MAX_CROP_TOP - rect->top,
+hmax - rect->top);
+}
 
-   f->width = decoder->rect.width;
-   f->height = decoder->rect.height;
+static void tvp5150_set_crop(struct tvp5150 *decoder, struct v4l2_rect *rect,
+  v4l2_std_id std)
+{
+   struct regmap *map = decoder->regmap;
+   unsigned int hmax;
 
-   f->code = MEDIA_BUS_FMT_UYVY8_2X8;
-   f->field = V4L2_FIELD_SEQ_TB;
-   f->colorspace = V4L2_COLORSPACE_SMPTE170M;
+   if (std & V4L2_STD_525_60)
+   hmax = TVP5150_V_MAX_525_60;
+   else
+   hmax = TVP5150_V_MAX_OTHERS;
 
-   v4l2_dbg(1, debug, sd, "width = %d, height = %d\n", f->width,
-   f->height);
-   return 0;
+   regmap_write(map, TVP5150_VERT_BLANKING_START, rect->top);
+   regmap_write(map, TVP5150_VERT_BLANKING_STOP,
+rect->top + rect->height - hmax);
+   regmap_write(map, TVP5150_ACT_VD_CROP_ST_MSB,
+rect->left >> TVP5150_CROP_SHIFT);
+   regmap_write(map, TVP5150_ACT_VD_CROP_ST_LSB,
+rect->left | (1 << TVP5150_CROP_SHIFT));
+   regmap_write(map, TVP5150_ACT_VD_CROP_STP_MSB,
+(rect->left + rect->width - TVP5150_MAX_CROP_LEFT) >>
+TVP5150_CROP_SHIFT);
+   regmap_write(map, TVP5150_ACT_VD_CROP_STP_LSB,
+rect->left + rect->width - TVP5150_MAX_CROP_LEFT);
+
+   decoder->rect = *rect;
 }
 
 static int tvp5150_s_crop(struct v4l2_subdev *sd, const struct v4l2_crop *a)
 {
-   struct v4l2_rect rect = a->c;
struct tvp5150 *decoder = to_tvp5150(sd);
+   struct v4l2_rect rect = a->c;
v4l2_std_id std;
-   unsigned int hmax;
 
v4l2_dbg(1, debug, sd, "%s left=%d, top=%d, width=%d, height=%d\n",
__func__, rect.left, rect.top, rect.width, rect.height);
@@ -858,42 +890,13 @@ static int tvp5150_s_crop(struct v4l2_subdev *sd, const 
struct v4l2_crop *a)
if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
return -EINVAL;
 
-   /* tvp5150 has some special limits */
-   rect.left = clamp(rect.left, 0, TVP5150_MAX_CROP_LEFT);
-   rect.width = clamp_t(unsigned int, rect.width,
-TVP5150_H_MAX - TVP5150_MAX_CROP_LEFT - rect.left,
-TVP5150_H_MAX - rect.left);
-   rect.top = clamp(rect.top, 0, TVP5150_MAX_CROP_TOP);
-
-   /* Calculate height based on current standard */
  

[PATCH v3 8/9] [media] tvp5150: Add sync lock interrupt handling

[Lucas Stach]

From: Philipp Zabel 

This patch adds an optional interrupt handler to handle the sync
lock interrupt and sync lock status.

Signed-off-by: Philipp Zabel 
Signed-off-by: Lucas Stach 
---
 drivers/media/i2c/tvp5150.c | 103 ++--
 drivers/media/i2c/tvp5150_reg.h |   2 +
 2 files changed, 100 insertions(+), 5 deletions(-)

diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index e0f5bc219ced..b5140253b648 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -9,6 +9,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -44,12 +45,14 @@ struct tvp5150 {
struct v4l2_mbus_framefmt format;
struct v4l2_rect rect;
struct regmap *regmap;
+   int irq;
 
v4l2_std_id norm;   /* Current set standard */
v4l2_std_id detected_norm;
u32 input;
u32 output;
int enable;
+   bool lock;
 };
 
 static inline struct tvp5150 *to_tvp5150(struct v4l2_subdev *sd)
@@ -716,6 +719,15 @@ static int tvp5150_set_std(struct v4l2_subdev *sd, 
v4l2_std_id std)
return 0;
 }
 
+static int tvp5150_g_std(struct v4l2_subdev *sd, v4l2_std_id *std)
+{
+   struct tvp5150 *decoder = to_tvp5150(sd);
+
+   *std = decoder->norm;
+
+   return 0;
+}
+
 static int tvp5150_s_std(struct v4l2_subdev *sd, v4l2_std_id std)
 {
struct tvp5150 *decoder = to_tvp5150(sd);
@@ -758,14 +770,25 @@ static v4l2_std_id tvp5150_read_std(struct v4l2_subdev 
*sd)
 
 static int tvp5150_reset(struct v4l2_subdev *sd, u32 val)
 {
+   struct tvp5150 *decoder = to_tvp5150(sd);
+   struct regmap *map = decoder->regmap;
+
/* Initializes TVP5150 to its default values */
tvp5150_write_inittab(sd, tvp5150_init_default);
 
-   /* Configure pins: FID, VSYNC, GPCL/VBLK, SCLK */
-   regmap_write(map, TVP5150_CONF_SHARED_PIN, 0x2);
-   /* Keep interrupt polarity active low */
-   regmap_write(map, TVP5150_INT_CONF, TVP5150_VDPOE);
-   regmap_write(map, TVP5150_INTT_CONFIG_REG_B, 0x0);
+   if (decoder->irq) {
+   /* Configure pins: FID, VSYNC, INTREQ, SCLK */
+   regmap_write(map, TVP5150_CONF_SHARED_PIN, 0x0);
+   /* Set interrupt polarity to active high */
+   regmap_write(map, TVP5150_INT_CONF, TVP5150_VDPOE | 0x1);
+   regmap_write(map, TVP5150_INTT_CONFIG_REG_B, 0x1);
+   } else {
+   /* Configure pins: FID, VSYNC, GPCL/VBLK, SCLK */
+   regmap_write(map, TVP5150_CONF_SHARED_PIN, 0x2);
+   /* Keep interrupt polarity active low */
+   regmap_write(map, TVP5150_INT_CONF, TVP5150_VDPOE);
+   regmap_write(map, TVP5150_INTT_CONFIG_REG_B, 0x0);
+   }
 
/* Initializes VDP registers */
tvp5150_vdp_init(sd, vbi_ram_default);
@@ -776,6 +799,33 @@ static int tvp5150_reset(struct v4l2_subdev *sd, u32 val)
return 0;
 }
 
+static irqreturn_t tvp5150_isr(int irq, void *dev_id)
+{
+   struct tvp5150 *decoder = dev_id;
+   struct regmap *map = decoder->regmap;
+   unsigned int active = 0, status = 0;
+
+   regmap_read(map, TVP5150_INT_STATUS_REG_A, );
+   if (status) {
+   regmap_write(map, TVP5150_INT_STATUS_REG_A, status);
+
+   if (status & TVP5150_INT_A_LOCK)
+   decoder->lock = !!(status & TVP5150_INT_A_LOCK_STATUS);
+
+   return IRQ_HANDLED;
+   }
+
+   regmap_read(map, TVP5150_INT_ACTIVE_REG_B, );
+   if (active) {
+   status = 0;
+   regmap_read(map, TVP5150_INT_STATUS_REG_B, );
+   if (status)
+   regmap_write(map, TVP5150_INT_RESET_REG_B, status);
+   }
+
+   return IRQ_HANDLED;
+}
+
 static int tvp5150_enable(struct v4l2_subdev *sd)
 {
struct tvp5150 *decoder = to_tvp5150(sd);
@@ -939,6 +989,35 @@ static int tvp5150_g_crop(struct v4l2_subdev *sd, struct 
v4l2_crop *a)
return 0;
 }
 
+static int tvp5150_s_stream(struct v4l2_subdev *sd, int enable)
+{
+   struct tvp5150 *decoder = container_of(sd, struct tvp5150, sd);
+
+   if (enable) {
+   /* Enable YUV(OUT7:0), clock */
+   regmap_update_bits(decoder->regmap, TVP5150_MISC_CTL, 0xd,
+   (decoder->bus_type == V4L2_MBUS_BT656) ? 0x9 : 0xd);
+   if (decoder->irq) {
+   /* Enable lock interrupt */
+   regmap_update_bits(decoder->regmap,
+  TVP5150_INT_ENABLE_REG_A,
+  TVP5150_INT_A_LOCK,
+  TVP5150_INT_A_LOCK);
+   }
+   } else {
+   /* Disable YUV(OUT7:0), SYNC, clock */
+   regmap_update_bits(decoder->regmap, TVP5150_MISC_CTL, 0xd, 0x0);
+

[PATCH v3 9/9] [media] tvp5150: disable output while signal not locked

[Lucas Stach]

From: Philipp Zabel 

To avoid short frames on stream start, keep output pins at high impedance
while we are not properly locked onto the input signal.

Signed-off-by: Philipp Zabel 
Signed-off-by: Lucas Stach 
---
 drivers/media/i2c/tvp5150.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index b5140253b648..13ce826a4093 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -51,6 +51,7 @@ struct tvp5150 {
v4l2_std_id detected_norm;
u32 input;
u32 output;
+   u32 oe;
int enable;
bool lock;
 };
@@ -809,8 +810,11 @@ static irqreturn_t tvp5150_isr(int irq, void *dev_id)
if (status) {
regmap_write(map, TVP5150_INT_STATUS_REG_A, status);
 
-   if (status & TVP5150_INT_A_LOCK)
+   if (status & TVP5150_INT_A_LOCK) {
decoder->lock = !!(status & TVP5150_INT_A_LOCK_STATUS);
+   regmap_update_bits(decoder->regmap, TVP5150_MISC_CTL,
+  0xd, decoder->lock ? decoder->oe : 
0);
+   }
 
return IRQ_HANDLED;
}
@@ -841,6 +845,7 @@ static int tvp5150_enable(struct v4l2_subdev *sd)
   0x7, 0x7);
/* disable HSYNC, VSYNC/PALI, AVID, and FID/GLCO */
regmap_update_bits(decoder->regmap, TVP5150_MISC_CTL, 0x4, 0x0);
+   decoder->oe = 0x9;
break;
case V4L2_MBUS_PARALLEL:
/* 8-bit YUV 4:2:2 */
@@ -848,6 +853,7 @@ static int tvp5150_enable(struct v4l2_subdev *sd)
   0x7, 0x0);
/* enable HSYNC, VSYNC/PALI, AVID, and FID/GLCO */
regmap_update_bits(decoder->regmap, TVP5150_MISC_CTL, 0x4, 0x4);
+   decoder->oe = 0xd;
break;
default:
return -EINVAL;
@@ -994,9 +1000,9 @@ static int tvp5150_s_stream(struct v4l2_subdev *sd, int 
enable)
struct tvp5150 *decoder = container_of(sd, struct tvp5150, sd);
 
if (enable) {
-   /* Enable YUV(OUT7:0), clock */
+   /* Enable YUV(OUT7:0), (SYNC), clock signal, if locked */
regmap_update_bits(decoder->regmap, TVP5150_MISC_CTL, 0xd,
-   (decoder->bus_type == V4L2_MBUS_BT656) ? 0x9 : 0xd);
+  decoder->lock ? decoder->oe : 0);
if (decoder->irq) {
/* Enable lock interrupt */
regmap_update_bits(decoder->regmap,
-- 
2.7.0

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

[PATCH v3 5/9] [media] tvp5150: split reset/enable routine

[Lucas Stach]

From: Philipp Zabel 

To trigger standard autodetection only the reset part of the routine
is necessary. Split this out to make it callable on its own.

Signed-off-by: Philipp Zabel 
Signed-off-by: Lucas Stach 
---
 drivers/media/i2c/tvp5150.c | 17 +
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index 21d044b564ad..f5e6bfa9dd7f 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -768,9 +768,6 @@ static v4l2_std_id tvp5150_read_std(struct v4l2_subdev *sd)
 
 static int tvp5150_reset(struct v4l2_subdev *sd, u32 val)
 {
-   struct tvp5150 *decoder = to_tvp5150(sd);
-   v4l2_std_id std;
-
/* Initializes TVP5150 to its default values */
tvp5150_write_inittab(sd, tvp5150_init_default);
 
@@ -780,6 +777,14 @@ static int tvp5150_reset(struct v4l2_subdev *sd, u32 val)
/* Selects decoder input */
tvp5150_selmux(sd);
 
+   return 0;
+}
+
+static int tvp5150_enable(struct v4l2_subdev *sd)
+{
+   struct tvp5150 *decoder = to_tvp5150(sd);
+   v4l2_std_id std;
+
/* Initializes TVP5150 to stream enabled values */
tvp5150_write_inittab(sd, tvp5150_init_enable);
 
@@ -844,6 +849,7 @@ static int tvp5150_enum_mbus_code(struct v4l2_subdev *sd,
return -EINVAL;
 
code->code = MEDIA_BUS_FMT_UYVY8_2X8;
+
return 0;
 }
 
@@ -1179,8 +1185,10 @@ static int tvp5150_set_format(struct v4l2_subdev *sd,
 
format->format = *mbus_format;
 
-   if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE)
+   if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
tvp5150_reset(sd, 0);
+   tvp5150_enable(sd);
+   }
 
v4l2_dbg(1, debug, sd, "width = %d, height = %d\n", mbus_format->width,
mbus_format->height);
@@ -1454,6 +1462,7 @@ static int tvp5150_probe(struct i2c_client *c,
}
v4l2_ctrl_handler_setup(>hdl);
 
+   tvp5150_reset(sd, 0);
/* Default is no cropping */
tvp5150_set_default(tvp5150_read_std(sd), >rect, >format);
 
-- 
2.7.0

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

[PATCH v3 6/9] [media] tvp5150: trigger autodetection on subdev open to reset cropping

[Lucas Stach]

From: Philipp Zabel 

If cropping isn't set explicitly by userspace, reset it to the maximum
possible rectangle in subdevice open if a standard change is detected.

Signed-off-by: Philipp Zabel 
Signed-off-by: Lucas Stach 
---
 drivers/media/i2c/tvp5150.c | 18 +-
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index f5e6bfa9dd7f..d0b5e148dcd8 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -46,6 +46,7 @@ struct tvp5150 {
struct regmap *regmap;
 
v4l2_std_id norm;   /* Current set standard */
+   v4l2_std_id detected_norm;
u32 input;
u32 output;
int enable;
@@ -1220,13 +1221,19 @@ static int tvp5150_open(struct v4l2_subdev *sd, struct 
v4l2_subdev_fh *fh)
struct tvp5150 *decoder = to_tvp5150(sd);
v4l2_std_id std;
 
-   if (decoder->norm == V4L2_STD_ALL)
+   if (decoder->norm == V4L2_STD_ALL) {
std = tvp5150_read_std(sd);
-   else
-   std = decoder->norm;
+   if (std != decoder->detected_norm) {
+   decoder->detected_norm = std;
+
+   if (std & V4L2_STD_525_60)
+   decoder->rect.height = TVP5150_V_MAX_525_60;
+   else
+   decoder->rect.height = TVP5150_V_MAX_OTHERS;
+   decoder->format.height = decoder->rect.height;
+   }
+   }
 
-   tvp5150_set_default(std, v4l2_subdev_get_try_crop(fh, 0),
-v4l2_subdev_get_try_format(fh, 0));
return 0;
 }
 #endif
@@ -1443,6 +1450,7 @@ static int tvp5150_probe(struct i2c_client *c,
}
 
core->norm = V4L2_STD_ALL;  /* Default is autodetect */
+   core->detected_norm = V4L2_STD_UNKNOWN;
core->input = TVP5150_COMPOSITE1;
core->enable = 1;
 
-- 
2.7.0

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

[PATCH v3 7/9] [media] tvp5150: remove pin configuration from initialization tables

[Lucas Stach]

From: Philipp Zabel 

To allow optional interrupt support, we want to configure the pin settings
dynamically. Move those register accesses out of the static initialization
tables.

Signed-off-by: Philipp Zabel 
Signed-off-by: Lucas Stach 
---
 drivers/media/i2c/tvp5150.c | 19 +++
 drivers/media/i2c/tvp5150_reg.h |  1 +
 2 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index d0b5e148dcd8..e0f5bc219ced 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -323,9 +323,6 @@ static const struct i2c_reg_value tvp5150_init_default[] = {
{ /* 0x0e */
TVP5150_LUMA_PROC_CTL_3,0x00
},
-   { /* 0x0f */
-   TVP5150_CONF_SHARED_PIN,0x08
-   },
{ /* 0x11 */
TVP5150_ACT_VD_CROP_ST_MSB,0x00
},
@@ -362,9 +359,6 @@ static const struct i2c_reg_value tvp5150_init_default[] = {
{ /* 0x1d */
TVP5150_INT_ENABLE_REG_B,0x00
},
-   { /* 0x1e */
-   TVP5150_INTT_CONFIG_REG_B,0x00
-   },
{ /* 0x28 */
TVP5150_VIDEO_STD,0x00
},
@@ -383,9 +377,6 @@ static const struct i2c_reg_value tvp5150_init_default[] = {
{ /* 0xc1 */
TVP5150_INT_ENABLE_REG_A,0x00
},
-   { /* 0xc2 */
-   TVP5150_INT_CONF,0x04
-   },
{ /* 0xc8 */
TVP5150_FIFO_INT_THRESHOLD,0x80
},
@@ -420,9 +411,7 @@ static const struct i2c_reg_value tvp5150_init_default[] = {
 
 /* Default values as sugested at TVP5150AM1 datasheet */
 static const struct i2c_reg_value tvp5150_init_enable[] = {
-   {
-   TVP5150_CONF_SHARED_PIN, 2
-   },{ /* Automatic offset and AGC enabled */
+   {   /* Automatic offset and AGC enabled */
TVP5150_ANAL_CHL_CTL, 0x15
},{ /* Activate YCrCb output 0x9 or 0xd ? */
TVP5150_MISC_CTL, 0x6f
@@ -772,6 +761,12 @@ static int tvp5150_reset(struct v4l2_subdev *sd, u32 val)
/* Initializes TVP5150 to its default values */
tvp5150_write_inittab(sd, tvp5150_init_default);
 
+   /* Configure pins: FID, VSYNC, GPCL/VBLK, SCLK */
+   regmap_write(map, TVP5150_CONF_SHARED_PIN, 0x2);
+   /* Keep interrupt polarity active low */
+   regmap_write(map, TVP5150_INT_CONF, TVP5150_VDPOE);
+   regmap_write(map, TVP5150_INTT_CONFIG_REG_B, 0x0);
+
/* Initializes VDP registers */
tvp5150_vdp_init(sd, vbi_ram_default);
 
diff --git a/drivers/media/i2c/tvp5150_reg.h b/drivers/media/i2c/tvp5150_reg.h
index 25a994944918..fc3bcb26413a 100644
--- a/drivers/media/i2c/tvp5150_reg.h
+++ b/drivers/media/i2c/tvp5150_reg.h
@@ -117,6 +117,7 @@
 #define TVP5150_INT_STATUS_REG_A0xc0 /* Interrupt status register A */
 #define TVP5150_INT_ENABLE_REG_A0xc1 /* Interrupt enable register A */
 #define TVP5150_INT_CONF0xc2 /* Interrupt configuration */
+#define   TVP5150_VDPOE BIT(2)
 #define TVP5150_VDP_CONF_RAM_DATA   0xc3 /* VDP configuration RAM data */
 #define TVP5150_CONF_RAM_ADDR_LOW   0xc4 /* Configuration RAM address low byte 
*/
 #define TVP5150_CONF_RAM_ADDR_HIGH  0xc5 /* Configuration RAM address high 
byte */
-- 
2.7.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 v3 06/22] media: Media Controller enable/disable source handler API

[Shuah Khan]

On 03/13/2016 02:11 PM, Sakari Ailus wrote:
> Hi Shuah,
> 
> On Thu, Mar 10, 2016 at 07:29:59AM -0700, Shuah Khan wrote:
>> On 03/10/2016 12:35 AM, Sakari Ailus wrote:
>>> Hi Shuah,
>>>
>>> On Thu, Feb 11, 2016 at 04:41:22PM -0700, Shuah Khan wrote:
 Add new fields to struct media_device to add enable_source, and
 disable_source handlers, and source_priv to stash driver private
 data that is used to run these handlers. The enable_source handler
 finds source entity for the passed in entity and checks if it is
 available. When link is found, it activates it. Disable source
 handler deactivates the link.

 Bridge driver is expected to implement and set these handlers.

 Signed-off-by: Shuah Khan 
 ---
  include/media/media-device.h | 30 ++
  1 file changed, 30 insertions(+)

 diff --git a/include/media/media-device.h b/include/media/media-device.h
 index 075a482..1a04644 100644
 --- a/include/media/media-device.h
 +++ b/include/media/media-device.h
 @@ -302,6 +302,11 @@ struct media_entity_notify {
   * @entity_notify: List of registered entity_notify callbacks
   * @lock: Entities list lock
   * @graph_mutex: Entities graph operation lock
 + *
 + * @source_priv: Driver Private data for enable/disable source handlers
 + * @enable_source: Enable Source Handler function pointer
 + * @disable_source: Disable Source Handler function pointer
 + *
   * @link_notify: Link state change notification callback
   *
   * This structure represents an abstract high-level media device. It 
 allows easy
 @@ -313,6 +318,26 @@ struct media_entity_notify {
   *
   * @model is a descriptive model name exported through sysfs. It doesn't 
 have to
   * be unique.
 + *
 + * @enable_source is a handler to find source entity for the
 + * sink entity  and activate the link between them if source
 + * entity is free. Drivers should call this handler before
 + * accessing the source.
 + *
 + * @disable_source is a handler to find source entity for the
 + * sink entity  and deactivate the link between them. Drivers
 + * should call this handler to release the source.
 + *
>>>
>>> Is there a particular reason you're not simply (de)activating the link, but
>>> instead add a new callback?
>>
>> These two handlers are separate for a couple of reasons:
>>
>> 1. Explicit and symmetric API is easier to use and maintain.
>>Similar what we do in other cases, register/unregister
>>get/put etc.
> 
> Link state is set explicitly (enabled or disabled). This is certainly not a
> reason to create a redundant API for link setup.
> 
>> 2. This is more important. Disable handler makes sure the
>>owner is releasing the resource. Otherwise, when some
>>other application does enable, the owner could loose
>>the resource, if enable and disable are the same.
>>
>>e.g: Video app is holding the resource, DVB app does
>>enable. Disable handler makes sure Video/owner  is the one
>>that is asking to do the release.
> 
> Based on the later patches in this set, the enable_source() callback of the 
> au0828 driver performs three things:
> 
> - Find the source entity,
> - Enable the link from some au0828 entity to the source and
> - Start the pipeline that begins from the I/O device node. The pipe object
>   is embedded in struct video_device.
> 
> disable_source() undoes this in reverse order.
> 
> That's in the au0828 driver and rightly so.
> 
> Then it gets murkier. enable_source() and disable_source() callbacks
> (through a few turns) will get called from v4l2-ioctl.c functions
> v4l_querycap, v4l_s_fmt, v4l_s_frequency, v4l_s_std and v4l_querystd. This
> is also performed in VB2 core vb2_core_streamon() function.
> 
> I certainly have no objections when it comes to blocking other processes
> from setting the format when a process holding a file handle to a device has
> e.g. set the format. The implementation is another matter.
> 
> What particularly does concern me in this patchset is:
> 
> - struct media_pipe is intended to be allocated by drivers embedded in
>   another struct holding information the driver needs related to the
>   pipeline. Moving this struct to struct video_device prevents this, which
>   translates to v4l_enable_media_source() and v4l_disable_media_source()
>   functions the patchset adds being specific to the au0828 driver. They
>   should be part of that driver, and may not be part of the V4L2 core.
>   struct media_pipe may not be added to struct video_device for the same
>   reason.

This media_pipe is associated with struct video_device though. I don't
understand your concern. I am viewing this media_pipe as part of the
registered video_device. video_device struct is in the au0828 device
and gets registered as a whole including the media_pipe 

> 
> - The IOCTL handlers 

Re: [PATCH 6/7] [media] gspca: fix a v4l2-compliance failure during VIDIOC_REQBUFS

[Antonio Ospite]

On Thu, 10 Mar 2016 15:54:37 +0100
Hans de Goede  wrote:

> Hi,
> 
> On 09-03-16 17:03, Antonio Ospite wrote:
> > When calling VIDIOC_REQBUFS v4l2-compliance fails with this message:
> >
> >fail: v4l2-test-buffers.cpp(476): q.reqbufs(node, 1)
> >test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: FAIL
> >
> > By looking at the v4l2-compliance code the failure happens when trying
> > to request V4L2_MEMORY_USERPTR buffers without freeing explicitly the
> > previously allocated V4L2_MEMORY_MMAP buffers.
> >
> > This would suggest that when changing the memory field in struct
> > v4l2_requestbuffers the driver is supposed to free automatically any
> > previous allocated buffers, and looking for inspiration at the code in
> > drivers/media/v4l2-core/videobuf2-core.c::vb2_core_reqbufs() seems to
> > confirm this interpretation; however gspca is just returning -EBUSY in
> > this case.
> >
> > Removing the special handling for the case of a different memory value
> > fixes the compliance failure.
> >
> > Signed-off-by: Antonio Ospite 
> > ---
> >
> > This should be safe, but I'd really like a comment from someone with a more
> > global knowledge of v4l2.
> >
> > If my interpretation about how drivers should behave when the value of the
> > memory field changes is correct, I could send also a documentation update 
> > for
> > Documentation/DocBook/media/v4l/vidioc-reqbufs.xml
> >
> > Just let me know.
> >
> > Thanks,
> > Antonio
> >
> >
> >   drivers/media/usb/gspca/gspca.c | 7 ---
> >   1 file changed, 7 deletions(-)
> >
> > diff --git a/drivers/media/usb/gspca/gspca.c 
> > b/drivers/media/usb/gspca/gspca.c
> > index 84b0d6a..915b6c7 100644
> > --- a/drivers/media/usb/gspca/gspca.c
> > +++ b/drivers/media/usb/gspca/gspca.c
> > @@ -1402,13 +1402,6 @@ static int vidioc_reqbufs(struct file *file, void 
> > *priv,
> > if (mutex_lock_interruptible(_dev->queue_lock))
> > return -ERESTARTSYS;
> >
> > -   if (gspca_dev->memory != GSPCA_MEMORY_NO
> > -   && gspca_dev->memory != GSPCA_MEMORY_READ
> > -   && gspca_dev->memory != rb->memory) {
> > -   ret = -EBUSY;
> > -   goto out;
> > -   }
> > -
> 
> reqbufs is used internally and this change will allow changing
> gspca_dev->memory from USERPTR / MMAP to GSPCA_MEMORY_READ
> please replace this check with a check to only allow
> rb->memory to be GSPCA_MEMORY_READ when coming from GSPCA_MEMORY_NO
> or GSPCA_MEMORY_READ
> 

OK, thanks, I'll take a look again later this week.

In the meantime, if patches from 1 to 5 are OK, can we have them merged
so I will just resubmit the last two in the set?

Ciao,
   Antonio

-- 
Antonio Ospite
http://ao2.it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
--
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: i.mx6 camera interface (CSI) and mainline kernel

[Philippe De Muyter]

Hi Steve and all,

I am busy porting to Steve's subdev version a driver that I had written
in intdev style, for the freescale imx6 linux version.  And I have a
problem :

My previous driver had the following ioctl function, which used
the V4L2_FRMIVAL_TYPE_CONTINUOUS type answer.

static int ioctl_enum_frameintervals(struct v4l2_int_device *s,
 struct v4l2_frmivalenum *fival)
{
if (fival->index)
return -EINVAL;
fival->type = V4L2_FRMIVAL_TYPE_CONTINUOUS;
fival->stepwise.min.numerator = 1;
fival->stepwise.min.denominator = MAX_FPS;
fival->stepwise.max.numerator = 1;
fival->stepwise.max.denominator = MIN_FPS;
fival->stepwise.step.numerator = 1;
fival->stepwise.step.denominator = 1;
return 0;
}

Now I see that Steve's related ioctl, using the 'enum_frame_interval'
pad function is :

static int vidioc_enum_frameintervals(struct file *file, void *priv,
  struct v4l2_frmivalenum *fival)
{
struct v4l2_subdev_frame_interval_enum fie ...;
...

ret = v4l2_subdev_call(dev->ep->sd, pad, enum_frame_interval,
   NULL, );

fival->type = V4L2_FRMIVAL_TYPE_DISCRETE;
fival->discrete = fie.interval;
return 0;
}

where type is arbitrarily set to V4L2_FRMIVAL_TYPE_DISCRETE, because of the
limitation of the struct v4l2_subdev_frame_interval_enum, which has no 'type',
field nor a 'stepwise' field.

How can I do to continue to provide de V4L2_FRMIVAL_TYPE_CONTINUOUS answer ?

TIA

Philippe

On Thu, Mar 10, 2016 at 08:30:02AM +0100, Hans Verkuil wrote:
> On 03/10/2016 01:12 AM, Steve Longerbeam wrote:
> > On 03/09/2016 02:44 PM, Tim Harvey wrote:
> >> On Tue, Mar 8, 2016 at 6:06 PM, Steve Longerbeam
> >>  wrote:
> >>> On 03/07/2016 08:19 AM, Tim Harvey wrote:
> >> 
> >>>
> >>> Hi Tim, good to hear it works for you on the Ventana boards.
> >>>
> >>> I've just pushed some more commits to the mx6-media-staging branch that
> >>> get the drivers/media/i2c/adv7180.c subdev working with the imx6 capture
> >>> backend. Images look perfect when switching to UYVY8_2X8 mbus code instead
> >>> of YUYV8_2X8. But I'm holding off on that change because this subdev is 
> >>> used
> >>> by Renesas targets and would likely corrupt captured images for those
> >>> targets. But I believe UYVY is the correct transmit order according to the
> >>> BT.656 standard.
> >>>
> >>> Another thing that should also be changed in drivers/media/i2c/adv7180.c
> >>> is the field type. It should be V4L2_FIELD_SEQ_TB for NTSC and 
> >>> V4L2_FIELD_SEQ_BT
> >>> for PAL.
> >>>
> >>> Steve
> >>>
> >>>
> >> Steve,
> >>
> >> with your latest patches, I'm crashing with an null-pointer-deref in
> >> adv7180_set_pad_format. What is your kernel config for
> >> CONFIG_MEDIA_CONTROLLER and CONFIG_VIDEO_V4L2_SUBDEV_API?
> > 
> > Right, I thought I fixed that, I was passing a NULL pad_cfg for
> > TRY_FORMAT, but that was fixed. Maybe you fetched a version
> > of mx6-media-staging while I was in the middle of debugging?
> > Try fetching again.
> > 
> > I tried with both CONFIG_MEDIA_CONTROLLER and
> > CONFIG_VIDEO_V4L2_SUBDEV_API enabled and both disabled, and
> > I don't get the null deref in adv7180_set_pad_format.
> > 
> > 
> >>
> >> Your tree contains about 16 or so patches on top of linux-media for
> >> imx6 capture. Are you close to the point where you will be posting a
> >> patch series? If so, please CC me for testing with the adv7180 sensor.
> > 
> > I guess I can try posting a series again, but there will likely be 
> > push-back from
> > Pengutronix. They have their own video capture driver for imx6. Last I 
> > heard (a while ago!)
> > their version did not implement scaling, colorspace conversion, or image 
> > rotation via
> > the IC. Instead their driver sends raw camera frames directly to memory, 
> > and image
> > conversion is carried out by separate mem2mem device. Our capture driver 
> > does
> > image conversion (scaling, CSC, and rotation) natively using the IC 
> > pre-processing channel.
> > We also have a mem2mem device that does conversion using IC post-processing,
> > which I have included in mx6-media-staging.
> > 
> > Also IIRC they did some pretty slick stuff with a video bus multiplexer 
> > subdev, which
> > can multiplex video from different sensors either using the internal mux in 
> > the SoC,
> > or can control an external mux via gpio. Our driver only supports the 
> > internal mux,
> > and does it with a platform data function.
> > 
> > But like I said, I don't what the latest status is of the Pengutronix video 
> > capture support.
> > 
> > Btw, I just pushed an update of mx6-media-staging that implements 
> > vidioc_[gs]_selection.
> 
> Steve & Pengutronix,
> 
> I would be happy to add drivers to staging, either Steve's, Pengutronix or 
> both.
> 
> The i.mx6 is quite popular 

Re: [PATCH 2/2] v4l2-ioctl: improve cropcap handling

[Niklas Söderlund]

Reviewed-by: Niklas Söderlund 

On 2016-02-29 11:16:40 +0100, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> If cropcap is implemented, then call it first to fill in the pixel
> aspect ratio. Don't return if cropcap returns ENOTTY or ENOIOCTLCMD,
> in that case just assume a 1:1 pixel aspect ratio.
> 
> After cropcap was called, then overwrite bounds and defrect with the
> g_selection result because the g_selection() result always has priority
> over anything that vidioc_cropcap returns.
> 
> Signed-off-by: Hans Verkuil 
> ---
>  drivers/media/v4l2-core/v4l2-ioctl.c | 35 +++
>  1 file changed, 27 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c 
> b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 67dbb03..a3db569 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -2158,11 +2158,37 @@ static int v4l_cropcap(const struct v4l2_ioctl_ops 
> *ops,
>  {
>   struct v4l2_cropcap *p = arg;
>   struct v4l2_selection s = { .type = p->type };
> - int ret;
> + int ret = -ENOTTY;
>  
>   if (ops->vidioc_g_selection == NULL)
>   return ops->vidioc_cropcap(file, fh, p);
>  
> + /*
> +  * Let cropcap fill in the pixelaspect if cropcap is available.
> +  * There is still no other way of obtaining this information.
> +  */
> + if (ops->vidioc_cropcap) {
> + ret = ops->vidioc_cropcap(file, fh, p);
> +
> + /*
> +  * If cropcap reports that it isn't implemented,
> +  * then just keep going.
> +  */
> + if (ret && ret != -ENOTTY && ret != -ENOIOCTLCMD)
> + return ret;
> + }
> +
> + if (ret) {
> + /*
> +  * cropcap wasn't implemented, so assume a 1:1 pixel
> +  * aspect ratio, otherwise keep what cropcap gave us.
> +  */
> + p->pixelaspect.numerator = 1;
> + p->pixelaspect.denominator = 1;
> + }
> +
> + /* Use g_selection() to fill in the bounds and defrect rectangles */
> +
>   /* obtaining bounds */
>   if (V4L2_TYPE_IS_OUTPUT(p->type))
>   s.target = V4L2_SEL_TGT_COMPOSE_BOUNDS;
> @@ -2185,13 +2211,6 @@ static int v4l_cropcap(const struct v4l2_ioctl_ops 
> *ops,
>   return ret;
>   p->defrect = s.r;
>  
> - /* setting trivial pixelaspect */
> - p->pixelaspect.numerator = 1;
> - p->pixelaspect.denominator = 1;
> -
> - if (ops->vidioc_cropcap)
> - return ops->vidioc_cropcap(file, fh, p);
> -
>   return 0;
>  }
>  
> -- 
> 2.7.0
> 

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

Re: [PATCH 1/2] v4l2-ioctl: simplify code

[Niklas Söderlund]

On 2016-03-14 13:52:17 +0100, Hans Verkuil wrote:
> On 03/14/2016 01:42 PM, Niklas Söderlund wrote:
> > Hi Hans,
> > 
> > On 2016-02-29 11:16:39 +0100, Hans Verkuil wrote:
> >> From: Hans Verkuil 
> >>
> >> Instead of a big if at the beginning, just check if g_selection == NULL
> >> and call the cropcap op immediately and return the result.
> >>
> >> No functional changes in this patch.
> >>
> >> Signed-off-by: Hans Verkuil 
> >> ---
> >>  drivers/media/v4l2-core/v4l2-ioctl.c | 44 
> >> ++--
> >>  1 file changed, 22 insertions(+), 22 deletions(-)
> >>
> >> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c 
> >> b/drivers/media/v4l2-core/v4l2-ioctl.c
> >> index 86c4c19..67dbb03 100644
> >> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> >> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> >> @@ -2157,33 +2157,33 @@ static int v4l_cropcap(const struct v4l2_ioctl_ops 
> >> *ops,
> >>struct file *file, void *fh, void *arg)
> >>  {
> >>struct v4l2_cropcap *p = arg;
> >> +  struct v4l2_selection s = { .type = p->type };
> >> +  int ret;
> >>  
> >> -  if (ops->vidioc_g_selection) {
> >> -  struct v4l2_selection s = { .type = p->type };
> >> -  int ret;
> >> +  if (ops->vidioc_g_selection == NULL)
> >> +  return ops->vidioc_cropcap(file, fh, p);
> > 
> > I might be missing something but is there a guarantee 
> > ops->vidioc_cropcap is not NULL here?
> 
> There is, either vidioc_g_selection or vidioc_cropcap will always be
> non-NULL. Since g_selection == NULL it follows that cropcap != NULL.
> 
> But I admit that it isn't exactly obvious since the test that ensures
> this is in determine_valid_ioctls() in v4l2-dev.c.

Nice, thanks for clarifying.

Reviewed-by: Niklas Söderlund 

-- 
Regards,
Niklas Söderlund
--
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: Any reason why media_entity_pads_init() isn't void?

[Mauro Carvalho Chehab]

Em Mon, 14 Mar 2016 08:52:51 -0300
Mauro Carvalho Chehab  escreveu:

> Em Mon, 14 Mar 2016 13:43:33 +0200
> Sakari Ailus  escreveu:
> 
> > Hi Mauro,
> > 
> > On Mon, Mar 14, 2016 at 08:27:38AM -0300, Mauro Carvalho Chehab wrote:  
> > > Em Mon, 14 Mar 2016 12:36:44 +0200
> > > Sakari Ailus  escreveu:
> > > 
> > > > Hi Hans,
> > > > 
> > > > On Mon, Mar 14, 2016 at 09:25:51AM +0100, Hans Verkuil wrote:
> > > > > I was fixing a sparse warning in media_entity_pads_init() and I 
> > > > > noticed
> > > > > that that function always returns 0. Any reason why this can't be 
> > > > > changed
> > > > > to a void function?  
> > > > 
> > > > I was thinking of the same function but I had a different question: why
> > > > would one call this *after* entity->graph_obj.mdev is set? It is set by
> > > > media_device_register_entity(), but once mdev it's set, you're not 
> > > > expected
> > > > to call pads_init anymore...
> > > 
> > > Ideally, drivers should *first* create mdev, and then create the
> > > graph objects, as all objects should be registered at the mdev, in
> > > order to get their object ID and to be registered at the mdev's object
> > > lists.
> > 
> > Right. I think it wouldn't hurt to have some comment hints in what's there
> > for legacy use cases... I can submit patches for some of these.  
> 
> Yeah, feel free to submit a patch for it. It could be good to add a
> warn_once() that would hit for the legacy case too.
> 
> > 
> > Currently what works is that you can register graph objects until the media
> > device node is exposed to the user.  
> 
> Yes.
> 
> > We don't have proper serialisation in
> > place to do that, do we? At least the framework functions leave it up to the
> > caller.
> > 
> > I think it wouldn't be a bad idea either to think about the serialisation
> > model a bit. It's been unchanged from the day one basically.  
> 
> Actually, the async logic does some sort of serialization, although it
> doesn't enforce it.
> 
> Javier touched on some cases where the logic was not right, but he
> didn't change everything. 
> 
> I agree with you here: it would be great to have this fixed in a better
> way.
> 
> That's said, this affects only non-PCI/USB devices. On PCI/USB, the
> main/bridge driver is always called first, and the subdev init only
> happens after it registers the I2C bus.
> 

Maybe we could do something like the patch below, to replace the I2C
probe routines, and then create some logic at the async ops that would
create the mdev and then call the SD-specific core.mc_probe() methods.

(patch doesn't compile yet, and it is not complete, but just to give
an idea on how we could do it).


diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index ff18444e19e4..36bf6c57eb64 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -1214,6 +1214,9 @@ static const struct v4l2_ctrl_ops tvp5150_ctrl_ops = {
.s_ctrl = tvp5150_s_ctrl,
 };
 
+static int tvp5150_mc_probe(struct v4l2_subdev *sd);
+
+
 static const struct v4l2_subdev_core_ops tvp5150_core_ops = {
.log_status = tvp5150_log_status,
.reset = tvp5150_reset,
@@ -1221,6 +1224,9 @@ static const struct v4l2_subdev_core_ops tvp5150_core_ops 
= {
.g_register = tvp5150_g_register,
.s_register = tvp5150_s_register,
 #endif
+#ifdef CONFIG_MEDIA_CONTROLLER
+   .mc_probe = tvp5150_mc_probe,
+#endif
.registered_async = tvp5150_registered_async,
 };
 
@@ -1438,11 +1444,33 @@ static const char * const tvp5150_test_patterns[2] = {
"Black screen"
 };
 
-static int tvp5150_probe(struct i2c_client *c,
-const struct i2c_device_id *id)
+static int __maybe_unused tvp5150_mc_probe(struct v4l2_subdev *sd)
+{
+   struct tvp5150 *core = to_tvp5150(sd);
+   int res;
+
+#if defined(CONFIG_MEDIA_CONTROLLER)
+   core->pads[DEMOD_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK;
+   core->pads[DEMOD_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE;
+   core->pads[DEMOD_PAD_VBI_OUT].flags = MEDIA_PAD_FL_SOURCE;
+
+   sd->entity.function = MEDIA_ENT_F_ATV_DECODER;
+
+   res = media_entity_pads_init(>entity, DEMOD_NUM_PADS, core->pads);
+   if (res < 0)
+   return res;
+
+   sd->entity.ops = _sd_media_ops;
+#endif
+   return 0;
+}
+
+static int __tvp5150_probe(struct i2c_client *c,
+  const struct i2c_device_id *id,
+  struct v4l2_subdev **sd_store)
 {
struct tvp5150 *core;
-   struct v4l2_subdev *sd;
+   struct v4l2_subdev *sd; /* FIXME: get rid of it */
struct device_node *np = c->dev.of_node;
int res;
 
@@ -1460,6 +1488,7 @@ static int tvp5150_probe(struct i2c_client *c,
return -ENOMEM;
 
sd = >sd;
+   *sd_store = sd;
 
if (IS_ENABLED(CONFIG_OF) && np) {
res = tvp5150_parse_dt(core, np);

Re: [PATCH 1/2] v4l2-ioctl: simplify code

[Hans Verkuil]

On 03/14/2016 01:42 PM, Niklas Söderlund wrote:
> Hi Hans,
> 
> On 2016-02-29 11:16:39 +0100, Hans Verkuil wrote:
>> From: Hans Verkuil 
>>
>> Instead of a big if at the beginning, just check if g_selection == NULL
>> and call the cropcap op immediately and return the result.
>>
>> No functional changes in this patch.
>>
>> Signed-off-by: Hans Verkuil 
>> ---
>>  drivers/media/v4l2-core/v4l2-ioctl.c | 44 
>> ++--
>>  1 file changed, 22 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c 
>> b/drivers/media/v4l2-core/v4l2-ioctl.c
>> index 86c4c19..67dbb03 100644
>> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
>> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
>> @@ -2157,33 +2157,33 @@ static int v4l_cropcap(const struct v4l2_ioctl_ops 
>> *ops,
>>  struct file *file, void *fh, void *arg)
>>  {
>>  struct v4l2_cropcap *p = arg;
>> +struct v4l2_selection s = { .type = p->type };
>> +int ret;
>>  
>> -if (ops->vidioc_g_selection) {
>> -struct v4l2_selection s = { .type = p->type };
>> -int ret;
>> +if (ops->vidioc_g_selection == NULL)
>> +return ops->vidioc_cropcap(file, fh, p);
> 
> I might be missing something but is there a guarantee 
> ops->vidioc_cropcap is not NULL here?

There is, either vidioc_g_selection or vidioc_cropcap will always be
non-NULL. Since g_selection == NULL it follows that cropcap != NULL.

But I admit that it isn't exactly obvious since the test that ensures
this is in determine_valid_ioctls() in v4l2-dev.c.

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 1/2] v4l2-ioctl: simplify code

[Niklas Söderlund]

Hi Hans,

On 2016-02-29 11:16:39 +0100, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> Instead of a big if at the beginning, just check if g_selection == NULL
> and call the cropcap op immediately and return the result.
> 
> No functional changes in this patch.
> 
> Signed-off-by: Hans Verkuil 
> ---
>  drivers/media/v4l2-core/v4l2-ioctl.c | 44 
> ++--
>  1 file changed, 22 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c 
> b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 86c4c19..67dbb03 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -2157,33 +2157,33 @@ static int v4l_cropcap(const struct v4l2_ioctl_ops 
> *ops,
>   struct file *file, void *fh, void *arg)
>  {
>   struct v4l2_cropcap *p = arg;
> + struct v4l2_selection s = { .type = p->type };
> + int ret;
>  
> - if (ops->vidioc_g_selection) {
> - struct v4l2_selection s = { .type = p->type };
> - int ret;
> + if (ops->vidioc_g_selection == NULL)
> + return ops->vidioc_cropcap(file, fh, p);

I might be missing something but is there a guarantee 
ops->vidioc_cropcap is not NULL here?

>  
> - /* obtaining bounds */
> - if (V4L2_TYPE_IS_OUTPUT(p->type))
> - s.target = V4L2_SEL_TGT_COMPOSE_BOUNDS;
> - else
> - s.target = V4L2_SEL_TGT_CROP_BOUNDS;
> + /* obtaining bounds */
> + if (V4L2_TYPE_IS_OUTPUT(p->type))
> + s.target = V4L2_SEL_TGT_COMPOSE_BOUNDS;
> + else
> + s.target = V4L2_SEL_TGT_CROP_BOUNDS;
>  
> - ret = ops->vidioc_g_selection(file, fh, );
> - if (ret)
> - return ret;
> - p->bounds = s.r;
> + ret = ops->vidioc_g_selection(file, fh, );
> + if (ret)
> + return ret;
> + p->bounds = s.r;
>  
> - /* obtaining defrect */
> - if (V4L2_TYPE_IS_OUTPUT(p->type))
> - s.target = V4L2_SEL_TGT_COMPOSE_DEFAULT;
> - else
> - s.target = V4L2_SEL_TGT_CROP_DEFAULT;
> + /* obtaining defrect */
> + if (V4L2_TYPE_IS_OUTPUT(p->type))
> + s.target = V4L2_SEL_TGT_COMPOSE_DEFAULT;
> + else
> + s.target = V4L2_SEL_TGT_CROP_DEFAULT;
>  
> - ret = ops->vidioc_g_selection(file, fh, );
> - if (ret)
> - return ret;
> - p->defrect = s.r;
> - }
> + ret = ops->vidioc_g_selection(file, fh, );
> + if (ret)
> + return ret;
> + p->defrect = s.r;
>  
>   /* setting trivial pixelaspect */
>   p->pixelaspect.numerator = 1;
> -- 
> 2.7.0
> 

-- 
Regards,
Niklas Söderlund
--
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: v4l2-compat-ioctl32: fix missing reserved field copy in put_v4l2_create32

[tiffany lin]

On Mon, 2016-03-14 at 08:28 -0300, Mauro Carvalho Chehab wrote:
> Em Mon, 14 Mar 2016 18:41:46 +0800
> Tiffany Lin  escreveu:
> 
> > Change-Id: Idac449fae5059a3ce255340e6da491f8bd83af7a
> 
> We don't need change-id at the Kernel, but we do need a proper patch
> description.
> 
Hi Mauro,

Sorry, I accidentally add change-id in this patch.
I had remove change-id and add patch description in 
[PATCH v2] media: v4l2-compat-ioctl32: fix missing reserved field copy
in put_v4l2_create32

best regards,
Tiffany


> Regards,
> Mauro
> 
> > ---
> >  drivers/media/v4l2-core/v4l2-compat-ioctl32.c |3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c 
> > b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> > index f38c076..109f687 100644
> > --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> > +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> > @@ -280,7 +280,8 @@ static int put_v4l2_format32(struct v4l2_format *kp, 
> > struct v4l2_format32 __user
> >  static int put_v4l2_create32(struct v4l2_create_buffers *kp, struct 
> > v4l2_create_buffers32 __user *up)
> >  {
> > if (!access_ok(VERIFY_WRITE, up, sizeof(struct v4l2_create_buffers32)) 
> > ||
> > -   copy_to_user(up, kp, offsetof(struct v4l2_create_buffers32, 
> > format)))
> > +   copy_to_user(up, kp, offsetof(struct v4l2_create_buffers32, 
> > format)) ||
> > +   copy_to_user(up->reserved, kp->reserved, sizeof(kp->reserved)))
> > return -EFAULT;
> > return __put_v4l2_format32(>format, >format);
> >  }


--
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: add GFP flag to media_*() that could get called in atomic context

[Sakari Ailus]

Hi Mauro,

On Mon, Mar 14, 2016 at 08:46:33AM -0300, Mauro Carvalho Chehab wrote:
> Em Mon, 14 Mar 2016 12:52:54 +0200
> Sakari Ailus  escreveu:
> 
> > Hi Mauro,
> > 
> > On Mon, Mar 14, 2016 at 07:13:58AM -0300, Mauro Carvalho Chehab wrote:
> > > Em Mon, 14 Mar 2016 09:22:37 +0200
> > > Sakari Ailus  escreveu:
> > >   
> > > > Hi Shuah,
> > > > 
> > > > On Sat, Mar 12, 2016 at 06:48:09PM -0700, Shuah Khan wrote:  
> > > > > Add GFP flags to media_create_pad_link(), media_create_intf_link(),
> > > > > media_devnode_create(), and media_add_link() that could get called
> > > > > in atomic context to allow callers to pass in the right flags for
> > > > > memory allocation.
> > > > > 
> > > > > tree-wide driver changes for media_*() GFP flags change:
> > > > > Change drivers to add gfpflags to interffaces, 
> > > > > media_create_pad_link(),
> > > > > media_create_intf_link() and media_devnode_create().
> > > > > 
> > > > > Signed-off-by: Shuah Khan 
> > > > > Suggested-by: Mauro Carvalho Chehab 
> > > > 
> > > > What's the use case for calling the above functions in an atomic 
> > > > context?
> > > >   
> > > 
> > > ALSA code seems to do a lot of stuff at atomic context. That's what
> > > happens on my test machine when au0828 gets probed before
> > > snd-usb-audio:
> > >   http://pastebin.com/LEX5LD5K
> > > 
> > > It seems that ALSA USB probe routine (usb_audio_probe) happens in
> > > atomic context.  
> > 
> > usb_audio_probe() grabs a mutex (register_mutex) on its own. It certainly
> > cannot be called in atomic context.
> > 
> > In the above log, what I did notice, though, was that because *we* grab
> > mdev->lock spinlock in media_device_register_entity(), we may not sleep
> > which is what the notify() callback implementation in au0828 driver does
> > (for memory allocation).
> 
> True. After looking into the code, the problem is that the notify
> callbacks are called with the spinlock hold. I don't see any reason
> to do that.

Notify callbacks, perhaps not, but the list is still protected by the
spinlock. It perhaps is not likely that another process would change it but
I don't think we can rely on that.

> 
> > Could we instead replace mdev->lock by a mutex?
> 
> We changed the code to use a spinlock for a reason: this fixed some
> troubles in the past with the code locking (can't remember the problem,
> but this was documented at the kernel logs and at the ML). Yet, the code
> under the spinlock never sleeps, so this is fine.

struct media_device.lock was added by this patch:

commit 53e269c102fbaf77e7dc526b1606ad4a48e57200
Author: Laurent Pinchart 
Date:   Wed Dec 9 08:40:00 2009 -0300

[media] media: Entities, pads and links

As video hardware pipelines become increasingly complex and
configurable, the current hardware description through v4l2 subdevices
reaches its limits. In addition to enumerating and configuring
subdevices, video camera drivers need a way to discover and modify at
runtime how those subdevices are connected. This is done through new
elements called entities, pads and links.

...

Signed-off-by: Laurent Pinchart 
Signed-off-by: Sakari Ailus 
Acked-by: Hans Verkuil 
Signed-off-by: Mauro Carvalho Chehab 

I think it was always a spinlock, for the reason you stated above as well:
it did not need to sleep. But if there is a need to sleep, I think we should
consider changing that.

> 
> Yet, in the future, we'll need to do a review of all the locking schema,
> in order to better support dynamic graph changes.

Agreed. I think more fine grained locking should be considered. The media
graph mutex will become a bottleneck at some point, especially if we make
the media devices system wide at some point.

> 
> > If there is no pressing need to implement atomic memory allocation I simply
> > wouldn't do it, especially in device initialisation where an allocation
> > failure will lead to probe failure as well.
> 
> The fix for this issue should be simple. See the enclosed code. Btw.
> it probably makes sense to add some code here to avoid starving the
> stack, as a notify callback could try to create an entity, with,
> in turn, would call the notify callback again.
> 
> I'll run some tests here to double check if it fixes the issue.
> 
> ---
> 
> [media] media-device: Don't call notify callbacks holding a spinlock
> 
> The notify routines may sleep. So, they can't be called in spinlock
> context. Also, they may want to call other media routines that might
> be spinning the spinlock, like creating a link.
> 
> This fixes the following bug:
> 
> [ 1839.510587] BUG: sleeping function called from invalid context at 
> mm/slub.c:1289
> [ 1839.510881] in_atomic(): 1, irqs_disabled(): 0, pid: 3479, name: modprobe
> [ 1839.511157] 

Re: Any reason why media_entity_pads_init() isn't void?

[Mauro Carvalho Chehab]

Em Mon, 14 Mar 2016 13:43:33 +0200
Sakari Ailus  escreveu:

> Hi Mauro,
> 
> On Mon, Mar 14, 2016 at 08:27:38AM -0300, Mauro Carvalho Chehab wrote:
> > Em Mon, 14 Mar 2016 12:36:44 +0200
> > Sakari Ailus  escreveu:
> >   
> > > Hi Hans,
> > > 
> > > On Mon, Mar 14, 2016 at 09:25:51AM +0100, Hans Verkuil wrote:  
> > > > I was fixing a sparse warning in media_entity_pads_init() and I noticed
> > > > that that function always returns 0. Any reason why this can't be 
> > > > changed
> > > > to a void function?
> > > 
> > > I was thinking of the same function but I had a different question: why
> > > would one call this *after* entity->graph_obj.mdev is set? It is set by
> > > media_device_register_entity(), but once mdev it's set, you're not 
> > > expected
> > > to call pads_init anymore...  
> > 
> > Ideally, drivers should *first* create mdev, and then create the
> > graph objects, as all objects should be registered at the mdev, in
> > order to get their object ID and to be registered at the mdev's object
> > lists.  
> 
> Right. I think it wouldn't hurt to have some comment hints in what's there
> for legacy use cases... I can submit patches for some of these.

Yeah, feel free to submit a patch for it. It could be good to add a
warn_once() that would hit for the legacy case too.

> 
> Currently what works is that you can register graph objects until the media
> device node is exposed to the user.

Yes.

> We don't have proper serialisation in
> place to do that, do we? At least the framework functions leave it up to the
> caller.
> 
> I think it wouldn't be a bad idea either to think about the serialisation
> model a bit. It's been unchanged from the day one basically.

Actually, the async logic does some sort of serialization, although it
doesn't enforce it.

Javier touched on some cases where the logic was not right, but he
didn't change everything. 

I agree with you here: it would be great to have this fixed in a better
way.

That's said, this affects only non-PCI/USB devices. On PCI/USB, the
main/bridge driver is always called first, and the subdev init only
happens after it registers the I2C bus.

-- 
Thanks,
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: [PATCH] media: add GFP flag to media_*() that could get called in atomic context

[Mauro Carvalho Chehab]

Em Mon, 14 Mar 2016 12:52:54 +0200
Sakari Ailus  escreveu:

> Hi Mauro,
> 
> On Mon, Mar 14, 2016 at 07:13:58AM -0300, Mauro Carvalho Chehab wrote:
> > Em Mon, 14 Mar 2016 09:22:37 +0200
> > Sakari Ailus  escreveu:
> >   
> > > Hi Shuah,
> > > 
> > > On Sat, Mar 12, 2016 at 06:48:09PM -0700, Shuah Khan wrote:  
> > > > Add GFP flags to media_create_pad_link(), media_create_intf_link(),
> > > > media_devnode_create(), and media_add_link() that could get called
> > > > in atomic context to allow callers to pass in the right flags for
> > > > memory allocation.
> > > > 
> > > > tree-wide driver changes for media_*() GFP flags change:
> > > > Change drivers to add gfpflags to interffaces, media_create_pad_link(),
> > > > media_create_intf_link() and media_devnode_create().
> > > > 
> > > > Signed-off-by: Shuah Khan 
> > > > Suggested-by: Mauro Carvalho Chehab 
> > > 
> > > What's the use case for calling the above functions in an atomic context?
> > >   
> > 
> > ALSA code seems to do a lot of stuff at atomic context. That's what
> > happens on my test machine when au0828 gets probed before
> > snd-usb-audio:
> > http://pastebin.com/LEX5LD5K
> > 
> > It seems that ALSA USB probe routine (usb_audio_probe) happens in
> > atomic context.  
> 
> usb_audio_probe() grabs a mutex (register_mutex) on its own. It certainly
> cannot be called in atomic context.
> 
> In the above log, what I did notice, though, was that because *we* grab
> mdev->lock spinlock in media_device_register_entity(), we may not sleep
> which is what the notify() callback implementation in au0828 driver does
> (for memory allocation).

True. After looking into the code, the problem is that the notify
callbacks are called with the spinlock hold. I don't see any reason
to do that.

> Could we instead replace mdev->lock by a mutex?

We changed the code to use a spinlock for a reason: this fixed some
troubles in the past with the code locking (can't remember the problem,
but this was documented at the kernel logs and at the ML). Yet, the code
under the spinlock never sleeps, so this is fine.

Yet, in the future, we'll need to do a review of all the locking schema,
in order to better support dynamic graph changes.

> If there is no pressing need to implement atomic memory allocation I simply
> wouldn't do it, especially in device initialisation where an allocation
> failure will lead to probe failure as well.

The fix for this issue should be simple. See the enclosed code. Btw.
it probably makes sense to add some code here to avoid starving the
stack, as a notify callback could try to create an entity, with,
in turn, would call the notify callback again.

I'll run some tests here to double check if it fixes the issue.

---

[media] media-device: Don't call notify callbacks holding a spinlock

The notify routines may sleep. So, they can't be called in spinlock
context. Also, they may want to call other media routines that might
be spinning the spinlock, like creating a link.

This fixes the following bug:

[ 1839.510587] BUG: sleeping function called from invalid context at 
mm/slub.c:1289
[ 1839.510881] in_atomic(): 1, irqs_disabled(): 0, pid: 3479, name: modprobe
[ 1839.511157] 4 locks held by modprobe/3479:
[ 1839.511415]  #0:  (>mutex){..}, at: [] 
__driver_attach+0xa3/0x160
[ 1839.512381]  #1:  (>mutex){..}, at: [] 
__driver_attach+0xb1/0x160
[ 1839.512388]  #2:  (register_mutex#5){+.+.+.}, at: [] 
usb_audio_probe+0x257/0x1c90 [snd_usb_audio]
[ 1839.512401]  #3:  (&(>lock)->rlock){+.+.+.}, at: [] 
media_device_register_entity+0x1cb/0x700 [media]
[ 1839.512412] CPU: 2 PID: 3479 Comm: modprobe Not tainted 4.5.0-rc3+ #49
[ 1839.512415] Hardware name:  /NUC5i7RYB, BIOS 
RYBDWi35.86A.0350.2015.0812.1722 08/12/2015
[ 1839.512417]   8803b3f6f288 81933901 
8803c4bae000
[ 1839.512422]  8803c4bae5c8 8803b3f6f2b0 811c6af5 
8803c4bae000
[ 1839.512427]  8285d7f6 0509 8803b3f6f2f0 
811c6ce5
[ 1839.512432] Call Trace:
[ 1839.512436]  [] dump_stack+0x85/0xc4
[ 1839.512440]  [] ___might_sleep+0x245/0x3a0
[ 1839.512443]  [] __might_sleep+0x95/0x1a0
[ 1839.512446]  [] kmem_cache_alloc_trace+0x20e/0x300
[ 1839.512451]  [] ? media_add_link+0x4d/0x140 [media]
[ 1839.512455]  [] media_add_link+0x4d/0x140 [media]
[ 1839.512459]  [] media_create_pad_link+0xa1/0x600 [media]
[ 1839.512463]  [] au0828_media_graph_notify+0x173/0x360 
[au0828]
[ 1839.512467]  [] ? media_gobj_create+0x1ba/0x480 [media]
[ 1839.512471]  [] media_device_register_entity+0x3ab/0x700 
[media]

(untested)

Signed-off-by: Mauro Carvalho Chehab 

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 6ba6e8f982fc..fc3c199e5500 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -587,14 +587,15 @@ int __must_check 

Re: Any reason why media_entity_pads_init() isn't void?

[Sakari Ailus]

Hi Mauro,

On Mon, Mar 14, 2016 at 08:27:38AM -0300, Mauro Carvalho Chehab wrote:
> Em Mon, 14 Mar 2016 12:36:44 +0200
> Sakari Ailus  escreveu:
> 
> > Hi Hans,
> > 
> > On Mon, Mar 14, 2016 at 09:25:51AM +0100, Hans Verkuil wrote:
> > > I was fixing a sparse warning in media_entity_pads_init() and I noticed
> > > that that function always returns 0. Any reason why this can't be changed
> > > to a void function?  
> > 
> > I was thinking of the same function but I had a different question: why
> > would one call this *after* entity->graph_obj.mdev is set? It is set by
> > media_device_register_entity(), but once mdev it's set, you're not expected
> > to call pads_init anymore...
> 
> Ideally, drivers should *first* create mdev, and then create the
> graph objects, as all objects should be registered at the mdev, in
> order to get their object ID and to be registered at the mdev's object
> lists.

Right. I think it wouldn't hurt to have some comment hints in what's there
for legacy use cases... I can submit patches for some of these.

Currently what works is that you can register graph objects until the media
device node is exposed to the user. We don't have proper serialisation in
place to do that, do we? At least the framework functions leave it up to the
caller.

I think it wouldn't be a bad idea either to think about the serialisation
model a bit. It's been unchanged from the day one basically.

> However, some legacy drivers used to do just the reverse. So, we had
> to add a code there, and at media_device_register_entity() to support
> the drivers that postpone the creation of the mdev instance.
> 
> Once this gets fixed everywhere, we can remove the code that supports
> the legacy behavior.

-- 
Kind regards,

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

DVBv5 Tools: VDR format is broken (recommended patch)

[Markus Heiser]

Hi Mauro,

sorry for bumping, but could you take a look at my 
libdvbv5/dvb-vdr-format.c patch? Please give me a 
short feedback / thanks a lot.

 --Markus--

Am 10.03.2016 um 15:07 schrieb Markus Heiser :

> Hi (Mauro),
> 
> below you will find my recommended patch for the broken 
> VDR format (libdvbv5/dvb-vdr-format.c).
> 
> There is only one point I have a doubt: I have no ATSC 
> experience (I'am in Europe/germ), so I simply added 
> an "A" at the field "satellite pos.". This is what 
> the w_scan tool does and this tool works fine with
> the vdr (please correct me if I'am wrong).
> 
> My test-case was the same as mentioned in the first
> mail (see below). Which means, I haven't tested with
> vdr, only with mpv.
> 
> Is there anyone how can take up this patch? 
> ... may be Mauro, the originator of vdr support?
> 
> With best regards
> 
> -- M --
> 
> 
> diff --git a/lib/libdvbv5/dvb-vdr-format.c b/lib/libdvbv5/dvb-vdr-format.c
> index 176a927..5151ebc 100644
> --- a/lib/libdvbv5/dvb-vdr-format.c
> +++ b/lib/libdvbv5/dvb-vdr-format.c
> @@ -310,13 +310,14 @@ int dvb_write_format_vdr(const char *fname,
>   fprintf(fp, "%s", entry->channel);
>   if (entry->vchannel)
>   fprintf(fp, ",%s", entry->vchannel);
> + fprintf(fp, ":");
> 
>   /*
>* Output frequency:
>*  in kHz for terrestrial/cable
>*  in MHz for satellite
>*/
> - fprintf(fp, ":%i:", freq / 1000);
> + fprintf(fp, "%i:", freq / 1000);
> 
>   /* Output modulation parameters */
>   fmt = [i];
> @@ -350,20 +351,28 @@ int dvb_write_format_vdr(const char *fname,
> 
>   fprintf(fp, "%s", table->table[data]);
>   }
> -
> - /* Output format type */
> - fprintf(fp, ":%s:", id);
> + fprintf(fp, ":");
> 
>   /*
> -  * Output satellite location
> -  * FIXME: probably require some adjustments to match the
> -  *format expected by VDR.
> +  * Output sources configuration for VDR
> +  *
> +  *   S (satellite) xy.z (orbital position in degrees) E or W 
> (east or west)
> + *
> + *   FIXME: in case of ATSC we use "A", this is what w_scan 
> does
>*/
> - switch(delsys) {
> - case SYS_DVBS:
> - case SYS_DVBS2:
> - fprintf(fp, "%s:", entry->location);
> +
> + if (entry->location) {
> +   switch(delsys) {
> +   case SYS_DVBS:
> +   case SYS_DVBS2:
> + fprintf(fp, "%s", entry->location);   break;
> +   default:
> + fprintf(fp, "%s", id);break;
> +   }
> + } else {
> +   fprintf(fp, "%s", id);
>   }
> + fprintf(fp, ":");
> 
>   /* Output symbol rate */
>   srate = 2750;
> @@ -408,10 +417,16 @@ int dvb_write_format_vdr(const char *fname,
>   /* Output Service ID */
>   fprintf(fp, "%d:", entry->service_id);
> 
> - /* Output SID, NID, TID and RID */
> - fprintf(fp, "0:0:0:");
> + /* Output Network ID */
> + fprintf(fp, "0:");
> +
> + /* Output Transport Stream ID */
> + fprintf(fp, "0:");
> 
> - fprintf(fp, "\n");
> + /* Output Radio ID
> +this is the last entry, tagged bei a new line (not a colon!)
> +  */
> + fprintf(fp, "0\n");
>   line++;
>   };
>   fclose (fp);
> 
> 
> -- M --
> 
> 
> Am 09.03.2016 um 16:43 schrieb Markus Heiser :
> 
>> Hi,
>> 
>> I tested DVBv5 tools, creating vdr channel lists. My first attemp
>> was to convert a dvbv5 channel list:
>> 
>>  -
>> # file: test_convert_in.conf
>> #
>> # converted with: dvb-format-convert -I DVBV5 -O VDR  test_convert_in.conf 
>> test_convert_out.conf
>> #
>> [Das Erste HD]
>>  SERVICE_ID = 10301
>>  VIDEO_PID = 5101
>>  AUDIO_PID = 5102 5103 5106 5108
>>  PID_0b = 5172 2171
>>  PID_06 = 5105 5104
>>  PID_05 = 1170
>>  LNB = UNIVERSAL
>>  FREQUENCY = 11494000
>>  INVERSION = OFF
>>  SYMBOL_RATE = 22000488
>>  INNER_FEC = 2/3
>>  MODULATION = PSK/8
>>  PILOT = ON
>>  ROLLOFF = 35
>>  POLARIZATION = HORIZONTAL
>>  STREAM_ID = 0
>>  DELIVERY_SYSTEM = DVBS2
>>  -
>> 
>> 
>> this results in a strange VDR channel (test_convert_out.conf):
>> 
>> 
>>  -
>> Das Erste 
>> HD:11494:S1HC23I0M5N1O35:S:(null):22000:5101:5102,5103,5106,5108:0:0:10301:0:0:0:
>>  -
>> 
>> 
>> so I created an 

Re: [PATCH] media: v4l2-compat-ioctl32: fix missing reserved field copy in put_v4l2_create32

[Mauro Carvalho Chehab]

Em Mon, 14 Mar 2016 18:41:46 +0800
Tiffany Lin  escreveu:

> Change-Id: Idac449fae5059a3ce255340e6da491f8bd83af7a

We don't need change-id at the Kernel, but we do need a proper patch
description.

Regards,
Mauro

> ---
>  drivers/media/v4l2-core/v4l2-compat-ioctl32.c |3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c 
> b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> index f38c076..109f687 100644
> --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> @@ -280,7 +280,8 @@ static int put_v4l2_format32(struct v4l2_format *kp, 
> struct v4l2_format32 __user
>  static int put_v4l2_create32(struct v4l2_create_buffers *kp, struct 
> v4l2_create_buffers32 __user *up)
>  {
>   if (!access_ok(VERIFY_WRITE, up, sizeof(struct v4l2_create_buffers32)) 
> ||
> - copy_to_user(up, kp, offsetof(struct v4l2_create_buffers32, 
> format)))
> + copy_to_user(up, kp, offsetof(struct v4l2_create_buffers32, 
> format)) ||
> + copy_to_user(up->reserved, kp->reserved, sizeof(kp->reserved)))
>   return -EFAULT;
>   return __put_v4l2_format32(>format, >format);
>  }
--
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: Any reason why media_entity_pads_init() isn't void?

[Mauro Carvalho Chehab]

Em Mon, 14 Mar 2016 12:36:44 +0200
Sakari Ailus  escreveu:

> Hi Hans,
> 
> On Mon, Mar 14, 2016 at 09:25:51AM +0100, Hans Verkuil wrote:
> > I was fixing a sparse warning in media_entity_pads_init() and I noticed
> > that that function always returns 0. Any reason why this can't be changed
> > to a void function?  
> 
> I was thinking of the same function but I had a different question: why
> would one call this *after* entity->graph_obj.mdev is set? It is set by
> media_device_register_entity(), but once mdev it's set, you're not expected
> to call pads_init anymore...

Ideally, drivers should *first* create mdev, and then create the
graph objects, as all objects should be registered at the mdev, in
order to get their object ID and to be registered at the mdev's object
lists.

However, some legacy drivers used to do just the reverse. So, we had
to add a code there, and at media_device_register_entity() to support
the drivers that postpone the creation of the mdev instance.

Once this gets fixed everywhere, we can remove the code that supports
the legacy behavior.

Regards,
Mauro

> 
> I'm fine making this return void.
> 
> > 
> > That return value is checked a zillion times in the media code. By making
> > it void it should simplify code all over.
> > 
> > See e.g. uvc_mc_init_entity in drivers/media/usb/uvc/uvc_entity.c: that
> > whole function can become a void function itself.  
> 


-- 
Thanks,
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: Question regarding internal webcams of tablet devices

[Dennis Wassenberg]

Hi,

do you know where I can get the source code of these drivers for
Baytrail, Anniedale or Cherrytrail? I am not familiar with the Android
kernel. I checked git://git.code.sf.net/p/android-x86/kernel but could
not find any PCI CSI2 host controller driver. At
https://android.googlesource.com/kernel/x86_64.git I found some MIPI
stuff at drivers/external_drivers/intel_media/ but no CSI-2 driver.

Thank you & best regards,

Dennis

On 08.03.2016 17:23, Daniel Glöckner wrote:
> Hi,
> 
> On Tue, Mar 08, 2016 at 04:21:01PM +0100, Dennis Wassenberg wrote:
>> However, at first there is the need to get a driver for the camera IO
>> host controller PCI device. Is there anybody how knows a driver for that
>> pci device or known if there will be a driver for that in the future? Is
>> this the right way to support this kind of cameras or is there an other
>> way to get such cameras working with linux?
> 
> I know that Intel wrote a GPLv2 driver for the CSI host controller in
> Merrifield, Baytrail, Anniedale and Cherrytrail. It is part of their
> Android kernel. You might have luck searching for an Android kernel
> for a Skylake tablet. But be careful, the driver I know only supports
> a fixed set of configurations. It seems like Intel expects every
> manufacturer to just copy their reference design down to the GPIO
> numbers used to reset the camera sensors.
> 
> Best regards,
> 
>   Daniel
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] media: v4l2-compat-ioctl32: fix missing reserved field copy in put_v4l2_create32

[Tiffany Lin]

In v4l2-compliance utility, test VIDIOC_CREATE_BUFS will check whether reserved
filed of v4l2_create_buffers filled with zero
Reserved field is filled with zero in v4l_create_bufs.
This patch copy reserved field of v4l2_create_buffer from kernel space to user
space

Signed-off-by: Tiffany Lin 
---

Changes since v1
- Remove Change-Id
- Add commit message

 drivers/media/v4l2-core/v4l2-compat-ioctl32.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c 
b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
index f38c076..109f687 100644
--- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
+++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
@@ -280,7 +280,8 @@ static int put_v4l2_format32(struct v4l2_format *kp, struct 
v4l2_format32 __user
 static int put_v4l2_create32(struct v4l2_create_buffers *kp, struct 
v4l2_create_buffers32 __user *up)
 {
if (!access_ok(VERIFY_WRITE, up, sizeof(struct v4l2_create_buffers32)) 
||
-   copy_to_user(up, kp, offsetof(struct v4l2_create_buffers32, 
format)))
+   copy_to_user(up, kp, offsetof(struct v4l2_create_buffers32, 
format)) ||
+   copy_to_user(up->reserved, kp->reserved, sizeof(kp->reserved)))
return -EFAULT;
return __put_v4l2_format32(>format, >format);
 }
-- 
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

Re: [PATCH] media: add GFP flag to media_*() that could get called in atomic context

[Sakari Ailus]

Hi Mauro,

On Mon, Mar 14, 2016 at 07:13:58AM -0300, Mauro Carvalho Chehab wrote:
> Em Mon, 14 Mar 2016 09:22:37 +0200
> Sakari Ailus  escreveu:
> 
> > Hi Shuah,
> > 
> > On Sat, Mar 12, 2016 at 06:48:09PM -0700, Shuah Khan wrote:
> > > Add GFP flags to media_create_pad_link(), media_create_intf_link(),
> > > media_devnode_create(), and media_add_link() that could get called
> > > in atomic context to allow callers to pass in the right flags for
> > > memory allocation.
> > > 
> > > tree-wide driver changes for media_*() GFP flags change:
> > > Change drivers to add gfpflags to interffaces, media_create_pad_link(),
> > > media_create_intf_link() and media_devnode_create().
> > > 
> > > Signed-off-by: Shuah Khan 
> > > Suggested-by: Mauro Carvalho Chehab   
> > 
> > What's the use case for calling the above functions in an atomic context?
> > 
> 
> ALSA code seems to do a lot of stuff at atomic context. That's what
> happens on my test machine when au0828 gets probed before
> snd-usb-audio:
>   http://pastebin.com/LEX5LD5K
> 
> It seems that ALSA USB probe routine (usb_audio_probe) happens in
> atomic context.

usb_audio_probe() grabs a mutex (register_mutex) on its own. It certainly
cannot be called in atomic context.

In the above log, what I did notice, though, was that because *we* grab
mdev->lock spinlock in media_device_register_entity(), we may not sleep
which is what the notify() callback implementation in au0828 driver does
(for memory allocation).

Could we instead replace mdev->lock by a mutex?

If there is no pressing need to implement atomic memory allocation I simply
wouldn't do it, especially in device initialisation where an allocation
failure will lead to probe failure as well.

-- 
Kind regards,

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

[PATCH] media: v4l2-compat-ioctl32: fix missing reserved field copy in put_v4l2_create32

[Tiffany Lin]

Change-Id: Idac449fae5059a3ce255340e6da491f8bd83af7a
---
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c 
b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
index f38c076..109f687 100644
--- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
+++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
@@ -280,7 +280,8 @@ static int put_v4l2_format32(struct v4l2_format *kp, struct 
v4l2_format32 __user
 static int put_v4l2_create32(struct v4l2_create_buffers *kp, struct 
v4l2_create_buffers32 __user *up)
 {
if (!access_ok(VERIFY_WRITE, up, sizeof(struct v4l2_create_buffers32)) 
||
-   copy_to_user(up, kp, offsetof(struct v4l2_create_buffers32, 
format)))
+   copy_to_user(up, kp, offsetof(struct v4l2_create_buffers32, 
format)) ||
+   copy_to_user(up->reserved, kp->reserved, sizeof(kp->reserved)))
return -EFAULT;
return __put_v4l2_format32(>format, >format);
 }
-- 
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

Re: Any reason why media_entity_pads_init() isn't void?

[Sakari Ailus]

Hi Hans,

On Mon, Mar 14, 2016 at 09:25:51AM +0100, Hans Verkuil wrote:
> I was fixing a sparse warning in media_entity_pads_init() and I noticed
> that that function always returns 0. Any reason why this can't be changed
> to a void function?

I was thinking of the same function but I had a different question: why
would one call this *after* entity->graph_obj.mdev is set? It is set by
media_device_register_entity(), but once mdev it's set, you're not expected
to call pads_init anymore...

I'm fine making this return void.

> 
> That return value is checked a zillion times in the media code. By making
> it void it should simplify code all over.
> 
> See e.g. uvc_mc_init_entity in drivers/media/usb/uvc/uvc_entity.c: that
> whole function can become a void function itself.

-- 
Regards,

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: [PATCH] media: add GFP flag to media_*() that could get called in atomic context

[Takashi Iwai]

On Mon, 14 Mar 2016 11:13:58 +0100,
Mauro Carvalho Chehab wrote:
> 
> Em Mon, 14 Mar 2016 09:22:37 +0200
> Sakari Ailus  escreveu:
> 
> > Hi Shuah,
> > 
> > On Sat, Mar 12, 2016 at 06:48:09PM -0700, Shuah Khan wrote:
> > > Add GFP flags to media_create_pad_link(), media_create_intf_link(),
> > > media_devnode_create(), and media_add_link() that could get called
> > > in atomic context to allow callers to pass in the right flags for
> > > memory allocation.
> > > 
> > > tree-wide driver changes for media_*() GFP flags change:
> > > Change drivers to add gfpflags to interffaces, media_create_pad_link(),
> > > media_create_intf_link() and media_devnode_create().
> > > 
> > > Signed-off-by: Shuah Khan 
> > > Suggested-by: Mauro Carvalho Chehab   
> > 
> > What's the use case for calling the above functions in an atomic context?
> > 
> 
> ALSA code seems to do a lot of stuff at atomic context. That's what
> happens on my test machine when au0828 gets probed before
> snd-usb-audio:
>   http://pastebin.com/LEX5LD5K
> 
> It seems that ALSA USB probe routine (usb_audio_probe) happens in
> atomic context.

No, usb_audio_probe() doesn't take a lock.  But
media_device_register_entity() seems taking a spinlock, instead.


Takashi
--
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: Any reason why media_entity_pads_init() isn't void?

[Mauro Carvalho Chehab]

Em Mon, 14 Mar 2016 09:25:51 +0100
Hans Verkuil  escreveu:

> I was fixing a sparse warning in media_entity_pads_init() and I noticed that 
> that
> function always returns 0. Any reason why this can't be changed to a void 
> function?
> 
> That return value is checked a zillion times in the media code. By making it 
> void
> it should simplify code all over.
> 
> See e.g. uvc_mc_init_entity in drivers/media/usb/uvc/uvc_entity.c: that whole
> function can become a void function itself.


This function were called media_entity_init(), and it used to have a
code that would allocate the links array. The way it is right now, I
don't see any reason to keep returning an error code.

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: [PATCH] media: add GFP flag to media_*() that could get called in atomic context

[Mauro Carvalho Chehab]

Em Mon, 14 Mar 2016 09:22:37 +0200
Sakari Ailus  escreveu:

> Hi Shuah,
> 
> On Sat, Mar 12, 2016 at 06:48:09PM -0700, Shuah Khan wrote:
> > Add GFP flags to media_create_pad_link(), media_create_intf_link(),
> > media_devnode_create(), and media_add_link() that could get called
> > in atomic context to allow callers to pass in the right flags for
> > memory allocation.
> > 
> > tree-wide driver changes for media_*() GFP flags change:
> > Change drivers to add gfpflags to interffaces, media_create_pad_link(),
> > media_create_intf_link() and media_devnode_create().
> > 
> > Signed-off-by: Shuah Khan 
> > Suggested-by: Mauro Carvalho Chehab   
> 
> What's the use case for calling the above functions in an atomic context?
> 

ALSA code seems to do a lot of stuff at atomic context. That's what
happens on my test machine when au0828 gets probed before
snd-usb-audio:
http://pastebin.com/LEX5LD5K

It seems that ALSA USB probe routine (usb_audio_probe) happens in
atomic context.

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: FW: [PATCH v5 0/8] Add MT8173 Video Encoder Driver and VPU Driver

[Hans Verkuil]

On 03/14/2016 10:04 AM, tiffany lin wrote:
> On Mon, 2016-03-14 at 08:21 +0100, Hans Verkuil wrote:
>> On 03/14/2016 08:12 AM, tiffany lin wrote:
>>> Hi Hans,
>>>
>>> After change to use "v4l-utils.git master branch", "V4l2-compliance
>>> -d /dev/video1" fail on "fail: v4l2-test-buffers.cpp(555):
>>> check_0(crbufs.reserved, sizeof(crbufs.reserved))".
>>>
>>> Check the source code and found
>>>
>>> memset(, 0xff, sizeof(crbufs));   -> crbufs to 0xff
>>> node->g_fmt(crbufs.format, i);
>>> crbufs.count = 0;
>>> crbufs.memory = m;
>>> fail_on_test(doioctl(node, VIDIOC_CREATE_BUFS, ));   
>>> fail_on_test(check_0(crbufs.reserved, sizeof(crbufs.reserved)));
>>> fail_on_test(crbufs.index != q.g_buffers());
>>>
>>> crbufs is initialized to fill with 0xff and after VIDIOC_CREATE_BUFS,
>>> crbufs.reserved field should be 0x0. But v4l2_m2m_create_bufs and
>>> vb2_create_bufs do not process reserved filed.
>>> Do we really need to check reserved filed filled with 0x0? Or we need to
>>> change vb2_create_bufs to fix this issue?
>>
>> The reserved field is zeroed in v4l_create_bufs() in v4l2-ioctl.c, so even 
>> before
>> vb2_create_bufs et al is called.
>>
>> The fact that it is no longer zeroed afterwards suggests that someone is 
>> messing
>> with the reserved field.
>>
>> You'll have to do a bit more digging, I'm afraid.
>>
> Hi Hans,
> 
> Thanks for your information.
> I found the root cause is in "put_v4l2_create32".
> It do not copy reserved field from kernel space to user space.
> After modification,"test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK"
> 
> diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> index f38c076..109f687 100644
> --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> @@ -280,7 +280,8 @@ static int put_v4l2_format32(struct v4l2_format *kp,
> struct v4l2_format32 __user
>  static int put_v4l2_create32(struct v4l2_create_buffers *kp, struct
> v4l2_create_buffers32 __user *up)
>  {
> if (!access_ok(VERIFY_WRITE, up, sizeof(struct
> v4l2_create_buffers32)) ||
> -   copy_to_user(up, kp, offsetof(struct v4l2_create_buffers32,
> format)))
> +   copy_to_user(up, kp, offsetof(struct v4l2_create_buffers32,
> format)) ||
> +   copy_to_user(up->reserved, kp->reserved,
> sizeof(kp->reserved)))
> return -EFAULT;
> return __put_v4l2_format32(>format, >format);
>  }

Yup, that's the cause. Can you post this as a 'proper' patch to the mailinglist?

I'll take it for kernel 4.6 (and I'll add a CC to the stable mailinglist to
get it backported as well).

Thanks!

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: FW: [PATCH v5 0/8] Add MT8173 Video Encoder Driver and VPU Driver

[tiffany lin]

On Mon, 2016-03-14 at 08:21 +0100, Hans Verkuil wrote:
> On 03/14/2016 08:12 AM, tiffany lin wrote:
> > Hi Hans,
> > 
> > After change to use "v4l-utils.git master branch", "V4l2-compliance
> > -d /dev/video1" fail on "fail: v4l2-test-buffers.cpp(555):
> > check_0(crbufs.reserved, sizeof(crbufs.reserved))".
> > 
> > Check the source code and found
> > 
> > memset(, 0xff, sizeof(crbufs));   -> crbufs to 0xff
> > node->g_fmt(crbufs.format, i);
> > crbufs.count = 0;
> > crbufs.memory = m;
> > fail_on_test(doioctl(node, VIDIOC_CREATE_BUFS, ));   
> > fail_on_test(check_0(crbufs.reserved, sizeof(crbufs.reserved)));
> > fail_on_test(crbufs.index != q.g_buffers());
> > 
> > crbufs is initialized to fill with 0xff and after VIDIOC_CREATE_BUFS,
> > crbufs.reserved field should be 0x0. But v4l2_m2m_create_bufs and
> > vb2_create_bufs do not process reserved filed.
> > Do we really need to check reserved filed filled with 0x0? Or we need to
> > change vb2_create_bufs to fix this issue?
> 
> The reserved field is zeroed in v4l_create_bufs() in v4l2-ioctl.c, so even 
> before
> vb2_create_bufs et al is called.
> 
> The fact that it is no longer zeroed afterwards suggests that someone is 
> messing
> with the reserved field.
> 
> You'll have to do a bit more digging, I'm afraid.
> 
Hi Hans,

Thanks for your information.
I found the root cause is in "put_v4l2_create32".
It do not copy reserved field from kernel space to user space.
After modification,"test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK"

diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
index f38c076..109f687 100644
--- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
+++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
@@ -280,7 +280,8 @@ static int put_v4l2_format32(struct v4l2_format *kp,
struct v4l2_format32 __user
 static int put_v4l2_create32(struct v4l2_create_buffers *kp, struct
v4l2_create_buffers32 __user *up)
 {
if (!access_ok(VERIFY_WRITE, up, sizeof(struct
v4l2_create_buffers32)) ||
-   copy_to_user(up, kp, offsetof(struct v4l2_create_buffers32,
format)))
+   copy_to_user(up, kp, offsetof(struct v4l2_create_buffers32,
format)) ||
+   copy_to_user(up->reserved, kp->reserved,
sizeof(kp->reserved)))
return -EFAULT;
return __put_v4l2_format32(>format, >format);
 }

best regards,
Tiffany

> 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

Any reason why media_entity_pads_init() isn't void?

[Hans Verkuil]

I was fixing a sparse warning in media_entity_pads_init() and I noticed that 
that
function always returns 0. Any reason why this can't be changed to a void 
function?

That return value is checked a zillion times in the media code. By making it 
void
it should simplify code all over.

See e.g. uvc_mc_init_entity in drivers/media/usb/uvc/uvc_entity.c: that whole
function can become a void function itself.

Regards,

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

[PATCH] media-entity: fix sparse warning in media_entity_pads_init()

[Hans Verkuil]

Fix this sparse warning:

drivers/media/media-entity.c:212:5: warning: context imbalance in 
'media_entity_pads_init' - different lock contexts for basic block

Signed-off-by: Hans Verkuil 

diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index e95070b..be29d62 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -217,20 +217,19 @@ int media_entity_pads_init(struct media_entity *entity, 
u16 num_pads,

entity->num_pads = num_pads;
entity->pads = pads;
-
-   if (mdev)
-   spin_lock(>lock);
-
for (i = 0; i < num_pads; i++) {
pads[i].entity = entity;
pads[i].index = i;
-   if (mdev)
-   media_gobj_create(mdev, MEDIA_GRAPH_PAD,
-   >pads[i].graph_obj);
}

-   if (mdev)
-   spin_unlock(>lock);
+   if (mdev == NULL)
+   return 0;
+
+   spin_lock(>lock);
+   for (i = 0; i < num_pads; i++)
+   media_gobj_create(mdev, MEDIA_GRAPH_PAD,
+ >pads[i].graph_obj);
+   spin_unlock(>lock);

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

Re: [PATCH] Add tw5864 driver

[Hans Verkuil]

Hi Andrey,

See below for a quick review of the code.

I agree with Greg's comment why this is added to staging instead of 
drivers/media/pci?

When you post the v2 patch, can you add the output of 'v4l2-compliance -s' to 
the
cover letter? Please use the latest v4l2-compliance version from the 
v4l-utils.git
repository when testing.

On 03/14/2016 02:59 AM, Andrey Utkin wrote:
> Support for boards based on Techwell TW5864 chip which provides
> multichannel video & audio grabbing and encoding (H.264, MJPEG,
> ADPCM G.726).
> 
> Signed-off-by: Andrey Utkin 
> Tested-by: Andrey Utkin 
> ---
>  MAINTAINERS  |7 +
>  drivers/staging/media/Kconfig|2 +
>  drivers/staging/media/Makefile   |1 +
>  drivers/staging/media/tw5864/Kconfig |   11 +
>  drivers/staging/media/tw5864/Makefile|3 +
>  drivers/staging/media/tw5864/tw5864-bs.h |  154 ++
>  drivers/staging/media/tw5864/tw5864-config.c |  359 +
>  drivers/staging/media/tw5864/tw5864-core.c   |  453 ++
>  drivers/staging/media/tw5864/tw5864-h264.c   |  183 +++
>  drivers/staging/media/tw5864/tw5864-reg.h| 2200 
> ++
>  drivers/staging/media/tw5864/tw5864-tables.h |  237 +++
>  drivers/staging/media/tw5864/tw5864-video.c  | 1364 
>  drivers/staging/media/tw5864/tw5864.h|  280 
>  include/linux/pci_ids.h  |1 +
>  14 files changed, 5255 insertions(+)
>  create mode 100644 drivers/staging/media/tw5864/Kconfig
>  create mode 100644 drivers/staging/media/tw5864/Makefile
>  create mode 100644 drivers/staging/media/tw5864/tw5864-bs.h
>  create mode 100644 drivers/staging/media/tw5864/tw5864-config.c
>  create mode 100644 drivers/staging/media/tw5864/tw5864-core.c
>  create mode 100644 drivers/staging/media/tw5864/tw5864-h264.c
>  create mode 100644 drivers/staging/media/tw5864/tw5864-reg.h
>  create mode 100644 drivers/staging/media/tw5864/tw5864-tables.h
>  create mode 100644 drivers/staging/media/tw5864/tw5864-video.c
>  create mode 100644 drivers/staging/media/tw5864/tw5864.h
> 



> diff --git a/drivers/staging/media/tw5864/tw5864-bs.h 
> b/drivers/staging/media/tw5864/tw5864-bs.h
> new file mode 100644
> index 000..8c1df7a
> --- /dev/null
> +++ b/drivers/staging/media/tw5864/tw5864-bs.h
> @@ -0,0 +1,154 @@



> +static inline void bs_direct_write(struct bs *s, u8 value)
> +{
> + *s->p = value;
> + s->p++;
> + s->i_left = 8;
> +}
> +
> +static inline void bs_write(struct bs *s, int i_count, u32 i_bits)

This one is a bit too big to be an inline IMHO.

> +{
> + if (s->p >= s->p_end - 4)
> + return;
> + while (i_count > 0) {
> + if (i_count < 32)
> + i_bits &= (1 << i_count) - 1;
> + if (i_count < s->i_left) {
> + *s->p = (*s->p << i_count) | i_bits;
> + s->i_left -= i_count;
> + break;
> + }
> + *s->p = (*s->p << s->i_left) | (i_bits >> (i_count -
> +s->i_left));
> + i_count -= s->i_left;
> + s->p++;
> + s->i_left = 8;
> + }
> +}
> +



> diff --git a/drivers/staging/media/tw5864/tw5864-config.c 
> b/drivers/staging/media/tw5864/tw5864-config.c
> new file mode 100644
> index 000..ff3e308
> --- /dev/null
> +++ b/drivers/staging/media/tw5864/tw5864-config.c
> @@ -0,0 +1,359 @@
> +/*
> + *  TW5864 driver - analog decoders configuration functions
> + *
> + *  Copyright (C) 2015 Bluecherry, LLC 
> + *  Author: Andrey Utkin 
> + *
> + *  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.
> + */
> +
> +#include "tw5864.h"
> +#include "tw5864-reg.h"
> +
> +#define TW5864_IIC_TIMEOUT  (3)
> +
> +static unsigned char tbl_pal_tw2864_common[] __used = {

Just use static const and remove the __used.

It would be nice to have a short comment before each array that gives a bit
more information.

> + 0x00, 0x00, 0x64, 0x11,
> + 0x80, 0x80, 0x00, 0x12,
> + 0x12, 0x20, 0x0a, 0xD0,
> + 0x00, 0x00, 0x07, 0x7F,
> +};
> +



> +static int __used i2c_multi_read(struct tw5864_dev *dev, u8 devid, u8 devfn,
> +  u8 *buf, u32 count)

Again, what's up with the __used? Just remove it.

> +{
> + int i = 0;
> 

Re: [PATCH] media: add GFP flag to media_*() that could get called in atomic context

[Sakari Ailus]

Hi Shuah,

On Sat, Mar 12, 2016 at 06:48:09PM -0700, Shuah Khan wrote:
> Add GFP flags to media_create_pad_link(), media_create_intf_link(),
> media_devnode_create(), and media_add_link() that could get called
> in atomic context to allow callers to pass in the right flags for
> memory allocation.
> 
> tree-wide driver changes for media_*() GFP flags change:
> Change drivers to add gfpflags to interffaces, media_create_pad_link(),
> media_create_intf_link() and media_devnode_create().
> 
> Signed-off-by: Shuah Khan 
> Suggested-by: Mauro Carvalho Chehab 

What's the use case for calling the above functions in an atomic context?

-- 
Regards,

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: FW: [PATCH v5 0/8] Add MT8173 Video Encoder Driver and VPU Driver

[Hans Verkuil]

On 03/14/2016 08:12 AM, tiffany lin wrote:
> Hi Hans,
> 
> After change to use "v4l-utils.git master branch", "V4l2-compliance
> -d /dev/video1" fail on "fail: v4l2-test-buffers.cpp(555):
> check_0(crbufs.reserved, sizeof(crbufs.reserved))".
> 
> Check the source code and found
> 
>   memset(, 0xff, sizeof(crbufs));   -> crbufs to 0xff
> node->g_fmt(crbufs.format, i);
> crbufs.count = 0;
> crbufs.memory = m;
> fail_on_test(doioctl(node, VIDIOC_CREATE_BUFS, ));   
> fail_on_test(check_0(crbufs.reserved, sizeof(crbufs.reserved)));
> fail_on_test(crbufs.index != q.g_buffers());
> 
> crbufs is initialized to fill with 0xff and after VIDIOC_CREATE_BUFS,
> crbufs.reserved field should be 0x0. But v4l2_m2m_create_bufs and
> vb2_create_bufs do not process reserved filed.
> Do we really need to check reserved filed filled with 0x0? Or we need to
> change vb2_create_bufs to fix this issue?

The reserved field is zeroed in v4l_create_bufs() in v4l2-ioctl.c, so even 
before
vb2_create_bufs et al is called.

The fact that it is no longer zeroed afterwards suggests that someone is messing
with the reserved field.

You'll have to do a bit more digging, I'm afraid.

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: FW: [PATCH v5 0/8] Add MT8173 Video Encoder Driver and VPU Driver

[tiffany lin]

On Mon, 2016-03-14 at 14:48 +0800, PoChun Lin (林柏君) wrote:
> 
> -Original Message-
> From: tiffany lin [mailto:tiffany@mediatek.com] 
> Sent: Wednesday, February 24, 2016 6:53 PM
> To: Hans Verkuil
> Cc: Hans Verkuil; daniel.thomp...@linaro.org; Rob Herring; Mauro Carvalho 
> Chehab; Matthias Brugger; Daniel Kurtz; Pawel Osciak; Eddie Huang (黃智傑); 
> Yingjoe Chen (陳英洲); devicet...@vger.kernel.org; linux-ker...@vger.kernel.org; 
> linux-arm-ker...@lists.infradead.org; linux-media@vger.kernel.org; 
> linux-media...@lists.infradead.org; PoChun Lin (林柏君)
> Subject: Re: [PATCH v5 0/8] Add MT8173 Video Encoder Driver and VPU Driver
> 
> On Wed, 2016-02-24 at 09:30 +0100, Hans Verkuil wrote:
> > Hi Tiffany,
> > 
> > On 02/24/16 09:23, tiffany lin wrote:
> > > Hi Hans,
> > > 
> > > On Tue, 2016-02-23 at 09:47 +0100, Hans Verkuil wrote:
> > >> On 02/23/16 09:11, Tiffany Lin wrote:
> > >>> ==
> > >>>  Introduction
> > >>> ==
> > >>>
> > >>> The purpose of this series is to add the driver for video codec hw 
> > >>> embedded in the Mediatek's MT8173 SoCs.
> > >>> Mediatek Video Codec is able to handle video encoding of in a range of 
> > >>> formats.
> > >>>
> > >>> This patch series also include VPU driver. Mediatek Video Codec 
> > >>> driver rely on VPU driver to load, communicate with VPU.
> > >>>
> > >>> Internally the driver uses videobuf2 framework and MTK IOMMU and MTK 
> > >>> SMI.
> > >>> MTK IOMMU[1] and MTK SMI[2] have not yet been merged, but we 
> > >>> wanted to start discussion about the driver earlier so it could be 
> > >>> merged sooner.
> > >>>
> > >>> [1]https://patchwork.kernel.org/patch/8335461/
> > >>> [2]https://patchwork.kernel.org/patch/7596181/
> > >>
> > >> 
> > >>
> > >>> v4l2-compliance test output:
> > >>> localhost ~ # /usr/bin/v4l2-compliance -d /dev/video1 Driver Info:
> > >>> Driver name   : mtk-vcodec-enc
> > >>> Card type : platform:mt8173
> > >>> Bus info  : platform:mt8173
> > >>> Driver version: 4.4.0
> > >>> Capabilities  : 0x84204000
> > >>> Video Memory-to-Memory Multiplanar
> > >>> Streaming
> > >>> Extended Pix Format
> > >>> Device Capabilities
> > >>> Device Caps   : 0x04204000
> > >>> Video Memory-to-Memory Multiplanar
> > >>> Streaming
> > >>> Extended Pix Format
> > >>>
> > >>> Compliance test for device /dev/video1 (not using libv4l2):
> > >>>
> > >>> Required ioctls:
> > >>> test VIDIOC_QUERYCAP: OK
> > >>>
> > >>> Allow for multiple opens:
> > >>> test second video open: OK
> > >>> test VIDIOC_QUERYCAP: OK
> > >>> test VIDIOC_G/S_PRIORITY: OK
> > >>>
> > >>> Debug ioctls:
> > >>> test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
> > >>> test VIDIOC_LOG_STATUS: OK (Not Supported)
> > >>>
> > >>> Input ioctls:
> > >>> test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
> > >>> test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
> > >>> test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
> > >>> test VIDIOC_ENUMAUDIO: OK (Not Supported)
> > >>> test VIDIOC_G/S/ENUMINPUT: OK (Not Supported)
> > >>> test VIDIOC_G/S_AUDIO: OK (Not Supported)
> > >>> Inputs: 0 Audio Inputs: 0 Tuners: 0
> > >>>
> > >>> Output ioctls:
> > >>> test VIDIOC_G/S_MODULATOR: OK (Not Supported)
> > >>> test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
> > >>> test VIDIOC_ENUMAUDOUT: OK (Not Supported)
> > >>> test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
> > >>> test VIDIOC_G/S_AUDOUT: OK (Not Supported)
> > >>> Outputs: 0 Audio Outputs: 0 Modulators: 0
> > >>>
> > >>> Input/Output configuration ioctls:
> > >>> test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
> > >>> test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
> > >>> test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
> > >>> test VIDIOC_G/S_EDID: OK (Not Supported)
> > >>>
> > >>> Control ioctls:
> > >>> test VIDIOC_QUERYCTRL/MENU: OK
> > >>> test VIDIOC_G/S_CTRL: OK
> > >>> test VIDIOC_G/S/TRY_EXT_CTRLS: OK
> > >>> test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK
> > >>> test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
> > >>> Standard Controls: 12 Private Controls: 0
> > >>>
> > >>> Format ioctls:
> > >>> test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
> > >>> test VIDIOC_G/S_PARM: OK
> > >>> test VIDIOC_G_FBUF: OK (Not Supported)
> > >>> test VIDIOC_G_FMT: OK
> > >>> test VIDIOC_TRY_FMT: OK
> > >>> test VIDIOC_S_FMT: OK
> > >>> test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
> > >>>
> > >>> Codec ioctls:
> > >>> test