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

[Steve Longerbeam <slongerbeam@gmail.com>]

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.


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