On Mon, Nov 11, 2024 at 06:16:27PM -0800, Stephen Boyd wrote: > Quoting Dmitry Baryshkov (2024-11-08 23:05:18) > > On Thu, Nov 07, 2024 at 04:28:24PM -0800, Stephen Boyd wrote: > > > Quoting Dmitry Baryshkov (2024-10-31 15:54:49) > > > > On Thu, Oct 31, 2024 at 02:45:29PM -0700, Stephen Boyd wrote: > > > > > Quoting Dmitry Baryshkov (2024-10-31 11:42:36) > > > > > > On Tue, Oct 29, 2024 at 01:15:51PM -0700, Stephen Boyd wrote: > > > Long story short, I don't see how we can avoid _any_ lane assignment > > > logic in drm_bridge. The logic shouldn't walk the entire bridge chain, > > > but it should at least act on the bridge that is a DP bridge. I think > > > you're saying pretty much the same thing here, but you want the lane > > > remapping to be done via the typec layer whereas I want it to be done in > > > the drm_bridge layer. To me it looks out of place to add a > > > typec_switch_desc inside each DP drm_bridge because we duplicate the > > > logic about USB type-c DP altmode lane assignment to each DP bridge. A > > > DP bridge should just think about DP and not know or care about USB > > > type-c. > > > > > > This is what's leading me to think we need some sort of lane assignment > > > capability at the DP connector. How that assignment flows from the DP > > > connector created in drm_bridge_connector.c to the hardware is where it > > > is less clear to me. Should that be implemented as a typec_switch_desc, > > > essentially out of band with drm_bridge, or as some drm_bridge_funcs > > > function similar to struct drm_bridge_funcs::hdmi_*()? If you look at > > > IT6505 in it6505_get_extcon_property() it actually wants to pull the > > > orientation of the type-c port with extcon_get_property(EXTCON_DISP_DP, > > > EXTCON_PROP_USB_TYPEC_POLARITY). Maybe pushing the orientation to the DP > > > bridge is backwards and we should be exposing this as some sort of > > > connector API that the drm_bridge can query whenever it wants. > > > > And it6505_get_extcon_property() / EXTCON_PROP_USB_TYPEC_POLARITY is a > > Type-C code, isn't it? > > > > Sort of? It's combining DP and USB_TYPEC enums there so it's not very > clear if it's one or the other instead of just both. But EXTCON_PROP_USB_TYPEC_POLARITY is just a Type-C, nothing about DP in it. > > > > and then a drm_bridge is created in cros-ec to terminate the bridge > > > chain. The displayport altmode driver will find the drm_bridge and the > > > drm_connector from the cros-ec node. When DP altmode is entered the > > > displayport altmode driver will set the drm_connector orientation based > > > on the presence of the dp-reverse-orientation property. We'll be able to > > > hook the hpd_notify() path in cros-ec by adding code to the drm_bridge > > > made there to do the HPD workaround. I'm not sure we need to use an > > > auxiliary device in this case, because it's a one-off solution for > > > cros-ec. And we don't even need to signal HPD from the cros-ec > > > drm_bridge because the oob_hotplug event will do it for us. If anything, > > > we need that displayport.c code to skip sending the hotplug event when > > > "no-hpd" is present in the cros-ec node. Note, this works for any number > > > of usb-c-connector nodes. And finally, DP bridges like IT6505 don't need > > > to implement a typec_switch_desc, they can simply support flipping the > > > orientation by querying the drm_connector for the bridge chain when they > > > see fit. ANX7625 can support that as well when it doesn't see the > > > 'orientation-switch' property. > > > > > > Did I miss anything? I suspect a drm_connector having an orientation is > > > the most controversial part of this proposal. > > > > Yes... I understand that having orientation-switch handling in the DRM > > driver sounds strange, but this is what we do in the QMP PHY driver. It > > makes the code easier, as it keeps lane remapping local to the place > > where it belongs - to the Type-C handlers. > > > > The QMP PHY is a type-c PHY, similar to ANX7625. It sits on the output > of the DP and USB PHYs and handles the type-c orientation and lane > merging for different USB type-c alternate modes. It's not a great > example of a plain DP bridge because it combines USB and USB type-c > features. > > Either way, doing this through Type-C handlers is weird because the port > orientation in the Type-C framework is for the connector and there is an > orientation control hardware that handles the orientation already. For > example, with the IT6505 part on Corsola, the orientation is controlled > by a redriver part that the EC controls. It takes the DP and USB signals > and routes them to the correct pins on the usb-c-connector depending on > the cable orientation. The input side pinout is basically 2 or 4 lanes > DP and 2 lanes USB and the output side pinout is the USB type-c pinout > SSTXRX1 and SSTXRX2. > > This redriver is equivalent to the QMP PHY type-c part. Maybe to bring > this example closer to QMP we can imagine if the QMP PHY was split into > two pairs of lanes, and the USB functionality wasn't used. The > orientation control for a usb-c-connector would be on a redriver that > takes 2 DP lanes from the QMP PHY as input. Saying that this QMP PHY is > the "orientation-switch" with that property in DT is confusing, because > it isn't controlling the orientation of the type-c port. The orientation > is handled by the redriver. That redriver may even be controlled by the > kernel as an orientation-switch. This is clear. > > I understand that the QMP PHY driver has implemented the lane control > for orientation with a typec_switch_desc, but the QMP PHY is a plain DP > PHY in this scenario. How would the type-c handlers work here? We > couldn't call them through the type-c framework as far as I can tell. If QMP PHY is a plain DP PHY, it usually has no support for lane remapping (e.g. phy-qcom-edp doesn't). Let me reiterate, please: lane management is outside of the DisplayPort spec, at least as far as I can understand it. All lane remapping (especially a dynamic one) is a pure vendor extension to the standard. I'm trying to find a way to support Corsola and Trogdor without adding "this is done specially for Google" kind of API. Usually that doesn't fly in the long term. I understand that using Type-C API for the DRM bridge sounds strange. But even the mentioned bridge uses Type-C API. It asks for the Type-C polarity, not the DP polarity. > This is why I'm thinking the end of the bridge chain needs to have some > sort of orientation. If we had that then the place where the chain ends > and becomes muxed onto the usb-c-connector, i.e. the redriver, would be > where the DP bridge is told that it needs to flip the lanes. In the > cases I have, the redriver is the EC, and so we've combined them all > together in one node, cros-ec-typec. In the QMP PHY case the redriver is > the QMP PHY type-c part that sits on the DP and USB PHYs and sends their > signals out of the SoC. > > Maybe the DT property in the ANX7625 or IT6505 node should be something > like "dp-orientation-switch" and then we have the type-c framework find > this property? Then we would need to add support for that property in > IT6505 using a typec_switch_desc, which is weird. I guess it all feels > like a hack because it's not always the case that the DP PHY is glued to > a USB type-c PHY. I think just "orientation-switch" is enough. In the end it's not a "typec-orientation-switch". -- With best wishes Dmitry
