Re: [PATCH] media: ov5640: Enable MIPI interface in ov5640_set_power_mipi()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 8/2/23 15:13, Jacopo Mondi wrote:
Hi Marek

Hi,

On Tue, Jul 25, 2023 at 12:35:00PM +0200, Marek Vasut wrote:
On 7/25/23 12:04, Jacopo Mondi wrote:
Hi Marek

Hi,

On Tue, Jul 25, 2023 at 11:41:18AM +0200, Marek Vasut wrote:
On 7/25/23 11:04, Jacopo Mondi wrote:
Hi Marek

Hi,

On Tue, Jul 25, 2023 at 12:22:10AM +0200, Marek Vasut wrote:
Set OV5640_REG_IO_MIPI_CTRL00 bit 2 to 1 instead of 0, since 1 means
MIPI CSI2 interface, while 0 means CPI parallel interface.

In the ov5640_set_power_mipi() the interface should obviously be set
to MIPI CSI2 since this functions is used to power up the sensor when
operated in MIPI CSI2 mode. The sensor should not be in CPI mode in
that case.

Does this actually help fixing your 'first frame corrupted issue' ?

Yes it does.


Do you think it's worth mentioning it in the commit message ?

Done in V2

I think the logic here was to power up the interface here  in
ov5640_set_power_mipi() with the CSI-2 interface disabled to enter
LP-11 mode (something some receivers like the imx6 one are
particularly sensible to).

Per OV5640 datasheet.

Register 0x300e bit 2 selects sensor interface mode between MIPI CSI2 and
CPI (parallel), it has nothing to do with LP modes .

Register 0x3019 bits [6:4] control LP00/LP11 mode on CSI2 lines.

Then at stream time the CSI-2 interface is actually enabled in
ov5640_set_stream_mipi() just before streaming is started.

Also the register documentation is very confusing and as reported in
ov5640_set_stream_mipi() it is also probably wrong (at least in the
datasheet version I have).

I would be particularly cautious in touching this sequence as it has
been validated to work with multiple receivers. Of course if it
actually fixes an issue for you it should be taken in, but ideally, as
this sensor is still used in a large number of evaluation boards, it
should be validated by other consumers of this driver (st, imx,
microchip and rensas to name a few).

Ok, I've been able to test on i.MX6Q which I was concerned for because
of its sensitivness to LP-11 detection.

Let alone that imx6 with ov5640 is broken on mainline because of
unrelated reasons [1] I've been able to confirm that the sensor still
works on this platform

Reviewed-by: Jacopo Mondi <jacopo.mondi@xxxxxxxxxxxxxxxx>
[Test on imx6q]
Tested-by: Jacopo Mondi <jacopo.mondi@xxxxxxxxxxxxxxxx>

Can you confirm you have tested with iMX8MP as well ?

I tested this great sensor on MN/MP/STM32MP15xx

[...]



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux