Re: [PATCH v2 0/2] media: i2c: ov5640: Re-work MIPI startup sequence

2018-08-15 Thread jacopo mondi
Hi Steve,

On Tue, Aug 14, 2018 at 04:53:26PM -0700, Steve Longerbeam wrote:
>
>
> On 08/14/2018 10:38 AM, jacopo mondi wrote:
> >Hi Steve,
> >
> >On Tue, Aug 14, 2018 at 09:51:04AM -0700, Steve Longerbeam wrote:
> >>Hi Jacopo,
> >>
> >>
> >>On 08/14/2018 08:35 AM, jacopo mondi wrote:
> >>>Hi Steve,
> >>>sorry for resurecting this.
> >>>
> >>>
> >I'm sorry I'm not sur I'm following. Does this mean that with that bug
> >you are referring to up here fixed by my last patch you have capture
> >working?
> No, capture still not working for me on SabreSD, even after fixing
> the bug in 476dec0 "media: ov5640: Add horizontal and vertical totals",
> by either using your patchset, or by running version 476dec0 of ov5640.c
> with the call to ov5640_set_timings() moved to the correct places as
> described below.
> 
> >>>I've been reported a bug on exposure handling that makes the first
> >>>captured frames all black. Both me and Hugues have tried to fix the
> >>>issue (him with a more complete series, but that's another topic).
> >>>See [1] and [2]
> >>>
> >>>It might be possible that you're getting blank frames with this series
> >>>applied? I never seen them as I'm skipping the first frames when
> >>>capturing, but I've now tested and without the exposure fixes (either
> >>>[1] or [2]) I actually have blank frames.
> >>>
> >>>If that's the case for you too (which I hope so much) would you be
> >>>available to test again this series with exposure fixes on top?
> >>>On my platform that actually makes all frames correct.
> >>>
> >>>Thanks
> >>>j
> >>>
> >>>[1] [PATCH 0/2] media: ov5640: Fix set_timings and auto-exposure
> >>>[2] [PATCH v2 0/5] Fix OV5640 exposure & gain
> >>>
> >>It's not clear to me which patch sets you would like me to test.
> >>Just [1] and [2], or [1], [2], and "media: i2c: ov5640: Re-work MIPI startup
> >>sequence"?
> >>
> >I have tested on my board the following:
> >v4.18-rc2 + MIPI Fix + Timings + Hugues' exposure fix
> >
> >Without Hugues' patches I get blank frames (the first ones at least)
> >Without MIPI startup reowkr and timings I get the LP-11 error on the
> >CSI-2 bus.
> >
> >As Hugues' series has to be rebased on mine, I have prepared a branch
> >here for you if you feel like testing it:
> >git://jmondi.org/linux ov5640/timings_exposure
>
> Hi Jacopo, that branch works on SabreSD!
>

YEAH! I'm so happy about this (and not to having asked you to test
yet-another-broken-patchset for no reason :)

> Feel free to add
>
> Tested-by: Steve Longerbeam 
> on i.MX6q SabreSD with MIPI CSI-2 OV5640 module
>
> to whichever ov5640 patches are appropriate.

Great, I'll send a v3 now collecting your tags and the most recent
version of the timings fix that was not included in v2 (but in the
branch you have tested).

My ideal plan would be to have these two patches merged, then Hugues'
series to fix exposure handling merged on top. This would, in my mind
make capture on CSI-2 work properly.

Of course, more testing is very welcome.

Thanks again
   j

>
> Steve
>
>


signature.asc
Description: PGP signature


Re: [PATCH v2 0/2] media: i2c: ov5640: Re-work MIPI startup sequence

2018-08-14 Thread Steve Longerbeam




On 08/14/2018 10:38 AM, jacopo mondi wrote:

Hi Steve,

On Tue, Aug 14, 2018 at 09:51:04AM -0700, Steve Longerbeam wrote:

Hi Jacopo,


On 08/14/2018 08:35 AM, jacopo mondi wrote:

Hi Steve,
sorry for resurecting this.



I'm sorry I'm not sur I'm following. Does this mean that with that bug
you are referring to up here fixed by my last patch you have capture
working?

No, capture still not working for me on SabreSD, even after fixing
the bug in 476dec0 "media: ov5640: Add horizontal and vertical totals",
by either using your patchset, or by running version 476dec0 of ov5640.c
with the call to ov5640_set_timings() moved to the correct places as
described below.


I've been reported a bug on exposure handling that makes the first
captured frames all black. Both me and Hugues have tried to fix the
issue (him with a more complete series, but that's another topic).
See [1] and [2]

It might be possible that you're getting blank frames with this series
applied? I never seen them as I'm skipping the first frames when
capturing, but I've now tested and without the exposure fixes (either
[1] or [2]) I actually have blank frames.

If that's the case for you too (which I hope so much) would you be
available to test again this series with exposure fixes on top?
On my platform that actually makes all frames correct.

Thanks
j

[1] [PATCH 0/2] media: ov5640: Fix set_timings and auto-exposure
[2] [PATCH v2 0/5] Fix OV5640 exposure & gain


It's not clear to me which patch sets you would like me to test.
Just [1] and [2], or [1], [2], and "media: i2c: ov5640: Re-work MIPI startup
sequence"?


I have tested on my board the following:
v4.18-rc2 + MIPI Fix + Timings + Hugues' exposure fix

Without Hugues' patches I get blank frames (the first ones at least)
Without MIPI startup reowkr and timings I get the LP-11 error on the
CSI-2 bus.

As Hugues' series has to be rebased on mine, I have prepared a branch
here for you if you feel like testing it:
git://jmondi.org/linux ov5640/timings_exposure


Hi Jacopo, that branch works on SabreSD!

Feel free to add

Tested-by: Steve Longerbeam 
on i.MX6q SabreSD with MIPI CSI-2 OV5640 module

to whichever ov5640 patches are appropriate.

Steve




Re: [PATCH v2 0/2] media: i2c: ov5640: Re-work MIPI startup sequence

2018-08-14 Thread jacopo mondi
Hi Steve,

On Tue, Aug 14, 2018 at 09:51:04AM -0700, Steve Longerbeam wrote:
> Hi Jacopo,
>
>
> On 08/14/2018 08:35 AM, jacopo mondi wrote:
> >Hi Steve,
> >sorry for resurecting this.
> >
> >
> >>>I'm sorry I'm not sur I'm following. Does this mean that with that bug
> >>>you are referring to up here fixed by my last patch you have capture
> >>>working?
> >>No, capture still not working for me on SabreSD, even after fixing
> >>the bug in 476dec0 "media: ov5640: Add horizontal and vertical totals",
> >>by either using your patchset, or by running version 476dec0 of ov5640.c
> >>with the call to ov5640_set_timings() moved to the correct places as
> >>described below.
> >>
> >I've been reported a bug on exposure handling that makes the first
> >captured frames all black. Both me and Hugues have tried to fix the
> >issue (him with a more complete series, but that's another topic).
> >See [1] and [2]
> >
> >It might be possible that you're getting blank frames with this series
> >applied? I never seen them as I'm skipping the first frames when
> >capturing, but I've now tested and without the exposure fixes (either
> >[1] or [2]) I actually have blank frames.
> >
> >If that's the case for you too (which I hope so much) would you be
> >available to test again this series with exposure fixes on top?
> >On my platform that actually makes all frames correct.
> >
> >Thanks
> >j
> >
> >[1] [PATCH 0/2] media: ov5640: Fix set_timings and auto-exposure
> >[2] [PATCH v2 0/5] Fix OV5640 exposure & gain
> >
>
> It's not clear to me which patch sets you would like me to test.
> Just [1] and [2], or [1], [2], and "media: i2c: ov5640: Re-work MIPI startup
> sequence"?
>
I have tested on my board the following:
v4.18-rc2 + MIPI Fix + Timings + Hugues' exposure fix

Without Hugues' patches I get blank frames (the first ones at least)
Without MIPI startup reowkr and timings I get the LP-11 error on the
CSI-2 bus.

As Hugues' series has to be rebased on mine, I have prepared a branch
here for you if you feel like testing it:
git://jmondi.org/linux ov5640/timings_exposure

Thanks
   j


> Steve
>
>
>


signature.asc
Description: PGP signature


Re: [PATCH v2 0/2] media: i2c: ov5640: Re-work MIPI startup sequence

2018-08-14 Thread Steve Longerbeam

Hi Jacopo,


On 08/14/2018 08:35 AM, jacopo mondi wrote:

Hi Steve,
sorry for resurecting this.



I'm sorry I'm not sur I'm following. Does this mean that with that bug
you are referring to up here fixed by my last patch you have capture
working?

No, capture still not working for me on SabreSD, even after fixing
the bug in 476dec0 "media: ov5640: Add horizontal and vertical totals",
by either using your patchset, or by running version 476dec0 of ov5640.c
with the call to ov5640_set_timings() moved to the correct places as
described below.


I've been reported a bug on exposure handling that makes the first
captured frames all black. Both me and Hugues have tried to fix the
issue (him with a more complete series, but that's another topic).
See [1] and [2]

It might be possible that you're getting blank frames with this series
applied? I never seen them as I'm skipping the first frames when
capturing, but I've now tested and without the exposure fixes (either
[1] or [2]) I actually have blank frames.

If that's the case for you too (which I hope so much) would you be
available to test again this series with exposure fixes on top?
On my platform that actually makes all frames correct.

Thanks
j

[1] [PATCH 0/2] media: ov5640: Fix set_timings and auto-exposure
[2] [PATCH v2 0/5] Fix OV5640 exposure & gain



It's not clear to me which patch sets you would like me to test.
Just [1] and [2], or [1], [2], and "media: i2c: ov5640: Re-work MIPI 
startup sequence"?


Steve





Re: [PATCH v2 0/2] media: i2c: ov5640: Re-work MIPI startup sequence

2018-08-14 Thread jacopo mondi
Hi Steve,
   sorry for resurecting this.

On Mon, Jul 16, 2018 at 09:26:13AM -0700, Steve Longerbeam wrote:
>
>
> On 07/16/2018 01:29 AM, jacopo mondi wrote:
> >Hi Steve,
> >thanks for keep testing it
> >
> >On Sat, Jul 14, 2018 at 01:02:32PM -0700, Steve Longerbeam wrote:
> >>
> >>On 07/14/2018 12:41 PM, Steve Longerbeam wrote:
> >>>Hi Jacopo,
> >>>
> >>>
> >>>On 07/14/2018 11:57 AM, Steve Longerbeam wrote:
> Hi Jacopo,
> 
> Pardon the late reply, see below.
> 
> On 07/11/2018 12:21 AM, jacopo mondi wrote:
> >Hi Steve,
> >
> >On Tue, Jul 10, 2018 at 02:10:54PM -0700, Steve Longerbeam wrote:
> >>Hi Jacopo,
> >>
> >>Sorry to report my testing on SabreSD has same result
> >>as last time. This series fixes the LP-11 timeout at stream
> >>on but captured images are still blank. I tried the 640x480
> >>mode with UYVY2X8. Here is the pad config:
> >This saddens me :(
> >
> >I'm capturing with the same format and sizes... this shouldn't be the
> >issue
> >
> >Could you confirm this matches what you have in your tree?
> >5dc2c80 media: ov5640: Fix timings setup code
> >b35e757 media: i2c: ov5640: Re-work MIPI startup sequence
> >3c4a737 media: ov5640: fix frame interval enumeration
> >41cb1c7 media: ov5640: adjust xclk_max
> >c3f3ba3 media: ov5640: add support of module orientation
> >ce85705 media: ov5640: add HFLIP/VFLIP controls support
> >8663341 media: ov5640: Program the visible resolution
> >476dec0 media: ov5640: Add horizontal and vertical totals
> >dba13a0 media: ov5640: Change horizontal and vertical resolutions name
> >8f57c2f media: ov5640: Init properly the SCLK dividers
> Yes, I have that commit sequence.
> 
> FWIW, I can verify what Jagan Teki reported earlier, that the driver
> still
> works on the SabreSD platform at:
> 
> dba13a0 media: ov5640: Change horizontal and vertical resolutions name
> 
> and is broken at:
> 
> 476dec0 media: ov5640: Add horizontal and vertical totals
> 
> with LP-11 timeout at the mipi csi-2 receiver:
> 
> [   80.763189] imx6-mipi-csi2: LP-11 timeout, phy_state = 0x0230
> [   80.769599] ipu1_csi1: pipeline start failed with -110
> >>>And I discovered the bug in 476dec0 "media: ov5640: Add horizontal and
> >>>vertical totals". The call to ov5640_set_timings() needs to be moved
> >>>before the
> >>>calls to ov5640_get_vts() and ov5640_get_hts(). But I see you have
> >>>discovered
> >>>that as well, and fixed in the second patch in your series.
> >>>
> >I'm sorry I'm not sur I'm following. Does this mean that with that bug
> >you are referring to up here fixed by my last patch you have capture
> >working?
>
> No, capture still not working for me on SabreSD, even after fixing
> the bug in 476dec0 "media: ov5640: Add horizontal and vertical totals",
> by either using your patchset, or by running version 476dec0 of ov5640.c
> with the call to ov5640_set_timings() moved to the correct places as
> described below.
>

I've been reported a bug on exposure handling that makes the first
captured frames all black. Both me and Hugues have tried to fix the
issue (him with a more complete series, but that's another topic).
See [1] and [2]

It might be possible that you're getting blank frames with this series
applied? I never seen them as I'm skipping the first frames when
capturing, but I've now tested and without the exposure fixes (either
[1] or [2]) I actually have blank frames.

If that's the case for you too (which I hope so much) would you be
available to test again this series with exposure fixes on top?
On my platform that actually makes all frames correct.

Thanks
   j

[1] [PATCH 0/2] media: ov5640: Fix set_timings and auto-exposure
[2] [PATCH v2 0/5] Fix OV5640 exposure & gain

> Steve
>
> >>But strangely, if I revert to 476dec0, and then move the call to
> >>ov5640_set_timings()
> >>to just after ov5640_load_regs() in ov5640_set_mode_exposure_calc() and
> >>ov5640_set_mode_direct(), the LP-11 timeouts are still present. So I can
> >>confirm
> >>this strangeness which you already pointed out below [1].
> >>
> >>
> >>>The version I'm sending here re-introduces some of the timings
> >>>parameters in the
> >>>initial configuration blob (not in the single mode ones), which
> >>>apparently has
> >>>to be at least initially programmed to allow the driver to later
> >>>program them
> >>>singularly in the 'set_timings()' function. Unfortunately I do not
> >>>have a real
> >>>rationale behind this which explains why it has to be done this
> >>>way :(
> >>>
> >>[1] here :)
> >>
> >>Steve
> >>
> >>
>


signature.asc
Description: PGP signature


Re: [PATCH v2 0/2] media: i2c: ov5640: Re-work MIPI startup sequence

2018-07-16 Thread Steve Longerbeam




On 07/16/2018 01:29 AM, jacopo mondi wrote:

Hi Steve,
thanks for keep testing it

On Sat, Jul 14, 2018 at 01:02:32PM -0700, Steve Longerbeam wrote:


On 07/14/2018 12:41 PM, Steve Longerbeam wrote:

Hi Jacopo,


On 07/14/2018 11:57 AM, Steve Longerbeam wrote:

Hi Jacopo,

Pardon the late reply, see below.

On 07/11/2018 12:21 AM, jacopo mondi wrote:

Hi Steve,

On Tue, Jul 10, 2018 at 02:10:54PM -0700, Steve Longerbeam wrote:

Hi Jacopo,

Sorry to report my testing on SabreSD has same result
as last time. This series fixes the LP-11 timeout at stream
on but captured images are still blank. I tried the 640x480
mode with UYVY2X8. Here is the pad config:

This saddens me :(

I'm capturing with the same format and sizes... this shouldn't be the
issue

Could you confirm this matches what you have in your tree?
5dc2c80 media: ov5640: Fix timings setup code
b35e757 media: i2c: ov5640: Re-work MIPI startup sequence
3c4a737 media: ov5640: fix frame interval enumeration
41cb1c7 media: ov5640: adjust xclk_max
c3f3ba3 media: ov5640: add support of module orientation
ce85705 media: ov5640: add HFLIP/VFLIP controls support
8663341 media: ov5640: Program the visible resolution
476dec0 media: ov5640: Add horizontal and vertical totals
dba13a0 media: ov5640: Change horizontal and vertical resolutions name
8f57c2f media: ov5640: Init properly the SCLK dividers

Yes, I have that commit sequence.

FWIW, I can verify what Jagan Teki reported earlier, that the driver
still
works on the SabreSD platform at:

dba13a0 media: ov5640: Change horizontal and vertical resolutions name

and is broken at:

476dec0 media: ov5640: Add horizontal and vertical totals

with LP-11 timeout at the mipi csi-2 receiver:

[   80.763189] imx6-mipi-csi2: LP-11 timeout, phy_state = 0x0230
[   80.769599] ipu1_csi1: pipeline start failed with -110

And I discovered the bug in 476dec0 "media: ov5640: Add horizontal and
vertical totals". The call to ov5640_set_timings() needs to be moved
before the
calls to ov5640_get_vts() and ov5640_get_hts(). But I see you have
discovered
that as well, and fixed in the second patch in your series.


I'm sorry I'm not sur I'm following. Does this mean that with that bug
you are referring to up here fixed by my last patch you have capture
working?


No, capture still not working for me on SabreSD, even after fixing
the bug in 476dec0 "media: ov5640: Add horizontal and vertical totals",
by either using your patchset, or by running version 476dec0 of ov5640.c
with the call to ov5640_set_timings() moved to the correct places as
described below.

Steve


But strangely, if I revert to 476dec0, and then move the call to
ov5640_set_timings()
to just after ov5640_load_regs() in ov5640_set_mode_exposure_calc() and
ov5640_set_mode_direct(), the LP-11 timeouts are still present. So I can
confirm
this strangeness which you already pointed out below [1].



The version I'm sending here re-introduces some of the timings
parameters in the
initial configuration blob (not in the single mode ones), which
apparently has
to be at least initially programmed to allow the driver to later
program them
singularly in the 'set_timings()' function. Unfortunately I do not
have a real
rationale behind this which explains why it has to be done this
way :(


[1] here :)

Steve






Re: [PATCH v2 0/2] media: i2c: ov5640: Re-work MIPI startup sequence

2018-07-16 Thread jacopo mondi
Hi Steve,
   thanks for keep testing it

On Sat, Jul 14, 2018 at 01:02:32PM -0700, Steve Longerbeam wrote:
>
>
> On 07/14/2018 12:41 PM, Steve Longerbeam wrote:
> >Hi Jacopo,
> >
> >
> >On 07/14/2018 11:57 AM, Steve Longerbeam wrote:
> >>Hi Jacopo,
> >>
> >>Pardon the late reply, see below.
> >>
> >>On 07/11/2018 12:21 AM, jacopo mondi wrote:
> >>>Hi Steve,
> >>>
> >>>On Tue, Jul 10, 2018 at 02:10:54PM -0700, Steve Longerbeam wrote:
> Hi Jacopo,
> 
> Sorry to report my testing on SabreSD has same result
> as last time. This series fixes the LP-11 timeout at stream
> on but captured images are still blank. I tried the 640x480
> mode with UYVY2X8. Here is the pad config:
> >>>This saddens me :(
> >>>
> >>>I'm capturing with the same format and sizes... this shouldn't be the
> >>>issue
> >>>
> >>>Could you confirm this matches what you have in your tree?
> >>>5dc2c80 media: ov5640: Fix timings setup code
> >>>b35e757 media: i2c: ov5640: Re-work MIPI startup sequence
> >>>3c4a737 media: ov5640: fix frame interval enumeration
> >>>41cb1c7 media: ov5640: adjust xclk_max
> >>>c3f3ba3 media: ov5640: add support of module orientation
> >>>ce85705 media: ov5640: add HFLIP/VFLIP controls support
> >>>8663341 media: ov5640: Program the visible resolution
> >>>476dec0 media: ov5640: Add horizontal and vertical totals
> >>>dba13a0 media: ov5640: Change horizontal and vertical resolutions name
> >>>8f57c2f media: ov5640: Init properly the SCLK dividers
> >>
> >>Yes, I have that commit sequence.
> >>
> >>FWIW, I can verify what Jagan Teki reported earlier, that the driver
> >>still
> >>works on the SabreSD platform at:
> >>
> >>dba13a0 media: ov5640: Change horizontal and vertical resolutions name
> >>
> >>and is broken at:
> >>
> >>476dec0 media: ov5640: Add horizontal and vertical totals
> >>
> >>with LP-11 timeout at the mipi csi-2 receiver:
> >>
> >>[   80.763189] imx6-mipi-csi2: LP-11 timeout, phy_state = 0x0230
> >>[   80.769599] ipu1_csi1: pipeline start failed with -110
> >
> >And I discovered the bug in 476dec0 "media: ov5640: Add horizontal and
> >vertical totals". The call to ov5640_set_timings() needs to be moved
> >before the
> >calls to ov5640_get_vts() and ov5640_get_hts(). But I see you have
> >discovered
> >that as well, and fixed in the second patch in your series.
> >

I'm sorry I'm not sur I'm following. Does this mean that with that bug
you are referring to up here fixed by my last patch you have capture
working?

Thanks
   j

>
> But strangely, if I revert to 476dec0, and then move the call to
> ov5640_set_timings()
> to just after ov5640_load_regs() in ov5640_set_mode_exposure_calc() and
> ov5640_set_mode_direct(), the LP-11 timeouts are still present. So I can
> confirm
> this strangeness which you already pointed out below [1].
>
>
> >
> >>
> >>>
> 
> >
> >The version I'm sending here re-introduces some of the timings
> >parameters in the
> >initial configuration blob (not in the single mode ones), which
> >apparently has
> >to be at least initially programmed to allow the driver to later
> >program them
> >singularly in the 'set_timings()' function. Unfortunately I do not
> >have a real
> >rationale behind this which explains why it has to be done this
> >way :(
> >
>
> [1] here :)
>
> Steve
>
>


signature.asc
Description: PGP signature


Re: [PATCH v2 0/2] media: i2c: ov5640: Re-work MIPI startup sequence

2018-07-14 Thread Steve Longerbeam




On 07/14/2018 12:41 PM, Steve Longerbeam wrote:

Hi Jacopo,


On 07/14/2018 11:57 AM, Steve Longerbeam wrote:

Hi Jacopo,

Pardon the late reply, see below.

On 07/11/2018 12:21 AM, jacopo mondi wrote:

Hi Steve,

On Tue, Jul 10, 2018 at 02:10:54PM -0700, Steve Longerbeam wrote:

Hi Jacopo,

Sorry to report my testing on SabreSD has same result
as last time. This series fixes the LP-11 timeout at stream
on but captured images are still blank. I tried the 640x480
mode with UYVY2X8. Here is the pad config:

This saddens me :(

I'm capturing with the same format and sizes... this shouldn't be the
issue

Could you confirm this matches what you have in your tree?
5dc2c80 media: ov5640: Fix timings setup code
b35e757 media: i2c: ov5640: Re-work MIPI startup sequence
3c4a737 media: ov5640: fix frame interval enumeration
41cb1c7 media: ov5640: adjust xclk_max
c3f3ba3 media: ov5640: add support of module orientation
ce85705 media: ov5640: add HFLIP/VFLIP controls support
8663341 media: ov5640: Program the visible resolution
476dec0 media: ov5640: Add horizontal and vertical totals
dba13a0 media: ov5640: Change horizontal and vertical resolutions name
8f57c2f media: ov5640: Init properly the SCLK dividers


Yes, I have that commit sequence.

FWIW, I can verify what Jagan Teki reported earlier, that the driver 
still

works on the SabreSD platform at:

dba13a0 media: ov5640: Change horizontal and vertical resolutions name

and is broken at:

476dec0 media: ov5640: Add horizontal and vertical totals

with LP-11 timeout at the mipi csi-2 receiver:

[   80.763189] imx6-mipi-csi2: LP-11 timeout, phy_state = 0x0230
[   80.769599] ipu1_csi1: pipeline start failed with -110


And I discovered the bug in 476dec0 "media: ov5640: Add horizontal and
vertical totals". The call to ov5640_set_timings() needs to be moved 
before the
calls to ov5640_get_vts() and ov5640_get_hts(). But I see you have 
discovered

that as well, and fixed in the second patch in your series.



But strangely, if I revert to 476dec0, and then move the call to 
ov5640_set_timings()

to just after ov5640_load_regs() in ov5640_set_mode_exposure_calc() and
ov5640_set_mode_direct(), the LP-11 timeouts are still present. So I can 
confirm

this strangeness which you already pointed out below [1].












The version I'm sending here re-introduces some of the timings 
parameters in the
initial configuration blob (not in the single mode ones), which 
apparently has
to be at least initially programmed to allow the driver to later 
program them
singularly in the 'set_timings()' function. Unfortunately I do not 
have a real
rationale behind this which explains why it has to be done this 
way :(




[1] here :)

Steve




Re: [PATCH v2 0/2] media: i2c: ov5640: Re-work MIPI startup sequence

2018-07-14 Thread Steve Longerbeam

Hi Jacopo,


On 07/14/2018 11:57 AM, Steve Longerbeam wrote:

Hi Jacopo,

Pardon the late reply, see below.

On 07/11/2018 12:21 AM, jacopo mondi wrote:

Hi Steve,

On Tue, Jul 10, 2018 at 02:10:54PM -0700, Steve Longerbeam wrote:

Hi Jacopo,

Sorry to report my testing on SabreSD has same result
as last time. This series fixes the LP-11 timeout at stream
on but captured images are still blank. I tried the 640x480
mode with UYVY2X8. Here is the pad config:

This saddens me :(

I'm capturing with the same format and sizes... this shouldn't be the
issue

Could you confirm this matches what you have in your tree?
5dc2c80 media: ov5640: Fix timings setup code
b35e757 media: i2c: ov5640: Re-work MIPI startup sequence
3c4a737 media: ov5640: fix frame interval enumeration
41cb1c7 media: ov5640: adjust xclk_max
c3f3ba3 media: ov5640: add support of module orientation
ce85705 media: ov5640: add HFLIP/VFLIP controls support
8663341 media: ov5640: Program the visible resolution
476dec0 media: ov5640: Add horizontal and vertical totals
dba13a0 media: ov5640: Change horizontal and vertical resolutions name
8f57c2f media: ov5640: Init properly the SCLK dividers


Yes, I have that commit sequence.

FWIW, I can verify what Jagan Teki reported earlier, that the driver 
still

works on the SabreSD platform at:

dba13a0 media: ov5640: Change horizontal and vertical resolutions name

and is broken at:

476dec0 media: ov5640: Add horizontal and vertical totals

with LP-11 timeout at the mipi csi-2 receiver:

[   80.763189] imx6-mipi-csi2: LP-11 timeout, phy_state = 0x0230
[   80.769599] ipu1_csi1: pipeline start failed with -110


And I discovered the bug in 476dec0 "media: ov5640: Add horizontal and
vertical totals". The call to ov5640_set_timings() needs to be moved 
before the
calls to ov5640_get_vts() and ov5640_get_hts(). But I see you have 
discovered

that as well, and fixed in the second patch in your series.


Steve










# media-ctl --get-v4l2 "'ov5640 1-003c':0"
     [fmt:UYVY8_2X8/640x480@1/30 field:none colorspace:srgb 
xfer:srgb

ycbcr:601 quantization:full-range]

Steve

On 07/10/2018 11:36 AM, Jacopo Mondi wrote:

Hello,
    this series fixes capture operations on i.MX6Q platforms (and 
possible other

platforms reported not working) using MIPI CSI-2 interface.

This iteration expands the v1 version with an additional fix, 
initially

submitted by Maxime in his series:
[PATCH v3 00/12] media: ov5640: Misc cleanup and improvements
https://www.spinics.net/lists/linux-media/msg134436.html

The original patch has been reported not fully fixing the issues by 
Daniel Mack

in his comment here below (on a Qualcomm platform if I'm not wrong):
https://www.spinics.net/lists/linux-media/msg134524.html
On my i.MX6Q testing platform that patch alone does not fix MIPI 
capture

neither.

The version I'm sending here re-introduces some of the timings 
parameters in the
initial configuration blob (not in the single mode ones), which 
apparently has
to be at least initially programmed to allow the driver to later 
program them
singularly in the 'set_timings()' function. Unfortunately I do not 
have a real

rationale behind this which explains why it has to be done this way :(

For the MIPI startup sequence re-work patch, no changes compared to 
v1.
Steve reported he has verified the LP-11 timout issue is solved on 
his testing

platform too. For more details, please refer to the v1 cover letter:
https://www.mail-archive.com/linux-media@vger.kernel.org/msg133352.html 



Thanks
    j

Jacopo Mondi (1):
   media: i2c: ov5640: Re-work MIPI startup sequence

Samuel Bobrowicz (1):
   media: ov5640: Fix timings setup code

  drivers/media/i2c/ov5640.c | 107 
++---

  1 file changed, 82 insertions(+), 25 deletions(-)

--
2.7.4







Re: [PATCH v2 0/2] media: i2c: ov5640: Re-work MIPI startup sequence

2018-07-14 Thread Steve Longerbeam

Hi Jacopo,

Pardon the late reply, see below.

On 07/11/2018 12:21 AM, jacopo mondi wrote:

Hi Steve,

On Tue, Jul 10, 2018 at 02:10:54PM -0700, Steve Longerbeam wrote:

Hi Jacopo,

Sorry to report my testing on SabreSD has same result
as last time. This series fixes the LP-11 timeout at stream
on but captured images are still blank. I tried the 640x480
mode with UYVY2X8. Here is the pad config:

This saddens me :(

I'm capturing with the same format and sizes... this shouldn't be the
issue

Could you confirm this matches what you have in your tree?
5dc2c80 media: ov5640: Fix timings setup code
b35e757 media: i2c: ov5640: Re-work MIPI startup sequence
3c4a737 media: ov5640: fix frame interval enumeration
41cb1c7 media: ov5640: adjust xclk_max
c3f3ba3 media: ov5640: add support of module orientation
ce85705 media: ov5640: add HFLIP/VFLIP controls support
8663341 media: ov5640: Program the visible resolution
476dec0 media: ov5640: Add horizontal and vertical totals
dba13a0 media: ov5640: Change horizontal and vertical resolutions name
8f57c2f media: ov5640: Init properly the SCLK dividers


Yes, I have that commit sequence.

FWIW, I can verify what Jagan Teki reported earlier, that the driver still
works on the SabreSD platform at:

dba13a0 media: ov5640: Change horizontal and vertical resolutions name

and is broken at:

476dec0 media: ov5640: Add horizontal and vertical totals

with LP-11 timeout at the mipi csi-2 receiver:

[   80.763189] imx6-mipi-csi2: LP-11 timeout, phy_state = 0x0230
[   80.769599] ipu1_csi1: pipeline start failed with -110


Steve






# media-ctl --get-v4l2 "'ov5640 1-003c':0"
         [fmt:UYVY8_2X8/640x480@1/30 field:none colorspace:srgb xfer:srgb
ycbcr:601 quantization:full-range]

Steve

On 07/10/2018 11:36 AM, Jacopo Mondi wrote:

Hello,
this series fixes capture operations on i.MX6Q platforms (and possible other
platforms reported not working) using MIPI CSI-2 interface.

This iteration expands the v1 version with an additional fix, initially
submitted by Maxime in his series:
[PATCH v3 00/12] media: ov5640: Misc cleanup and improvements
https://www.spinics.net/lists/linux-media/msg134436.html

The original patch has been reported not fully fixing the issues by Daniel Mack
in his comment here below (on a Qualcomm platform if I'm not wrong):
https://www.spinics.net/lists/linux-media/msg134524.html
On my i.MX6Q testing platform that patch alone does not fix MIPI capture
neither.

The version I'm sending here re-introduces some of the timings parameters in the
initial configuration blob (not in the single mode ones), which apparently has
to be at least initially programmed to allow the driver to later program them
singularly in the 'set_timings()' function. Unfortunately I do not have a real
rationale behind this which explains why it has to be done this way :(

For the MIPI startup sequence re-work patch, no changes compared to v1.
Steve reported he has verified the LP-11 timout issue is solved on his testing
platform too. For more details, please refer to the v1 cover letter:
https://www.mail-archive.com/linux-media@vger.kernel.org/msg133352.html

Thanks
j

Jacopo Mondi (1):
   media: i2c: ov5640: Re-work MIPI startup sequence

Samuel Bobrowicz (1):
   media: ov5640: Fix timings setup code

  drivers/media/i2c/ov5640.c | 107 ++---
  1 file changed, 82 insertions(+), 25 deletions(-)

--
2.7.4





Re: [PATCH v2 0/2] media: i2c: ov5640: Re-work MIPI startup sequence

2018-07-11 Thread jacopo mondi
Hi Steve,

On Tue, Jul 10, 2018 at 02:10:54PM -0700, Steve Longerbeam wrote:
> Hi Jacopo,
>
> Sorry to report my testing on SabreSD has same result
> as last time. This series fixes the LP-11 timeout at stream
> on but captured images are still blank. I tried the 640x480
> mode with UYVY2X8. Here is the pad config:

This saddens me :(

I'm capturing with the same format and sizes... this shouldn't be the
issue

Could you confirm this matches what you have in your tree?
5dc2c80 media: ov5640: Fix timings setup code
b35e757 media: i2c: ov5640: Re-work MIPI startup sequence
3c4a737 media: ov5640: fix frame interval enumeration
41cb1c7 media: ov5640: adjust xclk_max
c3f3ba3 media: ov5640: add support of module orientation
ce85705 media: ov5640: add HFLIP/VFLIP controls support
8663341 media: ov5640: Program the visible resolution
476dec0 media: ov5640: Add horizontal and vertical totals
dba13a0 media: ov5640: Change horizontal and vertical resolutions name
8f57c2f media: ov5640: Init properly the SCLK dividers

Thanks
   j

>
> # media-ctl --get-v4l2 "'ov5640 1-003c':0"
>         [fmt:UYVY8_2X8/640x480@1/30 field:none colorspace:srgb xfer:srgb
> ycbcr:601 quantization:full-range]
>
> Steve
>
> On 07/10/2018 11:36 AM, Jacopo Mondi wrote:
> >Hello,
> >this series fixes capture operations on i.MX6Q platforms (and possible 
> > other
> >platforms reported not working) using MIPI CSI-2 interface.
> >
> >This iteration expands the v1 version with an additional fix, initially
> >submitted by Maxime in his series:
> >[PATCH v3 00/12] media: ov5640: Misc cleanup and improvements
> >https://www.spinics.net/lists/linux-media/msg134436.html
> >
> >The original patch has been reported not fully fixing the issues by Daniel 
> >Mack
> >in his comment here below (on a Qualcomm platform if I'm not wrong):
> >https://www.spinics.net/lists/linux-media/msg134524.html
> >On my i.MX6Q testing platform that patch alone does not fix MIPI capture
> >neither.
> >
> >The version I'm sending here re-introduces some of the timings parameters in 
> >the
> >initial configuration blob (not in the single mode ones), which apparently 
> >has
> >to be at least initially programmed to allow the driver to later program them
> >singularly in the 'set_timings()' function. Unfortunately I do not have a 
> >real
> >rationale behind this which explains why it has to be done this way :(
> >
> >For the MIPI startup sequence re-work patch, no changes compared to v1.
> >Steve reported he has verified the LP-11 timout issue is solved on his 
> >testing
> >platform too. For more details, please refer to the v1 cover letter:
> >https://www.mail-archive.com/linux-media@vger.kernel.org/msg133352.html
> >
> >Thanks
> >j
> >
> >Jacopo Mondi (1):
> >   media: i2c: ov5640: Re-work MIPI startup sequence
> >
> >Samuel Bobrowicz (1):
> >   media: ov5640: Fix timings setup code
> >
> >  drivers/media/i2c/ov5640.c | 107 
> > ++---
> >  1 file changed, 82 insertions(+), 25 deletions(-)
> >
> >--
> >2.7.4
> >
>


signature.asc
Description: PGP signature


Re: [PATCH v2 0/2] media: i2c: ov5640: Re-work MIPI startup sequence

2018-07-10 Thread Steve Longerbeam

Hi Jacopo,

Sorry to report my testing on SabreSD has same result
as last time. This series fixes the LP-11 timeout at stream
on but captured images are still blank. I tried the 640x480
mode with UYVY2X8. Here is the pad config:

# media-ctl --get-v4l2 "'ov5640 1-003c':0"
        [fmt:UYVY8_2X8/640x480@1/30 field:none colorspace:srgb 
xfer:srgb ycbcr:601 quantization:full-range]


Steve

On 07/10/2018 11:36 AM, Jacopo Mondi wrote:

Hello,
this series fixes capture operations on i.MX6Q platforms (and possible other
platforms reported not working) using MIPI CSI-2 interface.

This iteration expands the v1 version with an additional fix, initially
submitted by Maxime in his series:
[PATCH v3 00/12] media: ov5640: Misc cleanup and improvements
https://www.spinics.net/lists/linux-media/msg134436.html

The original patch has been reported not fully fixing the issues by Daniel Mack
in his comment here below (on a Qualcomm platform if I'm not wrong):
https://www.spinics.net/lists/linux-media/msg134524.html
On my i.MX6Q testing platform that patch alone does not fix MIPI capture
neither.

The version I'm sending here re-introduces some of the timings parameters in the
initial configuration blob (not in the single mode ones), which apparently has
to be at least initially programmed to allow the driver to later program them
singularly in the 'set_timings()' function. Unfortunately I do not have a real
rationale behind this which explains why it has to be done this way :(

For the MIPI startup sequence re-work patch, no changes compared to v1.
Steve reported he has verified the LP-11 timout issue is solved on his testing
platform too. For more details, please refer to the v1 cover letter:
https://www.mail-archive.com/linux-media@vger.kernel.org/msg133352.html

Thanks
j

Jacopo Mondi (1):
   media: i2c: ov5640: Re-work MIPI startup sequence

Samuel Bobrowicz (1):
   media: ov5640: Fix timings setup code

  drivers/media/i2c/ov5640.c | 107 ++---
  1 file changed, 82 insertions(+), 25 deletions(-)

--
2.7.4