On 7/18/2016 3:42 PM, Tvrtko Ursulin wrote:
On 15/07/16 16:51, Goel, Akash wrote:
On 7/15/2016 5:10 PM, Tvrtko Ursulin wrote:
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?
From patch 6 or 7 saw the problem, when enabled flush interrupts from
boot (guc_log_level >= 0).
That means this patch should come before 6 or 7. :)
Also new HOST2GUC actions LOG_BUFFER_FILE_FLUSH_COMPLETE &
UK_LOG_ENABLE_LOGGING can execute concurrently with each other.
Right I see, from the worker/thread vs debugfs activity.
Will use mutex to serialize and place the patch earlier in the series.
Please suggest which would be better,
mutex_lock()
or
mutex_lock_interruptible().
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...
Sorry I missed the sleep.
Probably I did not see any problem, in spite of a spinlock, as _wait_for
macro does not sleep when used in atomic context, does a busy wait
instead.
I wonder about that in general, since in_atomic is not a reliable
indicator. But that is beside the point. You probably haven't seen it
because the action completes in the first shorter, atomic sleep, check.
Actually I had profiled host2guc_logbuffer_flush_complete() and saw that
on some occasions it was taking more than 100 micro seconds,
so presumably it would have went past the first wait.
But most of the times it was less than 10 micro seconds only.
ret = wait_for_us(host2guc_action_response(dev_priv, &status), 10);
if (ret)
ret = wait_for(host2guc_action_response(dev_priv, &status), 10);
Best regards
Akash
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
