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. > > 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. 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. 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.
