[PATCH] staging/media/davinci_vpfe: Use common error handling code in vpfe_attach_irq()

[SF Markus Elfring]

From: Markus Elfring 
Date: Fri, 3 Nov 2017 10:45:31 +0100

Add a jump target so that a bit of exception handling can be better reused
at the end of this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/staging/media/davinci_vpfe/vpfe_mc_capture.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/media/davinci_vpfe/vpfe_mc_capture.c 
b/drivers/staging/media/davinci_vpfe/vpfe_mc_capture.c
index bffe2153b910..80297d2df31d 100644
--- a/drivers/staging/media/davinci_vpfe/vpfe_mc_capture.c
+++ b/drivers/staging/media/davinci_vpfe/vpfe_mc_capture.c
@@ -309,8 +309,7 @@ static int vpfe_attach_irq(struct vpfe_device *vpfe_dev)
if (ret < 0) {
v4l2_err(_dev->v4l2_dev,
"Error: requesting VINT1 interrupt\n");
-   free_irq(vpfe_dev->ccdc_irq0, vpfe_dev);
-   return ret;
+   goto free_irq;
}
 
ret = request_irq(vpfe_dev->imp_dma_irq, vpfe_imp_dma_isr,
@@ -319,11 +318,14 @@ static int vpfe_attach_irq(struct vpfe_device *vpfe_dev)
v4l2_err(_dev->v4l2_dev,
 "Error: requesting IMP IRQ interrupt\n");
free_irq(vpfe_dev->ccdc_irq1, vpfe_dev);
-   free_irq(vpfe_dev->ccdc_irq0, vpfe_dev);
-   return ret;
+   goto free_irq;
}
 
return 0;
+
+free_irq:
+   free_irq(vpfe_dev->ccdc_irq0, vpfe_dev);
+   return ret;
 }
 
 /*
-- 
2.15.0

Re: 32bit userland cannot work with DVB drivers in 64bit kernel, design issue

[Menion]

For the moment, I am focusing on the frontend ioctl problem
According to the fops:

static const struct file_operations dvb_frontend_fops = {
.owner = THIS_MODULE,
.unlocked_ioctl = dvb_generic_ioctl,
.poll = dvb_frontend_poll,
.open = dvb_frontend_open,
.release = dvb_frontend_release,
.llseek = noop_llseek,
};

There is no support for compact_ioctl, so it must be added

2017-11-02 19:52 GMT+01:00 Alan Cox :
> On Thu, 2 Nov 2017 12:16:39 +0100
> Menion  wrote:
>
>> Hi all
>> I am investigating for Armbian, the feasability of running 32bit
>> userland on single board computers based on arm64 SoC, where only 64
>> bit kernel is available, for reducing the memory footprint.
>> I have discovered that there is a flaw in the DVB frontend ioctl (at
>> least) that prevents to do so.
>> in frontend.h the biggest problem is:
>>
>> struct dtv_properties {
>> __u32 num;
>> struct dtv_property *props;
>> };
>>
>> The master userland-kernel ioctl structure is based on an array set by
>> pointer, so the 32bit userland will allocate 32bit pointer (and the
>> resulting structure size) while the 64bit kernel will expect the 64bit
>> pointers
>
> And in some cases the pointer might end up aligned to the next 64bit
> boundary.
>
>> The void *reserved2 field will also give problem when crossing the
>> 32-64bits boundaries
>> As today the endire dvb linux infrastructure can only work in 32-32
>> and 64-64 bit mode
>
> If this isn't handled by the existing media compat_ioctl support then you
> can send patches to use compat_ioctl handlers to fix this.
>
> Alan

Re: [PATCH v4 2/3] media: ov7740: Document device tree bindings

[Sakari Ailus]

Hi Rob,

On Wed, Nov 01, 2017 at 04:51:57PM -0500, Rob Herring wrote:
> On Tue, Oct 31, 2017 at 09:11:44AM +0800, Wenyou Yang wrote:
> > Add the device tree binding documentation for the ov7740 sensor driver.
> > 
> > Signed-off-by: Wenyou Yang 
> > ---
> > 
> > Changes in v4: None
> > Changes in v3:
> >  - Explicitly document the "remote-endpoint" property.
> > 
> > Changes in v2: None
> > 
> >  .../devicetree/bindings/media/i2c/ov7740.txt   | 47 
> > ++
> >  1 file changed, 47 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov7740.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/media/i2c/ov7740.txt 
> > b/Documentation/devicetree/bindings/media/i2c/ov7740.txt
> > new file mode 100644
> > index ..af781c3a5f0e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/i2c/ov7740.txt
> > @@ -0,0 +1,47 @@
> > +* Omnivision OV7740 CMOS image sensor
> > +
> > +The Omnivision OV7740 image sensor supports multiple output image
> > +size, such as VGA, and QVGA, CIF and any size smaller. It also
> > +supports the RAW RGB and YUV output formats.
> > +
> > +The common video interfaces bindings (see video-interfaces.txt) should
> > +be used to specify link to the image data receiver. The OV7740 device
> > +node should contain one 'port' child node with an 'endpoint' subnode.
> > +
> > +Required Properties:
> > +- compatible:  "ovti,ov7740".
> > +- reg: I2C slave address of the sensor.
> > +- clocks:  Reference to the xvclk input clock.
> > +- clock-names: "xvclk".
> > +
> > +Optional Properties:
> > +- reset-gpios: Rreference to the GPIO connected to the reset_b pin,
> > +  if any. Active low with pull-ip resistor.
> > +- powerdown-gpios: Reference to the GPIO connected to the pwdn pin,
> > +  if any. Active high with pull-down resistor.
> > +
> > +Endpoint node mandatory properties:
> > +- remote-endpoint: A phandle to the bus receiver's endpoint node.
> 
> This is not really necessary. What's required is documenting how many 
> ports and how many endpoints for each port which you have above.

I actually requested adding that as the practice, as far as I've understood
it, has been to document all properties relevant for the hardware (apart
from things such as assigned-clocks etc.).

The port and endpoints have been elaborated above and I think that should
be fine as-is.

The graph bindings document (referred by video-interfaces.txt) the
remote-endpoint property in an endpoint as an optional property, however
you can't really use the sensor if it's not connected to anything. The same
goes for video-interfaces.txt: remote-endpoint in an endpoint is optional.

I have no objections removing remote-endpoint here, though, if you think
it's not relevant here.

-- 
Kind regards,

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

Re: [PATCH 2/2] media: s5p-mfc: fix lock confection - request_firmware() once and keep state

[Shuah Khan]

On 11/02/2017 06:43 PM, Marian Mihailescu wrote:
> I can confirm, with this patch, there is always error loading MFC in
> boot log, since FS is not mounted.
> 
> -Marian
> 

Please refrain from top posting to a kernel email threads. It is very difficult
to follow. Bottom post is the norm.

thanks,
-- Shuah 

[PATCH v1] [media] v4l2-ctrls: Don't validate BITMASK twice

[Andy Shevchenko]

There is no need to repeat what check_range() does for us, i.e. BITMASK
validation in v4l2_ctrl_new().

Signed-off-by: Andy Shevchenko 
---
 drivers/media/v4l2-core/v4l2-ctrls.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
b/drivers/media/v4l2-core/v4l2-ctrls.c
index c230bd5c6558..cbb2ef43945f 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -2013,10 +2013,6 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct 
v4l2_ctrl_handler *hdl,
handler_set_err(hdl, err);
return NULL;
}
-   if (type == V4L2_CTRL_TYPE_BITMASK && ((def & ~max) || min || step)) {
-   handler_set_err(hdl, -ERANGE);
-   return NULL;
-   }
if (is_array &&
(type == V4L2_CTRL_TYPE_BUTTON ||
 type == V4L2_CTRL_TYPE_CTRL_CLASS)) {
-- 
2.14.2

usb/media/em28xx: use-after-free in em28xx_dvb_fini

[Andrey Konovalov]

Hi!

I've got the following report while fuzzing the kernel with syzkaller.

On commit 3a99df9a3d14cd866b5516f8cba515a3bfd554ab (4.14-rc7+).

em28xx 1-1:2.0: New device a  @ 480 Mbps (eb1a:2801, interface 0, class 0)
em28xx 1-1:2.0: Audio interface 0 found (Vendor Class)
em28xx 1-1:2.0: chip ID is em2860
em28xx 1-1:2.0: Config register raw data: 0x22
em28xx 1-1:2.0: I2S Audio (3 sample rate(s))
em28xx 1-1:2.0: No AC97 audio processor
em28xx 1-1:2.0: Binding audio extension
em28xx 1-1:2.0: em28xx-audio.c: Copyright (C) 2006 Markus Rechberger
em28xx 1-1:2.0: em28xx-audio.c: Copyright (C) 2007-2016 Mauro Carvalho Chehab
em28xx 1-1:2.0: alt 0 doesn't exist on interface 7
usb 1-1: USB disconnect, device number 2
em28xx 1-1:2.0: Disconnecting
em28xx 1-1:2.0: Closing audio extension
em28xx 1-1:2.0: Freeing device
==
BUG: KASAN: use-after-free in em28xx_dvb_fini+0x74b/0x8e0
Read of size 1 at addr 880069d2c12c by task kworker/0:1/24

CPU: 0 PID: 24 Comm: kworker/0:1 Not tainted
4.14.0-rc7-44290-gf28444df2601-dirty #52
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Workqueue: usb_hub_wq hub_event
Call Trace:
 __dump_stack lib/dump_stack.c:16
 dump_stack+0xe1/0x157 lib/dump_stack.c:52
 print_address_description+0x71/0x234 mm/kasan/report.c:252
 kasan_report_error mm/kasan/report.c:351
 kasan_report+0x173/0x270 mm/kasan/report.c:409
 __asan_report_load1_noabort+0x19/0x20 mm/kasan/report.c:427
 em28xx_dvb_fini+0x74b/0x8e0 drivers/media/usb/em28xx/em28xx-dvb.c:2076
 em28xx_close_extension+0x71/0x220 drivers/media/usb/em28xx/em28xx-core.c:1122
 em28xx_usb_disconnect+0xd7/0x140 drivers/media/usb/em28xx/em28xx-cards.c:3763
 usb_unbind_interface+0x1b6/0x950 drivers/usb/core/driver.c:423
 __device_release_driver drivers/base/dd.c:861
 device_release_driver_internal+0x529/0x5f0 drivers/base/dd.c:893
 device_release_driver+0x1e/0x30 drivers/base/dd.c:918
 bus_remove_device+0x2fc/0x4b0 drivers/base/bus.c:565
 device_del+0x591/0xa70 drivers/base/core.c:1985
 usb_disable_device+0x223/0x710 drivers/usb/core/message.c:1170
 usb_disconnect+0x285/0x7f0 drivers/usb/core/hub.c:2205
 hub_port_connect drivers/usb/core/hub.c:4838
 hub_port_connect_change drivers/usb/core/hub.c:5093
 port_event drivers/usb/core/hub.c:5199
 hub_event_impl+0x10ec/0x3440 drivers/usb/core/hub.c:5311
 hub_event+0x38/0x50 drivers/usb/core/hub.c:5209
 process_one_work+0x925/0x15d0 kernel/workqueue.c:2113
 worker_thread+0xef/0x10d0 kernel/workqueue.c:2247
 kthread+0x346/0x410 kernel/kthread.c:231
 ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431

The buggy address belongs to the page:
page:ea0001a74b00 count:0 mapcount:-127 mapping:  (null) index:0x0
flags: 0x100()
raw: 0100   ff80
raw: ea00019f0320 88007fffa690 0002 
page dumped because: kasan: bad access detected

Memory state around the buggy address:
 880069d2c000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
 880069d2c080: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>880069d2c100: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
  ^
 880069d2c180: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
 880069d2c200: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
==

usb/media/pvrusb2: WARNING in pvr2_i2c_core_done/sysfs_remove_group

[Andrey Konovalov]

Hi!

I've got the following report while fuzzing the kernel with syzkaller.

On commit 3a99df9a3d14cd866b5516f8cba515a3bfd554ab (4.14-rc7+).

pvrusb2: Hardware description: OnAir Creator Hybrid USB tuner
pvrusb2: Invalid write control endpoint
...
pvrusb2: Invalid write control endpoint
pvrusb2: Module ID 3 (saa7115) for device OnAir Creator Hybrid USB
tuner failed to load.  Possible missing sub-device kernel module or
initialization failure within module.
cs53l32a 0-0011: chip found @ 0x22 (pvrusb2_a)
pvrusb2: Invalid write control endpoint
...
pvrusb2: Invalid write control endpoint
pvrusb2: Attached sub-driver cs53l32a
pvrusb2: Invalid write control endpoint
...
pvrusb2: Invalid write control endpoint
pvrusb2: Module ID 4 (tuner) for device OnAir Creator Hybrid USB tuner
failed to load.  Possible missing sub-device kernel module or
initialization failure within module.
pvrusb2: Device being rendered inoperable
pvrusb2: ***WARNING*** pvrusb2 driver initialization failed due to the
failure of one or more sub-device kernel modules.
pvrusb2: You need to resolve the failing condition before this driver
can function.  There should be some earlier messages giving more
information about the problem.
usb 1-1: USB disconnect, device number 11
sysfs group 'power' not found for kobject '0-0011'
[ cut here ]
WARNING: CPU: 0 PID: 2896 at fs/sysfs/group.c:237
sysfs_remove_group.cold.6+0x57/0x63
Modules linked in:
CPU: 0 PID: 2896 Comm: pvrusb2-context Not tainted
4.14.0-rc7-44290-gf28444df2601-dirty #52
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
task: 88006b752e00 task.stack: 88006b6c8000
RIP: 0010:sysfs_remove_group.cold.6+0x57/0x63 fs/sysfs/group.c:235
RSP: 0018:88006b6cfc28 EFLAGS: 00010292
RAX: 0032 RBX: 85b7a480 RCX: 812495b5
RDX:  RSI: 8124d76a RDI: 0005
RBP: 88006b6cfc48 R08: 88006b752e00 R09: 
R10:  R11:  R12: 880069a3e8a0
R13: 88006b9b5530 R14: 85b7a4c8 R15: 83c90160
FS:  () GS:88006ca0() knlGS:
CS:  0010 DS:  ES:  CR0: 80050033
CR2: 01e8a908 CR3: 63834000 CR4: 06f0
Call Trace:
 dpm_sysfs_remove+0x5d/0x70 drivers/base/power/sysfs.c:769
 device_del+0x2b5/0xa70 drivers/base/core.c:1962
 device_unregister+0x1a/0x40 drivers/base/core.c:2020
 i2c_unregister_device+0xfd/0x130 drivers/i2c/i2c-core-base.c:815
 __unregister_client+0x83/0x90 drivers/i2c/i2c-core-base.c:1413
 device_for_each_child+0xb2/0x110 drivers/base/core.c:2120
 i2c_del_adapter+0x2be/0x550 drivers/i2c/i2c-core-base.c:1477
 pvr2_i2c_core_done+0x79/0xcb drivers/media/usb/pvrusb2/pvrusb2-i2c-core.c:671
 pvr2_hdw_destroy+0x157/0x350 drivers/media/usb/pvrusb2/pvrusb2-hdw.c:2671
 pvr2_context_destroy+0x64/0x200 drivers/media/usb/pvrusb2/pvrusb2-context.c:79
 pvr2_context_check drivers/media/usb/pvrusb2/pvrusb2-context.c:146
 pvr2_context_thread_func+0x420/0x670
drivers/media/usb/pvrusb2/pvrusb2-context.c:167
 kthread+0x346/0x410 kernel/kthread.c:231
 ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431
Code: 8b 65 00 48 c1 ea 03 48 c1 e0 2a 80 3c 02 00 74 08 48 89 df e8
9e 70 e1 ff 48 8b 33 4c 89 e2 48 c7 c7 68 63 11 86 e8 66 89 aa ff <0f>
ff e9 63 fc ff ff 90 90 90 90 90 0f 1f 44 00 00 55 48 89 e5
---[ end trace c49faec9cc373c2a ]---
sysfs group 'power' not found for kobject 'i2c-0'
[ cut here ]
WARNING: CPU: 0 PID: 2896 at fs/sysfs/group.c:237
sysfs_remove_group.cold.6+0x57/0x63
Modules linked in:
CPU: 0 PID: 2896 Comm: pvrusb2-context Tainted: GW
4.14.0-rc7-44290-gf28444df2601-dirty #52
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
task: 88006b752e00 task.stack: 88006b6c8000
RIP: 0010:sysfs_remove_group.cold.6+0x57/0x63 fs/sysfs/group.c:235
RSP: 0018:88006b6cfcc0 EFLAGS: 00010282
RAX: 0031 RBX: 85b7a480 RCX: 812495b5
RDX:  RSI: 8124d76a RDI: 0005
RBP: 88006b6cfce0 R08: 88006b752e00 R09: 
R10:  R11:  R12: 88006998b4e0
R13: 880062ba0348 R14: 85b7a4c8 R15: 880062ba0898
FS:  () GS:88006ca0() knlGS:
CS:  0010 DS:  ES:  CR0: 80050033
CR2: 01e8a908 CR3: 63834000 CR4: 06f0
Call Trace:
 dpm_sysfs_remove+0x5d/0x70 drivers/base/power/sysfs.c:769
 device_del+0x2b5/0xa70 drivers/base/core.c:1962
 device_unregister+0x1a/0x40 drivers/base/core.c:2020
 i2c_del_adapter+0x3f8/0x550 drivers/i2c/i2c-core-base.c:1500
 pvr2_i2c_core_done+0x79/0xcb drivers/media/usb/pvrusb2/pvrusb2-i2c-core.c:671
 pvr2_hdw_destroy+0x157/0x350 drivers/media/usb/pvrusb2/pvrusb2-hdw.c:2671
 pvr2_context_destroy+0x64/0x200 drivers/media/usb/pvrusb2/pvrusb2-context.c:79
 pvr2_context_check 

Re: [RFC v4 17/17] [media] v4l: Document explicit synchronization behaviour

[Gustavo Padovan]

Hi Hans,

2017-11-03 Hans Verkuil :

> On 10/20/2017 11:50 PM, Gustavo Padovan wrote:
> > From: Gustavo Padovan 
> > 
> > Add section to VIDIOC_QBUF about it
> > 
> > v3:
> > - make the out_fence refer to the current buffer (Hans)
> > - Note what happens when the IN_FENCE is not set (Hans)
> > 
> > v2:
> > - mention that fences are files (Hans)
> > - rework for the new API
> > 
> > Signed-off-by: Gustavo Padovan 
> > ---
> >  Documentation/media/uapi/v4l/vidioc-qbuf.rst | 31 
> > 
> >  1 file changed, 31 insertions(+)
> > 
> > diff --git a/Documentation/media/uapi/v4l/vidioc-qbuf.rst 
> > b/Documentation/media/uapi/v4l/vidioc-qbuf.rst
> > index 9e448a4aa3aa..a65a50578bad 100644
> > --- a/Documentation/media/uapi/v4l/vidioc-qbuf.rst
> > +++ b/Documentation/media/uapi/v4l/vidioc-qbuf.rst
> > @@ -118,6 +118,37 @@ immediately with an ``EAGAIN`` error code when no 
> > buffer is available.
> >  The struct :c:type:`v4l2_buffer` structure is specified in
> >  :ref:`buffer`.
> >  
> > +Explicit Synchronization
> > +
> > +
> > +Explicit Synchronization allows us to control the synchronization of
> > +shared buffers from userspace by passing fences to the kernel and/or
> > +receiving them from it. Fences passed to the kernel are named in-fences and
> > +the kernel should wait on them to signal before using the buffer, i.e., 
> > queueing
> > +it to the driver. On the other side, the kernel can create out-fences for 
> > the
> > +buffers it queues to the drivers. Out-fences signal when the driver is
> > +finished with buffer, i.e., the buffer is ready. The fences are represented
> > +as a file and passed as a file descriptor to userspace.
> > +
> > +The in-fences are communicated to the kernel at the ``VIDIOC_QBUF`` ioctl
> > +using the ``V4L2_BUF_FLAG_IN_FENCE`` buffer
> > +flags and the `fence_fd` field. If an in-fence needs to be passed to the 
> > kernel,
> > +`fence_fd` should be set to the fence file descriptor number and the
> > +``V4L2_BUF_FLAG_IN_FENCE`` should be set as well Setting one but not the 
> > other
> 
> Missing '.' after 'as well'.
> 
> To what value is fence_fd set when VIDIOC_QBUF returns?

It should be -1 because we will be reusing the fence_fd field to return
the out_fence to userspace in the cases we don't need to use the
OUT_FENCE event. Like GPU drivers does with fences. That is the better
way to send the out fence back that I can think of at the moment.

> If you don't set the
> IN_FENCE flag, what should userspace set fence_fd to? (I recommend 0).

0 is still a valid fd, so the implementation is currently accepting -1
and 0.

> 
> > +will cause ``VIDIOC_QBUF`` to return with error.
> > +
> > +The fence_fd field (formely the reserved2 field) will be ignored if the
> 
> Drop the "(formely the reserved2 field)" part. We're not interested in the
> history here.
> 
> > +``V4L2_BUF_FLAG_IN_FENCE`` is not set.
> > +
> > +To get an out-fence back from V4L2 the ``V4L2_BUF_FLAG_OUT_FENCE`` flag 
> > should
> > +be set to ask for a fence to be attached to the buffer. To become aware of
> > +the out-fence created one should listen for the ``V4L2_EVENT_OUT_FENCE`` 
> > event.
> > +An event will be triggered for every buffer queued to the V4L2 driver with 
> > the
> > +``V4L2_BUF_FLAG_OUT_FENCE``.
> > +
> > +At streamoff the out-fences will either signal normally if the drivers 
> > waits
> 
> drivers -> driver
> 
> > +for the operations on the buffers to finish or signal with error if the
> 
> error -> an error
> 
> > +driver cancels the pending operations.
> >  
> >  Return Value
> >  
> > 
> 
> What should be done if the driver doesn't set ordered_in_driver? How does 
> userspace
> know whether in and/or out fences are supported? I'm leaning towards a new 
> capability
> flag for QUERYCAPS.

Yep. That is what we agreed last week in Prague.

> What does VIDIOC_QUERYBUF return w.r.t. the fence flags and fence_fd?

It does return the IN_FENCE flag if the fence didn't signal yet and the
OUT_FENCE one if the user set it on QBUF. The fence_fd is set to -1,
because the fd is specific to the pid using it.

Gustavo

net/media/em28xx: use-after-free in v4l2_fh_init

[Andrey Konovalov]

Hi!

I've got the following report while fuzzing the kernel with syzkaller.

On commit 3a99df9a3d14cd866b5516f8cba515a3bfd554ab (4.14-rc7+).

em28xx 1-1:0.0: analog set to bulk mode.
em28xx 1-1:0.0: Registering V4L2 extension
usb 1-1: USB disconnect, device number 39
em28xx 1-1:0.0: Disconnecting
em28xx 1-1:0.0: reading from i2c device at 0x4a failed (error=-5)
em28xx 1-1:0.0: Config register raw data: 0xffed
em28xx 1-1:0.0: AC97 chip type couldn't be determined
em28xx 1-1:0.0: No AC97 audio processor
em28xx 1-1:0.0: failed to create media graph
em28xx 1-1:0.0: V4L2 device video0 deregistered
em28xx 1-1:0.0: Binding DVB extension
==
BUG: KASAN: use-after-free in v4l2_fh_init+0x239/0x280
Read of size 8 at addr 88006aea0798 by task v4l_id/5819

CPU: 0 PID: 5819 Comm: v4l_id Not tainted
4.14.0-rc7-44290-gf28444df2601-dirty #52
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Call Trace:
 __dump_stack lib/dump_stack.c:16
 dump_stack+0xe1/0x157 lib/dump_stack.c:52
 print_address_description+0x71/0x234 mm/kasan/report.c:252
 kasan_report_error mm/kasan/report.c:351
 kasan_report+0x173/0x270 mm/kasan/report.c:409
 __asan_report_load8_noabort+0x19/0x20 mm/kasan/report.c:430
 v4l2_fh_init+0x239/0x280 drivers/media/v4l2-core/v4l2-fh.c:33
 v4l2_fh_open+0x76/0xa0 drivers/media/v4l2-core/v4l2-fh.c:70
 em28xx_v4l2_open+0x252/0x6f0 drivers/media/usb/em28xx/em28xx-video.c:2060
 v4l2_open+0x1b7/0x380 drivers/media/v4l2-core/v4l2-dev.c:425
 chrdev_open+0x1db/0x520 fs/char_dev.c:416
 do_dentry_open+0x735/0xe20 fs/open.c:752
 vfs_open+0x13e/0x230 fs/open.c:866
 do_last fs/namei.c:3387
 path_openat+0x722/0x2860 fs/namei.c:3527
 do_filp_open+0x13f/0x1d0 fs/namei.c:3562
 do_sys_open+0x362/0x4c0 fs/open.c:1059
 SYSC_open fs/open.c:1077
 SyS_open+0x32/0x40 fs/open.c:1072
 entry_SYSCALL_64_fastpath+0x23/0xc2 arch/x86/entry/entry_64.S:202
RIP: 0033:0x7f51f3ecb120
RSP: 002b:7ffc0140cb68 EFLAGS: 0246 ORIG_RAX: 0002
RAX: ffda RBX: 0046 RCX: 7f51f3ecb120
RDX: 7f51f4180138 RSI:  RDI: 7ffc0140df1e
RBP:  R08:  R09: 
R10:  R11: 0246 R12: 00400884
R13: 7ffc0140ccc0 R14:  R15: 

Allocated by task 2263:
 save_stack_trace+0x1b/0x20 arch/x86/kernel/stacktrace.c:59
 save_stack+0x43/0xd0 mm/kasan/kasan.c:447
 set_track mm/kasan/kasan.c:459
 kasan_kmalloc+0xc4/0xe0 mm/kasan/kasan.c:551
 kmem_cache_alloc_trace+0x11a/0x290 mm/slub.c:2772
 kmalloc ./include/linux/slab.h:493
 kzalloc ./include/linux/slab.h:666
 em28xx_v4l2_init+0x10c/0x3660 drivers/media/usb/em28xx/em28xx-video.c:2438
 em28xx_init_extension+0x11a/0x190 drivers/media/usb/em28xx/em28xx-core.c:1110
 request_module_async+0x6a/0x80 drivers/media/usb/em28xx/em28xx-cards.c:3161
 process_one_work+0x925/0x15d0 kernel/workqueue.c:2113
 worker_thread+0xef/0x10d0 kernel/workqueue.c:2247
 kthread+0x346/0x410 kernel/kthread.c:231
 ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431

Freed by task 2263:
 save_stack_trace+0x1b/0x20 arch/x86/kernel/stacktrace.c:59
 save_stack+0x43/0xd0 mm/kasan/kasan.c:447
 set_track mm/kasan/kasan.c:459
 kasan_slab_free+0x72/0xc0 mm/kasan/kasan.c:524
 slab_free_hook mm/slub.c:1390
 slab_free_freelist_hook mm/slub.c:1412
 slab_free mm/slub.c:2988
 kfree+0xf2/0x2e0 mm/slub.c:3919
 em28xx_free_v4l2 drivers/media/usb/em28xx/em28xx-video.c:2025
 kref_put ./include/linux/kref.h:70
 em28xx_v4l2_init+0x237f/0x3660 drivers/media/usb/em28xx/em28xx-video.c:2789
 em28xx_init_extension+0x11a/0x190 drivers/media/usb/em28xx/em28xx-core.c:1110
 request_module_async+0x6a/0x80 drivers/media/usb/em28xx/em28xx-cards.c:3161
 process_one_work+0x925/0x15d0 kernel/workqueue.c:2113
 worker_thread+0xef/0x10d0 kernel/workqueue.c:2247
 kthread+0x346/0x410 kernel/kthread.c:231
 ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431

The buggy address belongs to the object at 88006aea
 which belongs to the cache kmalloc-8192 of size 8192
The buggy address is located 1944 bytes inside of
 8192-byte region [88006aea, 88006aea2000)
The buggy address belongs to the page:
page:ea0001aba800 count:1 mapcount:0 mapping:  (null)
index:0x0 compound_mapcount: 0
flags: 0x1008100(slab|head)
raw: 01008100   000180030003
raw:  00010001 88006c402a80 
page dumped because: kasan: bad access detected

Memory state around the buggy address:
 88006aea0680: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
 88006aea0700: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>88006aea0780: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
 88006aea0800: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
 88006aea0880: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb

Re: usb/media/em28xx: use-after-free in em28xx_dvb_fini

[Andrey Konovalov]

On Fri, Nov 3, 2017 at 3:44 PM, Andrey Konovalov  wrote:
> Hi!
>
> I've got the following report while fuzzing the kernel with syzkaller.
>
> On commit 3a99df9a3d14cd866b5516f8cba515a3bfd554ab (4.14-rc7+).
>
> em28xx 1-1:2.0: New device a  @ 480 Mbps (eb1a:2801, interface 0, class 0)
> em28xx 1-1:2.0: Audio interface 0 found (Vendor Class)
> em28xx 1-1:2.0: chip ID is em2860
> em28xx 1-1:2.0: Config register raw data: 0x22
> em28xx 1-1:2.0: I2S Audio (3 sample rate(s))
> em28xx 1-1:2.0: No AC97 audio processor
> em28xx 1-1:2.0: Binding audio extension
> em28xx 1-1:2.0: em28xx-audio.c: Copyright (C) 2006 Markus Rechberger
> em28xx 1-1:2.0: em28xx-audio.c: Copyright (C) 2007-2016 Mauro Carvalho Chehab
> em28xx 1-1:2.0: alt 0 doesn't exist on interface 7
> usb 1-1: USB disconnect, device number 2
> em28xx 1-1:2.0: Disconnecting
> em28xx 1-1:2.0: Closing audio extension
> em28xx 1-1:2.0: Freeing device
> ==
> BUG: KASAN: use-after-free in em28xx_dvb_fini+0x74b/0x8e0
> Read of size 1 at addr 880069d2c12c by task kworker/0:1/24
>
> CPU: 0 PID: 24 Comm: kworker/0:1 Not tainted
> 4.14.0-rc7-44290-gf28444df2601-dirty #52
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> Workqueue: usb_hub_wq hub_event
> Call Trace:
>  __dump_stack lib/dump_stack.c:16
>  dump_stack+0xe1/0x157 lib/dump_stack.c:52
>  print_address_description+0x71/0x234 mm/kasan/report.c:252
>  kasan_report_error mm/kasan/report.c:351
>  kasan_report+0x173/0x270 mm/kasan/report.c:409
>  __asan_report_load1_noabort+0x19/0x20 mm/kasan/report.c:427
>  em28xx_dvb_fini+0x74b/0x8e0 drivers/media/usb/em28xx/em28xx-dvb.c:2076
>  em28xx_close_extension+0x71/0x220 drivers/media/usb/em28xx/em28xx-core.c:1122
>  em28xx_usb_disconnect+0xd7/0x140 drivers/media/usb/em28xx/em28xx-cards.c:3763
>  usb_unbind_interface+0x1b6/0x950 drivers/usb/core/driver.c:423
>  __device_release_driver drivers/base/dd.c:861
>  device_release_driver_internal+0x529/0x5f0 drivers/base/dd.c:893
>  device_release_driver+0x1e/0x30 drivers/base/dd.c:918
>  bus_remove_device+0x2fc/0x4b0 drivers/base/bus.c:565
>  device_del+0x591/0xa70 drivers/base/core.c:1985
>  usb_disable_device+0x223/0x710 drivers/usb/core/message.c:1170
>  usb_disconnect+0x285/0x7f0 drivers/usb/core/hub.c:2205
>  hub_port_connect drivers/usb/core/hub.c:4838
>  hub_port_connect_change drivers/usb/core/hub.c:5093
>  port_event drivers/usb/core/hub.c:5199
>  hub_event_impl+0x10ec/0x3440 drivers/usb/core/hub.c:5311
>  hub_event+0x38/0x50 drivers/usb/core/hub.c:5209
>  process_one_work+0x925/0x15d0 kernel/workqueue.c:2113
>  worker_thread+0xef/0x10d0 kernel/workqueue.c:2247
>  kthread+0x346/0x410 kernel/kthread.c:231
>  ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431
>
> The buggy address belongs to the page:
> page:ea0001a74b00 count:0 mapcount:-127 mapping:  (null) index:0x0
> flags: 0x100()
> raw: 0100   ff80
> raw: ea00019f0320 88007fffa690 0002 
> page dumped because: kasan: bad access detected
>
> Memory state around the buggy address:
>  880069d2c000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>  880069d2c080: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>>880069d2c100: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>   ^
>  880069d2c180: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>  880069d2c200: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> ==

-linux-ker...@vger.kernel.or
+linux-ker...@vger.kernel.org

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

[Nicolas Dufresne]

Le vendredi 03 novembre 2017 à 09:11 +0100, Marek Szyprowski a écrit :
> MFC driver supports only MMAP operation mode mainly due to the hardware
> restrictions of the addresses of the DMA buffers (MFC v5 hardware can
> access buffers only in 128MiB memory region starting from the base address
> of its firmware). When IOMMU is available, this requirement is easily
> fulfilled even for the buffers located anywhere in the memory - typically
> by mapping them in the DMA address space as close as possible to the
> firmware. Later hardware revisions don't have this limitations at all.
> 
> The second limitation of the MFC hardware related to the memory buffers
> is constant buffer address. Once the hardware has been initialized for
> operation on given buffer set, the addresses of the buffers cannot be
> changed.
> 
> With the above assumptions, a limited support for USERPTR and DMABUF
> operation modes can be added. The main requirement is to have all buffers
> known when starting hardware. This has been achieved by postponing
> hardware initialization once all the DMABUF or USERPTR buffers have been
> queued for the first time. Once then, buffers cannot be modified to point
> to other memory area.

I am concerned about enabling this support with existing userspace.
Userspace application will be left with some driver with this
limitation and other drivers without. I think it is harmful to enable
that feature without providing userspace the ability to know.

This is specially conflicting with let's say UVC driver providing
buffers, since UVC driver implementing CREATE_BUFS ioctl. So even if
userspace start making an effort to maintain the same DMABuf for the
same buffer index, if a new buffer is created, we won't be able to use
it.

> 
> This patch also removes unconditional USERPTR operation mode from encoder
> video node, because it doesn't work with v5 MFC hardware without IOMMU
> being enabled.
> 
> In case of MFC v5 a bidirectional queue flag has to be enabled as a
> workaround of the strange hardware behavior - MFC performs a few writes
> to source data during the operation.

Do you have more information about this ? It is quite terrible, since
if you enable buffer importation, the buffer might be multi-plex across
multiple encoder instance. That is another way this feature can be
harmful to existing userspace.

> 
> Signed-off-by: Seung-Woo Kim 
> [mszyprow: adapted to v4.14 code base, rewrote and extended commit message,
>  added checks for changing buffer addresses, added bidirectional queue
>  flags and comments]
> Signed-off-by: Marek Szyprowski 
> ---
> v2:
> - fixed copy/paste bug, which broke encoding support (thanks to Marian
>   Mihailescu for reporting it)
> - added checks for changing buffers DMA addresses
> - added bidirectional queue flags
> 
> v1:
> - inital version
> ---
>  drivers/media/platform/s5p-mfc/s5p_mfc.c |  23 +-
>  drivers/media/platform/s5p-mfc/s5p_mfc_dec.c | 111 
> +++
>  drivers/media/platform/s5p-mfc/s5p_mfc_enc.c |  64 +++
>  3 files changed, 147 insertions(+), 51 deletions(-)
> 
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c 
> b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> index 1839a86cc2a5..f1ab8d198158 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> @@ -754,6 +754,7 @@ static int s5p_mfc_open(struct file *file)
>   struct s5p_mfc_dev *dev = video_drvdata(file);
>   struct s5p_mfc_ctx *ctx = NULL;
>   struct vb2_queue *q;
> + unsigned int io_modes;
>   int ret = 0;
>  
>   mfc_debug_enter();
> @@ -839,16 +840,25 @@ static int s5p_mfc_open(struct file *file)
>   if (ret)
>   goto err_init_hw;
>   }
> +
> + io_modes = VB2_MMAP;
> + if (exynos_is_iommu_available(>plat_dev->dev) || !IS_TWOPORT(dev))
> + io_modes |= VB2_USERPTR | VB2_DMABUF;
> +
>   /* Init videobuf2 queue for CAPTURE */
>   q = >vq_dst;
>   q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
> + q->io_modes = io_modes;
> + /*
> +  * Destination buffers are always bidirectional, they use used as
> +  * reference data, which require READ access
> +  */
> + q->bidirectional = true;
>   q->drv_priv = >fh;
>   q->lock = >mfc_mutex;
>   if (vdev == dev->vfd_dec) {
> - q->io_modes = VB2_MMAP;
>   q->ops = get_dec_queue_ops();
>   } else if (vdev == dev->vfd_enc) {
> - q->io_modes = VB2_MMAP | VB2_USERPTR;
>   q->ops = get_enc_queue_ops();
>   } else {
>   ret = -ENOENT;
> @@ -869,13 +879,18 @@ static int s5p_mfc_open(struct file *file)
>   /* Init videobuf2 queue for OUTPUT */
>   q = >vq_src;
>   q->type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
> + q->io_modes = io_modes;
> + /*
> +  * MFV v5 performs write operations on source data, so make queue
> 

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

[Tobias Jakobi]

Tobias Jakobi wrote:
> Nicolas Dufresne wrote:
>> Le vendredi 03 novembre 2017 à 09:11 +0100, Marek Szyprowski a écrit :
>>> MFC driver supports only MMAP operation mode mainly due to the hardware
>>> restrictions of the addresses of the DMA buffers (MFC v5 hardware can
>>> access buffers only in 128MiB memory region starting from the base address
>>> of its firmware). When IOMMU is available, this requirement is easily
>>> fulfilled even for the buffers located anywhere in the memory - typically
>>> by mapping them in the DMA address space as close as possible to the
>>> firmware. Later hardware revisions don't have this limitations at all.
>>>
>>> The second limitation of the MFC hardware related to the memory buffers
>>> is constant buffer address. Once the hardware has been initialized for
>>> operation on given buffer set, the addresses of the buffers cannot be
>>> changed.
>>>
>>> With the above assumptions, a limited support for USERPTR and DMABUF
>>> operation modes can be added. The main requirement is to have all buffers
>>> known when starting hardware. This has been achieved by postponing
>>> hardware initialization once all the DMABUF or USERPTR buffers have been
>>> queued for the first time. Once then, buffers cannot be modified to point
>>> to other memory area.
>>
>> I am concerned about enabling this support with existing userspace.
>> Userspace application will be left with some driver with this
>> limitation and other drivers without. I think it is harmful to enable
>> that feature without providing userspace the ability to know.
> I voiced similar concern in my previous mail. I have to admit that I know very
> little about V4L2, but when I investigated this some time ago I saw that
> previously used/queued DMABUF buffers can be removed again, and, under certain
> conditions are 'reacquired', which issues a buf_cleanup() -- which does 
> nothing
> atm. My guess is that, in the worst case, one could trigger an IOMMU pagefault
> (or simply memory corruption, if not under IOMMU) this way.
> 
> Like Marek says, the hw needs to know DMA adresses of all available buffers on
> hw init. Hence a proper implementation would need to trigger some
> re-initialization once these adresses changes (DMABUFs 
> removed/reacquired/etc.).

Forgot to mention something here. Back when I looked at the code, I saw that the
MFC state machine has a state when the resolution of the video changes. IIRC
then this also triggers a reconfiguration of the hw buffers. I thought that
maybe one could re-use the states in the event of changed buffers. I didn't get
very far there, mainly because documentation of the whole state machine and the
interaction with the hw core is very sparse...

- Tobias




> These extra constraints which are introduced here, IMO they violate the API 
> then.
> 
> 
> 
>> This is specially conflicting with let's say UVC driver providing
>> buffers, since UVC driver implementing CREATE_BUFS ioctl. So even if
>> userspace start making an effort to maintain the same DMABuf for the
>> same buffer index, if a new buffer is created, we won't be able to use
>> it.
>>
>>>
>>> This patch also removes unconditional USERPTR operation mode from encoder
>>> video node, because it doesn't work with v5 MFC hardware without IOMMU
>>> being enabled.
>>>
>>> In case of MFC v5 a bidirectional queue flag has to be enabled as a
>>> workaround of the strange hardware behavior - MFC performs a few writes
>>> to source data during the operation.
>>
>> Do you have more information about this ? It is quite terrible, since
>> if you enable buffer importation, the buffer might be multi-plex across
>> multiple encoder instance. That is another way this feature can be
>> harmful to existing userspace.
> IIRC the hw stores some "housekeeping" data in the input buffers used during
> decoding. So input buffers are not strictly "input", but also output, in the
> following sense. If the buffer has total size N, and the data stored inside 
> has
> size n (with n < N), then the hw uses the remaining N-n bytes at the end of 
> the
> buffer for this housekeeping data. I'm not sure what happens if n equals N, or
> if that's even possible.
> 
> With best wishes,
> Tobias
> 
> 
>>
>>>
>>> Signed-off-by: Seung-Woo Kim 
>>> [mszyprow: adapted to v4.14 code base, rewrote and extended commit message,
>>>  added checks for changing buffer addresses, added bidirectional queue
>>>  flags and comments]
>>> Signed-off-by: Marek Szyprowski 
>>> ---
>>> v2:
>>> - fixed copy/paste bug, which broke encoding support (thanks to Marian
>>>   Mihailescu for reporting it)
>>> - added checks for changing buffers DMA addresses
>>> - added bidirectional queue flags
>>>
>>> v1:
>>> - inital version
>>> ---
>>>  drivers/media/platform/s5p-mfc/s5p_mfc.c |  23 +-
>>>  drivers/media/platform/s5p-mfc/s5p_mfc_dec.c | 111 
>>> +++
>>>  drivers/media/platform/s5p-mfc/s5p_mfc_enc.c |  

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

[Tobias Jakobi]

Nicolas Dufresne wrote:
> Le vendredi 03 novembre 2017 à 09:11 +0100, Marek Szyprowski a écrit :
>> MFC driver supports only MMAP operation mode mainly due to the hardware
>> restrictions of the addresses of the DMA buffers (MFC v5 hardware can
>> access buffers only in 128MiB memory region starting from the base address
>> of its firmware). When IOMMU is available, this requirement is easily
>> fulfilled even for the buffers located anywhere in the memory - typically
>> by mapping them in the DMA address space as close as possible to the
>> firmware. Later hardware revisions don't have this limitations at all.
>>
>> The second limitation of the MFC hardware related to the memory buffers
>> is constant buffer address. Once the hardware has been initialized for
>> operation on given buffer set, the addresses of the buffers cannot be
>> changed.
>>
>> With the above assumptions, a limited support for USERPTR and DMABUF
>> operation modes can be added. The main requirement is to have all buffers
>> known when starting hardware. This has been achieved by postponing
>> hardware initialization once all the DMABUF or USERPTR buffers have been
>> queued for the first time. Once then, buffers cannot be modified to point
>> to other memory area.
> 
> I am concerned about enabling this support with existing userspace.
> Userspace application will be left with some driver with this
> limitation and other drivers without. I think it is harmful to enable
> that feature without providing userspace the ability to know.
I voiced similar concern in my previous mail. I have to admit that I know very
little about V4L2, but when I investigated this some time ago I saw that
previously used/queued DMABUF buffers can be removed again, and, under certain
conditions are 'reacquired', which issues a buf_cleanup() -- which does nothing
atm. My guess is that, in the worst case, one could trigger an IOMMU pagefault
(or simply memory corruption, if not under IOMMU) this way.

Like Marek says, the hw needs to know DMA adresses of all available buffers on
hw init. Hence a proper implementation would need to trigger some
re-initialization once these adresses changes (DMABUFs removed/reacquired/etc.).
These extra constraints which are introduced here, IMO they violate the API 
then.



> This is specially conflicting with let's say UVC driver providing
> buffers, since UVC driver implementing CREATE_BUFS ioctl. So even if
> userspace start making an effort to maintain the same DMABuf for the
> same buffer index, if a new buffer is created, we won't be able to use
> it.
> 
>>
>> This patch also removes unconditional USERPTR operation mode from encoder
>> video node, because it doesn't work with v5 MFC hardware without IOMMU
>> being enabled.
>>
>> In case of MFC v5 a bidirectional queue flag has to be enabled as a
>> workaround of the strange hardware behavior - MFC performs a few writes
>> to source data during the operation.
> 
> Do you have more information about this ? It is quite terrible, since
> if you enable buffer importation, the buffer might be multi-plex across
> multiple encoder instance. That is another way this feature can be
> harmful to existing userspace.
IIRC the hw stores some "housekeeping" data in the input buffers used during
decoding. So input buffers are not strictly "input", but also output, in the
following sense. If the buffer has total size N, and the data stored inside has
size n (with n < N), then the hw uses the remaining N-n bytes at the end of the
buffer for this housekeeping data. I'm not sure what happens if n equals N, or
if that's even possible.

With best wishes,
Tobias


> 
>>
>> Signed-off-by: Seung-Woo Kim 
>> [mszyprow: adapted to v4.14 code base, rewrote and extended commit message,
>>  added checks for changing buffer addresses, added bidirectional queue
>>  flags and comments]
>> Signed-off-by: Marek Szyprowski 
>> ---
>> v2:
>> - fixed copy/paste bug, which broke encoding support (thanks to Marian
>>   Mihailescu for reporting it)
>> - added checks for changing buffers DMA addresses
>> - added bidirectional queue flags
>>
>> v1:
>> - inital version
>> ---
>>  drivers/media/platform/s5p-mfc/s5p_mfc.c |  23 +-
>>  drivers/media/platform/s5p-mfc/s5p_mfc_dec.c | 111 
>> +++
>>  drivers/media/platform/s5p-mfc/s5p_mfc_enc.c |  64 +++
>>  3 files changed, 147 insertions(+), 51 deletions(-)
>>
>> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c 
>> b/drivers/media/platform/s5p-mfc/s5p_mfc.c
>> index 1839a86cc2a5..f1ab8d198158 100644
>> --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
>> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
>> @@ -754,6 +754,7 @@ static int s5p_mfc_open(struct file *file)
>>  struct s5p_mfc_dev *dev = video_drvdata(file);
>>  struct s5p_mfc_ctx *ctx = NULL;
>>  struct vb2_queue *q;
>> +unsigned int io_modes;
>>  int ret = 0;
>>  
>>  mfc_debug_enter();
>> @@ 

[PATCH] uvc: Add D3DFMT_L8 support

[Nicolas Dufresne]

Microsoft HoloLense UVC sensor uses D3DFMT instead of FOURCC when
exposing formats. This add support for D3DFMT_L8 as exposed from
the Acer Windows Mixed Reality Headset.

Signed-off-by: Nicolas Dufresne 
---
 drivers/media/usb/uvc/uvc_driver.c | 5 +
 drivers/media/usb/uvc/uvcvideo.h   | 5 +
 2 files changed, 10 insertions(+)

diff --git a/drivers/media/usb/uvc/uvc_driver.c 
b/drivers/media/usb/uvc/uvc_driver.c
index 6d22b22cb35b..56f70851f88b 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -203,6 +203,11 @@ static struct uvc_format_desc uvc_fmts[] = {
.guid   = UVC_GUID_FORMAT_INZI,
.fcc= V4L2_PIX_FMT_INZI,
},
+   {
+   .name   = "Greyscale 8-bit (D3DFMT_L8)",
+   .guid   = UVC_GUID_FORMAT_D3DFMT_L8,
+   .fcc= V4L2_PIX_FMT_GREY,
+   },
 };
 
 /* 
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index 34c7ee6cc9e5..fbc1f433ff05 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -153,6 +153,11 @@
{ 'I',  'N',  'V',  'I', 0xdb, 0x57, 0x49, 0x5e, \
 0x8e, 0x3f, 0xf4, 0x79, 0x53, 0x2b, 0x94, 0x6f}
 
+#define UVC_GUID_FORMAT_D3DFMT_L8 \
+   {0x32, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x00, \
+0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
+
+
 /* 
  * Driver specific constants.
  */
-- 
2.13.6

[GIT PULL for 4.15] V4L2 async and imx274 fixes

[Sakari Ailus]

Hi Mauro,

Here are two bugfixes for V4L2 async and imx274.

Please pull.

The following changes since commit 9917fbcfa20ab987d6381fd0365665e5c1402d75:

  media: camss-vfe: always initialize reg at vfe_set_xbar_cfg() (2017-11-01 
12:25:59 -0400)

are available in the git repository at:

  ssh://linuxtv.org/git/sailus/media_tree.git for-4.15-6

for you to fetch changes up to f45a99c566983cb64726ce21ec24c51d80b144c3:

  media: v4l: async: fix return of unitialized variable ret (2017-11-03 
10:27:37 +0200)


Colin Ian King (2):
  media: imx274: fix missing return assignment from call to imx274_mode_regs
  media: v4l: async: fix return of unitialized variable ret

 drivers/media/i2c/imx274.c   | 2 +-
 drivers/media/v4l2-core/v4l2-async.c | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

-- 
Kind regards,

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

usb/media/dw2102: null-ptr-deref in dvb_usb_adapter_frontend_init/tt_s2_4600_frontend_attach

[Andrey Konovalov]

Hi!

I've got the following report while fuzzing the kernel with syzkaller.

On commit 3a99df9a3d14cd866b5516f8cba515a3bfd554ab (4.14-rc7+).

The report is a little confusing, as the top stack frame is not
actually present. As far as my debugging showed, the NULL pointer
that's being executed actually corresponds to
m88ds3103_pdata.get_dvb_frontend in tt_s2_4600_frontend_attach().

dw2102: su3000_identify_state
dvb-usb: found a 'TeVii S482 (tuner 1)' in warm state.
dw2102: su3000_power_ctrl: 1, initialized 0
dvb-usb: bulk message failed: -22 (2/-30720)
dvb-usb: will pass the complete MPEG2 transport stream to the software demuxer.
dvbdev: DVB: registering new adapter (TeVii S482 (tuner 1))
usb 1-1: media controller created
dvb-usb: bulk message failed: -22 (6/-30720)
dw2102: i2c transfer failed.
dvb-usb: bulk message failed: -22 (6/-30720)
dw2102: i2c transfer failed.
dvb-usb: bulk message failed: -22 (6/-30720)
dw2102: i2c transfer failed.
dvb-usb: bulk message failed: -22 (6/-30720)
dw2102: i2c transfer failed.
dvb-usb: bulk message failed: -22 (6/-30720)
dw2102: i2c transfer failed.
dvb-usb: bulk message failed: -22 (6/-30720)
dw2102: i2c transfer failed.
dvb-usb: MAC address: 02:02:02:02:02:02
dvbdev: dvb_create_media_entity: media entity 'dvb-demux' registered.
dvb-usb: bulk message failed: -22 (3/-30720)
dw2102: command 0x0e transfer failed.
dvb-usb: bulk message failed: -22 (3/-1)
dw2102: command 0x0e transfer failed.
dvb-usb: bulk message failed: -22 (3/-30720)
dw2102: command 0x0e transfer failed.
dvb-usb: bulk message failed: -22 (3/-1)
dw2102: command 0x0e transfer failed.
dvb-usb: bulk message failed: -22 (1/-1)
dw2102: command 0x51 transfer failed.
dvb-usb: bulk message failed: -22 (5/-30720)
dw2102: i2c transfer failed.
BUG: unable to handle kernel NULL pointer dereference at   (null)
IP:   (null)
PGD 6a9fb067 P4D 6a9fb067 PUD 684a4067 PMD 0
Oops: 0010 [#1] PREEMPT SMP KASAN
Modules linked in:
CPU: 1 PID: 40 Comm: kworker/1:1 Not tainted 4.14.0-rc7-44290-gf28444df2601 #50
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Workqueue: usb_hub_wq hub_event
task: 88006bfe9700 task.stack: 88006b89
RIP: 0010:  (null)
RSP: 0018:88006b8973d0 EFLAGS: 00010293
RAX: 88006bfe9700 RBX: 880069f77780 RCX: 840c0153
RDX:  RSI: 840c0161 RDI: 880060bc1980
RBP: 88006b8974b8 R08: 88006bfe9700 R09: 0005
R10: 88006bfe9700 R11: a23aacbae336f3e6 R12: 880060bc1980
R13: 8800629e5f00 R14: ffea R15: 8800629e56d8
FS:  () GS:88006cb0() knlGS:
CS:  0010 DS:  ES:  CR0: 80050033
CR2:  CR3: 6349a000 CR4: 06e0
Call Trace:
 dvb_usb_adapter_frontend_init+0x358/0x4b0
drivers/media/usb/dvb-usb/dvb-usb-dvb.c:286
 dvb_usb_adapter_init drivers/media/usb/dvb-usb/dvb-usb-init.c:86
 dvb_usb_init drivers/media/usb/dvb-usb/dvb-usb-init.c:162
 dvb_usb_device_init.cold.7+0x971/0x1029
drivers/media/usb/dvb-usb/dvb-usb-init.c:277
 dw2102_probe+0xa67/0xc50 drivers/media/usb/dvb-usb/dw2102.c:2406
 usb_probe_interface+0x324/0x940 drivers/usb/core/driver.c:361
 really_probe drivers/base/dd.c:413
 driver_probe_device+0x522/0x740 drivers/base/dd.c:557
 __device_attach_driver+0x25d/0x2d0 drivers/base/dd.c:653
 bus_for_each_drv+0xff/0x160 drivers/base/bus.c:463
 __device_attach+0x1a8/0x2a0 drivers/base/dd.c:710
 device_initial_probe+0x1f/0x30 drivers/base/dd.c:757
 bus_probe_device+0x1fc/0x2a0 drivers/base/bus.c:523
 device_add+0xc27/0x15a0 drivers/base/core.c:1835
 usb_set_configuration+0xd4f/0x17a0 drivers/usb/core/message.c:1932
 generic_probe+0xbb/0x120 drivers/usb/core/generic.c:174
 usb_probe_device+0xab/0x100 drivers/usb/core/driver.c:266
 really_probe drivers/base/dd.c:413
 driver_probe_device+0x522/0x740 drivers/base/dd.c:557
 __device_attach_driver+0x25d/0x2d0 drivers/base/dd.c:653
 bus_for_each_drv+0xff/0x160 drivers/base/bus.c:463
 __device_attach+0x1a8/0x2a0 drivers/base/dd.c:710
 device_initial_probe+0x1f/0x30 drivers/base/dd.c:757
 bus_probe_device+0x1fc/0x2a0 drivers/base/bus.c:523
 device_add+0xc27/0x15a0 drivers/base/core.c:1835
 usb_new_device+0x7fa/0x1090 drivers/usb/core/hub.c:2538
 hub_port_connect drivers/usb/core/hub.c:4987
 hub_port_connect_change drivers/usb/core/hub.c:5093
 port_event drivers/usb/core/hub.c:5199
 hub_event_impl+0x17b8/0x3440 drivers/usb/core/hub.c:5311
 hub_event+0x38/0x50 drivers/usb/core/hub.c:5209
 process_one_work+0x925/0x15d0 kernel/workqueue.c:2113
 worker_thread+0xef/0x10d0 kernel/workqueue.c:2247
 kthread+0x346/0x410 kernel/kthread.c:231
 ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431
Code:  Bad RIP value.
RIP:   (null) RSP: 88006b8973d0
CR2: 
---[ end trace ab991a6d52472450 ]---

Re: [PATCH v1 15/15] media: i2c: adv748x: Remove duplicate NULL check

[Kieran Bingham]

Hi Andy,

Thankyou for the patch.

On 31/10/17 14:21, Andy Shevchenko wrote:
> Since i2c_unregister_device() became NULL-aware we may remove duplicate
> NULL check.
> 
> Cc: Kieran Bingham 
> Cc: Mauro Carvalho Chehab 
> Cc: linux-media@vger.kernel.org
> Signed-off-by: Andy Shevchenko 

Acked-by: Kieran Bingham 

> ---
>  drivers/media/i2c/adv748x/adv748x-core.c | 6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/i2c/adv748x/adv748x-core.c 
> b/drivers/media/i2c/adv748x/adv748x-core.c
> index 5ee14f2c2747..10c3d469175c 100644
> --- a/drivers/media/i2c/adv748x/adv748x-core.c
> +++ b/drivers/media/i2c/adv748x/adv748x-core.c
> @@ -225,10 +225,8 @@ static void adv748x_unregister_clients(struct 
> adv748x_state *state)
>  {
>   unsigned int i;
>  
> - for (i = 1; i < ARRAY_SIZE(state->i2c_clients); ++i) {
> - if (state->i2c_clients[i])
> - i2c_unregister_device(state->i2c_clients[i]);
> - }
> + for (i = 1; i < ARRAY_SIZE(state->i2c_clients); ++i)
> + i2c_unregister_device(state->i2c_clients[i]);
>  }
>  
>  static int adv748x_initialise_clients(struct adv748x_state *state)
> 

[PATCH] media: imx274: fix missing return assignment from call to imx274_mode_regs

[Colin King]

From: Colin Ian King 

The variable ret is being checked for failure however it is not being set
from the return status from the call to imx274_mode_regs. Currently ret is
alwayus zero and the check is redundant. Fix this by assigning it.

Detected by CoverityScan, CID#1460278 ("Logically dead code")

Fixes: 0985dd306f72 ("media: imx274: V4l2 driver for Sony imx274 CMOS sensor")
Signed-off-by: Colin Ian King 
---
 drivers/media/i2c/imx274.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/imx274.c b/drivers/media/i2c/imx274.c
index ab6a5f31da74..800b9bf9cdd3 100644
--- a/drivers/media/i2c/imx274.c
+++ b/drivers/media/i2c/imx274.c
@@ -1059,7 +1059,7 @@ static int imx274_s_stream(struct v4l2_subdev *sd, int on)
 
if (on) {
/* load mode registers */
-   imx274_mode_regs(imx274, imx274->mode_index);
+   ret = imx274_mode_regs(imx274, imx274->mode_index);
if (ret)
goto fail;
 
-- 
2.14.1

Re: [PATCH v9 2/2] media:imx274 V4l2 driver for Sony imx274 CMOS sensor

[Leon Luo]

Hi Vishal,

In probe(), it calls imx274_load_default(imx274), which has I2C
register read/write to the IMX274. If it fails, it will exit probe().
So it works as a sensor detection function as you suggested.
Regards,
Leon Luo
1130 Cadillac CT
Milpitas, CA 95035
Phone: (510)371-1169
Fax: (408) 217-1960
Email: le...@leopardimaging.com
www.leopardimaging.com


On Thu, Nov 2, 2017 at 9:36 PM, Vishal Sagar  wrote:
> Hi Leon,
>
> I understand this fixes correctly freeing the v4l control handlers in probe().
>
> But if there is a scenario where the sensor is mounted on a removable 
> daughter card,
> shouldn't the probe fail if the daughter card is not connected?
> A sample read/write to an IMX274 register should be sufficient to confirm 
> this in the probe() and fail.
>
> Does it make sense to add this in the probe()?
>
> Regards
> Vishal Sagar
>
>> -Original Message-
>> From: linux-media-ow...@vger.kernel.org [mailto:linux-media-
>> ow...@vger.kernel.org] On Behalf Of Leon Luo
>> Sent: Thursday, October 26, 2017 12:21 PM
>> To: mche...@kernel.org; robh...@kernel.org; mark.rutl...@arm.com;
>> hans.verk...@cisco.com; sakari.ai...@linux.intel.com
>> Cc: linux-media@vger.kernel.org; devicet...@vger.kernel.org; linux-
>> ker...@vger.kernel.org; le...@leopardimaging.com; Soren Brinkmann
>> 
>> Subject: [PATCH v9 2/2] media:imx274 V4l2 driver for Sony imx274 CMOS sensor
>>
>> The imx274 is a Sony CMOS image sensor that has 1/2.5 image size.
>> It supports up to 3840x2160 (4K) 60fps, 1080p 120fps. The interface
>> is 4-lane MIPI CSI-2 running at 1.44Gbps each.
>>
>> This driver has been tested on Xilinx ZCU102 platform with a Leopard
>> LI-IMX274MIPI-FMC camera board.
>>
>> Support for the following features:
>> -Resolutions: 3840x2160, 1920x1080, 1280x720
>> -Frame rate: 3840x2160 : 5 – 60fps
>> 1920x1080 : 5 – 120fps
>> 1280x720 : 5 – 120fps
>> -Exposure time: 16 – (frame interval) micro-seconds
>> -Gain: 1x - 180x
>> -VFLIP: enable/disabledrivers/media/i2c/imx274.c
>> -Test pattern: 12 test patterns
>>
>> Signed-off-by: Leon Luo 
>> Tested-by: Sören Brinkmann 
>> Acked-by: Sakari Ailus 
>> Acked-by: Chris Kohn 
>> ---
>> v9:
>>  - remove v4l2_async_unregister_subdev from probe error
>> v8:
>>  - check division by zero error
>> v7:
>>  - use __v4l2_ctrl_s_ctrl instead of v4l2_ctrl_s_ctrl to have
>>clean mutex_lock/mutex_unlock in imx274_s_stream()
>>  - define imx274_tp_regs[] as static, move the test pattern reg
>>out of imx274_tp_regs[], and define it as a macro
>>IMX274_TEST_PATTERN_REG
>> v6:
>>  - remove media/v4l2-image-sizes.h from include header
>>  - make the header file alphabetical order
>>  - remove fmt->pad check in imx274_get_fmt,
>>the V4L2 subdev framework does it already
>>  - change 'struct reg_8 *regs' to 'struct reg_8 regs[n]',
>>where n is the exact numbers needed for this function
>>  - move MODULE_DEVICE_TABLE(of, imx274_of_id_table); closer
>>to imx274_of_id_table definition
>>  - remove return check of imx274_write_table in imx274_remove,
>>because it should remove all resources even i2c fails here
>>  - move imx274_load_default before v4l2_async_register_subdev
>> v5:
>>  - no changes
>> v4:
>>  - use 32-bit data type to avoid __divdi3 compile error for i386
>>  - clean up OR together error codesdrivers/media/i2c/imx274.c
>> v3:
>>  - clean up header files
>>  - use struct directly instead of alias #define
>>  - use v4l2_ctrl_s_ctrl instead of v4l2_s_ctrl
>>  - revise debug output
>>  - move static helpers closer to their call site
>>  - don't OR toegether error codes
>>  - use closest valid gain value instead of erroring out
>>  - assigne lock to the control handler and omit explicit locking
>>  - acquire mutex lock for imx274_get_fmt
>>  - remove format->pad check in imx274_set_fmt since the pad is always 0
>>  - pass struct v4l2_ctrl pointer in gain/exposure/vlip/test pattern controls
>>  - remove priv->ctrls.vflip->val = val in imx274_set_vflip()
>>  - remove priv->ctrls.test_pattern->val = val in imx274_set_test_pattern()
>>  - remove empty open/close callbacks
>>  - remove empty core ops
>>  - add more error labels in probe
>>  - use v4l2_async_unregister_subdev instead of v4l2_device_unregister_subdev
>>  - use dynamic debug
>>  - split start_stream to two steps: imx274_mode_regs() and
>> imx274_start_stream()
>>frame rate & exposure can be updated
>>between imx274_mode_regs() & imx274_start_stream()
>>
>> v2:
>>  - Fix Kconfig to not remove existing options
>> ---
>>  drivers/media/i2c/Kconfig  |7 +
>>  drivers/media/i2c/Makefile |1 +
>>  drivers/media/i2c/imx274.c | 1810
>> 
>>  3 files changed, 1818 insertions(+)
>>  create mode 100644 drivers/media/i2c/imx274.c
>>
>> diff --git a/drivers/media/i2c/Kconfig 

[PATCH] media: v4l: async: fix unregister for implicitly registered sub-device notifiers

[Niklas Söderlund]

The commit aef69d54755d45ed ("media: v4l: fwnode: Add a convenience
function for registering sensors") adds the function
v4l2_async_notifier_parse_fwnode_sensor_common() to parse and register a
subdevice and a subdev-notifier by parsing firmware information. This
new subdev-notifier is stored in the new field 'subdev_notifier' in
struct v4l2_subdev.

In v4l2_async_unregister_subdev() this field is used to unregister and
cleanup the subdev-notifier. A check for if the subdev-notifier is
initialized or not was forgotten leading to a NULL pointer dereference
in v4l2_async_notifier_cleanup() if a subdevice do not use the optional
convince function to initialize the field.

Fix this by checking in v4l2_async_notifier_cleanup() that it is
provided whit a notifier making it safe to call with a NULL parameter.

Fixes: aef69d54755d45ed ("media: v4l: fwnode: Add a convenience function for 
registering sensors")
Signed-off-by: Niklas Söderlund 
---
 drivers/media/v4l2-core/v4l2-async.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/v4l2-core/v4l2-async.c 
b/drivers/media/v4l2-core/v4l2-async.c
index 49f7eccc76dbbc3b..8684e002e72f280d 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -502,7 +502,7 @@ void v4l2_async_notifier_cleanup(struct v4l2_async_notifier 
*notifier)
 {
unsigned int i;
 
-   if (!notifier->max_subdevs)
+   if (!notifier || !notifier->max_subdevs)
return;
 
for (i = 0; i < notifier->num_subdevs; i++) {
-- 
2.14.2

Re: [PATCH v2] media: ttpci: remove autorepeat handling and use timer_setup

[Kees Cook]

On Fri, Nov 3, 2017 at 3:17 PM, Dmitry Torokhov
 wrote:
> On Thu, Nov 02, 2017 at 10:16:58PM -0200, Mauro Carvalho Chehab wrote:
>> Em Thu, 2 Nov 2017 16:50:37 -0700
>> Dmitry Torokhov  escreveu:
>>
>> > On Thu, Nov 02, 2017 at 04:24:27PM -0700, Kees Cook wrote:
>> > > On Tue, Oct 31, 2017 at 1:11 PM, Sean Young  wrote:
>> > > > Leave the autorepeat handling up to the input layer, and move
>> > > > to the new timer API.
>> > > >
>> > > > Compile tested only.
>> > > >
>> > > > Signed-off-by: Sean Young 
>> > >
>> > > Hi! Just checking up on this... the input timer conversion is blocked
>> > > by getting this sorted out, so I'd love to have something either
>> > > media, input, or timer tree can carry. :)
>> >
>> > Acked-by: Dmitry Torokhov 
>> >
>> > From my POV the patch is good. Mauro, do you want me to take it through
>> > my tree, or maybe you could create an immutable branch off 4.14-rc5 (or
>> > 6) with this commit and I will pull it in and then can apply Kees input
>> > core conversion patch?
>>
>> Feel free to apply it into your tree with my ack:
>>
>> Acked-by: Mauro Carvalho Chehab 
>
> Applied and pulled Kees' patch to the input core (dropping the timer_data
> business) on top.

Awesome, thanks! :)

-Kees

-- 
Kees Cook
Pixel Security

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

[Tim Harvey]

On Mon, Oct 23, 2017 at 10:05 AM, Tim Harvey  wrote:
>
> On Fri, Oct 20, 2017 at 9:23 AM, Hans Verkuil  wrote:
>
> >>
> >> I see the AVI infoframe has hdmi_quantization_range and
> >> hdmi_ycc_quantization_range along with vid_code.
> >>
> >> I'm not at all clear what to do with this information. Is there
> >> anything you see in the datasheet [1] that points to something I need
> >> to be doing?
> >
> > You can ignore hdmi_ycc_quantization_range, it is the 
> > hdmi_quantization_range
> > that you need to read out.
> >
> > The TDA can receive the following formats:
> >
> > RGB Full Range
> > RGB Limited Range
> > YUV Bt.601 (aka SMPTE 170M)
> > YUV Rec.709
> >
> > The YUV formats are always limited range.
> >
> > The TDA can transmit RGB and YUV to the SoC. You want RGB to be full range 
> > and
> > YUV to be limited range. YUV can be either 601 or 709.
> >
> > So if the TDA transmits RGB then you need to support the following 
> > conversions:
> >
> > RGB Full -> RGB Full
> > RGB Limited -> RGB Full
> > YUV 601 -> RGB Full
> > YUV 709 -> RGB Full
> >
> > And if the TDA transmits YUV then you need these conversions:
> >
> > RGB Full -> YUV601 or YUV709
> > RGB Limited -> YUV601 or YUV709
> > YUV601 -> YUV601
> > YUV709 -> YUV709
> >
> > For the RGB to YUV conversion you have a choice of converting to YUV601 or 
> > 709.
> > I recommend to either always convert to YUV601 or to let it depend on the 
> > resolution
> > (SDTV YUV601, HDTV YUV709).
> >
>
> Ok - this is a good explanation that I should be able to follow. I
> will make sure to take into account hdmi_quantization_range when I
> setup the colorspace conversion matrix for v3.

Hans,

I'm having trouble figuring out the conversion matrix to use between
limited and full.

Currently I have the following conversion matrices, the values which
came from some old vendor code:

/* Colorspace conversion matrix coefficients and offsets */
struct color_matrix_coefs {
/* Input offsets */
s16 offint1;
s16 offint2;
s16 offint3;
/* Coeficients */
s16 p11coef;
s16 p12coef;
s16 p13coef;
s16 p21coef;
s16 p22coef;
s16 p23coef;
s16 p31coef;
s16 p32coef;
s16 p33coef;
/* Output offsets */
s16 offout1;
s16 offout2;
s16 offout3;
};
/* Conversion matrixes */
enum {
ITU709_RGBLIMITED,
ITU601_RGBLIMITED,
RGBLIMITED_ITU601,
   };
   static const struct color_matrix_coefs conv_matrix[] = {
/* ITU709 -> RGBLimited */
{
-256, -2048,  -2048,
4096, -1875,   -750,
4096,  6307,  0,
4096, 0,   7431,
 256,   256,256,
},
/* YUV601 limited -> RGB limited */
{
-256, -2048,  -2048,
4096, -2860,  -1378,
4096,  5615,  0,
4096, 0,   7097,
256,256,256,
},
/* RGB limited -> ITU601 */
{
-256,  -256,   -256,
2404,  1225,467,
-1754, 2095,   -341,
-1388, -707,   2095,
256,   2048,   2048,
},
};

Assuming the above are correct this leaves me missing RGB limitted ->
RGB full, YUV601 -> RGB full, YUV709 -> RGB full, and RGB Full ->
YUV601.

I don't have documentation for the registers but I'm assuming the
input offset is applied first, then the multiplication by the coef,
then the output offset is applied. I'm looking over
https://en.wikipedia.org/wiki/YUV for colorspace conversion matrices
but I'm unable to figure out how to apply those to the above. Any
ideas?

Thanks,

Tim

[PATCH v2 2/2] media: s5p-mfc: fix lock confection - request_firmware() once and keep state

[Shuah Khan]

Driver calls request_firmware() whenever the device is opened for the
first time. As the device gets opened and closed, dev->num_inst == 1
is true several times. This is not necessary since the firmware is saved
in the fw_buf. s5p_mfc_load_firmware() copies the buffer returned by
the request_firmware() to dev->fw_buf.

fw_buf sticks around until it gets released from s5p_mfc_remove(), hence
there is no need to keep requesting firmware and copying it to fw_buf.

This might have been overlooked when changes are made to free fw_buf from
the device release interface s5p_mfc_release().

Fix s5p_mfc_load_firmware() to call request_firmware() once and keep state.
Change _probe() to load firmware once fw_buf has been allocated.

s5p_mfc_open() and it continues to call s5p_mfc_load_firmware() and init
hardware which is the step where firmware is written to the device.

This addresses the mfc_mutex contention due to repeated request_firmware()
calls from open() in the following circular locking warning:

[  552.194115] qtdemux0:sink/2710 is trying to acquire lock:
[  552.199488]  (>mfc_mutex){+.+.}, at: [] 
s5p_mfc_mmap+0x28/0xd4 [s5p_mfc]
[  552.207459]
   but task is already holding lock:
[  552.213264]  (>mmap_sem){}, at: [] vm_mmap_pgoff+0x44/0xb8
[  552.220284]
   which lock already depends on the new lock.

[  552.228429]
   the existing dependency chain (in reverse order) is:
[  552.235881]
   -> #2 (>mmap_sem){}:
[  552.241259]__might_fault+0x80/0xb0
[  552.245331]filldir64+0xc0/0x2f8
[  552.249144]call_filldir+0xb0/0x14c
[  552.253214]ext4_readdir+0x768/0x90c
[  552.257374]iterate_dir+0x74/0x168
[  552.261360]SyS_getdents64+0x7c/0x1a0
[  552.265608]ret_fast_syscall+0x0/0x28
[  552.269850]
   -> #1 (>i_mutex_dir_key#2){}:
[  552.276180]down_read+0x48/0x90
[  552.279904]lookup_slow+0x74/0x178
[  552.283889]walk_component+0x1a4/0x2e4
[  552.288222]link_path_walk+0x174/0x4a0
[  552.292555]path_openat+0x68/0x944
[  552.296541]do_filp_open+0x60/0xc4
[  552.300528]file_open_name+0xe4/0x114
[  552.304772]filp_open+0x28/0x48
[  552.308499]kernel_read_file_from_path+0x30/0x78
[  552.313700]_request_firmware+0x3ec/0x78c
[  552.318291]request_firmware+0x3c/0x54
[  552.322642]s5p_mfc_load_firmware+0x54/0x150 [s5p_mfc]
[  552.328358]s5p_mfc_open+0x4e4/0x550 [s5p_mfc]
[  552.94]v4l2_open+0xa0/0x104 [videodev]
[  552.338137]chrdev_open+0xa4/0x18c
[  552.342121]do_dentry_open+0x208/0x310
[  552.346454]path_openat+0x28c/0x944
[  552.350526]do_filp_open+0x60/0xc4
[  552.354512]do_sys_open+0x118/0x1c8
[  552.358586]ret_fast_syscall+0x0/0x28
[  552.362830]
   -> #0 (>mfc_mutex){+.+.}:
   -> #0 (>mfc_mutex){+.+.}:
[  552.368379]lock_acquire+0x6c/0x88
[  552.372364]__mutex_lock+0x68/0xa34
[  552.376437]mutex_lock_interruptible_nested+0x1c/0x24
[  552.382086]s5p_mfc_mmap+0x28/0xd4 [s5p_mfc]
[  552.386939]v4l2_mmap+0x54/0x88 [videodev]
[  552.391601]mmap_region+0x3a8/0x638
[  552.395673]do_mmap+0x330/0x3a4
[  552.399400]vm_mmap_pgoff+0x90/0xb8
[  552.403472]SyS_mmap_pgoff+0x90/0xc0
[  552.407632]ret_fast_syscall+0x0/0x28
[  552.411876]
   other info that might help us debug this:

[  552.419848] Chain exists of:
 >mfc_mutex --> >i_mutex_dir_key#2 --> >mmap_sem

[  552.431200]  Possible unsafe locking scenario:

[  552.437092]CPU0CPU1
[  552.441598]
[  552.446104]   lock(>mmap_sem);
[  552.449484]lock(>i_mutex_dir_key#2);
[  552.456329]lock(>mmap_sem);
[  552.46]   lock(>mfc_mutex);
[  552.465775]
*** DEADLOCK ***

Signed-off-by: Shuah Khan 
---
 drivers/media/platform/s5p-mfc/s5p_mfc.c| 6 ++
 drivers/media/platform/s5p-mfc/s5p_mfc_common.h | 3 +++
 drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c   | 5 +
 3 files changed, 14 insertions(+)

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c 
b/drivers/media/platform/s5p-mfc/s5p_mfc.c
index 5cf50b3..7b9a322 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
@@ -1311,6 +1311,12 @@ static int s5p_mfc_probe(struct platform_device *pdev)
goto err_dma;
}
 
+   /*
+* Load fails if fs isn't mounted. Try loading anyway.
+* _open() will load it, it it fails now. Ignore failure.
+*/
+   s5p_mfc_load_firmware(dev);
+
mutex_init(>mfc_mutex);
init_waitqueue_head(>queue);
dev->hw_lock = 0;
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h 

[PATCH v2 1/2] media: s5p-mfc: remove firmware buf null check in s5p_mfc_load_firmware()

[Shuah Khan]

s5p_mfc_load_firmware() will not get called if fw_buf.virt allocation
fails. The allocation happens very early on in the probe routine and
probe fails if allocation fails.

There is no need to check if it is null in s5p_mfc_load_firmware().
Remove the check.

Signed-off-by: Shuah Khan 
---
 drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c 
b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c
index 69ef9c2..46c9d67 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c
@@ -75,11 +75,6 @@ int s5p_mfc_load_firmware(struct s5p_mfc_dev *dev)
release_firmware(fw_blob);
return -ENOMEM;
}
-   if (!dev->fw_buf.virt) {
-   mfc_err("MFC firmware is not allocated\n");
-   release_firmware(fw_blob);
-   return -EINVAL;
-   }
memcpy(dev->fw_buf.virt, fw_blob->data, fw_blob->size);
wmb();
release_firmware(fw_blob);
-- 
2.7.4

[PATCH v2 0/2] Fix s5p-mfc lock contention in request firmware paths

[Shuah Khan]

This patch series fixes inefficiencies and lock contention in the request
firmware paths.

Changes since v2:
- Addressed Andre's review comments. Removed fw_buf->virt null check
  as it is not needed. Removed handling s5p_mfc_load_firmware() from
  probe routine. Simply try loading in case it works.

Shuah Khan (2):
  media: s5p-mfc: remove firmware buf null check in
s5p_mfc_load_firmware()
  media: s5p-mfc: fix lock confection - request_firmware() once and keep
state

 drivers/media/platform/s5p-mfc/s5p_mfc.c|  6 ++
 drivers/media/platform/s5p-mfc/s5p_mfc_common.h |  3 +++
 drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c   | 10 +-
 3 files changed, 14 insertions(+), 5 deletions(-)

-- 
2.7.4

Wir bieten jedem ein GÜNSTIGES Darlehnen zu TOP Konditionen an

[Martin Kelly]

Sehr geehrte Damen und Herren,

Sie brauchen Geld? Sie sind auf der suche nach einem Darlehnen? Seriös und 
unkompliziert?
Dann sind Sie hier bei uns genau richtig.
Durch unsere jahrelange Erfahrung und kompetente Beratung sind wir Europaweit 
tätig.

Wir bieten jedem ein GÜNSTIGES Darlehnen zu TOP Konditionen an. 
Darlehnen zwischen 5000 CHF/Euro bis zu 20 Millionen CHF/Euro möglich.
Wir erheben dazu 2% Zinssatz.

Lassen Sie sich von unserem kompetenten Team beraten. 

Zögern Sie nicht und kontaktieren Sie mich unter für weitere Infos & Anfragen 
unter
der eingeblendeten Email Adresse: 
Ich freue mich von Ihnen zu hören.

Re: [PATCH v2] media: ttpci: remove autorepeat handling and use timer_setup

[Dmitry Torokhov]

On Thu, Nov 02, 2017 at 10:16:58PM -0200, Mauro Carvalho Chehab wrote:
> Em Thu, 2 Nov 2017 16:50:37 -0700
> Dmitry Torokhov  escreveu:
> 
> > On Thu, Nov 02, 2017 at 04:24:27PM -0700, Kees Cook wrote:
> > > On Tue, Oct 31, 2017 at 1:11 PM, Sean Young  wrote:  
> > > > Leave the autorepeat handling up to the input layer, and move
> > > > to the new timer API.
> > > >
> > > > Compile tested only.
> > > >
> > > > Signed-off-by: Sean Young   
> > > 
> > > Hi! Just checking up on this... the input timer conversion is blocked
> > > by getting this sorted out, so I'd love to have something either
> > > media, input, or timer tree can carry. :)  
> > 
> > Acked-by: Dmitry Torokhov 
> > 
> > From my POV the patch is good. Mauro, do you want me to take it through
> > my tree, or maybe you could create an immutable branch off 4.14-rc5 (or
> > 6) with this commit and I will pull it in and then can apply Kees input
> > core conversion patch?
> 
> Feel free to apply it into your tree with my ack:
> 
> Acked-by: Mauro Carvalho Chehab 

Applied and pulled Kees' patch to the input core (dropping the timer_data
business) on top.

Thanks.

-- 
Dmitry

[PATCH 1/1] s2255drv: update firmware load.

[Dean Anderson]

fixes intermittent soft reboot issue with firmware load
increases wait time of reset, as required by HW

Signed-off-by: Dean Anderson 
---
 drivers/media/usb/s2255/s2255drv.c | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/media/usb/s2255/s2255drv.c 
b/drivers/media/usb/s2255/s2255drv.c
index b2f239c..8a8e314 100644
--- a/drivers/media/usb/s2255/s2255drv.c
+++ b/drivers/media/usb/s2255/s2255drv.c
@@ -350,7 +350,7 @@ static void s2255_fillbuff(struct s2255_vc *vc, struct 
s2255_buffer *buf,
   int jpgsize);
 static int s2255_set_mode(struct s2255_vc *vc, struct s2255_mode *mode);
 static int s2255_board_shutdown(struct s2255_dev *dev);
-static void s2255_fwload_start(struct s2255_dev *dev, int reset);
+static void s2255_fwload_start(struct s2255_dev *dev);
 static void s2255_destroy(struct s2255_dev *dev);
 static long s2255_vendor_req(struct s2255_dev *dev, unsigned char req,
 u16 index, u16 value, void *buf,
@@ -476,7 +476,7 @@ static void planar422p_to_yuv_packed(const unsigned char 
*in,
 static void s2255_reset_dsppower(struct s2255_dev *dev)
 {
s2255_vendor_req(dev, 0x40, 0x, 0x0001, NULL, 0, 1);
-   msleep(20);
+   msleep(50);
s2255_vendor_req(dev, 0x50, 0x, 0x, NULL, 0, 1);
msleep(600);
s2255_vendor_req(dev, 0x10, 0x, 0x, NULL, 0, 1);
@@ -1449,7 +1449,7 @@ static int s2255_open(struct file *file)
case S2255_FW_FAILED:
s2255_dev_err(>udev->dev,
"firmware load failed. retrying.\n");
-   s2255_fwload_start(dev, 1);
+   s2255_fwload_start(dev);
wait_event_timeout(dev->fw_data->wait_fw,
   ((atomic_read(>fw_data->fw_state)
 == S2255_FW_SUCCESS) ||
@@ -2208,10 +2208,9 @@ static void s2255_stop_readpipe(struct s2255_dev *dev)
return;
 }
 
-static void s2255_fwload_start(struct s2255_dev *dev, int reset)
+static void s2255_fwload_start(struct s2255_dev *dev)
 {
-   if (reset)
-   s2255_reset_dsppower(dev);
+   s2255_reset_dsppower(dev);
dev->fw_data->fw_size = dev->fw_data->fw->size;
atomic_set(>fw_data->fw_state, S2255_FW_NOTLOADED);
memcpy(dev->fw_data->pfw_data,
@@ -2336,7 +2335,7 @@ static int s2255_probe(struct usb_interface *interface,
retval = s2255_board_init(dev);
if (retval)
goto errorBOARDINIT;
-   s2255_fwload_start(dev, 0);
+   s2255_fwload_start(dev);
/* loads v4l specific */
retval = s2255_probe_v4l(dev);
if (retval)
-- 
2.7.4

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:   Sat Nov  4 05:00:27 CET 2017
media-tree git hash:9917fbcfa20ab987d6381fd0365665e5c1402d75
media_build git hash:   c93534951f5d66bef7f17f16293acf2be346b726
v4l-utils git hash: f28ea7e869751e284b739fb226c3d26e9f8c2b1a
gcc version:i686-linux-gcc (GCC) 7.1.0
sparse version: v0.5.0
smatch version: v0.5.0-3553-g78b2ea6
host hardware:  x86_64
host os:4.12.0-164

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

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

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

Re: [RFC v4 09/17] [media] vb2: add 'ordered_in_vb2' property to queues

[Hans Verkuil]

On 10/20/2017 11:50 PM, Gustavo Padovan wrote:
> From: Gustavo Padovan 
> 
> By setting this member on vb2_queue the driver tell vb2 core that
> it requires the buffers queued in QBUF to be queued with same order to the
> driver.

As we discussed in Prague this appears to be an unnecessary flag. In fact,
as far as I can tell it isn't set at all in this patch series.

Regards,

Hans

> 
> Signed-off-by: Gustavo Padovan 
> ---
>  include/media/videobuf2-core.h | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index 6dd3f0181107..fc333e10e7d8 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -505,6 +505,9 @@ struct vb2_buf_ops {
>   *   the same order they are dequeued from the driver. The default
>   *   is not ordered unless the driver sets this flag. As of now it
>   *   is mandatory for using explicit fences.
> + * @ordered_in_vb2: set by the driver to tell vb2 te guarantee the order
> + *   of buffer queue from userspace with QBUF() until they are
> + *   queued to the driver.
>   * @fileio:  file io emulator internal data, used only if emulator is active
>   * @threadio:thread io internal data, used only if thread is active
>   */
> @@ -558,6 +561,7 @@ struct vb2_queue {
>   unsigned intcopy_timestamp:1;
>   unsigned intlast_buffer_dequeued:1;
>   unsigned intordered_in_driver:1;
> + unsigned intordered_in_vb2:1;
>  
>   struct vb2_fileio_data  *fileio;
>   struct vb2_threadio_data*threadio;
> 

Re: [PATCH] media: v4l: async: fix return of unitialized variable ret

[Niklas Söderlund]

Hi Colin,

Thanks for your patch.

On 2017-11-03 06:58:27 +, Colin King wrote:
> From: Colin Ian King 
> 
> A shadow declaration of variable ret is being assigned a return error
> status and this value is being lost when the error exit goto's jump
> out of the local scope. This leads to an uninitalized error return value
> in the outer scope being returned. Fix this by removing the inner scoped
> declaration of variable ret.
> 
> Detected by CoverityScan, CID#1460380 ("Uninitialized scalar variable")
> 
> Fixes: fb45f436b818 ("media: v4l: async: Fix notifier complete callback error 
> handling")
> Signed-off-by: Colin Ian King 

Reviewed-by: Niklas Söderlund 

> ---
>  drivers/media/v4l2-core/v4l2-async.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-async.c 
> b/drivers/media/v4l2-core/v4l2-async.c
> index 49f7eccc76db..7020b2e6d158 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -550,7 +550,6 @@ int v4l2_async_register_subdev(struct v4l2_subdev *sd)
>   struct v4l2_device *v4l2_dev =
>   v4l2_async_notifier_find_v4l2_dev(notifier);
>   struct v4l2_async_subdev *asd;
> - int ret;
>  
>   if (!v4l2_dev)
>   continue;
> -- 
> 2.14.1
> 

-- 
Regards,
Niklas Söderlund

[PATCH] media: v4l: async: fix return of unitialized variable ret

[Colin King]

From: Colin Ian King 

A shadow declaration of variable ret is being assigned a return error
status and this value is being lost when the error exit goto's jump
out of the local scope. This leads to an uninitalized error return value
in the outer scope being returned. Fix this by removing the inner scoped
declaration of variable ret.

Detected by CoverityScan, CID#1460380 ("Uninitialized scalar variable")

Fixes: fb45f436b818 ("media: v4l: async: Fix notifier complete callback error 
handling")
Signed-off-by: Colin Ian King 
---
 drivers/media/v4l2-core/v4l2-async.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/media/v4l2-core/v4l2-async.c 
b/drivers/media/v4l2-core/v4l2-async.c
index 49f7eccc76db..7020b2e6d158 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -550,7 +550,6 @@ int v4l2_async_register_subdev(struct v4l2_subdev *sd)
struct v4l2_device *v4l2_dev =
v4l2_async_notifier_find_v4l2_dev(notifier);
struct v4l2_async_subdev *asd;
-   int ret;
 
if (!v4l2_dev)
continue;
-- 
2.14.1

Re: [RFC v4 04/17] WIP: [media] v4l2: add v4l2_event_queue_fh_with_cb()

[Hans Verkuil]

On 10/20/2017 11:49 PM, Gustavo Padovan wrote:
> From: Gustavo Padovan 
> 
> For some type of events we may require the event user in the
> kernel to run some operation when DQ_EVENT() is called.
> V4L2_EVENT_OUT_FENCE is the first user of this mechanism as it needs
> to call v4l2 core back to install a file descriptor.
> 
> This is WIP, I believe we are able to come up with better ways to do it,
> but as that is not the main part of the patchset I'll keep it like
> this for now.

I'm OK with this callback. I don't off-hand see a better way of doing this.

> 
> Signed-off-by: Gustavo Padovan 
> ---
>  drivers/media/v4l2-core/v4l2-event.c | 31 +++
>  include/media/v4l2-event.h   |  7 +++
>  2 files changed, 34 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-event.c 
> b/drivers/media/v4l2-core/v4l2-event.c
> index 313ee9d1f9ee..6274e3e174e0 100644
> --- a/drivers/media/v4l2-core/v4l2-event.c
> +++ b/drivers/media/v4l2-core/v4l2-event.c
> @@ -58,6 +58,9 @@ static int __v4l2_event_dequeue(struct v4l2_fh *fh, struct 
> v4l2_event *event)
>  
>   spin_unlock_irqrestore(>vdev->fh_lock, flags);
>  
> + if (kev->prepare)
> + kev->prepare(kev->data);
> +
>   return 0;
>  }
>  
> @@ -104,8 +107,11 @@ static struct v4l2_subscribed_event 
> *v4l2_event_subscribed(
>   return NULL;
>  }
>  
> -static void __v4l2_event_queue_fh(struct v4l2_fh *fh, const struct 
> v4l2_event *ev,
> - const struct timespec *ts)
> +static void __v4l2_event_queue_fh_with_cb(struct v4l2_fh *fh,

I wouldn't rename this function, just add the two arguments.

> +   const struct v4l2_event *ev,
> +   const struct timespec *ts,
> +   void (*prepare)(void *data),
> +   void *data)
>  {
>   struct v4l2_subscribed_event *sev;
>   struct v4l2_kevent *kev;
> @@ -155,6 +161,8 @@ static void __v4l2_event_queue_fh(struct v4l2_fh *fh, 
> const struct v4l2_event *e
>   kev->event.id = ev->id;
>   kev->event.timestamp = *ts;
>   kev->event.sequence = fh->sequence;
> + kev->prepare = prepare;
> + kev->data = data;
>   sev->in_use++;
>   list_add_tail(>list, >available);
>  
> @@ -177,7 +185,7 @@ void v4l2_event_queue(struct video_device *vdev, const 
> struct v4l2_event *ev)
>   spin_lock_irqsave(>fh_lock, flags);
>  
>   list_for_each_entry(fh, >fh_list, list)
> - __v4l2_event_queue_fh(fh, ev, );
> + __v4l2_event_queue_fh_with_cb(fh, ev, , NULL, NULL);
>  
>   spin_unlock_irqrestore(>fh_lock, flags);
>  }
> @@ -191,11 +199,26 @@ void v4l2_event_queue_fh(struct v4l2_fh *fh, const 
> struct v4l2_event *ev)
>   ktime_get_ts();
>  
>   spin_lock_irqsave(>vdev->fh_lock, flags);
> - __v4l2_event_queue_fh(fh, ev, );
> + __v4l2_event_queue_fh_with_cb(fh, ev, , NULL, NULL);
>   spin_unlock_irqrestore(>vdev->fh_lock, flags);
>  }
>  EXPORT_SYMBOL_GPL(v4l2_event_queue_fh);

I'd drop this function and replace it with a static inline in the v4l2-event.h
header that calls v4l2_event_queue_fh_with_cb.

>  
> +void v4l2_event_queue_fh_with_cb(struct v4l2_fh *fh,
> +  const struct v4l2_event *ev,
> +  void (*prepare)(void *data), void *data)
> +{
> + unsigned long flags;
> + struct timespec timestamp;
> +
> + ktime_get_ts();
> +
> + spin_lock_irqsave(>vdev->fh_lock, flags);
> + __v4l2_event_queue_fh_with_cb(fh, ev, , prepare, data);
> + spin_unlock_irqrestore(>vdev->fh_lock, flags);
> +}
> +EXPORT_SYMBOL_GPL(v4l2_event_queue_fh_with_cb);
> +
>  int v4l2_event_pending(struct v4l2_fh *fh)
>  {
>   return fh->navailable;
> diff --git a/include/media/v4l2-event.h b/include/media/v4l2-event.h
> index 2b794f2ad824..dc770257811e 100644
> --- a/include/media/v4l2-event.h
> +++ b/include/media/v4l2-event.h
> @@ -68,11 +68,14 @@ struct video_device;
>   * @list:List node for the v4l2_fh->available list.
>   * @sev: Pointer to parent v4l2_subscribed_event.
>   * @event:   The event itself.
> + * @prepare: Callback to prepare the event only at DQ_EVENT() time.

@data is not documented.

>   */
>  struct v4l2_kevent {
>   struct list_headlist;
>   struct v4l2_subscribed_event *sev;
>   struct v4l2_event   event;
> + void (*prepare)(void *data);
> + void *data;
>  };
>  
>  /**
> @@ -160,6 +163,10 @@ void v4l2_event_queue(struct video_device *vdev, const 
> struct v4l2_event *ev);
>   */
>  void v4l2_event_queue_fh(struct v4l2_fh *fh, const struct v4l2_event *ev);
>  
> +void v4l2_event_queue_fh_with_cb(struct v4l2_fh *fh,
> +  const struct v4l2_event *ev,
> +  void (*prepare)(void *data), void *data);
> +

Re: [RFC v4 12/17] [media] vb2: add explicit fence user API

[Hans Verkuil]

On 10/20/2017 11:50 PM, Gustavo Padovan wrote:
> From: Gustavo Padovan 
> 
> Turn the reserved2 field into fence_fd that we will use to send
> an in-fence to the kernel and return an out-fence from the kernel to
> userspace.
> 
> Two new flags were added, V4L2_BUF_FLAG_IN_FENCE, that should be used
> when sending a fence to the kernel to be waited on, and
> V4L2_BUF_FLAG_OUT_FENCE, to ask the kernel to give back an out-fence.
> 
> v3:
>   - make the out_fence refer to the current buffer (Hans)
> 
> v2: add documentation
> 
> Signed-off-by: Gustavo Padovan 
> ---
>  Documentation/media/uapi/v4l/buffer.rst   | 15 +++
>  drivers/media/usb/cpia2/cpia2_v4l.c   |  2 +-
>  drivers/media/v4l2-core/v4l2-compat-ioctl32.c |  4 ++--
>  drivers/media/v4l2-core/videobuf2-v4l2.c  |  2 +-
>  include/uapi/linux/videodev2.h|  4 +++-
>  5 files changed, 22 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/media/uapi/v4l/buffer.rst 
> b/Documentation/media/uapi/v4l/buffer.rst
> index ae6ee73f151c..99afc0837671 100644
> --- a/Documentation/media/uapi/v4l/buffer.rst
> +++ b/Documentation/media/uapi/v4l/buffer.rst
> @@ -648,6 +648,21 @@ Buffer Flags
>- Start Of Exposure. The buffer timestamp has been taken when the
>   exposure of the frame has begun. This is only valid for the
>   ``V4L2_BUF_TYPE_VIDEO_CAPTURE`` buffer type.
> +* .. _`V4L2-BUF-FLAG-IN-FENCE`:
> +
> +  - ``V4L2_BUF_FLAG_IN_FENCE``
> +  - 0x0020
> +  - Ask V4L2 to wait on fence passed in ``fence_fd`` field. The buffer
> + won't be queued to the driver until the fence signals.
> +
> +* .. _`V4L2-BUF-FLAG-OUT-FENCE`:
> +
> +  - ``V4L2_BUF_FLAG_OUT_FENCE``
> +  - 0x0040
> +  - Request a fence to be attached to the buffer. One should listen for
> + the ``V4L2_EVENT_OUT_FENCE`` event to receive the buffer ``index``
> + and the ``out_fence_fd`` attached to the buffer.  The event is
> + triggered at the moment the buffer is queued to the V4L2 driver.
>  
>  
>  
> diff --git a/drivers/media/usb/cpia2/cpia2_v4l.c 
> b/drivers/media/usb/cpia2/cpia2_v4l.c
> index 4666a642b2f6..d9d6b6a31751 100644
> --- a/drivers/media/usb/cpia2/cpia2_v4l.c
> +++ b/drivers/media/usb/cpia2/cpia2_v4l.c
> @@ -948,7 +948,7 @@ static int cpia2_dqbuf(struct file *file, void *fh, 
> struct v4l2_buffer *buf)
>   buf->sequence = cam->buffers[buf->index].seq;
>   buf->m.offset = cam->buffers[buf->index].data - cam->frame_buffer;
>   buf->length = cam->frame_size;
> - buf->reserved2 = 0;
> + buf->fence_fd = -1;
>   buf->reserved = 0;
>   memset(>timecode, 0, sizeof(buf->timecode));
>  
> diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c 
> b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> index 821f2aa299ae..3a31d318df2a 100644
> --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> @@ -370,7 +370,7 @@ struct v4l2_buffer32 {
>   __s32   fd;
>   } m;
>   __u32   length;
> - __u32   reserved2;
> + __s32   fence_fd;
>   __u32   reserved;
>  };
>  
> @@ -533,7 +533,7 @@ static int put_v4l2_buffer32(struct v4l2_buffer *kp, 
> struct v4l2_buffer32 __user
>   put_user(kp->timestamp.tv_usec, >timestamp.tv_usec) ||
>   copy_to_user(>timecode, >timecode, sizeof(struct 
> v4l2_timecode)) ||
>   put_user(kp->sequence, >sequence) ||
> - put_user(kp->reserved2, >reserved2) ||
> + put_user(kp->fence_fd, >fence_fd) ||
>   put_user(kp->reserved, >reserved) ||
>   put_user(kp->length, >length))
>   return -EFAULT;
> diff --git a/drivers/media/v4l2-core/videobuf2-v4l2.c 
> b/drivers/media/v4l2-core/videobuf2-v4l2.c
> index 0c0669976bdc..110fb45fef6f 100644
> --- a/drivers/media/v4l2-core/videobuf2-v4l2.c
> +++ b/drivers/media/v4l2-core/videobuf2-v4l2.c
> @@ -203,7 +203,7 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb, 
> void *pb)
>   b->timestamp = ns_to_timeval(vb->timestamp);
>   b->timecode = vbuf->timecode;
>   b->sequence = vbuf->sequence;
> - b->reserved2 = 0;
> + b->fence_fd = -1;
>   b->reserved = 0;
>  
>   if (q->is_multiplanar) {
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 2a432e8c18e3..8975fec4ab68 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -924,7 +924,7 @@ struct v4l2_buffer {
>   __s32   fd;
>   } m;
>   __u32   length;
> - __u32   reserved2;
> + __s32   fence_fd;

I think this should become:

union {
__s32 fence_fd;
__u32 reserved2;
};

Currently applications set 

Re: [PATCH 0/4] dma-buf: Silence dma_fence __rcu sparse warnings

[Christian König]

Patch #4 is Reviewed-by: Christian König .

The rest is Acked-by: Christian König .

Regards,
Christian.

Am 02.11.2017 um 21:03 schrieb Ville Syrjala:

From: Ville Syrjälä 

When building drm+i915 I get around 150 lines of sparse noise from
dma_fence __rcu warnings. This series eliminates all of that.

The first two patches were already posted by Chris, but there wasn't
any real reaction, so I figured I'd repost with a wider Cc list.

As for the other two patches, I'm no expert on dma_fence and I didn't
spend a lot of time looking at it so I can't be sure I annotated all
the accesses correctly. But I figured someone will scream at me if
I got it wrong ;)

Cc: Dave Airlie 
Cc: Jason Ekstrand 
Cc: linaro-mm-...@lists.linaro.org
Cc: linux-media@vger.kernel.org
Cc: Alex Deucher 
Cc: Christian König 
Cc: Sumit Semwal 
Cc: Chris Wilson 

Chris Wilson (2):
   drm/syncobj: Mark up the fence as an RCU protected pointer
   dma-buf/fence: Sparse wants __rcu on the object itself

Ville Syrjälä (2):
   drm/syncobj: Use proper methods for accessing rcu protected pointers
   dma-buf: Use rcu_assign_pointer() to set rcu protected pointers

  drivers/dma-buf/reservation.c |  2 +-
  drivers/gpu/drm/drm_syncobj.c | 11 +++
  include/drm/drm_syncobj.h |  2 +-
  include/linux/dma-fence.h |  2 +-
  4 files changed, 10 insertions(+), 7 deletions(-)

Re: [RFC v4 17/17] [media] v4l: Document explicit synchronization behaviour

[Hans Verkuil]

On 10/20/2017 11:50 PM, Gustavo Padovan wrote:
> From: Gustavo Padovan 
> 
> Add section to VIDIOC_QBUF about it
> 
> v3:
>   - make the out_fence refer to the current buffer (Hans)
>   - Note what happens when the IN_FENCE is not set (Hans)
> 
> v2:
>   - mention that fences are files (Hans)
>   - rework for the new API
> 
> Signed-off-by: Gustavo Padovan 
> ---
>  Documentation/media/uapi/v4l/vidioc-qbuf.rst | 31 
> 
>  1 file changed, 31 insertions(+)
> 
> diff --git a/Documentation/media/uapi/v4l/vidioc-qbuf.rst 
> b/Documentation/media/uapi/v4l/vidioc-qbuf.rst
> index 9e448a4aa3aa..a65a50578bad 100644
> --- a/Documentation/media/uapi/v4l/vidioc-qbuf.rst
> +++ b/Documentation/media/uapi/v4l/vidioc-qbuf.rst
> @@ -118,6 +118,37 @@ immediately with an ``EAGAIN`` error code when no buffer 
> is available.
>  The struct :c:type:`v4l2_buffer` structure is specified in
>  :ref:`buffer`.
>  
> +Explicit Synchronization
> +
> +
> +Explicit Synchronization allows us to control the synchronization of
> +shared buffers from userspace by passing fences to the kernel and/or
> +receiving them from it. Fences passed to the kernel are named in-fences and
> +the kernel should wait on them to signal before using the buffer, i.e., 
> queueing
> +it to the driver. On the other side, the kernel can create out-fences for the
> +buffers it queues to the drivers. Out-fences signal when the driver is
> +finished with buffer, i.e., the buffer is ready. The fences are represented
> +as a file and passed as a file descriptor to userspace.
> +
> +The in-fences are communicated to the kernel at the ``VIDIOC_QBUF`` ioctl
> +using the ``V4L2_BUF_FLAG_IN_FENCE`` buffer
> +flags and the `fence_fd` field. If an in-fence needs to be passed to the 
> kernel,
> +`fence_fd` should be set to the fence file descriptor number and the
> +``V4L2_BUF_FLAG_IN_FENCE`` should be set as well Setting one but not the 
> other

Missing '.' after 'as well'.

To what value is fence_fd set when VIDIOC_QBUF returns? If you don't set the
IN_FENCE flag, what should userspace set fence_fd to? (I recommend 0).

> +will cause ``VIDIOC_QBUF`` to return with error.
> +
> +The fence_fd field (formely the reserved2 field) will be ignored if the

Drop the "(formely the reserved2 field)" part. We're not interested in the
history here.

> +``V4L2_BUF_FLAG_IN_FENCE`` is not set.
> +
> +To get an out-fence back from V4L2 the ``V4L2_BUF_FLAG_OUT_FENCE`` flag 
> should
> +be set to ask for a fence to be attached to the buffer. To become aware of
> +the out-fence created one should listen for the ``V4L2_EVENT_OUT_FENCE`` 
> event.
> +An event will be triggered for every buffer queued to the V4L2 driver with 
> the
> +``V4L2_BUF_FLAG_OUT_FENCE``.
> +
> +At streamoff the out-fences will either signal normally if the drivers waits

drivers -> driver

> +for the operations on the buffers to finish or signal with error if the

error -> an error

> +driver cancels the pending operations.
>  
>  Return Value
>  
> 

What should be done if the driver doesn't set ordered_in_driver? How does 
userspace
know whether in and/or out fences are supported? I'm leaning towards a new 
capability
flag for QUERYCAPS.

What does VIDIOC_QUERYBUF return w.r.t. the fence flags and fence_fd?

Regards,

Hans

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

[Marek Szyprowski]

MFC driver supports only MMAP operation mode mainly due to the hardware
restrictions of the addresses of the DMA buffers (MFC v5 hardware can
access buffers only in 128MiB memory region starting from the base address
of its firmware). When IOMMU is available, this requirement is easily
fulfilled even for the buffers located anywhere in the memory - typically
by mapping them in the DMA address space as close as possible to the
firmware. Later hardware revisions don't have this limitations at all.

The second limitation of the MFC hardware related to the memory buffers
is constant buffer address. Once the hardware has been initialized for
operation on given buffer set, the addresses of the buffers cannot be
changed.

With the above assumptions, a limited support for USERPTR and DMABUF
operation modes can be added. The main requirement is to have all buffers
known when starting hardware. This has been achieved by postponing
hardware initialization once all the DMABUF or USERPTR buffers have been
queued for the first time. Once then, buffers cannot be modified to point
to other memory area.

This patch also removes unconditional USERPTR operation mode from encoder
video node, because it doesn't work with v5 MFC hardware without IOMMU
being enabled.

In case of MFC v5 a bidirectional queue flag has to be enabled as a
workaround of the strange hardware behavior - MFC performs a few writes
to source data during the operation.

Signed-off-by: Seung-Woo Kim 
[mszyprow: adapted to v4.14 code base, rewrote and extended commit message,
 added checks for changing buffer addresses, added bidirectional queue
 flags and comments]
Signed-off-by: Marek Szyprowski 
---
v2:
- fixed copy/paste bug, which broke encoding support (thanks to Marian
  Mihailescu for reporting it)
- added checks for changing buffers DMA addresses
- added bidirectional queue flags

v1:
- inital version
---
 drivers/media/platform/s5p-mfc/s5p_mfc.c |  23 +-
 drivers/media/platform/s5p-mfc/s5p_mfc_dec.c | 111 +++
 drivers/media/platform/s5p-mfc/s5p_mfc_enc.c |  64 +++
 3 files changed, 147 insertions(+), 51 deletions(-)

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c 
b/drivers/media/platform/s5p-mfc/s5p_mfc.c
index 1839a86cc2a5..f1ab8d198158 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
@@ -754,6 +754,7 @@ static int s5p_mfc_open(struct file *file)
struct s5p_mfc_dev *dev = video_drvdata(file);
struct s5p_mfc_ctx *ctx = NULL;
struct vb2_queue *q;
+   unsigned int io_modes;
int ret = 0;
 
mfc_debug_enter();
@@ -839,16 +840,25 @@ static int s5p_mfc_open(struct file *file)
if (ret)
goto err_init_hw;
}
+
+   io_modes = VB2_MMAP;
+   if (exynos_is_iommu_available(>plat_dev->dev) || !IS_TWOPORT(dev))
+   io_modes |= VB2_USERPTR | VB2_DMABUF;
+
/* Init videobuf2 queue for CAPTURE */
q = >vq_dst;
q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
+   q->io_modes = io_modes;
+   /*
+* Destination buffers are always bidirectional, they use used as
+* reference data, which require READ access
+*/
+   q->bidirectional = true;
q->drv_priv = >fh;
q->lock = >mfc_mutex;
if (vdev == dev->vfd_dec) {
-   q->io_modes = VB2_MMAP;
q->ops = get_dec_queue_ops();
} else if (vdev == dev->vfd_enc) {
-   q->io_modes = VB2_MMAP | VB2_USERPTR;
q->ops = get_enc_queue_ops();
} else {
ret = -ENOENT;
@@ -869,13 +879,18 @@ static int s5p_mfc_open(struct file *file)
/* Init videobuf2 queue for OUTPUT */
q = >vq_src;
q->type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
+   q->io_modes = io_modes;
+   /*
+* MFV v5 performs write operations on source data, so make queue
+* bidirectional to avoid IOMMU protection fault.
+*/
+   if (!IS_MFCV6_PLUS(dev))
+   q->bidirectional = true;
q->drv_priv = >fh;
q->lock = >mfc_mutex;
if (vdev == dev->vfd_dec) {
-   q->io_modes = VB2_MMAP;
q->ops = get_dec_queue_ops();
} else if (vdev == dev->vfd_enc) {
-   q->io_modes = VB2_MMAP | VB2_USERPTR;
q->ops = get_enc_queue_ops();
} else {
ret = -ENOENT;
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c 
b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
index e3e5c442902a..26ee8315e2cf 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
@@ -551,14 +551,27 @@ static int reqbufs_capture(struct s5p_mfc_dev *dev, 
struct s5p_mfc_ctx *ctx,
goto out;
}
 
-   WARN_ON(ctx->dst_bufs_cnt !=