| Message ID | 20190604145826.16424-4-imre.deak@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Fix TypeC port mode switching | expand |
On Tue, 04 Jun 2019, Imre Deak <imre.deak@intel.com> wrote: > Move the TypeC port handling functions to a new file for clarity. > > While at it: > - s/icl_tc_port_connected()/intel_tc_port_connected()/ > icl_tc_phy_disconnect(), will be unexported later. > > - s/intel_dp_get_fia_supported_lane_count()/intel_tc_port_fia_max_lane_count()/ > It's used for HDMI legacy mode too. > > - Simplify function interfaces by passing only dig_port to them. > > No functional changes. Some nitpicks below. BR, Jani. > > Cc: Animesh Manna <animesh.manna@intel.com> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > Cc: José Roberto de Souza <jose.souza@intel.com> > Signed-off-by: Imre Deak <imre.deak@intel.com> > --- > drivers/gpu/drm/i915/Makefile | 3 +- > drivers/gpu/drm/i915/Makefile.header-test | 1 + > drivers/gpu/drm/i915/intel_ddi.c | 6 +- > drivers/gpu/drm/i915/intel_dp.c | 227 +-------------------- > drivers/gpu/drm/i915/intel_dp.h | 2 - > drivers/gpu/drm/i915/intel_tc.c | 230 ++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_tc.h | 13 ++ > 7 files changed, 252 insertions(+), 230 deletions(-) > create mode 100644 drivers/gpu/drm/i915/intel_tc.c > create mode 100644 drivers/gpu/drm/i915/intel_tc.h > > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile > index c0a7b2994077..74c4d11d83eb 100644 > --- a/drivers/gpu/drm/i915/Makefile > +++ b/drivers/gpu/drm/i915/Makefile > @@ -171,7 +171,8 @@ i915-y += intel_audio.o \ > intel_psr.o \ > intel_quirks.o \ > intel_sideband.o \ > - intel_sprite.o > + intel_sprite.o \ > + intel_tc.o > i915-$(CONFIG_ACPI) += intel_acpi.o intel_opregion.o > i915-$(CONFIG_DRM_FBDEV_EMULATION) += intel_fbdev.o > > diff --git a/drivers/gpu/drm/i915/Makefile.header-test b/drivers/gpu/drm/i915/Makefile.header-test > index 6ef3b647ac65..e80e8e45b09c 100644 > --- a/drivers/gpu/drm/i915/Makefile.header-test > +++ b/drivers/gpu/drm/i915/Makefile.header-test > @@ -58,6 +58,7 @@ header_test := \ > intel_sdvo.h \ > intel_sideband.h \ > intel_sprite.h \ > + intel_tc.h \ > intel_tv.h \ > intel_uncore.h \ > intel_vdsc.h \ > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > index 350eaf54f01f..5a1c98438375 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -45,6 +45,7 @@ > #include "intel_lspcon.h" > #include "intel_panel.h" > #include "intel_psr.h" > +#include "intel_tc.h" > #include "intel_vdsc.h" > > struct ddi_buf_trans { > @@ -3904,7 +3905,6 @@ static int intel_ddi_compute_config(struct intel_encoder *encoder, > static void intel_ddi_encoder_suspend(struct intel_encoder *encoder) > { > struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base); > - struct drm_i915_private *i915 = to_i915(encoder->base.dev); > > intel_dp_encoder_suspend(encoder); > > @@ -3914,7 +3914,7 @@ static void intel_ddi_encoder_suspend(struct intel_encoder *encoder) > * even if the sink has disappeared while being suspended. > */ > if (dig_port->tc_legacy_port) > - icl_tc_phy_disconnect(i915, dig_port); > + icl_tc_phy_disconnect(dig_port); > } > > static void intel_ddi_encoder_reset(struct drm_encoder *drm_encoder) > @@ -3936,7 +3936,7 @@ static void intel_ddi_encoder_destroy(struct drm_encoder *encoder) > intel_dp_encoder_flush_work(encoder); > > if (intel_port_is_tc(i915, dig_port->base.port)) > - icl_tc_phy_disconnect(i915, dig_port); > + icl_tc_phy_disconnect(dig_port); > > drm_encoder_cleanup(encoder); > kfree(dig_port); > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 24b56b2a76c8..b69310bd9914 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -62,6 +62,7 @@ > #include "intel_panel.h" > #include "intel_psr.h" > #include "intel_sideband.h" > +#include "intel_tc.h" > #include "intel_vdsc.h" > > #define DP_DPRX_ESI_LEN 14 > @@ -211,46 +212,13 @@ static int intel_dp_max_common_rate(struct intel_dp *intel_dp) > return intel_dp->common_rates[intel_dp->num_common_rates - 1]; > } > > -static int intel_dp_get_fia_supported_lane_count(struct intel_dp *intel_dp) > -{ > - struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp); > - 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); > - intel_wakeref_t wakeref; > - u32 lane_info; > - > - if (tc_port == PORT_TC_NONE || dig_port->tc_type != TC_PORT_TYPEC) > - return 4; > - > - lane_info = 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); > - > - switch (lane_info) { > - default: > - MISSING_CASE(lane_info); > - case 1: > - case 2: > - case 4: > - case 8: > - return 1; > - case 3: > - case 12: > - return 2; > - case 15: > - return 4; > - } > -} > - > /* Theoretical max between source and sink */ > static int intel_dp_max_common_lane_count(struct intel_dp *intel_dp) > { > struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); > int source_max = intel_dig_port->max_lanes; > int sink_max = drm_dp_max_lane_count(intel_dp->dpcd); > - int fia_max = intel_dp_get_fia_supported_lane_count(intel_dp); > + int fia_max = intel_tc_port_fia_max_lane_count(intel_dig_port); > > return min3(source_max, sink_max, fia_max); > } > @@ -5231,195 +5199,6 @@ static bool icl_combo_port_connected(struct drm_i915_private *dev_priv, > return I915_READ(SDEISR) & SDE_DDI_HOTPLUG_ICP(port); > } > > -static const char *tc_type_name(enum tc_port_type type) > -{ > - static const char * const names[] = { > - [TC_PORT_UNKNOWN] = "unknown", > - [TC_PORT_LEGACY] = "legacy", > - [TC_PORT_TYPEC] = "typec", > - [TC_PORT_TBT] = "tbt", > - }; > - > - if (WARN_ON(type >= ARRAY_SIZE(names))) > - type = TC_PORT_UNKNOWN; > - > - return names[type]; > -} > - > -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) > -{ > - enum port port = intel_dig_port->base.port; > - enum tc_port_type old_type = intel_dig_port->tc_type; > - > - WARN_ON(is_legacy + is_typec + is_tbt != 1); > - > - if (is_legacy) > - intel_dig_port->tc_type = TC_PORT_LEGACY; > - else if (is_typec) > - intel_dig_port->tc_type = TC_PORT_TYPEC; > - else if (is_tbt) > - intel_dig_port->tc_type = TC_PORT_TBT; > - else > - return; > - > - /* Types are not supposed to be changed at runtime. */ > - WARN_ON(old_type != TC_PORT_UNKNOWN && > - old_type != intel_dig_port->tc_type); > - > - if (old_type != intel_dig_port->tc_type) > - DRM_DEBUG_KMS("Port %c has TC type %s\n", port_name(port), > - tc_type_name(intel_dig_port->tc_type)); > -} > - > -/* > - * This function implements the first part of the Connect Flow described by our > - * specification, Gen11 TypeC Programming chapter. The rest of the flow (reading > - * lanes, EDID, etc) is done as needed in the typical places. > - * > - * Unlike the other ports, type-C ports are not available to use as soon as we > - * get a hotplug. The type-C PHYs can be shared between multiple controllers: > - * display, USB, etc. As a result, handshaking through FIA is required around > - * connect and disconnect to cleanly transfer ownership with the controller and > - * set the type-C power state. > - * > - * We could opt to only do the connect flow when we actually try to use the AUX > - * channels or do a modeset, then immediately run the disconnect flow after > - * usage, but there are some implications on this for a dynamic environment: > - * things may go away or change behind our backs. So for now our driver is > - * always trying to acquire ownership of the controller as soon as it gets an > - * interrupt (or polls state and sees a port is connected) and only gives it > - * back when it sees a disconnect. Implementation of a more fine-grained model > - * will require a lot of coordination with user space and thorough testing for > - * the extra possible cases. > - */ > -static bool icl_tc_phy_connect(struct drm_i915_private *dev_priv, > - struct intel_digital_port *dig_port) > -{ > - enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port->base.port); > - u32 val; > - > - if (dig_port->tc_type != TC_PORT_LEGACY && > - dig_port->tc_type != TC_PORT_TYPEC) > - return true; > - > - val = I915_READ(PORT_TX_DFLEXDPPMS); > - if (!(val & DP_PHY_MODE_STATUS_COMPLETED(tc_port))) { > - DRM_DEBUG_KMS("DP PHY for TC port %d not ready\n", tc_port); > - 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); > - } > - > - /* > - * 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_type == TC_PORT_TYPEC && > - !(I915_READ(PORT_TX_DFLEXDPSP) & TC_LIVE_STATE_TC(tc_port))) { > - DRM_DEBUG_KMS("TC PHY %d sudden disconnect.\n", tc_port); > - icl_tc_phy_disconnect(dev_priv, dig_port); > - return false; > - } > - > - return true; > -} > - > -/* > - * See the comment at the connect function. This implements the Disconnect > - * Flow. > - */ > -void icl_tc_phy_disconnect(struct drm_i915_private *dev_priv, > - struct intel_digital_port *dig_port) > -{ > - enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port->base.port); > - > - if (dig_port->tc_type == TC_PORT_UNKNOWN) > - return; > - > - /* > - * TBT disconnection flow is read the live status, what was done in > - * caller. > - */ > - if (dig_port->tc_type == TC_PORT_TYPEC || > - dig_port->tc_type == 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); > - } > - > - DRM_DEBUG_KMS("Port %c TC type %s disconnected\n", > - port_name(dig_port->base.port), > - tc_type_name(dig_port->tc_type)); > - > - dig_port->tc_type = TC_PORT_UNKNOWN; > -} > - > -/* > - * The type-C ports are different because even when they are connected, they may > - * not be available/usable by the graphics driver: see the comment on > - * icl_tc_phy_connect(). So in our driver instead of adding the additional > - * concept of "usable" and make everything check for "connected and usable" we > - * define a port as "connected" when it is not only connected, but also when it > - * is usable by the rest of the driver. That maintains the old assumption that > - * connected ports are usable, and avoids exposing to the users objects they > - * can't really use. > - */ > -static bool icl_tc_port_connected(struct drm_i915_private *dev_priv, > - struct intel_digital_port *intel_dig_port) > -{ > - enum port port = intel_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 (!intel_dig_port->tc_legacy_port && > - I915_READ(SDEISR) & SDE_TC_HOTPLUG_ICP(tc_port)) { > - DRM_ERROR("VBT incorrectly claims port %c is not TypeC legacy\n", > - port_name(port)); > - intel_dig_port->tc_legacy_port = true; > - } > - is_legacy = intel_dig_port->tc_legacy_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) { > - icl_tc_phy_disconnect(dev_priv, intel_dig_port); > - > - return false; > - } > - > - icl_update_tc_port_type(dev_priv, intel_dig_port, is_legacy, is_typec, > - is_tbt); > - > - if (!icl_tc_phy_connect(dev_priv, intel_dig_port)) > - return false; > - > - return true; > -} > - > static bool icl_digital_port_connected(struct intel_encoder *encoder) > { > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > @@ -5428,7 +5207,7 @@ static bool icl_digital_port_connected(struct intel_encoder *encoder) > if (intel_port_is_combophy(dev_priv, encoder->port)) > return icl_combo_port_connected(dev_priv, dig_port); > else if (intel_port_is_tc(dev_priv, encoder->port)) > - return icl_tc_port_connected(dev_priv, dig_port); > + return intel_tc_port_connected(dig_port); > else > MISSING_CASE(encoder->hpd_pin); > > diff --git a/drivers/gpu/drm/i915/intel_dp.h b/drivers/gpu/drm/i915/intel_dp.h > index da70b1a41c83..657bbb1f5ed0 100644 > --- a/drivers/gpu/drm/i915/intel_dp.h > +++ b/drivers/gpu/drm/i915/intel_dp.h > @@ -112,8 +112,6 @@ bool intel_dp_get_colorimetry_status(struct intel_dp *intel_dp); > int intel_dp_link_required(int pixel_clock, int bpp); > int intel_dp_max_data_rate(int max_link_clock, int max_lanes); > bool intel_digital_port_connected(struct intel_encoder *encoder); > -void icl_tc_phy_disconnect(struct drm_i915_private *dev_priv, > - struct intel_digital_port *dig_port); > > static inline unsigned int intel_dp_unused_lane_mask(int lane_count) > { > diff --git a/drivers/gpu/drm/i915/intel_tc.c b/drivers/gpu/drm/i915/intel_tc.c > new file mode 100644 > index 000000000000..7a1b5870945f > --- /dev/null > +++ b/drivers/gpu/drm/i915/intel_tc.c > @@ -0,0 +1,230 @@ > +// SPDX-License-Identifier: MIT > +/* > + * Copyright © 2019 Intel Corporation > + */ From the Nitpicks Department, please add a blank line here. > +#include "intel_display.h" > +#include "i915_drv.h" > +#include "intel_tc.h" And sort the includes. > + > +static const char *tc_type_name(enum tc_port_type type) > +{ > + static const char * const names[] = { > + [TC_PORT_UNKNOWN] = "unknown", > + [TC_PORT_LEGACY] = "legacy", > + [TC_PORT_TYPEC] = "typec", > + [TC_PORT_TBT] = "tbt", > + }; > + > + if (WARN_ON(type >= ARRAY_SIZE(names))) > + type = TC_PORT_UNKNOWN; > + > + return names[type]; > +} > + > +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); > + enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port->base.port); > + intel_wakeref_t wakeref; > + u32 lane_info; > + > + if (tc_port == PORT_TC_NONE || dig_port->tc_type != TC_PORT_TYPEC) > + return 4; > + > + lane_info = 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); > + > + switch (lane_info) { > + default: > + MISSING_CASE(lane_info); > + case 1: > + case 2: > + case 4: > + case 8: > + return 1; > + case 3: > + case 12: > + return 2; > + case 15: > + return 4; > + } > +} > + > +/* > + * This function implements the first part of the Connect Flow described by our > + * specification, Gen11 TypeC Programming chapter. The rest of the flow (reading > + * lanes, EDID, etc) is done as needed in the typical places. > + * > + * Unlike the other ports, type-C ports are not available to use as soon as we > + * get a hotplug. The type-C PHYs can be shared between multiple controllers: > + * display, USB, etc. As a result, handshaking through FIA is required around > + * connect and disconnect to cleanly transfer ownership with the controller and > + * set the type-C power state. > + * > + * We could opt to only do the connect flow when we actually try to use the AUX > + * channels or do a modeset, then immediately run the disconnect flow after > + * usage, but there are some implications on this for a dynamic environment: > + * things may go away or change behind our backs. So for now our driver is > + * always trying to acquire ownership of the controller as soon as it gets an > + * interrupt (or polls state and sees a port is connected) and only gives it > + * back when it sees a disconnect. Implementation of a more fine-grained model > + * will require a lot of coordination with user space and thorough testing for > + * the extra possible cases. > + */ > +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; > + > + if (dig_port->tc_type != TC_PORT_LEGACY && > + dig_port->tc_type != TC_PORT_TYPEC) > + return true; > + > + val = I915_READ(PORT_TX_DFLEXDPPMS); > + if (!(val & DP_PHY_MODE_STATUS_COMPLETED(tc_port))) { > + DRM_DEBUG_KMS("DP PHY for TC port %d not ready\n", tc_port); > + 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); > + } > + > + /* > + * 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_type == TC_PORT_TYPEC && > + !(I915_READ(PORT_TX_DFLEXDPSP) & TC_LIVE_STATE_TC(tc_port))) { > + DRM_DEBUG_KMS("TC PHY %d sudden disconnect.\n", tc_port); > + icl_tc_phy_disconnect(dig_port); > + return false; > + } > + > + return true; > +} > + > +/* > + * See the comment at the connect function. This implements the Disconnect > + * Flow. > + */ > +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); > + > + if (dig_port->tc_type == TC_PORT_UNKNOWN) > + return; > + > + /* > + * TBT disconnection flow is read the live status, what was done in > + * caller. > + */ > + if (dig_port->tc_type == TC_PORT_TYPEC || > + dig_port->tc_type == 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); > + } > + > + DRM_DEBUG_KMS("Port %c TC type %s disconnected\n", > + port_name(dig_port->base.port), > + tc_type_name(dig_port->tc_type)); > + > + dig_port->tc_type = TC_PORT_UNKNOWN; > +} > + > +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) > +{ > + enum port port = intel_dig_port->base.port; > + enum tc_port_type old_type = intel_dig_port->tc_type; > + > + WARN_ON(is_legacy + is_typec + is_tbt != 1); > + > + if (is_legacy) > + intel_dig_port->tc_type = TC_PORT_LEGACY; > + else if (is_typec) > + intel_dig_port->tc_type = TC_PORT_TYPEC; > + else if (is_tbt) > + intel_dig_port->tc_type = TC_PORT_TBT; > + else > + return; > + > + /* Types are not supposed to be changed at runtime. */ > + WARN_ON(old_type != TC_PORT_UNKNOWN && > + old_type != intel_dig_port->tc_type); > + > + if (old_type != intel_dig_port->tc_type) > + DRM_DEBUG_KMS("Port %c has TC type %s\n", port_name(port), > + tc_type_name(intel_dig_port->tc_type)); > +} > + > + > +/* > + * The type-C ports are different because even when they are connected, they may > + * not be available/usable by the graphics driver: see the comment on > + * icl_tc_phy_connect(). So in our driver instead of adding the additional > + * concept of "usable" and make everything check for "connected and usable" we > + * define a port as "connected" when it is not only connected, but also when it > + * is usable by the rest of the driver. That maintains the old assumption that > + * connected ports are usable, and avoids exposing to the users objects they > + * can't really use. > + */ > +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("VBT incorrectly claims port %c is not TypeC legacy\n", > + port_name(port)); > + dig_port->tc_legacy_port = true; > + } > + is_legacy = dig_port->tc_legacy_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) { > + icl_tc_phy_disconnect(dig_port); > + > + return false; > + } > + > + icl_update_tc_port_type(dev_priv, dig_port, is_legacy, is_typec, > + is_tbt); > + > + if (!icl_tc_phy_connect(dig_port)) > + return false; > + > + return true; > +} > + > diff --git a/drivers/gpu/drm/i915/intel_tc.h b/drivers/gpu/drm/i915/intel_tc.h > new file mode 100644 > index 000000000000..94c62ac4a162 > --- /dev/null > +++ b/drivers/gpu/drm/i915/intel_tc.h > @@ -0,0 +1,13 @@ SPDX header here please. > +#ifndef __INTEL_TC_H__ > +#define __INTEL_TC_H__ > + > +#include <linux/types.h> > + > +struct intel_digital_port; > + > +void icl_tc_phy_disconnect(struct intel_digital_port *dig_port); I'd like to see this named intel_tc_* in the future. BR, Jani. > + > +bool intel_tc_port_connected(struct intel_digital_port *dig_port); > +int intel_tc_port_fia_max_lane_count(struct intel_digital_port *dig_port); > + > +#endif /* __INTEL_TC_H__ */
On Thu, 06 Jun 2019, Jani Nikula <jani.nikula@linux.intel.com> wrote: > On Tue, 04 Jun 2019, Imre Deak <imre.deak@intel.com> wrote: >> Move the TypeC port handling functions to a new file for clarity. >> >> While at it: >> - s/icl_tc_port_connected()/intel_tc_port_connected()/ >> icl_tc_phy_disconnect(), will be unexported later. >> >> - s/intel_dp_get_fia_supported_lane_count()/intel_tc_port_fia_max_lane_count()/ >> It's used for HDMI legacy mode too. >> >> - Simplify function interfaces by passing only dig_port to them. >> >> No functional changes. > > Some nitpicks below. Forgot, Acked-by: Jani Nikula <jani.nikula@intel.com> on the idea. > > BR, > Jani. > > >> >> Cc: Animesh Manna <animesh.manna@intel.com> >> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> >> Cc: José Roberto de Souza <jose.souza@intel.com> >> Signed-off-by: Imre Deak <imre.deak@intel.com> >> --- >> drivers/gpu/drm/i915/Makefile | 3 +- >> drivers/gpu/drm/i915/Makefile.header-test | 1 + >> drivers/gpu/drm/i915/intel_ddi.c | 6 +- >> drivers/gpu/drm/i915/intel_dp.c | 227 +-------------------- >> drivers/gpu/drm/i915/intel_dp.h | 2 - >> drivers/gpu/drm/i915/intel_tc.c | 230 ++++++++++++++++++++++ >> drivers/gpu/drm/i915/intel_tc.h | 13 ++ >> 7 files changed, 252 insertions(+), 230 deletions(-) >> create mode 100644 drivers/gpu/drm/i915/intel_tc.c >> create mode 100644 drivers/gpu/drm/i915/intel_tc.h >> >> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile >> index c0a7b2994077..74c4d11d83eb 100644 >> --- a/drivers/gpu/drm/i915/Makefile >> +++ b/drivers/gpu/drm/i915/Makefile >> @@ -171,7 +171,8 @@ i915-y += intel_audio.o \ >> intel_psr.o \ >> intel_quirks.o \ >> intel_sideband.o \ >> - intel_sprite.o >> + intel_sprite.o \ >> + intel_tc.o >> i915-$(CONFIG_ACPI) += intel_acpi.o intel_opregion.o >> i915-$(CONFIG_DRM_FBDEV_EMULATION) += intel_fbdev.o >> >> diff --git a/drivers/gpu/drm/i915/Makefile.header-test b/drivers/gpu/drm/i915/Makefile.header-test >> index 6ef3b647ac65..e80e8e45b09c 100644 >> --- a/drivers/gpu/drm/i915/Makefile.header-test >> +++ b/drivers/gpu/drm/i915/Makefile.header-test >> @@ -58,6 +58,7 @@ header_test := \ >> intel_sdvo.h \ >> intel_sideband.h \ >> intel_sprite.h \ >> + intel_tc.h \ >> intel_tv.h \ >> intel_uncore.h \ >> intel_vdsc.h \ >> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c >> index 350eaf54f01f..5a1c98438375 100644 >> --- a/drivers/gpu/drm/i915/intel_ddi.c >> +++ b/drivers/gpu/drm/i915/intel_ddi.c >> @@ -45,6 +45,7 @@ >> #include "intel_lspcon.h" >> #include "intel_panel.h" >> #include "intel_psr.h" >> +#include "intel_tc.h" >> #include "intel_vdsc.h" >> >> struct ddi_buf_trans { >> @@ -3904,7 +3905,6 @@ static int intel_ddi_compute_config(struct intel_encoder *encoder, >> static void intel_ddi_encoder_suspend(struct intel_encoder *encoder) >> { >> struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base); >> - struct drm_i915_private *i915 = to_i915(encoder->base.dev); >> >> intel_dp_encoder_suspend(encoder); >> >> @@ -3914,7 +3914,7 @@ static void intel_ddi_encoder_suspend(struct intel_encoder *encoder) >> * even if the sink has disappeared while being suspended. >> */ >> if (dig_port->tc_legacy_port) >> - icl_tc_phy_disconnect(i915, dig_port); >> + icl_tc_phy_disconnect(dig_port); >> } >> >> static void intel_ddi_encoder_reset(struct drm_encoder *drm_encoder) >> @@ -3936,7 +3936,7 @@ static void intel_ddi_encoder_destroy(struct drm_encoder *encoder) >> intel_dp_encoder_flush_work(encoder); >> >> if (intel_port_is_tc(i915, dig_port->base.port)) >> - icl_tc_phy_disconnect(i915, dig_port); >> + icl_tc_phy_disconnect(dig_port); >> >> drm_encoder_cleanup(encoder); >> kfree(dig_port); >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c >> index 24b56b2a76c8..b69310bd9914 100644 >> --- a/drivers/gpu/drm/i915/intel_dp.c >> +++ b/drivers/gpu/drm/i915/intel_dp.c >> @@ -62,6 +62,7 @@ >> #include "intel_panel.h" >> #include "intel_psr.h" >> #include "intel_sideband.h" >> +#include "intel_tc.h" >> #include "intel_vdsc.h" >> >> #define DP_DPRX_ESI_LEN 14 >> @@ -211,46 +212,13 @@ static int intel_dp_max_common_rate(struct intel_dp *intel_dp) >> return intel_dp->common_rates[intel_dp->num_common_rates - 1]; >> } >> >> -static int intel_dp_get_fia_supported_lane_count(struct intel_dp *intel_dp) >> -{ >> - struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp); >> - 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); >> - intel_wakeref_t wakeref; >> - u32 lane_info; >> - >> - if (tc_port == PORT_TC_NONE || dig_port->tc_type != TC_PORT_TYPEC) >> - return 4; >> - >> - lane_info = 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); >> - >> - switch (lane_info) { >> - default: >> - MISSING_CASE(lane_info); >> - case 1: >> - case 2: >> - case 4: >> - case 8: >> - return 1; >> - case 3: >> - case 12: >> - return 2; >> - case 15: >> - return 4; >> - } >> -} >> - >> /* Theoretical max between source and sink */ >> static int intel_dp_max_common_lane_count(struct intel_dp *intel_dp) >> { >> struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); >> int source_max = intel_dig_port->max_lanes; >> int sink_max = drm_dp_max_lane_count(intel_dp->dpcd); >> - int fia_max = intel_dp_get_fia_supported_lane_count(intel_dp); >> + int fia_max = intel_tc_port_fia_max_lane_count(intel_dig_port); >> >> return min3(source_max, sink_max, fia_max); >> } >> @@ -5231,195 +5199,6 @@ static bool icl_combo_port_connected(struct drm_i915_private *dev_priv, >> return I915_READ(SDEISR) & SDE_DDI_HOTPLUG_ICP(port); >> } >> >> -static const char *tc_type_name(enum tc_port_type type) >> -{ >> - static const char * const names[] = { >> - [TC_PORT_UNKNOWN] = "unknown", >> - [TC_PORT_LEGACY] = "legacy", >> - [TC_PORT_TYPEC] = "typec", >> - [TC_PORT_TBT] = "tbt", >> - }; >> - >> - if (WARN_ON(type >= ARRAY_SIZE(names))) >> - type = TC_PORT_UNKNOWN; >> - >> - return names[type]; >> -} >> - >> -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) >> -{ >> - enum port port = intel_dig_port->base.port; >> - enum tc_port_type old_type = intel_dig_port->tc_type; >> - >> - WARN_ON(is_legacy + is_typec + is_tbt != 1); >> - >> - if (is_legacy) >> - intel_dig_port->tc_type = TC_PORT_LEGACY; >> - else if (is_typec) >> - intel_dig_port->tc_type = TC_PORT_TYPEC; >> - else if (is_tbt) >> - intel_dig_port->tc_type = TC_PORT_TBT; >> - else >> - return; >> - >> - /* Types are not supposed to be changed at runtime. */ >> - WARN_ON(old_type != TC_PORT_UNKNOWN && >> - old_type != intel_dig_port->tc_type); >> - >> - if (old_type != intel_dig_port->tc_type) >> - DRM_DEBUG_KMS("Port %c has TC type %s\n", port_name(port), >> - tc_type_name(intel_dig_port->tc_type)); >> -} >> - >> -/* >> - * This function implements the first part of the Connect Flow described by our >> - * specification, Gen11 TypeC Programming chapter. The rest of the flow (reading >> - * lanes, EDID, etc) is done as needed in the typical places. >> - * >> - * Unlike the other ports, type-C ports are not available to use as soon as we >> - * get a hotplug. The type-C PHYs can be shared between multiple controllers: >> - * display, USB, etc. As a result, handshaking through FIA is required around >> - * connect and disconnect to cleanly transfer ownership with the controller and >> - * set the type-C power state. >> - * >> - * We could opt to only do the connect flow when we actually try to use the AUX >> - * channels or do a modeset, then immediately run the disconnect flow after >> - * usage, but there are some implications on this for a dynamic environment: >> - * things may go away or change behind our backs. So for now our driver is >> - * always trying to acquire ownership of the controller as soon as it gets an >> - * interrupt (or polls state and sees a port is connected) and only gives it >> - * back when it sees a disconnect. Implementation of a more fine-grained model >> - * will require a lot of coordination with user space and thorough testing for >> - * the extra possible cases. >> - */ >> -static bool icl_tc_phy_connect(struct drm_i915_private *dev_priv, >> - struct intel_digital_port *dig_port) >> -{ >> - enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port->base.port); >> - u32 val; >> - >> - if (dig_port->tc_type != TC_PORT_LEGACY && >> - dig_port->tc_type != TC_PORT_TYPEC) >> - return true; >> - >> - val = I915_READ(PORT_TX_DFLEXDPPMS); >> - if (!(val & DP_PHY_MODE_STATUS_COMPLETED(tc_port))) { >> - DRM_DEBUG_KMS("DP PHY for TC port %d not ready\n", tc_port); >> - 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); >> - } >> - >> - /* >> - * 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_type == TC_PORT_TYPEC && >> - !(I915_READ(PORT_TX_DFLEXDPSP) & TC_LIVE_STATE_TC(tc_port))) { >> - DRM_DEBUG_KMS("TC PHY %d sudden disconnect.\n", tc_port); >> - icl_tc_phy_disconnect(dev_priv, dig_port); >> - return false; >> - } >> - >> - return true; >> -} >> - >> -/* >> - * See the comment at the connect function. This implements the Disconnect >> - * Flow. >> - */ >> -void icl_tc_phy_disconnect(struct drm_i915_private *dev_priv, >> - struct intel_digital_port *dig_port) >> -{ >> - enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port->base.port); >> - >> - if (dig_port->tc_type == TC_PORT_UNKNOWN) >> - return; >> - >> - /* >> - * TBT disconnection flow is read the live status, what was done in >> - * caller. >> - */ >> - if (dig_port->tc_type == TC_PORT_TYPEC || >> - dig_port->tc_type == 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); >> - } >> - >> - DRM_DEBUG_KMS("Port %c TC type %s disconnected\n", >> - port_name(dig_port->base.port), >> - tc_type_name(dig_port->tc_type)); >> - >> - dig_port->tc_type = TC_PORT_UNKNOWN; >> -} >> - >> -/* >> - * The type-C ports are different because even when they are connected, they may >> - * not be available/usable by the graphics driver: see the comment on >> - * icl_tc_phy_connect(). So in our driver instead of adding the additional >> - * concept of "usable" and make everything check for "connected and usable" we >> - * define a port as "connected" when it is not only connected, but also when it >> - * is usable by the rest of the driver. That maintains the old assumption that >> - * connected ports are usable, and avoids exposing to the users objects they >> - * can't really use. >> - */ >> -static bool icl_tc_port_connected(struct drm_i915_private *dev_priv, >> - struct intel_digital_port *intel_dig_port) >> -{ >> - enum port port = intel_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 (!intel_dig_port->tc_legacy_port && >> - I915_READ(SDEISR) & SDE_TC_HOTPLUG_ICP(tc_port)) { >> - DRM_ERROR("VBT incorrectly claims port %c is not TypeC legacy\n", >> - port_name(port)); >> - intel_dig_port->tc_legacy_port = true; >> - } >> - is_legacy = intel_dig_port->tc_legacy_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) { >> - icl_tc_phy_disconnect(dev_priv, intel_dig_port); >> - >> - return false; >> - } >> - >> - icl_update_tc_port_type(dev_priv, intel_dig_port, is_legacy, is_typec, >> - is_tbt); >> - >> - if (!icl_tc_phy_connect(dev_priv, intel_dig_port)) >> - return false; >> - >> - return true; >> -} >> - >> static bool icl_digital_port_connected(struct intel_encoder *encoder) >> { >> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); >> @@ -5428,7 +5207,7 @@ static bool icl_digital_port_connected(struct intel_encoder *encoder) >> if (intel_port_is_combophy(dev_priv, encoder->port)) >> return icl_combo_port_connected(dev_priv, dig_port); >> else if (intel_port_is_tc(dev_priv, encoder->port)) >> - return icl_tc_port_connected(dev_priv, dig_port); >> + return intel_tc_port_connected(dig_port); >> else >> MISSING_CASE(encoder->hpd_pin); >> >> diff --git a/drivers/gpu/drm/i915/intel_dp.h b/drivers/gpu/drm/i915/intel_dp.h >> index da70b1a41c83..657bbb1f5ed0 100644 >> --- a/drivers/gpu/drm/i915/intel_dp.h >> +++ b/drivers/gpu/drm/i915/intel_dp.h >> @@ -112,8 +112,6 @@ bool intel_dp_get_colorimetry_status(struct intel_dp *intel_dp); >> int intel_dp_link_required(int pixel_clock, int bpp); >> int intel_dp_max_data_rate(int max_link_clock, int max_lanes); >> bool intel_digital_port_connected(struct intel_encoder *encoder); >> -void icl_tc_phy_disconnect(struct drm_i915_private *dev_priv, >> - struct intel_digital_port *dig_port); >> >> static inline unsigned int intel_dp_unused_lane_mask(int lane_count) >> { >> diff --git a/drivers/gpu/drm/i915/intel_tc.c b/drivers/gpu/drm/i915/intel_tc.c >> new file mode 100644 >> index 000000000000..7a1b5870945f >> --- /dev/null >> +++ b/drivers/gpu/drm/i915/intel_tc.c >> @@ -0,0 +1,230 @@ >> +// SPDX-License-Identifier: MIT >> +/* >> + * Copyright © 2019 Intel Corporation >> + */ > > From the Nitpicks Department, please add a blank line here. > >> +#include "intel_display.h" >> +#include "i915_drv.h" >> +#include "intel_tc.h" > > And sort the includes. > >> + >> +static const char *tc_type_name(enum tc_port_type type) >> +{ >> + static const char * const names[] = { >> + [TC_PORT_UNKNOWN] = "unknown", >> + [TC_PORT_LEGACY] = "legacy", >> + [TC_PORT_TYPEC] = "typec", >> + [TC_PORT_TBT] = "tbt", >> + }; >> + >> + if (WARN_ON(type >= ARRAY_SIZE(names))) >> + type = TC_PORT_UNKNOWN; >> + >> + return names[type]; >> +} >> + >> +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); >> + enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port->base.port); >> + intel_wakeref_t wakeref; >> + u32 lane_info; >> + >> + if (tc_port == PORT_TC_NONE || dig_port->tc_type != TC_PORT_TYPEC) >> + return 4; >> + >> + lane_info = 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); >> + >> + switch (lane_info) { >> + default: >> + MISSING_CASE(lane_info); >> + case 1: >> + case 2: >> + case 4: >> + case 8: >> + return 1; >> + case 3: >> + case 12: >> + return 2; >> + case 15: >> + return 4; >> + } >> +} >> + >> +/* >> + * This function implements the first part of the Connect Flow described by our >> + * specification, Gen11 TypeC Programming chapter. The rest of the flow (reading >> + * lanes, EDID, etc) is done as needed in the typical places. >> + * >> + * Unlike the other ports, type-C ports are not available to use as soon as we >> + * get a hotplug. The type-C PHYs can be shared between multiple controllers: >> + * display, USB, etc. As a result, handshaking through FIA is required around >> + * connect and disconnect to cleanly transfer ownership with the controller and >> + * set the type-C power state. >> + * >> + * We could opt to only do the connect flow when we actually try to use the AUX >> + * channels or do a modeset, then immediately run the disconnect flow after >> + * usage, but there are some implications on this for a dynamic environment: >> + * things may go away or change behind our backs. So for now our driver is >> + * always trying to acquire ownership of the controller as soon as it gets an >> + * interrupt (or polls state and sees a port is connected) and only gives it >> + * back when it sees a disconnect. Implementation of a more fine-grained model >> + * will require a lot of coordination with user space and thorough testing for >> + * the extra possible cases. >> + */ >> +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; >> + >> + if (dig_port->tc_type != TC_PORT_LEGACY && >> + dig_port->tc_type != TC_PORT_TYPEC) >> + return true; >> + >> + val = I915_READ(PORT_TX_DFLEXDPPMS); >> + if (!(val & DP_PHY_MODE_STATUS_COMPLETED(tc_port))) { >> + DRM_DEBUG_KMS("DP PHY for TC port %d not ready\n", tc_port); >> + 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); >> + } >> + >> + /* >> + * 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_type == TC_PORT_TYPEC && >> + !(I915_READ(PORT_TX_DFLEXDPSP) & TC_LIVE_STATE_TC(tc_port))) { >> + DRM_DEBUG_KMS("TC PHY %d sudden disconnect.\n", tc_port); >> + icl_tc_phy_disconnect(dig_port); >> + return false; >> + } >> + >> + return true; >> +} >> + >> +/* >> + * See the comment at the connect function. This implements the Disconnect >> + * Flow. >> + */ >> +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); >> + >> + if (dig_port->tc_type == TC_PORT_UNKNOWN) >> + return; >> + >> + /* >> + * TBT disconnection flow is read the live status, what was done in >> + * caller. >> + */ >> + if (dig_port->tc_type == TC_PORT_TYPEC || >> + dig_port->tc_type == 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); >> + } >> + >> + DRM_DEBUG_KMS("Port %c TC type %s disconnected\n", >> + port_name(dig_port->base.port), >> + tc_type_name(dig_port->tc_type)); >> + >> + dig_port->tc_type = TC_PORT_UNKNOWN; >> +} >> + >> +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) >> +{ >> + enum port port = intel_dig_port->base.port; >> + enum tc_port_type old_type = intel_dig_port->tc_type; >> + >> + WARN_ON(is_legacy + is_typec + is_tbt != 1); >> + >> + if (is_legacy) >> + intel_dig_port->tc_type = TC_PORT_LEGACY; >> + else if (is_typec) >> + intel_dig_port->tc_type = TC_PORT_TYPEC; >> + else if (is_tbt) >> + intel_dig_port->tc_type = TC_PORT_TBT; >> + else >> + return; >> + >> + /* Types are not supposed to be changed at runtime. */ >> + WARN_ON(old_type != TC_PORT_UNKNOWN && >> + old_type != intel_dig_port->tc_type); >> + >> + if (old_type != intel_dig_port->tc_type) >> + DRM_DEBUG_KMS("Port %c has TC type %s\n", port_name(port), >> + tc_type_name(intel_dig_port->tc_type)); >> +} >> + >> + >> +/* >> + * The type-C ports are different because even when they are connected, they may >> + * not be available/usable by the graphics driver: see the comment on >> + * icl_tc_phy_connect(). So in our driver instead of adding the additional >> + * concept of "usable" and make everything check for "connected and usable" we >> + * define a port as "connected" when it is not only connected, but also when it >> + * is usable by the rest of the driver. That maintains the old assumption that >> + * connected ports are usable, and avoids exposing to the users objects they >> + * can't really use. >> + */ >> +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("VBT incorrectly claims port %c is not TypeC legacy\n", >> + port_name(port)); >> + dig_port->tc_legacy_port = true; >> + } >> + is_legacy = dig_port->tc_legacy_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) { >> + icl_tc_phy_disconnect(dig_port); >> + >> + return false; >> + } >> + >> + icl_update_tc_port_type(dev_priv, dig_port, is_legacy, is_typec, >> + is_tbt); >> + >> + if (!icl_tc_phy_connect(dig_port)) >> + return false; >> + >> + return true; >> +} >> + >> diff --git a/drivers/gpu/drm/i915/intel_tc.h b/drivers/gpu/drm/i915/intel_tc.h >> new file mode 100644 >> index 000000000000..94c62ac4a162 >> --- /dev/null >> +++ b/drivers/gpu/drm/i915/intel_tc.h >> @@ -0,0 +1,13 @@ > > SPDX header here please. > >> +#ifndef __INTEL_TC_H__ >> +#define __INTEL_TC_H__ >> + >> +#include <linux/types.h> >> + >> +struct intel_digital_port; >> + >> +void icl_tc_phy_disconnect(struct intel_digital_port *dig_port); > > I'd like to see this named intel_tc_* in the future. > > BR, > Jani. > > >> + >> +bool intel_tc_port_connected(struct intel_digital_port *dig_port); >> +int intel_tc_port_fia_max_lane_count(struct intel_digital_port *dig_port); >> + >> +#endif /* __INTEL_TC_H__ */
On Thu, Jun 06, 2019 at 11:42:46AM +0300, Jani Nikula wrote: > On Tue, 04 Jun 2019, Imre Deak <imre.deak@intel.com> wrote: > > Move the TypeC port handling functions to a new file for clarity. > > > > While at it: > > - s/icl_tc_port_connected()/intel_tc_port_connected()/ > > icl_tc_phy_disconnect(), will be unexported later. > > > > - s/intel_dp_get_fia_supported_lane_count()/intel_tc_port_fia_max_lane_count()/ > > It's used for HDMI legacy mode too. > > > > - Simplify function interfaces by passing only dig_port to them. > > > > No functional changes. > > Some nitpicks below. > > BR, > Jani. > > > > > > Cc: Animesh Manna <animesh.manna@intel.com> > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > > Cc: José Roberto de Souza <jose.souza@intel.com> > > Signed-off-by: Imre Deak <imre.deak@intel.com> > > --- > > drivers/gpu/drm/i915/Makefile | 3 +- > > drivers/gpu/drm/i915/Makefile.header-test | 1 + > > drivers/gpu/drm/i915/intel_ddi.c | 6 +- > > drivers/gpu/drm/i915/intel_dp.c | 227 +-------------------- > > drivers/gpu/drm/i915/intel_dp.h | 2 - > > drivers/gpu/drm/i915/intel_tc.c | 230 ++++++++++++++++++++++ > > drivers/gpu/drm/i915/intel_tc.h | 13 ++ > > 7 files changed, 252 insertions(+), 230 deletions(-) > > create mode 100644 drivers/gpu/drm/i915/intel_tc.c > > create mode 100644 drivers/gpu/drm/i915/intel_tc.h > > > > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile > > index c0a7b2994077..74c4d11d83eb 100644 > > --- a/drivers/gpu/drm/i915/Makefile > > +++ b/drivers/gpu/drm/i915/Makefile > > @@ -171,7 +171,8 @@ i915-y += intel_audio.o \ > > intel_psr.o \ > > intel_quirks.o \ > > intel_sideband.o \ > > - intel_sprite.o > > + intel_sprite.o \ > > + intel_tc.o > > i915-$(CONFIG_ACPI) += intel_acpi.o intel_opregion.o > > i915-$(CONFIG_DRM_FBDEV_EMULATION) += intel_fbdev.o > > > > diff --git a/drivers/gpu/drm/i915/Makefile.header-test b/drivers/gpu/drm/i915/Makefile.header-test > > index 6ef3b647ac65..e80e8e45b09c 100644 > > --- a/drivers/gpu/drm/i915/Makefile.header-test > > +++ b/drivers/gpu/drm/i915/Makefile.header-test > > @@ -58,6 +58,7 @@ header_test := \ > > intel_sdvo.h \ > > intel_sideband.h \ > > intel_sprite.h \ > > + intel_tc.h \ > > intel_tv.h \ > > intel_uncore.h \ > > intel_vdsc.h \ > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > > index 350eaf54f01f..5a1c98438375 100644 > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > @@ -45,6 +45,7 @@ > > #include "intel_lspcon.h" > > #include "intel_panel.h" > > #include "intel_psr.h" > > +#include "intel_tc.h" > > #include "intel_vdsc.h" > > > > struct ddi_buf_trans { > > @@ -3904,7 +3905,6 @@ static int intel_ddi_compute_config(struct intel_encoder *encoder, > > static void intel_ddi_encoder_suspend(struct intel_encoder *encoder) > > { > > struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base); > > - struct drm_i915_private *i915 = to_i915(encoder->base.dev); > > > > intel_dp_encoder_suspend(encoder); > > > > @@ -3914,7 +3914,7 @@ static void intel_ddi_encoder_suspend(struct intel_encoder *encoder) > > * even if the sink has disappeared while being suspended. > > */ > > if (dig_port->tc_legacy_port) > > - icl_tc_phy_disconnect(i915, dig_port); > > + icl_tc_phy_disconnect(dig_port); > > } > > > > static void intel_ddi_encoder_reset(struct drm_encoder *drm_encoder) > > @@ -3936,7 +3936,7 @@ static void intel_ddi_encoder_destroy(struct drm_encoder *encoder) > > intel_dp_encoder_flush_work(encoder); > > > > if (intel_port_is_tc(i915, dig_port->base.port)) > > - icl_tc_phy_disconnect(i915, dig_port); > > + icl_tc_phy_disconnect(dig_port); > > > > drm_encoder_cleanup(encoder); > > kfree(dig_port); > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > index 24b56b2a76c8..b69310bd9914 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -62,6 +62,7 @@ > > #include "intel_panel.h" > > #include "intel_psr.h" > > #include "intel_sideband.h" > > +#include "intel_tc.h" > > #include "intel_vdsc.h" > > > > #define DP_DPRX_ESI_LEN 14 > > @@ -211,46 +212,13 @@ static int intel_dp_max_common_rate(struct intel_dp *intel_dp) > > return intel_dp->common_rates[intel_dp->num_common_rates - 1]; > > } > > > > -static int intel_dp_get_fia_supported_lane_count(struct intel_dp *intel_dp) > > -{ > > - struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp); > > - 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); > > - intel_wakeref_t wakeref; > > - u32 lane_info; > > - > > - if (tc_port == PORT_TC_NONE || dig_port->tc_type != TC_PORT_TYPEC) > > - return 4; > > - > > - lane_info = 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); > > - > > - switch (lane_info) { > > - default: > > - MISSING_CASE(lane_info); > > - case 1: > > - case 2: > > - case 4: > > - case 8: > > - return 1; > > - case 3: > > - case 12: > > - return 2; > > - case 15: > > - return 4; > > - } > > -} > > - > > /* Theoretical max between source and sink */ > > static int intel_dp_max_common_lane_count(struct intel_dp *intel_dp) > > { > > struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); > > int source_max = intel_dig_port->max_lanes; > > int sink_max = drm_dp_max_lane_count(intel_dp->dpcd); > > - int fia_max = intel_dp_get_fia_supported_lane_count(intel_dp); > > + int fia_max = intel_tc_port_fia_max_lane_count(intel_dig_port); > > > > return min3(source_max, sink_max, fia_max); > > } > > @@ -5231,195 +5199,6 @@ static bool icl_combo_port_connected(struct drm_i915_private *dev_priv, > > return I915_READ(SDEISR) & SDE_DDI_HOTPLUG_ICP(port); > > } > > > > -static const char *tc_type_name(enum tc_port_type type) > > -{ > > - static const char * const names[] = { > > - [TC_PORT_UNKNOWN] = "unknown", > > - [TC_PORT_LEGACY] = "legacy", > > - [TC_PORT_TYPEC] = "typec", > > - [TC_PORT_TBT] = "tbt", > > - }; > > - > > - if (WARN_ON(type >= ARRAY_SIZE(names))) > > - type = TC_PORT_UNKNOWN; > > - > > - return names[type]; > > -} > > - > > -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) > > -{ > > - enum port port = intel_dig_port->base.port; > > - enum tc_port_type old_type = intel_dig_port->tc_type; > > - > > - WARN_ON(is_legacy + is_typec + is_tbt != 1); > > - > > - if (is_legacy) > > - intel_dig_port->tc_type = TC_PORT_LEGACY; > > - else if (is_typec) > > - intel_dig_port->tc_type = TC_PORT_TYPEC; > > - else if (is_tbt) > > - intel_dig_port->tc_type = TC_PORT_TBT; > > - else > > - return; > > - > > - /* Types are not supposed to be changed at runtime. */ > > - WARN_ON(old_type != TC_PORT_UNKNOWN && > > - old_type != intel_dig_port->tc_type); > > - > > - if (old_type != intel_dig_port->tc_type) > > - DRM_DEBUG_KMS("Port %c has TC type %s\n", port_name(port), > > - tc_type_name(intel_dig_port->tc_type)); > > -} > > - > > -/* > > - * This function implements the first part of the Connect Flow described by our > > - * specification, Gen11 TypeC Programming chapter. The rest of the flow (reading > > - * lanes, EDID, etc) is done as needed in the typical places. > > - * > > - * Unlike the other ports, type-C ports are not available to use as soon as we > > - * get a hotplug. The type-C PHYs can be shared between multiple controllers: > > - * display, USB, etc. As a result, handshaking through FIA is required around > > - * connect and disconnect to cleanly transfer ownership with the controller and > > - * set the type-C power state. > > - * > > - * We could opt to only do the connect flow when we actually try to use the AUX > > - * channels or do a modeset, then immediately run the disconnect flow after > > - * usage, but there are some implications on this for a dynamic environment: > > - * things may go away or change behind our backs. So for now our driver is > > - * always trying to acquire ownership of the controller as soon as it gets an > > - * interrupt (or polls state and sees a port is connected) and only gives it > > - * back when it sees a disconnect. Implementation of a more fine-grained model > > - * will require a lot of coordination with user space and thorough testing for > > - * the extra possible cases. > > - */ > > -static bool icl_tc_phy_connect(struct drm_i915_private *dev_priv, > > - struct intel_digital_port *dig_port) > > -{ > > - enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port->base.port); > > - u32 val; > > - > > - if (dig_port->tc_type != TC_PORT_LEGACY && > > - dig_port->tc_type != TC_PORT_TYPEC) > > - return true; > > - > > - val = I915_READ(PORT_TX_DFLEXDPPMS); > > - if (!(val & DP_PHY_MODE_STATUS_COMPLETED(tc_port))) { > > - DRM_DEBUG_KMS("DP PHY for TC port %d not ready\n", tc_port); > > - 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); > > - } > > - > > - /* > > - * 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_type == TC_PORT_TYPEC && > > - !(I915_READ(PORT_TX_DFLEXDPSP) & TC_LIVE_STATE_TC(tc_port))) { > > - DRM_DEBUG_KMS("TC PHY %d sudden disconnect.\n", tc_port); > > - icl_tc_phy_disconnect(dev_priv, dig_port); > > - return false; > > - } > > - > > - return true; > > -} > > - > > -/* > > - * See the comment at the connect function. This implements the Disconnect > > - * Flow. > > - */ > > -void icl_tc_phy_disconnect(struct drm_i915_private *dev_priv, > > - struct intel_digital_port *dig_port) > > -{ > > - enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port->base.port); > > - > > - if (dig_port->tc_type == TC_PORT_UNKNOWN) > > - return; > > - > > - /* > > - * TBT disconnection flow is read the live status, what was done in > > - * caller. > > - */ > > - if (dig_port->tc_type == TC_PORT_TYPEC || > > - dig_port->tc_type == 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); > > - } > > - > > - DRM_DEBUG_KMS("Port %c TC type %s disconnected\n", > > - port_name(dig_port->base.port), > > - tc_type_name(dig_port->tc_type)); > > - > > - dig_port->tc_type = TC_PORT_UNKNOWN; > > -} > > - > > -/* > > - * The type-C ports are different because even when they are connected, they may > > - * not be available/usable by the graphics driver: see the comment on > > - * icl_tc_phy_connect(). So in our driver instead of adding the additional > > - * concept of "usable" and make everything check for "connected and usable" we > > - * define a port as "connected" when it is not only connected, but also when it > > - * is usable by the rest of the driver. That maintains the old assumption that > > - * connected ports are usable, and avoids exposing to the users objects they > > - * can't really use. > > - */ > > -static bool icl_tc_port_connected(struct drm_i915_private *dev_priv, > > - struct intel_digital_port *intel_dig_port) > > -{ > > - enum port port = intel_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 (!intel_dig_port->tc_legacy_port && > > - I915_READ(SDEISR) & SDE_TC_HOTPLUG_ICP(tc_port)) { > > - DRM_ERROR("VBT incorrectly claims port %c is not TypeC legacy\n", > > - port_name(port)); > > - intel_dig_port->tc_legacy_port = true; > > - } > > - is_legacy = intel_dig_port->tc_legacy_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) { > > - icl_tc_phy_disconnect(dev_priv, intel_dig_port); > > - > > - return false; > > - } > > - > > - icl_update_tc_port_type(dev_priv, intel_dig_port, is_legacy, is_typec, > > - is_tbt); > > - > > - if (!icl_tc_phy_connect(dev_priv, intel_dig_port)) > > - return false; > > - > > - return true; > > -} > > - > > static bool icl_digital_port_connected(struct intel_encoder *encoder) > > { > > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > > @@ -5428,7 +5207,7 @@ static bool icl_digital_port_connected(struct intel_encoder *encoder) > > if (intel_port_is_combophy(dev_priv, encoder->port)) > > return icl_combo_port_connected(dev_priv, dig_port); > > else if (intel_port_is_tc(dev_priv, encoder->port)) > > - return icl_tc_port_connected(dev_priv, dig_port); > > + return intel_tc_port_connected(dig_port); > > else > > MISSING_CASE(encoder->hpd_pin); > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.h b/drivers/gpu/drm/i915/intel_dp.h > > index da70b1a41c83..657bbb1f5ed0 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.h > > +++ b/drivers/gpu/drm/i915/intel_dp.h > > @@ -112,8 +112,6 @@ bool intel_dp_get_colorimetry_status(struct intel_dp *intel_dp); > > int intel_dp_link_required(int pixel_clock, int bpp); > > int intel_dp_max_data_rate(int max_link_clock, int max_lanes); > > bool intel_digital_port_connected(struct intel_encoder *encoder); > > -void icl_tc_phy_disconnect(struct drm_i915_private *dev_priv, > > - struct intel_digital_port *dig_port); > > > > static inline unsigned int intel_dp_unused_lane_mask(int lane_count) > > { > > diff --git a/drivers/gpu/drm/i915/intel_tc.c b/drivers/gpu/drm/i915/intel_tc.c > > new file mode 100644 > > index 000000000000..7a1b5870945f > > --- /dev/null > > +++ b/drivers/gpu/drm/i915/intel_tc.c > > @@ -0,0 +1,230 @@ > > +// SPDX-License-Identifier: MIT > > +/* > > + * Copyright © 2019 Intel Corporation > > + */ > > From the Nitpicks Department, please add a blank line here. Ok. > > > +#include "intel_display.h" > > +#include "i915_drv.h" > > +#include "intel_tc.h" > > And sort the includes. Ok, forgot that here.. > > > + > > +static const char *tc_type_name(enum tc_port_type type) > > +{ > > + static const char * const names[] = { > > + [TC_PORT_UNKNOWN] = "unknown", > > + [TC_PORT_LEGACY] = "legacy", > > + [TC_PORT_TYPEC] = "typec", > > + [TC_PORT_TBT] = "tbt", > > + }; > > + > > + if (WARN_ON(type >= ARRAY_SIZE(names))) > > + type = TC_PORT_UNKNOWN; > > + > > + return names[type]; > > +} > > + > > +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); > > + enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port->base.port); > > + intel_wakeref_t wakeref; > > + u32 lane_info; > > + > > + if (tc_port == PORT_TC_NONE || dig_port->tc_type != TC_PORT_TYPEC) > > + return 4; > > + > > + lane_info = 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); > > + > > + switch (lane_info) { > > + default: > > + MISSING_CASE(lane_info); > > + case 1: > > + case 2: > > + case 4: > > + case 8: > > + return 1; > > + case 3: > > + case 12: > > + return 2; > > + case 15: > > + return 4; > > + } > > +} > > + > > +/* > > + * This function implements the first part of the Connect Flow described by our > > + * specification, Gen11 TypeC Programming chapter. The rest of the flow (reading > > + * lanes, EDID, etc) is done as needed in the typical places. > > + * > > + * Unlike the other ports, type-C ports are not available to use as soon as we > > + * get a hotplug. The type-C PHYs can be shared between multiple controllers: > > + * display, USB, etc. As a result, handshaking through FIA is required around > > + * connect and disconnect to cleanly transfer ownership with the controller and > > + * set the type-C power state. > > + * > > + * We could opt to only do the connect flow when we actually try to use the AUX > > + * channels or do a modeset, then immediately run the disconnect flow after > > + * usage, but there are some implications on this for a dynamic environment: > > + * things may go away or change behind our backs. So for now our driver is > > + * always trying to acquire ownership of the controller as soon as it gets an > > + * interrupt (or polls state and sees a port is connected) and only gives it > > + * back when it sees a disconnect. Implementation of a more fine-grained model > > + * will require a lot of coordination with user space and thorough testing for > > + * the extra possible cases. > > + */ > > +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; > > + > > + if (dig_port->tc_type != TC_PORT_LEGACY && > > + dig_port->tc_type != TC_PORT_TYPEC) > > + return true; > > + > > + val = I915_READ(PORT_TX_DFLEXDPPMS); > > + if (!(val & DP_PHY_MODE_STATUS_COMPLETED(tc_port))) { > > + DRM_DEBUG_KMS("DP PHY for TC port %d not ready\n", tc_port); > > + 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); > > + } > > + > > + /* > > + * 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_type == TC_PORT_TYPEC && > > + !(I915_READ(PORT_TX_DFLEXDPSP) & TC_LIVE_STATE_TC(tc_port))) { > > + DRM_DEBUG_KMS("TC PHY %d sudden disconnect.\n", tc_port); > > + icl_tc_phy_disconnect(dig_port); > > + return false; > > + } > > + > > + return true; > > +} > > + > > +/* > > + * See the comment at the connect function. This implements the Disconnect > > + * Flow. > > + */ > > +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); > > + > > + if (dig_port->tc_type == TC_PORT_UNKNOWN) > > + return; > > + > > + /* > > + * TBT disconnection flow is read the live status, what was done in > > + * caller. > > + */ > > + if (dig_port->tc_type == TC_PORT_TYPEC || > > + dig_port->tc_type == 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); > > + } > > + > > + DRM_DEBUG_KMS("Port %c TC type %s disconnected\n", > > + port_name(dig_port->base.port), > > + tc_type_name(dig_port->tc_type)); > > + > > + dig_port->tc_type = TC_PORT_UNKNOWN; > > +} > > + > > +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) > > +{ > > + enum port port = intel_dig_port->base.port; > > + enum tc_port_type old_type = intel_dig_port->tc_type; > > + > > + WARN_ON(is_legacy + is_typec + is_tbt != 1); > > + > > + if (is_legacy) > > + intel_dig_port->tc_type = TC_PORT_LEGACY; > > + else if (is_typec) > > + intel_dig_port->tc_type = TC_PORT_TYPEC; > > + else if (is_tbt) > > + intel_dig_port->tc_type = TC_PORT_TBT; > > + else > > + return; > > + > > + /* Types are not supposed to be changed at runtime. */ > > + WARN_ON(old_type != TC_PORT_UNKNOWN && > > + old_type != intel_dig_port->tc_type); > > + > > + if (old_type != intel_dig_port->tc_type) > > + DRM_DEBUG_KMS("Port %c has TC type %s\n", port_name(port), > > + tc_type_name(intel_dig_port->tc_type)); > > +} > > + > > + > > +/* > > + * The type-C ports are different because even when they are connected, they may > > + * not be available/usable by the graphics driver: see the comment on > > + * icl_tc_phy_connect(). So in our driver instead of adding the additional > > + * concept of "usable" and make everything check for "connected and usable" we > > + * define a port as "connected" when it is not only connected, but also when it > > + * is usable by the rest of the driver. That maintains the old assumption that > > + * connected ports are usable, and avoids exposing to the users objects they > > + * can't really use. > > + */ > > +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("VBT incorrectly claims port %c is not TypeC legacy\n", > > + port_name(port)); > > + dig_port->tc_legacy_port = true; > > + } > > + is_legacy = dig_port->tc_legacy_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) { > > + icl_tc_phy_disconnect(dig_port); > > + > > + return false; > > + } > > + > > + icl_update_tc_port_type(dev_priv, dig_port, is_legacy, is_typec, > > + is_tbt); > > + > > + if (!icl_tc_phy_connect(dig_port)) > > + return false; > > + > > + return true; > > +} > > + > > diff --git a/drivers/gpu/drm/i915/intel_tc.h b/drivers/gpu/drm/i915/intel_tc.h > > new file mode 100644 > > index 000000000000..94c62ac4a162 > > --- /dev/null > > +++ b/drivers/gpu/drm/i915/intel_tc.h > > @@ -0,0 +1,13 @@ > > SPDX header here please. Ok, added that like: https://github.com/ideak/linux/blob/2dee09f687ba/drivers/gpu/drm/i915/intel_tc.h#L2 > > > +#ifndef __INTEL_TC_H__ > > +#define __INTEL_TC_H__ > > + > > +#include <linux/types.h> > > + > > +struct intel_digital_port; > > + > > +void icl_tc_phy_disconnect(struct intel_digital_port *dig_port); > > I'd like to see this named intel_tc_* in the future. Yep, thought about that rule, which is good, but I kept this one as-is, since later in this patchset it will get unexported. > > BR, > Jani. > > > > + > > +bool intel_tc_port_connected(struct intel_digital_port *dig_port); > > +int intel_tc_port_fia_max_lane_count(struct intel_digital_port *dig_port); > > + > > +#endif /* __INTEL_TC_H__ */ > > -- > Jani Nikula, Intel Open Source Graphics Center
On Tue, 2019-06-04 at 17:58 +0300, Imre Deak wrote: > Move the TypeC port handling functions to a new file for clarity. > > While at it: > - s/icl_tc_port_connected()/intel_tc_port_connected()/ > icl_tc_phy_disconnect(), will be unexported later. > > - > s/intel_dp_get_fia_supported_lane_count()/intel_tc_port_fia_max_lane_ > count()/ > It's used for HDMI legacy mode too. > > - Simplify function interfaces by passing only dig_port to them. > > No functional changes. Nice. Reviewed-by: José Roberto de Souza <jose.souza@intel.com> > > Cc: Animesh Manna <animesh.manna@intel.com> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > Cc: José Roberto de Souza <jose.souza@intel.com> > Signed-off-by: Imre Deak <imre.deak@intel.com> > --- > drivers/gpu/drm/i915/Makefile | 3 +- > drivers/gpu/drm/i915/Makefile.header-test | 1 + > drivers/gpu/drm/i915/intel_ddi.c | 6 +- > drivers/gpu/drm/i915/intel_dp.c | 227 +------------------- > - > drivers/gpu/drm/i915/intel_dp.h | 2 - > drivers/gpu/drm/i915/intel_tc.c | 230 > ++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_tc.h | 13 ++ > 7 files changed, 252 insertions(+), 230 deletions(-) > create mode 100644 drivers/gpu/drm/i915/intel_tc.c > create mode 100644 drivers/gpu/drm/i915/intel_tc.h > > diff --git a/drivers/gpu/drm/i915/Makefile > b/drivers/gpu/drm/i915/Makefile > index c0a7b2994077..74c4d11d83eb 100644 > --- a/drivers/gpu/drm/i915/Makefile > +++ b/drivers/gpu/drm/i915/Makefile > @@ -171,7 +171,8 @@ i915-y += intel_audio.o \ > intel_psr.o \ > intel_quirks.o \ > intel_sideband.o \ > - intel_sprite.o > + intel_sprite.o \ > + intel_tc.o > i915-$(CONFIG_ACPI) += intel_acpi.o intel_opregion.o > i915-$(CONFIG_DRM_FBDEV_EMULATION) += intel_fbdev.o > > diff --git a/drivers/gpu/drm/i915/Makefile.header-test > b/drivers/gpu/drm/i915/Makefile.header-test > index 6ef3b647ac65..e80e8e45b09c 100644 > --- a/drivers/gpu/drm/i915/Makefile.header-test > +++ b/drivers/gpu/drm/i915/Makefile.header-test > @@ -58,6 +58,7 @@ header_test := \ > intel_sdvo.h \ > intel_sideband.h \ > intel_sprite.h \ > + intel_tc.h \ > intel_tv.h \ > intel_uncore.h \ > intel_vdsc.h \ > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > b/drivers/gpu/drm/i915/intel_ddi.c > index 350eaf54f01f..5a1c98438375 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -45,6 +45,7 @@ > #include "intel_lspcon.h" > #include "intel_panel.h" > #include "intel_psr.h" > +#include "intel_tc.h" > #include "intel_vdsc.h" > > struct ddi_buf_trans { > @@ -3904,7 +3905,6 @@ static int intel_ddi_compute_config(struct > intel_encoder *encoder, > static void intel_ddi_encoder_suspend(struct intel_encoder *encoder) > { > struct intel_digital_port *dig_port = enc_to_dig_port(&encoder- > >base); > - struct drm_i915_private *i915 = to_i915(encoder->base.dev); > > intel_dp_encoder_suspend(encoder); > > @@ -3914,7 +3914,7 @@ static void intel_ddi_encoder_suspend(struct > intel_encoder *encoder) > * even if the sink has disappeared while being suspended. > */ > if (dig_port->tc_legacy_port) > - icl_tc_phy_disconnect(i915, dig_port); > + icl_tc_phy_disconnect(dig_port); > } > > static void intel_ddi_encoder_reset(struct drm_encoder *drm_encoder) > @@ -3936,7 +3936,7 @@ static void intel_ddi_encoder_destroy(struct > drm_encoder *encoder) > intel_dp_encoder_flush_work(encoder); > > if (intel_port_is_tc(i915, dig_port->base.port)) > - icl_tc_phy_disconnect(i915, dig_port); > + icl_tc_phy_disconnect(dig_port); > > drm_encoder_cleanup(encoder); > kfree(dig_port); > diff --git a/drivers/gpu/drm/i915/intel_dp.c > b/drivers/gpu/drm/i915/intel_dp.c > index 24b56b2a76c8..b69310bd9914 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -62,6 +62,7 @@ > #include "intel_panel.h" > #include "intel_psr.h" > #include "intel_sideband.h" > +#include "intel_tc.h" > #include "intel_vdsc.h" > > #define DP_DPRX_ESI_LEN 14 > @@ -211,46 +212,13 @@ static int intel_dp_max_common_rate(struct > intel_dp *intel_dp) > return intel_dp->common_rates[intel_dp->num_common_rates - 1]; > } > > -static int intel_dp_get_fia_supported_lane_count(struct intel_dp > *intel_dp) > -{ > - struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp); > - 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); > - intel_wakeref_t wakeref; > - u32 lane_info; > - > - if (tc_port == PORT_TC_NONE || dig_port->tc_type != > TC_PORT_TYPEC) > - return 4; > - > - lane_info = 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); > - > - switch (lane_info) { > - default: > - MISSING_CASE(lane_info); > - case 1: > - case 2: > - case 4: > - case 8: > - return 1; > - case 3: > - case 12: > - return 2; > - case 15: > - return 4; > - } > -} > - > /* Theoretical max between source and sink */ > static int intel_dp_max_common_lane_count(struct intel_dp *intel_dp) > { > struct intel_digital_port *intel_dig_port = > dp_to_dig_port(intel_dp); > int source_max = intel_dig_port->max_lanes; > int sink_max = drm_dp_max_lane_count(intel_dp->dpcd); > - int fia_max = intel_dp_get_fia_supported_lane_count(intel_dp); > + int fia_max = intel_tc_port_fia_max_lane_count(intel_dig_port); > > return min3(source_max, sink_max, fia_max); > } > @@ -5231,195 +5199,6 @@ static bool icl_combo_port_connected(struct > drm_i915_private *dev_priv, > return I915_READ(SDEISR) & SDE_DDI_HOTPLUG_ICP(port); > } > > -static const char *tc_type_name(enum tc_port_type type) > -{ > - static const char * const names[] = { > - [TC_PORT_UNKNOWN] = "unknown", > - [TC_PORT_LEGACY] = "legacy", > - [TC_PORT_TYPEC] = "typec", > - [TC_PORT_TBT] = "tbt", > - }; > - > - if (WARN_ON(type >= ARRAY_SIZE(names))) > - type = TC_PORT_UNKNOWN; > - > - return names[type]; > -} > - > -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) > -{ > - enum port port = intel_dig_port->base.port; > - enum tc_port_type old_type = intel_dig_port->tc_type; > - > - WARN_ON(is_legacy + is_typec + is_tbt != 1); > - > - if (is_legacy) > - intel_dig_port->tc_type = TC_PORT_LEGACY; > - else if (is_typec) > - intel_dig_port->tc_type = TC_PORT_TYPEC; > - else if (is_tbt) > - intel_dig_port->tc_type = TC_PORT_TBT; > - else > - return; > - > - /* Types are not supposed to be changed at runtime. */ > - WARN_ON(old_type != TC_PORT_UNKNOWN && > - old_type != intel_dig_port->tc_type); > - > - if (old_type != intel_dig_port->tc_type) > - DRM_DEBUG_KMS("Port %c has TC type %s\n", > port_name(port), > - tc_type_name(intel_dig_port->tc_type)); > -} > - > -/* > - * This function implements the first part of the Connect Flow > described by our > - * specification, Gen11 TypeC Programming chapter. The rest of the > flow (reading > - * lanes, EDID, etc) is done as needed in the typical places. > - * > - * Unlike the other ports, type-C ports are not available to use as > soon as we > - * get a hotplug. The type-C PHYs can be shared between multiple > controllers: > - * display, USB, etc. As a result, handshaking through FIA is > required around > - * connect and disconnect to cleanly transfer ownership with the > controller and > - * set the type-C power state. > - * > - * We could opt to only do the connect flow when we actually try to > use the AUX > - * channels or do a modeset, then immediately run the disconnect > flow after > - * usage, but there are some implications on this for a dynamic > environment: > - * things may go away or change behind our backs. So for now our > driver is > - * always trying to acquire ownership of the controller as soon as > it gets an > - * interrupt (or polls state and sees a port is connected) and only > gives it > - * back when it sees a disconnect. Implementation of a more fine- > grained model > - * will require a lot of coordination with user space and thorough > testing for > - * the extra possible cases. > - */ > -static bool icl_tc_phy_connect(struct drm_i915_private *dev_priv, > - struct intel_digital_port *dig_port) > -{ > - enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port- > >base.port); > - u32 val; > - > - if (dig_port->tc_type != TC_PORT_LEGACY && > - dig_port->tc_type != TC_PORT_TYPEC) > - return true; > - > - val = I915_READ(PORT_TX_DFLEXDPPMS); > - if (!(val & DP_PHY_MODE_STATUS_COMPLETED(tc_port))) { > - DRM_DEBUG_KMS("DP PHY for TC port %d not ready\n", > tc_port); > - 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); > - } > - > - /* > - * 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_type == TC_PORT_TYPEC && > - !(I915_READ(PORT_TX_DFLEXDPSP) & > TC_LIVE_STATE_TC(tc_port))) { > - DRM_DEBUG_KMS("TC PHY %d sudden disconnect.\n", > tc_port); > - icl_tc_phy_disconnect(dev_priv, dig_port); > - return false; > - } > - > - return true; > -} > - > -/* > - * See the comment at the connect function. This implements the > Disconnect > - * Flow. > - */ > -void icl_tc_phy_disconnect(struct drm_i915_private *dev_priv, > - struct intel_digital_port *dig_port) > -{ > - enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port- > >base.port); > - > - if (dig_port->tc_type == TC_PORT_UNKNOWN) > - return; > - > - /* > - * TBT disconnection flow is read the live status, what was > done in > - * caller. > - */ > - if (dig_port->tc_type == TC_PORT_TYPEC || > - dig_port->tc_type == 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); > - } > - > - DRM_DEBUG_KMS("Port %c TC type %s disconnected\n", > - port_name(dig_port->base.port), > - tc_type_name(dig_port->tc_type)); > - > - dig_port->tc_type = TC_PORT_UNKNOWN; > -} > - > -/* > - * The type-C ports are different because even when they are > connected, they may > - * not be available/usable by the graphics driver: see the comment > on > - * icl_tc_phy_connect(). So in our driver instead of adding the > additional > - * concept of "usable" and make everything check for "connected and > usable" we > - * define a port as "connected" when it is not only connected, but > also when it > - * is usable by the rest of the driver. That maintains the old > assumption that > - * connected ports are usable, and avoids exposing to the users > objects they > - * can't really use. > - */ > -static bool icl_tc_port_connected(struct drm_i915_private *dev_priv, > - struct intel_digital_port > *intel_dig_port) > -{ > - enum port port = intel_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 (!intel_dig_port->tc_legacy_port && > - I915_READ(SDEISR) & SDE_TC_HOTPLUG_ICP(tc_port)) { > - DRM_ERROR("VBT incorrectly claims port %c is not TypeC > legacy\n", > - port_name(port)); > - intel_dig_port->tc_legacy_port = true; > - } > - is_legacy = intel_dig_port->tc_legacy_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) { > - icl_tc_phy_disconnect(dev_priv, intel_dig_port); > - > - return false; > - } > - > - icl_update_tc_port_type(dev_priv, intel_dig_port, is_legacy, > is_typec, > - is_tbt); > - > - if (!icl_tc_phy_connect(dev_priv, intel_dig_port)) > - return false; > - > - return true; > -} > - > static bool icl_digital_port_connected(struct intel_encoder > *encoder) > { > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > @@ -5428,7 +5207,7 @@ static bool icl_digital_port_connected(struct > intel_encoder *encoder) > if (intel_port_is_combophy(dev_priv, encoder->port)) > return icl_combo_port_connected(dev_priv, dig_port); > else if (intel_port_is_tc(dev_priv, encoder->port)) > - return icl_tc_port_connected(dev_priv, dig_port); > + return intel_tc_port_connected(dig_port); > else > MISSING_CASE(encoder->hpd_pin); > > diff --git a/drivers/gpu/drm/i915/intel_dp.h > b/drivers/gpu/drm/i915/intel_dp.h > index da70b1a41c83..657bbb1f5ed0 100644 > --- a/drivers/gpu/drm/i915/intel_dp.h > +++ b/drivers/gpu/drm/i915/intel_dp.h > @@ -112,8 +112,6 @@ bool intel_dp_get_colorimetry_status(struct > intel_dp *intel_dp); > int intel_dp_link_required(int pixel_clock, int bpp); > int intel_dp_max_data_rate(int max_link_clock, int max_lanes); > bool intel_digital_port_connected(struct intel_encoder *encoder); > -void icl_tc_phy_disconnect(struct drm_i915_private *dev_priv, > - struct intel_digital_port *dig_port); > > static inline unsigned int intel_dp_unused_lane_mask(int lane_count) > { > diff --git a/drivers/gpu/drm/i915/intel_tc.c > b/drivers/gpu/drm/i915/intel_tc.c > new file mode 100644 > index 000000000000..7a1b5870945f > --- /dev/null > +++ b/drivers/gpu/drm/i915/intel_tc.c > @@ -0,0 +1,230 @@ > +// SPDX-License-Identifier: MIT > +/* > + * Copyright © 2019 Intel Corporation > + */ > +#include "intel_display.h" > +#include "i915_drv.h" > +#include "intel_tc.h" > + > +static const char *tc_type_name(enum tc_port_type type) > +{ > + static const char * const names[] = { > + [TC_PORT_UNKNOWN] = "unknown", > + [TC_PORT_LEGACY] = "legacy", > + [TC_PORT_TYPEC] = "typec", > + [TC_PORT_TBT] = "tbt", > + }; > + > + if (WARN_ON(type >= ARRAY_SIZE(names))) > + type = TC_PORT_UNKNOWN; > + > + return names[type]; > +} > + > +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); > + enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port- > >base.port); > + intel_wakeref_t wakeref; > + u32 lane_info; > + > + if (tc_port == PORT_TC_NONE || dig_port->tc_type != > TC_PORT_TYPEC) > + return 4; > + > + lane_info = 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); > + > + switch (lane_info) { > + default: > + MISSING_CASE(lane_info); > + case 1: > + case 2: > + case 4: > + case 8: > + return 1; > + case 3: > + case 12: > + return 2; > + case 15: > + return 4; > + } > +} > + > +/* > + * This function implements the first part of the Connect Flow > described by our > + * specification, Gen11 TypeC Programming chapter. The rest of the > flow (reading > + * lanes, EDID, etc) is done as needed in the typical places. > + * > + * Unlike the other ports, type-C ports are not available to use as > soon as we > + * get a hotplug. The type-C PHYs can be shared between multiple > controllers: > + * display, USB, etc. As a result, handshaking through FIA is > required around > + * connect and disconnect to cleanly transfer ownership with the > controller and > + * set the type-C power state. > + * > + * We could opt to only do the connect flow when we actually try to > use the AUX > + * channels or do a modeset, then immediately run the disconnect > flow after > + * usage, but there are some implications on this for a dynamic > environment: > + * things may go away or change behind our backs. So for now our > driver is > + * always trying to acquire ownership of the controller as soon as > it gets an > + * interrupt (or polls state and sees a port is connected) and only > gives it > + * back when it sees a disconnect. Implementation of a more fine- > grained model > + * will require a lot of coordination with user space and thorough > testing for > + * the extra possible cases. > + */ > +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; > + > + if (dig_port->tc_type != TC_PORT_LEGACY && > + dig_port->tc_type != TC_PORT_TYPEC) > + return true; > + > + val = I915_READ(PORT_TX_DFLEXDPPMS); > + if (!(val & DP_PHY_MODE_STATUS_COMPLETED(tc_port))) { > + DRM_DEBUG_KMS("DP PHY for TC port %d not ready\n", > tc_port); > + 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); > + } > + > + /* > + * 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_type == TC_PORT_TYPEC && > + !(I915_READ(PORT_TX_DFLEXDPSP) & > TC_LIVE_STATE_TC(tc_port))) { > + DRM_DEBUG_KMS("TC PHY %d sudden disconnect.\n", > tc_port); > + icl_tc_phy_disconnect(dig_port); > + return false; > + } > + > + return true; > +} > + > +/* > + * See the comment at the connect function. This implements the > Disconnect > + * Flow. > + */ > +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); > + > + if (dig_port->tc_type == TC_PORT_UNKNOWN) > + return; > + > + /* > + * TBT disconnection flow is read the live status, what was > done in > + * caller. > + */ > + if (dig_port->tc_type == TC_PORT_TYPEC || > + dig_port->tc_type == 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); > + } > + > + DRM_DEBUG_KMS("Port %c TC type %s disconnected\n", > + port_name(dig_port->base.port), > + tc_type_name(dig_port->tc_type)); > + > + dig_port->tc_type = TC_PORT_UNKNOWN; > +} > + > +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) > +{ > + enum port port = intel_dig_port->base.port; > + enum tc_port_type old_type = intel_dig_port->tc_type; > + > + WARN_ON(is_legacy + is_typec + is_tbt != 1); > + > + if (is_legacy) > + intel_dig_port->tc_type = TC_PORT_LEGACY; > + else if (is_typec) > + intel_dig_port->tc_type = TC_PORT_TYPEC; > + else if (is_tbt) > + intel_dig_port->tc_type = TC_PORT_TBT; > + else > + return; > + > + /* Types are not supposed to be changed at runtime. */ > + WARN_ON(old_type != TC_PORT_UNKNOWN && > + old_type != intel_dig_port->tc_type); > + > + if (old_type != intel_dig_port->tc_type) > + DRM_DEBUG_KMS("Port %c has TC type %s\n", > port_name(port), > + tc_type_name(intel_dig_port->tc_type)); > +} > + > + > +/* > + * The type-C ports are different because even when they are > connected, they may > + * not be available/usable by the graphics driver: see the comment > on > + * icl_tc_phy_connect(). So in our driver instead of adding the > additional > + * concept of "usable" and make everything check for "connected and > usable" we > + * define a port as "connected" when it is not only connected, but > also when it > + * is usable by the rest of the driver. That maintains the old > assumption that > + * connected ports are usable, and avoids exposing to the users > objects they > + * can't really use. > + */ > +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("VBT incorrectly claims port %c is not TypeC > legacy\n", > + port_name(port)); > + dig_port->tc_legacy_port = true; > + } > + is_legacy = dig_port->tc_legacy_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) { > + icl_tc_phy_disconnect(dig_port); > + > + return false; > + } > + > + icl_update_tc_port_type(dev_priv, dig_port, is_legacy, > is_typec, > + is_tbt); > + > + if (!icl_tc_phy_connect(dig_port)) > + return false; > + > + return true; > +} > + > diff --git a/drivers/gpu/drm/i915/intel_tc.h > b/drivers/gpu/drm/i915/intel_tc.h > new file mode 100644 > index 000000000000..94c62ac4a162 > --- /dev/null > +++ b/drivers/gpu/drm/i915/intel_tc.h > @@ -0,0 +1,13 @@ > +#ifndef __INTEL_TC_H__ > +#define __INTEL_TC_H__ > + > +#include <linux/types.h> > + > +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); > +int intel_tc_port_fia_max_lane_count(struct intel_digital_port > *dig_port); > + > +#endif /* __INTEL_TC_H__ */
On Thu, Jun 06, 2019 at 11:42:46AM +0300, Jani Nikula wrote: >On Tue, 04 Jun 2019, Imre Deak <imre.deak@intel.com> wrote: >> Move the TypeC port handling functions to a new file for clarity. >> >> While at it: >> - s/icl_tc_port_connected()/intel_tc_port_connected()/ >> icl_tc_phy_disconnect(), will be unexported later. >> >> - s/intel_dp_get_fia_supported_lane_count()/intel_tc_port_fia_max_lane_count()/ >> It's used for HDMI legacy mode too. >> >> - Simplify function interfaces by passing only dig_port to them. >> >> No functional changes. > >Some nitpicks below. > >BR, >Jani. > > >> >> Cc: Animesh Manna <animesh.manna@intel.com> >> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> >> Cc: José Roberto de Souza <jose.souza@intel.com> >> Signed-off-by: Imre Deak <imre.deak@intel.com> >> --- >> drivers/gpu/drm/i915/Makefile | 3 +- >> drivers/gpu/drm/i915/Makefile.header-test | 1 + >> drivers/gpu/drm/i915/intel_ddi.c | 6 +- >> drivers/gpu/drm/i915/intel_dp.c | 227 +-------------------- >> drivers/gpu/drm/i915/intel_dp.h | 2 - >> drivers/gpu/drm/i915/intel_tc.c | 230 ++++++++++++++++++++++ >> drivers/gpu/drm/i915/intel_tc.h | 13 ++ >> 7 files changed, 252 insertions(+), 230 deletions(-) >> create mode 100644 drivers/gpu/drm/i915/intel_tc.c >> create mode 100644 drivers/gpu/drm/i915/intel_tc.h >> >> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile >> index c0a7b2994077..74c4d11d83eb 100644 >> --- a/drivers/gpu/drm/i915/Makefile >> +++ b/drivers/gpu/drm/i915/Makefile >> @@ -171,7 +171,8 @@ i915-y += intel_audio.o \ >> intel_psr.o \ >> intel_quirks.o \ >> intel_sideband.o \ >> - intel_sprite.o >> + intel_sprite.o \ >> + intel_tc.o >> i915-$(CONFIG_ACPI) += intel_acpi.o intel_opregion.o >> i915-$(CONFIG_DRM_FBDEV_EMULATION) += intel_fbdev.o >> >> diff --git a/drivers/gpu/drm/i915/Makefile.header-test b/drivers/gpu/drm/i915/Makefile.header-test >> index 6ef3b647ac65..e80e8e45b09c 100644 >> --- a/drivers/gpu/drm/i915/Makefile.header-test >> +++ b/drivers/gpu/drm/i915/Makefile.header-test >> @@ -58,6 +58,7 @@ header_test := \ >> intel_sdvo.h \ >> intel_sideband.h \ >> intel_sprite.h \ >> + intel_tc.h \ >> intel_tv.h \ >> intel_uncore.h \ >> intel_vdsc.h \ >> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c >> index 350eaf54f01f..5a1c98438375 100644 >> --- a/drivers/gpu/drm/i915/intel_ddi.c >> +++ b/drivers/gpu/drm/i915/intel_ddi.c >> @@ -45,6 +45,7 @@ >> #include "intel_lspcon.h" >> #include "intel_panel.h" >> #include "intel_psr.h" >> +#include "intel_tc.h" >> #include "intel_vdsc.h" >> >> struct ddi_buf_trans { >> @@ -3904,7 +3905,6 @@ static int intel_ddi_compute_config(struct intel_encoder *encoder, >> static void intel_ddi_encoder_suspend(struct intel_encoder *encoder) >> { >> struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base); >> - struct drm_i915_private *i915 = to_i915(encoder->base.dev); >> >> intel_dp_encoder_suspend(encoder); >> >> @@ -3914,7 +3914,7 @@ static void intel_ddi_encoder_suspend(struct intel_encoder *encoder) >> * even if the sink has disappeared while being suspended. >> */ >> if (dig_port->tc_legacy_port) >> - icl_tc_phy_disconnect(i915, dig_port); >> + icl_tc_phy_disconnect(dig_port); >> } >> >> static void intel_ddi_encoder_reset(struct drm_encoder *drm_encoder) >> @@ -3936,7 +3936,7 @@ static void intel_ddi_encoder_destroy(struct drm_encoder *encoder) >> intel_dp_encoder_flush_work(encoder); >> >> if (intel_port_is_tc(i915, dig_port->base.port)) >> - icl_tc_phy_disconnect(i915, dig_port); >> + icl_tc_phy_disconnect(dig_port); >> >> drm_encoder_cleanup(encoder); >> kfree(dig_port); >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c >> index 24b56b2a76c8..b69310bd9914 100644 >> --- a/drivers/gpu/drm/i915/intel_dp.c >> +++ b/drivers/gpu/drm/i915/intel_dp.c >> @@ -62,6 +62,7 @@ >> #include "intel_panel.h" >> #include "intel_psr.h" >> #include "intel_sideband.h" >> +#include "intel_tc.h" >> #include "intel_vdsc.h" >> >> #define DP_DPRX_ESI_LEN 14 >> @@ -211,46 +212,13 @@ static int intel_dp_max_common_rate(struct intel_dp *intel_dp) >> return intel_dp->common_rates[intel_dp->num_common_rates - 1]; >> } >> >> -static int intel_dp_get_fia_supported_lane_count(struct intel_dp *intel_dp) >> -{ >> - struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp); >> - 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); >> - intel_wakeref_t wakeref; >> - u32 lane_info; >> - >> - if (tc_port == PORT_TC_NONE || dig_port->tc_type != TC_PORT_TYPEC) >> - return 4; >> - >> - lane_info = 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); >> - >> - switch (lane_info) { >> - default: >> - MISSING_CASE(lane_info); >> - case 1: >> - case 2: >> - case 4: >> - case 8: >> - return 1; >> - case 3: >> - case 12: >> - return 2; >> - case 15: >> - return 4; >> - } >> -} >> - >> /* Theoretical max between source and sink */ >> static int intel_dp_max_common_lane_count(struct intel_dp *intel_dp) >> { >> struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); >> int source_max = intel_dig_port->max_lanes; >> int sink_max = drm_dp_max_lane_count(intel_dp->dpcd); >> - int fia_max = intel_dp_get_fia_supported_lane_count(intel_dp); >> + int fia_max = intel_tc_port_fia_max_lane_count(intel_dig_port); >> >> return min3(source_max, sink_max, fia_max); >> } >> @@ -5231,195 +5199,6 @@ static bool icl_combo_port_connected(struct drm_i915_private *dev_priv, >> return I915_READ(SDEISR) & SDE_DDI_HOTPLUG_ICP(port); >> } >> >> -static const char *tc_type_name(enum tc_port_type type) >> -{ >> - static const char * const names[] = { >> - [TC_PORT_UNKNOWN] = "unknown", >> - [TC_PORT_LEGACY] = "legacy", >> - [TC_PORT_TYPEC] = "typec", >> - [TC_PORT_TBT] = "tbt", >> - }; >> - >> - if (WARN_ON(type >= ARRAY_SIZE(names))) >> - type = TC_PORT_UNKNOWN; >> - >> - return names[type]; >> -} >> - >> -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) >> -{ >> - enum port port = intel_dig_port->base.port; >> - enum tc_port_type old_type = intel_dig_port->tc_type; >> - >> - WARN_ON(is_legacy + is_typec + is_tbt != 1); >> - >> - if (is_legacy) >> - intel_dig_port->tc_type = TC_PORT_LEGACY; >> - else if (is_typec) >> - intel_dig_port->tc_type = TC_PORT_TYPEC; >> - else if (is_tbt) >> - intel_dig_port->tc_type = TC_PORT_TBT; >> - else >> - return; >> - >> - /* Types are not supposed to be changed at runtime. */ >> - WARN_ON(old_type != TC_PORT_UNKNOWN && >> - old_type != intel_dig_port->tc_type); >> - >> - if (old_type != intel_dig_port->tc_type) >> - DRM_DEBUG_KMS("Port %c has TC type %s\n", port_name(port), >> - tc_type_name(intel_dig_port->tc_type)); >> -} >> - >> -/* >> - * This function implements the first part of the Connect Flow described by our >> - * specification, Gen11 TypeC Programming chapter. The rest of the flow (reading >> - * lanes, EDID, etc) is done as needed in the typical places. >> - * >> - * Unlike the other ports, type-C ports are not available to use as soon as we >> - * get a hotplug. The type-C PHYs can be shared between multiple controllers: >> - * display, USB, etc. As a result, handshaking through FIA is required around >> - * connect and disconnect to cleanly transfer ownership with the controller and >> - * set the type-C power state. >> - * >> - * We could opt to only do the connect flow when we actually try to use the AUX >> - * channels or do a modeset, then immediately run the disconnect flow after >> - * usage, but there are some implications on this for a dynamic environment: >> - * things may go away or change behind our backs. So for now our driver is >> - * always trying to acquire ownership of the controller as soon as it gets an >> - * interrupt (or polls state and sees a port is connected) and only gives it >> - * back when it sees a disconnect. Implementation of a more fine-grained model >> - * will require a lot of coordination with user space and thorough testing for >> - * the extra possible cases. >> - */ >> -static bool icl_tc_phy_connect(struct drm_i915_private *dev_priv, >> - struct intel_digital_port *dig_port) >> -{ >> - enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port->base.port); >> - u32 val; >> - >> - if (dig_port->tc_type != TC_PORT_LEGACY && >> - dig_port->tc_type != TC_PORT_TYPEC) >> - return true; >> - >> - val = I915_READ(PORT_TX_DFLEXDPPMS); >> - if (!(val & DP_PHY_MODE_STATUS_COMPLETED(tc_port))) { >> - DRM_DEBUG_KMS("DP PHY for TC port %d not ready\n", tc_port); >> - 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); >> - } >> - >> - /* >> - * 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_type == TC_PORT_TYPEC && >> - !(I915_READ(PORT_TX_DFLEXDPSP) & TC_LIVE_STATE_TC(tc_port))) { >> - DRM_DEBUG_KMS("TC PHY %d sudden disconnect.\n", tc_port); >> - icl_tc_phy_disconnect(dev_priv, dig_port); >> - return false; >> - } >> - >> - return true; >> -} >> - >> -/* >> - * See the comment at the connect function. This implements the Disconnect >> - * Flow. >> - */ >> -void icl_tc_phy_disconnect(struct drm_i915_private *dev_priv, >> - struct intel_digital_port *dig_port) >> -{ >> - enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port->base.port); >> - >> - if (dig_port->tc_type == TC_PORT_UNKNOWN) >> - return; >> - >> - /* >> - * TBT disconnection flow is read the live status, what was done in >> - * caller. >> - */ >> - if (dig_port->tc_type == TC_PORT_TYPEC || >> - dig_port->tc_type == 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); >> - } >> - >> - DRM_DEBUG_KMS("Port %c TC type %s disconnected\n", >> - port_name(dig_port->base.port), >> - tc_type_name(dig_port->tc_type)); >> - >> - dig_port->tc_type = TC_PORT_UNKNOWN; >> -} >> - >> -/* >> - * The type-C ports are different because even when they are connected, they may >> - * not be available/usable by the graphics driver: see the comment on >> - * icl_tc_phy_connect(). So in our driver instead of adding the additional >> - * concept of "usable" and make everything check for "connected and usable" we >> - * define a port as "connected" when it is not only connected, but also when it >> - * is usable by the rest of the driver. That maintains the old assumption that >> - * connected ports are usable, and avoids exposing to the users objects they >> - * can't really use. >> - */ >> -static bool icl_tc_port_connected(struct drm_i915_private *dev_priv, >> - struct intel_digital_port *intel_dig_port) >> -{ >> - enum port port = intel_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 (!intel_dig_port->tc_legacy_port && >> - I915_READ(SDEISR) & SDE_TC_HOTPLUG_ICP(tc_port)) { >> - DRM_ERROR("VBT incorrectly claims port %c is not TypeC legacy\n", >> - port_name(port)); >> - intel_dig_port->tc_legacy_port = true; >> - } >> - is_legacy = intel_dig_port->tc_legacy_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) { >> - icl_tc_phy_disconnect(dev_priv, intel_dig_port); >> - >> - return false; >> - } >> - >> - icl_update_tc_port_type(dev_priv, intel_dig_port, is_legacy, is_typec, >> - is_tbt); >> - >> - if (!icl_tc_phy_connect(dev_priv, intel_dig_port)) >> - return false; >> - >> - return true; >> -} >> - >> static bool icl_digital_port_connected(struct intel_encoder *encoder) >> { >> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); >> @@ -5428,7 +5207,7 @@ static bool icl_digital_port_connected(struct intel_encoder *encoder) >> if (intel_port_is_combophy(dev_priv, encoder->port)) >> return icl_combo_port_connected(dev_priv, dig_port); >> else if (intel_port_is_tc(dev_priv, encoder->port)) >> - return icl_tc_port_connected(dev_priv, dig_port); >> + return intel_tc_port_connected(dig_port); >> else >> MISSING_CASE(encoder->hpd_pin); >> >> diff --git a/drivers/gpu/drm/i915/intel_dp.h b/drivers/gpu/drm/i915/intel_dp.h >> index da70b1a41c83..657bbb1f5ed0 100644 >> --- a/drivers/gpu/drm/i915/intel_dp.h >> +++ b/drivers/gpu/drm/i915/intel_dp.h >> @@ -112,8 +112,6 @@ bool intel_dp_get_colorimetry_status(struct intel_dp *intel_dp); >> int intel_dp_link_required(int pixel_clock, int bpp); >> int intel_dp_max_data_rate(int max_link_clock, int max_lanes); >> bool intel_digital_port_connected(struct intel_encoder *encoder); >> -void icl_tc_phy_disconnect(struct drm_i915_private *dev_priv, >> - struct intel_digital_port *dig_port); >> >> static inline unsigned int intel_dp_unused_lane_mask(int lane_count) >> { >> diff --git a/drivers/gpu/drm/i915/intel_tc.c b/drivers/gpu/drm/i915/intel_tc.c >> new file mode 100644 >> index 000000000000..7a1b5870945f >> --- /dev/null >> +++ b/drivers/gpu/drm/i915/intel_tc.c >> @@ -0,0 +1,230 @@ >> +// SPDX-License-Identifier: MIT >> +/* >> + * Copyright © 2019 Intel Corporation >> + */ > >From the Nitpicks Department, please add a blank line here. > >> +#include "intel_display.h" >> +#include "i915_drv.h" >> +#include "intel_tc.h" > >And sort the includes. humn... In some other projects I use the convention of always having the correspondent header to be included first and the rest sorted. Here it would be like: #include "intel_tc.h" #include "intel_display.h" #include "intel_drv.h" This way we guarantee that intel_tc.h header is self-contained: anything it needs, it includes or declares. If we adopted such convention I don't see why we would still need Makefile.header-test for example. Lucas De Marchi > >> + >> +static const char *tc_type_name(enum tc_port_type type) >> +{ >> + static const char * const names[] = { >> + [TC_PORT_UNKNOWN] = "unknown", >> + [TC_PORT_LEGACY] = "legacy", >> + [TC_PORT_TYPEC] = "typec", >> + [TC_PORT_TBT] = "tbt", >> + }; >> + >> + if (WARN_ON(type >= ARRAY_SIZE(names))) >> + type = TC_PORT_UNKNOWN; >> + >> + return names[type]; >> +} >> + >> +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); >> + enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port->base.port); >> + intel_wakeref_t wakeref; >> + u32 lane_info; >> + >> + if (tc_port == PORT_TC_NONE || dig_port->tc_type != TC_PORT_TYPEC) >> + return 4; >> + >> + lane_info = 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); >> + >> + switch (lane_info) { >> + default: >> + MISSING_CASE(lane_info); >> + case 1: >> + case 2: >> + case 4: >> + case 8: >> + return 1; >> + case 3: >> + case 12: >> + return 2; >> + case 15: >> + return 4; >> + } >> +} >> + >> +/* >> + * This function implements the first part of the Connect Flow described by our >> + * specification, Gen11 TypeC Programming chapter. The rest of the flow (reading >> + * lanes, EDID, etc) is done as needed in the typical places. >> + * >> + * Unlike the other ports, type-C ports are not available to use as soon as we >> + * get a hotplug. The type-C PHYs can be shared between multiple controllers: >> + * display, USB, etc. As a result, handshaking through FIA is required around >> + * connect and disconnect to cleanly transfer ownership with the controller and >> + * set the type-C power state. >> + * >> + * We could opt to only do the connect flow when we actually try to use the AUX >> + * channels or do a modeset, then immediately run the disconnect flow after >> + * usage, but there are some implications on this for a dynamic environment: >> + * things may go away or change behind our backs. So for now our driver is >> + * always trying to acquire ownership of the controller as soon as it gets an >> + * interrupt (or polls state and sees a port is connected) and only gives it >> + * back when it sees a disconnect. Implementation of a more fine-grained model >> + * will require a lot of coordination with user space and thorough testing for >> + * the extra possible cases. >> + */ >> +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; >> + >> + if (dig_port->tc_type != TC_PORT_LEGACY && >> + dig_port->tc_type != TC_PORT_TYPEC) >> + return true; >> + >> + val = I915_READ(PORT_TX_DFLEXDPPMS); >> + if (!(val & DP_PHY_MODE_STATUS_COMPLETED(tc_port))) { >> + DRM_DEBUG_KMS("DP PHY for TC port %d not ready\n", tc_port); >> + 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); >> + } >> + >> + /* >> + * 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_type == TC_PORT_TYPEC && >> + !(I915_READ(PORT_TX_DFLEXDPSP) & TC_LIVE_STATE_TC(tc_port))) { >> + DRM_DEBUG_KMS("TC PHY %d sudden disconnect.\n", tc_port); >> + icl_tc_phy_disconnect(dig_port); >> + return false; >> + } >> + >> + return true; >> +} >> + >> +/* >> + * See the comment at the connect function. This implements the Disconnect >> + * Flow. >> + */ >> +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); >> + >> + if (dig_port->tc_type == TC_PORT_UNKNOWN) >> + return; >> + >> + /* >> + * TBT disconnection flow is read the live status, what was done in >> + * caller. >> + */ >> + if (dig_port->tc_type == TC_PORT_TYPEC || >> + dig_port->tc_type == 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); >> + } >> + >> + DRM_DEBUG_KMS("Port %c TC type %s disconnected\n", >> + port_name(dig_port->base.port), >> + tc_type_name(dig_port->tc_type)); >> + >> + dig_port->tc_type = TC_PORT_UNKNOWN; >> +} >> + >> +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) >> +{ >> + enum port port = intel_dig_port->base.port; >> + enum tc_port_type old_type = intel_dig_port->tc_type; >> + >> + WARN_ON(is_legacy + is_typec + is_tbt != 1); >> + >> + if (is_legacy) >> + intel_dig_port->tc_type = TC_PORT_LEGACY; >> + else if (is_typec) >> + intel_dig_port->tc_type = TC_PORT_TYPEC; >> + else if (is_tbt) >> + intel_dig_port->tc_type = TC_PORT_TBT; >> + else >> + return; >> + >> + /* Types are not supposed to be changed at runtime. */ >> + WARN_ON(old_type != TC_PORT_UNKNOWN && >> + old_type != intel_dig_port->tc_type); >> + >> + if (old_type != intel_dig_port->tc_type) >> + DRM_DEBUG_KMS("Port %c has TC type %s\n", port_name(port), >> + tc_type_name(intel_dig_port->tc_type)); >> +} >> + >> + >> +/* >> + * The type-C ports are different because even when they are connected, they may >> + * not be available/usable by the graphics driver: see the comment on >> + * icl_tc_phy_connect(). So in our driver instead of adding the additional >> + * concept of "usable" and make everything check for "connected and usable" we >> + * define a port as "connected" when it is not only connected, but also when it >> + * is usable by the rest of the driver. That maintains the old assumption that >> + * connected ports are usable, and avoids exposing to the users objects they >> + * can't really use. >> + */ >> +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("VBT incorrectly claims port %c is not TypeC legacy\n", >> + port_name(port)); >> + dig_port->tc_legacy_port = true; >> + } >> + is_legacy = dig_port->tc_legacy_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) { >> + icl_tc_phy_disconnect(dig_port); >> + >> + return false; >> + } >> + >> + icl_update_tc_port_type(dev_priv, dig_port, is_legacy, is_typec, >> + is_tbt); >> + >> + if (!icl_tc_phy_connect(dig_port)) >> + return false; >> + >> + return true; >> +} >> + >> diff --git a/drivers/gpu/drm/i915/intel_tc.h b/drivers/gpu/drm/i915/intel_tc.h >> new file mode 100644 >> index 000000000000..94c62ac4a162 >> --- /dev/null >> +++ b/drivers/gpu/drm/i915/intel_tc.h >> @@ -0,0 +1,13 @@ > >SPDX header here please. > >> +#ifndef __INTEL_TC_H__ >> +#define __INTEL_TC_H__ >> + >> +#include <linux/types.h> >> + >> +struct intel_digital_port; >> + >> +void icl_tc_phy_disconnect(struct intel_digital_port *dig_port); > >I'd like to see this named intel_tc_* in the future. > >BR, >Jani. > > >> + >> +bool intel_tc_port_connected(struct intel_digital_port *dig_port); >> +int intel_tc_port_fia_max_lane_count(struct intel_digital_port *dig_port); >> + >> +#endif /* __INTEL_TC_H__ */ > >-- >Jani Nikula, Intel Open Source Graphics Center >_______________________________________________ >Intel-gfx mailing list >Intel-gfx@lists.freedesktop.org >https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, 10 Jun 2019, Lucas De Marchi <lucas.de.marchi@gmail.com> wrote: > On Thu, Jun 06, 2019 at 11:42:46AM +0300, Jani Nikula wrote: >>On Tue, 04 Jun 2019, Imre Deak <imre.deak@intel.com> wrote: >>From the Nitpicks Department, please add a blank line here. >> >>> +#include "intel_display.h" >>> +#include "i915_drv.h" >>> +#include "intel_tc.h" >> >>And sort the includes. > > humn... In some other projects I use the convention of always having the > correspondent header to be included first and the rest sorted. Here it > would be like: > > #include "intel_tc.h" > > #include "intel_display.h" > #include "intel_drv.h" > > This way we guarantee that intel_tc.h header is self-contained: anything > it needs, it includes or declares. If we adopted such convention I don't > see why we would still need Makefile.header-test for example. I think the long standing convention is to have <asm>, <linux>, and <drm> headers included before "local" headers. Eventually the version of header tests that I've submitted to kbuild upstream will obsolete the Makefile.header-test files, and I expect us to have simply header-test-y := $(notdir $(wildcard $(src)/*.h)) in there, testing everything. BR, Jani.
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index c0a7b2994077..74c4d11d83eb 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -171,7 +171,8 @@ i915-y += intel_audio.o \ intel_psr.o \ intel_quirks.o \ intel_sideband.o \ - intel_sprite.o + intel_sprite.o \ + intel_tc.o i915-$(CONFIG_ACPI) += intel_acpi.o intel_opregion.o i915-$(CONFIG_DRM_FBDEV_EMULATION) += intel_fbdev.o diff --git a/drivers/gpu/drm/i915/Makefile.header-test b/drivers/gpu/drm/i915/Makefile.header-test index 6ef3b647ac65..e80e8e45b09c 100644 --- a/drivers/gpu/drm/i915/Makefile.header-test +++ b/drivers/gpu/drm/i915/Makefile.header-test @@ -58,6 +58,7 @@ header_test := \ intel_sdvo.h \ intel_sideband.h \ intel_sprite.h \ + intel_tc.h \ intel_tv.h \ intel_uncore.h \ intel_vdsc.h \ diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 350eaf54f01f..5a1c98438375 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -45,6 +45,7 @@ #include "intel_lspcon.h" #include "intel_panel.h" #include "intel_psr.h" +#include "intel_tc.h" #include "intel_vdsc.h" struct ddi_buf_trans { @@ -3904,7 +3905,6 @@ static int intel_ddi_compute_config(struct intel_encoder *encoder, static void intel_ddi_encoder_suspend(struct intel_encoder *encoder) { struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base); - struct drm_i915_private *i915 = to_i915(encoder->base.dev); intel_dp_encoder_suspend(encoder); @@ -3914,7 +3914,7 @@ static void intel_ddi_encoder_suspend(struct intel_encoder *encoder) * even if the sink has disappeared while being suspended. */ if (dig_port->tc_legacy_port) - icl_tc_phy_disconnect(i915, dig_port); + icl_tc_phy_disconnect(dig_port); } static void intel_ddi_encoder_reset(struct drm_encoder *drm_encoder) @@ -3936,7 +3936,7 @@ static void intel_ddi_encoder_destroy(struct drm_encoder *encoder) intel_dp_encoder_flush_work(encoder); if (intel_port_is_tc(i915, dig_port->base.port)) - icl_tc_phy_disconnect(i915, dig_port); + icl_tc_phy_disconnect(dig_port); drm_encoder_cleanup(encoder); kfree(dig_port); diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 24b56b2a76c8..b69310bd9914 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -62,6 +62,7 @@ #include "intel_panel.h" #include "intel_psr.h" #include "intel_sideband.h" +#include "intel_tc.h" #include "intel_vdsc.h" #define DP_DPRX_ESI_LEN 14 @@ -211,46 +212,13 @@ static int intel_dp_max_common_rate(struct intel_dp *intel_dp) return intel_dp->common_rates[intel_dp->num_common_rates - 1]; } -static int intel_dp_get_fia_supported_lane_count(struct intel_dp *intel_dp) -{ - struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp); - 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); - intel_wakeref_t wakeref; - u32 lane_info; - - if (tc_port == PORT_TC_NONE || dig_port->tc_type != TC_PORT_TYPEC) - return 4; - - lane_info = 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); - - switch (lane_info) { - default: - MISSING_CASE(lane_info); - case 1: - case 2: - case 4: - case 8: - return 1; - case 3: - case 12: - return 2; - case 15: - return 4; - } -} - /* Theoretical max between source and sink */ static int intel_dp_max_common_lane_count(struct intel_dp *intel_dp) { struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); int source_max = intel_dig_port->max_lanes; int sink_max = drm_dp_max_lane_count(intel_dp->dpcd); - int fia_max = intel_dp_get_fia_supported_lane_count(intel_dp); + int fia_max = intel_tc_port_fia_max_lane_count(intel_dig_port); return min3(source_max, sink_max, fia_max); } @@ -5231,195 +5199,6 @@ static bool icl_combo_port_connected(struct drm_i915_private *dev_priv, return I915_READ(SDEISR) & SDE_DDI_HOTPLUG_ICP(port); } -static const char *tc_type_name(enum tc_port_type type) -{ - static const char * const names[] = { - [TC_PORT_UNKNOWN] = "unknown", - [TC_PORT_LEGACY] = "legacy", - [TC_PORT_TYPEC] = "typec", - [TC_PORT_TBT] = "tbt", - }; - - if (WARN_ON(type >= ARRAY_SIZE(names))) - type = TC_PORT_UNKNOWN; - - return names[type]; -} - -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) -{ - enum port port = intel_dig_port->base.port; - enum tc_port_type old_type = intel_dig_port->tc_type; - - WARN_ON(is_legacy + is_typec + is_tbt != 1); - - if (is_legacy) - intel_dig_port->tc_type = TC_PORT_LEGACY; - else if (is_typec) - intel_dig_port->tc_type = TC_PORT_TYPEC; - else if (is_tbt) - intel_dig_port->tc_type = TC_PORT_TBT; - else - return; - - /* Types are not supposed to be changed at runtime. */ - WARN_ON(old_type != TC_PORT_UNKNOWN && - old_type != intel_dig_port->tc_type); - - if (old_type != intel_dig_port->tc_type) - DRM_DEBUG_KMS("Port %c has TC type %s\n", port_name(port), - tc_type_name(intel_dig_port->tc_type)); -} - -/* - * This function implements the first part of the Connect Flow described by our - * specification, Gen11 TypeC Programming chapter. The rest of the flow (reading - * lanes, EDID, etc) is done as needed in the typical places. - * - * Unlike the other ports, type-C ports are not available to use as soon as we - * get a hotplug. The type-C PHYs can be shared between multiple controllers: - * display, USB, etc. As a result, handshaking through FIA is required around - * connect and disconnect to cleanly transfer ownership with the controller and - * set the type-C power state. - * - * We could opt to only do the connect flow when we actually try to use the AUX - * channels or do a modeset, then immediately run the disconnect flow after - * usage, but there are some implications on this for a dynamic environment: - * things may go away or change behind our backs. So for now our driver is - * always trying to acquire ownership of the controller as soon as it gets an - * interrupt (or polls state and sees a port is connected) and only gives it - * back when it sees a disconnect. Implementation of a more fine-grained model - * will require a lot of coordination with user space and thorough testing for - * the extra possible cases. - */ -static bool icl_tc_phy_connect(struct drm_i915_private *dev_priv, - struct intel_digital_port *dig_port) -{ - enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port->base.port); - u32 val; - - if (dig_port->tc_type != TC_PORT_LEGACY && - dig_port->tc_type != TC_PORT_TYPEC) - return true; - - val = I915_READ(PORT_TX_DFLEXDPPMS); - if (!(val & DP_PHY_MODE_STATUS_COMPLETED(tc_port))) { - DRM_DEBUG_KMS("DP PHY for TC port %d not ready\n", tc_port); - 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); - } - - /* - * 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_type == TC_PORT_TYPEC && - !(I915_READ(PORT_TX_DFLEXDPSP) & TC_LIVE_STATE_TC(tc_port))) { - DRM_DEBUG_KMS("TC PHY %d sudden disconnect.\n", tc_port); - icl_tc_phy_disconnect(dev_priv, dig_port); - return false; - } - - return true; -} - -/* - * See the comment at the connect function. This implements the Disconnect - * Flow. - */ -void icl_tc_phy_disconnect(struct drm_i915_private *dev_priv, - struct intel_digital_port *dig_port) -{ - enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port->base.port); - - if (dig_port->tc_type == TC_PORT_UNKNOWN) - return; - - /* - * TBT disconnection flow is read the live status, what was done in - * caller. - */ - if (dig_port->tc_type == TC_PORT_TYPEC || - dig_port->tc_type == 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); - } - - DRM_DEBUG_KMS("Port %c TC type %s disconnected\n", - port_name(dig_port->base.port), - tc_type_name(dig_port->tc_type)); - - dig_port->tc_type = TC_PORT_UNKNOWN; -} - -/* - * The type-C ports are different because even when they are connected, they may - * not be available/usable by the graphics driver: see the comment on - * icl_tc_phy_connect(). So in our driver instead of adding the additional - * concept of "usable" and make everything check for "connected and usable" we - * define a port as "connected" when it is not only connected, but also when it - * is usable by the rest of the driver. That maintains the old assumption that - * connected ports are usable, and avoids exposing to the users objects they - * can't really use. - */ -static bool icl_tc_port_connected(struct drm_i915_private *dev_priv, - struct intel_digital_port *intel_dig_port) -{ - enum port port = intel_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 (!intel_dig_port->tc_legacy_port && - I915_READ(SDEISR) & SDE_TC_HOTPLUG_ICP(tc_port)) { - DRM_ERROR("VBT incorrectly claims port %c is not TypeC legacy\n", - port_name(port)); - intel_dig_port->tc_legacy_port = true; - } - is_legacy = intel_dig_port->tc_legacy_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) { - icl_tc_phy_disconnect(dev_priv, intel_dig_port); - - return false; - } - - icl_update_tc_port_type(dev_priv, intel_dig_port, is_legacy, is_typec, - is_tbt); - - if (!icl_tc_phy_connect(dev_priv, intel_dig_port)) - return false; - - return true; -} - static bool icl_digital_port_connected(struct intel_encoder *encoder) { struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); @@ -5428,7 +5207,7 @@ static bool icl_digital_port_connected(struct intel_encoder *encoder) if (intel_port_is_combophy(dev_priv, encoder->port)) return icl_combo_port_connected(dev_priv, dig_port); else if (intel_port_is_tc(dev_priv, encoder->port)) - return icl_tc_port_connected(dev_priv, dig_port); + return intel_tc_port_connected(dig_port); else MISSING_CASE(encoder->hpd_pin); diff --git a/drivers/gpu/drm/i915/intel_dp.h b/drivers/gpu/drm/i915/intel_dp.h index da70b1a41c83..657bbb1f5ed0 100644 --- a/drivers/gpu/drm/i915/intel_dp.h +++ b/drivers/gpu/drm/i915/intel_dp.h @@ -112,8 +112,6 @@ bool intel_dp_get_colorimetry_status(struct intel_dp *intel_dp); int intel_dp_link_required(int pixel_clock, int bpp); int intel_dp_max_data_rate(int max_link_clock, int max_lanes); bool intel_digital_port_connected(struct intel_encoder *encoder); -void icl_tc_phy_disconnect(struct drm_i915_private *dev_priv, - struct intel_digital_port *dig_port); static inline unsigned int intel_dp_unused_lane_mask(int lane_count) { diff --git a/drivers/gpu/drm/i915/intel_tc.c b/drivers/gpu/drm/i915/intel_tc.c new file mode 100644 index 000000000000..7a1b5870945f --- /dev/null +++ b/drivers/gpu/drm/i915/intel_tc.c @@ -0,0 +1,230 @@ +// SPDX-License-Identifier: MIT +/* + * Copyright © 2019 Intel Corporation + */ +#include "intel_display.h" +#include "i915_drv.h" +#include "intel_tc.h" + +static const char *tc_type_name(enum tc_port_type type) +{ + static const char * const names[] = { + [TC_PORT_UNKNOWN] = "unknown", + [TC_PORT_LEGACY] = "legacy", + [TC_PORT_TYPEC] = "typec", + [TC_PORT_TBT] = "tbt", + }; + + if (WARN_ON(type >= ARRAY_SIZE(names))) + type = TC_PORT_UNKNOWN; + + return names[type]; +} + +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); + enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port->base.port); + intel_wakeref_t wakeref; + u32 lane_info; + + if (tc_port == PORT_TC_NONE || dig_port->tc_type != TC_PORT_TYPEC) + return 4; + + lane_info = 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); + + switch (lane_info) { + default: + MISSING_CASE(lane_info); + case 1: + case 2: + case 4: + case 8: + return 1; + case 3: + case 12: + return 2; + case 15: + return 4; + } +} + +/* + * This function implements the first part of the Connect Flow described by our + * specification, Gen11 TypeC Programming chapter. The rest of the flow (reading + * lanes, EDID, etc) is done as needed in the typical places. + * + * Unlike the other ports, type-C ports are not available to use as soon as we + * get a hotplug. The type-C PHYs can be shared between multiple controllers: + * display, USB, etc. As a result, handshaking through FIA is required around + * connect and disconnect to cleanly transfer ownership with the controller and + * set the type-C power state. + * + * We could opt to only do the connect flow when we actually try to use the AUX + * channels or do a modeset, then immediately run the disconnect flow after + * usage, but there are some implications on this for a dynamic environment: + * things may go away or change behind our backs. So for now our driver is + * always trying to acquire ownership of the controller as soon as it gets an + * interrupt (or polls state and sees a port is connected) and only gives it + * back when it sees a disconnect. Implementation of a more fine-grained model + * will require a lot of coordination with user space and thorough testing for + * the extra possible cases. + */ +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; + + if (dig_port->tc_type != TC_PORT_LEGACY && + dig_port->tc_type != TC_PORT_TYPEC) + return true; + + val = I915_READ(PORT_TX_DFLEXDPPMS); + if (!(val & DP_PHY_MODE_STATUS_COMPLETED(tc_port))) { + DRM_DEBUG_KMS("DP PHY for TC port %d not ready\n", tc_port); + 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); + } + + /* + * 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_type == TC_PORT_TYPEC && + !(I915_READ(PORT_TX_DFLEXDPSP) & TC_LIVE_STATE_TC(tc_port))) { + DRM_DEBUG_KMS("TC PHY %d sudden disconnect.\n", tc_port); + icl_tc_phy_disconnect(dig_port); + return false; + } + + return true; +} + +/* + * See the comment at the connect function. This implements the Disconnect + * Flow. + */ +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); + + if (dig_port->tc_type == TC_PORT_UNKNOWN) + return; + + /* + * TBT disconnection flow is read the live status, what was done in + * caller. + */ + if (dig_port->tc_type == TC_PORT_TYPEC || + dig_port->tc_type == 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); + } + + DRM_DEBUG_KMS("Port %c TC type %s disconnected\n", + port_name(dig_port->base.port), + tc_type_name(dig_port->tc_type)); + + dig_port->tc_type = TC_PORT_UNKNOWN; +} + +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) +{ + enum port port = intel_dig_port->base.port; + enum tc_port_type old_type = intel_dig_port->tc_type; + + WARN_ON(is_legacy + is_typec + is_tbt != 1); + + if (is_legacy) + intel_dig_port->tc_type = TC_PORT_LEGACY; + else if (is_typec) + intel_dig_port->tc_type = TC_PORT_TYPEC; + else if (is_tbt) + intel_dig_port->tc_type = TC_PORT_TBT; + else + return; + + /* Types are not supposed to be changed at runtime. */ + WARN_ON(old_type != TC_PORT_UNKNOWN && + old_type != intel_dig_port->tc_type); + + if (old_type != intel_dig_port->tc_type) + DRM_DEBUG_KMS("Port %c has TC type %s\n", port_name(port), + tc_type_name(intel_dig_port->tc_type)); +} + + +/* + * The type-C ports are different because even when they are connected, they may + * not be available/usable by the graphics driver: see the comment on + * icl_tc_phy_connect(). So in our driver instead of adding the additional + * concept of "usable" and make everything check for "connected and usable" we + * define a port as "connected" when it is not only connected, but also when it + * is usable by the rest of the driver. That maintains the old assumption that + * connected ports are usable, and avoids exposing to the users objects they + * can't really use. + */ +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("VBT incorrectly claims port %c is not TypeC legacy\n", + port_name(port)); + dig_port->tc_legacy_port = true; + } + is_legacy = dig_port->tc_legacy_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) { + icl_tc_phy_disconnect(dig_port); + + return false; + } + + icl_update_tc_port_type(dev_priv, dig_port, is_legacy, is_typec, + is_tbt); + + if (!icl_tc_phy_connect(dig_port)) + return false; + + return true; +} + diff --git a/drivers/gpu/drm/i915/intel_tc.h b/drivers/gpu/drm/i915/intel_tc.h new file mode 100644 index 000000000000..94c62ac4a162 --- /dev/null +++ b/drivers/gpu/drm/i915/intel_tc.h @@ -0,0 +1,13 @@ +#ifndef __INTEL_TC_H__ +#define __INTEL_TC_H__ + +#include <linux/types.h> + +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); +int intel_tc_port_fia_max_lane_count(struct intel_digital_port *dig_port); + +#endif /* __INTEL_TC_H__ */
Move the TypeC port handling functions to a new file for clarity. While at it: - s/icl_tc_port_connected()/intel_tc_port_connected()/ icl_tc_phy_disconnect(), will be unexported later. - s/intel_dp_get_fia_supported_lane_count()/intel_tc_port_fia_max_lane_count()/ It's used for HDMI legacy mode too. - Simplify function interfaces by passing only dig_port to them. No functional changes. Cc: Animesh Manna <animesh.manna@intel.com> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Cc: José Roberto de Souza <jose.souza@intel.com> Signed-off-by: Imre Deak <imre.deak@intel.com> --- drivers/gpu/drm/i915/Makefile | 3 +- drivers/gpu/drm/i915/Makefile.header-test | 1 + drivers/gpu/drm/i915/intel_ddi.c | 6 +- drivers/gpu/drm/i915/intel_dp.c | 227 +-------------------- drivers/gpu/drm/i915/intel_dp.h | 2 - drivers/gpu/drm/i915/intel_tc.c | 230 ++++++++++++++++++++++ drivers/gpu/drm/i915/intel_tc.h | 13 ++ 7 files changed, 252 insertions(+), 230 deletions(-) create mode 100644 drivers/gpu/drm/i915/intel_tc.c create mode 100644 drivers/gpu/drm/i915/intel_tc.h