Re: [PATCH v5 00/27] media: ov5640: Rework the clock tree programming for MIPI

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

 



Hi Jacopo,

On 3/23/22 10:50 AM, Jacopo Mondi wrote:
Hi Tomi thanks for testing

On Wed, Mar 23, 2022 at 10:51:04AM +0200, Tomi Valkeinen wrote:
Hi Jacopo,

On 24/02/2022 11:42, Jacopo Mondi wrote:
v1:
https://patchwork.linuxtv.org/project/linux-media/list/?series=7249
v2:
https://patchwork.linuxtv.org/project/linux-media/list/?series=7311
v3:
https://patchwork.linuxtv.org/project/linux-media/list/?series=7385
v4:
https://patchwork.linuxtv.org/project/linux-media/list/?series=7389

A branch for testing based on the most recent media-master is available at
https://git.sr.ht/~jmondi_/linux #jmondi/media-master/ov5640-v5

I tested these with DRA76 EVM & CAL, using CAL's legacy non-MC mode. It
doesn't work. I think there are two problems:

- CAL uses mbus codes like MEDIA_BUS_FMT_UYVY8_2X8 for CSI-2, not 1X16.
OV5640 used to support 2X8, but now it doesn't.

- OV5640 sets the default code to MEDIA_BUS_FMT_UYVY8_2X8, even for CSI-2
where it doesn't support MEDIA_BUS_FMT_UYVY8_2X8.

This might be worth an additional patch that decides what default
format to use based on the bus type.


I'd like to just change CAL and drop the 2X8 support and instead use 1X16,
but then any sensor that uses 2X8 would work. So I guess I need to change
the code to support both.

Anyway, both of those issues might also surface on other platforms, as
ov5640 behavior has changed.


I am facing the same "2X8" compatibility problem on ST platforms:
- st-mipid02 CSI-2 to parallel bridge [1] must be enhanced to support 1X16 formats - dcmi controller [2] must also be enhanced to support 1X16 and extra code to support 1X16 input to 2X8 output (as we only have as input the V4L2 format, not the mediabus one)
=> with current code, support with OV5640 is broken.

I feel that your proposal to let OV5640 accept 2X8 then silently switch to 1X16 can do the job without breaking dcmi/bridge support but need further testing to confirm.

Appart from that I don't really understand the logic behind naming "1X16" for CSI-2 serial formats, if "2X8" means 2 bytes to send one pixel, I would expect that "1X16" means 1 word to send one pixel (16bits wide bus), so how to differentiate 16 bits // code from CSI-2 code ?

For the time being I have reverted this commit in order to test the other topics of this patchset.

[1] drivers/media/i2c/st-mipid02.c
[2] drivers/media/platform/stm32/stm32-dcmi.c


I'm afraid sooner or later this should have happened ?

I think CSI-2 receivers should be updated, but I share your concerns
about breaking other platforms.

On one side we shouldn't be breaking userspace and this change might
break some assumptions in users' pipeline configuration scripts and
could prevent drivers that used to work together from being
compatible at all.

On the other side we would never be able to change anything at all if
such a change is expected to happen atomically on all platforms and
sensors.

As the change is so trivial I guess it's fair to expect users of
bridge drivers not compatible with 1X16 to fix them, but I cannot tell
if it's an acceptable policy or not.

As Sakari suggested we could also move all CSI-2 transmitters to use 1X16
and have receivers adjust as soon as someone detects a breakage.

I can revert the change that restricts the enumerated format to the
currently in use bus type[1] if desired, but I would prefer receivers
to adjust when needed. Is this acceptable ?

Thanks
   j

[1] "media: ov5640: Split DVP and CSI-2 formats

  Tomi



Best regards,
Hugues.



[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