| Message ID | 20240819223834.2049862-17-swboyd@chromium.org |
|---|---|
| State | New |
| Headers | show |
| Series | platform/chrome: Add DT USB/DP muxing/topology support | expand |
On Mon, Aug 19, 2024 at 03:38:30PM -0700, Stephen Boyd wrote: > @@ -671,6 +674,20 @@ static int cros_typec_configure_mux(struct cros_typec_data *typec, int port_num, > if (port->mux_flags == resp.flags && port->role == pd_ctrl->role) > return 0; > > + dp_enabled = resp.flags & USB_PD_MUX_DP_ENABLED; > + hpd_asserted = resp.flags & USB_PD_MUX_HPD_LVL; > + /* > + * Assume the first port to have HPD asserted is the one muxed to DP > + * (i.e. active_port). When there's only one port this delays setting > + * the active_port until HPD is asserted, but before that the > + * drm_connector looks disconnected so active_port doesn't need to be > + * set. > + */ > + if (dp_bridge && hpd_asserted && !dp_bridge->active_port) > + dp_bridge->active_port = port; > + > + is_active_port = !dp_bridge || dp_bridge->active_port == port; Why `!dp_bridge`? When will `dp_bridge` be NULL?
Quoting Tzung-Bi Shih (2024-08-22 07:37:05) > On Mon, Aug 19, 2024 at 03:38:30PM -0700, Stephen Boyd wrote: > > @@ -671,6 +674,20 @@ static int cros_typec_configure_mux(struct cros_typec_data *typec, int port_num, > > if (port->mux_flags == resp.flags && port->role == pd_ctrl->role) > > return 0; > > > > + dp_enabled = resp.flags & USB_PD_MUX_DP_ENABLED; > > + hpd_asserted = resp.flags & USB_PD_MUX_HPD_LVL; > > + /* > > + * Assume the first port to have HPD asserted is the one muxed to DP > > + * (i.e. active_port). When there's only one port this delays setting > > + * the active_port until HPD is asserted, but before that the > > + * drm_connector looks disconnected so active_port doesn't need to be > > + * set. > > + */ > > + if (dp_bridge && hpd_asserted && !dp_bridge->active_port) > > + dp_bridge->active_port = port; > > + > > + is_active_port = !dp_bridge || dp_bridge->active_port == port; > > Why `!dp_bridge`? When will `dp_bridge` be NULL? I'll add a comment. 'dp_bridge' is NULL when this driver is running on non-DT platforms, i.e. ACPI, or there isn't a graph/ports node for this device. The latter could happen if there's some AP controlled piece of hardware that is a typec switch, connected directly to a usb-c-connector. This is the case on Kukui where we send the DP lanes directly to the usb-c-connector.
On Fri, Aug 23, 2024 at 01:44:41PM -0700, Stephen Boyd wrote: > Quoting Tzung-Bi Shih (2024-08-22 07:37:05) > > On Mon, Aug 19, 2024 at 03:38:30PM -0700, Stephen Boyd wrote: ... > > > + /* > > > + * Assume the first port to have HPD asserted is the one muxed to DP > > > + * (i.e. active_port). When there's only one port this delays setting > > > + * the active_port until HPD is asserted, but before that the > > > + * drm_connector looks disconnected so active_port doesn't need to be > > > + * set. > > > + */ > > > + if (dp_bridge && hpd_asserted && !dp_bridge->active_port) > > > + dp_bridge->active_port = port; > > > + > > > + is_active_port = !dp_bridge || dp_bridge->active_port == port; > > > > Why `!dp_bridge`? When will `dp_bridge` be NULL? > > I'll add a comment. > > 'dp_bridge' is NULL when this driver is running on non-DT platforms, > i.e. ACPI, or there isn't a graph/ports node for this device. The latter > could happen if there's some AP controlled piece of hardware that is a > typec switch, connected directly to a usb-c-connector. This is the case > on Kukui where we send the DP lanes directly to the usb-c-connector. Thanks!
diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c index f7e184fa90c5..b32abd14825c 100644 --- a/drivers/platform/chrome/cros_ec_typec.c +++ b/drivers/platform/chrome/cros_ec_typec.c @@ -27,6 +27,7 @@ struct cros_typec_dp_bridge { struct cros_typec_data *typec_data; struct drm_dp_typec_bridge_dev *dev; + struct cros_typec_port *active_port; }; #define DP_PORT_VDO (DP_CONF_SET_PIN_ASSIGN(BIT(DP_PIN_ASSIGN_C) | BIT(DP_PIN_ASSIGN_D)) | \ @@ -651,6 +652,7 @@ static int cros_typec_configure_mux(struct cros_typec_data *typec, int port_num, struct ec_response_usb_pd_control_v2 *pd_ctrl) { struct cros_typec_port *port = typec->ports[port_num]; + struct cros_typec_dp_bridge *dp_bridge = typec->dp_bridge; struct ec_response_usb_pd_mux_info resp; struct ec_params_usb_pd_mux_info req = { .port = port_num, @@ -658,6 +660,7 @@ static int cros_typec_configure_mux(struct cros_typec_data *typec, int port_num, struct ec_params_usb_pd_mux_ack mux_ack; enum typec_orientation orientation; int ret; + bool dp_enabled, hpd_asserted, is_active_port; ret = cros_ec_cmd(typec->ec, 0, EC_CMD_USB_PD_MUX_INFO, &req, sizeof(req), &resp, sizeof(resp)); @@ -671,6 +674,20 @@ static int cros_typec_configure_mux(struct cros_typec_data *typec, int port_num, if (port->mux_flags == resp.flags && port->role == pd_ctrl->role) return 0; + dp_enabled = resp.flags & USB_PD_MUX_DP_ENABLED; + hpd_asserted = resp.flags & USB_PD_MUX_HPD_LVL; + /* + * Assume the first port to have HPD asserted is the one muxed to DP + * (i.e. active_port). When there's only one port this delays setting + * the active_port until HPD is asserted, but before that the + * drm_connector looks disconnected so active_port doesn't need to be + * set. + */ + if (dp_bridge && hpd_asserted && !dp_bridge->active_port) + dp_bridge->active_port = port; + + is_active_port = !dp_bridge || dp_bridge->active_port == port; + port->mux_flags = resp.flags; port->role = pd_ctrl->role; @@ -698,8 +715,11 @@ static int cros_typec_configure_mux(struct cros_typec_data *typec, int port_num, ret = cros_typec_enable_usb4(typec, port_num, pd_ctrl); } else if (port->mux_flags & USB_PD_MUX_TBT_COMPAT_ENABLED) { ret = cros_typec_enable_tbt(typec, port_num, pd_ctrl); - } else if (port->mux_flags & USB_PD_MUX_DP_ENABLED) { - ret = cros_typec_enable_dp(typec, port_num, pd_ctrl); + } else if (dp_enabled) { + ret = 0; + /* Ignore DP events for the non-active port */ + if (is_active_port) + ret = cros_typec_enable_dp(typec, port_num, pd_ctrl); } else if (port->mux_flags & USB_PD_MUX_SAFE_MODE) { ret = cros_typec_usb_safe_state(port); } else if (port->mux_flags & USB_PD_MUX_USB_ENABLED) { @@ -716,6 +736,9 @@ static int cros_typec_configure_mux(struct cros_typec_data *typec, int port_num, } mux_ack: + if (dp_bridge && !dp_enabled && is_active_port) + dp_bridge->active_port = NULL; + if (!typec->needs_mux_ack) return ret;
Most ARM based chromebooks with two usb-c-connector nodes and one DP controller are muxing the DP lanes between the two USB ports. This is done so that the type-c ports are at least equal in capability if not functionality. Either an analog mux is used to steer the DP signal to one or the other port, or a DP bridge chip has two lanes (e.g. DP ML0/ML1) wired to one type-c port while the other two (e.g. DP ML2/ML3) are wired to another type-c port. Implement the same algorithm that the EC has to figure out which type-c port has actually been muxed for DP altmode. Wait for the first type-c port to assert HPD, and treat that as the actively muxed port until the port exits DP altmode entirely. Allow HPD to be asserted or deasserted during this time. If the port isn't active, simply ignore those events and skip calling cros_typec_enable_dp(). Otherwise, pass the DP information to the typec subsystem so that the DP controller can respond to HPD events and pin configurations. The EC can mux the DP signal to any number of USB type-c ports. We only need to make sure that the active USB type-c port is tracked so that DP information about the other ports is ignored. Unfortunately, the EC doesn't hide these details from the AP so we have to reimplement the logic in the kernel. Cc: Prashant Malani <pmalani@chromium.org> Cc: Benson Leung <bleung@chromium.org> Cc: Tzung-Bi Shih <tzungbi@kernel.org> Cc: <chrome-platform@lists.linux.dev> Cc: Pin-yen Lin <treapking@chromium.org> Signed-off-by: Stephen Boyd <swboyd@chromium.org> --- drivers/platform/chrome/cros_ec_typec.c | 27 +++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-)