Re: [Intel-gfx] [PATCH v2 26/50] drm/i915/display/tc: Rename safe_mode functions ownership

From: Imre Deak <imre.deak@intel.com>
To: Matt Roper <matthew.d.roper@intel.com>
Cc: intel-gfx@lists.freedesktop.org, me@freedesktop.org
Subject: Re: [Intel-gfx] [PATCH v2 26/50] drm/i915/display/tc: Rename safe_mode functions ownership
Date: Fri, 26 Mar 2021 18:31:48 +0200	[thread overview]
Message-ID: <20210326163148.GO2237616@ideak-desk.fi.intel.com> (raw)
In-Reply-To: <20210325180720.401410-27-matthew.d.roper@intel.com>

On Thu, Mar 25, 2021 at 11:06:56AM -0700, Matt Roper wrote:
> From: José Roberto de Souza <jose.souza@intel.com>
> 
> When DP_PHY_MODE_STATUS_NOT_SAFE is set, it means that display
> has the control over the TC phy.
> The "not safe" naming is confusing using ownership make it easier
> to read also future platforms will have a new register that does the
> same job as DP_PHY_MODE_STATUS_NOT_SAFE but with the onwership name.
> 
> BSpec: 49294
> Cc: Imre Deak <imre.deak@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> Signed-off-by: Clinton Taylor <Clinton.A.Taylor@intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>

Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_tc.c | 35 ++++++++++++-------------
>  1 file changed, 17 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c
> index 71b8edafb1c3..c007803cfd30 100644
> --- a/drivers/gpu/drm/i915/display/intel_tc.c
> +++ b/drivers/gpu/drm/i915/display/intel_tc.c
> @@ -256,8 +256,8 @@ static bool icl_tc_phy_status_complete(struct intel_digital_port *dig_port)
>  	return val & DP_PHY_MODE_STATUS_COMPLETED(dig_port->tc_phy_fia_idx);
>  }
>  
> -static bool icl_tc_phy_set_safe_mode(struct intel_digital_port *dig_port,
> -				     bool enable)
> +static bool icl_tc_phy_take_ownership(struct intel_digital_port *dig_port,
> +				      bool take)
>  {
>  	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
>  	struct intel_uncore *uncore = &i915->uncore;
> @@ -267,20 +267,20 @@ static bool icl_tc_phy_set_safe_mode(struct intel_digital_port *dig_port,
>  				PORT_TX_DFLEXDPCSSS(dig_port->tc_phy_fia));
>  	if (val == 0xffffffff) {
>  		drm_dbg_kms(&i915->drm,
> -			    "Port %s: PHY in TCCOLD, can't set safe-mode to %s\n",
> -			    dig_port->tc_port_name, enableddisabled(enable));
> +			    "Port %s: PHY in TCCOLD, can't %s ownership\n",
> +			    dig_port->tc_port_name, take ? "take" : "release");
>  
>  		return false;
>  	}
>  
>  	val &= ~DP_PHY_MODE_STATUS_NOT_SAFE(dig_port->tc_phy_fia_idx);
> -	if (!enable)
> +	if (take)
>  		val |= DP_PHY_MODE_STATUS_NOT_SAFE(dig_port->tc_phy_fia_idx);
>  
>  	intel_uncore_write(uncore,
>  			   PORT_TX_DFLEXDPCSSS(dig_port->tc_phy_fia), val);
>  
> -	if (enable && wait_for(!icl_tc_phy_status_complete(dig_port), 10))
> +	if (!take && wait_for(!icl_tc_phy_status_complete(dig_port), 10))
>  		drm_dbg_kms(&i915->drm,
>  			    "Port %s: PHY complete clear timed out\n",
>  			    dig_port->tc_port_name);
> @@ -288,7 +288,7 @@ static bool icl_tc_phy_set_safe_mode(struct intel_digital_port *dig_port,
>  	return true;
>  }
>  
> -static bool icl_tc_phy_is_in_safe_mode(struct intel_digital_port *dig_port)
> +static bool icl_tc_phy_is_owned(struct intel_digital_port *dig_port)
>  {
>  	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
>  	struct intel_uncore *uncore = &i915->uncore;
> @@ -303,7 +303,7 @@ static bool icl_tc_phy_is_in_safe_mode(struct intel_digital_port *dig_port)
>  		return true;
>  	}
>  
> -	return !(val & DP_PHY_MODE_STATUS_NOT_SAFE(dig_port->tc_phy_fia_idx));
> +	return val & DP_PHY_MODE_STATUS_NOT_SAFE(dig_port->tc_phy_fia_idx);
>  }
>  
>  /*
> @@ -329,7 +329,7 @@ static void icl_tc_phy_connect(struct intel_digital_port *dig_port,
>  		goto out_set_tbt_alt_mode;
>  	}
>  
> -	if (!icl_tc_phy_set_safe_mode(dig_port, false) &&
> +	if (!icl_tc_phy_take_ownership(dig_port, true) &&
>  	    !drm_WARN_ON(&i915->drm, dig_port->tc_legacy_port))
>  		goto out_set_tbt_alt_mode;
>  
> @@ -348,7 +348,7 @@ static void icl_tc_phy_connect(struct intel_digital_port *dig_port,
>  	if (!(tc_port_live_status_mask(dig_port) & BIT(TC_PORT_DP_ALT))) {
>  		drm_dbg_kms(&i915->drm, "Port %s: PHY sudden disconnect\n",
>  			    dig_port->tc_port_name);
> -		goto out_set_safe_mode;
> +		goto out_release_phy;
>  	}
>  
>  	if (max_lanes < required_lanes) {
> @@ -356,15 +356,15 @@ static void icl_tc_phy_connect(struct intel_digital_port *dig_port,
>  			    "Port %s: PHY max lanes %d < required lanes %d\n",
>  			    dig_port->tc_port_name,
>  			    max_lanes, required_lanes);
> -		goto out_set_safe_mode;
> +		goto out_release_phy;
>  	}
>  
>  	dig_port->tc_mode = TC_PORT_DP_ALT;
>  
>  	return;
>  
> -out_set_safe_mode:
> -	icl_tc_phy_set_safe_mode(dig_port, true);
> +out_release_phy:
> +	icl_tc_phy_take_ownership(dig_port, false);
>  out_set_tbt_alt_mode:
>  	dig_port->tc_mode = TC_PORT_TBT_ALT;
>  }
> @@ -380,7 +380,7 @@ static void icl_tc_phy_disconnect(struct intel_digital_port *dig_port)
>  		/* Nothing to do, we never disconnect from legacy mode */
>  		break;
>  	case TC_PORT_DP_ALT:
> -		icl_tc_phy_set_safe_mode(dig_port, true);
> +		icl_tc_phy_take_ownership(dig_port, false);
>  		dig_port->tc_mode = TC_PORT_TBT_ALT;
>  		break;
>  	case TC_PORT_TBT_ALT:
> @@ -401,8 +401,8 @@ static bool icl_tc_phy_is_connected(struct intel_digital_port *dig_port)
>  		return dig_port->tc_mode == TC_PORT_TBT_ALT;
>  	}
>  
> -	if (icl_tc_phy_is_in_safe_mode(dig_port)) {
> -		drm_dbg_kms(&i915->drm, "Port %s: PHY still in safe mode\n",
> +	if (!icl_tc_phy_is_owned(dig_port)) {
> +		drm_dbg_kms(&i915->drm, "Port %s: PHY not owned\n",
>  			    dig_port->tc_port_name);
>  
>  		return false;
> @@ -417,10 +417,9 @@ intel_tc_port_get_current_mode(struct intel_digital_port *dig_port)
>  {
>  	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
>  	u32 live_status_mask = tc_port_live_status_mask(dig_port);
> -	bool in_safe_mode = icl_tc_phy_is_in_safe_mode(dig_port);
>  	enum tc_port_mode mode;
>  
> -	if (in_safe_mode ||
> +	if (!icl_tc_phy_is_owned(dig_port) ||
>  	    drm_WARN_ON(&i915->drm, !icl_tc_phy_status_complete(dig_port)))
>  		return TC_PORT_TBT_ALT;
>  
> -- 
> 2.25.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx