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

[jacopo mondi <jacopo@jmondi.org>]

[-- Attachment #1: Type: text/plain, Size: 3454 bytes --]

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 = 0x00000230
> >>[   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
>
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]