[libcamera-devel] [PATCH 00/12] raspberrypi: Report sensor orientation through DT

[Dave Stevenson]

On Tue, 7 Jul 2020 at 17:48, Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> Hi Dave,
>
> On Mon, Jul 06, 2020 at 06:33:41PM +0100, Dave Stevenson wrote:
> > On Sat, 4 Jul 2020 at 01:40, Laurent Pinchart wrote:
> > >
> > > Hi Dave,
> > >
> > > This patch series reports sensor orientation through DT for the OV5647,
> > > IMX219 and IMX477. The first 8 patches are backported from mainline,
> > > while the last 4 patches are new. Patch 09/12 could possibly be skipped
> > > for now, as we don't use the rotation property in the ov5647 DT overlay.
> > >
> > > The patches are based on top of rpi-5.4.y. libcamera patches will follow
> > > shortly.
> >
> > The patches look reasonable. I'd suggest that rotation really ought to
> > be a dtoverlay property rather than hard coded. Particularly with
> > IMX219 there is no defined "right way" of mounting it. IMX477 with the
> > tripod screw-hole does have a natural orientation.
>
> I agree, the rotation belongs to an overlay, as it will depend on how
> the camera module is integrated in the final product. This patch series
> moves the default rotation from the RPi IPA to the camera sensor
> overlays to provide the same default value as currently used, but it's
> really up to the user to decide on the correct value.
>
> > Are you expecting those to be merged into our kernel tree? We normally
> > use Github pull requests rather than mailing lists.
>
> I wanted the patches to appear on the libcamera-devel mailing list in
> case discussions were needed, and to provide a reference to the kernel
> modifications required by the corresponding libcamera patches I've
> posted. Should I send a pull request, or are there issues you think need
> to be addressed first ?

It'd be nice if rotation was a dt overlay parameter before creating
the PR, but otherwise it looks fine to me.

Where patches are backports of patches that have been merged to
mainline, we normally add a line
"Commit <full hash> upstream"
to the commit text, just to make Dom's job easier when he pulls stuff
forward to newer kernels.

  Dave

> > > Jacopo Mondi (8):
> > >   media: dt-bindings: video-interfaces: Document 'orientation' property
> > >   media: dt-bindings: video-interface: Replace 'rotation' description
> > >   media: v4l2-ctrl: Document V4L2_CID_CAMERA_ORIENTATION
> > >   media: v4l2-ctrl: Document V4L2_CID_CAMERA_SENSOR_ROTATION
> > >   media: v4l2-ctrls: Add camera orientation and rotation
> > >   media: v4l2-fwnode: Add helper to parse device properties
> > >   media: v4l2-ctrls: Add helper to register properties
> > >   media: i2c: imx219: Parse and register properties
> > >
> > > Laurent Pinchart (4):
> > >   media: i2c: ov5647: Parse and register properties
> > >   media: i2c: imx477: Parse and register properties
> > >   dt/dtoverlays: imx219: Set sensor rotation
> > >   dt/dtoverlays: imx477: Set sensor rotation
> > >
> > >  .../bindings/media/video-interfaces.txt       | 370 +++++++++++++++++-
> > >  .../media/uapi/v4l/ext-ctrls-camera.rst       | 151 +++++++
> > >  arch/arm/boot/dts/overlays/imx219-overlay.dts |   2 +
> > >  arch/arm/boot/dts/overlays/imx477-overlay.dts |   2 +
> > >  drivers/media/i2c/imx219.c                    |  12 +-
> > >  drivers/media/i2c/imx477.c                    |  12 +-
> > >  drivers/media/i2c/ov5647.c                    |  13 +-
> > >  drivers/media/v4l2-core/v4l2-ctrls.c          |  53 +++
> > >  drivers/media/v4l2-core/v4l2-fwnode.c         |  42 ++
> > >  include/media/v4l2-ctrls.h                    |  26 ++
> > >  include/media/v4l2-fwnode.h                   |  47 +++
> > >  include/uapi/linux/v4l2-controls.h            |   7 +
> > >  12 files changed, 731 insertions(+), 6 deletions(-)
>
> --
> Regards,
>
> Laurent Pinchart