[PATCH 1/2] drm/i915/huc: Move pending status before debug

[PATCH 1/2] drm/i915/huc: Move pending status before debug

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

This avoids confusing log message sequences such as:

 [] i915/skl_huc_ver01_07_1398.bin fw status: fetch SUCCESS, load SUCCESS
 [] HuC DMA transfer wait over with ret 0
 [] i915/skl_huc_ver01_07_1398.bin fw status: fetch SUCCESS, load SUCCESS

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
Cc: Michał Winiarski <michal.winiarski@intel.com>
---
 drivers/gpu/drm/i915/intel_huc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
index e660109fc51e..8cf104350e5b 100644
--- a/drivers/gpu/drm/i915/intel_huc.c
+++ b/drivers/gpu/drm/i915/intel_huc.c
@@ -213,6 +213,8 @@ int intel_huc_load(struct drm_i915_private *dev_priv)
 	if (huc_fw->fetch_status == INTEL_UC_FIRMWARE_NONE)
 		return 0;
 
+	huc_fw->load_status = INTEL_UC_FIRMWARE_PENDING;
+
 	DRM_DEBUG_DRIVER("%s fw status: fetch %s, load %s\n",
 		huc_fw->path,
 		intel_uc_fw_status_repr(huc_fw->fetch_status),
@@ -222,8 +224,6 @@ int intel_huc_load(struct drm_i915_private *dev_priv)
 	    huc_fw->load_status == INTEL_UC_FIRMWARE_FAIL)
 		return -ENOEXEC;
 
-	huc_fw->load_status = INTEL_UC_FIRMWARE_PENDING;
-
 	switch (huc_fw->fetch_status) {
 	case INTEL_UC_FIRMWARE_FAIL:
 		/* something went wrong :( */
-- 
2.9.3

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

[PATCH 2/2] drm/i915/huc: Demote fw load to notice log level

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Error is probably too high since it is an optional feature so
it is better not to create too much commotion and especially
cause BAT runs to log errors if firmware is not present.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
Cc: Michał Winiarski <michal.winiarski@intel.com>
---
 drivers/gpu/drm/i915/intel_huc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
index 8cf104350e5b..8817d404c616 100644
--- a/drivers/gpu/drm/i915/intel_huc.c
+++ b/drivers/gpu/drm/i915/intel_huc.c
@@ -262,7 +262,7 @@ int intel_huc_load(struct drm_i915_private *dev_priv)
 	if (huc_fw->load_status == INTEL_UC_FIRMWARE_PENDING)
 		huc_fw->load_status = INTEL_UC_FIRMWARE_FAIL;
 
-	DRM_ERROR("Failed to complete HuC uCode load with ret %d\n", err);
+	DRM_NOTE("Failed to complete HuC uCode load with ret %d\n", err);
 
 	return err;
 }
-- 
2.9.3

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

Re: [PATCH 2/2] drm/i915/huc: Demote fw load to notice log level

[Michał Winiarski]

On Wed, Mar 08, 2017 at 04:33:11PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Error is probably too high since it is an optional feature so
> it is better not to create too much commotion and especially
> cause BAT runs to log errors if firmware is not present.

I've seen confused users asking about HuC. It's a good idea IMO.

Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>

-Michał
 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> Cc: Michał Winiarski <michal.winiarski@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_huc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
> index 8cf104350e5b..8817d404c616 100644
> --- a/drivers/gpu/drm/i915/intel_huc.c
> +++ b/drivers/gpu/drm/i915/intel_huc.c
> @@ -262,7 +262,7 @@ int intel_huc_load(struct drm_i915_private *dev_priv)
>  	if (huc_fw->load_status == INTEL_UC_FIRMWARE_PENDING)
>  		huc_fw->load_status = INTEL_UC_FIRMWARE_FAIL;
>  
> -	DRM_ERROR("Failed to complete HuC uCode load with ret %d\n", err);
> +	DRM_NOTE("Failed to complete HuC uCode load with ret %d\n", err);
>  
>  	return err;
>  }
> -- 
> 2.9.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for series starting with [1/2] drm/i915/huc: Move pending status before debug

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915/huc: Move pending status before debug
URL   : https://patchwork.freedesktop.org/series/20936/
State : failure

== Summary ==

Series 20936v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/20936/revisions/1/mbox/

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                pass       -> INCOMPLETE (fi-skl-6770hq)
                fail       -> PASS       (fi-snb-2600) fdo#100007
Test gem_exec_suspend:
        Subgroup basic-s3:
                fail       -> PASS       (fi-kbl-7500u) fdo#100124
        Subgroup basic-s4-devices:
                dmesg-warn -> PASS       (fi-bxt-t5700)
                fail       -> DMESG-WARN (fi-kbl-7500u) fdo#100099
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                fail       -> PASS       (fi-kbl-7500u) fdo#100123
        Subgroup suspend-read-crc-pipe-b:
                fail       -> PASS       (fi-kbl-7500u) fdo#100123
        Subgroup suspend-read-crc-pipe-c:
                fail       -> PASS       (fi-kbl-7500u) fdo#100044

fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
fdo#100124 https://bugs.freedesktop.org/show_bug.cgi?id=100124
fdo#100099 https://bugs.freedesktop.org/show_bug.cgi?id=100099
fdo#100123 https://bugs.freedesktop.org/show_bug.cgi?id=100123
fdo#100044 https://bugs.freedesktop.org/show_bug.cgi?id=100044

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time: 464s
fi-bsw-n3050     total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39  time: 610s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time: 529s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time: 616s
fi-byt-j1900     total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27  time: 513s
fi-byt-n2820     total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31  time: 503s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 443s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 436s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time: 434s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 507s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 488s
fi-kbl-7500u     total:278  pass:259  dwarn:1   dfail:0   fail:0   skip:18  time: 478s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 511s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time: 595s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time: 501s
fi-skl-6770hq    total:51   pass:49   dwarn:0   dfail:0   fail:0   skip:1   time: 0s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time: 549s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time: 428s

6f8585956c95605b9c797ffde8f65e408b8d1e33 drm-tip: 2017y-03m-08d-14h-47m-28s UTC integration manifest
3804703 drm/i915/huc: Demote fw load to notice log level
75f07b5 drm/i915/huc: Move pending status before debug

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4104/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915/huc: Move pending status before debug

[Michal Wajdeczko]

On Wed, Mar 08, 2017 at 04:33:10PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> This avoids confusing log message sequences such as:
> 
>  [] i915/skl_huc_ver01_07_1398.bin fw status: fetch SUCCESS, load SUCCESS
>  [] HuC DMA transfer wait over with ret 0
>  [] i915/skl_huc_ver01_07_1398.bin fw status: fetch SUCCESS, load SUCCESS
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> Cc: Michał Winiarski <michal.winiarski@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_huc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
> index e660109fc51e..8cf104350e5b 100644
> --- a/drivers/gpu/drm/i915/intel_huc.c
> +++ b/drivers/gpu/drm/i915/intel_huc.c
> @@ -213,6 +213,8 @@ int intel_huc_load(struct drm_i915_private *dev_priv)
>  	if (huc_fw->fetch_status == INTEL_UC_FIRMWARE_NONE)
>  		return 0;
>  
> +	huc_fw->load_status = INTEL_UC_FIRMWARE_PENDING;
> +
>  	DRM_DEBUG_DRIVER("%s fw status: fetch %s, load %s\n",
>  		huc_fw->path,
>  		intel_uc_fw_status_repr(huc_fw->fetch_status),
> @@ -222,8 +224,6 @@ int intel_huc_load(struct drm_i915_private *dev_priv)
>  	    huc_fw->load_status == INTEL_UC_FIRMWARE_FAIL)
>  		return -ENOEXEC;

Please also remove above "if" statement as this will never be true any more ;)

Btw, it looks that your patch will likely fix the case when previous fw
loading fails, as in case of reaching (fetch=SUCCESS,load=FAIL) status
we're always returning -ENOEXEC.

-Michal

>  
> -	huc_fw->load_status = INTEL_UC_FIRMWARE_PENDING;
> -
>  	switch (huc_fw->fetch_status) {
>  	case INTEL_UC_FIRMWARE_FAIL:
>  		/* something went wrong :( */
> -- 
> 2.9.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 1/2] drm/i915/huc: Move pending status before debug

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

This avoids confusing log message sequences such as:

 [] i915/skl_huc_ver01_07_1398.bin fw status: fetch SUCCESS, load SUCCESS
 [] HuC DMA transfer wait over with ret 0
 [] i915/skl_huc_ver01_07_1398.bin fw status: fetch SUCCESS, load SUCCESS

v2:

Michal and Michał noticed that there is an impossible condition
introduced by v1, which is also preventing  second attempts of
firmware load after reset. This has now been removed.

I also removed the fetch status check on the fail path which
looks redundant as well.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
---
I've just realized these two patches would create a conflict for
Arek's GuC/HuC tidy series. Feel free to incorporate them into
that work then if it is easier.

 drivers/gpu/drm/i915/intel_huc.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
index e660109fc51e..35124a257f20 100644
--- a/drivers/gpu/drm/i915/intel_huc.c
+++ b/drivers/gpu/drm/i915/intel_huc.c
@@ -213,17 +213,13 @@ int intel_huc_load(struct drm_i915_private *dev_priv)
 	if (huc_fw->fetch_status == INTEL_UC_FIRMWARE_NONE)
 		return 0;
 
+	huc_fw->load_status = INTEL_UC_FIRMWARE_PENDING;
+
 	DRM_DEBUG_DRIVER("%s fw status: fetch %s, load %s\n",
 		huc_fw->path,
 		intel_uc_fw_status_repr(huc_fw->fetch_status),
 		intel_uc_fw_status_repr(huc_fw->load_status));
 
-	if (huc_fw->fetch_status == INTEL_UC_FIRMWARE_SUCCESS &&
-	    huc_fw->load_status == INTEL_UC_FIRMWARE_FAIL)
-		return -ENOEXEC;
-
-	huc_fw->load_status = INTEL_UC_FIRMWARE_PENDING;
-
 	switch (huc_fw->fetch_status) {
 	case INTEL_UC_FIRMWARE_FAIL:
 		/* something went wrong :( */
@@ -259,8 +255,7 @@ int intel_huc_load(struct drm_i915_private *dev_priv)
 	return 0;
 
 fail:
-	if (huc_fw->load_status == INTEL_UC_FIRMWARE_PENDING)
-		huc_fw->load_status = INTEL_UC_FIRMWARE_FAIL;
+	huc_fw->load_status = INTEL_UC_FIRMWARE_FAIL;
 
 	DRM_ERROR("Failed to complete HuC uCode load with ret %d\n", err);
 
-- 
2.9.3

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

✗ Fi.CI.BAT: failure for series starting with [v2,1/2] drm/i915/huc: Move pending status before debug (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [v2,1/2] drm/i915/huc: Move pending status before debug (rev2)
URL   : https://patchwork.freedesktop.org/series/20936/
State : failure

== Summary ==

Series 20936v2 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/20936/revisions/2/mbox/

Test gem_close_race:
        Subgroup basic-threads:
                pass       -> INCOMPLETE (fi-skl-6770hq) fdo#99742
Test gem_exec_flush:
        Subgroup basic-uc-pro-default:
                pass       -> INCOMPLETE (fi-skl-6700hq)
Test gem_exec_suspend:
        Subgroup basic-s4-devices:
                dmesg-warn -> PASS       (fi-bxt-t5700) fdo#100125
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-c:
                pass       -> FAIL       (fi-skl-6700k)

fdo#99742 https://bugs.freedesktop.org/show_bug.cgi?id=99742
fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time: 463s
fi-bsw-n3050     total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39  time: 621s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time: 506s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time: 591s
fi-byt-j1900     total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27  time: 505s
fi-byt-n2820     total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31  time: 503s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 435s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 437s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time: 443s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 503s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 489s
fi-kbl-7500u     total:278  pass:259  dwarn:1   dfail:0   fail:0   skip:18  time: 480s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 504s
fi-skl-6700hq    total:53   pass:45   dwarn:0   dfail:0   fail:0   skip:7   time: 0s
fi-skl-6700k     total:278  pass:255  dwarn:4   dfail:0   fail:1   skip:18  time: 497s
fi-skl-6770hq    total:12   pass:11   dwarn:0   dfail:0   fail:0   skip:0   time: 0s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time: 551s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time: 417s

226d87634d5f7aa9104d1829c7ef7f7133d94e39 drm-tip: 2017y-03m-08d-22h-43m-24s UTC integration manifest
12b702c drm/i915/huc: Demote fw load to notice log level
fde1d9d drm/i915/huc: Move pending status before debug

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4108/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx