Re: [PATCH v5 1/4] dt-bindings: media: Add bindings for OV5695

[Shunqian Zheng]

Hi Jacopo,


On 2018年01月10日 17:20, jacopo mondi wrote:

Hi Shunqian,

On Wed, Jan 10, 2018 at 10:06:04AM +0800, Shunqian Zheng wrote:

Add device tree binding documentation for the OV5695 sensor.

Signed-off-by: Shunqian Zheng 
---
  .../devicetree/bindings/media/i2c/ov5695.txt   | 41 ++
  1 file changed, 41 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov5695.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/ov5695.txt 
b/Documentation/devicetree/bindings/media/i2c/ov5695.txt
new file mode 100644
index 000..2f2f698
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov5695.txt
@@ -0,0 +1,41 @@
+* Omnivision OV5695 MIPI CSI-2 sensor
+
+Required Properties:
+- compatible: shall be "ovti,ov5695"
+- clocks: reference to the xvclk input clock
+- clock-names: shall be "xvclk"
+- avdd-supply: Analog voltage supply, 2.8 volts
+- dovdd-supply: Digital I/O voltage supply, 1.8 volts
+- dvdd-supply: Digital core voltage supply, 1.2 volts
+- reset-gpios: Low active reset gpio
+
+The device node shall contain one 'port' child node with an
+'endpoint' subnode for its digital output video port,
+in accordance with the video interface bindings defined in
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+The endpoint optional property 'data-lanes' shall be "<1 2>".

What happens if the property is not present? What's the default?
I think it depends on how the video receiver deal with, 'data-lanes' is 
optional as described in video-interfaces.txt,

but if somehow it used in DT, the value "<1 2>" is the right one.


I would:

Required Properties:
- compatible: ..


Option Endpoint Properties:
- data-lanes: ...


+
+Example:
+ {
+   camera-sensor: ov5695@36 {

You have inverted the label with the node name which should be generic

 ov5695: camera@36 {


 }
Thanks
j

Thank you very much~
Shunqian

Re: [PATCH v5 2/4] media: ov5695: add support for OV5695 sensor

[Shunqian Zheng]

Hi Jacopo,


On 2018年01月10日 17:08, jacopo mondi wrote:

Hello Shunqian,

On Wed, Jan 10, 2018 at 10:06:05AM +0800, Shunqian Zheng wrote:

[snip]


+static int __ov5695_start_stream(struct ov5695 *ov5695)
+{
+   int ret;
+
+   ret = ov5695_write_array(ov5695->client, ov5695_global_regs);
+   if (ret)
+   return ret;
+   ret = ov5695_write_array(ov5695->client, ov5695->cur_mode->reg_list);
+   if (ret)
+   return ret;
+
+   /* In case these controls are set before streaming */
+   ret = __v4l2_ctrl_handler_setup(>ctrl_handler);
+   if (ret)
+   return ret;
+
+   return ov5695_write_reg(ov5695->client, OV5695_REG_CTRL_MODE,
+   OV5695_REG_VALUE_08BIT, OV5695_MODE_STREAMING);
+}
+
+static int __ov5695_stop_stream(struct ov5695 *ov5695)
+{
+   return ov5695_write_reg(ov5695->client, OV5695_REG_CTRL_MODE,
+   OV5695_REG_VALUE_08BIT, OV5695_MODE_SW_STANDBY);
+}
+
+static int ov5695_s_stream(struct v4l2_subdev *sd, int on)
+{
+   struct ov5695 *ov5695 = to_ov5695(sd);
+   struct i2c_client *client = ov5695->client;
+   int ret = 0;
+
+   mutex_lock(>mutex);
+   on = !!on;
+   if (on == ov5695->streaming)
+   goto unlock_and_return;
+
+   if (on) {
+   ret = pm_runtime_get_sync(>dev);
+   if (ret < 0) {
+   pm_runtime_put_noidle(>dev);
+   goto unlock_and_return;
+   }
+
+   ret = __ov5695_start_stream(ov5695);
+   if (ret) {
+   v4l2_err(sd, "start stream failed while write regs\n");
+   pm_runtime_put(>dev);
+   goto unlock_and_return;
+   }
+   } else {
+   __ov5695_stop_stream(ov5695);
+   ret = pm_runtime_put(>dev);

I would return the result of __ov5695_stop_stream() instead of
pm_runtime_put().

I know I asked for this, but if the first s_stream(0) fails, the
sensor may not have been stopped but the interface will be put in
"streaming = 0" state, preventing a second s_stream(0) to be issued
because of your check "on == ov5695->streaming" a few lines above.

I can't tell how bad this is. Imho is acceptable but I would like to
hear someone else opinion here :)

How about not checking the return values of s_stream(0) branch?
It seems not much this driver can do if pm_runtime_put() fails.



+   }
+
+   ov5695->streaming = on;
+
+unlock_and_return:
+   mutex_unlock(>mutex);
+
+   return ret;
+}
+
+

[snip]


+static const struct of_device_id ov5695_of_match[] = {
+   { .compatible = "ovti,ov5695" },
+   {},
+};

If you don't list CONFIG_OF as a dependecy for this driver (which you
should not imho), please guard this with:

#if IS_ENABLED(CONFIG_OF)

#endif


+
+static struct i2c_driver ov5695_i2c_driver = {
+   .driver = {
+   .name = "ov5695",
+   .owner = THIS_MODULE,
+   .pm = _pm_ops,
+   .of_match_table = ov5695_of_match
+   },
+   .probe  = _probe,
+   .remove = _remove,
+};
+
+module_i2c_driver(ov5695_i2c_driver);
+
+MODULE_DESCRIPTION("OmniVision ov5695 sensor driver");
+MODULE_LICENSE("GPL v2");

As you've fixed my comments on v1, and with the above bits addressed:

Reviewed-by: Jacopo Mondi 

Thank you very much~
Shunqian


Thanks
j


--
1.9.1

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 Jan 11 05:00:16 CET 2018
media-tree git hash:e3ee691dbf24096ea51b3200946b11d68ce75361
media_build git hash:   46c9dc0a08499791cedfc7ee0df387e475f075a2
v4l-utils git hash: 3687804d3f411615d8d622f8bb3ffc060329a804
gcc version:i686-linux-gcc (GCC) 7.1.0
sparse version: v0.5.0-3911-g6f737e1f
smatch version: v0.5.0-3911-g6f737e1f
host hardware:  x86_64
host os:4.13.0-164

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

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

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

[PATCH v5 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI)

[Yong Deng]

Add binding documentation for Allwinner V3s CSI.

Signed-off-by: Yong Deng 
---
 .../devicetree/bindings/media/sun6i-csi.txt| 59 ++
 1 file changed, 59 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt

diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt 
b/Documentation/devicetree/bindings/media/sun6i-csi.txt
new file mode 100644
index 000..2ff47a9
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt
@@ -0,0 +1,59 @@
+Allwinner V3s Camera Sensor Interface
+-
+
+Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2
+interface and CSI1 is used for parallel interface.
+
+Required properties:
+  - compatible: value must be "allwinner,sun8i-v3s-csi"
+  - reg: base address and size of the memory-mapped region.
+  - interrupts: interrupt associated to this IP
+  - clocks: phandles to the clocks feeding the CSI
+* bus: the CSI interface clock
+* mod: the CSI module clock
+* ram: the CSI DRAM clock
+  - clock-names: the clock names mentioned above
+  - resets: phandles to the reset line driving the CSI
+
+Each CSI node should contain one 'port' child node with one child 'endpoint'
+node, according to the bindings defined in
+Documentation/devicetree/bindings/media/video-interfaces.txt. As mentioned
+above, the endpoint's bus type should be MIPI CSI-2 for CSI0 and parallel or
+Bt656 for CSI1.
+
+Endpoint node properties for CSI1
+-
+
+- remote-endpoint  : (required) a phandle to the bus receiver's endpoint
+  node
+- bus-width:   : (required) must be 8, 10, 12 or 16
+- pclk-sample  : (optional) (default: sample on falling edge)
+- hsync-active : (only required for parallel)
+- vsync-active : (only required for parallel)
+
+Example:
+
+csi1: csi@1cb4000 {
+   compatible = "allwinner,sun8i-v3s-csi";
+   reg = <0x01cb4000 0x1000>;
+   interrupts = ;
+   clocks = < CLK_BUS_CSI>,
+< CLK_CSI1_SCLK>,
+< CLK_DRAM_CSI>;
+   clock-names = "bus", "mod", "ram";
+   resets = < RST_BUS_CSI>;
+
+   port {
+   /* Parallel bus endpoint */
+   csi1_ep: endpoint {
+   remote-endpoint = <_ep>;
+   bus-width = <16>;
+
+   /* If hsync-active/vsync-active are missing,
+  embedded BT.656 sync is used */
+   hsync-active = <0>; /* Active low */
+   vsync-active = <0>; /* Active low */
+   pclk-sample = <1>;  /* Rising */
+   };
+   };
+};
-- 
1.8.3.1

[PATCH v5 2/2] media: V3s: Add support for Allwinner CSI.

[Yong Deng]

Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2
interface and CSI1 is used for parallel interface. This is not
documented in datasheet but by test and guess.

This patch implement a v4l2 framework driver for it.

Currently, the driver only support the parallel interface. MIPI-CSI2,
ISP's support are not included in this patch.

Signed-off-by: Yong Deng 
---
 MAINTAINERS|   8 +
 drivers/media/platform/Kconfig |   1 +
 drivers/media/platform/Makefile|   2 +
 drivers/media/platform/sunxi/sun6i-csi/Kconfig |   9 +
 drivers/media/platform/sunxi/sun6i-csi/Makefile|   3 +
 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c | 908 +
 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h | 143 
 .../media/platform/sunxi/sun6i-csi/sun6i_csi_reg.h | 196 +
 .../media/platform/sunxi/sun6i-csi/sun6i_video.c   | 741 +
 .../media/platform/sunxi/sun6i-csi/sun6i_video.h   |  53 ++
 10 files changed, 2064 insertions(+)
 create mode 100644 drivers/media/platform/sunxi/sun6i-csi/Kconfig
 create mode 100644 drivers/media/platform/sunxi/sun6i-csi/Makefile
 create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
 create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h
 create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_reg.h
 create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c
 create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_video.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 9501403..b792fe5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3783,6 +3783,14 @@ M:   Jaya Kumar 
 S: Maintained
 F: sound/pci/cs5535audio/
 
+CSI DRIVERS FOR ALLWINNER V3s
+M: Yong Deng 
+L: linux-media@vger.kernel.org
+T: git git://linuxtv.org/media_tree.git
+S: Maintained
+F: drivers/media/platform/sunxi/sun6i-csi/
+F: Documentation/devicetree/bindings/media/sun6i-csi.txt
+
 CW1200 WLAN driver
 M: Solomon Peachy 
 S: Maintained
diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index fd0c998..41017e3 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -150,6 +150,7 @@ source "drivers/media/platform/am437x/Kconfig"
 source "drivers/media/platform/xilinx/Kconfig"
 source "drivers/media/platform/rcar-vin/Kconfig"
 source "drivers/media/platform/atmel/Kconfig"
+source "drivers/media/platform/sunxi/sun6i-csi/Kconfig"
 
 config VIDEO_TI_CAL
tristate "TI CAL (Camera Adaptation Layer) driver"
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index 003b0bb..e6e9ce7 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -97,3 +97,5 @@ obj-$(CONFIG_VIDEO_QCOM_CAMSS)+= 
qcom/camss-8x16/
 obj-$(CONFIG_VIDEO_QCOM_VENUS) += qcom/venus/
 
 obj-y  += meson/
+
+obj-$(CONFIG_VIDEO_SUN6I_CSI)  += sunxi/sun6i-csi/
diff --git a/drivers/media/platform/sunxi/sun6i-csi/Kconfig 
b/drivers/media/platform/sunxi/sun6i-csi/Kconfig
new file mode 100644
index 000..314188a
--- /dev/null
+++ b/drivers/media/platform/sunxi/sun6i-csi/Kconfig
@@ -0,0 +1,9 @@
+config VIDEO_SUN6I_CSI
+   tristate "Allwinner V3s Camera Sensor Interface driver"
+   depends on VIDEO_V4L2 && COMMON_CLK && VIDEO_V4L2_SUBDEV_API && HAS_DMA
+   depends on ARCH_SUNXI || COMPILE_TEST
+   select VIDEOBUF2_DMA_CONTIG
+   select REGMAP_MMIO
+   select V4L2_FWNODE
+   ---help---
+  Support for the Allwinner Camera Sensor Interface Controller on V3s.
diff --git a/drivers/media/platform/sunxi/sun6i-csi/Makefile 
b/drivers/media/platform/sunxi/sun6i-csi/Makefile
new file mode 100644
index 000..213cb6b
--- /dev/null
+++ b/drivers/media/platform/sunxi/sun6i-csi/Makefile
@@ -0,0 +1,3 @@
+sun6i-csi-y += sun6i_video.o sun6i_csi.o
+
+obj-$(CONFIG_VIDEO_SUN6I_CSI) += sun6i-csi.o
diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c 
b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
new file mode 100644
index 000..9c341f0
--- /dev/null
+++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
@@ -0,0 +1,908 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2011-2018 Magewell Electronics Co., Ltd. (Nanjing)
+ * All rights reserved.
+ * Author: Yong Deng 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "sun6i_csi.h"
+#include "sun6i_csi_reg.h"
+
+#define MODULE_NAME"sun6i-csi"
+
+struct sun6i_csi_dev {
+   struct sun6i_csicsi;
+   struct device   *dev;
+
+   struct regmap   

[PATCH v5 0/2] Initial Allwinner V3s CSI Support

[Yong Deng]

This patchset add initial support for Allwinner V3s CSI.

Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2
interface and CSI1 is used for parallel interface. This is not
documented in datasheet but by test and guess.

This patchset implement a v4l2 framework driver and add a binding 
documentation for it. 

Currently, the driver only support the parallel interface. And has been
tested with a BT1120 signal which generating from FPGA. The following
fetures are not support with this patchset:
  - ISP 
  - MIPI-CSI2
  - Master clock for camera sensor
  - Power regulator for the front end IC

Changes in v5:
  * Using the new SPDX tags.
  * Fix MODULE_LICENSE.
  * Add many default cases and warning messages.
  * Detail the parallel bus properties
  * Fix some spelling and syntax mistakes.

Changes in v4:
  * Deal with the CSI 'INNER QUEUE'.
CSI will lookup the next dma buffer for next frame before the
the current frame done IRQ triggered. This is not documented
but reported by Ondřej Jirman.
The BSP code has workaround for this too. It skip to mark the
first buffer as frame done for VB2 and pass the second buffer
to CSI in the first frame done ISR call. Then in second frame
done ISR call, it mark the first buffer as frame done for VB2
and pass the third buffer to CSI. And so on. The bad thing is
that the first buffer will be written twice and the first frame
is dropped even the queued buffer is sufficient.
So, I make some improvement here. Pass the next buffer to CSI
just follow starting the CSI. In this case, the first frame
will be stored in first buffer, second frame in second buffer.
This mothed is used to avoid dropping the first frame, it
would also drop frame when lacking of queued buffer.
  * Fix: using a wrong mbus_code when getting the supported formats
  * Change all fourcc to pixformat
  * Change some function names

Changes in v3:
  * Get rid of struct sun6i_csi_ops
  * Move sun6i-csi to new directory drivers/media/platform/sunxi
  * Merge sun6i_csi.c and sun6i_csi_v3s.c into sun6i_csi.c
  * Use generic fwnode endpoints parser
  * Only support a single subdev to make things simple
  * Many complaintion fix

Changes in v2: 
  * Change sunxi-csi to sun6i-csi
  * Rebase to media_tree master branch 

Following is the 'v4l2-compliance -s -f' output, I have test this
with both interlaced and progressive signal:

# ./v4l2-compliance -s -f
v4l2-compliance SHA   : 6049ea8bd64f9d78ef87ef0c2b3dc9b5de1ca4a1

Driver Info:
Driver name   : sun6i-video
Card type : sun6i-csi
Bus info  : platform:csi
Driver version: 4.15.0
Capabilities  : 0x8421
Video Capture
Streaming
Extended Pix Format
Device Capabilities
Device Caps   : 0x0421
Video Capture
Streaming
Extended Pix Format

Compliance test for device /dev/video0 (not using libv4l2):

Required ioctls:
test VIDIOC_QUERYCAP: OK

Allow for multiple opens:
test second video open: OK
test VIDIOC_QUERYCAP: OK
test VIDIOC_G/S_PRIORITY: OK
test for unlimited opens: OK

Debug ioctls:
test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
test VIDIOC_LOG_STATUS: OK (Not Supported)

Input ioctls:
test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
test VIDIOC_ENUMAUDIO: OK (Not Supported)
test VIDIOC_G/S/ENUMINPUT: OK
test VIDIOC_G/S_AUDIO: OK (Not Supported)
Inputs: 1 Audio Inputs: 0 Tuners: 0

Output ioctls:
test VIDIOC_G/S_MODULATOR: OK (Not Supported)
test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
test VIDIOC_ENUMAUDOUT: OK (Not Supported)
test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
test VIDIOC_G/S_AUDOUT: OK (Not Supported)
Outputs: 0 Audio Outputs: 0 Modulators: 0

Input/Output configuration ioctls:
test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
test VIDIOC_G/S_EDID: OK (Not Supported)

Test input 0:

Control ioctls:
test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK (Not Supported)
test VIDIOC_QUERYCTRL: OK (Not Supported)
test VIDIOC_G/S_CTRL: OK (Not Supported)
test VIDIOC_G/S/TRY_EXT_CTRLS: OK (Not Supported)
test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK (Not Supported)
test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
Standard Controls: 0 Private Controls: 0

Format ioctls:
test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
test VIDIOC_G/S_PARM: OK (Not Supported)
test 

Re: [PATCH v5 0/5] Add OV5640 parallel interface and RGB565/YUYV support

[Yong]

Hi Maxime,

On Wed, 10 Jan 2018 16:37:24 +0100
Maxime Ripard  wrote:

> Hi Hugues,
> 
> On Mon, Jan 08, 2018 at 05:13:39PM +, Hugues FRUCHET wrote:
> > I'm using a ST board with OV5640 wired in parallel bus output in order 
> > to interface to my STM32 DCMI parallel interface.
> > Perhaps could you describe your setup so I could help on understanding 
> > the problem on your side. From my past experience with this sensor 
> > module, you can first check hsync/vsync polarities, the datasheet is 
> > buggy on VSYNC polarity as documented in patch 4/5.
> 
> It turns out that it was indeed a polarity issue.
> 
> It looks like that in order to operate properly, I need to setup the
> opposite polarity on HSYNC and VSYNC on the interface. I looked at the
> signals under a scope, and VSYNC is obviously inversed as you
> described. HSYNC, I'm not so sure since the HBLANK period seems very
> long, almost a line.
> 
> Since VSYNC at least looks correct, I'd be inclined to think that the
> polarity is inversed on at least the SoC I'm using it on.
> 
> Yong, did you test the V3S CSI driver with a parallel interface? With
> what sensor driver? Have you found some polarities issues like this?

Did you try it with Allwinner SoCs?

No. I only tested with a BT1120 signal generated by FPGA or ADV7611. HSYNC
and VSYNC are not used.

For V3s CSI driver, I will add the following to dt-bindings:
Endpoint node properties for CSI1
-

- remote-endpoint  : (required) a phandle to the bus receiver's endpoint
  node
- bus-width:   : (required) must be 8, 10, 12 or 16
- pclk-sample  : (optional) (default: sample on falling edge)
- hsync-active : (only required for parallel)
- vsync-active : (only required for parallel)

You could try diffrent hsync-active/vsync-active values here.

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


Thanks,
Yong

Re: [PATCH v5 0/5] Add OV5640 parallel interface and RGB565/YUYV support

[Sakari Ailus]

Hi Hugues,

On Wed, Jan 10, 2018 at 03:51:07PM +, Hugues FRUCHET wrote:
> Good news Maxime !
> 
> Have you seen that you can adapt the polarities through devicetree ?
> 
> +   /* Parallel bus endpoint */
> +   ov5640_to_parallel: endpoint {
> [...]
> +   hsync-active = <0>;
> +   vsync-active = <0>;
> +   pclk-sample = <1>;
> +   };
> 
> Doing so you can adapt to your SoC/board setup easily.
> 
> If you don't put those lines in devicetree, the ov5640 default init 
> sequence is used which set the polarity as defined in below comment:
> ov5640_set_stream_dvp()
> [...]
> +* Control lines polarity can be configured through
> +* devicetree endpoint control lines properties.
> +* If no endpoint control lines properties are set,
> +* polarity will be as below:
> +* - VSYNC: active high
> +* - HREF:  active low
> +* - PCLK:  active low
> +*/
> [...]

The properties are at the moment documented as mandatory in DT binding
documentation.

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

Re: [PATCH 3/4] tsi108_eth: use dma API properly

[Andy Shevchenko]

On Wed, Jan 10, 2018 at 8:03 PM, Christoph Hellwig  wrote:
> We need to pass a struct device to the dma API, even if some
> architectures still support that for legacy reasons, and should not mix
> it with the old PCI dma API.
>
> Note that the driver also seems to never actually unmap its dma mappings,
> but to fix that we'll need someone more familar with the driver.

> +   struct platform_device *pdev;

Do you really need platform_defice reference?

Perhaps

struct device *hdev; // hardware device


data->hdev = >dev;

Another idea

dev->dev.parent = >dev;

No new member needed.

>  };
>
>  /* Structure for a device driver */
> @@ -703,17 +705,18 @@ static int tsi108_send_packet(struct sk_buff * skb, 
> struct net_device *dev)
> data->txskbs[tx] = skb;
>
> if (i == 0) {
> -   data->txring[tx].buf0 = dma_map_single(NULL, 
> skb->data,
> -   skb_headlen(skb), DMA_TO_DEVICE);
> +   data->txring[tx].buf0 = 
> dma_map_single(>pdev->dev,
> +   skb->data, skb_headlen(skb),
> +   DMA_TO_DEVICE);
> data->txring[tx].len = skb_headlen(skb);
> misc |= TSI108_TX_SOF;
> } else {
> const skb_frag_t *frag = _shinfo(skb)->frags[i - 
> 1];
>
> -   data->txring[tx].buf0 = skb_frag_dma_map(NULL, frag,
> -0,
> -
> skb_frag_size(frag),
> -
> DMA_TO_DEVICE);
> +   data->txring[tx].buf0 =
> +   skb_frag_dma_map(>pdev->dev, frag,
> +   0, skb_frag_size(frag),
> +   DMA_TO_DEVICE);
> data->txring[tx].len = skb_frag_size(frag);
> }
>
> @@ -808,9 +811,9 @@ static int tsi108_refill_rx(struct net_device *dev, int 
> budget)
> if (!skb)
> break;
>
> -   data->rxring[rx].buf0 = dma_map_single(NULL, skb->data,
> -   TSI108_RX_SKB_SIZE,
> -   DMA_FROM_DEVICE);
> +   data->rxring[rx].buf0 = dma_map_single(>pdev->dev,
> +   skb->data, TSI108_RX_SKB_SIZE,
> +   DMA_FROM_DEVICE);
>
> /* Sometimes the hardware sets blen to zero after packet
>  * reception, even though the manual says that it's only ever
> @@ -1308,15 +1311,15 @@ static int tsi108_open(struct net_device *dev)
>data->id, dev->irq, dev->name);
> }
>
> -   data->rxring = dma_zalloc_coherent(NULL, rxring_size, >rxdma,
> -  GFP_KERNEL);
> +   data->rxring = dma_zalloc_coherent(>pdev->dev, rxring_size,
> +   >rxdma, GFP_KERNEL);
> if (!data->rxring)
> return -ENOMEM;
>
> -   data->txring = dma_zalloc_coherent(NULL, txring_size, >txdma,
> -  GFP_KERNEL);
> +   data->txring = dma_zalloc_coherent(>pdev->dev, txring_size,
> +   >txdma, GFP_KERNEL);
> if (!data->txring) {
> -   pci_free_consistent(NULL, rxring_size, data->rxring,
> +   dma_free_coherent(>pdev->dev, rxring_size, data->rxring,
> data->rxdma);
> return -ENOMEM;
> }
> @@ -1428,10 +1431,10 @@ static int tsi108_close(struct net_device *dev)
> dev_kfree_skb(skb);
> }
>
> -   dma_free_coherent(0,
> +   dma_free_coherent(>pdev->dev,
> TSI108_RXRING_LEN * sizeof(rx_desc),
> data->rxring, data->rxdma);
> -   dma_free_coherent(0,
> +   dma_free_coherent(>pdev->dev,
> TSI108_TXRING_LEN * sizeof(tx_desc),
> data->txring, data->txdma);
>
> @@ -1576,6 +1579,7 @@ tsi108_init_one(struct platform_device *pdev)
> printk("tsi108_eth%d: probe...\n", pdev->id);
> data = netdev_priv(dev);
> data->dev = dev;
> +   data->pdev = pdev;
>
> 
> pr_debug("tsi108_eth%d:regs:phyresgs:phy:irq_num=0x%x:0x%x:0x%x:0x%x\n",
> pdev->id, einfo->regs, einfo->phyregs,
> --
> 2.14.2
>



-- 
With Best Regards,
Andy Shevchenko

remove pci_dma_* abuses and workarounds V2

[Christoph Hellwig]

Back before the dawn of time pci_dma_* with a NULL pci_dev argument
was used for all kinds of things, e.g. dma mapping for non-PCI
devices.  All this has been long removed, but it turns out we
still care for a NULL pci_dev in the wrappers, and we still have
two odd USB drivers that use pci_dma_alloc_consistent for allocating
memory while ignoring the dma_addr_t entirely, and a network driver
mixing the already wrong usage of dma_* with a NULL device with a
single call to pci_free_consistent.

This series switches the two usb drivers to use plain kzalloc, the
net driver to properly use the dma API and then removes the handling
of the NULL pci_dev in the pci_dma_* wrappers.

Changes since V1:
 - remove allocation failure printks
 - use kcalloc
 - fix tsi108_eth
 - improve changelogs

[PATCH 1/4] media/ttusb-budget: remove pci_zalloc_coherent abuse

[Christoph Hellwig]

Switch to a plain kzalloc instead of pci_zalloc_coherent to allocate
memory for the USB DMA.

Signed-off-by: Christoph Hellwig 
---
 drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c | 18 --
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c 
b/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c
index a142b9dc0feb..ea40a24947ba 100644
--- a/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c
+++ b/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c
@@ -102,7 +102,6 @@ struct ttusb {
unsigned int isoc_in_pipe;
 
void *iso_buffer;
-   dma_addr_t iso_dma_handle;
 
struct urb *iso_urb[ISO_BUF_COUNT];
 
@@ -792,26 +791,17 @@ static void ttusb_free_iso_urbs(struct ttusb *ttusb)
 
for (i = 0; i < ISO_BUF_COUNT; i++)
usb_free_urb(ttusb->iso_urb[i]);
-
-   pci_free_consistent(NULL,
-   ISO_FRAME_SIZE * FRAMES_PER_ISO_BUF *
-   ISO_BUF_COUNT, ttusb->iso_buffer,
-   ttusb->iso_dma_handle);
+   kfree(ttusb->iso_buffer);
 }
 
 static int ttusb_alloc_iso_urbs(struct ttusb *ttusb)
 {
int i;
 
-   ttusb->iso_buffer = pci_zalloc_consistent(NULL,
- ISO_FRAME_SIZE * 
FRAMES_PER_ISO_BUF * ISO_BUF_COUNT,
- >iso_dma_handle);
-
-   if (!ttusb->iso_buffer) {
-   dprintk("%s: pci_alloc_consistent - not enough memory\n",
-   __func__);
+   ttusb->iso_buffer = kcalloc(FRAMES_PER_ISO_BUF * ISO_BUF_COUNT,
+   ISO_FRAME_SIZE, GFP_KERNEL);
+   if (!ttusb->iso_buffer)
return -ENOMEM;
-   }
 
for (i = 0; i < ISO_BUF_COUNT; i++) {
struct urb *urb;
-- 
2.14.2

[PATCH 2/4] media/ttusb-dev: remove pci_zalloc_coherent abuse

[Christoph Hellwig]

Switch to a plain kzalloc instead of pci_zalloc_coherent to allocate
memory for the USB DMA.

Signed-off-by: Christoph Hellwig 
---
 drivers/media/usb/ttusb-dec/ttusb_dec.c | 18 --
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/drivers/media/usb/ttusb-dec/ttusb_dec.c 
b/drivers/media/usb/ttusb-dec/ttusb_dec.c
index cdefb5dfbbdc..4d5acdf578a6 100644
--- a/drivers/media/usb/ttusb-dec/ttusb_dec.c
+++ b/drivers/media/usb/ttusb-dec/ttusb_dec.c
@@ -127,7 +127,6 @@ struct ttusb_dec {
struct urb  *irq_urb;
dma_addr_t  irq_dma_handle;
void*iso_buffer;
-   dma_addr_t  iso_dma_handle;
struct urb  *iso_urb[ISO_BUF_COUNT];
int iso_stream_count;
struct mutexiso_mutex;
@@ -1185,11 +1184,7 @@ static void ttusb_dec_free_iso_urbs(struct ttusb_dec 
*dec)
 
for (i = 0; i < ISO_BUF_COUNT; i++)
usb_free_urb(dec->iso_urb[i]);
-
-   pci_free_consistent(NULL,
-   ISO_FRAME_SIZE * (FRAMES_PER_ISO_BUF *
- ISO_BUF_COUNT),
-   dec->iso_buffer, dec->iso_dma_handle);
+   kfree(dec->iso_buffer);
 }
 
 static int ttusb_dec_alloc_iso_urbs(struct ttusb_dec *dec)
@@ -1198,15 +1193,10 @@ static int ttusb_dec_alloc_iso_urbs(struct ttusb_dec 
*dec)
 
dprintk("%s\n", __func__);
 
-   dec->iso_buffer = pci_zalloc_consistent(NULL,
-   ISO_FRAME_SIZE * 
(FRAMES_PER_ISO_BUF * ISO_BUF_COUNT),
-   >iso_dma_handle);
-
-   if (!dec->iso_buffer) {
-   dprintk("%s: pci_alloc_consistent - not enough memory\n",
-   __func__);
+   dec->iso_buffer = kcalloc(FRAMES_PER_ISO_BUF * ISO_BUF_COUNT,
+   ISO_FRAME_SIZE, GFP_KERNEL);
+   if (!dec->iso_buffer)
return -ENOMEM;
-   }
 
for (i = 0; i < ISO_BUF_COUNT; i++) {
struct urb *urb;
-- 
2.14.2

[PATCH 3/4] tsi108_eth: use dma API properly

[Christoph Hellwig]

We need to pass a struct device to the dma API, even if some
architectures still support that for legacy reasons, and should not mix
it with the old PCI dma API.

Note that the driver also seems to never actually unmap its dma mappings,
but to fix that we'll need someone more familar with the driver.

Signed-off-by: Christoph Hellwig 
---
 drivers/net/ethernet/tundra/tsi108_eth.c | 36 ++--
 1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/tundra/tsi108_eth.c 
b/drivers/net/ethernet/tundra/tsi108_eth.c
index 0624b71ab5d4..edcd1e60b30d 100644
--- a/drivers/net/ethernet/tundra/tsi108_eth.c
+++ b/drivers/net/ethernet/tundra/tsi108_eth.c
@@ -152,6 +152,8 @@ struct tsi108_prv_data {
u32 msg_enable; /* debug message level */
struct mii_if_info mii_if;
unsigned int init_media;
+
+   struct platform_device *pdev;
 };
 
 /* Structure for a device driver */
@@ -703,17 +705,18 @@ static int tsi108_send_packet(struct sk_buff * skb, 
struct net_device *dev)
data->txskbs[tx] = skb;
 
if (i == 0) {
-   data->txring[tx].buf0 = dma_map_single(NULL, skb->data,
-   skb_headlen(skb), DMA_TO_DEVICE);
+   data->txring[tx].buf0 = dma_map_single(>pdev->dev,
+   skb->data, skb_headlen(skb),
+   DMA_TO_DEVICE);
data->txring[tx].len = skb_headlen(skb);
misc |= TSI108_TX_SOF;
} else {
const skb_frag_t *frag = _shinfo(skb)->frags[i - 1];
 
-   data->txring[tx].buf0 = skb_frag_dma_map(NULL, frag,
-0,
-
skb_frag_size(frag),
-DMA_TO_DEVICE);
+   data->txring[tx].buf0 =
+   skb_frag_dma_map(>pdev->dev, frag,
+   0, skb_frag_size(frag),
+   DMA_TO_DEVICE);
data->txring[tx].len = skb_frag_size(frag);
}
 
@@ -808,9 +811,9 @@ static int tsi108_refill_rx(struct net_device *dev, int 
budget)
if (!skb)
break;
 
-   data->rxring[rx].buf0 = dma_map_single(NULL, skb->data,
-   TSI108_RX_SKB_SIZE,
-   DMA_FROM_DEVICE);
+   data->rxring[rx].buf0 = dma_map_single(>pdev->dev,
+   skb->data, TSI108_RX_SKB_SIZE,
+   DMA_FROM_DEVICE);
 
/* Sometimes the hardware sets blen to zero after packet
 * reception, even though the manual says that it's only ever
@@ -1308,15 +1311,15 @@ static int tsi108_open(struct net_device *dev)
   data->id, dev->irq, dev->name);
}
 
-   data->rxring = dma_zalloc_coherent(NULL, rxring_size, >rxdma,
-  GFP_KERNEL);
+   data->rxring = dma_zalloc_coherent(>pdev->dev, rxring_size,
+   >rxdma, GFP_KERNEL);
if (!data->rxring)
return -ENOMEM;
 
-   data->txring = dma_zalloc_coherent(NULL, txring_size, >txdma,
-  GFP_KERNEL);
+   data->txring = dma_zalloc_coherent(>pdev->dev, txring_size,
+   >txdma, GFP_KERNEL);
if (!data->txring) {
-   pci_free_consistent(NULL, rxring_size, data->rxring,
+   dma_free_coherent(>pdev->dev, rxring_size, data->rxring,
data->rxdma);
return -ENOMEM;
}
@@ -1428,10 +1431,10 @@ static int tsi108_close(struct net_device *dev)
dev_kfree_skb(skb);
}
 
-   dma_free_coherent(0,
+   dma_free_coherent(>pdev->dev,
TSI108_RXRING_LEN * sizeof(rx_desc),
data->rxring, data->rxdma);
-   dma_free_coherent(0,
+   dma_free_coherent(>pdev->dev,
TSI108_TXRING_LEN * sizeof(tx_desc),
data->txring, data->txdma);
 
@@ -1576,6 +1579,7 @@ tsi108_init_one(struct platform_device *pdev)
printk("tsi108_eth%d: probe...\n", pdev->id);
data = netdev_priv(dev);
data->dev = dev;
+   data->pdev = pdev;
 
pr_debug("tsi108_eth%d:regs:phyresgs:phy:irq_num=0x%x:0x%x:0x%x:0x%x\n",
pdev->id, einfo->regs, einfo->phyregs,
-- 
2.14.2

[PATCH 4/4] PCI: Remove NULL device handling from PCI DMA API

[Christoph Hellwig]

Historically some ISA drivers used the old PCI DMA API with a NULL pdev
argument, but these days this isn't used and not too useful due to the
per-device DMA ops, so remove it.

Signed-off-by: Christoph Hellwig 
---
 include/linux/pci-dma-compat.h | 27 +--
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/include/linux/pci-dma-compat.h b/include/linux/pci-dma-compat.h
index d1f9fdade1e0..0dd1a3f7b309 100644
--- a/include/linux/pci-dma-compat.h
+++ b/include/linux/pci-dma-compat.h
@@ -17,91 +17,90 @@ static inline void *
 pci_alloc_consistent(struct pci_dev *hwdev, size_t size,
 dma_addr_t *dma_handle)
 {
-   return dma_alloc_coherent(hwdev == NULL ? NULL : >dev, size, 
dma_handle, GFP_ATOMIC);
+   return dma_alloc_coherent(>dev, size, dma_handle, GFP_ATOMIC);
 }
 
 static inline void *
 pci_zalloc_consistent(struct pci_dev *hwdev, size_t size,
  dma_addr_t *dma_handle)
 {
-   return dma_zalloc_coherent(hwdev == NULL ? NULL : >dev,
-  size, dma_handle, GFP_ATOMIC);
+   return dma_zalloc_coherent(>dev, size, dma_handle, GFP_ATOMIC);
 }
 
 static inline void
 pci_free_consistent(struct pci_dev *hwdev, size_t size,
void *vaddr, dma_addr_t dma_handle)
 {
-   dma_free_coherent(hwdev == NULL ? NULL : >dev, size, vaddr, 
dma_handle);
+   dma_free_coherent(>dev, size, vaddr, dma_handle);
 }
 
 static inline dma_addr_t
 pci_map_single(struct pci_dev *hwdev, void *ptr, size_t size, int direction)
 {
-   return dma_map_single(hwdev == NULL ? NULL : >dev, ptr, size, 
(enum dma_data_direction)direction);
+   return dma_map_single(>dev, ptr, size, (enum 
dma_data_direction)direction);
 }
 
 static inline void
 pci_unmap_single(struct pci_dev *hwdev, dma_addr_t dma_addr,
 size_t size, int direction)
 {
-   dma_unmap_single(hwdev == NULL ? NULL : >dev, dma_addr, size, 
(enum dma_data_direction)direction);
+   dma_unmap_single(>dev, dma_addr, size, (enum 
dma_data_direction)direction);
 }
 
 static inline dma_addr_t
 pci_map_page(struct pci_dev *hwdev, struct page *page,
 unsigned long offset, size_t size, int direction)
 {
-   return dma_map_page(hwdev == NULL ? NULL : >dev, page, offset, 
size, (enum dma_data_direction)direction);
+   return dma_map_page(>dev, page, offset, size, (enum 
dma_data_direction)direction);
 }
 
 static inline void
 pci_unmap_page(struct pci_dev *hwdev, dma_addr_t dma_address,
   size_t size, int direction)
 {
-   dma_unmap_page(hwdev == NULL ? NULL : >dev, dma_address, size, 
(enum dma_data_direction)direction);
+   dma_unmap_page(>dev, dma_address, size, (enum 
dma_data_direction)direction);
 }
 
 static inline int
 pci_map_sg(struct pci_dev *hwdev, struct scatterlist *sg,
   int nents, int direction)
 {
-   return dma_map_sg(hwdev == NULL ? NULL : >dev, sg, nents, (enum 
dma_data_direction)direction);
+   return dma_map_sg(>dev, sg, nents, (enum 
dma_data_direction)direction);
 }
 
 static inline void
 pci_unmap_sg(struct pci_dev *hwdev, struct scatterlist *sg,
 int nents, int direction)
 {
-   dma_unmap_sg(hwdev == NULL ? NULL : >dev, sg, nents, (enum 
dma_data_direction)direction);
+   dma_unmap_sg(>dev, sg, nents, (enum 
dma_data_direction)direction);
 }
 
 static inline void
 pci_dma_sync_single_for_cpu(struct pci_dev *hwdev, dma_addr_t dma_handle,
size_t size, int direction)
 {
-   dma_sync_single_for_cpu(hwdev == NULL ? NULL : >dev, dma_handle, 
size, (enum dma_data_direction)direction);
+   dma_sync_single_for_cpu(>dev, dma_handle, size, (enum 
dma_data_direction)direction);
 }
 
 static inline void
 pci_dma_sync_single_for_device(struct pci_dev *hwdev, dma_addr_t dma_handle,
size_t size, int direction)
 {
-   dma_sync_single_for_device(hwdev == NULL ? NULL : >dev, 
dma_handle, size, (enum dma_data_direction)direction);
+   dma_sync_single_for_device(>dev, dma_handle, size, (enum 
dma_data_direction)direction);
 }
 
 static inline void
 pci_dma_sync_sg_for_cpu(struct pci_dev *hwdev, struct scatterlist *sg,
int nelems, int direction)
 {
-   dma_sync_sg_for_cpu(hwdev == NULL ? NULL : >dev, sg, nelems, 
(enum dma_data_direction)direction);
+   dma_sync_sg_for_cpu(>dev, sg, nelems, (enum 
dma_data_direction)direction);
 }
 
 static inline void
 pci_dma_sync_sg_for_device(struct pci_dev *hwdev, struct scatterlist *sg,
int nelems, int direction)
 {
-   dma_sync_sg_for_device(hwdev == NULL ? NULL : >dev, sg, nelems, 
(enum dma_data_direction)direction);
+   dma_sync_sg_for_device(>dev, sg, nelems, (enum 
dma_data_direction)direction);
 }
 
 static inline int
-- 
2.14.2

media_build modprobe issues with latest Centos7

[Werner, Zachary]

I'm trying to build using the latest media_build and media_tree
branches, but I'm getting an issue when running `modprobe -a cx23885`
(from dmesg):

[ 5373.246321] Linux video capture interface: v2.00
[ 5373.257419] rc_core: loading out-of-tree module taints kernel.
[ 5373.257452] rc_core: module verification failed: signature and/or
required key missing - tainting kernel
[ 5373.257939] rc_core: IR Remote Control driver registered, major 246
[ 5373.257945] WARNING: You are using an experimental version of the
media stack.
As the driver is backported to an older kernel, it doesn't offer
enough quality for its usage in production.
Use it with care.
Latest git patches (needed if you report a bug to linux-media@vger.kernel.org):
e3ee691dbf24096ea51b3200946b11d68ce75361 media: ov5640: add
support of RGB565 and YUYV formats
f22996db44e2db73b333de25a8939fef2bab9620 media: ov5640: add
support of DVP parallel interface
495f014d313df677015ceedf6cec5feba1ac6570 media: dt-bindings:
ov5640: refine CSI-2 and add parallel interface
[ 5373.432275] videobuf2_v4l2: Unknown symbol vb2_buffer_in_use (err 0)
[ 5373.432283] videobuf2_v4l2: Unknown symbol vb2_core_queue_init (err 0)
[ 5373.432290] videobuf2_v4l2: Unknown symbol vb2_verify_memory_type (err 0)
[ 5373.432297] videobuf2_v4l2: Unknown symbol vb2_core_reqbufs (err 0)
[ 5373.432303] videobuf2_v4l2: Unknown symbol vb2_core_expbuf (err 0)
[ 5373.432310] videobuf2_v4l2: Unknown symbol vb2_core_create_bufs (err 0)
[ 5373.432321] videobuf2_v4l2: disagrees about version of symbol vb2_write
[ 5373.432323] videobuf2_v4l2: Unknown symbol vb2_write (err -22)
[ 5373.432330] videobuf2_v4l2: Unknown symbol vb2_core_queue_release (err 0)
[ 5373.432343] videobuf2_v4l2: Unknown symbol vb2_core_prepare_buf (err 0)
[ 5373.432344] videobuf2_v4l2: disagrees about version of symbol vb2_read
[ 5373.432345] videobuf2_v4l2: Unknown symbol vb2_read (err -22)
[ 5373.432351] videobuf2_v4l2: Unknown symbol vb2_core_poll (err 0)
[ 5373.432357] videobuf2_v4l2: Unknown symbol vb2_core_streamon (err 0)
[ 5373.432364] videobuf2_v4l2: Unknown symbol vb2_core_querybuf (err 0)
[ 5373.432371] videobuf2_v4l2: Unknown symbol vb2_core_qbuf (err 0)
[ 5373.432372] videobuf2_v4l2: disagrees about version of symbol vb2_mmap
[ 5373.432373] videobuf2_v4l2: Unknown symbol vb2_mmap (err -22)
[ 5373.432379] videobuf2_v4l2: Unknown symbol vb2_core_dqbuf (err 0)
[ 5373.432386] videobuf2_v4l2: Unknown symbol vb2_core_streamoff (err 0)

`lsmod | grep v4l`:

v4l2_common21263  1 cx2341x
videodev  126499  3 cx2341x,v4l2_common,videobuf2_core
i2c_core   40756  6
drm,i2c_piix4,drm_kms_helper,v4l2_common,tveeprom,videodev

I have verified that there are no other videobuf or v4l modules
installed in the kernel modules directory, and reloading the modules
and rebooting does not solve the issue.

This was not an issue for kernel version `3.10.0-693`, but others
since then have been building but not loading properly with similar
issues. I'm currently using `3.10.0-693.11.6`.

I have had to make a couple small patches to make the media_build work
with Centos7. I have patched out from `backports.txt` the
`v4.14_compiler_h.patch` and `add v3.16_wait_on_bit.patch` lines as
the kernel appears to have fixed these issues. I have also removed the
`__LINUX_COMPILER_TYPES_H` from `compiler-gcc.h` as it was causing an
issue as well.

Any ideas on how to solve this or any troubleshooting tips?

Zach Werner (wernerz)

Re: [PATCH v7 0/6] V4L2 Explicit Synchronization

[Gustavo Padovan]

2018-01-10 Nicolas Dufresne :

> Le mercredi 10 janvier 2018 à 14:07 -0200, Gustavo Padovan a écrit :
> > v7 bring a fix for a crash when not using fences and a uAPI fix.
> > I've done a bit more of testing on it and also measured some
> > performance. On a intel laptop a DRM<->V4L2 pipeline with fences is
> > runnning twice as faster than the same pipeline with no fences.
> 
> What does it mean twice faster here ?

That capture then display on the screen for a given number of frames was
completing in about half of the time when using fences and passing then
to the other driver right away when they are received.

Gustavo

Re: [PATCH 1/1] media: entity: Add a nop variant of media_entity_cleanupr

[Arnd Bergmann]

On Tue, Jan 9, 2018 at 11:31 PM, Sakari Ailus
 wrote:

>> depends on VIDEO_V4L2_SUBDEV_API
>> ---help---
>>   This is a driver for the DW9714 camera lens voice coil.
>> @@ -636,7 +636,6 @@ config VIDEO_OV5670
>> tristate "OmniVision OV5670 sensor support"
>> depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
>> depends on MEDIA_CAMERA_SUPPORT
>> -   depends on MEDIA_CONTROLLER
>
> ov5670 does depend on MC at least right now. I guess it might not take much
> to make it optional. But it's more than this patch. :-)
>
>> select V4L2_FWNODE
>> ---help---
>>   This is a Video4Linux2 sensor-level driver for the OmniVision
>> @@ -667,7 +666,7 @@ config VIDEO_OV7670
>>
>>  config VIDEO_OV7740
>> tristate "OmniVision OV7740 sensor support"
>> -   depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
>> +   depends on I2C && VIDEO_V4L2
>
> Hmm. In here the ov7740 driver doesn't seem to depend on MC.

Right, this was on top of the earlier patch I sent that you rejected ;-)

>> depends on MEDIA_CAMERA_SUPPORT
>> ---help---
>>   This is a Video4Linux2 sensor-level driver for the OmniVision
>> @@ -815,7 +814,7 @@ comment "Flash devices"
>>
>>  config VIDEO_ADP1653
>> tristate "ADP1653 flash support"
>> -   depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
>> +   depends on I2C && VIDEO_V4L2
>> depends on MEDIA_CAMERA_SUPPORT
>> ---help---
>>   This is a driver for the ADP1653 flash controller. It is used for
>> @@ -823,7 +822,7 @@ config VIDEO_ADP1653
>>
>>  config VIDEO_LM3560
>> tristate "LM3560 dual flash driver support"
>> -   depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
>> +   depends on I2C && VIDEO_V4L2
>> depends on MEDIA_CAMERA_SUPPORT
>> select REGMAP_I2C
>> ---help---

Those two also failed to build

>> @@ -832,7 +831,7 @@ config VIDEO_LM3560
>>
>>  config VIDEO_LM3646
>> tristate "LM3646 dual flash driver support"
>> -   depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
>> +   depends on I2C && VIDEO_V4L2
>> depends on MEDIA_CAMERA_SUPPORT
>> select REGMAP_I2C
>> ---help---
>
> These also call media_entity_pads_init() unconditionally.
>
> How was this tested? :-)

Not before I sent it, what I meant is that I'd give it a try, blindly applying
the patch to my randconfig build tree to see what breaks.

The result after a day worth of randconfig builds is this one, which
basically matches what you concluded already:

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 03cf3a1a1e06..5d465221fbfa 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -310,14 +310,14 @@ config VIDEO_ML86V7667

 config VIDEO_AD5820
tristate "AD5820 lens voice coil support"
-   depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
+   depends on I2C && VIDEO_V4L2
---help---
  This is a driver for the AD5820 camera lens voice coil.
  It is used for example in Nokia N900 (RX-51).

 config VIDEO_DW9714
tristate "DW9714 lens voice coil support"
-   depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
+   depends on I2C && VIDEO_V4L2
depends on VIDEO_V4L2_SUBDEV_API
---help---
  This is a driver for the DW9714 camera lens voice coil.
@@ -636,7 +636,6 @@ config VIDEO_OV5670
tristate "OmniVision OV5670 sensor support"
depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
depends on MEDIA_CAMERA_SUPPORT
-   depends on MEDIA_CONTROLLER
select V4L2_FWNODE
---help---
  This is a Video4Linux2 sensor-level driver for the OmniVision
@@ -667,7 +666,7 @@ config VIDEO_OV7670

 config VIDEO_OV7740
tristate "OmniVision OV7740 sensor support"
-   depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
+   depends on I2C && VIDEO_V4L2
depends on MEDIA_CAMERA_SUPPORT
---help---
  This is a Video4Linux2 sensor-level driver for the OmniVision
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index d7a669058b5e..3f34a1126bd1 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -636,6 +636,11 @@ int media_entity_pads_init(struct media_entity
*entity, u16 num_pads,
  */
 static inline void media_entity_cleanup(struct media_entity *entity) {};

+#ifndef CONFIG_MEDIA_CONTROLLER
+#define media_entity_pads_init(e, n, p) 0
+#define media_entity_cleanup(e) do { } while (0)
+#endif
+
 /**
  * media_create_pad_link() - creates a link between two entities.
  *

I'll just drop that patch then from my build tree.

 Arnd

Re: [PATCH v7 0/6] V4L2 Explicit Synchronization

[Nicolas Dufresne]

Le mercredi 10 janvier 2018 à 14:07 -0200, Gustavo Padovan a écrit :
> v7 bring a fix for a crash when not using fences and a uAPI fix.
> I've done a bit more of testing on it and also measured some
> performance. On a intel laptop a DRM<->V4L2 pipeline with fences is
> runnning twice as faster than the same pipeline with no fences.

What does it mean twice faster here ?

Nicolas

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

[PATCH] media: v4l2-core: v4l2-mc: Add SPDX license identifier

[Shuah Khan]

Replace GPL license statement with SPDX GPL-2.0 license identifier.

Signed-off-by: Shuah Khan 
---
 drivers/media/v4l2-core/v4l2-mc.c | 11 +--
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-mc.c 
b/drivers/media/v4l2-core/v4l2-mc.c
index 303980b71aae..1297132acd4e 100644
--- a/drivers/media/v4l2-core/v4l2-mc.c
+++ b/drivers/media/v4l2-core/v4l2-mc.c
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: GPL-2.0 */
 /*
  * Media Controller ancillary functions
  *
@@ -5,16 +6,6 @@
  * Copyright (C) 2016 Shuah Khan 
  * Copyright (C) 2006-2010 Nokia Corporation
  * Copyright (c) 2016 Intel Corporation.
- *
- *  This program is free software; you can redistribute it and/or modify
- *  it under the terms of the GNU General Public License as published by
- *  the Free Software Foundation; either version 2 of the License, or
- *  (at your option) any later version.
- *
- *  This program is distributed in the hope that it will be useful,
- *  but WITHOUT ANY WARRANTY; without even the implied warranty of
- *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- *  GNU General Public License for more details.
  */
 
 #include 
-- 
2.14.1

[PATCH v7 6/6] [media] v4l: Document explicit synchronization behavior

[Gustavo Padovan]

From: Gustavo Padovan 

Add section to VIDIOC_QBUF about it

v5:
- Remove V4L2_CAP_ORDERED
- Add doc about V4L2_FMT_FLAG_UNORDERED

v4:
- Document ordering behavior for in-fences
- Document V4L2_CAP_ORDERED capability
- Remove doc about OUT_FENCE event
- Document immediate return of out-fence in QBUF

v3:
- make the out_fence refer to the current buffer (Hans)
- Note what happens when the IN_FENCE is not set (Hans)

v2:
- mention that fences are files (Hans)
- rework for the new API

Signed-off-by: Gustavo Padovan 
---
 Documentation/media/uapi/v4l/vidioc-qbuf.rst | 47 +++-
 Documentation/media/uapi/v4l/vidioc-querybuf.rst |  9 -
 2 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/Documentation/media/uapi/v4l/vidioc-qbuf.rst 
b/Documentation/media/uapi/v4l/vidioc-qbuf.rst
index 9e448a4aa3aa..8809397fb110 100644
--- a/Documentation/media/uapi/v4l/vidioc-qbuf.rst
+++ b/Documentation/media/uapi/v4l/vidioc-qbuf.rst
@@ -54,7 +54,7 @@ When the buffer is intended for output (``type`` is
 or ``V4L2_BUF_TYPE_VBI_OUTPUT``) applications must also initialize the
 ``bytesused``, ``field`` and ``timestamp`` fields, see :ref:`buffer`
 for details. Applications must also set ``flags`` to 0. The
-``reserved2`` and ``reserved`` fields must be set to 0. When using the
+``reserved`` field must be set to 0. When using the
 :ref:`multi-planar API `, the ``m.planes`` field must
 contain a userspace pointer to a filled-in array of struct
 :c:type:`v4l2_plane` and the ``length`` field must be set
@@ -118,6 +118,51 @@ immediately with an ``EAGAIN`` error code when no buffer 
is available.
 The struct :c:type:`v4l2_buffer` structure is specified in
 :ref:`buffer`.
 
+Explicit Synchronization
+
+
+Explicit Synchronization allows us to control the synchronization of
+shared buffers from userspace by passing fences to the kernel and/or
+receiving them from it. Fences passed to the kernel are named in-fences and
+the kernel should wait on them to signal before using the buffer, i.e., 
queueing
+it to the driver. On the other side, the kernel can create out-fences for the
+buffers it queues to the drivers. Out-fences signal when the driver is
+finished with buffer, i.e., the buffer is ready. The fences are represented
+as a file and passed as a file descriptor to userspace.
+
+The in-fences are communicated to the kernel at the ``VIDIOC_QBUF`` ioctl
+using the ``V4L2_BUF_FLAG_IN_FENCE`` buffer flag and the `fence_fd` field. If
+an in-fence needs to be passed to the kernel, `fence_fd` should be set to the
+fence file descriptor number and the ``V4L2_BUF_FLAG_IN_FENCE`` should be set
+as well. Setting one but not the other will cause ``VIDIOC_QBUF`` to return
+with error. The fence_fd field will be ignored if the
+``V4L2_BUF_FLAG_IN_FENCE`` is not set.
+
+The videobuf2-core will guarantee that all buffers queued with in-fence will
+be queued to the drivers in the same order. Fence may signal out of order, so
+this guarantee at videobuf2 is necessary to not change ordering.
+
+If the in-fence signals with an error the videobuf2 won't queue the buffer to
+the driver, instead it will flag it with an error. And then wait for the
+previous buffer to complete before asking userspace dequeue the buffer with
+error - to make sure we deliver the buffers back in the correct order.
+
+To get an out-fence back from V4L2 the ``V4L2_BUF_FLAG_OUT_FENCE`` flag should
+be set to ask for a fence to be attached to the buffer. The out-fence fd is
+sent to userspace as a ``VIDIOC_QBUF`` return argument on the `fence_fd` field.
+
+Note the the same `fence_fd` field is used for both sending the in-fence as
+input argument to receive the out-fence as a return argument.
+
+At streamoff the out-fences will either signal normally if the driver waits
+for the operations on the buffers to finish or signal with an error if the
+driver cancels the pending operations. Buffers with in-fences won't be queued
+to the driver if their fences signal. It will be cleaned up.
+
+The ``V4L2_FMT_FLAG_UNORDERED`` flag in ``VIDIOC_ENUM_FMT`` tells userspace
+that the current buffer queues is able to keep the ordering of buffers, i.e.,
+the dequeing of buffers will happen at the same order we queue them, with no
+reordering by the driver.
 
 Return Value
 
diff --git a/Documentation/media/uapi/v4l/vidioc-querybuf.rst 
b/Documentation/media/uapi/v4l/vidioc-querybuf.rst
index dd54747fabc9..df964c4d916b 100644
--- a/Documentation/media/uapi/v4l/vidioc-querybuf.rst
+++ b/Documentation/media/uapi/v4l/vidioc-querybuf.rst
@@ -44,7 +44,7 @@ and the ``index`` field. Valid index numbers range from zero 
to the
 number of buffers allocated with
 :ref:`VIDIOC_REQBUFS` (struct
 :c:type:`v4l2_requestbuffers` ``count``) minus
-one. The ``reserved`` and ``reserved2`` fields must be 

[PATCH v7 1/6] [media] vb2: add is_unordered callback for drivers

[Gustavo Padovan]

From: Gustavo Padovan 

Explicit synchronization benefits a lot from ordered queues, they fit
better in a pipeline with DRM for example so create a opt-in way for
drivers notify videobuf2 that the queue is unordered.

Drivers don't need implement it if the queue is ordered.

Signed-off-by: Gustavo Padovan 
---
 include/media/videobuf2-core.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index f3ee4c7c2fb3..583cdc06de79 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -370,6 +370,9 @@ struct vb2_buffer {
  * callback by calling vb2_buffer_done() with either
  * %VB2_BUF_STATE_DONE or %VB2_BUF_STATE_ERROR; may use
  * vb2_wait_for_all_buffers() function
+ * @is_unordered:  tell if the queue format is unordered. The default is
+ * assumed to be ordered and this function only needs to
+ * be implemented for unordered queues.
  * @buf_queue: passes buffer vb to the driver; driver may start
  * hardware operation on this buffer; driver should give
  * the buffer back by calling vb2_buffer_done() function;
@@ -393,6 +396,7 @@ struct vb2_ops {
 
int (*start_streaming)(struct vb2_queue *q, unsigned int count);
void (*stop_streaming)(struct vb2_queue *q);
+   int (*is_unordered)(struct vb2_queue *q);
 
void (*buf_queue)(struct vb2_buffer *vb);
 };
@@ -566,6 +570,7 @@ struct vb2_queue {
u32 cnt_wait_finish;
u32 cnt_start_streaming;
u32 cnt_stop_streaming;
+   u32 cnt_is_unordered;
 #endif
 };
 
-- 
2.14.3

[PATCH v7 2/6] [media] v4l: add 'unordered' flag to format description ioctl

[Gustavo Padovan]

From: Gustavo Padovan 

For explicit synchronization it important for userspace to know if the
format being used by the driver can deliver the buffers back to userspace
in the same order they were queued with QBUF.

Ordered streams fits nicely in a pipeline with DRM for example, where
ordered buffer are expected.

Signed-off-by: Gustavo Padovan 
---
 Documentation/media/uapi/v4l/vidioc-enum-fmt.rst | 3 +++
 include/uapi/linux/videodev2.h   | 1 +
 2 files changed, 4 insertions(+)

diff --git a/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst 
b/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
index 019c513df217..368115f44fc0 100644
--- a/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
+++ b/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
@@ -116,6 +116,9 @@ one until ``EINVAL`` is returned.
   - This format is not native to the device but emulated through
software (usually libv4l2), where possible try to use a native
format instead for better performance.
+* - ``V4L2_FMT_FLAG_UNORDERED``
+  - 0x0004
+  - This is a format that doesn't guarantee timely order of frames.
 
 
 Return Value
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 982718965180..58894cfe9479 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -716,6 +716,7 @@ struct v4l2_fmtdesc {
 
 #define V4L2_FMT_FLAG_COMPRESSED 0x0001
 #define V4L2_FMT_FLAG_EMULATED   0x0002
+#define V4L2_FMT_FLAG_UNORDERED  0x0004
 
/* Frame Size and frame rate enumeration */
 /*
-- 
2.14.3

[PATCH v7 5/6] [media] vb2: add out-fence support to QBUF

[Gustavo Padovan]

From: Gustavo Padovan 

If V4L2_BUF_FLAG_OUT_FENCE flag is present on the QBUF call we create
an out_fence and send its fd to userspace on the fence_fd field as a
return arg for the QBUF call.

The fence is signaled on buffer_done(), when the job on the buffer is
finished.

v8:
- return 0 as fence_fd if OUT_FENCE flag not used (Mauro)
- fix crash when checking not using fences in vb2_buffer_done()

v7:
- merge patch that add the infrastructure to out-fences into
this one (Alex Courbot)
- Do not install the fd if there is no fence. (Alex Courbot)
- do not report error on requeueing, just WARN_ON_ONCE() (Hans)

v6
- get rid of the V4L2_EVENT_OUT_FENCE event. We always keep the
ordering in vb2 for queueing in the driver, so the event is not
necessary anymore and the out_fence_fd is sent back to userspace
on QBUF call return arg
- do not allow requeueing with out-fences, instead mark the buffer
with an error and wake up to userspace.
- send the out_fence_fd back to userspace on the fence_fd field

v5:
- delay fd_install to DQ_EVENT (Hans)
- if queue is fully ordered send OUT_FENCE event right away
(Brian)
- rename 'q->ordered' to 'q->ordered_in_driver'
- merge change to implement OUT_FENCE event here

v4:
- return the out_fence_fd in the BUF_QUEUED event(Hans)

v3: - add WARN_ON_ONCE(q->ordered) on requeueing (Hans)
- set the OUT_FENCE flag if there is a fence pending (Hans)
- call fd_install() after vb2_core_qbuf() (Hans)
- clean up fence if vb2_core_qbuf() fails (Hans)
- add list to store sync_file and fence for the next queued buffer

v2: check if the queue is ordered.

Signed-off-by: Gustavo Padovan 
---
 drivers/media/common/videobuf/videobuf2-core.c | 101 +++--
 drivers/media/common/videobuf/videobuf2-v4l2.c |  28 ++-
 include/media/videobuf2-core.h |  22 ++
 3 files changed, 140 insertions(+), 11 deletions(-)

diff --git a/drivers/media/common/videobuf/videobuf2-core.c 
b/drivers/media/common/videobuf/videobuf2-core.c
index 777e3a2bc746..1f30d9efb7c8 100644
--- a/drivers/media/common/videobuf/videobuf2-core.c
+++ b/drivers/media/common/videobuf/videobuf2-core.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -357,6 +358,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum 
vb2_memory memory,
vb->planes[plane].length = plane_sizes[plane];
vb->planes[plane].min_length = plane_sizes[plane];
}
+   vb->out_fence_fd = -1;
q->bufs[vb->index] = vb;
 
/* Allocate video buffer memory for the MMAP type */
@@ -939,10 +941,22 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum 
vb2_buffer_state state)
case VB2_BUF_STATE_QUEUED:
break;
case VB2_BUF_STATE_REQUEUEING:
+   /* Requeuing with explicit synchronization, spit warning */
+   WARN_ON_ONCE(vb->out_fence);
+
if (q->start_streaming_called)
__enqueue_in_driver(vb);
-   return;
+   break;
default:
+   if (vb->out_fence) {
+   if (state == VB2_BUF_STATE_ERROR)
+   dma_fence_set_error(vb->out_fence, -EFAULT);
+   dma_fence_signal(vb->out_fence);
+   dma_fence_put(vb->out_fence);
+   vb->out_fence = NULL;
+   vb->out_fence_fd = -1;
+   }
+
/* Inform any processes that may be waiting for buffers */
wake_up(>done_wq);
break;
@@ -1341,6 +1355,65 @@ int vb2_core_prepare_buf(struct vb2_queue *q, unsigned 
int index, void *pb)
 }
 EXPORT_SYMBOL_GPL(vb2_core_prepare_buf);
 
+static inline const char *vb2_fence_get_driver_name(struct dma_fence *fence)
+{
+   return "vb2_fence";
+}
+
+static inline const char *vb2_fence_get_timeline_name(struct dma_fence *fence)
+{
+   return "vb2_fence_timeline";
+}
+
+static inline bool vb2_fence_enable_signaling(struct dma_fence *fence)
+{
+   return true;
+}
+
+static const struct dma_fence_ops vb2_fence_ops = {
+   .get_driver_name = vb2_fence_get_driver_name,
+   .get_timeline_name = vb2_fence_get_timeline_name,
+   .enable_signaling = vb2_fence_enable_signaling,
+   .wait = dma_fence_default_wait,
+};
+
+int vb2_setup_out_fence(struct vb2_queue *q, unsigned int index)
+{
+   struct vb2_buffer *vb;
+   u64 context;
+
+   vb = q->bufs[index];
+
+   vb->out_fence_fd = get_unused_fd_flags(O_CLOEXEC);
+
+   if (call_qop(q, is_unordered, q))
+   context = dma_fence_context_alloc(1);
+   else
+   context 

[PATCH v7 3/6] [media] vb2: add explicit fence user API

[Gustavo Padovan]

From: Gustavo Padovan 

Turn the reserved2 field into fence_fd that we will use to send
an in-fence to the kernel and return an out-fence from the kernel to
userspace.

Two new flags were added, V4L2_BUF_FLAG_IN_FENCE, that should be used
when sending a fence to the kernel to be waited on, and
V4L2_BUF_FLAG_OUT_FENCE, to ask the kernel to give back an out-fence.

v5:
- keep using reserved2 field for cpia2
- set fence_fd to 0 for now, for compat with userspace(Mauro)

v4:
- make it a union with reserved2 and fence_fd (Hans Verkuil)

v3:
- make the out_fence refer to the current buffer (Hans Verkuil)

v2: add documentation

Signed-off-by: Gustavo Padovan 
---
 Documentation/media/uapi/v4l/buffer.rst| 15 +++
 drivers/media/common/videobuf/videobuf2-v4l2.c |  2 +-
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c  |  4 ++--
 include/uapi/linux/videodev2.h |  7 ++-
 4 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/Documentation/media/uapi/v4l/buffer.rst 
b/Documentation/media/uapi/v4l/buffer.rst
index ae6ee73f151c..eeefbd2547e7 100644
--- a/Documentation/media/uapi/v4l/buffer.rst
+++ b/Documentation/media/uapi/v4l/buffer.rst
@@ -648,6 +648,21 @@ Buffer Flags
   - Start Of Exposure. The buffer timestamp has been taken when the
exposure of the frame has begun. This is only valid for the
``V4L2_BUF_TYPE_VIDEO_CAPTURE`` buffer type.
+* .. _`V4L2-BUF-FLAG-IN-FENCE`:
+
+  - ``V4L2_BUF_FLAG_IN_FENCE``
+  - 0x0020
+  - Ask V4L2 to wait on fence passed in ``fence_fd`` field. The buffer
+   won't be queued to the driver until the fence signals.
+
+* .. _`V4L2-BUF-FLAG-OUT-FENCE`:
+
+  - ``V4L2_BUF_FLAG_OUT_FENCE``
+  - 0x0040
+  - Request a fence to be attached to the buffer. The ``fence_fd``
+   field on
+   :ref:`VIDIOC_QBUF` is used as a return argument to send the out-fence
+   fd to userspace.
 
 
 
diff --git a/drivers/media/common/videobuf/videobuf2-v4l2.c 
b/drivers/media/common/videobuf/videobuf2-v4l2.c
index fac3cd6f901d..d838524a459e 100644
--- a/drivers/media/common/videobuf/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf/videobuf2-v4l2.c
@@ -203,7 +203,7 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb, void 
*pb)
b->timestamp = ns_to_timeval(vb->timestamp);
b->timecode = vbuf->timecode;
b->sequence = vbuf->sequence;
-   b->reserved2 = 0;
+   b->fence_fd = 0;
b->reserved = 0;
 
if (q->is_multiplanar) {
diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c 
b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
index e48d59046086..a11a0a2bed47 100644
--- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
+++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
@@ -370,7 +370,7 @@ struct v4l2_buffer32 {
__s32   fd;
} m;
__u32   length;
-   __u32   reserved2;
+   __s32   fence_fd;
__u32   reserved;
 };
 
@@ -533,7 +533,7 @@ static int put_v4l2_buffer32(struct v4l2_buffer *kp, struct 
v4l2_buffer32 __user
put_user(kp->timestamp.tv_usec, >timestamp.tv_usec) ||
copy_to_user(>timecode, >timecode, sizeof(struct 
v4l2_timecode)) ||
put_user(kp->sequence, >sequence) ||
-   put_user(kp->reserved2, >reserved2) ||
+   put_user(kp->fence_fd, >fence_fd) ||
put_user(kp->reserved, >reserved) ||
put_user(kp->length, >length))
return -EFAULT;
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 58894cfe9479..2d424aebdd1e 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -933,7 +933,10 @@ struct v4l2_buffer {
__s32   fd;
} m;
__u32   length;
-   __u32   reserved2;
+   union {
+   __s32   fence_fd;
+   __u32   reserved2;
+   };
__u32   reserved;
 };
 
@@ -970,6 +973,8 @@ struct v4l2_buffer {
 #define V4L2_BUF_FLAG_TSTAMP_SRC_SOE   0x0001
 /* mem2mem encoder/decoder */
 #define V4L2_BUF_FLAG_LAST 0x0010
+#define V4L2_BUF_FLAG_IN_FENCE 0x0020
+#define V4L2_BUF_FLAG_OUT_FENCE0x0040
 
 /**
  * struct v4l2_exportbuffer - export of video buffer as DMABUF file descriptor
-- 
2.14.3

[PATCH v7 4/6] [media] vb2: add in-fence support to QBUF

[Gustavo Padovan]

From: Gustavo Padovan 

Receive in-fence from userspace and add support for waiting on them
before queueing the buffer to the driver. Buffers can't be queued to the
driver before its fences signal. And a buffer can't be queue to the driver
out of the order they were queued from userspace. That means that even if
it fence signal it must wait all other buffers, ahead of it in the queue,
to signal first.

If the fence for some buffer fails we do not queue it to the driver,
instead we mark it as error and wait until the previous buffer is done
to notify userspace of the error. We wait here to deliver the buffers back
to userspace in order.

v8: - improve comments about fences with errors

v7:
- get rid of the fence array stuff for ordering and just use
get_num_buffers_ready() (Hans)
- fix issue of queuing the buffer twice (Hans)
- avoid the dma_fence_wait() in core_qbuf() (Alex)
- merge preparation commit in

v6:
- With fences always keep the order userspace queues the buffers.
- Protect in_fence manipulation with a lock (Brian Starkey)
- check if fences have the same context before adding a fence array
- Fix last_fence ref unbalance in __set_in_fence() (Brian Starkey)
- Clean up fence if __set_in_fence() fails (Brian Starkey)
- treat -EINVAL from dma_fence_add_callback() (Brian Starkey)

v5: - use fence_array to keep buffers ordered in vb2 core when
needed (Brian Starkey)
- keep backward compat on the reserved2 field (Brian Starkey)
- protect fence callback removal with lock (Brian Starkey)

v4:
- Add a comment about dma_fence_add_callback() not returning a
error (Hans)
- Call dma_fence_put(vb->in_fence) if fence signaled (Hans)
- select SYNC_FILE under config VIDEOBUF2_CORE (Hans)
- Move dma_fence_is_signaled() check to __enqueue_in_driver() (Hans)
- Remove list_for_each_entry() in __vb2_core_qbuf() (Hans)
-  Remove if (vb->state != VB2_BUF_STATE_QUEUED) from
vb2_start_streaming() (Hans)
- set IN_FENCE flags on __fill_v4l2_buffer (Hans)
- Queue buffers to the driver as soon as they are ready (Hans)
- call fill_user_buffer() after queuing the buffer (Hans)
- add err: label to clean up fence
- add dma_fence_wait() before calling vb2_start_streaming()

v3: - document fence parameter
- remove ternary if at vb2_qbuf() return (Mauro)
- do not change if conditions behaviour (Mauro)

v2:
- fix vb2_queue_or_prepare_buf() ret check
- remove check for VB2_MEMORY_DMABUF only (Javier)
- check num of ready buffers to start streaming
- when queueing, start from the first ready buffer
- handle queue cancel

Signed-off-by: Gustavo Padovan 
---
 drivers/media/common/videobuf/videobuf2-core.c | 166 ++---
 drivers/media/common/videobuf/videobuf2-v4l2.c |  29 -
 drivers/media/v4l2-core/Kconfig|  33 +
 include/media/videobuf2-core.h |  14 ++-
 4 files changed, 221 insertions(+), 21 deletions(-)

diff --git a/drivers/media/common/videobuf/videobuf2-core.c 
b/drivers/media/common/videobuf/videobuf2-core.c
index f7109f827f6e..777e3a2bc746 100644
--- a/drivers/media/common/videobuf/videobuf2-core.c
+++ b/drivers/media/common/videobuf/videobuf2-core.c
@@ -352,6 +352,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum 
vb2_memory memory,
vb->index = q->num_buffers + buffer;
vb->type = q->type;
vb->memory = memory;
+   spin_lock_init(>fence_cb_lock);
for (plane = 0; plane < num_planes; ++plane) {
vb->planes[plane].length = plane_sizes[plane];
vb->planes[plane].min_length = plane_sizes[plane];
@@ -936,7 +937,7 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum 
vb2_buffer_state state)
 
switch (state) {
case VB2_BUF_STATE_QUEUED:
-   return;
+   break;
case VB2_BUF_STATE_REQUEUEING:
if (q->start_streaming_called)
__enqueue_in_driver(vb);
@@ -946,6 +947,19 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum 
vb2_buffer_state state)
wake_up(>done_wq);
break;
}
+
+   /*
+* The check below verifies if there is a buffer in queue with an
+* error state. They are added to queue in the error state when
+* their in-fence fails to signal.
+* To not mess with buffer ordering we wait until the previous buffer
+* is done to mark the buffer in the error state as done and notify
+* userspace. So everytime a buffer is done we check the next one for
+* VB2_BUF_STATE_ERROR.
+*/
+   vb = list_next_entry(vb, queued_entry);
+   

[PATCH v7 0/6] V4L2 Explicit Synchronization

[Gustavo Padovan]

From: Gustavo Padovan 

Hi,

v7 bring a fix for a crash when not using fences and a uAPI fix.
I've done a bit more of testing on it and also measured some
performance. On a intel laptop a DRM<->V4L2 pipeline with fences is
runnning twice as faster than the same pipeline with no fences.

For more details on how fences work refer to patch 6 in this series.

The test tools I've been using are:
https://gitlab.collabora.com/padovan/drm-v4l2-test
https://gitlab.collabora.com/padovan/v4l2-fences-test

Please review,

Gustavo

Gustavo Padovan (6):
  [media] vb2: add is_unordered callback for drivers
  [media] v4l: add 'unordered' flag to format description ioctl
  [media] vb2: add explicit fence user API
  [media] vb2: add in-fence support to QBUF
  [media] vb2: add out-fence support to QBUF
  [media] v4l: Document explicit synchronization behavior

 Documentation/media/uapi/v4l/buffer.rst  |  15 ++
 Documentation/media/uapi/v4l/vidioc-enum-fmt.rst |   3 +
 Documentation/media/uapi/v4l/vidioc-qbuf.rst |  47 -
 Documentation/media/uapi/v4l/vidioc-querybuf.rst |   9 +-
 drivers/media/common/videobuf/videobuf2-core.c   | 253 +--
 drivers/media/common/videobuf/videobuf2-v4l2.c   |  51 -
 drivers/media/v4l2-core/Kconfig  |  33 +++
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c|   4 +-
 include/media/videobuf2-core.h   |  41 +++-
 include/uapi/linux/videodev2.h   |   8 +-
 10 files changed, 437 insertions(+), 27 deletions(-)

-- 
2.14.3

Re: [PATCH v5 0/5] Add OV5640 parallel interface and RGB565/YUYV support

[Hugues FRUCHET]

Good news Maxime !

Have you seen that you can adapt the polarities through devicetree ?

+   /* Parallel bus endpoint */
+   ov5640_to_parallel: endpoint {
[...]
+   hsync-active = <0>;
+   vsync-active = <0>;
+   pclk-sample = <1>;
+   };

Doing so you can adapt to your SoC/board setup easily.

If you don't put those lines in devicetree, the ov5640 default init 
sequence is used which set the polarity as defined in below comment:
ov5640_set_stream_dvp()
[...]
+* Control lines polarity can be configured through
+* devicetree endpoint control lines properties.
+* If no endpoint control lines properties are set,
+* polarity will be as below:
+* - VSYNC: active high
+* - HREF:  active low
+* - PCLK:  active low
+*/
[...]


Best regards,
Hugues.

On 01/10/2018 04:37 PM, Maxime Ripard wrote:
> Hi Hugues,
> 
> On Mon, Jan 08, 2018 at 05:13:39PM +, Hugues FRUCHET wrote:
>> I'm using a ST board with OV5640 wired in parallel bus output in order
>> to interface to my STM32 DCMI parallel interface.
>> Perhaps could you describe your setup so I could help on understanding
>> the problem on your side. From my past experience with this sensor
>> module, you can first check hsync/vsync polarities, the datasheet is
>> buggy on VSYNC polarity as documented in patch 4/5.
> 
> It turns out that it was indeed a polarity issue.
> 
> It looks like that in order to operate properly, I need to setup the
> opposite polarity on HSYNC and VSYNC on the interface. I looked at the
> signals under a scope, and VSYNC is obviously inversed as you
> described. HSYNC, I'm not so sure since the HBLANK period seems very
> long, almost a line.
> 
> Since VSYNC at least looks correct, I'd be inclined to think that the
> polarity is inversed on at least the SoC I'm using it on.
> 
> Yong, did you test the V3S CSI driver with a parallel interface? With
> what sensor driver? Have you found some polarities issues like this?
> 
> Thanks!
> Maxime
> 

Re: [PATCH v5 0/5] Add OV5640 parallel interface and RGB565/YUYV support

[Maxime Ripard]

Hi Hugues,

On Mon, Jan 08, 2018 at 05:13:39PM +, Hugues FRUCHET wrote:
> I'm using a ST board with OV5640 wired in parallel bus output in order 
> to interface to my STM32 DCMI parallel interface.
> Perhaps could you describe your setup so I could help on understanding 
> the problem on your side. From my past experience with this sensor 
> module, you can first check hsync/vsync polarities, the datasheet is 
> buggy on VSYNC polarity as documented in patch 4/5.

It turns out that it was indeed a polarity issue.

It looks like that in order to operate properly, I need to setup the
opposite polarity on HSYNC and VSYNC on the interface. I looked at the
signals under a scope, and VSYNC is obviously inversed as you
described. HSYNC, I'm not so sure since the HBLANK period seems very
long, almost a line.

Since VSYNC at least looks correct, I'd be inclined to think that the
polarity is inversed on at least the SoC I'm using it on.

Yong, did you test the V3S CSI driver with a parallel interface? With
what sensor driver? Have you found some polarities issues like this?

Thanks!
Maxime

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


signature.asc
Description: PGP signature

[PATCH] dvb: Save port number and provide sysfs attributes to pass values to udev

[David Howells]

Some devices, such as the DVBSky S952 and T982 cards, are dual port cards
that provide two cx23885 devices on the same PCI device, which means the
attributes available for writing udev rules are exactly the same, apart
from the adapter number.  Unfortunately, the adapter numbers are dependent
on the order in which things are initialised, so this can change over
different releases of the kernel.

The struct cx23885_tsport has a port number available, which is printed
during boot:

[   10.951517] DVBSky T982 port 1 MAC address: 00:17:42:54:09:87
...
[   10.984875] DVBSky T982 port 2 MAC address: 00:17:42:54:09:88

To make it possible to distinguish these in udev, do the following steps:

 (1) Save the port number into struct dvb_adapter.

 (2) Provide sysfs attributes to export port number and also MAC address,
 adapter number and type.  There are other fields that could perhaps be
 exported also.

The new sysfs attributes can be seen from userspace as:

[root@deneb ~]# ls /sys/class/dvb/dvb0.frontend0/
dev  device  dvb_adapter  dvb_mac  dvb_port  dvb_type
power  subsystem  uevent
[root@deneb ~]# cat /sys/class/dvb/dvb0.frontend0/dvb_*
0
00:17:42:54:09:87
0
frontend

They can be used in udev rules:

SUBSYSTEM=="dvb", ATTRS{vendor}=="0x14f1", ATTRS{device}=="0x8852", 
ATTRS{subsystem_device}=="0x0982", ATTR{dvb_mac}=="00:17:42:54:09:87", 
PROGRAM="/bin/sh -c 'K=%k; K=$${K#dvb}; printf dvb/adapter9820/%%s $${K#*.}'", 
SYMLINK+="%c"
SUBSYSTEM=="dvb", ATTRS{vendor}=="0x14f1", ATTRS{device}=="0x8852", 
ATTRS{subsystem_device}=="0x0982", ATTR{dvb_mac}=="00:17:42:54:09:88", 
PROGRAM="/bin/sh -c 'K=%k; K=$${K#dvb}; printf dvb/adapter9821/%%s $${K#*.}'", 
SYMLINK+="%c"

where the match is made with ATTR{dvb_mac} or similar.  The rules above
make symlinks from /dev/dvb/adapter982/* to /dev/dvb/adapterXX/*.

Note that binding the dvb-net device to a network interface and changing it
there does not reflect back into the the dvb_adapter struct and doesn't
change the MAC address here.  This means that a system with two identical
cards in it may need to distinguish them by some other means than MAC
address.

Signed-off-by: David Howells 
---

 drivers/media/dvb-core/dvbdev.c |   46 +++
 drivers/media/dvb-core/dvbdev.h |2 +
 drivers/media/pci/cx23885/cx23885-dvb.c |2 +
 3 files changed, 50 insertions(+)

diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
index 060c60ddfcc3..b3aa5ae3d57f 100644
--- a/drivers/media/dvb-core/dvbdev.c
+++ b/drivers/media/dvb-core/dvbdev.c
@@ -941,6 +941,51 @@ int dvb_usercopy(struct file *file,
return err;
 }
 
+static ssize_t dvb_adapter_show(struct device *dev,
+   struct device_attribute *attr, char *buf)
+{
+   struct dvb_device *dvbdev = dev_get_drvdata(dev);
+
+   return sprintf(buf, "%d\n", dvbdev->adapter->num);
+}
+static DEVICE_ATTR_RO(dvb_adapter);
+
+static ssize_t dvb_mac_show(struct device *dev,
+   struct device_attribute *attr, char *buf)
+{
+   struct dvb_device *dvbdev = dev_get_drvdata(dev);
+
+   return sprintf(buf, "%pM\n", dvbdev->adapter->proposed_mac);
+}
+static DEVICE_ATTR_RO(dvb_mac);
+
+static ssize_t dvb_port_show(struct device *dev,
+struct device_attribute *attr, char *buf)
+{
+   struct dvb_device *dvbdev = dev_get_drvdata(dev);
+
+   return sprintf(buf, "%d\n", dvbdev->adapter->port_num);
+}
+static DEVICE_ATTR_RO(dvb_port);
+
+static ssize_t dvb_type_show(struct device *dev,
+struct device_attribute *attr, char *buf)
+{
+   struct dvb_device *dvbdev = dev_get_drvdata(dev);
+
+   return sprintf(buf, "%s\n", dnames[dvbdev->type]);
+}
+static DEVICE_ATTR_RO(dvb_type);
+
+static struct attribute *dvb_class_attrs[] = {
+   _attr_dvb_adapter.attr,
+   _attr_dvb_mac.attr,
+   _attr_dvb_port.attr,
+   _attr_dvb_type.attr,
+   NULL
+};
+ATTRIBUTE_GROUPS(dvb_class);
+
 static int dvb_uevent(struct device *dev, struct kobj_uevent_env *env)
 {
struct dvb_device *dvbdev = dev_get_drvdata(dev);
@@ -981,6 +1026,7 @@ static int __init init_dvbdev(void)
retval = PTR_ERR(dvb_class);
goto error;
}
+   dvb_class->dev_groups = dvb_class_groups,
dvb_class->dev_uevent = dvb_uevent;
dvb_class->devnode = dvb_devnode;
return 0;
diff --git a/drivers/media/dvb-core/dvbdev.h b/drivers/media/dvb-core/dvbdev.h
index bbc1c20c0529..1d5a170e279a 100644
--- a/drivers/media/dvb-core/dvbdev.h
+++ b/drivers/media/dvb-core/dvbdev.h
@@ -83,6 +83,7 @@ struct dvb_frontend;
  * @device_list:   List with the DVB devices
  * @name:  Name of the adapter
  * @proposed_mac:  proposed MAC address for the adapter
+ * @port_num:  Port 

Re: [Linaro-mm-sig] [PATCH] dma-buf: make returning the exclusive fence optional

[Daniel Vetter]

On Wed, Jan 10, 2018 at 02:46:32PM +0100, Christian König wrote:
> Am 10.01.2018 um 14:21 schrieb Daniel Vetter:
> > On Wed, Jan 10, 2018 at 01:53:41PM +0100, Christian König wrote:
> > > Change reservation_object_get_fences_rcu to make the exclusive fence
> > > pointer optional.
> > > 
> > > If not specified the exclusive fence is put into the fence array as
> > > well.
> > > 
> > > This is helpful for a couple of cases where we need all fences in a
> > > single array.
> > > 
> > > Signed-off-by: Christian König 
> > Seeing the use-case for this would be a lot more interesting ...
> 
> Yeah, sorry the use case is a 20 patches set on amd-gfx.
> 
> Didn't wanted to post all those here as well.

Imo better to spam more lists instead of splitting up discussions ... It's
at least what we tend to do for i915 stuff, and no one seems to complain.
-Daniel

> 
> Christian.
> 
> > -Daniel
> > 
> > > ---
> > >   drivers/dma-buf/reservation.c | 31 ++-
> > >   1 file changed, 22 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
> > > index b759a569b7b8..461afa9febd4 100644
> > > --- a/drivers/dma-buf/reservation.c
> > > +++ b/drivers/dma-buf/reservation.c
> > > @@ -374,8 +374,9 @@ EXPORT_SYMBOL(reservation_object_copy_fences);
> > >* @pshared: the array of shared fence ptrs returned (array is 
> > > krealloc'd to
> > >* the required size, and must be freed by caller)
> > >*
> > > - * RETURNS
> > > - * Zero or -errno
> > > + * Retrieve all fences from the reservation object. If the pointer for 
> > > the
> > > + * exclusive fence is not specified the fence is put into the array of 
> > > the
> > > + * shared fences as well. Returns either zero or -ENOMEM.
> > >*/
> > >   int reservation_object_get_fences_rcu(struct reservation_object *obj,
> > > struct dma_fence **pfence_excl,
> > > @@ -389,8 +390,8 @@ int reservation_object_get_fences_rcu(struct 
> > > reservation_object *obj,
> > >   do {
> > >   struct reservation_object_list *fobj;
> > > - unsigned seq;
> > > - unsigned int i;
> > > + unsigned int i, seq;
> > > + size_t sz = 0;
> > >   shared_count = i = 0;
> > > @@ -402,9 +403,14 @@ int reservation_object_get_fences_rcu(struct 
> > > reservation_object *obj,
> > >   goto unlock;
> > >   fobj = rcu_dereference(obj->fence);
> > > - if (fobj) {
> > > + if (fobj)
> > > + sz += sizeof(*shared) * fobj->shared_max;
> > > +
> > > + if (!pfence_excl && fence_excl)
> > > + sz += sizeof(*shared);
> > > +
> > > + if (sz) {
> > >   struct dma_fence **nshared;
> > > - size_t sz = sizeof(*shared) * fobj->shared_max;
> > >   nshared = krealloc(shared, sz,
> > >  GFP_NOWAIT | __GFP_NOWARN);
> > > @@ -420,13 +426,19 @@ int reservation_object_get_fences_rcu(struct 
> > > reservation_object *obj,
> > >   break;
> > >   }
> > >   shared = nshared;
> > > - shared_count = fobj->shared_count;
> > > -
> > > + shared_count = fobj ? fobj->shared_count : 0;
> > >   for (i = 0; i < shared_count; ++i) {
> > >   shared[i] = 
> > > rcu_dereference(fobj->shared[i]);
> > >   if (!dma_fence_get_rcu(shared[i]))
> > >   break;
> > >   }
> > > +
> > > + if (!pfence_excl && fence_excl) {
> > > + shared[i] = fence_excl;
> > > + fence_excl = NULL;
> > > + ++i;
> > > + ++shared_count;
> > > + }
> > >   }
> > >   if (i != shared_count || read_seqcount_retry(>seq, 
> > > seq)) {
> > > @@ -448,7 +460,8 @@ int reservation_object_get_fences_rcu(struct 
> > > reservation_object *obj,
> > >   *pshared_count = shared_count;
> > >   *pshared = shared;
> > > - *pfence_excl = fence_excl;
> > > + if (pfence_excl)
> > > + *pfence_excl = fence_excl;
> > >   return ret;
> > >   }
> > > -- 
> > > 2.14.1
> > > 
> > > ___
> > > Linaro-mm-sig mailing list
> > > linaro-mm-...@lists.linaro.org
> > > https://lists.linaro.org/mailman/listinfo/linaro-mm-sig
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [Linaro-mm-sig] [PATCH] dma-buf: make returning the exclusive fence optional

[Christian König]

Am 10.01.2018 um 14:21 schrieb Daniel Vetter:

On Wed, Jan 10, 2018 at 01:53:41PM +0100, Christian König wrote:

Change reservation_object_get_fences_rcu to make the exclusive fence
pointer optional.

If not specified the exclusive fence is put into the fence array as
well.

This is helpful for a couple of cases where we need all fences in a
single array.

Signed-off-by: Christian König 

Seeing the use-case for this would be a lot more interesting ...


Yeah, sorry the use case is a 20 patches set on amd-gfx.

Didn't wanted to post all those here as well.

Christian.


-Daniel


---
  drivers/dma-buf/reservation.c | 31 ++-
  1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
index b759a569b7b8..461afa9febd4 100644
--- a/drivers/dma-buf/reservation.c
+++ b/drivers/dma-buf/reservation.c
@@ -374,8 +374,9 @@ EXPORT_SYMBOL(reservation_object_copy_fences);
   * @pshared: the array of shared fence ptrs returned (array is krealloc'd to
   * the required size, and must be freed by caller)
   *
- * RETURNS
- * Zero or -errno
+ * Retrieve all fences from the reservation object. If the pointer for the
+ * exclusive fence is not specified the fence is put into the array of the
+ * shared fences as well. Returns either zero or -ENOMEM.
   */
  int reservation_object_get_fences_rcu(struct reservation_object *obj,
  struct dma_fence **pfence_excl,
@@ -389,8 +390,8 @@ int reservation_object_get_fences_rcu(struct 
reservation_object *obj,
  
  	do {

struct reservation_object_list *fobj;
-   unsigned seq;
-   unsigned int i;
+   unsigned int i, seq;
+   size_t sz = 0;
  
  		shared_count = i = 0;
  
@@ -402,9 +403,14 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj,

goto unlock;
  
  		fobj = rcu_dereference(obj->fence);

-   if (fobj) {
+   if (fobj)
+   sz += sizeof(*shared) * fobj->shared_max;
+
+   if (!pfence_excl && fence_excl)
+   sz += sizeof(*shared);
+
+   if (sz) {
struct dma_fence **nshared;
-   size_t sz = sizeof(*shared) * fobj->shared_max;
  
  			nshared = krealloc(shared, sz,

   GFP_NOWAIT | __GFP_NOWARN);
@@ -420,13 +426,19 @@ int reservation_object_get_fences_rcu(struct 
reservation_object *obj,
break;
}
shared = nshared;
-   shared_count = fobj->shared_count;
-
+   shared_count = fobj ? fobj->shared_count : 0;
for (i = 0; i < shared_count; ++i) {
shared[i] = rcu_dereference(fobj->shared[i]);
if (!dma_fence_get_rcu(shared[i]))
break;
}
+
+   if (!pfence_excl && fence_excl) {
+   shared[i] = fence_excl;
+   fence_excl = NULL;
+   ++i;
+   ++shared_count;
+   }
}
  
  		if (i != shared_count || read_seqcount_retry(>seq, seq)) {

@@ -448,7 +460,8 @@ int reservation_object_get_fences_rcu(struct 
reservation_object *obj,
  
  	*pshared_count = shared_count;

*pshared = shared;
-   *pfence_excl = fence_excl;
+   if (pfence_excl)
+   *pfence_excl = fence_excl;
  
  	return ret;

  }
--
2.14.1

___
Linaro-mm-sig mailing list
linaro-mm-...@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/linaro-mm-sig

Re: [Linaro-mm-sig] [PATCH] dma-buf: make returning the exclusive fence optional

[Daniel Vetter]

On Wed, Jan 10, 2018 at 01:53:41PM +0100, Christian König wrote:
> Change reservation_object_get_fences_rcu to make the exclusive fence
> pointer optional.
> 
> If not specified the exclusive fence is put into the fence array as
> well.
> 
> This is helpful for a couple of cases where we need all fences in a
> single array.
> 
> Signed-off-by: Christian König 

Seeing the use-case for this would be a lot more interesting ...
-Daniel

> ---
>  drivers/dma-buf/reservation.c | 31 ++-
>  1 file changed, 22 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
> index b759a569b7b8..461afa9febd4 100644
> --- a/drivers/dma-buf/reservation.c
> +++ b/drivers/dma-buf/reservation.c
> @@ -374,8 +374,9 @@ EXPORT_SYMBOL(reservation_object_copy_fences);
>   * @pshared: the array of shared fence ptrs returned (array is krealloc'd to
>   * the required size, and must be freed by caller)
>   *
> - * RETURNS
> - * Zero or -errno
> + * Retrieve all fences from the reservation object. If the pointer for the
> + * exclusive fence is not specified the fence is put into the array of the
> + * shared fences as well. Returns either zero or -ENOMEM.
>   */
>  int reservation_object_get_fences_rcu(struct reservation_object *obj,
> struct dma_fence **pfence_excl,
> @@ -389,8 +390,8 @@ int reservation_object_get_fences_rcu(struct 
> reservation_object *obj,
>  
>   do {
>   struct reservation_object_list *fobj;
> - unsigned seq;
> - unsigned int i;
> + unsigned int i, seq;
> + size_t sz = 0;
>  
>   shared_count = i = 0;
>  
> @@ -402,9 +403,14 @@ int reservation_object_get_fences_rcu(struct 
> reservation_object *obj,
>   goto unlock;
>  
>   fobj = rcu_dereference(obj->fence);
> - if (fobj) {
> + if (fobj)
> + sz += sizeof(*shared) * fobj->shared_max;
> +
> + if (!pfence_excl && fence_excl)
> + sz += sizeof(*shared);
> +
> + if (sz) {
>   struct dma_fence **nshared;
> - size_t sz = sizeof(*shared) * fobj->shared_max;
>  
>   nshared = krealloc(shared, sz,
>  GFP_NOWAIT | __GFP_NOWARN);
> @@ -420,13 +426,19 @@ int reservation_object_get_fences_rcu(struct 
> reservation_object *obj,
>   break;
>   }
>   shared = nshared;
> - shared_count = fobj->shared_count;
> -
> + shared_count = fobj ? fobj->shared_count : 0;
>   for (i = 0; i < shared_count; ++i) {
>   shared[i] = rcu_dereference(fobj->shared[i]);
>   if (!dma_fence_get_rcu(shared[i]))
>   break;
>   }
> +
> + if (!pfence_excl && fence_excl) {
> + shared[i] = fence_excl;
> + fence_excl = NULL;
> + ++i;
> + ++shared_count;
> + }
>   }
>  
>   if (i != shared_count || read_seqcount_retry(>seq, seq)) {
> @@ -448,7 +460,8 @@ int reservation_object_get_fences_rcu(struct 
> reservation_object *obj,
>  
>   *pshared_count = shared_count;
>   *pshared = shared;
> - *pfence_excl = fence_excl;
> + if (pfence_excl)
> + *pfence_excl = fence_excl;
>  
>   return ret;
>  }
> -- 
> 2.14.1
> 
> ___
> Linaro-mm-sig mailing list
> linaro-mm-...@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/linaro-mm-sig

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[PATCH] dma-buf: make returning the exclusive fence optional

[Christian König]

Change reservation_object_get_fences_rcu to make the exclusive fence
pointer optional.

If not specified the exclusive fence is put into the fence array as
well.

This is helpful for a couple of cases where we need all fences in a
single array.

Signed-off-by: Christian König 
---
 drivers/dma-buf/reservation.c | 31 ++-
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
index b759a569b7b8..461afa9febd4 100644
--- a/drivers/dma-buf/reservation.c
+++ b/drivers/dma-buf/reservation.c
@@ -374,8 +374,9 @@ EXPORT_SYMBOL(reservation_object_copy_fences);
  * @pshared: the array of shared fence ptrs returned (array is krealloc'd to
  * the required size, and must be freed by caller)
  *
- * RETURNS
- * Zero or -errno
+ * Retrieve all fences from the reservation object. If the pointer for the
+ * exclusive fence is not specified the fence is put into the array of the
+ * shared fences as well. Returns either zero or -ENOMEM.
  */
 int reservation_object_get_fences_rcu(struct reservation_object *obj,
  struct dma_fence **pfence_excl,
@@ -389,8 +390,8 @@ int reservation_object_get_fences_rcu(struct 
reservation_object *obj,
 
do {
struct reservation_object_list *fobj;
-   unsigned seq;
-   unsigned int i;
+   unsigned int i, seq;
+   size_t sz = 0;
 
shared_count = i = 0;
 
@@ -402,9 +403,14 @@ int reservation_object_get_fences_rcu(struct 
reservation_object *obj,
goto unlock;
 
fobj = rcu_dereference(obj->fence);
-   if (fobj) {
+   if (fobj)
+   sz += sizeof(*shared) * fobj->shared_max;
+
+   if (!pfence_excl && fence_excl)
+   sz += sizeof(*shared);
+
+   if (sz) {
struct dma_fence **nshared;
-   size_t sz = sizeof(*shared) * fobj->shared_max;
 
nshared = krealloc(shared, sz,
   GFP_NOWAIT | __GFP_NOWARN);
@@ -420,13 +426,19 @@ int reservation_object_get_fences_rcu(struct 
reservation_object *obj,
break;
}
shared = nshared;
-   shared_count = fobj->shared_count;
-
+   shared_count = fobj ? fobj->shared_count : 0;
for (i = 0; i < shared_count; ++i) {
shared[i] = rcu_dereference(fobj->shared[i]);
if (!dma_fence_get_rcu(shared[i]))
break;
}
+
+   if (!pfence_excl && fence_excl) {
+   shared[i] = fence_excl;
+   fence_excl = NULL;
+   ++i;
+   ++shared_count;
+   }
}
 
if (i != shared_count || read_seqcount_retry(>seq, seq)) {
@@ -448,7 +460,8 @@ int reservation_object_get_fences_rcu(struct 
reservation_object *obj,
 
*pshared_count = shared_count;
*pshared = shared;
-   *pfence_excl = fence_excl;
+   if (pfence_excl)
+   *pfence_excl = fence_excl;
 
return ret;
 }
-- 
2.14.1

[PATCH] media: ts2020: avoid integer overflows on 32 bit machines

[Mauro Carvalho Chehab]

Before this patch, when compiled for arm32, the signal strength
were reported as:

Lock   (0x1f) Signal= 4294908.66dBm C/N= 12.79dB

Because of a 32 bit integer overflow. After it, it is properly
reported as:

Lock   (0x1f) Signal= -58.64dBm C/N= 12.79dB

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/dvb-frontends/ts2020.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/dvb-frontends/ts2020.c 
b/drivers/media/dvb-frontends/ts2020.c
index 931e5c98da8a..b879e1571469 100644
--- a/drivers/media/dvb-frontends/ts2020.c
+++ b/drivers/media/dvb-frontends/ts2020.c
@@ -368,7 +368,7 @@ static int ts2020_read_tuner_gain(struct dvb_frontend *fe, 
unsigned v_agc,
gain2 = clamp_t(long, gain2, 0, 13);
v_agc = clamp_t(long, v_agc, 400, 1100);
 
-   *_gain = -(gain1 * 2330 +
+   *_gain = -((__s64)gain1 * 2330 +
   gain2 * 3500 +
   v_agc * 24 / 10 * 10 +
   1);
@@ -386,7 +386,7 @@ static int ts2020_read_tuner_gain(struct dvb_frontend *fe, 
unsigned v_agc,
gain3 = clamp_t(long, gain3, 0, 6);
v_agc = clamp_t(long, v_agc, 600, 1600);
 
-   *_gain = -(gain1 * 2650 +
+   *_gain = -((__s64)gain1 * 2650 +
   gain2 * 3380 +
   gain3 * 2850 +
   v_agc * 176 / 100 * 10 -
-- 
2.14.3

Re: dvb usb issues since kernel 4.9

[Jesper Dangaard Brouer]

On Tue, 9 Jan 2018 10:58:30 -0800 Linus Torvalds 
 wrote:

> So I really think "you can use up 90% of CPU time with a UDP packet
> flood from the same network" is very very very different - and
> honestly not at all as important - as "you want to be able to use a
> USB DVB receiver and watch/record TV".
> 
> Because that whole "UDP packet flood from the same network" really is
> something you _fundamentally_ have other mitigations for.
> 
> I bet that whole commit was introduced because of a benchmark test,
> rather than real life. No?

I believe this have happened in real-life.  In the form of DNS servers
not being able to recover after long outage, where DNS-TTL had timeout
causing legitimate traffic to overload their DNS servers.  The goodput
answers/sec from their DNS servers were too low, when bringing them
online again. (Based on talk over beer at NetDevConf from a guy
claiming they ran DNS for AWS).


The commit 4cd13c21b207 ("softirq: Let ksoftirqd do its job") tries to
address a fundamental problem that the network stack have when
interacting with softirq in overload situations.
(Maybe we can come up with a better solution?)

Before this commit, when application run on same CPU as softirq, the
kernel have a bad "drop off cliff" behavior, when reaching above the
saturation point.

This is confirmed in CloudFlare blogpost[1], which used a kernel that
predates this commit. From[1] section: "A note on NUMA performance"
Quote:"
  1. Run receiver on another CPU, but on the same NUMA node as the RX
 queue. The performance as we saw above is around 360kpps.

  2. With receiver on exactly same CPU as the RX queue we can get up to
 ~430kpps. But it creates high variability. The performance drops down
 to zero if the NIC is overwhelmed with packets."

The behavior problem here is "performance drops down to zero if the NIC
is overwhelmed with packets".  That is a bad way to handle overload.
Not only when attacked, but also when bringing a service online after
an outage.

What essentially happens is that:
 1. softirq NAPI enqueue 64 packets into socket. 
 2. application dequeue 1 packet and invoke local_bh_enable()
 3. causing softirq to run in app-timeslice, again enq 64 packets
 4. app only see goodput of 1/128 of packets

That is essentially what Eric solved with his commit, avoiding (3)
local_bh_enable() to invoke softirq if ksoftirqd is already running.

Maybe we can come up with a better solution?
(as I do agree this was a too big-hammer affecting other use-cases)


[1] https://blog.cloudflare.com/how-to-receive-a-million-packets/

p.s. Regarding quote[1] point "1.", after Paolo Abeni optimized the UDP
code, that statement is no longer true.  It now (significantly) faster to
run/pin your UDP application to another CPU than the RX-CPU.
-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

[PATCH] media: lirc: Fix uninitialized variable in ir_lirc_transmit_ir()

[Dan Carpenter]

The "txbuf" is uninitialized when we call ir_raw_encode_scancode() so
this failure path would lead to a crash.

Fixes: a74b2bff5945 ("media: lirc: do not pass ERR_PTR to kfree")
Signed-off-by: Dan Carpenter 

diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index fae42f120aa4..5efe9cd2309a 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -295,7 +295,7 @@ static ssize_t ir_lirc_transmit_ir(struct file *file, const 
char __user *buf,
ret = ir_raw_encode_scancode(scan.rc_proto, scan.scancode,
 raw, LIRCBUF_SIZE);
if (ret < 0)
-   goto out_kfree;
+   goto out_free_raw;
 
count = ret;
 
@@ -366,6 +366,7 @@ static ssize_t ir_lirc_transmit_ir(struct file *file, const 
char __user *buf,
return n;
 out_kfree:
kfree(txbuf);
+out_free_raw:
kfree(raw);
 out_unlock:
mutex_unlock(>lock);

Re: [PATCH v5 1/4] dt-bindings: media: Add bindings for OV5695

[jacopo mondi]

On Wed, Jan 10, 2018 at 10:20:10AM +0100, jacopo mondi wrote:
> Hi Shunqian,
>
> On Wed, Jan 10, 2018 at 10:06:04AM +0800, Shunqian Zheng wrote:
> > Add device tree binding documentation for the OV5695 sensor.
> >
> > Signed-off-by: Shunqian Zheng 
> > ---
> >  .../devicetree/bindings/media/i2c/ov5695.txt   | 41 
> > ++
> >  1 file changed, 41 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov5695.txt
> >
> > diff --git a/Documentation/devicetree/bindings/media/i2c/ov5695.txt 
> > b/Documentation/devicetree/bindings/media/i2c/ov5695.txt
> > new file mode 100644
> > index 000..2f2f698
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/i2c/ov5695.txt
> > @@ -0,0 +1,41 @@
> > +* Omnivision OV5695 MIPI CSI-2 sensor
> > +
> > +Required Properties:
> > +- compatible: shall be "ovti,ov5695"
> > +- clocks: reference to the xvclk input clock
> > +- clock-names: shall be "xvclk"
> > +- avdd-supply: Analog voltage supply, 2.8 volts
> > +- dovdd-supply: Digital I/O voltage supply, 1.8 volts
> > +- dvdd-supply: Digital core voltage supply, 1.2 volts
> > +- reset-gpios: Low active reset gpio
> > +
> > +The device node shall contain one 'port' child node with an
> > +'endpoint' subnode for its digital output video port,
> > +in accordance with the video interface bindings defined in
> > +Documentation/devicetree/bindings/media/video-interfaces.txt.
> > +The endpoint optional property 'data-lanes' shall be "<1 2>".
>
> What happens if the property is not present? What's the default?
>
> I would:
>
> Required Properties:
> - compatible: ..
> 
>
> Option Endpoint Properties:

Optional, not Option, sorry about this

Re: [PATCH v5 1/4] dt-bindings: media: Add bindings for OV5695

[jacopo mondi]

Hi Shunqian,

On Wed, Jan 10, 2018 at 10:06:04AM +0800, Shunqian Zheng wrote:
> Add device tree binding documentation for the OV5695 sensor.
>
> Signed-off-by: Shunqian Zheng 
> ---
>  .../devicetree/bindings/media/i2c/ov5695.txt   | 41 
> ++
>  1 file changed, 41 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov5695.txt
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov5695.txt 
> b/Documentation/devicetree/bindings/media/i2c/ov5695.txt
> new file mode 100644
> index 000..2f2f698
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/ov5695.txt
> @@ -0,0 +1,41 @@
> +* Omnivision OV5695 MIPI CSI-2 sensor
> +
> +Required Properties:
> +- compatible: shall be "ovti,ov5695"
> +- clocks: reference to the xvclk input clock
> +- clock-names: shall be "xvclk"
> +- avdd-supply: Analog voltage supply, 2.8 volts
> +- dovdd-supply: Digital I/O voltage supply, 1.8 volts
> +- dvdd-supply: Digital core voltage supply, 1.2 volts
> +- reset-gpios: Low active reset gpio
> +
> +The device node shall contain one 'port' child node with an
> +'endpoint' subnode for its digital output video port,
> +in accordance with the video interface bindings defined in
> +Documentation/devicetree/bindings/media/video-interfaces.txt.
> +The endpoint optional property 'data-lanes' shall be "<1 2>".

What happens if the property is not present? What's the default?

I would:

Required Properties:
- compatible: ..


Option Endpoint Properties:
- data-lanes: ...

> +
> +Example:
> + {
> + camera-sensor: ov5695@36 {

You have inverted the label with the node name which should be generic

ov5695: camera@36 {


}
Thanks
   j

Re: [PATCH v5 2/4] media: ov5695: add support for OV5695 sensor

[jacopo mondi]

Hello Shunqian,

On Wed, Jan 10, 2018 at 10:06:05AM +0800, Shunqian Zheng wrote:

[snip]

> +static int __ov5695_start_stream(struct ov5695 *ov5695)
> +{
> + int ret;
> +
> + ret = ov5695_write_array(ov5695->client, ov5695_global_regs);
> + if (ret)
> + return ret;
> + ret = ov5695_write_array(ov5695->client, ov5695->cur_mode->reg_list);
> + if (ret)
> + return ret;
> +
> + /* In case these controls are set before streaming */
> + ret = __v4l2_ctrl_handler_setup(>ctrl_handler);
> + if (ret)
> + return ret;
> +
> + return ov5695_write_reg(ov5695->client, OV5695_REG_CTRL_MODE,
> + OV5695_REG_VALUE_08BIT, OV5695_MODE_STREAMING);
> +}
> +
> +static int __ov5695_stop_stream(struct ov5695 *ov5695)
> +{
> + return ov5695_write_reg(ov5695->client, OV5695_REG_CTRL_MODE,
> + OV5695_REG_VALUE_08BIT, OV5695_MODE_SW_STANDBY);
> +}
> +
> +static int ov5695_s_stream(struct v4l2_subdev *sd, int on)
> +{
> + struct ov5695 *ov5695 = to_ov5695(sd);
> + struct i2c_client *client = ov5695->client;
> + int ret = 0;
> +
> + mutex_lock(>mutex);
> + on = !!on;
> + if (on == ov5695->streaming)
> + goto unlock_and_return;
> +
> + if (on) {
> + ret = pm_runtime_get_sync(>dev);
> + if (ret < 0) {
> + pm_runtime_put_noidle(>dev);
> + goto unlock_and_return;
> + }
> +
> + ret = __ov5695_start_stream(ov5695);
> + if (ret) {
> + v4l2_err(sd, "start stream failed while write regs\n");
> + pm_runtime_put(>dev);
> + goto unlock_and_return;
> + }
> + } else {
> + __ov5695_stop_stream(ov5695);
> + ret = pm_runtime_put(>dev);

I would return the result of __ov5695_stop_stream() instead of
pm_runtime_put().

I know I asked for this, but if the first s_stream(0) fails, the
sensor may not have been stopped but the interface will be put in
"streaming = 0" state, preventing a second s_stream(0) to be issued
because of your check "on == ov5695->streaming" a few lines above.

I can't tell how bad this is. Imho is acceptable but I would like to
hear someone else opinion here :)

> + }
> +
> + ov5695->streaming = on;
> +
> +unlock_and_return:
> + mutex_unlock(>mutex);
> +
> + return ret;
> +}
> +
> +

[snip]

> +static const struct of_device_id ov5695_of_match[] = {
> + { .compatible = "ovti,ov5695" },
> + {},
> +};

If you don't list CONFIG_OF as a dependecy for this driver (which you
should not imho), please guard this with:

#if IS_ENABLED(CONFIG_OF)

#endif

> +
> +static struct i2c_driver ov5695_i2c_driver = {
> + .driver = {
> + .name = "ov5695",
> + .owner = THIS_MODULE,
> + .pm = _pm_ops,
> + .of_match_table = ov5695_of_match
> + },
> + .probe  = _probe,
> + .remove = _remove,
> +};
> +
> +module_i2c_driver(ov5695_i2c_driver);
> +
> +MODULE_DESCRIPTION("OmniVision ov5695 sensor driver");
> +MODULE_LICENSE("GPL v2");

As you've fixed my comments on v1, and with the above bits addressed:

Reviewed-by: Jacopo Mondi 

Thanks
   j

> --
> 1.9.1
>

Re: [PATCH 1/3] media/ttusb-budget: remove pci_zalloc_coherent abuse

[Christoph Hellwig]

On Tue, Jan 09, 2018 at 12:49:26PM -0800, Joe Perches wrote:
> This message doesn't make sense anymore and it might as well
> be deleted.
> 
> And it might be better to use kcalloc
> 
>   ttusb->iso_buffer = kcalloc(FRAMES_PER_ISO_BUF * ISO_BUF_COUNT,
>   ISO_FRAME_SIZE, GFP_KERNEL);

Sure.

Re: [PATCH 3/3] pci-dma-compat: remove handling of NULL pdev arguments

[Christoph Hellwig]

On Tue, Jan 09, 2018 at 06:25:44PM -0600, Bjorn Helgaas wrote:
> It looks like "pci_free_consistent(NULL" is still used in
> drivers/net/ethernet/tundra/tsi108_eth.c.

Yikes.  That one needs to pass the device is the platform dev to
the dma_map_* routines to start with, and mixing that with PCI
is pretty horrible.  I'll add a conversion of that driver to the
next resend.

> > Signed-off-by: Christoph Hellwig 
> 
> With Mauro's ack on the media/ttusb-dev patches, I could merge the
> whole series via the PCI tree?

Fine with me.