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

Message ID 20220224094313.233347-1-jacopo@jmondi.org (mailing list archive)
Headers
Series media: ov5640: Rework the clock tree programming for MIPI |

Message

Jacopo Mondi Feb. 24, 2022, 9:42 a.m. UTC
  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

v5 (Sakari):
- Stay strictly in 80 cols
- use clamp_t to avoid explicit cast
- use ov5640_timings() where possible

v4:
- Very minor update. Added tags and reworked enum_mbus_format as suggested
  by Laurent.

v3:
The series has now grown by 4 patches and the driver is now even larger
being the formats and the timings for DVP and CSI-2 defined separately.

Tested in CSI-2 mode with UYVY, RGB565, SBGGR and RGB24 in all supported modes.

Tested format and sizes enumeration with the new formats definition.

Tested frame rate handling:

	vblank = ( duration msec * pixe_rate MHz / htot - height)

  640x480 YUYV 15FPS (default 30 FPS)

	duration = 666666 msec
	pixel_rate = 48 Mhz
	htot = 1600
	vtot = 1999
	vblank = vtot - height = 1519

	$ v4l2-ctl -d /dev/v4l-subdev4 -c 0x009e0901=1519
	$ yavta -f YUYV -s 640x480 -c100 --skip 7 /dev/video0
	...
	10 (2) [-] any 11 614400 B 2189.317617 2189.317629 15.244 fps ts mono/EoF
	11 (3) [-] any 12 614400 B 2189.383212 2189.383224 15.245 fps ts mono/EoF
	12 (4) [-] any 13 614400 B 2189.448810 2189.448821 15.244 fps ts mono/EoF
	13 (5) [-] any 14 614400 B 2189.514405 2189.514417 15.245 fps ts mono/EoF
	14 (6) [-] any 15 614400 B 2189.580002 2189.580015 15.245 fps ts mono/EoF
	..

  2592x1944 YUVV 15 FPS (default)
	$ yavta -f YUYV -s 2592x1944 -c100 --skip 7 /dev/video0
	...
	6 (6) [-] any 7 10077696 B 2438.377592 2438.377605 15.009 fps ts mono/EoF
	7 (7) [-] any 8 10077696 B 2438.444219 2438.444233 15.009 fps ts mono/EoF
	8 (0) [-] any 9 10077696 B 2438.510846 2438.510860 15.009 fps ts mono/EoF
	9 (1) [-] any 10 10077696 B 2438.577474 2438.577488 15.009 fps ts mono/EoF
	10 (2) [-] any 11 10077696 B 2438.644101 2438.644116 15.009 fps ts mono/EoF
	11 (3) [-] any 12 10077696 B 2438.710727 2438.710740 15.009 fps ts mono/EoF
	12 (4) [-] any 13 10077696 B 2438.777358 2438.777370 15.008 fps ts mono/EoF
	13 (5) [-] any 14 10077696 B 2438.843984 2438.843998 15.009 fps ts mono/EoF
	14 (6) [-] any 15 10077696 B 2438.910611 2438.910623 15.009 fps ts mono/EoF
	15 (7) [-] any 16 10077696 B 2438.977238 2438.977252 15.009 fps ts mono/EoF
	16 (0) [-] any 17 10077696 B 2439.043865 2439.043877 15.009 fps ts mono
	...


To enable higher FPS the LINK_FREQ control should be made writable to increase
the pixel rate

  640x480 YUYV 60 FPS (pixel_rate = 96 Mhz)

	$ yavta -f YUYV -s 640x480 -c100 --skip 7 /dev/video0
 	...
	9 (1) [-] any 10 614400 B 57.098649 57.098667 59.995 fps ts mono/EoF
	10 (2) [-] any 11 614400 B 57.115314 57.115332 60.006 fps ts mono/EoF
	11 (3) [-] any 12 614400 B 57.131978 57.131994 60.010 fps ts mono/EoF
	12 (4) [-] any 13 614400 B 57.148645 57.148664 59.999 fps ts mono/EoF
	13 (5) [-] any 14 614400 B 57.165310 57.165328 60.006 fps ts mono/EoF
	14 (6) [-] any 15 614400 B 57.181977 57.181996 59.999 fps ts mono/EoF
	15 (7) [-] any 16 614400 B 57.198642 57.198660 60.006 fps ts mono/EoF

Changelog:

v2->v3:

- Eugen (thanks) reported regression in DVP mode :(
  To maintain the DVP timings un-changed in this version the mode definition now
  looks like

		/* 640x480 */
		.id		= OV5640_MODE_VGA_640_480,
		.dn_mode	= SUBSAMPLING,
		.pixel_rate	= OV5640_PIXEL_RATE_48M,
		.width		= 640,
		.height		= 480,
		.dvp_timings = {
			.analog_crop = {
				.left	= 0,
				.top	= 4,
				.width	= 2624,
				.height	= 1944,
			},
			.crop = {
				.left	= 16,
				.top	= 6,
				.width	= 640,
				.height	= 480,
			},
			.htot		= 1896,
			.vblank_def	= 600,
			.max_fps	= OV5640_60_FPS
		},
		.csi2_timings = {
			.analog_crop = {
				/* Feed the full valid pixel array to the ISP. */
				.left	= OV5640_PIXEL_ARRAY_LEFT,
				.top	= OV5640_PIXEL_ARRAY_TOP,
				.width	= OV5640_PIXEL_ARRAY_WIDTH,
				.height	= OV5640_PIXEL_ARRAY_HEIGHT,
			},
			.crop = {
				/* Maintain a minimum digital crop processing margins. */
				.left	= 2,
				.top	= 4,
				.width	= 640,
				.height	= 480,
			},
			.htot		= 1600,
			.vblank_def	= 520,
		},
		.reg_data	= ov5640_setting_low_res,
		.reg_data_size	= ARRAY_SIZE(ov5640_setting_low_res),

  with a .dvp_timings and a .csi2_timings members to separate the two.
  Is it nice ? No it's not, but it should help maintaining DVP users happy.

  Eugen: if you are willing to run another test round to confirm if this version
  does not regress DVP it would be great :)

- Split image formats between CSI-2 and DVP
- Remove RGB888 as per the CSIS discussion with Laurent
- Removed register tables for modes < 720 as they're all equal
- Minor fixes on Laurent's comments
- Add Adam's tag

v1 -> v2:
- rework the modes definition to process the full pixel array
- rework get_selection to report the correct BOUND and DEFAULT targets
- implement init_cfg
- minor style changes as suggested by Laurent
- test with 1 data lane

Jacopo Mondi (27):
  media: ov5640: Add pixel rate to modes
  media: ov5604: Re-arrange modes definition
  media: ov5640: Add ov5640_is_csi2() function
  media: ov5640: Associate bpp with formats
  media: ov5640: Add LINK_FREQ control
  media: ov5640: Update pixel_rate and link_freq
  media: ov5640: Rework CSI-2 clock tree
  media: ov5640: Rework timings programming
  media: ov5640: Fix 720x480 in RGB888 mode
  media: ov5640: Split DVP and CSI-2 timings
  media: ov5640: Provide timings accessor
  media: ov5640: Re-sort per-mode register tables
  media: ov5640: Remove duplicated mode settings
  media: ov5640: Remove ov5640_mode_init_data
  media: ov5640: Add HBLANK control
  media: ov5640: Add VBLANK control
  media: ov5640: Change CSI-2 timings to comply with FPS
  media: ov5640: Implement init_cfg
  media: ov5640: Implement get_selection
  media: ov5640: Limit frame_interval to DVP mode only
  media: ov5640: Register device properties
  media: ov5640: Add RGB565_1X16 format
  media: ov5640: Add BGR888 format
  media: ov5640: Restrict sizes to mbus code
  media: ov5640: Adjust format to bpp in s_fmt
  media: ov5640: Split DVP and CSI-2 formats
  media: ov5640: Move format mux config in format

 drivers/media/i2c/ov5640.c | 1615 ++++++++++++++++++++++++++----------
 1 file changed, 1160 insertions(+), 455 deletions(-)

--
2.35.0
  

Comments

Sakari Ailus March 4, 2022, 12:09 a.m. UTC | #1
Hello folks,

On Thu, Feb 24, 2022 at 10:42:46AM +0100, Jacopo Mondi wrote:
> 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

The set has been around for quite some time without tangible functional
changes, please do let me know if you have concerns merging it.

Thanks.
  
Eugen Hristev March 4, 2022, 8:41 a.m. UTC | #2
On 3/4/22 2:09 AM, Sakari Ailus wrote:

> Hello folks,
> 
> On Thu, Feb 24, 2022 at 10:42:46AM +0100, Jacopo Mondi wrote:
>> 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
> 
> The set has been around for quite some time without tangible functional
> changes, please do let me know if you have concerns merging it.
> 
> Thanks.
> 
> --
> Sakari Ailus
> 


Hello Sakari, Jacopo,

I retested this series and on my side, the regression in version 3 is 
gone, I can capture images fine now YUYV@1280x720 .
I also retested 1920x1080 in RAW, and it works fine.

You can add my :
Tested-by: Eugen Hristev <eugen.hristev@microchip.com>

I am not sure if you have to add it to the whole series, because I only 
tested parallel interface with atmel ISC controller on sama5d2_xplained.
Up to your judgment to add it to the patches that impact the parallel 
interface.

Thanks again Jacopo for improving the support for this sensor.

Eugen
  
Jacopo Mondi March 4, 2022, 8:45 a.m. UTC | #3
Hi Eugen

On Fri, Mar 04, 2022 at 08:41:23AM +0000, Eugen.Hristev@microchip.com wrote:
> On 3/4/22 2:09 AM, Sakari Ailus wrote:
>
> > Hello folks,
> >
> > On Thu, Feb 24, 2022 at 10:42:46AM +0100, Jacopo Mondi wrote:
> >> 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
> >
> > The set has been around for quite some time without tangible functional
> > changes, please do let me know if you have concerns merging it.
> >
> > Thanks.
> >
> > --
> > Sakari Ailus
> >
>
>
> Hello Sakari, Jacopo,
>
> I retested this series and on my side, the regression in version 3 is
> gone, I can capture images fine now YUYV@1280x720 .
> I also retested 1920x1080 in RAW, and it works fine.
>

fiuuuu, I can breath again now :)

> You can add my :
> Tested-by: Eugen Hristev <eugen.hristev@microchip.com>
>
> I am not sure if you have to add it to the whole series, because I only
> tested parallel interface with atmel ISC controller on sama5d2_xplained.
> Up to your judgment to add it to the patches that impact the parallel
> interface.
>
> Thanks again Jacopo for improving the support for this sensor.

Thank you for testing with parallel mode, I would have made lot of
people unhappy otherwise!

>
> Eugen
  
Sakari Ailus March 4, 2022, 8:52 a.m. UTC | #4
On Fri, Mar 04, 2022 at 09:45:29AM +0100, Jacopo Mondi wrote:
> > Thanks again Jacopo for improving the support for this sensor.
> 
> Thank you for testing with parallel mode, I would have made lot of
> people unhappy otherwise!

Thanks, guys!

Applied to my tree.
  
Tomi Valkeinen March 23, 2022, 8:51 a.m. UTC | #5
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.

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.

  Tomi
  
Sakari Ailus March 23, 2022, 8:54 a.m. UTC | #6
Moi,

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

How many drivers there are using 2X8 formats on CSI-2? I remember there has
been probably few, and it should be easy to fix them.

It's another question whether the 2X8 format should be kept for
compatibility. Perhaps the driver could allow setting it, but should then
return 1X16 when queried.
  
Laurent Pinchart March 23, 2022, 8:59 a.m. UTC | #7
Hi Tomi,

On Wed, Mar 23, 2022 at 10:51:04AM +0200, Tomi Valkeinen wrote:
> 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.
> 
> 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.

We really need to phase out 2X8 for CSI-2. Can you add a warning in that
case if you support both ?

> Anyway, both of those issues might also surface on other platforms, as 
> ov5640 behavior has changed.
  
Jacopo Mondi March 23, 2022, 9:50 a.m. UTC | #8
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'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
  
Laurent Pinchart March 23, 2022, 10:41 a.m. UTC | #9
On Wed, Mar 23, 2022 at 10:50:19AM +0100, 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'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 ?

That would be my preference too. How about implementing Sakari's
suggestion of turning the 2X8 formats into 1X16 in .set_fmt() for CSI-2
? That way we'll minimize any risk of breakage for userspace. Host-side
drivers that use the OV5640 will still need to be converted from 2X8 to
1X16, but that's in-kernel only and should be manageable.

> [1] "media: ov5640: Split DVP and CSI-2 formats
  
Jacopo Mondi March 28, 2022, 2:57 p.m. UTC | #10
Hi

On Wed, Mar 23, 2022 at 12:41:47PM +0200, Laurent Pinchart wrote:
> On Wed, Mar 23, 2022 at 10:50:19AM +0100, 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'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 ?
>
> That would be my preference too. How about implementing Sakari's
> suggestion of turning the 2X8 formats into 1X16 in .set_fmt() for CSI-2

It's quite a tedious job, as for each driver that uses 2X8, if one
doesn't know the device by name, searching online or in the driver for
hints about what interface it uses (or discerning it from the
manufacturing year) takes quite some time.

I tried at the best of my knowledge, focusing on image sensor and
ignoring video converters as most of them have an analogue output, for
which I'm not sure what version is more opportune.

The only outlier I have seen is ov5645, which is a CSI-2 sensor but
uses 2X8. I've fixed that and will send a patch.

> ? That way we'll minimize any risk of breakage for userspace. Host-side

As the format will be adjusted, if drivers do not mess-up and select
something completely different, we should be safer yes. It's a
best-effort approach though.

> drivers that use the OV5640 will still need to be converted from 2X8 to
> 1X16, but that's in-kernel only and should be manageable.
>

Agreed, I don't think fixing all receivers should block this series

(speaking of which: Sakari you have collected the series in one of
your branches, but it is not part of the media pull request for
v5.18-rc1. Should I expect this to be post-poned to v5.19 ?)

Thanks
  j
> > [1] "media: ov5640: Split DVP and CSI-2 formats
>
> --
> Regards,
>
> Laurent Pinchart
  
Sakari Ailus March 28, 2022, 8:50 p.m. UTC | #11
Hi Jacopo,

On Mon, Mar 28, 2022 at 04:57:28PM +0200, Jacopo Mondi wrote:
> (speaking of which: Sakari you have collected the series in one of
> your branches, but it is not part of the media pull request for
> v5.18-rc1. Should I expect this to be post-poned to v5.19 ?)

Yes.
  
Hugues FRUCHET April 7, 2022, 4:24 p.m. UTC | #12
Hi Jacopo,

Many thanks for this huge work !

I have tested the serie on ST platform setup using OV5640 CSI-2.
I have not yet tested the OV5640 parallel mode but will do also.

Find below the main feedback, I have replied with more details on each 
related patches:
1) "2X8" mediabus code support broken, I have reverted the patch to 
continue testing
2) frame interval support dropped in flavour to vblank control: I have 
proposed a patch to support both
3) some resolutions/framerate not supported (MAX_VTS to increase)
4) JPEG framerate is divided by 2 with 1280x720@15, 1280x720@30, 
1920x1080@15
=> I have not found a relevant patch to overcome this, seems linked to 
the OV5640_LINK_RATE_MAX limit (mipi_div).

RAW8 not tested but I can also (on my side, this is functional only for 
resolutions >= 720p).

Here is the summary of resolutions/format tested with this serie and my 
patches on top:


*  QCIF 176x144 RGB565 15fps => OK, got 15
*  QCIF 176x144 YUYV 15fps => OK, got 15
*  QCIF 176x144 JPEG 15fps => OK, got 15
*  QCIF 176x144 RGB565 30fps => OK, got 30
*  QCIF 176x144 YUYV 30fps => OK, got 30
*  QCIF 176x144 JPEG 30fps => OK, got 30
*  QVGA 320x240 RGB565 15fps => OK, got 15
*  QVGA 320x240 YUYV 15fps => OK, got 15
*  QVGA 320x240 JPEG 15fps => OK, got 15
*  QVGA 320x240 RGB565 30fps => OK, got 30
*  QVGA 320x240 YUYV 30fps => OK, got 30
*  QVGA 320x240 JPEG 30fps => OK, got 30
*  VGA 640x480 RGB565 15fps => OK, got 15
*  VGA 640x480 YUYV 15fps => OK, got 15
*  VGA 640x480 JPEG 15fps => OK, got 15
*  VGA 640x480 RGB565 30fps => OK, got 30
*  VGA 640x480 YUYV 30fps => OK, got 30
*  VGA 640x480 JPEG 30fps => OK, got 30
*  480p 720x480 RGB565 15fps => OK, got 15
*  480p 720x480 YUYV 15fps => OK, got 15
*  480p 720x480 JPEG 15fps => OK, got 15
*  480p 720x480 RGB565 30fps => OK, got 30
*  480p 720x480 YUYV 30fps => OK, got 30
*  480p 720x480 JPEG 30fps => OK, got 30
*  XGA 1024x768 RGB565 15fps => OK, got 15
*  XGA 1024x768 YUYV 15fps => OK, got 15
*  XGA 1024x768 JPEG 15fps => OK, got 15
*  XGA 1024x768 RGB565 30fps => OK, got 30
*  XGA 1024x768 YUYV 30fps => OK, got 30
*  XGA 1024x768 JPEG 30fps => OK, got 30
*  720p 1280x720 RGB565 15fps => OK, got 15
*  720p 1280x720 YUYV 15fps => OK, got 15
*  720p 1280x720 JPEG 15fps => KO: got 7
===============================^^
[10917.171528] ov5640 1-003c: rate=62000000, freq=248000000, htot=1600, 
height=720, vblank=1863

*  720p 1280x720 RGB565 30fps => OK, got 30
*  720p 1280x720 YUYV 30fps => OK, got 30
*  720p 1280x720 JPEG 30fps => KO: got 15
===============================^^
[10921.317180] ov5640 1-003c: rate=62000000, freq=248000000, htot=1600, 
height=720, vblank=560

*  HD 1920x1080 RGB565 15fps => OK, got 15
*  HD 1920x1080 YUYV 15fps => OK, got 15
*  HD 1920x1080 JPEG 15fps => KO: got 7
===============================^^
[10925.810657] ov5640 1-003c: rate=74000000, freq=296000000, htot=2234, 
height=1080, vblank=1128

*  5Mp 2592x1944 RGB565 15fps => OK, got 15
*  5Mp 2592x1944 YUYV 15fps => OK, got 15
*  5Mp 2592x1944 JPEG 15fps => OK, got 15


Best regards,
Hugues.


On 2/24/22 10:42 AM, 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
> 
> v5 (Sakari):
> - Stay strictly in 80 cols
> - use clamp_t to avoid explicit cast
> - use ov5640_timings() where possible
> 
> v4:
> - Very minor update. Added tags and reworked enum_mbus_format as suggested
>    by Laurent.
> 
> v3:
> The series has now grown by 4 patches and the driver is now even larger
> being the formats and the timings for DVP and CSI-2 defined separately.
> 
> Tested in CSI-2 mode with UYVY, RGB565, SBGGR and RGB24 in all supported modes.
> 
> Tested format and sizes enumeration with the new formats definition.
> 
> Tested frame rate handling:
> 
> 	vblank = ( duration msec * pixe_rate MHz / htot - height)
> 
>    640x480 YUYV 15FPS (default 30 FPS)
> 
> 	duration = 666666 msec
> 	pixel_rate = 48 Mhz
> 	htot = 1600
> 	vtot = 1999
> 	vblank = vtot - height = 1519
> 
> 	$ v4l2-ctl -d /dev/v4l-subdev4 -c 0x009e0901=1519
> 	$ yavta -f YUYV -s 640x480 -c100 --skip 7 /dev/video0
> 	...
> 	10 (2) [-] any 11 614400 B 2189.317617 2189.317629 15.244 fps ts mono/EoF
> 	11 (3) [-] any 12 614400 B 2189.383212 2189.383224 15.245 fps ts mono/EoF
> 	12 (4) [-] any 13 614400 B 2189.448810 2189.448821 15.244 fps ts mono/EoF
> 	13 (5) [-] any 14 614400 B 2189.514405 2189.514417 15.245 fps ts mono/EoF
> 	14 (6) [-] any 15 614400 B 2189.580002 2189.580015 15.245 fps ts mono/EoF
> 	..
> 
>    2592x1944 YUVV 15 FPS (default)
> 	$ yavta -f YUYV -s 2592x1944 -c100 --skip 7 /dev/video0
> 	...
> 	6 (6) [-] any 7 10077696 B 2438.377592 2438.377605 15.009 fps ts mono/EoF
> 	7 (7) [-] any 8 10077696 B 2438.444219 2438.444233 15.009 fps ts mono/EoF
> 	8 (0) [-] any 9 10077696 B 2438.510846 2438.510860 15.009 fps ts mono/EoF
> 	9 (1) [-] any 10 10077696 B 2438.577474 2438.577488 15.009 fps ts mono/EoF
> 	10 (2) [-] any 11 10077696 B 2438.644101 2438.644116 15.009 fps ts mono/EoF
> 	11 (3) [-] any 12 10077696 B 2438.710727 2438.710740 15.009 fps ts mono/EoF
> 	12 (4) [-] any 13 10077696 B 2438.777358 2438.777370 15.008 fps ts mono/EoF
> 	13 (5) [-] any 14 10077696 B 2438.843984 2438.843998 15.009 fps ts mono/EoF
> 	14 (6) [-] any 15 10077696 B 2438.910611 2438.910623 15.009 fps ts mono/EoF
> 	15 (7) [-] any 16 10077696 B 2438.977238 2438.977252 15.009 fps ts mono/EoF
> 	16 (0) [-] any 17 10077696 B 2439.043865 2439.043877 15.009 fps ts mono
> 	...
> 
> 
> To enable higher FPS the LINK_FREQ control should be made writable to increase
> the pixel rate
> 
>    640x480 YUYV 60 FPS (pixel_rate = 96 Mhz)
> 
> 	$ yavta -f YUYV -s 640x480 -c100 --skip 7 /dev/video0
>   	...
> 	9 (1) [-] any 10 614400 B 57.098649 57.098667 59.995 fps ts mono/EoF
> 	10 (2) [-] any 11 614400 B 57.115314 57.115332 60.006 fps ts mono/EoF
> 	11 (3) [-] any 12 614400 B 57.131978 57.131994 60.010 fps ts mono/EoF
> 	12 (4) [-] any 13 614400 B 57.148645 57.148664 59.999 fps ts mono/EoF
> 	13 (5) [-] any 14 614400 B 57.165310 57.165328 60.006 fps ts mono/EoF
> 	14 (6) [-] any 15 614400 B 57.181977 57.181996 59.999 fps ts mono/EoF
> 	15 (7) [-] any 16 614400 B 57.198642 57.198660 60.006 fps ts mono/EoF
> 
> Changelog:
> 
> v2->v3:
> 
> - Eugen (thanks) reported regression in DVP mode :(
>    To maintain the DVP timings un-changed in this version the mode definition now
>    looks like
> 
> 		/* 640x480 */
> 		.id		= OV5640_MODE_VGA_640_480,
> 		.dn_mode	= SUBSAMPLING,
> 		.pixel_rate	= OV5640_PIXEL_RATE_48M,
> 		.width		= 640,
> 		.height		= 480,
> 		.dvp_timings = {
> 			.analog_crop = {
> 				.left	= 0,
> 				.top	= 4,
> 				.width	= 2624,
> 				.height	= 1944,
> 			},
> 			.crop = {
> 				.left	= 16,
> 				.top	= 6,
> 				.width	= 640,
> 				.height	= 480,
> 			},
> 			.htot		= 1896,
> 			.vblank_def	= 600,
> 			.max_fps	= OV5640_60_FPS
> 		},
> 		.csi2_timings = {
> 			.analog_crop = {
> 				/* Feed the full valid pixel array to the ISP. */
> 				.left	= OV5640_PIXEL_ARRAY_LEFT,
> 				.top	= OV5640_PIXEL_ARRAY_TOP,
> 				.width	= OV5640_PIXEL_ARRAY_WIDTH,
> 				.height	= OV5640_PIXEL_ARRAY_HEIGHT,
> 			},
> 			.crop = {
> 				/* Maintain a minimum digital crop processing margins. */
> 				.left	= 2,
> 				.top	= 4,
> 				.width	= 640,
> 				.height	= 480,
> 			},
> 			.htot		= 1600,
> 			.vblank_def	= 520,
> 		},
> 		.reg_data	= ov5640_setting_low_res,
> 		.reg_data_size	= ARRAY_SIZE(ov5640_setting_low_res),
> 
>    with a .dvp_timings and a .csi2_timings members to separate the two.
>    Is it nice ? No it's not, but it should help maintaining DVP users happy.
> 
>    Eugen: if you are willing to run another test round to confirm if this version
>    does not regress DVP it would be great :)
> 
> - Split image formats between CSI-2 and DVP
> - Remove RGB888 as per the CSIS discussion with Laurent
> - Removed register tables for modes < 720 as they're all equal
> - Minor fixes on Laurent's comments
> - Add Adam's tag
> 
> v1 -> v2:
> - rework the modes definition to process the full pixel array
> - rework get_selection to report the correct BOUND and DEFAULT targets
> - implement init_cfg
> - minor style changes as suggested by Laurent
> - test with 1 data lane
> 
> Jacopo Mondi (27):
>    media: ov5640: Add pixel rate to modes
>    media: ov5604: Re-arrange modes definition
>    media: ov5640: Add ov5640_is_csi2() function
>    media: ov5640: Associate bpp with formats
>    media: ov5640: Add LINK_FREQ control
>    media: ov5640: Update pixel_rate and link_freq
>    media: ov5640: Rework CSI-2 clock tree
>    media: ov5640: Rework timings programming
>    media: ov5640: Fix 720x480 in RGB888 mode
>    media: ov5640: Split DVP and CSI-2 timings
>    media: ov5640: Provide timings accessor
>    media: ov5640: Re-sort per-mode register tables
>    media: ov5640: Remove duplicated mode settings
>    media: ov5640: Remove ov5640_mode_init_data
>    media: ov5640: Add HBLANK control
>    media: ov5640: Add VBLANK control
>    media: ov5640: Change CSI-2 timings to comply with FPS
>    media: ov5640: Implement init_cfg
>    media: ov5640: Implement get_selection
>    media: ov5640: Limit frame_interval to DVP mode only
>    media: ov5640: Register device properties
>    media: ov5640: Add RGB565_1X16 format
>    media: ov5640: Add BGR888 format
>    media: ov5640: Restrict sizes to mbus code
>    media: ov5640: Adjust format to bpp in s_fmt
>    media: ov5640: Split DVP and CSI-2 formats
>    media: ov5640: Move format mux config in format
> 
>   drivers/media/i2c/ov5640.c | 1615 ++++++++++++++++++++++++++----------
>   1 file changed, 1160 insertions(+), 455 deletions(-)
> 
> --
> 2.35.0
> 
> 
>
  
Hugues FRUCHET April 7, 2022, 4:25 p.m. UTC | #13
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.
  
Sakari Ailus April 8, 2022, 11:05 a.m. UTC | #14
Hi Hugues,

On Thu, Apr 07, 2022 at 06:25:11PM +0200, Hugues FRUCHET - FOSS wrote:
> 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 ?

Please see:

<URL:https://hverkuil.home.xs4all.nl/spec/userspace-api/v4l/subdev-formats.html#v4l2-mbus-pixelcode>

I.e. st-mipid02 and dcmi drivers should be fixed.
  
Hugues FRUCHET April 26, 2022, 12:32 p.m. UTC | #15
Hi Sakari,

On 4/8/22 1:05 PM, Sakari Ailus wrote:
> Hi Hugues,
> 
> On Thu, Apr 07, 2022 at 06:25:11PM +0200, Hugues FRUCHET - FOSS wrote:
>> 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 ?
> 
> Please see:
> 
> <URL:https://hverkuil.home.xs4all.nl/spec/userspace-api/v4l/subdev-formats.html#v4l2-mbus-pixelcode>
> 
> I.e. st-mipid02 and dcmi drivers should be fixed.
> 

OK, so "single clock cycle" convention for serial bus (copy/paste here 
for the sake of clarity):

4.13.3.4.1.1. Media Bus Pixel Codes

The media bus pixel codes document parallel formats. Should the pixel 
data be transported over a serial bus, the media bus pixel code that 
describes a parallel format that transfers a sample on a single clock 
cycle is used. For instance, both MEDIA_BUS_FMT_BGR888_1X24 and 
MEDIA_BUS_FMT_BGR888_3X8 are used on parallel busses for transferring an 
8 bits per sample BGR data, whereas on serial busses the data in this 
format is only referred to using MEDIA_BUS_FMT_BGR888_1X24. This is 
because there is effectively only a single way to transport that format 
on the serial busses.

I'll try to change both dcmi and mipid02 in that way...

Best regards,
Hugues.