[PATCH v2 4/4] ARM: dts: imx: Add TDA19971 HDMI Receiver to GW54xx

[Tim Harvey]

The GW54xx has a front-panel microHDMI connector routed to a TDA19971
which is connected the the IPU CSI when using IMX6Q.

Signed-off-by: Tim Harvey 
---
v2:
 - use new audio bindings
 - add HDMI audio input support

---
 arch/arm/boot/dts/imx6q-gw54xx.dts| 102 ++
 arch/arm/boot/dts/imx6qdl-gw54xx.dtsi |  29 +-
 2 files changed, 128 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/imx6q-gw54xx.dts 
b/arch/arm/boot/dts/imx6q-gw54xx.dts
index 56e5b50..99dac63 100644
--- a/arch/arm/boot/dts/imx6q-gw54xx.dts
+++ b/arch/arm/boot/dts/imx6q-gw54xx.dts
@@ -12,10 +12,27 @@
 /dts-v1/;
 #include "imx6q.dtsi"
 #include "imx6qdl-gw54xx.dtsi"
+#include 
 
 / {
model = "Gateworks Ventana i.MX6 Dual/Quad GW54XX";
compatible = "gw,imx6q-gw54xx", "gw,ventana", "fsl,imx6q";
+
+   sound-digital {
+   compatible = "simple-audio-card";
+   simple-audio-card,name = "tda1997x-audio";
+   simple-audio-card,dai-link@0 {
+   format = "i2s";
+   cpu {
+   sound-dai = <>;
+   };
+   codec {
+   bitclock-master;
+   frame-master;
+   sound-dai = <>;
+   };
+   };
+   };
 };
 
  {
@@ -35,6 +52,61 @@
};
};
};
+
+   tda1997x: codec@48 {
+   compatible = "nxp,tda19971";
+   pinctrl-names = "default";
+   pinctrl-0 = <_tda1997x>;
+   reg = <0x48>;
+   interrupt-parent = <>;
+   interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
+   DOVDD-supply = <_3p3v>;
+   AVDD-supply = <_reg>;
+   DVDD-supply = <_reg>;
+   #sound-dai-cells = <0>;
+   nxp,audout-format = "i2s";
+   nxp,audout-layout = <0>;
+   nxp,audout-width = <16>;
+   nxp,audout-mclk-fs = <128>;
+   /*
+* The 8bpp YUV422 semi-planar mode outputs CbCr[11:4]
+* and Y[11:4] across 16bits in the same cycle
+* which we map to VP[15:08]<->CSI_DATA[19:12]
+*/
+   nxp,vidout-portcfg =
+   /*G_Y_11_8<->VP[15:12]<->CSI_DATA[19:16]*/
+   < TDA1997X_VP24_V15_12 TDA1997X_G_Y_11_8 >,
+   /*G_Y_7_4<->VP[11:08]<->CSI_DATA[15:12]*/
+   < TDA1997X_VP24_V11_08 TDA1997X_G_Y_7_4 >,
+   /*R_CR_CBCR_11_8<->VP[07:04]<->CSI_DATA[11:08]*/
+   < TDA1997X_VP24_V07_04 TDA1997X_R_CR_CBCR_11_8 >,
+   /*R_CR_CBCR_7_4<->VP[03:00]<->CSI_DATA[07:04]*/
+   < TDA1997X_VP24_V03_00 TDA1997X_R_CR_CBCR_7_4 >;
+
+   port {
+   tda1997x_to_ipu1_csi0_mux: endpoint {
+   remote-endpoint = 
<_csi0_mux_from_parallel_sensor>;
+   bus-width = <16>;
+   hsync-active = <1>;
+   vsync-active = <1>;
+   data-active = <1>;
+   };
+   };
+   };
+};
+
+_csi0_from_ipu1_csi0_mux {
+   bus-width = <16>;
+};
+
+_csi0_mux_from_parallel_sensor {
+   remote-endpoint = <_to_ipu1_csi0_mux>;
+   bus-width = <16>;
+};
+
+_csi0 {
+   pinctrl-names = "default";
+   pinctrl-0 = <_ipu1_csi0>;
 };
 
 _csi1_from_ipu2_csi1_mux {
@@ -63,6 +135,30 @@
>;
};
 
+   pinctrl_ipu1_csi0: ipu1_csi0grp {
+   fsl,pins = <
+   MX6QDL_PAD_CSI0_DAT4__IPU1_CSI0_DATA04  0x1b0b0
+   MX6QDL_PAD_CSI0_DAT5__IPU1_CSI0_DATA05  0x1b0b0
+   MX6QDL_PAD_CSI0_DAT6__IPU1_CSI0_DATA06  0x1b0b0
+   MX6QDL_PAD_CSI0_DAT7__IPU1_CSI0_DATA07  0x1b0b0
+   MX6QDL_PAD_CSI0_DAT8__IPU1_CSI0_DATA08  0x1b0b0
+   MX6QDL_PAD_CSI0_DAT9__IPU1_CSI0_DATA09  0x1b0b0
+   MX6QDL_PAD_CSI0_DAT10__IPU1_CSI0_DATA10 0x1b0b0
+   MX6QDL_PAD_CSI0_DAT11__IPU1_CSI0_DATA11 0x1b0b0
+   MX6QDL_PAD_CSI0_DAT12__IPU1_CSI0_DATA12 0x1b0b0
+   MX6QDL_PAD_CSI0_DAT13__IPU1_CSI0_DATA13 0x1b0b0
+   MX6QDL_PAD_CSI0_DAT14__IPU1_CSI0_DATA14 0x1b0b0
+   MX6QDL_PAD_CSI0_DAT15__IPU1_CSI0_DATA15 0x1b0b0
+   MX6QDL_PAD_CSI0_DAT16__IPU1_CSI0_DATA16 0x1b0b0
+   MX6QDL_PAD_CSI0_DAT17__IPU1_CSI0_DATA17 0x1b0b0
+   MX6QDL_PAD_CSI0_DAT18__IPU1_CSI0_DATA18 0x1b0b0
+  

[PATCH v2 3/4] media: i2c: Add TDA1997x HDMI receiver driver

[Tim Harvey]

Add support for the TDA1997x HDMI receivers.

Cc: Hans Verkuil 
Signed-off-by: Tim Harvey 
---
v2:
 - implement dv timings enum/cap
 - remove deprecated g_mbus_config op
 - fix dv_query_timings
 - add EDID get/set handling
 - remove max-pixel-rate support
 - add audio codec DAI support
 - use new audio bindings

---
 drivers/media/i2c/Kconfig|9 +
 drivers/media/i2c/Makefile   |1 +
 drivers/media/i2c/tda1997x.c | 3336 ++
 include/dt-bindings/media/tda1997x.h |   78 +
 include/media/i2c/tda1997x.h |   53 +
 5 files changed, 3477 insertions(+)
 create mode 100644 drivers/media/i2c/tda1997x.c
 create mode 100644 include/dt-bindings/media/tda1997x.h
 create mode 100644 include/media/i2c/tda1997x.h

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 9415389..c2b0400 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -56,6 +56,15 @@ config VIDEO_TDA9840
  To compile this driver as a module, choose M here: the
  module will be called tda9840.
 
+config VIDEO_TDA1997X
+   tristate "NXP TDA1997x HDMI receiver"
+   depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API
+   ---help---
+ V4L2 subdevice driver for the NXP TDA1997x HDMI receivers.
+
+ To compile this driver as a module, choose M here: the
+ module will be called tda1997x.
+
 config VIDEO_TEA6415C
tristate "Philips TEA6415C audio processor"
depends on I2C
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index c843c18..58f2b2e 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_VIDEO_TVAUDIO) += tvaudio.o
 obj-$(CONFIG_VIDEO_TDA7432) += tda7432.o
 obj-$(CONFIG_VIDEO_SAA6588) += saa6588.o
 obj-$(CONFIG_VIDEO_TDA9840) += tda9840.o
+obj-$(CONFIG_VIDEO_TDA1997X) += tda1997x.o
 obj-$(CONFIG_VIDEO_TEA6415C) += tea6415c.o
 obj-$(CONFIG_VIDEO_TEA6420) += tea6420.o
 obj-$(CONFIG_VIDEO_SAA7110) += saa7110.o
diff --git a/drivers/media/i2c/tda1997x.c b/drivers/media/i2c/tda1997x.c
new file mode 100644
index 000..bf06684
--- /dev/null
+++ b/drivers/media/i2c/tda1997x.c
@@ -0,0 +1,3336 @@
+/*
+ * Copyright (C) 2017 Gateworks Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+/* debug level */
+static int debug;
+module_param(debug, int, 0644);
+MODULE_PARM_DESC(debug, "debug level (0-2)");
+
+/* Page 0x00 - General Control */
+#define REG_VERSION0x
+#define REG_INPUT_SEL  0x0001
+#define REG_SVC_MODE   0x0002
+#define REG_HPD_MAN_CTRL   0x0003
+#define REG_RT_MAN_CTRL0x0004
+#define REG_STANDBY_SOFT_RST   0x000A
+#define REG_HDMI_SOFT_RST  0x000B
+#define REG_HDMI_INFO_RST  0x000C
+#define REG_INT_FLG_CLR_TOP0x000E
+#define REG_INT_FLG_CLR_SUS0x000F
+#define REG_INT_FLG_CLR_DDC0x0010
+#define REG_INT_FLG_CLR_RATE   0x0011
+#define REG_INT_FLG_CLR_MODE   0x0012
+#define REG_INT_FLG_CLR_INFO   0x0013
+#define REG_INT_FLG_CLR_AUDIO  0x0014
+#define REG_INT_FLG_CLR_HDCP   0x0015
+#define REG_INT_FLG_CLR_AFE0x0016
+#define REG_INT_MASK_TOP   0x0017
+#define REG_INT_MASK_SUS   0x0018
+#define REG_INT_MASK_DDC   0x0019
+#define REG_INT_MASK_RATE  0x001A
+#define REG_INT_MASK_MODE  0x001B
+#define REG_INT_MASK_INFO  0x001C
+#define REG_INT_MASK_AUDIO 0x001D
+#define REG_INT_MASK_HDCP  0x001E
+#define REG_INT_MASK_AFE   0x001F
+#define REG_DETECT_5V  0x0020
+#define REG_SUS_STATUS 0x0021
+#define REG_V_PER  0x0022
+#define REG_H_PER  0x0025
+#define REG_HS_WIDTH   0x0027
+#define REG_FMT_H_TOT  0x0029
+#define REG_FMT_H_ACT  0x002b
+#define REG_FMT_H_FRONT0x002d
+#define REG_FMT_H_SYNC 0x002f
+#define REG_FMT_H_BACK 0x0031
+#define REG_FMT_V_TOT  0x0033
+#define REG_FMT_V_ACT  0x0035
+#define REG_FMT_V_FRONT_F1 0x0037
+#define REG_FMT_V_FRONT_F2 0x0038
+#define REG_FMT_V_SYNC 0x0039
+#define REG_FMT_V_BACK_F1  0x003a
+#define REG_FMT_V_BACK_F2  0x003b
+#define REG_FMT_DE_ACT 0x003c
+#define REG_RATE_CTRL  0x0040
+#define REG_CLK_MIN_RATE   0x0043
+#define REG_CLK_MAX_RATE   0x0046
+#define REG_CLK_A_STATUS   0x0049
+#define REG_CLK_A_RATE 0x004A
+#define REG_DRIFT_CLK_A_REG0x004D
+#define REG_CLK_B_STATUS 

[PATCH v2 2/4] media: dt-bindings: Add bindings for TDA1997X

[Tim Harvey]

Cc: Rob Herring 
Signed-off-by: Tim Harvey 
---
v2:
 - add vendor prefix and remove _ from vidout-portcfg
 - remove _ from labels
 - remove max-pixel-rate property
 - describe and provide example for single output port
 - use new audio port bindings

---
 .../devicetree/bindings/media/i2c/tda1997x.txt | 179 +
 1 file changed, 179 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/tda1997x.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/tda1997x.txt 
b/Documentation/devicetree/bindings/media/i2c/tda1997x.txt
new file mode 100644
index 000..269d7f0
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/tda1997x.txt
@@ -0,0 +1,179 @@
+Device-Tree bindings for the NXP TDA1997x HDMI receiver
+
+The TDA19971/73 are HDMI video receivers.
+
+The TDA19971 Video port output pins can be used as follows:
+ - RGB 8bit per color (24 bits total): R[11:4] B[11:4] G[11:4]
+ - YUV444 8bit per color (24 bits total): Y[11:4] Cr[11:4] Cb[11:4]
+ - YUV422 semi-planar 8bit per component (16 bits total): Y[11:4] CbCr[11:4]
+ - YUV422 semi-planar 10bit per component (20 bits total): Y[11:2] CbCr[11:2]
+ - YUV422 semi-planar 12bit per component (24 bits total): - Y[11:0] CbCr[11:0]
+ - YUV422 BT656 8bit per component (8 bits total): YCbCr[11:4] (2-cycles)
+ - YUV422 BT656 10bit per component (10 bits total): YCbCr[11:2] (2-cycles)
+ - YUV422 BT656 12bit per component (12 bits total): YCbCr[11:0] (2-cycles)
+
+The TDA19973 Video port output pins can be used as follows:
+ - RGB 12bit per color (36 bits total): R[11:0] B[11:0] G[11:0]
+ - YUV444 12bit per color (36 bits total): Y[11:0] Cb[11:0] Cr[11:0]
+ - YUV422 semi-planar 12bit per component (24 bits total): Y[11:0] CbCr[11:0]
+ - YUV422 BT656 12bit per component (12 bits total): YCbCr[11:0] (2-cycles)
+
+The Video port output pins are mapped via 4-bit 'pin groups' allowing
+for a variety fo connection possibilities including swapping pin order within
+pin groups. The video_portcfg device-tree property consists of register mapping
+pairs which map a chip-specific VP output register to a 4-bit pin group. If
+the pin group needs to be bit-swapped you can use the *_S pin-group defines.
+
+Required Properties:
+ - compatible  :
+  - "nxp,tda19971" for the TDA19971
+  - "nxp,tda19973" for the TDA19973
+ - reg : I2C slave address
+ - interrupts  : The interrupt number
+ - DOVDD-supply: Digital I/O supply
+ - DVDD-supply : Digital Core supply
+ - AVDD-supply : Analog supply
+ - nxp,vidout-portcfg  : array of pairs mapping VP output pins to pin groups.
+
+Optional Properties:
+ - nxp,audout-format   : DAI bus format: "i2s" or "spdif".
+ - nxp,audout-width: width of audio output data bus (1-4).
+ - nxp,audout-layout   : data layout (0=AP0 used, 1=AP0/AP1/AP2/AP3 used).
+ - nxp,audout-mclk-fs  : Multiplication factor between stream rate and codec
+ mclk.
+
+The device node must contain one 'port' child node for its digital output
+video port, in accordance with the video interface bindings defined in
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Optional Endpoint Properties:
+  The following three properties are defined in video-interfaces.txt and
+  are valid for source endpoints only:
+  - hsync-active: Horizontal synchronization polarity. Defaults to active high.
+  - vsync-active: Vertical synchronization polarity. Defaults to active high.
+  - data-active: Data polarity. Defaults to active high.
+
+Examples:
+ - VP[15:0] connected to IMX6 CSI_DATA[19:4] for 16bit YUV422
+   16bit I2S layout0 with a 128*fs clock (A_WS, AP0, A_CLK pins)
+   hdmi-receiver@48 {
+   compatible = "nxp,tda19971";
+   pinctrl-names = "default";
+   pinctrl-0 = <_tda1997x>;
+   reg = <0x48>;
+   interrupt-parent = <>;
+   interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
+   DOVDD-supply = <_3p3v>;
+   AVDD-supply = <_1p8v>;
+   DVDD-supply = <_1p8v>;
+   /* audio */
+   #sound-dai-cells = <0>;
+   nxp,audout-format = "i2s";
+   nxp,audout-layout = <0>;
+   nxp,audout-width = <16>;
+   nxp,audout-mclk-fs = <128>;
+   /*
+* The 8bpp YUV422 semi-planar mode outputs CbCr[11:4]
+* and Y[11:4] across 16bits in the same pixclk cycle.
+*/
+   nxp,vidout-portcfg =
+   /* Y[11:8]<->VP[15:12]<->CSI_DATA[19:16] */
+   < TDA1997X_VP24_V15_12 TDA1997X_G_Y_11_8 >,
+   /* Y[7:4]<->VP[11:08]<->CSI_DATA[15:12] */
+   < TDA1997X_VP24_V11_08 TDA1997X_G_Y_7_4 >,
+   /* CbCc[11:8]<->VP[07:04]<->CSI_DATA[11:8] */
+   < TDA1997X_VP24_V07_04 

[PATCH v2 1/4] MAINTAINERS: add entry for NXP TDA1997x driver

[Tim Harvey]

Signed-off-by: Tim Harvey 
---
 MAINTAINERS | 8 
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 2093060..de7124e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13019,6 +13019,14 @@ T: git git://linuxtv.org/mkrufky/tuners.git
 S: Maintained
 F: drivers/media/tuners/tda18271*
 
+TDA1997x MEDIA DRIVER
+M: Tim Harvey 
+L: linux-media@vger.kernel.org
+W: https://linuxtv.org
+Q: http://patchwork.linuxtv.org/project/linux-media/list/
+S: Maintained
+F: drivers/media/i2c/tda1997x.*
+
 TDA827x MEDIA DRIVER
 M: Michael Krufky 
 L: linux-media@vger.kernel.org
-- 
2.7.4

[PATCH v2 0/4] TDA1997x HDMI video receiver

[Tim Harvey]

This is a v4l2 subdev driver supporting the TDA1997x HDMI video receiver.

I've tested this on a Gateworks GW54xx with an IMX6Q which uses the TDA19971
with 16bits connected to the IMX6 CSI. For this configuration I've tested
both 16bit YUV422 and 8bit BT656 mode. While the driver should support the
TDA1993 I do not have one for testing.

Further potential development efforts include:
 - CEC support
 - HDCP support
 - mbus format selection support for bus widths that support multiple formats
 - TDA19972 support (2 inputs)

History:
v2:
 - encorporate feedback into dt bindings
 - change audio dt bindings
 - implement dv timings enum/cap
 - remove deprecated g_mbus_config op
 - fix dv_query_timings
 - add EDID get/set handling
 - remove max-pixel-rate support
 - add audio codec DAI support
 - added media-ctl and v4l2-compliance details

v1:
 - initial RFC

Media device topology:
# media-ctl -d /dev/media0 -p
Media controller API version 4.13.0

Media device information

driver  imx-media
model   imx-media
serial  
bus info
hw revision 0x0
driver version  4.13.0

Device topology
- entity 1: adv7180 2-0020 (1 pad, 1 link)
type V4L2 subdev subtype Unknown flags 20004
device node name /dev/v4l-subdev0
pad0: Source
[fmt:UYVY8_2X8/720x480 field:interlaced colorspace:smpte170m]
-> "ipu2_csi1_mux":1 []

- entity 3: tda19971 2-0048 (1 pad, 1 link)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev1
pad0: Source
[fmt:UYVY8_1X16/0x0]
[dv.caps:BT.656/1120 min:640x480@1300 
max:1920x1080@16500 stds:CEA-861,DMT caps:progressive]
[dv.query:no-link]
[dv.current:BT.656/1120 0x0p0 (0x0) stds: flags:]
-> "ipu1_csi0_mux":1 [ENABLED]

- entity 5: ipu1_vdic (3 pads, 3 links)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev2
pad0: Sink
[fmt:AYUV8_1X32/640x480@1/30 field:none colorspace:smpte170m 
xfer:709 ycbcr:601 quantization:lim-range]
<- "ipu1_csi0":1 []
<- "ipu1_csi1":1 []
pad1: Sink
[fmt:UYVY8_2X8/640x480@1/30 field:none colorspace:smpte170m 
xfer:709 ycbcr:601 quantization:lim-range]
pad2: Source
[fmt:AYUV8_1X32/640x480@1/60 field:none colorspace:smpte170m 
xfer:709 ycbcr:601 quantization:lim-range]
-> "ipu1_ic_prp":0 []

- entity 9: ipu2_vdic (3 pads, 3 links)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev3
pad0: Sink
[fmt:AYUV8_1X32/640x480@1/30 field:none colorspace:smpte170m 
xfer:709 ycbcr:601 quantization:lim-range]
<- "ipu2_csi0":1 []
<- "ipu2_csi1":1 []
pad1: Sink
[fmt:UYVY8_2X8/640x480@1/30 field:none colorspace:smpte170m 
xfer:709 ycbcr:601 quantization:lim-range]
pad2: Source
[fmt:AYUV8_1X32/640x480@1/60 field:none colorspace:smpte170m 
xfer:709 ycbcr:601 quantization:lim-range]
-> "ipu2_ic_prp":0 []

- entity 13: ipu1_ic_prp (3 pads, 5 links)
 type V4L2 subdev subtype Unknown flags 0
 device node name /dev/v4l-subdev4
pad0: Sink
[fmt:AYUV8_1X32/640x480@1/30 field:none colorspace:smpte170m 
xfer:709 ycbcr:601 quantization:lim-range]
<- "ipu1_vdic":2 []
<- "ipu1_csi0":1 []
<- "ipu1_csi1":1 []
pad1: Source
[fmt:AYUV8_1X32/640x480@1/30 field:none colorspace:smpte170m 
xfer:709 ycbcr:601 quantization:lim-range]
-> "ipu1_ic_prpenc":0 []
pad2: Source
[fmt:AYUV8_1X32/640x480@1/30 field:none colorspace:smpte170m 
xfer:709 ycbcr:601 quantization:lim-range]
-> "ipu1_ic_prpvf":0 []

- entity 17: ipu1_ic_prpenc (2 pads, 2 links)
 type V4L2 subdev subtype Unknown flags 0
 device node name /dev/v4l-subdev5
pad0: Sink
[fmt:AYUV8_1X32/640x480@1/30 field:none colorspace:smpte170m 
xfer:709 ycbcr:601 quantization:lim-range]
<- "ipu1_ic_prp":1 []
pad1: Source
[fmt:AYUV8_1X32/640x480@1/30 field:none colorspace:smpte170m 
xfer:709 ycbcr:601 quantization:lim-range]
-> "ipu1_ic_prpenc capture":0 []

- entity 20: ipu1_ic_prpenc capture (1 pad, 1 link)
 type Node subtype V4L flags 0
 device node name /dev/video0
pad0: Sink
<- "ipu1_ic_prpenc":1 []

- entity 26: ipu1_ic_prpvf (2 pads, 2 links)
 type V4L2 subdev subtype Unknown flags 0
 device node name /dev/v4l-subdev6
pad0: Sink
[fmt:AYUV8_1X32/640x480@1/30 field:none colorspace:smpte170m 
xfer:709 ycbcr:601 

cron job: media_tree daily build: ERRORS

[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:   Thu Oct 12 05:00:16 CEST 2017
media-tree git hash:8382e556b1a2f30c4bf866f021b33577a64f9ebf
media_build git hash:   33629e38ddda7a5a6ed0f727535c45f08c788bf3
v4l-utils git hash: 01c04f7c8ad1a91af33e20621eba9200f447737e
gcc version:i686-linux-gcc (GCC) 7.1.0
sparse version: v0.5.0
smatch version: v0.5.0-3553-g78b2ea6
host hardware:  x86_64
host os:4.12.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: WARNINGS
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: WARNINGS
linux-3.13.11-i686: ERRORS
linux-3.14.9-i686: WARNINGS
linux-3.15.2-i686: WARNINGS
linux-3.16.7-i686: WARNINGS
linux-3.17.8-i686: WARNINGS
linux-3.18.7-i686: WARNINGS
linux-3.19-i686: WARNINGS
linux-4.0.9-i686: WARNINGS
linux-4.1.33-i686: WARNINGS
linux-4.2.8-i686: WARNINGS
linux-4.3.6-i686: WARNINGS
linux-4.4.22-i686: WARNINGS
linux-4.5.7-i686: WARNINGS
linux-4.6.7-i686: WARNINGS
linux-4.7.5-i686: WARNINGS
linux-4.8-i686: OK
linux-4.9.26-i686: OK
linux-4.10.14-i686: OK
linux-4.11-i686: OK
linux-4.12.1-i686: OK
linux-4.13-i686: OK
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: WARNINGS
linux-3.13.11-x86_64: ERRORS
linux-3.14.9-x86_64: WARNINGS
linux-3.15.2-x86_64: WARNINGS
linux-3.16.7-x86_64: WARNINGS
linux-3.17.8-x86_64: WARNINGS
linux-3.18.7-x86_64: WARNINGS
linux-3.19-x86_64: WARNINGS
linux-4.0.9-x86_64: WARNINGS
linux-4.1.33-x86_64: WARNINGS
linux-4.2.8-x86_64: WARNINGS
linux-4.3.6-x86_64: WARNINGS
linux-4.4.22-x86_64: WARNINGS
linux-4.5.7-x86_64: WARNINGS
linux-4.6.7-x86_64: WARNINGS
linux-4.7.5-x86_64: WARNINGS
linux-4.8-x86_64: WARNINGS
linux-4.9.26-x86_64: WARNINGS
linux-4.10.14-x86_64: WARNINGS
linux-4.11-x86_64: WARNINGS
linux-4.12.1-x86_64: WARNINGS
linux-4.13-x86_64: OK
apps: OK
spec-git: OK

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

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

[PATCH v13] MAINTAINERS: add entry for Rockchip RGA driver

[Jacob Chen]

Signed-off-by: Jacob Chen 
---
 MAINTAINERS | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 6671f375f7fc..b13dae0cbf42 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11509,6 +11509,13 @@ F: drivers/hid/hid-roccat*
 F: include/linux/hid-roccat*
 F: Documentation/ABI/*/sysfs-driver-hid-roccat*
 
+ROCKCHIP RASTER 2D GRAPHIC ACCELERATION UNIT DRIVER
+M: Jacob chen 
+L: linux-media@vger.kernel.org
+S: Maintained
+F: drivers/media/platform/rockchip/rga/
+F: Documentation/devicetree/bindings/media/rockchip-rga.txt
+
 ROCKER DRIVER
 M: Jiri Pirko 
 L: net...@vger.kernel.org
-- 
2.14.1

Re: [PATCH v3 04/26] media: lirc_zilog: remove receiver

[Devin Heitmueller]

Hi Andy,

> 5. Rx and IR Learn both use the same external hardware.  Not
> coordinating Rx with Learn mode in the same driver, will prevent Learn
> operation from working.  That is, if Learn mode is ever implemented.
> (Once upon a time, I was planning on doing that.  But I have no time
> for that anymore.)

There's not really any infrastructure in Linux that maps to the
Zilog's "learning mode" functionality.  Usually I would just tell
users to do the learning under Windows and send me the resulting .ini
file (which we could then add to the database).

I had planned on getting rid of the database entirely and just
converting an MCE compatible pulse train to the blasting format
required by the Zilog firmware (using the awesome work you sent me
privately), but the fact of the matter is that nobody cares and MCEUSB
devices are $20 online.

> I'm glad someone remembers all this stuff.  I'm assuming you had more
> pain with this than I ever did.

This would be a safe assumption.  I probably put about a month's worth
of engineering into driver work for the Zilog, which seems
extraordinary given how simple something like an IR blaster/receiver
is supposed to be.  I guess that's the fun of proving out a new
hardware design as opposed to just making something work under Linux
that is already known to work under Windows.

> I never owned an HD-PVR.

I'm sure I have a spare or two if you really want one (not that you
have the time to muck with such things nowadays).  :-)

The HD-PVR was a bit of a weird case compared to devices like ivtv and
cx18 because it was technically multi-master (I2C commands came both
from the host and from the onboard SOC).  Hence you could have weird
cases where one would block the other at unexpected times.  I2C
commands to the Zilog would hold the bus which would delay the onboard
firmware from issuing commands to the video decoder (fun timing
issues).  There was also some weird edge case I don't recall the
details of that prompted them to add an I2C gate in later board
revisions.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

RE: [PATCH v5 3/3] intel-ipu3: cio2: Add new MIPI-CSI2 driver

[Zhi, Yong]

Hi, Sakari,

Thanks for your quick review!!

> -Original Message-
> From: linux-media-ow...@vger.kernel.org [mailto:linux-media-
> ow...@vger.kernel.org] On Behalf Of Sakari Ailus
> Sent: Tuesday, October 10, 2017 12:46 AM
> To: Zhi, Yong 
> Cc: linux-media@vger.kernel.org; sakari.ai...@linux.intel.com;
> hans.verk...@cisco.com; Zheng, Jian Xu ;
> tf...@chromium.org; Mani, Rajmohan ;
> Toivonen, Tuukka ; Yang, Hyungwoo
> ; Vijaykumar, Ramya
> ; Rapolu, Chiranjeevi
> 
> Subject: Re: [PATCH v5 3/3] intel-ipu3: cio2: Add new MIPI-CSI2 driver
> 
> Hi Yong,
> 
> Thanks for the update! Please see my comments below.
> 
> On Fri, Oct 06, 2017 at 06:39:01PM -0500, Yong Zhi wrote:
> > This patch adds CIO2 CSI-2 device driver for
> > Intel's IPU3 camera sub-system support.
> >
> > Signed-off-by: Yong Zhi 
> > Signed-off-by: Hyungwoo Yang 
> > Signed-off-by: Rajmohan Mani 
> > Signed-off-by: Vijaykumar Ramya 
> > ---
> >  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 |  452 +++
> >  7 files changed, 2533 insertions(+), 1 deletion(-)
> >  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
> >
> > diff --git a/drivers/media/pci/Kconfig b/drivers/media/pci/Kconfig
> > index da28e68c87d8..5932e225f9c0 100644
> > --- a/drivers/media/pci/Kconfig
> > +++ b/drivers/media/pci/Kconfig
> > @@ -54,5 +54,7 @@ source "drivers/media/pci/smipcie/Kconfig"
> >  source "drivers/media/pci/netup_unidvb/Kconfig"
> >  endif
> >
> > +source "drivers/media/pci/intel/ipu3/Kconfig"
> > +
> >  endif #MEDIA_PCI_SUPPORT
> >  endif #PCI
> > diff --git a/drivers/media/pci/Makefile b/drivers/media/pci/Makefile
> > index a7e8af0f64a7..d8f98434fb8c 100644
> > --- a/drivers/media/pci/Makefile
> > +++ b/drivers/media/pci/Makefile
> > @@ -13,7 +13,8 @@ obj-y+=   ttpci/  \
> > ddbridge/   \
> > saa7146/\
> > smipcie/\
> > -   netup_unidvb/
> > +   netup_unidvb/   \
> > +   intel/
> >
> >  obj-$(CONFIG_VIDEO_IVTV) += ivtv/
> >  obj-$(CONFIG_VIDEO_ZORAN) += zoran/
> > diff --git a/drivers/media/pci/intel/Makefile
> b/drivers/media/pci/intel/Makefile
> > new file mode 100644
> > index ..745c8b2a7819
> > --- /dev/null
> > +++ b/drivers/media/pci/intel/Makefile
> > @@ -0,0 +1,5 @@
> > +#
> > +# Makefile for the IPU3 cio2 and ImGU drivers
> > +#
> > +
> > +obj-y  += ipu3/
> > diff --git a/drivers/media/pci/intel/ipu3/Kconfig
> b/drivers/media/pci/intel/ipu3/Kconfig
> > new file mode 100644
> > index ..0861077a4dae
> > --- /dev/null
> > +++ b/drivers/media/pci/intel/ipu3/Kconfig
> > @@ -0,0 +1,19 @@
> > +config VIDEO_IPU3_CIO2
> > +   tristate "Intel ipu3-cio2 driver"
> > +   depends on VIDEO_V4L2 && PCI
> > +   depends on VIDEO_V4L2_SUBDEV_API
> > +   depends on MEDIA_CONTROLLER
> > +   depends on HAS_DMA
> > +   depends on ACPI
> > +   depends on X86_64 || COMPILE_TEST
> 
> Why X86_64?

We encountered some issue to build with 64bit previously, will fix in next 
update.
> 
> > +   select V4L2_FWNODE
> > +   select VIDEOBUF2_DMA_SG
> > +
> > +   ---help---
> > +   This is the Intel IPU3 CIO2 CSI-2 receiver unit, found in Intel
> > +   Skylake and Kaby Lake SoCs and used for capturing images and
> > +   video from a camera sensor.
> > +
> > +   Say Y or M here if you have a Skylake/Kaby Lake SoC with MIPI CSI-2
> > +   connected camera.
> > +   The module will be called ipu3-cio2.
> > diff --git a/drivers/media/pci/intel/ipu3/Makefile
> b/drivers/media/pci/intel/ipu3/Makefile
> > new file mode 100644
> > index ..20186e3ff2ae
> > --- /dev/null
> > +++ b/drivers/media/pci/intel/ipu3/Makefile
> > @@ -0,0 +1 @@
> > +obj-$(CONFIG_VIDEO_IPU3_CIO2) += ipu3-cio2.o
> > diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c
> b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
> > new file mode 100644
> > index ..2c2faa8f1d25
> > --- /dev/null
> > +++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
> > @@ -0,0 +1,2052 @@
> > +/*
> > + * Copyright (c) 2017 Intel Corporation.
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * 

MFC encoder: no dmabuf import

[Marian Mihailescu]

MFC encoder does not support dmabuf-import.
This is a problem when transcoding using MFC decoder, e.g.:

gst-launch-1.0 filesrc location=bunny_trailer_1080p.mov ! parsebin !
v4l2video4dec capture-io-mode=dmabuf ! v4l2video3h264enc
output-io-mode=dmabuf-import
extra-controls="encode,h264_level=10,h264_profile=4,frame_level_rate_control_enable=1,video_bitrate=800"
! h264parse ! matroskamux ! filesink location=transcoded.mkv

won't work.

Are there any plans for dmabuf import on MFC encoder?

---
Either I've been missing something or nothing has been going on. (K. E. Gordon)

Re: [PATCH v3 04/26] media: lirc_zilog: remove receiver

[Andy Walls]

Hi Sean and Devin:

On Wed, 2017-10-11 at 20:25 -0400, Devin Heitmueller wrote:
> > There's an ir_lock mutex in the driver to prevent simultaneous
> > access to the Rx and Tx functions of the z8.  Accessing Rx and Tx
> > functions of the chip together can cause it to do the wrong thing
> > (sometimes hang?), IIRC.
> > 
> > I'll see if I can dig up my old disassembly of the z8's firmware to
> > see if that interlock is strictly necessary.

Following up on this:

1. The I2C bus command and response buffers inside the Z8 appear to be
independent and non-overlapping (ignore the garbage assembly
mnemonics):

; I2C data buffer addresses
01bf srp   #%00   ; 01 00 ; Buffer pointer I2C addr E0 (70w) IR Tx  - 160 bytes
01c1 add   r0,@r0 ; 03 00 ; Buffer pointer I2C addr E1 (70r) IR Tx  -   4 bytes
01c3 add   r1,@r5 ; 03 15 ; Buffer pointer I2C addr E2 (71w) IR Rx  -   5 bytes
01c5 add   r1,@r0 ; 03 10 ; Buffer pointer I2C addr E3 (71r) IR Rx  -   5 bytes
01c7 add   r0,@r10; 03 0a ; Buffer pointer I2C addr E4 (72w) IR PB1 -   4 bytes
01c9 add   r1,@r10; 03 1a ; Buffer pointer I2C addr E5 (72r) IR PB1 -   4 bytes
01cb add   r0,@r5 ; 03 05 ; Buffer pointer I2C addr E6 (73w) IR Lrn -   1 bytes
01cd add   r0,r0  ; 02 00 ; Buffer pointer I2C addr E7 (73r) IR Lrn - 256 bytes

and the I2C handling routines appear to do the right thing in waiting
for "full" buffers before operating on them.

2. Z8 Port B is used by both Tx and Rx functions, but the functions
each only use 1 line of the I/O port and they use different lines.

3. Z8 Port C is unused.

4. Z8 Port A is used by both Tx functions, Rx functions, the I2C
interface.  If something inside the Z8 screws up the I2C bus comms with
the chip, it's likely the errant misconfiguration of Port A during
certain Rx and Tx operations.

5. Rx and IR Learn both use the same external hardware.  Not
coordinating Rx with Learn mode in the same driver, will prevent Learn
operation from working.  That is, if Learn mode is ever implemented. 
(Once upon a time, I was planning on doing that.  But I have no time
for that anymore.)  

> > Yes I know ir-kbd-i2c is in mainline, but as I said, I had reasons
> > for avoiding it when using Tx operation of the chip.  It's been 7
> > years, and I'm getting too old to remember the reasons. :P
> 
> Yeah, you definitely don't want to be issuing requests to the Rx and
> Tx addresses at the same time.  Very bad things will happen.
> 
> Also, what is the polling interval for ir-kbd-i2c?  If it's too high
> you can wedge the I2C bus (depending on the hardware design).
> Likewise, many people disable IR RX entirely (which is trivial to do
> with lirc_zilog through a modprobe optoin).  This is because early
> versions of the HDPVR had issues where polling too often can
> interfere
> with traffic that configures the component receiver chip.  This was
> improved in later versions of the design, but many people found it
> was
> just easiest to disable RX since they don't use it (i.e. they would
> use the blaster for controlling their STB but use a separate IR
> receiver).
> 
> Are you testing with video capture actively in use?  If you're
> testing
> the IR interface without an active HD capture in progress you're
> likely to miss some of these edge cases (in particular those which
> would hang the encoder).

I'm glad someone remembers all this stuff.  I'm assuming you had more
pain with this than I ever did.  I never owned an HD-PVR.

Regards,
Andy

> I'm not against the removal of duplicate code in general, but you
> have
> to tread carefully because there are plenty of non-obvious edge cases
> to consider.
> 
> Devin
> 

Exynos MFC issues on 4.14-rc4

[Marian Mihailescu]

I've been testing 4.14-rc4 on Odroid-XU4, and here's a kernel
complaint when running:

gst-launch-1.0 filesrc location=bunny_trailer_1080p.mov ! parsebin !
v4l2video4dec capture-io-mode=dmabuf ! v4l2video6convert
output-io-mode=dmabuf-import capture-io-mode=dmabuf ! kmssink

http://paste.debian.net/990353/

PS: on kernel 4.9 patched with MFC & GSC updates (almost up to date
with 4.14 I think) there was no "Wrong buffer/video queue type (1)"
message either


Either I've been missing something or nothing has been going on. (K. E. Gordon)

Re: [PATCH v3 04/26] media: lirc_zilog: remove receiver

[Devin Heitmueller]

> There's an ir_lock mutex in the driver to prevent simultaneous access to the 
> Rx and Tx functions of the z8.  Accessing Rx and Tx functions of the chip 
> together can cause it to do the wrong thing (sometimes hang?), IIRC.
>
> I'll see if I can dig up my old disassembly of the z8's firmware to see if 
> that interlock is strictly necessary.
>
> Yes I know ir-kbd-i2c is in mainline, but as I said, I had reasons for 
> avoiding it when using Tx operation of the chip.  It's been 7 years, and I'm 
> getting too old to remember the reasons. :P

Yeah, you definitely don't want to be issuing requests to the Rx and
Tx addresses at the same time.  Very bad things will happen.

Also, what is the polling interval for ir-kbd-i2c?  If it's too high
you can wedge the I2C bus (depending on the hardware design).
Likewise, many people disable IR RX entirely (which is trivial to do
with lirc_zilog through a modprobe optoin).  This is because early
versions of the HDPVR had issues where polling too often can interfere
with traffic that configures the component receiver chip.  This was
improved in later versions of the design, but many people found it was
just easiest to disable RX since they don't use it (i.e. they would
use the blaster for controlling their STB but use a separate IR
receiver).

Are you testing with video capture actively in use?  If you're testing
the IR interface without an active HD capture in progress you're
likely to miss some of these edge cases (in particular those which
would hang the encoder).

I'm not against the removal of duplicate code in general, but you have
to tread carefully because there are plenty of non-obvious edge cases
to consider.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

Re: [PATCH] media: staging/imx: do not return error in link_notify for unknown sources

[Russell King - ARM Linux]

On Thu, Oct 12, 2017 at 12:06:33AM +0100, Russell King - ARM Linux wrote:
> Now, if you mean "known" to be equivalent with "recognised by
> imx-media" then, as I've pointed out several times already, that
> statement is FALSE.  I'm not sure how many times I'm going to have to
> state this fact.  Let me re-iterate again.  On my imx219 driver, I
> have two subdevs.  Both subdevs have controls attached.  The pixel
> subdev is not "recognised" by imx-media, and without a modification
> like my or your patch, it fails.  However, with the modification,
> this "unrecognised" subdev _STILL_ has it's controls visible through
> imx-media.

If you refuse to believe what I'm saying, then read through:

http://git.armlinux.org.uk/cgit/linux-arm.git/commit/?h=csi-v6=e3f847cd37b007d55b76282414bfcf13abb8fc9a

and look at this:

# for f in 2 3 4 5 6 7 8 9; do v4l2-ctl -L -d /dev/video$f; done
# ./cam/cam-setup.sh
# v4l2-ctl -L -d /dev/video6

User Controls

   gain (int): min=256 max=4095 step=1 default=256 
value=256
 fim_enable (bool)   : default=0 value=0
fim_num_average (int): min=1 max=64 step=1 default=8 
value=8  fim_tolerance_min (int): min=2 max=200 step=1 
default=50 value=50
  fim_tolerance_max (int): min=0 max=500 step=1 default=0 
value=0
   fim_num_skip (int): min=0 max=256 step=1 default=2 
value=2
 fim_input_capture_edge (int): min=0 max=3 step=1 default=0 value=0
  fim_input_capture_channel (int): min=0 max=1 step=1 default=0 value=0

Camera Controls

 exposure_time_absolute (int): min=1 max=399 step=1 default=399 
value=166

Image Source Controls

  vertical_blanking (int): min=32 max=64814 step=1 default=2509
value=2509 flags=read-only
horizontal_blanking (int): min=2168 max=31472 step=1 
default=2168 value=2168 flags=read-only
  analogue_gain (int): min=1000 max=8000 step=1 
default=1000 value=1000
red_pixel_value (int): min=0 max=1023 step=1 default=0 
value=0 flags=inactive
  green_red_pixel_value (int): min=0 max=1023 step=1 default=0 
value=0 flags=inactive
   blue_pixel_value (int): min=0 max=1023 step=1 default=0 
value=0 flags=inactive
 green_blue_pixel_value (int): min=0 max=1023 step=1 default=0 
value=0 flags=inactive

Image Processing Controls

 pixel_rate (int64)  : min=16000 max=27840 step=1 
default=27840 value=27840 flags=read-only
   test_pattern (menu)   : min=0 max=9 default=0 value=0
0: Disabled
1: Solid Color
2: 100% Color Bars
3: Fade to Grey Color Bar
4: PN9
5: 16 Split Color Bar
6: 16 Split Inverted Color Bar
7: Column Counter
8: Inverted Column Counter
9: PN31

Now, the pixel array subdev has these controls:
gain
vertical_blanking
horizontal_blanking
analogue_gain
pixel_rate
exposure_time_absolute

The root subdev (which is the one which connects to your code) has
these controls:
red_pixel_value
green_red_pixel_value
blue_pixel_value
green_blue_pixel_value
test_pattern

As you can see from the above output, _all_ controls from _both_ subdevs
are listed.

Now, before you spot your old patch adding v4l2_pipeline_inherit_controls()
and try to blame that for this, nothing in my kernel calls that function,
so that patch can be dropped, and so it's not responsible for this.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

Re: [PATCH v3 04/26] media: lirc_zilog: remove receiver

[Andy Walls]

On October 11, 2017 5:02:37 PM EDT, Sean Young  wrote:
>On Wed, Oct 11, 2017 at 03:43:16PM -0400, Andy Walls wrote:
>> On Tue, 2017-10-10 at 08:17 +0100, Sean Young wrote:
>> > The ir-kbd-i2c module already handles this very well.
>> 
>> Hi Sean:
>> 
>> It's been years, but my recollection is that although ir-kdb-i2c
>might
>> handle receive well, but since the 4 i2c addresses (1 Rx, 1 Tx, 1 IR
>Tx
>> code learning, 1 custom Tx code) are all handled by the same Zilog
>> microcontroller internally, that splitting the Rx and Tx
>functionality
>> between two modules became problematic.
>> 
>> Believe me, if i could have leveraged ir-kdb-i2c back when I ported
>> this, I would have.  I think it's a very bad idea to remove the
>> receiver from lirc_zilog.
>> 
>> The cx18 and ivtv drivers both use lirc_zilog IIRC.  Have you looked
>at
>> the changes required to have the first I2C address used by one i2c
>> module, and the next one (or three) I2C addresses used by another i2c
>> module?
>
>This is already the case.
>
>In current kernels you can use ir-kbd-i2c for Rx and lirc_zilog for Tx,
>this works fine. In fact, the lirc_zilog Rx code produces lirccodes,
>(not mode2) and ir-kbd-i2c produces keycodes through rc-core, so
>ir-kbd-i2c
>(also mainline, not staging) is much more likely to be used for Rx. For
>Tx
>you have to use lirc_zilog.
>
>I haven't heard of any reports of problems (or observed this myself)
>when
>using these two modules together.
>
>As you point out, both drivers sending i2c commands to the same z8f0811
>
>with its z8 encore hand-coded i2c implementation. But they're using
>different
>addresses.
>
>So removing the lirc_zilog Rx doesn't make things worse, in fact, it
>just
>removes some code which is unlikely to be used.
>
>It's the hdpvr, ivtv, cx18 and pvrusb2 drivers which have a Tx
>interface. I
>have hdpvr and ivtv hardware to test.
>
>
>Sean
>
>> 
>> -Andy
>> 
>> 
>> > 
>> > Signed-off-by: Sean Young 
>> > ---
>> >  drivers/staging/media/lirc/lirc_zilog.c | 901
>+++---
>> > --
>> >  1 file changed, 76 insertions(+), 825 deletions(-)
>> > 
>> > diff --git a/drivers/staging/media/lirc/lirc_zilog.c
>> > b/drivers/staging/media/lirc/lirc_zilog.c
>> > index 6bd0717bf76e..757b3fc247ac 100644
>> > --- a/drivers/staging/media/lirc/lirc_zilog.c
>> > +++ b/drivers/staging/media/lirc/lirc_zilog.c
>> > @@ -64,28 +64,7 @@
>> >  /* Max transfer size done by I2C transfer functions */
>> >  #define MAX_XFER_SIZE  64
>> >  
>> > -struct IR;
>> > -
>> > -struct IR_rx {
>> > -  struct kref ref;
>> > -  struct IR *ir;
>> > -
>> > -  /* RX device */
>> > -  struct mutex client_lock;
>> > -  struct i2c_client *c;
>> > -
>> > -  /* RX polling thread data */
>> > -  struct task_struct *task;
>> > -
>> > -  /* RX read data */
>> > -  unsigned char b[3];
>> > -  bool hdpvr_data_fmt;
>> > -};
>> > -
>> >  struct IR_tx {
>> > -  struct kref ref;
>> > -  struct IR *ir;
>> > -
>> >/* TX device */
>> >struct mutex client_lock;
>> >struct i2c_client *c;
>> > @@ -93,39 +72,9 @@ struct IR_tx {
>> >/* TX additional actions needed */
>> >int need_boot;
>> >bool post_tx_ready_poll;
>> > -};
>> > -
>> > -struct IR {
>> > -  struct kref ref;
>> > -  struct list_head list;
>> > -
>> > -  /* FIXME spinlock access to l->features */
>> >struct lirc_dev *l;
>> > -  struct lirc_buffer rbuf;
>> > -
>> > -  struct mutex ir_lock;
>> > -  atomic_t open_count;
>> > -
>> > -  struct device *dev;
>> > -  struct i2c_adapter *adapter;
>> > -
>> > -  spinlock_t rx_ref_lock; /* struct IR_rx kref get()/put() */
>> > -  struct IR_rx *rx;
>> > -
>> > -  spinlock_t tx_ref_lock; /* struct IR_tx kref get()/put() */
>> > -  struct IR_tx *tx;
>> >  };
>> >  
>> > -/* IR transceiver instance object list */
>> > -/*
>> > - * This lock is used for the following:
>> > - * a. ir_devices_list access, insertions, deletions
>> > - * b. struct IR kref get()s and put()s
>> > - * c. serialization of ir_probe() for the two i2c_clients for a Z8
>> > - */
>> > -static DEFINE_MUTEX(ir_devices_lock);
>> > -static LIST_HEAD(ir_devices_list);
>> > -
>> >  /* Block size for IR transmitter */
>> >  #define TX_BLOCK_SIZE 99
>> >  
>> > @@ -153,348 +102,8 @@ struct tx_data_struct {
>> >  static struct tx_data_struct *tx_data;
>> >  static struct mutex tx_data_lock;
>> >  
>> > -
>> >  /* module parameters */
>> >  static bool debug;/* debug output */
>> > -static bool tx_only;  /* only handle the IR Tx function */
>> > -
>> > -
>> > -/* struct IR reference counting */
>> > -static struct IR *get_ir_device(struct IR *ir, bool
>> > ir_devices_lock_held)
>> > -{
>> > -  if (ir_devices_lock_held) {
>> > -  kref_get(>ref);
>> > -  } else {
>> > -  mutex_lock(_devices_lock);
>> > -  kref_get(>ref);
>> > -  mutex_unlock(_devices_lock);
>> > -  }
>> > -  return ir;
>> > -}
>> > -
>> > -static void release_ir_device(struct kref *ref)
>> > -{
>> > -  

Re: [PATCH] media: staging/imx: do not return error in link_notify for unknown sources

[Steve Longerbeam]

On 10/11/2017 04:06 PM, Russell King - ARM Linux wrote:

On Wed, Oct 11, 2017 at 03:14:26PM -0700, Steve Longerbeam wrote:


On 10/11/2017 02:49 PM, Russell King - ARM Linux wrote:

On Tue, Oct 03, 2017 at 12:09:13PM -0700, Steve Longerbeam wrote:

imx_media_link_notify() should not return error if the source subdevice
is not recognized by imx-media, that isn't an error. If the subdev has
controls they will be inherited starting from a known subdev.

What does "a known subdev" mean?

It refers to the previous sentence, "not recognized by imx-media". A
subdev that was not registered via async registration and so not in
imx-media's async subdev list. I could elaborate in the commit message
but it seems fairly obvious to me.

I don't think it's obvious, and I suspect you won't think it's obvious
in years to come (I talk from experience of some commentry I've added
in the past.)

Now, isn't it true that for a subdev to be part of a media device, it
has to be registered, and if it's part of a media device that is made
up of lots of different drivers, it has to use the async registration
code?  So, is it not also true that any subdev that is part of a
media device, it will be "known"?

Under what circumstances could a subdev be part of a media device but
not be "known" ?

Now, if you mean "known" to be equivalent with "recognised by
imx-media" then, as I've pointed out several times already, that
statement is FALSE.  I'm not sure how many times I'm going to have to
state this fact.  Let me re-iterate again.  On my imx219 driver, I
have two subdevs.  Both subdevs have controls attached.  The pixel
subdev is not "recognised" by imx-media, and without a modification
like my or your patch, it fails.  However, with the modification,
this "unrecognised" subdev _STILL_ has it's controls visible through
imx-media.


Well that's true, the controls for your pixel subdev (which was
not registered via async) still are visible to imx-media, so in that
sense the subdev is "known" to imx-media.



Hence, I believe your comment in the code and your commit message
are misleading and wrong.


Ok you convinced me, I'll fix the code comment and commit
message.

Steve

Re: [PATCH] media: staging/imx: do not return error in link_notify for unknown sources

[Russell King - ARM Linux]

On Wed, Oct 11, 2017 at 03:14:26PM -0700, Steve Longerbeam wrote:
> 
> 
> On 10/11/2017 02:49 PM, Russell King - ARM Linux wrote:
> >On Tue, Oct 03, 2017 at 12:09:13PM -0700, Steve Longerbeam wrote:
> >>imx_media_link_notify() should not return error if the source subdevice
> >>is not recognized by imx-media, that isn't an error. If the subdev has
> >>controls they will be inherited starting from a known subdev.
> >What does "a known subdev" mean?
> 
> It refers to the previous sentence, "not recognized by imx-media". A
> subdev that was not registered via async registration and so not in
> imx-media's async subdev list. I could elaborate in the commit message
> but it seems fairly obvious to me.

I don't think it's obvious, and I suspect you won't think it's obvious
in years to come (I talk from experience of some commentry I've added
in the past.)

Now, isn't it true that for a subdev to be part of a media device, it
has to be registered, and if it's part of a media device that is made
up of lots of different drivers, it has to use the async registration
code?  So, is it not also true that any subdev that is part of a
media device, it will be "known"?

Under what circumstances could a subdev be part of a media device but
not be "known" ?

Now, if you mean "known" to be equivalent with "recognised by
imx-media" then, as I've pointed out several times already, that
statement is FALSE.  I'm not sure how many times I'm going to have to
state this fact.  Let me re-iterate again.  On my imx219 driver, I
have two subdevs.  Both subdevs have controls attached.  The pixel
subdev is not "recognised" by imx-media, and without a modification
like my or your patch, it fails.  However, with the modification,
this "unrecognised" subdev _STILL_ has it's controls visible through
imx-media.

Hence, I believe your comment in the code and your commit message
are misleading and wrong.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

Re: [PATCH] build: Update backports/v2.6.37_dont_use_alloc_ordered_workqueue.patch

[Jasmin J.]

Hi!

Even with this patch Kernel 2.6.x can't be compiled due to "linux/bsearch.h"
missing. But this patch is the first step in the right direction.

BR,
   Jasmin

[PATCH] build: Update backports/v2.6.37_dont_use_alloc_ordered_workqueue.patch

[Jasmin J.]

From: Jasmin Jessich 

Signed-off-by: Jasmin Jessich 
---
 backports/v2.6.37_dont_use_alloc_ordered_workqueue.patch | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/backports/v2.6.37_dont_use_alloc_ordered_workqueue.patch 
b/backports/v2.6.37_dont_use_alloc_ordered_workqueue.patch
index 89f508f..19d45f3 100644
--- a/backports/v2.6.37_dont_use_alloc_ordered_workqueue.patch
+++ b/backports/v2.6.37_dont_use_alloc_ordered_workqueue.patch
@@ -2,13 +2,13 @@ diff --git a/drivers/media/pci/cx18/cx18-driver.c 
b/drivers/media/pci/cx18/cx18-
 index 004d8ac..6508785 100644
 --- a/drivers/media/pci/cx18/cx18-driver.c
 +++ b/drivers/media/pci/cx18/cx18-driver.c
-@@ -695,7 +695,7 @@ static int cx18_create_in_workq(struct cx18 *cx)
+@@ -697,7 +697,7 @@ static int cx18_create_in_workq(struct cx18 *cx)
  {
snprintf(cx->in_workq_name, sizeof(cx->in_workq_name), "%s-in",
 cx->v4l2_dev.name);
 -  cx->in_work_queue = alloc_ordered_workqueue(cx->in_workq_name, 0);
 +  cx->in_work_queue = create_singlethread_workqueue(cx->in_workq_name);
-   if (cx->in_work_queue == NULL) {
+   if (!cx->in_work_queue) {
CX18_ERR("Unable to create incoming mailbox handler thread\n");
return -ENOMEM;
 @@ -703,6 +703,18 @@ static int cx18_create_in_workq(struct cx18 *cx)
-- 
2.7.4

Re: [PATCH v3 1/2] staging: Introduce NVIDIA Tegra20 video decoder driver

[Dmitry Osipenko]

On 11.10.2017 23:47, Nicolas Dufresne wrote:
> Le mercredi 11 octobre 2017 à 23:08 +0300, Dmitry Osipenko a écrit :
>> diff --git a/drivers/staging/tegra-vde/TODO b/drivers/staging/tegra-
>> vde/TODO
>> new file mode 100644
>> index ..e98bbc7b3c19
>> --- /dev/null
>> +++ b/drivers/staging/tegra-vde/TODO
>> @@ -0,0 +1,5 @@
>> +TODO:
>> +   - Figure out how generic V4L2 API could be utilized by this
>> driver,
>> + implement it.
>> +
> 
> That is a very interesting effort, I think it's the first time someone
> is proposing an upstream driver for a Tegra platform.

Thanks!

 When I look
> tegra_vde_h264_decoder_ctx, it looks like the only thing that the HW is
> not parsing is the media header (pps/sps). Is that correct ?
> 

That's correct. I think it's quite common among embedded (mobile) and
desktop-grade decoders to require some auxiliary info from the media headers.

> I wonder how acceptable it would be to parse this inside the driver. It
> is no more complex then parsing an EDID. If that was possible, wrapping
> this driver as a v4l2 mem2mem should be rather simple. As a side
> effect, you'll automatically get some userspace working, notably
> GStreamer and FFmpeg.
> 

Parsing bitstream in kernel feels a bit dirty, although it's up to media
maintainers to decide.

> For the case even parsing the headers is too much from a kernel point
> of view, then I think you should have a look at the following effort.
> It's a proposal base on yet to be merged Request API. Hugues is also
> propose a libv4l2 adapter that makes the driver looks like a normal
> v4l2 m2m, hiding all the userspace parsing and table filling. This
> though, is long term plan to integrate state-less or parser-less
> encoders into linux-media. It seems rather overkill for state-full
> driver that requires parsed headers like PPS/SPS.
> 
> https://lwn.net/Articles/720797/
> 

I'll take a look at the Request API / libv4l2 adapter, thank you very much for
pointing to it.

Re: [PATCH] media: staging/imx: do not return error in link_notify for unknown sources

[Steve Longerbeam]

On 10/11/2017 02:49 PM, Russell King - ARM Linux wrote:

On Tue, Oct 03, 2017 at 12:09:13PM -0700, Steve Longerbeam wrote:

imx_media_link_notify() should not return error if the source subdevice
is not recognized by imx-media, that isn't an error. If the subdev has
controls they will be inherited starting from a known subdev.

What does "a known subdev" mean?


It refers to the previous sentence, "not recognized by imx-media". A
subdev that was not registered via async registration and so not in
imx-media's async subdev list. I could elaborate in the commit message
but it seems fairly obvious to me.

Steve

[PATCH] build: Add U32_MAX if not defined

[Jasmin J.]

From: Jasmin Jessich 

Compiling for Kernel 3.4 failed in "ir-lirc-codec.c" with
  error: 'U32_MAX' undeclared

Signed-off-by: Jasmin Jessich 
---
 v4l/compat.h  | 4 
 v4l/scripts/make_config_compat.pl | 1 +
 2 files changed, 5 insertions(+)

diff --git a/v4l/compat.h b/v4l/compat.h
index a20264c..15f2cd6 100644
--- a/v4l/compat.h
+++ b/v4l/compat.h
@@ -2166,4 +2166,8 @@ static inline unsigned long nsecs_to_jiffies_static(u64 n)
 
 #endif
 
+#ifdef NEED_U32_MAX
+#define U32_MAX ((u32)~0U)
+#endif
+
 #endif /*  _COMPAT_H */
diff --git a/v4l/scripts/make_config_compat.pl 
b/v4l/scripts/make_config_compat.pl
index 53ca439..9fdf10d 100644
--- a/v4l/scripts/make_config_compat.pl
+++ b/v4l/scripts/make_config_compat.pl
@@ -704,6 +704,7 @@ sub check_other_dependencies()
check_files_for_func("KEY_APPSELECT", "NEED_KEY_APPSELECT", 
"include/uapi/linux/input-event-codes.h");
check_files_for_func("PCI_DEVICE_SUB", "NEED_PCI_DEVICE_SUB", 
"include/linux/pci.h");
check_files_for_func("annotate_reachable", "NEED_ANNOTATE_REACHABLE", 
"include/linux/compiler.h");
+   check_files_for_func("U32_MAX", "NEED_U32_MAX", 
"include/linux/kernel.h");
 
# For tests for uapi-dependent logic
check_files_for_func_uapi("usb_endpoint_maxp", 
"NEED_USB_ENDPOINT_MAXP", "usb/ch9.h");
-- 
2.7.4

Re: [PATCH] media: staging/imx: do not return error in link_notify for unknown sources

[Russell King - ARM Linux]

On Tue, Oct 03, 2017 at 12:09:13PM -0700, Steve Longerbeam wrote:
> imx_media_link_notify() should not return error if the source subdevice
> is not recognized by imx-media, that isn't an error. If the subdev has
> controls they will be inherited starting from a known subdev.

What does "a known subdev" mean?

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

Re: [PATCH 04/24] media: v4l2-mediabus: convert flags to enums and document them

[Pavel Machek]

On Mon 2017-10-09 07:19:10, Mauro Carvalho Chehab wrote:
> There is a mess with media bus flags: there are two sets of
> flags, one used by parallel and ITU-R BT.656 outputs,
> and another one for CSI2.
> 
> Depending on the type, the same bit has different meanings.
> 

> @@ -86,11 +125,22 @@ enum v4l2_mbus_type {
>  /**
>   * struct v4l2_mbus_config - media bus configuration
>   * @type:in: interface type
> - * @flags:   in / out: configuration flags, depending on @type
> + * @pb_flags:in / out: configuration flags, if @type is
> + *   %V4L2_MBUS_PARALLEL or %V4L2_MBUS_BT656.
> + * @csi2_flags:  in / out: configuration flags, if @type is
> + *   %V4L2_MBUS_CSI2.
> + * @flag:access flags, no matter the @type.
> + *   Used just to avoid needing to rewrite the logic inside
> + *   soc_camera and pxa_camera drivers. Don't use on newer
> + *   drivers!
>   */
>  struct v4l2_mbus_config {
>   enum v4l2_mbus_type type;
> - unsigned int flags;
> + union {
> + enum v4l2_mbus_parallel_and_bt656_flags pb_flags;
> + enum v4l2_mbus_csi2_flags   csi2_flags;
> + unsigned intflag;
> + };
>  };
>  
>  static inline void v4l2_fill_pix_format(struct v4l2_pix_format
>  *pix_fmt,

The flags->flag conversion is quite subtle, and "flag" is confusing
because there is more than one inside. What about something like
__legacy_flags?

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH v3 04/26] media: lirc_zilog: remove receiver

[Sean Young]

On Wed, Oct 11, 2017 at 03:43:16PM -0400, Andy Walls wrote:
> On Tue, 2017-10-10 at 08:17 +0100, Sean Young wrote:
> > The ir-kbd-i2c module already handles this very well.
> 
> Hi Sean:
> 
> It's been years, but my recollection is that although ir-kdb-i2c might
> handle receive well, but since the 4 i2c addresses (1 Rx, 1 Tx, 1 IR Tx
> code learning, 1 custom Tx code) are all handled by the same Zilog
> microcontroller internally, that splitting the Rx and Tx functionality
> between two modules became problematic.
> 
> Believe me, if i could have leveraged ir-kdb-i2c back when I ported
> this, I would have.  I think it's a very bad idea to remove the
> receiver from lirc_zilog.
> 
> The cx18 and ivtv drivers both use lirc_zilog IIRC.  Have you looked at
> the changes required to have the first I2C address used by one i2c
> module, and the next one (or three) I2C addresses used by another i2c
> module?

This is already the case.

In current kernels you can use ir-kbd-i2c for Rx and lirc_zilog for Tx,
this works fine. In fact, the lirc_zilog Rx code produces lirccodes,
(not mode2) and ir-kbd-i2c produces keycodes through rc-core, so ir-kbd-i2c
(also mainline, not staging) is much more likely to be used for Rx. For Tx
you have to use lirc_zilog.

I haven't heard of any reports of problems (or observed this myself) when
using these two modules together.

As you point out, both drivers sending i2c commands to the same z8f0811 
with its z8 encore hand-coded i2c implementation. But they're using different
addresses.

So removing the lirc_zilog Rx doesn't make things worse, in fact, it just
removes some code which is unlikely to be used.

It's the hdpvr, ivtv, cx18 and pvrusb2 drivers which have a Tx interface. I
have hdpvr and ivtv hardware to test.


Sean

> 
> -Andy
> 
> 
> > 
> > Signed-off-by: Sean Young 
> > ---
> >  drivers/staging/media/lirc/lirc_zilog.c | 901 +++---
> > --
> >  1 file changed, 76 insertions(+), 825 deletions(-)
> > 
> > diff --git a/drivers/staging/media/lirc/lirc_zilog.c
> > b/drivers/staging/media/lirc/lirc_zilog.c
> > index 6bd0717bf76e..757b3fc247ac 100644
> > --- a/drivers/staging/media/lirc/lirc_zilog.c
> > +++ b/drivers/staging/media/lirc/lirc_zilog.c
> > @@ -64,28 +64,7 @@
> >  /* Max transfer size done by I2C transfer functions */
> >  #define MAX_XFER_SIZE  64
> >  
> > -struct IR;
> > -
> > -struct IR_rx {
> > -   struct kref ref;
> > -   struct IR *ir;
> > -
> > -   /* RX device */
> > -   struct mutex client_lock;
> > -   struct i2c_client *c;
> > -
> > -   /* RX polling thread data */
> > -   struct task_struct *task;
> > -
> > -   /* RX read data */
> > -   unsigned char b[3];
> > -   bool hdpvr_data_fmt;
> > -};
> > -
> >  struct IR_tx {
> > -   struct kref ref;
> > -   struct IR *ir;
> > -
> > /* TX device */
> > struct mutex client_lock;
> > struct i2c_client *c;
> > @@ -93,39 +72,9 @@ struct IR_tx {
> > /* TX additional actions needed */
> > int need_boot;
> > bool post_tx_ready_poll;
> > -};
> > -
> > -struct IR {
> > -   struct kref ref;
> > -   struct list_head list;
> > -
> > -   /* FIXME spinlock access to l->features */
> > struct lirc_dev *l;
> > -   struct lirc_buffer rbuf;
> > -
> > -   struct mutex ir_lock;
> > -   atomic_t open_count;
> > -
> > -   struct device *dev;
> > -   struct i2c_adapter *adapter;
> > -
> > -   spinlock_t rx_ref_lock; /* struct IR_rx kref get()/put() */
> > -   struct IR_rx *rx;
> > -
> > -   spinlock_t tx_ref_lock; /* struct IR_tx kref get()/put() */
> > -   struct IR_tx *tx;
> >  };
> >  
> > -/* IR transceiver instance object list */
> > -/*
> > - * This lock is used for the following:
> > - * a. ir_devices_list access, insertions, deletions
> > - * b. struct IR kref get()s and put()s
> > - * c. serialization of ir_probe() for the two i2c_clients for a Z8
> > - */
> > -static DEFINE_MUTEX(ir_devices_lock);
> > -static LIST_HEAD(ir_devices_list);
> > -
> >  /* Block size for IR transmitter */
> >  #define TX_BLOCK_SIZE  99
> >  
> > @@ -153,348 +102,8 @@ struct tx_data_struct {
> >  static struct tx_data_struct *tx_data;
> >  static struct mutex tx_data_lock;
> >  
> > -
> >  /* module parameters */
> >  static bool debug; /* debug output */
> > -static bool tx_only;   /* only handle the IR Tx function */
> > -
> > -
> > -/* struct IR reference counting */
> > -static struct IR *get_ir_device(struct IR *ir, bool
> > ir_devices_lock_held)
> > -{
> > -   if (ir_devices_lock_held) {
> > -   kref_get(>ref);
> > -   } else {
> > -   mutex_lock(_devices_lock);
> > -   kref_get(>ref);
> > -   mutex_unlock(_devices_lock);
> > -   }
> > -   return ir;
> > -}
> > -
> > -static void release_ir_device(struct kref *ref)
> > -{
> > -   struct IR *ir = container_of(ref, struct IR, ref);
> > -
> > -   /*
> > -* Things should be in this state by now:
> > -* ir->rx set to NULL and deallocated - happens before ir-
> > >rx->ir put()
> > - 

Re: [PATCH v3 1/2] staging: Introduce NVIDIA Tegra20 video decoder driver

[Nicolas Dufresne]

Le mercredi 11 octobre 2017 à 23:08 +0300, Dmitry Osipenko a écrit :
> diff --git a/drivers/staging/tegra-vde/TODO b/drivers/staging/tegra-
> vde/TODO
> new file mode 100644
> index ..e98bbc7b3c19
> --- /dev/null
> +++ b/drivers/staging/tegra-vde/TODO
> @@ -0,0 +1,5 @@
> +TODO:
> +   - Figure out how generic V4L2 API could be utilized by this
> driver,
> + implement it.
> +

That is a very interesting effort, I think it's the first time someone
is proposing an upstream driver for a Tegra platform. When I look
tegra_vde_h264_decoder_ctx, it looks like the only thing that the HW is
not parsing is the media header (pps/sps). Is that correct ?

I wonder how acceptable it would be to parse this inside the driver. It
is no more complex then parsing an EDID. If that was possible, wrapping
this driver as a v4l2 mem2mem should be rather simple. As a side
effect, you'll automatically get some userspace working, notably
GStreamer and FFmpeg.

For the case even parsing the headers is too much from a kernel point
of view, then I think you should have a look at the following effort.
It's a proposal base on yet to be merged Request API. Hugues is also
propose a libv4l2 adapter that makes the driver looks like a normal
v4l2 m2m, hiding all the userspace parsing and table filling. This
though, is long term plan to integrate state-less or parser-less
encoders into linux-media. It seems rather overkill for state-full
driver that requires parsed headers like PPS/SPS.

https://lwn.net/Articles/720797/

regards,
Nicolas

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

[PATCH v3 1/2] staging: Introduce NVIDIA Tegra20 video decoder driver

[Dmitry Osipenko]

Video decoder, found on NVIDIA Tegra20 SoC, supports a standard set of
video formats like H.264 / MPEG-4 / WMV / VC1. Currently driver supports
decoding of CAVLC H.264 only.

Signed-off-by: Dmitry Osipenko 
---
 .../bindings/arm/tegra/nvidia,tegra20-vde.txt  |   44 +
 drivers/staging/Kconfig|2 +
 drivers/staging/Makefile   |1 +
 drivers/staging/tegra-vde/Kconfig  |6 +
 drivers/staging/tegra-vde/Makefile |1 +
 drivers/staging/tegra-vde/TODO |5 +
 drivers/staging/tegra-vde/uapi.h   |  101 ++
 drivers/staging/tegra-vde/vde.c| 1109 
 8 files changed, 1269 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt
 create mode 100644 drivers/staging/tegra-vde/Kconfig
 create mode 100644 drivers/staging/tegra-vde/Makefile
 create mode 100644 drivers/staging/tegra-vde/TODO
 create mode 100644 drivers/staging/tegra-vde/uapi.h
 create mode 100644 drivers/staging/tegra-vde/vde.c

diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt 
b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt
new file mode 100644
index ..c3f847db8167
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt
@@ -0,0 +1,44 @@
+NVIDIA Tegra Video Decoder Engine
+
+Required properties:
+- compatible : "nvidia,tegra20-vde"
+- reg : Must contain 2 register ranges: registers and IRAM region that
+VDE uses for its internal needs and for passing some of decoding
+parameters.
+- reg-names : Must include the following entries:
+  - regs
+  - iram
+- interrupts : Must contain an entry for each entry in interrupt-names.
+- interrupt-names : Must include the following entries:
+  - ucq-error
+  - sync-token
+  - bsev
+  - bsea
+  - sxe
+- clocks : Must contain an entry for each entry in clock-names.
+  See ../clocks/clock-bindings.txt for details.
+- clock-names : Must include the following entries:
+  - vde
+- resets : Must contain an entry for each entry in reset-names.
+  See ../reset/reset.txt for details.
+- reset-names : Must include the following entries:
+  - vde
+
+Example:
+
+vde@6001a000 {
+   compatible = "nvidia,tegra20-vde";
+   reg = <0x6001a000 0x3D00/* VDE registers */
+   0x4400 0x3FC00>; /* IRAM region */
+   reg-names = "regs", "iram";
+   interrupts = , /* UCQ error interrupt */
+   , /* Sync token 
interrupt */
+   , /* BSE-V interrupt */
+   , /* BSE-A interrupt */
+   ; /* SXE interrupt */
+   interrupt-names = "ucq-error", "sync-token", "bsev", "bsea", "sxe";
+   clocks = <_car TEGRA20_CLK_VDE>;
+   clock-names = "vde";
+   resets = <_car 61>;
+   reset-names = "vde";
+};
diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig
index 554683912cff..10c982811093 100644
--- a/drivers/staging/Kconfig
+++ b/drivers/staging/Kconfig
@@ -118,4 +118,6 @@ source "drivers/staging/vboxvideo/Kconfig"
 
 source "drivers/staging/pi433/Kconfig"
 
+source "drivers/staging/tegra-vde/Kconfig"
+
 endif # STAGING
diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
index 8951c37d8d80..c5ef39767f22 100644
--- a/drivers/staging/Makefile
+++ b/drivers/staging/Makefile
@@ -49,3 +49,4 @@ obj-$(CONFIG_BCM2835_VCHIQ)   += vc04_services/
 obj-$(CONFIG_CRYPTO_DEV_CCREE) += ccree/
 obj-$(CONFIG_DRM_VBOXVIDEO)+= vboxvideo/
 obj-$(CONFIG_PI433)+= pi433/
+obj-$(CONFIG_TEGRA_VDE)+= tegra-vde/
diff --git a/drivers/staging/tegra-vde/Kconfig 
b/drivers/staging/tegra-vde/Kconfig
new file mode 100644
index ..730ee006de66
--- /dev/null
+++ b/drivers/staging/tegra-vde/Kconfig
@@ -0,0 +1,6 @@
+config TEGRA_VDE
+   tristate "NVIDIA Tegra Video Decoder Engine driver"
+   depends on ARCH_TEGRA_2x_SOC || COMPILE_TEST
+   help
+   Say Y here to enable support for the NVIDIA Tegra video decoder
+   driver.
diff --git a/drivers/staging/tegra-vde/Makefile 
b/drivers/staging/tegra-vde/Makefile
new file mode 100644
index ..e7c0df1174bf
--- /dev/null
+++ b/drivers/staging/tegra-vde/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_TEGRA_VDE)+= vde.o
diff --git a/drivers/staging/tegra-vde/TODO b/drivers/staging/tegra-vde/TODO
new file mode 100644
index ..e98bbc7b3c19
--- /dev/null
+++ b/drivers/staging/tegra-vde/TODO
@@ -0,0 +1,5 @@
+TODO:
+   - Figure out how generic V4L2 API could be utilized by this driver,
+ implement it.
+
+Contact: Dmitry Osipenko 
diff --git a/drivers/staging/tegra-vde/uapi.h b/drivers/staging/tegra-vde/uapi.h
new file mode 100644
index ..36d76278d27c
--- /dev/null
+++ b/drivers/staging/tegra-vde/uapi.h
@@ -0,0 +1,101 @@
+/*
+ * Copyright 

[PATCH v3 2/2] ARM: dts: tegra20: Add video decoder node

[Dmitry Osipenko]

Add a device node for the video decoder engine found on Tegra20.

Signed-off-by: Dmitry Osipenko 
---
 arch/arm/boot/dts/tegra20.dtsi | 17 +
 1 file changed, 17 insertions(+)

diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
index 7c85f97f72ea..1b5d54b6c0cb 100644
--- a/arch/arm/boot/dts/tegra20.dtsi
+++ b/arch/arm/boot/dts/tegra20.dtsi
@@ -249,6 +249,23 @@
*/
};
 
+   vde@6001a000 {
+   compatible = "nvidia,tegra20-vde";
+   reg = <0x6001a000 0x3D00/* VDE registers */
+  0x4400 0x3FC00>; /* IRAM region */
+   reg-names = "regs", "iram";
+   interrupts = , /* UCQ error 
interrupt */
+, /* Sync token 
interrupt */
+, /* BSE-V 
interrupt */
+, /* BSE-A 
interrupt */
+; /* SXE interrupt 
*/
+   interrupt-names = "ucq-error", "sync-token", "bsev", "bsea", 
"sxe";
+   clocks = <_car TEGRA20_CLK_VDE>;
+   clock-names = "vde";
+   resets = <_car 61>;
+   reset-names = "vde";
+   };
+
apbmisc@7800 {
compatible = "nvidia,tegra20-apbmisc";
reg = <0x7800 0x64   /* Chip revision */
-- 
2.14.2

[PATCH v3 0/2] NVIDIA Tegra20 video decoder driver

[Dmitry Osipenko]

This driver provides accelerated video decoding to NVIDIA Tegra20 SoC's,
it is a result of reverse-engineering efforts. Driver has been tested on
Toshiba AC100 and Acer A500, it should work on any Tegra20 device.

In userspace this driver is utilized by libvdpau-tegra [0] that implements
VDPAU interface, so any video player that supports VDPAU can provide
accelerated video decoding on Tegra20 on Linux.

[0] https://github.com/grate-driver/libvdpau-tegra

Change log:
v3:
- Suppressed compilation warnings reported by 'kbuild test robot'

v2:
- Addressed v1 review comments from Stephen Warren and Dan Carpenter
- Implemented runtime PM
- Miscellaneous code cleanups
- Changed 'TODO'
- CC'd media maintainers for the review as per Greg K-H request,
  v1 can be viewed at https://lkml.org/lkml/2017/9/25/606

Dmitry Osipenko (2):
  staging: Introduce NVIDIA Tegra20 video decoder driver
  ARM: dts: tegra20: Add video decoder node

 .../bindings/arm/tegra/nvidia,tegra20-vde.txt  |   44 +
 arch/arm/boot/dts/tegra20.dtsi |   17 +
 drivers/staging/Kconfig|2 +
 drivers/staging/Makefile   |1 +
 drivers/staging/tegra-vde/Kconfig  |6 +
 drivers/staging/tegra-vde/Makefile |1 +
 drivers/staging/tegra-vde/TODO |5 +
 drivers/staging/tegra-vde/uapi.h   |  101 ++
 drivers/staging/tegra-vde/vde.c| 1109 
 9 files changed, 1286 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt
 create mode 100644 drivers/staging/tegra-vde/Kconfig
 create mode 100644 drivers/staging/tegra-vde/Makefile
 create mode 100644 drivers/staging/tegra-vde/TODO
 create mode 100644 drivers/staging/tegra-vde/uapi.h
 create mode 100644 drivers/staging/tegra-vde/vde.c

-- 
2.14.2

Re: [PATCH v3 04/26] media: lirc_zilog: remove receiver

[Andy Walls]

On Tue, 2017-10-10 at 08:17 +0100, Sean Young wrote:
> The ir-kbd-i2c module already handles this very well.

Hi Sean:

It's been years, but my recollection is that although ir-kdb-i2c might
handle receive well, but since the 4 i2c addresses (1 Rx, 1 Tx, 1 IR Tx
code learning, 1 custom Tx code) are all handled by the same Zilog
microcontroller internally, that splitting the Rx and Tx functionality
between two modules became problematic.

Believe me, if i could have leveraged ir-kdb-i2c back when I ported
this, I would have.  I think it's a very bad idea to remove the
receiver from lirc_zilog.

The cx18 and ivtv drivers both use lirc_zilog IIRC.  Have you looked at
the changes required to have the first I2C address used by one i2c
module, and the next one (or three) I2C addresses used by another i2c
module?

-Andy


> 
> Signed-off-by: Sean Young 
> ---
>  drivers/staging/media/lirc/lirc_zilog.c | 901 +++---
> --
>  1 file changed, 76 insertions(+), 825 deletions(-)
> 
> diff --git a/drivers/staging/media/lirc/lirc_zilog.c
> b/drivers/staging/media/lirc/lirc_zilog.c
> index 6bd0717bf76e..757b3fc247ac 100644
> --- a/drivers/staging/media/lirc/lirc_zilog.c
> +++ b/drivers/staging/media/lirc/lirc_zilog.c
> @@ -64,28 +64,7 @@
>  /* Max transfer size done by I2C transfer functions */
>  #define MAX_XFER_SIZE  64
>  
> -struct IR;
> -
> -struct IR_rx {
> - struct kref ref;
> - struct IR *ir;
> -
> - /* RX device */
> - struct mutex client_lock;
> - struct i2c_client *c;
> -
> - /* RX polling thread data */
> - struct task_struct *task;
> -
> - /* RX read data */
> - unsigned char b[3];
> - bool hdpvr_data_fmt;
> -};
> -
>  struct IR_tx {
> - struct kref ref;
> - struct IR *ir;
> -
>   /* TX device */
>   struct mutex client_lock;
>   struct i2c_client *c;
> @@ -93,39 +72,9 @@ struct IR_tx {
>   /* TX additional actions needed */
>   int need_boot;
>   bool post_tx_ready_poll;
> -};
> -
> -struct IR {
> - struct kref ref;
> - struct list_head list;
> -
> - /* FIXME spinlock access to l->features */
>   struct lirc_dev *l;
> - struct lirc_buffer rbuf;
> -
> - struct mutex ir_lock;
> - atomic_t open_count;
> -
> - struct device *dev;
> - struct i2c_adapter *adapter;
> -
> - spinlock_t rx_ref_lock; /* struct IR_rx kref get()/put() */
> - struct IR_rx *rx;
> -
> - spinlock_t tx_ref_lock; /* struct IR_tx kref get()/put() */
> - struct IR_tx *tx;
>  };
>  
> -/* IR transceiver instance object list */
> -/*
> - * This lock is used for the following:
> - * a. ir_devices_list access, insertions, deletions
> - * b. struct IR kref get()s and put()s
> - * c. serialization of ir_probe() for the two i2c_clients for a Z8
> - */
> -static DEFINE_MUTEX(ir_devices_lock);
> -static LIST_HEAD(ir_devices_list);
> -
>  /* Block size for IR transmitter */
>  #define TX_BLOCK_SIZE99
>  
> @@ -153,348 +102,8 @@ struct tx_data_struct {
>  static struct tx_data_struct *tx_data;
>  static struct mutex tx_data_lock;
>  
> -
>  /* module parameters */
>  static bool debug;   /* debug output */
> -static bool tx_only; /* only handle the IR Tx function */
> -
> -
> -/* struct IR reference counting */
> -static struct IR *get_ir_device(struct IR *ir, bool
> ir_devices_lock_held)
> -{
> - if (ir_devices_lock_held) {
> - kref_get(>ref);
> - } else {
> - mutex_lock(_devices_lock);
> - kref_get(>ref);
> - mutex_unlock(_devices_lock);
> - }
> - return ir;
> -}
> -
> -static void release_ir_device(struct kref *ref)
> -{
> - struct IR *ir = container_of(ref, struct IR, ref);
> -
> - /*
> -  * Things should be in this state by now:
> -  * ir->rx set to NULL and deallocated - happens before ir-
> >rx->ir put()
> -  * ir->rx->task kthread stopped - happens before ir->rx->ir
> put()
> -  * ir->tx set to NULL and deallocated - happens before ir-
> >tx->ir put()
> -  * ir->open_count ==  0 - happens on final close()
> -  * ir_lock, tx_ref_lock, rx_ref_lock, all released
> -  */
> - if (ir->l)
> - lirc_unregister_device(ir->l);
> -
> - if (kfifo_initialized(>rbuf.fifo))
> - lirc_buffer_free(>rbuf);
> - list_del(>list);
> - kfree(ir);
> -}
> -
> -static int put_ir_device(struct IR *ir, bool ir_devices_lock_held)
> -{
> - int released;
> -
> - if (ir_devices_lock_held)
> - return kref_put(>ref, release_ir_device);
> -
> - mutex_lock(_devices_lock);
> - released = kref_put(>ref, release_ir_device);
> - mutex_unlock(_devices_lock);
> -
> - return released;
> -}
> -
> -/* struct IR_rx reference counting */
> -static struct IR_rx *get_ir_rx(struct IR *ir)
> -{
> - struct IR_rx *rx;
> -
> - spin_lock(>rx_ref_lock);
> - rx = ir->rx;
> - if (rx)
> - kref_get(>ref);
> - spin_unlock(>rx_ref_lock);

[PATCH] Simplify major/minor non-dynamic logic

[Mauro Carvalho Chehab]

changeset 6bbf7a855d20 ("media: dvbdev: convert DVB device types into an enum")
added a new warning on gcc 6:

>> drivers/media/dvb-core/dvbdev.c:86:1: warning: control reaches end of 
>> non-void function [-Wreturn-type]

That's because gcc is not smart enough to see that all types are
present at the switch. Also, the current code is not too optimized.

So, replace it to a more optimized one, based on a static table.

Reported-by: kbuild test robot 
Fixes: 6bbf7a855d20 ("media: dvbdev: convert DVB device types into an enum")
Signed-off-by: Mauro Carvalho Chehab 
---
I actually suggested this patch to be fold with changeset 6bbf7a855d20.
Unfortunately, I actually forgot to do that (I guess I did, but on a different
machine than the one I used today to pick it).

Anyway, that saves some code space with static minors and cleans up
a warning. So, let's apply it.

 drivers/media/dvb-core/dvbdev.c | 28 +---
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
index a53eb53a4fd5..060c60ddfcc3 100644
--- a/drivers/media/dvb-core/dvbdev.c
+++ b/drivers/media/dvb-core/dvbdev.c
@@ -68,22 +68,20 @@ static const char * const dnames[] = {
 #else
 #define DVB_MAX_IDS4
 
-static int nums2minor(int num, enum dvb_device_type type, int id)
-{
-   int n = (num << 6) | (id << 4);
+static const u8 minor_type[] = {
+   [DVB_DEVICE_VIDEO]  = 0,
+   [DVB_DEVICE_AUDIO]  = 1,
+   [DVB_DEVICE_SEC]= 2,
+   [DVB_DEVICE_FRONTEND]   = 3,
+   [DVB_DEVICE_DEMUX]  = 4,
+   [DVB_DEVICE_DVR]= 5,
+   [DVB_DEVICE_CA] = 6,
+   [DVB_DEVICE_NET]= 7,
+   [DVB_DEVICE_OSD]= 8,
+};
 
-   switch (type) {
-   case DVB_DEVICE_VIDEO:  return n;
-   case DVB_DEVICE_AUDIO:  return n | 1;
-   case DVB_DEVICE_SEC:return n | 2;
-   case DVB_DEVICE_FRONTEND:   return n | 3;
-   case DVB_DEVICE_DEMUX:  return n | 4;
-   case DVB_DEVICE_DVR:return n | 5;
-   case DVB_DEVICE_CA: return n | 6;
-   case DVB_DEVICE_NET:return n | 7;
-   case DVB_DEVICE_OSD:return n | 8;
-   }
-}
+#define nums2minor(num, type, id) \
+   (((num) << 6) | ((id) << 4) | minor_type[type])
 
 #define MAX_DVB_MINORS (DVB_MAX_ADAPTERS*64)
 #endif
-- 
2.13.6

Re: [PATCH v3 00/17] kernel-doc: add supported to document nested structs/

[Mauro Carvalho Chehab]

Em Mon, 9 Oct 2017 10:19:55 -0300
Mauro Carvalho Chehab  escreveu:

> > > I really don't want to add that much noise to the docs build; I think it
> > > will defeat any hope of cleaning up the warnings we already have.  I
> > > wonder if we could suppress warnings about nested structs by default, and
> > > add a flag or envar to turn them back on for those who want them?
> > 
> > This is what I vote for: +1
> >   
> > > In truth, now that I look, the real problem is that the warnings are
> > > repeated many times - as in, like 100 times:
> > > 
> > >> ./include/net/cfg80211.h:4115: warning: Function parameter or member 
> > >> 'wext.ibss' not described in 'wireless_dev'
> > >> ./include/net/cfg80211.h:4115: warning: Function parameter or member 
> > >> 'wext.ibss' not described in 'wireless_dev'
> > > <107 instances deleted...>
> > >> ./include/net/cfg80211.h:4115: warning: Function parameter or member 
> > >> 'wext.ibss' not described in 'wireless_dev'
> > > 
> > > That's not something that used to happen, and is certainly not helpful.
> > > Figuring that out would help a lot.  Can I get you to take a look at
> > > what's going on here?
> > 
> > Hi Jon, if you grep for 
> > 
> >  .. kernel-doc:: include/net/cfg80211.h
> > 
> > e.g. by:  grep cfg80211.h Documentation/driver-api/80211/cfg80211.rst | wc 
> > -l
> > you will see, that this directive is used 110 times in one reST file. If you
> > have 5 warnings about the kernel-doc markup in include/net/cfg80211.h you 
> > will
> > get 550 warnings about kernel-doc markup just for just one source code file.
> > 
> > This is because the kernel-doc parser is an external process which is called
> > (in this example) 110 times for one reST file and one source code file. If 
> > include/net/cfg80211.h is referred in other reST files, we got accordingly
> > more and more warnings .. thats really noise.   
> 
> I guess the solution here is simple: if any output selection argument
> is passed (-export, -internal, -function, -nofunction), only report
> warnings for things that match the output criteria. 
> 
> Not sure how easy is to implement that. I'll take a look.

It is actually very easy to suppress those warnings. See the enclosed
proof of concept patch.

Please notice that this patch is likely incomplete: all it does is that,
if --function or --nofunction is used, it won't print any warnings
for structs or enums.

I didn't check yet if this is not making it too silent.

If we're going to follow this way, IMHO, the best would be to define
a warning function and move the needed checks for $output_selection
and for $function for it to do the right thing, printing warnings only
for the stuff that will be output.

Jon,

Please let me know if you prefer go though this way or if, instead, you
opt to replace the kernel-doc perl script by a Sphinx module.

If you decide to keep with the perl script for now, I can work on an
improved version of this PoC.


Regards,
Mauro

scripts: kernel-doc: shut up enum/struct warnings if parsing only functions

There are two command line parameters that restrict the kernel-doc output
to output only functions. If this is used, it doesn't make any sense to
produce warnings about enums and structs. So, shut up such warnings.

Signed-off-by: Mauro Carvalho Chehab 

diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index 049044d95c0e..b4eebea6a8d6 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -1138,16 +1138,20 @@ sub dump_enum($$) {
push @parameterlist, $arg;
if (!$parameterdescs{$arg}) {
$parameterdescs{$arg} = $undescribed;
-   print STDERR "${file}:$.: warning: Enum value '$arg' ".
-   "not described in enum '$declaration_name'\n";
+   if ($output_selection != OUTPUT_INCLUDE &&
+   $output_selection != OUTPUT_EXCLUDE) {
+   print STDERR "${file}:$.: warning: Enum value '$arg' 
not described in enum '$declaration_name'\n";
+   }
}
$_members{$arg} = 1;
}
 
while (my ($k, $v) = each %parameterdescs) {
if (!exists($_members{$k})) {
-print STDERR "${file}:$.: warning: Excess enum value " .
- "'$k' description in '$declaration_name'\n";
+   if ($output_selection != OUTPUT_INCLUDE &&
+   $output_selection != OUTPUT_EXCLUDE) {
+print STDERR "${file}:$.: warning: Excess enum value '$k' 
description in '$declaration_name'\n";
+   }
}
 }
 
@@ -1353,9 +1357,12 @@ sub push_parameter() {
if (!defined $parameterdescs{$param} && $param !~ /^#/) {
$parameterdescs{$param} = $undescribed;
 
-   print STDERR
- "${file}:$.: warning: Function parameter or member 
'$param' not described in '$declaration_name'\n";
- 

[linuxtv-media:master 2747/2770] drivers/media/dvb-core/dvbdev.c:86:1: warning: control reaches end of non-void function

[kbuild test robot]

tree:   git://linuxtv.org/media_tree.git master
head:   01153bf04db18d5fcd30df64ffe428db7ff7bada
commit: 6bbf7a855d200ddd83494a9ceb95f9465f953f59 [2747/2770] media: dvbdev: 
convert DVB device types into an enum
config: x86_64-acpi-redef (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
git checkout 6bbf7a855d200ddd83494a9ceb95f9465f953f59
# save the attached .config to linux build tree
make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   drivers/media/dvb-core/dvbdev.c: In function 'nums2minor':
>> drivers/media/dvb-core/dvbdev.c:86:1: warning: control reaches end of 
>> non-void function [-Wreturn-type]
}
^

vim +86 drivers/media/dvb-core/dvbdev.c

70  
71  static int nums2minor(int num, enum dvb_device_type type, int id)
72  {
73  int n = (num << 6) | (id << 4);
74  
75  switch (type) {
76  case DVB_DEVICE_VIDEO:  return n;
77  case DVB_DEVICE_AUDIO:  return n | 1;
78  case DVB_DEVICE_SEC:return n | 2;
79  case DVB_DEVICE_FRONTEND:   return n | 3;
80  case DVB_DEVICE_DEMUX:  return n | 4;
81  case DVB_DEVICE_DVR:return n | 5;
82  case DVB_DEVICE_CA: return n | 6;
83  case DVB_DEVICE_NET:return n | 7;
84  case DVB_DEVICE_OSD:return n | 8;
85  }
  > 86  }
87  

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


.config.gz
Description: application/gzip

[PATCH] media: dvb: do some coding style cleanup

[Mauro Carvalho Chehab]

Fix a bunch of coding style issues found by checkpatch on the
part of the code that the previous patches touched.

WARNING: please, no space before tabs
+ * ^I^Icallback.$

ERROR: space required before the open parenthesis '('
+   switch(cmd) {

WARNING: line over 80 characters
+   err = dtv_property_process_get(fe, , tvp + i, 
file);

WARNING: line over 80 characters
+   err = fe->ops.diseqc_recv_slave_reply(fe, (struct 
dvb_diseqc_slave_reply*) parg);

ERROR: "(foo*)" should be "(foo *)"
+   err = fe->ops.diseqc_recv_slave_reply(fe, (struct 
dvb_diseqc_slave_reply*) parg);

WARNING: line over 80 characters
+   err = fe->ops.read_signal_strength(fe, (__u16 
*) parg);

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/dvb-core/dvb_demux.h|  2 +-
 drivers/media/dvb-core/dvb_frontend.c | 15 ---
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_demux.h 
b/drivers/media/dvb-core/dvb_demux.h
index 9a77be02cbb1..cc048f09aa85 100644
--- a/drivers/media/dvb-core/dvb_demux.h
+++ b/drivers/media/dvb-core/dvb_demux.h
@@ -101,7 +101,7 @@ struct dvb_demux_filter {
  * @cb:digital TV callbacks. depending on the feed type, it 
can be:
  * if the feed is TS, it contains a dmx_ts_cb() @ts callback;
  * if the feed is section, it contains a dmx_section_cb() @sec
- * callback.
+ * callback.
  *
  * @demux: pointer to  dvb_demux.
  * @priv:  private data that can optionally be used by a DVB driver.
diff --git a/drivers/media/dvb-core/dvb_frontend.c 
b/drivers/media/dvb-core/dvb_frontend.c
index 30c7357e980b..0c7897379535 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -2096,7 +2096,7 @@ static int dvb_frontend_handle_ioctl(struct file *file,
 
dev_dbg(fe->dvb->device, "%s:\n", __func__);
 
-   switch(cmd) {
+   switch (cmd) {
case FE_SET_PROPERTY: {
struct dtv_properties *tvps = parg;
struct dtv_property *tvp = NULL;
@@ -2164,7 +2164,8 @@ static int dvb_frontend_handle_ioctl(struct file *file,
}
}
for (i = 0; i < tvps->num; i++) {
-   err = dtv_property_process_get(fe, , tvp + i, 
file);
+   err = dtv_property_process_get(fe, ,
+  tvp + i, file);
if (err < 0) {
kfree(tvp);
return err;
@@ -2296,7 +2297,7 @@ static int dvb_frontend_handle_ioctl(struct file *file,
 
case FE_DISEQC_RECV_SLAVE_REPLY:
if (fe->ops.diseqc_recv_slave_reply)
-   err = fe->ops.diseqc_recv_slave_reply(fe, (struct 
dvb_diseqc_slave_reply*) parg);
+   err = fe->ops.diseqc_recv_slave_reply(fe, parg);
break;
 
case FE_ENABLE_HIGH_LNB_VOLTAGE:
@@ -2381,7 +2382,7 @@ static int dvb_frontend_handle_ioctl(struct file *file,
case FE_READ_BER:
if (fe->ops.read_ber) {
if (fepriv->thread)
-   err = fe->ops.read_ber(fe, (__u32 *) parg);
+   err = fe->ops.read_ber(fe, parg);
else
err = -EAGAIN;
}
@@ -2390,7 +2391,7 @@ static int dvb_frontend_handle_ioctl(struct file *file,
case FE_READ_SIGNAL_STRENGTH:
if (fe->ops.read_signal_strength) {
if (fepriv->thread)
-   err = fe->ops.read_signal_strength(fe, (__u16 
*) parg);
+   err = fe->ops.read_signal_strength(fe, parg);
else
err = -EAGAIN;
}
@@ -2399,7 +2400,7 @@ static int dvb_frontend_handle_ioctl(struct file *file,
case FE_READ_SNR:
if (fe->ops.read_snr) {
if (fepriv->thread)
-   err = fe->ops.read_snr(fe, (__u16 *) parg);
+   err = fe->ops.read_snr(fe, parg);
else
err = -EAGAIN;
}
@@ -2408,7 +2409,7 @@ static int dvb_frontend_handle_ioctl(struct file *file,
case FE_READ_UNCORRECTED_BLOCKS:
if (fe->ops.read_ucblocks) {
if (fepriv->thread)
-   err = fe->ops.read_ucblocks(fe, (__u32 *) parg);
+   err = fe->ops.read_ucblocks(fe, parg);
else
err = -EAGAIN;
}
-- 
2.13.6

[PATCH] media: dtv-frontend.rst fix a typo: algoritms -> algorithms

[Mauro Carvalho Chehab]

WARNING: 'algoritms' may be misspelled - perhaps 'algorithms'?
+responsible for tuning the device. It supports multiple algoritms to

Signed-off-by: Mauro Carvalho Chehab 
---
 Documentation/media/kapi/dtv-frontend.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/media/kapi/dtv-frontend.rst 
b/Documentation/media/kapi/dtv-frontend.rst
index 9f67b7a7387d..f1a2fdaab5ba 100644
--- a/Documentation/media/kapi/dtv-frontend.rst
+++ b/Documentation/media/kapi/dtv-frontend.rst
@@ -119,7 +119,7 @@ Satellite TV reception is::
 .. |delta|   unicode:: U+00394
 
 The ``drivers/media/dvb-core/dvb_frontend.c`` has a kernel thread with is
-responsible for tuning the device. It supports multiple algoritms to
+responsible for tuning the device. It supports multiple algorithms to
 detect a channel, as defined at enum :c:func:`dvbfe_algo`.
 
 The algorithm to be used is obtained via ``.get_frontend_algo``. If the driver
-- 
2.13.6

Re: [PATCH v2 08/12] intel-ipu3: params: compute and program ccs

[Andy Shevchenko]

On Wed, Oct 11, 2017 at 5:01 PM, Tuukka Toivonen
 wrote:
> On Wed, 2017-10-11 at 16:31 +0300, Andy Shevchenko wrote:
>> On Wed, Oct 11, 2017 at 10:29 AM, sakari.ai...@linux.intel.com
>>  wrote:
>> > On Wed, Oct 11, 2017 at 04:14:37AM +, Zhi, Yong wrote:

>> > > > > +static unsigned int ipu3_css_scaler_get_exp(unsigned int
>> > > > > counter,
>> > > > > +   unsigned int
>> > > > > divider) {
>> > > > > +   unsigned int i = 0;
>> > > > > +
>> > > > > +   while (counter <= divider / 2) {
>> > > > > +   divider /= 2;
>> > > > > +   i++;
>> > > > > +   }
>> > > > > +
>> > > > > +   return i;

>> Roughly like
>>
>> if (!counter || divider < counter)
>>  return 0;
>> return order_base_2(divider) - order_base_2(counter);
>
> The original loop is typical ran just couple of times, so I think
> that fls or division are probably slower than the original loop.
> Furthermore, these "optimizations" are also harder to read, so in
> my opinion there's no advantage in using them.

Honestly I'm opposing that.
It took me about minute to be clear what is going on on that loop
while fls() / ffs() / ilog2() like stuff can be read fast.

Like

int shift = order_base_2(divider) - order_base_2(counter);

return shift > 0 ? shift : 0;

And frankly I don't care about under the hoods of order_base_2(). I
care about this certain piece of code to be simpler.

One may put a comment line:

# Get log2 of how divider bigger than counter

And thinking more while writing this message the use of order_base_2()
actually explains what's going on here.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 08/12] intel-ipu3: params: compute and program ccs

[Tuukka Toivonen]

On Wed, 2017-10-11 at 16:31 +0300, Andy Shevchenko wrote:
> On Wed, Oct 11, 2017 at 10:29 AM, sakari.ai...@linux.intel.com
>  wrote:
> > On Wed, Oct 11, 2017 at 04:14:37AM +, Zhi, Yong wrote:
> > > > > +static unsigned int ipu3_css_scaler_get_exp(unsigned int
> > > > > counter,
> > > > > +   unsigned int
> > > > > divider) {
> > > > > +   unsigned int i = 0;
> > > > > +
> > > > > +   while (counter <= divider / 2) {
> > > > > +   divider /= 2;
> > > > > +   i++;
> > > > > +   }
> > > > > +
> > > > > +   return i;
> > return (!counter || divider < counter) ?
> >    0 : fls(divider / counter) - 1;
> 
> Extra division is here (I dunno if counter is always power of 2 but
> it
> doesn't matter for compiler).
> 
> Basically above calculates how much bits we need to shift divider to
> get it less than counter.
> 
> I would consider to use something from log2.h.
> 
> Roughly like
> 
> if (!counter || divider < counter)
>  return 0;
> return order_base_2(divider) - order_base_2(counter);

The original loop is typical ran just couple of times, so I think
that fls or division are probably slower than the original loop.
Furthermore, these "optimizations" are also harder to read, so in
my opinion there's no advantage in using them.

- Tuukka

add support of Geniatech X9320 DVB-S2 quad tuner

[Alexandre Da Costa]

Dear all,

I recently got a Geniatech X9320 DVB-S2 PCIe card :
https://www.geniatech.com/product/x9320-quad-dvb-s2s-tv-tuner/

The card is not listed in the LinuxTV so it's most probably not
supported as of now. I quickly tried it on a 4.9 kernel and it seems
that no driver was loaded.

On the hardware side, the board is based on the following ICs :
- Renesas µPD720201 USB 3.0 controller (4 ports)
- Cypress CY68013 USB bridge
- Montage M88DS3103as demodulator
- Unknown RF Tuner but most probably Montage M88TS2022. If required, I
can remove the RF shield and have a look at the chip.

The board is very similar to the Geniatech HD Star (hardware V3.0) :
https://www.linuxtv.org/wiki/index.php/Geniatech_HD_Star_DVB-S2_USB2.0

However, it reports a different idProduct. Instead of 0x3000
(idProduct of Geniatech HD Star), it returns the following list
(different idProduct for each DVB-S2 tuner) :
Bus 001 Device 005: ID 1f4d:3300 G-Tek Electronics Group
Bus 001 Device 004: ID 1f4d:3301 G-Tek Electronics Group
Bus 001 Device 003: ID 1f4d:3302 G-Tek Electronics Group
Bus 001 Device 002: ID 1f4d:3303 G-Tek Electronics Group

I assume that since the hardware is almost the same, the drivers used
for the HD Star should work with the X9320 card by adding the new
idProduct.

I've no experience in Linux driver development but I would give it a
try if someone can show me where to look at.

Regards,
Alex

Re: [PATCH 2/2] v4l: cadence: Add Cadence MIPI-CSI2 TX driver

[Benoit Parrot]

Hi Maxime,

Maxime Ripard  wrote on Wed [2017-Oct-11 
13:55:44 +0200]:
> Hi Benoit,
> 
> On Fri, Sep 29, 2017 at 06:21:25PM +, Benoit Parrot wrote:
> > > +struct csi2tx_priv {
> > > + struct device   *dev;
> > > + atomic_tcount;
> > > +
> > > + void __iomem*base;
> > > +
> > > + struct clk  *esc_clk;
> > > + struct clk  *p_clk;
> > > + struct clk  *pixel_clk[CSI2TX_STREAMS_MAX];
> > > +
> > > + struct v4l2_subdev  subdev;
> > > + struct media_padpads[CSI2TX_PAD_MAX];
> > > + struct v4l2_mbus_framefmt   pad_fmts[CSI2TX_PAD_MAX];
> > > +
> > > + boolhas_internal_dphy;
> > 
> > I assume dphy support is for a subsequent revision?
> 
> Yes, the situation is similar to the CSI2-RX driver.
> 
> > > + /*
> > > +  * We use the stream ID there, but it's wrong.
> > > +  *
> > > +  * A stream could very well send a data type that is
> > > +  * not equal to its stream ID. We need to find a
> > > +  * proper way to address it.
> > > +  */
> > 
> > I don't quite get the above comment, from the code below it looks like
> > you are using the current fmt as a source to provide the correct DT.
> > Am I missing soemthing?
> 
> Yes, so far the datatype is inferred from the format set. Is there
> anything wrong with that?

No, nothing wrong with that behavior it just doesn't not match the comment
above, where it is says that the DT is set to the stream ID...

Regards,
Benoit

Re: [PATCH v2 08/12] intel-ipu3: params: compute and program ccs

[Andy Shevchenko]

On Wed, Oct 11, 2017 at 10:29 AM, sakari.ai...@linux.intel.com
 wrote:
> On Wed, Oct 11, 2017 at 04:14:37AM +, Zhi, Yong wrote:

>> > > +static unsigned int ipu3_css_scaler_get_exp(unsigned int counter,
>> > > +   unsigned int divider) {
>> > > +   unsigned int i = 0;
>> > > +
>> > > +   while (counter <= divider / 2) {
>> > > +   divider /= 2;
>> > > +   i++;
>> > > +   }
>> > > +
>> > > +   return i;

> return (!counter || divider < counter) ?
>0 : fls(divider / counter) - 1;

Extra division is here (I dunno if counter is always power of 2 but it
doesn't matter for compiler).

Basically above calculates how much bits we need to shift divider to
get it less than counter.

I would consider to use something from log2.h.

Roughly like

if (!counter || divider < counter)
 return 0;
return order_base_2(divider) - order_base_2(counter);

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 2/2] v4l: cadence: Add Cadence MIPI-CSI2 RX driver

[Benoit Parrot]

Hi Maxime,

Maxime Ripard  wrote on Wed [2017-Oct-11 
11:24:09 +0200]:
> Hi Benoit,
> 
> On Fri, Sep 29, 2017 at 05:27:09PM +, Benoit Parrot wrote:
> > > +static int csi2rx_get_resources(struct csi2rx_priv *csi2rx,
> > > + struct platform_device *pdev)
> > > +{
> > > + struct resource *res;
> > > + unsigned char i;
> > > + u32 reg;
> > > +
> > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > > + csi2rx->base = devm_ioremap_resource(>dev, res);
> > > + if (IS_ERR(csi2rx->base))
> > > + return PTR_ERR(csi2rx->base);
> > > +
> > > + csi2rx->sys_clk = devm_clk_get(>dev, "sys_clk");
> > > + if (IS_ERR(csi2rx->sys_clk)) {
> > > + dev_err(>dev, "Couldn't get sys clock\n");
> > > + return PTR_ERR(csi2rx->sys_clk);
> > > + }
> > > +
> > > + csi2rx->p_clk = devm_clk_get(>dev, "p_clk");
> > > + if (IS_ERR(csi2rx->p_clk)) {
> > > + dev_err(>dev, "Couldn't get P clock\n");
> > > + return PTR_ERR(csi2rx->p_clk);
> > > + }
> > > +
> > > + csi2rx->dphy = devm_phy_optional_get(>dev, "dphy");
> > > + if (IS_ERR(csi2rx->dphy)) {
> > > + dev_err(>dev, "Couldn't get external D-PHY\n");
> > > + return PTR_ERR(csi2rx->dphy);
> > > + }
> > > +
> > > + /*
> > > +  * FIXME: Once we'll have external D-PHY support, the check
> > > +  * will need to be removed.
> > > +  */
> > > + if (csi2rx->dphy) {
> > > + dev_err(>dev, "External D-PHY not supported yet\n");
> > > + return -EINVAL;
> > > + }
> > 
> > I understand that in your current environment you do not have a
> > DPHY. But I am wondering in a real setup where you will have either
> > an internal or an external DPHY, how are they going to interact with
> > this driver or vice-versa?
> 
> It's difficult to give an answer with so little details. How would you
> choose between those two PHYs? Is there a mux, or should we just power
> one of the two? If that's the case, is there any use case were we
> might want to power both? If not, which one should we favor, in which
> situations?

Oops, I guess I should clarify, in this case I did not mean we would
have both an internal and an external DPHY. I just meant one or the other.
Basically just want to see how you would actually handle a DPHY here whether
it's internal or external?

For instance, using direct register access from within this driver or make
use of an separate phy driver...

> 
> I guess all those questions actually depend on the way the integration
> has been done, and we're not quite there yet. I guess we could do
> either a platform specific structure or a glue, depending on the
> complexity. The platform specific compatible will allow us to do that
> as we see fit anyway.
> 

Regards,
Benoit

Re: [PATCH] [media] vimc: Fix return value check in vimc_add_subdevs()

[Helen Koike]

Hello,


On 2017-10-11 08:16 AM, Wei Yongjun wrote:
> In case of error, the function platform_device_register_data() returns
> ERR_PTR() and never returns NULL. The NULL test in the return value check
> should be replaced with IS_ERR().
> 
> Signed-off-by: Wei Yongjun 
> ---
>  drivers/media/platform/vimc/vimc-core.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/vimc/vimc-core.c 
> b/drivers/media/platform/vimc/vimc-core.c
> index 51c0eee..fe088a9 100644
> --- a/drivers/media/platform/vimc/vimc-core.c
> +++ b/drivers/media/platform/vimc/vimc-core.c
> @@ -267,11 +267,12 @@ static struct component_match *vimc_add_subdevs(struct 
> vimc_device *vimc)
>   PLATFORM_DEVID_AUTO,
>   ,
>   sizeof(pdata));
> - if (!vimc->subdevs[i]) {
> + if (IS_ERR(vimc->subdevs[i])) {
> + match = ERR_CAST(vimc->subdevs[i]);
>   while (--i >= 0)
>   platform_device_unregister(vimc->subdevs[i]);
>  
> - return ERR_PTR(-ENOMEM);
> + return match;
>   }
>  
>   component_match_add(>pdev.dev, , vimc_comp_compare,
> 
> 
> 

Nice catch, thanks, looks good to me

Acked-by: Helen Koike 

Re: [PATCH] uvcvideo: Apply flags from device to actual properties

[Kieran Bingham]

Hi Edgar,

Sorry for not replying to you yesterday on IRC, but by the time I got to reply
to the message you weren't online..

On 11/10/17 12:56, Edgar Thier wrote:
> 
> Use flags the device exposes for UVC controls.
> This allows the device to define which property flags are set.
> 
> Since some cameras offer auto-adjustments for properties (e.g. auto-gain),
> the values of other properties (e.g. gain) can change in the camera.
> Examining the flags ensures that the driver is aware of such properties.
> 
> Signed-off-by: Edgar Thier 
> ---
>  drivers/media/usb/uvc/uvc_ctrl.c | 65 
> ++--
>  1 file changed, 50 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c 
> b/drivers/media/usb/uvc/uvc_ctrl.c
> index 20397ab..7fbfeef 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -1630,12 +1630,47 @@ static void uvc_ctrl_fixup_xu_info(struct uvc_device 
> *dev,
>  }
> 
>  /*
> + * Retrieve flags for a given control
> + */
> +static int uvc_ctrl_get_flags(struct uvc_device *dev, const struct 
> uvc_control *ctrl,
> + const struct uvc_control_info *info)
> +{
> + u8 *data;
> + int ret = 0;
> + int flags = 0;
> +
> + data = kmalloc(2, GFP_KERNEL);
> + if (data == NULL)
> + return -ENOMEM;
> +
> + ret = uvc_query_ctrl(dev, UVC_GET_INFO, ctrl->entity->id, dev->intfnum,
> +  info->selector, data, 1);
> + if (ret < 0) {
> + uvc_trace(UVC_TRACE_CONTROL,
> +   "GET_INFO failed on control %pUl/%u (%d).\n",
> +   info->entity, info->selector, ret);
> + } else {
> + flags = UVC_CTRL_FLAG_GET_MIN | UVC_CTRL_FLAG_GET_MAX
> + | UVC_CTRL_FLAG_GET_RES | UVC_CTRL_FLAG_GET_DEF
> + | (data[0] & UVC_CONTROL_CAP_GET ?
> +UVC_CTRL_FLAG_GET_CUR : 0)
> + | (data[0] & UVC_CONTROL_CAP_SET ?
> +UVC_CTRL_FLAG_SET_CUR : 0)
> + | (data[0] & UVC_CONTROL_CAP_AUTOUPDATE ?
> +UVC_CTRL_FLAG_AUTO_UPDATE : 0);
> + }
> + kfree(data);
> + return flags;
> +}
> +
> +/*
>   * Query control information (size and flags) for XU controls.
>   */
>  static int uvc_ctrl_fill_xu_info(struct uvc_device *dev,
>   const struct uvc_control *ctrl, struct uvc_control_info *info)
>  {
>   u8 *data;
> + int flags;
>   int ret;
> 
>   data = kmalloc(2, GFP_KERNEL);
> @@ -1659,24 +1694,14 @@ static int uvc_ctrl_fill_xu_info(struct uvc_device 
> *dev,
> 
>   info->size = le16_to_cpup((__le16 *)data);
> 
> - /* Query the control information (GET_INFO) */
> - ret = uvc_query_ctrl(dev, UVC_GET_INFO, ctrl->entity->id, dev->intfnum,
> -  info->selector, data, 1);
> - if (ret < 0) {
> + flags = uvc_ctrl_get_flags(dev, ctrl, info);
> +
> + if (flags < 0) {
>   uvc_trace(UVC_TRACE_CONTROL,
> -   "GET_INFO failed on control %pUl/%u (%d).\n",
> -   info->entity, info->selector, ret);
> +   "Failed to retrieve flags (%d).\n", ret);

I think we need to set 'ret' to be an error value here ?
ret = flags? or ret = -ENOMEM?

Otherwise in 'done:' ret will be the return value of the previous
uvc_query_ctrl() call.

>   goto done;
>   }> -
> - info->flags = UVC_CTRL_FLAG_GET_MIN | UVC_CTRL_FLAG_GET_MAX
> - | UVC_CTRL_FLAG_GET_RES | UVC_CTRL_FLAG_GET_DEF
> - | (data[0] & UVC_CONTROL_CAP_GET ?
> -UVC_CTRL_FLAG_GET_CUR : 0)
> - | (data[0] & UVC_CONTROL_CAP_SET ?
> -UVC_CTRL_FLAG_SET_CUR : 0)
> - | (data[0] & UVC_CONTROL_CAP_AUTOUPDATE ?
> -UVC_CTRL_FLAG_AUTO_UPDATE : 0);
> + info->flags = flags;
> 
>   uvc_ctrl_fixup_xu_info(dev, ctrl, info);
> 
> @@ -1890,6 +1915,7 @@ static int uvc_ctrl_add_info(struct uvc_device *dev, 
> struct uvc_control *ctrl,
>   const struct uvc_control_info *info)
>  {
>   int ret = 0;
> + int flags = 0;
> 
>   ctrl->info = *info;
>   INIT_LIST_HEAD(>info.mappings);
> @@ -1902,6 +1928,15 @@ static int uvc_ctrl_add_info(struct uvc_device *dev, 
> struct uvc_control *ctrl,
>   goto done;
>   }
> 
> + flags = uvc_ctrl_get_flags(dev, ctrl, info);
> +

I think the if could hug the call, and remove this blank line.

> + if (flags < 0) {
> + uvc_trace(UVC_TRACE_CONTROL,
> +   "Failed to retrieve flags (%d).\n", ret);
> + }

An if statement shouldn't have braces if it has only a single statement.

> +
> + ctrl->info.flags = flags;

This is still setting an error value into ctrl->info.flags in the event of a 
fault.

Perhaps it should be initialised 

Re: [PATCH] uvcvideo: Apply flags from device to actual properties

[Edgar Thier]

Use flags the device exposes for UVC controls.
This allows the device to define which property flags are set.

Since some cameras offer auto-adjustments for properties (e.g. auto-gain),
the values of other properties (e.g. gain) can change in the camera.
Examining the flags ensures that the driver is aware of such properties.

Signed-off-by: Edgar Thier 
---
 drivers/media/usb/uvc/uvc_ctrl.c | 65 ++--
 1 file changed, 50 insertions(+), 15 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index 20397ab..7fbfeef 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -1630,12 +1630,47 @@ static void uvc_ctrl_fixup_xu_info(struct uvc_device 
*dev,
 }

 /*
+ * Retrieve flags for a given control
+ */
+static int uvc_ctrl_get_flags(struct uvc_device *dev, const struct uvc_control 
*ctrl,
+   const struct uvc_control_info *info)
+{
+   u8 *data;
+   int ret = 0;
+   int flags = 0;
+
+   data = kmalloc(2, GFP_KERNEL);
+   if (data == NULL)
+   return -ENOMEM;
+
+   ret = uvc_query_ctrl(dev, UVC_GET_INFO, ctrl->entity->id, dev->intfnum,
+info->selector, data, 1);
+   if (ret < 0) {
+   uvc_trace(UVC_TRACE_CONTROL,
+ "GET_INFO failed on control %pUl/%u (%d).\n",
+ info->entity, info->selector, ret);
+   } else {
+   flags = UVC_CTRL_FLAG_GET_MIN | UVC_CTRL_FLAG_GET_MAX
+   | UVC_CTRL_FLAG_GET_RES | UVC_CTRL_FLAG_GET_DEF
+   | (data[0] & UVC_CONTROL_CAP_GET ?
+  UVC_CTRL_FLAG_GET_CUR : 0)
+   | (data[0] & UVC_CONTROL_CAP_SET ?
+  UVC_CTRL_FLAG_SET_CUR : 0)
+   | (data[0] & UVC_CONTROL_CAP_AUTOUPDATE ?
+  UVC_CTRL_FLAG_AUTO_UPDATE : 0);
+   }
+   kfree(data);
+   return flags;
+}
+
+/*
  * Query control information (size and flags) for XU controls.
  */
 static int uvc_ctrl_fill_xu_info(struct uvc_device *dev,
const struct uvc_control *ctrl, struct uvc_control_info *info)
 {
u8 *data;
+   int flags;
int ret;

data = kmalloc(2, GFP_KERNEL);
@@ -1659,24 +1694,14 @@ static int uvc_ctrl_fill_xu_info(struct uvc_device *dev,

info->size = le16_to_cpup((__le16 *)data);

-   /* Query the control information (GET_INFO) */
-   ret = uvc_query_ctrl(dev, UVC_GET_INFO, ctrl->entity->id, dev->intfnum,
-info->selector, data, 1);
-   if (ret < 0) {
+   flags = uvc_ctrl_get_flags(dev, ctrl, info);
+
+   if (flags < 0) {
uvc_trace(UVC_TRACE_CONTROL,
- "GET_INFO failed on control %pUl/%u (%d).\n",
- info->entity, info->selector, ret);
+ "Failed to retrieve flags (%d).\n", ret);
goto done;
}
-
-   info->flags = UVC_CTRL_FLAG_GET_MIN | UVC_CTRL_FLAG_GET_MAX
-   | UVC_CTRL_FLAG_GET_RES | UVC_CTRL_FLAG_GET_DEF
-   | (data[0] & UVC_CONTROL_CAP_GET ?
-  UVC_CTRL_FLAG_GET_CUR : 0)
-   | (data[0] & UVC_CONTROL_CAP_SET ?
-  UVC_CTRL_FLAG_SET_CUR : 0)
-   | (data[0] & UVC_CONTROL_CAP_AUTOUPDATE ?
-  UVC_CTRL_FLAG_AUTO_UPDATE : 0);
+   info->flags = flags;

uvc_ctrl_fixup_xu_info(dev, ctrl, info);

@@ -1890,6 +1915,7 @@ static int uvc_ctrl_add_info(struct uvc_device *dev, 
struct uvc_control *ctrl,
const struct uvc_control_info *info)
 {
int ret = 0;
+   int flags = 0;

ctrl->info = *info;
INIT_LIST_HEAD(>info.mappings);
@@ -1902,6 +1928,15 @@ static int uvc_ctrl_add_info(struct uvc_device *dev, 
struct uvc_control *ctrl,
goto done;
}

+   flags = uvc_ctrl_get_flags(dev, ctrl, info);
+
+   if (flags < 0) {
+   uvc_trace(UVC_TRACE_CONTROL,
+ "Failed to retrieve flags (%d).\n", ret);
+   }
+
+   ctrl->info.flags = flags;
+
ctrl->initialized = 1;

uvc_trace(UVC_TRACE_CONTROL, "Added control %pUl/%u to device %s "
-- 
2.7.4

Re: [PATCH 2/2] v4l: cadence: Add Cadence MIPI-CSI2 TX driver

[Maxime Ripard]

Hi Benoit,

On Fri, Sep 29, 2017 at 06:21:25PM +, Benoit Parrot wrote:
> > +struct csi2tx_priv {
> > +   struct device   *dev;
> > +   atomic_tcount;
> > +
> > +   void __iomem*base;
> > +
> > +   struct clk  *esc_clk;
> > +   struct clk  *p_clk;
> > +   struct clk  *pixel_clk[CSI2TX_STREAMS_MAX];
> > +
> > +   struct v4l2_subdev  subdev;
> > +   struct media_padpads[CSI2TX_PAD_MAX];
> > +   struct v4l2_mbus_framefmt   pad_fmts[CSI2TX_PAD_MAX];
> > +
> > +   boolhas_internal_dphy;
> 
> I assume dphy support is for a subsequent revision?

Yes, the situation is similar to the CSI2-RX driver.

> > +   /*
> > +* We use the stream ID there, but it's wrong.
> > +*
> > +* A stream could very well send a data type that is
> > +* not equal to its stream ID. We need to find a
> > +* proper way to address it.
> > +*/
> 
> I don't quite get the above comment, from the code below it looks like
> you are using the current fmt as a source to provide the correct DT.
> Am I missing soemthing?

Yes, so far the datatype is inferred from the format set. Is there
anything wrong with that?

> 
> > +   writel(CSI2TX_DT_CFG_DT(fmt->dt),
> > +  csi2tx->base + CSI2TX_DT_CFG_REG(stream));
> > +
> > +   writel(CSI2TX_DT_FORMAT_BYTES_PER_LINE(mfmt->width * fmt->bpp) |
> > +  CSI2TX_DT_FORMAT_MAX_LINE_NUM(mfmt->height + 1),
> > +  csi2tx->base + CSI2TX_DT_FORMAT_REG(stream));
> > +
> > +   /*
> > +* TODO: This needs to be calculated based on the
> > +* clock rate.
> > +*/
> > +   writel(CSI2TX_STREAM_IF_CFG_FILL_LEVEL(4),
> > +  csi2tx->base + CSI2TX_STREAM_IF_CFG_REG(stream));
> > +   }
> > +
> > +   /* Disable the configuration mode */
> > +   writel(0, csi2tx->base + CSI2TX_CONFIG_REG);
> > +
> > +   return 0;
> > +}
> > +
> > +static int csi2tx_stop(struct csi2tx_priv *csi2tx)
> > +{
> > +   /*
> > +* Let the last user turn off the lights
> > +*/
> > +   if (!atomic_dec_and_test(>count))
> > +   return 0;
> > +
> > +   /* FIXME: Disable the IP here */
> > +
> > +   return 0;
> > +}
> 
> At this point both _start() and _stop() only return 0.
> Are there any cases where any of these might fail to "start" or "stop"?

None that I know of.

There might be some errors with the video stream itself, but that
can be detected after the block has been enabled.

> > +   csi2tx->lanes = csi2tx_get_num_lanes(>dev);
> > +   if (csi2tx->lanes < 0) {
> > +   dev_err(>dev, "Invalid number of lanes, bailing out.\n");
> > +   ret = csi2tx->lanes;
> > +   goto err_free_priv;
> > +   }
> 
> csi2tx->lanes is unsigned so it will never be negative.

Ah, right, I'll change that.

Thanks!
Maxime

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


signature.asc
Description: PGP signature

Re: [PATCH v4] drm/bridge/sii8620: add remote control support

[Andrzej Hajda]

On 09.10.2017 10:44, Sean Young wrote:
> Hi,
>
> On Thu, Sep 21, 2017 at 12:46:04PM +0100, Sean Young wrote:
>> On Mon, Sep 18, 2017 at 04:37:52PM +0200, Hans Verkuil wrote:
>>> On 09/18/2017 04:15 PM, Maciej Purski wrote:
 Hi Hans,
 some time ago in reply to your email I described what messages does
 the MHL driver receive and at what time intervals.
 Regarding that information, do you think that a similar solution as
 in [1] is required? Would it be OK, if I just set REP_DELAY and REP_PERIOD
 to values, which I presented in my last email?
>>> Based on the timings you measured I would say that there is a 99% chance 
>>> that MHL
>>> uses exactly the same "Press and Hold" mechanism as CEC. Since you already 
>>> specify
>>> RC_PROTO_BIT_CEC in the driver, it will set REP_DELAY and REP_PERIOD to the 
>>> right
>>> values automatically.
>>>
>>> You will have to implement the same code as in cec-adap.c for the key press 
>>> handling,
>>> though. It's a bit tricky to get it right.
>>>
>>> Since you will have to do the same thing as I do in CEC, I wonder if it 
>>> would make
>>> more sense to move this code to the RC core. Basically it ensures that 
>>> repeat mode
>>> doesn't turn on until you get two identical key presses 550ms or less 
>>> apart. This
>>> is independent of REP_DELAY which can be changed by userspace.
>>>
>>> Sean, what do you think?
>> Yes, this makes sense. In fact, since IR protocols have different repeat
>> times, I would like to make this protocol dependant anyway.
>>
>> To be honest I find REP_DELAY of 500ms too long and REP_PERIOD of 125ms
>> too short; IOW it takes too long for a key to start repeating, and once
>> it starts repeating it is very hard to control. If I try to increase the
>> volume with my remote it first hardly changes at all and then after 500ms
>> the volume shoots up far too quickly, same thing when navigating menus.
>>
>> CEC dictates a delay period of 550ms, which is not great for the user IMO.
>>
>> Anyway I'll have a look at this over the weekend.
> I have started on this, but I haven't gotten it in a state where I'm
> happy to submit it. I hope to have it ready for v4.15; in the mean time,
> there is no reason to block this patch on this.
>
>
> Sean
>
>
>

Queued to drm-misc-next.

Thanks
Andrzej

[PATCH] [media] vimc: Fix return value check in vimc_add_subdevs()

[Wei Yongjun]

In case of error, the function platform_device_register_data() returns
ERR_PTR() and never returns NULL. The NULL test in the return value check
should be replaced with IS_ERR().

Signed-off-by: Wei Yongjun 
---
 drivers/media/platform/vimc/vimc-core.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/vimc/vimc-core.c 
b/drivers/media/platform/vimc/vimc-core.c
index 51c0eee..fe088a9 100644
--- a/drivers/media/platform/vimc/vimc-core.c
+++ b/drivers/media/platform/vimc/vimc-core.c
@@ -267,11 +267,12 @@ static struct component_match *vimc_add_subdevs(struct 
vimc_device *vimc)
PLATFORM_DEVID_AUTO,
,
sizeof(pdata));
-   if (!vimc->subdevs[i]) {
+   if (IS_ERR(vimc->subdevs[i])) {
+   match = ERR_CAST(vimc->subdevs[i]);
while (--i >= 0)
platform_device_unregister(vimc->subdevs[i]);
 
-   return ERR_PTR(-ENOMEM);
+   return match;
}
 
component_match_add(>pdev.dev, , vimc_comp_compare,

Re: [PATCH v7 1/7] media: add glossary.rst with a glossary of terms used at V4L2 spec

[Mauro Carvalho Chehab]

Em Wed, 11 Oct 2017 01:18:58 +0300
Sakari Ailus  escreveu:

> > So, if you look, for example, at the chapter 1 name:
> > "common API elements"
> > 
> > it implies that every single V4L2 device node supports what's there.
> > But that's not the case, for example, for what's described at
> > Documentation/media/uapi/v4l/querycap.rst (with is part of
> > chapter 1).  
> 
> Ah. I see what you mean.
> 
> > 
> > There are a couple of possible alternatives:
> > 
> > 1) define V4L2 device nodes excluding /dev/subdev, with is the
> >current approach;
> > 
> > 2) rewrite the entire V4L2 uAPI spec to explicitly talk, on each
> >section, if it applies or not to sub-devices;  
> 
> There are exceptions in the common API elements section even now. For
> instance, it mentions that radio devices don't support video streaming
> related IOCTLs. 

FYI, when this chapter was written, radio devices did support video
streaming ioctls :-)

As I explained on a previous email, Only several years after V4L2
spec was written, it was decided to nack using a radio device to 
control a video stream.

The real issue is that radio sets frequencies on multiples of 62.5 Hz,
while TV sets frequencies on multiples of 62.5 kHz. Due to that, the
core needs to know if the device is in radio or TV mode, in order to
handle VIDIOC_G_FREQUENCY/VIDIOC_S_FREQUENCY. There was a code there
that used to identify, but it was buggy, and there were no safe way
fix.

So, the "common API elements" was modified to forbid to use radio to
do video stream.

Later, a coarse grain logic was added at the v4l2 core in order to
present a different set of ioctls, based on the V4L2 device type,
and the chapter was modified again.

On that time, we were lazy enough to rewrite chapter 1. So,
we just add an exception "hack" at the chapter noticing that
radio devices don't stream.

I don't see a strong reason to not add other pontual exceptions
there where pertinent, but if we start having too many exceptions,
then is time to rewrite it in a way that would make more sense.

> Under common API elements, also the first section (opening
> and closing devices) refers to the interfaces section which, as we know,
> contains sub-device documentation.

Yeah, exceptions could be added, but it means that someone should read
the entire chapter and mention what doesn't apply for subdev.

> Do you see that something else is needed than telling which common API
> elements aren't relevant for sub-devices? (I didn't find explicit
> information in other sections, by a quick glance at least, telling which
> interfaces they apply to.)

No. I didn't read the entire chapter to see what doesn't apply for
sub-devices, but I'm pretty sure that there are many things. For
example, you can't read/write/mmap/stream a video on a subdev.

> Should we make such a change, this would be an additional argument for
> supporting VIDIOC_QUERYCAP on sub-devices.

Nah. We shouldn't add ioctls with the argument that it would be easier
to document. Also, if we add a subdev QUERYCAP, it would provide a
different set of information than a V4L2 device QUERYCAP.

> Further on, section 8, "Common
> definitions for V4L2 and V4L2 subdev interfaces", should probably be merged
> with the "common API elements" section.

Well, if we go to approach (2), we'll need to shrink the common
API definitions chapter, and add another chapter for the "uncommon"
features.

As an exercise: try to rewrite just open.rst to exclude sudevs where
not pertinent. Even a single definition like this:


The main driver always exposes one or more
:term:`V4L2 device nodes `
(see :ref:`v4l2_device_naming`) with are responsible for implementing
data streaming, if applicable.

will become a lot more complex to explain if we don't have any term
to refer to "all device node types created by the v4l2 core except
for /dev/v4l-subdev* and /dev/mediaXXX".

The "Related devices" and "Shared Data Streams" sections won't apply to
subdevs.

Most of what's written at the "Multiple Opens" section also don't
apply, as subdevs don't support streaming nor they accept the priority
mechanisms via VIDIOC_S_PRIORITY.

The notes at "Functions" section also don't apply to subdevs.

So, it is probably a lot easier to just create a new "open.rst"
for subdev API and remove all that doesn't apply - as proposed
in approach (3) (see below) - keeping the V4L2 one as-is.

In a matter of fact, most of the things explained at V4L2
part of the spec don't apply to subdevs. So, it is easier
to just tell what applies there (controls, events, format 
negotiation, crop/selection) than the opposite.

The dev-subdev.rst does a good job describing it. If we move
it from Documentation/media/uapi/v4l to Documentation/media/uapi/subdev,
all we need to do is to add chapters for the syscalls it
supports: open/close/ioctl, with can be a simplified version of what
is there at v4l, removing all things that don't apply to subdevs.

Re: [PATCH 2/2] v4l: cadence: Add Cadence MIPI-CSI2 TX driver

[Maxime Ripard]

Hi Sakari,

Sorry for the belated answer.

On Tue, Sep 26, 2017 at 08:00:14AM +, Sakari Ailus wrote:
> > On Fri, Sep 22, 2017 at 12:38:49PM +, Sakari Ailus wrote:
> > > > +   /*
> > > > +* Create a static mapping between the CSI virtual channels
> > > > +* and the input streams.
> > > 
> > > Which virtual channel is used here?
> > 
> > Like I was trying to explain in the cover letter, the virtual channel
> > is not under that block's control. The input video interfaces have an
> > additional signal that comes from the upstream device which sets the
> > virtual channel.
> 
> Oh, I missed while reviewing the set.
> 
> Presumably either driver would be in control of that then (this one or the
> upstream sub-device).

I don't really see how this driver could be under such control. I
guess it would depend on the integreation, but the upstream (sub-)
device is very likely to be under control of that signal, so I guess
it should be its role to change that. But we should also have that
information available so that the mixing in the CSI link is reported
properly in the media API (looking at Niklas' initial implementation).

> > > 
> > > > +*/
> > > > +   writel(CSI2TX_STREAM_IF_CFG_FILL_LEVEL(4),
> > > > +  csi2tx->base + CSI2TX_STREAM_IF_CFG_REG(stream));
> > > > +   }
> > > > +
> > > > +   /* Disable the configuration mode */
> > > > +   writel(0, csi2tx->base + CSI2TX_CONFIG_REG);
> > > 
> > > Shouldn't you start streaming on the downstream sub-device as well?
> > 
> > I appreciate it's a pretty weak argument, but the current setup we
> > have is in the FPGA is:
> > 
> > capture <- CSI2-RX <- CSI2-TX <- pattern generator
> > 
> > So far, the CSI2-RX block is calling its remote sub-device, which is
> > CSI2-TX. If CSI2-RX is calling its remote sub-device (CSI2-RX), we
> > just found ourselves in an endless loop.
> > 
> > I guess it should be easier, and fixable, when we'll have an actual
> > device without such a loopback.
> 
> What's the intended use case of the device, capture or output?

By device, you mean the CSI2-TX block, or something else?

If CSI2-TX, I guess it's more likely to be for output, but that might
be used for capture as well.

> How do you currently start the pipeline?

The capture device is the v4l2 device, and when the capture starts, it
enables the CSI2-RX which in turn enables CSI2-TX. The pattern
generator is enabled all the time.

> We have a few corner cases in V4L2 for such devices in graph parsing and
> stream control. The parsing of the device's fwnode graph endpoints are what
> the device can do, but it doesn't know where the parsing should continue
> and which part of the graph is already parsed.
> 
> That will be addressed but right now a driver just needs to know.

I'm not quite sure I got what you wanted me to fix or change.

Thanks!
Maxime

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


signature.asc
Description: PGP signature

Re: [PATCH v4 2/2] v4l: cadence: Add Cadence MIPI-CSI2 RX driver

[Maxime Ripard]

Hi Benoit,

On Fri, Sep 29, 2017 at 05:27:09PM +, Benoit Parrot wrote:
> > +static int csi2rx_get_resources(struct csi2rx_priv *csi2rx,
> > +   struct platform_device *pdev)
> > +{
> > +   struct resource *res;
> > +   unsigned char i;
> > +   u32 reg;
> > +
> > +   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +   csi2rx->base = devm_ioremap_resource(>dev, res);
> > +   if (IS_ERR(csi2rx->base))
> > +   return PTR_ERR(csi2rx->base);
> > +
> > +   csi2rx->sys_clk = devm_clk_get(>dev, "sys_clk");
> > +   if (IS_ERR(csi2rx->sys_clk)) {
> > +   dev_err(>dev, "Couldn't get sys clock\n");
> > +   return PTR_ERR(csi2rx->sys_clk);
> > +   }
> > +
> > +   csi2rx->p_clk = devm_clk_get(>dev, "p_clk");
> > +   if (IS_ERR(csi2rx->p_clk)) {
> > +   dev_err(>dev, "Couldn't get P clock\n");
> > +   return PTR_ERR(csi2rx->p_clk);
> > +   }
> > +
> > +   csi2rx->dphy = devm_phy_optional_get(>dev, "dphy");
> > +   if (IS_ERR(csi2rx->dphy)) {
> > +   dev_err(>dev, "Couldn't get external D-PHY\n");
> > +   return PTR_ERR(csi2rx->dphy);
> > +   }
> > +
> > +   /*
> > +* FIXME: Once we'll have external D-PHY support, the check
> > +* will need to be removed.
> > +*/
> > +   if (csi2rx->dphy) {
> > +   dev_err(>dev, "External D-PHY not supported yet\n");
> > +   return -EINVAL;
> > +   }
> 
> I understand that in your current environment you do not have a
> DPHY. But I am wondering in a real setup where you will have either
> an internal or an external DPHY, how are they going to interact with
> this driver or vice-versa?

It's difficult to give an answer with so little details. How would you
choose between those two PHYs? Is there a mux, or should we just power
one of the two? If that's the case, is there any use case were we
might want to power both? If not, which one should we favor, in which
situations?

I guess all those questions actually depend on the way the integration
has been done, and we're not quite there yet. I guess we could do
either a platform specific structure or a glue, depending on the
complexity. The platform specific compatible will allow us to do that
as we see fit anyway.

> > +
> > +   clk_prepare_enable(csi2rx->p_clk);
> > +   reg = readl(csi2rx->base + CSI2RX_DEVICE_CFG_REG);
> > +   clk_disable_unprepare(csi2rx->p_clk);
> > +
> > +   csi2rx->max_lanes = (reg & 7);
> > +   if (csi2rx->max_lanes > CSI2RX_LANES_MAX) {
> > +   dev_err(>dev, "Invalid number of lanes: %u\n",
> > +   csi2rx->max_lanes);
> > +   return -EINVAL;
> > +   }
> > +
> > +   csi2rx->max_streams = ((reg >> 4) & 7);
> > +   if (csi2rx->max_streams > CSI2RX_STREAMS_MAX) {
> > +   dev_err(>dev, "Invalid number of streams: %u\n",
> > +   csi2rx->max_streams);
> > +   return -EINVAL;
> > +   }
> > +
> > +   csi2rx->has_internal_dphy = (reg & BIT(3)) ? true : false;
> > +
> > +   /*
> > +* FIXME: Once we'll have internal D-PHY support, the check
> > +* will need to be removed.
> > +*/
> > +   if (csi2rx->has_internal_dphy) {
> > +   dev_err(>dev, "Internal D-PHY not supported yet\n");
> > +   return -EINVAL;
> > +   }
> > +
> > +   for (i = 0; i < csi2rx->max_streams; i++) {
> > +   char clk_name[16];
> > +
> > +   snprintf(clk_name, sizeof(clk_name), "pixel_if%u_clk", i);
> > +   csi2rx->pixel_clk[i] = devm_clk_get(>dev, clk_name);
> > +   if (IS_ERR(csi2rx->pixel_clk[i])) {
> > +   dev_err(>dev, "Couldn't get clock %s\n", 
> > clk_name);
> > +   return PTR_ERR(csi2rx->pixel_clk[i]);
> > +   }
> > +   }
> > +
> > +   return 0;
> > +}
> > +
> > +static int csi2rx_parse_dt(struct csi2rx_priv *csi2rx)
> > +{
> > +   struct v4l2_fwnode_endpoint v4l2_ep;
> > +   struct device_node *ep, *remote;
> 
> *remote is now unused.

It's fixed, thanks!
Maxime

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


signature.asc
Description: PGP signature

Re: [PATCH v7 5/7] media: open.rst: Adjust some terms to match the glossary

[Mauro Carvalho Chehab]

Em Wed, 11 Oct 2017 01:41:00 +0300
Sakari Ailus  escreveu:

> Hi Mauro,
> 
> On Tue, Oct 10, 2017 at 08:37:43AM -0300, Mauro Carvalho Chehab wrote:
> > Em Fri, 6 Oct 2017 15:48:22 +0300
> > Sakari Ailus  escreveu:
> >   
> > > Hi Mauro,
> > > 
> > > On Wed, Sep 27, 2017 at 07:23:47PM -0300, Mauro Carvalho Chehab wrote:  
> > > > As we now have a glossary, some terms used on open.rst
> > > > require adjustments.
> > > > 
> > > > Acked-by: Hans Verkuil 
> > > > Signed-off-by: Mauro Carvalho Chehab 
> > > > ---
> > > >  Documentation/media/uapi/v4l/open.rst | 12 ++--
> > > >  1 file changed, 6 insertions(+), 6 deletions(-)
> > > > 
> > > > diff --git a/Documentation/media/uapi/v4l/open.rst 
> > > > b/Documentation/media/uapi/v4l/open.rst
> > > > index f603bc9b49a0..0daf0c122c19 100644
> > > > --- a/Documentation/media/uapi/v4l/open.rst
> > > > +++ b/Documentation/media/uapi/v4l/open.rst
> > > > @@ -143,7 +143,7 @@ Related Devices
> > > >  Devices can support several functions. For example video capturing, VBI
> > > >  capturing and radio support.
> > > >  
> > > > -The V4L2 API creates different nodes for each of these functions.
> > > > +The V4L2 API creates different V4L2 device nodes for each of these 
> > > > functions.
> > > 
> > > A V4L2 device node is an instance of the V4L2 API. At the very least we
> > > should call them "V4L2 device node types", not device nodes only. This
> > > simply would suggests they're separate.  
> > 
> > OK, I added "types" there.
> >   
> > > 
> > > s/creates/defines/ ?  
> > 
> > It is meant to say create.
> > 
> > A device that supports both radio, video and VBI for the same V4L2
> > input will create three device nodes:
> > /dev/video0
> > /dev/radio0
> > /dev/vbi0
> > 
> > As all are associated to the same video input, and an ioctl send 
> > to one device may affect the other devices too, as they all associated
> > with the same hardware.  
> 
> Right. In this case I'd change the sentence. What would you think of this?
> 
> "Each of these functions is available via separate V4L2 device node."

Maybe replacing "is" by "should be" (or shall be?) would express the
requirement.

> 
> For it's not the V4L2 API that creates them. I failed to grasp what the
> original sentence meant. Was it about API, or framework, or were the
> devices nodes just separate or unlike as well?

Short answer:

>From uAPI PoV, it doesn't matter if it is the driver or the framework
that creates multiple device nodes.

What matters is that, if multiple types of capture/input are possible, 
multiple devnodes are created. Also, if one wants to control a radio
function, it should use /dev/radio, instead of /dev/video.

Long answer:

At kAPI, a typical register code, found on almost all non-webcam drivers is:

ret = video_register_device(>vdev, VFL_TYPE_GRABBER,
video_nr[dev->devno]);
if (ret)
goto error;

if (vbi_supported(dev)) {
ret = video_register_device(>vbi_dev, VFL_TYPE_VBI,
vbi_nr[dev->devno]);
if (ret)
goto error;
}

if (radio_supported(dev)) {
ret = video_register_device(>radio_dev, VFL_TYPE_RADIO,
radio_nr[dev->devno]);
if (ret)
goto error;
}

(note: it doesn't make sense to mention the above kAPI code at
 the uAPI documentation)


All tree devnodes are associated to the same capture or input device.

In the past, it was possible to use a /dev/radio to watch a video,
or /dev/video to listen radio, as they both corresponds to the same
V4L2 device, and the ioctl arguments used for radio and VBI are 
different. In other words, "/dev/radio" and "/dev/vbi" worked like a
sort of alias to the same device. Such support, however, required the
V4L2 core to do some tricks to identify the type of usage between
radio and video modes when handling tuner ioctls, like
VIDIOC_G_FREQUENCY/VIDIOC_S_FREQUENCY, making harder to prevent the
simultaneous usage of the device by a radio and a video application,
and causing some bugs (like returning a FM frequency on TV, or an
UHF frequency on radio).

So, several years ago, the API spec was modified to forbid such
usage, and support for switching the mode between radio/video mode
based on ioctl set usage was removed.

Yet, the v4l2 core use a coarse logic there: it only handles
differently stuff that are required to be different (like frequencies).
It won't check if, for example, a CTRL belongs to radio or video.
So, one can change a control using any device node, no matter if
the control belongs to video or radio.

So, in summary, the goal of the "Related Devices" at uAPI is to
mention that, if a V4L2 device supports multiple types, 

Re: [PATCH v2 08/12] intel-ipu3: params: compute and program ccs

[sakari.ai...@linux.intel.com]

On Wed, Oct 11, 2017 at 04:14:37AM +, Zhi, Yong wrote:
> Hi, Andy,
> 
> > -Original Message-
> > From: linux-media-ow...@vger.kernel.org [mailto:linux-media-
> > ow...@vger.kernel.org] On Behalf Of Andy Shevchenko
> > Sent: Friday, June 16, 2017 3:53 PM
> > To: Zhi, Yong 
> > Cc: Linux Media Mailing List ;
> > sakari.ai...@linux.intel.com; Zheng, Jian Xu ;
> > tf...@chromium.org; Mani, Rajmohan ;
> > Toivonen, Tuukka 
> > Subject: Re: [PATCH v2 08/12] intel-ipu3: params: compute and program ccs
> > 
> > On Thu, Jun 15, 2017 at 1:19 AM, Yong Zhi  wrote:
> > > A collection of routines that are mainly responsible to calculate the
> > > acc parameters.
> > 
> > > +static unsigned int ipu3_css_scaler_get_exp(unsigned int counter,
> > > +   unsigned int divider) {
> > > +   unsigned int i = 0;
> > > +
> > > +   while (counter <= divider / 2) {
> > > +   divider /= 2;
> > > +   i++;
> > > +   }
> > > +
> > > +   return i;
> > 
> > We have a lot of different helpers including one you may use instead of this
> > function.
> > 
> > It's *highly* recommended you learn what we have under lib/ (and not only
> > there) in kernel bewfore submitting a new version.
> > 
> 
> Tried to identify more places that could be re-implemented with lib
> helpers or more generic method, but we failed to spot any obvious
> candidate thus far.

How about:

return (!counter || divider < counter) ?
   0 : fls(divider / counter) - 1;

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

[PATCH v12 0/5] Add Rockchip RGA V4l2 support

[Jacob Chen]

change in V12:
- adding an entry to MAINTAINERS 
- fix checkpatch

change in V11:
- fix compile warning

change in V10:
- move to rockchip/rga
- changes according to comments
- some style changes

change in V9:
- remove protduff things
- test with the latest v4l2-compliance

change in V8:
- remove protduff things

change in V6,V7:
- correct warning in checkpatch.pl

change in V5:
- v4l2-compliance: handle invalid pxielformat
- v4l2-compliance: add subscribe_event
- add colorspace support

change in V4:
- document the controls.
- change according to Hans's comments

change in V3:
- rename the controls.
- add pm_runtime support.
- enable node by default.
- correct spelling in documents.

change in V2:
- generalize the controls.
- map buffers (10-50 us) in every cmd-run rather than in buffer-import to avoid 
get_free_pages failed on
actively used systems.
- remove status in dt-bindings examples.

Jacob Chen (5):
  dt-bindings: Document the Rockchip RGA bindings
  rockchip/rga: v4l2 m2m support
  MAINTAINERS: add entry for Rockchip RGA driver
  ARM: dts: rockchip: add RGA device node for RK3288
  arm64: dts: rockchip: add RGA device node for RK3399

 .../devicetree/bindings/media/rockchip-rga.txt |   33 +
 MAINTAINERS|7 +
 arch/arm/boot/dts/rk3288.dtsi  |   11 +
 arch/arm64/boot/dts/rockchip/rk3399.dtsi   |   11 +
 drivers/media/platform/Kconfig |   15 +
 drivers/media/platform/Makefile|2 +
 drivers/media/platform/rockchip/rga/Makefile   |3 +
 drivers/media/platform/rockchip/rga/rga-buf.c  |  154 +++
 drivers/media/platform/rockchip/rga/rga-hw.c   |  421 
 drivers/media/platform/rockchip/rga/rga-hw.h   |  437 +
 drivers/media/platform/rockchip/rga/rga.c  | 1012 
 drivers/media/platform/rockchip/rga/rga.h  |  125 +++
 12 files changed, 2231 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/rockchip-rga.txt
 create mode 100644 drivers/media/platform/rockchip/rga/Makefile
 create mode 100644 drivers/media/platform/rockchip/rga/rga-buf.c
 create mode 100644 drivers/media/platform/rockchip/rga/rga-hw.c
 create mode 100644 drivers/media/platform/rockchip/rga/rga-hw.h
 create mode 100644 drivers/media/platform/rockchip/rga/rga.c
 create mode 100644 drivers/media/platform/rockchip/rga/rga.h

-- 
2.14.1

[PATCH v12 3/5] MAINTAINERS: add entry for Rockchip RGA driver

[Jacob Chen]

Signed-off-by: Jacob Chen 
---
 MAINTAINERS | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 6671f375f7fc..335497bbc3f3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11509,6 +11509,13 @@ F: drivers/hid/hid-roccat*
 F: include/linux/hid-roccat*
 F: Documentation/ABI/*/sysfs-driver-hid-roccat*
 
+ROCKCHIP RASTER 2D GRAPHIC ACCELERATION UNIT DRIVER
+M: Jacob chen 
+L: linux-media@vger.kernel.org
+S: Maintained
+F: drivers/media/platform/rockchip/rga/
+F: Documentation/devicetree/bindings/media/rockchip-rga.txt
+
 ROCKER DRIVER
 M: Jiri Pirko 
 L: net...@vger.kernel.org
-- 
2.14.1

[PATCH v12 1/5] dt-bindings: Document the Rockchip RGA bindings

[Jacob Chen]

Add DT bindings documentation for Rockchip RGA

Signed-off-by: Jacob Chen 
Signed-off-by: Yakir Yang 
Acked-by: Rob Herring 
---
 .../devicetree/bindings/media/rockchip-rga.txt | 33 ++
 1 file changed, 33 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/rockchip-rga.txt

diff --git a/Documentation/devicetree/bindings/media/rockchip-rga.txt 
b/Documentation/devicetree/bindings/media/rockchip-rga.txt
new file mode 100644
index ..fd5276abfad6
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/rockchip-rga.txt
@@ -0,0 +1,33 @@
+device-tree bindings for rockchip 2D raster graphic acceleration controller 
(RGA)
+
+RGA is a standalone 2D raster graphic acceleration unit. It accelerates 2D
+graphics operations, such as point/line drawing, image scaling, rotation,
+BitBLT, alpha blending and image blur/sharpness.
+
+Required properties:
+- compatible: value should be one of the following
+   "rockchip,rk3288-rga";
+   "rockchip,rk3399-rga";
+
+- interrupts: RGA interrupt specifier.
+
+- clocks: phandle to RGA sclk/hclk/aclk clocks
+
+- clock-names: should be "aclk", "hclk" and "sclk"
+
+- resets: Must contain an entry for each entry in reset-names.
+  See ../reset/reset.txt for details.
+- reset-names: should be "core", "axi" and "ahb"
+
+Example:
+SoC-specific DT entry:
+   rga: rga@ff68 {
+   compatible = "rockchip,rk3399-rga";
+   reg = <0xff68 0x1>;
+   interrupts = ;
+   clocks = < ACLK_RGA>, < HCLK_RGA>, < SCLK_RGA_CORE>;
+   clock-names = "aclk", "hclk", "sclk";
+
+   resets = < SRST_RGA_CORE>, < SRST_A_RGA>, < 
SRST_H_RGA>;
+   reset-names = "core, "axi", "ahb";
+   };
-- 
2.14.1

[PATCH v12 5/5] arm64: dts: rockchip: add RGA device node for RK3399

[Jacob Chen]

This patch add the RGA dt config of RK3399 SoC.

Signed-off-by: Jacob Chen 
Signed-off-by: Yakir Yang 
---
 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index d79e9b3265b9..a3fab6af5a17 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -1204,6 +1204,17 @@
status = "disabled";
};
 
+   rga: rga@ff68 {
+   compatible = "rockchip,rk3399-rga";
+   reg = <0x0 0xff68 0x0 0x1>;
+   interrupts = ;
+   clocks = < ACLK_RGA>, < HCLK_RGA>, < SCLK_RGA_CORE>;
+   clock-names = "aclk", "hclk", "sclk";
+   resets = < SRST_RGA_CORE>, < SRST_A_RGA>, < 
SRST_H_RGA>;
+   reset-names = "core", "axi", "ahb";
+   power-domains = < RK3399_PD_RGA>;
+   };
+
efuse0: efuse@ff69 {
compatible = "rockchip,rk3399-efuse";
reg = <0x0 0xff69 0x0 0x80>;
-- 
2.14.1

[PATCH v12 2/5] rockchip/rga: v4l2 m2m support

[Jacob Chen]

Rockchip RGA is a separate 2D raster graphic acceleration unit. It
accelerates 2D graphics operations, such as point/line drawing, image
scaling, rotation, BitBLT, alpha blending and image blur/sharpness

The driver supports various operations from the rendering pipeline.
 - copy
 - fast solid color fill
 - rotation
 - flip
 - alpha blending

The code in rga-hw.c is used to configure regs according to operations
The code in rga-buf.c is used to create private mmu table for RGA.

Signed-off-by: Jacob Chen 
---
 drivers/media/platform/Kconfig|   15 +
 drivers/media/platform/Makefile   |2 +
 drivers/media/platform/rockchip/rga/Makefile  |3 +
 drivers/media/platform/rockchip/rga/rga-buf.c |  154 
 drivers/media/platform/rockchip/rga/rga-hw.c  |  421 ++
 drivers/media/platform/rockchip/rga/rga-hw.h  |  437 +++
 drivers/media/platform/rockchip/rga/rga.c | 1012 +
 drivers/media/platform/rockchip/rga/rga.h |  125 +++
 8 files changed, 2169 insertions(+)
 create mode 100644 drivers/media/platform/rockchip/rga/Makefile
 create mode 100644 drivers/media/platform/rockchip/rga/rga-buf.c
 create mode 100644 drivers/media/platform/rockchip/rga/rga-hw.c
 create mode 100644 drivers/media/platform/rockchip/rga/rga-hw.h
 create mode 100644 drivers/media/platform/rockchip/rga/rga.c
 create mode 100644 drivers/media/platform/rockchip/rga/rga.h

diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 7e7cc49b8674..3c6074ece15a 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -458,6 +458,21 @@ config VIDEO_RENESAS_VSP1
  To compile this driver as a module, choose M here: the module
  will be called vsp1.
 
+config VIDEO_ROCKCHIP_RGA
+   tristate "Rockchip Raster 2d Graphic Acceleration Unit"
+   depends on VIDEO_DEV && VIDEO_V4L2 && HAS_DMA
+   depends on ARCH_ROCKCHIP || COMPILE_TEST
+   select VIDEOBUF2_DMA_SG
+   select V4L2_MEM2MEM_DEV
+   default n
+   ---help---
+ This is a v4l2 driver for Rockchip SOC RGA 2d graphics accelerator.
+ Rockchip RGA is a separate 2D raster graphic acceleration unit.
+ It accelerates 2D graphics operations, such as point/line drawing,
+ image scaling, rotation, BitBLT, alpha blending and image 
blur/sharpness.
+
+ To compile this driver as a module choose m here.
+
 config VIDEO_TI_VPE
tristate "TI VPE (Video Processing Engine) driver"
depends on VIDEO_DEV && VIDEO_V4L2
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index c1ef946bf032..c179de1c98c3 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -62,6 +62,8 @@ obj-$(CONFIG_VIDEO_RENESAS_FDP1)  += rcar_fdp1.o
 obj-$(CONFIG_VIDEO_RENESAS_JPU)+= rcar_jpu.o
 obj-$(CONFIG_VIDEO_RENESAS_VSP1)   += vsp1/
 
+obj-$(CONFIG_VIDEO_ROCKCHIP_RGA)   += rockchip/rga/
+
 obj-y  += omap/
 
 obj-$(CONFIG_VIDEO_AM437X_VPFE)+= am437x/
diff --git a/drivers/media/platform/rockchip/rga/Makefile 
b/drivers/media/platform/rockchip/rga/Makefile
new file mode 100644
index ..92fe25490ccd
--- /dev/null
+++ b/drivers/media/platform/rockchip/rga/Makefile
@@ -0,0 +1,3 @@
+rockchip-rga-objs := rga.o rga-hw.o rga-buf.o
+
+obj-$(CONFIG_VIDEO_ROCKCHIP_RGA) += rockchip-rga.o
diff --git a/drivers/media/platform/rockchip/rga/rga-buf.c 
b/drivers/media/platform/rockchip/rga/rga-buf.c
new file mode 100644
index ..49cacc7a48d1
--- /dev/null
+++ b/drivers/media/platform/rockchip/rga/rga-buf.c
@@ -0,0 +1,154 @@
+/*
+ * Copyright (C) 2017 Fuzhou Rockchip Electronics Co.Ltd
+ * Author: Jacob Chen 
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "rga-hw.h"
+#include "rga.h"
+
+static int
+rga_queue_setup(struct vb2_queue *vq,
+   unsigned int *nbuffers, unsigned int *nplanes,
+   unsigned int sizes[], struct device *alloc_devs[])
+{
+   struct rga_ctx *ctx = vb2_get_drv_priv(vq);
+   struct rga_frame *f = rga_get_frame(ctx, vq->type);
+
+   if (IS_ERR(f))
+   return PTR_ERR(f);
+
+   if (*nplanes)
+   return sizes[0] < f->size ? -EINVAL : 0;
+
+   sizes[0] = f->size;
+   *nplanes = 1;
+
+   return 0;
+}
+
+static int rga_buf_prepare(struct vb2_buffer *vb)
+{
+   struct rga_ctx *ctx = 

[PATCH v12 4/5] ARM: dts: rockchip: add RGA device node for RK3288

[Jacob Chen]

This patch add the RGA dt config of rk3288 SoC.

Signed-off-by: Jacob Chen 
Signed-off-by: Yakir Yang 
---
 arch/arm/boot/dts/rk3288.dtsi | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
index a0a0b08bff74..8c6dfc0abc42 100644
--- a/arch/arm/boot/dts/rk3288.dtsi
+++ b/arch/arm/boot/dts/rk3288.dtsi
@@ -972,6 +972,17 @@
status = "disabled";
};
 
+   rga: rga@ff92 {
+   compatible = "rockchip,rk3288-rga";
+   reg = <0x0 0xff92 0x0 0x180>;
+   interrupts = ;
+   clocks = < ACLK_RGA>, < HCLK_RGA>, < SCLK_RGA>;
+   clock-names = "aclk", "hclk", "sclk";
+   power-domains = < RK3288_PD_VIO>;
+   resets = < SRST_RGA_CORE>, < SRST_RGA_AXI>, < 
SRST_RGA_AHB>;
+   reset-names = "core", "axi", "ahb";
+   };
+
vopb: vop@ff93 {
compatible = "rockchip,rk3288-vop";
reg = <0x0 0xff93 0x0 0x19c>;
-- 
2.14.1