Re: [19/38] ARM: dts: imx6-sabrelite: add video capture ports and connections

2016-06-16 Thread Gary Bisson
Steve, All,

On Tue, Jun 14, 2016 at 03:49:15PM -0700, Steve Longerbeam wrote:
> Defines the host video capture device node and an OV5642 camera sensor
> node on i2c2. The host capture device connects to the OV5642 via the
> parallel-bus mux input on the ipu1_csi0_mux.
> 
> Note there is a pin conflict with GPIO6. This pin functions as a power
> input pin to the OV5642, but ENET requires it to wake-up the ARM cores
> on normal RX and TX packet done events (see 6261c4c8). So by default,
> capture is disabled, enable by uncommenting __OV5642_CAPTURE__ macro.
> Ethernet will still work just not quite as well.

Actually the following patch fixes this issue and has already been
applied on Shawn's tree:
https://patchwork.kernel.org/patch/9153523/

Also, this follow-up patch declared the HW workaround for SabreLite:
https://patchwork.kernel.org/patch/9153525/

So ideally, once those two patches land on your base tree, you could get
rid of the #define and remove the HW workaround declaration.

Finally, I'll test the series on Sabre-Lite this week.

Regards,
Gary
--
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: [19/38] ARM: dts: imx6-sabrelite: add video capture ports and connections

2016-06-17 Thread Gary Bisson
Steve, All,

On Thu, Jun 16, 2016 at 10:32:31AM +0200, Gary Bisson wrote:
> Steve, All,
> 
> On Tue, Jun 14, 2016 at 03:49:15PM -0700, Steve Longerbeam wrote:
> > Defines the host video capture device node and an OV5642 camera sensor
> > node on i2c2. The host capture device connects to the OV5642 via the
> > parallel-bus mux input on the ipu1_csi0_mux.
> > 
> > Note there is a pin conflict with GPIO6. This pin functions as a power
> > input pin to the OV5642, but ENET requires it to wake-up the ARM cores
> > on normal RX and TX packet done events (see 6261c4c8). So by default,
> > capture is disabled, enable by uncommenting __OV5642_CAPTURE__ macro.
> > Ethernet will still work just not quite as well.
> 
> Actually the following patch fixes this issue and has already been
> applied on Shawn's tree:
> https://patchwork.kernel.org/patch/9153523/
> 
> Also, this follow-up patch declared the HW workaround for SabreLite:
> https://patchwork.kernel.org/patch/9153525/
> 
> So ideally, once those two patches land on your base tree, you could get
> rid of the #define and remove the HW workaround declaration.
> 
> Finally, I'll test the series on Sabre-Lite this week.

I've applied this series on top of Shawn tree (for-next branch) in order
not to worry about the GPIO6 workaround.

Although the camera seems to get enumerated properly, I can't seem to
get anything from it. See log:
http://pastebin.com/xnw1ujUq

In your cover letter, you said that you have not run through
v4l2-compliance. How have you tested the capture?

Also, why isn't the OV5640 MIPI camera declared on the SabreLite device
tree?

Let me know if I can help testing/updating things on the SabreLite.

Regards,
Gary
--
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: [19/38] ARM: dts: imx6-sabrelite: add video capture ports and connections

2016-06-20 Thread Gary Bisson
Steve, Jack, All,

On Fri, Jun 17, 2016 at 12:01:41PM -0700, Steve Longerbeam wrote:
> 
> On 06/17/2016 08:18 AM, Gary Bisson wrote:
> > Steve, All,
> > 
> > On Thu, Jun 16, 2016 at 10:32:31AM +0200, Gary Bisson wrote:
> > > Steve, All,
> > > 
> > > On Tue, Jun 14, 2016 at 03:49:15PM -0700, Steve Longerbeam wrote:
> > > > Defines the host video capture device node and an OV5642 camera sensor
> > > > node on i2c2. The host capture device connects to the OV5642 via the
> > > > parallel-bus mux input on the ipu1_csi0_mux.
> > > > 
> > > > Note there is a pin conflict with GPIO6. This pin functions as a power
> > > > input pin to the OV5642, but ENET requires it to wake-up the ARM cores
> > > > on normal RX and TX packet done events (see 6261c4c8). So by default,
> > > > capture is disabled, enable by uncommenting __OV5642_CAPTURE__ macro.
> > > > Ethernet will still work just not quite as well.
> > > Actually the following patch fixes this issue and has already been
> > > applied on Shawn's tree:
> > > https://patchwork.kernel.org/patch/9153523/
> > > 
> > > Also, this follow-up patch declared the HW workaround for SabreLite:
> > > https://patchwork.kernel.org/patch/9153525/
> > > 
> > > So ideally, once those two patches land on your base tree, you could get
> > > rid of the #define and remove the HW workaround declaration.
> > > 
> > > Finally, I'll test the series on Sabre-Lite this week.
> > I've applied this series on top of Shawn tree (for-next branch) in order
> > not to worry about the GPIO6 workaround.
> > 
> > Although the camera seems to get enumerated properly, I can't seem to
> > get anything from it. See log:
> > http://pastebin.com/xnw1ujUq
> 
> Hi Gary, the driver does not implement vidioc_cropcap, it has
> switched to the new selection APIs and v4l2src should be using
> vidioc_g_selection instead of vidioc_cropcap.

I confirm this was the issue, your patch fixes it.

> > In your cover letter, you said that you have not run through
> > v4l2-compliance. How have you tested the capture?
> 
> I use v4l2-ctl, and have used v4l2src in the past, but that was before
> switching to the selection APIs. Try the attached hack that adds
> vidioc_cropcap back in, and see how far you get on SabreLite with
> v4l2src. I tried  the following on SabreAuto:
> 
> gst-launch-1.0 v4l2src io_mode=4 !
> "video/x-raw,format=RGB16,width=640,height=480" ! fbdevsink

I confirm that works with OV5642 on SabreLite:
Tested-by: Gary Bisson 

> > Also, why isn't the OV5640 MIPI camera declared on the SabreLite device
> > tree?
> 
> See Jack Mitchell's patch at http://ix.io/TTg. Thanks Jack! I will work on
> incorporating it.

I've tried that patch have a some comments:
- When applied, no capture shows up any more, instead I have two m2m
  v4l2 devices [1].
- OV5640 Mipi is assigned the same address as OV5642, therefore both
  can't work at the same time right now. There's a register in the
  camera that allows to modify its I2C address, see this patch [2].
- How is the mclk working in this patch? It should be using the PWM3
  output to generate a ~22MHz clock. I would expect the use of a
  pwm-clock node [3].

Also another remark on both OV5642 and OV5640 patches, is it recommended
to use 0x8000 pin muxing value? This leaves it to the bootloader to
properly setup the I/O. It sounds safer to properly set them up in the
device tree in my opinion in order not to be dependent on the bootloader.

All the above remarks said, thanks for this work on i.MX camera support,
that feature will be highly appreciated.

Regards,
Gary

[1] http://pastebin.com/i5MrhB1h
[2] https://github.com/boundarydevices/linux-imx6/commit/f915806d
[3] 
http://lxr.free-electrons.com/source/Documentation/devicetree/bindings/clock/pwm-clock.txt
--
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: [19/38] ARM: dts: imx6-sabrelite: add video capture ports and connections

2016-06-20 Thread Gary Bisson
Jack, All,

On Mon, Jun 20, 2016 at 10:44:44AM +0100, Jack Mitchell wrote:
> 
> > I've tried that patch have a some comments:
> > - When applied, no capture shows up any more, instead I have two m2m
> >   v4l2 devices [1].
> > - OV5640 Mipi is assigned the same address as OV5642, therefore both
> 
> Yes, I only have one device attached in my scenario.

Thanks for confirming.

> >   can't work at the same time right now. There's a register in the
> >   camera that allows to modify its I2C address, see this patch [2].
> > - How is the mclk working in this patch? It should be using the PWM3
> 
> As mentioned I have an eCon sensor board [1] which generates it's own clock
> on the board and as such I don't need the PWM signal, just the two GPIOs.

Oh ok, thanks I didn't this sensor board was different than ours [1].

But in your patch, you specifically disable pwm3, what's the reason for
it?

> >   output to generate a ~22MHz clock. I would expect the use of a
> >   pwm-clock node [3].
> > 
> > Also another remark on both OV5642 and OV5640 patches, is it recommended
> > to use 0x8000 pin muxing value? This leaves it to the bootloader to
> 
> I also wondered about this, but didn't know if the pinmux driver did this
> based on the define name? I tried it both ways and it worked so I just left
> it as it was.

Actually my phrasing is wrong, the muxing is ok. Yes depending on the
name a pin will be muxed to one function or another. The problem is the
pad configuration (pull-up, pull-down etc..). I am not surprised that it
works, because the bootloader should properly set those. But it would be
safer IMO not to rely on it.

Regards,
Gary

[1] https://boundarydevices.com/product/nit6x_5mp_mipi/
--
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: [16/22] ARM: dts: nitrogen6x: Add dtsi for BD_HDMI_MIPI HDMI to MIPI CSI-2 receiver board

2016-10-10 Thread Gary Bisson
Hi Philipp, All,

On Fri, Oct 07, 2016 at 06:01:01PM +0200, Philipp Zabel wrote:
> Add device tree nodes for the BD_HDMI_MIPI HDMI to MIPI CSI-2 receiver
> board with a TC358743 connected to the Nitrogen6X MIPI CSI-2 input
> connector.

I've tested this series on my Nitrogen6x + BD_HDMI_MIPI daughter board
and have a few questions.

First, why is the tc358743 node in a separate dtsi file? Is this in
order to avoid a failed probe during bootup if the daughter board is
not present? Is this what should be done for every capture device that
targets this platform (like the OV5640 or OV5642)?

Can you provide some details on your testing procedure? In my case I've
reached a point where I get the same 'media-ctl --print-dot' output as
the one from your cover letter but I can't seem to set the EDID nor to
have a gstreamer pipeline (to fakesink). All the EDID v4l2-ctl commands
return "Inappropriate ioctl for device".

Do not hesitate to CC me to Boundary Devices related patches so I can
test them and give some feedback.

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


Re: [PATCH v2 00/21] Basic i.MX IPUv3 capture support

2016-10-17 Thread Gary Bisson
Hi Philipp,

On Fri, Oct 14, 2016 at 07:34:20PM +0200, Philipp Zabel wrote:
> Hi,
> 
> the second round removes the prepare_stream callback and instead lets the
> intermediate subdevices propagate s_stream calls to their sources rather
> than individually calling s_stream on each subdevice from the bridge driver.
> This is similar to how drm bridges recursively call into their next neighbor.
> It makes it easier to do bringup ordering on a per-link level, as long as the
> source preparation can be done at s_power, and the sink can just prepare, call
> s_stream on its source, and then enable itself inside s_stream. Obviously this
> would only work in a generic fashion if all asynchronous subdevices with both
> inputs and outputs would propagate s_stream to their source subdevices.
> 
> Changes since v1:
>  - Propagate field and colorspace in ipucsi_subdev_set_format.
>  - Remove v4l2_media_subdev_prepare_stream and v4l2_media_subdev_s_stream,
>let subdevices propagate s_stream calls to their upstream subdevices
>themselves.
>  - Various fixes (see individual patches for details)

For the whole series:
Tested-by: Gary Bisson 

Tested on Nitrogen6x + BD_HDMI_MIPI daughter board on linux-next
20161016.

This required using your v4l2-ctl patch to set the EDID if the source
output can't be forced:
https://patchwork.kernel.org/patch/6097201/
BTW, do you have any update on this? Because it looks like the
VIDIOC_SUBDEV_QUERYCAP hasn't been implemented since your patch (March
2015).

Then I followed the procedure you gave here:
https://patchwork.kernel.org/patch/9366503/

For those interested in trying it out, note that kmssink requires to use
Gstreamer 1.9.x.

I have a few remarks:
- I believe it would help having a patch that sets imx_v6_v7_defconfig
  with the proper options in this series
- Not related to this series, I couldn't boot the board unless I disable
  the PCIe driver, have you experienced the same issue?
- Is there a way not to set all the links manually using media-ctl? I
  expected all the formats to be negotiated automatically once a stream
  is properly detected.
- As discussed last week, the Nitrogen6x dtsi file shouldn't be
  included, instead an overlay would be more appropriate. Maybe the log
  should contain a comment about this.

Let me know if I need to add that Tested-by to every single patch so it
appears on Patchwork.

Regards,
Gary
--
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