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

[Chris Wilson]

On Tue, Jul 19, 2016 at 12:12:20PM +0100, Tvrtko Ursulin wrote:
> 
> On 10/07/16 14:41, akash.goel at intel.com wrote:
> >From: Sagar Arun Kamble <sagar.a.kamble at intel.com>
> >
> >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 at intel.com>
> >Signed-off-by: Akash Goel <akash.goel at intel.com>
> >---
> >  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)?

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.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre