[PATCH v2 09/23] drm/i915: Factor out common parts from TypeC port handling functions

Imre Deak
2019-06-20T14:06:21Z

Factor out helpers reading/parsing the TypeC specific registers, making
current users of them clearer and letting us use them later.

While at it also:
- Simplify icl_tc_phy_connect() with an early return in legacy mode.
- Simplify the live status check using one bitmask for all HPD bits.
- Remove a micro-optimisation of the repeated safe-mode clearing.
- Make sure we fix the legacy port flag in all cases.

Except for the last two, no functional changes.

v2:
- Don't do reg reads at variable declarations. (Jani)
- Prevent constant truncated compiler warning when assigning the
valid_hpd_mask. (Nick)
- s/intel_tc_port_get_lane_info/intel_tc_port_get_lane_mask/ (Ville)

Cc: José Roberto de Souza <jose.souza@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
drivers/gpu/drm/i915/display/intel_ddi.c | 11 +-
drivers/gpu/drm/i915/display/intel_tc.c | 185 ++++++++++++++---------
drivers/gpu/drm/i915/display/intel_tc.h | 1 +
3 files changed, 116 insertions(+), 81 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index c9143e2a6994..2be7cdc319ba 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -2996,8 +2996,7 @@ static void icl_program_mg_dp_mode(struct intel_digital_port *intel_dig_port)
{
struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
enum port port = intel_dig_port->base.port;
- enum tc_port tc_port = intel_port_to_tc(dev_priv, port);
- u32 ln0, ln1, lane_info;
+ u32 ln0, ln1, lane_mask;

if (intel_dig_port->tc_mode == TC_PORT_TBT_ALT)
return;
@@ -3010,11 +3009,9 @@ static void icl_program_mg_dp_mode(struct intel_digital_port *intel_dig_port)
ln0 &= ~(MG_DP_MODE_CFG_DP_X1_MODE | MG_DP_MODE_CFG_DP_X2_MODE);
ln1 &= ~(MG_DP_MODE_CFG_DP_X1_MODE | MG_DP_MODE_CFG_DP_X2_MODE);

- lane_info = (I915_READ(PORT_TX_DFLEXDPSP) &
- DP_LANE_ASSIGNMENT_MASK(tc_port)) >>
- DP_LANE_ASSIGNMENT_SHIFT(tc_port);
+ lane_mask = intel_tc_port_get_lane_mask(intel_dig_port);

- switch (lane_info) {
+ switch (lane_mask) {
case 0x1:
case 0x4:
break;
@@ -3039,7 +3036,7 @@ static void icl_program_mg_dp_mode(struct intel_digital_port *intel_dig_port)
MG_DP_MODE_CFG_DP_X2_MODE;
break;
default:
- MISSING_CASE(lane_info);
+ MISSING_CASE(lane_mask);
}
break;

diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c
index ca3b11e26474..fbf0a8c9361f 100644
--- a/drivers/gpu/drm/i915/display/intel_tc.c
+++ b/drivers/gpu/drm/i915/display/intel_tc.c
@@ -21,25 +21,34 @@ static const char *tc_port_mode_name(enum tc_port_mode mode)
return names[mode];
}

-int intel_tc_port_fia_max_lane_count(struct intel_digital_port *dig_port)
+u32 intel_tc_port_get_lane_mask(struct intel_digital_port *dig_port)
{
struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port->base.port);
+ u32 lane_mask;
+
+ lane_mask = I915_READ(PORT_TX_DFLEXDPSP);
+
+ return (lane_mask & DP_LANE_ASSIGNMENT_MASK(tc_port)) >>
+ DP_LANE_ASSIGNMENT_SHIFT(tc_port);
+}
+
+int intel_tc_port_fia_max_lane_count(struct intel_digital_port *dig_port)
+{
+ struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
intel_wakeref_t wakeref;
- u32 lane_info;
+ u32 lane_mask;

if (dig_port->tc_mode != TC_PORT_DP_ALT)
return 4;

- lane_info = 0;
+ lane_mask = 0;
with_intel_display_power(dev_priv, POWER_DOMAIN_DISPLAY_CORE, wakeref)
- lane_info = (I915_READ(PORT_TX_DFLEXDPSP) &
- DP_LANE_ASSIGNMENT_MASK(tc_port)) >>
- DP_LANE_ASSIGNMENT_SHIFT(tc_port);
+ lane_mask = intel_tc_port_get_lane_mask(dig_port);

- switch (lane_info) {
+ switch (lane_mask) {
default:
- MISSING_CASE(lane_info);
+ MISSING_CASE(lane_mask);
case 1:
case 2:
case 4:
@@ -53,6 +62,73 @@ int intel_tc_port_fia_max_lane_count(struct intel_digital_port *dig_port)
}
}

+static void tc_port_fixup_legacy_flag(struct intel_digital_port *dig_port,
+ u32 live_status_mask)
+{
+ u32 valid_hpd_mask = BIT(TC_PORT_LEGACY);
+
+ if (!dig_port->tc_legacy_port)
+ valid_hpd_mask = ~valid_hpd_mask;
+
+ if (!(live_status_mask & ~valid_hpd_mask))
+ return;
+
+ /* If live status mismatches the VBT flag, trust the live status. */
+ DRM_ERROR("Port %s: live status %08x mismatches the legacy port flag, fix flag\n",
+ dig_port->tc_port_name, live_status_mask);
+
+ dig_port->tc_legacy_port = !dig_port->tc_legacy_port;
+}
+
+static u32 tc_port_live_status_mask(struct intel_digital_port *dig_port)
+{
+ struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
+ enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port->base.port);
+ u32 mask = 0;
+ u32 val;
+
+ val = I915_READ(PORT_TX_DFLEXDPSP);
+
+ if (val & TC_LIVE_STATE_TBT(tc_port))
+ mask |= BIT(TC_PORT_TBT_ALT);
+ if (val & TC_LIVE_STATE_TC(tc_port))
+ mask |= BIT(TC_PORT_DP_ALT);
+
+ if (I915_READ(SDEISR) & SDE_TC_HOTPLUG_ICP(tc_port))
+ mask |= BIT(TC_PORT_LEGACY);
+
+ /* The sink can be connected only in a single mode. */
+ if (!WARN_ON(hweight32(mask) > 1))
+ tc_port_fixup_legacy_flag(dig_port, mask);
+
+ return mask;
+}
+
+static bool icl_tc_phy_status_complete(struct intel_digital_port *dig_port)
+{
+ struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
+ enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port->base.port);
+
+ return I915_READ(PORT_TX_DFLEXDPPMS) &
+ DP_PHY_MODE_STATUS_COMPLETED(tc_port);
+}
+
+static void icl_tc_phy_set_safe_mode(struct intel_digital_port *dig_port,
+ bool enable)
+{
+ struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
+ enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port->base.port);
+ u32 val;
+
+ val = I915_READ(PORT_TX_DFLEXDPCSSS);
+
+ val &= ~DP_PHY_MODE_STATUS_NOT_SAFE(tc_port);
+ if (!enable)
+ val |= DP_PHY_MODE_STATUS_NOT_SAFE(tc_port);
+
+ I915_WRITE(PORT_TX_DFLEXDPCSSS, val);
+}
+
/*
* This function implements the first part of the Connect Flow described by our
* specification, Gen11 TypeC Programming chapter. The rest of the flow (reading
@@ -76,38 +152,31 @@ int intel_tc_port_fia_max_lane_count(struct intel_digital_port *dig_port)
*/
static bool icl_tc_phy_connect(struct intel_digital_port *dig_port)
{
- struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
- enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port->base.port);
- u32 val;
+ u32 live_status_mask;

if (dig_port->tc_mode != TC_PORT_LEGACY &&
dig_port->tc_mode != TC_PORT_DP_ALT)
return true;

- val = I915_READ(PORT_TX_DFLEXDPPMS);
- if (!(val & DP_PHY_MODE_STATUS_COMPLETED(tc_port))) {
+ if (!icl_tc_phy_status_complete(dig_port)) {
DRM_DEBUG_KMS("Port %s: PHY not ready\n",
dig_port->tc_port_name);
WARN_ON(dig_port->tc_legacy_port);
return false;
}

- /*
- * This function may be called many times in a row without an HPD event
- * in between, so try to avoid the write when we can.
- */
- val = I915_READ(PORT_TX_DFLEXDPCSSS);
- if (!(val & DP_PHY_MODE_STATUS_NOT_SAFE(tc_port))) {
- val |= DP_PHY_MODE_STATUS_NOT_SAFE(tc_port);
- I915_WRITE(PORT_TX_DFLEXDPCSSS, val);
- }
+ icl_tc_phy_set_safe_mode(dig_port, false);
+
+ if (dig_port->tc_mode == TC_PORT_LEGACY)
+ return true;
+
+ live_status_mask = tc_port_live_status_mask(dig_port);

/*
* Now we have to re-check the live state, in case the port recently
* became disconnected. Not necessary for legacy mode.
*/
- if (dig_port->tc_mode == TC_PORT_DP_ALT &&
- !(I915_READ(PORT_TX_DFLEXDPSP) & TC_LIVE_STATE_TC(tc_port))) {
+ if (!(live_status_mask & BIT(TC_PORT_DP_ALT))) {
DRM_DEBUG_KMS("Port %s: PHY sudden disconnect\n",
dig_port->tc_port_name);
icl_tc_phy_disconnect(dig_port);
@@ -123,46 +192,35 @@ static bool icl_tc_phy_connect(struct intel_digital_port *dig_port)
*/
void icl_tc_phy_disconnect(struct intel_digital_port *dig_port)
{
- struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
- enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port->base.port);
-
- /*
- * TBT disconnection flow is read the live status, what was done in
- * caller.
- */
- if (dig_port->tc_mode == TC_PORT_DP_ALT ||
- dig_port->tc_mode == TC_PORT_LEGACY) {
- u32 val;
-
- val = I915_READ(PORT_TX_DFLEXDPCSSS);
- val &= ~DP_PHY_MODE_STATUS_NOT_SAFE(tc_port);
- I915_WRITE(PORT_TX_DFLEXDPCSSS, val);
+ switch (dig_port->tc_mode) {
+ case TC_PORT_LEGACY:
+ case TC_PORT_DP_ALT:
+ icl_tc_phy_set_safe_mode(dig_port, true);
+ dig_port->tc_mode = TC_PORT_TBT_ALT;
+ break;
+ case TC_PORT_TBT_ALT:
+ /* Nothing to do, we stay in TBT-alt mode */
+ break;
+ default:
+ MISSING_CASE(dig_port->tc_mode);
}

DRM_DEBUG_KMS("Port %s: mode %s disconnected\n",
dig_port->tc_port_name,
tc_port_mode_name(dig_port->tc_mode));
-
- dig_port->tc_mode = TC_PORT_TBT_ALT;
}

static void icl_update_tc_port_type(struct drm_i915_private *dev_priv,
struct intel_digital_port *intel_dig_port,
- bool is_legacy, bool is_typec, bool is_tbt)
+ u32 live_status_mask)
{
enum tc_port_mode old_mode = intel_dig_port->tc_mode;

- WARN_ON(is_legacy + is_typec + is_tbt != 1);
-
- if (is_legacy)
- intel_dig_port->tc_mode = TC_PORT_LEGACY;
- else if (is_typec)
- intel_dig_port->tc_mode = TC_PORT_DP_ALT;
- else if (is_tbt)
- intel_dig_port->tc_mode = TC_PORT_TBT_ALT;
- else
+ if (!live_status_mask)
return;

+ intel_dig_port->tc_mode = fls(live_status_mask) - 1;
+
if (old_mode != intel_dig_port->tc_mode)
DRM_DEBUG_KMS("Port %s: port has mode %s\n",
intel_dig_port->tc_port_name,
@@ -182,40 +240,19 @@ static void icl_update_tc_port_type(struct drm_i915_private *dev_priv,
bool intel_tc_port_connected(struct intel_digital_port *dig_port)
{
struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
- enum port port = dig_port->base.port;
- enum tc_port tc_port = intel_port_to_tc(dev_priv, port);
- bool is_legacy, is_typec, is_tbt;
- u32 dpsp;
-
- /*
- * Complain if we got a legacy port HPD, but VBT didn't mark the port as
- * legacy. Treat the port as legacy from now on.
- */
- if (!dig_port->tc_legacy_port &&
- I915_READ(SDEISR) & SDE_TC_HOTPLUG_ICP(tc_port)) {
- DRM_ERROR("Port %s: VBT incorrectly claims port is not TypeC legacy\n",
- dig_port->tc_port_name);
- dig_port->tc_legacy_port = true;
- }
- is_legacy = dig_port->tc_legacy_port;
+ u32 live_status_mask = tc_port_live_status_mask(dig_port);

/*
* The spec says we shouldn't be using the ISR bits for detecting
* between TC and TBT. We should use DFLEXDPSP.
*/
- dpsp = I915_READ(PORT_TX_DFLEXDPSP);
- is_typec = dpsp & TC_LIVE_STATE_TC(tc_port);
- is_tbt = dpsp & TC_LIVE_STATE_TBT(tc_port);
-
- if (!is_legacy && !is_typec && !is_tbt) {
+ if (!live_status_mask && !dig_port->tc_legacy_port) {
icl_tc_phy_disconnect(dig_port);

return false;
}

- icl_update_tc_port_type(dev_priv, dig_port, is_legacy, is_typec,
- is_tbt);
-
+ icl_update_tc_port_type(dev_priv, dig_port, live_status_mask);
if (!icl_tc_phy_connect(dig_port))
return false;

diff --git a/drivers/gpu/drm/i915/display/intel_tc.h b/drivers/gpu/drm/i915/display/intel_tc.h
index ca1735303252..8c338c45796d 100644
--- a/drivers/gpu/drm/i915/display/intel_tc.h
+++ b/drivers/gpu/drm/i915/display/intel_tc.h
@@ -13,6 +13,7 @@ struct intel_digital_port;
void icl_tc_phy_disconnect(struct intel_digital_port *dig_port);

bool intel_tc_port_connected(struct intel_digital_port *dig_port);
+u32 intel_tc_port_get_lane_mask(struct intel_digital_port *dig_port);
int intel_tc_port_fia_max_lane_count(struct intel_digital_port *dig_port);

void intel_tc_port_init(struct intel_digital_port *dig_port, bool is_legacy);