On Thu, Apr 20, 2023 at 5:10 PM Pin-yen Lin <treapking@xxxxxxxxxxxx> wrote: > > On Thu, Apr 20, 2023 at 2:10 PM Stephen Boyd <swboyd@xxxxxxxxxxxx> wrote: > > > > Quoting Stephen Boyd (2023-04-13 17:22:46) > > > Quoting Pin-yen Lin (2023-04-13 02:50:44) > > > > > > > > Actually the `mode-switch` property here is mainly because > > > > `fwnode_typec_mux_get`[1] and `typec_mux_match`[2] only return matches > > > > when the property is present. I am not sure what side effects would be > > > > if I remove the ID-matching condition in `typec_mux_match`, so I added > > > > the property here. > > > > > > > > Is it feasible to remove the `mode-switch` property here given the > > > > existing implementation of the Type-C framework? > > > > > > Omitting the mode-switch property would require changes to the type-c > > > framework. > > > > > > I'm wondering if we can have this anx driver register mode switches for > > > however many endpoints exist in the output port all the time when the > > > aux-bus node doesn't exist. Then the type-c framework can walk from the > > > usb-c-connector to each connected node looking for a device that is both > > > a drm_bridge and a mode-switch. When it finds that combination, it knows > > > that the mode-switch has been found. This hinges on the idea that a > > > device that would have the mode-switch property is a drm_bridge and > > > would register a mode-switch with the type-c framework. I spent some time working on this approach on the Type-C side. The issue I met is that the driver doesn't know whether a node is a drm_bridge before the anx7625 driver probes. When there is a "mode-switch" property in the node, the Type-C framework knows that "here is a mode switch, but the corresponding driver hasn't registered the typec_mux". So it returns -EPROBE_DEFER and retries later. However, if we remove the property, the Type-C framework won't know whether a node will be registered as a drm_bridge and register a typec_mux. Do you have other suggestions on this if we want to choose this approach? Best regards, Pin-yen > > > > > > It may be a little complicated though, because we would only register > > > one drm_bridge for the input to this anx device. The type-c walking code > > > would need to look at the graph endpoint, and find the parent device to > > > see if it is a drm_bridge. > > > > I've been thinking more about this. I think we should only have the > > 'mode-switch' property possible when the USB input pins (port@2) are > > connected and the DPI input pins are connected (port@0). Probably you > > don't have that case though? > > No we don't have the use case that uses the USB input pins on anx7625. > > > > In your case, this device should register either one or two drm_bridges > > that connect to whatever downstream is actually muxing the 2 DP lanes > > with the USB SS lanes onto the usb-c-connector. > > What do you mean by "muxing the 2 DP lanes with the USB SS lanes''? In > our use case, the USB data lanes from both ports are connected to a > USB hub, but the DP lanes are muxed by the crosspoint switch on > anx7625. HPD and AUX for the external display are muxed by the EC. You > can find the diagram at > https://lore.kernel.org/linux-usb/YxGzk6DNAt0aCvIY@xxxxxxxxxxxx/ > > > If that is the EC for > > ChromeOS, then the EC should have a binding that accepts some number of > > input ports for DP. The EC would act as a drm_bridge, or in this case > > probably two bridges, and also as two type-c switches for each > > drm_bridge corresponding to the usb-c-connector nodes. When DP is on the > > cable, the type-c switch/mux would signal to the drm_bridge that the > > display is 'connected' via DRM_BRIDGE_OP_DETECT and struct > > drm_bridge_funcs::detect(). Then the drm_bridge in this anx part would > > implement struct drm_bridge_funcs::atomic_enable() and configure the > > crosspoint switch the right way depending on the reg property of the > > output node in port@1. > > So there will be two drm bridges that act as the downstreams for > anx7625, and we find the downstream with connector_status_connected to > configure the crosspoint switch? How do we support that kind of > topology given that the drm bridge chain is currently a list? Are you > suggesting making the bridge topology to a tree, or maintaining the > two downstreams inside the anx7625 driver and not attaching them to > the bridge chain? > > Also, if we still register mode switches on the two downstream > bridges, why do you prefer that over the original approach that > register switches in the anx7625 driver? > > > > > Because you don't have the part that implements the orientation-switch, > > you don't need to implement the code for it. I think simply adding > > support in the binding for mode-switch and orientation-switch if this is > > directly wired to a usb-c-connector should be sufficient. Those > > properties would be at the top-level and not part of the graph binding.
