[libcamera-devel] [PATCH v4 0/4] Raspberry Pi: handle sensors more flexibly

[Laurent Pinchart]

On Tue, Apr 27, 2021 at 09:17:55PM +0100, Kieran Bingham wrote:
> Hi David,
> 
> On 27/04/2021 14:08, David Plowman wrote:
> > Hi
> > 
> > Here's a version 4 of this patch set. The only difference over v3 is a
> > new penultimate patch that adds new V4L2 controls for informing the
> > sensor of the colour gains.
> > 
> > I've actually added a control for each of the 4 Bayer colour channels
> > even though I use only two of them. It feels like one of those things
> > where some sensor may eventually want the ones that I might have been
> > tempted to miss out, at which point why not just include them? Though
> > I still wonder if having both GREENR and GREENB is overkill, and just
> > GREEN would have sufficed. Any thoughts on that?
> 
> I think adding them all in now makes more sense than adding some in now,
> and then finding the others are needed but the numbering is no longer
> consecutive because a control has been added in the meanwhile.
> 
> Would there ever be a (non-bayer?) sensor that would use only R,G,B? In
> which case, I would wonder which GREEN should be used... but I suspect
> that's not really an issue that will occur if sensors that need this
> data are always going to be some form of bayer.
> 
> > Once we're happy with the changes here I can apply them in the
> > Raspberry Pi Linux distribution, and then we can look to upstream them
> > too.
> 
> I would suggest doing this the other way around.
> 
> The V4L2 patch should already be posted to the V4L2 mailing list, to get
> early review comments.
> 
> "Upstream first" is always better ;-)

+1. We can merge things in libcamera without waiting for upstream in
some cases, but the code has to be on its way to upstream. In this
particular case, it means that the patches have to be posted to the
linux-media mailign list, with at least a general agreement on the
direction. Documentation for the new controls is thus needed.

Small details and upstreaming delays due to the schedule of the upstream
kernel merge windows are not blockers, we can add the controls to the
libcamera copy of the kernel headers once they're approved in principle
by upstream.

> > David Plowman (4):
> >   ipa: raspberrypi: Make CamHelper exposure methods virtual
> >   ipa: raspberrypi: Add CamHelper::ColourGainCode method
> >   include: linux: Add V4L2_CID_NOTIFY_GAIN_XXX controls
> >   ipa: raspberrypi: Update sensor's V4L2_CID_NOTIFY_GAIN_RED/BLUE
> >     controls when present
> > 
> >  include/libcamera/ipa/raspberrypi.mojom        |  1 +
> >  include/linux/v4l2-controls.h                  |  4 ++++
> >  src/ipa/raspberrypi/cam_helper.cpp             | 18 ++++++++++++++++++
> >  src/ipa/raspberrypi/cam_helper.hpp             |  8 +++++---
> >  src/ipa/raspberrypi/raspberrypi.cpp            | 13 +++++++++++++
> >  .../pipeline/raspberrypi/raspberrypi.cpp       | 10 ++++++++++
> >  6 files changed, 51 insertions(+), 3 deletions(-)

-- 
Regards,

Laurent Pinchart