| Message ID | m3h61u9jy2.fsf@t19.piap.pl |
|---|---|
| State | New |
| Headers | show |
| Series | Enable MIPI filtering by DT on i.MX8M* | expand |
Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes: >> +++ b/drivers/media/platform/nxp/imx-mipi-csis.c >> @@ -654,8 +654,7 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis, >> val = mipi_csis_read(csis, MIPI_CSIS_CMN_CTRL); >> val &= ~MIPI_CSIS_CMN_CTRL_LANE_NR_MASK; >> val |= (lanes - 1) << MIPI_CSIS_CMN_CTRL_LANE_NR_OFFSET; >> - if (csis->info->version == MIPI_CSIS_V3_3) >> - val |= MIPI_CSIS_CMN_CTRL_INTER_MODE; >> + val |= MIPI_CSIS_CMN_CTRL_INTER_MODE; /* enable filtering by DT */ > > The condition was added because the CSIS in the i.MX8MM doesn't > implement the INTERLEAVE_MODE field. We can't remove it unconditionally. Is this confirmed (and not just an incidental omission from the docs)? Same version (3.6.3), and even earlier version (3.3) has it... It would mean MM can't work with those sensors producing extra packets. I wonder what version is shown in the #0 register on 8MM (8MP shows 3060301). > You mentioned i.MX8MP, that's a platform where I'd like to see proper > support for *capturing* embedded data, not just dropping it. Have you > looked at how this could be implemented ? I had a brief look at it, but a) the embedded data is not very interesting in case of my IMX290, b) I don't want to interleave it with my image data (DMA buffers and what not) and I don't see a way to store it independently. If you want to store it along the image, the currect code does that - more or less correctly. This is the problem. The RM says "13.5.2.6.6 Null and Blanking Data For both the null and blanking data types CSIS V3.6.3 ignore the content of the packet payload data." which is half-truth, e.g. it needs the MIPI_CSIS_CMN_CTRL_INTER_MODE to do that, otherwise it messes it up. Several CSIC registers are named XXXXXn, suggesting more than one register, but the docs say only #0 exists. Nevertheless, the actual hardware seems to contain 3 packs of registers (the 4th one is weirder): 32E40000: 3060301 4705 F0000 DEADCAFE 32E40010: FFFFFFFF 0 0 0 32E40020: F0 900001F DEADCAFE DEADCAFE 32E40030: 1F4 0 0 0 32E40040: B0 4380780 0 DEADCAFE <<< ISP_CONFIG0 32E40050: 8FD 80008000 0 DEADCAFE <<< ISP_CONFIG1 32E40060: 8FE 80008000 0 DEADCAFE <<< ISP_CONFIG2 32E40070: 8FF 80008000 0 DEADCAFE ??? 32E40080: B0 4380780 0 DEADCAFE <<< SHADOW_CONFIG0 32E40090: 8FD 80008000 0 DEADCAFE <<< SHADOW_CONFIG1 32E400A0: 8FE 80008000 0 DEADCAFE <<< SHADOW_CONFIG2 32E400B0: 0 0 0 DEADCAFE 32E400C0: 0 7FFFFFFF 0 E4 32E400D0: 0 0 0 DEADCAFE 32E400E0: DEADCAFE DEADCAFE DEADCAFE DEADCAFE 32E400F0: DEADCAFE DEADCAFE DEADCAFE DEADCAFE 32E40100: 22E1 22E1 22E1 0 <<< FRAME_COUNTER* 32E40110: 0 0 0 0 <<< LINE_INTERRUPT_RATIO* 32E40120: 0 DEADCAFE DEADCAFE DEADCAFE This is the first CSI. The 3 frame counters are visibly active as well. The manual states (MIPI_CSIx_ISP_CONFIGn) "NOTE: Not described types are ignored" and even if not, I can't see what could we do with this extra data. Perhaps the CSIC internally has 3 output ports, but only the first one is connected to ISI and ISP?
I wrote: > This produces (test_pattern=5 which starts with black, using ISP): > Y = 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00... > UV = 80 80 80 80 80 80 80 80 80 80 80 80 80 80 80 80... > > Now I do (perhaps I should revert the patch instead): > ./devmem write32 0x32E50004 0x14305 > > and this does (= without DT filtering): > Y = E6 FF 36 1B 00 00 00 00 00 00 00 00 00 00 00 00... > UV = 85 6A 74 B4 7D 8C 80 80 80 80 80 80 80 80 80 80... The corruption is visible in ISP RAW-12 mode as well: CSI2 + ISP2 without DT filtering: IMX462 test patterns, Linux v6.14, 1280x720p25. Only 3 first 16-bit (12-bit on MIPI) RGGB values in each frame are changed (bits 3-0 of the third pixel aren't changed). 32EC0060h 0 Gasket 0 output disabled 32EC0090h 0 Gasket 1 output disabled 32EC0138h 2D8000h ISP Dewarp Control Register (ISP_DEWARP_CONTROL) ISP ID mode 0, ISP1: DT 0, ISP2: DT 2Ch (RAW12) left-just mode 32E50040h B0h ISP Configuration Register (MIPI_CSIS_ISPCONFIG_CH0) DT 2Ch (RAW12) pattern 1: 0 800 0 2AB changed into 2AA B02 B00 2AB pattern 2 and 3: FFF FFF FFF FFF... not altered at all pattern 4: 501 501 4C2 4C2 changed into FFF 7FF 7F2 4C2 pattern 5: 0 0 0 0 changed into 7EF FF7 FF0 0 pattern 6: 0 1 0 101 changed into FFF FFF FF0 101 pattern 7: 0 2AB 0 2AB changed into BAA BAB BA0 2AB RAW-12 on MIPI goes like this: lane0 lane1 lane2 7:4 lane2 3:0 lane3 ----------- MIPI Header (all 4 lanes) ---------- P1-11:4 P2-11:4 P1-3:0 P2-3:0 P3-11:4 P4-11:4 ... This means all the changed values are located in the first 4 bytes after the packet header (i.e., in the first byte after the header for each lane). Which IMHO smells like a hardware bug - especially given the problem manifests itself only on CSI2 (+ISP2, I haven't tried using ISI). Fortunately enabling DT filtering fixes it. I remember I was getting a bit different results on, I believe, the NXP's 5.15 kernels (with their = Verisilicon VVCam driver). Instead of simply changing the first 32 bits of the MIPI payload, the rest of the RGGB data was shifted a couple of pixels or so. Now, what do we do with it? Is anybody able to verify the CSIC version register value on i.MX8MM? Something like devmem read32 0x32E50000 (or 0x32E40000 for CSI1) WHILE RUNNING CAPTURE on that very CSI would do the trick (using your favorite instance of devmem/devmem2/etc). Alternatively one could add a debug printk to the csic module. And: is anybody able to check if the DT filtering works on i.MX8MM (= if my patch doesn't break it on 8MM)? Alternatively I guess we can add MIPI_CSIS_V3_6_3_1 for 8MP only. Anybody using i.MX8MM with ISP2 + CSI2 BTW? Is the corruption there as well? I understand it may be hard to spot, it's (usually a bright) dot in the left top corner.
Hi Krzysztof On Thu, May 22, 2025 at 02:06:49PM +0200, Krzysztof Hałasa wrote: > Hi Jacopo, > > Jacopo Mondi <jacopo.mondi@ideasonboard.com> writes: > > > However the i.MX8MP does implement INTERLEAVE_MODE, and it's anyway > > marked as V3.6.3, so DT filtering is there disabled as well. I guess > > this is intentional, see below... > > BTW the version register on iMX8MP is 3060301, which may possibly mean > something like 3.6.3.1. I wonder if 8MM has the same. > > > some registers like 32E4_000C are not listed in the peripheral memory > > map, so you're probably reading an invalid memory area there > > Sure - those are apparently wired to contain "DEADCAFE" (a hex value). > > > If you're capturing RAW12 in 1920x1080 these two registers are > > > > 32E40040: (MIPI_CSI1_ISP_CONFIG_CH0) = 0xb0 > > 32E40044: (MIPI_CSI1_ISP_RESOLUTION_CH0) = 0x4380780 > > 32E40048: (MIPI_CSI1_ISP_SYNC_CH0) = 0 > > 32E4004c: invalid > > Sure. > > >> 32E40050: 8FD 80008000 0 DEADCAFE <<< ISP_CONFIG1 > >> 32E40060: 8FE 80008000 0 DEADCAFE <<< ISP_CONFIG2 > >> 32E40070: 8FF 80008000 0 DEADCAFE ??? > > > > All of these are invalid registers > > Only those DEADCAFEs are strictly invalid, the rest is just > undocumented. With the notable exception of version register, they > aren't probably useful, though - due to the way the CSIC is connected to > the rest of the chip. > > I only mentioned them because Laurent asked about capturing embedded > data - I guess the registers could be used for that on some other chip > (apparently not on i.MX8MP). > > > We have been using 8mp extensively with sensors that produce embedded > > data and afaict ED are not in the final image. > > Well, I admit I haven't looked it down to this finest detail. The > visible effect was the image was slightly corrupted without the DT > filtering, so I assumed ED was doing that. > > I use two similar sensors: IMX290 (on CSI1) and IMX462 (CSI2). It > appears IMX290 doesn't cause the problem, while IMX462 does. This may > depend on the CSI used, though. Both sensors seem to produce the > following MIPI packets (counting from start of video frame, 1920x1080p25 > raw 12 bit): > - Frame Start: DT=0 > - a short Embedded data packet, DT=12h > - a NULL packet, line-sized, DT=0x10 > - 10 User Defined 8-bit Data Type 8 packets, line-sized, DT=0x37, called > apparently "Vertical OB" by Sony datasheet These are optically black pixels and should probably be discarded by the gasket as well as embedded data ? > - 1080 real lines, DT=0x2C, 12-bit RGGB data > - short Frame End packet, DT=1 > > I hope I got this right, this is straight from oscilloscope (only > checked IDs on IMX462, will confirm IMX290 later but it looks the same). > In 1280x1080p25 mode there are 4 (not 10) "vertical OB" packets, and 720 > RGGB lines instead of 1080. Is this correct ? you ask for 1280x1080 and get back 720 lines ? > > The ED packet is much shorted than an RGGB line data (2.32us vs 13.57 us > or 3.54us vs 13.88us - 1080p and 720p use different MIPI clock rates). > > So yes, this whole ED packet definitely doesn't end up in the image. Great to know! > IMX462 produces just a tiny 2-pixel dot in the left top corner, possibly > shifting some data to the right (I remember it did that, but I can't > observe it now - could be a kernel (driver) version change?). What are those two pixels ? Does the datasheet describes them ? > > > My understanding is that the gasket that connects the CSIS to the ISI > > and the ISP has a filtering register has well, and there is where ED > > gets discarded. Could you maybe check the value of register GASKET_0_CTRL > > to confirm this ? > > (with the filtering) > > MEDIA_BLOCK_CTRL: > 32EC0000h 1FFFFFFh Media Mix Software Reset Register (IMX_MEDIA_BLK_CTRL_SFT_RSTN) > 32EC0004h 70700h Media Mix Clock Enable Register (IMX_MEDIA_BLK_CTRL_CLK_EN) > Clock enabled: ISP_CLKs MIPI_CSI2_CLKs BUS_BLK_CLK > 32EC0008h 40000000h MIPI PHY Control Register (MIPI_RESET_DIV) > 32EC004Ch FC00h LCDIF ARCACHE Control Register (LCDIF_ARCACHE_CTRL) > 32EC0050h 1FF00000h ISI CACHE Control Register (ISI_CACHE_CTRL) > 32EC0060h 0 Gasket 0 output disabled > 32EC0090h 0 Gasket 1 output disabled indeed gaskets seems to be disabled when using the ISP > 32EC0138h 2D8000h ISP Dewarp Control Register (ISP_DEWARP_CONTROL) > ISP ID mode 0, ISP1: DT 0h (unknown), ISP2: DT 2Ch (RAW12) left-just mode But this other register has (again) one other filtering option and reading the value here it is set to filter RAW12 (mipi_isp2_data_type) Weirdly enough, I don't see this register being programmed in the mainline gasket driver... > > MIPI_CSI2: > 32E50000h 3060301h CSIS version (MIPI_CSIS_VERSION) > 32E50004h 4705h CSIS Common Control Register (MIPI_CSIS_CMN_CTRL) Do you mean 0x14705 ? I'm asking because Shadow Crtl is BIT(16). Surprisingly BIT(14) is marked as reserved in the datasheet version I have > Filtering by DT, Update shadow ctrl, 4 data lanes > 32E50008h F0000h CSIS Clock Control Register (MIPI_CSIS_CLK_CTRL) > 32E50010h FFFFFFFFh Interrupt mask register 0 (MIPI_CSIS_INTMSK) > 32E50020h F0h D-PHY status register (MIPI_CSIS_DPHYSTATUS) > Clock lane: Active, Data lanes: 0: Stop, 1: Stop, 2: Stop, 3: Stop > 32E50024h 600001Fh D-PHY common control register (MIPI_CSIS_DPHYCTRL) > 32E50030h 1F4h D-PHY Master and Slave Control register Low (DPHY_MASTER_SLAVE_CTRL_LOW) > 32E50040h B0h ISP Configuration Register (MIPI_CSIS_ISPCONFIG_CH0) > DT 2Ch (RAW12) > 32E50044h 2D00500h ISP Resolution Register (MIPI_CSIS_ISPRESOL_CH0) => 1280 * 720 > 32E50080h B0h Shadow Configuration Register (MIPI_CSIS_ISPCONFIG_CH0) > DT 2Ch (RAW12) > 32E50084h 2D00500h Shadow Resolution Register (MIPI_CSIS_ISPRESOL_CH0) => 1280 * 720 > 32E50100h 25E2h Frame Counter (FRAME_COUNTER_CH0) > > This produces (test_pattern=5 which starts with black, using ISP): > Y = 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00... > UV = 80 80 80 80 80 80 80 80 80 80 80 80 80 80 80 80... > > Now I do (perhaps I should revert the patch instead): > ./devmem write32 0x32E50004 0x14305 > > and this does: > Y = E6 FF 36 1B 00 00 00 00 00 00 00 00 00 00 00 00... > UV = 85 6A 74 B4 7D 8C 80 80 80 80 80 80 80 80 80 80... > > Maybe I could see where these values come from. > > > With test_pattern = 4 > Y = 52 52 4E 4D 14 14 00 00 00 00 00 51 52 52 4E 4D... > UV = 82 83 82 83 81 81 80 80 80 80 81 80 82 83 82 83... > changes into (without filtering): > Y = 9B 99 58 53 14 14 00 00 00 00 00 51 52 52 4E 4D... > UV = 77 AE 78 9A 81 83 80 80 80 80 81 80 82 83 82 83... > > The values are stable but it seems they are added/xored/etc. with the > original image data or something. > > > In particular the "Gasket 0 data type" is programmed in > > drivers/media/platform/nxp/imx8-isi/imx8-isi-gasket.c with the data > > type of the first stream reported by the sensor with get_frame_desc(). > > In your case, assuming RAW12 you should have 101100b in that register. > > Gaskets are disabled. I will try to do some tests tomorrow. > I know that the internal working of the gasket and the cross-bar are not entirely clear and very bad documented. I wouldn't be against enabling filtering on the CSIS but 1) if we ever want to capture embedded data the change should be reversed 2) it would be nice to actually understand where ED are filtered if gaskets are disabled > > Now, I think the idea was to use the gasket for filtering ED (and > > other non-image data) to be able to route them to the ISI for capture, > > while images are sent to the ISP for processing. This is currently not > > implemented and there are some unclear parts in the documentation > > about this, but overall my expectation is that ED are filtered by the > > gasket so you shouldn't need to enable DT filtering on the CSIS. > -- > Krzysztof "Chris" Hałasa > > Sieć Badawcza Łukasiewicz > Przemysłowy Instytut Automatyki i Pomiarów PIAP > Al. Jerozolimskie 202, 02-486 Warszawa
Hello, On 5/23/25 11:58 AM, Krzysztof Hałasa wrote: > Now, what do we do with it? > Is anybody able to verify the CSIC version register value on i.MX8MM? > Something like devmem read32 0x32E50000 (or 0x32E40000 for CSI1) WHILE > RUNNING CAPTURE on that very CSI would do the trick (using your > favorite instance of devmem/devmem2/etc). Alternatively one could add > a debug printk to the csic module. On i.MX8MM mipi_csi is at 32e30000. NXP kernel (6.12.3) with ov5640 camera: # devmem 0x32E30000 32 0x03060301 Regards,
Hi Sébastien, Sébastien Szymanski <sebastien.szymanski@armadeus.com> writes: > On i.MX8MM mipi_csi is at 32e30000. NXP kernel (6.12.3) with ov5640 camera: > > # devmem 0x32E30000 32 > 0x03060301 Thanks a lot. So at least the version register is the same as on i.MX8MP. I wonder if you could test if the CSIC possibly supports filtering by DT. How would such a test be performed? Maybe: while streaming (otherwise the system may lock up): I assume 'devmem 0x32E30004 32' shows 0x4305. # devmem 0x32E30004 32 0x14705 and: - does it change anything? If the streaming stops, you may try restarting and writing with devmem again - writing to this register directly frequently stops streaming on my 8MP. - what does the register contain after write? - you may also try: # devmem 0x32E30004 32 0xfffffffd # devmem 0x32E30004 32 On i.MX8MP the above returns: # ./devmem write32 0x32Ex0004 0xfffffffd # ./devmem read32 0x32Ex0004 0x0000FF05 (and the streaming still works, usually). The above is not a very good test, I admit, but it may show 8MM really doesn't have those/that bit. TIA.
Hi Jacopo, Jacopo Mondi <jacopo.mondi@ideasonboard.com> writes: >> - 10 User Defined 8-bit Data Type 8 packets, line-sized, DT=0x37, called >> apparently "Vertical OB" by Sony datasheet > > These are optically black pixels and should probably be discarded by > the gasket as well as embedded data ? Yes, apparently. >> I hope I got this right, this is straight from oscilloscope (only >> checked IDs on IMX462, will confirm IMX290 later but it looks the same). >> In 1280x1080p25 mode there are 4 (not 10) "vertical OB" packets, and 720 >> RGGB lines instead of 1080. > > Is this correct ? you ask for 1280x1080 and get back 720 lines ? Well, no, I just checked both modes and these are the differences. IOW, nothing unexpected. Wrong copy & paste or so. >> IMX462 produces just a tiny 2-pixel dot in the left top corner, possibly >> shifting some data to the right (I remember it did that, but I can't >> observe it now - could be a kernel (driver) version change?). > > What are those two pixels ? Does the datasheet describes them ? Nope, I guess it's a silicon bug. It corrupts 3 RAW-12 pixels, though (32 bits > 2 * 12 bits). >> 32EC0138h 2D8000h ISP Dewarp Control Register (ISP_DEWARP_CONTROL) >> ISP ID mode 0, ISP1: DT 0h (unknown), ISP2: DT 2Ch (RAW12) left-just mode > > But this other register has (again) one other filtering option and > reading the value here it is set to filter RAW12 (mipi_isp2_data_type) > > Weirdly enough, I don't see this register being programmed in the > mainline gasket driver... I guess it's drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c: static int rkisp1_gasket_enable(struct rkisp1_device *rkisp1, struct media_pad *source) { ... regmap_update_bits(rkisp1->gasket, ISP_DEWARP_CONTROL, mask, val); Now this register doesn't filter data: if you set it with a different value, the data still goes through. It's just processed differently. For example, my sensor is 12-bit (in addition to 10-bit). If I set ISP_DEWARP_CONTROL to 0xB68 (or 0x368) meaning RAW 14-bit, the image only gets darker - ISP thinks it's getting 14-bit samples. The only filtering (while using ISP) is, apparently, in the ISP Configuration Register (MIPI_CSIS_ISPCONFIG_CH0) and subsequently in its shadow counterpart. And it somehow may be enabled in CSIS Common Control Register (MIPI_CSIS_CMN_CTRL). But if you don't enable it there, the data is still filtered (e.g. wrong value in MIPI_CSIS_ISPCONFIG_CH0 prevents data flow). The filtering is only needed for preventing pixel corruption (these 3 pixels). So it means, for at least i.MX8MP, the DT filtering bit in MIPI_CSIS_CMN_CTRL should always be enabled. >> MIPI_CSI2: >> 32E50004h 4705h CSIS Common Control Register (MIPI_CSIS_CMN_CTRL) > > Do you mean 0x14705 ? I'm asking because Shadow Crtl is BIT(16). Surprisingly > BIT(14) is marked as reserved in the datasheet version I have No, it's 0x4705 (usually). With 0xFF05 (resulting from write with 0xfffffffd) it's still working correctly (with DT filtering). Write 0xfffffbfd (= no DT filtering) and the magic light pixels in the left top corner reappear. So it means the INTERLEAVE_MODE bits (11 and 10) are probably two independent bits, with bit 11 probably not used at all. In my copy (i.MX 8M Plus Applications Processor Reference Manual, Rev. 1, 06/2021), CSIS Common Control Register (MIPI_CSIx_CSIS_COMMON_CTRL): - bits 31-17, 15, 13, 12, 7-2 are zero and reserved (though bits 12 and 2 are additionally marked "This read-only field is reserved and always has the value 0") - bit 14 is reserved and shown as "1" - bit 16 is "UPDATE_SHADOW", and it clears itself after a write (unless the pipeline locks up or something alike) - bits 11 and 10 are "INTERLEAVE_MODE": Select Interleave mode ISP Configuration register of CH# is used for data interleave. 00 CH0 only, no data interleave 01 DT (Data type) only 10 Reserved 11 Reserved - bits 9 and 8 are LANE_NUMBER, 0 to 3 means 1 to 4. - bit 1 is SW_RESET - bit 0 is CSI_EN In fact, bit 2 does not "always have the value 0", it's set to 1. Both bits 14 and 2 can be reset to 0, though (without apparent change in the image).
Hi Krzysztof thanks for the detailed analysis On Thu, May 29, 2025 at 01:27:56PM +0200, Krzysztof Hałasa wrote: > Hi Jacopo, > > Jacopo Mondi <jacopo.mondi@ideasonboard.com> writes: > > >> - 10 User Defined 8-bit Data Type 8 packets, line-sized, DT=0x37, called > >> apparently "Vertical OB" by Sony datasheet > > > > These are optically black pixels and should probably be discarded by > > the gasket as well as embedded data ? > > Yes, apparently. > > >> I hope I got this right, this is straight from oscilloscope (only > >> checked IDs on IMX462, will confirm IMX290 later but it looks the same). > >> In 1280x1080p25 mode there are 4 (not 10) "vertical OB" packets, and 720 > >> RGGB lines instead of 1080. > > > > Is this correct ? you ask for 1280x1080 and get back 720 lines ? > > Well, no, I just checked both modes and these are the differences. > IOW, nothing unexpected. Wrong copy & paste or so. > > >> IMX462 produces just a tiny 2-pixel dot in the left top corner, possibly > >> shifting some data to the right (I remember it did that, but I can't > >> observe it now - could be a kernel (driver) version change?). > > > > What are those two pixels ? Does the datasheet describes them ? > > Nope, I guess it's a silicon bug. It corrupts 3 RAW-12 pixels, though > (32 bits > 2 * 12 bits). > > >> 32EC0138h 2D8000h ISP Dewarp Control Register (ISP_DEWARP_CONTROL) > >> ISP ID mode 0, ISP1: DT 0h (unknown), ISP2: DT 2Ch (RAW12) left-just mode > > > > But this other register has (again) one other filtering option and > > reading the value here it is set to filter RAW12 (mipi_isp2_data_type) > > > > Weirdly enough, I don't see this register being programmed in the > > mainline gasket driver... > > I guess it's drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c: > > static int rkisp1_gasket_enable(struct rkisp1_device *rkisp1, > struct media_pad *source) > { > ... > regmap_update_bits(rkisp1->gasket, ISP_DEWARP_CONTROL, mask, val); Ah indeed, thanks! > > Now this register doesn't filter data: if you set it with a different > value, the data still goes through. It's just processed differently. > For example, my sensor is 12-bit (in addition to 10-bit). If I set > ISP_DEWARP_CONTROL to 0xB68 (or 0x368) meaning RAW 14-bit, the image > only gets darker - ISP thinks it's getting 14-bit samples. Thanks for testing > > The only filtering (while using ISP) is, apparently, in the ISP > Configuration Register (MIPI_CSIS_ISPCONFIG_CH0) and subsequently in its > shadow counterpart. And it somehow may be enabled in CSIS Common Control > Register (MIPI_CSIS_CMN_CTRL). But if you don't enable it there, the > data is still filtered (e.g. wrong value in MIPI_CSIS_ISPCONFIG_CH0 > prevents data flow). The filtering is only needed for preventing pixel > corruption (these 3 pixels). Let me recap all of this. With: - MIPI_CSIx_CSIS_COMMON_CTRL[11:10] "Select Interleave mode" = 0x00 = CH0 only, no data interleave - MIPI_CSIx_ISP_CONFIGn[7:2] "Image Data Format" = 0x2c = RAW12 Embedded data and OB pixels are filtered (which means we're filtering on DT even if MIPI_CSIx_CSIS_COMMON_CTRL[11:10] = 0x0x would suggest filtering is not enabled) However corrupted pixels are still sent through. If you - MIPI_CSIx_CSIS_COMMON_CTRL[11:10] "Select Interleave mode" = 0x01 = DT (Data type) only Embedded data and OB pixels are still filtered, and your corrupt pixels are filtered as well. Now, why are "corrupted pixels" filtered away by enabling this option ? As far as I understand bad pixels are corrupt in their data values, the CSI-2 packet header which contains the DT is correct. Is my understanding correct ? > > So it means, for at least i.MX8MP, the DT filtering bit in > MIPI_CSIS_CMN_CTRL should always be enabled. It would be nice to actually understand what it does before enabling it unconditionally. I've cc-ed Xavier from NXP which maybe can help shed some light on that register function ? Thanks for the investigation > > >> MIPI_CSI2: > >> 32E50004h 4705h CSIS Common Control Register (MIPI_CSIS_CMN_CTRL) > > > > Do you mean 0x14705 ? I'm asking because Shadow Crtl is BIT(16). Surprisingly > > BIT(14) is marked as reserved in the datasheet version I have > > No, it's 0x4705 (usually). > With 0xFF05 (resulting from write with 0xfffffffd) it's still working > correctly (with DT filtering). Write 0xfffffbfd (= no DT filtering) and > the magic light pixels in the left top corner reappear. So it means the > INTERLEAVE_MODE bits (11 and 10) are probably two independent bits, with > bit 11 probably not used at all. > > > In my copy (i.MX 8M Plus Applications Processor Reference Manual, Rev. > 1, 06/2021), CSIS Common Control Register (MIPI_CSIx_CSIS_COMMON_CTRL): > - bits 31-17, 15, 13, 12, 7-2 are zero and reserved (though bits 12 and > 2 are additionally marked "This read-only field is reserved and always > has the value 0") > - bit 14 is reserved and shown as "1" > - bit 16 is "UPDATE_SHADOW", and it clears itself after a write (unless > the pipeline locks up or something alike) > - bits 11 and 10 are "INTERLEAVE_MODE": > Select Interleave mode > ISP Configuration register of CH# is used for data interleave. > 00 CH0 only, no data interleave > 01 DT (Data type) only > 10 Reserved > 11 Reserved > - bits 9 and 8 are LANE_NUMBER, 0 to 3 means 1 to 4. > - bit 1 is SW_RESET > - bit 0 is CSI_EN > > In fact, bit 2 does not "always have the value 0", it's set to 1. Both > bits 14 and 2 can be reset to 0, though (without apparent change in the > image). > -- > Krzysztof "Chris" Hałasa > > Sieć Badawcza Łukasiewicz > Przemysłowy Instytut Automatyki i Pomiarów PIAP > Al. Jerozolimskie 202, 02-486 Warszawa >
Jacopo, > Let me recap all of this. > > With: > > - MIPI_CSIx_CSIS_COMMON_CTRL[11:10] > "Select Interleave mode" = 0x00 = CH0 only, no data interleave > > - MIPI_CSIx_ISP_CONFIGn[7:2] > "Image Data Format" = 0x2c = RAW12 > > Embedded data and OB pixels are filtered (which means we're filtering > on DT even if MIPI_CSIx_CSIS_COMMON_CTRL[11:10] = 0x0x would suggest > filtering is not enabled) > > However corrupted pixels are still sent through. Right. But there is more: there are 3 separate "DT filters": 0x32E40040 0xB0 ISP Configuration Register (ISP_CONFIG_CH0) DT 2Ch (RAW12) 0x32E40044 0x2D00500 ISP Resolution Register (ISP_RESOLUTION_CH0) => 1280 * 720 0x32E40050 0xDC ISP Configuration Register (ISP_CONFIG_CH1) DT 37h (User Defined 8-bit Data Type 8) 0x32E40054 0x2D00500 ISP Resolution Register (ISP_RESOLUTION_CH1) => 1280 * 720 0x32E40060 0x48 ISP Configuration Register (ISP_CONFIG_CH2) DT 12h (Embedded 8-bit non Image Data) 0x32E40064 0x2D00500 ISP Resolution Register (ISP_RESOLUTION_CH2) => 1280 * 720 The 4th set looks the same, but doesn't appear to have its SHADOW register set, so I'll ignore it for now. I'm ignoring the SYNC registers as well (offsets ...0x58) - they are zeroed. It appears the 2 LS bits of ISP_CONFIG_CH* are the number of some output port. #0 goes to ISP, not sure about the others. Will have a look. I.e., I can disable CH0 and use CH1 or CH2 to feed image data do ISP - it works. MIPI_CSIx_CSIS_COMMON_CTRL: Bits 18, 17 and 16 respectively reload SHADOW registers for CH2, CH1 and CH0. For these tests, I have to use "devmem write 32-bit CSIS_COMMON_CTRL 0x7xxxx" instead of 0x1xxxx so that all 3 shadow sets are updated (and not only #0, the one in the docs and used by the drivers). 0x32E40080 0xB0 Shadow Configuration Register (SHADOW_CONFIG_CH0) DT 2Ch (RAW12) 0x32E40084 0x2D00500 Shadow Resolution Register (SHADOW_RESOLUTION_CH0) => 1280 * 720 0x32E40090 0xDC Shadow Configuration Register (SHADOW_CONFIG_CH1) DT 37h (User Defined 8-bit Data Type 8) 0x32E40094 0x2D00500 Shadow Resolution Register (SHADOW_RESOLUTION_CH1) => 1280 * 720 0x32E400A0 0x48 Shadow Configuration Register (SHADOW_CONFIG_CH2) DT 12h (Embedded 8-bit non Image Data) 0x32E400A4 0x2D00500 Shadow Resolution Register (SHADOW_RESOLUTION_CH2) => 1280 * 720 Also: 0x32E40100 0x5F10 Frame Counter (FRAME_COUNTER_CH0) 0x32E40104 0x57C4 Frame Counter (FRAME_COUNTER_CH1) 0x32E40108 0x33CF Frame Counter (FRAME_COUNTER_CH2) Data selected by all 3 sets is somehow fed to the ISP. Now the data isn't simply appended to the previous fragment. It seems the DT 37h lines (which appear before the actual image data on the MIPI bus) somehow overwrite (only) the first line of the image. I'm looking at it. INTERLEAVE_MODE = 0 or 2 => only CH0 is used, the first 32 bit in the image on CSI1 are corrupted. INTERLEAVE_MODE = 1 => all 3 CHannels are used, no corruption There appear to be some subtle differences between 0 and 2, and 1 vs 3. > If you > > - MIPI_CSIx_CSIS_COMMON_CTRL[11:10] > "Select Interleave mode" = 0x01 = DT (Data type) only > > Embedded data and OB pixels are still filtered, and your corrupt > pixels are filtered as well. Right. > Now, why are "corrupted pixels" filtered away by enabling this option ? > > As far as I understand bad pixels are corrupt in their data values, > the CSI-2 packet header which contains the DT is correct. Is my > understanding correct ? I think so. Maybe the corruption happens after the packets are received, on the 32-bit internal bus maybe. > It would be nice to actually understand what it does before enabling > it unconditionally. Still trying :-) Next Monday, maybe. Especially I don't know how do I receive multiple DT types, including 12-bit RAW pixels and 8-bit user data. I know it wasn't probably designed for this, but nevertheless.
diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c index 29523bb84d95..d53a4262b63d 100644 --- a/drivers/media/platform/nxp/imx-mipi-csis.c +++ b/drivers/media/platform/nxp/imx-mipi-csis.c @@ -654,8 +654,7 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis, val = mipi_csis_read(csis, MIPI_CSIS_CMN_CTRL); val &= ~MIPI_CSIS_CMN_CTRL_LANE_NR_MASK; val |= (lanes - 1) << MIPI_CSIS_CMN_CTRL_LANE_NR_OFFSET; - if (csis->info->version == MIPI_CSIS_V3_3) - val |= MIPI_CSIS_CMN_CTRL_INTER_MODE; + val |= MIPI_CSIS_CMN_CTRL_INTER_MODE; /* enable filtering by DT */ mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL, val); __mipi_csis_set_format(csis, format, csis_fmt);
In addition to raw image data, certain MIPI sensors send additional information like NULL packets or "embedded 8-bit non-image data". Without DT (data type) filtering, these packets end up in the frame buffer, corrupting it. Tested on i.MX8MP with IMX290 sensor. Signed-off-by: Krzysztof Hałasa <khalasa@piap.pl>