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/20/2016 2:42 PM, Chris Wilson wrote:
On Wed, Jul 20, 2016 at 09:51:45AM +0530, Goel, Akash wrote:


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.

Postmortem state is captured from i915_capture_error_state(), and as I
recall one of the raison d'etre for this facility was to include the guc
log in the error state.

Sorry I missed augmenting the error state with guc firmware logs.
For that also a prior flush will be needed, will do the flush without acquiring the struct_mutex.

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