Re: [PATCH v7 0/5] Add support for MAX96714/F and MAX96717/F GMSL2 ser/des

From: Tommaso Merciai
Date: Fri May 17 2024 - 10:16:24 EST

Next message: Guilherme Giacomo Simoes: "[PATCH] thermal: adding check if the thermal firmware is running"
Previous message: Steven Rostedt: "Re: [PATCH] tracing/treewide: Remove second parameter of __assign_str()"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi Julien,

On Tue, Apr 30, 2024 at 03:19:26PM +0200, Julien Massot wrote:
> Change since v6:
> - Remove mention of C-PHY for MAX96717, this serializer is D-PHY only
> - Remove bus-type requirement for MAX96717
> - Minor changes requested by Sakari
> - Workaround a MAX96717 issue, which occurs when stopping
> the CSI source before stopping the MAX96717 CSI receiver.
>
> Power management is not included in this patchset. The GMSL link is
> not always resuming when the deserializer is suspended without
> suspending the serializer.
>
> Change since v5:
> - Reverse fallback logic: max9671{4,7} can fallback to max9671{4,7}F
> - use const instead of enum for max9671{4,7}f compatible as suggested
>
> Change since v4:
> - Add support for MAX96717 and MAX96714 and use them as a fallback for
> MAX96717F and MAX96714F respectively
> - The drivers are now compatible with MAX96717 and MAX96714 since no change in
> the logic is needed
> - Reference 'i2c-gate' instead of 'i2c-controller' in the bindings
>
> Change since v3:
> - bindings
> - Renamed bindings to drop the 'f' suffix
> - Add bus type to MAX96717 and remove from MAX9674
> - Add lane-polarities to both bindings
>
> - drivers
> - Address changes requested by Sakari in v3
> - use v4l2_subdev_s_stream_helper for MAX96714
> - do not init regmap twice in the MAX96714 driver
> - Fix compilations on 32 bits platforms
>
> Change since v2:
> - Convert drivers to use CCI helpers
> - Use generic node name
> - Use 'powerdown' as gpio name instead of 'enable'
> - Add pattern generator support for MAX96714
>
> These patches add support for Maxim MAX96714F deserializer and
> MAX96717F serializer.
>
> MAX96714F has one GMSL2 input port and one CSI2 4 lanes output port,
> MAX96717F has one CSI2 input port and one GMSL2 output port.
>
> The drivers support the tunnel mode where all the
> CSI2 traffic coming from an imager is replicated through the deserializer
> output port.
>
> Both MAX96714F and MAX96717F are limited to a 3Gbps forward link rate
> leaving a maximum of 2.6Gbps for the video payload.

Thanks for your great work! :)
I test your series with the following hw:

alvium camera (GM2-319c) -> max96717 (non f variant) -> max96716a -> imx8mp-msc-sm2s-ep2 board

Note:
max96716a is pretty similar to max96714. I change only:

#define MAX96714_DEVICE_ID 0xb6
#define MAX96714F_DEVICE_ID 0xbe

And swapping lanes as you suggest:

max96714_csi0_out: endpoint {
data-lanes = <3 4 1 2>;
link-frequencies = /bits/ 64 <750000000>;
remote-endpoint = <&mipi_csi_0_in>;
};

On my current setup csi2 to gmsl2 board is always powered on then, I have
ERRB pin that is triggered at uboot lvl because is not handled.
I think we can andle later this case I can suggest to clear/unclear the ERRB_EN bit of REG5(0x5)
in the very beginning of the probe.

Apart of this
All is working properly on my side and also on Martin side (that is in CC) :)
I'm able to stream properly using:

# SETUP TOPOLOGY LINKS
media-ctl --links "'alvium-csi2 3-003c':0->'max96717 6-0040':0[1]"
media-ctl --links "'max96717 6-0040':1->'max96714 3-0028':0[1]"
media-ctl --links "'max96714 3-0028':1->'csis-32e40000.csi':0[1]"
media-ctl --links "'csis-32e40000.csi':1->'crossbar':0[1]"
media-ctl --links "'crossbar':3->'mxc_isi.0':0[1]"
media-ctl --links "'mxc_isi.0':1->'mxc_isi.0.capture':0[1]"

# SETUP TOPOLOGY ENTITIES
media-ctl -d /dev/media0 --set-v4l2 '"alvium-csi2 3-003c":0[fmt:UYVY8_1X16/1280x800@1/60 field:none]'
media-ctl -d /dev/media0 --set-v4l2 '"max96717 6-0040":0[fmt:UYVY8_1X16/1280x800 field:none]'
media-ctl -d /dev/media0 --set-v4l2 '"max96714 3-0028":0[fmt:UYVY8_1X16/1280x800 field:none]'
media-ctl -d /dev/media0 --set-v4l2 '"csis-32e40000.csi":0[fmt:UYVY8_1X16/1280x800 field:none]'
media-ctl -d /dev/media0 --set-v4l2 '"crossbar":0[fmt:UYVY8_1X16/1280x800 field:none]'
media-ctl -d /dev/media0 --set-v4l2 '"mxc_isi.0":0[fmt:UYVY8_1X16/1280x800 field:none]'

gst-launch-1.0 v4l2src io-mode=dmabuf blocksize=76800 ! video/x-raw,format=YUY2,width=1280,height=800 ! videoconvert ! waylandsink sync=false

# TOPOLOGY

Media controller API version 6.9.0

Media device information
------------------------
driver mxc-isi
model FSL Capture Media Device
serial
bus info platform:32e00000.isi
hw revision 0x0
driver version 6.9.0

Device topology
- entity 1: crossbar (5 pads, 4 links, 0 routes)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev0
pad0: Sink
[stream:0 fmt:UYVY8_1X16/1280x800 field:none]
<- "csis-32e40000.csi":1 [ENABLED,IMMUTABLE]
pad1: Sink
[stream:0 fmt:UYVY8_1X16/1920x1080 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:lim-range]
pad2: Sink
<- "mxc_isi.output":0 [ENABLED,IMMUTABLE]
pad3: Source
[stream:0 fmt:UYVY8_1X16/1280x800 field:none]
-> "mxc_isi.0":0 [ENABLED,IMMUTABLE]
pad4: Source
[stream:0 fmt:UYVY8_1X16/1920x1080 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:lim-range]
-> "mxc_isi.1":0 [ENABLED,IMMUTABLE]

- entity 7: mxc_isi.0 (2 pads, 2 links, 0 routes)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev1
pad0: Sink
[stream:0 fmt:UYVY8_1X16/1280x800 field:none
compose.bounds:(0,0)/1280x800
compose:(0,0)/1280x800]
<- "crossbar":3 [ENABLED,IMMUTABLE]
pad1: Source
[stream:0 fmt:YUV8_1X24/1280x800 field:none colorspace:jpeg xfer:srgb ycbcr:601 quantization:full-range
crop.bounds:(0,0)/1280x800
crop:(0,0)/1280x800]
-> "mxc_isi.0.capture":0 [ENABLED,IMMUTABLE]

- entity 10: mxc_isi.0.capture (1 pad, 1 link)
type Node subtype V4L flags 0
device node name /dev/video0
pad0: Sink
<- "mxc_isi.0":1 [ENABLED,IMMUTABLE]

- entity 18: mxc_isi.1 (2 pads, 2 links, 0 routes)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev2
pad0: Sink
[stream:0 fmt:UYVY8_1X16/1920x1080 field:none colorspace:jpeg xfer:srgb ycbcr:601 quantization:full-range
compose.bounds:(0,0)/1920x1080
compose:(0,0)/1920x1080]
<- "crossbar":4 [ENABLED,IMMUTABLE]
pad1: Source
[stream:0 fmt:YUV8_1X24/1920x1080 field:none colorspace:jpeg xfer:srgb ycbcr:601 quantization:full-range
crop.bounds:(0,0)/1920x1080
crop:(0,0)/1920x1080]
-> "mxc_isi.1.capture":0 [ENABLED,IMMUTABLE]

- entity 21: mxc_isi.1.capture (1 pad, 1 link)
type Node subtype V4L flags 0
device node name /dev/video1
pad0: Sink
<- "mxc_isi.1":1 [ENABLED,IMMUTABLE]

- entity 29: mxc_isi.output (1 pad, 1 link)
type Node subtype V4L flags 0
pad0: Source
-> "crossbar":2 [ENABLED,IMMUTABLE]

- entity 36: csis-32e40000.csi (2 pads, 2 links, 0 routes)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev3
pad0: Sink
[stream:0 fmt:UYVY8_1X16/1280x800 field:none]
<- "max96714 3-0028":1 [ENABLED]
pad1: Source
[stream:0 fmt:UYVY8_1X16/1280x800 field:none]
-> "crossbar":0 [ENABLED,IMMUTABLE]

- entity 41: max96714 3-0028 (2 pads, 2 links, 0 routes)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev4
pad0: Sink
[stream:0 fmt:UYVY8_1X16/1280x800 field:none]
<- "max96717 6-0040":1 [ENABLED,IMMUTABLE]
pad1: Source
[stream:0 fmt:UYVY8_1X16/1280x800 field:none]
-> "csis-32e40000.csi":0 [ENABLED]

- entity 46: max96717 6-0040 (2 pads, 2 links, 0 routes)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev5
pad0: Sink
[stream:0 fmt:UYVY8_1X16/1280x800 field:none]
<- "alvium-csi2 3-003c":0 [ENABLED,IMMUTABLE]
pad1: Source
[stream:0 fmt:UYVY8_1X16/1280x800 field:none]
-> "max96714 3-0028":0 [ENABLED,IMMUTABLE]

- entity 51: alvium-csi2 3-003c (1 pad, 1 link, 0 routes)
type V4L2 subdev subtype Sensor flags 0
device node name /dev/v4l-subdev6
pad0: Source
[stream:0 fmt:UYVY8_1X16/1280x800@1/60 field:none
crop.bounds:(0,0)/2064x1544
crop:(0,0)/640x480]
-> "max96717 6-0040":0 [ENABLED,IMMUTABLE]

All this test was done on top of linux-media tree 6.9.0-rc2.
Here my tags for all the series:

Reviewed-by: Tommaso Merciai <tomm.merciai@xxxxxxxxx>
Tested-by: Tommaso Merciai <tomm.merciai@xxxxxxxxx>

Thanks again for your great work.
Hope this series will be merged soon :)

Regards,
Tommaso

>
> Julien Massot (9):
> dt-bindings: media: add Maxim MAX96717 GMSL2 Serializer
> dt-bindings: media: add Maxim MAX96714 GMSL2 Deserializer
> media: i2c: add MAX96717 driver
> media: i2c: add MAX96714 driver
> drivers: media: max96717: stop the csi receiver before the source
>
> .../bindings/media/i2c/maxim,max96714.yaml | 174 +++
> .../bindings/media/i2c/maxim,max96717.yaml | 157 +++
> MAINTAINERS | 14 +
> drivers/media/i2c/Kconfig | 34 +
> drivers/media/i2c/Makefile | 2 +
> drivers/media/i2c/max96714.c | 1024 +++++++++++++++++
> drivers/media/i2c/max96717.c | 927 +++++++++++++++
> 7 files changed, 2332 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/i2c/maxim,max96714.yaml
> create mode 100644 Documentation/devicetree/bindings/media/i2c/maxim,max96717.yaml
> create mode 100644 drivers/media/i2c/max96714.c
> create mode 100644 drivers/media/i2c/max96717.c
>
> --
> 2.44.0
>
>

Next message: Guilherme Giacomo Simoes: "[PATCH] thermal: adding check if the thermal firmware is running"
Previous message: Steven Rostedt: "Re: [PATCH] tracing/treewide: Remove second parameter of __assign_str()"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]