Re: [PATCH v3 21/24] media: imx: Add MIPI CSI-2 Receiver subdev driver

[Philipp Zabel]

Hi Steve,

On Mon, 2017-02-13 at 15:20 -0800, Steve Longerbeam wrote:
[...]
> > It seems the OV5640 driver never puts its the CSI-2 lanes into stop
> > state while not streaming.
> 
> Yes I found that as well.
> 
> But good news, I finally managed to coax the OV5640's clock lane
> into LP-11 state! It was accomplished by setting bit 5 in OV5640 register
> 0x4800. The datasheet describes this bit as "Gate clock lane when no
> packet to transmit". But I may have also got this to work with the 
> additional
> write 1 to bits 4-6 in register 0x3019 ("MIPI CLK/data lane state in sleep
> mode" - setting 1 to these bits selects LP-11 for sleep mode).
> 
> So I am now finally able to call csi2_dphy_wait_stopstate() in
> csi2_s_stream(ON).

That's good news.

> So for the TC35874, you shouldn't see a timeout in csi2_s_stream(ON)
> any longer.
> 
> I have updated both ov5640.c and imx6-mipi-csi2.c in the wip branch.
> Can you try again? I have not applied your patch below, because I
> don't think that is needed anymore.

Thanks, I'll test tomorrow.

> And speaking of the TC35874, I received this module for the SabreLite
> from Boundary Devices (thanks BD!). So I can finally help you with
> debug/test. Are there any patches you need to send to me (against
> wip branch) for this support, or can I use what exists now in media
> tree? Also any scripts or help with running.

That's even better news. I have pushed my my wip branch, which contains
some colorspace work and experiments to pass through query/g_/s_std
subdev calls so bypassing the pipeline can be avoided. Also, there's the
Nitrogen6X device tree that I've been using to test:

https://git.pengutronix.de/git/pza/linux imx-media-staging-md-wip

> >   With the recent s_stream reordering,
> > streaming from TC358743 does not work anymore, since imx6-mipi-csi2
> > s_stream is called before tc358743 s_stream, while all lanes are still
> > in stop state. Then it waits for the clock lane to become active and
> > fails. I have applied the following patch to revert the reordering
> > locally to get it to work again.
> >
> > The initialization order, as Russell pointed out, should be:
> >
> > 1. reset the D-PHY.
> > 2. place MIPI sensor in LP-11 state
> > 3. perform D-PHY initialisation
> > 4. configure CSI2 lanes and de-assert resets and shutdown signals
> >
> > So csi2_s_stream should wait for stop state on all lanes (the result of
> > 2.) before dphy_init (3.), not wait for active clock afterwards. That
> > should happen only after sensor_s_stream was called. So unless we are
> > allowed to reorder steps 1. and 2., we might need the prepare_stream
> > callback after all.
> 
> Agreed!
> 
> See my new FIXME comment in imx6-mipi-csi2.c.

Looks good. I wonder if enabling the phy clock isn't part of step 3.
though.

> I agree we might need a new subdev op .prepare_stream(). This
> op would be implemented by imx6-mipi-csi2.c, and would carry
> out steps 3, 4, 5 (instead of currently by csi2_s_stream()). Then
> step 6 would finally become available as csi2_s_stream().
> 
> And then we must re-order stream on to start sensor first, then
> csi2, as in your patch below.

regards
Philipp

Re: [PATCH v3 21/24] media: imx: Add MIPI CSI-2 Receiver subdev driver

[Steve Longerbeam]

(#!##* Thunderbird! resending text only)

Hi Philipp,


On 02/13/2017 01:20 AM, Philipp Zabel wrote:

Hi Steve,

On Thu, 2017-02-09 at 15:51 -0800, Steve Longerbeam wrote:

On 02/09/2017 03:49 PM, Steve Longerbeam wrote:

On 02/08/2017 03:42 PM, Russell King - ARM Linux wrote:

On Wed, Feb 08, 2017 at 03:23:53PM -0800, Steve Longerbeam wrote:

Actually, this exact function already exists as
dw_mipi_dsi_phy_write in
drivers/gpu/drm/rockchip/dw-mipi-dsi.c, and it looks like the D-PHY
register 0x44 might contain a field called HSFREQRANGE_SEL.

Thanks for pointing out drivers/gpu/drm/rockchip/dw-mipi-dsi.c.
It's clear from that driver that there probably needs to be a fuller
treatment of the D-PHY programming here, but I don't know where
to find the MIPI CSI-2 D-PHY documentation for the i.MX6. The code
in imxcsi2_reset() was also pulled from FSL, and that's all I really
have
to go on for the D-PHY programming. I assume the D-PHY is also a
Synopsys core, like the host controller, but the i.MX6 manual doesn't
cover it.

Why exactly?  What problems are you seeing that would necessitate a
more detailed programming of the D-PHY?  From my testing, I can wind
a 2-lane MIPI bus on iMX6D from 912Mbps per lane down to (eg) 308Mbps
per lane with your existing code without any issues.

That's good to hear.

Just from my experience with struggles to get the CSI2 receiver
up and running with an active clock lane, and my suspicions that
some of that could be due to my lack of understanding of the D-PHY
programming.

But I should add that after a re-org of the sequence, it looks more stable
now. Tested on both the SabreSD and SabreLite with the OV5640.

It seems the OV5640 driver never puts its the CSI-2 lanes into stop
state while not streaming.


Yes I found that as well.

But good news, I finally managed to coax the OV5640's clock lane
into LP-11 state! It was accomplished by setting bit 5 in OV5640 register
0x4800. The datasheet describes this bit as "Gate clock lane when no
packet to transmit". But I may have also got this to work with the 
additional

write 1 to bits 4-6 in register 0x3019 ("MIPI CLK/data lane state in sleep
mode" - setting 1 to these bits selects LP-11 for sleep mode).

So I am now finally able to call csi2_dphy_wait_stopstate() in
csi2_s_stream(ON).

So for the TC35874, you shouldn't see a timeout in csi2_s_stream(ON)
any longer.

I have updated both ov5640.c and imx6-mipi-csi2.c in the wip branch.
Can you try again? I have not applied your patch below, because I
don't think that is needed anymore.

And speaking of the TC35874, I received this module for the SabreLite
from Boundary Devices (thanks BD!). So I can finally help you with
debug/test. Are there any patches you need to send to me (against
wip branch) for this support, or can I use what exists now in media
tree? Also any scripts or help with running.



  With the recent s_stream reordering,
streaming from TC358743 does not work anymore, since imx6-mipi-csi2
s_stream is called before tc358743 s_stream, while all lanes are still
in stop state. Then it waits for the clock lane to become active and
fails. I have applied the following patch to revert the reordering
locally to get it to work again.

The initialization order, as Russell pointed out, should be:

1. reset the D-PHY.
2. place MIPI sensor in LP-11 state
3. perform D-PHY initialisation
4. configure CSI2 lanes and de-assert resets and shutdown signals

So csi2_s_stream should wait for stop state on all lanes (the result of
2.) before dphy_init (3.), not wait for active clock afterwards. That
should happen only after sensor_s_stream was called. So unless we are
allowed to reorder steps 1. and 2., we might need the prepare_stream
callback after all.


Agreed!

See my new FIXME comment in imx6-mipi-csi2.c.

I agree we might need a new subdev op .prepare_stream(). This
op would be implemented by imx6-mipi-csi2.c, and would carry
out steps 3, 4, 5 (instead of currently by csi2_s_stream()). Then
step 6 would finally become available as csi2_s_stream().

And then we must re-order stream on to start sensor first, then
csi2, as in your patch below.

Steve


More specifically, the chapter 40.3.1 "Startup Sequence" in the i.MX6DQ
reference manual states:

1. Deassert presetn signal (global reset)
- This is probably the APB global reset, so not something we have to
  care about.
2. Configure MIPI Camera Sensor to put all Tx lanes in PL-11 state
3. D-PHY initialization (write 0x14 to address 0x44)
4. CSI2 Controller programming
- Set N_LANES
- Deassert PHY_SHUTDOWNZ
- Deassert PHY_RSTZ
- Deassert CSI2_RESETN
5. Read the PHY status register (PHY_STATE) to confirm that all data and
clock lanes of the D-PHY are in Stop State
6. Configure the MIPI Camera Sensor to start transmitting a clock on the
D-PHY clock lane
7. CSI2 Controller programming - Read the PHY status register
(PHY_STATE) to confirm that the D-PHY is receiving a clock on the
D-PHY clock lane

2. 

Re: [PATCH v3 21/24] media: imx: Add MIPI CSI-2 Receiver subdev driver

[Philipp Zabel]

Hi Steve,

On Thu, 2017-02-09 at 15:51 -0800, Steve Longerbeam wrote:
> 
> On 02/09/2017 03:49 PM, Steve Longerbeam wrote:
> >
> >
> > On 02/08/2017 03:42 PM, Russell King - ARM Linux wrote:
> >> On Wed, Feb 08, 2017 at 03:23:53PM -0800, Steve Longerbeam wrote:
>  Actually, this exact function already exists as 
>  dw_mipi_dsi_phy_write in
>  drivers/gpu/drm/rockchip/dw-mipi-dsi.c, and it looks like the D-PHY
>  register 0x44 might contain a field called HSFREQRANGE_SEL.
> >>> Thanks for pointing out drivers/gpu/drm/rockchip/dw-mipi-dsi.c.
> >>> It's clear from that driver that there probably needs to be a fuller
> >>> treatment of the D-PHY programming here, but I don't know where
> >>> to find the MIPI CSI-2 D-PHY documentation for the i.MX6. The code
> >>> in imxcsi2_reset() was also pulled from FSL, and that's all I really 
> >>> have
> >>> to go on for the D-PHY programming. I assume the D-PHY is also a
> >>> Synopsys core, like the host controller, but the i.MX6 manual doesn't
> >>> cover it.
> >> Why exactly?  What problems are you seeing that would necessitate a
> >> more detailed programming of the D-PHY?  From my testing, I can wind
> >> a 2-lane MIPI bus on iMX6D from 912Mbps per lane down to (eg) 308Mbps
> >> per lane with your existing code without any issues.
> >
> > That's good to hear.
> >
> > Just from my experience with struggles to get the CSI2 receiver
> > up and running with an active clock lane, and my suspicions that
> > some of that could be due to my lack of understanding of the D-PHY
> > programming.
> 
> But I should add that after a re-org of the sequence, it looks more stable
> now. Tested on both the SabreSD and SabreLite with the OV5640.

It seems the OV5640 driver never puts its the CSI-2 lanes into stop
state while not streaming. With the recent s_stream reordering,
streaming from TC358743 does not work anymore, since imx6-mipi-csi2
s_stream is called before tc358743 s_stream, while all lanes are still
in stop state. Then it waits for the clock lane to become active and
fails. I have applied the following patch to revert the reordering
locally to get it to work again.

The initialization order, as Russell pointed out, should be:

1. reset the D-PHY.
2. place MIPI sensor in LP-11 state
3. perform D-PHY initialisation
4. configure CSI2 lanes and de-assert resets and shutdown signals

So csi2_s_stream should wait for stop state on all lanes (the result of
2.) before dphy_init (3.), not wait for active clock afterwards. That
should happen only after sensor_s_stream was called. So unless we are
allowed to reorder steps 1. and 2., we might need the prepare_stream
callback after all.

More specifically, the chapter 40.3.1 "Startup Sequence" in the i.MX6DQ
reference manual states:

1. Deassert presetn signal (global reset)
   - This is probably the APB global reset, so not something we have to
 care about.
2. Configure MIPI Camera Sensor to put all Tx lanes in PL-11 state
3. D-PHY initialization (write 0x14 to address 0x44)
4. CSI2 Controller programming
   - Set N_LANES
   - Deassert PHY_SHUTDOWNZ
   - Deassert PHY_RSTZ
   - Deassert CSI2_RESETN
5. Read the PHY status register (PHY_STATE) to confirm that all data and
   clock lanes of the D-PHY are in Stop State
6. Configure the MIPI Camera Sensor to start transmitting a clock on the
   D-PHY clock lane
7. CSI2 Controller programming - Read the PHY status register
   (PHY_STATE) to confirm that the D-PHY is receiving a clock on the
   D-PHY clock lane

2. can be done in sensor s_power (which tc358743 currently does) only if
the sensor can still be configured in step 6.
Also, 3./4./5. are csi2 code, 6. is sensor code, and 7. is csi2 code
again. For reliable startup we need csi2 receiver code to be called on
both sides of the sensor enabling its clock lane.

--8<--
>From f12758caa3e1d05980cd2ac07587d125449fc860 Mon Sep 17 00:00:00 2001
From: Philipp Zabel 
Date: Mon, 13 Feb 2017 09:28:27 +0100
Subject: [PATCH] media: imx: revert streamon sequence change

Without this patch, starting streaming from a TC358743 MIPI CSI-2 source
fails with the following error messages:

imx6-mipi-csi2: clock lane timeout, phy_state = 0x06f0
ipu1_csi0: pipeline_set_stream failed with -110

The phy state above has the stopstateclk, rxulpsclknot, and
stopstatedata[3:0] bits set: at this point all lanes are in stop state,
no lane is in ultra low power state or active.
This is no suprise, since tc358743 s_stream(sd, 1) has not been called
due to the recently changed ordering. The imx6-mipi-csi2 s_stream does
wait for the clock lane to become active, so csi2_s_stream must happen
after tc358743_s_stream.

Signed-off-by: Philipp Zabel 
---
 drivers/staging/media/imx/imx-media-utils.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

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

Re: [PATCH v3 21/24] media: imx: Add MIPI CSI-2 Receiver subdev driver

[Steve Longerbeam]

On 02/09/2017 03:49 PM, Steve Longerbeam wrote:



On 02/08/2017 03:42 PM, Russell King - ARM Linux wrote:

On Wed, Feb 08, 2017 at 03:23:53PM -0800, Steve Longerbeam wrote:
Actually, this exact function already exists as 
dw_mipi_dsi_phy_write in

drivers/gpu/drm/rockchip/dw-mipi-dsi.c, and it looks like the D-PHY
register 0x44 might contain a field called HSFREQRANGE_SEL.

Thanks for pointing out drivers/gpu/drm/rockchip/dw-mipi-dsi.c.
It's clear from that driver that there probably needs to be a fuller
treatment of the D-PHY programming here, but I don't know where
to find the MIPI CSI-2 D-PHY documentation for the i.MX6. The code
in imxcsi2_reset() was also pulled from FSL, and that's all I really 
have

to go on for the D-PHY programming. I assume the D-PHY is also a
Synopsys core, like the host controller, but the i.MX6 manual doesn't
cover it.

Why exactly?  What problems are you seeing that would necessitate a
more detailed programming of the D-PHY?  From my testing, I can wind
a 2-lane MIPI bus on iMX6D from 912Mbps per lane down to (eg) 308Mbps
per lane with your existing code without any issues.


That's good to hear.

Just from my experience with struggles to get the CSI2 receiver
up and running with an active clock lane, and my suspicions that
some of that could be due to my lack of understanding of the D-PHY
programming.


But I should add that after a re-org of the sequence, it looks more stable
now. Tested on both the SabreSD and SabreLite with the OV5640.

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 21/24] media: imx: Add MIPI CSI-2 Receiver subdev driver

[Steve Longerbeam]

On 02/08/2017 03:42 PM, Russell King - ARM Linux wrote:

On Wed, Feb 08, 2017 at 03:23:53PM -0800, Steve Longerbeam wrote:

Actually, this exact function already exists as dw_mipi_dsi_phy_write in
drivers/gpu/drm/rockchip/dw-mipi-dsi.c, and it looks like the D-PHY
register 0x44 might contain a field called HSFREQRANGE_SEL.

Thanks for pointing out drivers/gpu/drm/rockchip/dw-mipi-dsi.c.
It's clear from that driver that there probably needs to be a fuller
treatment of the D-PHY programming here, but I don't know where
to find the MIPI CSI-2 D-PHY documentation for the i.MX6. The code
in imxcsi2_reset() was also pulled from FSL, and that's all I really have
to go on for the D-PHY programming. I assume the D-PHY is also a
Synopsys core, like the host controller, but the i.MX6 manual doesn't
cover it.

Why exactly?  What problems are you seeing that would necessitate a
more detailed programming of the D-PHY?  From my testing, I can wind
a 2-lane MIPI bus on iMX6D from 912Mbps per lane down to (eg) 308Mbps
per lane with your existing code without any issues.


That's good to hear.

Just from my experience with struggles to get the CSI2 receiver
up and running with an active clock lane, and my suspicions that
some of that could be due to my lack of understanding of the D-PHY
programming.

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 21/24] media: imx: Add MIPI CSI-2 Receiver subdev driver

[Philipp Zabel]

On Wed, 2017-02-08 at 15:23 -0800, Steve Longerbeam wrote:
[...]
> >> +
> >> +static int imxcsi2_get_fmt(struct v4l2_subdev *sd,
> >> + struct v4l2_subdev_pad_config *cfg,
> >> + struct v4l2_subdev_format *sdformat)
> >> +{
> >> +  struct imxcsi2_dev *csi2 = sd_to_dev(sd);
> >> +
> >> +  sdformat->format = csi2->format_mbus;
> > The output formats are different from the input formats, see the media
> > bus format discussion in the other thread. The input pad is the MIPI
> > CSI-2 bus, but the four output pads are connected to the muxes / CSIs
> > via a 16-bit parallel bus.
> >
> > So if the input format is UYVY8_1X16, for example, the output should be
> > set to UYVY8_2X8.
> 
> Since the output buses from the CSI2IPU gasket are 16-bit
> parallel buses, shouldn't an input format UYVY8_1X16 also be
> the same at the output?

I looked at the reference manual again, and I think I have read this
incorrectly, probably confused by the coloring in Figure 19-10 "YUV422-8
data reception" in the CSI2IPU chapter. It looks like indeed the 16-bit
bus carries UYVY8_1X16, whereas the internal 32-bit bus between MIPI
CSI-2 decoder and the CSI2IPU carries two samples (complete UYVY) per
pixel clock.
So UYVY is straight forward. It's the YUV420, RGB, and RAW formats that
would be interesting to describe correctly.

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 21/24] media: imx: Add MIPI CSI-2 Receiver subdev driver

[Russell King - ARM Linux]

On Wed, Feb 08, 2017 at 03:23:53PM -0800, Steve Longerbeam wrote:
> >Actually, this exact function already exists as dw_mipi_dsi_phy_write in
> >drivers/gpu/drm/rockchip/dw-mipi-dsi.c, and it looks like the D-PHY
> >register 0x44 might contain a field called HSFREQRANGE_SEL.
> 
> Thanks for pointing out drivers/gpu/drm/rockchip/dw-mipi-dsi.c.
> It's clear from that driver that there probably needs to be a fuller
> treatment of the D-PHY programming here, but I don't know where
> to find the MIPI CSI-2 D-PHY documentation for the i.MX6. The code
> in imxcsi2_reset() was also pulled from FSL, and that's all I really have
> to go on for the D-PHY programming. I assume the D-PHY is also a
> Synopsys core, like the host controller, but the i.MX6 manual doesn't
> cover it.

Why exactly?  What problems are you seeing that would necessitate a
more detailed programming of the D-PHY?  From my testing, I can wind
a 2-lane MIPI bus on iMX6D from 912Mbps per lane down to (eg) 308Mbps
per lane with your existing code without any issues.

-- 
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 21/24] media: imx: Add MIPI CSI-2 Receiver subdev driver

[Steve Longerbeam]

On 02/02/2017 03:50 AM, Philipp Zabel wrote:



+   struct v4l2_subdev *src_sd;
+   struct v4l2_subdev *sink_sd[CSI2_NUM_SRC_PADS];

I see no reason to store pointers to the remote v4l2_subdevs.


+   intinput_pad;
+   struct clk *dphy_clk;
+   struct clk *cfg_clk;
+   struct clk *pix_clk; /* what is this? */
+   void __iomem   *base;
+   int intr1;
+   int intr2;

The interrupts are not used, I'd remove them and the dead code in
_probe.


done.


+
+static inline u32 imxcsi2_read(struct imxcsi2_dev *csi2, unsigned int regoff)
+{
+   return readl(csi2->base + regoff);
+}
+
+static inline void imxcsi2_write(struct imxcsi2_dev *csi2, u32 val,
+unsigned int regoff)
+{
+   writel(val, csi2->base + regoff);
+}

Do those two wrappers really make the code more readable?


It doesn't really matter to me either way, I removed these
macros.


+
+static void imxcsi2_enable(struct imxcsi2_dev *csi2, bool enable)
+{
+   if (enable) {
+   imxcsi2_write(csi2, 0x, CSI2_PHY_SHUTDOWNZ);
+   imxcsi2_write(csi2, 0x, CSI2_DPHY_RSTZ);
+   imxcsi2_write(csi2, 0x, CSI2_RESETN);

Given that these registers only contain a single bit, and bits 31:1 are
documented as reserved, 0, I think these should write 1 instead of
0x.


Yes, these lines are lifted from the FSL BSP's mipi csi-2 driver. I
did notice this but left it in place because I was worried about
possible undocumented bits. I tried writing 1 to these registers
and the behavior is the same as before (still works).


+   } else {
+   imxcsi2_write(csi2, 0x0, CSI2_PHY_SHUTDOWNZ);
+   imxcsi2_write(csi2, 0x0, CSI2_DPHY_RSTZ);
+   imxcsi2_write(csi2, 0x0, CSI2_RESETN);
+   }
+}
+
+static void imxcsi2_reset(struct imxcsi2_dev *csi2)
+{
+   imxcsi2_enable(csi2, false);
+
+   imxcsi2_write(csi2, 0x0001, CSI2_PHY_TST_CTRL0);
+   imxcsi2_write(csi2, 0x, CSI2_PHY_TST_CTRL1);
+   imxcsi2_write(csi2, 0x, CSI2_PHY_TST_CTRL0);
+   imxcsi2_write(csi2, 0x0002, CSI2_PHY_TST_CTRL0);
+   imxcsi2_write(csi2, 0x00010044, CSI2_PHY_TST_CTRL1);
+   imxcsi2_write(csi2, 0x, CSI2_PHY_TST_CTRL0);
+   imxcsi2_write(csi2, 0x0014, CSI2_PHY_TST_CTRL1);
+   imxcsi2_write(csi2, 0x0002, CSI2_PHY_TST_CTRL0);
+   imxcsi2_write(csi2, 0x, CSI2_PHY_TST_CTRL0);

These magic constants should be replaced with proper defines for the
documented bitfields, if available.

#define PHY_TESTCLR BIT(0)
#define PHY_TESTCLK BIT(1)

#define PHY_TESTEN  BIT(16)

/* Clear PHY test interface */
imxcsi2_write(csi2, PHY_TESTCLR, CSI2_PHY_TST_CTRL0);
imxcsi2_write(csi2, 0, CSI2_PHY_TST_CTRL1);
imxcsi2_write(csi2, 0, CSI2_PHY_TST_CTRL0);

/* Raise test interface strobe signal */
imxcsi2_write(csi2, PHY_TESTCLK, CSI2_PHY_TST_CTRL0);

/* Configure address write on falling edge and lower strobe signal */
u8 addr = 0x44;
imxcsi2_write(csi2, PHY_TESTEN | addr, CSI2_PHY_TST_CTRL1);
imxcsi2_write(csi2, 0, CSI2_PHY_TST_CTRL0);

/* Configure data write on rising edge and raise strobe signal */
u8 data = 0x14;
imxcsi2_write(csi2, data, CSI2_PHY_TST_CTRL1);
imxcsi2_write(csi2, PHY_TESTCLK, CSI2_PHY_TST_CTRL0);

/* Clear strobe signal */
imxcsi2_write(csi2, 0, CSI2_PHY_TST_CTRL0);

The whole sequence should probably be encapsulated in a
dw_mipi_dphy_write function.

Actually, this exact function already exists as dw_mipi_dsi_phy_write in
drivers/gpu/drm/rockchip/dw-mipi-dsi.c, and it looks like the D-PHY
register 0x44 might contain a field called HSFREQRANGE_SEL.


Thanks for pointing out drivers/gpu/drm/rockchip/dw-mipi-dsi.c.
It's clear from that driver that there probably needs to be a fuller
treatment of the D-PHY programming here, but I don't know where
to find the MIPI CSI-2 D-PHY documentation for the i.MX6. The code
in imxcsi2_reset() was also pulled from FSL, and that's all I really have
to go on for the D-PHY programming. I assume the D-PHY is also a
Synopsys core, like the host controller, but the i.MX6 manual doesn't
cover it.

In any case I've created dw_mipi_csi2_phy_write(), modeled after
dw_mipi_dsi_phy_write(). The "0x14" value is a value derived for
a target max bandwidth per lane of 300 Mbps, at least that is what
drivers/gpu/drm/rockchip/dw-mipi-dsi.c suggests. I've added a FIXME
note that effect, that this value should be derived based on the D-PHY
PLL clock rate and the desired max lane bandwidth.



+   imxcsi2_enable(csi2, true);
+}
+
+static int imxcsi2_dphy_wait(struct imxcsi2_dev *csi2)
+{
+   u32 reg;
+   int i;
+
+   /* wait for mipi sensor ready */

Re: [PATCH v3 21/24] media: imx: Add MIPI CSI-2 Receiver subdev driver

[Philipp Zabel]

On Fri, 2017-01-06 at 18:11 -0800, Steve Longerbeam wrote:
> Adds MIPI CSI-2 Receiver subdev driver. This subdev is required
> for sensors with a MIPI CSI2 interface.
> 
> Signed-off-by: Steve Longerbeam 
> ---
>  drivers/staging/media/imx/Makefile|   1 +
>  drivers/staging/media/imx/imx-mipi-csi2.c | 501 
> ++
>  2 files changed, 502 insertions(+)
>  create mode 100644 drivers/staging/media/imx/imx-mipi-csi2.c
> 
> diff --git a/drivers/staging/media/imx/Makefile 
> b/drivers/staging/media/imx/Makefile
> index fe9e992..0decef7 100644
> --- a/drivers/staging/media/imx/Makefile
> +++ b/drivers/staging/media/imx/Makefile
> @@ -9,3 +9,4 @@ obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-ic.o
>  obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-csi.o
>  obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-smfc.o
>  obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-camif.o
> +obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-mipi-csi2.o
> diff --git a/drivers/staging/media/imx/imx-mipi-csi2.c 
> b/drivers/staging/media/imx/imx-mipi-csi2.c
> new file mode 100644
> index 000..daa6e1d
> --- /dev/null
> +++ b/drivers/staging/media/imx/imx-mipi-csi2.c
> @@ -0,0 +1,501 @@
> +/*
> + * MIPI CSI-2 Receiver Subdev for Freescale i.MX5/6 SOC.
> + *
> + * Copyright (c) 2012-2014 Mentor Graphics Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include "imx-media.h"
> +
> +/*
> + * there must be 5 pads: 1 input pad from sensor, and
> + * the 4 virtual channel output pads
> + */
> +#define CSI2_NUM_SINK_PADS  1
> +#define CSI2_NUM_SRC_PADS   4
> +#define CSI2_NUM_PADS   5
> +
> +struct imxcsi2_dev {
> + struct device  *dev;
> + struct imx_media_dev   *md;
> + struct v4l2_subdev  sd;
> + struct media_pad   pad[CSI2_NUM_PADS];
> + struct v4l2_mbus_framefmt format_mbus;
> + struct v4l2_subdev *src_sd;
> + struct v4l2_subdev *sink_sd[CSI2_NUM_SRC_PADS];

I see no reason to store pointers to the remote v4l2_subdevs.

> + intinput_pad;
> + struct clk *dphy_clk;
> + struct clk *cfg_clk;
> + struct clk *pix_clk; /* what is this? */
> + void __iomem   *base;
> + int intr1;
> + int intr2;

The interrupts are not used, I'd remove them and the dead code in
_probe.

> + struct v4l2_of_bus_mipi_csi2 bus;
> + boolon;
> + boolstream_on;
> +};
> +
> +#define DEVICE_NAME "imx6-mipi-csi2"
> +
> +/* Register offsets */
> +#define CSI2_VERSION0x000
> +#define CSI2_N_LANES0x004
> +#define CSI2_PHY_SHUTDOWNZ  0x008
> +#define CSI2_DPHY_RSTZ  0x00c
> +#define CSI2_RESETN 0x010
> +#define CSI2_PHY_STATE  0x014
> +#define CSI2_DATA_IDS_1 0x018
> +#define CSI2_DATA_IDS_2 0x01c
> +#define CSI2_ERR1   0x020
> +#define CSI2_ERR2   0x024
> +#define CSI2_MSK1   0x028
> +#define CSI2_MSK2   0x02c
> +#define CSI2_PHY_TST_CTRL0  0x030
> +#define CSI2_PHY_TST_CTRL1  0x034
> +#define CSI2_SFT_RESET  0xf00
> +
> +static inline struct imxcsi2_dev *sd_to_dev(struct v4l2_subdev *sdev)
> +{
> + return container_of(sdev, struct imxcsi2_dev, sd);
> +}
> +
> +static inline u32 imxcsi2_read(struct imxcsi2_dev *csi2, unsigned int regoff)
> +{
> + return readl(csi2->base + regoff);
> +}
> +
> +static inline void imxcsi2_write(struct imxcsi2_dev *csi2, u32 val,
> +  unsigned int regoff)
> +{
> + writel(val, csi2->base + regoff);
> +}

Do those two wrappers really make the code more readable?

> +static void imxcsi2_set_lanes(struct imxcsi2_dev *csi2)
> +{
> + int lanes = csi2->bus.num_data_lanes;
> +
> + imxcsi2_write(csi2, lanes - 1, CSI2_N_LANES);
> +}
> +
> +static void imxcsi2_enable(struct imxcsi2_dev *csi2, bool enable)
> +{
> + if (enable) {
> + imxcsi2_write(csi2, 0x, CSI2_PHY_SHUTDOWNZ);
> + imxcsi2_write(csi2, 0x, CSI2_DPHY_RSTZ);
> + imxcsi2_write(csi2, 0x, CSI2_RESETN);

Given that these registers only contain a single bit, and bits 31:1 are
documented as reserved, 0, I think these should write 1 instead of
0x.

> + } else {
> + imxcsi2_write(csi2, 0x0, CSI2_PHY_SHUTDOWNZ);
> + imxcsi2_write(csi2, 0x0, CSI2_DPHY_RSTZ);
> + imxcsi2_write(csi2, 0x0, CSI2_RESETN);
> + }
> +}
> +
> +static void imxcsi2_reset(struct imxcsi2_dev *csi2)
> +{
> + 

Re: [PATCH v3 21/24] media: imx: Add MIPI CSI-2 Receiver subdev driver

[Steve Longerbeam]

On 02/01/2017 03:44 PM, Russell King - ARM Linux wrote:

On Fri, Jan 06, 2017 at 06:11:39PM -0800, Steve Longerbeam wrote:

+static int imxcsi2_get_fmt(struct v4l2_subdev *sd,
+  struct v4l2_subdev_pad_config *cfg,
+  struct v4l2_subdev_format *sdformat)
+{
+   struct imxcsi2_dev *csi2 = sd_to_dev(sd);
+
+   sdformat->format = csi2->format_mbus;
+
+   return 0;
+}

Hi Steve,

This isn't correct, and I suspect the other get_fmt implementations are
the same - I've just checked imx-csi.c, and that also appears to have
the same issue.

When get_fmt() is called with sdformat->which == V4L2_SUBDEV_FORMAT_TRY,
you need to return the try format rather than the current format.  See
the second paragraph of Documentation/media/uapi/v4l/dev-subdev.rst's
"Format Negotiation" section, where it talks about using
V4L2_SUBDEV_FORMAT_TRY with both VIDIOC_SUBDEV_G_FMT and
VIDIOC_SUBDEV_S_FMT.


Yes that's wrong. I'll fix.

Btw I read over Documentation/media/uapi/v4l/dev-subdev.rst (can't
remember if I ever did!), and it clears up a lot. I do see I'm doing some
other things wrong as well:

-  Formats should be propagated from sink pads to source pads. Modifying
   a format on a source pad should not modify the format on any sink
   pad.

I don't believe I'm affecting the source pad formats during sink pad
negotiation, or vice-versa, yet. But I will, once the pixel width alignment
optimization is implemented based on whether the output format is planar.
And I'll keep this direction-of-propagation rule in mind when I do so.

-  Sub-devices that scale frames using variable scaling factors should
   reset the scale factors to default values when sink pads formats are
   modified. If the 1:1 scaling ratio is supported, this means that
   source pads formats should be reset to the sink pads formats.

I'm not resetting the scaling factors on sink pad format change in
the scaling subdevs (imx-ic-prpenc and imx-ic-prpvf). Will fix that.

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 21/24] media: imx: Add MIPI CSI-2 Receiver subdev driver

[Russell King - ARM Linux]

On Fri, Jan 06, 2017 at 06:11:39PM -0800, Steve Longerbeam wrote:
> +static int imxcsi2_get_fmt(struct v4l2_subdev *sd,
> +struct v4l2_subdev_pad_config *cfg,
> +struct v4l2_subdev_format *sdformat)
> +{
> + struct imxcsi2_dev *csi2 = sd_to_dev(sd);
> +
> + sdformat->format = csi2->format_mbus;
> +
> + return 0;
> +}

Hi Steve,

This isn't correct, and I suspect the other get_fmt implementations are
the same - I've just checked imx-csi.c, and that also appears to have
the same issue.

When get_fmt() is called with sdformat->which == V4L2_SUBDEV_FORMAT_TRY,
you need to return the try format rather than the current format.  See
the second paragraph of Documentation/media/uapi/v4l/dev-subdev.rst's
"Format Negotiation" section, where it talks about using
V4L2_SUBDEV_FORMAT_TRY with both VIDIOC_SUBDEV_G_FMT and
VIDIOC_SUBDEV_S_FMT.

-- 
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 21/24] media: imx: Add MIPI CSI-2 Receiver subdev driver

[Russell King - ARM Linux]

Hi Steve,

On Tue, Jan 31, 2017 at 05:02:40PM -0800, Steve Longerbeam wrote:
> But this also puts a requirement on MIPI sensors that s_power(ON)
> should only place the D_PHY in LP-11, and _not_ start the clock lane.
> But perhaps that is correct behavior anyway.

If the CSI2 DPHY is held in reset state, it shouldn't matter what the
sensor does.  In the case of IMX219, it needs a full setup of the
device, including enabling it to stream (so it starts the clock lane
etc) in order to get it into LP-11 state.  Merely disabling the XCLR
signal leaves the lanes grounded.

I do seem to remember reading in one of the MIPI specs that this is
rather expected behaviour, though I can't point at a paragraph this
late in the night.

So, the only way to satisfy these requirements is this order:

- assert PHY reset signals (so blocking any activity on the CSI lanes)
- initialise the sensor (including allowing it to start streaming and
  then stopping the stream - at that point, the lanes will be in LP-11.)
- deassert the resets as per the iMX6 documentation and follow the
  remaining procedure.

I'll look at your other points tomorrow.

Thanks.

-- 
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 21/24] media: imx: Add MIPI CSI-2 Receiver subdev driver

[Steve Longerbeam]

On 01/31/2017 01:49 AM, Philipp Zabel wrote:

On Tue, 2017-01-31 at 00:01 +, Russell King - ARM Linux wrote:
[...]

The iMX6 manuals call for a very specific seven sequence of initialisation
for CSI2, which begins with:

1. reset the D-PHY.
2. place MIPI sensor in LP-11 state
3. perform D-PHY initialisation
4. configure CSI2 lanes and de-assert resets and shutdown signals

Since you reset the CSI2 at power up and then release it, how do you
guarantee that the published sequence is followed?


Hi Russell,

In "40.3.1 Startup Sequence", it states that step 1 is to "De-assert
CSI2 presetn signal (global reset)". I can't find any description of
this signal in the manual, that statement is the only mention of it. I
don't know if this the D-PHY reset signal,  it sounds more like
CSI2_RESETN (CSI-2 host controller reset).

In any case, I re-reviewed the published sequence in the manual,
and it does look like there are a couple problems.

The pipeline power up sequence in imx-media driver is as follows:
s_power(ON) op is called first on the imx6-mipi-csi2, in which CSI2 and
D-PHY resets are asserted and then de-asserted via the CSI2_RESETN
and CSI2_DPHY_RSTZ registers, the D-PHY is initialized, and lanes set.

At this point the MIPI sensor may be powered down (in fact, in OV5640
case, the PWDN pin is asserted). So there could be a problem here,
I don't think the D-PHY is considered in the LP-11 stop state when the
D-PHY master is powered off :). A fix might simply be to reverse power
on, sensor first so that it can be placed in LP-11, then imx6-mipi-csi2.

The following steps are carried out by s_stream() calls. Sensor s_stream(ON)
is called first which starts a clock on the clock lane. Then imx6-mipi-csi2
s_stream(ON) in which the PHY_STATE register is polled to confirm the D-PHY
stop state, then looks for active clock on lock lane.

There could be a problem there too. Again should be fixed simply by
calling stream-on on the imx6-mipi-csi2 first, then sensor.

So I will try the following sequence:

1. sensor power on (put D-PHY in LP-11 stop state).
2. csi-2 power on (deassert CSI2 and D-PHY resets, D-PHY init, verify 
LP-11).

3. sensor stream on (starts clock on clock lane).
4. csi-2 stream on (confirm clock on clock lane).

That comes closest to meeting the sequence requirements.

But this also puts a requirement on MIPI sensors that s_power(ON)
should only place the D_PHY in LP-11, and _not_ start the clock lane.
But perhaps that is correct behavior anyway.

Steve



With Philipp's driver, this is easy, because there is a prepare_stream
callback which gives the sensor an opportunity to get everything
correctly configured according to the negotiated parameters, and place
the sensor in LP-11 state.

Some sensors do not power up in LP-11 state, but need to be programmed
fully before being asked to momentarily stream.  Only at that point is
the sensor guaranteed to be in the required LP-11 state.

Do you expect that 1. and 2. could depend on the negotiated parameters
in any way on some hardware? I had removed the prepare_stream callback
from my driver in v2 because for my use case at least the above sequence
could be realized by

1. in imx-mipi-csi2 s_power(1)
2. in MIPI sensor s_power(1)
3./4. in imx-mipi-csi2 s_stream(1)
4. in MIPI sensor s_stream(1)

as long as the sensor is correctly put back into LP-11 in s_stream(0).

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 21/24] media: imx: Add MIPI CSI-2 Receiver subdev driver

[Philipp Zabel]

On Tue, 2017-01-31 at 00:01 +, Russell King - ARM Linux wrote:
[...]
> The iMX6 manuals call for a very specific seven sequence of initialisation
> for CSI2, which begins with:
> 
> 1. reset the D-PHY.
> 2. place MIPI sensor in LP-11 state
> 3. perform D-PHY initialisation
> 4. configure CSI2 lanes and de-assert resets and shutdown signals
> 
> Since you reset the CSI2 at power up and then release it, how do you
> guarantee that the published sequence is followed?
> 
> With Philipp's driver, this is easy, because there is a prepare_stream
> callback which gives the sensor an opportunity to get everything
> correctly configured according to the negotiated parameters, and place
> the sensor in LP-11 state.
> 
> Some sensors do not power up in LP-11 state, but need to be programmed
> fully before being asked to momentarily stream.  Only at that point is
> the sensor guaranteed to be in the required LP-11 state.

Do you expect that 1. and 2. could depend on the negotiated parameters
in any way on some hardware? I had removed the prepare_stream callback
from my driver in v2 because for my use case at least the above sequence
could be realized by

1. in imx-mipi-csi2 s_power(1)
2. in MIPI sensor s_power(1)
3./4. in imx-mipi-csi2 s_stream(1)
4. in MIPI sensor s_stream(1)

as long as the sensor is correctly put back into LP-11 in s_stream(0).

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 21/24] media: imx: Add MIPI CSI-2 Receiver subdev driver

[Steve Longerbeam]

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

On Fri, Jan 06, 2017 at 06:11:39PM -0800, Steve Longerbeam wrote:

+++ b/drivers/staging/media/imx/imx-mipi-csi2.c

...

+#define DEVICE_NAME "imx6-mipi-csi2"

Why is the device/driver named imx6-mipi-csi2, but the module named
imx-mipi-csi2 - could there be some consistency here please?


right, that was missed after renaming the device node. 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 21/24] media: imx: Add MIPI CSI-2 Receiver subdev driver

[Russell King - ARM Linux]

On Fri, Jan 06, 2017 at 06:11:39PM -0800, Steve Longerbeam wrote:
> +++ b/drivers/staging/media/imx/imx-mipi-csi2.c
...
> +#define DEVICE_NAME "imx6-mipi-csi2"

Why is the device/driver named imx6-mipi-csi2, but the module named
imx-mipi-csi2 - could there be some consistency here please?

Thanks.

-- 
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 21/24] media: imx: Add MIPI CSI-2 Receiver subdev driver

[Russell King - ARM Linux]

On Fri, Jan 06, 2017 at 06:11:39PM -0800, Steve Longerbeam wrote:
> +static void imxcsi2_enable(struct imxcsi2_dev *csi2, bool enable)
> +{
> + if (enable) {
> + imxcsi2_write(csi2, 0x, CSI2_PHY_SHUTDOWNZ);
> + imxcsi2_write(csi2, 0x, CSI2_DPHY_RSTZ);
> + imxcsi2_write(csi2, 0x, CSI2_RESETN);
> + } else {
> + imxcsi2_write(csi2, 0x0, CSI2_PHY_SHUTDOWNZ);
> + imxcsi2_write(csi2, 0x0, CSI2_DPHY_RSTZ);
> + imxcsi2_write(csi2, 0x0, CSI2_RESETN);
> + }
> +}
> +
> +static void imxcsi2_reset(struct imxcsi2_dev *csi2)
> +{
> + imxcsi2_enable(csi2, false);
> +
> + imxcsi2_write(csi2, 0x0001, CSI2_PHY_TST_CTRL0);
> + imxcsi2_write(csi2, 0x, CSI2_PHY_TST_CTRL1);
> + imxcsi2_write(csi2, 0x, CSI2_PHY_TST_CTRL0);
> + imxcsi2_write(csi2, 0x0002, CSI2_PHY_TST_CTRL0);
> + imxcsi2_write(csi2, 0x00010044, CSI2_PHY_TST_CTRL1);
> + imxcsi2_write(csi2, 0x, CSI2_PHY_TST_CTRL0);
> + imxcsi2_write(csi2, 0x0014, CSI2_PHY_TST_CTRL1);
> + imxcsi2_write(csi2, 0x0002, CSI2_PHY_TST_CTRL0);
> + imxcsi2_write(csi2, 0x, CSI2_PHY_TST_CTRL0);
> +
> + imxcsi2_enable(csi2, true);
> +}
> +
> +static int imxcsi2_dphy_wait(struct imxcsi2_dev *csi2)
> +{
> + u32 reg;
> + int i;
> +
> + /* wait for mipi sensor ready */
> + for (i = 0; i < 50; i++) {
> + reg = imxcsi2_read(csi2, CSI2_PHY_STATE);
> + if (reg != 0x200)
> + break;
> + usleep_range(1, 2);
> + }
> +
> + if (i >= 50) {
> + v4l2_err(>sd,
> +  "wait for clock lane timeout, phy_state = 0x%08x\n",
> +  reg);
> + return -ETIME;
> + }
> +
> + /* wait for mipi stable */
> + for (i = 0; i < 50; i++) {
> + reg = imxcsi2_read(csi2, CSI2_ERR1);
> + if (reg == 0x0)
> + break;
> + usleep_range(1, 2);
> + }
> +
> + if (i >= 50) {
> + v4l2_err(>sd,
> +  "wait for controller timeout, err1 = 0x%08x\n",
> +  reg);
> + return -ETIME;
> + }
> +
> + /* finally let's wait for active clock on the clock lane */
> + for (i = 0; i < 50; i++) {
> + reg = imxcsi2_read(csi2, CSI2_PHY_STATE);
> + if (reg & (1 << 8))
> + break;
> + usleep_range(1, 2);
> + }
> +
> + if (i >= 50) {
> + v4l2_err(>sd,
> +  "wait for active clock timeout, phy_state = 0x%08x\n",
> +  reg);
> + return -ETIME;
> + }
> +
> + v4l2_info(>sd, "ready, dphy version 0x%x\n",
> +   imxcsi2_read(csi2, CSI2_VERSION));
> +
> + return 0;
> +}
...
> +static int imxcsi2_s_power(struct v4l2_subdev *sd, int on)
> +{
> + struct imxcsi2_dev *csi2 = sd_to_dev(sd);
> +
> + if (on && !csi2->on) {
> + v4l2_info(>sd, "power ON\n");
> + clk_prepare_enable(csi2->cfg_clk);
> + clk_prepare_enable(csi2->dphy_clk);
> + imxcsi2_set_lanes(csi2);
> + imxcsi2_reset(csi2);

The iMX6 manuals call for a very specific seven sequence of initialisation
for CSI2, which begins with:

1. reset the D-PHY.
2. place MIPI sensor in LP-11 state
3. perform D-PHY initialisation
4. configure CSI2 lanes and de-assert resets and shutdown signals

Since you reset the CSI2 at power up and then release it, how do you
guarantee that the published sequence is followed?

With Philipp's driver, this is easy, because there is a prepare_stream
callback which gives the sensor an opportunity to get everything
correctly configured according to the negotiated parameters, and place
the sensor in LP-11 state.

Some sensors do not power up in LP-11 state, but need to be programmed
fully before being asked to momentarily stream.  Only at that point is
the sensor guaranteed to be in the required LP-11 state.

-- 
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 21/24] media: imx: Add MIPI CSI-2 Receiver subdev driver

[Russell King - ARM Linux]

On Fri, Jan 06, 2017 at 06:11:39PM -0800, Steve Longerbeam wrote:
> Adds MIPI CSI-2 Receiver subdev driver. This subdev is required
> for sensors with a MIPI CSI2 interface.
> 
> Signed-off-by: Steve Longerbeam 

Applying: media: imx: Add MIPI CSI-2 Receiver subdev driver
.git/rebase-apply/patch:522: new blank line at EOF.
+
warning: 1 line adds whitespace errors.

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

[PATCH v3 21/24] media: imx: Add MIPI CSI-2 Receiver subdev driver

[Steve Longerbeam]

Adds MIPI CSI-2 Receiver subdev driver. This subdev is required
for sensors with a MIPI CSI2 interface.

Signed-off-by: Steve Longerbeam 
---
 drivers/staging/media/imx/Makefile|   1 +
 drivers/staging/media/imx/imx-mipi-csi2.c | 501 ++
 2 files changed, 502 insertions(+)
 create mode 100644 drivers/staging/media/imx/imx-mipi-csi2.c

diff --git a/drivers/staging/media/imx/Makefile 
b/drivers/staging/media/imx/Makefile
index fe9e992..0decef7 100644
--- a/drivers/staging/media/imx/Makefile
+++ b/drivers/staging/media/imx/Makefile
@@ -9,3 +9,4 @@ obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-ic.o
 obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-csi.o
 obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-smfc.o
 obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-camif.o
+obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-mipi-csi2.o
diff --git a/drivers/staging/media/imx/imx-mipi-csi2.c 
b/drivers/staging/media/imx/imx-mipi-csi2.c
new file mode 100644
index 000..daa6e1d
--- /dev/null
+++ b/drivers/staging/media/imx/imx-mipi-csi2.c
@@ -0,0 +1,501 @@
+/*
+ * MIPI CSI-2 Receiver Subdev for Freescale i.MX5/6 SOC.
+ *
+ * Copyright (c) 2012-2014 Mentor Graphics Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "imx-media.h"
+
+/*
+ * there must be 5 pads: 1 input pad from sensor, and
+ * the 4 virtual channel output pads
+ */
+#define CSI2_NUM_SINK_PADS  1
+#define CSI2_NUM_SRC_PADS   4
+#define CSI2_NUM_PADS   5
+
+struct imxcsi2_dev {
+   struct device  *dev;
+   struct imx_media_dev   *md;
+   struct v4l2_subdev  sd;
+   struct media_pad   pad[CSI2_NUM_PADS];
+   struct v4l2_mbus_framefmt format_mbus;
+   struct v4l2_subdev *src_sd;
+   struct v4l2_subdev *sink_sd[CSI2_NUM_SRC_PADS];
+   intinput_pad;
+   struct clk *dphy_clk;
+   struct clk *cfg_clk;
+   struct clk *pix_clk; /* what is this? */
+   void __iomem   *base;
+   int intr1;
+   int intr2;
+   struct v4l2_of_bus_mipi_csi2 bus;
+   boolon;
+   boolstream_on;
+};
+
+#define DEVICE_NAME "imx6-mipi-csi2"
+
+/* Register offsets */
+#define CSI2_VERSION0x000
+#define CSI2_N_LANES0x004
+#define CSI2_PHY_SHUTDOWNZ  0x008
+#define CSI2_DPHY_RSTZ  0x00c
+#define CSI2_RESETN 0x010
+#define CSI2_PHY_STATE  0x014
+#define CSI2_DATA_IDS_1 0x018
+#define CSI2_DATA_IDS_2 0x01c
+#define CSI2_ERR1   0x020
+#define CSI2_ERR2   0x024
+#define CSI2_MSK1   0x028
+#define CSI2_MSK2   0x02c
+#define CSI2_PHY_TST_CTRL0  0x030
+#define CSI2_PHY_TST_CTRL1  0x034
+#define CSI2_SFT_RESET  0xf00
+
+static inline struct imxcsi2_dev *sd_to_dev(struct v4l2_subdev *sdev)
+{
+   return container_of(sdev, struct imxcsi2_dev, sd);
+}
+
+static inline u32 imxcsi2_read(struct imxcsi2_dev *csi2, unsigned int regoff)
+{
+   return readl(csi2->base + regoff);
+}
+
+static inline void imxcsi2_write(struct imxcsi2_dev *csi2, u32 val,
+unsigned int regoff)
+{
+   writel(val, csi2->base + regoff);
+}
+
+static void imxcsi2_set_lanes(struct imxcsi2_dev *csi2)
+{
+   int lanes = csi2->bus.num_data_lanes;
+
+   imxcsi2_write(csi2, lanes - 1, CSI2_N_LANES);
+}
+
+static void imxcsi2_enable(struct imxcsi2_dev *csi2, bool enable)
+{
+   if (enable) {
+   imxcsi2_write(csi2, 0x, CSI2_PHY_SHUTDOWNZ);
+   imxcsi2_write(csi2, 0x, CSI2_DPHY_RSTZ);
+   imxcsi2_write(csi2, 0x, CSI2_RESETN);
+   } else {
+   imxcsi2_write(csi2, 0x0, CSI2_PHY_SHUTDOWNZ);
+   imxcsi2_write(csi2, 0x0, CSI2_DPHY_RSTZ);
+   imxcsi2_write(csi2, 0x0, CSI2_RESETN);
+   }
+}
+
+static void imxcsi2_reset(struct imxcsi2_dev *csi2)
+{
+   imxcsi2_enable(csi2, false);
+
+   imxcsi2_write(csi2, 0x0001, CSI2_PHY_TST_CTRL0);
+   imxcsi2_write(csi2, 0x, CSI2_PHY_TST_CTRL1);
+   imxcsi2_write(csi2, 0x, CSI2_PHY_TST_CTRL0);
+   imxcsi2_write(csi2, 0x0002, CSI2_PHY_TST_CTRL0);
+   imxcsi2_write(csi2, 0x00010044, CSI2_PHY_TST_CTRL1);
+   imxcsi2_write(csi2, 0x, CSI2_PHY_TST_CTRL0);
+   imxcsi2_write(csi2, 0x0014, CSI2_PHY_TST_CTRL1);
+   imxcsi2_write(csi2, 0x0002, CSI2_PHY_TST_CTRL0);
+   imxcsi2_write(csi2, 0x, CSI2_PHY_TST_CTRL0);
+
+   imxcsi2_enable(csi2, true);
+}
+