[Intel-gfx] [PATCH] drm/i915/uc: Don't complain about FW versions when overridden

[Michal Wajdeczko]

On Sat, 07 Dec 2019 02:01:55 +0100, <John.C.Harrison at intel.com> wrote:

> From: John Harrison <John.C.Harrison at Intel.com>
>
> If a FW override is present then a version mis-match is actually
> ignored. The warning notice was still being printed, though. Which
> could confuse people by implying that the load had failed when it had
> actually succeeded. Given that the whole point of the override is to
> use different versions of FW, there isn't really much point reporting
> the mis-match.
>
> So, only print the notice when actually failing the load and avoid any
> potential confusion.
>
> v2: Original patch added a new 'ignore the previous notice' notice.
> Now it just suppresses the existing notice. Review feedback from
> Michal W.

This is not exactly from my previous feedback. I'm not sold that
suppressing message at fw fetch step about loading incompatible fw
(that may later fail to load) is a good idea. Note that loaded fw
version will be only printed when we successfully load it (see
intel_uc_init_hw). In case of any fw load failure (which is now
more likely to be caused by fw/drv version mismatch) we will not
print which fw version we attempted to load (except fw blob name,
which is not guaranteed to follow major.minor naming schema).

Michal

>
> Signed-off-by: John Harrison <John.C.Harrison at Intel.com>
> CC: Michal Wajdeczko <michal.wajdeczko at intel.com>
> ---
>  drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 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 66a30ab7044a..aa1b7ad02b56 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> @@ -351,16 +351,15 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw,  
> struct drm_i915_private *i915)
>  	uc_fw->minor_ver_found = FIELD_GET(CSS_SW_VERSION_UC_MINOR,
>  					   css->sw_version);
> -	if (uc_fw->major_ver_found != uc_fw->major_ver_wanted ||
> -	    uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) {
> +	if ((uc_fw->major_ver_found != uc_fw->major_ver_wanted ||
> +	     uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) &&
> +	    !intel_uc_fw_is_overridden(uc_fw)) {
>  		dev_notice(dev, "%s firmware %s: unexpected version: %u.%u !=  
> %u.%u\n",
>  			   intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
>  			   uc_fw->major_ver_found, uc_fw->minor_ver_found,
>  			   uc_fw->major_ver_wanted, uc_fw->minor_ver_wanted);
> -		if (!intel_uc_fw_is_overridden(uc_fw)) {
> -			err = -ENOEXEC;
> -			goto fail;
> -		}
> +		err = -ENOEXEC;
> +		goto fail;
>  	}
> 	obj = i915_gem_object_create_shmem_from_data(i915, fw->data, fw->size);