On 8/12/2016 7:25 PM, Tvrtko Ursulin wrote:
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.
Yes did not expected a contention for this mutex, hence thought it use
just around the code where it is actually needed.
Will move it before the forcewake, as you suggested, to conform to the
rules.
Best regards
Akash
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
