[PATCH] drm/i915/uc: Extra info notice about FW version mis-match vs overrides

[PATCH] drm/i915/uc: Extra info notice about FW version mis-match vs overrides

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

If a FW override is present then a version mis-match is actually
ignored. The warning message was still being printed, though. Which
could confuse people by implying that the load had failed due to the
mis-match when actually something else had failed.

This patch adds an extra message to say that the mis-match is being
ignored if an override is present.

Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 3 +++
 1 file changed, 3 insertions(+)

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..c1ae807b07ae 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -361,6 +361,9 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw, struct drm_i915_private *i915)
 			err = -ENOEXEC;
 			goto fail;
 		}
+
+		dev_notice(dev, "%s firmware %s: Firmware override so ignoring version mis-match\n",
+			   intel_uc_fw_type_repr(uc_fw->type), uc_fw->path);
 	}
 
 	obj = i915_gem_object_create_shmem_from_data(i915, fw->data, fw->size);
-- 
2.21.0.5.gaeb582a983

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915/uc: Extra info notice about FW version mis-match vs overrides

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

If a FW override is present then a version mis-match is actually
ignored. The warning message was still being printed, though. Which
could confuse people by implying that the load had failed due to the
mis-match when actually something else had failed.

This patch adds an extra message to say that the mis-match is being
ignored if an override is present.

Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 3 +++
 1 file changed, 3 insertions(+)

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..c1ae807b07ae 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -361,6 +361,9 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw, struct drm_i915_private *i915)
 			err = -ENOEXEC;
 			goto fail;
 		}
+
+		dev_notice(dev, "%s firmware %s: Firmware override so ignoring version mis-match\n",
+			   intel_uc_fw_type_repr(uc_fw->type), uc_fw->path);
 	}
 
 	obj = i915_gem_object_create_shmem_from_data(i915, fw->data, fw->size);
-- 
2.21.0.5.gaeb582a983

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/uc: Extra info notice about FW version mis-match vs overrides

[Michal Wajdeczko]

On Thu, 21 Nov 2019 01:27:23 +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 message was still being printed, though. Which

It wasn't a "warning", just "notice"

> could confuse people by implying that the load had failed due to the
> mis-match when actually something else had failed.

The mis-match still might be a reason why something else failed.
If there is possible confusing, it's likely due to lack of or incomplete
message from this other failure point. So we should make sure that all
failure points correctly indicate the failure reason to avoid confusing.
Do you recall what this other confusing failure was?

>
> This patch adds an extra message to say that the mis-match is being
> ignored if an override is present.
>
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> ---
>  drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> 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..c1ae807b07ae 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> @@ -361,6 +361,9 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw,  
> struct drm_i915_private *i915)
>  			err = -ENOEXEC;
>  			goto fail;
>  		}
> +
> +		dev_notice(dev, "%s firmware %s: Firmware override so ignoring  
> version mis-match\n",
> +			   intel_uc_fw_type_repr(uc_fw->type), uc_fw->path);

If you still want to include clear statement about mis-match being ignored,
then maybe instead of adding new message it could be combined with old one:

         if (uc_fw->major_ver_found != uc_fw->major_ver_wanted ||
             uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) {
-               dev_notice(dev, "%s firmware %s: unexpected version: %u.%u  
!= %u.%u\n",
+               dev_notice(dev, "%s firmware %s: %s: %u.%u != %u.%u\n",
                            intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
+                          intel_uc_fw_is_overridden(uc_fw) ?
+                          "ignoring unexpected version" : "wrong version",
                            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)) {

Michal
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/uc: Extra info notice about FW version mis-match vs overrides

[Michal Wajdeczko]

On Thu, 21 Nov 2019 01:27:23 +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 message was still being printed, though. Which

It wasn't a "warning", just "notice"

> could confuse people by implying that the load had failed due to the
> mis-match when actually something else had failed.

The mis-match still might be a reason why something else failed.
If there is possible confusing, it's likely due to lack of or incomplete
message from this other failure point. So we should make sure that all
failure points correctly indicate the failure reason to avoid confusing.
Do you recall what this other confusing failure was?

>
> This patch adds an extra message to say that the mis-match is being
> ignored if an override is present.
>
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> ---
>  drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> 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..c1ae807b07ae 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> @@ -361,6 +361,9 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw,  
> struct drm_i915_private *i915)
>  			err = -ENOEXEC;
>  			goto fail;
>  		}
> +
> +		dev_notice(dev, "%s firmware %s: Firmware override so ignoring  
> version mis-match\n",
> +			   intel_uc_fw_type_repr(uc_fw->type), uc_fw->path);

If you still want to include clear statement about mis-match being ignored,
then maybe instead of adding new message it could be combined with old one:

         if (uc_fw->major_ver_found != uc_fw->major_ver_wanted ||
             uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) {
-               dev_notice(dev, "%s firmware %s: unexpected version: %u.%u  
!= %u.%u\n",
+               dev_notice(dev, "%s firmware %s: %s: %u.%u != %u.%u\n",
                            intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
+                          intel_uc_fw_is_overridden(uc_fw) ?
+                          "ignoring unexpected version" : "wrong version",
                            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)) {

Michal
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/uc: Extra info notice about FW version mis-match vs overrides

[John Harrison]

On 11/21/2019 02:43, Michal Wajdeczko wrote:
> On Thu, 21 Nov 2019 01:27:23 +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 message was still being printed, though. Which
>
> It wasn't a "warning", just "notice"
>
>> could confuse people by implying that the load had failed due to the
>> mis-match when actually something else had failed.
>
> The mis-match still might be a reason why something else failed.
> If there is possible confusing, it's likely due to lack of or incomplete
> message from this other failure point. So we should make sure that all
> failure points correctly indicate the failure reason to avoid confusing.
> Do you recall what this other confusing failure was?

Sorry, bad commit message comment. The point was that you could 
successfully load the GuC FW but then something entirely unrelated fails 
(with or without appropriate error message). However, the first apparent 
failure in dmesg is the GuC version mis-match. Therefore a user (or even 
developer) might assume that all subsequent issues are caused by the FW 
mismatch causing the GuC to not load at all and hence not investigate 
any later messages until this first one is fixed/understood.

So the point is just to avoid people wasting time investigating 
something that is not actually an error.


>
>>
>> This patch adds an extra message to say that the mis-match is being
>> ignored if an override is present.
>>
>> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
>> ---
>>  drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> 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..c1ae807b07ae 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>> @@ -361,6 +361,9 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw, 
>> struct drm_i915_private *i915)
>>              err = -ENOEXEC;
>>              goto fail;
>>          }
>> +
>> +        dev_notice(dev, "%s firmware %s: Firmware override so 
>> ignoring version mis-match\n",
>> +               intel_uc_fw_type_repr(uc_fw->type), uc_fw->path);
>
> If you still want to include clear statement about mis-match being 
> ignored,
> then maybe instead of adding new message it could be combined with old 
> one:
>
Or maybe just move the mis-match notice into the 'goto fail' section and 
not print anything at all in the case of an override. On the grounds 
that if someone is specifying an override then it is almost certainly 
because the default version is not what they want. So yes, it obviously 
is going to be mis-match.


> if (uc_fw->major_ver_found != uc_fw->major_ver_wanted ||
>             uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) {
> -               dev_notice(dev, "%s firmware %s: unexpected version: 
> %u.%u != %u.%u\n",
> +               dev_notice(dev, "%s firmware %s: %s: %u.%u != %u.%u\n",
>                            intel_uc_fw_type_repr(uc_fw->type), 
> uc_fw->path,
> +                          intel_uc_fw_is_overridden(uc_fw) ?
> +                          "ignoring unexpected version" : "wrong 
> version",
>                            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)) {
>
> Michal

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[John.C.Harrison]

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

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);
-- 
2.21.0.5.gaeb582a983

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/uc: Extra info notice about FW version mis-match vs overrides

[Michal Wajdeczko]

On Fri, 06 Dec 2019 22:21:37 +0100, John Harrison  
<John.C.Harrison@intel.com> wrote:

> On 11/21/2019 02:43, Michal Wajdeczko wrote:
>> On Thu, 21 Nov 2019 01:27:23 +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 message was still being printed, though. Which
>>
>> It wasn't a "warning", just "notice"
>>
>>> could confuse people by implying that the load had failed due to the
>>> mis-match when actually something else had failed.
>>
>> The mis-match still might be a reason why something else failed.
>> If there is possible confusing, it's likely due to lack of or incomplete
>> message from this other failure point. So we should make sure that all
>> failure points correctly indicate the failure reason to avoid confusing.
>> Do you recall what this other confusing failure was?
>
> Sorry, bad commit message comment. The point was that you could  
> successfully load the GuC FW but then something entirely unrelated fails  
> (with or without appropriate error message). However, the first apparent  
> failure in dmesg is the GuC version mis-match. Therefore a user (or even

again, it wasn't error message, just a notice to clearly indicate that
fw version being used is not matching tested and guaranteed configuration.

> developer) might assume that all subsequent issues are caused by the FW  
> mismatch causing the GuC to not load at all and hence not investigate

We fail to fetch mismatched fw only if user is trying to cheat with us
(by renaming mismatched fw blob to expected filename). In case of
override, fetch will continue, but we may fail due to hard to predict
ABI incompatibility/breakage.

> any later messages until this first one is fixed/understood.

If there are any later errors, we have to account that these might
be caused by mismatched fw, otherwise, we would use that other fw
as default one.

>
> So the point is just to avoid people wasting time investigating  
> something that is not actually an error.

Ordinary user is never expected to see this notice.

Users/developers that override fw path must expect this notice.
There is a still risk of potential issues caused by loading
mismatched firmware that will not have proper error message.

>
>
>>
>>>
>>> This patch adds an extra message to say that the mis-match is being
>>> ignored if an override is present.
>>>
>>> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
>>> ---
>>>  drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> 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..c1ae807b07ae 100644
>>> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>>> @@ -361,6 +361,9 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw,  
>>> struct drm_i915_private *i915)
>>>              err = -ENOEXEC;
>>>              goto fail;
>>>          }
>>> +
>>> +        dev_notice(dev, "%s firmware %s: Firmware override so  
>>> ignoring version mis-match\n",
>>> +               intel_uc_fw_type_repr(uc_fw->type), uc_fw->path);
>>
>> If you still want to include clear statement about mis-match being  
>> ignored,
>> then maybe instead of adding new message it could be combined with old  
>> one:
>>
> Or maybe just move the mis-match notice into the 'goto fail' section and  
> not print anything at all in the case of an override. On the grounds  
> that if someone is specifying an override then it is almost certainly  
> because the default version is not what they want. So yes, it obviously  
> is going to be mis-match.

You can safely (and silently) override fw according to fw version
compatibility schema: same major and newer minor are OK to use.

This notice message was capturing cases when you try to bend the rules:
use mismatched major (as it is unlikely to work, since it indicates no
backward compatibility on ABI level, otherwise major version should stay
the same) or use of legacy minor (possibly will also not work due to
missing feature that is expected/used by driver).

Michal

>
>
>> if (uc_fw->major_ver_found != uc_fw->major_ver_wanted ||
>>             uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) {
>> -               dev_notice(dev, "%s firmware %s: unexpected version:  
>> %u.%u != %u.%u\n",
>> +               dev_notice(dev, "%s firmware %s: %s: %u.%u != %u.%u\n",
>>                            intel_uc_fw_type_repr(uc_fw->type),  
>> uc_fw->path,
>> +                          intel_uc_fw_is_overridden(uc_fw) ?
>> +                          "ignoring unexpected version" : "wrong  
>> version",
>>                            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)) {
>>
>> Michal
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [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@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);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v3] drm/i915/uc: More useful FW version mis-match notice

[John.C.Harrison]

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. So add an extra tag to the notice to say whether
the mis-match caused a failure to load (normal case) or was ignored
(override case).

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.
v3: Always print the notice, but add a tag about override or failure.
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 | 6 ++++--
 1 file changed, 4 insertions(+), 2 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..deae1e89de0e 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -353,10 +353,12 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw, struct drm_i915_private *i915)
 
 	if (uc_fw->major_ver_found != uc_fw->major_ver_wanted ||
 	    uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) {
-		dev_notice(dev, "%s firmware %s: unexpected version: %u.%u != %u.%u\n",
+		dev_notice(dev, "%s firmware %s: unexpected version: %u.%u != %u.%u, %s\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);
+			   uc_fw->major_ver_wanted, uc_fw->minor_ver_wanted,
+			   intel_uc_fw_is_overridden(uc_fw) ?
+			   "ignoring" : "aborting");
 		if (!intel_uc_fw_is_overridden(uc_fw)) {
 			err = -ENOEXEC;
 			goto fail;
-- 
2.21.0.5.gaeb582a983

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/uc: Extra info notice about FW version mis-match vs overrides

[John Harrison]

Oops. Ignore this email. Accidentally sent the same thing twice.

On 11/20/2019 16:31, 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 message was still being printed, though. Which
> could confuse people by implying that the load had failed due to the
> mis-match when actually something else had failed.
>
> This patch adds an extra message to say that the mis-match is being
> ignored if an override is present.
>
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> ---
>   drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> 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..c1ae807b07ae 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> @@ -361,6 +361,9 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw, struct drm_i915_private *i915)
>   			err = -ENOEXEC;
>   			goto fail;
>   		}
> +
> +		dev_notice(dev, "%s firmware %s: Firmware override so ignoring version mis-match\n",
> +			   intel_uc_fw_type_repr(uc_fw->type), uc_fw->path);
>   	}
>   
>   	obj = i915_gem_object_create_shmem_from_data(i915, fw->data, fw->size);

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915/uc: Extra info notice about FW version mis-match vs overrides

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

If a FW override is present then a version mis-match is actually
ignored. The warning message was still being printed, though. Which
could confuse people by implying that the load had failed due to the
mis-match when actually something else had failed.

This patch adds an extra message to say that the mis-match is being
ignored if an override is present.

Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 3 +++
 1 file changed, 3 insertions(+)

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..c1ae807b07ae 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -361,6 +361,9 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw, struct drm_i915_private *i915)
 			err = -ENOEXEC;
 			goto fail;
 		}
+
+		dev_notice(dev, "%s firmware %s: Firmware override so ignoring version mis-match\n",
+			   intel_uc_fw_type_repr(uc_fw->type), uc_fw->path);
 	}
 
 	obj = i915_gem_object_create_shmem_from_data(i915, fw->data, fw->size);
-- 
2.21.0.5.gaeb582a983

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx