Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1570212 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v3 00/24] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-01-31 01:50 +0100
    Re: [PATCH v3 00/24] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-01-31 02:10 +0100
      Re: [PATCH v3 00/24] i.MX Media Driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-01-31 03:10 +0100
    Re: [PATCH v3 00/24] i.MX Media Driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-01-31 02:30 +0100
      Re: [PATCH v3 00/24] i.MX Media Driver Philipp Zabel <p.zabel@pengutronix.de> - 2017-01-31 11:00 +0100
      Re: [PATCH v3 00/24] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-01-31 11:30 +0100
      Re: [PATCH v3 00/24] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-01-31 12:10 +0100
        Re: [PATCH v3 00/24] i.MX Media Driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-01 00:50 +0100
          Re: [PATCH v3 00/24] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-02-01 01:30 +0100
            Re: [PATCH v3 00/24] i.MX Media Driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-01 03:00 +0100
              Re: [PATCH v3 00/24] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-02-01 10:30 +0100

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

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: 0x00000200

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.

[toc] | [next] | [standalone]

#1570231

On Tue, Jan 31, 2017 at 12:45:11AM +0000, 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_smfc                6639  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.

[toc] | [prev] | [next] | [standalone]

#1570252

On 01/30/2017 05:06 PM, Russell King - ARM Linux wrote:
> On Tue, Jan 31, 2017 at 12:45:11AM +0000, 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_smfc                6639  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_mipi            25988  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).
>

[toc] | [prev] | [next] | [standalone]

#1570235

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: 0x00000200
>
> 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

[toc] | [prev] | [next] | [standalone]

#1570539

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: 0x00000200
> >
> > 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

[toc] | [prev] | [next] | [standalone]

#1570576

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.

[toc] | [prev] | [next] | [standalone]

#1570609

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.

[toc] | [prev] | [next] | [standalone]

#1571209

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.
>

[toc] | [prev] | [next] | [standalone]

#1571219

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.

So, for your FIM, you need to know if there is any reduction in frame
rate through that pipeline, and looking for a "MEDIA_ENT_F_CAM_SENSOR"
subdev node isn't going to tell you that.  The frame rate really needs
to be carried through and I suspect you need to accept the frame rate
into each subdev so it can be passed along the chain by the application
configuring the pipeline.

The last bit from the above is the "I-want-your-bus-format" bit which
I haven't fully worked out how to eliminate - I understand the reason
you need that (so you can appropriately configure the CSI with the CSI2
data type code.)  The CSI2 data type code comes from the format
configured on the CSI sink pad, so the only information you're really
using there is "are we sinking data from a CSI2 interface."

You _could_ walk down the graph from the CSI looking for a subdev that
responds to g_mbus_config that reports CSI2, but I'm not sure that's
going to last - I've seen an email from Hans saying that he'd like
g_mbus_config to go away (to patch 13/24, for the vidsw_g_mbus_config()
function):

"I am not certain this op is needed at all. In the current kernel this
 op is onlyused by soc_camera, pxa_camera and omap3isp (somewhat dubious).
 Normally this information should come from the device tree and there
 should be no need for this op.

 My (tentative) long-term plan was to get rid of this op.

 If you don't need it, then I recommend it is removed."

So, if that goes away, the CSI subdev needs a completely different way
to get that information, and it shouldn't be coming from the camera
sensor subdev, but (imho) really from the CSI2 subdev.

This is probably something that needs to be discussed with media people
to work out how to replace the g_mbus_config call with something more
acceptable to resolve this issue.

-- 
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.

[toc] | [prev] | [next] | [standalone]

#1571241

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
> +-----------------------+                                |
>                ^--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.
>
> So, for your FIM, you need to know if there is any reduction in frame
> rate through that pipeline, and looking for a "MEDIA_ENT_F_CAM_SENSOR"
> subdev node isn't going to tell you that.  The frame rate really needs
> to be carried through and I suspect you need to accept the frame rate
> into each subdev so it can be passed along the chain by the application
> configuring the pipeline.
>
> The last bit from the above is the "I-want-your-bus-format" bit which
> I haven't fully worked out how to eliminate - I understand the reason
> you need that (so you can appropriately configure the CSI with the CSI2
> data type code.)  The CSI2 data type code comes from the format
> configured on the CSI sink pad, so the only information you're really
> using there is "are we sinking data from a CSI2 interface."
>
> You _could_ walk down the graph from the CSI looking for a subdev that
> responds to g_mbus_config that reports CSI2, but I'm not sure that's
> going to last - I've seen an email from Hans saying that he'd like
> g_mbus_config to go away (to patch 13/24, for the vidsw_g_mbus_config()
> function):
>
> "I am not certain this op is needed at all. In the current kernel this
>   op is onlyused by soc_camera, pxa_camera and omap3isp (somewhat dubious).
>   Normally this information should come from the device tree and there
>   should be no need for this op.
>
>   My (tentative) long-term plan was to get rid of this op.
>
>   If you don't need it, then I recommend it is removed."
>
> So, if that goes away, the CSI subdev needs a completely different way
> to get that information, and it shouldn't be coming from the camera
> sensor subdev, but (imho) really from the CSI2 subdev.
>
> This is probably something that needs to be discussed with media people
> to work out how to replace the g_mbus_config call with something more
> acceptable to resolve this issue.
>

[toc] | [prev] | [next] | [standalone]

#1571355

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.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web