[Intel-gfx] [PATCH 3/3] drm/i915/guc: Use intel_wait_for_register_fw() while sending over MMIO

[Chris Wilson]

On Fri, Apr 07, 2017 at 01:32:12PM +0000, Michal Wajdeczko wrote:
> Waiting for the response status in scratch register can be done
> using our generic function that waits for the expected register
> state. Since completion of the GuC commands can take longer than
> 2us mark that wait as bit slower to extend atomic wait to 10us.
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Suggested-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_uc.c | 27 ++++++++-------------------
>  1 file changed, 8 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index c117424..432b540 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -360,19 +360,6 @@ void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
>  }
>  
>  /*
> - * Read GuC command/status register (SOFT_SCRATCH_0)
> - * Return true if it contains a response rather than a command
> - */
> -static bool guc_recv(struct intel_guc *guc, u32 *status)
> -{
> -	struct drm_i915_private *dev_priv = guc_to_i915(guc);
> -
> -	u32 val = I915_READ(SOFT_SCRATCH(0));
> -	*status = val;
> -	return INTEL_GUC_RECV_IS_RESPONSE(val);
> -}
> -
> -/*
>   * This function implements the MMIO based host to GuC interface.
>   */
>  int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
> @@ -399,13 +386,15 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
>  	I915_WRITE(GUC_SEND_INTERRUPT, GUC_SEND_TRIGGER);
>  
>  	/*
> -	 * Fast commands should complete in less than 10us, so sample quickly
> -	 * up to that length of time, then switch to a slower sleep-wait loop.
> -	 * No inte_guc_send command should ever take longer than 10ms.
> +	 * No GuC command should ever take longer than 10ms.
> +	 * Fast commands should still complete in 10us.
>  	 */
> -	ret = wait_for_us(guc_recv(guc, &status), 10);
> -	if (ret)
> -		ret = wait_for(guc_recv(guc, &status), 10);
> +	ret = intel_wait_for_register_fw(dev_priv,
> +					 SOFT_SCRATCH(0),
> +					 INTEL_GUC_RECV_MASK,
> +					 INTEL_GUC_RECV_MASK,
> +					 false, 10);
> +	status = I915_READ(SOFT_SCRATCH(0));

Reporting the final register value is common -- I think most of the
remaining wait_for_us() holdouts are because they wanted to interpret
the value after the loop. I wonder if adding an out param for the value
to wait_for_register would cover the remaining missing users?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre