Message-ID: <iegnn5xoosqpk52hvipcr73aliwhqtsq6r6ctvt5756bhy6yen@rqcdongb7fdf>
Date: Wed, 21 May 2025 11:08:13 +0200
From: Jacopo Mondi <jacopo.mondi@...asonboard.com>
To: Krzysztof Hałasa <khalasa@...p.pl>
Cc: Laurent Pinchart <laurent.pinchart@...asonboard.com>, 
	Rui Miguel Silva <rmfrfs@...il.com>, Martin Kepplinger <martink@...teo.de>, 
	Purism Kernel Team <kernel@...i.sm>, Mauro Carvalho Chehab <mchehab@...nel.org>, 
	Shawn Guo <shawnguo@...nel.org>, Sascha Hauer <s.hauer@...gutronix.de>, 
	Pengutronix Kernel Team <kernel@...gutronix.de>, Fabio Estevam <festevam@...il.com>, linux-media@...r.kernel.org, 
	imx@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Enable MIPI filtering by DT on i.MX8M*

Hi Krzysztof, Laurent
On Tue, May 20, 2025 at 02:35:18PM +0200, Krzysztof Hałasa wrote:

> Laurent Pinchart <laurent.pinchart@...asonboard.com> writes:

Nit: the patch subject should be something like

media: imx-mipi-csis: Enable DT filerting

>
> >> +++ 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.

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...

>
> 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.

Embedded data != null and blanking
They have DT=0x12 and not 0x10 or 0x11

>
> 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)

some registers like 32E4_000C are not listed in the peripheral memory
map, so you're probably reading an invalid memory area there

>
> 32E40000:  3060301     4705    F0000 DEADCAFE

I presume these are:

32E4_0000 = version ? (not documented in my version of TRM)
32E4_0004 = (MIPI_CSI1_CSIS_COMMON_CTRL)
32E4_0008 = ((MIPI_CSI1_CSIS_CLOCK_CTRL))
32E4_000C = invalid address


> 32E40010: FFFFFFFF        0        0        0


> 32E40020:       F0  900001F DEADCAFE DEADCAFE
> 32E40030:      1F4        0        0        0
> 32E40040:       B0  4380780        0 DEADCAFE <<< ISP_CONFIG0

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


> 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

etc etc

> 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.

The ISP filtering register only supports image formats, not embedded
data.

>
> Perhaps the CSIC internally has 3 output ports, but only the first one

The CSIS seems to have multiple channels indeed, but at the moment
only #0 seems to be supported.

We have been using 8mp extensively with sensors that produce embedded
data and afaict ED are not in the final image.

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 ?

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.

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.

Let's find out why this doesn't happen...

> is connected to ISI and ISP?
> --
> Krzysztof "Chris" Hałasa
>
> Sieć Badawcza Łukasiewicz
> Przemysłowy Instytut Automatyki i Pomiarów PIAP
> Al. Jerozolimskie 202, 02-486 Warszawa
>

Powered by blists - more mailing lists