Re: [PATCH v3 00/24] i.MX Media Driver

[Philipp Zabel]

Hi Steve,

On Tue, 2017-02-14 at 18:27 -0800, Steve Longerbeam wrote:
[...]
> >
> > # Provide an EDID to the HDMI source
> > v4l2-ctl -d /dev/v4l-subdev2 --set-edid=file=edid-1080p.hex
> 
> I can probably generate this Intel hex file myself from sysfs
> edid outputs, but for convenience do you mind sending me this
> file? I have a 1080p HDMI source I can plug into the tc358743.

I copied the EDID off of some random 1080p HDMI monitor,
probably using something like:

xxd -g1 /sys/class/drm/card0-HDMI-A-1/edid | cut -c 9-56

--8<--
 00 ff ff ff ff ff ff 00 09 d1 89 78 45 54 00 00
 2a 14 01 03 80 35 1e 78 2e b8 45 a1 59 55 9f 28
 0d 50 54 a5 6b 80 81 c0 81 00 81 80 a9 c0 b3 00
 d1 c0 01 01 01 01 02 3a 80 18 71 38 2d 40 58 2c
 45 00 13 2a 21 00 00 1e 00 00 00 ff 00 4e 41 41
 30 36 32 39 36 53 4c 30 0a 20 00 00 00 fd 00 32
 4c 1e 53 11 00 0a 20 20 20 20 20 20 00 00 00 fc
 00 42 65 6e 51 20 47 4c 32 34 34 30 48 0a 01 18
 02 03 22 f1 4f 90 05 04 03 02 01 11 12 13 14 06
 07 15 16 1f 23 09 07 07 65 03 0c 00 10 00 83 01
 00 00 02 3a 80 18 71 38 2d 40 58 2c 45 00 13 2a
 21 00 00 1f 01 1d 80 18 71 1c 16 20 58 2c 25 00
 13 2a 21 00 00 9f 01 1d 00 72 51 d0 1e 20 6e 28
 55 00 13 2a 21 00 00 1e 8c 0a d0 8a 20 e0 2d 10
 10 3e 96 00 13 2a 21 00 00 18 00 00 00 00 00 00
 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 eb
-->8--

> The other problem here is that my version of v4l2-ctl, built from
> master branch of g...@github.com:gjasny/v4l-utils.git, does not
> support taking a subdev node:
> 
> root@mx6q:~# v4l2-ctl -d /dev/v4l-subdev15 --get-edid=format=hex
> VIDIOC_QUERYCAP: failed: Inappropriate ioctl for device
> /dev/v4l-subdev15: not a v4l2 node
> 
> Is this something you added yourself, or where can I find this version
> of v4l2-ctrl?

Ah right, I still have no proper fix for that. v4l-ctl bails out if it
can't VIDIOC_QUERYCAP, which is an ioctl not supported on subdevices.
I have just patched it out locally:

--8<--
diff --git a/utils/v4l2-ctl/v4l2-ctl.cpp b/utils/v4l2-ctl/v4l2-ctl.cpp
index 886a91d093ae..fa15a49375ae 100644
--- a/utils/v4l2-ctl/v4l2-ctl.cpp
+++ b/utils/v4l2-ctl/v4l2-ctl.cpp
@@ -1214,10 +1214,7 @@ int main(int argc, char **argv)
}
 
verbose = options[OptVerbose];
-   if (doioctl(fd, VIDIOC_QUERYCAP, )) {
-   fprintf(stderr, "%s: not a v4l2 node\n", device);
-   exit(1);
-   }
+   doioctl(fd, VIDIOC_QUERYCAP, );
capabilities = vcap.capabilities;
if (capabilities & V4L2_CAP_DEVICE_CAPS)
capabilities = vcap.device_caps;
-->8--

Note that setting the EDID is not necessary if you can force the mode on
your HDMI source.

regards
Philipp

Re: [PATCH v3 00/24] i.MX Media Driver

[Steve Longerbeam]

Hi Philipp,

I've created a test branch off my imx-media-staging-md-wip called tc358743,
which cherry-picks a couple of your commits from your 
imx-media-staging-md-wip

branch:

[media] tc358743: set entity function to video interface bridge
[media] tc358743: put lanes in STOP state before starting streaming

And one more commit that enables the tc358743 in the DT for sabrelite:

ARM: dts: imx6-sabrelite: switch to tc358743

which is based off your work in imx6qdl-nitrogen6x-bd-hdmi-mipi.dtsi.

With that the tc358743 is loading fine, and is present in the media graph:

root@mx6q:~# dmesg | grep -i tc358
[   11.056799] imx-media: Registered subdev tc358743 1-000f
[   11.122133] imx-media: imx_media_create_link: tc358743 1-000f:0 -> 
imx6-mipi-csi2:0

[   11.490274] tc358743 1-000f: tc358743 found @ 0x1e (21a4000.i2c)


But I'm not able to get to testing streaming yet, see below.


On 01/31/2017 05:54 AM, Philipp Zabel wrote:

Hi Steve,

I have just tested the imx-media-staging-md-wip branch on a Nitrogen6X
with a tc358743 (BD_HDMI_MIPI HDMI to MIPI CSI-2 receiver board). Some
observations:

# Link pipeline
media-ctl -l "'tc358743 1-000f':0->'imx6-mipi-csi2':0[1]"
media-ctl -l "'imx6-mipi-csi2':1->'ipu1_csi0_mux':0[1]"
media-ctl -l "'ipu1_csi0_mux':2->'ipu1_csi0':0[1]"
media-ctl -l "'ipu1_csi0':2->'ipu1_csi0 capture':0[1]"


This works fine, I can create these links.



# Provide an EDID to the HDMI source
v4l2-ctl -d /dev/v4l-subdev2 --set-edid=file=edid-1080p.hex


I can probably generate this Intel hex file myself from sysfs
edid outputs, but for convenience do you mind sending me this
file? I have a 1080p HDMI source I can plug into the tc358743.

The other problem here is that my version of v4l2-ctl, built from
master branch of g...@github.com:gjasny/v4l-utils.git, does not
support taking a subdev node:

root@mx6q:~# v4l2-ctl -d /dev/v4l-subdev15 --get-edid=format=hex
VIDIOC_QUERYCAP: failed: Inappropriate ioctl for device
/dev/v4l-subdev15: not a v4l2 node

Is this something you added yourself, or where can I find this version
of v4l2-ctrl?

Thanks,
Steve


# At this point the HDMI source is enabled and sends a 1080p60 signal
# Configure detected DV timings
media-ctl --set-dv "'tc358743 1-000f':0"

# Set pad formats
media-ctl --set-v4l2 "'tc358743 1-000f':0[fmt:UYVY/1920x1080]"
media-ctl --set-v4l2 "'imx6-mipi-csi2':1[fmt:UYVY2X8/1920x1080]"
media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080]"
media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/1920x1080]"

v4l2-ctl -d /dev/video4 -V
# This still is configured to 640x480, which is inconsistent with
# the 'ipu1_csi0':2 pad format. The pad set_fmt above should
# have set this, too.

v4l2-ctl --list-formats -d /dev/video4
# This lists all the RGB formats, which it shouldn't. There is
# no CSC in this pipeline, so we should be limited to YUV formats
# only.

# Set capture format
v4l2-ctl -d /dev/video4 -v width=1920,height=1080,pixelformat=UYVY

v4l2-ctl -d /dev/video4 -V
# Now the capture format is correctly configured to 1920x1080.

v4l2-ctl -d 4 --list-frameintervals=width=1920,height=1080,pixelformat=UYVY
# This lists nothing. We should at least provide the 'ipu1_csi0':2 pad
# frame interval. In the future this should list fractions achievable
# via frame skipping.

v4l2-compliance -d /dev/video4
# This fails two tests:
# fail: v4l2-test-input-output.cpp(383): std == 0
# fail: v4l2-test-input-output.cpp(449): invalid attributes for input 0
# test VIDIOC_G/S/ENUMINPUT: FAIL
# and
# fail: v4l2-test-controls.cpp(782): subscribe event for control 'User 
Controls' failed
# test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: FAIL

# (Slowly) stream JPEG images to a display host:
gst-launch-1.0 -v v4l2src device=/dev/video4 ! jpegenc ! rtpjpegpay ! udpsink

I've done this a few times, and sometimes I only get a "ipu1_csi0: EOF
timeout" message when starting streaming.

regards
Philipp

Re: [PATCH v3 00/24] i.MX Media Driver

[Steve Longerbeam]

On 02/02/2017 02:29 PM, Russell King - ARM Linux wrote:

On Thu, Feb 02, 2017 at 11:12:41AM -0800, Steve Longerbeam wrote:

Here is the current .queue_setup() op in imx-media-capture.c:

static int capture_queue_setup(struct vb2_queue *vq,
unsigned int *nbuffers,
unsigned int *nplanes,
unsigned int sizes[],
struct device *alloc_devs[])
{
 struct capture_priv *priv = vb2_get_drv_priv(vq);
 struct v4l2_pix_format *pix = >vdev.fmt.fmt.pix;
 unsigned int count = *nbuffers;

 if (vq->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
 return -EINVAL;

 if (*nplanes) {
 if (*nplanes != 1 || sizes[0] < pix->sizeimage)
 return -EINVAL;
 count += vq->num_buffers;
 }

 while (pix->sizeimage * count > VID_MEM_LIMIT)
 count--;

That's a weird way of writing:

unsigned int max_num = VID_MEM_LIMIT / pix->sizeimage;
count = max(count, max_num);


I think you mean min() there, but yes thanks, fixed.

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 v3 00/24] i.MX Media Driver

[Steve Longerbeam]

On 02/03/2017 06:41 AM, Laurent Pinchart wrote:

Hello,

On Wednesday 01 Feb 2017 16:19:27 Steve Longerbeam wrote:

On 02/01/2017 01:30 AM, Philipp Zabel wrote:


media-ctl propagates the output pad format to all remote subdevices'
input pads for all enabled links:

https://git.linuxtv.org/v4l-utils.git/tree/utils/media-ctl/libv4l2subdev.c
#n693

Ah cool, I wasn't aware media-ctl did this, but it makes sense and
makes it easier on the user.

To be precise, userspace is responsible for propagating formats *between*
subdevs (source to sink, over a link) and drivers for propagating formats *in*
subdevs (sink to source, inside the subdev).


Hi Laurent, yes thanks for that clarification.

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 v3 00/24] i.MX Media Driver

[Laurent Pinchart]

Hello,

On Wednesday 01 Feb 2017 16:19:27 Steve Longerbeam wrote:
> On 02/01/2017 01:30 AM, Philipp Zabel wrote:
> > On Tue, 2017-01-31 at 17:26 -0800, Steve Longerbeam wrote:
> > [...]
> > 
> >>> # Set pad formats
> >>> media-ctl --set-v4l2 "'tc358743 1-000f':0[fmt:UYVY/1920x1080]"
> >>> media-ctl --set-v4l2 "'imx6-mipi-csi2':1[fmt:UYVY2X8/1920x1080]"
> >>> media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080]"
> >>> media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/1920x1080]"
> >>> 
> >>> v4l2-ctl -d /dev/video4 -V
> >>> # This still is configured to 640x480, which is inconsistent with
> >>> # the 'ipu1_csi0':2 pad format. The pad set_fmt above should
> >>> # have set this, too.
> >> 
> >> Because you've only configured the source pads,
> >> and not the sink pads. The ipu_csi source format is
> >> dependent on the sink format - output crop window is
> >> limited by max input sensor frame, and since sink pad is
> >> still at 640x480, output is reduced to that.
> > 
> > No, it is set (see below). What happens is that capture_g_fmt_vid_cap
> > just returns the capture devices' priv->vdev.fmt, even if it is
> > incompatible with the connected csi subdevice's output pad format.
> > 
> > priv->vdev.fmt was never changed from the default set in
> > imx_media_capture_device_register, because capture_s/try_fmt_vid_cap
> > were not called yet.
> 
> Ah, yep, this is a bug. Need to modify the capture device's
> width/height at .set_fmt() in the subdev's device-node source
> pad (csi and prpenc/vf).
> 
> >> Maybe I'm missing something, is it expected behavior that
> >> a source format should be automatically propagated to
> >> the sink?
> > 
> > media-ctl propagates the output pad format to all remote subdevices'
> > input pads for all enabled links:
> > 
> > https://git.linuxtv.org/v4l-utils.git/tree/utils/media-ctl/libv4l2subdev.c
> > #n693
>
> Ah cool, I wasn't aware media-ctl did this, but it makes sense and
> makes it easier on the user.

To be precise, userspace is responsible for propagating formats *between* 
subdevs (source to sink, over a link) and drivers for propagating formats *in* 
subdevs (sink to source, inside the subdev).

> >>> v4l2-ctl --list-formats -d /dev/video4
> >>> # This lists all the RGB formats, which it shouldn't. There is
> >>> # no CSC in this pipeline, so we should be limited to YUV formats
> >>> # only.
> >> 
> >> right, need to fix that. Probably by poking the attached
> >> source subdev (csi or prpenc/vf) for its supported formats.
> > 
> > You are right, in bayer/raw mode only one specific format should be
> > listed, depending on the CSI output pad format.

-- 
Regards,

Laurent Pinchart

--
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: vb2 queue_setup documentation clarification (was "Re: [PATCH v3 00/24] i.MX Media Driver")

[Hans Verkuil]

On 03/02/17 15:21, Laurent Pinchart wrote:

Hi Steve,

(stripping the CC list a bit and adding Sakari Ailus)

On Thursday 02 Feb 2017 11:12:41 Steve Longerbeam wrote:

On 02/02/2017 10:58 AM, Russell King - ARM Linux wrote:


[snip]


It seems to me that if you don't take account of the existing queue
size, your camif_queue_setup() has the side effect that each time
either of these are called.  Hence, the vb2 queue increases by the
same amount each time, which is probably what you don't want.

The documentation on queue_setup() leaves much to be desired:
  * @queue_setup: called from VIDIOC_REQBUFS() and VIDIOC_CREATE_BUFS()
  *  handlers before memory allocation. It can be  called
  *  twice: if the original number of requested  buffers
  *  could not be allocated, then it will be called a
  *  second time with the actually allocated number of
  *  buffers to verify if that is OK.
  *  The driver should return the required number of buffers
  *  in \*num_buffers, the required number of planes per
  *  buffer in \*num_planes, the size of each plane should be
  *  set in the sizes\[\] array and optional per-plane
  *  allocator specific device in the alloc_devs\[\] array.
  *  When called from VIDIOC_REQBUFS,() \*num_planes == 0,
  *  the driver has to use the currently configured format to
  *  determine the plane sizes and \*num_buffers is the total
  *  number of buffers that are being allocated. When called
  *  from VIDIOC_CREATE_BUFS,() \*num_planes != 0 and it
  *  describes the requested number of planes and sizes\[\]
  *  contains the requested plane sizes. If either
  *  \*num_planes or the requested sizes are invalid callback
  *  must return %-EINVAL. In this case \*num_buffers are
  *  being allocated additionally to q->num_buffers.

That's really really ambiguous, because the "In this case" part doesn't
really tell you which case it's talking about - but it seems to me looking
at the code that it's referring to the VIDIOC_CREATE_BUFS case.


Yes, I caught this when adding fixes from v4l2-compliance testing, which
is not part of the version 3 driver. I agree it is a confusing API. When
called from VIDIOC_CREATE_BUFS (indicated by *num_planes != 0),
*num_buffers is supposed to be requested buffers _in addition_ to
already requested q->num_buffers, which is important info and
should be emphasized a little more than the "oh by the way" fashion
in the prototype description, IMHO.


Hans, Sakari, any opinion ?


This certainly could be improved.

The total number of buffers is q->num_buffers + *num_buffer where q->num_buffers
is 0 when called from VIDIOC_REQBUFS and can be > 0 when called from 
VIDIOC_CREATEBUFS.

So if you want to ensure that a certain minimum number of buffers is allocated,
then you would code that as:

if (q->num_buffers + *num_buffers < 3)
*num_buffers = 3 - q->num_buffers;

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

vb2 queue_setup documentation clarification (was "Re: [PATCH v3 00/24] i.MX Media Driver")

[Laurent Pinchart]

Hi Steve,

(stripping the CC list a bit and adding Sakari Ailus)

On Thursday 02 Feb 2017 11:12:41 Steve Longerbeam wrote:
> On 02/02/2017 10:58 AM, Russell King - ARM Linux wrote:

[snip]

> > It seems to me that if you don't take account of the existing queue
> > size, your camif_queue_setup() has the side effect that each time
> > either of these are called.  Hence, the vb2 queue increases by the
> > same amount each time, which is probably what you don't want.
> > 
> > The documentation on queue_setup() leaves much to be desired:
> >   * @queue_setup: called from VIDIOC_REQBUFS() and VIDIOC_CREATE_BUFS() 
> >   *  handlers before memory allocation. It can be  called
> >   *  twice: if the original number of requested  buffers
> >   *  could not be allocated, then it will be called a
> >   *  second time with the actually allocated number of
> >   *  buffers to verify if that is OK.
> >   *  The driver should return the required number of buffers
> >   *  in \*num_buffers, the required number of planes per
> >   *  buffer in \*num_planes, the size of each plane should be
> >   *  set in the sizes\[\] array and optional per-plane
> >   *  allocator specific device in the alloc_devs\[\] array.
> >   *  When called from VIDIOC_REQBUFS,() \*num_planes == 0,
> >   *  the driver has to use the currently configured format to
> >   *  determine the plane sizes and \*num_buffers is the total
> >   *  number of buffers that are being allocated. When called
> >   *  from VIDIOC_CREATE_BUFS,() \*num_planes != 0 and it
> >   *  describes the requested number of planes and sizes\[\]
> >   *  contains the requested plane sizes. If either
> >   *  \*num_planes or the requested sizes are invalid callback
> >   *  must return %-EINVAL. In this case \*num_buffers are
> >   *  being allocated additionally to q->num_buffers.
> >
> > That's really really ambiguous, because the "In this case" part doesn't
> > really tell you which case it's talking about - but it seems to me looking
> > at the code that it's referring to the VIDIOC_CREATE_BUFS case.
> 
> Yes, I caught this when adding fixes from v4l2-compliance testing, which
> is not part of the version 3 driver. I agree it is a confusing API. When
> called from VIDIOC_CREATE_BUFS (indicated by *num_planes != 0),
> *num_buffers is supposed to be requested buffers _in addition_ to
> already requested q->num_buffers, which is important info and
> should be emphasized a little more than the "oh by the way" fashion
> in the prototype description, IMHO.

Hans, Sakari, any opinion ?

[snip]

-- 
Regards,

Laurent Pinchart

--
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 00/24] i.MX Media Driver

[Russell King - ARM Linux]

On Thu, Feb 02, 2017 at 11:12:41AM -0800, Steve Longerbeam wrote:
> Here is the current .queue_setup() op in imx-media-capture.c:
> 
> static int capture_queue_setup(struct vb2_queue *vq,
>unsigned int *nbuffers,
>unsigned int *nplanes,
>unsigned int sizes[],
>struct device *alloc_devs[])
> {
> struct capture_priv *priv = vb2_get_drv_priv(vq);
> struct v4l2_pix_format *pix = >vdev.fmt.fmt.pix;
> unsigned int count = *nbuffers;
> 
> if (vq->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> return -EINVAL;
> 
> if (*nplanes) {
> if (*nplanes != 1 || sizes[0] < pix->sizeimage)
> return -EINVAL;
> count += vq->num_buffers;
> }
> 
> while (pix->sizeimage * count > VID_MEM_LIMIT)
> count--;

That's a weird way of writing:

unsigned int max_num = VID_MEM_LIMIT / pix->sizeimage;
count = max(count, max_num);

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
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 00/24] i.MX Media Driver

[Steve Longerbeam]

On 02/02/2017 10:58 AM, Russell King - ARM Linux wrote:

On Thu, Feb 02, 2017 at 10:26:55AM -0800, Steve Longerbeam wrote:

On 02/02/2017 09:56 AM, Russell King - ARM Linux wrote:

and for whatever reason we end up falling out through free_ring.  This
is VERY bad news, because it means that the ring which SMFC took a copy
of is now freed beneath its feet.

Yes, that is bad. That was a bug, if imx_media_dma_buf_queue_from_vb()
returned error, the ring should not have been freed, it should have only
returned the error. And further bad stuff happens from that point on.

But all of this is gone in version 4.

I think there's an error in how you think the queue_setup() works.

camif_queue_setup() always returns the number of buffers between
IMX_MEDIA_MIN_RING_BUFS and IMX_MEDIA_MAX_RING_BUFS.  However, it seems
that, looking through the videobuf2-core.c code, that the value is
passed to __vb2_queue_alloc() to allocate the specified number of
_additional_ buffers over and on-top of the existing q->num_buffers:

static int __vb2_queue_alloc(struct vb2_queue *q, enum vb2_memory memory,
  unsigned int num_buffers, unsigned int num_planes,
  const unsigned plane_sizes[VB2_MAX_PLANES])
{
 for (buffer = 0; buffer < num_buffers; ++buffer) {
...
 vb->index = q->num_buffers + buffer;

and

int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
 unsigned int *count)
{
 unsigned int num_buffers, allocated_buffers, num_planes = 0;
...
 num_buffers = min_t(unsigned int, *count, VB2_MAX_FRAME);
 num_buffers = max_t(unsigned int, num_buffers, q->min_buffers_needed);
...
 /*
  * Ask the driver how many buffers and planes per buffer it requires.
  * Driver also sets the size and allocator context for each plane.
  */
 ret = call_qop(q, queue_setup, q, _buffers, _planes,
plane_sizes, q->alloc_devs);
 if (ret)
 return ret;

 /* Finally, allocate buffers and video memory */
 allocated_buffers =
 __vb2_queue_alloc(q, memory, num_buffers, num_planes, 
plane_sizes);

or:

int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory,
 unsigned int *count, unsigned requested_planes,
 const unsigned requested_sizes[])
{
 unsigned int num_planes = 0, num_buffers, allocated_buffers;
...
 num_buffers = min(*count, VB2_MAX_FRAME - q->num_buffers);
 if (requested_planes && requested_sizes) {
 num_planes = requested_planes;
...
 /*
  * Ask the driver, whether the requested number of buffers, planes per
  * buffer and their sizes are acceptable
  */
 ret = call_qop(q, queue_setup, q, _buffers,
_planes, plane_sizes, q->alloc_devs);
 if (ret)
 return ret;

 /* Finally, allocate buffers and video memory */
 allocated_buffers = __vb2_queue_alloc(q, memory, num_buffers,
 num_planes, plane_sizes);


It seems to me that if you don't take account of the existing queue
size, your camif_queue_setup() has the side effect that each time
either of these are called.  Hence, the vb2 queue increases by the
same amount each time, which is probably what you don't want.

The documentation on queue_setup() leaves much to be desired:

  * @queue_setup:called from VIDIOC_REQBUFS() and VIDIOC_CREATE_BUFS()
  *  handlers before memory allocation. It can be called
  *  twice: if the original number of requested buffers
  *  could not be allocated, then it will be called a
  *  second time with the actually allocated number of
  *  buffers to verify if that is OK.
  *  The driver should return the required number of buffers
  *  in \*num_buffers, the required number of planes per
  *  buffer in \*num_planes, the size of each plane should 
be
  *  set in the sizes\[\] array and optional per-plane
  *  allocator specific device in the alloc_devs\[\] array.
  *  When called from VIDIOC_REQBUFS,() \*num_planes == 0,
  *  the driver has to use the currently configured format 
to
  *  determine the plane sizes and \*num_buffers is the 
total
  *  number of buffers that are being allocated. When called
  *  from VIDIOC_CREATE_BUFS,() \*num_planes != 0 and it
  *  describes the requested number of planes and sizes\[\]
  *  contains the requested plane sizes. If either
  *  \*num_planes or the requested sizes are invalid 
callback
  *  

Re: [PATCH v3 00/24] i.MX Media Driver

[Russell King - ARM Linux]

On Thu, Feb 02, 2017 at 10:26:55AM -0800, Steve Longerbeam wrote:
> On 02/02/2017 09:56 AM, Russell King - ARM Linux wrote:
> >and for whatever reason we end up falling out through free_ring.  This
> >is VERY bad news, because it means that the ring which SMFC took a copy
> >of is now freed beneath its feet.
> 
> Yes, that is bad. That was a bug, if imx_media_dma_buf_queue_from_vb()
> returned error, the ring should not have been freed, it should have only
> returned the error. And further bad stuff happens from that point on.
> 
> But all of this is gone in version 4.

I think there's an error in how you think the queue_setup() works.

camif_queue_setup() always returns the number of buffers between
IMX_MEDIA_MIN_RING_BUFS and IMX_MEDIA_MAX_RING_BUFS.  However, it seems
that, looking through the videobuf2-core.c code, that the value is
passed to __vb2_queue_alloc() to allocate the specified number of
_additional_ buffers over and on-top of the existing q->num_buffers:

static int __vb2_queue_alloc(struct vb2_queue *q, enum vb2_memory memory,
 unsigned int num_buffers, unsigned int num_planes,
 const unsigned plane_sizes[VB2_MAX_PLANES])
{
for (buffer = 0; buffer < num_buffers; ++buffer) {
...
vb->index = q->num_buffers + buffer;

and

int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
unsigned int *count)
{
unsigned int num_buffers, allocated_buffers, num_planes = 0;
...
num_buffers = min_t(unsigned int, *count, VB2_MAX_FRAME);
num_buffers = max_t(unsigned int, num_buffers, q->min_buffers_needed);
...
/*
 * Ask the driver how many buffers and planes per buffer it requires.
 * Driver also sets the size and allocator context for each plane.
 */
ret = call_qop(q, queue_setup, q, _buffers, _planes,
   plane_sizes, q->alloc_devs);
if (ret)
return ret;

/* Finally, allocate buffers and video memory */
allocated_buffers =
__vb2_queue_alloc(q, memory, num_buffers, num_planes, 
plane_sizes);

or:

int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory,
unsigned int *count, unsigned requested_planes,
const unsigned requested_sizes[])
{
unsigned int num_planes = 0, num_buffers, allocated_buffers;
...
num_buffers = min(*count, VB2_MAX_FRAME - q->num_buffers);
if (requested_planes && requested_sizes) {
num_planes = requested_planes;
...
/*
 * Ask the driver, whether the requested number of buffers, planes per
 * buffer and their sizes are acceptable
 */
ret = call_qop(q, queue_setup, q, _buffers,
   _planes, plane_sizes, q->alloc_devs);
if (ret)
return ret;

/* Finally, allocate buffers and video memory */
allocated_buffers = __vb2_queue_alloc(q, memory, num_buffers,
num_planes, plane_sizes);


It seems to me that if you don't take account of the existing queue
size, your camif_queue_setup() has the side effect that each time
either of these are called.  Hence, the vb2 queue increases by the
same amount each time, which is probably what you don't want.

The documentation on queue_setup() leaves much to be desired:

 * @queue_setup:called from VIDIOC_REQBUFS() and VIDIOC_CREATE_BUFS()
 *  handlers before memory allocation. It can be called
 *  twice: if the original number of requested buffers
 *  could not be allocated, then it will be called a
 *  second time with the actually allocated number of
 *  buffers to verify if that is OK.
 *  The driver should return the required number of buffers
 *  in \*num_buffers, the required number of planes per
 *  buffer in \*num_planes, the size of each plane should be
 *  set in the sizes\[\] array and optional per-plane
 *  allocator specific device in the alloc_devs\[\] array.
 *  When called from VIDIOC_REQBUFS,() \*num_planes == 0,
 *  the driver has to use the currently configured format to
 *  determine the plane sizes and \*num_buffers is the total
 *  number of buffers that are being allocated. When called
 *  from VIDIOC_CREATE_BUFS,() \*num_planes != 0 and it
 *  describes the requested number of planes and sizes\[\]
 *  contains the requested plane sizes. If either
 *  \*num_planes or the requested sizes are invalid callback
 *  must return %-EINVAL. In this case \*num_buffers are
 *  being allocated 

Re: [PATCH v3 00/24] i.MX Media Driver

[Steve Longerbeam]

On 02/02/2017 09:56 AM, Russell King - ARM Linux wrote:

On Thu, Feb 02, 2017 at 05:22:46PM +, Russell King - ARM Linux wrote:

I thought, maybe, it's the IPU overwriting past the end of the buffer,
but I've added checks and that doesn't seem to have fired.  I also
wondered if it was some kind of use-after-free of the ring, so I made
imx_media_free_dma_buf_ring() memset the ring to 0x5a5a5a5a before
kfree()ing it... doesn't look like it's that either.  I'm going to
continue poking to see if I can figure out what's going on.

I take that back... here's a use-after-free of that buffer, on the
very first run:

Alignment trap: not handling instruction e1921f9f at []
Unhandled fault: alignment exception (0x001) at 0x5a5a5b5e
pgd = c0004000
[5a5a5b5e] *pgd=
Internal error: : 1 [#1] SMP ARM
Modules linked in: imx_csi(C) rfcomm bnep bluetooth nfsd imx_camif(C) imx_ic(C) 
imx_smfc(C) caam_jr snd_soc_imx_spdif snd_soc_imx_sgtl5000 
snd_soc_fsl_asoc_card imx_media(C) uvcvideo imx_mipi_csi2(C) 
imx_media_common(C) imx219 snd_soc_sgtl5000 snd_soc_imx_audmux caam 
video_multiplexer imx_sdma imx2_wdt coda v4l2_mem2mem videobuf2_v4l2 
videobuf2_dma_contig videobuf2_core rc_cec snd_soc_fsl_ssi snd_soc_fsl_spdif 
videobuf2_vmalloc videobuf2_memops imx_pcm_dma imx_thermal dw_hdmi_ahb_audio 
dw_hdmi_cec etnaviv fuse rc_pinnacle_pctv_hd
CPU: 0 PID: 99 Comm: kworker/0:3 Tainted: G C  4.10.0-rc6+ #2103
Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
Workqueue: lru-add-drain wq_barrier_func
task: ee4e24c0 task.stack: ee6da000
PC is at __lock_acquire+0xd4/0x17b0
LR is at lock_acquire+0xd8/0x250
pc : []lr : []psr: 20070193
sp : ee6dbb60  ip : 0001  fp : ee6dbbe4
r10: e9efad60  r9 : c0a70384  r8 : 
r7 : c0a38680  r6 :   r5 : ee4e24c0  r4 : c1400408
r3 :   r2 : 5a5a5b5e  r1 :   r0 : 5a5a5a5a
Flags: nzCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment none
Control: 10c5387d  Table: 3d7ec04a  DAC: 0051
Process kworker/0:3 (pid: 99, stack limit = 0xee6da210)
Stack: (0xee6dbb60 to 0xee6dc000)
bb60: c0a38680 0002 c0b9d8c4 ee4e29a8 ee6dbc04 ee6dbb80 c0089708 c0088d44
bb80: ee6dbb9c 050f c00867fc c0086728 ee6dbbf4 ee6dbba0 87eba239 c035aa2f
bba0: 0001 ee4e29a8 c00c4f84 0001 0026 0560e36b  
bbc0: 60070193 e9efad60   c0a70384  ee6dbc3c ee6dbbe8
bbe0: c008b108 c0089400 0001 0080  bf0d2a8c  
bc00: c008b108 c0089400 0001 c09e04ec  e9efad50 60070193 bf0d2a8c
bc20: 0139  c09e04ec ee6dbcec ee6dbc6c ee6dbc40 c07016f4 c008b03c
bc40: 0001  bf0d2a8c ee6dbcec ee6dbc84 e9efac00 e9efad50 ee9785c4
bc60: ee6dbc84 ee6dbc70 bf0d2a8c c07016b4 ee978410 e9efb400 ee6dbca4 ee6dbc88
bc80: bf1224b8 bf0d2a7c bf122458 ee88d4c0 e9efb400 e9efb410 ee6dbce4 ee6dbca8
bca0: c009f5dc bf122464 0001 c09e04ec  e9efb400 c009f9f8 e9efb400
bcc0: e9efb400 e9efb410  0009 f4001100 ee6dbe38 ee6dbd04 ee6dbce8
bce0: c009f984 c009f544 c0701d10  e9efb400 e9efb460 ee6dbd24 ee6dbd08
bd00: c009fa00 c009f96c c09d0418 e9efb400 e9efb460 e9efb410 ee6dbd44 ee6dbd28
bd20: c00a3174 c009f9cc c00a30c4  ee6dbd90 ee4a3010 ee6dbd54 ee6dbd48
bd40: c009ecf0 c00a30d0 ee6dbd84 ee6dbd58 c0409328 c009ecdc c09d0448 0001
bd60: 0026 ef1efc10 c09e756c ee4a3010 0026 ef008400 ee6dbdcc ee6dbd88
bd80: c0409458 c040928c 0001  0001 0002 0003 000a
bda0: 000b 000c 000d 000e ee6dbdcc c09d52d0  
bdc0: ee6dbddc ee6dbdd0 c009ecf0 c0409408 ee6dbe04 ee6dbde0 c009ee24 c009ecdc
bde0: ee6dbe38 f4000100 f400010c c09e0af0 03eb c0a38a78 ee6dbe34 ee6dbe08
be00: c00094c8 c009edd4 ee4e24c0 c0701d50 20070013  ee6dbe6c ef7be600
be20: ee6da000 c09f5dc6 ee6dbe9c ee6dbe38 c00149f0 c0009488 0001 ee4e2988
be40:  60070093 20070013 ddb9799c 20070013 ee6dbef0 ef7be600 c09e04ec
be60: c09f5dc6 ee6dbe9c 0288 ee6dbe88 c008b60c c0701d50 20070013 
be80: 0051  ddb9799c ddb97998 ee6dbebc ee6dbea0 c0083824 c0701d20
bea0: c004e9c4 ee6e6d80 ddb97978 ef7ba940 ee6dbecc ee6dbec0 c004e9d8 c00837e8
bec0: ee6dbf2c ee6dbed0 c0050958 c004e9d0 0001  c0050898 
bee0: c0701d8c ee4e24c0 000f  c0a73e7c c0bc8834  c08947f8
bf00: 0008 ee6e6d80 ee6e6d98 ee6e6d98 0008 ef7ba940 ef7ba940 c09dd900
bf20: ee6dbf44 ee6dbf30 c0050e78 c0050774 ee6e6d80 ef7ba974 ee6dbf7c ee6dbf48
bf40: c0051094 c0050e54  ee6e8ac0 ee509eb8 ee509e80  ee6e8ac0
bf60: ee509eb8 ee6e6d80 ef0c9e58 c0050e88 ee6dbfac ee6dbf80 c0057b90 c0050e94
bf80: ee6da000 ee6e8ac0 c0057a88     
bfa0:  ee6dbfb0 c000fdf0 c0057a94    
bfc0:        
bfe0:     0013  3fffd861 3fffdc61
Backtrace:
[] 

Re: [PATCH v3 00/24] i.MX Media Driver

[Russell King - ARM Linux]

On Thu, Feb 02, 2017 at 05:22:46PM +, Russell King - ARM Linux wrote:
> I thought, maybe, it's the IPU overwriting past the end of the buffer,
> but I've added checks and that doesn't seem to have fired.  I also
> wondered if it was some kind of use-after-free of the ring, so I made
> imx_media_free_dma_buf_ring() memset the ring to 0x5a5a5a5a before
> kfree()ing it... doesn't look like it's that either.  I'm going to
> continue poking to see if I can figure out what's going on.

I take that back... here's a use-after-free of that buffer, on the
very first run:

Alignment trap: not handling instruction e1921f9f at []
Unhandled fault: alignment exception (0x001) at 0x5a5a5b5e
pgd = c0004000
[5a5a5b5e] *pgd=
Internal error: : 1 [#1] SMP ARM
Modules linked in: imx_csi(C) rfcomm bnep bluetooth nfsd imx_camif(C) imx_ic(C) 
imx_smfc(C) caam_jr snd_soc_imx_spdif snd_soc_imx_sgtl5000 
snd_soc_fsl_asoc_card imx_media(C) uvcvideo imx_mipi_csi2(C) 
imx_media_common(C) imx219 snd_soc_sgtl5000 snd_soc_imx_audmux caam 
video_multiplexer imx_sdma imx2_wdt coda v4l2_mem2mem videobuf2_v4l2 
videobuf2_dma_contig videobuf2_core rc_cec snd_soc_fsl_ssi snd_soc_fsl_spdif 
videobuf2_vmalloc videobuf2_memops imx_pcm_dma imx_thermal dw_hdmi_ahb_audio 
dw_hdmi_cec etnaviv fuse rc_pinnacle_pctv_hd
CPU: 0 PID: 99 Comm: kworker/0:3 Tainted: G C  4.10.0-rc6+ #2103
Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
Workqueue: lru-add-drain wq_barrier_func
task: ee4e24c0 task.stack: ee6da000
PC is at __lock_acquire+0xd4/0x17b0
LR is at lock_acquire+0xd8/0x250
pc : []lr : []psr: 20070193
sp : ee6dbb60  ip : 0001  fp : ee6dbbe4
r10: e9efad60  r9 : c0a70384  r8 : 
r7 : c0a38680  r6 :   r5 : ee4e24c0  r4 : c1400408
r3 :   r2 : 5a5a5b5e  r1 :   r0 : 5a5a5a5a
Flags: nzCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment none
Control: 10c5387d  Table: 3d7ec04a  DAC: 0051
Process kworker/0:3 (pid: 99, stack limit = 0xee6da210)
Stack: (0xee6dbb60 to 0xee6dc000)
bb60: c0a38680 0002 c0b9d8c4 ee4e29a8 ee6dbc04 ee6dbb80 c0089708 c0088d44
bb80: ee6dbb9c 050f c00867fc c0086728 ee6dbbf4 ee6dbba0 87eba239 c035aa2f
bba0: 0001 ee4e29a8 c00c4f84 0001 0026 0560e36b  
bbc0: 60070193 e9efad60   c0a70384  ee6dbc3c ee6dbbe8
bbe0: c008b108 c0089400 0001 0080  bf0d2a8c  
bc00: c008b108 c0089400 0001 c09e04ec  e9efad50 60070193 bf0d2a8c
bc20: 0139  c09e04ec ee6dbcec ee6dbc6c ee6dbc40 c07016f4 c008b03c
bc40: 0001  bf0d2a8c ee6dbcec ee6dbc84 e9efac00 e9efad50 ee9785c4
bc60: ee6dbc84 ee6dbc70 bf0d2a8c c07016b4 ee978410 e9efb400 ee6dbca4 ee6dbc88
bc80: bf1224b8 bf0d2a7c bf122458 ee88d4c0 e9efb400 e9efb410 ee6dbce4 ee6dbca8
bca0: c009f5dc bf122464 0001 c09e04ec  e9efb400 c009f9f8 e9efb400
bcc0: e9efb400 e9efb410  0009 f4001100 ee6dbe38 ee6dbd04 ee6dbce8
bce0: c009f984 c009f544 c0701d10  e9efb400 e9efb460 ee6dbd24 ee6dbd08
bd00: c009fa00 c009f96c c09d0418 e9efb400 e9efb460 e9efb410 ee6dbd44 ee6dbd28
bd20: c00a3174 c009f9cc c00a30c4  ee6dbd90 ee4a3010 ee6dbd54 ee6dbd48
bd40: c009ecf0 c00a30d0 ee6dbd84 ee6dbd58 c0409328 c009ecdc c09d0448 0001
bd60: 0026 ef1efc10 c09e756c ee4a3010 0026 ef008400 ee6dbdcc ee6dbd88
bd80: c0409458 c040928c 0001  0001 0002 0003 000a
bda0: 000b 000c 000d 000e ee6dbdcc c09d52d0  
bdc0: ee6dbddc ee6dbdd0 c009ecf0 c0409408 ee6dbe04 ee6dbde0 c009ee24 c009ecdc
bde0: ee6dbe38 f4000100 f400010c c09e0af0 03eb c0a38a78 ee6dbe34 ee6dbe08
be00: c00094c8 c009edd4 ee4e24c0 c0701d50 20070013  ee6dbe6c ef7be600
be20: ee6da000 c09f5dc6 ee6dbe9c ee6dbe38 c00149f0 c0009488 0001 ee4e2988
be40:  60070093 20070013 ddb9799c 20070013 ee6dbef0 ef7be600 c09e04ec
be60: c09f5dc6 ee6dbe9c 0288 ee6dbe88 c008b60c c0701d50 20070013 
be80: 0051  ddb9799c ddb97998 ee6dbebc ee6dbea0 c0083824 c0701d20
bea0: c004e9c4 ee6e6d80 ddb97978 ef7ba940 ee6dbecc ee6dbec0 c004e9d8 c00837e8
bec0: ee6dbf2c ee6dbed0 c0050958 c004e9d0 0001  c0050898 
bee0: c0701d8c ee4e24c0 000f  c0a73e7c c0bc8834  c08947f8
bf00: 0008 ee6e6d80 ee6e6d98 ee6e6d98 0008 ef7ba940 ef7ba940 c09dd900
bf20: ee6dbf44 ee6dbf30 c0050e78 c0050774 ee6e6d80 ef7ba974 ee6dbf7c ee6dbf48
bf40: c0051094 c0050e54  ee6e8ac0 ee509eb8 ee509e80  ee6e8ac0
bf60: ee509eb8 ee6e6d80 ef0c9e58 c0050e88 ee6dbfac ee6dbf80 c0057b90 c0050e94
bf80: ee6da000 ee6e8ac0 c0057a88     
bfa0:  ee6dbfb0 c000fdf0 c0057a94    
bfc0:        
bfe0:     0013  3fffd861 3fffdc61
Backtrace:
[] (__lock_acquire) from [] (lock_acquire+0xd8/0x250)
 

Re: [PATCH v3 00/24] i.MX Media Driver

[Steve Longerbeam]

Hi Russell,

I don't recommend spending too much time debugging this
OOPS. The dma buffer ring has been removed completely
in version 4 (which I'm trying to get ready to post hopefully
by end of this week).

Steve


On 02/02/2017 09:22 AM, Russell King - ARM Linux wrote:

I seem to be getting some sort of memory corruption with this driver.

I've had two instances now of uninitialised spinlocks in
imx_media_dma_buf_get_active() which show that the spinlock being
taken in this function is all-zeros.

That very quickly leads to an oops, where I've seen buf->ring is
NULL in imx_media_dma_buf_set_active().

Not quite sure what's going on, but the trigger (at least for me) is
to change my gstreamer pipeline from:

DISPLAY=:0 gst-launch-1.0 -v v4l2src device=/dev/video3 ! bayer2rgbneon ! 
xvimagesink

to

DISPLAY=:0 gst-launch-1.0 -v v4l2src device=/dev/video3 ! queue ! bayer2rgbneon 
! xvimagesink

and it seems to take as little as two or three attempts to provoke the
kernel to totally die.

I've just tried a third time.  I can run the first gstreamer command
five times.  The I ran the second command and immediately got this:

INFO: trying to register non-static key.
the code is fine but needs lockdep annotation.
turning off the locking correctness validator.
CPU: 0 PID: 1008 Comm: Xorg Tainted: G C  4.10.0-rc6+ #2103
Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
Backtrace:
[] (dump_backtrace) from [] (show_stack+0x18/0x1c)
  r6:600f0193 r5: r4: r3:
[] (show_stack) from [] (dump_stack+0xa4/0xdc)
[] (dump_stack) from [] (register_lock_class+0x1d4/0x554)
  r6:c1400408 r5: r4: r3:ee47a4c0
[] (register_lock_class) from [] 
(__lock_acquire+0x80/0x17b0)
  r10:d995f760 r9:c0a70384 r8: r7:c0a38680 r6: r5:ee47a4c0
  r4:c1400408
[] (__lock_acquire) from [] (lock_acquire+0xd8/0x250)
  r10: r9:c0a70384 r8: r7: r6:d995f760 r5:600f0193
  r4:
[] (lock_acquire) from [] (_raw_spin_lock_irqsave+0x4c/0x60)
  r10:ed501e64 r9:c09e04ec r8: r7:0139 r6:bf0d7a8c r5:600f0193
  r4:d995f750
[] (_raw_spin_lock_irqsave) from [] 
(imx_media_dma_buf_get_active+0x1c/0x94 [imx_media_common])
  r6:e98b2c10 r5:d995f750 r4:d995f600
[] (imx_media_dma_buf_get_active [imx_media_common]) from 
[] (imx_smfc_eof_interrupt+0x60/0x124 [imx_smfc])
  r5:ee935dc4 r4:ee935c10
[] (imx_smfc_eof_interrupt [imx_smfc]) from [] 
(__handle_irq_event_percpu+0xa4/0x428)
  r6:e98b2c10 r5:e98b2c00 r4:ebfb6d40 r3:bf12c458
[] (__handle_irq_event_percpu) from [] 
(handle_irq_event_percpu+0x24/0x60)
  r10:ed501fb0 r9:f4001100 r8:0009 r7: r6:e98b2c10 r5:e98b2c00
  r4:e98b2c00
[] (handle_irq_event_percpu) from [] 
(handle_irq_event+0x40/0x64)
  r5:e98b2c60 r4:e98b2c00
[] (handle_irq_event) from [] (handle_level_irq+0xb0/0x138)
  r6:e98b2c10 r5:e98b2c60 r4:e98b2c00 r3:c09d0418
[] (handle_level_irq) from [] (generic_handle_irq+0x20/0x30)
  r6:ee4a3010 r5:ed501f08 r4: r3:c00a30c4
[] (generic_handle_irq) from [] (ipu_irq_handle+0xa8/0xd8)
[] (ipu_irq_handle) from [] (ipu_irq_handler+0x5c/0xb4)
  r8:ef008400 r7:0026 r6:ee4a3010 r5:c09e756c r4:ef1efc10
[] (ipu_irq_handler) from [] (generic_handle_irq+0x20/0x30)
  r6: r5: r4:c09d52d0
[] (generic_handle_irq) from [] 
(__handle_domain_irq+0x5c/0xb8)
[] (__handle_domain_irq) from [] (gic_handle_irq+0x4c/0x9c)
  r8:c0a38a78 r7:03eb r6:c09e0af0 r5:f400010c r4:f4000100 r3:ed501fb0
[] (gic_handle_irq) from [] (__irq_usr+0x58/0x80)
Exception stack(0xed501fb0 to 0xed501ff8)
1fa0: b698b4e0  0042c000 b698c708
1fc0: 0010 81231b10 81231b18 80e89670 b698b4e0 8114957c 7f79b000 81149438
1fe0: 7f79b248 bee08b98 7f708609 b6904220 600f0030 
  r10:7f79b000 r9:8114957c r8:10c5387d r7:10c5387d r6: r5:600f0030
  r4:b6904220 r3:ee47a4c0
[ cut here ]
WARNING: CPU: 0 PID: 1008 at 
/home/rmk/git/linux-rmk/drivers/staging/media/imx/imx-smfc.c:159 
imx_smfc_eof_interrupt+0x118/0x124 [imx_smfc]
Modules linked in: imx_csi(C) rfcomm bnep bluetooth nfsd imx_camif(C) imx_ic(C) 
imx_smfc(C) caam_jr snd_soc_imx_sgtl5000 uvcvideo snd_soc_fsl_asoc_card 
snd_soc_imx_spdif imx_media(C) imx_mipi_csi2(C) imx_media_common(C) 
snd_soc_imx_audmux imx219 snd_soc_sgtl5000 caam video_multiplexer imx_sdma 
imx2_wdt rc_cec snd_soc_fsl_ssi coda v4l2_mem2mem videobuf2_v4l2 
videobuf2_dma_contig videobuf2_core snd_soc_fsl_spdif imx_pcm_dma 
videobuf2_vmalloc dw_hdmi_ahb_audio dw_hdmi_cec videobuf2_memops imx_thermal 
etnaviv fuse rc_pinnacle_pctv_hd
CPU: 0 PID: 1008 Comm: Xorg Tainted: G C  4.10.0-rc6+ #2103
Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
Backtrace:
[] (dump_backtrace) from [] (show_stack+0x18/0x1c)
  r6:600f0193 r5: r4: r3:
[] (show_stack) from [] (dump_stack+0xa4/0xdc)
[] (dump_stack) from [] (__warn+0xdc/0x108)
  r6:bf12d004 r5: r4: r3:ee47a4c0
[] 

Re: [PATCH v3 00/24] i.MX Media Driver

[Russell King - ARM Linux]

I seem to be getting some sort of memory corruption with this driver.

I've had two instances now of uninitialised spinlocks in
imx_media_dma_buf_get_active() which show that the spinlock being
taken in this function is all-zeros.

That very quickly leads to an oops, where I've seen buf->ring is
NULL in imx_media_dma_buf_set_active().

Not quite sure what's going on, but the trigger (at least for me) is
to change my gstreamer pipeline from:

DISPLAY=:0 gst-launch-1.0 -v v4l2src device=/dev/video3 ! bayer2rgbneon ! 
xvimagesink

to

DISPLAY=:0 gst-launch-1.0 -v v4l2src device=/dev/video3 ! queue ! bayer2rgbneon 
! xvimagesink

and it seems to take as little as two or three attempts to provoke the
kernel to totally die.

I've just tried a third time.  I can run the first gstreamer command
five times.  The I ran the second command and immediately got this:

INFO: trying to register non-static key.
the code is fine but needs lockdep annotation.
turning off the locking correctness validator.
CPU: 0 PID: 1008 Comm: Xorg Tainted: G C  4.10.0-rc6+ #2103
Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
Backtrace:
[] (dump_backtrace) from [] (show_stack+0x18/0x1c)
 r6:600f0193 r5: r4: r3:
[] (show_stack) from [] (dump_stack+0xa4/0xdc)
[] (dump_stack) from [] (register_lock_class+0x1d4/0x554)
 r6:c1400408 r5: r4: r3:ee47a4c0
[] (register_lock_class) from [] 
(__lock_acquire+0x80/0x17b0)
 r10:d995f760 r9:c0a70384 r8: r7:c0a38680 r6: r5:ee47a4c0
 r4:c1400408
[] (__lock_acquire) from [] (lock_acquire+0xd8/0x250)
 r10: r9:c0a70384 r8: r7: r6:d995f760 r5:600f0193
 r4:
[] (lock_acquire) from [] (_raw_spin_lock_irqsave+0x4c/0x60)
 r10:ed501e64 r9:c09e04ec r8: r7:0139 r6:bf0d7a8c r5:600f0193
 r4:d995f750
[] (_raw_spin_lock_irqsave) from [] 
(imx_media_dma_buf_get_active+0x1c/0x94 [imx_media_common])
 r6:e98b2c10 r5:d995f750 r4:d995f600
[] (imx_media_dma_buf_get_active [imx_media_common]) from 
[] (imx_smfc_eof_interrupt+0x60/0x124 [imx_smfc])
 r5:ee935dc4 r4:ee935c10
[] (imx_smfc_eof_interrupt [imx_smfc]) from [] 
(__handle_irq_event_percpu+0xa4/0x428)
 r6:e98b2c10 r5:e98b2c00 r4:ebfb6d40 r3:bf12c458
[] (__handle_irq_event_percpu) from [] 
(handle_irq_event_percpu+0x24/0x60)
 r10:ed501fb0 r9:f4001100 r8:0009 r7: r6:e98b2c10 r5:e98b2c00
 r4:e98b2c00
[] (handle_irq_event_percpu) from [] 
(handle_irq_event+0x40/0x64)
 r5:e98b2c60 r4:e98b2c00
[] (handle_irq_event) from [] (handle_level_irq+0xb0/0x138)
 r6:e98b2c10 r5:e98b2c60 r4:e98b2c00 r3:c09d0418
[] (handle_level_irq) from [] (generic_handle_irq+0x20/0x30)
 r6:ee4a3010 r5:ed501f08 r4: r3:c00a30c4
[] (generic_handle_irq) from [] (ipu_irq_handle+0xa8/0xd8)
[] (ipu_irq_handle) from [] (ipu_irq_handler+0x5c/0xb4)
 r8:ef008400 r7:0026 r6:ee4a3010 r5:c09e756c r4:ef1efc10
[] (ipu_irq_handler) from [] (generic_handle_irq+0x20/0x30)
 r6: r5: r4:c09d52d0
[] (generic_handle_irq) from [] 
(__handle_domain_irq+0x5c/0xb8)
[] (__handle_domain_irq) from [] (gic_handle_irq+0x4c/0x9c)
 r8:c0a38a78 r7:03eb r6:c09e0af0 r5:f400010c r4:f4000100 r3:ed501fb0
[] (gic_handle_irq) from [] (__irq_usr+0x58/0x80)
Exception stack(0xed501fb0 to 0xed501ff8)
1fa0: b698b4e0  0042c000 b698c708
1fc0: 0010 81231b10 81231b18 80e89670 b698b4e0 8114957c 7f79b000 81149438
1fe0: 7f79b248 bee08b98 7f708609 b6904220 600f0030 
 r10:7f79b000 r9:8114957c r8:10c5387d r7:10c5387d r6: r5:600f0030
 r4:b6904220 r3:ee47a4c0
[ cut here ]
WARNING: CPU: 0 PID: 1008 at 
/home/rmk/git/linux-rmk/drivers/staging/media/imx/imx-smfc.c:159 
imx_smfc_eof_interrupt+0x118/0x124 [imx_smfc]
Modules linked in: imx_csi(C) rfcomm bnep bluetooth nfsd imx_camif(C) imx_ic(C) 
imx_smfc(C) caam_jr snd_soc_imx_sgtl5000 uvcvideo snd_soc_fsl_asoc_card 
snd_soc_imx_spdif imx_media(C) imx_mipi_csi2(C) imx_media_common(C) 
snd_soc_imx_audmux imx219 snd_soc_sgtl5000 caam video_multiplexer imx_sdma 
imx2_wdt rc_cec snd_soc_fsl_ssi coda v4l2_mem2mem videobuf2_v4l2 
videobuf2_dma_contig videobuf2_core snd_soc_fsl_spdif imx_pcm_dma 
videobuf2_vmalloc dw_hdmi_ahb_audio dw_hdmi_cec videobuf2_memops imx_thermal 
etnaviv fuse rc_pinnacle_pctv_hd
CPU: 0 PID: 1008 Comm: Xorg Tainted: G C  4.10.0-rc6+ #2103
Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
Backtrace:
[] (dump_backtrace) from [] (show_stack+0x18/0x1c)
 r6:600f0193 r5: r4: r3:
[] (show_stack) from [] (dump_stack+0xa4/0xdc)
[] (dump_stack) from [] (__warn+0xdc/0x108)
 r6:bf12d004 r5: r4: r3:ee47a4c0
[] (__warn) from [] (warn_slowpath_null+0x28/0x30)
 r10:ed501e64 r8: r7:0139 r6:e98b2c10 r5:ee935dc4 r4:ee935c10
[] (warn_slowpath_null) from [] 
(imx_smfc_eof_interrupt+0x118/0x124 [imx_smfc])
[] (imx_smfc_eof_interrupt [imx_smfc]) from [] 
(__handle_irq_event_percpu+0xa4/0x428)
 

Re: [PATCH v3 00/24] i.MX Media Driver

[Steve Longerbeam]

On 02/01/2017 01:30 AM, Philipp Zabel wrote:

On Tue, 2017-01-31 at 17:26 -0800, Steve Longerbeam wrote:
[...]

# Set pad formats
media-ctl --set-v4l2 "'tc358743 1-000f':0[fmt:UYVY/1920x1080]"
media-ctl --set-v4l2 "'imx6-mipi-csi2':1[fmt:UYVY2X8/1920x1080]"
media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080]"
media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/1920x1080]"

v4l2-ctl -d /dev/video4 -V
# This still is configured to 640x480, which is inconsistent with
# the 'ipu1_csi0':2 pad format. The pad set_fmt above should
# have set this, too.

Because you've only configured the source pads,
and not the sink pads. The ipu_csi source format is
dependent on the sink format - output crop window is
limited by max input sensor frame, and since sink pad is
still at 640x480, output is reduced to that.

No, it is set (see below). What happens is that capture_g_fmt_vid_cap
just returns the capture devices' priv->vdev.fmt, even if it is
incompatible with the connected csi subdevice's output pad format.

priv->vdev.fmt was never changed from the default set in
imx_media_capture_device_register, because capture_s/try_fmt_vid_cap
were not called yet.


Ah, yep, this is a bug. Need to modify the capture device's
width/height at .set_fmt() in the subdev's device-node source
pad (csi and prpenc/vf).


Maybe I'm missing something, is it expected behavior that
a source format should be automatically propagated to
the sink?

media-ctl propagates the output pad format to all remote subdevices'
input pads for all enabled links:

https://git.linuxtv.org/v4l-utils.git/tree/utils/media-ctl/libv4l2subdev.c#n693


Ah cool, I wasn't aware media-ctl did this, but it makes sense and
makes it easier on the user.

Steve




v4l2-ctl --list-formats -d /dev/video4
# This lists all the RGB formats, which it shouldn't. There is
# no CSC in this pipeline, so we should be limited to YUV formats
# only.

right, need to fix that. Probably by poking the attached
source subdev (csi or prpenc/vf) for its supported formats.

You are right, in bayer/raw mode only one specific format should be
listed, depending on the CSI output pad format.

regards
Philipp



--
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 00/24] i.MX Media Driver

[Russell King - ARM Linux]

On Wed, Feb 01, 2017 at 11:42:31AM +0100, Philipp Zabel wrote:
> On Wed, 2017-02-01 at 10:11 +, Russell King - ARM Linux wrote:
> Right, it's just that in the latest version there is no v4l2_subdev for
> fifos and idmac. There is the capture interface entity that represents
> one of the IDMAC write channels, but that doesn't have a pad and format
> configuration.
> The SMFC entity was removed because the fifo can be considered part of
> the link between CSI and IDMAC. There is no manual configuration
> necessary as the SMFC itself can't do anything to the data that flows
> through it. There is no reason to present it to userspace as a no-op
> entity.
> So in the direct CSI -> SMFC -> IDMAC case, the CSI source pad now is
> the nearest neighbor pad to the IDMAC capture video device.

Ok, that sounds fine then.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
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 00/24] i.MX Media Driver

[Philipp Zabel]

On Wed, 2017-02-01 at 10:11 +, Russell King - ARM Linux wrote:
> On Wed, Feb 01, 2017 at 10:30:57AM +0100, Philipp Zabel wrote:
> > On Tue, 2017-01-31 at 17:26 -0800, Steve Longerbeam wrote:
> > [...]
> > > right, need to fix that. Probably by poking the attached
> > > source subdev (csi or prpenc/vf) for its supported formats.
> > 
> > You are right, in bayer/raw mode only one specific format should be
> > listed, depending on the CSI output pad format.
> 
> It depends what Steve means by "source subdev".
> 
> It should be the next sub-device below the bridge - if we have this
> setup of subdev's:
> 
> ---> CSI ---> SMFC ---> IDMAC
> 
> then the format configured at the SMFC's output pad is what matters,
> not what was configured at CSI.

Right, it's just that in the latest version there is no v4l2_subdev for
fifos and idmac. There is the capture interface entity that represents
one of the IDMAC write channels, but that doesn't have a pad and format
configuration.
The SMFC entity was removed because the fifo can be considered part of
the link between CSI and IDMAC. There is no manual configuration
necessary as the SMFC itself can't do anything to the data that flows
through it. There is no reason to present it to userspace as a no-op
entity.
So in the direct CSI -> SMFC -> IDMAC case, the CSI source pad now is
the nearest neighbor pad to the IDMAC capture video device.

> It's the responsibility of SMFC and CSI to make sure that their source
> pads are configured with a compatible format for their corresponding
> source pad, and it's also the sink subdev's responsibility to check
> that the configuration across a link is valid (possibly via
> v4l2_subdev_link_validate(), or a more intensive or relaxed test if
> required.)
> 
> For example:
> 
> - when CSI's source pad is configured with a RGGB output format,
>   userspace media-ctl will also set that on SMFC's sink pad.
> - when SMFC's sink pad is configured, SMFC should configure it's
>   source pad with an identical format (RGGB).
> - when SMFC's source pad is configured, it should refuse to change
>   the format, because SMFC can't modify pixel the format - it's
>   just a buffer.
>
> When starting to stream (Documentation/media/kapi/v4l2-subdev.rst) the
> link validation function is called, and:
> 
> - the SMFC driver's link_validate function will be called to validate
>   the CSI -> SMFC link.  This allows the SMFC to be sure that there's
>   a compatible configuration - and, since the link does not allow
>   format conversion, it should verify that the format on the CSI's
>   source pad is the same as SMFC's sink pad.
> 
> Not only does this match what the hardware's doing, it also means that,
> because there's no format conversion between the CSI's hardware output
> and IDMAC, we don't need to care about trying to fetch the CSI's source
> pad configuration from the IDMAC end - we can fetch that information
> from our neighbour's SMFC's source pad _or_ our own sink pad if we have
> one.

See above. All this is correct for the remaining entities, just the CSI
source pad now takes the role of the SMFC source pad as nearest neighbor
to the IDMAC capture video device.

> To see why this is an important, consider what the effect would be if
> SMFC did have the capability to change the pixel format.  That means the
> format presented to the IDMAC block would depend on the configuration of
> SMFC, and the CSI's source pad format is no longer relevant to IDMAC.

Yes, this is exactly the case for the CSI -> IC PRP -> IC PRPVF -> IDMAC
route, as the IC can do color space conversion. Here, (only) the IC
PRPVF source pad should determine the capture video device's format, and
the negotiation between CSI->IC PRP and between IC PRP->IC PRPVF should
happen as you say.

regards
Philipp

--
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 00/24] i.MX Media Driver

[Russell King - ARM Linux]

On Wed, Feb 01, 2017 at 10:30:57AM +0100, Philipp Zabel wrote:
> On Tue, 2017-01-31 at 17:26 -0800, Steve Longerbeam wrote:
> [...]
> > right, need to fix that. Probably by poking the attached
> > source subdev (csi or prpenc/vf) for its supported formats.
> 
> You are right, in bayer/raw mode only one specific format should be
> listed, depending on the CSI output pad format.

It depends what Steve means by "source subdev".

It should be the next sub-device below the bridge - if we have this
setup of subdev's:

---> CSI ---> SMFC ---> IDMAC

then the format configured at the SMFC's output pad is what matters,
not what was configured at CSI.

It's the responsibility of SMFC and CSI to make sure that their source
pads are configured with a compatible format for their corresponding
source pad, and it's also the sink subdev's responsibility to check
that the configuration across a link is valid (possibly via
v4l2_subdev_link_validate(), or a more intensive or relaxed test if
required.)

For example:

- when CSI's source pad is configured with a RGGB output format,
  userspace media-ctl will also set that on SMFC's sink pad.
- when SMFC's sink pad is configured, SMFC should configure it's
  source pad with an identical format (RGGB).
- when SMFC's source pad is configured, it should refuse to change
  the format, because SMFC can't modify pixel the format - it's
  just a buffer.

When starting to stream (Documentation/media/kapi/v4l2-subdev.rst) the
link validation function is called, and:

- the SMFC driver's link_validate function will be called to validate
  the CSI -> SMFC link.  This allows the SMFC to be sure that there's
  a compatible configuration - and, since the link does not allow
  format conversion, it should verify that the format on the CSI's
  source pad is the same as SMFC's sink pad.

Not only does this match what the hardware's doing, it also means that,
because there's no format conversion between the CSI's hardware output
and IDMAC, we don't need to care about trying to fetch the CSI's source
pad configuration from the IDMAC end - we can fetch that information
from our neighbour's SMFC's source pad _or_ our own sink pad if we have
one.

To see why this is an important, consider what the effect would be if
SMFC did have the capability to change the pixel format.  That means the
format presented to the IDMAC block would depend on the configuration of
SMFC, and the CSI's source pad format is no longer relevant to IDMAC.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
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 00/24] i.MX Media Driver

[Philipp Zabel]

On Tue, 2017-01-31 at 17:26 -0800, Steve Longerbeam wrote:
[...]
> > # Set pad formats
> > media-ctl --set-v4l2 "'tc358743 1-000f':0[fmt:UYVY/1920x1080]"
> > media-ctl --set-v4l2 "'imx6-mipi-csi2':1[fmt:UYVY2X8/1920x1080]"
> > media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080]"
> > media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/1920x1080]"
> >
> > v4l2-ctl -d /dev/video4 -V
> > # This still is configured to 640x480, which is inconsistent with
> > # the 'ipu1_csi0':2 pad format. The pad set_fmt above should
> > # have set this, too.
> 
> Because you've only configured the source pads,
> and not the sink pads. The ipu_csi source format is
> dependent on the sink format - output crop window is
> limited by max input sensor frame, and since sink pad is
> still at 640x480, output is reduced to that.

No, it is set (see below). What happens is that capture_g_fmt_vid_cap
just returns the capture devices' priv->vdev.fmt, even if it is
incompatible with the connected csi subdevice's output pad format.

priv->vdev.fmt was never changed from the default set in
imx_media_capture_device_register, because capture_s/try_fmt_vid_cap
were not called yet.

> Maybe I'm missing something, is it expected behavior that
> a source format should be automatically propagated to
> the sink?

media-ctl propagates the output pad format to all remote subdevices'
input pads for all enabled links:

https://git.linuxtv.org/v4l-utils.git/tree/utils/media-ctl/libv4l2subdev.c#n693

> > v4l2-ctl --list-formats -d /dev/video4
> > # This lists all the RGB formats, which it shouldn't. There is
> > # no CSC in this pipeline, so we should be limited to YUV formats
> > # only.
> 
> right, need to fix that. Probably by poking the attached
> source subdev (csi or prpenc/vf) for its supported formats.

You are right, in bayer/raw mode only one specific format should be
listed, depending on the CSI output pad format.

regards
Philipp

--
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 00/24] i.MX Media Driver

[Russell King - ARM Linux]

On Tue, Jan 31, 2017 at 05:54:52PM -0800, Steve Longerbeam wrote:
> On 01/31/2017 04:23 PM, Russell King - ARM Linux wrote:
> First, thank you for the explanation, it clears up a lot.
> 
> But of_parse_subdev() is used to parse the OF graph starting
> from the CSI ports, to discover all the nodes to add to subdev
> async registration. It also forms the media link info to be used
> later to create the media graph, after all discovered subdevs
> have come online (registered themselves). This happens at
> driver load time, it doesn't have anything to do with pad
> negotiation.

Right, but I'm discussing why you need to know which is the sensor
subdev, and why you need to get parameters from the sensor subdev.

> >   I think the
> >issue here is how you're going about dealing with the subdevs.
> >Here's the subdev setup:
> >
> >+-camera+
> >| pixel array -> binner |---> csi2 --> ipu1csi0 mux --> csi0 --> smfc --> 
> >idmac
> >+---+
> >
> >How the subdevs are supposed to work is that you start from the first
> >subdev in sequence (in this case the pixel array) and negotiate with
> >the driver through the TRY formats on its source pad, as well as
> >negotiating with the sink pad of the directly neighbouring subdev.
> >
> >The neighbouring subdev propagates the configuration in a driver
> >specific manner from its source pad to the sink pad, giving a default
> >configuration at its source.
> 
> Let me try to re-phrase. You mean the subdev's set_fmt(), when
> configured  its source pad(s), should call set_fmt() at the connected
> sink subdev to automatically propagate the format to the sink's pad?

No.  For any individual subdev, if you configure it's _sink_ then it
should propagate the configuration to its _source_, potentially
modifying the configuration according to its function.  It should
never forward the configuration to the other side of any links.

The responsibility for setting up the neighbours source pad is the
userspace media application.

See Documentation/media/uapi/v4l/dev-subdev.rst and the section
named "Format Negotiation".

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
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 00/24] i.MX Media Driver

[Steve Longerbeam]

On 01/31/2017 04:23 PM, Russell King - ARM Linux wrote:

On Tue, Jan 31, 2017 at 03:43:22PM -0800, Steve Longerbeam wrote:


On 01/31/2017 03:00 AM, Russell King - ARM Linux wrote:

Just like smiapp, the camera sensor block (which is the very far end
of the pipeline) is marked with MEDIA_ENT_F_CAM_SENSOR.  However, in
front of that is the binner, which just like smiapp gets a separate
entity.  It's this entity which is connected to the mipi-csi2 subdev.

wow, ok got it.

So the sensor pipeline and binner, and the OF graph connecting
them, are described in the device tree I presume.

No - because the binner and sensor are on the same die, it's even
one single device, there's no real separation of the two devices.

The reason there's no real separation is because the binning is done
as part of the process of reading the array - sometimes before the
analog voltage is converted to its digital pixel value representation.

The separation comes because of the requirements of the v4l2 media
subdevs, which requires scalers to have a sink pad and a source pad.
(Please see the v4l2 documentation, I think I've already quoted this:

..  _MEDIA-ENT-F-PROC-VIDEO-SCALER:

-  ``MEDIA_ENT_F_PROC_VIDEO_SCALER``

-  Video scaler. An entity capable of video scaling must have
   at least one sink pad and one source pad, and scale the
   video frame(s) received on its sink pad(s) to a different
   resolution output on its source pad(s). The range of
   supported scaling ratios is entity-specific and can differ
   between the horizontal and vertical directions (in particular
   scaling can be supported in one direction only). Binning and
   skipping are considered as scaling.

(Oh yes, I see it was the mail to which you were replying to...)

So, in order to configure the scaling (which can be none, /2 and /4)
we have to expose two subdevs - one which is the scaler, and has a
source pad connected to the upstream (in this case CSI2 receiver)
and a sink pad immutably connected to the camera sensor.

Since the split is entirely down to the V4L2 implementation requirements,
it's not something that should be reflected in DT - we don't put
implementation details in DT.

It's just the same (as I've already said) as the SMIAPP camera driver,
the reason I'm pointing you towards that is because this is an already
mainlined camera driver which nicely illustrates how my driver is
structured from the v4l2 subdev API point of view.


The OF graph AFAIK, has no information about which ports are sinks
and which are sources, so of_parse_subdev() tries to determine that
based on the compatible string of the device node. So ATM
of_parse_subdev() assumes there is nothing but the imx6-mipi-csi2,
video-multiplexer, and camera sensors upstream from the CSI ports
in the OF graph.

I realize that's not a robust solution, and is the reason for the
"no sensor attached" below.

Is there any way to determine from the OF graph the data-direction
of a port (whether it is a sink or a source)? If so it will make
of_parse_subdev() much more robust.

I'm not sure why you need to know the data direction.


First, thank you for the explanation, it clears up a lot.

But of_parse_subdev() is used to parse the OF graph starting
from the CSI ports, to discover all the nodes to add to subdev
async registration. It also forms the media link info to be used
later to create the media graph, after all discovered subdevs
have come online (registered themselves). This happens at
driver load time, it doesn't have anything to do with pad
negotiation.


   I think the
issue here is how you're going about dealing with the subdevs.
Here's the subdev setup:

+-camera+
| pixel array -> binner |---> csi2 --> ipu1csi0 mux --> csi0 --> smfc --> idmac
+---+

How the subdevs are supposed to work is that you start from the first
subdev in sequence (in this case the pixel array) and negotiate with
the driver through the TRY formats on its source pad, as well as
negotiating with the sink pad of the directly neighbouring subdev.

The neighbouring subdev propagates the configuration in a driver
specific manner from its source pad to the sink pad, giving a default
configuration at its source.


Let me try to re-phrase. You mean the subdev's set_fmt(), when
configured  its source pad(s), should call set_fmt() at the connected
sink subdev to automatically propagate the format to the sink's pad?



This process repeats throughout the chain all the way up to the bridge
device.

Now, where things go wrong is that you want to know what each type of
these subdevs are, and the reason you want that is so you can do this
(for example - I know similar stuff happens with the "sensor" stuff
further up the chain as well):

+-camera+
| pixel array -> binner |---> csi2 --> ipu1csi0 mux --> csi0 --> smfc --> idmac
+---+|

Re: [PATCH v3 00/24] i.MX Media Driver

[Steve Longerbeam]

On 01/31/2017 05:54 AM, Philipp Zabel wrote:

Hi Steve,

I have just tested the imx-media-staging-md-wip branch on a Nitrogen6X
with a tc358743 (BD_HDMI_MIPI HDMI to MIPI CSI-2 receiver board). Some
observations:

# Link pipeline
media-ctl -l "'tc358743 1-000f':0->'imx6-mipi-csi2':0[1]"
media-ctl -l "'imx6-mipi-csi2':1->'ipu1_csi0_mux':0[1]"
media-ctl -l "'ipu1_csi0_mux':2->'ipu1_csi0':0[1]"
media-ctl -l "'ipu1_csi0':2->'ipu1_csi0 capture':0[1]"

# Provide an EDID to the HDMI source
v4l2-ctl -d /dev/v4l-subdev2 --set-edid=file=edid-1080p.hex
# At this point the HDMI source is enabled and sends a 1080p60 signal
# Configure detected DV timings
media-ctl --set-dv "'tc358743 1-000f':0"

# Set pad formats
media-ctl --set-v4l2 "'tc358743 1-000f':0[fmt:UYVY/1920x1080]"
media-ctl --set-v4l2 "'imx6-mipi-csi2':1[fmt:UYVY2X8/1920x1080]"
media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080]"
media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/1920x1080]"

v4l2-ctl -d /dev/video4 -V
# This still is configured to 640x480, which is inconsistent with
# the 'ipu1_csi0':2 pad format. The pad set_fmt above should
# have set this, too.


Because you've only configured the source pads,
and not the sink pads. The ipu_csi source format is
dependent on the sink format - output crop window is
limited by max input sensor frame, and since sink pad is
still at 640x480, output is reduced to that.

Maybe I'm missing something, is it expected behavior that
a source format should be automatically propagated to
the sink?



v4l2-ctl --list-formats -d /dev/video4
# This lists all the RGB formats, which it shouldn't. There is
# no CSC in this pipeline, so we should be limited to YUV formats
# only.


right, need to fix that. Probably by poking the attached
source subdev (csi or prpenc/vf) for its supported formats.




# Set capture format
v4l2-ctl -d /dev/video4 -v width=1920,height=1080,pixelformat=UYVY

v4l2-ctl -d /dev/video4 -V
# Now the capture format is correctly configured to 1920x1080.

v4l2-ctl -d 4 --list-frameintervals=width=1920,height=1080,pixelformat=UYVY
# This lists nothing. We should at least provide the 'ipu1_csi0':2 pad
# frame interval. In the future this should list fractions achievable
# via frame skipping.


yes, need to implement g_frame_interval.


v4l2-compliance -d /dev/video4
# This fails two tests:
# fail: v4l2-test-input-output.cpp(383): std == 0
# fail: v4l2-test-input-output.cpp(449): invalid attributes for input 0
# test VIDIOC_G/S/ENUMINPUT: FAIL
# and
# fail: v4l2-test-controls.cpp(782): subscribe event for control 'User 
Controls' failed
# test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: FAIL

# (Slowly) stream JPEG images to a display host:
gst-launch-1.0 -v v4l2src device=/dev/video4 ! jpegenc ! rtpjpegpay ! udpsink

I've done this a few times, and sometimes I only get a "ipu1_csi0: EOF
timeout" message when starting streaming.


It's hard to say what is going on there, it would be great if I could get my
hands on a Nitrogen6X with the tc35874 to help you debug.

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 v3 00/24] i.MX Media Driver

[Russell King - ARM Linux]

On Tue, Jan 31, 2017 at 03:43:22PM -0800, Steve Longerbeam wrote:
> 
> 
> On 01/31/2017 03:00 AM, Russell King - ARM Linux wrote:
> >Just like smiapp, the camera sensor block (which is the very far end
> >of the pipeline) is marked with MEDIA_ENT_F_CAM_SENSOR.  However, in
> >front of that is the binner, which just like smiapp gets a separate
> >entity.  It's this entity which is connected to the mipi-csi2 subdev.
> 
> wow, ok got it.
> 
> So the sensor pipeline and binner, and the OF graph connecting
> them, are described in the device tree I presume.

No - because the binner and sensor are on the same die, it's even
one single device, there's no real separation of the two devices.

The reason there's no real separation is because the binning is done
as part of the process of reading the array - sometimes before the
analog voltage is converted to its digital pixel value representation.

The separation comes because of the requirements of the v4l2 media
subdevs, which requires scalers to have a sink pad and a source pad.
(Please see the v4l2 documentation, I think I've already quoted this:

   ..  _MEDIA-ENT-F-PROC-VIDEO-SCALER:

   -  ``MEDIA_ENT_F_PROC_VIDEO_SCALER``

   -  Video scaler. An entity capable of video scaling must have
  at least one sink pad and one source pad, and scale the
  video frame(s) received on its sink pad(s) to a different
  resolution output on its source pad(s). The range of
  supported scaling ratios is entity-specific and can differ
  between the horizontal and vertical directions (in particular
  scaling can be supported in one direction only). Binning and
  skipping are considered as scaling.

(Oh yes, I see it was the mail to which you were replying to...)

So, in order to configure the scaling (which can be none, /2 and /4)
we have to expose two subdevs - one which is the scaler, and has a
source pad connected to the upstream (in this case CSI2 receiver)
and a sink pad immutably connected to the camera sensor.

Since the split is entirely down to the V4L2 implementation requirements,
it's not something that should be reflected in DT - we don't put
implementation details in DT.

It's just the same (as I've already said) as the SMIAPP camera driver,
the reason I'm pointing you towards that is because this is an already
mainlined camera driver which nicely illustrates how my driver is
structured from the v4l2 subdev API point of view.

> The OF graph AFAIK, has no information about which ports are sinks
> and which are sources, so of_parse_subdev() tries to determine that
> based on the compatible string of the device node. So ATM
> of_parse_subdev() assumes there is nothing but the imx6-mipi-csi2,
> video-multiplexer, and camera sensors upstream from the CSI ports
> in the OF graph.
> 
> I realize that's not a robust solution, and is the reason for the
> "no sensor attached" below.
> 
> Is there any way to determine from the OF graph the data-direction
> of a port (whether it is a sink or a source)? If so it will make
> of_parse_subdev() much more robust.

I'm not sure why you need to know the data direction.  I think the
issue here is how you're going about dealing with the subdevs.
Here's the subdev setup:

+-camera+
| pixel array -> binner |---> csi2 --> ipu1csi0 mux --> csi0 --> smfc --> idmac
+---+

How the subdevs are supposed to work is that you start from the first
subdev in sequence (in this case the pixel array) and negotiate with
the driver through the TRY formats on its source pad, as well as
negotiating with the sink pad of the directly neighbouring subdev.

The neighbouring subdev propagates the configuration in a driver
specific manner from its source pad to the sink pad, giving a default
configuration at its source.

This process repeats throughout the chain all the way up to the bridge
device.

Now, where things go wrong is that you want to know what each type of
these subdevs are, and the reason you want that is so you can do this
(for example - I know similar stuff happens with the "sensor" stuff
further up the chain as well):

+-camera+
| pixel array -> binner |---> csi2 --> ipu1csi0 mux --> csi0 --> smfc --> idmac
+---+|
  ^--I-want-your-bus-format-and-frame-rate---'

which goes against the negotiation mechanism of v4l2 subdevs.  This
is broken - it bypass the subdev negotiation that has been performed
on the intervening subdevs between the "sensor" and the csi0 subdevs,
so if there were a subdev in that chain that (eg) reduced the frame
rate by discarding the odd frames, you'd be working with the wrong
frame interval for your frame interval monitor at csi.

Note that the "MEDIA_ENT_F_PROC_VIDEO_SCALER" subdev type is documented
as not only supports scaling as in changing the size of the image, but
also in terms of skipping frames, which means a reduction in frame rate.

Re: [PATCH v3 00/24] i.MX Media Driver

[Steve Longerbeam]

On 01/31/2017 03:00 AM, Russell King - ARM Linux wrote:

On Mon, Jan 30, 2017 at 05:22:01PM -0800, Steve Longerbeam wrote:

I'm also having trouble finding a datasheet for it, but from what
I've read, it has a MIPI CSI-2 interface. It should work fine as long
as it presents a single source pad, registers asynchronously, and
sets its entity function to MEDIA_ENT_F_CAM_SENSOR.

Yes, it is MIPI CSI-2, and yes it has a single source pad, registers
asynchronously, but that's about as far as it goes.

The structure is a camera sensor followed by some processing.  So just
like the smiapp code, I've ended up with multiple subdevs describing
each stage of the sensors pipeline.

Just like smiapp, the camera sensor block (which is the very far end
of the pipeline) is marked with MEDIA_ENT_F_CAM_SENSOR.  However, in
front of that is the binner, which just like smiapp gets a separate
entity.  It's this entity which is connected to the mipi-csi2 subdev.


wow, ok got it.

So the sensor pipeline and binner, and the OF graph connecting
them, are described in the device tree I presume.

The OF graph AFAIK, has no information about which ports are sinks
and which are sources, so of_parse_subdev() tries to determine that
based on the compatible string of the device node. So ATM
of_parse_subdev() assumes there is nothing but the imx6-mipi-csi2,
video-multiplexer, and camera sensors upstream from the CSI ports
in the OF graph.

I realize that's not a robust solution, and is the reason for the
"no sensor attached" below.

Is there any way to determine from the OF graph the data-direction
of a port (whether it is a sink or a source)? If so it will make
of_parse_subdev() much more robust.

Steve



Unlike smiapp, which does not set an entity function, I set my binner
entity as MEDIA_ENT_F_PROC_VIDEO_SCALER on the basis that that is
what V4L2 documentation recommend:

 -  ..  row 27

..  _MEDIA-ENT-F-PROC-VIDEO-SCALER:

-  ``MEDIA_ENT_F_PROC_VIDEO_SCALER``

-  Video scaler. An entity capable of video scaling must have
   at least one sink pad and one source pad, and scale the
   video frame(s) received on its sink pad(s) to a different
   resolution output on its source pad(s). The range of
   supported scaling ratios is entity-specific and can differ
   between the horizontal and vertical directions (in particular
   scaling can be supported in one direction only). Binning and
   skipping are considered as scaling.

This causes attempts to configure the ipu1_csi0 interface to fail:

media-ctl -v -d /dev/media1 --set-v4l2 '"ipu1_csi0":1[fmt:SGBRG8/512x512@1/30]'
Opening media device /dev/media1
Enumerating entities
Found 29 entities
Enumerating pads and links
Setting up format SGBRG8 512x512 on pad ipu1_csi0/1
Unable to set format: No such device (-19)
Unable to setup formats: No such device (19)

and in the kernel log:

ipu1_csi0: no sensor attached

And yes, I already know that my next problem is going to be that the bayer
formats are not supported in your driver (just like Philipp's driver) but
adding them should not be difficult... but only once this issue is resolved.



--
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 00/24] i.MX Media Driver

[Russell King - ARM Linux]

On Tue, Jan 31, 2017 at 03:25:10PM +0100, Philipp Zabel wrote:
> On Tue, 2017-01-31 at 14:54 +0100, Philipp Zabel wrote:
> > Hi Steve,
> > 
> > I have just tested the imx-media-staging-md-wip branch on a Nitrogen6X
> > with a tc358743 (BD_HDMI_MIPI HDMI to MIPI CSI-2 receiver board). Some
> > observations:
> > 
> > # Link pipeline
> > media-ctl -l "'tc358743 1-000f':0->'imx6-mipi-csi2':0[1]"
> > media-ctl -l "'imx6-mipi-csi2':1->'ipu1_csi0_mux':0[1]"
> > media-ctl -l "'ipu1_csi0_mux':2->'ipu1_csi0':0[1]"
> > media-ctl -l "'ipu1_csi0':2->'ipu1_csi0 capture':0[1]"
> > 
> > # Provide an EDID to the HDMI source
> > v4l2-ctl -d /dev/v4l-subdev2 --set-edid=file=edid-1080p.hex
> > # At this point the HDMI source is enabled and sends a 1080p60 signal
> > # Configure detected DV timings
> > media-ctl --set-dv "'tc358743 1-000f':0"
> > 
> > # Set pad formats
> > media-ctl --set-v4l2 "'tc358743 1-000f':0[fmt:UYVY/1920x1080]"
> > media-ctl --set-v4l2 "'imx6-mipi-csi2':1[fmt:UYVY2X8/1920x1080]"
> > media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080]"
> 
> I noticed this seems to get ignored. The format is incorrectly set to
> UYVY even though I request UYVY2X8 (see CSI2IPU chapter, Figure 19-10.
> YUV422-8 data reception in the reference manual), but it seems to work
> anyway.

That's because the driver at imx-csi level bypasses the proper media pad
formats on its sink pads, and instead goes poking about at the "sensor"
to get the format.

This is one of the reasons it wants to know which entity is the "sensor".

The "sensor" stuff in there needs to be scrapped...

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
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 00/24] i.MX Media Driver

[Philipp Zabel]

On Tue, 2017-01-31 at 14:54 +0100, Philipp Zabel wrote:
> Hi Steve,
> 
> I have just tested the imx-media-staging-md-wip branch on a Nitrogen6X
> with a tc358743 (BD_HDMI_MIPI HDMI to MIPI CSI-2 receiver board). Some
> observations:
> 
> # Link pipeline
> media-ctl -l "'tc358743 1-000f':0->'imx6-mipi-csi2':0[1]"
> media-ctl -l "'imx6-mipi-csi2':1->'ipu1_csi0_mux':0[1]"
> media-ctl -l "'ipu1_csi0_mux':2->'ipu1_csi0':0[1]"
> media-ctl -l "'ipu1_csi0':2->'ipu1_csi0 capture':0[1]"
> 
> # Provide an EDID to the HDMI source
> v4l2-ctl -d /dev/v4l-subdev2 --set-edid=file=edid-1080p.hex
> # At this point the HDMI source is enabled and sends a 1080p60 signal
> # Configure detected DV timings
> media-ctl --set-dv "'tc358743 1-000f':0"
> 
> # Set pad formats
> media-ctl --set-v4l2 "'tc358743 1-000f':0[fmt:UYVY/1920x1080]"
> media-ctl --set-v4l2 "'imx6-mipi-csi2':1[fmt:UYVY2X8/1920x1080]"
> media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080]"

I noticed this seems to get ignored. The format is incorrectly set to
UYVY even though I request UYVY2X8 (see CSI2IPU chapter, Figure 19-10.
YUV422-8 data reception in the reference manual), but it seems to work
anyway.

regards
Philipp
--
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 00/24] i.MX Media Driver

[Russell King - ARM Linux]

On Tue, Jan 31, 2017 at 02:35:00PM +0100, Philipp Zabel wrote:
> On Tue, 2017-01-31 at 13:14 +, Russell King - ARM Linux wrote:
> > This isn't limited to the serial side - the parallel bus side between
> > the CSI2 interface and CSI2IPU wrapper, and the CSI2IPU wrapper and
> > the CS0/1 interfaces is much the same with 10-bit bayer.
> > 
> > Think of the CSI2 <-> CSI2IPU bit as the 4-lane case, lane 0 ending
> > up on the least significant 8 bits of the 32-bit bus, lane 3 on the
> > top 8-bits.
> > 
> > Post CSI2IPU, it talks about a 16-bit bus in the diagrams, so that's
> > kind of the 2-lane case above...
> 
> You are right, on the parallel buses the format most definitely is not
> MEDIA_BUS_FMT_SBGGR10_1X10. We don't have any representation of the
> 32-bit bus between CSI2 host and CSI2IPU gasket because we model the two
> as a single entity, but the four 16-bit parallel buses between the
> CSI2IPU gasket and the IPU1/2 CSI0/1 probably should be set to a custom
> format describing this accurately.

Yep.  I should also point out that there's a very odd transformation
going on somewhere, and I don't yet know where.

The sensor is definitely outputting GBRG format, but what seems to get
written into memory is RGGB format.  It's somewhere post CSI, because
when I was using the (broken) CSI compander with 10 bit bayer, the red
compander channel affected the red channel output from the camera, but
changed the green component written to memory... it's very much like
either the first line gets lost somewhere, or the odd/even lines are
transposed.  It could also be a gstreamer bug.  As I say, it's not
something I've looked into deeply enough yet... too many other issues
to chase down!

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
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 00/24] i.MX Media Driver

[Philipp Zabel]

Hi Steve,

I have just tested the imx-media-staging-md-wip branch on a Nitrogen6X
with a tc358743 (BD_HDMI_MIPI HDMI to MIPI CSI-2 receiver board). Some
observations:

# Link pipeline
media-ctl -l "'tc358743 1-000f':0->'imx6-mipi-csi2':0[1]"
media-ctl -l "'imx6-mipi-csi2':1->'ipu1_csi0_mux':0[1]"
media-ctl -l "'ipu1_csi0_mux':2->'ipu1_csi0':0[1]"
media-ctl -l "'ipu1_csi0':2->'ipu1_csi0 capture':0[1]"

# Provide an EDID to the HDMI source
v4l2-ctl -d /dev/v4l-subdev2 --set-edid=file=edid-1080p.hex
# At this point the HDMI source is enabled and sends a 1080p60 signal
# Configure detected DV timings
media-ctl --set-dv "'tc358743 1-000f':0"

# Set pad formats
media-ctl --set-v4l2 "'tc358743 1-000f':0[fmt:UYVY/1920x1080]"
media-ctl --set-v4l2 "'imx6-mipi-csi2':1[fmt:UYVY2X8/1920x1080]"
media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080]"
media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/1920x1080]"

v4l2-ctl -d /dev/video4 -V
# This still is configured to 640x480, which is inconsistent with
# the 'ipu1_csi0':2 pad format. The pad set_fmt above should
# have set this, too.

v4l2-ctl --list-formats -d /dev/video4
# This lists all the RGB formats, which it shouldn't. There is
# no CSC in this pipeline, so we should be limited to YUV formats
# only.

# Set capture format
v4l2-ctl -d /dev/video4 -v width=1920,height=1080,pixelformat=UYVY

v4l2-ctl -d /dev/video4 -V
# Now the capture format is correctly configured to 1920x1080.

v4l2-ctl -d 4 --list-frameintervals=width=1920,height=1080,pixelformat=UYVY
# This lists nothing. We should at least provide the 'ipu1_csi0':2 pad
# frame interval. In the future this should list fractions achievable
# via frame skipping.

v4l2-compliance -d /dev/video4
# This fails two tests:
# fail: v4l2-test-input-output.cpp(383): std == 0
# fail: v4l2-test-input-output.cpp(449): invalid attributes for input 0
# test VIDIOC_G/S/ENUMINPUT: FAIL
# and
# fail: v4l2-test-controls.cpp(782): subscribe event for control 'User 
Controls' failed
# test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: FAIL

# (Slowly) stream JPEG images to a display host:
gst-launch-1.0 -v v4l2src device=/dev/video4 ! jpegenc ! rtpjpegpay ! udpsink

I've done this a few times, and sometimes I only get a "ipu1_csi0: EOF
timeout" message when starting streaming.

regards
Philipp

--
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 00/24] i.MX Media Driver

[Philipp Zabel]

On Tue, 2017-01-31 at 13:14 +, Russell King - ARM Linux wrote:
> On Tue, Jan 31, 2017 at 11:09:24AM +0100, Philipp Zabel wrote:
> > On Mon, 2017-01-30 at 13:06 +, Russell King - ARM Linux wrote:
> > > To help illustrate my point, consider the difference between
> > > MEDIA_BUS_FMT_RGB565_1X16 and MEDIA_BUS_FMT_RGB565_2X8_BE or
> > > MEDIA_BUS_FMT_RGB565_2X8_LE.  RGB565_1X16 means 1 pixel over an effective
> > > 16-bit wide bus (if it's not 16-bit, then it has to be broken up into
> > > separate "samples".)  RGB565_2X8 means 1 pixel as two 8-bit samples.
> > > 
> > > So, the 10-bit bayer is 1 pixel as 1.25 bytes.  Or is it, over a serial
> > > bus.  Using the RGB565 case, 10-bit bayer over a 4 lane CSI bus becomes
> > > interesting:
> > > 
> > >   first byte  2nd 3rd
> > > lane 1P0 9:2  S0  P7 9:2
> > > lane 2P1 9:2  P4 9:2  S1
> > > lane 3P2 9:2  P5 9:2  P8 9:2
> > > lane 4P3 9:2  P6 9:2  P9 9:2
> > > 
> > > S0 = P0/P1/P2/P3 least significant two bits
> > > S1 = P4/P5/P6/P7 least significant two bits
> > > 
> > > or 2 lane CSI:
> > >   first byte  2nd 3rd 4th 5th
> > > lane 1P0 9:2  P2  S0  P5  P7
> > > lane 2P1 9:2  P3  P4  P6  S1
> > > 
> > > or 1 lane CSI:
> > > lane 1P0 P1 P2 P3 S0 P4 P5 P6 P7 S1 P8 P9 ...
> > 
> > These do look like three different media bus formats to me.
> 
> This isn't limited to the serial side - the parallel bus side between
> the CSI2 interface and CSI2IPU wrapper, and the CSI2IPU wrapper and
> the CS0/1 interfaces is much the same with 10-bit bayer.
> 
> Think of the CSI2 <-> CSI2IPU bit as the 4-lane case, lane 0 ending
> up on the least significant 8 bits of the 32-bit bus, lane 3 on the
> top 8-bits.
> 
> Post CSI2IPU, it talks about a 16-bit bus in the diagrams, so that's
> kind of the 2-lane case above...

You are right, on the parallel buses the format most definitely is not
MEDIA_BUS_FMT_SBGGR10_1X10. We don't have any representation of the
32-bit bus between CSI2 host and CSI2IPU gasket because we model the two
as a single entity, but the four 16-bit parallel buses between the
CSI2IPU gasket and the IPU1/2 CSI0/1 probably should be set to a custom
format describing this accurately.

regards
Philipp

--
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 00/24] i.MX Media Driver

[Russell King - ARM Linux]

On Tue, Jan 31, 2017 at 11:09:24AM +0100, Philipp Zabel wrote:
> On Mon, 2017-01-30 at 13:06 +, Russell King - ARM Linux wrote:
> > To help illustrate my point, consider the difference between
> > MEDIA_BUS_FMT_RGB565_1X16 and MEDIA_BUS_FMT_RGB565_2X8_BE or
> > MEDIA_BUS_FMT_RGB565_2X8_LE.  RGB565_1X16 means 1 pixel over an effective
> > 16-bit wide bus (if it's not 16-bit, then it has to be broken up into
> > separate "samples".)  RGB565_2X8 means 1 pixel as two 8-bit samples.
> > 
> > So, the 10-bit bayer is 1 pixel as 1.25 bytes.  Or is it, over a serial
> > bus.  Using the RGB565 case, 10-bit bayer over a 4 lane CSI bus becomes
> > interesting:
> > 
> > first byte  2nd 3rd
> > lane 1  P0 9:2  S0  P7 9:2
> > lane 2  P1 9:2  P4 9:2  S1
> > lane 3  P2 9:2  P5 9:2  P8 9:2
> > lane 4  P3 9:2  P6 9:2  P9 9:2
> > 
> > S0 = P0/P1/P2/P3 least significant two bits
> > S1 = P4/P5/P6/P7 least significant two bits
> > 
> > or 2 lane CSI:
> > first byte  2nd 3rd 4th 5th
> > lane 1  P0 9:2  P2  S0  P5  P7
> > lane 2  P1 9:2  P3  P4  P6  S1
> > 
> > or 1 lane CSI:
> > lane 1  P0 P1 P2 P3 S0 P4 P5 P6 P7 S1 P8 P9 ...
> 
> These do look like three different media bus formats to me.

This isn't limited to the serial side - the parallel bus side between
the CSI2 interface and CSI2IPU wrapper, and the CSI2IPU wrapper and
the CS0/1 interfaces is much the same with 10-bit bayer.

Think of the CSI2 <-> CSI2IPU bit as the 4-lane case, lane 0 ending
up on the least significant 8 bits of the 32-bit bus, lane 3 on the
top 8-bits.

Post CSI2IPU, it talks about a 16-bit bus in the diagrams, so that's
kind of the 2-lane case above...

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
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 00/24] i.MX Media Driver

[Russell King - ARM Linux]

On Mon, Jan 30, 2017 at 05:22:01PM -0800, Steve Longerbeam wrote:
> I'm also having trouble finding a datasheet for it, but from what
> I've read, it has a MIPI CSI-2 interface. It should work fine as long
> as it presents a single source pad, registers asynchronously, and
> sets its entity function to MEDIA_ENT_F_CAM_SENSOR.

Yes, it is MIPI CSI-2, and yes it has a single source pad, registers
asynchronously, but that's about as far as it goes.

The structure is a camera sensor followed by some processing.  So just
like the smiapp code, I've ended up with multiple subdevs describing
each stage of the sensors pipeline.

Just like smiapp, the camera sensor block (which is the very far end
of the pipeline) is marked with MEDIA_ENT_F_CAM_SENSOR.  However, in
front of that is the binner, which just like smiapp gets a separate
entity.  It's this entity which is connected to the mipi-csi2 subdev.

Unlike smiapp, which does not set an entity function, I set my binner
entity as MEDIA_ENT_F_PROC_VIDEO_SCALER on the basis that that is
what V4L2 documentation recommend:

-  ..  row 27

   ..  _MEDIA-ENT-F-PROC-VIDEO-SCALER:

   -  ``MEDIA_ENT_F_PROC_VIDEO_SCALER``

   -  Video scaler. An entity capable of video scaling must have
  at least one sink pad and one source pad, and scale the
  video frame(s) received on its sink pad(s) to a different
  resolution output on its source pad(s). The range of
  supported scaling ratios is entity-specific and can differ
  between the horizontal and vertical directions (in particular
  scaling can be supported in one direction only). Binning and
  skipping are considered as scaling.

This causes attempts to configure the ipu1_csi0 interface to fail:

media-ctl -v -d /dev/media1 --set-v4l2 '"ipu1_csi0":1[fmt:SGBRG8/512x512@1/30]'
Opening media device /dev/media1
Enumerating entities
Found 29 entities
Enumerating pads and links
Setting up format SGBRG8 512x512 on pad ipu1_csi0/1
Unable to set format: No such device (-19)
Unable to setup formats: No such device (19)

and in the kernel log:

ipu1_csi0: no sensor attached

And yes, I already know that my next problem is going to be that the bayer
formats are not supported in your driver (just like Philipp's driver) but
adding them should not be difficult... but only once this issue is resolved.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
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 00/24] i.MX Media Driver

[Russell King - ARM Linux]

On Mon, Jan 30, 2017 at 05:22:01PM -0800, Steve Longerbeam wrote:
> Edit: I see a subdev that is missing: the video mux. Did you enable
> CONFIG_VIDEO_MULTIPLEXER?

Yes, and that's where the problem is - the video-multiplexer is 
missing the module aliases to allow it to be automatically loaded.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
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 00/24] i.MX Media Driver

[Philipp Zabel]

On Mon, 2017-01-30 at 13:06 +, Russell King - ARM Linux wrote:
> > The central issue seems to be that I think media pad links / media bus
> > formats should describe physical links, such as parallel or serial
> > buses, and the formats of pixels flowing through them, whereas Steve
> > would like to extend them to describe software transports and in-memory
> > formats.
> 
> This probably isn't the right place to attach this comment in this
> thread, but... the issue of media bus formats matching physical buses
> is an argument that I think is already lost.

It is unfortunate that the parallel format definitions have been reused
for the MIPI logical formats, but I suppose we have to live with that.
Still, I think this is not a reason to open the floodgates and start
describing in-memory formats using MEDIA_BUS_FMT_*

Does at least the combination of logical format and number of lanes
uniquiely describe the physical format?
For the 4-lane LVDS bus formats there are JEIDA/VESA variants where just
the bit ordering is different (MEDIA_BUS_FMT_RGB888_1X7X4_SPWG,
MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA).

> For example, take the 10-bit bayer formats:
> 
> #define MEDIA_BUS_FMT_SBGGR10_1X10  0x3007
> #define MEDIA_BUS_FMT_SGBRG10_1X10  0x300e
> #define MEDIA_BUS_FMT_SGRBG10_1X10  0x300a
> #define MEDIA_BUS_FMT_SRGGB10_1X10  0x300f
> 
> These are commonly used on CSI serial buses (see the smiapp driver for
> example).  From the description at the top of the file, it says the
> 1X10 means that one pixel is transferred as one 10-bit sample.
>
> However, the format on wire is somewhat different - four pixels are
> transmitted over five bytes:
> 
>   P0  P1  P2  P3  P0  P1  P2  P3
>   8-bit   8-bit   8-bit   8-bit   2-bit   2-bit   2-bit   2-bit
> 
> This gives two problems:
> 1) it doesn't fit in any sensible kind of "one pixel transferred as
>N M-bit samples" description because the pixel/sample values
>(depending how you look at them) are broken up.
> 
> 2) changing this will probably be a user visible change, as things
>like smiapp are already in use.
> 
> So, I think what we actually have is the media bus formats describing
> the _logical_ bus format.  Yes, one pixel is transferred as one 10-bit
> sample in this case.

Yes. I suppose one way to look at it is that the MIPI CSI-2 specified
formats are representations of corresponding parallel bus formats.

> To help illustrate my point, consider the difference between
> MEDIA_BUS_FMT_RGB565_1X16 and MEDIA_BUS_FMT_RGB565_2X8_BE or
> MEDIA_BUS_FMT_RGB565_2X8_LE.  RGB565_1X16 means 1 pixel over an effective
> 16-bit wide bus (if it's not 16-bit, then it has to be broken up into
> separate "samples".)  RGB565_2X8 means 1 pixel as two 8-bit samples.
> 
> So, the 10-bit bayer is 1 pixel as 1.25 bytes.  Or is it, over a serial
> bus.  Using the RGB565 case, 10-bit bayer over a 4 lane CSI bus becomes
> interesting:
> 
>   first byte  2nd 3rd
> lane 1P0 9:2  S0  P7 9:2
> lane 2P1 9:2  P4 9:2  S1
> lane 3P2 9:2  P5 9:2  P8 9:2
> lane 4P3 9:2  P6 9:2  P9 9:2
> 
> S0 = P0/P1/P2/P3 least significant two bits
> S1 = P4/P5/P6/P7 least significant two bits
> 
> or 2 lane CSI:
>   first byte  2nd 3rd 4th 5th
> lane 1P0 9:2  P2  S0  P5  P7
> lane 2P1 9:2  P3  P4  P6  S1
> 
> or 1 lane CSI:
> lane 1P0 P1 P2 P3 S0 P4 P5 P6 P7 S1 P8 P9 ...

These do look like three different media bus formats to me.

regards
Philipp

--
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 00/24] i.MX Media Driver

[Philipp Zabel]

On Mon, 2017-01-30 at 17:22 -0800, Steve Longerbeam wrote:
> 
> On 01/30/2017 04:45 PM, Russell King - ARM Linux wrote:
> >
> > Hi,
> >
> > Trying this driver with an imx219 camera (which works with Philipp's
> > driver) results in not much happening... no /dev/media* node for it,
> > no subdevs, no nothing.  No clues as to what's missing either.  Only
> > messages from imx-media are from registering the various subdevs.
> >
> > [   37.444877] imx-media: Registered subdev imx6-mipi-csi2
> > [   37.444973] imx-media: Registered subdev imx219 0-0010
> > [   38.868740] imx-media: Registered subdev ipu1_ic_prpenc
> > [   38.869265] imx-media: Registered subdev ipu1_ic_prpvf
> > [   38.869425] imx-media: Registered subdev ipu1_ic_pp0
> > [   38.870086] imx-media: Registered subdev ipu1_ic_pp1
> > [   38.871510] imx-media: Registered subdev ipu2_ic_prpenc
> > [   38.871743] imx-media: Registered subdev ipu1_smfc0
> > [   38.873043] imx-media: Registered subdev ipu1_smfc1
> > [   38.873225] imx-media: Registered subdev ipu2_ic_prpvf
> > [   38.875027] imx-media: Registered subdev ipu2_smfc0
> > [   38.875320] imx-media: Registered subdev ipu2_ic_pp0
> > [   38.877148] imx-media: Registered subdev ipu2_smfc1
> > [   38.877436] imx-media: Registered subdev ipu2_ic_pp1
> > [   38.932089] imx-media: Registered subdev camif0
> > [   38.956538] imx-media: Registered subdev camif1
> > [   38.959148] imx-media: Registered subdev camif2
> > [   38.964353] imx-media: Registered subdev camif3
> > [  206.502077] imx-media: Registered subdev ipu1_csi0
> > [  206.503304] imx-media: Registered subdev ipu1_csi1
> > [  206.503814] imx-media: Registered subdev ipu2_csi0
> > [  206.504281] imx-media: Registered subdev ipu2_csi1
> >
> > I also get:
> >
> > [   37.200072] imx6-mipi-csi2: data lanes: 2
> > [   37.200077] imx6-mipi-csi2: flags: 0x0200
> >
> > and from what I can see, all modules from drivers/staging/media/imx/ are
> > loaded (had to load imx-csi by hand because of the brokenness in the
> > drivers/gpu/ipu code attaching an device_node pointer after registering
> > the platform device, which changes what userspace sees in the modalias
> > file.)
> >
> > Any clues at what to look at?
> 
> Hi Russell,
> 
> I'm not familiar with IMX219, can you send me the source for the
> imx219 subdev? I don't see it in 4.10-rc1.
> 
> I'm also having trouble finding a datasheet for it, but from what
> I've read, it has a MIPI CSI-2 interface. It should work fine as long
> as it presents a single source pad, registers asynchronously, and
> sets its entity function to MEDIA_ENT_F_CAM_SENSOR.

What about MEDIA_ENT_F_VID_IF_BRIDGE?

regards
Philipp

--
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 00/24] i.MX Media Driver

[Steve Longerbeam]

On 01/30/2017 05:06 PM, Russell King - ARM Linux wrote:

On Tue, Jan 31, 2017 at 12:45:11AM +, Russell King - ARM Linux wrote:

Trying this driver with an imx219 camera (which works with Philipp's
driver) results in not much happening... no /dev/media* node for it,
no subdevs, no nothing.  No clues as to what's missing either.  Only
messages from imx-media are from registering the various subdevs.

Another issue:

imx_csi 5491  4
imx_camif  11654  4
imx_ic 23961  8
imx_smfc6639  4
imx_media  23308  1 imx_csi
imx_mipi_csi2   5544  1
imx_media_common   12701  6 
imx_csi,imx_smfc,imx_media,imx_mipi_csi2,imx_camif,imx_ic
imx219 21205  2

So how does one remove any of these modules, say, while developing a
camera driver?  Having to reboot to test an update makes it painfully
slow for testing.


Unload is not working yet, it's on the TODO list.

But FWIW, here's how it currently looks in version 4
(on the SabreSD):

imx_media_csi   9663  4
imx_media_ic   12688  6
imx_media_capture  10201  2 imx_media_ic,imx_media_csi
imx_media_vdic  6909  2
imx_mipi_csi2   6293  1
ov5640_mipi25988  1
imx_media  15532  0
imx_media_common   16093  6 
imx_media_ic,imx_media,imx_media_csi,imx_mipi_cs

i2,imx_media_capture,imx_media_vdic


Steve




Philipp's driver can do this (once the unload bugs are fixed, which I
have patches for).



--
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 00/24] i.MX Media Driver

[Steve Longerbeam]

On 01/30/2017 04:45 PM, Russell King - ARM Linux wrote:


Hi,

Trying this driver with an imx219 camera (which works with Philipp's
driver) results in not much happening... no /dev/media* node for it,
no subdevs, no nothing.  No clues as to what's missing either.  Only
messages from imx-media are from registering the various subdevs.

[   37.444877] imx-media: Registered subdev imx6-mipi-csi2
[   37.444973] imx-media: Registered subdev imx219 0-0010
[   38.868740] imx-media: Registered subdev ipu1_ic_prpenc
[   38.869265] imx-media: Registered subdev ipu1_ic_prpvf
[   38.869425] imx-media: Registered subdev ipu1_ic_pp0
[   38.870086] imx-media: Registered subdev ipu1_ic_pp1
[   38.871510] imx-media: Registered subdev ipu2_ic_prpenc
[   38.871743] imx-media: Registered subdev ipu1_smfc0
[   38.873043] imx-media: Registered subdev ipu1_smfc1
[   38.873225] imx-media: Registered subdev ipu2_ic_prpvf
[   38.875027] imx-media: Registered subdev ipu2_smfc0
[   38.875320] imx-media: Registered subdev ipu2_ic_pp0
[   38.877148] imx-media: Registered subdev ipu2_smfc1
[   38.877436] imx-media: Registered subdev ipu2_ic_pp1
[   38.932089] imx-media: Registered subdev camif0
[   38.956538] imx-media: Registered subdev camif1
[   38.959148] imx-media: Registered subdev camif2
[   38.964353] imx-media: Registered subdev camif3
[  206.502077] imx-media: Registered subdev ipu1_csi0
[  206.503304] imx-media: Registered subdev ipu1_csi1
[  206.503814] imx-media: Registered subdev ipu2_csi0
[  206.504281] imx-media: Registered subdev ipu2_csi1

I also get:

[   37.200072] imx6-mipi-csi2: data lanes: 2
[   37.200077] imx6-mipi-csi2: flags: 0x0200

and from what I can see, all modules from drivers/staging/media/imx/ are
loaded (had to load imx-csi by hand because of the brokenness in the
drivers/gpu/ipu code attaching an device_node pointer after registering
the platform device, which changes what userspace sees in the modalias
file.)

Any clues at what to look at?


Hi Russell,

I'm not familiar with IMX219, can you send me the source for the
imx219 subdev? I don't see it in 4.10-rc1.

I'm also having trouble finding a datasheet for it, but from what
I've read, it has a MIPI CSI-2 interface. It should work fine as long
as it presents a single source pad, registers asynchronously, and
sets its entity function to MEDIA_ENT_F_CAM_SENSOR.

Since I see it was registered asynchronously from the above, it
must have been added to the device tree. But given that there
is no /dev/media? node, the media driver is probably waiting for
another subdev to register, I don't know what that would be.

Can you send me the full patch on top of the v3 driver and I'll
try to find what's missing.

Edit: I see a subdev that is missing: the video mux. Did you enable
CONFIG_VIDEO_MULTIPLEXER?

Finally, what platform does this IMX219 sensor module plug into?


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 v3 00/24] i.MX Media Driver

[Russell King - ARM Linux]

On Tue, Jan 31, 2017 at 12:45:11AM +, Russell King - ARM Linux wrote:
> Trying this driver with an imx219 camera (which works with Philipp's
> driver) results in not much happening... no /dev/media* node for it,
> no subdevs, no nothing.  No clues as to what's missing either.  Only
> messages from imx-media are from registering the various subdevs.

Another issue:

imx_csi 5491  4
imx_camif  11654  4
imx_ic 23961  8
imx_smfc6639  4
imx_media  23308  1 imx_csi
imx_mipi_csi2   5544  1
imx_media_common   12701  6 
imx_csi,imx_smfc,imx_media,imx_mipi_csi2,imx_camif,imx_ic
imx219 21205  2

So how does one remove any of these modules, say, while developing a
camera driver?  Having to reboot to test an update makes it painfully
slow for testing.

Philipp's driver can do this (once the unload bugs are fixed, which I
have patches for).

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
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 00/24] i.MX Media Driver

[Russell King - ARM Linux]

On Fri, Jan 06, 2017 at 06:11:18PM -0800, Steve Longerbeam wrote:
> Philipp Zabel (3):
>   ARM: dts: imx6qdl: Add mipi_ipu1/2 multiplexers, mipi_csi, and their
> connections
>   add mux and video interface bridge entity functions
>   platform: add video-multiplexer subdevice driver
> 
> Steve Longerbeam (21):
>   [media] dt-bindings: Add bindings for i.MX media driver
>   ARM: dts: imx6qdl: Add compatible, clocks, irqs to MIPI CSI-2 node
>   ARM: dts: imx6qdl: add media device
>   ARM: dts: imx6qdl-sabrelite: remove erratum ERR006687 workaround
>   ARM: dts: imx6-sabrelite: add OV5642 and OV5640 camera sensors
>   ARM: dts: imx6-sabresd: add OV5642 and OV5640 camera sensors
>   ARM: dts: imx6-sabreauto: create i2cmux for i2c3
>   ARM: dts: imx6-sabreauto: add reset-gpios property for max7310_b
>   ARM: dts: imx6-sabreauto: add pinctrl for gpt input capture
>   ARM: dts: imx6-sabreauto: add the ADV7180 video decoder
>   UAPI: Add media UAPI Kbuild file
>   media: Add userspace header file for i.MX
>   media: Add i.MX media core driver
>   media: imx: Add CSI subdev driver
>   media: imx: Add SMFC subdev driver
>   media: imx: Add IC subdev drivers
>   media: imx: Add Camera Interface subdev driver
>   media: imx: Add MIPI CSI-2 Receiver subdev driver
>   media: imx: Add MIPI CSI-2 OV5640 sensor subdev driver
>   media: imx: Add Parallel OV5642 sensor subdev driver
>   ARM: imx_v6_v7_defconfig: Enable staging video4linux drivers

Hi,

Trying this driver with an imx219 camera (which works with Philipp's
driver) results in not much happening... no /dev/media* node for it,
no subdevs, no nothing.  No clues as to what's missing either.  Only
messages from imx-media are from registering the various subdevs.

[   37.444877] imx-media: Registered subdev imx6-mipi-csi2
[   37.444973] imx-media: Registered subdev imx219 0-0010
[   38.868740] imx-media: Registered subdev ipu1_ic_prpenc
[   38.869265] imx-media: Registered subdev ipu1_ic_prpvf
[   38.869425] imx-media: Registered subdev ipu1_ic_pp0
[   38.870086] imx-media: Registered subdev ipu1_ic_pp1
[   38.871510] imx-media: Registered subdev ipu2_ic_prpenc
[   38.871743] imx-media: Registered subdev ipu1_smfc0
[   38.873043] imx-media: Registered subdev ipu1_smfc1
[   38.873225] imx-media: Registered subdev ipu2_ic_prpvf
[   38.875027] imx-media: Registered subdev ipu2_smfc0
[   38.875320] imx-media: Registered subdev ipu2_ic_pp0
[   38.877148] imx-media: Registered subdev ipu2_smfc1
[   38.877436] imx-media: Registered subdev ipu2_ic_pp1
[   38.932089] imx-media: Registered subdev camif0
[   38.956538] imx-media: Registered subdev camif1
[   38.959148] imx-media: Registered subdev camif2
[   38.964353] imx-media: Registered subdev camif3
[  206.502077] imx-media: Registered subdev ipu1_csi0
[  206.503304] imx-media: Registered subdev ipu1_csi1
[  206.503814] imx-media: Registered subdev ipu2_csi0
[  206.504281] imx-media: Registered subdev ipu2_csi1

I also get:

[   37.200072] imx6-mipi-csi2: data lanes: 2
[   37.200077] imx6-mipi-csi2: flags: 0x0200

and from what I can see, all modules from drivers/staging/media/imx/ are
loaded (had to load imx-csi by hand because of the brokenness in the
drivers/gpu/ipu code attaching an device_node pointer after registering
the platform device, which changes what userspace sees in the modalias
file.)

Any clues at what to look at?

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
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 00/24] i.MX Media Driver

[Russell King - ARM Linux]

> The central issue seems to be that I think media pad links / media bus
> formats should describe physical links, such as parallel or serial
> buses, and the formats of pixels flowing through them, whereas Steve
> would like to extend them to describe software transports and in-memory
> formats.

This probably isn't the right place to attach this comment in this
thread, but... the issue of media bus formats matching physical buses
is an argument that I think is already lost.

For example, take the 10-bit bayer formats:

#define MEDIA_BUS_FMT_SBGGR10_1X10  0x3007
#define MEDIA_BUS_FMT_SGBRG10_1X10  0x300e
#define MEDIA_BUS_FMT_SGRBG10_1X10  0x300a
#define MEDIA_BUS_FMT_SRGGB10_1X10  0x300f

These are commonly used on CSI serial buses (see the smiapp driver for
example).  From the description at the top of the file, it says the
1X10 means that one pixel is transferred as one 10-bit sample.

However, the format on wire is somewhat different - four pixels are
transmitted over five bytes:

P0  P1  P2  P3  P0  P1  P2  P3
8-bit   8-bit   8-bit   8-bit   2-bit   2-bit   2-bit   2-bit

This gives two problems:
1) it doesn't fit in any sensible kind of "one pixel transferred as
   N M-bit samples" description because the pixel/sample values
   (depending how you look at them) are broken up.

2) changing this will probably be a user visible change, as things
   like smiapp are already in use.

So, I think what we actually have is the media bus formats describing
the _logical_ bus format.  Yes, one pixel is transferred as one 10-bit
sample in this case.

To help illustrate my point, consider the difference between
MEDIA_BUS_FMT_RGB565_1X16 and MEDIA_BUS_FMT_RGB565_2X8_BE or
MEDIA_BUS_FMT_RGB565_2X8_LE.  RGB565_1X16 means 1 pixel over an effective
16-bit wide bus (if it's not 16-bit, then it has to be broken up into
separate "samples".)  RGB565_2X8 means 1 pixel as two 8-bit samples.

So, the 10-bit bayer is 1 pixel as 1.25 bytes.  Or is it, over a serial
bus.  Using the RGB565 case, 10-bit bayer over a 4 lane CSI bus becomes
interesting:

first byte  2nd 3rd
lane 1  P0 9:2  S0  P7 9:2
lane 2  P1 9:2  P4 9:2  S1
lane 3  P2 9:2  P5 9:2  P8 9:2
lane 4  P3 9:2  P6 9:2  P9 9:2

S0 = P0/P1/P2/P3 least significant two bits
S1 = P4/P5/P6/P7 least significant two bits

or 2 lane CSI:
first byte  2nd 3rd 4th 5th
lane 1  P0 9:2  P2  S0  P5  P7
lane 2  P1 9:2  P3  P4  P6  S1

or 1 lane CSI:
lane 1  P0 P1 P2 P3 S0 P4 P5 P6 P7 S1 P8 P9 ...

etc.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
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 00/24] i.MX Media Driver

[Steve Longerbeam]

On 01/24/2017 03:27 AM, Philipp Zabel wrote:

Hi Steve, Hans,

[added Laurent to Cc: who I believe might have an opinion on the media
bus formats, too. Sorry for the wall of text, I have put a marker where
the MEDIA_BUS argument starts]

The central issue seems to be that I think media pad links / media bus
formats should describe physical links, such as parallel or serial
buses, and the formats of pixels flowing through them, whereas Steve
would like to extend them to describe software transports and in-memory
formats.


Hi Philipp, Hans,

I've decided to pull the dma read/write channel linking between
pads. Although I haven't heard any other feedback yet, I agree it is
controversial and it is fairly clear it violates the current media bus
concept that describes physical links.

So the VDIC entity will support only the high motion mode, and
the post-processor entities will be removed. We've talked in the
past of adding full VDIC support to the IPU image conversion API
in the IPUv3 driver, so the complete motion compensation modes
can live there eventually, accessed from a mem2mem device. I'm
also considering leaving the low/medium motion support in the VDIC
entity, but accessed eventually from a separate device node sink pad
(from a future output device), instead of from a subdev pad.

So now there will be four capture device nodes per IPU: two linked
directly from each CSI IDMAC source pad, one from the prp-encode
source pad, and one from the prp-viewfinder source pad.

I will post version 4 in a couple days.

Steve



On Mon, 2017-01-23 at 15:08 -0800, Steve Longerbeam wrote:
[...]

And I'm actually in total agreement with that. I definitely agree that there
should be a mechanism in the media framework that allows passing video
buffers from a source pad to a sink pad using a software queue, with no
involvement from userland.

That is the other part of the argument. I do not agree that these
software queue "links" should be presented to userspace as media pad
links between two entities of a media device.
First, that would limit the links to subdevices contained in the same
media graph, while this should work between any two capture and output
queues of different devices.

It sounds like we are talking about two different new proposed features.

We are talking about the same thing, but we both want a different user
interface.
Technically, the issue is to trigger the DMA read channel of a mem2mem
device automatically whenever another capture device's DMA write channel
signals a finished frame. Where we disagree is how to present this to
userspace.

You represent the capture DMA write channel and mem2mem DMA read channel
as pads on media entites and configure the in-kernel software queue
between the two using a media pad link. At the same time a different
representation of the same DMA write and read channels (the capture
vb2_queue of the capture device and the output vb2_queue of the mem2mem
device) would be used for operation in the classic, userspace controlled
mode via dmabuf passing.

I don't want the software-only link in the media graph, but instead use
the vb2_queue representation for both cases, and implement the in-kernel
queue link on top of the vb2_queue interface. This would allow userspace
to have control over buffer allocations and format, and thus avoid
unexpected performance implications: it is impossible for userspace to
understand which media entity link, when enabled, will cause a
significant increase in memory bandwidth usage, or even how much.
Also the same mechanism could then be used to link any two devices in a
generic manner, instead of special casing the software queue link for
two devices that happen to be part of the same media graph.


My proposal is to implement a software buffer queue between pads.
Beyond enabling the link between pads using the existing media controller
API, userspace is not involved after that. The fact that this link is
accomplished with a software buffer queue is not known, and doesn't
need to be known, by userspace.

I don't think this is a good thing for the reasons stated above and
below:
Since the software buffer queue is opaque to userspace, it is completely
out of userspace control which format is chosen and how the buffers are
allocated.
By using media bus formats to configure software links the kernel
pretends to userspace that there is a physical connection where there
isn't one.
Also, the media entity graph would quickly become very unreadable if we
were to add all devices to it that could reasonably be linked with
software queues.


Your proposal, if I have it right, is to allow linking two v4l2 device
vb2 queues
(i.e. /dev/videoX -> /dev/videoY), using a new user level API, in a free-run
mode such that v4l2 buffers get passed from one device's vb2 queue to the
other without requiring the v4l2 user program to actively forward those
buffers.

Yes.


There isn't anything that would preclude one from the other, they can
both exist. But they 

Re: [PATCH v3 00/24] i.MX Media Driver

[Philipp Zabel]

On Mon, 2017-01-23 at 12:08 +0100, Hans Verkuil wrote:
> On 01/23/2017 12:00 PM, Philipp Zabel wrote:
> > On Fri, 2017-01-20 at 21:39 +0100, Hans Verkuil wrote:
[...]
> As long as it is mentioned in the TODO, and ideally in the Kconfig as well,
> then I'm fine with it.
>
> The big advantage of being in the kernel is that it is much easier to start
> providing fixes, improvements, etc. If you use a staging driver you know
> that there is no guarantee whatsoever with respect to stable ABI/APIs.

Of course, but there should be a clear way how to progress on those
issues that are documented as blockers, otherwise the driver will linger
in staging.
Worse, currently we are not even in agreement what to put into the TODO.

regards
Philipp

--
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 00/24] i.MX Media Driver

[Philipp Zabel]

Hi Steve, Hans,

[added Laurent to Cc: who I believe might have an opinion on the media
bus formats, too. Sorry for the wall of text, I have put a marker where
the MEDIA_BUS argument starts]

The central issue seems to be that I think media pad links / media bus
formats should describe physical links, such as parallel or serial
buses, and the formats of pixels flowing through them, whereas Steve
would like to extend them to describe software transports and in-memory
formats.

On Mon, 2017-01-23 at 15:08 -0800, Steve Longerbeam wrote:
[...]
> >>> And I'm actually in total agreement with that. I definitely agree that 
> >>> there
> >>> should be a mechanism in the media framework that allows passing video
> >>> buffers from a source pad to a sink pad using a software queue, with no
> >>> involvement from userland.
> > That is the other part of the argument. I do not agree that these
> > software queue "links" should be presented to userspace as media pad
> > links between two entities of a media device.
> > First, that would limit the links to subdevices contained in the same
> > media graph, while this should work between any two capture and output
> > queues of different devices.
> 
> It sounds like we are talking about two different new proposed features.

We are talking about the same thing, but we both want a different user
interface.
Technically, the issue is to trigger the DMA read channel of a mem2mem
device automatically whenever another capture device's DMA write channel
signals a finished frame. Where we disagree is how to present this to
userspace.

You represent the capture DMA write channel and mem2mem DMA read channel
as pads on media entites and configure the in-kernel software queue
between the two using a media pad link. At the same time a different
representation of the same DMA write and read channels (the capture
vb2_queue of the capture device and the output vb2_queue of the mem2mem
device) would be used for operation in the classic, userspace controlled
mode via dmabuf passing.

I don't want the software-only link in the media graph, but instead use
the vb2_queue representation for both cases, and implement the in-kernel
queue link on top of the vb2_queue interface. This would allow userspace
to have control over buffer allocations and format, and thus avoid
unexpected performance implications: it is impossible for userspace to
understand which media entity link, when enabled, will cause a
significant increase in memory bandwidth usage, or even how much.
Also the same mechanism could then be used to link any two devices in a
generic manner, instead of special casing the software queue link for
two devices that happen to be part of the same media graph.

> My proposal is to implement a software buffer queue between pads.
> Beyond enabling the link between pads using the existing media controller
> API, userspace is not involved after that. The fact that this link is 
> accomplished with a software buffer queue is not known, and doesn't
> need to be known, by userspace.

I don't think this is a good thing for the reasons stated above and
below:
Since the software buffer queue is opaque to userspace, it is completely
out of userspace control which format is chosen and how the buffers are
allocated.
By using media bus formats to configure software links the kernel
pretends to userspace that there is a physical connection where there
isn't one.
Also, the media entity graph would quickly become very unreadable if we
were to add all devices to it that could reasonably be linked with
software queues.

> Your proposal, if I have it right, is to allow linking two v4l2 device 
> vb2 queues
> (i.e. /dev/videoX -> /dev/videoY), using a new user level API, in a free-run
> mode such that v4l2 buffers get passed from one device's vb2 queue to the
> other without requiring the v4l2 user program to actively forward those 
> buffers.

Yes.

> There isn't anything that would preclude one from the other, they can
> both exist. But they are different ideas. One implements software queues
> at the _pad level_ and is opaque to userspace, the other links queues
> at the _device level_ using a new user API, but once the link is 
> established, also does not require any involvement from userspace.

Well, they are different ideas of how the userspace interface _for the
same thing_ should look like.

> What I'm saying is we can do _both_.

What I am saying is we shouldn't do the pad link interface for the
software queues. In my opinion it is the wrong abstraction, and apart
from the convenience of being able to switch the links on with a single
media-ctl invocation, I see too many downsides.

> > Assume for example, we want to encode the captured, deinterlaced video
> > to h.264 with the coda VPU driver. A software queue link could be
> > established between the CSI capture and the VDIC deinterlacer input,
> 
> That's already available in the media graph. By linking CSI and
> VDIC entities. The capture device 

Re: [PATCH v3 00/24] i.MX Media Driver

[Steve Longerbeam]

On 01/23/2017 03:00 AM, Philipp Zabel wrote:

On Fri, 2017-01-20 at 21:39 +0100, Hans Verkuil wrote:
[...]

There is a VDIC entity in the i.MX IPU that performs de-interlacing with
hardware filters for motion compensation. Some of the motion compensation
modes ("low" and "medium" motion) require that the VDIC receive video
frame fields from memory buffers (dedicated dma channels in the
IPU are used to transfer those buffers into the VDIC).

So one option to support those modes would be to pass the raw buffers
from a camera sensor up to userspace to a capture device, and then pass
them back to the VDIC for de-interlacing using a mem2mem device.

Philipp and I are both in agreement that, since userland is not interested
in the intermediate interlaced buffers in this case, but only the final
result (motion compensated, de-interlaced frames), it is more efficient
to provide a media link that allows passing those intermediate frames
directly from a camera source pad to VDIC sink pad, without having
to route them through userspace.

So in order to support that, I've implemented a simple FIFO dma buffer
queue in the driver to allow passing video buffers directly from a source
to a sink. It is modeled loosely off the vb2 state machine and API, but
simpler (for instance it only allows contiguous, cache-coherent buffers).

This is where Philipp has an argument, that this should be done with a
new API in videobuf2.

That is one part of the argument. I'm glad to understand now that we
agree about this.


And I'm actually in total agreement with that. I definitely agree that there
should be a mechanism in the media framework that allows passing video
buffers from a source pad to a sink pad using a software queue, with no
involvement from userland.

That is the other part of the argument. I do not agree that these
software queue "links" should be presented to userspace as media pad
links between two entities of a media device.
First, that would limit the links to subdevices contained in the same
media graph, while this should work between any two capture and output
queues of different devices.


It sounds like we are talking about two different new proposed features.

My proposal is to implement a software buffer queue between pads.
Beyond enabling the link between pads using the existing media controller
API, userspace is not involved after that. The fact that this link is 
accomplished

with a software buffer queue is not known, and doesn't need to be known,
by userspace.

Your proposal, if I have it right, is to allow linking two v4l2 device 
vb2 queues

(i.e. /dev/videoX -> /dev/videoY), using a new user level API, in a free-run
mode such that v4l2 buffers get passed from one device's vb2 queue to the
other without requiring the v4l2 user program to actively forward those 
buffers.


There isn't anything that would preclude one from the other, they can
both exist. But they are different ideas. One implements software queues
at the _pad level_ and is opaque to userspace, the other links queues
at the _device level_ using a new user API, but once the link is 
established,

also does not require any involvement from userspace.

What I'm saying is we can do _both_.



Assume for example, we want to encode the captured, deinterlaced video
to h.264 with the coda VPU driver. A software queue link could be
established between the CSI capture and the VDIC deinterlacer input,


That's already available in the media graph. By linking CSI and
VDIC entities. The capture device will then already be providing
de-interlaced video, and ...


just as between the VDIC deinterlacer output and the coda VPU input.
Technically, there would be no difference between those two linked
capture/output queue pairs. But the coda driver is a completely separate
mem2mem device. And since it is not part of the i.MX media graph, there
is no entity pad to link to.


your free-run queue linking could then be used to link the (already)
de-interlaced stream to the coda device for h.264 encode.

The other idea would be to eventually make the coda device part of
the media graph as an entity. Then this link would instead be via pads.


Or assume there is an USB analog capture device that produces interlaced
frames. I think it should be possible to connect its capture queue to
the VDIC deinterlacer output queue just the same way as linking the CSI
to the VDIC (in software queue mode).


Right, for devices that are outside the i.MX media graph, such as a USB
capture device (or coda), access to the i.MX entities such as the VDIC would
require an i.MX mem2mem device with media links to the VDIC. The USB
capture device would forward its captured frames to mem2mem (maybe
using your free-run vb2 queue linking idea):

usb device -> i.mx mem2mem device -> VDIC entity -> i.mx mem2mem device




Second, the subdevice pad formats describe wire formats, not memory
formats. The user might want to choose between 4:2:2 and 4:2:0
subsampled YUV formats for the 

Re: [PATCH v3 00/24] i.MX Media Driver

[Hans Verkuil]

On 01/23/2017 12:00 PM, Philipp Zabel wrote:
> On Fri, 2017-01-20 at 21:39 +0100, Hans Verkuil wrote:
> [...]
>>> There is a VDIC entity in the i.MX IPU that performs de-interlacing with
>>> hardware filters for motion compensation. Some of the motion compensation
>>> modes ("low" and "medium" motion) require that the VDIC receive video
>>> frame fields from memory buffers (dedicated dma channels in the
>>> IPU are used to transfer those buffers into the VDIC).
>>>
>>> So one option to support those modes would be to pass the raw buffers
>>> from a camera sensor up to userspace to a capture device, and then pass
>>> them back to the VDIC for de-interlacing using a mem2mem device.
>>>
>>> Philipp and I are both in agreement that, since userland is not interested
>>> in the intermediate interlaced buffers in this case, but only the final
>>> result (motion compensated, de-interlaced frames), it is more efficient
>>> to provide a media link that allows passing those intermediate frames
>>> directly from a camera source pad to VDIC sink pad, without having
>>> to route them through userspace.
>>>
>>> So in order to support that, I've implemented a simple FIFO dma buffer
>>> queue in the driver to allow passing video buffers directly from a source
>>> to a sink. It is modeled loosely off the vb2 state machine and API, but
>>> simpler (for instance it only allows contiguous, cache-coherent buffers).
>>>
>>> This is where Philipp has an argument, that this should be done with a
>>> new API in videobuf2.
> 
> That is one part of the argument. I'm glad to understand now that we
> agree about this.
> 
>>> And I'm actually in total agreement with that. I definitely agree that there
>>> should be a mechanism in the media framework that allows passing video
>>> buffers from a source pad to a sink pad using a software queue, with no
>>> involvement from userland.
> 
> That is the other part of the argument. I do not agree that these
> software queue "links" should be presented to userspace as media pad
> links between two entities of a media device. 
> 
> First, that would limit the links to subdevices contained in the same
> media graph, while this should work between any two capture and output
> queues of different devices.
> Assume for example, we want to encode the captured, deinterlaced video
> to h.264 with the coda VPU driver. A software queue link could be
> established between the CSI capture and the VDIC deinterlacer input,
> just as between the VDIC deinterlacer output and the coda VPU input.
> Technically, there would be no difference between those two linked
> capture/output queue pairs. But the coda driver is a completely separate
> mem2mem device. And since it is not part of the i.MX media graph, there
> is no entity pad to link to.
> Or assume there is an USB analog capture device that produces interlaced
> frames. I think it should be possible to connect its capture queue to
> the VDIC deinterlacer output queue just the same way as linking the CSI
> to the VDIC (in software queue mode).
> 
> Second, the subdevice pad formats describe wire formats, not memory
> formats. The user might want to choose between 4:2:2 and 4:2:0
> subsampled YUV formats for the intermediate buffer, for example,
> depending on memory bandwidth constraints and quality requirements. This
> is impossible with the media entity / subdevice pad links.
> 
> I think an interface where userspace configures the capture and output
> queues via v4l2 API, passes dma buffers around from one to the other
> queue, and then puts both queues into a free running mode would be a
> much better fit for this mechanism.
> 
>>> My only disagreement is when this should be implemented. I think it is
>>> fine to keep my custom implementation of this in the driver for now. Once
>>> an extension of vb2 is ready to support this feature, it would be fairly
>>> straightforward to strip out my custom implementation and go with the
>>> new API.
>>
>> For a staging driver this isn't necessary, as long as it is documented in
>> the TODO file that this needs to be fixed before it can be moved out of
>> staging. The whole point of staging is that there is still work to be
>> done in the driver, after all :-)
> 
> Absolutely. The reason I am arguing against merging the mem2mem media
> control links so vehemently is that I am convinced the userspace
> interface is wrong, and I am afraid that even though in staging, it
> might become established.

As long as it is mentioned in the TODO, and ideally in the Kconfig as well,
then I'm fine with it.

The big advantage of being in the kernel is that it is much easier to start
providing fixes, improvements, etc. If you use a staging driver you know
that there is no guarantee whatsoever with respect to stable ABI/APIs.

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  

Re: [PATCH v3 00/24] i.MX Media Driver

[Philipp Zabel]

On Fri, 2017-01-20 at 21:39 +0100, Hans Verkuil wrote:
[...]
> > There is a VDIC entity in the i.MX IPU that performs de-interlacing with
> > hardware filters for motion compensation. Some of the motion compensation
> > modes ("low" and "medium" motion) require that the VDIC receive video
> > frame fields from memory buffers (dedicated dma channels in the
> > IPU are used to transfer those buffers into the VDIC).
> > 
> > So one option to support those modes would be to pass the raw buffers
> > from a camera sensor up to userspace to a capture device, and then pass
> > them back to the VDIC for de-interlacing using a mem2mem device.
> > 
> > Philipp and I are both in agreement that, since userland is not interested
> > in the intermediate interlaced buffers in this case, but only the final
> > result (motion compensated, de-interlaced frames), it is more efficient
> > to provide a media link that allows passing those intermediate frames
> > directly from a camera source pad to VDIC sink pad, without having
> > to route them through userspace.
> > 
> > So in order to support that, I've implemented a simple FIFO dma buffer
> > queue in the driver to allow passing video buffers directly from a source
> > to a sink. It is modeled loosely off the vb2 state machine and API, but
> > simpler (for instance it only allows contiguous, cache-coherent buffers).
> > 
> > This is where Philipp has an argument, that this should be done with a
> > new API in videobuf2.

That is one part of the argument. I'm glad to understand now that we
agree about this.

> > And I'm actually in total agreement with that. I definitely agree that there
> > should be a mechanism in the media framework that allows passing video
> > buffers from a source pad to a sink pad using a software queue, with no
> > involvement from userland.

That is the other part of the argument. I do not agree that these
software queue "links" should be presented to userspace as media pad
links between two entities of a media device. 

First, that would limit the links to subdevices contained in the same
media graph, while this should work between any two capture and output
queues of different devices.
Assume for example, we want to encode the captured, deinterlaced video
to h.264 with the coda VPU driver. A software queue link could be
established between the CSI capture and the VDIC deinterlacer input,
just as between the VDIC deinterlacer output and the coda VPU input.
Technically, there would be no difference between those two linked
capture/output queue pairs. But the coda driver is a completely separate
mem2mem device. And since it is not part of the i.MX media graph, there
is no entity pad to link to.
Or assume there is an USB analog capture device that produces interlaced
frames. I think it should be possible to connect its capture queue to
the VDIC deinterlacer output queue just the same way as linking the CSI
to the VDIC (in software queue mode).

Second, the subdevice pad formats describe wire formats, not memory
formats. The user might want to choose between 4:2:2 and 4:2:0
subsampled YUV formats for the intermediate buffer, for example,
depending on memory bandwidth constraints and quality requirements. This
is impossible with the media entity / subdevice pad links.

I think an interface where userspace configures the capture and output
queues via v4l2 API, passes dma buffers around from one to the other
queue, and then puts both queues into a free running mode would be a
much better fit for this mechanism.

> > My only disagreement is when this should be implemented. I think it is
> > fine to keep my custom implementation of this in the driver for now. Once
> > an extension of vb2 is ready to support this feature, it would be fairly
> > straightforward to strip out my custom implementation and go with the
> > new API.
> 
> For a staging driver this isn't necessary, as long as it is documented in
> the TODO file that this needs to be fixed before it can be moved out of
> staging. The whole point of staging is that there is still work to be
> done in the driver, after all :-)

Absolutely. The reason I am arguing against merging the mem2mem media
control links so vehemently is that I am convinced the userspace
interface is wrong, and I am afraid that even though in staging, it
might become established.

regards
Philipp

--
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 00/24] i.MX Media Driver

[Hans Verkuil]

On 01/20/2017 07:40 PM, Steve Longerbeam wrote:
> Hi Hans, Philipp,
> 
> 
> On 01/20/2017 08:31 AM, Philipp Zabel wrote:
>> Hi Hans,
>>
>> On Fri, 2017-01-20 at 14:52 +0100, Hans Verkuil wrote:
>>> Hi Steve, Philipp,
>>>
>>> On 01/07/2017 03:11 AM, Steve Longerbeam wrote:
 In version 3:

 Changes suggested by Rob Herring :

- prepended FIM node properties with vendor prefix "fsl,".

- make mipi csi-2 receiver compatible string SoC specific:
  "fsl,imx6-mipi-csi2" instead of "fsl,imx-mipi-csi2".

- redundant "_clk" removed from mipi csi-2 receiver clock-names 
 property.

- removed board-specific info from the media driver binding doc. These
  were all related to sensor bindings, which already are (adv7180)
  or will be (ov564x) covered in separate binding docs. All reference
  board info not related to DT bindings has been moved to
  Documentation/media/v4l-drivers/imx.rst.

- removed "_mipi" from the OV5640 compatible string.

 Changes suggested by Vladimir Zapolskiy :

Mostly cosmetic/non-functional changes which I won't list here, except
for the following:

- spin_lock_irqsave() changed to spin_lock() in a couple interrupt 
 handlers.

- fixed some unnecessary of_node_put()'s in for_each_child_of_node() 
 loops.

- check/handle return code from required reg property of CSI port nodes.

- check/handle return code from clk_prepare_enable().

 Changes suggested by Fabio Estevam :

- switch to VGEN3 Analog Vdd supply assuming rev. C SabreSD boards.

- finally got around to passing valid IOMUX pin config values to the
  pin groups.

 Other changes:

- removed the FIM properties that overrided the v4l2 FIM control 
 defaults
  values. This was left-over from a requirement of a customer and is not
  necessary here.

- The FIM must be explicitly enabled in the fim child node under the CSI
  port nodes, using the status property. If not enabled, FIM v4l2 
 controls
  will not appear in the video capture driver.

- brought in additional media types patch from Philipp Zabel. Use new
  MEDIA_ENT_F_VID_IF_BRIDGE in mipi csi-2 receiver subdev.

- brought in latest platform generic video multiplexer subdevice driver
  from Philipp Zabel (squashed with patch that uses new 
 MEDIA_ENT_F_MUX).

- removed imx-media-of.h, moved those prototypes into imx-media.h.
>>> Based on the discussion on the mailinglist it seems everyone agrees that 
>>> this
>>> is the preferred driver, correct?
>> No. I have some major reservations against the custom mem2mem framework
>> embedded in Steve's driver.
>> I think it is a misuse of the media entity links (which should describe
>> hardware connections) for something that should be done at the vb2 level
>> (letting one device's capture EOF interrupt trigger the next device's
>> m2m device_run without going through userspace).
>> Steve and I disagree on that point, so we'd appreciate if we could get
>> some more eyes on the above issue.
> 
> This needs some background first, so let me first describe one example
> pipeline in this driver.
> 
> There is a VDIC entity in the i.MX IPU that performs de-interlacing with
> hardware filters for motion compensation. Some of the motion compensation
> modes ("low" and "medium" motion) require that the VDIC receive video
> frame fields from memory buffers (dedicated dma channels in the
> IPU are used to transfer those buffers into the VDIC).
> 
> So one option to support those modes would be to pass the raw buffers
> from a camera sensor up to userspace to a capture device, and then pass
> them back to the VDIC for de-interlacing using a mem2mem device.
> 
> Philipp and I are both in agreement that, since userland is not interested
> in the intermediate interlaced buffers in this case, but only the final
> result (motion compensated, de-interlaced frames), it is more efficient
> to provide a media link that allows passing those intermediate frames
> directly from a camera source pad to VDIC sink pad, without having
> to route them through userspace.
> 
> So in order to support that, I've implemented a simple FIFO dma buffer
> queue in the driver to allow passing video buffers directly from a source
> to a sink. It is modeled loosely off the vb2 state machine and API, but
> simpler (for instance it only allows contiguous, cache-coherent buffers).
> 
> This is where Philipp has an argument, that this should be done with a
> new API in videobuf2.
> 
> And I'm actually in total agreement with that. I definitely agree that there
> should be a mechanism in the media framework that allows passing video
> buffers from a source pad 

Re: [PATCH v3 00/24] i.MX Media Driver

[Steve Longerbeam]

Hi Hans, Philipp,


On 01/20/2017 08:31 AM, Philipp Zabel wrote:

Hi Hans,

On Fri, 2017-01-20 at 14:52 +0100, Hans Verkuil wrote:

Hi Steve, Philipp,

On 01/07/2017 03:11 AM, Steve Longerbeam wrote:

In version 3:

Changes suggested by Rob Herring :

   - prepended FIM node properties with vendor prefix "fsl,".

   - make mipi csi-2 receiver compatible string SoC specific:
 "fsl,imx6-mipi-csi2" instead of "fsl,imx-mipi-csi2".

   - redundant "_clk" removed from mipi csi-2 receiver clock-names property.

   - removed board-specific info from the media driver binding doc. These
 were all related to sensor bindings, which already are (adv7180)
 or will be (ov564x) covered in separate binding docs. All reference
 board info not related to DT bindings has been moved to
 Documentation/media/v4l-drivers/imx.rst.

   - removed "_mipi" from the OV5640 compatible string.

Changes suggested by Vladimir Zapolskiy :

   Mostly cosmetic/non-functional changes which I won't list here, except
   for the following:

   - spin_lock_irqsave() changed to spin_lock() in a couple interrupt handlers.

   - fixed some unnecessary of_node_put()'s in for_each_child_of_node() loops.

   - check/handle return code from required reg property of CSI port nodes.

   - check/handle return code from clk_prepare_enable().

Changes suggested by Fabio Estevam :

   - switch to VGEN3 Analog Vdd supply assuming rev. C SabreSD boards.

   - finally got around to passing valid IOMUX pin config values to the
 pin groups.

Other changes:

   - removed the FIM properties that overrided the v4l2 FIM control defaults
 values. This was left-over from a requirement of a customer and is not
 necessary here.

   - The FIM must be explicitly enabled in the fim child node under the CSI
 port nodes, using the status property. If not enabled, FIM v4l2 controls
 will not appear in the video capture driver.

   - brought in additional media types patch from Philipp Zabel. Use new
 MEDIA_ENT_F_VID_IF_BRIDGE in mipi csi-2 receiver subdev.

   - brought in latest platform generic video multiplexer subdevice driver
 from Philipp Zabel (squashed with patch that uses new MEDIA_ENT_F_MUX).

   - removed imx-media-of.h, moved those prototypes into imx-media.h.

Based on the discussion on the mailinglist it seems everyone agrees that this
is the preferred driver, correct?

No. I have some major reservations against the custom mem2mem framework
embedded in Steve's driver.
I think it is a misuse of the media entity links (which should describe
hardware connections) for something that should be done at the vb2 level
(letting one device's capture EOF interrupt trigger the next device's
m2m device_run without going through userspace).
Steve and I disagree on that point, so we'd appreciate if we could get
some more eyes on the above issue.


This needs some background first, so let me first describe one example
pipeline in this driver.

There is a VDIC entity in the i.MX IPU that performs de-interlacing with
hardware filters for motion compensation. Some of the motion compensation
modes ("low" and "medium" motion) require that the VDIC receive video
frame fields from memory buffers (dedicated dma channels in the
IPU are used to transfer those buffers into the VDIC).

So one option to support those modes would be to pass the raw buffers
from a camera sensor up to userspace to a capture device, and then pass
them back to the VDIC for de-interlacing using a mem2mem device.

Philipp and I are both in agreement that, since userland is not interested
in the intermediate interlaced buffers in this case, but only the final
result (motion compensated, de-interlaced frames), it is more efficient
to provide a media link that allows passing those intermediate frames
directly from a camera source pad to VDIC sink pad, without having
to route them through userspace.

So in order to support that, I've implemented a simple FIFO dma buffer
queue in the driver to allow passing video buffers directly from a source
to a sink. It is modeled loosely off the vb2 state machine and API, but
simpler (for instance it only allows contiguous, cache-coherent buffers).

This is where Philipp has an argument, that this should be done with a
new API in videobuf2.

And I'm actually in total agreement with that. I definitely agree that there
should be a mechanism in the media framework that allows passing video
buffers from a source pad to a sink pad using a software queue, with no
involvement from userland.

My only disagreement is when this should be implemented. I think it is
fine to keep my custom implementation of this in the driver for now. Once
an extension of vb2 is ready to support this feature, it would be fairly
straightforward to strip out my custom implementation and go with the
new API.

Steve


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the 

Re: [PATCH v3 00/24] i.MX Media Driver

[Philipp Zabel]

Hi Hans,

On Fri, 2017-01-20 at 14:52 +0100, Hans Verkuil wrote:
> Hi Steve, Philipp,
> 
> On 01/07/2017 03:11 AM, Steve Longerbeam wrote:
> > In version 3:
> > 
> > Changes suggested by Rob Herring :
> > 
> >   - prepended FIM node properties with vendor prefix "fsl,".
> > 
> >   - make mipi csi-2 receiver compatible string SoC specific:
> > "fsl,imx6-mipi-csi2" instead of "fsl,imx-mipi-csi2".
> > 
> >   - redundant "_clk" removed from mipi csi-2 receiver clock-names property.
> > 
> >   - removed board-specific info from the media driver binding doc. These
> > were all related to sensor bindings, which already are (adv7180)
> > or will be (ov564x) covered in separate binding docs. All reference
> > board info not related to DT bindings has been moved to
> > Documentation/media/v4l-drivers/imx.rst.
> > 
> >   - removed "_mipi" from the OV5640 compatible string.
> > 
> > Changes suggested by Vladimir Zapolskiy :
> > 
> >   Mostly cosmetic/non-functional changes which I won't list here, except
> >   for the following:
> > 
> >   - spin_lock_irqsave() changed to spin_lock() in a couple interrupt 
> > handlers.
> > 
> >   - fixed some unnecessary of_node_put()'s in for_each_child_of_node() 
> > loops.
> > 
> >   - check/handle return code from required reg property of CSI port nodes.
> > 
> >   - check/handle return code from clk_prepare_enable().
> > 
> > Changes suggested by Fabio Estevam :
> > 
> >   - switch to VGEN3 Analog Vdd supply assuming rev. C SabreSD boards.
> > 
> >   - finally got around to passing valid IOMUX pin config values to the
> > pin groups.
> > 
> > Other changes:
> > 
> >   - removed the FIM properties that overrided the v4l2 FIM control defaults
> > values. This was left-over from a requirement of a customer and is not
> > necessary here.
> > 
> >   - The FIM must be explicitly enabled in the fim child node under the CSI
> > port nodes, using the status property. If not enabled, FIM v4l2 controls
> > will not appear in the video capture driver.
> > 
> >   - brought in additional media types patch from Philipp Zabel. Use new
> > MEDIA_ENT_F_VID_IF_BRIDGE in mipi csi-2 receiver subdev.
> > 
> >   - brought in latest platform generic video multiplexer subdevice driver
> > from Philipp Zabel (squashed with patch that uses new MEDIA_ENT_F_MUX).
> > 
> >   - removed imx-media-of.h, moved those prototypes into imx-media.h.
> 
> Based on the discussion on the mailinglist it seems everyone agrees that this
> is the preferred driver, correct?

No. I have some major reservations against the custom mem2mem framework
embedded in Steve's driver.
I think it is a misuse of the media entity links (which should describe
hardware connections) for something that should be done at the vb2 level
(letting one device's capture EOF interrupt trigger the next device's
m2m device_run without going through userspace).
Steve and I disagree on that point, so we'd appreciate if we could get
some more eyes on the above issue.

regards
Philipp

--
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 00/24] i.MX Media Driver

[Hans Verkuil]

Hi Steve, Philipp,

On 01/07/2017 03:11 AM, Steve Longerbeam wrote:
> In version 3:
> 
> Changes suggested by Rob Herring :
> 
>   - prepended FIM node properties with vendor prefix "fsl,".
> 
>   - make mipi csi-2 receiver compatible string SoC specific:
> "fsl,imx6-mipi-csi2" instead of "fsl,imx-mipi-csi2".
> 
>   - redundant "_clk" removed from mipi csi-2 receiver clock-names property.
> 
>   - removed board-specific info from the media driver binding doc. These
> were all related to sensor bindings, which already are (adv7180)
> or will be (ov564x) covered in separate binding docs. All reference
> board info not related to DT bindings has been moved to
> Documentation/media/v4l-drivers/imx.rst.
> 
>   - removed "_mipi" from the OV5640 compatible string.
> 
> Changes suggested by Vladimir Zapolskiy :
> 
>   Mostly cosmetic/non-functional changes which I won't list here, except
>   for the following:
> 
>   - spin_lock_irqsave() changed to spin_lock() in a couple interrupt handlers.
> 
>   - fixed some unnecessary of_node_put()'s in for_each_child_of_node() loops.
> 
>   - check/handle return code from required reg property of CSI port nodes.
> 
>   - check/handle return code from clk_prepare_enable().
> 
> Changes suggested by Fabio Estevam :
> 
>   - switch to VGEN3 Analog Vdd supply assuming rev. C SabreSD boards.
> 
>   - finally got around to passing valid IOMUX pin config values to the
> pin groups.
> 
> Other changes:
> 
>   - removed the FIM properties that overrided the v4l2 FIM control defaults
> values. This was left-over from a requirement of a customer and is not
> necessary here.
> 
>   - The FIM must be explicitly enabled in the fim child node under the CSI
> port nodes, using the status property. If not enabled, FIM v4l2 controls
> will not appear in the video capture driver.
> 
>   - brought in additional media types patch from Philipp Zabel. Use new
> MEDIA_ENT_F_VID_IF_BRIDGE in mipi csi-2 receiver subdev.
> 
>   - brought in latest platform generic video multiplexer subdevice driver
> from Philipp Zabel (squashed with patch that uses new MEDIA_ENT_F_MUX).
> 
>   - removed imx-media-of.h, moved those prototypes into imx-media.h.

Based on the discussion on the mailinglist it seems everyone agrees that this
is the preferred driver, correct?

There are a bunch of review comments, so I will wait for a v4. I plan to merge
that staging driver unless there are major issues with it.

I am not sure if I would merge those sensor drivers in staging, I'll have to
take a closer look at those once v4 is posted.

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 v3 00/24] i.MX Media Driver

[Tim Harvey]

On Thu, Jan 12, 2017 at 2:32 PM, Steve Longerbeam  wrote:
> Hi Tim,
>
>
> On 01/12/2017 01:13 PM, Tim Harvey wrote:
>>
>>
 Now that your driver is hooking into the current media framework, I'm
 not at all clear on how to link and configure the media entities.
>>>
>>>
>>> It's all documented at Documentation/media/v4l-drivers/imx.rst.
>>> Follow the SabreAuto pipeline setup example.
>>>
>> ah yes... it helps to read your patches! You did a great job on the
>> documentation.
>>
>> Regarding the The ipu1_csi0_mux/ipu2_csi1_mux entities which have 1
>> source and 2 sinks (which makes sense for a mux) how do you know which
>> sink pad you should use (in your adv7180 example you use the 2nd sink
>> pad vs the first)?
>
>
> The adv7180 can only go to the parallel input pad (ipu1_csi0_mux:1
> on quad). The other input pads select from the mipi csi-2 receiver virtual
> channels.

right - my question was how does the user know which pad is which. I
see that the imx6q.dtsi makes it clear that port0 (sink1) is the mipi
port and port1 is the parallel port (sink2). Do you know how a user
would determine this from runtime information (maybe something via
media-ctl or /sys/class/media that I haven't yet found) or perhaps
this is to be taken care of by documentation or referring to the dts?

>
> Have you generated a dot graph? It makes it much easier to
> visualize:
>
> # media-ctl --print-dot > graph.dot
>
> then on your host:
>
> % dot -Tpng graph.dot > graph.png
>

Yes - that makes it much easier to understand the possible links.

I notice 'media-ctl --print-topology' shows link and pad type fields,
but 'media-ctl --print-dot' does not include the pad type field (maybe
newer versions do or will in the future)

Do you know how one goes about determining the possible format types
possible for each pad?

>
>
>>
>> As my hardware is the same as the SabreAuto except that my adv7180 is
>> on i2c-2@0x20 I follow your example from
>> Documentation/media/v4l-drivers/imx.rst:
>>
>> # Setup links
>> media-ctl -l '"adv7180 2-0020":0 -> "ipu1_csi0_mux":1[1]'
>> media-ctl -l '"ipu1_csi0_mux":2 -> "ipu1_csi0":0[1]'
>> media-ctl -l '"ipu1_csi0":1 -> "ipu1_smfc0":0[1]'
>> media-ctl -l '"ipu1_smfc0":1 -> "ipu1_ic_prpvf":0[1]'
>> media-ctl -l '"ipu1_ic_prpvf":1 -> "camif0":0[1]'
>> media-ctl -l '"camif0":1 -> "camif0 devnode":0[1]'
>>
>> # Configure pads
>> media-ctl -V "\"adv7180 2-0020\":0 [fmt:UYVY2X8/720x480]"
>> media-ctl -V "\"ipu1_csi0_mux\":1 [fmt:UYVY2X8/720x480]"
>> media-ctl -V "\"ipu1_csi0_mux\":2 [fmt:UYVY2X8/720x480]"
>> media-ctl -V "\"ipu1_csi0\":0 [fmt:UYVY2X8/720x480]"
>> media-ctl -V "\"ipu1_csi0\":1 [fmt:UYVY2X8/720x480]"
>> media-ctl -V "\"ipu1_smfc0\":0 [fmt:UYVY2X8/720x480]"
>> media-ctl -V "\"ipu1_smfc0\":1 [fmt:UYVY2X8/720x480]"
>> media-ctl -V "\"ipu1_ic_prpvf\":0 [fmt:UYVY2X8/720x480]"
>> # pad field types for camif can be any format prpvf supports
>> export outputfmt="UYVY2X8/720x480"
>> media-ctl -V "\"ipu1_ic_prpvf\":1 [fmt:$outputfmt]"
>> media-ctl -V "\"camif0\":0 [fmt:$outputfmt]"
>> media-ctl -V "\"camif0\":1 [fmt:$outputfmt]"
>>
>> # select AIN1
>> v4l2-ctl -d0 -i0
>> Video input set to 0 (ADV7180 Composite on Ain1: ok)
>> v4l2-ctl -d0 --set-fmt-video=width=720,height=480,pixelformat=UYVY
>> # capture a single raw frame
>> v4l2-ctl -d0 --stream-mmap --stream-to=/x.raw --stream-count=1
>> [ 2092.056394] camif0: pipeline_set_stream failed with -32
>> VIDIOC_STREAMON: failed: Broken pipe
>>
>> Enabling debug in drivers/media/media-entity.c I see:
>> [   38.870087] imx-media soc:media@0: link validation failed for
>> "ipu1_smfc0":1 -> "ipu1_ic_prpvf":0, error -32
>>
>> Looking at ipu1_smfc0 and ipu1_ic_prpvf with media-ctl I see:
>> - entity 12: ipu1_ic_prpvf (2 pads, 8 links)
>>   type V4L2 subdev subtype Unknown flags 0
>>   device node name /dev/v4l-subdev3
>>  pad0: Sink
>>  [fmt:UYVY2X8/720x480 field:alternate]
>>  <- "ipu1_csi0":1 []
>>  <- "ipu1_csi1":1 []
>>  <- "ipu1_smfc0":1 [ENABLED]
>>  <- "ipu1_smfc1":1 []
>>  pad1: Source
>>  [fmt:UYVY2X8/720x480 field:none]
>>  -> "camif0":0 [ENABLED]
>>  -> "camif1":0 []
>>  -> "ipu1_ic_pp0":0 []
>>  -> "ipu1_ic_pp1":0 []
>>
>> - entity 45: ipu1_smfc0 (2 pads, 5 links)
>>   type V4L2 subdev subtype Unknown flags 0
>>   device node name /dev/v4l-subdev14
>>  pad0: Sink
>>  [fmt:UYVY2X8/720x480]
>>  <- "ipu1_csi0":1 [ENABLED]
>>  pad1: Source
>>  [fmt:UYVY2X8/720x480]
>>  -> "ipu1_ic_prpvf":0 [ENABLED]
>>  -> "ipu1_ic_pp0":0 []
>>  -> "camif0":0 []
>>  -> "camif1":0 []
>>
>> Any ideas what is going wrong here? Seems like its perhaps a field
>> type mismatch.
>
>
> Yes, exactly, you'll 

Re: [PATCH v3 00/24] i.MX Media Driver

[Steve Longerbeam]

Hi Tim,


On 01/12/2017 01:13 PM, Tim Harvey wrote:



Now that your driver is hooking into the current media framework, I'm
not at all clear on how to link and configure the media entities.


It's all documented at Documentation/media/v4l-drivers/imx.rst.
Follow the SabreAuto pipeline setup example.


ah yes... it helps to read your patches! You did a great job on the
documentation.

Regarding the The ipu1_csi0_mux/ipu2_csi1_mux entities which have 1
source and 2 sinks (which makes sense for a mux) how do you know which
sink pad you should use (in your adv7180 example you use the 2nd sink
pad vs the first)?


The adv7180 can only go to the parallel input pad (ipu1_csi0_mux:1
on quad). The other input pads select from the mipi csi-2 receiver virtual
channels.

Have you generated a dot graph? It makes it much easier to
visualize:

# media-ctl --print-dot > graph.dot

then on your host:

% dot -Tpng graph.dot > graph.png




As my hardware is the same as the SabreAuto except that my adv7180 is
on i2c-2@0x20 I follow your example from
Documentation/media/v4l-drivers/imx.rst:

# Setup links
media-ctl -l '"adv7180 2-0020":0 -> "ipu1_csi0_mux":1[1]'
media-ctl -l '"ipu1_csi0_mux":2 -> "ipu1_csi0":0[1]'
media-ctl -l '"ipu1_csi0":1 -> "ipu1_smfc0":0[1]'
media-ctl -l '"ipu1_smfc0":1 -> "ipu1_ic_prpvf":0[1]'
media-ctl -l '"ipu1_ic_prpvf":1 -> "camif0":0[1]'
media-ctl -l '"camif0":1 -> "camif0 devnode":0[1]'

# Configure pads
media-ctl -V "\"adv7180 2-0020\":0 [fmt:UYVY2X8/720x480]"
media-ctl -V "\"ipu1_csi0_mux\":1 [fmt:UYVY2X8/720x480]"
media-ctl -V "\"ipu1_csi0_mux\":2 [fmt:UYVY2X8/720x480]"
media-ctl -V "\"ipu1_csi0\":0 [fmt:UYVY2X8/720x480]"
media-ctl -V "\"ipu1_csi0\":1 [fmt:UYVY2X8/720x480]"
media-ctl -V "\"ipu1_smfc0\":0 [fmt:UYVY2X8/720x480]"
media-ctl -V "\"ipu1_smfc0\":1 [fmt:UYVY2X8/720x480]"
media-ctl -V "\"ipu1_ic_prpvf\":0 [fmt:UYVY2X8/720x480]"
# pad field types for camif can be any format prpvf supports
export outputfmt="UYVY2X8/720x480"
media-ctl -V "\"ipu1_ic_prpvf\":1 [fmt:$outputfmt]"
media-ctl -V "\"camif0\":0 [fmt:$outputfmt]"
media-ctl -V "\"camif0\":1 [fmt:$outputfmt]"

# select AIN1
v4l2-ctl -d0 -i0
Video input set to 0 (ADV7180 Composite on Ain1: ok)
v4l2-ctl -d0 --set-fmt-video=width=720,height=480,pixelformat=UYVY
# capture a single raw frame
v4l2-ctl -d0 --stream-mmap --stream-to=/x.raw --stream-count=1
[ 2092.056394] camif0: pipeline_set_stream failed with -32
VIDIOC_STREAMON: failed: Broken pipe

Enabling debug in drivers/media/media-entity.c I see:
[   38.870087] imx-media soc:media@0: link validation failed for
"ipu1_smfc0":1 -> "ipu1_ic_prpvf":0, error -32

Looking at ipu1_smfc0 and ipu1_ic_prpvf with media-ctl I see:
- entity 12: ipu1_ic_prpvf (2 pads, 8 links)
  type V4L2 subdev subtype Unknown flags 0
  device node name /dev/v4l-subdev3
 pad0: Sink
 [fmt:UYVY2X8/720x480 field:alternate]
 <- "ipu1_csi0":1 []
 <- "ipu1_csi1":1 []
 <- "ipu1_smfc0":1 [ENABLED]
 <- "ipu1_smfc1":1 []
 pad1: Source
 [fmt:UYVY2X8/720x480 field:none]
 -> "camif0":0 [ENABLED]
 -> "camif1":0 []
 -> "ipu1_ic_pp0":0 []
 -> "ipu1_ic_pp1":0 []

- entity 45: ipu1_smfc0 (2 pads, 5 links)
  type V4L2 subdev subtype Unknown flags 0
  device node name /dev/v4l-subdev14
 pad0: Sink
 [fmt:UYVY2X8/720x480]
 <- "ipu1_csi0":1 [ENABLED]
 pad1: Source
 [fmt:UYVY2X8/720x480]
 -> "ipu1_ic_prpvf":0 [ENABLED]
 -> "ipu1_ic_pp0":0 []
 -> "camif0":0 []
 -> "camif1":0 []

Any ideas what is going wrong here? Seems like its perhaps a field
type mismatch.


Yes, exactly, you'll need to set the field types on every pad in your
pipeline.


  Is my outputfmt incorrect perhaps? I likely have
misunderstood the pad type comments in your documentation.


Attached is an update doc (from branch imx-media-staging-md-v7 on my fork).
I recently upgraded my v4l-utils package and media-ctl now supports 
specifying
the field type in the pad format strings. If you don't have the latest 
v4l-utils, it's

fairly straightforward to cross-build.










Additionally I've found that on an IMX6S/IMX6DL we crash while
registering the media-ic subdev's:



Yep, I only have quad boards here so I haven't gotten around to
testing on S/DL.

But it looks like I forgot to clear out the csi subdev pointer array before
passing it to imx_media_of_parse(). I think that might explain the OOPS
above. Try this patch:

diff --git a/drivers/staging/media/imx/imx-media-dev.c
b/drivers/staging/media/imx/imx-media-dev.c
index 357654d..0cf2d61 100644
--- a/drivers/staging/media/imx/imx-media-dev.c
+++ b/drivers/staging/media/imx/imx-media-dev.c
@@ -379,7 +379,7 @@ static int imx_media_probe(struct platform_device *pdev)
  {

Re: [PATCH v3 00/24] i.MX Media Driver

[Tim Harvey]

On Wed, Jan 11, 2017 at 7:22 PM, Steve Longerbeam
 wrote:
> Hi Tim,
>
>
> On 01/11/2017 03:14 PM, Tim Harvey wrote:
>>
>>
>> 
>>
>> Hi Steve,
>>
>> I took a stab at testing this today on a gw51xx which has an adv7180
>> hooked up as follows:
>> - i2c3@0x20
>> - 8bit data bus from DAT12 to DAT19, HSYNC, VSYNC, PIXCLK on CSI0 pads
>> (CSI0_IPU1)
>> - PWRDWN# on MX6QDL_PAD_CSI0_DATA_EN__GPIO5_IO20
>> - IRQ# on MX6QDL_PAD_CSI0_DAT5__GPIO5_IO23
>> - all three analog inputs available to off-board connector
>>
>> My patch to the imx6qdl-gw51xx dtsi is:
>
>
> As long as you used the patch to imx6qdl-sabreauto.dtsti that adds
> the adv7180 support as a guide, you should be ok here.

yes - fairly straightforward

>
>> 
>>
>>
>>
>> On an IMX6Q I'm getting the following when the adv7180 module loads:
>> [   12.862477] adv7180 2-0020: chip found @ 0x20 (21a8000.i2c)
>> [   12.907767] imx-media: Registered subdev adv7180 2-0020
>> [   12.907793] imx-media soc:media@0: Entity type for entity adv7180
>> 2-0020 was not initialized!
>> [   12.907867] imx-media: imx_media_create_link: adv7180 2-0020:0 ->
>> ipu1_csi0_mux:1
>>
>> Is the warning that adv7180 was not initialized expected and or an issue?
>
>
> Yeah it's still a bug in the adv7180 driver, needs fixing.
>

ok - ignoring

>>
>> Now that your driver is hooking into the current media framework, I'm
>> not at all clear on how to link and configure the media entities.
>
>
> It's all documented at Documentation/media/v4l-drivers/imx.rst.
> Follow the SabreAuto pipeline setup example.
>

ah yes... it helps to read your patches! You did a great job on the
documentation.

Regarding the The ipu1_csi0_mux/ipu2_csi1_mux entities which have 1
source and 2 sinks (which makes sense for a mux) how do you know which
sink pad you should use (in your adv7180 example you use the 2nd sink
pad vs the first)?

As my hardware is the same as the SabreAuto except that my adv7180 is
on i2c-2@0x20 I follow your example from
Documentation/media/v4l-drivers/imx.rst:

# Setup links
media-ctl -l '"adv7180 2-0020":0 -> "ipu1_csi0_mux":1[1]'
media-ctl -l '"ipu1_csi0_mux":2 -> "ipu1_csi0":0[1]'
media-ctl -l '"ipu1_csi0":1 -> "ipu1_smfc0":0[1]'
media-ctl -l '"ipu1_smfc0":1 -> "ipu1_ic_prpvf":0[1]'
media-ctl -l '"ipu1_ic_prpvf":1 -> "camif0":0[1]'
media-ctl -l '"camif0":1 -> "camif0 devnode":0[1]'

# Configure pads
media-ctl -V "\"adv7180 2-0020\":0 [fmt:UYVY2X8/720x480]"
media-ctl -V "\"ipu1_csi0_mux\":1 [fmt:UYVY2X8/720x480]"
media-ctl -V "\"ipu1_csi0_mux\":2 [fmt:UYVY2X8/720x480]"
media-ctl -V "\"ipu1_csi0\":0 [fmt:UYVY2X8/720x480]"
media-ctl -V "\"ipu1_csi0\":1 [fmt:UYVY2X8/720x480]"
media-ctl -V "\"ipu1_smfc0\":0 [fmt:UYVY2X8/720x480]"
media-ctl -V "\"ipu1_smfc0\":1 [fmt:UYVY2X8/720x480]"
media-ctl -V "\"ipu1_ic_prpvf\":0 [fmt:UYVY2X8/720x480]"
# pad field types for camif can be any format prpvf supports
export outputfmt="UYVY2X8/720x480"
media-ctl -V "\"ipu1_ic_prpvf\":1 [fmt:$outputfmt]"
media-ctl -V "\"camif0\":0 [fmt:$outputfmt]"
media-ctl -V "\"camif0\":1 [fmt:$outputfmt]"

# select AIN1
v4l2-ctl -d0 -i0
Video input set to 0 (ADV7180 Composite on Ain1: ok)
v4l2-ctl -d0 --set-fmt-video=width=720,height=480,pixelformat=UYVY
# capture a single raw frame
v4l2-ctl -d0 --stream-mmap --stream-to=/x.raw --stream-count=1
[ 2092.056394] camif0: pipeline_set_stream failed with -32
VIDIOC_STREAMON: failed: Broken pipe

Enabling debug in drivers/media/media-entity.c I see:
[   38.870087] imx-media soc:media@0: link validation failed for
"ipu1_smfc0":1 -> "ipu1_ic_prpvf":0, error -32

Looking at ipu1_smfc0 and ipu1_ic_prpvf with media-ctl I see:
- entity 12: ipu1_ic_prpvf (2 pads, 8 links)
 type V4L2 subdev subtype Unknown flags 0
 device node name /dev/v4l-subdev3
pad0: Sink
[fmt:UYVY2X8/720x480 field:alternate]
<- "ipu1_csi0":1 []
<- "ipu1_csi1":1 []
<- "ipu1_smfc0":1 [ENABLED]
<- "ipu1_smfc1":1 []
pad1: Source
[fmt:UYVY2X8/720x480 field:none]
-> "camif0":0 [ENABLED]
-> "camif1":0 []
-> "ipu1_ic_pp0":0 []
-> "ipu1_ic_pp1":0 []

- entity 45: ipu1_smfc0 (2 pads, 5 links)
 type V4L2 subdev subtype Unknown flags 0
 device node name /dev/v4l-subdev14
pad0: Sink
[fmt:UYVY2X8/720x480]
<- "ipu1_csi0":1 [ENABLED]
pad1: Source
[fmt:UYVY2X8/720x480]
-> "ipu1_ic_prpvf":0 [ENABLED]
-> "ipu1_ic_pp0":0 []
-> "camif0":0 []
-> "camif1":0 []

Any ideas what is going wrong here? Seems like its perhaps a field
type mismatch. Is my outputfmt incorrect perhaps? I likely have
misunderstood the pad type comments in your documentation.

>
>
>> 
>>
>>
>>
>> Additionally I've found that on an IMX6S/IMX6DL we crash while
>> registering the media-ic 

Re: [PATCH v3 00/24] i.MX Media Driver

[Steve Longerbeam]

Hi Tim,


On 01/11/2017 03:14 PM, Tim Harvey wrote:




Hi Steve,

I took a stab at testing this today on a gw51xx which has an adv7180
hooked up as follows:
- i2c3@0x20
- 8bit data bus from DAT12 to DAT19, HSYNC, VSYNC, PIXCLK on CSI0 pads
(CSI0_IPU1)
- PWRDWN# on MX6QDL_PAD_CSI0_DATA_EN__GPIO5_IO20
- IRQ# on MX6QDL_PAD_CSI0_DAT5__GPIO5_IO23
- all three analog inputs available to off-board connector

My patch to the imx6qdl-gw51xx dtsi is:


As long as you used the patch to imx6qdl-sabreauto.dtsti that adds
the adv7180 support as a guide, you should be ok here.






On an IMX6Q I'm getting the following when the adv7180 module loads:
[   12.862477] adv7180 2-0020: chip found @ 0x20 (21a8000.i2c)
[   12.907767] imx-media: Registered subdev adv7180 2-0020
[   12.907793] imx-media soc:media@0: Entity type for entity adv7180
2-0020 was not initialized!
[   12.907867] imx-media: imx_media_create_link: adv7180 2-0020:0 ->
ipu1_csi0_mux:1

Is the warning that adv7180 was not initialized expected and or an issue?


Yeah it's still a bug in the adv7180 driver, needs fixing.



Now that your driver is hooking into the current media framework, I'm
not at all clear on how to link and configure the media entities.


It's all documented at Documentation/media/v4l-drivers/imx.rst.
Follow the SabreAuto pipeline setup example.







Additionally I've found that on an IMX6S/IMX6DL we crash while
registering the media-ic subdev's:
[3.975473] imx-media: Registered subdev ipu1_csi1_mux
[3.980921] imx-media: Registered subdev ipu1_csi0_mux
[4.003205] imx-media: Registered subdev ipu1_ic_prpenc
[4.025373] imx-media: Registered subdev ipu1_ic_prpvf
[4.037944] [ cut here ]
[4.042571] Kernel BUG at c06717dc [verbose debug info unavailable]
[4.048845] Internal error: Oops - BUG: 0 [#1] SMP ARM
[4.053990] Modules linked in:
[4.057076] CPU: 1 PID: 1 Comm: swapper/0 Not tainted
4.9.0-rc6-00524-g84dad6e-dirty #446
[4.065260] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
...
[4.296250] [] (v4l2_subdev_init) from []
(imx_ic_probe+0x94/0x1ac)
[4.304271] [] (imx_ic_probe) from []
(platform_drv_probe+0x54/0xb8)
[4.312373]  r9:c0d5e858 r8: r7:fdfb r6:c0e5dbf8
r5:da603810 r4:c16738d8
[4.320129] [] (platform_drv_probe) from []
(driver_probe_device+0x20c/0x2c0)
[4.329010]  r7:c0e5dbf8 r6: r5:da603810 r4:c16738d8
[4.334681] [] (driver_probe_device) from []
(__driver_attach+0xc8/0xcc)
[4.343129]  r9:c0d5e858 r8: r7: r6:da603844
r5:c0e5dbf8 r4:da603810
[4.350889] [] (__driver_attach) from []
(bus_for_each_dev+0x74/0xa8)
[4.359078]  r7: r6:c0515a2c r5:c0e5dbf8 r4:
[4.364753] [] (bus_for_each_dev) from []
(driver_attach+0x20/0x28)

I assume there is an iteration that needs a test on a missing pointer
only available on chips with both IPU's or PRP


Yep, I only have quad boards here so I haven't gotten around to
testing on S/DL.

But it looks like I forgot to clear out the csi subdev pointer array before
passing it to imx_media_of_parse(). I think that might explain the OOPS
above. Try this patch:

diff --git a/drivers/staging/media/imx/imx-media-dev.c 
b/drivers/staging/media/imx/imx-media-dev.c

index 357654d..0cf2d61 100644
--- a/drivers/staging/media/imx/imx-media-dev.c
+++ b/drivers/staging/media/imx/imx-media-dev.c
@@ -379,7 +379,7 @@ static int imx_media_probe(struct platform_device *pdev)
 {
struct device *dev = >dev;
struct device_node *node = dev->of_node;
-   struct imx_media_subdev *csi[4];
+   struct imx_media_subdev *csi[4] = {0};
struct imx_media_dev *imxmd;
int ret;


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 v3 00/24] i.MX Media Driver

[Tim Harvey]

On Fri, Jan 6, 2017 at 6:11 PM, Steve Longerbeam  wrote:
> In version 3:
>
> Changes suggested by Rob Herring :
>
>   - prepended FIM node properties with vendor prefix "fsl,".
>
>   - make mipi csi-2 receiver compatible string SoC specific:
> "fsl,imx6-mipi-csi2" instead of "fsl,imx-mipi-csi2".
>
>   - redundant "_clk" removed from mipi csi-2 receiver clock-names property.
>
>   - removed board-specific info from the media driver binding doc. These
> were all related to sensor bindings, which already are (adv7180)
> or will be (ov564x) covered in separate binding docs. All reference
> board info not related to DT bindings has been moved to
> Documentation/media/v4l-drivers/imx.rst.
>
>   - removed "_mipi" from the OV5640 compatible string.
>
> Changes suggested by Vladimir Zapolskiy :
>
>   Mostly cosmetic/non-functional changes which I won't list here, except
>   for the following:
>
>   - spin_lock_irqsave() changed to spin_lock() in a couple interrupt handlers.
>
>   - fixed some unnecessary of_node_put()'s in for_each_child_of_node() loops.
>
>   - check/handle return code from required reg property of CSI port nodes.
>
>   - check/handle return code from clk_prepare_enable().
>
> Changes suggested by Fabio Estevam :
>
>   - switch to VGEN3 Analog Vdd supply assuming rev. C SabreSD boards.
>
>   - finally got around to passing valid IOMUX pin config values to the
> pin groups.
>
> Other changes:
>
>   - removed the FIM properties that overrided the v4l2 FIM control defaults
> values. This was left-over from a requirement of a customer and is not
> necessary here.
>
>   - The FIM must be explicitly enabled in the fim child node under the CSI
> port nodes, using the status property. If not enabled, FIM v4l2 controls
> will not appear in the video capture driver.
>
>   - brought in additional media types patch from Philipp Zabel. Use new
> MEDIA_ENT_F_VID_IF_BRIDGE in mipi csi-2 receiver subdev.
>
>   - brought in latest platform generic video multiplexer subdevice driver
> from Philipp Zabel (squashed with patch that uses new MEDIA_ENT_F_MUX).
>
>   - removed imx-media-of.h, moved those prototypes into imx-media.h.
>
>


Hi Steve,

I took a stab at testing this today on a gw51xx which has an adv7180
hooked up as follows:
- i2c3@0x20
- 8bit data bus from DAT12 to DAT19, HSYNC, VSYNC, PIXCLK on CSI0 pads
(CSI0_IPU1)
- PWRDWN# on MX6QDL_PAD_CSI0_DATA_EN__GPIO5_IO20
- IRQ# on MX6QDL_PAD_CSI0_DAT5__GPIO5_IO23
- all three analog inputs available to off-board connector

My patch to the imx6qdl-gw51xx dtsi is:
diff --git a/arch/arm/boot/dts/imx6qdl-gw51xx.dtsi
b/arch/arm/boot/dts/imx6qdl-gw51xx.dtsi
index afec2c7..2583d72 100644
--- a/arch/arm/boot/dts/imx6qdl-gw51xx.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw51xx.dtsi
@@ -165,6 +174,52 @@
pinctrl-names = "default";
pinctrl-0 = <_i2c3>;
status = "okay";
+
+   camera: adv7180@20 {
+   compatible = "adi,adv7180";
+   pinctrl-names = "default";
+   pinctrl-0 = <_adv7180>;
+   reg = <0x20>;
+   powerdown-gpio = < 20 GPIO_ACTIVE_LOW>;
+   interrupt-parent = <>;
+   interrupts = <23 GPIO_ACTIVE_LOW>;
+   inputs = <0x00 0x01 0x02>;
+   input-names = "ADV7180 Composite on Ain1",
+ "ADV7180 Composite on Ain2",
+ "ADV7180 Composite on Ain3";
+
+   port {
+   adv7180_to_ipu1_csi0_mux: endpoint {
+   remote-endpoint =
<_csi0_mux_from_parallel_sensor>;
+   bus-width = <8>;
+   };
+   };
+   };
+};
+
+_csi0_from_ipu1_csi0_mux {
+   bus-width = <8>;
+};
+
+_csi0_mux_from_parallel_sensor {
+   remote-endpoint = <_to_ipu1_csi0_mux>;
+   bus-width = <8>;
+};
+
+_csi0 {
+   pinctrl-names = "default";
+   pinctrl-0 = <_ipu1_csi0>;
+
+   /* enable frame interval monitor on this port */
+   fim {
+   status = "okay";
+   };
 };

  {
@@ -236,6 +291,13 @@

  {
imx6qdl-gw51xx {
+   pinctrl_adv7180: adv7180grp {
+   fsl,pins = <
+   MX6QDL_PAD_CSI0_DAT5__GPIO5_IO23
 0x0001b0b0 /* VIDDEC_IRQ# */
+   MX6QDL_PAD_CSI0_DATA_EN__GPIO5_IO20
 0x4001b0b0 /* VIDDEC_EN */
+   >;
+   };
+
pinctrl_enet: enetgrp {
fsl,pins = <
MX6QDL_PAD_RGMII_RXC__RGMII_RXC 0x1b030
@@ -306,6 +368,22 @@
>;
};

+   pinctrl_ipu1_csi0: ipu1csi0grp { /* IPU1_CSI0: 8-bit input */
+   fsl,pins = <
+
MX6QDL_PAD_CSI0_DAT12__IPU1_CSI0_DATA120x1b0b0
+

[PATCH v3 00/24] i.MX Media Driver

[Steve Longerbeam]

In version 3:

Changes suggested by Rob Herring :

  - prepended FIM node properties with vendor prefix "fsl,".

  - make mipi csi-2 receiver compatible string SoC specific:
"fsl,imx6-mipi-csi2" instead of "fsl,imx-mipi-csi2".

  - redundant "_clk" removed from mipi csi-2 receiver clock-names property.

  - removed board-specific info from the media driver binding doc. These
were all related to sensor bindings, which already are (adv7180)
or will be (ov564x) covered in separate binding docs. All reference
board info not related to DT bindings has been moved to
Documentation/media/v4l-drivers/imx.rst.

  - removed "_mipi" from the OV5640 compatible string.

Changes suggested by Vladimir Zapolskiy :

  Mostly cosmetic/non-functional changes which I won't list here, except
  for the following:

  - spin_lock_irqsave() changed to spin_lock() in a couple interrupt handlers.

  - fixed some unnecessary of_node_put()'s in for_each_child_of_node() loops.

  - check/handle return code from required reg property of CSI port nodes.

  - check/handle return code from clk_prepare_enable().

Changes suggested by Fabio Estevam :

  - switch to VGEN3 Analog Vdd supply assuming rev. C SabreSD boards.

  - finally got around to passing valid IOMUX pin config values to the
pin groups.

Other changes:

  - removed the FIM properties that overrided the v4l2 FIM control defaults
values. This was left-over from a requirement of a customer and is not
necessary here.

  - The FIM must be explicitly enabled in the fim child node under the CSI
port nodes, using the status property. If not enabled, FIM v4l2 controls
will not appear in the video capture driver.

  - brought in additional media types patch from Philipp Zabel. Use new
MEDIA_ENT_F_VID_IF_BRIDGE in mipi csi-2 receiver subdev.

  - brought in latest platform generic video multiplexer subdevice driver
from Philipp Zabel (squashed with patch that uses new MEDIA_ENT_F_MUX).

  - removed imx-media-of.h, moved those prototypes into imx-media.h.


Philipp Zabel (3):
  ARM: dts: imx6qdl: Add mipi_ipu1/2 multiplexers, mipi_csi, and their
connections
  add mux and video interface bridge entity functions
  platform: add video-multiplexer subdevice driver

Steve Longerbeam (21):
  [media] dt-bindings: Add bindings for i.MX media driver
  ARM: dts: imx6qdl: Add compatible, clocks, irqs to MIPI CSI-2 node
  ARM: dts: imx6qdl: add media device
  ARM: dts: imx6qdl-sabrelite: remove erratum ERR006687 workaround
  ARM: dts: imx6-sabrelite: add OV5642 and OV5640 camera sensors
  ARM: dts: imx6-sabresd: add OV5642 and OV5640 camera sensors
  ARM: dts: imx6-sabreauto: create i2cmux for i2c3
  ARM: dts: imx6-sabreauto: add reset-gpios property for max7310_b
  ARM: dts: imx6-sabreauto: add pinctrl for gpt input capture
  ARM: dts: imx6-sabreauto: add the ADV7180 video decoder
  UAPI: Add media UAPI Kbuild file
  media: Add userspace header file for i.MX
  media: Add i.MX media core driver
  media: imx: Add CSI subdev driver
  media: imx: Add SMFC subdev driver
  media: imx: Add IC subdev drivers
  media: imx: Add Camera Interface subdev driver
  media: imx: Add MIPI CSI-2 Receiver subdev driver
  media: imx: Add MIPI CSI-2 OV5640 sensor subdev driver
  media: imx: Add Parallel OV5642 sensor subdev driver
  ARM: imx_v6_v7_defconfig: Enable staging video4linux drivers

 Documentation/devicetree/bindings/media/imx.txt|   57 +
 .../bindings/media/video-multiplexer.txt   |   59 +
 Documentation/media/uapi/mediactl/media-types.rst  |   22 +
 Documentation/media/v4l-drivers/imx.rst|  443 ++
 arch/arm/boot/dts/imx6dl-sabrelite.dts |5 +
 arch/arm/boot/dts/imx6dl-sabresd.dts   |5 +
 arch/arm/boot/dts/imx6dl.dtsi  |  187 +
 arch/arm/boot/dts/imx6q-sabrelite.dts  |6 +
 arch/arm/boot/dts/imx6q-sabresd.dts|5 +
 arch/arm/boot/dts/imx6q.dtsi   |  127 +
 arch/arm/boot/dts/imx6qdl-sabreauto.dtsi   |  147 +-
 arch/arm/boot/dts/imx6qdl-sabrelite.dtsi   |  122 +-
 arch/arm/boot/dts/imx6qdl-sabresd.dtsi |  114 +-
 arch/arm/boot/dts/imx6qdl.dtsi |   25 +-
 arch/arm/configs/imx_v6_v7_defconfig   |   12 +-
 drivers/media/platform/Kconfig |8 +
 drivers/media/platform/Makefile|2 +
 drivers/media/platform/video-multiplexer.c |  472 +++
 drivers/staging/media/Kconfig  |2 +
 drivers/staging/media/Makefile |1 +
 drivers/staging/media/imx/Kconfig  |   36 +
 drivers/staging/media/imx/Makefile |   15 +
 drivers/staging/media/imx/TODO |   22 +
 drivers/staging/media/imx/imx-camif.c  | 1000 +
 drivers/staging/media/imx/imx-csi.c|  644 +++