| Message ID | 20220203175009.558868-1-jeanmichel.hautbois@ideasonboard.com |
|---|---|
| Headers | show |
| Series | Add support for BCM2835 camera interface (unicam) | expand |
Hi Jean-Michel, Am 03.02.22 um 18:49 schrieb Jean-Michel Hautbois: > 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. > i understand, that you want to have this integrated fast but you send v3 of this series yesterday. This is not enough time for reviewers. Usually 1 to 2 weeks. Best regards
Hi Jean-Michel, Am 03.02.22 um 18:50 schrieb Jean-Michel Hautbois: > 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 <jeanmichel.hautbois@ideasonboard.com> > --- > 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>; after you added the reg-names to the binding, please don't forget to add them here ... > + interrupts = <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>; > + status = "disabled"; > + }; > + > + csi1: csi@7e801000 { > + compatible = "brcm,bcm2835-unicam"; > + reg = <0x7e801000 0x800>, > + <0x7e802004 0x4>; and here. Otherwise this looks good to me. Best regards > + interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>; > + status = "disabled"; > + }; > + > pixelvalve3: pixelvalve@7ec12000 { > compatible = "brcm,bcm2711-pixelvalve3"; > reg = <0x7ec12000 0x100>;
Hi Stefan, On Thu, Feb 03, 2022 at 08:55:42PM +0100, Stefan Wahren wrote: > Am 03.02.22 um 18:49 schrieb Jean-Michel Hautbois: > > 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. > > i understand, that you want to have this integrated fast but you send v3 > of this series yesterday. This is not enough time for reviewers. Usually > 1 to 2 weeks. v3 was incorrectly based on the out-of-tree ISP driver, which caused some patches to not apply correctly on top of mainline. I've asked Jean-Michel to fix this and send a v4, as it was difficult to review v3.
Hi Jean-Michel, Thank you for the patch. On Thu, Feb 03, 2022 at 06:50:00PM +0100, Jean-Michel Hautbois wrote: > Introduce the dt-bindings documentation for bcm2835 CCP2/CSI2 Unicam > camera interface. Also add a MAINTAINERS entry for it. You can drop the last sentence now that the MAINTAINERS entry has been moved out. > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > > --- > v4: > - make MAINTAINERS its own patch > - describe the reg and clocks correctly > - use a vendor entry for the number of data lanes > --- > .../bindings/media/brcm,bcm2835-unicam.yaml | 110 ++++++++++++++++++ > 1 file changed, 110 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml > > diff --git a/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml b/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml > new file mode 100644 > index 000000000000..0725a0267c60 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml > @@ -0,0 +1,110 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/media/brcm,bcm2835-unicam.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Broadcom BCM283x Camera Interface (Unicam) > + > +maintainers: > + - Raspberry Pi Kernel Maintenance <kernel-list@raspberrypi.com> > + > +description: |- > + The Unicam block on BCM283x SoCs is the receiver for either > + CSI-2 or CCP2 data from image sensors or similar devices. > + > + The main platform using this SoC is the Raspberry Pi family of boards. > + On the Pi the VideoCore firmware can also control this hardware block, > + and driving it from two different processors will cause issues. > + To avoid this, the firmware checks the device tree configuration > + during boot. If it finds device tree nodes starting by csi then "[...] device tree nodes whose name starts with 'csi' then [...]" > + it will stop the firmware accessing the block, and it can then > + safely be used via the device tree binding. > + > +properties: > + compatible: > + const: brcm,bcm2835-unicam > + > + reg: > + items: > + - description: Unicam block. > + - description: Clock Manager Image (CMI) block. As Stefan pointed out, you need reg-names: items: - const: main - const: cmi Alternatives for main could be core or unicam. Dave, do you have a preference ? > + > + interrupts: > + maxItems: 1 > + > + clocks: > + items: > + - description: Clock to drive the LP state machine of Unicam. > + - description: Clock for the vpu (core clock). s/vpu/VPU/ > + > + clock-names: > + items: > + - const: lp > + - const: vpu > + > + power-domains: > + items: > + - description: Unicam power domain > + > + brcm,num-data-lanes: > + $ref: /schemas/types.yaml#/definitions/uint32 > + enum: [ 2, 4 ] > + description: Number of data lanes on the csi bus I'd write description: | Number of CSI-2 data lanes supported by this Unicam instance. The number of data lanes actively used is specified with the data-lanes endpoint property. > + > + port: > + additionalProperties: false Shouldn't this be unevaluatedProperties ? I would also put it after the $ref line. > + $ref: /schemas/graph.yaml#/$defs/port-base > + > + properties: > + endpoint: > + $ref: /schemas/media/video-interfaces.yaml# > + unevaluatedProperties: false > + > + properties: > + data-lanes: true > + link-frequencies: true link-frequencies is specified on the sensor side, not here. You can drop it. > + > + required: > + - data-lanes > + - link-frequencies > + > + required: > + - endpoint > + > +required: > + - compatible > + - reg > + - interrupts > + - clocks > + - clock-names > + - power-domains > + - brcm,num-data-lanes > + - port > + > +additionalProperties: False > + > +examples: > + - | > + #include <dt-bindings/clock/bcm2835.h> > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + #include <dt-bindings/power/raspberrypi-power.h> > + csi1: csi@7e801000 { > + compatible = "brcm,bcm2835-unicam"; > + reg = <0x7e801000 0x800>, > + <0x7e802004 0x4>; > + interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&clocks BCM2835_CLOCK_CAM1>, > + <&firmware_clocks 4>; > + clock-names = "lp", "vpu"; > + power-domains = <&power RPI_POWER_DOMAIN_UNICAM1>; > + brcm,num-data-lanes = <2>; > + port { > + csi1_ep: endpoint { Wrong indentation. > + remote-endpoint = <&imx219_0>; > + data-lanes = <1 2>; > + link-frequencies = /bits/ 64 <456000000>; Drop link-frequencies here too. With these small issues fixed, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + }; > + }; > + }; > +...
Hi Jean-Michel, Thank you for the patch. On Thu, Feb 03, 2022 at 06:50:05PM +0100, Jean-Michel Hautbois wrote: > 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 <jeanmichel.hautbois@ideasonboard.com> > --- > 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 Align the values like done for the previous macros. > + > 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); Adding routing support isn't mentioned in the commit message. Stick to switching to .init_cfg() in this patch, and add routing support separately. > + > + 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); This isn't right, you're removing initialization of the default format. Patch 11/12 fixes it, but this one breaks bisection, which isn't good. > - > ret = media_entity_pads_init(&imx219->sd.entity, 1, &imx219->pad); > if (ret) { > dev_err(dev, "failed to init entity pads: %d\n", ret);
Am Donnerstag, 3. Februar 2022, 18:50:00 CET schrieb Jean-Michel Hautbois: > Introduce the dt-bindings documentation for bcm2835 CCP2/CSI2 Unicam > camera interface. Also add a MAINTAINERS entry for it. > > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > > --- > v4: > - make MAINTAINERS its own patch > - describe the reg and clocks correctly > - use a vendor entry for the number of data lanes > --- > .../bindings/media/brcm,bcm2835-unicam.yaml | 110 ++++++++++++++++++ > 1 file changed, 110 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml > > diff --git > a/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml > b/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml new file > mode 100644 > index 000000000000..0725a0267c60 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml > @@ -0,0 +1,110 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/media/brcm,bcm2835-unicam.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Broadcom BCM283x Camera Interface (Unicam) > + > +maintainers: > + - Raspberry Pi Kernel Maintenance <kernel-list@raspberrypi.com> > + > +description: |- > + The Unicam block on BCM283x SoCs is the receiver for either > + CSI-2 or CCP2 data from image sensors or similar devices. > + > + The main platform using this SoC is the Raspberry Pi family of boards. > + On the Pi the VideoCore firmware can also control this hardware block, > + and driving it from two different processors will cause issues. > + To avoid this, the firmware checks the device tree configuration > + during boot. If it finds device tree nodes starting by csi then > + it will stop the firmware accessing the block, and it can then > + safely be used via the device tree binding. > + > +properties: > + compatible: > + const: brcm,bcm2835-unicam > + > + reg: > + items: > + - description: Unicam block. > + - description: Clock Manager Image (CMI) block. > + > + interrupts: > + maxItems: 1 > + > + clocks: > + items: > + - description: Clock to drive the LP state machine of Unicam. > + - description: Clock for the vpu (core clock). > + > + clock-names: > + items: > + - const: lp > + - const: vpu > + > + power-domains: > + items: > + - description: Unicam power domain > + > + brcm,num-data-lanes: > + $ref: /schemas/types.yaml#/definitions/uint32 > + enum: [ 2, 4 ] > + description: Number of data lanes on the csi bus There is already data-lanes in Documentation/devicetree/bindings/media/video- interfaces.yaml. AFAICS these two are identical. Can't the video- interface.yaml be used for this? I'm no expert TBH. Regards, Alexander
Hi Alexander, On Fri, Feb 04, 2022 at 09:50:06AM +0100, Alexander Stein wrote: > Am Donnerstag, 3. Februar 2022, 18:50:00 CET schrieb Jean-Michel Hautbois: > > Introduce the dt-bindings documentation for bcm2835 CCP2/CSI2 Unicam > > camera interface. Also add a MAINTAINERS entry for it. > > > > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > > > > --- > > v4: > > - make MAINTAINERS its own patch > > - describe the reg and clocks correctly > > - use a vendor entry for the number of data lanes > > --- > > .../bindings/media/brcm,bcm2835-unicam.yaml | 110 ++++++++++++++++++ > > 1 file changed, 110 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml > > > > diff --git > > a/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml > > b/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml new file > > mode 100644 > > index 000000000000..0725a0267c60 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml > > @@ -0,0 +1,110 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/media/brcm,bcm2835-unicam.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Broadcom BCM283x Camera Interface (Unicam) > > + > > +maintainers: > > + - Raspberry Pi Kernel Maintenance <kernel-list@raspberrypi.com> > > + > > +description: |- > > + The Unicam block on BCM283x SoCs is the receiver for either > > + CSI-2 or CCP2 data from image sensors or similar devices. > > + > > + The main platform using this SoC is the Raspberry Pi family of boards. > > + On the Pi the VideoCore firmware can also control this hardware block, > > + and driving it from two different processors will cause issues. > > + To avoid this, the firmware checks the device tree configuration > > + during boot. If it finds device tree nodes starting by csi then > > + it will stop the firmware accessing the block, and it can then > > + safely be used via the device tree binding. > > + > > +properties: > > + compatible: > > + const: brcm,bcm2835-unicam > > + > > + reg: > > + items: > > + - description: Unicam block. > > + - description: Clock Manager Image (CMI) block. > > + > > + interrupts: > > + maxItems: 1 > > + > > + clocks: > > + items: > > + - description: Clock to drive the LP state machine of Unicam. > > + - description: Clock for the vpu (core clock). > > + > > + clock-names: > > + items: > > + - const: lp > > + - const: vpu > > + > > + power-domains: > > + items: > > + - description: Unicam power domain > > + > > + brcm,num-data-lanes: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + enum: [ 2, 4 ] > > + description: Number of data lanes on the csi bus > > There is already data-lanes in Documentation/devicetree/bindings/media/video- > interfaces.yaml. AFAICS these two are identical. Can't the video- > interface.yaml be used for this? I'm no expert TBH. This is the number of data lanes that the Unicam instance supports. There are two Unicam instances, and they can have 2 or 4 data lanes depending on the SoC. The data-lanes endpoint property indicates the number of lanes used on a particular board.
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