Re: [PATCH v6] drm/i915/skl+: Add and enable DP AUX CH mutex

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Tue, Mar 06, 2018 at 12:11:03PM -0800, José Roberto de Souza wrote:
> When PSR/PSR2/GTC is enabled hardware can do AUX transactions by it
> self, so lets use the mutex register that is available in gen9+ to
> avoid concurrent access by hardware and driver.
> Older gen handling will be done separated.
> 
> Reference: https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-skl-vol12-display.pdf
> Page 198 - AUX programming sequence

Please let's drop this attempt and move without this while spec
doesn't receive the proper updates HW teams are discussing.

Let's move our focus to get psr disabled when trying to use aux.
(in a way that we don't kill sink_crc... at least while this is our
only automated way to test PSR)

Thanks,
Rodrigo.

> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
> 
> Changelog:
> v2
> - removed the PSR dependency, now getting lock all the times when available
> - renamed functions to avoid nested calls
> - moved register bits right after the DP_AUX_CH_MUTEX()
> - removed 'drm/i915: keep AUX powered while PSR is enabled' Dhinakaran Pandiyan will sent a better and final version
> v3
> - rebased on top of Ville's AUX series
> - moved port registers to above DP_AUX_CH_MUTEX()
> - using intel_wait_for_register() instead of the internal version
> v4
> - removed virtual function to get mutex register address
> - enabling the mutex back only on aux channel init
> - added the aux channel name to the timeout debug message
> v5
> - renamed DP_AUX_CH_MUTEX() parameter to aux_ch
> - renamed goto label when intel_dp_aux_ch_trylock() fails
> v6
> - enabling mutex in the first trylock to cover all power management cases
> - saving and restoring reserved bits in the mutex register
> 
>  drivers/gpu/drm/i915/i915_reg.h |  9 +++++++
>  drivers/gpu/drm/i915/intel_dp.c | 58 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 67 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 258e86eb37d5..da1dcef24ebf 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5388,6 +5388,15 @@ enum {
>  #define   DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL(c) (((c) - 1) << 5)
>  #define   DP_AUX_CH_CTL_SYNC_PULSE_SKL(c)   ((c) - 1)
>  
> +#define _DPA_AUX_CH_MUTEX	(dev_priv->info.display_mmio_offset + 0x6402C)
> +#define _DPB_AUX_CH_MUTEX	(dev_priv->info.display_mmio_offset + 0x6412C)
> +#define _DPC_AUX_CH_MUTEX	(dev_priv->info.display_mmio_offset + 0x6422C)
> +#define _DPD_AUX_CH_MUTEX	(dev_priv->info.display_mmio_offset + 0x6432C)
> +#define _DPF_AUX_CH_MUTEX	(dev_priv->info.display_mmio_offset + 0x6452C)
> +#define DP_AUX_CH_MUTEX(aux_ch)	_MMIO_PORT(aux_ch, _DPA_AUX_CH_MUTEX, _DPB_AUX_CH_MUTEX)
> +#define   DP_AUX_CH_MUTEX_ENABLE		(1 << 31)
> +#define   DP_AUX_CH_MUTEX_STATUS		(1 << 30)
> +
>  /*
>   * Computing GMCH M and N values for the Display Port link
>   *
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index c722a6750e90..b8ff5fa41b71 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1065,6 +1065,57 @@ static uint32_t skl_get_aux_send_ctl(struct intel_dp *intel_dp,
>  	       DP_AUX_CH_CTL_SYNC_PULSE_SKL(32);
>  }
>  
> +static bool intel_dp_aux_ch_trylock(struct intel_dp *intel_dp)
> +{
> +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> +	struct drm_i915_private *dev_priv =
> +			to_i915(intel_dig_port->base.base.dev);
> +	u32 val;
> +
> +	if (INTEL_GEN(dev_priv) < 9)
> +		return true;
> +
> +	/* A read to mutex register will request the lock if mutex is enabled */
> +	val = I915_READ(DP_AUX_CH_MUTEX(intel_dp->aux_ch));
> +	if ((val & DP_AUX_CH_MUTEX_ENABLE) == 0) {
> +		/* Mutex is disabled, enables it */
> +		I915_WRITE(DP_AUX_CH_MUTEX(intel_dp->aux_ch),
> +			   val | DP_AUX_CH_MUTEX_ENABLE);
> +		/* Now that the mutex is enabled, request the lock */
> +		val = I915_READ(DP_AUX_CH_MUTEX(intel_dp->aux_ch));
> +	}
> +
> +	/* Lock is acquired when status bit is unset */
> +	if ((val & DP_AUX_CH_MUTEX_STATUS) == 0)
> +		return true;
> +
> +	/* Try to get the lock for 2msec(+-4 aux transactions) before give up */
> +	if (intel_wait_for_register(dev_priv, DP_AUX_CH_MUTEX(intel_dp->aux_ch),
> +				    DP_AUX_CH_MUTEX_STATUS, 0, 2)) {
> +		DRM_DEBUG_KMS("aux channel %c locked for 2msec, timing out\n",
> +			      aux_ch_name(intel_dp->aux_ch));
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
> +static void intel_dp_aux_ch_unlock(struct intel_dp *intel_dp)
> +{
> +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> +	struct drm_i915_private *dev_priv =
> +			to_i915(intel_dig_port->base.base.dev);
> +	u32 val;
> +
> +	if (INTEL_GEN(dev_priv) < 9)
> +		return;
> +
> +	/* Set the status bit releases the mutex */
> +	val = I915_READ(DP_AUX_CH_MUTEX(intel_dp->aux_ch));
> +	val |= DP_AUX_CH_MUTEX_STATUS;
> +	I915_WRITE(DP_AUX_CH_MUTEX(intel_dp->aux_ch), val);
> +}
> +
>  static int
>  intel_dp_aux_xfer(struct intel_dp *intel_dp,
>  		  const uint8_t *send, int send_bytes,
> @@ -1104,6 +1155,11 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
>  
>  	intel_dp_check_edp(intel_dp);
>  
> +	if (!intel_dp_aux_ch_trylock(intel_dp)) {
> +		ret = -EBUSY;
> +		goto out_aux_ch_unlocked;
> +	}
> +
>  	/* Try to wait for any previous AUX channel activity */
>  	for (try = 0; try < 3; try++) {
>  		status = I915_READ_NOTRACE(ch_ctl);
> @@ -1226,6 +1282,8 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
>  
>  	ret = recv_bytes;
>  out:
> +	intel_dp_aux_ch_unlock(intel_dp);
> +out_aux_ch_unlocked:
>  	pm_qos_update_request(&dev_priv->pm_qos, PM_QOS_DEFAULT_VALUE);
>  
>  	if (vdd)
> -- 
> 2.16.2
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx