drm/i915/selftests: ring all doorbells in igt_guc_doorbells
diff mbox series

Message ID 20180827223614.22789-1-daniele.ceraolospurio@intel.com
State New
Headers show
Series
  • drm/i915/selftests: ring all doorbells in igt_guc_doorbells
Related show

Commit Message

Daniele Ceraolo Spurio Aug. 27, 2018, 10:36 p.m. UTC
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@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 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(-)

Comments

Michel Thierry Aug. 27, 2018, 11:33 p.m. UTC | #1
On 8/27/2018 3:36 PM, Daniele Ceraolo Spurio wrote:
> We currently verify that all doorbells can be registerd with GuC and
						^registered
> 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@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
>   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)
>   #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);
Note to self, this is WQ_NOOP + three u32 with 0's

> +	} 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:
> 

Reviewed-by: Michel Thierry <michel.thierry@intel.com>
Katarzyna Dec Aug. 28, 2018, 12:34 p.m. UTC | #2
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@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
>  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@intel.com>

> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Chris Wilson Aug. 28, 2018, 12:47 p.m. UTC | #3
Quoting Katarzyna Dec (2018-08-28 13:34:16)
> 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@intel.com>
> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> > Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
[snip]
> Selftests are new topic for me, but this one looks fairly simple. I hope
> I understand it correctly.
> Acked-by: Katarzyna Dec <katarzyna.dec@intel.com>

Fixed up the spelling mistake and pushed. Thanks for the review and more
testing,
-Chris
Daniele Ceraolo Spurio Aug. 28, 2018, 4:16 p.m. UTC | #4
<snip>


>> @@ -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);
> Note to self, this is WQ_NOOP + three u32 with 0's
> 

Actually we don't care what's in the 3 u32 following the header. When 
WQ_TYPE_NOOP is used GuC will just bump the WQ head based on the 
provided length and then move to the next request, so it is going to 
jump over them.

Daniele
Daniele Ceraolo Spurio Aug. 28, 2018, 4:21 p.m. UTC | #5
<snip>


>> 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?
> 

Yes they're unused. I'm assuming when the FW support was first added to 
i915 we just defined all the cases available in the FW at the time, even 
if we didn't actually use them. Personally I think it is worth keeping 
them as having only part of the modes defined would be less clear IMO.

Daniele

Patch
diff mbox series

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)
 #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: