2022-02-04 09:36:45

by Jean-Michel Hautbois

[permalink] [raw]
Subject: [RFC PATCH v4 00/12] Add support for BCM2835 camera interface (unicam)

Hello !

This patch series adds the BCM2835 CCP2/CSI2 camera interface named
unicam. This driver is already used in the out-of-tree linux-rpi
repository [1], and this work aims to support it in mainline.

The series is based on top of:
- Rebased on top of 5.16 Tomi Valkeinen's multiplexed stream work, on
his multistream/work branch [2] which has been submitted as v10 at the
time of this writing. The objective is to demonstrate the use of
multiplexed streams on a real world widely used example as well as
supporting unicam mainline.
- The "Add 12bit and 14bit luma-only formats" series [3] is partly
applied (the Y10P format bug) the new formats are now part of this
series.

The series is made of 12 patches:
- 1/12 and 2/12 introduce the new formats needed for the unicam driver
- 3/12 introduces dt-bindings documentation
- 4/12 adds the MAINTAINERS entry
- 5/12 adds the driver support in media/platform
- 6/12 introduces the csi nodes in the bcm2711 file, in a disabled state
- 7/12 to 11/12 modifies imx219 driver to make it use the multiplexed
streams API
- 12/12 is the imx219 dtsi file tested on my RPi 4b with the mainline
dtb and not the downstream dtb anymore. *This patch is not intended to
be applied*.

All those patches are in my tree [4].

Patch 5/12 comes from the linux-rpi work [1] with substantial
modifications:
- Removed the non-mc API which is deprecated, and not needed upstream
- Moved from one video node with two source pads (image and embedded) to
two video nodes
- Added a subdev between the sensor and the video nodes to properly
route the streams
- Added support for multiplexed streams API

In order to test it, one will need a RPi board and the camv2 (imx219)
sensor. An updated v4l-utils is also needed [5] to have support for
multiplexed streams.

v4l2-compliance passes on both video devices, without streaming testing
though with one exception: as the colorspace support is removed in v3,
we now have:

test VIDIOC_G_FMT: OK
fail: v4l2-test-formats.cpp(363): colorspace >= 0xff
fail: v4l2-test-formats.cpp(465): testColorspace(!node->is_io_mc,
pix.pixelformat, pix.colorspace, pix.ycbcr_enc, pix.quantization)
test VIDIOC_TRY_FMT: FAIL
fail: v4l2-test-formats.cpp(363): colorspace >= 0xff
fail: v4l2-test-formats.cpp(465): testColorspace(!node->is_io_mc,
pix.pixelformat, pix.colorspace, pix.ycbcr_enc, pix.quantization)
test VIDIOC_S_FMT: FAIL

This series since its v2 does integrate the device tree nodes into the
upstream BCM2835 or BCM2711 device tree support.

In order to properly configure the media pipeline, it is needed to call
the usual ioctls, and configure routing in order to send the embedded
data from the sensor to the "unicam-embedded" device node :

```
media=0
media-ctl -d${media} -l "'imx219 2-0010':0->'unicam-subdev':0 [1]"
media-ctl -d${media} -l "'unicam-subdev':1->'unicam-image':0 [1]"
media-ctl -d${media} -v -R "'unicam-subdev' [0/0->1/0[1],0/1->2/0[1]]"
media-ctl -d${media} -V "'imx219 2-0010':0/0 [fmt:SRGGB10_1X10/3280x2464 field:none]"
v4l2-ctl -d0 --set-fmt-video width=3280,height=2464,pixelformat='pRAA',field=none
media-ctl -d${media} -v -V "'imx219 2-0010':0/1 [fmt:METADATA_8/16384x1 field:none]"
media-ctl -d${media} -p
```

Be sure to configure the routes before setting the format, as s_routing
resets the default format.

The media-ctl topology is:
```
pi@raspberrypi:~ $ media-ctl -d${media} -p
Media controller API version 5.16.0

Media device information
------------------------
driver unicam
model unicam
serial
bus info platform:fe801000.csi
hw revision 0x0
driver version 5.16.0

Device topology
- entity 1: unicam-subdev (3 pads, 3 links, 2 routes)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev0
routes:
0/0 -> 1/0 [ACTIVE]
0/1 -> 2/0 [ACTIVE]
pad0: Sink
[stream:0 fmt:SRGGB10_1X10/3280x2464 field:none colorspace:raw]
[stream:1 fmt:METADATA_8/16384x1 field:none]
<- "imx219 2-0010":0 [ENABLED,IMMUTABLE]
pad1: Source
[stream:0 fmt:SRGGB10_1X10/3280x2464 field:none colorspace:raw]
-> "unicam-image":0 [ENABLED,IMMUTABLE]
pad2: Source
[stream:0 fmt:METADATA_8/16384x1 field:none]
-> "unicam-embedded":0 [ENABLED,IMMUTABLE]

- entity 5: imx219 2-0010 (1 pad, 1 link, 2 routes)
type V4L2 subdev subtype Sensor flags 0
device node name /dev/v4l-subdev1
routes:
0/0 -> 0/0 [ACTIVE, IMMUTABLE, SOURCE]
0/0 -> 0/1 [ACTIVE, SOURCE]
pad0: Source
[stream:0 fmt:SRGGB10_1X10/3280x2464 field:none colorspace:raw
crop.bounds:(8,8)/3280x2464
crop:(8,8)/3280x2464]
[stream:1 fmt:METADATA_8/16384x1 field:none
crop.bounds:(8,8)/3280x2464
crop:(8,8)/3280x2464]
-> "unicam-subdev":0 [ENABLED,IMMUTABLE]

- entity 9: unicam-image (1 pad, 1 link, 0 route)
type Node subtype V4L flags 1
device node name /dev/video0
pad0: Sink
<- "unicam-subdev":1 [ENABLED,IMMUTABLE]

- entity 15: unicam-embedded (1 pad, 1 link, 0 route)
type Node subtype V4L flags 0
device node name /dev/video1
pad0: Sink
<- "unicam-subdev":2 [ENABLED,IMMUTABLE]

```

Then a frame can be capture with yavta:
`yavta --capture=1 /dev/video0 -F/tmp/test-#.bin`

In v3, capture on the metadata node (/dev/video1 in my case) can't be
started if capture on image node (/dev/video0) is not already started.
This can be tested using yavta, letting it capture frames indefinitely
and start another yavta instance on the /dev/video1 node.


Side note:
My tree [4] also includes a backport for the unicam-isp drivers, it is
then possible, and it has been successfully tested, to use libcamera and
qcam to display the camera output.

[1]: https://github.com/raspberrypi/linux/tree/rpi-5.15.y
[2]: https://git.kernel.org/pub/scm/linux/kernel/git/tomba/linux.git/log/?h=multistream/work
[3]: https://patchwork.linuxtv.org/project/linux-media/list/?series=7099
[4]: https://github.com/jhautbois/linux-rpi/tree/jmh/unicam-multiplexed-streams
[5]: https://github.com/jhautbois/v4l-utils/tree/jmh/tomi-multiplexed-streams

Jean-Michel Hautbois (12):
media: v4l: Add V4L2-PIX-FMT-Y12P format
media: v4l: Add V4L2-PIX-FMT-Y14P format
dt-bindings: media: Add bindings for bcm2835-unicam
media: MAINTAINERS: add bcm2835 unicam driver
media: bcm2835-unicam: Add support for CCP2/CSI2 camera interface
ARM: dts: bcm2711: Add unicam CSI nodes
media: imx219: Rename mbus codes array
media: imx219: Switch from open to init_cfg
media: imx219: Introduce the set_routing operation
media: imx219: use a local v4l2_subdev to simplify reading
media: imx219: Add support for the V4L2 subdev active state
media: bcm283x: Include the imx219 node

.../bindings/media/brcm,bcm2835-unicam.yaml | 110 +
.../media/v4l/pixfmt-yuv-luma.rst | 44 +
MAINTAINERS | 8 +
arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 1 +
arch/arm/boot/dts/bcm2711-rpi.dtsi | 15 +
arch/arm/boot/dts/bcm2711.dtsi | 16 +
arch/arm/boot/dts/bcm283x-rpi-imx219.dtsi | 102 +
drivers/media/i2c/imx219.c | 344 ++-
drivers/media/platform/Kconfig | 1 +
drivers/media/platform/Makefile | 2 +
drivers/media/platform/bcm2835/Kconfig | 21 +
drivers/media/platform/bcm2835/Makefile | 3 +
.../media/platform/bcm2835/bcm2835-unicam.c | 2586 +++++++++++++++++
.../media/platform/bcm2835/vc4-regs-unicam.h | 253 ++
drivers/media/v4l2-core/v4l2-ioctl.c | 2 +
include/uapi/linux/videodev2.h | 2 +
16 files changed, 3362 insertions(+), 148 deletions(-)
create mode 100644 Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml
create mode 100644 arch/arm/boot/dts/bcm283x-rpi-imx219.dtsi
create mode 100644 drivers/media/platform/bcm2835/Kconfig
create mode 100644 drivers/media/platform/bcm2835/Makefile
create mode 100644 drivers/media/platform/bcm2835/bcm2835-unicam.c
create mode 100644 drivers/media/platform/bcm2835/vc4-regs-unicam.h

--
2.32.0


2022-02-04 13:44:21

by Jean-Michel Hautbois

[permalink] [raw]
Subject: [RFC PATCH v4 02/12] media: v4l: Add V4L2-PIX-FMT-Y14P format

This is a packed grey-scale image format with a depth of 14 bits per
pixel. Every four consecutive samples are packed into seven bytes. Each
of the first four bytes contain the eight high order bits of the pixels,
and the three following bytes contains the six least significants bits
of each pixel, in the same order.

As the other formats only needed 5 bytes before, append two bytes in the
documentation array.

Signed-off-by: Jean-Michel Hautbois <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
---
.../media/v4l/pixfmt-yuv-luma.rst | 33 +++++++++++++++++++
drivers/media/v4l2-core/v4l2-ioctl.c | 1 +
include/uapi/linux/videodev2.h | 1 +
3 files changed, 35 insertions(+)

diff --git a/Documentation/userspace-api/media/v4l/pixfmt-yuv-luma.rst b/Documentation/userspace-api/media/v4l/pixfmt-yuv-luma.rst
index 2d9d588eedcd..2f38b888ca19 100644
--- a/Documentation/userspace-api/media/v4l/pixfmt-yuv-luma.rst
+++ b/Documentation/userspace-api/media/v4l/pixfmt-yuv-luma.rst
@@ -36,6 +36,8 @@ are often referred to as greyscale formats.
- Byte 2
- Byte 3
- Byte 4
+ - Byte 5
+ - Byte 6

* .. _V4L2-PIX-FMT-GREY:

@@ -47,6 +49,8 @@ are often referred to as greyscale formats.
- ...
- ...
- ...
+ - ...
+ - ...

* .. _V4L2-PIX-FMT-Y10:

@@ -58,6 +62,8 @@ are often referred to as greyscale formats.
- ...
- ...
- ...
+ - ...
+ - ...

* .. _V4L2-PIX-FMT-Y10BPACK:

@@ -69,6 +75,8 @@ are often referred to as greyscale formats.
- Y'\ :sub:`1`\ [3:0] Y'\ :sub:`2`\ [9:6]
- Y'\ :sub:`2`\ [5:0] Y'\ :sub:`3`\ [9:8]
- Y'\ :sub:`3`\ [7:0]
+ - ...
+ - ...

* .. _V4L2-PIX-FMT-Y10P:

@@ -80,6 +88,8 @@ are often referred to as greyscale formats.
- Y'\ :sub:`2`\ [9:2]
- Y'\ :sub:`3`\ [9:2]
- Y'\ :sub:`3`\ [1:0] Y'\ :sub:`2`\ [1:0] Y'\ :sub:`1`\ [1:0] Y'\ :sub:`0`\ [1:0]
+ - ...
+ - ...

* .. _V4L2-PIX-FMT-Y12:

@@ -91,6 +101,8 @@ are often referred to as greyscale formats.
- ...
- ...
- ...
+ - ...
+ - ...

* .. _V4L2-PIX-FMT-Y12P:

@@ -102,6 +114,8 @@ are often referred to as greyscale formats.
- Y'\ :sub:`1`\ [3:0] Y'\ :sub:`0`\ [3:0]
- ...
- ...
+ - ...
+ - ...

* .. _V4L2-PIX-FMT-Y14:

@@ -113,6 +127,21 @@ are often referred to as greyscale formats.
- ...
- ...
- ...
+ - ...
+ - ...
+
+ * .. _V4L2-PIX-FMT-Y14P:
+
+ - ``V4L2_PIX_FMT_Y14P``
+ - 'Y14P'
+
+ - Y'\ :sub:`0`\ [13:6]
+ - Y'\ :sub:`1`\ [13:6]
+ - Y'\ :sub:`2`\ [13:6]
+ - Y'\ :sub:`3`\ [13:6]
+ - Y'\ :sub:`1`\ [1:0] Y'\ :sub:`0`\ [5:0]
+ - Y'\ :sub:`2`\ [3:0] Y'\ :sub:`1`\ [5:2]
+ - Y'\ :sub:`3`\ [5:0] Y'\ :sub:`2`\ [5:4]

* .. _V4L2-PIX-FMT-Y16:

@@ -124,6 +153,8 @@ are often referred to as greyscale formats.
- ...
- ...
- ...
+ - ...
+ - ...

* .. _V4L2-PIX-FMT-Y16-BE:

@@ -135,6 +166,8 @@ are often referred to as greyscale formats.
- ...
- ...
- ...
+ - ...
+ - ...

.. raw:: latex

diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index 64a72a6b2132..131105e8afac 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -1265,6 +1265,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
case V4L2_PIX_FMT_Y10BPACK: descr = "10-bit Greyscale (Packed)"; break;
case V4L2_PIX_FMT_Y10P: descr = "10-bit Greyscale (MIPI Packed)"; break;
case V4L2_PIX_FMT_Y12P: descr = "12-bit Greyscale (MIPI Packed)"; break;
+ case V4L2_PIX_FMT_Y14P: descr = "14-bit Greyscale (MIPI Packed)"; break;
case V4L2_PIX_FMT_Y8I: descr = "Interleaved 8-bit Greyscale"; break;
case V4L2_PIX_FMT_Y12I: descr = "Interleaved 12-bit Greyscale"; break;
case V4L2_PIX_FMT_Z16: descr = "16-bit Depth"; break;
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index bef488205fea..18611289217f 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -570,6 +570,7 @@ struct v4l2_pix_format {
#define V4L2_PIX_FMT_Y10BPACK v4l2_fourcc('Y', '1', '0', 'B') /* 10 Greyscale bit-packed */
#define V4L2_PIX_FMT_Y10P v4l2_fourcc('Y', '1', '0', 'P') /* 10 Greyscale, MIPI RAW10 packed */
#define V4L2_PIX_FMT_Y12P v4l2_fourcc('Y', '1', '2', 'P') /* 12 Greyscale, MIPI RAW12 packed */
+#define V4L2_PIX_FMT_Y14P v4l2_fourcc('Y', '1', '4', 'P') /* 14 Greyscale, MIPI RAW14 packed */

/* Palette formats */
#define V4L2_PIX_FMT_PAL8 v4l2_fourcc('P', 'A', 'L', '8') /* 8 8-bit palette */
--
2.32.0

2022-02-04 18:43:45

by Jean-Michel Hautbois

[permalink] [raw]
Subject: [RFC PATCH v4 04/12] media: MAINTAINERS: add bcm2835 unicam driver

Add the BCM2835 Camera driver named Unicam in the list.

Signed-off-by: Jean-Michel Hautbois <[email protected]>
---
MAINTAINERS | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 1868a6002df8..740cf86c56dc 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3670,6 +3670,13 @@ N: bcm113*
N: bcm216*
N: kona

+BROADCOM BCM2835 CAMERA DRIVER
+M: Raspberry Pi Kernel Maintenance <[email protected]>
+L: [email protected]
+S: Maintained
+F: Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml
+F: arch/arm/boot/dts/bcm283x*
+
BROADCOM BCM47XX MIPS ARCHITECTURE
M: Hauke Mehrtens <[email protected]>
M: Rafał Miłecki <[email protected]>
--
2.32.0

2022-02-04 20:35:12

by Laurent Pinchart

[permalink] [raw]
Subject: Re: [RFC PATCH v4 04/12] media: MAINTAINERS: add bcm2835 unicam driver

Hi Jean-Michel,

Thank you for the patch.

On Thu, Feb 03, 2022 at 06:50:01PM +0100, Jean-Michel Hautbois wrote:
> Add the BCM2835 Camera driver named Unicam in the list.
>
> Signed-off-by: Jean-Michel Hautbois <[email protected]>
> ---
> MAINTAINERS | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1868a6002df8..740cf86c56dc 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3670,6 +3670,13 @@ N: bcm113*
> N: bcm216*
> N: kona
>
> +BROADCOM BCM2835 CAMERA DRIVER
> +M: Raspberry Pi Kernel Maintenance <[email protected]>
> +L: [email protected]
> +S: Maintained
> +F: Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml
> +F: arch/arm/boot/dts/bcm283x*

Add the driver here, and move this patch later in the series after
adding all the files listed above. With this fixed,

Reviewed-by: Laurent Pinchart <[email protected]>

> +
> BROADCOM BCM47XX MIPS ARCHITECTURE
> M: Hauke Mehrtens <[email protected]>
> M: Rafał Miłecki <[email protected]>

--
Regards,

Laurent Pinchart

2022-02-04 22:03:42

by Jean-Michel Hautbois

[permalink] [raw]
Subject: [RFC PATCH v4 08/12] media: imx219: Switch from open to init_cfg

Use the init_cfg pad level operation instead of the internal subdev
open operation to set default formats on the pads.
While at it, make the imx219_pad_ops more easier to read.

Signed-off-by: Jean-Michel Hautbois <[email protected]>
---
drivers/media/i2c/imx219.c | 138 +++++++++++++++++++++----------------
1 file changed, 80 insertions(+), 58 deletions(-)

diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
index 74dba5e61201..b68d35046725 100644
--- a/drivers/media/i2c/imx219.c
+++ b/drivers/media/i2c/imx219.c
@@ -118,6 +118,10 @@
#define IMX219_PIXEL_ARRAY_WIDTH 3280U
#define IMX219_PIXEL_ARRAY_HEIGHT 2464U

+/* Embedded metadata stream structure */
+#define IMX219_EMBEDDED_LINE_WIDTH 16384
+#define IMX219_NUM_EMBEDDED_LINES 1
+
struct imx219_reg {
u16 address;
u8 val;
@@ -668,51 +672,6 @@ static u32 imx219_get_format_code(struct imx219 *imx219, u32 code)
return imx219_mbus_formats[i];
}

-static void imx219_set_default_format(struct imx219 *imx219)
-{
- struct v4l2_mbus_framefmt *fmt;
-
- fmt = &imx219->fmt;
- fmt->code = MEDIA_BUS_FMT_SRGGB10_1X10;
- fmt->colorspace = V4L2_COLORSPACE_SRGB;
- fmt->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(fmt->colorspace);
- fmt->quantization = V4L2_MAP_QUANTIZATION_DEFAULT(true,
- fmt->colorspace,
- fmt->ycbcr_enc);
- fmt->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(fmt->colorspace);
- fmt->width = supported_modes[0].width;
- fmt->height = supported_modes[0].height;
- fmt->field = V4L2_FIELD_NONE;
-}
-
-static int imx219_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
-{
- struct imx219 *imx219 = to_imx219(sd);
- struct v4l2_mbus_framefmt *try_fmt =
- v4l2_subdev_get_try_format(sd, fh->state, 0);
- struct v4l2_rect *try_crop;
-
- mutex_lock(&imx219->mutex);
-
- /* Initialize try_fmt */
- try_fmt->width = supported_modes[0].width;
- try_fmt->height = supported_modes[0].height;
- try_fmt->code = imx219_get_format_code(imx219,
- MEDIA_BUS_FMT_SRGGB10_1X10);
- try_fmt->field = V4L2_FIELD_NONE;
-
- /* Initialize try_crop rectangle. */
- try_crop = v4l2_subdev_get_try_crop(sd, fh->state, 0);
- try_crop->top = IMX219_PIXEL_ARRAY_TOP;
- try_crop->left = IMX219_PIXEL_ARRAY_LEFT;
- try_crop->width = IMX219_PIXEL_ARRAY_WIDTH;
- try_crop->height = IMX219_PIXEL_ARRAY_HEIGHT;
-
- mutex_unlock(&imx219->mutex);
-
- return 0;
-}
-
static int imx219_set_ctrl(struct v4l2_ctrl *ctrl)
{
struct imx219 *imx219 =
@@ -802,6 +761,76 @@ static const struct v4l2_ctrl_ops imx219_ctrl_ops = {
.s_ctrl = imx219_set_ctrl,
};

+static void imx219_init_formats(struct v4l2_subdev_state *state)
+{
+ struct v4l2_mbus_framefmt *format;
+
+ format = v4l2_state_get_stream_format(state, 0, 0);
+ format->code = imx219_mbus_formats[0];
+ format->width = supported_modes[0].width;
+ format->height = supported_modes[0].height;
+ format->field = V4L2_FIELD_NONE;
+ format->colorspace = V4L2_COLORSPACE_RAW;
+
+ if (state->routing.routes[1].flags & V4L2_SUBDEV_ROUTE_FL_ACTIVE) {
+ format = v4l2_state_get_stream_format(state, 0, 1);
+ format->code = MEDIA_BUS_FMT_METADATA_8;
+ format->width = IMX219_EMBEDDED_LINE_WIDTH;
+ format->height = 1;
+ format->field = V4L2_FIELD_NONE;
+ format->colorspace = V4L2_COLORSPACE_DEFAULT;
+ }
+}
+
+static int _imx219_set_routing(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *state)
+{
+ struct v4l2_subdev_route routes[] = {
+ {
+ .source_pad = 0,
+ .source_stream = 0,
+ .flags = V4L2_SUBDEV_ROUTE_FL_IMMUTABLE |
+ V4L2_SUBDEV_ROUTE_FL_SOURCE |
+ V4L2_SUBDEV_ROUTE_FL_ACTIVE,
+ },
+ {
+ .source_pad = 0,
+ .source_stream = 1,
+ .flags = V4L2_SUBDEV_ROUTE_FL_SOURCE |
+ V4L2_SUBDEV_ROUTE_FL_ACTIVE,
+ }
+ };
+
+ struct v4l2_subdev_krouting routing = {
+ .num_routes = ARRAY_SIZE(routes),
+ .routes = routes,
+ };
+
+ int ret;
+
+ ret = v4l2_subdev_set_routing(sd, state, &routing);
+ if (ret)
+ return ret;
+
+ imx219_init_formats(state);
+
+ return 0;
+}
+
+static int imx219_init_cfg(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *state)
+{
+ int ret;
+
+ v4l2_subdev_lock_state(state);
+
+ ret = _imx219_set_routing(sd, state);
+
+ v4l2_subdev_unlock_state(state);
+
+ return ret;
+}
+
static int imx219_enum_mbus_code(struct v4l2_subdev *sd,
struct v4l2_subdev_state *sd_state,
struct v4l2_subdev_mbus_code_enum *code)
@@ -1255,11 +1284,12 @@ static const struct v4l2_subdev_video_ops imx219_video_ops = {
};

static const struct v4l2_subdev_pad_ops imx219_pad_ops = {
- .enum_mbus_code = imx219_enum_mbus_code,
- .get_fmt = imx219_get_pad_format,
- .set_fmt = imx219_set_pad_format,
- .get_selection = imx219_get_selection,
- .enum_frame_size = imx219_enum_frame_size,
+ .init_cfg = imx219_init_cfg,
+ .enum_mbus_code = imx219_enum_mbus_code,
+ .get_fmt = imx219_get_pad_format,
+ .set_fmt = imx219_set_pad_format,
+ .get_selection = imx219_get_selection,
+ .enum_frame_size = imx219_enum_frame_size,
};

static const struct v4l2_subdev_ops imx219_subdev_ops = {
@@ -1268,10 +1298,6 @@ static const struct v4l2_subdev_ops imx219_subdev_ops = {
.pad = &imx219_pad_ops,
};

-static const struct v4l2_subdev_internal_ops imx219_internal_ops = {
- .open = imx219_open,
-};
-
/* Initialize control handlers */
static int imx219_init_controls(struct imx219 *imx219)
{
@@ -1520,7 +1546,6 @@ static int imx219_probe(struct i2c_client *client)
goto error_power_off;

/* Initialize subdev */
- imx219->sd.internal_ops = &imx219_internal_ops;
imx219->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
V4L2_SUBDEV_FL_HAS_EVENTS;
imx219->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
@@ -1528,9 +1553,6 @@ static int imx219_probe(struct i2c_client *client)
/* Initialize source pad */
imx219->pad.flags = MEDIA_PAD_FL_SOURCE;

- /* Initialize default format */
- imx219_set_default_format(imx219);
-
ret = media_entity_pads_init(&imx219->sd.entity, 1, &imx219->pad);
if (ret) {
dev_err(dev, "failed to init entity pads: %d\n", ret);
--
2.32.0

2022-02-04 22:37:58

by Jean-Michel Hautbois

[permalink] [raw]
Subject: [RFC PATCH v4 07/12] media: imx219: Rename mbus codes array

The imx219 is using the name codes[] for the mbus format which is not
easy to read and know what it means. Change it to imx219_mbus_formats.

Signed-off-by: Jean-Michel Hautbois <[email protected]>
---
drivers/media/i2c/imx219.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
index e10af3f74b38..74dba5e61201 100644
--- a/drivers/media/i2c/imx219.c
+++ b/drivers/media/i2c/imx219.c
@@ -429,7 +429,7 @@ static const char * const imx219_supply_name[] = {
* - v flip
* - h&v flips
*/
-static const u32 codes[] = {
+static const u32 imx219_mbus_formats[] = {
MEDIA_BUS_FMT_SRGGB10_1X10,
MEDIA_BUS_FMT_SGRBG10_1X10,
MEDIA_BUS_FMT_SGBRG10_1X10,
@@ -655,17 +655,17 @@ static u32 imx219_get_format_code(struct imx219 *imx219, u32 code)

lockdep_assert_held(&imx219->mutex);

- for (i = 0; i < ARRAY_SIZE(codes); i++)
- if (codes[i] == code)
+ for (i = 0; i < ARRAY_SIZE(imx219_mbus_formats); i++)
+ if (imx219_mbus_formats[i] == code)
break;

- if (i >= ARRAY_SIZE(codes))
+ if (i >= ARRAY_SIZE(imx219_mbus_formats))
i = 0;

i = (i & ~3) | (imx219->vflip->val ? 2 : 0) |
(imx219->hflip->val ? 1 : 0);

- return codes[i];
+ return imx219_mbus_formats[i];
}

static void imx219_set_default_format(struct imx219 *imx219)
@@ -808,11 +808,11 @@ static int imx219_enum_mbus_code(struct v4l2_subdev *sd,
{
struct imx219 *imx219 = to_imx219(sd);

- if (code->index >= (ARRAY_SIZE(codes) / 4))
+ if (code->index >= (ARRAY_SIZE(imx219_mbus_formats) / 4))
return -EINVAL;

mutex_lock(&imx219->mutex);
- code->code = imx219_get_format_code(imx219, codes[code->index * 4]);
+ code->code = imx219_get_format_code(imx219, imx219_mbus_formats[code->index * 4]);
mutex_unlock(&imx219->mutex);

return 0;
@@ -908,14 +908,14 @@ static int imx219_set_pad_format(struct v4l2_subdev *sd,

mutex_lock(&imx219->mutex);

- for (i = 0; i < ARRAY_SIZE(codes); i++)
- if (codes[i] == fmt->format.code)
+ for (i = 0; i < ARRAY_SIZE(imx219_mbus_formats); i++)
+ if (imx219_mbus_formats[i] == fmt->format.code)
break;
- if (i >= ARRAY_SIZE(codes))
+ if (i >= ARRAY_SIZE(imx219_mbus_formats))
i = 0;

/* Bayer order varies with flips */
- fmt->format.code = imx219_get_format_code(imx219, codes[i]);
+ fmt->format.code = imx219_get_format_code(imx219, imx219_mbus_formats[i]);

mode = v4l2_find_nearest_size(supported_modes,
ARRAY_SIZE(supported_modes),
--
2.32.0

2022-02-04 22:44:49

by Jean-Michel Hautbois

[permalink] [raw]
Subject: [RFC PATCH v4 12/12] media: bcm283x: Include the imx219 node

WARNING:
This patch is only used to demonstrate how the imx219 node is included
in the bcm2711-rpi-4-b device tree, and is not intended to be merged.

Configure the csi1 endpoint, add the imx219 node and connect it through
the i2c mux.

Signed-off-by: Jean-Michel Hautbois <[email protected]>
---
arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 1 +
arch/arm/boot/dts/bcm283x-rpi-imx219.dtsi | 102 ++++++++++++++++++++++
2 files changed, 103 insertions(+)
create mode 100644 arch/arm/boot/dts/bcm283x-rpi-imx219.dtsi

diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
index 4432412044de..f7625b70fe57 100644
--- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
+++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
@@ -4,6 +4,7 @@
#include "bcm2711-rpi.dtsi"
#include "bcm283x-rpi-usb-peripheral.dtsi"
#include "bcm283x-rpi-wifi-bt.dtsi"
+#include "bcm283x-rpi-imx219.dtsi"

/ {
compatible = "raspberrypi,4-model-b", "brcm,bcm2711";
diff --git a/arch/arm/boot/dts/bcm283x-rpi-imx219.dtsi b/arch/arm/boot/dts/bcm283x-rpi-imx219.dtsi
new file mode 100644
index 000000000000..f2c6a85fd731
--- /dev/null
+++ b/arch/arm/boot/dts/bcm283x-rpi-imx219.dtsi
@@ -0,0 +1,102 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <dt-bindings/clock/bcm2835.h>
+
+/ {
+ compatible = "brcm,bcm2835";
+
+ imx219_vdig: fixedregulator@1 {
+ compatible = "regulator-fixed";
+ regulator-name = "imx219_vdig";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ };
+
+ imx219_vddl: fixedregulator@2 {
+ compatible = "regulator-fixed";
+ regulator-name = "imx219_vddl";
+ regulator-min-microvolt = <1200000>;
+ regulator-max-microvolt = <1200000>;
+ };
+
+ imx219_clk: imx219_clk {
+ #clock-cells = <0>;
+ compatible = "fixed-clock";
+ clock-frequency = <24000000>;
+ clock-output-names = "24MHz-clock";
+ };
+
+ cam1_reg: cam1_reg {
+ compatible = "regulator-fixed";
+ regulator-name = "imx219_vana";
+ enable-active-high;
+ status = "okay";
+ gpio = <&expgpio 5 GPIO_ACTIVE_HIGH>;
+ };
+
+ i2c0mux {
+ compatible = "i2c-mux-pinctrl";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ i2c-parent = <&i2c0>;
+
+ pinctrl-names = "i2c0", "i2c_csi_dsi";
+ pinctrl-0 = <&i2c0_gpio0>;
+ pinctrl-1 = <&i2c0_gpio44>;
+
+ i2c@0 {
+ reg = <0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+
+ i2c@1 {
+ reg = <1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ imx219: sensor@10 {
+ compatible = "sony,imx219";
+ reg = <0x10>;
+ status = "okay";
+
+ clocks = <&imx219_clk>;
+ clock-names = "xclk";
+
+ VANA-supply = <&cam1_reg>; /* 2.8v */
+ VDIG-supply = <&imx219_vdig>; /* 1.8v */
+ VDDL-supply = <&imx219_vddl>; /* 1.2v */
+
+ rotation = <0>;
+ orientation = <0>;
+
+ port {
+ imx219_0: endpoint {
+ remote-endpoint = <&csi1_ep>;
+ clock-lanes = <0>;
+ data-lanes = <1 2>;
+ clock-noncontinuous;
+ link-frequencies = /bits/ 64 <456000000>;
+ };
+ };
+ };
+ };
+ };
+};
+
+&csi1 {
+ status="okay";
+ num-data-lanes = <2>;
+ port {
+ csi1_ep: endpoint {
+ remote-endpoint = <&imx219_0>;
+ data-lanes = <1 2>;
+ clock-lanes = <0>;
+ };
+ };
+};
+
+&i2c0 {
+ /delete-property/ pinctrl-names;
+ /delete-property/ pinctrl-0;
+};
+
--
2.32.0

2022-02-06 16:00:46

by Laurent Pinchart

[permalink] [raw]
Subject: Re: [RFC PATCH v4 12/12] media: bcm283x: Include the imx219 node

Hi Jean-Michel,

Thank you for the patch.

On Thu, Feb 03, 2022 at 06:50:09PM +0100, Jean-Michel Hautbois wrote:
> WARNING:
> This patch is only used to demonstrate how the imx219 node is included
> in the bcm2711-rpi-4-b device tree, and is not intended to be merged.

You can add a [DNI] (it stands for Do Not Integrate) tag at the
beginning of the subject line to indicate this.

> Configure the csi1 endpoint, add the imx219 node and connect it through
> the i2c mux.
>
> Signed-off-by: Jean-Michel Hautbois <[email protected]>
> ---
> arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 1 +
> arch/arm/boot/dts/bcm283x-rpi-imx219.dtsi | 102 ++++++++++++++++++++++
> 2 files changed, 103 insertions(+)
> create mode 100644 arch/arm/boot/dts/bcm283x-rpi-imx219.dtsi
>
> diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> index 4432412044de..f7625b70fe57 100644
> --- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> +++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> @@ -4,6 +4,7 @@
> #include "bcm2711-rpi.dtsi"
> #include "bcm283x-rpi-usb-peripheral.dtsi"
> #include "bcm283x-rpi-wifi-bt.dtsi"
> +#include "bcm283x-rpi-imx219.dtsi"

Drop this, and turn arch/arm/boot/dts/bcm283x-rpi-imx219.dtsi into an
overlay. That can be merged in mainline. It can be done separately from
this series too.

>
> / {
> compatible = "raspberrypi,4-model-b", "brcm,bcm2711";
> diff --git a/arch/arm/boot/dts/bcm283x-rpi-imx219.dtsi b/arch/arm/boot/dts/bcm283x-rpi-imx219.dtsi
> new file mode 100644
> index 000000000000..f2c6a85fd731
> --- /dev/null
> +++ b/arch/arm/boot/dts/bcm283x-rpi-imx219.dtsi
> @@ -0,0 +1,102 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <dt-bindings/clock/bcm2835.h>

You also need a header for GPIO_ACTIVE_HIGH.

> +
> +/ {
> + compatible = "brcm,bcm2835";
> +
> + imx219_vdig: fixedregulator@1 {
> + compatible = "regulator-fixed";
> + regulator-name = "imx219_vdig";
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + };
> +
> + imx219_vddl: fixedregulator@2 {
> + compatible = "regulator-fixed";
> + regulator-name = "imx219_vddl";
> + regulator-min-microvolt = <1200000>;
> + regulator-max-microvolt = <1200000>;
> + };
> +
> + imx219_clk: imx219_clk {
> + #clock-cells = <0>;
> + compatible = "fixed-clock";
> + clock-frequency = <24000000>;
> + clock-output-names = "24MHz-clock";
> + };
> +
> + cam1_reg: cam1_reg {
> + compatible = "regulator-fixed";
> + regulator-name = "imx219_vana";
> + enable-active-high;
> + status = "okay";
> + gpio = <&expgpio 5 GPIO_ACTIVE_HIGH>;
> + };
> +
> + i2c0mux {
> + compatible = "i2c-mux-pinctrl";

The mux shouldn't be part of this overlay, this has been discussed
separately.

> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + i2c-parent = <&i2c0>;
> +
> + pinctrl-names = "i2c0", "i2c_csi_dsi";
> + pinctrl-0 = <&i2c0_gpio0>;
> + pinctrl-1 = <&i2c0_gpio44>;
> +
> + i2c@0 {
> + reg = <0>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> +
> + i2c@1 {
> + reg = <1>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + imx219: sensor@10 {
> + compatible = "sony,imx219";
> + reg = <0x10>;
> + status = "okay";
> +
> + clocks = <&imx219_clk>;
> + clock-names = "xclk";
> +
> + VANA-supply = <&cam1_reg>; /* 2.8v */
> + VDIG-supply = <&imx219_vdig>; /* 1.8v */
> + VDDL-supply = <&imx219_vddl>; /* 1.2v */
> +
> + rotation = <0>;
> + orientation = <0>;
> +
> + port {
> + imx219_0: endpoint {
> + remote-endpoint = <&csi1_ep>;
> + clock-lanes = <0>;
> + data-lanes = <1 2>;
> + clock-noncontinuous;
> + link-frequencies = /bits/ 64 <456000000>;
> + };
> + };
> + };
> + };
> + };
> +};
> +
> +&csi1 {
> + status="okay";

Missing spaces around =.

> + num-data-lanes = <2>;
> + port {
> + csi1_ep: endpoint {
> + remote-endpoint = <&imx219_0>;
> + data-lanes = <1 2>;
> + clock-lanes = <0>;
> + };
> + };
> +};
> +
> +&i2c0 {
> + /delete-property/ pinctrl-names;
> + /delete-property/ pinctrl-0;
> +};
> +

--
Regards,

Laurent Pinchart

2022-02-07 06:18:12

by Jean-Michel Hautbois

[permalink] [raw]
Subject: [RFC PATCH v4 06/12] ARM: dts: bcm2711: Add unicam CSI nodes

Add both MIPI CSI-2 nodes in the core bcm2711 tree. Use the 3-cells
interrupt declaration, corresponding clocks and default as disabled.

Signed-off-by: Jean-Michel Hautbois <[email protected]>
---
arch/arm/boot/dts/bcm2711-rpi.dtsi | 15 +++++++++++++++
arch/arm/boot/dts/bcm2711.dtsi | 16 ++++++++++++++++
2 files changed, 31 insertions(+)

diff --git a/arch/arm/boot/dts/bcm2711-rpi.dtsi b/arch/arm/boot/dts/bcm2711-rpi.dtsi
index ca266c5d9f9b..97ee494891af 100644
--- a/arch/arm/boot/dts/bcm2711-rpi.dtsi
+++ b/arch/arm/boot/dts/bcm2711-rpi.dtsi
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
#include "bcm2835-rpi.dtsi"

+#include <dt-bindings/power/raspberrypi-power.h>
#include <dt-bindings/reset/raspberrypi,firmware-reset.h>

/ {
@@ -18,6 +19,20 @@ aliases {
};
};

+&csi0 {
+ clocks = <&clocks BCM2835_CLOCK_CAM0>,
+ <&firmware_clocks 4>;
+ clock-names = "lp", "vpu";
+ power-domains = <&power RPI_POWER_DOMAIN_UNICAM0>;
+};
+
+&csi1 {
+ clocks = <&clocks BCM2835_CLOCK_CAM1>,
+ <&firmware_clocks 4>;
+ clock-names = "lp", "vpu";
+ power-domains = <&power RPI_POWER_DOMAIN_UNICAM1>;
+};
+
&firmware {
firmware_clocks: clocks {
compatible = "raspberrypi,firmware-clocks";
diff --git a/arch/arm/boot/dts/bcm2711.dtsi b/arch/arm/boot/dts/bcm2711.dtsi
index dff18fc9a906..312a74601839 100644
--- a/arch/arm/boot/dts/bcm2711.dtsi
+++ b/arch/arm/boot/dts/bcm2711.dtsi
@@ -293,6 +293,22 @@ hvs: hvs@7e400000 {
interrupts = <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>;
};

+ csi0: csi@7e800000 {
+ compatible = "brcm,bcm2835-unicam";
+ reg = <0x7e800000 0x800>,
+ <0x7e802000 0x4>;
+ interrupts = <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>;
+ status = "disabled";
+ };
+
+ csi1: csi@7e801000 {
+ compatible = "brcm,bcm2835-unicam";
+ reg = <0x7e801000 0x800>,
+ <0x7e802004 0x4>;
+ interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
+ status = "disabled";
+ };
+
pixelvalve3: pixelvalve@7ec12000 {
compatible = "brcm,bcm2711-pixelvalve3";
reg = <0x7ec12000 0x100>;
--
2.32.0