Re: [PATCH 00/38] i.MX5/6 Video Capture

[Steve Longerbeam]

Hi Tim,

Pardon late reply, I'm on vacation. See below...


On 06/28/2016 11:54 AM, Tim Harvey wrote:

On Tue, Jun 14, 2016 at 3:48 PM, Steve Longerbeam  wrote:

Tested on imx6q SabreAuto with ADV7180, and imx6q SabreSD with
mipi-csi2 OV5640. There is device-tree support also for imx6qdl
SabreLite, but that is not tested. Also, this driver should
theoretically work on i.MX5 targets, but that is also untested.

Not run through v4l2-compliance yet, but that is in my queue.



Steve,

I've tested this series successfully with a Gateworks Ventana GW5300
which has an IMX6Q and an adv7180 attached to IPU2_CSI1.

First of all, a big 'thank you' for taking the time to rebase and
re-submit this series!

However based on the lack of feedback of the individual patches as
well as the fact they touch varied subsystems I think we are going to
have better luck getting this functionality accepted if you broke it
into separate series.

Here are my thoughts:


   gpu: ipu-v3: Add Video Deinterlacer unit
   gpu: ipu-cpmem: Add ipu_cpmem_set_uv_offset()
   gpu: ipu-cpmem: Add ipu_cpmem_get_burstsize()
   gpu: ipu-v3: Add ipu_get_num()
   gpu: ipu-v3: Add IDMA channel linking support
   gpu: ipu-v3: Add ipu_set_vdi_src_mux()
   gpu: ipu-v3: Add VDI input IDMAC channels
   gpu: ipu-v3: Add ipu_csi_set_src()
   gpu: ipu-v3: Add ipu_ic_set_src()
   gpu: ipu-v3: set correct full sensor frame for PAL/NTSC
   gpu: ipu-v3: Fix CSI data format for 16-bit media bus formats
   gpu: ipu-v3: Fix IRT usage
   gpu: ipu-v3: Fix CSI0 blur in NTSC format
   gpu: ipu-ic: Add complete image conversion support with tiling
   gpu: ipu-ic: allow multiple handles to ic
   gpu: ipu-v3: rename CSI client device

These are all enhancements to the ipu-v3 driver shared by DRM and
maintained by Philipp Zabel and there is no way to 'stage' them.


Just a note here, all these patches to ipu-v3 driver are to the
capture units, and should have no effect on DRM.


Philipp, these have bee submitted previously with little to no changes
or feedback - can we get sign-off or comment on these from you?

Next I would submit the set of drivers that depend on the above into
staging as Hans has said he would accept those (assuming the deps are
accepted). Also, you should submit the drivers first in your series,
then the imx6q.dtsi/imx6qdl.dtsi patches following such as:


   media: imx: Add MIPI CSI-2 Receiver driver
   media: Add camera interface driver for i.MX5/6
   media: Add i.MX5/6 mem2mem driver
   media: imx: Add video switch
   ARM: imx_v6_v7_defconfig: Enable staging video4linux drivers
   ARM: dts: imx6qdl: Add mipi_ipu1/2 video muxes, mipi_csi, and their 
connections
   ARM: dts: imx6qdl: Flesh out MIPI CSI2 receiver node
   ARM: dts: imx6qdl: add mem2mem device for sabre* boards


Ok I will reorder the patches.


I think this last one should add the mem2mem nodes to imx6q.dtsi and
imx6dl.dtsi to be useable by all boards with IPUs right?


Yeah, I'll move the mem2mem nodes to imx6qdl.dtsi and imx6q.dtsi.



After this we have a platform that many imx6 boards with capture
devices can build on.

In parallel, you have a couple of other dependencies before saber*
boards have full capture support that need to through other trees:


   gpio: pca953x: Add reset-gpios property

This should be submitted through the linux-gpio list/subsystem.


I've really got a lot on my plate, I'd appreciate if someone else
could help me out with that.




   clocksource/drivers/imx: add input capture support

Not sure what path this one goes through but it looks to me this patch
adds a feature that only some boards may require (input-capture).


Well, the input-capture support should be usable by any imx6 based
target, and not just for v4l2 subsystem.




   media: imx: Add support for MIPI CSI-2 OV5640
   media: imx: Add support for Parallel OV5642

shouldn't these be subdev drivers?


Well, they _are_ subdev drivers. I guess you mean they should be
moved to drivers/media/i2c? Agreed, at some point they need some
cleaning up and probably consolidated into a single subdev and moved
there.


  Perhaps the can make it into
staging at least in the form you have them now.


   v4l: Add signal lock status to source change events
   media: imx: Add support for ADV7180 Video Decoder

These should be dropped (the 1st is a dependency of the 2nd) and
instead we should be using the subdev driver. I believe you have this
working, and I have been somewhat successful with some of your patches
as well although I still have a 'rolling image' and do not appear to
be getting signal detect interrupts after the first one (which is
likely causing the rolling).


Tim, I've started a new branch 'adv718x' in my mediatree github fork
and moved all the patches to drivers/media/i2c/adv7180.c there. Note
the first commit in that branch!

As I'm currently on vacation and away from the h/w I won't be able to test
this branch with imx6 backend until I return on 7/6. Once the branch is

Re: [PATCH 00/38] i.MX5/6 Video Capture

[Hans Verkuil]

On 06/28/2016 10:10 PM, Hans Verkuil wrote:
> On 06/28/2016 08:54 PM, Tim Harvey wrote:
>> On Tue, Jun 14, 2016 at 3:48 PM, Steve Longerbeam  
>> wrote:
>>> Tested on imx6q SabreAuto with ADV7180, and imx6q SabreSD with
>>> mipi-csi2 OV5640. There is device-tree support also for imx6qdl
>>> SabreLite, but that is not tested. Also, this driver should
>>> theoretically work on i.MX5 targets, but that is also untested.
>>>
>>> Not run through v4l2-compliance yet, but that is in my queue.
>>>
>>>
>>
>> Steve,
>>
>> I've tested this series successfully with a Gateworks Ventana GW5300
>> which has an IMX6Q and an adv7180 attached to IPU2_CSI1.
>>
>> First of all, a big 'thank you' for taking the time to rebase and
>> re-submit this series!
>>
>> However based on the lack of feedback of the individual patches as
> 
> It's on my TODO list, but the series was a lot larger than I expected (and
> touched on a lot of subsystems as well), so I postponed looking at this
> until I have a bit more time.

I scanned through it and the only thing I won't accept in staging is the
adv7180 driver. I also have a question about patch 28, but I'll ask that
separately. The ov5642 is also a duplicate, but the mainline version is tied
to soc_camera, so I have no problem with adding a non-soc-camera driver.

>> well as the fact they touch varied subsystems I think we are going to
>> have better luck getting this functionality accepted if you broke it
>> into separate series.
>>
>> Here are my thoughts:
>>
>>>   gpu: ipu-v3: Add Video Deinterlacer unit
>>>   gpu: ipu-cpmem: Add ipu_cpmem_set_uv_offset()
>>>   gpu: ipu-cpmem: Add ipu_cpmem_get_burstsize()
>>>   gpu: ipu-v3: Add ipu_get_num()
>>>   gpu: ipu-v3: Add IDMA channel linking support
>>>   gpu: ipu-v3: Add ipu_set_vdi_src_mux()
>>>   gpu: ipu-v3: Add VDI input IDMAC channels
>>>   gpu: ipu-v3: Add ipu_csi_set_src()
>>>   gpu: ipu-v3: Add ipu_ic_set_src()
>>>   gpu: ipu-v3: set correct full sensor frame for PAL/NTSC
>>>   gpu: ipu-v3: Fix CSI data format for 16-bit media bus formats
>>>   gpu: ipu-v3: Fix IRT usage
>>>   gpu: ipu-v3: Fix CSI0 blur in NTSC format
>>>   gpu: ipu-ic: Add complete image conversion support with tiling
>>>   gpu: ipu-ic: allow multiple handles to ic
>>>   gpu: ipu-v3: rename CSI client device
>>
>> These are all enhancements to the ipu-v3 driver shared by DRM and
>> maintained by Philipp Zabel and there is no way to 'stage' them.
>> Philipp, these have bee submitted previously with little to no changes
>> or feedback - can we get sign-off or comment on these from you?
> 
> I'd like to know the status of this as well. If this can't go in, then
> accepting the v4l2 driver in staging will likely be very difficult if not
> impossible.

This is the first thing that needs to happen. Philipp, please take a look at 
this!

Thanks,

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 00/38] i.MX5/6 Video Capture

[Hans Verkuil]

On 06/28/2016 08:54 PM, Tim Harvey wrote:
> On Tue, Jun 14, 2016 at 3:48 PM, Steve Longerbeam  
> wrote:
>> Tested on imx6q SabreAuto with ADV7180, and imx6q SabreSD with
>> mipi-csi2 OV5640. There is device-tree support also for imx6qdl
>> SabreLite, but that is not tested. Also, this driver should
>> theoretically work on i.MX5 targets, but that is also untested.
>>
>> Not run through v4l2-compliance yet, but that is in my queue.
>>
>>
> 
> Steve,
> 
> I've tested this series successfully with a Gateworks Ventana GW5300
> which has an IMX6Q and an adv7180 attached to IPU2_CSI1.
> 
> First of all, a big 'thank you' for taking the time to rebase and
> re-submit this series!
> 
> However based on the lack of feedback of the individual patches as

It's on my TODO list, but the series was a lot larger than I expected (and
touched on a lot of subsystems as well), so I postponed looking at this
until I have a bit more time.

> well as the fact they touch varied subsystems I think we are going to
> have better luck getting this functionality accepted if you broke it
> into separate series.
> 
> Here are my thoughts:
> 
>>   gpu: ipu-v3: Add Video Deinterlacer unit
>>   gpu: ipu-cpmem: Add ipu_cpmem_set_uv_offset()
>>   gpu: ipu-cpmem: Add ipu_cpmem_get_burstsize()
>>   gpu: ipu-v3: Add ipu_get_num()
>>   gpu: ipu-v3: Add IDMA channel linking support
>>   gpu: ipu-v3: Add ipu_set_vdi_src_mux()
>>   gpu: ipu-v3: Add VDI input IDMAC channels
>>   gpu: ipu-v3: Add ipu_csi_set_src()
>>   gpu: ipu-v3: Add ipu_ic_set_src()
>>   gpu: ipu-v3: set correct full sensor frame for PAL/NTSC
>>   gpu: ipu-v3: Fix CSI data format for 16-bit media bus formats
>>   gpu: ipu-v3: Fix IRT usage
>>   gpu: ipu-v3: Fix CSI0 blur in NTSC format
>>   gpu: ipu-ic: Add complete image conversion support with tiling
>>   gpu: ipu-ic: allow multiple handles to ic
>>   gpu: ipu-v3: rename CSI client device
> 
> These are all enhancements to the ipu-v3 driver shared by DRM and
> maintained by Philipp Zabel and there is no way to 'stage' them.
> Philipp, these have bee submitted previously with little to no changes
> or feedback - can we get sign-off or comment on these from you?

I'd like to know the status of this as well. If this can't go in, then
accepting the v4l2 driver in staging will likely be very difficult if not
impossible.

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 00/38] i.MX5/6 Video Capture

[Tim Harvey]

On Tue, Jun 14, 2016 at 3:48 PM, Steve Longerbeam  wrote:
> Tested on imx6q SabreAuto with ADV7180, and imx6q SabreSD with
> mipi-csi2 OV5640. There is device-tree support also for imx6qdl
> SabreLite, but that is not tested. Also, this driver should
> theoretically work on i.MX5 targets, but that is also untested.
>
> Not run through v4l2-compliance yet, but that is in my queue.
>
>

Steve,

I've tested this series successfully with a Gateworks Ventana GW5300
which has an IMX6Q and an adv7180 attached to IPU2_CSI1.

First of all, a big 'thank you' for taking the time to rebase and
re-submit this series!

However based on the lack of feedback of the individual patches as
well as the fact they touch varied subsystems I think we are going to
have better luck getting this functionality accepted if you broke it
into separate series.

Here are my thoughts:

>   gpu: ipu-v3: Add Video Deinterlacer unit
>   gpu: ipu-cpmem: Add ipu_cpmem_set_uv_offset()
>   gpu: ipu-cpmem: Add ipu_cpmem_get_burstsize()
>   gpu: ipu-v3: Add ipu_get_num()
>   gpu: ipu-v3: Add IDMA channel linking support
>   gpu: ipu-v3: Add ipu_set_vdi_src_mux()
>   gpu: ipu-v3: Add VDI input IDMAC channels
>   gpu: ipu-v3: Add ipu_csi_set_src()
>   gpu: ipu-v3: Add ipu_ic_set_src()
>   gpu: ipu-v3: set correct full sensor frame for PAL/NTSC
>   gpu: ipu-v3: Fix CSI data format for 16-bit media bus formats
>   gpu: ipu-v3: Fix IRT usage
>   gpu: ipu-v3: Fix CSI0 blur in NTSC format
>   gpu: ipu-ic: Add complete image conversion support with tiling
>   gpu: ipu-ic: allow multiple handles to ic
>   gpu: ipu-v3: rename CSI client device

These are all enhancements to the ipu-v3 driver shared by DRM and
maintained by Philipp Zabel and there is no way to 'stage' them.
Philipp, these have bee submitted previously with little to no changes
or feedback - can we get sign-off or comment on these from you?

Next I would submit the set of drivers that depend on the above into
staging as Hans has said he would accept those (assuming the deps are
accepted). Also, you should submit the drivers first in your series,
then the imx6q.dtsi/imx6qdl.dtsi patches following such as:

>   media: imx: Add MIPI CSI-2 Receiver driver
>   media: Add camera interface driver for i.MX5/6
>   media: Add i.MX5/6 mem2mem driver
>   media: imx: Add video switch
>   ARM: imx_v6_v7_defconfig: Enable staging video4linux drivers
>   ARM: dts: imx6qdl: Add mipi_ipu1/2 video muxes, mipi_csi, and their 
> connections
>   ARM: dts: imx6qdl: Flesh out MIPI CSI2 receiver node
>   ARM: dts: imx6qdl: add mem2mem device for sabre* boards

I think this last one should add the mem2mem nodes to imx6q.dtsi and
imx6dl.dtsi to be useable by all boards with IPUs right?

After this we have a platform that many imx6 boards with capture
devices can build on.

In parallel, you have a couple of other dependencies before saber*
boards have full capture support that need to through other trees:

>   gpio: pca953x: Add reset-gpios property

This should be submitted through the linux-gpio list/subsystem.

>   clocksource/drivers/imx: add input capture support

Not sure what path this one goes through but it looks to me this patch
adds a feature that only some boards may require (input-capture).

>   media: imx: Add support for MIPI CSI-2 OV5640
>   media: imx: Add support for Parallel OV5642

shouldn't these be subdev drivers? Perhaps the can make it into
staging at least in the form you have them now.

>   v4l: Add signal lock status to source change events
>   media: imx: Add support for ADV7180 Video Decoder

These should be dropped (the 1st is a dependency of the 2nd) and
instead we should be using the subdev driver. I believe you have this
working, and I have been somewhat successful with some of your patches
as well although I still have a 'rolling image' and do not appear to
be getting signal detect interrupts after the first one (which is
likely causing the rolling).

>   media: adv7180: add power pin control
>   media: adv7180: implement g_parm

These seem very reasonable and indeed go to linux-media but perhaps
should be split out from the imx6 patchset to be able to get more
attention on them?

>   ARM: dts: imx6-sabrelite: add video capture ports and connections
>   ARM: dts: imx6-sabresd: add video capture ports and connections
>   ARM: dts: imx6-sabreauto: create i2cmux for i2c3
>   ARM: dts: imx6-sabreauto: add reset-gpios property for max7310
>   ARM: dts: imx6-sabreauto: add pinctrl for gpt input capture
>   ARM: dts: imx6-sabreauto: add video capture ports and connections

These should probably be last in a series on their own as there are
several dependencies within them for things that need to take
alternate submission paths.

Regards,

Tim
--
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 00/38] i.MX5/6 Video Capture

[Hans Verkuil]

On 06/17/2016 07:35 PM, Steve Longerbeam wrote:
> 
> 
> On 06/17/2016 12:10 AM, Hans Verkuil wrote:
>> On 06/16/2016 07:02 PM, Steve Longerbeam wrote:
>>> On 06/16/2016 02:49 AM, Jack Mitchell wrote:
 On 16/06/16 02:37, Steve Longerbeam wrote:
> Hi Jack,
>
> On 06/15/2016 03:43 AM, Jack Mitchell wrote:
>> 
>> Trying to use a user pointer rather than mmap also fails and causes a 
>> kernel splat.
>>
> Hmm, I've tested userptr with the mem2mem driver, but maybe never
> with video capture. I tried "v4l2-ctl -d/dev/video0 --stream-user=8" but
> that returns "VIDIOC_QBUF: failed: Invalid argument", haven't tracked
> down why (could be a bug in v4l2-ctl). Can you share the splat?
>
 On re-checking the splat was the same v4l_cropcap that was mentioned 
 before so I don't think it's related. The error I get back is:

 VIDIOC_QBUF error 22, Invalid argument

 I'm using the example program the the v4l2 docs [1].
>>> I found the cause at least in my case. After enabling dynamic debug in
>>> videobuf2-dma-contig.c, "v4l2-ctl -d/dev/video0 --stream-user=8" gives
>>> me
>>>
>>> [  468.826046] user data must be aligned to 64 bytes
>>>
>>>
>>>
>>> But even getting past that alignment issue, I've only tested userptr (in 
>>> mem2mem
>>> driver) by giving the driver a user address of a mmap'ed kernel contiguous
>>> buffer. A true discontiguous user buffer may not work, the IPU DMA does not
>>> support scatter-gather.
>> I don't think VB2_USERPTR should be enabled in this case due to the DMA 
>> limitations.
>> It won't allow you to use malloc()ed memory and the hack that allows you to 
>> pass
>> contiguous memory is superseded by the DMABUF mode.
> 
> Hi Hans, yes, I was going to suggest that. I will remove USERPTR
> from both capture and mem2mem io_modes flags. Although I can
> see where userptr support is still useful, that is for legacy middleware
> that are still using mmap'ed userptrs and have not yet converted to
> passing around dmabuf fd's.
> 
> But I've been perplexed for while on this, why vb2_dc_get_userptr() 
> resorts to
> scatter-gather (when the given user buffer is found not to have valid 
> pfn's).
> Shouldn't it be assumed that driver users of the vb2 dma-contig allocator
> only support contiguous dma as the name implies? Maybe the issue is that
> users that set VB2_USERPTR are implying they support scatter/gather, but
> users of vb2-dma-contig could also be implying they do not, and would be
> using vb2-dma-sg if that were the case.

At the end of the vb2_dc_get_userptr() function it checks if the result
is physically contiguous (the call to vb2_dc_get_contiguous_size). If not,
then it returns an error.

I hate it that dma_contig accepts user pointers at all, it should never
have been implemented like that. There is no way for applications to know
that even though the driver support USERPTR, the buffer still has to be
phys. contig. :-(

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 00/38] i.MX5/6 Video Capture

[Steve Longerbeam]

On 06/17/2016 12:10 AM, Hans Verkuil wrote:

On 06/16/2016 07:02 PM, Steve Longerbeam wrote:

On 06/16/2016 02:49 AM, Jack Mitchell wrote:

On 16/06/16 02:37, Steve Longerbeam wrote:

Hi Jack,

On 06/15/2016 03:43 AM, Jack Mitchell wrote:


Trying to use a user pointer rather than mmap also fails and causes a kernel 
splat.


Hmm, I've tested userptr with the mem2mem driver, but maybe never
with video capture. I tried "v4l2-ctl -d/dev/video0 --stream-user=8" but
that returns "VIDIOC_QBUF: failed: Invalid argument", haven't tracked
down why (could be a bug in v4l2-ctl). Can you share the splat?


On re-checking the splat was the same v4l_cropcap that was mentioned before so 
I don't think it's related. The error I get back is:

VIDIOC_QBUF error 22, Invalid argument

I'm using the example program the the v4l2 docs [1].

I found the cause at least in my case. After enabling dynamic debug in
videobuf2-dma-contig.c, "v4l2-ctl -d/dev/video0 --stream-user=8" gives
me

[  468.826046] user data must be aligned to 64 bytes



But even getting past that alignment issue, I've only tested userptr (in mem2mem
driver) by giving the driver a user address of a mmap'ed kernel contiguous
buffer. A true discontiguous user buffer may not work, the IPU DMA does not
support scatter-gather.

I don't think VB2_USERPTR should be enabled in this case due to the DMA 
limitations.
It won't allow you to use malloc()ed memory and the hack that allows you to pass
contiguous memory is superseded by the DMABUF mode.


Hi Hans, yes, I was going to suggest that. I will remove USERPTR
from both capture and mem2mem io_modes flags. Although I can
see where userptr support is still useful, that is for legacy middleware
that are still using mmap'ed userptrs and have not yet converted to
passing around dmabuf fd's.

But I've been perplexed for while on this, why vb2_dc_get_userptr() 
resorts to
scatter-gather (when the given user buffer is found not to have valid 
pfn's).

Shouldn't it be assumed that driver users of the vb2 dma-contig allocator
only support contiguous dma as the name implies? Maybe the issue is that
users that set VB2_USERPTR are implying they support scatter/gather, but
users of vb2-dma-contig could also be implying they do not, and would be
using vb2-dma-sg if that were the case.

Steve

--
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 00/38] i.MX5/6 Video Capture

[Hans Verkuil]

On 06/16/2016 07:02 PM, Steve Longerbeam wrote:
> On 06/16/2016 02:49 AM, Jack Mitchell wrote:
>>
>> On 16/06/16 02:37, Steve Longerbeam wrote:
>>> Hi Jack,
>>>
>>> On 06/15/2016 03:43 AM, Jack Mitchell wrote:
 
 Trying to use a user pointer rather than mmap also fails and causes a 
 kernel splat.

>>>
>>> Hmm, I've tested userptr with the mem2mem driver, but maybe never
>>> with video capture. I tried "v4l2-ctl -d/dev/video0 --stream-user=8" but
>>> that returns "VIDIOC_QBUF: failed: Invalid argument", haven't tracked
>>> down why (could be a bug in v4l2-ctl). Can you share the splat?
>>>
>>
>> On re-checking the splat was the same v4l_cropcap that was mentioned before 
>> so I don't think it's related. The error I get back is:
>>
>> VIDIOC_QBUF error 22, Invalid argument
>>
>> I'm using the example program the the v4l2 docs [1].
> 
> I found the cause at least in my case. After enabling dynamic debug in
> videobuf2-dma-contig.c, "v4l2-ctl -d/dev/video0 --stream-user=8" gives
> me
> 
> [  468.826046] user data must be aligned to 64 bytes
> 
> 
> 
> But even getting past that alignment issue, I've only tested userptr (in 
> mem2mem
> driver) by giving the driver a user address of a mmap'ed kernel contiguous
> buffer. A true discontiguous user buffer may not work, the IPU DMA does not
> support scatter-gather.

I don't think VB2_USERPTR should be enabled in this case due to the DMA 
limitations.
It won't allow you to use malloc()ed memory and the hack that allows you to pass
contiguous memory is superseded by the DMABUF mode.

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 00/38] i.MX5/6 Video Capture

[Nicolas Dufresne]

Le jeudi 16 juin 2016 à 10:02 -0700, Steve Longerbeam a écrit :
> I found the cause at least in my case. After enabling dynamic debug in
> videobuf2-dma-contig.c, "v4l2-ctl -d/dev/video0 --stream-user=8" gives
> me
> 
> [  468.826046] user data must be aligned to 64 bytes
> 
> 
> 
> But even getting past that alignment issue, I've only tested userptr (in 
> mem2mem
> driver) by giving the driver a user address of a mmap'ed kernel contiguous
> buffer. A true discontiguous user buffer may not work, the IPU DMA does not
> support scatter-gather.

If it's dma-contig, you'll need page aligned and contiguous memory.
What some test application do when testing their driver with that, is
to allocate memory using another device, or m2m device, that uses the
same allocator.

regards,
Nicolas

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

Re: [PATCH 00/38] i.MX5/6 Video Capture

[Nicolas Dufresne]

Le jeudi 16 juin 2016 à 10:02 -0700, Steve Longerbeam a écrit :
> I found the cause at least in my case. After enabling dynamic debug in
> videobuf2-dma-contig.c, "v4l2-ctl -d/dev/video0 --stream-user=8" gives
> me
> 
> [  468.826046] user data must be aligned to 64 bytes
> 
> 
> 
> But even getting past that alignment issue, I've only tested userptr (in 
> mem2mem
> driver) by giving the driver a user address of a mmap'ed kernel contiguous
> buffer. A true discontiguous user buffer may not work, the IPU DMA does not
> support scatter-gather.

If it's dma-contig, you'll need page aligned and contiguous memory.
What some test application do when testing their driver with that, is
to allocate memory using another device, or m2m device, that uses the
same allocator.

regards,
Nicolas
--
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 00/38] i.MX5/6 Video Capture

[Steve Longerbeam]

On 06/16/2016 02:49 AM, Jack Mitchell wrote:
>
> On 16/06/16 02:37, Steve Longerbeam wrote:
>> Hi Jack,
>>
>> On 06/15/2016 03:43 AM, Jack Mitchell wrote:
>>> 
>>> Trying to use a user pointer rather than mmap also fails and causes a 
>>> kernel splat.
>>>
>>
>> Hmm, I've tested userptr with the mem2mem driver, but maybe never
>> with video capture. I tried "v4l2-ctl -d/dev/video0 --stream-user=8" but
>> that returns "VIDIOC_QBUF: failed: Invalid argument", haven't tracked
>> down why (could be a bug in v4l2-ctl). Can you share the splat?
>>
>
> On re-checking the splat was the same v4l_cropcap that was mentioned before 
> so I don't think it's related. The error I get back is:
>
> VIDIOC_QBUF error 22, Invalid argument
>
> I'm using the example program the the v4l2 docs [1].

I found the cause at least in my case. After enabling dynamic debug in
videobuf2-dma-contig.c, "v4l2-ctl -d/dev/video0 --stream-user=8" gives
me

[  468.826046] user data must be aligned to 64 bytes



But even getting past that alignment issue, I've only tested userptr (in mem2mem
driver) by giving the driver a user address of a mmap'ed kernel contiguous
buffer. A true discontiguous user buffer may not work, the IPU DMA does not
support scatter-gather.

Steve

--
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 00/38] i.MX5/6 Video Capture

[Jack Mitchell]

On 16/06/16 02:37, Steve Longerbeam wrote:

Hi Jack,

On 06/15/2016 03:43 AM, Jack Mitchell wrote:


Trying to use a user pointer rather than mmap also fails and causes a kernel 
splat.



Hmm, I've tested userptr with the mem2mem driver, but maybe never
with video capture. I tried "v4l2-ctl -d/dev/video0 --stream-user=8" but
that returns "VIDIOC_QBUF: failed: Invalid argument", haven't tracked
down why (could be a bug in v4l2-ctl). Can you share the splat?



On re-checking the splat was the same v4l_cropcap that was mentioned 
before so I don't think it's related. The error I get back is:


VIDIOC_QBUF error 22, Invalid argument

I'm using the example program the the v4l2 docs [1].

Cheers,
Jack

[1] https://linuxtv.org/downloads/v4l-dvb-apis/capture-example.html




Apart from that and a few v4l2-compliance tests failing which you already 
mentioned, it seems to work OK. I'll try and do some more testing and see if I 
can come back with some more feedback.


Thanks!


Steve


--
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 00/38] i.MX5/6 Video Capture

[Steve Longerbeam]

Hi Jack,

On 06/15/2016 03:43 AM, Jack Mitchell wrote:
> 
> Trying to use a user pointer rather than mmap also fails and causes a kernel 
> splat.
>

Hmm, I've tested userptr with the mem2mem driver, but maybe never
with video capture. I tried "v4l2-ctl -d/dev/video0 --stream-user=8" but
that returns "VIDIOC_QBUF: failed: Invalid argument", haven't tracked
down why (could be a bug in v4l2-ctl). Can you share the splat?


> Apart from that and a few v4l2-compliance tests failing which you already 
> mentioned, it seems to work OK. I'll try and do some more testing and see if 
> I can come back with some more feedback.

Thanks!


Steve

--
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 00/38] i.MX5/6 Video Capture

[Hans Verkuil]

On 06/15/16 12:43, Jack Mitchell wrote:
> On 14/06/16 23:48, Steve Longerbeam wrote:
>> Tested on imx6q SabreAuto with ADV7180, and imx6q SabreSD with
>> mipi-csi2 OV5640. There is device-tree support also for imx6qdl
>> SabreLite, but that is not tested. Also, this driver should
>> theoretically work on i.MX5 targets, but that is also untested.
> 
> I tested this on a sabrelite with ov5640 MIPI camera. I managed to capture 
> multiple images of a few different resolutions without issue. I did get a 
> kernel splat at one point though:
> 
> [  905.469680] WARNING: CPU: 3 PID: 349 at 
> drivers/media/v4l2-core/v4l2-ioctl.c:2174 v4l_cropcap+0x15c/0x190

Known bug. Fix is waiting to be merged.

Regards,

Hans

> [  905.482308] Modules linked in: dw_hdmi_ahb_audio evbug
> [  905.489079] CPU: 3 PID: 349 Comm: capture-example Tainted: GW  
>  4.7.0-rc1-00095-g921736c0-dirty #7
> [  905.502069] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
> [  905.510116] Backtrace:
> [  905.514066] [] (dump_backtrace) from [] 
> (show_stack+0x18/0x1c)
> [  905.523161]  r7: r6:600f0013 r5: r4:c0e21bbc
> [  905.530442] [] (show_stack) from [] 
> (dump_stack+0xb4/0xe8)
> [  905.539210] [] (dump_stack) from [] (__warn+0xd8/0x104)
> [  905.547680]  r9:c06219b0 r8:087e r7:0009 r6:c0c51b10 r5: 
> r4:
> [  905.557086] [] (__warn) from [] 
> (warn_slowpath_null+0x28/0x30)
> [  905.566172]  r9:c5e09680 r8: r7:c63c0e00 r6:c5e09680 r5:c0a7b2fc 
> r4:c6223e18
> [  905.575577] [] (warn_slowpath_null) from [] 
> (v4l_cropcap+0x15c/0x190)
> [  905.585296] [] (v4l_cropcap) from [] 
> (__video_do_ioctl+0x280/0x300)
> [  905.594824]  r8: r7:c6248000 r6:c02c563a r5:0003 r4:c0621854
> [  905.603176] [] (__video_do_ioctl) from [] 
> (video_usercopy+0x208/0x520)
> [  905.612994]  r10:c6223e18 r9:c5e09680 r8:bef3fb60 r7: r6: 
> r5:002c
> [  905.622542]  r4:c02c563a
> [  905.626692] [] (video_usercopy) from [] 
> (video_ioctl2+0x14/0x1c)
> [  905.636129]  r10: r9:c6222000 r8:c62e0088 r7:bef3fb60 r6:c02c563a 
> r5:c5e09680
> [  905.645837]  r4:c6248000
> [  905.650146] [] (video_ioctl2) from [] 
> (v4l2_ioctl+0xac/0xe8)
> [  905.659402] [] (v4l2_ioctl) from [] 
> (do_vfs_ioctl+0x9c/0xa28)
> [  905.668768]  r9:c6222000 r8:0003 r7:c022cc90 r6:c5e09680 r5:c61fcc68 
> r4:bef3fb60
> [  905.678598] [] (do_vfs_ioctl) from [] 
> (SyS_ioctl+0x3c/0x64)
> [  905.687898]  r10: r9:c6222000 r8:bef3fb60 r7:c02c563a r6:c5e09680 
> r5:0003
> [  905.697911]  r4:c5e09680
> [  905.702505] [] (SyS_ioctl) from [] 
> (ret_fast_syscall+0x0/0x1c)
> [  905.712135]  r9:c6222000 r8:c0107dc4 r7:0036 r6:000107cc r5: 
> r4:00012170
> [  905.722143] ---[ end trace 772a5fdfa424cbd1 ]---
> 
> Trying to use a user pointer rather than mmap also fails and causes a kernel 
> splat.
> 
> Apart from that and a few v4l2-compliance tests failing which you already 
> mentioned, it seems to work OK. I'll try and do some more testing and see if 
> I can come back with some more feedback.
> 
> Tested-by: Jack Mitchell 
> 
> Cheers,
> Jack.
> 
>>
>> Not run through v4l2-compliance yet, but that is in my queue.
>>
>>
>> Philipp Zabel (2):
>>   ARM: dts: imx6qdl: Add mipi_ipu1/2 video muxes, mipi_csi, and their
>> connections
>>   media: imx: Add video switch
>>
>> Steve Longerbeam (35):
>>   gpu: ipu-v3: Add Video Deinterlacer unit
>>   gpu: ipu-cpmem: Add ipu_cpmem_set_uv_offset()
>>   gpu: ipu-cpmem: Add ipu_cpmem_get_burstsize()
>>   gpu: ipu-v3: Add ipu_get_num()
>>   gpu: ipu-v3: Add IDMA channel linking support
>>   gpu: ipu-v3: Add ipu_set_vdi_src_mux()
>>   gpu: ipu-v3: Add VDI input IDMAC channels
>>   gpu: ipu-v3: Add ipu_csi_set_src()
>>   gpu: ipu-v3: Add ipu_ic_set_src()
>>   gpu: ipu-v3: set correct full sensor frame for PAL/NTSC
>>   gpu: ipu-v3: Fix CSI data format for 16-bit media bus formats
>>   gpu: ipu-v3: Fix IRT usage
>>   gpu: ipu-ic: Add complete image conversion support with tiling
>>   gpu: ipu-ic: allow multiple handles to ic
>>   gpu: ipu-v3: rename CSI client device
>>   ARM: dts: imx6qdl: Flesh out MIPI CSI2 receiver node
>>   ARM: dts: imx6-sabrelite: add video capture ports and connections
>>   ARM: dts: imx6-sabresd: add video capture ports and connections
>>   ARM: dts: imx6-sabreauto: create i2cmux for i2c3
>>   ARM: dts: imx6-sabreauto: add reset-gpios property for max7310
>>   ARM: dts: imx6-sabreauto: add pinctrl for gpt input capture
>>   ARM: dts: imx6-sabreauto: add video capture ports and connections
>>   ARM: dts: imx6qdl: add mem2mem device for sabre* boards
>>   gpio: pca953x: Add reset-gpios property
>>   clocksource/drivers/imx: add input capture support
>>   v4l: Add signal lock status to source change events
>>   media: Add camera interface driver for i.MX5/6
>>   media: imx: Add MIPI CSI-2 Receiver driver
>>   media: imx: Add support for MIPI CSI-2 OV5640
>>   media: imx: Add 

Re: [PATCH 00/38] i.MX5/6 Video Capture

[Jack Mitchell]

On 14/06/16 23:48, Steve Longerbeam wrote:

Tested on imx6q SabreAuto with ADV7180, and imx6q SabreSD with
mipi-csi2 OV5640. There is device-tree support also for imx6qdl
SabreLite, but that is not tested. Also, this driver should
theoretically work on i.MX5 targets, but that is also untested.


I tested this on a sabrelite with ov5640 MIPI camera. I managed to 
capture multiple images of a few different resolutions without issue. I 
did get a kernel splat at one point though:


[  905.469680] WARNING: CPU: 3 PID: 349 at 
drivers/media/v4l2-core/v4l2-ioctl.c:2174 v4l_cropcap+0x15c/0x190

[  905.482308] Modules linked in: dw_hdmi_ahb_audio evbug
[  905.489079] CPU: 3 PID: 349 Comm: capture-example Tainted: GW 
  4.7.0-rc1-00095-g921736c0-dirty #7

[  905.502069] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
[  905.510116] Backtrace:
[  905.514066] [] (dump_backtrace) from [] 
(show_stack+0x18/0x1c)

[  905.523161]  r7: r6:600f0013 r5: r4:c0e21bbc
[  905.530442] [] (show_stack) from [] 
(dump_stack+0xb4/0xe8)
[  905.539210] [] (dump_stack) from [] 
(__warn+0xd8/0x104)
[  905.547680]  r9:c06219b0 r8:087e r7:0009 r6:c0c51b10 
r5: r4:
[  905.557086] [] (__warn) from [] 
(warn_slowpath_null+0x28/0x30)
[  905.566172]  r9:c5e09680 r8: r7:c63c0e00 r6:c5e09680 
r5:c0a7b2fc r4:c6223e18
[  905.575577] [] (warn_slowpath_null) from [] 
(v4l_cropcap+0x15c/0x190)
[  905.585296] [] (v4l_cropcap) from [] 
(__video_do_ioctl+0x280/0x300)

[  905.594824]  r8: r7:c6248000 r6:c02c563a r5:0003 r4:c0621854
[  905.603176] [] (__video_do_ioctl) from [] 
(video_usercopy+0x208/0x520)
[  905.612994]  r10:c6223e18 r9:c5e09680 r8:bef3fb60 r7: 
r6: r5:002c

[  905.622542]  r4:c02c563a
[  905.626692] [] (video_usercopy) from [] 
(video_ioctl2+0x14/0x1c)
[  905.636129]  r10: r9:c6222000 r8:c62e0088 r7:bef3fb60 
r6:c02c563a r5:c5e09680

[  905.645837]  r4:c6248000
[  905.650146] [] (video_ioctl2) from [] 
(v4l2_ioctl+0xac/0xe8)
[  905.659402] [] (v4l2_ioctl) from [] 
(do_vfs_ioctl+0x9c/0xa28)
[  905.668768]  r9:c6222000 r8:0003 r7:c022cc90 r6:c5e09680 
r5:c61fcc68 r4:bef3fb60
[  905.678598] [] (do_vfs_ioctl) from [] 
(SyS_ioctl+0x3c/0x64)
[  905.687898]  r10: r9:c6222000 r8:bef3fb60 r7:c02c563a 
r6:c5e09680 r5:0003

[  905.697911]  r4:c5e09680
[  905.702505] [] (SyS_ioctl) from [] 
(ret_fast_syscall+0x0/0x1c)
[  905.712135]  r9:c6222000 r8:c0107dc4 r7:0036 r6:000107cc 
r5: r4:00012170

[  905.722143] ---[ end trace 772a5fdfa424cbd1 ]---

Trying to use a user pointer rather than mmap also fails and causes a 
kernel splat.


Apart from that and a few v4l2-compliance tests failing which you 
already mentioned, it seems to work OK. I'll try and do some more 
testing and see if I can come back with some more feedback.


Tested-by: Jack Mitchell 

Cheers,
Jack.



Not run through v4l2-compliance yet, but that is in my queue.


Philipp Zabel (2):
  ARM: dts: imx6qdl: Add mipi_ipu1/2 video muxes, mipi_csi, and their
connections
  media: imx: Add video switch

Steve Longerbeam (35):
  gpu: ipu-v3: Add Video Deinterlacer unit
  gpu: ipu-cpmem: Add ipu_cpmem_set_uv_offset()
  gpu: ipu-cpmem: Add ipu_cpmem_get_burstsize()
  gpu: ipu-v3: Add ipu_get_num()
  gpu: ipu-v3: Add IDMA channel linking support
  gpu: ipu-v3: Add ipu_set_vdi_src_mux()
  gpu: ipu-v3: Add VDI input IDMAC channels
  gpu: ipu-v3: Add ipu_csi_set_src()
  gpu: ipu-v3: Add ipu_ic_set_src()
  gpu: ipu-v3: set correct full sensor frame for PAL/NTSC
  gpu: ipu-v3: Fix CSI data format for 16-bit media bus formats
  gpu: ipu-v3: Fix IRT usage
  gpu: ipu-ic: Add complete image conversion support with tiling
  gpu: ipu-ic: allow multiple handles to ic
  gpu: ipu-v3: rename CSI client device
  ARM: dts: imx6qdl: Flesh out MIPI CSI2 receiver node
  ARM: dts: imx6-sabrelite: add video capture ports and connections
  ARM: dts: imx6-sabresd: add video capture ports and connections
  ARM: dts: imx6-sabreauto: create i2cmux for i2c3
  ARM: dts: imx6-sabreauto: add reset-gpios property for max7310
  ARM: dts: imx6-sabreauto: add pinctrl for gpt input capture
  ARM: dts: imx6-sabreauto: add video capture ports and connections
  ARM: dts: imx6qdl: add mem2mem device for sabre* boards
  gpio: pca953x: Add reset-gpios property
  clocksource/drivers/imx: add input capture support
  v4l: Add signal lock status to source change events
  media: Add camera interface driver for i.MX5/6
  media: imx: Add MIPI CSI-2 Receiver driver
  media: imx: Add support for MIPI CSI-2 OV5640
  media: imx: Add support for Parallel OV5642
  media: imx: Add support for ADV7180 Video Decoder
  media: adv7180: add power pin control
  media: adv7180: implement g_parm
  media: Add i.MX5/6 mem2mem driver
  ARM: imx_v6_v7_defconfig: Enable staging video4linux drivers

Suresh Dhandapani (1):
  gpu: ipu-v3: Fix CSI0 blur in NTSC format