From: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
To: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Cc: linux-media@vger.kernel.org,
Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
Sakari Ailus <sakari.ailus@linux.intel.com>,
Kieran Bingham <kieran.bingham@ideasonboard.com>,
Jai Luthra <j-luthra@ti.com>, Vaishnav Achath <vaishnav.a@ti.com>
Subject: Re: [PATCH v1 1/3] media: ti: cal: Add support for 1X16 mbus formats
Date: Fri, 24 Feb 2023 11:02:44 +0200 [thread overview]
Message-ID: <49b1eca2-8997-0ff1-3151-d9656ef16fb8@ideasonboard.com> (raw)
In-Reply-To: <20230223180325.zicvisvqf6vv3ypa@uno.localdomain>
[-- Attachment #1: Type: text/plain, Size: 3085 bytes --]
On 23/02/2023 20:03, Jacopo Mondi wrote:
> Hi Tomi
>
> On Thu, Feb 23, 2023 at 07:52:48PM +0200, Tomi Valkeinen wrote:
>> On 23/02/2023 19:10, Jacopo Mondi wrote:
>>> Hi Tomi
>>>
>>> On Wed, Feb 22, 2023 at 02:56:28PM +0200, Tomi Valkeinen wrote:
>>>> For legacy reasons the CAL driver uses 2X8 mbus formats for the CSI-2
>>>> link (e.g. MEDIA_BUS_FMT_YUYV8_2X8). 1X16 formats are more correct and
>>>> used by many drivers, so add 1X16 support to CAL.
>>>>
>>>> We keep the 2X8 formats for backward compatibility.
>>>
>>> Eh, this is the usual question about what we should consider a
>>> to be a userspace breakage or not.
>>>
>>> I presume the reason to maintain 2X8 formats is (assuming the CAL
>>> interface is CSI-2 only, right ?) because sensor drivers that only
>>> support 2X8 formats will then fail to link_validate() if you remove
>>
>> Yes.
>>
>>> all 2X8 formats here. I'm of the opinion that we should bite the
>>> bullet and move to 1X16. If any driver that used to work with CAL now
>>> breaks, the sensor driver will have to be fixed.
>>>
>>> In my humble experience, that's what we did with the ov5640 sensor. We
>>
>> Yes, you did.
>>
>>> removed the 2X8 formats in CSI-2 mode and some platform driver broke
>>> and then had been fixed (it happened in the same release cycle), win win.
>>
>> No, CAL is still broken =). OV5640 is the only sensor I have. I just haven't
>> had time to look at this and fix it (and no one has complained).
>>
>
> Ups, I was thinking at the ST and microchip receivers, I thought CAL
> was fixed already :)
>
> See ? win win (almost)
>
>>> I know it's controversial, let's see what others think.
>>
>> I'm all for dropping the 2X8 formats, if that's the consensus. It would keep
>> the driver simpler, as we could keep the 1:1 relationship between pixel
>> formats and mbus codes.
>>
>> I'll look at your other comments tomorrow.
>>
>
> And I'll look at your last patch tomorrow if I can get a media graph
> dump!
I have attached the txt and dot versions of the graph of my FPDLink
setup with three cameras (MC mode with streams).
Some clarifications, which may not be obvious:
The current upstream driver supports two distinct modes, non-MC (legacy)
and MC modes, selectable with the cal_mc_api module parameter.
Supporting the legacy mode does make the driver rather confusing in some
areas...
With this series, the non-MC mode is unchanged, but the MC mode will be
extended to support streams.
The non-MC mode (afaics, I have never much used it) only supports a
simple setup with a single sensor subdevice connected directly to CAL.
As we don't have MC, the subdevice is not visible to the userspace, and
thus the CAL driver does tricks like collects the controls from the
sensor, and exposes them from CAL's video node.
And this is why we have two sets of ioctl ops in cal-video.c,
cal_ioctl_legacy_ops and cal_ioctl_mc_ops, as the behavior is quite
different between the two modes.
Describing this makes me wonder if the non-MC mode could be dropped...
MC mode has been supported for some years now.
Tomi
[-- Attachment #2: graph.dot --]
[-- Type: application/msword-template, Size: 8119 bytes --]
[-- Attachment #3: graph.txt --]
[-- Type: text/plain, Size: 12303 bytes --]
Media controller API version 6.2.0
Media device information
------------------------
driver cal
model CAL
serial
bus info platform:489b0000.cal
hw revision 0x40000300
driver version 6.2.0
Device topology
- entity 1: CAMERARX0 (9 pads, 65 links, 0 route)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev0
pad0: Sink
[stream:0 fmt:SRGGB12_1X12/1936x1100 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:lim-range]
<- "ds90ub960 4-003d":4 [ENABLED,IMMUTABLE]
pad1: Source
[stream:0 fmt:SRGGB12_1X12/1936x1100 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:lim-range]
-> "CAL output 0":0 [ENABLED]
-> "CAL output 1":0 []
-> "CAL output 2":0 []
-> "CAL output 3":0 []
-> "CAL output 4":0 []
-> "CAL output 5":0 []
-> "CAL output 6":0 []
-> "CAL output 7":0 []
pad2: Source
[stream:0 fmt:SRGGB12_1X12/1936x1100 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:lim-range]
-> "CAL output 0":0 []
-> "CAL output 1":0 [ENABLED]
-> "CAL output 2":0 []
-> "CAL output 3":0 []
-> "CAL output 4":0 []
-> "CAL output 5":0 []
-> "CAL output 6":0 []
-> "CAL output 7":0 []
pad3: Source
[stream:0 fmt:UYVY8_1X16/1280x720 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:lim-range]
-> "CAL output 0":0 []
-> "CAL output 1":0 []
-> "CAL output 2":0 [ENABLED]
-> "CAL output 3":0 []
-> "CAL output 4":0 []
-> "CAL output 5":0 []
-> "CAL output 6":0 []
-> "CAL output 7":0 []
pad4: Source
-> "CAL output 0":0 []
-> "CAL output 1":0 []
-> "CAL output 2":0 []
-> "CAL output 3":0 []
-> "CAL output 4":0 []
-> "CAL output 5":0 []
-> "CAL output 6":0 []
-> "CAL output 7":0 []
pad5: Source
[stream:0 fmt:unknown/1936x1 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:lim-range]
-> "CAL output 0":0 []
-> "CAL output 1":0 []
-> "CAL output 2":0 []
-> "CAL output 3":0 []
-> "CAL output 4":0 [ENABLED]
-> "CAL output 5":0 []
-> "CAL output 6":0 []
-> "CAL output 7":0 []
pad6: Source
[stream:0 fmt:unknown/1936x1 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:lim-range]
-> "CAL output 0":0 []
-> "CAL output 1":0 []
-> "CAL output 2":0 []
-> "CAL output 3":0 []
-> "CAL output 4":0 []
-> "CAL output 5":0 [ENABLED]
-> "CAL output 6":0 []
-> "CAL output 7":0 []
pad7: Source
[stream:0 fmt:unknown/1280x1 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:lim-range]
-> "CAL output 0":0 []
-> "CAL output 1":0 []
-> "CAL output 2":0 []
-> "CAL output 3":0 []
-> "CAL output 4":0 []
-> "CAL output 5":0 []
-> "CAL output 6":0 [ENABLED]
-> "CAL output 7":0 []
pad8: Source
-> "CAL output 0":0 []
-> "CAL output 1":0 []
-> "CAL output 2":0 []
-> "CAL output 3":0 []
-> "CAL output 4":0 []
-> "CAL output 5":0 []
-> "CAL output 6":0 []
-> "CAL output 7":0 []
- entity 11: CAMERARX1 (9 pads, 64 links, 0 route)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev1
pad0: Sink
[stream:0 fmt:UYVY8_2X8/640x480 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:lim-range]
pad1: Source
[stream:0 fmt:UYVY8_2X8/640x480 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:lim-range]
-> "CAL output 0":0 []
-> "CAL output 1":0 []
-> "CAL output 2":0 []
-> "CAL output 3":0 []
-> "CAL output 4":0 []
-> "CAL output 5":0 []
-> "CAL output 6":0 []
-> "CAL output 7":0 []
pad2: Source
-> "CAL output 0":0 []
-> "CAL output 1":0 []
-> "CAL output 2":0 []
-> "CAL output 3":0 []
-> "CAL output 4":0 []
-> "CAL output 5":0 []
-> "CAL output 6":0 []
-> "CAL output 7":0 []
pad3: Source
-> "CAL output 0":0 []
-> "CAL output 1":0 []
-> "CAL output 2":0 []
-> "CAL output 3":0 []
-> "CAL output 4":0 []
-> "CAL output 5":0 []
-> "CAL output 6":0 []
-> "CAL output 7":0 []
pad4: Source
-> "CAL output 0":0 []
-> "CAL output 1":0 []
-> "CAL output 2":0 []
-> "CAL output 3":0 []
-> "CAL output 4":0 []
-> "CAL output 5":0 []
-> "CAL output 6":0 []
-> "CAL output 7":0 []
pad5: Source
-> "CAL output 0":0 []
-> "CAL output 1":0 []
-> "CAL output 2":0 []
-> "CAL output 3":0 []
-> "CAL output 4":0 []
-> "CAL output 5":0 []
-> "CAL output 6":0 []
-> "CAL output 7":0 []
pad6: Source
-> "CAL output 0":0 []
-> "CAL output 1":0 []
-> "CAL output 2":0 []
-> "CAL output 3":0 []
-> "CAL output 4":0 []
-> "CAL output 5":0 []
-> "CAL output 6":0 []
-> "CAL output 7":0 []
pad7: Source
-> "CAL output 0":0 []
-> "CAL output 1":0 []
-> "CAL output 2":0 []
-> "CAL output 3":0 []
-> "CAL output 4":0 []
-> "CAL output 5":0 []
-> "CAL output 6":0 []
-> "CAL output 7":0 []
pad8: Source
-> "CAL output 0":0 []
-> "CAL output 1":0 []
-> "CAL output 2":0 []
-> "CAL output 3":0 []
-> "CAL output 4":0 []
-> "CAL output 5":0 []
-> "CAL output 6":0 []
-> "CAL output 7":0 []
- entity 21: ds90ub960 4-003d (6 pads, 4 links, 0 route)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev2
pad0: Sink
[stream:0 fmt:SRGGB12_1X12/1936x1100 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:lim-range]
<- "ds90ub953 4-0044":1 [ENABLED,IMMUTABLE]
pad1: Sink
[stream:0 fmt:SRGGB12_1X12/1936x1100 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:lim-range]
<- "ds90ub953 4-0045":1 [ENABLED,IMMUTABLE]
pad2: Sink
[stream:0 fmt:UYVY8_1X16/1280x720 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:lim-range]
<- "ds90ub913a 4-0046":1 [ENABLED,IMMUTABLE]
pad3: Sink
pad4: Source
[stream:0 fmt:SRGGB12_1X12/1936x1100 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:lim-range]
-> "CAMERARX0":0 [ENABLED,IMMUTABLE]
pad5: Source
- entity 30: ds90ub913a 4-0046 (2 pads, 2 links, 0 route)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev3
pad0: Sink
[stream:0 fmt:UYVY8_2X8/1280x720 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:lim-range]
<- "ov10635 7-0030":0 [ENABLED,IMMUTABLE]
pad1: Source
[stream:0 fmt:UYVY8_1X16/1280x720 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:lim-range]
-> "ds90ub960 4-003d":2 [ENABLED,IMMUTABLE]
- entity 35: ds90ub953 4-0045 (2 pads, 2 links, 0 route)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev4
pad0: Sink
[stream:0 fmt:SRGGB12_1X12/1936x1100 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:lim-range]
<- "imx390 6-0021":0 [ENABLED,IMMUTABLE]
pad1: Source
[stream:0 fmt:SRGGB12_1X12/1936x1100 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:lim-range]
-> "ds90ub960 4-003d":1 [ENABLED,IMMUTABLE]
- entity 40: ds90ub953 4-0044 (2 pads, 2 links, 0 route)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev5
pad0: Sink
[stream:0 fmt:SRGGB12_1X12/1936x1100 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:lim-range]
<- "imx390 5-0021":0 [ENABLED,IMMUTABLE]
pad1: Source
[stream:0 fmt:SRGGB12_1X12/1936x1100 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:lim-range]
-> "ds90ub960 4-003d":0 [ENABLED,IMMUTABLE]
- entity 45: imx390 5-0021 (1 pad, 1 link, 0 route)
type V4L2 subdev subtype Sensor flags 0
device node name /dev/v4l-subdev6
pad0: Source
[stream:0 fmt:SRGGB12_1X12/1936x1100 field:none colorspace:smpte170m]
-> "ds90ub953 4-0044":0 [ENABLED,IMMUTABLE]
- entity 49: imx390 6-0021 (1 pad, 1 link, 0 route)
type V4L2 subdev subtype Sensor flags 0
device node name /dev/v4l-subdev7
pad0: Source
[stream:0 fmt:SRGGB12_1X12/1936x1100 field:none colorspace:smpte170m]
-> "ds90ub953 4-0045":0 [ENABLED,IMMUTABLE]
- entity 53: ov10635 7-0030 (1 pad, 1 link, 0 route)
type V4L2 subdev subtype Sensor flags 0
device node name /dev/v4l-subdev8
pad0: Source
[stream:0 fmt:UYVY8_2X8/1280x720 field:none colorspace:smpte170m]
-> "ds90ub913a 4-0046":0 [ENABLED,IMMUTABLE]
- entity 57: CAL output 0 (1 pad, 16 links, 0 route)
type Node subtype V4L flags 0
device node name /dev/video0
pad0: Sink
<- "CAMERARX0":1 [ENABLED]
<- "CAMERARX0":2 []
<- "CAMERARX0":3 []
<- "CAMERARX0":4 []
<- "CAMERARX0":5 []
<- "CAMERARX0":6 []
<- "CAMERARX0":7 []
<- "CAMERARX0":8 []
<- "CAMERARX1":1 []
<- "CAMERARX1":2 []
<- "CAMERARX1":3 []
<- "CAMERARX1":4 []
<- "CAMERARX1":5 []
<- "CAMERARX1":6 []
<- "CAMERARX1":7 []
<- "CAMERARX1":8 []
- entity 93: CAL output 1 (1 pad, 16 links, 0 route)
type Node subtype V4L flags 0
device node name /dev/video1
pad0: Sink
<- "CAMERARX0":1 []
<- "CAMERARX0":2 [ENABLED]
<- "CAMERARX0":3 []
<- "CAMERARX0":4 []
<- "CAMERARX0":5 []
<- "CAMERARX0":6 []
<- "CAMERARX0":7 []
<- "CAMERARX0":8 []
<- "CAMERARX1":1 []
<- "CAMERARX1":2 []
<- "CAMERARX1":3 []
<- "CAMERARX1":4 []
<- "CAMERARX1":5 []
<- "CAMERARX1":6 []
<- "CAMERARX1":7 []
<- "CAMERARX1":8 []
- entity 129: CAL output 2 (1 pad, 16 links, 0 route)
type Node subtype V4L flags 0
device node name /dev/video2
pad0: Sink
<- "CAMERARX0":1 []
<- "CAMERARX0":2 []
<- "CAMERARX0":3 [ENABLED]
<- "CAMERARX0":4 []
<- "CAMERARX0":5 []
<- "CAMERARX0":6 []
<- "CAMERARX0":7 []
<- "CAMERARX0":8 []
<- "CAMERARX1":1 []
<- "CAMERARX1":2 []
<- "CAMERARX1":3 []
<- "CAMERARX1":4 []
<- "CAMERARX1":5 []
<- "CAMERARX1":6 []
<- "CAMERARX1":7 []
<- "CAMERARX1":8 []
- entity 165: CAL output 3 (1 pad, 16 links, 0 route)
type Node subtype V4L flags 0
device node name /dev/video3
pad0: Sink
<- "CAMERARX0":1 []
<- "CAMERARX0":2 []
<- "CAMERARX0":3 []
<- "CAMERARX0":4 []
<- "CAMERARX0":5 []
<- "CAMERARX0":6 []
<- "CAMERARX0":7 []
<- "CAMERARX0":8 []
<- "CAMERARX1":1 []
<- "CAMERARX1":2 []
<- "CAMERARX1":3 []
<- "CAMERARX1":4 []
<- "CAMERARX1":5 []
<- "CAMERARX1":6 []
<- "CAMERARX1":7 []
<- "CAMERARX1":8 []
- entity 201: CAL output 4 (1 pad, 16 links, 0 route)
type Node subtype V4L flags 0
device node name /dev/video4
pad0: Sink
<- "CAMERARX0":1 []
<- "CAMERARX0":2 []
<- "CAMERARX0":3 []
<- "CAMERARX0":4 []
<- "CAMERARX0":5 [ENABLED]
<- "CAMERARX0":6 []
<- "CAMERARX0":7 []
<- "CAMERARX0":8 []
<- "CAMERARX1":1 []
<- "CAMERARX1":2 []
<- "CAMERARX1":3 []
<- "CAMERARX1":4 []
<- "CAMERARX1":5 []
<- "CAMERARX1":6 []
<- "CAMERARX1":7 []
<- "CAMERARX1":8 []
- entity 237: CAL output 5 (1 pad, 16 links, 0 route)
type Node subtype V4L flags 0
device node name /dev/video5
pad0: Sink
<- "CAMERARX0":1 []
<- "CAMERARX0":2 []
<- "CAMERARX0":3 []
<- "CAMERARX0":4 []
<- "CAMERARX0":5 []
<- "CAMERARX0":6 [ENABLED]
<- "CAMERARX0":7 []
<- "CAMERARX0":8 []
<- "CAMERARX1":1 []
<- "CAMERARX1":2 []
<- "CAMERARX1":3 []
<- "CAMERARX1":4 []
<- "CAMERARX1":5 []
<- "CAMERARX1":6 []
<- "CAMERARX1":7 []
<- "CAMERARX1":8 []
- entity 273: CAL output 6 (1 pad, 16 links, 0 route)
type Node subtype V4L flags 0
device node name /dev/video6
pad0: Sink
<- "CAMERARX0":1 []
<- "CAMERARX0":2 []
<- "CAMERARX0":3 []
<- "CAMERARX0":4 []
<- "CAMERARX0":5 []
<- "CAMERARX0":6 []
<- "CAMERARX0":7 [ENABLED]
<- "CAMERARX0":8 []
<- "CAMERARX1":1 []
<- "CAMERARX1":2 []
<- "CAMERARX1":3 []
<- "CAMERARX1":4 []
<- "CAMERARX1":5 []
<- "CAMERARX1":6 []
<- "CAMERARX1":7 []
<- "CAMERARX1":8 []
- entity 309: CAL output 7 (1 pad, 16 links, 0 route)
type Node subtype V4L flags 0
device node name /dev/video7
pad0: Sink
<- "CAMERARX0":1 []
<- "CAMERARX0":2 []
<- "CAMERARX0":3 []
<- "CAMERARX0":4 []
<- "CAMERARX0":5 []
<- "CAMERARX0":6 []
<- "CAMERARX0":7 []
<- "CAMERARX0":8 []
<- "CAMERARX1":1 []
<- "CAMERARX1":2 []
<- "CAMERARX1":3 []
<- "CAMERARX1":4 []
<- "CAMERARX1":5 []
<- "CAMERARX1":6 []
<- "CAMERARX1":7 []
<- "CAMERARX1":8 []
next prev parent reply other threads:[~2023-02-24 9:03 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-22 12:56 [PATCH v1 0/3] media: ti: cal: Streams support Tomi Valkeinen
2023-02-22 12:56 ` [PATCH v1 1/3] media: ti: cal: Add support for 1X16 mbus formats Tomi Valkeinen
2023-02-23 17:10 ` Jacopo Mondi
2023-02-23 17:52 ` Tomi Valkeinen
2023-02-23 18:03 ` Jacopo Mondi
2023-02-24 9:02 ` Tomi Valkeinen [this message]
2023-02-24 9:08 ` Tomi Valkeinen
2023-02-22 12:56 ` [PATCH v1 2/3] media: ti: cal: Use subdev state Tomi Valkeinen
2023-02-23 17:32 ` Jacopo Mondi
2023-02-24 9:15 ` Tomi Valkeinen
2023-02-22 12:56 ` [PATCH v1 3/3] media: ti: cal: add multiplexed streams support Tomi Valkeinen
2023-02-24 15:48 ` Jacopo Mondi
2023-02-24 17:20 ` niklas soderlund
2023-02-28 16:56 ` Jacopo Mondi
2023-02-27 8:35 ` Tomi Valkeinen
2023-02-27 8:45 ` Jacopo Mondi
2023-02-28 8:54 ` Sakari Ailus
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=49b1eca2-8997-0ff1-3151-d9656ef16fb8@ideasonboard.com \
--to=tomi.valkeinen@ideasonboard.com \
--cc=j-luthra@ti.com \
--cc=jacopo.mondi@ideasonboard.com \
--cc=kieran.bingham@ideasonboard.com \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-media@vger.kernel.org \
--cc=sakari.ailus@linux.intel.com \
--cc=vaishnav.a@ti.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox