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

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

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

-- 
Regards,

Laurent Pinchart