* [PATCH v2 0/2] media: i2c: ov5640: Re-work MIPI startup sequence @ 2018-07-10 18:36 Jacopo Mondi 2018-07-10 18:36 ` [PATCH v2 1/2] " Jacopo Mondi ` (2 more replies) 0 siblings, 3 replies; 15+ messages in thread From: Jacopo Mondi @ 2018-07-10 18:36 UTC (permalink / raw) To: mchehab, laurent.pinchart, maxime.ripard, sam, jagan, festevam, pza, steve_longerbeam, hugues.fruchet, loic.poulain, daniel Cc: Jacopo Mondi, linux-media 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 1/2] media: i2c: ov5640: Re-work MIPI startup sequence 2018-07-10 18:36 [PATCH v2 0/2] media: i2c: ov5640: Re-work MIPI startup sequence Jacopo Mondi @ 2018-07-10 18:36 ` Jacopo Mondi 2018-07-10 18:36 ` [PATCH v2 2/2] media: ov5640: Fix timings setup code Jacopo Mondi 2018-07-10 21:10 ` [PATCH v2 0/2] media: i2c: ov5640: Re-work MIPI startup sequence Steve Longerbeam 2 siblings, 0 replies; 15+ messages in thread From: Jacopo Mondi @ 2018-07-10 18:36 UTC (permalink / raw) To: mchehab, laurent.pinchart, maxime.ripard, sam, jagan, festevam, pza, steve_longerbeam, hugues.fruchet, loic.poulain, daniel Cc: Jacopo Mondi, linux-media Rework the MIPI interface startup sequence with the following changes: - Remove MIPI bus initialization from the initial settings blob - At set_power(1) time power up MIPI Tx/Rx and set data and clock lanes in LP11 during 'sleep' and 'idle' with MIPI clock in non-continuous mode. - At s_stream time enable/disable the MIPI interface output. - Restore default settings at set_power(0) time. Before this commit the sensor MIPI interface was initialized with settings that require a start/stop sequence at power-up time in order to force lanes into LP11 state, as they were initialized in LP00 when in 'sleep mode', which is assumed to be the sensor manual definition for the D-PHY defined stop mode. The stream start/stop was performed by enabling disabling clock gating, and had the side effect to change the lanes sleep mode configuration when stream was stopped. Clock gating/ungating: - ret = ov5640_mod_reg(sensor, OV5640_REG_MIPI_CTRL00, BIT(5), - on ? 0 : BIT(5)); - if (ret) Set lanes in LP11 when in 'sleep mode': - ret = ov5640_write_reg(sensor, OV5640_REG_PAD_OUTPUT00, - on ? 0x00 : 0x70); This commit fixes an issue reported by Jagan Teki on i.MX6 platforms that prevents the host interface from powering up correctly: https://lkml.org/lkml/2018/6/1/38 It also improves MIPI capture operations stability on my testing platform where MIPI capture often failed and returned all-purple frames. fixes: f22996db44e2 ("media: ov5640: add support of DVP parallel interface") Reported-by: Jagan Teki <jagan@amarulasolutions.com> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- drivers/media/i2c/ov5640.c | 91 ++++++++++++++++++++++++++++++++++++---------- 1 file changed, 71 insertions(+), 20 deletions(-) diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c index 1ecbb7a..7bbd1d7 100644 --- a/drivers/media/i2c/ov5640.c +++ b/drivers/media/i2c/ov5640.c @@ -286,10 +286,10 @@ static const struct reg_value ov5640_init_setting_30fps_VGA[] = { {0x3a0d, 0x04, 0, 0}, {0x3a14, 0x03, 0, 0}, {0x3a15, 0xd8, 0, 0}, {0x4001, 0x02, 0, 0}, {0x4004, 0x02, 0, 0}, {0x3000, 0x00, 0, 0}, {0x3002, 0x1c, 0, 0}, {0x3004, 0xff, 0, 0}, {0x3006, 0xc3, 0, 0}, - {0x300e, 0x45, 0, 0}, {0x302e, 0x08, 0, 0}, {0x4300, 0x3f, 0, 0}, + {0x302e, 0x08, 0, 0}, {0x4300, 0x3f, 0, 0}, {0x501f, 0x00, 0, 0}, {0x4713, 0x03, 0, 0}, {0x4407, 0x04, 0, 0}, {0x440e, 0x00, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0}, - {0x4837, 0x0a, 0, 0}, {0x4800, 0x04, 0, 0}, {0x3824, 0x02, 0, 0}, + {0x4837, 0x0a, 0, 0}, {0x3824, 0x02, 0, 0}, {0x5000, 0xa7, 0, 0}, {0x5001, 0xa3, 0, 0}, {0x5180, 0xff, 0, 0}, {0x5181, 0xf2, 0, 0}, {0x5182, 0x00, 0, 0}, {0x5183, 0x14, 0, 0}, {0x5184, 0x25, 0, 0}, {0x5185, 0x24, 0, 0}, {0x5186, 0x09, 0, 0}, @@ -1102,12 +1102,18 @@ static int ov5640_set_stream_mipi(struct ov5640_dev *sensor, bool on) { int ret; - ret = ov5640_mod_reg(sensor, OV5640_REG_MIPI_CTRL00, BIT(5), - on ? 0 : BIT(5)); - if (ret) - return ret; - ret = ov5640_write_reg(sensor, OV5640_REG_PAD_OUTPUT00, - on ? 0x00 : 0x70); + /* + * Enable/disable the MIPI interface + * + * 0x300e = on ? 0x45 : 0x40 + * [7:5] = 001 : 2 data lanes mode + * [4] = 0 : Power up MIPI HS Tx + * [3] = 0 : Power up MIPI LS Rx + * [2] = 1/0 : MIPI interface enable/disable + * [1:0] = 01/00: FIXME: 'debug' + */ + ret = ov5640_write_reg(sensor, OV5640_REG_IO_MIPI_CTRL00, + on ? 0x45 : 0x40); if (ret) return ret; @@ -1786,23 +1792,68 @@ static int ov5640_set_power(struct ov5640_dev *sensor, bool on) if (ret) goto power_off; + /* We're done here for DVP bus, while CSI-2 needs setup. */ + if (sensor->ep.bus_type != V4L2_MBUS_CSI2) + return 0; + + /* + * Power up MIPI HS Tx and LS Rx; 2 data lanes mode + * + * 0x300e = 0x40 + * [7:5] = 001 : 2 data lanes mode + * [4] = 0 : Power up MIPI HS Tx + * [3] = 0 : Power up MIPI LS Rx + * [2] = 0 : MIPI interface disabled + */ + ret = ov5640_write_reg(sensor, + OV5640_REG_IO_MIPI_CTRL00, 0x40); + if (ret) + goto power_off; + + /* + * Gate clock and set LP11 in 'no packets mode' (idle) + * + * 0x4800 = 0x24 + * [5] = 1 : Gate clock when 'no packets' + * [2] = 1 : MIPI bus in LP11 when 'no packets' + */ + ret = ov5640_write_reg(sensor, + OV5640_REG_MIPI_CTRL00, 0x24); + if (ret) + goto power_off; + + /* + * Set data lanes and clock in LP11 when 'sleeping' + * + * 0x3019 = 0x70 + * [6] = 1 : MIPI data lane 2 in LP11 when 'sleeping' + * [5] = 1 : MIPI data lane 1 in LP11 when 'sleeping' + * [4] = 1 : MIPI clock lane in LP11 when 'sleeping' + */ + ret = ov5640_write_reg(sensor, + OV5640_REG_PAD_OUTPUT00, 0x70); + if (ret) + goto power_off; + + /* Give lanes some time to coax into LP11 state. */ + usleep_range(500, 1000); + + } else { if (sensor->ep.bus_type == V4L2_MBUS_CSI2) { - /* - * start streaming briefly followed by stream off in - * order to coax the clock lane into LP-11 state. - */ - ret = ov5640_set_stream_mipi(sensor, true); - if (ret) - goto power_off; - usleep_range(1000, 2000); - ret = ov5640_set_stream_mipi(sensor, false); - if (ret) - goto power_off; + /* Reset MIPI bus settings to their default values. */ + ov5640_write_reg(sensor, + OV5640_REG_IO_MIPI_CTRL00, 0x58); + ov5640_write_reg(sensor, + OV5640_REG_MIPI_CTRL00, 0x04); + ov5640_write_reg(sensor, + OV5640_REG_PAD_OUTPUT00, 0x00); } - return 0; + ov5640_set_power_off(sensor); } + return 0; + power_off: ov5640_set_power_off(sensor); return ret; -- 2.7.4 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 2/2] media: ov5640: Fix timings setup code 2018-07-10 18:36 [PATCH v2 0/2] media: i2c: ov5640: Re-work MIPI startup sequence Jacopo Mondi 2018-07-10 18:36 ` [PATCH v2 1/2] " Jacopo Mondi @ 2018-07-10 18:36 ` Jacopo Mondi 2018-07-10 21:10 ` [PATCH v2 0/2] media: i2c: ov5640: Re-work MIPI startup sequence Steve Longerbeam 2 siblings, 0 replies; 15+ messages in thread From: Jacopo Mondi @ 2018-07-10 18:36 UTC (permalink / raw) To: mchehab, laurent.pinchart, maxime.ripard, sam, jagan, festevam, pza, steve_longerbeam, hugues.fruchet, loic.poulain, daniel Cc: Jacopo Mondi, linux-media From: Samuel Bobrowicz <sam@elite-embedded.com> The current code, when changing the mode and changing the scaling or sampling parameters, will look at the horizontal and vertical total size, which, since 5999f381e023 ("media: ov5640: Add horizontal and vertical totals") has been moved from the static register initialization to after the mode change. That means that the values are no longer set up before the code retrieves them, which is obviously a bug. In addition, restore timings settings in the initial configuration register blob only, to have MIPI capture operations work again. Fixes: 5999f381e023 ("media: ov5640: Add horizontal and vertical totals") Signed-off-by: Samuel Bobrowicz <sam@elite-embedded.com> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> [re-introduce timing parameters in initial configuration blob] Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- Compared to Maxime's and Sam's original version, this one re-introduces timing configuration parameters in the init_settings configuration blob. On my testing platform this fixes MIPI capture operations, that with the original patch version applied was failing anyway. Thanks j --- drivers/media/i2c/ov5640.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c index 7bbd1d7..bbcb908 100644 --- a/drivers/media/i2c/ov5640.c +++ b/drivers/media/i2c/ov5640.c @@ -277,7 +277,9 @@ static const struct reg_value ov5640_init_setting_30fps_VGA[] = { {0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0}, {0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0}, {0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0}, - {0x3810, 0x00, 0, 0}, + {0x3808, 0x02, 0, 0}, {0x3809, 0x80, 0, 0}, {0x380a, 0x01, 0, 0}, + {0x380b, 0xe0, 0, 0}, {0x380c, 0x07, 0, 0}, {0x380d, 0x68, 0, 0}, + {0x380e, 0x03, 0, 0}, {0x380f, 0xd8, 0, 0}, {0x3810, 0x00, 0, 0}, {0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0}, {0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0}, {0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0}, @@ -1484,6 +1486,10 @@ static int ov5640_set_mode_exposure_calc(struct ov5640_dev *sensor, if (ret < 0) return ret; + ret = ov5640_set_timings(sensor, mode); + if (ret < 0) + return ret; + /* read capture VTS */ ret = ov5640_get_vts(sensor); if (ret < 0) @@ -1611,6 +1617,10 @@ static int ov5640_set_mode_direct(struct ov5640_dev *sensor, if (ret < 0) return ret; + ret = ov5640_set_timings(sensor, mode); + if (ret < 0) + return ret; + /* turn auto gain/exposure back on for direct mode */ ret = __v4l2_ctrl_s_ctrl(sensor->ctrls.auto_gain, 1); if (ret) @@ -1658,10 +1668,6 @@ static int ov5640_set_mode(struct ov5640_dev *sensor, if (ret < 0) return ret; - ret = ov5640_set_timings(sensor, mode); - if (ret < 0) - return ret; - ret = ov5640_set_binning(sensor, dn_mode != SCALING); if (ret < 0) return ret; -- 2.7.4 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 0/2] media: i2c: ov5640: Re-work MIPI startup sequence 2018-07-10 18:36 [PATCH v2 0/2] media: i2c: ov5640: Re-work MIPI startup sequence Jacopo Mondi 2018-07-10 18:36 ` [PATCH v2 1/2] " Jacopo Mondi 2018-07-10 18:36 ` [PATCH v2 2/2] media: ov5640: Fix timings setup code Jacopo Mondi @ 2018-07-10 21:10 ` Steve Longerbeam 2018-07-11 7:21 ` jacopo mondi 2 siblings, 1 reply; 15+ messages in thread From: Steve Longerbeam @ 2018-07-10 21:10 UTC (permalink / raw) To: Jacopo Mondi, mchehab, laurent.pinchart, maxime.ripard, sam, jagan, festevam, pza, hugues.fruchet, loic.poulain, daniel Cc: linux-media 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 > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 0/2] media: i2c: ov5640: Re-work MIPI startup sequence 2018-07-10 21:10 ` [PATCH v2 0/2] media: i2c: ov5640: Re-work MIPI startup sequence Steve Longerbeam @ 2018-07-11 7:21 ` jacopo mondi 2018-07-14 18:57 ` Steve Longerbeam 0 siblings, 1 reply; 15+ messages in thread From: jacopo mondi @ 2018-07-11 7:21 UTC (permalink / raw) To: Steve Longerbeam Cc: mchehab, laurent.pinchart, maxime.ripard, sam, jagan, festevam, pza, hugues.fruchet, loic.poulain, daniel, linux-media [-- Attachment #1: Type: text/plain, Size: 3105 bytes --] 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 > > > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 0/2] media: i2c: ov5640: Re-work MIPI startup sequence 2018-07-11 7:21 ` jacopo mondi @ 2018-07-14 18:57 ` Steve Longerbeam 2018-07-14 19:41 ` Steve Longerbeam 0 siblings, 1 reply; 15+ messages in thread From: Steve Longerbeam @ 2018-07-14 18:57 UTC (permalink / raw) To: jacopo mondi Cc: mchehab, laurent.pinchart, maxime.ripard, sam, jagan, festevam, pza, hugues.fruchet, loic.poulain, daniel, linux-media 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 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 >>> ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 0/2] media: i2c: ov5640: Re-work MIPI startup sequence 2018-07-14 18:57 ` Steve Longerbeam @ 2018-07-14 19:41 ` Steve Longerbeam 2018-07-14 20:02 ` Steve Longerbeam 0 siblings, 1 reply; 15+ messages in thread From: Steve Longerbeam @ 2018-07-14 19:41 UTC (permalink / raw) To: jacopo mondi Cc: mchehab, laurent.pinchart, maxime.ripard, sam, jagan, festevam, pza, hugues.fruchet, loic.poulain, daniel, linux-media 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 >>>> > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 0/2] media: i2c: ov5640: Re-work MIPI startup sequence 2018-07-14 19:41 ` Steve Longerbeam @ 2018-07-14 20:02 ` Steve Longerbeam 2018-07-16 8:29 ` jacopo mondi 0 siblings, 1 reply; 15+ messages in thread From: Steve Longerbeam @ 2018-07-14 20:02 UTC (permalink / raw) To: jacopo mondi Cc: mchehab, laurent.pinchart, maxime.ripard, sam, jagan, festevam, pza, hugues.fruchet, loic.poulain, daniel, linux-media 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. > 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 0/2] media: i2c: ov5640: Re-work MIPI startup sequence 2018-07-14 20:02 ` Steve Longerbeam @ 2018-07-16 8:29 ` jacopo mondi 2018-07-16 16:26 ` Steve Longerbeam 0 siblings, 1 reply; 15+ messages in thread From: jacopo mondi @ 2018-07-16 8:29 UTC (permalink / raw) To: Steve Longerbeam Cc: mchehab, laurent.pinchart, maxime.ripard, sam, jagan, festevam, pza, hugues.fruchet, loic.poulain, daniel, linux-media [-- 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 --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 0/2] media: i2c: ov5640: Re-work MIPI startup sequence 2018-07-16 8:29 ` jacopo mondi @ 2018-07-16 16:26 ` Steve Longerbeam 2018-08-14 15:35 ` jacopo mondi 0 siblings, 1 reply; 15+ messages in thread From: Steve Longerbeam @ 2018-07-16 16:26 UTC (permalink / raw) To: jacopo mondi Cc: mchehab, laurent.pinchart, maxime.ripard, sam, jagan, festevam, pza, hugues.fruchet, loic.poulain, daniel, linux-media 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 = 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? 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 >> >> ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 0/2] media: i2c: ov5640: Re-work MIPI startup sequence 2018-07-16 16:26 ` Steve Longerbeam @ 2018-08-14 15:35 ` jacopo mondi 2018-08-14 16:51 ` Steve Longerbeam 0 siblings, 1 reply; 15+ messages in thread From: jacopo mondi @ 2018-08-14 15:35 UTC (permalink / raw) To: Steve Longerbeam Cc: mchehab, laurent.pinchart, maxime.ripard, sam, jagan, festevam, pza, hugues.fruchet, loic.poulain, daniel, linux-media [-- Attachment #1: Type: text/plain, Size: 4874 bytes --] 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 = 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? > > 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 > >> > >> > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 0/2] media: i2c: ov5640: Re-work MIPI startup sequence 2018-08-14 15:35 ` jacopo mondi @ 2018-08-14 16:51 ` Steve Longerbeam 2018-08-14 17:38 ` jacopo mondi 0 siblings, 1 reply; 15+ messages in thread From: Steve Longerbeam @ 2018-08-14 16:51 UTC (permalink / raw) To: jacopo mondi, Steve Longerbeam Cc: mchehab, laurent.pinchart, maxime.ripard, sam, jagan, festevam, pza, hugues.fruchet, loic.poulain, daniel, linux-media Hi Jacopo, On 08/14/2018 08:35 AM, jacopo mondi wrote: > Hi Steve, > sorry for resurecting this. > > <snip> >>> 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 0/2] media: i2c: ov5640: Re-work MIPI startup sequence 2018-08-14 16:51 ` Steve Longerbeam @ 2018-08-14 17:38 ` jacopo mondi 2018-08-14 23:53 ` Steve Longerbeam 0 siblings, 1 reply; 15+ messages in thread From: jacopo mondi @ 2018-08-14 17:38 UTC (permalink / raw) To: Steve Longerbeam Cc: mchehab, laurent.pinchart, maxime.ripard, sam, jagan, festevam, pza, hugues.fruchet, loic.poulain, daniel, linux-media [-- Attachment #1: Type: text/plain, Size: 2127 bytes --] 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. > > > ><snip> > >>>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 > > > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 0/2] media: i2c: ov5640: Re-work MIPI startup sequence 2018-08-14 17:38 ` jacopo mondi @ 2018-08-14 23:53 ` Steve Longerbeam 2018-08-15 9:00 ` jacopo mondi 0 siblings, 1 reply; 15+ messages in thread From: Steve Longerbeam @ 2018-08-14 23:53 UTC (permalink / raw) To: jacopo mondi Cc: mchehab, laurent.pinchart, maxime.ripard, sam, jagan, festevam, pza, hugues.fruchet, loic.poulain, daniel, linux-media 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. >>> >>> <snip> >>>>> 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 <slongerbeam@gmail.com> on i.MX6q SabreSD with MIPI CSI-2 OV5640 module to whichever ov5640 patches are appropriate. Steve ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 0/2] media: i2c: ov5640: Re-work MIPI startup sequence 2018-08-14 23:53 ` Steve Longerbeam @ 2018-08-15 9:00 ` jacopo mondi 0 siblings, 0 replies; 15+ messages in thread From: jacopo mondi @ 2018-08-15 9:00 UTC (permalink / raw) To: Steve Longerbeam Cc: mchehab, laurent.pinchart, maxime.ripard, sam, jagan, festevam, pza, hugues.fruchet, loic.poulain, daniel, linux-media [-- Attachment #1: Type: text/plain, Size: 3096 bytes --] 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. > >>> > >>><snip> > >>>>>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 <slongerbeam@gmail.com> > 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 > > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2018-08-15 11:52 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-07-10 18:36 [PATCH v2 0/2] media: i2c: ov5640: Re-work MIPI startup sequence Jacopo Mondi 2018-07-10 18:36 ` [PATCH v2 1/2] " Jacopo Mondi 2018-07-10 18:36 ` [PATCH v2 2/2] media: ov5640: Fix timings setup code Jacopo Mondi 2018-07-10 21:10 ` [PATCH v2 0/2] media: i2c: ov5640: Re-work MIPI startup sequence Steve Longerbeam 2018-07-11 7:21 ` jacopo mondi 2018-07-14 18:57 ` Steve Longerbeam 2018-07-14 19:41 ` Steve Longerbeam 2018-07-14 20:02 ` Steve Longerbeam 2018-07-16 8:29 ` jacopo mondi 2018-07-16 16:26 ` Steve Longerbeam 2018-08-14 15:35 ` jacopo mondi 2018-08-14 16:51 ` Steve Longerbeam 2018-08-14 17:38 ` jacopo mondi 2018-08-14 23:53 ` Steve Longerbeam 2018-08-15 9:00 ` jacopo mondi
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.