cron job: media_tree daily build: ERRORS

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

Results of the daily build of media_tree:

date:   Sat Dec 16 05:00:18 CET 2017
media-tree git hash:45267fed3e55845c5b4b279162b273040ae4f587
media_build git hash:   4058fea6b2507661d66af5298c048d7c55338f42
v4l-utils git hash: b638ef8b15813494e82148dccd9c0411daa214d9
gcc version:i686-linux-gcc (GCC) 7.1.0
sparse version: v0.5.0-3911-g6f737e1f
smatch version: v0.5.0-3911-g6f737e1f
host hardware:  x86_64
host os:4.13.0-164

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-multi: OK
linux-git-arm-pxa: OK
linux-git-arm-stm32: 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: ERRORS
linux-2.6.37.6-i686: ERRORS
linux-2.6.38.8-i686: ERRORS
linux-2.6.39.4-i686: ERRORS
linux-3.0.60-i686: ERRORS
linux-3.1.10-i686: ERRORS
linux-3.2.37-i686: ERRORS
linux-3.3.8-i686: ERRORS
linux-3.4.27-i686: ERRORS
linux-3.5.7-i686: ERRORS
linux-3.6.11-i686: ERRORS
linux-3.7.4-i686: ERRORS
linux-3.8-i686: ERRORS
linux-3.9.2-i686: ERRORS
linux-3.10.1-i686: ERRORS
linux-3.11.1-i686: ERRORS
linux-3.12.67-i686: ERRORS
linux-3.13.11-i686: ERRORS
linux-3.14.9-i686: ERRORS
linux-3.15.2-i686: ERRORS
linux-3.16.7-i686: ERRORS
linux-3.17.8-i686: ERRORS
linux-3.18.7-i686: ERRORS
linux-3.19-i686: ERRORS
linux-4.0.9-i686: ERRORS
linux-4.1.33-i686: ERRORS
linux-4.2.8-i686: ERRORS
linux-4.3.6-i686: ERRORS
linux-4.4.22-i686: ERRORS
linux-4.5.7-i686: ERRORS
linux-4.6.7-i686: ERRORS
linux-4.7.5-i686: ERRORS
linux-4.8-i686: ERRORS
linux-4.9.26-i686: ERRORS
linux-4.10.14-i686: ERRORS
linux-4.11-i686: ERRORS
linux-4.12.1-i686: ERRORS
linux-4.13-i686: ERRORS
linux-4.14-i686: ERRORS
linux-2.6.36.4-x86_64: ERRORS
linux-2.6.37.6-x86_64: ERRORS
linux-2.6.38.8-x86_64: ERRORS
linux-2.6.39.4-x86_64: ERRORS
linux-3.0.60-x86_64: ERRORS
linux-3.1.10-x86_64: ERRORS
linux-3.2.37-x86_64: ERRORS
linux-3.3.8-x86_64: ERRORS
linux-3.4.27-x86_64: ERRORS
linux-3.5.7-x86_64: ERRORS
linux-3.6.11-x86_64: ERRORS
linux-3.7.4-x86_64: ERRORS
linux-3.8-x86_64: ERRORS
linux-3.9.2-x86_64: ERRORS
linux-3.10.1-x86_64: ERRORS
linux-3.11.1-x86_64: ERRORS
linux-3.12.67-x86_64: ERRORS
linux-3.13.11-x86_64: ERRORS
linux-3.14.9-x86_64: ERRORS
linux-3.15.2-x86_64: ERRORS
linux-3.16.7-x86_64: ERRORS
linux-3.17.8-x86_64: ERRORS
linux-3.18.7-x86_64: ERRORS
linux-3.19-x86_64: ERRORS
linux-4.0.9-x86_64: ERRORS
linux-4.1.33-x86_64: ERRORS
linux-4.2.8-x86_64: ERRORS
linux-4.3.6-x86_64: ERRORS
linux-4.4.22-x86_64: ERRORS
linux-4.5.7-x86_64: ERRORS
linux-4.6.7-x86_64: ERRORS
linux-4.7.5-x86_64: ERRORS
linux-4.8-x86_64: ERRORS
linux-4.9.26-x86_64: ERRORS
linux-4.10.14-x86_64: ERRORS
linux-4.11-x86_64: ERRORS
linux-4.12.1-x86_64: ERRORS
linux-4.13-x86_64: ERRORS
linux-4.14-x86_64: ERRORS
apps: OK
spec-git: OK
smatch: OK

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

http://www.xs4all.nl/~hverkuil/spec/index.html


Re: more build failures

2017-12-15 Thread Vincent McIntyre
On Fri, Dec 15, 2017 at 11:41:13PM +1100, Vincent McIntyre wrote:
> 
> ...
> 
> $ make allyesconfig
> make -C /home/me/git/clones/media_build/v4l allyesconfig
> make[1]: Entering directory '/home/me/git/clones/media_build/v4l'
> No version yet, using 4.4.0-103-generic
> make[2]: Entering directory '/home/me/git/clones/media_build/linux'
> Syncing with dir ../media/
> Applying patches for kernel 4.4.0-103-generic
> patch -s -f -N -p1 -i ../backports/api_version.patch
> patch -s -f -N -p1 -i ../backports/pr_fmt.patch
> patch -s -f -N -p1 -i ../backports/debug.patch
> patch -s -f -N -p1 -i ../backports/drx39xxj.patch
> patch -s -f -N -p1 -i ../backports/v4.14_compiler_h.patch
> patch -s -f -N -p1 -i ../backports/v4.14_saa7146_timer_cast.patch
> patch -s -f -N -p1 -i ../backports/v4.14_module_param_call.patch
> patch -s -f -N -p1 -i ../backports/v4.12_revert_solo6x10_copykerneluser.patch
> patch -s -f -N -p1 -i ../backports/v4.10_sched_signal.patch
> 1 out of 1 hunk FAILED
> The text leading up to this was:
> --
> |diff --git a/drivers/staging/media/lirc/lirc_zilog.c 
> b/drivers/staging/media/lirc/lirc_zilog.c
> |index 015e41bd036e..fd61081b47d9 100644
> |--- a/drivers/staging/media/lirc/lirc_zilog.c
> |+++ b/drivers/staging/media/lirc/lirc_zilog.c
> --
> No file to patch.  Skipping patch.
> 1 out of 1 hunk ignored
> Makefile:130: recipe for target 'apply_patches' failed
> make[2]: *** [apply_patches] Error 1
> make[2]: Leaving directory '/home/me/git/clones/media_build/linux'
> Makefile:374: recipe for target 'allyesconfig' failed
> make[1]: *** [allyesconfig] Error 2
> make[1]: Leaving directory '/home/me/git/clones/media_build/v4l'
> Makefile:26: recipe for target 'allyesconfig' failed
> make: *** [allyesconfig] Error 2
> can't select all drivers at ./build line 525
> + status=29
> + date
> Friday 15 December  23:29:17 AEDT 2017
> + [ 0 = 29 ]

I managed to get past the failure above with this change

 - media: rc: move ir-lirc-codec.c contents into lirc_dev.c
   media: lirc: remove last remnants of lirc kapi
 - Sean removed lirc_zilog.c, so it no longer needs patching

--- a/backports/v4.10_sched_signal.patch
+++ b/backports/v4.10_sched_signal.patch
@@ -195,19 +195,6 @@ index 0e8025b7b4dd..8c59d4f53200 100644
  #include 
  #include 
  #include 
-diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
-index db1e7b70c998..fc03068e22b5 100644
 a/drivers/media/rc/lirc_dev.c
-+++ b/drivers/media/rc/lirc_dev.c
-@@ -18,7 +18,7 @@
- #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
- 
- #include 
--#include 
-+#include 
- #include 
- #include 
- #include 
 diff --git a/drivers/media/usb/cpia2/cpia2_core.c 
b/drivers/media/usb/cpia2/cpia2_core.c
 index 0efba0da0a45..5d8aa65ab40b 100644
 --- a/drivers/media/usb/cpia2/cpia2_core.c
@@ -246,19 +233,6 @@ index 0b5c43f7e020..36bd904946bd 100644
  #include 
  #include 
  
-diff --git a/drivers/staging/media/lirc/lirc_zilog.c 
b/drivers/staging/media/lirc/lirc_zilog.c
-index 015e41bd036e..fd61081b47d9 100644
 a/drivers/staging/media/lirc/lirc_zilog.c
-+++ b/drivers/staging/media/lirc/lirc_zilog.c
-@@ -42,7 +42,7 @@
- #include 
- #include 
- #include 
--#include 
-+#include 
- #include 
- #include 
- #include 


However it falls over later in a way I don't think I can help with.

...
  CC [M]  /home/me/git/clones/media_build/v4l/pvrusb2-i2c-core.o
  CC [M]  /home/me/git/clones/media_build/v4l/pvrusb2-audio.o
  CC [M]  /home/me/git/clones/media_build/v4l/pvrusb2-encoder.o
  CC [M]  /home/me/git/clones/media_build/v4l/pvrusb2-video-v4l.o
  CC [M]  /home/me/git/clones/media_build/v4l/pvrusb2-eeprom.o
  CC [M]  /home/me/git/clones/media_build/v4l/pvrusb2-main.o
  CC [M]  /home/me/git/clones/media_build/v4l/pvrusb2-hdw.o
/home/me/git/clones/media_build/v4l/pvrusb2-hdw.c: In function 
'pvr2_send_request_ex':
/home/me/git/clones/media_build/v4l/pvrusb2-hdw.c:3651:7: error: implicit 
declaration of function 'usb_urb_ep_type_check' 
[-Werror=implicit-function-declaration]
   if (usb_urb_ep_type_check(hdw->ctl_write_urb)) {
   ^
cc1: some warnings being treated as errors
scripts/Makefile.build:258: recipe for target 
'/home/me/git/clones/media_build/v4l/pvrusb2-hdw.o' failed
make[3]: *** [/home/me/git/clones/media_build/v4l/pvrusb2-hdw.o] Error 1
Makefile:1423: recipe for target '_module_/home/me/git/clones/media_build/v4l' 
failed
make[2]: *** [_module_/home/me/git/clones/media_build/v4l] Error 2
make[2]: Leaving directory '/usr/src/linux-headers-4.4.0-103-generic'
Makefile:51: recipe for target 'default' failed
make[1]: *** [default] Error 2
make[1]: Leaving directory '/home/me/git/clones/media_build/v4l'
Makefile:26: recipe for target 'all' failed
make: *** [all] Error 2

Cheers
Vince


[RFC 4/5] ARM: dts: sun8i: a83t: Add support for the ir interface

2017-12-15 Thread Philipp Rossak
The ir interface is like the H3 at 0x01f02000 located and is exactly
the same. This patch adds support for the ir interface on the A83T.

Signed-off-by: Philipp Rossak 
---
 arch/arm/boot/dts/sun8i-a83t.dtsi | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi 
b/arch/arm/boot/dts/sun8i-a83t.dtsi
index 5edb645b506f..5dbf2f0891c1 100644
--- a/arch/arm/boot/dts/sun8i-a83t.dtsi
+++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
@@ -470,6 +470,16 @@
#reset-cells = <1>;
};
 
+   ir: ir@01f02000 {
+   compatible = "allwinner,sun5i-a13-ir";
+   clocks = <_ccu CLK_APB0_IR>, <_ccu CLK_IR>;
+   clock-names = "apb", "ir";
+   resets = <_ccu RST_APB0_IR>;
+   interrupts = ;
+   reg = <0x01f02000 0x40>;
+   status = "disabled";
+   };
+
r_pio: pinctrl@1f02c00 {
compatible = "allwinner,sun8i-a83t-r-pinctrl";
reg = <0x01f02c00 0x400>;
-- 
2.11.0



[RFC 5/5] ARM: dts: sun8i: a83t: bananapi-m3: Enable IR controller

2017-12-15 Thread Philipp Rossak
The Bananapi M3 has an onboard IR receiver.
This enables the onboard IR receiver subnode.
Other than the other IR receivers this one needs a base clock frequency
of 300 Hz (3 MHz), to be able to work.

Signed-off-by: Philipp Rossak 
---
 arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts | 7 +++
 arch/arm/boot/dts/sun8i-a83t.dtsi| 2 +-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts 
b/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts
index c606af3dbfed..2c92c501cd59 100644
--- a/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts
+++ b/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts
@@ -88,6 +88,13 @@
/* TODO GL830 USB-to-SATA bridge downstream w/ GPIO power controls */
 };
 
+ {
+   pinctrl-names = "default";
+   pinctrl-0 = <_pins_a>;
+   base-clk-frequency = <300>;
+   status = "okay";
+};
+
  {
pinctrl-names = "default";
pinctrl-0 = <_pins>;
diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi 
b/arch/arm/boot/dts/sun8i-a83t.dtsi
index 5dbf2f0891c1..679ce3a66b4b 100644
--- a/arch/arm/boot/dts/sun8i-a83t.dtsi
+++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
@@ -470,7 +470,7 @@
#reset-cells = <1>;
};
 
-   ir: ir@01f02000 {
+   ir: ir@1f02000 {
compatible = "allwinner,sun5i-a13-ir";
clocks = <_ccu CLK_APB0_IR>, <_ccu CLK_IR>;
clock-names = "apb", "ir";
-- 
2.11.0



[RFC 2/5] [media] dt: bindings: Update binding documentation for sunxi IR controller

2017-12-15 Thread Philipp Rossak
This patch updates documentation for Device-Tree bindings for sunxi IR
controller and adds the new requiered property for the base clock frequency.

Signed-off-by: Philipp Rossak 
---
 Documentation/devicetree/bindings/media/sunxi-ir.txt | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/sunxi-ir.txt 
b/Documentation/devicetree/bindings/media/sunxi-ir.txt
index 91648c569b1e..5f4960c61245 100644
--- a/Documentation/devicetree/bindings/media/sunxi-ir.txt
+++ b/Documentation/devicetree/bindings/media/sunxi-ir.txt
@@ -1,12 +1,13 @@
 Device-Tree bindings for SUNXI IR controller found in sunXi SoC family
 
 Required properties:
-- compatible   : "allwinner,sun4i-a10-ir" or "allwinner,sun5i-a13-ir"
-- clocks   : list of clock specifiers, corresponding to
- entries in clock-names property;
-- clock-names  : should contain "apb" and "ir" entries;
-- interrupts   : should contain IR IRQ number;
-- reg  : should contain IO map address for IR.
+- compatible : "allwinner,sun4i-a10-ir" or "allwinner,sun5i-a13-ir"
+- clocks : list of clock specifiers, corresponding to
+   entries in clock-names property;
+- clock-names: should contain "apb" and "ir" entries;
+- interrupts : should contain IR IRQ number;
+- reg: should contain IO map address for IR.
+- base-clk-frequency  : should contain the base clock frequency
 
 Optional properties:
 - linux,rc-map-name: see rc.txt file in the same directory.
@@ -21,5 +22,6 @@ ir0: ir@1c21800 {
resets = <_rst 1>;
interrupts = <0 5 1>;
reg = <0x01C21800 0x40>;
+   base-clk-frequency = <800>;
linux,rc-map-name = "rc-rc6-mce";
 };
-- 
2.11.0



[RFC 3/5] ARM: dts: sun8i: a83t: Add the ir pin for the A83T

2017-12-15 Thread Philipp Rossak
The CIR Pin of the A83T is located at PL12

Signed-off-by: Philipp Rossak 
---
 arch/arm/boot/dts/sun8i-a83t.dtsi | 5 +
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi 
b/arch/arm/boot/dts/sun8i-a83t.dtsi
index 19acae1b4089..5edb645b506f 100644
--- a/arch/arm/boot/dts/sun8i-a83t.dtsi
+++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
@@ -488,6 +488,11 @@
drive-strength = <20>;
bias-pull-up;
};
+
+   ir_pins_a: ir@0 {
+   pins = "PL12";
+   function = "s_cir_rx";
+   };
};
 
r_rsb: rsb@1f03400 {
-- 
2.11.0



[RFC 1/5] [media] rc: update sunxi-ir driver to get base frequency from devicetree

2017-12-15 Thread Philipp Rossak
This patch updates the sunxi-ir driver to set the ir base clock from
devicetree.

This is neccessary since there are different ir recievers on the
market, that operate with different frequencys. So this value needs to
be set depending on the attached receiver.

Signed-off-by: Philipp Rossak 
---
 drivers/media/rc/sunxi-cir.c | 20 +++-
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/media/rc/sunxi-cir.c b/drivers/media/rc/sunxi-cir.c
index 97f367b446c4..55b53d6463e9 100644
--- a/drivers/media/rc/sunxi-cir.c
+++ b/drivers/media/rc/sunxi-cir.c
@@ -72,12 +72,6 @@
 /* CIR_REG register idle threshold */
 #define REG_CIR_ITHR(val)(((val) << 8) & (GENMASK(15, 8)))
 
-/* Required frequency for IR0 or IR1 clock in CIR mode */
-#define SUNXI_IR_BASE_CLK 800
-/* Frequency after IR internal divider  */
-#define SUNXI_IR_CLK  (SUNXI_IR_BASE_CLK / 64)
-/* Sample period in ns */
-#define SUNXI_IR_SAMPLE   (10ul / SUNXI_IR_CLK)
 /* Noise threshold in samples  */
 #define SUNXI_IR_RXNOISE  1
 /* Idle Threshold in samples */
@@ -122,7 +116,7 @@ static irqreturn_t sunxi_ir_irq(int irqno, void *dev_id)
/* for each bit in fifo */
dt = readb(ir->base + SUNXI_IR_RXFIFO_REG);
rawir.pulse = (dt & 0x80) != 0;
-   rawir.duration = ((dt & 0x7f) + 1) * SUNXI_IR_SAMPLE;
+   rawir.duration = ((dt & 0x7f) + 1) * 
ir->rc->rx_resolution;
ir_raw_event_store_with_filter(ir->rc, );
}
}
@@ -148,6 +142,7 @@ static int sunxi_ir_probe(struct platform_device *pdev)
struct device_node *dn = dev->of_node;
struct resource *res;
struct sunxi_ir *ir;
+   u32 b_clk_freq;
 
ir = devm_kzalloc(dev, sizeof(struct sunxi_ir), GFP_KERNEL);
if (!ir)
@@ -172,6 +167,12 @@ static int sunxi_ir_probe(struct platform_device *pdev)
return PTR_ERR(ir->clk);
}
 
+   /* Required frequency for IR0 or IR1 clock in CIR mode */
+   if (of_property_read_u32(dn, "base-clk-frequency", _clk_freq)) {
+   dev_err(dev, "failed to get ir base clock frequency.\n");
+   return -ENODATA;
+   }
+
/* Reset (optional) */
ir->rst = devm_reset_control_get_optional_exclusive(dev, NULL);
if (IS_ERR(ir->rst))
@@ -180,7 +181,7 @@ static int sunxi_ir_probe(struct platform_device *pdev)
if (ret)
return ret;
 
-   ret = clk_set_rate(ir->clk, SUNXI_IR_BASE_CLK);
+   ret = clk_set_rate(ir->clk, b_clk_freq);
if (ret) {
dev_err(dev, "set ir base clock failed!\n");
goto exit_reset_assert;
@@ -225,7 +226,8 @@ static int sunxi_ir_probe(struct platform_device *pdev)
ir->rc->map_name = ir->map_name ?: RC_MAP_EMPTY;
ir->rc->dev.parent = dev;
ir->rc->allowed_protocols = RC_PROTO_BIT_ALL_IR_DECODER;
-   ir->rc->rx_resolution = SUNXI_IR_SAMPLE;
+   /* Frequency after IR internal divider with sample period in ns */
+   ir->rc->rx_resolution = (10ul / (b_clk_freq / 64));
ir->rc->timeout = MS_TO_NS(SUNXI_IR_TIMEOUT);
ir->rc->driver_name = SUNXI_IR_DEV;
 
-- 
2.11.0



[RFC 0/5] IR support for A83T - sunxi-ir driver update

2017-12-15 Thread Philipp Rossak
This patch series adds support for the sunxi A83T ir module and enhances the 
sunxi-ir driver.
Right now the base clock frequency for the ir driver is a hard coded define and 
is set to 8 MHz.
This works for the most common ir receivers. On the Sinovoip Bananapi M3 the ir 
receiver needs,
a 3 MHz base clock frequency to work without problems with this driver (like in 
the legacy kernel).

To fix this issue I reworked the driver that this value could be set over the 
devicetree.

About 37 devices would have a devicetree change if this patch series would be 
applied.
Therfore I would like to ask you to give me some feedback about the patch 
series, before I finialize it.


Thanks in advance!

Philipp


Philipp Rossak (5):
  [media] rc: update sunxi-ir driver to get base frequency from
devicetree
  [media] dt: bindings: Update binding documentation for sunxi IR
controller
  ARM: dts: sun8i: a83t: Add the ir pin for the A83T
  ARM: dts: sun8i: a83t: Add support for the ir interface
  ARM: dts: sun8i: a83t: bananapi-m3: Enable IR controller

 Documentation/devicetree/bindings/media/sunxi-ir.txt | 14 --
 arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts |  7 +++
 arch/arm/boot/dts/sun8i-a83t.dtsi| 15 +++
 drivers/media/rc/sunxi-cir.c | 20 +++-
 4 files changed, 41 insertions(+), 15 deletions(-)

-- 
2.11.0



Re: [PATCH 1/2] media: dt-bindings: coda: Add compatible for CodaHx4 on i.MX51

2017-12-15 Thread Rob Herring
On Wed, Dec 13, 2017 at 03:09:17PM +0100, Philipp Zabel wrote:
> Add a compatible for the CodaHx4 VPU used on i.MX51.
> 
> Signed-off-by: Philipp Zabel 
> ---
>  Documentation/devicetree/bindings/media/coda.txt | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Rob Herring 


[BUG] NULL pointer dereference in pvr2_v4l2_internal_check

2017-12-15 Thread Oleksandr Ostrenko

Dear all,

Unplugging the TV tuner (WinTV HVR-1900) from USB port causes a NULL 
pointer dereference in pvr2_v4l2_internal_check:


[ 2128.129776] usb 1-1: USB disconnect, device number 6
[ 2128.129987] pvrusb2: Device being rendered inoperable
[ 2128.130055] BUG: unable to handle kernel NULL pointer dereference at 
0360

[ 2128.130082] IP: pvr2_v4l2_internal_check+0x41/0x60 [pvrusb2]
[ 2128.130085] PGD 0 P4D 0
[ 2128.130092] Oops:  [#1] PREEMPT SMP
[ 2128.130097] Modules linked in: tda10048 tda18271 tda8290 tuner 
lirc_zilog(C) lirc_dev cx25840 rc_core pvrusb2(O) tveeprom cx2341x 
dvb_core v4l2_common rfcomm af_packet 8021q garp mrp stp llc nf_log_ipv6 
xt_comment nf_log_ipv4 nf_log_common xt_LOG xt_limit ip6t_REJECT 
nf_reject_ipv6 nf_conntrack_ipv6 nf_defrag_ipv6 ipt_REJECT 
nf_reject_ipv4 xt_pkttype xt_tcpudp iptable_filter snd_hda_codec_hdmi 
ip6table_mangle nf_conntrack_netbios_ns nf_conntrack_broadcast 
nf_conntrack_ipv4 nf_defrag_ipv4 ip_tables xt_conntrack nf_conntrack 
ip6table_filter ip6_tables x_tables bnep arc4 xfs libcrc32c 
snd_hda_codec_realtek intel_rapl snd_hda_codec_generic 
x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel kvm iwlmvm 
snd_hda_intel raid1 irqbypass crct10dif_pclmul snd_hda_codec mac80211 
crc32_pclmul snd_hda_core
[ 2128.130176]  ghash_clmulni_intel snd_hwdep pcbc snd_pcm iwlwifi 
snd_timer dell_laptop aesni_intel md_mod hid_multitouch dell_wmi 
iTCO_wdt aes_x86_64 snd rtsx_pci_ms iTCO_vendor_support btusb 
crypto_simd uvcvideo dell_smbios btrtl glue_helper dell_smm_hwmon 
wmi_bmof dcdbas joydev hci_uart cryptd pcspkr cfg80211 videobuf2_vmalloc 
memstick btbcm serdev videobuf2_memops r8169 btqca soundcore btintel 
videobuf2_v4l2 mii int3403_thermal i2c_i801 videobuf2_core videodev 
bluetooth battery ac sparse_keymap ecdh_generic fan thermal idma64 
pinctrl_sunrisepoint pinctrl_intel tpm_crb tpm_tis tpm_tis_core tpm 
processor_thermal_device intel_lpss_acpi intel_soc_dts_iosf 
int3402_thermal dell_rbtn int340x_thermal_zone mei_me rfkill 
int3400_thermal intel_lpss_pci acpi_pad mei acpi_thermal_rel intel_lpss 
intel_pch_thermal
[ 2128.130252]  acpi_als kfifo_buf shpchp industrialio btrfs xor 
zstd_decompress zstd_compress xxhash hid_generic usbhid i915 raid6_pq 
rtsx_pci_sdmmc mmc_core mxm_wmi crc32c_intel i2c_algo_bit drm_kms_helper 
syscopyarea sysfillrect sysimgblt xhci_pci fb_sys_fops serio_raw 
xhci_hcd rtsx_pci drm usbcore wmi video i2c_hid button sg dm_multipath 
dm_mod scsi_dh_rdac scsi_dh_emc scsi_dh_alua
[ 2128.130300] CPU: 6 PID: 2310 Comm: pvrusb2-context Tainted: G 
C O4.14.6-1.g45f120a-default #1
[ 2128.130303] Hardware name: Dell Inc. Inspiron 7559/0H0CC0, BIOS 1.1.8 
04/17/2016

[ 2128.130306] task: 880cae4f6000 task.stack: b3a7c2548000
[ 2128.130320] RIP: 0010:pvr2_v4l2_internal_check+0x41/0x60 [pvrusb2]
[ 2128.130324] RSP: 0018:b3a7c254bec8 EFLAGS: 00010246
[ 2128.130328] RAX:  RBX: 880caf05e780 RCX: 
c0ffe970
[ 2128.130331] RDX: 880c90ca1b60 RSI: 0001 RDI: 

[ 2128.130334] RBP: 880cac83eb00 R08: c1016a78 R09: 
03d2
[ 2128.130337] R10: 03a9 R11: 003d0900 R12: 
b3a7c24ffc18
[ 2128.130340] R13: 880cae4f6000 R14:  R15: 
c1000ae0
[ 2128.130344] FS:  () GS:880cc1d8() 
knlGS:

[ 2128.130347] CS:  0010 DS:  ES:  CR0: 80050033
[ 2128.130350] CR2: 0360 CR3: 00024ec09005 CR4: 
003606e0
[ 2128.130354] DR0:  DR1:  DR2: 

[ 2128.130357] DR3:  DR6: fffe0ff0 DR7: 
0400

[ 2128.130359] Call Trace:
[ 2128.130378]  pvr2_context_thread_func+0xa6/0x2a0 [pvrusb2]
[ 2128.130388]  ? finish_wait+0x80/0x80
[ 2128.130394]  kthread+0x118/0x130
[ 2128.130399]  ? kthread_create_on_node+0x40/0x40
[ 2128.130406]  ret_from_fork+0x25/0x30
[ 2128.130412] Code: 8b 7f 38 e8 d2 e4 ff ff 48 8b 7b 40 e8 c9 e4 ff ff 
48 8b 43 38 48 8b 90 60 03 00 00 48 05 60 03 00 00 48 39 d0 75 d6 48 8b 
43 40 <48> 8b 90 60 03 00 00 48 05 60 03 00 00 48 39 d0 75 c0 48 89 df
[ 2128.130491] RIP: pvr2_v4l2_internal_check+0x41/0x60 [pvrusb2] RSP: 
b3a7c254bec8

[ 2128.130494] CR2: 0360
[ 2128.130499] ---[ end trace b7d1a2a4867177f2 ]---

Upon reconnect the device is no longer recognized by the driver and no 
firmware is uploaded:


[ 2135.323115] usb 1-1: new high-speed USB device number 7 using xhci_hcd
[ 2135.481292] usb 1-1: New USB device found, idVendor=2040, idProduct=7300
[ 2135.481302] usb 1-1: New USB device strings: Mfr=1, Product=2, 
SerialNumber=3

[ 2135.481306] usb 1-1: Product: WinTV
[ 2135.481310] usb 1-1: Manufacturer: Hauppauge
[ 2135.481313] usb 1-1: SerialNumber: 7300-00-F04BADA0
[ 2135.482726] pvrusb2: Hardware description: WinTV HVR-1900 Model 73xxx

This effectively breaks the driver until after a reboot of the kernel. 
Can this be 

[PATCH] [media] netup_unidvb: use PCI_EXP_DEVCTL2_COMP_TIMEOUT macro

2017-12-15 Thread Bjorn Helgaas
From: Bjorn Helgaas 

Use the existing PCI_EXP_DEVCTL2_COMP_TIMEOUT macro instead of hard-coding
the PCIe Completion Timeout Value mask.  No functional change intended.

Signed-off-by: Bjorn Helgaas 
---
 drivers/media/pci/netup_unidvb/netup_unidvb_core.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/pci/netup_unidvb/netup_unidvb_core.c 
b/drivers/media/pci/netup_unidvb/netup_unidvb_core.c
index 11829c0fa138..6cf88a0bf458 100644
--- a/drivers/media/pci/netup_unidvb/netup_unidvb_core.c
+++ b/drivers/media/pci/netup_unidvb/netup_unidvb_core.c
@@ -862,7 +862,7 @@ static int netup_unidvb_initdev(struct pci_dev *pci_dev,
PCI_EXP_DEVCTL_NOSNOOP_EN, 0);
/* Adjust PCIe completion timeout. */
pcie_capability_clear_and_set_word(pci_dev,
-   PCI_EXP_DEVCTL2, 0xf, 0x2);
+   PCI_EXP_DEVCTL2, PCI_EXP_DEVCTL2_COMP_TIMEOUT, 0x2);
 
if (netup_unidvb_request_mmio(pci_dev)) {
dev_err(_dev->dev,



Re: [PATCH] pvrusb2: correctly return V4L2_PIX_FMT_MPEG in enum_fmt

2017-12-15 Thread Oleksandr Ostrenko

Dear Hans,

With your patch applied to kernel 4.14.6 on openSUSE I can get the video 
working, changing channels also works. But I don't have any audio 
output. Even if I just do:


$ cat /dev/video0 > test.mpg

then when I open test.mpg with vlc there is no sound as if it is muted.
I tried 'v4l2-ctl -c mute=0', 'v4l2-ctl --set-audio-input=0; v4l2-ctl 
--set-audio-input=1' so far but nothing changed. Is there something else 
I could try to get audio output?


Best,
Oleksandr

BTW, on Mint with kernel 4.13.0 regardless of the patch I get a kernel 
panic as soon as I plug in the device. Something fishy going on there.


On 14.12.17 10:54, Hans Verkuil wrote:

On 14/12/17 10:52, Oleksandr Ostrenko wrote:

On Thursday, December 14, 2017 12:44:42 AM CET Hans Verkuil wrote:

The pvrusb2 code appears to have a some old workaround code for xawtv that
causes a WARN() due to an unrecognized pixelformat 0 in v4l2_ioctl.c.

Since all other MPEG drivers fill this in correctly, it is a safe assumption
that this particular problem no longer exists.

While I'm at it, clean up the code a bit.

Signed-off-by: Hans Verkuil 
---
I'll try to give this a spin in the morning with xawtv and my ivtv card
(that also uses V4L2_PIX_FMT_MPEG), just to make sure xawtv no longer
breaks if it sees it.

Oleksandr, are you able to test this as well on your pvrusb2?


Thanks, Hans, this fixes the original issue on Linux Mint with kernel
4.8.17. Haven't tried it on openSUSE yet. Still, in xawtv I get no TV
reception but just a black screen and error messages like:

no way to get: 128x96 32 bit TrueColor (LE: bgr-)
no way to get: 128x96 32 bit TrueColor (LE: bgr-)
no way to get: 128x96 32 bit TrueColor (LE: bgr-)
no way to get: 128x96 32 bit TrueColor (LE: bgr-)
no way to get: 384x288 32 bit TrueColor (LE: bgr-)

Is this another bug?


No. xawtv simply doesn't support MPEG formats. So this is what I would expect.

Regards,

Hans



Best,
Oleksandr



Regards,

Hans
---
diff --git a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c
b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c index 4320bda9352d..cc90be364a30
100644
--- a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c
+++ b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c
@@ -78,18 +78,6 @@ static int vbi_nr[PVR_NUM] = {[0 ... PVR_NUM-1] = -1};
  module_param_array(vbi_nr, int, NULL, 0444);
  MODULE_PARM_DESC(vbi_nr, "Offset for device's vbi dev minor");

-static struct v4l2_fmtdesc pvr_fmtdesc [] = {
-   {
-   .index  = 0,
-   .type   = V4L2_BUF_TYPE_VIDEO_CAPTURE,
-   .flags  = V4L2_FMT_FLAG_COMPRESSED,
-   .description= "MPEG1/2",
-   // This should really be V4L2_PIX_FMT_MPEG, but xawtv
-   // breaks when I do that.
-   .pixelformat= 0, // V4L2_PIX_FMT_MPEG,
-   }
-};
-
  #define PVR_FORMAT_PIX  0
  #define PVR_FORMAT_VBI  1

@@ -99,17 +87,11 @@ static struct v4l2_format pvr_format [] = {
.fmt= {
.pix= {
.width  = 720,
-   .height = 576,
-   // This should really be V4L2_PIX_FMT_MPEG,
-   // but xawtv breaks when I do that.
-   .pixelformat= 0, // V4L2_PIX_FMT_MPEG,
+   .height = 576,
+   .pixelformat= V4L2_PIX_FMT_MPEG,
.field  = V4L2_FIELD_INTERLACED,
-   .bytesperline   = 0,  // doesn't make sense
- // here
-   //FIXME : Don't know what to put here...
-   .sizeimage  = (32*1024),
-   .colorspace = 0, // doesn't make sense here
-   .priv   = 0
+   /* FIXME : Don't know what to put here... */
+   .sizeimage  = 32 * 1024,
}
}
},
@@ -407,11 +389,11 @@ static int pvr2_g_frequency(struct file *file, void
*priv, struct v4l2_frequency

  static int pvr2_enum_fmt_vid_cap(struct file *file, void *priv, struct
v4l2_fmtdesc *fd) {
-   /* Only one format is supported : mpeg.*/
-   if (fd->index != 0)
+   /* Only one format is supported: MPEG. */
+   if (fd->index)
return -EINVAL;

-   memcpy(fd, pvr_fmtdesc, sizeof(struct v4l2_fmtdesc));
+   fd->pixelformat = V4L2_PIX_FMT_MPEG;
return 0;
  }









smime.p7s
Description: S/MIME Cryptographic Signature


[GIT PULL for 4.16 v2] Intel IPU3 CIO2 CSI-2 receiver driver

2017-12-15 Thread Sakari Ailus
Hi Mauro,

Here's the Intel IPU3 CIO2 CSI-2 receiver driver, with the accompanying
format definitions.

Since the previous pull request dealing with this, I've squashed Yong's
patch to the patch introducing the driver, addressing two issues:

- unused "phys" variable and

- memory allocated on stack based on a function argument.



Please pull.


The following changes since commit b32a2b42f76cdfd06b4b58a1ddf987ba329ae34e:

  media: ddbridge: improve error handling logic on fe attach failures 
(2017-12-13 10:19:41 -0500)

are available in the git repository at:

  ssh://linuxtv.org/git/sailus/media_tree.git ipu3

for you to fetch changes up to 012b03bf357ecb0807c790e8f0b3bcff7e079ae2:

  intel-ipu3: cio2: add new MIPI-CSI2 driver (2017-12-15 17:56:04 +0200)


Yong Zhi (3):
  videodev2.h, v4l2-ioctl: add IPU3 raw10 color format
  doc-rst: add IPU3 raw10 bayer pixel format definitions
  intel-ipu3: cio2: add new MIPI-CSI2 driver

 Documentation/media/uapi/v4l/pixfmt-rgb.rst|1 +
 .../media/uapi/v4l/pixfmt-srggb10-ipu3.rst |  335 
 MAINTAINERS|8 +
 drivers/media/pci/Kconfig  |2 +
 drivers/media/pci/Makefile |3 +-
 drivers/media/pci/intel/Makefile   |5 +
 drivers/media/pci/intel/ipu3/Kconfig   |   19 +
 drivers/media/pci/intel/ipu3/Makefile  |1 +
 drivers/media/pci/intel/ipu3/ipu3-cio2.c   | 2048 
 drivers/media/pci/intel/ipu3/ipu3-cio2.h   |  449 +
 drivers/media/v4l2-core/v4l2-ioctl.c   |4 +
 include/uapi/linux/videodev2.h |6 +
 12 files changed, 2880 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/media/uapi/v4l/pixfmt-srggb10-ipu3.rst
 create mode 100644 drivers/media/pci/intel/Makefile
 create mode 100644 drivers/media/pci/intel/ipu3/Kconfig
 create mode 100644 drivers/media/pci/intel/ipu3/Makefile
 create mode 100644 drivers/media/pci/intel/ipu3/ipu3-cio2.c
 create mode 100644 drivers/media/pci/intel/ipu3/ipu3-cio2.h

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi


Re: [PATCH v2 1/3] media: V3s: Add support for Allwinner CSI.

2017-12-15 Thread Maxime Ripard
Hi,

On Fri, Dec 15, 2017 at 07:01:40PM +0800, Yong wrote:
> Hi Maxime,
> 
> On Fri, 15 Dec 2017 11:50:47 +0100
> Maxime Ripard  wrote:
> 
> > Hi Yong,
> > 
> > On Mon, Dec 04, 2017 at 05:45:11PM +0800, Yong wrote:
> > > I just noticed that you are using the second iteration?
> > > Have you received my third iteration?
> > 
> > Sorry for the late reply, and for not coming back to you yet about
> > that test. No, this is still in your v2. I've definitely received your
> > v3, I just didn't have time to update to it yet.
> > 
> > But don't worry, my mail was mostly to know if you had tested that
> > setup on your side to try to nail down the issue on my end, not really
> > a review or comment that would prevent your patch from going in.
> 
> I mean,
> The v2 exactly has a bug which may cause the CSI writing frame to 
> a wrong memory address.

Ah, sorry I misunderstood you then. I'll definitely test your v3..

> BTW, should I send a new version. I have made some improve sine v3.

.. or your v4 :)

Yes, please send a new version.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


Re: [RFC PATCH 0/9] media: base request API support

2017-12-15 Thread Nicolas Dufresne
Le vendredi 15 décembre 2017 à 16:56 +0900, Alexandre Courbot a écrit :
> Here is a new attempt at the request API, following the UAPI we agreed on in
> Prague. Hopefully this can be used as the basis to move forward.
> 
> This series only introduces the very basics of how requests work: allocate a
> request, queue buffers to it, queue the request itself, wait for it to 
> complete,
> reuse it. It does *not* yet use Hans' work with controls setting. I have
> preferred to submit it this way for now as it allows us to concentrate on the
> basic request/buffer flow, which was harder to get properly than I initially
> thought. I still have a gut feeling that it can be improved, with less 
> back-and-
> forth into drivers.
> 
> Plugging in controls support should not be too hard a task (basically just 
> apply
> the saved controls when the request starts), and I am looking at it now.
> 
> The resulting vim2m driver can be successfully used with requests, and my 
> tests
> so far have been successful.
> 
> There are still some rougher edges:
> 
> * locking is currently quite coarse-grained
> * too many #ifdef CONFIG_MEDIA_CONTROLLER in the code, as the request API
>   depends on it - I plan to craft the headers so that it becomes unnecessary.
>   As it is, some of the code will probably not even compile if
>   CONFIG_MEDIA_CONTROLLER is not set

Would it be possible to explain why this relation between request and
the media controller ? Why couldn't request be created from video
devices ?

> 
> But all in all I think the request flow should be clear and easy to review, 
> and
> the possibility of custom queue and entity support implementations should give
> us the flexibility we need to support more specific use-cases (I expect the
> generic implementations to be sufficient most of the time though).
> 
> A very simple test program exercising this API is available here (don't forget
> to adapt the /dev/media0 hardcoding):
> https://gist.github.com/Gnurou/dbc3776ed97ea7d4ce6041ea15eb0438
> 
> Looking forward to your feedback and comments!
> 
> Alexandre Courbot (8):
>   media: add request API core and UAPI
>   media: request: add generic queue
>   media: request: add generic entity ops
>   media: vb2: add support for requests
>   media: vb2: add support for requests in QBUF ioctl
>   media: v4l2-mem2mem: add request support
>   media: vim2m: add media device
>   media: vim2m: add request support
> 
> Hans Verkuil (1):
>   videodev2.h: Add request field to v4l2_buffer
> 
>  drivers/media/Makefile|   4 +-
>  drivers/media/media-device.c  |   6 +
>  drivers/media/media-request-entity-generic.c  |  56 
>  drivers/media/media-request-queue-generic.c   | 150 ++
>  drivers/media/media-request.c | 390 
> ++
>  drivers/media/platform/vim2m.c|  46 +++
>  drivers/media/usb/cpia2/cpia2_v4l.c   |   2 +-
>  drivers/media/v4l2-core/v4l2-compat-ioctl32.c |   7 +-
>  drivers/media/v4l2-core/v4l2-ioctl.c  |  99 ++-
>  drivers/media/v4l2-core/v4l2-mem2mem.c|  34 +++
>  drivers/media/v4l2-core/videobuf2-core.c  |  59 +++-
>  drivers/media/v4l2-core/videobuf2-v4l2.c  |  32 ++-
>  include/media/media-device.h  |   3 +
>  include/media/media-entity.h  |   6 +
>  include/media/media-request.h | 282 +++
>  include/media/v4l2-mem2mem.h  |  19 ++
>  include/media/videobuf2-core.h|  25 +-
>  include/media/videobuf2-v4l2.h|   2 +
>  include/uapi/linux/media.h|  11 +
>  include/uapi/linux/videodev2.h|   3 +-
>  20 files changed, 1216 insertions(+), 20 deletions(-)
>  create mode 100644 drivers/media/media-request-entity-generic.c
>  create mode 100644 drivers/media/media-request-queue-generic.c
>  create mode 100644 drivers/media/media-request.c
>  create mode 100644 include/media/media-request.h
> 

signature.asc
Description: This is a digitally signed message part


Re: [RFC PATCH 0/9] media: base request API support

2017-12-15 Thread Nicolas Dufresne
Le vendredi 15 décembre 2017 à 16:56 +0900, Alexandre Courbot a écrit :
> Here is a new attempt at the request API, following the UAPI we agreed on in
> Prague. Hopefully this can be used as the basis to move forward.
> 
> This series only introduces the very basics of how requests work: allocate a
> request, queue buffers to it, queue the request itself, wait for it to 
> complete,
> reuse it. It does *not* yet use Hans' work with controls setting. I have
> preferred to submit it this way for now as it allows us to concentrate on the
> basic request/buffer flow, which was harder to get properly than I initially
> thought. I still have a gut feeling that it can be improved, with less 
> back-and-
> forth into drivers.
> 
> Plugging in controls support should not be too hard a task (basically just 
> apply
> the saved controls when the request starts), and I am looking at it now.
> 
> The resulting vim2m driver can be successfully used with requests, and my 
> tests
> so far have been successful.
> 
> There are still some rougher edges:
> 
> * locking is currently quite coarse-grained
> * too many #ifdef CONFIG_MEDIA_CONTROLLER in the code, as the request API
>   depends on it - I plan to craft the headers so that it becomes unnecessary.
>   As it is, some of the code will probably not even compile if
>   CONFIG_MEDIA_CONTROLLER is not set
> 
> But all in all I think the request flow should be clear and easy to review, 
> and
> the possibility of custom queue and entity support implementations should give
> us the flexibility we need to support more specific use-cases (I expect the
> generic implementations to be sufficient most of the time though).
> 
> A very simple test program exercising this API is available here (don't forget
> to adapt the /dev/media0 hardcoding):
> https://gist.github.com/Gnurou/dbc3776ed97ea7d4ce6041ea15eb0438

It looks like the example uses Hans control work you just mention.
Notably, it uses v4l2_ext_controls ctrls.request.

> 
> Looking forward to your feedback and comments!
> 
> Alexandre Courbot (8):
>   media: add request API core and UAPI
>   media: request: add generic queue
>   media: request: add generic entity ops
>   media: vb2: add support for requests
>   media: vb2: add support for requests in QBUF ioctl
>   media: v4l2-mem2mem: add request support
>   media: vim2m: add media device
>   media: vim2m: add request support
> 
> Hans Verkuil (1):
>   videodev2.h: Add request field to v4l2_buffer
> 
>  drivers/media/Makefile|   4 +-
>  drivers/media/media-device.c  |   6 +
>  drivers/media/media-request-entity-generic.c  |  56 
>  drivers/media/media-request-queue-generic.c   | 150 ++
>  drivers/media/media-request.c | 390 
> ++
>  drivers/media/platform/vim2m.c|  46 +++
>  drivers/media/usb/cpia2/cpia2_v4l.c   |   2 +-
>  drivers/media/v4l2-core/v4l2-compat-ioctl32.c |   7 +-
>  drivers/media/v4l2-core/v4l2-ioctl.c  |  99 ++-
>  drivers/media/v4l2-core/v4l2-mem2mem.c|  34 +++
>  drivers/media/v4l2-core/videobuf2-core.c  |  59 +++-
>  drivers/media/v4l2-core/videobuf2-v4l2.c  |  32 ++-
>  include/media/media-device.h  |   3 +
>  include/media/media-entity.h  |   6 +
>  include/media/media-request.h | 282 +++
>  include/media/v4l2-mem2mem.h  |  19 ++
>  include/media/videobuf2-core.h|  25 +-
>  include/media/videobuf2-v4l2.h|   2 +
>  include/uapi/linux/media.h|  11 +
>  include/uapi/linux/videodev2.h|   3 +-
>  20 files changed, 1216 insertions(+), 20 deletions(-)
>  create mode 100644 drivers/media/media-request-entity-generic.c
>  create mode 100644 drivers/media/media-request-queue-generic.c
>  create mode 100644 drivers/media/media-request.c
>  create mode 100644 include/media/media-request.h
> 

signature.asc
Description: This is a digitally signed message part


[PATCH] staging: android: ion: Fix dma direction for dma_sync_sg_for_cpu/device

2017-12-15 Thread Sushmita Susheelendra
Use the direction argument passed into begin_cpu_access
and end_cpu_access when calling the dma_sync_sg_for_cpu/device.
The actual cache primitive called depends on the direction
passed in.

Signed-off-by: Sushmita Susheelendra 
---
 drivers/staging/android/ion/ion.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/android/ion/ion.c 
b/drivers/staging/android/ion/ion.c
index a7d9b0e..f480885 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -346,7 +346,7 @@ static int ion_dma_buf_begin_cpu_access(struct dma_buf 
*dmabuf,
mutex_lock(>lock);
list_for_each_entry(a, >attachments, list) {
dma_sync_sg_for_cpu(a->dev, a->table->sgl, a->table->nents,
-   DMA_BIDIRECTIONAL);
+   direction);
}
mutex_unlock(>lock);
 
@@ -368,7 +368,7 @@ static int ion_dma_buf_end_cpu_access(struct dma_buf 
*dmabuf,
mutex_lock(>lock);
list_for_each_entry(a, >attachments, list) {
dma_sync_sg_for_device(a->dev, a->table->sgl, a->table->nents,
-  DMA_BIDIRECTIONAL);
+  direction);
}
mutex_unlock(>lock);
 
-- 
1.9.1



Re: [PATCH] [media] tda18212: fix use-after-free in tda18212_remove()

2017-12-15 Thread Daniel Scheller
On Fri, 15 Dec 2017 21:06:32 +0200
Antti Palosaari  wrote:

> On 12/15/2017 08:40 PM, Daniel Scheller wrote:
> > On Fri, 15 Dec 2017 20:12:18 +0200
> > Antti Palosaari  wrote:
> >>
> >> em28xx does it currently just correct.
> >> 1) unregister frontend  
> > 
> > Note that this is a call to em28xx_unregister_dvb(), which in turn
> > does dvb_unregister_frontend() and then dvb_frontend_detach() (at
> > this stage, fe resources are gone).
> >   
> >> 2) remove I2C SEC
> >> 3) remove I2C tuner
> >> 4) remove I2C demod (frees shared frontend data)  
> > 
> > Yes, but ie. EM2874_BOARD_KWORLD_UB435Q_V3 is a combination of a
> > "legacy" demod frontend - lgdt3305 actually - plus the tda18212
> > i2cclient (just like in ddb with stv0367+tda18212 or
> > cxd2841er+tda18212), I'm sure this will yield the same report.
> > 
> > Maybe another approach: Implement the tuner_ops.release callback,
> > and then move the memset+NULL assignment right there (instead of
> > just removing it), but this likely will cause issues when the i2c
> > client is removed before detach if we don't keep track of this ie
> > somewhere in tda18212_dev (new state var - if _remove is called,
> > check if the tuner was released, and if not, call release
> > (memset/set NULL), then free). Still with the two other drivers in
> > mind though. If they're wrong aswell, I'll rather fix up ddbridge
> > of course.  
> 
> Whole memset thing could be removed from tda18212, there is something 
> likely wrong if those are needed. But it is another issue.

On a side note: After some few more glances, there are many other
drivers in media/tuners/ that would require such treatment (tbh I just
peeked into tda18212 when investigating the KASAN report).

> Your main issue is somehow to get order of demod/tuner destroy
> correct. I don't even like idea whole shared frontend data is owned
> by the demod driver instance, but currently it is there and due to
> that this should be released lastly. General design goal is also do
> things like register things in order and unregister just
> reverse-order.

Fully agreeing on the last bit. I'll see how to improve on this in
ddbridge. Yet, very likely other drivers (and I have a feeling there
are quite some) with this issue (wrong teardown order) remain.

It might indeed be better if in frontend_ops, tuner_ops would be a ptr
to some struct that is managed by the tuner driver itself, that would
save from such issues.

Anyway, thank you very much for your input! (and apologies for any
noise or nonsense)

Best regards,
Daniel Scheller
-- 
https://github.com/herrnst


Re: [PATCH] [media] tda18212: fix use-after-free in tda18212_remove()

2017-12-15 Thread Antti Palosaari



On 12/15/2017 08:40 PM, Daniel Scheller wrote:

On Fri, 15 Dec 2017 20:12:18 +0200
Antti Palosaari  wrote:


On 12/15/2017 08:00 PM, Daniel Scheller wrote:

Hi,

On Fri, 15 Dec 2017 19:30:18 +0200
Antti Palosaari  wrote:

Thanks for your reply.
   

Hello
I think shared frontend structure, which is owned by demod driver,
should be there and valid on time tuner driver is removed. And thus
should not happen. Did you make driver unload on different order
eg. not just reverse order than driver load?

IMHO these should go always

on load:
1) load demod driver (which makes shared frontend structure where
also some tuner driver data lives)
2) load tuner driver
3) register frontend

on unload
1) unregister frontend
2) remove tuner driver
3) remove demod driver (frees shared data)


In ddbridge, we do (like in usb/em28xx and platform/sti/c8sectpfe,
both also use some demod+tda18212 combo):

dvb_unregister_frontend();
dvb_frontend_detach();
module_put(tda18212client->...owner);
i2c_unregister_device(tda18212client);

fe_detach() clears out the frontend references and frees/invalidates
the allocated resources. tuner_ops obviously isn't there then
anymore.


yeah, but that's even ideally wrong. frontend design currently relies
to shared data which is owned by demod driver and thus it should be
last thing to be removed. Sure change like you did prevents issue,
but logically it is still wrong and may not work on some other case.



The two mentioned drivers will very likely yield the same (or
similar) KASAN report. em28xx was even changed lately to do the
teardown the way ddbridge does in 910b0797fa9e8 ([1], cc'ing
Matthias here).

With that commit in mind I'm a bit unsure on what is correct or not.
OTOH, as dvb_frontend_detach() cleans up everything, IMHO there's no
need for the tuner driver to try to clean up further.

Please advise.

[1]
https://git.linuxtv.org/media_tree.git/commit/?id=910b0797fa9e8.


em28xx does it currently just correct.
1) unregister frontend


Note that this is a call to em28xx_unregister_dvb(), which in turn does
dvb_unregister_frontend() and then dvb_frontend_detach() (at this
stage, fe resources are gone).


2) remove I2C SEC
3) remove I2C tuner
4) remove I2C demod (frees shared frontend data)


Yes, but ie. EM2874_BOARD_KWORLD_UB435Q_V3 is a combination of a
"legacy" demod frontend - lgdt3305 actually - plus the tda18212
i2cclient (just like in ddb with stv0367+tda18212 or
cxd2841er+tda18212), I'm sure this will yield the same report.

Maybe another approach: Implement the tuner_ops.release callback, and
then move the memset+NULL assignment right there (instead of just
removing it), but this likely will cause issues when the i2c client is
removed before detach if we don't keep track of this ie somewhere in
tda18212_dev (new state var - if _remove is called, check if the tuner
was released, and if not, call release (memset/set NULL), then
free). Still with the two other drivers in mind though. If they're
wrong aswell, I'll rather fix up ddbridge of course.


Whole memset thing could be removed from tda18212, there is something 
likely wrong if those are needed. But it is another issue.


Your main issue is somehow to get order of demod/tuner destroy correct. 
I don't even like idea whole shared frontend data is owned by the demod 
driver instance, but currently it is there and due to that this should 
be released lastly. General design goal is also do things like register 
things in order and unregister just reverse-order.


regards
Antti

--
http://palosaari.fi/


Re: [PATCH] [media] tda18212: fix use-after-free in tda18212_remove()

2017-12-15 Thread Daniel Scheller
On Fri, 15 Dec 2017 20:12:18 +0200
Antti Palosaari  wrote:

> On 12/15/2017 08:00 PM, Daniel Scheller wrote:
> > Hi,
> > 
> > On Fri, 15 Dec 2017 19:30:18 +0200
> > Antti Palosaari  wrote:
> > 
> > Thanks for your reply.
> >   
> >> Hello
> >> I think shared frontend structure, which is owned by demod driver,
> >> should be there and valid on time tuner driver is removed. And thus
> >> should not happen. Did you make driver unload on different order
> >> eg. not just reverse order than driver load?
> >>
> >> IMHO these should go always
> >>
> >> on load:
> >> 1) load demod driver (which makes shared frontend structure where
> >> also some tuner driver data lives)
> >> 2) load tuner driver
> >> 3) register frontend
> >>
> >> on unload
> >> 1) unregister frontend
> >> 2) remove tuner driver
> >> 3) remove demod driver (frees shared data)  
> > 
> > In ddbridge, we do (like in usb/em28xx and platform/sti/c8sectpfe,
> > both also use some demod+tda18212 combo):
> > 
> > dvb_unregister_frontend();
> > dvb_frontend_detach();
> > module_put(tda18212client->...owner);
> > i2c_unregister_device(tda18212client);
> > 
> > fe_detach() clears out the frontend references and frees/invalidates
> > the allocated resources. tuner_ops obviously isn't there then
> > anymore.  
> 
> yeah, but that's even ideally wrong. frontend design currently relies
> to shared data which is owned by demod driver and thus it should be
> last thing to be removed. Sure change like you did prevents issue,
> but logically it is still wrong and may not work on some other case.
> 
> > 
> > The two mentioned drivers will very likely yield the same (or
> > similar) KASAN report. em28xx was even changed lately to do the
> > teardown the way ddbridge does in 910b0797fa9e8 ([1], cc'ing
> > Matthias here).
> > 
> > With that commit in mind I'm a bit unsure on what is correct or not.
> > OTOH, as dvb_frontend_detach() cleans up everything, IMHO there's no
> > need for the tuner driver to try to clean up further.
> > 
> > Please advise.
> > 
> > [1]
> > https://git.linuxtv.org/media_tree.git/commit/?id=910b0797fa9e8.  
> 
> em28xx does it currently just correct.
> 1) unregister frontend

Note that this is a call to em28xx_unregister_dvb(), which in turn does
dvb_unregister_frontend() and then dvb_frontend_detach() (at this
stage, fe resources are gone).

> 2) remove I2C SEC
> 3) remove I2C tuner
> 4) remove I2C demod (frees shared frontend data)

Yes, but ie. EM2874_BOARD_KWORLD_UB435Q_V3 is a combination of a
"legacy" demod frontend - lgdt3305 actually - plus the tda18212
i2cclient (just like in ddb with stv0367+tda18212 or
cxd2841er+tda18212), I'm sure this will yield the same report.

Maybe another approach: Implement the tuner_ops.release callback, and
then move the memset+NULL assignment right there (instead of just
removing it), but this likely will cause issues when the i2c client is
removed before detach if we don't keep track of this ie somewhere in
tda18212_dev (new state var - if _remove is called, check if the tuner
was released, and if not, call release (memset/set NULL), then
free). Still with the two other drivers in mind though. If they're
wrong aswell, I'll rather fix up ddbridge of course.

Best regards,
Daniel Scheller
-- 
https://github.com/herrnst


Re: [PATCH] [media] tda18212: fix use-after-free in tda18212_remove()

2017-12-15 Thread Antti Palosaari



On 12/15/2017 08:00 PM, Daniel Scheller wrote:

Hi,

On Fri, 15 Dec 2017 19:30:18 +0200
Antti Palosaari  wrote:

Thanks for your reply.


Hello
I think shared frontend structure, which is owned by demod driver,
should be there and valid on time tuner driver is removed. And thus
should not happen. Did you make driver unload on different order eg.
not just reverse order than driver load?

IMHO these should go always

on load:
1) load demod driver (which makes shared frontend structure where
also some tuner driver data lives)
2) load tuner driver
3) register frontend

on unload
1) unregister frontend
2) remove tuner driver
3) remove demod driver (frees shared data)


In ddbridge, we do (like in usb/em28xx and platform/sti/c8sectpfe, both
also use some demod+tda18212 combo):

dvb_unregister_frontend();
dvb_frontend_detach();
module_put(tda18212client->...owner);
i2c_unregister_device(tda18212client);

fe_detach() clears out the frontend references and frees/invalidates
the allocated resources. tuner_ops obviously isn't there then anymore.


yeah, but that's even ideally wrong. frontend design currently relies to 
shared data which is owned by demod driver and thus it should be last 
thing to be removed. Sure change like you did prevents issue, but 
logically it is still wrong and may not work on some other case.




The two mentioned drivers will very likely yield the same (or
similar) KASAN report. em28xx was even changed lately to do the teardown
the way ddbridge does in 910b0797fa9e8 ([1], cc'ing Matthias
here).

With that commit in mind I'm a bit unsure on what is correct or not.
OTOH, as dvb_frontend_detach() cleans up everything, IMHO there's no
need for the tuner driver to try to clean up further.

Please advise.

[1] https://git.linuxtv.org/media_tree.git/commit/?id=910b0797fa9e8.


em28xx does it currently just correct.
1) unregister frontend
2) remove I2C SEC
3) remove I2C tuner
4) remove I2C demod (frees shared frontend data)

regards
Antti

--
http://palosaari.fi/


Re: [PATCH] [media] tda18212: fix use-after-free in tda18212_remove()

2017-12-15 Thread Daniel Scheller
Hi,

On Fri, 15 Dec 2017 19:30:18 +0200
Antti Palosaari  wrote:

Thanks for your reply.

> Hello
> I think shared frontend structure, which is owned by demod driver, 
> should be there and valid on time tuner driver is removed. And thus 
> should not happen. Did you make driver unload on different order eg.
> not just reverse order than driver load?
> 
> IMHO these should go always
> 
> on load:
> 1) load demod driver (which makes shared frontend structure where
> also some tuner driver data lives)
> 2) load tuner driver
> 3) register frontend
> 
> on unload
> 1) unregister frontend
> 2) remove tuner driver
> 3) remove demod driver (frees shared data)

In ddbridge, we do (like in usb/em28xx and platform/sti/c8sectpfe, both
also use some demod+tda18212 combo):

dvb_unregister_frontend();
dvb_frontend_detach();
module_put(tda18212client->...owner);
i2c_unregister_device(tda18212client);

fe_detach() clears out the frontend references and frees/invalidates
the allocated resources. tuner_ops obviously isn't there then anymore.

The two mentioned drivers will very likely yield the same (or
similar) KASAN report. em28xx was even changed lately to do the teardown
the way ddbridge does in 910b0797fa9e8 ([1], cc'ing Matthias
here).

With that commit in mind I'm a bit unsure on what is correct or not.
OTOH, as dvb_frontend_detach() cleans up everything, IMHO there's no
need for the tuner driver to try to clean up further.

Please advise.

[1] https://git.linuxtv.org/media_tree.git/commit/?id=910b0797fa9e8.

Best regards,
Daniel

> On 12/15/2017 06:43 PM, Daniel Scheller wrote:
> > From: Daniel Scheller 
> > 
> > When the driver gets unloaded via it's tda18212_remove() function,
> > all frontend structures may already have been freed as
> > controlling/bridge drivers already used dvb_frontend_detach() in
> > their teardown process. Since __dvb_frontend_free now releases and
> > clears all structures, the memset and the NULL assignment in
> > tda18212_remove() leads to this KASAN report (invoked via ddbridge,
> > which does dvb_frontend_detach() first, followed by
> > i2c_unregister_device()):
> > 
> >[  154.028353]
> > ==
> > [  154.028396] BUG: KASAN: use-after-free in
> > tda18212_remove+0x5c/0xb0 [tda18212] [  154.028415] Write of size
> > 288 at addr 880108b554d8 by task rmmod/285
> > 
> >[  154.028442] CPU: 0 PID: 285 Comm: rmmod Tainted: G
> > C   4.15.0-rc1-13682-g1363f325bc44 #1 [  154.028444] Hardware
> > name: Gigabyte Technology Co., Ltd. P35-DS3/P35-DS3, BIOS F3
> > 06/11/2007 [  154.028445] Call Trace: [  154.028452]
> > dump_stack+0x46/0x61 [  154.028458]
> > print_address_description+0x79/0x270 [  154.028462]  ?
> > tda18212_remove+0x5c/0xb0 [tda18212] [  154.028465]
> > kasan_report+0x229/0x340 [  154.028468]  memset+0x1f/0x40
> >[  154.028472]  tda18212_remove+0x5c/0xb0 [tda18212]
> >[  154.028476]  i2c_device_remove+0x97/0xe0
> >[  154.028481]  device_release_driver_internal+0x267/0x510
> >[  154.028484]  bus_remove_device+0x296/0x470
> >[  154.028486]  device_del+0x35c/0x890
> >[  154.028489]  ? __device_links_no_driver+0x1c0/0x1c0
> >[  154.028493]  ? cxd2841er_get_algo+0x10/0x10 [cxd2841er]
> >[  154.028497]  ? cxd2841er_get_algo+0x10/0x10 [cxd2841er]
> >[  154.028500]  ? __module_text_address+0xe/0x140
> >[  154.028503]  device_unregister+0x9/0x20
> >[  154.028509]  dvb_input_detach.isra.24+0x286/0x480 [ddbridge]
> >[  154.028514]  ddb_ports_detach+0x176/0x630 [ddbridge]
> >[  154.028519]  ddb_remove+0x3c/0xb0 [ddbridge]
> >[  154.028523]  pci_device_remove+0x93/0x1d0
> >[  154.028526]  device_release_driver_internal+0x267/0x510
> >[  154.028529]  driver_detach+0xb9/0x1b0
> >[  154.028532]  bus_remove_driver+0xd0/0x1f0
> >[  154.028536]  pci_unregister_driver+0x25/0x210
> >[  154.028541]  module_exit_ddbridge+0xc/0x45 [ddbridge]
> >[  154.028544]  SyS_delete_module+0x314/0x440
> >[  154.028546]  ? free_module+0x5b0/0x5b0
> >[  154.028550]  ? exit_to_usermode_loop+0xa9/0xc0
> >[  154.028552]  ? free_module+0x5b0/0x5b0
> >[  154.028554]  do_syscall_64+0x179/0x4c0
> >[  154.028557]  ? do_page_fault+0x1b/0x60
> >[  154.028560]  entry_SYSCALL64_slow_path+0x25/0x25
> >[  154.028563] RIP: 0033:0x7f6c40930de7
> >[  154.028565] RSP: 002b:7ffc060d6ab8 EFLAGS: 0206
> > ORIG_RAX: 00b0 [  154.028569] RAX: ffda
> > RBX:  RCX: 7f6c40930de7 [  154.028571] RDX:
> > 000a RSI: 0800 RDI: 02053268
> > [  154.028573] RBP: 02053200 R08:  R09:
> > 1999 [  154.028574] R10: 0891 R11:
> > 0206 R12: 7ffc060d74ef [  154.028576] R13:
> >  R14: 02053200 R15: 02052010
> > 
> >[  154.028588] Allocated by task 164:
> >[  

Re: [PATCH] [media] tda18212: fix use-after-free in tda18212_remove()

2017-12-15 Thread Antti Palosaari

Hello
I think shared frontend structure, which is owned by demod driver, 
should be there and valid on time tuner driver is removed. And thus 
should not happen. Did you make driver unload on different order eg. not 
just reverse order than driver load?


IMHO these should go always

on load:
1) load demod driver (which makes shared frontend structure where also 
some tuner driver data lives)

2) load tuner driver
3) register frontend

on unload
1) unregister frontend
2) remove tuner driver
3) remove demod driver (frees shared data)


regards
Antti


On 12/15/2017 06:43 PM, Daniel Scheller wrote:

From: Daniel Scheller 

When the driver gets unloaded via it's tda18212_remove() function, all
frontend structures may already have been freed as controlling/bridge
drivers already used dvb_frontend_detach() in their teardown process.
Since __dvb_frontend_free now releases and clears all structures, the
memset and the NULL assignment in tda18212_remove() leads to this KASAN
report (invoked via ddbridge, which does dvb_frontend_detach() first,
followed by i2c_unregister_device()):

   [  154.028353] 
==
   [  154.028396] BUG: KASAN: use-after-free in tda18212_remove+0x5c/0xb0 
[tda18212]
   [  154.028415] Write of size 288 at addr 880108b554d8 by task rmmod/285

   [  154.028442] CPU: 0 PID: 285 Comm: rmmod Tainted: G C   
4.15.0-rc1-13682-g1363f325bc44 #1
   [  154.028444] Hardware name: Gigabyte Technology Co., Ltd. P35-DS3/P35-DS3, 
BIOS F3 06/11/2007
   [  154.028445] Call Trace:
   [  154.028452]  dump_stack+0x46/0x61
   [  154.028458]  print_address_description+0x79/0x270
   [  154.028462]  ? tda18212_remove+0x5c/0xb0 [tda18212]
   [  154.028465]  kasan_report+0x229/0x340
   [  154.028468]  memset+0x1f/0x40
   [  154.028472]  tda18212_remove+0x5c/0xb0 [tda18212]
   [  154.028476]  i2c_device_remove+0x97/0xe0
   [  154.028481]  device_release_driver_internal+0x267/0x510
   [  154.028484]  bus_remove_device+0x296/0x470
   [  154.028486]  device_del+0x35c/0x890
   [  154.028489]  ? __device_links_no_driver+0x1c0/0x1c0
   [  154.028493]  ? cxd2841er_get_algo+0x10/0x10 [cxd2841er]
   [  154.028497]  ? cxd2841er_get_algo+0x10/0x10 [cxd2841er]
   [  154.028500]  ? __module_text_address+0xe/0x140
   [  154.028503]  device_unregister+0x9/0x20
   [  154.028509]  dvb_input_detach.isra.24+0x286/0x480 [ddbridge]
   [  154.028514]  ddb_ports_detach+0x176/0x630 [ddbridge]
   [  154.028519]  ddb_remove+0x3c/0xb0 [ddbridge]
   [  154.028523]  pci_device_remove+0x93/0x1d0
   [  154.028526]  device_release_driver_internal+0x267/0x510
   [  154.028529]  driver_detach+0xb9/0x1b0
   [  154.028532]  bus_remove_driver+0xd0/0x1f0
   [  154.028536]  pci_unregister_driver+0x25/0x210
   [  154.028541]  module_exit_ddbridge+0xc/0x45 [ddbridge]
   [  154.028544]  SyS_delete_module+0x314/0x440
   [  154.028546]  ? free_module+0x5b0/0x5b0
   [  154.028550]  ? exit_to_usermode_loop+0xa9/0xc0
   [  154.028552]  ? free_module+0x5b0/0x5b0
   [  154.028554]  do_syscall_64+0x179/0x4c0
   [  154.028557]  ? do_page_fault+0x1b/0x60
   [  154.028560]  entry_SYSCALL64_slow_path+0x25/0x25
   [  154.028563] RIP: 0033:0x7f6c40930de7
   [  154.028565] RSP: 002b:7ffc060d6ab8 EFLAGS: 0206 ORIG_RAX: 
00b0
   [  154.028569] RAX: ffda RBX:  RCX: 
7f6c40930de7
   [  154.028571] RDX: 000a RSI: 0800 RDI: 
02053268
   [  154.028573] RBP: 02053200 R08:  R09: 
1999
   [  154.028574] R10: 0891 R11: 0206 R12: 
7ffc060d74ef
   [  154.028576] R13:  R14: 02053200 R15: 
02052010

   [  154.028588] Allocated by task 164:
   [  154.028603]  cxd2841er_attach+0xc3/0x7f0 [cxd2841er]
   [  154.028608]  demod_attach_cxd28xx+0x14c/0x3f0 [ddbridge]
   [  154.028612]  dvb_input_attach+0x671/0x1e20 [ddbridge]
   [  154.028616]  ddb_ports_attach+0x453/0xd00 [ddbridge]
   [  154.028620]  ddb_init+0x4b3/0xa30 [ddbridge]
   [  154.028624]  ddb_probe+0xa51/0xfe0 [ddbridge]
   [  154.028627]  pci_device_probe+0x279/0x480
   [  154.028630]  driver_probe_device+0x46f/0x7a0
   [  154.028632]  __driver_attach+0x133/0x170
   [  154.028634]  bus_for_each_dev+0x10a/0x190
   [  154.028637]  bus_add_driver+0x2a3/0x5a0
   [  154.028639]  driver_register+0x182/0x3a0
   [  154.028641]  0xa016808f
   [  154.028643]  do_one_initcall+0x77/0x1d0
   [  154.028646]  do_init_module+0x1c2/0x548
   [  154.028648]  load_module+0x5e61/0x8df0
   [  154.028650]  SyS_finit_module+0x142/0x150
   [  154.028652]  do_syscall_64+0x179/0x4c0
   [  154.028654]  return_from_SYSCALL_64+0x0/0x65

   [  154.028664] Freed by task 285:
   [  154.028676]  kfree+0x6c/0xa0
   [  154.028682]  __dvb_frontend_free+0x81/0xb0 [dvb_core]
   [  154.028687]  dvb_input_detach.isra.24+0x17c/0x480 [ddbridge]
   [  154.028691]  ddb_ports_detach+0x176/0x630 

[GIT PULL FOR v4.16] RC fixes

2017-12-15 Thread Sean Young
Hi Mauro,

Some regressions and improvements since the lirc scancodes changes. 

Thanks,

Sean

The following changes since commit 0ca4e3130402caea8731a7b54afde56a6edb17c9:

  media: pxa_camera: rename the soc_camera_ prefix to pxa_camera_ (2017-12-14 
12:40:01 -0500)

are available in the Git repository at:

  git://linuxtv.org/syoung/media_tree.git for-v4.16b

for you to fetch changes up to 4cb896c68d5296e2af834033a34575f61fc5228f:

  media: ir-spi: add SPDX identifier (2017-12-15 16:42:11 +)


Andi Shyti (1):
  media: ir-spi: add SPDX identifier

Sean Young (8):
  media: imon: auto-config ffdc 26 device
  media: imon: remove unused function tv2int
  media: rc: bang in ir_do_keyup
  media: lirc: when transmitting scancodes, block until transmit is done
  media: rc: iguanair: simplify tx loop
  media: lirc: do not pass ERR_PTR to kfree
  media: lirc: no need to recalculate duration
  media: lirc: release lock before sleep

 Documentation/media/uapi/rc/lirc-write.rst |  4 +-
 drivers/media/rc/iguanair.c| 19 -
 drivers/media/rc/imon.c| 28 +++--
 drivers/media/rc/ir-spi.c  | 15 +++
 drivers/media/rc/lirc_dev.c| 67 ++
 drivers/media/rc/rc-main.c |  2 +-
 6 files changed, 53 insertions(+), 82 deletions(-)


[GIT PULL] Samsung SoC related updates

2017-12-15 Thread Sylwester Nawrocki
Hi Mauro,

The following changes since commit 0ca4e3130402caea8731a7b54afde56a6edb17c9:

  media: pxa_camera: rename the soc_camera_ prefix to pxa_camera_ (2017-12-14 
12:40:01 -0500)

are available in the git repository at:

  git://linuxtv.org/snawrocki/samsung.git for-v4.16/media/next

for you to fetch changes up to 8d10c3a3fa56badd9d8691b59a88e7f00fdeaa7b:

  s5p-jpeg: Fix off-by-one problem (2017-12-15 17:33:50 +0100)


Arnd Bergmann (1):
  exynos4-is: properly initialize frame format

Flavio Ceolin (1):
  s5p-jpeg: Fix off-by-one problem

Marek Szyprowski (3):
  exynos-gsc: Drop obsolete capabilities
  exynos4-is: Drop obsolete capabilities
  exynos4-is: Remove dependency on obsolete SoC support

Shuah Khan (2):
  s5p-mfc: Remove firmware buf null check in s5p_mfc_load_firmware()
  s5p-mfc: Fix lock contention - request_firmware() once

Simon Shields (1):
  exynos4-is: Check pipe is valid before calling subdev

Sylwester Nawrocki (1):
  s5p-mfc: Fix encoder menu controls initialization

 drivers/media/platform/exynos-gsc/gsc-m2m.c |  4 +---
 drivers/media/platform/exynos4-is/Kconfig   |  2 +-
 drivers/media/platform/exynos4-is/fimc-core.c   |  2 +-
 drivers/media/platform/exynos4-is/fimc-isp.c| 14 +++---
 drivers/media/platform/exynos4-is/fimc-lite.c   |  2 +-
 drivers/media/platform/exynos4-is/fimc-m2m.c| 10 +-
 drivers/media/platform/s5p-jpeg/jpeg-core.c |  2 +-
 drivers/media/platform/s5p-mfc/s5p_mfc.c|  6 ++
 drivers/media/platform/s5p-mfc/s5p_mfc_common.h |  3 +++
 drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c   | 10 +-
 drivers/media/platform/s5p-mfc/s5p_mfc_enc.c|  2 +-
 include/media/drv-intf/exynos-fimc.h|  3 ++-
 12 files changed, 30 insertions(+), 30 deletions(-)

-- 
Regards,
Sylwester


[PATCH] [media] tda18212: fix use-after-free in tda18212_remove()

2017-12-15 Thread Daniel Scheller
From: Daniel Scheller 

When the driver gets unloaded via it's tda18212_remove() function, all
frontend structures may already have been freed as controlling/bridge
drivers already used dvb_frontend_detach() in their teardown process.
Since __dvb_frontend_free now releases and clears all structures, the
memset and the NULL assignment in tda18212_remove() leads to this KASAN
report (invoked via ddbridge, which does dvb_frontend_detach() first,
followed by i2c_unregister_device()):

  [  154.028353] 
==
  [  154.028396] BUG: KASAN: use-after-free in tda18212_remove+0x5c/0xb0 
[tda18212]
  [  154.028415] Write of size 288 at addr 880108b554d8 by task rmmod/285

  [  154.028442] CPU: 0 PID: 285 Comm: rmmod Tainted: G C   
4.15.0-rc1-13682-g1363f325bc44 #1
  [  154.028444] Hardware name: Gigabyte Technology Co., Ltd. P35-DS3/P35-DS3, 
BIOS F3 06/11/2007
  [  154.028445] Call Trace:
  [  154.028452]  dump_stack+0x46/0x61
  [  154.028458]  print_address_description+0x79/0x270
  [  154.028462]  ? tda18212_remove+0x5c/0xb0 [tda18212]
  [  154.028465]  kasan_report+0x229/0x340
  [  154.028468]  memset+0x1f/0x40
  [  154.028472]  tda18212_remove+0x5c/0xb0 [tda18212]
  [  154.028476]  i2c_device_remove+0x97/0xe0
  [  154.028481]  device_release_driver_internal+0x267/0x510
  [  154.028484]  bus_remove_device+0x296/0x470
  [  154.028486]  device_del+0x35c/0x890
  [  154.028489]  ? __device_links_no_driver+0x1c0/0x1c0
  [  154.028493]  ? cxd2841er_get_algo+0x10/0x10 [cxd2841er]
  [  154.028497]  ? cxd2841er_get_algo+0x10/0x10 [cxd2841er]
  [  154.028500]  ? __module_text_address+0xe/0x140
  [  154.028503]  device_unregister+0x9/0x20
  [  154.028509]  dvb_input_detach.isra.24+0x286/0x480 [ddbridge]
  [  154.028514]  ddb_ports_detach+0x176/0x630 [ddbridge]
  [  154.028519]  ddb_remove+0x3c/0xb0 [ddbridge]
  [  154.028523]  pci_device_remove+0x93/0x1d0
  [  154.028526]  device_release_driver_internal+0x267/0x510
  [  154.028529]  driver_detach+0xb9/0x1b0
  [  154.028532]  bus_remove_driver+0xd0/0x1f0
  [  154.028536]  pci_unregister_driver+0x25/0x210
  [  154.028541]  module_exit_ddbridge+0xc/0x45 [ddbridge]
  [  154.028544]  SyS_delete_module+0x314/0x440
  [  154.028546]  ? free_module+0x5b0/0x5b0
  [  154.028550]  ? exit_to_usermode_loop+0xa9/0xc0
  [  154.028552]  ? free_module+0x5b0/0x5b0
  [  154.028554]  do_syscall_64+0x179/0x4c0
  [  154.028557]  ? do_page_fault+0x1b/0x60
  [  154.028560]  entry_SYSCALL64_slow_path+0x25/0x25
  [  154.028563] RIP: 0033:0x7f6c40930de7
  [  154.028565] RSP: 002b:7ffc060d6ab8 EFLAGS: 0206 ORIG_RAX: 
00b0
  [  154.028569] RAX: ffda RBX:  RCX: 
7f6c40930de7
  [  154.028571] RDX: 000a RSI: 0800 RDI: 
02053268
  [  154.028573] RBP: 02053200 R08:  R09: 
1999
  [  154.028574] R10: 0891 R11: 0206 R12: 
7ffc060d74ef
  [  154.028576] R13:  R14: 02053200 R15: 
02052010

  [  154.028588] Allocated by task 164:
  [  154.028603]  cxd2841er_attach+0xc3/0x7f0 [cxd2841er]
  [  154.028608]  demod_attach_cxd28xx+0x14c/0x3f0 [ddbridge]
  [  154.028612]  dvb_input_attach+0x671/0x1e20 [ddbridge]
  [  154.028616]  ddb_ports_attach+0x453/0xd00 [ddbridge]
  [  154.028620]  ddb_init+0x4b3/0xa30 [ddbridge]
  [  154.028624]  ddb_probe+0xa51/0xfe0 [ddbridge]
  [  154.028627]  pci_device_probe+0x279/0x480
  [  154.028630]  driver_probe_device+0x46f/0x7a0
  [  154.028632]  __driver_attach+0x133/0x170
  [  154.028634]  bus_for_each_dev+0x10a/0x190
  [  154.028637]  bus_add_driver+0x2a3/0x5a0
  [  154.028639]  driver_register+0x182/0x3a0
  [  154.028641]  0xa016808f
  [  154.028643]  do_one_initcall+0x77/0x1d0
  [  154.028646]  do_init_module+0x1c2/0x548
  [  154.028648]  load_module+0x5e61/0x8df0
  [  154.028650]  SyS_finit_module+0x142/0x150
  [  154.028652]  do_syscall_64+0x179/0x4c0
  [  154.028654]  return_from_SYSCALL_64+0x0/0x65

  [  154.028664] Freed by task 285:
  [  154.028676]  kfree+0x6c/0xa0
  [  154.028682]  __dvb_frontend_free+0x81/0xb0 [dvb_core]
  [  154.028687]  dvb_input_detach.isra.24+0x17c/0x480 [ddbridge]
  [  154.028691]  ddb_ports_detach+0x176/0x630 [ddbridge]
  [  154.028695]  ddb_remove+0x3c/0xb0 [ddbridge]
  [  154.028697]  pci_device_remove+0x93/0x1d0
  [  154.028700]  device_release_driver_internal+0x267/0x510
  [  154.028702]  driver_detach+0xb9/0x1b0
  [  154.028705]  bus_remove_driver+0xd0/0x1f0
  [  154.028707]  pci_unregister_driver+0x25/0x210
  [  154.028711]  module_exit_ddbridge+0xc/0x45 [ddbridge]
  [  154.028714]  SyS_delete_module+0x314/0x440
  [  154.028716]  do_syscall_64+0x179/0x4c0
  [  154.028718]  return_from_SYSCALL_64+0x0/0x65

  [  154.028729] The buggy address belongs to the object at 880108b55340
  which belongs to the cache kmalloc-2048 of size 2048
  [  

Re: [PATCH 5/5] v4l2: async: Add debug output to v4l2-async module

2017-12-15 Thread Sakari Ailus
Hi Jacopo,

On Wed, Dec 13, 2017 at 07:26:20PM +0100, Jacopo Mondi wrote:
> The v4l2-async module operations are quite complex to follow, due to the
> asynchronous nature of subdevices and notifiers registration and
> matching procedures. In order to help with debugging of failed or
> erroneous matching between a subdevice and the notifier collected
> async_subdevice it gets matched against, introduce a few dev_dbg() calls
> in v4l2_async core operations.
> 
> Protect the debug operations with a Kconfig defined symbol, to make sure
> when debugging is disabled, no additional code or data is added to the
> module.
> 
> Notifiers are identified by the name of the subdevice or v4l2_dev they are
> registered by, while subdevice matching which now happens on endpoints,
> need a longer description built walking the fwnode graph backwards
> collecting parent nodes names (otherwise we would have had printouts
> like: "Matching "endpoint" with "endpoint"" which are not that useful).
> 
> Signed-off-by: Jacopo Mondi 
> 
> ---
> For fwnodes backed by OF, I may have used the "%pOF" format modifier to
> get the full node name instead of parsing the fwnode graph by myself with
> "v4l2_async_fwnode_full_name()". Unfortunately I'm not aware of anything
> like "%pOF" for ACPI backed fwnodes. Also, walking the fwnode graph by
> myself allows me to reduce the depth, to reduce the debug messages output
> length which is anyway long enough to result disturbing on a 80columns
> terminal window.

ACPI doesn't have such at the moment. I think printing the full path would
still be better. There isn't that much more to print after all.

> ---
> 
>  drivers/media/v4l2-core/Kconfig  |  8 
>  drivers/media/v4l2-core/v4l2-async.c | 81 
> 
>  2 files changed, 89 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig
> index a35c336..8331736 100644
> --- a/drivers/media/v4l2-core/Kconfig
> +++ b/drivers/media/v4l2-core/Kconfig
> @@ -17,6 +17,14 @@ config VIDEO_ADV_DEBUG
> V4L devices.
> In doubt, say N.
> 
> +config VIDEO_V4L2_ASYNC_DEBUG
> + bool "Enable debug functionalities for V4L2 async module"
> + depends on VIDEO_V4L2

I'm not sure I'd add a Kconfig option. This is adding a fairly simple
function only to the kernel.

> + default n
> + ---help---
> +   Say Y here to enable debug output in V4L2 async module.
> +   In doubt, say N.
> +
>  config VIDEO_FIXED_MINOR_RANGES
>   bool "Enable old-style fixed minor ranges on drivers/video devices"
>   default n
> diff --git a/drivers/media/v4l2-core/v4l2-async.c 
> b/drivers/media/v4l2-core/v4l2-async.c
> index c13a781..307e1a5 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -8,6 +8,10 @@
>   * published by the Free Software Foundation.
>   */
> 
> +#if defined(CONFIG_VIDEO_V4L2_ASYNC_DEBUG)
> +#define DEBUG

Do you need this?

> +#endif
> +
>  #include 
>  #include 
>  #include 
> @@ -25,6 +29,52 @@
>  #include 
>  #include 
> 
> +#if defined(CONFIG_VIDEO_V4L2_ASYNC_DEBUG)
> +#define V4L2_ASYNC_FWNODE_NAME_LEN   512
> +
> +static void __v4l2_async_fwnode_full_name(char *name,
> +   unsigned int len,
> +   unsigned int max_depth,
> +   struct fwnode_handle *fwnode)
> +{
> + unsigned int buf_len = len < V4L2_ASYNC_FWNODE_NAME_LEN ?
> +len : V4L2_ASYNC_FWNODE_NAME_LEN;
> + char __tmp[V4L2_ASYNC_FWNODE_NAME_LEN];

That's a bit too much to allocate from the stack I think.

> + struct fwnode_handle *parent;
> +
> + memset(name, 0, buf_len);
> + buf_len -= snprintf(__tmp, buf_len, "%s", fwnode_get_name(fwnode));
> +
> + parent = fwnode;
> + while ((parent = fwnode_get_parent(parent)) && buf_len &&
> + --max_depth) {
> + buf_len -= snprintf(name, buf_len, "%s/%s",
> + fwnode_get_name(parent), __tmp);
> + strcpy(__tmp, name);
> + }
> +}
> +
> +static void v4l2_async_fwnode_full_name(char *name,
> + unsigned int len,
> + struct fwnode_handle *fwnode)
> +{
> + /*
> +  * Usually 4 as nesting level is sufficient to identify an
> +  * endpoint firmware node uniquely.
> +  */
> + __v4l2_async_fwnode_full_name(name, len, 4, fwnode);
> +}
> +
> +#else /* CONFIG_VIDEO_V4L2_ASYNC_DEBUG */
> +#define V4L2_ASYNC_FWNODE_NAME_LEN   0
> +
> +static void v4l2_async_fwnode_full_name(char *name,
> + unsigned int len,
> + struct fwnode_handle *fwnode)
> +{
> +}
> +#endif /* CONFIG_VIDEO_V4L2_ASYNC_DEBUG */
> +
>  static struct device *v4l2_async_notifier_dev(
>   struct 

Re: [PATCH v2] media: s5p-mfc: Add support for V4L2_MEMORY_DMABUF type

2017-12-15 Thread Hans Verkuil
Hi Marek,

On 14/12/17 15:11, Marek Szyprowski wrote:
> Hi Hans,
> 
> I would like to get your opinion on this patch. Do you think it makes sense 
> to:
> 
> 1. add limited support for USERPTR and DMA-buf import? (limited means driver 
> will accept setting buffer pointer/fd only once after reqbufs for each buffer 
> index)

I don't like this. It's unexpected almost-but-not-quite behavior that will make
life very difficult for userspace.

> 
> 2. add a V4L2 device flag to let userspace to discover if device support 
> queue buffer reconfiguration on-fly or not?

This seems to me a better approach. It should be possible to implement most/all 
of this
in vb2, but we need to find a way to signal this to the user.

Is this an MFC limitation for the decoder, encoder or both? And is it a 
limitation
of the capture or output side or both?

Regards,

Hans

> 
> Here is the discussion: https://patchwork.linuxtv.org/patch/45305/
> 
> Best regards
> Marek Szyprowski, PhD
> Samsung R Institute Poland
> 
> 
> On 2017-11-06 20:21, Nicolas Dufresne wrote:
>> Le lundi 06 novembre 2017 à 10:28 +0100, Marek Szyprowski a écrit :
>>> On 2017-11-03 14:45, Nicolas Dufresne wrote:
 Le vendredi 03 novembre 2017 à 09:11 +0100, Marek Szyprowski a écrit :
> MFC driver supports only MMAP operation mode mainly due to the hardware
> restrictions of the addresses of the DMA buffers (MFC v5 hardware can
> access buffers only in 128MiB memory region starting from the base address
> of its firmware). When IOMMU is available, this requirement is easily
> fulfilled even for the buffers located anywhere in the memory - typically
> by mapping them in the DMA address space as close as possible to the
> firmware. Later hardware revisions don't have this limitations at all.
>
> The second limitation of the MFC hardware related to the memory buffers
> is constant buffer address. Once the hardware has been initialized for
> operation on given buffer set, the addresses of the buffers cannot be
> changed.
>
> With the above assumptions, a limited support for USERPTR and DMABUF
> operation modes can be added. The main requirement is to have all buffers
> known when starting hardware. This has been achieved by postponing
> hardware initialization once all the DMABUF or USERPTR buffers have been
> queued for the first time. Once then, buffers cannot be modified to point
> to other memory area.
 I am concerned about enabling this support with existing userspace.
 Userspace application will be left with some driver with this
 limitation and other drivers without. I think it is harmful to enable
 that feature without providing userspace the ability to know.

 This is specially conflicting with let's say UVC driver providing
 buffers, since UVC driver implementing CREATE_BUFS ioctl. So even if
 userspace start making an effort to maintain the same DMABuf for the
 same buffer index, if a new buffer is created, we won't be able to use
 it.
>>> Sorry, but I don't get this as an 'issue'. The typical use scenario is to
>>> configure buffer queue and start streaming. Basically ReqBufs, stream on and
>>> a sequence of bufq/bufdq. This is handled without any problem by MFC driver
>>> with this patch. After releasing a queue with reqbufs(0), one can use
>>> different set of buffers.
>> In real life, you often have capture code decorelated from the encoder
>> code. At least, it's the case in GStreamer. The encoder have no
>> information about how many buffers were pre-allocated by let's say the
>> capture driver. As a side effect, we cannot make sure the importation
>> queue is of the same size (amount of buffer). Specially if you have UVC
>> driver that allow allocating more buffers at run-time. This is used in
>> real-life to compensate additional latency that get's added when a
>> pipeline topology is changed (at run-time). Only workaround I had in
>> mind, would be to always prepare the queue with the maximum (32), and
>> use this as a cache size, but for now, this is not how the deployed
>> userspace works unfortunately.
>>
>> Your reqbuf(0) technique cause a break in the stream (probably a new
>> keyframe), as you are forced to STREAMOFF. This is often unwanted, and
>> it may create a time discontinuity in case the allocation took time.
>>
>>> What is the point of changing the buffers during the streaming? IMHO it was
>>> one of the biggest pathology of the V4L2 USERPTR API that the buffers
>>> were in
>>> fact 'created' on the first queue operation. By creating I mean creating all
>>> the kernel all needed structures and mappings between the real memory (user
>>> ptr value) and the buffer index. The result of this was userspace, which
>>> don't
>>> use buffer indices and always queues buffers with index = 0, what means that
>>> kernel has to reacquire direct access to each buffer every single frame.
>>> That
>>> is highly inefficient 

[PATCH v8] intel-ipu3: cio2: fix two warnings in the code

2017-12-15 Thread Yong Zhi
Fix two warnings reported by static analysis tool:

ipu3-cio2.c:1899:16: warning: Variable length array is used.

In function 'cio2_pci_probe':
ipu3-cio2.c:1726:14: warning: variable 'phys' set
but not used [-Wunused-but-set-variable]

Signed-off-by: Yong Zhi 
---
Hi, Mauro, thanks for catching the warnings.

Hi, Sakari, can you squash the patch to your tree? Thanks!!

 drivers/media/pci/intel/ipu3/ipu3-cio2.c | 16 ++--
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c 
b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
index 4295bdb8b192..941caa987dab 100644
--- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c
+++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
@@ -1723,7 +1723,6 @@ static int cio2_pci_probe(struct pci_dev *pci_dev,
  const struct pci_device_id *id)
 {
struct cio2_device *cio2;
-   phys_addr_t phys;
void __iomem *const *iomap;
int r;
 
@@ -1741,8 +1740,6 @@ static int cio2_pci_probe(struct pci_dev *pci_dev,
dev_info(_dev->dev, "device 0x%x (rev: 0x%x)\n",
 pci_dev->device, pci_dev->revision);
 
-   phys = pci_resource_start(pci_dev, CIO2_PCI_BAR);
-
r = pcim_iomap_regions(pci_dev, 1 << CIO2_PCI_BAR, pci_name(pci_dev));
if (r) {
dev_err(_dev->dev, "failed to remap I/O memory (%d)\n", r);
@@ -1896,7 +1893,6 @@ static void arrange(void *ptr, size_t elem_size, size_t 
elems, size_t start)
{ 0, start - 1 },
{ start, elems - 1 },
};
-   u8 tmp[elem_size];
 
 #define arr_size(a) ((a)->end - (a)->begin + 1)
 
@@ -1915,12 +1911,12 @@ static void arrange(void *ptr, size_t elem_size, size_t 
elems, size_t start)
 
/* Swap the entries in two parts of the array. */
for (i = 0; i < size0; i++) {
-   memcpy(tmp, ptr + elem_size * (arr[1].begin + i),
-  elem_size);
-   memcpy(ptr + elem_size * (arr[1].begin + i),
-  ptr + elem_size * (arr[0].begin + i), elem_size);
-   memcpy(ptr + elem_size * (arr[0].begin + i), tmp,
-  elem_size);
+   u8 *d = ptr + elem_size * (arr[1].begin + i);
+   u8 *s = ptr + elem_size * (arr[0].begin + i);
+   size_t j;
+
+   for (j = 0; j < elem_size; j++)
+   swap(d[j], s[j]);
}
 
if (arr_size([0]) > arr_size([1])) {
-- 
2.7.4



Re: [PATCH 4/5] v4l2: async: Postpone subdev_notifier registration

2017-12-15 Thread Sakari Ailus
Hi Jacopo,

On Wed, Dec 13, 2017 at 07:26:19PM +0100, Jacopo Mondi wrote:
> Currently, subdevice notifiers are tested against all available
> subdevices as soon as they get registered. It often happens anyway
> that the subdevice they are connected to is not yet initialized, as
> it usually gets registered later in drivers' code. This makes debug
> of v4l2_async particularly painful, as identifying a notifier with
> an unitialized subdevice is tricky as they don't have a valid
> 'struct device *' or 'struct fwnode_handle *' to be identified with.
> 
> In order to make sure that the notifier's subdevices is initialized
> when the notifier is tesed against available subdevices post-pone the
> actual notifier registration at subdevice registration time.
> 
> It is worth noting that post-poning registration of a subdevice notifier
> does not impact on the completion of the notifiers chain, as even if a
> subdev notifier completes as soon as it gets registered, the complete()
> call chain cannot be upscaled as long as the subdevice the notifiers
> belongs to is not registered.

Let me rephrase to make sure I understand the problem correctly ---

A sub-device notifier is registered but the notifier's sub-device is not
registered yet, and finding a match for this notifier leads, to, well
problems. Is that the reason for this patch?

I think there could be simpler solutions to address this.

I wonder if we could simply check for sub-device notifier's v4l2_dev field,
and fail in matching if it's not set. v4l2_device_register_subdev() would
still need to proceed with calling v4l2_async_notifier_try_all_subdevs()
and v4l2_async_notifier_try_complete() if that was the case.

What do you think?

> 
> Also, it is now safe to access a notifier 'struct device *' as we're now
> sure it is properly initialized when the notifier is actually
> registered.
> 
> Signed-off-by: Jacopo Mondi 
> ---
>  drivers/media/v4l2-core/v4l2-async.c | 65 
> +++-
>  include/media/v4l2-async.h   |  2 ++
>  2 files changed, 43 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-async.c 
> b/drivers/media/v4l2-core/v4l2-async.c
> index 0a1bf1d..c13a781 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -25,6 +25,13 @@
>  #include 
>  #include 
> 
> +static struct device *v4l2_async_notifier_dev(
> + struct v4l2_async_notifier *notifier)
> +{
> + return notifier->v4l2_dev ? notifier->v4l2_dev->dev :
> + notifier->sd->dev;
> +}
> +
>  static int v4l2_async_notifier_call_bound(struct v4l2_async_notifier *n,
> struct v4l2_subdev *subdev,
> struct v4l2_async_subdev *asd)
> @@ -124,19 +131,6 @@ static struct v4l2_async_subdev *v4l2_async_find_match(
>   return NULL;
>  }
> 
> -/* Find the sub-device notifier registered by a sub-device driver. */
> -static struct v4l2_async_notifier *v4l2_async_find_subdev_notifier(
> - struct v4l2_subdev *sd)
> -{
> - struct v4l2_async_notifier *n;
> -
> - list_for_each_entry(n, _list, list)
> - if (n->sd == sd)
> - return n;
> -
> - return NULL;
> -}
> -
>  /* Get v4l2_device related to the notifier if one can be found. */
>  static struct v4l2_device *v4l2_async_notifier_find_v4l2_dev(
>   struct v4l2_async_notifier *notifier)
> @@ -160,7 +154,7 @@ static bool v4l2_async_notifier_can_complete(
> 
>   list_for_each_entry(sd, >done, async_list) {
>   struct v4l2_async_notifier *subdev_notifier =
> - v4l2_async_find_subdev_notifier(sd);
> + sd->subdev_notifier;
> 
>   if (subdev_notifier &&
>   !v4l2_async_notifier_can_complete(subdev_notifier))
> @@ -228,7 +222,7 @@ static int v4l2_async_match_notify(struct 
> v4l2_async_notifier *notifier,
>   /*
>* See if the sub-device has a notifier. If not, return here.
>*/
> - subdev_notifier = v4l2_async_find_subdev_notifier(sd);
> + subdev_notifier = sd->subdev_notifier;
>   if (!subdev_notifier || subdev_notifier->parent)
>   return 0;
> 
> @@ -294,7 +288,7 @@ static void v4l2_async_notifier_unbind_all_subdevs(
> 
>   list_for_each_entry_safe(sd, tmp, >done, async_list) {
>   struct v4l2_async_notifier *subdev_notifier =
> - v4l2_async_find_subdev_notifier(sd);
> + sd->subdev_notifier;
> 
>   if (subdev_notifier)
>   v4l2_async_notifier_unbind_all_subdevs(subdev_notifier);
> @@ -371,8 +365,7 @@ static bool v4l2_async_notifier_fwnode_has_async_subdev(
> 
>  static int __v4l2_async_notifier_register(struct v4l2_async_notifier 
> *notifier)
>  {
> - 

RE: [RESEND GIT PULL for 4.16] Intel IPU3 CIO2 CSI-2 receiver driver

2017-12-15 Thread Zhi, Yong
Hi, Mauro and Sakari,

Sorry for the late response, the fix of the 2 warnings is attached.

Best regards,

Yong

From: Zhi, Yong
Sent: Friday, December 08, 2017 3:32 PM
To: Mauro Carvalho Chehab; Sakari Ailus
Cc: linux-media@vger.kernel.org; Mani, Rajmohan
Subject: RE: [RESEND GIT PULL for 4.16] Intel IPU3 CIO2 CSI-2 receiver driver

Hi, Mauro,

> -Original Message-
> From: Mauro Carvalho Chehab [mailto:mche...@s-opensource.com]
> Sent: Friday, December 8, 2017 7:00 AM
> To: Sakari Ailus 
> Cc: linux-media@vger.kernel.org; Mani, Rajmohan
> ; Zhi, Yong 
> Subject: Re: [RESEND GIT PULL for 4.16] Intel IPU3 CIO2 CSI-2 receiver driver
>
> Em Fri, 1 Dec 2017 16:31:36 +0200
> Sakari Ailus  escreveu:
>
> > Hi Mauro,
> >
> > Here's the Intel IPU3 CIO2 CSI-2 receiver driver, with the
> > accompanying format definitions.
>
> This patch generates two warnings:
>
> drivers/media/pci/intel/ipu3/ipu3-cio2.c:1899:16: warning: Variable length
> array is used.
> drivers/media/pci/intel/ipu3/ipu3-cio2.c: In function 'cio2_pci_probe':
> drivers/media/pci/intel/ipu3/ipu3-cio2.c:1726:14: warning: variable 'phys'
> set but not used [-Wunused-but-set-variable]
>   phys_addr_t phys;
>   ^~~~
>
> We should never use variable-length array on Kernel, as Linux stack is very
> limited, and we have static analyzers to check for it at compilation time.
>
> Also, the logic should check if pci_resource_start() succeeded, instead of
> just ignoring it.
>
> Please fix.
>

Thanks for catching the warnings, for the variable size array, from the code 
context,
the size is limited to 128 bytes, maybe this language feature itself is not 
recommended,
we will send a patch to address above soon.

>
> >
> > Please pull.
> >
> >
> > The following changes since commit
> be9b53c83792e3898755dce90f8c632d40e7c83e:
> >
> >   media: dvb-frontends: complete kernel-doc markups (2017-11-30
> > 04:19:05 -0500)
> >
> > are available in the git repository at:
> >
> >   ssh://linuxtv.org/git/sailus/media_tree.git ipu3
> >
> > for you to fetch changes up to
> f178207daa68e817ab6fd702d81ed7c8637ab72c:
> >
> >   intel-ipu3: cio2: add new MIPI-CSI2 driver (2017-11-30 14:19:47
> > +0200)
> >
> > 
> > Yong Zhi (3):
> >   videodev2.h, v4l2-ioctl: add IPU3 raw10 color format
> >   doc-rst: add IPU3 raw10 bayer pixel format definitions
> >   intel-ipu3: cio2: add new MIPI-CSI2 driver
> >
> >  Documentation/media/uapi/v4l/pixfmt-rgb.rst|1 +
> >  .../media/uapi/v4l/pixfmt-srggb10-ipu3.rst |  335 
> >  MAINTAINERS|8 +
> >  drivers/media/pci/Kconfig  |2 +
> >  drivers/media/pci/Makefile |3 +-
> >  drivers/media/pci/intel/Makefile   |5 +
> >  drivers/media/pci/intel/ipu3/Kconfig   |   19 +
> >  drivers/media/pci/intel/ipu3/Makefile  |1 +
> >  drivers/media/pci/intel/ipu3/ipu3-cio2.c   | 2052
> 
> >  drivers/media/pci/intel/ipu3/ipu3-cio2.h   |  449 +
> >  drivers/media/v4l2-core/v4l2-ioctl.c   |4 +
> >  include/uapi/linux/videodev2.h |6 +
> >  12 files changed, 2884 insertions(+), 1 deletion(-)  create mode
> > 100644 Documentation/media/uapi/v4l/pixfmt-srggb10-ipu3.rst
> >  create mode 100644 drivers/media/pci/intel/Makefile  create mode
> > 100644 drivers/media/pci/intel/ipu3/Kconfig
> >  create mode 100644 drivers/media/pci/intel/ipu3/Makefile
> >  create mode 100644 drivers/media/pci/intel/ipu3/ipu3-cio2.c
> >  create mode 100644 drivers/media/pci/intel/ipu3/ipu3-cio2.h
> >
>
>
>
> Thanks,
> Mauro
From d7e9baf13b1a8c8308906a7ed213f75d12756c24 Mon Sep 17 00:00:00 2001
From: Yong Zhi 
Date: Tue, 12 Dec 2017 10:16:31 -0600
Subject: [PATCH] [PATCH v8] intel-ipu3: cio2: fix two warnings in the code

Fix two warnings reported by Mauro Carvalho Chehab:

ipu3-cio2.c:1899:16: warning: Variable length array is used.

In function 'cio2_pci_probe':
ipu3-cio2.c:1726:14: warning: variable 'phys' set
but not used [-Wunused-but-set-variable]

Hi, Sakari, can you squash the patch to your tree?

Signed-off-by: Yong Zhi 
---
 drivers/media/pci/intel/ipu3/ipu3-cio2.c | 16 ++--
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
index 4295bdb8b192..941caa987dab 100644
--- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c
+++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
@@ -1723,7 +1723,6 @@ static int cio2_pci_probe(struct pci_dev *pci_dev,
 			  const struct pci_device_id *id)
 {
 	struct cio2_device *cio2;
-	phys_addr_t phys;
 	void __iomem *const *iomap;
 	int r;
 
@@ -1741,8 +1740,6 @@ 

[GIT PULL FOR v4.16] Various fixes

2017-12-15 Thread Hans Verkuil
The following changes since commit 0ca4e3130402caea8731a7b54afde56a6edb17c9:

  media: pxa_camera: rename the soc_camera_ prefix to pxa_camera_ (2017-12-14 
12:40:01 -0500)

are available in the Git repository at:

  git://linuxtv.org/hverkuil/media_tree.git for-v4.16c

for you to fetch changes up to 03672a5bd2ba76b8b54a296e51e85919529d23b8:

  bdisp: Fix a possible sleep-in-atomic bug in bdisp_hw_save_request 
(2017-12-15 15:57:12 +0100)


Flavio Ceolin (1):
  media: pxa_camera: disable and unprepare the clock source on error

Hans Verkuil (1):
  pvrusb2: correctly return V4L2_PIX_FMT_MPEG in enum_fmt

Jacopo Mondi (1):
  v4l: sh_mobile_ceu: Return buffers on streamoff()

Jia-Ju Bai (1):
  bdisp: Fix a possible sleep-in-atomic bug in bdisp_hw_save_request

Lucas Stach (1):
  coda: set min_buffers_needed

Philipp Zabel (5):
  coda: fix capture TRY_FMT for YUYV with non-MB-aligned widths
  coda: round up frame sizes to multiples of 16 for MPEG-4 decoder
  coda: allocate space for mpeg4 decoder mvcol buffer
  coda: use correct offset for mpeg4 decoder mvcol buffer
  vb2: clear V4L2_BUF_FLAG_LAST when filling vb2_buffer

Stanimir Varbanov (1):
  media: vb2: unify calling of set_page_dirty_lock

 drivers/media/platform/coda/coda-bit.c   | 23 
---
 drivers/media/platform/coda/coda-common.c| 13 ++---
 drivers/media/platform/pxa_camera.c  |  4 +++-
 drivers/media/platform/soc_camera/sh_mobile_ceu_camera.c |  7 ++-
 drivers/media/platform/sti/bdisp/bdisp-hw.c  |  2 +-
 drivers/media/usb/pvrusb2/pvrusb2-v4l2.c | 32 
+++-
 drivers/media/v4l2-core/videobuf2-dma-contig.c   |  6 --
 drivers/media/v4l2-core/videobuf2-dma-sg.c   |  7 +++
 drivers/media/v4l2-core/videobuf2-v4l2.c |  2 ++
 9 files changed, 48 insertions(+), 48 deletions(-)


Re: [PATCH 2/2] media: coda: Add i.MX51 (CodaHx4) support

2017-12-15 Thread Philipp Zabel
Hi Hans,

On Fri, 2017-12-15 at 15:22 +0100, Hans Verkuil wrote:
> Hi Philipp,
> 
> On 13/12/17 15:09, Philipp Zabel wrote:
> > Add support for the CodaHx4 VPU used on i.MX51.
> > 
> > Decoding h.264, MPEG-4, and MPEG-2 video works, as well as encoding
> > h.264. MPEG-4 encoding is not enabled, it currently produces visual
> > artifacts.
> > 
> > Signed-off-by: Philipp Zabel 
> > ---
> >  drivers/media/platform/coda/coda-bit.c| 45 
> > ++-
> >  drivers/media/platform/coda/coda-common.c | 44 
> > +++---
> >  drivers/media/platform/coda/coda.h|  1 +
> >  3 files changed, 74 insertions(+), 16 deletions(-)
> > 
> 
> 
> 
> > +   [CODA_IMX51] = {
> > +   .firmware = {
> > +   "vpu_fw_imx51.bin",
> > +   "vpu/vpu_fw_imx51.bin",
> > +   "v4l-codahx4-imx51.bin"
> > +   },
> > +   .product  = CODA_HX4,
> > +   .codecs   = codahx4_codecs,
> > +   .num_codecs   = ARRAY_SIZE(codahx4_codecs),
> > +   .vdevs= codahx4_video_devices,
> > +   .num_vdevs= ARRAY_SIZE(codahx4_video_devices),
> > +   .workbuf_size = 128 * 1024,
> > +   .tempbuf_size = 304 * 1024,
> > +   .iram_size= 0x14000,
> > +   },
> 
> What's the status of the firmware? Is it going to be available in some 
> firmware
> repository? I remember when testing other imx devices that it was a bit tricky
> to get hold of the firmware. And googling v4l-codahx4-imx51.bin doesn't find
> anything other than this patch.

As far as I am aware, so far all efforts to get these firmware binaries
relicensed in a way that makes them redistributable in linux-firmware
have not succeeded.

They are distributed by NXP directly in the firmware-imx package.
The http://git.yoctoproject.org/cgit/cgit.cgi/meta-fsl-arm/ repository
contains links to the latest version:

  wget http://www.nxp.com/lgfiles/NMG/MAD/YOCTO/firmware-imx-5.4.bin
  dd if=firmware-imx-5.4.bin bs=34087 skip=1 | tar xj
  cat firmware-imx-5.4/COPYING

regards
Philipp


Re: [PATCH 2/2] bdisp: Fix a possible sleep-in-atomic bug in bdisp_hw_save_request

2017-12-15 Thread Fabien DESSENNE
Hi


Thank you for the patch.


On 12/12/17 14:47, Jia-Ju Bai wrote:
> The driver may sleep under a spinlock.
> The function call path is:
> bdisp_device_run (acquire the spinlock)
>bdisp_hw_update
>  bdisp_hw_save_request
>devm_kzalloc(GFP_KERNEL) --> may sleep
>
> To fix it, GFP_KERNEL is replaced with GFP_ATOMIC.
>
> This bug is found by my static analysis tool(DSAC) and checked by my code 
> review.
>
> Signed-off-by: Jia-Ju Bai 
Reviewed-by: Fabien Dessenne 
> ---
>   drivers/media/platform/sti/bdisp/bdisp-hw.c |2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/sti/bdisp/bdisp-hw.c 
> b/drivers/media/platform/sti/bdisp/bdisp-hw.c
> index 4b62ceb..7b45b43 100644
> --- a/drivers/media/platform/sti/bdisp/bdisp-hw.c
> +++ b/drivers/media/platform/sti/bdisp/bdisp-hw.c
> @@ -1064,7 +1064,7 @@ static void bdisp_hw_save_request(struct bdisp_ctx *ctx)
>   if (!copy_node[i]) {
>   copy_node[i] = devm_kzalloc(ctx->bdisp_dev->dev,
>   sizeof(*copy_node[i]),
> - GFP_KERNEL);
> + GFP_ATOMIC);
>   if (!copy_node[i])
>   return;
>   }


Re: [PATCH 1/2] bdisp: Fix a possible sleep-in-atomic bug in bdisp_hw_reset

2017-12-15 Thread Fabien DESSENNE
Hi

On 12/12/17 14:47, Jia-Ju Bai wrote:
> The driver may sleep under a spinlock.
> The function call path is:
> bdisp_device_run (acquire the spinlock)
>bdisp_hw_reset
>  msleep --> may sleep
>
> To fix it, msleep is replaced with mdelay.

May I suggest you to use readl_poll_timeout_atomic (instead of the whole 
"for" block): this fixes the problem and simplifies the code?

>
> This bug is found by my static analysis tool(DSAC) and checked by my code 
> review.
>
> Signed-off-by: Jia-Ju Bai 
> ---
>   drivers/media/platform/sti/bdisp/bdisp-hw.c |2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/sti/bdisp/bdisp-hw.c 
> b/drivers/media/platform/sti/bdisp/bdisp-hw.c
> index b7892f3..4b62ceb 100644
> --- a/drivers/media/platform/sti/bdisp/bdisp-hw.c
> +++ b/drivers/media/platform/sti/bdisp/bdisp-hw.c
> @@ -382,7 +382,7 @@ int bdisp_hw_reset(struct bdisp_dev *bdisp)
>   for (i = 0; i < POLL_RST_MAX; i++) {
>   if (readl(bdisp->regs + BLT_STA1) & BLT_STA1_IDLE)
>   break;
> - msleep(POLL_RST_DELAY_MS);
> + mdelay(POLL_RST_DELAY_MS);
>   }
>   if (i == POLL_RST_MAX)
>   dev_err(bdisp->dev, "Reset timeout\n");


Re: [PATCH 3/5] include: v4l2_async: Add 'owner' field to notifier

2017-12-15 Thread Sakari Ailus
Hi Jacopo,

On Wed, Dec 13, 2017 at 07:26:18PM +0100, Jacopo Mondi wrote:
> Notifiers can be registered as root notifiers (identified by a 'struct
> v4l2_device *') or subdevice notifiers (identified by a 'struct
> v4l2_subdev *'). In order to identify a notifier no matter if it is root
> or not, add a 'struct fwnode_handle *owner' field, whose name can be
> printed out for debug purposes.
> 
> Signed-off-by: Jacopo Mondi 

You'll have struct device either through the v4l2_device or v4l2_subdev. Do
you need an additional field for this?

-- 
Sakari Ailus
sakari.ai...@linux.intel.com


Re: [PATCH/RFC v2 10/15] adv748x: csi2: add translation from pixelcode to CSI-2 datatype

2017-12-15 Thread jacopo mondi
Hi Kieran,

On Thu, Dec 14, 2017 at 10:25:36PM +, Kieran Bingham wrote:
> Hi Niklas,
>
> On 14/12/17 19:08, Niklas Söderlund wrote:
> > This will be needed to fill out the frame descriptor information
> > correctly.
> >
> > Signed-off-by: Niklas Söderlund 
> > ---
> >  drivers/media/i2c/adv748x/adv748x-csi2.c | 22 ++
> >  1 file changed, 22 insertions(+)
> >
> > diff --git a/drivers/media/i2c/adv748x/adv748x-csi2.c 
> > b/drivers/media/i2c/adv748x/adv748x-csi2.c
> > index 2a5dff8c571013bf..a2a6d93077204731 100644
> > --- a/drivers/media/i2c/adv748x/adv748x-csi2.c
> > +++ b/drivers/media/i2c/adv748x/adv748x-csi2.c
> > @@ -18,6 +18,28 @@
> >
> >  #include "adv748x.h"
> >
> > +struct adv748x_csi2_format {
> > +   unsigned int code;
> > +   unsigned int datatype;
> > +};
> > +
> > +static const struct adv748x_csi2_format adv748x_csi2_formats[] = {
> > +   { .code = MEDIA_BUS_FMT_RGB888_1X24,.datatype = 0x24, },
> > +   { .code = MEDIA_BUS_FMT_UYVY8_1X16, .datatype = 0x1e, },
> > +   { .code = MEDIA_BUS_FMT_UYVY8_2X8,  .datatype = 0x1e, },
> > +   { .code = MEDIA_BUS_FMT_YUYV10_2X10,.datatype = 0x1e, },

YUV 422 10 bit is associated to data type 0x1d in CSI-2 specifications

> > +};
>
> Is the datatype mapping specific to the ADV748x here?
> or are these generic/common CSI2 mappings?
>
> What do those datatype magic numbers represent?

They are fixed mappings defined by CSI-2 specifications and they
should probably be generic to all drivers imho

>
> --
> Kieran
>
> > +
> > +static unsigned int adv748x_csi2_code_to_datatype(unsigned int code)
> > +{
> > +   unsigned int i;
> > +
> > +   for (i = 0; i < ARRAY_SIZE(adv748x_csi2_formats); i++)
> > +   if (adv748x_csi2_formats[i].code == code)
> > +   return adv748x_csi2_formats[i].datatype;
> > +   return 0;
> > +}
> > +
> >  static bool is_txa(struct adv748x_csi2 *tx)
> >  {
> > return tx == >state->txa;
> >


Re: [PATCH 2/5] device property: Add fwnode_get_name() operation

2017-12-15 Thread Sakari Ailus
Hi Jacopo,

Thanks for the patch.

Could you cc the next version to linux-a...@vger.kernel.org, please? Cc
Mika, too.

On Wed, Dec 13, 2017 at 07:26:17PM +0100, Jacopo Mondi wrote:
> Add operation to retrieve the device name from a fwnode handle.
> 
> Signed-off-by: Jacopo Mondi 
> ---
>  drivers/acpi/property.c  |  6 ++
>  drivers/base/property.c  | 12 
>  drivers/of/property.c|  6 ++
>  include/linux/fwnode.h   |  2 ++
>  include/linux/property.h |  1 +
>  5 files changed, 27 insertions(+)
> 
> diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
> index e26ea20..1e3971c 100644
> --- a/drivers/acpi/property.c
> +++ b/drivers/acpi/property.c
> @@ -1186,6 +1186,11 @@ acpi_fwnode_property_read_string_array(const struct 
> fwnode_handle *fwnode,
>  val, nval);
>  }
>  
> +static const char *acpi_fwnode_get_name(const struct fwnode_handle *fwnode)
> +{
> + return acpi_dev_name(to_acpi_device_node(fwnode));

This works for device nodes but will fail miserably for non-device ACPI
nodes.

The ACPI nodes don't currently have a name such as the DT nodes do, it
would certainly help debugging if they did.

What you'll at least need to do is to check to_acpi_device_node() does not
return NULL.

acpi_dev_name() should be made const as well if the function is still used
in v2.

> +}
> +
>  static struct fwnode_handle *
>  acpi_fwnode_get_named_child_node(const struct fwnode_handle *fwnode,
>const char *childname)
> @@ -1281,6 +1286,7 @@ static int acpi_fwnode_graph_parse_endpoint(const 
> struct fwnode_handle *fwnode,
>   acpi_fwnode_property_read_string_array, \
>   .get_parent = acpi_node_get_parent, \
>   .get_next_child_node = acpi_get_next_subnode,   \
> + .get_name = acpi_fwnode_get_name,   \
>   .get_named_child_node = acpi_fwnode_get_named_child_node, \
>   .get_reference_args = acpi_fwnode_get_reference_args,   \
>   .graph_get_next_endpoint =  \
> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index 851b1b6..a87b4a9 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -950,6 +950,18 @@ int device_add_properties(struct device *dev,
>  EXPORT_SYMBOL_GPL(device_add_properties);
>  
>  /**
> + * fwnode_get_name - Return the fwnode_handle name
> + * @fwnode: Firmware node to get name from
> + *
> + * Returns a pointer to the firmware node name
> + */
> +const char *fwnode_get_name(const struct fwnode_handle *fwnode)
> +{
> + return fwnode_call_ptr_op(fwnode, get_name);
> +}
> +EXPORT_SYMBOL(fwnode_get_name);
> +
> +/**
>   * fwnode_get_next_parent - Iterate to the node's parent
>   * @fwnode: Firmware whose parent is retrieved
>   *
> diff --git a/drivers/of/property.c b/drivers/of/property.c
> index 8ad33a4..6c195a8 100644
> --- a/drivers/of/property.c
> +++ b/drivers/of/property.c
> @@ -875,6 +875,11 @@ of_fwnode_property_read_string_array(const struct 
> fwnode_handle *fwnode,
>   of_property_count_strings(node, propname);
>  }
>  
> +static const char *of_fwnode_get_name(const struct fwnode_handle *fwnode)
> +{
> + return of_node_full_name(to_of_node(fwnode));
> +}
> +
>  static struct fwnode_handle *
>  of_fwnode_get_parent(const struct fwnode_handle *fwnode)
>  {
> @@ -988,6 +993,7 @@ const struct fwnode_operations of_fwnode_ops = {
>   .property_present = of_fwnode_property_present,
>   .property_read_int_array = of_fwnode_property_read_int_array,
>   .property_read_string_array = of_fwnode_property_read_string_array,
> + .get_name = of_fwnode_get_name,
>   .get_parent = of_fwnode_get_parent,
>   .get_next_child_node = of_fwnode_get_next_child_node,
>   .get_named_child_node = of_fwnode_get_named_child_node,
> diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
> index 411a84c..5d3a8c6 100644
> --- a/include/linux/fwnode.h
> +++ b/include/linux/fwnode.h
> @@ -57,6 +57,7 @@ struct fwnode_reference_args {
>   *otherwise.
>   * @property_read_string_array: Read an array of string properties. Return 
> zero
>   *   on success, a negative error code otherwise.
> + * @get_name: Return the fwnode name.
>   * @get_parent: Return the parent of an fwnode.
>   * @get_next_child_node: Return the next child node in an iteration.
>   * @get_named_child_node: Return a child node with a given name.
> @@ -81,6 +82,7 @@ struct fwnode_operations {
>   (*property_read_string_array)(const struct fwnode_handle *fwnode_handle,
> const char *propname, const char **val,
> size_t nval);
> + const char *(*get_name)(const struct fwnode_handle *fwnode);
>   struct fwnode_handle *(*get_parent)(const struct 

Re: [PATCH] v4l: rcar-csi2: Don't bail out from probe on no ep

2017-12-15 Thread Niklas Söderlund
Hi Hans,

On 2017-12-15 15:06:44 +0100, Hans Verkuil wrote:
> Niklas,
> 
> Did you look at this? If I should take it, can you Ack it? If you are
> going to squash or add it to our of your own patch series, then let me
> know and I can remove it from my todo queue.

I did look at it and talked to Jacobo about it. I think I have a better 
solution to his problem which I hope to try out in the near future. If 
my workaround turns out to not solve his problem I will squash this in 
when I resend the rcar-csi2 patch-set so in any case I think you can 
drop it from your todo queue.

The reason I'm a bit reluctant to ack this straight away is that I think 
it's kind of good that rcar-csi2 fails to probe if it finds no endpoints 
to work with, there is little use for the driver instance then. The 
problem Jacobo is trying to fix is related to how the rcar-vin Gen3 
group parses DT and I made a small mistake there which was discovered by 
Jacobo. And since the original fault is in the rcar-vin driver I think 
the issue should be fixed in that driver.

> 
> Regards,
> 
>   Hans
> 
> On 05/12/17 21:41, Jacopo Mondi wrote:
> > When rcar-csi interface is not connected to any endpoint, it fails and
> > bails out from probe before registering its own video subdevice.
> > This prevents rcar-vin registered notifier from completing and no
> > subdevice is ever registered, also for other properly connected csi
> > interfaces.
> > 
> > Fix this not returning an error when no endpoint is connected to a csi
> > interface and let the driver complete its probe function and register its
> > own video subdevice.
> > 
> > ---
> > Niklas,
> >please squash this patch in your next rcar-csi2 series (if you like it ;)
> > 
> > As we have discussed this is particularly useful for gmsl setup, where 
> > adv748x
> > is connected to CSI20 and max9286 to CSI40/CSI41. If we disable adv748x 
> > from DTS
> > we need CSI20 probe to complete anyhow otherwise no subdevice gets 
> > registered
> > for the two deserializers.
> > 
> > Please note we cannot disable CSI20 entirely otherwise VIN's graph parsing
> > breaks.
> > 
> > Thanks
> >j
> > 
> > ---
> >  drivers/media/platform/rcar-vin/rcar-csi2.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c 
> > b/drivers/media/platform/rcar-vin/rcar-csi2.c
> > index 2793efb..90c4062 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> > @@ -928,8 +928,8 @@ static int rcar_csi2_parse_dt(struct rcar_csi2 *priv)
> > 
> > ep = of_graph_get_endpoint_by_regs(priv->dev->of_node, 0, 0);
> > if (!ep) {
> > -   dev_err(priv->dev, "Not connected to subdevice\n");
> > -   return -EINVAL;
> > +   dev_dbg(priv->dev, "Not connected to subdevice\n");
> > +   return 0;
> > }
> > 
> > ret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(ep), _ep);
> > --
> > 2.7.4
> > 
> 

-- 
Regards,
Niklas Söderlund


Re: [PATCH 2/2] media: coda: Add i.MX51 (CodaHx4) support

2017-12-15 Thread Hans Verkuil
Hi Philipp,

On 13/12/17 15:09, Philipp Zabel wrote:
> Add support for the CodaHx4 VPU used on i.MX51.
> 
> Decoding h.264, MPEG-4, and MPEG-2 video works, as well as encoding
> h.264. MPEG-4 encoding is not enabled, it currently produces visual
> artifacts.
> 
> Signed-off-by: Philipp Zabel 
> ---
>  drivers/media/platform/coda/coda-bit.c| 45 
> ++-
>  drivers/media/platform/coda/coda-common.c | 44 +++---
>  drivers/media/platform/coda/coda.h|  1 +
>  3 files changed, 74 insertions(+), 16 deletions(-)
> 



> + [CODA_IMX51] = {
> + .firmware = {
> + "vpu_fw_imx51.bin",
> + "vpu/vpu_fw_imx51.bin",
> + "v4l-codahx4-imx51.bin"
> + },
> + .product  = CODA_HX4,
> + .codecs   = codahx4_codecs,
> + .num_codecs   = ARRAY_SIZE(codahx4_codecs),
> + .vdevs= codahx4_video_devices,
> + .num_vdevs= ARRAY_SIZE(codahx4_video_devices),
> + .workbuf_size = 128 * 1024,
> + .tempbuf_size = 304 * 1024,
> + .iram_size= 0x14000,
> + },

What's the status of the firmware? Is it going to be available in some firmware
repository? I remember when testing other imx devices that it was a bit tricky
to get hold of the firmware. And googling v4l-codahx4-imx51.bin doesn't find
anything other than this patch.

Regards,

Hans


Re: [PATCH 1/2] bdisp: Fix a possible sleep-in-atomic bug in bdisp_hw_reset

2017-12-15 Thread Hans Verkuil
Fabien or Benjamin, can you take a look at these two patches?

I'm a bit hesitant applying this since e.g. this bdisp_hw_reset() function 
might wait
for up to a second, which is a mite long for an interrupt :-)

Regards,

Hans

On 12/12/17 14:47, Jia-Ju Bai wrote:
> The driver may sleep under a spinlock.
> The function call path is:
> bdisp_device_run (acquire the spinlock)
>   bdisp_hw_reset
> msleep --> may sleep
> 
> To fix it, msleep is replaced with mdelay.
> 
> This bug is found by my static analysis tool(DSAC) and checked by my code 
> review.
> 
> Signed-off-by: Jia-Ju Bai 
> ---
>  drivers/media/platform/sti/bdisp/bdisp-hw.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/sti/bdisp/bdisp-hw.c 
> b/drivers/media/platform/sti/bdisp/bdisp-hw.c
> index b7892f3..4b62ceb 100644
> --- a/drivers/media/platform/sti/bdisp/bdisp-hw.c
> +++ b/drivers/media/platform/sti/bdisp/bdisp-hw.c
> @@ -382,7 +382,7 @@ int bdisp_hw_reset(struct bdisp_dev *bdisp)
>   for (i = 0; i < POLL_RST_MAX; i++) {
>   if (readl(bdisp->regs + BLT_STA1) & BLT_STA1_IDLE)
>   break;
> - msleep(POLL_RST_DELAY_MS);
> + mdelay(POLL_RST_DELAY_MS);
>   }
>   if (i == POLL_RST_MAX)
>   dev_err(bdisp->dev, "Reset timeout\n");
> 



Re: [PATCH/RFC v2 07/15] rcar-csi2: use frame description information to configure CSI-2 bus

2017-12-15 Thread jacopo mondi
Hi Niklas,
   thanks for the patch!

On Thu, Dec 14, 2017 at 08:08:27PM +0100, Niklas Söderlund wrote:
> The driver now have access to frame descriptor information, use it. Only
> enable the virtual channels which are described in the frame descriptor
> and calculate the link based on all enabled streams.
>
> With multiplexed stream support it's now possible to have different
> formats on the different source pads. Make source formats independent
> off each other and disallowing a format on the multiplexed sink.
>
> Signed-off-by: Niklas Söderlund 
> ---
>  drivers/media/platform/rcar-vin/rcar-csi2.c | 112 
> ++--
>  1 file changed, 58 insertions(+), 54 deletions(-)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c 
> b/drivers/media/platform/rcar-vin/rcar-csi2.c
> index 6b607b2e31e26063..2dd7d03d622d5510 100644
> --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> @@ -296,24 +296,22 @@ static const struct phtw_testdin_data 
> testdin_data_v3m_e3[] = {
>  #define CSI0CLKFREQRANGE(n)  ((n & 0x3f) << 16)
>
>  struct rcar_csi2_format {
> - unsigned int code;
>   unsigned int datatype;
>   unsigned int bpp;
>  };
>
>  static const struct rcar_csi2_format rcar_csi2_formats[] = {
> - { .code = MEDIA_BUS_FMT_RGB888_1X24,.datatype = 0x24, .bpp = 24 },
> - { .code = MEDIA_BUS_FMT_UYVY8_1X16, .datatype = 0x1e, .bpp = 16 },
> - { .code = MEDIA_BUS_FMT_UYVY8_2X8,  .datatype = 0x1e, .bpp = 16 },
> - { .code = MEDIA_BUS_FMT_YUYV10_2X10,.datatype = 0x1e, .bpp = 16 },
> + { .datatype = 0x1e, .bpp = 16 },
> + { .datatype = 0x24, .bpp = 24 },
>  };
>
> -static const struct rcar_csi2_format *rcar_csi2_code_to_fmt(unsigned int 
> code)
> +static const struct rcar_csi2_format
> +*rcar_csi2_datatype_to_fmt(unsigned int datatype)
>  {
>   unsigned int i;
>
>   for (i = 0; i < ARRAY_SIZE(rcar_csi2_formats); i++)
> - if (rcar_csi2_formats[i].code == code)
> + if (rcar_csi2_formats[i].datatype == datatype)
>   return rcar_csi2_formats + i;
>
>   return NULL;
> @@ -355,7 +353,7 @@ struct rcar_csi2 {
>   struct v4l2_async_notifier notifier;
>   struct v4l2_async_subdev remote;
>
> - struct v4l2_mbus_framefmt mf;
> + struct v4l2_mbus_framefmt mf[4];
>
>   struct mutex lock;
>   int stream_count[4];
> @@ -411,25 +409,14 @@ static int rcar_csi2_wait_phy_start(struct rcar_csi2 
> *priv)
>   return -ETIMEDOUT;
>  }
>
> -static int rcar_csi2_calc_mbps(struct rcar_csi2 *priv, unsigned int bpp)
> +static int rcar_csi2_calc_mbps(struct rcar_csi2 *priv,
> +struct v4l2_subdev *source,
> +struct v4l2_mbus_frame_desc *fd)
>  {
> - struct media_pad *pad, *source_pad;
> - struct v4l2_subdev *source = NULL;
>   struct v4l2_ctrl *ctrl;
> + unsigned int i, bpp = 0;
>   u64 mbps;
>
> - /* Get remote subdevice */
> - pad = >subdev.entity.pads[RCAR_CSI2_SINK];
> - source_pad = media_entity_remote_pad(pad);
> - if (!source_pad) {
> - dev_err(priv->dev, "Could not find remote source pad\n");
> - return -ENODEV;
> - }
> - source = media_entity_to_v4l2_subdev(source_pad->entity);
> -
> - dev_dbg(priv->dev, "Using source %s pad: %u\n", source->name,
> - source_pad->index);
> -
>   /* Read the pixel rate control from remote */
>   ctrl = v4l2_ctrl_find(source->ctrl_handler, V4L2_CID_PIXEL_RATE);
>   if (!ctrl) {
> @@ -438,6 +425,21 @@ static int rcar_csi2_calc_mbps(struct rcar_csi2 *priv, 
> unsigned int bpp)
>   return -EINVAL;
>   }
>
> + /* Calculate total bpp */
> + for (i = 0; i < fd->num_entries; i++) {
> + const struct rcar_csi2_format *format;
> +
> + format = rcar_csi2_datatype_to_fmt(
> + fd->entry[i].bus.csi2.data_type);
> + if (!format) {
> + dev_err(priv->dev, "Unknown data type: %d\n",
> + fd->entry[i].bus.csi2.data_type);
> + return -EINVAL;
> + }
> +
> + bpp += format->bpp;
> + }
> +
>   /* Calculate the phypll */
>   mbps = v4l2_ctrl_g_ctrl_int64(ctrl) * bpp;
>   do_div(mbps, priv->lanes * 100);
> @@ -489,39 +491,33 @@ static int rcar_csi2_set_phtw(struct rcar_csi2 *priv, 
> unsigned int mbps)
>   return 0;
>  }
>
> -static int rcar_csi2_start(struct rcar_csi2 *priv)
> +static int rcar_csi2_start(struct rcar_csi2 *priv, struct v4l2_subdev 
> *source,
> +struct v4l2_mbus_frame_desc *fd)

I'm not sure I got this right, but with the new s_stream pad
operation, and with rcar-csi2 endpoints connected to differenct VIN
instances, is it possible for "rcar_csi2_s_stream()" to be called on
the same 

Re: [PATCH] v4l: rcar-csi2: Don't bail out from probe on no ep

2017-12-15 Thread Hans Verkuil
Niklas,

Did you look at this? If I should take it, can you Ack it? If you are
going to squash or add it to our of your own patch series, then let me
know and I can remove it from my todo queue.

Regards,

Hans

On 05/12/17 21:41, Jacopo Mondi wrote:
> When rcar-csi interface is not connected to any endpoint, it fails and
> bails out from probe before registering its own video subdevice.
> This prevents rcar-vin registered notifier from completing and no
> subdevice is ever registered, also for other properly connected csi
> interfaces.
> 
> Fix this not returning an error when no endpoint is connected to a csi
> interface and let the driver complete its probe function and register its
> own video subdevice.
> 
> ---
> Niklas,
>please squash this patch in your next rcar-csi2 series (if you like it ;)
> 
> As we have discussed this is particularly useful for gmsl setup, where adv748x
> is connected to CSI20 and max9286 to CSI40/CSI41. If we disable adv748x from 
> DTS
> we need CSI20 probe to complete anyhow otherwise no subdevice gets registered
> for the two deserializers.
> 
> Please note we cannot disable CSI20 entirely otherwise VIN's graph parsing
> breaks.
> 
> Thanks
>j
> 
> ---
>  drivers/media/platform/rcar-vin/rcar-csi2.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c 
> b/drivers/media/platform/rcar-vin/rcar-csi2.c
> index 2793efb..90c4062 100644
> --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> @@ -928,8 +928,8 @@ static int rcar_csi2_parse_dt(struct rcar_csi2 *priv)
> 
>   ep = of_graph_get_endpoint_by_regs(priv->dev->of_node, 0, 0);
>   if (!ep) {
> - dev_err(priv->dev, "Not connected to subdevice\n");
> - return -EINVAL;
> + dev_dbg(priv->dev, "Not connected to subdevice\n");
> + return 0;
>   }
> 
>   ret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(ep), _ep);
> --
> 2.7.4
> 



Re: [PATCH v10 3/4] [media] platform: Add Synopsys DesignWare HDMI RX PHY e405 Driver

2017-12-15 Thread Hans Verkuil
On 11/12/17 18:41, Jose Abreu wrote:
> This adds support for the Synopsys DesignWare HDMI RX PHY e405. This
> phy receives and decodes HDMI video that is delivered to a controller.
> 
> Main features included in this driver are:
>   - Equalizer algorithm that chooses the phy best settings
>   according to the detected HDMI cable characteristics.
>   - Support for scrambling
>   - Support for HDMI 2.0 modes up to 6G (HDMI 4k@60Hz).
> 
> The driver was implemented as a standalone V4L2 subdevice and the
> phy interface with the controller was implemented using V4L2 ioctls. I
> do not know if this is the best option but it is not possible to use the
> existing API functions directly as we need specific functions that will
> be called by the controller at specific configuration stages.
> 
> There is also a bidirectional communication between controller and phy:
> The phy must provide functions that the controller will call (i.e.
> configuration functions) and the controller must provide read/write
> callbacks, as well as other specific functions.
> 
> Signed-off-by: Jose Abreu 
> Cc: Joao Pinto 
> Cc: Mauro Carvalho Chehab 
> Cc: Hans Verkuil 
> Cc: Sylwester Nawrocki 
> Cc: Philippe Ombredanne 

Reviewed-by: Hans Verkuil 

Regards,

Hans

> ---
> Changes from v9:
>   - Use SPDX License ID (Philippe)
> Changes from v4:
>   - Use usleep_range (Sylwester)
>   - Remove some comments (Sylwester)
>   - Parse phy version from of_device_id (Sylwester)
>   - Use "cfg" instead of "cfg-clk" (Sylwester, Rob)
>   - Change some messages to dev_dbg (Sylwester)
> Changes from v3:
>   - Use v4l2 async API (Sylwester)
>   - Use clock API (Sylwester)
>   - Add compatible string (Sylwester)
> Changes from RFC:
>   - Remove a bunch of functions that can be collapsed into
>   a single config() function
>   - Add comments for the callbacks and structures (Hans)
> ---
>  drivers/media/platform/Kconfig|   2 +
>  drivers/media/platform/Makefile   |   2 +
>  drivers/media/platform/dwc/Kconfig|   8 +
>  drivers/media/platform/dwc/Makefile   |   1 +
>  drivers/media/platform/dwc/dw-hdmi-phy-e405.c | 822 
> ++
>  drivers/media/platform/dwc/dw-hdmi-phy-e405.h |  41 ++
>  include/media/dwc/dw-hdmi-phy-pdata.h | 106 
>  7 files changed, 982 insertions(+)
>  create mode 100644 drivers/media/platform/dwc/Kconfig
>  create mode 100644 drivers/media/platform/dwc/Makefile
>  create mode 100644 drivers/media/platform/dwc/dw-hdmi-phy-e405.c
>  create mode 100644 drivers/media/platform/dwc/dw-hdmi-phy-e405.h
>  create mode 100644 include/media/dwc/dw-hdmi-phy-pdata.h
> 
> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> index fd0c998..0187cbd 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -33,6 +33,8 @@ source "drivers/media/platform/omap/Kconfig"
>  
>  source "drivers/media/platform/blackfin/Kconfig"
>  
> +source "drivers/media/platform/dwc/Kconfig"
> +
>  config VIDEO_SH_VOU
>   tristate "SuperH VOU video output driver"
>   depends on MEDIA_CAMERA_SUPPORT
> diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
> index 003b0bb..2e879c6 100644
> --- a/drivers/media/platform/Makefile
> +++ b/drivers/media/platform/Makefile
> @@ -97,3 +97,5 @@ obj-$(CONFIG_VIDEO_QCOM_CAMSS)  += 
> qcom/camss-8x16/
>  obj-$(CONFIG_VIDEO_QCOM_VENUS)   += qcom/venus/
>  
>  obj-y+= meson/
> +
> +obj-y+= dwc/
> diff --git a/drivers/media/platform/dwc/Kconfig 
> b/drivers/media/platform/dwc/Kconfig
> new file mode 100644
> index 000..361d38d
> --- /dev/null
> +++ b/drivers/media/platform/dwc/Kconfig
> @@ -0,0 +1,8 @@
> +config VIDEO_DWC_HDMI_PHY_E405
> + tristate "Synopsys Designware HDMI RX PHY e405 driver"
> + depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
> + help
> +   Support for Synopsys Designware HDMI RX PHY. Version is e405.
> +
> +   To compile this driver as a module, choose M here. The module
> +   will be called dw-hdmi-phy-e405.
> diff --git a/drivers/media/platform/dwc/Makefile 
> b/drivers/media/platform/dwc/Makefile
> new file mode 100644
> index 000..fc3b62c
> --- /dev/null
> +++ b/drivers/media/platform/dwc/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_VIDEO_DWC_HDMI_PHY_E405) += dw-hdmi-phy-e405.o
> diff --git a/drivers/media/platform/dwc/dw-hdmi-phy-e405.c 
> b/drivers/media/platform/dwc/dw-hdmi-phy-e405.c
> new file mode 100644
> index 000..e781cc1
> --- /dev/null
> +++ b/drivers/media/platform/dwc/dw-hdmi-phy-e405.c
> @@ -0,0 +1,822 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +// Copyright (c) 2017 

Re: [PATCH v10 2/4] MAINTAINERS: Add entry for Synopsys DesignWare HDMI drivers

2017-12-15 Thread Hans Verkuil
On 15/12/17 14:35, Hans Verkuil wrote:
> On 11/12/17 18:41, Jose Abreu wrote:
>> Add an entry for Synopsys DesignWare HDMI Receivers drivers
>> and phys.
>>
>> Signed-off-by: Jose Abreu 
>> Cc: Joao Pinto 
> 
> Reviewed-by: Hans Verkuil 

Oops, I was a bit too quick here. You're missing this file:

Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt

Regards,

Hans

> 
> Regards,
> 
>   Hans
> 
> 
> 
>> ---
>>  MAINTAINERS | 7 +++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 7a52a66..a1675bc 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -13108,6 +13108,13 @@ L:  net...@vger.kernel.org
>>  S:  Supported
>>  F:  drivers/net/ethernet/synopsys/
>>  
>> +SYNOPSYS DESIGNWARE HDMI RECEIVERS AND PHY DRIVERS
>> +M:  Jose Abreu 
>> +L:  linux-media@vger.kernel.org
>> +S:  Maintained
>> +F:  drivers/media/platform/dwc/*
>> +F:  include/media/dwc/*
>> +
>>  SYNOPSYS DESIGNWARE I2C DRIVER
>>  M:  Jarkko Nikula 
>>  R:  Andy Shevchenko 
>>
> 



Re: [PATCH v10 1/4] dt-bindings: media: Document Synopsys DesignWare HDMI RX

2017-12-15 Thread Hans Verkuil
On 11/12/17 18:41, Jose Abreu wrote:
> Document the bindings for the Synopsys DesignWare HDMI RX.
> 
> Signed-off-by: Jose Abreu 
> Acked-by: Rob Herring  (v8)
> Cc: Joao Pinto 
> Cc: Rob Herring 
> Cc: Mark Rutland 
> Cc: Mauro Carvalho Chehab 
> Cc: Hans Verkuil 
> Cc: Sylwester Nawrocki 
> Cc: devicet...@vger.kernel.org

Reviewed-by: Hans Verkuil 

Regards,

Hans

> ---
> Changes from v7:
>   - Remove SoC specific bindings (Rob)
> Changes from v6:
>   - Document which properties are required/optional (Sylwester)
>   - Drop compatible string for SoC (Sylwester)
>   - Reword edid-phandle property (Sylwester)
>   - Typo fixes (Sylwester)
> Changes from v4:
>   - Use "cfg" instead of "cfg-clk" (Rob)
>   - Change node names (Rob)
> Changes from v3:
>   - Document the new DT bindings suggested by Sylwester
> Changes from v2:
>   - Document edid-phandle property
> ---
>  .../devicetree/bindings/media/snps,dw-hdmi-rx.txt  | 58 
> ++
>  1 file changed, 58 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt 
> b/Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt
> new file mode 100644
> index 000..1dc09c6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt
> @@ -0,0 +1,58 @@
> +Synopsys DesignWare HDMI RX Decoder
> +===
> +
> +This document defines device tree properties for the Synopsys DesignWare HDMI
> +RX Decoder (DWC HDMI RX).
> +
> +The properties bellow belong to the Synopsys DesignWare HDMI RX Decoder node.
> +
> +Required properties:
> +
> +- compatible: Shall be "snps,dw-hdmi-rx".
> +- reg: Memory mapped base address and length of the DWC HDMI RX registers.
> +- interrupts: Reference to the DWC HDMI RX interrupt and the HDMI 5V sense
> +interrupt.
> +- clocks: Reference to the config clock.
> +- clock-names: Shall be "cfg".
> +- #address-cells: Shall be 1.
> +- #size-cells: Shall be 0.
> +
> +Optional properties:
> +
> +- edid-phandle: Reference to the EDID handler block; if this property is not
> +specified it is assumed that EDID is handled by device described by parent
> +node of the HDMI RX node. You should not specify this property if your HDMI 
> RX
> +controller does not have CEC.
> +
> +You also have to create a subnode for the PHY device. PHY node properties are
> +as follows.
> +
> +Required properties:
> +
> +- compatible: Shall be "snps,dw-hdmi-phy-e405".
> +- reg: Shall be the JTAG address of the PHY.
> +- clocks: Reference to the config clock.
> +- clock-names: Shall be "cfg".
> +
> +Example:
> +
> +hdmi_rx: hdmi-rx@0 {
> + compatible = "snps,dw-hdmi-rx";
> + reg = <0x0 0x1>;
> + interrupts = <1 2>;
> + edid-phandle = <_hdmi_edid>;
> +
> + clocks = <_hdmi_refclk>;
> + clock-names = "cfg";
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + hdmi-phy@fc {
> + compatible = "snps,dw-hdmi-phy-e405";
> + reg = <0xfc>;
> +
> + clocks = <_hdmi_refclk>;
> + clock-names = "cfg";
> + };
> +};
> 



Re: [PATCH v10 2/4] MAINTAINERS: Add entry for Synopsys DesignWare HDMI drivers

2017-12-15 Thread Hans Verkuil
On 11/12/17 18:41, Jose Abreu wrote:
> Add an entry for Synopsys DesignWare HDMI Receivers drivers
> and phys.
> 
> Signed-off-by: Jose Abreu 
> Cc: Joao Pinto 

Reviewed-by: Hans Verkuil 

Regards,

Hans



> ---
>  MAINTAINERS | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 7a52a66..a1675bc 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -13108,6 +13108,13 @@ L:   net...@vger.kernel.org
>  S:   Supported
>  F:   drivers/net/ethernet/synopsys/
>  
> +SYNOPSYS DESIGNWARE HDMI RECEIVERS AND PHY DRIVERS
> +M:   Jose Abreu 
> +L:   linux-media@vger.kernel.org
> +S:   Maintained
> +F:   drivers/media/platform/dwc/*
> +F:   include/media/dwc/*
> +
>  SYNOPSYS DESIGNWARE I2C DRIVER
>  M:   Jarkko Nikula 
>  R:   Andy Shevchenko 
> 



RE: [PATCH v9 0/2] media: ov7740: Add a V4L2 sensor-level driver

2017-12-15 Thread Wenyou.Yang
Hi Sakari,

Do you have some comments on this version?


Best Regards,
Wenyou Yang

> -Original Message-
> From: linux-media-ow...@vger.kernel.org [mailto:linux-media-
> ow...@vger.kernel.org] On Behalf Of Wenyou Yang
> Sent: 2017年12月11日 9:32
> To: Mauro Carvalho Chehab ; Rob Herring
> ; Mark Rutland 
> Cc: linux-ker...@vger.kernel.org; Nicolas Ferre - M43238
> ; devicet...@vger.kernel.org; Sakari Ailus
> ; Jonathan Corbet ; Hans Verkuil
> ; linux-arm-ker...@lists.infradead.org; Linux Media 
> Mailing List
> ; Wenyou Yang - A41535
> 
> Subject: [PATCH v9 0/2] media: ov7740: Add a V4L2 sensor-level driver
> 
> Add a Video4Linux2 sensor-level driver for the OmniVision OV7740 VGA camera
> image sensor.
> 
> Changes in v9:
>  - Use the new SPDX ids.
> 
> Changes in v8:
>  - As the registers are written at stream start, remove the written
>code from the set fmt function.
> 
> Changes in v7:
>  - Add Acked-by tag.
>  - Fix the wrong handle of default register configuration.
>  - Add the missed assignment of ov7740->frmsize.
> 
> Changes in v6:
>  - Remove unnecessary #include .
>  - Remove unnecessary comments and extra newline.
>  - Add const for some structures.
>  - Add the check of the return value from regmap_write().
>  - Simplify the calling of __v4l2_ctrl_handler_setup().
>  - Add the default format initialization function.
>  - Integrate the set_power() and enable/disable the clock into
>one function.
> 
> Changes in v5:
>  - Squash the driver and MAINTAINERS entry patches to one.
>  - Precede the driver patch with the bindings patch.
> 
> Changes in v4:
>  - Assign 'val' a initial value to avoid warning: 'val' may be
>used uninitialized.
>  - Rename REG_REG15 to avoid warning: "REG_REG15" redefined.
> 
> Changes in v3:
>  - Explicitly document the "remote-endpoint" property.
>  - Put the MAINTAINERS change to a separate patch.
> 
> Changes in v2:
>  - Split off the bindings into a separate patch.
>  - Add a new entry to the MAINTAINERS file.
> 
> Wenyou Yang (2):
>   media: ov7740: Document device tree bindings
>   media: i2c: Add the ov7740 image sensor driver
> 
>  .../devicetree/bindings/media/i2c/ov7740.txt   |   47 +
>  MAINTAINERS|8 +
>  drivers/media/i2c/Kconfig  |8 +
>  drivers/media/i2c/Makefile |1 +
>  drivers/media/i2c/ov7740.c | 1216 
> 
>  5 files changed, 1280 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov7740.txt
>  create mode 100644 drivers/media/i2c/ov7740.c
> 
> --
> 2.15.0



Re: [PATCH/RFC v2 06/15] rcar-csi2: use frame description information when propagating .s_stream()

2017-12-15 Thread Sakari Ailus
Hi Niklas,

On Thu, Dec 14, 2017 at 08:08:26PM +0100, Niklas Söderlund wrote:
> Use the frame description from the remote subdevice of the rcar-csi2's
> sink pad to get the remote pad and stream pad needed to propagate the
> .s_stream() operation.
> 
> The CSI-2 virtual channel which should be acted upon can be determined
> by looking at which of the rcar-csi2 source pad the .s_stream() was
> called on. This is because the rcar-csi2 acts as a demultiplexer for the
> CSI-2 link on the one sink pad and outputs each virtual channel on a
> distinct and known source pad.
> 
> Signed-off-by: Niklas Söderlund 
> ---
>  drivers/media/platform/rcar-vin/rcar-csi2.c | 58 
> -
>  1 file changed, 41 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c 
> b/drivers/media/platform/rcar-vin/rcar-csi2.c
> index e0f56cc3d25179a9..6b607b2e31e26063 100644
> --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> @@ -614,20 +614,31 @@ static void rcar_csi2_stop(struct rcar_csi2 *priv)
>   rcar_csi2_reset(priv);
>  }
>  
> -static int rcar_csi2_sd_info(struct rcar_csi2 *priv, struct v4l2_subdev **sd)
> +static int rcar_csi2_get_source_info(struct rcar_csi2 *priv,
> +  struct v4l2_subdev **subdev,

I wonder if using struct media_pad for this would be cleaner.

> +  unsigned int *pad,
> +  struct v4l2_mbus_frame_desc *fd)
>  {
> - struct media_pad *pad;
> + struct media_pad *remote_pad;
>  
> - pad = media_entity_remote_pad(>pads[RCAR_CSI2_SINK]);
> - if (!pad) {
> - dev_err(priv->dev, "Could not find remote pad\n");
> + /* Get source subdevice and pad */
> + remote_pad = media_entity_remote_pad(>pads[RCAR_CSI2_SINK]);
> + if (!remote_pad) {
> + dev_err(priv->dev, "Could not find remote source pad\n");
>   return -ENODEV;
>   }
> + *subdev = media_entity_to_v4l2_subdev(remote_pad->entity);
> + *pad = remote_pad->index;
>  
> - *sd = media_entity_to_v4l2_subdev(pad->entity);
> - if (!*sd) {
> - dev_err(priv->dev, "Could not find remote subdevice\n");
> - return -ENODEV;
> + /* Get frame descriptor */
> + if (v4l2_subdev_call(*subdev, pad, get_frame_desc, *pad, fd)) {
> + dev_err(priv->dev, "Could not read frame desc\n");
> + return -EINVAL;
> + }
> +
> + if (fd->type != V4L2_MBUS_FRAME_DESC_TYPE_CSI2) {
> + dev_err(priv->dev, "Frame desc do not describe CSI-2 link");
> + return -EINVAL;

I think this should also work with drivers that do not support frame
descriptors.

Alternatively support could be added for all drivers. In practice this
would mean having a few bus specific implementations of get_frame_desc op
that would dig the information from the frame format.

Perhaps the former option would make sense here, for now.

>   }
>  
>   return 0;
> @@ -637,9 +648,10 @@ static int rcar_csi2_s_stream(struct v4l2_subdev *sd, 
> unsigned int pad,
> unsigned int stream, int enable)
>  {
>   struct rcar_csi2 *priv = sd_to_csi2(sd);
> + struct v4l2_mbus_frame_desc fd;
>   struct v4l2_subdev *nextsd;
> - unsigned int i, count = 0;
> - int ret, vc;
> + unsigned int i, rpad, count = 0;
> + int ret, vc, rstream = -1;
>  
>   /* Only allow stream control on source pads and valid vc */
>   vc = rcar_csi2_pad_to_vc(pad);
> @@ -650,11 +662,23 @@ static int rcar_csi2_s_stream(struct v4l2_subdev *sd, 
> unsigned int pad,
>   if (stream != 0)
>   return -EINVAL;
>  
> - mutex_lock(>lock);
> -
> - ret = rcar_csi2_sd_info(priv, );
> + /* Get information about multiplexed link */
> + ret = rcar_csi2_get_source_info(priv, , , );
>   if (ret)
> - goto out;
> + return ret;
> +
> + /* Get stream on multiplexed link */
> + for (i = 0; i < fd.num_entries; i++)
> + if (fd.entry[i].bus.csi2.channel == vc)
> + rstream = fd.entry[i].stream;

Virtual channel does not equate to the stream. You'll need the data type,
too.

You should actually obtain this from the configured routes instead.

How does this work btw. if you have several streams on the same virtual
channel that only have different data types?

> +
> + if (rstream < 0) {
> + dev_err(priv->dev, "Could not find stream for vc %u\n", vc);
> + return -EINVAL;
> + }
> +
> + /* Start or stop the requested stream */
> + mutex_lock(>lock);
>  
>   for (i = 0; i < 4; i++)
>   count += priv->stream_count[i];
> @@ -673,14 +697,14 @@ static int rcar_csi2_s_stream(struct v4l2_subdev *sd, 
> unsigned int pad,
>   }
>  
>   if (enable && priv->stream_count[vc] == 0) 

more build failures

2017-12-15 Thread Vincent McIntyre
Hi,

Don't get me wrong, I appreciate the vast amount of work going on here.
Just letting you know what I'm seeing.

+ date
Friday 15 December  23:28:52 AEDT 2017
+ uname -a
Linux ubuntu 4.4.0-103-generic #126-Ubuntu SMP Mon Dec 4 16:23:28 UTC 2017 
x86_64 x86_64 x86_64 GNU/Linux
+ cat /proc/version_signature
Ubuntu 4.4.0-103.126-generic 4.4.98

...

+ git --no-pager log -1
commit 4058fea6b2507661d66af5298c048d7c55338f42
Author: Jasmin Jessich 
Date:   Tue Dec 12 12:49:01 2017 -0500

fixup v3.13_ddbridge_pcimsi.patch

Required after the ddbridge 0.9.32 bump in media_tree.

Signed-off-by: Jasmin Jessich 
Signed-off-by: Daniel Scheller 

...

+ git --no-pager log -1
commit 0393e735649dc41358adb7b603bd57dad1ed3260
Author: Laurent Pinchart 
Date:   Tue Oct 17 13:15:54 2017 -0400

media: uvcvideo: Stream error events carry no data

According to the UVC specification, stream error events carry no data.
Fix a buffer overflow (that should be harmless given data alignment)
when reporting the stream error event by removing the data byte from the
message.

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

...

$ make allyesconfig
make -C /home/me/git/clones/media_build/v4l allyesconfig
make[1]: Entering directory '/home/me/git/clones/media_build/v4l'
No version yet, using 4.4.0-103-generic
make[2]: Entering directory '/home/me/git/clones/media_build/linux'
Syncing with dir ../media/
Applying patches for kernel 4.4.0-103-generic
patch -s -f -N -p1 -i ../backports/api_version.patch
patch -s -f -N -p1 -i ../backports/pr_fmt.patch
patch -s -f -N -p1 -i ../backports/debug.patch
patch -s -f -N -p1 -i ../backports/drx39xxj.patch
patch -s -f -N -p1 -i ../backports/v4.14_compiler_h.patch
patch -s -f -N -p1 -i ../backports/v4.14_saa7146_timer_cast.patch
patch -s -f -N -p1 -i ../backports/v4.14_module_param_call.patch
patch -s -f -N -p1 -i ../backports/v4.12_revert_solo6x10_copykerneluser.patch
patch -s -f -N -p1 -i ../backports/v4.10_sched_signal.patch
1 out of 1 hunk FAILED
The text leading up to this was:
--
|diff --git a/drivers/staging/media/lirc/lirc_zilog.c 
b/drivers/staging/media/lirc/lirc_zilog.c
|index 015e41bd036e..fd61081b47d9 100644
|--- a/drivers/staging/media/lirc/lirc_zilog.c
|+++ b/drivers/staging/media/lirc/lirc_zilog.c
--
No file to patch.  Skipping patch.
1 out of 1 hunk ignored
Makefile:130: recipe for target 'apply_patches' failed
make[2]: *** [apply_patches] Error 1
make[2]: Leaving directory '/home/me/git/clones/media_build/linux'
Makefile:374: recipe for target 'allyesconfig' failed
make[1]: *** [allyesconfig] Error 2
make[1]: Leaving directory '/home/me/git/clones/media_build/v4l'
Makefile:26: recipe for target 'allyesconfig' failed
make: *** [allyesconfig] Error 2
can't select all drivers at ./build line 525
+ status=29
+ date
Friday 15 December  23:29:17 AEDT 2017
+ [ 0 = 29 ]




Re: [GIT PULL FOR v4.16] staging/media: add NVIDIA Tegra video decoder driver

2017-12-15 Thread Dmitry Osipenko
On 14.12.2017 14:06, Mauro Carvalho Chehab wrote:
> Em Tue, 12 Dec 2017 16:28:40 +0100
> Hans Verkuil  escreveu:
> 
>> This adds a new NVIDIA Tegra video decoder driver. It is depending on the
>> request API work since it is a stateless codec, so for now park this in 
>> staging.
>>
>> The dts patches should go through nvidia's tree.
>>
>> Regards,
>>
>>  Hans
>>
>> The following changes since commit 330dada5957e3ca0c8811b14c45e3ac42c694651:
>>
>>   media: dvb_frontend: fix return error code (2017-12-12 07:50:14 -0500)
>>
>> are available in the Git repository at:
>>
>>   git://linuxtv.org/hverkuil/media_tree.git tegradec
>>
>> for you to fetch changes up to c3c530f45e48b33a2cc49cdeec246d255a5ca7db:
>>
>>   staging: media: Introduce NVIDIA Tegra video decoder driver (2017-12-12 
>> 16:06:06 +0100)
>>
>> 
>> Dmitry Osipenko (2):
>>   media: dt: bindings: Add binding for NVIDIA Tegra Video Decoder Engine
>>   staging: media: Introduce NVIDIA Tegra video decoder driver
> 
> Ok, clearly, there are some things that are not OK on the driver,
> otherwise, it won't be merging at staging. Yet, there warnings
> there that should be considered before moving it out of staging:

Sure, I'm aware of the checkpatch warnings and some of them aren't legit, others
aren't very important and would be corrected later. The main reason of going
into staging should be the lack of V4L2 interface support in the driver
(necessary V4L API isn't there yet), see TODO. Certainly there are other things
to be done besides the V4L interface before de-staging, going into staging is a
very good variant right now, thanks for allowing to do it!

[snip]


Re: [PATCH/RFC v2 05/15] rcar-csi2: count usage for each source pad

2017-12-15 Thread Sakari Ailus
On Thu, Dec 14, 2017 at 08:08:25PM +0100, Niklas Söderlund wrote:
> The R-Car CSI-2 hardware can output the same virtual channel
> simultaneously to more then one R-Car VIN. For this reason we need to
> move the usage counting from the global device to each source pad.
> 
> If a source pads usage count go from 0 to 1 we need to signal that a new
> stream should start, likewise if it go from 1 to 0 we need to stop a
> stream. At the same time we only want to start or stop the R-Car
> CSI-2 device only on the first or last stream.
> 
> Signed-off-by: Niklas Söderlund 
> ---
>  drivers/media/platform/rcar-vin/rcar-csi2.c | 38 
> +++--
>  1 file changed, 30 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c 
> b/drivers/media/platform/rcar-vin/rcar-csi2.c
> index 8ce0bfeef1113f9c..e0f56cc3d25179a9 100644
> --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> @@ -328,6 +328,14 @@ enum rcar_csi2_pads {
>   NR_OF_RCAR_CSI2_PAD,
>  };
>  
> +static int rcar_csi2_pad_to_vc(unsigned int pad)
> +{
> + if (pad < RCAR_CSI2_SOURCE_VC0 || pad > RCAR_CSI2_SOURCE_VC3)
> + return -EINVAL;
> +
> + return pad - RCAR_CSI2_SOURCE_VC0;
> +}
> +
>  struct rcar_csi2_info {
>   const struct phypll_hsfreqrange *hsfreqrange;
>   const struct phtw_testdin_data *testdin_data;
> @@ -350,7 +358,7 @@ struct rcar_csi2 {
>   struct v4l2_mbus_framefmt mf;
>  
>   struct mutex lock;
> - int stream_count;
> + int stream_count[4];

Why 4?

>  
>   unsigned short lanes;
>   unsigned char lane_swap[4];
> @@ -630,7 +638,13 @@ static int rcar_csi2_s_stream(struct v4l2_subdev *sd, 
> unsigned int pad,
>  {
>   struct rcar_csi2 *priv = sd_to_csi2(sd);
>   struct v4l2_subdev *nextsd;
> - int ret;
> + unsigned int i, count = 0;
> + int ret, vc;
> +
> + /* Only allow stream control on source pads and valid vc */
> + vc = rcar_csi2_pad_to_vc(pad);
> + if (vc < 0)
> + return vc;
>  
>   /* Only one stream on each source pad */
>   if (stream != 0)
> @@ -642,7 +656,10 @@ static int rcar_csi2_s_stream(struct v4l2_subdev *sd, 
> unsigned int pad,
>   if (ret)
>   goto out;
>  
> - if (enable && priv->stream_count == 0) {
> + for (i = 0; i < 4; i++)
> + count += priv->stream_count[i];
> +
> + if (enable && count == 0) {
>   pm_runtime_get_sync(priv->dev);
>  
>   ret = rcar_csi2_start(priv);
> @@ -650,20 +667,23 @@ static int rcar_csi2_s_stream(struct v4l2_subdev *sd, 
> unsigned int pad,
>   pm_runtime_put(priv->dev);
>   goto out;
>   }
> + } else if (!enable && count == 1) {
> + rcar_csi2_stop(priv);
> + pm_runtime_put(priv->dev);
> + }
>  
> + if (enable && priv->stream_count[vc] == 0) {
>   ret = v4l2_subdev_call(nextsd, video, s_stream, 1);
>   if (ret) {
>   rcar_csi2_stop(priv);
>   pm_runtime_put(priv->dev);
>   goto out;
>   }
> - } else if (!enable && priv->stream_count == 1) {
> - rcar_csi2_stop(priv);
> + } else if (!enable && priv->stream_count[vc] == 1) {

Do I understand correctly that you can start and streams here one by one,
independently of each other?

Sometimes this might not be the case. I wonder if we should have a way to
tell that to the caller.

>   ret = v4l2_subdev_call(nextsd, video, s_stream, 0);
> - pm_runtime_put(priv->dev);
>   }
>  
> - priv->stream_count += enable ? 1 : -1;
> + priv->stream_count[vc] += enable ? 1 : -1;
>  out:
>   mutex_unlock(>lock);
>  
> @@ -919,7 +939,9 @@ static int rcar_csi2_probe(struct platform_device *pdev)
>   priv->dev = >dev;
>  
>   mutex_init(>lock);
> - priv->stream_count = 0;
> +
> + for (i = 0; i < 4; i++)
> + priv->stream_count[i] = 0;
>  
>   ret = rcar_csi2_probe_resources(priv, pdev);
>   if (ret) {
> -- 
> 2.15.1
> 

-- 
Sakari Ailus
sakari.ai...@linux.intel.com


Re: [PATCH/RFC v2 03/15] rcar-vin: use the pad and stream aware s_stream

2017-12-15 Thread Sakari Ailus
Hej,

On Thu, Dec 14, 2017 at 08:08:23PM +0100, Niklas Söderlund wrote:
> To work with multiplexed streams the pad and stream aware s_stream
> operation needs to be used.
> 
> Signed-off-by: Niklas Söderlund 
> ---
>  drivers/media/platform/rcar-vin/rcar-dma.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c 
> b/drivers/media/platform/rcar-vin/rcar-dma.c
> index cf30e5fceb1d493a..8435491535060eae 100644
> --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> @@ -1180,7 +1180,7 @@ static int rvin_set_stream(struct rvin_dev *vin, int on)
>  
>   if (!on) {
>   media_pipeline_stop(vin->vdev.entity.pads);
> - return v4l2_subdev_call(sd, video, s_stream, 0);
> + return v4l2_subdev_call(sd, pad, s_stream, pad->index, 0, 0);

Have you thought of adding a wrapper for the s_stream callback?

I think you should either change all s_stream callbacks from video to pad,
or add a wrapper which then calls the video op instead of the pad op if the
pad op does not exist. Otherwise we again have two non-interoperable
classes of drivers for no good reason.

Thinking about it, I'm not all that certain changing all instances would be
that much work in the end; it should be done anyway. Devices that have a
single stream (i.e. everything right now) just don't care about the pad
number.

>   }
>  
>   fmt.pad = pad->index;
> @@ -1239,12 +1239,14 @@ static int rvin_set_stream(struct rvin_dev *vin, int 
> on)
>   if (media_pipeline_start(vin->vdev.entity.pads, pipe))
>   return -EPIPE;
>  
> - ret = v4l2_subdev_call(sd, video, s_stream, 1);
> + ret = v4l2_subdev_call(sd, pad, s_stream, pad->index, 0, 1);
>   if (ret == -ENOIOCTLCMD)
>   ret = 0;
>   if (ret)
>   media_pipeline_stop(vin->vdev.entity.pads);
>  
> + vin_dbg(vin, "pad: %u stream: 0 enable: %d\n", pad->index, on);
> +
>   return ret;
>  }
>  
> -- 
> 2.15.1
> 

-- 
Sakari Ailus
sakari.ai...@linux.intel.com


Re: [PATCH/RFC v2 02/15] rcar-vin: use pad as the starting point for a pipeline

2017-12-15 Thread Sakari Ailus
On Thu, Dec 14, 2017 at 08:08:22PM +0100, Niklas Söderlund wrote:
> The pipeline will be moved from the entity to the pads; reflect this in
> the media pipeline function API.

I'll merge this to "media: entity: Use pad as the starting point for a
pipeline" if you're fine with that.

I haven't compiled everything for some time, and newly added drivers may be
lacking these changes. I'll re-check that soonish.

> 
> Signed-off-by: Niklas Söderlund 
> ---
>  drivers/media/platform/rcar-vin/rcar-dma.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c 
> b/drivers/media/platform/rcar-vin/rcar-dma.c
> index 03a914361a33125c..cf30e5fceb1d493a 100644
> --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> @@ -1179,7 +1179,7 @@ static int rvin_set_stream(struct rvin_dev *vin, int on)
>   return -EPIPE;
>  
>   if (!on) {
> - media_pipeline_stop(>vdev.entity);
> + media_pipeline_stop(vin->vdev.entity.pads);
>   return v4l2_subdev_call(sd, video, s_stream, 0);
>   }
>  
> @@ -1235,15 +1235,15 @@ static int rvin_set_stream(struct rvin_dev *vin, int 
> on)
>   fmt.format.height != vin->format.height)
>   return -EPIPE;
>  
> - pipe = sd->entity.pipe ? sd->entity.pipe : >vdev.pipe;
> - if (media_pipeline_start(>vdev.entity, pipe))
> + pipe = sd->entity.pads->pipe ? sd->entity.pads->pipe : >vdev.pipe;
> + if (media_pipeline_start(vin->vdev.entity.pads, pipe))
>   return -EPIPE;
>  
>   ret = v4l2_subdev_call(sd, video, s_stream, 1);
>   if (ret == -ENOIOCTLCMD)
>   ret = 0;
>   if (ret)
> - media_pipeline_stop(>vdev.entity);
> + media_pipeline_stop(vin->vdev.entity.pads);
>  
>   return ret;
>  }
> -- 
> 2.15.1
> 

-- 
Sakari Ailus
sakari.ai...@linux.intel.com


Re: [PATCH/RFC v2 01/15] v4l2-subdev.h: add pad and stream aware s_stream

2017-12-15 Thread Sakari Ailus
Hejssan Niklas,

Tack för uppdaterade lappor!

On Thu, Dec 14, 2017 at 08:08:21PM +0100, Niklas Söderlund wrote:
> To be able to start and stop individual streams of a multiplexed pad the
> s_stream operation needs to be both pad and stream aware. Add a new
> operation to pad ops to facilitate this.
> 
> Signed-off-by: Niklas Söderlund 
> ---
>  include/media/v4l2-subdev.h | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index a30a94fad8dbacde..7288209338a48fda 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -669,6 +669,9 @@ struct v4l2_subdev_pad_config {
>   *
>   * @set_frame_desc: set the low level media bus frame parameters, @fd array
>   *  may be adjusted by the subdev driver to device 
> capabilities.
> + *
> + * @s_stream: used to notify the driver that a stream will start or has
> + *   stopped.

This is actually the callback which is used to control the stream state.
The above suggests that it's a notification of something that has happened
(or about to happen). How about:

Enable or disable streaming on a sub-device pad.

>   */
>  struct v4l2_subdev_pad_ops {
>   int (*init_cfg)(struct v4l2_subdev *sd,
> @@ -713,6 +716,8 @@ struct v4l2_subdev_pad_ops {
>  struct v4l2_subdev_routing *route);
>   int (*set_routing)(struct v4l2_subdev *sd,
>  struct v4l2_subdev_routing *route);
> + int (*s_stream)(struct v4l2_subdev *sd, unsigned int pad,
> + unsigned int stream, int enable);

How about bool for enable?

>  };
>  
>  /**

-- 
Sakari Ailus
sakari.ai...@linux.intel.com


Re: [PATCH v10 4/4] [media] platform: Add Synopsys DesignWare HDMI RX Controller Driver

2017-12-15 Thread Jose Abreu
Hi Hans,

On 13-12-2017 20:49, Hans Verkuil wrote:
> On 13/12/17 15:00, Jose Abreu wrote:
>>
>> Indeed. I compared the values with the spec and they are not
>> correct. Even hsync is wrong. I already corrected in the code the
>> hsync but regarding interlace I'm not seeing an easy way to do
>> this without using interrupts in each vsync because the register
>> I was toggling does not behave as I expected (I misunderstood the
>> databook). Maybe we should not detect interlaced modes for now?
>> Or not fill the il_ fields?
> As I mentioned above you as long as you get a good backporch value you
> can deduce from whether it is an odd or even number to which field it
> belongs and fill in the other values. So I think you only need to read
> these values for one field.
>
> Filling in good values here (at least as far as is possible since not all
> hardware can give it) will help debugging issues, even if you otherwise do
> not support interlaced.

Ok, I will fill the fields.

Until the end of January I will be quite busy in another project
so if you could review the remaining patches of this series I
would appreciate very much ... This way when I have the time I
can code all the changes and send them at once.

Thanks and Best Regards,
Jose Miguel Abreu

>
> Regards,
>
>   Hans



Re: [PATCH] v4l2-dv-timings: add v4l2_hdmi_colorimetry()

2017-12-15 Thread Jose Abreu
Hi Hans,

On 15-12-2017 09:48, Hans Verkuil wrote:
> Add the v4l2_hdmi_colorimetry() function so we have a single function
> that determines the colorspace, YCbCr encoding, quantization range and
> transfer function from the InfoFrame data.

You could also make AVI infoframe optional and return RGB in that
case.

Anyway, I took a quick look at the spec and everything seems ok.

Reviewed-by: Jose Abreu 

Best Regards,
Jose Miguel Abreu

>
> Signed-off-by: Hans Verkuil 
> ---
> Tim, you can add this patch to your driver patch series and just call it.
> Note that this colorspace information is what the HDMI gives you, if you
> need to convert this than you will need to update this information accordingly
> (e.g. lim to full range, 601 to 709 conversions, etc.) before giving it to
> userspace.
>
> Jose, I'm not sure if you need it in your driver, but this is probably useful
> regardless.
>
> Regards,
>
>   Hans
> ---
>  drivers/media/v4l2-core/v4l2-dv-timings.c | 141 
> ++
>  include/media/v4l2-dv-timings.h   |  21 +
>  2 files changed, 162 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-dv-timings.c 
> b/drivers/media/v4l2-core/v4l2-dv-timings.c
> index 930f9c53a64e..0182d3d3f807 100644
> --- a/drivers/media/v4l2-core/v4l2-dv-timings.c
> +++ b/drivers/media/v4l2-core/v4l2-dv-timings.c
> @@ -27,6 +27,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  MODULE_AUTHOR("Hans Verkuil");
>  MODULE_DESCRIPTION("V4L2 DV Timings Helper Functions");
> @@ -814,3 +815,143 @@ struct v4l2_fract v4l2_calc_aspect_ratio(u8 
> hor_landscape, u8 vert_portrait)
>   return aspect;
>  }
>  EXPORT_SYMBOL_GPL(v4l2_calc_aspect_ratio);
> +
> +/** v4l2_hdmi_rx_colorimetry - determine HDMI colorimetry information
> + *   based on various InfoFrames.
> + * @avi - the AVI InfoFrame
> + * @hdmi - the HDMI Vendor InfoFrame, may be NULL
> + * @height - the frame height
> + *
> + * Determines the HDMI colorimetry information, i.e. how the HDMI
> + * pixel color data should be interpreted.
> + *
> + * Note that some of the newer features (DCI-P3, HDR) are not yet
> + * implemented: the hdmi.h header needs to be updated to the HDMI 2.0
> + * and CTA-861-G standards.
> + */
> +struct v4l2_hdmi_colorimetry
> +v4l2_hdmi_rx_colorimetry(const struct hdmi_avi_infoframe *avi,
> +  const struct hdmi_vendor_infoframe *hdmi,
> +  unsigned int height)
> +{
> + struct v4l2_hdmi_colorimetry c = {
> + V4L2_COLORSPACE_SRGB,
> + V4L2_YCBCR_ENC_DEFAULT,
> + V4L2_QUANTIZATION_FULL_RANGE,
> + V4L2_XFER_FUNC_SRGB
> + };
> + bool is_ce = avi->video_code || (hdmi && hdmi->vic);
> + bool is_sdtv = height <= 576;
> + bool default_is_lim_range_rgb = avi->video_code > 1;
> +
> + switch (avi->colorspace) {
> + case HDMI_COLORSPACE_RGB:
> + /* RGB pixel encoding */
> + switch (avi->colorimetry) {
> + case HDMI_COLORIMETRY_EXTENDED:
> + switch (avi->extended_colorimetry) {
> + case HDMI_EXTENDED_COLORIMETRY_ADOBE_RGB:
> + c.colorspace = V4L2_COLORSPACE_ADOBERGB;
> + c.xfer_func = V4L2_XFER_FUNC_ADOBERGB;
> + break;
> + case HDMI_EXTENDED_COLORIMETRY_BT2020:
> + c.colorspace = V4L2_COLORSPACE_BT2020;
> + c.xfer_func = V4L2_XFER_FUNC_709;
> + break;
> + default:
> + break;
> + }
> + break;
> + default:
> + break;
> + }
> + switch (avi->quantization_range) {
> + case HDMI_QUANTIZATION_RANGE_LIMITED:
> + c.quantization = V4L2_QUANTIZATION_LIM_RANGE;
> + break;
> + case HDMI_QUANTIZATION_RANGE_FULL:
> + break;
> + default:
> + if (default_is_lim_range_rgb)
> + c.quantization = V4L2_QUANTIZATION_LIM_RANGE;
> + break;
> + }
> + break;
> +
> + default:
> + /* YCbCr pixel encoding */
> + c.quantization = V4L2_QUANTIZATION_LIM_RANGE;
> + switch (avi->colorimetry) {
> + case HDMI_COLORIMETRY_NONE:
> + if (!is_ce)
> + break;
> + if (is_sdtv) {
> + c.colorspace = V4L2_COLORSPACE_SMPTE170M;
> + c.ycbcr_enc = V4L2_YCBCR_ENC_601;
> + } else {
> + c.colorspace = V4L2_COLORSPACE_REC709;
> + c.ycbcr_enc = V4L2_YCBCR_ENC_709;
> + 

Re: [PATCH v2 1/3] media: V3s: Add support for Allwinner CSI.

2017-12-15 Thread Yong
Hi Maxime,

On Fri, 15 Dec 2017 11:50:47 +0100
Maxime Ripard  wrote:

> Hi Yong,
> 
> On Mon, Dec 04, 2017 at 05:45:11PM +0800, Yong wrote:
> > I just noticed that you are using the second iteration?
> > Have you received my third iteration?
> 
> Sorry for the late reply, and for not coming back to you yet about
> that test. No, this is still in your v2. I've definitely received your
> v3, I just didn't have time to update to it yet.
> 
> But don't worry, my mail was mostly to know if you had tested that
> setup on your side to try to nail down the issue on my end, not really
> a review or comment that would prevent your patch from going in.

I mean,
The v2 exactly has a bug which may cause the CSI writing frame to 
a wrong memory address.

BTW, should I send a new version. I have made some improve sine v3.

> 
> Maxime
> 
> -- 
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com


Thanks,
Yong


Re: [PATCH v2 1/3] media: V3s: Add support for Allwinner CSI.

2017-12-15 Thread Maxime Ripard
Hi Yong,

On Mon, Dec 04, 2017 at 05:45:11PM +0800, Yong wrote:
> I just noticed that you are using the second iteration?
> Have you received my third iteration?

Sorry for the late reply, and for not coming back to you yet about
that test. No, this is still in your v2. I've definitely received your
v3, I just didn't have time to update to it yet.

But don't worry, my mail was mostly to know if you had tested that
setup on your side to try to nail down the issue on my end, not really
a review or comment that would prevent your patch from going in.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature


Re: [RFC PATCH 1/9] media: add request API core and UAPI

2017-12-15 Thread Philippe Ombredanne
Alexandre:

On Fri, Dec 15, 2017 at 8:56 AM, Alexandre Courbot
 wrote:
> The request API provides a way to group buffers and device parameters
> into units of work to be queued and executed. This patch introduces the
> UAPI and core framework.
>
> This patch is based on the previous work by Laurent Pinchart. The core
> has changed considerably, but the UAPI is mostly untouched.
>
> Signed-off-by: Alexandre Courbot 

 --- /dev/null
> +++ b/drivers/media/media-request.c
> @@ -0,0 +1,390 @@
> +/*
> + * Request and request queue base management
> + *
> + * Copyright (C) 2017, The Chromium OS Authors.  All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * 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.
> + */

Have you considered using the new SPDX tags instead of this fine but
long legalese? And if other Chromium contributors could follow suit
and you could spread the word that would be even better!

See Thomas doc patches [1] for details.
Thanks!

[1] https://lkml.org/lkml/2017/12/4/934
-- 
Cordially
Philippe Ombredanne


Re: [PATCH 2/9] v4l: vsp1: Print the correct blending unit name in debug messages

2017-12-15 Thread Kieran Bingham
Hi Laurent,

On 03/12/17 10:57, Laurent Pinchart wrote:
> The DRM pipelines can use either the BRU or the BRS for blending. Make
> sure the right name is used in debugging messages to avoid confusion.

This could likely tag along with the preceding [PATCH 1/9] on it's short cut to
mainline before the rest of the CRC series is reviewed.

> Signed-off-by: Laurent Pinchart 

Reviewed-by: Kieran Bingham 

> ---
>  drivers/media/platform/vsp1/vsp1_drm.c | 12 +++-
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_drm.c 
> b/drivers/media/platform/vsp1/vsp1_drm.c
> index ac85942162c1..0fe541084f5c 100644
> --- a/drivers/media/platform/vsp1/vsp1_drm.c
> +++ b/drivers/media/platform/vsp1/vsp1_drm.c
> @@ -400,8 +400,11 @@ static int vsp1_du_setup_rpf_pipe(struct vsp1_device 
> *vsp1,
>   struct v4l2_subdev_selection sel;
>   struct v4l2_subdev_format format;
>   const struct v4l2_rect *crop;
> + const char *bru_name;
>   int ret;
>  
> + bru_name = pipe->bru->type == VSP1_ENTITY_BRU ? "BRU" : "BRS";
> +
>   /*
>* Configure the format on the RPF sink pad and propagate it up to the
>* BRU sink pad.
> @@ -473,9 +476,9 @@ static int vsp1_du_setup_rpf_pipe(struct vsp1_device 
> *vsp1,
>   if (ret < 0)
>   return ret;
>  
> - dev_dbg(vsp1->dev, "%s: set format %ux%u (%x) on BRU pad %u\n",
> + dev_dbg(vsp1->dev, "%s: set format %ux%u (%x) on %s pad %u\n",
>   __func__, format.format.width, format.format.height,
> - format.format.code, format.pad);
> + format.format.code, bru_name, format.pad);
>  
>   sel.pad = bru_input;
>   sel.target = V4L2_SEL_TGT_COMPOSE;
> @@ -486,10 +489,9 @@ static int vsp1_du_setup_rpf_pipe(struct vsp1_device 
> *vsp1,
>   if (ret < 0)
>   return ret;
>  
> - dev_dbg(vsp1->dev,
> - "%s: set selection (%u,%u)/%ux%u on BRU pad %u\n",
> + dev_dbg(vsp1->dev, "%s: set selection (%u,%u)/%ux%u on %s pad %u\n",
>   __func__, sel.r.left, sel.r.top, sel.r.width, sel.r.height,
> - sel.pad);
> + bru_name, sel.pad);
>  
>   return 0;
>  }
> 


Re: [PATCH 1/9] v4l: vsp1: Fix display stalls when requesting too many inputs

2017-12-15 Thread Kieran Bingham
Hi Laurent,

As this is a prevents hardware hangs, and is a distinct patch on it's own - I
feel it should be on an accelerated path to integration, and should be merged
separately from the rest of the CRC feature series.

On 03/12/17 10:57, Laurent Pinchart wrote:
> Make sure we don't accept more inputs than the hardware can handle. This
> is a temporary fix to avoid display stall, we need to instead allocate
> the BRU or BRS to display pipelines dynamically based on the number of
> planes they each use.
> 
> Signed-off-by: Laurent Pinchart 

Reviewed-by: Kieran Bingham 

> ---
>  drivers/media/platform/vsp1/vsp1_drm.c | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_drm.c 
> b/drivers/media/platform/vsp1/vsp1_drm.c
> index 7ce69f23f50a..ac85942162c1 100644
> --- a/drivers/media/platform/vsp1/vsp1_drm.c
> +++ b/drivers/media/platform/vsp1/vsp1_drm.c
> @@ -530,6 +530,15 @@ void vsp1_du_atomic_flush(struct device *dev, unsigned 
> int pipe_index)
>   struct vsp1_rwpf *rpf = vsp1->rpf[i];
>   unsigned int j;
>  
> + /*
> +  * Make sure we don't accept more inputs than the hardware can
> +  * handle. This is a temporary fix to avoid display stall, we
> +  * need to instead allocate the BRU or BRS to display pipelines
> +  * dynamically based on the number of planes they each use.
> +  */
> + if (pipe->num_inputs >= pipe->bru->source_pad)
> + pipe->inputs[i] = NULL;
> +
>   if (!pipe->inputs[i])
>   continue;
>  
> 


[PATCH] v4l2-dv-timings: add v4l2_hdmi_colorimetry()

2017-12-15 Thread Hans Verkuil
Add the v4l2_hdmi_colorimetry() function so we have a single function
that determines the colorspace, YCbCr encoding, quantization range and
transfer function from the InfoFrame data.

Signed-off-by: Hans Verkuil 
---
Tim, you can add this patch to your driver patch series and just call it.
Note that this colorspace information is what the HDMI gives you, if you
need to convert this than you will need to update this information accordingly
(e.g. lim to full range, 601 to 709 conversions, etc.) before giving it to
userspace.

Jose, I'm not sure if you need it in your driver, but this is probably useful
regardless.

Regards,

Hans
---
 drivers/media/v4l2-core/v4l2-dv-timings.c | 141 ++
 include/media/v4l2-dv-timings.h   |  21 +
 2 files changed, 162 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-dv-timings.c 
b/drivers/media/v4l2-core/v4l2-dv-timings.c
index 930f9c53a64e..0182d3d3f807 100644
--- a/drivers/media/v4l2-core/v4l2-dv-timings.c
+++ b/drivers/media/v4l2-core/v4l2-dv-timings.c
@@ -27,6 +27,7 @@
 #include 
 #include 
 #include 
+#include 

 MODULE_AUTHOR("Hans Verkuil");
 MODULE_DESCRIPTION("V4L2 DV Timings Helper Functions");
@@ -814,3 +815,143 @@ struct v4l2_fract v4l2_calc_aspect_ratio(u8 
hor_landscape, u8 vert_portrait)
return aspect;
 }
 EXPORT_SYMBOL_GPL(v4l2_calc_aspect_ratio);
+
+/** v4l2_hdmi_rx_colorimetry - determine HDMI colorimetry information
+ * based on various InfoFrames.
+ * @avi - the AVI InfoFrame
+ * @hdmi - the HDMI Vendor InfoFrame, may be NULL
+ * @height - the frame height
+ *
+ * Determines the HDMI colorimetry information, i.e. how the HDMI
+ * pixel color data should be interpreted.
+ *
+ * Note that some of the newer features (DCI-P3, HDR) are not yet
+ * implemented: the hdmi.h header needs to be updated to the HDMI 2.0
+ * and CTA-861-G standards.
+ */
+struct v4l2_hdmi_colorimetry
+v4l2_hdmi_rx_colorimetry(const struct hdmi_avi_infoframe *avi,
+const struct hdmi_vendor_infoframe *hdmi,
+unsigned int height)
+{
+   struct v4l2_hdmi_colorimetry c = {
+   V4L2_COLORSPACE_SRGB,
+   V4L2_YCBCR_ENC_DEFAULT,
+   V4L2_QUANTIZATION_FULL_RANGE,
+   V4L2_XFER_FUNC_SRGB
+   };
+   bool is_ce = avi->video_code || (hdmi && hdmi->vic);
+   bool is_sdtv = height <= 576;
+   bool default_is_lim_range_rgb = avi->video_code > 1;
+
+   switch (avi->colorspace) {
+   case HDMI_COLORSPACE_RGB:
+   /* RGB pixel encoding */
+   switch (avi->colorimetry) {
+   case HDMI_COLORIMETRY_EXTENDED:
+   switch (avi->extended_colorimetry) {
+   case HDMI_EXTENDED_COLORIMETRY_ADOBE_RGB:
+   c.colorspace = V4L2_COLORSPACE_ADOBERGB;
+   c.xfer_func = V4L2_XFER_FUNC_ADOBERGB;
+   break;
+   case HDMI_EXTENDED_COLORIMETRY_BT2020:
+   c.colorspace = V4L2_COLORSPACE_BT2020;
+   c.xfer_func = V4L2_XFER_FUNC_709;
+   break;
+   default:
+   break;
+   }
+   break;
+   default:
+   break;
+   }
+   switch (avi->quantization_range) {
+   case HDMI_QUANTIZATION_RANGE_LIMITED:
+   c.quantization = V4L2_QUANTIZATION_LIM_RANGE;
+   break;
+   case HDMI_QUANTIZATION_RANGE_FULL:
+   break;
+   default:
+   if (default_is_lim_range_rgb)
+   c.quantization = V4L2_QUANTIZATION_LIM_RANGE;
+   break;
+   }
+   break;
+
+   default:
+   /* YCbCr pixel encoding */
+   c.quantization = V4L2_QUANTIZATION_LIM_RANGE;
+   switch (avi->colorimetry) {
+   case HDMI_COLORIMETRY_NONE:
+   if (!is_ce)
+   break;
+   if (is_sdtv) {
+   c.colorspace = V4L2_COLORSPACE_SMPTE170M;
+   c.ycbcr_enc = V4L2_YCBCR_ENC_601;
+   } else {
+   c.colorspace = V4L2_COLORSPACE_REC709;
+   c.ycbcr_enc = V4L2_YCBCR_ENC_709;
+   }
+   c.xfer_func = V4L2_XFER_FUNC_709;
+   break;
+   case HDMI_COLORIMETRY_ITU_601:
+   c.colorspace = V4L2_COLORSPACE_SMPTE170M;
+   c.ycbcr_enc = V4L2_YCBCR_ENC_601;
+   c.xfer_func = V4L2_XFER_FUNC_709;
+   break;
+   

Re: [PATCH] media: v4l: xilinx: Use SPDX-License-Identifier

2017-12-15 Thread Mauro Carvalho Chehab
Em Fri, 15 Dec 2017 00:02:21 +0200
Laurent Pinchart  escreveu:

> Hi Mauro,
> 
> On Thursday, 14 December 2017 23:50:03 EET Mauro Carvalho Chehab wrote:
> > Em Thu, 14 Dec 2017 21:57:06 +0100 Greg KH escreveu:  
> > > On Thu, Dec 14, 2017 at 10:44:16PM +0200, Laurent Pinchart wrote:  
> > >> On Thursday, 14 December 2017 22:08:51 EET Greg KH wrote:  
> > >>> On Thu, Dec 14, 2017 at 09:05:27PM +0200, Laurent Pinchart wrote:  
> >  On Thursday, 14 December 2017 20:54:39 EET Joe Perches wrote:  
> > > On Thu, 2017-12-14 at 20:37 +0200, Laurent Pinchart wrote:  
> > >> On Thursday, 14 December 2017 20:32:20 EET Joe Perches wrote:  
> > >>> On Thu, 2017-12-14 at 20:28 +0200, Laurent Pinchart wrote:  
> >  On Thursday, 14 December 2017 19:05:27 EET Mauro Carvalho Chehab
> >  wrote:  
> > > Em Fri,  8 Dec 2017 18:05:37 +0530 Dhaval Shah escreveu:  
> > >> SPDX-License-Identifier is used for the Xilinx Video IP and
> > >> related drivers.
> > >> 
> > >> Signed-off-by: Dhaval Shah   
> > > 
> > > Hi Dhaval,
> > > 
> > > You're not listed as one of the Xilinx driver maintainers. I'm
> > > afraid that, without their explicit acks, sent to the ML, I
> > > can't accept a patch touching at the driver's license tags.  
> >  
> >  The patch doesn't change the license, I don't see why it would
> >  cause any issue. Greg isn't listed as the maintainer or copyright
> >  holder of any of the 10k+ files to which he added an SPDX license
> >  header in the last kernel release.  
> > >>> 
> > >>> Adding a comment line that describes an implicit or
> > >>> explicit license is different than removing the license
> > >>> text itself.  
> > >> 
> > >> The SPDX license header is meant to be equivalent to the license
> > >> text.  
> > > 
> > > I understand that.
> > > At a minimum, removing BSD license text is undesirable
> > > 
> > > as that license states:
> > >  ** Redistributions of source code must retain the above copyright
> > >  *  notice, this list of conditions and the following disclaimer.
> > > 
> > > etc...  
> >  
> >  But this patch only removes the following text:
> >  
> >  - * This program is free software; you can redistribute it and/or
> >  modify
> >  - * it under the terms of the GNU General Public License version 2 as
> >  - * published by the Free Software Foundation.
> >  
> >  and replaces it by the corresponding SPDX header.
> >    
> > >> The only reason why the large SPDX patch didn't touch the whole
> > >> kernel in one go was that it was easier to split in in multiple
> > >> chunks.  
> > > 
> > > Not really, it was scripted.  
> >  
> >  But still manually reviewed as far as I know.
> >    
> > >> This is no different than not including the full GPL license in
> > >> every header file but only pointing to it through its name and
> > >> reference, as every kernel source file does.  
> > > 
> > > Not every kernel source file had a license text
> > > or a reference to another license file.  
> >  
> >  Correct, but the files touched by this patch do.
> >  
> >  This issue is in no way specific to linux-media and should be
> >  decided upon at the top level, not on a per-subsystem basis. Greg,
> >  could you comment on this ?  
> > >>> 
> > >>> Comment on what exactly?  I don't understand the problem here, care to
> > >>> summarize it?  
> > >> 
> > >> In a nutshell (if I understand it correctly), Dhaval Shah submitted
> > >> https:// patchwork.kernel.org/patch/10102451/ which replaces
> > >> 
> > >> +// SPDX-License-Identifier: GPL-2.0
> > >> [...]
> > >> - *
> > >> - * This program is free software; you can redistribute it and/or modify
> > >> - * it under the terms of the GNU General Public License version 2 as
> > >> - * published by the Free Software Foundation.
> > >> 
> > >> in all .c and .h files of the Xilinx V4L2 driver
> > >> (drivers/media/platform/
> > >> xilinx). I have reviewed the patch and acked it. Mauro then rejected it,
> > >> stating that he can't accept a change to license text without an
> > >> explicit ack from the official driver's maintainers. My position is
> > >> that such a change doesn't change the license and thus doesn't need to
> > >> track all copyright holders, and can be merged without an explicit ack
> > >> from the respective maintainers.  
> > > 
> > > Yes, I agree with you, no license is being changed here, and no
> > > copyright is either.
> > > 
> > > BUT, I know that most major companies are reviewing this process right
> > > now.  We have gotten approval from almost all of the major kernel
> > > developer companies to do this, which is great, and supports 

Re: [PATCH] em28xx: split up em28xx_dvb_init to reduce stack size

2017-12-15 Thread Arnd Bergmann
On Thu, Dec 14, 2017 at 6:10 PM, Mauro Carvalho Chehab
 wrote:
> Em Mon, 11 Dec 2017 13:05:02 +0100
> Arnd Bergmann  escreveu:
>
>> With CONFIG_KASAN, the init function uses a large amount of kernel stack:
>>
>> drivers/media/usb/em28xx/em28xx-dvb.c: In function 'em28xx_dvb_init.part.4':
>> drivers/media/usb/em28xx/em28xx-dvb.c:2061:1: error: the frame size of 3232 
>> bytes is larger than 2048 bytes [-Werror=frame-larger-than=]
>>
>> Using gcc-7 with -fsanitize-address-use-after-scope makes this even worse:
>>
>> drivers/media/usb/em28xx/em28xx-dvb.c: In function 'em28xx_dvb_init':
>> drivers/media/usb/em28xx/em28xx-dvb.c:2069:1: error: the frame size of 4280 
>> bytes is larger than 3072 bytes [-Werror=frame-larger-than=]
>>
>> By splitting out each part of the switch/case statement that has its own 
>> local
>> variables into a separate function, no single one of them uses more than 500 
>> bytes,
>> and with a noinline_for_stack annotation we can ensure that they are not 
>> merged
>> back together.
>
> The right fix here is really to find a way to simplify the new method
> of binding I2C devices by using some ancillary routines.
>
> I'll keep this patch on my queue for now, but my plan is to try to solve
> this issue instead of applying it, maybe on the next weeks (as the
> volume of patches reduce due to end of year vacations and Seasons).

That's ok, thanks. We have a workaround in linux-mm that partially solves this
problem to the point where the stack size goes down to 1600 bytes with KASAN,
that by itself would be sufficient to let us enable CONFIG_FRAME_WARN
again for 64-bit platforms with the default 2048 byte warning limit. I reposted
that patch mostly since I want to lower the frame sizes further so we can
reduce the warning limit to 1280 bytes for 64-bit architectures in the future.
There around 10 patches needed for that, and they mostly seem to address
actual issues, so I'd like them to get addressed eventually and set the limit
low enough that the warnings we get on 64-bit are more useful again.

However, could you please revisit two other patches:

https://patchwork.linuxtv.org/patch/45716/ dvb-frontends: fix i2c
access helpers for KASAN
https://patchwork.linuxtv.org/patch/45709/ r820t: fix r820t_write_reg for KASAN

These are currently the ones I'm most interested in getting merged
into v4.15 and LTS kernels for the stack size reduction, since they
are blocking the patch that enables CONFIG_FRAME_WARN for
allmodconfig.

Thanks,

   Arnd


Re: [PATCH] media: v4l: xilinx: Use SPDX-License-Identifier

2017-12-15 Thread Mauro Carvalho Chehab
Em Fri, 15 Dec 2017 10:55:26 +0530
Dhaval Shah  escreveu:

> Hi Laurent/Mauro/Greg,
> 
> On Fri, Dec 15, 2017 at 3:32 AM, Laurent Pinchart
>  wrote:
> > Hi Mauro,
> >
> > On Thursday, 14 December 2017 23:50:03 EET Mauro Carvalho Chehab wrote:
> >> Em Thu, 14 Dec 2017 21:57:06 +0100 Greg KH escreveu:
> >> > On Thu, Dec 14, 2017 at 10:44:16PM +0200, Laurent Pinchart wrote:
> >> >> On Thursday, 14 December 2017 22:08:51 EET Greg KH wrote:
> >> >>> On Thu, Dec 14, 2017 at 09:05:27PM +0200, Laurent Pinchart wrote:
> >>  On Thursday, 14 December 2017 20:54:39 EET Joe Perches wrote:
> >> > On Thu, 2017-12-14 at 20:37 +0200, Laurent Pinchart wrote:
> >> >> On Thursday, 14 December 2017 20:32:20 EET Joe Perches wrote:
> >> >>> On Thu, 2017-12-14 at 20:28 +0200, Laurent Pinchart wrote:
> >>  On Thursday, 14 December 2017 19:05:27 EET Mauro Carvalho Chehab
> >>  wrote:
> >> > Em Fri,  8 Dec 2017 18:05:37 +0530 Dhaval Shah escreveu:
> >> >> SPDX-License-Identifier is used for the Xilinx Video IP and
> >> >> related drivers.
> >> >>
> >> >> Signed-off-by: Dhaval Shah 
> >> >
> >> > Hi Dhaval,
> >> >
> >> > You're not listed as one of the Xilinx driver maintainers. I'm
> >> > afraid that, without their explicit acks, sent to the ML, I
> >> > can't accept a patch touching at the driver's license tags.
> >> 
> >>  The patch doesn't change the license, I don't see why it would
> >>  cause any issue. Greg isn't listed as the maintainer or copyright
> >>  holder of any of the 10k+ files to which he added an SPDX license
> >>  header in the last kernel release.
> >> >>>
> >> >>> Adding a comment line that describes an implicit or
> >> >>> explicit license is different than removing the license
> >> >>> text itself.
> >> >>
> >> >> The SPDX license header is meant to be equivalent to the license
> >> >> text.
> >> >
> >> > I understand that.
> >> > At a minimum, removing BSD license text is undesirable
> >> >
> >> > as that license states:
> >> >  ** Redistributions of source code must retain the above 
> >> > copyright
> >> >  *  notice, this list of conditions and the following disclaimer.
> >> >
> >> > etc...
> >> 
> >>  But this patch only removes the following text:
> >> 
> >>  - * This program is free software; you can redistribute it and/or
> >>  modify
> >>  - * it under the terms of the GNU General Public License version 2 as
> >>  - * published by the Free Software Foundation.
> >> 
> >>  and replaces it by the corresponding SPDX header.
> >> 
> >> >> The only reason why the large SPDX patch didn't touch the whole
> >> >> kernel in one go was that it was easier to split in in multiple
> >> >> chunks.
> >> >
> >> > Not really, it was scripted.
> >> 
> >>  But still manually reviewed as far as I know.
> >> 
> >> >> This is no different than not including the full GPL license in
> >> >> every header file but only pointing to it through its name and
> >> >> reference, as every kernel source file does.
> >> >
> >> > Not every kernel source file had a license text
> >> > or a reference to another license file.
> >> 
> >>  Correct, but the files touched by this patch do.
> >> 
> >>  This issue is in no way specific to linux-media and should be
> >>  decided upon at the top level, not on a per-subsystem basis. Greg,
> >>  could you comment on this ?
> >> >>>
> >> >>> Comment on what exactly?  I don't understand the problem here, care to
> >> >>> summarize it?
> >> >>
> >> >> In a nutshell (if I understand it correctly), Dhaval Shah submitted
> >> >> https:// patchwork.kernel.org/patch/10102451/ which replaces
> >> >>
> >> >> +// SPDX-License-Identifier: GPL-2.0
> >> >> [...]
> >> >> - *
> >> >> - * This program is free software; you can redistribute it and/or modify
> >> >> - * it under the terms of the GNU General Public License version 2 as
> >> >> - * published by the Free Software Foundation.
> >> >>
> >> >> in all .c and .h files of the Xilinx V4L2 driver
> >> >> (drivers/media/platform/
> >> >> xilinx). I have reviewed the patch and acked it. Mauro then rejected it,
> >> >> stating that he can't accept a change to license text without an
> >> >> explicit ack from the official driver's maintainers. My position is
> >> >> that such a change doesn't change the license and thus doesn't need to
> >> >> track all copyright holders, and can be merged without an explicit ack
> >> >> from the respective maintainers.
> >> >
> >> > Yes, I agree with you, no license is being changed here, and no
> >> > copyright is either.
> >> >
> >> > BUT, I know that most major companies are reviewing this 

Re: [RFC PATCH] media: v4l2-device: Link subdevices to their parent devices if available

2017-12-15 Thread Sakari Ailus
On Fri, Dec 15, 2017 at 01:32:21PM +0900, Tomasz Figa wrote:
> Currently v4l2_device_register_subdev_nodes() does not initialize the
> dev_parent field of the video_device structs it creates for subdevices
> being registered. This leads to __video_register_device() falling back
> to the parent device of associated v4l2_device struct, which often does
> not match the physical device the subdevice is registered for.
> 
> Due to the problem above, the links between real devices and v4l-subdev
> nodes cannot be obtained from sysfs, which might be confusing for the
> userspace trying to identify the hardware.
> 
> Fix this by initializing the dev_parent field of the video_device struct
> with the value of dev field of the v4l2_subdev struct. In case of
> subdevices without a parent struct device, the field will be NULL and the
> old behavior will be preserved by the semantics of
> __video_register_device().
> 
> Signed-off-by: Tomasz Figa 

Acked-by: Sakari Ailus 

-- 
Sakari Ailus
sakari.ai...@linux.intel.com


[GIT PULL FOR v4.16, v2] media: imx: Add better OF graph support

2017-12-15 Thread Hans Verkuil
Note: the new v4l2-async work makes it possible to simplify this. That
will be done in follow-up patches. It's easier to do that if this is in
first.

This v2 is just a rebased version of v1 to fix merge conflicts.

Regards,

Hans

The following changes since commit 0ca4e3130402caea8731a7b54afde56a6edb17c9:

  media: pxa_camera: rename the soc_camera_ prefix to pxa_camera_ (2017-12-14 
12:40:01 -0500)

are available in the Git repository at:

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

for you to fetch changes up to c7db71987c4fdbfcc62cb01f5f88fee25e2d0df0:

  media: staging/imx: update TODO (2017-12-15 09:46:20 +0100)


Steve Longerbeam (9):
  media: staging/imx: get CSI bus type from nearest upstream entity
  media: staging/imx: remove static media link arrays
  media: staging/imx: of: allow for recursing downstream
  media: staging/imx: remove devname string from imx_media_subdev
  media: staging/imx: pass fwnode handle to find/add async subdev
  media: staging/imx: remove static subdev arrays
  media: staging/imx: convert static vdev lists to list_head
  media: staging/imx: reorder function prototypes
  media: staging/imx: update TODO

 drivers/staging/media/imx/TODO|  63 +++--
 drivers/staging/media/imx/imx-ic-prp.c|   4 +-
 drivers/staging/media/imx/imx-media-capture.c |   2 +
 drivers/staging/media/imx/imx-media-csi.c | 187 
+++
 drivers/staging/media/imx/imx-media-dev.c | 401 
+-
 drivers/staging/media/imx/imx-media-internal-sd.c | 253 
++--
 drivers/staging/media/imx/imx-media-of.c  | 278 

 drivers/staging/media/imx/imx-media-utils.c   | 122 --
 drivers/staging/media/imx/imx-media.h | 187 
++-
 9 files changed, 721 insertions(+), 776 deletions(-)


[RFC PATCH 0/9] media: base request API support

2017-12-15 Thread Alexandre Courbot
Here is a new attempt at the request API, following the UAPI we agreed on in
Prague. Hopefully this can be used as the basis to move forward.

This series only introduces the very basics of how requests work: allocate a
request, queue buffers to it, queue the request itself, wait for it to complete,
reuse it. It does *not* yet use Hans' work with controls setting. I have
preferred to submit it this way for now as it allows us to concentrate on the
basic request/buffer flow, which was harder to get properly than I initially
thought. I still have a gut feeling that it can be improved, with less back-and-
forth into drivers.

Plugging in controls support should not be too hard a task (basically just apply
the saved controls when the request starts), and I am looking at it now.

The resulting vim2m driver can be successfully used with requests, and my tests
so far have been successful.

There are still some rougher edges:

* locking is currently quite coarse-grained
* too many #ifdef CONFIG_MEDIA_CONTROLLER in the code, as the request API
  depends on it - I plan to craft the headers so that it becomes unnecessary.
  As it is, some of the code will probably not even compile if
  CONFIG_MEDIA_CONTROLLER is not set

But all in all I think the request flow should be clear and easy to review, and
the possibility of custom queue and entity support implementations should give
us the flexibility we need to support more specific use-cases (I expect the
generic implementations to be sufficient most of the time though).

A very simple test program exercising this API is available here (don't forget
to adapt the /dev/media0 hardcoding):
https://gist.github.com/Gnurou/dbc3776ed97ea7d4ce6041ea15eb0438

Looking forward to your feedback and comments!

Alexandre Courbot (8):
  media: add request API core and UAPI
  media: request: add generic queue
  media: request: add generic entity ops
  media: vb2: add support for requests
  media: vb2: add support for requests in QBUF ioctl
  media: v4l2-mem2mem: add request support
  media: vim2m: add media device
  media: vim2m: add request support

Hans Verkuil (1):
  videodev2.h: Add request field to v4l2_buffer

 drivers/media/Makefile|   4 +-
 drivers/media/media-device.c  |   6 +
 drivers/media/media-request-entity-generic.c  |  56 
 drivers/media/media-request-queue-generic.c   | 150 ++
 drivers/media/media-request.c | 390 ++
 drivers/media/platform/vim2m.c|  46 +++
 drivers/media/usb/cpia2/cpia2_v4l.c   |   2 +-
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c |   7 +-
 drivers/media/v4l2-core/v4l2-ioctl.c  |  99 ++-
 drivers/media/v4l2-core/v4l2-mem2mem.c|  34 +++
 drivers/media/v4l2-core/videobuf2-core.c  |  59 +++-
 drivers/media/v4l2-core/videobuf2-v4l2.c  |  32 ++-
 include/media/media-device.h  |   3 +
 include/media/media-entity.h  |   6 +
 include/media/media-request.h | 282 +++
 include/media/v4l2-mem2mem.h  |  19 ++
 include/media/videobuf2-core.h|  25 +-
 include/media/videobuf2-v4l2.h|   2 +
 include/uapi/linux/media.h|  11 +
 include/uapi/linux/videodev2.h|   3 +-
 20 files changed, 1216 insertions(+), 20 deletions(-)
 create mode 100644 drivers/media/media-request-entity-generic.c
 create mode 100644 drivers/media/media-request-queue-generic.c
 create mode 100644 drivers/media/media-request.c
 create mode 100644 include/media/media-request.h

-- 
2.15.1.504.g5279b80103-goog