Re: [PATCH v4 5/5] media: bcm2835-unicam: Add TODO file

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

 



Hi Sakari

On Wed, 2 Dec 2020 at 22:07, Sakari Ailus <sakari.ailus@xxxxxx> wrote:
>
> Hi Jacopo,
>
> On Tue, Nov 10, 2020 at 06:40:36PM +0100, Jacopo Mondi wrote:
> > The bcm2835-unicam driver is currently in staging mainly for
> > two reasons:
> > - Handling of CSI-2 embedded data
> > - Usage of both media controller API and subdev kAPI
> >
> > Provide a more detailed description of the currently on-going design
> > discussions in the associated TODO file.
> >
> > Signed-off-by: Jacopo Mondi <jacopo@xxxxxxxxxx>
> > ---
> >  drivers/staging/media/bcm2835-unicam/TODO | 37 +++++++++++++++++++++++
> >  1 file changed, 37 insertions(+)
> >  create mode 100644 drivers/staging/media/bcm2835-unicam/TODO
> >
> > diff --git a/drivers/staging/media/bcm2835-unicam/TODO b/drivers/staging/media/bcm2835-unicam/TODO
> > new file mode 100644
> > index 0000000000000..c7840872eea4c
> > --- /dev/null
> > +++ b/drivers/staging/media/bcm2835-unicam/TODO
> > @@ -0,0 +1,37 @@
> > +BCM2835 Unicam driver TODO list
> > +===============================
> > +
> > +The unicam driver could be considered ready to be moved out of the staging
> > +directory in terms of code quality and expected functionalities.
> > +
> > +However there currently are two design issues that suggest the driver is
> > +better kept in staging for the time being.
> > +
> > +CSI-2 Embedded data support:
> > +----------------------------
> > +
> > +The RaspberryPi camera stack and camera applications rely on the availability of
> > +the sensor produced CSI-2 embedded data, whose support is currently not
> > +finalized in mainline Linux.
> > +
> > +The driver conditionally registers an additional video device node
> > +'unicam-embedded' with a single sink pad which connects to the sensor
> > +sub-device source pad #1 to expose ancillary data.
> > +
> > +Currently none of the mainline sensor drivers register more than a single pad,
> > +and consequentially no embedded data from the sensor are exposed to userspace.
> > +
> > +The current implementation is then subject to changes depending on how support
> > +for CSI-2 embedded data gets finalized in Linux.
>
> Are you looking to use out-of-tree sensor drivers that have two pads? I'd
> rather see sensor drivers merged to mainline proper.
>
> But as noted in the other e-mail, we need VC / stream support so this needs
> to be addressed for reasons not related to Unicam.

There's a downstream patch for imx219 that adds the second pad [1].
The imx477 driver that is currently only out-of-tree also supports it,
and should be upstreamed once this first wave of patches have got
somewhere.

[1] https://github.com/raspberrypi/linux/commit/fa8131cb1399f2c22de3f29e08ec1658db76552b
It's on the rpi-5.10.y branch too, but that is still being frequently
rebased so no stable commit hashes

> > +
> > +Media controller support:
> > +-------------------------
> > +
> > +Due to compatibility reasons with the existing RaspberryPi software ecosystem
> > +the unicam driver implements the media controller interface to allow the
> > +enumeration of the connected entities but handles the configuration of the
> > +sensor sub-device using the v4l2-subdev kAPI instead of delegating that to
> > +user-space.
> > +
> > +Discussions are on-going on how this should be better handled (driver option,
> > +KConfig option etc etc).
>
> That's a fair use case. There are two ways to handle it, either in the
> kernel where the choice affects how the user space looks like. You have an
> option of module parameter or Kconfig option there, and both are true
> annoyances.
>
> Another option is to work around it in the user space, wrapping the IOCTL
> calls. This way no kernel build or module load time parameters would be
> needed to switch between the two sets of user space programs.
>
> We probably can't decide it now, but could an MC-only driver + user space
> compatibility layer be an option here?

Iff the user-space compatibility layer worked with all standard users
(eg v4l2-ctl, FFmpeg and Gstreamer), then it's plausible, but is that
realistic?

The non-MC approach is mainly for things like the TC358743 HDMI->CSI
bridge and ADV728x-M chips where MC adds nothing, and they do just
work with the likes of FFmpeg and Gstreamer.
Image sensors are generally going to be used under libcamera umbrella,
so there MC works.

Kconfig isn't an option for us as one kernel build needs to support
all potential source devices via DT / runtime changes alone.

I haven't looked in detail at what the previous VC / stream patches
proposed as the API for passing the configuration. We only need MC for
the more complex use cases, so if that VC/stream API usage can be
detected at probe then we can switch to MC. It doesn't seem
unreasonable to expect any sensor drivers to be upgraded to correctly
use the new API even if they don't actually produce embedded data.
We also want MC for ADV748x HDMI&analog->CSI bridges that expose
multiple subdevs, but there we can look at the upstream endpoint and
see if it has any sink pads. Sink pads mean MC is needed. (I now have
a board with an ADV7482 on, so when time allows I'm intending to have
an experiment with it).
Use a module parameter as a last resort should the detection fail, or
I guess if you want to override the detected setting for some reason
(a simple sensor being used outside of libcamera for example).

Cheers.
  Dave



[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