[Intel-gfx] [PATCH 07/15] drm/i915/guc: Flush directly in log unregister

[Sagar Arun Kamble]

On 2/27/2018 6:22 PM, Michał Winiarski wrote:
> Having both guc_flush_logs and guc_log_flush functions is confusing.
> While we could just rename things, guc_flush_logs implementation is
> quite simple. Let's get rid of it and move its content to unregister.
>
> Signed-off-by: Michał Winiarski <michal.winiarski at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble at intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble at intel.com>

this patch reminds me of need to do guc_log_flush in capture_uc_state 
prior to reset in case user wants to read
updated log vma from error state. is this need valid?
> ---
>   drivers/gpu/drm/i915/intel_guc_log.c | 35 +++++++++++++++--------------------
>   1 file changed, 15 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
> index cbbdb400fa17..0800c5317510 100644
> --- a/drivers/gpu/drm/i915/intel_guc_log.c
> +++ b/drivers/gpu/drm/i915/intel_guc_log.c
> @@ -443,25 +443,6 @@ static void guc_log_capture_logs(struct intel_guc *guc)
>   	intel_runtime_pm_put(dev_priv);
>   }
>   
> -static void guc_flush_logs(struct intel_guc *guc)
> -{
> -	struct drm_i915_private *dev_priv = guc_to_i915(guc);
> -
> -	/*
> -	 * Before initiating the forceful flush, wait for any pending/ongoing
> -	 * flush to complete otherwise forceful flush may not actually happen.
> -	 */
> -	flush_work(&guc->log.runtime.flush_work);
> -
> -	/* Ask GuC to update the log buffer state */
> -	intel_runtime_pm_get(dev_priv);
> -	guc_log_flush(guc);
> -	intel_runtime_pm_put(dev_priv);
> -
> -	/* GuC would have updated log buffer by now, so capture it */
> -	guc_log_capture_logs(guc);
> -}
> -
>   int intel_guc_log_create(struct intel_guc *guc)
>   {
>   	struct i915_vma *vma;
> @@ -628,14 +609,28 @@ int intel_guc_log_register(struct intel_guc *guc)
>   
>   void intel_guc_log_unregister(struct intel_guc *guc)
>   {
> +	struct drm_i915_private *dev_priv = guc_to_i915(guc);
> +
>   	guc_log_flush_irq_disable(guc);
> +
> +	/*
> +	 * Before initiating the forceful flush, wait for any pending/ongoing
> +	 * flush to complete otherwise forceful flush may not actually happen.
> +	 */
> +	flush_work(&guc->log.runtime.flush_work);
> +
>   	/*
>   	 * Once logging is disabled, GuC won't generate logs & send an
>   	 * interrupt. But there could be some data in the log buffer
>   	 * which is yet to be captured. So request GuC to update the log
>   	 * buffer state and then collect the left over logs.
>   	 */
> -	guc_flush_logs(guc);
> +	intel_runtime_pm_get(dev_priv);
> +	guc_log_flush(guc);
> +	intel_runtime_pm_put(dev_priv);
> +
> +	/* GuC would have updated log buffer by now, so capture it */
> +	guc_log_capture_logs(guc);
>   
>   	mutex_lock(&guc->log.runtime.lock);
>   

-- 
Thanks,
Sagar