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

[Dave Stevenson]

On Tue, 7 Jul 2020 at 22:40, Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> Hi Dave,
>
> On Tue, Jul 07, 2020 at 10:13:04PM +0100, Dave Stevenson wrote:
> > On Tue, 7 Jul 2020 at 17:48, Laurent Pinchart wrote:
> > > 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.
>
> I wasn't aware of the DT parameters mechanism. I'll adapt the patches
> accordingly, and will also reference upstream commit for backported
> patches.

Thanks. If you're stuck then create the PR and I'll either add any
additional changes, or make comment as to what is needed.

Phil will be wanting an update to
https://github.com/raspberrypi/linux/blob/rpi-5.4.y/arch/arm/boot/dts/overlays/README
as well so that the help text reflects any parameters.

> > 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.
> >
> > > > > 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