[RFC 00/12] staging: media: max96712: Add support for streams and multiple sensors

[RFC 00/12] staging: media: max96712: Add support for streams and multiple sensors

[Laurentiu Palcu]

Hi,

This series adds more functionality to the existing max96712 staging
driver allowing multiple sensors to be connected through other
compatible serializers. I tried to split the changes in smaller logical
changes to make them easier to review while not altering the existing
VPG functionality but I could squash all of them together if needed.

The series only supports tunneling mode and uses the first MIPI-CSI
port. Support for more functionality can be added later, if needed.

I sent the set as a RFC because it depends on Sakari's pending internal
pads patch which is needed if we want to have an elegant solution for
allowing the user to switch between streaming from sensors or just
video pattern generation.

Also, the set depends on my previous series which was not yet merged:
https://patchwork.linuxtv.org/project/linux-media/list/?series=14255

Thanks,
Laurentiu

Laurentiu Palcu (11):
  dt-bindings: i2c: maxim,max96712: add a couple of new properties
  staging: media: max96712: convert to using CCI register access helpers
  staging: media: max96712: change DT parsing routine
  staging: media: max96712: add link frequency V4L2 control
  staging: media: max96712: add I2C mux support
  staging: media: max96712: add support for streams
  staging: media: max96712: allow enumerating MBUS codes
  staging: media: max96712: add set_fmt routine
  staging: media: max96712: add gpiochip functionality
  staging: media: max96712: add fsync support
  staging: media: max96712: allow streaming from connected sensors

Sakari Ailus (1):
  media: mc: Add INTERNAL pad flag

 .../bindings/media/i2c/maxim,max96712.yaml    |   45 +
 .../media/mediactl/media-types.rst            |    8 +
 drivers/media/mc/mc-entity.c                  |   10 +-
 drivers/staging/media/max96712/max96712.c     | 1406 +++++++++++++++--
 include/uapi/linux/media.h                    |    1 +
 5 files changed, 1352 insertions(+), 118 deletions(-)

-- 
2.44.1

[RFC 02/12] dt-bindings: i2c: maxim,max96712: add a couple of new properties

[Laurentiu Palcu]

Add new properties for configuring FSYNC functionality and operation
mode, as the chip can support both tunneling and pixel modes.

While at it, add the maxim,max96724 compatible to the bindings since it
was already added in the driver some time back.

Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
---
 .../bindings/media/i2c/maxim,max96712.yaml    | 45 +++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/i2c/maxim,max96712.yaml b/Documentation/devicetree/bindings/media/i2c/maxim,max96712.yaml
index 26f85151afbd3..410004f3a032f 100644
--- a/Documentation/devicetree/bindings/media/i2c/maxim,max96712.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/maxim,max96712.yaml
@@ -36,6 +36,48 @@ properties:
 
   enable-gpios: true
 
+  '#gpio-cells':
+    const: 2
+    description: |
+      First cell is the GPIO pin number, second cell is the flags. The GPIO pin
+      number must be in range of [0, 11].
+
+  gpio-controller: true
+
+  maxim,operation-mode:
+    description: |
+      Deserializer mode of operation: 0 - tunneling mode, 1 - pixel mode
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [0, 1]
+    default: 0
+
+  maxim,fsync-config:
+    description: |
+      Frame synchronization (FSYNC) is used to align images sent from multiple
+      sources in surround-view applications and is required for concatenation.
+      In FSYNC mode, the deserializer sends a sync signal to each serializer;
+      the serializers then send the signal to the connected sensor.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    minItems: 2
+    items:
+      - description: |
+          FSYNC mode:
+            0 - off, no FSYNC generation
+            1 - internal, GPIO is not used as input or output
+            2 - master, GPIO pin is used to drive a slave deserializer
+            3 - slave, GPIO pin is used as FSYNC input driven by a master device
+        enum: [0, 1, 2, 3]
+        default: 0
+      - description: |
+          FSYNC TX ID: GPIO ID used for transmitting FSYNC signal
+        minimum: 0
+        maximum: 31
+        default: 0
+      - description: |
+          FSYNC pin: 0 - MFP0, 1 - MFP7. Not used for internal mode.
+        enum: [0, 1]
+        default: 0
+
   ports:
     $ref: /schemas/graph.yaml#/properties/ports
 
@@ -92,6 +134,9 @@ examples:
     #include <dt-bindings/gpio/gpio.h>
     #include <dt-bindings/media/video-interfaces.h>
 
+    maxim,operation-mode = <0>;
+    maxim,fsync-config = <1 0>;
+
     i2c@e6508000 {
             #address-cells = <1>;
             #size-cells = <0>;
-- 
2.44.1

Re: [RFC 02/12] dt-bindings: i2c: maxim,max96712: add a couple of new properties

[Rob Herring]

On Fri, Jan 31, 2025 at 06:33:56PM +0200, Laurentiu Palcu wrote:
> Add new properties for configuring FSYNC functionality and operation
> mode, as the chip can support both tunneling and pixel modes.
> 
> While at it, add the maxim,max96724 compatible to the bindings since it
> was already added in the driver some time back.

I don't see that change.

> 
> Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
> ---
>  .../bindings/media/i2c/maxim,max96712.yaml    | 45 +++++++++++++++++++
>  1 file changed, 45 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/maxim,max96712.yaml b/Documentation/devicetree/bindings/media/i2c/maxim,max96712.yaml
> index 26f85151afbd3..410004f3a032f 100644
> --- a/Documentation/devicetree/bindings/media/i2c/maxim,max96712.yaml
> +++ b/Documentation/devicetree/bindings/media/i2c/maxim,max96712.yaml
> @@ -36,6 +36,48 @@ properties:
>  
>    enable-gpios: true
>  
> +  '#gpio-cells':
> +    const: 2
> +    description: |

Don't need '|' if no formatting.

> +      First cell is the GPIO pin number, second cell is the flags. The GPIO pin
> +      number must be in range of [0, 11].
> +
> +  gpio-controller: true
> +
> +  maxim,operation-mode:
> +    description: |
> +      Deserializer mode of operation: 0 - tunneling mode, 1 - pixel mode
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [0, 1]
> +    default: 0
> +
> +  maxim,fsync-config:
> +    description: |
> +      Frame synchronization (FSYNC) is used to align images sent from multiple
> +      sources in surround-view applications and is required for concatenation.
> +      In FSYNC mode, the deserializer sends a sync signal to each serializer;
> +      the serializers then send the signal to the connected sensor.
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    minItems: 2
> +    items:
> +      - description: |
> +          FSYNC mode:
> +            0 - off, no FSYNC generation
> +            1 - internal, GPIO is not used as input or output
> +            2 - master, GPIO pin is used to drive a slave deserializer
> +            3 - slave, GPIO pin is used as FSYNC input driven by a master device
> +        enum: [0, 1, 2, 3]
> +        default: 0
> +      - description: |
> +          FSYNC TX ID: GPIO ID used for transmitting FSYNC signal
> +        minimum: 0
> +        maximum: 31
> +        default: 0
> +      - description: |
> +          FSYNC pin: 0 - MFP0, 1 - MFP7. Not used for internal mode.
> +        enum: [0, 1]
> +        default: 0
> +
>    ports:
>      $ref: /schemas/graph.yaml#/properties/ports
>  
> @@ -92,6 +134,9 @@ examples:
>      #include <dt-bindings/gpio/gpio.h>
>      #include <dt-bindings/media/video-interfaces.h>
>  
> +    maxim,operation-mode = <0>;
> +    maxim,fsync-config = <1 0>;
> +
>      i2c@e6508000 {
>              #address-cells = <1>;
>              #size-cells = <0>;
> -- 
> 2.44.1
> 

Re: [RFC 00/12] staging: media: max96712: Add support for streams and multiple sensors

[Niklas Söderlund]

Hi Laurentiu,

Thanks for your work. I'm happy someone with a both GMSL cameras and a 
max96712 found time to work on this driver.

On 2025-01-31 18:33:54 +0200, Laurentiu Palcu wrote:
> Hi,
> 
> This series adds more functionality to the existing max96712 staging
> driver allowing multiple sensors to be connected through other
> compatible serializers. I tried to split the changes in smaller logical
> changes to make them easier to review while not altering the existing
> VPG functionality but I could squash all of them together if needed.

With this series and it's listed dependencies applied my CI tests using 
the VPG breaks. The controls to select test-pattern seems to be invalid,

    $ yavta --set-control '0x009f0903 0' /dev/v4l-subdev6
    Device /dev/v4l-subdev6 opened.
    unable to set control 0x009f0903: Permission denied (13).
    Unable to get format: Inappropriate ioctl for device (25).

    (/dev/v4l-subdev6 here is max96712 1-0049)

    $ yavta -c10 --file=/tmp/vin-capture/isp0-checkerboard-#.bin /dev/video0
    Device /dev/video0 opened.
    Device `R_Car_VIN' on `platform:e6ef0000.video' (driver 'rcar_vin') supports video, capture, without mplanes.
    Video format: ABGR32 (34325241) 1920x1080 (stride 7680) field none buffer size 8294400
    8 buffers requested.
    length: 8294400 offset: 0 timestamp type/source: mono/EoF
    Buffer 0/0 mapped at address 0xffffbe5d7000.
    length: 8294400 offset: 32768 timestamp type/source: mono/EoF
    Buffer 1/0 mapped at address 0xffffbddee000.
    length: 8294400 offset: 65536 timestamp type/source: mono/EoF
    Buffer 2/0 mapped at address 0xffffbd605000.
    length: 8294400 offset: 98304 timestamp type/source: mono/EoF
    Buffer 3/0 mapped at address 0xffffbce1c000.
    length: 8294400 offset: 131072 timestamp type/source: mono/EoF
    Buffer 4/0 mapped at address 0xffffbc633000.
    length: 8294400 offset: 163840 timestamp type/source: mono/EoF
    Buffer 5/0 mapped at address 0xffffbbe4a000.
    length: 8294400 offset: 196608 timestamp type/source: mono/EoF
    Buffer 6/0 mapped at address 0xffffbb661000.
    length: 8294400 offset: 229376 timestamp type/source: mono/EoF
    Buffer 7/0 mapped at address 0xffffbae78000.
    Unable to start streaming: Invalid argument (22).

I read in patch 12/12 "The user can also switch over to testing the test 
pattern by configuring the routes accordingly", but not how to do that 
to achieve the same functionality as the staging driver. Inspecting the 
media graph gives little help. Would it be possible to extend the cover 
letter with this information?

To be clear, I don't care about the change in behavior as this driver 
obviously primary aim should be to  support GMSL2 cameras, not 
test-pattern generation. It is important for me that it is possible to 
enable the test pattern generation $somehow at runtime (i.e. no DTS 
changes). As this is the only method I have to test a bunch of boards.

It would also be nice if the patterns generated (output frames) as 
closely as possible would resembles what is generated today. That way I 
don't have to alter my CI pipelines too much :-)

> 
> The series only supports tunneling mode and uses the first MIPI-CSI
> port. Support for more functionality can be added later, if needed.
> 
> I sent the set as a RFC because it depends on Sakari's pending internal
> pads patch which is needed if we want to have an elegant solution for
> allowing the user to switch between streaming from sensors or just
> video pattern generation.
> 
> Also, the set depends on my previous series which was not yet merged:
> https://patchwork.linuxtv.org/project/linux-media/list/?series=14255
> 
> Thanks,
> Laurentiu
> 
> Laurentiu Palcu (11):
>   dt-bindings: i2c: maxim,max96712: add a couple of new properties
>   staging: media: max96712: convert to using CCI register access helpers
>   staging: media: max96712: change DT parsing routine
>   staging: media: max96712: add link frequency V4L2 control
>   staging: media: max96712: add I2C mux support
>   staging: media: max96712: add support for streams
>   staging: media: max96712: allow enumerating MBUS codes
>   staging: media: max96712: add set_fmt routine
>   staging: media: max96712: add gpiochip functionality
>   staging: media: max96712: add fsync support
>   staging: media: max96712: allow streaming from connected sensors
> 
> Sakari Ailus (1):
>   media: mc: Add INTERNAL pad flag
> 
>  .../bindings/media/i2c/maxim,max96712.yaml    |   45 +
>  .../media/mediactl/media-types.rst            |    8 +
>  drivers/media/mc/mc-entity.c                  |   10 +-
>  drivers/staging/media/max96712/max96712.c     | 1406 +++++++++++++++--
>  include/uapi/linux/media.h                    |    1 +
>  5 files changed, 1352 insertions(+), 118 deletions(-)
> 
> -- 
> 2.44.1
> 

-- 
Kind Regards,
Niklas Söderlund

Re: [RFC 00/12] staging: media: max96712: Add support for streams and multiple sensors

[Laurentiu Palcu]

Hi Niklas,

Thanks for giving this series a test. More comments below.

On Tue, Feb 04, 2025 at 01:39:25PM +0100, Niklas Söderlund wrote:
> Hi Laurentiu,
> 
> Thanks for your work. I'm happy someone with a both GMSL cameras and a 
> max96712 found time to work on this driver.

I don't have a max96712 unfortunately. Our setup is based on max96724.

> 
> On 2025-01-31 18:33:54 +0200, Laurentiu Palcu wrote:
> > Hi,
> > 
> > This series adds more functionality to the existing max96712 staging
> > driver allowing multiple sensors to be connected through other
> > compatible serializers. I tried to split the changes in smaller logical
> > changes to make them easier to review while not altering the existing
> > VPG functionality but I could squash all of them together if needed.
> 
> With this series and it's listed dependencies applied my CI tests using 
> the VPG breaks. The controls to select test-pattern seems to be invalid,
> 
>     $ yavta --set-control '0x009f0903 0' /dev/v4l-subdev6
>     Device /dev/v4l-subdev6 opened.
>     unable to set control 0x009f0903: Permission denied (13).
>     Unable to get format: Inappropriate ioctl for device (25).

That's my bad... :/ I have never tried to change test patterns and I
didn't spot the bug. The below change should fix that:

diff --git a/drivers/staging/media/max96712/max96712.c b/drivers/staging/media/max96712/max96712.c
index b4c3d1d3c9539..6d6dea0a335c7 100644
--- a/drivers/staging/media/max96712/max96712.c
+++ b/drivers/staging/media/max96712/max96712.c
@@ -1315,10 +1315,10 @@ static int max96712_v4l2_register(struct max96712_priv *priv)

        v4l2_ctrl_handler_init(&priv->ctrl_handler, 2);

-       v4l2_ctrl_new_int_menu(&priv->ctrl_handler, NULL, V4L2_CID_LINK_FREQ,
+       link_freq_ctrl = v4l2_ctrl_new_int_menu(&priv->ctrl_handler, NULL, V4L2_CID_LINK_FREQ,
                               0, 0, &priv->link_freq);

-       link_freq_ctrl = v4l2_ctrl_new_std_menu_items(&priv->ctrl_handler, &max96712_ctrl_ops,
+       v4l2_ctrl_new_std_menu_items(&priv->ctrl_handler, &max96712_ctrl_ops,
                                                      V4L2_CID_TEST_PATTERN,
                                                      ARRAY_SIZE(max96712_test_pattern) - 1,
                                                      0, 0, max96712_test_pattern);

> 
>     (/dev/v4l-subdev6 here is max96712 1-0049)
> 
>     $ yavta -c10 --file=/tmp/vin-capture/isp0-checkerboard-#.bin /dev/video0
>     Device /dev/video0 opened.
>     Device `R_Car_VIN' on `platform:e6ef0000.video' (driver 'rcar_vin') supports video, capture, without mplanes.
>     Video format: ABGR32 (34325241) 1920x1080 (stride 7680) field none buffer size 8294400
>     8 buffers requested.
>     length: 8294400 offset: 0 timestamp type/source: mono/EoF
>     Buffer 0/0 mapped at address 0xffffbe5d7000.
>     length: 8294400 offset: 32768 timestamp type/source: mono/EoF
>     Buffer 1/0 mapped at address 0xffffbddee000.
>     length: 8294400 offset: 65536 timestamp type/source: mono/EoF
>     Buffer 2/0 mapped at address 0xffffbd605000.
>     length: 8294400 offset: 98304 timestamp type/source: mono/EoF
>     Buffer 3/0 mapped at address 0xffffbce1c000.
>     length: 8294400 offset: 131072 timestamp type/source: mono/EoF
>     Buffer 4/0 mapped at address 0xffffbc633000.
>     length: 8294400 offset: 163840 timestamp type/source: mono/EoF
>     Buffer 5/0 mapped at address 0xffffbbe4a000.
>     length: 8294400 offset: 196608 timestamp type/source: mono/EoF
>     Buffer 6/0 mapped at address 0xffffbb661000.
>     length: 8294400 offset: 229376 timestamp type/source: mono/EoF
>     Buffer 7/0 mapped at address 0xffffbae78000.
>     Unable to start streaming: Invalid argument (22).
> 
> I read in patch 12/12 "The user can also switch over to testing the test 
> pattern by configuring the routes accordingly", but not how to do that 
> to achieve the same functionality as the staging driver. Inspecting the 
> media graph gives little help. Would it be possible to extend the cover 
> letter with this information?

I can do that, sure, but routing setup depends on the board you test on... :/
Basically, the deserializer media node has 6 pads now. Pad 4 is first CSI
output port and pad 6 is the internal VPG source pad. By default, the route
from internal VPG pad to pad 4 is active. So, you shouldn't need to set any
routes on max96712 node for testing VPG. This is how it looks like:

- entity 120: max96712 2-0027 (7 pads, 5 links, 1 route)      
              type V4L2 subdev subtype Unknown flags 0
              device node name /dev/v4l-subdev11                                                        
        routes:                          
                6/0 -> 4/0 [ACTIVE]
        pad0: Sink                                                                                      
                <- "max96717 8-0040":1 []                                                               
        pad1: Sink                              
                <- "max96717 9-0040":1 []                                                               
        pad2: Sink                                                                                                                                                                                              
                <- "max96717 10-0040":1 []          
        pad3: Sink                                                                                      
                <- "max96717 14-0040":1 []                                                              
        pad4: Source                            
                [stream:0 fmt:RGB888_1X24/1920x1080 field:none colorspace:srgb]
                -> "csidev-4ad30000.csi":0 []
        pad5: Source
        pad6: Sink, Internal
                [stream:0 fmt:RGB888_1X24/1920x1080 field:none colorspace:srgb]

Below is the test script I use to set the routings and links for all the
pipeline in order to test VPG. I'm testing on an i.MX95 board.

./media-ctl -r
./media-ctl -d /dev/media0 -R '"max96712 2-0027" [6/0 -> 4/0 [1]]'
./media-ctl -d /dev/media0 --set-v4l2 '"max96712 2-0027":6/0 [fmt:RGB24/1920x1080 field:none]'
./media-ctl -d /dev/media0 -R '"csidev-4ad30000.csi" [0/0 -> 1/0 [1]]'
./media-ctl -d /dev/media0 --set-v4l2 '"csidev-4ad30000.csi":0/0 [fmt:RGB24/1920x1080 field:none]'
./media-ctl -d /dev/media0 -R '"4ac10000.syscon:formatter@20" [0/0 -> 1/0 [1]]'
./media-ctl -d /dev/media0 --set-v4l2 '"4ac10000.syscon:formatter@20":0/0 [fmt:RGB24/1920x1080 field:none]'
./media-ctl -d /dev/media0 -R '"crossbar" [2/0 -> 7/0 [1]]'
./media-ctl -d /dev/media0 --set-v4l2 '"crossbar":2/0 [fmt:RGB24/1920x1080 field:none]'
./media-ctl -d /dev/media0 --set-v4l2 '"mxc_isi.2":0/0 [fmt:RGB24/1920x1080 field:none]'
./media-ctl -d /dev/media0 -l "'max96712 2-0027':4 -> 'csidev-4ad30000.csi':0 [1]"

./v4l2-ctl --device /dev/video2 --set-fmt-video=width=1920,height=1080,pixelformat=RGB3 --stream-mmap --stream-count=100

However, I suspect that, in your case, the downstream drivers do not support
streams and streaming cannot start. But I might be wrong though...

> 
> To be clear, I don't care about the change in behavior as this driver 
> obviously primary aim should be to  support GMSL2 cameras, not 
> test-pattern generation. It is important for me that it is possible to 
> enable the test pattern generation $somehow at runtime (i.e. no DTS 
> changes). As this is the only method I have to test a bunch of boards.

That's the idea. I can switch between capturing from sensors and generating the
test pattern at runtime. I don't do any changes in the DTB. However, I believe
the downstream devices need to support streams as well in order to work.

> 
> It would also be nice if the patterns generated (output frames) as 
> closely as possible would resembles what is generated today. That way I 
> don't have to alter my CI pipelines too much :-)

I didn't touch the pattern generation part at all. From what I can see, it
looks the same.

Thanks,
Laurentiu

> 
> > 
> > The series only supports tunneling mode and uses the first MIPI-CSI
> > port. Support for more functionality can be added later, if needed.
> > 
> > I sent the set as a RFC because it depends on Sakari's pending internal
> > pads patch which is needed if we want to have an elegant solution for
> > allowing the user to switch between streaming from sensors or just
> > video pattern generation.
> > 
> > Also, the set depends on my previous series which was not yet merged:
> > https://patchwork.linuxtv.org/project/linux-media/list/?series=14255
> > 
> > Thanks,
> > Laurentiu
> > 
> > Laurentiu Palcu (11):
> >   dt-bindings: i2c: maxim,max96712: add a couple of new properties
> >   staging: media: max96712: convert to using CCI register access helpers
> >   staging: media: max96712: change DT parsing routine
> >   staging: media: max96712: add link frequency V4L2 control
> >   staging: media: max96712: add I2C mux support
> >   staging: media: max96712: add support for streams
> >   staging: media: max96712: allow enumerating MBUS codes
> >   staging: media: max96712: add set_fmt routine
> >   staging: media: max96712: add gpiochip functionality
> >   staging: media: max96712: add fsync support
> >   staging: media: max96712: allow streaming from connected sensors
> > 
> > Sakari Ailus (1):
> >   media: mc: Add INTERNAL pad flag
> > 
> >  .../bindings/media/i2c/maxim,max96712.yaml    |   45 +
> >  .../media/mediactl/media-types.rst            |    8 +
> >  drivers/media/mc/mc-entity.c                  |   10 +-
> >  drivers/staging/media/max96712/max96712.c     | 1406 +++++++++++++++--
> >  include/uapi/linux/media.h                    |    1 +
> >  5 files changed, 1352 insertions(+), 118 deletions(-)
> > 
> > -- 
> > 2.44.1
> > 
> 
> -- 
> Kind Regards,
> Niklas Söderlund