Re: [PATCH v3 01/24] [media] dt-bindings: Add bindings for i.MX media driver

2017-01-16 Thread Steve Longerbeam 



On 01/16/2017 04:09 AM, Philipp Zabel wrote:

On Fri, 2017-01-13 at 11:03 -0800, Steve Longerbeam wrote:

On 01/13/2017 03:55 AM, Philipp Zabel wrote:

Am Freitag, den 06.01.2017, 18:11 -0800 schrieb Steve Longerbeam:

Add bindings documentation for the i.MX media driver.

Signed-off-by: Steve Longerbeam 
---
   Documentation/devicetree/bindings/media/imx.txt | 57 
+
   1 file changed, 57 insertions(+)
   create mode 100644 Documentation/devicetree/bindings/media/imx.txt

diff --git a/Documentation/devicetree/bindings/media/imx.txt 
b/Documentation/devicetree/bindings/media/imx.txt
new file mode 100644
index 000..254b64a
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/imx.txt
@@ -0,0 +1,57 @@
+Freescale i.MX Media Video Devices
+
+Video Media Controller node
+---
+
+This is the parent media controller node for video capture support.
+
+Required properties:
+- compatible : "fsl,imx-media";

Would you be opposed to calling this "capture-subsystem" instead of
"imx-media"? We already use "fsl,imx-display-subsystem" and
"fsl,imx-gpu-subsystem" for the display and GPU compound devices.

sure. Some pie-in-the-sky day when DRM and media are unified,
there could be a single device that handles them all,

Indeed :)


  but for now
I'm fine with "fsl,capture-subsystem".

Actually, I meant fsl,imx-capture-subsystem.


right, I caught my error and that is the name chosen.


  fsl,imx-media-subsystem
would be fine, too. Either way, I'll be happy if it looks similar to the
other two.

[...]

This is a clever method to get better frame timestamps. Too bad about
the routing requirements. Can this be used on Nitrogen6X?

Absolutely, this support just needs use of the input-capture channels in the
imx GPT. I still need to submit the patch to the imx-gpt driver that adds an
input capture API, so at this point fsl,input-capture-channel has no effect,
but it does work (tested on SabreAuto).

Nice.

[...]

+Required properties:
+- compatible   : "fsl,imx6-mipi-csi2";

I think this should get an additional "snps,dw-mipi-csi2" compatible,
since the only i.MX6 specific part is the bolted-on IPU2CSI gasket.

right, minus the gasket it's a Synopsys core. I'll add that compatible flag.
Or should wait until the day this subdev is exported for general use, after
pulling out the gasket specifics?

It can be added right away.


ok, I will add.

Steve




+- reg   : physical base address and length of the register set;
+- clocks   : the MIPI CSI-2 receiver requires three clocks: hsi_tx
+  (the DPHY clock), video_27m, and eim_sel;

Note that hsi_tx is incorrectly named. CCGR3[CG8] just happens to be the
shared gate bit that gates the HSI clocks as well as the MIPI
"ac_clk_125m", "cfg_clk", "ips_clk", and "pll_refclk" inputs to the mipi
csi-2 core, but we are missing shared gate clocks in the clock tree for
these.

Yes, so many clocks for the MIPI core. Why so many? I would think
there would need to be at most three: a clock for the MIPI CSI-2 core
and HSI core, and a clock for the D-PHY (oh and maybe a clock for an
M-PHY if there is one). I have no clue what all these other clocks are.
But anyway, a single gating bit, CCGR3[CG8], seems to enable them all.

I would imagine the CSI-2 core has a high-speed clock input from the
D-PHY for serial input, an APB clock for register access (ips_clk), and
a pixel clock input for the parallel output (pixel_clk), at least.
The D-PHY will have a PLL reference input (pll_refclk?) and probably its
own register clock (cfg_clk?).

I've looked at the MIPI DSI chapter, and it looks like ac_clk_125m is
used for DSI only.


Both cfg_clk and pll_refclk are sourced from video_27m, so "cfg" ->
video_27m seems fine.
But I don't get "dphy".

I presume it's the clock for the D-PHY.


   Which input clock would that correspond to?
"pll_refclk?"

the mux at CDCDR says it comes from PLL3_120M, or PLL2_PFD2.

I think that makes sense.

regards
Philipp

Re: [PATCH v3 01/24] [media] dt-bindings: Add bindings for i.MX media driver

2017-01-16 Thread Steve Longerbeam 



On 01/16/2017 04:09 AM, Philipp Zabel wrote:

On Fri, 2017-01-13 at 11:03 -0800, Steve Longerbeam wrote:

On 01/13/2017 03:55 AM, Philipp Zabel wrote:

Am Freitag, den 06.01.2017, 18:11 -0800 schrieb Steve Longerbeam:

Add bindings documentation for the i.MX media driver.

Signed-off-by: Steve Longerbeam 
---
   Documentation/devicetree/bindings/media/imx.txt | 57 
+
   1 file changed, 57 insertions(+)
   create mode 100644 Documentation/devicetree/bindings/media/imx.txt

diff --git a/Documentation/devicetree/bindings/media/imx.txt 
b/Documentation/devicetree/bindings/media/imx.txt
new file mode 100644
index 000..254b64a
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/imx.txt
@@ -0,0 +1,57 @@
+Freescale i.MX Media Video Devices
+
+Video Media Controller node
+---
+
+This is the parent media controller node for video capture support.
+
+Required properties:
+- compatible : "fsl,imx-media";

Would you be opposed to calling this "capture-subsystem" instead of
"imx-media"? We already use "fsl,imx-display-subsystem" and
"fsl,imx-gpu-subsystem" for the display and GPU compound devices.

sure. Some pie-in-the-sky day when DRM and media are unified,
there could be a single device that handles them all,

Indeed :)


  but for now
I'm fine with "fsl,capture-subsystem".

Actually, I meant fsl,imx-capture-subsystem.


right, I caught my error and that is the name chosen.


  fsl,imx-media-subsystem
would be fine, too. Either way, I'll be happy if it looks similar to the
other two.

[...]

This is a clever method to get better frame timestamps. Too bad about
the routing requirements. Can this be used on Nitrogen6X?

Absolutely, this support just needs use of the input-capture channels in the
imx GPT. I still need to submit the patch to the imx-gpt driver that adds an
input capture API, so at this point fsl,input-capture-channel has no effect,
but it does work (tested on SabreAuto).

Nice.

[...]

+Required properties:
+- compatible   : "fsl,imx6-mipi-csi2";

I think this should get an additional "snps,dw-mipi-csi2" compatible,
since the only i.MX6 specific part is the bolted-on IPU2CSI gasket.

right, minus the gasket it's a Synopsys core. I'll add that compatible flag.
Or should wait until the day this subdev is exported for general use, after
pulling out the gasket specifics?

It can be added right away.


ok, I will add.

Steve




+- reg   : physical base address and length of the register set;
+- clocks   : the MIPI CSI-2 receiver requires three clocks: hsi_tx
+  (the DPHY clock), video_27m, and eim_sel;

Note that hsi_tx is incorrectly named. CCGR3[CG8] just happens to be the
shared gate bit that gates the HSI clocks as well as the MIPI
"ac_clk_125m", "cfg_clk", "ips_clk", and "pll_refclk" inputs to the mipi
csi-2 core, but we are missing shared gate clocks in the clock tree for
these.

Yes, so many clocks for the MIPI core. Why so many? I would think
there would need to be at most three: a clock for the MIPI CSI-2 core
and HSI core, and a clock for the D-PHY (oh and maybe a clock for an
M-PHY if there is one). I have no clue what all these other clocks are.
But anyway, a single gating bit, CCGR3[CG8], seems to enable them all.

I would imagine the CSI-2 core has a high-speed clock input from the
D-PHY for serial input, an APB clock for register access (ips_clk), and
a pixel clock input for the parallel output (pixel_clk), at least.
The D-PHY will have a PLL reference input (pll_refclk?) and probably its
own register clock (cfg_clk?).

I've looked at the MIPI DSI chapter, and it looks like ac_clk_125m is
used for DSI only.


Both cfg_clk and pll_refclk are sourced from video_27m, so "cfg" ->
video_27m seems fine.
But I don't get "dphy".

I presume it's the clock for the D-PHY.


   Which input clock would that correspond to?
"pll_refclk?"

the mux at CDCDR says it comes from PLL3_120M, or PLL2_PFD2.

I think that makes sense.

regards
Philipp

Re: [PATCH v3 01/24] [media] dt-bindings: Add bindings for i.MX media driver

2017-01-16 Thread Philipp Zabel 
On Fri, 2017-01-13 at 11:03 -0800, Steve Longerbeam wrote:
> 
> On 01/13/2017 03:55 AM, Philipp Zabel wrote:
> > Am Freitag, den 06.01.2017, 18:11 -0800 schrieb Steve Longerbeam:
> >> Add bindings documentation for the i.MX media driver.
> >>
> >> Signed-off-by: Steve Longerbeam 
> >> ---
> >>   Documentation/devicetree/bindings/media/imx.txt | 57 
> >> +
> >>   1 file changed, 57 insertions(+)
> >>   create mode 100644 Documentation/devicetree/bindings/media/imx.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/media/imx.txt 
> >> b/Documentation/devicetree/bindings/media/imx.txt
> >> new file mode 100644
> >> index 000..254b64a
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/media/imx.txt
> >> @@ -0,0 +1,57 @@
> >> +Freescale i.MX Media Video Devices
> >> +
> >> +Video Media Controller node
> >> +---
> >> +
> >> +This is the parent media controller node for video capture support.
> >> +
> >> +Required properties:
> >> +- compatible : "fsl,imx-media";
> > Would you be opposed to calling this "capture-subsystem" instead of
> > "imx-media"? We already use "fsl,imx-display-subsystem" and
> > "fsl,imx-gpu-subsystem" for the display and GPU compound devices.
> 
> sure. Some pie-in-the-sky day when DRM and media are unified,
> there could be a single device that handles them all,

Indeed :)

>  but for now
> I'm fine with "fsl,capture-subsystem".

Actually, I meant fsl,imx-capture-subsystem. fsl,imx-media-subsystem
would be fine, too. Either way, I'll be happy if it looks similar to the
other two.

[...]
> > This is a clever method to get better frame timestamps. Too bad about
> > the routing requirements. Can this be used on Nitrogen6X?
> 
> Absolutely, this support just needs use of the input-capture channels in the
> imx GPT. I still need to submit the patch to the imx-gpt driver that adds an
> input capture API, so at this point fsl,input-capture-channel has no effect,
> but it does work (tested on SabreAuto).

Nice.

[...]
> >> +Required properties:
> >> +- compatible  : "fsl,imx6-mipi-csi2";
> > I think this should get an additional "snps,dw-mipi-csi2" compatible,
> > since the only i.MX6 specific part is the bolted-on IPU2CSI gasket.
> 
> right, minus the gasket it's a Synopsys core. I'll add that compatible flag.
> Or should wait until the day this subdev is exported for general use, after
> pulling out the gasket specifics?

It can be added right away.

> >> +- reg   : physical base address and length of the register set;
> >> +- clocks  : the MIPI CSI-2 receiver requires three clocks: hsi_tx
> >> +  (the DPHY clock), video_27m, and eim_sel;
> > Note that hsi_tx is incorrectly named. CCGR3[CG8] just happens to be the
> > shared gate bit that gates the HSI clocks as well as the MIPI
> > "ac_clk_125m", "cfg_clk", "ips_clk", and "pll_refclk" inputs to the mipi
> > csi-2 core, but we are missing shared gate clocks in the clock tree for
> > these.
> 
> Yes, so many clocks for the MIPI core. Why so many? I would think
> there would need to be at most three: a clock for the MIPI CSI-2 core
> and HSI core, and a clock for the D-PHY (oh and maybe a clock for an
> M-PHY if there is one). I have no clue what all these other clocks are.
> But anyway, a single gating bit, CCGR3[CG8], seems to enable them all.

I would imagine the CSI-2 core has a high-speed clock input from the
D-PHY for serial input, an APB clock for register access (ips_clk), and
a pixel clock input for the parallel output (pixel_clk), at least.
The D-PHY will have a PLL reference input (pll_refclk?) and probably its
own register clock (cfg_clk?).

I've looked at the MIPI DSI chapter, and it looks like ac_clk_125m is
used for DSI only.

> > Both cfg_clk and pll_refclk are sourced from video_27m, so "cfg" ->
> > video_27m seems fine.
> > But I don't get "dphy".
> 
> I presume it's the clock for the D-PHY.
>
> >   Which input clock would that correspond to?
> > "pll_refclk?"
> 
> the mux at CDCDR says it comes from PLL3_120M, or PLL2_PFD2.

I think that makes sense.

regards
Philipp

Re: [PATCH v3 01/24] [media] dt-bindings: Add bindings for i.MX media driver

2017-01-16 Thread Philipp Zabel 
On Fri, 2017-01-13 at 11:03 -0800, Steve Longerbeam wrote:
> 
> On 01/13/2017 03:55 AM, Philipp Zabel wrote:
> > Am Freitag, den 06.01.2017, 18:11 -0800 schrieb Steve Longerbeam:
> >> Add bindings documentation for the i.MX media driver.
> >>
> >> Signed-off-by: Steve Longerbeam 
> >> ---
> >>   Documentation/devicetree/bindings/media/imx.txt | 57 
> >> +
> >>   1 file changed, 57 insertions(+)
> >>   create mode 100644 Documentation/devicetree/bindings/media/imx.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/media/imx.txt 
> >> b/Documentation/devicetree/bindings/media/imx.txt
> >> new file mode 100644
> >> index 000..254b64a
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/media/imx.txt
> >> @@ -0,0 +1,57 @@
> >> +Freescale i.MX Media Video Devices
> >> +
> >> +Video Media Controller node
> >> +---
> >> +
> >> +This is the parent media controller node for video capture support.
> >> +
> >> +Required properties:
> >> +- compatible : "fsl,imx-media";
> > Would you be opposed to calling this "capture-subsystem" instead of
> > "imx-media"? We already use "fsl,imx-display-subsystem" and
> > "fsl,imx-gpu-subsystem" for the display and GPU compound devices.
> 
> sure. Some pie-in-the-sky day when DRM and media are unified,
> there could be a single device that handles them all,

Indeed :)

>  but for now
> I'm fine with "fsl,capture-subsystem".

Actually, I meant fsl,imx-capture-subsystem. fsl,imx-media-subsystem
would be fine, too. Either way, I'll be happy if it looks similar to the
other two.

[...]
> > This is a clever method to get better frame timestamps. Too bad about
> > the routing requirements. Can this be used on Nitrogen6X?
> 
> Absolutely, this support just needs use of the input-capture channels in the
> imx GPT. I still need to submit the patch to the imx-gpt driver that adds an
> input capture API, so at this point fsl,input-capture-channel has no effect,
> but it does work (tested on SabreAuto).

Nice.

[...]
> >> +Required properties:
> >> +- compatible  : "fsl,imx6-mipi-csi2";
> > I think this should get an additional "snps,dw-mipi-csi2" compatible,
> > since the only i.MX6 specific part is the bolted-on IPU2CSI gasket.
> 
> right, minus the gasket it's a Synopsys core. I'll add that compatible flag.
> Or should wait until the day this subdev is exported for general use, after
> pulling out the gasket specifics?

It can be added right away.

> >> +- reg   : physical base address and length of the register set;
> >> +- clocks  : the MIPI CSI-2 receiver requires three clocks: hsi_tx
> >> +  (the DPHY clock), video_27m, and eim_sel;
> > Note that hsi_tx is incorrectly named. CCGR3[CG8] just happens to be the
> > shared gate bit that gates the HSI clocks as well as the MIPI
> > "ac_clk_125m", "cfg_clk", "ips_clk", and "pll_refclk" inputs to the mipi
> > csi-2 core, but we are missing shared gate clocks in the clock tree for
> > these.
> 
> Yes, so many clocks for the MIPI core. Why so many? I would think
> there would need to be at most three: a clock for the MIPI CSI-2 core
> and HSI core, and a clock for the D-PHY (oh and maybe a clock for an
> M-PHY if there is one). I have no clue what all these other clocks are.
> But anyway, a single gating bit, CCGR3[CG8], seems to enable them all.

I would imagine the CSI-2 core has a high-speed clock input from the
D-PHY for serial input, an APB clock for register access (ips_clk), and
a pixel clock input for the parallel output (pixel_clk), at least.
The D-PHY will have a PLL reference input (pll_refclk?) and probably its
own register clock (cfg_clk?).

I've looked at the MIPI DSI chapter, and it looks like ac_clk_125m is
used for DSI only.

> > Both cfg_clk and pll_refclk are sourced from video_27m, so "cfg" ->
> > video_27m seems fine.
> > But I don't get "dphy".
> 
> I presume it's the clock for the D-PHY.
>
> >   Which input clock would that correspond to?
> > "pll_refclk?"
> 
> the mux at CDCDR says it comes from PLL3_120M, or PLL2_PFD2.

I think that makes sense.

regards
Philipp

Re: [PATCH v3 01/24] [media] dt-bindings: Add bindings for i.MX media driver

2017-01-13 Thread Steve Longerbeam 



On 01/13/2017 03:55 AM, Philipp Zabel wrote:

Am Freitag, den 06.01.2017, 18:11 -0800 schrieb Steve Longerbeam:

Add bindings documentation for the i.MX media driver.

Signed-off-by: Steve Longerbeam 
---
  Documentation/devicetree/bindings/media/imx.txt | 57 +
  1 file changed, 57 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/media/imx.txt

diff --git a/Documentation/devicetree/bindings/media/imx.txt 
b/Documentation/devicetree/bindings/media/imx.txt
new file mode 100644
index 000..254b64a
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/imx.txt
@@ -0,0 +1,57 @@
+Freescale i.MX Media Video Devices
+
+Video Media Controller node
+---
+
+This is the parent media controller node for video capture support.
+
+Required properties:
+- compatible : "fsl,imx-media";

Would you be opposed to calling this "capture-subsystem" instead of
"imx-media"? We already use "fsl,imx-display-subsystem" and
"fsl,imx-gpu-subsystem" for the display and GPU compound devices.


sure. Some pie-in-the-sky day when DRM and media are unified,
there could be a single device that handles them all, but for now
I'm fine with "fsl,capture-subsystem".


+- ports  : Should contain a list of phandles pointing to camera
+  sensor interface ports of IPU devices
+
+
+fim child node
+--
+
+This is an optional child node of the ipu_csi port nodes. If present and
+available, it enables the Frame Interval Monitor. Its properties can be
+used to modify the method in which the FIM measures frame intervals.
+Refer to Documentation/media/v4l-drivers/imx.rst for more info on the
+Frame Interval Monitor.
+
+Optional properties:
+- fsl,input-capture-channel: an input capture channel and channel flags,
+specified as . The channel number
+must be 0 or 1. The flags can be
+IRQ_TYPE_EDGE_RISING, IRQ_TYPE_EDGE_FALLING, or
+IRQ_TYPE_EDGE_BOTH, and specify which input
+capture signal edge will trigger the input
+capture event. If an input capture channel is
+specified, the FIM will use this method to
+measure frame intervals instead of via the EOF
+interrupt. The input capture method is much
+preferred over EOF as it is not subject to
+interrupt latency errors. However it requires
+routing the VSYNC or FIELD output signals of
+the camera sensor to one of the i.MX input
+capture pads (SD1_DAT0, SD1_DAT1), which also
+gives up support for SD1.

This is a clever method to get better frame timestamps. Too bad about
the routing requirements. Can this be used on Nitrogen6X?


Absolutely, this support just needs use of the input-capture channels in the
imx GPT. I still need to submit the patch to the imx-gpt driver that adds an
input capture API, so at this point fsl,input-capture-channel has no effect,
but it does work (tested on SabreAuto).




+
+mipi_csi2 node
+--
+
+This is the device node for the MIPI CSI-2 Receiver, required for MIPI
+CSI-2 sensors.
+
+Required properties:
+- compatible   : "fsl,imx6-mipi-csi2";

I think this should get an additional "snps,dw-mipi-csi2" compatible,
since the only i.MX6 specific part is the bolted-on IPU2CSI gasket.


right, minus the gasket it's a Synopsys core. I'll add that compatible flag.
Or should wait until the day this subdev is exported for general use, after
pulling out the gasket specifics?





+- reg   : physical base address and length of the register set;
+- clocks   : the MIPI CSI-2 receiver requires three clocks: hsi_tx
+  (the DPHY clock), video_27m, and eim_sel;

Note that hsi_tx is incorrectly named. CCGR3[CG8] just happens to be the
shared gate bit that gates the HSI clocks as well as the MIPI
"ac_clk_125m", "cfg_clk", "ips_clk", and "pll_refclk" inputs to the mipi
csi-2 core, but we are missing shared gate clocks in the clock tree for
these.


Yes, so many clocks for the MIPI core. Why so many? I would think
there would need to be at most three: a clock for the MIPI CSI-2 core
and HSI core, and a clock for the D-PHY (oh and maybe a clock for an
M-PHY if there is one). I have no clue what all these other clocks are.
But anyway, a single gating bit, CCGR3[CG8], seems to enable them all.


Both cfg_clk and pll_refclk are sourced from video_27m, so "cfg" ->
video_27m seems fine.
But I don't get "dphy".


I presume it's the clock for the D-PHY.


  Which input clock would that correspond to?
"pll_refclk?"


the mux at CDCDR says it comes from PLL3_120M, or PLL2_PFD2.



Also the pixel clock input is a gate after

Re: [PATCH v3 01/24] [media] dt-bindings: Add bindings for i.MX media driver

2017-01-13 Thread Steve Longerbeam 



On 01/13/2017 03:55 AM, Philipp Zabel wrote:

Am Freitag, den 06.01.2017, 18:11 -0800 schrieb Steve Longerbeam:

Add bindings documentation for the i.MX media driver.

Signed-off-by: Steve Longerbeam 
---
  Documentation/devicetree/bindings/media/imx.txt | 57 +
  1 file changed, 57 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/media/imx.txt

diff --git a/Documentation/devicetree/bindings/media/imx.txt 
b/Documentation/devicetree/bindings/media/imx.txt
new file mode 100644
index 000..254b64a
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/imx.txt
@@ -0,0 +1,57 @@
+Freescale i.MX Media Video Devices
+
+Video Media Controller node
+---
+
+This is the parent media controller node for video capture support.
+
+Required properties:
+- compatible : "fsl,imx-media";

Would you be opposed to calling this "capture-subsystem" instead of
"imx-media"? We already use "fsl,imx-display-subsystem" and
"fsl,imx-gpu-subsystem" for the display and GPU compound devices.


sure. Some pie-in-the-sky day when DRM and media are unified,
there could be a single device that handles them all, but for now
I'm fine with "fsl,capture-subsystem".


+- ports  : Should contain a list of phandles pointing to camera
+  sensor interface ports of IPU devices
+
+
+fim child node
+--
+
+This is an optional child node of the ipu_csi port nodes. If present and
+available, it enables the Frame Interval Monitor. Its properties can be
+used to modify the method in which the FIM measures frame intervals.
+Refer to Documentation/media/v4l-drivers/imx.rst for more info on the
+Frame Interval Monitor.
+
+Optional properties:
+- fsl,input-capture-channel: an input capture channel and channel flags,
+specified as . The channel number
+must be 0 or 1. The flags can be
+IRQ_TYPE_EDGE_RISING, IRQ_TYPE_EDGE_FALLING, or
+IRQ_TYPE_EDGE_BOTH, and specify which input
+capture signal edge will trigger the input
+capture event. If an input capture channel is
+specified, the FIM will use this method to
+measure frame intervals instead of via the EOF
+interrupt. The input capture method is much
+preferred over EOF as it is not subject to
+interrupt latency errors. However it requires
+routing the VSYNC or FIELD output signals of
+the camera sensor to one of the i.MX input
+capture pads (SD1_DAT0, SD1_DAT1), which also
+gives up support for SD1.

This is a clever method to get better frame timestamps. Too bad about
the routing requirements. Can this be used on Nitrogen6X?


Absolutely, this support just needs use of the input-capture channels in the
imx GPT. I still need to submit the patch to the imx-gpt driver that adds an
input capture API, so at this point fsl,input-capture-channel has no effect,
but it does work (tested on SabreAuto).




+
+mipi_csi2 node
+--
+
+This is the device node for the MIPI CSI-2 Receiver, required for MIPI
+CSI-2 sensors.
+
+Required properties:
+- compatible   : "fsl,imx6-mipi-csi2";

I think this should get an additional "snps,dw-mipi-csi2" compatible,
since the only i.MX6 specific part is the bolted-on IPU2CSI gasket.


right, minus the gasket it's a Synopsys core. I'll add that compatible flag.
Or should wait until the day this subdev is exported for general use, after
pulling out the gasket specifics?





+- reg   : physical base address and length of the register set;
+- clocks   : the MIPI CSI-2 receiver requires three clocks: hsi_tx
+  (the DPHY clock), video_27m, and eim_sel;

Note that hsi_tx is incorrectly named. CCGR3[CG8] just happens to be the
shared gate bit that gates the HSI clocks as well as the MIPI
"ac_clk_125m", "cfg_clk", "ips_clk", and "pll_refclk" inputs to the mipi
csi-2 core, but we are missing shared gate clocks in the clock tree for
these.


Yes, so many clocks for the MIPI core. Why so many? I would think
there would need to be at most three: a clock for the MIPI CSI-2 core
and HSI core, and a clock for the D-PHY (oh and maybe a clock for an
M-PHY if there is one). I have no clue what all these other clocks are.
But anyway, a single gating bit, CCGR3[CG8], seems to enable them all.


Both cfg_clk and pll_refclk are sourced from video_27m, so "cfg" ->
video_27m seems fine.
But I don't get "dphy".


I presume it's the clock for the D-PHY.


  Which input clock would that correspond to?
"pll_refclk?"


the mux at CDCDR says it comes from PLL3_120M, or PLL2_PFD2.



Also the pixel clock input is a gate after aclk_podf (which we call
eim_podf),

Re: [PATCH v3 01/24] [media] dt-bindings: Add bindings for i.MX media driver

2017-01-13 Thread Philipp Zabel 
Am Freitag, den 06.01.2017, 18:11 -0800 schrieb Steve Longerbeam:
> Add bindings documentation for the i.MX media driver.
> 
> Signed-off-by: Steve Longerbeam 
> ---
>  Documentation/devicetree/bindings/media/imx.txt | 57 
> +
>  1 file changed, 57 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/imx.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/imx.txt 
> b/Documentation/devicetree/bindings/media/imx.txt
> new file mode 100644
> index 000..254b64a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/imx.txt
> @@ -0,0 +1,57 @@
> +Freescale i.MX Media Video Devices
> +
> +Video Media Controller node
> +---
> +
> +This is the parent media controller node for video capture support.
> +
> +Required properties:
> +- compatible : "fsl,imx-media";

Would you be opposed to calling this "capture-subsystem" instead of
"imx-media"? We already use "fsl,imx-display-subsystem" and
"fsl,imx-gpu-subsystem" for the display and GPU compound devices.

> +- ports  : Should contain a list of phandles pointing to camera
> +sensor interface ports of IPU devices
> +
> +
> +fim child node
> +--
> +
> +This is an optional child node of the ipu_csi port nodes. If present and
> +available, it enables the Frame Interval Monitor. Its properties can be
> +used to modify the method in which the FIM measures frame intervals.
> +Refer to Documentation/media/v4l-drivers/imx.rst for more info on the
> +Frame Interval Monitor.
> +
> +Optional properties:
> +- fsl,input-capture-channel: an input capture channel and channel flags,
> +  specified as . The channel number
> +  must be 0 or 1. The flags can be
> +  IRQ_TYPE_EDGE_RISING, IRQ_TYPE_EDGE_FALLING, or
> +  IRQ_TYPE_EDGE_BOTH, and specify which input
> +  capture signal edge will trigger the input
> +  capture event. If an input capture channel is
> +  specified, the FIM will use this method to
> +  measure frame intervals instead of via the EOF
> +  interrupt. The input capture method is much
> +  preferred over EOF as it is not subject to
> +  interrupt latency errors. However it requires
> +  routing the VSYNC or FIELD output signals of
> +  the camera sensor to one of the i.MX input
> +  capture pads (SD1_DAT0, SD1_DAT1), which also
> +  gives up support for SD1.

This is a clever method to get better frame timestamps. Too bad about
the routing requirements. Can this be used on Nitrogen6X?

> +
> +mipi_csi2 node
> +--
> +
> +This is the device node for the MIPI CSI-2 Receiver, required for MIPI
> +CSI-2 sensors.
> +
> +Required properties:
> +- compatible : "fsl,imx6-mipi-csi2";

I think this should get an additional "snps,dw-mipi-csi2" compatible,
since the only i.MX6 specific part is the bolted-on IPU2CSI gasket.

> +- reg   : physical base address and length of the register set;
> +- clocks : the MIPI CSI-2 receiver requires three clocks: hsi_tx
> +  (the DPHY clock), video_27m, and eim_sel;

Note that hsi_tx is incorrectly named. CCGR3[CG8] just happens to be the
shared gate bit that gates the HSI clocks as well as the MIPI
"ac_clk_125m", "cfg_clk", "ips_clk", and "pll_refclk" inputs to the mipi
csi-2 core, but we are missing shared gate clocks in the clock tree for
these.
Both cfg_clk and pll_refclk are sourced from video_27m, so "cfg" ->
video_27m seems fine.
But I don't get "dphy". Which input clock would that correspond to?
"pll_refclk?"
Also the pixel clock input is a gate after aclk_podf (which we call
eim_podf), not aclk_sel (eim_sel).

> +- clock-names: must contain "dphy", "cfg", "pix";
> +
> +Optional properties:
> +- interrupts : must contain two level-triggered interrupts,
> +  in order: 100 and 101;

regards
Philipp

Re: [PATCH v3 01/24] [media] dt-bindings: Add bindings for i.MX media driver

2017-01-13 Thread Philipp Zabel 
Am Freitag, den 06.01.2017, 18:11 -0800 schrieb Steve Longerbeam:
> Add bindings documentation for the i.MX media driver.
> 
> Signed-off-by: Steve Longerbeam 
> ---
>  Documentation/devicetree/bindings/media/imx.txt | 57 
> +
>  1 file changed, 57 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/imx.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/imx.txt 
> b/Documentation/devicetree/bindings/media/imx.txt
> new file mode 100644
> index 000..254b64a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/imx.txt
> @@ -0,0 +1,57 @@
> +Freescale i.MX Media Video Devices
> +
> +Video Media Controller node
> +---
> +
> +This is the parent media controller node for video capture support.
> +
> +Required properties:
> +- compatible : "fsl,imx-media";

Would you be opposed to calling this "capture-subsystem" instead of
"imx-media"? We already use "fsl,imx-display-subsystem" and
"fsl,imx-gpu-subsystem" for the display and GPU compound devices.

> +- ports  : Should contain a list of phandles pointing to camera
> +sensor interface ports of IPU devices
> +
> +
> +fim child node
> +--
> +
> +This is an optional child node of the ipu_csi port nodes. If present and
> +available, it enables the Frame Interval Monitor. Its properties can be
> +used to modify the method in which the FIM measures frame intervals.
> +Refer to Documentation/media/v4l-drivers/imx.rst for more info on the
> +Frame Interval Monitor.
> +
> +Optional properties:
> +- fsl,input-capture-channel: an input capture channel and channel flags,
> +  specified as . The channel number
> +  must be 0 or 1. The flags can be
> +  IRQ_TYPE_EDGE_RISING, IRQ_TYPE_EDGE_FALLING, or
> +  IRQ_TYPE_EDGE_BOTH, and specify which input
> +  capture signal edge will trigger the input
> +  capture event. If an input capture channel is
> +  specified, the FIM will use this method to
> +  measure frame intervals instead of via the EOF
> +  interrupt. The input capture method is much
> +  preferred over EOF as it is not subject to
> +  interrupt latency errors. However it requires
> +  routing the VSYNC or FIELD output signals of
> +  the camera sensor to one of the i.MX input
> +  capture pads (SD1_DAT0, SD1_DAT1), which also
> +  gives up support for SD1.

This is a clever method to get better frame timestamps. Too bad about
the routing requirements. Can this be used on Nitrogen6X?

> +
> +mipi_csi2 node
> +--
> +
> +This is the device node for the MIPI CSI-2 Receiver, required for MIPI
> +CSI-2 sensors.
> +
> +Required properties:
> +- compatible : "fsl,imx6-mipi-csi2";

I think this should get an additional "snps,dw-mipi-csi2" compatible,
since the only i.MX6 specific part is the bolted-on IPU2CSI gasket.

> +- reg   : physical base address and length of the register set;
> +- clocks : the MIPI CSI-2 receiver requires three clocks: hsi_tx
> +  (the DPHY clock), video_27m, and eim_sel;

Note that hsi_tx is incorrectly named. CCGR3[CG8] just happens to be the
shared gate bit that gates the HSI clocks as well as the MIPI
"ac_clk_125m", "cfg_clk", "ips_clk", and "pll_refclk" inputs to the mipi
csi-2 core, but we are missing shared gate clocks in the clock tree for
these.
Both cfg_clk and pll_refclk are sourced from video_27m, so "cfg" ->
video_27m seems fine.
But I don't get "dphy". Which input clock would that correspond to?
"pll_refclk?"
Also the pixel clock input is a gate after aclk_podf (which we call
eim_podf), not aclk_sel (eim_sel).

> +- clock-names: must contain "dphy", "cfg", "pix";
> +
> +Optional properties:
> +- interrupts : must contain two level-triggered interrupts,
> +  in order: 100 and 101;

regards
Philipp