[PATCH v1 0/2] Revert vendor property from anx7625 bindings

[PATCH v1 0/2] Revert vendor property from anx7625 bindings

[Robert Foss]

An issue[1] related to how the V4L2_FWNODE_BUS_TYPE_PARALLEL flag is mis-used
was found in recent addition to the anx7625 driver.

In order to not introduce this issue into the ABI, let's revert the changes
to the anx7625 dt-binding related to this.

[1] https://lore.kernel.org/all/YiTruiCIkyxs3jTC@pendragon.ideasonboard.com/

Robert Foss (2):
  Revert "dt-bindings:drm/bridge:anx7625:add vendor define"
  Revert "arm64: dts: mt8183: jacuzzi: Fix bus properties in anx's DSI
    endpoint"

 .../display/bridge/analogix,anx7625.yaml      | 65 +------------------
 .../dts/mediatek/mt8183-kukui-jacuzzi.dtsi    |  2 -
 2 files changed, 2 insertions(+), 65 deletions(-)

-- 
2.32.0

[PATCH v1 1/2] Revert "dt-bindings:drm/bridge:anx7625:add vendor define"

[Robert Foss]

This reverts commit a43661e7e819b100e1f833a35018560a1d9abb39.
---
 .../display/bridge/analogix,anx7625.yaml      | 65 +------------------
 1 file changed, 2 insertions(+), 63 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
index 1d3e88daca041..ab48ab2f4240d 100644
--- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
@@ -43,70 +43,14 @@ properties:
   vdd33-supply:
     description: Regulator that provides the supply 3.3V power.
 
-  analogix,lane0-swing:
-    $ref: /schemas/types.yaml#/definitions/uint8-array
-    minItems: 1
-    maxItems: 20
-    description:
-      an array of swing register setting for DP tx lane0 PHY.
-      Registers 0~9 are Swing0_Pre0, Swing1_Pre0, Swing2_Pre0,
-      Swing3_Pre0, Swing0_Pre1, Swing1_Pre1, Swing2_Pre1, Swing0_Pre2,
-      Swing1_Pre2, Swing0_Pre3, they are for [Boost control] and
-      [Swing control] setting.
-      Registers 0~9, bit 3:0 is [Boost control], these bits control
-      post cursor manual, increase the [Boost control] to increase
-      Pre-emphasis value.
-      Registers 0~9, bit 6:4 is [Swing control], these bits control
-      swing manual, increase [Swing control] setting to add Vp-p value
-      for each Swing, Pre.
-      Registers 10~19 are Swing0_Pre0, Swing1_Pre0, Swing2_Pre0,
-      Swing3_Pre0, Swing0_Pre1, Swing1_Pre1, Swing2_Pre1, Swing0_Pre2,
-      Swing1_Pre2, Swing0_Pre3, they are for [R select control] and
-      [R Termination control] setting.
-      Registers 10~19, bit 4:0 is [R select control], these bits are
-      compensation manual, increase it can enhance IO driven strength
-      and Vp-p.
-      Registers 10~19, bit 5:6 is [R termination control], these bits
-      adjust 50ohm impedance of DP tx termination. 00:55 ohm,
-      01:50 ohm(default), 10:45 ohm, 11:40 ohm.
-
-  analogix,lane1-swing:
-    $ref: /schemas/types.yaml#/definitions/uint8-array
-    minItems: 1
-    maxItems: 20
-    description:
-      an array of swing register setting for DP tx lane1 PHY.
-      DP TX lane1 swing register setting same with lane0
-      swing, please refer lane0-swing property description.
-
-  analogix,audio-enable:
-    type: boolean
-    description: let the driver enable audio HDMI codec function or not.
-
   ports:
     $ref: /schemas/graph.yaml#/properties/ports
 
     properties:
       port@0:
-        $ref: /schemas/graph.yaml#/$defs/port-base
-        unevaluatedProperties: false
+        $ref: /schemas/graph.yaml#/properties/port
         description:
-          MIPI DSI/DPI input.
-
-        properties:
-          endpoint:
-            $ref: /schemas/media/video-interfaces.yaml#
-            type: object
-            additionalProperties: false
-
-            properties:
-              remote-endpoint: true
-
-              bus-type:
-                enum: [1, 5]
-                default: 1
-
-              data-lanes: true
+          Video port for MIPI DSI input.
 
       port@1:
         $ref: /schemas/graph.yaml#/properties/port
@@ -143,9 +87,6 @@ examples:
             vdd10-supply = <&pp1000_mipibrdg>;
             vdd18-supply = <&pp1800_mipibrdg>;
             vdd33-supply = <&pp3300_mipibrdg>;
-            analogix,audio-enable;
-            analogix,lane0-swing = /bits/ 8 <0x14 0x54 0x64 0x74>;
-            analogix,lane1-swing = /bits/ 8 <0x14 0x54 0x64 0x74>;
 
             ports {
                 #address-cells = <1>;
@@ -155,8 +96,6 @@ examples:
                     reg = <0>;
                     anx7625_in: endpoint {
                         remote-endpoint = <&mipi_dsi>;
-                        bus-type = <5>;
-                        data-lanes = <0 1 2 3>;
                     };
                 };
 
-- 
2.32.0

[PATCH v1 2/2] Revert "arm64: dts: mt8183: jacuzzi: Fix bus properties in anx's DSI endpoint"

[Robert Foss]

This reverts commit 32568ae37596b529628ac09b875f4874e614f63f.
---
 arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi b/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi
index e8f133dc96b95..8f7bf33f607da 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi
@@ -171,8 +171,6 @@ port@0 {
 
 			anx7625_in: endpoint {
 				remote-endpoint = <&dsi_out>;
-				bus-type = <5>;
-				data-lanes = <0 1 2 3>;
 			};
 		};
 
-- 
2.32.0

Re: [PATCH v1 0/2] Revert vendor property from anx7625 bindings

[Laurent Pinchart]

Hi Rob,

Thank you for the patch.

On Mon, Mar 07, 2022 at 04:45:56PM +0100, Robert Foss wrote:
> An issue[1] related to how the V4L2_FWNODE_BUS_TYPE_PARALLEL flag is mis-used
> was found in recent addition to the anx7625 driver.
> 
> In order to not introduce this issue into the ABI, let's revert the changes
> to the anx7625 dt-binding related to this.
> 
> [1] https://lore.kernel.org/all/YiTruiCIkyxs3jTC@pendragon.ideasonboard.com/
> 
> Robert Foss (2):
>   Revert "dt-bindings:drm/bridge:anx7625:add vendor define"
>   Revert "arm64: dts: mt8183: jacuzzi: Fix bus properties in anx's DSI
>     endpoint"

If this is enough to avoid the wrong bus-type becoming an ABI, even if
the corresponding driver support isn't reverted, then, for the whole
series,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  .../display/bridge/analogix,anx7625.yaml      | 65 +------------------
>  .../dts/mediatek/mt8183-kukui-jacuzzi.dtsi    |  2 -
>  2 files changed, 2 insertions(+), 65 deletions(-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH v1 2/2] Revert "arm64: dts: mt8183: jacuzzi: Fix bus properties in anx's DSI endpoint"

[Robert Foss]

Signed-off-by: Robert Foss <robert.foss@linaro.org>

On Mon, 7 Mar 2022 at 16:46, Robert Foss <robert.foss@linaro.org> wrote:
>
> This reverts commit 32568ae37596b529628ac09b875f4874e614f63f.
> ---
>  arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi b/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi
> index e8f133dc96b95..8f7bf33f607da 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi
> @@ -171,8 +171,6 @@ port@0 {
>
>                         anx7625_in: endpoint {
>                                 remote-endpoint = <&dsi_out>;
> -                               bus-type = <5>;
> -                               data-lanes = <0 1 2 3>;
>                         };
>                 };
>
> --
> 2.32.0
>

Re: [PATCH v1 1/2] Revert "dt-bindings:drm/bridge:anx7625:add vendor define"

[Robert Foss]

On Mon, 7 Mar 2022 at 16:46, Robert Foss <robert.foss@linaro.org> wrote:
>
> This reverts commit a43661e7e819b100e1f833a35018560a1d9abb39.
> ---
>  .../display/bridge/analogix,anx7625.yaml      | 65 +------------------
>  1 file changed, 2 insertions(+), 63 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> index 1d3e88daca041..ab48ab2f4240d 100644
> --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> @@ -43,70 +43,14 @@ properties:
>    vdd33-supply:
>      description: Regulator that provides the supply 3.3V power.
>
> -  analogix,lane0-swing:
> -    $ref: /schemas/types.yaml#/definitions/uint8-array
> -    minItems: 1
> -    maxItems: 20
> -    description:
> -      an array of swing register setting for DP tx lane0 PHY.
> -      Registers 0~9 are Swing0_Pre0, Swing1_Pre0, Swing2_Pre0,
> -      Swing3_Pre0, Swing0_Pre1, Swing1_Pre1, Swing2_Pre1, Swing0_Pre2,
> -      Swing1_Pre2, Swing0_Pre3, they are for [Boost control] and
> -      [Swing control] setting.
> -      Registers 0~9, bit 3:0 is [Boost control], these bits control
> -      post cursor manual, increase the [Boost control] to increase
> -      Pre-emphasis value.
> -      Registers 0~9, bit 6:4 is [Swing control], these bits control
> -      swing manual, increase [Swing control] setting to add Vp-p value
> -      for each Swing, Pre.
> -      Registers 10~19 are Swing0_Pre0, Swing1_Pre0, Swing2_Pre0,
> -      Swing3_Pre0, Swing0_Pre1, Swing1_Pre1, Swing2_Pre1, Swing0_Pre2,
> -      Swing1_Pre2, Swing0_Pre3, they are for [R select control] and
> -      [R Termination control] setting.
> -      Registers 10~19, bit 4:0 is [R select control], these bits are
> -      compensation manual, increase it can enhance IO driven strength
> -      and Vp-p.
> -      Registers 10~19, bit 5:6 is [R termination control], these bits
> -      adjust 50ohm impedance of DP tx termination. 00:55 ohm,
> -      01:50 ohm(default), 10:45 ohm, 11:40 ohm.
> -
> -  analogix,lane1-swing:
> -    $ref: /schemas/types.yaml#/definitions/uint8-array
> -    minItems: 1
> -    maxItems: 20
> -    description:
> -      an array of swing register setting for DP tx lane1 PHY.
> -      DP TX lane1 swing register setting same with lane0
> -      swing, please refer lane0-swing property description.
> -
> -  analogix,audio-enable:
> -    type: boolean
> -    description: let the driver enable audio HDMI codec function or not.
> -
>    ports:
>      $ref: /schemas/graph.yaml#/properties/ports
>
>      properties:
>        port@0:
> -        $ref: /schemas/graph.yaml#/$defs/port-base
> -        unevaluatedProperties: false
> +        $ref: /schemas/graph.yaml#/properties/port
>          description:
> -          MIPI DSI/DPI input.
> -
> -        properties:
> -          endpoint:
> -            $ref: /schemas/media/video-interfaces.yaml#
> -            type: object
> -            additionalProperties: false
> -
> -            properties:
> -              remote-endpoint: true
> -
> -              bus-type:
> -                enum: [1, 5]
> -                default: 1
> -
> -              data-lanes: true
> +          Video port for MIPI DSI input.
>
>        port@1:
>          $ref: /schemas/graph.yaml#/properties/port
> @@ -143,9 +87,6 @@ examples:
>              vdd10-supply = <&pp1000_mipibrdg>;
>              vdd18-supply = <&pp1800_mipibrdg>;
>              vdd33-supply = <&pp3300_mipibrdg>;
> -            analogix,audio-enable;
> -            analogix,lane0-swing = /bits/ 8 <0x14 0x54 0x64 0x74>;
> -            analogix,lane1-swing = /bits/ 8 <0x14 0x54 0x64 0x74>;
>
>              ports {
>                  #address-cells = <1>;
> @@ -155,8 +96,6 @@ examples:
>                      reg = <0>;
>                      anx7625_in: endpoint {
>                          remote-endpoint = <&mipi_dsi>;
> -                        bus-type = <5>;
> -                        data-lanes = <0 1 2 3>;
>                      };
>                  };
>
> --
> 2.32.0
>

Signed-off-by: Robert Foss <robert.foss@linaro.org>

Re: [PATCH v1 1/2] Revert "dt-bindings:drm/bridge:anx7625:add vendor define"

[Rob Herring]

On Mon, Mar 07, 2022 at 04:45:57PM +0100, Robert Foss wrote:
> This reverts commit a43661e7e819b100e1f833a35018560a1d9abb39.

S-o-b and reason for the revert?

> ---
>  .../display/bridge/analogix,anx7625.yaml      | 65 +------------------
>  1 file changed, 2 insertions(+), 63 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> index 1d3e88daca041..ab48ab2f4240d 100644
> --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> @@ -43,70 +43,14 @@ properties:
>    vdd33-supply:
>      description: Regulator that provides the supply 3.3V power.
>  
> -  analogix,lane0-swing:
> -    $ref: /schemas/types.yaml#/definitions/uint8-array
> -    minItems: 1
> -    maxItems: 20
> -    description:
> -      an array of swing register setting for DP tx lane0 PHY.
> -      Registers 0~9 are Swing0_Pre0, Swing1_Pre0, Swing2_Pre0,
> -      Swing3_Pre0, Swing0_Pre1, Swing1_Pre1, Swing2_Pre1, Swing0_Pre2,
> -      Swing1_Pre2, Swing0_Pre3, they are for [Boost control] and
> -      [Swing control] setting.
> -      Registers 0~9, bit 3:0 is [Boost control], these bits control
> -      post cursor manual, increase the [Boost control] to increase
> -      Pre-emphasis value.
> -      Registers 0~9, bit 6:4 is [Swing control], these bits control
> -      swing manual, increase [Swing control] setting to add Vp-p value
> -      for each Swing, Pre.
> -      Registers 10~19 are Swing0_Pre0, Swing1_Pre0, Swing2_Pre0,
> -      Swing3_Pre0, Swing0_Pre1, Swing1_Pre1, Swing2_Pre1, Swing0_Pre2,
> -      Swing1_Pre2, Swing0_Pre3, they are for [R select control] and
> -      [R Termination control] setting.
> -      Registers 10~19, bit 4:0 is [R select control], these bits are
> -      compensation manual, increase it can enhance IO driven strength
> -      and Vp-p.
> -      Registers 10~19, bit 5:6 is [R termination control], these bits
> -      adjust 50ohm impedance of DP tx termination. 00:55 ohm,
> -      01:50 ohm(default), 10:45 ohm, 11:40 ohm.
> -
> -  analogix,lane1-swing:
> -    $ref: /schemas/types.yaml#/definitions/uint8-array
> -    minItems: 1
> -    maxItems: 20
> -    description:
> -      an array of swing register setting for DP tx lane1 PHY.
> -      DP TX lane1 swing register setting same with lane0
> -      swing, please refer lane0-swing property description.

These apply to the DP side, so no need to revert this part.

> -
> -  analogix,audio-enable:
> -    type: boolean
> -    description: let the driver enable audio HDMI codec function or not.
> -

Not sure on this one...

>    ports:
>      $ref: /schemas/graph.yaml#/properties/ports
>  
>      properties:
>        port@0:
> -        $ref: /schemas/graph.yaml#/$defs/port-base
> -        unevaluatedProperties: false
> +        $ref: /schemas/graph.yaml#/properties/port
>          description:
> -          MIPI DSI/DPI input.
> -
> -        properties:
> -          endpoint:
> -            $ref: /schemas/media/video-interfaces.yaml#
> -            type: object
> -            additionalProperties: false
> -
> -            properties:
> -              remote-endpoint: true
> -
> -              bus-type:
> -                enum: [1, 5]

I think the error here is really 1 should be 4 which corresponds to 
D-PHY which is used by both CSI and DSI. Otherwise, I don't really see 
the issue with bus-type being shared between CSI and DSI.

> -                default: 1
> -
> -              data-lanes: true
> +          Video port for MIPI DSI input.
>  
>        port@1:
>          $ref: /schemas/graph.yaml#/properties/port
> @@ -143,9 +87,6 @@ examples:
>              vdd10-supply = <&pp1000_mipibrdg>;
>              vdd18-supply = <&pp1800_mipibrdg>;
>              vdd33-supply = <&pp3300_mipibrdg>;
> -            analogix,audio-enable;
> -            analogix,lane0-swing = /bits/ 8 <0x14 0x54 0x64 0x74>;
> -            analogix,lane1-swing = /bits/ 8 <0x14 0x54 0x64 0x74>;
>  
>              ports {
>                  #address-cells = <1>;
> @@ -155,8 +96,6 @@ examples:
>                      reg = <0>;
>                      anx7625_in: endpoint {
>                          remote-endpoint = <&mipi_dsi>;
> -                        bus-type = <5>;
> -                        data-lanes = <0 1 2 3>;
>                      };
>                  };
>  
> -- 
> 2.32.0
> 
> 

Re: [PATCH v1 2/2] Revert "arm64: dts: mt8183: jacuzzi: Fix bus properties in anx's DSI endpoint"

[Rob Herring]

On Mon, Mar 07, 2022 at 04:45:58PM +0100, Robert Foss wrote:
> This reverts commit 32568ae37596b529628ac09b875f4874e614f63f.
> ---
>  arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi b/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi
> index e8f133dc96b95..8f7bf33f607da 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi
> @@ -171,8 +171,6 @@ port@0 {
>  
>  			anx7625_in: endpoint {
>  				remote-endpoint = <&dsi_out>;
> -				bus-type = <5>;
> -				data-lanes = <0 1 2 3>;

Well, this was clearly wrong. Connected to a DSI output, but parallel 
interface with lanes...

We should have a schema to disallow this combination.

Rob

Re: [PATCH v1 1/2] Revert "dt-bindings:drm/bridge:anx7625:add vendor define"

[Robert Foss]

On Mon, 7 Mar 2022 at 17:38, Rob Herring <robh@kernel.org> wrote:
>
> On Mon, Mar 07, 2022 at 04:45:57PM +0100, Robert Foss wrote:
> > This reverts commit a43661e7e819b100e1f833a35018560a1d9abb39.
>
> S-o-b and reason for the revert?
>
> > ---
> >  .../display/bridge/analogix,anx7625.yaml      | 65 +------------------
> >  1 file changed, 2 insertions(+), 63 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > index 1d3e88daca041..ab48ab2f4240d 100644
> > --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > @@ -43,70 +43,14 @@ properties:
> >    vdd33-supply:
> >      description: Regulator that provides the supply 3.3V power.
> >
> > -  analogix,lane0-swing:
> > -    $ref: /schemas/types.yaml#/definitions/uint8-array
> > -    minItems: 1
> > -    maxItems: 20
> > -    description:
> > -      an array of swing register setting for DP tx lane0 PHY.
> > -      Registers 0~9 are Swing0_Pre0, Swing1_Pre0, Swing2_Pre0,
> > -      Swing3_Pre0, Swing0_Pre1, Swing1_Pre1, Swing2_Pre1, Swing0_Pre2,
> > -      Swing1_Pre2, Swing0_Pre3, they are for [Boost control] and
> > -      [Swing control] setting.
> > -      Registers 0~9, bit 3:0 is [Boost control], these bits control
> > -      post cursor manual, increase the [Boost control] to increase
> > -      Pre-emphasis value.
> > -      Registers 0~9, bit 6:4 is [Swing control], these bits control
> > -      swing manual, increase [Swing control] setting to add Vp-p value
> > -      for each Swing, Pre.
> > -      Registers 10~19 are Swing0_Pre0, Swing1_Pre0, Swing2_Pre0,
> > -      Swing3_Pre0, Swing0_Pre1, Swing1_Pre1, Swing2_Pre1, Swing0_Pre2,
> > -      Swing1_Pre2, Swing0_Pre3, they are for [R select control] and
> > -      [R Termination control] setting.
> > -      Registers 10~19, bit 4:0 is [R select control], these bits are
> > -      compensation manual, increase it can enhance IO driven strength
> > -      and Vp-p.
> > -      Registers 10~19, bit 5:6 is [R termination control], these bits
> > -      adjust 50ohm impedance of DP tx termination. 00:55 ohm,
> > -      01:50 ohm(default), 10:45 ohm, 11:40 ohm.
> > -
> > -  analogix,lane1-swing:
> > -    $ref: /schemas/types.yaml#/definitions/uint8-array
> > -    minItems: 1
> > -    maxItems: 20
> > -    description:
> > -      an array of swing register setting for DP tx lane1 PHY.
> > -      DP TX lane1 swing register setting same with lane0
> > -      swing, please refer lane0-swing property description.
>
> These apply to the DP side, so no need to revert this part.

Ack.

>
> > -
> > -  analogix,audio-enable:
> > -    type: boolean
> > -    description: let the driver enable audio HDMI codec function or not.
> > -
>
> Not sure on this one...

These additions are independent from my reading of this, would you
like a v2 with only the bus-type related changes reverted?

>
> >    ports:
> >      $ref: /schemas/graph.yaml#/properties/ports
> >
> >      properties:
> >        port@0:
> > -        $ref: /schemas/graph.yaml#/$defs/port-base
> > -        unevaluatedProperties: false
> > +        $ref: /schemas/graph.yaml#/properties/port
> >          description:
> > -          MIPI DSI/DPI input.
> > -
> > -        properties:
> > -          endpoint:
> > -            $ref: /schemas/media/video-interfaces.yaml#
> > -            type: object
> > -            additionalProperties: false
> > -
> > -            properties:
> > -              remote-endpoint: true
> > -
> > -              bus-type:
> > -                enum: [1, 5]
>
> I think the error here is really 1 should be 4 which corresponds to
> D-PHY which is used by both CSI and DSI. Otherwise, I don't really see
> the issue with bus-type being shared between CSI and DSI.

I think that would be a correct solution. And ignoring everything
else, the range of this property is something that should be fixed.

But that would mean that CPI (camera parallel interface) and DPI
(display parallel interface) would share the
V4L2_FWNODE_BUS_TYPE_PARALLEL enum. I think that would be perfectly
functional, but it is not what V4L2_FWNODE_BUS_TYPE_PARALLEL is
documented to represent. As far as I can see it's only intended to
represent CPI.

Instead of having V4L2_FWNODE_BUS_TYPE_PARALLEL represent two
standards, I think they should be split. And possibly
V4L2_FWNODE_BUS_TYPE_PARALLEL should be renamed for CPI, but that is a
separate story. This would provide for the neatest and most legible
solution. If this solution is implemented, this range would be
incorrect. Additionally the snippet reverted in 2/2 of this series
would no longer be valid.

As it stands V4L2_FWNODE_BUS_TYPE_PARALLEL was used to represent DPI
due to not being caught in the review process.

>
> > -                default: 1
> > -
> > -              data-lanes: true
> > +          Video port for MIPI DSI input.
> >
> >        port@1:
> >          $ref: /schemas/graph.yaml#/properties/port
> > @@ -143,9 +87,6 @@ examples:
> >              vdd10-supply = <&pp1000_mipibrdg>;
> >              vdd18-supply = <&pp1800_mipibrdg>;
> >              vdd33-supply = <&pp3300_mipibrdg>;
> > -            analogix,audio-enable;
> > -            analogix,lane0-swing = /bits/ 8 <0x14 0x54 0x64 0x74>;
> > -            analogix,lane1-swing = /bits/ 8 <0x14 0x54 0x64 0x74>;
> >
> >              ports {
> >                  #address-cells = <1>;
> > @@ -155,8 +96,6 @@ examples:
> >                      reg = <0>;
> >                      anx7625_in: endpoint {
> >                          remote-endpoint = <&mipi_dsi>;
> > -                        bus-type = <5>;
> > -                        data-lanes = <0 1 2 3>;
> >                      };
> >                  };
> >
> > --
> > 2.32.0

Signed-off-by: Robert Foss <robert.foss@linaro.org>

Re: [PATCH v1 1/2] Revert "dt-bindings:drm/bridge:anx7625:add vendor define"

[Laurent Pinchart]

On Mon, Mar 07, 2022 at 05:57:47PM +0100, Robert Foss wrote:
> On Mon, 7 Mar 2022 at 17:38, Rob Herring <robh@kernel.org> wrote:
> >
> > On Mon, Mar 07, 2022 at 04:45:57PM +0100, Robert Foss wrote:
> > > This reverts commit a43661e7e819b100e1f833a35018560a1d9abb39.
> >
> > S-o-b and reason for the revert?
> >
> > > ---
> > >  .../display/bridge/analogix,anx7625.yaml      | 65 +------------------
> > >  1 file changed, 2 insertions(+), 63 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > > index 1d3e88daca041..ab48ab2f4240d 100644
> > > --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > > +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > > @@ -43,70 +43,14 @@ properties:
> > >    vdd33-supply:
> > >      description: Regulator that provides the supply 3.3V power.
> > >
> > > -  analogix,lane0-swing:
> > > -    $ref: /schemas/types.yaml#/definitions/uint8-array
> > > -    minItems: 1
> > > -    maxItems: 20
> > > -    description:
> > > -      an array of swing register setting for DP tx lane0 PHY.
> > > -      Registers 0~9 are Swing0_Pre0, Swing1_Pre0, Swing2_Pre0,
> > > -      Swing3_Pre0, Swing0_Pre1, Swing1_Pre1, Swing2_Pre1, Swing0_Pre2,
> > > -      Swing1_Pre2, Swing0_Pre3, they are for [Boost control] and
> > > -      [Swing control] setting.
> > > -      Registers 0~9, bit 3:0 is [Boost control], these bits control
> > > -      post cursor manual, increase the [Boost control] to increase
> > > -      Pre-emphasis value.
> > > -      Registers 0~9, bit 6:4 is [Swing control], these bits control
> > > -      swing manual, increase [Swing control] setting to add Vp-p value
> > > -      for each Swing, Pre.
> > > -      Registers 10~19 are Swing0_Pre0, Swing1_Pre0, Swing2_Pre0,
> > > -      Swing3_Pre0, Swing0_Pre1, Swing1_Pre1, Swing2_Pre1, Swing0_Pre2,
> > > -      Swing1_Pre2, Swing0_Pre3, they are for [R select control] and
> > > -      [R Termination control] setting.
> > > -      Registers 10~19, bit 4:0 is [R select control], these bits are
> > > -      compensation manual, increase it can enhance IO driven strength
> > > -      and Vp-p.
> > > -      Registers 10~19, bit 5:6 is [R termination control], these bits
> > > -      adjust 50ohm impedance of DP tx termination. 00:55 ohm,
> > > -      01:50 ohm(default), 10:45 ohm, 11:40 ohm.
> > > -
> > > -  analogix,lane1-swing:
> > > -    $ref: /schemas/types.yaml#/definitions/uint8-array
> > > -    minItems: 1
> > > -    maxItems: 20
> > > -    description:
> > > -      an array of swing register setting for DP tx lane1 PHY.
> > > -      DP TX lane1 swing register setting same with lane0
> > > -      swing, please refer lane0-swing property description.
> >
> > These apply to the DP side, so no need to revert this part.
> 
> Ack.
> 
> >
> > > -
> > > -  analogix,audio-enable:
> > > -    type: boolean
> > > -    description: let the driver enable audio HDMI codec function or not.
> > > -
> >
> > Not sure on this one...
> 
> These additions are independent from my reading of this, would you
> like a v2 with only the bus-type related changes reverted?
> 
> >
> > >    ports:
> > >      $ref: /schemas/graph.yaml#/properties/ports
> > >
> > >      properties:
> > >        port@0:
> > > -        $ref: /schemas/graph.yaml#/$defs/port-base
> > > -        unevaluatedProperties: false
> > > +        $ref: /schemas/graph.yaml#/properties/port
> > >          description:
> > > -          MIPI DSI/DPI input.
> > > -
> > > -        properties:
> > > -          endpoint:
> > > -            $ref: /schemas/media/video-interfaces.yaml#
> > > -            type: object
> > > -            additionalProperties: false
> > > -
> > > -            properties:
> > > -              remote-endpoint: true
> > > -
> > > -              bus-type:
> > > -                enum: [1, 5]
> >
> > I think the error here is really 1 should be 4 which corresponds to
> > D-PHY which is used by both CSI and DSI. Otherwise, I don't really see
> > the issue with bus-type being shared between CSI and DSI.
> 
> I think that would be a correct solution. And ignoring everything
> else, the range of this property is something that should be fixed.
> 
> But that would mean that CPI (camera parallel interface) and DPI
> (display parallel interface) would share the
> V4L2_FWNODE_BUS_TYPE_PARALLEL enum. I think that would be perfectly
> functional, but it is not what V4L2_FWNODE_BUS_TYPE_PARALLEL is
> documented to represent. As far as I can see it's only intended to
> represent CPI.

Are you aware of any standard documenting camera parallel interfaces
with separate sync signals ? I was looking for that the other day, and
couldn't find much. CPI seems to be an old MIPI standard, but I couldn't
find any public document, I'not not sure if it actually matches.

Another common parallel interface in SoCs is AXI4 Stream, which we will
likely need a bus type for. We'll then have to decide on how to handle
on-SoC custom parallel buses.

> Instead of having V4L2_FWNODE_BUS_TYPE_PARALLEL represent two
> standards, I think they should be split. And possibly
> V4L2_FWNODE_BUS_TYPE_PARALLEL should be renamed for CPI, but that is a
> separate story. This would provide for the neatest and most legible
> solution. If this solution is implemented, this range would be
> incorrect. Additionally the snippet reverted in 2/2 of this series
> would no longer be valid.
> 
> As it stands V4L2_FWNODE_BUS_TYPE_PARALLEL was used to represent DPI
> due to not being caught in the review process.

We may end up using those values, but I think it should be discussed and
not rushed in v5.17 if possible.

> > > -                default: 1
> > > -
> > > -              data-lanes: true
> > > +          Video port for MIPI DSI input.
> > >
> > >        port@1:
> > >          $ref: /schemas/graph.yaml#/properties/port
> > > @@ -143,9 +87,6 @@ examples:
> > >              vdd10-supply = <&pp1000_mipibrdg>;
> > >              vdd18-supply = <&pp1800_mipibrdg>;
> > >              vdd33-supply = <&pp3300_mipibrdg>;
> > > -            analogix,audio-enable;
> > > -            analogix,lane0-swing = /bits/ 8 <0x14 0x54 0x64 0x74>;
> > > -            analogix,lane1-swing = /bits/ 8 <0x14 0x54 0x64 0x74>;
> > >
> > >              ports {
> > >                  #address-cells = <1>;
> > > @@ -155,8 +96,6 @@ examples:
> > >                      reg = <0>;
> > >                      anx7625_in: endpoint {
> > >                          remote-endpoint = <&mipi_dsi>;
> > > -                        bus-type = <5>;
> > > -                        data-lanes = <0 1 2 3>;
> > >                      };
> > >                  };
> > >
> > > --
> > > 2.32.0
> 
> Signed-off-by: Robert Foss <robert.foss@linaro.org>

-- 
Regards,

Laurent Pinchart

Re: [PATCH v1 1/2] Revert "dt-bindings:drm/bridge:anx7625:add vendor define"

[Rob Herring]

On Mon, Mar 7, 2022 at 11:11 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> On Mon, Mar 07, 2022 at 05:57:47PM +0100, Robert Foss wrote:
> > On Mon, 7 Mar 2022 at 17:38, Rob Herring <robh@kernel.org> wrote:
> > >
> > > On Mon, Mar 07, 2022 at 04:45:57PM +0100, Robert Foss wrote:
> > > > This reverts commit a43661e7e819b100e1f833a35018560a1d9abb39.
> > >
> > > S-o-b and reason for the revert?
> > >
> > > > ---
> > > >  .../display/bridge/analogix,anx7625.yaml      | 65 +------------------
> > > >  1 file changed, 2 insertions(+), 63 deletions(-)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > > > index 1d3e88daca041..ab48ab2f4240d 100644
> > > > --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > > > +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > > > @@ -43,70 +43,14 @@ properties:
> > > >    vdd33-supply:
> > > >      description: Regulator that provides the supply 3.3V power.
> > > >
> > > > -  analogix,lane0-swing:
> > > > -    $ref: /schemas/types.yaml#/definitions/uint8-array
> > > > -    minItems: 1
> > > > -    maxItems: 20
> > > > -    description:
> > > > -      an array of swing register setting for DP tx lane0 PHY.
> > > > -      Registers 0~9 are Swing0_Pre0, Swing1_Pre0, Swing2_Pre0,
> > > > -      Swing3_Pre0, Swing0_Pre1, Swing1_Pre1, Swing2_Pre1, Swing0_Pre2,
> > > > -      Swing1_Pre2, Swing0_Pre3, they are for [Boost control] and
> > > > -      [Swing control] setting.
> > > > -      Registers 0~9, bit 3:0 is [Boost control], these bits control
> > > > -      post cursor manual, increase the [Boost control] to increase
> > > > -      Pre-emphasis value.
> > > > -      Registers 0~9, bit 6:4 is [Swing control], these bits control
> > > > -      swing manual, increase [Swing control] setting to add Vp-p value
> > > > -      for each Swing, Pre.
> > > > -      Registers 10~19 are Swing0_Pre0, Swing1_Pre0, Swing2_Pre0,
> > > > -      Swing3_Pre0, Swing0_Pre1, Swing1_Pre1, Swing2_Pre1, Swing0_Pre2,
> > > > -      Swing1_Pre2, Swing0_Pre3, they are for [R select control] and
> > > > -      [R Termination control] setting.
> > > > -      Registers 10~19, bit 4:0 is [R select control], these bits are
> > > > -      compensation manual, increase it can enhance IO driven strength
> > > > -      and Vp-p.
> > > > -      Registers 10~19, bit 5:6 is [R termination control], these bits
> > > > -      adjust 50ohm impedance of DP tx termination. 00:55 ohm,
> > > > -      01:50 ohm(default), 10:45 ohm, 11:40 ohm.
> > > > -
> > > > -  analogix,lane1-swing:
> > > > -    $ref: /schemas/types.yaml#/definitions/uint8-array
> > > > -    minItems: 1
> > > > -    maxItems: 20
> > > > -    description:
> > > > -      an array of swing register setting for DP tx lane1 PHY.
> > > > -      DP TX lane1 swing register setting same with lane0
> > > > -      swing, please refer lane0-swing property description.
> > >
> > > These apply to the DP side, so no need to revert this part.
> >
> > Ack.
> >
> > >
> > > > -
> > > > -  analogix,audio-enable:
> > > > -    type: boolean
> > > > -    description: let the driver enable audio HDMI codec function or not.
> > > > -
> > >
> > > Not sure on this one...
> >
> > These additions are independent from my reading of this, would you
> > like a v2 with only the bus-type related changes reverted?

Yes.

> >
> > >
> > > >    ports:
> > > >      $ref: /schemas/graph.yaml#/properties/ports
> > > >
> > > >      properties:
> > > >        port@0:
> > > > -        $ref: /schemas/graph.yaml#/$defs/port-base
> > > > -        unevaluatedProperties: false
> > > > +        $ref: /schemas/graph.yaml#/properties/port
> > > >          description:
> > > > -          MIPI DSI/DPI input.
> > > > -
> > > > -        properties:
> > > > -          endpoint:
> > > > -            $ref: /schemas/media/video-interfaces.yaml#
> > > > -            type: object
> > > > -            additionalProperties: false
> > > > -
> > > > -            properties:
> > > > -              remote-endpoint: true
> > > > -
> > > > -              bus-type:
> > > > -                enum: [1, 5]
> > >
> > > I think the error here is really 1 should be 4 which corresponds to
> > > D-PHY which is used by both CSI and DSI. Otherwise, I don't really see
> > > the issue with bus-type being shared between CSI and DSI.
> >
> > I think that would be a correct solution. And ignoring everything
> > else, the range of this property is something that should be fixed.
> >
> > But that would mean that CPI (camera parallel interface) and DPI
> > (display parallel interface) would share the
> > V4L2_FWNODE_BUS_TYPE_PARALLEL enum. I think that would be perfectly
> > functional, but it is not what V4L2_FWNODE_BUS_TYPE_PARALLEL is
> > documented to represent. As far as I can see it's only intended to
> > represent CPI.
>
> Are you aware of any standard documenting camera parallel interfaces
> with separate sync signals ? I was looking for that the other day, and
> couldn't find much. CPI seems to be an old MIPI standard, but I couldn't
> find any public document, I'not not sure if it actually matches.

I don't recall. Generally, I think the camera side is not quite the
mess the display side is with all the formats.

> Another common parallel interface in SoCs is AXI4 Stream, which we will
> likely need a bus type for. We'll then have to decide on how to handle
> on-SoC custom parallel buses.

Except for those maybe.

> > Instead of having V4L2_FWNODE_BUS_TYPE_PARALLEL represent two
> > standards, I think they should be split. And possibly
> > V4L2_FWNODE_BUS_TYPE_PARALLEL should be renamed for CPI, but that is a
> > separate story. This would provide for the neatest and most legible
> > solution. If this solution is implemented, this range would be
> > incorrect. Additionally the snippet reverted in 2/2 of this series
> > would no longer be valid.
> >
> > As it stands V4L2_FWNODE_BUS_TYPE_PARALLEL was used to represent DPI
> > due to not being caught in the review process.
>
> We may end up using those values, but I think it should be discussed and
> not rushed in v5.17 if possible.

Given it's not actually used (correctly), agreed.

Rob

Re: [PATCH v1 2/2] Revert "arm64: dts: mt8183: jacuzzi: Fix bus properties in anx's DSI endpoint"

[Chen-Yu Tsai]

On Tue, Mar 8, 2022 at 12:20 AM Robert Foss <robert.foss@linaro.org> wrote:
>
> Signed-off-by: Robert Foss <robert.foss@linaro.org>

Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>

I think we need to send this directly to the soc maintainers to get it
picked up before the final 5.17 release?

>
> On Mon, 7 Mar 2022 at 16:46, Robert Foss <robert.foss@linaro.org> wrote:
> >
> > This reverts commit 32568ae37596b529628ac09b875f4874e614f63f.
> > ---
> >  arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi | 2 --
> >  1 file changed, 2 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi b/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi
> > index e8f133dc96b95..8f7bf33f607da 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi
> > +++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi
> > @@ -171,8 +171,6 @@ port@0 {
> >
> >                         anx7625_in: endpoint {
> >                                 remote-endpoint = <&dsi_out>;
> > -                               bus-type = <5>;
> > -                               data-lanes = <0 1 2 3>;
> >                         };
> >                 };
> >
> > --
> > 2.32.0
> >