[Intel-gfx] [PATCH] drm/i915/uc: Acknowledge successful firmware load to the user

[Michal Wajdeczko]

On Fri, 19 Jul 2019 16:14:27 +0200, Chris Wilson  
<chris at chris-wilson.co.uk> wrote:

> Let the user know that we have successfully loaded the firmware from the
> filesystem and downloaded it to the uC, with a info-level message. In
> the process, we can replace the random pointer print out and
> uninteresting debug events.
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> ---
>  drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 17 +++++------------
>  1 file changed, 5 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c  
> b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> index 8ce7210907c0..f04fa051e333 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> @@ -61,9 +61,6 @@ void intel_uc_fw_fetch(struct drm_i915_private  
> *dev_priv,
>  			 intel_uc_fw_type_repr(uc_fw->type), uc_fw->path);
> 	uc_fw->fetch_status = INTEL_UC_FIRMWARE_PENDING;
> -	DRM_DEBUG_DRIVER("%s fw fetch %s\n",
> -			 intel_uc_fw_type_repr(uc_fw->type),
> -			 intel_uc_fw_status_repr(uc_fw->fetch_status));
> 	err = request_firmware(&fw, uc_fw->path, &pdev->dev);
>  	if (err) {
> @@ -72,9 +69,6 @@ void intel_uc_fw_fetch(struct drm_i915_private  
> *dev_priv,
>  		goto fail;
>  	}
> -	DRM_DEBUG_DRIVER("%s fw size %zu ptr %p\n",
> -			 intel_uc_fw_type_repr(uc_fw->type), fw->size, fw);
> -
>  	/* Check the size of the blob before examining buffer contents */
>  	if (fw->size < sizeof(struct uc_css_header)) {
>  		DRM_WARN("%s: Unexpected firmware size (%zu, min %zu)\n",
> @@ -173,18 +167,17 @@ void intel_uc_fw_fetch(struct drm_i915_private  
> *dev_priv,
>  	uc_fw->obj = obj;
>  	uc_fw->size = fw->size;
>  	uc_fw->fetch_status = INTEL_UC_FIRMWARE_SUCCESS;
> -	DRM_DEBUG_DRIVER("%s fw fetch %s\n",
> -			 intel_uc_fw_type_repr(uc_fw->type),
> -			 intel_uc_fw_status_repr(uc_fw->fetch_status));
> +
> +	dev_info(dev_priv->drm.dev,
> +		 "%s: Loaded firmware '%s', v%u.%u\n",
> +		 intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
> +		 uc_fw->major_ver_found, uc_fw->minor_ver_found);

Here we can only confirm that fw was "fetched" but it's too far
to say it's "loaded" as we will load it to the uc in next steps.

btw, in function intel_uc_fw_upload() we already have:

	DRM_INFO("%s: Loaded firmware %s (version %u.%u)\n",
		 intel_uc_fw_type_repr(uc_fw->type),
		 uc_fw->path,
		 uc_fw->major_ver_found, uc_fw->minor_ver_found);

what is wrong with this one ? (except that it doesn't use dev_info)

> 	release_firmware(fw);
>  	return;
> fail:
>  	uc_fw->fetch_status = INTEL_UC_FIRMWARE_FAIL;
> -	DRM_DEBUG_DRIVER("%s fw fetch %s\n",
> -			 intel_uc_fw_type_repr(uc_fw->type),
> -			 intel_uc_fw_status_repr(uc_fw->fetch_status));
> 	DRM_WARN("%s: Failed to fetch firmware %s (error %d)\n",
>  		 intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, err);