drm/i915/uc: Don't complain about FW versions when overridden
diff mbox series

Message ID 20191207010155.24943-1-John.C.Harrison@Intel.com
State New
Headers show
Series
  • drm/i915/uc: Don't complain about FW versions when overridden
Related show

Commit Message

John Harrison Dec. 7, 2019, 1:01 a.m. UTC
From: John Harrison <John.C.Harrison@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.

Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
CC: Michal Wajdeczko <michal.wajdeczko@intel.com>
---
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

Comments

Michal Wajdeczko Dec. 7, 2019, 8:19 p.m. UTC | #1
On Sat, 07 Dec 2019 02:01:55 +0100, <John.C.Harrison@intel.com> wrote:

> From: John Harrison <John.C.Harrison@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@Intel.com>
> CC: Michal Wajdeczko <michal.wajdeczko@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);

Patch
diff mbox series

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);