pd_alt_mode_dfp: Default to 4 lanes of DP

Currently our default PD policy states that if dock that supports DP
alt mode also advertises Multi-function preferred, we configure the
superspeed muxes to use 2 lanes for DisplayPort and 2 lanes for
SuperSpeed USB. However, this is done without regard as to what the
actual usage of the SuperSpeed ports may be.

We've historically made this trade off, but it results in degraded
display performance.  Users may be better served if we prioritize
display performance over SuperSpeed USB.

The ideal solution involves adding additional plumbing such that we
can configure the chosen pin configuration from the AP based upon
other heuristics.

BUG=b:178635286
BRANCH=as many as feasible?
TEST=Build and flash DUT, plug in a dock with SuperSpeed USB ports,
verify that 4 lanes of DP are configured.

Signed-off-by: Aseda Aboagye <aaboagye@google.com>
Change-Id: Ie2d47588012aceb7f13312b6947b885f8f7034a8
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2653315
Tested-by: Aseda Aboagye <aaboagye@chromium.org>
Auto-Submit: Aseda Aboagye <aaboagye@chromium.org>
Reviewed-by: Rob Barnes <robbarnes@google.com>
Reviewed-by: Diana Z <dzigterman@chromium.org>
Commit-Queue: Diana Z <dzigterman@chromium.org>

1 files changed, 22 insertions, 26 deletions

diff --git a/common/usb_pd_alt_mode_dfp.c b/common/usb_pd_alt_mode_dfp.c
index fe9e46f35b..e3964d4ca7 100644
--- a/common/usb_pd_alt_mode_dfp.c
+++ b/common/usb_pd_alt_mode_dfp.c
@@ -173,6 +173,10 @@ void pd_prepare_sysjump(void)
  * possible pin config depending on whether its a converter DP->(VGA|HDMI) or DP
  * output.  If UFP is a USB-C receptacle it may assert C/D/E/F.  The DFP USB-C
  * receptacle must always choose C/D in those cases.
+ *
+ * TODO(b/178635286): We should add the necessary plumbing to let the AP change
+ * our selected pin mode after selecting our default here.  But for now, let's
+ * default to not preferring multi-function pin modes.
  */
 int pd_dfp_dp_get_pin_mode(int port, uint32_t status)
 {
@@ -189,9 +193,8 @@ int pd_dfp_dp_get_pin_mode(int port, uint32_t status)
 	/* TODO(crosbug.com/p/39656) revisit with DFP that can be a sink */
 	pin_caps = PD_DP_PIN_CAPS(mode_caps);
 
-	/* if don't want multi-function then ignore those pin configs */
-	if (!PD_VDO_DPSTS_MF_PREF(status))
-		pin_caps &= ~MODE_DP_PIN_MF_MASK;
+	/* We do not prefer multi-function pin modes. */
+	pin_caps &= ~MODE_DP_PIN_MF_MASK;
 
 	/* TODO(crosbug.com/p/39656) revisit if DFP drives USB Gen 2 signals */
 	pin_caps &= ~MODE_DP_PIN_BR2_MASK;
@@ -1083,31 +1086,20 @@ __overridable uint8_t get_dp_pin_mode(int port)
 	return pd_dfp_dp_get_pin_mode(port, dp_status[port]);
 }
 
-static mux_state_t svdm_dp_get_mux_mode(int port)
-{
-	int mf_pref = PD_VDO_DPSTS_MF_PREF(dp_status[port]);
-	int pin_mode = get_dp_pin_mode(port);
-	/*
-	 * Multi-function operation is only allowed if that pin config is
-	 * supported.
-	 */
-	if ((pin_mode & MODE_DP_PIN_MF_MASK) && mf_pref)
-		return USB_PD_MUX_DOCK;
-	else
-		return USB_PD_MUX_DP_ENABLED;
-}
-
 __overridable int svdm_dp_config(int port, uint32_t *payload)
 {
 	int opos = pd_alt_mode(port, TCPC_TX_SOP, USB_SID_DISPLAYPORT);
-	int mf_pref = PD_VDO_DPSTS_MF_PREF(dp_status[port]);
 	uint8_t pin_mode = get_dp_pin_mode(port);
-	mux_state_t mux_mode = svdm_dp_get_mux_mode(port);
 
 	if (!pin_mode)
 		return 0;
 
-	CPRINTS("pin_mode: %x, mf: %d, mux: %d", pin_mode, mf_pref, mux_mode);
+	/*
+	 * TODO(b/178635286): We should add the necessary plumbing to let the AP
+	 * change this after our default.  But for now, let's default to 4-lanes
+	 * of DP.
+	 */
+	CPRINTS("DP PinCfg: 0x%x", pin_mode);
 
 	/*
 	 * Place the USB Type-C pins that are to be re-configured to DisplayPort
@@ -1115,10 +1107,9 @@ __overridable int svdm_dp_config(int port, uint32_t *payload)
 	 * superspeed signals can remain connected. For USB_PD_MUX_DP_ENABLED,
 	 * disconnect the superspeed signals here, before the pins are
 	 * re-configured to DisplayPort (in svdm_dp_post_config, when we receive
-	 * the config ack).
+	 * the config ack).  We are always selecting USB_PD_MUX_DP_ENABLED.
 	 */
-	if (mux_mode == USB_PD_MUX_DP_ENABLED)
-		usb_mux_set_safe_mode(port);
+	usb_mux_set_safe_mode(port);
 
 	payload[0] = VDO(USB_SID_DISPLAYPORT, 1,
 			 CMD_DP_CONFIG | VDO_OPOS(opos));
@@ -1143,12 +1134,17 @@ int svdm_get_hpd_gpio(int port)
 
 __overridable void svdm_dp_post_config(int port)
 {
-	mux_state_t mux_mode = svdm_dp_get_mux_mode(port);
+	/*
+	 * TODO(b/178635286): We should add the necessary plumbing to let the AP
+	 * change this after our default.  But for now, let's default to 4-lanes
+	 * of DP.
+	 */
+
 	/* Connect the SBU and USB lines to the connector. */
 	if (IS_ENABLED(CONFIG_USBC_PPC_SBU))
 		ppc_set_sbu(port, 1);
-	usb_mux_set(port, mux_mode, USB_SWITCH_CONNECT,
-		polarity_rm_dts(pd_get_polarity(port)));
+	usb_mux_set(port, USB_PD_MUX_DP_ENABLED, USB_SWITCH_CONNECT,
+		    polarity_rm_dts(pd_get_polarity(port)));
 
 	dp_flags[port] |= DP_FLAGS_DP_ON;
 	if (!(dp_flags[port] & DP_FLAGS_HPD_HI_PENDING))