Re: [PATCH 14/18] drm/i915: Forcefully flush GuC log buffer on reset

Tvrtko Ursulin
2016-08-15T15:48:19Z

On 15/08/16 15:49, akash.goel@intel.com wrote:
> From: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>
> Before capturing the GuC logs as a part of error state, there should be a
> force log buffer flush action sent to GuC before proceeding with GPU reset
> and re-initializing GUC. There could be some data in the log buffer which
> is yet to be captured and those logs would be particularly useful to
> understand that why the GPU reset was initiated.
>
> v2:
> - Avoid the wait via flush_work, to serialize against an ongoing log
> buffer flush, from the error state capture path. (Chris)

Could you explain if the patch does anything now that the flush has been
removed?

In fact I don't even understand what it was doing before. :)

If the idea is to send a flush command to GuC so it can raise an
interrupt for a partially full buffer, then i915_guc_flush_logs should
send the flush command and wait for that interrupt/work.

But the function is first waiting for the work item to complete and then
sending the flush command to the GuC. So I am confused.

Regards,

Tvrtko

> - Rebase.
>
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Signed-off-by: Akash Goel <akash.goel@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gpu_error.c | 2 ++
> drivers/gpu/drm/i915/i915_guc_submission.c | 30 ++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_guc.h | 1 +
> 3 files changed, 33 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index 94297aa..b73c671 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -1301,6 +1301,8 @@ static void i915_gem_capture_guc_log_buffer(struct drm_i915_private *dev_priv,
> if (!dev_priv->guc.log.vma || (i915.guc_log_level < 0))
> return;
>
> + i915_guc_flush_logs(dev_priv, false);
> +
> error->guc_log = i915_error_object_create(dev_priv,
> dev_priv->guc.log.vma);
> }
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index b8d6313..85df2f3 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -185,6 +185,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
> *
> @@ -1536,6 +1546,26 @@ void i915_guc_capture_logs(struct drm_i915_private *dev_priv)
> intel_runtime_pm_put(dev_priv);
> }
>
> +void i915_guc_flush_logs(struct drm_i915_private *dev_priv, bool can_wait)
> +{
> + if (!i915.enable_guc_submission || (i915.guc_log_level < 0))
> + return;
> +
> + /* First disable the interrupts, will be renabled afterwards */
> + gen9_disable_guc_interrupts(dev_priv);
> +
> + /* Before initiating the forceful flush, wait for any pending/ongoing
> + * flush to complete otherwise forceful flush may not happen, but wait
> + * can't be done for some paths like error state capture in which case
> + * take a chance & directly attempt the forceful flush.
> + */
> + if (can_wait)
> + flush_work(&dev_priv->guc.log.flush_work);
> +
> + /* Ask GuC to update the log buffer state */
> + host2guc_force_logbuffer_flush(&dev_priv->guc);
> +}
> +
> void i915_guc_unregister(struct drm_i915_private *dev_priv)
> {
> if (!i915.enable_guc_submission)
> diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
> index 8598f38..d7eda42 100644
> --- a/drivers/gpu/drm/i915/intel_guc.h
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -182,6 +182,7 @@ int i915_guc_wq_check_space(struct drm_i915_gem_request *rq);
> void i915_guc_submission_disable(struct drm_i915_private *dev_priv);
> void i915_guc_submission_fini(struct drm_i915_private *dev_priv);
> void i915_guc_capture_logs(struct drm_i915_private *dev_priv);
> +void i915_guc_flush_logs(struct drm_i915_private *dev_priv, bool can_wait);
> void i915_guc_register(struct drm_i915_private *dev_priv);
> void i915_guc_unregister(struct drm_i915_private *dev_priv);
>
>