FYI: i.MX8MP ISP (RKISP1) MI registers corruption

[Krzysztof Hałasa]

Hi Stefan,

Stefan Klug <stefan.klug at ideasonboard.com> writes:

> Just a quick heads up. I ran the tester and so far no unexpected
> results. I'll run it from time to time after a reboot to see if I ever
> hit that condition.
>
> How does your device tree look like? Any chance the ISP is clocked for
> overdrive but the device is not or something similar? Although I have a
> hard time imagining how that would lead to such effects.

Interesting.
I tested it on two different devices (a Compulab UCM-based camera and
a Solidrun Hummingboard Mate) and it's the same on both. I think the
first one uses 1600 MHz industrial CPU:

(U-boot) CPU: i.MX8MP[8] rev1.1 at 1200 MHz

Not sure about the Hummingboard.

Both cameras apparently are connected to the second MIPI.

Well... maybe if it's only the second ISP, and there is only one camera,
then we could reroute the data to the first ISP? The MIPI receiver has
a crossbar I think.
And the other way around: for a test, one could reroute MIPI0 to ISP1.
Will have a look.

The DT has the usual stuff (for the second MIPI/ISP):

&i2c6 {
	clock-frequency = <400000>;
	pinctrl-names = "default";
	pinctrl-0 = <&pinctrl_i2c6>;
	single-master;
	status = "okay";

	imx462_camera at 1a {
		compatible = "sony,imx462";
		reg = <0x1a>;
		clocks = <&clk IMX8MP_CLK_IPP_DO_CLKO2>;
		clock-names = "xclk";
		clock-frequency = <37125000>;
		reset-gpios = <&gpio2 1 GPIO_ACTIVE_HIGH>;
		status = "okay";

		port {
			imx462_mipi_ep: endpoint {
				data-lanes = <1 2 3 4>;
				clock-lanes = <0>;
				link-frequencies = /bits/ 64 <222750000 148500000>;
				remote-endpoint = <&mipi_csi_1_in>;
			};
		};
	};

};

&mipi_csi_1 {
    status = "okay";

    ports {
        port at 0 {
            reg = <0>;
            mipi_csi_1_in: endpoint {
                remote-endpoint = <&imx462_mipi_ep>;
                data-lanes = <1 2 3 4>;
            };
        };

        port at 1 {
            reg = <1>;
            mipi_csi_1_out: endpoint {
                remote-endpoint = <&isp_1_in>;
            };
        };
    };
};

&isp_1 {
    status = "okay";

    ports {
        port at 1 {
            isp_1_in: endpoint {
                bus-type = <MEDIA_BUS_TYPE_PARALLEL>;
                remote-endpoint = <&mipi_csi_1_out>;
            };
        };
    };
};

The CCM registers show basically (p. 229 in i.MX8MP ref manual):
  8 MEDIA_ISP           mux 7       post 0 SYSTEM_PLL2_DIV2 = 500 MHz
 20 MEDIA_AXI           mux 1 pre 1 post 0 SYSTEM_PLL2_CLK / 2 = 500 MHz
 21 MEDIA_APB           mux 2 pre 3 post 0 SYSTEM_PLL1_CLK / 4 = 200 MHz
123 MEDIA_MIPI_PHY1_REF mux 0 pre 0 post 0 24M_REF_CLK = 24 MHz
125 MEDIA_CAM2_PIX      mux 2 pre 0 post 0 SYSTEM_PLL2_DIV4 = 250 MHz

The first 3 are at the max values, MEDIA_MIPI_PHY1_REF max is 125 MHz,
MEDIA_CAM2_PIX max is 266 MHz. Maybe I should try changing these clocks,
but not sure how do I do that (any change causes rkisp1 driver loading
to fail). Will look at it.

BTW the double read and double write in NXP driver (isp-vvcam) were
introduced by (in their repo):

Author: hexing <Xing.He at verisilicon.com>  2022-08-05 10:19:49
Committer: Robby Cai <robby.cai at nxp.com>  2022-08-08 04:50:48

M865SW-1031: the second isp port jump frames

Reason:mi read or write reg occasionally it does not take effect

WorkAround:read or write twice of mi reg

---------------------------- vvcam/isp/isp_ioctl.c ----------------------------
index 60741bd..e0d3048 100644
@@ -118,5 +118,8 @@ void isp_write_reg(struct isp_ic_dev *dev, u32 offset, u32 val)
 	if (offset >= ISP_REG_SIZE)
 		return;
-	__raw_writel(val, dev->base + offset);
+	writel(val, dev->base + offset);
+	if ((offset >= REG_ADDR(mi_mp_y_base_ad_init))
+		&& (offset <= REG_ADDR(mi_mp_y_pic_size)))
+		writel(val, dev->base + offset);
 //	  isp_info("%s	addr 0x%08x val 0x%08x\n", __func__, offset, val);
 }
@@ -128,5 +131,8 @@ u32 isp_read_reg(struct isp_ic_dev *dev, u32 offset)
 	if (offset >= ISP_REG_SIZE)
 		return 0;
-	val = __raw_readl(dev->base + offset);
+	val = readl(dev->base + offset);
+	if ((offset >= REG_ADDR(mi_mp_y_base_ad_init))
+		&& (offset <= REG_ADDR(mi_mp_y_pic_size)))
+		val = readl(dev->base + offset);
 //	  isp_info("%s	addr 0x%08x val 0x%08x\n", __func__, offset, val);
 	return val;

-- 
Krzysztof "Chris" Hałasa

Sieć Badawcza Łukasiewicz
Przemysłowy Instytut Automatyki i Pomiarów PIAP
Al. Jerozolimskie 202, 02-486 Warszawa