All of lore.kernel.org
 help / color / mirror / Atom feed
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Martin Kepplinger <martin.kepplinger@puri.sm>
Cc: devicetree@vger.kernel.org, festevam@gmail.com,
	kernel@pengutronix.de, linux-imx@nxp.com,
	linux-media@vger.kernel.org, marex@denx.de,
	p.zabel@pengutronix.de, rmfrfs@gmail.com, robh@kernel.org,
	slongerbeam@gmail.com
Subject: Re: [PATCH 00/23] media: imx: imx7-mipi-csis: Add i.MX8MM support / imx8mq support
Date: Fri, 21 May 2021 16:36:41 +0300	[thread overview]
Message-ID: <YKe3aQHr+BsJ3ueY@pendragon.ideasonboard.com> (raw)
In-Reply-To: <bd5dc783e39d750693ac2b49050681c5e3088330.camel@puri.sm>

Hi Martin,

On Fri, May 21, 2021 at 01:02:30PM +0200, Martin Kepplinger wrote:
> Am Freitag, dem 21.05.2021 um 12:43 +0300 schrieb Laurent Pinchart:
> > On Fri, May 21, 2021 at 11:25:20AM +0200, Martin Kepplinger wrote:
> > > Am Donnerstag, dem 20.05.2021 um 15:37 +0300 schrieb Laurent Pinchart:
> > > > On Thu, May 20, 2021 at 12:54:27PM +0200, Martin Kepplinger wrote:
> > > > > Am Mittwoch, dem 19.05.2021 um 18:46 +0300 schrieb Laurent Pinchart:
> > > > > > On Wed, May 19, 2021 at 05:21:11PM +0200, Martin Kepplinger wrote:
> > > > > > > Am Mittwoch, dem 19.05.2021 um 04:14 +0300 schrieb Laurent Pinchart:
> > > > > > > > On Tue, May 18, 2021 at 04:39:00PM +0200, Martin Kepplinger wrote:
> > > > > > > > > Am Sonntag, dem 16.05.2021 um 01:55 +0300 schrieb Laurent Pinchart:
> > > > > > > > > > On Tue, May 04, 2021 at 05:59:39PM +0200, Martin Kepplinger wrote:
> > > > 
> > > > [snip]
> > > > 
> > > > > I fixed mipi -> csi link. I had the DT port descriptions for
> > > > > mipi csi wrong.
> > > > 
> > > > \o/
> > > > 
> > > > > now, just because I think it makes sense, I do:
> > > > > 
> > > > > media-ctl --set-v4l2 "'csi':0 [fmt:SGBRG10/640x480
> > > > > colorspace:raw]"
> > > > > 
> > > > > which now prints:
> > > > > 
> > > > > Device topology
> > > > > - entity 1: csi (2 pads, 2 links)
> > > > >             type V4L2 subdev subtype Unknown flags 0
> > > > >             device node name /dev/v4l-subdev0
> > > > >         pad0: Sink
> > > > >                 [fmt:SGBRG10_1X10/640x480 field:none colorspace:raw xfer:none ycbcr:601 quantization:full-range]
> > > > >                 <- "imx8mq-mipi-csis.0":1 [ENABLED,IMMUTABLE]
> > > > >         pad1: Source
> > > > >                 [fmt:SGBRG10_1X10/640x480 field:none colorspace:raw xfer:none ycbcr:601 quantization:full-range]
> > > > >                 -> "csi capture":0 [ENABLED,IMMUTABLE]
> > > > > 
> > > > > - entity 4: csi capture (1 pad, 1 link)
> > > > >             type Node subtype V4L flags 0
> > > > >             device node name /dev/video1
> > > > >         pad0: Sink
> > > > >                 <- "csi":1 [ENABLED,IMMUTABLE]
> > > > > 
> > > > > - entity 10: imx8mq-mipi-csis.0 (2 pads, 2 links)
> > > > >              type V4L2 subdev subtype Unknown flags 0
> > > > >              device node name /dev/v4l-subdev1
> > > > >         pad0: Sink
> > > > >                 <- "hi846 2-0020":0 []
> > > > >         pad1: Source
> > > > >                 -> "csi":0 [ENABLED,IMMUTABLE]
> > > > 
> > > > This subdev doesn't seem to report formats on its sink and source pads,
> > > > which is weird. I've had a quick look at the .get_fmt() and .set_fmt()
> > > > implementations in the code you've posted, and they're wrong. They
> > > > shouldn't pass the calls to the source subdev with v4l2_subdev_call(),
> > > > they should instead implement get and set format on this subdev. You can
> > > > look at the imx7-mipi-csis driver to see how that's done. Once you'll
> > > > have fixed this, you'll have to set the format on each pad with
> > > > media-ctl to make sure formats through the pipeline match.
> > > > 
> > > > The only location where you imx8mq-mipi-csis driver should use
> > > > v4l2_subdev_call() is in .s_stream(), to propagate the operation to the
> > > > source.
> > > > 
> > > > By the way, I'd replace every occurence of "csis" with "csi2" in your
> > > > driver. The name "csis" in the i.MX7 driver comes from the CSI-2 RX IP
> > > > core that is named CSIS. That's not the case on the i.MX8QM.
> > > > 
> > > > > - entity 15: hi846 2-0020 (1 pad, 1 link)
> > > > >              type V4L2 subdev subtype Sensor flags 0
> > > > >              device node name /dev/v4l-subdev2
> > > > >         pad0: Source
> > > > >                 [fmt:SGBRG10_1X10/640x480 field:none colorspace:raw]
> > > > >                 -> "imx8mq-mipi-csis.0":0 []
> > > > 
> > > > You need to enable this link, the following should do
> > > > 
> > > > media-ctl -l "'hi846 2-0020':0 -> 'imx8mq-mipi-csis.0':0 [1]"
> > > 
> > > ok makes sense, even though I basically just allow a set of formats
> > > without yet having to configure anything format-specific (I can at
> > > least use bits-per-pixel later, so it makes sense to have them).
> > > nevermind. I again append the current driver I use here.
> > > 
> > > then I do:
> > > 
> > > media-ctl --set-v4l2 "'csi':0 [fmt:SGBRG10/640x480 colorspace:raw]"
> > > media-ctl --set-v4l2 "'imx8mq-mipi-csi2.0':0 [fmt:SGBRG10/640x480
> > > colorspace:raw]"
> > > media-ctl -l "'hi846 2-0020':0 -> 'imx8mq-mipi-csi2.0':0 [1]"
> > > 
> > > which gets me:
> > > 
> > > Device topology
> > > - entity 1: csi (2 pads, 2 links)
> > >             type V4L2 subdev subtype Unknown flags 0
> > >             device node name /dev/v4l-subdev0
> > >         pad0: Sink
> > >                 [fmt:SGBRG10_1X10/640x480 field:none colorspace:raw xfer:none ycbcr:601 quantization:full-range]
> > >                 <- "imx8mq-mipi-csi2.0":1 [ENABLED,IMMUTABLE]
> > >         pad1: Source
> > >                 [fmt:SGBRG10_1X10/640x480 field:none colorspace:raw xfer:none ycbcr:601 quantization:full-range]
> > >                 -> "csi capture":0 [ENABLED,IMMUTABLE]
> > > 
> > > - entity 4: csi capture (1 pad, 1 link)
> > >             type Node subtype V4L flags 0
> > >             device node name /dev/video0
> > >         pad0: Sink
> > >                 <- "csi":1 [ENABLED,IMMUTABLE]
> > > 
> > > - entity 10: imx8mq-mipi-csi2.0 (2 pads, 2 links)
> > >              type V4L2 subdev subtype Unknown flags 0
> > >              device node name /dev/v4l-subdev1
> > >         pad0: Sink
> > >                 [fmt:SGBRG10_1X10/640x480]
> > >                 <- "hi846 2-0020":0 [ENABLED]
> > >         pad1: Source
> > >                 [fmt:SGBRG10_1X10/640x480]
> > >                 -> "csi":0 [ENABLED,IMMUTABLE]
> > > 
> > > - entity 15: hi846 2-0020 (1 pad, 1 link)
> > >              type V4L2 subdev subtype Sensor flags 0
> > >              device node name /dev/v4l-subdev2
> > >         pad0: Source
> > >                 [fmt:SGBRG10_1X10/640x480 field:none colorspace:raw]
> > >                 -> "imx8mq-mipi-csi2.0":0 [ENABLED]
> > 
> > This looks better.
> > 
> > > but streaming still fails with:
> > > 
> > > [  352.255129] imx7-csi 30a90000.csi1_bridge: media bus code not
> > > compatible with the pixel format set on the video node: 1 != 0
> > 
> > What is the capture command line ? Can you trace this (I assume the
> > message is printed by capture_validate_fmt(), it's not present in
> > mainline so I don't know what 1 and 0 correspond to, even though I
> > suspect they would be IPUV3_COLORSPACE_* values) to see why it fails
> > ?
> 
> capture command:
> 
> v4l2-ctl -d "/dev/v4l/by-path/platform-30a90000.csi1_bridge-video-
> index0" --set-fmt-video=width=640,height=480 --stream-mmap --stream-
> to=test.raw --stream-count=1
> 
> I'll have to continue after the weekend, but let's share some logs.
> Yes, "1 != 0" is from capture_validate_fmt():
> 
> priv->vdev.cc->cs != cc->cs
> 
> When I print the format the imx_media_find_mbus_format() finds and do:
> 
> media-ctl --set-v4l2 "'csi':0 [fmt:SGBRG10/640x480 colorspace:raw]"
> 
> I see:
> 
> [  184.251144] mc: media_release: Media Release
> [  184.254397] selected specific mbus code 0 for list nr 0 (fourcc 0x59565955)
> [  184.264564] selected specific mbus code 0 for list nr 0 (fourcc 0x59565955)
> [  184.274763] selected specific mbus code 0 for list nr 21 (fourcc 0x36314247)
> [  184.285102] selected specific mbus code 0 for list nr 21 (fourcc 0x36314247)
> [  184.295383] selected specific mbus code 0 for list nr 21 (fourcc 0x36314247)
> [  184.305752] selected specific mbus code 0 for list nr 21 (fourcc 0x36314247)
> 
> 21 is the correct bayer format I want, but there's 0, so
> "MEDIA_BUS_FMT_UYVY8_2X8" found the first 2 times. That is
> IPUV3_COLORSPACE_YUV (1) while the correct Bayer format 21 is
> IPUV3_COLORSPACE_RGB (0).
> 
> so some format settings not yet correct.

You need to specify the capture pixel format to v4l2-ctl. The driver
defaults to YUYV (I think) otherwise. The CSI bridge will pad data with
0's on the right, so you need SGBRG16 (if I recall correctly, try
SGRBG10 if it doesn't work).

-- 
Regards,

Laurent Pinchart

  reply	other threads:[~2021-05-21 13:36 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-13  2:29 [PATCH 00/23] media: imx: imx7-mipi-csis: Add i.MX8MM support Laurent Pinchart
2021-04-13  2:29 ` [PATCH 01/23] media: imx: imx7_mipi_csis: Fix logging of only error event counters Laurent Pinchart
2021-04-26 11:01   ` Frieder Schrempf
2021-05-15 21:54     ` Laurent Pinchart
2021-04-13  2:29 ` [PATCH 02/23] media: imx: imx7_mipi_csis: Count the CSI-2 debug interrupts Laurent Pinchart
2021-04-26 11:39   ` Frieder Schrempf
2021-04-13  2:29 ` [PATCH 03/23] media: imx: imx7_mipi_csis: Update ISP_CONFIG macros for quad pixel mode Laurent Pinchart
2021-04-26 11:41   ` Frieder Schrempf
2021-04-13  2:29 ` [PATCH 04/23] media: imx: imx7_mipi_csis: Move static data to top of mipi_csis_dump_regs() Laurent Pinchart
2021-04-26 11:46   ` Frieder Schrempf
2021-04-13  2:29 ` [PATCH 05/23] media: imx: imx7_mipi_csis: Minimize locking in get/set format Laurent Pinchart
2021-04-13  2:29 ` [PATCH 06/23] media: imx: imx7_mipi_csis: Don't set subdev data Laurent Pinchart
2021-04-13  2:29 ` [PATCH 07/23] media: imx: imx7-mipi-csis: Reorganize code in sections Laurent Pinchart
2021-04-13  2:29 ` [PATCH 08/23] media: imx: imx7_mipi_csis: Set the CLKSETTLE register field Laurent Pinchart
2021-04-13  2:30 ` [PATCH 09/23] media: imx: imx7_mipi_csis: Drop unused csis_hw_reset structure Laurent Pinchart
2021-04-13  2:30 ` [PATCH 10/23] media: imx: imx7_mipi_csis: Store CSI-2 data type in format structure Laurent Pinchart
2021-04-13  2:30 ` [PATCH 11/23] media: imx: imx7_mipi_csis: Drop csi_state phy field Laurent Pinchart
2021-04-13  2:30 ` [PATCH 12/23] media: imx: imx7_mipi_csis: Rename mipi_sd to sd Laurent Pinchart
2021-04-13  2:30 ` [PATCH 13/23] media: imx: imx7_mipi_csis: Rename csi_state flag field to state Laurent Pinchart
2021-04-13  2:30 ` [PATCH 14/23] media: imx: imx7_mipi_csis: Turn csi_state irq field into local variable Laurent Pinchart
2021-04-13  2:30 ` [PATCH 15/23] media: imx: imx7_mipi_csis: Don't pass pdev to mipi_csis_parse_dt() Laurent Pinchart
2021-04-13  2:30 ` [PATCH 16/23] media: imx: imx7_mipi_csis: Pass csi_state to mipi_csis_subdev_init() Laurent Pinchart
2021-04-13  2:30 ` [PATCH 17/23] media: imx: imx7_mipi_csis: Drop csi_state pdev field Laurent Pinchart
2021-04-13  2:30 ` [PATCH 18/23] media: imx: imx7_mipi_csis: Make csi_state num_clocks field unsigned Laurent Pinchart
2021-04-13  2:30 ` [PATCH 19/23] media: imx: imx7_mipi_csis: Reorganize csi_state structure Laurent Pinchart
2021-04-13  2:30 ` [PATCH 20/23] media: imx: imx7_mipi_csis: Reorganize mipi_csis_probe() Laurent Pinchart
2021-04-13  2:30 ` [PATCH 21/23] media: imx: imx7_mipi_csis: Reject invalid data-lanes settings Laurent Pinchart
2021-04-13  2:30 ` [PATCH 22/23] dt-bindings: media: nxp,imx7-mipi-csi2: Add i.MX8MM support Laurent Pinchart
2021-04-13 16:00   ` Rob Herring
2021-04-18 20:15   ` [PATCH v1.1 " Laurent Pinchart
2021-04-13  2:30 ` [PATCH 23/23] media: imx: imx7_mipi_csis: " Laurent Pinchart
2021-04-27 10:57   ` Marco Felsch
2021-05-15 22:10     ` Laurent Pinchart
2021-04-15  9:27 ` [PATCH 00/23] media: imx: imx7-mipi-csis: " Rui Miguel Silva
2021-04-18 20:21   ` Laurent Pinchart
2021-04-18 20:14 ` [PATCH 24/23] media: imx: imx7_mipi_csis: Update MAINTAINERS Laurent Pinchart
2021-04-18 22:22   ` Rui Miguel Silva
2021-04-21 15:27 ` [PATCH 00/23] media: imx: imx7-mipi-csis: Add i.MX8MM support Tim Harvey
2021-04-26 10:35   ` Frieder Schrempf
2021-04-27 11:00     ` Marco Felsch
2021-05-15 22:46       ` Laurent Pinchart
2021-05-04 15:59 ` [PATCH 00/23] media: imx: imx7-mipi-csis: Add i.MX8MM support / imx8mq support Martin Kepplinger
2021-05-15 22:55   ` Laurent Pinchart
2021-05-18 14:39     ` Martin Kepplinger
2021-05-19  1:14       ` Laurent Pinchart
2021-05-19  9:33         ` Martin Kepplinger
2021-05-19 15:21         ` Martin Kepplinger
2021-05-19 15:46           ` Laurent Pinchart
2021-05-20 10:54             ` Martin Kepplinger
2021-05-20 12:37               ` Laurent Pinchart
2021-05-21  9:25                 ` Martin Kepplinger
2021-05-21  9:43                   ` Laurent Pinchart
2021-05-21 11:02                     ` Martin Kepplinger
2021-05-21 13:36                       ` Laurent Pinchart [this message]
2021-05-25  7:32                         ` Martin Kepplinger
2021-05-25 11:25                           ` Laurent Pinchart

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YKe3aQHr+BsJ3ueY@pendragon.ideasonboard.com \
    --to=laurent.pinchart@ideasonboard.com \
    --cc=devicetree@vger.kernel.org \
    --cc=festevam@gmail.com \
    --cc=kernel@pengutronix.de \
    --cc=linux-imx@nxp.com \
    --cc=linux-media@vger.kernel.org \
    --cc=marex@denx.de \
    --cc=martin.kepplinger@puri.sm \
    --cc=p.zabel@pengutronix.de \
    --cc=rmfrfs@gmail.com \
    --cc=robh@kernel.org \
    --cc=slongerbeam@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.