Quoting Pin-yen Lin (2023-03-31 02:11:39) > diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > index b42553ac505c..604c7391d74f 100644 > --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > @@ -12,7 +12,8 @@ maintainers: > > description: | > The ANX7625 is an ultra-low power 4K Mobile HD Transmitter > - designed for portable devices. > + designed for portable devices. Product brief is available at > + https://www.analogix.com/en/system/files/AA-002291-PB-6-ANX7625_ProductBrief.pdf > > properties: > compatible: > @@ -112,9 +113,40 @@ properties: > data-lanes: true > > port@1: > - $ref: /schemas/graph.yaml#/properties/port > + $ref: /schemas/graph.yaml#/$defs/port-base > description: > - Video port for panel or connector. > + Video port for panel or connector. Each endpoint connects to a video > + output downstream, and the "data-lanes" property is used to describe > + the pin connections. 0, 1, 2, 3 in "data-lanes" maps to SSRX1, SSTX1, > + SSRX2, SSTX2, respectively. > + > + patternProperties: > + "^endpoint@[01]$": > + $ref: /schemas/media/video-interfaces.yaml# > + properties: > + reg: true > + > + remote-endpoint: true > + > + data-lanes: > + oneOf: > + - items: > + - enum: [0, 1, 2, 3] > + > + - items: > + - const: 0 > + - const: 1 > + > + - items: > + - const: 2 > + - const: 3 > + > + mode-switch: Is it possible to not have this property? Can we have the driver for this anx device look at the remote-endpoint and if it sees that it is not a drm_bridge or panel on the other end, or a DP connector, that it should register a typec mode switch (or two depending on the number of endpoints in port@1)? Is there any case where that doesn't hold true? I see these possible scenarios: 1. DPI to DP bridge steering DP to one of two usb-c-connectors In this case, endpoint@0 is connected to one usb-c-connector and endpoint@1 is connected to another usb-c-connector. The input endpoint is only connected to DPI. The USB endpoint is not present (although I don't see this described in the binding either, so we would need a port@2, entirely optional to describe USB3 input). The driver will register two mode switches. 2. DPI to DP bridge with USB3 to one usb-c-connector In this case, endpoint@1 doesn't exist. The SSTX1/2 and SSRX1/2 pins are all connected to a usb-c-connector node. The input ports (0 and 2) are connected to both DPI and USB. The device acts as both a mode-switch and an orientation-switch. It registers both switches. I wonder if there is any benefit to describing SBU connections or CC connections? Maybe we don't register the orientation-switch if the SBU or CC connection isn't described? 3. DPI to DP bridge connected to eDP panel In this case, endpoint@1 doesn't exist. The USB endpoint is not present (port@2). Depending on how the crosspoint should be configured, we'll need to use data-lanes in the port@1 endpoint to describe which SSTRX pair to use (1 or 2). Or we'll have to use the endpoint's reg property to describe which pair to drive DP on. Presumably the default configuration is SSRX2/SSTX2 providing 2 lanes of DP to an eDP panel. The endpoint@0 in port@1 will be connected to a drm_panel, and the driver will be able to detect this properly by checking for the existence of an aux-bus node or the return value of of_dp_aux_populate_bus(). 4. DPI to DP bridge connected to DP connector This is similar to the eDP panel scenario #3. In this case, endpoint@1 doesn't exist. The USB endpoint is not present (port@2). Same story about port@1 and lane configuration, but we don't have an aux-bus node. In this case, the drivers/gpu/drm/bridge/display-connector.c driver will probe for the dp-connector node and add a drm_bridge. This anx driver will similarly add a drm_bridge, but it needs to look at the node connected on port@1:endpoint@0 with drm_of_get_bridge() and check if it is a drm_bridge (DP connector) or if it is some type-c thing (connector or orientation-switch). I think having this mode-switch property here lets us avoid calling drm_of_get_bridge() unconditionally in anx7625_parse_dt(). drm_of_get_bridge() will always return -EPROBE_DEFER when this is the last drm_bridge in the chain and the other side of the endpoint is a type-c thing (scenarios #1 and #2). Maybe we should teach drm_of_get_bridge() that a drm_bridge might be connected to a type-c device and have it not return -EPROBE_DEFER in that case. Or make some new API like drm_of_get_bridge_typec() that checks if the typec framework knows about the endpoint in question (as either a typec switch or a connector) and returns a NULL bridge pointer. If we had that then I think this property is not necessary. Hopefully the usb-c-connector can always be registered with the typec framework? I'm worried that the driver that registers the usb-c-connector node may want to form a struct typec_port with typec_register_port() and that will get stuck in a similar -EPROBE_DEFER loop waiting for this mode-switch to appear. So having this property also avoids that problem by telling typec framework to wait until this driver can register a mode-switch. TL;DR: Is this mode-switch property a workaround for probe defer? Can we figure out where the mode switch is in software and not have the property in DT? If we can it would certainly improve things because forgetting to add the property can lead to broken behavior, and we don't do anything like this for chains of drm_bridge devices. We just describe the display chain and let the kernel figure out which bridge should handle hpd, edid reading, or mode detection, etc.
