Re: [PATCH v3 08/14] media: v4l: Add definitions for MPEG2 frame format and header metadata

[kbuild test robot]

Hi Florent,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on v4.17-rc5 next-20180516]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Paul-Kocialkowski/Sunxi-Cedrus-driver-for-the-Allwinner-Video-Engine-using-media-requests/20180508-004955
base:   git://linuxtv.org/media_tree.git master
config: x86_64-acpi-redef (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All warnings (new ones prefixed by >>):

>> ./usr/include/linux/v4l2-controls.h:1082: found __[us]{8,16,32,64} type 
>> without #include 

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


.config.gz
Description: application/gzip

[PATCH 1/3] dt-bindings: Add vendor prefix for AKM

[bingbu . cao]

From: Bingbu Cao 

Asahi Kasei Microdevices (AKM) offer a variety of
advanced sensing devices based on compound semiconductor
technology and sophisticated IC products featuring
analog/digital mixed-signal technology.

Signed-off-by: Bingbu Cao 
Signed-off-by: Tianshu Qiu 
---
 Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt 
b/Documentation/devicetree/bindings/vendor-prefixes.txt
index b5f978a4cac6..0e6159665e66 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -14,6 +14,7 @@ adh   AD Holdings Plc.
 adiAnalog Devices, Inc.
 advantech  Advantech Corporation
 aeroflexgaislerAeroflex Gaisler AB
+akmAsahi Kasei Microdevices
 al Annapurna Labs
 allo   Allo.com
 allwinner  Allwinner Technology Co., Ltd.
-- 
1.9.1

[PATCH 2/3] dt-bindings: Add bindings for AKM ak7375 voice coil lens

[bingbu . cao]

From: Bingbu Cao 

Add device tree bindings for AKM ak7375 voice coil lens
driver. This chip is used to drive a lens in a camera module.

Signed-off-by: Tianshu Qiu 
Signed-off-by: Bingbu Cao 
---
 Documentation/devicetree/bindings/media/i2c/akm,ak7375.txt | 8 
 1 file changed, 8 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/akm,ak7375.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/akm,ak7375.txt 
b/Documentation/devicetree/bindings/media/i2c/akm,ak7375.txt
new file mode 100644
index ..ec7f3c82dc57
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/akm,ak7375.txt
@@ -0,0 +1,8 @@
+Asahi Kasei Microdevices AK7375 voice coil lens driver
+
+AK7375 is a 12-bit DAC intended for controlling voice coil lenses.
+
+Mandatory properties:
+
+- compatible: "akm,ak7375"
+- reg: I2C slave address
-- 
1.9.1

[PATCH 3/3] media: ak7375: Add ak7375 lens voice coil driver

[bingbu . cao]

From: Bingbu Cao 

Add a V4L2 sub-device driver for the ak7375 lens voice coil.
This is a voice coil module using the I2C bus to control the
focus position.

Signed-off-by: Tianshu Qiu 
Signed-off-by: Bingbu Cao 
---
 MAINTAINERS|   8 ++
 drivers/media/i2c/Kconfig  |  10 ++
 drivers/media/i2c/Makefile |   1 +
 drivers/media/i2c/ak7375.c | 274 +
 4 files changed, 293 insertions(+)
 create mode 100644 drivers/media/i2c/ak7375.c

diff --git a/MAINTAINERS b/MAINTAINERS
index e73a55a6a855..7a8fea7b90fe 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -625,6 +625,14 @@ T: git git://linuxtv.org/anttip/media_tree.git
 S: Maintained
 F: drivers/media/usb/airspy/
 
+AKM AK7375 LENS VOICE COIL DRIVER
+M: Tianshu Qiu 
+L: linux-media@vger.kernel.org
+T: git git://linuxtv.org/media_tree.git
+S: Maintained
+F: drivers/media/i2c/ak7375.c
+F: Documentation/devicetree/bindings/media/i2c/akm,ak7375.txt
+
 ALACRITECH GIGABIT ETHERNET DRIVER
 M: Lino Sanfilippo 
 S: Maintained
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 1f9d7c6aa31a..92b7e9b1588c 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -326,6 +326,16 @@ config VIDEO_AD5820
  This is a driver for the AD5820 camera lens voice coil.
  It is used for example in Nokia N900 (RX-51).
 
+config VIDEO_AK7375
+   tristate "AK7375 lens voice coil support"
+   depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
+   depends on VIDEO_V4L2_SUBDEV_API
+   help
+ This is a driver for the AK7375 camera lens voice coil.
+ AK7375 is a 12 bit DAC with 120mA output current sink
+ capability. This is designed for linear control of
+ voice coil motors, controlled via I2C serial interface.
+
 config VIDEO_DW9714
tristate "DW9714 lens voice coil support"
depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 16fc34eda5cc..4261f66a29e6 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_VIDEO_SAA7127) += saa7127.o
 obj-$(CONFIG_VIDEO_SAA7185) += saa7185.o
 obj-$(CONFIG_VIDEO_SAA6752HS) += saa6752hs.o
 obj-$(CONFIG_VIDEO_AD5820)  += ad5820.o
+obj-$(CONFIG_VIDEO_AK7375)  += ak7375.o
 obj-$(CONFIG_VIDEO_DW9714)  += dw9714.o
 obj-$(CONFIG_VIDEO_DW9807)  += dw9807.o
 obj-$(CONFIG_VIDEO_ADV7170) += adv7170.o
diff --git a/drivers/media/i2c/ak7375.c b/drivers/media/i2c/ak7375.c
new file mode 100644
index ..7379fe35f4f4
--- /dev/null
+++ b/drivers/media/i2c/ak7375.c
@@ -0,0 +1,274 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2018 Intel Corporation
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define AK7375_MAX_FOCUS_POS   4095
+/*
+ * This sets the minimum granularity for the focus positions.
+ * A value of 1 gives maximum accuracy for a desired focus position
+ */
+#define AK7375_FOCUS_STEPS 1
+/*
+ * This acts as the minimum granularity of lens movement.
+ * Keep this value power of 2, so the control steps can be
+ * uniformly adjusted for gradual lens movement, with desired
+ * number of control steps.
+ */
+#define AK7375_CTRL_STEPS  64
+#define AK7375_CTRL_DELAY_US   1000
+
+#define AK7375_REG_POSITION0x0
+#define AK7375_REG_CONT0x2
+#define AK7375_MODE_ACTIVE 0x0
+#define AK7375_MODE_STANDBY0x40
+
+/* ak7375 device structure */
+struct ak7375_device {
+   struct v4l2_ctrl_handler ctrls_vcm;
+   struct v4l2_subdev sd;
+   struct v4l2_ctrl *focus;
+};
+
+static inline struct ak7375_device *to_ak7375_vcm(struct v4l2_ctrl *ctrl)
+{
+   return container_of(ctrl->handler, struct ak7375_device, ctrls_vcm);
+}
+
+static inline struct ak7375_device *sd_to_ak7375_vcm(struct v4l2_subdev 
*subdev)
+{
+   return container_of(subdev, struct ak7375_device, sd);
+}
+
+static int ak7375_i2c_write(struct ak7375_device *ak7375,
+   u8 addr, u16 data, int size)
+{
+   struct i2c_client *client = v4l2_get_subdevdata(>sd);
+   int ret;
+   u8 buf[3];
+
+   if (size != 1 && size != 2)
+   return -EINVAL;
+   buf[0] = addr;
+   buf[2] = data & 0xff;
+   if (size == 2)
+   buf[1] = data >> 8;
+   ret = i2c_master_send(client, (const char *)buf, size + 1);
+   if (ret < 0)
+   return ret;
+   if (ret != size + 1)
+   return -EIO;
+   return 0;
+}
+
+static int ak7375_set_ctrl(struct v4l2_ctrl *ctrl)
+{
+   struct ak7375_device *dev_vcm = to_ak7375_vcm(ctrl);
+
+   if (ctrl->id == V4L2_CID_FOCUS_ABSOLUTE)
+   return ak7375_i2c_write(dev_vcm, AK7375_REG_POSITION,
+   ctrl->val << 4, 2);
+
+   return 

[PATCH] media: imx319: Add imx319 camera sensor driver

[bingbu . cao]

From: Bingbu Cao 

Add a V4L2 sub-device driver for the Sony IMX319 image sensor.
This is a camera sensor using the I2C bus for control and the
CSI-2 bus for data.

Signed-off-by: Bingbu Cao 
Signed-off-by: Tianshu Qiu 
---
 MAINTAINERS|7 +
 drivers/media/i2c/Kconfig  |   11 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/imx319.c | 2433 
 4 files changed, 2452 insertions(+)
 create mode 100644 drivers/media/i2c/imx319.c

diff --git a/MAINTAINERS b/MAINTAINERS
index e73a55a6a855..87b6c338d827 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13084,6 +13084,13 @@ S: Maintained
 F: drivers/media/i2c/imx274.c
 F: Documentation/devicetree/bindings/media/i2c/imx274.txt
 
+SONY IMX319 SENSOR DRIVER
+M: Bingbu Cao 
+L: linux-media@vger.kernel.org
+T: git git://linuxtv.org/media_tree.git
+S: Maintained
+F: drivers/media/i2c/imx319.c
+
 SONY MEMORYSTICK CARD SUPPORT
 M: Alex Dubov 
 W: http://tifmxx.berlios.de/
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 1f9d7c6aa31a..c3d279cc293e 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -604,6 +604,17 @@ config VIDEO_IMX274
  This is a V4L2 sensor-level driver for the Sony IMX274
  CMOS image sensor.
 
+config VIDEO_IMX319
+   tristate "Sony IMX319 sensor support"
+   depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
+   depends on MEDIA_CAMERA_SUPPORT
+   help
+ This is a Video4Linux2 sensor driver for the Sony
+ IMX319 camera.
+
+ To compile this driver as a module, choose M here: the
+ module will be called imx319.
+
 config VIDEO_OV2640
tristate "OmniVision OV2640 sensor support"
depends on VIDEO_V4L2 && I2C
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 16fc34eda5cc..3adb3be4a486 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -104,5 +104,6 @@ obj-$(CONFIG_VIDEO_OV2659)  += ov2659.o
 obj-$(CONFIG_VIDEO_TC358743)   += tc358743.o
 obj-$(CONFIG_VIDEO_IMX258) += imx258.o
 obj-$(CONFIG_VIDEO_IMX274) += imx274.o
+obj-$(CONFIG_VIDEO_IMX319) += imx319.o
 
 obj-$(CONFIG_SDR_MAX2175) += max2175.o
diff --git a/drivers/media/i2c/imx319.c b/drivers/media/i2c/imx319.c
new file mode 100644
index ..e6a918ec9036
--- /dev/null
+++ b/drivers/media/i2c/imx319.c
@@ -0,0 +1,2433 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2018 Intel Corporation
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define IMX319_REG_MODE_SELECT 0x0100
+#define IMX319_MODE_STANDBY0x00
+#define IMX319_MODE_STREAMING  0x01
+
+/* Chip ID */
+#define IMX319_REG_CHIP_ID 0x0016
+#define IMX319_CHIP_ID 0x0319
+
+/* V_TIMING internal */
+#define IMX319_REG_FLL 0x0340
+#define IMX319_FLL_MAX 0x
+
+/* Exposure control */
+#define IMX319_REG_EXPOSURE0x0202
+#define IMX319_EXPOSURE_MIN1
+#define IMX319_EXPOSURE_STEP   1
+#define IMX319_EXPOSURE_DEFAULT0x04ee
+
+/* Analog gain control */
+#define IMX319_REG_ANALOG_GAIN 0x0204
+#define IMX319_ANA_GAIN_MIN0
+#define IMX319_ANA_GAIN_MAX960
+#define IMX319_ANA_GAIN_STEP   1
+#define IMX319_ANA_GAIN_DEFAULT0
+
+/* Digital gain control */
+#define IMX319_REG_DPGA_USE_GLOBAL_GAIN0x3ff9
+#define IMX319_REG_DIG_GAIN_GLOBAL 0x020e
+#define IMX319_DGTL_GAIN_MIN   256
+#define IMX319_DGTL_GAIN_MAX   4095
+#define IMX319_DGTL_GAIN_STEP  1
+#define IMX319_DGTL_GAIN_DEFAULT   256
+
+/* Test Pattern Control */
+#define IMX319_REG_TEST_PATTERN0x0600
+#define IMX319_TEST_PATTERN_DISABLED   0
+#define IMX319_TEST_PATTERN_SOLID_COLOR1
+#define IMX319_TEST_PATTERN_COLOR_BARS 2
+#define IMX319_TEST_PATTERN_GRAY_COLOR_BARS3
+#define IMX319_TEST_PATTERN_PN94
+
+/* Flip Control */
+#define IMX319_REG_ORIENTATION 0x0101
+
+struct imx319_reg {
+   u16 address;
+   u8 val;
+};
+
+struct imx319_reg_list {
+   u32 num_of_regs;
+   const struct imx319_reg *regs;
+};
+
+/* Mode : resolution and related config */
+struct imx319_mode {
+   /* Frame width */
+   u32 width;
+   /* Frame height */
+   u32 height;
+
+   /* V-timing */
+   u32 fll_def;
+   u32 fll_min;
+
+   /* H-timing */
+   u32 llp;
+
+   /* Default register values */
+   struct imx319_reg_list reg_list;
+};
+
+struct imx319 {
+   struct v4l2_subdev sd;
+   struct media_pad pad;
+
+   struct v4l2_ctrl_handler ctrl_handler;
+   /* V4L2 Controls */
+   struct v4l2_ctrl 

[PATCH] media: imx355: Add imx355 camera sensor driver

[bingbu . cao]

From: Bingbu Cao 

Add a V4L2 sub-device driver for the Sony IMX355 image sensor.
This is a camera sensor using the I2C bus for control and the
CSI-2 bus for data.

Signed-off-by: Tianshu Qiu 
Signed-off-by: Bingbu Cao 
---
 MAINTAINERS|7 +
 drivers/media/i2c/Kconfig  |   11 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/imx355.c | 1741 
 4 files changed, 1760 insertions(+)
 create mode 100644 drivers/media/i2c/imx355.c

diff --git a/MAINTAINERS b/MAINTAINERS
index e73a55a6a855..0921e9ae744e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13084,6 +13084,13 @@ S: Maintained
 F: drivers/media/i2c/imx274.c
 F: Documentation/devicetree/bindings/media/i2c/imx274.txt
 
+SONY IMX355 SENSOR DRIVER
+M: Tianshu Qiu 
+L: linux-media@vger.kernel.org
+T: git git://linuxtv.org/media_tree.git
+S: Maintained
+F: drivers/media/i2c/imx355.c
+
 SONY MEMORYSTICK CARD SUPPORT
 M: Alex Dubov 
 W: http://tifmxx.berlios.de/
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 1f9d7c6aa31a..505e7aa30b8d 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -604,6 +604,17 @@ config VIDEO_IMX274
  This is a V4L2 sensor-level driver for the Sony IMX274
  CMOS image sensor.
 
+config VIDEO_IMX355
+   tristate "Sony IMX355 sensor support"
+   depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
+   depends on MEDIA_CAMERA_SUPPORT
+   help
+ This is a Video4Linux2 sensor driver for the Sony
+ IMX355 camera.
+
+ To compile this driver as a module, choose M here: the
+ module will be called imx355.
+
 config VIDEO_OV2640
tristate "OmniVision OV2640 sensor support"
depends on VIDEO_V4L2 && I2C
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 16fc34eda5cc..dff2fb0dc3fa 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -104,5 +104,6 @@ obj-$(CONFIG_VIDEO_OV2659)  += ov2659.o
 obj-$(CONFIG_VIDEO_TC358743)   += tc358743.o
 obj-$(CONFIG_VIDEO_IMX258) += imx258.o
 obj-$(CONFIG_VIDEO_IMX274) += imx274.o
+obj-$(CONFIG_VIDEO_IMX355) += imx355.o
 
 obj-$(CONFIG_SDR_MAX2175) += max2175.o
diff --git a/drivers/media/i2c/imx355.c b/drivers/media/i2c/imx355.c
new file mode 100644
index ..6b936055acde
--- /dev/null
+++ b/drivers/media/i2c/imx355.c
@@ -0,0 +1,1741 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2017 - 2018 Intel Corporation
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define IMX355_REG_MODE_SELECT 0x0100
+#define IMX355_MODE_STANDBY0x00
+#define IMX355_MODE_STREAMING  0x01
+
+/* Chip ID */
+#define IMX355_REG_CHIP_ID 0x0016
+#define IMX355_CHIP_ID 0x0355
+
+/* V_TIMING internal */
+#define IMX355_REG_FLL 0x0340
+#define IMX355_FLL_MAX 0x
+
+/* Exposure control */
+#define IMX355_REG_EXPOSURE0x0202
+#define IMX355_EXPOSURE_MIN1
+#define IMX355_EXPOSURE_STEP   1
+#define IMX355_EXPOSURE_DEFAULT0x0282
+
+/* Analog gain control */
+#define IMX355_REG_ANALOG_GAIN 0x0204
+#define IMX355_ANA_GAIN_MIN0
+#define IMX355_ANA_GAIN_MAX960
+#define IMX355_ANA_GAIN_STEP   1
+#define IMX355_ANA_GAIN_DEFAULT0
+
+/* Digital gain control */
+#define IMX355_REG_DPGA_USE_GLOBAL_GAIN0x3070
+#define IMX355_REG_DIG_GAIN_GLOBAL 0x020e
+#define IMX355_DGTL_GAIN_MIN   256
+#define IMX355_DGTL_GAIN_MAX   4095
+#define IMX355_DGTL_GAIN_STEP  1
+#define IMX355_DGTL_GAIN_DEFAULT   256
+
+/* Test Pattern Control */
+#define IMX355_REG_TEST_PATTERN0x0600
+#define IMX355_TEST_PATTERN_DISABLED   0
+#define IMX355_TEST_PATTERN_SOLID_COLOR1
+#define IMX355_TEST_PATTERN_COLOR_BARS 2
+#define IMX355_TEST_PATTERN_GRAY_COLOR_BARS3
+#define IMX355_TEST_PATTERN_PN94
+
+/* Flip Control */
+#define IMX355_REG_ORIENTATION 0x0101
+
+struct imx355_reg {
+   u16 address;
+   u8 val;
+};
+
+struct imx355_reg_list {
+   u32 num_of_regs;
+   const struct imx355_reg *regs;
+};
+
+/* Mode : resolution and related config */
+struct imx355_mode {
+   /* Frame width */
+   u32 width;
+   /* Frame height */
+   u32 height;
+
+   /* V-timing */
+   u32 fll_def;
+   u32 fll_min;
+
+   /* H-timing */
+   u32 llp;
+
+   /* Default register values */
+   struct imx355_reg_list reg_list;
+};
+
+struct imx355 {
+   struct v4l2_subdev sd;
+   struct media_pad pad;
+
+   struct v4l2_ctrl_handler ctrl_handler;
+   /* V4L2 Controls */
+   struct v4l2_ctrl 

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 May 17 05:00:12 CEST 2018
media-tree git hash:7e6b6b945272c20f6b78d319e07f27897a8373c9
media_build git hash:   d72556c0502c096c089c99c58ee4a02a13133361
v4l-utils git hash: e2038ec6451293787b929338c2a671c732b8693d
gcc version:i686-linux-gcc (GCC) 8.1.0
sparse version: 0.5.2-RC1
smatch version: 0.5.1
host hardware:  x86_64
host os:4.15.0-3-amd64

linux-git-arm-at91: WARNINGS
linux-git-arm-davinci: OK
linux-git-arm-multi: WARNINGS
linux-git-arm-pxa: WARNINGS
linux-git-arm-stm32: OK
linux-git-arm64: WARNINGS
linux-git-i686: WARNINGS
linux-git-mips: OK
linux-git-powerpc64: WARNINGS
linux-git-sh: OK
linux-git-x86_64: WARNINGS
Check COMPILE_TEST: OK
linux-2.6.36.4-i686: ERRORS
linux-2.6.36.4-x86_64: ERRORS
linux-2.6.37.6-i686: ERRORS
linux-2.6.37.6-x86_64: ERRORS
linux-2.6.38.8-i686: ERRORS
linux-2.6.38.8-x86_64: ERRORS
linux-2.6.39.4-i686: ERRORS
linux-2.6.39.4-x86_64: ERRORS
linux-3.0.101-i686: ERRORS
linux-3.0.101-x86_64: ERRORS
linux-3.1.10-i686: ERRORS
linux-3.1.10-x86_64: ERRORS
linux-3.2.101-i686: ERRORS
linux-3.2.101-x86_64: ERRORS
linux-3.3.8-i686: ERRORS
linux-3.3.8-x86_64: ERRORS
linux-3.4.113-i686: ERRORS
linux-3.4.113-x86_64: ERRORS
linux-3.5.7-i686: ERRORS
linux-3.5.7-x86_64: ERRORS
linux-3.6.11-i686: ERRORS
linux-3.6.11-x86_64: ERRORS
linux-3.7.10-i686: ERRORS
linux-3.7.10-x86_64: ERRORS
linux-3.8.13-i686: ERRORS
linux-3.8.13-x86_64: ERRORS
linux-3.9.11-i686: ERRORS
linux-3.9.11-x86_64: ERRORS
linux-3.10.108-i686: ERRORS
linux-3.10.108-x86_64: ERRORS
linux-3.11.10-i686: ERRORS
linux-3.11.10-x86_64: ERRORS
linux-3.12.74-i686: ERRORS
linux-3.12.74-x86_64: ERRORS
linux-3.13.11-i686: ERRORS
linux-3.13.11-x86_64: ERRORS
linux-3.14.79-i686: ERRORS
linux-3.14.79-x86_64: ERRORS
linux-3.15.10-i686: ERRORS
linux-3.15.10-x86_64: ERRORS
linux-3.16.56-i686: ERRORS
linux-3.16.56-x86_64: ERRORS
linux-3.17.8-i686: ERRORS
linux-3.17.8-x86_64: ERRORS
linux-3.18.102-i686: ERRORS
linux-3.18.102-x86_64: ERRORS
linux-3.19.8-i686: ERRORS
linux-3.19.8-x86_64: ERRORS
linux-4.0.9-i686: ERRORS
linux-4.0.9-x86_64: ERRORS
linux-4.1.51-i686: ERRORS
linux-4.1.51-x86_64: ERRORS
linux-4.2.8-i686: ERRORS
linux-4.2.8-x86_64: ERRORS
linux-4.3.6-i686: ERRORS
linux-4.3.6-x86_64: ERRORS
linux-4.4.109-i686: ERRORS
linux-4.4.109-x86_64: ERRORS
linux-4.5.7-i686: ERRORS
linux-4.5.7-x86_64: ERRORS
linux-4.6.7-i686: ERRORS
linux-4.6.7-x86_64: ERRORS
linux-4.7.10-i686: ERRORS
linux-4.7.10-x86_64: ERRORS
linux-4.8.17-i686: ERRORS
linux-4.8.17-x86_64: ERRORS
linux-4.9.91-i686: ERRORS
linux-4.9.91-x86_64: ERRORS
linux-4.10.17-i686: ERRORS
linux-4.10.17-x86_64: ERRORS
linux-4.11.12-i686: ERRORS
linux-4.11.12-x86_64: ERRORS
linux-4.12.14-i686: ERRORS
linux-4.12.14-x86_64: ERRORS
linux-4.13.16-i686: ERRORS
linux-4.13.16-x86_64: ERRORS
linux-4.14.31-i686: WARNINGS
linux-4.14.31-x86_64: WARNINGS
linux-4.15.14-i686: WARNINGS
linux-4.15.14-x86_64: WARNINGS
linux-4.16.8-i686: WARNINGS
linux-4.16.8-x86_64: WARNINGS
linux-4.17-rc4-i686: WARNINGS
linux-4.17-rc4-x86_64: WARNINGS
apps: OK
spec-git: OK
sparse: WARNINGS

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] v4l: Add support for STD ioctls on subdev nodes

[Niklas Söderlund]

Signed-off-by: Niklas Söderlund 
---
 Documentation/media/uapi/v4l/vidioc-g-std.rst| 14 ++
 Documentation/media/uapi/v4l/vidioc-querystd.rst | 11 +++
 drivers/media/v4l2-core/v4l2-subdev.c| 12 
 include/uapi/linux/v4l2-subdev.h |  3 +++
 4 files changed, 32 insertions(+), 8 deletions(-)

---

Hi Hans,

I have tested this on Renesas Gen3 Salvator-XS M3-N using the AFE 
subdevice from the adv748x driver together with the R-Car VIN and CSI-2 
pipeline.  

I wrote a prototype patch for v4l2-ctl which adds three new options 
(--get-subdev-standard, --set-subdev-standard and 
--get-subdev-detected-standard) to ease testing which I plan to submit 
after some cleanup if this patch receives positive feedback.

If you or anyone else is interested in testing this patch the v4l2-utils 
prototype patches are available at

git://git.ragnatech.se/v4l-utils#subdev-std

Regards,
// Niklas

diff --git a/Documentation/media/uapi/v4l/vidioc-g-std.rst 
b/Documentation/media/uapi/v4l/vidioc-g-std.rst
index 90791ab51a5371b8..8d94f0404df270db 100644
--- a/Documentation/media/uapi/v4l/vidioc-g-std.rst
+++ b/Documentation/media/uapi/v4l/vidioc-g-std.rst
@@ -2,14 +2,14 @@
 
 .. _VIDIOC_G_STD:
 
-
-ioctl VIDIOC_G_STD, VIDIOC_S_STD
-
+**
+ioctl VIDIOC_G_STD, VIDIOC_S_STD, VIDIOC_SUBDEV_G_STD, VIDIOC_SUBDEV_S_STD
+**
 
 Name
 
 
-VIDIOC_G_STD - VIDIOC_S_STD - Query or select the video standard of the 
current input
+VIDIOC_G_STD - VIDIOC_S_STD - VIDIOC_SUBDEV_G_STD - VIDIOC_SUBDEV_S_STD - 
Query or select the video standard of the current input
 
 
 Synopsis
@@ -21,6 +21,12 @@ Synopsis
 .. c:function:: int ioctl( int fd, VIDIOC_S_STD, const v4l2_std_id *argp )
 :name: VIDIOC_S_STD
 
+.. c:function:: int ioctl( int fd, VIDIOC_SUBDEV_G_STD, v4l2_std_id *argp )
+:name: VIDIOC_SUBDEV_G_STD
+
+.. c:function:: int ioctl( int fd, VIDIOC_SUBDEV_S_STD, const v4l2_std_id 
*argp )
+:name: VIDIOC_SUBDEV_S_STD
+
 
 Arguments
 =
diff --git a/Documentation/media/uapi/v4l/vidioc-querystd.rst 
b/Documentation/media/uapi/v4l/vidioc-querystd.rst
index cf40bca19b9f8665..a8385cc7481869dd 100644
--- a/Documentation/media/uapi/v4l/vidioc-querystd.rst
+++ b/Documentation/media/uapi/v4l/vidioc-querystd.rst
@@ -2,14 +2,14 @@
 
 .. _VIDIOC_QUERYSTD:
 
-*
-ioctl VIDIOC_QUERYSTD
-*
+*
+ioctl VIDIOC_QUERYSTD, VIDIOC_SUBDEV_QUERYSTD
+*
 
 Name
 
 
-VIDIOC_QUERYSTD - Sense the video standard received by the current input
+VIDIOC_QUERYSTD - VIDIOC_SUBDEV_QUERYSTD - Sense the video standard received 
by the current input
 
 
 Synopsis
@@ -18,6 +18,9 @@ Synopsis
 .. c:function:: int ioctl( int fd, VIDIOC_QUERYSTD, v4l2_std_id *argp )
 :name: VIDIOC_QUERYSTD
 
+.. c:function:: int ioctl( int fd, VIDIOC_SUBDEV_QUERYSTD, v4l2_std_id *argp )
+:name: VIDIOC_SUBDEV_QUERYSTD
+
 
 Arguments
 =
diff --git a/drivers/media/v4l2-core/v4l2-subdev.c 
b/drivers/media/v4l2-core/v4l2-subdev.c
index f9eed938d3480b74..a156b1812e923721 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -494,6 +494,18 @@ static long subdev_do_ioctl(struct file *file, unsigned 
int cmd, void *arg)
 
case VIDIOC_SUBDEV_S_DV_TIMINGS:
return v4l2_subdev_call(sd, video, s_dv_timings, arg);
+
+   case VIDIOC_SUBDEV_G_STD:
+   return v4l2_subdev_call(sd, video, g_std, arg);
+
+   case VIDIOC_SUBDEV_S_STD: {
+   v4l2_std_id *std = arg;
+
+   return v4l2_subdev_call(sd, video, s_std, *std);
+   }
+
+   case VIDIOC_SUBDEV_QUERYSTD:
+   return v4l2_subdev_call(sd, video, querystd, arg);
 #endif
default:
return v4l2_subdev_call(sd, core, ioctl, cmd, arg);
diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h
index c95a53e6743cb040..133696a1f324ffdc 100644
--- a/include/uapi/linux/v4l2-subdev.h
+++ b/include/uapi/linux/v4l2-subdev.h
@@ -170,8 +170,11 @@ struct v4l2_subdev_selection {
 #define VIDIOC_SUBDEV_G_SELECTION  _IOWR('V', 61, struct 
v4l2_subdev_selection)
 #define VIDIOC_SUBDEV_S_SELECTION  _IOWR('V', 62, struct 
v4l2_subdev_selection)
 /* The following ioctls are identical to the ioctls in videodev2.h */
+#define VIDIOC_SUBDEV_G_STD_IOR('V', 23, v4l2_std_id)
+#define VIDIOC_SUBDEV_S_STD_IOW('V', 24, v4l2_std_id)
 #define VIDIOC_SUBDEV_G_EDID   _IOWR('V', 40, struct v4l2_edid)
 #define VIDIOC_SUBDEV_S_EDID   _IOWR('V', 41, struct v4l2_edid)
+#define 

Re: [PATCH 01/11] media: tm6000: fix potential Spectre variant 1

[Gustavo A. R. Silva]

On 05/15/2018 02:39 PM, Dan Carpenter wrote:

Dan,

These are all the Spectre media issues I see smatch is reporting in
linux-next-20180515:

drivers/media/cec/cec-pin-error-inj.c:170 cec_pin_error_inj_parse_line()
warn: potential spectre issue 'pin->error_inj_args'
drivers/media/dvb-core/dvb_ca_en50221.c:1479 dvb_ca_en50221_io_write() warn:
potential spectre issue 'ca->slot_info' (local cap)
drivers/media/dvb-core/dvb_net.c:252 handle_one_ule_extension() warn:
potential spectre issue 'p->ule_next_hdr'

I pulled the latest changes from the smatch repository and compiled it.

I'm running smatch v0.5.0-4459-g2f66d40 now. Is this the latest version?

I wonder if there is anything I might be missing.



You'd need to rebuild the db (possibly twice but definitely once).



Hi Dan,

After rebuilding the db (once), these are all the Spectre media warnings 
I get:


drivers/media/pci/ddbridge/ddbridge-core.c:233 ddb_redirect() warn: 
potential spectre issue 'ddbs'
drivers/media/pci/ddbridge/ddbridge-core.c:243 ddb_redirect() warn: 
potential spectre issue 'pdev->port'
drivers/media/pci/ddbridge/ddbridge-core.c:252 ddb_redirect() warn: 
potential spectre issue 'idev->input'
drivers/media/dvb-core/dvb_ca_en50221.c:1400 
dvb_ca_en50221_io_do_ioctl() warn: potential spectre issue 
'ca->slot_info' (local cap)
drivers/media/dvb-core/dvb_ca_en50221.c:1479 dvb_ca_en50221_io_write() 
warn: potential spectre issue 'ca->slot_info' (local cap)
drivers/media/dvb-core/dvb_net.c:252 handle_one_ule_extension() warn: 
potential spectre issue 'p->ule_next_hdr'
drivers/media/dvb-core/dvb_net.c:1483 dvb_net_do_ioctl() warn: potential 
spectre issue 'dvbnet->device' (local cap)
drivers/media/cec/cec-pin-error-inj.c:170 cec_pin_error_inj_parse_line() 
warn: potential spectre issue 'pin->error_inj_args'


I just want to double check if you are getting the same output. In case 
you are getting the same, then what Mauro commented about these issues:


https://patchwork.linuxtv.org/project/linux-media/list/?submitter=7277

being resolved by commit 3ad3b7a2ebaefae37a7eafed0779324987ca5e56 seems 
to be correct.


Thanks
--
Gustavo

Re: [PATCH v2] media: helene: add I2C device probe function

[Katsuhiro Suzuki]

Hello Abylay,

> -Original Message-
> From: Abylay Ospan 
> Sent: Wednesday, May 16, 2018 7:54 PM
> To: Suzuki, Katsuhiro/鈴木 勝博 
> Cc: Mauro Carvalho Chehab ; linux-media
> ; Masami Hiramatsu ;
> Jassi Brar ; linux-arm-ker...@lists.infradead.org;
> linux-ker...@vger.kernel.org
> Subject: Re: [PATCH v2] media: helene: add I2C device probe function
> 
> Hi Katsuhiro,
> 
> Thanks for patch.
> 
> What is the purpose to rework helene_set_params(_t|_s) ?
> other part of this patch looks ok for me, but not tested due to lack of spare 
> time ;(
> 

I'm using Socionext SC1501A (or MN88443x) ISDB-S/ISDB-T demodulator with this 
tuner. This demodulator has 1 port, and can use ISDB-S or ISDB-T exclusively.

So I think I cannot use ISDB-S features of this tuner if I use helene_attach(), 
and I cannot use ISDB-T if I use helene_attach_s() too.


Regards,
--
Katsuhiro Suzuki


> 
> 2018-05-16 4:37 GMT-04:00 Katsuhiro Suzuki   >:
> 
> 
>   This patch adds I2C probe function to use dvb_module_probe()
>   with this driver.
> 
>   Signed-off-by: Katsuhiro Suzuki   >
> 
>   ---
> 
>   Changes since v1:
> - Add documents for dvb_frontend member of helene_config
>   ---
>drivers/media/dvb-frontends/helene.c | 88 ++--
>drivers/media/dvb-frontends/helene.h |  3 +
>2 files changed, 87 insertions(+), 4 deletions(-)
> 
>   diff --git a/drivers/media/dvb-frontends/helene.c
> b/drivers/media/dvb-frontends/helene.c
>   index a0d0b53c91d7..04033f0c278b 100644
>   --- a/drivers/media/dvb-frontends/helene.c
>   +++ b/drivers/media/dvb-frontends/helene.c
>   @@ -666,7 +666,7 @@ static int helene_set_params_s(struct dvb_frontend 
> *fe)
>   return 0;
>}
> 
>   -static int helene_set_params(struct dvb_frontend *fe)
>   +static int helene_set_params_t(struct dvb_frontend *fe)
>{
>   u8 data[MAX_WRITE_REGSIZE];
>   u32 frequency;
>   @@ -835,6 +835,19 @@ static int helene_set_params(struct dvb_frontend 
> *fe)
>   return 0;
>}
> 
>   +static int helene_set_params(struct dvb_frontend *fe)
>   +{
>   +   struct dtv_frontend_properties *p = >dtv_property_cache;
>   +
>   +   if (p->delivery_system == SYS_DVBT ||
>   +   p->delivery_system == SYS_DVBT2 ||
>   +   p->delivery_system == SYS_ISDBT ||
>   +   p->delivery_system == SYS_DVBC_ANNEX_A)
>   +   return helene_set_params_t(fe);
>   +
>   +   return helene_set_params_s(fe);
>   +}
>   +
>static int helene_get_frequency(struct dvb_frontend *fe, u32 
> *frequency)
>{
>   struct helene_priv *priv = fe->tuner_priv;
>   @@ -843,7 +856,7 @@ static int helene_get_frequency(struct dvb_frontend 
> *fe,
> u32 *frequency)
>   return 0;
>}
> 
>   -static const struct dvb_tuner_ops helene_tuner_ops = {
>   +static const struct dvb_tuner_ops helene_tuner_ops_t = {
>   .info = {
>   .name = "Sony HELENE Ter tuner",
>   .frequency_min = 100,
>   @@ -853,7 +866,7 @@ static const struct dvb_tuner_ops helene_tuner_ops 
> = {
>   .init = helene_init,
>   .release = helene_release,
>   .sleep = helene_sleep,
>   -   .set_params = helene_set_params,
>   +   .set_params = helene_set_params_t,
>   .get_frequency = helene_get_frequency,
>};
> 
>   @@ -871,6 +884,20 @@ static const struct dvb_tuner_ops 
> helene_tuner_ops_s
> = {
>   .get_frequency = helene_get_frequency,
>};
> 
>   +static const struct dvb_tuner_ops helene_tuner_ops = {
>   +   .info = {
>   +   .name = "Sony HELENE Sat/Ter tuner",
>   +   .frequency_min = 50,
>   +   .frequency_max = 12,
>   +   .frequency_step = 1000,
>   +   },
>   +   .init = helene_init,
>   +   .release = helene_release,
>   +   .sleep = helene_sleep,
>   +   .set_params = helene_set_params,
>   +   .get_frequency = helene_get_frequency,
>   +};
>   +
>/* power-on tuner
> * call once after reset
> */
>   @@ -1032,7 +1059,7 @@ struct dvb_frontend *helene_attach(struct
> dvb_frontend *fe,
>   if (fe->ops.i2c_gate_ctrl)
>   fe->ops.i2c_gate_ctrl(fe, 0);
> 
>   -   memcpy(>ops.tuner_ops, _tuner_ops,
>   +   memcpy(>ops.tuner_ops, _tuner_ops_t,
> 

Re: [PATCH] media: helene: fix tuning frequency of satellite

[Katsuhiro Suzuki]

Hello Abylay,

> -Original Message-
> From: Abylay Ospan 
> Sent: Wednesday, May 16, 2018 7:56 PM
> To: Suzuki, Katsuhiro/鈴木 勝博 
> Cc: Mauro Carvalho Chehab ; linux-media
> ; Masami Hiramatsu ;
> Jassi Brar ; linux-arm-ker...@lists.infradead.org;
> linux-ker...@vger.kernel.org
> Subject: Re: [PATCH] media: helene: fix tuning frequency of satellite
> 
> True.
> I'm curious but how did it worked before ...
> Which hardware (dvb adapter) are you using ?
> 

I'm using evaluation boards of my company. So it's not exist in market...
Tuner module is SONY SUT-PJ series for ISDB-S/ISDB-T (not for DVB).

Regards,
--
Katsuhiro Suzuki


> 2018-05-16 4:41 GMT-04:00 Katsuhiro Suzuki   >:
> 
> 
>   This patch fixes tuning frequency of satellite to kHz. That as same
>   as terrestrial one.
> 
>   Signed-off-by: Katsuhiro Suzuki   >
>   ---
>drivers/media/dvb-frontends/helene.c | 2 +-
>1 file changed, 1 insertion(+), 1 deletion(-)
> 
>   diff --git a/drivers/media/dvb-frontends/helene.c
> b/drivers/media/dvb-frontends/helene.c
>   index 04033f0c278b..0a4f312c4368 100644
>   --- a/drivers/media/dvb-frontends/helene.c
>   +++ b/drivers/media/dvb-frontends/helene.c
>   @@ -523,7 +523,7 @@ static int helene_set_params_s(struct dvb_frontend 
> *fe)
>   enum helene_tv_system_t tv_system;
>   struct dtv_frontend_properties *p = >dtv_property_cache;
>   struct helene_priv *priv = fe->tuner_priv;
>   -   int frequencykHz = p->frequency;
>   +   int frequencykHz = p->frequency / 1000;
>   uint32_t frequency4kHz = 0;
>   u32 symbol_rate = p->symbol_rate/1000;
> 
>   --
>   2.17.0
> 
> 
> 
> 
> 
> 
> --
> 
> Abylay Ospan,
> NetUP Inc.
> http://www.netup.tv 

Re: [PATCH v9 4/8] media: vsp1: Convert display lists to use new body pool

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Thursday, 3 May 2018 16:35:43 EEST Kieran Bingham wrote:
> Adapt the dl->body0 object to use an object from the body pool. This
> greatly reduces the pressure on the TLB for IPMMU use cases, as all of
> the lists use a single allocation for the main body.
> 
> The CLU and LUT objects pre-allocate a pool containing three bodies,
> allowing a userspace update before the hardware has committed a previous
> set of tables.
> 
> Bodies are no longer 'freed' in interrupt context, but instead released
> back to their respective pools. This allows us to remove the garbage
> collector in the DLM.
> 
> Signed-off-by: Kieran Bingham 

Reviewed-by: Laurent Pinchart 

> ---
> v9:
>  - Remove redundant reference to gc_bodies
> 
> v8:
>  - Don't pass dlm->pool through vsp1_dl_list_alloc()  as it's already in the
> dlm. - Fix up comments
> 
> v4-v7:
>  - No changes (except rebases)
> 
> v3:
>  - 's/fragment/body', 's/fragments/bodies/'
>  - CLU/LUT now allocate 3 bodies
>  - vsp1_dl_list_fragments_free -> vsp1_dl_list_bodies_put
> 
> v2:
>  - Use dl->body0->max_entries to determine header offset, instead of the
>global constant VSP1_DL_NUM_ENTRIES which is incorrect.
>  - squash updates for LUT, CLU, and fragment cleanup into single patch.
>(Not fully bisectable when separated)
> ---
>  drivers/media/platform/vsp1/vsp1_clu.c |  27 ++-
>  drivers/media/platform/vsp1/vsp1_clu.h |   1 +-
>  drivers/media/platform/vsp1/vsp1_dl.c  | 221 ++
>  drivers/media/platform/vsp1/vsp1_dl.h  |   3 +-
>  drivers/media/platform/vsp1/vsp1_lut.c |  27 ++-
>  drivers/media/platform/vsp1/vsp1_lut.h |   1 +-
>  6 files changed, 100 insertions(+), 180 deletions(-)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_clu.c
> b/drivers/media/platform/vsp1/vsp1_clu.c index ebfbb915dcdc..8efa12f5e53f
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_clu.c
> +++ b/drivers/media/platform/vsp1/vsp1_clu.c
> @@ -19,6 +19,8 @@
>  #define CLU_MIN_SIZE 4U
>  #define CLU_MAX_SIZE 8190U
> 
> +#define CLU_SIZE (17 * 17 * 17)
> +
>  /*
> ---
> -- * Device Access
>   */
> @@ -43,19 +45,19 @@ static int clu_set_table(struct vsp1_clu *clu, struct
> v4l2_ctrl *ctrl) struct vsp1_dl_body *dlb;
>   unsigned int i;
> 
> - dlb = vsp1_dl_body_alloc(clu->entity.vsp1, 1 + 17 * 17 * 17);
> + dlb = vsp1_dl_body_get(clu->pool);
>   if (!dlb)
>   return -ENOMEM;
> 
>   vsp1_dl_body_write(dlb, VI6_CLU_ADDR, 0);
> - for (i = 0; i < 17 * 17 * 17; ++i)
> + for (i = 0; i < CLU_SIZE; ++i)
>   vsp1_dl_body_write(dlb, VI6_CLU_DATA, ctrl->p_new.p_u32[i]);
> 
>   spin_lock_irq(>lock);
>   swap(clu->clu, dlb);
>   spin_unlock_irq(>lock);
> 
> - vsp1_dl_body_free(dlb);
> + vsp1_dl_body_put(dlb);
>   return 0;
>  }
> 
> @@ -216,8 +218,16 @@ static void clu_configure(struct vsp1_entity *entity,
>   }
>  }
> 
> +static void clu_destroy(struct vsp1_entity *entity)
> +{
> + struct vsp1_clu *clu = to_clu(>subdev);
> +
> + vsp1_dl_body_pool_destroy(clu->pool);
> +}
> +
>  static const struct vsp1_entity_operations clu_entity_ops = {
>   .configure = clu_configure,
> + .destroy = clu_destroy,
>  };
> 
>  /*
> ---
> -- @@ -243,6 +253,17 @@ struct vsp1_clu *vsp1_clu_create(struct vsp1_device
> *vsp1) if (ret < 0)
>   return ERR_PTR(ret);
> 
> + /*
> +  * Pre-allocate a body pool, with 3 bodies allowing a userspace update
> +  * before the hardware has committed a previous set of tables, handling
> +  * both the queued and pending dl entries. One extra entry is added to
> +  * the CLU_SIZE to allow for the VI6_CLU_ADDR header.
> +  */
> + clu->pool = vsp1_dl_body_pool_create(clu->entity.vsp1, 3, CLU_SIZE + 1,
> +  0);
> + if (!clu->pool)
> + return ERR_PTR(-ENOMEM);
> +
>   /* Initialize the control handler. */
>   v4l2_ctrl_handler_init(>ctrls, 2);
>   v4l2_ctrl_new_custom(>ctrls, _table_control, NULL);
> diff --git a/drivers/media/platform/vsp1/vsp1_clu.h
> b/drivers/media/platform/vsp1/vsp1_clu.h index c45e6e707592..cef2f44481ba
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_clu.h
> +++ b/drivers/media/platform/vsp1/vsp1_clu.h
> @@ -32,6 +32,7 @@ struct vsp1_clu {
>   spinlock_t lock;
>   unsigned int mode;
>   struct vsp1_dl_body *clu;
> + struct vsp1_dl_body_pool *pool;
>  };
> 
>  static inline struct vsp1_clu *to_clu(struct v4l2_subdev *subdev)
> diff --git a/drivers/media/platform/vsp1/vsp1_dl.c
> b/drivers/media/platform/vsp1/vsp1_dl.c index 41ace89a585b..617c46a03dec
> 100644
> --- 

Re: [PATCH v9 3/8] media: vsp1: Provide a body pool

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Thursday, 3 May 2018 16:35:42 EEST Kieran Bingham wrote:
> Each display list allocates a body to store register values in a dma
> accessible buffer from a dma_alloc_wc() allocation. Each of these
> results in an entry in the IOMMU TLB, and a large number of display list
> allocations adds pressure to this resource.
> 
> Reduce TLB pressure on the IPMMUs by allocating multiple display list
> bodies in a single allocation, and providing these to the display list
> through a 'body pool'. A pool can be allocated by the display list
> manager or entities which require their own body allocations.
> 
> Signed-off-by: Kieran Bingham 

Reviewed-by: Laurent Pinchart 

> ---
> v8:
>  - Update commit message
>  - Fix comments and descriptions
> 
> v4:
>  - Provide comment explaining extra allocation on body pool
>highlighting area for optimisation later.
> 
> v3:
>  - s/fragment/body/, s/fragments/bodies/
>  - qty -> num_bodies
>  - indentation fix
>  - s/vsp1_dl_body_pool_{alloc,free}/vsp1_dl_body_pool_{create,destroy}/'
>  - Add kerneldoc to non-static functions
> 
> v2:
>  - assign dlb->dma correctly
> ---
>  drivers/media/platform/vsp1/vsp1_dl.c | 163 +++-
>  drivers/media/platform/vsp1/vsp1_dl.h |   8 +-
>  2 files changed, 171 insertions(+)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_dl.c
> b/drivers/media/platform/vsp1/vsp1_dl.c index 51965c30dec2..41ace89a585b
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_dl.c
> +++ b/drivers/media/platform/vsp1/vsp1_dl.c
> @@ -41,6 +41,8 @@ struct vsp1_dl_entry {
>  /**
>   * struct vsp1_dl_body - Display list body
>   * @list: entry in the display list list of bodies
> + * @free: entry in the pool free body list
> + * @pool: pool to which this body belongs
>   * @vsp1: the VSP1 device
>   * @entries: array of entries
>   * @dma: DMA address of the entries
> @@ -50,6 +52,9 @@ struct vsp1_dl_entry {
>   */
>  struct vsp1_dl_body {
>   struct list_head list;
> + struct list_head free;
> +
> + struct vsp1_dl_body_pool *pool;
>   struct vsp1_device *vsp1;
> 
>   struct vsp1_dl_entry *entries;
> @@ -61,6 +66,30 @@ struct vsp1_dl_body {
>  };
> 
>  /**
> + * struct vsp1_dl_body_pool - display list body pool
> + * @dma: DMA address of the entries
> + * @size: size of the full DMA memory pool in bytes
> + * @mem: CPU memory pointer for the pool
> + * @bodies: Array of DLB structures for the pool
> + * @free: List of free DLB entries
> + * @lock: Protects the free list
> + * @vsp1: the VSP1 device
> + */
> +struct vsp1_dl_body_pool {
> + /* DMA allocation */
> + dma_addr_t dma;
> + size_t size;
> + void *mem;
> +
> + /* Body management */
> + struct vsp1_dl_body *bodies;
> + struct list_head free;
> + spinlock_t lock;
> +
> + struct vsp1_device *vsp1;
> +};
> +
> +/**
>   * struct vsp1_dl_list - Display list
>   * @list: entry in the display list manager lists
>   * @dlm: the display list manager
> @@ -104,6 +133,7 @@ enum vsp1_dl_mode {
>   * @active: list currently being processed (loaded) by hardware
>   * @queued: list queued to the hardware (written to the DL registers)
>   * @pending: list waiting to be queued to the hardware
> + * @pool: body pool for the display list bodies
>   * @gc_work: bodies garbage collector work struct
>   * @gc_bodies: array of display list bodies waiting to be freed
>   */
> @@ -119,6 +149,8 @@ struct vsp1_dl_manager {
>   struct vsp1_dl_list *queued;
>   struct vsp1_dl_list *pending;
> 
> + struct vsp1_dl_body_pool *pool;
> +
>   struct work_struct gc_work;
>   struct list_head gc_bodies;
>  };
> @@ -127,6 +159,137 @@ struct vsp1_dl_manager {
>   * Display List Body Management
>   */
> 
> +/**
> + * vsp1_dl_body_pool_create - Create a pool of bodies from a single
> allocation + * @vsp1: The VSP1 device
> + * @num_bodies: The number of bodies to allocate
> + * @num_entries: The maximum number of entries that a body can contain
> + * @extra_size: Extra allocation provided for the bodies
> + *
> + * Allocate a pool of display list bodies each with enough memory to
> contain the + * requested number of entries plus the @extra_size.
> + *
> + * Return a pointer to a pool on success or NULL if memory can't be
> allocated. + */
> +struct vsp1_dl_body_pool *
> +vsp1_dl_body_pool_create(struct vsp1_device *vsp1, unsigned int num_bodies,
> +  unsigned int num_entries, size_t extra_size)
> +{
> + struct vsp1_dl_body_pool *pool;
> + size_t dlb_size;
> + unsigned int i;
> +
> + pool = kzalloc(sizeof(*pool), GFP_KERNEL);
> + if (!pool)
> + return NULL;
> +
> + pool->vsp1 = vsp1;
> +
> + /*
> +  * TODO: 'extra_size' is only used by vsp1_dlm_create(), to allocate
> +  * extra memory for the display list header. We need only one header per
> +  * display list, not per 

[PATCH] dt-bindings: media: rcar_vin: fix style for ports and endpoints

[Niklas Söderlund]

The style for referring to ports and endpoint are wrong. Refer to them
using lowercase and a unit address, port@x and endpoint@x.

Signed-off-by: Niklas Söderlund 
Reported-by: Geert Uytterhoeven 
---
 .../devicetree/bindings/media/rcar_vin.txt| 20 +--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt 
b/Documentation/devicetree/bindings/media/rcar_vin.txt
index c2c57dcf73f4851b..a574b9c037c05a3c 100644
--- a/Documentation/devicetree/bindings/media/rcar_vin.txt
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
@@ -45,23 +45,23 @@ The per-board settings Gen2 platforms:
 The per-board settings Gen3 platforms:
 
 Gen3 platforms can support both a single connected parallel input source
-from external SoC pins (port0) and/or multiple parallel input sources
-from local SoC CSI-2 receivers (port1) depending on SoC.
+from external SoC pins (port@0) and/or multiple parallel input sources
+from local SoC CSI-2 receivers (port@1) depending on SoC.
 
 - renesas,id - ID number of the VIN, VINx in the documentation.
 - ports
-- port 0 - sub-node describing a single endpoint connected to the VIN
+- port@0 - sub-node describing a single endpoint connected to the VIN
   from external SoC pins described in video-interfaces.txt[1].
-  Describing more then one endpoint in port 0 is invalid. Only VIN
-  instances that are connected to external pins should have port 0.
-- port 1 - sub-nodes describing one or more endpoints connected to
+  Describing more then one endpoint in port@0 is invalid. Only VIN
+  instances that are connected to external pins should have port@0.
+- port@1 - sub-nodes describing one or more endpoints connected to
   the VIN from local SoC CSI-2 receivers. The endpoint numbers must
   use the following schema.
 
-- Endpoint 0 - sub-node describing the endpoint connected to CSI20
-- Endpoint 1 - sub-node describing the endpoint connected to CSI21
-- Endpoint 2 - sub-node describing the endpoint connected to CSI40
-- Endpoint 3 - sub-node describing the endpoint connected to CSI41
+- endpoint@0 - sub-node describing the endpoint connected to CSI20
+- endpoint@1 - sub-node describing the endpoint connected to CSI21
+- endpoint@2 - sub-node describing the endpoint connected to CSI40
+- endpoint@3 - sub-node describing the endpoint connected to CSI41
 
 Device node example for Gen2 platforms
 --
-- 
2.17.0

[PATCH] rcar-vin: sync which hardware buffer to start capture from

[Niklas Söderlund]

When starting the VIN capture procedure we are not guaranteed that the
first buffer writing to is VnMB1 to which we assigned the first buffer
queued. This is problematic for two reasons. Buffers might not be
dequeued in the same order they where queued for capture. Future
features planed for the VIN driver is support for outputing frames in
SEQ_TB/BT format and to do that it's important that capture starts from
the first buffer slot, VnMB1.

We are guaranteed that capturing always happens in sequence (VnMB1 ->
VnMB2 -> VnMB3 -> VnMB1). So drop up to two frames when starting
capturing so that the driver always returns buffers in the same order
they are queued and prepare for SEQ_TB/BT output.

Signed-off-by: Niklas Söderlund 
---
 drivers/media/platform/rcar-vin/rcar-dma.c | 16 +++-
 drivers/media/platform/rcar-vin/rcar-vin.h |  2 ++
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c 
b/drivers/media/platform/rcar-vin/rcar-dma.c
index ac07f99e3516a620..cfe5d7a9d44ee0e1 100644
--- a/drivers/media/platform/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/rcar-vin/rcar-dma.c
@@ -856,7 +856,7 @@ static int rvin_capture_start(struct rvin_dev *vin)
/* Continuous Frame Capture Mode */
rvin_write(vin, VNFC_C_FRAME, VNFC_REG);
 
-   vin->state = RUNNING;
+   vin->state = STARTING;
 
return 0;
 }
@@ -910,6 +910,20 @@ static irqreturn_t rvin_irq(int irq, void *data)
vnms = rvin_read(vin, VNMS_REG);
slot = (vnms & VNMS_FBS_MASK) >> VNMS_FBS_SHIFT;
 
+   /*
+* To hand buffers back in a known order to userspace start
+* to capture first from slot 0.
+*/
+   if (vin->state == STARTING) {
+   if (slot != 0) {
+   vin_dbg(vin, "Starting sync slot: %d\n", slot);
+   goto done;
+   }
+
+   vin_dbg(vin, "Capture start synced!\n");
+   vin->state = RUNNING;
+   }
+
/* Capture frame */
if (vin->queue_buf[slot]) {
vin->queue_buf[slot]->field = vin->format.field;
diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h 
b/drivers/media/platform/rcar-vin/rcar-vin.h
index c2aef789b491ab31..ff747e22d8cfb643 100644
--- a/drivers/media/platform/rcar-vin/rcar-vin.h
+++ b/drivers/media/platform/rcar-vin/rcar-vin.h
@@ -53,11 +53,13 @@ enum rvin_csi_id {
 
 /**
  * STOPPED  - No operation in progress
+ * STARTING - Capture starting up
  * RUNNING  - Operation in progress have buffers
  * STOPPING - Stopping operation
  */
 enum rvin_dma_state {
STOPPED = 0,
+   STARTING,
RUNNING,
STOPPING,
 };
-- 
2.17.0

[PATCH] rcar-csi2: set default format if a unsupported one is requested

[Niklas Söderlund]

Instead of failing the set_fmt() if a unsupported format is requested
set a default one and return the changed format to the user.

Signed-off-by: Niklas Söderlund 
Reported-by: Sakari Ailus 
---
 drivers/media/platform/rcar-vin/rcar-csi2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c 
b/drivers/media/platform/rcar-vin/rcar-csi2.c
index e64f07fe184e7675..daef72d410a3425d 100644
--- a/drivers/media/platform/rcar-vin/rcar-csi2.c
+++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
@@ -613,7 +613,7 @@ static int rcsi2_set_pad_format(struct v4l2_subdev *sd,
struct v4l2_mbus_framefmt *framefmt;
 
if (!rcsi2_code_to_fmt(format->format.code))
-   return -EINVAL;
+   format->format.code = rcar_csi2_formats[0].code;
 
if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
priv->mf = format->format;
-- 
2.17.0

Proposal

[Miss Zeliha Omer Faruk]

Hello

Greetings to you please i have a business proposal for you contact me
for more detailes asap thanks.

Best Regards,
Miss.Zeliha ömer faruk
Esentepe Mahallesi Büyükdere
Caddesi Kristal Kule Binasi
No:215
Sisli - Istanbul, Turkey

Re: [PATCH 6/6] ARM: dts: rcar-gen2: Add 'data-active' property

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2018-05-16 18:32:32 +0200, Jacopo Mondi wrote:
> The 'data-active' property needs to be specified when using embedded
> synchronization. Add it to the Gen-2 boards using composite video input.
> 
> Signed-off-by: Jacopo Mondi 
> ---
>  arch/arm/boot/dts/r8a7790-lager.dts   | 1 +
>  arch/arm/boot/dts/r8a7791-koelsch.dts | 1 +
>  arch/arm/boot/dts/r8a7791-porter.dts  | 1 +
>  arch/arm/boot/dts/r8a7793-gose.dts| 1 +
>  arch/arm/boot/dts/r8a7794-alt.dts | 1 +
>  arch/arm/boot/dts/r8a7794-silk.dts| 1 +
>  6 files changed, 6 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/r8a7790-lager.dts 
> b/arch/arm/boot/dts/r8a7790-lager.dts
> index b56b309..48fcb44 100644
> --- a/arch/arm/boot/dts/r8a7790-lager.dts
> +++ b/arch/arm/boot/dts/r8a7790-lager.dts
> @@ -893,6 +893,7 @@
> 
>   vin1ep0: endpoint {
>   remote-endpoint = <>;
> + data-active = <1>;
>   };
>   };
>  };
> diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts 
> b/arch/arm/boot/dts/r8a7791-koelsch.dts
> index 9967666..fa0b25f 100644
> --- a/arch/arm/boot/dts/r8a7791-koelsch.dts
> +++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
> @@ -868,6 +868,7 @@
> 
>   vin1ep: endpoint {
>   remote-endpoint = <>;
> + data-active = <1>;

Depending on how we interpret the data-active property this can be good 
or bad. But if we interpret it as the polarity of the VIn_CLKENB pin 
this is not good as this is not connected for the adv7180 on Koelsch.

I have not checked all the Gen2 schematics as I'm still not sure how to 
interpret the property.


>   };
>   };
>  };
> diff --git a/arch/arm/boot/dts/r8a7791-porter.dts 
> b/arch/arm/boot/dts/r8a7791-porter.dts
> index 055a7f1..96b9605 100644
> --- a/arch/arm/boot/dts/r8a7791-porter.dts
> +++ b/arch/arm/boot/dts/r8a7791-porter.dts
> @@ -391,6 +391,7 @@
> 
>   vin0ep: endpoint {
>   remote-endpoint = <>;
> + data-active = <1>;
>   };
>   };
>  };
> diff --git a/arch/arm/boot/dts/r8a7793-gose.dts 
> b/arch/arm/boot/dts/r8a7793-gose.dts
> index 9d3fba2..80b4fa8 100644
> --- a/arch/arm/boot/dts/r8a7793-gose.dts
> +++ b/arch/arm/boot/dts/r8a7793-gose.dts
> @@ -779,6 +779,7 @@
> 
>   vin1ep: endpoint {
>   remote-endpoint = <_out>;
> + data-active = <1>;
>   };
>   };
>  };
> diff --git a/arch/arm/boot/dts/r8a7794-alt.dts 
> b/arch/arm/boot/dts/r8a7794-alt.dts
> index 4bbb9cc..00df605d 100644
> --- a/arch/arm/boot/dts/r8a7794-alt.dts
> +++ b/arch/arm/boot/dts/r8a7794-alt.dts
> @@ -380,6 +380,7 @@
> 
>   vin0ep: endpoint {
>   remote-endpoint = <>;
> + data-active = <1>;
>   };
>   };
>  };
> diff --git a/arch/arm/boot/dts/r8a7794-silk.dts 
> b/arch/arm/boot/dts/r8a7794-silk.dts
> index c0c5d31..ed17376 100644
> --- a/arch/arm/boot/dts/r8a7794-silk.dts
> +++ b/arch/arm/boot/dts/r8a7794-silk.dts
> @@ -480,6 +480,7 @@
> 
>   vin0ep: endpoint {
>   remote-endpoint = <>;
> + data-active = <1>;
>   };
>   };
>  };
> --
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH 5/6] ARM: dts: rcar-gen2: Remove unused VIN properties

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2018-05-16 18:32:31 +0200, Jacopo Mondi wrote:
> The 'bus-width' and 'pclk-sample' properties are not parsed by the VIN
> driver and only confuse users. Remove them in all Gen2 SoC that used
> them.
> 
> Signed-off-by: Jacopo Mondi 
> ---
>  arch/arm/boot/dts/r8a7790-lager.dts   | 3 ---
>  arch/arm/boot/dts/r8a7791-koelsch.dts | 3 ---
>  arch/arm/boot/dts/r8a7791-porter.dts  | 1 -
>  arch/arm/boot/dts/r8a7793-gose.dts| 3 ---
>  arch/arm/boot/dts/r8a7794-alt.dts | 1 -
>  arch/arm/boot/dts/r8a7794-silk.dts| 1 -
>  6 files changed, 12 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/r8a7790-lager.dts 
> b/arch/arm/boot/dts/r8a7790-lager.dts
> index 063fdb6..b56b309 100644
> --- a/arch/arm/boot/dts/r8a7790-lager.dts
> +++ b/arch/arm/boot/dts/r8a7790-lager.dts
> @@ -873,10 +873,8 @@
>   port {
>   vin0ep2: endpoint {
>   remote-endpoint = <_out>;
> - bus-width = <24>;

I can't really make up my mind if this is a good thing or not. Device 
tree describes the hardware and not what the drivers make use of. And 
the fact is that this bus is 24 bits wide. So I'm not sure we should 
remove these properties. But I would love to hear what others think 
about this.

>   hsync-active = <0>;
>   vsync-active = <0>;
> - pclk-sample = <1>;
>   data-active = <1>;
>   };
>   };
> @@ -895,7 +893,6 @@
> 
>   vin1ep0: endpoint {
>   remote-endpoint = <>;
> - bus-width = <8>;
>   };
>   };
>  };
> diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts 
> b/arch/arm/boot/dts/r8a7791-koelsch.dts
> index f40321a..9967666 100644
> --- a/arch/arm/boot/dts/r8a7791-koelsch.dts
> +++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
> @@ -849,10 +849,8 @@
> 
>   vin0ep2: endpoint {
>   remote-endpoint = <_out>;
> - bus-width = <24>;
>   hsync-active = <0>;
>   vsync-active = <0>;
> - pclk-sample = <1>;
>   data-active = <1>;
>   };
>   };
> @@ -870,7 +868,6 @@
> 
>   vin1ep: endpoint {
>   remote-endpoint = <>;
> - bus-width = <8>;
>   };
>   };
>  };
> diff --git a/arch/arm/boot/dts/r8a7791-porter.dts 
> b/arch/arm/boot/dts/r8a7791-porter.dts
> index c14e6fe..055a7f1 100644
> --- a/arch/arm/boot/dts/r8a7791-porter.dts
> +++ b/arch/arm/boot/dts/r8a7791-porter.dts
> @@ -391,7 +391,6 @@
> 
>   vin0ep: endpoint {
>   remote-endpoint = <>;
> - bus-width = <8>;
>   };
>   };
>  };
> diff --git a/arch/arm/boot/dts/r8a7793-gose.dts 
> b/arch/arm/boot/dts/r8a7793-gose.dts
> index 9ed6961..9d3fba2 100644
> --- a/arch/arm/boot/dts/r8a7793-gose.dts
> +++ b/arch/arm/boot/dts/r8a7793-gose.dts
> @@ -759,10 +759,8 @@
> 
>   vin0ep2: endpoint {
>   remote-endpoint = <_out>;
> - bus-width = <24>;
>   hsync-active = <0>;
>   vsync-active = <0>;
> - pclk-sample = <1>;
>   data-active = <1>;
>   };
>   };
> @@ -781,7 +779,6 @@
> 
>   vin1ep: endpoint {
>   remote-endpoint = <_out>;
> - bus-width = <8>;
>   };
>   };
>  };
> diff --git a/arch/arm/boot/dts/r8a7794-alt.dts 
> b/arch/arm/boot/dts/r8a7794-alt.dts
> index 26a8834..4bbb9cc 100644
> --- a/arch/arm/boot/dts/r8a7794-alt.dts
> +++ b/arch/arm/boot/dts/r8a7794-alt.dts
> @@ -380,7 +380,6 @@
> 
>   vin0ep: endpoint {
>   remote-endpoint = <>;
> - bus-width = <8>;
>   };
>   };
>  };
> diff --git a/arch/arm/boot/dts/r8a7794-silk.dts 
> b/arch/arm/boot/dts/r8a7794-silk.dts
> index 351cb3b..c0c5d31 100644
> --- a/arch/arm/boot/dts/r8a7794-silk.dts
> +++ b/arch/arm/boot/dts/r8a7794-silk.dts
> @@ -480,7 +480,6 @@
> 
>   vin0ep: endpoint {
>   remote-endpoint = <>;
> - bus-width = <8>;
>   };
>   };
>  };
> --
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH 4/6] media: rcar-vin: Handle CLOCKENB pin polarity

[Niklas Söderlund]

Hi Jacopo,

Thanks for your patch.

I'm happy that you dig into this as it clearly needs doing!

On 2018-05-16 18:32:30 +0200, Jacopo Mondi wrote:
> Handle CLOCKENB pin polarity, or use HSYNC in its place if polarity is
> not specified.
> 
> Signed-off-by: Jacopo Mondi 
> ---
>  drivers/media/platform/rcar-vin/rcar-dma.c | 11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c 
> b/drivers/media/platform/rcar-vin/rcar-dma.c
> index ac07f99..7a84eae 100644
> --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> @@ -123,6 +123,8 @@
>  /* Video n Data Mode Register 2 bits */
>  #define VNDMR2_VPS   (1 << 30)
>  #define VNDMR2_HPS   (1 << 29)
> +#define VNDMR2_CES   (1 << 28)
> +#define VNDMR2_CHS   (1 << 23)
>  #define VNDMR2_FTEV  (1 << 17)
>  #define VNDMR2_VLV(n)((n & 0xf) << 12)
>  
> @@ -691,6 +693,15 @@ static int rvin_setup(struct rvin_dev *vin)
>   dmr2 |= VNDMR2_VPS;
>  
>   /*
> +  * Clock-enable active level select.
> +  * Use HSYNC as enable if not specified
> +  */
> + if (vin->mbus_cfg.flags & V4L2_MBUS_DATA_ACTIVE_LOW)
> + dmr2 |= VNDMR2_CES;
> + else if (!(vin->mbus_cfg.flags & V4L2_MBUS_DATA_ACTIVE_HIGH))
> + dmr2 |= VNDMR2_CHS;

After studying the datasheet for a while I'm getting more and more 
convinced this should be (with context leftout in this patch context) 
something like this:

/* Hsync Signal Polarity Select */
if (!(vin->mbus_cfg.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW))
dmr2 |= VNDMR2_HPS;

/* Vsync Signal Polarity Select */
if (!(vin->mbus_cfg.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW))
dmr2 |= VNDMR2_VPS;

/* Clock Enable Signal Polarity Select */
if (!(vin->mbus_cfg.flags & V4L2_MBUS_DATA_ACTIVE_LOW))
dmr2 |= VNDMR2_CES;

/* Use HSYNC as clock enable if VIn_CLKENB is not available. */
if (!(vin->mbus_cfg.flags & (V4L2_MBUS_DATA_ACTIVE_LOW | 
V4L2_MBUS_DATA_ACTIVE_HIGH)))
dmr2 |= VNDMR2_CHS;

Or am I misunderstanding something?

> +
> + /*
>* Output format
>*/
>   switch (vin->format.pixelformat) {
> -- 
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH 3/6] media: rcar-vin: Handle data-active property

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2018-05-16 18:32:29 +0200, Jacopo Mondi wrote:
> The data-active property has to be specified when running with embedded
> synchronization. The VIN peripheral can use HSYNC in place of CLOCKENB
> when the CLOCKENB pin is not connected, this requires explicit
> synchronization to be in use.

Is this really the intent of the data-active property? I read the 
video-interfaces.txt document as such as if no hsync-active, 
vsync-active and data-active we should use the embedded synchronization 
else set the polarity for the requested pins. What am I not 
understanding here?

> 
> Now that the driver supports 'data-active' property, it makes not sense
> to zero the mbus configuration flags when running with implicit synch
> (V4L2_MBUS_BT656).
> 
> Signed-off-by: Jacopo Mondi 
> ---
>  drivers/media/platform/rcar-vin/rcar-core.c | 10 --
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c 
> b/drivers/media/platform/rcar-vin/rcar-core.c
> index d3072e1..075d08f 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -531,15 +531,21 @@ static int rvin_digital_parse_v4l2(struct device *dev,
>   return -ENOTCONN;
>  
>   vin->mbus_cfg.type = vep->bus_type;
> + vin->mbus_cfg.flags = vep->bus.parallel.flags;
>  
>   switch (vin->mbus_cfg.type) {
>   case V4L2_MBUS_PARALLEL:
>   vin_dbg(vin, "Found PARALLEL media bus\n");
> - vin->mbus_cfg.flags = vep->bus.parallel.flags;
>   break;
>   case V4L2_MBUS_BT656:
>   vin_dbg(vin, "Found BT656 media bus\n");
> - vin->mbus_cfg.flags = 0;
> +
> + if (!(vin->mbus_cfg.flags & V4L2_MBUS_DATA_ACTIVE_HIGH) &&
> + !(vin->mbus_cfg.flags & V4L2_MBUS_DATA_ACTIVE_LOW)) {
> + vin_err(vin,
> + "Missing data enable signal polarity 
> property\n");

I fear this can't be an error as that would break backward comp ability 
with existing dtb's.

> + return -EINVAL;
> + }
>   break;
>   default:
>   vin_err(vin, "Unknown media bus type\n");
> -- 
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH 2/6] dt-bindings: media: rcar-vin: Document data-active

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2018-05-16 18:32:28 +0200, Jacopo Mondi wrote:
> Document 'data-active' property in R-Car VIN device tree bindings.
> The property is optional when running with explicit synchronization
> (eg. BT.601) but mandatory when embedded synchronization is in use (eg.
> BT.656) as specified by the hardware manual.
> 
> Signed-off-by: Jacopo Mondi 
> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt 
> b/Documentation/devicetree/bindings/media/rcar_vin.txt
> index c53ce4e..17eac8a 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -63,6 +63,11 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
>   If both HSYNC and VSYNC polarities are not specified, embedded
>   synchronization is selected.
> 
> +- data-active: active state of data enable signal (CLOCKENB pin).

I'm not sure what you mean by active state here. video-interfaces.txt 
defines data-active as 'similar to HSYNC and VSYNC, specifies data line 
polarity' so I assume this is the polarity of the CLOCKENB pin?

> +  0/1 for LOW/HIGH respectively. If not specified, use HSYNC as
> +  data enable signal. When using embedded synchronization this
> +  property is mandatory.

I'm confused, why is this mandatory if we have no embedded sync (that is 
hsync-active and vsync-active not defined)? I can't find any reference 
to this in the Gen2 datasheet but I'm sure I'm just missing it :-)

> +
>  - port 1 - sub-nodes describing one or more endpoints connected to
>the VIN from local SoC CSI-2 receivers. The endpoint numbers must
>use the following schema.
> --
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH 1/6] dt-bindings: media: rcar-vin: Describe optional ep properties

[Niklas Söderlund]

Hi Jacopo,

Thanks for your patch.

On 2018-05-16 18:32:27 +0200, Jacopo Mondi wrote:
> Describe the optional endpoint properties for endpoint nodes of port@0
> and port@1 of the R-Car VIN driver device tree bindings documentation.
> 
> Signed-off-by: Jacopo Mondi 

Acked-by: Niklas Söderlund 

> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 13 -
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt 
> b/Documentation/devicetree/bindings/media/rcar_vin.txt
> index a19517e1..c53ce4e 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -53,6 +53,16 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
>from external SoC pins described in video-interfaces.txt[1].
>Describing more then one endpoint in port 0 is invalid. Only VIN
>instances that are connected to external pins should have port 0.
> +
> +  - Optional properties for endpoint nodes of port@0:
> +- hsync-active: active state of the HSYNC signal, 0/1 for LOW/HIGH
> +   respectively. Default is active high.
> +- vsync-active: active state of the VSYNC signal, 0/1 for LOW/HIGH
> +   respectively. Default is active high.
> +
> + If both HSYNC and VSYNC polarities are not specified, embedded
> + synchronization is selected.
> +
>  - port 1 - sub-nodes describing one or more endpoints connected to
>the VIN from local SoC CSI-2 receivers. The endpoint numbers must
>use the following schema.
> @@ -62,6 +72,8 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
>  - Endpoint 2 - sub-node describing the endpoint connected to CSI40
>  - Endpoint 3 - sub-node describing the endpoint connected to CSI41
> 
> +  Endpoint nodes of port@1 do not support any optional endpoint property.
> +
>  Device node example for Gen2 platforms
>  --
> 
> @@ -112,7 +124,6 @@ Board setup example for Gen2 platforms (vin1 composite 
> video input)
> 
>  vin1ep0: endpoint {
>  remote-endpoint = <>;
> -bus-width = <8>;
>  };
>  };
>  };
> --
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH 0/6] media: rcar-vin: Brush endpoint properties

[Niklas Söderlund]

Hi Jacopo,

On 2018-05-16 18:32:26 +0200, Jacopo Mondi wrote:
> Hello,
>this series touches the bindings and the driver handling endpoint
> properties for digital subdevices of the R-Car VIN driver.
> 
> The first patch simply documents what are the endpoint properties supported
> at the moment, then the second one extends them with 'data-active'.
> 
> As the VIN hardware allows to use HSYNC as data enable signal when the CLCKENB
> pin is left unconnected, the 'data-active' property presence determinates
> if HSYNC has to be used or not as data enable signal. As a consequence, when
> running with embedded synchronism, and there is not HSYNC signal, it becomes
> mandatory to specify 'data-active' polarity in DTS.
> 
> To address this, all Gen-2 boards featuring a composite video input and
> running with embedded synchronization, now need that property to be specified
> in DTS. Before adding it, remove un-used properties as 'pclk-sample' and
> 'bus-width' from the Gen-2 bindings, as they are not parsed by the VIN driver
> and only confuse users.
> 
> Niklas, as you already know I don't have any Gen2 board. Could you give this
> a spin on Koelsch if you like the series?

I tested this on my Koelsch and capture is still working :-)

> 
> Thanks
>j
> 
> Jacopo Mondi (6):
>   dt-bindings: media: rcar-vin: Describe optional ep properties
>   dt-bindings: media: rcar-vin: Document data-active
>   media: rcar-vin: Handle data-active property
>   media: rcar-vin: Handle CLOCKENB pin polarity
>   ARM: dts: rcar-gen2: Remove unused VIN properties
>   ARM: dts: rcar-gen2: Add 'data-active' property
> 
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 18 +-
>  arch/arm/boot/dts/r8a7790-lager.dts  |  4 +---
>  arch/arm/boot/dts/r8a7791-koelsch.dts|  4 +---
>  arch/arm/boot/dts/r8a7791-porter.dts |  2 +-
>  arch/arm/boot/dts/r8a7793-gose.dts   |  4 +---
>  arch/arm/boot/dts/r8a7794-alt.dts|  2 +-
>  arch/arm/boot/dts/r8a7794-silk.dts   |  2 +-
>  drivers/media/platform/rcar-vin/rcar-core.c  | 10 --
>  drivers/media/platform/rcar-vin/rcar-dma.c   | 11 +++
>  9 files changed, 42 insertions(+), 15 deletions(-)
> 
> --
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund

[PATCH v3 0/2] IR decoding using BPF

[Sean Young]

The kernel IR decoders (drivers/media/rc/ir-*-decoder.c) support the most
widely used IR protocols, but there are many protocols which are not
supported[1]. For example, the lirc-remotes[2] repo has over 2700 remotes,
many of which are not supported by rc-core. There is a "long tail" of
unsupported IR protocols, for which lircd is need to decode the IR .

IR encoding is done in such a way that some simple circuit can decode it;
therefore, bpf is ideal.

In order to support all these protocols, here we have bpf based IR decoding.
The idea is that user-space can define a decoder in bpf, attach it to
the rc device through the lirc chardev.

Separate work is underway to extend ir-keytable to have an extensive library
of bpf-based decoders, and a much expanded library of rc keymaps.

Another future application would be to compile IRP[3] to a IR BPF program, and
so support virtually every remote without having to write a decoder for each.
It might also be possible to support non-button devices such as analog
directional pads or air conditioning remote controls and decode the target
temperature in bpf, and pass that to an input device.

Thanks,

Sean Young

[1] http://www.hifi-remote.com/wiki/index.php?title=DecodeIR
[2] https://sourceforge.net/p/lirc-remotes/code/ci/master/tree/remotes/
[3] http://www.hifi-remote.com/wiki/index.php?title=IRP_Notation

Changes since v2:
 - Fixed locking issues
 - Improved self-test to cover more cases
 - Rebased on bpf-next again

Changes since v1:
 - Code review comments from Y Song  and
   Randy Dunlap 
 - Re-wrote sample bpf to be selftest
 - Renamed RAWIR_DECODER -> RAWIR_EVENT (Kconfig, context, bpf prog type)
 - Rebase on bpf-next
 - Introduced bpf_rawir_event context structure with simpler access checking

Sean Young (2):
  media: rc: introduce BPF_PROG_RAWIR_EVENT
  bpf: add selftest for rawir_event type program

 drivers/media/rc/Kconfig  |  13 +
 drivers/media/rc/Makefile |   1 +
 drivers/media/rc/bpf-rawir-event.c| 363 ++
 drivers/media/rc/lirc_dev.c   |  24 ++
 drivers/media/rc/rc-core-priv.h   |  24 ++
 drivers/media/rc/rc-ir-raw.c  |  14 +-
 include/linux/bpf_rcdev.h |  30 ++
 include/linux/bpf_types.h |   3 +
 include/uapi/linux/bpf.h  |  55 ++-
 kernel/bpf/syscall.c  |   7 +
 tools/bpf/bpftool/prog.c  |   1 +
 tools/include/uapi/linux/bpf.h|  57 ++-
 tools/lib/bpf/libbpf.c|   1 +
 tools/testing/selftests/bpf/Makefile  |   8 +-
 tools/testing/selftests/bpf/bpf_helpers.h |   6 +
 tools/testing/selftests/bpf/test_rawir.sh |  37 ++
 .../selftests/bpf/test_rawir_event_kern.c |  26 ++
 .../selftests/bpf/test_rawir_event_user.c | 130 +++
 18 files changed, 792 insertions(+), 8 deletions(-)
 create mode 100644 drivers/media/rc/bpf-rawir-event.c
 create mode 100644 include/linux/bpf_rcdev.h
 create mode 100755 tools/testing/selftests/bpf/test_rawir.sh
 create mode 100644 tools/testing/selftests/bpf/test_rawir_event_kern.c
 create mode 100644 tools/testing/selftests/bpf/test_rawir_event_user.c

-- 
2.17.0

[PATCH v3 1/2] media: rc: introduce BPF_PROG_RAWIR_EVENT

[Sean Young]

Add support for BPF_PROG_RAWIR_EVENT. This type of BPF program can call
rc_keydown() to reported decoded IR scancodes, or rc_repeat() to report
that the last key should be repeated.

The bpf program can be attached to using the bpf(BPF_PROG_ATTACH) syscall;
the target_fd must be the /dev/lircN device.

Signed-off-by: Sean Young 
---
 drivers/media/rc/Kconfig   |  13 ++
 drivers/media/rc/Makefile  |   1 +
 drivers/media/rc/bpf-rawir-event.c | 363 +
 drivers/media/rc/lirc_dev.c|  24 ++
 drivers/media/rc/rc-core-priv.h|  24 ++
 drivers/media/rc/rc-ir-raw.c   |  14 +-
 include/linux/bpf_rcdev.h  |  30 +++
 include/linux/bpf_types.h  |   3 +
 include/uapi/linux/bpf.h   |  55 -
 kernel/bpf/syscall.c   |   7 +
 10 files changed, 531 insertions(+), 3 deletions(-)
 create mode 100644 drivers/media/rc/bpf-rawir-event.c
 create mode 100644 include/linux/bpf_rcdev.h

diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
index eb2c3b6eca7f..2172d65b0213 100644
--- a/drivers/media/rc/Kconfig
+++ b/drivers/media/rc/Kconfig
@@ -25,6 +25,19 @@ config LIRC
   passes raw IR to and from userspace, which is needed for
   IR transmitting (aka "blasting") and for the lirc daemon.
 
+config BPF_RAWIR_EVENT
+   bool "Support for eBPF programs attached to lirc devices"
+   depends on BPF_SYSCALL
+   depends on RC_CORE=y
+   depends on LIRC
+   help
+  Allow attaching eBPF programs to a lirc device using the bpf(2)
+  syscall command BPF_PROG_ATTACH. This is supported for raw IR
+  receivers.
+
+  These eBPF programs can be used to decode IR into scancodes, for
+  IR protocols not supported by the kernel decoders.
+
 menuconfig RC_DECODERS
bool "Remote controller decoders"
depends on RC_CORE
diff --git a/drivers/media/rc/Makefile b/drivers/media/rc/Makefile
index 2e1c87066f6c..74907823bef8 100644
--- a/drivers/media/rc/Makefile
+++ b/drivers/media/rc/Makefile
@@ -5,6 +5,7 @@ obj-y += keymaps/
 obj-$(CONFIG_RC_CORE) += rc-core.o
 rc-core-y := rc-main.o rc-ir-raw.o
 rc-core-$(CONFIG_LIRC) += lirc_dev.o
+rc-core-$(CONFIG_BPF_RAWIR_EVENT) += bpf-rawir-event.o
 obj-$(CONFIG_IR_NEC_DECODER) += ir-nec-decoder.o
 obj-$(CONFIG_IR_RC5_DECODER) += ir-rc5-decoder.o
 obj-$(CONFIG_IR_RC6_DECODER) += ir-rc6-decoder.o
diff --git a/drivers/media/rc/bpf-rawir-event.c 
b/drivers/media/rc/bpf-rawir-event.c
new file mode 100644
index ..7cb48b8d87b5
--- /dev/null
+++ b/drivers/media/rc/bpf-rawir-event.c
@@ -0,0 +1,363 @@
+// SPDX-License-Identifier: GPL-2.0
+// bpf-rawir-event.c - handles bpf
+//
+// Copyright (C) 2018 Sean Young 
+
+#include 
+#include 
+#include 
+#include "rc-core-priv.h"
+
+/*
+ * BPF interface for raw IR
+ */
+const struct bpf_prog_ops rawir_event_prog_ops = {
+};
+
+BPF_CALL_1(bpf_rc_repeat, struct bpf_rawir_event*, event)
+{
+   struct ir_raw_event_ctrl *ctrl;
+
+   ctrl = container_of(event, struct ir_raw_event_ctrl, bpf_rawir_event);
+
+   rc_repeat(ctrl->dev);
+
+   return 0;
+}
+
+static const struct bpf_func_proto rc_repeat_proto = {
+   .func  = bpf_rc_repeat,
+   .gpl_only  = true, /* rc_repeat is EXPORT_SYMBOL_GPL */
+   .ret_type  = RET_INTEGER,
+   .arg1_type = ARG_PTR_TO_CTX,
+};
+
+BPF_CALL_4(bpf_rc_keydown, struct bpf_rawir_event*, event, u32, protocol,
+  u32, scancode, u32, toggle)
+{
+   struct ir_raw_event_ctrl *ctrl;
+
+   ctrl = container_of(event, struct ir_raw_event_ctrl, bpf_rawir_event);
+
+   rc_keydown(ctrl->dev, protocol, scancode, toggle != 0);
+
+   return 0;
+}
+
+static const struct bpf_func_proto rc_keydown_proto = {
+   .func  = bpf_rc_keydown,
+   .gpl_only  = true, /* rc_keydown is EXPORT_SYMBOL_GPL */
+   .ret_type  = RET_INTEGER,
+   .arg1_type = ARG_PTR_TO_CTX,
+   .arg2_type = ARG_ANYTHING,
+   .arg3_type = ARG_ANYTHING,
+   .arg4_type = ARG_ANYTHING,
+};
+
+static const struct bpf_func_proto *
+rawir_event_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
+{
+   switch (func_id) {
+   case BPF_FUNC_rc_repeat:
+   return _repeat_proto;
+   case BPF_FUNC_rc_keydown:
+   return _keydown_proto;
+   case BPF_FUNC_map_lookup_elem:
+   return _map_lookup_elem_proto;
+   case BPF_FUNC_map_update_elem:
+   return _map_update_elem_proto;
+   case BPF_FUNC_map_delete_elem:
+   return _map_delete_elem_proto;
+   case BPF_FUNC_ktime_get_ns:
+   return _ktime_get_ns_proto;
+   case BPF_FUNC_tail_call:
+   return _tail_call_proto;
+   case BPF_FUNC_get_prandom_u32:
+   return _get_prandom_u32_proto;
+   case BPF_FUNC_trace_printk:
+   if (capable(CAP_SYS_ADMIN))
+   return 

[PATCH v3 2/2] bpf: add selftest for rawir_event type program

[Sean Young]

This is simple test over rc-loopback.

Signed-off-by: Sean Young 
---
 tools/bpf/bpftool/prog.c  |   1 +
 tools/include/uapi/linux/bpf.h|  57 +++-
 tools/lib/bpf/libbpf.c|   1 +
 tools/testing/selftests/bpf/Makefile  |   8 +-
 tools/testing/selftests/bpf/bpf_helpers.h |   6 +
 tools/testing/selftests/bpf/test_rawir.sh |  37 +
 .../selftests/bpf/test_rawir_event_kern.c |  26 
 .../selftests/bpf/test_rawir_event_user.c | 130 ++
 8 files changed, 261 insertions(+), 5 deletions(-)
 create mode 100755 tools/testing/selftests/bpf/test_rawir.sh
 create mode 100644 tools/testing/selftests/bpf/test_rawir_event_kern.c
 create mode 100644 tools/testing/selftests/bpf/test_rawir_event_user.c

diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 9bdfdf2d3fbe..8889a4ee8577 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -71,6 +71,7 @@ static const char * const prog_type_name[] = {
[BPF_PROG_TYPE_SK_MSG]  = "sk_msg",
[BPF_PROG_TYPE_RAW_TRACEPOINT]  = "raw_tracepoint",
[BPF_PROG_TYPE_CGROUP_SOCK_ADDR] = "cgroup_sock_addr",
+   [BPF_PROG_TYPE_RAWIR_EVENT] = "rawir_event",
 };
 
 static void print_boot_time(__u64 nsecs, char *buf, unsigned int size)
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 1205d86a7a29..243e141e8a5b 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -141,6 +141,7 @@ enum bpf_prog_type {
BPF_PROG_TYPE_SK_MSG,
BPF_PROG_TYPE_RAW_TRACEPOINT,
BPF_PROG_TYPE_CGROUP_SOCK_ADDR,
+   BPF_PROG_TYPE_RAWIR_EVENT,
 };
 
 enum bpf_attach_type {
@@ -158,6 +159,7 @@ enum bpf_attach_type {
BPF_CGROUP_INET6_CONNECT,
BPF_CGROUP_INET4_POST_BIND,
BPF_CGROUP_INET6_POST_BIND,
+   BPF_RAWIR_EVENT,
__MAX_BPF_ATTACH_TYPE
 };
 
@@ -1829,7 +1831,6 @@ union bpf_attr {
  * Return
  * 0 on success, or a negative error in case of failure.
  *
- *
  * int bpf_fib_lookup(void *ctx, struct bpf_fib_lookup *params, int plen, u32 
flags)
  * Description
  * Do FIB lookup in kernel tables using parameters in *params*.
@@ -1856,6 +1857,7 @@ union bpf_attr {
  * Egress device index on success, 0 if packet needs to continue
  * up the stack for further processing or a negative error in case
  * of failure.
+ *
  * int bpf_sock_hash_update(struct bpf_sock_ops_kern *skops, struct bpf_map 
*map, void *key, u64 flags)
  * Description
  * Add an entry to, or update a sockhash *map* referencing sockets.
@@ -1902,6 +1904,35 @@ union bpf_attr {
  * egress otherwise). This is the only flag supported for now.
  * Return
  * **SK_PASS** on success, or **SK_DROP** on error.
+ *
+ * int bpf_rc_keydown(void *ctx, u32 protocol, u32 scancode, u32 toggle)
+ * Description
+ * Report decoded scancode with toggle value. For use in
+ * BPF_PROG_TYPE_RAWIR_EVENT, to report a successfully
+ * decoded scancode. This is will generate a keydown event,
+ * and a keyup event once the scancode is no longer repeated.
+ *
+ * *ctx* pointer to bpf_rawir_event, *protocol* is decoded
+ * protocol (see RC_PROTO_* enum).
+ *
+ * Some protocols include a toggle bit, in case the button
+ * was released and pressed again between consecutive scancodes,
+ * copy this bit into *toggle* if it exists, else set to 0.
+ *
+ * Return
+ * Always return 0 (for now)
+ *
+ * int bpf_rc_repeat(void *ctx)
+ * Description
+ * Repeat the last decoded scancode; some IR protocols like
+ * NEC have a special IR message for repeat last button,
+ * in case user is holding a button down; the scancode is
+ * not repeated.
+ *
+ * *ctx* pointer to bpf_rawir_event.
+ *
+ * Return
+ * Always return 0 (for now)
  */
 #define __BPF_FUNC_MAPPER(FN)  \
FN(unspec), \
@@ -1976,7 +2007,9 @@ union bpf_attr {
FN(fib_lookup), \
FN(sock_hash_update),   \
FN(msg_redirect_hash),  \
-   FN(sk_redirect_hash),
+   FN(sk_redirect_hash),   \
+   FN(rc_repeat),  \
+   FN(rc_keydown),
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call
@@ -2043,6 +2076,26 @@ enum bpf_hdr_start_off {
BPF_HDR_START_NET,
 };
 
+/*
+ * user accessible mirror of in-kernel ir_raw_event
+ */
+#define BPF_RAWIR_EVENT_SPACE  0
+#define BPF_RAWIR_EVENT_PULSE  1
+#define BPF_RAWIR_EVENT_TIMEOUT2
+#define BPF_RAWIR_EVENT_RESET  3
+#define BPF_RAWIR_EVENT_CARRIER 

Re: [RFC, libv4l]: Make libv4l2 usable on devices with complex pipeline

[Pavel Machek]

Hi!

> > #modes: 2
> > Driver name: OMAP 3 resizer
> 
> It probably makes sense to place the driver name before #modes.

I dropped the driver for now. It served no purpose...

I got the patches to work on N900. With them (and right)

> From what I understood, the "#foo: " is a tag that makes the
> parser to expect for  of "foo".
> 
> I would also add a first line at the beginning describing the
> version of the format, just in case we add more stuff that
> would require to change something at the format.

Ok, done.

> Except for that, it seems ok.

Good, thanks!

Here's current version of the patch. Code will need moving to
libv4l2. Would "v4l2_open_pipeline()" be suitable name to use?

Best regards,
Pavel

diff --git a/contrib/test/sdlcam.c b/contrib/test/sdlcam.c
index cc43a10..95f85f9 100644
--- a/contrib/test/sdlcam.c
+++ b/contrib/test/sdlcam.c
@@ -8,7 +8,7 @@
Copyright 2017 Pavel Machek, LGPL
 
Needs sdl2, sdl2_image libraries. sudo aptitude install libsdl2-dev
-   libsdl2-image-dev on Debian systems.
+   libsdl2-image-dev libjpeg-dev on Debian systems.
 */
 
 #include 
@@ -26,6 +26,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 
@@ -1172,11 +1173,18 @@ static void sdl_iteration(struct sdl *m)
}
 }
 
+static int open_complex(char *name, int v4l2_flags);
+
 static void cam_open(struct dev_info *dev, char *name)
 {
struct v4l2_format *fmt = >fmt;
 
-   dev->fd = v4l2_open(name, O_RDWR);
+   //  dev->fd = v4l2_open(name, O_RDWR);
+   //dev->fd = open_complex("/my/tui/camera/n900.cv", 0);
+   dev->fd = open_complex("/data/pavel/g/v4l-utils/test.txt", 0);
+   //dev->fd = open(name, O_RDWR);
+   //printf("fd = %d\n", dev->fd);
+   //dev->fd = v4l2_fd_open(dev->fd, 0);
if (dev->fd < 0) {
printf("video %s open failed: %m\n", name);
exit(1);
@@ -1198,6 +1206,179 @@ static void cam_open(struct dev_info *dev, char *name)
 
 /* -- */
 
+static void scan_devices(char **device_names, int *device_fds, int num)
+{
+   struct dirent **namelist;
+   int n;
+   char *class_v4l = "/sys/class/video4linux";
+
+   n = scandir(class_v4l, , NULL, alphasort);
+   if (n < 0) {
+   perror("scandir");
+   return;
+   }
+   
+   while (n--) {
+   if (namelist[n]->d_name[0] != '.') {
+   char filename[1024], content[1024];
+   sprintf(filename, "%s/%s/name", class_v4l, 
namelist[n]->d_name);
+   FILE *f = fopen(filename, "r");
+   if (!f) {
+   printf("Strange, can't open %s", filename);
+   } else {
+   fgets(content, 1024, f);
+   fclose(f);
+   printf("device %s : %s\n", namelist[n]->d_name, 
content);
+   int i;
+   for (i = num-1; i >=0; i--) {
+   if (!strcmp(content, device_names[i])) {
+   sprintf(filename, "/dev/%s", 
namelist[n]->d_name);
+   device_fds[i] = open(filename, 
O_RDWR);
+   if (device_fds[i] < 0) {
+   printf("Error opening 
%s: %m\n", filename);
+   }
+   printf("*** found match - %s 
%d\n", filename, device_fds[i]);
+   }
+   }
+   }
+   }
+   free(namelist[n]);
+   }
+   free(namelist);
+  
+}
+
+static int open_complex(char *name, int v4l2_flags)
+{
+#define perr(s) printf("v4l2: open_complex: " s "\n");
+#define BUF 256
+   FILE *f = fopen(name, "r");
+
+   int res = -1;
+   char buf[BUF];
+   int version, num_modes, num_devices, num_controls;
+   int dev, control;
+
+   if (!f) {
+   perr("open of .cv file failed: %m");
+   goto err;
+   }
+
+   if (fscanf(f, "Complex Video: %d\n", ) != 1) {
+   perr(".cv file does not have required header");
+   goto close;
+   }
+
+   if (version != 0) {
+   perr(".cv file has unknown version");
+   goto close;
+   }
+  
+   if (fscanf(f, "#modes: %d\n", _modes) != 1) {
+   perr("could not parse modes");
+   goto close;
+   }
+
+   if (num_modes != 1) {
+   perr("only single mode is supported for now");
+   goto close;
+   }
+
+   if (fscanf(f, "Mode: %s\n", buf) != 1) {
+   perr("could not 

Re: [PATCH v2 4/4] media: rcar-vin: Add support for R-Car R8A77995 SoC

[Niklas Söderlund]

Hi Jacopo,

On 2018-05-16 14:16:56 +0200, Jacopo Mondi wrote:
> Add R-Car R8A77995 SoC to the rcar-vin supported ones.
> 
> Signed-off-by: Jacopo Mondi 
> Reviewed-by: Niklas Söderlund 
> Reviewed-by: Laurent Pinchart 
> ---
>  drivers/media/platform/rcar-vin/rcar-core.c | 16 
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c 
> b/drivers/media/platform/rcar-vin/rcar-core.c
> index ea74c55..fba8610 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -1104,6 +1104,10 @@ static const struct rvin_group_route 
> _rcar_info_r8a77970_routes[] = {
>   { /* Sentinel */ }
>  };
>  
> +static const struct rvin_group_route _rcar_info_r8a77995_routes[] = {

Nitpick and I see that the error is not yours but present in other but 
not all route declarations. But the intention is not to have a leading _ 
here.  A patch to clean that up for _rcar_info_r8a77965_routes[] and 
_rcar_info_r8a77970_routes[] would be welcome, or I can do it if you are 
over loaded.

> + { /* Sentinel */ }
> +};
> +
>  static const struct rvin_info rcar_info_r8a77970 = {
>   .model = RCAR_GEN3,
>   .use_mc = true,
> @@ -1112,6 +1116,14 @@ static const struct rvin_info rcar_info_r8a77970 = {
>   .routes = _rcar_info_r8a77970_routes,
>  };
>  
> +static const struct rvin_info rcar_info_r8a77995 = {
> + .model = RCAR_GEN3,
> + .use_mc = true,
> + .max_width = 4096,
> + .max_height = 4096,
> + .routes = _rcar_info_r8a77995_routes,
> +};
> +
>  static const struct of_device_id rvin_of_id_table[] = {
>   {
>   .compatible = "renesas,vin-r8a7778",
> @@ -1153,6 +1165,10 @@ static const struct of_device_id rvin_of_id_table[] = {
>   .compatible = "renesas,vin-r8a77970",
>   .data = _info_r8a77970,
>   },
> + {
> + .compatible = "renesas,vin-r8a77995",
> + .data = _info_r8a77995,
> + },
>   { /* Sentinel */ },
>  };
>  MODULE_DEVICE_TABLE(of, rvin_of_id_table);
> -- 
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH v2 3/4] media: rcar-vin: Handle digital subdev in link_notify

[Niklas Söderlund]

Hi Jacopo,

Thanks for your patch,

On 2018-05-16 14:16:55 +0200, Jacopo Mondi wrote:
> Handle digital subdevices in link_notify callback. If the notified link
> involves a digital subdevice, do not change routing of the VIN-CSI-2
> devices.
> 
> Signed-off-by: Jacopo Mondi 
> ---
>  drivers/media/platform/rcar-vin/rcar-core.c | 30 
> +++--
>  1 file changed, 28 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c 
> b/drivers/media/platform/rcar-vin/rcar-core.c
> index 1003c8c..ea74c55 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -168,10 +168,36 @@ static int rvin_group_link_notify(struct media_link 
> *link, u32 flags,
>   }
>  
>   /* Add the new link to the existing mask and check if it works. */
> - csi_id = rvin_group_entity_to_csi_id(group, link->source->entity);
>   channel = rvin_group_csi_pad_to_channel(link->source->index);
> - mask_new = mask & rvin_group_get_mask(vin, csi_id, channel);
> + csi_id = rvin_group_entity_to_csi_id(group, link->source->entity);
> + if (csi_id == -ENODEV) {
> + struct v4l2_subdev *sd;
> + unsigned int i;
> +
> + /*
> +  * Make sure the source entity subdevice is registered as
> +  * a digital input of one of the enabled VINs if it is not
> +  * one of the CSI-2 subdevices.
> +  *
> +  * No hardware configuration required for digital inputs,
> +  * we can return here.
> +  */
> + sd = media_entity_to_v4l2_subdev(link->source->entity);
> + for (i = 0; i < RCAR_VIN_NUM; i++) {
> + if (group->vin[i] && group->vin[i]->digital &&
> + group->vin[i]->digital->subdev == sd) {
> + ret = 0;
> + goto out;
> + }
> + }
> +
> + vin_err(vin, "Subdevice %s not registered to any VIN\n",
> + link->source->entity->name);
> + ret = -ENODEV;
> + goto out;
> + }

I like this patch, you made it so simple. I feared this would be the 
ugly part when adding parallel support to Gen3. All that is missing is 
handling of vin->mbus_cfg or how you think we best handle that for the 
different input buses.

>  
> + mask_new = mask & rvin_group_get_mask(vin, csi_id, channel);
>   vin_dbg(vin, "Try link change mask: 0x%x new: 0x%x\n", mask, mask_new);
>  
>   if (!mask_new) {
> -- 
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH v2 2/4] media: rcar-vin: Handle mc in digital notifier ops

[Niklas Söderlund]

Hi Jacopo,

Thanks for your patch.

I only have one generic comment to this patch as I fear comments on 1/4 
have the potential to change quiet a few things here :-(

On 2018-05-16 14:16:54 +0200, Jacopo Mondi wrote:
> Handle media-controller in the digital notifier operations (bound,
> unbind and complete) registered for VIN instances handling a digital
> input subdevice.
> 
> Signed-off-by: Jacopo Mondi 
> ---
>  drivers/media/platform/rcar-vin/rcar-core.c | 94 
> -
>  1 file changed, 65 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c 
> b/drivers/media/platform/rcar-vin/rcar-core.c
> index 0ea21ab..1003c8c 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -379,7 +379,7 @@ static int rvin_find_pad(struct v4l2_subdev *sd, int 
> direction)
>   * Digital async notifier
>   */
>  
> -/* The vin lock should be held when calling the subdevice attach and detach 
> */
> +/* The vin lock should be held when calling the subdevice attach. */
>  static int rvin_digital_subdevice_attach(struct rvin_dev *vin,
>struct v4l2_subdev *subdev)
>  {
> @@ -388,15 +388,6 @@ static int rvin_digital_subdevice_attach(struct rvin_dev 
> *vin,
>   };
>   int ret;
>  
> - /* Find source and sink pad of remote subdevice */
> - ret = rvin_find_pad(subdev, MEDIA_PAD_FL_SOURCE);
> - if (ret < 0)
> - return ret;
> - vin->digital->source_pad = ret;
> -
> - ret = rvin_find_pad(subdev, MEDIA_PAD_FL_SINK);
> - vin->digital->sink_pad = ret < 0 ? 0 : ret;
> -
>   /* Find compatible subdevices mbus format */
>   vin->mbus_code = 0;
>   code.index = 0;
> @@ -450,23 +441,14 @@ static int rvin_digital_subdevice_attach(struct 
> rvin_dev *vin,
>  
>   vin->vdev.ctrl_handler = >ctrl_handler;
>  
> - vin->digital->subdev = subdev;
> -
>   return 0;
>  }
>  
> -static void rvin_digital_subdevice_detach(struct rvin_dev *vin)

Please don't fold this function into the caller. I tired the same thing 
when adding Gen3 support but keeping the _attach and _detach functions 
keeps a nice abstraction on what happens when we add and remove a 
parallel subdevice to the internal data structures separated from the 
housekeeping stuff in the callbacks :-)


> -{
> - rvin_v4l2_unregister(vin);
> - v4l2_ctrl_handler_free(>ctrl_handler);
> -
> - vin->vdev.ctrl_handler = NULL;
> - vin->digital->subdev = NULL;
> -}
> -
>  static int rvin_digital_notify_complete(struct v4l2_async_notifier *notifier)
>  {
>   struct rvin_dev *vin = notifier_to_vin(notifier);
> + struct media_entity *source;
> + struct media_entity *sink;
>   int ret;
>  
>   ret = v4l2_device_register_subdev_nodes(>v4l2_dev);
> @@ -475,7 +457,26 @@ static int rvin_digital_notify_complete(struct 
> v4l2_async_notifier *notifier)
>   return ret;
>   }
>  
> - return rvin_v4l2_register(vin);
> + if (!video_is_registered(>vdev)) {
> + ret = rvin_v4l2_register(vin);
> + if (ret < 0)
> + return ret;
> + }
> +
> + if (!vin->info->use_mc)
> + return 0;
> +
> + /* If we're running with media controller, link the subdevice. */
> + source = >digital->subdev->entity;
> + sink = >vdev.entity;
> +
> + ret = media_create_pad_link(source, vin->digital->source_pad,
> + sink, vin->digital->sink_pad, 0);
> + if (ret)
> + vin_err(vin, "Error adding link from %s to %s\n",
> + source->name, sink->name);
> +
> + return ret;
>  }
>  
>  static void rvin_digital_notify_unbind(struct v4l2_async_notifier *notifier,
> @@ -487,7 +488,14 @@ static void rvin_digital_notify_unbind(struct 
> v4l2_async_notifier *notifier,
>   vin_dbg(vin, "unbind digital subdev %s\n", subdev->name);
>  
>   mutex_lock(>lock);
> - rvin_digital_subdevice_detach(vin);
> +
> + rvin_v4l2_unregister(vin);
> + vin->digital->subdev = NULL;
> + if (!vin->info->use_mc) {
> + v4l2_ctrl_handler_free(>ctrl_handler);
> + vin->vdev.ctrl_handler = NULL;
> + }
> +
>   mutex_unlock(>lock);
>  }
>  
> @@ -499,10 +507,29 @@ static int rvin_digital_notify_bound(struct 
> v4l2_async_notifier *notifier,
>   int ret;
>  
>   mutex_lock(>lock);
> - ret = rvin_digital_subdevice_attach(vin, subdev);
> - mutex_unlock(>lock);
> - if (ret)
> +
> + /* Find source and sink pad of remote subdevice */
> + ret = rvin_find_pad(subdev, MEDIA_PAD_FL_SOURCE);
> + if (ret < 0) {
> + mutex_unlock(>lock);
>   return ret;
> + }
> + vin->digital->source_pad = ret;
> +
> + ret = rvin_find_pad(subdev, MEDIA_PAD_FL_SINK);
> + vin->digital->sink_pad = ret < 0 ? 0 : ret;
> +
> + 

Re: [PATCH v2 1/4] media: rcar-vin: Parse digital input in mc-path

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work!

First let me apologies for the use of the keyword 'digital' in the 
driver it should have been parallel... Someday we should remedy this.

If you touch any parts of the code where such a transition make sens I 
would not complain about the intermixed use of digital/parallel. Once 
your work is done we could follow up with a cleanup patch to complete 
the transition. Or if you rather stick with digital here I'm fine with 
that too.

On 2018-05-16 14:16:53 +0200, Jacopo Mondi wrote:
> Add support for digital input subdevices to Gen-3 rcar-vin.
> The Gen-3, media-controller compliant, version has so far only accepted
> CSI-2 input subdevices. Remove assumptions on the supported bus_type and
> accepted number of subdevices, and allow digital input connections on port@0.
> 
> Signed-off-by: Jacopo Mondi 
> ---
>  drivers/media/platform/rcar-vin/rcar-core.c | 99 
> +++--
>  drivers/media/platform/rcar-vin/rcar-vin.h  | 15 +
>  2 files changed, 93 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c 
> b/drivers/media/platform/rcar-vin/rcar-core.c
> index d3072e1..0ea21ab 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -562,7 +562,7 @@ static int rvin_digital_graph_init(struct rvin_dev *vin)
>   return ret;
>  
>   if (!vin->digital)
> - return -ENODEV;
> + return -ENOTCONN;
>  
>   vin_dbg(vin, "Found digital subdevice %pOF\n",
>   to_of_node(vin->digital->asd.match.fwnode));
> @@ -703,15 +703,13 @@ static int rvin_mc_parse_of_endpoint(struct device *dev,
>  {
>   struct rvin_dev *vin = dev_get_drvdata(dev);
>  
> - if (vep->base.port != 1 || vep->base.id >= RVIN_CSI_MAX)
> + if (vep->base.port != RVIN_PORT_CSI2 || vep->base.id >= RVIN_CSI_MAX)

I don't like this RVIN_PORT_CSI2. It makes the code harder to read as I 
have have to go and lookup which port RVIN_PORT_CSI2 represents. I would 
rater just keep vep->base.port != 1 as I think it's much clearer whats 
going on. And it's not as we will move the CSI-2 input to a different 
port as it's described in the bindings.

>   return -EINVAL;
>  
>   if (!of_device_is_available(to_of_node(asd->match.fwnode))) {
> -
>   vin_dbg(vin, "OF device %pOF disabled, ignoring\n",
>   to_of_node(asd->match.fwnode));
>   return -ENOTCONN;
> -
>   }
>  
>   if (vin->group->csi[vep->base.id].fwnode) {
> @@ -720,6 +718,8 @@ static int rvin_mc_parse_of_endpoint(struct device *dev,
>   return -ENOTCONN;
>   }
>  
> + vin->mbus_cfg.type = V4L2_MBUS_CSI2;
> + vin->mbus_cfg.flags = 0;

I like this move of mbus_cfg handling! Makes the two cases more aligned 
which are good. Unfortunately I fear it needs more work :-(

With this series addition of parallel input support. There are now two 
input types CSI-2 and parallel which can be changed at runtime for the 
same VIN. The mbus connected to the VIN will therefor be different 
depending on which media graph link is connected to a particular VIN and 
this effects hardware configuration, see 'vin->mbus_cfg.type == 
V4L2_MBUS_CSI2' in rvin_setup().

Maybe the best solution is to move mbus_cfg into struct 
rvin_graph_entity, rename that struct rvin_parallel_input and cache the 
parallel input settings in there, much like we do today for the pads.
Remove mbus_cfg from struct rvin_dev and replace it with a bool flag 
(input_csi or similar)?

In rvin_setup() use this flag to check which input type is in use and if 
it's needed fetch mbus_cfg from this cache. Then in 
rvin_group_link_notify() you could handle this input_csi flag depending 
on which link is activated. But I'm open to other solutions.

>   vin->group->csi[vep->base.id].fwnode = asd->match.fwnode;
>  
>   vin_dbg(vin, "Add group OF device %pOF to slot %u\n",
> @@ -742,7 +742,14 @@ static int rvin_mc_parse_of_graph(struct rvin_dev *vin)
>   return -EINVAL;
>   }
>  
> - /* If not all VIN's are registered don't register the notifier. */
> + /* Collect digital subdevices in this VIN device node. */
> + ret = rvin_digital_graph_init(vin);
> + if (ret < 0 && ret != -ENOTCONN) {
> + mutex_unlock(>group->lock);
> + return ret;
> + }

Why do you need to add this here? Is it not better to in 
rcar_vin_probe() do something like:

ret = rvin_digital_graph_init(vin);
if (ret < 0)
goto error;

if (vin->info->use_mc) {
ret = rvin_mc_init(vin);
if (ret < 0)
goto error;
}

That way we can try and keep to two code paths separated and at lest for 
me that increases readability a lot.

> +
> + /* Only the last registered VIN instance collects CSI-2 subdevices. */
>   for (i = 0; i < RCAR_VIN_NUM; i++)
>   if 

[PATCH] media: gl861: fix probe of dvb_usb_gl861

[mika . batsman]

From: Mika Båtsman 

Probe of dvb_usb_gl861 was working at least with v4.4. Noticed the issue
with v4.13 but according to similar issues the problem started with v4.9.

[   15.288065] transfer buffer not dma capable
[   15.288090] WARNING: CPU: 2 PID: 493 at drivers/usb/core/hcd.c:1595 
usb_hcd_map_urb_for_dma+0x4e2/0x640
...CUT...
[   15.288791] dvb_usb_gl861: probe of 3-7:1.0 failed with error -5

Tested with MSI Mega Sky 580 DVB-T Tuner [GL861]

Cc: sta...@vger.kernel.org
Signed-off-by: Mika Båtsman 
---
 drivers/media/usb/dvb-usb-v2/gl861.c | 17 +++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/dvb-usb-v2/gl861.c 
b/drivers/media/usb/dvb-usb-v2/gl861.c
index b1b09c5..0a988e3 100644
--- a/drivers/media/usb/dvb-usb-v2/gl861.c
+++ b/drivers/media/usb/dvb-usb-v2/gl861.c
@@ -20,15 +20,22 @@ static int gl861_i2c_msg(struct dvb_usb_device *d, u8 addr,
u16 value = addr << (8 + 1);
int wo = (rbuf == NULL || rlen == 0); /* write-only */
u8 req, type;
+   int ret;
+   void *dmadata;
 
if (wo) {
req = GL861_REQ_I2C_WRITE;
type = GL861_WRITE;
+   dmadata = kmemdup(wbuf, wlen, GFP_KERNEL);
} else { /* rw */
req = GL861_REQ_I2C_READ;
type = GL861_READ;
+   dmadata = kmalloc(rlen, GFP_KERNEL);
}
 
+   if (!dmadata)
+   return -ENOMEM;
+
switch (wlen) {
case 1:
index = wbuf[0];
@@ -45,8 +52,14 @@ static int gl861_i2c_msg(struct dvb_usb_device *d, u8 addr,
 
msleep(1); /* avoid I2C errors */
 
-   return usb_control_msg(d->udev, usb_rcvctrlpipe(d->udev, 0), req, type,
-  value, index, rbuf, rlen, 2000);
+   ret = usb_control_msg(d->udev, usb_rcvctrlpipe(d->udev, 0), req, type,
+  value, index, dmadata, rlen, 2000);
+
+   if (!wo)
+   memcpy(rbuf, dmadata, rlen);
+
+   kfree(dmadata);
+   return ret;
 }
 
 /* I2C */
-- 
2.7.4

Re: [Intel-gfx] [PATCH v2 2/5] drm/i915: hdmi: add CEC notifier to intel_hdmi

[Neil Armstrong]

Hi Ville,

On 16/05/2018 16:07, Ville Syrjälä wrote:
> On Wed, May 16, 2018 at 09:40:17AM +0200, Neil Armstrong wrote:
>> On 16/05/2018 09:31, Neil Armstrong wrote:
>>> Hi Ville,
>>>
>>> On 15/05/2018 17:35, Ville Syrjälä wrote:
 On Tue, May 15, 2018 at 04:42:19PM +0200, Neil Armstrong wrote:
> This patchs adds the cec_notifier feature to the intel_hdmi part
> of the i915 DRM driver. It uses the HDMI DRM connector name to 
> differentiate
> between each HDMI ports.
> The changes will allow the i915 HDMI code to notify EDID and HPD changes
> to an eventual CEC adapter.
>
> Signed-off-by: Neil Armstrong 
> ---
>  drivers/gpu/drm/i915/Kconfig  |  1 +
>  drivers/gpu/drm/i915/intel_drv.h  |  2 ++
>  drivers/gpu/drm/i915/intel_hdmi.c | 12 
>  3 files changed, 15 insertions(+)
>
>>>
>>> [..]
>>>
>  }
>  
> @@ -2031,6 +2037,8 @@ static void chv_hdmi_pre_enable(struct 
> intel_encoder *encoder,
>  
>  static void intel_hdmi_destroy(struct drm_connector *connector)
>  {
> + if (intel_attached_hdmi(connector)->notifier)
> + cec_notifier_put(intel_attached_hdmi(connector)->notifier);
>   kfree(to_intel_connector(connector)->detect_edid);
>   drm_connector_cleanup(connector);
>   kfree(connector);
> @@ -2358,6 +2366,10 @@ void intel_hdmi_init_connector(struct 
> intel_digital_port *intel_dig_port,
>   u32 temp = I915_READ(PEG_BAND_GAP_DATA);
>   I915_WRITE(PEG_BAND_GAP_DATA, (temp & ~0xf) | 0xd);
>   }
> +
> + intel_hdmi->notifier = cec_notifier_get_conn(dev->dev, connector->name);

 What are we doing with the connector name here? Those are not actually
 guaranteed to be stable, and any change in the connector probe order
 may cause the name to change.
>>>
>>> The i915 driver can expose multiple HDMI connectors, but each connected will
>>> have a different EDID and CEC physical address, so we will need a different 
>>> notifier
>>> for each connector.
>>>
>>> The connector name is DRM dependent, but user-space actually uses this name 
>>> for
>>> operations, so I supposed it was kind of stable.
>>>
>>> Anyway, is there another stable id/name/whatever that can be used to make a 
>>> name to
>>> distinguish the HDMI connectors ?
>>
>> Would "HDMI %c", port_name(port) be OK to use ?
> 
> Yeah, something like seems like a reasonable approach. That does mean
> we have to be a little careful with changing enum port or port_name()
> in the future, but I guess we could just add a small function to
> generated the name/id specifically for this thing.
> 
> We're also going to have to think what to do with enum port and
> port_name() on ICL+ though. There we won't just have A-F but also
> TC1-TC. Hmm. Looks like what we have for those ports in our
> codebase ATM is a bit wonky since we haven't even changed
> port_name() to give us the TC type names.
> 
> Also we might not want "HDMI" in the identifier since the physical
> port is not HDMI specific. There are different things we could call
> these but I think we could just go with a generic "Port A-F" and
> "Port TC1-TC" approach. I think something like that should work
> fine for current and upcoming hardware. And in theory that could
> then be used for other things as well which need a stable
> identifier.
> 
> Oh, and now I recall that input subsystem at least has some kind
> of "physical path" property on devices. So I guess what we're
> dicussing is a somewhat similar idea. I think input drivers
> generally include the pci/usb/etc. device in the path as well.
> Even though we currently only ever have the one pci device it
> would seem like decent idea to include that information in our
> identifiers as well. Or what do you think?
> 

Thanks for the clarifications !

Having a "Port " will be great indeed, no need to specify HDMI since
only HDMI connectors will get a CEC notifier anyway.

The issue is that port_name() returns a character, which is lame.
Would it be acceptable to introduce a :

const char *port_identifier(struct drm_i915_private *dev_priv,
enum port port)
{
char *id = devm_kzalloc(dev_priv->drm->dev, 16, GFP_KERNEL);

if (id)
return NULL;

snprintf("Port %c", port_name(port));

return id;
}

and use the result of this for the cec_notifier connector name ?

Neil

Re: [PATCH] dma-fence: Make dma_fence_add_callback() fail if signaled with error

[Ezequiel Garcia]

On Wed, 2018-05-16 at 12:26 +0200, Lucas Stach wrote:
> Am Mittwoch, den 16.05.2018, 11:42 +0200 schrieb Daniel Vetter:
> > On Tue, May 15, 2018 at 01:16:30PM +0100, Chris Wilson wrote:
> > > Quoting Ezequiel Garcia (2018-05-14 22:28:31)
> > > > On Mon, 2018-05-14 at 18:48 +0200, Daniel Vetter wrote:
> > > > > On Fri, May 11, 2018 at 08:27:41AM +0100, Chris Wilson wrote:
> > > > > > Quoting Ezequiel Garcia (2018-05-09 21:14:49)
> > > > > > > Change how dma_fence_add_callback() behaves, when the fence
> > > > > > > has error-signaled by the time it is being add. After this commit,
> > > > > > > dma_fence_add_callback() returns the fence error, if it
> > > > > > > has error-signaled before dma_fence_add_callback() is called.
> > > > > > 
> > > > > > Why? What problem are you trying to solve? fence->error does not 
> > > > > > imply
> > > > > > that the fence has yet been signaled, and the caller wants a 
> > > > > > callback
> > > > > > when it is signaled.
> > > > > 
> > > > > On top this is incosistent, e.g. we don't do the same for any of the 
> > > > > other
> > > > > dma_fence interfaces. Plus there's the issue that you might alias 
> > > > > errno
> > > > > values with fence errno values.
> > > > > 
> > > > 
> > > > Right.
> > > > 
> > > > > I think keeping the error codes from the functions you're calling 
> > > > > distinct
> > > > > from the error code of the fence itself makes a lot of sense. The 
> > > > > first
> > > > > tells you whether your request worked out (or why not), the second 
> > > > > tells
> > > > > you whether the asynchronous dma operation (gpu rendering, page flip,
> > > > > whatever) that the dma_fence represents worked out (or why not). 
> > > > > That's 2
> > > > > distinct things imo.
> > > > > 
> > > > > Might be good to show us the driver code that needs this behaviour so 
> > > > > we
> > > > > can discuss how to best handle your use-case.
> > > > > 
> > > > 
> > > > This change arose while discussing the in-fences support for 
> > > > video4linux.
> > > > Here's the patch that calls dma_fence_add_callback 
> > > > https://lkml.org/lkml/2018/5/4/766.
> > > > 
> > > > The code snippet currently looks something like this:
> > > > 
> > > > if (vb->in_fence) {
> > > > ret = dma_fence_add_callback(vb->in_fence, 
> > > > >fence_cb,
> > > > 
> > > >  vb2_qbuf_fence_cb);
> > > > /* is the fence signaled? */
> > > > if (ret == -ENOENT) {
> > > > 
> > > > dma_fence_put(vb->in_fence);
> > > > vb->in_fence = NULL;
> > > > } else if (ret)
> > > > {
> > > > goto unlock;
> > > > }
> > > > }
> > > > 
> > > > In this use case, if the callback is added successfully,
> > > > the video4linux core defers the activation of the buffer
> > > > until the fence signals.
> > > > 
> > > > If the fence is signaled (currently disregarding of errors)
> > > > then the buffer is assumed to be ready to be activated,
> > > > and so it gets queued for hardware usage.
> > > > 
> > > > Giving some more thought to this, I'm not so sure what is
> > > > the right action if a fence signaled with error. In this case,
> > > > it appears to me that we shouldn't be using this buffer
> > > > if its in-fence is in error, but perhaps I'm missing
> > > > something.
> > > 
> > > What I have in mind for async errors is to skip the operation and
> > > propagate the error onto the next fence. Mostly because those async
> > > errors may include fatal errors such as unable to pin the backing
> > > storage for the operation, but even "trivial" errors such as an early
> > > operation failing means that this request is then subject to garbage-in,
> > > garbage-out. However, for trivial errors I would just propagate the
> > > error status (so the caller knows something went wrong if they care, but
> > > in all likelihood no one will notice) and continue on with the glitchy
> > > operation.
> > 
> > In general, there's not really any hard rule about propagating fence
> > errors across devices. It's mostly just used by drivers internally to keep
> > track of failed stuff (gpu hangs or anything else async like Chris
> > describes here).
> > 
> > For v4l I'm not sure you want to care much about this, since right now the
> > main use of fence errors is gpu hang recovery (whether it's the driver or
> > hw that's hung doesn't matter here).
> 
> Yes, my understanding is that fence signal and errors are two distinct
> things that should not be conflated like it is done in this patch.
> 
> In my understanding signaling a fence means the HW or SW component
> which added the fence is done with the buffer and will not touch it
> anymore. In case of an unrecoverable error the fence will be signaled
> with error status set, so we correctly reflect the buffer status as
> being free to be used by whoever is waiting for it, but may contain

Re: Donation

[M.M. Fridman]

I Mikhail Fridman. has selected you specially as one of my beneficiaries
for my Charitable Donation, Just as I have declared on May 23, 2016 to give
my fortune as charity.

Check the link below for confirmation:

http://www.ibtimes.co.uk/russias-second-wealthiest-man-mikhail-fridman-plans-leaving-14-2bn-fortune-charity-1561604

Reply as soon as possible with further directives.

Best Regards,
Mikhail Fridman.

[PATCH 6/6] ARM: dts: rcar-gen2: Add 'data-active' property

[Jacopo Mondi]

The 'data-active' property needs to be specified when using embedded
synchronization. Add it to the Gen-2 boards using composite video input.

Signed-off-by: Jacopo Mondi 
---
 arch/arm/boot/dts/r8a7790-lager.dts   | 1 +
 arch/arm/boot/dts/r8a7791-koelsch.dts | 1 +
 arch/arm/boot/dts/r8a7791-porter.dts  | 1 +
 arch/arm/boot/dts/r8a7793-gose.dts| 1 +
 arch/arm/boot/dts/r8a7794-alt.dts | 1 +
 arch/arm/boot/dts/r8a7794-silk.dts| 1 +
 6 files changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7790-lager.dts 
b/arch/arm/boot/dts/r8a7790-lager.dts
index b56b309..48fcb44 100644
--- a/arch/arm/boot/dts/r8a7790-lager.dts
+++ b/arch/arm/boot/dts/r8a7790-lager.dts
@@ -893,6 +893,7 @@

vin1ep0: endpoint {
remote-endpoint = <>;
+   data-active = <1>;
};
};
 };
diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts 
b/arch/arm/boot/dts/r8a7791-koelsch.dts
index 9967666..fa0b25f 100644
--- a/arch/arm/boot/dts/r8a7791-koelsch.dts
+++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
@@ -868,6 +868,7 @@

vin1ep: endpoint {
remote-endpoint = <>;
+   data-active = <1>;
};
};
 };
diff --git a/arch/arm/boot/dts/r8a7791-porter.dts 
b/arch/arm/boot/dts/r8a7791-porter.dts
index 055a7f1..96b9605 100644
--- a/arch/arm/boot/dts/r8a7791-porter.dts
+++ b/arch/arm/boot/dts/r8a7791-porter.dts
@@ -391,6 +391,7 @@

vin0ep: endpoint {
remote-endpoint = <>;
+   data-active = <1>;
};
};
 };
diff --git a/arch/arm/boot/dts/r8a7793-gose.dts 
b/arch/arm/boot/dts/r8a7793-gose.dts
index 9d3fba2..80b4fa8 100644
--- a/arch/arm/boot/dts/r8a7793-gose.dts
+++ b/arch/arm/boot/dts/r8a7793-gose.dts
@@ -779,6 +779,7 @@

vin1ep: endpoint {
remote-endpoint = <_out>;
+   data-active = <1>;
};
};
 };
diff --git a/arch/arm/boot/dts/r8a7794-alt.dts 
b/arch/arm/boot/dts/r8a7794-alt.dts
index 4bbb9cc..00df605d 100644
--- a/arch/arm/boot/dts/r8a7794-alt.dts
+++ b/arch/arm/boot/dts/r8a7794-alt.dts
@@ -380,6 +380,7 @@

vin0ep: endpoint {
remote-endpoint = <>;
+   data-active = <1>;
};
};
 };
diff --git a/arch/arm/boot/dts/r8a7794-silk.dts 
b/arch/arm/boot/dts/r8a7794-silk.dts
index c0c5d31..ed17376 100644
--- a/arch/arm/boot/dts/r8a7794-silk.dts
+++ b/arch/arm/boot/dts/r8a7794-silk.dts
@@ -480,6 +480,7 @@

vin0ep: endpoint {
remote-endpoint = <>;
+   data-active = <1>;
};
};
 };
--
2.7.4

[PATCH 0/6] media: rcar-vin: Brush endpoint properties

[Jacopo Mondi]

Hello,
   this series touches the bindings and the driver handling endpoint
properties for digital subdevices of the R-Car VIN driver.

The first patch simply documents what are the endpoint properties supported
at the moment, then the second one extends them with 'data-active'.

As the VIN hardware allows to use HSYNC as data enable signal when the CLCKENB
pin is left unconnected, the 'data-active' property presence determinates
if HSYNC has to be used or not as data enable signal. As a consequence, when
running with embedded synchronism, and there is not HSYNC signal, it becomes
mandatory to specify 'data-active' polarity in DTS.

To address this, all Gen-2 boards featuring a composite video input and
running with embedded synchronization, now need that property to be specified
in DTS. Before adding it, remove un-used properties as 'pclk-sample' and
'bus-width' from the Gen-2 bindings, as they are not parsed by the VIN driver
and only confuse users.

Niklas, as you already know I don't have any Gen2 board. Could you give this
a spin on Koelsch if you like the series?

Thanks
   j

Jacopo Mondi (6):
  dt-bindings: media: rcar-vin: Describe optional ep properties
  dt-bindings: media: rcar-vin: Document data-active
  media: rcar-vin: Handle data-active property
  media: rcar-vin: Handle CLOCKENB pin polarity
  ARM: dts: rcar-gen2: Remove unused VIN properties
  ARM: dts: rcar-gen2: Add 'data-active' property

 Documentation/devicetree/bindings/media/rcar_vin.txt | 18 +-
 arch/arm/boot/dts/r8a7790-lager.dts  |  4 +---
 arch/arm/boot/dts/r8a7791-koelsch.dts|  4 +---
 arch/arm/boot/dts/r8a7791-porter.dts |  2 +-
 arch/arm/boot/dts/r8a7793-gose.dts   |  4 +---
 arch/arm/boot/dts/r8a7794-alt.dts|  2 +-
 arch/arm/boot/dts/r8a7794-silk.dts   |  2 +-
 drivers/media/platform/rcar-vin/rcar-core.c  | 10 --
 drivers/media/platform/rcar-vin/rcar-dma.c   | 11 +++
 9 files changed, 42 insertions(+), 15 deletions(-)

--
2.7.4

[PATCH 4/6] media: rcar-vin: Handle CLOCKENB pin polarity

[Jacopo Mondi]

Handle CLOCKENB pin polarity, or use HSYNC in its place if polarity is
not specified.

Signed-off-by: Jacopo Mondi 
---
 drivers/media/platform/rcar-vin/rcar-dma.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c 
b/drivers/media/platform/rcar-vin/rcar-dma.c
index ac07f99..7a84eae 100644
--- a/drivers/media/platform/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/rcar-vin/rcar-dma.c
@@ -123,6 +123,8 @@
 /* Video n Data Mode Register 2 bits */
 #define VNDMR2_VPS (1 << 30)
 #define VNDMR2_HPS (1 << 29)
+#define VNDMR2_CES (1 << 28)
+#define VNDMR2_CHS (1 << 23)
 #define VNDMR2_FTEV(1 << 17)
 #define VNDMR2_VLV(n)  ((n & 0xf) << 12)
 
@@ -691,6 +693,15 @@ static int rvin_setup(struct rvin_dev *vin)
dmr2 |= VNDMR2_VPS;
 
/*
+* Clock-enable active level select.
+* Use HSYNC as enable if not specified
+*/
+   if (vin->mbus_cfg.flags & V4L2_MBUS_DATA_ACTIVE_LOW)
+   dmr2 |= VNDMR2_CES;
+   else if (!(vin->mbus_cfg.flags & V4L2_MBUS_DATA_ACTIVE_HIGH))
+   dmr2 |= VNDMR2_CHS;
+
+   /*
 * Output format
 */
switch (vin->format.pixelformat) {
-- 
2.7.4

[PATCH 2/6] dt-bindings: media: rcar-vin: Document data-active

[Jacopo Mondi]

Document 'data-active' property in R-Car VIN device tree bindings.
The property is optional when running with explicit synchronization
(eg. BT.601) but mandatory when embedded synchronization is in use (eg.
BT.656) as specified by the hardware manual.

Signed-off-by: Jacopo Mondi 
---
 Documentation/devicetree/bindings/media/rcar_vin.txt | 5 +
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt 
b/Documentation/devicetree/bindings/media/rcar_vin.txt
index c53ce4e..17eac8a 100644
--- a/Documentation/devicetree/bindings/media/rcar_vin.txt
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
@@ -63,6 +63,11 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
If both HSYNC and VSYNC polarities are not specified, embedded
synchronization is selected.

+- data-active: active state of data enable signal (CLOCKENB pin).
+  0/1 for LOW/HIGH respectively. If not specified, use HSYNC as
+  data enable signal. When using embedded synchronization this
+  property is mandatory.
+
 - port 1 - sub-nodes describing one or more endpoints connected to
   the VIN from local SoC CSI-2 receivers. The endpoint numbers must
   use the following schema.
--
2.7.4

[PATCH 5/6] ARM: dts: rcar-gen2: Remove unused VIN properties

[Jacopo Mondi]

The 'bus-width' and 'pclk-sample' properties are not parsed by the VIN
driver and only confuse users. Remove them in all Gen2 SoC that used
them.

Signed-off-by: Jacopo Mondi 
---
 arch/arm/boot/dts/r8a7790-lager.dts   | 3 ---
 arch/arm/boot/dts/r8a7791-koelsch.dts | 3 ---
 arch/arm/boot/dts/r8a7791-porter.dts  | 1 -
 arch/arm/boot/dts/r8a7793-gose.dts| 3 ---
 arch/arm/boot/dts/r8a7794-alt.dts | 1 -
 arch/arm/boot/dts/r8a7794-silk.dts| 1 -
 6 files changed, 12 deletions(-)

diff --git a/arch/arm/boot/dts/r8a7790-lager.dts 
b/arch/arm/boot/dts/r8a7790-lager.dts
index 063fdb6..b56b309 100644
--- a/arch/arm/boot/dts/r8a7790-lager.dts
+++ b/arch/arm/boot/dts/r8a7790-lager.dts
@@ -873,10 +873,8 @@
port {
vin0ep2: endpoint {
remote-endpoint = <_out>;
-   bus-width = <24>;
hsync-active = <0>;
vsync-active = <0>;
-   pclk-sample = <1>;
data-active = <1>;
};
};
@@ -895,7 +893,6 @@

vin1ep0: endpoint {
remote-endpoint = <>;
-   bus-width = <8>;
};
};
 };
diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts 
b/arch/arm/boot/dts/r8a7791-koelsch.dts
index f40321a..9967666 100644
--- a/arch/arm/boot/dts/r8a7791-koelsch.dts
+++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
@@ -849,10 +849,8 @@

vin0ep2: endpoint {
remote-endpoint = <_out>;
-   bus-width = <24>;
hsync-active = <0>;
vsync-active = <0>;
-   pclk-sample = <1>;
data-active = <1>;
};
};
@@ -870,7 +868,6 @@

vin1ep: endpoint {
remote-endpoint = <>;
-   bus-width = <8>;
};
};
 };
diff --git a/arch/arm/boot/dts/r8a7791-porter.dts 
b/arch/arm/boot/dts/r8a7791-porter.dts
index c14e6fe..055a7f1 100644
--- a/arch/arm/boot/dts/r8a7791-porter.dts
+++ b/arch/arm/boot/dts/r8a7791-porter.dts
@@ -391,7 +391,6 @@

vin0ep: endpoint {
remote-endpoint = <>;
-   bus-width = <8>;
};
};
 };
diff --git a/arch/arm/boot/dts/r8a7793-gose.dts 
b/arch/arm/boot/dts/r8a7793-gose.dts
index 9ed6961..9d3fba2 100644
--- a/arch/arm/boot/dts/r8a7793-gose.dts
+++ b/arch/arm/boot/dts/r8a7793-gose.dts
@@ -759,10 +759,8 @@

vin0ep2: endpoint {
remote-endpoint = <_out>;
-   bus-width = <24>;
hsync-active = <0>;
vsync-active = <0>;
-   pclk-sample = <1>;
data-active = <1>;
};
};
@@ -781,7 +779,6 @@

vin1ep: endpoint {
remote-endpoint = <_out>;
-   bus-width = <8>;
};
};
 };
diff --git a/arch/arm/boot/dts/r8a7794-alt.dts 
b/arch/arm/boot/dts/r8a7794-alt.dts
index 26a8834..4bbb9cc 100644
--- a/arch/arm/boot/dts/r8a7794-alt.dts
+++ b/arch/arm/boot/dts/r8a7794-alt.dts
@@ -380,7 +380,6 @@

vin0ep: endpoint {
remote-endpoint = <>;
-   bus-width = <8>;
};
};
 };
diff --git a/arch/arm/boot/dts/r8a7794-silk.dts 
b/arch/arm/boot/dts/r8a7794-silk.dts
index 351cb3b..c0c5d31 100644
--- a/arch/arm/boot/dts/r8a7794-silk.dts
+++ b/arch/arm/boot/dts/r8a7794-silk.dts
@@ -480,7 +480,6 @@

vin0ep: endpoint {
remote-endpoint = <>;
-   bus-width = <8>;
};
};
 };
--
2.7.4

[PATCH 1/6] dt-bindings: media: rcar-vin: Describe optional ep properties

[Jacopo Mondi]

Describe the optional endpoint properties for endpoint nodes of port@0
and port@1 of the R-Car VIN driver device tree bindings documentation.

Signed-off-by: Jacopo Mondi 
---
 Documentation/devicetree/bindings/media/rcar_vin.txt | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt 
b/Documentation/devicetree/bindings/media/rcar_vin.txt
index a19517e1..c53ce4e 100644
--- a/Documentation/devicetree/bindings/media/rcar_vin.txt
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
@@ -53,6 +53,16 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
   from external SoC pins described in video-interfaces.txt[1].
   Describing more then one endpoint in port 0 is invalid. Only VIN
   instances that are connected to external pins should have port 0.
+
+  - Optional properties for endpoint nodes of port@0:
+- hsync-active: active state of the HSYNC signal, 0/1 for LOW/HIGH
+ respectively. Default is active high.
+- vsync-active: active state of the VSYNC signal, 0/1 for LOW/HIGH
+ respectively. Default is active high.
+
+   If both HSYNC and VSYNC polarities are not specified, embedded
+   synchronization is selected.
+
 - port 1 - sub-nodes describing one or more endpoints connected to
   the VIN from local SoC CSI-2 receivers. The endpoint numbers must
   use the following schema.
@@ -62,6 +72,8 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
 - Endpoint 2 - sub-node describing the endpoint connected to CSI40
 - Endpoint 3 - sub-node describing the endpoint connected to CSI41

+  Endpoint nodes of port@1 do not support any optional endpoint property.
+
 Device node example for Gen2 platforms
 --

@@ -112,7 +124,6 @@ Board setup example for Gen2 platforms (vin1 composite 
video input)

 vin1ep0: endpoint {
 remote-endpoint = <>;
-bus-width = <8>;
 };
 };
 };
--
2.7.4

[PATCH 3/6] media: rcar-vin: Handle data-active property

[Jacopo Mondi]

The data-active property has to be specified when running with embedded
synchronization. The VIN peripheral can use HSYNC in place of CLOCKENB
when the CLOCKENB pin is not connected, this requires explicit
synchronization to be in use.

Now that the driver supports 'data-active' property, it makes not sense
to zero the mbus configuration flags when running with implicit synch
(V4L2_MBUS_BT656).

Signed-off-by: Jacopo Mondi 
---
 drivers/media/platform/rcar-vin/rcar-core.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-core.c 
b/drivers/media/platform/rcar-vin/rcar-core.c
index d3072e1..075d08f 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -531,15 +531,21 @@ static int rvin_digital_parse_v4l2(struct device *dev,
return -ENOTCONN;
 
vin->mbus_cfg.type = vep->bus_type;
+   vin->mbus_cfg.flags = vep->bus.parallel.flags;
 
switch (vin->mbus_cfg.type) {
case V4L2_MBUS_PARALLEL:
vin_dbg(vin, "Found PARALLEL media bus\n");
-   vin->mbus_cfg.flags = vep->bus.parallel.flags;
break;
case V4L2_MBUS_BT656:
vin_dbg(vin, "Found BT656 media bus\n");
-   vin->mbus_cfg.flags = 0;
+
+   if (!(vin->mbus_cfg.flags & V4L2_MBUS_DATA_ACTIVE_HIGH) &&
+   !(vin->mbus_cfg.flags & V4L2_MBUS_DATA_ACTIVE_LOW)) {
+   vin_err(vin,
+   "Missing data enable signal polarity 
property\n");
+   return -EINVAL;
+   }
break;
default:
vin_err(vin, "Unknown media bus type\n");
-- 
2.7.4

RE: [RESEND PATCH v9 2/2] media: dw9807: Add dw9807 vcm driver

[Yeh, Andy]

Hi Mauro,


>-Original Message-
>From: Mauro Carvalho Chehab [mailto:mchehab+sams...@kernel.org] 
>Sent: Thursday, May 10, 2018 3:04 AM
>To: Yeh, Andy 
>Cc: linux-media@vger.kernel.org; sakari.ai...@linux.intel.com; 
>devicet...@vger.kernel.org; tf...@chromium.org; jac...@jmondi.org; Chiang, 
>AlanX 
>Subject: Re: [RESEND PATCH v9 2/2] media: dw9807: Add dw9807 vcm driver
>
>This adds a new warning.
>
>Thanks,
>Mauro
> 
>drivers/media/i2c/dw9807.c: In function 'dw9807_set_dac':
>drivers/media/i2c/dw9807.c:81:16: warning: unused variable 'retry' 
> [-Wunused-variable]
>  int val, ret, retry = 0;
>^
>
>Please either fix or fold the following patch.
>

I noticed you just submitted a patch to the list to address the warning. Thanks.
https://patchwork.linuxtv.org/patch/49575/

Just in the meantime, I uploaded the same one before noticing your patch.  I 
would like to obsolete mine, so let me know if you agree too. Thanks.
https://patchwork.linuxtv.org/patch/49574/


Regards, Andy

>diff --git a/drivers/media/i2c/dw9807.c b/drivers/media/i2c/dw9807.c index 
>28ede2b47acf..6ebb98717fb1 100644
>--- a/drivers/media/i2c/dw9807.c
>+++ b/drivers/media/i2c/dw9807.c
>@@ -78,7 +78,7 @@ static int dw9807_set_dac(struct i2c_client *client, u16 
>data)
>const char tx_data[3] = {
>DW9807_MSB_ADDR, ((data >> 8) & 0x03), (data & 0xff)
>};
>-   int val, ret, retry = 0;
>+   int val, ret;
> 
>/*
> * According to the datasheet, need to check the bus status before we

[PATCH] media: dw9807: remove an unused var

[Mauro Carvalho Chehab]

drivers/media/i2c/dw9807.c: In function 'dw9807_set_dac':
drivers/media/i2c/dw9807.c:81:16: warning: unused variable 'retry' 
[-Wunused-variable]
  int val, ret, retry = 0;
^

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/i2c/dw9807.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/dw9807.c b/drivers/media/i2c/dw9807.c
index 28ede2b47acf..6ebb98717fb1 100644
--- a/drivers/media/i2c/dw9807.c
+++ b/drivers/media/i2c/dw9807.c
@@ -78,7 +78,7 @@ static int dw9807_set_dac(struct i2c_client *client, u16 data)
const char tx_data[3] = {
DW9807_MSB_ADDR, ((data >> 8) & 0x03), (data & 0xff)
};
-   int val, ret, retry = 0;
+   int val, ret;
 
/*
 * According to the datasheet, need to check the bus status before we
-- 
2.17.0

[PATCH] media: dw9807: Fix a warning: unused variable 'retry'

[Andy Yeh]

Fix a warning reported by compiler, along with an incremental patch.
---
 drivers/media/i2c/dw9807.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/dw9807.c b/drivers/media/i2c/dw9807.c
index 28ede2b..6ebb987 100644
--- a/drivers/media/i2c/dw9807.c
+++ b/drivers/media/i2c/dw9807.c
@@ -78,7 +78,7 @@ static int dw9807_set_dac(struct i2c_client *client, u16 data)
const char tx_data[3] = {
DW9807_MSB_ADDR, ((data >> 8) & 0x03), (data & 0xff)
};
-   int val, ret, retry = 0;
+   int val, ret;
 
/*
 * According to the datasheet, need to check the bus status before we
-- 
2.7.4

Re: [PATCH v6 04/17] media: rkisp1: add Rockchip MIPI Synopsys DPHY driver

[Tomasz Figa]

Hi Jacob,

On Wed, May 16, 2018 at 11:54 PM Jacob Chen  wrote:

> 2018-05-16 22:39 GMT+08:00 Jacob Chen :
> > Hi Laurent,
> >
> > 2018-05-16 13:20 GMT+08:00 Laurent Pinchart <
laurent.pinch...@ideasonboard.com>:
> >> Hi Jacob,
> >>
> >> Thank you for the patch.
> >>
> >> On Thursday, 8 March 2018 11:47:54 EEST Jacob Chen wrote:
> >>> From: Jacob Chen 
> >>>
> >>> This commit adds a subdev driver for Rockchip MIPI Synopsys DPHY
driver
> >>
> >> Should this really be a subdev driver ? After a quick look at the
code, the
> >> only parameters you need to configure the PHY is the number of lanes
and the
> >> data rate. Implementing the whole subdev API seems overcomplicated to
me,
> >> especially given that the D-PHY doesn't deal with video streams as
such, but
> >> operates one level down. Shouldn't we model the D-PHY using the Linux
PHY
> >> framework ? I believe all the features you need are there except for a
D-PHY-
> >> specific configuration function that should be very easy to add.
> >>
> >
> > It deserves a subdev driver since the ISP is not the only user.
> > Other driver, like VIP, use it too.
> >
> >

> For example, if there are two sensors connected to a rk3399 board.

> Sensor1 --> DPHY1
> Sensor2 --> DPHY2

> With a subdev phy driver, i can choose either ISP or VIP for
> sensor1/sensor2 by enable/disable media link in the run time.
> 1.
> Sensor1 --> DPHY1 ---> VIP
> Sensor2 --> DPHY2 ---> ISP1
> 2.
> Sensor1 --> DPHY1 ---> ISP1
> Sensor2 --> DPHY2 ---> VIP


What is VIP?

Also, if we model the DPHY using the PHY interface, it will be still
possible to achieve the same, just by toggling the link between sensor and
VIP or ISP1:

1.

Sensor1 ---|~|--- VIP
 \ | (PHY interface)
  \   DPHY1
   \   | (PHY interface)
\---| |-- ISP1

Sensor2 ---| |-- VIP
 \ | (PHY interface)
  \   DPHY2
   \   | (PHY interface)
\---|~|-- ISP1

2.

Sensor1 ---| |-- VIP
 \ | (PHY interface)
  \   DPHY1
   \   | (PHY interface)
\---|~|-- ISP1

Sensor2 ---|~|-- VIP
 \ | (PHY interface)
  \   DPHY2
   \   | (PHY interface)
\---| |-- ISP1

Best regards,
Tomasz

Re: [PATCH v6 04/17] media: rkisp1: add Rockchip MIPI Synopsys DPHY driver

[Jacob Chen]

2018-05-16 22:39 GMT+08:00 Jacob Chen :
> Hi Laurent,
>
> 2018-05-16 13:20 GMT+08:00 Laurent Pinchart 
> :
>> Hi Jacob,
>>
>> Thank you for the patch.
>>
>> On Thursday, 8 March 2018 11:47:54 EEST Jacob Chen wrote:
>>> From: Jacob Chen 
>>>
>>> This commit adds a subdev driver for Rockchip MIPI Synopsys DPHY driver
>>
>> Should this really be a subdev driver ? After a quick look at the code, the
>> only parameters you need to configure the PHY is the number of lanes and the
>> data rate. Implementing the whole subdev API seems overcomplicated to me,
>> especially given that the D-PHY doesn't deal with video streams as such, but
>> operates one level down. Shouldn't we model the D-PHY using the Linux PHY
>> framework ? I believe all the features you need are there except for a D-PHY-
>> specific configuration function that should be very easy to add.
>>
>
> It deserves a subdev driver since the ISP is not the only user.
> Other driver, like VIP, use it too.
>
>

For example, if there are two sensors connected to a rk3399 board.

Sensor1 --> DPHY1
Sensor2 --> DPHY2

With a subdev phy driver, i can choose either ISP or VIP for
sensor1/sensor2 by enable/disable media link in the run time.
1.
Sensor1 --> DPHY1 ---> VIP
Sensor2 --> DPHY2 ---> ISP1
2.
Sensor1 --> DPHY1 ---> ISP1
Sensor2 --> DPHY2 ---> VIP



>>> Signed-off-by: Jacob Chen 
>>> Signed-off-by: Shunqian Zheng 
>>> Signed-off-by: Tomasz Figa 
>>> ---
>>>  .../media/platform/rockchip/isp1/mipi_dphy_sy.c| 868 ++
>>>  .../media/platform/rockchip/isp1/mipi_dphy_sy.h|  15 +
>>>  2 files changed, 883 insertions(+)
>>>  create mode 100644 drivers/media/platform/rockchip/isp1/mipi_dphy_sy.c
>>>  create mode 100644 drivers/media/platform/rockchip/isp1/mipi_dphy_sy.h
>>>
>>> diff --git a/drivers/media/platform/rockchip/isp1/mipi_dphy_sy.c
>>> b/drivers/media/platform/rockchip/isp1/mipi_dphy_sy.c new file mode 100644
>>> index ..32140960557a
>>> --- /dev/null
>>> +++ b/drivers/media/platform/rockchip/isp1/mipi_dphy_sy.c
>>> @@ -0,0 +1,868 @@
>>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>>> +/*
>>> + * Rockchip MIPI Synopsys DPHY driver
>>> + *
>>> + * Copyright (C) 2017 Fuzhou Rockchip Electronics Co., Ltd.
>>> + */
>>> +
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +
>>> +#define RK3288_GRF_SOC_CON6  0x025c
>>> +#define RK3288_GRF_SOC_CON8  0x0264
>>> +#define RK3288_GRF_SOC_CON9  0x0268
>>> +#define RK3288_GRF_SOC_CON10 0x026c
>>> +#define RK3288_GRF_SOC_CON14 0x027c
>>> +#define RK3288_GRF_SOC_STATUS21  0x02d4
>>> +#define RK3288_GRF_IO_VSEL   0x0380
>>> +#define RK3288_GRF_SOC_CON15 0x03a4
>>> +
>>> +#define RK3399_GRF_SOC_CON9  0x6224
>>> +#define RK3399_GRF_SOC_CON21 0x6254
>>> +#define RK3399_GRF_SOC_CON22 0x6258
>>> +#define RK3399_GRF_SOC_CON23 0x625c
>>> +#define RK3399_GRF_SOC_CON24 0x6260
>>> +#define RK3399_GRF_SOC_CON25 0x6264
>>> +#define RK3399_GRF_SOC_STATUS1   0xe2a4
>>> +
>>> +#define CLOCK_LANE_HS_RX_CONTROL 0x34
>>> +#define LANE0_HS_RX_CONTROL  0x44
>>> +#define LANE1_HS_RX_CONTROL  0x54
>>> +#define LANE2_HS_RX_CONTROL  0x84
>>> +#define LANE3_HS_RX_CONTROL  0x94
>>> +#define HS_RX_DATA_LANES_THS_SETTLE_CONTROL  0x75
>>> +
>>> +/*
>>> + * CSI HOST
>>> + */
>>> +#define CSIHOST_PHY_TEST_CTRL0   0x30
>>> +#define CSIHOST_PHY_TEST_CTRL1   0x34
>>> +#define CSIHOST_PHY_SHUTDOWNZ0x08
>>> +#define CSIHOST_DPHY_RSTZ0x0c
>>> +
>>> +#define PHY_TESTEN_ADDR  (0x1 << 16)
>>> +#define PHY_TESTEN_DATA  (0x0 << 16)
>>> +#define PHY_TESTCLK  (0x1 << 1)
>>> +#define PHY_TESTCLR  (0x1 << 0)
>>> +#define THS_SETTLE_COUNTER_THRESHOLD 0x04
>>> +
>>> +#define HIWORD_UPDATE(val, mask, shift) \
>>> + ((val) << (shift) | (mask) << ((shift) + 16))
>>> +
>>> +enum mipi_dphy_sy_pads {
>>> + MIPI_DPHY_SY_PAD_SINK = 0,
>>> + MIPI_DPHY_SY_PAD_SOURCE,
>>> + MIPI_DPHY_SY_PADS_NUM,
>>> +};
>>> +
>>> +enum dphy_reg_id {
>>> + GRF_DPHY_RX0_TURNDISABLE = 0,
>>> + GRF_DPHY_RX0_FORCERXMODE,
>>> + GRF_DPHY_RX0_FORCETXSTOPMODE,
>>> + GRF_DPHY_RX0_ENABLE,
>>> + GRF_DPHY_RX0_TESTCLR,
>>> + GRF_DPHY_RX0_TESTCLK,
>>> + GRF_DPHY_RX0_TESTEN,
>>> + GRF_DPHY_RX0_TESTDIN,
>>> + GRF_DPHY_RX0_TURNREQUEST,
>>> + GRF_DPHY_RX0_TESTDOUT,
>>> + GRF_DPHY_TX0_TURNDISABLE,
>>> + GRF_DPHY_TX0_FORCERXMODE,
>>> + GRF_DPHY_TX0_FORCETXSTOPMODE,
>>> + GRF_DPHY_TX0_TURNREQUEST,
>>> + GRF_DPHY_TX1RX1_TURNDISABLE,
>>> + GRF_DPHY_TX1RX1_FORCERXMODE,
>>> + 

Re: [PATCH v6 04/17] media: rkisp1: add Rockchip MIPI Synopsys DPHY driver

[Jacob Chen]

Hi Laurent,

2018-05-16 13:20 GMT+08:00 Laurent Pinchart :
> Hi Jacob,
>
> Thank you for the patch.
>
> On Thursday, 8 March 2018 11:47:54 EEST Jacob Chen wrote:
>> From: Jacob Chen 
>>
>> This commit adds a subdev driver for Rockchip MIPI Synopsys DPHY driver
>
> Should this really be a subdev driver ? After a quick look at the code, the
> only parameters you need to configure the PHY is the number of lanes and the
> data rate. Implementing the whole subdev API seems overcomplicated to me,
> especially given that the D-PHY doesn't deal with video streams as such, but
> operates one level down. Shouldn't we model the D-PHY using the Linux PHY
> framework ? I believe all the features you need are there except for a D-PHY-
> specific configuration function that should be very easy to add.
>

It deserves a subdev driver since the ISP is not the only user.
Other driver, like VIP, use it too.


>> Signed-off-by: Jacob Chen 
>> Signed-off-by: Shunqian Zheng 
>> Signed-off-by: Tomasz Figa 
>> ---
>>  .../media/platform/rockchip/isp1/mipi_dphy_sy.c| 868 ++
>>  .../media/platform/rockchip/isp1/mipi_dphy_sy.h|  15 +
>>  2 files changed, 883 insertions(+)
>>  create mode 100644 drivers/media/platform/rockchip/isp1/mipi_dphy_sy.c
>>  create mode 100644 drivers/media/platform/rockchip/isp1/mipi_dphy_sy.h
>>
>> diff --git a/drivers/media/platform/rockchip/isp1/mipi_dphy_sy.c
>> b/drivers/media/platform/rockchip/isp1/mipi_dphy_sy.c new file mode 100644
>> index ..32140960557a
>> --- /dev/null
>> +++ b/drivers/media/platform/rockchip/isp1/mipi_dphy_sy.c
>> @@ -0,0 +1,868 @@
>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> +/*
>> + * Rockchip MIPI Synopsys DPHY driver
>> + *
>> + * Copyright (C) 2017 Fuzhou Rockchip Electronics Co., Ltd.
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#define RK3288_GRF_SOC_CON6  0x025c
>> +#define RK3288_GRF_SOC_CON8  0x0264
>> +#define RK3288_GRF_SOC_CON9  0x0268
>> +#define RK3288_GRF_SOC_CON10 0x026c
>> +#define RK3288_GRF_SOC_CON14 0x027c
>> +#define RK3288_GRF_SOC_STATUS21  0x02d4
>> +#define RK3288_GRF_IO_VSEL   0x0380
>> +#define RK3288_GRF_SOC_CON15 0x03a4
>> +
>> +#define RK3399_GRF_SOC_CON9  0x6224
>> +#define RK3399_GRF_SOC_CON21 0x6254
>> +#define RK3399_GRF_SOC_CON22 0x6258
>> +#define RK3399_GRF_SOC_CON23 0x625c
>> +#define RK3399_GRF_SOC_CON24 0x6260
>> +#define RK3399_GRF_SOC_CON25 0x6264
>> +#define RK3399_GRF_SOC_STATUS1   0xe2a4
>> +
>> +#define CLOCK_LANE_HS_RX_CONTROL 0x34
>> +#define LANE0_HS_RX_CONTROL  0x44
>> +#define LANE1_HS_RX_CONTROL  0x54
>> +#define LANE2_HS_RX_CONTROL  0x84
>> +#define LANE3_HS_RX_CONTROL  0x94
>> +#define HS_RX_DATA_LANES_THS_SETTLE_CONTROL  0x75
>> +
>> +/*
>> + * CSI HOST
>> + */
>> +#define CSIHOST_PHY_TEST_CTRL0   0x30
>> +#define CSIHOST_PHY_TEST_CTRL1   0x34
>> +#define CSIHOST_PHY_SHUTDOWNZ0x08
>> +#define CSIHOST_DPHY_RSTZ0x0c
>> +
>> +#define PHY_TESTEN_ADDR  (0x1 << 16)
>> +#define PHY_TESTEN_DATA  (0x0 << 16)
>> +#define PHY_TESTCLK  (0x1 << 1)
>> +#define PHY_TESTCLR  (0x1 << 0)
>> +#define THS_SETTLE_COUNTER_THRESHOLD 0x04
>> +
>> +#define HIWORD_UPDATE(val, mask, shift) \
>> + ((val) << (shift) | (mask) << ((shift) + 16))
>> +
>> +enum mipi_dphy_sy_pads {
>> + MIPI_DPHY_SY_PAD_SINK = 0,
>> + MIPI_DPHY_SY_PAD_SOURCE,
>> + MIPI_DPHY_SY_PADS_NUM,
>> +};
>> +
>> +enum dphy_reg_id {
>> + GRF_DPHY_RX0_TURNDISABLE = 0,
>> + GRF_DPHY_RX0_FORCERXMODE,
>> + GRF_DPHY_RX0_FORCETXSTOPMODE,
>> + GRF_DPHY_RX0_ENABLE,
>> + GRF_DPHY_RX0_TESTCLR,
>> + GRF_DPHY_RX0_TESTCLK,
>> + GRF_DPHY_RX0_TESTEN,
>> + GRF_DPHY_RX0_TESTDIN,
>> + GRF_DPHY_RX0_TURNREQUEST,
>> + GRF_DPHY_RX0_TESTDOUT,
>> + GRF_DPHY_TX0_TURNDISABLE,
>> + GRF_DPHY_TX0_FORCERXMODE,
>> + GRF_DPHY_TX0_FORCETXSTOPMODE,
>> + GRF_DPHY_TX0_TURNREQUEST,
>> + GRF_DPHY_TX1RX1_TURNDISABLE,
>> + GRF_DPHY_TX1RX1_FORCERXMODE,
>> + GRF_DPHY_TX1RX1_FORCETXSTOPMODE,
>> + GRF_DPHY_TX1RX1_ENABLE,
>> + GRF_DPHY_TX1RX1_MASTERSLAVEZ,
>> + GRF_DPHY_TX1RX1_BASEDIR,
>> + GRF_DPHY_TX1RX1_ENABLECLK,
>> + GRF_DPHY_TX1RX1_TURNREQUEST,
>> + GRF_DPHY_RX1_SRC_SEL,
>> + /* rk3288 only */
>> + GRF_CON_DISABLE_ISP,
>> + GRF_CON_ISP_DPHY_SEL,
>> + GRF_DSI_CSI_TESTBUS_SEL,
>> + GRF_DVP_V18SEL,
>> + /* below is for rk3399 only */
>> + GRF_DPHY_RX0_CLK_INV_SEL,
>> + GRF_DPHY_RX1_CLK_INV_SEL,
>> +};
>> +
>> +struct dphy_reg {
>> + u32 offset;
>> +   

Re: [Intel-gfx] [PATCH v2 2/5] drm/i915: hdmi: add CEC notifier to intel_hdmi

[Ville Syrjälä]

On Wed, May 16, 2018 at 09:40:17AM +0200, Neil Armstrong wrote:
> On 16/05/2018 09:31, Neil Armstrong wrote:
> > Hi Ville,
> > 
> > On 15/05/2018 17:35, Ville Syrjälä wrote:
> >> On Tue, May 15, 2018 at 04:42:19PM +0200, Neil Armstrong wrote:
> >>> This patchs adds the cec_notifier feature to the intel_hdmi part
> >>> of the i915 DRM driver. It uses the HDMI DRM connector name to 
> >>> differentiate
> >>> between each HDMI ports.
> >>> The changes will allow the i915 HDMI code to notify EDID and HPD changes
> >>> to an eventual CEC adapter.
> >>>
> >>> Signed-off-by: Neil Armstrong 
> >>> ---
> >>>  drivers/gpu/drm/i915/Kconfig  |  1 +
> >>>  drivers/gpu/drm/i915/intel_drv.h  |  2 ++
> >>>  drivers/gpu/drm/i915/intel_hdmi.c | 12 
> >>>  3 files changed, 15 insertions(+)
> >>>
> > 
> > [..]
> > 
> >>>  }
> >>>  
> >>> @@ -2031,6 +2037,8 @@ static void chv_hdmi_pre_enable(struct 
> >>> intel_encoder *encoder,
> >>>  
> >>>  static void intel_hdmi_destroy(struct drm_connector *connector)
> >>>  {
> >>> + if (intel_attached_hdmi(connector)->notifier)
> >>> + cec_notifier_put(intel_attached_hdmi(connector)->notifier);
> >>>   kfree(to_intel_connector(connector)->detect_edid);
> >>>   drm_connector_cleanup(connector);
> >>>   kfree(connector);
> >>> @@ -2358,6 +2366,10 @@ void intel_hdmi_init_connector(struct 
> >>> intel_digital_port *intel_dig_port,
> >>>   u32 temp = I915_READ(PEG_BAND_GAP_DATA);
> >>>   I915_WRITE(PEG_BAND_GAP_DATA, (temp & ~0xf) | 0xd);
> >>>   }
> >>> +
> >>> + intel_hdmi->notifier = cec_notifier_get_conn(dev->dev, connector->name);
> >>
> >> What are we doing with the connector name here? Those are not actually
> >> guaranteed to be stable, and any change in the connector probe order
> >> may cause the name to change.
> > 
> > The i915 driver can expose multiple HDMI connectors, but each connected will
> > have a different EDID and CEC physical address, so we will need a different 
> > notifier
> > for each connector.
> > 
> > The connector name is DRM dependent, but user-space actually uses this name 
> > for
> > operations, so I supposed it was kind of stable.
> > 
> > Anyway, is there another stable id/name/whatever that can be used to make a 
> > name to
> > distinguish the HDMI connectors ?
> 
> Would "HDMI %c", port_name(port) be OK to use ?

Yeah, something like seems like a reasonable approach. That does mean
we have to be a little careful with changing enum port or port_name()
in the future, but I guess we could just add a small function to
generated the name/id specifically for this thing.

We're also going to have to think what to do with enum port and
port_name() on ICL+ though. There we won't just have A-F but also
TC1-TC. Hmm. Looks like what we have for those ports in our
codebase ATM is a bit wonky since we haven't even changed
port_name() to give us the TC type names.

Also we might not want "HDMI" in the identifier since the physical
port is not HDMI specific. There are different things we could call
these but I think we could just go with a generic "Port A-F" and
"Port TC1-TC" approach. I think something like that should work
fine for current and upcoming hardware. And in theory that could
then be used for other things as well which need a stable
identifier.

Oh, and now I recall that input subsystem at least has some kind
of "physical path" property on devices. So I guess what we're
dicussing is a somewhat similar idea. I think input drivers
generally include the pci/usb/etc. device in the path as well.
Even though we currently only ever have the one pci device it
would seem like decent idea to include that information in our
identifiers as well. Or what do you think?

-- 
Ville Syrjälä
Intel

Re: [PATCH v2 1/2] media: rc: introduce BPF_PROG_RAWIR_EVENT

[Sean Young]

On Tue, May 15, 2018 at 07:50:19PM +0100, Sean Young wrote:
> Add support for BPF_PROG_RAWIR_EVENT. This type of BPF program can call
> rc_keydown() to reported decoded IR scancodes, or rc_repeat() to report
> that the last key should be repeated.
> 
> The bpf program can be attached to using the bpf(BPF_PROG_ATTACH) syscall;
> the target_fd must be the /dev/lircN device.

The locking of the list of attached bpf programs is broken in various ways.
I'll have to rework this for v3.


Sean

> 
> Signed-off-by: Sean Young 
> ---
>  drivers/media/rc/Kconfig   |  10 +
>  drivers/media/rc/Makefile  |   1 +
>  drivers/media/rc/bpf-rawir-event.c | 322 +
>  drivers/media/rc/lirc_dev.c|  28 +++
>  drivers/media/rc/rc-core-priv.h|  19 ++
>  drivers/media/rc/rc-ir-raw.c   |   3 +
>  include/linux/bpf_rcdev.h  |  30 +++
>  include/linux/bpf_types.h  |   3 +
>  include/uapi/linux/bpf.h   |  55 -
>  kernel/bpf/syscall.c   |   7 +
>  10 files changed, 477 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/media/rc/bpf-rawir-event.c
>  create mode 100644 include/linux/bpf_rcdev.h
> 
> diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
> index eb2c3b6eca7f..55747af5b978 100644
> --- a/drivers/media/rc/Kconfig
> +++ b/drivers/media/rc/Kconfig
> @@ -25,6 +25,16 @@ config LIRC
>  passes raw IR to and from userspace, which is needed for
>  IR transmitting (aka "blasting") and for the lirc daemon.
>  
> +config BPF_RAWIR_EVENT
> + bool "Enable attaching BPF programs to lirc devices"
> + depends on BPF_SYSCALL
> + depends on RC_CORE=y
> + depends on LIRC
> + help
> +Enable this option to make it possible to load custom IR
> +decoders written in BPF. These decoders are type
> +BPF_PROG_TYPE_RAW_IR_EVENT.
> +
>  menuconfig RC_DECODERS
>   bool "Remote controller decoders"
>   depends on RC_CORE
> diff --git a/drivers/media/rc/Makefile b/drivers/media/rc/Makefile
> index 2e1c87066f6c..74907823bef8 100644
> --- a/drivers/media/rc/Makefile
> +++ b/drivers/media/rc/Makefile
> @@ -5,6 +5,7 @@ obj-y += keymaps/
>  obj-$(CONFIG_RC_CORE) += rc-core.o
>  rc-core-y := rc-main.o rc-ir-raw.o
>  rc-core-$(CONFIG_LIRC) += lirc_dev.o
> +rc-core-$(CONFIG_BPF_RAWIR_EVENT) += bpf-rawir-event.o
>  obj-$(CONFIG_IR_NEC_DECODER) += ir-nec-decoder.o
>  obj-$(CONFIG_IR_RC5_DECODER) += ir-rc5-decoder.o
>  obj-$(CONFIG_IR_RC6_DECODER) += ir-rc6-decoder.o
> diff --git a/drivers/media/rc/bpf-rawir-event.c 
> b/drivers/media/rc/bpf-rawir-event.c
> new file mode 100644
> index ..8007841977d6
> --- /dev/null
> +++ b/drivers/media/rc/bpf-rawir-event.c
> @@ -0,0 +1,322 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// bpf-rawir-event.c - handles bpf
> +//
> +// Copyright (C) 2018 Sean Young 
> +
> +#include 
> +#include 
> +#include "rc-core-priv.h"
> +
> +/*
> + * BPF interface for raw IR
> + */
> +const struct bpf_prog_ops rawir_event_prog_ops = {
> +};
> +
> +BPF_CALL_1(bpf_rc_repeat, struct bpf_rawir_event*, event)
> +{
> + struct ir_raw_event_ctrl *ctrl;
> +
> + ctrl = container_of(event, struct ir_raw_event_ctrl, bpf_rawir_event);
> +
> + rc_repeat(ctrl->dev);
> +
> + return 0;
> +}
> +
> +static const struct bpf_func_proto rc_repeat_proto = {
> + .func  = bpf_rc_repeat,
> + .gpl_only  = true, /* rc_repeat is EXPORT_SYMBOL_GPL */
> + .ret_type  = RET_INTEGER,
> + .arg1_type = ARG_PTR_TO_CTX,
> +};
> +
> +BPF_CALL_4(bpf_rc_keydown, struct bpf_rawir_event*, event, u32, protocol,
> +u32, scancode, u32, toggle)
> +{
> + struct ir_raw_event_ctrl *ctrl;
> +
> + ctrl = container_of(event, struct ir_raw_event_ctrl, bpf_rawir_event);
> +
> + rc_keydown(ctrl->dev, protocol, scancode, toggle != 0);
> +
> + return 0;
> +}
> +
> +static const struct bpf_func_proto rc_keydown_proto = {
> + .func  = bpf_rc_keydown,
> + .gpl_only  = true, /* rc_keydown is EXPORT_SYMBOL_GPL */
> + .ret_type  = RET_INTEGER,
> + .arg1_type = ARG_PTR_TO_CTX,
> + .arg2_type = ARG_ANYTHING,
> + .arg3_type = ARG_ANYTHING,
> + .arg4_type = ARG_ANYTHING,
> +};
> +
> +static const struct bpf_func_proto *
> +rawir_event_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> +{
> + switch (func_id) {
> + case BPF_FUNC_rc_repeat:
> + return _repeat_proto;
> + case BPF_FUNC_rc_keydown:
> + return _keydown_proto;
> + case BPF_FUNC_map_lookup_elem:
> + return _map_lookup_elem_proto;
> + case BPF_FUNC_map_update_elem:
> + return _map_update_elem_proto;
> + case BPF_FUNC_map_delete_elem:
> + return _map_delete_elem_proto;
> + case BPF_FUNC_ktime_get_ns:
> + return _ktime_get_ns_proto;
> + case BPF_FUNC_tail_call:
> + return _tail_call_proto;
> + case 

Re: [PATCH 01/11] media: tm6000: fix potential Spectre variant 1

[Mauro Carvalho Chehab]

Em Wed, 16 May 2018 16:11:08 +0300
Dan Carpenter  escreveu:

> On Tue, May 15, 2018 at 04:00:33PM -0300, Mauro Carvalho Chehab wrote:
> > Yeah, that's the same I'm getting from media upstream.
> >   
> > > drivers/media/cec/cec-pin-error-inj.c:170 cec_pin_error_inj_parse_line() 
> > > warn: potential spectre issue 'pin->error_inj_args'  
> > 
> > This one seems a false positive, as the index var is u8 and the
> > array has 256 elements, as the userspace input from 'op' is 
> > initialized with:
> > 
> > u8 v;
> > u32 op;
> > 
> > if (!kstrtou8(token, 0, ))
> > op = v;
> >   
> 
> It's hard to silence this because Smatch stores the current user
> controlled range list, not what it was initially.  I wrote all this code
> to detect bounds checking errors, so there wasn't any need to save the
> range list before the bounds check.  Since "op" is a u32, I can't even
> go by the type of the index

Yeah, I was thinking that is would be harder to clean this up on
smatch. I proposed a patch to the ML that simplifies the logic,
making easier for both humans and Smatch to better understand how
the arrays are indexed.

> 
> > > drivers/media/dvb-core/dvb_ca_en50221.c:1479 dvb_ca_en50221_io_write() 
> > > warn: potential spectre issue 'ca->slot_info' (local cap)  
> > 
> > This one seems a real issue to me. Sent a patch for it.
> >   
> > > drivers/media/dvb-core/dvb_net.c:252 handle_one_ule_extension() warn: 
> > > potential spectre issue 'p->ule_next_hdr'  
> > 
> > I failed to see what's wrong here, or if this is exploited.   
> 
> Oh...  Huh.  This is a bug in smatch.  That line looks like:
> 
>   p->ule_sndu_type = ntohs(*(__be16 *)(p->ule_next_hdr + ((p->ule_dbit ? 
> 2 : 3) * ETH_ALEN)));
> 
> Smatch see the ntohs() and marks everything inside it as untrusted
> network data.  I'll fix this.

Thanks!

Regards,
Mauro

Re: [PATCH 01/11] media: tm6000: fix potential Spectre variant 1

[Dan Carpenter]

On Tue, May 15, 2018 at 04:00:33PM -0300, Mauro Carvalho Chehab wrote:
> Yeah, that's the same I'm getting from media upstream.
> 
> > drivers/media/cec/cec-pin-error-inj.c:170 cec_pin_error_inj_parse_line() 
> > warn: potential spectre issue 'pin->error_inj_args'
> 
> This one seems a false positive, as the index var is u8 and the
> array has 256 elements, as the userspace input from 'op' is 
> initialized with:
> 
>   u8 v;
>   u32 op;
> 
>   if (!kstrtou8(token, 0, ))
>   op = v;
> 

It's hard to silence this because Smatch stores the current user
controlled range list, not what it was initially.  I wrote all this code
to detect bounds checking errors, so there wasn't any need to save the
range list before the bounds check.  Since "op" is a u32, I can't even
go by the type of the index

> > drivers/media/dvb-core/dvb_ca_en50221.c:1479 dvb_ca_en50221_io_write() 
> > warn: potential spectre issue 'ca->slot_info' (local cap)
> 
> This one seems a real issue to me. Sent a patch for it.
> 
> > drivers/media/dvb-core/dvb_net.c:252 handle_one_ule_extension() warn: 
> > potential spectre issue 'p->ule_next_hdr'
> 
> I failed to see what's wrong here, or if this is exploited. 

Oh...  Huh.  This is a bug in smatch.  That line looks like:

p->ule_sndu_type = ntohs(*(__be16 *)(p->ule_next_hdr + ((p->ule_dbit ? 
2 : 3) * ETH_ALEN)));

Smatch see the ntohs() and marks everything inside it as untrusted
network data.  I'll fix this.

regards,
dan carpenter

[PATCH v2 3/4] media: rcar-vin: Handle digital subdev in link_notify

[Jacopo Mondi]

Handle digital subdevices in link_notify callback. If the notified link
involves a digital subdevice, do not change routing of the VIN-CSI-2
devices.

Signed-off-by: Jacopo Mondi 
---
 drivers/media/platform/rcar-vin/rcar-core.c | 30 +++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-core.c 
b/drivers/media/platform/rcar-vin/rcar-core.c
index 1003c8c..ea74c55 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -168,10 +168,36 @@ static int rvin_group_link_notify(struct media_link 
*link, u32 flags,
}
 
/* Add the new link to the existing mask and check if it works. */
-   csi_id = rvin_group_entity_to_csi_id(group, link->source->entity);
channel = rvin_group_csi_pad_to_channel(link->source->index);
-   mask_new = mask & rvin_group_get_mask(vin, csi_id, channel);
+   csi_id = rvin_group_entity_to_csi_id(group, link->source->entity);
+   if (csi_id == -ENODEV) {
+   struct v4l2_subdev *sd;
+   unsigned int i;
+
+   /*
+* Make sure the source entity subdevice is registered as
+* a digital input of one of the enabled VINs if it is not
+* one of the CSI-2 subdevices.
+*
+* No hardware configuration required for digital inputs,
+* we can return here.
+*/
+   sd = media_entity_to_v4l2_subdev(link->source->entity);
+   for (i = 0; i < RCAR_VIN_NUM; i++) {
+   if (group->vin[i] && group->vin[i]->digital &&
+   group->vin[i]->digital->subdev == sd) {
+   ret = 0;
+   goto out;
+   }
+   }
+
+   vin_err(vin, "Subdevice %s not registered to any VIN\n",
+   link->source->entity->name);
+   ret = -ENODEV;
+   goto out;
+   }
 
+   mask_new = mask & rvin_group_get_mask(vin, csi_id, channel);
vin_dbg(vin, "Try link change mask: 0x%x new: 0x%x\n", mask, mask_new);
 
if (!mask_new) {
-- 
2.7.4

[PATCH v2 4/4] media: rcar-vin: Add support for R-Car R8A77995 SoC

[Jacopo Mondi]

Add R-Car R8A77995 SoC to the rcar-vin supported ones.

Signed-off-by: Jacopo Mondi 
Reviewed-by: Niklas Söderlund 
Reviewed-by: Laurent Pinchart 
---
 drivers/media/platform/rcar-vin/rcar-core.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/drivers/media/platform/rcar-vin/rcar-core.c 
b/drivers/media/platform/rcar-vin/rcar-core.c
index ea74c55..fba8610 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -1104,6 +1104,10 @@ static const struct rvin_group_route 
_rcar_info_r8a77970_routes[] = {
{ /* Sentinel */ }
 };
 
+static const struct rvin_group_route _rcar_info_r8a77995_routes[] = {
+   { /* Sentinel */ }
+};
+
 static const struct rvin_info rcar_info_r8a77970 = {
.model = RCAR_GEN3,
.use_mc = true,
@@ -1112,6 +1116,14 @@ static const struct rvin_info rcar_info_r8a77970 = {
.routes = _rcar_info_r8a77970_routes,
 };
 
+static const struct rvin_info rcar_info_r8a77995 = {
+   .model = RCAR_GEN3,
+   .use_mc = true,
+   .max_width = 4096,
+   .max_height = 4096,
+   .routes = _rcar_info_r8a77995_routes,
+};
+
 static const struct of_device_id rvin_of_id_table[] = {
{
.compatible = "renesas,vin-r8a7778",
@@ -1153,6 +1165,10 @@ static const struct of_device_id rvin_of_id_table[] = {
.compatible = "renesas,vin-r8a77970",
.data = _info_r8a77970,
},
+   {
+   .compatible = "renesas,vin-r8a77995",
+   .data = _info_r8a77995,
+   },
{ /* Sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, rvin_of_id_table);
-- 
2.7.4

[PATCH v2 0/4] rcar-vin: Add support for digital input on Gen3

[Jacopo Mondi]

Hello,
   this series add support for 'digital' input to the Gen3 version of rcar-vin
driver.

'Digital' inputs (the terms comes from the existing Gen2 version of the driver)
describe parallel video input sources connected to a VIN instance. So far, the
Gen3-version of the driver (the media-controller compliant one) only supported
CSI-2 inputs.

This series extends the device tree parsing to accept a connection on port@0,
and parses the 'digital' subdevice reusing the Gen-2 functions.

Compared to v1, this series drops all changes to rcar-dma module, as Niklas
sent a proper fix for both crop and compose rectangle managment and field
signal toggling method. This series is thus based on the media master branch
with Niklas' series on top.

Compared to v1 I incorporated Niklas' suggestion to re-use Gen2 functions for
parsing the digital input device nodes, and register one notifier for each VIN
that has an active connection on port@0. The 'group' notifier then only collects
async subdevices for CSI-2 inputs.

A separate series for the VIN4 and HDMI input enabling on Draak board has been
sent to renesas-soc list.

The vin-tests repository patches to automate capture testing have been extended
to support D3 board and capture from HDMI output, and patches have been sent
to Niklas.

Tested capturing HDMI input images on D3 and for backward compatibility on
Salvator-X M3-W too (seems like I didn't break anything there).

Patches for testing on D3 are available at:
git://jmondi.org/linux d3/media-master/driver-v2
git://jmondi.org/linux d3/media-master/dts
git://jmondi.org/linux d3/media-master/test
git://jmondi.org/vin-tests d3

Patches to test on M3-W (based on latest renesas drivers, which includes an
older version of VIN series, but has CSI-2 driver) available at:
git://jmondi.org/linux d3/renesas-drivers/test

Thanks
j

Jacopo Mondi (4):
  media: rcar-vin: Parse digital input in mc-path
  media: rcar-vin: Handle mc in digital notifier ops
  media: rcar-vin: Handle digital subdev in link_notify
  media: rcar-vin: Add support for R-Car R8A77995 SoC

 drivers/media/platform/rcar-vin/rcar-core.c | 239 ++--
 drivers/media/platform/rcar-vin/rcar-vin.h  |  15 ++
 2 files changed, 202 insertions(+), 52 deletions(-)

--
2.7.4

[PATCH v2 2/4] media: rcar-vin: Handle mc in digital notifier ops

[Jacopo Mondi]

Handle media-controller in the digital notifier operations (bound,
unbind and complete) registered for VIN instances handling a digital
input subdevice.

Signed-off-by: Jacopo Mondi 
---
 drivers/media/platform/rcar-vin/rcar-core.c | 94 -
 1 file changed, 65 insertions(+), 29 deletions(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-core.c 
b/drivers/media/platform/rcar-vin/rcar-core.c
index 0ea21ab..1003c8c 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -379,7 +379,7 @@ static int rvin_find_pad(struct v4l2_subdev *sd, int 
direction)
  * Digital async notifier
  */
 
-/* The vin lock should be held when calling the subdevice attach and detach */
+/* The vin lock should be held when calling the subdevice attach. */
 static int rvin_digital_subdevice_attach(struct rvin_dev *vin,
 struct v4l2_subdev *subdev)
 {
@@ -388,15 +388,6 @@ static int rvin_digital_subdevice_attach(struct rvin_dev 
*vin,
};
int ret;
 
-   /* Find source and sink pad of remote subdevice */
-   ret = rvin_find_pad(subdev, MEDIA_PAD_FL_SOURCE);
-   if (ret < 0)
-   return ret;
-   vin->digital->source_pad = ret;
-
-   ret = rvin_find_pad(subdev, MEDIA_PAD_FL_SINK);
-   vin->digital->sink_pad = ret < 0 ? 0 : ret;
-
/* Find compatible subdevices mbus format */
vin->mbus_code = 0;
code.index = 0;
@@ -450,23 +441,14 @@ static int rvin_digital_subdevice_attach(struct rvin_dev 
*vin,
 
vin->vdev.ctrl_handler = >ctrl_handler;
 
-   vin->digital->subdev = subdev;
-
return 0;
 }
 
-static void rvin_digital_subdevice_detach(struct rvin_dev *vin)
-{
-   rvin_v4l2_unregister(vin);
-   v4l2_ctrl_handler_free(>ctrl_handler);
-
-   vin->vdev.ctrl_handler = NULL;
-   vin->digital->subdev = NULL;
-}
-
 static int rvin_digital_notify_complete(struct v4l2_async_notifier *notifier)
 {
struct rvin_dev *vin = notifier_to_vin(notifier);
+   struct media_entity *source;
+   struct media_entity *sink;
int ret;
 
ret = v4l2_device_register_subdev_nodes(>v4l2_dev);
@@ -475,7 +457,26 @@ static int rvin_digital_notify_complete(struct 
v4l2_async_notifier *notifier)
return ret;
}
 
-   return rvin_v4l2_register(vin);
+   if (!video_is_registered(>vdev)) {
+   ret = rvin_v4l2_register(vin);
+   if (ret < 0)
+   return ret;
+   }
+
+   if (!vin->info->use_mc)
+   return 0;
+
+   /* If we're running with media controller, link the subdevice. */
+   source = >digital->subdev->entity;
+   sink = >vdev.entity;
+
+   ret = media_create_pad_link(source, vin->digital->source_pad,
+   sink, vin->digital->sink_pad, 0);
+   if (ret)
+   vin_err(vin, "Error adding link from %s to %s\n",
+   source->name, sink->name);
+
+   return ret;
 }
 
 static void rvin_digital_notify_unbind(struct v4l2_async_notifier *notifier,
@@ -487,7 +488,14 @@ static void rvin_digital_notify_unbind(struct 
v4l2_async_notifier *notifier,
vin_dbg(vin, "unbind digital subdev %s\n", subdev->name);
 
mutex_lock(>lock);
-   rvin_digital_subdevice_detach(vin);
+
+   rvin_v4l2_unregister(vin);
+   vin->digital->subdev = NULL;
+   if (!vin->info->use_mc) {
+   v4l2_ctrl_handler_free(>ctrl_handler);
+   vin->vdev.ctrl_handler = NULL;
+   }
+
mutex_unlock(>lock);
 }
 
@@ -499,10 +507,29 @@ static int rvin_digital_notify_bound(struct 
v4l2_async_notifier *notifier,
int ret;
 
mutex_lock(>lock);
-   ret = rvin_digital_subdevice_attach(vin, subdev);
-   mutex_unlock(>lock);
-   if (ret)
+
+   /* Find source and sink pad of remote subdevice */
+   ret = rvin_find_pad(subdev, MEDIA_PAD_FL_SOURCE);
+   if (ret < 0) {
+   mutex_unlock(>lock);
return ret;
+   }
+   vin->digital->source_pad = ret;
+
+   ret = rvin_find_pad(subdev, MEDIA_PAD_FL_SINK);
+   vin->digital->sink_pad = ret < 0 ? 0 : ret;
+
+   vin->digital->subdev = subdev;
+
+   if (!vin->info->use_mc) {
+   ret = rvin_digital_subdevice_attach(vin, subdev);
+   if (ret) {
+   mutex_unlock(>lock);
+   return ret;
+   }
+   }
+
+   mutex_unlock(>lock);
 
v4l2_set_subdev_hostdata(subdev, vin);
 
@@ -555,9 +582,10 @@ static int rvin_digital_graph_init(struct rvin_dev *vin)
 {
int ret;
 
-   ret = v4l2_async_notifier_parse_fwnode_endpoints(
+   ret = v4l2_async_notifier_parse_fwnode_endpoints_by_port(
vin->dev, >notifier,
-   sizeof(struct rvin_graph_entity), rvin_digital_parse_v4l2);
+

[PATCH v2 1/4] media: rcar-vin: Parse digital input in mc-path

[Jacopo Mondi]

Add support for digital input subdevices to Gen-3 rcar-vin.
The Gen-3, media-controller compliant, version has so far only accepted
CSI-2 input subdevices. Remove assumptions on the supported bus_type and
accepted number of subdevices, and allow digital input connections on port@0.

Signed-off-by: Jacopo Mondi 
---
 drivers/media/platform/rcar-vin/rcar-core.c | 99 +++--
 drivers/media/platform/rcar-vin/rcar-vin.h  | 15 +
 2 files changed, 93 insertions(+), 21 deletions(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-core.c 
b/drivers/media/platform/rcar-vin/rcar-core.c
index d3072e1..0ea21ab 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -562,7 +562,7 @@ static int rvin_digital_graph_init(struct rvin_dev *vin)
return ret;
 
if (!vin->digital)
-   return -ENODEV;
+   return -ENOTCONN;
 
vin_dbg(vin, "Found digital subdevice %pOF\n",
to_of_node(vin->digital->asd.match.fwnode));
@@ -703,15 +703,13 @@ static int rvin_mc_parse_of_endpoint(struct device *dev,
 {
struct rvin_dev *vin = dev_get_drvdata(dev);
 
-   if (vep->base.port != 1 || vep->base.id >= RVIN_CSI_MAX)
+   if (vep->base.port != RVIN_PORT_CSI2 || vep->base.id >= RVIN_CSI_MAX)
return -EINVAL;
 
if (!of_device_is_available(to_of_node(asd->match.fwnode))) {
-
vin_dbg(vin, "OF device %pOF disabled, ignoring\n",
to_of_node(asd->match.fwnode));
return -ENOTCONN;
-
}
 
if (vin->group->csi[vep->base.id].fwnode) {
@@ -720,6 +718,8 @@ static int rvin_mc_parse_of_endpoint(struct device *dev,
return -ENOTCONN;
}
 
+   vin->mbus_cfg.type = V4L2_MBUS_CSI2;
+   vin->mbus_cfg.flags = 0;
vin->group->csi[vep->base.id].fwnode = asd->match.fwnode;
 
vin_dbg(vin, "Add group OF device %pOF to slot %u\n",
@@ -742,7 +742,14 @@ static int rvin_mc_parse_of_graph(struct rvin_dev *vin)
return -EINVAL;
}
 
-   /* If not all VIN's are registered don't register the notifier. */
+   /* Collect digital subdevices in this VIN device node. */
+   ret = rvin_digital_graph_init(vin);
+   if (ret < 0 && ret != -ENOTCONN) {
+   mutex_unlock(>group->lock);
+   return ret;
+   }
+
+   /* Only the last registered VIN instance collects CSI-2 subdevices. */
for (i = 0; i < RCAR_VIN_NUM; i++)
if (vin->group->vin[i])
count++;
@@ -752,22 +759,33 @@ static int rvin_mc_parse_of_graph(struct rvin_dev *vin)
return 0;
}
 
-   /*
-* Have all VIN's look for subdevices. Some subdevices will overlap
-* but the parser function can handle it, so each subdevice will
-* only be registered once with the notifier.
-*/
-
-   vin->group->notifier = >notifier;
-
+   vin->group->notifier = NULL;
for (i = 0; i < RCAR_VIN_NUM; i++) {
+   struct v4l2_async_notifier *notifier;
+
if (!vin->group->vin[i])
continue;
 
+   /* This VIN alread has digitial subdevices registered, skip. */
+   notifier = >group->vin[i]->notifier;
+   if (notifier->num_subdevs)
+   continue;
+
+   /* This VIN instance notifier will collect all CSI-2 subdevs. */
+   if (!vin->group->notifier) {
+   vin->group->v4l2_dev = >group->vin[i]->v4l2_dev;
+   vin->group->notifier = >group->vin[i]->notifier;
+   }
+
+   /*
+* Some CSI-2 subdevices will overlap but the parser function
+* can handle it, so each subdevice will only be registered
+* once with the group notifier.
+*/
ret = v4l2_async_notifier_parse_fwnode_endpoints_by_port(
vin->group->vin[i]->dev, vin->group->notifier,
-   sizeof(struct v4l2_async_subdev), 1,
-   rvin_mc_parse_of_endpoint);
+   sizeof(struct v4l2_async_subdev),
+   RVIN_PORT_CSI2, rvin_mc_parse_of_endpoint);
if (ret) {
mutex_unlock(>group->lock);
return ret;
@@ -776,25 +794,64 @@ static int rvin_mc_parse_of_graph(struct rvin_dev *vin)
 
mutex_unlock(>group->lock);
 
-   vin->group->notifier->ops = _group_notify_ops;
+   /*
+* Go and register all notifiers for digital subdevs, and
+* the group notifier for CSI-2 subdevs, if any.
+*/
+   for (i = 0; i < RCAR_VIN_NUM; i++) {
+   struct rvin_dev *ivin = vin->group->vin[i];
+   struct 

Re: [PATCHv13 12/28] v4l2-ctrls: add core request support

[Sakari Ailus]

On Thu, May 03, 2018 at 04:53:02PM +0200, Hans Verkuil wrote:
> @@ -1059,6 +1077,11 @@ int v4l2_ctrl_subscribe_event(struct v4l2_fh *fh,
>   */
>  __poll_t v4l2_ctrl_poll(struct file *file, struct poll_table_struct *wait);
>  
> +void v4l2_ctrl_request_setup(struct media_request *req,
> +  struct v4l2_ctrl_handler *hdl);
> +void v4l2_ctrl_request_complete(struct media_request *req,
> + struct v4l2_ctrl_handler *hdl);
> +

The two kAPI functions appear to be without documentation.

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

Re: [PATCH v2] media: helene: add I2C device probe function

[Abylay Ospan]

Hi Katsuhiro,

Thanks for patch.

What is the purpose to rework helene_set_params(_t|_s) ?
other part of this patch looks ok for me, but not tested due to lack
of spare time ;(

2018-05-16 4:37 GMT-04:00 Katsuhiro Suzuki :
> This patch adds I2C probe function to use dvb_module_probe()
> with this driver.
>
> Signed-off-by: Katsuhiro Suzuki 
>
> ---
>
> Changes since v1:
>   - Add documents for dvb_frontend member of helene_config
> ---
>  drivers/media/dvb-frontends/helene.c | 88 ++--
>  drivers/media/dvb-frontends/helene.h |  3 +
>  2 files changed, 87 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/dvb-frontends/helene.c 
> b/drivers/media/dvb-frontends/helene.c
> index a0d0b53c91d7..04033f0c278b 100644
> --- a/drivers/media/dvb-frontends/helene.c
> +++ b/drivers/media/dvb-frontends/helene.c
> @@ -666,7 +666,7 @@ static int helene_set_params_s(struct dvb_frontend *fe)
> return 0;
>  }
>
> -static int helene_set_params(struct dvb_frontend *fe)
> +static int helene_set_params_t(struct dvb_frontend *fe)
>  {
> u8 data[MAX_WRITE_REGSIZE];
> u32 frequency;
> @@ -835,6 +835,19 @@ static int helene_set_params(struct dvb_frontend *fe)
> return 0;
>  }
>
> +static int helene_set_params(struct dvb_frontend *fe)
> +{
> +   struct dtv_frontend_properties *p = >dtv_property_cache;
> +
> +   if (p->delivery_system == SYS_DVBT ||
> +   p->delivery_system == SYS_DVBT2 ||
> +   p->delivery_system == SYS_ISDBT ||
> +   p->delivery_system == SYS_DVBC_ANNEX_A)
> +   return helene_set_params_t(fe);
> +
> +   return helene_set_params_s(fe);
> +}
> +
>  static int helene_get_frequency(struct dvb_frontend *fe, u32 *frequency)
>  {
> struct helene_priv *priv = fe->tuner_priv;
> @@ -843,7 +856,7 @@ static int helene_get_frequency(struct dvb_frontend *fe, 
> u32 *frequency)
> return 0;
>  }
>
> -static const struct dvb_tuner_ops helene_tuner_ops = {
> +static const struct dvb_tuner_ops helene_tuner_ops_t = {
> .info = {
> .name = "Sony HELENE Ter tuner",
> .frequency_min = 100,
> @@ -853,7 +866,7 @@ static const struct dvb_tuner_ops helene_tuner_ops = {
> .init = helene_init,
> .release = helene_release,
> .sleep = helene_sleep,
> -   .set_params = helene_set_params,
> +   .set_params = helene_set_params_t,
> .get_frequency = helene_get_frequency,
>  };
>
> @@ -871,6 +884,20 @@ static const struct dvb_tuner_ops helene_tuner_ops_s = {
> .get_frequency = helene_get_frequency,
>  };
>
> +static const struct dvb_tuner_ops helene_tuner_ops = {
> +   .info = {
> +   .name = "Sony HELENE Sat/Ter tuner",
> +   .frequency_min = 50,
> +   .frequency_max = 12,
> +   .frequency_step = 1000,
> +   },
> +   .init = helene_init,
> +   .release = helene_release,
> +   .sleep = helene_sleep,
> +   .set_params = helene_set_params,
> +   .get_frequency = helene_get_frequency,
> +};
> +
>  /* power-on tuner
>   * call once after reset
>   */
> @@ -1032,7 +1059,7 @@ struct dvb_frontend *helene_attach(struct dvb_frontend 
> *fe,
> if (fe->ops.i2c_gate_ctrl)
> fe->ops.i2c_gate_ctrl(fe, 0);
>
> -   memcpy(>ops.tuner_ops, _tuner_ops,
> +   memcpy(>ops.tuner_ops, _tuner_ops_t,
> sizeof(struct dvb_tuner_ops));
> fe->tuner_priv = priv;
> dev_info(>i2c->dev,
> @@ -1042,6 +1069,59 @@ struct dvb_frontend *helene_attach(struct dvb_frontend 
> *fe,
>  }
>  EXPORT_SYMBOL(helene_attach);
>
> +static int helene_probe(struct i2c_client *client,
> +   const struct i2c_device_id *id)
> +{
> +   struct helene_config *config = client->dev.platform_data;
> +   struct dvb_frontend *fe = config->fe;
> +   struct device *dev = >dev;
> +   struct helene_priv *priv;
> +
> +   priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +   if (!priv)
> +   return -ENOMEM;
> +
> +   priv->i2c_address = client->addr;
> +   priv->i2c = client->adapter;
> +   priv->set_tuner_data = config->set_tuner_priv;
> +   priv->set_tuner = config->set_tuner_callback;
> +   priv->xtal = config->xtal;
> +
> +   if (fe->ops.i2c_gate_ctrl)
> +   fe->ops.i2c_gate_ctrl(fe, 1);
> +
> +   if (helene_x_pon(priv) != 0)
> +   return -EINVAL;
> +
> +   if (fe->ops.i2c_gate_ctrl)
> +   fe->ops.i2c_gate_ctrl(fe, 0);
> +
> +   memcpy(>ops.tuner_ops, _tuner_ops,
> +  sizeof(struct dvb_tuner_ops));
> +   fe->tuner_priv = priv;
> +   i2c_set_clientdata(client, priv);
> +
> +   dev_info(dev, "Sony HELENE attached on addr=%x at I2C adapter %p\n",
> +priv->i2c_address, priv->i2c);
> +
> +   return 0;
> +}
> 

Re: [PATCH] media: helene: fix tuning frequency of satellite

[Abylay Ospan]

True.
I'm curious but how did it worked before ...
Which hardware (dvb adapter) are you using ?

2018-05-16 4:41 GMT-04:00 Katsuhiro Suzuki :
> This patch fixes tuning frequency of satellite to kHz. That as same
> as terrestrial one.
>
> Signed-off-by: Katsuhiro Suzuki 
> ---
>  drivers/media/dvb-frontends/helene.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/dvb-frontends/helene.c 
> b/drivers/media/dvb-frontends/helene.c
> index 04033f0c278b..0a4f312c4368 100644
> --- a/drivers/media/dvb-frontends/helene.c
> +++ b/drivers/media/dvb-frontends/helene.c
> @@ -523,7 +523,7 @@ static int helene_set_params_s(struct dvb_frontend *fe)
> enum helene_tv_system_t tv_system;
> struct dtv_frontend_properties *p = >dtv_property_cache;
> struct helene_priv *priv = fe->tuner_priv;
> -   int frequencykHz = p->frequency;
> +   int frequencykHz = p->frequency / 1000;
> uint32_t frequency4kHz = 0;
> u32 symbol_rate = p->symbol_rate/1000;
>
> --
> 2.17.0
>



-- 
Abylay Ospan,
NetUP Inc.
http://www.netup.tv

Re: [PATCH] media: helene: fix xtal frequency setting at power on

[Abylay Ospan]

Acked-by: Abylay Ospan 

2018-05-16 4:41 GMT-04:00 Katsuhiro Suzuki :
> This patch fixes crystal frequency setting when power on this device.
>
> Signed-off-by: Katsuhiro Suzuki 
> ---
>  drivers/media/dvb-frontends/helene.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/dvb-frontends/helene.c 
> b/drivers/media/dvb-frontends/helene.c
> index 0a4f312c4368..8fcf7a00782a 100644
> --- a/drivers/media/dvb-frontends/helene.c
> +++ b/drivers/media/dvb-frontends/helene.c
> @@ -924,7 +924,10 @@ static int helene_x_pon(struct helene_priv *priv)
> helene_write_regs(priv, 0x99, cdata, sizeof(cdata));
>
> /* 0x81 - 0x94 */
> -   data[0] = 0x18; /* xtal 24 MHz */
> +   if (priv->xtal == SONY_HELENE_XTAL_16000)
> +   data[0] = 0x10; /* xtal 16 MHz */
> +   else
> +   data[0] = 0x18; /* xtal 24 MHz */
> data[1] = (uint8_t)(0x80 | (0x04 & 0x1F)); /* 4 x 25 = 100uA */
> data[2] = (uint8_t)(0x80 | (0x26 & 0x7F)); /* 38 x 0.25 = 9.5pF */
> data[3] = 0x80; /* REFOUT signal output 500mVpp */
> --
> 2.17.0
>



-- 
Abylay Ospan,
NetUP Inc.
http://www.netup.tv

Re: [PATCHv13 12/28] v4l2-ctrls: add core request support

[Hans Verkuil]

On 05/16/18 12:19, Sakari Ailus wrote:
> Hi Hans,
> 
> On Thu, May 03, 2018 at 04:53:02PM +0200, Hans Verkuil wrote:
>> From: Hans Verkuil 
>>
>> Integrate the request support. This adds the v4l2_ctrl_request_complete
>> and v4l2_ctrl_request_setup functions to complete a request and (as a
>> helper function) to apply a request to the hardware.
>>
>> It takes care of queuing requests and correctly chaining control values
>> in the request queue.
>>
>> Note that when a request is marked completed it will copy control values
>> to the internal request state. This can be optimized in the future since
>> this is sub-optimal when dealing with large compound and/or array controls.
>>
>> For the initial 'stateless codec' use-case the current implementation is
>> sufficient.
>>
>> Signed-off-by: Hans Verkuil 
>> ---
>>  drivers/media/v4l2-core/v4l2-ctrls.c | 331 ++-
>>  include/media/v4l2-ctrls.h   |  23 ++
>>  2 files changed, 348 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
>> b/drivers/media/v4l2-core/v4l2-ctrls.c
>> index da4cc1485dc4..56b986185463 100644
>> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
>> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
>> @@ -3429,6 +3602,152 @@ int __v4l2_ctrl_s_ctrl_string(struct v4l2_ctrl 
>> *ctrl, const char *s)
>>  }
>>  EXPORT_SYMBOL(__v4l2_ctrl_s_ctrl_string);
>>  
>> +void v4l2_ctrl_request_complete(struct media_request *req,
>> +struct v4l2_ctrl_handler *main_hdl)
>> +{
>> +struct media_request_object *obj;
>> +struct v4l2_ctrl_handler *hdl;
>> +struct v4l2_ctrl_ref *ref;
>> +
>> +if (!req || !main_hdl)
>> +return;
>> +
>> +obj = media_request_object_find(req, _ops, main_hdl);
>> +if (!obj)
>> +return;
>> +hdl = container_of(obj, struct v4l2_ctrl_handler, req_obj);
>> +
>> +list_for_each_entry(ref, >ctrl_refs, node) {
>> +struct v4l2_ctrl *ctrl = ref->ctrl;
>> +struct v4l2_ctrl *master = ctrl->cluster[0];
>> +unsigned int i;
>> +
>> +if (ctrl->flags & V4L2_CTRL_FLAG_VOLATILE) {
>> +ref->req = ref;
>> +
>> +v4l2_ctrl_lock(master);
>> +/* g_volatile_ctrl will update the current control 
>> values */
>> +for (i = 0; i < master->ncontrols; i++)
>> +cur_to_new(master->cluster[i]);
>> +call_op(master, g_volatile_ctrl);
>> +new_to_req(ref);
>> +v4l2_ctrl_unlock(master);
>> +continue;
>> +}
>> +if (ref->req == ref)
>> +continue;
>> +
>> +v4l2_ctrl_lock(ctrl);
>> +if (ref->req)
>> +ptr_to_ptr(ctrl, ref->req->p_req, ref->p_req);
>> +else
>> +ptr_to_ptr(ctrl, ctrl->p_cur, ref->p_req);
>> +v4l2_ctrl_unlock(ctrl);
>> +}
>> +
>> +WARN_ON(!hdl->request_is_queued);
>> +list_del_init(>requests_queued);
>> +hdl->request_is_queued = false;
>> +media_request_object_complete(obj);
>> +media_request_object_put(obj);
>> +}
>> +EXPORT_SYMBOL(v4l2_ctrl_request_complete);
>> +
>> +void v4l2_ctrl_request_setup(struct media_request *req,
>> + struct v4l2_ctrl_handler *main_hdl)
> 
> Drivers are expected to use this function internally to make use of the
> control values in the request. Is that your thinking as well?

Not quite. For simple drivers like stateless codecs this driver just applies
all the controls in the request to the hardware. However, this only works for
cases where you can do this just before you process the buffer.

More complex drivers are unlikely to be able to use this function. Instead
they will have to know about HW requirements like how long it will take for
a sensor to actually use a new value.

> The problem with this implementation is that once a driver eventually gets
> a callback (s_ctrl), the callback doesn't have the information on the
> request. That means the driver has no means to associate the control value
> to the request anymore --- and that is against the very purpose of the
> function.

s_ctrl just applies the value to the hardware, it shouldn't have to care
about the request. Scheduling when a control value in a request will have
to be applied to the hardware is something that the driver will have to
coordinate.

In any case, this v4l2_ctrl_request_setup() function is unsuitable for
such complex devices. We will need something smarter for that.

I have not looked at that since it is out of scope of stateless codecs.
The core problem here is how to schedule when you apply controls in a
request given the pending buffer queue.

Regards,

Hans

> Instead, I'd add a new argument to the callback function --- the request
> --- or add another callback 

Re: [PATCHv13 12/28] v4l2-ctrls: add core request support

[Sakari Ailus]

On Wed, May 16, 2018 at 01:19:34PM +0300, Sakari Ailus wrote:
> Hi Hans,
> 
> On Thu, May 03, 2018 at 04:53:02PM +0200, Hans Verkuil wrote:
> > From: Hans Verkuil 
> > 
> > Integrate the request support. This adds the v4l2_ctrl_request_complete
> > and v4l2_ctrl_request_setup functions to complete a request and (as a
> > helper function) to apply a request to the hardware.
> > 
> > It takes care of queuing requests and correctly chaining control values
> > in the request queue.
> > 
> > Note that when a request is marked completed it will copy control values
> > to the internal request state. This can be optimized in the future since
> > this is sub-optimal when dealing with large compound and/or array controls.
> > 
> > For the initial 'stateless codec' use-case the current implementation is
> > sufficient.
> > 
> > Signed-off-by: Hans Verkuil 
> > ---
> >  drivers/media/v4l2-core/v4l2-ctrls.c | 331 ++-
> >  include/media/v4l2-ctrls.h   |  23 ++
> >  2 files changed, 348 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
> > b/drivers/media/v4l2-core/v4l2-ctrls.c
> > index da4cc1485dc4..56b986185463 100644
> > --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> > +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> > @@ -3429,6 +3602,152 @@ int __v4l2_ctrl_s_ctrl_string(struct v4l2_ctrl 
> > *ctrl, const char *s)
> >  }
> >  EXPORT_SYMBOL(__v4l2_ctrl_s_ctrl_string);
> >  
> > +void v4l2_ctrl_request_complete(struct media_request *req,
> > +   struct v4l2_ctrl_handler *main_hdl)
> > +{
> > +   struct media_request_object *obj;
> > +   struct v4l2_ctrl_handler *hdl;
> > +   struct v4l2_ctrl_ref *ref;
> > +
> > +   if (!req || !main_hdl)
> > +   return;
> > +
> > +   obj = media_request_object_find(req, _ops, main_hdl);
> > +   if (!obj)
> > +   return;
> > +   hdl = container_of(obj, struct v4l2_ctrl_handler, req_obj);
> > +
> > +   list_for_each_entry(ref, >ctrl_refs, node) {
> > +   struct v4l2_ctrl *ctrl = ref->ctrl;
> > +   struct v4l2_ctrl *master = ctrl->cluster[0];
> > +   unsigned int i;
> > +
> > +   if (ctrl->flags & V4L2_CTRL_FLAG_VOLATILE) {
> > +   ref->req = ref;
> > +
> > +   v4l2_ctrl_lock(master);
> > +   /* g_volatile_ctrl will update the current control 
> > values */
> > +   for (i = 0; i < master->ncontrols; i++)
> > +   cur_to_new(master->cluster[i]);
> > +   call_op(master, g_volatile_ctrl);
> > +   new_to_req(ref);
> > +   v4l2_ctrl_unlock(master);
> > +   continue;
> > +   }
> > +   if (ref->req == ref)
> > +   continue;
> > +
> > +   v4l2_ctrl_lock(ctrl);
> > +   if (ref->req)
> > +   ptr_to_ptr(ctrl, ref->req->p_req, ref->p_req);
> > +   else
> > +   ptr_to_ptr(ctrl, ctrl->p_cur, ref->p_req);
> > +   v4l2_ctrl_unlock(ctrl);
> > +   }
> > +
> > +   WARN_ON(!hdl->request_is_queued);
> > +   list_del_init(>requests_queued);
> > +   hdl->request_is_queued = false;
> > +   media_request_object_complete(obj);
> > +   media_request_object_put(obj);
> > +}
> > +EXPORT_SYMBOL(v4l2_ctrl_request_complete);
> > +
> > +void v4l2_ctrl_request_setup(struct media_request *req,
> > +struct v4l2_ctrl_handler *main_hdl)
> 
> Drivers are expected to use this function internally to make use of the
> control values in the request. Is that your thinking as well?
> 
> The problem with this implementation is that once a driver eventually gets
> a callback (s_ctrl), the callback doesn't have the information on the
> request. That means the driver has no means to associate the control value
> to the request anymore --- and that is against the very purpose of the
> function.
> 
> Instead, I'd add a new argument to the callback function --- the request
> --- or add another callback function to be used for applying control values
> for requests. Or alternatively, provide an easy way to enumerate the
> controls and their values in a control handler. For the driver must store

To address this fully --- using S_EXT_CTRLS on uAPI to the control handler
should likely be prevented as long as there are request objects related to
that handler. Or at least request objects that are not completed.

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

Re: [PATCH] [Patch v2] usbtv: Fix refcounting mixup

[Hans Verkuil]

On 05/16/18 11:23, Oliver Neukum wrote:
> Am Dienstag, den 15.05.2018, 18:01 +0200 schrieb Hans Verkuil:
>> On 05/15/2018 05:46 PM, Oliver Neukum wrote:
>>> Am Dienstag, den 15.05.2018, 16:28 +0200 schrieb Hans Verkuil:
 On 05/15/18 15:07, Oliver Neukum wrote:
> 
>  usbtv_audio_fail:
>   /* we must not free at this point */
> - usb_get_dev(usbtv->udev);
> + v4l2_device_get(>v4l2_dev);

 This is very confusing. I think it is much better to move the
>>>
>>> Yes. It confused me.
>>>
 v4l2_device_register() call from usbtv_video_init to this probe function.
>>>
>>> Yes, but it is called here. So you want to do it after registering the
>>> audio?
>>
>> No, before. It's a global data structure, so this can be done before the
>> call to usbtv_video_init() as part of the top-level initialization of the
>> driver.
> 
> Eh, but we cannot create a V4L device before the first device
> is connected and we must certainly create multiple V4L devices if
> multiple physical devices are connected.

v4l2_device_register is a terrible name. It does not create devices
or register with anything, it just initializes a root data structure. I have
proposed renaming this to v4l2_root_init() in the past, but people didn't
want a big rename action.

BTW, with 'global data structure' I meant a data structure in struct usbtv.
All I meant to say is that v4l2_device_register should be called in probe(),
not in usbtv_video_init().

Regards,

Hans

> 
> Maybe I am dense. Please elaborate.
> It seem to me that the driver is confusing because it uses
> multiple refcounts.
> 
>   Regards
>   Oliver
> 

Re: [PATCH] dma-fence: Make dma_fence_add_callback() fail if signaled with error

[Lucas Stach]

Am Mittwoch, den 16.05.2018, 11:42 +0200 schrieb Daniel Vetter:
> On Tue, May 15, 2018 at 01:16:30PM +0100, Chris Wilson wrote:
> > Quoting Ezequiel Garcia (2018-05-14 22:28:31)
> > > On Mon, 2018-05-14 at 18:48 +0200, Daniel Vetter wrote:
> > > > On Fri, May 11, 2018 at 08:27:41AM +0100, Chris Wilson wrote:
> > > > > Quoting Ezequiel Garcia (2018-05-09 21:14:49)
> > > > > > Change how dma_fence_add_callback() behaves, when the fence
> > > > > > has error-signaled by the time it is being add. After this commit,
> > > > > > dma_fence_add_callback() returns the fence error, if it
> > > > > > has error-signaled before dma_fence_add_callback() is called.
> > > > > 
> > > > > Why? What problem are you trying to solve? fence->error does not imply
> > > > > that the fence has yet been signaled, and the caller wants a callback
> > > > > when it is signaled.
> > > > 
> > > > On top this is incosistent, e.g. we don't do the same for any of the 
> > > > other
> > > > dma_fence interfaces. Plus there's the issue that you might alias errno
> > > > values with fence errno values.
> > > > 
> > > 
> > > Right.
> > > 
> > > > I think keeping the error codes from the functions you're calling 
> > > > distinct
> > > > from the error code of the fence itself makes a lot of sense. The first
> > > > tells you whether your request worked out (or why not), the second tells
> > > > you whether the asynchronous dma operation (gpu rendering, page flip,
> > > > whatever) that the dma_fence represents worked out (or why not). That's 
> > > > 2
> > > > distinct things imo.
> > > > 
> > > > Might be good to show us the driver code that needs this behaviour so we
> > > > can discuss how to best handle your use-case.
> > > > 
> > > 
> > > This change arose while discussing the in-fences support for video4linux.
> > > Here's the patch that calls dma_fence_add_callback 
> > > https://lkml.org/lkml/2018/5/4/766.
> > > 
> > > The code snippet currently looks something like this:
> > > 
> > > if (vb->in_fence) {
> > > ret = dma_fence_add_callback(vb->in_fence, >fence_cb,
> > > 
> > >  vb2_qbuf_fence_cb);
> > > /* is the fence signaled? */
> > > if (ret == -ENOENT) {
> > > 
> > > dma_fence_put(vb->in_fence);
> > > vb->in_fence = NULL;
> > > } else if (ret)
> > > {
> > > goto unlock;
> > > }
> > > }
> > > 
> > > In this use case, if the callback is added successfully,
> > > the video4linux core defers the activation of the buffer
> > > until the fence signals.
> > > 
> > > If the fence is signaled (currently disregarding of errors)
> > > then the buffer is assumed to be ready to be activated,
> > > and so it gets queued for hardware usage.
> > > 
> > > Giving some more thought to this, I'm not so sure what is
> > > the right action if a fence signaled with error. In this case,
> > > it appears to me that we shouldn't be using this buffer
> > > if its in-fence is in error, but perhaps I'm missing
> > > something.
> > 
> > What I have in mind for async errors is to skip the operation and
> > propagate the error onto the next fence. Mostly because those async
> > errors may include fatal errors such as unable to pin the backing
> > storage for the operation, but even "trivial" errors such as an early
> > operation failing means that this request is then subject to garbage-in,
> > garbage-out. However, for trivial errors I would just propagate the
> > error status (so the caller knows something went wrong if they care, but
> > in all likelihood no one will notice) and continue on with the glitchy
> > operation.
> 
> In general, there's not really any hard rule about propagating fence
> errors across devices. It's mostly just used by drivers internally to keep
> track of failed stuff (gpu hangs or anything else async like Chris
> describes here).
> 
> For v4l I'm not sure you want to care much about this, since right now the
> main use of fence errors is gpu hang recovery (whether it's the driver or
> hw that's hung doesn't matter here).

Yes, my understanding is that fence signal and errors are two distinct
things that should not be conflated like it is done in this patch.

In my understanding signaling a fence means the HW or SW component
which added the fence is done with the buffer and will not touch it
anymore. In case of an unrecoverable error the fence will be signaled
with error status set, so we correctly reflect the buffer status as
being free to be used by whoever is waiting for it, but may contain
garbage.

If a fence waiter cares about the buffer content and may wish to skip
its operation if the fence is signaled with an error it should do it by
explicitly checking the fence error status, instead of making this
implicit behavior.

Regards,
Lucas

Re: [PATCHv13 12/28] v4l2-ctrls: add core request support

[Sakari Ailus]

Hi Hans,

On Thu, May 03, 2018 at 04:53:02PM +0200, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> Integrate the request support. This adds the v4l2_ctrl_request_complete
> and v4l2_ctrl_request_setup functions to complete a request and (as a
> helper function) to apply a request to the hardware.
> 
> It takes care of queuing requests and correctly chaining control values
> in the request queue.
> 
> Note that when a request is marked completed it will copy control values
> to the internal request state. This can be optimized in the future since
> this is sub-optimal when dealing with large compound and/or array controls.
> 
> For the initial 'stateless codec' use-case the current implementation is
> sufficient.
> 
> Signed-off-by: Hans Verkuil 
> ---
>  drivers/media/v4l2-core/v4l2-ctrls.c | 331 ++-
>  include/media/v4l2-ctrls.h   |  23 ++
>  2 files changed, 348 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
> b/drivers/media/v4l2-core/v4l2-ctrls.c
> index da4cc1485dc4..56b986185463 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -3429,6 +3602,152 @@ int __v4l2_ctrl_s_ctrl_string(struct v4l2_ctrl *ctrl, 
> const char *s)
>  }
>  EXPORT_SYMBOL(__v4l2_ctrl_s_ctrl_string);
>  
> +void v4l2_ctrl_request_complete(struct media_request *req,
> + struct v4l2_ctrl_handler *main_hdl)
> +{
> + struct media_request_object *obj;
> + struct v4l2_ctrl_handler *hdl;
> + struct v4l2_ctrl_ref *ref;
> +
> + if (!req || !main_hdl)
> + return;
> +
> + obj = media_request_object_find(req, _ops, main_hdl);
> + if (!obj)
> + return;
> + hdl = container_of(obj, struct v4l2_ctrl_handler, req_obj);
> +
> + list_for_each_entry(ref, >ctrl_refs, node) {
> + struct v4l2_ctrl *ctrl = ref->ctrl;
> + struct v4l2_ctrl *master = ctrl->cluster[0];
> + unsigned int i;
> +
> + if (ctrl->flags & V4L2_CTRL_FLAG_VOLATILE) {
> + ref->req = ref;
> +
> + v4l2_ctrl_lock(master);
> + /* g_volatile_ctrl will update the current control 
> values */
> + for (i = 0; i < master->ncontrols; i++)
> + cur_to_new(master->cluster[i]);
> + call_op(master, g_volatile_ctrl);
> + new_to_req(ref);
> + v4l2_ctrl_unlock(master);
> + continue;
> + }
> + if (ref->req == ref)
> + continue;
> +
> + v4l2_ctrl_lock(ctrl);
> + if (ref->req)
> + ptr_to_ptr(ctrl, ref->req->p_req, ref->p_req);
> + else
> + ptr_to_ptr(ctrl, ctrl->p_cur, ref->p_req);
> + v4l2_ctrl_unlock(ctrl);
> + }
> +
> + WARN_ON(!hdl->request_is_queued);
> + list_del_init(>requests_queued);
> + hdl->request_is_queued = false;
> + media_request_object_complete(obj);
> + media_request_object_put(obj);
> +}
> +EXPORT_SYMBOL(v4l2_ctrl_request_complete);
> +
> +void v4l2_ctrl_request_setup(struct media_request *req,
> +  struct v4l2_ctrl_handler *main_hdl)

Drivers are expected to use this function internally to make use of the
control values in the request. Is that your thinking as well?

The problem with this implementation is that once a driver eventually gets
a callback (s_ctrl), the callback doesn't have the information on the
request. That means the driver has no means to associate the control value
to the request anymore --- and that is against the very purpose of the
function.

Instead, I'd add a new argument to the callback function --- the request
--- or add another callback function to be used for applying control values
for requests. Or alternatively, provide an easy way to enumerate the
controls and their values in a control handler. For the driver must store
that value in the request itself to be able to use it: the current
implementation in vim2m is such that controls are simply set to the driver
as they'd arrive from the uAPI directly. This way also anyone having access
to the video device could set whatever control values that would end up
being used in processing the request.

Feel free to point out if I'm mistaken in my analysis.

> +{
> + struct media_request_object *obj;
> + struct v4l2_ctrl_handler *hdl;
> + struct v4l2_ctrl_ref *ref;
> +
> + if (!req || !main_hdl)
> + return;
> +
> + obj = media_request_object_find(req, _ops, main_hdl);
> + if (!obj)
> + return;
> + if (obj->completed) {
> + media_request_object_put(obj);
> + return;
> + }
> + hdl = container_of(obj, struct v4l2_ctrl_handler, req_obj);
> +
> + mutex_lock(hdl->lock);
> +
> +

Re: [PATCH] dma-fence: Make dma_fence_add_callback() fail if signaled with error

[Daniel Vetter]

On Tue, May 15, 2018 at 01:16:30PM +0100, Chris Wilson wrote:
> Quoting Ezequiel Garcia (2018-05-14 22:28:31)
> > On Mon, 2018-05-14 at 18:48 +0200, Daniel Vetter wrote:
> > > On Fri, May 11, 2018 at 08:27:41AM +0100, Chris Wilson wrote:
> > > > Quoting Ezequiel Garcia (2018-05-09 21:14:49)
> > > > > Change how dma_fence_add_callback() behaves, when the fence
> > > > > has error-signaled by the time it is being add. After this commit,
> > > > > dma_fence_add_callback() returns the fence error, if it
> > > > > has error-signaled before dma_fence_add_callback() is called.
> > > > 
> > > > Why? What problem are you trying to solve? fence->error does not imply
> > > > that the fence has yet been signaled, and the caller wants a callback
> > > > when it is signaled.
> > > 
> > > On top this is incosistent, e.g. we don't do the same for any of the other
> > > dma_fence interfaces. Plus there's the issue that you might alias errno
> > > values with fence errno values.
> > > 
> > 
> > Right.
> > 
> > > I think keeping the error codes from the functions you're calling distinct
> > > from the error code of the fence itself makes a lot of sense. The first
> > > tells you whether your request worked out (or why not), the second tells
> > > you whether the asynchronous dma operation (gpu rendering, page flip,
> > > whatever) that the dma_fence represents worked out (or why not). That's 2
> > > distinct things imo.
> > > 
> > > Might be good to show us the driver code that needs this behaviour so we
> > > can discuss how to best handle your use-case.
> > > 
> > 
> > This change arose while discussing the in-fences support for video4linux.
> > Here's the patch that calls dma_fence_add_callback 
> > https://lkml.org/lkml/2018/5/4/766.
> > 
> > The code snippet currently looks something like this:
> > 
> > if (vb->in_fence) {
> > ret = dma_fence_add_callback(vb->in_fence, >fence_cb,
> > 
> >  vb2_qbuf_fence_cb);
> > /* is the fence signaled? */
> > if (ret == -ENOENT) {
> > 
> > dma_fence_put(vb->in_fence);
> > vb->in_fence = NULL;
> > } else if (ret)
> > {
> > goto unlock;
> > }
> > }
> > 
> > In this use case, if the callback is added successfully,
> > the video4linux core defers the activation of the buffer
> > until the fence signals.
> > 
> > If the fence is signaled (currently disregarding of errors)
> > then the buffer is assumed to be ready to be activated,
> > and so it gets queued for hardware usage.
> > 
> > Giving some more thought to this, I'm not so sure what is
> > the right action if a fence signaled with error. In this case,
> > it appears to me that we shouldn't be using this buffer
> > if its in-fence is in error, but perhaps I'm missing
> > something.
> 
> What I have in mind for async errors is to skip the operation and
> propagate the error onto the next fence. Mostly because those async
> errors may include fatal errors such as unable to pin the backing
> storage for the operation, but even "trivial" errors such as an early
> operation failing means that this request is then subject to garbage-in,
> garbage-out. However, for trivial errors I would just propagate the
> error status (so the caller knows something went wrong if they care, but
> in all likelihood no one will notice) and continue on with the glitchy
> operation.

In general, there's not really any hard rule about propagating fence
errors across devices. It's mostly just used by drivers internally to keep
track of failed stuff (gpu hangs or anything else async like Chris
describes here).

For v4l I'm not sure you want to care much about this, since right now the
main use of fence errors is gpu hang recovery (whether it's the driver or
hw that's hung doesn't matter here).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH] [Patch v2] usbtv: Fix refcounting mixup

[Oliver Neukum]

Am Dienstag, den 15.05.2018, 18:01 +0200 schrieb Hans Verkuil:
> On 05/15/2018 05:46 PM, Oliver Neukum wrote:
> > Am Dienstag, den 15.05.2018, 16:28 +0200 schrieb Hans Verkuil:
> > > On 05/15/18 15:07, Oliver Neukum wrote:

> > > >  usbtv_audio_fail:
> > > > /* we must not free at this point */
> > > > -   usb_get_dev(usbtv->udev);
> > > > +   v4l2_device_get(>v4l2_dev);
> > > 
> > > This is very confusing. I think it is much better to move the
> > 
> > Yes. It confused me.
> > 
> > > v4l2_device_register() call from usbtv_video_init to this probe function.
> > 
> > Yes, but it is called here. So you want to do it after registering the
> > audio?
> 
> No, before. It's a global data structure, so this can be done before the
> call to usbtv_video_init() as part of the top-level initialization of the
> driver.

Eh, but we cannot create a V4L device before the first device
is connected and we must certainly create multiple V4L devices if
multiple physical devices are connected.

Maybe I am dense. Please elaborate.
It seem to me that the driver is confusing because it uses
multiple refcounts.

Regards
Oliver

Re: [PATCH v2 2/2] ARM: dts: r8a7740: Add CEU0

[Simon Horman]

On Wed, May 16, 2018 at 09:40:09AM +0200, Geert Uytterhoeven wrote:
> Hi Jacopo,
> 
> On Thu, Apr 26, 2018 at 8:24 PM, Jacopo Mondi  
> wrote:
> > Describe CEU0 peripheral for Renesas R-Mobile A1 R8A7740 Soc.
> >
> > Reported-by: Geert Uytterhoeven 
> > Signed-off-by: Jacopo Mondi 
> 
> Thanks for your patch!
> 
> Reviewed-by: Geert Uytterhoeven 
> 
> Minor question below.
> 
> > --- a/arch/arm/boot/dts/r8a7740.dtsi
> > +++ b/arch/arm/boot/dts/r8a7740.dtsi
> > @@ -67,6 +67,16 @@
> > power-domains = <_d4>;
> > };
> >
> > +   ceu0: ceu@fe91 {
> > +   reg = <0xfe91 0x3000>;
> > +   compatible = "renesas,r8a7740-ceu";
> > +   interrupts = ;
> > +   clocks = <_clks R8A7740_CLK_CEU20>;
> > +   clock-names = "ceu20";
> 
> Why the "clock-names" property? It's not mentioned in the DT bindings, and
> may cause issues if the bindings are ever amended.

I have dropped that property for now.

> 
> > +   power-domains = <_a4r>;
> > +   status = "disabled";
> > +   };
> > +
> 
> Gr{oetje,eeting}s,
> 
> Geert
> 
> -- 
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- 
> ge...@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like 
> that.
> -- Linus Torvalds
> 

[PATCH] media: helene: fix tuning frequency of satellite

[Katsuhiro Suzuki]

This patch fixes tuning frequency of satellite to kHz. That as same
as terrestrial one.

Signed-off-by: Katsuhiro Suzuki 
---
 drivers/media/dvb-frontends/helene.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/dvb-frontends/helene.c 
b/drivers/media/dvb-frontends/helene.c
index 04033f0c278b..0a4f312c4368 100644
--- a/drivers/media/dvb-frontends/helene.c
+++ b/drivers/media/dvb-frontends/helene.c
@@ -523,7 +523,7 @@ static int helene_set_params_s(struct dvb_frontend *fe)
enum helene_tv_system_t tv_system;
struct dtv_frontend_properties *p = >dtv_property_cache;
struct helene_priv *priv = fe->tuner_priv;
-   int frequencykHz = p->frequency;
+   int frequencykHz = p->frequency / 1000;
uint32_t frequency4kHz = 0;
u32 symbol_rate = p->symbol_rate/1000;
 
-- 
2.17.0

[PATCH] media: helene: fix xtal frequency setting at power on

[Katsuhiro Suzuki]

This patch fixes crystal frequency setting when power on this device.

Signed-off-by: Katsuhiro Suzuki 
---
 drivers/media/dvb-frontends/helene.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/media/dvb-frontends/helene.c 
b/drivers/media/dvb-frontends/helene.c
index 0a4f312c4368..8fcf7a00782a 100644
--- a/drivers/media/dvb-frontends/helene.c
+++ b/drivers/media/dvb-frontends/helene.c
@@ -924,7 +924,10 @@ static int helene_x_pon(struct helene_priv *priv)
helene_write_regs(priv, 0x99, cdata, sizeof(cdata));
 
/* 0x81 - 0x94 */
-   data[0] = 0x18; /* xtal 24 MHz */
+   if (priv->xtal == SONY_HELENE_XTAL_16000)
+   data[0] = 0x10; /* xtal 16 MHz */
+   else
+   data[0] = 0x18; /* xtal 24 MHz */
data[1] = (uint8_t)(0x80 | (0x04 & 0x1F)); /* 4 x 25 = 100uA */
data[2] = (uint8_t)(0x80 | (0x26 & 0x7F)); /* 38 x 0.25 = 9.5pF */
data[3] = 0x80; /* REFOUT signal output 500mVpp */
-- 
2.17.0

[PATCH v2] media: helene: add I2C device probe function

[Katsuhiro Suzuki]

This patch adds I2C probe function to use dvb_module_probe()
with this driver.

Signed-off-by: Katsuhiro Suzuki 

---

Changes since v1:
  - Add documents for dvb_frontend member of helene_config
---
 drivers/media/dvb-frontends/helene.c | 88 ++--
 drivers/media/dvb-frontends/helene.h |  3 +
 2 files changed, 87 insertions(+), 4 deletions(-)

diff --git a/drivers/media/dvb-frontends/helene.c 
b/drivers/media/dvb-frontends/helene.c
index a0d0b53c91d7..04033f0c278b 100644
--- a/drivers/media/dvb-frontends/helene.c
+++ b/drivers/media/dvb-frontends/helene.c
@@ -666,7 +666,7 @@ static int helene_set_params_s(struct dvb_frontend *fe)
return 0;
 }
 
-static int helene_set_params(struct dvb_frontend *fe)
+static int helene_set_params_t(struct dvb_frontend *fe)
 {
u8 data[MAX_WRITE_REGSIZE];
u32 frequency;
@@ -835,6 +835,19 @@ static int helene_set_params(struct dvb_frontend *fe)
return 0;
 }
 
+static int helene_set_params(struct dvb_frontend *fe)
+{
+   struct dtv_frontend_properties *p = >dtv_property_cache;
+
+   if (p->delivery_system == SYS_DVBT ||
+   p->delivery_system == SYS_DVBT2 ||
+   p->delivery_system == SYS_ISDBT ||
+   p->delivery_system == SYS_DVBC_ANNEX_A)
+   return helene_set_params_t(fe);
+
+   return helene_set_params_s(fe);
+}
+
 static int helene_get_frequency(struct dvb_frontend *fe, u32 *frequency)
 {
struct helene_priv *priv = fe->tuner_priv;
@@ -843,7 +856,7 @@ static int helene_get_frequency(struct dvb_frontend *fe, 
u32 *frequency)
return 0;
 }
 
-static const struct dvb_tuner_ops helene_tuner_ops = {
+static const struct dvb_tuner_ops helene_tuner_ops_t = {
.info = {
.name = "Sony HELENE Ter tuner",
.frequency_min = 100,
@@ -853,7 +866,7 @@ static const struct dvb_tuner_ops helene_tuner_ops = {
.init = helene_init,
.release = helene_release,
.sleep = helene_sleep,
-   .set_params = helene_set_params,
+   .set_params = helene_set_params_t,
.get_frequency = helene_get_frequency,
 };
 
@@ -871,6 +884,20 @@ static const struct dvb_tuner_ops helene_tuner_ops_s = {
.get_frequency = helene_get_frequency,
 };
 
+static const struct dvb_tuner_ops helene_tuner_ops = {
+   .info = {
+   .name = "Sony HELENE Sat/Ter tuner",
+   .frequency_min = 50,
+   .frequency_max = 12,
+   .frequency_step = 1000,
+   },
+   .init = helene_init,
+   .release = helene_release,
+   .sleep = helene_sleep,
+   .set_params = helene_set_params,
+   .get_frequency = helene_get_frequency,
+};
+
 /* power-on tuner
  * call once after reset
  */
@@ -1032,7 +1059,7 @@ struct dvb_frontend *helene_attach(struct dvb_frontend 
*fe,
if (fe->ops.i2c_gate_ctrl)
fe->ops.i2c_gate_ctrl(fe, 0);
 
-   memcpy(>ops.tuner_ops, _tuner_ops,
+   memcpy(>ops.tuner_ops, _tuner_ops_t,
sizeof(struct dvb_tuner_ops));
fe->tuner_priv = priv;
dev_info(>i2c->dev,
@@ -1042,6 +1069,59 @@ struct dvb_frontend *helene_attach(struct dvb_frontend 
*fe,
 }
 EXPORT_SYMBOL(helene_attach);
 
+static int helene_probe(struct i2c_client *client,
+   const struct i2c_device_id *id)
+{
+   struct helene_config *config = client->dev.platform_data;
+   struct dvb_frontend *fe = config->fe;
+   struct device *dev = >dev;
+   struct helene_priv *priv;
+
+   priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+   if (!priv)
+   return -ENOMEM;
+
+   priv->i2c_address = client->addr;
+   priv->i2c = client->adapter;
+   priv->set_tuner_data = config->set_tuner_priv;
+   priv->set_tuner = config->set_tuner_callback;
+   priv->xtal = config->xtal;
+
+   if (fe->ops.i2c_gate_ctrl)
+   fe->ops.i2c_gate_ctrl(fe, 1);
+
+   if (helene_x_pon(priv) != 0)
+   return -EINVAL;
+
+   if (fe->ops.i2c_gate_ctrl)
+   fe->ops.i2c_gate_ctrl(fe, 0);
+
+   memcpy(>ops.tuner_ops, _tuner_ops,
+  sizeof(struct dvb_tuner_ops));
+   fe->tuner_priv = priv;
+   i2c_set_clientdata(client, priv);
+
+   dev_info(dev, "Sony HELENE attached on addr=%x at I2C adapter %p\n",
+priv->i2c_address, priv->i2c);
+
+   return 0;
+}
+
+static const struct i2c_device_id helene_id[] = {
+   { "helene", },
+   {}
+};
+MODULE_DEVICE_TABLE(i2c, helene_id);
+
+static struct i2c_driver helene_driver = {
+   .driver = {
+   .name = "helene",
+   },
+   .probe= helene_probe,
+   .id_table = helene_id,
+};
+module_i2c_driver(helene_driver);
+
 MODULE_DESCRIPTION("Sony HELENE Sat/Ter tuner driver");
 MODULE_AUTHOR("Abylay Ospan ");
 MODULE_LICENSE("GPL");
diff --git 

Re: [PATCH v6 09/17] media: rkisp1: add rockchip isp1 core driver

[Tomasz Figa]

Hi Jacob, Shunqian,

On Thu, Mar 8, 2018 at 6:49 PM Jacob Chen  wrote:
[snip]
> +static const struct of_device_id rkisp1_plat_of_match[] = {
> +   {
> +   .compatible = "rockchip,rk3288-cif-isp",
> +   .data = _isp_clk_data,
> +   }, {
> +   .compatible = "rockchip,rk3399-cif-isp",
> +   .data = _isp_clk_data,
> +   },
> +   {},
> +};

We need MODULE_DEVICE_TABLE() here.

Best regards,
Tomasz

Re: [PATCH v2 3/5] mfd: cros-ec: Introduce CEC commands and events definitions.

[Neil Armstrong]

Hi Hans,

On 15/05/2018 17:28, Hans Verkuil wrote:
> On 05/15/2018 04:42 PM, Neil Armstrong wrote:
>> The EC can expose a CEC bus, this patch adds the CEC related definitions
>> needed by the cros-ec-cec driver.
>> Having a 16 byte mkbp event size makes it possible to send CEC
>> messages from the EC to the AP directly inside the mkbp event
>> instead of first doing a notification and then a read.
>>
>> Signed-off-by: Stefan Adolfsson 
>> Signed-off-by: Neil Armstrong 
>> ---
>>  drivers/platform/chrome/cros_ec_proto.c | 42 +
>>  include/linux/mfd/cros_ec.h |  2 +-
>>  include/linux/mfd/cros_ec_commands.h| 80 
>> +
>>  3 files changed, 114 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/platform/chrome/cros_ec_proto.c 
>> b/drivers/platform/chrome/cros_ec_proto.c
>> index e7bbdf9..ba47f79 100644
>> --- a/drivers/platform/chrome/cros_ec_proto.c
>> +++ b/drivers/platform/chrome/cros_ec_proto.c
>> @@ -504,29 +504,53 @@ int cros_ec_cmd_xfer_status(struct cros_ec_device 
>> *ec_dev,
>>  }
>>  EXPORT_SYMBOL(cros_ec_cmd_xfer_status);
>>  
>> +static int get_next_event_xfer(struct cros_ec_device *ec_dev,
>> +   struct cros_ec_command *msg,
>> +   int version, uint32_t size)
>> +{
>> +int ret;
>> +
>> +msg->version = version;
>> +msg->command = EC_CMD_GET_NEXT_EVENT;
>> +msg->insize = size;
>> +msg->outsize = 0;
>> +
>> +ret = cros_ec_cmd_xfer(ec_dev, msg);
>> +if (ret > 0) {
>> +ec_dev->event_size = ret - 1;
>> +memcpy(_dev->event_data, msg->data, size);
>> +}
>> +
>> +return ret;
>> +}
>> +
>>  static int get_next_event(struct cros_ec_device *ec_dev)
>>  {
>>  u8 buffer[sizeof(struct cros_ec_command) + sizeof(ec_dev->event_data)];
>>  struct cros_ec_command *msg = (struct cros_ec_command *)
>> +static int cmd_version = 1;
>>  int ret;
>>  
>> +BUILD_BUG_ON(sizeof(union ec_response_get_next_data_v1) != 16);
> 
> Use the define instead of hardcoding 16. I'm not really sure why you need 
> this.
> If cec_message uses the right define for the array size (see my comment 
> below),
> then this really can't go wrong, can it?

This is taken from the chrome kernelk, to be sure the size is ok, but yes it 
should be 16, I'll see
if I can drop this.

> 
>> +
>>  if (ec_dev->suspended) {
>>  dev_dbg(ec_dev->dev, "Device suspended.\n");
>>  return -EHOSTDOWN;
>>  }
>>  
>> -msg->version = 0;
>> -msg->command = EC_CMD_GET_NEXT_EVENT;
>> -msg->insize = sizeof(ec_dev->event_data);
>> -msg->outsize = 0;
>> +if (cmd_version == 1) {
>> +ret = get_next_event_xfer(ec_dev, msg, cmd_version,
>> +  sizeof(ec_dev->event_data));
>> +if (ret != EC_RES_INVALID_VERSION)
>> +return ret;
>>  
>> -ret = cros_ec_cmd_xfer(ec_dev, msg);
>> -if (ret > 0) {
>> -ec_dev->event_size = ret - 1;
>> -memcpy(_dev->event_data, msg->data,
>> -   sizeof(ec_dev->event_data));
>> +/* Fallback to version 0 for future send attempts */
>> +cmd_version = 0;
>>  }
>>  
>> +ret = get_next_event_xfer(ec_dev, msg, cmd_version,
>> +  sizeof(struct ec_response_get_next_event));
>> +
>>  return ret;
>>  }
>>  
>> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
>> index 2d4e23c..f3415eb 100644
>> --- a/include/linux/mfd/cros_ec.h
>> +++ b/include/linux/mfd/cros_ec.h
>> @@ -147,7 +147,7 @@ struct cros_ec_device {
>>  bool mkbp_event_supported;
>>  struct blocking_notifier_head event_notifier;
>>  
>> -struct ec_response_get_next_event event_data;
>> +struct ec_response_get_next_event_v1 event_data;
>>  int event_size;
>>  u32 host_event_wake_mask;
>>  };
>> diff --git a/include/linux/mfd/cros_ec_commands.h 
>> b/include/linux/mfd/cros_ec_commands.h
>> index f2edd99..18df466 100644
>> --- a/include/linux/mfd/cros_ec_commands.h
>> +++ b/include/linux/mfd/cros_ec_commands.h
>> @@ -804,6 +804,8 @@ enum ec_feature_code {
>>  EC_FEATURE_MOTION_SENSE_FIFO = 24,
>>  /* EC has RTC feature that can be controlled by host commands */
>>  EC_FEATURE_RTC = 27,
>> +/* EC supports CEC commands */
>> +EC_FEATURE_CEC = 35,
>>  };
>>  
>>  #define EC_FEATURE_MASK_0(event_code) (1UL << (event_code % 32))
>> @@ -2078,6 +2080,12 @@ enum ec_mkbp_event {
>>  /* EC sent a sysrq command */
>>  EC_MKBP_EVENT_SYSRQ = 6,
>>  
>> +/* Notify the AP that something happened on CEC */
>> +EC_MKBP_CEC_EVENT = 8,
>> +
>> +/* Send an incoming CEC message to the AP */
>> +EC_MKBP_EVENT_CEC_MESSAGE = 9,
>> +
>>  /* Number of MKBP events */
>>  EC_MKBP_EVENT_COUNT,
>>  };
>> @@ -2093,12 +2101,31 @@ union ec_response_get_next_data 

Re: [PATCH v2 4/5] mfd: cros_ec_dev: Add CEC sub-device registration

[Neil Armstrong]

Hi Enric,

On 15/05/2018 18:40, Enric Balletbo Serra wrote:
> Hi Neil,
> 
> I suspect that this patch will conflict with some patches that will be
> queued for 4.18 that also introduces new devices, well, for now I
> don't see these merged in the Lee's tree.

Indeed, I found your patches, I'll rebase this one when Lee pushes them in his 
tree.

> 
> Based on some reviews I got when I send a patch to this file ...
> 
> 2018-05-15 17:29 GMT+02:00 Hans Verkuil :
>> On 05/15/2018 04:42 PM, Neil Armstrong wrote:
>>> The EC can expose a CEC bus, thus add the cros-ec-cec MFD sub-device
>>> when the CEC feature bit is present.
>>>
>>> Signed-off-by: Neil Armstrong 
>>
>> For what it is worth (not an MFD expert):
>>
>> Acked-by: Hans Verkuil 
>>
>> Thanks!
>>
>> Hans
>>
>>> ---
>>>  drivers/mfd/cros_ec_dev.c | 16 
>>>  1 file changed, 16 insertions(+)
>>>
>>> diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
>>> index eafd06f..57064ec 100644
>>> --- a/drivers/mfd/cros_ec_dev.c
>>> +++ b/drivers/mfd/cros_ec_dev.c
>>> @@ -383,6 +383,18 @@ static void cros_ec_sensors_register(struct 
>>> cros_ec_dev *ec)
>>>   kfree(msg);
>>>  }
>>>
>>> +static void cros_ec_cec_register(struct cros_ec_dev *ec)
>>> +{
>>> + int ret;
>>> + struct mfd_cell cec_cell = {
>>> + .name = "cros-ec-cec",
>>> + };
>>> +
>>> + ret = mfd_add_devices(ec->dev, 0, _cell, 1, NULL, 0, NULL);
>>> + if (ret)
>>> + dev_err(ec->dev, "failed to add EC CEC\n");
>>> +}
>>> +
> 
> Do not create a single function to only call mfd_add_devices, instead
> do the following on top:
> 
> static const struct mfd_cell cros_ec_cec_cells[] = {
> { .name = "cros-ec-cec" }
> };

OK

> 
> 
>>>  static int ec_device_probe(struct platform_device *pdev)
>>>  {
>>>   int retval = -ENOMEM;
>>> @@ -422,6 +434,10 @@ static int ec_device_probe(struct platform_device 
>>> *pdev)
>>>   if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE))
>>>   cros_ec_sensors_register(ec);
>>>
>>> + /* check whether this EC handles CEC. */
>>> + if (cros_ec_check_features(ec, EC_FEATURE_CEC))
>>> + cros_ec_cec_register(ec);
>>> +
> 
> and use PLATFORM_DEVID_AUTO and the ARRAY_SIZE macro, something like this.
> 
> /* Check whether this EC instance handles CEC */
> if (cros_ec_check_features(ec, EC_FEATURE_CEC)) {
> retval = mfd_add_devices(ec->dev, PLATFORM_DEVID_AUTO,
>   cros_ec_cec_cells,
>   
> ARRAY_SIZE(cros_ec_cec_cells),
>   NULL, 0, NULL);
> if (retval)
> dev_err(ec->dev, "failed to add cros-ec-cec device: %d\n",
>  retval);
> }

Ok, like the RTC registration.

Thanks,
Neil

> 
> Best regards,
>   Enric
> 
>>>   /* Take control of the lightbar from the EC. */
>>>   lb_manual_suspend_ctrl(ec, 1);
>>>
>>>
>>
>> ___
>> dri-devel mailing list
>> dri-de...@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 2/2] ARM: dts: r8a7740: Add CEU0

[Geert Uytterhoeven]

Hi Jacopo,

On Thu, Apr 26, 2018 at 8:24 PM, Jacopo Mondi  wrote:
> Describe CEU0 peripheral for Renesas R-Mobile A1 R8A7740 Soc.
>
> Reported-by: Geert Uytterhoeven 
> Signed-off-by: Jacopo Mondi 

Thanks for your patch!

Reviewed-by: Geert Uytterhoeven 

Minor question below.

> --- a/arch/arm/boot/dts/r8a7740.dtsi
> +++ b/arch/arm/boot/dts/r8a7740.dtsi
> @@ -67,6 +67,16 @@
> power-domains = <_d4>;
> };
>
> +   ceu0: ceu@fe91 {
> +   reg = <0xfe91 0x3000>;
> +   compatible = "renesas,r8a7740-ceu";
> +   interrupts = ;
> +   clocks = <_clks R8A7740_CLK_CEU20>;
> +   clock-names = "ceu20";

Why the "clock-names" property? It's not mentioned in the DT bindings, and
may cause issues if the bindings are ever amended.

> +   power-domains = <_a4r>;
> +   status = "disabled";
> +   };
> +

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [Intel-gfx] [PATCH v2 2/5] drm/i915: hdmi: add CEC notifier to intel_hdmi

[Neil Armstrong]

On 16/05/2018 09:31, Neil Armstrong wrote:
> Hi Ville,
> 
> On 15/05/2018 17:35, Ville Syrjälä wrote:
>> On Tue, May 15, 2018 at 04:42:19PM +0200, Neil Armstrong wrote:
>>> This patchs adds the cec_notifier feature to the intel_hdmi part
>>> of the i915 DRM driver. It uses the HDMI DRM connector name to differentiate
>>> between each HDMI ports.
>>> The changes will allow the i915 HDMI code to notify EDID and HPD changes
>>> to an eventual CEC adapter.
>>>
>>> Signed-off-by: Neil Armstrong 
>>> ---
>>>  drivers/gpu/drm/i915/Kconfig  |  1 +
>>>  drivers/gpu/drm/i915/intel_drv.h  |  2 ++
>>>  drivers/gpu/drm/i915/intel_hdmi.c | 12 
>>>  3 files changed, 15 insertions(+)
>>>
> 
> [..]
> 
>>>  }
>>>  
>>> @@ -2031,6 +2037,8 @@ static void chv_hdmi_pre_enable(struct intel_encoder 
>>> *encoder,
>>>  
>>>  static void intel_hdmi_destroy(struct drm_connector *connector)
>>>  {
>>> +   if (intel_attached_hdmi(connector)->notifier)
>>> +   cec_notifier_put(intel_attached_hdmi(connector)->notifier);
>>> kfree(to_intel_connector(connector)->detect_edid);
>>> drm_connector_cleanup(connector);
>>> kfree(connector);
>>> @@ -2358,6 +2366,10 @@ void intel_hdmi_init_connector(struct 
>>> intel_digital_port *intel_dig_port,
>>> u32 temp = I915_READ(PEG_BAND_GAP_DATA);
>>> I915_WRITE(PEG_BAND_GAP_DATA, (temp & ~0xf) | 0xd);
>>> }
>>> +
>>> +   intel_hdmi->notifier = cec_notifier_get_conn(dev->dev, connector->name);
>>
>> What are we doing with the connector name here? Those are not actually
>> guaranteed to be stable, and any change in the connector probe order
>> may cause the name to change.
> 
> The i915 driver can expose multiple HDMI connectors, but each connected will
> have a different EDID and CEC physical address, so we will need a different 
> notifier
> for each connector.
> 
> The connector name is DRM dependent, but user-space actually uses this name 
> for
> operations, so I supposed it was kind of stable.
> 
> Anyway, is there another stable id/name/whatever that can be used to make a 
> name to
> distinguish the HDMI connectors ?

Would "HDMI %c", port_name(port) be OK to use ?

Neil

> 
> Neil
> 
>>
>>> +   if (!intel_hdmi->notifier)
>>> +   DRM_DEBUG_KMS("CEC notifier get failed\n");
>>>  }
>>>  
>>>  void intel_hdmi_init(struct drm_i915_private *dev_priv,
>>> -- 
>>> 2.7.4
>>>
>>> ___
>>> Intel-gfx mailing list
>>> intel-...@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
> 

Re: [Intel-gfx] [PATCH v2 2/5] drm/i915: hdmi: add CEC notifier to intel_hdmi

[Neil Armstrong]

Hi Ville,

On 15/05/2018 17:35, Ville Syrjälä wrote:
> On Tue, May 15, 2018 at 04:42:19PM +0200, Neil Armstrong wrote:
>> This patchs adds the cec_notifier feature to the intel_hdmi part
>> of the i915 DRM driver. It uses the HDMI DRM connector name to differentiate
>> between each HDMI ports.
>> The changes will allow the i915 HDMI code to notify EDID and HPD changes
>> to an eventual CEC adapter.
>>
>> Signed-off-by: Neil Armstrong 
>> ---
>>  drivers/gpu/drm/i915/Kconfig  |  1 +
>>  drivers/gpu/drm/i915/intel_drv.h  |  2 ++
>>  drivers/gpu/drm/i915/intel_hdmi.c | 12 
>>  3 files changed, 15 insertions(+)
>>

[..]

>>  }
>>  
>> @@ -2031,6 +2037,8 @@ static void chv_hdmi_pre_enable(struct intel_encoder 
>> *encoder,
>>  
>>  static void intel_hdmi_destroy(struct drm_connector *connector)
>>  {
>> +if (intel_attached_hdmi(connector)->notifier)
>> +cec_notifier_put(intel_attached_hdmi(connector)->notifier);
>>  kfree(to_intel_connector(connector)->detect_edid);
>>  drm_connector_cleanup(connector);
>>  kfree(connector);
>> @@ -2358,6 +2366,10 @@ void intel_hdmi_init_connector(struct 
>> intel_digital_port *intel_dig_port,
>>  u32 temp = I915_READ(PEG_BAND_GAP_DATA);
>>  I915_WRITE(PEG_BAND_GAP_DATA, (temp & ~0xf) | 0xd);
>>  }
>> +
>> +intel_hdmi->notifier = cec_notifier_get_conn(dev->dev, connector->name);
> 
> What are we doing with the connector name here? Those are not actually
> guaranteed to be stable, and any change in the connector probe order
> may cause the name to change.

The i915 driver can expose multiple HDMI connectors, but each connected will
have a different EDID and CEC physical address, so we will need a different 
notifier
for each connector.

The connector name is DRM dependent, but user-space actually uses this name for
operations, so I supposed it was kind of stable.

Anyway, is there another stable id/name/whatever that can be used to make a 
name to
distinguish the HDMI connectors ?

Neil

> 
>> +if (!intel_hdmi->notifier)
>> +DRM_DEBUG_KMS("CEC notifier get failed\n");
>>  }
>>  
>>  void intel_hdmi_init(struct drm_i915_private *dev_priv,
>> -- 
>> 2.7.4
>>
>> ___
>> Intel-gfx mailing list
>> intel-...@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>