Re: [PATCH 13/17] drm/i915: New lock to serialize the Host2GuC actions

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

 




On 10/07/16 14:41, akash.goel@xxxxxxxxx wrote:
From: Akash Goel <akash.goel@xxxxxxxxx>

With the addition of new Host2GuC actions related to GuC logging, there
is a need of a lock to serialize them, as they can execute concurrently
with each other and also with other existing actions.

After which patch in this series is this required?


Signed-off-by: Akash Goel <akash.goel@xxxxxxxxx>
---
  drivers/gpu/drm/i915/i915_guc_submission.c | 3 +++
  drivers/gpu/drm/i915/intel_guc.h           | 3 +++
  2 files changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 6043166..c1e637f 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -88,6 +88,7 @@ static int host2guc_action(struct intel_guc *guc, u32 *data, u32 len)
  		return -EINVAL;

  	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
+	spin_lock(&guc->action_lock);

The code below can sleep waiting for a response from GuC so you cannot use a spinlock. Mutex I suppose...


  	dev_priv->guc.action_count += 1;
  	dev_priv->guc.action_cmd = data[0];
@@ -126,6 +127,7 @@ static int host2guc_action(struct intel_guc *guc, u32 *data, u32 len)
  	}
  	dev_priv->guc.action_status = status;

+	spin_unlock(&guc->action_lock);
  	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);

  	return ret;
@@ -1304,6 +1306,7 @@ int i915_guc_submission_init(struct drm_i915_private *dev_priv)
  		return -ENOMEM;

  	ida_init(&guc->ctx_ids);
+	spin_lock_init(&guc->action_lock);

I think this should go to guc_client_alloc which is where the guc client object is allocated and initialized.

  	guc_create_log(guc);
  	guc_create_ads(guc);

diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index d56bde6..611f4a7 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -157,6 +157,9 @@ struct intel_guc {

  	uint64_t submissions[I915_NUM_ENGINES];
  	uint32_t last_seqno[I915_NUM_ENGINES];
+
+	/* To serialize the Host2GuC actions */
+	spinlock_t action_lock;
  };

  /* intel_guc_loader.c */


Regards,

Tvrtko

_______________________________________________
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