* [PATCH v1 0/2] Revert vendor property from anx7625 bindings
@ 2022-03-07 15:45 Robert Foss
2022-03-07 15:45 ` [PATCH v1 1/2] Revert "dt-bindings:drm/bridge:anx7625:add vendor define" Robert Foss
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Robert Foss @ 2022-03-07 15:45 UTC (permalink / raw)
To: airlied, daniel, robh+dt, matthias.bgg, robert.foss, xji, hsinyi,
dri-devel, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, Laurent Pinchart, Chen-Yu Tsai
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
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH v1 1/2] Revert "dt-bindings:drm/bridge:anx7625:add vendor define" 2022-03-07 15:45 [PATCH v1 0/2] Revert vendor property from anx7625 bindings Robert Foss @ 2022-03-07 15:45 ` Robert Foss 2022-03-07 16:20 ` Robert Foss 2022-03-07 16:38 ` Rob Herring 2022-03-07 15:45 ` [PATCH v1 2/2] Revert "arm64: dts: mt8183: jacuzzi: Fix bus properties in anx's DSI endpoint" Robert Foss 2022-03-07 15:51 ` [PATCH v1 0/2] Revert vendor property from anx7625 bindings Laurent Pinchart 2 siblings, 2 replies; 12+ messages in thread From: Robert Foss @ 2022-03-07 15:45 UTC (permalink / raw) To: airlied, daniel, robh+dt, matthias.bgg, robert.foss, xji, hsinyi, dri-devel, devicetree, linux-kernel, linux-arm-kernel, linux-mediatek, Laurent Pinchart, Chen-Yu Tsai 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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v1 1/2] Revert "dt-bindings:drm/bridge:anx7625:add vendor define" 2022-03-07 15:45 ` [PATCH v1 1/2] Revert "dt-bindings:drm/bridge:anx7625:add vendor define" Robert Foss @ 2022-03-07 16:20 ` Robert Foss 2022-03-07 16:38 ` Rob Herring 1 sibling, 0 replies; 12+ messages in thread From: Robert Foss @ 2022-03-07 16:20 UTC (permalink / raw) To: airlied, daniel, robh+dt, matthias.bgg, robert.foss, xji, hsinyi, dri-devel, devicetree, linux-kernel, linux-arm-kernel, linux-mediatek, Laurent Pinchart, Chen-Yu Tsai 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> ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 1/2] Revert "dt-bindings:drm/bridge:anx7625:add vendor define" 2022-03-07 15:45 ` [PATCH v1 1/2] Revert "dt-bindings:drm/bridge:anx7625:add vendor define" Robert Foss 2022-03-07 16:20 ` Robert Foss @ 2022-03-07 16:38 ` Rob Herring 2022-03-07 16:57 ` Robert Foss 1 sibling, 1 reply; 12+ messages in thread From: Rob Herring @ 2022-03-07 16:38 UTC (permalink / raw) To: Robert Foss Cc: airlied, daniel, matthias.bgg, xji, hsinyi, dri-devel, devicetree, linux-kernel, linux-arm-kernel, linux-mediatek, Laurent Pinchart, Chen-Yu Tsai 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 > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 1/2] Revert "dt-bindings:drm/bridge:anx7625:add vendor define" 2022-03-07 16:38 ` Rob Herring @ 2022-03-07 16:57 ` Robert Foss 2022-03-07 17:10 ` Laurent Pinchart 0 siblings, 1 reply; 12+ messages in thread From: Robert Foss @ 2022-03-07 16:57 UTC (permalink / raw) To: Rob Herring Cc: airlied, daniel, matthias.bgg, xji, hsinyi, dri-devel, devicetree, linux-kernel, linux-arm-kernel, linux-mediatek, Laurent Pinchart, Chen-Yu Tsai 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> ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 1/2] Revert "dt-bindings:drm/bridge:anx7625:add vendor define" 2022-03-07 16:57 ` Robert Foss @ 2022-03-07 17:10 ` Laurent Pinchart 2022-03-07 17:32 ` Rob Herring 0 siblings, 1 reply; 12+ messages in thread From: Laurent Pinchart @ 2022-03-07 17:10 UTC (permalink / raw) To: Robert Foss Cc: Rob Herring, airlied, daniel, matthias.bgg, xji, hsinyi, dri-devel, devicetree, linux-kernel, linux-arm-kernel, linux-mediatek, Chen-Yu Tsai 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 1/2] Revert "dt-bindings:drm/bridge:anx7625:add vendor define" 2022-03-07 17:10 ` Laurent Pinchart @ 2022-03-07 17:32 ` Rob Herring 0 siblings, 0 replies; 12+ messages in thread From: Rob Herring @ 2022-03-07 17:32 UTC (permalink / raw) To: Laurent Pinchart, Robert Foss Cc: David Airlie, Daniel Vetter, Matthias Brugger, Xin Ji, Hsin-Yi Wang, dri-devel, devicetree, linux-kernel, linux-arm-kernel, moderated list:ARM/Mediatek SoC support, Chen-Yu Tsai 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v1 2/2] Revert "arm64: dts: mt8183: jacuzzi: Fix bus properties in anx's DSI endpoint" 2022-03-07 15:45 [PATCH v1 0/2] Revert vendor property from anx7625 bindings Robert Foss 2022-03-07 15:45 ` [PATCH v1 1/2] Revert "dt-bindings:drm/bridge:anx7625:add vendor define" Robert Foss @ 2022-03-07 15:45 ` Robert Foss 2022-03-07 16:20 ` Robert Foss 2022-03-07 16:43 ` Rob Herring 2022-03-07 15:51 ` [PATCH v1 0/2] Revert vendor property from anx7625 bindings Laurent Pinchart 2 siblings, 2 replies; 12+ messages in thread From: Robert Foss @ 2022-03-07 15:45 UTC (permalink / raw) To: airlied, daniel, robh+dt, matthias.bgg, robert.foss, xji, hsinyi, dri-devel, devicetree, linux-kernel, linux-arm-kernel, linux-mediatek, Laurent Pinchart, Chen-Yu Tsai 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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v1 2/2] Revert "arm64: dts: mt8183: jacuzzi: Fix bus properties in anx's DSI endpoint" 2022-03-07 15:45 ` [PATCH v1 2/2] Revert "arm64: dts: mt8183: jacuzzi: Fix bus properties in anx's DSI endpoint" Robert Foss @ 2022-03-07 16:20 ` Robert Foss 2022-03-08 3:47 ` Chen-Yu Tsai 2022-03-07 16:43 ` Rob Herring 1 sibling, 1 reply; 12+ messages in thread From: Robert Foss @ 2022-03-07 16:20 UTC (permalink / raw) To: airlied, daniel, robh+dt, matthias.bgg, robert.foss, xji, hsinyi, dri-devel, devicetree, linux-kernel, linux-arm-kernel, linux-mediatek, Laurent Pinchart, Chen-Yu Tsai 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 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 2/2] Revert "arm64: dts: mt8183: jacuzzi: Fix bus properties in anx's DSI endpoint" 2022-03-07 16:20 ` Robert Foss @ 2022-03-08 3:47 ` Chen-Yu Tsai 0 siblings, 0 replies; 12+ messages in thread From: Chen-Yu Tsai @ 2022-03-08 3:47 UTC (permalink / raw) To: Robert Foss, Arnd Bergmann, Olof Johansson Cc: airlied, daniel, robh+dt, matthias.bgg, xji, hsinyi, dri-devel, devicetree, linux-kernel, linux-arm-kernel, linux-mediatek, Laurent Pinchart 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 > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 2/2] Revert "arm64: dts: mt8183: jacuzzi: Fix bus properties in anx's DSI endpoint" 2022-03-07 15:45 ` [PATCH v1 2/2] Revert "arm64: dts: mt8183: jacuzzi: Fix bus properties in anx's DSI endpoint" Robert Foss 2022-03-07 16:20 ` Robert Foss @ 2022-03-07 16:43 ` Rob Herring 1 sibling, 0 replies; 12+ messages in thread From: Rob Herring @ 2022-03-07 16:43 UTC (permalink / raw) To: Robert Foss Cc: airlied, daniel, matthias.bgg, xji, hsinyi, dri-devel, devicetree, linux-kernel, linux-arm-kernel, linux-mediatek, Laurent Pinchart, Chen-Yu Tsai 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 0/2] Revert vendor property from anx7625 bindings 2022-03-07 15:45 [PATCH v1 0/2] Revert vendor property from anx7625 bindings Robert Foss 2022-03-07 15:45 ` [PATCH v1 1/2] Revert "dt-bindings:drm/bridge:anx7625:add vendor define" Robert Foss 2022-03-07 15:45 ` [PATCH v1 2/2] Revert "arm64: dts: mt8183: jacuzzi: Fix bus properties in anx's DSI endpoint" Robert Foss @ 2022-03-07 15:51 ` Laurent Pinchart 2 siblings, 0 replies; 12+ messages in thread From: Laurent Pinchart @ 2022-03-07 15:51 UTC (permalink / raw) To: Robert Foss Cc: airlied, daniel, robh+dt, matthias.bgg, xji, hsinyi, dri-devel, devicetree, linux-kernel, linux-arm-kernel, linux-mediatek, Chen-Yu Tsai 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2022-03-08 3:48 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-03-07 15:45 [PATCH v1 0/2] Revert vendor property from anx7625 bindings Robert Foss 2022-03-07 15:45 ` [PATCH v1 1/2] Revert "dt-bindings:drm/bridge:anx7625:add vendor define" Robert Foss 2022-03-07 16:20 ` Robert Foss 2022-03-07 16:38 ` Rob Herring 2022-03-07 16:57 ` Robert Foss 2022-03-07 17:10 ` Laurent Pinchart 2022-03-07 17:32 ` Rob Herring 2022-03-07 15:45 ` [PATCH v1 2/2] Revert "arm64: dts: mt8183: jacuzzi: Fix bus properties in anx's DSI endpoint" Robert Foss 2022-03-07 16:20 ` Robert Foss 2022-03-08 3:47 ` Chen-Yu Tsai 2022-03-07 16:43 ` Rob Herring 2022-03-07 15:51 ` [PATCH v1 0/2] Revert vendor property from anx7625 bindings Laurent Pinchart
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).