Re: [PATCH v4 00/36] i.MX Media Driver

2017-02-18 Thread Steve Longerbeam



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

On Thu, Feb 16, 2017 at 02:27:41PM -0800, Steve Longerbeam wrote:



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

On Wed, Feb 15, 2017 at 06:19:02PM -0800, Steve Longerbeam wrote:

In version 4:


With this version, I get:

[28762.892053] imx6-mipi-csi2: LP-11 timeout, phy_state = 0x
[28762.899409] ipu1_csi0: pipeline_set_stream failed with -110



Right, in the imx219, on exit from s_power(), the clock and data lanes
must be placed in the LP-11 state. This has been done in the ov5640 and
tc358743 subdevs.


The only way to do that is to enable streaming from the sensor, wait
an initialisation time, and then disable streaming, and wait for the
current line to finish.  There is _no_ other way to get the sensor to
place its clock and data lines into LP-11 state.

For that to happen, we need to program the sensor a bit more than we
currently do at power on (to a minimal resolution, and setting up the
PLLs), and introduce another 4ms on top of the 8ms or so that the
runtime resume function already takes.


This is basically the same procedure that was necessary to get the
OV5640 to enter LP-11 on all its lanes. Power-on procedure writes
an initial register set that gets the sensor to a default resolution,
turn on streaming briefly (I wait 1msec which is probably too long,
but it's not clear to me how to determine that wait time), and then
disable streaming. All lanes are then in LP-11 state.


Steve


Re: [PATCH v4 00/36] i.MX Media Driver

2017-02-18 Thread Sakari Ailus
Hi Philipp and Russell,

On Fri, Feb 17, 2017 at 04:04:30PM +0100, Philipp Zabel wrote:
> On Fri, 2017-02-17 at 14:22 +0200, Sakari Ailus wrote:
> > Hi Philipp, Steve and Russell,
> > 
> > On Fri, Feb 17, 2017 at 12:43:38PM +0100, Philipp Zabel wrote:
> > > On Thu, 2017-02-16 at 14:27 -0800, Steve Longerbeam wrote:
> > > > 
> > > > On 02/16/2017 02:20 PM, Russell King - ARM Linux wrote:
> > > > > On Wed, Feb 15, 2017 at 06:19:02PM -0800, Steve Longerbeam wrote:
> > > > >> In version 4:
> > > > >
> > > > > With this version, I get:
> > > > >
> > > > > [28762.892053] imx6-mipi-csi2: LP-11 timeout, phy_state = 0x
> > > > > [28762.899409] ipu1_csi0: pipeline_set_stream failed with -110
> > > > >
> > > > 
> > > > Right, in the imx219, on exit from s_power(), the clock and data lanes
> > > > must be placed in the LP-11 state. This has been done in the ov5640 and
> > > > tc358743 subdevs.
> > > > 
> > > > If we want to bring in the patch that adds a .prepare_stream() op,
> > > > the csi-2 bus would need to be placed in LP-11 in that op instead.
> > > > 
> > > > Philipp, should I go ahead and add your .prepare_stream() patch?
> > > 
> > > I think with Russell's explanation of how the imx219 sensor operates,
> > > we'll have to do something before calling the sensor s_stream, but right
> > > now I'm still unsure what exactly.
> > 
> > Indeed there appears to be no other way to achieve the LP-11 state than
> > going through the streaming state for this particular sensor, apart from
> > starting streaming.
> > 
> > Is there a particular reason why you're waiting for the transmitter to
> > transfer to LP-11 state? That appears to be the last step which is done in
> > the csi2_s_stream() callback.
> > 
> > What the sensor does next is to start streaming, and the first thing it does
> > in that process is to switch to LP-11 state.
> > 
> > Have you tried what happens if you simply drop the LP-11 check? To me that
> > would seem the right thing to do.
> 
> Removing the wait for LP-11 alone might not be an issue in my case, as
> the TC358743 is known to be in stop state all along. So I just have to
> make sure that the time between s_stream(csi2) starting the receiver and
> s_stream(tc358743) causing LP-11 to be changed to the next state is long
> enough for the receiver to detect LP-11 (which I really can't, I just
> have to pray I2C transmissions are slow enough).

Fair enough; it appears that the timing of the bus setup is indeed ill
defined between the transmitter and the receiver. So there can be hardware
specific matters in stream starting that have to be taken into account. :-(

This is quite annoying, as there does not appear to be a good way to tell
the sensor to set the receiver to LP-11 state without going through the
streaming state. If there was, just doing that in s_power(, 1) callback
would be quite practical.

I guess then there's no really a way to avoid having an extra callback that
would explicitly tell the sensor to go to LP-11 state. It should be no issue
if the transmitter is already in that state from power-on, but the new
callback should guarantee that.

Another question is that how far do you need to proceed with streaming in a
case where you want to go to LP-11 through streaming? Is simply starting
streaming and stopping it right after enough? On some devices it might be
but not on others. As the receiver is not started yet, you can't wait for
the first frame to start either. And how long it would take for the first
frame to start is not defined either in general case: or a driver such as
SMIA that's not exactly aware of the underlying hardware but is relying on a
standard device interface and behaviour, such approach could be best effort
only. Of course it's possible to make changes to the driver if you encounter
a combination of a sensor and a receiver that doesn't seem to work, but
still it's hardly an ideal solution.

How about calling the new callback phy_prepare(), for instance? We could
document that it must explicitly set up the transmitter PHY in LP-11 state
for CSI-2. The current documentation states that the device should be
already in LP-11 after power-on but that apparently is not the case in
general.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk


Re: [PATCH v4 00/36] i.MX Media Driver

2017-02-17 Thread Philipp Zabel
On Fri, 2017-02-17 at 14:22 +0200, Sakari Ailus wrote:
> Hi Philipp, Steve and Russell,
> 
> On Fri, Feb 17, 2017 at 12:43:38PM +0100, Philipp Zabel wrote:
> > On Thu, 2017-02-16 at 14:27 -0800, Steve Longerbeam wrote:
> > > 
> > > On 02/16/2017 02:20 PM, Russell King - ARM Linux wrote:
> > > > On Wed, Feb 15, 2017 at 06:19:02PM -0800, Steve Longerbeam wrote:
> > > >> In version 4:
> > > >
> > > > With this version, I get:
> > > >
> > > > [28762.892053] imx6-mipi-csi2: LP-11 timeout, phy_state = 0x
> > > > [28762.899409] ipu1_csi0: pipeline_set_stream failed with -110
> > > >
> > > 
> > > Right, in the imx219, on exit from s_power(), the clock and data lanes
> > > must be placed in the LP-11 state. This has been done in the ov5640 and
> > > tc358743 subdevs.
> > > 
> > > If we want to bring in the patch that adds a .prepare_stream() op,
> > > the csi-2 bus would need to be placed in LP-11 in that op instead.
> > > 
> > > Philipp, should I go ahead and add your .prepare_stream() patch?
> > 
> > I think with Russell's explanation of how the imx219 sensor operates,
> > we'll have to do something before calling the sensor s_stream, but right
> > now I'm still unsure what exactly.
> 
> Indeed there appears to be no other way to achieve the LP-11 state than
> going through the streaming state for this particular sensor, apart from
> starting streaming.
> 
> Is there a particular reason why you're waiting for the transmitter to
> transfer to LP-11 state? That appears to be the last step which is done in
> the csi2_s_stream() callback.
> 
> What the sensor does next is to start streaming, and the first thing it does
> in that process is to switch to LP-11 state.
> 
> Have you tried what happens if you simply drop the LP-11 check? To me that
> would seem the right thing to do.

Removing the wait for LP-11 alone might not be an issue in my case, as
the TC358743 is known to be in stop state all along. So I just have to
make sure that the time between s_stream(csi2) starting the receiver and
s_stream(tc358743) causing LP-11 to be changed to the next state is long
enough for the receiver to detect LP-11 (which I really can't, I just
have to pray I2C transmissions are slow enough).

The problems start if we have to enable the D-PHY and deassert resets
either before the sensor enters LP-11 state or after it already started
streaming, because we don't know when the sensor drives that state on
the bus.

The latter case I is simulated easily by again changing the order so
that the "sensor" (tc358743) is enabled before the CSI-2 receiver D-PHY
initialization. The result is that captures time out, presumably because
the receiver never entered HS mode because it didn't see LP-11. The
PHY_STATE register contains 0x200, meaning RXCLKACTIVEHS (which we
should wait for in step 7.) is never set.

I tried to test the former by instead modifying the tc358743 driver a
bit:

--8<--
diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index 39d4cdd328c0f..43df80903215b 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -1378,8 +1378,6 @@ static int tc358743_s_dv_timings(struct v4l2_subdev *sd,
state->timings = *timings;
 
enable_stream(sd, false);
-   tc358743_set_pll(sd);
-   tc358743_set_csi(sd);
 
return 0;
 }
@@ -1469,6 +1467,11 @@ static int tc358743_g_mbus_config(struct v4l2_subdev *sd,
 
 static int tc358743_s_stream(struct v4l2_subdev *sd, int enable)
 {
+   if (enable) {
+   tc358743_set_pll(sd);
+   tc358743_set_csi(sd);
+   tc358743_set_csi_color_space(sd);
+   }
enable_stream(sd, enable);
if (!enable) {
/* Put all lanes in PL-11 state (STOPSTATE) */
@@ -1657,9 +1660,6 @@ static int tc358743_set_fmt(struct v4l2_subdev *sd,
state->vout_color_sel = vout_color_sel;
 
enable_stream(sd, false);
-   tc358743_set_pll(sd);
-   tc358743_set_csi(sd);
-   tc358743_set_csi_color_space(sd);
 
return 0;
 }
-->8--

That should enable the CSI-2 Tx and put it in LP-11 only after the CSI-2
receiver is enabled, right before starting streaming.

That did seem to work the few times I tested, but I have no idea how
this will behave with other chips that do something else to the bus
while not streaming, and whether it is ok to enable the CSI right after
the sensor without waiting for the CSI-2 bus to settle.

regards
Philipp



Re: [PATCH v4 00/36] i.MX Media Driver

2017-02-17 Thread Russell King - ARM Linux
On Fri, Feb 17, 2017 at 02:22:14PM +0200, Sakari Ailus wrote:
> Hi Philipp, Steve and Russell,
> 
> On Fri, Feb 17, 2017 at 12:43:38PM +0100, Philipp Zabel wrote:
> > I think with Russell's explanation of how the imx219 sensor operates,
> > we'll have to do something before calling the sensor s_stream, but right
> > now I'm still unsure what exactly.
> 
> Indeed there appears to be no other way to achieve the LP-11 state than
> going through the streaming state for this particular sensor, apart from
> starting streaming.
> 
> Is there a particular reason why you're waiting for the transmitter to
> transfer to LP-11 state? That appears to be the last step which is done in
> the csi2_s_stream() callback.
> 
> What the sensor does next is to start streaming, and the first thing it does
> in that process is to switch to LP-11 state.
> 
> Have you tried what happens if you simply drop the LP-11 check? To me that
> would seem the right thing to do.

The Freescale documentation for iMX6's CSI2 receiver (chapter 40.3.1)
specifies a very specific sequence to be followed to safely bring up the
CSI2 receiver.  Bold text gets used, which implies emphasis on certain
points, which suggests that it's important to follow it.

Presumably, the reason for this is to ensure that a state machine within
the CSI2 receiver is properly synchronised to the incoming data stream,
and while avoiding the sequence may work, it may not be guaranteed to
work every time.

I guess we need someone from NXP to comment.

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


Re: [PATCH v4 00/36] i.MX Media Driver

2017-02-17 Thread Sakari Ailus
Hi Philipp, Steve and Russell,

On Fri, Feb 17, 2017 at 12:43:38PM +0100, Philipp Zabel wrote:
> On Thu, 2017-02-16 at 14:27 -0800, Steve Longerbeam wrote:
> > 
> > On 02/16/2017 02:20 PM, Russell King - ARM Linux wrote:
> > > On Wed, Feb 15, 2017 at 06:19:02PM -0800, Steve Longerbeam wrote:
> > >> In version 4:
> > >
> > > With this version, I get:
> > >
> > > [28762.892053] imx6-mipi-csi2: LP-11 timeout, phy_state = 0x
> > > [28762.899409] ipu1_csi0: pipeline_set_stream failed with -110
> > >
> > 
> > Right, in the imx219, on exit from s_power(), the clock and data lanes
> > must be placed in the LP-11 state. This has been done in the ov5640 and
> > tc358743 subdevs.
> > 
> > If we want to bring in the patch that adds a .prepare_stream() op,
> > the csi-2 bus would need to be placed in LP-11 in that op instead.
> > 
> > Philipp, should I go ahead and add your .prepare_stream() patch?
> 
> I think with Russell's explanation of how the imx219 sensor operates,
> we'll have to do something before calling the sensor s_stream, but right
> now I'm still unsure what exactly.

Indeed there appears to be no other way to achieve the LP-11 state than
going through the streaming state for this particular sensor, apart from
starting streaming.

Is there a particular reason why you're waiting for the transmitter to
transfer to LP-11 state? That appears to be the last step which is done in
the csi2_s_stream() callback.

What the sensor does next is to start streaming, and the first thing it does
in that process is to switch to LP-11 state.

Have you tried what happens if you simply drop the LP-11 check? To me that
would seem the right thing to do.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk


Re: [PATCH v4 00/36] i.MX Media Driver

2017-02-17 Thread Philipp Zabel
On Thu, 2017-02-16 at 14:27 -0800, Steve Longerbeam wrote:
> 
> On 02/16/2017 02:20 PM, Russell King - ARM Linux wrote:
> > On Wed, Feb 15, 2017 at 06:19:02PM -0800, Steve Longerbeam wrote:
> >> In version 4:
> >
> > With this version, I get:
> >
> > [28762.892053] imx6-mipi-csi2: LP-11 timeout, phy_state = 0x
> > [28762.899409] ipu1_csi0: pipeline_set_stream failed with -110
> >
> 
> Right, in the imx219, on exit from s_power(), the clock and data lanes
> must be placed in the LP-11 state. This has been done in the ov5640 and
> tc358743 subdevs.
> 
> If we want to bring in the patch that adds a .prepare_stream() op,
> the csi-2 bus would need to be placed in LP-11 in that op instead.
> 
> Philipp, should I go ahead and add your .prepare_stream() patch?

I think with Russell's explanation of how the imx219 sensor operates,
we'll have to do something before calling the sensor s_stream, but right
now I'm still unsure what exactly.

regards
Philipp



Re: [PATCH v4 00/36] i.MX Media Driver

2017-02-17 Thread Philipp Zabel
On Fri, 2017-02-17 at 10:56 +, Russell King - ARM Linux wrote:
> On Fri, Feb 17, 2017 at 11:39:11AM +0100, Philipp Zabel wrote:
> > On Thu, 2017-02-16 at 22:57 +, Russell King - ARM Linux wrote:
> > > On Thu, Feb 16, 2017 at 02:27:41PM -0800, Steve Longerbeam wrote:
> > > > 
> > > > 
> > > > On 02/16/2017 02:20 PM, Russell King - ARM Linux wrote:
> > > > >On Wed, Feb 15, 2017 at 06:19:02PM -0800, Steve Longerbeam wrote:
> > > > >>In version 4:
> > > > >
> > > > >With this version, I get:
> > > > >
> > > > >[28762.892053] imx6-mipi-csi2: LP-11 timeout, phy_state = 0x
> > > > >[28762.899409] ipu1_csi0: pipeline_set_stream failed with -110
> > > > >
> > > > 
> > > > Right, in the imx219, on exit from s_power(), the clock and data lanes
> > > > must be placed in the LP-11 state. This has been done in the ov5640 and
> > > > tc358743 subdevs.
> > > 
> > > The only way to do that is to enable streaming from the sensor, wait
> > > an initialisation time, and then disable streaming, and wait for the
> > > current line to finish.  There is _no_ other way to get the sensor to
> > > place its clock and data lines into LP-11 state.
> > 
> > I thought going through LP-11 is part of the D-PHY transmitter
> > initialization, during the LP->HS wakeup sequence. But then I have no
> > access to MIPI specs.
> 
> The D-PHY transmitter initialisation *only* happens as part of the
> wake-up from standby to streaming mode.  That is because Sony expect
> that you program the sensor, and then when you switch it to streaming
> mode, it computes the D-PHY parameters from the PLL, input clock rate
> (you have to tell it the clock rate in 1/256 MHz units), number of
> lanes, and other parameters.
> 
> It is possible to program the D-PHY parameters manually, but that
> doesn't change the above sequence in any way (it just avoids the
> chip computing the values, it doesn't result in any change of
> behaviour on the bus.)
>
> The IMX219 specifications are clear: the clock and data lines are
> held low (LP-00 state) after releasing the hardware enable signal.
> There's a period of chip initialisation, and then you can access the
> I2C bus and configure it.  There's a further period of initialisation
> where charge pumps are getting to their operating state.  Then, you
> set the streaming bit, and a load more initialisation happens before
> the CSI bus enters LP-11 state and the first frame pops out.  When
> entering standby, the last frame is completed, and then the CSI bus
> enters LP-11 state.

How about firing off a thread in imx6-mipi-csi2 prepare_stream that
spins on the LP-11 check and then continues with the receiver D-PHY
initialization once the condition is met? I think we should have at
least 100 us to do this, but maybe the IMX219 can be programmed to stay
in LP-11 for a longer time.

> SMIA are slightly different - mostly following what I've said above,
> but the clock and data lines are tristated after releasing the
> xshutdown signal, and they remain tristated until the clock line
> starts toggling before the first frame appears.  There appears to
> be no point that the clock line enters LP-11 state before it starts
> toggling.  When entering standby, the last frame is completed, and
> the CSI bus enters tristate mode (so floating.)  There is no way to
> get these sensors into LP-11 state.

I have no idea what to do about those.

regards
Philipp



Re: [PATCH v4 00/36] i.MX Media Driver

2017-02-17 Thread Russell King - ARM Linux
On Fri, Feb 17, 2017 at 11:39:11AM +0100, Philipp Zabel wrote:
> On Thu, 2017-02-16 at 22:57 +, Russell King - ARM Linux wrote:
> > On Thu, Feb 16, 2017 at 02:27:41PM -0800, Steve Longerbeam wrote:
> > > 
> > > 
> > > On 02/16/2017 02:20 PM, Russell King - ARM Linux wrote:
> > > >On Wed, Feb 15, 2017 at 06:19:02PM -0800, Steve Longerbeam wrote:
> > > >>In version 4:
> > > >
> > > >With this version, I get:
> > > >
> > > >[28762.892053] imx6-mipi-csi2: LP-11 timeout, phy_state = 0x
> > > >[28762.899409] ipu1_csi0: pipeline_set_stream failed with -110
> > > >
> > > 
> > > Right, in the imx219, on exit from s_power(), the clock and data lanes
> > > must be placed in the LP-11 state. This has been done in the ov5640 and
> > > tc358743 subdevs.
> > 
> > The only way to do that is to enable streaming from the sensor, wait
> > an initialisation time, and then disable streaming, and wait for the
> > current line to finish.  There is _no_ other way to get the sensor to
> > place its clock and data lines into LP-11 state.
> 
> I thought going through LP-11 is part of the D-PHY transmitter
> initialization, during the LP->HS wakeup sequence. But then I have no
> access to MIPI specs.

The D-PHY transmitter initialisation *only* happens as part of the
wake-up from standby to streaming mode.  That is because Sony expect
that you program the sensor, and then when you switch it to streaming
mode, it computes the D-PHY parameters from the PLL, input clock rate
(you have to tell it the clock rate in 1/256 MHz units), number of
lanes, and other parameters.

It is possible to program the D-PHY parameters manually, but that
doesn't change the above sequence in any way (it just avoids the
chip computing the values, it doesn't result in any change of
behaviour on the bus.)

The IMX219 specifications are clear: the clock and data lines are
held low (LP-00 state) after releasing the hardware enable signal.
There's a period of chip initialisation, and then you can access the
I2C bus and configure it.  There's a further period of initialisation
where charge pumps are getting to their operating state.  Then, you
set the streaming bit, and a load more initialisation happens before
the CSI bus enters LP-11 state and the first frame pops out.  When
entering standby, the last frame is completed, and then the CSI bus
enters LP-11 state.

SMIA are slightly different - mostly following what I've said above,
but the clock and data lines are tristated after releasing the
xshutdown signal, and they remain tristated until the clock line
starts toggling before the first frame appears.  There appears to
be no point that the clock line enters LP-11 state before it starts
toggling.  When entering standby, the last frame is completed, and
the CSI bus enters tristate mode (so floating.)  There is no way to
get these sensors into 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.


Re: [PATCH v4 00/36] i.MX Media Driver

2017-02-17 Thread Philipp Zabel
On Thu, 2017-02-16 at 22:57 +, Russell King - ARM Linux wrote:
> On Thu, Feb 16, 2017 at 02:27:41PM -0800, Steve Longerbeam wrote:
> > 
> > 
> > On 02/16/2017 02:20 PM, Russell King - ARM Linux wrote:
> > >On Wed, Feb 15, 2017 at 06:19:02PM -0800, Steve Longerbeam wrote:
> > >>In version 4:
> > >
> > >With this version, I get:
> > >
> > >[28762.892053] imx6-mipi-csi2: LP-11 timeout, phy_state = 0x
> > >[28762.899409] ipu1_csi0: pipeline_set_stream failed with -110
> > >
> > 
> > Right, in the imx219, on exit from s_power(), the clock and data lanes
> > must be placed in the LP-11 state. This has been done in the ov5640 and
> > tc358743 subdevs.
> 
> The only way to do that is to enable streaming from the sensor, wait
> an initialisation time, and then disable streaming, and wait for the
> current line to finish.  There is _no_ other way to get the sensor to
> place its clock and data lines into LP-11 state.

I thought going through LP-11 is part of the D-PHY transmitter
initialization, during the LP->HS wakeup sequence. But then I have no
access to MIPI specs.
It is unfortunate that the i.MX6 MIPI CSI-2 core needs software
assistance here, but could it be possible to trigger that sequence in
the sensor and then without waiting switching to polling for LP-11 state
in the i.MX6 MIPI CSI-2 receiver?

> For that to happen, we need to program the sensor a bit more than we
> currently do at power on (to a minimal resolution, and setting up the
> PLLs), and introduce another 4ms on top of the 8ms or so that the
> runtime resume function already takes.
> 
> Looking at the SMIA driver, things are worse, and I suspect that it also
> will not work with the current setup - the SMIA spec shows that the CSI
> clock and data lines are tristated while the sensor is not streaming,
> which means they aren't held at a guaranteed LP-11 state, even if that
> driver momentarily enabled streaming.  Hence, Freescale's (or is it
> Synopsis') requirement may actually be difficult to satisfy.
> 
> However, I regard runtime PM broken with the current imx-capture setup.
> At the moment, power is controlled at the sensor by whether the media
> links are enabled.  So, if you have an enabled link coming off the
> sensor, the sensor will be powered up, whether you're using it or not.
> 
> Given that the number of applications out there that know about the
> media subdevs is really quite small, this combination makes having
> runtime PM in sensor devices completely pointless - they can't sleep
> as long as they have an enabled link, which could be persistent over
> many days or weeks.

regards
Philipp



Re: [PATCH v4 00/36] i.MX Media Driver

2017-02-16 Thread Russell King - ARM Linux
On Thu, Feb 16, 2017 at 02:27:41PM -0800, Steve Longerbeam wrote:
> 
> 
> On 02/16/2017 02:20 PM, Russell King - ARM Linux wrote:
> >On Wed, Feb 15, 2017 at 06:19:02PM -0800, Steve Longerbeam wrote:
> >>In version 4:
> >
> >With this version, I get:
> >
> >[28762.892053] imx6-mipi-csi2: LP-11 timeout, phy_state = 0x
> >[28762.899409] ipu1_csi0: pipeline_set_stream failed with -110
> >
> 
> Right, in the imx219, on exit from s_power(), the clock and data lanes
> must be placed in the LP-11 state. This has been done in the ov5640 and
> tc358743 subdevs.

The only way to do that is to enable streaming from the sensor, wait
an initialisation time, and then disable streaming, and wait for the
current line to finish.  There is _no_ other way to get the sensor to
place its clock and data lines into LP-11 state.

For that to happen, we need to program the sensor a bit more than we
currently do at power on (to a minimal resolution, and setting up the
PLLs), and introduce another 4ms on top of the 8ms or so that the
runtime resume function already takes.

Looking at the SMIA driver, things are worse, and I suspect that it also
will not work with the current setup - the SMIA spec shows that the CSI
clock and data lines are tristated while the sensor is not streaming,
which means they aren't held at a guaranteed LP-11 state, even if that
driver momentarily enabled streaming.  Hence, Freescale's (or is it
Synopsis') requirement may actually be difficult to satisfy.

However, I regard runtime PM broken with the current imx-capture setup.
At the moment, power is controlled at the sensor by whether the media
links are enabled.  So, if you have an enabled link coming off the
sensor, the sensor will be powered up, whether you're using it or not.

Given that the number of applications out there that know about the
media subdevs is really quite small, this combination makes having
runtime PM in sensor devices completely pointless - they can't sleep
as long as they have an enabled link, which could be persistent over
many days or weeks.

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


Re: [PATCH v4 00/36] i.MX Media Driver

2017-02-16 Thread Steve Longerbeam



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

On Wed, Feb 15, 2017 at 06:19:02PM -0800, Steve Longerbeam wrote:

In version 4:


With this version, I get:

[28762.892053] imx6-mipi-csi2: LP-11 timeout, phy_state = 0x
[28762.899409] ipu1_csi0: pipeline_set_stream failed with -110



Right, in the imx219, on exit from s_power(), the clock and data lanes
must be placed in the LP-11 state. This has been done in the ov5640 and
tc358743 subdevs.

If we want to bring in the patch that adds a .prepare_stream() op,
the csi-2 bus would need to be placed in LP-11 in that op instead.

Philipp, should I go ahead and add your .prepare_stream() patch?

Steve


Re: [PATCH v4 00/36] i.MX Media Driver

2017-02-16 Thread Russell King - ARM Linux
On Wed, Feb 15, 2017 at 06:19:02PM -0800, Steve Longerbeam wrote:
> In version 4:

With this version, I get:

[28762.892053] imx6-mipi-csi2: LP-11 timeout, phy_state = 0x
[28762.899409] ipu1_csi0: pipeline_set_stream failed with -110

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


Re: [PATCH v4 00/36] i.MX Media Driver

2017-02-16 Thread Steve Longerbeam



On 02/16/2017 03:37 AM, Russell King - ARM Linux wrote:

Two problems.

On Wed, Feb 15, 2017 at 06:19:02PM -0800, Steve Longerbeam wrote:

  media: imx: propagate sink pad formats to source pads


1) It looks like all cases aren't being caught:

- entity 74: ipu1_csi0 (3 pads, 4 links)
 type V4L2 subdev subtype Unknown flags 0
 device node name /dev/v4l-subdev13
pad0: Sink
[fmt:SRGGB8/816x616 field:none]
<- "ipu1_csi0_mux":2 [ENABLED]
pad1: Source
[fmt:AYUV32/816x616 field:none
 crop.bounds:(0,0)/816x616
 crop:(0,0)/816x616]
-> "ipu1_ic_prp":0 []
-> "ipu1_vdic":0 []
pad2: Source
[fmt:SRGGB8/816x616 field:none
 crop.bounds:(0,0)/816x616
 crop:(0,0)/816x616]
-> "ipu1_csi0 capture":0 [ENABLED]

While the size has been propagated to pad1, the format has not.


Right, Philipp also caught this. I need to finish propagating all
params from sink to source pads (mbus code and field, and colorimetry
eventually).



2) /dev/video* device node assignment

I've no idea at the moment how the correct /dev/video* node should be
chosen - initially with Philipp and your previous code, it was
/dev/video3 after initial boot.  Philipp's was consistently /dev/video3.
Yours changed to /dev/video7 when removing and re-inserting the modules
(having fixed that locally.)  This version makes CSI0 be /dev/video7,
but after a remove+reinsert, it becomes (eg) /dev/video8.

/dev/v4l/by-path/platform-capture-subsystem-video-index4 also is not a
stable path - the digit changes (it's supposed to be a stable path.)
After a remove+reinsert, it becomes (eg)
/dev/v4l/by-path/platform-capture-subsystem-video-index5.
/dev/v4l/by-id doesn't contain a symlink for this either.

What this means is that it's very hard to script the setup, because
there's no easy way to know what device is the capture device.  While
it may be possible to do:

media-ctl -d /dev/media1 -p | \
grep -A2 ': ipu1_csi0 capture' | \
sed -n 's|.*\(/dev/video[0-9]*\).*|\1|p'

that's hardly a nice solution - while it fixes the setup script, it
doesn't stop the pain of having to delve around to find the correct
device to use for gstreamer to test with.



I'll try to nail down the main capture node numbers, even after module
reload.

Steve



Re: [PATCH v4 00/36] i.MX Media Driver

2017-02-16 Thread Russell King - ARM Linux
Two problems.

On Wed, Feb 15, 2017 at 06:19:02PM -0800, Steve Longerbeam wrote:
>   media: imx: propagate sink pad formats to source pads

1) It looks like all cases aren't being caught:

- entity 74: ipu1_csi0 (3 pads, 4 links)
 type V4L2 subdev subtype Unknown flags 0
 device node name /dev/v4l-subdev13
pad0: Sink
[fmt:SRGGB8/816x616 field:none]
<- "ipu1_csi0_mux":2 [ENABLED]
pad1: Source
[fmt:AYUV32/816x616 field:none
 crop.bounds:(0,0)/816x616
 crop:(0,0)/816x616]
-> "ipu1_ic_prp":0 []
-> "ipu1_vdic":0 []
pad2: Source
[fmt:SRGGB8/816x616 field:none
 crop.bounds:(0,0)/816x616
 crop:(0,0)/816x616]
-> "ipu1_csi0 capture":0 [ENABLED]

While the size has been propagated to pad1, the format has not.

2) /dev/video* device node assignment

I've no idea at the moment how the correct /dev/video* node should be
chosen - initially with Philipp and your previous code, it was
/dev/video3 after initial boot.  Philipp's was consistently /dev/video3.
Yours changed to /dev/video7 when removing and re-inserting the modules
(having fixed that locally.)  This version makes CSI0 be /dev/video7,
but after a remove+reinsert, it becomes (eg) /dev/video8.

/dev/v4l/by-path/platform-capture-subsystem-video-index4 also is not a
stable path - the digit changes (it's supposed to be a stable path.)
After a remove+reinsert, it becomes (eg)
/dev/v4l/by-path/platform-capture-subsystem-video-index5.
/dev/v4l/by-id doesn't contain a symlink for this either.

What this means is that it's very hard to script the setup, because
there's no easy way to know what device is the capture device.  While
it may be possible to do:

media-ctl -d /dev/media1 -p | \
grep -A2 ': ipu1_csi0 capture' | \
sed -n 's|.*\(/dev/video[0-9]*\).*|\1|p'

that's hardly a nice solution - while it fixes the setup script, it
doesn't stop the pain of having to delve around to find the correct
device to use for gstreamer to test with.

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