Re: [RFC 1/1] omap3isp: Ignore endpoints with invalid configuration

[Sakari Ailus]

Pavel Machek wrote:

On Tue 2017-02-28 14:45:31, Sakari Ailus wrote:

Pavel Machek wrote:

Tested-by: Pavel Machek 


Thanks!

I've applied the patch, plus yours, to the ccp2 branch.


Thanks!

https://git.linuxtv.org/sailus/media_tree.git/log/?h=ccp2 ? I don't
see recent changes there...


I accidentally pushed it to a wrong place. Pushing again...

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

Re: [PATCH v4 15/36] platform: add video-multiplexer subdevice driver

[Steve Longerbeam]

On 02/27/2017 06:41 AM, Rob Herring wrote:

On Wed, Feb 15, 2017 at 06:19:17PM -0800, Steve Longerbeam wrote:

From: Philipp Zabel 

This driver can handle SoC internal and external video bus multiplexers,
controlled either by register bit fields or by a GPIO. The subdevice
passes through frame interval and mbus configuration of the active input
to the output side.

Signed-off-by: Sascha Hauer 
Signed-off-by: Philipp Zabel 

--

- fixed a cut error in vidsw_remove(): v4l2_async_register_subdev()
   should be unregister.

- added media_entity_cleanup() and v4l2_device_unregister_subdev()
   to vidsw_remove().

- added missing MODULE_DEVICE_TABLE().
   Suggested-by: Javier Martinez Canillas 

- there was a line left over from a previous iteration that negated
   the new way of determining the pad count just before it which
   has been removed (num_pads = of_get_child_count(np)).

- Philipp Zabel has developed a set of patches that allow adding
   to the subdev async notifier waiting list using a chaining method
   from the async registered callbacks (v4l2_of_subdev_registered()
   and the prep patches for that). For now, I've removed the use of
   v4l2_of_subdev_registered() for the vidmux driver's registered
   callback. This doesn't affect the functionality of this driver,
   but allows for it to be merged now, before adding the chaining
   support.

Signed-off-by: Steve Longerbeam 
---
  .../bindings/media/video-multiplexer.txt   |  59 +++

Please make this a separate commit.


Done.

Steve

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:   Wed Mar  1 05:00:22 CET 2017
media-tree git hash:e6b377dbbb944d5e3ceef4e5d429fc5c841e3692
media_build git hash:   9d6cebc34b27fea784dec19085970d9b4df9783e
v4l-utils git hash: 646bb9c368a8b65cdea6d934c9022067541d13a9
gcc version:i686-linux-gcc (GCC) 6.2.0
sparse version: v0.5.0-3553-g78b2ea6
smatch version: v0.5.0-3553-g78b2ea6
host hardware:  x86_64
host os:4.9.0-164

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

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

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

Streaming audio from my device in VLC causes the whole system to freeze

[Alexandre-Xavier L-L]

Hi,

I have a few devices that work on Linux (for example, the Plextor
ConvertX PX-AV100U which uses the em28xx driver) and I wonder: When I
open their audio stream with VLC, I select the corresponding audio
device (example: hw:0,0) and it starts streaming, but almost instantly
freezes my OS.

I press "caps lock" or "num lock" and the light on the keyboard won't
change, which lets me think the kernel has frozen. The sound will
continue playing if any and the cursor can still move, but clicking
anywhere won't do anything. The computer doesn't react to any input.

How do I diagnose the cause of the problem? There is no kernel panic.
Is there any way that I can figure out where this bug comes from? I've
searched log files, but can't find anything.

Thanks,
Alexandre-Xavier

Re: [PATCH 1/4] v4l: vsp1: Implement partition algorithm restrictions

[Kuninori Morimoto]

Hi Laurent, Kieran

> > Testing SRU-UDS scaling 768x576 - 768x576 - 640x480 in RGB24: fail
> > Testing SRU-UDS scaling 768x576 - 768x576 - 768x576 in RGB24: pass
> > Testing SRU-UDS scaling 768x576 - 768x576 - 1024x768 in RGB24: pass
> > Testing SRU-UDS scaling 768x576 - 1536x1152 - 1280x960 in RGB24: pass
> > Testing SRU-UDS scaling 768x576 - 1536x1152 - 1536x1152 in RGB24: pass
> > Testing SRU-UDS scaling 768x576 - 1536x1152 - 2048x1536 in RGB24: pass
> > Testing UDS-SRU scaling 768x576 - 640x480 - 640x480 in RGB24: pass
> > Testing UDS-SRU scaling 768x576 - 640x480 - 1280x960 in RGB24: pass
> > Testing UDS-SRU scaling 768x576 - 768x576 - 768x576 in RGB24: pass
> > Testing UDS-SRU scaling 768x576 - 768x576 - 1536x1152 in RGB24: pass
> > Testing UDS-SRU scaling 768x576 - 1024x768 - 1024x768 in RGB24: pass
> > Testing UDS-SRU scaling 768x576 - 1024x768 - 2048x1536 in RGB24: pass
> > Testing SRU-UDS scaling 768x576 - 768x576 - 640x480 in YUV444M: fail
> > Testing SRU-UDS scaling 768x576 - 768x576 - 768x576 in YUV444M: pass
> > Testing SRU-UDS scaling 768x576 - 768x576 - 1024x768 in YUV444M: pass
> > Testing SRU-UDS scaling 768x576 - 1536x1152 - 1280x960 in YUV444M: pass
> > Testing SRU-UDS scaling 768x576 - 1536x1152 - 1536x1152 in YUV444M: pass
> > Testing SRU-UDS scaling 768x576 - 1536x1152 - 2048x1536 in YUV444M: hangs
> > Testing UDS-SRU scaling 768x576 - 640x480 - 640x480 in YUV444M: pass
> > Testing UDS-SRU scaling 768x576 - 640x480 - 1280x960 in YUV444M: fail
> > Testing UDS-SRU scaling 768x576 - 768x576 - 768x576 in YUV444M: pass
> > Testing UDS-SRU scaling 768x576 - 768x576 - 1536x1152 in YUV444M: pass
> > Testing UDS-SRU scaling 768x576 - 1024x768 - 1024x768 in YUV444M: pass
> > Testing UDS-SRU scaling 768x576 - 1024x768 - 2048x1536 in YUV444M: hangs
> (snip)
> > However, from the above tests it looks like the hardware can live with more 
> > relaxed restrictions than the ones implemented here. I haven't tested all 
> > UDS 
> > scaling ratios, and certainly not under all memory bus load conditions, I 
> > might thus be too optimistic. Morimoto-san, would it be possible to get 
> > more 
> > information about this from the hardware team, to check whether the above 
> > two 
> > restrictions need to be honoured, or whether they come from an older 
> > hardware 
> > version ?
> 
> I asked it to HW team.
> Please wait

We still not yet get clear answer from HW team.
It is still researching

Best regards
---
Kuninori Morimoto

Re: [PATCH v4 24/36] [media] add Omnivision OV5640 sensor driver

[Steve Longerbeam]

On 02/27/2017 06:45 AM, Rob Herring wrote:

On Wed, Feb 15, 2017 at 06:19:26PM -0800, Steve Longerbeam wrote:

This driver is based on ov5640_mipi.c from Freescale imx_3.10.17_1.0.0_beta
branch, modified heavily to bring forward to latest interfaces and code
cleanup.

Signed-off-by: Steve Longerbeam 
---
  .../devicetree/bindings/media/i2c/ov5640.txt   |   43 +

Please split to separate commit.


Done.




  drivers/media/i2c/Kconfig  |7 +
  drivers/media/i2c/Makefile |1 +
  drivers/media/i2c/ov5640.c | 2109 
  4 files changed, 2160 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov5640.txt
  create mode 100644 drivers/media/i2c/ov5640.c

diff --git a/Documentation/devicetree/bindings/media/i2c/ov5640.txt 
b/Documentation/devicetree/bindings/media/i2c/ov5640.txt
new file mode 100644
index 000..4607bbe
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov5640.txt
@@ -0,0 +1,43 @@
+* Omnivision OV5640 MIPI CSI-2 sensor
+
+Required Properties:
+- compatible: should be "ovti,ov5640"
+- clocks: reference to the xclk input clock.
+- clock-names: should be "xclk".
+- DOVDD-supply: Digital I/O voltage supply, 1.8 volts
+- AVDD-supply: Analog voltage supply, 2.8 volts
+- DVDD-supply: Digital core voltage supply, 1.5 volts
+
+Optional Properties:
+- reset-gpios: reference to the GPIO connected to the reset pin, if any.
+- pwdn-gpios: reference to the GPIO connected to the pwdn pin, if any.

Use powerdown-gpios here as that is a somewhat standard name.


Done.



Both need to state what is the active state.


Done.

Steve

Re: [PATCH v4 01/36] [media] dt-bindings: Add bindings for i.MX media driver

[Steve Longerbeam]

Hi Rob,


On 02/27/2017 06:38 AM, Rob Herring wrote:

On Wed, Feb 15, 2017 at 06:19:03PM -0800, Steve Longerbeam wrote:

Add bindings documentation for the i.MX media driver.

Signed-off-by: Steve Longerbeam 
---
  Documentation/devicetree/bindings/media/imx.txt | 66 +
  1 file changed, 66 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/media/imx.txt

diff --git a/Documentation/devicetree/bindings/media/imx.txt 
b/Documentation/devicetree/bindings/media/imx.txt
new file mode 100644
index 000..fd5af50
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/imx.txt
@@ -0,0 +1,66 @@
+Freescale i.MX Media Video Device
+=
+
+Video Media Controller node
+---
+
+This is the media controller node for video capture support. It is a
+virtual device that lists the camera serial interface nodes that the
+media device will control.
+
+Required properties:
+- compatible : "fsl,imx-capture-subsystem";
+- ports  : Should contain a list of phandles pointing to camera
+   sensor interface ports of IPU devices
+
+example:
+
+capture-subsystem {
+   compatible = "fsl,capture-subsystem";
+   ports = <_csi0>, <_csi1>;
+};
+
+fim child node
+--
+
+This is an optional child node of the ipu_csi port nodes. If present and
+available, it enables the Frame Interval Monitor. Its properties can be
+used to modify the method in which the FIM measures frame intervals.
+Refer to Documentation/media/v4l-drivers/imx.rst for more info on the
+Frame Interval Monitor.

This should have a compatible string.


I don't think so. The fim child node does not represent a device. The
CSI supports monitoring frame intervals (reporting via a v4l2 event
when a measured frame interval falls outside the nominal interval
by some tolerance value). The fim child node is only to group properties
for the FIM under a common child node.


+
+Optional properties:
+- fsl,input-capture-channel: an input capture channel and channel flags,
+specified as . The channel number
+must be 0 or 1. The flags can be
+IRQ_TYPE_EDGE_RISING, IRQ_TYPE_EDGE_FALLING, or
+IRQ_TYPE_EDGE_BOTH, and specify which input
+capture signal edge will trigger the input
+capture event. If an input capture channel is
+specified, the FIM will use this method to
+measure frame intervals instead of via the EOF
+interrupt. The input capture method is much
+preferred over EOF as it is not subject to
+interrupt latency errors. However it requires
+routing the VSYNC or FIELD output signals of
+the camera sensor to one of the i.MX input
+capture pads (SD1_DAT0, SD1_DAT1), which also
+gives up support for SD1.
+
+
+mipi_csi2 node
+--
+
+This is the device node for the MIPI CSI-2 Receiver, required for MIPI
+CSI-2 sensors.
+
+Required properties:
+- compatible   : "fsl,imx6-mipi-csi2", "snps,dw-mipi-csi2";
+- reg   : physical base address and length of the register set;
+- clocks   : the MIPI CSI-2 receiver requires three clocks: hsi_tx
+  (the DPHY clock), video_27m, and eim_podf;
+- clock-names  : must contain "dphy", "cfg", "pix";

Don't you need ports to describe the sensor and IPU connections?


Done.

Steve

[PATCH] v4l: vsp1: Disable HSV formats on Gen3 hardware

[Laurent Pinchart]

While all VSP instances can process HSV internally, on Gen3 hardware
reading or writing HSV24 or HSV32 from/to memory causes the device to
hang. Disable those pixel formats on Gen3 hardware.

Signed-off-by: Laurent Pinchart 
---
 drivers/media/platform/vsp1/vsp1_pipe.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_pipe.c 
b/drivers/media/platform/vsp1/vsp1_pipe.c
index 3f1acf68dc6e..35364f594e19 100644
--- a/drivers/media/platform/vsp1/vsp1_pipe.c
+++ b/drivers/media/platform/vsp1/vsp1_pipe.c
@@ -157,9 +157,15 @@ const struct vsp1_format_info *vsp1_get_format_info(struct 
vsp1_device *vsp1,
 {
unsigned int i;
 
-   /* Special case, the VYUY format is supported on Gen2 only. */
-   if (vsp1->info->gen != 2 && fourcc == V4L2_PIX_FMT_VYUY)
-   return NULL;
+   /* Special case, the VYUY and HSV formats are supported on Gen2 only. */
+   if (vsp1->info->gen != 2) {
+   switch (fourcc) {
+   case V4L2_PIX_FMT_VYUY:
+   case V4L2_PIX_FMT_HSV24:
+   case V4L2_PIX_FMT_HSV32:
+   return NULL;
+   }
+   }
 
for (i = 0; i < ARRAY_SIZE(vsp1_video_formats); ++i) {
const struct vsp1_format_info *info = _video_formats[i];
-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 3/3] v4l: vsp1: wpf: Implement rotation support

[Laurent Pinchart]

Hi Sakari,

On Tuesday 28 Feb 2017 23:13:34 Sakari Ailus wrote:
> On Tue, Feb 28, 2017 at 05:03:20PM +0200, Laurent Pinchart wrote:
> > Some WPF instances, on Gen3 devices, can perform 90° rotation when
> > writing frames to memory. Implement support for this using the
> > V4L2_CID_ROTATE control.
> > 
> > Signed-off-by: Laurent Pinchart
> > 
> > ---
> > 
> >  drivers/media/platform/vsp1/vsp1_rpf.c   |   2 +-
> >  drivers/media/platform/vsp1/vsp1_rwpf.c  |   5 +
> >  drivers/media/platform/vsp1/vsp1_rwpf.h  |   3 +-
> >  drivers/media/platform/vsp1/vsp1_video.c |  12 +-
> >  drivers/media/platform/vsp1/vsp1_wpf.c   | 205 ++
> >  5 files changed, 175 insertions(+), 52 deletions(-)

[snip]

> > diff --git a/drivers/media/platform/vsp1/vsp1_rwpf.h
> > b/drivers/media/platform/vsp1/vsp1_rwpf.h index
> > 1c98aff3da5d..b4ffc38f48af 100644
> > --- a/drivers/media/platform/vsp1/vsp1_rwpf.h
> > +++ b/drivers/media/platform/vsp1/vsp1_rwpf.h
> > @@ -56,9 +56,10 @@ struct vsp1_rwpf {
> > 
> > struct {
> > spinlock_t lock;
> > -   struct v4l2_ctrl *ctrls[2];
> > +   struct v4l2_ctrl *ctrls[3];
> 
> At least what comes to this patch --- having a field for each control would
> look much nicer in the code. Is there a particular reason for having an
> array with all the controls in it?

Not really. I need to put the three controls in a cluster, so I stored them in 
an array to make sure they would be properly contiguous in memory. I can also 
use three separate pointers, it shouldn't hurt.

> > unsigned int pending;
> > unsigned int active;
> > +   bool rotate;
> > } flip;
> > 
> > struct vsp1_rwpf_memory mem;

-- 
Regards,

Laurent Pinchart

Re: ir-keytable: infinite loops, segfaults

[Sean Young]

On Sat, Feb 25, 2017 at 02:08:39AM +1100, Vincent McIntyre wrote:
> On 2/22/17, Sean Young  wrote:
> 
> > So it's still using the old keymap. I've attached a new one.
> 
> That works, thanks.
> 
> >>   # vol down
> >> 1487676637.746348: event type EV_MSC(0x04): scancode = 0x0105
> >> 1487676637.746348: event type EV_SYN(0x00).
> >>   # vol up
> >> 1487676642.746321: event type EV_MSC(0x04): scancode = 0x0115
> >> 1487676642.746321: event type EV_SYN(0x00).
> >
> > Oops, that's a bug. 0x should be 0x. I've attached a new version of
> > the patch which should fix that.
> >
> 
> I am still getting the high bits set. I checked the code and the patch
> was correctly applied,
> I see where you are applying a 0xff mask to the ircode values.
> 
> 
> Test run:
> # Found /sys/class/rc/rc0/ (/dev/input/event5) with:
> Driver imon, table rc-imon-mce
> Supported protocols: rc-6
> Enabled protocols: rc-6
> Name: iMON Remote (15c2:ffdc)
> bus: 3, vendor/product: 15c2:ffdc, version: 0x
> Repeat delay = 500 ms, repeat period = 125 ms
> Found /sys/class/rc/rc1/ (/dev/input/event15) with:
> Driver dvb_usb_cxusb, table rc-dvico-mce
> Supported protocols: nec
> Enabled protocols:
> Name: IR-receiver inside an USB DVB re
> bus: 3, vendor/product: 0fe9:db78, version: 0x827b
> Repeat delay = 500 ms, repeat period = 125 ms
> Found /sys/class/rc/rc2/ (/dev/input/event16) with:
> Driver dvb_usb_af9035, table rc-empty
> Supported protocols: nec
> Enabled protocols:
> Name: Leadtek WinFast DTV Dongle Dual
> bus: 3, vendor/product: 0413:6a05, version: 0x0200
> Repeat delay = 500 ms, repeat period = 125 ms
> 
> #  ir-keytable -r -t -d /dev/input/event15
> scancode 0x0101 = KEY_RECORD (0xa7)
> scancode 0x0102 = KEY_TV (0x179)
> scancode 0x0103 = KEY_0 (0x0b)
> scancode 0x0105 = KEY_VOLUMEDOWN (0x72)
> scancode 0x0107 = KEY_4 (0x05)
> scancode 0x0109 = KEY_CHANNELDOWN (0x193)
> scancode 0x010a = KEY_EPG (0x16d)
> scancode 0x010b = KEY_1 (0x02)
> scancode 0x010d = KEY_STOP (0x80)
> scancode 0x010e = KEY_MP3 (0x187)
> scancode 0x010f = KEY_PREVIOUSSONG (0xa5)
> scancode 0x0111 = KEY_CHANNELUP (0x192)
> scancode 0x0112 = KEY_NEXTSONG (0xa3)
> scancode 0x0113 = KEY_ANGLE (0x173)
> scancode 0x0115 = KEY_VOLUMEUP (0x73)
> scancode 0x0116 = KEY_SETUP (0x8d)
> scancode 0x0117 = KEY_2 (0x03)
> scancode 0x0119 = KEY_OPEN (0x86)
> scancode 0x011a = KEY_DVD (0x185)
> scancode 0x011b = KEY_3 (0x04)
> scancode 0x011e = KEY_FAVORITES (0x16c)
> scancode 0x011f = KEY_ZOOM (0x174)
> scancode 0x0142 = KEY_ENTER (0x1c)
> scancode 0x0143 = KEY_REWIND (0xa8)
> scancode 0x0146 = KEY_POWER2 (0x164)
> scancode 0x0147 = KEY_PLAYPAUSE (0xa4)
> scancode 0x0148 = KEY_7 (0x08)
> scancode 0x0149 = KEY_BACK (0x9e)
> scancode 0x014c = KEY_8 (0x09)
> scancode 0x014d = KEY_MENU (0x8b)
> scancode 0x014e = KEY_POWER (0x74)
> scancode 0x014f = KEY_FASTFORWARD (0xd0)
> scancode 0x0150 = KEY_5 (0x06)
> scancode 0x0151 = KEY_UP (0x67)
> scancode 0x0152 = KEY_CAMERA (0xd4)
> scancode 0x0153 = KEY_DOWN (0x6c)
> scancode 0x0154 = KEY_6 (0x07)
> scancode 0x0155 = KEY_TAB (0x0f)
> scancode 0x0157 = KEY_MUTE (0x71)
> scancode 0x0158 = KEY_9 (0x0a)
> scancode 0x0159 = KEY_INFO (0x166)
> scancode 0x015a = KEY_TUNER (0x182)
> scancode 0x015b = KEY_LEFT (0x69)
> scancode 0x015e = KEY_OK (0x160)
> scancode 0x015f = KEY_RIGHT (0x6a)
> Enabled protocols: other jvc sony nec sanyo mce-kbd rc-6 sharp xmp
> Testing events. Please, press CTRL-C to abort.
>  # '1'
> 1487948112.709532: event type EV_MSC(0x04): scancode = 0x010b
> 1487948112.709532: event type EV_SYN(0x00).
>  # '2'
> 1487948137.229455: event type EV_MSC(0x04): scancode = 0x0117
> 1487948137.229455: event type EV_SYN(0x00).
>  # '8'
> 1487948233.341489: event type EV_MSC(0x04): scancode = 0x014c
> 1487948233.341489: event type EV_SYN(0x00).
>  # '9'
> 1487948248.417547: event type EV_MSC(0x04): scancode = 0x0158
> 1487948248.417547: event type EV_SYN(0x00).
>  # volume_down
> 1487948270.537497: event type EV_MSC(0x04): scancode = 0x0105
> 1487948270.537497: event type EV_SYN(0x00).
>  # volume_up
> 1487948464.425435: event type EV_MSC(0x04): scancode = 0x0115
> 1487948464.425435: event type EV_SYN(0x00).

Sorry Vincent, but are you sure you're running the patch with the
& 0xff mask? That should have solved it.


Sean

Re: [PATCH] [media] tw5864: use dev_warn instead of WARN to shut up warning

[Andrey Utkin]

On Tue, Feb 28, 2017 at 10:14:37PM +0100, Arnd Bergmann wrote:
> tw5864_frameinterval_get() only initializes its output when it successfully
> identifies the video standard in tw5864_input. We get a warning here because
> gcc can't always track the state if initialized warnings across a WARN()
> macro, and thinks it might get used incorrectly in tw5864_s_parm:
> 
> media/pci/tw5864/tw5864-video.c: In function 'tw5864_s_parm':
> media/pci/tw5864/tw5864-video.c:816:38: error: 'time_base.numerator' may be 
> used uninitialized in this function [-Werror=maybe-uninitialized]
> media/pci/tw5864/tw5864-video.c:819:31: error: 'time_base.denominator' may be 
> used uninitialized in this function [-Werror=maybe-uninitialized]
> 
> Using dev_warn() instead of WARN() avoids the __branch_check__() in
> unlikely and lets the compiler see that the initialization is correct.
> 
> Signed-off-by: Arnd Bergmann 

Thanks for the patch.
Acked-by: Andrey Utkin 

See the note below.

> ---
>  drivers/media/pci/tw5864/tw5864-video.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/pci/tw5864/tw5864-video.c 
> b/drivers/media/pci/tw5864/tw5864-video.c
> index 9421216bb942..4d9994a11c22 100644
> --- a/drivers/media/pci/tw5864/tw5864-video.c
> +++ b/drivers/media/pci/tw5864/tw5864-video.c
> @@ -717,6 +717,8 @@ static void tw5864_frame_interval_set(struct tw5864_input 
> *input)
>  static int tw5864_frameinterval_get(struct tw5864_input *input,
>   struct v4l2_fract *frameinterval)
>  {
> + struct tw5864_dev *dev = input->root;
> +
>   switch (input->std) {
>   case STD_NTSC:
>   frameinterval->numerator = 1001;
> @@ -728,8 +730,8 @@ static int tw5864_frameinterval_get(struct tw5864_input 
> *input,
>   frameinterval->denominator = 25;
>   break;
>   default:
> - WARN(1, "tw5864_frameinterval_get requested for unknown std 
> %d\n",
> -  input->std);
> + dev_warn(>pci->dev, "tw5864_frameinterval_get requested 
> for unknown std %d\n",
> +  input->std);
>   return -EINVAL;
>   }

Looks good, though, arguably, it could fit 80 columns better if you put
the string literal to separate line.

[PATCH] [media] tw5864: use dev_warn instead of WARN to shut up warning

[Arnd Bergmann]

tw5864_frameinterval_get() only initializes its output when it successfully
identifies the video standard in tw5864_input. We get a warning here because
gcc can't always track the state if initialized warnings across a WARN()
macro, and thinks it might get used incorrectly in tw5864_s_parm:

media/pci/tw5864/tw5864-video.c: In function 'tw5864_s_parm':
media/pci/tw5864/tw5864-video.c:816:38: error: 'time_base.numerator' may be 
used uninitialized in this function [-Werror=maybe-uninitialized]
media/pci/tw5864/tw5864-video.c:819:31: error: 'time_base.denominator' may be 
used uninitialized in this function [-Werror=maybe-uninitialized]

Using dev_warn() instead of WARN() avoids the __branch_check__() in
unlikely and lets the compiler see that the initialization is correct.

Signed-off-by: Arnd Bergmann 
---
 drivers/media/pci/tw5864/tw5864-video.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/media/pci/tw5864/tw5864-video.c 
b/drivers/media/pci/tw5864/tw5864-video.c
index 9421216bb942..4d9994a11c22 100644
--- a/drivers/media/pci/tw5864/tw5864-video.c
+++ b/drivers/media/pci/tw5864/tw5864-video.c
@@ -717,6 +717,8 @@ static void tw5864_frame_interval_set(struct tw5864_input 
*input)
 static int tw5864_frameinterval_get(struct tw5864_input *input,
struct v4l2_fract *frameinterval)
 {
+   struct tw5864_dev *dev = input->root;
+
switch (input->std) {
case STD_NTSC:
frameinterval->numerator = 1001;
@@ -728,8 +730,8 @@ static int tw5864_frameinterval_get(struct tw5864_input 
*input,
frameinterval->denominator = 25;
break;
default:
-   WARN(1, "tw5864_frameinterval_get requested for unknown std 
%d\n",
-input->std);
+   dev_warn(>pci->dev, "tw5864_frameinterval_get requested 
for unknown std %d\n",
+input->std);
return -EINVAL;
}
 
-- 
2.9.0

Re: [PATCH v2 3/3] v4l: vsp1: wpf: Implement rotation support

[Sakari Ailus]

Hi Laurent,

On Tue, Feb 28, 2017 at 05:03:20PM +0200, Laurent Pinchart wrote:
> Some WPF instances, on Gen3 devices, can perform 90° rotation when
> writing frames to memory. Implement support for this using the
> V4L2_CID_ROTATE control.
> 
> Signed-off-by: Laurent Pinchart 
> ---
>  drivers/media/platform/vsp1/vsp1_rpf.c   |   2 +-
>  drivers/media/platform/vsp1/vsp1_rwpf.c  |   5 +
>  drivers/media/platform/vsp1/vsp1_rwpf.h  |   3 +-
>  drivers/media/platform/vsp1/vsp1_video.c |  12 +-
>  drivers/media/platform/vsp1/vsp1_wpf.c   | 205 
> +++
>  5 files changed, 175 insertions(+), 52 deletions(-)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_rpf.c 
> b/drivers/media/platform/vsp1/vsp1_rpf.c
> index f5a9a4c8c74d..8feddd59cf8d 100644
> --- a/drivers/media/platform/vsp1/vsp1_rpf.c
> +++ b/drivers/media/platform/vsp1/vsp1_rpf.c
> @@ -106,7 +106,7 @@ static void rpf_configure(struct vsp1_entity *entity,
>* of the pipeline.
>*/
>   output = vsp1_entity_get_pad_format(wpf, wpf->config,
> - RWPF_PAD_SOURCE);
> + RWPF_PAD_SINK);
>  
>   crop.width = pipe->partition.width * input_width
>  / output->width;
> diff --git a/drivers/media/platform/vsp1/vsp1_rwpf.c 
> b/drivers/media/platform/vsp1/vsp1_rwpf.c
> index 7d52c88a583e..cfd8f1904fa6 100644
> --- a/drivers/media/platform/vsp1/vsp1_rwpf.c
> +++ b/drivers/media/platform/vsp1/vsp1_rwpf.c
> @@ -121,6 +121,11 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev 
> *subdev,
>   RWPF_PAD_SOURCE);
>   *format = fmt->format;
>  
> + if (rwpf->flip.rotate) {
> + format->width = fmt->format.height;
> + format->height = fmt->format.width;
> + }
> +
>  done:
>   mutex_unlock(>entity.lock);
>   return ret;
> diff --git a/drivers/media/platform/vsp1/vsp1_rwpf.h 
> b/drivers/media/platform/vsp1/vsp1_rwpf.h
> index 1c98aff3da5d..b4ffc38f48af 100644
> --- a/drivers/media/platform/vsp1/vsp1_rwpf.h
> +++ b/drivers/media/platform/vsp1/vsp1_rwpf.h
> @@ -56,9 +56,10 @@ struct vsp1_rwpf {
>  
>   struct {
>   spinlock_t lock;
> - struct v4l2_ctrl *ctrls[2];
> + struct v4l2_ctrl *ctrls[3];

At least what comes to this patch --- having a field for each control would
look much nicer in the code. Is there a particular reason for having an
array with all the controls in it?

>   unsigned int pending;
>   unsigned int active;
> + bool rotate;
>   } flip;
>  
>   struct vsp1_rwpf_memory mem;

-- 
Regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk

Re: [RFC 1/1] omap3isp: Ignore endpoints with invalid configuration

[Pavel Machek]

On Tue 2017-02-28 14:45:31, Sakari Ailus wrote:
> Pavel Machek wrote:
> >Tested-by: Pavel Machek 
> 
> Thanks!
> 
> I've applied the patch, plus yours, to the ccp2 branch.

Thanks!

https://git.linuxtv.org/sailus/media_tree.git/log/?h=ccp2 ? I don't
see recent changes there...

Best regards,
Pavel

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


signature.asc
Description: Digital signature

Re: [PATCH 4/6] omap3isp: Disable streaming at driver unbind time

[Sakari Ailus]

On Tue, Feb 28, 2017 at 04:53:11PM +0200, Laurent Pinchart wrote:
> Hi Sakari,
> 
> On Tuesday 28 Feb 2017 16:00:01 Sakari Ailus wrote:
> > > On Monday 20 Feb 2017 17:22:20 Sakari Ailus wrote:
> > >> Once the driver is unbound accessing the hardware is not allowed anymore.
> > >> Due to this, disable streaming when the device driver is unbound. The
> > >> states of the associated objects related to Media controller and
> > >> videobuf2 frameworks are updated as well, just like if the application
> > >> disabled streaming explicitly.
> > >> 
> > >> Signed-off-by: Sakari Ailus 
> > > 
> > > This looks mostly good to me, although I'm a bit concerned about race
> > > conditions related to buffer handling. I don't think this patch introduces
> > > any new one though, so
> > > 
> > > Reviewed-by: Laurent Pinchart 
> > > 
> > > We'll have to go through buffer management at some point in the near
> > > future, including from a V4L2 API point of view I think.
> > 
> > Thanks for the review!
> > 
> > Are you happy with me sending a pull request on the set, or would you
> > prefer to pick the omap3isp patches? In the latter case I'll send a fix
> > for the issue in the first patch.
> 
> Feel free to send a pull request, I don't have anything conflicting queued 
> for 
> v4.12.

Ack. I'll send a pull request then.

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

Re: [PATCH] media: vpif: request enable-gpios

[Bartosz Golaszewski]

2017-02-22 14:27 GMT+01:00 Bartosz Golaszewski :
> This change is needed to make vpif capture work on the da850-evm board
> where the capture function must be selected on the UI expander.
>
> Signed-off-by: Bartosz Golaszewski 
> ---
>  drivers/media/platform/davinci/vpif_capture.c | 7 +++
>  1 file changed, 7 insertions(+)
>

Disregard this patch - we need to determine a better solution for this
functionality.

Thanks,
Bartosz

Re: em28xx: new board id [1d19:6901]

[Frank Schäfer]

Am 27.02.2017 um 21:21 schrieb Łukasz Strzeszkowski:

Hi,

I’ve found a new device which is not listed

model: LogiLink VG0011
vendor/product: [1d19:6901] Dexatek Technology Ltd.

mode: analog

I am unable to load a driver, because there is no such vendor in driver list.

dmesg output:
[ 1232.506295] usb 2-4: new high-speed USB device number 4 using xhci_hcd
[ 1232.637496] usb 2-4: New USB device found, idVendor=1d19, idProduct=6901
[ 1232.637500] usb 2-4: New USB device strings: Mfr=0, Product=1, SerialNumber=0
[ 1232.637502] usb 2-4: Product: USB 2861 Video
[ 1232.660061] usbcore: registered new interface driver snd-usb-audio


Regards
Łukasz Strzeszkowski


You can (temporarily) add the device id to the em28xx driver at runtime:

modprobe em28xx
echo 1d19 6901 > /sys/module/em28xx/drivers/usb:em28xx/new_id

Then plug in the device and see what happens.

HTH,
Frank

[PATCH v3 0/2] media: dt-bindings: extend the vpif bindings

[Bartosz Golaszewski]

This series adds pdata quirks and other changes required to make vpif
work on the da850-evm board.

v1 -> v2:
- added patch 3/3
- specified the purpose of port@0 and port@1 nodes

v2 -> v3:
- removed patch 3/3 - it may take some more time to determine the
  correct solution for enable-gpios, so I decided to respin the
  series without it and send it later as a follow-up
- added Rob Herring's acks

Bartosz Golaszewski (2):
  media: dt-bindings: vpif: fix whitespace errors
  media: dt-bindings: vpif: extend the example with an output port

 .../devicetree/bindings/media/ti,da850-vpif.txt| 50 --
 1 file changed, 37 insertions(+), 13 deletions(-)

-- 
2.9.3

[PATCH v3 1/2] media: dt-bindings: vpif: fix whitespace errors

[Bartosz Golaszewski]

The examples have been copied from the DT with whitespace errors. Fix
them.

Signed-off-by: Bartosz Golaszewski 
Acked-by: Rob Herring 
---
 Documentation/devicetree/bindings/media/ti,da850-vpif.txt | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/ti,da850-vpif.txt 
b/Documentation/devicetree/bindings/media/ti,da850-vpif.txt
index 6d25d7f..9c7510b 100644
--- a/Documentation/devicetree/bindings/media/ti,da850-vpif.txt
+++ b/Documentation/devicetree/bindings/media/ti,da850-vpif.txt
@@ -30,15 +30,15 @@ I2C-connected TVP5147 decoder:
 
port {
vpif_ch0: endpoint@0 {
- reg = <0>;
- bus-width = <8>;
- remote-endpoint = <>;
+   reg = <0>;
+   bus-width = <8>;
+   remote-endpoint = <>;
};
 
vpif_ch1: endpoint@1 {
- reg = <1>;
- bus-width = <8>;
- data-shift = <8>;
+   reg = <1>;
+   bus-width = <8>;
+   data-shift = <8>;
};
};
};
-- 
2.9.3

[PATCH v3 2/2] media: dt-bindings: vpif: extend the example with an output port

[Bartosz Golaszewski]

This makes the example more or less correspond with the da850-evm
hardware setup.

Signed-off-by: Bartosz Golaszewski 
Acked-by: Rob Herring 
---
 .../devicetree/bindings/media/ti,da850-vpif.txt| 40 +-
 1 file changed, 32 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/ti,da850-vpif.txt 
b/Documentation/devicetree/bindings/media/ti,da850-vpif.txt
index 9c7510b..df7182a 100644
--- a/Documentation/devicetree/bindings/media/ti,da850-vpif.txt
+++ b/Documentation/devicetree/bindings/media/ti,da850-vpif.txt
@@ -16,8 +16,10 @@ Required properties:
 Video Capture:
 
 VPIF has a 16-bit parallel bus input, supporting 2 8-bit channels or a
-single 16-bit channel.  It should contain at least one port child node
-with child 'endpoint' node. Please refer to the bindings defined in
+single 16-bit channel. It should contain one or two port child nodes
+with child 'endpoint' node. If there are two ports then port@0 must
+describe the input and port@1 output channels. Please refer to the
+bindings defined in
 Documentation/devicetree/bindings/media/video-interfaces.txt.
 
 Example using 2 8-bit input channels, one of which is connected to an
@@ -28,19 +30,26 @@ I2C-connected TVP5147 decoder:
reg = <0x217000 0x1000>;
interrupts = <92>;
 
-   port {
-   vpif_ch0: endpoint@0 {
+   port@0 {
+   vpif_input_ch0: endpoint@0 {
reg = <0>;
bus-width = <8>;
-   remote-endpoint = <>;
+   remote-endpoint = <_in>;
};
 
-   vpif_ch1: endpoint@1 {
+   vpif_input_ch1: endpoint@1 {
reg = <1>;
bus-width = <8>;
data-shift = <8>;
};
};
+
+   port@1 {
+   vpif_output_ch0: endpoint {
+   bus-width = <8>;
+   remote-endpoint = <_out>;
+   };
+   };
};
 
 [ ... ]
@@ -53,13 +62,28 @@ I2C-connected TVP5147 decoder:
status = "okay";
 
port {
-   composite: endpoint {
+   composite_in: endpoint {
hsync-active = <1>;
vsync-active = <1>;
pclk-sample = <0>;
 
/* VPIF channel 0 (lower 8-bits) */
-   remote-endpoint = <_ch0>;
+   remote-endpoint = <_input_ch0>;
+   bus-width = <8>;
+   };
+   };
+   };
+
+   adv7343@2a {
+   compatible = "adi,adv7343";
+   reg = <0x2a>;
+
+   port {
+   composite_out: endpoint {
+   adi,dac-enable = <1 1 1>;
+   adi,sd-dac-enable = <1>;
+
+   remote-endpoint = <_output_ch0>;
bus-width = <8>;
};
};
-- 
2.9.3

[PATCH v3 7/8] v4l: Define a pixel format for the R-Car VSP1 2-D histogram engine

[Laurent Pinchart]

From: Niklas Söderlund 

The format is used on the R-Car VSP1 video queues that carry
2-D histogram statistics data.

Signed-off-by: Niklas Söderlund 
Reviewed-by: Laurent Pinchart 
Signed-off-by: Laurent Pinchart 
---
 Documentation/media/uapi/v4l/meta-formats.rst  |   1 +
 .../media/uapi/v4l/pixfmt-meta-vsp1-hgt.rst| 120 +
 drivers/media/v4l2-core/v4l2-ioctl.c   |   1 +
 include/uapi/linux/videodev2.h |   3 +-
 4 files changed, 124 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/media/uapi/v4l/pixfmt-meta-vsp1-hgt.rst

diff --git a/Documentation/media/uapi/v4l/meta-formats.rst 
b/Documentation/media/uapi/v4l/meta-formats.rst
index 05ab91e12f10..01e24e3df571 100644
--- a/Documentation/media/uapi/v4l/meta-formats.rst
+++ b/Documentation/media/uapi/v4l/meta-formats.rst
@@ -13,3 +13,4 @@ These formats are used for the :ref:`metadata` interface only.
 :maxdepth: 1
 
 pixfmt-meta-vsp1-hgo
+pixfmt-meta-vsp1-hgt
diff --git a/Documentation/media/uapi/v4l/pixfmt-meta-vsp1-hgt.rst 
b/Documentation/media/uapi/v4l/pixfmt-meta-vsp1-hgt.rst
new file mode 100644
index ..fb9f79466319
--- /dev/null
+++ b/Documentation/media/uapi/v4l/pixfmt-meta-vsp1-hgt.rst
@@ -0,0 +1,120 @@
+.. -*- coding: utf-8; mode: rst -*-
+
+.. _v4l2-meta-fmt-vsp1-hgt:
+
+***
+V4L2_META_FMT_VSP1_HGT ('VSPT')
+***
+
+Renesas R-Car VSP1 2-D Histogram Data
+
+
+Description
+===
+
+This format describes histogram data generated by the Renesas R-Car VSP1
+2-D Histogram (HGT) engine.
+
+The VSP1 HGT is a histogram computation engine that operates on HSV
+data. It operates on a possibly cropped and subsampled input image and
+computes the sum, maximum and minimum of the S component as well as a
+weighted frequency histogram based on the H and S components.
+
+The histogram is a matrix of 6 Hue and 32 Saturation buckets, 192 in
+total. Each HSV value is added to one or more buckets with a weight
+between 1 and 16 depending on the Hue areas configuration. Finding the
+corresponding buckets is done by inspecting the H and S value independently.
+
+The Saturation position **n** (0 - 31) of the bucket in the matrix is
+found by the expression:
+
+n = S / 8
+
+The Hue position **m** (0 - 5) of the bucket in the matrix depends on
+how the HGT Hue areas are configured. There are 6 user configurable Hue
+Areas which can be configured to cover overlapping Hue values:
+
+::
+
+ Area 0   Area 1   Area 2   Area 3   Area 4   Area 
5
+     

+   \   /|  |\   /|  |\   /|  |\   /|  |\   /|  |\   /| 
 |\   /
+\ / |  | \ / |  | \ / |  | \ / |  | \ / |  | \ / | 
 | \ /
+ X  |  |  X  |  |  X  |  |  X  |  |  X  |  |  X  | 
 |  X
+/ \ |  | / \ |  | / \ |  | / \ |  | / \ |  | / \ | 
 | / \
+   /   \|  |/   \|  |/   \|  |/   \|  |/   \|  |/   \| 
 |/   \
+  5U   0L  0U   1L  1U   2L  2U   3L  3U   4L  4U   5L 
 5U   0L
+<0..Hue 
Value255>
+
+When two consecutive areas don't overlap (n+1L is equal to nU) the boundary
+value is considered as part of the lower area.
+
+Pixels with a hue value included in the centre of an area (between nL and nU
+included) are attributed to that single area and given a weight of 16. Pixels
+with a hue value included in the overlapping region between two areas (between
+n+1L and nU excluded) are attributed to both areas and given a weight for each
+of these areas proportional to their position along the diagonal lines
+(rounded down).
+
+The Hue area setup must match one of the following constrains:
+
+::
+
+0L <= 0U <= 1L <= 1U <= 2L <= 2U <= 3L <= 3U <= 4L <= 4U <= 5L <= 5U
+
+::
+
+0U <= 1L <= 1U <= 2L <= 2U <= 3L <= 3U <= 4L <= 4U <= 5L <= 5U <= 0L
+
+**Byte Order.**
+All data is stored in memory in little endian format. Each cell in the tables
+contains one byte.
+
+.. flat-table:: VSP1 HGT Data - (776 bytes)
+:header-rows:  2
+:stub-columns: 0
+
+* - Offset
+  - :cspan:`4` Memory
+* -
+  - [31:24]
+  - [23:16]
+  - [15:8]
+  - [7:0]
+* - 0
+  - -
+  - S max [7:0]
+  - -
+  - S min [7:0]
+* - 4
+  - :cspan:`4` S sum [31:0]
+* - 8
+  - :cspan:`4` Histogram bucket (m=0, n=0) [31:0]
+* - 12
+  - :cspan:`4` Histogram bucket (m=0, n=1) [31:0]
+* -
+  - :cspan:`4` ...
+* - 132
+  - :cspan:`4` Histogram bucket (m=0, n=31) [31:0]
+* - 136
+  - :cspan:`4` Histogram bucket (m=1, n=0) [31:0]
+* -

[PATCH v3 8/8] v4l: vsp1: Add HGT support

[Laurent Pinchart]

From: Niklas Söderlund 

The HGT is a Histogram Generator Two-Dimensions. It computes a weighted
frequency histograms for hue and saturation areas over a configurable
region of the image with optional subsampling.

Signed-off-by: Niklas Söderlund 
Reviewed-by: Laurent Pinchart 
Signed-off-by: Laurent Pinchart 
---
 drivers/media/platform/vsp1/Makefile  |   2 +-
 drivers/media/platform/vsp1/vsp1.h|   3 +
 drivers/media/platform/vsp1/vsp1_drv.c|  32 -
 drivers/media/platform/vsp1/vsp1_entity.c |  14 ++
 drivers/media/platform/vsp1/vsp1_hgt.c| 222 ++
 drivers/media/platform/vsp1/vsp1_hgt.h|  42 ++
 drivers/media/platform/vsp1/vsp1_pipe.c   |  16 +++
 drivers/media/platform/vsp1/vsp1_pipe.h   |   2 +
 drivers/media/platform/vsp1/vsp1_regs.h   |   9 ++
 drivers/media/platform/vsp1/vsp1_video.c  |   6 +
 10 files changed, 343 insertions(+), 5 deletions(-)
 create mode 100644 drivers/media/platform/vsp1/vsp1_hgt.c
 create mode 100644 drivers/media/platform/vsp1/vsp1_hgt.h

diff --git a/drivers/media/platform/vsp1/Makefile 
b/drivers/media/platform/vsp1/Makefile
index 8ab6a063569e..a33afc385a48 100644
--- a/drivers/media/platform/vsp1/Makefile
+++ b/drivers/media/platform/vsp1/Makefile
@@ -3,7 +3,7 @@ vsp1-y  += vsp1_dl.o vsp1_drm.o 
vsp1_video.o
 vsp1-y += vsp1_rpf.o vsp1_rwpf.o vsp1_wpf.o
 vsp1-y += vsp1_clu.o vsp1_hsit.o vsp1_lut.o
 vsp1-y += vsp1_bru.o vsp1_sru.o vsp1_uds.o
-vsp1-y += vsp1_hgo.o vsp1_histo.o
+vsp1-y += vsp1_hgo.o vsp1_hgt.o vsp1_histo.o
 vsp1-y += vsp1_lif.o
 
 obj-$(CONFIG_VIDEO_RENESAS_VSP1)   += vsp1.o
diff --git a/drivers/media/platform/vsp1/vsp1.h 
b/drivers/media/platform/vsp1/vsp1.h
index 0ba7521c01b4..85387a64179a 100644
--- a/drivers/media/platform/vsp1/vsp1.h
+++ b/drivers/media/platform/vsp1/vsp1.h
@@ -33,6 +33,7 @@ struct vsp1_platform_data;
 struct vsp1_bru;
 struct vsp1_clu;
 struct vsp1_hgo;
+struct vsp1_hgt;
 struct vsp1_hsit;
 struct vsp1_lif;
 struct vsp1_lut;
@@ -52,6 +53,7 @@ struct vsp1_uds;
 #define VSP1_HAS_WPF_VFLIP (1 << 5)
 #define VSP1_HAS_WPF_HFLIP (1 << 6)
 #define VSP1_HAS_HGO   (1 << 7)
+#define VSP1_HAS_HGT   (1 << 8)
 
 struct vsp1_device_info {
u32 version;
@@ -76,6 +78,7 @@ struct vsp1_device {
struct vsp1_bru *bru;
struct vsp1_clu *clu;
struct vsp1_hgo *hgo;
+   struct vsp1_hgt *hgt;
struct vsp1_hsit *hsi;
struct vsp1_hsit *hst;
struct vsp1_lif *lif;
diff --git a/drivers/media/platform/vsp1/vsp1_drv.c 
b/drivers/media/platform/vsp1/vsp1_drv.c
index 0acc8ed6ac59..048446af5ae7 100644
--- a/drivers/media/platform/vsp1/vsp1_drv.c
+++ b/drivers/media/platform/vsp1/vsp1_drv.c
@@ -31,6 +31,7 @@
 #include "vsp1_dl.h"
 #include "vsp1_drm.h"
 #include "vsp1_hgo.h"
+#include "vsp1_hgt.h"
 #include "vsp1_hsit.h"
 #include "vsp1_lif.h"
 #include "vsp1_lut.h"
@@ -161,6 +162,16 @@ static int vsp1_uapi_create_links(struct vsp1_device *vsp1)
return ret;
}
 
+   if (vsp1->hgt) {
+   ret = 
media_create_pad_link(>hgt->histo.entity.subdev.entity,
+   HISTO_PAD_SOURCE,
+   >hgt->histo.video.entity, 0,
+   MEDIA_LNK_FL_ENABLED |
+   MEDIA_LNK_FL_IMMUTABLE);
+   if (ret < 0)
+   return ret;
+   }
+
if (vsp1->lif) {
ret = media_create_pad_link(>wpf[0]->entity.subdev.entity,
RWPF_PAD_SOURCE,
@@ -305,6 +316,17 @@ static int vsp1_create_entities(struct vsp1_device *vsp1)
  >entities);
}
 
+   if (vsp1->info->features & VSP1_HAS_HGT && vsp1->info->uapi) {
+   vsp1->hgt = vsp1_hgt_create(vsp1);
+   if (IS_ERR(vsp1->hgt)) {
+   ret = PTR_ERR(vsp1->hgt);
+   goto done;
+   }
+
+   list_add_tail(>hgt->histo.entity.list_dev,
+ >entities);
+   }
+
/*
 * The LIF is only supported when used in conjunction with the DU, in
 * which case the userspace API is disabled. If the userspace API is
@@ -591,7 +613,8 @@ static const struct vsp1_device_info vsp1_device_infos[] = {
.model = "VSP1-S",
.gen = 2,
.features = VSP1_HAS_BRU | VSP1_HAS_CLU | VSP1_HAS_HGO
- | VSP1_HAS_LUT | VSP1_HAS_SRU | VSP1_HAS_WPF_VFLIP,
+  

[PATCH v3 4/8] v4l: vsp1: Fix HGO and HGT routing register addresses

[Laurent Pinchart]

The addresses are incorrect, fix them.

Signed-off-by: Laurent Pinchart 
---
 drivers/media/platform/vsp1/vsp1_regs.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_regs.h 
b/drivers/media/platform/vsp1/vsp1_regs.h
index 47b1dee044fb..61369e267667 100644
--- a/drivers/media/platform/vsp1/vsp1_regs.h
+++ b/drivers/media/platform/vsp1/vsp1_regs.h
@@ -328,8 +328,8 @@
 #define VI6_DPR_ROUTE_RT_MASK  (0x3f << 0)
 #define VI6_DPR_ROUTE_RT_SHIFT 0
 
-#define VI6_DPR_HGO_SMPPT  0x2050
-#define VI6_DPR_HGT_SMPPT  0x2054
+#define VI6_DPR_HGO_SMPPT  0x2054
+#define VI6_DPR_HGT_SMPPT  0x2058
 #define VI6_DPR_SMPPT_TGW_MASK (7 << 8)
 #define VI6_DPR_SMPPT_TGW_SHIFT8
 #define VI6_DPR_SMPPT_PT_MASK  (0x3f << 0)
-- 
Regards,

Laurent Pinchart

[PATCH v3 6/8] v4l: vsp1: Add HGO support

[Laurent Pinchart]

The HGO is a Histogram Generator One-Dimension. It computes per-channel
histograms over a configurable region of the image with optional
subsampling.

Signed-off-by: Laurent Pinchart 
---
 drivers/media/platform/vsp1/Makefile  |   2 +-
 drivers/media/platform/vsp1/vsp1.h|   3 +
 drivers/media/platform/vsp1/vsp1_drv.c|  42 --
 drivers/media/platform/vsp1/vsp1_entity.c |  16 +++
 drivers/media/platform/vsp1/vsp1_hgo.c| 228 ++
 drivers/media/platform/vsp1/vsp1_hgo.h|  45 ++
 drivers/media/platform/vsp1/vsp1_pipe.c   |  16 +++
 drivers/media/platform/vsp1/vsp1_pipe.h   |   2 +
 drivers/media/platform/vsp1/vsp1_regs.h   |  20 ++-
 drivers/media/platform/vsp1/vsp1_video.c  |   6 +
 10 files changed, 367 insertions(+), 13 deletions(-)
 create mode 100644 drivers/media/platform/vsp1/vsp1_hgo.c
 create mode 100644 drivers/media/platform/vsp1/vsp1_hgo.h

diff --git a/drivers/media/platform/vsp1/Makefile 
b/drivers/media/platform/vsp1/Makefile
index c559536f7867..8ab6a063569e 100644
--- a/drivers/media/platform/vsp1/Makefile
+++ b/drivers/media/platform/vsp1/Makefile
@@ -3,7 +3,7 @@ vsp1-y  += vsp1_dl.o vsp1_drm.o 
vsp1_video.o
 vsp1-y += vsp1_rpf.o vsp1_rwpf.o vsp1_wpf.o
 vsp1-y += vsp1_clu.o vsp1_hsit.o vsp1_lut.o
 vsp1-y += vsp1_bru.o vsp1_sru.o vsp1_uds.o
-vsp1-y += vsp1_histo.o
+vsp1-y += vsp1_hgo.o vsp1_histo.o
 vsp1-y += vsp1_lif.o
 
 obj-$(CONFIG_VIDEO_RENESAS_VSP1)   += vsp1.o
diff --git a/drivers/media/platform/vsp1/vsp1.h 
b/drivers/media/platform/vsp1/vsp1.h
index b23fa879a9aa..0ba7521c01b4 100644
--- a/drivers/media/platform/vsp1/vsp1.h
+++ b/drivers/media/platform/vsp1/vsp1.h
@@ -32,6 +32,7 @@ struct vsp1_entity;
 struct vsp1_platform_data;
 struct vsp1_bru;
 struct vsp1_clu;
+struct vsp1_hgo;
 struct vsp1_hsit;
 struct vsp1_lif;
 struct vsp1_lut;
@@ -50,6 +51,7 @@ struct vsp1_uds;
 #define VSP1_HAS_CLU   (1 << 4)
 #define VSP1_HAS_WPF_VFLIP (1 << 5)
 #define VSP1_HAS_WPF_HFLIP (1 << 6)
+#define VSP1_HAS_HGO   (1 << 7)
 
 struct vsp1_device_info {
u32 version;
@@ -73,6 +75,7 @@ struct vsp1_device {
 
struct vsp1_bru *bru;
struct vsp1_clu *clu;
+   struct vsp1_hgo *hgo;
struct vsp1_hsit *hsi;
struct vsp1_hsit *hst;
struct vsp1_lif *lif;
diff --git a/drivers/media/platform/vsp1/vsp1_drv.c 
b/drivers/media/platform/vsp1/vsp1_drv.c
index 83a6669a6328..0acc8ed6ac59 100644
--- a/drivers/media/platform/vsp1/vsp1_drv.c
+++ b/drivers/media/platform/vsp1/vsp1_drv.c
@@ -30,6 +30,7 @@
 #include "vsp1_clu.h"
 #include "vsp1_dl.h"
 #include "vsp1_drm.h"
+#include "vsp1_hgo.h"
 #include "vsp1_hsit.h"
 #include "vsp1_lif.h"
 #include "vsp1_lut.h"
@@ -150,6 +151,16 @@ static int vsp1_uapi_create_links(struct vsp1_device *vsp1)
return ret;
}
 
+   if (vsp1->hgo) {
+   ret = 
media_create_pad_link(>hgo->histo.entity.subdev.entity,
+   HISTO_PAD_SOURCE,
+   >hgo->histo.video.entity, 0,
+   MEDIA_LNK_FL_ENABLED |
+   MEDIA_LNK_FL_IMMUTABLE);
+   if (ret < 0)
+   return ret;
+   }
+
if (vsp1->lif) {
ret = media_create_pad_link(>wpf[0]->entity.subdev.entity,
RWPF_PAD_SOURCE,
@@ -283,6 +294,17 @@ static int vsp1_create_entities(struct vsp1_device *vsp1)
 
list_add_tail(>hst->entity.list_dev, >entities);
 
+   if (vsp1->info->features & VSP1_HAS_HGO && vsp1->info->uapi) {
+   vsp1->hgo = vsp1_hgo_create(vsp1);
+   if (IS_ERR(vsp1->hgo)) {
+   ret = PTR_ERR(vsp1->hgo);
+   goto done;
+   }
+
+   list_add_tail(>hgo->histo.entity.list_dev,
+ >entities);
+   }
+
/*
 * The LIF is only supported when used in conjunction with the DU, in
 * which case the userspace API is disabled. If the userspace API is
@@ -568,8 +590,8 @@ static const struct vsp1_device_info vsp1_device_infos[] = {
.version = VI6_IP_VERSION_MODEL_VSPS_H2,
.model = "VSP1-S",
.gen = 2,
-   .features = VSP1_HAS_BRU | VSP1_HAS_CLU | VSP1_HAS_LUT
- | VSP1_HAS_SRU | VSP1_HAS_WPF_VFLIP,
+   .features = VSP1_HAS_BRU | VSP1_HAS_CLU | VSP1_HAS_HGO
+ | VSP1_HAS_LUT | VSP1_HAS_SRU | VSP1_HAS_WPF_VFLIP,
.rpf_count = 5,
.uds_count = 3,
.wpf_count = 4,
@@ 

[PATCH v3 0/8] R-Car VSP1 Histogram Support

[Laurent Pinchart]

Hello,

This patch series implements support for the Renesas R-Car VSP1 1-D and 2-D
histogram generators (HGO and HGT). It is based on top of the VSP1 rotation
patches and available for convenience at

git://linuxtv.org/pinchartl/media.git vsp1-histogram-v3-20170228

The series starts with the implementation and documentation of the new V4L2
metadata API (1/8), followed by three VSP1 patches that add generic histogram
support to the driver (2/8 to 4/8). Patches 5/8 and 7/8 then add new pixel
formats for the R-Car VSP1 1-D and 2-D histograms, and patches 6/8 and 8/8
implement support for those histogram generators in the VSP1 driver.

Laurent Pinchart (6):
  v4l: Add metadata buffer type and format
  v4l: vsp1: Add histogram support
  v4l: vsp1: Support histogram generators in pipeline configuration
  v4l: vsp1: Fix HGO and HGT routing register addresses
  v4l: Define a pixel format for the R-Car VSP1 1-D histogram engine
  v4l: vsp1: Add HGO support

Niklas Söderlund (2):
  v4l: Define a pixel format for the R-Car VSP1 2-D histogram engine
  v4l: vsp1: Add HGT support

 Documentation/media/uapi/v4l/buffer.rst|   3 +
 Documentation/media/uapi/v4l/dev-meta.rst  |  62 ++
 Documentation/media/uapi/v4l/devices.rst   |   1 +
 Documentation/media/uapi/v4l/meta-formats.rst  |  16 +
 .../media/uapi/v4l/pixfmt-meta-vsp1-hgo.rst| 168 ++
 .../media/uapi/v4l/pixfmt-meta-vsp1-hgt.rst| 120 
 Documentation/media/uapi/v4l/pixfmt.rst|   1 +
 Documentation/media/uapi/v4l/vidioc-querycap.rst   |   3 +
 Documentation/media/videodev2.h.rst.exceptions |   2 +
 drivers/media/platform/Kconfig |   1 +
 drivers/media/platform/vsp1/Makefile   |   1 +
 drivers/media/platform/vsp1/vsp1.h |   6 +
 drivers/media/platform/vsp1/vsp1_drm.c |   2 +-
 drivers/media/platform/vsp1/vsp1_drv.c |  70 ++-
 drivers/media/platform/vsp1/vsp1_entity.c  | 154 -
 drivers/media/platform/vsp1/vsp1_entity.h  |   8 +-
 drivers/media/platform/vsp1/vsp1_hgo.c | 228 
 drivers/media/platform/vsp1/vsp1_hgo.h |  45 ++
 drivers/media/platform/vsp1/vsp1_hgt.c | 222 +++
 drivers/media/platform/vsp1/vsp1_hgt.h |  42 ++
 drivers/media/platform/vsp1/vsp1_histo.c   | 646 +
 drivers/media/platform/vsp1/vsp1_histo.h   |  84 +++
 drivers/media/platform/vsp1/vsp1_pipe.c|  38 +-
 drivers/media/platform/vsp1/vsp1_pipe.h|   4 +
 drivers/media/platform/vsp1/vsp1_regs.h|  33 +-
 drivers/media/platform/vsp1/vsp1_video.c   |  30 +-
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c  |  19 +
 drivers/media/v4l2-core/v4l2-dev.c |  16 +-
 drivers/media/v4l2-core/v4l2-ioctl.c   |  36 ++
 drivers/media/v4l2-core/videobuf2-v4l2.c   |   3 +
 include/media/v4l2-ioctl.h |  17 +
 include/trace/events/v4l2.h|   1 +
 include/uapi/linux/videodev2.h |  17 +
 33 files changed, 2051 insertions(+), 48 deletions(-)
 create mode 100644 Documentation/media/uapi/v4l/dev-meta.rst
 create mode 100644 Documentation/media/uapi/v4l/meta-formats.rst
 create mode 100644 Documentation/media/uapi/v4l/pixfmt-meta-vsp1-hgo.rst
 create mode 100644 Documentation/media/uapi/v4l/pixfmt-meta-vsp1-hgt.rst
 create mode 100644 drivers/media/platform/vsp1/vsp1_hgo.c
 create mode 100644 drivers/media/platform/vsp1/vsp1_hgo.h
 create mode 100644 drivers/media/platform/vsp1/vsp1_hgt.c
 create mode 100644 drivers/media/platform/vsp1/vsp1_hgt.h
 create mode 100644 drivers/media/platform/vsp1/vsp1_histo.c
 create mode 100644 drivers/media/platform/vsp1/vsp1_histo.h

-- 
Regards,

Laurent Pinchart

[PATCH v3 5/8] v4l: Define a pixel format for the R-Car VSP1 1-D histogram engine

[Laurent Pinchart]

The format is used on the R-Car VSP1 video queues that carry
1-D histogram statistics data.

Signed-off-by: Laurent Pinchart 
Acked-by: Sakari Ailus 
---
 Documentation/media/uapi/v4l/meta-formats.rst  |  15 ++
 .../media/uapi/v4l/pixfmt-meta-vsp1-hgo.rst| 168 +
 Documentation/media/uapi/v4l/pixfmt.rst|   1 +
 drivers/media/v4l2-core/v4l2-ioctl.c   |   1 +
 include/uapi/linux/videodev2.h |   3 +
 5 files changed, 188 insertions(+)
 create mode 100644 Documentation/media/uapi/v4l/meta-formats.rst
 create mode 100644 Documentation/media/uapi/v4l/pixfmt-meta-vsp1-hgo.rst

diff --git a/Documentation/media/uapi/v4l/meta-formats.rst 
b/Documentation/media/uapi/v4l/meta-formats.rst
new file mode 100644
index ..05ab91e12f10
--- /dev/null
+++ b/Documentation/media/uapi/v4l/meta-formats.rst
@@ -0,0 +1,15 @@
+.. -*- coding: utf-8; mode: rst -*-
+
+.. _meta-formats:
+
+
+Metadata Formats
+
+
+These formats are used for the :ref:`metadata` interface only.
+
+
+.. toctree::
+:maxdepth: 1
+
+pixfmt-meta-vsp1-hgo
diff --git a/Documentation/media/uapi/v4l/pixfmt-meta-vsp1-hgo.rst 
b/Documentation/media/uapi/v4l/pixfmt-meta-vsp1-hgo.rst
new file mode 100644
index ..8d37bb313493
--- /dev/null
+++ b/Documentation/media/uapi/v4l/pixfmt-meta-vsp1-hgo.rst
@@ -0,0 +1,168 @@
+.. -*- coding: utf-8; mode: rst -*-
+
+.. _v4l2-meta-fmt-vsp1-hgo:
+
+***
+V4L2_META_FMT_VSP1_HGO ('VSPH')
+***
+
+Renesas R-Car VSP1 1-D Histogram Data
+
+
+Description
+===
+
+This format describes histogram data generated by the Renesas R-Car VSP1 1-D
+Histogram (HGO) engine.
+
+The VSP1 HGO is a histogram computation engine that can operate on RGB, YCrCb
+or HSV data. It operates on a possibly cropped and subsampled input image and
+computes the minimum, maximum and sum of all pixels as well as per-channel
+histograms.
+
+The HGO can compute histograms independently per channel, on the maximum of the
+three channels (RGB data only) or on the Y channel only (YCbCr only). It can
+additionally output the histogram with 64 or 256 bins, resulting in four
+possible modes of operation.
+
+- In *64 bins normal mode*, the HGO operates on the three channels 
independently
+  to compute three 64-bins histograms. RGB, YCbCr and HSV image formats are
+  supported.
+- In *64 bins maximum mode*, the HGO operates on the maximum of the (R, G, B)
+  channels to compute a single 64-bins histogram. Only the RGB image format is
+  supported.
+- In *256 bins normal mode*, the HGO operates on the Y channel to compute a
+  single 256-bins histogram. Only the YCbCr image format is supported.
+- In *256 bins maximum mode*, the HGO operates on the maximum of the (R, G, B)
+  channels to compute a single 256-bins histogram. Only the RGB image format is
+  supported.
+
+**Byte Order.**
+All data is stored in memory in little endian format. Each cell in the tables
+contains one byte.
+
+.. flat-table:: VSP1 HGO Data - 64 Bins, Normal Mode (792 bytes)
+:header-rows:  2
+:stub-columns: 0
+
+* - Offset
+  - :cspan:`4` Memory
+* -
+  - [31:24]
+  - [23:16]
+  - [15:8]
+  - [7:0]
+* - 0
+  - -
+  - R/Cr/H max [7:0]
+  - -
+  - R/Cr/H min [7:0]
+* - 4
+  - -
+  - G/Y/S max [7:0]
+  - -
+  - G/Y/S min [7:0]
+* - 8
+  - -
+  - B/Cb/V max [7:0]
+  - -
+  - B/Cb/V min [7:0]
+* - 12
+  - :cspan:`4` R/Cr/H sum [31:0]
+* - 16
+  - :cspan:`4` G/Y/S sum [31:0]
+* - 20
+  - :cspan:`4` B/Cb/V sum [31:0]
+* - 24
+  - :cspan:`4` R/Cr/H bin 0 [31:0]
+* -
+  - :cspan:`4` ...
+* - 276
+  - :cspan:`4` R/Cr/H bin 63 [31:0]
+* - 280
+  - :cspan:`4` G/Y/S bin 0 [31:0]
+* -
+  - :cspan:`4` ...
+* - 532
+  - :cspan:`4` G/Y/S bin 63 [31:0]
+* - 536
+  - :cspan:`4` B/Cb/V bin 0 [31:0]
+* -
+  - :cspan:`4` ...
+* - 788
+  - :cspan:`4` B/Cb/V bin 63 [31:0]
+
+.. flat-table:: VSP1 HGO Data - 64 Bins, Max Mode (264 bytes)
+:header-rows:  2
+:stub-columns: 0
+
+* - Offset
+  - :cspan:`4` Memory
+* -
+  - [31:24]
+  - [23:16]
+  - [15:8]
+  - [7:0]
+* - 0
+  - -
+  - max(R,G,B) max [7:0]
+  - -
+  - max(R,G,B) min [7:0]
+* - 4
+  - :cspan:`4` max(R,G,B) sum [31:0]
+* - 8
+  - :cspan:`4` max(R,G,B) bin 0 [31:0]
+* -
+  - :cspan:`4` ...
+* - 260
+  - :cspan:`4` max(R,G,B) bin 63 [31:0]
+
+.. flat-table:: VSP1 HGO Data - 256 Bins, Normal Mode (1032 bytes)
+:header-rows:  2
+:stub-columns: 0
+
+* - Offset
+  - :cspan:`4` Memory
+* -
+  - [31:24]
+  - [23:16]
+  - [15:8]
+  - [7:0]
+* - 0
+  - -
+  - Y max [7:0]
+  - -
+  

[PATCH v3 1/8] v4l: Add metadata buffer type and format

[Laurent Pinchart]

The metadata buffer type is used to transfer metadata between userspace
and kernelspace through a V4L2 buffers queue. It comes with a new
metadata capture capability and format description.

Signed-off-by: Laurent Pinchart 
Tested-by: Guennadi Liakhovetski 
Acked-by: Sakari Ailus 
Acked-by: Hans Verkuil 
---
 Documentation/media/uapi/v4l/buffer.rst  |  3 ++
 Documentation/media/uapi/v4l/dev-meta.rst| 62 
 Documentation/media/uapi/v4l/devices.rst |  1 +
 Documentation/media/uapi/v4l/vidioc-querycap.rst |  3 ++
 Documentation/media/videodev2.h.rst.exceptions   |  2 +
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c| 19 
 drivers/media/v4l2-core/v4l2-dev.c   | 16 +++---
 drivers/media/v4l2-core/v4l2-ioctl.c | 34 +
 drivers/media/v4l2-core/videobuf2-v4l2.c |  3 ++
 include/media/v4l2-ioctl.h   | 17 +++
 include/trace/events/v4l2.h  |  1 +
 include/uapi/linux/videodev2.h   | 13 +
 12 files changed, 168 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/media/uapi/v4l/dev-meta.rst

diff --git a/Documentation/media/uapi/v4l/buffer.rst 
b/Documentation/media/uapi/v4l/buffer.rst
index 5c58db98ab7a..02834ce7fa4d 100644
--- a/Documentation/media/uapi/v4l/buffer.rst
+++ b/Documentation/media/uapi/v4l/buffer.rst
@@ -418,6 +418,9 @@ enum v4l2_buf_type
   - 12
   - Buffer for Software Defined Radio (SDR) output stream, see
:ref:`sdr`.
+* - ``V4L2_BUF_TYPE_META_CAPTURE``
+  - 13
+  - Buffer for metadata capture, see :ref:`metadata`.
 
 
 
diff --git a/Documentation/media/uapi/v4l/dev-meta.rst 
b/Documentation/media/uapi/v4l/dev-meta.rst
new file mode 100644
index ..b6044c54082a
--- /dev/null
+++ b/Documentation/media/uapi/v4l/dev-meta.rst
@@ -0,0 +1,62 @@
+.. -*- coding: utf-8; mode: rst -*-
+
+.. _metadata:
+
+**
+Metadata Interface
+**
+
+Metadata refers to any non-image data that supplements video frames with
+additional information. This may include statistics computed over the image
+or frame capture parameters supplied by the image source. This interface is
+intended for transfer of metadata to userspace and control of that operation.
+
+The metadata interface is implemented on video capture device nodes. The device
+can be dedicated to metadata or can implement both video and metadata capture
+as specified in its reported capabilities.
+
+.. note::
+
+This is an :ref:`experimental` interface and may
+change in the future.
+
+Querying Capabilities
+=
+
+Device nodes supporting the metadata interface set the 
``V4L2_CAP_META_CAPTURE``
+flag in the ``device_caps`` field of the
+:c:type:`v4l2_capability` structure returned by the :c:func:`VIDIOC_QUERYCAP`
+ioctl. That flag means the device can capture metadata to memory.
+
+At least one of the read/write or streaming I/O methods must be supported.
+
+
+Data Format Negotiation
+===
+
+The metadata device uses the :ref:`format` ioctls to select the capture format.
+The metadata buffer content format is bound to that selected format. In 
addition
+to the basic :ref:`format` ioctls, the :c:func:`VIDIOC_ENUM_FMT` ioctl must be
+supported as well.
+
+To use the :ref:`format` ioctls applications set the ``type`` field of the
+:c:type:`v4l2_format` structure to ``V4L2_BUF_TYPE_META_CAPTURE`` and use the
+:c:type:`v4l2_meta_format` ``meta`` member of the ``fmt`` union as needed per
+the desired operation. Both drivers and applications must set the remainder of
+the :c:type:`v4l2_format` structure to 0.
+
+.. _v4l2-meta-format:
+.. flat-table:: struct v4l2_meta_format
+:header-rows:  0
+:stub-columns: 0
+:widths:   1 1 2
+
+* - __u32
+  - ``dataformat``
+  - The data format, set by the application. This is a little endian
+:ref:`four character code `. V4L2 defines metadata formats
+in :ref:`meta-formats`.
+* - __u32
+  - ``buffersize``
+  - Maximum buffer size in bytes required for data. The value is set by the
+driver.
diff --git a/Documentation/media/uapi/v4l/devices.rst 
b/Documentation/media/uapi/v4l/devices.rst
index 5c3d6c29e12c..fb7f8c26cf09 100644
--- a/Documentation/media/uapi/v4l/devices.rst
+++ b/Documentation/media/uapi/v4l/devices.rst
@@ -25,3 +25,4 @@ Interfaces
 dev-touch
 dev-event
 dev-subdev
+dev-meta
diff --git a/Documentation/media/uapi/v4l/vidioc-querycap.rst 
b/Documentation/media/uapi/v4l/vidioc-querycap.rst
index 165d8314327e..12e0d9a63cd8 100644
--- a/Documentation/media/uapi/v4l/vidioc-querycap.rst
+++ b/Documentation/media/uapi/v4l/vidioc-querycap.rst
@@ -236,6 +236,9 @@ specification the ioctl returns an ``EINVAL`` error code.
 * - 

[PATCH v2 3/3] v4l: vsp1: wpf: Implement rotation support

[Laurent Pinchart]

Some WPF instances, on Gen3 devices, can perform 90° rotation when
writing frames to memory. Implement support for this using the
V4L2_CID_ROTATE control.

Signed-off-by: Laurent Pinchart 
---
 drivers/media/platform/vsp1/vsp1_rpf.c   |   2 +-
 drivers/media/platform/vsp1/vsp1_rwpf.c  |   5 +
 drivers/media/platform/vsp1/vsp1_rwpf.h  |   3 +-
 drivers/media/platform/vsp1/vsp1_video.c |  12 +-
 drivers/media/platform/vsp1/vsp1_wpf.c   | 205 +++
 5 files changed, 175 insertions(+), 52 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_rpf.c 
b/drivers/media/platform/vsp1/vsp1_rpf.c
index f5a9a4c8c74d..8feddd59cf8d 100644
--- a/drivers/media/platform/vsp1/vsp1_rpf.c
+++ b/drivers/media/platform/vsp1/vsp1_rpf.c
@@ -106,7 +106,7 @@ static void rpf_configure(struct vsp1_entity *entity,
 * of the pipeline.
 */
output = vsp1_entity_get_pad_format(wpf, wpf->config,
-   RWPF_PAD_SOURCE);
+   RWPF_PAD_SINK);
 
crop.width = pipe->partition.width * input_width
   / output->width;
diff --git a/drivers/media/platform/vsp1/vsp1_rwpf.c 
b/drivers/media/platform/vsp1/vsp1_rwpf.c
index 7d52c88a583e..cfd8f1904fa6 100644
--- a/drivers/media/platform/vsp1/vsp1_rwpf.c
+++ b/drivers/media/platform/vsp1/vsp1_rwpf.c
@@ -121,6 +121,11 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev *subdev,
RWPF_PAD_SOURCE);
*format = fmt->format;
 
+   if (rwpf->flip.rotate) {
+   format->width = fmt->format.height;
+   format->height = fmt->format.width;
+   }
+
 done:
mutex_unlock(>entity.lock);
return ret;
diff --git a/drivers/media/platform/vsp1/vsp1_rwpf.h 
b/drivers/media/platform/vsp1/vsp1_rwpf.h
index 1c98aff3da5d..b4ffc38f48af 100644
--- a/drivers/media/platform/vsp1/vsp1_rwpf.h
+++ b/drivers/media/platform/vsp1/vsp1_rwpf.h
@@ -56,9 +56,10 @@ struct vsp1_rwpf {
 
struct {
spinlock_t lock;
-   struct v4l2_ctrl *ctrls[2];
+   struct v4l2_ctrl *ctrls[3];
unsigned int pending;
unsigned int active;
+   bool rotate;
} flip;
 
struct vsp1_rwpf_memory mem;
diff --git a/drivers/media/platform/vsp1/vsp1_video.c 
b/drivers/media/platform/vsp1/vsp1_video.c
index 5239e08fabc3..795a3ca9ca03 100644
--- a/drivers/media/platform/vsp1/vsp1_video.c
+++ b/drivers/media/platform/vsp1/vsp1_video.c
@@ -187,9 +187,13 @@ static void vsp1_video_pipeline_setup_partitions(struct 
vsp1_pipeline *pipe)
struct vsp1_entity *entity;
unsigned int div_size;
 
+   /*
+* Partitions are computed on the size before rotation, use the format
+* at the WPF sink.
+*/
format = vsp1_entity_get_pad_format(>output->entity,
pipe->output->entity.config,
-   RWPF_PAD_SOURCE);
+   RWPF_PAD_SINK);
div_size = format->width;
 
/* Gen2 hardware doesn't require image partitioning. */
@@ -229,9 +233,13 @@ static struct v4l2_rect vsp1_video_partition(struct 
vsp1_pipeline *pipe,
struct v4l2_rect partition;
unsigned int modulus;
 
+   /*
+* Partitions are computed on the size before rotation, use the format
+* at the WPF sink.
+*/
format = vsp1_entity_get_pad_format(>output->entity,
pipe->output->entity.config,
-   RWPF_PAD_SOURCE);
+   RWPF_PAD_SINK);
 
/* A single partition simply processes the output size in full. */
if (pipe->partitions <= 1) {
diff --git a/drivers/media/platform/vsp1/vsp1_wpf.c 
b/drivers/media/platform/vsp1/vsp1_wpf.c
index 25a2ed6e2e18..82935d97cad9 100644
--- a/drivers/media/platform/vsp1/vsp1_wpf.c
+++ b/drivers/media/platform/vsp1/vsp1_wpf.c
@@ -43,32 +43,94 @@ static inline void vsp1_wpf_write(struct vsp1_rwpf *wpf,
 enum wpf_flip_ctrl {
WPF_CTRL_VFLIP = 0,
WPF_CTRL_HFLIP = 1,
-   WPF_CTRL_MAX,
+   WPF_CTRL_ROTATE = 2,
 };
 
+static int vsp1_wpf_set_rotation(struct vsp1_rwpf *wpf, unsigned int rotation)
+{
+   struct vsp1_video *video = wpf->video;
+   struct v4l2_mbus_framefmt *sink_format;
+   struct v4l2_mbus_framefmt *source_format;
+   bool rotate;
+   int ret = 0;
+
+   /*
+* Only consider the 0°/180° from/to 90°/270° modifications, the rest
+* is taken care of by the flipping configuration.
+*/
+   rotate = rotation == 90 || rotation == 270;
+   if (rotate == wpf->flip.rotate)
+  

[PATCH v2 2/3] v4l: Clearly document interactions between formats, controls and buffers

[Laurent Pinchart]

V4L2 exposes parameters that influence buffers sizes through the format
ioctls (VIDIOC_G_FMT, VIDIOC_TRY_FMT and VIDIO_S_FMT). Other parameters
not part of the format structure may also influence buffer sizes or
buffer layout in general. One existing such parameter is rotation, which
is implemented by the VIDIOC_ROTATE control and thus exposed through the
V4L2 control ioctls.

The interaction between those parameters and buffers is currently only
partially specified by the V4L2 API. In particular interactions between
controls and buffers isn't specified at all. The behaviour of the
VIDIOC_S_FMT ioctl when buffers are allocated is also not fully
specified.

This commit clearly defines and documents the interactions between
formats, controls and buffers.

The preparatory discussions for the documentation change considered
completely disallowing controls that change the buffer size or layout,
in favour of extending the format API with a new ioctl that would bundle
those controls with format information. The idea has been rejected, as
this would essentially be a restricted version of the upcoming request
API that wouldn't bring any additional value.

Another option we have considered was to mandate the use of the request
API to modify controls that influence buffer size or layout. This has
also been rejected on the grounds that requiring the request API to
change rotation even when streaming is stopped would significantly
complicate implementation of drivers and usage of the V4L2 API for
applications.

Applications will however be required to use the upcoming request API to
change at runtime formats or controls that influence the buffer size or
layout, because of the need to synchronize buffers with the formats and
controls. Otherwise there would be no way to interpret the content of a
buffer correctly.

Signed-off-by: Laurent Pinchart 
---
 Documentation/media/uapi/v4l/buffer.rst | 88 +
 1 file changed, 88 insertions(+)

diff --git a/Documentation/media/uapi/v4l/buffer.rst 
b/Documentation/media/uapi/v4l/buffer.rst
index ac58966ccb9b..5c58db98ab7a 100644
--- a/Documentation/media/uapi/v4l/buffer.rst
+++ b/Documentation/media/uapi/v4l/buffer.rst
@@ -34,6 +34,94 @@ flags are copied from the OUTPUT video buffer to the CAPTURE 
video
 buffer.
 
 
+Interactions between formats, controls and buffers
+==
+
+V4L2 exposes parameters that influence the buffer size, or the way data is
+laid out in the buffer. Those parameters are exposed through both formats and
+controls. One example of such a control is the ``V4L2_CID_ROTATE`` control
+that modifies the direction in which pixels are stored in the buffer, as well
+as the buffer size when the selected format includes padding at the end of
+lines.
+
+The set of information needed to interpret the content of a buffer (e.g. the
+pixel format, the line stride, the tiling orientation or the rotation) is
+collectively referred to in the rest of this section as the buffer layout.
+
+Modifying formats or controls that influence the buffer size or layout require
+the stream to be stopped. Any attempt at such a modification while the stream
+is active shall cause the format or control set ioctl to return the ``EBUSY``
+error code.
+
+Controls that only influence the buffer layout can be modified at any time
+when the stream is stopped. As they don't influence the buffer size, no
+special handling is needed to synchronize those controls with buffer
+allocation.
+
+Formats and controls that influence the buffer size interact with buffer
+allocation. As buffer allocation is an expensive operation, drivers should
+allow format or controls that influence the buffer size to be changed with
+buffers allocated. A typical ioctl sequence to modify format and controls is
+
+ #. VIDIOC_STREAMOFF
+ #. VIDIOC_S_FMT
+ #. VIDIOC_S_EXT_CTRLS
+ #. VIDIOC_QBUF
+ #. VIDIOC_STREAMON
+
+Queued buffers must be large enough for the new format or controls.
+
+Drivers shall return a ``ENOSPC`` error in response to format change
+(:c:func:`VIDIOC_S_FMT`) or control changes (:c:func:`VIDIOC_S_CTRL` or
+:c:func:`VIDIOC_S_EXT_CTRLS`) if buffers too small for the new format are
+currently queued. As a simplification, drivers are allowed to return an error
+from these ioctls if any buffer is currently queued, without checking the
+queued buffers sizes. Drivers shall also return a ``ENOSPC`` error from the
+:c:func:`VIDIOC_QBUF` ioctl if the buffer being queued is too small for the
+current format or controls. Together, these requirements ensure that queued
+buffers will always be large enough for the configured format and controls.
+
+Userspace applications can query the buffer size required for a given format
+and controls by first setting the desired control values and then trying the
+desired format. The :c:func:`VIDIOC_TRY_FMT` ioctl will return the required
+buffer size.
+
+ #. 

[PATCH v2 0/3] Renesas R-Car VSP1 rotation support

[Laurent Pinchart]

Hello everybody,

This patch series implement support for rotation in the VSP1 driver. It
contains an update of the rotation support part of the "[PATCH 00/13]
Renesas R-Car VSP: Scaling and rotation support on Gen3" series.

Patch 1/3 starts by fixing the multi-line comment style through the whole
driver to avoid adding new offenders in patch 3/3.

Patch 2/3 is to biggest addition since v1. It clarifies interactions between
formats, controls and buffers in the V4L2 API documentation. The proposal is
based on an IRC meeting held on 2016-09-15 in the #v4l channel on freenode
which I have tried to translate as accurately as possible into documentation.

Patch 3/3 implements rotation support in the vsp1 driver. Beside being rebased
on top of the current vsp1/next branch, it has mostly been kept unchanged
compared to v1.

For your convenience the code is available at

git://linuxtv.org/pinchartl/media.git vsp1-rotation-v2-20170228

Laurent Pinchart (3):
  v4l: vsp1: Fix multi-line comment style
  v4l: Clearly document interactions between formats, controls and
buffers
  v4l: vsp1: wpf: Implement rotation support

 Documentation/media/uapi/v4l/buffer.rst   |  88 
 drivers/media/platform/vsp1/vsp1_bru.c|  27 ++--
 drivers/media/platform/vsp1/vsp1_dl.c |  27 ++--
 drivers/media/platform/vsp1/vsp1_drm.c|  21 ++-
 drivers/media/platform/vsp1/vsp1_drv.c|  12 +-
 drivers/media/platform/vsp1/vsp1_entity.c |   9 +-
 drivers/media/platform/vsp1/vsp1_hsit.c   |   3 +-
 drivers/media/platform/vsp1/vsp1_lif.c|   6 +-
 drivers/media/platform/vsp1/vsp1_pipe.c   |   9 +-
 drivers/media/platform/vsp1/vsp1_rpf.c|  11 +-
 drivers/media/platform/vsp1/vsp1_rwpf.c   |  11 +-
 drivers/media/platform/vsp1/vsp1_rwpf.h   |   3 +-
 drivers/media/platform/vsp1/vsp1_sru.c|   3 +-
 drivers/media/platform/vsp1/vsp1_uds.c|   3 +-
 drivers/media/platform/vsp1/vsp1_video.c  |  45 +--
 drivers/media/platform/vsp1/vsp1_wpf.c| 215 +++---
 16 files changed, 382 insertions(+), 111 deletions(-)

-- 
Regards,

Laurent Pinchart

[PATCH v2 1/3] v4l: vsp1: Fix multi-line comment style

[Laurent Pinchart]

Fix all multi-line comments to comply with the kernel coding style.

Signed-off-by: Laurent Pinchart 
---
 drivers/media/platform/vsp1/vsp1_bru.c| 27 -
 drivers/media/platform/vsp1/vsp1_dl.c | 27 -
 drivers/media/platform/vsp1/vsp1_drm.c| 21 +---
 drivers/media/platform/vsp1/vsp1_drv.c| 12 +++
 drivers/media/platform/vsp1/vsp1_entity.c |  9 ++---
 drivers/media/platform/vsp1/vsp1_hsit.c   |  3 ++-
 drivers/media/platform/vsp1/vsp1_lif.c|  6 --
 drivers/media/platform/vsp1/vsp1_pipe.c   |  9 ++---
 drivers/media/platform/vsp1/vsp1_rpf.c|  9 ++---
 drivers/media/platform/vsp1/vsp1_rwpf.c   |  6 --
 drivers/media/platform/vsp1/vsp1_sru.c|  3 ++-
 drivers/media/platform/vsp1/vsp1_uds.c|  3 ++-
 drivers/media/platform/vsp1/vsp1_video.c  | 33 ---
 drivers/media/platform/vsp1/vsp1_wpf.c| 12 +++
 14 files changed, 120 insertions(+), 60 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_bru.c 
b/drivers/media/platform/vsp1/vsp1_bru.c
index ee8355c28f94..85362c5ef57a 100644
--- a/drivers/media/platform/vsp1/vsp1_bru.c
+++ b/drivers/media/platform/vsp1/vsp1_bru.c
@@ -251,7 +251,8 @@ static int bru_set_selection(struct v4l2_subdev *subdev,
sel->r.left = clamp_t(unsigned int, sel->r.left, 0, format->width - 1);
sel->r.top = clamp_t(unsigned int, sel->r.top, 0, format->height - 1);
 
-   /* Scaling isn't supported, the compose rectangle size must be identical
+   /*
+* Scaling isn't supported, the compose rectangle size must be identical
 * to the sink format size.
 */
format = vsp1_entity_get_pad_format(>entity, config, sel->pad);
@@ -300,13 +301,15 @@ static void bru_configure(struct vsp1_entity *entity,
format = vsp1_entity_get_pad_format(>entity, bru->entity.config,
bru->entity.source_pad);
 
-   /* The hardware is extremely flexible but we have no userspace API to
+   /*
+* The hardware is extremely flexible but we have no userspace API to
 * expose all the parameters, nor is it clear whether we would have use
 * cases for all the supported modes. Let's just harcode the parameters
 * to sane default values for now.
 */
 
-   /* Disable dithering and enable color data normalization unless the
+   /*
+* Disable dithering and enable color data normalization unless the
 * format at the pipeline output is premultiplied.
 */
flags = pipe->output ? pipe->output->format.flags : 0;
@@ -314,7 +317,8 @@ static void bru_configure(struct vsp1_entity *entity,
   flags & V4L2_PIX_FMT_FLAG_PREMUL_ALPHA ?
   0 : VI6_BRU_INCTRL_NRM);
 
-   /* Set the background position to cover the whole output image and
+   /*
+* Set the background position to cover the whole output image and
 * configure its color.
 */
vsp1_bru_write(bru, dl, VI6_BRU_VIRRPF_SIZE,
@@ -325,7 +329,8 @@ static void bru_configure(struct vsp1_entity *entity,
vsp1_bru_write(bru, dl, VI6_BRU_VIRRPF_COL, bru->bgcolor |
   (0xff << VI6_BRU_VIRRPF_COL_A_SHIFT));
 
-   /* Route BRU input 1 as SRC input to the ROP unit and configure the ROP
+   /*
+* Route BRU input 1 as SRC input to the ROP unit and configure the ROP
 * unit with a NOP operation to make BRU input 1 available as the
 * Blend/ROP unit B SRC input.
 */
@@ -337,7 +342,8 @@ static void bru_configure(struct vsp1_entity *entity,
bool premultiplied = false;
u32 ctrl = 0;
 
-   /* Configure all Blend/ROP units corresponding to an enabled BRU
+   /*
+* Configure all Blend/ROP units corresponding to an enabled BRU
 * input for alpha blending. Blend/ROP units corresponding to
 * disabled BRU inputs are used in ROP NOP mode to ignore the
 * SRC input.
@@ -352,13 +358,15 @@ static void bru_configure(struct vsp1_entity *entity,
 |  VI6_BRU_CTRL_AROP(VI6_ROP_NOP);
}
 
-   /* Select the virtual RPF as the Blend/ROP unit A DST input to
+   /*
+* Select the virtual RPF as the Blend/ROP unit A DST input to
 * serve as a background color.
 */
if (i == 0)
ctrl |= VI6_BRU_CTRL_DSTSEL_VRPF;
 
-   /* Route BRU inputs 0 to 3 as SRC inputs to Blend/ROP units A to
+   /*
+* Route BRU inputs 0 to 3 as SRC inputs to Blend/ROP units A to
 * D in that order. The Blend/ROP unit B SRC is hardwired to the
 * ROP unit output, the corresponding register 

Re: [PATCH 4/6] omap3isp: Disable streaming at driver unbind time

[Laurent Pinchart]

Hi Sakari,

On Tuesday 28 Feb 2017 16:00:01 Sakari Ailus wrote:
> > On Monday 20 Feb 2017 17:22:20 Sakari Ailus wrote:
> >> Once the driver is unbound accessing the hardware is not allowed anymore.
> >> Due to this, disable streaming when the device driver is unbound. The
> >> states of the associated objects related to Media controller and
> >> videobuf2 frameworks are updated as well, just like if the application
> >> disabled streaming explicitly.
> >> 
> >> Signed-off-by: Sakari Ailus 
> > 
> > This looks mostly good to me, although I'm a bit concerned about race
> > conditions related to buffer handling. I don't think this patch introduces
> > any new one though, so
> > 
> > Reviewed-by: Laurent Pinchart 
> > 
> > We'll have to go through buffer management at some point in the near
> > future, including from a V4L2 API point of view I think.
> 
> Thanks for the review!
> 
> Are you happy with me sending a pull request on the set, or would you
> prefer to pick the omap3isp patches? In the latter case I'll send a fix
> for the issue in the first patch.

Feel free to send a pull request, I don't have anything conflicting queued for 
v4.12.

-- 
Regards,

Laurent Pinchart

Re: [RFC 08/13] smiapp-pll: Take existing divisor into account in minimum divisor check

[Sakari Ailus]

On Tue, Feb 28, 2017 at 03:09:21PM +0100, Pavel Machek wrote:
> Can I get you to apply this one? :-).

Let me try to understand again what does that change actually do. I'll find
the time during the rest of this week.

I'm starting to think we need a test suite for the PLL calculator...

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk

Re: [RFC 08/13] smiapp-pll: Take existing divisor into account in minimum divisor check

[Pavel Machek]

Hi!

> > On Tue, Feb 14, 2017 at 02:40:04PM +0100, Pavel Machek wrote:
> > > From: Sakari Ailus 
> > > 
> > > Required added multiplier (and divisor) calculation did not take into
> > > account the existing divisor when checking the values against the
> > > minimum divisor. Do just that.
> > > 
> > > Signed-off-by: Sakari Ailus 
> > > Signed-off-by: Ivaylo Dimitrov 
> > > Signed-off-by: Pavel Machek 
> > 
> > I need to understand again why did I write this patch. :-)
> 
> Can you just trust your former self?
> 
> > Could you send me the smiapp driver output with debug level messages
> > enabled, please?
> 
> > I think the problem was with the secondary sensor.
> 
> I believe it was with the main sensor, actually. Anyway, here are the
> messages.

Can I get you to apply this one? :-).

Thanks,
Pavel

> [0.791290] smiapp 2-0010: could not get clock (-517)
> [2.705352] smiapp 2-0010: GPIO lookup for consumer xshutdown
> [2.705352] smiapp 2-0010: using device tree for GPIO lookup
> [2.705413] smiapp 2-0010: using lookup tables for GPIO lookup
> [2.705413] smiapp 2-0010: lookup for GPIO xshutdown failed
> [2.875244] smiapp 2-0010: lane_op_clock_ratio: 1
> [2.875274] smiapp 2-0010: binning: 1x1
> [2.875274] smiapp 2-0010: min / max pre_pll_clk_div: 1 / 4
> [2.875305] smiapp 2-0010: pre-pll check: min / max
> pre_pll_clk_div: 1 / 1
> [2.875305] smiapp 2-0010: mul 25 / div 2
> [2.875305] smiapp 2-0010: pll_op check: min / max pre_pll_clk_div:
> 1 / 1
> [2.875335] smiapp 2-0010: pre_pll_clk_div 1
> [2.875335] smiapp 2-0010: more_mul_max: max_pll_multiplier check:
> 1
> [2.875335] smiapp 2-0010: more_mul_max: max_pll_op_freq_hz check:
> 1
> [2.875335] smiapp 2-0010: more_mul_max: max_op_sys_clk_div check:
> 1
> [2.875366] smiapp 2-0010: more_mul_max: min_pll_multiplier check:
> 1
> [2.875366] smiapp 2-0010: more_mul_min: min_pll_op_freq_hz check:
> 1
> [2.875366] smiapp 2-0010: more_mul_min: min_pll_multiplier check:
> 1
> [2.875396] smiapp 2-0010: more_mul_factor: 1
> [2.875396] smiapp 2-0010: more_mul_factor: min_op_sys_clk_div: 1
> [2.875396] smiapp 2-0010: final more_mul: 1
> [2.875427] smiapp 2-0010: op_sys_clk_div: 2
> [2.875427] smiapp 2-0010: op_pix_clk_div: 10
> [2.875427] smiapp 2-0010: pre_pll_clk_div 1
> [2.875457] smiapp 2-0010: pll_multiplier  25
> [2.875457] smiapp 2-0010: vt_sys_clk_div  2
> [2.875457] smiapp 2-0010: vt_pix_clk_div  10
> [2.875457] smiapp 2-0010: ext_clk_freq_hz 960
> [2.875488] smiapp 2-0010: pll_ip_clk_freq_hz  960
> [2.875488] smiapp 2-0010: pll_op_clk_freq_hz  24000
> [2.875488] smiapp 2-0010: vt_sys_clk_freq_hz  12000
> [2.875518] smiapp 2-0010: vt_pix_clk_freq_hz  1200
> [2.876068] smiapp 2-0010: lane_op_clock_ratio: 1
> [2.876068] smiapp 2-0010: binning: 1x1
> [2.876098] smiapp 2-0010: min / max pre_pll_clk_div: 1 / 4
> [2.876098] smiapp 2-0010: pre-pll check: min / max
> pre_pll_clk_div: 1 / 1
> [2.876098] smiapp 2-0010: mul 25 / div 2
> [2.876129] smiapp 2-0010: pll_op check: min / max pre_pll_clk_div:
> 1 / 1
> [2.876129] smiapp 2-0010: pre_pll_clk_div 1
> [2.876129] smiapp 2-0010: more_mul_max: max_pll_multiplier check:
> 1
> [2.876159] smiapp 2-0010: more_mul_max: max_pll_op_freq_hz check:
> 1
> [2.876159] smiapp 2-0010: more_mul_max: max_op_sys_clk_div check:
> 1
> [2.876159] smiapp 2-0010: more_mul_max: min_pll_multiplier check:
> 1
> [2.876190] smiapp 2-0010: more_mul_min: min_pll_op_freq_hz check:
> 1
> [2.876190] smiapp 2-0010: more_mul_min: min_pll_multiplier check:
> 1
> [2.876190] smiapp 2-0010: more_mul_factor: 1
> [2.876190] smiapp 2-0010: more_mul_factor: min_op_sys_clk_div: 1
> [2.876220] smiapp 2-0010: final more_mul: 1
> [2.876220] smiapp 2-0010: op_sys_clk_div: 2
> [2.876220] smiapp 2-0010: op_pix_clk_div: 10
> [2.876251] smiapp 2-0010: pre_pll_clk_div 1
> [2.876251] smiapp 2-0010: pll_multiplier  25
> [2.876251] smiapp 2-0010: vt_sys_clk_div  2
> [2.876251] smiapp 2-0010: vt_pix_clk_div  10
> [2.876281] smiapp 2-0010: ext_clk_freq_hz 960
> [2.876281] smiapp 2-0010: pll_ip_clk_freq_hz  960
> [2.876281] smiapp 2-0010: pll_op_clk_freq_hz  24000
> [2.876312] smiapp 2-0010: vt_sys_clk_freq_hz  12000
> [2.876312] smiapp 2-0010: vt_pix_clk_freq_hz  1200
> ...
> [4.728973] udevd[216]: starting version 175
> [8.031494] smiapp 2-0010: lane_op_clock_ratio: 1
> [8.031524] smiapp 2-0010: binning: 1x1
> [8.031524] smiapp 2-0010: min / max pre_pll_clk_div: 1 / 4
> [8.031524] smiapp 2-0010: pre-pll check: min / max
> pre_pll_clk_div: 1 / 1
> [8.031555] smiapp 2-0010: mul 25 / div 2
> [8.031555] 

Re: [PATCH 4/6] omap3isp: Disable streaming at driver unbind time

[Sakari Ailus]

Laurent Pinchart wrote:

Hi Sakari,

Thank you for the patch.

On Monday 20 Feb 2017 17:22:20 Sakari Ailus wrote:

Once the driver is unbound accessing the hardware is not allowed anymore.
Due to this, disable streaming when the device driver is unbound. The
states of the associated objects related to Media controller and videobuf2
frameworks are updated as well, just like if the application disabled
streaming explicitly.

Signed-off-by: Sakari Ailus 


This looks mostly good to me, although I'm a bit concerned about race
conditions related to buffer handling. I don't think this patch introduces any
new one though, so

Reviewed-by: Laurent Pinchart 

We'll have to go through buffer management at some point in the near future,
including from a V4L2 API point of view I think.


Thanks for the review!

Are you happy with me sending a pull request on the set, or would you 
prefer to pick the omap3isp patches? In the latter case I'll send a fix 
for the issue in the first patch.


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

Re: [WARNING: A/V UNSCANNABLE][Merge tag 'media/v4.11-1' of git] ff58d005cd: BUG: unable to handle kernel NULL pointer dereference at 0000039c

[Thomas Gleixner]

On Mon, 27 Feb 2017, Linus Torvalds wrote:
> So I don't disagree that in a perfect world all drivers should just
> handle it. It's just that it's not realistic.
> 
> The fact that we have now *twice* gotten an oops report or a "this
> machine doesn't boot" report etc within a week or so of merging the
> problematic patch does *not* indicate that it's easy to fix or rare.
> 
> Quite the reverse.
> 
> It indicates that it's just rare enough that core developers don't see
> it, but it's common enough to have triggered issues in random places.
> 
> And it will just get *much* worse when you then get the random
> end-users that usually have older machines than the developers who
> actually test daily development -git trees.

I tend to disagree.

The retrigger mechanism has been there forever, at least the history git
tree which goes back to 2.5.0 has it and as it was in the initial commit is
it was there in 2.4 already.

We broke that in 4.2 when the x86 interrupt mechanism was reworked
completely. That went pretty much unnoticed until somebody moved from 4.1
to 4.9 and discovered that edge interrupts got lost. It's not surprising
that it went unnoticed because lots of stuff moved towards MSI (which has
the retrigger still enabled) and the devices which are prone to the 'lost
edge' issue are limited.

So we had that retrigger exposing crappy older drivers to the spurious
interrupt until 2 years ago. The two drivers which have been exposed by
bringing the retrigger back are post 4.2 or have been wreckaged post 4.2.

Due to the fact that the old drivers have been exposed over many years to
the spurious retrigger (that "feature" exists on old hardware as well) I
don't think it's much of a problem.

We also had the DEBUG_SHIRQ active until we had to disable it in
6d83f94db95cf (2.6.38) but not because it exposed crappy interrupt
handlers. We had to disable due to a functional problem described in the
commit message. I know for sure that distros had enabled DEBUG_SHIRQ (and
some still have the reduced functionality of it enabled).

What I find more problematic is:

 - to keep this 'lost edge' regression around, which really can render
   hardware useless.

 - to ignore the fact that these buggy interrupt handlers can be exposed by
   spurious interrupt events (as I showed in the other mail) "naturaly",
   but with a way smaller probability. If a user runs into such a spurious
   problem he has absolutely NO chance to debug it at all. Exposing it by
   the retrigger mechanism makes the detection more "reliable" and allows
   debugging it. The backtraces are pretty telling and clear.

Thanks,

tglx

Re: [PATCH 4/6] omap3isp: Disable streaming at driver unbind time

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Monday 20 Feb 2017 17:22:20 Sakari Ailus wrote:
> Once the driver is unbound accessing the hardware is not allowed anymore.
> Due to this, disable streaming when the device driver is unbound. The
> states of the associated objects related to Media controller and videobuf2
> frameworks are updated as well, just like if the application disabled
> streaming explicitly.
> 
> Signed-off-by: Sakari Ailus 

This looks mostly good to me, although I'm a bit concerned about race 
conditions related to buffer handling. I don't think this patch introduces any 
new one though, so

Reviewed-by: Laurent Pinchart 

We'll have to go through buffer management at some point in the near future, 
including from a V4L2 API point of view I think.

> ---
>  drivers/media/platform/omap3isp/ispvideo.c | 23 +++
>  1 file changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/media/platform/omap3isp/ispvideo.c
> b/drivers/media/platform/omap3isp/ispvideo.c index a3ca2a4..c04d357 100644
> --- a/drivers/media/platform/omap3isp/ispvideo.c
> +++ b/drivers/media/platform/omap3isp/ispvideo.c
> @@ -1191,22 +1191,17 @@ isp_video_streamon(struct file *file, void *fh, enum
> v4l2_buf_type type)
>  }
> 
>  static int
> -isp_video_streamoff(struct file *file, void *fh, enum v4l2_buf_type type)
> +__isp_video_streamoff(struct isp_video *video)
>  {
> - struct isp_video_fh *vfh = to_isp_video_fh(fh);
> - struct isp_video *video = video_drvdata(file);
>   struct isp_pipeline *pipe = to_isp_pipeline(>video.entity);
>   enum isp_pipeline_state state;
>   unsigned int streaming;
>   unsigned long flags;
> 
> - if (type != video->type)
> - return -EINVAL;
> -
>   mutex_lock(>stream_lock);
> 
>   mutex_lock(>queue_lock);
> - streaming = vb2_is_streaming(>queue);
> + streaming = video->queue && vb2_is_streaming(video->queue);
>   mutex_unlock(>queue_lock);
> 
>   if (!streaming)
> @@ -1229,7 +1224,7 @@ isp_video_streamoff(struct file *file, void *fh, enum
> v4l2_buf_type type) omap3isp_video_cancel_stream(video);
> 
>   mutex_lock(>queue_lock);
> - vb2_streamoff(>queue, type);
> + vb2_streamoff(video->queue, video->queue->type);
>   mutex_unlock(>queue_lock);
>   video->queue = NULL;
>   video->error = false;
> @@ -1245,6 +1240,17 @@ isp_video_streamoff(struct file *file, void *fh, enum
> v4l2_buf_type type) }
> 
>  static int
> +isp_video_streamoff(struct file *file, void *fh, enum v4l2_buf_type type)
> +{
> + struct isp_video *video = video_drvdata(file);
> +
> + if (type != video->type)
> + return -EINVAL;
> +
> + return __isp_video_streamoff(video);
> +}
> +
> +static int
>  isp_video_enum_input(struct file *file, void *fh, struct v4l2_input *input)
> {
>   if (input->index > 0)
> @@ -1494,5 +1500,6 @@ int omap3isp_video_register(struct isp_video *video,
> struct v4l2_device *vdev)
> 
>  void omap3isp_video_unregister(struct isp_video *video)
>  {
> + __isp_video_streamoff(video);
>   video_unregister_device(>video);
>  }

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/6] omap3isp: Call video_unregister_device() unconditionally

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Monday 20 Feb 2017 17:22:18 Sakari Ailus wrote:
> video_unregister_device() can be called on a never or an already
> unregistered device. Drop the redundant check.
> 
> Signed-off-by: Sakari Ailus 

Reviewed-by: Laurent Pinchart 

> ---
>  drivers/media/platform/omap3isp/ispvideo.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/omap3isp/ispvideo.c
> b/drivers/media/platform/omap3isp/ispvideo.c index 218e6d7..9e9b18c 100644
> --- a/drivers/media/platform/omap3isp/ispvideo.c
> +++ b/drivers/media/platform/omap3isp/ispvideo.c
> @@ -1495,6 +1495,5 @@ int omap3isp_video_register(struct isp_video *video,
> struct v4l2_device *vdev)
> 
>  void omap3isp_video_unregister(struct isp_video *video)
>  {
> - if (video_is_registered(>video))
> - video_unregister_device(>video);
> + video_unregister_device(>video);
>  }

-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/6] omap3isp: Remove misleading comment

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Monday 20 Feb 2017 17:22:19 Sakari Ailus wrote:
> The intent of the check the comment is related to is to ensure we are
> streaming --- still. Not that streaming wouldn't be enabled yet. Remove
> it.
> 
> Signed-off-by: Sakari Ailus 

Reviewed-by: Laurent Pinchart 
> ---
>  drivers/media/platform/omap3isp/ispvideo.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/media/platform/omap3isp/ispvideo.c
> b/drivers/media/platform/omap3isp/ispvideo.c index 9e9b18c..a3ca2a4 100644
> --- a/drivers/media/platform/omap3isp/ispvideo.c
> +++ b/drivers/media/platform/omap3isp/ispvideo.c
> @@ -1205,7 +1205,6 @@ isp_video_streamoff(struct file *file, void *fh, enum
> v4l2_buf_type type)
> 
>   mutex_lock(>stream_lock);
> 
> - /* Make sure we're not streaming yet. */
>   mutex_lock(>queue_lock);
>   streaming = vb2_is_streaming(>queue);
>   mutex_unlock(>queue_lock);

-- 
Regards,

Laurent Pinchart

Re: [PATCH 6/6] media: devnode: Rename mdev argument as devnode

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Monday 20 Feb 2017 17:22:22 Sakari Ailus wrote:
> Historically, mdev argument name was being used on both struct
> media_device and struct media_devnode. Recently most occurrences of mdev
> referring to struct media_devnode were replaced by devnode, which makes
> more sense. Fix the last remaining occurrence.
> 
> Fixes: 163f1e93e9950 ("[media] media-devnode: fix namespace mess")
> Signed-off-by: Sakari Ailus 
> Acked-by: Hans Verkuil 

Reviewed-by: Laurent Pinchart 

> ---
>  drivers/media/media-device.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index c51e2e5..fce91b5 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -537,9 +537,9 @@ static DEVICE_ATTR(model, S_IRUGO, show_model, NULL);
>   * Registration/unregistration
>   */
> 
> -static void media_device_release(struct media_devnode *mdev)
> +static void media_device_release(struct media_devnode *devnode)
>  {
> - dev_dbg(mdev->parent, "Media device released\n");
> + dev_dbg(devnode->parent, "Media device released\n");
>  }
> 
>  /**

-- 
Regards,

Laurent Pinchart

Re: [PATCH 5/6] media: Remove useless curly braces and parentheses

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Monday 20 Feb 2017 17:22:21 Sakari Ailus wrote:
> Signed-off-by: Sakari Ailus 
> Acked-by: Hans Verkuil 

Reviewed-by: Laurent Pinchart 

> ---
>  drivers/media/media-device.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index 760e3e4..c51e2e5 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -591,9 +591,8 @@ int __must_check media_device_register_entity(struct
> media_device *mdev, >pads[i].graph_obj);
> 
>   /* invoke entity_notify callbacks */
> - list_for_each_entry_safe(notify, next, >entity_notify, list) {
> - (notify)->notify(entity, notify->notify_data);
> - }
> + list_for_each_entry_safe(notify, next, >entity_notify, list)
> + notify->notify(entity, notify->notify_data);
> 
>   if (mdev->entity_internal_idx_max
> 
>   >= mdev->pm_count_walk.ent_enum.idx_max) {

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/6] omap3isp: Don't rely on devm for memory resource management

[Sakari Ailus]

Laurent Pinchart wrote:

Hi Sakari,

Thank you for the patch.

On Monday 20 Feb 2017 17:22:17 Sakari Ailus wrote:

...

@@ -516,9 +516,12 @@ int omap3isp_hist_init(struct isp_device *isp)
hist->event_type = V4L2_EVENT_OMAP3ISP_HIST;

ret = omap3isp_stat_init(hist, "histogram", _subdev_ops);
+
+err:
if (ret) {
-   if (hist->dma_ch)
+   if (!IS_ERR(hist->dma_ch))


I think this change is wrong. dma_ch is initialize to NULL by kzalloc(). You
will end up calling dma_release_channel() on a NULL channel if
omap3isp_stat_init() fails and HIST_CONFIG_DMA is false. The check should be

if (!IS_ERR_OR_NULL(hist->dma_ch))


Good catch! I'll fix that.



Apart from that,

Reviewed-by: Laurent Pinchart 


Thanks!


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

[PATCH] cec: don't Feature Abort msgs from Unregistered

[Hans Verkuil]

Feature Abort shouldn't be sent in reply to messages from Unregistered,
since that would make it a broadcast message.

Signed-off-by: Hans Verkuil 
---
 drivers/media/cec/cec-adap.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/media/cec/cec-adap.c b/drivers/media/cec/cec-adap.c
index 46b7da6df9b5..25d0a835921f 100644
--- a/drivers/media/cec/cec-adap.c
+++ b/drivers/media/cec/cec-adap.c
@@ -1615,6 +1615,9 @@ static int cec_feature_abort_reason(struct cec_adapter 
*adap,
 */
if (msg->msg[1] == CEC_MSG_FEATURE_ABORT)
return 0;
+   /* Don't Feature Abort messages from 'Unregistered' */
+   if (cec_msg_initiator(msg) == CEC_LOG_ADDR_UNREGISTERED)
+   return 0;
cec_msg_set_reply_to(_msg, msg);
cec_msg_feature_abort(_msg, msg->msg[1], reason);
return cec_transmit_msg(adap, _msg, false);
--
2.11.0

Re: [PATCH 1/6] omap3isp: Don't rely on devm for memory resource management

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Monday 20 Feb 2017 17:22:17 Sakari Ailus wrote:
> devm functions are fine for managing resources that are directly related
> to the device at hand and that have no other dependencies. However, a
> process holding a file handle to a device created by a driver for a device
> may result in the file handle left behind after the device is long gone.
> This will result in accessing released (and potentially reallocated)
> memory.
> 
> Instead, manage the memory resources in the driver. Releasing the
> resources can be later on bound to e.g. by releasing a reference.
> 
> Signed-off-by: Sakari Ailus 
> ---
>  drivers/media/platform/omap3isp/isp.c | 18 --
>  drivers/media/platform/omap3isp/isph3a_aewb.c | 24 +---
> drivers/media/platform/omap3isp/isph3a_af.c   | 24 +---
> drivers/media/platform/omap3isp/isphist.c | 11 +++
>  drivers/media/platform/omap3isp/ispstat.c |  2 ++
>  5 files changed, 55 insertions(+), 24 deletions(-)

[snip]

> diff --git a/drivers/media/platform/omap3isp/isphist.c
> b/drivers/media/platform/omap3isp/isphist.c index a4ed5d1..9e448c6 100644
> --- a/drivers/media/platform/omap3isp/isphist.c
> +++ b/drivers/media/platform/omap3isp/isphist.c
> @@ -476,9 +476,9 @@ int omap3isp_hist_init(struct isp_device *isp)
>  {
>   struct ispstat *hist = >isp_hist;
>   struct omap3isp_hist_config *hist_cfg;
> - int ret = -1;
> + int ret;
> 
> - hist_cfg = devm_kzalloc(isp->dev, sizeof(*hist_cfg), GFP_KERNEL);
> + hist_cfg = kzalloc(sizeof(*hist_cfg), GFP_KERNEL);
>   if (hist_cfg == NULL)
>   return -ENOMEM;
> 
> @@ -500,7 +500,7 @@ int omap3isp_hist_init(struct isp_device *isp)
>   if (IS_ERR(hist->dma_ch)) {
>   ret = PTR_ERR(hist->dma_ch);
>   if (ret == -EPROBE_DEFER)
> - return ret;
> + goto err;
> 
>   hist->dma_ch = NULL;
>   dev_warn(isp->dev,
> @@ -516,9 +516,12 @@ int omap3isp_hist_init(struct isp_device *isp)
>   hist->event_type = V4L2_EVENT_OMAP3ISP_HIST;
> 
>   ret = omap3isp_stat_init(hist, "histogram", _subdev_ops);
> +
> +err:
>   if (ret) {
> - if (hist->dma_ch)
> + if (!IS_ERR(hist->dma_ch))

I think this change is wrong. dma_ch is initialize to NULL by kzalloc(). You 
will end up calling dma_release_channel() on a NULL channel if 
omap3isp_stat_init() fails and HIST_CONFIG_DMA is false. The check should be

if (!IS_ERR_OR_NULL(hist->dma_ch))

Apart from that,

Reviewed-by: Laurent Pinchart 

>   dma_release_channel(hist->dma_ch);
> + kfree(hist_cfg);
>   }
> 
>   return ret;

[snip]

-- 
Regards,

Laurent Pinchart

Re: [RFC 1/1] omap3isp: Ignore endpoints with invalid configuration

[Sakari Ailus]

Pavel Machek wrote:

Tested-by: Pavel Machek 


Thanks!

I've applied the patch, plus yours, to the ccp2 branch.

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

Re: [RFC 1/1] omap3isp: Ignore endpoints with invalid configuration

[Pavel Machek]

On Tue 2017-02-28 14:02:30, Sakari Ailus wrote:
> If endpoint has an invalid configuration, ignore it instead of happily
> proceeding to use it nonetheless. Ignoring such an endpoint is better than
> failing since there could be multiple endpoints, only some of which are
> bad.
> 
> Signed-off-by: Sakari Ailus 
> ---
> Hi Pavel,
> 
> How about this one? isp_fwnode_parse() is expected to return an error if
> there's one but currently it's quite shy. With this patch, the faulty
> endpoint is simply ignored. This is completely untested so far.

Does not seem to break anything.

Tested-by: Pavel Machek 

Pavel

>  drivers/media/platform/omap3isp/isp.c | 8 +---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/platform/omap3isp/isp.c 
> b/drivers/media/platform/omap3isp/isp.c
> index 95850b9..8026221 100644
> --- a/drivers/media/platform/omap3isp/isp.c
> +++ b/drivers/media/platform/omap3isp/isp.c
> @@ -2120,10 +2120,12 @@ static int isp_fwnodes_parse(struct device *dev,
>   if (!isd)
>   goto error;
>  
> - notifier->subdevs[notifier->num_subdevs] = >asd;
> + if (isp_fwnode_parse(dev, fwn, isd)) {
> + devm_kfree(dev, isd);
> + continue;
> + }
>  
> - if (isp_fwnode_parse(dev, fwn, isd))
> - goto error;
> + notifier->subdevs[notifier->num_subdevs] = >asd;
>  
>   isd->asd.match.fwnode.fwn =
>   fwnode_graph_get_remote_port_parent(fwn);

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


signature.asc
Description: Digital signature

[RFC 1/1] omap3isp: Ignore endpoints with invalid configuration

[Sakari Ailus]

If endpoint has an invalid configuration, ignore it instead of happily
proceeding to use it nonetheless. Ignoring such an endpoint is better than
failing since there could be multiple endpoints, only some of which are
bad.

Signed-off-by: Sakari Ailus 
---
Hi Pavel,

How about this one? isp_fwnode_parse() is expected to return an error if
there's one but currently it's quite shy. With this patch, the faulty
endpoint is simply ignored. This is completely untested so far.

 drivers/media/platform/omap3isp/isp.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/omap3isp/isp.c 
b/drivers/media/platform/omap3isp/isp.c
index 95850b9..8026221 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -2120,10 +2120,12 @@ static int isp_fwnodes_parse(struct device *dev,
if (!isd)
goto error;
 
-   notifier->subdevs[notifier->num_subdevs] = >asd;
+   if (isp_fwnode_parse(dev, fwn, isd)) {
+   devm_kfree(dev, isd);
+   continue;
+   }
 
-   if (isp_fwnode_parse(dev, fwn, isd))
-   goto error;
+   notifier->subdevs[notifier->num_subdevs] = >asd;
 
isd->asd.match.fwnode.fwn =
fwnode_graph_get_remote_port_parent(fwn);
-- 
2.7.4

[PATCH] omap3isp: Parse CSI1 configuration from the device tree.

[Pavel Machek]

Add support for parsing CSI1 configuration.

Signed-off-by: Pavel Machek 

---

> Please find my comments below.

Thanks for comments. They are fixed now, plus I fixed the checkpatch
stuff that was possible.

It should be ready to apply to the right branch.

diff --git a/drivers/media/platform/omap3isp/isp.c 
b/drivers/media/platform/omap3isp/isp.c
index 245225a..b8eef2f 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -2032,6 +2032,7 @@ static int isp_fwnode_parse(struct device *dev, struct 
fwnode_handle *fwn,
struct v4l2_fwnode_endpoint vfwn;
unsigned int i;
int ret;
+   bool csi1 = false;
 
ret = v4l2_fwnode_endpoint_parse(fwn, );
if (ret)
@@ -2059,38 +2060,88 @@ static int isp_fwnode_parse(struct device *dev, struct 
fwnode_handle *fwn,
 
case ISP_OF_PHY_CSIPHY1:
case ISP_OF_PHY_CSIPHY2:
-   /* FIXME: always assume CSI-2 for now. */
+   switch (vfwn.bus_type) {
+   case V4L2_MBUS_CCP2:
+   case V4L2_MBUS_CSI1:
+   dev_dbg(dev, "csi1 configuration\n");
+   csi1 = true;
+   break;
+   case V4L2_MBUS_CSI2:
+   dev_dbg(dev, "csi2 configuration\n");
+   csi1 = false;
+   break;
+   default:
+   dev_err(dev, "unkonwn bus type\n");
+   }
+
switch (vfwn.base.port) {
case ISP_OF_PHY_CSIPHY1:
-   buscfg->interface = ISP_INTERFACE_CSI2C_PHY1;
+   if (csi1)
+   buscfg->interface = ISP_INTERFACE_CCP2B_PHY1;
+   else
+   buscfg->interface = ISP_INTERFACE_CSI2C_PHY1;
break;
case ISP_OF_PHY_CSIPHY2:
-   buscfg->interface = ISP_INTERFACE_CSI2A_PHY2;
+   if (csi1)
+   buscfg->interface = ISP_INTERFACE_CCP2B_PHY2;
+   else
+   buscfg->interface = ISP_INTERFACE_CSI2A_PHY2;
break;
+   default:
+   dev_err(dev, "bad port\n");
}
-   buscfg->bus.csi2.lanecfg.clk.pos = 
vfwn.bus.mipi_csi2.clock_lane;
-   buscfg->bus.csi2.lanecfg.clk.pol =
-   vfwn.bus.mipi_csi2.lane_polarities[0];
-   dev_dbg(dev, "clock lane polarity %u, pos %u\n",
-   buscfg->bus.csi2.lanecfg.clk.pol,
-   buscfg->bus.csi2.lanecfg.clk.pos);
-
-   for (i = 0; i < ISP_CSIPHY2_NUM_DATA_LANES; i++) {
-   buscfg->bus.csi2.lanecfg.data[i].pos =
-   vfwn.bus.mipi_csi2.data_lanes[i];
-   buscfg->bus.csi2.lanecfg.data[i].pol =
-   vfwn.bus.mipi_csi2.lane_polarities[i + 1];
+   if (csi1) {
+   buscfg->bus.ccp2.lanecfg.clk.pos =
+   vfwn.bus.mipi_csi1.clock_lane;
+   buscfg->bus.ccp2.lanecfg.clk.pol =
+   vfwn.bus.mipi_csi1.lane_polarity[0];
+   dev_dbg(dev, "clock lane polarity %u, pos %u\n",
+   buscfg->bus.ccp2.lanecfg.clk.pol,
+   buscfg->bus.ccp2.lanecfg.clk.pos);
+
+   buscfg->bus.ccp2.lanecfg.data[0].pos =
+   vfwn.bus.mipi_csi1.data_lane;
+   buscfg->bus.ccp2.lanecfg.data[0].pol =
+   vfwn.bus.mipi_csi1.lane_polarity[1];
+
dev_dbg(dev, "data lane %u polarity %u, pos %u\n", i,
-   buscfg->bus.csi2.lanecfg.data[i].pol,
-   buscfg->bus.csi2.lanecfg.data[i].pos);
+   buscfg->bus.ccp2.lanecfg.data[0].pol,
+   buscfg->bus.ccp2.lanecfg.data[0].pos);
+
+   buscfg->bus.ccp2.strobe_clk_pol =
+   vfwn.bus.mipi_csi1.clock_inv;
+   buscfg->bus.ccp2.phy_layer = vfwn.bus.mipi_csi1.strobe;
+   buscfg->bus.ccp2.ccp2_mode =
+   vfwn.bus_type == V4L2_MBUS_CCP2;
+   buscfg->bus.ccp2.vp_clk_pol = 1;
+
+   buscfg->bus.ccp2.crc = 1;
+   } else {
+   buscfg->bus.csi2.lanecfg.clk.pos =
+   vfwn.bus.mipi_csi2.clock_lane;
+   buscfg->bus.csi2.lanecfg.clk.pol =
+   vfwn.bus.mipi_csi2.lane_polarities[0];
+   dev_dbg(dev, "clock lane polarity %u, pos %u\n",
+   

Re: [PATCHv4 1/9] video: add hotplug detect notifier support

[Daniel Vetter]

On Tue, Feb 28, 2017 at 10:23:57AM +0100, Hans Verkuil wrote:
> On 02/28/17 09:51, Daniel Vetter wrote:
> > On Mon, Feb 27, 2017 at 05:46:51PM +, Russell King - ARM Linux wrote:
> > > On Mon, Feb 27, 2017 at 06:21:05PM +0100, Hans Verkuil wrote:
> > > > On 02/27/2017 06:04 PM, Russell King - ARM Linux wrote:
> > > > > I'm afraid that I walked away from this after it became clear that 
> > > > > there
> > > > > was little hope for any forward progress being made in a timely manner
> > > > > for multiple reasons (mainly the core CEC code being out of mainline.)
> > > > 
> > > > In case you missed it: the core CEC code was moved out of staging and 
> > > > into
> > > > mainline in 4.10.
> > > 
> > > I was aware (even though I've not been publishing anything, I do keep
> > > dw-hdmi-cec and tda9950/tda998x up to date with every final kernel
> > > release.)
> > > 
> > > > > If you can think of a better approach, then I'm sure there's lots of
> > > > > people who'd be willing to do the coding for it... if not, I'm not
> > > > > sure where we go from here (apart from keeping code in private/vendor
> > > > > trees.)
> > > > 
> > > > For CEC there are just two things that it needs: the physical address
> > > > (contained in the EDID) and it needs to be informed when the EDID 
> > > > disappears
> > > > (typically due to a disconnect), in which case the physical address
> > > > becomes invalid (f.f.f.f).
> > > 
> > > Yep.  CEC really only needs to know "have new phys address" and
> > > "disconnect" provided that CEC drivers understand that they may receive
> > > a new phys address with no intervening disconnect.  (Consider the case
> > > where EDID changes, but the HDMI driver failed to spot the HPD signal
> > > pulse - unfortunately, there's hardware out there where HPD is next to
> > > useless.)
> > 
> > Ok, simplifying the CEC stuff like that would be a lot better I think, to
> > avoid overlap with other in-kernel interfaces. I still have some
> > questions, but this might be my misunderstanding of how CEC works:
> > 
> > I thought that CEC is driven through a special separate wire in the HDMI
> > bus, with the receiver in the TV. Which means that we'd have a 1:1
> > relationship between HDMI connector and CEC port. That's at least the
> > use-case I've heard of for Intel boards. Are there other use-cases where
> > we do not have a 1:1 relationship between HDMI connector and CEC port? Imo
> > notifier really only makes sense as a quick hack, or if you really
> > have n:m for at least one of n,m != 1. Otherwise some very specific
> > callback service which just provides the information the CEC side needs
> > seems like a much better idea to me.
> 
> For the current set of CEC drivers it is 1:1.
> 
> I am fairly certain you can get n:1 situations where multiple HDMI connectors
> use a single CEC adapter. I'm thinking primarily about HDMI switches here. 
> Also
> TVs with multiple inputs (basically a switch as well).
> 
> I do not support such hardware yet, though. Or to be more specific: I've never
> tested this, so I am not sure if this would work in the current framework, or
> if I need to do some more work for that.
> 
> That said, each CEC adapter would have only 0 or 1 HDMI outputs and 0 or more
> HDMI inputs. More than one HDMI outputs is AFAICT not possible.
> 
> > 
> > > > Russell, do you have pending code that needs the ELD support in the
> > > > notifier?  CEC doesn't need it, so from my perspective it can be
> > > > dropped in the first version.
> > > 
> > > I was looking for that while writing the previous mail, and I think
> > > it's time to drop it - I had thought dw-hdmi-*audio would use it, or
> > > the ASoC people, but it's still got no users, so I think it's time
> > > to drop it.
> > 
> > For ELD I'd definitely say let's please use the hdmi-codec.h. It's what's
> > in tree, so better to converge on one solution instead of proliferating
> > even more. The entire sad story of multiple people inventing similar
> > wheels without talking with each another is water down the river, can't
> > fix that anymore :(
> 
> I'll drop that in my next patch series.
> 
> > 
> > > I have seen some patch sets go by which are making use of the notifier,
> > > but I haven't paid close attention to how they're using it or what
> > > they're using it for... as I sort of implied in my previous mail, I
> > > had lost interest in mainline wrt CEC stuff due to the glacial rate
> > > of progress.  (That's not a criticism.)
> > 
> > Maybe some docs that would describe the flow we want to achieve here would
> > help? Doesn't need to be more than a few lines, but reconstructing that
> > from the various driver patches later on is indeed hard.
> 
> I'll add some more documentation for the next version.
> 
> But in a nutshell: each HDMI connector (in or out) has to notify the CEC
> driver when the physical address changes (when an EDID is read or set, or
> when the HPD goes down). It also needs to provide the current 

Re: [PATCHv4 1/9] video: add hotplug detect notifier support

[Hans Verkuil]

On 02/28/17 09:51, Daniel Vetter wrote:

On Mon, Feb 27, 2017 at 05:46:51PM +, Russell King - ARM Linux wrote:

On Mon, Feb 27, 2017 at 06:21:05PM +0100, Hans Verkuil wrote:

On 02/27/2017 06:04 PM, Russell King - ARM Linux wrote:

I'm afraid that I walked away from this after it became clear that there
was little hope for any forward progress being made in a timely manner
for multiple reasons (mainly the core CEC code being out of mainline.)


In case you missed it: the core CEC code was moved out of staging and into
mainline in 4.10.


I was aware (even though I've not been publishing anything, I do keep
dw-hdmi-cec and tda9950/tda998x up to date with every final kernel
release.)


If you can think of a better approach, then I'm sure there's lots of
people who'd be willing to do the coding for it... if not, I'm not
sure where we go from here (apart from keeping code in private/vendor
trees.)


For CEC there are just two things that it needs: the physical address
(contained in the EDID) and it needs to be informed when the EDID disappears
(typically due to a disconnect), in which case the physical address
becomes invalid (f.f.f.f).


Yep.  CEC really only needs to know "have new phys address" and
"disconnect" provided that CEC drivers understand that they may receive
a new phys address with no intervening disconnect.  (Consider the case
where EDID changes, but the HDMI driver failed to spot the HPD signal
pulse - unfortunately, there's hardware out there where HPD is next to
useless.)


Ok, simplifying the CEC stuff like that would be a lot better I think, to
avoid overlap with other in-kernel interfaces. I still have some
questions, but this might be my misunderstanding of how CEC works:

I thought that CEC is driven through a special separate wire in the HDMI
bus, with the receiver in the TV. Which means that we'd have a 1:1
relationship between HDMI connector and CEC port. That's at least the
use-case I've heard of for Intel boards. Are there other use-cases where
we do not have a 1:1 relationship between HDMI connector and CEC port? Imo
notifier really only makes sense as a quick hack, or if you really
have n:m for at least one of n,m != 1. Otherwise some very specific
callback service which just provides the information the CEC side needs
seems like a much better idea to me.


For the current set of CEC drivers it is 1:1.

I am fairly certain you can get n:1 situations where multiple HDMI connectors
use a single CEC adapter. I'm thinking primarily about HDMI switches here. Also
TVs with multiple inputs (basically a switch as well).

I do not support such hardware yet, though. Or to be more specific: I've never
tested this, so I am not sure if this would work in the current framework, or
if I need to do some more work for that.

That said, each CEC adapter would have only 0 or 1 HDMI outputs and 0 or more
HDMI inputs. More than one HDMI outputs is AFAICT not possible.




Russell, do you have pending code that needs the ELD support in the
notifier?  CEC doesn't need it, so from my perspective it can be
dropped in the first version.


I was looking for that while writing the previous mail, and I think
it's time to drop it - I had thought dw-hdmi-*audio would use it, or
the ASoC people, but it's still got no users, so I think it's time
to drop it.


For ELD I'd definitely say let's please use the hdmi-codec.h. It's what's
in tree, so better to converge on one solution instead of proliferating
even more. The entire sad story of multiple people inventing similar
wheels without talking with each another is water down the river, can't
fix that anymore :(


I'll drop that in my next patch series.




I have seen some patch sets go by which are making use of the notifier,
but I haven't paid close attention to how they're using it or what
they're using it for... as I sort of implied in my previous mail, I
had lost interest in mainline wrt CEC stuff due to the glacial rate
of progress.  (That's not a criticism.)


Maybe some docs that would describe the flow we want to achieve here would
help? Doesn't need to be more than a few lines, but reconstructing that
from the various driver patches later on is indeed hard.


I'll add some more documentation for the next version.

But in a nutshell: each HDMI connector (in or out) has to notify the CEC
driver when the physical address changes (when an EDID is read or set, or
when the HPD goes down). It also needs to provide the current physical
address when the CEC driver is first loaded. This latter requirement is
handled by the notifier framework which remembers the EDID and will create
a notify event as soon as the CEC driver registers itself.

Regards,

Hans

Re: [PATCH] [media] tw5864: handle unknown video std gracefully

[Andrey Utkin]

On Tue, Feb 28, 2017 at 09:20:53AM +0100, Arnd Bergmann wrote:
> On Tue, Feb 28, 2017 at 2:08 AM, Andrey Utkin
>  wrote:
> > Retcode checking takes place everywhere, but currently it overwrites
> > supplied structs with potentially-uninitialized values. To make it
> > cleaner, it should be (e.g. tw5864_g_parm())
> >
> > ret = tw5864_frameinterval_get(input, >timeperframe);
> > if (ret)
> > return ret;
> > cp->timeperframe.numerator *= input->frame_interval;
> > cp->capturemode = 0;
> > cp->readbuffers = 2;
> > return 0;
> >
> > and not
> >
> > ret = tw5864_frameinterval_get(input, >timeperframe);
> > cp->timeperframe.numerator *= input->frame_interval;
> > cp->capturemode = 0;
> > cp->readbuffers = 2;
> > return ret;
> >
> > That would resolve your concerns of uninitialized values propagation
> > without writing bogus values 1/1 in case of failure. I think I'd
> > personally prefer a called function to leave my data structs intact when
> > it fails.
> 
> That seems reasonable, I can try to come up with a new version that
> incorporates this change, but I haven't been able to avoid the warning
> without either removing the WARN() or adding an initialization.

I don't mind dropping WARN().

Thanks for your elaborate reply.

Re: [PATCH 1/4] v4l2: device_register_subdev_nodes: allow calling multiple times

[Pavel Machek]

Hi!

> Please find my comments below.

Thanks for quick review, will fix.

> > switch (vfwn.base.port) {
> > case ISP_OF_PHY_CSIPHY1:
> > -   buscfg->interface = ISP_INTERFACE_CSI2C_PHY1;
> > +   if (csi1)
> 
> You could compare vfwn.bus_type == V4L2_MBUS_CSI2 for this. But if you
> choose the local variable, please make it bool instead.

I prefer variable, will switch to bool.

> > +
> > +   buscfg->bus.ccp2.lanecfg.data[0].pos = 1;
> 
> Shouldn't this be vfwn.bus.mipi_csi1.data_lane ?
> 
> > +   buscfg->bus.ccp2.lanecfg.data[0].pol = 0;
> 
> And this one is vfwn.bus.mipi_csi1.lane_polarity[1] .

Thanks for catching this.

Checkpatch issues will be fixed.

 Pavel

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


signature.asc
Description: Digital signature

Re: [PATCHv4 1/9] video: add hotplug detect notifier support

[Daniel Vetter]

On Mon, Feb 27, 2017 at 05:46:51PM +, Russell King - ARM Linux wrote:
> On Mon, Feb 27, 2017 at 06:21:05PM +0100, Hans Verkuil wrote:
> > On 02/27/2017 06:04 PM, Russell King - ARM Linux wrote:
> > > I'm afraid that I walked away from this after it became clear that there
> > > was little hope for any forward progress being made in a timely manner
> > > for multiple reasons (mainly the core CEC code being out of mainline.)
> > 
> > In case you missed it: the core CEC code was moved out of staging and into
> > mainline in 4.10.
> 
> I was aware (even though I've not been publishing anything, I do keep
> dw-hdmi-cec and tda9950/tda998x up to date with every final kernel
> release.)
> 
> > > If you can think of a better approach, then I'm sure there's lots of
> > > people who'd be willing to do the coding for it... if not, I'm not
> > > sure where we go from here (apart from keeping code in private/vendor
> > > trees.)
> > 
> > For CEC there are just two things that it needs: the physical address
> > (contained in the EDID) and it needs to be informed when the EDID disappears
> > (typically due to a disconnect), in which case the physical address
> > becomes invalid (f.f.f.f).
> 
> Yep.  CEC really only needs to know "have new phys address" and
> "disconnect" provided that CEC drivers understand that they may receive
> a new phys address with no intervening disconnect.  (Consider the case
> where EDID changes, but the HDMI driver failed to spot the HPD signal
> pulse - unfortunately, there's hardware out there where HPD is next to
> useless.)

Ok, simplifying the CEC stuff like that would be a lot better I think, to
avoid overlap with other in-kernel interfaces. I still have some
questions, but this might be my misunderstanding of how CEC works:

I thought that CEC is driven through a special separate wire in the HDMI
bus, with the receiver in the TV. Which means that we'd have a 1:1
relationship between HDMI connector and CEC port. That's at least the
use-case I've heard of for Intel boards. Are there other use-cases where
we do not have a 1:1 relationship between HDMI connector and CEC port? Imo
notifier really only makes sense as a quick hack, or if you really
have n:m for at least one of n,m != 1. Otherwise some very specific
callback service which just provides the information the CEC side needs
seems like a much better idea to me.

> > Russell, do you have pending code that needs the ELD support in the
> > notifier?  CEC doesn't need it, so from my perspective it can be
> > dropped in the first version.
> 
> I was looking for that while writing the previous mail, and I think
> it's time to drop it - I had thought dw-hdmi-*audio would use it, or
> the ASoC people, but it's still got no users, so I think it's time
> to drop it.

For ELD I'd definitely say let's please use the hdmi-codec.h. It's what's
in tree, so better to converge on one solution instead of proliferating
even more. The entire sad story of multiple people inventing similar
wheels without talking with each another is water down the river, can't
fix that anymore :(

> I have seen some patch sets go by which are making use of the notifier,
> but I haven't paid close attention to how they're using it or what
> they're using it for... as I sort of implied in my previous mail, I
> had lost interest in mainline wrt CEC stuff due to the glacial rate
> of progress.  (That's not a criticism.)

Maybe some docs that would describe the flow we want to achieve here would
help? Doesn't need to be more than a few lines, but reconstructing that
from the various driver patches later on is indeed hard.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH] [media] tw5864: handle unknown video std gracefully

[Arnd Bergmann]

On Tue, Feb 28, 2017 at 2:08 AM, Andrey Utkin
 wrote:
> Hi Arnd,
>
> Thanks for sending this patch.
>
> On Mon, Feb 27, 2017 at 09:32:34PM +0100, Arnd Bergmann wrote:
>> tw5864_frameinterval_get() only initializes its output when it successfully
>> identifies the video standard in tw5864_input. We get a warning here because
>> gcc can't always track the state if initialized warnings across a WARN()
>> macro, and thinks it might get used incorrectly in tw5864_s_parm:
>>
>> media/pci/tw5864/tw5864-video.c: In function 'tw5864_s_parm':
>> media/pci/tw5864/tw5864-video.c:816:38: error: 'time_base.numerator' may be 
>> used uninitialized in this function [-Werror=maybe-uninitialized]
>> media/pci/tw5864/tw5864-video.c:819:31: error: 'time_base.denominator' may 
>> be used uninitialized in this function [-Werror=maybe-uninitialized]
>
> I think behaviour of tw5864_frameinterval_get() is ok.
> I don't see how WARN() could affect gcc state tracking. There's "return
> -EINVAL" right after WARN() which lets caller handle the failure case
> gracefully. Maybe I just don't see how confusing WARN() can be for gcc
> in this situation, but it's not as confusing as BUG() would be, right?

The problem is on architectures that use "unlikely()" within WARN(),
in combination with CONFIG_PROFILE_ANNOTATED_BRANCHES().

That option invokes this macro:


#define __branch_check__(x, expect) ({  \
int __r;\
static struct ftrace_likely_data\
__attribute__((__aligned__(4))) \

__attribute__((section("_ftrace_annotated_branch"))) \
__f = { \
.data.func = __func__,  \
.data.file = __FILE__,  \
.data.line = __LINE__,  \
};  \
__r = likely_notrace(x);\
ftrace_likely_update(&__f, __r, expect); \
__r;\
})

and after the condition has been passed through it, gcc is sufficiently
confused that it forgets everything about which variables have been
initialized and which haven't based on the condition.

> I see the reason of that warning is
>
>  - time_base being not initialized in tw5864_s_parm()
>  - gcc being too dumb to recognize that we have checked the retcode in
>tw5864_s_parm() and proceed only when we are sure we have correctly
>initialized time_base.
>
> Is that you compiling with manually added -Werror=maybe-uninitialized or
> is that default compilation flags? I don't remember encountering that
> and I doubt a lot of kernel code compiles without warnings with such
> flag.

I build with -Werror locally to turn all warnings into errors,
-Wmaybe-uninitialized
is turned on by default with gcc-4.9 and higher.

> Also, which GCC version are you using?

I see this happening on arm64 with gcc-5.2.1, gcc-6.3.1 and gcc-7.0.1,
but not with gcc-4.9.3.

I did not run into this one on arm or x86 with any of the above compiler
versions during randconfig testing.

>> This particular use happens to be ok, but we do copy the uninitialized
>> output of tw5864_frameinterval_get() into other memory without checking
>> the return code, interestingly without getting a warning here.
>
> Retcode checking takes place everywhere, but currently it overwrites
> supplied structs with potentially-uninitialized values. To make it
> cleaner, it should be (e.g. tw5864_g_parm())
>
> ret = tw5864_frameinterval_get(input, >timeperframe);
> if (ret)
> return ret;
> cp->timeperframe.numerator *= input->frame_interval;
> cp->capturemode = 0;
> cp->readbuffers = 2;
> return 0;
>
> and not
>
> ret = tw5864_frameinterval_get(input, >timeperframe);
> cp->timeperframe.numerator *= input->frame_interval;
> cp->capturemode = 0;
> cp->readbuffers = 2;
> return ret;
>
> That would resolve your concerns of uninitialized values propagation
> without writing bogus values 1/1 in case of failure. I think I'd
> personally prefer a called function to leave my data structs intact when
> it fails.

That seems reasonable, I can try to come up with a new version that
incorporates this change, but I haven't been able to avoid the warning
without either removing the WARN() or adding an initialization.

>> This initializes the output to 1/1s for the case, to make sure we do
>> get an intialization that doesn't cause a division-by-zero exception
>> in case we end up using this uninitialized data later.
>
> Personally I won't object against such patch, but I find it a bit too
> much "defensive" for kernel coding taste.
>
> Making sure somebody