[libcamera-devel] Constraints on the sensor subdevice

[Jacopo Mondi]

Hi Dave,

On Fri, Feb 21, 2020 at 11:25:05AM +0000, Dave Stevenson wrote:
> Hi Jacopo
>

[snip]

> > > If I got you right here, on your platform the image format selected on
> > > the CSI-2 receiver gets propagated to the sensor subdevice by
> > > performing the 4cc->mbus translation logic in kernel space. This
> > > implies the sensor subdevice and the CSI-2 receiver drivers have to be
> > > tightly coupled, unless complex mbus selection heuristics are
> > > implemented in the CSI-2 receiver driver to support a wide range of
> > > sensor drivers.
> >
> > Yes, this is correct.  The CSI-2 Rx and sensor subdevice are
> > inherently coupled on our platform.
> >
> > There is a distinction to make here though.  A CSI-2 Rx could either
> > be directly connected to an ISP pipeline, or dump out image data into
> > memory to be read back by an ISP.  For the latter case, it might also
> > be possible for the CSI-2 block to do some simple format conversion,
> > or even rescaling.  In our platform, we only allow our CSI-2 RX to
> > optionally do a data unpack operation (so a format conversion).  So,
> > during probe, the kernel driver enumerates the subdevice formats, and
> > generates a list of V4L formats to advertise - and these formats may
> > be duplicated to represent packed and unpacked options.
> >
> > > This seems not standard behaviour to me, but I can't tell if upstream
> > > would be fine with it or not. From a libcamera perspective, you won't
> > > be able to use CameraSensor::getFormat() but you will have to select
> > > which format to set on the CSI-2 receiver taking into account the mbus
> > > selection logic performed by the same driver.
> > >
> > > Can I ask if you have any requirement that prevents you from decoupling that
> > > logic in kernel space, and leave the mbus code selection on the sensor
> > > to userspace ? It seems a lot of complication in the driver that
> > > should instead live in your pipeline handler...
> > >
> >
> > With our approach described above, there is no need to apply a MBUS
> > format on the sensor subdevice as well as a V4L format on the CSI-2 Rx
> > device as they are inherently the same operation, if that makes sense
> > :)
> >
> > For more complicated topologies (where the CSI-2 Rx could rescale for
> > example), I understand that we may want to decouple this, but for now,
> > MBUS codes are essentially hidden from our pipeline handler.
>
> I originally wrote this driver based on the TI VPFE [1]. That's a
> non-MC device, and suited our requirements nicely.
> From an application perspective we want the simple use cases, so you
> set the format and that is what you get. Beyond repacking from eg
> V4L2_PIX_FMT_SBGGR10P to V4L2_PIX_FMT_SBGGR10 there is no image
> processing going on.
>
> The change to use media controller was actually your libcamera
> requirement for enumeration of devices. We have no need for it, so
> would sooner rip it back out.
> Complicating userspace apps for such a simple hardware block is nuts.

I'm sorry I really don't agree here, but I don't think this comes
unexpected :)

Implementing heuristics for 4cc->mbus code translation in kernel space
requires the csi-2 receiver and the sensor subdevice driver to be
somehow coupled, or better, it's very hard to guarantee that your csi-2
driver format selection logic work with all sensor drivers out there.
Am I wrong ?

Delegating that to userspace would allow you to de-couple the two drivers
completely, but I understand libcamera was not there when you first
designed that. Now that you have a place in userspace where to
implement those heuristics (your pipeline handler) I really would not
push them to kernel space.

Again, my 2c only, the linux-media community will provide much more
interesting feedbacks then mine :)

Thanks
   j

>
>   Dave
>
> [1] https://elixir.bootlin.com/linux/latest/source/drivers/media/platform/am437x
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20200221/6a73aee6/attachment.sig>