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(-)
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>; }; };
On Mon, 7 Mar 2022 at 16:46, Robert Foss robert.foss@linaro.org wrote:
Signed-off-by: Robert Foss robert.foss@linaro.org
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?
These apply to the DP side, so no need to revert this part.
Not sure on this one...
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.
On Mon, 7 Mar 2022 at 17:38, Rob Herring robh@kernel.org wrote:
Ack.
These additions are independent from my reading of this, would you like a v2 with only the bus-type related changes reverted?
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.
Signed-off-by: Robert Foss robert.foss@linaro.org
On Mon, Mar 07, 2022 at 05:57:47PM +0100, Robert Foss wrote:
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.
We may end up using those values, but I think it should be discussed and not rushed in v5.17 if possible.
On Mon, Mar 7, 2022 at 11:11 AM Laurent Pinchart laurent.pinchart@ideasonboard.com wrote:
Yes.
I don't recall. Generally, I think the camera side is not quite the mess the display side is with all the formats.
Except for those maybe.
Given it's not actually used (correctly), agreed.
Rob
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>; }; };
Signed-off-by: Robert Foss robert.foss@linaro.org
On Mon, 7 Mar 2022 at 16:46, Robert Foss robert.foss@linaro.org wrote:
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, Mar 07, 2022 at 04:45:58PM +0100, Robert Foss wrote:
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
Hi Rob,
Thank you for the patch.
On Mon, Mar 07, 2022 at 04:45:56PM +0100, Robert Foss wrote:
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
dri-devel@lists.freedesktop.org
-
Chen-Yu Tsai -
Laurent Pinchart -
Rob Herring -
Robert Foss