[igt-dev] [PATCH] tests: Skip tests using LUTs when the driver doesn't support it.

[igt-dev] [PATCH] tests: Skip tests using LUTs when the driver doesn't support it.

[Mark Yacoub]

From: Mark Yacoub <markyacoub@google.com>

[Why]
Some drivers such as msm do not support CTM and their CRTC doesn't have
LUT props. Tests should skip relevant tests when it's not supported.

[How]
Check for the LUT pipe prop before starting relevant subtest.

Tested on Trogdor (msm)

Signed-off-by: Mark Yacoub <markyacoub@chromium.org>
---
 tests/kms_color.c |  8 ++++++++
 tests/kms_plane.c | 52 ++++++++++++++++++++++++++++-------------------
 2 files changed, 39 insertions(+), 21 deletions(-)

diff --git a/tests/kms_color.c b/tests/kms_color.c
index 775f3596..99a59cd8 100644
--- a/tests/kms_color.c
+++ b/tests/kms_color.c
@@ -905,6 +905,14 @@ igt_main
 	}
 
 	for_each_pipe_static(pipe) {
+		igt_fixture {
+			igt_require_f(igt_pipe_has_prop(&data.display, pipe,
+											IGT_CRTC_GAMMA_LUT) &&
+								igt_pipe_has_prop(&data.display, pipe,
+												IGT_CRTC_DEGAMMA_LUT),
+							"Driver doesn't support CTM.\n");
+		}
+
 		igt_subtest_group
 			run_tests_for_pipe(&data, pipe);
 
diff --git a/tests/kms_plane.c b/tests/kms_plane.c
index 405d4c21..d82214ed 100644
--- a/tests/kms_plane.c
+++ b/tests/kms_plane.c
@@ -1090,28 +1090,38 @@ run_tests_for_pipe_plane(data_t *data)
 		igt_require(data->display.pipes[pipe].n_planes > 0);
 	}
 
-	igt_describe("verify the pixel formats for given plane and pipe");
-	igt_subtest_with_dynamic_f("pixel-format") {
-		count = 0;
-		for_each_pipe(&data->display, pipe) {
-			igt_dynamic_f("pipe-%s-planes", kmstest_pipe_name(pipe))
-				test_pixel_formats(data, pipe);
-			if (is_pipe_limit_reached(++count))
-				break;
-		}
-	}
-	igt_describe("verify the pixel formats for given plane and pipe with source clamping");
-	igt_subtest_with_dynamic_f("pixel-format-source-clamping") {
-		count = 0;
-		for_each_pipe(&data->display, pipe) {
-			igt_dynamic_f("pipe-%s-planes", kmstest_pipe_name(pipe)) {
-				data->crop = 4;
-				test_pixel_formats(data, pipe);
+		igt_describe("Verify the pixel formats for given plane and pipe");
+		igt_subtest_group {
+			igt_fixture {
+				igt_require_f(igt_pipe_has_prop(&data->display, pipe,
+												IGT_CRTC_GAMMA_LUT) &&
+								  igt_pipe_has_prop(&data->display, pipe,
+													IGT_CRTC_DEGAMMA_LUT),
+							  "Driver doesn't support CTM.\n");
 			}
-			if (is_pipe_limit_reached(++count))
-				break;
-		}
-	}
+
+			igt_subtest_with_dynamic_f("pixel-format") {
+				count = 0;
+				for_each_pipe(&data->display, pipe) {
+					igt_dynamic_f("pipe-%s-planes", kmstest_pipe_name(pipe))
+						test_pixel_formats(data, pipe);
+					if (is_pipe_limit_reached(++count))
+						break;
+				}
+			}
+		    igt_subtest_with_dynamic_f("pixel-format-source-clamping") {
+				count = 0;
+				for_each_pipe(&data->display, pipe) {
+				    igt_dynamic_f("pipe-%s-planes", kmstest_pipe_name(pipe)) {
+						data->crop = 4;
+						test_pixel_formats(data, pipe);
+				    }
+				    if (is_pipe_limit_reached(++count))
+						break;
+				}
+		    }
+        }
+
 
 	data->crop = 0;
 	igt_describe("verify plane position using two planes to create a fully covered screen");
-- 
2.34.0.rc1.387.gb447b232ab-goog

[igt-dev] ✗ Fi.CI.BAT: failure for tests: Skip tests using LUTs when the driver doesn't support it.

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 5680 bytes --]

== Series Details ==

Series: tests: Skip tests using LUTs when the driver doesn't support it.
URL   : https://patchwork.freedesktop.org/series/96984/
State : failure

== Summary ==

CI Bug Log - changes from IGT_6281 -> IGTPW_6404
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_6404 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_6404, 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_6404/index.html

Participating hosts (37 -> 35)
------------------------------

  Additional (2): fi-bsw-nick fi-tgl-u2 
  Missing    (4): fi-ctg-p8600 fi-ilk-m540 fi-bsw-cyan fi-hsw-4200u 

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live@hangcheck:
    - fi-icl-u2:          [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6281/fi-icl-u2/igt@i915_selftest@live@hangcheck.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6404/fi-icl-u2/igt@i915_selftest@live@hangcheck.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@core_hotunplug@unbind-rebind:
    - fi-tgl-u2:          NOTRUN -> [INCOMPLETE][3] ([i915#4006])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6404/fi-tgl-u2/igt@core_hotunplug@unbind-rebind.html

  * igt@gem_huc_copy@huc-copy:
    - fi-tgl-u2:          NOTRUN -> [SKIP][4] ([i915#2190])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6404/fi-tgl-u2/igt@gem_huc_copy@huc-copy.html

  * igt@kms_chamelium@dp-crc-fast:
    - fi-bsw-nick:        NOTRUN -> [SKIP][5] ([fdo#109271] / [fdo#111827]) +8 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6404/fi-bsw-nick/igt@kms_chamelium@dp-crc-fast.html

  * igt@kms_chamelium@dp-hpd-fast:
    - fi-tgl-u2:          NOTRUN -> [SKIP][6] ([fdo#109284] / [fdo#111827]) +8 similar issues
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6404/fi-tgl-u2/igt@kms_chamelium@dp-hpd-fast.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
    - fi-tgl-u2:          NOTRUN -> [SKIP][7] ([i915#4103]) +1 similar issue
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6404/fi-tgl-u2/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html

  * igt@kms_force_connector_basic@force-load-detect:
    - fi-tgl-u2:          NOTRUN -> [SKIP][8] ([fdo#109285])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6404/fi-tgl-u2/igt@kms_force_connector_basic@force-load-detect.html

  * igt@prime_vgem@basic-fence-flip:
    - fi-bsw-nick:        NOTRUN -> [SKIP][9] ([fdo#109271]) +63 similar issues
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6404/fi-bsw-nick/igt@prime_vgem@basic-fence-flip.html

  * igt@prime_vgem@basic-userptr:
    - fi-tgl-u2:          NOTRUN -> [SKIP][10] ([i915#3301])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6404/fi-tgl-u2/igt@prime_vgem@basic-userptr.html

  * igt@runner@aborted:
    - fi-icl-u2:          NOTRUN -> [FAIL][11] ([i915#3363] / [i915#4006] / [i915#4312])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6404/fi-icl-u2/igt@runner@aborted.html
    - fi-tgl-u2:          NOTRUN -> [FAIL][12] ([i915#1602] / [i915#2722] / [i915#4312])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6404/fi-tgl-u2/igt@runner@aborted.html

  
#### Possible fixes ####

  * igt@gem_exec_suspend@basic-s0:
    - fi-tgl-1115g4:      [FAIL][13] ([i915#1888]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6281/fi-tgl-1115g4/igt@gem_exec_suspend@basic-s0.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6404/fi-tgl-1115g4/igt@gem_exec_suspend@basic-s0.html

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

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1602]: https://gitlab.freedesktop.org/drm/intel/issues/1602
  [i915#1888]: https://gitlab.freedesktop.org/drm/intel/issues/1888
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#2722]: https://gitlab.freedesktop.org/drm/intel/issues/2722
  [i915#3301]: https://gitlab.freedesktop.org/drm/intel/issues/3301
  [i915#3303]: https://gitlab.freedesktop.org/drm/intel/issues/3303
  [i915#3363]: https://gitlab.freedesktop.org/drm/intel/issues/3363
  [i915#4006]: https://gitlab.freedesktop.org/drm/intel/issues/4006
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312


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

  * CI: CI-20190529 -> None
  * IGT: IGT_6281 -> IGTPW_6404

  CI-20190529: 20190529
  CI_DRM_10886: 1497355cdd0a857944ecbe19163503b26d9fe92f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_6404: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6404/index.html
  IGT_6281: 551f33d68fc780e3d264abd69ec59fe1af1bb65b @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git

== Logs ==

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

[-- Attachment #2: Type: text/html, Size: 6776 bytes --]

Re: [igt-dev] [PATCH] tests: Skip tests using LUTs when the driver doesn't support it.

[Jessica Zhang]

Hi Mark,

On 11/16/2021 9:31 AM, Mark Yacoub wrote:
> From: Mark Yacoub <markyacoub@google.com>
>
> [Why]
> Some drivers such as msm do not support CTM and their CRTC doesn't have
> LUT props. Tests should skip relevant tests when it's not supported.
>
> [How]
> Check for the LUT pipe prop before starting relevant subtest.
>
> Tested on Trogdor (msm)
>
> Signed-off-by: Mark Yacoub <markyacoub@chromium.org>
> ---
>   tests/kms_color.c |  8 ++++++++
>   tests/kms_plane.c | 52 ++++++++++++++++++++++++++++-------------------
>   2 files changed, 39 insertions(+), 21 deletions(-)
>
> diff --git a/tests/kms_color.c b/tests/kms_color.c
> index 775f3596..99a59cd8 100644
> --- a/tests/kms_color.c
> +++ b/tests/kms_color.c
> @@ -905,6 +905,14 @@ igt_main
>   	}
>   
>   	for_each_pipe_static(pipe) {
> +		igt_fixture {
> +			igt_require_f(igt_pipe_has_prop(&data.display, pipe,
> +											IGT_CRTC_GAMMA_LUT) &&
> +								igt_pipe_has_prop(&data.display, pipe,
> +												IGT_CRTC_DEGAMMA_LUT),
> +							"Driver doesn't support CTM.\n");
> +		}
> +
Spacing looks a little messy and makes the code hard to read. Please 
reformat to something more legible.
>   		igt_subtest_group
>   			run_tests_for_pipe(&data, pipe);
>   
> diff --git a/tests/kms_plane.c b/tests/kms_plane.c
> index 405d4c21..d82214ed 100644
> --- a/tests/kms_plane.c
> +++ b/tests/kms_plane.c
> @@ -1090,28 +1090,38 @@ run_tests_for_pipe_plane(data_t *data)
>   		igt_require(data->display.pipes[pipe].n_planes > 0);
>   	}
>   
> -	igt_describe("verify the pixel formats for given plane and pipe");
> -	igt_subtest_with_dynamic_f("pixel-format") {
> -		count = 0;
> -		for_each_pipe(&data->display, pipe) {
> -			igt_dynamic_f("pipe-%s-planes", kmstest_pipe_name(pipe))
> -				test_pixel_formats(data, pipe);
> -			if (is_pipe_limit_reached(++count))
> -				break;
> -		}
> -	}
> -	igt_describe("verify the pixel formats for given plane and pipe with source clamping");
> -	igt_subtest_with_dynamic_f("pixel-format-source-clamping") {
> -		count = 0;
> -		for_each_pipe(&data->display, pipe) {
> -			igt_dynamic_f("pipe-%s-planes", kmstest_pipe_name(pipe)) {
> -				data->crop = 4;
> -				test_pixel_formats(data, pipe);
> +		igt_describe("Verify the pixel formats for given plane and pipe");
> +		igt_subtest_group {
> +			igt_fixture {
> +				igt_require_f(igt_pipe_has_prop(&data->display, pipe,
> +												IGT_CRTC_GAMMA_LUT) &&
> +								  igt_pipe_has_prop(&data->display, pipe,
> +													IGT_CRTC_DEGAMMA_LUT),
> +							  "Driver doesn't support CTM.\n");

Same here.

Thanks,

Jessica Zhang

>   			}
> -			if (is_pipe_limit_reached(++count))
> -				break;
> -		}
> -	}
> +
> +			igt_subtest_with_dynamic_f("pixel-format") {
> +				count = 0;
> +				for_each_pipe(&data->display, pipe) {
> +					igt_dynamic_f("pipe-%s-planes", kmstest_pipe_name(pipe))
> +						test_pixel_formats(data, pipe);
> +					if (is_pipe_limit_reached(++count))
> +						break;
> +				}
> +			}
> +		    igt_subtest_with_dynamic_f("pixel-format-source-clamping") {
> +				count = 0;
> +				for_each_pipe(&data->display, pipe) {
> +				    igt_dynamic_f("pipe-%s-planes", kmstest_pipe_name(pipe)) {
> +						data->crop = 4;
> +						test_pixel_formats(data, pipe);
> +				    }
> +				    if (is_pipe_limit_reached(++count))
> +						break;
> +				}
> +		    }
> +        }
> +
>   
>   	data->crop = 0;
>   	igt_describe("verify plane position using two planes to create a fully covered screen");

Re: [igt-dev] [PATCH] tests: Skip tests using LUTs when the driver doesn't support it.

[Petri Latvala]

On Tue, Nov 16, 2021 at 12:31:31PM -0500, Mark Yacoub wrote:
> From: Mark Yacoub <markyacoub@google.com>
> 
> [Why]
> Some drivers such as msm do not support CTM and their CRTC doesn't have
> LUT props. Tests should skip relevant tests when it's not supported.
> 
> [How]
> Check for the LUT pipe prop before starting relevant subtest.
> 
> Tested on Trogdor (msm)
> 
> Signed-off-by: Mark Yacoub <markyacoub@chromium.org>
> ---
>  tests/kms_color.c |  8 ++++++++
>  tests/kms_plane.c | 52 ++++++++++++++++++++++++++++-------------------
>  2 files changed, 39 insertions(+), 21 deletions(-)
> 
> diff --git a/tests/kms_color.c b/tests/kms_color.c
> index 775f3596..99a59cd8 100644
> --- a/tests/kms_color.c
> +++ b/tests/kms_color.c
> @@ -905,6 +905,14 @@ igt_main
>  	}
>  
>  	for_each_pipe_static(pipe) {
> +		igt_fixture {
> +			igt_require_f(igt_pipe_has_prop(&data.display, pipe,
> +											IGT_CRTC_GAMMA_LUT) &&
> +								igt_pipe_has_prop(&data.display, pipe,
> +												IGT_CRTC_DEGAMMA_LUT),
> +							"Driver doesn't support CTM.\n");
> +		}

I went through the subtests and they mostly do this already for all
the props they're using. The only exception is *-legacy-gamma, in
test_pipe_legacy_gamma(). (notably, the legacy_gamma_reset() function
does check that the prop exists)



> +
>  		igt_subtest_group
>  			run_tests_for_pipe(&data, pipe);
>  
> diff --git a/tests/kms_plane.c b/tests/kms_plane.c
> index 405d4c21..d82214ed 100644
> --- a/tests/kms_plane.c
> +++ b/tests/kms_plane.c
> @@ -1090,28 +1090,38 @@ run_tests_for_pipe_plane(data_t *data)
>  		igt_require(data->display.pipes[pipe].n_planes > 0);
>  	}
>  
> -	igt_describe("verify the pixel formats for given plane and pipe");
> -	igt_subtest_with_dynamic_f("pixel-format") {
> -		count = 0;
> -		for_each_pipe(&data->display, pipe) {
> -			igt_dynamic_f("pipe-%s-planes", kmstest_pipe_name(pipe))
> -				test_pixel_formats(data, pipe);
> -			if (is_pipe_limit_reached(++count))
> -				break;
> -		}
> -	}
> -	igt_describe("verify the pixel formats for given plane and pipe with source clamping");
> -	igt_subtest_with_dynamic_f("pixel-format-source-clamping") {
> -		count = 0;
> -		for_each_pipe(&data->display, pipe) {
> -			igt_dynamic_f("pipe-%s-planes", kmstest_pipe_name(pipe)) {
> -				data->crop = 4;
> -				test_pixel_formats(data, pipe);
> +		igt_describe("Verify the pixel formats for given plane and pipe");
> +		igt_subtest_group {
> +			igt_fixture {
> +				igt_require_f(igt_pipe_has_prop(&data->display, pipe,
> +												IGT_CRTC_GAMMA_LUT) &&
> +								  igt_pipe_has_prop(&data->display, pipe,
> +													IGT_CRTC_DEGAMMA_LUT),
> +							  "Driver doesn't support CTM.\n");

These tests don't use these props, they use the SETGAMMA ioctl.


-- 
Petri Latvala


>  			}
> -			if (is_pipe_limit_reached(++count))
> -				break;
> -		}
> -	}
> +
> +			igt_subtest_with_dynamic_f("pixel-format") {
> +				count = 0;
> +				for_each_pipe(&data->display, pipe) {
> +					igt_dynamic_f("pipe-%s-planes", kmstest_pipe_name(pipe))
> +						test_pixel_formats(data, pipe);
> +					if (is_pipe_limit_reached(++count))
> +						break;
> +				}
> +			}
> +		    igt_subtest_with_dynamic_f("pixel-format-source-clamping") {
> +				count = 0;
> +				for_each_pipe(&data->display, pipe) {
> +				    igt_dynamic_f("pipe-%s-planes", kmstest_pipe_name(pipe)) {
> +						data->crop = 4;
> +						test_pixel_formats(data, pipe);
> +				    }
> +				    if (is_pipe_limit_reached(++count))
> +						break;
> +				}
> +		    }
> +        }
> +
>  
>  	data->crop = 0;
>  	igt_describe("verify plane position using two planes to create a fully covered screen");
> -- 
> 2.34.0.rc1.387.gb447b232ab-goog
>