[PATCH v6 0/3] media: imx: add support for imx8mq MIPI RX

[PATCH v6 0/3] media: imx: add support for imx8mq MIPI RX

[Martin Kepplinger]

hi,

This patch series adds a driver for the i.MX8MQ CSI MIPI receiver / controller.

It includes the driver, the dt-bindings and the DT addition to the SoC dtsi.
I test it using libcamera. Thanks to Laurent who helped a lot. I'm happy for
any feedback,

                           martin

revision history
----------------
v6: (thank you Laurent and Rob)
* add reviewed-by tag to binding
* statically allocate clk_bulk_data
* fix how the hs_settle value is applied
* remove s_power calls
* remove the link_setup() callback implementation and make the link immutable
* more cleanups according to Laurents' review from v5

v5: (thank you Laurent)
* fix reset usage by using the already supported reset controller driver
* remove clko2 (totally unrelated clock / had been included by accident)
* rename pxl clock to ui
https://lore.kernel.org/linux-media/20210618095753.114557-1-martin.kepplinger@puri.sm/

v4: (thank you Rob and Marco)
* create fsl,mipi-phy-gpr custom dt property instead of confusing "phy"
* add imx8mq-specific compatibile to imx8mq.dtsi for future use
https://lore.kernel.org/linux-media/20210614121522.2944593-1-martin.kepplinger@puri.sm/

v3: (thank you, Rob and Laurent)
among minor other things according to v2 review, changes include:
* better describe the clocks
* rename DT property "phy-reset" to "reset" and "phy-gpr" to "phy"
https://lore.kernel.org/linux-media/20210608104128.1616028-1-martin.kepplinger@puri.sm/T/#t

v2: (thank you, Dan and Guido)
among fixes according to v1 reviews, changes include:
* remove status property from dt-bindings example
* define a few bits in order to have less magic values
* use "imx8mq_mipi_csi_" as local function prefix
* read DT properties only during probe()
* remove dead code (log_status)
* add imx8mq_mipi_csi_release_icc()
* fix imx8mq_mipi_csi_init_icc()
https://lore.kernel.org/linux-media/20210531112326.90094-1-martin.kepplinger@puri.sm/

v1:
https://lore.kernel.org/linux-media/20210527075407.3180744-1-martin.kepplinger@puri.sm/T/#t


Martin Kepplinger (3):
  dt-bindings: media: document the nxp,imx8mq-mipi-csi2 receiver phy and
    controller
  media: imx: add a driver for i.MX8MQ mipi csi rx phy and controller
  arm64: dts: imx8mq: add mipi csi phy and csi bridge descriptions

 .../bindings/media/nxp,imx8mq-mipi-csi2.yaml  | 173 ++++
 arch/arm64/boot/dts/freescale/imx8mq.dtsi     | 104 ++
 drivers/staging/media/imx/Makefile            |   1 +
 drivers/staging/media/imx/imx8mq-mipi-csi2.c  | 949 ++++++++++++++++++
 4 files changed, 1227 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
 create mode 100644 drivers/staging/media/imx/imx8mq-mipi-csi2.c

-- 
2.30.2

[PATCH v6 1/3] dt-bindings: media: document the nxp,imx8mq-mipi-csi2 receiver phy and controller

[Martin Kepplinger]

The i.MX8MQ SoC integrates a different MIPI CSI receiver as the i.MX8MM so
describe the DT bindings for it.

Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../bindings/media/nxp,imx8mq-mipi-csi2.yaml  | 173 ++++++++++++++++++
 1 file changed, 173 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml

diff --git a/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml b/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
new file mode 100644
index 000000000000..97222485f223
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
@@ -0,0 +1,173 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/nxp,imx8mq-mipi-csi2.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP i.MX8MQ MIPI CSI-2 receiver
+
+maintainers:
+  - Martin Kepplinger <martin.kepplinger@puri.sm>
+
+description: |-
+  This binding covers the CSI-2 RX PHY and host controller included in the
+  NXP i.MX8MQ SoC. It handles the sensor/image input and process for all the
+  input imaging devices.
+
+properties:
+  compatible:
+    enum:
+      - fsl,imx8mq-mipi-csi2
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: core is the RX Controller Core Clock input. This clock
+                     must be exactly equal to or faster than the receive
+                     byteclock from the RX DPHY.
+      - description: esc is the Rx Escape Clock. This must be the same escape
+                     clock that the RX DPHY receives.
+      - description: ui is the pixel clock (phy_ref up to 333Mhz).
+                     See the reference manual for details.
+
+  clock-names:
+    items:
+      - const: core
+      - const: esc
+      - const: ui
+
+  power-domains:
+    maxItems: 1
+
+  resets:
+    items:
+      - description: CORE_RESET reset register bit definition
+      - description: PHY_REF_RESET reset register bit definition
+      - description: ESC_RESET reset register bit definition
+
+  fsl,mipi-phy-gpr:
+    description: |
+      The phandle to the imx8mq syscon iomux-gpr with the register
+      for setting RX_ENABLE for the mipi receiver.
+
+      The format should be as follows:
+      <gpr req_gpr>
+      gpr is the phandle to general purpose register node.
+      req_gpr is the gpr register offset of RX_ENABLE for the mipi phy.
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    items:
+      items:
+        - description: The 'gpr' is the phandle to general purpose register node.
+        - description: The 'req_gpr' is the gpr register offset containing
+                       CSI2_1_RX_ENABLE or CSI2_2_RX_ENABLE respectively.
+          maximum: 0xff
+
+  interconnects:
+    maxItems: 1
+
+  interconnect-names:
+    const: dram
+
+  ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+
+    properties:
+      port@0:
+        $ref: /schemas/graph.yaml#/$defs/port-base
+        unevaluatedProperties: false
+        description:
+          Input port node, single endpoint describing the CSI-2 transmitter.
+
+        properties:
+          endpoint:
+            $ref: video-interfaces.yaml#
+            unevaluatedProperties: false
+
+            properties:
+              data-lanes:
+                items:
+                  minItems: 1
+                  maxItems: 4
+                  items:
+                    - const: 1
+                    - const: 2
+                    - const: 3
+                    - const: 4
+
+            required:
+              - data-lanes
+
+      port@1:
+        $ref: /schemas/graph.yaml#/properties/port
+        description:
+          Output port node
+
+    required:
+      - port@0
+      - port@1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - power-domains
+  - resets
+  - fsl,mipi-phy-gpr
+  - ports
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/imx8mq-clock.h>
+    #include <dt-bindings/interconnect/imx8mq.h>
+
+    csi@30a70000 {
+        compatible = "fsl,imx8mq-mipi-csi2";
+        reg = <0x30a70000 0x1000>;
+        clocks = <&clk IMX8MQ_CLK_CSI1_CORE>,
+                 <&clk IMX8MQ_CLK_CSI1_ESC>,
+                 <&clk IMX8MQ_CLK_CSI1_PHY_REF>;
+        clock-names = "core", "esc", "ui";
+        assigned-clocks = <&clk IMX8MQ_CLK_CSI1_CORE>,
+                          <&clk IMX8MQ_CLK_CSI1_PHY_REF>,
+                          <&clk IMX8MQ_CLK_CSI1_ESC>;
+        assigned-clock-rates = <266000000>, <200000000>, <66000000>;
+        assigned-clock-parents = <&clk IMX8MQ_SYS1_PLL_266M>,
+                                 <&clk IMX8MQ_SYS2_PLL_1000M>,
+                                 <&clk IMX8MQ_SYS1_PLL_800M>;
+        power-domains = <&pgc_mipi_csi1>;
+        resets = <&src IMX8MQ_RESET_MIPI_CSI1_CORE_RESET>,
+                 <&src IMX8MQ_RESET_MIPI_CSI1_PHY_REF_RESET>,
+                 <&src IMX8MQ_RESET_MIPI_CSI1_ESC_RESET>;
+        fsl,mipi-phy-gpr = <&iomuxc_gpr 0x88>;
+        interconnects = <&noc IMX8MQ_ICM_CSI1 &noc IMX8MQ_ICS_DRAM>;
+        interconnect-names = "dram";
+
+        ports {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            port@0 {
+                reg = <0>;
+
+                imx8mm_mipi_csi_in: endpoint {
+                    remote-endpoint = <&imx477_out>;
+                    data-lanes = <1 2 3 4>;
+                };
+            };
+
+            port@1 {
+                reg = <1>;
+
+                imx8mm_mipi_csi_out: endpoint {
+                    remote-endpoint = <&csi_in>;
+                };
+            };
+        };
+    };
+
+...
-- 
2.30.2

Re: [PATCH v6 1/3] dt-bindings: media: document the nxp,imx8mq-mipi-csi2 receiver phy and controller

[Rob Herring]

On Wed, 14 Jul 2021 13:19:29 +0200, Martin Kepplinger wrote:
> The i.MX8MQ SoC integrates a different MIPI CSI receiver as the i.MX8MM so
> describe the DT bindings for it.
> 
> Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
>  .../bindings/media/nxp,imx8mq-mipi-csi2.yaml  | 173 ++++++++++++++++++
>  1 file changed, 173 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Error: Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.example.dts:37.28-29 syntax error
FATAL ERROR: Unable to parse input tree
make[1]: *** [scripts/Makefile.lib:380: Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.example.dt.yaml] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1418: dt_binding_check] Error 2
\ndoc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1505152

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [PATCH v6 1/3] dt-bindings: media: document the nxp,imx8mq-mipi-csi2 receiver phy and controller

[Laurent Pinchart]

Hi Martin,

Thank you for the patch.

On Wed, Jul 14, 2021 at 01:19:29PM +0200, Martin Kepplinger wrote:
> The i.MX8MQ SoC integrates a different MIPI CSI receiver as the i.MX8MM so
> describe the DT bindings for it.
> 
> Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
>  .../bindings/media/nxp,imx8mq-mipi-csi2.yaml  | 173 ++++++++++++++++++
>  1 file changed, 173 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
> 
> diff --git a/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml b/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
> new file mode 100644
> index 000000000000..97222485f223
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
> @@ -0,0 +1,173 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/nxp,imx8mq-mipi-csi2.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP i.MX8MQ MIPI CSI-2 receiver
> +
> +maintainers:
> +  - Martin Kepplinger <martin.kepplinger@puri.sm>
> +
> +description: |-
> +  This binding covers the CSI-2 RX PHY and host controller included in the
> +  NXP i.MX8MQ SoC. It handles the sensor/image input and process for all the
> +  input imaging devices.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - fsl,imx8mq-mipi-csi2
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: core is the RX Controller Core Clock input. This clock
> +                     must be exactly equal to or faster than the receive
> +                     byteclock from the RX DPHY.
> +      - description: esc is the Rx Escape Clock. This must be the same escape
> +                     clock that the RX DPHY receives.
> +      - description: ui is the pixel clock (phy_ref up to 333Mhz).

Where did you get the 333MHz limit from ? The information I've received
indicate a limit of 125MHz for the UI clock (and 266 and 133 MHz for the
core and esc clocks respectively).

With this addressed,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +                     See the reference manual for details.
> +
> +  clock-names:
> +    items:
> +      - const: core
> +      - const: esc
> +      - const: ui
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  resets:
> +    items:
> +      - description: CORE_RESET reset register bit definition
> +      - description: PHY_REF_RESET reset register bit definition
> +      - description: ESC_RESET reset register bit definition
> +
> +  fsl,mipi-phy-gpr:
> +    description: |
> +      The phandle to the imx8mq syscon iomux-gpr with the register
> +      for setting RX_ENABLE for the mipi receiver.
> +
> +      The format should be as follows:
> +      <gpr req_gpr>
> +      gpr is the phandle to general purpose register node.
> +      req_gpr is the gpr register offset of RX_ENABLE for the mipi phy.
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    items:
> +      items:
> +        - description: The 'gpr' is the phandle to general purpose register node.
> +        - description: The 'req_gpr' is the gpr register offset containing
> +                       CSI2_1_RX_ENABLE or CSI2_2_RX_ENABLE respectively.
> +          maximum: 0xff
> +
> +  interconnects:
> +    maxItems: 1
> +
> +  interconnect-names:
> +    const: dram
> +
> +  ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +
> +    properties:
> +      port@0:
> +        $ref: /schemas/graph.yaml#/$defs/port-base
> +        unevaluatedProperties: false
> +        description:
> +          Input port node, single endpoint describing the CSI-2 transmitter.
> +
> +        properties:
> +          endpoint:
> +            $ref: video-interfaces.yaml#
> +            unevaluatedProperties: false
> +
> +            properties:
> +              data-lanes:
> +                items:
> +                  minItems: 1
> +                  maxItems: 4
> +                  items:
> +                    - const: 1
> +                    - const: 2
> +                    - const: 3
> +                    - const: 4
> +
> +            required:
> +              - data-lanes
> +
> +      port@1:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description:
> +          Output port node
> +
> +    required:
> +      - port@0
> +      - port@1
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - power-domains
> +  - resets
> +  - fsl,mipi-phy-gpr
> +  - ports
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/imx8mq-clock.h>
> +    #include <dt-bindings/interconnect/imx8mq.h>
> +
> +    csi@30a70000 {
> +        compatible = "fsl,imx8mq-mipi-csi2";
> +        reg = <0x30a70000 0x1000>;
> +        clocks = <&clk IMX8MQ_CLK_CSI1_CORE>,
> +                 <&clk IMX8MQ_CLK_CSI1_ESC>,
> +                 <&clk IMX8MQ_CLK_CSI1_PHY_REF>;
> +        clock-names = "core", "esc", "ui";
> +        assigned-clocks = <&clk IMX8MQ_CLK_CSI1_CORE>,
> +                          <&clk IMX8MQ_CLK_CSI1_PHY_REF>,
> +                          <&clk IMX8MQ_CLK_CSI1_ESC>;
> +        assigned-clock-rates = <266000000>, <200000000>, <66000000>;
> +        assigned-clock-parents = <&clk IMX8MQ_SYS1_PLL_266M>,
> +                                 <&clk IMX8MQ_SYS2_PLL_1000M>,
> +                                 <&clk IMX8MQ_SYS1_PLL_800M>;
> +        power-domains = <&pgc_mipi_csi1>;
> +        resets = <&src IMX8MQ_RESET_MIPI_CSI1_CORE_RESET>,
> +                 <&src IMX8MQ_RESET_MIPI_CSI1_PHY_REF_RESET>,
> +                 <&src IMX8MQ_RESET_MIPI_CSI1_ESC_RESET>;
> +        fsl,mipi-phy-gpr = <&iomuxc_gpr 0x88>;
> +        interconnects = <&noc IMX8MQ_ICM_CSI1 &noc IMX8MQ_ICS_DRAM>;
> +        interconnect-names = "dram";
> +
> +        ports {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            port@0 {
> +                reg = <0>;
> +
> +                imx8mm_mipi_csi_in: endpoint {
> +                    remote-endpoint = <&imx477_out>;
> +                    data-lanes = <1 2 3 4>;
> +                };
> +            };
> +
> +            port@1 {
> +                reg = <1>;
> +
> +                imx8mm_mipi_csi_out: endpoint {
> +                    remote-endpoint = <&csi_in>;
> +                };
> +            };
> +        };
> +    };
> +
> +...

-- 
Regards,

Laurent Pinchart

Re: [PATCH v6 1/3] dt-bindings: media: document the nxp,imx8mq-mipi-csi2 receiver phy and controller

[Martin Kepplinger]

Am Mittwoch, dem 14.07.2021 um 20:53 +0300 schrieb Laurent Pinchart:
> Hi Martin,
> 
> Thank you for the patch.
> 
> On Wed, Jul 14, 2021 at 01:19:29PM +0200, Martin Kepplinger wrote:
> > The i.MX8MQ SoC integrates a different MIPI CSI receiver as the
> > i.MX8MM so
> > describe the DT bindings for it.
> > 
> > Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm>
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > ---
> >  .../bindings/media/nxp,imx8mq-mipi-csi2.yaml  | 173
> > ++++++++++++++++++
> >  1 file changed, 173 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/media/nxp,imx8mq-
> > mipi-csi2.yaml
> > b/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
> > new file mode 100644
> > index 000000000000..97222485f223
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-
> > csi2.yaml
> > @@ -0,0 +1,173 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: 
> > http://devicetree.org/schemas/media/nxp,imx8mq-mipi-csi2.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: NXP i.MX8MQ MIPI CSI-2 receiver
> > +
> > +maintainers:
> > +  - Martin Kepplinger <martin.kepplinger@puri.sm>
> > +
> > +description: |-
> > +  This binding covers the CSI-2 RX PHY and host controller
> > included in the
> > +  NXP i.MX8MQ SoC. It handles the sensor/image input and process
> > for all the
> > +  input imaging devices.
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - fsl,imx8mq-mipi-csi2
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    items:
> > +      - description: core is the RX Controller Core Clock input.
> > This clock
> > +                     must be exactly equal to or faster than the
> > receive
> > +                     byteclock from the RX DPHY.
> > +      - description: esc is the Rx Escape Clock. This must be the
> > same escape
> > +                     clock that the RX DPHY receives.
> > +      - description: ui is the pixel clock (phy_ref up to 333Mhz).
> 
> Where did you get the 333MHz limit from ? The information I've
> received
> indicate a limit of 125MHz for the UI clock (and 266 and 133 MHz for
> the
> core and esc clocks respectively).

The latest ref.manual revison has this corrected to 333Mhz. Look at the
"changelog" they have in the RM, to find it quickly.

> 
> With this addressed,
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> > +                     See the reference manual for details.
> > +
> > +  clock-names:
> > +    items:
> > +      - const: core
> > +      - const: esc
> > +      - const: ui
> > +
> > +  power-domains:
> > +    maxItems: 1
> > +
> > +  resets:
> > +    items:
> > +      - description: CORE_RESET reset register bit definition
> > +      - description: PHY_REF_RESET reset register bit definition
> > +      - description: ESC_RESET reset register bit definition
> > +
> > +  fsl,mipi-phy-gpr:
> > +    description: |
> > +      The phandle to the imx8mq syscon iomux-gpr with the register
> > +      for setting RX_ENABLE for the mipi receiver.
> > +
> > +      The format should be as follows:
> > +      <gpr req_gpr>
> > +      gpr is the phandle to general purpose register node.
> > +      req_gpr is the gpr register offset of RX_ENABLE for the mipi
> > phy.
> > +    $ref: /schemas/types.yaml#/definitions/phandle-array
> > +    items:
> > +      items:
> > +        - description: The 'gpr' is the phandle to general purpose
> > register node.
> > +        - description: The 'req_gpr' is the gpr register offset
> > containing
> > +                       CSI2_1_RX_ENABLE or CSI2_2_RX_ENABLE
> > respectively.
> > +          maximum: 0xff
> > +
> > +  interconnects:
> > +    maxItems: 1
> > +
> > +  interconnect-names:
> > +    const: dram
> > +
> > +  ports:
> > +    $ref: /schemas/graph.yaml#/properties/ports
> > +
> > +    properties:
> > +      port@0:
> > +        $ref: /schemas/graph.yaml#/$defs/port-base
> > +        unevaluatedProperties: false
> > +        description:
> > +          Input port node, single endpoint describing the CSI-2
> > transmitter.
> > +
> > +        properties:
> > +          endpoint:
> > +            $ref: video-interfaces.yaml#
> > +            unevaluatedProperties: false
> > +
> > +            properties:
> > +              data-lanes:
> > +                items:
> > +                  minItems: 1
> > +                  maxItems: 4
> > +                  items:
> > +                    - const: 1
> > +                    - const: 2
> > +                    - const: 3
> > +                    - const: 4
> > +
> > +            required:
> > +              - data-lanes
> > +
> > +      port@1:
> > +        $ref: /schemas/graph.yaml#/properties/port
> > +        description:
> > +          Output port node
> > +
> > +    required:
> > +      - port@0
> > +      - port@1
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - clocks
> > +  - clock-names
> > +  - power-domains
> > +  - resets
> > +  - fsl,mipi-phy-gpr
> > +  - ports
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/clock/imx8mq-clock.h>
> > +    #include <dt-bindings/interconnect/imx8mq.h>
> > +
> > +    csi@30a70000 {
> > +        compatible = "fsl,imx8mq-mipi-csi2";
> > +        reg = <0x30a70000 0x1000>;
> > +        clocks = <&clk IMX8MQ_CLK_CSI1_CORE>,
> > +                 <&clk IMX8MQ_CLK_CSI1_ESC>,
> > +                 <&clk IMX8MQ_CLK_CSI1_PHY_REF>;
> > +        clock-names = "core", "esc", "ui";
> > +        assigned-clocks = <&clk IMX8MQ_CLK_CSI1_CORE>,
> > +                          <&clk IMX8MQ_CLK_CSI1_PHY_REF>,
> > +                          <&clk IMX8MQ_CLK_CSI1_ESC>;
> > +        assigned-clock-rates = <266000000>, <200000000>,
> > <66000000>;
> > +        assigned-clock-parents = <&clk IMX8MQ_SYS1_PLL_266M>,
> > +                                 <&clk IMX8MQ_SYS2_PLL_1000M>,
> > +                                 <&clk IMX8MQ_SYS1_PLL_800M>;
> > +        power-domains = <&pgc_mipi_csi1>;
> > +        resets = <&src IMX8MQ_RESET_MIPI_CSI1_CORE_RESET>,
> > +                 <&src IMX8MQ_RESET_MIPI_CSI1_PHY_REF_RESET>,
> > +                 <&src IMX8MQ_RESET_MIPI_CSI1_ESC_RESET>;
> > +        fsl,mipi-phy-gpr = <&iomuxc_gpr 0x88>;
> > +        interconnects = <&noc IMX8MQ_ICM_CSI1 &noc
> > IMX8MQ_ICS_DRAM>;
> > +        interconnect-names = "dram";
> > +
> > +        ports {
> > +            #address-cells = <1>;
> > +            #size-cells = <0>;
> > +
> > +            port@0 {
> > +                reg = <0>;
> > +
> > +                imx8mm_mipi_csi_in: endpoint {
> > +                    remote-endpoint = <&imx477_out>;
> > +                    data-lanes = <1 2 3 4>;
> > +                };
> > +            };
> > +
> > +            port@1 {
> > +                reg = <1>;
> > +
> > +                imx8mm_mipi_csi_out: endpoint {
> > +                    remote-endpoint = <&csi_in>;
> > +                };
> > +            };
> > +        };
> > +    };
> > +
> > +...
> 

[PATCH v6 2/3] media: imx: add a driver for i.MX8MQ mipi csi rx phy and controller

[Martin Kepplinger]

Add a driver to support the i.MX8MQ MIPI CSI receiver. The hardware side
is based on
https://source.codeaurora.org/external/imx/linux-imx/tree/drivers/media/platform/imx8/mxc-mipi-csi2_yav.c?h=imx_5.4.70_2.3.0

It's built as part of VIDEO_IMX7_CSI because that's documented to support
i.MX8M platforms. This driver adds i.MX8MQ support where currently only the
i.MX8MM platform has been supported.

Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm>
---
 drivers/staging/media/imx/Makefile           |   1 +
 drivers/staging/media/imx/imx8mq-mipi-csi2.c | 949 +++++++++++++++++++
 2 files changed, 950 insertions(+)
 create mode 100644 drivers/staging/media/imx/imx8mq-mipi-csi2.c

diff --git a/drivers/staging/media/imx/Makefile b/drivers/staging/media/imx/Makefile
index 6ac33275cc97..19c2fc54d424 100644
--- a/drivers/staging/media/imx/Makefile
+++ b/drivers/staging/media/imx/Makefile
@@ -16,3 +16,4 @@ obj-$(CONFIG_VIDEO_IMX_CSI) += imx6-mipi-csi2.o
 
 obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-media-csi.o
 obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-mipi-csis.o
+obj-$(CONFIG_VIDEO_IMX7_CSI) += imx8mq-mipi-csi2.o
diff --git a/drivers/staging/media/imx/imx8mq-mipi-csi2.c b/drivers/staging/media/imx/imx8mq-mipi-csi2.c
new file mode 100644
index 000000000000..949b3ef7a20a
--- /dev/null
+++ b/drivers/staging/media/imx/imx8mq-mipi-csi2.c
@@ -0,0 +1,949 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Freescale i.MX8MQ SoC series MIPI-CSI2 receiver driver
+ *
+ * Copyright (C) 2021 Purism SPC
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/errno.h>
+#include <linux/interconnect.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/reset.h>
+#include <linux/spinlock.h>
+
+#include <media/v4l2-common.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-fwnode.h>
+#include <media/v4l2-mc.h>
+#include <media/v4l2-subdev.h>
+
+#define MIPI_CSI2_DRIVER_NAME			"imx8mq-mipi-csi2"
+#define MIPI_CSI2_SUBDEV_NAME			MIPI_CSI2_DRIVER_NAME
+
+#define MIPI_CSI2_PAD_SINK			0
+#define MIPI_CSI2_PAD_SOURCE			1
+#define MIPI_CSI2_PADS_NUM			2
+
+#define MIPI_CSI2_DEF_PIX_WIDTH			640
+#define MIPI_CSI2_DEF_PIX_HEIGHT		480
+
+/* Register map definition */
+
+/* i.MX8MQ CSI-2 controller CSR */
+#define CSI2RX_CFG_NUM_LANES			0x100
+#define CSI2RX_CFG_DISABLE_DATA_LANES		0x104
+#define CSI2RX_BIT_ERR				0x108
+#define CSI2RX_IRQ_STATUS			0x10c
+#define CSI2RX_IRQ_MASK				0x110
+#define CSI2RX_IRQ_MASK_ALL			0x1ff
+#define CSI2RX_IRQ_MASK_ULPS_STATUS_CHANGE	0x8
+#define CSI2RX_ULPS_STATUS			0x114
+#define CSI2RX_PPI_ERRSOT_HS			0x118
+#define CSI2RX_PPI_ERRSOTSYNC_HS		0x11c
+#define CSI2RX_PPI_ERRESC			0x120
+#define CSI2RX_PPI_ERRSYNCESC			0x124
+#define CSI2RX_PPI_ERRCONTROL			0x128
+#define CSI2RX_CFG_DISABLE_PAYLOAD_0		0x12c
+#define CSI2RX_CFG_VID_P_FIFO_SEND_LEVEL	0x188
+#define CSI2RX_CFG_DISABLE_PAYLOAD_1		0x130
+
+enum {
+	ST_POWERED	= 1,
+	ST_STREAMING	= 2,
+	ST_SUSPENDED	= 4,
+};
+
+static const char * const imx8mq_mipi_csi_clk_id[] = {
+	"core",
+	"esc",
+	"ui",
+};
+
+#define CSI2_NUM_CLKS	ARRAY_SIZE(imx8mq_mipi_csi_clk_id)
+
+#define	GPR_CSI2_1_RX_ENABLE		BIT(13)
+#define	GPR_CSI2_1_VID_INTFC_ENB	BIT(12)
+#define	GPR_CSI2_1_HSEL			BIT(10)
+#define	GPR_CSI2_1_CONT_CLK_MODE	BIT(8)
+#define	GPR_CSI2_1_S_PRG_RXHS_SETTLE(x)	(((x) & 0x3f) << 2)
+
+/*
+ * The send level configures the number of entries that must accumulate in
+ * the Pixel FIFO before the data will be transferred to the video output.
+ * See https://community.nxp.com/t5/i-MX-Processors/IMX8M-MIPI-CSI-Host-Controller-send-level/m-p/864005/highlight/true#M131704
+ */
+#define CSI2RX_SEND_LEVEL			64
+
+struct csi_state {
+	struct device *dev;
+	void __iomem *regs;
+	struct clk_bulk_data clks[CSI2_NUM_CLKS];
+	struct reset_control *rst;
+	struct regulator *mipi_phy_regulator;
+
+	struct v4l2_subdev sd;
+	struct media_pad pads[MIPI_CSI2_PADS_NUM];
+	struct v4l2_async_notifier notifier;
+	struct v4l2_subdev *src_sd;
+
+	struct v4l2_fwnode_bus_mipi_csi2 bus;
+
+	struct mutex lock; /* Protect csi2_fmt, format_mbus, state, hs_settle*/
+	const struct csi2_pix_format *csi2_fmt;
+	struct v4l2_mbus_framefmt format_mbus[MIPI_CSI2_PADS_NUM];
+	u32 state;
+	u32 hs_settle;
+
+	struct regmap *phy_gpr;
+	u8 phy_gpr_reg;
+
+	struct icc_path			*icc_path;
+	s32				icc_path_bw;
+};
+
+/* -----------------------------------------------------------------------------
+ * Format helpers
+ */
+
+struct csi2_pix_format {
+	u32 code;
+	u8 width;
+};
+
+static const struct csi2_pix_format imx8mq_mipi_csi_formats[] = {
+	/* RAW (Bayer and greyscale) formats. */
+	{
+		.code = MEDIA_BUS_FMT_SBGGR8_1X8,
+		.width = 8,
+	}, {
+		.code = MEDIA_BUS_FMT_SGBRG8_1X8,
+		.width = 8,
+	}, {
+		.code = MEDIA_BUS_FMT_SGRBG8_1X8,
+		.width = 8,
+	}, {
+		.code = MEDIA_BUS_FMT_SRGGB8_1X8,
+		.width = 8,
+	}, {
+		.code = MEDIA_BUS_FMT_Y8_1X8,
+		.width = 8,
+	}, {
+		.code = MEDIA_BUS_FMT_SBGGR10_1X10,
+		.width = 10,
+	}, {
+		.code = MEDIA_BUS_FMT_SGBRG10_1X10,
+		.width = 10,
+	}, {
+		.code = MEDIA_BUS_FMT_SGRBG10_1X10,
+		.width = 10,
+	}, {
+		.code = MEDIA_BUS_FMT_SRGGB10_1X10,
+		.width = 10,
+	}, {
+		.code = MEDIA_BUS_FMT_Y10_1X10,
+		.width = 10,
+	}, {
+		.code = MEDIA_BUS_FMT_SBGGR12_1X12,
+		.width = 12,
+	}, {
+		.code = MEDIA_BUS_FMT_SGBRG12_1X12,
+		.width = 12,
+	}, {
+		.code = MEDIA_BUS_FMT_SGRBG12_1X12,
+		.width = 12,
+	}, {
+		.code = MEDIA_BUS_FMT_SRGGB12_1X12,
+		.width = 12,
+	}, {
+		.code = MEDIA_BUS_FMT_Y12_1X12,
+		.width = 12,
+	}, {
+		.code = MEDIA_BUS_FMT_SBGGR14_1X14,
+		.width = 14,
+	}, {
+		.code = MEDIA_BUS_FMT_SGBRG14_1X14,
+		.width = 14,
+	}, {
+		.code = MEDIA_BUS_FMT_SGRBG14_1X14,
+		.width = 14,
+	}, {
+		.code = MEDIA_BUS_FMT_SRGGB14_1X14,
+		.width = 14,
+	}, {
+	/* YUV formats */
+		.code = MEDIA_BUS_FMT_YUYV8_2X8,
+		.width = 16,
+	}, {
+		.code = MEDIA_BUS_FMT_YUYV8_1X16,
+		.width = 16,
+	}
+};
+
+static const struct csi2_pix_format *find_csi2_format(u32 code)
+{
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(imx8mq_mipi_csi_formats); i++)
+		if (code == imx8mq_mipi_csi_formats[i].code)
+			return &imx8mq_mipi_csi_formats[i];
+	return NULL;
+}
+
+/* -----------------------------------------------------------------------------
+ * Hardware configuration
+ */
+
+static inline void imx8mq_mipi_csi_write(struct csi_state *state, u32 reg, u32 val)
+{
+	writel(val, state->regs + reg);
+}
+
+static int imx8mq_mipi_csi_sw_reset(struct csi_state *state)
+{
+	int ret;
+
+	ret = reset_control_assert(state->rst);
+	if (ret < 0) {
+		dev_err(state->dev, "Failed to assert resets: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static void imx8mq_mipi_csi_system_enable(struct csi_state *state, int on)
+{
+	if (!on) {
+		imx8mq_mipi_csi_write(state, CSI2RX_CFG_DISABLE_DATA_LANES, 0xf);
+		return;
+	}
+
+	regmap_update_bits(state->phy_gpr,
+			   state->phy_gpr_reg,
+			   0x3fff,
+			   GPR_CSI2_1_RX_ENABLE |
+			   GPR_CSI2_1_VID_INTFC_ENB |
+			   GPR_CSI2_1_HSEL |
+			   GPR_CSI2_1_CONT_CLK_MODE |
+			   GPR_CSI2_1_S_PRG_RXHS_SETTLE(state->hs_settle));
+}
+
+static void imx8mq_mipi_csi_set_params(struct csi_state *state)
+{
+	int lanes = state->bus.num_data_lanes;
+
+	imx8mq_mipi_csi_write(state, CSI2RX_CFG_NUM_LANES, lanes - 1);
+	imx8mq_mipi_csi_write(state, CSI2RX_CFG_DISABLE_DATA_LANES,
+			      (0xf << lanes) & 0xf);
+	imx8mq_mipi_csi_write(state, CSI2RX_IRQ_MASK, CSI2RX_IRQ_MASK_ALL);
+	imx8mq_mipi_csi_write(state, 0x180, 1);
+	/* vid_vc */
+	imx8mq_mipi_csi_write(state, 0x184, 1);
+	imx8mq_mipi_csi_write(state, 0x188, CSI2RX_SEND_LEVEL);
+}
+
+static int imx8mq_mipi_csi_clk_enable(struct csi_state *state)
+{
+	return clk_bulk_prepare_enable(CSI2_NUM_CLKS, state->clks);
+}
+
+static void imx8mq_mipi_csi_clk_disable(struct csi_state *state)
+{
+	clk_bulk_disable_unprepare(CSI2_NUM_CLKS, state->clks);
+}
+
+static int imx8mq_mipi_csi_clk_get(struct csi_state *state)
+{
+	unsigned int i;
+
+	for (i = 0; i < CSI2_NUM_CLKS; i++)
+		state->clks[i].id = imx8mq_mipi_csi_clk_id[i];
+
+	return devm_clk_bulk_get(state->dev, CSI2_NUM_CLKS, state->clks);
+}
+
+static int imx8mq_mipi_csi_calc_hs_settle(struct csi_state *state)
+{
+	u32 width = state->format_mbus[MIPI_CSI2_PAD_SINK].width;
+	u32 height = state->format_mbus[MIPI_CSI2_PAD_SINK].height;
+	s64 link_freq;
+	u32 lane_rate;
+
+	/* Calculate the line rate from the pixel rate. */
+	link_freq = v4l2_get_link_freq(state->src_sd->ctrl_handler,
+				       state->csi2_fmt->width,
+				       state->bus.num_data_lanes * 2);
+	if (link_freq < 0) {
+		dev_err(state->dev, "Unable to obtain link frequency: %d\n",
+			(int)link_freq);
+		return link_freq;
+	}
+
+	lane_rate = link_freq * 2;
+	if (lane_rate < 80000000 || lane_rate > 1500000000) {
+		dev_dbg(state->dev, "Out-of-bound lane rate %u\n", lane_rate);
+		return -EINVAL;
+	}
+
+	/* https://community.nxp.com/t5/i-MX-Processors/Explenation-for-HS-SETTLE-parameter-in-MIPI-CSI-D-PHY-registers/m-p/764275/highlight/true#M118744 */
+	if (lane_rate < 250000000)
+		state->hs_settle = 0xb;
+	else if (lane_rate < 500000000)
+		state->hs_settle = 0x8;
+	else
+		state->hs_settle = 0x6;
+
+	dev_dbg(state->dev, "start stream: %ux%u lane rate %u hs_settle %u\n",
+		width, height, lane_rate, state->hs_settle);
+
+	return 0;
+}
+
+static int imx8mq_mipi_csi_start_stream(struct csi_state *state)
+{
+	int ret;
+
+	ret = imx8mq_mipi_csi_sw_reset(state);
+	if (ret)
+		return ret;
+
+	imx8mq_mipi_csi_set_params(state);
+	imx8mq_mipi_csi_calc_hs_settle(state);
+	imx8mq_mipi_csi_system_enable(state, true);
+
+	return 0;
+}
+
+static void imx8mq_mipi_csi_stop_stream(struct csi_state *state)
+{
+	imx8mq_mipi_csi_system_enable(state, false);
+}
+
+/* -----------------------------------------------------------------------------
+ * V4L2 subdev operations
+ */
+
+static struct csi_state *mipi_sd_to_csi2_state(struct v4l2_subdev *sdev)
+{
+	return container_of(sdev, struct csi_state, sd);
+}
+
+static int imx8mq_mipi_csi_s_stream(struct v4l2_subdev *sd, int enable)
+{
+	struct csi_state *state = mipi_sd_to_csi2_state(sd);
+	int ret;
+
+	imx8mq_mipi_csi_write(state, CSI2RX_IRQ_MASK,
+			      CSI2RX_IRQ_MASK_ULPS_STATUS_CHANGE);
+
+	if (enable) {
+		ret = pm_runtime_get_sync(state->dev);
+		if (ret < 0) {
+			pm_runtime_put_noidle(state->dev);
+			return ret;
+		}
+	}
+
+	mutex_lock(&state->lock);
+
+	if (enable) {
+		if (state->state & ST_SUSPENDED) {
+			ret = -EBUSY;
+			goto unlock;
+		}
+
+		ret = imx8mq_mipi_csi_start_stream(state);
+		if (ret < 0)
+			goto unlock;
+
+		ret = v4l2_subdev_call(state->src_sd, video, s_stream, 1);
+		if (ret < 0)
+			goto unlock;
+
+		state->state |= ST_STREAMING;
+	} else {
+		v4l2_subdev_call(state->src_sd, video, s_stream, 0);
+		imx8mq_mipi_csi_stop_stream(state);
+		state->state &= ~ST_STREAMING;
+	}
+
+unlock:
+	mutex_unlock(&state->lock);
+
+	if (!enable || ret < 0)
+		pm_runtime_put(state->dev);
+
+	return ret;
+}
+
+static struct v4l2_mbus_framefmt *
+imx8mq_mipi_csi_get_format(struct csi_state *state,
+			   struct v4l2_subdev_state *sd_state,
+			   enum v4l2_subdev_format_whence which,
+			   unsigned int pad)
+{
+	if (which == V4L2_SUBDEV_FORMAT_TRY)
+		return v4l2_subdev_get_try_format(&state->sd, sd_state, pad);
+
+	return &state->format_mbus[pad];
+}
+
+static int imx8mq_mipi_csi_init_cfg(struct v4l2_subdev *sd,
+				    struct v4l2_subdev_state *sd_state)
+{
+	struct csi_state *state = mipi_sd_to_csi2_state(sd);
+	struct v4l2_mbus_framefmt *fmt_sink;
+	struct v4l2_mbus_framefmt *fmt_source;
+	enum v4l2_subdev_format_whence which;
+
+	which = sd_state ? V4L2_SUBDEV_FORMAT_TRY : V4L2_SUBDEV_FORMAT_ACTIVE;
+	fmt_sink = imx8mq_mipi_csi_get_format(state, sd_state, which,
+					      MIPI_CSI2_PAD_SINK);
+
+	fmt_sink->code = MEDIA_BUS_FMT_SGBRG10_1X10;
+	fmt_sink->width = MIPI_CSI2_DEF_PIX_WIDTH;
+	fmt_sink->height = MIPI_CSI2_DEF_PIX_HEIGHT;
+	fmt_sink->field = V4L2_FIELD_NONE;
+
+	fmt_sink->colorspace = V4L2_COLORSPACE_RAW;
+	fmt_sink->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(fmt_sink->colorspace);
+	fmt_sink->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(fmt_sink->colorspace);
+	fmt_sink->quantization =
+		V4L2_MAP_QUANTIZATION_DEFAULT(false, fmt_sink->colorspace,
+					      fmt_sink->ycbcr_enc);
+
+	/*
+	 * When called from imx8mq_mipi_csi_subdev_init() to initialize the
+	 * active configuration, sd_state is NULL, which indicates there's no
+	 * source pad configuration to set.
+	 */
+	if (!sd_state)
+		return 0;
+
+	fmt_source = imx8mq_mipi_csi_get_format(state, sd_state, which,
+						MIPI_CSI2_PAD_SOURCE);
+	*fmt_source = *fmt_sink;
+
+	return 0;
+}
+
+static int imx8mq_mipi_csi_get_fmt(struct v4l2_subdev *sd,
+				   struct v4l2_subdev_state *sd_state,
+				   struct v4l2_subdev_format *sdformat)
+{
+	struct csi_state *state = mipi_sd_to_csi2_state(sd);
+	struct v4l2_mbus_framefmt *fmt;
+
+	fmt = imx8mq_mipi_csi_get_format(state, sd_state, sdformat->which,
+					 sdformat->pad);
+
+	mutex_lock(&state->lock);
+
+	sdformat->format = *fmt;
+
+	mutex_unlock(&state->lock);
+
+	return 0;
+}
+
+static int imx8mq_mipi_csi_enum_mbus_code(struct v4l2_subdev *sd,
+					  struct v4l2_subdev_state *sd_state,
+					  struct v4l2_subdev_mbus_code_enum *code)
+{
+	struct csi_state *state = mipi_sd_to_csi2_state(sd);
+
+	/*
+	 * We can't transcode in any way, the source format is identical
+	 * to the sink format.
+	 */
+	if (code->pad == MIPI_CSI2_PAD_SOURCE) {
+		struct v4l2_mbus_framefmt *fmt;
+
+		if (code->index > 0)
+			return -EINVAL;
+
+		fmt = imx8mq_mipi_csi_get_format(state, sd_state, code->which,
+						 code->pad);
+		code->code = fmt->code;
+		return 0;
+	}
+
+	if (code->pad != MIPI_CSI2_PAD_SINK)
+		return -EINVAL;
+
+	if (code->index >= ARRAY_SIZE(imx8mq_mipi_csi_formats))
+		return -EINVAL;
+
+	code->code = imx8mq_mipi_csi_formats[code->index].code;
+
+	return 0;
+}
+
+static int imx8mq_mipi_csi_set_fmt(struct v4l2_subdev *sd,
+				   struct v4l2_subdev_state *sd_state,
+				   struct v4l2_subdev_format *sdformat)
+{
+	struct csi_state *state = mipi_sd_to_csi2_state(sd);
+	struct csi2_pix_format const *csi2_fmt;
+	struct v4l2_mbus_framefmt *fmt;
+
+	/*
+	 * The device can't transcode in any way, the source format can't be
+	 * modified.
+	 */
+	if (sdformat->pad == MIPI_CSI2_PAD_SOURCE)
+		return imx8mq_mipi_csi_get_fmt(sd, sd_state, sdformat);
+
+	if (sdformat->pad != MIPI_CSI2_PAD_SINK)
+		return -EINVAL;
+
+	csi2_fmt = find_csi2_format(sdformat->format.code);
+	if (!csi2_fmt)
+		csi2_fmt = &imx8mq_mipi_csi_formats[0];
+
+	fmt = imx8mq_mipi_csi_get_format(state, sd_state, sdformat->which,
+					 sdformat->pad);
+
+	mutex_lock(&state->lock);
+
+	fmt->code = csi2_fmt->code;
+	fmt->width = sdformat->format.width;
+	fmt->height = sdformat->format.height;
+
+	sdformat->format = *fmt;
+
+	/* Propagate the format from sink to source. */
+	fmt = imx8mq_mipi_csi_get_format(state, sd_state, sdformat->which,
+					 MIPI_CSI2_PAD_SOURCE);
+	*fmt = sdformat->format;
+
+	/* Store the CSI2 format descriptor for active formats. */
+	if (sdformat->which == V4L2_SUBDEV_FORMAT_ACTIVE)
+		state->csi2_fmt = csi2_fmt;
+
+	mutex_unlock(&state->lock);
+
+	return 0;
+}
+
+static const struct v4l2_subdev_video_ops imx8mq_mipi_csi_video_ops = {
+	.s_stream	= imx8mq_mipi_csi_s_stream,
+};
+
+static const struct v4l2_subdev_pad_ops imx8mq_mipi_csi_pad_ops = {
+	.init_cfg		= imx8mq_mipi_csi_init_cfg,
+	.enum_mbus_code		= imx8mq_mipi_csi_enum_mbus_code,
+	.get_fmt		= imx8mq_mipi_csi_get_fmt,
+	.set_fmt		= imx8mq_mipi_csi_set_fmt,
+};
+
+static const struct v4l2_subdev_ops imx8mq_mipi_csi_subdev_ops = {
+	.video	= &imx8mq_mipi_csi_video_ops,
+	.pad	= &imx8mq_mipi_csi_pad_ops,
+};
+
+/* -----------------------------------------------------------------------------
+ * Media entity operations
+ */
+
+static const struct media_entity_operations imx8mq_mipi_csi_entity_ops = {
+	.link_validate	= v4l2_subdev_link_validate,
+	.get_fwnode_pad = v4l2_subdev_get_fwnode_pad_1_to_1,
+};
+
+/* -----------------------------------------------------------------------------
+ * Async subdev notifier
+ */
+
+static struct csi_state *
+mipi_notifier_to_csi2_state(struct v4l2_async_notifier *n)
+{
+	return container_of(n, struct csi_state, notifier);
+}
+
+static int imx8mq_mipi_csi_notify_bound(struct v4l2_async_notifier *notifier,
+					struct v4l2_subdev *sd,
+					struct v4l2_async_subdev *asd)
+{
+	struct csi_state *state = mipi_notifier_to_csi2_state(notifier);
+	struct media_pad *sink = &state->sd.entity.pads[MIPI_CSI2_PAD_SINK];
+
+	state->src_sd = sd;
+
+	return v4l2_create_fwnode_links_to_pad(sd, sink, MEDIA_LNK_FL_ENABLED |
+					       MEDIA_LNK_FL_IMMUTABLE);
+}
+
+static const struct v4l2_async_notifier_operations imx8mq_mipi_csi_notify_ops = {
+	.bound = imx8mq_mipi_csi_notify_bound,
+};
+
+static int imx8mq_mipi_csi_async_register(struct csi_state *state)
+{
+	struct v4l2_fwnode_endpoint vep = {
+		.bus_type = V4L2_MBUS_CSI2_DPHY,
+	};
+	struct v4l2_async_subdev *asd;
+	struct fwnode_handle *ep;
+	unsigned int i;
+	int ret;
+
+	v4l2_async_notifier_init(&state->notifier);
+
+	ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(state->dev), 0, 0,
+					     FWNODE_GRAPH_ENDPOINT_NEXT);
+	if (!ep)
+		return -ENOTCONN;
+
+	ret = v4l2_fwnode_endpoint_parse(ep, &vep);
+	if (ret)
+		goto err_parse;
+
+	for (i = 0; i < vep.bus.mipi_csi2.num_data_lanes; ++i) {
+		if (vep.bus.mipi_csi2.data_lanes[i] != i + 1) {
+			dev_err(state->dev,
+				"data lanes reordering is not supported");
+			ret = -EINVAL;
+			goto err_parse;
+		}
+	}
+
+	state->bus = vep.bus.mipi_csi2;
+
+	dev_dbg(state->dev, "data lanes: %d flags: 0x%08x\n",
+		state->bus.num_data_lanes,
+		state->bus.flags);
+
+	asd = v4l2_async_notifier_add_fwnode_remote_subdev(&state->notifier,
+							   ep, struct v4l2_async_subdev);
+	if (IS_ERR(asd)) {
+		ret = PTR_ERR(asd);
+		goto err_parse;
+	}
+
+	fwnode_handle_put(ep);
+
+	state->notifier.ops = &imx8mq_mipi_csi_notify_ops;
+
+	ret = v4l2_async_subdev_notifier_register(&state->sd, &state->notifier);
+	if (ret)
+		return ret;
+
+	return v4l2_async_register_subdev(&state->sd);
+
+err_parse:
+	fwnode_handle_put(ep);
+
+	return ret;
+}
+
+/* -----------------------------------------------------------------------------
+ * Suspend/resume
+ */
+
+static int imx8mq_mipi_csi_pm_suspend(struct device *dev, bool runtime)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct csi_state *state = mipi_sd_to_csi2_state(sd);
+	int ret = 0;
+
+	mutex_lock(&state->lock);
+
+	if (state->state & ST_POWERED) {
+		imx8mq_mipi_csi_stop_stream(state);
+		imx8mq_mipi_csi_clk_disable(state);
+		state->state &= ~ST_POWERED;
+		if (!runtime)
+			state->state |= ST_SUSPENDED;
+	}
+
+	mutex_unlock(&state->lock);
+
+	ret = icc_set_bw(state->icc_path, 0, 0);
+	if (ret)
+		dev_err(dev, "icc_set_bw failed with %d\n", ret);
+
+	return ret ? -EAGAIN : 0;
+}
+
+static int imx8mq_mipi_csi_pm_resume(struct device *dev, bool runtime)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct csi_state *state = mipi_sd_to_csi2_state(sd);
+	int ret = 0;
+
+	ret = icc_set_bw(state->icc_path, 0, state->icc_path_bw);
+	if (ret) {
+		dev_err(dev, "icc_set_bw failed with %d\n", ret);
+		return ret;
+	}
+
+	mutex_lock(&state->lock);
+
+	if (!runtime && !(state->state & ST_SUSPENDED))
+		goto unlock;
+
+	if (!(state->state & ST_POWERED)) {
+		state->state |= ST_POWERED;
+		ret = imx8mq_mipi_csi_clk_enable(state);
+	}
+	if (state->state & ST_STREAMING) {
+		ret = imx8mq_mipi_csi_start_stream(state);
+		if (ret)
+			goto unlock;
+	}
+
+	state->state &= ~ST_SUSPENDED;
+
+unlock:
+	mutex_unlock(&state->lock);
+
+	return ret ? -EAGAIN : 0;
+}
+
+static int __maybe_unused imx8mq_mipi_csi_suspend(struct device *dev)
+{
+	return imx8mq_mipi_csi_pm_suspend(dev, false);
+}
+
+static int __maybe_unused imx8mq_mipi_csi_resume(struct device *dev)
+{
+	return imx8mq_mipi_csi_pm_resume(dev, false);
+}
+
+static int __maybe_unused imx8mq_mipi_csi_runtime_suspend(struct device *dev)
+{
+	return imx8mq_mipi_csi_pm_suspend(dev, true);
+}
+
+static int __maybe_unused imx8mq_mipi_csi_runtime_resume(struct device *dev)
+{
+	return imx8mq_mipi_csi_pm_resume(dev, true);
+}
+
+static const struct dev_pm_ops imx8mq_mipi_csi_pm_ops = {
+	SET_RUNTIME_PM_OPS(imx8mq_mipi_csi_runtime_suspend,
+			   imx8mq_mipi_csi_runtime_resume,
+			   NULL)
+	SET_SYSTEM_SLEEP_PM_OPS(imx8mq_mipi_csi_suspend, imx8mq_mipi_csi_resume)
+};
+
+/* -----------------------------------------------------------------------------
+ * Probe/remove & platform driver
+ */
+
+static int imx8mq_mipi_csi_subdev_init(struct csi_state *state)
+{
+	struct v4l2_subdev *sd = &state->sd;
+
+	v4l2_subdev_init(sd, &imx8mq_mipi_csi_subdev_ops);
+	sd->owner = THIS_MODULE;
+	snprintf(sd->name, sizeof(sd->name), "%s %s",
+		 MIPI_CSI2_SUBDEV_NAME, dev_name(state->dev));
+
+	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+
+	sd->entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
+	sd->entity.ops = &imx8mq_mipi_csi_entity_ops;
+
+	sd->dev = state->dev;
+
+	state->csi2_fmt = &imx8mq_mipi_csi_formats[0];
+	imx8mq_mipi_csi_init_cfg(sd, NULL);
+
+	state->pads[MIPI_CSI2_PAD_SINK].flags = MEDIA_PAD_FL_SINK
+					 | MEDIA_PAD_FL_MUST_CONNECT;
+	state->pads[MIPI_CSI2_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE
+					   | MEDIA_PAD_FL_MUST_CONNECT;
+	return media_entity_pads_init(&sd->entity, MIPI_CSI2_PADS_NUM,
+				      state->pads);
+}
+
+static void imx8mq_mipi_csi_release_icc(struct platform_device *pdev)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(&pdev->dev);
+	struct csi_state *state = mipi_sd_to_csi2_state(sd);
+
+	icc_put(state->icc_path);
+}
+
+static int imx8mq_mipi_csi_init_icc(struct platform_device *pdev)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(&pdev->dev);
+	struct csi_state *state = mipi_sd_to_csi2_state(sd);
+
+	/* Optional interconnect request */
+	state->icc_path = of_icc_get(&pdev->dev, "dram");
+	if (IS_ERR_OR_NULL(state->icc_path))
+		return PTR_ERR_OR_ZERO(state->icc_path);
+
+	state->icc_path_bw = MBps_to_icc(700);
+
+	return 0;
+}
+
+static int imx8mq_mipi_csi_parse_dt(struct csi_state *state)
+{
+	struct device *dev = state->dev;
+	struct device_node *np = state->dev->of_node;
+	struct device_node *node;
+	phandle ph;
+	u32 out_val[2];
+	int ret = 0;
+
+	state->rst = devm_reset_control_array_get_exclusive(dev);
+	if (IS_ERR(state->rst)) {
+		dev_err(dev, "Failed to get reset: %pe\n", state->rst);
+		return PTR_ERR(state->rst);
+	}
+
+	ret = of_property_read_u32_array(np, "fsl,mipi-phy-gpr", out_val,
+					 ARRAY_SIZE(out_val));
+	if (ret) {
+		dev_err(dev, "no fsl,mipi-phy-gpr property found: %d\n", ret);
+		return ret;
+	}
+
+	ph = *out_val;
+
+	node = of_find_node_by_phandle(ph);
+	if (!node) {
+		dev_err(dev, "Error finding node by phandle\n");
+		return -ENODEV;
+	}
+	state->phy_gpr = syscon_node_to_regmap(node);
+	of_node_put(node);
+	if (IS_ERR(state->phy_gpr)) {
+		dev_err(dev, "failed to get gpr regmap: %pe\n", state->phy_gpr);
+		return PTR_ERR(state->phy_gpr);
+	}
+
+	state->phy_gpr_reg = out_val[1];
+	dev_dbg(dev, "phy gpr register set to 0x%x\n", state->phy_gpr_reg);
+
+	return ret;
+}
+
+static int imx8mq_mipi_csi_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct csi_state *state;
+	int ret;
+
+	state = devm_kzalloc(dev, sizeof(*state), GFP_KERNEL);
+	if (!state)
+		return -ENOMEM;
+
+	state->dev = dev;
+
+	ret = imx8mq_mipi_csi_parse_dt(state);
+	if (ret < 0) {
+		dev_err(dev, "Failed to parse device tree: %d\n", ret);
+		return ret;
+	}
+
+	/* Acquire resources. */
+	state->regs = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(state->regs))
+		return PTR_ERR(state->regs);
+
+	ret = imx8mq_mipi_csi_clk_get(state);
+	if (ret < 0)
+		return ret;
+
+	platform_set_drvdata(pdev, &state->sd);
+
+	mutex_init(&state->lock);
+
+	ret = imx8mq_mipi_csi_subdev_init(state);
+	if (ret < 0)
+		goto mutex;
+
+	ret = imx8mq_mipi_csi_init_icc(pdev);
+	if (ret)
+		goto mutex;
+
+	/* Enable runtime PM. */
+	pm_runtime_enable(dev);
+	if (!pm_runtime_enabled(dev)) {
+		ret = imx8mq_mipi_csi_pm_resume(dev, true);
+		if (ret < 0)
+			goto icc;
+	}
+
+	ret = imx8mq_mipi_csi_async_register(state);
+	if (ret < 0)
+		goto cleanup;
+
+	return 0;
+
+cleanup:
+	pm_runtime_disable(&pdev->dev);
+	imx8mq_mipi_csi_pm_suspend(&pdev->dev, true);
+
+	media_entity_cleanup(&state->sd.entity);
+	v4l2_async_notifier_unregister(&state->notifier);
+	v4l2_async_notifier_cleanup(&state->notifier);
+	v4l2_async_unregister_subdev(&state->sd);
+icc:
+	imx8mq_mipi_csi_release_icc(pdev);
+mutex:
+	mutex_destroy(&state->lock);
+
+	return ret;
+}
+
+static int imx8mq_mipi_csi_remove(struct platform_device *pdev)
+{
+	struct v4l2_subdev *sd = platform_get_drvdata(pdev);
+	struct csi_state *state = mipi_sd_to_csi2_state(sd);
+
+	v4l2_async_notifier_unregister(&state->notifier);
+	v4l2_async_notifier_cleanup(&state->notifier);
+	v4l2_async_unregister_subdev(&state->sd);
+
+	pm_runtime_disable(&pdev->dev);
+	imx8mq_mipi_csi_pm_suspend(&pdev->dev, true);
+	media_entity_cleanup(&state->sd.entity);
+	mutex_destroy(&state->lock);
+	pm_runtime_set_suspended(&pdev->dev);
+	imx8mq_mipi_csi_release_icc(pdev);
+
+	return 0;
+}
+
+static const struct of_device_id imx8mq_mipi_csi_of_match[] = {
+	{ .compatible = "fsl,imx8mq-mipi-csi2", },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, imx8mq_mipi_csi_of_match);
+
+static struct platform_driver imx8mq_mipi_csi_driver = {
+	.probe		= imx8mq_mipi_csi_probe,
+	.remove		= imx8mq_mipi_csi_remove,
+	.driver		= {
+		.of_match_table = imx8mq_mipi_csi_of_match,
+		.name		= MIPI_CSI2_DRIVER_NAME,
+		.pm		= &imx8mq_mipi_csi_pm_ops,
+	},
+};
+
+module_platform_driver(imx8mq_mipi_csi_driver);
+
+MODULE_DESCRIPTION("i.MX8MQ MIPI CSI-2 receiver driver");
+MODULE_AUTHOR("Martin Kepplinger <martin.kepplinger@puri.sm>");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:imx8mq-mipi-csi2");
-- 
2.30.2

Re: [PATCH v6 2/3] media: imx: add a driver for i.MX8MQ mipi csi rx phy and controller

[Laurent Pinchart]

Hi Martin,

Thank you for the patch.

On Wed, Jul 14, 2021 at 01:19:30PM +0200, Martin Kepplinger wrote:
> Add a driver to support the i.MX8MQ MIPI CSI receiver. The hardware side
> is based on
> https://source.codeaurora.org/external/imx/linux-imx/tree/drivers/media/platform/imx8/mxc-mipi-csi2_yav.c?h=imx_5.4.70_2.3.0
> 
> It's built as part of VIDEO_IMX7_CSI because that's documented to support
> i.MX8M platforms. This driver adds i.MX8MQ support where currently only the
> i.MX8MM platform has been supported.
> 
> Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm>
> ---
>  drivers/staging/media/imx/Makefile           |   1 +
>  drivers/staging/media/imx/imx8mq-mipi-csi2.c | 949 +++++++++++++++++++
>  2 files changed, 950 insertions(+)
>  create mode 100644 drivers/staging/media/imx/imx8mq-mipi-csi2.c
> 
> diff --git a/drivers/staging/media/imx/Makefile b/drivers/staging/media/imx/Makefile
> index 6ac33275cc97..19c2fc54d424 100644
> --- a/drivers/staging/media/imx/Makefile
> +++ b/drivers/staging/media/imx/Makefile
> @@ -16,3 +16,4 @@ obj-$(CONFIG_VIDEO_IMX_CSI) += imx6-mipi-csi2.o
>  
>  obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-media-csi.o
>  obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-mipi-csis.o
> +obj-$(CONFIG_VIDEO_IMX7_CSI) += imx8mq-mipi-csi2.o
> diff --git a/drivers/staging/media/imx/imx8mq-mipi-csi2.c b/drivers/staging/media/imx/imx8mq-mipi-csi2.c
> new file mode 100644
> index 000000000000..949b3ef7a20a
> --- /dev/null
> +++ b/drivers/staging/media/imx/imx8mq-mipi-csi2.c
> @@ -0,0 +1,949 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Freescale i.MX8MQ SoC series MIPI-CSI2 receiver driver

Maybe they should be called NXP these days :-)

> + *
> + * Copyright (C) 2021 Purism SPC
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/errno.h>
> +#include <linux/interconnect.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/reset.h>
> +#include <linux/spinlock.h>
> +
> +#include <media/v4l2-common.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-fwnode.h>
> +#include <media/v4l2-mc.h>
> +#include <media/v4l2-subdev.h>
> +
> +#define MIPI_CSI2_DRIVER_NAME			"imx8mq-mipi-csi2"
> +#define MIPI_CSI2_SUBDEV_NAME			MIPI_CSI2_DRIVER_NAME
> +
> +#define MIPI_CSI2_PAD_SINK			0
> +#define MIPI_CSI2_PAD_SOURCE			1
> +#define MIPI_CSI2_PADS_NUM			2
> +
> +#define MIPI_CSI2_DEF_PIX_WIDTH			640
> +#define MIPI_CSI2_DEF_PIX_HEIGHT		480
> +
> +/* Register map definition */
> +
> +/* i.MX8MQ CSI-2 controller CSR */
> +#define CSI2RX_CFG_NUM_LANES			0x100
> +#define CSI2RX_CFG_DISABLE_DATA_LANES		0x104
> +#define CSI2RX_BIT_ERR				0x108
> +#define CSI2RX_IRQ_STATUS			0x10c
> +#define CSI2RX_IRQ_MASK				0x110
> +#define CSI2RX_IRQ_MASK_ALL			0x1ff
> +#define CSI2RX_IRQ_MASK_ULPS_STATUS_CHANGE	0x8
> +#define CSI2RX_ULPS_STATUS			0x114
> +#define CSI2RX_PPI_ERRSOT_HS			0x118
> +#define CSI2RX_PPI_ERRSOTSYNC_HS		0x11c
> +#define CSI2RX_PPI_ERRESC			0x120
> +#define CSI2RX_PPI_ERRSYNCESC			0x124
> +#define CSI2RX_PPI_ERRCONTROL			0x128
> +#define CSI2RX_CFG_DISABLE_PAYLOAD_0		0x12c
> +#define CSI2RX_CFG_VID_P_FIFO_SEND_LEVEL	0x188
> +#define CSI2RX_CFG_DISABLE_PAYLOAD_1		0x130
> +
> +enum {
> +	ST_POWERED	= 1,
> +	ST_STREAMING	= 2,
> +	ST_SUSPENDED	= 4,
> +};
> +
> +static const char * const imx8mq_mipi_csi_clk_id[] = {
> +	"core",
> +	"esc",
> +	"ui",
> +};
> +
> +#define CSI2_NUM_CLKS	ARRAY_SIZE(imx8mq_mipi_csi_clk_id)
> +
> +#define	GPR_CSI2_1_RX_ENABLE		BIT(13)
> +#define	GPR_CSI2_1_VID_INTFC_ENB	BIT(12)
> +#define	GPR_CSI2_1_HSEL			BIT(10)
> +#define	GPR_CSI2_1_CONT_CLK_MODE	BIT(8)
> +#define	GPR_CSI2_1_S_PRG_RXHS_SETTLE(x)	(((x) & 0x3f) << 2)
> +
> +/*
> + * The send level configures the number of entries that must accumulate in
> + * the Pixel FIFO before the data will be transferred to the video output.
> + * See https://community.nxp.com/t5/i-MX-Processors/IMX8M-MIPI-CSI-Host-Controller-send-level/m-p/864005/highlight/true#M131704
> + */
> +#define CSI2RX_SEND_LEVEL			64
> +
> +struct csi_state {
> +	struct device *dev;
> +	void __iomem *regs;
> +	struct clk_bulk_data clks[CSI2_NUM_CLKS];
> +	struct reset_control *rst;
> +	struct regulator *mipi_phy_regulator;
> +
> +	struct v4l2_subdev sd;
> +	struct media_pad pads[MIPI_CSI2_PADS_NUM];
> +	struct v4l2_async_notifier notifier;
> +	struct v4l2_subdev *src_sd;
> +
> +	struct v4l2_fwnode_bus_mipi_csi2 bus;
> +
> +	struct mutex lock; /* Protect csi2_fmt, format_mbus, state, hs_settle*/

Missing space before */

> +	const struct csi2_pix_format *csi2_fmt;
> +	struct v4l2_mbus_framefmt format_mbus[MIPI_CSI2_PADS_NUM];
> +	u32 state;
> +	u32 hs_settle;
> +
> +	struct regmap *phy_gpr;
> +	u8 phy_gpr_reg;
> +
> +	struct icc_path			*icc_path;
> +	s32				icc_path_bw;
> +};
> +
> +/* -----------------------------------------------------------------------------
> + * Format helpers
> + */
> +
> +struct csi2_pix_format {
> +	u32 code;
> +	u8 width;
> +};
> +
> +static const struct csi2_pix_format imx8mq_mipi_csi_formats[] = {
> +	/* RAW (Bayer and greyscale) formats. */
> +	{
> +		.code = MEDIA_BUS_FMT_SBGGR8_1X8,
> +		.width = 8,
> +	}, {
> +		.code = MEDIA_BUS_FMT_SGBRG8_1X8,
> +		.width = 8,
> +	}, {
> +		.code = MEDIA_BUS_FMT_SGRBG8_1X8,
> +		.width = 8,
> +	}, {
> +		.code = MEDIA_BUS_FMT_SRGGB8_1X8,
> +		.width = 8,
> +	}, {
> +		.code = MEDIA_BUS_FMT_Y8_1X8,
> +		.width = 8,
> +	}, {
> +		.code = MEDIA_BUS_FMT_SBGGR10_1X10,
> +		.width = 10,
> +	}, {
> +		.code = MEDIA_BUS_FMT_SGBRG10_1X10,
> +		.width = 10,
> +	}, {
> +		.code = MEDIA_BUS_FMT_SGRBG10_1X10,
> +		.width = 10,
> +	}, {
> +		.code = MEDIA_BUS_FMT_SRGGB10_1X10,
> +		.width = 10,
> +	}, {
> +		.code = MEDIA_BUS_FMT_Y10_1X10,
> +		.width = 10,
> +	}, {
> +		.code = MEDIA_BUS_FMT_SBGGR12_1X12,
> +		.width = 12,
> +	}, {
> +		.code = MEDIA_BUS_FMT_SGBRG12_1X12,
> +		.width = 12,
> +	}, {
> +		.code = MEDIA_BUS_FMT_SGRBG12_1X12,
> +		.width = 12,
> +	}, {
> +		.code = MEDIA_BUS_FMT_SRGGB12_1X12,
> +		.width = 12,
> +	}, {
> +		.code = MEDIA_BUS_FMT_Y12_1X12,
> +		.width = 12,
> +	}, {
> +		.code = MEDIA_BUS_FMT_SBGGR14_1X14,
> +		.width = 14,
> +	}, {
> +		.code = MEDIA_BUS_FMT_SGBRG14_1X14,
> +		.width = 14,
> +	}, {
> +		.code = MEDIA_BUS_FMT_SGRBG14_1X14,
> +		.width = 14,
> +	}, {
> +		.code = MEDIA_BUS_FMT_SRGGB14_1X14,
> +		.width = 14,
> +	}, {
> +	/* YUV formats */
> +		.code = MEDIA_BUS_FMT_YUYV8_2X8,
> +		.width = 16,
> +	}, {
> +		.code = MEDIA_BUS_FMT_YUYV8_1X16,
> +		.width = 16,
> +	}
> +};
> +
> +static const struct csi2_pix_format *find_csi2_format(u32 code)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(imx8mq_mipi_csi_formats); i++)
> +		if (code == imx8mq_mipi_csi_formats[i].code)
> +			return &imx8mq_mipi_csi_formats[i];
> +	return NULL;
> +}
> +
> +/* -----------------------------------------------------------------------------
> + * Hardware configuration
> + */
> +
> +static inline void imx8mq_mipi_csi_write(struct csi_state *state, u32 reg, u32 val)
> +{
> +	writel(val, state->regs + reg);
> +}
> +
> +static int imx8mq_mipi_csi_sw_reset(struct csi_state *state)
> +{
> +	int ret;
> +
> +	ret = reset_control_assert(state->rst);

That's peculiar, is there no need to deassert reset ?

> +	if (ret < 0) {
> +		dev_err(state->dev, "Failed to assert resets: %d\n", ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static void imx8mq_mipi_csi_system_enable(struct csi_state *state, int on)
> +{
> +	if (!on) {
> +		imx8mq_mipi_csi_write(state, CSI2RX_CFG_DISABLE_DATA_LANES, 0xf);
> +		return;
> +	}
> +
> +	regmap_update_bits(state->phy_gpr,
> +			   state->phy_gpr_reg,
> +			   0x3fff,
> +			   GPR_CSI2_1_RX_ENABLE |
> +			   GPR_CSI2_1_VID_INTFC_ENB |
> +			   GPR_CSI2_1_HSEL |
> +			   GPR_CSI2_1_CONT_CLK_MODE |
> +			   GPR_CSI2_1_S_PRG_RXHS_SETTLE(state->hs_settle));
> +}
> +
> +static void imx8mq_mipi_csi_set_params(struct csi_state *state)
> +{
> +	int lanes = state->bus.num_data_lanes;
> +
> +	imx8mq_mipi_csi_write(state, CSI2RX_CFG_NUM_LANES, lanes - 1);
> +	imx8mq_mipi_csi_write(state, CSI2RX_CFG_DISABLE_DATA_LANES,
> +			      (0xf << lanes) & 0xf);
> +	imx8mq_mipi_csi_write(state, CSI2RX_IRQ_MASK, CSI2RX_IRQ_MASK_ALL);
> +	imx8mq_mipi_csi_write(state, 0x180, 1);
> +	/* vid_vc */
> +	imx8mq_mipi_csi_write(state, 0x184, 1);
> +	imx8mq_mipi_csi_write(state, 0x188, CSI2RX_SEND_LEVEL);
> +}
> +
> +static int imx8mq_mipi_csi_clk_enable(struct csi_state *state)
> +{
> +	return clk_bulk_prepare_enable(CSI2_NUM_CLKS, state->clks);
> +}
> +
> +static void imx8mq_mipi_csi_clk_disable(struct csi_state *state)
> +{
> +	clk_bulk_disable_unprepare(CSI2_NUM_CLKS, state->clks);
> +}
> +
> +static int imx8mq_mipi_csi_clk_get(struct csi_state *state)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < CSI2_NUM_CLKS; i++)
> +		state->clks[i].id = imx8mq_mipi_csi_clk_id[i];
> +
> +	return devm_clk_bulk_get(state->dev, CSI2_NUM_CLKS, state->clks);
> +}
> +
> +static int imx8mq_mipi_csi_calc_hs_settle(struct csi_state *state)
> +{
> +	u32 width = state->format_mbus[MIPI_CSI2_PAD_SINK].width;
> +	u32 height = state->format_mbus[MIPI_CSI2_PAD_SINK].height;
> +	s64 link_freq;
> +	u32 lane_rate;
> +
> +	/* Calculate the line rate from the pixel rate. */
> +	link_freq = v4l2_get_link_freq(state->src_sd->ctrl_handler,
> +				       state->csi2_fmt->width,
> +				       state->bus.num_data_lanes * 2);
> +	if (link_freq < 0) {
> +		dev_err(state->dev, "Unable to obtain link frequency: %d\n",
> +			(int)link_freq);
> +		return link_freq;
> +	}
> +
> +	lane_rate = link_freq * 2;
> +	if (lane_rate < 80000000 || lane_rate > 1500000000) {
> +		dev_dbg(state->dev, "Out-of-bound lane rate %u\n", lane_rate);
> +		return -EINVAL;
> +	}
> +
> +	/* https://community.nxp.com/t5/i-MX-Processors/Explenation-for-HS-SETTLE-parameter-in-MIPI-CSI-D-PHY-registers/m-p/764275/highlight/true#M118744 */
> +	if (lane_rate < 250000000)
> +		state->hs_settle = 0xb;
> +	else if (lane_rate < 500000000)
> +		state->hs_settle = 0x8;
> +	else
> +		state->hs_settle = 0x6;

We could possibly compute this value based on the formula from the table
in that page, but maybe that's overkill ? If you want to give it a try,
it would be along those lines.

	/*
	 * The D-PHY specification requires Ths-settle to be in the range
	 * 85ns + 6*UI to 140ns + 10*UI, with the unit interval UI being half
	 * the clock period.
	 *
	 * The Ths-settle value is expressed in the hardware as a multiple of
	 * the Esc clock period:
	 *
	 * Ths-settle = (PRG_RXHS_SETTLE + 1) * Tperiod of RxClkInEsc
	 *
	 * Due to the one cycle inaccuracy introduced by rounding, the
	 * documentation recommends picking a value away from the boundaries.
	 * Let's pick the average.
	 */
	esc_clk_rate = clk_get_rate(...);

	min_ths_settle = 85 + 6 * 1000000 / (lane_rate / 1000);
	max_ths_settle = 140 + 10 * 1000000 / (lane_rate / 1000);
	ths_settle = (min_ths_settle + max_ths_settle) / 2;

	state->hs_settle = ths_settle * esc_clk_rate / 1000000000 - 1;

> +
> +	dev_dbg(state->dev, "start stream: %ux%u lane rate %u hs_settle %u\n",
> +		width, height, lane_rate, state->hs_settle);
> +
> +	return 0;
> +}
> +
> +static int imx8mq_mipi_csi_start_stream(struct csi_state *state)
> +{
> +	int ret;
> +
> +	ret = imx8mq_mipi_csi_sw_reset(state);
> +	if (ret)
> +		return ret;
> +
> +	imx8mq_mipi_csi_set_params(state);
> +	imx8mq_mipi_csi_calc_hs_settle(state);
> +	imx8mq_mipi_csi_system_enable(state, true);
> +
> +	return 0;
> +}
> +
> +static void imx8mq_mipi_csi_stop_stream(struct csi_state *state)
> +{
> +	imx8mq_mipi_csi_system_enable(state, false);
> +}
> +
> +/* -----------------------------------------------------------------------------
> + * V4L2 subdev operations
> + */
> +
> +static struct csi_state *mipi_sd_to_csi2_state(struct v4l2_subdev *sdev)
> +{
> +	return container_of(sdev, struct csi_state, sd);
> +}
> +
> +static int imx8mq_mipi_csi_s_stream(struct v4l2_subdev *sd, int enable)
> +{
> +	struct csi_state *state = mipi_sd_to_csi2_state(sd);
> +	int ret;
> +
> +	imx8mq_mipi_csi_write(state, CSI2RX_IRQ_MASK,
> +			      CSI2RX_IRQ_MASK_ULPS_STATUS_CHANGE);
> +
> +	if (enable) {
> +		ret = pm_runtime_get_sync(state->dev);

You can call pm_runtime_resume_and_get() and drop the
pm_runtime_put_noidle() right below.

> +		if (ret < 0) {
> +			pm_runtime_put_noidle(state->dev);
> +			return ret;
> +		}
> +	}
> +
> +	mutex_lock(&state->lock);
> +
> +	if (enable) {
> +		if (state->state & ST_SUSPENDED) {
> +			ret = -EBUSY;
> +			goto unlock;
> +		}
> +
> +		ret = imx8mq_mipi_csi_start_stream(state);
> +		if (ret < 0)
> +			goto unlock;
> +
> +		ret = v4l2_subdev_call(state->src_sd, video, s_stream, 1);
> +		if (ret < 0)
> +			goto unlock;
> +
> +		state->state |= ST_STREAMING;
> +	} else {
> +		v4l2_subdev_call(state->src_sd, video, s_stream, 0);
> +		imx8mq_mipi_csi_stop_stream(state);
> +		state->state &= ~ST_STREAMING;
> +	}
> +
> +unlock:
> +	mutex_unlock(&state->lock);
> +
> +	if (!enable || ret < 0)
> +		pm_runtime_put(state->dev);
> +
> +	return ret;
> +}
> +
> +static struct v4l2_mbus_framefmt *
> +imx8mq_mipi_csi_get_format(struct csi_state *state,
> +			   struct v4l2_subdev_state *sd_state,
> +			   enum v4l2_subdev_format_whence which,
> +			   unsigned int pad)
> +{
> +	if (which == V4L2_SUBDEV_FORMAT_TRY)
> +		return v4l2_subdev_get_try_format(&state->sd, sd_state, pad);
> +
> +	return &state->format_mbus[pad];
> +}
> +
> +static int imx8mq_mipi_csi_init_cfg(struct v4l2_subdev *sd,
> +				    struct v4l2_subdev_state *sd_state)
> +{
> +	struct csi_state *state = mipi_sd_to_csi2_state(sd);
> +	struct v4l2_mbus_framefmt *fmt_sink;
> +	struct v4l2_mbus_framefmt *fmt_source;
> +	enum v4l2_subdev_format_whence which;
> +
> +	which = sd_state ? V4L2_SUBDEV_FORMAT_TRY : V4L2_SUBDEV_FORMAT_ACTIVE;
> +	fmt_sink = imx8mq_mipi_csi_get_format(state, sd_state, which,
> +					      MIPI_CSI2_PAD_SINK);
> +
> +	fmt_sink->code = MEDIA_BUS_FMT_SGBRG10_1X10;
> +	fmt_sink->width = MIPI_CSI2_DEF_PIX_WIDTH;
> +	fmt_sink->height = MIPI_CSI2_DEF_PIX_HEIGHT;
> +	fmt_sink->field = V4L2_FIELD_NONE;
> +
> +	fmt_sink->colorspace = V4L2_COLORSPACE_RAW;
> +	fmt_sink->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(fmt_sink->colorspace);
> +	fmt_sink->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(fmt_sink->colorspace);
> +	fmt_sink->quantization =
> +		V4L2_MAP_QUANTIZATION_DEFAULT(false, fmt_sink->colorspace,
> +					      fmt_sink->ycbcr_enc);
> +
> +	/*
> +	 * When called from imx8mq_mipi_csi_subdev_init() to initialize the
> +	 * active configuration, sd_state is NULL, which indicates there's no
> +	 * source pad configuration to set.
> +	 */
> +	if (!sd_state)
> +		return 0;

This isn't true anymore, you should set the source pad format in both
cases.

With these small issues addressed,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +
> +	fmt_source = imx8mq_mipi_csi_get_format(state, sd_state, which,
> +						MIPI_CSI2_PAD_SOURCE);
> +	*fmt_source = *fmt_sink;
> +
> +	return 0;
> +}
> +
> +static int imx8mq_mipi_csi_get_fmt(struct v4l2_subdev *sd,
> +				   struct v4l2_subdev_state *sd_state,
> +				   struct v4l2_subdev_format *sdformat)
> +{
> +	struct csi_state *state = mipi_sd_to_csi2_state(sd);
> +	struct v4l2_mbus_framefmt *fmt;
> +
> +	fmt = imx8mq_mipi_csi_get_format(state, sd_state, sdformat->which,
> +					 sdformat->pad);
> +
> +	mutex_lock(&state->lock);
> +
> +	sdformat->format = *fmt;
> +
> +	mutex_unlock(&state->lock);
> +
> +	return 0;
> +}
> +
> +static int imx8mq_mipi_csi_enum_mbus_code(struct v4l2_subdev *sd,
> +					  struct v4l2_subdev_state *sd_state,
> +					  struct v4l2_subdev_mbus_code_enum *code)
> +{
> +	struct csi_state *state = mipi_sd_to_csi2_state(sd);
> +
> +	/*
> +	 * We can't transcode in any way, the source format is identical
> +	 * to the sink format.
> +	 */
> +	if (code->pad == MIPI_CSI2_PAD_SOURCE) {
> +		struct v4l2_mbus_framefmt *fmt;
> +
> +		if (code->index > 0)
> +			return -EINVAL;
> +
> +		fmt = imx8mq_mipi_csi_get_format(state, sd_state, code->which,
> +						 code->pad);
> +		code->code = fmt->code;
> +		return 0;
> +	}
> +
> +	if (code->pad != MIPI_CSI2_PAD_SINK)
> +		return -EINVAL;
> +
> +	if (code->index >= ARRAY_SIZE(imx8mq_mipi_csi_formats))
> +		return -EINVAL;
> +
> +	code->code = imx8mq_mipi_csi_formats[code->index].code;
> +
> +	return 0;
> +}
> +
> +static int imx8mq_mipi_csi_set_fmt(struct v4l2_subdev *sd,
> +				   struct v4l2_subdev_state *sd_state,
> +				   struct v4l2_subdev_format *sdformat)
> +{
> +	struct csi_state *state = mipi_sd_to_csi2_state(sd);
> +	struct csi2_pix_format const *csi2_fmt;
> +	struct v4l2_mbus_framefmt *fmt;
> +
> +	/*
> +	 * The device can't transcode in any way, the source format can't be
> +	 * modified.
> +	 */
> +	if (sdformat->pad == MIPI_CSI2_PAD_SOURCE)
> +		return imx8mq_mipi_csi_get_fmt(sd, sd_state, sdformat);
> +
> +	if (sdformat->pad != MIPI_CSI2_PAD_SINK)
> +		return -EINVAL;
> +
> +	csi2_fmt = find_csi2_format(sdformat->format.code);
> +	if (!csi2_fmt)
> +		csi2_fmt = &imx8mq_mipi_csi_formats[0];
> +
> +	fmt = imx8mq_mipi_csi_get_format(state, sd_state, sdformat->which,
> +					 sdformat->pad);
> +
> +	mutex_lock(&state->lock);
> +
> +	fmt->code = csi2_fmt->code;
> +	fmt->width = sdformat->format.width;
> +	fmt->height = sdformat->format.height;
> +
> +	sdformat->format = *fmt;
> +
> +	/* Propagate the format from sink to source. */
> +	fmt = imx8mq_mipi_csi_get_format(state, sd_state, sdformat->which,
> +					 MIPI_CSI2_PAD_SOURCE);
> +	*fmt = sdformat->format;
> +
> +	/* Store the CSI2 format descriptor for active formats. */
> +	if (sdformat->which == V4L2_SUBDEV_FORMAT_ACTIVE)
> +		state->csi2_fmt = csi2_fmt;
> +
> +	mutex_unlock(&state->lock);
> +
> +	return 0;
> +}
> +
> +static const struct v4l2_subdev_video_ops imx8mq_mipi_csi_video_ops = {
> +	.s_stream	= imx8mq_mipi_csi_s_stream,
> +};
> +
> +static const struct v4l2_subdev_pad_ops imx8mq_mipi_csi_pad_ops = {
> +	.init_cfg		= imx8mq_mipi_csi_init_cfg,
> +	.enum_mbus_code		= imx8mq_mipi_csi_enum_mbus_code,
> +	.get_fmt		= imx8mq_mipi_csi_get_fmt,
> +	.set_fmt		= imx8mq_mipi_csi_set_fmt,
> +};
> +
> +static const struct v4l2_subdev_ops imx8mq_mipi_csi_subdev_ops = {
> +	.video	= &imx8mq_mipi_csi_video_ops,
> +	.pad	= &imx8mq_mipi_csi_pad_ops,
> +};
> +
> +/* -----------------------------------------------------------------------------
> + * Media entity operations
> + */
> +
> +static const struct media_entity_operations imx8mq_mipi_csi_entity_ops = {
> +	.link_validate	= v4l2_subdev_link_validate,
> +	.get_fwnode_pad = v4l2_subdev_get_fwnode_pad_1_to_1,
> +};
> +
> +/* -----------------------------------------------------------------------------
> + * Async subdev notifier
> + */
> +
> +static struct csi_state *
> +mipi_notifier_to_csi2_state(struct v4l2_async_notifier *n)
> +{
> +	return container_of(n, struct csi_state, notifier);
> +}
> +
> +static int imx8mq_mipi_csi_notify_bound(struct v4l2_async_notifier *notifier,
> +					struct v4l2_subdev *sd,
> +					struct v4l2_async_subdev *asd)
> +{
> +	struct csi_state *state = mipi_notifier_to_csi2_state(notifier);
> +	struct media_pad *sink = &state->sd.entity.pads[MIPI_CSI2_PAD_SINK];
> +
> +	state->src_sd = sd;
> +
> +	return v4l2_create_fwnode_links_to_pad(sd, sink, MEDIA_LNK_FL_ENABLED |
> +					       MEDIA_LNK_FL_IMMUTABLE);
> +}
> +
> +static const struct v4l2_async_notifier_operations imx8mq_mipi_csi_notify_ops = {
> +	.bound = imx8mq_mipi_csi_notify_bound,
> +};
> +
> +static int imx8mq_mipi_csi_async_register(struct csi_state *state)
> +{
> +	struct v4l2_fwnode_endpoint vep = {
> +		.bus_type = V4L2_MBUS_CSI2_DPHY,
> +	};
> +	struct v4l2_async_subdev *asd;
> +	struct fwnode_handle *ep;
> +	unsigned int i;
> +	int ret;
> +
> +	v4l2_async_notifier_init(&state->notifier);
> +
> +	ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(state->dev), 0, 0,
> +					     FWNODE_GRAPH_ENDPOINT_NEXT);
> +	if (!ep)
> +		return -ENOTCONN;
> +
> +	ret = v4l2_fwnode_endpoint_parse(ep, &vep);
> +	if (ret)
> +		goto err_parse;
> +
> +	for (i = 0; i < vep.bus.mipi_csi2.num_data_lanes; ++i) {
> +		if (vep.bus.mipi_csi2.data_lanes[i] != i + 1) {
> +			dev_err(state->dev,
> +				"data lanes reordering is not supported");
> +			ret = -EINVAL;
> +			goto err_parse;
> +		}
> +	}
> +
> +	state->bus = vep.bus.mipi_csi2;
> +
> +	dev_dbg(state->dev, "data lanes: %d flags: 0x%08x\n",
> +		state->bus.num_data_lanes,
> +		state->bus.flags);
> +
> +	asd = v4l2_async_notifier_add_fwnode_remote_subdev(&state->notifier,
> +							   ep, struct v4l2_async_subdev);
> +	if (IS_ERR(asd)) {
> +		ret = PTR_ERR(asd);
> +		goto err_parse;
> +	}
> +
> +	fwnode_handle_put(ep);
> +
> +	state->notifier.ops = &imx8mq_mipi_csi_notify_ops;
> +
> +	ret = v4l2_async_subdev_notifier_register(&state->sd, &state->notifier);
> +	if (ret)
> +		return ret;
> +
> +	return v4l2_async_register_subdev(&state->sd);
> +
> +err_parse:
> +	fwnode_handle_put(ep);
> +
> +	return ret;
> +}
> +
> +/* -----------------------------------------------------------------------------
> + * Suspend/resume
> + */
> +
> +static int imx8mq_mipi_csi_pm_suspend(struct device *dev, bool runtime)
> +{
> +	struct v4l2_subdev *sd = dev_get_drvdata(dev);
> +	struct csi_state *state = mipi_sd_to_csi2_state(sd);
> +	int ret = 0;
> +
> +	mutex_lock(&state->lock);
> +
> +	if (state->state & ST_POWERED) {
> +		imx8mq_mipi_csi_stop_stream(state);
> +		imx8mq_mipi_csi_clk_disable(state);
> +		state->state &= ~ST_POWERED;
> +		if (!runtime)
> +			state->state |= ST_SUSPENDED;
> +	}
> +
> +	mutex_unlock(&state->lock);
> +
> +	ret = icc_set_bw(state->icc_path, 0, 0);
> +	if (ret)
> +		dev_err(dev, "icc_set_bw failed with %d\n", ret);
> +
> +	return ret ? -EAGAIN : 0;
> +}
> +
> +static int imx8mq_mipi_csi_pm_resume(struct device *dev, bool runtime)
> +{
> +	struct v4l2_subdev *sd = dev_get_drvdata(dev);
> +	struct csi_state *state = mipi_sd_to_csi2_state(sd);
> +	int ret = 0;
> +
> +	ret = icc_set_bw(state->icc_path, 0, state->icc_path_bw);
> +	if (ret) {
> +		dev_err(dev, "icc_set_bw failed with %d\n", ret);
> +		return ret;
> +	}
> +
> +	mutex_lock(&state->lock);
> +
> +	if (!runtime && !(state->state & ST_SUSPENDED))
> +		goto unlock;
> +
> +	if (!(state->state & ST_POWERED)) {
> +		state->state |= ST_POWERED;
> +		ret = imx8mq_mipi_csi_clk_enable(state);
> +	}
> +	if (state->state & ST_STREAMING) {
> +		ret = imx8mq_mipi_csi_start_stream(state);
> +		if (ret)
> +			goto unlock;
> +	}
> +
> +	state->state &= ~ST_SUSPENDED;
> +
> +unlock:
> +	mutex_unlock(&state->lock);
> +
> +	return ret ? -EAGAIN : 0;
> +}
> +
> +static int __maybe_unused imx8mq_mipi_csi_suspend(struct device *dev)
> +{
> +	return imx8mq_mipi_csi_pm_suspend(dev, false);
> +}
> +
> +static int __maybe_unused imx8mq_mipi_csi_resume(struct device *dev)
> +{
> +	return imx8mq_mipi_csi_pm_resume(dev, false);
> +}
> +
> +static int __maybe_unused imx8mq_mipi_csi_runtime_suspend(struct device *dev)
> +{
> +	return imx8mq_mipi_csi_pm_suspend(dev, true);
> +}
> +
> +static int __maybe_unused imx8mq_mipi_csi_runtime_resume(struct device *dev)
> +{
> +	return imx8mq_mipi_csi_pm_resume(dev, true);
> +}
> +
> +static const struct dev_pm_ops imx8mq_mipi_csi_pm_ops = {
> +	SET_RUNTIME_PM_OPS(imx8mq_mipi_csi_runtime_suspend,
> +			   imx8mq_mipi_csi_runtime_resume,
> +			   NULL)
> +	SET_SYSTEM_SLEEP_PM_OPS(imx8mq_mipi_csi_suspend, imx8mq_mipi_csi_resume)
> +};
> +
> +/* -----------------------------------------------------------------------------
> + * Probe/remove & platform driver
> + */
> +
> +static int imx8mq_mipi_csi_subdev_init(struct csi_state *state)
> +{
> +	struct v4l2_subdev *sd = &state->sd;
> +
> +	v4l2_subdev_init(sd, &imx8mq_mipi_csi_subdev_ops);
> +	sd->owner = THIS_MODULE;
> +	snprintf(sd->name, sizeof(sd->name), "%s %s",
> +		 MIPI_CSI2_SUBDEV_NAME, dev_name(state->dev));
> +
> +	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> +
> +	sd->entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
> +	sd->entity.ops = &imx8mq_mipi_csi_entity_ops;
> +
> +	sd->dev = state->dev;
> +
> +	state->csi2_fmt = &imx8mq_mipi_csi_formats[0];
> +	imx8mq_mipi_csi_init_cfg(sd, NULL);
> +
> +	state->pads[MIPI_CSI2_PAD_SINK].flags = MEDIA_PAD_FL_SINK
> +					 | MEDIA_PAD_FL_MUST_CONNECT;
> +	state->pads[MIPI_CSI2_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE
> +					   | MEDIA_PAD_FL_MUST_CONNECT;
> +	return media_entity_pads_init(&sd->entity, MIPI_CSI2_PADS_NUM,
> +				      state->pads);
> +}
> +
> +static void imx8mq_mipi_csi_release_icc(struct platform_device *pdev)
> +{
> +	struct v4l2_subdev *sd = dev_get_drvdata(&pdev->dev);
> +	struct csi_state *state = mipi_sd_to_csi2_state(sd);
> +
> +	icc_put(state->icc_path);
> +}
> +
> +static int imx8mq_mipi_csi_init_icc(struct platform_device *pdev)
> +{
> +	struct v4l2_subdev *sd = dev_get_drvdata(&pdev->dev);
> +	struct csi_state *state = mipi_sd_to_csi2_state(sd);
> +
> +	/* Optional interconnect request */
> +	state->icc_path = of_icc_get(&pdev->dev, "dram");
> +	if (IS_ERR_OR_NULL(state->icc_path))
> +		return PTR_ERR_OR_ZERO(state->icc_path);
> +
> +	state->icc_path_bw = MBps_to_icc(700);
> +
> +	return 0;
> +}
> +
> +static int imx8mq_mipi_csi_parse_dt(struct csi_state *state)
> +{
> +	struct device *dev = state->dev;
> +	struct device_node *np = state->dev->of_node;
> +	struct device_node *node;
> +	phandle ph;
> +	u32 out_val[2];
> +	int ret = 0;
> +
> +	state->rst = devm_reset_control_array_get_exclusive(dev);
> +	if (IS_ERR(state->rst)) {
> +		dev_err(dev, "Failed to get reset: %pe\n", state->rst);
> +		return PTR_ERR(state->rst);
> +	}
> +
> +	ret = of_property_read_u32_array(np, "fsl,mipi-phy-gpr", out_val,
> +					 ARRAY_SIZE(out_val));
> +	if (ret) {
> +		dev_err(dev, "no fsl,mipi-phy-gpr property found: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ph = *out_val;
> +
> +	node = of_find_node_by_phandle(ph);
> +	if (!node) {
> +		dev_err(dev, "Error finding node by phandle\n");
> +		return -ENODEV;
> +	}
> +	state->phy_gpr = syscon_node_to_regmap(node);
> +	of_node_put(node);
> +	if (IS_ERR(state->phy_gpr)) {
> +		dev_err(dev, "failed to get gpr regmap: %pe\n", state->phy_gpr);
> +		return PTR_ERR(state->phy_gpr);
> +	}
> +
> +	state->phy_gpr_reg = out_val[1];
> +	dev_dbg(dev, "phy gpr register set to 0x%x\n", state->phy_gpr_reg);
> +
> +	return ret;
> +}
> +
> +static int imx8mq_mipi_csi_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct csi_state *state;
> +	int ret;
> +
> +	state = devm_kzalloc(dev, sizeof(*state), GFP_KERNEL);
> +	if (!state)
> +		return -ENOMEM;
> +
> +	state->dev = dev;
> +
> +	ret = imx8mq_mipi_csi_parse_dt(state);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to parse device tree: %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* Acquire resources. */
> +	state->regs = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(state->regs))
> +		return PTR_ERR(state->regs);
> +
> +	ret = imx8mq_mipi_csi_clk_get(state);
> +	if (ret < 0)
> +		return ret;
> +
> +	platform_set_drvdata(pdev, &state->sd);
> +
> +	mutex_init(&state->lock);
> +
> +	ret = imx8mq_mipi_csi_subdev_init(state);
> +	if (ret < 0)
> +		goto mutex;
> +
> +	ret = imx8mq_mipi_csi_init_icc(pdev);
> +	if (ret)
> +		goto mutex;
> +
> +	/* Enable runtime PM. */
> +	pm_runtime_enable(dev);
> +	if (!pm_runtime_enabled(dev)) {
> +		ret = imx8mq_mipi_csi_pm_resume(dev, true);
> +		if (ret < 0)
> +			goto icc;
> +	}
> +
> +	ret = imx8mq_mipi_csi_async_register(state);
> +	if (ret < 0)
> +		goto cleanup;
> +
> +	return 0;
> +
> +cleanup:
> +	pm_runtime_disable(&pdev->dev);
> +	imx8mq_mipi_csi_pm_suspend(&pdev->dev, true);
> +
> +	media_entity_cleanup(&state->sd.entity);
> +	v4l2_async_notifier_unregister(&state->notifier);
> +	v4l2_async_notifier_cleanup(&state->notifier);
> +	v4l2_async_unregister_subdev(&state->sd);
> +icc:
> +	imx8mq_mipi_csi_release_icc(pdev);
> +mutex:
> +	mutex_destroy(&state->lock);
> +
> +	return ret;
> +}
> +
> +static int imx8mq_mipi_csi_remove(struct platform_device *pdev)
> +{
> +	struct v4l2_subdev *sd = platform_get_drvdata(pdev);
> +	struct csi_state *state = mipi_sd_to_csi2_state(sd);
> +
> +	v4l2_async_notifier_unregister(&state->notifier);
> +	v4l2_async_notifier_cleanup(&state->notifier);
> +	v4l2_async_unregister_subdev(&state->sd);
> +
> +	pm_runtime_disable(&pdev->dev);
> +	imx8mq_mipi_csi_pm_suspend(&pdev->dev, true);
> +	media_entity_cleanup(&state->sd.entity);
> +	mutex_destroy(&state->lock);
> +	pm_runtime_set_suspended(&pdev->dev);
> +	imx8mq_mipi_csi_release_icc(pdev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id imx8mq_mipi_csi_of_match[] = {
> +	{ .compatible = "fsl,imx8mq-mipi-csi2", },
> +	{ /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, imx8mq_mipi_csi_of_match);
> +
> +static struct platform_driver imx8mq_mipi_csi_driver = {
> +	.probe		= imx8mq_mipi_csi_probe,
> +	.remove		= imx8mq_mipi_csi_remove,
> +	.driver		= {
> +		.of_match_table = imx8mq_mipi_csi_of_match,
> +		.name		= MIPI_CSI2_DRIVER_NAME,
> +		.pm		= &imx8mq_mipi_csi_pm_ops,
> +	},
> +};
> +
> +module_platform_driver(imx8mq_mipi_csi_driver);
> +
> +MODULE_DESCRIPTION("i.MX8MQ MIPI CSI-2 receiver driver");
> +MODULE_AUTHOR("Martin Kepplinger <martin.kepplinger@puri.sm>");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:imx8mq-mipi-csi2");

-- 
Regards,

Laurent Pinchart

Re: [PATCH v6 2/3] media: imx: add a driver for i.MX8MQ mipi csi rx phy and controller

[Martin Kepplinger]

Am Mittwoch, dem 14.07.2021 um 21:24 +0300 schrieb Laurent Pinchart:
> Hi Martin,
> 
> Thank you for the patch.

thank you for reviewing.

> 
> On Wed, Jul 14, 2021 at 01:19:30PM +0200, Martin Kepplinger wrote:
> > Add a driver to support the i.MX8MQ MIPI CSI receiver. The hardware
> > side
> > is based on
> >  
> > https://source.codeaurora.org/external/imx/linux-imx/tree/drivers/media/platform/imx8/mxc-mipi-csi2_yav.c?h=imx_5.4.70_2.3.0
> > 
> > It's built as part of VIDEO_IMX7_CSI because that's documented to
> > support
> > i.MX8M platforms. This driver adds i.MX8MQ support where currently
> > only the
> > i.MX8MM platform has been supported.
> > 
> > Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm>
> > ---
> >  drivers/staging/media/imx/Makefile           |   1 +
> >  drivers/staging/media/imx/imx8mq-mipi-csi2.c | 949
> > +++++++++++++++++++
> >  2 files changed, 950 insertions(+)
> >  create mode 100644 drivers/staging/media/imx/imx8mq-mipi-csi2.c
> > 
> > diff --git a/drivers/staging/media/imx/Makefile
> > b/drivers/staging/media/imx/Makefile
> > index 6ac33275cc97..19c2fc54d424 100644
> > --- a/drivers/staging/media/imx/Makefile
> > +++ b/drivers/staging/media/imx/Makefile
> > @@ -16,3 +16,4 @@ obj-$(CONFIG_VIDEO_IMX_CSI) += imx6-mipi-csi2.o
> >  
> >  obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-media-csi.o
> >  obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-mipi-csis.o
> > +obj-$(CONFIG_VIDEO_IMX7_CSI) += imx8mq-mipi-csi2.o
> > diff --git a/drivers/staging/media/imx/imx8mq-mipi-csi2.c
> > b/drivers/staging/media/imx/imx8mq-mipi-csi2.c
> > new file mode 100644
> > index 000000000000..949b3ef7a20a
> > --- /dev/null
> > +++ b/drivers/staging/media/imx/imx8mq-mipi-csi2.c
> > @@ -0,0 +1,949 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Freescale i.MX8MQ SoC series MIPI-CSI2 receiver driver
> 
> Maybe they should be called NXP these days :-)
> 
> > + *
> > + * Copyright (C) 2021 Purism SPC
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/delay.h>
> > +#include <linux/errno.h>
> > +#include <linux/interconnect.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/io.h>
> > +#include <linux/kernel.h>
> > +#include <linux/mfd/syscon.h>
> > +#include <linux/module.h>
> > +#include <linux/mutex.h>
> > +#include <linux/of.h>
> > +#include <linux/of_device.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/regmap.h>
> > +#include <linux/regulator/consumer.h>
> > +#include <linux/reset.h>
> > +#include <linux/spinlock.h>
> > +
> > +#include <media/v4l2-common.h>
> > +#include <media/v4l2-device.h>
> > +#include <media/v4l2-fwnode.h>
> > +#include <media/v4l2-mc.h>
> > +#include <media/v4l2-subdev.h>
> > +
> > +#define MIPI_CSI2_DRIVER_NAME                  "imx8mq-mipi-csi2"
> > +#define
> > MIPI_CSI2_SUBDEV_NAME                  MIPI_CSI2_DRIVER_NAME
> > +
> > +#define MIPI_CSI2_PAD_SINK                     0
> > +#define MIPI_CSI2_PAD_SOURCE                   1
> > +#define MIPI_CSI2_PADS_NUM                     2
> > +
> > +#define MIPI_CSI2_DEF_PIX_WIDTH                        640
> > +#define MIPI_CSI2_DEF_PIX_HEIGHT               480
> > +
> > +/* Register map definition */
> > +
> > +/* i.MX8MQ CSI-2 controller CSR */
> > +#define CSI2RX_CFG_NUM_LANES                   0x100
> > +#define CSI2RX_CFG_DISABLE_DATA_LANES          0x104
> > +#define CSI2RX_BIT_ERR                         0x108
> > +#define CSI2RX_IRQ_STATUS                      0x10c
> > +#define CSI2RX_IRQ_MASK                                0x110
> > +#define CSI2RX_IRQ_MASK_ALL                    0x1ff
> > +#define CSI2RX_IRQ_MASK_ULPS_STATUS_CHANGE     0x8
> > +#define CSI2RX_ULPS_STATUS                     0x114
> > +#define CSI2RX_PPI_ERRSOT_HS                   0x118
> > +#define CSI2RX_PPI_ERRSOTSYNC_HS               0x11c
> > +#define CSI2RX_PPI_ERRESC                      0x120
> > +#define CSI2RX_PPI_ERRSYNCESC                  0x124
> > +#define CSI2RX_PPI_ERRCONTROL                  0x128
> > +#define CSI2RX_CFG_DISABLE_PAYLOAD_0           0x12c
> > +#define CSI2RX_CFG_VID_P_FIFO_SEND_LEVEL       0x188
> > +#define CSI2RX_CFG_DISABLE_PAYLOAD_1           0x130
> > +
> > +enum {
> > +       ST_POWERED      = 1,
> > +       ST_STREAMING    = 2,
> > +       ST_SUSPENDED    = 4,
> > +};
> > +
> > +static const char * const imx8mq_mipi_csi_clk_id[] = {
> > +       "core",
> > +       "esc",
> > +       "ui",
> > +};
> > +
> > +#define CSI2_NUM_CLKS  ARRAY_SIZE(imx8mq_mipi_csi_clk_id)
> > +
> > +#define        GPR_CSI2_1_RX_ENABLE            BIT(13)
> > +#define        GPR_CSI2_1_VID_INTFC_ENB        BIT(12)
> > +#define        GPR_CSI2_1_HSEL                 BIT(10)
> > +#define        GPR_CSI2_1_CONT_CLK_MODE        BIT(8)
> > +#define        GPR_CSI2_1_S_PRG_RXHS_SETTLE(x) (((x) & 0x3f) << 2)
> > +
> > +/*
> > + * The send level configures the number of entries that must
> > accumulate in
> > + * the Pixel FIFO before the data will be transferred to the video
> > output.
> > + * See  
> > https://community.nxp.com/t5/i-MX-Processors/IMX8M-MIPI-CSI-Host-Controller-send-level/m-p/864005/highlight/true#M131704
> > + */
> > +#define CSI2RX_SEND_LEVEL                      64
> > +
> > +struct csi_state {
> > +       struct device *dev;
> > +       void __iomem *regs;
> > +       struct clk_bulk_data clks[CSI2_NUM_CLKS];
> > +       struct reset_control *rst;
> > +       struct regulator *mipi_phy_regulator;
> > +
> > +       struct v4l2_subdev sd;
> > +       struct media_pad pads[MIPI_CSI2_PADS_NUM];
> > +       struct v4l2_async_notifier notifier;
> > +       struct v4l2_subdev *src_sd;
> > +
> > +       struct v4l2_fwnode_bus_mipi_csi2 bus;
> > +
> > +       struct mutex lock; /* Protect csi2_fmt, format_mbus, state,
> > hs_settle*/
> 
> Missing space before */
> 
> > +       const struct csi2_pix_format *csi2_fmt;
> > +       struct v4l2_mbus_framefmt format_mbus[MIPI_CSI2_PADS_NUM];
> > +       u32 state;
> > +       u32 hs_settle;
> > +
> > +       struct regmap *phy_gpr;
> > +       u8 phy_gpr_reg;
> > +
> > +       struct icc_path                 *icc_path;
> > +       s32                             icc_path_bw;
> > +};
> > +
> > +/* ---------------------------------------------------------------
> > --------------
> > + * Format helpers
> > + */
> > +
> > +struct csi2_pix_format {
> > +       u32 code;
> > +       u8 width;
> > +};
> > +
> > +static const struct csi2_pix_format imx8mq_mipi_csi_formats[] = {
> > +       /* RAW (Bayer and greyscale) formats. */
> > +       {
> > +               .code = MEDIA_BUS_FMT_SBGGR8_1X8,
> > +               .width = 8,
> > +       }, {
> > +               .code = MEDIA_BUS_FMT_SGBRG8_1X8,
> > +               .width = 8,
> > +       }, {
> > +               .code = MEDIA_BUS_FMT_SGRBG8_1X8,
> > +               .width = 8,
> > +       }, {
> > +               .code = MEDIA_BUS_FMT_SRGGB8_1X8,
> > +               .width = 8,
> > +       }, {
> > +               .code = MEDIA_BUS_FMT_Y8_1X8,
> > +               .width = 8,
> > +       }, {
> > +               .code = MEDIA_BUS_FMT_SBGGR10_1X10,
> > +               .width = 10,
> > +       }, {
> > +               .code = MEDIA_BUS_FMT_SGBRG10_1X10,
> > +               .width = 10,
> > +       }, {
> > +               .code = MEDIA_BUS_FMT_SGRBG10_1X10,
> > +               .width = 10,
> > +       }, {
> > +               .code = MEDIA_BUS_FMT_SRGGB10_1X10,
> > +               .width = 10,
> > +       }, {
> > +               .code = MEDIA_BUS_FMT_Y10_1X10,
> > +               .width = 10,
> > +       }, {
> > +               .code = MEDIA_BUS_FMT_SBGGR12_1X12,
> > +               .width = 12,
> > +       }, {
> > +               .code = MEDIA_BUS_FMT_SGBRG12_1X12,
> > +               .width = 12,
> > +       }, {
> > +               .code = MEDIA_BUS_FMT_SGRBG12_1X12,
> > +               .width = 12,
> > +       }, {
> > +               .code = MEDIA_BUS_FMT_SRGGB12_1X12,
> > +               .width = 12,
> > +       }, {
> > +               .code = MEDIA_BUS_FMT_Y12_1X12,
> > +               .width = 12,
> > +       }, {
> > +               .code = MEDIA_BUS_FMT_SBGGR14_1X14,
> > +               .width = 14,
> > +       }, {
> > +               .code = MEDIA_BUS_FMT_SGBRG14_1X14,
> > +               .width = 14,
> > +       }, {
> > +               .code = MEDIA_BUS_FMT_SGRBG14_1X14,
> > +               .width = 14,
> > +       }, {
> > +               .code = MEDIA_BUS_FMT_SRGGB14_1X14,
> > +               .width = 14,
> > +       }, {
> > +       /* YUV formats */
> > +               .code = MEDIA_BUS_FMT_YUYV8_2X8,
> > +               .width = 16,
> > +       }, {
> > +               .code = MEDIA_BUS_FMT_YUYV8_1X16,
> > +               .width = 16,
> > +       }
> > +};
> > +
> > +static const struct csi2_pix_format *find_csi2_format(u32 code)
> > +{
> > +       unsigned int i;
> > +
> > +       for (i = 0; i < ARRAY_SIZE(imx8mq_mipi_csi_formats); i++)
> > +               if (code == imx8mq_mipi_csi_formats[i].code)
> > +                       return &imx8mq_mipi_csi_formats[i];
> > +       return NULL;
> > +}
> > +
> > +/* ---------------------------------------------------------------
> > --------------
> > + * Hardware configuration
> > + */
> > +
> > +static inline void imx8mq_mipi_csi_write(struct csi_state *state,
> > u32 reg, u32 val)
> > +{
> > +       writel(val, state->regs + reg);
> > +}
> > +
> > +static int imx8mq_mipi_csi_sw_reset(struct csi_state *state)
> > +{
> > +       int ret;
> > +
> > +       ret = reset_control_assert(state->rst);
> 
> That's peculiar, is there no need to deassert reset ?

I tried different things here that would look more intuitive, but in
the end only this worked, which is directly taken from
https://source.codeaurora.org/external/imx/linux-imx/tree/drivers/media/platform/imx8/mxc-mipi-csi2_yav.c?h=imx_5.4.70_2.3.0#n105
(actual register value read from DT) that results in exactly the same
register bits set like this assertation.

Re: [PATCH v6 2/3] media: imx: add a driver for i.MX8MQ mipi csi rx phy and controller

[Laurent Pinchart]

Hi Martin,

On Thu, Jul 15, 2021 at 08:49:51AM +0200, Martin Kepplinger wrote:
> Am Mittwoch, dem 14.07.2021 um 21:24 +0300 schrieb Laurent Pinchart:
> > Hi Martin,
> > 
> > Thank you for the patch.
> 
> thank you for reviewing.
> 
> > On Wed, Jul 14, 2021 at 01:19:30PM +0200, Martin Kepplinger wrote:
> > > Add a driver to support the i.MX8MQ MIPI CSI receiver. The hardware
> > > side
> > > is based on
> > >  
> > > https://source.codeaurora.org/external/imx/linux-imx/tree/drivers/media/platform/imx8/mxc-mipi-csi2_yav.c?h=imx_5.4.70_2.3.0
> > > 
> > > It's built as part of VIDEO_IMX7_CSI because that's documented to
> > > support
> > > i.MX8M platforms. This driver adds i.MX8MQ support where currently
> > > only the
> > > i.MX8MM platform has been supported.
> > > 
> > > Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm>
> > > ---
> > >  drivers/staging/media/imx/Makefile           |   1 +
> > >  drivers/staging/media/imx/imx8mq-mipi-csi2.c | 949
> > > +++++++++++++++++++
> > >  2 files changed, 950 insertions(+)
> > >  create mode 100644 drivers/staging/media/imx/imx8mq-mipi-csi2.c
> > > 
> > > diff --git a/drivers/staging/media/imx/Makefile
> > > b/drivers/staging/media/imx/Makefile
> > > index 6ac33275cc97..19c2fc54d424 100644
> > > --- a/drivers/staging/media/imx/Makefile
> > > +++ b/drivers/staging/media/imx/Makefile
> > > @@ -16,3 +16,4 @@ obj-$(CONFIG_VIDEO_IMX_CSI) += imx6-mipi-csi2.o
> > >  
> > >  obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-media-csi.o
> > >  obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-mipi-csis.o
> > > +obj-$(CONFIG_VIDEO_IMX7_CSI) += imx8mq-mipi-csi2.o
> > > diff --git a/drivers/staging/media/imx/imx8mq-mipi-csi2.c
> > > b/drivers/staging/media/imx/imx8mq-mipi-csi2.c
> > > new file mode 100644
> > > index 000000000000..949b3ef7a20a
> > > --- /dev/null
> > > +++ b/drivers/staging/media/imx/imx8mq-mipi-csi2.c
> > > @@ -0,0 +1,949 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Freescale i.MX8MQ SoC series MIPI-CSI2 receiver driver
> > 
> > Maybe they should be called NXP these days :-)
> > 
> > > + *
> > > + * Copyright (C) 2021 Purism SPC
> > > + */
> > > +
> > > +#include <linux/clk.h>
> > > +#include <linux/delay.h>
> > > +#include <linux/errno.h>
> > > +#include <linux/interconnect.h>
> > > +#include <linux/interrupt.h>
> > > +#include <linux/io.h>
> > > +#include <linux/kernel.h>
> > > +#include <linux/mfd/syscon.h>
> > > +#include <linux/module.h>
> > > +#include <linux/mutex.h>
> > > +#include <linux/of.h>
> > > +#include <linux/of_device.h>
> > > +#include <linux/platform_device.h>
> > > +#include <linux/pm_runtime.h>
> > > +#include <linux/regmap.h>
> > > +#include <linux/regulator/consumer.h>
> > > +#include <linux/reset.h>
> > > +#include <linux/spinlock.h>
> > > +
> > > +#include <media/v4l2-common.h>
> > > +#include <media/v4l2-device.h>
> > > +#include <media/v4l2-fwnode.h>
> > > +#include <media/v4l2-mc.h>
> > > +#include <media/v4l2-subdev.h>
> > > +
> > > +#define MIPI_CSI2_DRIVER_NAME                  "imx8mq-mipi-csi2"
> > > +#define
> > > MIPI_CSI2_SUBDEV_NAME                  MIPI_CSI2_DRIVER_NAME
> > > +
> > > +#define MIPI_CSI2_PAD_SINK                     0
> > > +#define MIPI_CSI2_PAD_SOURCE                   1
> > > +#define MIPI_CSI2_PADS_NUM                     2
> > > +
> > > +#define MIPI_CSI2_DEF_PIX_WIDTH                        640
> > > +#define MIPI_CSI2_DEF_PIX_HEIGHT               480
> > > +
> > > +/* Register map definition */
> > > +
> > > +/* i.MX8MQ CSI-2 controller CSR */
> > > +#define CSI2RX_CFG_NUM_LANES                   0x100
> > > +#define CSI2RX_CFG_DISABLE_DATA_LANES          0x104
> > > +#define CSI2RX_BIT_ERR                         0x108
> > > +#define CSI2RX_IRQ_STATUS                      0x10c
> > > +#define CSI2RX_IRQ_MASK                                0x110
> > > +#define CSI2RX_IRQ_MASK_ALL                    0x1ff
> > > +#define CSI2RX_IRQ_MASK_ULPS_STATUS_CHANGE     0x8
> > > +#define CSI2RX_ULPS_STATUS                     0x114
> > > +#define CSI2RX_PPI_ERRSOT_HS                   0x118
> > > +#define CSI2RX_PPI_ERRSOTSYNC_HS               0x11c
> > > +#define CSI2RX_PPI_ERRESC                      0x120
> > > +#define CSI2RX_PPI_ERRSYNCESC                  0x124
> > > +#define CSI2RX_PPI_ERRCONTROL                  0x128
> > > +#define CSI2RX_CFG_DISABLE_PAYLOAD_0           0x12c
> > > +#define CSI2RX_CFG_VID_P_FIFO_SEND_LEVEL       0x188
> > > +#define CSI2RX_CFG_DISABLE_PAYLOAD_1           0x130
> > > +
> > > +enum {
> > > +       ST_POWERED      = 1,
> > > +       ST_STREAMING    = 2,
> > > +       ST_SUSPENDED    = 4,
> > > +};
> > > +
> > > +static const char * const imx8mq_mipi_csi_clk_id[] = {
> > > +       "core",
> > > +       "esc",
> > > +       "ui",
> > > +};
> > > +
> > > +#define CSI2_NUM_CLKS  ARRAY_SIZE(imx8mq_mipi_csi_clk_id)
> > > +
> > > +#define        GPR_CSI2_1_RX_ENABLE            BIT(13)
> > > +#define        GPR_CSI2_1_VID_INTFC_ENB        BIT(12)
> > > +#define        GPR_CSI2_1_HSEL                 BIT(10)
> > > +#define        GPR_CSI2_1_CONT_CLK_MODE        BIT(8)
> > > +#define        GPR_CSI2_1_S_PRG_RXHS_SETTLE(x) (((x) & 0x3f) << 2)
> > > +
> > > +/*
> > > + * The send level configures the number of entries that must
> > > accumulate in
> > > + * the Pixel FIFO before the data will be transferred to the video
> > > output.
> > > + * See  
> > > https://community.nxp.com/t5/i-MX-Processors/IMX8M-MIPI-CSI-Host-Controller-send-level/m-p/864005/highlight/true#M131704
> > > + */
> > > +#define CSI2RX_SEND_LEVEL                      64
> > > +
> > > +struct csi_state {
> > > +       struct device *dev;
> > > +       void __iomem *regs;
> > > +       struct clk_bulk_data clks[CSI2_NUM_CLKS];
> > > +       struct reset_control *rst;
> > > +       struct regulator *mipi_phy_regulator;
> > > +
> > > +       struct v4l2_subdev sd;
> > > +       struct media_pad pads[MIPI_CSI2_PADS_NUM];
> > > +       struct v4l2_async_notifier notifier;
> > > +       struct v4l2_subdev *src_sd;
> > > +
> > > +       struct v4l2_fwnode_bus_mipi_csi2 bus;
> > > +
> > > +       struct mutex lock; /* Protect csi2_fmt, format_mbus, state,
> > > hs_settle*/
> > 
> > Missing space before */
> > 
> > > +       const struct csi2_pix_format *csi2_fmt;
> > > +       struct v4l2_mbus_framefmt format_mbus[MIPI_CSI2_PADS_NUM];
> > > +       u32 state;
> > > +       u32 hs_settle;
> > > +
> > > +       struct regmap *phy_gpr;
> > > +       u8 phy_gpr_reg;
> > > +
> > > +       struct icc_path                 *icc_path;
> > > +       s32                             icc_path_bw;
> > > +};
> > > +
> > > +/* ---------------------------------------------------------------
> > > --------------
> > > + * Format helpers
> > > + */
> > > +
> > > +struct csi2_pix_format {
> > > +       u32 code;
> > > +       u8 width;
> > > +};
> > > +
> > > +static const struct csi2_pix_format imx8mq_mipi_csi_formats[] = {
> > > +       /* RAW (Bayer and greyscale) formats. */
> > > +       {
> > > +               .code = MEDIA_BUS_FMT_SBGGR8_1X8,
> > > +               .width = 8,
> > > +       }, {
> > > +               .code = MEDIA_BUS_FMT_SGBRG8_1X8,
> > > +               .width = 8,
> > > +       }, {
> > > +               .code = MEDIA_BUS_FMT_SGRBG8_1X8,
> > > +               .width = 8,
> > > +       }, {
> > > +               .code = MEDIA_BUS_FMT_SRGGB8_1X8,
> > > +               .width = 8,
> > > +       }, {
> > > +               .code = MEDIA_BUS_FMT_Y8_1X8,
> > > +               .width = 8,
> > > +       }, {
> > > +               .code = MEDIA_BUS_FMT_SBGGR10_1X10,
> > > +               .width = 10,
> > > +       }, {
> > > +               .code = MEDIA_BUS_FMT_SGBRG10_1X10,
> > > +               .width = 10,
> > > +       }, {
> > > +               .code = MEDIA_BUS_FMT_SGRBG10_1X10,
> > > +               .width = 10,
> > > +       }, {
> > > +               .code = MEDIA_BUS_FMT_SRGGB10_1X10,
> > > +               .width = 10,
> > > +       }, {
> > > +               .code = MEDIA_BUS_FMT_Y10_1X10,
> > > +               .width = 10,
> > > +       }, {
> > > +               .code = MEDIA_BUS_FMT_SBGGR12_1X12,
> > > +               .width = 12,
> > > +       }, {
> > > +               .code = MEDIA_BUS_FMT_SGBRG12_1X12,
> > > +               .width = 12,
> > > +       }, {
> > > +               .code = MEDIA_BUS_FMT_SGRBG12_1X12,
> > > +               .width = 12,
> > > +       }, {
> > > +               .code = MEDIA_BUS_FMT_SRGGB12_1X12,
> > > +               .width = 12,
> > > +       }, {
> > > +               .code = MEDIA_BUS_FMT_Y12_1X12,
> > > +               .width = 12,
> > > +       }, {
> > > +               .code = MEDIA_BUS_FMT_SBGGR14_1X14,
> > > +               .width = 14,
> > > +       }, {
> > > +               .code = MEDIA_BUS_FMT_SGBRG14_1X14,
> > > +               .width = 14,
> > > +       }, {
> > > +               .code = MEDIA_BUS_FMT_SGRBG14_1X14,
> > > +               .width = 14,
> > > +       }, {
> > > +               .code = MEDIA_BUS_FMT_SRGGB14_1X14,
> > > +               .width = 14,
> > > +       }, {
> > > +       /* YUV formats */
> > > +               .code = MEDIA_BUS_FMT_YUYV8_2X8,
> > > +               .width = 16,
> > > +       }, {
> > > +               .code = MEDIA_BUS_FMT_YUYV8_1X16,
> > > +               .width = 16,
> > > +       }
> > > +};
> > > +
> > > +static const struct csi2_pix_format *find_csi2_format(u32 code)
> > > +{
> > > +       unsigned int i;
> > > +
> > > +       for (i = 0; i < ARRAY_SIZE(imx8mq_mipi_csi_formats); i++)
> > > +               if (code == imx8mq_mipi_csi_formats[i].code)
> > > +                       return &imx8mq_mipi_csi_formats[i];
> > > +       return NULL;
> > > +}
> > > +
> > > +/* ---------------------------------------------------------------
> > > --------------
> > > + * Hardware configuration
> > > + */
> > > +
> > > +static inline void imx8mq_mipi_csi_write(struct csi_state *state,
> > > u32 reg, u32 val)
> > > +{
> > > +       writel(val, state->regs + reg);
> > > +}
> > > +
> > > +static int imx8mq_mipi_csi_sw_reset(struct csi_state *state)
> > > +{
> > > +       int ret;
> > > +
> > > +       ret = reset_control_assert(state->rst);
> > 
> > That's peculiar, is there no need to deassert reset ?
> 
> I tried different things here that would look more intuitive, but in
> the end only this worked, which is directly taken from
> https://source.codeaurora.org/external/imx/linux-imx/tree/drivers/media/platform/imx8/mxc-mipi-csi2_yav.c?h=imx_5.4.70_2.3.0#n105
> (actual register value read from DT) that results in exactly the same
> register bits set like this assertation.

It's very likely that these are self-clearing reset bits.

I would have adviced using reset_control_assert(), but it looks like the
reset controller driver doesn't support that operation. Could you add a
comment here to explain what's going on, maybe with a note to tell that
the reset-imx7 driver should implement the .reset() operation ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH v6 2/3] media: imx: add a driver for i.MX8MQ mipi csi rx phy and controller

[Martin Kepplinger]

Am Mittwoch, dem 14.07.2021 um 21:24 +0300 schrieb Laurent Pinchart:
> Hi Martin,
> 
> Thank you for the patch.
> 
> On Wed, Jul 14, 2021 at 01:19:30PM +0200, Martin Kepplinger wrote:
> > Add a driver to support the i.MX8MQ MIPI CSI receiver. The hardware
> > side
> > is based on
> > https://source.codeaurora.org/external/imx/linux-imx/tree/drivers/media/platform/imx8/mxc-mipi-csi2_yav.c?h=imx_5.4.70_2.3.0
> > 
> > It's built as part of VIDEO_IMX7_CSI because that's documented to
> > support
> > i.MX8M platforms. This driver adds i.MX8MQ support where currently
> > only the
> > i.MX8MM platform has been supported.
> > 
> > Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm>
> > ---
> >  drivers/staging/media/imx/Makefile           |   1 +
> >  drivers/staging/media/imx/imx8mq-mipi-csi2.c | 949
> > +++++++++++++++++++
> >  2 files changed, 950 insertions(+)
> >  create mode 100644 drivers/staging/media/imx/imx8mq-mipi-csi2.c
> > 
> > diff --git a/drivers/staging/media/imx/Makefile
> > b/drivers/staging/media/imx/Makefile
> > index 6ac33275cc97..19c2fc54d424 100644
> > --- a/drivers/staging/media/imx/Makefile
> > +++ b/drivers/staging/media/imx/Makefile
> > @@ -16,3 +16,4 @@ obj-$(CONFIG_VIDEO_IMX_CSI) += imx6-mipi-csi2.o
> >  
> >  obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-media-csi.o
> >  obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-mipi-csis.o
> > +obj-$(CONFIG_VIDEO_IMX7_CSI) += imx8mq-mipi-csi2.o
> > diff --git a/drivers/staging/media/imx/imx8mq-mipi-csi2.c
> > b/drivers/staging/media/imx/imx8mq-mipi-csi2.c
> > new file mode 100644
> > index 000000000000..949b3ef7a20a
> > --- /dev/null
> > +++ b/drivers/staging/media/imx/imx8mq-mipi-csi2.c
> > @@ -0,0 +1,949 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Freescale i.MX8MQ SoC series MIPI-CSI2 receiver driver
> 
> Maybe they should be called NXP these days :-)
> 
> > + *
> > + * Copyright (C) 2021 Purism SPC
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/delay.h>
> > +#include <linux/errno.h>
> > +#include <linux/interconnect.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/io.h>
> > +#include <linux/kernel.h>
> > +#include <linux/mfd/syscon.h>
> > +#include <linux/module.h>
> > +#include <linux/mutex.h>
> > +#include <linux/of.h>
> > +#include <linux/of_device.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/regmap.h>
> > +#include <linux/regulator/consumer.h>
> > +#include <linux/reset.h>
> > +#include <linux/spinlock.h>
> > +
> > +#include <media/v4l2-common.h>
> > +#include <media/v4l2-device.h>
> > +#include <media/v4l2-fwnode.h>
> > +#include <media/v4l2-mc.h>
> > +#include <media/v4l2-subdev.h>
> > +
> > +#define MIPI_CSI2_DRIVER_NAME                  "imx8mq-mipi-csi2"
> > +#define
> > MIPI_CSI2_SUBDEV_NAME                  MIPI_CSI2_DRIVER_NAME
> > +
> > +#define MIPI_CSI2_PAD_SINK                     0
> > +#define MIPI_CSI2_PAD_SOURCE                   1
> > +#define MIPI_CSI2_PADS_NUM                     2
> > +
> > +#define MIPI_CSI2_DEF_PIX_WIDTH                        640
> > +#define MIPI_CSI2_DEF_PIX_HEIGHT               480
> > +
> > +/* Register map definition */
> > +
> > +/* i.MX8MQ CSI-2 controller CSR */
> > +#define CSI2RX_CFG_NUM_LANES                   0x100
> > +#define CSI2RX_CFG_DISABLE_DATA_LANES          0x104
> > +#define CSI2RX_BIT_ERR                         0x108
> > +#define CSI2RX_IRQ_STATUS                      0x10c
> > +#define CSI2RX_IRQ_MASK                                0x110
> > +#define CSI2RX_IRQ_MASK_ALL                    0x1ff
> > +#define CSI2RX_IRQ_MASK_ULPS_STATUS_CHANGE     0x8
> > +#define CSI2RX_ULPS_STATUS                     0x114
> > +#define CSI2RX_PPI_ERRSOT_HS                   0x118
> > +#define CSI2RX_PPI_ERRSOTSYNC_HS               0x11c
> > +#define CSI2RX_PPI_ERRESC                      0x120
> > +#define CSI2RX_PPI_ERRSYNCESC                  0x124
> > +#define CSI2RX_PPI_ERRCONTROL                  0x128
> > +#define CSI2RX_CFG_DISABLE_PAYLOAD_0           0x12c
> > +#define CSI2RX_CFG_VID_P_FIFO_SEND_LEVEL       0x188
> > +#define CSI2RX_CFG_DISABLE_PAYLOAD_1           0x130
> > +
> > +enum {
> > +       ST_POWERED      = 1,
> > +       ST_STREAMING    = 2,
> > +       ST_SUSPENDED    = 4,
> > +};
> > +
> > +static const char * const imx8mq_mipi_csi_clk_id[] = {
> > +       "core",
> > +       "esc",
> > +       "ui",
> > +};
> > +
> > +#define CSI2_NUM_CLKS  ARRAY_SIZE(imx8mq_mipi_csi_clk_id)
> > +
> > +#define        GPR_CSI2_1_RX_ENABLE            BIT(13)
> > +#define        GPR_CSI2_1_VID_INTFC_ENB        BIT(12)
> > +#define        GPR_CSI2_1_HSEL                 BIT(10)
> > +#define        GPR_CSI2_1_CONT_CLK_MODE        BIT(8)
> > +#define        GPR_CSI2_1_S_PRG_RXHS_SETTLE(x) (((x) & 0x3f) << 2)
> > +
> > +/*
> > + * The send level configures the number of entries that must
> > accumulate in
> > + * the Pixel FIFO before the data will be transferred to the video
> > output.
> > + * See 
> > https://community.nxp.com/t5/i-MX-Processors/IMX8M-MIPI-CSI-Host-Controller-send-level/m-p/864005/highlight/true#M131704
> > + */
> > +#define CSI2RX_SEND_LEVEL                      64
> > +
> > +struct csi_state {
> > +       struct device *dev;
> > +       void __iomem *regs;
> > +       struct clk_bulk_data clks[CSI2_NUM_CLKS];
> > +       struct reset_control *rst;
> > +       struct regulator *mipi_phy_regulator;
> > +
> > +       struct v4l2_subdev sd;
> > +       struct media_pad pads[MIPI_CSI2_PADS_NUM];
> > +       struct v4l2_async_notifier notifier;
> > +       struct v4l2_subdev *src_sd;
> > +
> > +       struct v4l2_fwnode_bus_mipi_csi2 bus;
> > +
> > +       struct mutex lock; /* Protect csi2_fmt, format_mbus, state,
> > hs_settle*/
> 
> Missing space before */
> 
> > +       const struct csi2_pix_format *csi2_fmt;
> > +       struct v4l2_mbus_framefmt format_mbus[MIPI_CSI2_PADS_NUM];
> > +       u32 state;
> > +       u32 hs_settle;
> > +
> > +       struct regmap *phy_gpr;
> > +       u8 phy_gpr_reg;
> > +
> > +       struct icc_path                 *icc_path;
> > +       s32                             icc_path_bw;
> > +};
> > +
> > +/* ---------------------------------------------------------------
> > --------------
> > + * Format helpers
> > + */
> > +
> > +struct csi2_pix_format {
> > +       u32 code;
> > +       u8 width;
> > +};
> > +
> > +static const struct csi2_pix_format imx8mq_mipi_csi_formats[] = {
> > +       /* RAW (Bayer and greyscale) formats. */
> > +       {
> > +               .code = MEDIA_BUS_FMT_SBGGR8_1X8,
> > +               .width = 8,
> > +       }, {
> > +               .code = MEDIA_BUS_FMT_SGBRG8_1X8,
> > +               .width = 8,
> > +       }, {
> > +               .code = MEDIA_BUS_FMT_SGRBG8_1X8,
> > +               .width = 8,
> > +       }, {
> > +               .code = MEDIA_BUS_FMT_SRGGB8_1X8,
> > +               .width = 8,
> > +       }, {
> > +               .code = MEDIA_BUS_FMT_Y8_1X8,
> > +               .width = 8,
> > +       }, {
> > +               .code = MEDIA_BUS_FMT_SBGGR10_1X10,
> > +               .width = 10,
> > +       }, {
> > +               .code = MEDIA_BUS_FMT_SGBRG10_1X10,
> > +               .width = 10,
> > +       }, {
> > +               .code = MEDIA_BUS_FMT_SGRBG10_1X10,
> > +               .width = 10,
> > +       }, {
> > +               .code = MEDIA_BUS_FMT_SRGGB10_1X10,
> > +               .width = 10,
> > +       }, {
> > +               .code = MEDIA_BUS_FMT_Y10_1X10,
> > +               .width = 10,
> > +       }, {
> > +               .code = MEDIA_BUS_FMT_SBGGR12_1X12,
> > +               .width = 12,
> > +       }, {
> > +               .code = MEDIA_BUS_FMT_SGBRG12_1X12,
> > +               .width = 12,
> > +       }, {
> > +               .code = MEDIA_BUS_FMT_SGRBG12_1X12,
> > +               .width = 12,
> > +       }, {
> > +               .code = MEDIA_BUS_FMT_SRGGB12_1X12,
> > +               .width = 12,
> > +       }, {
> > +               .code = MEDIA_BUS_FMT_Y12_1X12,
> > +               .width = 12,
> > +       }, {
> > +               .code = MEDIA_BUS_FMT_SBGGR14_1X14,
> > +               .width = 14,
> > +       }, {
> > +               .code = MEDIA_BUS_FMT_SGBRG14_1X14,
> > +               .width = 14,
> > +       }, {
> > +               .code = MEDIA_BUS_FMT_SGRBG14_1X14,
> > +               .width = 14,
> > +       }, {
> > +               .code = MEDIA_BUS_FMT_SRGGB14_1X14,
> > +               .width = 14,
> > +       }, {
> > +       /* YUV formats */
> > +               .code = MEDIA_BUS_FMT_YUYV8_2X8,
> > +               .width = 16,
> > +       }, {
> > +               .code = MEDIA_BUS_FMT_YUYV8_1X16,
> > +               .width = 16,
> > +       }
> > +};
> > +
> > +static const struct csi2_pix_format *find_csi2_format(u32 code)
> > +{
> > +       unsigned int i;
> > +
> > +       for (i = 0; i < ARRAY_SIZE(imx8mq_mipi_csi_formats); i++)
> > +               if (code == imx8mq_mipi_csi_formats[i].code)
> > +                       return &imx8mq_mipi_csi_formats[i];
> > +       return NULL;
> > +}
> > +
> > +/* ---------------------------------------------------------------
> > --------------
> > + * Hardware configuration
> > + */
> > +
> > +static inline void imx8mq_mipi_csi_write(struct csi_state *state,
> > u32 reg, u32 val)
> > +{
> > +       writel(val, state->regs + reg);
> > +}
> > +
> > +static int imx8mq_mipi_csi_sw_reset(struct csi_state *state)
> > +{
> > +       int ret;
> > +
> > +       ret = reset_control_assert(state->rst);
> 
> That's peculiar, is there no need to deassert reset ?
> 
> > +       if (ret < 0) {
> > +               dev_err(state->dev, "Failed to assert resets:
> > %d\n", ret);
> > +               return ret;
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> > +static void imx8mq_mipi_csi_system_enable(struct csi_state *state,
> > int on)
> > +{
> > +       if (!on) {
> > +               imx8mq_mipi_csi_write(state,
> > CSI2RX_CFG_DISABLE_DATA_LANES, 0xf);
> > +               return;
> > +       }
> > +
> > +       regmap_update_bits(state->phy_gpr,
> > +                          state->phy_gpr_reg,
> > +                          0x3fff,
> > +                          GPR_CSI2_1_RX_ENABLE |
> > +                          GPR_CSI2_1_VID_INTFC_ENB |
> > +                          GPR_CSI2_1_HSEL |
> > +                          GPR_CSI2_1_CONT_CLK_MODE |
> > +                          GPR_CSI2_1_S_PRG_RXHS_SETTLE(state-
> > >hs_settle));
> > +}
> > +
> > +static void imx8mq_mipi_csi_set_params(struct csi_state *state)
> > +{
> > +       int lanes = state->bus.num_data_lanes;
> > +
> > +       imx8mq_mipi_csi_write(state, CSI2RX_CFG_NUM_LANES, lanes -
> > 1);
> > +       imx8mq_mipi_csi_write(state, CSI2RX_CFG_DISABLE_DATA_LANES,
> > +                             (0xf << lanes) & 0xf);
> > +       imx8mq_mipi_csi_write(state, CSI2RX_IRQ_MASK,
> > CSI2RX_IRQ_MASK_ALL);
> > +       imx8mq_mipi_csi_write(state, 0x180, 1);
> > +       /* vid_vc */
> > +       imx8mq_mipi_csi_write(state, 0x184, 1);
> > +       imx8mq_mipi_csi_write(state, 0x188, CSI2RX_SEND_LEVEL);
> > +}
> > +
> > +static int imx8mq_mipi_csi_clk_enable(struct csi_state *state)
> > +{
> > +       return clk_bulk_prepare_enable(CSI2_NUM_CLKS, state->clks);
> > +}
> > +
> > +static void imx8mq_mipi_csi_clk_disable(struct csi_state *state)
> > +{
> > +       clk_bulk_disable_unprepare(CSI2_NUM_CLKS, state->clks);
> > +}
> > +
> > +static int imx8mq_mipi_csi_clk_get(struct csi_state *state)
> > +{
> > +       unsigned int i;
> > +
> > +       for (i = 0; i < CSI2_NUM_CLKS; i++)
> > +               state->clks[i].id = imx8mq_mipi_csi_clk_id[i];
> > +
> > +       return devm_clk_bulk_get(state->dev, CSI2_NUM_CLKS, state-
> > >clks);
> > +}
> > +
> > +static int imx8mq_mipi_csi_calc_hs_settle(struct csi_state *state)
> > +{
> > +       u32 width = state->format_mbus[MIPI_CSI2_PAD_SINK].width;
> > +       u32 height = state->format_mbus[MIPI_CSI2_PAD_SINK].height;
> > +       s64 link_freq;
> > +       u32 lane_rate;
> > +
> > +       /* Calculate the line rate from the pixel rate. */
> > +       link_freq = v4l2_get_link_freq(state->src_sd->ctrl_handler,
> > +                                      state->csi2_fmt->width,
> > +                                      state->bus.num_data_lanes *
> > 2);
> > +       if (link_freq < 0) {
> > +               dev_err(state->dev, "Unable to obtain link
> > frequency: %d\n",
> > +                       (int)link_freq);
> > +               return link_freq;
> > +       }
> > +
> > +       lane_rate = link_freq * 2;
> > +       if (lane_rate < 80000000 || lane_rate > 1500000000) {
> > +               dev_dbg(state->dev, "Out-of-bound lane rate %u\n",
> > lane_rate);
> > +               return -EINVAL;
> > +       }
> > +
> > +       /* 
> > https://community.nxp.com/t5/i-MX-Processors/Explenation-for-HS-SETTLE-parameter-in-MIPI-CSI-D-PHY-registers/m-p/764275/highlight/true#M118744
> >  */
> > +       if (lane_rate < 250000000)
> > +               state->hs_settle = 0xb;
> > +       else if (lane_rate < 500000000)
> > +               state->hs_settle = 0x8;
> > +       else
> > +               state->hs_settle = 0x6;
> 
> We could possibly compute this value based on the formula from the
> table
> in that page, but maybe that's overkill ? If you want to give it a
> try,
> it would be along those lines.
> 
>         /*
>          * The D-PHY specification requires Ths-settle to be in the
> range
>          * 85ns + 6*UI to 140ns + 10*UI, with the unit interval UI
> being half
>          * the clock period.
>          *
>          * The Ths-settle value is expressed in the hardware as a
> multiple of
>          * the Esc clock period:
>          *
>          * Ths-settle = (PRG_RXHS_SETTLE + 1) * Tperiod of RxClkInEsc
>          *
>          * Due to the one cycle inaccuracy introduced by rounding,
> the
>          * documentation recommends picking a value away from the
> boundaries.
>          * Let's pick the average.
>          */
>         esc_clk_rate = clk_get_rate(...);
> 
>         min_ths_settle = 85 + 6 * 1000000 / (lane_rate / 1000);
>         max_ths_settle = 140 + 10 * 1000000 / (lane_rate / 1000);
>         ths_settle = (min_ths_settle + max_ths_settle) / 2;
> 
>         state->hs_settle = ths_settle * esc_clk_rate / 1000000000 -
> 1;
> 

I experimented a bit but would like to leave this as a task for later
if that's ok. it's correct and simple now. also, using clks[i].clk
based on the name string would feel better to submit seperately later.

> 

Re: [PATCH v6 2/3] media: imx: add a driver for i.MX8MQ mipi csi rx phy and controller

[Laurent Pinchart]

Hi Martin,

On Thu, Jul 15, 2021 at 09:37:24AM +0200, Martin Kepplinger wrote:
> Am Mittwoch, dem 14.07.2021 um 21:24 +0300 schrieb Laurent Pinchart:
> > On Wed, Jul 14, 2021 at 01:19:30PM +0200, Martin Kepplinger wrote:
> > > Add a driver to support the i.MX8MQ MIPI CSI receiver. The hardware side
> > > is based on
> > > https://source.codeaurora.org/external/imx/linux-imx/tree/drivers/media/platform/imx8/mxc-mipi-csi2_yav.c?h=imx_5.4.70_2.3.0
> > > 
> > > It's built as part of VIDEO_IMX7_CSI because that's documented to support
> > > i.MX8M platforms. This driver adds i.MX8MQ support where currently only the
> > > i.MX8MM platform has been supported.
> > > 
> > > Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm>
> > > ---
> > >  drivers/staging/media/imx/Makefile           |   1 +
> > >  drivers/staging/media/imx/imx8mq-mipi-csi2.c | 949 +++++++++++++++++++
> > >  2 files changed, 950 insertions(+)
> > >  create mode 100644 drivers/staging/media/imx/imx8mq-mipi-csi2.c
> > > 
> > > diff --git a/drivers/staging/media/imx/Makefile
> > > b/drivers/staging/media/imx/Makefile
> > > index 6ac33275cc97..19c2fc54d424 100644
> > > --- a/drivers/staging/media/imx/Makefile
> > > +++ b/drivers/staging/media/imx/Makefile
> > > @@ -16,3 +16,4 @@ obj-$(CONFIG_VIDEO_IMX_CSI) += imx6-mipi-csi2.o

[snip]

> > > +static int imx8mq_mipi_csi_calc_hs_settle(struct csi_state *state)
> > > +{
> > > +       u32 width = state->format_mbus[MIPI_CSI2_PAD_SINK].width;
> > > +       u32 height = state->format_mbus[MIPI_CSI2_PAD_SINK].height;
> > > +       s64 link_freq;
> > > +       u32 lane_rate;
> > > +
> > > +       /* Calculate the line rate from the pixel rate. */
> > > +       link_freq = v4l2_get_link_freq(state->src_sd->ctrl_handler,
> > > +                                      state->csi2_fmt->width,
> > > +                                      state->bus.num_data_lanes * 2);
> > > +       if (link_freq < 0) {
> > > +               dev_err(state->dev, "Unable to obtain link frequency: %d\n",
> > > +                       (int)link_freq);
> > > +               return link_freq;
> > > +       }
> > > +
> > > +       lane_rate = link_freq * 2;
> > > +       if (lane_rate < 80000000 || lane_rate > 1500000000) {
> > > +               dev_dbg(state->dev, "Out-of-bound lane rate %u\n", lane_rate);
> > > +               return -EINVAL;
> > > +       }
> > > +
> > > +       /* https://community.nxp.com/t5/i-MX-Processors/Explenation-for-HS-SETTLE-parameter-in-MIPI-CSI-D-PHY-registers/m-p/764275/highlight/true#M118744 */
> > > +       if (lane_rate < 250000000)
> > > +               state->hs_settle = 0xb;
> > > +       else if (lane_rate < 500000000)
> > > +               state->hs_settle = 0x8;
> > > +       else
> > > +               state->hs_settle = 0x6;
> > 
> > We could possibly compute this value based on the formula from the table
> > in that page, but maybe that's overkill ? If you want to give it a try,
> > it would be along those lines.
> > 
> >         /*
> >          * The D-PHY specification requires Ths-settle to be in the range
> >          * 85ns + 6*UI to 140ns + 10*UI, with the unit interval UI being half
> >          * the clock period.
> >          *
> >          * The Ths-settle value is expressed in the hardware as a multiple of
> >          * the Esc clock period:
> >          *
> >          * Ths-settle = (PRG_RXHS_SETTLE + 1) * Tperiod of RxClkInEsc
> >          *
> >          * Due to the one cycle inaccuracy introduced by rounding, the
> >          * documentation recommends picking a value away from the boundaries.
> >          * Let's pick the average.
> >          */
> >         esc_clk_rate = clk_get_rate(...);
> > 
> >         min_ths_settle = 85 + 6 * 1000000 / (lane_rate / 1000);
> >         max_ths_settle = 140 + 10 * 1000000 / (lane_rate / 1000);
> >         ths_settle = (min_ths_settle + max_ths_settle) / 2;
> > 
> >         state->hs_settle = ths_settle * esc_clk_rate / 1000000000 - 1;
> 
> I experimented a bit but would like to leave this as a task for later
> if that's ok. it's correct and simple now. also, using clks[i].clk
> based on the name string would feel better to submit seperately later.

That's OK with me, but I may then submit a patch on top fairly soon :-)
Have you been able to test if this code works on your device ? The main
reason why I think it's better is that it doesn't hardcode a specific
escape clock frequency assumption, so it should be able to accommodate a
wider range of use cases. If we change it later, there's always a risk
of regressions, while if we do this from the start, we'll figure out
quickly if it doesn't work in some cases.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v6 2/3] media: imx: add a driver for i.MX8MQ mipi csi rx phy and controller

[Martin Kepplinger]

Am Freitag, dem 16.07.2021 um 00:52 +0300 schrieb Laurent Pinchart:
> Hi Martin,
> 
> On Thu, Jul 15, 2021 at 09:37:24AM +0200, Martin Kepplinger wrote:
> > Am Mittwoch, dem 14.07.2021 um 21:24 +0300 schrieb Laurent
> > Pinchart:
> > > On Wed, Jul 14, 2021 at 01:19:30PM +0200, Martin Kepplinger
> > > wrote:
> > > > Add a driver to support the i.MX8MQ MIPI CSI receiver. The
> > > > hardware side
> > > > is based on
> > > > https://source.codeaurora.org/external/imx/linux-imx/tree/drivers/media/platform/imx8/mxc-mipi-csi2_yav.c?h=imx_5.4.70_2.3.0
> > > > 
> > > > It's built as part of VIDEO_IMX7_CSI because that's documented
> > > > to support
> > > > i.MX8M platforms. This driver adds i.MX8MQ support where
> > > > currently only the
> > > > i.MX8MM platform has been supported.
> > > > 
> > > > Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm>
> > > > ---
> > > >  drivers/staging/media/imx/Makefile           |   1 +
> > > >  drivers/staging/media/imx/imx8mq-mipi-csi2.c | 949
> > > > +++++++++++++++++++
> > > >  2 files changed, 950 insertions(+)
> > > >  create mode 100644 drivers/staging/media/imx/imx8mq-mipi-
> > > > csi2.c
> > > > 
> > > > diff --git a/drivers/staging/media/imx/Makefile
> > > > b/drivers/staging/media/imx/Makefile
> > > > index 6ac33275cc97..19c2fc54d424 100644
> > > > --- a/drivers/staging/media/imx/Makefile
> > > > +++ b/drivers/staging/media/imx/Makefile
> > > > @@ -16,3 +16,4 @@ obj-$(CONFIG_VIDEO_IMX_CSI) += imx6-mipi-
> > > > csi2.o
> 
> [snip]
> 
> > > > +static int imx8mq_mipi_csi_calc_hs_settle(struct csi_state
> > > > *state)
> > > > +{
> > > > +       u32 width = state-
> > > > >format_mbus[MIPI_CSI2_PAD_SINK].width;
> > > > +       u32 height = state-
> > > > >format_mbus[MIPI_CSI2_PAD_SINK].height;
> > > > +       s64 link_freq;
> > > > +       u32 lane_rate;
> > > > +
> > > > +       /* Calculate the line rate from the pixel rate. */
> > > > +       link_freq = v4l2_get_link_freq(state->src_sd-
> > > > >ctrl_handler,
> > > > +                                      state->csi2_fmt->width,
> > > > +                                      state-
> > > > >bus.num_data_lanes * 2);
> > > > +       if (link_freq < 0) {
> > > > +               dev_err(state->dev, "Unable to obtain link
> > > > frequency: %d\n",
> > > > +                       (int)link_freq);
> > > > +               return link_freq;
> > > > +       }
> > > > +
> > > > +       lane_rate = link_freq * 2;
> > > > +       if (lane_rate < 80000000 || lane_rate > 1500000000) {
> > > > +               dev_dbg(state->dev, "Out-of-bound lane rate
> > > > %u\n", lane_rate);
> > > > +               return -EINVAL;
> > > > +       }
> > > > +
> > > > +       /*
> > > > https://community.nxp.com/t5/i-MX-Processors/Explenation-for-HS-SETTLE-parameter-in-MIPI-CSI-D-PHY-registers/m-p/764275/highlight/true#M118744
> > > >  */
> > > > +       if (lane_rate < 250000000)
> > > > +               state->hs_settle = 0xb;
> > > > +       else if (lane_rate < 500000000)
> > > > +               state->hs_settle = 0x8;
> > > > +       else
> > > > +               state->hs_settle = 0x6;
> > > 
> > > We could possibly compute this value based on the formula from
> > > the table
> > > in that page, but maybe that's overkill ? If you want to give it
> > > a try,
> > > it would be along those lines.
> > > 
> > >         /*
> > >          * The D-PHY specification requires Ths-settle to be in
> > > the range
> > >          * 85ns + 6*UI to 140ns + 10*UI, with the unit interval
> > > UI being half
> > >          * the clock period.
> > >          *
> > >          * The Ths-settle value is expressed in the hardware as a
> > > multiple of
> > >          * the Esc clock period:
> > >          *
> > >          * Ths-settle = (PRG_RXHS_SETTLE + 1) * Tperiod of
> > > RxClkInEsc
> > >          *
> > >          * Due to the one cycle inaccuracy introduced by
> > > rounding, the
> > >          * documentation recommends picking a value away from the
> > > boundaries.
> > >          * Let's pick the average.
> > >          */
> > >         esc_clk_rate = clk_get_rate(...);
> > > 
> > >         min_ths_settle = 85 + 6 * 1000000 / (lane_rate / 1000);
> > >         max_ths_settle = 140 + 10 * 1000000 / (lane_rate / 1000);
> > >         ths_settle = (min_ths_settle + max_ths_settle) / 2;
> > > 
> > >         state->hs_settle = ths_settle * esc_clk_rate / 1000000000
> > > - 1;
> > 
> > I experimented a bit but would like to leave this as a task for
> > later
> > if that's ok. it's correct and simple now. also, using clks[i].clk
> > based on the name string would feel better to submit seperately
> > later.
> 
> That's OK with me, but I may then submit a patch on top fairly soon
> :-)
> Have you been able to test if this code works on your device ? The
> main
> reason why I think it's better is that it doesn't hardcode a specific
> escape clock frequency assumption, so it should be able to
> accommodate a
> wider range of use cases. If we change it later, there's always a
> risk
> of regressions, while if we do this from the start, we'll figure out
> quickly if it doesn't work in some cases.
> 

taking your code basically as-is doesn't yet work, but it helps a bit.
tbh I don't even know how to correctly read that table / calculation:
what is the exact relation of the calculated Ths_settle time inverval
to the hs_settle register bits?

if the 2 of us can't quickly figure it out I can ask NXP via that
community forum issue and I created
https://source.puri.sm/Librem5/linux-next/-/issues/340 so I won't
forget about it.

thanks!

Re: [PATCH v6 2/3] media: imx: add a driver for i.MX8MQ mipi csi rx phy and controller

[Laurent Pinchart]

Hi Martin,

On Fri, Jul 16, 2021 at 10:47:14AM +0200, Martin Kepplinger wrote:
> Am Freitag, dem 16.07.2021 um 00:52 +0300 schrieb Laurent Pinchart:
> > On Thu, Jul 15, 2021 at 09:37:24AM +0200, Martin Kepplinger wrote:
> > > Am Mittwoch, dem 14.07.2021 um 21:24 +0300 schrieb Laurent Pinchart:
> > > > On Wed, Jul 14, 2021 at 01:19:30PM +0200, Martin Kepplinger wrote:
> > > > > Add a driver to support the i.MX8MQ MIPI CSI receiver. The hardware side
> > > > > is based on
> > > > > https://source.codeaurora.org/external/imx/linux-imx/tree/drivers/media/platform/imx8/mxc-mipi-csi2_yav.c?h=imx_5.4.70_2.3.0
> > > > > 
> > > > > It's built as part of VIDEO_IMX7_CSI because that's documented to support
> > > > > i.MX8M platforms. This driver adds i.MX8MQ support where currently only the
> > > > > i.MX8MM platform has been supported.
> > > > > 
> > > > > Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm>
> > > > > ---
> > > > >  drivers/staging/media/imx/Makefile           |   1 +
> > > > >  drivers/staging/media/imx/imx8mq-mipi-csi2.c | 949 +++++++++++++++++++
> > > > >  2 files changed, 950 insertions(+)
> > > > >  create mode 100644 drivers/staging/media/imx/imx8mq-mipi-csi2.c
> > > > > 
> > > > > diff --git a/drivers/staging/media/imx/Makefile b/drivers/staging/media/imx/Makefile
> > > > > index 6ac33275cc97..19c2fc54d424 100644
> > > > > --- a/drivers/staging/media/imx/Makefile
> > > > > +++ b/drivers/staging/media/imx/Makefile
> > > > > @@ -16,3 +16,4 @@ obj-$(CONFIG_VIDEO_IMX_CSI) += imx6-mipi-csi2.o
> > 
> > [snip]
> > 
> > > > > +static int imx8mq_mipi_csi_calc_hs_settle(struct csi_state *state)
> > > > > +{
> > > > > +       u32 width = state->format_mbus[MIPI_CSI2_PAD_SINK].width;
> > > > > +       u32 height = state->format_mbus[MIPI_CSI2_PAD_SINK].height;
> > > > > +       s64 link_freq;
> > > > > +       u32 lane_rate;
> > > > > +
> > > > > +       /* Calculate the line rate from the pixel rate. */
> > > > > +       link_freq = v4l2_get_link_freq(state->src_sd->ctrl_handler,
> > > > > +                                      state->csi2_fmt->width,
> > > > > +                                      state->bus.num_data_lanes * 2);
> > > > > +       if (link_freq < 0) {
> > > > > +               dev_err(state->dev, "Unable to obtain link frequency: %d\n",
> > > > > +                       (int)link_freq);
> > > > > +               return link_freq;
> > > > > +       }
> > > > > +
> > > > > +       lane_rate = link_freq * 2;
> > > > > +       if (lane_rate < 80000000 || lane_rate > 1500000000) {
> > > > > +               dev_dbg(state->dev, "Out-of-bound lane rate %u\n", lane_rate);
> > > > > +               return -EINVAL;
> > > > > +       }
> > > > > +
> > > > > +       /* https://community.nxp.com/t5/i-MX-Processors/Explenation-for-HS-SETTLE-parameter-in-MIPI-CSI-D-PHY-registers/m-p/764275/highlight/true#M118744 */
> > > > > +       if (lane_rate < 250000000)
> > > > > +               state->hs_settle = 0xb;
> > > > > +       else if (lane_rate < 500000000)
> > > > > +               state->hs_settle = 0x8;
> > > > > +       else
> > > > > +               state->hs_settle = 0x6;
> > > > 
> > > > We could possibly compute this value based on the formula from the table
> > > > in that page, but maybe that's overkill ? If you want to give it a try,
> > > > it would be along those lines.
> > > > 
> > > >         /*
> > > >          * The D-PHY specification requires Ths-settle to be in the range
> > > >          * 85ns + 6*UI to 140ns + 10*UI, with the unit interval UI being half
> > > >          * the clock period.
> > > >          *
> > > >          * The Ths-settle value is expressed in the hardware as a multiple of
> > > >          * the Esc clock period:
> > > >          *
> > > >          * Ths-settle = (PRG_RXHS_SETTLE + 1) * Tperiod of RxClkInEsc
> > > >          *
> > > >          * Due to the one cycle inaccuracy introduced by rounding, the
> > > >          * documentation recommends picking a value away from the boundaries.
> > > >          * Let's pick the average.
> > > >          */
> > > >         esc_clk_rate = clk_get_rate(...);
> > > > 
> > > >         min_ths_settle = 85 + 6 * 1000000 / (lane_rate / 1000);
> > > >         max_ths_settle = 140 + 10 * 1000000 / (lane_rate / 1000);
> > > >         ths_settle = (min_ths_settle + max_ths_settle) / 2;
> > > > 
> > > >         state->hs_settle = ths_settle * esc_clk_rate / 1000000000 - 1;
> > > 
> > > I experimented a bit but would like to leave this as a task for later
> > > if that's ok. it's correct and simple now. also, using clks[i].clk
> > > based on the name string would feel better to submit seperately
> > > later.
> > 
> > That's OK with me, but I may then submit a patch on top fairly soon :-)
> > Have you been able to test if this code works on your device ? The main
> > reason why I think it's better is that it doesn't hardcode a specific
> > escape clock frequency assumption, so it should be able to accommodate a
> > wider range of use cases. If we change it later, there's always a risk
> > of regressions, while if we do this from the start, we'll figure out
> > quickly if it doesn't work in some cases.
> 
> taking your code basically as-is doesn't yet work, but it helps a bit.

Thanks for testing.

> tbh I don't even know how to correctly read that table / calculation:
> what is the exact relation of the calculated Ths_settle time inverval
> to the hs_settle register bits?

The PRG_RXHS_SETTLE field stores a number of timer ticks to cover the
Ths-settle internal. The D-PHY arms the timer when it detects the
transition to LP-00, and ignores transitions on the lane until the timer
expires. The timer is clocked by the escape clock.

What hs_settle value do you currently use, and what value does my code
produce ?

> if the 2 of us can't quickly figure it out I can ask NXP via that
> community forum issue and I created
> https://source.puri.sm/Librem5/linux-next/-/issues/340 so I won't
> forget about it.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v6 2/3] media: imx: add a driver for i.MX8MQ mipi csi rx phy and controller

[Martin Kepplinger]

Am Freitag, dem 16.07.2021 um 13:47 +0300 schrieb Laurent Pinchart:
> Hi Martin,
> 
> On Fri, Jul 16, 2021 at 10:47:14AM +0200, Martin Kepplinger wrote:
> > Am Freitag, dem 16.07.2021 um 00:52 +0300 schrieb Laurent Pinchart:
> > > On Thu, Jul 15, 2021 at 09:37:24AM +0200, Martin Kepplinger
> > > wrote:
> > > > Am Mittwoch, dem 14.07.2021 um 21:24 +0300 schrieb Laurent
> > > > Pinchart:
> > > > > On Wed, Jul 14, 2021 at 01:19:30PM +0200, Martin Kepplinger
> > > > > wrote:
> > > > > > Add a driver to support the i.MX8MQ MIPI CSI receiver. The
> > > > > > hardware side
> > > > > > is based on
> > > > > > https://source.codeaurora.org/external/imx/linux-imx/tree/drivers/media/platform/imx8/mxc-mipi-csi2_yav.c?h=imx_5.4.70_2.3.0
> > > > > > 
> > > > > > It's built as part of VIDEO_IMX7_CSI because that's
> > > > > > documented to support
> > > > > > i.MX8M platforms. This driver adds i.MX8MQ support where
> > > > > > currently only the
> > > > > > i.MX8MM platform has been supported.
> > > > > > 
> > > > > > Signed-off-by: Martin Kepplinger
> > > > > > <martin.kepplinger@puri.sm>
> > > > > > ---
> > > > > >  drivers/staging/media/imx/Makefile           |   1 +
> > > > > >  drivers/staging/media/imx/imx8mq-mipi-csi2.c | 949
> > > > > > +++++++++++++++++++
> > > > > >  2 files changed, 950 insertions(+)
> > > > > >  create mode 100644 drivers/staging/media/imx/imx8mq-mipi-
> > > > > > csi2.c
> > > > > > 
> > > > > > diff --git a/drivers/staging/media/imx/Makefile
> > > > > > b/drivers/staging/media/imx/Makefile
> > > > > > index 6ac33275cc97..19c2fc54d424 100644
> > > > > > --- a/drivers/staging/media/imx/Makefile
> > > > > > +++ b/drivers/staging/media/imx/Makefile
> > > > > > @@ -16,3 +16,4 @@ obj-$(CONFIG_VIDEO_IMX_CSI) += imx6-mipi-
> > > > > > csi2.o
> > > 
> > > [snip]
> > > 
> > > > > > +static int imx8mq_mipi_csi_calc_hs_settle(struct csi_state
> > > > > > *state)
> > > > > > +{
> > > > > > +       u32 width = state-
> > > > > > >format_mbus[MIPI_CSI2_PAD_SINK].width;
> > > > > > +       u32 height = state-
> > > > > > >format_mbus[MIPI_CSI2_PAD_SINK].height;
> > > > > > +       s64 link_freq;
> > > > > > +       u32 lane_rate;
> > > > > > +
> > > > > > +       /* Calculate the line rate from the pixel rate. */
> > > > > > +       link_freq = v4l2_get_link_freq(state->src_sd-
> > > > > > >ctrl_handler,
> > > > > > +                                      state->csi2_fmt-
> > > > > > >width,
> > > > > > +                                      state-
> > > > > > >bus.num_data_lanes * 2);
> > > > > > +       if (link_freq < 0) {
> > > > > > +               dev_err(state->dev, "Unable to obtain link
> > > > > > frequency: %d\n",
> > > > > > +                       (int)link_freq);
> > > > > > +               return link_freq;
> > > > > > +       }
> > > > > > +
> > > > > > +       lane_rate = link_freq * 2;
> > > > > > +       if (lane_rate < 80000000 || lane_rate > 1500000000)
> > > > > > {
> > > > > > +               dev_dbg(state->dev, "Out-of-bound lane rate
> > > > > > %u\n", lane_rate);
> > > > > > +               return -EINVAL;
> > > > > > +       }
> > > > > > +
> > > > > > +       /*
> > > > > > https://community.nxp.com/t5/i-MX-Processors/Explenation-for-HS-SETTLE-parameter-in-MIPI-CSI-D-PHY-registers/m-p/764275/highlight/true#M118744
> > > > > >  */
> > > > > > +       if (lane_rate < 250000000)
> > > > > > +               state->hs_settle = 0xb;
> > > > > > +       else if (lane_rate < 500000000)
> > > > > > +               state->hs_settle = 0x8;
> > > > > > +       else
> > > > > > +               state->hs_settle = 0x6;
> > > > > 
> > > > > We could possibly compute this value based on the formula
> > > > > from the table
> > > > > in that page, but maybe that's overkill ? If you want to give
> > > > > it a try,
> > > > > it would be along those lines.
> > > > > 
> > > > >         /*
> > > > >          * The D-PHY specification requires Ths-settle to be
> > > > > in the range
> > > > >          * 85ns + 6*UI to 140ns + 10*UI, with the unit
> > > > > interval UI being half
> > > > >          * the clock period.
> > > > >          *
> > > > >          * The Ths-settle value is expressed in the hardware
> > > > > as a multiple of
> > > > >          * the Esc clock period:
> > > > >          *
> > > > >          * Ths-settle = (PRG_RXHS_SETTLE + 1) * Tperiod of
> > > > > RxClkInEsc
> > > > >          *
> > > > >          * Due to the one cycle inaccuracy introduced by
> > > > > rounding, the
> > > > >          * documentation recommends picking a value away from
> > > > > the boundaries.
> > > > >          * Let's pick the average.
> > > > >          */
> > > > >         esc_clk_rate = clk_get_rate(...);
> > > > > 
> > > > >         min_ths_settle = 85 + 6 * 1000000 / (lane_rate /
> > > > > 1000);
> > > > >         max_ths_settle = 140 + 10 * 1000000 / (lane_rate /
> > > > > 1000);
> > > > >         ths_settle = (min_ths_settle + max_ths_settle) / 2;
> > > > > 
> > > > >         state->hs_settle = ths_settle * esc_clk_rate /
> > > > > 1000000000 - 1;
> > > > 
> > > > I experimented a bit but would like to leave this as a task for
> > > > later
> > > > if that's ok. it's correct and simple now. also, using
> > > > clks[i].clk
> > > > based on the name string would feel better to submit seperately
> > > > later.
> > > 
> > > That's OK with me, but I may then submit a patch on top fairly
> > > soon :-)
> > > Have you been able to test if this code works on your device ?
> > > The main
> > > reason why I think it's better is that it doesn't hardcode a
> > > specific
> > > escape clock frequency assumption, so it should be able to
> > > accommodate a
> > > wider range of use cases. If we change it later, there's always a
> > > risk
> > > of regressions, while if we do this from the start, we'll figure
> > > out
> > > quickly if it doesn't work in some cases.
> > 
> > taking your code basically as-is doesn't yet work, but it helps a
> > bit.
> 
> Thanks for testing.
> 
> > tbh I don't even know how to correctly read that table /
> > calculation:
> > what is the exact relation of the calculated Ths_settle time
> > inverval
> > to the hs_settle register bits?
> 
> The PRG_RXHS_SETTLE field stores a number of timer ticks to cover the
> Ths-settle internal. The D-PHY arms the timer when it detects the
> transition to LP-00, and ignores transitions on the lane until the
> timer
> expires. The timer is clocked by the escape clock.
> 
> What hs_settle value do you currently use, and what value does my
> code
> produce ?
> 
> > if the 2 of us can't quickly figure it out I can ask NXP via that
> > community forum issue and I created
> > https://source.puri.sm/Librem5/linux-next/-/issues/340 so I won't
> > forget about it.
> 

hi Laurent,

the below patch for hs_settle works (and calculates either the same or
+/- 1 for the hs_settle value, compared to the table), but getting the
esc clock looks really scary how I do it: how would you do that?


@@ -284,6 +285,9 @@ static int imx8mq_mipi_csi_calc_hs_settle(struct
csi_state *state)
 {
 	s64 link_freq;
 	u32 lane_rate;
+	u32 esc_clk_rate = 0;
+	u32 i, min_ths_settle, max_ths_settle, ths_settle_ns,
esc_clk_period_ns;
+	char *p;
 
 	/* Calculate the line rate from the pixel rate. */
 	link_freq = v4l2_get_link_freq(state->src_sd->ctrl_handler,
@@ -302,20 +306,44 @@ static int imx8mq_mipi_csi_calc_hs_settle(struct
csi_state *state)
 	}
 
 	/*
-	 * The following table is the source for this:
-	 *
https://community.nxp.com/t5/i-MX-Processors/Explenation-for-HS-SETTLE-parameter-in-MIPI-CSI-D-PHY-registers/m-p/764275/highlight/true#M118744
-	 * but it would be even better to calculate the value for any
-	 * given datarate.
+	 * The D-PHY specification requires Ths-settle to be in the
range
+	 * 85ns + 6*UI to 140ns + 10*UI, with the unit interval UI
being half
+	 * the clock period.
+	 *
+	 * The Ths-settle value is expressed in the hardware as a
multiple of
+	 * the Esc clock period:
+	 *
+	 * Ths-settle = (PRG_RXHS_SETTLE + 1) * Tperiod of RxClkInEsc
+	 *
+	 * Due to the one cycle inaccuracy introduced by rounding, the
+	 * documentation recommends picking a value away from the
boundaries.
+	 * Let's pick the average.
 	 */
-	if (lane_rate < 250000000)
-		state->hs_settle = 0xb;
-	else if (lane_rate < 500000000)
-		state->hs_settle = 0x8;
-	else
-		state->hs_settle = 0x6;
-
-	dev_dbg(state->dev, "start stream: lane rate %u hs_settle
%u\n",
-		lane_rate, state->hs_settle);
+	for (i = 0; i < CSI2_NUM_CLKS; i++) {
+		p = (char *)__clk_get_name(state->clks[i].clk);
+		/* we're getting csi1_esc here */
+		if (strlen(p) > 7)
+			p += 5;
+
+		dev_dbg(state->dev, "comparing: %s to esc\n", p);
+		if (!strcmp(p, "esc"))
+			esc_clk_rate = clk_get_rate(state-
>clks[i].clk);
+	}
+
+	if (!esc_clk_rate)
+		dev_err(state->dev, "Could not get esc clock rate\n");
+
+	dev_dbg(state->dev, "esc clk rate: %u\n", esc_clk_rate);
+	esc_clk_period_ns = 1000000000 / esc_clk_rate;
+
+	min_ths_settle = 85 + 6 * 1000000 / (lane_rate / 1000);
+	max_ths_settle = 140 + 10 * 1000000 / (lane_rate / 1000);
+	ths_settle_ns = (min_ths_settle + max_ths_settle) / 2;
+
+	state->hs_settle = ths_settle_ns / esc_clk_period_ns - 1;
+
+	dev_dbg(state->dev, "lane rate %u Ths_settle %u hs_settle
%u\n",
+		lane_rate, ths_settle_ns, state->hs_settle);
 
 	return 0;

Re: [PATCH v6 2/3] media: imx: add a driver for i.MX8MQ mipi csi rx phy and controller

[Martin Kepplinger]

Am Montag, dem 19.07.2021 um 12:46 +0200 schrieb Martin Kepplinger:
> Am Freitag, dem 16.07.2021 um 13:47 +0300 schrieb Laurent Pinchart:
> > Hi Martin,
> > 
> > On Fri, Jul 16, 2021 at 10:47:14AM +0200, Martin Kepplinger wrote:
> > > Am Freitag, dem 16.07.2021 um 00:52 +0300 schrieb Laurent
> > > Pinchart:
> > > > On Thu, Jul 15, 2021 at 09:37:24AM +0200, Martin Kepplinger
> > > > wrote:
> > > > > Am Mittwoch, dem 14.07.2021 um 21:24 +0300 schrieb Laurent
> > > > > Pinchart:
> > > > > > On Wed, Jul 14, 2021 at 01:19:30PM +0200, Martin Kepplinger
> > > > > > wrote:
> > > > > > > Add a driver to support the i.MX8MQ MIPI CSI receiver.
> > > > > > > The
> > > > > > > hardware side
> > > > > > > is based on
> > > > > > > https://source.codeaurora.org/external/imx/linux-imx/tree/drivers/media/platform/imx8/mxc-mipi-csi2_yav.c?h=imx_5.4.70_2.3.0
> > > > > > > 
> > > > > > > It's built as part of VIDEO_IMX7_CSI because that's
> > > > > > > documented to support
> > > > > > > i.MX8M platforms. This driver adds i.MX8MQ support where
> > > > > > > currently only the
> > > > > > > i.MX8MM platform has been supported.
> > > > > > > 
> > > > > > > Signed-off-by: Martin Kepplinger
> > > > > > > <martin.kepplinger@puri.sm>
> > > > > > > ---
> > > > > > >  drivers/staging/media/imx/Makefile           |   1 +
> > > > > > >  drivers/staging/media/imx/imx8mq-mipi-csi2.c | 949
> > > > > > > +++++++++++++++++++
> > > > > > >  2 files changed, 950 insertions(+)
> > > > > > >  create mode 100644 drivers/staging/media/imx/imx8mq-
> > > > > > > mipi-
> > > > > > > csi2.c
> > > > > > > 
> > > > > > > diff --git a/drivers/staging/media/imx/Makefile
> > > > > > > b/drivers/staging/media/imx/Makefile
> > > > > > > index 6ac33275cc97..19c2fc54d424 100644
> > > > > > > --- a/drivers/staging/media/imx/Makefile
> > > > > > > +++ b/drivers/staging/media/imx/Makefile
> > > > > > > @@ -16,3 +16,4 @@ obj-$(CONFIG_VIDEO_IMX_CSI) += imx6-
> > > > > > > mipi-
> > > > > > > csi2.o
> > > > 
> > > > [snip]
> > > > 
> > > > > > > +static int imx8mq_mipi_csi_calc_hs_settle(struct
> > > > > > > csi_state
> > > > > > > *state)
> > > > > > > +{
> > > > > > > +       u32 width = state-
> > > > > > > > format_mbus[MIPI_CSI2_PAD_SINK].width;
> > > > > > > +       u32 height = state-
> > > > > > > > format_mbus[MIPI_CSI2_PAD_SINK].height;
> > > > > > > +       s64 link_freq;
> > > > > > > +       u32 lane_rate;
> > > > > > > +
> > > > > > > +       /* Calculate the line rate from the pixel rate.
> > > > > > > */
> > > > > > > +       link_freq = v4l2_get_link_freq(state->src_sd-
> > > > > > > > ctrl_handler,
> > > > > > > +                                      state->csi2_fmt-
> > > > > > > > width,
> > > > > > > +                                      state-
> > > > > > > > bus.num_data_lanes * 2);
> > > > > > > +       if (link_freq < 0) {
> > > > > > > +               dev_err(state->dev, "Unable to obtain
> > > > > > > link
> > > > > > > frequency: %d\n",
> > > > > > > +                       (int)link_freq);
> > > > > > > +               return link_freq;
> > > > > > > +       }
> > > > > > > +
> > > > > > > +       lane_rate = link_freq * 2;
> > > > > > > +       if (lane_rate < 80000000 || lane_rate >
> > > > > > > 1500000000)
> > > > > > > {
> > > > > > > +               dev_dbg(state->dev, "Out-of-bound lane
> > > > > > > rate
> > > > > > > %u\n", lane_rate);
> > > > > > > +               return -EINVAL;
> > > > > > > +       }
> > > > > > > +
> > > > > > > +       /*
> > > > > > > https://community.nxp.com/t5/i-MX-Processors/Explenation-for-HS-SETTLE-parameter-in-MIPI-CSI-D-PHY-registers/m-p/764275/highlight/true#M118744
> > > > > > >  */
> > > > > > > +       if (lane_rate < 250000000)
> > > > > > > +               state->hs_settle = 0xb;
> > > > > > > +       else if (lane_rate < 500000000)
> > > > > > > +               state->hs_settle = 0x8;
> > > > > > > +       else
> > > > > > > +               state->hs_settle = 0x6;
> > > > > > 
> > > > > > We could possibly compute this value based on the formula
> > > > > > from the table
> > > > > > in that page, but maybe that's overkill ? If you want to
> > > > > > give
> > > > > > it a try,
> > > > > > it would be along those lines.
> > > > > > 
> > > > > >         /*
> > > > > >          * The D-PHY specification requires Ths-settle to
> > > > > > be
> > > > > > in the range
> > > > > >          * 85ns + 6*UI to 140ns + 10*UI, with the unit
> > > > > > interval UI being half
> > > > > >          * the clock period.
> > > > > >          *
> > > > > >          * The Ths-settle value is expressed in the
> > > > > > hardware
> > > > > > as a multiple of
> > > > > >          * the Esc clock period:
> > > > > >          *
> > > > > >          * Ths-settle = (PRG_RXHS_SETTLE + 1) * Tperiod of
> > > > > > RxClkInEsc
> > > > > >          *
> > > > > >          * Due to the one cycle inaccuracy introduced by
> > > > > > rounding, the
> > > > > >          * documentation recommends picking a value away
> > > > > > from
> > > > > > the boundaries.
> > > > > >          * Let's pick the average.
> > > > > >          */
> > > > > >         esc_clk_rate = clk_get_rate(...);
> > > > > > 
> > > > > >         min_ths_settle = 85 + 6 * 1000000 / (lane_rate /
> > > > > > 1000);
> > > > > >         max_ths_settle = 140 + 10 * 1000000 / (lane_rate /
> > > > > > 1000);
> > > > > >         ths_settle = (min_ths_settle + max_ths_settle) / 2;
> > > > > > 
> > > > > >         state->hs_settle = ths_settle * esc_clk_rate /
> > > > > > 1000000000 - 1;
> > > > > 
> > > > > I experimented a bit but would like to leave this as a task
> > > > > for
> > > > > later
> > > > > if that's ok. it's correct and simple now. also, using
> > > > > clks[i].clk
> > > > > based on the name string would feel better to submit
> > > > > seperately
> > > > > later.
> > > > 
> > > > That's OK with me, but I may then submit a patch on top fairly
> > > > soon :-)
> > > > Have you been able to test if this code works on your device ?
> > > > The main
> > > > reason why I think it's better is that it doesn't hardcode a
> > > > specific
> > > > escape clock frequency assumption, so it should be able to
> > > > accommodate a
> > > > wider range of use cases. If we change it later, there's always
> > > > a
> > > > risk
> > > > of regressions, while if we do this from the start, we'll
> > > > figure
> > > > out
> > > > quickly if it doesn't work in some cases.
> > > 
> > > taking your code basically as-is doesn't yet work, but it helps a
> > > bit.
> > 
> > Thanks for testing.
> > 
> > > tbh I don't even know how to correctly read that table /
> > > calculation:
> > > what is the exact relation of the calculated Ths_settle time
> > > inverval
> > > to the hs_settle register bits?
> > 
> > The PRG_RXHS_SETTLE field stores a number of timer ticks to cover
> > the
> > Ths-settle internal. The D-PHY arms the timer when it detects the
> > transition to LP-00, and ignores transitions on the lane until the
> > timer
> > expires. The timer is clocked by the escape clock.
> > 
> > What hs_settle value do you currently use, and what value does my
> > code
> > produce ?
> > 
> > > if the 2 of us can't quickly figure it out I can ask NXP via that
> > > community forum issue and I created
> > > https://source.puri.sm/Librem5/linux-next/-/issues/340 so I won't
> > > forget about it.
> > 
> 
> hi Laurent,
> 
> the below patch for hs_settle works (and calculates either the same
> or
> +/- 1 for the hs_settle value, compared to the table), but getting
> the
> esc clock looks really scary how I do it: how would you do that?
> 
> 
> @@ -284,6 +285,9 @@ static int imx8mq_mipi_csi_calc_hs_settle(struct
> csi_state *state)
>  {
>         s64 link_freq;
>         u32 lane_rate;
> +       u32 esc_clk_rate = 0;
> +       u32 i, min_ths_settle, max_ths_settle, ths_settle_ns,
> esc_clk_period_ns;
> +       char *p;
>  
>         /* Calculate the line rate from the pixel rate. */
>         link_freq = v4l2_get_link_freq(state->src_sd->ctrl_handler,
> @@ -302,20 +306,44 @@ static int
> imx8mq_mipi_csi_calc_hs_settle(struct
> csi_state *state)
>         }
>  
>         /*
> -        * The following table is the source for this:
> -        *
> https://community.nxp.com/t5/i-MX-Processors/Explenation-for-HS-SETTLE-parameter-in-MIPI-CSI-D-PHY-registers/m-p/764275/highlight/true#M118744
> -        * but it would be even better to calculate the value for any
> -        * given datarate.
> +        * The D-PHY specification requires Ths-settle to be in the
> range
> +        * 85ns + 6*UI to 140ns + 10*UI, with the unit interval UI
> being half
> +        * the clock period.
> +        *
> +        * The Ths-settle value is expressed in the hardware as a
> multiple of
> +        * the Esc clock period:
> +        *
> +        * Ths-settle = (PRG_RXHS_SETTLE + 1) * Tperiod of RxClkInEsc
> +        *
> +        * Due to the one cycle inaccuracy introduced by rounding,
> the
> +        * documentation recommends picking a value away from the
> boundaries.
> +        * Let's pick the average.
>          */
> -       if (lane_rate < 250000000)
> -               state->hs_settle = 0xb;
> -       else if (lane_rate < 500000000)
> -               state->hs_settle = 0x8;
> -       else
> -               state->hs_settle = 0x6;
> -
> -       dev_dbg(state->dev, "start stream: lane rate %u hs_settle
> %u\n",
> -               lane_rate, state->hs_settle);
> +       for (i = 0; i < CSI2_NUM_CLKS; i++) {
> +               p = (char *)__clk_get_name(state->clks[i].clk);
> +               /* we're getting csi1_esc here */
> +               if (strlen(p) > 7)
> +                       p += 5;
> +
> +               dev_dbg(state->dev, "comparing: %s to esc\n", p);
> +               if (!strcmp(p, "esc"))
> +                       esc_clk_rate = clk_get_rate(state-
> > clks[i].clk);
> +       }
> +
> +       if (!esc_clk_rate)
> +               dev_err(state->dev, "Could not get esc clock
> rate\n");
> +
> +       dev_dbg(state->dev, "esc clk rate: %u\n", esc_clk_rate);
> +       esc_clk_period_ns = 1000000000 / esc_clk_rate;
> +
> +       min_ths_settle = 85 + 6 * 1000000 / (lane_rate / 1000);
> +       max_ths_settle = 140 + 10 * 1000000 / (lane_rate / 1000);
> +       ths_settle_ns = (min_ths_settle + max_ths_settle) / 2;
> +
> +       state->hs_settle = ths_settle_ns / esc_clk_period_ns - 1;
> +
> +       dev_dbg(state->dev, "lane rate %u Ths_settle %u hs_settle
> %u\n",
> +               lane_rate, ths_settle_ns, state->hs_settle);
>  
>         return 0;
> 

the below code works too and looks better to me. Other drivers do it
similarly when looking for a clock:


	for (i = 0; i < CSI2_NUM_CLKS; i++) {
		p = (char *)__clk_get_name(state->clks[i].clk);

		dev_dbg(state->dev, "looking for esc clock: %s\n", p);

		if (!strcmp(p, "esc") || !strcmp(p, "csi1_esc") ||
		    !strcmp(p, "csi2_esc"))
			esc_clk_rate = clk_get_rate(state-
>clks[i].clk);
	}

	if (!esc_clk_rate) {
		dev_err(state->dev, "Could not find esc clock.\n");
		return -EINVAL;
	}

	dev_dbg(state->dev, "esc clk rate: %u\n", esc_clk_rate);
	esc_clk_period_ns = 1000000000 / esc_clk_rate;

	min_ths_settle = 85 + 6 * 1000000 / (lane_rate / 1000);
	max_ths_settle = 140 + 10 * 1000000 / (lane_rate / 1000);
	ths_settle_ns = (min_ths_settle + max_ths_settle) / 2;

	state->hs_settle = ths_settle_ns / esc_clk_period_ns - 1;

[PATCH v6 3/3] arm64: dts: imx8mq: add mipi csi phy and csi bridge descriptions

[Martin Kepplinger]

Describe the 2 available CSI interfaces on the i.MX8MQ with the MIPI-CSI2
receiver (new driver) and the CSI Bridge that provides the user buffers
(existing driver).

An image sensor is to be connected to the MIPIs' second port, to be described
in board files.

Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm>
---
 arch/arm64/boot/dts/freescale/imx8mq.dtsi | 104 ++++++++++++++++++++++
 1 file changed, 104 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
index 91df9c5350ae..e026a39bddce 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -1099,6 +1099,110 @@ uart4: serial@30a60000 {
 				status = "disabled";
 			};
 
+			mipi_csi1: csi@30a70000 {
+				compatible = "fsl,imx8mq-mipi-csi2";
+				reg = <0x30a70000 0x1000>;
+				clocks = <&clk IMX8MQ_CLK_CSI1_CORE>,
+				   <&clk IMX8MQ_CLK_CSI1_ESC>,
+				   <&clk IMX8MQ_CLK_CSI1_PHY_REF>;
+				clock-names = "core", "esc", "ui";
+				assigned-clocks = <&clk IMX8MQ_CLK_CSI1_CORE>,
+				    <&clk IMX8MQ_CLK_CSI1_PHY_REF>,
+				    <&clk IMX8MQ_CLK_CSI1_ESC>;
+				assigned-clock-rates = <266000000>, <333000000>, <66000000>;
+				assigned-clock-parents = <&clk IMX8MQ_SYS1_PLL_266M>,
+					<&clk IMX8MQ_SYS2_PLL_1000M>,
+					<&clk IMX8MQ_SYS1_PLL_800M>;
+				power-domains = <&pgc_mipi_csi1>;
+				resets = <&src IMX8MQ_RESET_MIPI_CSI1_CORE_RESET>,
+					 <&src IMX8MQ_RESET_MIPI_CSI1_PHY_REF_RESET>,
+					 <&src IMX8MQ_RESET_MIPI_CSI1_ESC_RESET>;
+				fsl,mipi-phy-gpr = <&iomuxc_gpr 0x88>;
+				interconnects = <&noc IMX8MQ_ICM_CSI1 &noc IMX8MQ_ICS_DRAM>;
+				interconnect-names = "dram";
+				status = "disabled";
+
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					port@0 {
+						reg = <0>;
+
+						csi1_mipi_ep: endpoint {
+							remote-endpoint = <&csi1_ep>;
+						};
+					};
+				};
+			};
+
+			csi1: csi@30a90000 {
+				compatible = "fsl,imx8mq-csi", "fsl,imx7-csi";
+				reg = <0x30a90000 0x10000>;
+				interrupts = <GIC_SPI 42 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&clk IMX8MQ_CLK_CSI1_ROOT>;
+				clock-names = "mclk";
+				status = "disabled";
+
+				port {
+					csi1_ep: endpoint {
+						remote-endpoint = <&csi1_mipi_ep>;
+					};
+				};
+			};
+
+			mipi_csi2: csi@30b60000 {
+				compatible = "fsl,imx8mq-mipi-csi2";
+				reg = <0x30b60000 0x1000>;
+				clocks = <&clk IMX8MQ_CLK_CSI2_CORE>,
+				   <&clk IMX8MQ_CLK_CSI2_ESC>,
+				   <&clk IMX8MQ_CLK_CSI2_PHY_REF>;
+				clock-names = "core", "esc", "ui";
+				assigned-clocks = <&clk IMX8MQ_CLK_CSI2_CORE>,
+				    <&clk IMX8MQ_CLK_CSI2_PHY_REF>,
+				    <&clk IMX8MQ_CLK_CSI2_ESC>;
+				assigned-clock-rates = <266000000>, <333000000>, <66000000>;
+				assigned-clock-parents = <&clk IMX8MQ_SYS1_PLL_266M>,
+					<&clk IMX8MQ_SYS2_PLL_1000M>,
+					<&clk IMX8MQ_SYS1_PLL_800M>;
+				power-domains = <&pgc_mipi_csi2>;
+				resets = <&src IMX8MQ_RESET_MIPI_CSI2_CORE_RESET>,
+					 <&src IMX8MQ_RESET_MIPI_CSI2_PHY_REF_RESET>,
+					 <&src IMX8MQ_RESET_MIPI_CSI2_ESC_RESET>;
+				fsl,mipi-phy-gpr = <&iomuxc_gpr 0xa4>;
+				interconnects = <&noc IMX8MQ_ICM_CSI2 &noc IMX8MQ_ICS_DRAM>;
+				interconnect-names = "dram";
+				status = "disabled";
+
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					port@0 {
+						reg = <0>;
+
+						csi2_mipi_ep: endpoint {
+							remote-endpoint = <&csi2_ep>;
+						};
+					};
+				};
+			};
+
+			csi2: csi@30b80000 {
+				compatible = "fsl,imx8mq-csi", "fsl,imx7-csi";
+				reg = <0x30b80000 0x10000>;
+				interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&clk IMX8MQ_CLK_CSI2_ROOT>;
+				clock-names = "mclk";
+				status = "disabled";
+
+				port {
+					csi2_ep: endpoint {
+						remote-endpoint = <&csi2_mipi_ep>;
+					};
+				};
+			};
+
 			mu: mailbox@30aa0000 {
 				compatible = "fsl,imx8mq-mu", "fsl,imx6sx-mu";
 				reg = <0x30aa0000 0x10000>;
-- 
2.30.2