Re: [PATCH] drm/i915/tc: Implement the TC cold exit sequence

[Imre Deak <imre.deak@intel.com>]

On Mon, Sep 30, 2019 at 05:55:36PM -0700, José Roberto de Souza wrote:
> This is required for legacy/static TC ports as IOM is not aware of
> the connection and will not trigger the TC cold exit.
> 
> BSpec: 21750
> BSpsc: 49294
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_tc.c | 34 ++++++++++++++++++++-----
>  drivers/gpu/drm/i915/i915_reg.h         |  1 +
>  2 files changed, 29 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c
> index 7773169b7331..09b78027bdd5 100644
> --- a/drivers/gpu/drm/i915/display/intel_tc.c
> +++ b/drivers/gpu/drm/i915/display/intel_tc.c
> @@ -7,6 +7,7 @@
>  #include "intel_display.h"
>  #include "intel_display_types.h"
>  #include "intel_dp_mst.h"
> +#include "intel_sideband.h"
>  #include "intel_tc.h"
>  
>  static const char *tc_port_mode_name(enum tc_port_mode mode)
> @@ -169,6 +170,22 @@ static void tc_port_fixup_legacy_flag(struct intel_digital_port *dig_port,
>  	dig_port->tc_legacy_port = !dig_port->tc_legacy_port;
>  }
>  
> +static int tc_cold_exit_request(struct drm_i915_private *dev_priv)
> +{
> +	int ret;
> +
> +	do {
> +		ret = sandybridge_pcode_write_timeout(dev_priv,
> +						      ICL_PCODE_EXIT_TCCOLD, 0,
> +						      250, 1);
> +
> +	} while (ret == -EAGAIN);
> +
> +	DRM_DEBUG_KMS("tccold exit %s\n", ret == 0 ? "succeeded" : "failed");
> +
> +	return ret;
> +}
> +
>  static u32 tc_port_live_status_mask(struct intel_digital_port *dig_port)
>  {
>  	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> @@ -177,13 +194,21 @@ static u32 tc_port_live_status_mask(struct intel_digital_port *dig_port)
>  	u32 mask = 0;
>  	u32 val;
>  
> +	if (intel_uncore_read(uncore, SDEISR) & SDE_TC_HOTPLUG_ICP(tc_port))
> +		mask |= BIT(TC_PORT_LEGACY);
> +
>  	val = intel_uncore_read(uncore,
>  				PORT_TX_DFLEXDPSP(dig_port->tc_phy_fia));
>  
>  	if (val == 0xffffffff) {
> -		DRM_DEBUG_KMS("Port %s: PHY in TCCOLD, nothing connected\n",
> -			      dig_port->tc_port_name);
> -		return mask;
> +		if (mask)
> +			tc_cold_exit_request(i915);

The semantic of the mbox command this needs is inherently racy on
systems with preemption, the instruction to use it is:

"""
Issue GT Driver mailbox command to exit TCCOLD, then complete steps b-d
within 500 milliseconds to prevent re-entry.
"""

We'd have to reach the point where we enable the AUX power in 500ms,
which can't be guaranteed. Moreover TCCOLD could be entered just after
reading PORT_TX_DFLEXDPSP, so we may not detect it.

What we can do - after discussing with HW folks - is to first request
AUX power and then issue the mbox command, which prevents TCCOLD
re-entry until releasing the AUX power request. This also needs ignoring
the timeout of the AUX power enabling ACK, since it will only be ACKed
after exiting TCCOLD.

So I think we should block TCCOLD this way in __intel_tc_port_lock():

	if tc_link_refcount==0:
		intel_display_power_get(POWER_DOMAIN_AUX_<port>)
		tc_cold_exit_request()
		if intel_tc_port_needs_reset():
			intel_tc_port_reset_mode()
		if tc_mode != LEGACY:
			intel_display_power_put(POWER_DOMAIN_AUX_<port>)

then unblock TCCOLD in intel_tc_port_unlock():

	if tc_link_refcount==0 and tc_mode == LEGACY:
		intel_display_power_put(POWER_DOMAIN_AUX_<port>)
	
> +
> +		if (val == 0xffffffff) {
> +			DRM_DEBUG_KMS("Port %s: PHY in TCCOLD, nothing connected\n",
> +				      dig_port->tc_port_name);
> +			return mask;
> +		}
>  	}
>  
>  	if (val & TC_LIVE_STATE_TBT(dig_port->tc_phy_fia_idx))
> @@ -191,9 +216,6 @@ static u32 tc_port_live_status_mask(struct intel_digital_port *dig_port)
>  	if (val & TC_LIVE_STATE_TC(dig_port->tc_phy_fia_idx))
>  		mask |= BIT(TC_PORT_DP_ALT);
>  
> -	if (intel_uncore_read(uncore, SDEISR) & SDE_TC_HOTPLUG_ICP(tc_port))
> -		mask |= BIT(TC_PORT_LEGACY);
> -
>  	/* The sink can be connected only in a single mode. */
>  	if (!WARN_ON(hweight32(mask) > 1))
>  		tc_port_fixup_legacy_flag(dig_port, mask);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 058aa5ca8b73..35c3724b7fef 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -8860,6 +8860,7 @@ enum {
>  #define     ICL_PCODE_MEM_SS_READ_QGV_POINT_INFO(point)	(((point) << 16) | (0x1 << 8))
>  #define   GEN6_PCODE_READ_D_COMP		0x10
>  #define   GEN6_PCODE_WRITE_D_COMP		0x11
> +#define   ICL_PCODE_EXIT_TCCOLD			0x12
>  #define   HSW_PCODE_DE_WRITE_FREQ_REQ		0x17
>  #define   DISPLAY_IPS_CONTROL			0x19
>              /* See also IPS_CTL */
> -- 
> 2.23.0
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx