cron job: media_tree daily build: OK

[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:   Tue Mar 22 04:00:32 CET 2016
git branch: test
git hash:   b39950960d2b890c21465c69c7c0e4ff6253c6b5
gcc version:i686-linux-gcc (GCC) 5.3.0
sparse version: v0.5.0-56-g7647c77
smatch version: v0.5.0-3353-gcae47da
host hardware:  x86_64
host os:4.4.0-164

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-exynos: OK
linux-git-arm-mx: OK
linux-git-arm-omap: OK
linux-git-arm-omap1: OK
linux-git-arm-pxa: OK
linux-git-blackfin-bf561: OK
linux-git-i686: OK
linux-git-m32r: OK
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
linux-2.6.36.4-i686: OK
linux-2.6.37.6-i686: OK
linux-2.6.38.8-i686: OK
linux-2.6.39.4-i686: OK
linux-3.0.60-i686: OK
linux-3.1.10-i686: OK
linux-3.2.37-i686: OK
linux-3.3.8-i686: OK
linux-3.4.27-i686: OK
linux-3.5.7-i686: OK
linux-3.6.11-i686: OK
linux-3.7.4-i686: OK
linux-3.8-i686: OK
linux-3.9.2-i686: OK
linux-3.10.1-i686: OK
linux-3.11.1-i686: OK
linux-3.12.23-i686: OK
linux-3.13.11-i686: OK
linux-3.14.9-i686: OK
linux-3.15.2-i686: OK
linux-3.16.7-i686: OK
linux-3.17.8-i686: OK
linux-3.18.7-i686: OK
linux-3.19-i686: OK
linux-4.0-i686: OK
linux-4.1.1-i686: OK
linux-4.2-i686: OK
linux-4.3-i686: OK
linux-4.4-i686: OK
linux-4.5-i686: OK
linux-2.6.36.4-x86_64: OK
linux-2.6.37.6-x86_64: OK
linux-2.6.38.8-x86_64: OK
linux-2.6.39.4-x86_64: OK
linux-3.0.60-x86_64: OK
linux-3.1.10-x86_64: OK
linux-3.2.37-x86_64: OK
linux-3.3.8-x86_64: OK
linux-3.4.27-x86_64: OK
linux-3.5.7-x86_64: OK
linux-3.6.11-x86_64: OK
linux-3.7.4-x86_64: OK
linux-3.8-x86_64: OK
linux-3.9.2-x86_64: OK
linux-3.10.1-x86_64: OK
linux-3.11.1-x86_64: OK
linux-3.12.23-x86_64: OK
linux-3.13.11-x86_64: OK
linux-3.14.9-x86_64: OK
linux-3.15.2-x86_64: OK
linux-3.16.7-x86_64: OK
linux-3.17.8-x86_64: OK
linux-3.18.7-x86_64: OK
linux-3.19-x86_64: OK
linux-4.0-x86_64: OK
linux-4.1.1-x86_64: OK
linux-4.2-x86_64: OK
linux-4.3-x86_64: OK
linux-4.4-x86_64: OK
linux-4.5-x86_64: OK
apps: OK
spec-git: OK
sparse: WARNINGS
smatch: ERRORS

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

http://www.xs4all.nl/~hverkuil/spec/media.html
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] media: au0828 fix au0828_v4l2_close() dev_state race condition

[Shuah Khan]

au0828_v4l2_close() check for dev_state == DEV_DISCONNECTED will fail to
detect the device disconnected state correctly, if au0828_v4l2_open() runs
to set the DEV_INITIALIZED bit. A loop test of bind/unbind found this bug
by increasing the likelihood of au0828_v4l2_open() occurring while unbind
is in progress. When au0828_v4l2_close() fails to detect that the device
is in disconnect state, it attempts to power down the device and fails with
the following general protection fault:

[  260.992962] Call Trace:
[  260.993008]  [] ? xc5000_sleep+0x8f/0xd0 [xc5000]
[  260.993095]  [] ? fe_standby+0x3c/0x50 [tuner]
[  260.993186]  [] au0828_v4l2_close+0x53c/0x620 [au0828]
[  260.993298]  [] v4l2_release+0xf0/0x210 [videodev]
[  260.993382]  [] __fput+0x1fc/0x6c0
[  260.993449]  [] fput+0xe/0x10
[  260.993519]  [] task_work_run+0x133/0x1f0
[  260.993602]  [] exit_to_usermode_loop+0x140/0x170
[  260.993681]  [] syscall_return_slowpath+0x16a/0x1a0
[  260.993754]  [] entry_SYSCALL_64_fastpath+0xa6/0xa8

Signed-off-by: Shuah Khan 
---
 drivers/media/usb/au0828/au0828-video.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/usb/au0828/au0828-video.c 
b/drivers/media/usb/au0828/au0828-video.c
index 13f6dab..88dcc6e 100644
--- a/drivers/media/usb/au0828/au0828-video.c
+++ b/drivers/media/usb/au0828/au0828-video.c
@@ -1075,7 +1075,7 @@ static int au0828_v4l2_close(struct file *filp)
del_timer_sync(>vbi_timeout);
}
 
-   if (dev->dev_state == DEV_DISCONNECTED)
+   if (dev->dev_state & DEV_DISCONNECTED)
goto end;
 
if (dev->users == 1) {
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] sound/usb: fix to release stream resources from media_snd_device_delete()

[Shuah Khan]

On 03/19/2016 07:31 AM, Shuah Khan wrote:
> On 03/19/2016 06:10 AM, Mauro Carvalho Chehab wrote:
>> Em Fri, 18 Mar 2016 20:50:31 -0600
>> Shuah Khan  escreveu:
>>
>>> Fix to release stream resources from media_snd_device_delete() before
>>> media device is unregistered. Without this change, stream resource free
>>> is attempted after the media device is unregistered which would result
>>> in use-after-free errors.
>>>
>>> Signed-off-by: Shuah Khan 
>>> ---
>>>
>>> - Ran bind/unbind loop (1000 iteration) test on snd-usb-audio
>>>   while running mc_nextgen_test loop (1000 iterations) in parallel.
>>> - Ran bind/unbind and rmmod/modprobe tests on both drivers. Also
>>>   generated graphs when after bind/unbind, rmmod/modprobe. Graphs
>>>   look good.
>>> - Note: Please apply the following patch to fix memory leak:
>>>   sound/usb: Fix memory leak in media_snd_stream_delete() during unbind
>>>   https://lkml.org/lkml/2016/3/16/1050
>>
>> Yeah, a way better!
>>
>> For normal bind/unbind, it seems to be working fine. Also
>> for driver's rmmod, so:
>>
>> Tested-by: Mauro Carvalho Chehab 
>>
>> PS.:
>> ===
>>
>> There are still some troubles if we run an infinite loop
>> binding/unbinding au0828 and another one binding/unbinding
>> snd-usb-audio, like this one:
> 
> Yes. I noticed this one when I was running a loop of 1000 on au0828.
> I couldn't reproduce this when I tested this patch.
> 
> P.S: au08282 loops takes longer btw since au0828 initialization is lot more
> complex. We have to look at this one.
> 
>>
>>
>> [   91.556188] [ cut here ]
>> [   91.556500] WARNING: CPU: 1 PID: 2920 at drivers/media/media-entity.c:392 
>> __media_entity_pipeline_start+0x271/0xce0 [media]()
>> [   91.556626] Modules linked in: ir_lirc_codec lirc_dev au8522_dig xc5000 
>> tuner au8522_decoder au8522_common au0828 videobuf2_vmalloc videobuf2_memops 
>> videobuf2_v4l2 videobuf2_core tveeprom dvb_core rc_core v4l2_common videodev 
>> snd_usb_audio snd_usbmidi_lib snd_rawmidi snd_seq_device media 
>> cpufreq_powersave cpufreq_conservative cpufreq_userspace cpufreq_stats 
>> parport_pc ppdev lp parport snd_hda_codec_hdmi intel_rapl 
>> x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel iTCO_wdt kvm 
>> iTCO_vendor_support irqbypass crct10dif_pclmul crc32_pclmul crc32c_intel 
>> ghash_clmulni_intel sha256_ssse3 sha256_generic hmac drbg 
>> snd_hda_codec_realtek i915 snd_hda_codec_generic aesni_intel aes_x86_64 
>> btusb lrw btrtl gf128mul snd_hda_intel glue_helper ablk_helper btbcm btintel 
>> cryptd psmouse snd_hda_codec bluetooth
>> [   91.556693]  snd_hwdep i2c_algo_bit sg snd_hda_core serio_raw pcspkr 
>> evdev drm_kms_helper snd_pcm rfkill drm snd_timer mei_me snd i2c_i801 
>> soundcore lpc_ich mei mfd_core battery video dw_dmac i2c_designware_platform 
>> dw_dmac_core i2c_designware_core acpi_pad button tpm_tis tpm ext4 crc16 
>> mbcache jbd2 dm_mod sd_mod hid_generic usbhid ahci libahci libata e1000e 
>> xhci_pci ptp scsi_mod ehci_pci ehci_hcd pps_core xhci_hcd fan thermal 
>> sdhci_acpi sdhci mmc_core i2c_hid hid
>> [   91.556748] CPU: 1 PID: 2920 Comm: v4l_id Tainted: G  D W   
>> 4.5.0+ #62
>> [   91.556751] Hardware name:  /NUC5i7RYB, BIOS 
>> RYBDWi35.86A.0350.2015.0812.1722 08/12/2015
>> [   91.556754]  a0e247a0 8803a3d17b08 819447c1 
>> 
>> [   91.556759]  88009bbe17c0 8803a3d17b48 8114fd16 
>> a0e20651
>> [   91.556763]  8803a47c9c58 8803a477d2c8 8803a5ac96f8 
>> dc00
>> [   91.556768] Call Trace:
>> [   91.556774]  [] dump_stack+0x85/0xc4
>> [   91.556778]  [] warn_slowpath_common+0xc6/0x120
>> [   91.556783]  [] ? 
>> __media_entity_pipeline_start+0x271/0xce0 [media]
>> [   91.556786]  [] warn_slowpath_null+0x1a/0x20
>> [   91.556790]  [] 
>> __media_entity_pipeline_start+0x271/0xce0 [media]
>> [   91.556794]  [] ? __schedule+0x78a/0x2570
>> [   91.556797]  [] ? memset+0x28/0x30
>> [   91.556802]  [] ? media_entity_pipeline_stop+0x60/0x60 
>> [media]
>> [   91.556806]  [] ? trace_hardirqs_on+0xd/0x10
>> [   91.556810]  [] ? au0828_enable_source+0x55/0x9f0 
>> [au0828]
>> [   91.556813]  [] ? mutex_trylock+0x400/0x400
>> [   91.556818]  [] ? au0828_v4l2_close+0xb3/0x590 [au0828]
>> [   91.556822]  [] au0828_enable_source+0x3f4/0x9f0 
>> [au0828]
>> [   91.556824]  [] ? mutex_trylock+0x400/0x400
>> [   91.556834]  [] v4l_enable_media_source+0x66/0x90 
>> [videodev]
>> [   91.556839]  [] au0828_v4l2_close+0x25a/0x590 [au0828]
>> [   91.556846]  [] v4l2_release+0xf0/0x210 [videodev]
>> [   91.556849]  [] __fput+0x20f/0x6d0
>> [   91.556853]  [] fput+0xe/0x10
>> [   91.556856]  [] task_work_run+0x137/0x200
>> [   91.556859]  [] exit_to_usermode_loop+0x154/0x180
>> [   91.556863]  [] ? trace_hardirqs_on_caller+0x16/0x590
>> [   91.556866]  [] syscall_return_slowpath+0x186/0x1c0
>> [   91.556869]  [] 

Re: [RFC PATCH 1/3] [media] v4l2-mc.h: Add a S-Video C input PAD to demod enum

[Javier Martinez Canillas]

Hello Mauro,

On 03/21/2016 06:15 PM, Mauro Carvalho Chehab wrote:
> Em Mon, 21 Mar 2016 16:48:12 -0300
> Javier Martinez Canillas  escreveu:
> 
>> Hello Hans,
>>
>> On 03/21/2016 04:30 PM, Hans Verkuil wrote:
>>
>> [snip]
>>
>>
>> Can you please provide an example of a media pipeline that user-space 
>> should
>> use with this approach? AFAICT whatever PADs are created when 
>> initiliazing
>> the PADs for an entity, will be exposed to user-space in the media graph.
>>
>> So I'm not understading how it will be used in practice. I don't mean 
>> that
>> your approach is not correct, is just I'm not getting it :)  
>
> Why would userspace need to use the pads? This is for legacy drivers 
> (right?)
> where the pipeline is fixed anyway.
>  

 I asked because the user needs to setup the links in the media pipeline to
 choose  which input connection will be linked to the tvp5150 decoder. But 
 it
 doesn't matter if we are going with the single sink pad approach since the
 user will always do something like:  
>>>
>>> Why? The user will use an application that uses ENUM/S/G_INPUT for this. 
>>> We're
>>> talking legacy drivers ('interface centric drivers' would be a better 
>>> description)
>>> where we don't even expose the v4l-subdevX device nodes. Explicitly 
>>> programming
>>> a media pipeline is something you do for complex devices (embedded systems 
>>> and
>>> the like). Not for simple and generally fixed pipelines. Utterly pointless.
>>>  
>>
>> Mauro was talking about legacy 'interface centric' PC-consumer's hardware but
>> my test system is an embedded board that also has a tvp5150 decoder. The
>> board has an OMAP3 and the tvp5150 is attached to the SoC ISP. Is this one:
>>
>> https://www.isee.biz/products/igep-expansion-boards/igepv2-expansion
> 
> Yeah, subdevs should be prepared to work with both "interface centric" and
> "media controller centric" approaches.
> 
> Yet, I don't think using one sink pad for tvp5150 is a bad thing.
> 
>> As you can see, the board has 2 RCA connectors and each one is routed a 
>> tvp5150
>> composite input and both connectors can be used for S-Video. So the user 
>> needs
>> to setup the pipeline manually to choose which input connection to use.
> 
> Actually, on your tests, you were not able to make this work, nor
> S-Video is officially supported by the manufacturer. So, in the
> specific case of IGEPv2, I would not be adding a S-Video connector.
>

Yes, we can leave that for now. As you said I was not able to make it
work and when contacted an engineer working for the manufacturer, he
told me that in theory it should work but they have never tested it.

> Btw, even on devices with an S-Video connector and tvp5150, at
> least on my tests, the driver were not able to setup S-Video. It
> seems that there's something more than just setting the mux.
>

That could explain why it was not working for me, but in any case I
can focus on the two composite inputs for now that I can test on my
board easily. The S-video input support can be added as follow up.

> - 
> Thanks,
> Mauro
> 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 1/3] [media] v4l2-mc.h: Add a S-Video C input PAD to demod enum

[Javier Martinez Canillas]

Hello Mauro,

On 03/21/2016 06:20 PM, Mauro Carvalho Chehab wrote:
> Em Mon, 21 Mar 2016 16:20:09 -0300
> Javier Martinez Canillas  escreveu:
> 
>>> BTW, if the tvp5150 needs to know which composite connector is connected
>>> to which hardware pin, then you still may need to be explicit w.r.t. the
>>> number of pads. I just thought of that.
>>>  
>>
>> The tvp5150 doesn't care about that, as Mauro said is just a mux so you can
>> have logic in the .link_setup that does the mux depending on the remote
>> entity (that's in fact how I implemented and is currently in mainline).
>>
>> Now, the user needs to know which entity is mapped to which input pin.
>> All its know from the HW documentation is that for example the left
>> RCA connector is AIP1A and the one inf the right is connected to AIP1B.
>>
>> So there could be a convention that the composite connected to AIP1A pin
>> (the default) is Composite0 and the connected to AIP1B is Composite1.
> 
> 
> We should avoid a convention here... instead, we should support
> properties via the properties API to export enough info for userspace
> to know what physical connectors correspond to each "connection" entity.
> 
> As we've discussed previously, such properties can be:
>   - connector's name
>   - color
>   - position
> etc...
>

Right, I forgot about the properties API.
 
> 
> Regards,
> Mauro
> 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] [media] media: am437x-vpfe: ensure ret is initialized

[Colin King]

From: Colin Ian King 

ret should be initialized to 0; for example if pfe->fmt.fmt.pix.field
is V4L2_FIELD_NONE then ret will contain garbage from the
uninitialized state causing garbage to be returned if it is non-zero.

Signed-off-by: Colin Ian King 
---
 drivers/media/platform/am437x/am437x-vpfe.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/am437x/am437x-vpfe.c 
b/drivers/media/platform/am437x/am437x-vpfe.c
index de32e3a..7d14732 100644
--- a/drivers/media/platform/am437x/am437x-vpfe.c
+++ b/drivers/media/platform/am437x/am437x-vpfe.c
@@ -1047,7 +1047,7 @@ static int vpfe_get_ccdc_image_format(struct vpfe_device 
*vpfe,
 static int vpfe_config_ccdc_image_format(struct vpfe_device *vpfe)
 {
enum ccdc_frmfmt frm_fmt = CCDC_FRMFMT_INTERLACED;
-   int ret;
+   int ret = 0;
 
vpfe_dbg(2, vpfe, "vpfe_config_ccdc_image_format\n");
 
-- 
2.7.3

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC 0/2] pxa_camera transition to v4l2 standalone device

[Hans Verkuil]

On 03/21/2016 11:42 PM, Robert Jarzmik wrote:
> Hans Verkuil  writes:
> 
>>> Input ioctls:
>>> test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
>>> test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
>>> test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
>>> test VIDIOC_ENUMAUDIO: OK (Not Supported)
>>> fail: v4l2-test-input-output.cpp(418): G_INPUT not supported 
>>> for a capture device
>>
>> ENUM/G/S_INPUT is missing and is required for capture devices.
> Okay, that one will be easy I think :) It's a mono-sensor mono-videostream IP.
> I will add that when for RFC v2.
> 
>>> Format ioctls:
>>> test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
>>> test VIDIOC_G/S_PARM: OK (Not Supported)
>>> test VIDIOC_G_FBUF: OK (Not Supported)
>>> fail: v4l2-test-formats.cpp(329): pixelformat != 
>>> V4L2_PIX_FMT_JPEG && colorspace == V4L2_COLORSPACE_JPEG
>>> fail: v4l2-test-formats.cpp(432): 
>>> testColorspace(pix.pixelformat, pix.colorspace, pix.ycbcr_enc, 
>>> pix.quantization)
>>
>> The sensor should almost certainly use COLORSPACE_SRGB. Certainly not
>> COLORSPACE_JPEG.
> Ah even for YUYV format, I didn't know ...

The pixel format != colorspace. You can read more about that here, if you are
interested:

https://hverkuil.home.xs4all.nl/spec/media.html#colorspaces

> 
>>> Buffer ioctls:
>>> test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
>>> fail: v4l2-test-buffers.cpp(571): q.has_expbuf(node)
>>
>> You are missing .vidioc_expbuf = vbs_ioctl_expbuf and the vb2 io_mode
>> VB2_DMABUF.
> Nope, I have :
>   .vidioc_expbuf  = vb2_ioctl_expbuf,
>   vq->io_modes = VB2_MMAP | VB2_DMABUF;
> 
> So it's something more subtle, and I have a bit of work to understand what.

Fix the colorspace issue first, I wonder if it is just fallout from that earlier
issue.

Regards,

Hans

> 
> Cheers.
> 
> --
> Robert
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC 0/2] pxa_camera transition to v4l2 standalone device

[Robert Jarzmik]

Hans Verkuil  writes:

>> Input ioctls:
>>  test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
>>  test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
>>  test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
>>  test VIDIOC_ENUMAUDIO: OK (Not Supported)
>>  fail: v4l2-test-input-output.cpp(418): G_INPUT not supported 
>> for a capture device
>
> ENUM/G/S_INPUT is missing and is required for capture devices.
Okay, that one will be easy I think :) It's a mono-sensor mono-videostream IP.
I will add that when for RFC v2.

>>  Format ioctls:
>>  test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
>>  test VIDIOC_G/S_PARM: OK (Not Supported)
>>  test VIDIOC_G_FBUF: OK (Not Supported)
>>  fail: v4l2-test-formats.cpp(329): pixelformat != 
>> V4L2_PIX_FMT_JPEG && colorspace == V4L2_COLORSPACE_JPEG
>>  fail: v4l2-test-formats.cpp(432): 
>> testColorspace(pix.pixelformat, pix.colorspace, pix.ycbcr_enc, 
>> pix.quantization)
>
> The sensor should almost certainly use COLORSPACE_SRGB. Certainly not
> COLORSPACE_JPEG.
Ah even for YUYV format, I didn't know ...

>>  Buffer ioctls:
>>  test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
>>  fail: v4l2-test-buffers.cpp(571): q.has_expbuf(node)
>
> You are missing .vidioc_expbuf = vbs_ioctl_expbuf and the vb2 io_mode
> VB2_DMABUF.
Nope, I have :
.vidioc_expbuf  = vb2_ioctl_expbuf,
vq->io_modes = VB2_MMAP | VB2_DMABUF;

So it's something more subtle, and I have a bit of work to understand what.

Cheers.

--
Robert
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC 0/2] pxa_camera transition to v4l2 standalone device

[Hans Verkuil]

On 03/21/2016 11:26 PM, Robert Jarzmik wrote:
> Hans Verkuil  writes:
> 
>> On 03/19/2016 10:01 PM, Robert Jarzmik wrote:
>>> Hi Hans and Guennadi,
>>>
>>> As Hans is converting sh_mobile_ceu_camera.c,
>>
>> That's not going as fast as I hoped. This driver is quite complex and 
>> extracting
>> it from soc-camera isn't easy. I also can't spend as much time as I'd like 
>> on this.
>>
>>> let's see how close our ports are
>>> to see if there are things we could either reuse of change.
>>>
>>> The port is assuming :
>>>  - the formation translation is transferred into soc_mediabus, so that it 
>>> can be
>>>reused across all v4l2 devices
>>
>> At best this will be a temporary helper source. I never liked soc_mediabus, 
>> I don't
>> believe it is the right approach.
> As long as you provide a better approach, especially for the dynamic formats
> translation, it should be fine.
> 
>> But I have no problem if it is used for now to simplify the soc-camera
>> dependency removal.
> Ok.
> 
>>>  - pxa_camera is ported
>>>
>>> This sets a ground of discussion for soc_camera adherence removal from
>>> pxa_camera. I'd like to have a comment from Hans if this is what he has in 
>>> mind,
>>> and Guennadi if he agrees to transfer the soc xlate stuff to soc_mediabus.
>>
>> Can you provide the output of 'v4l2-compliance -s' with your new pxa driver?
>> I would be curious to see the result of that.
> Of course, with [1] added (initial format init and querycap strings), I have
> the following results. I have no idea where VIDIOC_EXPBUF failure comes from,
> while the colorspace issues are a consequence from the MT9M111 sensor which
> provides the couple ("VYUY", V4L2_COLORSPACE_JPEG) format (which I also don't
> understand why it is a failure) :
> 
> Driver Info:
>   Driver name   : pxa27x-camera
>   Card type : PXA_Camera
>   Bus info  : platform:pxa-camera
>   Driver version: 4.5.0
>   Capabilities  : 0x8421
>   Video Capture
>   Streaming
>   Extended Pix Format
>   Device Capabilities
>   Device Caps   : 0x0421
>   Video Capture
>   Streaming
>   Extended Pix Format
> 
> Compliance test for device /dev/video0 (not using libv4l2):
> 
> Required ioctls:
>   test VIDIOC_QUERYCAP: OK
> 
> Allow for multiple opens:
>   test second video open: OK
>   test VIDIOC_QUERYCAP: OK
>   test VIDIOC_G/S_PRIORITY: OK
> 
> Debug ioctls:
>   test VIDIOC_DBG_G/S_REGISTER: OK
>   test VIDIOC_LOG_STATUS: OK (Not Supported)
> 
> Input ioctls:
>   test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
>   test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
>   test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
>   test VIDIOC_ENUMAUDIO: OK (Not Supported)
>   fail: v4l2-test-input-output.cpp(418): G_INPUT not supported 
> for a capture device

ENUM/G/S_INPUT is missing and is required for capture devices.

>   test VIDIOC_G/S/ENUMINPUT: FAIL
>   test VIDIOC_G/S_AUDIO: OK (Not Supported)
>   Inputs: 0 Audio Inputs: 0 Tuners: 0
> 
> Output ioctls:
>   test VIDIOC_G/S_MODULATOR: OK (Not Supported)
>   test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
>   test VIDIOC_ENUMAUDOUT: OK (Not Supported)
>   test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
>   test VIDIOC_G/S_AUDOUT: OK (Not Supported)
>   Outputs: 0 Audio Outputs: 0 Modulators: 0
> 
> Input/Output configuration ioctls:
>   test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
>   test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
>   test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
>   test VIDIOC_G/S_EDID: OK (Not Supported)
> 
>   Control ioctls:
>   test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK (Not Supported)
>   test VIDIOC_QUERYCTRL: OK (Not Supported)
>   test VIDIOC_G/S_CTRL: OK (Not Supported)
>   test VIDIOC_G/S/TRY_EXT_CTRLS: OK (Not Supported)
>   test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK (Not Supported)
>   test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
>   Standard Controls: 0 Private Controls: 0
> 
>   Format ioctls:
>   test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
>   test VIDIOC_G/S_PARM: OK (Not Supported)
>   test VIDIOC_G_FBUF: OK (Not Supported)
>   fail: v4l2-test-formats.cpp(329): pixelformat != 
> V4L2_PIX_FMT_JPEG && colorspace == V4L2_COLORSPACE_JPEG
>   fail: v4l2-test-formats.cpp(432): 
> testColorspace(pix.pixelformat, pix.colorspace, pix.ycbcr_enc, 
> pix.quantization)

The sensor should almost certainly use COLORSPACE_SRGB. Certainly not 
COLORSPACE_JPEG.

>   test VIDIOC_G_FMT: FAIL
>   test VIDIOC_TRY_FMT: OK (Not Supported)
>   test VIDIOC_S_FMT: OK (Not Supported)
>   test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)

Re: [PATCH RFC 0/2] pxa_camera transition to v4l2 standalone device

[Robert Jarzmik]

Hans Verkuil  writes:

> On 03/19/2016 10:01 PM, Robert Jarzmik wrote:
>> Hi Hans and Guennadi,
>> 
>> As Hans is converting sh_mobile_ceu_camera.c,
>
> That's not going as fast as I hoped. This driver is quite complex and 
> extracting
> it from soc-camera isn't easy. I also can't spend as much time as I'd like on 
> this.
>
>> let's see how close our ports are
>> to see if there are things we could either reuse of change.
>> 
>> The port is assuming :
>>  - the formation translation is transferred into soc_mediabus, so that it 
>> can be
>>reused across all v4l2 devices
>
> At best this will be a temporary helper source. I never liked soc_mediabus, I 
> don't
> believe it is the right approach.
As long as you provide a better approach, especially for the dynamic formats
translation, it should be fine.

> But I have no problem if it is used for now to simplify the soc-camera
> dependency removal.
Ok.

>>  - pxa_camera is ported
>> 
>> This sets a ground of discussion for soc_camera adherence removal from
>> pxa_camera. I'd like to have a comment from Hans if this is what he has in 
>> mind,
>> and Guennadi if he agrees to transfer the soc xlate stuff to soc_mediabus.
>
> Can you provide the output of 'v4l2-compliance -s' with your new pxa driver?
> I would be curious to see the result of that.
Of course, with [1] added (initial format init and querycap strings), I have
the following results. I have no idea where VIDIOC_EXPBUF failure comes from,
while the colorspace issues are a consequence from the MT9M111 sensor which
provides the couple ("VYUY", V4L2_COLORSPACE_JPEG) format (which I also don't
understand why it is a failure) :

Driver Info:
Driver name   : pxa27x-camera
Card type : PXA_Camera
Bus info  : platform:pxa-camera
Driver version: 4.5.0
Capabilities  : 0x8421
Video Capture
Streaming
Extended Pix Format
Device Capabilities
Device Caps   : 0x0421
Video Capture
Streaming
Extended Pix Format

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

Required ioctls:
test VIDIOC_QUERYCAP: OK

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

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

Input ioctls:
test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
test VIDIOC_ENUMAUDIO: OK (Not Supported)
fail: v4l2-test-input-output.cpp(418): G_INPUT not supported 
for a capture device
test VIDIOC_G/S/ENUMINPUT: FAIL
test VIDIOC_G/S_AUDIO: OK (Not Supported)
Inputs: 0 Audio Inputs: 0 Tuners: 0

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

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

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

Format ioctls:
test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
test VIDIOC_G/S_PARM: OK (Not Supported)
test VIDIOC_G_FBUF: OK (Not Supported)
fail: v4l2-test-formats.cpp(329): pixelformat != 
V4L2_PIX_FMT_JPEG && colorspace == V4L2_COLORSPACE_JPEG
fail: v4l2-test-formats.cpp(432): 
testColorspace(pix.pixelformat, pix.colorspace, pix.ycbcr_enc, pix.quantization)
test VIDIOC_G_FMT: FAIL
test VIDIOC_TRY_FMT: OK (Not Supported)
test VIDIOC_S_FMT: OK (Not Supported)
test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
test Cropping: OK (Not Supported)
test Composing: OK (Not Supported)
test Scaling: OK

Codec ioctls:
test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
test VIDIOC_G_ENC_INDEX: OK (Not Supported)

Re: [RFC PATCH 1/3] [media] v4l2-mc.h: Add a S-Video C input PAD to demod enum

[Mauro Carvalho Chehab]

Em Mon, 21 Mar 2016 16:20:09 -0300
Javier Martinez Canillas  escreveu:

> > BTW, if the tvp5150 needs to know which composite connector is connected
> > to which hardware pin, then you still may need to be explicit w.r.t. the
> > number of pads. I just thought of that.
> >  
> 
> The tvp5150 doesn't care about that, as Mauro said is just a mux so you can
> have logic in the .link_setup that does the mux depending on the remote
> entity (that's in fact how I implemented and is currently in mainline).
> 
> Now, the user needs to know which entity is mapped to which input pin.
> All its know from the HW documentation is that for example the left
> RCA connector is AIP1A and the one inf the right is connected to AIP1B.
> 
> So there could be a convention that the composite connected to AIP1A pin
> (the default) is Composite0 and the connected to AIP1B is Composite1.


We should avoid a convention here... instead, we should support
properties via the properties API to export enough info for userspace
to know what physical connectors correspond to each "connection" entity.

As we've discussed previously, such properties can be:
- connector's name
- color
- position
etc...


Regards,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 1/3] [media] v4l2-mc.h: Add a S-Video C input PAD to demod enum

[Mauro Carvalho Chehab]

Em Mon, 21 Mar 2016 16:48:12 -0300
Javier Martinez Canillas  escreveu:

> Hello Hans,
> 
> On 03/21/2016 04:30 PM, Hans Verkuil wrote:
> 
> [snip]
> 
> 
>  Can you please provide an example of a media pipeline that user-space 
>  should
>  use with this approach? AFAICT whatever PADs are created when 
>  initiliazing
>  the PADs for an entity, will be exposed to user-space in the media graph.
> 
>  So I'm not understading how it will be used in practice. I don't mean 
>  that
>  your approach is not correct, is just I'm not getting it :)  
> >>>
> >>> Why would userspace need to use the pads? This is for legacy drivers 
> >>> (right?)
> >>> where the pipeline is fixed anyway.
> >>>  
> >>
> >> I asked because the user needs to setup the links in the media pipeline to
> >> choose  which input connection will be linked to the tvp5150 decoder. But 
> >> it
> >> doesn't matter if we are going with the single sink pad approach since the
> >> user will always do something like:  
> > 
> > Why? The user will use an application that uses ENUM/S/G_INPUT for this. 
> > We're
> > talking legacy drivers ('interface centric drivers' would be a better 
> > description)
> > where we don't even expose the v4l-subdevX device nodes. Explicitly 
> > programming
> > a media pipeline is something you do for complex devices (embedded systems 
> > and
> > the like). Not for simple and generally fixed pipelines. Utterly pointless.
> >  
> 
> Mauro was talking about legacy 'interface centric' PC-consumer's hardware but
> my test system is an embedded board that also has a tvp5150 decoder. The
> board has an OMAP3 and the tvp5150 is attached to the SoC ISP. Is this one:
> 
> https://www.isee.biz/products/igep-expansion-boards/igepv2-expansion

Yeah, subdevs should be prepared to work with both "interface centric" and
"media controller centric" approaches.

Yet, I don't think using one sink pad for tvp5150 is a bad thing.

> As you can see, the board has 2 RCA connectors and each one is routed a 
> tvp5150
> composite input and both connectors can be used for S-Video. So the user needs
> to setup the pipeline manually to choose which input connection to use.

Actually, on your tests, you were not able to make this work, nor
S-Video is officially supported by the manufacturer. So, in the
specific case of IGEPv2, I would not be adding a S-Video connector.

Btw, even on devices with an S-Video connector and tvp5150, at
least on my tests, the driver were not able to setup S-Video. It
seems that there's something more than just setting the mux.

- 
Thanks,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 1/3] [media] v4l2-mc.h: Add a S-Video C input PAD to demod enum

[Javier Martinez Canillas]

Hello Mauro,

On 03/21/2016 03:34 PM, Mauro Carvalho Chehab wrote:
> Em Mon, 21 Mar 2016 15:24:00 -0300
> Javier Martinez Canillas  escreveu:
> 
>> Hello Hans,
>>
>> On 03/21/2016 03:08 PM, Hans Verkuil wrote:
>>> On 03/21/2016 06:50 PM, Mauro Carvalho Chehab wrote:
 Hi Hans,

 Em Mon, 21 Mar 2016 17:01:43 +0100
 Hans Verkuil  escreveu:

>> A reasonable solution to simplify converting legacy drivers without 
>> creating
>> these global ugly pad indices is to add a new video (and probably audio) 
>> op
>> 'g_pad_of_type(type)' where you ask the subdev entity to return which 
>> pad carries
>> signals of a certain type.  
>
> This basically puts a layer between the low-level pads as defined by the 
> entity
> and the 'meta-pads' that a generic MC link creator would need to handle 
> legacy
> drivers. The nice thing is that this is wholly inside the kernel so we can
> modify it at will later without impacting userspace.

 I prepared a long answer to your email, but I guess we're not at the
 same page.

 Let be clear on my view. Please let me know where you disagree:

 1) I'm not defending Javier's patchset. I have my restrictions to
 it too. My understanding is that he sent this as a RFC for feeding
 our discussions for the media summit.

 Javier, please correct me if I'm wrong.

>>
>> That's correct. I wanted to have some patches that were aligned to what
>> were discussed so far in order to have more examples to contribute in
>> the media summit discussion (since I won't be there).
>>
>> The patches are RFC and not meant to upstream since there are too many
>> open questions. I just hoped that having more examples could help of
>> them. I was specially interested in the DT bindings using OF graph to
>> lookup the connectors and the level of detail there.
>>
 2) I don't understand what you're calling as "meta-pads". For me, a
 PAD is a physical set of pins. 
>>>
>>> Poorly worded on my side. I'll elaborate below.
>>>
 3) IMO, the best is to have just one PAD for a decoder input. That makes
 everything simple, yet functional.

 In my view, the input PAD will be linked to several "input connections".
 So, in the case of tvp5150, it will have:

- composite 1
- composite 2
- s-video

 4) On that view, the input PAD is actually a set of pins. In the
 case of tvp5150, the pins that compose the input PADs are
 AIP1A and AIP1B.

 The output PAD is also a set of pins YOUT0 to YOUT7, plus some other
 pins for sync. Yet, it should, IMHO, have just one output PAD at
 the MC graph.
>>>
>>> Indeed. So a tvp5150 has three sink pads and one source pad (pixel port).
>>
>> Why 3 sink pads? Are we going to model each possible connection as a PAD
>> instead of an entity or are you talking about physical pins? Because if
>> is the latter, then the tvp5150 has only 2 (Composite1 shares S-Video Y
>> and Composite2 shares C signal).
>>
>>> Other similar devices may have different numbers of sink pads (say four
>>> composite sinks and no S-Video sinks). So the pads the entity creates
>>> should match what the hardware supports.
>>>
>>> So far, so good.
>>>
>>
>> I'm confused. I thought that the latest agreed approach was to model the
>> actual connection signals and input pins as PADs instead of a simplied
>> model that just each connection as a sink.
>>  
>>> If we want to create code that can more-or-less automatically create a MC
>>> topology for legacy drivers, then we would like to be able to map a 
>>> high-level
>>> description like 'the first S-Video sink pad' into the actual pad. So you'd
>>> have a 'MAP_PAD_SVID_1' define that, when passed to the g_pad_of_type() op
>>> would return the actual pad index for the first S-Video sink pad (or an 
>>> error
>>> if there isn't one). That's what I meant with 'meta-pad' (and let's just
>>> forget about that name, poor choice from my side).
>>>
>>
>> Can you please provide an example of a media pipeline that user-space should
>> use with this approach? AFAICT whatever PADs are created when initiliazing
>> the PADs for an entity, will be exposed to user-space in the media graph.
>>
>> So I'm not understading how it will be used in practice. I don't mean that
>> your approach is not correct, is just I'm not getting it :)
>>
>>> What I think Javier's patch did was to require subdevs that have an S-Video 
>>> pad
>>> to use the DEMOD_PAD_C_INPUT + IF_INPUT pad indices for that. That's really
>>> wrong. The subdev driver decides how many pads there are and which pad is
>>> assigned to which index. That shouldn't be forced on them from the outside
>>> because that won't scale.
>>>
>>
>> Yes, that was something that Mauro suggested in [0] as a possible approach
>> but I also was not sure about it and mentioned in the patch comments.
>>

Re: [PATCH v3] media: Support Intersil/Techwell TW686x-based video capture cards

[Ezequiel Garcia]

On 21 March 2016 at 08:44, Hans Verkuil  wrote:
> Hi Ezequiel,
>
> On 03/02/2016 03:30 PM, Ezequiel Garcia wrote:
>> This commit introduces the support for the Techwell TW686x video
>> capture IC. This hardware supports a few DMA modes, including
>> scatter-gather and frame (contiguous).
>>
>> This commit makes little use of the DMA engine and instead has
>> a memcpy based implementation. DMA frame and scatter-gather modes
>> support may be added in the future.
>>
>> Currently supported chips:
>> - TW6864 (4 video channels),
>> - TW6865 (4 video channels, not tested, second generation chip),
>> - TW6868 (8 video channels but only 4 first channels using
>>built-in video decoder are supported, not tested),
>> - TW6869 (8 video channels, second generation chip).
>>
>> Cc: Krzysztof Hałasa 
>> Signed-off-by: Ezequiel Garcia 
>
> I've tested this with my PCIe tw6869 card (arrived this week).
>
> Video is fine, but only the first audio input is available. I suspect you
> have only one audio input?
>

The driver exposes the eight audio channels as subdevices
(ALSA substreams) of a single device.

$ sudo arecord -l
 List of CAPTURE Hardware Devices 
[..]
card 1: tw686x [tw686x], device 0: tw686x [tw686x PCM]
  Subdevices: 8/8
  Subdevice #0: vch0 audio
  Subdevice #1: vch1 audio
  Subdevice #2: vch2 audio
  Subdevice #3: vch3 audio
  Subdevice #4: vch4 audio
  Subdevice #5: vch5 audio
  Subdevice #6: vch6 audio
  Subdevice #7: vch7 audio

> Regarding the memcpy: if you have a patch for me that reverts back to a 
> non-memcpy
> situation, then I can do duration tests for you.
>

Yes, that would be great. I've just pushed my staging branch:

http://git.infradead.org/users/ezequielg/linux/shortlog/refs/heads/tw686x-upstream-for-v4.7

It introduces a dma_mode parameter which let's the user pick the
DMA operation. It accepts these values: memcpy, contig, sg. Only
"memcpy" and "contig" are working, while "sg" needs more work.

Feel free to test it and let me know how it goes.

Thanks,
-- 
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 1/3] [media] v4l2-mc.h: Add a S-Video C input PAD to demod enum

[Javier Martinez Canillas]

Hello Hans,

On 03/21/2016 04:30 PM, Hans Verkuil wrote:

[snip]


 Can you please provide an example of a media pipeline that user-space 
 should
 use with this approach? AFAICT whatever PADs are created when initiliazing
 the PADs for an entity, will be exposed to user-space in the media graph.

 So I'm not understading how it will be used in practice. I don't mean that
 your approach is not correct, is just I'm not getting it :)
>>>
>>> Why would userspace need to use the pads? This is for legacy drivers 
>>> (right?)
>>> where the pipeline is fixed anyway.
>>>
>>
>> I asked because the user needs to setup the links in the media pipeline to
>> choose  which input connection will be linked to the tvp5150 decoder. But it
>> doesn't matter if we are going with the single sink pad approach since the
>> user will always do something like:
> 
> Why? The user will use an application that uses ENUM/S/G_INPUT for this. We're
> talking legacy drivers ('interface centric drivers' would be a better 
> description)
> where we don't even expose the v4l-subdevX device nodes. Explicitly 
> programming
> a media pipeline is something you do for complex devices (embedded systems and
> the like). Not for simple and generally fixed pipelines. Utterly pointless.
>

Mauro was talking about legacy 'interface centric' PC-consumer's hardware but
my test system is an embedded board that also has a tvp5150 decoder. The
board has an OMAP3 and the tvp5150 is attached to the SoC ISP. Is this one:

https://www.isee.biz/products/igep-expansion-boards/igepv2-expansion

As you can see, the board has 2 RCA connectors and each one is routed a tvp5150
composite input and both connectors can be used for S-Video. So the user needs
to setup the pipeline manually to choose which input connection to use.

But on a second read of the thread, it seems that you were referring to the
meta-pads only for the 'interace centric' drivers so maybe I misunderstood you.

Sorry for the noise if that was the case.
 
>>
>> $ media-ctl -r -l '"Composite0":0->"tvp5150 1-005c":0[1]'
>>
>> IOW, there will always choose the only connection source pad and tvp5150 
>> sink.
>>
>> There will be two source pads for the tvp5150 though, 1 for video and other
>> for VBI. But I guess this is not an issue since that's easier to standardize.
> 
> Not all devices have VBI. Some devices may have *only* VBI (although the last
> driver of that kind was removed from the kernel a long time ago), there may
> be multiple video source pads, and when we add HDMI I can think of a lot more
> complex scenarios. So source pads shouldn't have their pad indices imposed on
> them by outside 'arrangements'. It is really the wrong approach, regardless of
> whether we talk about sink or source pads.
>

Ok, thanks for the explanation.
 
> Regards,
> 
>   Hans
> 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

FOR YOUR INFORMATION DEAR BENEFICIARY,

[David Kedogo]

FOR YOUR INFORMATION DEAR BENEFICIARY,

Your Over-due ATM Card payment of $1.5MUSD by the UN Office have
deposited to the ATM MASTER CARD OFFICE. All you have to do now is to
contact the Office Manager Dr. peter Akupa Boni at:
(peterakupa...@gmail.com) and Phone number: +229 61 31 07 78  , he
will give you direction on how you will receive your ATM MASTER CARD
immediately.

Contact person: Dr. peter Akupa,
E-mail: (peterakupa...@gmail.com )

You have to also contact him with the following details:

1. Your full name:
2. Your contact cell phone number:
3. Your age:
4. your Email

Thanks
David Kedogo
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 1/3] [media] v4l2-mc.h: Add a S-Video C input PAD to demod enum

[Hans Verkuil]

On 03/21/2016 08:20 PM, Javier Martinez Canillas wrote:
> Hello Hans,
> 
> On 03/21/2016 04:06 PM, Hans Verkuil wrote:
>> On 03/21/2016 07:24 PM, Javier Martinez Canillas wrote:
>>> Hello Hans,
>>>
>>> On 03/21/2016 03:08 PM, Hans Verkuil wrote:
 On 03/21/2016 06:50 PM, Mauro Carvalho Chehab wrote:
> Hi Hans,
>
> Em Mon, 21 Mar 2016 17:01:43 +0100
> Hans Verkuil  escreveu:
>
>>> A reasonable solution to simplify converting legacy drivers without 
>>> creating
>>> these global ugly pad indices is to add a new video (and probably 
>>> audio) op
>>> 'g_pad_of_type(type)' where you ask the subdev entity to return which 
>>> pad carries
>>> signals of a certain type.  
>>
>> This basically puts a layer between the low-level pads as defined by the 
>> entity
>> and the 'meta-pads' that a generic MC link creator would need to handle 
>> legacy
>> drivers. The nice thing is that this is wholly inside the kernel so we 
>> can
>> modify it at will later without impacting userspace.
>
> I prepared a long answer to your email, but I guess we're not at the
> same page.
>
> Let be clear on my view. Please let me know where you disagree:
>
> 1) I'm not defending Javier's patchset. I have my restrictions to
> it too. My understanding is that he sent this as a RFC for feeding
> our discussions for the media summit.
>
> Javier, please correct me if I'm wrong.
>
>>>
>>> That's correct. I wanted to have some patches that were aligned to what
>>> were discussed so far in order to have more examples to contribute in
>>> the media summit discussion (since I won't be there).
>>>
>>> The patches are RFC and not meant to upstream since there are too many
>>> open questions. I just hoped that having more examples could help of
>>> them. I was specially interested in the DT bindings using OF graph to
>>> lookup the connectors and the level of detail there.
>>>
> 2) I don't understand what you're calling as "meta-pads". For me, a
> PAD is a physical set of pins. 

 Poorly worded on my side. I'll elaborate below.

> 3) IMO, the best is to have just one PAD for a decoder input. That makes
> everything simple, yet functional.
>
> In my view, the input PAD will be linked to several "input connections".
> So, in the case of tvp5150, it will have:
>
>   - composite 1
>   - composite 2
>   - s-video
>
> 4) On that view, the input PAD is actually a set of pins. In the
> case of tvp5150, the pins that compose the input PADs are
> AIP1A and AIP1B.
>
> The output PAD is also a set of pins YOUT0 to YOUT7, plus some other
> pins for sync. Yet, it should, IMHO, have just one output PAD at
> the MC graph.

 Indeed. So a tvp5150 has three sink pads and one source pad (pixel port).
>>>
>>> Why 3 sink pads? Are we going to model each possible connection as a PAD
>>> instead of an entity or are you talking about physical pins? Because if
>>> is the latter, then the tvp5150 has only 2 (Composite1 shares S-Video Y
>>> and Composite2 shares C signal).
>>
>> I'd go with Mauro's proposal of a single pad. And I didn't look into detail
>> in the hardware specs of the tvp5150, so that's why I got it wrong.
>>
> 
> Ok.
> 
 Other similar devices may have different numbers of sink pads (say four
 composite sinks and no S-Video sinks). So the pads the entity creates
 should match what the hardware supports.

 So far, so good.

>>>
>>> I'm confused. I thought that the latest agreed approach was to model the
>>> actual connection signals and input pins as PADs instead of a simplied
>>> model that just each connection as a sink.
>>
>> My opinion is to just use the simple option (one pad) if you can get away
>> with it. I.e. in this case adding more sink pads doesn't add any useful
>> information. In the case of an adv7604 it does provide useful information 
>> since
>> you need to program the adv7604 based on how it is hooked up.
>>
> 
> Agreed.
>  
>> BTW, if the tvp5150 needs to know which composite connector is connected
>> to which hardware pin, then you still may need to be explicit w.r.t. the
>> number of pads. I just thought of that.
>>
> 
> The tvp5150 doesn't care about that, as Mauro said is just a mux so you can
> have logic in the .link_setup that does the mux depending on the remote
> entity (that's in fact how I implemented and is currently in mainline).
> 
> Now, the user needs to know which entity is mapped to which input pin.
> All its know from the HW documentation is that for example the left
> RCA connector is AIP1A and the one inf the right is connected to AIP1B.
> 
> So there could be a convention that the composite connected to AIP1A pin
> (the default) is Composite0 and the connected to AIP1B is Composite1.
> 
 If we want to create code that can more-or-less automatically 

Re: [RFC PATCH 1/3] [media] v4l2-mc.h: Add a S-Video C input PAD to demod enum

[Mauro Carvalho Chehab]

Em Mon, 21 Mar 2016 15:24:00 -0300
Javier Martinez Canillas  escreveu:

> Hello Hans,
> 
> On 03/21/2016 03:08 PM, Hans Verkuil wrote:
> > On 03/21/2016 06:50 PM, Mauro Carvalho Chehab wrote:
> >> Hi Hans,
> >>
> >> Em Mon, 21 Mar 2016 17:01:43 +0100
> >> Hans Verkuil  escreveu:
> >>
>  A reasonable solution to simplify converting legacy drivers without 
>  creating
>  these global ugly pad indices is to add a new video (and probably audio) 
>  op
>  'g_pad_of_type(type)' where you ask the subdev entity to return which 
>  pad carries
>  signals of a certain type.  
> >>>
> >>> This basically puts a layer between the low-level pads as defined by the 
> >>> entity
> >>> and the 'meta-pads' that a generic MC link creator would need to handle 
> >>> legacy
> >>> drivers. The nice thing is that this is wholly inside the kernel so we can
> >>> modify it at will later without impacting userspace.
> >>
> >> I prepared a long answer to your email, but I guess we're not at the
> >> same page.
> >>
> >> Let be clear on my view. Please let me know where you disagree:
> >>
> >> 1) I'm not defending Javier's patchset. I have my restrictions to
> >> it too. My understanding is that he sent this as a RFC for feeding
> >> our discussions for the media summit.
> >>
> >> Javier, please correct me if I'm wrong.
> >>
> 
> That's correct. I wanted to have some patches that were aligned to what
> were discussed so far in order to have more examples to contribute in
> the media summit discussion (since I won't be there).
> 
> The patches are RFC and not meant to upstream since there are too many
> open questions. I just hoped that having more examples could help of
> them. I was specially interested in the DT bindings using OF graph to
> lookup the connectors and the level of detail there.
> 
> >> 2) I don't understand what you're calling as "meta-pads". For me, a
> >> PAD is a physical set of pins. 
> > 
> > Poorly worded on my side. I'll elaborate below.
> > 
> >> 3) IMO, the best is to have just one PAD for a decoder input. That makes
> >> everything simple, yet functional.
> >>
> >> In my view, the input PAD will be linked to several "input connections".
> >> So, in the case of tvp5150, it will have:
> >>
> >>- composite 1
> >>- composite 2
> >>- s-video
> >>
> >> 4) On that view, the input PAD is actually a set of pins. In the
> >> case of tvp5150, the pins that compose the input PADs are
> >> AIP1A and AIP1B.
> >>
> >> The output PAD is also a set of pins YOUT0 to YOUT7, plus some other
> >> pins for sync. Yet, it should, IMHO, have just one output PAD at
> >> the MC graph.
> > 
> > Indeed. So a tvp5150 has three sink pads and one source pad (pixel port).
> 
> Why 3 sink pads? Are we going to model each possible connection as a PAD
> instead of an entity or are you talking about physical pins? Because if
> is the latter, then the tvp5150 has only 2 (Composite1 shares S-Video Y
> and Composite2 shares C signal).
> 
> > Other similar devices may have different numbers of sink pads (say four
> > composite sinks and no S-Video sinks). So the pads the entity creates
> > should match what the hardware supports.
> > 
> > So far, so good.
> >
> 
> I'm confused. I thought that the latest agreed approach was to model the
> actual connection signals and input pins as PADs instead of a simplied
> model that just each connection as a sink.
>  
> > If we want to create code that can more-or-less automatically create a MC
> > topology for legacy drivers, then we would like to be able to map a 
> > high-level
> > description like 'the first S-Video sink pad' into the actual pad. So you'd
> > have a 'MAP_PAD_SVID_1' define that, when passed to the g_pad_of_type() op
> > would return the actual pad index for the first S-Video sink pad (or an 
> > error
> > if there isn't one). That's what I meant with 'meta-pad' (and let's just
> > forget about that name, poor choice from my side).
> >
> 
> Can you please provide an example of a media pipeline that user-space should
> use with this approach? AFAICT whatever PADs are created when initiliazing
> the PADs for an entity, will be exposed to user-space in the media graph.
> 
> So I'm not understading how it will be used in practice. I don't mean that
> your approach is not correct, is just I'm not getting it :)
> 
> > What I think Javier's patch did was to require subdevs that have an S-Video 
> > pad
> > to use the DEMOD_PAD_C_INPUT + IF_INPUT pad indices for that. That's really
> > wrong. The subdev driver decides how many pads there are and which pad is
> > assigned to which index. That shouldn't be forced on them from the outside
> > because that won't scale.
> >
> 
> Yes, that was something that Mauro suggested in [0] as a possible approach
> but I also was not sure about it and mentioned in the patch comments.
> 
> > But you can make an op that asks 'which pad carries this signal?'. That's 
> > fine.
> >
> 

Re: [RFC PATCH 1/3] [media] v4l2-mc.h: Add a S-Video C input PAD to demod enum

[Hans Verkuil]

On 03/21/2016 07:34 PM, Mauro Carvalho Chehab wrote:
> Em Mon, 21 Mar 2016 15:24:00 -0300
> Javier Martinez Canillas  escreveu:
> 
>> Hello Hans,
>>
>> On 03/21/2016 03:08 PM, Hans Verkuil wrote:
>>> On 03/21/2016 06:50 PM, Mauro Carvalho Chehab wrote:
 Hi Hans,

 Em Mon, 21 Mar 2016 17:01:43 +0100
 Hans Verkuil  escreveu:

>> A reasonable solution to simplify converting legacy drivers without 
>> creating
>> these global ugly pad indices is to add a new video (and probably audio) 
>> op
>> 'g_pad_of_type(type)' where you ask the subdev entity to return which 
>> pad carries
>> signals of a certain type.  
>
> This basically puts a layer between the low-level pads as defined by the 
> entity
> and the 'meta-pads' that a generic MC link creator would need to handle 
> legacy
> drivers. The nice thing is that this is wholly inside the kernel so we can
> modify it at will later without impacting userspace.

 I prepared a long answer to your email, but I guess we're not at the
 same page.

 Let be clear on my view. Please let me know where you disagree:

 1) I'm not defending Javier's patchset. I have my restrictions to
 it too. My understanding is that he sent this as a RFC for feeding
 our discussions for the media summit.

 Javier, please correct me if I'm wrong.

>>
>> That's correct. I wanted to have some patches that were aligned to what
>> were discussed so far in order to have more examples to contribute in
>> the media summit discussion (since I won't be there).
>>
>> The patches are RFC and not meant to upstream since there are too many
>> open questions. I just hoped that having more examples could help of
>> them. I was specially interested in the DT bindings using OF graph to
>> lookup the connectors and the level of detail there.
>>
 2) I don't understand what you're calling as "meta-pads". For me, a
 PAD is a physical set of pins. 
>>>
>>> Poorly worded on my side. I'll elaborate below.
>>>
 3) IMO, the best is to have just one PAD for a decoder input. That makes
 everything simple, yet functional.

 In my view, the input PAD will be linked to several "input connections".
 So, in the case of tvp5150, it will have:

- composite 1
- composite 2
- s-video

 4) On that view, the input PAD is actually a set of pins. In the
 case of tvp5150, the pins that compose the input PADs are
 AIP1A and AIP1B.

 The output PAD is also a set of pins YOUT0 to YOUT7, plus some other
 pins for sync. Yet, it should, IMHO, have just one output PAD at
 the MC graph.
>>>
>>> Indeed. So a tvp5150 has three sink pads and one source pad (pixel port).
>>
>> Why 3 sink pads? Are we going to model each possible connection as a PAD
>> instead of an entity or are you talking about physical pins? Because if
>> is the latter, then the tvp5150 has only 2 (Composite1 shares S-Video Y
>> and Composite2 shares C signal).
>>
>>> Other similar devices may have different numbers of sink pads (say four
>>> composite sinks and no S-Video sinks). So the pads the entity creates
>>> should match what the hardware supports.
>>>
>>> So far, so good.
>>>
>>
>> I'm confused. I thought that the latest agreed approach was to model the
>> actual connection signals and input pins as PADs instead of a simplied
>> model that just each connection as a sink.
>>  
>>> If we want to create code that can more-or-less automatically create a MC
>>> topology for legacy drivers, then we would like to be able to map a 
>>> high-level
>>> description like 'the first S-Video sink pad' into the actual pad. So you'd
>>> have a 'MAP_PAD_SVID_1' define that, when passed to the g_pad_of_type() op
>>> would return the actual pad index for the first S-Video sink pad (or an 
>>> error
>>> if there isn't one). That's what I meant with 'meta-pad' (and let's just
>>> forget about that name, poor choice from my side).
>>>
>>
>> Can you please provide an example of a media pipeline that user-space should
>> use with this approach? AFAICT whatever PADs are created when initiliazing
>> the PADs for an entity, will be exposed to user-space in the media graph.
>>
>> So I'm not understading how it will be used in practice. I don't mean that
>> your approach is not correct, is just I'm not getting it :)
>>
>>> What I think Javier's patch did was to require subdevs that have an S-Video 
>>> pad
>>> to use the DEMOD_PAD_C_INPUT + IF_INPUT pad indices for that. That's really
>>> wrong. The subdev driver decides how many pads there are and which pad is
>>> assigned to which index. That shouldn't be forced on them from the outside
>>> because that won't scale.
>>>
>>
>> Yes, that was something that Mauro suggested in [0] as a possible approach
>> but I also was not sure about it and mentioned in the patch comments.
>>
>>> But you can 

Re: [RFC PATCH 1/3] [media] v4l2-mc.h: Add a S-Video C input PAD to demod enum

[Hans Verkuil]

On 03/21/2016 07:23 PM, Mauro Carvalho Chehab wrote:
>> Indeed. So a tvp5150 has three sink pads and one source pad (pixel port).
> 
> I would actually map tvp5150 with just one sink pad, with 3 links
> (one for each connector).
> 
> In other words, I'm mapping tvp5150 input mux via links, and the
> output of its mux as the sink pad.
> 
> IMHO, this works a way better than one sink pad per input at its
> internal mux.

You're right, that would work better for this specific device.

Regards,

Hans

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 1/3] [media] v4l2-mc.h: Add a S-Video C input PAD to demod enum

[Hans Verkuil]

On 03/21/2016 07:24 PM, Javier Martinez Canillas wrote:
> Hello Hans,
> 
> On 03/21/2016 03:08 PM, Hans Verkuil wrote:
>> On 03/21/2016 06:50 PM, Mauro Carvalho Chehab wrote:
>>> Hi Hans,
>>>
>>> Em Mon, 21 Mar 2016 17:01:43 +0100
>>> Hans Verkuil  escreveu:
>>>
> A reasonable solution to simplify converting legacy drivers without 
> creating
> these global ugly pad indices is to add a new video (and probably audio) 
> op
> 'g_pad_of_type(type)' where you ask the subdev entity to return which pad 
> carries
> signals of a certain type.  

 This basically puts a layer between the low-level pads as defined by the 
 entity
 and the 'meta-pads' that a generic MC link creator would need to handle 
 legacy
 drivers. The nice thing is that this is wholly inside the kernel so we can
 modify it at will later without impacting userspace.
>>>
>>> I prepared a long answer to your email, but I guess we're not at the
>>> same page.
>>>
>>> Let be clear on my view. Please let me know where you disagree:
>>>
>>> 1) I'm not defending Javier's patchset. I have my restrictions to
>>> it too. My understanding is that he sent this as a RFC for feeding
>>> our discussions for the media summit.
>>>
>>> Javier, please correct me if I'm wrong.
>>>
> 
> That's correct. I wanted to have some patches that were aligned to what
> were discussed so far in order to have more examples to contribute in
> the media summit discussion (since I won't be there).
> 
> The patches are RFC and not meant to upstream since there are too many
> open questions. I just hoped that having more examples could help of
> them. I was specially interested in the DT bindings using OF graph to
> lookup the connectors and the level of detail there.
> 
>>> 2) I don't understand what you're calling as "meta-pads". For me, a
>>> PAD is a physical set of pins. 
>>
>> Poorly worded on my side. I'll elaborate below.
>>
>>> 3) IMO, the best is to have just one PAD for a decoder input. That makes
>>> everything simple, yet functional.
>>>
>>> In my view, the input PAD will be linked to several "input connections".
>>> So, in the case of tvp5150, it will have:
>>>
>>> - composite 1
>>> - composite 2
>>> - s-video
>>>
>>> 4) On that view, the input PAD is actually a set of pins. In the
>>> case of tvp5150, the pins that compose the input PADs are
>>> AIP1A and AIP1B.
>>>
>>> The output PAD is also a set of pins YOUT0 to YOUT7, plus some other
>>> pins for sync. Yet, it should, IMHO, have just one output PAD at
>>> the MC graph.
>>
>> Indeed. So a tvp5150 has three sink pads and one source pad (pixel port).
> 
> Why 3 sink pads? Are we going to model each possible connection as a PAD
> instead of an entity or are you talking about physical pins? Because if
> is the latter, then the tvp5150 has only 2 (Composite1 shares S-Video Y
> and Composite2 shares C signal).

I'd go with Mauro's proposal of a single pad. And I didn't look into detail
in the hardware specs of the tvp5150, so that's why I got it wrong.

>> Other similar devices may have different numbers of sink pads (say four
>> composite sinks and no S-Video sinks). So the pads the entity creates
>> should match what the hardware supports.
>>
>> So far, so good.
>>
> 
> I'm confused. I thought that the latest agreed approach was to model the
> actual connection signals and input pins as PADs instead of a simplied
> model that just each connection as a sink.

My opinion is to just use the simple option (one pad) if you can get away
with it. I.e. in this case adding more sink pads doesn't add any useful
information. In the case of an adv7604 it does provide useful information since
you need to program the adv7604 based on how it is hooked up.

BTW, if the tvp5150 needs to know which composite connector is connected
to which hardware pin, then you still may need to be explicit w.r.t. the
number of pads. I just thought of that.

>> If we want to create code that can more-or-less automatically create a MC
>> topology for legacy drivers, then we would like to be able to map a 
>> high-level
>> description like 'the first S-Video sink pad' into the actual pad. So you'd
>> have a 'MAP_PAD_SVID_1' define that, when passed to the g_pad_of_type() op
>> would return the actual pad index for the first S-Video sink pad (or an error
>> if there isn't one). That's what I meant with 'meta-pad' (and let's just
>> forget about that name, poor choice from my side).
>>
> 
> Can you please provide an example of a media pipeline that user-space should
> use with this approach? AFAICT whatever PADs are created when initiliazing
> the PADs for an entity, will be exposed to user-space in the media graph.
> 
> So I'm not understading how it will be used in practice. I don't mean that
> your approach is not correct, is just I'm not getting it :)

Why would userspace need to use the pads? This is for legacy drivers (right?)
where the pipeline is fixed anyway.

Re: [RFC PATCH 1/3] [media] v4l2-mc.h: Add a S-Video C input PAD to demod enum

[Javier Martinez Canillas]

Hello Hans,

On 03/21/2016 04:06 PM, Hans Verkuil wrote:
> On 03/21/2016 07:24 PM, Javier Martinez Canillas wrote:
>> Hello Hans,
>>
>> On 03/21/2016 03:08 PM, Hans Verkuil wrote:
>>> On 03/21/2016 06:50 PM, Mauro Carvalho Chehab wrote:
 Hi Hans,

 Em Mon, 21 Mar 2016 17:01:43 +0100
 Hans Verkuil  escreveu:

>> A reasonable solution to simplify converting legacy drivers without 
>> creating
>> these global ugly pad indices is to add a new video (and probably audio) 
>> op
>> 'g_pad_of_type(type)' where you ask the subdev entity to return which 
>> pad carries
>> signals of a certain type.  
>
> This basically puts a layer between the low-level pads as defined by the 
> entity
> and the 'meta-pads' that a generic MC link creator would need to handle 
> legacy
> drivers. The nice thing is that this is wholly inside the kernel so we can
> modify it at will later without impacting userspace.

 I prepared a long answer to your email, but I guess we're not at the
 same page.

 Let be clear on my view. Please let me know where you disagree:

 1) I'm not defending Javier's patchset. I have my restrictions to
 it too. My understanding is that he sent this as a RFC for feeding
 our discussions for the media summit.

 Javier, please correct me if I'm wrong.

>>
>> That's correct. I wanted to have some patches that were aligned to what
>> were discussed so far in order to have more examples to contribute in
>> the media summit discussion (since I won't be there).
>>
>> The patches are RFC and not meant to upstream since there are too many
>> open questions. I just hoped that having more examples could help of
>> them. I was specially interested in the DT bindings using OF graph to
>> lookup the connectors and the level of detail there.
>>
 2) I don't understand what you're calling as "meta-pads". For me, a
 PAD is a physical set of pins. 
>>>
>>> Poorly worded on my side. I'll elaborate below.
>>>
 3) IMO, the best is to have just one PAD for a decoder input. That makes
 everything simple, yet functional.

 In my view, the input PAD will be linked to several "input connections".
 So, in the case of tvp5150, it will have:

- composite 1
- composite 2
- s-video

 4) On that view, the input PAD is actually a set of pins. In the
 case of tvp5150, the pins that compose the input PADs are
 AIP1A and AIP1B.

 The output PAD is also a set of pins YOUT0 to YOUT7, plus some other
 pins for sync. Yet, it should, IMHO, have just one output PAD at
 the MC graph.
>>>
>>> Indeed. So a tvp5150 has three sink pads and one source pad (pixel port).
>>
>> Why 3 sink pads? Are we going to model each possible connection as a PAD
>> instead of an entity or are you talking about physical pins? Because if
>> is the latter, then the tvp5150 has only 2 (Composite1 shares S-Video Y
>> and Composite2 shares C signal).
> 
> I'd go with Mauro's proposal of a single pad. And I didn't look into detail
> in the hardware specs of the tvp5150, so that's why I got it wrong.
> 

Ok.

>>> Other similar devices may have different numbers of sink pads (say four
>>> composite sinks and no S-Video sinks). So the pads the entity creates
>>> should match what the hardware supports.
>>>
>>> So far, so good.
>>>
>>
>> I'm confused. I thought that the latest agreed approach was to model the
>> actual connection signals and input pins as PADs instead of a simplied
>> model that just each connection as a sink.
> 
> My opinion is to just use the simple option (one pad) if you can get away
> with it. I.e. in this case adding more sink pads doesn't add any useful
> information. In the case of an adv7604 it does provide useful information 
> since
> you need to program the adv7604 based on how it is hooked up.
>

Agreed.
 
> BTW, if the tvp5150 needs to know which composite connector is connected
> to which hardware pin, then you still may need to be explicit w.r.t. the
> number of pads. I just thought of that.
>

The tvp5150 doesn't care about that, as Mauro said is just a mux so you can
have logic in the .link_setup that does the mux depending on the remote
entity (that's in fact how I implemented and is currently in mainline).

Now, the user needs to know which entity is mapped to which input pin.
All its know from the HW documentation is that for example the left
RCA connector is AIP1A and the one inf the right is connected to AIP1B.

So there could be a convention that the composite connected to AIP1A pin
(the default) is Composite0 and the connected to AIP1B is Composite1.

>>> If we want to create code that can more-or-less automatically create a MC
>>> topology for legacy drivers, then we would like to be able to map a 
>>> high-level
>>> description like 'the first S-Video sink pad' into the actual pad. So you'd
>>> have a 

Re: [RFC PATCH 1/3] [media] v4l2-mc.h: Add a S-Video C input PAD to demod enum

[Javier Martinez Canillas]

Hello Hans,

On 03/21/2016 03:08 PM, Hans Verkuil wrote:
> On 03/21/2016 06:50 PM, Mauro Carvalho Chehab wrote:
>> Hi Hans,
>>
>> Em Mon, 21 Mar 2016 17:01:43 +0100
>> Hans Verkuil  escreveu:
>>
 A reasonable solution to simplify converting legacy drivers without 
 creating
 these global ugly pad indices is to add a new video (and probably audio) op
 'g_pad_of_type(type)' where you ask the subdev entity to return which pad 
 carries
 signals of a certain type.  
>>>
>>> This basically puts a layer between the low-level pads as defined by the 
>>> entity
>>> and the 'meta-pads' that a generic MC link creator would need to handle 
>>> legacy
>>> drivers. The nice thing is that this is wholly inside the kernel so we can
>>> modify it at will later without impacting userspace.
>>
>> I prepared a long answer to your email, but I guess we're not at the
>> same page.
>>
>> Let be clear on my view. Please let me know where you disagree:
>>
>> 1) I'm not defending Javier's patchset. I have my restrictions to
>> it too. My understanding is that he sent this as a RFC for feeding
>> our discussions for the media summit.
>>
>> Javier, please correct me if I'm wrong.
>>

That's correct. I wanted to have some patches that were aligned to what
were discussed so far in order to have more examples to contribute in
the media summit discussion (since I won't be there).

The patches are RFC and not meant to upstream since there are too many
open questions. I just hoped that having more examples could help of
them. I was specially interested in the DT bindings using OF graph to
lookup the connectors and the level of detail there.

>> 2) I don't understand what you're calling as "meta-pads". For me, a
>> PAD is a physical set of pins. 
> 
> Poorly worded on my side. I'll elaborate below.
> 
>> 3) IMO, the best is to have just one PAD for a decoder input. That makes
>> everything simple, yet functional.
>>
>> In my view, the input PAD will be linked to several "input connections".
>> So, in the case of tvp5150, it will have:
>>
>>  - composite 1
>>  - composite 2
>>  - s-video
>>
>> 4) On that view, the input PAD is actually a set of pins. In the
>> case of tvp5150, the pins that compose the input PADs are
>> AIP1A and AIP1B.
>>
>> The output PAD is also a set of pins YOUT0 to YOUT7, plus some other
>> pins for sync. Yet, it should, IMHO, have just one output PAD at
>> the MC graph.
> 
> Indeed. So a tvp5150 has three sink pads and one source pad (pixel port).

Why 3 sink pads? Are we going to model each possible connection as a PAD
instead of an entity or are you talking about physical pins? Because if
is the latter, then the tvp5150 has only 2 (Composite1 shares S-Video Y
and Composite2 shares C signal).

> Other similar devices may have different numbers of sink pads (say four
> composite sinks and no S-Video sinks). So the pads the entity creates
> should match what the hardware supports.
> 
> So far, so good.
>

I'm confused. I thought that the latest agreed approach was to model the
actual connection signals and input pins as PADs instead of a simplied
model that just each connection as a sink.
 
> If we want to create code that can more-or-less automatically create a MC
> topology for legacy drivers, then we would like to be able to map a high-level
> description like 'the first S-Video sink pad' into the actual pad. So you'd
> have a 'MAP_PAD_SVID_1' define that, when passed to the g_pad_of_type() op
> would return the actual pad index for the first S-Video sink pad (or an error
> if there isn't one). That's what I meant with 'meta-pad' (and let's just
> forget about that name, poor choice from my side).
>

Can you please provide an example of a media pipeline that user-space should
use with this approach? AFAICT whatever PADs are created when initiliazing
the PADs for an entity, will be exposed to user-space in the media graph.

So I'm not understading how it will be used in practice. I don't mean that
your approach is not correct, is just I'm not getting it :)

> What I think Javier's patch did was to require subdevs that have an S-Video 
> pad
> to use the DEMOD_PAD_C_INPUT + IF_INPUT pad indices for that. That's really
> wrong. The subdev driver decides how many pads there are and which pad is
> assigned to which index. That shouldn't be forced on them from the outside
> because that won't scale.
>

Yes, that was something that Mauro suggested in [0] as a possible approach
but I also was not sure about it and mentioned in the patch comments.

> But you can make an op that asks 'which pad carries this signal?'. That's 
> fine.
>
> I hope this clarifies matters.
> 
> Regards,
> 
>   Hans
> 

[0]: http://www.spinics.net/lists/linux-media/msg98042.html

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to 

Re: [RFC PATCH 1/3] [media] v4l2-mc.h: Add a S-Video C input PAD to demod enum

[Mauro Carvalho Chehab]

Em Mon, 21 Mar 2016 19:08:32 +0100
Hans Verkuil  escreveu:

> On 03/21/2016 06:50 PM, Mauro Carvalho Chehab wrote:
> > Hi Hans,
> > 
> > Em Mon, 21 Mar 2016 17:01:43 +0100
> > Hans Verkuil  escreveu:
> >   
> >>> A reasonable solution to simplify converting legacy drivers without 
> >>> creating
> >>> these global ugly pad indices is to add a new video (and probably audio) 
> >>> op
> >>> 'g_pad_of_type(type)' where you ask the subdev entity to return which pad 
> >>> carries
> >>> signals of a certain type.
> >>
> >> This basically puts a layer between the low-level pads as defined by the 
> >> entity
> >> and the 'meta-pads' that a generic MC link creator would need to handle 
> >> legacy
> >> drivers. The nice thing is that this is wholly inside the kernel so we can
> >> modify it at will later without impacting userspace.  
> > 
> > I prepared a long answer to your email, but I guess we're not at the
> > same page.
> > 
> > Let be clear on my view. Please let me know where you disagree:
> > 
> > 1) I'm not defending Javier's patchset. I have my restrictions to
> > it too. My understanding is that he sent this as a RFC for feeding
> > our discussions for the media summit.
> > 
> > Javier, please correct me if I'm wrong.
> > 
> > 2) I don't understand what you're calling as "meta-pads". For me, a
> > PAD is a physical set of pins.   
> 
> Poorly worded on my side. I'll elaborate below.
> 
> > 3) IMO, the best is to have just one PAD for a decoder input. That makes
> > everything simple, yet functional.
> > 
> > In my view, the input PAD will be linked to several "input connections".
> > So, in the case of tvp5150, it will have:
> > 
> > - composite 1
> > - composite 2
> > - s-video
> > 
> > 4) On that view, the input PAD is actually a set of pins. In the
> > case of tvp5150, the pins that compose the input PADs are
> > AIP1A and AIP1B.
> > 
> > The output PAD is also a set of pins YOUT0 to YOUT7, plus some other
> > pins for sync. Yet, it should, IMHO, have just one output PAD at
> > the MC graph.  
> 
> Indeed. So a tvp5150 has three sink pads and one source pad (pixel port).

I would actually map tvp5150 with just one sink pad, with 3 links
(one for each connector).

In other words, I'm mapping tvp5150 input mux via links, and the
output of its mux as the sink pad.

IMHO, this works a way better than one sink pad per input at its
internal mux.

Regards,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 1/3] [media] v4l2-mc.h: Add a S-Video C input PAD to demod enum

[Hans Verkuil]

On 03/21/2016 06:50 PM, Mauro Carvalho Chehab wrote:
> Hi Hans,
> 
> Em Mon, 21 Mar 2016 17:01:43 +0100
> Hans Verkuil  escreveu:
> 
>>> A reasonable solution to simplify converting legacy drivers without creating
>>> these global ugly pad indices is to add a new video (and probably audio) op
>>> 'g_pad_of_type(type)' where you ask the subdev entity to return which pad 
>>> carries
>>> signals of a certain type.  
>>
>> This basically puts a layer between the low-level pads as defined by the 
>> entity
>> and the 'meta-pads' that a generic MC link creator would need to handle 
>> legacy
>> drivers. The nice thing is that this is wholly inside the kernel so we can
>> modify it at will later without impacting userspace.
> 
> I prepared a long answer to your email, but I guess we're not at the
> same page.
> 
> Let be clear on my view. Please let me know where you disagree:
> 
> 1) I'm not defending Javier's patchset. I have my restrictions to
> it too. My understanding is that he sent this as a RFC for feeding
> our discussions for the media summit.
> 
> Javier, please correct me if I'm wrong.
> 
> 2) I don't understand what you're calling as "meta-pads". For me, a
> PAD is a physical set of pins. 

Poorly worded on my side. I'll elaborate below.

> 3) IMO, the best is to have just one PAD for a decoder input. That makes
> everything simple, yet functional.
> 
> In my view, the input PAD will be linked to several "input connections".
> So, in the case of tvp5150, it will have:
> 
>   - composite 1
>   - composite 2
>   - s-video
> 
> 4) On that view, the input PAD is actually a set of pins. In the
> case of tvp5150, the pins that compose the input PADs are
> AIP1A and AIP1B.
> 
> The output PAD is also a set of pins YOUT0 to YOUT7, plus some other
> pins for sync. Yet, it should, IMHO, have just one output PAD at
> the MC graph.

Indeed. So a tvp5150 has three sink pads and one source pad (pixel port).
Other similar devices may have different numbers of sink pads (say four
composite sinks and no S-Video sinks). So the pads the entity creates
should match what the hardware supports.

So far, so good.

If we want to create code that can more-or-less automatically create a MC
topology for legacy drivers, then we would like to be able to map a high-level
description like 'the first S-Video sink pad' into the actual pad. So you'd
have a 'MAP_PAD_SVID_1' define that, when passed to the g_pad_of_type() op
would return the actual pad index for the first S-Video sink pad (or an error
if there isn't one). That's what I meant with 'meta-pad' (and let's just
forget about that name, poor choice from my side).

What I think Javier's patch did was to require subdevs that have an S-Video pad
to use the DEMOD_PAD_C_INPUT + IF_INPUT pad indices for that. That's really
wrong. The subdev driver decides how many pads there are and which pad is
assigned to which index. That shouldn't be forced on them from the outside
because that won't scale.

But you can make an op that asks 'which pad carries this signal?'. That's fine.

I hope this clarifies matters.

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 1/3] [media] v4l2-mc.h: Add a S-Video C input PAD to demod enum

[Mauro Carvalho Chehab]

Hi Hans,

Em Mon, 21 Mar 2016 17:01:43 +0100
Hans Verkuil  escreveu:

> > A reasonable solution to simplify converting legacy drivers without creating
> > these global ugly pad indices is to add a new video (and probably audio) op
> > 'g_pad_of_type(type)' where you ask the subdev entity to return which pad 
> > carries
> > signals of a certain type.  
> 
> This basically puts a layer between the low-level pads as defined by the 
> entity
> and the 'meta-pads' that a generic MC link creator would need to handle legacy
> drivers. The nice thing is that this is wholly inside the kernel so we can
> modify it at will later without impacting userspace.

I prepared a long answer to your email, but I guess we're not at the
same page.

Let be clear on my view. Please let me know where you disagree:

1) I'm not defending Javier's patchset. I have my restrictions to
it too. My understanding is that he sent this as a RFC for feeding
our discussions for the media summit.

Javier, please correct me if I'm wrong.

2) I don't understand what you're calling as "meta-pads". For me, a
PAD is a physical set of pins. 

3) IMO, the best is to have just one PAD for a decoder input. That makes
everything simple, yet functional.

In my view, the input PAD will be linked to several "input connections".
So, in the case of tvp5150, it will have:

- composite 1
- composite 2
- s-video

4) On that view, the input PAD is actually a set of pins. In the
case of tvp5150, the pins that compose the input PADs are
AIP1A and AIP1B.

The output PAD is also a set of pins YOUT0 to YOUT7, plus some other
pins for sync. Yet, it should, IMHO, have just one output PAD at
the MC graph.

-- 
Thanks,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] dma-buf: Update docs for SYNC ioctl

[Daniel Vetter]

On Mon, Mar 21, 2016 at 01:26:58PM +0100, David Herrmann wrote:
> Hi
> 
> On Mon, Mar 21, 2016 at 8:51 AM, Daniel Vetter  wrote:
> > Just a bit of wording polish plus mentioning that it can fail and must
> > be restarted.
> >
> > Requested by Sumit.
> >
> > v2: Fix them typos (Hans).
> >
> > Cc: Chris Wilson 
> > Cc: Tiago Vignatti 
> > Cc: Stéphane Marchesin 
> > Cc: David Herrmann 
> > Cc: Sumit Semwal 
> > Cc: Daniel Vetter 
> > CC: linux-media@vger.kernel.org
> > Cc: dri-de...@lists.freedesktop.org
> > Cc: linaro-mm-...@lists.linaro.org
> > Cc: intel-...@lists.freedesktop.org
> > Cc: de...@driverdev.osuosl.org
> > Cc: Hans Verkuil 
> > Acked-by: Sumit Semwal 
> > Signed-off-by: Daniel Vetter 
> > ---
> >  Documentation/dma-buf-sharing.txt | 11 ++-
> >  drivers/dma-buf/dma-buf.c |  2 +-
> >  2 files changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/Documentation/dma-buf-sharing.txt 
> > b/Documentation/dma-buf-sharing.txt
> > index 32ac32e773e1..ca44c5820585 100644
> > --- a/Documentation/dma-buf-sharing.txt
> > +++ b/Documentation/dma-buf-sharing.txt
> > @@ -352,7 +352,8 @@ Being able to mmap an export dma-buf buffer object has 
> > 2 main use-cases:
> >
> > No special interfaces, userspace simply calls mmap on the dma-buf fd, 
> > making
> > sure that the cache synchronization ioctl (DMA_BUF_IOCTL_SYNC) is 
> > *always*
> > -   used when the access happens. This is discussed next paragraphs.
> > +   used when the access happens. Note that DMA_BUF_IOCTL_SYNC can fail with
> > +   -EAGAIN or -EINTR, in which case it must be restarted.
> 
> What is "restart on EAGAIN" supposed to mean? Or more generally, what
> does EAGAIN tell the caller?

Do what drmIoctl does essentially.

while (ret == -1 && (errno == EAGAIN || errno == EINTR)
ret = ioctl();

Typed from memery, too lazy to look it up in the source ;-) I'm trying to
sell the idea of a real dma-buf manpage to Sumit, we should clarify this
in detail there.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 1/3] [media] v4l2-mc.h: Add a S-Video C input PAD to demod enum

[Hans Verkuil]

On 03/21/2016 04:05 PM, Hans Verkuil wrote:
> On 03/21/2016 03:40 PM, Mauro Carvalho Chehab wrote:
>> Em Fri, 18 Mar 2016 18:33:39 +0100
>> Hans Verkuil  escreveu:
>>
>>> On 03/18/2016 04:45 PM, Hans Verkuil wrote:
 On 03/09/2016 08:09 PM, Javier Martinez Canillas wrote:  
> The enum demod_pad_index list the PADs that an analog TV demod has but
> in some decoders the S-Video Y (luminance) and C (chrominance) signals
> are carried by different connectors. So a single DEMOD_PAD_IF_INPUT is
> not enough and an additional PAD is needed in the case of S-Video for
> the additional C signal.
>
> Add a DEMOD_PAD_C_INPUT that can be used for this case and the existing
> DEMOD_PAD_IF_INPUT can be used for either Composite or the Y signal.
>
> Suggested-by: Mauro Carvalho Chehab 
> Signed-off-by: Javier Martinez Canillas 
>
> ---
> Hello,
>
> This change was suggested by Mauro in [0] although is still not clear
> if this is the way forward since changing PAD indexes can break the
> uAPI depending on how the PADs are looked up. Another alternative is
> to have a PAD type as Mauro mentioned on the same email but since the
> series are RFC, I'm making this change as an example and hopping that
> the patches can help with the discussion.
>
> [0]: http://www.spinics.net/lists/linux-media/msg98042.html
>
> Best regards,
> Javier
>
>  include/media/v4l2-mc.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/include/media/v4l2-mc.h b/include/media/v4l2-mc.h
> index 98a938aabdfb..47c00c288a06 100644
> --- a/include/media/v4l2-mc.h
> +++ b/include/media/v4l2-mc.h
> @@ -94,7 +94,8 @@ enum if_aud_dec_pad_index {
>   * @DEMOD_NUM_PADS:  Maximum number of output pads.
>   */
>  enum demod_pad_index {
> - DEMOD_PAD_IF_INPUT,
> + DEMOD_PAD_IF_INPUT, /* S-Video Y input or Composite */
> + DEMOD_PAD_C_INPUT,  /* S-Video C input or Composite */
>   DEMOD_PAD_VID_OUT,
>   DEMOD_PAD_VBI_OUT,
>   DEMOD_PAD_AUDIO_OUT,
>  

 These pad index enums are butt ugly and won't scale in the long run. An 
 entity
 should have just as many pads as it needs and no more.

 If you want to have an heuristic so you can find which pad carries e.g.
 composite video, then it is better to ask the subdev for that.

 E.g.: err = v4l2_subdev_call(sd, pad, g_signal_pad, V4L2_PAD_Y_SIG_INPUT, 
 )

 The subdev driver knows which pad has which signal, so this does not rely 
 on
 hardcoding all combinations of possible pad types and you can still 
 heuristically
 build a media graph for legacy drivers.
>>
>> Yes, accessing PADs via a hardcoded index is butt ugly.
>>
>> For sure, we need a better strategy than that. This is one of the things
>> we need to discuss at the media summit.
>>
 What we do now is reminiscent of the bad old days when the input numbers 
 (as
 returned by ENUMINPUT) where mapped to the i2c routing (basically pads). I 
 worked
 hard to get rid of that hardcoded relationship and I don't like to see it 
 coming
 back.
>>
>> No, this is completely unrelated with ENUMINPUT. 
>>
>> With VIDIOC_*INPUT ioctls, a hardcoded list of inputs can happen only at
>> the Kernel side, as, userspace should not rely on the input index, but,
>> instead, should call VIDIOC_ENUMINPUT.
>>
>> However, the media controller currently lacks an "ENUMPADS" ioctl that
>> would tell userspace what kind of data each PAD contains. Due to that,
>> on entities with more than one sink pad and/or more than one source
>> pad, the application should rely on the PAD index.
>>
>> That also reflects on the Kernel side, that forces drivers to do
>> things like:
>>
>>  struct tvp5150 *core = to_tvp5150(sd);
>>  int res;
>>
>>  core->pads[DEMOD_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK;
>>  core->pads[DEMOD_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE;
>>  core->pads[DEMOD_PAD_VBI_OUT].flags = MEDIA_PAD_FL_SOURCE;
>>
>>  res = media_entity_pads_init(>entity, DEMOD_NUM_PADS, core->pads);
>>
>> hardcoding the PAD indexes.
>>
>> The enums that are right now at v4l2-mc.h just prevents the mess to
>> spread all over the drivers, while we don't have a better solution, as
>> at least it will prevent two different devices with the very same type
>> to have a completely different set of PADs, with would cause lots of
>> pain on drivers that work with a multiple set of entities of the same
>> type.
> 
> This is already device specific. The video and audio s_routing ops are there
> precisely because the routing between devices is board specific. It links
> entities with each other the way we had to before we had the media controller.
> 
> Subdev entities should *not* use these fake pads. It's going to be a 
> 

Re: [PATCH 4/7] scatterlist: add sg_alloc_table_from_buf() helper

[Boris Brezillon]

On Tue,  8 Mar 2016 12:15:12 +0100
Boris Brezillon  wrote:

> sg_alloc_table_from_buf() provides an easy solution to create an sg_table
> from a virtual address pointer. This function takes care of dealing with
> vmallocated buffers, buffer alignment, or DMA engine limitations (maximum
> DMA transfer size).
> 
> Signed-off-by: Boris Brezillon 
> ---
>  include/linux/scatterlist.h |  24 
>  lib/scatterlist.c   | 142 
> 
>  2 files changed, 166 insertions(+)
> 
> diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
> index 556ec1e..4a75362 100644
> --- a/include/linux/scatterlist.h
> +++ b/include/linux/scatterlist.h
> @@ -41,6 +41,27 @@ struct sg_table {
>   unsigned int orig_nents;/* original size of list */
>  };
>  
> +/**
> + * struct sg_constraints - SG constraints structure
> + *
> + * @max_chunk_len: maximum chunk buffer length. Each SG entry has to be 
> smaller
> + *  than this value. Zero means no constraint.
> + * @required_alignment: minimum alignment. Is used for both size and pointer
> + *   alignment. If this constraint is not met, the function
> + *   should return -EINVAL.
> + * @preferred_alignment: preferred alignment. Mainly used to optimize
> + *throughput when the DMA engine performs better when
> + *doing aligned accesses.
> + *
> + * This structure is here to help sg_alloc_table_from_buf() create the 
> optimal
> + * SG list based on DMA engine constraints.
> + */
> +struct sg_constraints {
> + size_t max_chunk_len;
> + size_t required_alignment;
> + size_t preferred_alignment;
> +};
> +

Hm, we seem to have another case in NAND controller drivers. Some
drivers do not support scattergather accesses and have to provide a
single physically contiguous memory region to transfer the whole
NAND page.

Could we add a ->max_entries field to the sg_contraints struct to allow
those drivers to use the generic mtd_map_buf() function, and fallback
to a bounce buffer approach (or PIO access approach) when
sg_alloc_table_from_buf() fails to fulfill this constraint.

We could also define another 'non-sg' function to do the 'physically
contiguous' check, but in any case, I'd like to avoid letting each
driver code its own checking function.

What do you think?

>  /*
>   * Notes on SG table design.
>   *
> @@ -265,6 +286,9 @@ int sg_alloc_table_from_pages(struct sg_table *sgt,
>   struct page **pages, unsigned int n_pages,
>   unsigned long offset, unsigned long size,
>   gfp_t gfp_mask);
> +int sg_alloc_table_from_buf(struct sg_table *sgt, const void *buf, size_t 
> len,
> + const struct sg_constraints *constraints,
> + gfp_t gfp_mask);
>  
>  size_t sg_copy_buffer(struct scatterlist *sgl, unsigned int nents, void *buf,
> size_t buflen, off_t skip, bool to_buffer);
> diff --git a/lib/scatterlist.c b/lib/scatterlist.c
> index bafa993..706b583 100644
> --- a/lib/scatterlist.c
> +++ b/lib/scatterlist.c
> @@ -433,6 +433,148 @@ int sg_alloc_table_from_pages(struct sg_table *sgt,
>  }
>  EXPORT_SYMBOL(sg_alloc_table_from_pages);
>  
> +static size_t sg_buf_chunk_len(const void *buf, size_t len,
> +const struct sg_constraints *cons)
> +{
> + size_t chunk_len = len;
> +
> + if (cons->max_chunk_len)
> + chunk_len = min_t(size_t, chunk_len, cons->max_chunk_len);
> +
> + if (is_vmalloc_addr(buf))
> + chunk_len = min_t(size_t, chunk_len,
> +   PAGE_SIZE - offset_in_page(buf));
> +
> + if (!IS_ALIGNED((unsigned long)buf, cons->preferred_alignment)) {
> + const void *aligned_buf = PTR_ALIGN(buf,
> + cons->preferred_alignment);
> + size_t unaligned_len = (unsigned long)(aligned_buf - buf);
> +
> + chunk_len = min_t(size_t, chunk_len, unaligned_len);
> + } else if (chunk_len > cons->preferred_alignment) {
> + chunk_len &= ~(cons->preferred_alignment - 1);
> + }
> +
> + return chunk_len;
> +}
> +
> +#define sg_for_each_chunk_in_buf(buf, len, chunk_len, constraints)   \
> + for (chunk_len = sg_buf_chunk_len(buf, len, constraints);   \
> +  len;   \
> +  len -= chunk_len, buf += chunk_len,\
> +  chunk_len = sg_buf_chunk_len(buf, len, constraints))
> +
> +static int sg_check_constraints(struct sg_constraints *cons,
> + const void *buf, size_t len)
> +{
> + if (!cons->required_alignment)
> + cons->required_alignment = 1;
> +
> + if (!cons->preferred_alignment)
> + cons->preferred_alignment = cons->required_alignment;
> +
> 

Re: [RFC PATCH 1/3] [media] v4l2-mc.h: Add a S-Video C input PAD to demod enum

[Hans Verkuil]

On 03/21/2016 03:40 PM, Mauro Carvalho Chehab wrote:
> Em Fri, 18 Mar 2016 18:33:39 +0100
> Hans Verkuil  escreveu:
> 
>> On 03/18/2016 04:45 PM, Hans Verkuil wrote:
>>> On 03/09/2016 08:09 PM, Javier Martinez Canillas wrote:  
 The enum demod_pad_index list the PADs that an analog TV demod has but
 in some decoders the S-Video Y (luminance) and C (chrominance) signals
 are carried by different connectors. So a single DEMOD_PAD_IF_INPUT is
 not enough and an additional PAD is needed in the case of S-Video for
 the additional C signal.

 Add a DEMOD_PAD_C_INPUT that can be used for this case and the existing
 DEMOD_PAD_IF_INPUT can be used for either Composite or the Y signal.

 Suggested-by: Mauro Carvalho Chehab 
 Signed-off-by: Javier Martinez Canillas 

 ---
 Hello,

 This change was suggested by Mauro in [0] although is still not clear
 if this is the way forward since changing PAD indexes can break the
 uAPI depending on how the PADs are looked up. Another alternative is
 to have a PAD type as Mauro mentioned on the same email but since the
 series are RFC, I'm making this change as an example and hopping that
 the patches can help with the discussion.

 [0]: http://www.spinics.net/lists/linux-media/msg98042.html

 Best regards,
 Javier

  include/media/v4l2-mc.h | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

 diff --git a/include/media/v4l2-mc.h b/include/media/v4l2-mc.h
 index 98a938aabdfb..47c00c288a06 100644
 --- a/include/media/v4l2-mc.h
 +++ b/include/media/v4l2-mc.h
 @@ -94,7 +94,8 @@ enum if_aud_dec_pad_index {
   * @DEMOD_NUM_PADS:   Maximum number of output pads.
   */
  enum demod_pad_index {
 -  DEMOD_PAD_IF_INPUT,
 +  DEMOD_PAD_IF_INPUT, /* S-Video Y input or Composite */
 +  DEMOD_PAD_C_INPUT,  /* S-Video C input or Composite */
DEMOD_PAD_VID_OUT,
DEMOD_PAD_VBI_OUT,
DEMOD_PAD_AUDIO_OUT,
  
>>>
>>> These pad index enums are butt ugly and won't scale in the long run. An 
>>> entity
>>> should have just as many pads as it needs and no more.
>>>
>>> If you want to have an heuristic so you can find which pad carries e.g.
>>> composite video, then it is better to ask the subdev for that.
>>>
>>> E.g.: err = v4l2_subdev_call(sd, pad, g_signal_pad, V4L2_PAD_Y_SIG_INPUT, 
>>> )
>>>
>>> The subdev driver knows which pad has which signal, so this does not rely on
>>> hardcoding all combinations of possible pad types and you can still 
>>> heuristically
>>> build a media graph for legacy drivers.
> 
> Yes, accessing PADs via a hardcoded index is butt ugly.
> 
> For sure, we need a better strategy than that. This is one of the things
> we need to discuss at the media summit.
> 
>>> What we do now is reminiscent of the bad old days when the input numbers (as
>>> returned by ENUMINPUT) where mapped to the i2c routing (basically pads). I 
>>> worked
>>> hard to get rid of that hardcoded relationship and I don't like to see it 
>>> coming
>>> back.
> 
> No, this is completely unrelated with ENUMINPUT. 
> 
> With VIDIOC_*INPUT ioctls, a hardcoded list of inputs can happen only at
> the Kernel side, as, userspace should not rely on the input index, but,
> instead, should call VIDIOC_ENUMINPUT.
> 
> However, the media controller currently lacks an "ENUMPADS" ioctl that
> would tell userspace what kind of data each PAD contains. Due to that,
> on entities with more than one sink pad and/or more than one source
> pad, the application should rely on the PAD index.
> 
> That also reflects on the Kernel side, that forces drivers to do
> things like:
> 
>   struct tvp5150 *core = to_tvp5150(sd);
>   int res;
> 
>   core->pads[DEMOD_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK;
>   core->pads[DEMOD_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE;
>   core->pads[DEMOD_PAD_VBI_OUT].flags = MEDIA_PAD_FL_SOURCE;
> 
>   res = media_entity_pads_init(>entity, DEMOD_NUM_PADS, core->pads);
> 
> hardcoding the PAD indexes.
> 
> The enums that are right now at v4l2-mc.h just prevents the mess to
> spread all over the drivers, while we don't have a better solution, as
> at least it will prevent two different devices with the very same type
> to have a completely different set of PADs, with would cause lots of
> pain on drivers that work with a multiple set of entities of the same
> type.

This is already device specific. The video and audio s_routing ops are there
precisely because the routing between devices is board specific. It links
entities with each other the way we had to before we had the media controller.

Subdev entities should *not* use these fake pads. It's going to be a nightmare.

A reasonable solution to simplify converting legacy drivers without creating
these global ugly pad indices is to add a new video (and probably audio) 

Re: [RFC PATCH 1/3] [media] v4l2-mc.h: Add a S-Video C input PAD to demod enum

[Mauro Carvalho Chehab]

Em Fri, 18 Mar 2016 18:33:39 +0100
Hans Verkuil  escreveu:

> On 03/18/2016 04:45 PM, Hans Verkuil wrote:
> > On 03/09/2016 08:09 PM, Javier Martinez Canillas wrote:  
> >> The enum demod_pad_index list the PADs that an analog TV demod has but
> >> in some decoders the S-Video Y (luminance) and C (chrominance) signals
> >> are carried by different connectors. So a single DEMOD_PAD_IF_INPUT is
> >> not enough and an additional PAD is needed in the case of S-Video for
> >> the additional C signal.
> >>
> >> Add a DEMOD_PAD_C_INPUT that can be used for this case and the existing
> >> DEMOD_PAD_IF_INPUT can be used for either Composite or the Y signal.
> >>
> >> Suggested-by: Mauro Carvalho Chehab 
> >> Signed-off-by: Javier Martinez Canillas 
> >>
> >> ---
> >> Hello,
> >>
> >> This change was suggested by Mauro in [0] although is still not clear
> >> if this is the way forward since changing PAD indexes can break the
> >> uAPI depending on how the PADs are looked up. Another alternative is
> >> to have a PAD type as Mauro mentioned on the same email but since the
> >> series are RFC, I'm making this change as an example and hopping that
> >> the patches can help with the discussion.
> >>
> >> [0]: http://www.spinics.net/lists/linux-media/msg98042.html
> >>
> >> Best regards,
> >> Javier
> >>
> >>  include/media/v4l2-mc.h | 3 ++-
> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/include/media/v4l2-mc.h b/include/media/v4l2-mc.h
> >> index 98a938aabdfb..47c00c288a06 100644
> >> --- a/include/media/v4l2-mc.h
> >> +++ b/include/media/v4l2-mc.h
> >> @@ -94,7 +94,8 @@ enum if_aud_dec_pad_index {
> >>   * @DEMOD_NUM_PADS:   Maximum number of output pads.
> >>   */
> >>  enum demod_pad_index {
> >> -  DEMOD_PAD_IF_INPUT,
> >> +  DEMOD_PAD_IF_INPUT, /* S-Video Y input or Composite */
> >> +  DEMOD_PAD_C_INPUT,  /* S-Video C input or Composite */
> >>DEMOD_PAD_VID_OUT,
> >>DEMOD_PAD_VBI_OUT,
> >>DEMOD_PAD_AUDIO_OUT,
> >>  
> > 
> > These pad index enums are butt ugly and won't scale in the long run. An 
> > entity
> > should have just as many pads as it needs and no more.
> > 
> > If you want to have an heuristic so you can find which pad carries e.g.
> > composite video, then it is better to ask the subdev for that.
> > 
> > E.g.: err = v4l2_subdev_call(sd, pad, g_signal_pad, V4L2_PAD_Y_SIG_INPUT, 
> > )
> > 
> > The subdev driver knows which pad has which signal, so this does not rely on
> > hardcoding all combinations of possible pad types and you can still 
> > heuristically
> > build a media graph for legacy drivers.

Yes, accessing PADs via a hardcoded index is butt ugly.

For sure, we need a better strategy than that. This is one of the things
we need to discuss at the media summit.

> > What we do now is reminiscent of the bad old days when the input numbers (as
> > returned by ENUMINPUT) where mapped to the i2c routing (basically pads). I 
> > worked
> > hard to get rid of that hardcoded relationship and I don't like to see it 
> > coming
> > back.

No, this is completely unrelated with ENUMINPUT. 

With VIDIOC_*INPUT ioctls, a hardcoded list of inputs can happen only at
the Kernel side, as, userspace should not rely on the input index, but,
instead, should call VIDIOC_ENUMINPUT.

However, the media controller currently lacks an "ENUMPADS" ioctl that
would tell userspace what kind of data each PAD contains. Due to that,
on entities with more than one sink pad and/or more than one source
pad, the application should rely on the PAD index.

That also reflects on the Kernel side, that forces drivers to do
things like:

struct tvp5150 *core = to_tvp5150(sd);
int res;

core->pads[DEMOD_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK;
core->pads[DEMOD_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE;
core->pads[DEMOD_PAD_VBI_OUT].flags = MEDIA_PAD_FL_SOURCE;

res = media_entity_pads_init(>entity, DEMOD_NUM_PADS, core->pads);

hardcoding the PAD indexes.

The enums that are right now at v4l2-mc.h just prevents the mess to
spread all over the drivers, while we don't have a better solution, as
at least it will prevent two different devices with the very same type
to have a completely different set of PADs, with would cause lots of
pain on drivers that work with a multiple set of entities of the same
type.

Please notice that this is not a problem with connectors. It is a
broader problem at the MC infra and API, with affects all subdevs.

> > Actually, I am not sure if a new subdev op will work at all. This 
> > information
> > really belongs to the device tree or card info. Just as it is done today 
> > with
> > the audio and video s_routing op. The op basically sets up the routing (i.e.
> > pads). We didn't have pads (or an MC) when I made that op, but the purpose
> > is the same.

I didn't think yet on the implementation, but it should be something that the
core should be 

Re: [PATCH 6/6] drivers/media/dvb-usb-dvb: postpone kfree(mdev)

[kbuild test robot]

Hi Max,

[auto build test WARNING on v4.5-rc7]
[cannot apply to sailus-media/master linuxtv-media/master next-20160321]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improving the system]

url:
https://github.com/0day-ci/linux/commits/Max-Kellermann/drivers-media-dvb-core-en50221-move-code-to-dvb_ca_private_free/20160321-213556
config: xtensa-allyesconfig (attached as .config)
reproduce:
wget 
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
 -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=xtensa 

All warnings (new ones prefixed by >>):

   drivers/media/usb/dvb-usb/dvb-usb-dvb.c: In function 
'dvb_usb_media_device_unregister':
>> drivers/media/usb/dvb-usb/dvb-usb-dvb.c:148:2: warning: ISO C90 forbids 
>> mixed declarations and code [-Wdeclaration-after-statement]
 struct media_device *mdev = adap->dvb_adap.mdev;
 ^

vim +148 drivers/media/usb/dvb-usb/dvb-usb-dvb.c

   132  
   133  static int  dvb_usb_media_device_register(struct dvb_usb_adapter *adap)
   134  {
   135  #ifdef CONFIG_MEDIA_CONTROLLER_DVB
   136  return media_device_register(adap->dvb_adap.mdev);
   137  #else
   138  return 0;
   139  #endif
   140  }
   141  
   142  static void dvb_usb_media_device_unregister(struct dvb_usb_adapter 
*adap)
   143  {
   144  #ifdef CONFIG_MEDIA_CONTROLLER_DVB
   145  if (!adap->dvb_adap.mdev)
   146  return;
   147  
 > 148  struct media_device *mdev = adap->dvb_adap.mdev;
   149  adap->dvb_adap.mdev = NULL;
   150  media_device_unregister(mdev);
   151  /* media_device_cleanup() and kfree() will be called by the
   152 callback function dvb_usb_media_device_release() */
   153  #endif
   154  }
   155  
   156  int dvb_usb_adapter_dvb_init(struct dvb_usb_adapter *adap, short 
*adapter_nums)

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


.config.gz
Description: Binary data

[PATCH v2] fence: add missing descriptions for fence

[Luis de Bethencourt]

The members child_list and active_list were added to the fence struct
without descriptions for the Documentation. Adding these.

Fixes: b55b54b5db33 ("staging/android: remove struct sync_pt")
Signed-off-by: Luis de Bethencourt 
Reviewed-by: Javier Martinez Canillas 
---
Hi,

This second version fixes the commit message as suggested by Javier Martinez.
https://lkml.org/lkml/2016/3/19/135

Thanks,
Luis

 include/linux/fence.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/fence.h b/include/linux/fence.h
index bb52201..ba5b678 100644
--- a/include/linux/fence.h
+++ b/include/linux/fence.h
@@ -49,6 +49,8 @@ struct fence_cb;
  * @timestamp: Timestamp when the fence was signaled.
  * @status: Optional, only valid if < 0, must be set before calling
  * fence_signal, indicates that the fence has completed with an error.
+ * @child_list: list of children fences
+ * @active_list: list of active fences
  *
  * the flags member must be manipulated and read using the appropriate
  * atomic ops (bit_*), so taking the spinlock will not be needed most
-- 
2.6.4

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/6] drivers/media/dvb-core/en50221: move code to dvb_ca_private_free()

[kbuild test robot]

Hi Max,

[auto build test WARNING on v4.5-rc7]
[also build test WARNING on next-20160321]
[cannot apply to sailus-media/master linuxtv-media/master]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improving the system]

url:
https://github.com/0day-ci/linux/commits/Max-Kellermann/drivers-media-dvb-core-en50221-move-code-to-dvb_ca_private_free/20160321-213556
config: xtensa-allyesconfig (attached as .config)
reproduce:
wget 
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
 -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=xtensa 

All warnings (new ones prefixed by >>):

   drivers/media/dvb-core/dvb_ca_en50221.c: In function 'dvb_ca_private_free':
>> drivers/media/dvb-core/dvb_ca_en50221.c:167:2: warning: ISO C90 forbids 
>> mixed declarations and code [-Wdeclaration-after-statement]
 unsigned int i;
 ^

vim +167 drivers/media/dvb-core/dvb_ca_en50221.c

   151  /* Flag indicating the thread should wake up now */
   152  unsigned int wakeup:1;
   153  
   154  /* Delay the main thread should use */
   155  unsigned long delay;
   156  
   157  /* Slot to start looking for data to read from in the next 
user-space read operation */
   158  int next_read_slot;
   159  
   160  /* mutex serializing ioctls */
   161  struct mutex ioctl_mutex;
   162  };
   163  
   164  static void dvb_ca_private_free(struct dvb_ca_private *ca)
   165  {
   166  dvb_unregister_device(ca->dvbdev);
 > 167  unsigned int i;
   168  for (i = 0; i < ca->slot_count; i++) {
   169  vfree(ca->slot_info[i].rx_buffer.data);
   170  }
   171  kfree(ca->slot_info);
   172  kfree(ca);
   173  }
   174  
   175  static void dvb_ca_en50221_thread_wakeup(struct dvb_ca_private *ca);

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


.config.gz
Description: Binary data

Re: [PATCH v2] [media] media-device: use kref for media_device instance

[Shuah Khan]

On 03/21/2016 05:58 AM, Mauro Carvalho Chehab wrote:
> Em Mon, 21 Mar 2016 13:10:33 +0200
> Laurent Pinchart  escreveu:
> 
>> Hi Mauro,
>>
>> Thank you for the patch.
>>
>> On Friday 18 Mar 2016 21:42:16 Mauro Carvalho Chehab wrote:
>>> Now that the media_device can be used by multiple drivers,
>>> via devres, we need to be sure that it will be dropped only
>>> when all drivers stop using it.  
>>
>> I've discussed this with Shuah previously and I'm surprised to see that the 
>> problem hasn't been fixed : using devres for this purpose is just plain 
>> wrong. 
> 
> I didn't follow your discussions with Shuah. I'm pretty sure I didn't
> see any such reply to the /22 patch series. 
> 
> For sure there are other approaches, although I wouldn't say that this
> approach is plain wrong. It was actually suggested by Greg KH at the
> USB summit, back in 2011:
>   https://lkml.org/lkml/2011/8/21/61
> 
> It works fine in the cases like the ones Shuah is currently addressing: 
> USB devices that have multiple interfaces handled by independent drivers.
> 
> Going further, right now, as far as I'm aware of, there are only two use
> cases for a driver-independent media_device struct in the media subsystem
> (on the upstream Kernel):
> 
> - USB devices with USB Audio Class: au0828 and em28xx drivers,
>   plus snd-usb-audio;
> 
> - bt878/bt879 PCI devices, where the DVB driver is independent
>   from the V4L2 one (affects bt87x and bttv drivers).
> 
> The devres approach fits well for both use cases.
> 
> Ok, there are a plenty of OOT SoC drivers that might benefit of some
> other solution, but we should care about them only if/when they got
> upstreamed.
> 
>> The empty media_device_release_devres() function should have given you a 
>> hint.
>>
>> What we need instead is a list of media devices indexed by struct device 
>> (for 
>> this use case) or by struct device_node (for DT use cases). It will both 
>> simplify the code and get rid of the devres abuse.
> 
> Yeah, Shuah's approach should be changed to a different one, in order to
> work for DT use cases. It would be good to have a real DT use case for us
> to validate the solution, before we start implementing something in the
> wild.
> 
> Still, it would very likely need a kref there, in order to destroy the
> media controller struct only after all drivers stop using it.
> 
>> Shuah, if I recall correctly you worked on implementing such a solution 
>> after 
>> our last discussion on the topic. Could you please update us on the status ?
> 
> I saw a Shuah's email proposing to discuss this at the media summit.

Right. Now that the USB devices with USB Audio Class work is in upstream,
I have a bit of breathing room. :)

I have a working solution for the media get api, ready for RFC. I didn't want
to add that in the middle of Media Controller Next Gen and the au0828 and
snd-usb-audio work. We have had several issues we had to address and fix.

I will send RFC patches for the media get API which is close to getting done.

> 
>> In the mean time, let's hold off on this patch, and merge a proper solution 
>> instead.
> 
> Well, we should send a fix for the current issues for Kernel 4.6.

We can't hold off on this patch. Also this patch will make a good base for
the RFC series and changing our devres with media get. We still have the
same issues to address, such as how do we differentiate how the media device
is allocated. This is necessary when it is time to release the media device
in the unregister path.

> 
> As the number of drivers that would be using this internal API is small
> (right now, only 2 drivers), replacing devres by some other strategy
> in the future should be easy.
> 

Yes. Please see above. It is a very simple change out. I have it in the
prototype already.

thanks,
-- Shuah


-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shua...@osg.samsung.com | (970) 217-8978
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/6] drivers/media/dvb-usb-dvb: postpone kfree(mdev)

[kbuild test robot]

Hi Max,

[auto build test ERROR on v4.5-rc7]
[cannot apply to sailus-media/master linuxtv-media/master next-20160321]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improving the system]

url:
https://github.com/0day-ci/linux/commits/Max-Kellermann/drivers-media-dvb-core-en50221-move-code-to-dvb_ca_private_free/20160321-213556
config: x86_64-randconfig-x000-201612 (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/media/usb/dvb-usb/dvb-usb-dvb.c: In function 
'dvb_usb_media_device_release':
>> drivers/media/usb/dvb-usb/dvb-usb-dvb.c:100:2: error: implicit declaration 
>> of function 'media_device_cleanup' [-Werror=implicit-function-declaration]
 media_device_cleanup(mdev);
 ^
   drivers/media/usb/dvb-usb/dvb-usb-dvb.c: At top level:
   drivers/media/usb/dvb-usb/dvb-usb-dvb.c:98:13: warning: 
'dvb_usb_media_device_release' defined but not used [-Wunused-function]
static void dvb_usb_media_device_release(struct media_device *mdev)
^
   In file included from include/uapi/linux/stddef.h:1:0,
from include/linux/stddef.h:4,
from include/uapi/linux/posix_types.h:4,
from include/uapi/linux/types.h:13,
from include/linux/types.h:5,
from include/uapi/linux/sysinfo.h:4,
from include/uapi/linux/kernel.h:4,
from include/linux/cache.h:4,
from include/linux/time.h:4,
from include/linux/input.h:11,
from drivers/media/usb/dvb-usb/dvb-usb.h:13,
from drivers/media/usb/dvb-usb/dvb-usb-common.h:12,
from drivers/media/usb/dvb-usb/dvb-usb-dvb.c:9:
   drivers/media/usb/dvb-usb/dvb-usb-dvb.c: In function 
'dvb_usb_adapter_frontend_init':
   include/linux/compiler.h:147:6: warning: 'ret' may be used uninitialized in 
this function [-Wmaybe-uninitialized]
 if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
 ^
   drivers/media/usb/dvb-usb/dvb-usb-dvb.c:289:6: note: 'ret' was declared here
 int ret, i;
 ^
   cc1: some warnings being treated as errors

vim +/media_device_cleanup +100 drivers/media/usb/dvb-usb/dvb-usb-dvb.c

94  deb_ts("stop pid: 0x%04x, feedtype: %d\n", dvbdmxfeed->pid, 
dvbdmxfeed->type);
95  return dvb_usb_ctrl_feed(dvbdmxfeed, 0);
96  }
97  
98  static void dvb_usb_media_device_release(struct media_device *mdev)
99  {
 > 100  media_device_cleanup(mdev);
   101  kfree(mdev);
   102  }
   103  

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


.config.gz
Description: Binary data

Re: [PATCH v2] [media] media-device: use kref for media_device instance

[Shuah Khan]

On 03/21/2016 05:10 AM, Laurent Pinchart wrote:
> Hi Mauro,
> 
> Thank you for the patch.
> 
> On Friday 18 Mar 2016 21:42:16 Mauro Carvalho Chehab wrote:
>> Now that the media_device can be used by multiple drivers,
>> via devres, we need to be sure that it will be dropped only
>> when all drivers stop using it.
> 
> I've discussed this with Shuah previously and I'm surprised to see that the 
> problem hasn't been fixed : using devres for this purpose is just plain 
> wrong. 
> The empty media_device_release_devres() function should have given you a hint.
> 
> What we need instead is a list of media devices indexed by struct device (for 
> this use case) or by struct device_node (for DT use cases). It will both 
> simplify the code and get rid of the devres abuse.
> 
> Shuah, if I recall correctly you worked on implementing such a solution after 
> our last discussion on the topic. Could you please update us on the status ?
> 

It is work in progress. I have a working prototype for au0828 which is an easier
case. I am working on resolving a couple of issues to differentiate media 
devices
allocated using the global media device list vs. the ones embedded in the driver
data structures. We have many of those.

I had to put this work on the back burner to get the au0882 and snd-usb-audio
wrapped up. I can get the RFC patches on top of the au0882 and snd-usb-audio.
We can discuss them at the upcoming media summit.

> In the mean time, let's hold off on this patch, and merge a proper solution 
> instead.

I think Mauro's restructure helps us with such differentiation and it will be
easy enough to change out devres to get media get API.

thanks,
-- Shuah

-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shua...@osg.samsung.com | (970) 217-8978
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 6/6] drivers/media/dvb-usb-dvb: postpone kfree(mdev)

[Max Kellermann]

Fixes use-after-free bug which occurs when I disconnect my DVB-S
received while VDR is running.

Signed-off-by: Max Kellermann 
---
 drivers/media/usb/dvb-usb/dvb-usb-dvb.c |   14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/media/usb/dvb-usb/dvb-usb-dvb.c 
b/drivers/media/usb/dvb-usb/dvb-usb-dvb.c
index 9ddfcab..7859479 100644
--- a/drivers/media/usb/dvb-usb/dvb-usb-dvb.c
+++ b/drivers/media/usb/dvb-usb/dvb-usb-dvb.c
@@ -95,6 +95,12 @@ static int dvb_usb_stop_feed(struct dvb_demux_feed 
*dvbdmxfeed)
return dvb_usb_ctrl_feed(dvbdmxfeed, 0);
 }
 
+static void dvb_usb_media_device_release(struct media_device *mdev)
+{
+   media_device_cleanup(mdev);
+   kfree(mdev);
+}
+
 static int dvb_usb_media_device_init(struct dvb_usb_adapter *adap)
 {
 #ifdef CONFIG_MEDIA_CONTROLLER_DVB
@@ -113,6 +119,7 @@ static int dvb_usb_media_device_init(struct dvb_usb_adapter 
*adap)
strcpy(mdev->bus_info, udev->devpath);
mdev->hw_revision = le16_to_cpu(udev->descriptor.bcdDevice);
mdev->driver_version = LINUX_VERSION_CODE;
+   mdev->release = dvb_usb_media_device_release;
 
media_device_init(mdev);
 
@@ -138,10 +145,11 @@ static void dvb_usb_media_device_unregister(struct 
dvb_usb_adapter *adap)
if (!adap->dvb_adap.mdev)
return;
 
-   media_device_unregister(adap->dvb_adap.mdev);
-   media_device_cleanup(adap->dvb_adap.mdev);
-   kfree(adap->dvb_adap.mdev);
+   struct media_device *mdev = adap->dvb_adap.mdev;
adap->dvb_adap.mdev = NULL;
+   media_device_unregister(mdev);
+   /* media_device_cleanup() and kfree() will be called by the
+  callback function dvb_usb_media_device_release() */
 #endif
 }
 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/6] drivers/media/media-device: add "release" callback

[Max Kellermann]

Allow the client to free its data structures only after all files have
been closed (fixing use-after-free bugs).

Signed-off-by: Max Kellermann 
---
 drivers/media/media-device.c |9 +++--
 include/media/media-device.h |2 ++
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 5c4669c..a3901f9 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -551,9 +551,14 @@ static DEVICE_ATTR(model, S_IRUGO, show_model, NULL);
  * Registration/unregistration
  */
 
-static void media_device_release(struct media_devnode *mdev)
+static void media_device_release(struct media_devnode *devnode)
 {
-   dev_dbg(mdev->parent, "Media device released\n");
+   struct media_device *mdev = to_media_device(devnode);
+
+   dev_dbg(devnode->parent, "Media device released\n");
+
+   if (mdev->release)
+   mdev->release(mdev);
 }
 
 /**
diff --git a/include/media/media-device.h b/include/media/media-device.h
index d385589..d184d0c 100644
--- a/include/media/media-device.h
+++ b/include/media/media-device.h
@@ -326,6 +326,8 @@ struct media_device {
 
int (*link_notify)(struct media_link *link, u32 flags,
   unsigned int notification);
+
+   void (*release)(struct media_device *mdev);
 };
 
 #ifdef CONFIG_MEDIA_CONTROLLER

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/6] drivers/media/dvb-core/en50221: postpone release until file is closed

[Max Kellermann]

Fixes use-after-free bug which occurs when I disconnect my DVB-S
received while VDR is running.

Signed-off-by: Max Kellermann 
---
 drivers/media/dvb-core/dvb_ca_en50221.c |   23 ++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c 
b/drivers/media/dvb-core/dvb_ca_en50221.c
index e33364c..dfc686a 100644
--- a/drivers/media/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb-core/dvb_ca_en50221.c
@@ -148,6 +148,9 @@ struct dvb_ca_private {
/* Flag indicating if the CA device is open */
unsigned int open:1;
 
+   /* Flag indicating if the CA device is released */
+   unsigned int released:1;
+
/* Flag indicating the thread should wake up now */
unsigned int wakeup:1;
 
@@ -1392,6 +1395,11 @@ static int dvb_ca_en50221_io_read_condition(struct 
dvb_ca_private *ca,
int found = 0;
u8 hdr[2];
 
+   if (ca->released) {
+   *result = -ENODEV;
+   return 1;
+   }
+
slot = ca->next_read_slot;
while ((slot_count < ca->slot_count) && (!found)) {
if (ca->slot_info[slot].slot_state != DVB_CA_SLOTSTATE_RUNNING)
@@ -1595,6 +1603,9 @@ static int dvb_ca_en50221_io_release(struct inode *inode, 
struct file *file)
 
err = dvb_generic_release(inode, file);
 
+   if (ca->released)
+   dvb_ca_private_free(ca);
+
module_put(ca->pub->owner);
 
return err;
@@ -1701,6 +1712,7 @@ int dvb_ca_en50221_init(struct dvb_adapter *dvb_adapter,
}
init_waitqueue_head(>wait_queue);
ca->open = 0;
+   ca->released = 0;
ca->wakeup = 0;
ca->next_read_slot = 0;
pubca->private = ca;
@@ -1765,12 +1777,21 @@ void dvb_ca_en50221_release(struct dvb_ca_en50221 
*pubca)
 
dprintk("%s\n", __func__);
 
+   BUG_ON(ca->released);
+
/* shutdown the thread if there was one */
kthread_stop(ca->thread);
 
for (i = 0; i < ca->slot_count; i++) {
dvb_ca_en50221_slot_shutdown(ca, i);
}
-   dvb_ca_private_free(ca);
+
+   if (ca->open) {
+   ca->released = 1;
+   mb();
+   wake_up_interruptible(>wait_queue);
+   } else
+   dvb_ca_private_free(ca);
+
pubca->private = NULL;
 }

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/6] drivers/media/media-devnode: clear private_data before put_device()

[Max Kellermann]

Callbacks invoked from put_device() may free the struct media_devnode
pointer, so any cleanup needs to be done before put_device().

Signed-off-by: Max Kellermann 
---
 drivers/media/media-devnode.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/media-devnode.c b/drivers/media/media-devnode.c
index 4d7e8dd..e263816 100644
--- a/drivers/media/media-devnode.c
+++ b/drivers/media/media-devnode.c
@@ -196,10 +196,11 @@ static int media_release(struct inode *inode, struct file 
*filp)
if (mdev->fops->release)
mdev->fops->release(filp);
 
+   filp->private_data = NULL;
+
/* decrease the refcount unconditionally since the release()
   return value is ignored. */
put_device(>dev);
-   filp->private_data = NULL;
return 0;
 }
 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/6] drivers/media/media-device: move debug log before _devnode_unregister()

[Max Kellermann]

After media_devnode_unregister(), the struct media_device may be freed
already, and dereferencing it may crash.

Signed-off-by: Max Kellermann 
---
 drivers/media/media-device.c |3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index e9219f5..5c4669c 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -745,9 +745,8 @@ void media_device_unregister(struct media_device *mdev)
spin_unlock(>lock);
 
device_remove_file(>devnode.dev, _attr_model);
+   dev_dbg(mdev->dev, "Media device unregistering\n");
media_devnode_unregister(>devnode);
-
-   dev_dbg(mdev->dev, "Media device unregistered\n");
 }
 EXPORT_SYMBOL_GPL(media_device_unregister);
 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/6] drivers/media/dvb-core/en50221: move code to dvb_ca_private_free()

[Max Kellermann]

Prepare for postponing the call until all file handles have been
closed.

Signed-off-by: Max Kellermann 
---
 drivers/media/dvb-core/dvb_ca_en50221.c |   16 
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c 
b/drivers/media/dvb-core/dvb_ca_en50221.c
index f82cd1f..e33364c 100644
--- a/drivers/media/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb-core/dvb_ca_en50221.c
@@ -161,6 +161,17 @@ struct dvb_ca_private {
struct mutex ioctl_mutex;
 };
 
+static void dvb_ca_private_free(struct dvb_ca_private *ca)
+{
+   dvb_unregister_device(ca->dvbdev);
+   unsigned int i;
+   for (i = 0; i < ca->slot_count; i++) {
+   vfree(ca->slot_info[i].rx_buffer.data);
+   }
+   kfree(ca->slot_info);
+   kfree(ca);
+}
+
 static void dvb_ca_en50221_thread_wakeup(struct dvb_ca_private *ca);
 static int dvb_ca_en50221_read_data(struct dvb_ca_private *ca, int slot, u8 * 
ebuf, int ecount);
 static int dvb_ca_en50221_write_data(struct dvb_ca_private *ca, int slot, u8 * 
ebuf, int ecount);
@@ -1759,10 +1770,7 @@ void dvb_ca_en50221_release(struct dvb_ca_en50221 *pubca)
 
for (i = 0; i < ca->slot_count; i++) {
dvb_ca_en50221_slot_shutdown(ca, i);
-   vfree(ca->slot_info[i].rx_buffer.data);
}
-   kfree(ca->slot_info);
-   dvb_unregister_device(ca->dvbdev);
-   kfree(ca);
+   dvb_ca_private_free(ca);
pubca->private = NULL;
 }

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] dma-buf: Update docs for SYNC ioctl

[Tiago Vignatti]

On 03/21/2016 04:51 AM, Daniel Vetter wrote:

Just a bit of wording polish plus mentioning that it can fail and must
be restarted.

Requested by Sumit.

v2: Fix them typos (Hans).

Cc: Chris Wilson 
Cc: Tiago Vignatti 
Cc: Stéphane Marchesin 
Cc: David Herrmann 
Cc: Sumit Semwal 
Cc: Daniel Vetter 
CC: linux-media@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: linaro-mm-...@lists.linaro.org
Cc: intel-...@lists.freedesktop.org
Cc: de...@driverdev.osuosl.org
Cc: Hans Verkuil 
Acked-by: Sumit Semwal 
Signed-off-by: Daniel Vetter 


Reviewed-by: Tiago Vignatti 

Best regards,

Tiago



---
  Documentation/dma-buf-sharing.txt | 11 ++-
  drivers/dma-buf/dma-buf.c |  2 +-
  2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/Documentation/dma-buf-sharing.txt 
b/Documentation/dma-buf-sharing.txt
index 32ac32e773e1..ca44c5820585 100644
--- a/Documentation/dma-buf-sharing.txt
+++ b/Documentation/dma-buf-sharing.txt
@@ -352,7 +352,8 @@ Being able to mmap an export dma-buf buffer object has 2 
main use-cases:

 No special interfaces, userspace simply calls mmap on the dma-buf fd, 
making
 sure that the cache synchronization ioctl (DMA_BUF_IOCTL_SYNC) is *always*
-   used when the access happens. This is discussed next paragraphs.
+   used when the access happens. Note that DMA_BUF_IOCTL_SYNC can fail with
+   -EAGAIN or -EINTR, in which case it must be restarted.

 Some systems might need some sort of cache coherency management e.g. when
 CPU and GPU domains are being accessed through dma-buf at the same time. To
@@ -366,10 +367,10 @@ Being able to mmap an export dma-buf buffer object has 2 
main use-cases:
 want (with the new data being consumed by the GPU or say scanout 
device)
   - munmap once you don't need the buffer any more

-Therefore, for correctness and optimal performance, systems with the memory
-cache shared by the GPU and CPU i.e. the "coherent" and also the
-"incoherent" are always required to use SYNC_START and SYNC_END before and
-after, respectively, when accessing the mapped address.
+For correctness and optimal performance, it is always required to use
+SYNC_START and SYNC_END before and after, respectively, when accessing the
+mapped address. Userspace cannot rely on coherent access, even when there
+are systems where it just works without calling these ioctls.

  2. Supporting existing mmap interfaces in importers

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 774a60f4309a..4a2c07ee6677 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -612,7 +612,7 @@ EXPORT_SYMBOL_GPL(dma_buf_begin_cpu_access);
   * @dmabuf:   [in]buffer to complete cpu access for.
   * @direction:[in]length of range for cpu access.
   *
- * This call must always succeed.
+ * Can return negative error values, returns 0 on success.
   */
  int dma_buf_end_cpu_access(struct dma_buf *dmabuf,
   enum dma_data_direction direction)



--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] dma-buf,drm,ion: Propagate error code from dma_buf_start_cpu_access()

[Tiago Vignatti]

On 03/18/2016 05:02 PM, Chris Wilson wrote:

Drivers, especially i915.ko, can fail during the initial migration of a
dma-buf for CPU access. However, the error code from the driver was not
being propagated back to ioctl and so userspace was blissfully ignorant
of the failure. Rendering corruption ensues.

Whilst fixing the ioctl to return the error code from
dma_buf_start_cpu_access(), also do the same for
dma_buf_end_cpu_access().  For most drivers, dma_buf_end_cpu_access()
cannot fail. i915.ko however, as most drivers would, wants to avoid being
uninterruptible (as would be required to guarrantee no failure when
flushing the buffer to the device). As userspace already has to handle
errors from the SYNC_IOCTL, take advantage of this to be able to restart
the syscall across signals.

This fixes a coherency issue for i915.ko as well as reducing the
uninterruptible hold upon its BKL, the struct_mutex.

Fixes commit c11e391da2a8fe973c3c2398452000bed505851e
Author: Daniel Vetter 
Date:   Thu Feb 11 20:04:51 2016 -0200

 dma-buf: Add ioctls to allow userspace to flush

Testcase: igt/gem_concurrent_blit/*dmabuf*interruptible
Testcase: igt/prime_mmap_coherency/ioctl-errors
Signed-off-by: Chris Wilson 
Cc: Tiago Vignatti 
Cc: Stéphane Marchesin 
Cc: David Herrmann 
Cc: Sumit Semwal 
Cc: Daniel Vetter 
CC: linux-media@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: linaro-mm-...@lists.linaro.org
Cc: intel-...@lists.freedesktop.org
Cc: de...@driverdev.osuosl.org


Reviewed-by: Tiago Vignatti 

Best regards,

Tiago

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[GIT PULL FOR v4.7] Various fixes, small device_caps enhancement

[Hans Verkuil]

Various fixes.

Note the small enhancement where the device_caps field is added to struct 
video_device.

This means that the core now knows the caps of the device. This will be useful
going forward.

My five patches were originally part of a pull request for 4.6 that never made 
it in
for some reason. Trying again :-)

Regards,

Hans

The following changes since commit b39950960d2b890c21465c69c7c0e4ff6253c6b5:

  [media] media: au0828 fix to clear enable/disable/change source handlers 
(2016-03-18 07:37:22 -0300)

are available in the git repository at:

  git://linuxtv.org/hverkuil/media_tree.git for-v4.7a

for you to fetch changes up to ff0fbc445cab75cba0399d7d553657b2a5a6d4e1:

  m5mols: potential uninitialized variable (2016-03-21 13:34:31 +0100)


Arnd Bergmann (2):
  cobalt: add MTD dependency
  am437x-vfpe: fix typo in vpfe_get_app_input_index

Dan Carpenter (3):
  am437x-vpfe: fix an uninitialized variable bug
  cx23885: uninitialized variable in cx23885_av_work_handler()
  m5mols: potential uninitialized variable

Hans Verkuil (5):
  v4l2: add device_caps to struct video_device
  v4l2-pci-skeleton.c: fill in device_caps in video_device
  vivid: set device_caps in video_device.
  v4l2-ioctl: simplify code
  v4l2-ioctl: improve cropcap handling

Sudip Mukherjee (2):
  cx231xx: fix memory leak
  dw2102: fix unreleased firmware

Tiffany Lin (1):
  media: v4l2-compat-ioctl32: fix missing reserved field copy in 
put_v4l2_create32

 Documentation/video4linux/v4l2-pci-skeleton.c |  5 ++---
 drivers/media/i2c/m5mols/m5mols_controls.c|  2 +-
 drivers/media/pci/cobalt/Kconfig  |  1 +
 drivers/media/pci/cx23885/cx23885-av.c|  2 +-
 drivers/media/platform/am437x/am437x-vpfe.c   |  4 ++--
 drivers/media/platform/vivid/vivid-core.c | 22 ++
 drivers/media/usb/cx231xx/cx231xx-417.c   |  9 
 drivers/media/usb/dvb-usb/dw2102.c|  3 +++
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c |  3 ++-
 drivers/media/v4l2-core/v4l2-ioctl.c  | 81 
+--
 include/media/v4l2-dev.h  |  3 +++
 11 files changed, 86 insertions(+), 49 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] drivers/media/media-devnode: add missing mutex lock in error handler

[Max Kellermann]

All accesses to media_devnode_nums must be protected with
media_devnode_lock.  The error code path in media_devnode_register()
did not do this, however.

Signed-off-by: Max Kellermann 
---
 drivers/media/media-devnode.c |3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/media/media-devnode.c b/drivers/media/media-devnode.c
index cea35bf..4d7e8dd 100644
--- a/drivers/media/media-devnode.c
+++ b/drivers/media/media-devnode.c
@@ -266,8 +266,11 @@ int __must_check media_devnode_register(struct 
media_devnode *mdev,
return 0;
 
 error:
+   mutex_lock(_devnode_lock);
cdev_del(>cdev);
clear_bit(mdev->minor, media_devnode_nums);
+   mutex_unlock(_devnode_lock);
+
return ret;
 }
 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] drivers/media/rc: postpone kfree(rc_dev)

[Max Kellermann]

On 2016/03/21 13:24, Mauro Carvalho Chehab  wrote:
> Please, always send us your Signed-off-by on your patches, as described at:
>   
> https://linuxtv.org/wiki/index.php/Development:_Submitting_Patches#Developer.27s_Certificate_of_Origin_1.1

Sorry, I forgot this, and remembered only right after the patches were
already sent.  I will resend.

Max
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] drivers/media/rc: postpone kfree(rc_dev)

[Max Kellermann]

CONFIG_DEBUG_KOBJECT_RELEASE found this bug.

Signed-off-by: Max Kellermann 
---
 drivers/media/rc/rc-main.c |7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index 1042fa3..cb3e8db 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -1248,6 +1248,9 @@ unlock:
 
 static void rc_dev_release(struct device *device)
 {
+   struct rc_dev *dev = to_rc_dev(device);
+
+   kfree(dev);
 }
 
 #define ADD_HOTPLUG_VAR(fmt, val...)   \
@@ -1369,7 +1372,9 @@ void rc_free_device(struct rc_dev *dev)
 
put_device(>dev);
 
-   kfree(dev);
+   /* kfree(dev) will be called by the callback function
+  rc_dev_release() */
+
module_put(THIS_MODULE);
 }
 EXPORT_SYMBOL_GPL(rc_free_device);

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] dma-buf: Update docs for SYNC ioctl

[David Herrmann]

Hi

On Mon, Mar 21, 2016 at 8:51 AM, Daniel Vetter  wrote:
> Just a bit of wording polish plus mentioning that it can fail and must
> be restarted.
>
> Requested by Sumit.
>
> v2: Fix them typos (Hans).
>
> Cc: Chris Wilson 
> Cc: Tiago Vignatti 
> Cc: Stéphane Marchesin 
> Cc: David Herrmann 
> Cc: Sumit Semwal 
> Cc: Daniel Vetter 
> CC: linux-media@vger.kernel.org
> Cc: dri-de...@lists.freedesktop.org
> Cc: linaro-mm-...@lists.linaro.org
> Cc: intel-...@lists.freedesktop.org
> Cc: de...@driverdev.osuosl.org
> Cc: Hans Verkuil 
> Acked-by: Sumit Semwal 
> Signed-off-by: Daniel Vetter 
> ---
>  Documentation/dma-buf-sharing.txt | 11 ++-
>  drivers/dma-buf/dma-buf.c |  2 +-
>  2 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/dma-buf-sharing.txt 
> b/Documentation/dma-buf-sharing.txt
> index 32ac32e773e1..ca44c5820585 100644
> --- a/Documentation/dma-buf-sharing.txt
> +++ b/Documentation/dma-buf-sharing.txt
> @@ -352,7 +352,8 @@ Being able to mmap an export dma-buf buffer object has 2 
> main use-cases:
>
> No special interfaces, userspace simply calls mmap on the dma-buf fd, 
> making
> sure that the cache synchronization ioctl (DMA_BUF_IOCTL_SYNC) is *always*
> -   used when the access happens. This is discussed next paragraphs.
> +   used when the access happens. Note that DMA_BUF_IOCTL_SYNC can fail with
> +   -EAGAIN or -EINTR, in which case it must be restarted.

What is "restart on EAGAIN" supposed to mean? Or more generally, what
does EAGAIN tell the caller?

Thanks
David

> Some systems might need some sort of cache coherency management e.g. when
> CPU and GPU domains are being accessed through dma-buf at the same time. 
> To
> @@ -366,10 +367,10 @@ Being able to mmap an export dma-buf buffer object has 
> 2 main use-cases:
> want (with the new data being consumed by the GPU or say scanout 
> device)
>   - munmap once you don't need the buffer any more
>
> -Therefore, for correctness and optimal performance, systems with the 
> memory
> -cache shared by the GPU and CPU i.e. the "coherent" and also the
> -"incoherent" are always required to use SYNC_START and SYNC_END before 
> and
> -after, respectively, when accessing the mapped address.
> +For correctness and optimal performance, it is always required to use
> +SYNC_START and SYNC_END before and after, respectively, when accessing 
> the
> +mapped address. Userspace cannot rely on coherent access, even when there
> +are systems where it just works without calling these ioctls.
>
>  2. Supporting existing mmap interfaces in importers
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 774a60f4309a..4a2c07ee6677 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -612,7 +612,7 @@ EXPORT_SYMBOL_GPL(dma_buf_begin_cpu_access);
>   * @dmabuf:[in]buffer to complete cpu access for.
>   * @direction: [in]length of range for cpu access.
>   *
> - * This call must always succeed.
> + * Can return negative error values, returns 0 on success.
>   */
>  int dma_buf_end_cpu_access(struct dma_buf *dmabuf,
>enum dma_data_direction direction)
> --
> 2.8.0.rc3
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] drivers/media/media-devnode: add missing mutex lock in error handler

[Max Kellermann]

Signed-off-by: Max Kellermann 
---
 drivers/media/media-devnode.c |3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/media/media-devnode.c b/drivers/media/media-devnode.c
index cea35bf..4d7e8dd 100644
--- a/drivers/media/media-devnode.c
+++ b/drivers/media/media-devnode.c
@@ -266,8 +266,11 @@ int __must_check media_devnode_register(struct 
media_devnode *mdev,
return 0;
 
 error:
+   mutex_lock(_devnode_lock);
cdev_del(>cdev);
clear_bit(mdev->minor, media_devnode_nums);
+   mutex_unlock(_devnode_lock);
+
return ret;
 }
 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] drivers/media/media-devnode: add missing mutex lock in error handler

[Mauro Carvalho Chehab]

Em Mon, 21 Mar 2016 12:33:12 +0100
Max Kellermann  escreveu:


Please, always send us your Signed-off-by on your patches, as described at:

https://linuxtv.org/wiki/index.php/Development:_Submitting_Patches#Developer.27s_Certificate_of_Origin_1.1

Also, please add a description to the patch.

Thanks,
Mauro

> ---
>  drivers/media/media-devnode.c |3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/media/media-devnode.c b/drivers/media/media-devnode.c
> index cea35bf..4d7e8dd 100644
> --- a/drivers/media/media-devnode.c
> +++ b/drivers/media/media-devnode.c
> @@ -266,8 +266,11 @@ int __must_check media_devnode_register(struct 
> media_devnode *mdev,
>   return 0;
>  
>  error:
> + mutex_lock(_devnode_lock);
>   cdev_del(>cdev);
>   clear_bit(mdev->minor, media_devnode_nums);
> + mutex_unlock(_devnode_lock);
> +
>   return ret;
>  }
>  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


-- 
Thanks,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] drivers/media/rc: postpone kfree(rc_dev)

[Mauro Carvalho Chehab]

Hi Max,

Em Mon, 21 Mar 2016 12:33:05 +0100
Max Kellermann  escreveu:

> CONFIG_DEBUG_KOBJECT_RELEASE found this bug.

Please, always send us your Signed-off-by on your patches, as described at:

https://linuxtv.org/wiki/index.php/Development:_Submitting_Patches#Developer.27s_Certificate_of_Origin_1.1



> ---
>  drivers/media/rc/rc-main.c |7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
> index 1042fa3..cb3e8db 100644
> --- a/drivers/media/rc/rc-main.c
> +++ b/drivers/media/rc/rc-main.c
> @@ -1248,6 +1248,9 @@ unlock:
>  
>  static void rc_dev_release(struct device *device)
>  {
> + struct rc_dev *dev = to_rc_dev(device);
> +
> + kfree(dev);
>  }
>  
>  #define ADD_HOTPLUG_VAR(fmt, val...) \
> @@ -1369,7 +1372,9 @@ void rc_free_device(struct rc_dev *dev)
>  
>   put_device(>dev);
>  
> - kfree(dev);
> + /* kfree(dev) will be called by the callback function
> +rc_dev_release() */
> +
>   module_put(THIS_MODULE);
>  }
>  EXPORT_SYMBOL_GPL(rc_free_device);
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


-- 
Thanks,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] [media] media-device: use kref for media_device instance

[Mauro Carvalho Chehab]

Em Mon, 21 Mar 2016 13:10:33 +0200
Laurent Pinchart  escreveu:

> Hi Mauro,
> 
> Thank you for the patch.
> 
> On Friday 18 Mar 2016 21:42:16 Mauro Carvalho Chehab wrote:
> > Now that the media_device can be used by multiple drivers,
> > via devres, we need to be sure that it will be dropped only
> > when all drivers stop using it.  
> 
> I've discussed this with Shuah previously and I'm surprised to see that the 
> problem hasn't been fixed : using devres for this purpose is just plain 
> wrong. 

I didn't follow your discussions with Shuah. I'm pretty sure I didn't
see any such reply to the /22 patch series. 

For sure there are other approaches, although I wouldn't say that this
approach is plain wrong. It was actually suggested by Greg KH at the
USB summit, back in 2011:
https://lkml.org/lkml/2011/8/21/61

It works fine in the cases like the ones Shuah is currently addressing: 
USB devices that have multiple interfaces handled by independent drivers.

Going further, right now, as far as I'm aware of, there are only two use
cases for a driver-independent media_device struct in the media subsystem
(on the upstream Kernel):

- USB devices with USB Audio Class: au0828 and em28xx drivers,
  plus snd-usb-audio;

- bt878/bt879 PCI devices, where the DVB driver is independent
  from the V4L2 one (affects bt87x and bttv drivers).

The devres approach fits well for both use cases.

Ok, there are a plenty of OOT SoC drivers that might benefit of some
other solution, but we should care about them only if/when they got
upstreamed.

> The empty media_device_release_devres() function should have given you a hint.
> 
> What we need instead is a list of media devices indexed by struct device (for 
> this use case) or by struct device_node (for DT use cases). It will both 
> simplify the code and get rid of the devres abuse.

Yeah, Shuah's approach should be changed to a different one, in order to
work for DT use cases. It would be good to have a real DT use case for us
to validate the solution, before we start implementing something in the
wild.

Still, it would very likely need a kref there, in order to destroy the
media controller struct only after all drivers stop using it.

> Shuah, if I recall correctly you worked on implementing such a solution after 
> our last discussion on the topic. Could you please update us on the status ?

I saw a Shuah's email proposing to discuss this at the media summit.

> In the mean time, let's hold off on this patch, and merge a proper solution 
> instead.

Well, we should send a fix for the current issues for Kernel 4.6.

As the number of drivers that would be using this internal API is small
(right now, only 2 drivers), replacing devres by some other strategy
in the future should be easy.

Regards,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] media: Support Intersil/Techwell TW686x-based video capture cards

[Hans Verkuil]

Hi Ezequiel,

On 03/02/2016 03:30 PM, Ezequiel Garcia wrote:
> This commit introduces the support for the Techwell TW686x video
> capture IC. This hardware supports a few DMA modes, including
> scatter-gather and frame (contiguous).
> 
> This commit makes little use of the DMA engine and instead has
> a memcpy based implementation. DMA frame and scatter-gather modes
> support may be added in the future.
> 
> Currently supported chips:
> - TW6864 (4 video channels),
> - TW6865 (4 video channels, not tested, second generation chip),
> - TW6868 (8 video channels but only 4 first channels using
>built-in video decoder are supported, not tested),
> - TW6869 (8 video channels, second generation chip).
> 
> Cc: Krzysztof Hałasa 
> Signed-off-by: Ezequiel Garcia 

I've tested this with my PCIe tw6869 card (arrived this week).

Video is fine, but only the first audio input is available. I suspect you
have only one audio input?

Regarding the memcpy: if you have a patch for me that reverts back to a 
non-memcpy
situation, then I can do duration tests for you.

Unless you've ordered one of these PCIe cards yourself?

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/4] tw686x-kh: add audio support to the TODO list

[Hans Verkuil]

From: Hans Verkuil 

The mainline tw686x driver also supports audio, that's missing here
as well.

Signed-off-by: Hans Verkuil 
---
 drivers/staging/media/tw686x-kh/TODO | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/media/tw686x-kh/TODO 
b/drivers/staging/media/tw686x-kh/TODO
index f226e80..480a495 100644
--- a/drivers/staging/media/tw686x-kh/TODO
+++ b/drivers/staging/media/tw686x-kh/TODO
@@ -1,3 +1,6 @@
-TODO: implement V4L2_FIELD_INTERLACED* mode(s).
+TODO:
+
+- implement V4L2_FIELD_INTERLACED* mode(s).
+- add audio support
 
 Please Cc: patches to Krzysztof Halasa .
-- 
2.7.0

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/4] tw686x-kh: specify that the DMA is 32 bits

[Hans Verkuil]

From: Hans Verkuil 

Set vb2_queue.gfp_flags to GFP_DMA32. Otherwise it will start to
create bounce buffers which is something you want to avoid since
those are in limited supply.

Signed-off-by: Hans Verkuil 
---
 drivers/staging/media/tw686x-kh/tw686x-kh-video.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/media/tw686x-kh/tw686x-kh-video.c 
b/drivers/staging/media/tw686x-kh/tw686x-kh-video.c
index fb7a784..caf045a 100644
--- a/drivers/staging/media/tw686x-kh/tw686x-kh-video.c
+++ b/drivers/staging/media/tw686x-kh/tw686x-kh-video.c
@@ -766,6 +766,7 @@ int tw686x_video_init(struct tw686x_dev *dev)
vc->vidq.timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
vc->vidq.min_buffers_needed = 2;
vc->vidq.lock = >vb_mutex;
+   vc->vidq.gfp_flags = GFP_DMA32;
 
err = vb2_queue_init(>vidq);
if (err)
-- 
2.7.0

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/4] media: Support Intersil/Techwell TW686x-based video capture cards

[Hans Verkuil]

From: Ezequiel Garcia 

This commit introduces the support for the Techwell TW686x video
capture IC. This hardware supports a few DMA modes, including
scatter-gather and frame (contiguous).

This commit makes little use of the DMA engine and instead has
a memcpy based implementation. DMA frame and scatter-gather modes
support may be added in the future.

Currently supported chips:
- TW6864 (4 video channels),
- TW6865 (4 video channels, not tested, second generation chip),
- TW6868 (8 video channels but only 4 first channels using
   built-in video decoder are supported, not tested),
- TW6869 (8 video channels, second generation chip).

Cc: Krzysztof Halasa 
Signed-off-by: Ezequiel Garcia 
Signed-off-by: Hans Verkuil 
Tested-by: Hans Verkuil 
---
 MAINTAINERS |   8 +
 drivers/media/pci/Kconfig   |   1 +
 drivers/media/pci/Makefile  |   1 +
 drivers/media/pci/tw686x/Kconfig|  18 +
 drivers/media/pci/tw686x/Makefile   |   3 +
 drivers/media/pci/tw686x/tw686x-audio.c | 386 +
 drivers/media/pci/tw686x/tw686x-core.c  | 415 ++
 drivers/media/pci/tw686x/tw686x-regs.h  | 122 +
 drivers/media/pci/tw686x/tw686x-video.c | 927 
 drivers/media/pci/tw686x/tw686x.h   | 158 ++
 10 files changed, 2039 insertions(+)
 create mode 100644 drivers/media/pci/tw686x/Kconfig
 create mode 100644 drivers/media/pci/tw686x/Makefile
 create mode 100644 drivers/media/pci/tw686x/tw686x-audio.c
 create mode 100644 drivers/media/pci/tw686x/tw686x-core.c
 create mode 100644 drivers/media/pci/tw686x/tw686x-regs.h
 create mode 100644 drivers/media/pci/tw686x/tw686x-video.c
 create mode 100644 drivers/media/pci/tw686x/tw686x.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 99bd725..605fd55 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9,6 +9,14 @@ W: https://linuxtv.org
 S: Odd Fixes
 F: drivers/media/pci/tw68/
 
+TW686X VIDEO4LINUX DRIVER
+M: Ezequiel Garcia 
+L: linux-media@vger.kernel.org
+T: git git://linuxtv.org/media_tree.git
+W: http://linuxtv.org
+S: Maintained
+F: drivers/media/pci/tw686x/
+
 TPM DEVICE DRIVER
 M: Peter Huewe 
 M: Marcel Selhorst 
diff --git a/drivers/media/pci/Kconfig b/drivers/media/pci/Kconfig
index 48a611b..4f6467f 100644
--- a/drivers/media/pci/Kconfig
+++ b/drivers/media/pci/Kconfig
@@ -14,6 +14,7 @@ source "drivers/media/pci/meye/Kconfig"
 source "drivers/media/pci/solo6x10/Kconfig"
 source "drivers/media/pci/sta2x11/Kconfig"
 source "drivers/media/pci/tw68/Kconfig"
+source "drivers/media/pci/tw686x/Kconfig"
 source "drivers/media/pci/zoran/Kconfig"
 endif
 
diff --git a/drivers/media/pci/Makefile b/drivers/media/pci/Makefile
index 5f8aacb..2e54c36 100644
--- a/drivers/media/pci/Makefile
+++ b/drivers/media/pci/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_VIDEO_BT848) += bt8xx/
 obj-$(CONFIG_VIDEO_SAA7134) += saa7134/
 obj-$(CONFIG_VIDEO_SAA7164) += saa7164/
 obj-$(CONFIG_VIDEO_TW68) += tw68/
+obj-$(CONFIG_VIDEO_TW686X) += tw686x/
 obj-$(CONFIG_VIDEO_DT3155) += dt3155/
 obj-$(CONFIG_VIDEO_MEYE) += meye/
 obj-$(CONFIG_STA2X11_VIP) += sta2x11/
diff --git a/drivers/media/pci/tw686x/Kconfig b/drivers/media/pci/tw686x/Kconfig
new file mode 100644
index 000..fb85369
--- /dev/null
+++ b/drivers/media/pci/tw686x/Kconfig
@@ -0,0 +1,18 @@
+config VIDEO_TW686X
+   tristate "Intersil/Techwell TW686x video capture cards"
+   depends on PCI && VIDEO_DEV && VIDEO_V4L2 && SND
+   depends on HAS_DMA
+   select VIDEOBUF2_VMALLOC
+   select SND_PCM
+   help
+ Support for Intersil/Techwell TW686x-based frame grabber cards.
+
+ Currently supported chips:
+ - TW6864 (4 video channels),
+ - TW6865 (4 video channels, not tested, second generation chip),
+ - TW6868 (8 video channels but only 4 first channels using
+   built-in video decoder are supported, not tested),
+ - TW6869 (8 video channels, second generation chip).
+
+ To compile this driver as a module, choose M here: the module
+ will be named tw686x.
diff --git a/drivers/media/pci/tw686x/Makefile 
b/drivers/media/pci/tw686x/Makefile
new file mode 100644
index 000..9981954
--- /dev/null
+++ b/drivers/media/pci/tw686x/Makefile
@@ -0,0 +1,3 @@
+tw686x-objs := tw686x-core.o tw686x-video.o tw686x-audio.o
+
+obj-$(CONFIG_VIDEO_TW686X) += tw686x.o
diff --git a/drivers/media/pci/tw686x/tw686x-audio.c 
b/drivers/media/pci/tw686x/tw686x-audio.c
new file mode 100644
index 000..91459ab
--- /dev/null
+++ b/drivers/media/pci/tw686x/tw686x-audio.c
@@ -0,0 +1,386 @@
+/*
+ * Copyright (C) 2015 VanguardiaSur - www.vanguardiasur.com.ar
+ *
+ * Based on the audio support from the tw6869 driver:
+ * Copyright 

[PATCH 0/4] tw686x drivers

[Hans Verkuil]

From: Hans Verkuil 

Even though the two tw686x drivers have been posted before I thought I'd
post them again before I make a pull request due to the fact that I had
to make a few changes to the staging driver and because of the unusual
circumstances.

Krzysztof, I renamed the driver (and sources) to tw686x-kh to prevent
conflicts with the name of Ezequiel's driver.

I also prevent it from being built if VIDEO_TW686X is already selected
so we don't install two drivers for the same hardware.

I also added two patches: the first adds the GFP_DMA32 flag to ensure
the DMA buffers are in 32 bit memory (you probably never tested it on
a 64 bit system). The second mentions adds audio support to the TODO list.

For those who haven't paid attention:

We've ended up with two drivers: one only supports V4L2_FIELD_SEQ_BT,
FIELD_TOP and FIELD_BOTTOM interlaced modes and no audio, the other only
supports FIELD_INTERLACED by way of a memcpy and has audio support.

Part of the reason is weird hardware design that has different DMA modes
depending on the field settings. Krzysztof didn't need FIELD_INTERLACED,
so he never implemented that, and unfortunately when Ezequiel took his
code as starting point he only implemented the FIELD_INTERLACED format.
The memcpy was needed due to unstable DMA when using the DMA mode that
can do FIELD_INTERLACED. It is not known at this time if that unstable
behavior is specific to the hardware Ezequiel is using or if it is
inherent to the tw686x.

In the ideal world both feature sets should be merged into one driver.

But for now I decided to add Ezequiel's driver to the mainline and
Krzysztof's driver to staging. The reason for moving Ezequiel's driver
to mainline is that application support for FIELD_INTERLACED is standard,
whereas FIELD_TOP/BOTTOM/SEQ_BT is pretty rare. In addition, Ezequiel's
driver has audio support.

My hope is that someone will merge the feature sets and we can get rid
of one of the two drivers.

I have tested both drivers with this card:

http://www.nanzoom.com/product/nz-2108E.shtml

(available on ebay)

If there are no comments, then I'll make a pull request, probably next
week.

Regards,

Hans

Ezequiel Garcia (1):
  media: Support Intersil/Techwell TW686x-based video capture cards

Hans Verkuil (2):
  tw686x-kh: specify that the DMA is 32 bits
  tw686x-kh: add audio support to the TODO list

Krzysztof Hałasa (1):
  TW686x frame grabber driver

 MAINTAINERS   |   8 +
 drivers/media/pci/Kconfig |   1 +
 drivers/media/pci/Makefile|   1 +
 drivers/media/pci/tw686x/Kconfig  |  18 +
 drivers/media/pci/tw686x/Makefile |   3 +
 drivers/media/pci/tw686x/tw686x-audio.c   | 386 +
 drivers/media/pci/tw686x/tw686x-core.c| 415 ++
 drivers/media/pci/tw686x/tw686x-regs.h| 122 +++
 drivers/media/pci/tw686x/tw686x-video.c   | 927 ++
 drivers/media/pci/tw686x/tw686x.h | 158 
 drivers/staging/media/Kconfig |   2 +
 drivers/staging/media/Makefile|   1 +
 drivers/staging/media/tw686x-kh/Kconfig   |  17 +
 drivers/staging/media/tw686x-kh/Makefile  |   3 +
 drivers/staging/media/tw686x-kh/TODO  |   6 +
 drivers/staging/media/tw686x-kh/tw686x-kh-core.c  | 140 
 drivers/staging/media/tw686x-kh/tw686x-kh-regs.h  | 103 +++
 drivers/staging/media/tw686x-kh/tw686x-kh-video.c | 821 +++
 drivers/staging/media/tw686x-kh/tw686x-kh.h   | 118 +++
 19 files changed, 3250 insertions(+)
 create mode 100644 drivers/media/pci/tw686x/Kconfig
 create mode 100644 drivers/media/pci/tw686x/Makefile
 create mode 100644 drivers/media/pci/tw686x/tw686x-audio.c
 create mode 100644 drivers/media/pci/tw686x/tw686x-core.c
 create mode 100644 drivers/media/pci/tw686x/tw686x-regs.h
 create mode 100644 drivers/media/pci/tw686x/tw686x-video.c
 create mode 100644 drivers/media/pci/tw686x/tw686x.h
 create mode 100644 drivers/staging/media/tw686x-kh/Kconfig
 create mode 100644 drivers/staging/media/tw686x-kh/Makefile
 create mode 100644 drivers/staging/media/tw686x-kh/TODO
 create mode 100644 drivers/staging/media/tw686x-kh/tw686x-kh-core.c
 create mode 100644 drivers/staging/media/tw686x-kh/tw686x-kh-regs.h
 create mode 100644 drivers/staging/media/tw686x-kh/tw686x-kh-video.c
 create mode 100644 drivers/staging/media/tw686x-kh/tw686x-kh.h

-- 
2.7.0

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/4] TW686x frame grabber driver

[Hans Verkuil]

From: Krzysztof Hałasa 

A driver for Intersil/Techwell TW686x-based PCIe frame grabbers.

Signed-off-by: Krzysztof Halasa 
Signed-off-by: Hans Verkuil 
Tested-by: Hans Verkuil 
[hans.verk...@cisco.com: renamed staging tw686x to tw686x-kh to prevent naming 
conflicts]
[hans.verk...@cisco.com: don't build tw686x-kh if tw686x is already selected to 
prevent conflicts]
---
 drivers/staging/media/Kconfig |   2 +
 drivers/staging/media/Makefile|   1 +
 drivers/staging/media/tw686x-kh/Kconfig   |  17 +
 drivers/staging/media/tw686x-kh/Makefile  |   3 +
 drivers/staging/media/tw686x-kh/TODO  |   3 +
 drivers/staging/media/tw686x-kh/tw686x-kh-core.c  | 140 
 drivers/staging/media/tw686x-kh/tw686x-kh-regs.h  | 103 +++
 drivers/staging/media/tw686x-kh/tw686x-kh-video.c | 820 ++
 drivers/staging/media/tw686x-kh/tw686x-kh.h   | 118 
 9 files changed, 1207 insertions(+)
 create mode 100644 drivers/staging/media/tw686x-kh/Kconfig
 create mode 100644 drivers/staging/media/tw686x-kh/Makefile
 create mode 100644 drivers/staging/media/tw686x-kh/TODO
 create mode 100644 drivers/staging/media/tw686x-kh/tw686x-kh-core.c
 create mode 100644 drivers/staging/media/tw686x-kh/tw686x-kh-regs.h
 create mode 100644 drivers/staging/media/tw686x-kh/tw686x-kh-video.c
 create mode 100644 drivers/staging/media/tw686x-kh/tw686x-kh.h

diff --git a/drivers/staging/media/Kconfig b/drivers/staging/media/Kconfig
index 0078b6a..de7e9f5 100644
--- a/drivers/staging/media/Kconfig
+++ b/drivers/staging/media/Kconfig
@@ -37,6 +37,8 @@ source "drivers/staging/media/omap4iss/Kconfig"
 
 source "drivers/staging/media/timb/Kconfig"
 
+source "drivers/staging/media/tw686x-kh/Kconfig"
+
 # Keep LIRC at the end, as it has sub-menus
 source "drivers/staging/media/lirc/Kconfig"
 
diff --git a/drivers/staging/media/Makefile b/drivers/staging/media/Makefile
index 9149588..60a35b3 100644
--- a/drivers/staging/media/Makefile
+++ b/drivers/staging/media/Makefile
@@ -8,3 +8,4 @@ obj-$(CONFIG_VIDEO_OMAP1)   += omap1/
 obj-$(CONFIG_VIDEO_OMAP4)  += omap4iss/
 obj-$(CONFIG_DVB_MN88472)   += mn88472/
 obj-$(CONFIG_VIDEO_TIMBERDALE)  += timb/
+obj-$(CONFIG_VIDEO_TW686X_KH)  += tw686x-kh/
diff --git a/drivers/staging/media/tw686x-kh/Kconfig 
b/drivers/staging/media/tw686x-kh/Kconfig
new file mode 100644
index 000..6264d30
--- /dev/null
+++ b/drivers/staging/media/tw686x-kh/Kconfig
@@ -0,0 +1,17 @@
+config VIDEO_TW686X_KH
+   tristate "Intersil/Techwell TW686x Video For Linux"
+   depends on VIDEO_DEV && PCI && VIDEO_V4L2
+   depends on !(VIDEO_TW686X=y || VIDEO_TW686X=m) || COMPILE_TEST
+   select VIDEOBUF2_DMA_SG
+   help
+ Support for Intersil/Techwell TW686x-based frame grabber cards.
+
+ Currently supported chips:
+ - TW6864 (4 video channels),
+ - TW6865 (4 video channels, not tested, second generation chip),
+ - TW6868 (8 video channels but only 4 first channels using
+   built-in video decoder are supported, not tested),
+ - TW6869 (8 video channels, second generation chip).
+
+ To compile this driver as a module, choose M here: the module
+ will be named tw686x-kh.
diff --git a/drivers/staging/media/tw686x-kh/Makefile 
b/drivers/staging/media/tw686x-kh/Makefile
new file mode 100644
index 000..2a36a38
--- /dev/null
+++ b/drivers/staging/media/tw686x-kh/Makefile
@@ -0,0 +1,3 @@
+tw686x-kh-objs := tw686x-kh-core.o tw686x-kh-video.o
+
+obj-$(CONFIG_VIDEO_TW686X_KH) += tw686x-kh.o
diff --git a/drivers/staging/media/tw686x-kh/TODO 
b/drivers/staging/media/tw686x-kh/TODO
new file mode 100644
index 000..f226e80
--- /dev/null
+++ b/drivers/staging/media/tw686x-kh/TODO
@@ -0,0 +1,3 @@
+TODO: implement V4L2_FIELD_INTERLACED* mode(s).
+
+Please Cc: patches to Krzysztof Halasa .
diff --git a/drivers/staging/media/tw686x-kh/tw686x-kh-core.c 
b/drivers/staging/media/tw686x-kh/tw686x-kh-core.c
new file mode 100644
index 000..62cccd1
--- /dev/null
+++ b/drivers/staging/media/tw686x-kh/tw686x-kh-core.c
@@ -0,0 +1,140 @@
+/*
+ * Copyright (C) 2015 Industrial Research Institute for Automation
+ * and Measurements PIAP
+ *
+ * Written by Krzysztof Ha?asa.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of version 2 of the GNU General Public License
+ * as published by the Free Software Foundation.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "tw686x-kh.h"
+#include "tw686x-kh-regs.h"
+
+static irqreturn_t tw686x_irq(int irq, void *dev_id)
+{
+   struct tw686x_dev *dev = (struct tw686x_dev *)dev_id;
+   u32 int_status = reg_read(dev, INT_STATUS); /* cleared on read */
+   unsigned long flags;
+   unsigned handled = 0;
+
+   if (int_status) {
+   

[PATCH] drivers/media/media-devnode: add missing mutex lock in error handler

[Max Kellermann]

---
 drivers/media/media-devnode.c |3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/media/media-devnode.c b/drivers/media/media-devnode.c
index cea35bf..4d7e8dd 100644
--- a/drivers/media/media-devnode.c
+++ b/drivers/media/media-devnode.c
@@ -266,8 +266,11 @@ int __must_check media_devnode_register(struct 
media_devnode *mdev,
return 0;
 
 error:
+   mutex_lock(_devnode_lock);
cdev_del(>cdev);
clear_bit(mdev->minor, media_devnode_nums);
+   mutex_unlock(_devnode_lock);
+
return ret;
 }
 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] drivers/media/rc: postpone kfree(rc_dev)

[Max Kellermann]

CONFIG_DEBUG_KOBJECT_RELEASE found this bug.
---
 drivers/media/rc/rc-main.c |7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index 1042fa3..cb3e8db 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -1248,6 +1248,9 @@ unlock:
 
 static void rc_dev_release(struct device *device)
 {
+   struct rc_dev *dev = to_rc_dev(device);
+
+   kfree(dev);
 }
 
 #define ADD_HOTPLUG_VAR(fmt, val...)   \
@@ -1369,7 +1372,9 @@ void rc_free_device(struct rc_dev *dev)
 
put_device(>dev);
 
-   kfree(dev);
+   /* kfree(dev) will be called by the callback function
+  rc_dev_release() */
+
module_put(THIS_MODULE);
 }
 EXPORT_SYMBOL_GPL(rc_free_device);

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] [media] media-device: use kref for media_device instance

[Laurent Pinchart]

Hi Mauro,

Thank you for the patch.

On Friday 18 Mar 2016 21:42:16 Mauro Carvalho Chehab wrote:
> Now that the media_device can be used by multiple drivers,
> via devres, we need to be sure that it will be dropped only
> when all drivers stop using it.

I've discussed this with Shuah previously and I'm surprised to see that the 
problem hasn't been fixed : using devres for this purpose is just plain wrong. 
The empty media_device_release_devres() function should have given you a hint.

What we need instead is a list of media devices indexed by struct device (for 
this use case) or by struct device_node (for DT use cases). It will both 
simplify the code and get rid of the devres abuse.

Shuah, if I recall correctly you worked on implementing such a solution after 
our last discussion on the topic. Could you please update us on the status ?

In the mean time, let's hold off on this patch, and merge a proper solution 
instead.

> Signed-off-by: Mauro Carvalho Chehab 
> ---
> 
> v2: The kref is now used only when media_device is allocated via
> the media_device*_devress. This warrants that other drivers won't be
> affected, and that we can keep media_device_cleanup() balanced with
> media_device_init().
> 
>  drivers/media/media-device.c   | 117 ++
>  drivers/media/usb/au0828/au0828-core.c |   3 +-
>  include/media/media-device.h   |  28 
>  sound/usb/media.c  |   3 +-
>  4 files changed, 118 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index c32fa15cc76e..4a97d92a7e7d 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -707,11 +707,16 @@ void media_device_init(struct media_device *mdev)
>  }
>  EXPORT_SYMBOL_GPL(media_device_init);
> 
> -void media_device_cleanup(struct media_device *mdev)
> +static void __media_device_cleanup(struct media_device *mdev)
>  {
>   ida_destroy(>entity_internal_idx);
>   mdev->entity_internal_idx_max = 0;
>   media_entity_graph_walk_cleanup(>pm_count_walk);
> +}
> +
> +void media_device_cleanup(struct media_device *mdev)
> +{
> + __media_device_cleanup(mdev);
>   mutex_destroy(>graph_mutex);
>  }
>  EXPORT_SYMBOL_GPL(media_device_cleanup);
> @@ -721,6 +726,9 @@ int __must_check __media_device_register(struct
> media_device *mdev, {
>   int ret;
> 
> + /* Check if mdev was ever registered at all */
> + mutex_lock(>graph_mutex);
> +
>   /* Register the device node. */
>   mdev->devnode.fops = _device_fops;
>   mdev->devnode.parent = mdev->dev;
> @@ -731,17 +739,19 @@ int __must_check __media_device_register(struct
> media_device *mdev,
> 
>   ret = media_devnode_register(>devnode, owner);
>   if (ret < 0)
> - return ret;
> + goto err;
> 
>   ret = device_create_file(>devnode.dev, _attr_model);
>   if (ret < 0) {
>   media_devnode_unregister(>devnode);
> - return ret;
> + goto err;
>   }
> 
>   dev_dbg(mdev->dev, "Media device registered\n");
> 
> - return 0;
> +err:
> + mutex_unlock(>graph_mutex);
> + return ret;
>  }
>  EXPORT_SYMBOL_GPL(__media_device_register);
> 
> @@ -773,24 +783,13 @@ void media_device_unregister_entity_notify(struct
> media_device *mdev, }
>  EXPORT_SYMBOL_GPL(media_device_unregister_entity_notify);
> 
> -void media_device_unregister(struct media_device *mdev)
> +static void __media_device_unregister(struct media_device *mdev)
>  {
>   struct media_entity *entity;
>   struct media_entity *next;
>   struct media_interface *intf, *tmp_intf;
>   struct media_entity_notify *notify, *nextp;
> 
> - if (mdev == NULL)
> - return;
> -
> - mutex_lock(>graph_mutex);
> -
> - /* Check if mdev was ever registered at all */
> - if (!media_devnode_is_registered(>devnode)) {
> - mutex_unlock(>graph_mutex);
> - return;
> - }
> -
>   /* Remove all entities from the media device */
>   list_for_each_entry_safe(entity, next, >entities, graph_obj.list)
>   __media_device_unregister_entity(entity);
> @@ -807,38 +806,98 @@ void media_device_unregister(struct media_device
> *mdev) kfree(intf);
>   }
> 
> - mutex_unlock(>graph_mutex);
> -
> - device_remove_file(>devnode.dev, _attr_model);
> - media_devnode_unregister(>devnode);
> + /* Check if mdev devnode was registered */
> + if (media_devnode_is_registered(>devnode)) {
> + device_remove_file(>devnode.dev, _attr_model);
> + media_devnode_unregister(>devnode);
> + }
> 
>   dev_dbg(mdev->dev, "Media device unregistered\n");
>  }
> +
> +void media_device_unregister(struct media_device *mdev)
> +{
> + if (mdev == NULL)
> + return;
> +
> + mutex_lock(>graph_mutex);
> + __media_device_unregister(mdev);
> + 

Re: cron job: media_tree daily build: OK

[Hans Verkuil]

On 03/21/2016 10:57 AM, Mauro Carvalho Chehab wrote:
> Em Mon, 21 Mar 2016 05:05:07 +0100
> "Hans Verkuil"  escreveu:
> 
>> 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:Mon Mar 21 04:00:22 CET 2016
>> git branch:  test
>> git hash:b39950960d2b890c21465c69c7c0e4ff6253c6b5
>> gcc version: i686-linux-gcc (GCC) 5.3.0
>> sparse version:  v0.5.0-56-g7647c77
>> smatch version:  Warning: /share/smatch/smatch_data/ is not accessible.
>> Use --no-data or --data to suppress this message.
>> v0.5.0-3353-gcae47da
> 
> Not sure how you're running smatch, but what I do here is to run
> it as:
>   //smatch
> 
> This way, it finds the smatch_data dir at / suppress several false positive error messages from the smatch logs.
> 
> As a side effect, it simplifies a little bit the procedure to update
> smatch's version.

It was a botched smatch install. It should be OK for the next build.

Thanks for noticing this!

Hans

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: cron job: media_tree daily build: OK

[Mauro Carvalho Chehab]

Em Mon, 21 Mar 2016 06:57:08 -0300
Mauro Carvalho Chehab  escreveu:

> Em Mon, 21 Mar 2016 05:05:07 +0100
> "Hans Verkuil"  escreveu:
> 
> > 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:   Mon Mar 21 04:00:22 CET 2016
> > git branch: test
> > git hash:   b39950960d2b890c21465c69c7c0e4ff6253c6b5
> > gcc version:i686-linux-gcc (GCC) 5.3.0
> > sparse version: v0.5.0-56-g7647c77
> > smatch version: Warning: /share/smatch/smatch_data/ is not accessible.
> > Use --no-data or --data to suppress this message.
> > v0.5.0-3353-gcae47da  
> 
> Not sure how you're running smatch, but what I do here is to run
> it as:
>   //smatch
> 
> This way, it finds the smatch_data dir at / suppress several false positive error messages from the smatch logs.
> 
> As a side effect, it simplifies a little bit the procedure to update
> smatch's version.
> 
> Regards

As a reference, those are what I get here with the latest version of
smatch (plus a patch to avoid a "Function too hairy." warning, posted
in the end of this email):

$ make ARCH=i386  CF=-D__CHECK_ENDIAN__ CONFIG_DEBUG_SECTION_MISMATCH=y C=1 W=1 
CHECK='/devel/smatch/smatch -p=kernel' M=drivers/staging/media
$ make ARCH=i386  CF=-D__CHECK_ENDIAN__ CONFIG_DEBUG_SECTION_MISMATCH=y C=1 W=1 
CHECK='/devel/smatch/smatch -p=kernel' M=drivers/media
drivers/media/tuners/tuner-xc2028.c:1498 xc2028_attach() error: potential null 
dereference 'priv'.  (kzalloc returns null)
drivers/media/radio/radio-aztech.c:87 aztech_alloc() warn: possible memory leak 
of 'az'
drivers/media/pci/mantis/mantis_uart.c:105 mantis_uart_work() warn: this loop 
depends on readl() succeeding
drivers/media/dvb-core/dvb_frontend.c:272 dvb_frontend_get_event() warn: 
inconsistent returns 'sem:>sem'.
  Locked on:   line 246
   line 253
   line 264
   line 272
  Unlocked on: line 261
drivers/media/pci/ddbridge/ddbridge-core.c:1007 input_tasklet() warn: this loop 
depends on readl() succeeding
drivers/media/pci/ddbridge/ddbridge-core.c:1351 flashio() warn: this loop 
depends on readl() succeeding
drivers/media/pci/ddbridge/ddbridge-core.c:1371 flashio() warn: this loop 
depends on readl() succeeding
drivers/media/tuners/tuner-simple.c:1102 simple_tuner_attach() error: potential 
null dereference 'priv'.  (kzalloc returns null)
drivers/media/radio/radio-typhoon.c:79 typhoon_alloc() warn: possible memory 
leak of 'ty'
drivers/media/radio/radio-aimslab.c:73 rtrack_alloc() warn: possible memory 
leak of 'rt'
drivers/media/radio/radio-zoltrix.c:83 zoltrix_alloc() warn: possible memory 
leak of 'zol'
drivers/media/radio/radio-gemtek.c:189 gemtek_alloc() warn: possible memory 
leak of 'gt'
drivers/media/tuners/tda9887.c:692 tda9887_attach() error: potential null 
dereference 'priv'.  (kzalloc returns null)
drivers/media/radio/radio-trust.c:60 trust_alloc() warn: possible memory leak 
of 'tr'
drivers/media/tuners/tda18271-fe.c:1317 tda18271_attach() error: potential null 
dereference 'priv'.  (kzalloc returns null)
drivers/media/tuners/xc5000.c:1418 xc5000_attach() error: potential null 
dereference 'priv'.  (kzalloc returns null)
drivers/media/tuners/xc4000.c:1690 xc4000_attach() error: potential null 
dereference 'priv'.  (kzalloc returns null)
drivers/media/tuners/mxl5007t.c:878 mxl5007t_attach() error: potential null 
dereference 'state'.  (kzalloc returns null)
drivers/media/rc/ir-lirc-codec.c:289 ir_lirc_ioctl() warn: check for integer 
overflow 'val'
drivers/media/tuners/r820t.c:2342 r820t_attach() error: potential null 
dereference 'priv'.  (kzalloc returns null)
drivers/media/rc/st_rc.c:110 st_rc_rx_interrupt() warn: this loop depends on 
readl() succeeding
drivers/media/pci/bt8xx/dvb-bt8xx.c:720 frontend_init() warn: possible memory 
leak of 'state'
./arch/x86/include/asm/bitops.h:416:22: warning: asm output is not an lvalue
./arch/x86/include/asm/bitops.h:416:22: warning: asm output is not an lvalue
./arch/x86/include/asm/bitops.h:457:22: warning: asm output is not an lvalue
./arch/x86/include/asm/bitops.h:457:22: warning: asm output is not an lvalue
./arch/x86/include/asm/bitops.h:457:22: warning: asm output is not an lvalue
./arch/x86/include/asm/bitops.h:457:22: warning: asm output is not an lvalue
./arch/x86/include/asm/bitops.h:457:22: warning: asm output is not an lvalue
./arch/x86/include/asm/bitops.h:457:22: warning: asm output is not an lvalue
drivers/media/usb/uvc/uvc_ctrl.c:1387 uvc_ctrl_begin() warn: inconsistent 
returns 'mutex:>ctrl_mutex'.
  Locked on:   line 1387
  Unlocked on: line 1387
drivers/media/usb/pvrusb2/pvrusb2-hdw.c:3676 pvr2_send_request_ex() error: we 
previously assumed 'write_data' could be null (see line 3648)
drivers/media/usb/pvrusb2/pvrusb2-hdw.c:3829 pvr2_send_request_ex() error: we 

Re: cron job: media_tree daily build: OK

[Mauro Carvalho Chehab]

Em Mon, 21 Mar 2016 05:05:07 +0100
"Hans Verkuil"  escreveu:

> 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: Mon Mar 21 04:00:22 CET 2016
> git branch:   test
> git hash: b39950960d2b890c21465c69c7c0e4ff6253c6b5
> gcc version:  i686-linux-gcc (GCC) 5.3.0
> sparse version:   v0.5.0-56-g7647c77
> smatch version:   Warning: /share/smatch/smatch_data/ is not accessible.
> Use --no-data or --data to suppress this message.
> v0.5.0-3353-gcae47da

Not sure how you're running smatch, but what I do here is to run
it as:
//smatch

This way, it finds the smatch_data dir at /

[PATCHv2 0/2] v4l2-ioctl: cropcap improvements

[Hans Verkuil]

From: Hans Verkuil 

The first patch just simplifies the logic in the code and makes no
functional changes.

The second patch improves the vidioc_cropcap handling with respect to
obtaining the pixel aspect ratio.

It was a bit buggy which I realized after reviewing the new rcar-vin driver.

Regards,

Hans

Changes for v2:

- Added a sanity check for ops->vidioc_cropcap. While it shouldn't be NULL
  (determine_valid_ioctls() guards against that), it is not obvious from the
  code and since determine_valid_ioctls() is in a different source as well
  there is too much chance for someone to mess this up. I feel happier with
  a check in place.

Hans Verkuil (2):
  v4l2-ioctl: simplify code
  v4l2-ioctl: improve cropcap handling

 drivers/media/v4l2-core/v4l2-ioctl.c | 78 
 1 file changed, 52 insertions(+), 26 deletions(-)

-- 
2.7.0

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 1/2] v4l2-ioctl: simplify code

[Hans Verkuil]

From: Hans Verkuil 

Instead of a big if at the beginning, just check if g_selection == NULL
and call the cropcap op immediately and return the result.

No functional changes in this patch.

Signed-off-by: Hans Verkuil 
---
 drivers/media/v4l2-core/v4l2-ioctl.c | 51 
 1 file changed, 29 insertions(+), 22 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c 
b/drivers/media/v4l2-core/v4l2-ioctl.c
index 6bf5a3e..3cf8d3a 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -2160,33 +2160,40 @@ static int v4l_cropcap(const struct v4l2_ioctl_ops *ops,
struct file *file, void *fh, void *arg)
 {
struct v4l2_cropcap *p = arg;
+   struct v4l2_selection s = { .type = p->type };
+   int ret;
 
-   if (ops->vidioc_g_selection) {
-   struct v4l2_selection s = { .type = p->type };
-   int ret;
+   if (ops->vidioc_g_selection == NULL) {
+   /*
+* The determine_valid_ioctls() call already should ensure
+* that ops->vidioc_cropcap != NULL, but just in case...
+*/
+   if (ops->vidioc_cropcap)
+   return ops->vidioc_cropcap(file, fh, p);
+   return -ENOTTY;
+   }
 
-   /* obtaining bounds */
-   if (V4L2_TYPE_IS_OUTPUT(p->type))
-   s.target = V4L2_SEL_TGT_COMPOSE_BOUNDS;
-   else
-   s.target = V4L2_SEL_TGT_CROP_BOUNDS;
+   /* obtaining bounds */
+   if (V4L2_TYPE_IS_OUTPUT(p->type))
+   s.target = V4L2_SEL_TGT_COMPOSE_BOUNDS;
+   else
+   s.target = V4L2_SEL_TGT_CROP_BOUNDS;
 
-   ret = ops->vidioc_g_selection(file, fh, );
-   if (ret)
-   return ret;
-   p->bounds = s.r;
+   ret = ops->vidioc_g_selection(file, fh, );
+   if (ret)
+   return ret;
+   p->bounds = s.r;
 
-   /* obtaining defrect */
-   if (V4L2_TYPE_IS_OUTPUT(p->type))
-   s.target = V4L2_SEL_TGT_COMPOSE_DEFAULT;
-   else
-   s.target = V4L2_SEL_TGT_CROP_DEFAULT;
+   /* obtaining defrect */
+   if (V4L2_TYPE_IS_OUTPUT(p->type))
+   s.target = V4L2_SEL_TGT_COMPOSE_DEFAULT;
+   else
+   s.target = V4L2_SEL_TGT_CROP_DEFAULT;
 
-   ret = ops->vidioc_g_selection(file, fh, );
-   if (ret)
-   return ret;
-   p->defrect = s.r;
-   }
+   ret = ops->vidioc_g_selection(file, fh, );
+   if (ret)
+   return ret;
+   p->defrect = s.r;
 
/* setting trivial pixelaspect */
p->pixelaspect.numerator = 1;
-- 
2.7.0

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 2/2] v4l2-ioctl: improve cropcap handling

[Hans Verkuil]

From: Hans Verkuil 

If cropcap is implemented, then call it first to fill in the pixel
aspect ratio. Don't return if cropcap returns ENOTTY or ENOIOCTLCMD,
in that case just assume a 1:1 pixel aspect ratio.

After cropcap was called, then overwrite bounds and defrect with the
g_selection result because the g_selection() result always has priority
over anything that vidioc_cropcap returns.

Signed-off-by: Hans Verkuil 
---
 drivers/media/v4l2-core/v4l2-ioctl.c | 35 +++
 1 file changed, 27 insertions(+), 8 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c 
b/drivers/media/v4l2-core/v4l2-ioctl.c
index 3cf8d3a..61eb810 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -2161,7 +2161,7 @@ static int v4l_cropcap(const struct v4l2_ioctl_ops *ops,
 {
struct v4l2_cropcap *p = arg;
struct v4l2_selection s = { .type = p->type };
-   int ret;
+   int ret = -ENOTTY;
 
if (ops->vidioc_g_selection == NULL) {
/*
@@ -2173,6 +2173,32 @@ static int v4l_cropcap(const struct v4l2_ioctl_ops *ops,
return -ENOTTY;
}
 
+   /*
+* Let cropcap fill in the pixelaspect if cropcap is available.
+* There is still no other way of obtaining this information.
+*/
+   if (ops->vidioc_cropcap) {
+   ret = ops->vidioc_cropcap(file, fh, p);
+
+   /*
+* If cropcap reports that it isn't implemented,
+* then just keep going.
+*/
+   if (ret && ret != -ENOTTY && ret != -ENOIOCTLCMD)
+   return ret;
+   }
+
+   if (ret) {
+   /*
+* cropcap wasn't implemented, so assume a 1:1 pixel
+* aspect ratio, otherwise keep what cropcap gave us.
+*/
+   p->pixelaspect.numerator = 1;
+   p->pixelaspect.denominator = 1;
+   }
+
+   /* Use g_selection() to fill in the bounds and defrect rectangles */
+
/* obtaining bounds */
if (V4L2_TYPE_IS_OUTPUT(p->type))
s.target = V4L2_SEL_TGT_COMPOSE_BOUNDS;
@@ -2195,13 +2221,6 @@ static int v4l_cropcap(const struct v4l2_ioctl_ops *ops,
return ret;
p->defrect = s.r;
 
-   /* setting trivial pixelaspect */
-   p->pixelaspect.numerator = 1;
-   p->pixelaspect.denominator = 1;
-
-   if (ops->vidioc_cropcap)
-   return ops->vidioc_cropcap(file, fh, p);
-
return 0;
 }
 
-- 
2.7.0

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC 0/2] pxa_camera transition to v4l2 standalone device

[Hans Verkuil]

On 03/19/2016 10:01 PM, Robert Jarzmik wrote:
> Hi Hans and Guennadi,
> 
> As Hans is converting sh_mobile_ceu_camera.c,

That's not going as fast as I hoped. This driver is quite complex and extracting
it from soc-camera isn't easy. I also can't spend as much time as I'd like on 
this.

> let's see how close our ports are
> to see if there are things we could either reuse of change.
> 
> The port is assuming :
>  - the formation translation is transferred into soc_mediabus, so that it can 
> be
>reused across all v4l2 devices

At best this will be a temporary helper source. I never liked soc_mediabus, I 
don't
believe it is the right approach. But I have no problem if it is used for now to
simplify the soc-camera dependency removal.

>  - pxa_camera is ported
> 
> This sets a ground of discussion for soc_camera adherence removal from
> pxa_camera. I'd like to have a comment from Hans if this is what he has in 
> mind,
> and Guennadi if he agrees to transfer the soc xlate stuff to soc_mediabus.

Can you provide the output of 'v4l2-compliance -s' with your new pxa driver?
I would be curious to see the result of that.

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] dma-buf: Update docs for SYNC ioctl

[Hans Verkuil]

On 03/21/2016 08:51 AM, Daniel Vetter wrote:
> Just a bit of wording polish plus mentioning that it can fail and must
> be restarted.
> 
> Requested by Sumit.
> 
> v2: Fix them typos (Hans).
> 
> Cc: Chris Wilson 
> Cc: Tiago Vignatti 
> Cc: Stéphane Marchesin 
> Cc: David Herrmann 
> Cc: Sumit Semwal 
> Cc: Daniel Vetter 
> CC: linux-media@vger.kernel.org
> Cc: dri-de...@lists.freedesktop.org
> Cc: linaro-mm-...@lists.linaro.org
> Cc: intel-...@lists.freedesktop.org
> Cc: de...@driverdev.osuosl.org
> Cc: Hans Verkuil 
> Acked-by: Sumit Semwal 
> Signed-off-by: Daniel Vetter 
> ---
>  Documentation/dma-buf-sharing.txt | 11 ++-
>  drivers/dma-buf/dma-buf.c |  2 +-
>  2 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/dma-buf-sharing.txt 
> b/Documentation/dma-buf-sharing.txt
> index 32ac32e773e1..ca44c5820585 100644
> --- a/Documentation/dma-buf-sharing.txt
> +++ b/Documentation/dma-buf-sharing.txt
> @@ -352,7 +352,8 @@ Being able to mmap an export dma-buf buffer object has 2 
> main use-cases:
>  
> No special interfaces, userspace simply calls mmap on the dma-buf fd, 
> making
> sure that the cache synchronization ioctl (DMA_BUF_IOCTL_SYNC) is *always*
> -   used when the access happens. This is discussed next paragraphs.
> +   used when the access happens. Note that DMA_BUF_IOCTL_SYNC can fail with
> +   -EAGAIN or -EINTR, in which case it must be restarted.
>  
> Some systems might need some sort of cache coherency management e.g. when
> CPU and GPU domains are being accessed through dma-buf at the same time. 
> To
> @@ -366,10 +367,10 @@ Being able to mmap an export dma-buf buffer object has 
> 2 main use-cases:
> want (with the new data being consumed by the GPU or say scanout 
> device)
>   - munmap once you don't need the buffer any more
>  
> -Therefore, for correctness and optimal performance, systems with the 
> memory
> -cache shared by the GPU and CPU i.e. the "coherent" and also the
> -"incoherent" are always required to use SYNC_START and SYNC_END before 
> and
> -after, respectively, when accessing the mapped address.
> +For correctness and optimal performance, it is always required to use
> +SYNC_START and SYNC_END before and after, respectively, when accessing 
> the
> +mapped address. Userspace cannot rely on coherent access, even when there
> +are systems where it just works without calling these ioctls.
>  
>  2. Supporting existing mmap interfaces in importers
>  
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 774a60f4309a..4a2c07ee6677 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -612,7 +612,7 @@ EXPORT_SYMBOL_GPL(dma_buf_begin_cpu_access);
>   * @dmabuf:  [in]buffer to complete cpu access for.
>   * @direction:   [in]length of range for cpu access.
>   *
> - * This call must always succeed.
> + * Can return negative error values, returns 0 on success.
>   */
>  int dma_buf_end_cpu_access(struct dma_buf *dmabuf,
>  enum dma_data_direction direction)
> 

Much better :-)

Acked-by: Hans Verkuil 

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] dma-buf: Update docs for SYNC ioctl

[Daniel Vetter]

Just a bit of wording polish plus mentioning that it can fail and must
be restarted.

Requested by Sumit.

v2: Fix them typos (Hans).

Cc: Chris Wilson 
Cc: Tiago Vignatti 
Cc: Stéphane Marchesin 
Cc: David Herrmann 
Cc: Sumit Semwal 
Cc: Daniel Vetter 
CC: linux-media@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: linaro-mm-...@lists.linaro.org
Cc: intel-...@lists.freedesktop.org
Cc: de...@driverdev.osuosl.org
Cc: Hans Verkuil 
Acked-by: Sumit Semwal 
Signed-off-by: Daniel Vetter 
---
 Documentation/dma-buf-sharing.txt | 11 ++-
 drivers/dma-buf/dma-buf.c |  2 +-
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/Documentation/dma-buf-sharing.txt 
b/Documentation/dma-buf-sharing.txt
index 32ac32e773e1..ca44c5820585 100644
--- a/Documentation/dma-buf-sharing.txt
+++ b/Documentation/dma-buf-sharing.txt
@@ -352,7 +352,8 @@ Being able to mmap an export dma-buf buffer object has 2 
main use-cases:
 
No special interfaces, userspace simply calls mmap on the dma-buf fd, making
sure that the cache synchronization ioctl (DMA_BUF_IOCTL_SYNC) is *always*
-   used when the access happens. This is discussed next paragraphs.
+   used when the access happens. Note that DMA_BUF_IOCTL_SYNC can fail with
+   -EAGAIN or -EINTR, in which case it must be restarted.
 
Some systems might need some sort of cache coherency management e.g. when
CPU and GPU domains are being accessed through dma-buf at the same time. To
@@ -366,10 +367,10 @@ Being able to mmap an export dma-buf buffer object has 2 
main use-cases:
want (with the new data being consumed by the GPU or say scanout device)
  - munmap once you don't need the buffer any more
 
-Therefore, for correctness and optimal performance, systems with the memory
-cache shared by the GPU and CPU i.e. the "coherent" and also the
-"incoherent" are always required to use SYNC_START and SYNC_END before and
-after, respectively, when accessing the mapped address.
+For correctness and optimal performance, it is always required to use
+SYNC_START and SYNC_END before and after, respectively, when accessing the
+mapped address. Userspace cannot rely on coherent access, even when there
+are systems where it just works without calling these ioctls.
 
 2. Supporting existing mmap interfaces in importers
 
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 774a60f4309a..4a2c07ee6677 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -612,7 +612,7 @@ EXPORT_SYMBOL_GPL(dma_buf_begin_cpu_access);
  * @dmabuf:[in]buffer to complete cpu access for.
  * @direction: [in]length of range for cpu access.
  *
- * This call must always succeed.
+ * Can return negative error values, returns 0 on success.
  */
 int dma_buf_end_cpu_access(struct dma_buf *dmabuf,
   enum dma_data_direction direction)
-- 
2.8.0.rc3

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] dma-buf: Update docs for SYNC ioctl

[Daniel Vetter]

On Mon, Mar 21, 2016 at 8:40 AM, Hans Verkuil  wrote:
>> +For correctness and optimal performance, it is always required to use
>> +SYNC_START and SYNC_END before and after, respectively, when accessing 
>> the
>> +mapped address. Userspace cannot on coherent access, even when there are
>
> "Userspace cannot on coherent access"? Do you mean "cannot do"? Sorry, the
> meaning isn't clear to me.

"cannot rely on". I'll send out v2 asap (and let's hope the coffee
works this time around).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] dma-buf: Update docs for SYNC ioctl

[Hans Verkuil]

Hi Daniel,

Two small comments:

On 03/21/2016 08:30 AM, Daniel Vetter wrote:
> Just a bit of wording polish plus mentioning that it can fail and must
> be restarted.
> 
> Requested by Sumit.
> 
> Cc: Chris Wilson 
> Cc: Tiago Vignatti 
> Cc: Stéphane Marchesin 
> Cc: David Herrmann 
> Cc: Sumit Semwal 
> Cc: Daniel Vetter 
> CC: linux-media@vger.kernel.org
> Cc: dri-de...@lists.freedesktop.org
> Cc: linaro-mm-...@lists.linaro.org
> Cc: intel-...@lists.freedesktop.org
> Cc: de...@driverdev.osuosl.org
> Signed-off-by: Daniel Vetter 
> ---
>  Documentation/dma-buf-sharing.txt | 11 ++-
>  drivers/dma-buf/dma-buf.c |  2 +-
>  2 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/dma-buf-sharing.txt 
> b/Documentation/dma-buf-sharing.txt
> index 32ac32e773e1..5c4e3e586ec8 100644
> --- a/Documentation/dma-buf-sharing.txt
> +++ b/Documentation/dma-buf-sharing.txt
> @@ -352,7 +352,8 @@ Being able to mmap an export dma-buf buffer object has 2 
> main use-cases:
>  
> No special interfaces, userspace simply calls mmap on the dma-buf fd, 
> making
> sure that the cache synchronization ioctl (DMA_BUF_IOCTL_SYNC) is *always*
> -   used when the access happens. This is discussed next paragraphs.
> +   used when the access happens. Note that DMA_BUF_IOCTL_SYNC can fail with
> +   -EGAIN or -EINTR, in which case it must be restarted.

EGAIN -> EAGAIN

>  
> Some systems might need some sort of cache coherency management e.g. when
> CPU and GPU domains are being accessed through dma-buf at the same time. 
> To
> @@ -366,10 +367,10 @@ Being able to mmap an export dma-buf buffer object has 
> 2 main use-cases:
> want (with the new data being consumed by the GPU or say scanout 
> device)
>   - munmap once you don't need the buffer any more
>  
> -Therefore, for correctness and optimal performance, systems with the 
> memory
> -cache shared by the GPU and CPU i.e. the "coherent" and also the
> -"incoherent" are always required to use SYNC_START and SYNC_END before 
> and
> -after, respectively, when accessing the mapped address.
> +For correctness and optimal performance, it is always required to use
> +SYNC_START and SYNC_END before and after, respectively, when accessing 
> the
> +mapped address. Userspace cannot on coherent access, even when there are

"Userspace cannot on coherent access"? Do you mean "cannot do"? Sorry, the
meaning isn't clear to me.

Regards,

Hans

> +systems where it just works without calling these ioctls.
>  
>  2. Supporting existing mmap interfaces in importers
>  
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 774a60f4309a..4a2c07ee6677 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -612,7 +612,7 @@ EXPORT_SYMBOL_GPL(dma_buf_begin_cpu_access);
>   * @dmabuf:  [in]buffer to complete cpu access for.
>   * @direction:   [in]length of range for cpu access.
>   *
> - * This call must always succeed.
> + * Can return negative error values, returns 0 on success.
>   */
>  int dma_buf_end_cpu_access(struct dma_buf *dmabuf,
>  enum dma_data_direction direction)
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] dma-buf: Update docs for SYNC ioctl

[Sumit Semwal]

On 21 March 2016 at 13:00, Daniel Vetter  wrote:
> Just a bit of wording polish plus mentioning that it can fail and must
> be restarted.
>
> Requested by Sumit.
>
> Cc: Chris Wilson 
> Cc: Tiago Vignatti 
> Cc: Stéphane Marchesin 
> Cc: David Herrmann 
> Cc: Sumit Semwal 
> Cc: Daniel Vetter 
> CC: linux-media@vger.kernel.org
> Cc: dri-de...@lists.freedesktop.org
> Cc: linaro-mm-...@lists.linaro.org
> Cc: intel-...@lists.freedesktop.org
> Cc: de...@driverdev.osuosl.org
> Signed-off-by: Daniel Vetter 
Acked-by: Sumit Semwal 

> ---
>  Documentation/dma-buf-sharing.txt | 11 ++-
>  drivers/dma-buf/dma-buf.c |  2 +-
>  2 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/dma-buf-sharing.txt 
> b/Documentation/dma-buf-sharing.txt
> index 32ac32e773e1..5c4e3e586ec8 100644
> --- a/Documentation/dma-buf-sharing.txt
> +++ b/Documentation/dma-buf-sharing.txt
> @@ -352,7 +352,8 @@ Being able to mmap an export dma-buf buffer object has 2 
> main use-cases:
>
> No special interfaces, userspace simply calls mmap on the dma-buf fd, 
> making
> sure that the cache synchronization ioctl (DMA_BUF_IOCTL_SYNC) is *always*
> -   used when the access happens. This is discussed next paragraphs.
> +   used when the access happens. Note that DMA_BUF_IOCTL_SYNC can fail with
> +   -EGAIN or -EINTR, in which case it must be restarted.
>
> Some systems might need some sort of cache coherency management e.g. when
> CPU and GPU domains are being accessed through dma-buf at the same time. 
> To
> @@ -366,10 +367,10 @@ Being able to mmap an export dma-buf buffer object has 
> 2 main use-cases:
> want (with the new data being consumed by the GPU or say scanout 
> device)
>   - munmap once you don't need the buffer any more
>
> -Therefore, for correctness and optimal performance, systems with the 
> memory
> -cache shared by the GPU and CPU i.e. the "coherent" and also the
> -"incoherent" are always required to use SYNC_START and SYNC_END before 
> and
> -after, respectively, when accessing the mapped address.
> +For correctness and optimal performance, it is always required to use
> +SYNC_START and SYNC_END before and after, respectively, when accessing 
> the
> +mapped address. Userspace cannot on coherent access, even when there are
> +systems where it just works without calling these ioctls.
>
>  2. Supporting existing mmap interfaces in importers
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 774a60f4309a..4a2c07ee6677 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -612,7 +612,7 @@ EXPORT_SYMBOL_GPL(dma_buf_begin_cpu_access);
>   * @dmabuf:[in]buffer to complete cpu access for.
>   * @direction: [in]length of range for cpu access.
>   *
> - * This call must always succeed.
> + * Can return negative error values, returns 0 on success.
>   */
>  int dma_buf_end_cpu_access(struct dma_buf *dmabuf,
>enum dma_data_direction direction)
> --
> 2.8.0.rc3
>

Best regards,
Sumit.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] dma-buf: Update docs for SYNC ioctl

[Daniel Vetter]

Just a bit of wording polish plus mentioning that it can fail and must
be restarted.

Requested by Sumit.

Cc: Chris Wilson 
Cc: Tiago Vignatti 
Cc: Stéphane Marchesin 
Cc: David Herrmann 
Cc: Sumit Semwal 
Cc: Daniel Vetter 
CC: linux-media@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: linaro-mm-...@lists.linaro.org
Cc: intel-...@lists.freedesktop.org
Cc: de...@driverdev.osuosl.org
Signed-off-by: Daniel Vetter 
---
 Documentation/dma-buf-sharing.txt | 11 ++-
 drivers/dma-buf/dma-buf.c |  2 +-
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/Documentation/dma-buf-sharing.txt 
b/Documentation/dma-buf-sharing.txt
index 32ac32e773e1..5c4e3e586ec8 100644
--- a/Documentation/dma-buf-sharing.txt
+++ b/Documentation/dma-buf-sharing.txt
@@ -352,7 +352,8 @@ Being able to mmap an export dma-buf buffer object has 2 
main use-cases:
 
No special interfaces, userspace simply calls mmap on the dma-buf fd, making
sure that the cache synchronization ioctl (DMA_BUF_IOCTL_SYNC) is *always*
-   used when the access happens. This is discussed next paragraphs.
+   used when the access happens. Note that DMA_BUF_IOCTL_SYNC can fail with
+   -EGAIN or -EINTR, in which case it must be restarted.
 
Some systems might need some sort of cache coherency management e.g. when
CPU and GPU domains are being accessed through dma-buf at the same time. To
@@ -366,10 +367,10 @@ Being able to mmap an export dma-buf buffer object has 2 
main use-cases:
want (with the new data being consumed by the GPU or say scanout device)
  - munmap once you don't need the buffer any more
 
-Therefore, for correctness and optimal performance, systems with the memory
-cache shared by the GPU and CPU i.e. the "coherent" and also the
-"incoherent" are always required to use SYNC_START and SYNC_END before and
-after, respectively, when accessing the mapped address.
+For correctness and optimal performance, it is always required to use
+SYNC_START and SYNC_END before and after, respectively, when accessing the
+mapped address. Userspace cannot on coherent access, even when there are
+systems where it just works without calling these ioctls.
 
 2. Supporting existing mmap interfaces in importers
 
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 774a60f4309a..4a2c07ee6677 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -612,7 +612,7 @@ EXPORT_SYMBOL_GPL(dma_buf_begin_cpu_access);
  * @dmabuf:[in]buffer to complete cpu access for.
  * @direction: [in]length of range for cpu access.
  *
- * This call must always succeed.
+ * Can return negative error values, returns 0 on success.
  */
 int dma_buf_end_cpu_access(struct dma_buf *dmabuf,
   enum dma_data_direction direction)
-- 
2.8.0.rc3

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] media/dvb-core: fix inverted check

[Olli Salonen]

Hi Max,

Already in the tree:
http://git.linuxtv.org/media_tree.git/commit/drivers/media/dvb-core?id=711f3fba6ffd3914fd1b5ed9faf8d22bab6f2203

Cheers,
-olli

On 18 March 2016 at 23:31, Max Kellermann  wrote:
> Breakage caused by commit f50d51661a
>
> Signed-off-by: Max Kellermann 
> ---
>  drivers/media/dvb-core/dvbdev.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
> index 560450a..c756d4b 100644
> --- a/drivers/media/dvb-core/dvbdev.c
> +++ b/drivers/media/dvb-core/dvbdev.c
> @@ -682,7 +682,7 @@ int dvb_create_media_graph(struct dvb_adapter *adap,
> if (demux && ca) {
> ret = media_create_pad_link(demux, 1, ca,
> 0, MEDIA_LNK_FL_ENABLED);
> -   if (!ret)
> +   if (ret)
> return -ENOMEM;
> }
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] dma-buf, drm, ion: Propagate error code from dma_buf_start_cpu_access()

[Sumit Semwal]

On 19 March 2016 at 15:39, Daniel Vetter  wrote:
> On Fri, Mar 18, 2016 at 08:02:39PM +, Chris Wilson wrote:
>> Drivers, especially i915.ko, can fail during the initial migration of a
>> dma-buf for CPU access. However, the error code from the driver was not
>> being propagated back to ioctl and so userspace was blissfully ignorant
>> of the failure. Rendering corruption ensues.
>>
>> Whilst fixing the ioctl to return the error code from
>> dma_buf_start_cpu_access(), also do the same for
>> dma_buf_end_cpu_access().  For most drivers, dma_buf_end_cpu_access()
>> cannot fail. i915.ko however, as most drivers would, wants to avoid being
>> uninterruptible (as would be required to guarrantee no failure when
>> flushing the buffer to the device). As userspace already has to handle
>> errors from the SYNC_IOCTL, take advantage of this to be able to restart
>> the syscall across signals.
>>
>> This fixes a coherency issue for i915.ko as well as reducing the
>> uninterruptible hold upon its BKL, the struct_mutex.
>>
>> Fixes commit c11e391da2a8fe973c3c2398452000bed505851e
>> Author: Daniel Vetter 
>> Date:   Thu Feb 11 20:04:51 2016 -0200
>>
>> dma-buf: Add ioctls to allow userspace to flush
>>
>> Testcase: igt/gem_concurrent_blit/*dmabuf*interruptible
>> Testcase: igt/prime_mmap_coherency/ioctl-errors
>> Signed-off-by: Chris Wilson 
>> Cc: Tiago Vignatti 
>> Cc: Stéphane Marchesin 
>> Cc: David Herrmann 
>> Cc: Sumit Semwal 
>> Cc: Daniel Vetter 
>> CC: linux-media@vger.kernel.org
>> Cc: dri-de...@lists.freedesktop.org
>> Cc: linaro-mm-...@lists.linaro.org
>> Cc: intel-...@lists.freedesktop.org
>> Cc: de...@driverdev.osuosl.org
>
> Applied to drm-misc, I'll send a pull to Dave the next few days if no one
> screams.
> -Daniel
Thanks for pulling it via drm-misc, Daniel.
Chris, I feel since this is an API change, it also needs an update to
the Documentation file.
With that, and a minor nit below, please feel free to add
Acked-by: Sumit Semwal 

>
>> ---
>>  drivers/dma-buf/dma-buf.c | 17 +++--
>>  drivers/gpu/drm/i915/i915_gem_dmabuf.c| 15 +--
>>  drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c |  5 +++--
>>  drivers/gpu/drm/udl/udl_fb.c  |  4 ++--
>>  drivers/staging/android/ion/ion.c |  6 --
>>  include/linux/dma-buf.h   |  6 +++---
>>  6 files changed, 28 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>> index 9810d1df0691..774a60f4309a 100644
>> --- a/drivers/dma-buf/dma-buf.c
>> +++ b/drivers/dma-buf/dma-buf.c
>> @@ -259,6 +259,7 @@ static long dma_buf_ioctl(struct file *file,
>>   struct dma_buf *dmabuf;
>>   struct dma_buf_sync sync;
>>   enum dma_data_direction direction;
>> + int ret;
>>
>>   dmabuf = file->private_data;
>>
>> @@ -285,11 +286,11 @@ static long dma_buf_ioctl(struct file *file,
>>   }
>>
>>   if (sync.flags & DMA_BUF_SYNC_END)
>> - dma_buf_end_cpu_access(dmabuf, direction);
>> + ret = dma_buf_end_cpu_access(dmabuf, direction);
>>   else
>> - dma_buf_begin_cpu_access(dmabuf, direction);
>> + ret = dma_buf_begin_cpu_access(dmabuf, direction);
>>
>> - return 0;
>> + return ret;
>>   default:
>>   return -ENOTTY;
>>   }
>> @@ -613,13 +614,17 @@ EXPORT_SYMBOL_GPL(dma_buf_begin_cpu_access);
>>   *
>>   * This call must always succeed.
>>   */
Perhaps update the above comment to reflect the change as well?

>> -void dma_buf_end_cpu_access(struct dma_buf *dmabuf,
>> - enum dma_data_direction direction)
>> +int dma_buf_end_cpu_access(struct dma_buf *dmabuf,
>> +enum dma_data_direction direction)
>>  {
>> + int ret = 0;
>> +
>>   WARN_ON(!dmabuf);
>>
>>   if (dmabuf->ops->end_cpu_access)
>> - dmabuf->ops->end_cpu_access(dmabuf, direction);
>> + ret = dmabuf->ops->end_cpu_access(dmabuf, direction);
>> +
>> + return ret;
>>  }
<< snip>>

Best regards,
Sumit.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html