Re: [PATCH 08/17] drm/i915: Forcefully flush GuC log buffer on reset

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

 





On 7/19/2016 4:51 PM, Chris Wilson wrote:
On Tue, Jul 19, 2016 at 12:12:20PM +0100, Tvrtko Ursulin wrote:

On 10/07/16 14:41, akash.goel@xxxxxxxxx wrote:
From: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx>

If GuC logs are being captured, there should be a force log buffer flush
action sent to GuC before proceeding with GPU reset and re-initializing
GUC. Those logs would be useful to understand why the GPU reset was
initiated.

v2: Rebase.

Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx>
Signed-off-by: Akash Goel <akash.goel@xxxxxxxxx>
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 32 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_irq.c            |  2 ++
 drivers/gpu/drm/i915/intel_guc.h           |  1 +
 3 files changed, 35 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 9b436fa..8cc31c6 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -183,6 +183,16 @@ static int host2guc_logbuffer_flush_complete(struct intel_guc *guc)
 	return host2guc_action(guc, data, 1);
 }

+static int host2guc_force_logbuffer_flush(struct intel_guc *guc)
+{
+	u32 data[2];
+
+	data[0] = HOST2GUC_ACTION_FORCE_LOG_BUFFER_FLUSH;
+	data[1] = 0;
+
+	return host2guc_action(guc, data, 2);
+}
+
 /*
  * Initialise, update, or clear doorbell data shared with the GuC
  *
@@ -1404,6 +1414,28 @@ void i915_guc_capture_logs(struct drm_device *dev)
 	intel_runtime_pm_put(dev_priv);
 }

+void i915_guc_capture_logs_on_reset(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	mutex_lock(&dev->struct_mutex);

Not sure what are the repercussion of taking the mutex on the
i915_reset_and_wakeup and path (error capture, hangcheck, dont' know
this area well). Check with Chris and Mika I suppose (cc-ed)?


Took the struct_mutex, just to avoid a very remote possibility where
i915_guc_capture_logs_on_reset & debugfs function i915_guc_log_control executes concurrently.

Flat out invalid to take struct_mutex on the error capture path, or any
lock at all really (just in case of driver bugs). Consider it to be an
atomic context that may preempt the driver at any point.

Actually I see that i915_reset() too takes the struct_mutex right at the beginning and I have plugged the call to i915_guc_capture_logs_on_reset() just before that.

Also it is being called after i915_error_wake_up(), so any client waiting on a request would have backed off and any new attempt by clients to lock the struct_mutex should see i915_reset_in_progress as
true.

Best regards
Akash
-Chris

_______________________________________________
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