Re: [PATCH] drm/i915/selftests: ring all doorbells in igt_guc_doorbells

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Aug 27, 2018 at 03:36:14PM -0700, Daniele Ceraolo Spurio wrote:
> We currently verify that all doorbells can be registerd with GuC and
> HW but don't check that all works as expected after a db ring.
> 
> Do a nop ring of all doorbells to make sure we haven't misprogrammed
> any WQ or stage descriptor data. This will also help validating
> upcoming changes in the db programming flow.
> 
> Cc: Michel Thierry <michel.thierry@xxxxxxxxx>
> Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/intel_guc_fwif.h       |  1 +
>  drivers/gpu/drm/i915/intel_guc_submission.c | 25 +++++++++-----
>  drivers/gpu/drm/i915/intel_guc_submission.h |  4 +++
>  drivers/gpu/drm/i915/selftests/intel_guc.c  | 38 +++++++++++++++++++++
>  4 files changed, 59 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h
> index 1a0f2a39cef9..8382d591c784 100644
> --- a/drivers/gpu/drm/i915/intel_guc_fwif.h
> +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
> @@ -49,6 +49,7 @@
>  #define   WQ_TYPE_BATCH_BUF		(0x1 << WQ_TYPE_SHIFT)
>  #define   WQ_TYPE_PSEUDO		(0x2 << WQ_TYPE_SHIFT)
>  #define   WQ_TYPE_INORDER		(0x3 << WQ_TYPE_SHIFT)
> +#define   WQ_TYPE_NOOP			(0x4 << WQ_TYPE_SHIFT)

I got general question to this ^ defines. Do I correctly see that PSEUDO and BATCH_BUF are not
used anywhere?

>  #define WQ_TARGET_SHIFT			10
>  #define WQ_LEN_SHIFT			16
>  #define WQ_NO_WCFLUSH_WAIT		(1 << 27)
> diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
> index 195adbd0ebf7..07b9d313b019 100644
> --- a/drivers/gpu/drm/i915/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/intel_guc_submission.c
> @@ -456,6 +456,9 @@ static void guc_wq_item_append(struct intel_guc_client *client,
>  	 */
>  	BUILD_BUG_ON(wqi_size != 16);
>  
> +	/* We expect the WQ to be active if we're appending items to it */
> +	GEM_BUG_ON(desc->wq_status != WQ_STATUS_ACTIVE);
> + 
>  	/* Free space is guaranteed. */
>  	wq_off = READ_ONCE(desc->tail);
>  	GEM_BUG_ON(CIRC_SPACE(wq_off, READ_ONCE(desc->head),
> @@ -465,15 +468,19 @@ static void guc_wq_item_append(struct intel_guc_client *client,
>  	/* WQ starts from the page after doorbell / process_desc */
>  	wqi = client->vaddr + wq_off + GUC_DB_SIZE;
>  
> -	/* Now fill in the 4-word work queue item */
> -	wqi->header = WQ_TYPE_INORDER |
> -		      (wqi_len << WQ_LEN_SHIFT) |
> -		      (target_engine << WQ_TARGET_SHIFT) |
> -		      WQ_NO_WCFLUSH_WAIT;
> -	wqi->context_desc = context_desc;
> -	wqi->submit_element_info = ring_tail << WQ_RING_TAIL_SHIFT;
> -	GEM_BUG_ON(ring_tail > WQ_RING_TAIL_MAX);
> -	wqi->fence_id = fence_id;
> +	if (I915_SELFTEST_ONLY(client->use_nop_wqi)) {
> +		wqi->header = WQ_TYPE_NOOP | (wqi_len << WQ_LEN_SHIFT);
> +	} else {
> +		/* Now fill in the 4-word work queue item */
> +		wqi->header = WQ_TYPE_INORDER |
> +			      (wqi_len << WQ_LEN_SHIFT) |
> +			      (target_engine << WQ_TARGET_SHIFT) |
> +			      WQ_NO_WCFLUSH_WAIT;
> +		wqi->context_desc = context_desc;
> +		wqi->submit_element_info = ring_tail << WQ_RING_TAIL_SHIFT;
> +		GEM_BUG_ON(ring_tail > WQ_RING_TAIL_MAX);
> +		wqi->fence_id = fence_id;
> +	}
>  
>  	/* Make the update visible to GuC */
>  	WRITE_ONCE(desc->tail, (wq_off + wqi_size) & (GUC_WQ_SIZE - 1));
> diff --git a/drivers/gpu/drm/i915/intel_guc_submission.h b/drivers/gpu/drm/i915/intel_guc_submission.h
> index fb081cefef93..169c54568340 100644
> --- a/drivers/gpu/drm/i915/intel_guc_submission.h
> +++ b/drivers/gpu/drm/i915/intel_guc_submission.h
> @@ -28,6 +28,7 @@
>  #include <linux/spinlock.h>
>  
>  #include "i915_gem.h"
> +#include "i915_selftest.h"
>  
>  struct drm_i915_private;
>  
> @@ -71,6 +72,9 @@ struct intel_guc_client {
>  	spinlock_t wq_lock;
>  	/* Per-engine counts of GuC submissions */
>  	u64 submissions[I915_NUM_ENGINES];
> +
> +	/* For testing purposes, use nop WQ items instead of real ones */
> +	I915_SELFTEST_DECLARE(bool use_nop_wqi);
>  };
>  
>  int intel_guc_submission_init(struct intel_guc *guc);
> diff --git a/drivers/gpu/drm/i915/selftests/intel_guc.c b/drivers/gpu/drm/i915/selftests/intel_guc.c
> index 407c98fb9170..3154fd2f625d 100644
> --- a/drivers/gpu/drm/i915/selftests/intel_guc.c
> +++ b/drivers/gpu/drm/i915/selftests/intel_guc.c
> @@ -65,6 +65,40 @@ static int check_all_doorbells(struct intel_guc *guc)
>  	return 0;
>  }
>  
> +static int ring_doorbell_nop(struct intel_guc_client *client)
> +{
> +	int err;
> +	struct guc_process_desc *desc = __get_process_desc(client);
> +
> +	client->use_nop_wqi = true;
> +
> +	spin_lock_irq(&client->wq_lock);
> +
> +	guc_wq_item_append(client, 0, 0, 0, 0);
> +	guc_ring_doorbell(client);
> +
> +	spin_unlock_irq(&client->wq_lock);
> +
> +	client->use_nop_wqi = false;
> +
> +	/* if there are no issues GuC will update the WQ head and keep the
> +	 * WQ in active status
> +	 */
> +	err = wait_for(READ_ONCE(desc->head) == READ_ONCE(desc->tail), 10);
> +	if (err) {
> +		pr_err("doorbell %u ring failed!\n", client->doorbell_id);
> +		return -EIO;
> +	}
> +
> +	if (desc->wq_status != WQ_STATUS_ACTIVE) {
> +		pr_err("doorbell %u ring put WQ in bad state (%u)!\n",
> +		       client->doorbell_id, desc->wq_status);
> +		return -EIO;
> +	}
> +
> +	return 0;
> +}
> +
>  /*
>   * Basic client sanity check, handy to validate create_clients.
>   */
> @@ -332,6 +366,10 @@ static int igt_guc_doorbells(void *arg)
>  		err = check_all_doorbells(guc);
>  		if (err)
>  			goto out;
> +
> +		err = ring_doorbell_nop(clients[i]);
> +		if (err)
> +			goto out;
>  	}
>  
>  out:
> -- 
> 2.18.0
> 
Selftests are new topic for me, but this one looks fairly simple. I hope
I understand it correctly.
Acked-by: Katarzyna Dec <katarzyna.dec@xxxxxxxxx>

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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux