Hi Laurent Pinchart, thanks for your comment. On Fri, Sep 20, 2019 at 12:46:21PM +0300, Laurent Pinchart wrote: > Hello Xin Ji, > > Thank you for the patch. > > On Fri, Sep 20, 2019 at 06:05:03AM +0000, Xin Ji wrote: > > The ANX7625 is an ultra-low power 4K Mobile HD Transmitter designed > > for portable device. It converts MIPI to DisplayPort 1.3 4K. > > I assume you meant MIPI DSI ? MIPI has released more standards than DSI, > so it doesn't hurt to specify this explicitly. It support DSI and DPI, I will to point out. > > According to > https://www.analogix.com/en/system/files/AA-002291-PB-6-ANX7625_ProductBrief_0.pdf, > the ANX7625 supports for MIPI DSI and DPI on the input side. > Furthermore, it seems to output DisplayPort on USB Type-C. Should this > be documented ? It can support both eDP output or USB Type-C output. > > > You can add support to your board with binding. > > > > Example: > > anx_bridge: anx7625@58 { > > compatible = "analogix,anx7625"; > > reg = <0x58>; > > low-power-mode = <1>; > > dsi-supported = <1>; > > dsi-channel-id = <1>; > > dsi-lanes-num = <4>; > > internal-pannel-supported = <1>; > > pon-gpios = <&gpio0 45 GPIO_ACTIVE_LOW>; > > reset-gpios = <&gpio0 73 GPIO_ACTIVE_LOW>; > > status = "okay"; > > port { > > anx7625_1_in: endpoint { > > remote-endpoint = <&mipi_dsi_bridge_1>; > > }; > > }; > > }; > > > > Signed-off-by: Xin Ji <xji@xxxxxxxxxxxxxxxx> > > --- > > .../bindings/display/bridge/anx7625.yaml | 84 ++++++++++++++++++++++ > > 1 file changed, 84 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/display/bridge/anx7625.yaml > > > > diff --git a/Documentation/devicetree/bindings/display/bridge/anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/anx7625.yaml > > new file mode 100644 > > index 0000000..95fe18b > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/display/bridge/anx7625.yaml > > @@ -0,0 +1,84 @@ > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > +# Copyright 2019 Analogix Semiconductor, Inc. > > +%YAML 1.2 > > +--- > > +$id: "http://devicetree.org/schemas/display/bridge/anx7625.yaml#"; > > +$schema: "http://devicetree.org/meta-schemas/core.yaml#"; > > + > > +title: Analogix ANX7625 SlimPort (4K Mobile HD Transmitter) > > + > > +maintainers: > > + - Xin Ji <xji@xxxxxxxxxxxxxxxx> > > + > > +description: | > > + The ANX7625 is an ultra-low power 4K Mobile HD Transmitter > > + designed for portable devices. > > + > > +properties: > > + compatible: > > + items: > > + - const: analogix,anx7625 > > + > > + reg: > > + maxItems: 1 > > + > > + low-power-gpios: > > + description: Low power mode support feature > > + maxItems: 1 > > + > > + hpd-gpios: > > + description: used for HPD interrupt > > + maxItems: 1 > > + > > + pon-gpios: > > + description: used for power on chip control > > + maxItems: 1 > > + > > + reset-gpios: > > + description: used for reset chip control > > + maxItems: 1 > > How about mentioning which pin of the ANX7625 each GPIO refers to ? For > the low-power, pon and reset GPIOs I assume they directly control the > chip. We have standard names for some GPIOs, such as reset or enable. Is > there one of the low-power and pon GPIOs that would qualify as an enable > signal ? OK, I think pon-gpios can qualify as an enable. > > What is the HPD GPIO for ? Does the chip output and HPD signal ? Once the anx7625 received eDP HPD signal, the firmware will report HPD interrupt to AP through defined gpio interrupt pin "hpd-gpios". It used for interrupt between anx7625 and AP, not used for output. > > > + > > + extcon-supported: > > + description: external connector interface support flag > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + > > + internal-pannel-supported: > > + description: indicate does internal pannel exist or not > > + $ref: /schemas/types.yaml#/definitions/uint32 > > s/pannel/panel/ OK, will fix it. > > Are those two properties mutually exclusive ? If so you should combine > them in a single required property with two values. My feeling is that > they should be dropped though, please see below. Yes, they are mutually exclusive. There are 3 case, one is support google "external connector" framework, one support internal panel, the other is support normal eDP output, so I defined two flags to distinguish them here. Based on your comment below, I'll define output port to distinguish them. > > > + > > + dsi-supported: > > + description: support MIPI DSI or DPI > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + > > + dsi-channel-id: > > + description: dsi channel index > > + $ref: /schemas/types.yaml#/definitions/uint32 > > This does not belong to DT, the virtual channel used by the DSI source > should be queried at runtime by communicating between drivers. I didn't know where can quiry this value. Can you give me more detail? > > > + > > + dsi-lanes-num: > > + description: dsi lanes used num > > + $ref: /schemas/types.yaml#/definitions/uint32 > > Please use the standard data-lanes property as defined in > video-interface.txt. It should be specified in the DSI endpoints. OK, I'll fetch the dsi lanes from DSI endpoints. > > > + > > + port@0: > > + type: object > > + description: > > + A port node pointing to MIPI DSI host port node. > > You need at least 3 ports: > > - a DPI input port I'll add DPI input port. > - a DSI input port > - an output port > > The dsi-supported property should be dropped, detecting the type of > input should be done based on which of the DPI or DSI input port is > connected. OK. > > Assuming the ANX7625 can also output DisplayPort directly without going > through USB Type-C (I can't verify that without the datasheet), I think > you should use two output ports, one for USB Type-C and one for > DisplayPort. The extcon-supported and internal-pannel-supported > properties should be removed, and deduced from the connect output port. I think there should exist 3 output port, one is for extern connector, one is for normal eDP, other is for internal panel eDP. > > > + > > +required: > > + - compatible > > + - reg > > + - dsi-channel-id > > + - dsi-lanes-num > > + - port@0 > > + > > +example: > > + - | > > + anx_bridge: anx7625@58 { > > + compatible = "analogix,anx7625"; > > + reg = <0x58>; > > + low-power-gpios = <0>; > > + dsi-supported = <1>; > > + dsi-channel-id = <1>; > > + dsi-lanes-num = <4>; > > + hpd-gpios = <&gpio1 19 IRQ_TYPE_LEVEL_LOW>; > > + status = "okay"; > > + }; > > You mention the port@0 node as being required, but it's missing from the > example. OK, I'll change it in the next series. > > -- > Regards, > > Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
