[igt-dev] [PATCH i-g-t] lib: Handle chamelium failures gracefully

[igt-dev] [PATCH i-g-t] lib: Handle chamelium failures gracefully

[Chris Wilson]

Don't cause CI to abort a run if an _unrelated_ fixture checks for a
display and chamelium is dead.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 lib/igt_kms.c | 54 +++++++++++++++++++++++++++++++++++----------------
 1 file changed, 37 insertions(+), 17 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index e5d8e82c9..65b36c338 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1922,6 +1922,40 @@ __get_crtc_mask_for_pipe(drmModeRes *resources, igt_pipe_t *pipe)
 	return (1 << offset);
 }
 
+static bool chamelium_ok(int drm_fd)
+{
+	bool ok = true;
+#ifdef HAVE_CHAMELIUM
+	struct chamelium *chamelium;
+
+	chamelium = chamelium_init_rpc_only();
+	if (chamelium) {
+		ok = false;
+
+		if (!chamelium_wait_reachable(chamelium, 20)) {
+			igt_debug("cannot reach the configured chamelium!\n");
+			goto out;
+		}
+
+		if (!chamelium_plug_all(chamelium)) {
+			igt_debug("failed to plug all the chamelium ports!\n");
+			goto out;
+		}
+
+		if (!chamelium_wait_all_configured_ports_connected(chamelium, drm_fd)) {
+			igt_debug("not all configured chamelium ports are connected!\n");
+			goto out;
+		}
+
+		ok = true;
+out:
+		chamelium_deinit_rpc_only(chamelium);
+	}
+#endif
+
+	return ok;
+}
+
 /**
  * igt_display_require:
  * @display: a pointer to an #igt_display_t structure
@@ -1944,6 +1978,9 @@ void igt_display_require(igt_display_t *display, int drm_fd)
 
 	LOG_INDENT(display, "init");
 
+	if (!chamelium_ok(drm_fd))
+		goto out;
+
 	display->drm_fd = drm_fd;
 	is_i915_dev = is_i915_device(drm_fd);
 
@@ -1953,23 +1990,6 @@ void igt_display_require(igt_display_t *display, int drm_fd)
 	if (!resources)
 		goto out;
 
-#ifdef HAVE_CHAMELIUM
-	{
-		struct chamelium *chamelium;
-
-		chamelium = chamelium_init_rpc_only();
-		if (chamelium) {
-			igt_abort_on_f(!chamelium_wait_reachable(chamelium, 20),
-				       "cannot reach the configured chamelium!\n");
-			igt_abort_on_f(!chamelium_plug_all(chamelium),
-				       "failed to plug all the chamelium ports!\n");
-			igt_abort_on_f(!chamelium_wait_all_configured_ports_connected(chamelium, drm_fd),
-				       "not all configured chamelium ports are connected!\n");
-			chamelium_deinit_rpc_only(chamelium);
-		}
-	}
-#endif
-
 	/*
 	 * With non-contiguous pipes display, crtc mapping is not always same
 	 * as pipe mapping, In i915 pipe is enum id of i915's crtc object.
-- 
2.29.2

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✗ Fi.CI.BAT: failure for lib: Handle chamelium failures gracefully

[Patchwork]

[-- Attachment #1.1: Type: text/plain, Size: 5955 bytes --]

== Series Details ==

Series: lib: Handle chamelium failures gracefully
URL   : https://patchwork.freedesktop.org/series/83646/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_9298 -> IGTPW_5148
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_5148 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_5148, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5148/index.html

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in IGTPW_5148:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-icl-u2:          NOTRUN -> [SKIP][1] +38 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5148/fi-icl-u2/igt@kms_chamelium@hdmi-hpd-fast.html

  * igt@kms_cursor_legacy@basic-flip-before-cursor-legacy:
    - fi-cml-u2:          NOTRUN -> [SKIP][2] +37 similar issues
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5148/fi-cml-u2/igt@kms_cursor_legacy@basic-flip-before-cursor-legacy.html

  
Known issues
------------

  Here are the changes found in IGTPW_5148 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_store@basic:
    - fi-tgl-y:           [PASS][3] -> [DMESG-WARN][4] ([i915#402]) +2 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9298/fi-tgl-y/igt@gem_exec_store@basic.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5148/fi-tgl-y/igt@gem_exec_store@basic.html

  * igt@i915_module_load@reload:
    - fi-byt-j1900:       [PASS][5] -> [DMESG-WARN][6] ([i915#1982])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9298/fi-byt-j1900/igt@i915_module_load@reload.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5148/fi-byt-j1900/igt@i915_module_load@reload.html

  * igt@kms_busy@basic@flip:
    - fi-kbl-soraka:      [PASS][7] -> [DMESG-WARN][8] ([i915#1982])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9298/fi-kbl-soraka/igt@kms_busy@basic@flip.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5148/fi-kbl-soraka/igt@kms_busy@basic@flip.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c:
    - fi-tgl-y:           [PASS][9] -> [DMESG-WARN][10] ([i915#1982])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9298/fi-tgl-y/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5148/fi-tgl-y/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html

  
#### Possible fixes ####

  * igt@debugfs_test@read_all_entries:
    - fi-icl-u2:          [{ABORT}][11] ([i915#1814]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9298/fi-icl-u2/igt@debugfs_test@read_all_entries.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5148/fi-icl-u2/igt@debugfs_test@read_all_entries.html
    - fi-cml-u2:          [{ABORT}][13] ([i915#1814]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9298/fi-cml-u2/igt@debugfs_test@read_all_entries.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5148/fi-cml-u2/igt@debugfs_test@read_all_entries.html

  * igt@i915_selftest@live@execlists:
    - fi-cml-s:           [INCOMPLETE][15] -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9298/fi-cml-s/igt@i915_selftest@live@execlists.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5148/fi-cml-s/igt@i915_selftest@live@execlists.html

  * igt@kms_flip@basic-flip-vs-wf_vblank@a-edp1:
    - fi-bsw-kefka:       [FAIL][17] ([i915#2122]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9298/fi-bsw-kefka/igt@kms_flip@basic-flip-vs-wf_vblank@a-edp1.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5148/fi-bsw-kefka/igt@kms_flip@basic-flip-vs-wf_vblank@a-edp1.html

  * igt@vgem_basic@dmabuf-export:
    - fi-tgl-y:           [DMESG-WARN][19] ([i915#402]) -> [PASS][20] +1 similar issue
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9298/fi-tgl-y/igt@vgem_basic@dmabuf-export.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5148/fi-tgl-y/igt@vgem_basic@dmabuf-export.html

  
#### Warnings ####

  * igt@gem_exec_suspend@basic-s3:
    - fi-tgl-y:           [DMESG-WARN][21] ([i915#2411] / [i915#402]) -> [DMESG-WARN][22] ([i915#2411])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9298/fi-tgl-y/igt@gem_exec_suspend@basic-s3.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5148/fi-tgl-y/igt@gem_exec_suspend@basic-s3.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [i915#1814]: https://gitlab.freedesktop.org/drm/intel/issues/1814
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2122]: https://gitlab.freedesktop.org/drm/intel/issues/2122
  [i915#2411]: https://gitlab.freedesktop.org/drm/intel/issues/2411
  [i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402


Participating hosts (43 -> 39)
------------------------------

  Missing    (4): fi-ilk-m540 fi-bsw-cyan fi-bdw-samus fi-hsw-4200u 


Build changes
-------------

  * CI: CI-20190529 -> None
  * IGT: IGT_5839 -> IGTPW_5148

  CI-20190529: 20190529
  CI_DRM_9298: 2ce495e94d74a40f558359b51ff3aee60a7d4801 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_5148: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5148/index.html
  IGT_5839: 2dbd64a6301e36eb432bc50ad7021fabaeebd1f4 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5148/index.html

[-- Attachment #1.2: Type: text/html, Size: 7274 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t] lib: Handle chamelium failures gracefully

[Petri Latvala]

On Mon, Nov 09, 2020 at 03:16:58PM +0000, Chris Wilson wrote:
> Don't cause CI to abort a run if an _unrelated_ fixture checks for a
> display and chamelium is dead.

If there's a chamelium configured and it's dead, we do want to abort
though.

What's an unrelated fixture in this context?


-- 
Petri Latvala



> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  lib/igt_kms.c | 54 +++++++++++++++++++++++++++++++++++----------------
>  1 file changed, 37 insertions(+), 17 deletions(-)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index e5d8e82c9..65b36c338 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -1922,6 +1922,40 @@ __get_crtc_mask_for_pipe(drmModeRes *resources, igt_pipe_t *pipe)
>  	return (1 << offset);
>  }
>  
> +static bool chamelium_ok(int drm_fd)
> +{
> +	bool ok = true;
> +#ifdef HAVE_CHAMELIUM
> +	struct chamelium *chamelium;
> +
> +	chamelium = chamelium_init_rpc_only();
> +	if (chamelium) {
> +		ok = false;
> +
> +		if (!chamelium_wait_reachable(chamelium, 20)) {
> +			igt_debug("cannot reach the configured chamelium!\n");
> +			goto out;
> +		}
> +
> +		if (!chamelium_plug_all(chamelium)) {
> +			igt_debug("failed to plug all the chamelium ports!\n");
> +			goto out;
> +		}
> +
> +		if (!chamelium_wait_all_configured_ports_connected(chamelium, drm_fd)) {
> +			igt_debug("not all configured chamelium ports are connected!\n");
> +			goto out;
> +		}
> +
> +		ok = true;
> +out:
> +		chamelium_deinit_rpc_only(chamelium);
> +	}
> +#endif
> +
> +	return ok;
> +}
> +
>  /**
>   * igt_display_require:
>   * @display: a pointer to an #igt_display_t structure
> @@ -1944,6 +1978,9 @@ void igt_display_require(igt_display_t *display, int drm_fd)
>  
>  	LOG_INDENT(display, "init");
>  
> +	if (!chamelium_ok(drm_fd))
> +		goto out;
> +
>  	display->drm_fd = drm_fd;
>  	is_i915_dev = is_i915_device(drm_fd);
>  
> @@ -1953,23 +1990,6 @@ void igt_display_require(igt_display_t *display, int drm_fd)
>  	if (!resources)
>  		goto out;
>  
> -#ifdef HAVE_CHAMELIUM
> -	{
> -		struct chamelium *chamelium;
> -
> -		chamelium = chamelium_init_rpc_only();
> -		if (chamelium) {
> -			igt_abort_on_f(!chamelium_wait_reachable(chamelium, 20),
> -				       "cannot reach the configured chamelium!\n");
> -			igt_abort_on_f(!chamelium_plug_all(chamelium),
> -				       "failed to plug all the chamelium ports!\n");
> -			igt_abort_on_f(!chamelium_wait_all_configured_ports_connected(chamelium, drm_fd),
> -				       "not all configured chamelium ports are connected!\n");
> -			chamelium_deinit_rpc_only(chamelium);
> -		}
> -	}
> -#endif
> -
>  	/*
>  	 * With non-contiguous pipes display, crtc mapping is not always same
>  	 * as pipe mapping, In i915 pipe is enum id of i915's crtc object.
> -- 
> 2.29.2
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t] lib: Handle chamelium failures gracefully

[Chris Wilson]

Quoting Petri Latvala (2020-11-11 07:52:10)
> On Mon, Nov 09, 2020 at 03:16:58PM +0000, Chris Wilson wrote:
> > Don't cause CI to abort a run if an _unrelated_ fixture checks for a
> > display and chamelium is dead.
> 
> If there's a chamelium configured and it's dead, we do want to abort
> though.

If there's no display, we don't run display tests. The GPUs are still
very interesting from my pov.
 
> What's an unrelated fixture in this context?

The fixture is after we have completed the subtest. So we get
	SUCCESS; ABORT
when we never intended to use the display at all.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t] lib: Handle chamelium failures gracefully

[Petri Latvala]

On Wed, Nov 11, 2020 at 09:56:55AM +0000, Chris Wilson wrote:
> Quoting Petri Latvala (2020-11-11 07:52:10)
> > On Mon, Nov 09, 2020 at 03:16:58PM +0000, Chris Wilson wrote:
> > > Don't cause CI to abort a run if an _unrelated_ fixture checks for a
> > > display and chamelium is dead.
> > 
> > If there's a chamelium configured and it's dead, we do want to abort
> > though.
> 
> If there's no display, we don't run display tests. The GPUs are still
> very interesting from my pov.

For CI results the display tests becoming skips is awfully painful. If
the chamelium is dead, we (CI) want to be notified and people tend to
spot new aborts easier than a "we have no display" skip that might
even be filtered by cibuglog.

It's a tradeoff and there is no solution that satisfies everyone, we
went with the one that leads us fixing the hw earlier.

>  
> > What's an unrelated fixture in this context?
> 
> The fixture is after we have completed the subtest. So we get
> 	SUCCESS; ABORT
> when we never intended to use the display at all.

Ah, the cleanup fixtures, I see.


-- 
Petri Latvala
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t] lib: Handle chamelium failures gracefully

[Chris Wilson]

Quoting Petri Latvala (2020-11-11 10:29:37)
> On Wed, Nov 11, 2020 at 09:56:55AM +0000, Chris Wilson wrote:
> > Quoting Petri Latvala (2020-11-11 07:52:10)
> > > What's an unrelated fixture in this context?
> > 
> > The fixture is after we have completed the subtest. So we get
> >       SUCCESS; ABORT
> > when we never intended to use the display at all.
> 
> Ah, the cleanup fixtures, I see.

In this case, not a cleanup per se, but an opening fixture for a new
bunch of tests (sometimes a new subgroup) which will not be executed.

Not that that helps.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev