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

Laurent Pinchart
2025-07-23T12:09:47Z

On Wed, Jul 23, 2025 at 02:06:43PM +0200, Krzysztof Hałasa wrote:
> Stefan Klug 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.

There's a crossbar switch in the ISI, but the connections from CSI-2
receivers to ISPs are fixed.

Have you tried reporting the issue to NXP ?

> 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@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@0 {
> reg = <0>;
> mipi_csi_1_in: endpoint {
> remote-endpoint = <&imx462_mipi_ep>;
> data-lanes = <1 2 3 4>;
> };
> };
>
> port@1 {
> reg = <1>;
> mipi_csi_1_out: endpoint {
> remote-endpoint = <&isp_1_in>;
> };
> };
> };
> };
>
> &isp_1 {
> status = "okay";
>
> ports {
> port@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@verisilicon.com> 2022-08-05 10:19:49
> Committer: Robby Cai <robby.cai@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;
>

--
Regards,

Laurent Pinchart