Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] tests/i915_suspend: Free device list after *-without-i915 subtests
On Mon, Feb 13, 2023 at 10:51:39AM +0100, Zbigniew Kempczyński wrote: > On Fri, Feb 10, 2023 at 10:33:21PM +0100, Janusz Krzysztofik wrote: > > On Thursday, 9 February 2023 20:32:31 CET Janusz Krzysztofik wrote: > > > If any of *-without-i915 subtests fails or skips for any reason, it may > > > leave the i915 module unloaded while keeping our device list populated > > > with initially collected data. In a follow up igt_fixture section we then > > > try to reopen the device. If the test has been executed with a device > > > filter specified, an attempt to open the device finds a matching entry > > > that belongs to the no longer existing device in that initially collected > > > device list, fails to stat() it, concludes that's because of the device > > > having been already open, and returns an error. > > > > > > Fix this potentially confusing test result by freeing the potentially > > > outdated device list before continuing with drm_open_driver(). > > > > Freeing device list occurred not safe if device scan was not performed > > before. > > I can see 3 potential solutions: > > 1) force device rescan instead of free before calling drm_open_driver(), > > 2) teach igt_device_free() to return immediately if the device list has not > >been allocated, > > 3) provide a has_device_list() helper for to be used if not sure before > >calling igt_device_free(). > > > > Any preferences? > > I would enforce rescan. > > BTW I wonder how it can happen if runner is executing each subtest > in new process so you're starting from scratch and rescan should be > executed automatically. > > Is is the case you're running few tests from the console? For the record, igt_runner has --multiple-mode where multiple subtests are executed in the same exec. -- Petri Latvala
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 6/6] lib/igt_device_scan: Improve Intel discrete GPU selection
On Fri, Jan 27, 2023 at 11:53:38AM +, Tvrtko Ursulin wrote:
>
> On 27/01/2023 11:39, Petri Latvala wrote:
> > On Fri, Jan 27, 2023 at 11:12:41AM +, Tvrtko Ursulin wrote:
> > > From: Tvrtko Ursulin
> > >
> > > Now that DRM subsystem can contain PCI cards with the vendor set to Intel
> > > but they are not Intel GPUs, we need a better selection logic than looking
> > > at the vendor. Use the driver name instead.
> > >
> > > Caveat that the driver key was on a blacklist so far, and although I can't
> > > imagine it can be slow to probe, this is something to double check.
> > >
> > > Signed-off-by: Tvrtko Ursulin
> > > Cc: Kamil Konieczny
> > > Cc: Zbigniew Kempczyński
> > > ---
> > > lib/igt_device_scan.c | 7 +--
> > > 1 file changed, 5 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
> > > index ed128d24dd10..8b767eed202d 100644
> > > --- a/lib/igt_device_scan.c
> > > +++ b/lib/igt_device_scan.c
> > > @@ -237,6 +237,7 @@ struct igt_device {
> > > char *vendor;
> > > char *device;
> > > char *pci_slot_name;
> > > + char *driver;
> > > int gpu_index; /* For more than one GPU with same vendor and
> > > device. */
> > > char *codename; /* For grouping by codename */
> > > @@ -440,7 +441,6 @@ static bool is_on_blacklist(const char *what)
> > > "resource3", "resource4",
> > > "resource5",
> > > "resource0_wc", "resource1_wc",
> > > "resource2_wc",
> > > "resource3_wc", "resource4_wc",
> > > "resource5_wc",
> > > - "driver",
> > > "uevent", NULL};
> > > const char *key;
> > > int i = 0;
> > > @@ -662,6 +662,8 @@ static struct igt_device
> > > *igt_device_new_from_udev(struct udev_device *dev)
> > > get_pci_vendor_device(idev, , );
> > > idev->codename = __pci_codename(vendor, device);
> > > idev->dev_type = __pci_devtype(vendor, device,
> > > idev->pci_slot_name);
> > > + idev->driver = strdup_nullsafe(get_attr(idev, "driver"));
> > > + igt_assert(idev->driver);
> > > }
> > > return idev;
> > > @@ -776,7 +778,7 @@ static bool __find_first_i915_card(struct
> > > igt_device_card *card, bool discrete)
> > > igt_list_for_each_entry(dev, _devs.all, link) {
> > > - if (!is_pci_subsystem(dev) || !is_vendor_matched(dev, "intel"))
> > > + if (!is_pci_subsystem(dev) || strcmp(dev->driver, "i915"))
> >
> > Is this the time to prepare for that other driver as well?
>
> Ha, didn't think of that TBH, but AFAICT this patch will work for that case
> too, no?
Ah, now that I read the surrounding code...
Indeed the function is for finding i915 devices in particular, used by
gem_wsim and intel_gpu_top. Having that function find devices driven
by xe might lead to incompatibilities that need to be resolved or at
least compatibility fully understood, which is not the case for either
at this time I assume. In other words, out of scope for this patch.
--
Petri Latvala
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 6/6] lib/igt_device_scan: Improve Intel discrete GPU selection
On Fri, Jan 27, 2023 at 11:12:41AM +, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin
>
> Now that DRM subsystem can contain PCI cards with the vendor set to Intel
> but they are not Intel GPUs, we need a better selection logic than looking
> at the vendor. Use the driver name instead.
>
> Caveat that the driver key was on a blacklist so far, and although I can't
> imagine it can be slow to probe, this is something to double check.
>
> Signed-off-by: Tvrtko Ursulin
> Cc: Kamil Konieczny
> Cc: Zbigniew Kempczyński
> ---
> lib/igt_device_scan.c | 7 +--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
> index ed128d24dd10..8b767eed202d 100644
> --- a/lib/igt_device_scan.c
> +++ b/lib/igt_device_scan.c
> @@ -237,6 +237,7 @@ struct igt_device {
> char *vendor;
> char *device;
> char *pci_slot_name;
> + char *driver;
> int gpu_index; /* For more than one GPU with same vendor and device. */
>
> char *codename; /* For grouping by codename */
> @@ -440,7 +441,6 @@ static bool is_on_blacklist(const char *what)
> "resource3", "resource4", "resource5",
> "resource0_wc", "resource1_wc",
> "resource2_wc",
> "resource3_wc", "resource4_wc",
> "resource5_wc",
> - "driver",
> "uevent", NULL};
> const char *key;
> int i = 0;
> @@ -662,6 +662,8 @@ static struct igt_device *igt_device_new_from_udev(struct
> udev_device *dev)
> get_pci_vendor_device(idev, , );
> idev->codename = __pci_codename(vendor, device);
> idev->dev_type = __pci_devtype(vendor, device,
> idev->pci_slot_name);
> + idev->driver = strdup_nullsafe(get_attr(idev, "driver"));
> + igt_assert(idev->driver);
> }
>
> return idev;
> @@ -776,7 +778,7 @@ static bool __find_first_i915_card(struct igt_device_card
> *card, bool discrete)
>
> igt_list_for_each_entry(dev, _devs.all, link) {
>
> - if (!is_pci_subsystem(dev) || !is_vendor_matched(dev, "intel"))
> + if (!is_pci_subsystem(dev) || strcmp(dev->driver, "i915"))
Is this the time to prepare for that other driver as well?
--
Petri Latvala
> continue;
>
> cmp = strncmp(dev->pci_slot_name, INTEGRATED_I915_GPU_PCI_ID,
> @@ -1023,6 +1025,7 @@ static void igt_device_free(struct igt_device *dev)
> free(dev->drm_render);
> free(dev->vendor);
> free(dev->device);
> + free(dev->driver);
> free(dev->pci_slot_name);
> g_hash_table_destroy(dev->attrs_ht);
> g_hash_table_destroy(dev->props_ht);
> --
> 2.34.1
>
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 1/2] lib/dmabuf_sync_file: move common stuff into lib
On Thu, Dec 01, 2022 at 04:49:43PM +, Matthew Auld wrote:
> So we can use this across different tests.
>
> Signed-off-by: Matthew Auld
> Cc: Kamil Konieczny
> Cc: Andrzej Hajda
> Cc: Nirmoy Das
> ---
> lib/dmabuf_sync_file.c | 138 +++
> lib/dmabuf_sync_file.h | 19 ++
> lib/meson.build | 1 +
> tests/dmabuf_sync_file.c | 135 ++
> 4 files changed, 164 insertions(+), 129 deletions(-)
> create mode 100644 lib/dmabuf_sync_file.c
> create mode 100644 lib/dmabuf_sync_file.h
>
> diff --git a/lib/dmabuf_sync_file.c b/lib/dmabuf_sync_file.c
> new file mode 100644
> index ..24e0f96d
> --- /dev/null
> +++ b/lib/dmabuf_sync_file.c
> @@ -0,0 +1,138 @@
> +// SPDX-License-Identifier: MIT
> +
> +#ifdef __linux__
> +#include
> +#endif
> +#include
> +
> +#include "igt.h"
> +#include "igt_vgem.h"
> +#include "sw_sync.h"
> +
> +#include "dmabuf_sync_file.h"
> +
> +struct igt_dma_buf_sync_file {
> + __u32 flags;
> + __s32 fd;
> +};
> +
> +#define IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE _IOWR(DMA_BUF_BASE, 2, struct
> igt_dma_buf_sync_file)
> +#define IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE _IOW(DMA_BUF_BASE, 3, struct
> igt_dma_buf_sync_file)
> +
> +bool has_dmabuf_export_sync_file(int fd)
> +{
> + struct vgem_bo bo;
> + int dmabuf, ret;
> + struct igt_dma_buf_sync_file arg;
> +
> + bo.width = 1;
> + bo.height = 1;
> + bo.bpp = 32;
> + vgem_create(fd, );
> +
> + dmabuf = prime_handle_to_fd(fd, bo.handle);
> + gem_close(fd, bo.handle);
> +
> + arg.flags = DMA_BUF_SYNC_WRITE;
> + arg.fd = -1;
> +
> + ret = igt_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE, );
> + close(dmabuf);
> + igt_assert(ret == 0 || errno == ENOTTY);
> +
> + return ret == 0;
> +}
> +
> +int dmabuf_export_sync_file(int dmabuf, uint32_t flags)
> +{
> + struct igt_dma_buf_sync_file arg;
> +
> + arg.flags = flags;
> + arg.fd = -1;
> + do_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE, );
> +
> + return arg.fd;
> +}
> +
> +bool has_dmabuf_import_sync_file(int fd)
> +{
> + struct vgem_bo bo;
> + int dmabuf, timeline, fence, ret;
> + struct igt_dma_buf_sync_file arg;
> +
> + bo.width = 1;
> + bo.height = 1;
> + bo.bpp = 32;
> + vgem_create(fd, );
> +
> + dmabuf = prime_handle_to_fd(fd, bo.handle);
> + gem_close(fd, bo.handle);
> +
> + timeline = sw_sync_timeline_create();
> + fence = sw_sync_timeline_create_fence(timeline, 1);
> + sw_sync_timeline_inc(timeline, 1);
> +
> + arg.flags = DMA_BUF_SYNC_RW;
> + arg.fd = fence;
> +
> + ret = igt_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE, );
> + close(dmabuf);
> + close(fence);
> + igt_assert(ret == 0 || errno == ENOTTY);
> +
> + return ret == 0;
> +}
> +
> +void dmabuf_import_sync_file(int dmabuf, uint32_t flags, int sync_fd)
> +{
> + struct igt_dma_buf_sync_file arg;
> +
> + arg.flags = flags;
> + arg.fd = sync_fd;
> + do_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE, );
> +}
> +
> +void
> +dmabuf_import_timeline_fence(int dmabuf, uint32_t flags,
> + int timeline, uint32_t seqno)
> +{
> + int fence;
> +
> + fence = sw_sync_timeline_create_fence(timeline, seqno);
> + dmabuf_import_sync_file(dmabuf, flags, fence);
> + close(fence);
> +}
> +
> +bool dmabuf_busy(int dmabuf, uint32_t flags)
> +{
> + struct pollfd pfd = { .fd = dmabuf };
> +
> + /* If DMA_BUF_SYNC_WRITE is set, we don't want to set POLLIN or
> + * else poll() may return a non-zero value if there are only read
> + * fences because POLLIN is ready even if POLLOUT isn't.
> + */
> + if (flags & DMA_BUF_SYNC_WRITE)
> + pfd.events |= POLLOUT;
> + else if (flags & DMA_BUF_SYNC_READ)
> + pfd.events |= POLLIN;
> +
> + return poll(, 1, 0) == 0;
> +}
> +
> +bool sync_file_busy(int sync_file)
> +{
> + struct pollfd pfd = { .fd = sync_file, .events = POLLIN };
> + return poll(, 1, 0) == 0;
> +}
> +
> +bool dmabuf_sync_file_busy(int dmabuf, uint32_t flags)
> +{
> + int sync_file;
> + bool busy;
> +
> + sync_file = dmabuf_export_sync_file(dmabuf, flags);
> + busy = sync_file_busy(sync_file);
> + close(sync_file);
> +
> + return busy;
> +}
Add documentation to all non-static functions in lib.
--
Petri Latvala
>
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t v2 6/9] tests/i915/query: sanity check the unallocated tracking
info.unallocated_size);
> + } else {
> + igt_assert(info.unallocated_cpu_visible_size ==
> +info.unallocated_size);
> + }
> }
>
> igt_assert(r1.memory_class == I915_MEMORY_CLASS_SYSTEM ||
> @@ -623,6 +708,58 @@ static void test_query_regions_sanity_check(int fd)
> igt_assert(!(r1.memory_class == r2.memory_class &&
>r1.memory_instance == r2.memory_instance));
> }
> +
> + {
> + struct igt_list_head handles;
> + struct object_handle oh = {};
> +
> + IGT_INIT_LIST_HEAD();
> +
> + oh.handle =
> + gem_create_with_cpu_access_in_memory_regions
> + (fd, 4096,
> + INTEL_MEMORY_REGION_ID(r1.memory_class,
> + r1.memory_instance));
> + igt_list_add(, );
> + upload(fd, , 1);
> +
> + /*
> + * System wide metrics should be censored if we
> + * lack the correct permissions.
> + */
> + igt_fork(child, 1) {
> + igt_drop_root();
> +
> + memset(regions, 0, item.length);
> + i915_query_items(fd, , 1);
> + info = regions->regions[i];
> +
> + igt_assert(info.unallocated_cpu_visible_size ==
> +info.probed_cpu_visible_size);
> + igt_assert(info.unallocated_size ==
> +info.probed_size);
> + }
> +
> + igt_waitchildren();
> +
> + memset(regions, 0, item.length);
> + i915_query_items(fd, , 1);
> + info = regions->regions[i];
> +
> + if (r1.memory_class == I915_MEMORY_CLASS_DEVICE) {
> + igt_assert(info.unallocated_cpu_visible_size <
> +info.probed_cpu_visible_size);
> + igt_assert(info.unallocated_size <
> +info.probed_size);
> + } else {
> + igt_assert(info.unallocated_cpu_visible_size ==
> +info.probed_cpu_visible_size);
> + igt_assert(info.unallocated_size ==
> +info.probed_size);
> + }
> +
> + gem_close(fd, oh.handle);
> + }
> }
>
> /* All devices should at least have system memory */
> @@ -631,6 +768,134 @@ static void test_query_regions_sanity_check(int fd)
> free(regions);
> }
>
> +#define rounddown(x, y) (x - (x % y))
> +#define SZ_64K (1ULL << 16)
> +
> +static void fill_unallocated(int fd, struct drm_i915_query_item *item, int
> idx,
> + bool cpu_access)
> +{
> + struct drm_i915_memory_region_info new_info, old_info;
> + struct drm_i915_query_memory_regions *regions;
> + struct drm_i915_gem_memory_class_instance ci;
> + struct object_handle *iter, *tmp;
> + struct igt_list_head handles;
> + uint32_t num_handles;
> + uint64_t rem, total;
> + int id;
> +
> + srand(time(NULL));
> +
> + IGT_INIT_LIST_HEAD();
> +
> + regions = (struct drm_i915_query_memory_regions *)item->data_ptr;
from_user_pointer(item->data_ptr)
--
Petri Latvala
> + memset(regions, 0, item->length);
> + i915_query_items(fd, item, 1);
> + new_info = regions->regions[idx];
> + ci = new_info.region;
> +
> + id = INTEL_MEMORY_REGION_ID(ci.memory_class, ci.memory_instance);
> +
> + if (cpu_access)
> + rem = new_info.unallocated_cpu_visible_size / 4;
> + else
> + rem = new_info.unallocated_size / 4;
> +
> + rem = rounddown(rem, SZ_64K);
> + igt_assert_neq(rem, 0);
> + num_handles = 0;
> + total = 0;
> + do {
> + struct object_handle *oh;
> + uint64_t size;
> +
> + size = rand() % rem;
> + size = rounddown(size, SZ_64K);
> + size = max_t(uint64_t
Re: [Intel-gfx] [PATCH v5 i-g-t 2/3] tests/i915/query: Add descriptions to existing tests
On Fri, Jun 03, 2022 at 09:25:51AM -0700, john.c.harri...@intel.com wrote:
> From: John Harrison
>
> None of the query tests had a description. So make some up.
>
> Signed-off-by: John Harrison
> ---
> tests/i915/i915_query.c | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/tests/i915/i915_query.c b/tests/i915/i915_query.c
> index 246a979af72a..35a91d245ec1 100644
> --- a/tests/i915/i915_query.c
> +++ b/tests/i915/i915_query.c
> @@ -923,34 +923,41 @@ igt_main
> devid = intel_get_drm_devid(fd);
> }
>
> + igt_describe("Test reponse to an invalid query call");
Typo, reponse -> response.
--
Petri Latvala
> igt_subtest("query-garbage")
> test_query_garbage(fd);
>
> + igt_describe("Test response to invalid DRM_I915_QUERY_TOPOLOGY_INFO
> query");
> igt_subtest("query-topology-garbage-items") {
> igt_require(query_topology_supported(fd));
> test_query_topology_garbage_items(fd);
> }
>
> + igt_describe("Guardband test for DRM_I915_QUERY_TOPOLOGY_INFO query");
> igt_subtest("query-topology-kernel-writes") {
> igt_require(query_topology_supported(fd));
> test_query_topology_kernel_writes(fd);
> }
>
> + igt_describe("Verify DRM_I915_QUERY_TOPOLOGY_INFO query fails when it
> is not supported");
> igt_subtest("query-topology-unsupported") {
> igt_require(!query_topology_supported(fd));
> test_query_topology_unsupported(fd);
> }
>
> + igt_describe("Compare new DRM_I915_QUERY_TOPOLOGY_INFO query with
> legacy (sub)slice getparams");
> igt_subtest("query-topology-coherent-slice-mask") {
> igt_require(query_topology_supported(fd));
> test_query_topology_coherent_slice_mask(fd);
> }
>
> + igt_describe("More compare new DRM_I915_QUERY_TOPOLOGY_INFO query with
> legacy (sub)slice getparams");
> igt_subtest("query-topology-matches-eu-total") {
> igt_require(query_topology_supported(fd));
> test_query_topology_matches_eu_total(fd);
> }
>
> + igt_describe("Verify DRM_I915_QUERY_TOPOLOGY_INFO query against
> hardcoded known values for certain platforms");
> igt_subtest("query-topology-known-pci-ids") {
> igt_require(query_topology_supported(fd));
> igt_require(IS_HASWELL(devid) || IS_BROADWELL(devid) ||
> @@ -959,16 +966,19 @@ igt_main
> test_query_topology_known_pci_ids(fd, devid);
> }
>
> + igt_describe("Test DRM_I915_QUERY_GEOMETRY_SUBSLICES query");
> igt_subtest("test-query-geometry-subslices") {
> igt_require(query_geometry_subslices_supported(fd));
> test_query_geometry_subslices(fd);
> }
>
> + igt_describe("Dodgy returned data tests for
> DRM_I915_QUERY_MEMORY_REGIONS");
> igt_subtest("query-regions-garbage-items") {
> igt_require(query_regions_supported(fd));
> test_query_regions_garbage_items(fd);
> }
>
> + igt_describe("Basic tests for DRM_I915_QUERY_MEMORY_REGIONS");
> igt_subtest("query-regions-sanity-check") {
> igt_require(query_regions_supported(fd));
> test_query_regions_sanity_check(fd);
> @@ -979,9 +989,11 @@ igt_main
> igt_require(query_engine_info_supported(fd));
> }
>
> + igt_describe("Negative tests for DRM_I915_QUERY_ENGINE_INFO");
> igt_subtest("engine-info-invalid")
> engines_invalid(fd);
>
> + igt_describe("Positive tests for DRM_I915_QUERY_ENGINE_INFO");
> igt_subtest("engine-info")
> engines(fd);
> }
> --
> 2.36.0
>
Re: [Intel-gfx] [igt-dev] [PATCH v3 i-g-t 1/2] include/drm-uapi: Update to latest i915_drm.h
On Thu, Jun 02, 2022 at 05:54:03PM -0700, john.c.harri...@intel.com wrote:
> From: John Harrison
>
> Update to the latest master version of the DRM UAPI header file.
>
> NB: Had to remove '__user' keywords as they do not appear to be
> supported outside of kernel builds.
>
> Signed-off-by: John Harrison
> ---
> include/drm-uapi/i915_drm.h | 410
> 1 file changed, 318 insertions(+), 92 deletions(-)
>
> diff --git a/include/drm-uapi/i915_drm.h b/include/drm-uapi/i915_drm.h
> index 9c9e1afa61ba..5d4166eb80a3 100644
> --- a/include/drm-uapi/i915_drm.h
> +++ b/include/drm-uapi/i915_drm.h
> @@ -24,8 +24,8 @@
> *
> */
>
> -#ifndef _I915_DRM_H_
> -#define _I915_DRM_H_
> +#ifndef _UAPI_I915_DRM_H_
> +#define _UAPI_I915_DRM_H_
This and the note about having to remove __user tells me you didn't
get this header through `make headers_install`.
Also please include the used kernel sha in the commit message.
I tried to replicate this with the current drm-tip version and there
are some differences. Most probably because of not using
headers_install for this patch. I don't know what branch 'master
version' refers to.
--
Petri Latvala
>
> #include "drm.h"
>
> @@ -75,7 +75,7 @@ extern "C" {
> * redefine the interface more easily than an ever growing struct of
> * increasing complexity, and for large parts of that interface to be
> * entirely optional. The downside is more pointer chasing; chasing across
> - * the boundary with pointers encapsulated inside u64.
> + * the __user boundary with pointers encapsulated inside u64.
> *
> * Example chaining:
> *
> @@ -154,25 +154,77 @@ enum i915_mocs_table_index {
> I915_MOCS_CACHED,
> };
>
> -/*
> +/**
> + * enum drm_i915_gem_engine_class - uapi engine type enumeration
> + *
> * Different engines serve different roles, and there may be more than one
> - * engine serving each role. enum drm_i915_gem_engine_class provides a
> - * classification of the role of the engine, which may be used when
> requesting
> - * operations to be performed on a certain subset of engines, or for
> providing
> - * information about that group.
> + * engine serving each role. This enum provides a classification of the role
> + * of the engine, which may be used when requesting operations to be
> performed
> + * on a certain subset of engines, or for providing information about that
> + * group.
> */
> enum drm_i915_gem_engine_class {
> + /**
> + * @I915_ENGINE_CLASS_RENDER:
> + *
> + * Render engines support instructions used for 3D, Compute (GPGPU),
> + * and programmable media workloads. These instructions fetch data and
> + * dispatch individual work items to threads that operate in parallel.
> + * The threads run small programs (called "kernels" or "shaders") on
> + * the GPU's execution units (EUs).
> + */
> I915_ENGINE_CLASS_RENDER= 0,
> +
> + /**
> + * @I915_ENGINE_CLASS_COPY:
> + *
> + * Copy engines (also referred to as "blitters") support instructions
> + * that move blocks of data from one location in memory to another,
> + * or that fill a specified location of memory with fixed data.
> + * Copy engines can perform pre-defined logical or bitwise operations
> + * on the source, destination, or pattern data.
> + */
> I915_ENGINE_CLASS_COPY = 1,
> +
> + /**
> + * @I915_ENGINE_CLASS_VIDEO:
> + *
> + * Video engines (also referred to as "bit stream decode" (BSD) or
> + * "vdbox") support instructions that perform fixed-function media
> + * decode and encode.
> + */
> I915_ENGINE_CLASS_VIDEO = 2,
> +
> + /**
> + * @I915_ENGINE_CLASS_VIDEO_ENHANCE:
> + *
> + * Video enhancement engines (also referred to as "vebox") support
> + * instructions related to image enhancement.
> + */
> I915_ENGINE_CLASS_VIDEO_ENHANCE = 3,
>
> - /* should be kept compact */
> + /**
> + * @I915_ENGINE_CLASS_COMPUTE:
> + *
> + * Compute engines support a subset of the instructions available
> + * on render engines: compute engines support Compute (GPGPU) and
> + * programmable media workloads, but do not support the 3D pipeline.
> + */
> + I915_ENGINE_CLASS_COMPUTE = 4,
> +
> + /* Values in this enum should be kept compact. */
>
> + /**
> + * @I915_ENGINE_CLASS_INVALID:
> + *
> + * Placeholder value to represent an invalid engine class assignment
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 1/3] lib/igt_device_scan: Free filtered devices in igt_devices_free
On Mon, May 30, 2022 at 04:40:06AM +0200, Zbigniew Kempczyński wrote:
> On Fri, May 27, 2022 at 11:50:40AM +0100, Tvrtko Ursulin wrote:
> > From: Tvrtko Ursulin
> >
> > Fix a possible oversight.
>
> Yes, properly coded in igt_device_scan() only. Thanks for spotting this.
>
> >
> > Signed-off-by: Tvrtko Ursulin
> > ---
> > lib/igt_device_scan.c | 5 +
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
> > index 3c23fe0eb520..a30433ae2cff 100644
> > --- a/lib/igt_device_scan.c
> > +++ b/lib/igt_device_scan.c
> > @@ -814,6 +814,11 @@ void igt_devices_free(void)
> > igt_device_free(dev);
> > free(dev);
> > }
> > +
> > + igt_list_for_each_entry_safe(dev, tmp, _devs.filtered, link) {
> > + igt_list_del(>link);
> > + free(dev);
> > + }
>
> Small nit - I would change the order (filtered list I would remove first).
> igt_device_free() also frees dev->devnode, ... so if we would change the
> code to be more "parallel" it would be better to avoid use-after-free.
>
> With this:
>
> Reviewed-by: Zbigniew Kempczyński
Tvrtko is away this week so I made this change and merged.
--
Petri Latvala
>
> --
> Zbigniew
>
> > }
> >
> > /**
> > --
> > 2.32.0
> >
Re: [Intel-gfx] [PATCH i-g-t 3/3] intel_gpu_top: Free all memory on exit
On Fri, May 27, 2022 at 11:50:42AM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin
>
> Be nice and explicitly free all memory on exit.
>
> Also fix a Valgrind reported unitilised conditional jump.
>
> Signed-off-by: Tvrtko Ursulin
> Cc: Petri Latvala
Reviewed-by: Petri Latvala
> ---
> tools/intel_gpu_top.c | 51 +++
> 1 file changed, 42 insertions(+), 9 deletions(-)
>
> diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
> index 26986a822bb7..997aff582ff7 100644
> --- a/tools/intel_gpu_top.c
> +++ b/tools/intel_gpu_top.c
> @@ -437,6 +437,36 @@ static struct engines *discover_engines(char *device)
> return engines;
> }
>
> +static void free_engines(struct engines *engines)
> +{
> + struct pmu_counter **pmu, *free_list[] = {
> + >r_gpu,
> + >r_pkg,
> + >imc_reads,
> + >imc_writes,
> + NULL
> + };
> + unsigned int i;
> +
> + for (pmu = _list[0]; *pmu; pmu++) {
> + if ((*pmu)->present)
> + free((char *)(*pmu)->units);
> + }
> +
> + for (i = 0; i < engines->num_engines; i++) {
> + struct engine *engine = engine_ptr(engines, i);
> +
> + free((char *)engine->name);
> + free((char *)engine->short_name);
> + free((char *)engine->display_name);
> + }
> +
> + closedir(engines->root);
> +
> + free(engines->class);
> + free(engines);
> +}
> +
> #define _open_pmu(type, cnt, pmu, fd) \
> ({ \
> int fd__; \
> @@ -1073,7 +1103,7 @@ static size_t freadat2buf(char *buf, const size_t sz,
> DIR *at, const char *name)
> return count;
> }
>
> -static struct clients *scan_clients(struct clients *clients)
> +static struct clients *scan_clients(struct clients *clients, bool display)
> {
> struct dirent *proc_dent;
> struct client *c;
> @@ -1181,7 +1211,7 @@ next:
> break;
> }
>
> - return display_clients(clients);
> + return display ? display_clients(clients) : clients;
> }
>
> static const char *bars[] = { " ", "▏", "▎", "▍", "▌", "▋", "▊", "▉", "█" };
> @@ -2391,7 +2421,7 @@ static void process_stdin(unsigned int timeout_us)
>
> static bool has_drm_fdinfo(const struct igt_device_card *card)
> {
> - struct drm_client_fdinfo info;
> + struct drm_client_fdinfo info = { };
> unsigned int cnt;
> int fd;
>
> @@ -2572,7 +2602,7 @@ int main(int argc, char **argv)
> }
>
> pmu_sample(engines);
> - scan_clients(clients);
> + scan_clients(clients, false);
> codename = igt_device_get_pretty_name(, false);
>
> while (!stop_top) {
> @@ -2599,7 +2629,7 @@ int main(int argc, char **argv)
> pmu_sample(engines);
> t = (double)(engines->ts.cur - engines->ts.prev) / 1e9;
>
> - disp_clients = scan_clients(clients);
> + disp_clients = scan_clients(clients, true);
>
> if (stop_top)
> break;
> @@ -2649,21 +2679,24 @@ int main(int argc, char **argv)
> pops->close_struct();
> }
>
> - if (stop_top)
> - break;
> -
> if (disp_clients != clients)
> free_clients(disp_clients);
>
> + if (stop_top)
> + break;
> +
> if (output_mode == INTERACTIVE)
> process_stdin(period_us);
> else
> usleep(period_us);
> }
>
> + if (clients)
> + free_clients(clients);
> +
> free(codename);
> err:
> - free(engines);
> + free_engines(engines);
> free(pmu_device);
> exit:
> igt_devices_free();
> --
> 2.32.0
>
Re: [Intel-gfx] [PATCH i-g-t 2/3] lib/drm_fdinfo: Ensure buffer is null terminated
On Fri, May 27, 2022 at 11:50:41AM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin
>
> Ensure buffer is null terminated at the point where the read ended and not
> at the end of the whole buffer. Otherwise string parsing can stray into
> un-initialised memory.
>
> Signed-off-by: Tvrtko Ursulin
Reviewed-by: Petri Latvala
> ---
> lib/igt_drm_fdinfo.c | 8
> lib/igt_drm_fdinfo.h | 4 ++--
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/lib/igt_drm_fdinfo.c b/lib/igt_drm_fdinfo.c
> index b422f67a4ace..250d9e8917f2 100644
> --- a/lib/igt_drm_fdinfo.c
> +++ b/lib/igt_drm_fdinfo.c
> @@ -44,12 +44,12 @@ static size_t read_fdinfo(char *buf, const size_t sz, int
> at, const char *name)
> if (fd < 0)
> return 0;
>
> - buf[sz - 1] = 0;
> - count = read(fd, buf, sz);
> - buf[sz - 1] = 0;
> + count = read(fd, buf, sz - 1);
> + if (count > 0)
> + buf[count - 1] = 0;
> close(fd);
>
> - return count;
> + return count > 0 ? count : 0;
> }
>
> static int parse_engine(char *line, struct drm_client_fdinfo *info,
> diff --git a/lib/igt_drm_fdinfo.h b/lib/igt_drm_fdinfo.h
> index 5db63e28b07e..8759471615bd 100644
> --- a/lib/igt_drm_fdinfo.h
> +++ b/lib/igt_drm_fdinfo.h
> @@ -46,7 +46,7 @@ struct drm_client_fdinfo {
> * igt_parse_drm_fdinfo: Parses the drm fdinfo file
> *
> * @drm_fd: DRM file descriptor
> - * @info: Structure to populate with read data
> + * @info: Structure to populate with read data. Must be zeroed.
> *
> * Returns the number of valid drm fdinfo keys found or zero if not all
> * mandatory keys were present or no engines found.
> @@ -58,7 +58,7 @@ unsigned int igt_parse_drm_fdinfo(int drm_fd, struct
> drm_client_fdinfo *info);
> *
> * @dir: File descriptor pointing to /proc/pid/fdinfo directory
> * @fd: String representation of the file descriptor number to parse.
> - * @info: Structure to populate with read data
> + * @info: Structure to populate with read data. Must be zeroed.
> *
> * Returns the number of valid drm fdinfo keys found or zero if not all
> * mandatory keys were present or no engines found.
> --
> 2.32.0
>
Re: [Intel-gfx] [PATCH i-g-t] intel_gpu_top: Don't show client header if no kernel support
On Fri, May 27, 2022 at 08:53:04AM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin
>
> On kernels without support for the feature we should skip showing the
> clients header to avoid confusing users.
>
> Simply briefly open a render node to the selected device during init and
> look if the relevant fields are present in the fdinfo data.
>
> Signed-off-by: Tvrtko Ursulin
> Issue: https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/issues/120
> ---
> tools/intel_gpu_top.c | 22 --
> 1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
> index 1984c10dca29..26986a822bb7 100644
> --- a/tools/intel_gpu_top.c
> +++ b/tools/intel_gpu_top.c
> @@ -2389,6 +2389,23 @@ static void process_stdin(unsigned int timeout_us)
> process_normal_stdin();
> }
>
> +static bool has_drm_fdinfo(const struct igt_device_card *card)
> +{
> + struct drm_client_fdinfo info;
> + unsigned int cnt;
> + int fd;
> +
> + fd = open(card->render, O_RDWR);
> + if (fd < 0)
> + return false;
> +
> + cnt = igt_parse_drm_fdinfo(fd, );
> +
> + close(fd);
> +
> + return cnt > 0;
> +}
> +
> static void show_help_screen(void)
> {
> printf(
> @@ -2545,8 +2562,9 @@ int main(int argc, char **argv)
>
> ret = EXIT_SUCCESS;
>
> - clients = init_clients(card.pci_slot_name[0] ?
> -card.pci_slot_name : IGPU_PCI);
> + if (has_drm_fdinfo())
> + clients = init_clients(card.pci_slot_name[0] ?
> +card.pci_slot_name : IGPU_PCI);
Checked all usage of 'clients' below this, and everything handles NULL
properly.
That said, nothing seems to free() it, am I reading that correctly?
Anyway, that can be left for another patch, this change is
Reviewed-by: Petri Latvala
> init_engine_classes(engines);
> if (clients) {
> clients->num_classes = engines->num_classes;
> --
> 2.32.0
>
Re: [Intel-gfx] [PATCH v2] drm/i915: fix i915_gem_object_wait_moving_fence
On Fri, Apr 08, 2022 at 09:42:05AM +0100, Matthew Auld wrote:
> All of CI is just failing with the following, which prevents loading of
> the module:
>
> i915 :03:00.0: [drm] *ERROR* Scratch setup failed
>
> Best guess is that this comes from the pin_map() for the scratch page,
> which does an i915_gem_object_wait_moving_fence() somewhere. It looks
> like this now calls into dma_resv_wait_timeout() which can return the
> remaining timeout, leading to the caller thinking this is an error.
>
> v2(Lucas): handle ret == 0
>
> Fixes: 1d7f5e6c5240 ("drm/i915: drop bo->moving dependency")
> Signed-off-by: Matthew Auld
> Cc: Christian König
> Cc: Lucas De Marchi
> Cc: Daniel Vetter
> Reviewed-by: Christian König #v1
For the record, patchwork is disabled at this time. Trybot is still up
if you want CI to verify this.
--
Petri Latvala
> ---
> drivers/gpu/drm/i915/gem/i915_gem_object.c | 11 +--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c
> b/drivers/gpu/drm/i915/gem/i915_gem_object.c
> index 2998d895a6b3..747ac65e060f 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
> @@ -772,9 +772,16 @@ int i915_gem_object_get_moving_fence(struct
> drm_i915_gem_object *obj,
> int i915_gem_object_wait_moving_fence(struct drm_i915_gem_object *obj,
> bool intr)
> {
> + long ret;
> +
> assert_object_held(obj);
> - return dma_resv_wait_timeout(obj->base. resv, DMA_RESV_USAGE_KERNEL,
> - intr, MAX_SCHEDULE_TIMEOUT);
> +
> + ret = dma_resv_wait_timeout(obj->base. resv, DMA_RESV_USAGE_KERNEL,
> + intr, MAX_SCHEDULE_TIMEOUT);
> + if (!ret)
> + ret = -ETIME;
> +
> + return ret < 0 ? ret : 0;
> }
>
> #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
> --
> 2.34.1
>
Re: [Intel-gfx] [PATCH i-g-t v3] tests/gem_lmem_swapping: limit lmem to 4G
On Mon, Mar 28, 2022 at 11:08:59AM +0100, Matthew Auld wrote:
> From: CQ Tang
>
> On some systems lmem can be as large as 16G, which seems to trigger
> various CI timeouts, and in the best case just takes a long time. For
> the purposes of the test we should be able to limit to 4G, without any
> big loss in coverage.
>
> v2:
> - No need to try again without the modparam; if it's not supported it
>will still load the driver just fine.
> v3(Petri):
> - Add a helpful debug print in case the kernel is missing support for
>the lmem_size modparam.
>
> Signed-off-by: CQ Tang
> Signed-off-by: Matthew Auld
> Cc: Thomas Hellström
> Cc: Nirmoy Das
> Cc: Petri Latvala
> Reviewed-by: Thomas Hellström
> Reviewed-by: Nirmoy Das
> ---
> tests/i915/gem_lmem_swapping.c | 12 +++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/tests/i915/gem_lmem_swapping.c b/tests/i915/gem_lmem_swapping.c
> index 31644bcd..6cf1acec 100644
> --- a/tests/i915/gem_lmem_swapping.c
> +++ b/tests/i915/gem_lmem_swapping.c
> @@ -526,11 +526,20 @@ igt_main_args("", long_options, help_str, opt_handler,
> NULL)
>
> igt_fixture {
> struct intel_execution_engine2 *e;
> + char *tmp;
>
> - i915 = drm_open_driver(DRIVER_INTEL);
> + igt_i915_driver_unload();
> + igt_assert_eq(igt_i915_driver_load("lmem_size=4096"), 0);
> +
> + i915 = __drm_open_driver(DRIVER_INTEL);
> igt_require_gem(i915);
> igt_require(gem_has_lmem(i915));
>
> + tmp = __igt_params_get(i915, "lmem_size");
> + if (!tmp)
> + igt_info("lmem_size modparam not supported on this
> kernel. Continuing with full lmem size. This may result in CI timeouts.");
> + free(tmp);
Newline at the end missing. With that added,
Reviewed-by: Petri Latvala
> +
> regions = gem_get_query_memory_regions(i915);
> igt_require(regions);
>
> @@ -556,6 +565,7 @@ igt_main_args("", long_options, help_str, opt_handler,
> NULL)
> intel_ctx_destroy(i915, ctx);
> free(regions);
> close(i915);
> + igt_i915_driver_unload();
> }
>
> igt_exit();
> --
> 2.34.1
>
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t v2] tests/gem_lmem_swapping: limit lmem to 4G
On Mon, Mar 28, 2022 at 10:29:59AM +0100, Matthew Auld wrote:
> From: CQ Tang
>
> On some systems lmem can be as large as 16G, which seems to trigger
> various CI timeouts, and in the best case just takes a long time. For
> the purposes of the test we should be able to limit to 4G, without any
> big loss in coverage.
>
> v2:
> - No need to try again without the modparam; if it's not supported it
>will still load the driver just fine.
>
> Signed-off-by: CQ Tang
> Signed-off-by: Matthew Auld
> Cc: Thomas Hellström
> Cc: Nirmoy Das
> Reviewed-by: Thomas Hellström
> Reviewed-by: Nirmoy Das
> ---
> tests/i915/gem_lmem_swapping.c | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/tests/i915/gem_lmem_swapping.c b/tests/i915/gem_lmem_swapping.c
> index 31644bcd..69f7bae9 100644
> --- a/tests/i915/gem_lmem_swapping.c
> +++ b/tests/i915/gem_lmem_swapping.c
> @@ -527,7 +527,10 @@ igt_main_args("", long_options, help_str, opt_handler,
> NULL)
> igt_fixture {
> struct intel_execution_engine2 *e;
>
> - i915 = drm_open_driver(DRIVER_INTEL);
> + igt_i915_driver_unload();
> + igt_assert_eq(igt_i915_driver_load("lmem_size=4096"), 0);
> +
> + i915 = __drm_open_driver(DRIVER_INTEL);
A debug print would still be lovely if the param is
missing. __igt_params_get() handily tells you if lmem_size exists.
igt_debug might not be good for that kind of a print, log buffer isn't
dumped on igt_runner timeouts. igt_info maybe, igt_warn might be
overkill.
--
Petri Latvala
> igt_require_gem(i915);
> igt_require(gem_has_lmem(i915));
>
> @@ -556,6 +559,7 @@ igt_main_args("", long_options, help_str, opt_handler,
> NULL)
> intel_ctx_destroy(i915, ctx);
> free(regions);
> close(i915);
> + igt_i915_driver_unload();
> }
>
> igt_exit();
> --
> 2.34.1
>
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 3/4] tests/gem_lmem_swapping: limit lmem to 4G
On Thu, Mar 24, 2022 at 02:26:20PM +, Matthew Auld wrote:
> From: CQ Tang
>
> On some systems lmem can be as large as 16G, which seems to trigger
> various CI timeouts, and in the best case just takes a long time. For
> the purposes of the test we should be able to limit to 4G, without any
> big loss in coverage.
>
> Signed-off-by: CQ Tang
> Signed-off-by: Matthew Auld
> Cc: Thomas Hellström
> Cc: Nirmoy Das
> ---
> tests/i915/gem_lmem_swapping.c | 9 -
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/tests/i915/gem_lmem_swapping.c b/tests/i915/gem_lmem_swapping.c
> index 995a663f..ad1c989c 100644
> --- a/tests/i915/gem_lmem_swapping.c
> +++ b/tests/i915/gem_lmem_swapping.c
> @@ -526,7 +526,13 @@ igt_main_args("", long_options, help_str, opt_handler,
> NULL)
> igt_fixture {
> struct intel_execution_engine2 *e;
>
> - i915 = drm_open_driver(DRIVER_INTEL);
> + igt_i915_driver_unload();
> + if (igt_i915_driver_load("lmem_size=4096")) {
> + igt_debug("i915 missing lmem_size modparam support\n");
> + igt_assert_eq(igt_i915_driver_load(NULL), 0);
> + }
Does the driver load truly fail with an unknown param?
--
Petri Latvala
> +
> + i915 = __drm_open_driver(DRIVER_INTEL);
> igt_require_gem(i915);
> igt_require(gem_has_lmem(i915));
>
> @@ -554,6 +560,7 @@ igt_main_args("", long_options, help_str, opt_handler,
> NULL)
> igt_fixture {
> free(regions);
> close(i915);
> + igt_i915_driver_unload();
> }
>
> igt_exit();
> --
> 2.34.1
>
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 3/4] tests/gem_lmem_swapping: limit lmem to 4G
On Fri, Mar 25, 2022 at 08:40:45AM +, Matthew Auld wrote:
> On 24/03/2022 17:47, Dixit, Ashutosh wrote:
> > On Thu, 24 Mar 2022 07:26:20 -0700, Matthew Auld wrote:
> > >
> > > @@ -554,6 +560,7 @@ igt_main_args("", long_options, help_str,
> > > opt_handler, NULL)
> > > igt_fixture {
> > > free(regions);
> > > close(i915);
> > > + igt_i915_driver_unload();
> >
> > I thought we'd reload the module with default params here but when the next
> > test runs the module gets loaded automatically so maybe this is ok?
>
> Yeah, that at least matches my understanding. Adding Petri in case he has
> some comments here.
Yes, the convention is to either leave the module loaded with
defaults, or leave it unloaded. If the next test happens to be one
that wants to load the module with different params, we save some
time.
If loading the module again doesn't work we should see some fireworks
in CI results elsewhere anyway. Due to module loading problems we used
to limit them to known places (reloading tests, selftests, ...) so we
might need to revisit this topic later. But no need to FUD it at this
time.
--
Petri Latvala
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 01/11] tests/i915/i915_hangman: Add descriptions
On Mon, Dec 13, 2021 at 03:29:04PM -0800, john.c.harri...@intel.com wrote:
> From: John Harrison
>
> Added descriptions of the various sub-tests and the test as a whole.
>
> Signed-off-by: John Harrison
> ---
> tests/i915/i915_hangman.c | 11 +--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/tests/i915/i915_hangman.c b/tests/i915/i915_hangman.c
> index 4c18c22db..025bb8713 100644
> --- a/tests/i915/i915_hangman.c
> +++ b/tests/i915/i915_hangman.c
> @@ -46,6 +46,8 @@
> static int device = -1;
> static int sysfs = -1;
>
> +IGT_TEST_DESCRIPTION("Tests for hang detection and recovery");
> +
> static bool has_error_state(int dir)
> {
> bool result;
> @@ -315,9 +317,9 @@ static void hangcheck_unterminated(void)
>
> gem_execbuf(device, );
> if (gem_wait(device, handle, _ns) != 0) {
> - /* need to manually trigger an hang to clean before failing */
> + /* need to manually trigger a hang to clean before failing */
> igt_force_gpu_reset(device);
> - igt_assert_f(0, "unterminated batch did not trigger an hang!");
> + igt_assert_f(0, "unterminated batch did not trigger a hang!");
Ouch, this is a bug that could use a drive-by fix in this same commit:
Add a newline after that text.
With that,
Reviewed-by: Petri Latvala
> }
> }
>
> @@ -341,9 +343,11 @@ igt_main
> igt_require(has_error_state(sysfs));
> }
>
> + igt_describe("Basic error capture");
> igt_subtest("error-state-basic")
> test_error_state_basic();
>
> + igt_describe("Per engine error capture");
> igt_subtest_with_dynamic("error-state-capture") {
> for_each_ctx_engine(device, ctx, e) {
> igt_dynamic_f("%s", e->name)
> @@ -351,6 +355,7 @@ igt_main
> }
> }
>
> + igt_describe("Per engine hang recovery (spin)");
> igt_subtest_with_dynamic("engine-hang") {
> int has_gpu_reset = 0;
> struct drm_i915_getparam gp = {
> @@ -369,6 +374,7 @@ igt_main
> }
> }
>
> + igt_describe("Per engine hang recovery (invalid CS)");
> igt_subtest_with_dynamic("engine-error") {
> int has_gpu_reset = 0;
> struct drm_i915_getparam gp = {
> @@ -386,6 +392,7 @@ igt_main
> }
> }
>
> + igt_describe("Check that executing unintialised memory causes a hang");
> igt_subtest("hangcheck-unterminated")
> hangcheck_unterminated();
>
> --
> 2.25.1
>
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 5/6] intel_gpu_top: Remove clients support
On Fri, Nov 26, 2021 at 01:07:24PM +, Tvrtko Ursulin wrote:
>
> On 19/11/2021 12:59, Tvrtko Ursulin wrote:
> > From: Tvrtko Ursulin
> >
> > When kernel feature was removed the intel_gpu_top part was forgotten.
> >
> > Signed-off-by: Tvrtko Ursulin
> > Cc: Daniel Vetter
>
> Will someone ack this or we carry this code until it ships, if it hasn't
> already?
Does that question mean client busyness will some day return?
Either way,
Acked-by: Petri Latvala
>
> Regards,
>
> Tvrtko
>
> > ---
> > man/intel_gpu_top.rst | 4 -
> > tools/intel_gpu_top.c | 810 +-
> > 2 files changed, 1 insertion(+), 813 deletions(-)
> >
> > diff --git a/man/intel_gpu_top.rst b/man/intel_gpu_top.rst
> > index f4dbfc5b44d9..b3b765b05feb 100644
> > --- a/man/intel_gpu_top.rst
> > +++ b/man/intel_gpu_top.rst
> > @@ -56,10 +56,6 @@ Supported keys:
> > 'q'Exit from the tool.
> > 'h'Show interactive help.
> > '1'Toggle between aggregated engine class and physical engine
> > mode.
> > -'n'Toggle display of numeric client busyness overlay.
> > -'s'Toggle between sort modes (runtime, total runtime, pid, client
> > id).
> > -'i'Toggle display of clients which used no GPU time.
> > -'H'Toggle between per PID aggregation and individual clients.
> > DEVICE SELECTION
> >
> > diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
> > index 7311038a39f4..41c59a72c09d 100644
> > --- a/tools/intel_gpu_top.c
> > +++ b/tools/intel_gpu_top.c
> > @@ -627,562 +627,6 @@ static void pmu_sample(struct engines *engines)
> > }
> > }
> > -enum client_status {
> > - FREE = 0, /* mbz */
> > - ALIVE,
> > - PROBE
> > -};
> > -
> > -struct clients;
> > -
> > -struct client {
> > - struct clients *clients;
> > -
> > - enum client_status status;
> > - int sysfs_root;
> > - int busy_root;
> > - unsigned int id;
> > - unsigned int pid;
> > - char name[24];
> > - char print_name[24];
> > - unsigned int samples;
> > - unsigned long total_runtime;
> > - unsigned long last_runtime;
> > - struct engines *engines;
> > - unsigned long *val;
> > - uint64_t *last;
> > -};
> > -
> > -struct clients {
> > - unsigned int num_clients;
> > - unsigned int active_clients;
> > -
> > - unsigned int num_classes;
> > - struct engine_class *class;
> > -
> > - char sysfs_root[128];
> > -
> > - struct client *client;
> > -};
> > -
> > -#define for_each_client(clients, c, tmp) \
> > - for ((tmp) = (clients)->num_clients, c = (clients)->client; \
> > -(tmp > 0); (tmp)--, (c)++)
> > -
> > -static struct clients *init_clients(const char *drm_card)
> > -{
> > - struct clients *clients;
> > - const char *slash;
> > - ssize_t ret;
> > - int dir;
> > -
> > - clients = malloc(sizeof(*clients));
> > - if (!clients)
> > - return NULL;
> > -
> > - memset(clients, 0, sizeof(*clients));
> > -
> > - if (drm_card) {
> > - slash = rindex(drm_card, '/');
> > - assert(slash);
> > - } else {
> > - slash = "card0";
> > - }
> > -
> > - ret = snprintf(clients->sysfs_root, sizeof(clients->sysfs_root),
> > - "/sys/class/drm/%s/clients/", slash);
> > - assert(ret > 0 && ret < sizeof(clients->sysfs_root));
> > -
> > - dir = open(clients->sysfs_root, O_DIRECTORY | O_RDONLY);
> > - if (dir < 0) {
> > - free(clients);
> > - clients = NULL;
> > - } else {
> > - close(dir);
> > - }
> > -
> > - return clients;
> > -}
> > -
> > -static int __read_to_buf(int fd, char *buf, unsigned int bufsize)
> > -{
> > - ssize_t ret;
> > - int err;
> > -
> > - ret = read(fd, buf, bufsize - 1);
> > - err = errno;
> > - if (ret < 1) {
> > - errno = ret < 0 ? err : ENOMSG;
> > -
> > - return -1;
> > - }
> > -
> > - if (ret > 1 && buf[ret - 1] == '\n')
> > - buf[ret - 1] = '\0';
> > - else
> > - buf[ret] = '\0';
> > -
> > - return 0;
> > -}
> > -
> >
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 3/6] igt/core: Fix build warning
On Fri, Nov 19, 2021 at 05:41:08PM +0200, Petri Latvala wrote:
> On Fri, Nov 19, 2021 at 03:34:54PM +, Tvrtko Ursulin wrote:
> > On 19/11/2021 13:53, Petri Latvala wrote:
> > > On Fri, Nov 19, 2021 at 12:59:42PM +, Tvrtko Ursulin wrote:
> > Okay I wasn't sufficiently focused while trying to fix this. No idea then
> > apart for playing with pragmas.
How's this:
diff --git a/lib/igt_core.c b/lib/igt_core.c
index ec05535c..6a4d0270 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -2727,6 +2727,8 @@ void igt_log(const char *domain, enum igt_log_level
level, const char *format, .
}
static pthread_key_t __vlog_line_continuation;
+static const bool __dummy_true = true;
+static const bool __dummy_false = false;
igt_constructor {
pthread_key_create(&__vlog_line_continuation, NULL);
@@ -2751,6 +2753,7 @@ void igt_vlog(const char *domain, enum igt_log_level
level, const char *format,
FILE *file;
char *line, *formatted_line;
char *thread_id;
+ void *line_continuation;
const char *program_name;
const char *igt_log_level_str[] = {
"DEBUG",
@@ -2785,7 +2788,8 @@ void igt_vlog(const char *domain, enum igt_log_level
level, const char *format,
if (vasprintf(, format, args) == -1)
return;
- if (pthread_getspecific(__vlog_line_continuation)) {
+ line_continuation = pthread_getspecific(__vlog_line_continuation);
+ if (line_continuation != NULL && *(bool *)line_continuation) {
formatted_line = strdup(line);
if (!formatted_line)
goto out;
@@ -2796,9 +2800,9 @@ void igt_vlog(const char *domain, enum igt_log_level
level, const char *format,
}
if (line[strlen(line) - 1] == '\n')
- pthread_setspecific(__vlog_line_continuation, (void*) false);
+ pthread_setspecific(__vlog_line_continuation, &__dummy_false);
else
- pthread_setspecific(__vlog_line_continuation, (void*) true);
+ pthread_setspecific(__vlog_line_continuation, &__dummy_true);
/* append log buffer */
_igt_log_buffer_append(formatted_line);
diff --git a/lib/igt_thread.c b/lib/igt_thread.c
index 5bdda410..0d7bce80 100644
--- a/lib/igt_thread.c
+++ b/lib/igt_thread.c
@@ -29,6 +29,7 @@
#include "igt_thread.h"
static pthread_key_t __igt_is_main_thread;
+static const bool __dummy_true = true;
static _Atomic(bool) __thread_failed = false;
@@ -65,5 +66,5 @@ bool igt_thread_is_main(void)
igt_constructor {
pthread_key_create(&__igt_is_main_thread, NULL);
- pthread_setspecific(__igt_is_main_thread, (void*) 0x1);
+ pthread_setspecific(__igt_is_main_thread, &__dummy_true);
}
Re: [Intel-gfx] [PATCH i-g-t 3/6] igt/core: Fix build warning
On Fri, Nov 19, 2021 at 03:34:54PM +, Tvrtko Ursulin wrote:
>
> On 19/11/2021 13:53, Petri Latvala wrote:
> > On Fri, Nov 19, 2021 at 12:59:42PM +, Tvrtko Ursulin wrote:
> > > From: Tvrtko Ursulin
> > >
> > > .../lib/igt_thread.c: In function ‘__igt_uniqueigt_constructor_l66’:
> > > .../lib/igt_thread.c:68:9: warning: ‘pthread_setspecific’ expecting 1
> > > byte in a region of size 0 [-Wstringop-overread]
> > > 68 | pthread_setspecific(__igt_is_main_thread, (void*) 0x1);
> > >| ^~~~~~~~~~
> > >
> > > Signed-off-by: Tvrtko Ursulin
> > > Cc: Petri Latvala
> > > ---
> > > lib/igt_core.c | 6 --
> > > lib/igt_thread.c | 4 +++-
> > > 2 files changed, 7 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/lib/igt_core.c b/lib/igt_core.c
> > > index ec05535cd56e..acb9743c4a24 100644
> > > --- a/lib/igt_core.c
> > > +++ b/lib/igt_core.c
> > > @@ -2752,6 +2752,8 @@ void igt_vlog(const char *domain, enum
> > > igt_log_level level, const char *format,
> > > char *line, *formatted_line;
> > > char *thread_id;
> > > const char *program_name;
> > > + const uintptr_t false_key = 0;
> > > + const uintptr_t true_key = 1;
> > > const char *igt_log_level_str[] = {
> > > "DEBUG",
> > > "INFO",
> > > @@ -2796,9 +2798,9 @@ void igt_vlog(const char *domain, enum
> > > igt_log_level level, const char *format,
> > > }
> > > if (line[strlen(line) - 1] == '\n')
> > > - pthread_setspecific(__vlog_line_continuation, (void*) false);
> > > + pthread_setspecific(__vlog_line_continuation, _key);
> > > else
> > > - pthread_setspecific(__vlog_line_continuation, (void*) true);
> > > + pthread_setspecific(__vlog_line_continuation, _key);
> >
> > Quoting the usage of this:
> > if (pthread_getspecific(__vlog_line_continuation)) {
> >
> > That's going to give a non-null pointer in both cases.
>
> Doh..
>
> >
> >
> >
> > > /* append log buffer */
> > > _igt_log_buffer_append(formatted_line);
> > > diff --git a/lib/igt_thread.c b/lib/igt_thread.c
> > > index 5bdda4102def..f5de2d57eaaa 100644
> > > --- a/lib/igt_thread.c
> > > +++ b/lib/igt_thread.c
> > > @@ -64,6 +64,8 @@ bool igt_thread_is_main(void)
> > > }
> > > igt_constructor {
> > > + const uintptr_t key = 1;
> > > +
> > > pthread_key_create(&__igt_is_main_thread, NULL);
> > > - pthread_setspecific(__igt_is_main_thread, (void*) 0x1);
> > > + pthread_setspecific(__igt_is_main_thread, );
> >
> > This is fine, __igt_is_main_thread key will have non-null only on the
> > main thread. But still a bit sus slapping the address of a
> > function-local variable to setspecific, we might just be trading a
> > compiler warning for a new future one.
>
> And this is then the same - why do you say this one is okay? :)
Because setspecific for this key is called from only one thread so it
kinda works.
--
Petri Latvala
>
> Okay I wasn't sufficiently focused while trying to fix this. No idea then
> apart for playing with pragmas.
>
> Regards,
>
> Tvrtko
Re: [Intel-gfx] [PATCH i-g-t 3/6] igt/core: Fix build warning
On Fri, Nov 19, 2021 at 12:59:42PM +, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin
>
> .../lib/igt_thread.c: In function ‘__igt_uniqueigt_constructor_l66’:
> .../lib/igt_thread.c:68:9: warning: ‘pthread_setspecific’ expecting 1 byte in
> a region of size 0 [-Wstringop-overread]
>68 | pthread_setspecific(__igt_is_main_thread, (void*) 0x1);
> | ^~
>
> Signed-off-by: Tvrtko Ursulin
> Cc: Petri Latvala
> ---
> lib/igt_core.c | 6 --
> lib/igt_thread.c | 4 +++-
> 2 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index ec05535cd56e..acb9743c4a24 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -2752,6 +2752,8 @@ void igt_vlog(const char *domain, enum igt_log_level
> level, const char *format,
> char *line, *formatted_line;
> char *thread_id;
> const char *program_name;
> + const uintptr_t false_key = 0;
> + const uintptr_t true_key = 1;
> const char *igt_log_level_str[] = {
> "DEBUG",
> "INFO",
> @@ -2796,9 +2798,9 @@ void igt_vlog(const char *domain, enum igt_log_level
> level, const char *format,
> }
>
> if (line[strlen(line) - 1] == '\n')
> - pthread_setspecific(__vlog_line_continuation, (void*) false);
> + pthread_setspecific(__vlog_line_continuation, _key);
> else
> - pthread_setspecific(__vlog_line_continuation, (void*) true);
> + pthread_setspecific(__vlog_line_continuation, _key);
Quoting the usage of this:
if (pthread_getspecific(__vlog_line_continuation)) {
That's going to give a non-null pointer in both cases.
>
> /* append log buffer */
> _igt_log_buffer_append(formatted_line);
> diff --git a/lib/igt_thread.c b/lib/igt_thread.c
> index 5bdda4102def..f5de2d57eaaa 100644
> --- a/lib/igt_thread.c
> +++ b/lib/igt_thread.c
> @@ -64,6 +64,8 @@ bool igt_thread_is_main(void)
> }
>
> igt_constructor {
> + const uintptr_t key = 1;
> +
> pthread_key_create(&__igt_is_main_thread, NULL);
> - pthread_setspecific(__igt_is_main_thread, (void*) 0x1);
> + pthread_setspecific(__igt_is_main_thread, );
This is fine, __igt_is_main_thread key will have non-null only on the
main thread. But still a bit sus slapping the address of a
function-local variable to setspecific, we might just be trading a
compiler warning for a new future one.
--
Petri Latvala
> }
> --
> 2.32.0
>
Re: [Intel-gfx] [PATCH 2/3] drm/i915/dg2: Add initial gt/ctx/engine workarounds
L_BLOCK_CLKGATE_DIS |
> + EGRESS_BLOCK_CLKGATE_DIS | TAG_BLOCK_CLKGATE_DIS);
> +
> + /* Wa_14010948348:dg2_g10 */
> + wa_write_or(wal, UNSLCGCTL9430, MSQDUNIT_CLKGATE_DIS);
> +
> + /* Wa_14011037102:dg2_g10 */
> + wa_write_or(wal, UNSLCGCTL9444, LTCDD_CLKGATE_DIS);
> +
> + /* Wa_14011371254:dg2_g10 */
> + wa_write_or(wal, SLICE_UNIT_LEVEL_CLKGATE, NODEDSS_CLKGATE_DIS);
> +
> + /* Wa_14011431319:dg2_g10 */
> + wa_write_or(wal, UNSLCGCTL9440, GAMTLBOACS_CLKGATE_DIS |
> + GAMTLBVDBOX7_CLKGATE_DIS |
> + GAMTLBVDBOX6_CLKGATE_DIS |
> + GAMTLBVDBOX5_CLKGATE_DIS |
> + GAMTLBVDBOX4_CLKGATE_DIS |
> + GAMTLBVDBOX3_CLKGATE_DIS |
> + GAMTLBVDBOX2_CLKGATE_DIS |
> + GAMTLBVDBOX1_CLKGATE_DIS |
> + GAMTLBVDBOX0_CLKGATE_DIS |
> + GAMTLBKCR_CLKGATE_DIS |
> + GAMTLBGUC_CLKGATE_DIS |
> + GAMTLBBLT_CLKGATE_DIS);
> + wa_write_or(wal, UNSLCGCTL9444, GAMTLBGFXA0_CLKGATE_DIS |
> + GAMTLBGFXA1_CLKGATE_DIS |
> + GAMTLBCOMPA0_CLKGATE_DIS |
> + GAMTLBCOMPA1_CLKGATE_DIS |
> + GAMTLBCOMPB0_CLKGATE_DIS |
> + GAMTLBCOMPB1_CLKGATE_DIS |
> + GAMTLBCOMPC0_CLKGATE_DIS |
> + GAMTLBCOMPC1_CLKGATE_DIS |
> + GAMTLBCOMPD0_CLKGATE_DIS |
> + GAMTLBCOMPD1_CLKGATE_DIS |
> + GAMTLBMERT_CLKGATE_DIS |
> + GAMTLBVEBOX3_CLKGATE_DIS |
> + GAMTLBVEBOX2_CLKGATE_DIS |
> + GAMTLBVEBOX1_CLKGATE_DIS |
> + GAMTLBVEBOX0_CLKGATE_DIS);
> +
> + /* Wa_14010569222:dg2_g10 */
> + wa_write_or(wal, UNSLICE_UNIT_LEVEL_CLKGATE,
> + GAMEDIA_CLKGATE_DIS);
> +
> + /* Wa_14011028019:dg2_g10 */
> + wa_write_or(wal, SSMCGCTL9530, RTFUNIT_CLKGATE_DIS);
> + }
> +
> + if (IS_DG2_GRAPHICS_STEP(gt->i915, G10, STEP_A0, STEP_B0) ||
> + IS_DG2_GRAPHICS_STEP(gt->i915, G11, STEP_A0, STEP_B0)) {
> + /* Wa_14012362059:dg2 */
> + wa_write_or(wal, GEN12_MERT_MOD_CTRL, FORCE_MISS_FTLB);
> + }
> +
> + /* Wa_1509235366:dg2 */
> + wa_write_or(wal, GEN12_GAMCNTRL_CTRL, INVALIDATION_BROADCAST_MODE_DIS |
> + GLOBAL_INVALIDATION_MODE);
> +
> + /* Wa_14014830051:dg2 */
> + wa_write_clr(wal, SARB_CHICKEN1, COMP_CKN_IN);
> +}
> +
> static void
> gt_init_workarounds(struct intel_gt *gt, struct i915_wa_list *wal)
> {
> struct drm_i915_private *i915 = gt->i915;
>
> - if (IS_XEHPSDV(i915))
> + if (IS_DG2(i915))
> + dg2_gt_workarounds_init(gt, wal);
> + else if (IS_XEHPSDV(i915))
> xehpsdv_gt_workarounds_init(gt, wal);
> else if (IS_DG1(i915))
> dg1_gt_workarounds_init(gt, wal);
> @@ -1739,6 +1882,34 @@ static void xehpsdv_whitelist_build(struct
> intel_engine_cs *engine)
> allow_read_ctx_timestamp(engine);
> }
>
> +static void dg2_whitelist_build(struct intel_engine_cs *engine)
> +{
> + struct i915_wa_list *w = >whitelist;
> +
> + allow_read_ctx_timestamp(engine);
> +
> + switch (engine->class) {
> + case RENDER_CLASS:
> + /*
> + * Wa_1507100340:dg2_g10
> + *
> + * This covers 4 registers which are next to one another :
> + * - PS_INVOCATION_COUNT
> + * - PS_INVOCATION_COUNT_UDW
> + * - PS_DEPTH_COUNT
> + * - PS_DEPTH_COUNT_UDW
> + */
> + if (IS_DG2_GRAPHICS_STEP(engine->i915, G10, STEP_A0, STEP_B0))
> + whitelist_reg_ext(w, PS_INVOCATION_COUNT,
> + RING_FORCE_TO_NONPRIV_ACCESS_RD |
> + RING_FORCE_TO_NONPRIV_RANGE_4);
> +
> + break;
> + default:
> + break;
> + }
> +}
> +
> void intel_engine_init_whitelist(struct intel_engine_cs *engine)
> {
> struct drm_i915_private *i915 = engine->i915;
> @@ -1746,7 +1917,9 @@ void intel_engine_init_whitelist(struct intel_engine_cs
> *engine)
>
> wa_init_start(w, "whitelist&q
Re: [Intel-gfx] ✗ Fi.CI.IGT: failure for i915: Initial workarounds for Xe_HP SDV and DG2
On Fri, Nov 12, 2021 at 12:02:24PM +0200, Petri Latvala wrote: > On Thu, Nov 11, 2021 at 09:57:34PM +0200, Vudum, Lakshminarayana wrote: > > spec@arb_gpu_shader_fp64@execution@built-in-functions@fs-abs-dvec3 test is > > not in CI bug log yet. > > > > So, I can address this failure and re-report the results. FYI @Latvala, > > Petri > > piglit results from postmerge are fed to cibuglog only if there's > failures to keep the cpu usage required by test listing under > control. Because of that, handling premerge failures like this is a > bit awkward. Recommendation for this is to just ignore it, looks like > snb just had a bad day running anything. Having said that, fi-snb-2600 had troubles running anything with this too. Same for a few other platforms. And after merging this, they haven't booted up. An actual regression? fi-ivb-3770 fi-snb-2520m fi-snb-2600 fi-ilk-650 fi-ilk-m540 fi-elk-e7500 fi-bwr-2160 fi-pnv-d510 -- Petri Latvala > > > -- > Petri Latvala > > > > > > Lakshmi. > > -Original Message- > > From: Roper, Matthew D > > Sent: Thursday, November 11, 2021 11:14 AM > > To: intel-gfx@lists.freedesktop.org > > Cc: Vudum, Lakshminarayana > > Subject: Re: ✗ Fi.CI.IGT: failure for i915: Initial workarounds for Xe_HP > > SDV and DG2 > > > > On Wed, Nov 03, 2021 at 02:16:42AM +, Patchwork wrote: > > > == Series Details == > > > > > > Series: i915: Initial workarounds for Xe_HP SDV and DG2 > > > URL : https://patchwork.freedesktop.org/series/96513/ > > > State : failure > > > > > > == Summary == > > > > > > CI Bug Log - changes from CI_DRM_10830_full -> Patchwork_21509_full > > > > > > > > > Summary > > > --- > > > > > > **FAILURE** > > > > > > Serious unknown changes coming with Patchwork_21509_full absolutely > > > need to be > > > verified manually. > > > > > > If you think the reported changes have nothing to do with the changes > > > introduced in Patchwork_21509_full, please notify your bug team to > > > allow them > > > to document this new failure mode, which will reduce false positives in > > > CI. > > > > > > > > > > > > Participating hosts (10 -> 11) > > > -- > > > > > > Additional (1): pig-snb-2600 > > > > > > Possible new issues > > > --- > > > > > > Here are the unknown changes that may have been introduced in > > > Patchwork_21509_full: > > > > > > ### Piglit changes ### > > > > > > Possible regressions > > > > > > * spec@arb_gpu_shader_fp64@execution@built-in-functions@fs-abs-dvec3 > > > (NEW): > > > - pig-snb-2600: NOTRUN -> [FAIL][1] +25298 similar issues > > >[1]: > > > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21509/pig-snb-2600/ > > > spec@arb_gpu_shader_fp64@execution@built-in-functi...@fs-abs-dvec3.htm > > > l > > > > piglit: error: waffle_display_connect failed due to > > WAFFLE_ERROR_UNKNOWN: open drm file for gbm failed > > > > Seems to be a problem with piglit opening the DRM file handle on this new > > machine; the Xe_HP SDV and DG2 patches here wouldn't have affected the > > behavior of SNB. > > > > Series applies to drm-intel-gt-next. Thanks Clint and Anusha for the > > reviews. > > > > > > Matt > > > > > > > > > > > New tests > > > - > > > > > > New tests have been introduced between CI_DRM_10830_full and > > > Patchwork_21509_full: > > > > > > ### New Piglit tests (24855) ### > > > > > > * fast_color_clear@all-colors: > > > - Statuses : 1 fail(s) > > > - Exec time: [0.05] s > > > > > > * fast_color_clear@fast-slow-clear-interaction: > > > - Statuses : 1 fail(s) > > > - Exec time: [0.06] s > > > > > > * fast_color_clear@fcc-blit-between-clears: > > > - Statuses : 1 fail(s) > > > - Exec time: [0.04] s > > > > > > * fast_color_clear@fcc-read-after-clear blit rb: > > > - Statuses : 1 fail(s) > > > - Exec time: [0.04] s > > > > > > * fast_color_clear@fcc-read-after-clear blit tex: > > >
Re: [Intel-gfx] ✗ Fi.CI.IGT: failure for i915: Initial workarounds for Xe_HP SDV and DG2
On Thu, Nov 11, 2021 at 09:57:34PM +0200, Vudum, Lakshminarayana wrote: > spec@arb_gpu_shader_fp64@execution@built-in-functions@fs-abs-dvec3 test is > not in CI bug log yet. > > So, I can address this failure and re-report the results. FYI @Latvala, Petri piglit results from postmerge are fed to cibuglog only if there's failures to keep the cpu usage required by test listing under control. Because of that, handling premerge failures like this is a bit awkward. Recommendation for this is to just ignore it, looks like snb just had a bad day running anything. -- Petri Latvala > > Lakshmi. > -Original Message- > From: Roper, Matthew D > Sent: Thursday, November 11, 2021 11:14 AM > To: intel-gfx@lists.freedesktop.org > Cc: Vudum, Lakshminarayana > Subject: Re: ✗ Fi.CI.IGT: failure for i915: Initial workarounds for Xe_HP SDV > and DG2 > > On Wed, Nov 03, 2021 at 02:16:42AM +, Patchwork wrote: > > == Series Details == > > > > Series: i915: Initial workarounds for Xe_HP SDV and DG2 > > URL : https://patchwork.freedesktop.org/series/96513/ > > State : failure > > > > == Summary == > > > > CI Bug Log - changes from CI_DRM_10830_full -> Patchwork_21509_full > > > > > > Summary > > --- > > > > **FAILURE** > > > > Serious unknown changes coming with Patchwork_21509_full absolutely need > > to be > > verified manually. > > > > If you think the reported changes have nothing to do with the changes > > introduced in Patchwork_21509_full, please notify your bug team to allow > > them > > to document this new failure mode, which will reduce false positives in > > CI. > > > > > > > > Participating hosts (10 -> 11) > > -- > > > > Additional (1): pig-snb-2600 > > > > Possible new issues > > --- > > > > Here are the unknown changes that may have been introduced in > > Patchwork_21509_full: > > > > ### Piglit changes ### > > > > Possible regressions > > > > * spec@arb_gpu_shader_fp64@execution@built-in-functions@fs-abs-dvec3 > > (NEW): > > - pig-snb-2600: NOTRUN -> [FAIL][1] +25298 similar issues > >[1]: > > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21509/pig-snb-2600/ > > spec@arb_gpu_shader_fp64@execution@built-in-functi...@fs-abs-dvec3.htm > > l > > piglit: error: waffle_display_connect failed due to > WAFFLE_ERROR_UNKNOWN: open drm file for gbm failed > > Seems to be a problem with piglit opening the DRM file handle on this new > machine; the Xe_HP SDV and DG2 patches here wouldn't have affected the > behavior of SNB. > > Series applies to drm-intel-gt-next. Thanks Clint and Anusha for the reviews. > > > Matt > > > > > > > New tests > > - > > > > New tests have been introduced between CI_DRM_10830_full and > > Patchwork_21509_full: > > > > ### New Piglit tests (24855) ### > > > > * fast_color_clear@all-colors: > > - Statuses : 1 fail(s) > > - Exec time: [0.05] s > > > > * fast_color_clear@fast-slow-clear-interaction: > > - Statuses : 1 fail(s) > > - Exec time: [0.06] s > > > > * fast_color_clear@fcc-blit-between-clears: > > - Statuses : 1 fail(s) > > - Exec time: [0.04] s > > > > * fast_color_clear@fcc-read-after-clear blit rb: > > - Statuses : 1 fail(s) > > - Exec time: [0.04] s > > > > * fast_color_clear@fcc-read-after-clear blit tex: > > - Statuses : 1 fail(s) > > - Exec time: [0.05] s > > > > * fast_color_clear@fcc-read-after-clear copy rb: > > - Statuses : 1 fail(s) > > - Exec time: [0.05] s > > > > * fast_color_clear@fcc-read-after-clear copy tex: > > - Statuses : 1 fail(s) > > - Exec time: [0.04] s > > > > * fast_color_clear@fcc-read-after-clear read_pixels rb: > > - Statuses : 1 fail(s) > > - Exec time: [0.04] s > > > > * fast_color_clear@fcc-read-after-clear read_pixels tex: > > - Statuses : 1 fail(s) > > - Exec time: [0.07] s > > > > * fast_color_clear@fcc-read-after-clear sample tex: > > - Statuses : 1 fail(s) > > - Exec time: [0.05] s > > > > * fast_color_clear@fcc-read-to-pbo-after-clear: > > - Statuses : 1 fail(s) > > - Exec time: [0.05] s > > > > * fa
Re: [Intel-gfx] [PATCH i-g-t 4/8] tests/i915/gem_exec_capture: Use contexts and engines properly
On Wed, Nov 03, 2021 at 11:49:47AM -0700, John Harrison wrote:
> On 11/3/2021 02:36, Petri Latvala wrote:
> > On Tue, Nov 02, 2021 at 06:45:38PM -0700, John Harrison wrote:
> > > On 11/2/2021 16:34, Matthew Brost wrote:
> > > > On Thu, Oct 21, 2021 at 04:40:40PM -0700, john.c.harri...@intel.com
> > > > wrote:
> > > > > From: John Harrison
> > > > >
> > > > > Some of the capture tests were using explicit contexts, some not. Some
> > > > > were poking the per engine pre-emption timeout, some not. This would
> > > > > lead to sporadic failures due to random timeouts, contexts being
> > > > > banned depending upon how many subtests were run and/or how many
> > > > > engines a given platform has, and other such failures.
> > > > >
> > > > > So, update all tests to be conistent.
> > > > >
> > > > > Signed-off-by: John Harrison
> > > > > ---
> > > > >tests/i915/gem_exec_capture.c | 80
> > > > > +--
> > > > >1 file changed, 58 insertions(+), 22 deletions(-)
> > > > >
> > > > > diff --git a/tests/i915/gem_exec_capture.c
> > > > > b/tests/i915/gem_exec_capture.c
> > > > > index c85c198f7..e373d24ed 100644
> > > > > --- a/tests/i915/gem_exec_capture.c
> > > > > +++ b/tests/i915/gem_exec_capture.c
> > > > > @@ -204,8 +204,19 @@ static int check_error_state(int dir, struct
> > > > > offset *obj_offsets, int obj_count,
> > > > > return blobs;
> > > > >}
> > > > > +static void configure_hangs(int fd, const struct
> > > > > intel_execution_engine2 *e, int ctxt_id)
> > > > > +{
> > > > > + /* Ensure fast hang detection */
> > > > > + gem_engine_property_printf(fd, e->name, "preempt_timeout_ms",
> > > > > "%d", 250);
> > > > > + gem_engine_property_printf(fd, e->name,
> > > > > "heartbeat_interval_ms", "%d", 500);
> > > > #define for 250, 500?
> > > Is there any point? There is no special reason for the values other than
> > > small enough to be fast and long enough to not be too small to be usable.
> > > So
> > > there isn't really any particular name to give them beyond
> > > 'SHORT_PREEMPT_TIMEOUT' or some such. And the whole point of the helper
> > > function is that the values are programmed in one place only and not used
> > > anywhere else. So there is no worry about repetition of magic numbers.
> > In about one year everyone has forgotten this explanation and will
> > wonder if it's related to some in-kernel behaviour or if there's some
> > other reason these values have been chosen.
> >
> > So at least a comment why the values are these, please.
> There is a comment already. Not sure what more can be added that is
> meaningful other than changing it to "Ensure fast hang detection by picking
> some random numbers out of the air that seem to be vaguely plausible".
Fair enough.
--
Petri Latvala
Re: [Intel-gfx] [PATCH i-g-t 4/8] tests/i915/gem_exec_capture: Use contexts and engines properly
On Tue, Nov 02, 2021 at 06:45:38PM -0700, John Harrison wrote:
> On 11/2/2021 16:34, Matthew Brost wrote:
> > On Thu, Oct 21, 2021 at 04:40:40PM -0700, john.c.harri...@intel.com wrote:
> > > From: John Harrison
> > >
> > > Some of the capture tests were using explicit contexts, some not. Some
> > > were poking the per engine pre-emption timeout, some not. This would
> > > lead to sporadic failures due to random timeouts, contexts being
> > > banned depending upon how many subtests were run and/or how many
> > > engines a given platform has, and other such failures.
> > >
> > > So, update all tests to be conistent.
> > >
> > > Signed-off-by: John Harrison
> > > ---
> > > tests/i915/gem_exec_capture.c | 80 +--
> > > 1 file changed, 58 insertions(+), 22 deletions(-)
> > >
> > > diff --git a/tests/i915/gem_exec_capture.c b/tests/i915/gem_exec_capture.c
> > > index c85c198f7..e373d24ed 100644
> > > --- a/tests/i915/gem_exec_capture.c
> > > +++ b/tests/i915/gem_exec_capture.c
> > > @@ -204,8 +204,19 @@ static int check_error_state(int dir, struct offset
> > > *obj_offsets, int obj_count,
> > > return blobs;
> > > }
> > > +static void configure_hangs(int fd, const struct intel_execution_engine2
> > > *e, int ctxt_id)
> > > +{
> > > + /* Ensure fast hang detection */
> > > + gem_engine_property_printf(fd, e->name, "preempt_timeout_ms", "%d",
> > > 250);
> > > + gem_engine_property_printf(fd, e->name, "heartbeat_interval_ms", "%d",
> > > 500);
> > #define for 250, 500?
> Is there any point? There is no special reason for the values other than
> small enough to be fast and long enough to not be too small to be usable. So
> there isn't really any particular name to give them beyond
> 'SHORT_PREEMPT_TIMEOUT' or some such. And the whole point of the helper
> function is that the values are programmed in one place only and not used
> anywhere else. So there is no worry about repetition of magic numbers.
In about one year everyone has forgotten this explanation and will
wonder if it's related to some in-kernel behaviour or if there's some
other reason these values have been chosen.
So at least a comment why the values are these, please.
--
Petri Latvala
>
>
> >
> > > +
> > > + /* Allow engine based resets and disable banning */
> > > + igt_allow_hang(fd, ctxt_id, HANG_ALLOW_CAPTURE);
> > > +}
> > > +
> > > static void __capture1(int fd, int dir, uint64_t ahnd, const
> > > intel_ctx_t *ctx,
> > > -unsigned ring, uint32_t target, uint64_t target_size)
> > > +const struct intel_execution_engine2 *e,
> > > +uint32_t target, uint64_t target_size)
> > > {
> > > const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
> > > struct drm_i915_gem_exec_object2 obj[4];
> > > @@ -219,6 +230,8 @@ static void __capture1(int fd, int dir, uint64_t
> > > ahnd, const intel_ctx_t *ctx,
> > > struct offset offset;
> > > int i;
> > > + configure_hangs(fd, e, ctx->id);
> > > +
> > > memset(obj, 0, sizeof(obj));
> > > obj[SCRATCH].handle = gem_create(fd, 4096);
> > > obj[SCRATCH].flags = EXEC_OBJECT_WRITE;
> > > @@ -297,7 +310,7 @@ static void __capture1(int fd, int dir, uint64_t
> > > ahnd, const intel_ctx_t *ctx,
> > > memset(, 0, sizeof(execbuf));
> > > execbuf.buffers_ptr = (uintptr_t)obj;
> > > execbuf.buffer_count = ARRAY_SIZE(obj);
> > > - execbuf.flags = ring;
> > > + execbuf.flags = e->flags;
> > > if (gen > 3 && gen < 6)
> > > execbuf.flags |= I915_EXEC_SECURE;
> > > execbuf.rsvd1 = ctx->id;
> > > @@ -326,7 +339,8 @@ static void __capture1(int fd, int dir, uint64_t
> > > ahnd, const intel_ctx_t *ctx,
> > > gem_close(fd, obj[SCRATCH].handle);
> > > }
> > > -static void capture(int fd, int dir, const intel_ctx_t *ctx, unsigned
> > > ring)
> > > +static void capture(int fd, int dir, const intel_ctx_t *ctx,
> > > + const struct intel_execution_engine2 *e)
> > > {
> > > uint32_t handle;
> > > uint64_t ahnd;
> > > @@ -335,7 +349,7 @@ static void capture(int
Re: [Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915: Simplify handling of modifiers (rev10)
On Mon, Oct 18, 2021 at 07:10:02PM +0300, Imre Deak wrote: > On Mon, Oct 18, 2021 at 06:42:38PM +0300, Petri Latvala wrote: > > On Mon, Oct 18, 2021 at 03:10:54PM +0300, Imre Deak wrote: > > > Hi Petri, Tomi, > > > > > > could you check the failure below? > > > > > > On Fri, Oct 15, 2021 at 11:19:13AM +, Patchwork wrote: > > > > == Series Details == > > > > > > > > Series: drm/i915: Simplify handling of modifiers (rev10) > > > > URL : https://patchwork.freedesktop.org/series/95579/ > > > > State : failure > > > > > > > > == Summary == > > > > > > > > CI Bug Log - changes from CI_DRM_10741_full -> Patchwork_21343_full > > > > > > > > > > > > Summary > > > > --- > > > > > > > > **FAILURE** > > > > > > > > Serious unknown changes coming with Patchwork_21343_full absolutely > > > > need to be > > > > verified manually. > > > > > > > > If you think the reported changes have nothing to do with the changes > > > > introduced in Patchwork_21343_full, please notify your bug team to > > > > allow them > > > > to document this new failure mode, which will reduce false positives > > > > in CI. > > > > > > > > Possible new issues > > > > --- > > > > > > > > Here are the unknown changes that may have been introduced in > > > > Patchwork_21343_full: > > > > > > > > ### IGT changes ### > > > > > > > > Possible regressions > > > > > > > > * igt@kms_vblank@pipe-a-wait-busy: > > > > - shard-skl: [PASS][1] -> [INCOMPLETE][2] > > > >[1]: > > > > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10741/shard-skl1/igt@kms_vbl...@pipe-a-wait-busy.html > > > >[2]: > > > > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21343/shard-skl1/igt@kms_vbl...@pipe-a-wait-busy.html > > > > > > This is from Patchwork_21343/shard-skl1/19/dmesg.log , where the above > > > test passes and is followed by more passing tests, until > > > kms_busy/extended-modeset-hang-oldfb > > > > > > also passes at least according to igt_runner.log, though I can't see it in > > > dmesg.log. After that: > > > > > > [1091.672412] Overall timeout time exceeded, stopping. > > > > > > Is it just an overall timeout problem, or some test after > > > kms_busy/extended-modeset-hand-oldfb? > > > > The test passed but igt_runner's journal.txt for it was left > > empty. Reason for that is unknown (fs corruption, or something like > > that). Because overall timeout got triggered, igt_results was invoked > > against that shard execution in jenkins master host, overwriting the > > DUT-written one, and because the journal didn't state the subtest > > started, it was parsed as being incomplete. > > Ok. Maybe igt_runner could sync/stat the file if the write happened as > expected? It does. https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/runner/executor.c#L881 ... this is a good time for someone to point out a reason to need fsync() instead of fdatasync(). > I can see the same truncation at least on the following > shard-skl test runs: > > CI_DRM_10743/shard-skl7/16/118/journal.txt > Patchwork_21337/shard-skl9/15/45/journal.txt > Patchwork_21343/shard-skl1/19/72/journal.txt > Patchwork_21302/shard-skl7/23/45/journal.txt > Patchwork_21362/shard-skl8/21/12/journal.txt > Patchwork_21317/shard-skl3/11/0/journal.txt > > I can't see anything obvious in dmesg, so I think the issue is unrelated > to changes in the patchset. Would it make sense to open a ticket for the > above particular file-truncated issue? Yeah the common property here is shard-skl. They are the only platforms in CI that use eMMC as their main disk, that could be a red herring or a strong correlation, unknown as of yet. A bug could be filed (against IGT) for this issue so it's at least written down and not forgotten. A cibuglog filter is hard to create for it so let's leave that out. That means we shouldn't poke Lakshmi with this but just consider this series having a pass from CI, that was the only reported regression. -- Petri Latvala > > > The logs unfortunately were not able to give any indication as to why > > the journal file was left empty. igt_runner syncs it when w
Re: [Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915: Simplify handling of modifiers (rev10)
On Mon, Oct 18, 2021 at 03:10:54PM +0300, Imre Deak wrote:
> Hi Petri, Tomi,
>
> could you check the failure below?
>
> On Fri, Oct 15, 2021 at 11:19:13AM +, Patchwork wrote:
> > == Series Details ==
> >
> > Series: drm/i915: Simplify handling of modifiers (rev10)
> > URL : https://patchwork.freedesktop.org/series/95579/
> > State : failure
> >
> > == Summary ==
> >
> > CI Bug Log - changes from CI_DRM_10741_full -> Patchwork_21343_full
> >
> >
> > Summary
> > ---
> >
> > **FAILURE**
> >
> > Serious unknown changes coming with Patchwork_21343_full absolutely need
> > to be
> > verified manually.
> >
> > If you think the reported changes have nothing to do with the changes
> > introduced in Patchwork_21343_full, please notify your bug team to allow
> > them
> > to document this new failure mode, which will reduce false positives in
> > CI.
> >
> > Possible new issues
> > ---
> >
> > Here are the unknown changes that may have been introduced in
> > Patchwork_21343_full:
> >
> > ### IGT changes ###
> >
> > Possible regressions
> >
> > * igt@kms_vblank@pipe-a-wait-busy:
> > - shard-skl: [PASS][1] -> [INCOMPLETE][2]
> >[1]:
> > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10741/shard-skl1/igt@kms_vbl...@pipe-a-wait-busy.html
> >[2]:
> > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21343/shard-skl1/igt@kms_vbl...@pipe-a-wait-busy.html
>
> This is from Patchwork_21343/shard-skl1/19/dmesg.log , where the above
> test passes and is followed by more passing tests, until
> kms_busy/extended-modeset-hang-oldfb
>
> also passes at least according to igt_runner.log, though I can't see it in
> dmesg.log. After that:
>
> [1091.672412] Overall timeout time exceeded, stopping.
>
> Is it just an overall timeout problem, or some test after
> kms_busy/extended-modeset-hand-oldfb?
The test passed but igt_runner's journal.txt for it was left
empty. Reason for that is unknown (fs corruption, or something like
that). Because overall timeout got triggered, igt_results was invoked
against that shard execution in jenkins master host, overwriting the
DUT-written one, and because the journal didn't state the subtest
started, it was parsed as being incomplete.
The logs unfortunately were not able to give any indication as to why
the journal file was left empty. igt_runner syncs it when writing to
it, and the test execution continued normally after that particular
test.
--
Petri Latvala
>
> > Suppressed
> >
> > The following results come from untrusted machines, tests, or statuses.
> > They do not affect the overall result.
> >
> > * {igt@kms_bw@linear-tiling-3-displays-3840x2160p}:
> > - shard-snb: NOTRUN -> [FAIL][3]
> >[3]:
> > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21343/shard-snb6/igt@kms...@linear-tiling-3-displays-3840x2160p.html
> >
> > * {igt@kms_bw@linear-tiling-4-displays-1920x1080p}:
> > - shard-apl: NOTRUN -> [DMESG-FAIL][4]
> >[4]:
> > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21343/shard-apl3/igt@kms...@linear-tiling-4-displays-1920x1080p.html
> >
> >
> > Known issues
> >
> >
> > Here are the changes found in Patchwork_21343_full that come from known
> > issues:
> >
> > ### IGT changes ###
> >
> > Issues hit
> >
> > * igt@feature_discovery@chamelium:
> > - shard-tglb: NOTRUN -> [SKIP][5] ([fdo#111827])
> >[5]:
> > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21343/shard-tglb3/igt@feature_discov...@chamelium.html
> >
> > * igt@gem_ctx_persistence@hostile:
> > - shard-tglb: [PASS][6] -> [FAIL][7] ([i915#2410])
> >[6]:
> > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10741/shard-tglb2/igt@gem_ctx_persiste...@hostile.html
> >[7]:
> > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21343/shard-tglb5/igt@gem_ctx_persiste...@hostile.html
> >
> > * igt@gem_ctx_persistence@legacy-engines-mixed:
> > - shard-snb: NOTRUN -> [SKIP][8] ([fdo#109271] / [i915#1099])
> > +3 similar issues
> >[8]:
> > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21343/shard-snb6/igt@gem_ctx_persiste...@legacy-engines-mixed.html
> >
> > * igt@gem_exec_fair@basic-none-rrul@rcs0:
> > - shard-kbl
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 1/1] tests/i915/query: Query, parse and validate the hwconfig table
On Thu, Sep 16, 2021 at 10:27:41AM -0700, John Harrison wrote:
> On 9/16/2021 01:59, Petri Latvala wrote:
> > On Wed, Sep 15, 2021 at 02:55:58PM -0700, john.c.harri...@intel.com wrote:
> > > From: Rodrigo Vivi
> > >
> > > Newer platforms have an embedded table giving details about that
> > > platform's hardware configuration. This table can be retrieved from
> > > the KMD via the existing query API. So add a test for it as both an
> > > example of how to fetch the table and to validate the contents as much
> > > as is possible.
> > >
> > > Signed-off-by: Rodrigo Vivi
> > > Signed-off-by: John Harrison
> > > Cc: Slawomir Milczarek
> > > Reviewed-by: Matthew Brost
> > > ---
> > > include/drm-uapi/i915_drm.h | 1 +
> > > lib/intel_hwconfig_types.h | 106 +++
> > > tests/i915/i915_query.c | 168
> > > 3 files changed, 275 insertions(+)
> > > create mode 100644 lib/intel_hwconfig_types.h
> > >
> > > diff --git a/include/drm-uapi/i915_drm.h b/include/drm-uapi/i915_drm.h
> > > index b9632bb2c..ae0c8dfad 100644
> > > --- a/include/drm-uapi/i915_drm.h
> > > +++ b/include/drm-uapi/i915_drm.h
> > > @@ -2451,6 +2451,7 @@ struct drm_i915_query_item {
> > > #define DRM_I915_QUERY_ENGINE_INFO 2
> > > #define DRM_I915_QUERY_PERF_CONFIG 3
> > > #define DRM_I915_QUERY_MEMORY_REGIONS 4
> > > +#define DRM_I915_QUERY_HWCONFIG_TABLE 5
> > > /* Must be kept compact -- no holes and well documented */
> > Please update i915_drm.h with a copy from the kernel and state in the
> > commit message which kernel commit sha it's from. If this change is
> > not in the kernel yet, add this token to lib/i915/i915_drm_local.h
> > instead.
> >
> >
> Neither side is merged yet. My understanding is that all sides need to be
> posted in parallel for CI to work. Once green and reviewed, the kernel side
> gets merged first. Then the IGT/UMD patches get updated with the official
> kernel headers, reposted and then merged.
That lockstep dancing removal is the point of i915_drm_local.h. IGT
side can even be merged that way before kernel side is ready, then
updated with the real thing later at whatever cadence.
--
Petri Latvala
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 1/1] tests/i915/query: Query, parse and validate the hwconfig table
On Wed, Sep 15, 2021 at 02:55:58PM -0700, john.c.harri...@intel.com wrote:
> From: Rodrigo Vivi
>
> Newer platforms have an embedded table giving details about that
> platform's hardware configuration. This table can be retrieved from
> the KMD via the existing query API. So add a test for it as both an
> example of how to fetch the table and to validate the contents as much
> as is possible.
>
> Signed-off-by: Rodrigo Vivi
> Signed-off-by: John Harrison
> Cc: Slawomir Milczarek
> Reviewed-by: Matthew Brost
> ---
> include/drm-uapi/i915_drm.h | 1 +
> lib/intel_hwconfig_types.h | 106 +++
> tests/i915/i915_query.c | 168
> 3 files changed, 275 insertions(+)
> create mode 100644 lib/intel_hwconfig_types.h
>
> diff --git a/include/drm-uapi/i915_drm.h b/include/drm-uapi/i915_drm.h
> index b9632bb2c..ae0c8dfad 100644
> --- a/include/drm-uapi/i915_drm.h
> +++ b/include/drm-uapi/i915_drm.h
> @@ -2451,6 +2451,7 @@ struct drm_i915_query_item {
> #define DRM_I915_QUERY_ENGINE_INFO 2
> #define DRM_I915_QUERY_PERF_CONFIG 3
> #define DRM_I915_QUERY_MEMORY_REGIONS 4
> +#define DRM_I915_QUERY_HWCONFIG_TABLE 5
> /* Must be kept compact -- no holes and well documented */
Please update i915_drm.h with a copy from the kernel and state in the
commit message which kernel commit sha it's from. If this change is
not in the kernel yet, add this token to lib/i915/i915_drm_local.h
instead.
--
Petri Latvala
>
> /**
> diff --git a/lib/intel_hwconfig_types.h b/lib/intel_hwconfig_types.h
> new file mode 100644
> index 0..c9961e6bd
> --- /dev/null
> +++ b/lib/intel_hwconfig_types.h
> @@ -0,0 +1,106 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2021 Intel Corporation
> + */
> +
> +#ifndef _INTEL_HWCONFIG_TYPES_H_
> +#define _INTEL_HWCONFIG_TYPES_H_
> +
> +#include "intel_chipset.h"
> +
> +/**
> + * enum intel_hwconfig - Global definition of hwconfig table attributes
> + *
> + * Intel devices provide a KLV (Key/Length/Value) table containing
> + * the static hardware configuration for that platform.
> + * This enum defines the current attribute keys for this KLV.
> + */
> +enum intel_hwconfig {
> + INTEL_HWCONFIG_MAX_SLICES_SUPPORTED = 1,
> + INTEL_HWCONFIG_MAX_DUAL_SUBSLICES_SUPPORTED,/* 2 */
> + INTEL_HWCONFIG_MAX_NUM_EU_PER_DSS, /* 3 */
> + INTEL_HWCONFIG_NUM_PIXEL_PIPES, /* 4 */
> + INTEL_HWCONFIG_DEPRECATED_MAX_NUM_GEOMETRY_PIPES, /* 5 */
> + INTEL_HWCONFIG_DEPRECATED_L3_CACHE_SIZE_IN_KB, /* 6 */
> + INTEL_HWCONFIG_DEPRECATED_L3_BANK_COUNT,/* 7 */
> + INTEL_HWCONFIG_L3_CACHE_WAYS_SIZE_IN_BYTES, /* 8 */
> + INTEL_HWCONFIG_L3_CACHE_WAYS_PER_SECTOR,/* 9 */
> + INTEL_HWCONFIG_MAX_MEMORY_CHANNELS, /* 10 */
> + INTEL_HWCONFIG_MEMORY_TYPE, /* 11 */
> + INTEL_HWCONFIG_CACHE_TYPES, /* 12 */
> + INTEL_HWCONFIG_LOCAL_MEMORY_PAGE_SIZES_SUPPORTED, /* 13 */
> + INTEL_HWCONFIG_DEPRECATED_SLM_SIZE_IN_KB, /* 14 */
> + INTEL_HWCONFIG_NUM_THREADS_PER_EU, /* 15 */
> + INTEL_HWCONFIG_TOTAL_VS_THREADS,/* 16 */
> + INTEL_HWCONFIG_TOTAL_GS_THREADS,/* 17 */
> + INTEL_HWCONFIG_TOTAL_HS_THREADS,/* 18 */
> + INTEL_HWCONFIG_TOTAL_DS_THREADS,/* 19 */
> + INTEL_HWCONFIG_TOTAL_VS_THREADS_POCS, /* 20 */
> + INTEL_HWCONFIG_TOTAL_PS_THREADS,/* 21 */
> + INTEL_HWCONFIG_DEPRECATED_MAX_FILL_RATE,/* 22 */
> + INTEL_HWCONFIG_MAX_RCS, /* 23 */
> + INTEL_HWCONFIG_MAX_CCS, /* 24 */
> + INTEL_HWCONFIG_MAX_VCS, /* 25 */
> + INTEL_HWCONFIG_MAX_VECS,/* 26 */
> + INTEL_HWCONFIG_MAX_COPY_CS, /* 27 */
> + INTEL_HWCONFIG_DEPRECATED_URB_SIZE_IN_KB, /* 28 */
> + INTEL_HWCONFIG_MIN_VS_URB_ENTRIES, /* 29 */
> + INTEL_HWCONFIG_MAX_VS_URB_ENTRIES, /* 30 */
> + INTEL_HWCONFIG_MIN_PCS_URB_ENTRIES, /* 31 */
> + INTEL_HWCONFIG_MAX_PCS_URB_ENTRIES, /* 32 */
> + INTEL_HWCONFIG_MIN_HS_URB_ENTRIES, /* 33 */
> + INTEL_HWCONFIG_MAX_HS_URB_ENTRIES, /* 34 */
> +
Re: [Intel-gfx] ✗ Fi.CI.IGT: failure for use DYNAMIC_DEBUG to implement DRM.debug (rev2)
On Mon, Sep 06, 2021 at 11:04:13AM +0100, Tvrtko Ursulin wrote: > > On 03/09/2021 14:01, Petri Latvala wrote: > > On Fri, Sep 03, 2021 at 12:29:51PM +0100, Tvrtko Ursulin wrote: > > > > > > On 03/09/2021 01:31, jim.cro...@gmail.com wrote: > > > > > > > > > > > > On Tue, Aug 31, 2021 at 5:38 PM Patchwork > > > > > > > <mailto:patchw...@emeril.freedesktop.org>> wrote: > > > > > > > > __ > > > > *Patch Details* > > > > *Series:* use DYNAMIC_DEBUG to implement DRM.debug (rev2) > > > > *URL:* https://patchwork.freedesktop.org/series/93914/ > > > > <https://patchwork.freedesktop.org/series/93914/> > > > > *State:* failure > > > > *Details:* > > > > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20931/index.html > > > > > > > > <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20931/index.html> > > > > > > > > > > > >CI Bug Log - changes from CI_DRM_10541_full -> > > > > Patchwork_20931_full > > > > > > > > > > > > Summary > > > > > > > > *FAILURE* > > > > > > > > Serious unknown changes coming with Patchwork_20931_full absolutely > > > > need to be > > > > verified manually. > > > > > > > > If you think the reported changes have nothing to do with the > > > > changes > > > > introduced in Patchwork_20931_full, please notify your bug team to > > > > allow them > > > > to document this new failure mode, which will reduce false > > > > positives > > > > in CI. > > > > > > > > > > > > hi Team ! > > > > > > > > I think I need a bit of orientation. > > > > > > > > > > > > Possible new issues > > > > > > > > Here are the unknown changes that may have been introduced in > > > > Patchwork_20931_full: > > > > > > > > > > > >IGT changes > > > > > > > > > > > > Possible regressions > > > > > > > >* igt@gem_exec_schedule@u-submit-golden-slice@vcs0: > > > >o shard-skl: NOTRUN -> INCOMPLETE > > > > > > > > <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20931/shard-skl10/igt@gem_exec_schedule@u-submit-golden-sl...@vcs0.html> > > > > > > > > > > > > Warnings > > > > > > > >* igt@i915_pm_dc@dc9-dpms: > > > >o shard-skl: SKIP > > > > > > > > <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10541/shard-skl6/igt@i915_pm...@dc9-dpms.html> > > > > ([fdo#109271]) -> FAIL > > > > > > > > <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20931/shard-skl8/igt@i915_pm...@dc9-dpms.html> > > > > > > > > > > > > > > > > Im assuming the FAIL is the sticking point, > > > > > > Both INCOMPLETE and FAIL will cause a failure to be declared, but in this > > > case it looks to me this series is not at fault. > > > > > > 1) > > > > > > The gem_exec_schedule failure looks like a test runner timeout issue (and > > > apparently test does not handle well running the the fence timeout > > > enabled). > > > > > > @Petri - does the below look like IGT runner running our of time budget > > > for the test run? Would it log > > > > > > [1051.943629] [114/138] ( 11s left) gem_exec_schedule > > > (u-submit-golden-slice) > > > Starting subtest: u-submit-golden-slice > > > Starting dynamic subtest: rcs0 > > > Dynamic subtest rcs0: SUCCESS (80.175s) > > > Starting dynamic subtest: bcs0 > > > Dynamic subtest bcs0: SUCCESS (80.195s) > > > Starting dynamic subtest: vcs0 > > > Dynamic subtest vcs0: SUCCESS (80.243s) > > > Starting dynamic subtest: vecs0 > > > > > > Interesting part is that according to dmesg it never got to the vecs0 > > > dynamic subtest - any idea what happened there? > > > > Yep, we ran out of time. We still had 11 seconds left to execute &
Re: [Intel-gfx] ✗ Fi.CI.IGT: failure for use DYNAMIC_DEBUG to implement DRM.debug (rev2)
On Fri, Sep 03, 2021 at 12:29:51PM +0100, Tvrtko Ursulin wrote: > > On 03/09/2021 01:31, jim.cro...@gmail.com wrote: > > > > > > On Tue, Aug 31, 2021 at 5:38 PM Patchwork > > > <mailto:patchw...@emeril.freedesktop.org>> wrote: > > > > __ > > *Patch Details* > > *Series:* use DYNAMIC_DEBUG to implement DRM.debug (rev2) > > *URL:* https://patchwork.freedesktop.org/series/93914/ > > <https://patchwork.freedesktop.org/series/93914/> > > *State:*failure > > *Details:* > > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20931/index.html > > <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20931/index.html> > > > > > > CI Bug Log - changes from CI_DRM_10541_full -> Patchwork_20931_full > > > > > > Summary > > > > *FAILURE* > > > > Serious unknown changes coming with Patchwork_20931_full absolutely > > need to be > > verified manually. > > > > If you think the reported changes have nothing to do with the changes > > introduced in Patchwork_20931_full, please notify your bug team to > > allow them > > to document this new failure mode, which will reduce false positives > > in CI. > > > > > > hi Team ! > > > > I think I need a bit of orientation. > > > > > > Possible new issues > > > > Here are the unknown changes that may have been introduced in > > Patchwork_20931_full: > > > > > > IGT changes > > > > > > Possible regressions > > > > * igt@gem_exec_schedule@u-submit-golden-slice@vcs0: > > o shard-skl: NOTRUN -> INCOMPLETE > > > > <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20931/shard-skl10/igt@gem_exec_schedule@u-submit-golden-sl...@vcs0.html> > > > > > > Warnings > > > > * igt@i915_pm_dc@dc9-dpms: > > o shard-skl: SKIP > > > > <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10541/shard-skl6/igt@i915_pm...@dc9-dpms.html> > > ([fdo#109271]) -> FAIL > > > > <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20931/shard-skl8/igt@i915_pm...@dc9-dpms.html> > > > > > > > > Im assuming the FAIL is the sticking point, > > Both INCOMPLETE and FAIL will cause a failure to be declared, but in this > case it looks to me this series is not at fault. > > 1) > > The gem_exec_schedule failure looks like a test runner timeout issue (and > apparently test does not handle well running the the fence timeout enabled). > > @Petri - does the below look like IGT runner running our of time budget for > the test run? Would it log > > [1051.943629] [114/138] ( 11s left) gem_exec_schedule (u-submit-golden-slice) > Starting subtest: u-submit-golden-slice > Starting dynamic subtest: rcs0 > Dynamic subtest rcs0: SUCCESS (80.175s) > Starting dynamic subtest: bcs0 > Dynamic subtest bcs0: SUCCESS (80.195s) > Starting dynamic subtest: vcs0 > Dynamic subtest vcs0: SUCCESS (80.243s) > Starting dynamic subtest: vecs0 > > Interesting part is that according to dmesg it never got to the vecs0 dynamic > subtest - any idea what happened there? Yep, we ran out of time. We still had 11 seconds left to execute something but then that test took centuries. > > 2) > > I915_pm_dc I'd say you just gotten unlucky that test went from always > skipping on SKL to trying to run it and then it failed. But I don't know > enough about the test to tell you why. Adding Jigar and Anshuman as test > author and reviewer who might be able to shed some light here. > > Regards, > > Tvrtko > > > I found code that seemed to be relevant > > > > [jimc@frodo igt-ci-tags.git]$ git remote -v > > origin https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags.git > > <https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags.git> (fetch) > > origin https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags.git > > <https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags.git> (push) > > > > I built it, got an error, threw that to google, > > found a patch on i-g-t from > > commit 1ff3e5ae99ceb66d2926d58635d0379ce971065a (HEAD -> master) > > Author: Lyude Paul mailto:ly...@redhat.com>> > > Date: Mon Apr 15 14:57:23 2019 -0400 > > > > and applied it > > it fixed the one problem > > > > then I looked at previous head > > > > commit f052e49a43cc9704ea5f240df15dd9d3dfed68ab (origin/master, origin/HEAD) > > Author: Simon Ser mailto:simon@intel.com>> > > Date: Wed Apr 24 19:15:26 2019 +0300 > > > > It sure seems that tree is stale. That tree's master ref does not get updated. It's only for storing tags. That test result comparison was too long to fit into patchwork so the build information at the bottom is missing, but the BAT results have it: IGT_6193: 080869f804cb86b25a38889e5ce9a870571cd8c4 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git -- Petri Latvala
Re: [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Free all DMC payloads
On Mon, Aug 09, 2021 at 03:05:37PM -0700, Lucas De Marchi wrote: > On Mon, Aug 09, 2021 at 10:00:34PM +, Patchwork wrote: > > == Series Details == > > > > Series: drm/i915: Free all DMC payloads > > URL : https://patchwork.freedesktop.org/series/93521/ > > State : failure > > > > == Summary == > > > > CI Bug Log - changes from CI_DRM_10461 -> Patchwork_20789 > > > > > > Summary > > --- > > > > **FAILURE** > > > > Serious unknown changes coming with Patchwork_20789 absolutely need to be > > verified manually. > > > > If you think the reported changes have nothing to do with the changes > > introduced in Patchwork_20789, 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/Patchwork_20789/index.html > > > > Possible new issues > > --- > > > > Here are the unknown changes that may have been introduced in > > Patchwork_20789: > > > > ### IGT changes ### > > > > Possible regressions > > > > * igt@core_hotunplug@unbind-rebind: > >- fi-rkl-guc: [PASS][1] -> [DMESG-WARN][2] > > [1]: > > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10461/fi-rkl-guc/igt@core_hotunp...@unbind-rebind.html > > [2]: > > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20789/fi-rkl-guc/igt@core_hotunp...@unbind-rebind.html > > > AccessDenied > Access Denied > H0EEPVCR70HST0VN > 493NvO3WzGD1zhkvRRk6un+6HruE4hYwXX3W4OCSZWpluozyHRqL5h3rprp7q7erF2wu5xQa0is= > > > ??? > > I noticed this happening sometimes and working after some minutes/hours. > Is there a perm going out of sync on CI systems? Because of reasons, the patchwork post happens before syncing the files to AWS. The file sync can fail or take an undetermined amount of time so it's done asynchronously. Typically the files get synced within around 2 minutes of posting to patchwork but sometimes (~once a month) sync fails and the files get there as part of the next test result sync job. -- Petri Latvala
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 1/7] lib/i915/gem_mman: add FIXED mmap mode
On Wed, Jul 28, 2021 at 06:53:50PM -0700, Dixit, Ashutosh wrote: > On Tue, 27 Jul 2021 23:08:40 -0700, Petri Latvala wrote: > > > > On Tue, Jul 27, 2021 at 07:01:24PM -0700, Dixit, Ashutosh wrote: > > > On Mon, 26 Jul 2021 05:03:04 -0700, Matthew Auld wrote: > > > > > > > > diff --git a/lib/i915/gem_mman.c b/lib/i915/gem_mman.c > > > > index 4b4f2114..e2514f0c 100644 > > > > --- a/lib/i915/gem_mman.c > > > > +++ b/lib/i915/gem_mman.c > > > > @@ -497,6 +497,43 @@ void *gem_mmap_offset__cpu(int fd, uint32_t > > > > handle, uint64_t offset, > > > > return ptr; > > > > } > > > > > > > > +#define LOCAL_I915_MMAP_OFFSET_FIXED 4 > > > > > > Cc: @Petri Latvala > > > > > > This use of LOCAL declarations is more related to the methodology we > > > follow > > > in IGT rather than this patch. We have seen in the past that such LOCAL's > > > linger on in the code for months and years till someone decides to clean > > > them so the question is can we prevent these LOCAL's from getting > > > introduced in the first place. > > > > > > One reason for these is that we sync IGT headers with drm-next whereas IGT > > > is used to test drm-tip. So the delta between the two results in such > > > LOCAL's as in this case. > > > > > > My proposal is that even if we don't start sync'ing IGT headers with > > > drm-tip (instead of drm-next) we allow direct modification of the headers > > > when needed to avoid introducing such LOCAL's. So in the above case we > > > would add: > > > > > > #define I915_MMAP_OFFSET_FIXED 4 > > > > > > to i915_drm.h as part of this patch and then just use > > > I915_MMAP_OFFSET_FIXED. If another sync happens from drm-next before this > > > #define has appeared there, the compile will break and whoever syncs will > > > need to add this again to i915_drm.h. > > > > I don't like that kind of a breakage at all. That enforces mandatory > > fixups to some poor developer working on unrelated code who doesn't > > necessarily know how to even fix it easily. > > > > Of course an argument can be made that it's an i915 token in an i915 > > header so it will be the i915 people doing it, but for a general case > > that's going to cause more harm than it solves problems, I feel. > > OK, let's not change anything with the headers we import for now. > > > > What do people think about a scheme such as this? The other, perhaps > > > better, option of course is to sync the headers directly with drm-tip > > > (whenever and as often as needed). But the goal in both cases is to avoid > > > LOCAL's, or other things like #ifndef's distributed throughout multiple > > > source files which we also do in such cases. A centralized internal header > > > to contain such declarations might not be so bad. Thanks. > > > > A separate manually written header for new tokens that are not yet in > > drm-next might be the least bad of all options. Although now that I've > > said it, the perfect world would have new tokens done like this: > > > > #ifndef I915_MMAP_OFFSET_FIXED > > #define I915_MMAP_OFFSET_FIXED 4 > > #else > > _Static_assert(I915_MMAP_OFFSET_FIXED == 4, "ABI broken, yikes"); > > #endif > > > > In a different language wrapping all that in a > > > > MAYBE_DECLARE(I915_MMAP_OFFSET_FIXED, 4) > > > > might be easier but with C preprocessor it's a bit more... involved. A > > separate build-time script to generate that header maybe? Such a > > script could also just completely omit the definition if header copies > > already introduce the token. > > IMO all this wouldn't do much more that what gcc already does. For example, > for this: > > #define I915_MMAP_OFFSET_FIXED 4 > #define I915_MMAP_OFFSET_FIXED 4 > > gcc silently ignores the second #define, whereas for: > > #define I915_MMAP_OFFSET_FIXED 4 > #define I915_MMAP_OFFSET_FIXED 5 > > gcc will warn that second I915_MMAP_OFFSET_FIXED is redefined. > > And gcc will error out on things like struct redeclaration. Ah that's good then! (For the record, C99 6.10.3 states this is even standard C behaviour) > > > Recap: > > > > 1) We have kernel headers copied into IGT to ensure it builds fine > > without latest-and-greatest headers installed on the system. > > > > 2) Copies are from drm-next to ensure the next person to copy the > > headers doesn't accidentally drop definitions that
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 1/7] lib/i915/gem_mman: add FIXED mmap mode
On Tue, Jul 27, 2021 at 07:01:24PM -0700, Dixit, Ashutosh wrote: > On Mon, 26 Jul 2021 05:03:04 -0700, Matthew Auld wrote: > > > > diff --git a/lib/i915/gem_mman.c b/lib/i915/gem_mman.c > > index 4b4f2114..e2514f0c 100644 > > --- a/lib/i915/gem_mman.c > > +++ b/lib/i915/gem_mman.c > > @@ -497,6 +497,43 @@ void *gem_mmap_offset__cpu(int fd, uint32_t handle, > > uint64_t offset, > > return ptr; > > } > > > > +#define LOCAL_I915_MMAP_OFFSET_FIXED 4 > > Cc: @Petri Latvala > > This use of LOCAL declarations is more related to the methodology we follow > in IGT rather than this patch. We have seen in the past that such LOCAL's > linger on in the code for months and years till someone decides to clean > them so the question is can we prevent these LOCAL's from getting > introduced in the first place. > > One reason for these is that we sync IGT headers with drm-next whereas IGT > is used to test drm-tip. So the delta between the two results in such > LOCAL's as in this case. > > My proposal is that even if we don't start sync'ing IGT headers with > drm-tip (instead of drm-next) we allow direct modification of the headers > when needed to avoid introducing such LOCAL's. So in the above case we > would add: > > #define I915_MMAP_OFFSET_FIXED 4 > > to i915_drm.h as part of this patch and then just use > I915_MMAP_OFFSET_FIXED. If another sync happens from drm-next before this > #define has appeared there, the compile will break and whoever syncs will > need to add this again to i915_drm.h. I don't like that kind of a breakage at all. That enforces mandatory fixups to some poor developer working on unrelated code who doesn't necessarily know how to even fix it easily. Of course an argument can be made that it's an i915 token in an i915 header so it will be the i915 people doing it, but for a general case that's going to cause more harm than it solves problems, I feel. > What do people think about a scheme such as this? The other, perhaps > better, option of course is to sync the headers directly with drm-tip > (whenever and as often as needed). But the goal in both cases is to avoid > LOCAL's, or other things like #ifndef's distributed throughout multiple > source files which we also do in such cases. A centralized internal header > to contain such declarations might not be so bad. Thanks. A separate manually written header for new tokens that are not yet in drm-next might be the least bad of all options. Although now that I've said it, the perfect world would have new tokens done like this: #ifndef I915_MMAP_OFFSET_FIXED #define I915_MMAP_OFFSET_FIXED 4 #else _Static_assert(I915_MMAP_OFFSET_FIXED == 4, "ABI broken, yikes"); #endif In a different language wrapping all that in a MAYBE_DECLARE(I915_MMAP_OFFSET_FIXED, 4) might be easier but with C preprocessor it's a bit more... involved. A separate build-time script to generate that header maybe? Such a script could also just completely omit the definition if header copies already introduce the token. Recap: 1) We have kernel headers copied into IGT to ensure it builds fine without latest-and-greatest headers installed on the system. 2) Copies are from drm-next to ensure the next person to copy the headers doesn't accidentally drop definitions that originate from a vendor-specific tree. (That same reason is also for why one shouldn't edit the headers manually) 3) To get to drm-next, the kernel code needs to be tested with IGT first, so we need new definitions to test that kernel code in some form. 4) LOCAL_* definitions that are cleaned up later when actual definitions reach drm-next sounds good in theory but in practice the cleaning part is often forgotten. Either way, I think the code using new definitions should use the intended final names so we should just entirely drop the practice of declaring anything LOCAL_*. That way the cleanup is limited to one place. -- Petri Latvala ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] Revert "drm: add a locked version of drm_is_current_master"
On Tue, Jun 22, 2021 at 09:54:09AM +0200, Daniel Vetter wrote:
> This reverts commit 1815d9c86e3090477fbde066ff314a7e9721ee0f.
>
> Unfortunately this inverts the locking hierarchy, so back to the
> drawing board. Full lockdep splat below:
>
> ==
> WARNING: possible circular locking dependency detected
> 5.13.0-rc7-CI-CI_DRM_10254+ #1 Not tainted
> --
> kms_frontbuffer/1087 is trying to acquire lock:
> 88810dcd01a8 (>master_mutex){+.+.}-{3:3}, at:
> drm_is_current_master+0x1b/0x40
> but task is already holding lock:
> 88810dcd0488 (>mode_config.mutex){+.+.}-{3:3}, at:
> drm_mode_getconnector+0x1c6/0x4a0
> which lock already depends on the new lock.
> the existing dependency chain (in reverse order) is:
> -> #2 (>mode_config.mutex){+.+.}-{3:3}:
>__mutex_lock+0xab/0x970
>drm_client_modeset_probe+0x22e/0xca0
>__drm_fb_helper_initial_config_and_unlock+0x42/0x540
>intel_fbdev_initial_config+0xf/0x20 [i915]
>async_run_entry_fn+0x28/0x130
>process_one_work+0x26d/0x5c0
>worker_thread+0x37/0x380
>kthread+0x144/0x170
>ret_from_fork+0x1f/0x30
> -> #1 (>modeset_mutex){+.+.}-{3:3}:
>__mutex_lock+0xab/0x970
>drm_client_modeset_commit_locked+0x1c/0x180
>drm_client_modeset_commit+0x1c/0x40
>__drm_fb_helper_restore_fbdev_mode_unlocked+0x88/0xb0
>drm_fb_helper_set_par+0x34/0x40
>intel_fbdev_set_par+0x11/0x40 [i915]
>fbcon_init+0x270/0x4f0
>visual_init+0xc6/0x130
>do_bind_con_driver+0x1e5/0x2d0
>do_take_over_console+0x10e/0x180
>do_fbcon_takeover+0x53/0xb0
>register_framebuffer+0x22d/0x310
>__drm_fb_helper_initial_config_and_unlock+0x36c/0x540
>intel_fbdev_initial_config+0xf/0x20 [i915]
>async_run_entry_fn+0x28/0x130
>process_one_work+0x26d/0x5c0
>worker_thread+0x37/0x380
>kthread+0x144/0x170
>ret_from_fork+0x1f/0x30
> -> #0 (>master_mutex){+.+.}-{3:3}:
>__lock_acquire+0x151e/0x2590
>lock_acquire+0xd1/0x3d0
>__mutex_lock+0xab/0x970
>drm_is_current_master+0x1b/0x40
>drm_mode_getconnector+0x37e/0x4a0
>drm_ioctl_kernel+0xa8/0xf0
>drm_ioctl+0x1e8/0x390
>__x64_sys_ioctl+0x6a/0xa0
>do_syscall_64+0x39/0xb0
>entry_SYSCALL_64_after_hwframe+0x44/0xae
> other info that might help us debug this:
> Chain exists of: >master_mutex --> >modeset_mutex -->
> >mode_config.mutex
> Possible unsafe locking scenario:
>CPU0CPU1
>
> lock(>mode_config.mutex);
>lock(>modeset_mutex);
>lock(>mode_config.mutex);
> lock(>master_mutex);
> *** DEADLOCK ***
> 1 lock held by kms_frontbuffer/1087:
> #0: 88810dcd0488 (>mode_config.mutex){+.+.}-{3:3}, at:
> drm_mode_getconnector+0x1c6/0x4a0
> stack backtrace:
> CPU: 7 PID: 1087 Comm: kms_frontbuffer Not tainted
> 5.13.0-rc7-CI-CI_DRM_10254+ #1
> Hardware name: Intel Corporation Ice Lake Client Platform/IceLake U DDR4
> SODIMM PD RVP TLC, BIOS ICLSFWR1.R00.3234.A01.1906141750 06/14/2019
> Call Trace:
> dump_stack+0x7f/0xad
> check_noncircular+0x12e/0x150
> __lock_acquire+0x151e/0x2590
> lock_acquire+0xd1/0x3d0
> __mutex_lock+0xab/0x970
> drm_is_current_master+0x1b/0x40
> drm_mode_getconnector+0x37e/0x4a0
> drm_ioctl_kernel+0xa8/0xf0
> drm_ioctl+0x1e8/0x390
> __x64_sys_ioctl+0x6a/0xa0
> do_syscall_64+0x39/0xb0
> entry_SYSCALL_64_after_hwframe+0x44/0xae
>
> daniel@phenom:~/linux/drm-misc-fixes$ dim fixes
> 1815d9c86e3090477fbde066ff314a7e9721ee0f
> Fixes: 1815d9c86e30 ("drm: add a locked version of drm_is_current_master")
> Cc: Desmond Cheong Zhi Xi
> Cc: Emil Velikov
> Cc: sta...@vger.kernel.org
> Signed-off-by: Daniel Vetter
> Cc: Maarten Lankhorst
> Cc: Maxime Ripard
> Cc: Thomas Zimmermann
> Cc: David Airlie
> Cc: Daniel Vetter
You have your "dim fixes" command line in the commit message.
This goes through Intel's CI shortly, if it agrees with this then
Testcase: igt/debugfs_test/read_all_entries
Acked-by: Petri Latvala
> ---
> drivers/gpu/drm/drm_auth.c | 51 ++
> 1 file changed, 19 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
> index 86d4b72e95cb..232abbba3686 100644
> --- a/drivers/gpu/drm/drm_auth.c
> +++ b/drivers/gpu/drm/drm_auth
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] tests/kms_dp_dsc: Avoid SIGSEGV when release DRM connector.
On Mon, May 31, 2021 at 11:39:22PM +0800, Lee Shawn C wrote:
> Got SIGSEGV fault while running kms_dp_dsc test but did not
> connect DP DSC capable monitor on eDP/DP port. This test daemon
> should "SKIP" test without any problem. We found kms_dp_dsc
> can't get proper drmModeConnector and caused this SIGSEGV fault
> when release it. Make sure drmModeConnector is available before
> free it can avoid this issue.
>
> Signed-off-by: Lee Shawn C
Reviewed-by: Petri Latvala
> ---
> tests/kms_dp_dsc.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/tests/kms_dp_dsc.c b/tests/kms_dp_dsc.c
> index 2446fd82bba3..ea7c9f4f72ce 100644
> --- a/tests/kms_dp_dsc.c
> +++ b/tests/kms_dp_dsc.c
> @@ -262,7 +262,7 @@ igt_main
> data_t data = {};
> igt_output_t *output;
> drmModeRes *res;
> - drmModeConnector *connector;
> + drmModeConnector *connector = NULL;
> int i, test_conn_cnt, test_cnt;
> int tests[] = {DRM_MODE_CONNECTOR_eDP, DRM_MODE_CONNECTOR_DisplayPort};
>
> @@ -311,7 +311,8 @@ igt_main
> }
>
> igt_fixture {
> - drmModeFreeConnector(connector);
> + if (connector)
> + drmModeFreeConnector(connector);
> drmModeFreeResources(res);
> close(data.debugfs_fd);
> close(data.drm_fd);
> --
> 2.17.1
>
> ___
> igt-dev mailing list
> igt-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] [RFC] tests/drm_read: Fix subtest invalid-buffer
On Fri, May 28, 2021 at 10:02:47AM +0530, Vidya Srinivas wrote:
> Using (void *)-1 directly in read is aborting on chrome systems.
> Following message is seen.
>
> Starting subtest: invalid-buffer
> *** buffer overflow detected ***: terminated
> Received signal SIGABRT.
> Stack trace:
> Aborted (core dumped)
>
> Patch just adds a pointer variable and uses it in read.
>
> Signed-off-by: Vidya Srinivas
> ---
> tests/drm_read.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tests/drm_read.c b/tests/drm_read.c
> index ccf9d822fd8d..2fdec5be4078 100644
> --- a/tests/drm_read.c
> +++ b/tests/drm_read.c
> @@ -103,10 +103,11 @@ static void teardown(int fd)
> static void test_invalid_buffer(int in)
> {
> int fd = setup(in, 0);
> + void *add = (void *)-1;
>
> alarm(1);
>
> - igt_assert_eq(read(fd, (void *)-1, 4096), -1);
> + igt_assert_eq(read(fd, add, 4096), -1);
> igt_assert_eq(errno, EFAULT);
>
> teardown(fd);
This looked weird but then I checked what glibc is actually
doing. This is FORTIFY_SOURCE in action, and read() checks the buffer
with __builtin_object_size() that it has room for the read. Which it
can only do here if the address is a literal.
Reviewed-by: Petri Latvala
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] [RFC] tests/kms_big_fb: Wait for vblank before collecting CRC
On Thu, Jun 10, 2021 at 08:38:42AM +, Srinivas, Vidya wrote: > Hello Juha-Pekka, > > https://patchwork.freedesktop.org/series/90389/#rev7 shows PASS for all CI. > However I don’t see kms_big_fb all the subtests running in CI. In the logs I > see pass for linear-32bpp-rotate-0 The default view in the CI results only shows tests that have issues. "view -> shards all" from the top shows all tests. https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5907/shards-all.html?testfilter=kms_big_fb -- Petri Latvala ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] [RFC] tests/kms_plane_alpha_blend: Fix coverage-vs-premult-vs-constant tests
On Tue, Jun 01, 2021 at 05:15:39PM +0530, Vidya Srinivas wrote: > tests/kms_plane_alpha_blend: Fix coverage-vs-premult-vs-constant tests > > Few Gen11 systems show CRC mismatch. Make coverage-vs-premult-vs-constant > code similar to constant_alpha_min or basic_alpha > > Signed-off-by: Vidya Srinivas Please make the first line of the commit message a statement that tells what change you're making, and in the full text block state why that's done. "Fix a-b-c tests" is useless later when browsing oneliner git logs. It doesn't even tell which problem is fixed. Meaning, something like: == tests/kms_plane_alpha_blend: Don't set primary fb color in coverage-vs-premult-vs-constant Similar change has already been done in tests xxx and yyy. This fixes CRC mismatches seen with some Gen11 systems. Signed-off-by etc == -- Petri Latvala ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] gem_watchdog: Skip test if default request expiry is not compiled in
On Mon, May 24, 2021 at 03:01:13PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin
>
> Test incorrectly assumes no modparam means default expiry, while in
> reality no modparam means old kernel / de-selected feature in which
> case test should skip.
>
> Signed-off-by: Tvrtko Ursulin
> ---
> tests/i915/gem_watchdog.c | 16 +++-
> 1 file changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/tests/i915/gem_watchdog.c b/tests/i915/gem_watchdog.c
> index 8f9fb17750fb..89992a3c5099 100644
> --- a/tests/i915/gem_watchdog.c
> +++ b/tests/i915/gem_watchdog.c
> @@ -630,6 +630,7 @@ igt_main
>
> igt_fixture {
> struct drm_i915_query_item item;
> + const unsigned int timeout = 1;
> char *tmp;
>
> i915 = drm_open_driver_master(DRIVER_INTEL);
> @@ -639,16 +640,13 @@ igt_main
> igt_require_gem(i915);
>
> tmp = __igt_params_get(i915, "request_timeout_ms");
> - if (tmp) {
> - const unsigned int timeout = 1;
> + igt_skip_on_f(!tmp || !atoi(tmp),
> + "Request expiry not supported!");
Newline missing at the end.
--
Petri Latvala
> + free(tmp);
>
> - igt_params_save_and_set(i915, "request_timeout_ms",
> - "%u", timeout * 1000);
> - default_timeout_wait_s = timeout * 5;
> - free(tmp);
> - } else {
> - default_timeout_wait_s = 12;
> - }
> + igt_params_save_and_set(i915, "request_timeout_ms", "%u",
> + timeout * 1000);
> + default_timeout_wait_s = timeout * 5;
>
> i915 = gem_reopen_driver(i915); /* Apply modparam. */
>
> --
> 2.30.2
>
> ___
> igt-dev mailing list
> igt-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 0/9] DG1/LMEM uAPI basics v2
eate.c| 1 + > tests/i915/gem_exec_endless.c | 1 + > tests/i915/gem_exec_fair.c | 1 + > tests/i915/gem_exec_fence.c | 1 + > tests/i915/gem_exec_flush.c | 1 + > tests/i915/gem_exec_gttfill.c | 1 + > tests/i915/gem_exec_latency.c | 1 + > tests/i915/gem_exec_lut_handle.c| 1 + > tests/i915/gem_exec_nop.c | 1 + > tests/i915/gem_exec_parallel.c | 1 + > tests/i915/gem_exec_params.c| 1 + > tests/i915/gem_exec_reloc.c | 1 + > tests/i915/gem_exec_schedule.c | 1 + > tests/i915/gem_exec_store.c | 1 + > tests/i915/gem_exec_suspend.c | 1 + > tests/i915/gem_exec_whisper.c | 1 + > tests/i915/gem_fd_exhaustion.c | 2 +- > tests/i915/gem_fence_thrash.c | 2 +- > tests/i915/gem_fence_upload.c | 2 +- > tests/i915/gem_fenced_exec_thrash.c | 1 + > tests/i915/gem_flink_race.c | 2 +- > tests/i915/gem_gpgpu_fill.c | 61 +++- > tests/i915/gem_gtt_cpu_tlb.c| 2 +- > tests/i915/gem_gtt_hog.c| 1 + > tests/i915/gem_gtt_speed.c | 2 +- > tests/i915/gem_huc_copy.c | 1 + > tests/i915/gem_linear_blits.c | 1 + > tests/i915/gem_lut_handle.c | 2 +- > tests/i915/gem_madvise.c| 2 +- > tests/i915/gem_media_fill.c | 57 ++- > tests/i915/gem_mmap.c | 2 +- > tests/i915/gem_mmap_gtt.c | 1 + > tests/i915/gem_mmap_offset.c| 1 + > tests/i915/gem_mmap_wc.c| 2 +- > tests/i915/gem_ppgtt.c | 1 + > tests/i915/gem_pread.c | 2 +- > tests/i915/gem_pwrite.c | 2 +- > tests/i915/gem_readwrite.c | 2 +- > tests/i915/gem_reset_stats.c| 1 + > tests/i915/gem_ringfill.c | 1 + > tests/i915/gem_set_tiling_vs_gtt.c | 2 +- > tests/i915/gem_set_tiling_vs_pwrite.c | 2 +- > tests/i915/gem_shrink.c | 1 + > tests/i915/gem_softpin.c| 1 + > tests/i915/gem_streaming_writes.c | 1 + > tests/i915/gem_sync.c | 1 + > tests/i915/gem_tiled_fence_blits.c | 1 + > tests/i915/gem_tiled_pread_basic.c | 2 +- > tests/i915/gem_tiled_pread_pwrite.c | 2 +- > tests/i915/gem_tiled_swapping.c | 2 +- > tests/i915/gem_tiled_wb.c | 2 +- > tests/i915/gem_tiled_wc.c | 2 +- > tests/i915/gem_tiling_max_stride.c | 2 +- > tests/i915/gem_unfence_active_buffers.c | 1 + > tests/i915/gem_unref_active_buffers.c | 1 + > tests/i915/gem_userptr_blits.c | 1 + > tests/i915/gem_vm_create.c | 1 + > tests/i915/gem_wait.c | 1 + > tests/i915/gem_watchdog.c | 1 + > tests/i915/gem_workarounds.c| 1 + > tests/i915/gen3_mixed_blits.c | 1 + > tests/i915/gen3_render_linear_blits.c | 1 + > tests/i915/gen3_render_mixed_blits.c| 1 + > tests/i915/gen3_render_tiledx_blits.c | 1 + > tests/i915/gen3_render_tiledy_blits.c | 1 + > tests/i915/gen7_exec_parse.c| 1 + > tests/i915/gen9_exec_parse.c| 1 + > tests/i915/i915_hangman.c | 1 + > tests/i915/i915_module_load.c | 2 +- > tests/i915/i915_pm_rc6_residency.c | 1 + > tests/i915/i915_pm_rpm.c| 1 + > tests/i915/i915_suspend.c | 1 + > tests/i915/perf_pmu.c | 1 + > tests/i915/sysfs_clients.c | 1 + > tests/i915/sysfs_timeslice_duration.c | 1 + > tests/kms_big_fb.c | 2 +- > tests/kms_ccs.c | 2 +- > tests/kms_flip.c| 2 +- > tests/kms_frontbuffer_tracking.c| 1 + > tests/kms_getfb.c | 2 +- > tests/prime_busy.c | 1 + > tests/prime_mmap.c | 2 +- > tests/prime_mmap_kms.c | 2 +- > tests/prime_self_import.c | 2 +- > tests/prime_vgem.c | 1 + > tools/intel_reg.c | 2 +- > 149 files changed, 1447 insertions(+), 134 deletions(-) > create mode 100644 lib/i915/gem_create.h > create mode 100644 lib/i915/intel_memory_region.c > create mode 100644 lib/i915/intel_memory_region.h Series is Acked-by: Petri Latvala ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 00/12] DG1/LMEM uAPI basics
On Wed, May 19, 2021 at 12:36:17PM +0100, Matthew Auld wrote: > On Wed, 19 May 2021 at 12:00, Petri Latvala wrote: > > > > On Wed, May 19, 2021 at 11:45:17AM +0100, Matthew Auld wrote: > > > On Wed, 19 May 2021 at 09:49, Petri Latvala > > > wrote: > > > > > > > > On Wed, May 19, 2021 at 09:13:37AM +0100, Matthew Auld wrote: > > > > > On Tue, 11 May 2021 at 17:52, Matthew Auld > > > > > wrote: > > > > > > > > > > > > Just the really basic stuff, which unlocks adding more interesting > > > > > > testcases > > > > > > later, like gem_lmem_swapping. > > > > > > > > > > > > On the kernel side we landed the uAPI bits[1] behind CONFIG_BROKEN, > > > > > > which is > > > > > > already enabled in CI builds, so it should be possible to get some > > > > > > more BAT > > > > > > testing(outside of just the selftests) on DG1 to the point where we > > > > > > can start to > > > > > > exercise the LMEM paths with the new bits of uAPI. > > > > > > > > > > > > [1] https://patchwork.freedesktop.org/series/89648/ > > > > > > > > > > Petri, any thoughts on this series? As an initial step we just need > > > > > some way to start exercising the new bits of uAPI, and from that we > > > > > can add more interesting test cases. > > > > > > > > This series is in a confused state. First there's the addition of > > > > local definitions and ioctl tokens, then they are replaced with the > > > > proper stuff... > > > > > > Yeah, I think that's how it is internally. Maybe the idea with that > > > was to somehow land the igt changes first, before the kernel stuff > > > potentially landed? I can clean this up and just start with the proper > > > stuff. > > > > > > > > > > > When this was starting to get developed the idea was to avoid icky > > > > cases that break _testing_. Not tests, testing. Remember when engine > > > > discovery was being developed and we had cases where for_each_engine > > > > loop didn't progress, causing stuff like > > > > > > > > for_each_engine(...) > > > > igt_subtest(...) > > > > > > > > to never enter a subtest? > > > > > > > > Pushing for stubbing memory regions ultimately wanted to avoid cases > > > > where for_each_combination(memregions) breaks test enumeration. > > > > > > > > It all boils down to: Can that break? Can we have cases where the > > > > query gives garbage? Can it give two million smem regions? Can it give > > > > 0 regions, or -1 regions? And what happens then? > > > > > > On integrated platforms it can only return one region: smem. If we > > > somehow don't have a smem region then the i915 module load would have > > > failed, since we must have been unable to populate the > > > i915->mm.regions. On DG1 we just get the extra lmem region, and for Xe > > > HP multi-tile we get a few more lmem regions, but again if we can't > > > populate the i915->mm.regions with the required regions then we fail > > > driver init. The only "optional" region is stolen memory, but for that > > > we don't expose it to userspace. > > > > > > The query will fail on !CONFIG_BROKEN kernels though, where it just > > > returns -ENODEV, or of course some other error if the user provided an > > > invalid query. > > > > Behaviour between success/failure is business as usual. The danger in > > the initial discussions for this was token value overloading or such, > > stuff like IGT thinking it's calling DRM_IOCTL_DISTANCE_TO_LUNCHTIME > > but that value was meanwhile taken by > > DRM_IOCTL_HALT_AND_CATCH_FIRE. Of course the query change is not a new > > ioctl but is value mismatch a possibility in a theoretical worst case > > and how does the breakage show in testing? > > Hmm, do you mean if we decide to change the uAPI before dropping the > CONFIG_BROKEN? That's for sure a possibility. Yes. For ioctl numbers the realistic scenario is that someone else takes over the value but that's not the case here for the query token... > Maybe we can do something like the prelim thing? > > /** XXX: these are subject to change, hence leaving some holes */ > I915_GEM_CREATE_EXT_MEMORY_REGIONS 0xff > DRM_I915_QUERY_MEMORY_REGIONS 0xff > > That way, if we do need to change anything here we can add the new > version(using the lowest available value at the time) and also keep > the old version which should be backwards compat, then migrate igt > over to the new and then drop the old from the kernel? And then at > some point also drop CONFIG_BROKEN once the uAPI is final? > > I think it should be the case that all other related values are > effectively namespaced within that. That might be overkill for this if the only real danger is the query token having the wrong meaning, and that value being fully in i915 control. After mulling back and forth I'm ready to slap an A-b on this series after the cleanup mentioned above. -- Petri Latvala ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 00/12] DG1/LMEM uAPI basics
On Wed, May 19, 2021 at 11:45:17AM +0100, Matthew Auld wrote: > On Wed, 19 May 2021 at 09:49, Petri Latvala wrote: > > > > On Wed, May 19, 2021 at 09:13:37AM +0100, Matthew Auld wrote: > > > On Tue, 11 May 2021 at 17:52, Matthew Auld wrote: > > > > > > > > Just the really basic stuff, which unlocks adding more interesting > > > > testcases > > > > later, like gem_lmem_swapping. > > > > > > > > On the kernel side we landed the uAPI bits[1] behind CONFIG_BROKEN, > > > > which is > > > > already enabled in CI builds, so it should be possible to get some more > > > > BAT > > > > testing(outside of just the selftests) on DG1 to the point where we can > > > > start to > > > > exercise the LMEM paths with the new bits of uAPI. > > > > > > > > [1] https://patchwork.freedesktop.org/series/89648/ > > > > > > Petri, any thoughts on this series? As an initial step we just need > > > some way to start exercising the new bits of uAPI, and from that we > > > can add more interesting test cases. > > > > This series is in a confused state. First there's the addition of > > local definitions and ioctl tokens, then they are replaced with the > > proper stuff... > > Yeah, I think that's how it is internally. Maybe the idea with that > was to somehow land the igt changes first, before the kernel stuff > potentially landed? I can clean this up and just start with the proper > stuff. > > > > > When this was starting to get developed the idea was to avoid icky > > cases that break _testing_. Not tests, testing. Remember when engine > > discovery was being developed and we had cases where for_each_engine > > loop didn't progress, causing stuff like > > > > for_each_engine(...) > > igt_subtest(...) > > > > to never enter a subtest? > > > > Pushing for stubbing memory regions ultimately wanted to avoid cases > > where for_each_combination(memregions) breaks test enumeration. > > > > It all boils down to: Can that break? Can we have cases where the > > query gives garbage? Can it give two million smem regions? Can it give > > 0 regions, or -1 regions? And what happens then? > > On integrated platforms it can only return one region: smem. If we > somehow don't have a smem region then the i915 module load would have > failed, since we must have been unable to populate the > i915->mm.regions. On DG1 we just get the extra lmem region, and for Xe > HP multi-tile we get a few more lmem regions, but again if we can't > populate the i915->mm.regions with the required regions then we fail > driver init. The only "optional" region is stolen memory, but for that > we don't expose it to userspace. > > The query will fail on !CONFIG_BROKEN kernels though, where it just > returns -ENODEV, or of course some other error if the user provided an > invalid query. Behaviour between success/failure is business as usual. The danger in the initial discussions for this was token value overloading or such, stuff like IGT thinking it's calling DRM_IOCTL_DISTANCE_TO_LUNCHTIME but that value was meanwhile taken by DRM_IOCTL_HALT_AND_CATCH_FIRE. Of course the query change is not a new ioctl but is value mismatch a possibility in a theoretical worst case and how does the breakage show in testing? -- Petri Latvala ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 00/12] DG1/LMEM uAPI basics
On Wed, May 19, 2021 at 09:13:37AM +0100, Matthew Auld wrote: > On Tue, 11 May 2021 at 17:52, Matthew Auld wrote: > > > > Just the really basic stuff, which unlocks adding more interesting testcases > > later, like gem_lmem_swapping. > > > > On the kernel side we landed the uAPI bits[1] behind CONFIG_BROKEN, which is > > already enabled in CI builds, so it should be possible to get some more BAT > > testing(outside of just the selftests) on DG1 to the point where we can > > start to > > exercise the LMEM paths with the new bits of uAPI. > > > > [1] https://patchwork.freedesktop.org/series/89648/ > > Petri, any thoughts on this series? As an initial step we just need > some way to start exercising the new bits of uAPI, and from that we > can add more interesting test cases. This series is in a confused state. First there's the addition of local definitions and ioctl tokens, then they are replaced with the proper stuff... When this was starting to get developed the idea was to avoid icky cases that break _testing_. Not tests, testing. Remember when engine discovery was being developed and we had cases where for_each_engine loop didn't progress, causing stuff like for_each_engine(...) igt_subtest(...) to never enter a subtest? Pushing for stubbing memory regions ultimately wanted to avoid cases where for_each_combination(memregions) breaks test enumeration. It all boils down to: Can that break? Can we have cases where the query gives garbage? Can it give two million smem regions? Can it give 0 regions, or -1 regions? And what happens then? Is it just gem_create_ext that's hiding behind CONFIG_BROKEN or also the querying? -- Petri Latvala ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 07/12] i915_drm.h sync
On Tue, May 11, 2021 at 05:51:12PM +0100, Matthew Auld wrote:
> Sync to get gem_create_ext and the regions query stuff.
Kernel commit sha in commit message please.
--
Petri Latvala
>
> Signed-off-by: Matthew Auld
> ---
> include/drm-uapi/i915_drm.h | 394
> 1 file changed, 360 insertions(+), 34 deletions(-)
>
> diff --git a/include/drm-uapi/i915_drm.h b/include/drm-uapi/i915_drm.h
> index bf9ea471..a1c0030c 100644
> --- a/include/drm-uapi/i915_drm.h
> +++ b/include/drm-uapi/i915_drm.h
> @@ -62,8 +62,8 @@ extern "C" {
> #define I915_ERROR_UEVENT"ERROR"
> #define I915_RESET_UEVENT"RESET"
>
> -/*
> - * i915_user_extension: Base class for defining a chain of extensions
> +/**
> + * struct i915_user_extension - Base class for defining a chain of extensions
> *
> * Many interfaces need to grow over time. In most cases we can simply
> * extend the struct and have userspace pass in more data. Another option,
> @@ -76,12 +76,58 @@ extern "C" {
> * increasing complexity, and for large parts of that interface to be
> * entirely optional. The downside is more pointer chasing; chasing across
> * the boundary with pointers encapsulated inside u64.
> + *
> + * Example chaining:
> + *
> + * .. code-block:: C
> + *
> + * struct i915_user_extension ext3 {
> + * .next_extension = 0, // end
> + * .name = ...,
> + * };
> + * struct i915_user_extension ext2 {
> + * .next_extension = (uintptr_t),
> + * .name = ...,
> + * };
> + * struct i915_user_extension ext1 {
> + * .next_extension = (uintptr_t),
> + * .name = ...,
> + * };
> + *
> + * Typically the struct i915_user_extension would be embedded in some uAPI
> + * struct, and in this case we would feed it the head of the chain(i.e ext1),
> + * which would then apply all of the above extensions.
> + *
> */
> struct i915_user_extension {
> + /**
> + * @next_extension:
> + *
> + * Pointer to the next struct i915_user_extension, or zero if the end.
> + */
> __u64 next_extension;
> + /**
> + * @name: Name of the extension.
> + *
> + * Note that the name here is just some integer.
> + *
> + * Also note that the name space for this is not global for the whole
> + * driver, but rather its scope/meaning is limited to the specific piece
> + * of uAPI which has embedded the struct i915_user_extension.
> + */
> __u32 name;
> - __u32 flags; /* All undefined bits must be zero. */
> - __u32 rsvd[4]; /* Reserved for future use; must be zero. */
> + /**
> + * @flags: MBZ
> + *
> + * All undefined bits must be zero.
> + */
> + __u32 flags;
> + /**
> + * @rsvd: MBZ
> + *
> + * Reserved for future use; must be zero.
> + */
> + __u32 rsvd[4];
> };
>
> /*
> @@ -360,6 +406,7 @@ typedef struct _drm_i915_sarea {
> #define DRM_I915_QUERY 0x39
> #define DRM_I915_GEM_VM_CREATE 0x3a
> #define DRM_I915_GEM_VM_DESTROY 0x3b
> +#define DRM_I915_GEM_CREATE_EXT 0x3c
> /* Must be kept compact -- no holes */
>
> #define DRM_IOCTL_I915_INIT DRM_IOW( DRM_COMMAND_BASE +
> DRM_I915_INIT, drm_i915_init_t)
> @@ -392,6 +439,7 @@ typedef struct _drm_i915_sarea {
> #define DRM_IOCTL_I915_GEM_ENTERVT DRM_IO(DRM_COMMAND_BASE +
> DRM_I915_GEM_ENTERVT)
> #define DRM_IOCTL_I915_GEM_LEAVEVT DRM_IO(DRM_COMMAND_BASE +
> DRM_I915_GEM_LEAVEVT)
> #define DRM_IOCTL_I915_GEM_CREATEDRM_IOWR(DRM_COMMAND_BASE +
> DRM_I915_GEM_CREATE, struct drm_i915_gem_create)
> +#define DRM_IOCTL_I915_GEM_CREATE_EXTDRM_IOWR(DRM_COMMAND_BASE +
> DRM_I915_GEM_CREATE_EXT, struct drm_i915_gem_create_ext)
> #define DRM_IOCTL_I915_GEM_PREAD DRM_IOW (DRM_COMMAND_BASE +
> DRM_I915_GEM_PREAD, struct drm_i915_gem_pread)
> #define DRM_IOCTL_I915_GEM_PWRITEDRM_IOW (DRM_COMMAND_BASE +
> DRM_I915_GEM_PWRITE, struct drm_i915_gem_pwrite)
> #define DRM_IOCTL_I915_GEM_MMAP DRM_IOWR(DRM_COMMAND_BASE +
> DRM_I915_GEM_MMAP, struct drm_i915_gem_mmap)
> @@ -943,6 +991,7 @@ struct drm_i915_gem_exec_object {
> __u64 offset;
> };
>
> +/* DRM_IOCTL_I915_GEM_EXECBUFFER was removed in Linux 5.13 */
> struct drm_i915_gem_execbuffer {
> /**
>* List of buffers to be validated with their relocations to be
> @@ -1053,12 +1102,12 @@ struct drm_i915_gem_exec_fence {
> __u32 flags;
> };
>
> -/**
> +/*
> * See drm_i915_gem_exe
[Intel-gfx] [ANNOUNCE] igt-gpu-tools 1.26
-BEGIN PGP SIGNED MESSAGE- Hash: SHA512 A new igt-gpu-tools release is available with the following changes: - - Autotools support has been entirely dropped in favor of only meson. (Arkadiusz Hiler) - - Tests can now signal that the whole test round should be aborted. (Arkadiusz Hiler) - - Various robustness improvements for Chamelium use. (Arkadiusz Hiler, Kunal Joshi, Imre Deak, et al) - - Device filtering improvements for multi-device use. (Arkadiusz Hiler) - - Device filtering for various Intel tools like intel_gpu_top. (Ayaz A Siddiqui) - - Overhauled kernel parameter handling. (Jani Nikula) - - Introduced an i915 batchbuffer facility. (Zbigniew Kempczyński) - - Improvements for testing nouveau. (Lyude Paul) - - More readable and useful output for lsgpu and other tools that list devices. (Tvrtko Ursulin) - - intel_gpu_top can now show per-client busyness stats. (Tvrtko Ursulin) - - igt_runner can now limit the disk space used by a single test. (Petri Latvala) And many other bug fixes, improvements, cleanups and new tests. Full changelog: Abhinav Kumar (5): tools: rename intel_dp_compliance_hotplug to igt_dp_compliance_hotplug lib/igt_kms: move some of the useful dump functions to igt_kms lib/igt_fb: move the CTS fill framebuffer to igt_fb lib tools: move terminal utility functions to a separate file tools: add support for msm_dp_compliance to IGT Adam Miszczak (3): i915/gem_partial_pwrite_pread: Remove libdrm dependency lib: sync i915_pciids.h with kernel lib/i915: Add DG1 platform definition Anand Moon (1): tests/kms_setmode: basic Improve accuracy with using of confidence interval Andrzej Turko (1): lib/i915: Split gem_create.c from ioctl_wrappers.c Ankit Nautiyal (3): lib/igt_kms: Add support for detecting connector events tests/kms_content_protection: Use library functions for handling uevents lib: Use generic names for APIs that handle uevents Anshuman Gupta (7): tests/i915_pm_lpsp: Nuke the panel-fitter test lib/igt_pm: Add lib func to get lpsp capability tests/i915_pm_lpsp: lpsp platform agnostic support tests/i915_pm_lpsp: screens-disabled subtest use igt_wait tests/i915_pm_rpm: lpsp/non-lpsp screen mode_set_data tests/i915_pm_rpm: Fix plane_subtest assertion tests/i915_pm_lpsp: Add igt_wait to test lpsp Apoorva Singh (4): i915/gem_mmap: Modified offset in subtest "bad-size" i915/gem_mmap_wc: Align subtests with correct function calls i915/gem_render_copy_redux: Remove redundant checks i915/gem_ctx_param: Add subtests description Arjun Melkaveri (2): i915/gem_exec_nop: Fixed Crash issue seen on few platform i915/gem_syncc: Exercise all physical engine selection and legacy rings Arkadiusz Hiler (43): lib/tests: Extract fork helpers lib/tests: Add support for redirecting fork output to /dev/null lib: Make it possible to abort the whole execution from inside of a test runner/runner_tests: Extract helper for inspecting test result runner: Abort the run when test exits with IGT_EXIT_ABORT lib/chamelium: Clear error after checking if chamelium is reachable lib/chamelium: Make it clear that function asserts lib/chamelium: Add functions to initialize XMLRPC only lib/kms: Try to plug all Chamelium ports, abort if it fails chamelium: Retry XMLRPC call when chamelond fails talking with a receiver Remove files related to release 1.25 accidentally added to the repo tests/kms_chamelium: Fix dp-mode-timings test test/kms_chamelium: Start with disabling modeset tests/kms_chamelium: Issue disabling modeset when resetting state tests/kms_chamelium: Test HPD for different mode handling scenarios lib: Support multiple filters lib/drmtest: Introduce __drm_open_driver_another test/kms_prime: Use drm_open_driver_another lib/igt_core: Make assert on invalid magic blocks nesting more verbose lib/igt_core: Disallow nesting of igt_dynamic inside igt_dynamic lib/tests: Add tests for magic control blocks nesting lib/igt_chamelium: Sleep when doing autodiscovery lib/igt_kms: Make igt_display_require() + chamelium more robust lib/igt_core: Don't kill the world after a failed fork python: Stop using cElementTree runner/resultgen: Explain why json creation might have failed lib/core: Print thread:tid with igt_log for non-main threads lib/core: Handle asserts in threads igt/core: Disallow igt_require/skip in non-main threads Build api_intel_bb with Autotools too tests/kms_atomic_transition: Add explicit curly brackets tests/i915_pm_lpsp: Fix compilation warning tests: Add feature_discovery MAINTAINERS: Change Arek's email address tests/feature_discovery: Fix things spotted by GitLab's CI meson: Drop ':' in
Re: [Intel-gfx] [PATCH i-g-t] gem_watchdog: Fix autotools build
On Thu, Apr 08, 2021 at 10:13:16PM +0200, Daniel Vetter wrote: > On Thu, Apr 01, 2021 at 03:03:49PM +0300, Petri Latvala wrote: > > On Thu, Apr 01, 2021 at 12:43:16PM +0100, Tvrtko Ursulin wrote: > > > From: Tvrtko Ursulin > > > > > > Correcting a brain malfunction while typing in Makefile.sources. > > > > > > Signed-off-by: Tvrtko Ursulin > > > > Reviewed-by: Petri Latvala > > Isn't autotools now going away with Arek's series? Yes. But this breakage happened before autotools removal landed. -- Petri Latvala > -Daniel > > > > > > > > --- > > > tests/Makefile.sources | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/tests/Makefile.sources b/tests/Makefile.sources > > > index e992285fedc5..194df8e27dd0 100644 > > > --- a/tests/Makefile.sources > > > +++ b/tests/Makefile.sources > > > @@ -464,7 +464,7 @@ TESTS_progs += gem_wait > > > gem_wait_SOURCES = i915/gem_wait.c > > > > > > TESTS_progs += gem_watchdog > > > -gem_exec_watchdog_SOURCES = i915/gem_watchdog.c > > > +gem_watchdog_SOURCES = i915/gem_watchdog.c > > > > > > TESTS_progs += gem_workarounds > > > gem_workarounds_SOURCES = i915/gem_workarounds.c > > > -- > > > 2.27.0 > > > > > > ___ > > > 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 > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] gem_watchdog: Fix autotools build
On Thu, Apr 01, 2021 at 12:43:16PM +0100, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin > > Correcting a brain malfunction while typing in Makefile.sources. > > Signed-off-by: Tvrtko Ursulin Reviewed-by: Petri Latvala > --- > tests/Makefile.sources | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tests/Makefile.sources b/tests/Makefile.sources > index e992285fedc5..194df8e27dd0 100644 > --- a/tests/Makefile.sources > +++ b/tests/Makefile.sources > @@ -464,7 +464,7 @@ TESTS_progs += gem_wait > gem_wait_SOURCES = i915/gem_wait.c > > TESTS_progs += gem_watchdog > -gem_exec_watchdog_SOURCES = i915/gem_watchdog.c > +gem_watchdog_SOURCES = i915/gem_watchdog.c > > TESTS_progs += gem_workarounds > gem_workarounds_SOURCES = i915/gem_workarounds.c > -- > 2.27.0 > > ___ > 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
Re: [Intel-gfx] [PATCH 2/3] drm/i915/gem: Drop relocation support on all new hardware (v5)
On Mon, Mar 15, 2021 at 10:29:20PM -0700, Ashutosh Dixit wrote:
> From: Jason Ekstrand
>
> The Vulkan driver in Mesa for Intel hardware never uses relocations if
> it's running on a version of i915 that supports at least softpin which
> all versions of i915 supporting Gen12 do. On the OpenGL side, Gen12+ is
> only supported by iris which never uses relocations. The older i965
> driver in Mesa does use relocations but it only supports Intel hardware
> through Gen11 and has been deprecated for all hardware Gen9+. The
> compute driver also never uses relocations. This only leaves the media
> driver which is supposed to be switching to softpin going forward.
> Making softpin a requirement for all future hardware seems reasonable.
>
> There is one piece of hardware enabled by default in i915: RKL which was
> enabled by e22fa6f0a976 which has not yet landed in drm-next so this
> almost but not really a userspace API change for RKL. If it becomes a
> problem, we can always add !IS_ROCKETLAKE(eb->i915) to the condition.
>
> Rejecting relocations starting with newer Gen12 platforms has the
> benefit that we don't have to bother supporting it on platforms with
> local memory. Given how much CPU touching of memory is required for
> relocations, not having to do so on platforms where not all memory is
> directly CPU-accessible carries significant advantages.
>
> v2 (Jason Ekstrand):
> - Allow TGL-LP platforms as they've already shipped
>
> v3 (Jason Ekstrand):
> - WARN_ON platforms with LMEM support in case the check is wrong
>
> v4 (Jason Ekstrand):
> - Call out Rocket Lake in the commit message
>
> v5 (Jason Ekstrand):
> - Drop the HAS_LMEM check as it's already covered by the version check
>
> Signed-off-by: Jason Ekstrand
> Reviewed-by: Zbigniew Kempczyński
> Reviewed-by: Maarten Lankhorst
> ---
> drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 13 ++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> index 99772f37bff60..f66cff2943baa 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> @@ -1764,7 +1764,8 @@ eb_relocate_vma_slow(struct i915_execbuffer *eb, struct
> eb_vma *ev)
> return err;
> }
>
> -static int check_relocations(const struct drm_i915_gem_exec_object2 *entry)
> +static int check_relocations(const struct i915_execbuffer *eb,
> + const struct drm_i915_gem_exec_object2 *entry)
> {
> const char __user *addr, *end;
> unsigned long size;
> @@ -1774,6 +1775,12 @@ static int check_relocations(const struct
> drm_i915_gem_exec_object2 *entry)
> if (size == 0)
> return 0;
>
> + /* Relocations are disallowed for all platforms after TGL-LP. This
> + * also covers all platforms with local memory.
> + */
> + if (INTEL_GEN(eb->i915) >= 12 && !IS_TIGERLAKE(eb->i915))
> + return -EINVAL;
> +
> if (size > N_RELOC(ULONG_MAX))
> return -EINVAL;
>
> @@ -1807,7 +1814,7 @@ static int eb_copy_relocations(const struct
> i915_execbuffer *eb)
> if (nreloc == 0)
> continue;
>
> - err = check_relocations(>exec[i]);
> + err = check_relocations(eb, >exec[i]);
> if (err)
> goto err;
>
> @@ -1880,7 +1887,7 @@ static int eb_prefault_relocations(const struct
> i915_execbuffer *eb)
> for (i = 0; i < count; i++) {
> int err;
>
> - err = check_relocations(>exec[i]);
> + err = check_relocations(eb, >exec[i]);
> if (err)
> return err;
> }
Disclaimer: I don't know deeply how any of this works.
check_relocations() is only called if eb_relocate_parse goes on its
slowpath, fast path doesn't check for gen. Should it, or am I
misunderstanding something?
--
Petri Latvala
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/hdcp: Disable the QSES check for HDCP 1.4 over MST (rev2)
On Wed, Jan 27, 2021 at 09:13:36PM +0200, Vudum, Lakshminarayana wrote: > I am not totally sure why shard run is not triggered here > https://patchwork.freedesktop.org/series/8/#rev2 > @Latvala, Petri any help here? The results were there but reporting it failed. Re-reported it and it's now on patchwork. -- Petri Latvala ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t v2] runner: Don't kill a test on taint if watching timeouts
On Thu, Jan 07, 2021 at 09:49:23AM +, Chris Wilson wrote: > Quoting Petri Latvala (2021-01-07 09:40:02) > > On Wed, Jan 06, 2021 at 09:41:37AM +, Chris Wilson wrote: > > > Quoting Janusz Krzysztofik (2020-12-04 19:50:07) > > > > We may still be interested in results of a test even if it has tainted > > > > the kernel. On the other hand, we need to kill the test on taint if no > > > > other means of killing it on a jam is active. > > > > > > > > If abort on both kernel taint or a timeout is requested, decrease all > > > > potential timeouts significantly while the taint is detected instead of > > > > aborting immediately. However, report the taint as the reason of the > > > > abort if a timeout decreased by the taint expires. > > > > > > This has the nasty side effect of not stopping the test run after a > > > kernel taint. Instead the next test inherits the tainted condition from > > > the previous test and usually ends up being declared incomplete. > > > > > > False positives are frustrating. > > > -Chris > > > > > > Do you have a link to a test run where this happened? This patch > > didn't change the between-tests abort checks. > > The taint is from the warnings in the penultimate test: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9550/shard-skl7/igt_runner20.txt Ah, I see. https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9550/shard-skl7/dmesg20.txt This is the tainting WARN I presume: <4>[ 917.575173] Memory manager not clean during takedown. <4>[ 917.575272] WARNING: CPU: 2 PID: 7 at drivers/gpu/drm/drm_mm.c:999 drm_mm_takedown+0x51/0x100 That happens after @gem, before @evict. In other words, this is all in the same exec() of i915_selftest --run-subtest live. Incorrect _dynamic_ subtest gets blamed. Getting the killing right here is a bit tricky, possibly doable. Or rather, getting the blame right is doable, killing will be inherently racy... -- Petri Latvala ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t v2] runner: Don't kill a test on taint if watching timeouts
On Wed, Jan 06, 2021 at 09:41:37AM +, Chris Wilson wrote: > Quoting Janusz Krzysztofik (2020-12-04 19:50:07) > > We may still be interested in results of a test even if it has tainted > > the kernel. On the other hand, we need to kill the test on taint if no > > other means of killing it on a jam is active. > > > > If abort on both kernel taint or a timeout is requested, decrease all > > potential timeouts significantly while the taint is detected instead of > > aborting immediately. However, report the taint as the reason of the > > abort if a timeout decreased by the taint expires. > > This has the nasty side effect of not stopping the test run after a > kernel taint. Instead the next test inherits the tainted condition from > the previous test and usually ends up being declared incomplete. > > False positives are frustrating. > -Chris Do you have a link to a test run where this happened? This patch didn't change the between-tests abort checks. -- Petri Latvala ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✗ Fi.CI.BUILD: failure for series starting with [01/16] drm/i915/display: move needs_modeset to an inline in header
On Mon, Dec 21, 2020 at 11:03:42AM +0200, Jani Nikula wrote: > On Mon, 21 Dec 2020, Patchwork wrote: > > == Series Details == > > > > Series: series starting with [01/16] drm/i915/display: move needs_modeset > > to an inline in header > > URL : https://patchwork.freedesktop.org/series/85129/ > > State : failure > > > > == Summary == > > > > Applying: drm/i915/display: move needs_modeset to an inline in header > > Applying: drm/i915/display: move to_intel_frontbuffer to header > > Applying: drm/i915/display: fix misused comma > > Applying: drm/i915: refactor cursor code out of i915_display.c > > Applying: drm/i915: refactor i915 plane code into separate file. > > Applying: drm/i915: refactor some crtc code out of intel display. (v2) > > error: sha1 information is lacking or useless > > (drivers/gpu/drm/i915/Makefile). > > error: could not build fake ancestor > > hint: Use 'git am --show-current-patch=diff' to see the failed patch > > Patch failed at 0006 drm/i915: refactor some crtc code out of intel > > display. (v2) > > When you have resolved this problem, run "git am --continue". > > If you prefer to skip this patch, run "git am --skip" instead. > > To restore the original branch and stop patching, run "git am --abort". > > I wonder what gives here. The same thing has been happening for several > versions of the series, including mine. I would've applied the early > patches already if I'd gotten some test results. What gives is the part of the patch that contains -#define INTEL_CRTC_FUNCS \ -.gamma_set = drm_atomic_helper_legacy_gamma_set, \ which doesn't apply anymore after commit 6ca2ab8086af8434a4c0990882321a345c3cc2c6 Author: Tomi Valkeinen Date: Fri Dec 11 13:42:36 2020 +0200 drm: automatic legacy gamma support -- Petri Latvala ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 2/2] i915/query: Directly check query results against GETPARAM
On Mon, Dec 07, 2020 at 04:11:50PM +, Chris Wilson wrote: > Simplify the cross-check by asserting that the existence of an engine in > the list matches the existence of the engine as reported by GETPARAM. > By using the comparison, we check both directions at once. > > Signed-off-by: Chris Wilson > Cc: Petri Latvala For the series, Reviewed-by: Petri Latvala Thanks! ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t v2] runner: Don't kill a test on taint if watching timeouts
On Fri, Dec 04, 2020 at 08:50:07PM +0100, Janusz Krzysztofik wrote:
> We may still be interested in results of a test even if it has tainted
> the kernel. On the other hand, we need to kill the test on taint if no
> other means of killing it on a jam is active.
>
> If abort on both kernel taint or a timeout is requested, decrease all
> potential timeouts significantly while the taint is detected instead of
> aborting immediately. However, report the taint as the reason of the
> abort if a timeout decreased by the taint expires.
>
> v2: Fix missing show_kernel_task_state() lost on rebase conflict
> resolution (Chris - thanks!)
>
> Signed-off-by: Janusz Krzysztofik
The effects of this is that we sometimes now get more logs from a test
at the cost of it not directly showing up as an incomplete. We would
still get the igt@runner@aborted result for it so overall we still
catch tainting cases.
Chris's comments have been clarified off-list not to mean directly
opposing this patch, so
Reviewed-by: Petri Latvala
> ---
> runner/executor.c | 26 --
> 1 file changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/runner/executor.c b/runner/executor.c
> index 1688ae41d..faf272d85 100644
> --- a/runner/executor.c
> +++ b/runner/executor.c
> @@ -726,6 +726,8 @@ static const char *need_to_timeout(struct settings
> *settings,
> double time_since_kill,
> size_t disk_usage)
> {
> + int decrease = 1;
> +
> if (killed) {
> /*
>* Timeout after being killed is a hardcoded amount
> @@ -753,20 +755,32 @@ static const char *need_to_timeout(struct settings
> *settings,
> }
>
> /*
> - * If we're configured to care about taints, kill the
> - * test if there's a taint.
> + * If we're configured to care about taints,
> + * decrease timeouts in use if there's a taint,
> + * or kill the test if no timeouts have been requested.
>*/
> if (settings->abort_mask & ABORT_TAINT &&
> - is_tainted(taints))
> - return "Killing the test because the kernel is tainted.\n";
> + is_tainted(taints)) {
> + /* list of timeouts that may postpone immediate kill on taint */
> + if (settings->per_test_timeout || settings->inactivity_timeout)
> + decrease = 10;
> + else
> + return "Killing the test because the kernel is
> tainted.\n";
> + }
>
> if (settings->per_test_timeout != 0 &&
> - time_since_subtest > settings->per_test_timeout)
> + time_since_subtest > settings->per_test_timeout / decrease) {
> + if (decrease > 1)
> + return "Killing the test because the kernel is
> tainted.\n";
> return show_kernel_task_state("Per-test timeout exceeded.
> Killing the current test with SIGQUIT.\n");
> + }
>
> if (settings->inactivity_timeout != 0 &&
> - time_since_activity > settings->inactivity_timeout)
> + time_since_activity > settings->inactivity_timeout / decrease ) {
> + if (decrease > 1)
> + return "Killing the test because the kernel is
> tainted.\n";
> return show_kernel_task_state("Inactivity timeout exceeded.
> Killing the current test with SIGQUIT.\n");
> + }
>
> if (disk_usage_limit_exceeded(settings, disk_usage))
> return "Disk usage limit exceeded.\n";
> --
> 2.21.1
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Do not call hsw_set_frame_start_delay for dsi
On Wed, Nov 18, 2020 at 11:13:31PM -0800, Manasi Navare wrote:
> This should fix the boot oops for dsi
>
> Fixes: 4e3cdb4535e7 ("drm/i915/dp: Master/Slave enable/disable sequence for
> bigjoiner")
> Signed-off-by: Manasi Navare
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 5c07c74d4397..739be96e998d 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -7211,7 +7211,7 @@ static void hsw_crtc_enable(struct intel_atomic_state
> *state,
> if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv))
> bdw_set_pipemisc(new_crtc_state);
>
> - if (!new_crtc_state->bigjoiner_slave ||
> !transcoder_is_dsi(cpu_transcoder)) {
> + if (!new_crtc_state->bigjoiner_slave &&
> !transcoder_is_dsi(cpu_transcoder)) {
> if (!transcoder_is_dsi(cpu_transcoder))
> intel_set_transcoder_timings(new_crtc_state);
>
> @@ -7224,7 +7224,7 @@ static void hsw_crtc_enable(struct intel_atomic_state
> *state,
> intel_cpu_transcoder_set_m_n(new_crtc_state,
>_crtc_state->fdi_m_n,
> NULL);
>
> - hsw_set_frame_start_delay(new_crtc_state);
> + hsw_set_frame_start_delay(new_crtc_state);
Indentation for this is wrong now.
--
Petri Latvala
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/guc: Update to GuC v49
On Wed, Sep 16, 2020 at 06:22:45PM -0700, John Harrison wrote: > Hello, > > The failures below all appear to be because the new GuC firmware was not > found on the test system. > > My understanding is that all we need to do to get the CI system to update > with new firmwares is to push the firmware to a branch on the FDO > drm-firmware repo and then send a pull request to this mailing list. That > was done yesterday. That pull request used an ssh:// url though. Can you send it again with a git:// url? I suppose that's a plausible reason why I don't see the binaries in CI's deploy dir. -- Petri Latvala ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t v6 23/24] tests/core_hotunplug: Un-blocklist *bind* subtests
On Fri, Sep 11, 2020 at 02:00:11PM +0200, Janusz Krzysztofik wrote: > Hi Petri, > > On Fri, 2020-09-11 at 14:51 +0300, Petri Latvala wrote: > > On Fri, Sep 11, 2020 at 12:30:38PM +0200, Janusz Krzysztofik wrote: > > > Subject: [PATCH i-g-t v6 23/24] tests/core_hotunplug: Un-blocklist *bind* > > > subtests > > > > Change to > > > > intel-ci: Un-blocklist *bind* subtests of core_hotunplug > > > > OK, and I guess the same applies to "tests/core_hotunplug: Add unbind- > rebind subtest to BAT scope" (if accepted). Speaking of accepted, now that the results are in, for the two intel-ci patches: Acked-by: Petri Latvala ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t v6 21/24] tests/core_hotunplug: HSW/BDW audio issue workaround
On Fri, Sep 11, 2020 at 03:15:43PM +0200, Janusz Krzysztofik wrote:
> Hi Petri,
>
> On Fri, 2020-09-11 at 15:22 +0300, Petri Latvala wrote:
> > On Fri, Sep 11, 2020 at 12:30:36PM +0200, Janusz Krzysztofik wrote:
> > > Unbinding the i915 driver on some Haswell and Broadwell platforms with
> > > Azalia audio results in a kernel WARNING on "i915 raw-wakerefs=1
> > > wakelocks=1 on cleanup". The issue can be worked around by manually
> > > enabling runtime power management for the conflicting audio adapter.
> > > Use that method but also display a warning to preserve visibility of
> > > the issue. Also tag the workaround with a FIXME comment.
> > >
> > > v2: Extend the scope of the workaround over Broadwell
> > >
> > > Signed-off-by: Janusz Krzysztofik
> > > ---
> > > tests/core_hotunplug.c | 15 +++
> > > 1 file changed, 15 insertions(+)
> > >
> > > diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
> > > index ac106d964..3e2a76ddb 100644
> > > --- a/tests/core_hotunplug.c
> > > +++ b/tests/core_hotunplug.c
> > > @@ -484,8 +484,23 @@ igt_main
> > > igt_skip_on_f(fd_drm < 0, "No known DRM device found\n");
> > >
> > > if (is_i915_device(fd_drm)) {
> > > + uint32_t devid = intel_get_drm_devid(fd_drm);
> > > +
> > > gem_quiescent_gpu(fd_drm);
> > > igt_require_gem(fd_drm);
> > > +
> > > + /**
> > > + * FIXME: Unbinding the i915 driver on some Haswell
> > > + * platforms with Azalia audio results in a kernel WARN
> > > + * on "i915 raw-wakerefs=1 wakelocks=1 on cleanup". The
> > > + * below CI friendly user level workaround prevents the
> > > + * warning from appearing. Drop this hack as soon as
> > > + * this is fixed in the kernel.
> > > + */
> > > + if (igt_warn_on_f(IS_HASWELL(devid) ||
> > > + IS_BROADWELL(devid),
> > > + "Manually enabling audio PM to work around a kernel
> > > WARN\n"))
> > > + igt_pm_enable_audio_runtime_pm();
> >
> > What happens without this? Is it just a kernel warning, or does the
> > operation also fail?
>
> runner: This test was killed due to a kernel taint (0x200).
> (https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4901/shard-hsw4/igt@core_hotunp...@unbind-rebind.html)
>
> That happens before the test completes so no results of the operation
> are reported.
Ah, right. I had a brainfart. Indeed this igt_warn is better.
Reviewed-by: Petri Latvala
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t v6 21/24] tests/core_hotunplug: HSW/BDW audio issue workaround
On Fri, Sep 11, 2020 at 12:30:36PM +0200, Janusz Krzysztofik wrote:
> Unbinding the i915 driver on some Haswell and Broadwell platforms with
> Azalia audio results in a kernel WARNING on "i915 raw-wakerefs=1
> wakelocks=1 on cleanup". The issue can be worked around by manually
> enabling runtime power management for the conflicting audio adapter.
> Use that method but also display a warning to preserve visibility of
> the issue. Also tag the workaround with a FIXME comment.
>
> v2: Extend the scope of the workaround over Broadwell
>
> Signed-off-by: Janusz Krzysztofik
> ---
> tests/core_hotunplug.c | 15 +++
> 1 file changed, 15 insertions(+)
>
> diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
> index ac106d964..3e2a76ddb 100644
> --- a/tests/core_hotunplug.c
> +++ b/tests/core_hotunplug.c
> @@ -484,8 +484,23 @@ igt_main
> igt_skip_on_f(fd_drm < 0, "No known DRM device found\n");
>
> if (is_i915_device(fd_drm)) {
> + uint32_t devid = intel_get_drm_devid(fd_drm);
> +
> gem_quiescent_gpu(fd_drm);
> igt_require_gem(fd_drm);
> +
> + /**
> + * FIXME: Unbinding the i915 driver on some Haswell
> + * platforms with Azalia audio results in a kernel WARN
> + * on "i915 raw-wakerefs=1 wakelocks=1 on cleanup". The
> + * below CI friendly user level workaround prevents the
> + * warning from appearing. Drop this hack as soon as
> + * this is fixed in the kernel.
> + */
> + if (igt_warn_on_f(IS_HASWELL(devid) ||
> + IS_BROADWELL(devid),
> + "Manually enabling audio PM to work around a kernel
> WARN\n"))
> + igt_pm_enable_audio_runtime_pm();
What happens without this? Is it just a kernel warning, or does the
operation also fail?
If the former, what does this gain? All it does is we lose the
capability to track whether the kernel still has that issue, we still
have to filter this warning in cibuglog.
--
Petri Latvala
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t v6 24/24] tests/core_hotunplug: Add unbind-rebind subtest to BAT scope
On Fri, Sep 11, 2020 at 12:30:39PM +0200, Janusz Krzysztofik wrote: > Subject: [PATCH i-g-t v6 24/24] tests/core_hotunplug: Add unbind-rebind > subtest to BAT scope Same here, prefix with intel-ci -- Petri Latvala ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t v6 23/24] tests/core_hotunplug: Un-blocklist *bind* subtests
On Fri, Sep 11, 2020 at 12:30:38PM +0200, Janusz Krzysztofik wrote: > Subject: [PATCH i-g-t v6 23/24] tests/core_hotunplug: Un-blocklist *bind* > subtests Change to intel-ci: Un-blocklist *bind* subtests of core_hotunplug -- Petri Latvala ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 1/1] intel-gpu-top: Support for client stats
On Fri, Sep 04, 2020 at 02:06:07PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin
>
> Adds support for per-client engine busyness stats i915 exports in sysfs
> and produces output like the below:
>
> ==
> intel-gpu-top - 935/ 935 MHz;0% RC6; 14.73 Watts; 1097 irqs/s
>
> IMC reads: 1401 MiB/s
> IMC writes:4 MiB/s
>
> ENGINE BUSY MI_SEMA MI_WAIT
> Render/3D/0 63.73% |███ | 3% 0%
>Blitter/09.53% |██▊ | 6% 0%
> Video/0 39.32% |███▊ | 16% 0%
> Video/1 15.62% |▋ | 0% 0%
> VideoEnhance/00.00% | | 0% 0%
>
> PIDNAME RCS BCS VCS VECS
> 4084gem_wsim |█▌ ||█ || || |
> 4086gem_wsim |█▌ || ||███|| |
> ==
>
> Apart from the existing physical engine utilization it now also shows
> utilization per client and per engine class.
>
> v2:
> * Version to match removal of global enable_stats toggle.
> * Plus various fixes.
>
> v3:
> * Support brief backward jumps in client stats.
>
> Signed-off-by: Tvrtko Ursulin
> ---
> tools/intel_gpu_top.c | 539 +-
> 1 file changed, 528 insertions(+), 11 deletions(-)
>
> diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
> index cae01c25b920..9eac569e75de 100644
> --- a/tools/intel_gpu_top.c
> +++ b/tools/intel_gpu_top.c
> @@ -679,23 +679,347 @@ static void pmu_sample(struct engines *engines)
> }
> }
>
> +enum client_status {
> + FREE = 0, /* mbz */
> + ALIVE,
> + PROBE
> +};
> +
> +struct clients;
> +
> +struct client {
> + struct clients *clients;
> +
> + enum client_status status;
> + unsigned int id;
> + unsigned int pid;
> + char name[128];
> + unsigned int samples;
> + unsigned long total;
> + struct engines *engines;
> + unsigned long *val;
> + uint64_t *last;
> +};
> +
> +struct engine_class {
> + unsigned int class;
> + const char *name;
> + unsigned int num_engines;
> +};
> +
> +struct clients {
> + unsigned int num_classes;
> + struct engine_class *class;
> +
> + unsigned int num_clients;
> + struct client *client;
> +};
> +
> +#define for_each_client(clients, c, tmp) \
> + for ((tmp) = (clients)->num_clients, c = (clients)->client; \
> + (tmp > 0); (tmp)--, (c)++)
> +
> +static struct clients *init_clients(void)
> +{
> + struct clients *clients = malloc(sizeof(*clients));
> +
> + return memset(clients, 0, sizeof(*clients));
> +}
> +
> +#define SYSFS_CLIENTS "/sys/class/drm/card0/clients"
Now that intel_gpu_top supports device selection, this path works
every time only 60% of the time, right?
--
Petri Latvala
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t v9 0/4] Add support for testing writeback connectors
On Fri, Sep 04, 2020 at 12:09:18PM +0100, Liviu Dudau wrote: > On Sun, Aug 30, 2020 at 01:44:06PM -0400, Rodrigo Siqueira wrote: > > Hi, > > Hi, > > Can this series be merged? Thanks for the poke. It's merged now. > Neither me nor Brian managed to get accepted > in the i-g-t committers list, so I cannot push it. Let's fix that. Please apply in gitlab. -- Petri Latvala ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✗ Fi.CI.IGT: failure for tests/core_hotunplug: Fixes and enhancements (rev4)
On Mon, Aug 24, 2020 at 10:42:10AM +0200, Janusz Krzysztofik wrote:
> On Fri, 2020-08-21 at 17:40 +, Patchwork wrote:
> > Patch Details
> > Series: tests/core_hotunplug: Fixes and enhancements (rev4)
> > URL:https://patchwork.freedesktop.org/series/79671/
> > State: failure
> > Details:https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4901/index.html
> > CI Bug Log - changes from CI_DRM_8913_full -> IGTPW_4901_full
> > Summary
> > FAILURE
> >
> > Serious unknown changes coming with IGTPW_4901_full absolutely need to be
> > verified manually.
> >
> > If you think the reported changes have nothing to do with the changes
> > introduced in IGTPW_4901_full, 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_4901/index.html
> >
> > Possible new issues
> > Here are the unknown changes that may have been introduced in
> > IGTPW_4901_full:
> >
> > IGT changes
> > Possible regressions
> > {igt@core_hotunplug@hotrebind-lateclose} (NEW):
> >
> > shard-snb: NOTRUN -> FAIL
> >
> > shard-glk: NOTRUN -> FAIL
> >
> > shard-kbl: NOTRUN -> FAIL
>
> This is an existing but formerly not reported GPU hang driver issue
> exhibited by the test, not a regression. The issue needs to be fixed
> in the driver for the test to succeed. As one can see from CI reports,
> the test succesfuly recovers from that condition and subsequent tests
> are not affected.
>
> >
> > {igt@core_hotunplug@hotunbind-lateclose} (NEW):
> >
> > shard-hsw: NOTRUN -> INCOMPLETE +3 similar issues
>
> This is a known driver issue, already reported by igt@device
> _reset@unbind-reset-rebind. This test shows clearly that the issue has
> nothing to do with device reset, only with driver unbind-rebind cycle.
> The driver needs to be fixed for the kernel warning not be triggered
> and the tests succeed.
Is there an ETA for the driver fix?
>
> I think the *bind* subtests in their current shape are perfectly ready
> for inclusion in CI runs.
Agreed otherwise, except for the known incomplete. Introducing new
incompletes without a fix in sight is real bad.
--
Petri Latvala
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 1/4] i915/perf: 32bit printf cleanup
On Fri, Aug 21, 2020 at 12:45:23AM +0300, Lionel Landwerlin wrote:
> On 20/08/2020 20:26, Chris Wilson wrote:
> > Use PRI[du]64 as necessary for 32bit builds.
> >
> > Signed-off-by: Chris Wilson
>
> Reviewed-by: Lionel Landwerlin
>
Was this for just 1/4 or the whole series?
This one is for the whole series:
Reviewed-by: Petri Latvala
>
> Thanks!
>
> -Lionel
>
> > ---
> > tests/i915/perf.c| 8
> > tools/i915-perf/i915_perf_recorder.c | 2 +-
> > 2 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/tests/i915/perf.c b/tests/i915/perf.c
> > index 92edc9f1f..a894fd382 100644
> > --- a/tests/i915/perf.c
> > +++ b/tests/i915/perf.c
> > @@ -2077,7 +2077,7 @@ test_blocking(uint64_t requested_oa_period, bool
> > set_kernel_hrtimer, uint64_t ke
> > user_ns = (end_times.tms_utime - start_times.tms_utime) * tick_ns;
> > kernel_ns = (end_times.tms_stime - start_times.tms_stime) * tick_ns;
> > - igt_debug("%d blocking reads during test with %lu Hz OA sampling
> > (expect no more than %d)\n",
> > + igt_debug("%d blocking reads during test with %"PRIu64" Hz OA sampling
> > (expect no more than %d)\n",
> > n, NSEC_PER_SEC / oa_period, max_iterations);
> > igt_debug("%d extra iterations seen, not related to periodic sampling
> > (e.g. context switches)\n",
> > n_extra_iterations);
> > @@ -2265,7 +2265,7 @@ test_polling(uint64_t requested_oa_period, bool
> > set_kernel_hrtimer, uint64_t ker
> > user_ns = (end_times.tms_utime - start_times.tms_utime) * tick_ns;
> > kernel_ns = (end_times.tms_stime - start_times.tms_stime) * tick_ns;
> > - igt_debug("%d non-blocking reads during test with %lu Hz OA sampling
> > (expect no more than %d)\n",
> > + igt_debug("%d non-blocking reads during test with %"PRIu64" Hz OA
> > sampling (expect no more than %d)\n",
> > n, NSEC_PER_SEC / oa_period, max_iterations);
> > igt_debug("%d extra iterations seen, not related to periodic sampling
> > (e.g. context switches)\n",
> > n_extra_iterations);
> > @@ -2357,7 +2357,7 @@ num_valid_reports_captured(struct
> > drm_i915_perf_open_param *param,
> > int64_t start, end;
> > int num_reports = 0;
> > - igt_debug("Expected duration = %lu\n", *duration_ns);
> > + igt_debug("Expected duration = %"PRId64"\n", *duration_ns);
> > stream_fd = __perf_open(drm_fd, param, true);
> > @@ -2389,7 +2389,7 @@ num_valid_reports_captured(struct
> > drm_i915_perf_open_param *param,
> > *duration_ns = end - start;
> > - igt_debug("Actual duration = %lu\n", *duration_ns);
> > + igt_debug("Actual duration = %"PRIu64"\n", *duration_ns);
> > return num_reports;
> > }
> > diff --git a/tools/i915-perf/i915_perf_recorder.c
> > b/tools/i915-perf/i915_perf_recorder.c
> > index 7671f39b4..adc41c29f 100644
> > --- a/tools/i915-perf/i915_perf_recorder.c
> > +++ b/tools/i915-perf/i915_perf_recorder.c
> > @@ -1001,7 +1001,7 @@ main(int argc, char *argv[])
> > }
> > ctx.oa_exponent = oa_exponent_for_period(ctx.timestamp_frequency,
> > perf_period);
> > - fprintf(stdout, "Opening perf stream with metric_id=%lu
> > oa_exponent=%u\n",
> > + fprintf(stdout, "Opening perf stream with metric_id=%"PRIu64"
> > oa_exponent=%u\n",
> > ctx.metric_set->perf_oa_metrics_set, ctx.oa_exponent);
> > ctx.perf_fd = perf_open();
>
>
> ___
> igt-dev mailing list
> igt-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] lib: Use unsigned gen for forward compatible tests
On Mon, Aug 10, 2020 at 10:09:46AM +0200, Zbigniew Kempczyński wrote: > On Thu, Aug 06, 2020 at 03:45:29PM +0100, Chris Wilson wrote: > > Unknown, so future, gen are marked as -1 which we want to treat as -1u > > so that always pass >= gen checks. > > Do we really want to enable the tests when platform is not fully > enabled in IGT? What does "fully enabled" mean? If the test is checking for just "gen > x", the test should work already. If the test is also checking for "gen < y" then we get a spurious failure, but either way CI is going to tell you that something is not passing. Without this it will be a skip, along with skipping in the case that should just work already without actual test changes. -- Petri Latvala ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 1/2] lib/i915: Report scheduler caps for timeslicing
On Wed, May 13, 2020 at 06:02:23PM +0100, Chris Wilson wrote: > diff --git a/include/drm-uapi/i915_drm.h b/include/drm-uapi/i915_drm.h In a separate commit please, with the commit message stating which kernel git sha it's from. -- Petri Latvala ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] i915/gem_ringfill: Do a basic pass over all engines simultaneously
On Mon, May 11, 2020 at 10:53:58AM +0100, Chris Wilson wrote:
> Quoting Petri Latvala (2020-05-11 10:49:10)
> > On Mon, May 11, 2020 at 10:39:24AM +0100, Chris Wilson wrote:
> > > Change the basic pre-mergetest to do a single pass over all engines
> > > simultaneously. This should take no longer than checking a single
> > > engine, while providing just the right amount of stress regardless of
> > > machine size.
> > >
> > > v2: Move around the quiescence and requires to avoid calling them from
> > > the children.
> > >
> > > Signed-off-by: Chris Wilson
> > > Cc: Petri Latvala
> > > ---
> > > tests/i915/gem_ringfill.c | 31 ---
> > > tests/intel-ci/fast-feedback.testlist | 2 +-
> > > 2 files changed, 24 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/tests/i915/gem_ringfill.c b/tests/i915/gem_ringfill.c
> > > index a2157bd6f..05b997ed4 100644
> > > --- a/tests/i915/gem_ringfill.c
> > > +++ b/tests/i915/gem_ringfill.c
> > > @@ -178,13 +178,11 @@ static void run_test(int fd, unsigned ring,
> > > unsigned flags, unsigned timeout)
> > > struct drm_i915_gem_execbuffer2 execbuf;
> > > igt_hang_t hang;
> > >
> > > - gem_require_ring(fd, ring);
> > > - igt_require(gem_can_store_dword(fd, ring));
> > > -
> > > - if (flags & (SUSPEND | HIBERNATE))
> > > + if (flags & (SUSPEND | HIBERNATE)) {
> > > run_test(fd, ring, 0, 0);
> > > + gem_quiescent_gpu(fd);
> > > + }
> > >
> > > - gem_quiescent_gpu(fd);
> > > igt_require(setup_execbuf(fd, , obj, reloc, ring) == 0);
> >
> > What about this one?
>
> If that fails, I'll accept the punishment of having to debug a
> mysterious segfault. That's a require for v3.10.
It will just assert() out, saying you can't skip in a fork. See the v1
BAT results.
If you add a comment stating that it shouldn't usually fire and having
it in a function sometimes called in a child process is intentional,
Reviewed-by: Petri Latvala
--
Petri Latvala
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] i915/gem_ringfill: Do a basic pass over all engines simultaneously
On Mon, May 11, 2020 at 10:39:24AM +0100, Chris Wilson wrote:
> Change the basic pre-mergetest to do a single pass over all engines
> simultaneously. This should take no longer than checking a single
> engine, while providing just the right amount of stress regardless of
> machine size.
>
> v2: Move around the quiescence and requires to avoid calling them from
> the children.
>
> Signed-off-by: Chris Wilson
> Cc: Petri Latvala
> ---
> tests/i915/gem_ringfill.c | 31 ---
> tests/intel-ci/fast-feedback.testlist | 2 +-
> 2 files changed, 24 insertions(+), 9 deletions(-)
>
> diff --git a/tests/i915/gem_ringfill.c b/tests/i915/gem_ringfill.c
> index a2157bd6f..05b997ed4 100644
> --- a/tests/i915/gem_ringfill.c
> +++ b/tests/i915/gem_ringfill.c
> @@ -178,13 +178,11 @@ static void run_test(int fd, unsigned ring, unsigned
> flags, unsigned timeout)
> struct drm_i915_gem_execbuffer2 execbuf;
> igt_hang_t hang;
>
> - gem_require_ring(fd, ring);
> - igt_require(gem_can_store_dword(fd, ring));
> -
> - if (flags & (SUSPEND | HIBERNATE))
> + if (flags & (SUSPEND | HIBERNATE)) {
> run_test(fd, ring, 0, 0);
> + gem_quiescent_gpu(fd);
> + }
>
> - gem_quiescent_gpu(fd);
> igt_require(setup_execbuf(fd, , obj, reloc, ring) == 0);
What about this one?
--
Petri Latvala
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] i915/gem_ringfill: Do a basic pass over all engines simultaneously
On Mon, May 11, 2020 at 09:21:41AM +0100, Chris Wilson wrote:
> Change the basic pre-mergetest to do a single pass over all engines
> simultaneously. This should take no longer than checking a single
> engine, while providing just the right amount of stress regardless of
> machine size.
>
> Signed-off-by: Chris Wilson
> ---
> tests/i915/gem_ringfill.c | 10 ++
> tests/intel-ci/fast-feedback.testlist | 2 +-
> 2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/tests/i915/gem_ringfill.c b/tests/i915/gem_ringfill.c
> index a2157bd6f..a90758e08 100644
> --- a/tests/i915/gem_ringfill.c
> +++ b/tests/i915/gem_ringfill.c
> @@ -292,6 +292,16 @@ igt_main
> }
> }
>
> + igt_subtest("basic-all") {
> + const struct intel_execution_engine2 *e;
> +
> + __for_each_physical_engine(fd, e)
> + igt_fork(child, 1)
> + run_test(fd, e->flags, 0, 1);
Will it happen simultaneously though without synchronization?
(Obvious quip about executing too fast)
run_test() calls igt_require in a few places, and skips in child
processes are problematic.
--
Petri Latvala
> +
> + igt_waitchildren();
> + }
> +
> igt_fixture
> close(fd);
> }
> diff --git a/tests/intel-ci/fast-feedback.testlist
> b/tests/intel-ci/fast-feedback.testlist
> index 2ccad4386..e2ed0a1d6 100644
> --- a/tests/intel-ci/fast-feedback.testlist
> +++ b/tests/intel-ci/fast-feedback.testlist
> @@ -35,7 +35,7 @@ igt@gem_mmap@basic
> igt@gem_mmap_gtt@basic
> igt@gem_render_linear_blits@basic
> igt@gem_render_tiled_blits@basic
> -igt@gem_ringfill@basic-default-forked
> +igt@gem_ringfill@basic-all
> igt@gem_sync@basic-all
> igt@gem_sync@basic-each
> igt@gem_tiled_blits@basic
> --
> 2.26.2
>
> ___
> igt-dev mailing list
> igt-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t v15] tests: Add a test for device hot unplug
On Wed, Apr 15, 2020 at 03:15:15PM +0200, Janusz Krzysztofik wrote: > From: Janusz Krzysztofik > > There is a test which verifies unloading of i915 driver module but no > test exists that checks how a driver behaves when it gets unbound from > a device or when the device gets unplugged. Implement such test using > sysfs interface. > > Two minimalistic subtests - "unbind-rebind" and "unplug-rescan" - > perform the named operations on a DRM device which is believed to be > not in use. Another pair of subtests named "hotunbind-lateclose" and > hotunplug-lateclose" do the same on a DRM device while keeping its file > descriptor open and close it thereafter. > > v2: Run a subprocess with dummy_load instead of external command > (Antonio). > v3: Run dummy_load from the test process directly (Antonio). > v4: Run dummy_load from inside subtests (Antonio). > v5: Try to restore the device to a working state after each subtest > (Petri, Daniel). > v6: Run workload inside an igt helper subprocess so resources consumed > by the workload are cleaned up automatically on workload subprocess > crash, without affecting test results, > - move the igt helper with workload back from subtests to initial > fixture so workload crash also does not affect test results, > - other cleanups suggested by Katarzyna and Chris. > v7: No changes. > v8: Move workload functions back from fixture to subtests, > - register different actions and different workloads in respective > tables and iterate over those tables while enumerating subtests, > - introduce new subtest flavors by simply omitting module unload step, > - instead of simply requesting bus rescan or not, introduce action > specific device recovery helpers, required specifically with those > new subtests not touching the module, > - split workload functions in two parts, one spawning the workload, > the other waiting for its completion, > - for the new subtests not requiring module unload, run workload > functions directly from the test process and use new workload > completion wait functions in place of subprocess completion wait, > - take more control over logging, longjumps and exit codes in > workload subprocesses, > - add some debug messages for easy progress watching, > - move function API descriptions on top of respective typedefs. > v9: All changes after Daniel's comments - thanks! > - flatten the code, don't try to create a midlayer (Daniel), > - provide minimal subtests that even don't keep device open (Daniel), > - don't use driver unbind in more advanced subtests (Daniel), > - provide subtests with different level of resources allocated > during device unplug (Daniel), > - provide subtests which check driver behavior after device hot > unplug (Daniel). > v10 Rename variables and function arguments to something that > indicates they're file descriptors (Daniel), > - introduce a data structure that contains various file descriptors > and a helper function to set them all (Daniel), > - fix strange indentation (Daniel), > - limit scope to first three subtests as the initial set of tests to > merge (Daniel). > v11 Fix typos in some comments, > - use SPDX license identifier, > - include a per-patch changelog in the commit message (Daniel). > v12 We don't use SPDX license identifiers nor GPL-2.0 in IGT (Petri), > - avoid chipset, make sure we reopen the same device (Chris), > - rename subtest "drm_open-hotunplug" to "hotunplug-lateclose", > - add subtest "hotunbind-lateclose" (less affected by IOMMU issues), > - move some redundant code to helpers, > - reorder some helpers, > - reword some messages and comments, > - clean up headers. > v13 Add test / subtest descriptions (patchwork). > v14 Extract redundant device rescan and reopen code to a 'healthcheck' > helper, > - call igt_abort_on_f() on device reopen failure (Petri), > - if any timeout set with igt_set_timeout() inside a subtest expires, > call igt_abort_on_f() from a follow-up igt_fixture (Petri), > - when running on a i915 device, require GEM and call > igt_abort_on_f() if no usable GEM is detected on device reopen. > v15 Add the test to CI blacklist (Martin). > > Signed-off-by: Janusz Krzysztofik > Cc: Antonio Argenziano > Cc: Petri Latvala > Cc: Daniel Vetter > Cc: Katarzyna Dec > Cc: Martin Peres > Acked-by: Chris Wilson > --- > tests/Makefile.sources | 1 + > tests/core_hotunplug.c | 300 +++ > tests/intel-ci/blacklist.txt | 1 + > tests/meson.build|
Re: [Intel-gfx] [PATCH v7 i-g-t 2/4] kms_writeback: Add initial writeback tests
On Wed, Apr 15, 2020 at 12:06:18PM +0200, Maxime Ripard wrote: > On Wed, Apr 15, 2020 at 09:49:46AM +, Simon Ser wrote: > > > > + igt_output_set_prop_value(output, > > > > IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR, (uint64_t)out_fence_ptr); > > > > > > On ARM (32bits), this cast creates a compilation error since the > > > pointer size isn't an uint64_t. > > > > Does casting to uintptr_t before uint64_t fix it? > > It does yep. Casting to uintptr_t alone also works to_user_pointer(out_fence_ptr) -- Petri Latvala ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] i915/perf_pmu: Add the missing igt_dynamic to dynamic rcs* test selection
On Mon, Apr 06, 2020 at 09:53:09AM +0100, Chris Wilson wrote:
> An important ingredient to using igt_subtest_with_dynamic is to include
> an igt_dynamic at some point.
>
> Reported-by: Petri Latvala
> Fixes: 311cb1b360b7 ("i915/perf_pmu: Dynamic active engine tests")
> Signed-off-by: Chris Wilson
> Cc: Petri Latvala
Thanks,
Reviewed-by: Petri Latvala
If someone (tm) is feeling like there's not enough to do, perf_pmu
could use a refactoring with the dynamic subtests to do
igt_subtest_with_dynamic() {
igt_require(system-wide things);
igt_dynamic_f() { igt_require(engine-specific thing); }
}
so that skips happen for a whole subtest at a time for things like gen
checks.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] lib/igt_fb: change comments with fd description
On Mon, Mar 30, 2020 at 06:56:36PM -0300, Melissa Wen wrote: > Generalize description of fd since it is not restricted to i915 driver fd > > Signed-off-by: Melissa Wen Reviewed-by: Petri Latvala Please send future IGT patches to igt-...@lists.freedesktop.org please. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] tests/kms_plane_alpha_blend: Correct typo in the name and comments of a subtest
On Mon, Mar 30, 2020 at 06:55:32PM -0300, Melissa Wen wrote: > Typo found in word transparent. > Correct the word transparant, replacing the last letter -a- with -e- > (transpar-a-nt to transpar-e-nt). > > Signed-off-by: Melissa Wen Reviewed-by: Petri Latvala Martin, test rename, ack when cibuglog side is ready to merge this. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✗ Fi.CI.IGT: failure for tests/gem_userptr_blits: Refresh other now MMAP_GTT dependent subtests (rev2)
Lakshmi, see below.
On Tue, Mar 17, 2020 at 09:53:51AM +0100, Janusz Krzysztofik wrote:
> Hi,
>
> On Mon, 2020-03-16 at 19:25 +, Patchwork wrote:
> > == Series Details ==
> >
> > Series: tests/gem_userptr_blits: Refresh other now MMAP_GTT dependent
> > subtests (rev2)
> > URL : https://patchwork.freedesktop.org/series/74201/
> > State : failure
> >
> > == Summary ==
> >
> > CI Bug Log - changes from CI_DRM_8137_full -> IGTPW_4307_full
> >
> >
> > Summary
> > ---
> >
> > **FAILURE**
>
> False positive, see below.
>
> > Serious unknown changes coming with IGTPW_4307_full absolutely need to be
> > verified manually.
> >
> > If you think the reported changes have nothing to do with the changes
> > introduced in IGTPW_4307_full, 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_4307/index.html
> >
> > Possible new issues
> > ---
> >
> > Here are the unknown changes that may have been introduced in
> > IGTPW_4307_full:
> >
> > ### IGT changes ###
> >
> > Possible regressions
> >
> > * igt@gem_ctx_shared@single-timeline:
> > - shard-snb: NOTRUN -> [FAIL][1]
> >[1]:
> > https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4307/shard-snb4/igt@gem_ctx_sha...@single-timeline.html
> > - shard-hsw: NOTRUN -> [FAIL][2] +1 similar issue
> >[2]:
> > https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4307/shard-hsw2/igt@gem_ctx_sha...@single-timeline.html
> >
> > * igt@gem_exec_fence@basic-await@vcs0:
> > - shard-kbl: [PASS][3] -> [FAIL][4] +3 similar issues
> >[3]:
> > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8137/shard-kbl1/igt@gem_exec_fence@basic-aw...@vcs0.html
> >[4]:
> > https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4307/shard-kbl7/igt@gem_exec_fence@basic-aw...@vcs0.html
> > - shard-iclb: [PASS][5] -> [FAIL][6]
> >[5]:
> > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8137/shard-iclb1/igt@gem_exec_fence@basic-aw...@vcs0.html
> >[6]:
> > https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4307/shard-iclb5/igt@gem_exec_fence@basic-aw...@vcs0.html
> > - shard-apl: [PASS][7] -> [FAIL][8] +2 similar issues
> >[7]:
> > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8137/shard-apl4/igt@gem_exec_fence@basic-aw...@vcs0.html
> >[8]:
> > https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4307/shard-apl4/igt@gem_exec_fence@basic-aw...@vcs0.html
> > - shard-glk: [PASS][9] -> [FAIL][10] +2 similar issues
> >[9]:
> > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8137/shard-glk6/igt@gem_exec_fence@basic-aw...@vcs0.html
> >[10]:
> > https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4307/shard-glk4/igt@gem_exec_fence@basic-aw...@vcs0.html
> > - shard-tglb: [PASS][11] -> [FAIL][12] +2 similar issues
> >[11]:
> > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8137/shard-tglb6/igt@gem_exec_fence@basic-aw...@vcs0.html
> >[12]:
> > https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4307/shard-tglb5/igt@gem_exec_fence@basic-aw...@vcs0.html
>
> Not related.
>
> >
> > * {igt@gem_userptr_blits@process-exit-mmap-busy@gtt} (NEW):
> > - shard-iclb: NOTRUN -> [SKIP][13] +7 similar issues
> >[13]:
> > https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4307/shard-iclb3/igt@gem_userptr_blits@process-exit-mmap-b...@gtt.html
> >
> > * {igt@gem_userptr_blits@process-exit-mmap-busy@uc} (NEW):
> > - shard-tglb: NOTRUN -> [SKIP][14] +11 similar issues
> >[14]:
> > https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4307/shard-tglb5/igt@gem_userptr_blits@process-exit-mmap-b...@uc.html
>
> Expected behavior.
>
> >
> > * igt@gem_wait@wait-rcs0:
> > - shard-hsw: [PASS][15] -> [FAIL][16]
> >[15]:
> > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8137/shard-hsw2/igt@gem_w...@wait-rcs0.html
> >[16]:
> > https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4307/shard-hsw2/igt@gem_w...@wait-rcs0.html
> >
> > * igt@kms_cursor_legacy@flip-vs-cursor-busy-crc-atomic:
> > - shard-snb: [PASS][17] -> [FAIL][18] +1 similar issue
> >[17]:
> > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8137/shard-snb2/igt@kms_cursor_leg...@flip-vs-cursor-busy-crc-atomic.html
> >[18]:
> > https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4307/shard-snb5/igt@kms_cursor_leg...@flip-vs-cursor-busy-crc-atomic.html
>
> Not related.
>
> Thanks,
> Janusz
>
>
> >
> >
> > Warnings
> >
> > * igt@gem_exec_reloc@basic-spin-bsd:
> > - shard-snb: [FAIL][19] ([i915#757]) -> [TIMEOUT][20]
> >[19]:
> > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8137/shard-snb4/igt@gem_exec_re...@basic-spin-bsd.html
> >[20]:
> >
Re: [Intel-gfx] [PATCH i-g-t 1/2] intel-ci: Tweak blacklist for very long running stability tests
On Mon, Mar 16, 2020 at 10:54:26AM +, Chris Wilson wrote:
> To exclude yynamic tests just use their group name?
Yes, the igt_subtest_with_dynamic("somename") macro creates a subtest
entry point just like igt_subtest, for the purposes of testlists and
blacklists.
>
> Signed-off-by: Chris Wilson
> Cc: Petri Latvala
> ---
> tests/intel-ci/blacklist.txt | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tests/intel-ci/blacklist.txt b/tests/intel-ci/blacklist.txt
> index 948b47569..184c23c37 100644
> --- a/tests/intel-ci/blacklist.txt
> +++ b/tests/intel-ci/blacklist.txt
> @@ -60,9 +60,9 @@ igt@gem_sync@(?!.*basic).*
> igt@gem_tiled_swapping@(?!non-threaded).*
> igt@gem_userptr_blits@(major|minor|forked|mlocked|swapping).*
> igt@gem_wait@.*hang.*
> -igt@sysfs_heartbeat_timeout@long.*
> -igt@sysfs_preemption_timeout@off.*
> -igt@sysfs_timeslice_duration@off.*
> +igt@sysfs_heartbeat_timeout@long
> +igt@sysfs_preemption_timeout@off
> +igt@sysfs_timeslice_duration@off
Please fix the test names along with this change. I spent some minutes
trying to figure out what changes, before I realized
sysfs_heartbeat_timeout doesn't exist. It's
sysfs_heartbeat_interval. sysfs_preemption_timeout is
sysfs_preempt_timeout.
Ways to doublecheck:
igt_runner -L -t igt@sysfs_heartbeat_timeout@long build/tests
igt_runner -L -b tests/intel-ci/blacklist.txt build/tests
https://patchwork.freedesktop.org/series/74263/
Acked-by: Petri Latvala
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [CI] PR for TGL DMC v2.06
On 2/28/20 8:52 PM, Souza, Jose wrote: On Fri, 2020-02-28 at 12:21 +0200, Petri Latvala wrote: On Thu, Feb 27, 2020 at 11:42:03PM +, Souza, Jose wrote: The following changes since commit efcfa03ae6100dfe523ebf612e03c3a90fc4c794: linux-firmware: Update firmware file for Intel Bluetooth AX201 (2020- 02-24 07:43:42 -0500) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-firmware tgl_dmc_2.06 for you to fetch changes up to e2396319167724e9ffddc377f300469923fccdcb: i915: Add DMC firmware v2.06 for TGL (2020-02-27 15:24:56 2020 -0800) José Roberto de Souza (1): i915: Add DMC firmware v2.06 for TGL WHENCE | 3 +++ i915/tgl_dmc_ver2_06.bin | Bin 0 -> 18660 bytes 2 files changed, 3 insertions(+) create mode 100644 i915/tgl_dmc_ver2_06.bin Patchwork didn't pick up this PR, I suspect the extra newlines to be the issue. Can you try resending this without the automatic newlines before the commit shas? If patchwork recognizes it as a pull request, it will appear in here: https://patchwork.freedesktop.org/api/1.0/projects/intel-gfx/events/?page=1=2020-02-20=pull-request-new Hi Petri Still needed? According to https://patchwork.freedesktop.org/patch/355624/?series=74048=2 it was manually picked. Not for getting the binary downloaded to CI anymore, but useful for pinpointing what went wrong so we can avoid this in the future. But I believe Arek has been (or plans to) messaging you about this. Petri Latvala ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] i915/gem_softpin: Limit the noreloc test runtime
On Fri, Feb 28, 2020 at 10:12:36AM +, Chris Wilson wrote:
> Use a fixed duration rather than a fixed amount of work.
>
> Closes: https://gitlab.freedesktop.org/drm/intel/issues/1325
> Signed-off-by: Chris Wilson
Reviewed-by: Petri Latvala
> ---
> tests/i915/gem_softpin.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tests/i915/gem_softpin.c b/tests/i915/gem_softpin.c
> index 2c258f443..98c7ab63b 100644
> --- a/tests/i915/gem_softpin.c
> +++ b/tests/i915/gem_softpin.c
> @@ -442,7 +442,7 @@ static void test_noreloc(int fd, enum sleep sleep)
> uint64_t offset;
> uint32_t handle;
> uint32_t *batch, *b;
> - int i, loop;
> + int i, loop = 0;
>
> handle = gem_create(fd, (ARRAY_SIZE(object)+1)*size);
> gem_write(fd, handle, 0, , sizeof(bbe));
> @@ -494,11 +494,11 @@ static void test_noreloc(int fd, enum sleep sleep)
> munmap(batch, size);
>
> execbuf.buffer_count = ARRAY_SIZE(object);
> - for (loop = 0; loop < 1024; loop++) {
> + igt_until_timeout(5) {
> igt_permute_array(object, ARRAY_SIZE(object)-1, xchg_offset);
> gem_execbuf(fd, );
>
> - if ((loop & 127) == 0) {
> + if ((loop++ & 127) == 0) {
> switch (sleep) {
> case NOSLEEP:
> break;
> --
> 2.25.1
>
> ___
> igt-dev mailing list
> igt-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [CI] PR for TGL DMC v2.06
On Thu, Feb 27, 2020 at 11:42:03PM +, Souza, Jose wrote: > The following changes since commit > efcfa03ae6100dfe523ebf612e03c3a90fc4c794: > > linux-firmware: Update firmware file for Intel Bluetooth AX201 (2020- > 02-24 07:43:42 -0500) > > are available in the Git repository at: > > git://anongit.freedesktop.org/drm/drm-firmware tgl_dmc_2.06 > > for you to fetch changes up to > e2396319167724e9ffddc377f300469923fccdcb: > > i915: Add DMC firmware v2.06 for TGL (2020-02-27 15:24:56 2020 -0800) > > > José Roberto de Souza (1): > i915: Add DMC firmware v2.06 for TGL > > WHENCE | 3 +++ > i915/tgl_dmc_ver2_06.bin | Bin 0 -> 18660 bytes > 2 files changed, 3 insertions(+) > create mode 100644 i915/tgl_dmc_ver2_06.bin Patchwork didn't pick up this PR, I suspect the extra newlines to be the issue. Can you try resending this without the automatic newlines before the commit shas? If patchwork recognizes it as a pull request, it will appear in here: https://patchwork.freedesktop.org/api/1.0/projects/intel-gfx/events/?page=1=2020-02-20=pull-request-new -- Petri Latvala ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] perf: Avoid the regular drm_open_driver exithandler
On Tue, Feb 25, 2020 at 11:44:54AM +, Chris Wilson wrote: > Due to the way i915_perf_open() works, it installs a wakeref on the fd. > This wakeref prevents the normal drm_open_driver() exithandler from > returning (as that waits for all wakerefs to ensure the GPU is idle). We > need to manually control the nesting of cleanup, and so need to use > __drm_open_driver() to avoid the default exithandler. > > References: https://gitlab.freedesktop.org/drm/intel/issues/1085#note_419148 > Signed-off-by: Chris Wilson Reviewed-by: Petri Latvala > --- > tests/perf.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/tests/perf.c b/tests/perf.c > index 7b11f668c..869e5f1e9 100644 > --- a/tests/perf.c > +++ b/tests/perf.c > @@ -4489,7 +4489,8 @@ igt_main >*/ > igt_assert_eq(drm_fd, -1); > > - drm_fd = drm_open_driver(DRIVER_INTEL); > + /* Avoid the normal exithandler, our perf-fd interferes */ > + drm_fd = __drm_open_driver(DRIVER_INTEL); > igt_require_gem(drm_fd); > > devid = intel_get_drm_devid(drm_fd); > -- > 2.25.1 > > ___ > 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
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t v2 5/5] i915/gem_ctx_isolation.c - If initialization fails, exit
On Thu, Feb 13, 2020 at 11:29:48AM -0800, Dale B Stimson wrote:
> On 2020-02-13 10:29:55, Petri Latvala wrote:
> > On Wed, Feb 12, 2020 at 05:28:40PM -0800, Dale B Stimson wrote:
> > > At the start of igt_main, failure of the initial tests for successful
> > > initialization transfer control to the end of an igt_fixture block.
> > > From there, execution of the main per-engine loop is attempted.
> > > Instead, the test should be caused to exit.
> > >
> > > If initialization fails, exit.
> > >
> > > Signed-off-by: Dale B Stimson
> > > ---
> > > tests/i915/gem_ctx_isolation.c | 15 +++
> > > 1 file changed, 15 insertions(+)
> > >
> > > diff --git a/tests/i915/gem_ctx_isolation.c
> > > b/tests/i915/gem_ctx_isolation.c
> > > index 07ffbb84a..b11158dab 100644
> > > --- a/tests/i915/gem_ctx_isolation.c
> > > +++ b/tests/i915/gem_ctx_isolation.c
> > > @@ -898,10 +898,13 @@ igt_main
> > > int fd = -1;
> > > struct eng_mmio_base_table_s *mbp = NULL;
> > > uint32_t mmio_base = 0;
> > > + /* igt_fixture block is skipped if --list-subtests, so start with true.
> > > */
> > > + bool init_successful = true;
> > >
> > > igt_fixture {
> > > int gen;
> > >
> > > + init_successful = false;
> > > fd = drm_open_driver(DRIVER_INTEL);
> > > igt_require_gem(fd);
> > > igt_require(gem_has_contexts(fd));
> > > @@ -916,8 +919,20 @@ igt_main
> > > igt_skip_on(gen > LAST_KNOWN_GEN);
> > >
> > > mbp = gem_engine_mmio_base_info_get(fd);
> > > + init_successful = true;
> > > }
> > >
> > > + if (!init_successful) {
> > > + igt_exit_early();
> > > + }
> > > +
> >
> > NAK. All this dancing around the infrastructure just makes changing
> > the infrastructure later be awkward and produce weird errors.
> >
> > If something in the fixture failed, with this code you never enter the
> > subtest, making the test result 'notrun' instead of the correct 'skip'
> > or 'fail'.
> >
> > What is the problem this is trying to solve? Incorrect engine list
> > used? If you have a subtest per static engine, all CI does is execute
> > per static engine. Converting this test to use dynamic subtests for
> > engines is the way forward.
> >
> >
> > --
> > Petri Latvala
>
> NAK understood and accepted.
>
> I will address this in a different way, targeting the underlying issues
> instead of the symptom. Please see my patch (just sent to ML):
> lib/i915/gem_engine_topology.c - intel_get_current_engine invalid result
>
> To answer to your question about what this was trying to solve:
>
> Briefly, and specifically addressing gem_ctx_isolation:
>
> As-is observed behavior when open (or debugfs open) fails: per-engine loop
> executes forever:
> Subtest vecs0-nonpriv: FAIL
> Subtest vecs0-nonpriv-switch: FAIL
> Subtest vecs0-clean: FAIL
> Subtest vecs0-dirty-create: FAIL
> Subtest vecs0-dirty-switch: FAIL
> Subtest vecs0-none: FAIL
> Subtest vecs0-S3: FAIL
> Subtest vecs0-S4: FAIL
> Subtest vecs0-reset: FAIL
> And repeat, ad infinitum for vecs0
>
Ah, the good old non-progressing engine loop. We already have fixes
for two of the occurrences, you have found a third one. =(
--
Petri Latvala
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t v2 5/5] i915/gem_ctx_isolation.c - If initialization fails, exit
On Wed, Feb 12, 2020 at 05:28:40PM -0800, Dale B Stimson wrote:
> At the start of igt_main, failure of the initial tests for successful
> initialization transfer control to the end of an igt_fixture block.
> From there, execution of the main per-engine loop is attempted.
> Instead, the test should be caused to exit.
>
> If initialization fails, exit.
>
> Signed-off-by: Dale B Stimson
> ---
> tests/i915/gem_ctx_isolation.c | 15 +++
> 1 file changed, 15 insertions(+)
>
> diff --git a/tests/i915/gem_ctx_isolation.c b/tests/i915/gem_ctx_isolation.c
> index 07ffbb84a..b11158dab 100644
> --- a/tests/i915/gem_ctx_isolation.c
> +++ b/tests/i915/gem_ctx_isolation.c
> @@ -898,10 +898,13 @@ igt_main
> int fd = -1;
> struct eng_mmio_base_table_s *mbp = NULL;
> uint32_t mmio_base = 0;
> + /* igt_fixture block is skipped if --list-subtests, so start with true.
> */
> + bool init_successful = true;
>
> igt_fixture {
> int gen;
>
> + init_successful = false;
> fd = drm_open_driver(DRIVER_INTEL);
> igt_require_gem(fd);
> igt_require(gem_has_contexts(fd));
> @@ -916,8 +919,20 @@ igt_main
> igt_skip_on(gen > LAST_KNOWN_GEN);
>
> mbp = gem_engine_mmio_base_info_get(fd);
> + init_successful = true;
> }
>
> + if (!init_successful) {
> + igt_exit_early();
> + }
> +
NAK. All this dancing around the infrastructure just makes changing
the infrastructure later be awkward and produce weird errors.
If something in the fixture failed, with this code you never enter the
subtest, making the test result 'notrun' instead of the correct 'skip'
or 'fail'.
What is the problem this is trying to solve? Incorrect engine list
used? If you have a subtest per static engine, all CI does is execute
per static engine. Converting this test to use dynamic subtests for
engines is the way forward.
--
Petri Latvala
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 4/5] i915: Exercise sysfs heartbeat controls
On Mon, Jan 27, 2020 at 12:18:17PM +, Chris Wilson wrote: > We [will] expose various per-engine scheduling controls. One of which, > 'heartbeat_duration_ms', defines how often we send a heartbeat down the > engine to check upon the health of the engine. If a heartbeat does not > complete within the interval (or two), the engine is declared hung. > > Signed-off-by: Chris Wilson > --- > tests/Makefile.sources| 3 + > tests/i915/sysfs_heartbeat_interval.c | 466 ++ > tests/meson.build | 1 + > 3 files changed, 470 insertions(+) > create mode 100644 tests/i915/sysfs_heartbeat_interval.c > > diff --git a/tests/Makefile.sources b/tests/Makefile.sources > index fc9e04e97..fd6f67a73 100644 > --- a/tests/Makefile.sources > +++ b/tests/Makefile.sources > @@ -102,6 +102,9 @@ TESTS_progs = \ > vgem_slow \ > $(NULL) > > +TESTS_progs += sysfs_heartbeat_interval > +sysfs_heartbeat_interval_SOURCES = i915/sysfs_heartbeat_interval Another missing .c -- Petri Latvala ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 3/5] i915: Exercise preemption timeout controls in sysfs
t(engine, "name");
> + if (!name) {
> + close(engine);
> + continue;
> + }
> +
> + igt_dynamic(name) {
> + if (file) {
> + struct stat st;
> +
> + igt_require(fstatat(engine, file, ,
> 0) == 0);
> + }
> +
> + test(i915, engine);
> + }
> +
> + close(engine);
> + }
> + }
> +}
> diff --git a/lib/i915/gem_engine_topology.h b/lib/i915/gem_engine_topology.h
> index 7a2e21f66..456c806f5 100644
> --- a/lib/i915/gem_engine_topology.h
> +++ b/lib/i915/gem_engine_topology.h
> @@ -77,4 +77,7 @@ int gem_engine_property_scanf(int i915, const char *engine,
> const char *attr,
> const char *fmt, ...);
> uint32_t gem_engine_mmio_base(int i915, const char *engine);
>
> +void dyn_sysfs_engines(int i915, int engines, const char *file,
> +void (*test)(int i915, int engine));
> +
> #endif /* GEM_ENGINE_TOPOLOGY_H */
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index 7c5693457..fc9e04e97 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -102,6 +102,9 @@ TESTS_progs = \
> vgem_slow \
> $(NULL)
>
> +TESTS_progs += sysfs_preempt_timeout
> +sysfs_preempt_timeout_SOURCES = i915/sysfs_preempt_timeout
Your .c dropped off.
--
Petri Latvala
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 1/3] i915/gem_mmio_base.c - get mmio_base from debugfs (if possible)
On Tue, Feb 11, 2020 at 11:37:32AM +0200, Petri Latvala wrote: > On Tue, Feb 11, 2020 at 11:30:03AM +0200, Jani Nikula wrote: > > On Tue, 11 Feb 2020, Petri Latvala wrote: > > >> diff --git a/lib/i915/gem_mmio_base.c b/lib/i915/gem_mmio_base.c > > >> new file mode 100644 > > >> index 0..8718c092f > > >> --- /dev/null > > >> +++ b/lib/i915/gem_mmio_base.c > > >> @@ -0,0 +1,346 @@ > > >> +// Copyright (C) 2020 Intel Corporation > > >> +// > > >> +// SPDX-License-Identifier: MIT > > > > > > We don't use SPDX headers in IGT, please use the MIT license comment > > > block instead. > > > > Why not? Should we start? > > I'm all for it, but preferably not trickled in but as separate conversion! I have been informed from people with experience that converting to SPDX is more convoluted than I thought. Thus, take-backsies! SPDX for new files is ok. You can quote me on this. -- Petri Latvala ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 1/3] i915/gem_mmio_base.c - get mmio_base from debugfs (if possible)
On Tue, Feb 11, 2020 at 11:30:03AM +0200, Jani Nikula wrote: > On Tue, 11 Feb 2020, Petri Latvala wrote: > >> diff --git a/lib/i915/gem_mmio_base.c b/lib/i915/gem_mmio_base.c > >> new file mode 100644 > >> index 0..8718c092f > >> --- /dev/null > >> +++ b/lib/i915/gem_mmio_base.c > >> @@ -0,0 +1,346 @@ > >> +// Copyright (C) 2020 Intel Corporation > >> +// > >> +// SPDX-License-Identifier: MIT > > > > We don't use SPDX headers in IGT, please use the MIT license comment > > block instead. > > Why not? Should we start? I'm all for it, but preferably not trickled in but as separate conversion! -- Petri Latvala ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 1/3] i915/gem_mmio_base.c - get mmio_base from debugfs (if possible)
On Mon, Feb 10, 2020 at 04:46:11PM -0800, Dale B Stimson wrote:
> Signed-off-by: Dale B Stimson
> ---
> lib/Makefile.sources | 2 +
> lib/i915/gem_mmio_base.c | 346 +++
> lib/i915/gem_mmio_base.h | 19 +++
> lib/igt.h| 1 +
> lib/meson.build | 1 +
> 5 files changed, 369 insertions(+)
> create mode 100644 lib/i915/gem_mmio_base.c
> create mode 100644 lib/i915/gem_mmio_base.h
>
> diff --git a/lib/Makefile.sources b/lib/Makefile.sources
> index 3e573f267..4c5d50d5d 100644
> --- a/lib/Makefile.sources
> +++ b/lib/Makefile.sources
> @@ -7,6 +7,8 @@ lib_source_list = \
> i915/gem_context.h \
> i915/gem_engine_topology.c \
> i915/gem_engine_topology.h \
> + i915/gem_mmio_base.c\
> + i915/gem_mmio_base.h\
> i915/gem_scheduler.c\
> i915/gem_scheduler.h\
> i915/gem_submission.c \
> diff --git a/lib/i915/gem_mmio_base.c b/lib/i915/gem_mmio_base.c
> new file mode 100644
> index 0..8718c092f
> --- /dev/null
> +++ b/lib/i915/gem_mmio_base.c
> @@ -0,0 +1,346 @@
> +// Copyright (C) 2020 Intel Corporation
> +//
> +// SPDX-License-Identifier: MIT
We don't use SPDX headers in IGT, please use the MIT license comment
block instead.
--
Petri Latvala
> +
> +#include
> +
> +#include
> +
> +#include "igt.h"
> +
> +struct eng_mmio_base_s {
> + char name[8];
> + uint32_t mmio_base;
> +};
> +
> +struct eng_mmio_base_table_s {
> + unsigned int mb_cnt;
> + struct eng_mmio_base_s mb_tab[GEM_MAX_ENGINES];
> +};
> +
> +
> +static struct eng_mmio_base_table_s *_gem_engine_mmio_info_dup(
> + const struct eng_mmio_base_table_s *mbpi)
> +{
> + struct eng_mmio_base_table_s *mbpo;
> + size_t nbytes;
> +
> + nbytes = offsetof(typeof(struct eng_mmio_base_table_s),
> mb_tab[mbpi->mb_cnt]);
> + mbpo = malloc(nbytes);
> + igt_assert(mbpo);
> + memcpy(mbpo, mbpi, nbytes);
> +
> + return mbpo;
> +}
> +
> +void gem_engine_mmio_base_info_free(struct eng_mmio_base_table_s *mbp)
> +{
> + free(mbp);
> +}
> +
> +static void _gem_engine_mmio_info_legacy_add(struct eng_mmio_base_table_s
> *mbp,
> + const char *eng_name, uint32_t mmio_base)
> +{
> + if (mmio_base) {
> + strncpy(mbp->mb_tab[mbp->mb_cnt].name, eng_name,
> + sizeof(mbp->mb_tab[0].name));
> + mbp->mb_tab[mbp->mb_cnt].mmio_base = mmio_base;
> + mbp->mb_cnt++;
> + }
> +}
> +
> +/**
> + * _gem_engine_mmio_base_info_get_legacy:
> + * @fd_dev: file descriptor upon which device is open or -1 to use defaults.
> + *
> + * Provides per-engine mmio_base information from legacy built-in values
> + * for the case when the information is not otherwise available.
> + *
> + * Returns:
> + * Pointer to dynamically allocated struct eng_mmio_base_table_s describing
> + * engine config or NULL.
> + * The allocated size does not include unused engine entries.
> + * If non-NULL, it is caller's responsibility to free.
> + */
> +static struct eng_mmio_base_table_s
> *_gem_engine_mmio_base_info_get_legacy(int fd_dev)
> +{
> + int gen;
> + uint32_t mmio_base;
> + struct eng_mmio_base_table_s mbt;
> + struct eng_mmio_base_table_s *mbp;
> +
> + memset(, 0, sizeof(mbt));
> +
> + gen = intel_gen(intel_get_drm_devid(fd_dev));
> +
> + /* The mmio_base values for engine instances 1 and higher cannot
> + * be reliability determinated a priori. */
> +
> + _gem_engine_mmio_info_legacy_add(, "rcs0", 0x2000);
> + _gem_engine_mmio_info_legacy_add(, "bcs0", 0x22000);
> +
> + if (gen < 6)
> + mmio_base = 0x4000;
> + else if (gen < 11)
> + mmio_base = 0x12000;
> + else
> + mmio_base = 0x1c;
> + _gem_engine_mmio_info_legacy_add(, "vcs0", mmio_base);
> +
> + if (gen < 11)
> + mmio_base = 0x1a000;
> + else
> + mmio_base = 0x1c8000;
> + _gem_engine_mmio_info_legacy_add(, "vecs0", mmio_base);
> +
> + if (mbt.mb_cnt <= 0)
> + return NULL;
> +
> + mbp = _gem_engine_mmio_info_dup();
> +
> + return mbp;
> +}
> +
> +
> +/**
> + * _gem_engine_mmio_base_info_get_debugfs:
> + * @fd_dev: file descriptor upon which device is open or -1 to use defaults.
> + *
> + * Obtains per-engine mmio_base information from debugfs.
> + *
> + * Returns:
> + * Pointer to dyn
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] intel-ci: Enable gem_exec_whisper
On Mon, Feb 10, 2020 at 10:11:35AM +, Chris Wilson wrote: > In hindsignt, Your h is damaged. -- Petri Latvala ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC PATCH i-g-t 0/1] tests/gem_mmap_offset: Exercise mapping to userptr
On Fri, Jan 31, 2020 at 02:12:33PM +0100, Janusz Krzysztofik wrote: > I'm adding a cover letter in case it is required for having a related > kernel side patch been tested with this one. For the record, Test-With doesn't require the IGT side to have a cover letter. -- Petri Latvala ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] i915/gem_ctx_isolation: Use static iterator
On Thu, Jan 23, 2020 at 12:01:34PM +, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin
>
> Quick fixup to the test so correct way of iterating the static engine list
> is used. More comprehensive fixes to the test are in progress.
>
> Signed-off-by: Tvrtko Ursulin
Reviewed-by: Petri Latvala
> ---
> tests/i915/gem_ctx_isolation.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tests/i915/gem_ctx_isolation.c b/tests/i915/gem_ctx_isolation.c
> index 6aa27133cbb7..8b72a16ad86f 100644
> --- a/tests/i915/gem_ctx_isolation.c
> +++ b/tests/i915/gem_ctx_isolation.c
> @@ -857,6 +857,7 @@ static unsigned int __has_context_isolation(int fd)
> igt_main
> {
> unsigned int has_context_isolation = 0;
> + const struct intel_execution_engine2 *e;
> int fd = -1;
>
> igt_fixture {
> @@ -876,8 +877,7 @@ igt_main
> igt_skip_on(gen > LAST_KNOWN_GEN);
> }
>
> - for (const struct intel_execution_engine2 *e = intel_execution_engines2;
> - e->name; e++) {
> + __for_each_static_engine(e) {
> igt_subtest_group {
> igt_fixture {
> igt_require(has_context_isolation & (1 <<
> e->class));
> --
> 2.20.1
>
> ___
> 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
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] i915/gem_engine_topology: Generate engine names based on class
On Wed, Jan 22, 2020 at 10:15:56AM +, Tvrtko Ursulin wrote:
>
> On 22/01/2020 10:10, Tvrtko Ursulin wrote:
> > From: Tvrtko Ursulin
> >
> > With the introduction of dynamic subtests we got one step closer towards
> > eliminating the duality of static and dynamic engine enumeration.
> >
> > This patch makes one more step in that direction by removing the
> > dependency on the static list when generating probed engine names.
> >
> > Signed-off-by: Tvrtko Ursulin
> > Cc: Andi Shyti
> > Cc: Petri Latvala
> > ---
> > lib/i915/gem_engine_topology.c | 39 +++---
> > lib/igt_gt.h | 2 +-
> > 2 files changed, 23 insertions(+), 18 deletions(-)
> >
> > diff --git a/lib/i915/gem_engine_topology.c b/lib/i915/gem_engine_topology.c
> > index 790d455ff2ad..eff231800b77 100644
> > --- a/lib/i915/gem_engine_topology.c
> > +++ b/lib/i915/gem_engine_topology.c
> > @@ -97,39 +97,43 @@ static void ctx_map_engines(int fd, struct
> > intel_engine_data *ed,
> > gem_context_set_param(fd, param);
> > }
> > +static const char *class_names[] = {
> > + [I915_ENGINE_CLASS_RENDER]= "rcs",
> > + [I915_ENGINE_CLASS_COPY] = "bcs",
> > + [I915_ENGINE_CLASS_VIDEO] = "vcs",
> > + [I915_ENGINE_CLASS_VIDEO_ENHANCE] = "vecs",
> > +};
> > +
> > static void init_engine(struct intel_execution_engine2 *e2,
> > int class, int instance, uint64_t flags)
> > {
> > - const struct intel_execution_engine2 *__e2;
> > - static const char *unknown_name = "unknown",
> > - *virtual_name = "virtual";
> > + int ret;
> > e2->class= class;
> > e2->instance = instance;
> > - e2->flags= flags;
> > /* engine is a virtual engine */
> > if (class == I915_ENGINE_CLASS_INVALID &&
> > instance == I915_ENGINE_CLASS_INVALID_VIRTUAL) {
> > - e2->name = virtual_name;
> > + strcpy(e2->name, "virtual");
> > e2->is_virtual = true;
> > return;
> > + } else {
> > + e2->is_virtual = false;
> > }
> > - __for_each_static_engine(__e2)
> > - if (__e2->class == class && __e2->instance == instance)
> > - break;
> > -
> > - if (__e2->name) {
> > - e2->name = __e2->name;
> > + if (class < ARRAY_SIZE(class_names)) {
> > + e2->flags = flags;
> > + ret = snprintf(e2->name, sizeof(e2->name), "%s%u",
> > + class_names[class], instance);
> > } else {
> > igt_warn("found unknown engine (%d, %d)\n", class, instance);
> > - e2->name = unknown_name;
> > e2->flags = -1;
> > + ret = snprintf(e2->name, sizeof(e2->name), "%u-%u",
> > + class, instance);
> > }
> > - /* just to remark it */
> > - e2->is_virtual = false;
> > + igt_assert(ret < sizeof(e2->name));
> > }
> > static void query_engine_list(int fd, struct intel_engine_data *ed)
> > @@ -223,7 +227,7 @@ struct intel_engine_data intel_init_engine_list(int fd,
> > uint32_t ctx_id)
> > struct intel_execution_engine2 *__e2 =
> > _data.engines[engine_data.nengines];
> > - __e2->name = e2->name;
> > + strcpy(__e2->name, e2->name);
> > __e2->instance = e2->instance;
> > __e2->class = e2->class;
> > __e2->flags = e2->flags;
> > @@ -297,12 +301,11 @@ struct intel_execution_engine2
> > gem_eb_flags_to_engine(unsigned int flags)
> > .class = -1,
> > .instance = -1,
> > .flags = -1,
> > - .name = "invalid"
> > };
> > if (ring == I915_EXEC_DEFAULT) {
> > e2__.flags = I915_EXEC_DEFAULT;
> > - e2__.name = "default";
> > + strcpy(e2__.name, "default");
> > } else {
> > const struct intel_execution_engine2 *e2;
> > @@ -310,6 +313,8 @@ struct intel_execution_engine2
> > gem_eb_flags_to_engine(unsigned int flags)
> > if (e2->flags == rin
Re: [Intel-gfx] [RESEND PATCH i-g-t] tests/prime_vgem: Give meaningful messages on SKIP
On Thu, Jan 16, 2020 at 11:13:26AM +0100, Janusz Krzysztofik wrote:
> Messages displayed on SKIPs introduced by commit 92caadb4e551
> ("tests/prime_vgem: Skip basic-read/write subtests if not supported")
> don't inform clearly enough that those SKIPs are expected behavior.
> Fix it.
>
> Signed-off-by: Janusz Krzysztofik
> Reviewed-by: Ewelina Musial
> ---
> Assuming the R-b: from Ewelina is sufficient, can someone push this,
> please? I can't do that myself as I have no push permissions.
>
Pushed, thanks for the patch and review.
--
Petri Latvala
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] tests/i915/gem_mmap_offset: Add new API test for gem_mmap_offset
TE,
> + t->type);
> + if (!ptr)
> + continue;
> +
> + igt_set_timeout(1, t->name);
> + /* no set-domain as we want to verify the pagefault is async */
> + ptr[256] = 0;
> + igt_reset_timeout();
> +
> + munmap(ptr, 4096);
> + }
> +
> + igt_spin_free(i915, spin);
> +}
> +
> +static void close_race(int i915, int timeout)
> +{
> + const int ncpus = sysconf(_SC_NPROCESSORS_ONLN);
> + _Atomic uint32_t *handles;
> + size_t len = ALIGN((ncpus + 1) * sizeof(uint32_t), 4096);
> +
> + handles = mmap64(0, len, PROT_WRITE, MAP_SHARED | MAP_ANON, -1, 0);
> + igt_assert(handles != MAP_FAILED);
> +
> + igt_fork(child, ncpus + 1) {
> + do {
> + struct drm_i915_gem_mmap_offset mmap_arg = {};
> + const int i = 1 + random() % ncpus;
> + uint32_t old;
> +
> + mmap_arg.handle = gem_create(i915, 4096);
> + mmap_arg.flags = I915_MMAP_OFFSET_WB;
> + old = atomic_exchange([i], mmap_arg.handle);
This will require adding libatomic to this executable's dependencies
with meson. Look to handling of gem_create.c for an example.
--
Petri Latvala
> + ioctl(i915, DRM_IOCTL_GEM_CLOSE, );
> +
> + if (ioctl(i915,
> + DRM_IOCTL_I915_GEM_MMAP_OFFSET,
> + _arg) != -1) {
> + void *ptr;
> +
> + ptr = mmap64(0, 4096,
> + PROT_WRITE, MAP_SHARED, i915,
> + mmap_arg.offset);
> + if (ptr != MAP_FAILED) {
> + *(volatile uint32_t *)ptr = 0;
> + munmap(ptr, 4096);
> + }
> + }
> +
> + } while (!READ_ONCE(handles[0]));
> + }
> +
> + sleep(timeout);
> + handles[0] = 1;
> + igt_waitchildren();
> +
> + for (int i = 1; i <= ncpus; i++)
> + ioctl(i915, DRM_IOCTL_GEM_CLOSE, handles[i]);
> + munmap(handles, len);
> +}
> +
> +static uint64_t atomic_compare_swap_u64(_Atomic(uint64_t) *ptr,
> + uint64_t oldval, uint64_t newval)
> +{
> + atomic_compare_exchange_strong(ptr, , newval);
> + return oldval;
> +}
> +
> +static uint64_t get_npages(_Atomic(uint64_t) *global, uint64_t npages)
> +{
> + uint64_t try, old, max;
> +
> + max = *global;
> + do {
> + old = max;
> + try = 1 + npages % (max / 2);
> + max -= try;
> + } while ((max = atomic_compare_swap_u64(global, old, max)) != old);
> +
> + return try;
> +}
> +
> +struct thread_clear {
> + _Atomic(uint64_t) max;
> + int timeout;
> + int i915;
> +};
> +
> +static int create_ioctl(int i915, struct drm_i915_gem_create *create)
> +{
> + int err = 0;
> +
> + if (igt_ioctl(i915, DRM_IOCTL_I915_GEM_CREATE, create)) {
> + err = -errno;
> + igt_assume(err != 0);
> + }
> +
> + errno = 0;
> + return err;
> +}
> +
> +static void *thread_clear(void *data)
> +{
> + struct thread_clear *arg = data;
> + const struct mmap_offset *t;
> + unsigned long checked = 0;
> + int i915 = arg->i915;
> +
> + t = mmap_offset_types;
> + igt_until_timeout(arg->timeout) {
> + struct drm_i915_gem_create create = {};
> + uint64_t npages;
> + void *ptr;
> +
> + npages = random();
> + npages <<= 32;
> + npages |= random();
> + npages = get_npages(>max, npages);
> + create.size = npages << 12;
> +
> + create_ioctl(i915, );
> + ptr = __gem_mmap_offset(i915, create.handle, 0, create.size,
> + PROT_READ | PROT_WRITE,
> + t->type);
> + /* No set-domains as we are being as naughty as possible */
> + for (uint64_t page = 0; ptr && page < npages; page++) {
> + uint64_t x[8] = {
> + page * 4096 +
> + sizeof(x) * ((page % (4096 - sizeof(x))
> / sizeof(x)))
> + };
> +
> + if (page & 1)
> +
Re: [Intel-gfx] [PATCH i-g-t 1/2] Remove i915/gem_cpu_reloc
On Thu, Nov 28, 2019 at 08:34:50AM +, Chris Wilson wrote: > The test does not do what is on the tin since the kernel smartly > replaces the stalls with gpu relocations, and all the test is providing > is a trivial amount of stress beyond gem_exec_reloc. > > Signed-off-by: Chris Wilson > --- > tests/Makefile.sources | 3 - > tests/i915/gem_cpu_reloc.c | 309 - > tests/meson.build | 1 - Remove igt@gem_cpu_reloc@basic from fast-feedback.testlist too. -- Petri Latvala ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] cve: Add checker for cve-2019-0155
On Fri, Nov 22, 2019 at 09:20:11AM +, Chris Wilson wrote: > Quoting Petri Latvala (2019-11-22 09:14:07) > > On Thu, Nov 21, 2019 at 05:19:30PM +0200, Mika Kuoppala wrote: > > > Add vulnerability checker for cve-2019-0155 > > > > > > v2: sync, bailout early if no parser (Chris) > > > > > > Cc: Chris Wilson > > > Cc: Jon Bloomfield > > > Cc: Joonas Lahtinen > > > References: http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2019-0155 > > > References: > > > https://www.intel.com/content/www/us/en/security-center/advisory/intel-sa-00242.html > > > Signed-off-by: Mika Kuoppala > > > --- > > > Makefile.am | 2 +- > > > configure.ac | 1 + > > > cve/Makefile.am | 14 ++ > > > cve/Makefile.sources | 5 + > > > cve/cve-2019-0155.c | 470 +++ > > > cve/meson.build | 12 ++ > > > meson.build | 1 + > > > > Why do we need a new source directory and new install directory for > > this? Can't this be in tools/? > > Because we would like to carve out a niche for these. If Google asks for > a verifier for every single bug we encounter, it's going to be a huge > directory. Ok. -- Petri Latvala ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] cve: Add checker for cve-2019-0155
On Thu, Nov 21, 2019 at 05:19:30PM +0200, Mika Kuoppala wrote: > Add vulnerability checker for cve-2019-0155 > > v2: sync, bailout early if no parser (Chris) > > Cc: Chris Wilson > Cc: Jon Bloomfield > Cc: Joonas Lahtinen > References: http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2019-0155 > References: > https://www.intel.com/content/www/us/en/security-center/advisory/intel-sa-00242.html > Signed-off-by: Mika Kuoppala > --- > Makefile.am | 2 +- > configure.ac | 1 + > cve/Makefile.am | 14 ++ > cve/Makefile.sources | 5 + > cve/cve-2019-0155.c | 470 +++ > cve/meson.build | 12 ++ > meson.build | 1 + Why do we need a new source directory and new install directory for this? Can't this be in tools/? -- Petri Latvala ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [ANNOUNCEMENT] Documenting tests with igt_describe()
On Tue, Nov 19, 2019 at 02:37:17PM +0200, Lionel Landwerlin wrote: > On 08/11/2019 11:04, Arkadiusz Hiler wrote: > > On Thu, Nov 07, 2019 at 09:09:34PM +, Chris Wilson wrote: > > > Quoting Arkadiusz Hiler (2019-11-07 17:38:20) > > > > We don't want you to translate C into English, we want you to provide a > > > > bit of > > > > that extra information that you would have put in the comments anyway. > > > The comments should exist and are _inline_ with the code. > > And I encourage doing so. Detailed comments what this particular > > non-obvious chunk of code is doing are always welcome. > > > > > In all the examples of igt_describe() I have seen, it is nowhere near > > > the code so is useless; the information conveyed does not assist > > > anyone in diagnosing or debugging the problem, so I yet to understand > > > how it helps. > > They are supposed to document not the implementation but what is the > > grand idea behind a given subtest, so they are on a subtest level (or a > > group of subtests), which is our basic testing unit - this is what fails > > in CI, this is what you execute locally to reproduce the issue. > > > > Can you truly debug an issue or understand what the failure means > > without knowing what the test is supposed to prove? > > > > A lot of people here have struggled with this and often it takes a lot > > of time and requires advanced archeology with `git blame` hoping that > > there is at least one detailed enough commit message in there. > > > > If not then talking to people and relying on our tribal knowledge is > > required. > > > > As I have mentioned - getting the bigger picture from implementation > > details is hard. I get that you are not affected by this, but please do > > not deny the others. > > > I kind of agree with Chris that I don't find that additional macro useful > from the point of view of reading a particular test. > > A comment above the test function seems more appropriate, at least you don't > have to look at 2 different places to find out about a particular test. > > > Unless there is some untold goal here, like producing some kind of report in > an automated way. Like this one? https://drm.pages.freedesktop.org/igt-gpu-tools/igt-kms-tests.html#kms_chamelium -- Petri Latvala ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
