Re: [PATCH 2/4] drm/i915/guc: move guc_ring_doorbell() nearer to callsite

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 08/06/16 11:55, Dave Gordon wrote:
> Just code movement, no actual change to the function. This is in
> preparation for the next patch, which will reorganise all the other
> doorbell code, but doesn't change this function. So let's shuffle it
> down near its caller rather than leaving it mixed in with the setup
> code. Unlike the doorbell management code, this function is somewhat
> time-critical, so putting it near its caller may even yield a tiny
> performance improvement.
>
> Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_guc_submission.c | 110 ++++++++++++++---------------
>   1 file changed, 55 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index 2db1182..7510841 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -185,61 +185,6 @@ static void guc_init_doorbell(struct intel_guc *guc,
>   	doorbell->cookie = 0;
>   }
>
> -static int guc_ring_doorbell(struct i915_guc_client *gc)
> -{
> -	struct guc_process_desc *desc;
> -	union guc_doorbell_qw db_cmp, db_exc, db_ret;
> -	union guc_doorbell_qw *db;
> -	int attempt = 2, ret = -EAGAIN;
> -
> -	desc = gc->client_base + gc->proc_desc_offset;
> -
> -	/* Update the tail so it is visible to GuC */
> -	desc->tail = gc->wq_tail;
> -
> -	/* current cookie */
> -	db_cmp.db_status = GUC_DOORBELL_ENABLED;
> -	db_cmp.cookie = gc->cookie;
> -
> -	/* cookie to be updated */
> -	db_exc.db_status = GUC_DOORBELL_ENABLED;
> -	db_exc.cookie = gc->cookie + 1;
> -	if (db_exc.cookie == 0)
> -		db_exc.cookie = 1;
> -
> -	/* pointer of current doorbell cacheline */
> -	db = gc->client_base + gc->doorbell_offset;
> -
> -	while (attempt--) {
> -		/* lets ring the doorbell */
> -		db_ret.value_qw = atomic64_cmpxchg((atomic64_t *)db,
> -			db_cmp.value_qw, db_exc.value_qw);
> -
> -		/* if the exchange was successfully executed */
> -		if (db_ret.value_qw == db_cmp.value_qw) {
> -			/* db was successfully rung */
> -			gc->cookie = db_exc.cookie;
> -			ret = 0;
> -			break;
> -		}
> -
> -		/* XXX: doorbell was lost and need to acquire it again */
> -		if (db_ret.db_status == GUC_DOORBELL_DISABLED)
> -			break;
> -
> -		DRM_ERROR("Cookie mismatch. Expected %d, returned %d\n",
> -			  db_cmp.cookie, db_ret.cookie);
> -
> -		/* update the cookie to newly read cookie from GuC */
> -		db_cmp.cookie = db_ret.cookie;
> -		db_exc.cookie = db_ret.cookie + 1;
> -		if (db_exc.cookie == 0)
> -			db_exc.cookie = 1;
> -	}
> -
> -	return ret;
> -}
> -
>   static void guc_disable_doorbell(struct intel_guc *guc,
>   				 struct i915_guc_client *client)
>   {
> @@ -543,6 +488,61 @@ static void guc_add_workqueue_item(struct i915_guc_client *gc,
>   	kunmap_atomic(base);
>   }
>
> +static int guc_ring_doorbell(struct i915_guc_client *gc)
> +{
> +	struct guc_process_desc *desc;
> +	union guc_doorbell_qw db_cmp, db_exc, db_ret;
> +	union guc_doorbell_qw *db;
> +	int attempt = 2, ret = -EAGAIN;
> +
> +	desc = gc->client_base + gc->proc_desc_offset;
> +
> +	/* Update the tail so it is visible to GuC */
> +	desc->tail = gc->wq_tail;
> +
> +	/* current cookie */
> +	db_cmp.db_status = GUC_DOORBELL_ENABLED;
> +	db_cmp.cookie = gc->cookie;
> +
> +	/* cookie to be updated */
> +	db_exc.db_status = GUC_DOORBELL_ENABLED;
> +	db_exc.cookie = gc->cookie + 1;
> +	if (db_exc.cookie == 0)
> +		db_exc.cookie = 1;
> +
> +	/* pointer of current doorbell cacheline */
> +	db = gc->client_base + gc->doorbell_offset;
> +
> +	while (attempt--) {
> +		/* lets ring the doorbell */
> +		db_ret.value_qw = atomic64_cmpxchg((atomic64_t *)db,
> +			db_cmp.value_qw, db_exc.value_qw);
> +
> +		/* if the exchange was successfully executed */
> +		if (db_ret.value_qw == db_cmp.value_qw) {
> +			/* db was successfully rung */
> +			gc->cookie = db_exc.cookie;
> +			ret = 0;
> +			break;
> +		}
> +
> +		/* XXX: doorbell was lost and need to acquire it again */
> +		if (db_ret.db_status == GUC_DOORBELL_DISABLED)
> +			break;
> +
> +		DRM_ERROR("Cookie mismatch. Expected %d, returned %d\n",
> +			  db_cmp.cookie, db_ret.cookie);
> +
> +		/* update the cookie to newly read cookie from GuC */
> +		db_cmp.cookie = db_ret.cookie;
> +		db_exc.cookie = db_ret.cookie + 1;
> +		if (db_exc.cookie == 0)
> +			db_exc.cookie = 1;
> +	}
> +
> +	return ret;
> +}
> +
>   /**
>    * i915_guc_submit() - Submit commands through GuC
>    * @rq:		request associated with the commands
>

Diffstat is correct, did not read the individual lines. :)

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx