diff mbox

[v2,2/6] drm/i915/guc: refactor guc_init_doorbell_hw()

Message ID 1469124942-17943-3-git-send-email-david.s.gordon@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Dave Gordon July 21, 2016, 6:15 p.m. UTC 
We have essentially the same code in each of two different loops, so we
can refactor it into a little helper function.

Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 54 +++++++++++++++++-------------
 1 file changed, 30 insertions(+), 24 deletions(-)

Comments

Tvrtko Ursulin July 22, 2016, 10:01 a.m. UTC | #1 
On 21/07/16 19:15, Dave Gordon wrote:
> We have essentially the same code in each of two different loops, so we
> can refactor it into a little helper function.
>
> Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_guc_submission.c | 54 +++++++++++++++++-------------
>   1 file changed, 30 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index 9c5b81b..816bdca 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -698,32 +698,47 @@ static void gem_release_guc_obj(struct drm_i915_gem_object *obj)
>   	kfree(client);
>   }
>
> +/* Check that a doorbell register is in the expected state */
> +static bool guc_doorbell_check(struct intel_guc *guc, uint16_t db_id)
> +{
> +	struct drm_i915_private *dev_priv = guc_to_i915(guc);
> +	i915_reg_t drbreg = GEN8_DRBREGL(db_id);
> +	uint32_t value = I915_READ(drbreg);
> +	bool enabled = (value & GUC_DOORBELL_ENABLED) != 0;
> +	bool expected = test_bit(db_id, guc->doorbell_bitmap);
> +
> +	if (enabled == expected)
> +		return true;
> +
> +	DRM_DEBUG_DRIVER("Doorbell %d (reg 0x%x) 0x%x, should be %s\n",
> +			 db_id, drbreg.reg, value,
> +			 expected ? "active" : "inactive");
> +
> +	return false;
> +}
> +
>   /*
>    * Borrow the first client to set up & tear down each unused doorbell
>    * in turn, to ensure that all doorbell h/w is (re)initialised.
>    */
>   static void guc_init_doorbell_hw(struct intel_guc *guc)
>   {
> -	struct drm_i915_private *dev_priv = guc_to_i915(guc);
>   	struct i915_guc_client *client = guc->execbuf_client;
> -	uint16_t db_id, i;
> -	int err;
> +	uint16_t db_id;
> +	int i, err;
>
> +	/* Save client's original doorbell selection */
>   	db_id = client->doorbell_id;
>
>   	for (i = 0; i < GUC_MAX_DOORBELLS; ++i) {
> -		i915_reg_t drbreg = GEN8_DRBREGL(i);
> -		u32 value = I915_READ(drbreg);
> -
> -		if (test_bit(i, guc->doorbell_bitmap))
> +		/* Skip if doorbell is OK */
> +		if (guc_doorbell_check(guc, i))
>   			continue;
>
>   		err = guc_update_doorbell_id(guc, client, i);
> -
> -		/* Report update failure or unexpectedly active doorbell */
> -		if (err || (i != db_id && (value & GUC_DOORBELL_ENABLED)))
> -			DRM_DEBUG_DRIVER("Doorbell %d (reg 0x%x) was 0x%x, err %d\n",
> -					  i, drbreg.reg, value, err);
> +		if (err)
> +			DRM_DEBUG_DRIVER("Doorbell %d update failed, err %d\n",
> +					i, err);
>   	}
>
>   	/* Restore to original value */
> @@ -732,18 +747,9 @@ static void guc_init_doorbell_hw(struct intel_guc *guc)
>   		DRM_ERROR("Failed to restore doorbell to %d, err %d\n",
>   			db_id, err);
>
> -	for (i = 0; i < GUC_MAX_DOORBELLS; ++i) {
> -		i915_reg_t drbreg = GEN8_DRBREGL(i);
> -		u32 value = I915_READ(drbreg);
> -
> -		if (test_bit(i, guc->doorbell_bitmap))
> -			continue;
> -
> -		if (i != db_id && (value & GUC_DOORBELL_ENABLED))
> -			DRM_DEBUG_DRIVER("Doorbell %d (reg 0x%x) finally 0x%x\n",
> -					  i, drbreg.reg, value);
> -
> -	}
> +	/* Read back & verify all doorbell registers */
> +	for (i = 0; i < GUC_MAX_DOORBELLS; ++i)
> +		(void)guc_doorbell_check(guc, i);
>   }
>
>   /**
>

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

Regards,

Tvrtko
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 9c5b81b..816bdca 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -698,32 +698,47 @@  static void gem_release_guc_obj(struct drm_i915_gem_object *obj)
 	kfree(client);
 }
 
+/* Check that a doorbell register is in the expected state */
+static bool guc_doorbell_check(struct intel_guc *guc, uint16_t db_id)
+{
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+	i915_reg_t drbreg = GEN8_DRBREGL(db_id);
+	uint32_t value = I915_READ(drbreg);
+	bool enabled = (value & GUC_DOORBELL_ENABLED) != 0;
+	bool expected = test_bit(db_id, guc->doorbell_bitmap);
+
+	if (enabled == expected)
+		return true;
+
+	DRM_DEBUG_DRIVER("Doorbell %d (reg 0x%x) 0x%x, should be %s\n",
+			 db_id, drbreg.reg, value,
+			 expected ? "active" : "inactive");
+
+	return false;
+}
+
 /*
  * Borrow the first client to set up & tear down each unused doorbell
  * in turn, to ensure that all doorbell h/w is (re)initialised.
  */
 static void guc_init_doorbell_hw(struct intel_guc *guc)
 {
-	struct drm_i915_private *dev_priv = guc_to_i915(guc);
 	struct i915_guc_client *client = guc->execbuf_client;
-	uint16_t db_id, i;
-	int err;
+	uint16_t db_id;
+	int i, err;
 
+	/* Save client's original doorbell selection */
 	db_id = client->doorbell_id;
 
 	for (i = 0; i < GUC_MAX_DOORBELLS; ++i) {
-		i915_reg_t drbreg = GEN8_DRBREGL(i);
-		u32 value = I915_READ(drbreg);
-
-		if (test_bit(i, guc->doorbell_bitmap))
+		/* Skip if doorbell is OK */
+		if (guc_doorbell_check(guc, i))
 			continue;
 
 		err = guc_update_doorbell_id(guc, client, i);
-
-		/* Report update failure or unexpectedly active doorbell */
-		if (err || (i != db_id && (value & GUC_DOORBELL_ENABLED)))
-			DRM_DEBUG_DRIVER("Doorbell %d (reg 0x%x) was 0x%x, err %d\n",
-					  i, drbreg.reg, value, err);
+		if (err)
+			DRM_DEBUG_DRIVER("Doorbell %d update failed, err %d\n",
+					i, err);
 	}
 
 	/* Restore to original value */
@@ -732,18 +747,9 @@  static void guc_init_doorbell_hw(struct intel_guc *guc)
 		DRM_ERROR("Failed to restore doorbell to %d, err %d\n",
 			db_id, err);
 
-	for (i = 0; i < GUC_MAX_DOORBELLS; ++i) {
-		i915_reg_t drbreg = GEN8_DRBREGL(i);
-		u32 value = I915_READ(drbreg);
-
-		if (test_bit(i, guc->doorbell_bitmap))
-			continue;
-
-		if (i != db_id && (value & GUC_DOORBELL_ENABLED))
-			DRM_DEBUG_DRIVER("Doorbell %d (reg 0x%x) finally 0x%x\n",
-					  i, drbreg.reg, value);
-
-	}
+	/* Read back & verify all doorbell registers */
+	for (i = 0; i < GUC_MAX_DOORBELLS; ++i)
+		(void)guc_doorbell_check(guc, i);
 }
 
 /**