[PATCH v3 00/14] media: rkisp1: Add support for i.MX8MP

[PATCH v3 00/14] media: rkisp1: Add support for i.MX8MP

[Paul Elder]

This series depends on v3 of "dt-bindings: media: Add macros for video
interface bus types" [1].

This series extends the rkisp1 driver to support the ISP found in the
NXP i.MX8MP SoC.

The ISP IP cores in the Rockchip RK3399 (known as the "Rockchip ISP1")
and in the NXP i.MX8MP have the same origin, and have slightly diverged
over time as they are now independently developed (afaik) by Rockchip
and VeriSilicon. The latter is marketed under the name "ISP8000Nano",
and is close enough to the RK3399 ISP that it can easily be supported by
the same driver.

The last two patches add support for UYVY output format, which can be
implemented on the ISP version in the i.MX8MP but not in the one in the
RK3399.

This version of the series specifically has been tested on a Polyhex
Debix model A with an imx219 (Raspberry Pi cam v2).

[1] https://lore.kernel.org/linux-media/20220615221410.27459-2-laurent.pinchart@ideasonboard.com/

Laurent Pinchart (3):
  dt-bindings: media: rkisp1: Add i.MX8MP ISP example
  media: rkisp1: Add and use rkisp1_has_feature() macro
  media: rkisp1: Configure gasket on i.MX8MP

Paul Elder (11):
  dt-bindings: media: rkisp1: Add i.MX8MP ISP to compatible
  media: rkisp1: Add match data for i.MX8MP ISP
  media: rkisp1: Add and set registers for crop for i.MX8MP
  media: rkisp1: Add and set registers for output size config on i.MX8MP
  media: rkisp1: Add i.MX8MP-specific registers for MI and resizer
  media: rkisp1: Shift DMA buffer addresses on i.MX8MP
  media: rkisp1: Add register definitions for the test pattern generator
  media: rkisp1: Fix RSZ_CTRL bits for i.MX8MP
  media: rkisp1: Support devices without self path
  media: rkisp1: Add YC swap capability
  media: rkisp1: Add UYVY as an output format

 .../bindings/media/rockchip-isp1.yaml         |  79 ++++++++++-
 .../platform/rockchip/rkisp1/rkisp1-capture.c | 102 +++++++++++---
 .../platform/rockchip/rkisp1/rkisp1-common.h  |  32 +++++
 .../platform/rockchip/rkisp1/rkisp1-debug.c   |  14 +-
 .../platform/rockchip/rkisp1/rkisp1-dev.c     |  67 +++++++--
 .../platform/rockchip/rkisp1/rkisp1-isp.c     | 128 +++++++++++++++++-
 .../platform/rockchip/rkisp1/rkisp1-regs.h    |  90 ++++++++++++
 .../platform/rockchip/rkisp1/rkisp1-resizer.c |  35 ++++-
 include/uapi/linux/rkisp1-config.h            |   2 +
 9 files changed, 509 insertions(+), 40 deletions(-)

-- 
2.35.1

[PATCH v3 01/14] dt-bindings: media: rkisp1: Add i.MX8MP ISP to compatible

[Paul Elder]

The i.MX8MP ISP is compatbile with the rkisp1 driver. Add it to the list
of compatible strings. While at it, expand on the description of the
clocks to make it clear which clock in the i.MX8MP ISP they map to,
based on the names from the datasheet (which are confusing).

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 Documentation/devicetree/bindings/media/rockchip-isp1.yaml | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/rockchip-isp1.yaml b/Documentation/devicetree/bindings/media/rockchip-isp1.yaml
index b3661d7d4357..95cf945f7ac5 100644
--- a/Documentation/devicetree/bindings/media/rockchip-isp1.yaml
+++ b/Documentation/devicetree/bindings/media/rockchip-isp1.yaml
@@ -16,6 +16,7 @@ description: |
 properties:
   compatible:
     enum:
+      - fsl,imx8mp-isp
       - rockchip,px30-cif-isp
       - rockchip,rk3399-cif-isp
 
@@ -36,9 +37,9 @@ properties:
     minItems: 3
     items:
       # isp0 and isp1
-      - description: ISP clock
-      - description: ISP AXI clock
-      - description: ISP AHB clock
+      - description: ISP clock (for imx8mp, clk)
+      - description: ISP AXI clock (for imx8mp, m_hclk)
+      - description: ISP AHB clock (for imx8mp, hclk)
       # only for isp1
       - description: ISP Pixel clock
 
-- 
2.35.1

[PATCH v3 02/14] dt-bindings: media: rkisp1: Add i.MX8MP ISP example

[Paul Elder]

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Add an example to the rockchip-isp1 DT binding that showcases usage of
the parallel input of the ISP, connected to the CSI-2 receiver internal
to the i.MX8MP.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 .../bindings/media/rockchip-isp1.yaml         | 72 +++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/rockchip-isp1.yaml b/Documentation/devicetree/bindings/media/rockchip-isp1.yaml
index 95cf945f7ac5..88d9bc378f79 100644
--- a/Documentation/devicetree/bindings/media/rockchip-isp1.yaml
+++ b/Documentation/devicetree/bindings/media/rockchip-isp1.yaml
@@ -285,3 +285,75 @@ examples:
             };
         };
     };
+
+  - |
+    #include <dt-bindings/clock/imx8mp-clock.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/media/video-interfaces.h>
+    #include <dt-bindings/power/imx8mp-power.h>
+
+    parent2: parent {
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        isp@32e10000 {
+            compatible = "fsl,imx8mp-isp";
+            reg = <0x32e10000 0x10000>;
+            interrupts = <GIC_SPI 74 IRQ_TYPE_LEVEL_HIGH>;
+            clocks = <&clk IMX8MP_CLK_MEDIA_ISP_ROOT>,
+                     <&clk IMX8MP_CLK_MEDIA_AXI_ROOT>,
+                     <&clk IMX8MP_CLK_MEDIA_APB_ROOT>;
+            clock-names = "isp", "hclk", "aclk";
+            assigned-clocks = <&clk IMX8MP_CLK_MEDIA_ISP>;
+            assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_500M>;
+            assigned-clock-rates = <500000000>;
+            power-domains = <&media_blk_ctrl IMX8MP_MEDIABLK_PD_ISP>;
+            fsl,blk-ctrl = <&media_blk_ctrl 0>;
+
+            ports {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                port@1 {
+                    reg = <1>;
+                    isp0_in: endpoint {
+                        bus-type = <MEDIA_BUS_TYPE_PARALLEL>;
+                        remote-endpoint = <&mipi_csi_0_out>;
+                    };
+                };
+            };
+        };
+
+        csi@32e40000 {
+            compatible = "fsl,imx8mp-mipi-csi2", "fsl,imx8mm-mipi-csi2";
+            reg = <0x32e40000 0x10000>;
+            interrupts = <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;
+            clock-frequency = <500000000>;
+            clocks = <&clk IMX8MP_CLK_MEDIA_APB_ROOT>,
+                     <&clk IMX8MP_CLK_MEDIA_CAM1_PIX_ROOT>,
+                     <&clk IMX8MP_CLK_MEDIA_MIPI_PHY1_REF_ROOT>,
+                     <&clk IMX8MP_CLK_MEDIA_AXI_ROOT>;
+            clock-names = "pclk", "wrap", "phy", "axi";
+            assigned-clocks = <&clk IMX8MP_CLK_MEDIA_CAM1_PIX>;
+            assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_1000M>;
+            assigned-clock-rates = <500000000>;
+            power-domains = <&media_blk_ctrl IMX8MP_MEDIABLK_PD_MIPI_CSI2_1>;
+
+            ports {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                port@0 {
+                    reg = <0>;
+                };
+
+                port@1 {
+                    reg = <1>;
+                    mipi_csi_0_out: endpoint {
+                        remote-endpoint = <&isp0_in>;
+                    };
+                };
+            };
+        };
+    };
+...
-- 
2.35.1

[PATCH v3 03/14] media: rkisp1: Add and use rkisp1_has_feature() macro

[Paul Elder]

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Simplify feature tests with a macro that shortens lines.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 .../media/platform/rockchip/rkisp1/rkisp1-common.h |  3 +++
 .../media/platform/rockchip/rkisp1/rkisp1-dev.c    | 14 +++++++-------
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
index a1293c45aae1..fc33c185b99f 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
@@ -111,6 +111,9 @@ enum rkisp1_feature {
 	RKISP1_FEATURE_MIPI_CSI2 = BIT(0),
 };
 
+#define rkisp1_has_feature(rkisp1, feature) \
+	((rkisp1)->info->features & RKISP1_FEATURE_##feature)
+
 /*
  * struct rkisp1_info - Model-specific ISP Information
  *
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
index f2475c6235ea..e348d8c86861 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
@@ -206,7 +206,7 @@ static int rkisp1_subdev_notifier_register(struct rkisp1_device *rkisp1)
 		switch (reg) {
 		case 0:
 			/* MIPI CSI-2 port */
-			if (!(rkisp1->info->features & RKISP1_FEATURE_MIPI_CSI2)) {
+			if (!rkisp1_has_feature(rkisp1, MIPI_CSI2)) {
 				dev_err(rkisp1->dev,
 					"internal CSI must be available for port 0\n");
 				ret = -EINVAL;
@@ -338,7 +338,7 @@ static int rkisp1_create_links(struct rkisp1_device *rkisp1)
 	unsigned int i;
 	int ret;
 
-	if (rkisp1->info->features & RKISP1_FEATURE_MIPI_CSI2) {
+	if (rkisp1_has_feature(rkisp1, MIPI_CSI2)) {
 		/* Link the CSI receiver to the ISP. */
 		ret = media_create_pad_link(&rkisp1->csi.sd.entity,
 					    RKISP1_CSI_PAD_SRC,
@@ -390,7 +390,7 @@ static int rkisp1_create_links(struct rkisp1_device *rkisp1)
 
 static void rkisp1_entities_unregister(struct rkisp1_device *rkisp1)
 {
-	if (rkisp1->info->features & RKISP1_FEATURE_MIPI_CSI2)
+	if (rkisp1_has_feature(rkisp1, MIPI_CSI2))
 		rkisp1_csi_unregister(rkisp1);
 	rkisp1_params_unregister(rkisp1);
 	rkisp1_stats_unregister(rkisp1);
@@ -423,7 +423,7 @@ static int rkisp1_entities_register(struct rkisp1_device *rkisp1)
 	if (ret)
 		goto error;
 
-	if (rkisp1->info->features & RKISP1_FEATURE_MIPI_CSI2) {
+	if (rkisp1_has_feature(rkisp1, MIPI_CSI2)) {
 		ret = rkisp1_csi_register(rkisp1);
 		if (ret)
 			goto error;
@@ -590,7 +590,7 @@ static int rkisp1_probe(struct platform_device *pdev)
 		goto err_unreg_v4l2_dev;
 	}
 
-	if (rkisp1->info->features & RKISP1_FEATURE_MIPI_CSI2) {
+	if (rkisp1_has_feature(rkisp1, MIPI_CSI2)) {
 		ret = rkisp1_csi_init(rkisp1);
 		if (ret)
 			goto err_unreg_media_dev;
@@ -611,7 +611,7 @@ static int rkisp1_probe(struct platform_device *pdev)
 err_unreg_entities:
 	rkisp1_entities_unregister(rkisp1);
 err_cleanup_csi:
-	if (rkisp1->info->features & RKISP1_FEATURE_MIPI_CSI2)
+	if (rkisp1_has_feature(rkisp1, MIPI_CSI2))
 		rkisp1_csi_cleanup(rkisp1);
 err_unreg_media_dev:
 	media_device_unregister(&rkisp1->media_dev);
@@ -630,7 +630,7 @@ static int rkisp1_remove(struct platform_device *pdev)
 	v4l2_async_nf_cleanup(&rkisp1->notifier);
 
 	rkisp1_entities_unregister(rkisp1);
-	if (rkisp1->info->features & RKISP1_FEATURE_MIPI_CSI2)
+	if (rkisp1_has_feature(rkisp1, MIPI_CSI2))
 		rkisp1_csi_cleanup(rkisp1);
 	rkisp1_debug_cleanup(rkisp1);
 
-- 
2.35.1

[PATCH v3 04/14] media: rkisp1: Add match data for i.MX8MP ISP

[Paul Elder]

Add match data to the rkisp1 driver to match the i.MX8MP ISP.

Although the new version number isn't very precise, it ought to be fine
as the other version numbers aren't precise either, and we have separate
feature flags for important version-specific features. Despite this
version number being seemingly unimportant, it is added to distinguish
it from the ISP versions integrated in rockchip SoCs.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Rob Herring <robh@kernel.org>

---
Changes in v3:
- Remove todo for improving the version number
- Expand the commit message to address the version number
---
 .../platform/rockchip/rkisp1/rkisp1-dev.c     | 22 +++++++++++++++++++
 include/uapi/linux/rkisp1-config.h            |  2 ++
 2 files changed, 24 insertions(+)

diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
index e348d8c86861..69464ce91d59 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
@@ -496,6 +496,24 @@ static const struct rkisp1_info rk3399_isp_info = {
 	.features = RKISP1_FEATURE_MIPI_CSI2,
 };
 
+static const char * const imx8mp_isp_clks[] = {
+	"isp",
+	"hclk",
+	"aclk",
+};
+
+static const struct rkisp1_isr_data imx8mp_isp_isrs[] = {
+	{ NULL, rkisp1_isr },
+};
+
+static const struct rkisp1_info imx8mp_isp_info = {
+	.clks = imx8mp_isp_clks,
+	.clk_size = ARRAY_SIZE(imx8mp_isp_clks),
+	.isrs = imx8mp_isp_isrs,
+	.isr_size = ARRAY_SIZE(imx8mp_isp_isrs),
+	.isp_ver = IMX8MP_V10,
+};
+
 static const struct of_device_id rkisp1_of_match[] = {
 	{
 		.compatible = "rockchip,px30-cif-isp",
@@ -505,6 +523,10 @@ static const struct of_device_id rkisp1_of_match[] = {
 		.compatible = "rockchip,rk3399-cif-isp",
 		.data = &rk3399_isp_info,
 	},
+	{
+		.compatible = "fsl,imx8mp-isp",
+		.data = &imx8mp_isp_info,
+	},
 	{},
 };
 MODULE_DEVICE_TABLE(of, rkisp1_of_match);
diff --git a/include/uapi/linux/rkisp1-config.h b/include/uapi/linux/rkisp1-config.h
index 730673ecc63d..f602442c2018 100644
--- a/include/uapi/linux/rkisp1-config.h
+++ b/include/uapi/linux/rkisp1-config.h
@@ -179,12 +179,14 @@
  * @RKISP1_V11: declared in the original vendor code, but not used
  * @RKISP1_V12: used at least in rk3326 and px30
  * @RKISP1_V13: used at least in rk1808
+ * @IMX8MP_V10: used in at least imx8mp
  */
 enum rkisp1_cif_isp_version {
 	RKISP1_V10 = 10,
 	RKISP1_V11,
 	RKISP1_V12,
 	RKISP1_V13,
+	IMX8MP_V10,
 };
 
 enum rkisp1_cif_isp_histogram_mode {
-- 
2.35.1

[PATCH v3 05/14] media: rkisp1: Configure gasket on i.MX8MP

[Paul Elder]

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

The i.MX8MP has a gasket between the CSI-2 receiver and the ISP.
Configure and enable it when starting the ISP, and disable it when
stopping.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 .../platform/rockchip/rkisp1/rkisp1-common.h  |   5 +
 .../platform/rockchip/rkisp1/rkisp1-dev.c     |  16 +++
 .../platform/rockchip/rkisp1/rkisp1-isp.c     | 128 +++++++++++++++++-
 3 files changed, 147 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
index fc33c185b99f..1f9f373aa2a5 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
@@ -24,6 +24,7 @@
 #include "rkisp1-regs.h"
 
 struct dentry;
+struct regmap;
 
 /*
  * flags on the 'direction' field in struct rkisp1_mbus_info' that indicate
@@ -448,6 +449,8 @@ struct rkisp1_debug {
  * @dev:	   a pointer to the struct device
  * @clk_size:	   number of clocks
  * @clks:	   array of clocks
+ * @gasket:	   the gasket - i.MX8MP only
+ * @gasket_id:	   the gasket ID (0 or 1) - i.MX8MP only
  * @v4l2_dev:	   v4l2_device variable
  * @media_dev:	   media_device variable
  * @notifier:	   a notifier to register on the v4l2-async API to be notified on the sensor
@@ -468,6 +471,8 @@ struct rkisp1_device {
 	struct device *dev;
 	unsigned int clk_size;
 	struct clk_bulk_data clks[RKISP1_MAX_BUS_CLK];
+	struct regmap *gasket;
+	unsigned int gasket_id;
 	struct v4l2_device v4l2_dev;
 	struct media_device media_dev;
 	struct v4l2_async_notifier notifier;
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
index 69464ce91d59..245caf1725aa 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
@@ -10,6 +10,7 @@
 
 #include <linux/clk.h>
 #include <linux/interrupt.h>
+#include <linux/mfd/syscon.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_graph.h>
@@ -579,6 +580,21 @@ static int rkisp1_probe(struct platform_device *pdev)
 		return ret;
 	rkisp1->clk_size = info->clk_size;
 
+	if (info->isp_ver == IMX8MP_V10) {
+		unsigned int id;
+
+		rkisp1->gasket = syscon_regmap_lookup_by_phandle_args(dev->of_node,
+								      "fsl,blk-ctrl",
+								      1, &id);
+		if (IS_ERR(rkisp1->gasket)) {
+			ret = PTR_ERR(rkisp1->gasket);
+			dev_err(dev, "failed to get gasket: %d\n", ret);
+			return ret;
+		}
+
+		rkisp1->gasket_id = id;
+	}
+
 	pm_runtime_enable(&pdev->dev);
 
 	ret = pm_runtime_resume_and_get(&pdev->dev);
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
index 585cf3f53469..029191f4338d 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
@@ -10,6 +10,7 @@
 
 #include <linux/iopoll.h>
 #include <linux/pm_runtime.h>
+#include <linux/regmap.h>
 #include <linux/videodev2.h>
 #include <linux/vmalloc.h>
 
@@ -87,6 +88,115 @@ rkisp1_isp_get_pad_crop(struct rkisp1_isp *isp,
 		return v4l2_subdev_get_try_crop(&isp->sd, &state, pad);
 }
 
+/* -----------------------------------------------------------------------------
+ * Media block control (i.MX8MP only)
+ */
+
+#define ISP_DEWARP_CONTROL				0x0138
+
+#define ISP_DEWARP_CONTROL_MIPI_CSI2_HS_POLARITY	BIT(22)
+#define ISP_DEWARP_CONTROL_MIPI_CSI2_VS_SEL_RISING	(0 << 20)
+#define ISP_DEWARP_CONTROL_MIPI_CSI2_VS_SEL_NEGATIVE	(1 << 20)
+#define ISP_DEWARP_CONTROL_MIPI_CSI2_VS_SEL_POSITIVE	(2 << 20)
+#define ISP_DEWARP_CONTROL_MIPI_CSI2_VS_SEL_FALLING	(3 << 20)
+#define ISP_DEWARP_CONTROL_MIPI_CSI2_VS_SEL_MASK	GENMASK(21, 20)
+#define ISP_DEWARP_CONTROL_MIPI_ISP2_LEFT_JUST_MODE	BIT(19)
+#define ISP_DEWARP_CONTROL_MIPI_ISP2_DATA_TYPE(dt)	((dt) << 13)
+#define ISP_DEWARP_CONTROL_MIPI_ISP2_DATA_TYPE_MASK	GENMASK(18, 13)
+
+#define ISP_DEWARP_CONTROL_MIPI_CSI1_HS_POLARITY	BIT(12)
+#define ISP_DEWARP_CONTROL_MIPI_CSI1_VS_SEL_RISING	(0 << 10)
+#define ISP_DEWARP_CONTROL_MIPI_CSI1_VS_SEL_NEGATIVE	(1 << 10)
+#define ISP_DEWARP_CONTROL_MIPI_CSI1_VS_SEL_POSITIVE	(2 << 10)
+#define ISP_DEWARP_CONTROL_MIPI_CSI1_VS_SEL_FALLING	(3 << 10)
+#define ISP_DEWARP_CONTROL_MIPI_CSI1_VS_SEL_MASK	GENMASK(11, 10)
+#define ISP_DEWARP_CONTROL_MIPI_ISP1_LEFT_JUST_MODE	BIT(9)
+#define ISP_DEWARP_CONTROL_MIPI_ISP1_DATA_TYPE(dt)	((dt) << 3)
+#define ISP_DEWARP_CONTROL_MIPI_ISP1_DATA_TYPE_MASK	GENMASK(8, 3)
+
+#define ISP_DEWARP_CONTROL_GPR_ISP_1_DISABLE		BIT(1)
+#define ISP_DEWARP_CONTROL_GPR_ISP_0_DISABLE		BIT(0)
+
+static int rkisp1_gasket_enable(struct rkisp1_device *rkisp1,
+				struct media_pad *source)
+{
+	struct v4l2_subdev *source_sd;
+	struct v4l2_mbus_frame_desc fd;
+	unsigned int dt;
+	u32 mask;
+	u32 val;
+	int ret;
+
+	/*
+	 * Configure and enable the gasket with the CSI-2 data type. Set the
+	 * vsync polarity as active high, as that is what the ISP is configured
+	 * to expect in ISP_ACQ_PROP. Enable left justification, as the i.MX8MP
+	 * ISP has a 16-bit wide input and expects data to be left-aligned.
+	 */
+
+	source_sd = media_entity_to_v4l2_subdev(source->entity);
+	ret = v4l2_subdev_call(source_sd, pad, get_frame_desc,
+			       source->index, &fd);
+	if (ret) {
+		dev_err(rkisp1->dev,
+			"failed to get frame descriptor from '%s':%u: %d\n",
+			source_sd->name, 0, ret);
+		return ret;
+	}
+
+	if (fd.num_entries != 1) {
+		dev_err(rkisp1->dev, "invalid frame descriptor for '%s':%u\n",
+			source_sd->name, 0);
+		return -EINVAL;
+	}
+
+	dt = fd.entry[0].bus.csi2.dt;
+
+	if (rkisp1->gasket_id == 0) {
+		mask = ISP_DEWARP_CONTROL_MIPI_CSI1_HS_POLARITY
+		     | ISP_DEWARP_CONTROL_MIPI_CSI1_VS_SEL_MASK
+		     | ISP_DEWARP_CONTROL_MIPI_ISP1_LEFT_JUST_MODE
+		     | ISP_DEWARP_CONTROL_MIPI_ISP1_DATA_TYPE_MASK
+		     | ISP_DEWARP_CONTROL_GPR_ISP_0_DISABLE;
+		val = ISP_DEWARP_CONTROL_MIPI_CSI1_VS_SEL_POSITIVE
+		    | ISP_DEWARP_CONTROL_MIPI_ISP1_LEFT_JUST_MODE
+		    | ISP_DEWARP_CONTROL_MIPI_ISP1_DATA_TYPE(dt);
+	} else {
+		mask = ISP_DEWARP_CONTROL_MIPI_CSI2_HS_POLARITY
+		     | ISP_DEWARP_CONTROL_MIPI_CSI2_VS_SEL_MASK
+		     | ISP_DEWARP_CONTROL_MIPI_ISP2_LEFT_JUST_MODE
+		     | ISP_DEWARP_CONTROL_MIPI_ISP2_DATA_TYPE_MASK
+		     | ISP_DEWARP_CONTROL_GPR_ISP_1_DISABLE;
+		val = ISP_DEWARP_CONTROL_MIPI_CSI2_VS_SEL_POSITIVE
+		    | ISP_DEWARP_CONTROL_MIPI_ISP2_LEFT_JUST_MODE
+		    | ISP_DEWARP_CONTROL_MIPI_ISP2_DATA_TYPE(dt);
+	}
+
+	regmap_update_bits(rkisp1->gasket, ISP_DEWARP_CONTROL, mask, val);
+
+	return 0;
+}
+
+static void rkisp1_gasket_disable(struct rkisp1_device *rkisp1)
+{
+	u32 mask;
+	u32 val;
+
+	if (rkisp1->gasket_id == 1) {
+		mask = ISP_DEWARP_CONTROL_MIPI_ISP2_LEFT_JUST_MODE
+		     | ISP_DEWARP_CONTROL_MIPI_ISP2_DATA_TYPE_MASK
+		     | ISP_DEWARP_CONTROL_GPR_ISP_1_DISABLE;
+		val = ISP_DEWARP_CONTROL_GPR_ISP_1_DISABLE;
+	} else {
+		mask = ISP_DEWARP_CONTROL_MIPI_ISP1_LEFT_JUST_MODE
+		     | ISP_DEWARP_CONTROL_MIPI_ISP1_DATA_TYPE_MASK
+		     | ISP_DEWARP_CONTROL_GPR_ISP_0_DISABLE;
+		val = ISP_DEWARP_CONTROL_GPR_ISP_0_DISABLE;
+	}
+
+	regmap_update_bits(rkisp1->gasket, ISP_DEWARP_CONTROL, mask, val);
+}
+
 /* ----------------------------------------------------------------------------
  * Camera Interface registers configurations
  */
@@ -304,6 +414,9 @@ static void rkisp1_isp_stop(struct rkisp1_isp *isp)
 		     RKISP1_CIF_VI_IRCL_MIPI_SW_RST |
 		     RKISP1_CIF_VI_IRCL_ISP_SW_RST);
 	rkisp1_write(rkisp1, RKISP1_CIF_VI_IRCL, 0x0);
+
+	if (rkisp1->info->isp_ver == IMX8MP_V10)
+		rkisp1_gasket_disable(rkisp1);
 }
 
 static void rkisp1_config_clk(struct rkisp1_isp *isp)
@@ -328,13 +441,20 @@ static void rkisp1_config_clk(struct rkisp1_isp *isp)
 	}
 }
 
-static void rkisp1_isp_start(struct rkisp1_isp *isp)
+static int rkisp1_isp_start(struct rkisp1_isp *isp, struct media_pad *source)
 {
 	struct rkisp1_device *rkisp1 = isp->rkisp1;
 	u32 val;
+	int ret;
 
 	rkisp1_config_clk(isp);
 
+	if (rkisp1->info->isp_ver == IMX8MP_V10) {
+		ret = rkisp1_gasket_enable(rkisp1, source);
+		if (ret)
+			return ret;
+	}
+
 	/* Activate ISP */
 	val = rkisp1_read(rkisp1, RKISP1_CIF_ISP_CTRL);
 	val |= RKISP1_CIF_ISP_CTRL_ISP_CFG_UPD |
@@ -344,6 +464,8 @@ static void rkisp1_isp_start(struct rkisp1_isp *isp)
 
 	if (isp->src_fmt->pixel_enc != V4L2_PIXEL_ENC_BAYER)
 		rkisp1_params_post_configure(&rkisp1->params);
+
+	return 0;
 }
 
 /* ----------------------------------------------------------------------------
@@ -882,7 +1004,9 @@ static int rkisp1_isp_s_stream(struct v4l2_subdev *sd, int enable)
 	if (ret)
 		goto mutex_unlock;
 
-	rkisp1_isp_start(isp);
+	ret = rkisp1_isp_start(isp, source_pad);
+	if (ret)
+		goto mutex_unlock;
 
 	ret = v4l2_subdev_call(rkisp1->source, video, s_stream, true);
 	if (ret) {
-- 
2.35.1

[PATCH v3 06/14] media: rkisp1: Add and set registers for crop for i.MX8MP

[Paul Elder]

The ISP version in the i.MX8MP has a separate set of registers for crop
that is currently not handled. Add a feature flag to determine which
type of crop the ISP supports and set the crop registers based on that.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
---
Changes since v2:

- Document the RKISP1_FEATURE_DUAL_CROP and RKISP1_FEATURE_RSZ_CROP bits
- Use the rkisp1_has_feature() macro
---
 .../platform/rockchip/rkisp1/rkisp1-common.h  |  4 ++++
 .../platform/rockchip/rkisp1/rkisp1-debug.c   | 14 +++++++++++++-
 .../platform/rockchip/rkisp1/rkisp1-dev.c     |  7 +++++--
 .../platform/rockchip/rkisp1/rkisp1-regs.h    |  9 +++++++++
 .../platform/rockchip/rkisp1/rkisp1-resizer.c | 19 +++++++++++++++++--
 5 files changed, 48 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
index 1f9f373aa2a5..d8851dca026f 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
@@ -103,6 +103,8 @@ enum rkisp1_isp_pad {
  * enum rkisp1_feature - ISP features
  *
  * @RKISP1_FEATURE_MIPI_CSI2: The ISP has an internal MIPI CSI-2 receiver
+ * @RKISP1_FEATURE_DUAL_CROP: The ISP has the dual crop block at the resizer input
+ * @RKISP1_FEATURE_RSZ_CROP: The ISP supports cropping in the resizer
  *
  * The ISP features are stored in a bitmask in &rkisp1_info.features and allow
  * the driver to implement support for features present in some ISP versions
@@ -110,6 +112,8 @@ enum rkisp1_isp_pad {
  */
 enum rkisp1_feature {
 	RKISP1_FEATURE_MIPI_CSI2 = BIT(0),
+	RKISP1_FEATURE_DUAL_CROP = BIT(1),
+	RKISP1_FEATURE_RSZ_CROP = BIT(2),
 };
 
 #define rkisp1_has_feature(rkisp1, feature) \
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-debug.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-debug.c
index 71df3dc95e6f..d7208dbafd11 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-debug.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-debug.c
@@ -115,9 +115,21 @@ static int rkisp1_debug_dump_rsz_regs_show(struct seq_file *m, void *p)
 		RKISP1_DEBUG_SHD_REG(RSZ_PHASE_VC),
 		{ /* Sentinel */ },
 	};
+	static const struct rkisp1_debug_register crop_registers[] = {
+		RKISP1_DEBUG_SHD_REG(RSZ_CROP_X_DIR),
+		RKISP1_DEBUG_SHD_REG(RSZ_CROP_Y_DIR),
+		RKISP1_DEBUG_REG(RSZ_FRAME_RATE),
+		RKISP1_DEBUG_REG(RSZ_FORMAT_CONV_CTRL),
+		{ /* Sentinel */ },
+	};
 	struct rkisp1_resizer *rsz = m->private;
 
-	return rkisp1_debug_dump_regs(rsz->rkisp1, m, rsz->regs_base, registers);
+	rkisp1_debug_dump_regs(rsz->rkisp1, m, rsz->regs_base, registers);
+	if (rkisp1_has_feature(rsz->rkisp1, RSZ_CROP))
+		rkisp1_debug_dump_regs(rsz->rkisp1, m, rsz->regs_base,
+				       crop_registers);
+
+	return 0;
 }
 DEFINE_SHOW_ATTRIBUTE(rkisp1_debug_dump_rsz_regs);
 
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
index 245caf1725aa..4fca4db091c8 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
@@ -475,7 +475,8 @@ static const struct rkisp1_info px30_isp_info = {
 	.isrs = px30_isp_isrs,
 	.isr_size = ARRAY_SIZE(px30_isp_isrs),
 	.isp_ver = RKISP1_V12,
-	.features = RKISP1_FEATURE_MIPI_CSI2,
+	.features = RKISP1_FEATURE_MIPI_CSI2
+		  | RKISP1_FEATURE_DUAL_CROP,
 };
 
 static const char * const rk3399_isp_clks[] = {
@@ -494,7 +495,8 @@ static const struct rkisp1_info rk3399_isp_info = {
 	.isrs = rk3399_isp_isrs,
 	.isr_size = ARRAY_SIZE(rk3399_isp_isrs),
 	.isp_ver = RKISP1_V10,
-	.features = RKISP1_FEATURE_MIPI_CSI2,
+	.features = RKISP1_FEATURE_MIPI_CSI2
+		  | RKISP1_FEATURE_DUAL_CROP,
 };
 
 static const char * const imx8mp_isp_clks[] = {
@@ -513,6 +515,7 @@ static const struct rkisp1_info imx8mp_isp_info = {
 	.isrs = imx8mp_isp_isrs,
 	.isr_size = ARRAY_SIZE(imx8mp_isp_isrs),
 	.isp_ver = IMX8MP_V10,
+	.features = RKISP1_FEATURE_RSZ_CROP,
 };
 
 static const struct of_device_id rkisp1_of_match[] = {
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h
index 421cc73355db..cd6ce66945c4 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h
@@ -168,6 +168,9 @@
 #define RKISP1_CIF_RSZ_CTRL_CFG_UPD_AUTO		BIT(9)
 #define RKISP1_CIF_RSZ_SCALER_FACTOR			BIT(16)
 
+/* RSZ_CROP_[XY]_DIR */
+#define RKISP1_CIF_RSZ_CROP_XY_DIR(start, end)		((end) << 16 | (start) << 0)
+
 /* MI_IMSC - MI_MIS - MI_RIS - MI_ICR - MI_ISR */
 #define RKISP1_CIF_MI_FRAME(stream)			BIT((stream)->id)
 #define RKISP1_CIF_MI_MBLK_LINE				BIT(2)
@@ -925,6 +928,12 @@
 #define RKISP1_CIF_RSZ_PHASE_HC_SHD		0x004C
 #define RKISP1_CIF_RSZ_PHASE_VY_SHD		0x0050
 #define RKISP1_CIF_RSZ_PHASE_VC_SHD		0x0054
+#define RKISP1_CIF_RSZ_CROP_X_DIR		0x0058
+#define RKISP1_CIF_RSZ_CROP_Y_DIR		0x005C
+#define RKISP1_CIF_RSZ_CROP_X_DIR_SHD		0x0060
+#define RKISP1_CIF_RSZ_CROP_Y_DIR_SHD		0x0064
+#define RKISP1_CIF_RSZ_FRAME_RATE		0x0068
+#define RKISP1_CIF_RSZ_FORMAT_CONV_CTRL		0x006C
 
 #define RKISP1_CIF_MI_BASE			0x00001400
 #define RKISP1_CIF_MI_CTRL			(RKISP1_CIF_MI_BASE + 0x00000000)
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c
index f76afd8112b2..cefc3cfb733c 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c
@@ -244,6 +244,7 @@ static void rkisp1_rsz_config_regs(struct rkisp1_resizer *rsz,
 {
 	u32 ratio, rsz_ctrl = 0;
 	unsigned int i;
+	u32 val;
 
 	/* No phase offset */
 	rkisp1_rsz_write(rsz, RKISP1_CIF_RSZ_PHASE_HY, 0);
@@ -292,6 +293,18 @@ static void rkisp1_rsz_config_regs(struct rkisp1_resizer *rsz,
 
 	rkisp1_rsz_write(rsz, RKISP1_CIF_RSZ_CTRL, rsz_ctrl);
 
+	if (rkisp1_has_feature(rsz->rkisp1, RSZ_CROP)) {
+		val = RKISP1_CIF_RSZ_CROP_XY_DIR(src_y->left, src_y->left + src_y->width - 1);
+		rkisp1_rsz_write(rsz, RKISP1_CIF_RSZ_CROP_X_DIR, val);
+		val = RKISP1_CIF_RSZ_CROP_XY_DIR(src_y->top, src_y->top + src_y->height - 1);
+		rkisp1_rsz_write(rsz, RKISP1_CIF_RSZ_CROP_Y_DIR, val);
+
+		val = RKISP1_CIF_RSZ_FORMAT_CONV_CTRL_RSZ_INPUT_FORMAT_YCBCR_422
+		    | RKISP1_CIF_RSZ_FORMAT_CONV_CTRL_RSZ_OUTPUT_FORMAT_YCBCR_420
+		    | RKISP1_CIF_RSZ_FORMAT_CONV_CTRL_RSZ_PACK_FORMAT_SEMI_PLANAR;
+		rkisp1_rsz_write(rsz, RKISP1_CIF_RSZ_FORMAT_CONV_CTRL, val);
+	}
+
 	rkisp1_rsz_update_shadow(rsz, when);
 }
 
@@ -695,7 +708,8 @@ static int rkisp1_rsz_s_stream(struct v4l2_subdev *sd, int enable)
 	enum rkisp1_shadow_regs_when when = RKISP1_SHADOW_REGS_SYNC;
 
 	if (!enable) {
-		rkisp1_dcrop_disable(rsz, RKISP1_SHADOW_REGS_ASYNC);
+		if (rkisp1_has_feature(rkisp1, DUAL_CROP))
+			rkisp1_dcrop_disable(rsz, RKISP1_SHADOW_REGS_ASYNC);
 		rkisp1_rsz_disable(rsz, RKISP1_SHADOW_REGS_ASYNC);
 		return 0;
 	}
@@ -705,7 +719,8 @@ static int rkisp1_rsz_s_stream(struct v4l2_subdev *sd, int enable)
 
 	mutex_lock(&rsz->ops_lock);
 	rkisp1_rsz_config(rsz, when);
-	rkisp1_dcrop_config(rsz);
+	if (rkisp1_has_feature(rkisp1, DUAL_CROP))
+		rkisp1_dcrop_config(rsz);
 
 	mutex_unlock(&rsz->ops_lock);
 	return 0;
-- 
2.35.1

[PATCH v3 07/14] media: rkisp1: Add and set registers for output size config on i.MX8MP

[Paul Elder]

The ISP version in the i.MX8MP has a set of registers currently not
handled by the driver for output size configuration. Add a feature flag
to determine if the ISP requires this, and set the registers based on
that.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
Changes since v2:

- Document the RKISP1_FEATURE_MAIN_STRIDE bit
- Use the rkisp1_has_feature() macro
---
 drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c | 8 ++++++++
 drivers/media/platform/rockchip/rkisp1/rkisp1-common.h  | 2 ++
 drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c     | 3 ++-
 drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h    | 9 +++++++++
 4 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
index d4540684ea9a..5c6299ab0f78 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
@@ -420,6 +420,14 @@ static void rkisp1_mp_config(struct rkisp1_capture *cap)
 	rkisp1_write(rkisp1, cap->config->mi.cr_size_init,
 		     rkisp1_pixfmt_comp_size(pixm, RKISP1_PLANE_CR));
 
+	if (rkisp1_has_feature(rkisp1, MAIN_STRIDE)) {
+		rkisp1_write(rkisp1, RKISP1_CIF_MI_MP_Y_LLENGTH, pixm->width);
+		rkisp1_write(rkisp1, RKISP1_CIF_MI_MP_Y_PIC_WIDTH, pixm->width);
+		rkisp1_write(rkisp1, RKISP1_CIF_MI_MP_Y_PIC_HEIGHT, pixm->height);
+		rkisp1_write(rkisp1, RKISP1_CIF_MI_MP_Y_PIC_SIZE,
+			     pixm->width * pixm->height);
+	}
+
 	rkisp1_irq_frame_end_enable(cap);
 
 	/* set uv swapping for semiplanar formats */
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
index d8851dca026f..047c670b2310 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
@@ -105,6 +105,7 @@ enum rkisp1_isp_pad {
  * @RKISP1_FEATURE_MIPI_CSI2: The ISP has an internal MIPI CSI-2 receiver
  * @RKISP1_FEATURE_DUAL_CROP: The ISP has the dual crop block at the resizer input
  * @RKISP1_FEATURE_RSZ_CROP: The ISP supports cropping in the resizer
+ * @RKISP1_FEATURE_MAIN_STRIDE: The ISP supports configurable stride on the main path
  *
  * The ISP features are stored in a bitmask in &rkisp1_info.features and allow
  * the driver to implement support for features present in some ISP versions
@@ -114,6 +115,7 @@ enum rkisp1_feature {
 	RKISP1_FEATURE_MIPI_CSI2 = BIT(0),
 	RKISP1_FEATURE_DUAL_CROP = BIT(1),
 	RKISP1_FEATURE_RSZ_CROP = BIT(2),
+	RKISP1_FEATURE_MAIN_STRIDE = BIT(3),
 };
 
 #define rkisp1_has_feature(rkisp1, feature) \
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
index 4fca4db091c8..1a3ea5a4af05 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
@@ -515,7 +515,8 @@ static const struct rkisp1_info imx8mp_isp_info = {
 	.isrs = imx8mp_isp_isrs,
 	.isr_size = ARRAY_SIZE(imx8mp_isp_isrs),
 	.isp_ver = IMX8MP_V10,
-	.features = RKISP1_FEATURE_RSZ_CROP,
+	.features = RKISP1_FEATURE_RSZ_CROP
+		  | RKISP1_FEATURE_MAIN_STRIDE,
 };
 
 static const struct of_device_id rkisp1_of_match[] = {
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h
index cd6ce66945c4..ed34c752be99 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h
@@ -1012,6 +1012,15 @@
 #define RKISP1_CIF_MI_SP_CB_BASE_AD_INIT2	(RKISP1_CIF_MI_BASE + 0x00000140)
 #define RKISP1_CIF_MI_SP_CR_BASE_AD_INIT2	(RKISP1_CIF_MI_BASE + 0x00000144)
 #define RKISP1_CIF_MI_XTD_FORMAT_CTRL		(RKISP1_CIF_MI_BASE + 0x00000148)
+#define RKISP1_CIF_MI_MP_HANDSHAKE_0		(RKISP1_CIF_MI_BASE + 0x0000014C)
+#define RKISP1_CIF_MI_MP_Y_LLENGTH		(RKISP1_CIF_MI_BASE + 0x00000150)
+#define RKISP1_CIF_MI_MP_Y_SLICE_OFFSET		(RKISP1_CIF_MI_BASE + 0x00000154)
+#define RKISP1_CIF_MI_MP_C_SLICE_OFFSET		(RKISP1_CIF_MI_BASE + 0x00000158)
+#define RKISP1_CIF_MI_OUTPUT_ALIGN_FORMAT	(RKISP1_CIF_MI_BASE + 0x0000015C)
+#define RKISP1_CIF_MI_MP_OUTPUT_FIFO_SIZE	(RKISP1_CIF_MI_BASE + 0x00000160)
+#define RKISP1_CIF_MI_MP_Y_PIC_WIDTH		(RKISP1_CIF_MI_BASE + 0x00000164)
+#define RKISP1_CIF_MI_MP_Y_PIC_HEIGHT		(RKISP1_CIF_MI_BASE + 0x00000168)
+#define RKISP1_CIF_MI_MP_Y_PIC_SIZE		(RKISP1_CIF_MI_BASE + 0x0000016C)
 
 #define RKISP1_CIF_SMIA_BASE			0x00001A00
 #define RKISP1_CIF_SMIA_CTRL			(RKISP1_CIF_SMIA_BASE + 0x00000000)
-- 
2.35.1

[PATCH v3 08/14] media: rkisp1: Add i.MX8MP-specific registers for MI and resizer

[Paul Elder]

Add register definitions for resizer format conversion control and for
the memory interface output that are specific to the ISP version in the
i.MX8MP.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
---
Changes since v2:

- Add missing newlines
---
 .../platform/rockchip/rkisp1/rkisp1-regs.h    | 35 +++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h
index ed34c752be99..6597c563f892 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h
@@ -171,6 +171,23 @@
 /* RSZ_CROP_[XY]_DIR */
 #define RKISP1_CIF_RSZ_CROP_XY_DIR(start, end)		((end) << 16 | (start) << 0)
 
+/* RSZ_FORMAT_CONV_CTRL */
+#define RKISP1_CIF_RSZ_FORMAT_CONV_CTRL_RSZ_INPUT_FORMAT_YCBCR_400	(0 << 0)
+#define RKISP1_CIF_RSZ_FORMAT_CONV_CTRL_RSZ_INPUT_FORMAT_YCBCR_420	(1 << 0)
+#define RKISP1_CIF_RSZ_FORMAT_CONV_CTRL_RSZ_INPUT_FORMAT_YCBCR_422	(2 << 0)
+#define RKISP1_CIF_RSZ_FORMAT_CONV_CTRL_RSZ_INPUT_FORMAT_YCBCR_444	(3 << 0)
+#define RKISP1_CIF_RSZ_FORMAT_CONV_CTRL_RSZ_OUTPUT_FORMAT_YCBCR_400	(0 << 2)
+#define RKISP1_CIF_RSZ_FORMAT_CONV_CTRL_RSZ_OUTPUT_FORMAT_YCBCR_420	(1 << 2)
+#define RKISP1_CIF_RSZ_FORMAT_CONV_CTRL_RSZ_OUTPUT_FORMAT_YCBCR_422	(2 << 2)
+#define RKISP1_CIF_RSZ_FORMAT_CONV_CTRL_RSZ_OUTPUT_FORMAT_YCBCR_444	(3 << 2)
+#define RKISP1_CIF_RSZ_FORMAT_CONV_CTRL_CFG_Y_FULL			BIT(5)
+#define RKISP1_CIF_RSZ_FORMAT_CONV_CTRL_CFG_CBCR_FULL			BIT(6)
+#define RKISP1_CIF_RSZ_FORMAT_CONV_CTRL_CFG_422NOCOSITED		BIT(7)
+#define RKISP1_CIF_RSZ_FORMAT_CONV_CTRL_DATA_WIDTH_10_BIT_ENABLE	BIT(8)
+#define RKISP1_CIF_RSZ_FORMAT_CONV_CTRL_DATA_WIDTH_10_BIT_METHOD	BIT(9)
+#define RKISP1_CIF_RSZ_FORMAT_CONV_CTRL_RSZ_PACK_FORMAT_PLANAR		(0 << 10)
+#define RKISP1_CIF_RSZ_FORMAT_CONV_CTRL_RSZ_PACK_FORMAT_SEMI_PLANAR	(1 << 10)
+
 /* MI_IMSC - MI_MIS - MI_RIS - MI_ICR - MI_ISR */
 #define RKISP1_CIF_MI_FRAME(stream)			BIT((stream)->id)
 #define RKISP1_CIF_MI_MBLK_LINE				BIT(2)
@@ -213,6 +230,24 @@
 #define RKISP1_CIF_MI_XTD_FMT_CTRL_SP_CB_CR_SWAP	BIT(1)
 #define RKISP1_CIF_MI_XTD_FMT_CTRL_DMA_CB_CR_SWAP	BIT(2)
 
+/* MI_OUTPUT_ALIGN_FORMAT */
+#define RKISP1_CIF_OUTPUT_ALIGN_FORMAT_MP_LSB_ALIGNMENT			BIT(0)
+#define RKISP1_CIF_OUTPUT_ALIGN_FORMAT_MP_BYTE_SWAP_BYTES		BIT(1)
+#define RKISP1_CIF_OUTPUT_ALIGN_FORMAT_MP_BYTE_SWAP_WORDS		BIT(2)
+#define RKISP1_CIF_OUTPUT_ALIGN_FORMAT_MP_BYTE_SWAP_DWORDS		BIT(3)
+#define RKISP1_CIF_OUTPUT_ALIGN_FORMAT_SP_BYTE_SWAP_BYTES		BIT(4)
+#define RKISP1_CIF_OUTPUT_ALIGN_FORMAT_SP_BYTE_SWAP_WORDS		BIT(5)
+#define RKISP1_CIF_OUTPUT_ALIGN_FORMAT_SP_BYTE_SWAP_DWORDS		BIT(6)
+#define RKISP1_CIF_OUTPUT_ALIGN_FORMAT_DMA_BYTE_SWAP_BYTES		BIT(7)
+#define RKISP1_CIF_OUTPUT_ALIGN_FORMAT_DMA_BYTE_SWAP_WORDS		BIT(8)
+#define RKISP1_CIF_OUTPUT_ALIGN_FORMAT_DMA_BYTE_SWAP_DWORDS		BIT(9)
+
+/* MI_MP_OUTPUT_FIFO_SIZE */
+#define RKISP1_CIF_MI_MP_OUTPUT_FIFO_SIZE_OUTPUT_FIFO_DEPTH_FULL	(0 << 0)
+#define RKISP1_CIF_MI_MP_OUTPUT_FIFO_SIZE_OUTPUT_FIFO_DEPTH_HALF	(1 << 0)
+#define RKISP1_CIF_MI_MP_OUTPUT_FIFO_SIZE_OUTPUT_FIFO_DEPTH_QUARTER	(2 << 0)
+#define RKISP1_CIF_MI_MP_OUTPUT_FIFO_SIZE_OUTPUT_FIFO_DEPTH_EIGHT	(3 << 0)
+
 /* VI_CCL */
 #define RKISP1_CIF_CCL_CIF_CLK_DIS			BIT(2)
 /* VI_ISP_CLK_CTRL */
-- 
2.35.1

[PATCH v3 09/14] media: rkisp1: Shift DMA buffer addresses on i.MX8MP

[Paul Elder]

On the ISP that is integrated in the i.MX8MP, DMA addresses have been
extended to 34 bits, with the 32 MSBs stored in the DMA address
registers and the 2 LSBs set to 0. Shift the buffer addresses right by 2
on that platform.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
Changes since v2:

- Document the RKISP1_FEATURE_DMA_34BIT bit
- Use the rkisp1_has_feature() macro
---
 .../platform/rockchip/rkisp1/rkisp1-capture.c  | 18 ++++++++++--------
 .../platform/rockchip/rkisp1/rkisp1-common.h   |  2 ++
 .../platform/rockchip/rkisp1/rkisp1-dev.c      |  3 ++-
 3 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
index 5c6299ab0f78..55e863b762e6 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
@@ -624,6 +624,8 @@ static void rkisp1_dummy_buf_destroy(struct rkisp1_capture *cap)
 
 static void rkisp1_set_next_buf(struct rkisp1_capture *cap)
 {
+	u8 shift = rkisp1_has_feature(cap->rkisp1, DMA_34BIT) ? 2 : 0;
+
 	cap->buf.curr = cap->buf.next;
 	cap->buf.next = NULL;
 
@@ -636,7 +638,7 @@ static void rkisp1_set_next_buf(struct rkisp1_capture *cap)
 		buff_addr = cap->buf.next->buff_addr;
 
 		rkisp1_write(cap->rkisp1, cap->config->mi.y_base_ad_init,
-			     buff_addr[RKISP1_PLANE_Y]);
+			     buff_addr[RKISP1_PLANE_Y] >> shift);
 		/*
 		 * In order to support grey format we capture
 		 * YUV422 planar format from the camera and
@@ -645,17 +647,17 @@ static void rkisp1_set_next_buf(struct rkisp1_capture *cap)
 		if (cap->pix.cfg->fourcc == V4L2_PIX_FMT_GREY) {
 			rkisp1_write(cap->rkisp1,
 				     cap->config->mi.cb_base_ad_init,
-				     cap->buf.dummy.dma_addr);
+				     cap->buf.dummy.dma_addr >> shift);
 			rkisp1_write(cap->rkisp1,
 				     cap->config->mi.cr_base_ad_init,
-				     cap->buf.dummy.dma_addr);
+				     cap->buf.dummy.dma_addr >> shift);
 		} else {
 			rkisp1_write(cap->rkisp1,
 				     cap->config->mi.cb_base_ad_init,
-				     buff_addr[RKISP1_PLANE_CB]);
+				     buff_addr[RKISP1_PLANE_CB] >> shift);
 			rkisp1_write(cap->rkisp1,
 				     cap->config->mi.cr_base_ad_init,
-				     buff_addr[RKISP1_PLANE_CR]);
+				     buff_addr[RKISP1_PLANE_CR] >> shift);
 		}
 	} else {
 		/*
@@ -663,11 +665,11 @@ static void rkisp1_set_next_buf(struct rkisp1_capture *cap)
 		 * throw data if there is no available buffer.
 		 */
 		rkisp1_write(cap->rkisp1, cap->config->mi.y_base_ad_init,
-			     cap->buf.dummy.dma_addr);
+			     cap->buf.dummy.dma_addr >> shift);
 		rkisp1_write(cap->rkisp1, cap->config->mi.cb_base_ad_init,
-			     cap->buf.dummy.dma_addr);
+			     cap->buf.dummy.dma_addr >> shift);
 		rkisp1_write(cap->rkisp1, cap->config->mi.cr_base_ad_init,
-			     cap->buf.dummy.dma_addr);
+			     cap->buf.dummy.dma_addr >> shift);
 	}
 
 	/* Set plane offsets */
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
index 047c670b2310..fff5f5264386 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
@@ -106,6 +106,7 @@ enum rkisp1_isp_pad {
  * @RKISP1_FEATURE_DUAL_CROP: The ISP has the dual crop block at the resizer input
  * @RKISP1_FEATURE_RSZ_CROP: The ISP supports cropping in the resizer
  * @RKISP1_FEATURE_MAIN_STRIDE: The ISP supports configurable stride on the main path
+ * @RKISP1_FEATURE_DMA_34BIT: The ISP uses 34-bit DMA addresses
  *
  * The ISP features are stored in a bitmask in &rkisp1_info.features and allow
  * the driver to implement support for features present in some ISP versions
@@ -116,6 +117,7 @@ enum rkisp1_feature {
 	RKISP1_FEATURE_DUAL_CROP = BIT(1),
 	RKISP1_FEATURE_RSZ_CROP = BIT(2),
 	RKISP1_FEATURE_MAIN_STRIDE = BIT(3),
+	RKISP1_FEATURE_DMA_34BIT = BIT(4),
 };
 
 #define rkisp1_has_feature(rkisp1, feature) \
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
index 1a3ea5a4af05..2b13962c5c32 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
@@ -516,7 +516,8 @@ static const struct rkisp1_info imx8mp_isp_info = {
 	.isr_size = ARRAY_SIZE(imx8mp_isp_isrs),
 	.isp_ver = IMX8MP_V10,
 	.features = RKISP1_FEATURE_RSZ_CROP
-		  | RKISP1_FEATURE_MAIN_STRIDE,
+		  | RKISP1_FEATURE_MAIN_STRIDE
+		  | RKISP1_FEATURE_DMA_34BIT,
 };
 
 static const struct of_device_id rkisp1_of_match[] = {
-- 
2.35.1

[PATCH v3 10/14] media: rkisp1: Add register definitions for the test pattern generator

[Paul Elder]

Add register definitions and value macros for the test pattern generator
block in the ISP.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Dafna Hirschfeld <dafna@fastmail.com>
---
 .../platform/rockchip/rkisp1/rkisp1-regs.h    | 32 +++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h
index 6597c563f892..3dd1bfec288f 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h
@@ -718,6 +718,27 @@
 #define RKISP1_CIF_ISP_DPF_SPATIAL_COEFF_MAX		0x1F
 #define RKISP1_CIF_ISP_DPF_NLL_COEFF_N_MAX		0x3FF
 
+/* TPG */
+#define RKISP1_CIF_ISP_TPG_CTRL_ENA			BIT(0)
+#define RKISP1_CIF_ISP_TPG_CTRL_IMG_3X3_COLOR_BLOCK	(0 << 1)
+#define RKISP1_CIF_ISP_TPG_CTRL_IMG_COLOR_BAR		(1 << 1)
+#define RKISP1_CIF_ISP_TPG_CTRL_IMG_GRAY_BAR		(2 << 1)
+#define RKISP1_CIF_ISP_TPG_CTRL_IMG_HIGHLIGHT_GRID	(3 << 1)
+#define RKISP1_CIF_ISP_TPG_CTRL_IMG_RAND		(4 << 1)
+#define RKISP1_CIF_ISP_TPG_CTRL_CFA_RGGB		(0 << 4)
+#define RKISP1_CIF_ISP_TPG_CTRL_CFA_GRBG		(1 << 4)
+#define RKISP1_CIF_ISP_TPG_CTRL_CFA_GBRB		(2 << 4)
+#define RKISP1_CIF_ISP_TPG_CTRL_CFA_BGGR		(3 << 4)
+#define RKISP1_CIF_ISP_TPG_CTRL_DEPTH_8			(0 << 6)
+#define RKISP1_CIF_ISP_TPG_CTRL_DEPTH_10		(1 << 6)
+#define RKISP1_CIF_ISP_TPG_CTRL_DEPTH_12		(2 << 6)
+#define RKISP1_CIF_ISP_TPG_CTRL_DEF_SYNC		BIT(8)
+#define RKISP1_CIF_ISP_TPG_CTRL_MAX_SYNC		BIT(9)
+#define RKISP1_CIF_ISP_TPG_CTRL_SOL_1080P		(0 << 10)
+#define RKISP1_CIF_ISP_TPG_CTRL_SOL_720P		(1 << 10)
+#define RKISP1_CIF_ISP_TPG_CTRL_SOL_4K			(2 << 10)
+#define RKISP1_CIF_ISP_TPG_CTRL_SOL_USER_DEFINED	(3 << 10)
+
 /* =================================================================== */
 /*                            CIF Registers                            */
 /* =================================================================== */
@@ -913,6 +934,17 @@
 #define RKISP1_CIF_ISP_SH_DELAY			(RKISP1_CIF_ISP_SH_BASE + 0x00000008)
 #define RKISP1_CIF_ISP_SH_TIME			(RKISP1_CIF_ISP_SH_BASE + 0x0000000C)
 
+#define RKISP1_CIF_ISP_TPG_BASE			0x00000700
+#define RKISP1_CIF_ISP_TPG_CTRL			(RKISP1_CIF_ISP_TPG_BASE + 0x00000000)
+#define RKISP1_CIF_ISP_TPG_TOTAL_IN		(RKISP1_CIF_ISP_TPG_BASE + 0x00000004)
+#define RKISP1_CIF_ISP_TPG_ACT_IN		(RKISP1_CIF_ISP_TPG_BASE + 0x00000008)
+#define RKISP1_CIF_ISP_TPG_FP_IN		(RKISP1_CIF_ISP_TPG_BASE + 0x0000000C)
+#define RKISP1_CIF_ISP_TPG_BP_IN		(RKISP1_CIF_ISP_TPG_BASE + 0x00000010)
+#define RKISP1_CIF_ISP_TPG_W_IN			(RKISP1_CIF_ISP_TPG_BASE + 0x00000014)
+#define RKISP1_CIF_ISP_TPG_GAP_IN		(RKISP1_CIF_ISP_TPG_BASE + 0x00000018)
+#define RKISP1_CIF_ISP_TPG_GAP_STD_IN		(RKISP1_CIF_ISP_TPG_BASE + 0x0000001C)
+#define RKISP1_CIF_ISP_TPG_RANDOM_SEED_IN	(RKISP1_CIF_ISP_TPG_BASE + 0x00000020)
+
 #define RKISP1_CIF_C_PROC_BASE			0x00000800
 #define RKISP1_CIF_C_PROC_CTRL			(RKISP1_CIF_C_PROC_BASE + 0x00000000)
 #define RKISP1_CIF_C_PROC_CONTRAST		(RKISP1_CIF_C_PROC_BASE + 0x00000004)
-- 
2.35.1

[PATCH v3 11/14] media: rkisp1: Fix RSZ_CTRL bits for i.MX8MP

[Paul Elder]

The ISP that is integrated in the i.MX8MP supports cropping, which
results in a different RSZ_CTRL register layout compared to the RK3399.
Add new definitions for these bits, and use them when the
RKISP1_FEATURE_RSZ_CROP feature is set.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
---
Changes since v2:

- Condition on RKISP1_FEATURE_RSZ_CROP feature
- Rename bits
- Use the rkisp1_has_feature() macro
---
 drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h   |  5 +++++
 .../media/platform/rockchip/rkisp1/rkisp1-resizer.c    | 10 ++++++++--
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h
index 3dd1bfec288f..39b2ac58196e 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h
@@ -168,6 +168,11 @@
 #define RKISP1_CIF_RSZ_CTRL_CFG_UPD_AUTO		BIT(9)
 #define RKISP1_CIF_RSZ_SCALER_FACTOR			BIT(16)
 
+/* For resizer instances that support cropping */
+#define RKISP1_CIF_RSZ_CTRL_CROP_ENABLE			BIT(8)
+#define RKISP1_CIF_RSZ_CTRL_CROP_CFG_UPD		BIT(9)
+#define RKISP1_CIF_RSZ_CTRL_CROP_CFG_UPD_AUTO		BIT(10)
+
 /* RSZ_CROP_[XY]_DIR */
 #define RKISP1_CIF_RSZ_CROP_XY_DIR(start, end)		((end) << 16 | (start) << 0)
 
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c
index cefc3cfb733c..7ff7b608fc3f 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c
@@ -209,9 +209,15 @@ static void rkisp1_rsz_update_shadow(struct rkisp1_resizer *rsz,
 	u32 ctrl_cfg = rkisp1_rsz_read(rsz, RKISP1_CIF_RSZ_CTRL);
 
 	if (when == RKISP1_SHADOW_REGS_ASYNC)
-		ctrl_cfg |= RKISP1_CIF_RSZ_CTRL_CFG_UPD_AUTO;
+		if (rkisp1_has_feature(rsz->rkisp1, RSZ_CROP))
+			ctrl_cfg |= RKISP1_CIF_RSZ_CTRL_CROP_CFG_UPD_AUTO;
+		else
+			ctrl_cfg |= RKISP1_CIF_RSZ_CTRL_CFG_UPD_AUTO;
 	else
-		ctrl_cfg |= RKISP1_CIF_RSZ_CTRL_CFG_UPD;
+		if (rkisp1_has_feature(rsz->rkisp1, RSZ_CROP))
+			ctrl_cfg |= RKISP1_CIF_RSZ_CTRL_CROP_CFG_UPD;
+		else
+			ctrl_cfg |= RKISP1_CIF_RSZ_CTRL_CFG_UPD;
 
 	rkisp1_rsz_write(rsz, RKISP1_CIF_RSZ_CTRL, ctrl_cfg);
 }
-- 
2.35.1

[PATCH v3 12/14] media: rkisp1: Support devices without self path

[Paul Elder]

Some hardware supported by the rkisp1 driver, such as the ISP in the
i.MX8MP, don't have a self path. Add a feature flag for this, and
massage the rest of the driver to support the lack of a self path.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
Changes since v2:

- Simplify rkisp1_path_count()
- Use the rkisp1_has_feature() macro
---
 .../platform/rockchip/rkisp1/rkisp1-capture.c      |  9 ++++++---
 .../media/platform/rockchip/rkisp1/rkisp1-common.h | 14 ++++++++++++++
 .../media/platform/rockchip/rkisp1/rkisp1-dev.c    |  9 ++++++---
 .../platform/rockchip/rkisp1/rkisp1-resizer.c      |  6 ++++--
 4 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
index 55e863b762e6..94e173706eb4 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
@@ -708,6 +708,7 @@ irqreturn_t rkisp1_capture_isr(int irq, void *ctx)
 {
 	struct device *dev = ctx;
 	struct rkisp1_device *rkisp1 = dev_get_drvdata(dev);
+	unsigned int dev_count = rkisp1_path_count(rkisp1);
 	unsigned int i;
 	u32 status;
 
@@ -717,7 +718,7 @@ irqreturn_t rkisp1_capture_isr(int irq, void *ctx)
 
 	rkisp1_write(rkisp1, RKISP1_CIF_MI_ICR, status);
 
-	for (i = 0; i < ARRAY_SIZE(rkisp1->capture_devs); ++i) {
+	for (i = 0; i < dev_count; ++i) {
 		struct rkisp1_capture *cap = &rkisp1->capture_devs[i];
 
 		if (!(status & RKISP1_CIF_MI_FRAME(cap)))
@@ -874,6 +875,7 @@ static void rkisp1_cap_stream_enable(struct rkisp1_capture *cap)
 {
 	struct rkisp1_device *rkisp1 = cap->rkisp1;
 	struct rkisp1_capture *other = &rkisp1->capture_devs[cap->id ^ 1];
+	bool has_self_path = rkisp1_has_feature(rkisp1, SELF_PATH);
 
 	cap->ops->set_data_path(cap);
 	cap->ops->config(cap);
@@ -891,7 +893,7 @@ static void rkisp1_cap_stream_enable(struct rkisp1_capture *cap)
 	 * This's also required because the second FE maybe corrupt
 	 * especially when run at 120fps.
 	 */
-	if (!other->is_streaming) {
+	if (!has_self_path || !other->is_streaming) {
 		/* force cfg update */
 		rkisp1_write(rkisp1, RKISP1_CIF_MI_INIT,
 			     RKISP1_CIF_MI_INIT_SOFT_UPD);
@@ -1445,10 +1447,11 @@ rkisp1_capture_init(struct rkisp1_device *rkisp1, enum rkisp1_stream_id id)
 
 int rkisp1_capture_devs_register(struct rkisp1_device *rkisp1)
 {
+	unsigned int dev_count = rkisp1_path_count(rkisp1);
 	unsigned int i;
 	int ret;
 
-	for (i = 0; i < ARRAY_SIZE(rkisp1->capture_devs); i++) {
+	for (i = 0; i < dev_count; i++) {
 		struct rkisp1_capture *cap = &rkisp1->capture_devs[i];
 
 		rkisp1_capture_init(rkisp1, i);
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
index fff5f5264386..8b6c0977ee91 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
@@ -118,6 +118,7 @@ enum rkisp1_feature {
 	RKISP1_FEATURE_RSZ_CROP = BIT(2),
 	RKISP1_FEATURE_MAIN_STRIDE = BIT(3),
 	RKISP1_FEATURE_DMA_34BIT = BIT(4),
+	RKISP1_FEATURE_SELF_PATH = BIT(5),
 };
 
 #define rkisp1_has_feature(rkisp1, feature) \
@@ -548,6 +549,19 @@ int rkisp1_cap_enum_mbus_codes(struct rkisp1_capture *cap,
  */
 const struct rkisp1_mbus_info *rkisp1_mbus_info_get_by_index(unsigned int index);
 
+/*
+ * rkisp1_path_count - Return the number of paths supported by the device
+ *
+ * Some devices only have a main path, while other device have both a main path
+ * and a self path. This function returns the number of paths that this device
+ * has, based on the feature flags. It should be used insted of checking
+ * ARRAY_SIZE of capture_devs/resizer_devs.
+ */
+static inline unsigned int rkisp1_path_count(struct rkisp1_device *rkisp1)
+{
+	return rkisp1_has_feature(rkisp1, SELF_PATH) ? 2 : 1;
+}
+
 /*
  * rkisp1_sd_adjust_crop_rect - adjust a rectangle to fit into another rectangle.
  *
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
index 2b13962c5c32..8f3001bf7562 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
@@ -336,6 +336,7 @@ static const struct dev_pm_ops rkisp1_pm_ops = {
 
 static int rkisp1_create_links(struct rkisp1_device *rkisp1)
 {
+	unsigned int dev_count = rkisp1_path_count(rkisp1);
 	unsigned int i;
 	int ret;
 
@@ -351,7 +352,7 @@ static int rkisp1_create_links(struct rkisp1_device *rkisp1)
 	}
 
 	/* create ISP->RSZ->CAP links */
-	for (i = 0; i < 2; i++) {
+	for (i = 0; i < dev_count; i++) {
 		struct media_entity *resizer =
 			&rkisp1->resizer_devs[i].sd.entity;
 		struct media_entity *capture =
@@ -476,7 +477,8 @@ static const struct rkisp1_info px30_isp_info = {
 	.isr_size = ARRAY_SIZE(px30_isp_isrs),
 	.isp_ver = RKISP1_V12,
 	.features = RKISP1_FEATURE_MIPI_CSI2
-		  | RKISP1_FEATURE_DUAL_CROP,
+		  | RKISP1_FEATURE_DUAL_CROP
+		  | RKISP1_FEATURE_SELF_PATH,
 };
 
 static const char * const rk3399_isp_clks[] = {
@@ -496,7 +498,8 @@ static const struct rkisp1_info rk3399_isp_info = {
 	.isr_size = ARRAY_SIZE(rk3399_isp_isrs),
 	.isp_ver = RKISP1_V10,
 	.features = RKISP1_FEATURE_MIPI_CSI2
-		  | RKISP1_FEATURE_DUAL_CROP,
+		  | RKISP1_FEATURE_DUAL_CROP
+		  | RKISP1_FEATURE_SELF_PATH,
 };
 
 static const char * const imx8mp_isp_clks[] = {
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c
index 7ff7b608fc3f..891a622124e2 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c
@@ -712,6 +712,7 @@ static int rkisp1_rsz_s_stream(struct v4l2_subdev *sd, int enable)
 	struct rkisp1_device *rkisp1 = rsz->rkisp1;
 	struct rkisp1_capture *other = &rkisp1->capture_devs[rsz->id ^ 1];
 	enum rkisp1_shadow_regs_when when = RKISP1_SHADOW_REGS_SYNC;
+	bool has_self_path = rkisp1_has_feature(rkisp1, SELF_PATH);
 
 	if (!enable) {
 		if (rkisp1_has_feature(rkisp1, DUAL_CROP))
@@ -720,7 +721,7 @@ static int rkisp1_rsz_s_stream(struct v4l2_subdev *sd, int enable)
 		return 0;
 	}
 
-	if (other->is_streaming)
+	if (has_self_path && other->is_streaming)
 		when = RKISP1_SHADOW_REGS_ASYNC;
 
 	mutex_lock(&rsz->ops_lock);
@@ -808,10 +809,11 @@ static int rkisp1_rsz_register(struct rkisp1_resizer *rsz)
 
 int rkisp1_resizer_devs_register(struct rkisp1_device *rkisp1)
 {
+	unsigned int dev_count = rkisp1_path_count(rkisp1);
 	unsigned int i;
 	int ret;
 
-	for (i = 0; i < ARRAY_SIZE(rkisp1->resizer_devs); i++) {
+	for (i = 0; i < dev_count; i++) {
 		struct rkisp1_resizer *rsz = &rkisp1->resizer_devs[i];
 
 		rsz->rkisp1 = rkisp1;
-- 
2.35.1

[PATCH v3 13/14] media: rkisp1: Add YC swap capability

[Paul Elder]

The ISP version in the i.MX8MP has an MI_OUTPUT_ALIGN_FORMAT register
that the rk3399 does not have. This register allows swapping bytes,
which can be used to implement UYVY from YUYV.

Add a feature flag to signify the presence of this feature, and add it
to the i.MX8MP match data. Also add it as a flag to the format info in
the list of supported formats by the capture v4l2 devices, and update
enum_fmt and s_fmt to take it into account.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 .../platform/rockchip/rkisp1/rkisp1-capture.c | 26 ++++++++++++++-----
 .../platform/rockchip/rkisp1/rkisp1-common.h  |  2 ++
 .../platform/rockchip/rkisp1/rkisp1-dev.c     |  3 ++-
 3 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
index 94e173706eb4..29565440b6e7 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
@@ -47,13 +47,15 @@ enum rkisp1_plane {
  * @fourcc: pixel format
  * @fmt_type: helper filed for pixel format
  * @uv_swap: if cb cr swapped, for yuv
+ * @yc_swap: if y and cb/cr swapped, for yuv
  * @write_format: defines how YCbCr self picture data is written to memory
  * @output_format: defines sp output format
  * @mbus: the mbus code on the src resizer pad that matches the pixel format
  */
 struct rkisp1_capture_fmt_cfg {
 	u32 fourcc;
-	u8 uv_swap;
+	u32 uv_swap : 1;
+	u32 yc_swap : 1;
 	u32 write_format;
 	u32 output_format;
 	u32 mbus;
@@ -1126,10 +1128,14 @@ rkisp1_fill_pixfmt(struct v4l2_pix_format_mplane *pixm,
 static const struct rkisp1_capture_fmt_cfg *
 rkisp1_find_fmt_cfg(const struct rkisp1_capture *cap, const u32 pixelfmt)
 {
+	bool yc_swap_support = rkisp1_has_feature(cap->rkisp1, MI_OUTPUT_ALIGN);
 	unsigned int i;
 
 	for (i = 0; i < cap->config->fmt_size; i++) {
-		if (cap->config->fmts[i].fourcc == pixelfmt)
+		const struct rkisp1_capture_fmt_cfg *fmt = &cap->config->fmts[i];
+
+		if (fmt->fourcc == pixelfmt &&
+		    (!fmt->yc_swap || yc_swap_support))
 			return &cap->config->fmts[i];
 	}
 	return NULL;
@@ -1199,23 +1205,29 @@ static int rkisp1_enum_fmt_vid_cap_mplane(struct file *file, void *priv,
 {
 	struct rkisp1_capture *cap = video_drvdata(file);
 	const struct rkisp1_capture_fmt_cfg *fmt = NULL;
+	bool yc_swap_support = rkisp1_has_feature(cap->rkisp1, MI_OUTPUT_ALIGN);
 	unsigned int i, n = 0;
 
-	if (!f->mbus_code) {
-		if (f->index >= cap->config->fmt_size)
-			return -EINVAL;
+	if (f->index >= cap->config->fmt_size)
+		return -EINVAL;
 
+	if (!f->mbus_code && yc_swap_support) {
 		fmt = &cap->config->fmts[f->index];
 		f->pixelformat = fmt->fourcc;
 		return 0;
 	}
 
 	for (i = 0; i < cap->config->fmt_size; i++) {
-		if (cap->config->fmts[i].mbus != f->mbus_code)
+		fmt = &cap->config->fmts[i];
+
+		if (f->mbus_code && fmt->mbus != f->mbus_code)
+			continue;
+
+		if (!yc_swap_support && fmt->yc_swap)
 			continue;
 
 		if (n++ == f->index) {
-			f->pixelformat = cap->config->fmts[i].fourcc;
+			f->pixelformat = fmt->fourcc;
 			return 0;
 		}
 	}
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
index 8b6c0977ee91..8b317060ab97 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
@@ -107,6 +107,7 @@ enum rkisp1_isp_pad {
  * @RKISP1_FEATURE_RSZ_CROP: The ISP supports cropping in the resizer
  * @RKISP1_FEATURE_MAIN_STRIDE: The ISP supports configurable stride on the main path
  * @RKISP1_FEATURE_DMA_34BIT: The ISP uses 34-bit DMA addresses
+ * @RKISP1_FEATURE_MI_OUTPUT_ALIGN: The ISP has the MI_OUTPUT_ALIGN_FORMAT register
  *
  * The ISP features are stored in a bitmask in &rkisp1_info.features and allow
  * the driver to implement support for features present in some ISP versions
@@ -119,6 +120,7 @@ enum rkisp1_feature {
 	RKISP1_FEATURE_MAIN_STRIDE = BIT(3),
 	RKISP1_FEATURE_DMA_34BIT = BIT(4),
 	RKISP1_FEATURE_SELF_PATH = BIT(5),
+	RKISP1_FEATURE_MI_OUTPUT_ALIGN = BIT(6),
 };
 
 #define rkisp1_has_feature(rkisp1, feature) \
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
index 8f3001bf7562..35fc1479f9cd 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
@@ -520,7 +520,8 @@ static const struct rkisp1_info imx8mp_isp_info = {
 	.isp_ver = IMX8MP_V10,
 	.features = RKISP1_FEATURE_RSZ_CROP
 		  | RKISP1_FEATURE_MAIN_STRIDE
-		  | RKISP1_FEATURE_DMA_34BIT,
+		  | RKISP1_FEATURE_DMA_34BIT
+		  | RKISP1_FEATURE_MI_OUTPUT_ALIGN,
 };
 
 static const struct of_device_id rkisp1_of_match[] = {
-- 
2.35.1

[PATCH v3 14/14] media: rkisp1: Add UYVY as an output format

[Paul Elder]

Add support for UYVY as an output format. The uv_swap bit in the
MI_XTD_FORMAT_CTRL register that is used for the NV formats does not
work for packed YUV formats. Thus, UYVY support is implemented via
byte-swapping. This method clearly does not work for implementing
support for YVYU and VYUY.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 .../platform/rockchip/rkisp1/rkisp1-capture.c | 41 +++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
index 29565440b6e7..8a6a5d25a3b8 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
@@ -97,6 +97,12 @@ static const struct rkisp1_capture_fmt_cfg rkisp1_mp_fmts[] = {
 		.uv_swap = 0,
 		.write_format = RKISP1_MI_CTRL_MP_WRITE_YUVINT,
 		.mbus = MEDIA_BUS_FMT_YUYV8_2X8,
+	}, {
+		.fourcc = V4L2_PIX_FMT_UYVY,
+		.uv_swap = 0,
+		.yc_swap = 1,
+		.write_format = RKISP1_MI_CTRL_MP_WRITE_YUVINT,
+		.mbus = MEDIA_BUS_FMT_YUYV8_2X8,
 	}, {
 		.fourcc = V4L2_PIX_FMT_YUV422P,
 		.uv_swap = 0,
@@ -221,6 +227,13 @@ static const struct rkisp1_capture_fmt_cfg rkisp1_sp_fmts[] = {
 		.write_format = RKISP1_MI_CTRL_SP_WRITE_INT,
 		.output_format = RKISP1_MI_CTRL_SP_OUTPUT_YUV422,
 		.mbus = MEDIA_BUS_FMT_YUYV8_2X8,
+	}, {
+		.fourcc = V4L2_PIX_FMT_UYVY,
+		.uv_swap = 0,
+		.yc_swap = 1,
+		.write_format = RKISP1_MI_CTRL_SP_WRITE_INT,
+		.output_format = RKISP1_MI_CTRL_SP_OUTPUT_YUV422,
+		.mbus = MEDIA_BUS_FMT_YUYV8_2X8,
 	}, {
 		.fourcc = V4L2_PIX_FMT_YUV422P,
 		.uv_swap = 0,
@@ -442,6 +455,20 @@ static void rkisp1_mp_config(struct rkisp1_capture *cap)
 		rkisp1_write(rkisp1, RKISP1_CIF_MI_XTD_FORMAT_CTRL, reg);
 	}
 
+	/*
+	 * U/V swapping with the MI_XTD_FORMAT_CTRL register only works for
+	 * NV12/NV21 and NV16/NV61, so instead use byte swap to support UYVY.
+	 * YVYU and VYUY cannot be supported with this method.
+	 */
+	if (rkisp1->info->features & RKISP1_FEATURE_MI_OUTPUT_ALIGN) {
+		reg = rkisp1_read(rkisp1, RKISP1_CIF_MI_OUTPUT_ALIGN_FORMAT);
+		if (cap->pix.cfg->yc_swap)
+			reg |= RKISP1_CIF_OUTPUT_ALIGN_FORMAT_MP_BYTE_SWAP_BYTES;
+		else
+			reg &= ~RKISP1_CIF_OUTPUT_ALIGN_FORMAT_MP_BYTE_SWAP_BYTES;
+		rkisp1_write(rkisp1, RKISP1_CIF_MI_OUTPUT_ALIGN_FORMAT, reg);
+	}
+
 	rkisp1_mi_config_ctrl(cap);
 
 	reg = rkisp1_read(rkisp1, RKISP1_CIF_MI_CTRL);
@@ -483,6 +510,20 @@ static void rkisp1_sp_config(struct rkisp1_capture *cap)
 		rkisp1_write(rkisp1, RKISP1_CIF_MI_XTD_FORMAT_CTRL, reg);
 	}
 
+	/*
+	 * U/V swapping with the MI_XTD_FORMAT_CTRL register only works for
+	 * NV12/NV21 and NV16/NV61, so instead use byte swap to support UYVY.
+	 * YVYU and VYUY cannot be supported with this method.
+	 */
+	if (rkisp1->info->features & RKISP1_FEATURE_MI_OUTPUT_ALIGN) {
+		reg = rkisp1_read(rkisp1, RKISP1_CIF_MI_OUTPUT_ALIGN_FORMAT);
+		if (cap->pix.cfg->yc_swap)
+			reg |= RKISP1_CIF_OUTPUT_ALIGN_FORMAT_SP_BYTE_SWAP_BYTES;
+		else
+			reg &= ~RKISP1_CIF_OUTPUT_ALIGN_FORMAT_SP_BYTE_SWAP_BYTES;
+		rkisp1_write(rkisp1, RKISP1_CIF_MI_OUTPUT_ALIGN_FORMAT, reg);
+	}
+
 	rkisp1_mi_config_ctrl(cap);
 
 	mi_ctrl = rkisp1_read(rkisp1, RKISP1_CIF_MI_CTRL);
-- 
2.35.1

Re: [PATCH v3 01/14] dt-bindings: media: rkisp1: Add i.MX8MP ISP to compatible

[Krzysztof Kozlowski]

On 18/11/2022 10:39, Paul Elder wrote:
> The i.MX8MP ISP is compatbile with the rkisp1 driver. Add it to the list
> of compatible strings. While at it, expand on the description of the
> clocks to make it clear which clock in the i.MX8MP ISP they map to,
> based on the names from the datasheet (which are confusing).
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

This is a friendly reminder during the review process.

It looks like you received a tag and forgot to add it.

If you do not know the process, here is a short explanation:
Please add Acked-by/Reviewed-by/Tested-by tags when posting new
versions. However, there's no need to repost patches *only* to add the
tags. The upstream maintainer will do that for acks received on the
version they apply.

https://elixir.bootlin.com/linux/v5.17/source/Documentation/process/submitting-patches.rst#L540

If a tag was not added on purpose, please state why and what changed.

Best regards,
Krzysztof

Re: [PATCH v3 02/14] dt-bindings: media: rkisp1: Add i.MX8MP ISP example

[Krzysztof Kozlowski]

On 18/11/2022 10:39, Paul Elder wrote:
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Add an example to the rockchip-isp1 DT binding that showcases usage of
> the parallel input of the ISP, connected to the CSI-2 receiver internal
> to the i.MX8MP.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Missing SoB.

> ---
>  .../bindings/media/rockchip-isp1.yaml         | 72 +++++++++++++++++++
>  1 file changed, 72 insertions(+)
> 

I don't know what do you demonstrate there... usage of endpoints? That's
the only difference. Such usage is the same everywhere, nothing specific
to this example. You already have two examples, so I don't think this
brings anything more.

Best regards,
Krzysztof

Re: [PATCH v3 02/14] dt-bindings: media: rkisp1: Add i.MX8MP ISP example

[Rob Herring]

On Fri, 18 Nov 2022 18:39:19 +0900, Paul Elder wrote:
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Add an example to the rockchip-isp1 DT binding that showcases usage of
> the parallel input of the ISP, connected to the CSI-2 receiver internal
> to the i.MX8MP.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  .../bindings/media/rockchip-isp1.yaml         | 72 +++++++++++++++++++
>  1 file changed, 72 insertions(+)
> 

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:
Documentation/devicetree/bindings/media/rockchip-isp1.example.dts:199:18: fatal error: dt-bindings/media/video-interfaces.h: No such file or directory
  199 |         #include <dt-bindings/media/video-interfaces.h>
      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[1]: *** [scripts/Makefile.lib:406: Documentation/devicetree/bindings/media/rockchip-isp1.example.dtb] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1492: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20221118093931.1284465-3-paul.elder@ideasonboard.com

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 after running the above command.

Re: [PATCH v3 01/14] dt-bindings: media: rkisp1: Add i.MX8MP ISP to compatible

[Paul Elder]

On Fri, Nov 18, 2022 at 02:02:28PM +0100, Krzysztof Kozlowski wrote:
> On 18/11/2022 10:39, Paul Elder wrote:
> > The i.MX8MP ISP is compatbile with the rkisp1 driver. Add it to the list
> > of compatible strings. While at it, expand on the description of the
> > clocks to make it clear which clock in the i.MX8MP ISP they map to,
> > based on the names from the datasheet (which are confusing).
> > 
> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> This is a friendly reminder during the review process.
> 
> It looks like you received a tag and forgot to add it.

Sorry, indeed I did. This series has been a bit hectic...


Thanks,

Paul

> 
> If you do not know the process, here is a short explanation:
> Please add Acked-by/Reviewed-by/Tested-by tags when posting new
> versions. However, there's no need to repost patches *only* to add the
> tags. The upstream maintainer will do that for acks received on the
> version they apply.
> 
> https://elixir.bootlin.com/linux/v5.17/source/Documentation/process/submitting-patches.rst#L540
> 
> If a tag was not added on purpose, please state why and what changed.
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH v3 02/14] dt-bindings: media: rkisp1: Add i.MX8MP ISP example

[Paul Elder]

On Fri, Nov 18, 2022 at 07:31:19AM -0600, Rob Herring wrote:
> 
> On Fri, 18 Nov 2022 18:39:19 +0900, Paul Elder wrote:
> > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> > Add an example to the rockchip-isp1 DT binding that showcases usage of
> > the parallel input of the ISP, connected to the CSI-2 receiver internal
> > to the i.MX8MP.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  .../bindings/media/rockchip-isp1.yaml         | 72 +++++++++++++++++++
> >  1 file changed, 72 insertions(+)
> > 
> 
> 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:
> Documentation/devicetree/bindings/media/rockchip-isp1.example.dts:199:18: fatal error: dt-bindings/media/video-interfaces.h: No such file or directory
>   199 |         #include <dt-bindings/media/video-interfaces.h>
>       |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> compilation terminated.
> make[1]: *** [scripts/Makefile.lib:406: Documentation/devicetree/bindings/media/rockchip-isp1.example.dtb] Error 1
> make[1]: *** Waiting for unfinished jobs....
> make: *** [Makefile:1492: dt_binding_check] Error 2
> 
> doc reference errors (make refcheckdocs):
> 
> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20221118093931.1284465-3-paul.elder@ideasonboard.com
> 
> This check can fail if there are any dependencies. The base for a patch
> series is generally the most recent rc1.

In the cover letter I mention:

"This series depends on v3 of "dt-bindings: media: Add macros for video
interface bus types" [1]."

[1] https://lore.kernel.org/linux-media/20220615221410.27459-2-laurent.pinchart@ideasonboard.com/


Paul

> 
> 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 after running the above command.
> 

Re: [PATCH v3 02/14] dt-bindings: media: rkisp1: Add i.MX8MP ISP example

[Paul Elder]

On Fri, Nov 18, 2022 at 02:06:14PM +0100, Krzysztof Kozlowski wrote:
> On 18/11/2022 10:39, Paul Elder wrote:
> > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> > Add an example to the rockchip-isp1 DT binding that showcases usage of
> > the parallel input of the ISP, connected to the CSI-2 receiver internal
> > to the i.MX8MP.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Missing SoB.

I don't quite understand. I see an SoB right there.

> 
> > ---
> >  .../bindings/media/rockchip-isp1.yaml         | 72 +++++++++++++++++++
> >  1 file changed, 72 insertions(+)
> > 
> 
> I don't know what do you demonstrate there... usage of endpoints? That's
> the only difference. Such usage is the same everywhere, nothing specific

I guess...? Doesn't the same argument apply against the px30 example too
then?

> to this example. You already have two examples, so I don't think this
> brings anything more.

We do have usage of this in imx8mp.dtsi and overlays for the ISP, but
those patches haven't been sent/merged yet, so in the meantime I think
there is value in providing an example here for the imx8mp.


Thanks,

Paul

Re: [PATCH v3 03/14] media: rkisp1: Add and use rkisp1_has_feature() macro

[Dafna Hirschfeld]

On 18.11.2022 18:39, Paul Elder wrote:
>From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
>Simplify feature tests with a macro that shortens lines.
>
>Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
>Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>---
> .../media/platform/rockchip/rkisp1/rkisp1-common.h |  3 +++
> .../media/platform/rockchip/rkisp1/rkisp1-dev.c    | 14 +++++++-------
> 2 files changed, 10 insertions(+), 7 deletions(-)
>
>diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
>index a1293c45aae1..fc33c185b99f 100644
>--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
>+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
>@@ -111,6 +111,9 @@ enum rkisp1_feature {
> 	RKISP1_FEATURE_MIPI_CSI2 = BIT(0),
> };
>
>+#define rkisp1_has_feature(rkisp1, feature) \
>+	((rkisp1)->info->features & RKISP1_FEATURE_##feature)

maybe instead of that string concatination you can remove the 'RKISP1_FEATURE' prefix from the
enum

thanks,
Dafna

>+
> /*
>  * struct rkisp1_info - Model-specific ISP Information
>  *
>diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
>index f2475c6235ea..e348d8c86861 100644
>--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
>+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
>@@ -206,7 +206,7 @@ static int rkisp1_subdev_notifier_register(struct rkisp1_device *rkisp1)
> 		switch (reg) {
> 		case 0:
> 			/* MIPI CSI-2 port */
>-			if (!(rkisp1->info->features & RKISP1_FEATURE_MIPI_CSI2)) {
>+			if (!rkisp1_has_feature(rkisp1, MIPI_CSI2)) {
> 				dev_err(rkisp1->dev,
> 					"internal CSI must be available for port 0\n");
> 				ret = -EINVAL;
>@@ -338,7 +338,7 @@ static int rkisp1_create_links(struct rkisp1_device *rkisp1)
> 	unsigned int i;
> 	int ret;
>
>-	if (rkisp1->info->features & RKISP1_FEATURE_MIPI_CSI2) {
>+	if (rkisp1_has_feature(rkisp1, MIPI_CSI2)) {
> 		/* Link the CSI receiver to the ISP. */
> 		ret = media_create_pad_link(&rkisp1->csi.sd.entity,
> 					    RKISP1_CSI_PAD_SRC,
>@@ -390,7 +390,7 @@ static int rkisp1_create_links(struct rkisp1_device *rkisp1)
>
> static void rkisp1_entities_unregister(struct rkisp1_device *rkisp1)
> {
>-	if (rkisp1->info->features & RKISP1_FEATURE_MIPI_CSI2)
>+	if (rkisp1_has_feature(rkisp1, MIPI_CSI2))
> 		rkisp1_csi_unregister(rkisp1);
> 	rkisp1_params_unregister(rkisp1);
> 	rkisp1_stats_unregister(rkisp1);
>@@ -423,7 +423,7 @@ static int rkisp1_entities_register(struct rkisp1_device *rkisp1)
> 	if (ret)
> 		goto error;
>
>-	if (rkisp1->info->features & RKISP1_FEATURE_MIPI_CSI2) {
>+	if (rkisp1_has_feature(rkisp1, MIPI_CSI2)) {
> 		ret = rkisp1_csi_register(rkisp1);
> 		if (ret)
> 			goto error;
>@@ -590,7 +590,7 @@ static int rkisp1_probe(struct platform_device *pdev)
> 		goto err_unreg_v4l2_dev;
> 	}
>
>-	if (rkisp1->info->features & RKISP1_FEATURE_MIPI_CSI2) {
>+	if (rkisp1_has_feature(rkisp1, MIPI_CSI2)) {
> 		ret = rkisp1_csi_init(rkisp1);
> 		if (ret)
> 			goto err_unreg_media_dev;
>@@ -611,7 +611,7 @@ static int rkisp1_probe(struct platform_device *pdev)
> err_unreg_entities:
> 	rkisp1_entities_unregister(rkisp1);
> err_cleanup_csi:
>-	if (rkisp1->info->features & RKISP1_FEATURE_MIPI_CSI2)
>+	if (rkisp1_has_feature(rkisp1, MIPI_CSI2))
> 		rkisp1_csi_cleanup(rkisp1);
> err_unreg_media_dev:
> 	media_device_unregister(&rkisp1->media_dev);
>@@ -630,7 +630,7 @@ static int rkisp1_remove(struct platform_device *pdev)
> 	v4l2_async_nf_cleanup(&rkisp1->notifier);
>
> 	rkisp1_entities_unregister(rkisp1);
>-	if (rkisp1->info->features & RKISP1_FEATURE_MIPI_CSI2)
>+	if (rkisp1_has_feature(rkisp1, MIPI_CSI2))
> 		rkisp1_csi_cleanup(rkisp1);
> 	rkisp1_debug_cleanup(rkisp1);
>
>-- 
>2.35.1
>

Re: [PATCH v3 02/14] dt-bindings: media: rkisp1: Add i.MX8MP ISP example

[Laurent Pinchart]

Hi Krzysztof,

On Fri, Nov 18, 2022 at 02:06:14PM +0100, Krzysztof Kozlowski wrote:
> On 18/11/2022 10:39, Paul Elder wrote:
> > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> > Add an example to the rockchip-isp1 DT binding that showcases usage of
> > the parallel input of the ISP, connected to the CSI-2 receiver internal
> > to the i.MX8MP.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Missing SoB.
> 
> > ---
> >  .../bindings/media/rockchip-isp1.yaml         | 72 +++++++++++++++++++
> >  1 file changed, 72 insertions(+)
> > 
> 
> I don't know what do you demonstrate there... usage of endpoints? That's
> the only difference. Such usage is the same everywhere, nothing specific
> to this example. You already have two examples, so I don't think this
> brings anything more.

The i.MX8MP is the only SoC integrating this ISP (and supported in
mainlineà that has an external CSI-2 receiver, as opposed to using the
CSI-2 receiver from the ISP. This patch this showcases the DT
integration for that use case. If you think it's not worth it, I'm fine
dropping it.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 03/14] media: rkisp1: Add and use rkisp1_has_feature() macro

[Laurent Pinchart]

Hi Dafna,

On Sat, Nov 19, 2022 at 01:03:22PM +0200, Dafna Hirschfeld wrote:
> On 18.11.2022 18:39, Paul Elder wrote:
> > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> > Simplify feature tests with a macro that shortens lines.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > ---
> >  .../media/platform/rockchip/rkisp1/rkisp1-common.h |  3 +++
> >  .../media/platform/rockchip/rkisp1/rkisp1-dev.c    | 14 +++++++-------
> >  2 files changed, 10 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> > index a1293c45aae1..fc33c185b99f 100644
> > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> > @@ -111,6 +111,9 @@ enum rkisp1_feature {
> >  	RKISP1_FEATURE_MIPI_CSI2 = BIT(0),
> >  };
> > 
> > +#define rkisp1_has_feature(rkisp1, feature) \
> > +	((rkisp1)->info->features & RKISP1_FEATURE_##feature)
> 
> maybe instead of that string concatination you can remove the
> 'RKISP1_FEATURE' prefix from the enum

We could, but I'm worried this would create short names subject to
namespace clashes (such as "MIPI_CSI2" for instance).

> > +
> >  /*
> >   * struct rkisp1_info - Model-specific ISP Information
> >   *
> > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
> > index f2475c6235ea..e348d8c86861 100644
> > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
> > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
> > @@ -206,7 +206,7 @@ static int rkisp1_subdev_notifier_register(struct rkisp1_device *rkisp1)
> >  		switch (reg) {
> >  		case 0:
> >  			/* MIPI CSI-2 port */
> > -			if (!(rkisp1->info->features & RKISP1_FEATURE_MIPI_CSI2)) {
> > +			if (!rkisp1_has_feature(rkisp1, MIPI_CSI2)) {
> >  				dev_err(rkisp1->dev,
> >  					"internal CSI must be available for port 0\n");
> >  				ret = -EINVAL;
> > @@ -338,7 +338,7 @@ static int rkisp1_create_links(struct rkisp1_device *rkisp1)
> >  	unsigned int i;
> >  	int ret;
> > 
> > -	if (rkisp1->info->features & RKISP1_FEATURE_MIPI_CSI2) {
> > +	if (rkisp1_has_feature(rkisp1, MIPI_CSI2)) {
> >  		/* Link the CSI receiver to the ISP. */
> >  		ret = media_create_pad_link(&rkisp1->csi.sd.entity,
> >  					    RKISP1_CSI_PAD_SRC,
> > @@ -390,7 +390,7 @@ static int rkisp1_create_links(struct rkisp1_device *rkisp1)
> > 
> >  static void rkisp1_entities_unregister(struct rkisp1_device *rkisp1)
> >  {
> > -	if (rkisp1->info->features & RKISP1_FEATURE_MIPI_CSI2)
> > +	if (rkisp1_has_feature(rkisp1, MIPI_CSI2))
> >  		rkisp1_csi_unregister(rkisp1);
> >  	rkisp1_params_unregister(rkisp1);
> >  	rkisp1_stats_unregister(rkisp1);
> > @@ -423,7 +423,7 @@ static int rkisp1_entities_register(struct rkisp1_device *rkisp1)
> >  	if (ret)
> >  		goto error;
> > 
> > -	if (rkisp1->info->features & RKISP1_FEATURE_MIPI_CSI2) {
> > +	if (rkisp1_has_feature(rkisp1, MIPI_CSI2)) {
> >  		ret = rkisp1_csi_register(rkisp1);
> >  		if (ret)
> >  			goto error;
> > @@ -590,7 +590,7 @@ static int rkisp1_probe(struct platform_device *pdev)
> >  		goto err_unreg_v4l2_dev;
> >  	}
> > 
> > -	if (rkisp1->info->features & RKISP1_FEATURE_MIPI_CSI2) {
> > +	if (rkisp1_has_feature(rkisp1, MIPI_CSI2)) {
> >  		ret = rkisp1_csi_init(rkisp1);
> >  		if (ret)
> >  			goto err_unreg_media_dev;
> > @@ -611,7 +611,7 @@ static int rkisp1_probe(struct platform_device *pdev)
> >  err_unreg_entities:
> >  	rkisp1_entities_unregister(rkisp1);
> >  err_cleanup_csi:
> > -	if (rkisp1->info->features & RKISP1_FEATURE_MIPI_CSI2)
> > +	if (rkisp1_has_feature(rkisp1, MIPI_CSI2))
> >  		rkisp1_csi_cleanup(rkisp1);
> >  err_unreg_media_dev:
> >  	media_device_unregister(&rkisp1->media_dev);
> > @@ -630,7 +630,7 @@ static int rkisp1_remove(struct platform_device *pdev)
> >  	v4l2_async_nf_cleanup(&rkisp1->notifier);
> > 
> >  	rkisp1_entities_unregister(rkisp1);
> > -	if (rkisp1->info->features & RKISP1_FEATURE_MIPI_CSI2)
> > +	if (rkisp1_has_feature(rkisp1, MIPI_CSI2))
> >  		rkisp1_csi_cleanup(rkisp1);
> >  	rkisp1_debug_cleanup(rkisp1);
> > 

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 02/14] dt-bindings: media: rkisp1: Add i.MX8MP ISP example

[Krzysztof Kozlowski]

On 19/11/2022 17:59, Laurent Pinchart wrote:
> Hi Krzysztof,
> 
> On Fri, Nov 18, 2022 at 02:06:14PM +0100, Krzysztof Kozlowski wrote:
>> On 18/11/2022 10:39, Paul Elder wrote:
>>> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>
>>> Add an example to the rockchip-isp1 DT binding that showcases usage of
>>> the parallel input of the ISP, connected to the CSI-2 receiver internal
>>> to the i.MX8MP.
>>>
>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>
>> Missing SoB.
>>
>>> ---
>>>  .../bindings/media/rockchip-isp1.yaml         | 72 +++++++++++++++++++
>>>  1 file changed, 72 insertions(+)
>>>
>>
>> I don't know what do you demonstrate there... usage of endpoints? That's
>> the only difference. Such usage is the same everywhere, nothing specific
>> to this example. You already have two examples, so I don't think this
>> brings anything more.
> 
> The i.MX8MP is the only SoC integrating this ISP (and supported in
> mainlineà that has an external CSI-2 receiver, as opposed to using the
> CSI-2 receiver from the ISP. This patch this showcases the DT
> integration for that use case. If you think it's not worth it, I'm fine
> dropping it.

The purpose of examples are not to demonstrate the SoC, but only this
given binding.
> 

Best regards,
Krzysztof

Re: [PATCH v3 02/14] dt-bindings: media: rkisp1: Add i.MX8MP ISP example

[Krzysztof Kozlowski]

On 19/11/2022 07:55, Paul Elder wrote:
> On Fri, Nov 18, 2022 at 02:06:14PM +0100, Krzysztof Kozlowski wrote:
>> On 18/11/2022 10:39, Paul Elder wrote:
>>> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>
>>> Add an example to the rockchip-isp1 DT binding that showcases usage of
>>> the parallel input of the ISP, connected to the CSI-2 receiver internal
>>> to the i.MX8MP.
>>>
>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>
>> Missing SoB.
> 
> I don't quite understand. I see an SoB right there.

Laurent did not sent it. Did you run checkpatch before sending?

> 
>>
>>> ---
>>>  .../bindings/media/rockchip-isp1.yaml         | 72 +++++++++++++++++++
>>>  1 file changed, 72 insertions(+)
>>>
>>
>> I don't know what do you demonstrate there... usage of endpoints? That's
>> the only difference. Such usage is the same everywhere, nothing specific
> 
> I guess...? Doesn't the same argument apply against the px30 example too
> then?
> 
>> to this example. You already have two examples, so I don't think this
>> brings anything more.
> 
> We do have usage of this in imx8mp.dtsi and overlays for the ISP, but
> those patches haven't been sent/merged yet, so in the meantime I think
> there is value in providing an example here for the imx8mp.

The examples are not for demonstrating imx8mp or any other soc, but this
one given binding. Changing compatibles and few properties is not a
different example - from "exampleness" point of view it is very similar.

Best regards,
Krzysztof

Re: [PATCH v3 02/14] dt-bindings: media: rkisp1: Add i.MX8MP ISP example

[Paul Elder]

On Sun, Nov 20, 2022 at 11:36:31AM +0100, Krzysztof Kozlowski wrote:
> On 19/11/2022 07:55, Paul Elder wrote:
> > On Fri, Nov 18, 2022 at 02:06:14PM +0100, Krzysztof Kozlowski wrote:
> >> On 18/11/2022 10:39, Paul Elder wrote:
> >>> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>>
> >>> Add an example to the rockchip-isp1 DT binding that showcases usage of
> >>> the parallel input of the ISP, connected to the CSI-2 receiver internal
> >>> to the i.MX8MP.
> >>>
> >>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>
> >> Missing SoB.
> > 
> > I don't quite understand. I see an SoB right there.
> 
> Laurent did not sent it. Did you run checkpatch before sending?

That's why he's on the "From:" in the beginning. checkpatch says it's
fine.

> 
> > 
> >>
> >>> ---
> >>>  .../bindings/media/rockchip-isp1.yaml         | 72 +++++++++++++++++++
> >>>  1 file changed, 72 insertions(+)
> >>>
> >>
> >> I don't know what do you demonstrate there... usage of endpoints? That's
> >> the only difference. Such usage is the same everywhere, nothing specific
> > 
> > I guess...? Doesn't the same argument apply against the px30 example too
> > then?
> > 
> >> to this example. You already have two examples, so I don't think this
> >> brings anything more.
> > 
> > We do have usage of this in imx8mp.dtsi and overlays for the ISP, but
> > those patches haven't been sent/merged yet, so in the meantime I think
> > there is value in providing an example here for the imx8mp.
> 
> The examples are not for demonstrating imx8mp or any other soc, but this
> one given binding. Changing compatibles and few properties is not a
> different example - from "exampleness" point of view it is very similar.

Ah okay, I see.


Paul

Re: [PATCH v3 02/14] dt-bindings: media: rkisp1: Add i.MX8MP ISP example

[Krzysztof Kozlowski]

On 21/11/2022 06:09, Paul Elder wrote:
> On Sun, Nov 20, 2022 at 11:36:31AM +0100, Krzysztof Kozlowski wrote:
>> On 19/11/2022 07:55, Paul Elder wrote:
>>> On Fri, Nov 18, 2022 at 02:06:14PM +0100, Krzysztof Kozlowski wrote:
>>>> On 18/11/2022 10:39, Paul Elder wrote:
>>>>> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>>>
>>>>> Add an example to the rockchip-isp1 DT binding that showcases usage of
>>>>> the parallel input of the ISP, connected to the CSI-2 receiver internal
>>>>> to the i.MX8MP.
>>>>>
>>>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>>
>>>> Missing SoB.
>>>
>>> I don't quite understand. I see an SoB right there.
>>
>> Laurent did not sent it. Did you run checkpatch before sending?
> 
> That's why he's on the "From:" in the beginning. checkpatch says it's
> fine.

Ah, indeed, checkpatch misses that feature (it's part of Greg's
verify_signedoff.sh). Anyway, your SoB is missing, as Laurent did not
send the patch.

Best regards,
Krzysztof

Re: [PATCH v3 02/14] dt-bindings: media: rkisp1: Add i.MX8MP ISP example

[Laurent Pinchart]

On Mon, Nov 21, 2022 at 09:04:29AM +0100, Krzysztof Kozlowski wrote:
> On 21/11/2022 06:09, Paul Elder wrote:
> > On Sun, Nov 20, 2022 at 11:36:31AM +0100, Krzysztof Kozlowski wrote:
> >> On 19/11/2022 07:55, Paul Elder wrote:
> >>> On Fri, Nov 18, 2022 at 02:06:14PM +0100, Krzysztof Kozlowski wrote:
> >>>> On 18/11/2022 10:39, Paul Elder wrote:
> >>>>> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>>>>
> >>>>> Add an example to the rockchip-isp1 DT binding that showcases usage of
> >>>>> the parallel input of the ISP, connected to the CSI-2 receiver internal
> >>>>> to the i.MX8MP.
> >>>>>
> >>>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>>>
> >>>> Missing SoB.
> >>>
> >>> I don't quite understand. I see an SoB right there.
> >>
> >> Laurent did not sent it. Did you run checkpatch before sending?
> > 
> > That's why he's on the "From:" in the beginning. checkpatch says it's
> > fine.
> 
> Ah, indeed, checkpatch misses that feature (it's part of Greg's
> verify_signedoff.sh). Anyway, your SoB is missing, as Laurent did not
> send the patch.

I thought adding an SoB was only required either when making changes or
when pushing commits through git, not when forwarding patches on mailing
lists ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 02/14] dt-bindings: media: rkisp1: Add i.MX8MP ISP example

[Krzysztof Kozlowski]

On 21/11/2022 11:38, Laurent Pinchart wrote:
> On Mon, Nov 21, 2022 at 09:04:29AM +0100, Krzysztof Kozlowski wrote:
>> On 21/11/2022 06:09, Paul Elder wrote:
>>> On Sun, Nov 20, 2022 at 11:36:31AM +0100, Krzysztof Kozlowski wrote:
>>>> On 19/11/2022 07:55, Paul Elder wrote:
>>>>> On Fri, Nov 18, 2022 at 02:06:14PM +0100, Krzysztof Kozlowski wrote:
>>>>>> On 18/11/2022 10:39, Paul Elder wrote:
>>>>>>> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>>>>>
>>>>>>> Add an example to the rockchip-isp1 DT binding that showcases usage of
>>>>>>> the parallel input of the ISP, connected to the CSI-2 receiver internal
>>>>>>> to the i.MX8MP.
>>>>>>>
>>>>>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>>>>
>>>>>> Missing SoB.
>>>>>
>>>>> I don't quite understand. I see an SoB right there.
>>>>
>>>> Laurent did not sent it. Did you run checkpatch before sending?
>>>
>>> That's why he's on the "From:" in the beginning. checkpatch says it's
>>> fine.
>>
>> Ah, indeed, checkpatch misses that feature (it's part of Greg's
>> verify_signedoff.sh). Anyway, your SoB is missing, as Laurent did not
>> send the patch.
> 
> I thought adding an SoB was only required either when making changes or
> when pushing commits through git, not when forwarding patches on mailing
> lists ?

Anyone touching the file should signed it off. You cannot send it
without touching (e.g. git format-patch).

https://elixir.bootlin.com/linux/v5.19-rc5/source/Documentation/process/submitting-patches.rst#L397

https://elixir.bootlin.com/linux/v5.19-rc5/source/Documentation/process/submitting-patches.rst#L420

Best regards,
Krzysztof

Re: [PATCH v3 02/14] dt-bindings: media: rkisp1: Add i.MX8MP ISP example

[Laurent Pinchart]

On Mon, Nov 21, 2022 at 12:16:41PM +0100, Krzysztof Kozlowski wrote:
> On 21/11/2022 11:38, Laurent Pinchart wrote:
> > On Mon, Nov 21, 2022 at 09:04:29AM +0100, Krzysztof Kozlowski wrote:
> >> On 21/11/2022 06:09, Paul Elder wrote:
> >>> On Sun, Nov 20, 2022 at 11:36:31AM +0100, Krzysztof Kozlowski wrote:
> >>>> On 19/11/2022 07:55, Paul Elder wrote:
> >>>>> On Fri, Nov 18, 2022 at 02:06:14PM +0100, Krzysztof Kozlowski wrote:
> >>>>>> On 18/11/2022 10:39, Paul Elder wrote:
> >>>>>>> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>>>>>>
> >>>>>>> Add an example to the rockchip-isp1 DT binding that showcases usage of
> >>>>>>> the parallel input of the ISP, connected to the CSI-2 receiver internal
> >>>>>>> to the i.MX8MP.
> >>>>>>>
> >>>>>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>>>>>
> >>>>>> Missing SoB.
> >>>>>
> >>>>> I don't quite understand. I see an SoB right there.
> >>>>
> >>>> Laurent did not sent it. Did you run checkpatch before sending?
> >>>
> >>> That's why he's on the "From:" in the beginning. checkpatch says it's
> >>> fine.
> >>
> >> Ah, indeed, checkpatch misses that feature (it's part of Greg's
> >> verify_signedoff.sh). Anyway, your SoB is missing, as Laurent did not
> >> send the patch.
> > 
> > I thought adding an SoB was only required either when making changes or
> > when pushing commits through git, not when forwarding patches on mailing
> > lists ?
> 
> Anyone touching the file should signed it off. You cannot send it
> without touching (e.g. git format-patch).
> 
> https://elixir.bootlin.com/linux/v5.19-rc5/source/Documentation/process/submitting-patches.rst#L397
> 
> https://elixir.bootlin.com/linux/v5.19-rc5/source/Documentation/process/submitting-patches.rst#L420

The second link states

  SoB chains should reflect the **real** route a patch took as it was
  propagated to the maintainers and ultimately to Linus, with the first
  SoB entry signalling primary authorship of a single author.

This series will (eventually) be upstreamed by me through a pull request
to Mauro. Paul's SoB will thus not be needed. Of course you have no way
to know this when reviewing the series on the list.

Adding a SoB line when taking a patch in a git tree is standard
practice, but when posting unmodified patches to a mailing list, there's
more of a grey area. Look at
https://lore.kernel.org/all/20221024113058.096628238@linuxfoundation.org/
for instance, posted by Greg, but without his SoB.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 02/14] dt-bindings: media: rkisp1: Add i.MX8MP ISP example

[Krzysztof Kozlowski]

On 21/11/2022 14:50, Laurent Pinchart wrote:
> On Mon, Nov 21, 2022 at 12:16:41PM +0100, Krzysztof Kozlowski wrote:
>> On 21/11/2022 11:38, Laurent Pinchart wrote:
>>> On Mon, Nov 21, 2022 at 09:04:29AM +0100, Krzysztof Kozlowski wrote:
>>>> On 21/11/2022 06:09, Paul Elder wrote:
>>>>> On Sun, Nov 20, 2022 at 11:36:31AM +0100, Krzysztof Kozlowski wrote:
>>>>>> On 19/11/2022 07:55, Paul Elder wrote:
>>>>>>> On Fri, Nov 18, 2022 at 02:06:14PM +0100, Krzysztof Kozlowski wrote:
>>>>>>>> On 18/11/2022 10:39, Paul Elder wrote:
>>>>>>>>> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>>>>>>>
>>>>>>>>> Add an example to the rockchip-isp1 DT binding that showcases usage of
>>>>>>>>> the parallel input of the ISP, connected to the CSI-2 receiver internal
>>>>>>>>> to the i.MX8MP.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>>>>>>
>>>>>>>> Missing SoB.
>>>>>>>
>>>>>>> I don't quite understand. I see an SoB right there.
>>>>>>
>>>>>> Laurent did not sent it. Did you run checkpatch before sending?
>>>>>
>>>>> That's why he's on the "From:" in the beginning. checkpatch says it's
>>>>> fine.
>>>>
>>>> Ah, indeed, checkpatch misses that feature (it's part of Greg's
>>>> verify_signedoff.sh). Anyway, your SoB is missing, as Laurent did not
>>>> send the patch.
>>>
>>> I thought adding an SoB was only required either when making changes or
>>> when pushing commits through git, not when forwarding patches on mailing
>>> lists ?
>>
>> Anyone touching the file should signed it off. You cannot send it
>> without touching (e.g. git format-patch).
>>
>> https://elixir.bootlin.com/linux/v5.19-rc5/source/Documentation/process/submitting-patches.rst#L397
>>
>> https://elixir.bootlin.com/linux/v5.19-rc5/source/Documentation/process/submitting-patches.rst#L420
> 
> The second link states
> 
>   SoB chains should reflect the **real** route a patch took as it was
>   propagated to the maintainers and ultimately to Linus, with the first
>   SoB entry signalling primary authorship of a single author.
> 
> This series will (eventually) be upstreamed by me through a pull request
> to Mauro. Paul's SoB will thus not be needed. Of course you have no way
> to know this when reviewing the series on the list.
> 
> Adding a SoB line when taking a patch in a git tree is standard
> practice, but when posting unmodified patches to a mailing list, there's
> more of a grey area. Look at
> https://lore.kernel.org/all/20221024113058.096628238@linuxfoundation.org/
> for instance, posted by Greg, but without his SoB.

I have no clue what Paul modified here what not. I am not going to
investigate and I have no way to actually perform such investigation. I
cannot verify the source.

The case with Greg, is indeed surprising, but I could perform the
verification, because the work is both public and in known place.

It's expected for submitter to certify (c) from the list which was BTW
expressed also many times during many reviews by many people.

Best regards,
Krzysztof

Re: [PATCH v3 02/14] dt-bindings: media: rkisp1: Add i.MX8MP ISP example

[Krzysztof Kozlowski]

On 21/11/2022 17:37, Krzysztof Kozlowski wrote:
> On 21/11/2022 14:50, Laurent Pinchart wrote:
>> On Mon, Nov 21, 2022 at 12:16:41PM +0100, Krzysztof Kozlowski wrote:
>>> On 21/11/2022 11:38, Laurent Pinchart wrote:
>>>> On Mon, Nov 21, 2022 at 09:04:29AM +0100, Krzysztof Kozlowski wrote:
>>>>> On 21/11/2022 06:09, Paul Elder wrote:
>>>>>> On Sun, Nov 20, 2022 at 11:36:31AM +0100, Krzysztof Kozlowski wrote:
>>>>>>> On 19/11/2022 07:55, Paul Elder wrote:
>>>>>>>> On Fri, Nov 18, 2022 at 02:06:14PM +0100, Krzysztof Kozlowski wrote:
>>>>>>>>> On 18/11/2022 10:39, Paul Elder wrote:
>>>>>>>>>> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>>>>>>>>
>>>>>>>>>> Add an example to the rockchip-isp1 DT binding that showcases usage of
>>>>>>>>>> the parallel input of the ISP, connected to the CSI-2 receiver internal
>>>>>>>>>> to the i.MX8MP.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>>>>>>>
>>>>>>>>> Missing SoB.
>>>>>>>>
>>>>>>>> I don't quite understand. I see an SoB right there.
>>>>>>>
>>>>>>> Laurent did not sent it. Did you run checkpatch before sending?
>>>>>>
>>>>>> That's why he's on the "From:" in the beginning. checkpatch says it's
>>>>>> fine.
>>>>>
>>>>> Ah, indeed, checkpatch misses that feature (it's part of Greg's
>>>>> verify_signedoff.sh). Anyway, your SoB is missing, as Laurent did not
>>>>> send the patch.
>>>>
>>>> I thought adding an SoB was only required either when making changes or
>>>> when pushing commits through git, not when forwarding patches on mailing
>>>> lists ?
>>>
>>> Anyone touching the file should signed it off. You cannot send it
>>> without touching (e.g. git format-patch).
>>>
>>> https://elixir.bootlin.com/linux/v5.19-rc5/source/Documentation/process/submitting-patches.rst#L397
>>>
>>> https://elixir.bootlin.com/linux/v5.19-rc5/source/Documentation/process/submitting-patches.rst#L420
>>
>> The second link states
>>
>>   SoB chains should reflect the **real** route a patch took as it was
>>   propagated to the maintainers and ultimately to Linus, with the first
>>   SoB entry signalling primary authorship of a single author.
>>
>> This series will (eventually) be upstreamed by me through a pull request
>> to Mauro. Paul's SoB will thus not be needed. Of course you have no way
>> to know this when reviewing the series on the list.
>>
>> Adding a SoB line when taking a patch in a git tree is standard
>> practice, but when posting unmodified patches to a mailing list, there's
>> more of a grey area. Look at
>> https://lore.kernel.org/all/20221024113058.096628238@linuxfoundation.org/
>> for instance, posted by Greg, but without his SoB.
> 
> I have no clue what Paul modified here what not. I am not going to
> investigate and I have no way to actually perform such investigation. I
> cannot verify the source.

BTW, rebasing is modifying and Paul probably did it (or is likely that
will rebase in the future). Greg did not perform rebases on these, I think.

> 
> The case with Greg, is indeed surprising, but I could perform the
> verification, because the work is both public and in known place.
> 
> It's expected for submitter to certify (c) from the list which was BTW
> expressed also many times during many reviews by many people.
> 
> Best regards,
> Krzysztof
> 

Best regards,
Krzysztof

Re: [PATCH v3 02/14] dt-bindings: media: rkisp1: Add i.MX8MP ISP example

[Laurent Pinchart]

Hi Krzysztof,

(CC'ing Greg)

On Mon, Nov 21, 2022 at 05:37:33PM +0100, Krzysztof Kozlowski wrote:
> On 21/11/2022 14:50, Laurent Pinchart wrote:
> > On Mon, Nov 21, 2022 at 12:16:41PM +0100, Krzysztof Kozlowski wrote:
> >> On 21/11/2022 11:38, Laurent Pinchart wrote:
> >>> On Mon, Nov 21, 2022 at 09:04:29AM +0100, Krzysztof Kozlowski wrote:
> >>>> On 21/11/2022 06:09, Paul Elder wrote:
> >>>>> On Sun, Nov 20, 2022 at 11:36:31AM +0100, Krzysztof Kozlowski wrote:
> >>>>>> On 19/11/2022 07:55, Paul Elder wrote:
> >>>>>>> On Fri, Nov 18, 2022 at 02:06:14PM +0100, Krzysztof Kozlowski wrote:
> >>>>>>>> On 18/11/2022 10:39, Paul Elder wrote:
> >>>>>>>>> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>>>>>>>>
> >>>>>>>>> Add an example to the rockchip-isp1 DT binding that showcases usage of
> >>>>>>>>> the parallel input of the ISP, connected to the CSI-2 receiver internal
> >>>>>>>>> to the i.MX8MP.
> >>>>>>>>>
> >>>>>>>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>>>>>>>
> >>>>>>>> Missing SoB.
> >>>>>>>
> >>>>>>> I don't quite understand. I see an SoB right there.
> >>>>>>
> >>>>>> Laurent did not sent it. Did you run checkpatch before sending?
> >>>>>
> >>>>> That's why he's on the "From:" in the beginning. checkpatch says it's
> >>>>> fine.
> >>>>
> >>>> Ah, indeed, checkpatch misses that feature (it's part of Greg's
> >>>> verify_signedoff.sh). Anyway, your SoB is missing, as Laurent did not
> >>>> send the patch.
> >>>
> >>> I thought adding an SoB was only required either when making changes or
> >>> when pushing commits through git, not when forwarding patches on mailing
> >>> lists ?
> >>
> >> Anyone touching the file should signed it off. You cannot send it
> >> without touching (e.g. git format-patch).
> >>
> >> https://elixir.bootlin.com/linux/v5.19-rc5/source/Documentation/process/submitting-patches.rst#L397
> >>
> >> https://elixir.bootlin.com/linux/v5.19-rc5/source/Documentation/process/submitting-patches.rst#L420
> > 
> > The second link states
> > 
> >   SoB chains should reflect the **real** route a patch took as it was
> >   propagated to the maintainers and ultimately to Linus, with the first
> >   SoB entry signalling primary authorship of a single author.
> > 
> > This series will (eventually) be upstreamed by me through a pull request
> > to Mauro. Paul's SoB will thus not be needed. Of course you have no way
> > to know this when reviewing the series on the list.
> > 
> > Adding a SoB line when taking a patch in a git tree is standard
> > practice, but when posting unmodified patches to a mailing list, there's
> > more of a grey area. Look at
> > https://lore.kernel.org/all/20221024113058.096628238@linuxfoundation.org/
> > for instance, posted by Greg, but without his SoB.
> 
> I have no clue what Paul modified here what not. I am not going to
> investigate and I have no way to actually perform such investigation. I
> cannot verify the source.
> 
> The case with Greg, is indeed surprising, but I could perform the
> verification, because the work is both public and in known place.
> 
> It's expected for submitter to certify (c) from the list which was BTW
> expressed also many times during many reviews by many people.

Given that this patch will be dropped anyway, it doesn't matter much in
this specific case, but for future reference, I've CC'ed Greg to get his
opinion on the matter.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 00/14] media: rkisp1: Add support for i.MX8MP

[Laurent Pinchart]

Hi Adam,

On Wed, Feb 15, 2023 at 07:57:53AM -0600, Adam Ford wrote:
> On Fri, Nov 18, 2022 at 3:44 AM Paul Elder wrote:
> 
> > This series depends on v3 of "dt-bindings: media: Add macros for video
> > interface bus types" [1].
> >
> > This series extends the rkisp1 driver to support the ISP found in the
> > NXP i.MX8MP SoC.
> 
> I'm going to spend some time testing this over the weekend.  Is there a V4
> pending, or should I just test whatever is in Laurent's repo?

I've updated all the v6.2-based branches on
https://gitlab.com/ideasonboard/nxp/linux.git (and renamed them to
v6.2/*). Beside a rebase, the v6.2/isp branch contains (at the bottom) 6
additional patches that I've previously posted to the linux-media
mailing list (feel free to review them ;-)).

My only concern with this series is with patch "media: rkisp1: Add match
data for i.MX8MP ISP", and in particular with the following hunk:

 enum rkisp1_cif_isp_version {
 	RKISP1_V10 = 10,
 	RKISP1_V11,
 	RKISP1_V12,
 	RKISP1_V13,
+	IMX8MP_V10,
 };

It's not a very nice versioning scheme :-S I'll see if I can find
something better, but regardless of that, I'll post v4 with the goal of
merging it in v6.4.

> I have an IMX219 camera with 4-lane support and an i.MX8M Plus kit from
> Beacon, and I want to test the RGGB bayer conversion to see how well it
> works.
> 
> > The ISP IP cores in the Rockchip RK3399 (known as the "Rockchip ISP1")
> > and in the NXP i.MX8MP have the same origin, and have slightly diverged
> > over time as they are now independently developed (afaik) by Rockchip
> > and VeriSilicon. The latter is marketed under the name "ISP8000Nano",
> > and is close enough to the RK3399 ISP that it can easily be supported by
> > the same driver.
>
> Is there a reason the driver cannot be renamed to a more generic name than
> rkisp1 if the Rockchip and VeriSilicon had similar origins?  Having the
> name Rockchip referenced from an NXP i.MX8M Plus seems odd to me.

The common roots of the IP core predate both Rockchip and VeriSilicon.
Those two implementations have now diverged (as with all forks), so
either name would be wrong in some cases :-S

> > The last two patches add support for UYVY output format, which can be
> > implemented on the ISP version in the i.MX8MP but not in the one in the
> > RK3399.
> >
> > This version of the series specifically has been tested on a Polyhex
> > Debix model A with an imx219 (Raspberry Pi cam v2).
> >
> > [1] https://lore.kernel.org/linux-media/20220615221410.27459-2-laurent.pinchart@ideasonboard.com/
> >
> > Laurent Pinchart (3):
> >   dt-bindings: media: rkisp1: Add i.MX8MP ISP example
> >   media: rkisp1: Add and use rkisp1_has_feature() macro
> >   media: rkisp1: Configure gasket on i.MX8MP
> >
> > Paul Elder (11):
> >   dt-bindings: media: rkisp1: Add i.MX8MP ISP to compatible
> >   media: rkisp1: Add match data for i.MX8MP ISP
> >   media: rkisp1: Add and set registers for crop for i.MX8MP
> >   media: rkisp1: Add and set registers for output size config on i.MX8MP
> >   media: rkisp1: Add i.MX8MP-specific registers for MI and resizer
> >   media: rkisp1: Shift DMA buffer addresses on i.MX8MP
> >   media: rkisp1: Add register definitions for the test pattern generator
> >   media: rkisp1: Fix RSZ_CTRL bits for i.MX8MP
> >   media: rkisp1: Support devices without self path
> >   media: rkisp1: Add YC swap capability
> >   media: rkisp1: Add UYVY as an output format
> >
> >  .../bindings/media/rockchip-isp1.yaml         |  79 ++++++++++-
> >  .../platform/rockchip/rkisp1/rkisp1-capture.c | 102 +++++++++++---
> >  .../platform/rockchip/rkisp1/rkisp1-common.h  |  32 +++++
> >  .../platform/rockchip/rkisp1/rkisp1-debug.c   |  14 +-
> >  .../platform/rockchip/rkisp1/rkisp1-dev.c     |  67 +++++++--
> >  .../platform/rockchip/rkisp1/rkisp1-isp.c     | 128 +++++++++++++++++-
> >  .../platform/rockchip/rkisp1/rkisp1-regs.h    |  90 ++++++++++++
> >  .../platform/rockchip/rkisp1/rkisp1-resizer.c |  35 ++++-
> >  include/uapi/linux/rkisp1-config.h            |   2 +
> >  9 files changed, 509 insertions(+), 40 deletions(-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 00/14] media: rkisp1: Add support for i.MX8MP

[Adam Ford]

[-- Attachment #1: Type: text/plain, Size: 6111 bytes --]

On Wed, Feb 15, 2023 at 5:55 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Adam,
>
> On Wed, Feb 15, 2023 at 07:57:53AM -0600, Adam Ford wrote:
> > On Fri, Nov 18, 2022 at 3:44 AM Paul Elder wrote:
> >
> > > This series depends on v3 of "dt-bindings: media: Add macros for video
> > > interface bus types" [1].
> > >
> > > This series extends the rkisp1 driver to support the ISP found in the
> > > NXP i.MX8MP SoC.
> >
> > I'm going to spend some time testing this over the weekend.  Is there a V4
> > pending, or should I just test whatever is in Laurent's repo?
>
> I've updated all the v6.2-based branches on
> https://gitlab.com/ideasonboard/nxp/linux.git (and renamed them to
> v6.2/*). Beside a rebase, the v6.2/isp branch contains (at the bottom) 6
> additional patches that I've previously posted to the linux-media
> mailing list (feel free to review them ;-)).

I grabbed your v6.2 series, and applied some updates to enable an
imx219 camera and routed it through the ISP and configured the camera
to SRGGB10_1X10/640x480 and had the ISP convert to YUYV8_2X8/640x480
and it captured just fine.

With that, I think you can add

Tested-by: Adam Ford <aford173@gmail.com> #imx8mp-beacon

I haven't experimented with the resizer yet,but  I did have some
questions on the AWB.  The AWB appears to be available on the 8MP per
the TRM, and  I see reference to AWB in the driver, but when I query
the subdev via yavta, I didn't see anything obvious on how to enable
it.  My pipeline (attached) shows klisp1_params as video2 and
rkisp1_stats as video1.  I attempted to query both without much
success.

root@beacon-imx8mp-kit:~# yavta -l /dev/video2
Device /dev/video2 opened.
Device `rkisp1_params' on `platform:rkisp1' (driver 'rkisp1') supports
meta-data, output, without mplanes.
unable to query control 0xc0000000: Inappropriate ioctl for device (25).
Meta-data format: RK1P (50314b52) buffer size 3048
root@beacon-imx8mp-kit:~# yavta -l /dev/video1
Device /dev/video1 opened.
Device `rkisp1_stats' on `platform:rkisp1' (driver 'rkisp1') supports
meta-data, capture, without mplanes.
unable to query control 0xc0000000: Inappropriate ioctl for device (25).
Meta-data format: RK1S (53314b52) buffer size 260
root@beacon-imx8mp-kit:~#

Is there documentation somewhere on where to test the AWB?  This is of
particular interest to me, because the RGGB format of the camera comes
across with a green tint.  I am able to remove this green-ness on a
different platform using some AWB on the ARM, but I'd rather do it in
hardware if possible.

Thanks

adam


>
> My only concern with this series is with patch "media: rkisp1: Add match
> data for i.MX8MP ISP", and in particular with the following hunk:
>
>  enum rkisp1_cif_isp_version {
>         RKISP1_V10 = 10,
>         RKISP1_V11,
>         RKISP1_V12,
>         RKISP1_V13,
> +       IMX8MP_V10,
>  };
>
> It's not a very nice versioning scheme :-S I'll see if I can find
> something better, but regardless of that, I'll post v4 with the goal of
> merging it in v6.4.
>
> > I have an IMX219 camera with 4-lane support and an i.MX8M Plus kit from
> > Beacon, and I want to test the RGGB bayer conversion to see how well it
> > works.
> >
> > > The ISP IP cores in the Rockchip RK3399 (known as the "Rockchip ISP1")
> > > and in the NXP i.MX8MP have the same origin, and have slightly diverged
> > > over time as they are now independently developed (afaik) by Rockchip
> > > and VeriSilicon. The latter is marketed under the name "ISP8000Nano",
> > > and is close enough to the RK3399 ISP that it can easily be supported by
> > > the same driver.
> >
> > Is there a reason the driver cannot be renamed to a more generic name than
> > rkisp1 if the Rockchip and VeriSilicon had similar origins?  Having the
> > name Rockchip referenced from an NXP i.MX8M Plus seems odd to me.
>
> The common roots of the IP core predate both Rockchip and VeriSilicon.
> Those two implementations have now diverged (as with all forks), so
> either name would be wrong in some cases :-S
>
> > > The last two patches add support for UYVY output format, which can be
> > > implemented on the ISP version in the i.MX8MP but not in the one in the
> > > RK3399.
> > >
> > > This version of the series specifically has been tested on a Polyhex
> > > Debix model A with an imx219 (Raspberry Pi cam v2).
> > >
> > > [1] https://lore.kernel.org/linux-media/20220615221410.27459-2-laurent.pinchart@ideasonboard.com/
> > >
> > > Laurent Pinchart (3):
> > >   dt-bindings: media: rkisp1: Add i.MX8MP ISP example
> > >   media: rkisp1: Add and use rkisp1_has_feature() macro
> > >   media: rkisp1: Configure gasket on i.MX8MP
> > >
> > > Paul Elder (11):
> > >   dt-bindings: media: rkisp1: Add i.MX8MP ISP to compatible
> > >   media: rkisp1: Add match data for i.MX8MP ISP
> > >   media: rkisp1: Add and set registers for crop for i.MX8MP
> > >   media: rkisp1: Add and set registers for output size config on i.MX8MP
> > >   media: rkisp1: Add i.MX8MP-specific registers for MI and resizer
> > >   media: rkisp1: Shift DMA buffer addresses on i.MX8MP
> > >   media: rkisp1: Add register definitions for the test pattern generator
> > >   media: rkisp1: Fix RSZ_CTRL bits for i.MX8MP
> > >   media: rkisp1: Support devices without self path
> > >   media: rkisp1: Add YC swap capability
> > >   media: rkisp1: Add UYVY as an output format
> > >
> > >  .../bindings/media/rockchip-isp1.yaml         |  79 ++++++++++-
> > >  .../platform/rockchip/rkisp1/rkisp1-capture.c | 102 +++++++++++---
> > >  .../platform/rockchip/rkisp1/rkisp1-common.h  |  32 +++++
> > >  .../platform/rockchip/rkisp1/rkisp1-debug.c   |  14 +-
> > >  .../platform/rockchip/rkisp1/rkisp1-dev.c     |  67 +++++++--
> > >  .../platform/rockchip/rkisp1/rkisp1-isp.c     | 128 +++++++++++++++++-
> > >  .../platform/rockchip/rkisp1/rkisp1-regs.h    |  90 ++++++++++++
> > >  .../platform/rockchip/rkisp1/rkisp1-resizer.c |  35 ++++-
> > >  include/uapi/linux/rkisp1-config.h            |   2 +
> > >  9 files changed, 509 insertions(+), 40 deletions(-)
>
> --
> Regards,
>
> Laurent Pinchart

[-- Attachment #2: imx8mp-beacon.png --]
[-- Type: image/png, Size: 35350 bytes --]

Re: [PATCH v3 00/14] media: rkisp1: Add support for i.MX8MP

[Adam Ford]

On Fri, Nov 18, 2022 at 3:44 AM Paul Elder <paul.elder@ideasonboard.com> wrote:
>
> This series depends on v3 of "dt-bindings: media: Add macros for video
> interface bus types" [1].
>
> This series extends the rkisp1 driver to support the ISP found in the
> NXP i.MX8MP SoC.
>
> The ISP IP cores in the Rockchip RK3399 (known as the "Rockchip ISP1")
> and in the NXP i.MX8MP have the same origin, and have slightly diverged
> over time as they are now independently developed (afaik) by Rockchip
> and VeriSilicon. The latter is marketed under the name "ISP8000Nano",
> and is close enough to the RK3399 ISP that it can easily be supported by
> the same driver.
>
> The last two patches add support for UYVY output format, which can be
> implemented on the ISP version in the i.MX8MP but not in the one in the
> RK3399.
>
> This version of the series specifically has been tested on a Polyhex
> Debix model A with an imx219 (Raspberry Pi cam v2).
>
> [1] https://lore.kernel.org/linux-media/20220615221410.27459-2-laurent.pinchart@ideasonboard.com/
>
> Laurent Pinchart (3):
>   dt-bindings: media: rkisp1: Add i.MX8MP ISP example
>   media: rkisp1: Add and use rkisp1_has_feature() macro
>   media: rkisp1: Configure gasket on i.MX8MP
>
> Paul Elder (11):
>   dt-bindings: media: rkisp1: Add i.MX8MP ISP to compatible
>   media: rkisp1: Add match data for i.MX8MP ISP
>   media: rkisp1: Add and set registers for crop for i.MX8MP
>   media: rkisp1: Add and set registers for output size config on i.MX8MP
>   media: rkisp1: Add i.MX8MP-specific registers for MI and resizer
>   media: rkisp1: Shift DMA buffer addresses on i.MX8MP
>   media: rkisp1: Add register definitions for the test pattern generator
>   media: rkisp1: Fix RSZ_CTRL bits for i.MX8MP
>   media: rkisp1: Support devices without self path
>   media: rkisp1: Add YC swap capability
>   media: rkisp1: Add UYVY as an output format
>

Paul / Laurent,

I noticed an unexpected behaviour on the imx8mp.

If I setup my pipeline for 640x480, it works just fine using an imx219
camera configured for SRGGB10_1X10.

However, when I try to configure the pipeline to use the same camera
at 1920x1080 (no resizing), the ISP source keeps defaulting to 640x480

Media device information
------------------------
driver          rkisp1
model           rkisp1
serial
bus info        platform:rkisp1
hw revision     0xe
driver version  6.2.0

Device topology
- entity 1: rkisp1_isp (4 pads, 4 links)
            type V4L2 subdev subtype Unknown flags 0
            device node name /dev/v4l-subdev0
pad0: Sink
[fmt:SRGGB10_1X10/1920x1080 field:none colorspace:raw xfer:none
ycbcr:601 quantization:full-range
crop.bounds:(0,0)/1920x1080
crop:(0,0)/640x480]
<- "csis-32e40000.csi":1 [ENABLED]
pad1: Sink
[fmt:unknown/0x0 field:none]
<- "rkisp1_params":0 [ENABLED,IMMUTABLE]
pad2: Source
[fmt:YUYV8_2X8/640x480 field:none colorspace:raw xfer:none ycbcr:601
quantization:lim-range
crop.bounds:(0,0)/640x480
crop:(0,0)/640x480]
-> "rkisp1_resizer_mainpath":0 [ENABLED]
pad3: Source
[fmt:unknown/0x0 field:none]
-> "rkisp1_stats":0 [ENABLED,IMMUTABLE]

- entity 6: rkisp1_resizer_mainpath (2 pads, 2 links)
            type V4L2 subdev subtype Unknown flags 0
            device node name /dev/v4l-subdev1
pad0: Sink
[fmt:YUYV8_2X8/1920x1080 field:none colorspace:srgb xfer:srgb
ycbcr:601 quantization:lim-range
crop.bounds:(0,0)/1920x1080
crop:(0,0)/640x480]
<- "rkisp1_isp":2 [ENABLED]
pad1: Source
[fmt:YUYV8_2X8/1920x1080 field:none colorspace:srgb xfer:srgb
ycbcr:601 quantization:lim-range]
-> "rkisp1_mainpath":0 [ENABLED,IMMUTABLE]

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

- entity 13: rkisp1_stats (1 pad, 1 link)
             type Node subtype V4L flags 0
             device node name /dev/video1
pad0: Sink
<- "rkisp1_isp":3 [ENABLED,IMMUTABLE]

- entity 17: rkisp1_params (1 pad, 1 link)
             type Node subtype V4L flags 0
             device node name /dev/video2
pad0: Source
-> "rkisp1_isp":1 [ENABLED,IMMUTABLE]

- entity 29: csis-32e40000.csi (2 pads, 2 links)
             type V4L2 subdev subtype Unknown flags 0
             device node name /dev/v4l-subdev2
pad0: Sink
[fmt:SRGGB10_1X10/1920x1080 field:none colorspace:srgb xfer:srgb
ycbcr:601 quantization:full-range]
<- "imx219 1-0010":0 [ENABLED]
pad1: Source
[fmt:SRGGB10_1X10/1920x1080 field:none colorspace:srgb xfer:srgb
ycbcr:601 quantization:full-range]
-> "rkisp1_isp":0 [ENABLED]

- entity 34: imx219 1-0010 (1 pad, 1 link)
             type V4L2 subdev subtype Sensor flags 0
             device node name /dev/v4l-subdev3
pad0: Source
[fmt:SRGGB10_1X10/1920x1080 field:none colorspace:srgb xfer:srgb
ycbcr:601 quantization:full-range
crop.bounds:(8,8)/3280x2464
crop:(688,700)/1920x1080]
-> "csis-32e40000.csi":0 [ENABLED]

It's at this point that everything except the ISP source is 1920x1080.

When I try to set the ISP sink to 1080, it ends up being 640x480 and
the resizer sink is also changed to 640x480

root@beacon-imx8mp-kit:~# media-ctl -v -V "'rkisp1_isp':2
[fmt:YUYV8_2X8/1920x1080 field:none]"
Opening media device /dev/media0
Enumerating entities
looking up device: 81:3
looking up device: 81:4
looking up device: 81:0
looking up device: 81:1
looking up device: 81:2
looking up device: 81:5
looking up device: 81:6
Found 7 entities
Enumerating pads and links
Setting up format YUYV8_2X8 1920x1080 on pad rkisp1_isp/2
Format set: YUYV8_2X8 640x480
Setting up format YUYV8_2X8 640x480 on pad rkisp1_resizer_mainpath/0
Format set: YUYV8_2X8 640x480


It's my understanding that the ISP should be able to handle 1920x1080,
and the resizer sink should match the ISP source.

With the pipeline improperly setup, the capture fails.

adam


>  .../bindings/media/rockchip-isp1.yaml         |  79 ++++++++++-
>  .../platform/rockchip/rkisp1/rkisp1-capture.c | 102 +++++++++++---
>  .../platform/rockchip/rkisp1/rkisp1-common.h  |  32 +++++
>  .../platform/rockchip/rkisp1/rkisp1-debug.c   |  14 +-
>  .../platform/rockchip/rkisp1/rkisp1-dev.c     |  67 +++++++--
>  .../platform/rockchip/rkisp1/rkisp1-isp.c     | 128 +++++++++++++++++-
>  .../platform/rockchip/rkisp1/rkisp1-regs.h    |  90 ++++++++++++
>  .../platform/rockchip/rkisp1/rkisp1-resizer.c |  35 ++++-
>  include/uapi/linux/rkisp1-config.h            |   2 +
>  9 files changed, 509 insertions(+), 40 deletions(-)
>
> --
> 2.35.1
>

Re: [PATCH v3 00/14] media: rkisp1: Add support for i.MX8MP

[Jacopo Mondi]

Hi Adam
   sorry to jump up without being involved in the conversation

On Sat, Feb 18, 2023 at 10:14:08AM -0600, Adam Ford wrote:
> On Wed, Feb 15, 2023 at 5:55 PM Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> >
> > Hi Adam,
> >
> > On Wed, Feb 15, 2023 at 07:57:53AM -0600, Adam Ford wrote:
> > > On Fri, Nov 18, 2022 at 3:44 AM Paul Elder wrote:
> > >
> > > > This series depends on v3 of "dt-bindings: media: Add macros for video
> > > > interface bus types" [1].
> > > >
> > > > This series extends the rkisp1 driver to support the ISP found in the
> > > > NXP i.MX8MP SoC.
> > >
> > > I'm going to spend some time testing this over the weekend.  Is there a V4
> > > pending, or should I just test whatever is in Laurent's repo?
> >
> > I've updated all the v6.2-based branches on
> > https://gitlab.com/ideasonboard/nxp/linux.git (and renamed them to
> > v6.2/*). Beside a rebase, the v6.2/isp branch contains (at the bottom) 6
> > additional patches that I've previously posted to the linux-media
> > mailing list (feel free to review them ;-)).
>
> I grabbed your v6.2 series, and applied some updates to enable an
> imx219 camera and routed it through the ISP and configured the camera
> to SRGGB10_1X10/640x480 and had the ISP convert to YUYV8_2X8/640x480
> and it captured just fine.
>
> With that, I think you can add
>
> Tested-by: Adam Ford <aford173@gmail.com> #imx8mp-beacon
>
> I haven't experimented with the resizer yet,but  I did have some
> questions on the AWB.  The AWB appears to be available on the 8MP per
> the TRM, and  I see reference to AWB in the driver, but when I query
> the subdev via yavta, I didn't see anything obvious on how to enable
> it.  My pipeline (attached) shows klisp1_params as video2 and
> rkisp1_stats as video1.  I attempted to query both without much
> success.

As you might be aware there's no magic button to "turn AWB on". The
ISP enables the implementation of AWB algorithms that consumes the
statistics the ISP produces on the raw images it is fed with and
allows to program the color gains to realize colors balancing.

The implementation of such algorithms doesn't live in the driver but
rather in a separate component usually running in user space. With
libcamera we're creating a userspace camera stack where it is possible
to implement such algorithms, and the i.MX8MP is fairly well supported
by the "rkisp1" component.
https://git.libcamera.org/libcamera/libcamera.git/tree/src/libcamera/pipeline/rkisp1
https://git.libcamera.org/libcamera/libcamera.git/tree/src/ipa/rkisp1/algorithms

There's probably one single patch still out of tree in libcamera to flip
the switch and enable i.MX8MP support through the RkISP1 component.

If you're willing to give it a spin let me know and I can try
support you in testing it.


>
> root@beacon-imx8mp-kit:~# yavta -l /dev/video2
> Device /dev/video2 opened.
> Device `rkisp1_params' on `platform:rkisp1' (driver 'rkisp1') supports
> meta-data, output, without mplanes.
> unable to query control 0xc0000000: Inappropriate ioctl for device (25).
> Meta-data format: RK1P (50314b52) buffer size 3048
> root@beacon-imx8mp-kit:~# yavta -l /dev/video1
> Device /dev/video1 opened.
> Device `rkisp1_stats' on `platform:rkisp1' (driver 'rkisp1') supports
> meta-data, capture, without mplanes.
> unable to query control 0xc0000000: Inappropriate ioctl for device (25).
> Meta-data format: RK1S (53314b52) buffer size 260
> root@beacon-imx8mp-kit:~#
>
> Is there documentation somewhere on where to test the AWB?  This is of
> particular interest to me, because the RGGB format of the camera comes
> across with a green tint.  I am able to remove this green-ness on a
> different platform using some AWB on the ARM, but I'd rather do it in
> hardware if possible.
>
> Thanks
>
> adam
>
>
> >
> > My only concern with this series is with patch "media: rkisp1: Add match
> > data for i.MX8MP ISP", and in particular with the following hunk:
> >
> >  enum rkisp1_cif_isp_version {
> >         RKISP1_V10 = 10,
> >         RKISP1_V11,
> >         RKISP1_V12,
> >         RKISP1_V13,
> > +       IMX8MP_V10,
> >  };
> >
> > It's not a very nice versioning scheme :-S I'll see if I can find
> > something better, but regardless of that, I'll post v4 with the goal of
> > merging it in v6.4.
> >
> > > I have an IMX219 camera with 4-lane support and an i.MX8M Plus kit from
> > > Beacon, and I want to test the RGGB bayer conversion to see how well it
> > > works.
> > >
> > > > The ISP IP cores in the Rockchip RK3399 (known as the "Rockchip ISP1")
> > > > and in the NXP i.MX8MP have the same origin, and have slightly diverged
> > > > over time as they are now independently developed (afaik) by Rockchip
> > > > and VeriSilicon. The latter is marketed under the name "ISP8000Nano",
> > > > and is close enough to the RK3399 ISP that it can easily be supported by
> > > > the same driver.
> > >
> > > Is there a reason the driver cannot be renamed to a more generic name than
> > > rkisp1 if the Rockchip and VeriSilicon had similar origins?  Having the
> > > name Rockchip referenced from an NXP i.MX8M Plus seems odd to me.
> >
> > The common roots of the IP core predate both Rockchip and VeriSilicon.
> > Those two implementations have now diverged (as with all forks), so
> > either name would be wrong in some cases :-S
> >
> > > > The last two patches add support for UYVY output format, which can be
> > > > implemented on the ISP version in the i.MX8MP but not in the one in the
> > > > RK3399.
> > > >
> > > > This version of the series specifically has been tested on a Polyhex
> > > > Debix model A with an imx219 (Raspberry Pi cam v2).
> > > >
> > > > [1] https://lore.kernel.org/linux-media/20220615221410.27459-2-laurent.pinchart@ideasonboard.com/
> > > >
> > > > Laurent Pinchart (3):
> > > >   dt-bindings: media: rkisp1: Add i.MX8MP ISP example
> > > >   media: rkisp1: Add and use rkisp1_has_feature() macro
> > > >   media: rkisp1: Configure gasket on i.MX8MP
> > > >
> > > > Paul Elder (11):
> > > >   dt-bindings: media: rkisp1: Add i.MX8MP ISP to compatible
> > > >   media: rkisp1: Add match data for i.MX8MP ISP
> > > >   media: rkisp1: Add and set registers for crop for i.MX8MP
> > > >   media: rkisp1: Add and set registers for output size config on i.MX8MP
> > > >   media: rkisp1: Add i.MX8MP-specific registers for MI and resizer
> > > >   media: rkisp1: Shift DMA buffer addresses on i.MX8MP
> > > >   media: rkisp1: Add register definitions for the test pattern generator
> > > >   media: rkisp1: Fix RSZ_CTRL bits for i.MX8MP
> > > >   media: rkisp1: Support devices without self path
> > > >   media: rkisp1: Add YC swap capability
> > > >   media: rkisp1: Add UYVY as an output format
> > > >
> > > >  .../bindings/media/rockchip-isp1.yaml         |  79 ++++++++++-
> > > >  .../platform/rockchip/rkisp1/rkisp1-capture.c | 102 +++++++++++---
> > > >  .../platform/rockchip/rkisp1/rkisp1-common.h  |  32 +++++
> > > >  .../platform/rockchip/rkisp1/rkisp1-debug.c   |  14 +-
> > > >  .../platform/rockchip/rkisp1/rkisp1-dev.c     |  67 +++++++--
> > > >  .../platform/rockchip/rkisp1/rkisp1-isp.c     | 128 +++++++++++++++++-
> > > >  .../platform/rockchip/rkisp1/rkisp1-regs.h    |  90 ++++++++++++
> > > >  .../platform/rockchip/rkisp1/rkisp1-resizer.c |  35 ++++-
> > > >  include/uapi/linux/rkisp1-config.h            |   2 +
> > > >  9 files changed, 509 insertions(+), 40 deletions(-)
> >
> > --
> > Regards,
> >
> > Laurent Pinchart

Re: [PATCH v3 00/14] media: rkisp1: Add support for i.MX8MP

[Jacopo Mondi]

Hi Adam

On Wed, Feb 22, 2023 at 05:39:30PM -0600, Adam Ford wrote:
> On Fri, Nov 18, 2022 at 3:44 AM Paul Elder <paul.elder@ideasonboard.com> wrote:
> >
> > This series depends on v3 of "dt-bindings: media: Add macros for video
> > interface bus types" [1].
> >
> > This series extends the rkisp1 driver to support the ISP found in the
> > NXP i.MX8MP SoC.
> >
> > The ISP IP cores in the Rockchip RK3399 (known as the "Rockchip ISP1")
> > and in the NXP i.MX8MP have the same origin, and have slightly diverged
> > over time as they are now independently developed (afaik) by Rockchip
> > and VeriSilicon. The latter is marketed under the name "ISP8000Nano",
> > and is close enough to the RK3399 ISP that it can easily be supported by
> > the same driver.
> >
> > The last two patches add support for UYVY output format, which can be
> > implemented on the ISP version in the i.MX8MP but not in the one in the
> > RK3399.
> >
> > This version of the series specifically has been tested on a Polyhex
> > Debix model A with an imx219 (Raspberry Pi cam v2).
> >
> > [1] https://lore.kernel.org/linux-media/20220615221410.27459-2-laurent.pinchart@ideasonboard.com/
> >
> > Laurent Pinchart (3):
> >   dt-bindings: media: rkisp1: Add i.MX8MP ISP example
> >   media: rkisp1: Add and use rkisp1_has_feature() macro
> >   media: rkisp1: Configure gasket on i.MX8MP
> >
> > Paul Elder (11):
> >   dt-bindings: media: rkisp1: Add i.MX8MP ISP to compatible
> >   media: rkisp1: Add match data for i.MX8MP ISP
> >   media: rkisp1: Add and set registers for crop for i.MX8MP
> >   media: rkisp1: Add and set registers for output size config on i.MX8MP
> >   media: rkisp1: Add i.MX8MP-specific registers for MI and resizer
> >   media: rkisp1: Shift DMA buffer addresses on i.MX8MP
> >   media: rkisp1: Add register definitions for the test pattern generator
> >   media: rkisp1: Fix RSZ_CTRL bits for i.MX8MP
> >   media: rkisp1: Support devices without self path
> >   media: rkisp1: Add YC swap capability
> >   media: rkisp1: Add UYVY as an output format
> >
>
> Paul / Laurent,
>
> I noticed an unexpected behaviour on the imx8mp.
>
> If I setup my pipeline for 640x480, it works just fine using an imx219
> camera configured for SRGGB10_1X10.
>
> However, when I try to configure the pipeline to use the same camera
> at 1920x1080 (no resizing), the ISP source keeps defaulting to 640x480
>
> Media device information
> ------------------------
> driver          rkisp1
> model           rkisp1
> serial
> bus info        platform:rkisp1
> hw revision     0xe
> driver version  6.2.0
>
> Device topology
> - entity 1: rkisp1_isp (4 pads, 4 links)
>             type V4L2 subdev subtype Unknown flags 0
>             device node name /dev/v4l-subdev0
> pad0: Sink
> [fmt:SRGGB10_1X10/1920x1080 field:none colorspace:raw xfer:none
> ycbcr:601 quantization:full-range
> crop.bounds:(0,0)/1920x1080
> crop:(0,0)/640x480]
> <- "csis-32e40000.csi":1 [ENABLED]
> pad1: Sink
> [fmt:unknown/0x0 field:none]
> <- "rkisp1_params":0 [ENABLED,IMMUTABLE]
> pad2: Source
> [fmt:YUYV8_2X8/640x480 field:none colorspace:raw xfer:none ycbcr:601
> quantization:lim-range
> crop.bounds:(0,0)/640x480
> crop:(0,0)/640x480]
> -> "rkisp1_resizer_mainpath":0 [ENABLED]
> pad3: Source
> [fmt:unknown/0x0 field:none]
> -> "rkisp1_stats":0 [ENABLED,IMMUTABLE]
>
> - entity 6: rkisp1_resizer_mainpath (2 pads, 2 links)
>             type V4L2 subdev subtype Unknown flags 0
>             device node name /dev/v4l-subdev1
> pad0: Sink
> [fmt:YUYV8_2X8/1920x1080 field:none colorspace:srgb xfer:srgb
> ycbcr:601 quantization:lim-range
> crop.bounds:(0,0)/1920x1080
> crop:(0,0)/640x480]
> <- "rkisp1_isp":2 [ENABLED]
> pad1: Source
> [fmt:YUYV8_2X8/1920x1080 field:none colorspace:srgb xfer:srgb
> ycbcr:601 quantization:lim-range]
> -> "rkisp1_mainpath":0 [ENABLED,IMMUTABLE]
>
> - entity 9: rkisp1_mainpath (1 pad, 1 link)
>             type Node subtype V4L flags 0
>             device node name /dev/video0
> pad0: Sink
> <- "rkisp1_resizer_mainpath":1 [ENABLED,IMMUTABLE]
>
> - entity 13: rkisp1_stats (1 pad, 1 link)
>              type Node subtype V4L flags 0
>              device node name /dev/video1
> pad0: Sink
> <- "rkisp1_isp":3 [ENABLED,IMMUTABLE]
>
> - entity 17: rkisp1_params (1 pad, 1 link)
>              type Node subtype V4L flags 0
>              device node name /dev/video2
> pad0: Source
> -> "rkisp1_isp":1 [ENABLED,IMMUTABLE]
>
> - entity 29: csis-32e40000.csi (2 pads, 2 links)
>              type V4L2 subdev subtype Unknown flags 0
>              device node name /dev/v4l-subdev2
> pad0: Sink
> [fmt:SRGGB10_1X10/1920x1080 field:none colorspace:srgb xfer:srgb
> ycbcr:601 quantization:full-range]
> <- "imx219 1-0010":0 [ENABLED]
> pad1: Source
> [fmt:SRGGB10_1X10/1920x1080 field:none colorspace:srgb xfer:srgb
> ycbcr:601 quantization:full-range]
> -> "rkisp1_isp":0 [ENABLED]
>
> - entity 34: imx219 1-0010 (1 pad, 1 link)
>              type V4L2 subdev subtype Sensor flags 0
>              device node name /dev/v4l-subdev3
> pad0: Source
> [fmt:SRGGB10_1X10/1920x1080 field:none colorspace:srgb xfer:srgb
> ycbcr:601 quantization:full-range
> crop.bounds:(8,8)/3280x2464
> crop:(688,700)/1920x1080]
> -> "csis-32e40000.csi":0 [ENABLED]

Not sure if it's my email reader but I'm sorry I can't read this.
Could you use an online paste service next time if lines are long and
gets wrapped in this way ?

>
> It's at this point that everything except the ISP source is 1920x1080.
>
> When I try to set the ISP sink to 1080, it ends up being 640x480 and
> the resizer sink is also changed to 640x480
>
> root@beacon-imx8mp-kit:~# media-ctl -v -V "'rkisp1_isp':2
> [fmt:YUYV8_2X8/1920x1080 field:none]"
> Opening media device /dev/media0
> Enumerating entities
> looking up device: 81:3
> looking up device: 81:4
> looking up device: 81:0
> looking up device: 81:1
> looking up device: 81:2
> looking up device: 81:5
> looking up device: 81:6
> Found 7 entities
> Enumerating pads and links
> Setting up format YUYV8_2X8 1920x1080 on pad rkisp1_isp/2
> Format set: YUYV8_2X8 640x480
> Setting up format YUYV8_2X8 640x480 on pad rkisp1_resizer_mainpath/0
> Format set: YUYV8_2X8 640x480
>

I'm on https://gitlab.com/ideasonboard/nxp/linux/-/tree/v6.2/merge
with an additional patch to enable the ISP on the board I'm testing
with

I've tested capturing YUYV 1920x1080 with the following script

------------------------------------------------------------------------------
#!/bin/bash -x

media-ctl -r

# Link entities
media-ctl -l '"ar0521 1-0036":0 -> "csis-32e40000.csi":0'[1]
media-ctl -l '"csis-32e40000.csi":1 -> "rkisp1_isp":0'[1]
media-ctl -l '"rkisp1_isp":2 -> "rkisp1_resizer_mainpath":0'[1]

# Setup format
media-ctl -v -V '"ar0521 1-0036":0 [fmt:SGRBG8_1X8/2592x1944]'
media-ctl -v -V '"csis-32e40000.csi":1 [fmt:SGRBG8_1X8/2592x1944]'
media-ctl -v -V '"rkisp1_isp":2 [fmt:YUYV8_2X8/2592x1944]'
media-ctl  -v -V '"rkisp1_resizer_mainpath":1 [fmt:YUYV8_2X8/1920x1080]'

yavta -f YUYV -s 1920x1080 -c10 /dev/video0 --file=/tmp/frame-#.bin
------------------------------------------------------------------------------

You will have to adjust it for imx219

Unless you tweak the sensor's exposure and gains manually do not
expect nice images. You would need an auto-exposure and gain routine
like the one implemented in libcamera.

>
> It's my understanding that the ISP should be able to handle 1920x1080,
> and the resizer sink should match the ISP source.
>
> With the pipeline improperly setup, the capture fails.
>
> adam
>
>
> >  .../bindings/media/rockchip-isp1.yaml         |  79 ++++++++++-
> >  .../platform/rockchip/rkisp1/rkisp1-capture.c | 102 +++++++++++---
> >  .../platform/rockchip/rkisp1/rkisp1-common.h  |  32 +++++
> >  .../platform/rockchip/rkisp1/rkisp1-debug.c   |  14 +-
> >  .../platform/rockchip/rkisp1/rkisp1-dev.c     |  67 +++++++--
> >  .../platform/rockchip/rkisp1/rkisp1-isp.c     | 128 +++++++++++++++++-
> >  .../platform/rockchip/rkisp1/rkisp1-regs.h    |  90 ++++++++++++
> >  .../platform/rockchip/rkisp1/rkisp1-resizer.c |  35 ++++-
> >  include/uapi/linux/rkisp1-config.h            |   2 +
> >  9 files changed, 509 insertions(+), 40 deletions(-)
> >
> > --
> > 2.35.1
> >

Re: [PATCH v3 00/14] media: rkisp1: Add support for i.MX8MP

[Laurent Pinchart]

Hi Adam,

On Wed, Feb 22, 2023 at 05:39:30PM -0600, Adam Ford wrote:
> On Fri, Nov 18, 2022 at 3:44 AM Paul Elder wrote:
> >
> > This series depends on v3 of "dt-bindings: media: Add macros for video
> > interface bus types" [1].
> >
> > This series extends the rkisp1 driver to support the ISP found in the
> > NXP i.MX8MP SoC.
> >
> > The ISP IP cores in the Rockchip RK3399 (known as the "Rockchip ISP1")
> > and in the NXP i.MX8MP have the same origin, and have slightly diverged
> > over time as they are now independently developed (afaik) by Rockchip
> > and VeriSilicon. The latter is marketed under the name "ISP8000Nano",
> > and is close enough to the RK3399 ISP that it can easily be supported by
> > the same driver.
> >
> > The last two patches add support for UYVY output format, which can be
> > implemented on the ISP version in the i.MX8MP but not in the one in the
> > RK3399.
> >
> > This version of the series specifically has been tested on a Polyhex
> > Debix model A with an imx219 (Raspberry Pi cam v2).
> >
> > [1] https://lore.kernel.org/linux-media/20220615221410.27459-2-laurent.pinchart@ideasonboard.com/
> >
> > Laurent Pinchart (3):
> >   dt-bindings: media: rkisp1: Add i.MX8MP ISP example
> >   media: rkisp1: Add and use rkisp1_has_feature() macro
> >   media: rkisp1: Configure gasket on i.MX8MP
> >
> > Paul Elder (11):
> >   dt-bindings: media: rkisp1: Add i.MX8MP ISP to compatible
> >   media: rkisp1: Add match data for i.MX8MP ISP
> >   media: rkisp1: Add and set registers for crop for i.MX8MP
> >   media: rkisp1: Add and set registers for output size config on i.MX8MP
> >   media: rkisp1: Add i.MX8MP-specific registers for MI and resizer
> >   media: rkisp1: Shift DMA buffer addresses on i.MX8MP
> >   media: rkisp1: Add register definitions for the test pattern generator
> >   media: rkisp1: Fix RSZ_CTRL bits for i.MX8MP
> >   media: rkisp1: Support devices without self path
> >   media: rkisp1: Add YC swap capability
> >   media: rkisp1: Add UYVY as an output format
> 
> Paul / Laurent,
> 
> I noticed an unexpected behaviour on the imx8mp.
> 
> If I setup my pipeline for 640x480, it works just fine using an imx219
> camera configured for SRGGB10_1X10.
> 
> However, when I try to configure the pipeline to use the same camera
> at 1920x1080 (no resizing), the ISP source keeps defaulting to 640x480
> 
> Media device information
> ------------------------
> driver          rkisp1
> model           rkisp1
> serial
> bus info        platform:rkisp1
> hw revision     0xe
> driver version  6.2.0
> 
> Device topology
> - entity 1: rkisp1_isp (4 pads, 4 links)
>             type V4L2 subdev subtype Unknown flags 0
>             device node name /dev/v4l-subdev0
> pad0: Sink [fmt:SRGGB10_1X10/1920x1080 field:none colorspace:raw xfer:none ycbcr:601 quantization:full-range crop.bounds:(0,0)/1920x1080 crop:(0,0)/640x480]

You're cropping the image to 640x480 here. You need to set the crop
rectangle to 1920x1080.

As Jacopo mentioned, I wouldn't recommend exercising the ISP directly.
Not only do you need to setup the pipeline, but you would also need to
implement all the imaging algorithms in userspace. libcamera will do all
this for you.

> <- "csis-32e40000.csi":1 [ENABLED]
> pad1: Sink [fmt:unknown/0x0 field:none]
> <- "rkisp1_params":0 [ENABLED,IMMUTABLE]
> pad2: Source [fmt:YUYV8_2X8/640x480 field:none colorspace:raw xfer:none ycbcr:601 quantization:lim-range crop.bounds:(0,0)/640x480 crop:(0,0)/640x480]
> -> "rkisp1_resizer_mainpath":0 [ENABLED]
> pad3: Source [fmt:unknown/0x0 field:none]
> -> "rkisp1_stats":0 [ENABLED,IMMUTABLE]
> 
> - entity 6: rkisp1_resizer_mainpath (2 pads, 2 links)
>             type V4L2 subdev subtype Unknown flags 0
>             device node name /dev/v4l-subdev1
> pad0: Sink [fmt:YUYV8_2X8/1920x1080 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:lim-range crop.bounds:(0,0)/1920x1080 crop:(0,0)/640x480]
> <- "rkisp1_isp":2 [ENABLED]
> pad1: Source [fmt:YUYV8_2X8/1920x1080 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:lim-range]
> -> "rkisp1_mainpath":0 [ENABLED,IMMUTABLE]
> 
> - entity 9: rkisp1_mainpath (1 pad, 1 link)
>             type Node subtype V4L flags 0
>             device node name /dev/video0
> pad0: Sink
> <- "rkisp1_resizer_mainpath":1 [ENABLED,IMMUTABLE]
> 
> - entity 13: rkisp1_stats (1 pad, 1 link)
>              type Node subtype V4L flags 0
>              device node name /dev/video1
> pad0: Sink
> <- "rkisp1_isp":3 [ENABLED,IMMUTABLE]
> 
> - entity 17: rkisp1_params (1 pad, 1 link)
>              type Node subtype V4L flags 0
>              device node name /dev/video2
> pad0: Source
> -> "rkisp1_isp":1 [ENABLED,IMMUTABLE]
> 
> - entity 29: csis-32e40000.csi (2 pads, 2 links)
>              type V4L2 subdev subtype Unknown flags 0
>              device node name /dev/v4l-subdev2
> pad0: Sink [fmt:SRGGB10_1X10/1920x1080 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:full-range]
> <- "imx219 1-0010":0 [ENABLED]
> pad1: Source [fmt:SRGGB10_1X10/1920x1080 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:full-range]
> -> "rkisp1_isp":0 [ENABLED]
> 
> - entity 34: imx219 1-0010 (1 pad, 1 link)
>              type V4L2 subdev subtype Sensor flags 0
>              device node name /dev/v4l-subdev3
> pad0: Source [fmt:SRGGB10_1X10/1920x1080 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:full-range crop.bounds:(8,8)/3280x2464 crop:(688,700)/1920x1080]
> -> "csis-32e40000.csi":0 [ENABLED]
> 
> It's at this point that everything except the ISP source is 1920x1080.
> 
> When I try to set the ISP sink to 1080, it ends up being 640x480 and
> the resizer sink is also changed to 640x480
> 
> root@beacon-imx8mp-kit:~# media-ctl -v -V "'rkisp1_isp':2
> [fmt:YUYV8_2X8/1920x1080 field:none]"
> Opening media device /dev/media0
> Enumerating entities
> looking up device: 81:3
> looking up device: 81:4
> looking up device: 81:0
> looking up device: 81:1
> looking up device: 81:2
> looking up device: 81:5
> looking up device: 81:6
> Found 7 entities
> Enumerating pads and links
> Setting up format YUYV8_2X8 1920x1080 on pad rkisp1_isp/2
> Format set: YUYV8_2X8 640x480
> Setting up format YUYV8_2X8 640x480 on pad rkisp1_resizer_mainpath/0
> Format set: YUYV8_2X8 640x480
> 
> 
> It's my understanding that the ISP should be able to handle 1920x1080,
> and the resizer sink should match the ISP source.
> 
> With the pipeline improperly setup, the capture fails.
> 
> >  .../bindings/media/rockchip-isp1.yaml         |  79 ++++++++++-
> >  .../platform/rockchip/rkisp1/rkisp1-capture.c | 102 +++++++++++---
> >  .../platform/rockchip/rkisp1/rkisp1-common.h  |  32 +++++
> >  .../platform/rockchip/rkisp1/rkisp1-debug.c   |  14 +-
> >  .../platform/rockchip/rkisp1/rkisp1-dev.c     |  67 +++++++--
> >  .../platform/rockchip/rkisp1/rkisp1-isp.c     | 128 +++++++++++++++++-
> >  .../platform/rockchip/rkisp1/rkisp1-regs.h    |  90 ++++++++++++
> >  .../platform/rockchip/rkisp1/rkisp1-resizer.c |  35 ++++-
> >  include/uapi/linux/rkisp1-config.h            |   2 +
> >  9 files changed, 509 insertions(+), 40 deletions(-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 00/14] media: rkisp1: Add support for i.MX8MP

[Adam Ford]

On Thu, Feb 23, 2023 at 8:26 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Adam,
>
> On Wed, Feb 22, 2023 at 05:39:30PM -0600, Adam Ford wrote:
> > On Fri, Nov 18, 2022 at 3:44 AM Paul Elder wrote:
> > >
> > > This series depends on v3 of "dt-bindings: media: Add macros for video
> > > interface bus types" [1].
> > >
> > > This series extends the rkisp1 driver to support the ISP found in the
> > > NXP i.MX8MP SoC.
> > >
> > > The ISP IP cores in the Rockchip RK3399 (known as the "Rockchip ISP1")
> > > and in the NXP i.MX8MP have the same origin, and have slightly diverged
> > > over time as they are now independently developed (afaik) by Rockchip
> > > and VeriSilicon. The latter is marketed under the name "ISP8000Nano",
> > > and is close enough to the RK3399 ISP that it can easily be supported by
> > > the same driver.
> > >
> > > The last two patches add support for UYVY output format, which can be
> > > implemented on the ISP version in the i.MX8MP but not in the one in the
> > > RK3399.
> > >
> > > This version of the series specifically has been tested on a Polyhex
> > > Debix model A with an imx219 (Raspberry Pi cam v2).
> > >
> > > [1] https://lore.kernel.org/linux-media/20220615221410.27459-2-laurent.pinchart@ideasonboard.com/
> > >
> > > Laurent Pinchart (3):
> > >   dt-bindings: media: rkisp1: Add i.MX8MP ISP example
> > >   media: rkisp1: Add and use rkisp1_has_feature() macro
> > >   media: rkisp1: Configure gasket on i.MX8MP
> > >
> > > Paul Elder (11):
> > >   dt-bindings: media: rkisp1: Add i.MX8MP ISP to compatible
> > >   media: rkisp1: Add match data for i.MX8MP ISP
> > >   media: rkisp1: Add and set registers for crop for i.MX8MP
> > >   media: rkisp1: Add and set registers for output size config on i.MX8MP
> > >   media: rkisp1: Add i.MX8MP-specific registers for MI and resizer
> > >   media: rkisp1: Shift DMA buffer addresses on i.MX8MP
> > >   media: rkisp1: Add register definitions for the test pattern generator
> > >   media: rkisp1: Fix RSZ_CTRL bits for i.MX8MP
> > >   media: rkisp1: Support devices without self path
> > >   media: rkisp1: Add YC swap capability
> > >   media: rkisp1: Add UYVY as an output format
> >
> > Paul / Laurent,
> >
> > I noticed an unexpected behaviour on the imx8mp.
> >
> > If I setup my pipeline for 640x480, it works just fine using an imx219
> > camera configured for SRGGB10_1X10.
> >
> > However, when I try to configure the pipeline to use the same camera
> > at 1920x1080 (no resizing), the ISP source keeps defaulting to 640x480
> >
> > Media device information
> > ------------------------
> > driver          rkisp1
> > model           rkisp1
> > serial
> > bus info        platform:rkisp1
> > hw revision     0xe
> > driver version  6.2.0
> >
> > Device topology
> > - entity 1: rkisp1_isp (4 pads, 4 links)
> >             type V4L2 subdev subtype Unknown flags 0
> >             device node name /dev/v4l-subdev0
> > pad0: Sink [fmt:SRGGB10_1X10/1920x1080 field:none colorspace:raw xfer:none ycbcr:601 quantization:full-range crop.bounds:(0,0)/1920x1080 crop:(0,0)/640x480]
>
> You're cropping the image to 640x480 here. You need to set the crop
> rectangle to 1920x1080.
>
> As Jacopo mentioned, I wouldn't recommend exercising the ISP directly.
> Not only do you need to setup the pipeline, but you would also need to
> implement all the imaging algorithms in userspace. libcamera will do all
> this for you.

I'll give that a try.  My current employer has a v4l2src requirement,
but I can likely make an argument to switch to libcamera.  I didn't
catch the cropping part. Thanks for that.

adam
>
> > <- "csis-32e40000.csi":1 [ENABLED]
> > pad1: Sink [fmt:unknown/0x0 field:none]
> > <- "rkisp1_params":0 [ENABLED,IMMUTABLE]
> > pad2: Source [fmt:YUYV8_2X8/640x480 field:none colorspace:raw xfer:none ycbcr:601 quantization:lim-range crop.bounds:(0,0)/640x480 crop:(0,0)/640x480]
> > -> "rkisp1_resizer_mainpath":0 [ENABLED]
> > pad3: Source [fmt:unknown/0x0 field:none]
> > -> "rkisp1_stats":0 [ENABLED,IMMUTABLE]
> >
> > - entity 6: rkisp1_resizer_mainpath (2 pads, 2 links)
> >             type V4L2 subdev subtype Unknown flags 0
> >             device node name /dev/v4l-subdev1
> > pad0: Sink [fmt:YUYV8_2X8/1920x1080 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:lim-range crop.bounds:(0,0)/1920x1080 crop:(0,0)/640x480]
> > <- "rkisp1_isp":2 [ENABLED]
> > pad1: Source [fmt:YUYV8_2X8/1920x1080 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:lim-range]
> > -> "rkisp1_mainpath":0 [ENABLED,IMMUTABLE]
> >
> > - entity 9: rkisp1_mainpath (1 pad, 1 link)
> >             type Node subtype V4L flags 0
> >             device node name /dev/video0
> > pad0: Sink
> > <- "rkisp1_resizer_mainpath":1 [ENABLED,IMMUTABLE]
> >
> > - entity 13: rkisp1_stats (1 pad, 1 link)
> >              type Node subtype V4L flags 0
> >              device node name /dev/video1
> > pad0: Sink
> > <- "rkisp1_isp":3 [ENABLED,IMMUTABLE]
> >
> > - entity 17: rkisp1_params (1 pad, 1 link)
> >              type Node subtype V4L flags 0
> >              device node name /dev/video2
> > pad0: Source
> > -> "rkisp1_isp":1 [ENABLED,IMMUTABLE]
> >
> > - entity 29: csis-32e40000.csi (2 pads, 2 links)
> >              type V4L2 subdev subtype Unknown flags 0
> >              device node name /dev/v4l-subdev2
> > pad0: Sink [fmt:SRGGB10_1X10/1920x1080 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:full-range]
> > <- "imx219 1-0010":0 [ENABLED]
> > pad1: Source [fmt:SRGGB10_1X10/1920x1080 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:full-range]
> > -> "rkisp1_isp":0 [ENABLED]
> >
> > - entity 34: imx219 1-0010 (1 pad, 1 link)
> >              type V4L2 subdev subtype Sensor flags 0
> >              device node name /dev/v4l-subdev3
> > pad0: Source [fmt:SRGGB10_1X10/1920x1080 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:full-range crop.bounds:(8,8)/3280x2464 crop:(688,700)/1920x1080]
> > -> "csis-32e40000.csi":0 [ENABLED]
> >
> > It's at this point that everything except the ISP source is 1920x1080.
> >
> > When I try to set the ISP sink to 1080, it ends up being 640x480 and
> > the resizer sink is also changed to 640x480
> >
> > root@beacon-imx8mp-kit:~# media-ctl -v -V "'rkisp1_isp':2
> > [fmt:YUYV8_2X8/1920x1080 field:none]"
> > Opening media device /dev/media0
> > Enumerating entities
> > looking up device: 81:3
> > looking up device: 81:4
> > looking up device: 81:0
> > looking up device: 81:1
> > looking up device: 81:2
> > looking up device: 81:5
> > looking up device: 81:6
> > Found 7 entities
> > Enumerating pads and links
> > Setting up format YUYV8_2X8 1920x1080 on pad rkisp1_isp/2
> > Format set: YUYV8_2X8 640x480
> > Setting up format YUYV8_2X8 640x480 on pad rkisp1_resizer_mainpath/0
> > Format set: YUYV8_2X8 640x480
> >
> >
> > It's my understanding that the ISP should be able to handle 1920x1080,
> > and the resizer sink should match the ISP source.
> >
> > With the pipeline improperly setup, the capture fails.
> >
> > >  .../bindings/media/rockchip-isp1.yaml         |  79 ++++++++++-
> > >  .../platform/rockchip/rkisp1/rkisp1-capture.c | 102 +++++++++++---
> > >  .../platform/rockchip/rkisp1/rkisp1-common.h  |  32 +++++
> > >  .../platform/rockchip/rkisp1/rkisp1-debug.c   |  14 +-
> > >  .../platform/rockchip/rkisp1/rkisp1-dev.c     |  67 +++++++--
> > >  .../platform/rockchip/rkisp1/rkisp1-isp.c     | 128 +++++++++++++++++-
> > >  .../platform/rockchip/rkisp1/rkisp1-regs.h    |  90 ++++++++++++
> > >  .../platform/rockchip/rkisp1/rkisp1-resizer.c |  35 ++++-
> > >  include/uapi/linux/rkisp1-config.h            |   2 +
> > >  9 files changed, 509 insertions(+), 40 deletions(-)
>
> --
> Regards,
>
> Laurent Pinchart

Re: [PATCH v3 00/14] media: rkisp1: Add support for i.MX8MP

[Laurent Pinchart]

Hi Adam,

On Thu, Feb 23, 2023 at 10:10:28AM -0600, Adam Ford wrote:
> On Thu, Feb 23, 2023 at 8:26 AM Laurent Pinchart wrote:
> > On Wed, Feb 22, 2023 at 05:39:30PM -0600, Adam Ford wrote:
> > > On Fri, Nov 18, 2022 at 3:44 AM Paul Elder wrote:
> > > >
> > > > This series depends on v3 of "dt-bindings: media: Add macros for video
> > > > interface bus types" [1].
> > > >
> > > > This series extends the rkisp1 driver to support the ISP found in the
> > > > NXP i.MX8MP SoC.
> > > >
> > > > The ISP IP cores in the Rockchip RK3399 (known as the "Rockchip ISP1")
> > > > and in the NXP i.MX8MP have the same origin, and have slightly diverged
> > > > over time as they are now independently developed (afaik) by Rockchip
> > > > and VeriSilicon. The latter is marketed under the name "ISP8000Nano",
> > > > and is close enough to the RK3399 ISP that it can easily be supported by
> > > > the same driver.
> > > >
> > > > The last two patches add support for UYVY output format, which can be
> > > > implemented on the ISP version in the i.MX8MP but not in the one in the
> > > > RK3399.
> > > >
> > > > This version of the series specifically has been tested on a Polyhex
> > > > Debix model A with an imx219 (Raspberry Pi cam v2).
> > > >
> > > > [1] https://lore.kernel.org/linux-media/20220615221410.27459-2-laurent.pinchart@ideasonboard.com/
> > > >
> > > > Laurent Pinchart (3):
> > > >   dt-bindings: media: rkisp1: Add i.MX8MP ISP example
> > > >   media: rkisp1: Add and use rkisp1_has_feature() macro
> > > >   media: rkisp1: Configure gasket on i.MX8MP
> > > >
> > > > Paul Elder (11):
> > > >   dt-bindings: media: rkisp1: Add i.MX8MP ISP to compatible
> > > >   media: rkisp1: Add match data for i.MX8MP ISP
> > > >   media: rkisp1: Add and set registers for crop for i.MX8MP
> > > >   media: rkisp1: Add and set registers for output size config on i.MX8MP
> > > >   media: rkisp1: Add i.MX8MP-specific registers for MI and resizer
> > > >   media: rkisp1: Shift DMA buffer addresses on i.MX8MP
> > > >   media: rkisp1: Add register definitions for the test pattern generator
> > > >   media: rkisp1: Fix RSZ_CTRL bits for i.MX8MP
> > > >   media: rkisp1: Support devices without self path
> > > >   media: rkisp1: Add YC swap capability
> > > >   media: rkisp1: Add UYVY as an output format
> > >
> > > Paul / Laurent,
> > >
> > > I noticed an unexpected behaviour on the imx8mp.
> > >
> > > If I setup my pipeline for 640x480, it works just fine using an imx219
> > > camera configured for SRGGB10_1X10.
> > >
> > > However, when I try to configure the pipeline to use the same camera
> > > at 1920x1080 (no resizing), the ISP source keeps defaulting to 640x480
> > >
> > > Media device information
> > > ------------------------
> > > driver          rkisp1
> > > model           rkisp1
> > > serial
> > > bus info        platform:rkisp1
> > > hw revision     0xe
> > > driver version  6.2.0
> > >
> > > Device topology
> > > - entity 1: rkisp1_isp (4 pads, 4 links)
> > >             type V4L2 subdev subtype Unknown flags 0
> > >             device node name /dev/v4l-subdev0
> > > pad0: Sink [fmt:SRGGB10_1X10/1920x1080 field:none colorspace:raw xfer:none ycbcr:601 quantization:full-range crop.bounds:(0,0)/1920x1080 crop:(0,0)/640x480]
> >
> > You're cropping the image to 640x480 here. You need to set the crop
> > rectangle to 1920x1080.
> >
> > As Jacopo mentioned, I wouldn't recommend exercising the ISP directly.
> > Not only do you need to setup the pipeline, but you would also need to
> > implement all the imaging algorithms in userspace. libcamera will do all
> > this for you.
> 
> I'll give that a try.  My current employer has a v4l2src requirement,
> but I can likely make an argument to switch to libcamera.  I didn't
> catch the cropping part. Thanks for that.

libcamera has a GStreamer source element called libcamerasrc which is
aimed to be a drop-in replacement for v4l2src. Some features may be
missing, let us know and we can discuss how to add them.

> > > <- "csis-32e40000.csi":1 [ENABLED]
> > > pad1: Sink [fmt:unknown/0x0 field:none]
> > > <- "rkisp1_params":0 [ENABLED,IMMUTABLE]
> > > pad2: Source [fmt:YUYV8_2X8/640x480 field:none colorspace:raw xfer:none ycbcr:601 quantization:lim-range crop.bounds:(0,0)/640x480 crop:(0,0)/640x480]
> > > -> "rkisp1_resizer_mainpath":0 [ENABLED]
> > > pad3: Source [fmt:unknown/0x0 field:none]
> > > -> "rkisp1_stats":0 [ENABLED,IMMUTABLE]
> > >
> > > - entity 6: rkisp1_resizer_mainpath (2 pads, 2 links)
> > >             type V4L2 subdev subtype Unknown flags 0
> > >             device node name /dev/v4l-subdev1
> > > pad0: Sink [fmt:YUYV8_2X8/1920x1080 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:lim-range crop.bounds:(0,0)/1920x1080 crop:(0,0)/640x480]
> > > <- "rkisp1_isp":2 [ENABLED]
> > > pad1: Source [fmt:YUYV8_2X8/1920x1080 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:lim-range]
> > > -> "rkisp1_mainpath":0 [ENABLED,IMMUTABLE]
> > >
> > > - entity 9: rkisp1_mainpath (1 pad, 1 link)
> > >             type Node subtype V4L flags 0
> > >             device node name /dev/video0
> > > pad0: Sink
> > > <- "rkisp1_resizer_mainpath":1 [ENABLED,IMMUTABLE]
> > >
> > > - entity 13: rkisp1_stats (1 pad, 1 link)
> > >              type Node subtype V4L flags 0
> > >              device node name /dev/video1
> > > pad0: Sink
> > > <- "rkisp1_isp":3 [ENABLED,IMMUTABLE]
> > >
> > > - entity 17: rkisp1_params (1 pad, 1 link)
> > >              type Node subtype V4L flags 0
> > >              device node name /dev/video2
> > > pad0: Source
> > > -> "rkisp1_isp":1 [ENABLED,IMMUTABLE]
> > >
> > > - entity 29: csis-32e40000.csi (2 pads, 2 links)
> > >              type V4L2 subdev subtype Unknown flags 0
> > >              device node name /dev/v4l-subdev2
> > > pad0: Sink [fmt:SRGGB10_1X10/1920x1080 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:full-range]
> > > <- "imx219 1-0010":0 [ENABLED]
> > > pad1: Source [fmt:SRGGB10_1X10/1920x1080 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:full-range]
> > > -> "rkisp1_isp":0 [ENABLED]
> > >
> > > - entity 34: imx219 1-0010 (1 pad, 1 link)
> > >              type V4L2 subdev subtype Sensor flags 0
> > >              device node name /dev/v4l-subdev3
> > > pad0: Source [fmt:SRGGB10_1X10/1920x1080 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:full-range crop.bounds:(8,8)/3280x2464 crop:(688,700)/1920x1080]
> > > -> "csis-32e40000.csi":0 [ENABLED]
> > >
> > > It's at this point that everything except the ISP source is 1920x1080.
> > >
> > > When I try to set the ISP sink to 1080, it ends up being 640x480 and
> > > the resizer sink is also changed to 640x480
> > >
> > > root@beacon-imx8mp-kit:~# media-ctl -v -V "'rkisp1_isp':2
> > > [fmt:YUYV8_2X8/1920x1080 field:none]"
> > > Opening media device /dev/media0
> > > Enumerating entities
> > > looking up device: 81:3
> > > looking up device: 81:4
> > > looking up device: 81:0
> > > looking up device: 81:1
> > > looking up device: 81:2
> > > looking up device: 81:5
> > > looking up device: 81:6
> > > Found 7 entities
> > > Enumerating pads and links
> > > Setting up format YUYV8_2X8 1920x1080 on pad rkisp1_isp/2
> > > Format set: YUYV8_2X8 640x480
> > > Setting up format YUYV8_2X8 640x480 on pad rkisp1_resizer_mainpath/0
> > > Format set: YUYV8_2X8 640x480
> > >
> > >
> > > It's my understanding that the ISP should be able to handle 1920x1080,
> > > and the resizer sink should match the ISP source.
> > >
> > > With the pipeline improperly setup, the capture fails.
> > >
> > > >  .../bindings/media/rockchip-isp1.yaml         |  79 ++++++++++-
> > > >  .../platform/rockchip/rkisp1/rkisp1-capture.c | 102 +++++++++++---
> > > >  .../platform/rockchip/rkisp1/rkisp1-common.h  |  32 +++++
> > > >  .../platform/rockchip/rkisp1/rkisp1-debug.c   |  14 +-
> > > >  .../platform/rockchip/rkisp1/rkisp1-dev.c     |  67 +++++++--
> > > >  .../platform/rockchip/rkisp1/rkisp1-isp.c     | 128 +++++++++++++++++-
> > > >  .../platform/rockchip/rkisp1/rkisp1-regs.h    |  90 ++++++++++++
> > > >  .../platform/rockchip/rkisp1/rkisp1-resizer.c |  35 ++++-
> > > >  include/uapi/linux/rkisp1-config.h            |   2 +
> > > >  9 files changed, 509 insertions(+), 40 deletions(-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 00/14] media: rkisp1: Add support for i.MX8MP

[Nicolas Dufresne]

Hi Adam,

Le jeudi 23 février 2023 à 10:10 -0600, Adam Ford a écrit :
> On Thu, Feb 23, 2023 at 8:26 AM Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> > 
> > Hi Adam,
> > 
> > On Wed, Feb 22, 2023 at 05:39:30PM -0600, Adam Ford wrote:
> > > On Fri, Nov 18, 2022 at 3:44 AM Paul Elder wrote:
> > > > 
> > > > This series depends on v3 of "dt-bindings: media: Add macros for video
> > > > interface bus types" [1].
> > > > 
> > > > This series extends the rkisp1 driver to support the ISP found in the
> > > > NXP i.MX8MP SoC.
> > > > 
> > > > The ISP IP cores in the Rockchip RK3399 (known as the "Rockchip ISP1")
> > > > and in the NXP i.MX8MP have the same origin, and have slightly diverged
> > > > over time as they are now independently developed (afaik) by Rockchip
> > > > and VeriSilicon. The latter is marketed under the name "ISP8000Nano",
> > > > and is close enough to the RK3399 ISP that it can easily be supported by
> > > > the same driver.
> > > > 
> > > > The last two patches add support for UYVY output format, which can be
> > > > implemented on the ISP version in the i.MX8MP but not in the one in the
> > > > RK3399.
> > > > 
> > > > This version of the series specifically has been tested on a Polyhex
> > > > Debix model A with an imx219 (Raspberry Pi cam v2).
> > > > 
> > > > [1] https://lore.kernel.org/linux-media/20220615221410.27459-2-laurent.pinchart@ideasonboard.com/
> > > > 
> > > > Laurent Pinchart (3):
> > > >   dt-bindings: media: rkisp1: Add i.MX8MP ISP example
> > > >   media: rkisp1: Add and use rkisp1_has_feature() macro
> > > >   media: rkisp1: Configure gasket on i.MX8MP
> > > > 
> > > > Paul Elder (11):
> > > >   dt-bindings: media: rkisp1: Add i.MX8MP ISP to compatible
> > > >   media: rkisp1: Add match data for i.MX8MP ISP
> > > >   media: rkisp1: Add and set registers for crop for i.MX8MP
> > > >   media: rkisp1: Add and set registers for output size config on i.MX8MP
> > > >   media: rkisp1: Add i.MX8MP-specific registers for MI and resizer
> > > >   media: rkisp1: Shift DMA buffer addresses on i.MX8MP
> > > >   media: rkisp1: Add register definitions for the test pattern generator
> > > >   media: rkisp1: Fix RSZ_CTRL bits for i.MX8MP
> > > >   media: rkisp1: Support devices without self path
> > > >   media: rkisp1: Add YC swap capability
> > > >   media: rkisp1: Add UYVY as an output format
> > > 
> > > Paul / Laurent,
> > > 
> > > I noticed an unexpected behaviour on the imx8mp.
> > > 
> > > If I setup my pipeline for 640x480, it works just fine using an imx219
> > > camera configured for SRGGB10_1X10.
> > > 
> > > However, when I try to configure the pipeline to use the same camera
> > > at 1920x1080 (no resizing), the ISP source keeps defaulting to 640x480
> > > 
> > > Media device information
> > > ------------------------
> > > driver          rkisp1
> > > model           rkisp1
> > > serial
> > > bus info        platform:rkisp1
> > > hw revision     0xe
> > > driver version  6.2.0
> > > 
> > > Device topology
> > > - entity 1: rkisp1_isp (4 pads, 4 links)
> > >             type V4L2 subdev subtype Unknown flags 0
> > >             device node name /dev/v4l-subdev0
> > > pad0: Sink [fmt:SRGGB10_1X10/1920x1080 field:none colorspace:raw xfer:none ycbcr:601 quantization:full-range crop.bounds:(0,0)/1920x1080 crop:(0,0)/640x480]
> > 
> > You're cropping the image to 640x480 here. You need to set the crop
> > rectangle to 1920x1080.
> > 
> > As Jacopo mentioned, I wouldn't recommend exercising the ISP directly.
> > Not only do you need to setup the pipeline, but you would also need to
> > implement all the imaging algorithms in userspace. libcamera will do all
> > this for you.
> 
> I'll give that a try.  My current employer has a v4l2src requirement,
> but I can likely make an argument to switch to libcamera.  I didn't
> catch the cropping part. Thanks for that.

I'd hope you can transparently replace v4l2src with libcamerasrc, the plugins
currently lives inside the libcamera project. If not, I'd really like to know
why. We can work together on adding missing controls (this is something I'm
starting on soon).

regards,
Nicolas

> 
> adam
> > 
> > > <- "csis-32e40000.csi":1 [ENABLED]
> > > pad1: Sink [fmt:unknown/0x0 field:none]
> > > <- "rkisp1_params":0 [ENABLED,IMMUTABLE]
> > > pad2: Source [fmt:YUYV8_2X8/640x480 field:none colorspace:raw xfer:none ycbcr:601 quantization:lim-range crop.bounds:(0,0)/640x480 crop:(0,0)/640x480]
> > > -> "rkisp1_resizer_mainpath":0 [ENABLED]
> > > pad3: Source [fmt:unknown/0x0 field:none]
> > > -> "rkisp1_stats":0 [ENABLED,IMMUTABLE]
> > > 
> > > - entity 6: rkisp1_resizer_mainpath (2 pads, 2 links)
> > >             type V4L2 subdev subtype Unknown flags 0
> > >             device node name /dev/v4l-subdev1
> > > pad0: Sink [fmt:YUYV8_2X8/1920x1080 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:lim-range crop.bounds:(0,0)/1920x1080 crop:(0,0)/640x480]
> > > <- "rkisp1_isp":2 [ENABLED]
> > > pad1: Source [fmt:YUYV8_2X8/1920x1080 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:lim-range]
> > > -> "rkisp1_mainpath":0 [ENABLED,IMMUTABLE]
> > > 
> > > - entity 9: rkisp1_mainpath (1 pad, 1 link)
> > >             type Node subtype V4L flags 0
> > >             device node name /dev/video0
> > > pad0: Sink
> > > <- "rkisp1_resizer_mainpath":1 [ENABLED,IMMUTABLE]
> > > 
> > > - entity 13: rkisp1_stats (1 pad, 1 link)
> > >              type Node subtype V4L flags 0
> > >              device node name /dev/video1
> > > pad0: Sink
> > > <- "rkisp1_isp":3 [ENABLED,IMMUTABLE]
> > > 
> > > - entity 17: rkisp1_params (1 pad, 1 link)
> > >              type Node subtype V4L flags 0
> > >              device node name /dev/video2
> > > pad0: Source
> > > -> "rkisp1_isp":1 [ENABLED,IMMUTABLE]
> > > 
> > > - entity 29: csis-32e40000.csi (2 pads, 2 links)
> > >              type V4L2 subdev subtype Unknown flags 0
> > >              device node name /dev/v4l-subdev2
> > > pad0: Sink [fmt:SRGGB10_1X10/1920x1080 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:full-range]
> > > <- "imx219 1-0010":0 [ENABLED]
> > > pad1: Source [fmt:SRGGB10_1X10/1920x1080 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:full-range]
> > > -> "rkisp1_isp":0 [ENABLED]
> > > 
> > > - entity 34: imx219 1-0010 (1 pad, 1 link)
> > >              type V4L2 subdev subtype Sensor flags 0
> > >              device node name /dev/v4l-subdev3
> > > pad0: Source [fmt:SRGGB10_1X10/1920x1080 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:full-range crop.bounds:(8,8)/3280x2464 crop:(688,700)/1920x1080]
> > > -> "csis-32e40000.csi":0 [ENABLED]
> > > 
> > > It's at this point that everything except the ISP source is 1920x1080.
> > > 
> > > When I try to set the ISP sink to 1080, it ends up being 640x480 and
> > > the resizer sink is also changed to 640x480
> > > 
> > > root@beacon-imx8mp-kit:~# media-ctl -v -V "'rkisp1_isp':2
> > > [fmt:YUYV8_2X8/1920x1080 field:none]"
> > > Opening media device /dev/media0
> > > Enumerating entities
> > > looking up device: 81:3
> > > looking up device: 81:4
> > > looking up device: 81:0
> > > looking up device: 81:1
> > > looking up device: 81:2
> > > looking up device: 81:5
> > > looking up device: 81:6
> > > Found 7 entities
> > > Enumerating pads and links
> > > Setting up format YUYV8_2X8 1920x1080 on pad rkisp1_isp/2
> > > Format set: YUYV8_2X8 640x480
> > > Setting up format YUYV8_2X8 640x480 on pad rkisp1_resizer_mainpath/0
> > > Format set: YUYV8_2X8 640x480
> > > 
> > > 
> > > It's my understanding that the ISP should be able to handle 1920x1080,
> > > and the resizer sink should match the ISP source.
> > > 
> > > With the pipeline improperly setup, the capture fails.
> > > 
> > > >  .../bindings/media/rockchip-isp1.yaml         |  79 ++++++++++-
> > > >  .../platform/rockchip/rkisp1/rkisp1-capture.c | 102 +++++++++++---
> > > >  .../platform/rockchip/rkisp1/rkisp1-common.h  |  32 +++++
> > > >  .../platform/rockchip/rkisp1/rkisp1-debug.c   |  14 +-
> > > >  .../platform/rockchip/rkisp1/rkisp1-dev.c     |  67 +++++++--
> > > >  .../platform/rockchip/rkisp1/rkisp1-isp.c     | 128 +++++++++++++++++-
> > > >  .../platform/rockchip/rkisp1/rkisp1-regs.h    |  90 ++++++++++++
> > > >  .../platform/rockchip/rkisp1/rkisp1-resizer.c |  35 ++++-
> > > >  include/uapi/linux/rkisp1-config.h            |   2 +
> > > >  9 files changed, 509 insertions(+), 40 deletions(-)
> > 
> > --
> > Regards,
> > 
> > Laurent Pinchart

Re: [PATCH v3 00/14] media: rkisp1: Add support for i.MX8MP

[Adam Ford]

On Fri, Feb 24, 2023 at 12:24 PM Nicolas Dufresne <nicolas@ndufresne.ca> wrote:
>
> Hi Adam,
>
> Le jeudi 23 février 2023 à 10:10 -0600, Adam Ford a écrit :
> > On Thu, Feb 23, 2023 at 8:26 AM Laurent Pinchart
> > <laurent.pinchart@ideasonboard.com> wrote:
> > >
> > > Hi Adam,
> > >
> > > On Wed, Feb 22, 2023 at 05:39:30PM -0600, Adam Ford wrote:
> > > > On Fri, Nov 18, 2022 at 3:44 AM Paul Elder wrote:
> > > > >
> > > > > This series depends on v3 of "dt-bindings: media: Add macros for video
> > > > > interface bus types" [1].
> > > > >
> > > > > This series extends the rkisp1 driver to support the ISP found in the
> > > > > NXP i.MX8MP SoC.
> > > > >
> > > > > The ISP IP cores in the Rockchip RK3399 (known as the "Rockchip ISP1")
> > > > > and in the NXP i.MX8MP have the same origin, and have slightly diverged
> > > > > over time as they are now independently developed (afaik) by Rockchip
> > > > > and VeriSilicon. The latter is marketed under the name "ISP8000Nano",
> > > > > and is close enough to the RK3399 ISP that it can easily be supported by
> > > > > the same driver.
> > > > >
> > > > > The last two patches add support for UYVY output format, which can be
> > > > > implemented on the ISP version in the i.MX8MP but not in the one in the
> > > > > RK3399.
> > > > >
> > > > > This version of the series specifically has been tested on a Polyhex
> > > > > Debix model A with an imx219 (Raspberry Pi cam v2).
> > > > >
> > > > > [1] https://lore.kernel.org/linux-media/20220615221410.27459-2-laurent.pinchart@ideasonboard.com/
> > > > >
> > > > > Laurent Pinchart (3):
> > > > >   dt-bindings: media: rkisp1: Add i.MX8MP ISP example
> > > > >   media: rkisp1: Add and use rkisp1_has_feature() macro
> > > > >   media: rkisp1: Configure gasket on i.MX8MP
> > > > >
> > > > > Paul Elder (11):
> > > > >   dt-bindings: media: rkisp1: Add i.MX8MP ISP to compatible
> > > > >   media: rkisp1: Add match data for i.MX8MP ISP
> > > > >   media: rkisp1: Add and set registers for crop for i.MX8MP
> > > > >   media: rkisp1: Add and set registers for output size config on i.MX8MP
> > > > >   media: rkisp1: Add i.MX8MP-specific registers for MI and resizer
> > > > >   media: rkisp1: Shift DMA buffer addresses on i.MX8MP
> > > > >   media: rkisp1: Add register definitions for the test pattern generator
> > > > >   media: rkisp1: Fix RSZ_CTRL bits for i.MX8MP
> > > > >   media: rkisp1: Support devices without self path
> > > > >   media: rkisp1: Add YC swap capability
> > > > >   media: rkisp1: Add UYVY as an output format
> > > >
> > > > Paul / Laurent,
> > > >
> > > > I noticed an unexpected behaviour on the imx8mp.
> > > >
> > > > If I setup my pipeline for 640x480, it works just fine using an imx219
> > > > camera configured for SRGGB10_1X10.
> > > >
> > > > However, when I try to configure the pipeline to use the same camera
> > > > at 1920x1080 (no resizing), the ISP source keeps defaulting to 640x480
> > > >
> > > > Media device information
> > > > ------------------------
> > > > driver          rkisp1
> > > > model           rkisp1
> > > > serial
> > > > bus info        platform:rkisp1
> > > > hw revision     0xe
> > > > driver version  6.2.0
> > > >
> > > > Device topology
> > > > - entity 1: rkisp1_isp (4 pads, 4 links)
> > > >             type V4L2 subdev subtype Unknown flags 0
> > > >             device node name /dev/v4l-subdev0
> > > > pad0: Sink [fmt:SRGGB10_1X10/1920x1080 field:none colorspace:raw xfer:none ycbcr:601 quantization:full-range crop.bounds:(0,0)/1920x1080 crop:(0,0)/640x480]
> > >
> > > You're cropping the image to 640x480 here. You need to set the crop
> > > rectangle to 1920x1080.
> > >
> > > As Jacopo mentioned, I wouldn't recommend exercising the ISP directly.
> > > Not only do you need to setup the pipeline, but you would also need to
> > > implement all the imaging algorithms in userspace. libcamera will do all
> > > this for you.
> >
> > I'll give that a try.  My current employer has a v4l2src requirement,
> > but I can likely make an argument to switch to libcamera.  I didn't
> > catch the cropping part. Thanks for that.
>
> I'd hope you can transparently replace v4l2src with libcamerasrc, the plugins
> currently lives inside the libcamera project. If not, I'd really like to know
> why. We can work together on adding missing controls (this is something I'm
> starting on soon).

I plan to give it a try.  From what I've read it appears to be the
right thing to do.  I just need to carve out some time to get it
installed.  I mostly wanted to check out a camera adapter board my
company made, test some updates I pushed for the imx219 on a second
platform, and get more familiar with the ISP on the 8MP.

I'll open a separate thread if I have questions on the cameralib.
Thanks for all the feedback.  I look forward to seeing this driver
merged.

adam
>
> regards,
> Nicolas
>
> >
> > adam
> > >
> > > > <- "csis-32e40000.csi":1 [ENABLED]
> > > > pad1: Sink [fmt:unknown/0x0 field:none]
> > > > <- "rkisp1_params":0 [ENABLED,IMMUTABLE]
> > > > pad2: Source [fmt:YUYV8_2X8/640x480 field:none colorspace:raw xfer:none ycbcr:601 quantization:lim-range crop.bounds:(0,0)/640x480 crop:(0,0)/640x480]
> > > > -> "rkisp1_resizer_mainpath":0 [ENABLED]
> > > > pad3: Source [fmt:unknown/0x0 field:none]
> > > > -> "rkisp1_stats":0 [ENABLED,IMMUTABLE]
> > > >
> > > > - entity 6: rkisp1_resizer_mainpath (2 pads, 2 links)
> > > >             type V4L2 subdev subtype Unknown flags 0
> > > >             device node name /dev/v4l-subdev1
> > > > pad0: Sink [fmt:YUYV8_2X8/1920x1080 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:lim-range crop.bounds:(0,0)/1920x1080 crop:(0,0)/640x480]
> > > > <- "rkisp1_isp":2 [ENABLED]
> > > > pad1: Source [fmt:YUYV8_2X8/1920x1080 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:lim-range]
> > > > -> "rkisp1_mainpath":0 [ENABLED,IMMUTABLE]
> > > >
> > > > - entity 9: rkisp1_mainpath (1 pad, 1 link)
> > > >             type Node subtype V4L flags 0
> > > >             device node name /dev/video0
> > > > pad0: Sink
> > > > <- "rkisp1_resizer_mainpath":1 [ENABLED,IMMUTABLE]
> > > >
> > > > - entity 13: rkisp1_stats (1 pad, 1 link)
> > > >              type Node subtype V4L flags 0
> > > >              device node name /dev/video1
> > > > pad0: Sink
> > > > <- "rkisp1_isp":3 [ENABLED,IMMUTABLE]
> > > >
> > > > - entity 17: rkisp1_params (1 pad, 1 link)
> > > >              type Node subtype V4L flags 0
> > > >              device node name /dev/video2
> > > > pad0: Source
> > > > -> "rkisp1_isp":1 [ENABLED,IMMUTABLE]
> > > >
> > > > - entity 29: csis-32e40000.csi (2 pads, 2 links)
> > > >              type V4L2 subdev subtype Unknown flags 0
> > > >              device node name /dev/v4l-subdev2
> > > > pad0: Sink [fmt:SRGGB10_1X10/1920x1080 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:full-range]
> > > > <- "imx219 1-0010":0 [ENABLED]
> > > > pad1: Source [fmt:SRGGB10_1X10/1920x1080 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:full-range]
> > > > -> "rkisp1_isp":0 [ENABLED]
> > > >
> > > > - entity 34: imx219 1-0010 (1 pad, 1 link)
> > > >              type V4L2 subdev subtype Sensor flags 0
> > > >              device node name /dev/v4l-subdev3
> > > > pad0: Source [fmt:SRGGB10_1X10/1920x1080 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:full-range crop.bounds:(8,8)/3280x2464 crop:(688,700)/1920x1080]
> > > > -> "csis-32e40000.csi":0 [ENABLED]
> > > >
> > > > It's at this point that everything except the ISP source is 1920x1080.
> > > >
> > > > When I try to set the ISP sink to 1080, it ends up being 640x480 and
> > > > the resizer sink is also changed to 640x480
> > > >
> > > > root@beacon-imx8mp-kit:~# media-ctl -v -V "'rkisp1_isp':2
> > > > [fmt:YUYV8_2X8/1920x1080 field:none]"
> > > > Opening media device /dev/media0
> > > > Enumerating entities
> > > > looking up device: 81:3
> > > > looking up device: 81:4
> > > > looking up device: 81:0
> > > > looking up device: 81:1
> > > > looking up device: 81:2
> > > > looking up device: 81:5
> > > > looking up device: 81:6
> > > > Found 7 entities
> > > > Enumerating pads and links
> > > > Setting up format YUYV8_2X8 1920x1080 on pad rkisp1_isp/2
> > > > Format set: YUYV8_2X8 640x480
> > > > Setting up format YUYV8_2X8 640x480 on pad rkisp1_resizer_mainpath/0
> > > > Format set: YUYV8_2X8 640x480
> > > >
> > > >
> > > > It's my understanding that the ISP should be able to handle 1920x1080,
> > > > and the resizer sink should match the ISP source.
> > > >
> > > > With the pipeline improperly setup, the capture fails.
> > > >
> > > > >  .../bindings/media/rockchip-isp1.yaml         |  79 ++++++++++-
> > > > >  .../platform/rockchip/rkisp1/rkisp1-capture.c | 102 +++++++++++---
> > > > >  .../platform/rockchip/rkisp1/rkisp1-common.h  |  32 +++++
> > > > >  .../platform/rockchip/rkisp1/rkisp1-debug.c   |  14 +-
> > > > >  .../platform/rockchip/rkisp1/rkisp1-dev.c     |  67 +++++++--
> > > > >  .../platform/rockchip/rkisp1/rkisp1-isp.c     | 128 +++++++++++++++++-
> > > > >  .../platform/rockchip/rkisp1/rkisp1-regs.h    |  90 ++++++++++++
> > > > >  .../platform/rockchip/rkisp1/rkisp1-resizer.c |  35 ++++-
> > > > >  include/uapi/linux/rkisp1-config.h            |   2 +
> > > > >  9 files changed, 509 insertions(+), 40 deletions(-)
> > >
> > > --
> > > Regards,
> > >
> > > Laurent Pinchart
>

Re: [PATCH v3 00/14] media: rkisp1: Add support for i.MX8MP

[Tommaso Merciai]

Hello Paul,

On Fri, Nov 18, 2022 at 06:39:17PM +0900, Paul Elder wrote:
> This series depends on v3 of "dt-bindings: media: Add macros for video
> interface bus types" [1].
> 
> This series extends the rkisp1 driver to support the ISP found in the
> NXP i.MX8MP SoC.
> 
> The ISP IP cores in the Rockchip RK3399 (known as the "Rockchip ISP1")
> and in the NXP i.MX8MP have the same origin, and have slightly diverged
> over time as they are now independently developed (afaik) by Rockchip
> and VeriSilicon. The latter is marketed under the name "ISP8000Nano",
> and is close enough to the RK3399 ISP that it can easily be supported by
> the same driver.
> 
> The last two patches add support for UYVY output format, which can be
> implemented on the ISP version in the i.MX8MP but not in the one in the
> RK3399.
> 
> This version of the series specifically has been tested on a Polyhex
> Debix model A with an imx219 (Raspberry Pi cam v2).
> 
> [1] https://lore.kernel.org/linux-media/20220615221410.27459-2-laurent.pinchart@ideasonboard.com/

I tested your series on imx274 on imx8mp-evk csi0.
All looks good on my side.
Thanks for your work!

Tested-by: Tommaso Merciai <tomm.merciai@gmail.com>

Regards,
Tommaso

> 
> Laurent Pinchart (3):
>   dt-bindings: media: rkisp1: Add i.MX8MP ISP example
>   media: rkisp1: Add and use rkisp1_has_feature() macro
>   media: rkisp1: Configure gasket on i.MX8MP
> 
> Paul Elder (11):
>   dt-bindings: media: rkisp1: Add i.MX8MP ISP to compatible
>   media: rkisp1: Add match data for i.MX8MP ISP
>   media: rkisp1: Add and set registers for crop for i.MX8MP
>   media: rkisp1: Add and set registers for output size config on i.MX8MP
>   media: rkisp1: Add i.MX8MP-specific registers for MI and resizer
>   media: rkisp1: Shift DMA buffer addresses on i.MX8MP
>   media: rkisp1: Add register definitions for the test pattern generator
>   media: rkisp1: Fix RSZ_CTRL bits for i.MX8MP
>   media: rkisp1: Support devices without self path
>   media: rkisp1: Add YC swap capability
>   media: rkisp1: Add UYVY as an output format
> 
>  .../bindings/media/rockchip-isp1.yaml         |  79 ++++++++++-
>  .../platform/rockchip/rkisp1/rkisp1-capture.c | 102 +++++++++++---
>  .../platform/rockchip/rkisp1/rkisp1-common.h  |  32 +++++
>  .../platform/rockchip/rkisp1/rkisp1-debug.c   |  14 +-
>  .../platform/rockchip/rkisp1/rkisp1-dev.c     |  67 +++++++--
>  .../platform/rockchip/rkisp1/rkisp1-isp.c     | 128 +++++++++++++++++-
>  .../platform/rockchip/rkisp1/rkisp1-regs.h    |  90 ++++++++++++
>  .../platform/rockchip/rkisp1/rkisp1-resizer.c |  35 ++++-
>  include/uapi/linux/rkisp1-config.h            |   2 +
>  9 files changed, 509 insertions(+), 40 deletions(-)
> 
> -- 
> 2.35.1
>

Re: [PATCH v3 00/14] media: rkisp1: Add support for i.MX8MP

[Hans Verkuil]

Hi Paul,

On 18/11/2022 10:39, Paul Elder wrote:
> This series depends on v3 of "dt-bindings: media: Add macros for video
> interface bus types" [1].
> 
> This series extends the rkisp1 driver to support the ISP found in the
> NXP i.MX8MP SoC.
> 
> The ISP IP cores in the Rockchip RK3399 (known as the "Rockchip ISP1")
> and in the NXP i.MX8MP have the same origin, and have slightly diverged
> over time as they are now independently developed (afaik) by Rockchip
> and VeriSilicon. The latter is marketed under the name "ISP8000Nano",
> and is close enough to the RK3399 ISP that it can easily be supported by
> the same driver.
> 
> The last two patches add support for UYVY output format, which can be
> implemented on the ISP version in the i.MX8MP but not in the one in the
> RK3399.
> 
> This version of the series specifically has been tested on a Polyhex
> Debix model A with an imx219 (Raspberry Pi cam v2).

There were comments for the first few patches, but I haven't seen a v4.

I'm marking this series as 'Changes Requested' in patchwork, just so you
know.

Regards,

	Hans

> 
> [1] https://lore.kernel.org/linux-media/20220615221410.27459-2-laurent.pinchart@ideasonboard.com/
> 
> Laurent Pinchart (3):
>   dt-bindings: media: rkisp1: Add i.MX8MP ISP example
>   media: rkisp1: Add and use rkisp1_has_feature() macro
>   media: rkisp1: Configure gasket on i.MX8MP
> 
> Paul Elder (11):
>   dt-bindings: media: rkisp1: Add i.MX8MP ISP to compatible
>   media: rkisp1: Add match data for i.MX8MP ISP
>   media: rkisp1: Add and set registers for crop for i.MX8MP
>   media: rkisp1: Add and set registers for output size config on i.MX8MP
>   media: rkisp1: Add i.MX8MP-specific registers for MI and resizer
>   media: rkisp1: Shift DMA buffer addresses on i.MX8MP
>   media: rkisp1: Add register definitions for the test pattern generator
>   media: rkisp1: Fix RSZ_CTRL bits for i.MX8MP
>   media: rkisp1: Support devices without self path
>   media: rkisp1: Add YC swap capability
>   media: rkisp1: Add UYVY as an output format
> 
>  .../bindings/media/rockchip-isp1.yaml         |  79 ++++++++++-
>  .../platform/rockchip/rkisp1/rkisp1-capture.c | 102 +++++++++++---
>  .../platform/rockchip/rkisp1/rkisp1-common.h  |  32 +++++
>  .../platform/rockchip/rkisp1/rkisp1-debug.c   |  14 +-
>  .../platform/rockchip/rkisp1/rkisp1-dev.c     |  67 +++++++--
>  .../platform/rockchip/rkisp1/rkisp1-isp.c     | 128 +++++++++++++++++-
>  .../platform/rockchip/rkisp1/rkisp1-regs.h    |  90 ++++++++++++
>  .../platform/rockchip/rkisp1/rkisp1-resizer.c |  35 ++++-
>  include/uapi/linux/rkisp1-config.h            |   2 +
>  9 files changed, 509 insertions(+), 40 deletions(-)
> 

Re: [PATCH v3 04/14] media: rkisp1: Add match data for i.MX8MP ISP

[Adam Ford]

On Fri, Nov 18, 2022 at 3:44 AM Paul Elder <paul.elder@ideasonboard.com> wrote:
>
> Add match data to the rkisp1 driver to match the i.MX8MP ISP.
>
> Although the new version number isn't very precise, it ought to be fine
> as the other version numbers aren't precise either, and we have separate
> feature flags for important version-specific features. Despite this
> version number being seemingly unimportant, it is added to distinguish
> it from the ISP versions integrated in rockchip SoCs.
>
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
>

Paul,

It's been nearly a year since this commit was sent.  I noticed it
hasn't been applied, and I was curious to know if there is any
movement here?  I'm happy to test on my 8MP if necessary.

Thanks!

adam

> ---
> Changes in v3:
> - Remove todo for improving the version number
> - Expand the commit message to address the version number
> ---
>  .../platform/rockchip/rkisp1/rkisp1-dev.c     | 22 +++++++++++++++++++
>  include/uapi/linux/rkisp1-config.h            |  2 ++
>  2 files changed, 24 insertions(+)
>
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
> index e348d8c86861..69464ce91d59 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
> @@ -496,6 +496,24 @@ static const struct rkisp1_info rk3399_isp_info = {
>         .features = RKISP1_FEATURE_MIPI_CSI2,
>  };
>
> +static const char * const imx8mp_isp_clks[] = {
> +       "isp",
> +       "hclk",
> +       "aclk",
> +};
> +
> +static const struct rkisp1_isr_data imx8mp_isp_isrs[] = {
> +       { NULL, rkisp1_isr },
> +};
> +
> +static const struct rkisp1_info imx8mp_isp_info = {
> +       .clks = imx8mp_isp_clks,
> +       .clk_size = ARRAY_SIZE(imx8mp_isp_clks),
> +       .isrs = imx8mp_isp_isrs,
> +       .isr_size = ARRAY_SIZE(imx8mp_isp_isrs),
> +       .isp_ver = IMX8MP_V10,
> +};
> +
>  static const struct of_device_id rkisp1_of_match[] = {
>         {
>                 .compatible = "rockchip,px30-cif-isp",
> @@ -505,6 +523,10 @@ static const struct of_device_id rkisp1_of_match[] = {
>                 .compatible = "rockchip,rk3399-cif-isp",
>                 .data = &rk3399_isp_info,
>         },
> +       {
> +               .compatible = "fsl,imx8mp-isp",
> +               .data = &imx8mp_isp_info,
> +       },
>         {},
>  };
>  MODULE_DEVICE_TABLE(of, rkisp1_of_match);
> diff --git a/include/uapi/linux/rkisp1-config.h b/include/uapi/linux/rkisp1-config.h
> index 730673ecc63d..f602442c2018 100644
> --- a/include/uapi/linux/rkisp1-config.h
> +++ b/include/uapi/linux/rkisp1-config.h
> @@ -179,12 +179,14 @@
>   * @RKISP1_V11: declared in the original vendor code, but not used
>   * @RKISP1_V12: used at least in rk3326 and px30
>   * @RKISP1_V13: used at least in rk1808
> + * @IMX8MP_V10: used in at least imx8mp
>   */
>  enum rkisp1_cif_isp_version {
>         RKISP1_V10 = 10,
>         RKISP1_V11,
>         RKISP1_V12,
>         RKISP1_V13,
> +       IMX8MP_V10,
>  };
>
>  enum rkisp1_cif_isp_histogram_mode {
> --
> 2.35.1
>