Skip to contentSkip to conversation

i915/query: Enable QUERY_TOPOLOGY_INFO for all gens

PATCH In review Stuart Summers 3 participants 7 messages Feb 9, 2019

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 i915/query: Enable QUERY_TOPOLOGY_INFO for all gens

Currently the QUERY_TOPOLOGY_INFO IOCTL is only being tested on haswell, broadwell, skylake, kabylake, and coffeelake. The i915 driver doesn't prevent other platforms from accessing this ioctl, so modify IGT to at least test the interface, even if the platform/gen doesn't contain multiple GTs.

---
 tests/i915/query.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)
tests/i915/query.c +2 -7
index 3e7fd140..e4757661 100644
@@ -428,9 +428,6 @@ test_query_topology_known_pci_ids(int fd, int devid)
 	int n_slices = 0, n_subslices = 0;
 	int s, ss;
 
-	/* The GT size on some Broadwell skus is not defined, skip those. */
-	igt_skip_on(dev_info->gt == 0);
-
 	memset(&item, 0, sizeof(item));
 	item.query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
 	i915_query_items(fd, &item, 1);
@@ -477,7 +474,8 @@ test_query_topology_known_pci_ids(int fd, int devid)
 		igt_assert_eq(n_subslices, 3 * 3);
 		break;
 	default:
-		igt_assert(false);
+		/* nothing to check */
+		break;
 	}
 
 	free(topo_info);
@@ -524,9 +522,6 @@ igt_main
 
 	igt_subtest("query-topology-known-pci-ids") {
 		igt_require(query_topology_supported(fd));
-		igt_require(IS_HASWELL(devid) || IS_BROADWELL(devid) ||
-			    IS_SKYLAKE(devid) || IS_KABYLAKE(devid) ||
-			    IS_COFFEELAKE(devid));
 		test_query_topology_known_pci_ids(fd, devid);
 	}
 
Signature · 6 lines
2.20.1

_______________________________________________
igt-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/igt-dev
#1 lore raw

Quoting Stuart Summers (2019-02-09 00:11:46)

Quoted 2 lines

Currently the QUERY_TOPOLOGY_INFO IOCTL is only being tested on haswell, broadwell, skylake, kabylake, and coffeelake.

No, it's tested everywhere it's supported across the other tests. Only this particular test is skipped if there is no hardcoded truth value as only a small number of known pci-ids are supplied.

Skip is the more accurate result (since the test is not doing anything it sets out to do, and tests should be reasonably succinct), but you could move the skip down into the switch() so that we don't repeat ourselves. -Chris
_______________________________________________
igt-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/igt-dev

#2 lore raw
Quoted 11 lines · Chris Wilson

Quoting Stuart Summers (2019-02-09 00:11:46)

Quoted 3 lines

Currently the QUERY_TOPOLOGY_INFO IOCTL is only being tested on haswell,
broadwell, skylake, kabylake, and coffeelake.

No, it's tested everywhere it's supported across the other tests. Only
this particular test is skipped if there is no hardcoded truth value as
only a small number of known pci-ids are supplied.

Quoted 7 lines

Skip is the more accurate result (since the test is not doing anything it
sets out to do, and tests should be reasonably succinct), but you could
move the skip down into the switch() so that we don't repeat ourselves.
-Chris

My reasoning for removing this was to exercise the asserts we are calling within i915_query_items() for those platforms which do not implement dev_info->gt. So we are checking something here, even if it doesn't specifically analyze the slices/subslices for each platform.

I don't think the way it is currently implemented is wrong, but feel the test could be used to potentially catch a bug if the driver were to miss something in its dev_info structure for some platform, for instance.

If you feel strongly we should keep this the way it is, I can drop this.

Thanks,
Stuart

_______________________________________________
igt-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/igt-dev

#3 lore raw

Quoting Summers, Stuart (2019-02-09 00:32:06)

Quoted 19 lines · Chris Wilson
Quoted 11 lines

Quoting Stuart Summers (2019-02-09 00:11:46)

Quoted 3 lines

Currently the QUERY_TOPOLOGY_INFO IOCTL is only being tested on haswell,
broadwell, skylake, kabylake, and coffeelake.

No, it's tested everywhere it's supported across the other tests. Only
this particular test is skipped if there is no hardcoded truth value as
only a small number of known pci-ids are supplied.

Quoted 6 lines

Skip is the more accurate result (since the test is not doing anything it
sets out to do, and tests should be reasonably succinct), but you could
move the skip down into the switch() so that we don't repeat ourselves.

Quoted 4 lines

My reasoning for removing this was to exercise the asserts we are calling within i915_query_items() for those platforms which do not implement dev_info->gt. So we are checking something here, even if it doesn't specifically analyze the slices/subslices for each platform.

But they are already covered. This test is very specifically about comparing the reported topology for a small set of known topologies and verifying they match.
-Chris
_______________________________________________
igt-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/igt-dev

#4 lore raw
Quoted 28 lines · Chris Wilson

Quoting Summers, Stuart (2019-02-09 00:32:06)

Quoted 20 lines · Chris Wilson
Quoted 12 lines

Quoting Stuart Summers (2019-02-09 00:11:46)

Quoted 3 lines

Currently the QUERY_TOPOLOGY_INFO IOCTL is only being tested on haswell,
broadwell, skylake, kabylake, and coffeelake.

No, it's tested everywhere it's supported across the other tests. Only
this particular test is skipped if there is no hardcoded truth value
as
only a small number of known pci-ids are supplied.

Quoted 6 lines

Skip is the more accurate result (since the test is not doing anything it
sets out to do, and tests should be reasonably succinct), but you could
move the skip down into the switch() so that we don't repeat ourselves.

Quoted 6 lines

My reasoning for removing this was to exercise the asserts we are calling within i915_query_items() for those platforms which do not implement dev_info->gt. So we are checking something here, even if it
doesn't specifically analyze the slices/subslices for each platform.

Quoted 5 lines

But they are already covered. This test is very specifically about comparing the reported topology for a small set of known topologies and
verifying they match.
-Chris

Very true, and in more depth than this. I'll drop this. Thanks for the feedback!

_______________________________________________
igt-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/igt-dev

#5 lore raw

[igt-dev] ✓ Fi.CI.BAT: success for i915/query: Enable QUERY_TOPOLOGY_INFO for all gens

In reply to Stuart Summers
== Series Details ==

== Series Details ==

Series: i915/query: Enable QUERY_TOPOLOGY_INFO for all gens
URL : https://patchwork.freedesktop.org/series/56441/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5571 -> IGTPW_2368
====================================================

Summary
-------

**SUCCESS**

No regressions found.

External URL: https://patchwork.freedesktop.org/api/1.0/series/56441/revisions/1/mbox/

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

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

### IGT changes ###

#### Suppressed ####

The following results come from untrusted machines, tests, or statuses. They do not affect the overall result.

* igt@pm_rpm@module-reload:
- {fi-icl-y}: PASS -> INCOMPLETE

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

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

### IGT changes ###

#### Issues hit ####

* igt@kms_busy@basic-flip-a:
- fi-gdg-551: PASS -> FAIL [fdo#103182] +1

* igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
- fi-apl-guc: PASS -> DMESG-WARN [fdo#108529] / [fdo#108566]

* igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
- fi-byt-clapper: PASS -> FAIL [fdo#103191] / [fdo#107362] +1

#### Possible fixes ####

* igt@kms_chamelium@hdmi-hpd-fast:
- fi-kbl-7500u: FAIL [fdo#109485] -> PASS

* igt@kms_pipe_crc_basic@read-crc-pipe-b:
- fi-byt-clapper: FAIL [fdo#107362] -> PASS +1

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

[fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
[fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
[fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
[fdo#108529]: https://bugs.freedesktop.org/show_bug.cgi?id=108529
[fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
[fdo#109485]: https://bugs.freedesktop.org/show_bug.cgi?id=109485

Participating hosts (50 -> 45)
------------------------------

Missing (5): fi-kbl-soraka fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-bdw-samus

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

* IGT: IGT_4815 -> IGTPW_2368

CI_DRM_5571: dd61c93126a4c9eea6a6b2cc9d731dd77d92e0a2 @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_2368: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2368/
IGT_4815: 947301563259726b65ce47d3a3fe37931ed42efe @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2368/
_______________________________________________
igt-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/igt-dev

#6 lore raw

[igt-dev] ✓ Fi.CI.IGT: success for i915/query: Enable QUERY_TOPOLOGY_INFO for all gens

In reply to Stuart Summers
== Series Details ==

== Series Details ==

Series: i915/query: Enable QUERY_TOPOLOGY_INFO for all gens
URL : https://patchwork.freedesktop.org/series/56441/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5571_full -> IGTPW_2368_full
====================================================

Summary
-------

**SUCCESS**

No regressions found.

External URL: https://patchwork.freedesktop.org/api/1.0/series/56441/revisions/1/mbox/

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

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

### IGT changes ###

#### Issues hit ####

* igt@gem_mmap_gtt@hang:
- shard-kbl: PASS -> INCOMPLETE [fdo#103665]
- shard-snb: PASS -> INCOMPLETE [fdo#105411]

* igt@kms_busy@extended-modeset-hang-newfb-render-b:
- shard-kbl: NOTRUN -> DMESG-WARN [fdo#107956]

* igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-c:
- shard-glk: NOTRUN -> DMESG-WARN [fdo#107956]

* igt@kms_cursor_crc@cursor-256x256-random:
- shard-apl: PASS -> FAIL [fdo#103232] +4

* igt@kms_cursor_crc@cursor-256x256-suspend:
- shard-glk: NOTRUN -> FAIL [fdo#103232]

* igt@kms_cursor_crc@cursor-64x21-random:
- shard-kbl: PASS -> FAIL [fdo#103232] +1

* igt@kms_flip@flip-vs-expired-vblank-interruptible:
- shard-glk: PASS -> FAIL [fdo#102887] / [fdo#105363]

* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render:
- shard-apl: PASS -> FAIL [fdo#103167] +3

* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-onoff:
- shard-kbl: PASS -> FAIL [fdo#103167]

* igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-mmap-wc:
- shard-glk: PASS -> FAIL [fdo#103167] +6

* igt@kms_plane@pixel-format-pipe-c-planes:
- shard-apl: PASS -> FAIL [fdo#103166] +4

* igt@kms_plane_alpha_blend@pipe-b-alpha-opaque-fb:
- shard-apl: PASS -> FAIL [fdo#108145]

* igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
- shard-glk: PASS -> FAIL [fdo#103166] +4

* igt@kms_plane_multiple@atomic-pipe-b-tiling-x:
- shard-kbl: PASS -> FAIL [fdo#103166] +1

* igt@perf_pmu@most-busy-check-all-bcs0:
- shard-snb: PASS -> INCOMPLETE [fdo#105411] / [fdo#107469]

* igt@pm_rpm@system-suspend-execbuf:
- shard-kbl: PASS -> INCOMPLETE [fdo#103665] / [fdo#107807]

#### Possible fixes ####

* igt@i915_query@query-topology-known-pci-ids:
- shard-apl: {SKIP} [fdo#109271] -> PASS
- shard-glk: {SKIP} [fdo#109271] -> PASS

* igt@kms_cursor_crc@cursor-128x128-random:
- shard-apl: FAIL [fdo#103232] -> PASS +8

* igt@kms_cursor_crc@cursor-128x128-suspend:
- shard-apl: FAIL [fdo#103191] / [fdo#103232] -> PASS

* igt@kms_cursor_crc@cursor-64x64-onscreen:
- shard-kbl: FAIL [fdo#103232] -> PASS

* igt@kms_cursor_crc@cursor-alpha-opaque:
- shard-apl: FAIL [fdo#109350] -> PASS

* igt@kms_flip@modeset-vs-vblank-race-interruptible:
- shard-glk: FAIL [fdo#103060] -> PASS

* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-gtt:
- shard-apl: FAIL [fdo#103167] -> PASS +5

* igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-mmap-cpu:
- shard-glk: FAIL [fdo#103167] -> PASS +1

* igt@kms_frontbuffer_tracking@fbc-farfromfence:
- shard-snb: INCOMPLETE [fdo#105411] -> PASS

* igt@kms_plane_multiple@atomic-pipe-b-tiling-none:
- shard-glk: FAIL [fdo#103166] -> PASS +2

* igt@kms_plane_multiple@atomic-pipe-c-tiling-yf:
- shard-apl: FAIL [fdo#103166] -> PASS +3

* igt@kms_setmode@basic:
- shard-apl: FAIL [fdo#99912] -> PASS

* igt@kms_universal_plane@universal-plane-pipe-b-functional:
- shard-kbl: FAIL [fdo#103166] -> PASS +1

* igt@kms_vblank@pipe-c-ts-continuation-suspend:
- shard-hsw: FAIL [fdo#104894] -> PASS

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

[fdo#102887]: https://bugs.freedesktop.org/show_bug.cgi?id=102887
[fdo#103060]: https://bugs.freedesktop.org/show_bug.cgi?id=103060
[fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
[fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
[fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
[fdo#104894]: https://bugs.freedesktop.org/show_bug.cgi?id=104894
[fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
[fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
[fdo#107469]: https://bugs.freedesktop.org/show_bug.cgi?id=107469
[fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
[fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
[fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
[fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
[fdo#109350]: https://bugs.freedesktop.org/show_bug.cgi?id=109350
[fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912

Participating hosts (7 -> 5)
------------------------------

Missing (2): shard-skl shard-iclb

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

* IGT: IGT_4815 -> IGTPW_2368
* Piglit: piglit_4509 -> None

CI_DRM_5571: dd61c93126a4c9eea6a6b2cc9d731dd77d92e0a2 @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_2368: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2368/
IGT_4815: 947301563259726b65ce47d3a3fe37931ed42efe @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2368/
_______________________________________________
igt-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/igt-dev