On 12/08/16 07:25, 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. v2: Use mutex in place of spinlock to serialize, as sleep can happen while waiting for the action's response from GuC. (Tvrtko) 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 1a2d648..cb9672b 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); + mutex_lock(&guc->action_lock);
I would probably take the mutex before grabbing forcewake as a general rule. Not that I think it matters in this case since we don't expect any contention on this one.
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; + mutex_unlock(&guc->action_lock); intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL); return ret; @@ -1312,6 +1314,7 @@ int i915_guc_submission_init(struct drm_i915_private *dev_priv) return -ENOMEM; ida_init(&guc->ctx_ids); + mutex_init(&guc->action_lock); 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 96ef7dc..e4ec8d8 100644 --- a/drivers/gpu/drm/i915/intel_guc.h +++ b/drivers/gpu/drm/i915/intel_guc.h @@ -156,6 +156,9 @@ struct intel_guc { uint64_t submissions[I915_NUM_ENGINES]; uint32_t last_seqno[I915_NUM_ENGINES]; + + /* To serialize the Host2GuC actions */ + struct mutex action_lock; }; /* intel_guc_loader.c */
With or without the mutex vs forcewake ordering change: Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx