[Intel-gfx] [PATCH] drm/i915/guc: Log significant events at the info level

[Chris Wilson]

On Mon, Feb 06, 2017 at 11:32:06AM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> 
> Currently to establish whether GuC firmware has been loaded or
> submission enabled (default DRM log level), one has to detect
> the absence of the message saying that the load has been skipped
> and infer the opposite.
> 
> It is better to log the fact GuC firmware has been loaded and/or
> submission enabled explicitly to avoid any guesswork when looking
> at the logs.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Cc: Arkadiusz Hiler <arkadiusz.hiler at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_guc_loader.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
> index 2f1cf9aea04e..ccf55d6a164e 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -520,10 +520,6 @@ int intel_guc_setup(struct drm_i915_private *dev_priv)
>  
>  	guc_fw->load_status = INTEL_UC_FIRMWARE_SUCCESS;
>  
> -	DRM_DEBUG_DRIVER("GuC fw status: fetch %s, load %s\n",
> -		intel_uc_fw_status_repr(guc_fw->fetch_status),
> -		intel_uc_fw_status_repr(guc_fw->load_status));
> -
>  	intel_guc_auth_huc(dev_priv);
>  
>  	if (i915.enable_guc_submission) {
> @@ -536,6 +532,11 @@ int intel_guc_setup(struct drm_i915_private *dev_priv)
>  		guc_interrupts_capture(dev_priv);
>  	}
>  
> +	DRM_INFO("GuC %s (firmware %s [%u.%u])\n",
> +		 i915.enable_guc_submission ? "submission enabled" : "loaded",
> +		 guc_fw->path,
> +		 guc_fw->major_ver_wanted, guc_fw->minor_ver_wanted);

I can see that you've considered that this will now be consumed by
users and have made the language more understandable. I think you need
to explain what the numbers are, i.e. "[version %u.%u]".

Other than that, looks good, we don't confuse the user when guc isn't
being used (so that don't start trying to fix it and probably breaking
their system in the process! ;),

Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>

We are missing the guc version in the error state, if you are feeling on
a roll...
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre