[Intel-gfx] [PATCH 03/23] drm/i915: Move the TypeC port handling code to a separate file

Imre Deak imre.deak at intel.com
Thu Jun 6 09:09:05 UTC 2019


On Thu, Jun 06, 2019 at 11:42:46AM +0300, Jani Nikula wrote:
> On Tue, 04 Jun 2019, Imre Deak <imre.deak at 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 at intel.com>
> > Cc: Paulo Zanoni <paulo.r.zanoni at intel.com>
> > Cc: José Roberto de Souza <jose.souza at intel.com>
> > Signed-off-by: Imre Deak <imre.deak at 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


More information about the Intel-gfx mailing list