lkml.org 
[lkml]   [2021]   [Dec]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH v4 1/6] media: i2c: ov5640: Remain in power down for DVP mode unless streaming
Date
On 6/29/21 1:47 PM, Eugen Hristev - M18282 wrote:
> On 9/9/20 7:16 PM, Hugues FRUCHET wrote:
>> Hi Prabhakar,
>>
>> As discussed separately I would prefer to better understand issue before
>> going to this patch.
>> Nevertheless I have some remarks in code in case we'll need it at the end.
>>
>> On 9/4/20 10:18 PM, Lad Prabhakar wrote:
>>> Keep the sensor in software power down mode and wake up only in
>>> ov5640_set_stream_dvp() callback.
>>>
>>> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>>> Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
>>> Tested-by: Jacopo Mondi <jacopo@jmondi.org>
>>> ---
>>> drivers/media/i2c/ov5640.c | 19 ++++++++++++++++---
>>> 1 file changed, 16 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
>>> index 2fe4a7ac0592..880fde73a030 100644
>>> --- a/drivers/media/i2c/ov5640.c
>>> +++ b/drivers/media/i2c/ov5640.c
>>> @@ -34,6 +34,8 @@
>>> #define OV5640_REG_SYS_RESET02 0x3002
>>> #define OV5640_REG_SYS_CLOCK_ENABLE02 0x3006
>>> #define OV5640_REG_SYS_CTRL0 0x3008
>>> +#define OV5640_REG_SYS_CTRL0_SW_PWDN 0x42
>>> +#define OV5640_REG_SYS_CTRL0_SW_PWUP 0x02
>>
>> For the time being this section was only referring to registers
>> addresses and bit details was explained into a comment right before
>> affectation, see OV5640_REG_IO_MIPI_CTRL00 for example.
>>
>>> #define OV5640_REG_CHIP_ID 0x300a
>>> #define OV5640_REG_IO_MIPI_CTRL00 0x300e
>>> #define OV5640_REG_PAD_OUTPUT_ENABLE01 0x3017
>>> @@ -1120,6 +1122,12 @@ static int ov5640_load_regs(struct ov5640_dev *sensor,
>>> val = regs->val;
>>> mask = regs->mask;
>>>
>>> + /* remain in power down mode for DVP */
>>> + if (regs->reg_addr == OV5640_REG_SYS_CTRL0 &&
>>> + val == OV5640_REG_SYS_CTRL0_SW_PWUP &&
>>> + sensor->ep.bus_type != V4L2_MBUS_CSI2_DPHY)
>>> + continue;
>>> +
>>
>> I understand that more or less register OV5640_REG_SYS_CTRL0 (0x3008)
>> has been partially removed from big init sequence: for power-up part,
>> but power-dwn remains at very beginning of sequence.
>> We should completely remove 0x3008 from init sequence, including
>> power-dwn, and introduce a new function ov5640_sw_powerdown(on/off) that
>> should be called at the right place instead.
>>
>>
>>> if (mask)
>>> ret = ov5640_mod_reg(sensor, reg_addr, mask, val);
>>> else
>>> @@ -1297,9 +1305,14 @@ static int ov5640_set_stream_dvp(struct ov5640_dev *sensor, bool on)
>>> * PAD OUTPUT ENABLE 02
>>> * - [7:2]: D[5:0] output enable
>>> */
>>> - return ov5640_write_reg(sensor,
>>> - OV5640_REG_PAD_OUTPUT_ENABLE02,
>>> - on ? 0xfc : 0);
>>> + ret = ov5640_write_reg(sensor, OV5640_REG_PAD_OUTPUT_ENABLE02,
>>> + on ? 0xfc : 0);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + return ov5640_write_reg(sensor, OV5640_REG_SYS_CTRL0, on ?
>>> + OV5640_REG_SYS_CTRL0_SW_PWUP :
>>> + OV5640_REG_SYS_CTRL0_SW_PWDN);
>>> }
>>>
>>> static int ov5640_set_stream_mipi(struct ov5640_dev *sensor, bool on)
>>>
>>
>>
>> BR,
>> Hugues.
>>
>
>
> Hello everyone,
>
> When I updated driver in my tree with this patch, I noticed that my
> setup using the ATMEL isc platform + ov5640 in parallel bus mode stopped
> working.
>
> It looks like the culprit is this patch.
> I am not sure which is the best way to fix it.
> It looks like initially things work fine when probing the driver, but
> when we want to start streaming at a later point, the register SYS_CTRL0
> cannot be written anymore.
> Here is an output of the log:
>
> --- Opening /dev/video0...
> Trying source module v4l2...
> /dev/video0 opened.
> No input was specified, using the first.
> ov5640 1-003c: ov5640_write_reg: error: reg=3008, val=2
> atmel-sama5d2-isc f0008000.isc: stream on failed in subdev -121
> Error starting stream.
> VIDIOC_STREAMON: Remote I/O error
> Unable to use mmap. Using read instead.
> Unable to use read.
>
> I am using a simple application to read from /dev/video0 (which is
> registered by the atmel-isc once the sensor probed)
>
> I have a feeling that something is wrong related to power, but I cannot
> tell for sure.
>
> Reverting this patch makes it work fine again.
>
> Help is appreciated,
> Thanks,
> Eugen
>


Gentle ping.

I would like to send a revert patch if nobody cares about this
driver/patch except me.

Thanks,
Eugen
\
 
 \ /
  Last update: 2021-12-21 08:39    [W:0.096 / U:0.072 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site