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 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] [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] [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] [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] [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] 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] ✗ 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:
> >
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 lef
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] [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/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] [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
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] [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)&ext3,
> + * .name = ...,
> + * };
> + * struct i915_user_extension ext1 {
> + * .next_extension = (uintptr_t)&ext2,
> + * .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;
> };
>
> -/**
> +/*
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] [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 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] [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] 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] ✗ 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.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] [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 (&dev->master_mutex){+.+.}-{3:3}, at:
> drm_is_current_master+0x1b/0x40
> but task is already holding lock:
> 88810dcd0488 (&dev->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 (&dev->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 (&client->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 (&dev->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: &dev->master_mutex --> &client->modeset_mutex -->
> &dev->mode_config.mutex
> Possible unsafe locking scenario:
>CPU0CPU1
>
> lock(&dev->mode_config.mutex);
>lock(&client->modeset_mutex);
>lock(&dev->mode_config.mutex);
> lock(&dev->master_mutex);
> *** DEADLOCK ***
> 1 lock held by kms_frontbuffer/1087:
> #0: 88810dcd0488 (&dev->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
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 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
[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 meso
Re: [Intel-gfx] [PATCH i-g-t] intel-ci/blacklist: Exclude gem_exec_parse lri
On Mon, Jan 07, 2019 at 04:40:15PM +, Chris Wilson wrote: > These exercise a certain HW misfeature, no longer protected by the > kernel cmdparser due to obsolete userspace requirements. > > Signed-off-by: Chris Wilson > Cc: Joonas Lahtinen > --- > tests/intel-ci/blacklist.txt | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tests/intel-ci/blacklist.txt b/tests/intel-ci/blacklist.txt > index 73d127603..0c6b87695 100644 > --- a/tests/intel-ci/blacklist.txt > +++ b/tests/intel-ci/blacklist.txt > @@ -35,6 +35,7 @@ igt@gem_exec_gttfill@(?!.*basic).* > igt@gem_exec_latency(@.*)? > igt@gem_exec_lut_handle(@.*)? > igt@gem_exec_nop@(?!.*(basic|signal-all)).* > +igt@gem_exec_parse@.*lri.* > igt@gem_exec_reloc@(?!.*basic).* > igt@gem_exec_suspend@(?!.*basic).* > igt@gem_exec_whisper@(?!normal$).* Put this in a separate section with a comment (commit message seems sufficient) that explains why it's blacklisted. With that, 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] Make force work with multiple drivers available
On Fri, Jan 11, 2019 at 11:13:37AM -0200, Rodrigo Siqueira wrote:
> The force option allows users to specify which driver they want that IGT
> uses. Nonetheless, if the user has two or more loaded drivers in his
> system, the force option will not work as expected because IGT will take
> the first driver found at /dev/dri. This problem can be reproduced in a
> QEMU VM that using Bochs and VKMS. This patch handles this scenario by
> ensuring that IGT uses the forced module specified via IGT_FORCE_DRIVER.
>
> Signed-off-by: Rodrigo Siqueira
> ---
> lib/drmtest.c | 12
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/lib/drmtest.c b/lib/drmtest.c
> index 35914c50..7c124ac6 100644
> --- a/lib/drmtest.c
> +++ b/lib/drmtest.c
> @@ -250,10 +250,8 @@ static int open_device(const char *name, unsigned int
> chipset)
> goto err;
>
> forced = forced_driver();
> - if (forced && chipset == DRIVER_ANY && !strcmp(forced, dev_name)) {
> - igt_debug("Force option used: Using driver %s\n", dev_name);
> - return fd;
> - }
> + if (forced && chipset == DRIVER_ANY && strcmp(forced, dev_name))
> + goto err;
Yep, an obvious logic error by me.
Reviewed-by: Petri Latvala
and merging this momentarily.
___
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/selftest: Allow filtering of individual subtests
On Tue, Jan 22, 2019 at 06:35:21PM +, Chris Wilson wrote:
> Take an environment variable, SELFTESTS=foo,bar, and pass that along to
> the kernel (as i915.st_filter=foo,bar) to provide fine grained test
> selection. This can be either as an exact match to select only that
> test, or to exclude only test. For example,
>
> SELFTESTS=igt_vma_create,igt_vma_pin1 i915_selftest --run mock_vma
> SELFTESTS=!igt_vma_create i915_selftest --run mock_vma
>
> Signed-off-by: Chris Wilson
> ---
> tests/i915/selftest.c | 18 --
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/tests/i915/selftest.c b/tests/i915/selftest.c
> index 80e515c61..b8d7f0af1 100644
> --- a/tests/i915/selftest.c
> +++ b/tests/i915/selftest.c
> @@ -28,10 +28,16 @@ IGT_TEST_DESCRIPTION("Basic unit tests for i915.ko");
>
> igt_main
> {
> - igt_kselftests("i915",
> -"mock_selftests=-1 disable_display=1",
> -NULL, "mock");
> - igt_kselftests("i915",
> -"live_selftests=-1 disable_display=1",
> -"live_selftests", "live");
> + const char *env = getenv("SELFTESTS");
> + char opts[1024];
> +
> + igt_assert(snprintf(opts, sizeof(opts),
> + "mock_selftests=-1 disable_display=1 st_filter=%s",
> + env) < sizeof(opts));
I don't particularly like passing NULL to %s, even though glibc makes
it print "(null)".
--
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_create: Do not build create-clear for MIPS
On Mon, Apr 01, 2019 at 04:39:24PM +0200, Guillaume Tucker wrote:
> The MIPS architecture doesn't provide the hardware atomics that are
> required for the "create-clear" sub-test such as
> __sync_add_and_fetch(). As a simple and pragmatic solution, disable
> this sub-test when building for MIPS. A better approach would be to
> add a fallback implementation for these operations.
>
> Fixes: 6727e17c00b2 ("i915/gem_create: Verify that all new objects are clear")
> Signed-off-by: Guillaume Tucker
> ---
> tests/i915/gem_create.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/tests/i915/gem_create.c b/tests/i915/gem_create.c
> index 2a861ca8a7ec..8a48496e6c19 100644
> --- a/tests/i915/gem_create.c
> +++ b/tests/i915/gem_create.c
> @@ -142,6 +142,7 @@ static void invalid_nonaligned_size(int fd)
> gem_close(fd, handle);
> }
>
> +#if !defined(__mips__) /* MIPS doesn't provide the required hardware atomics
> */
> static uint64_t get_npages(uint64_t *global, uint64_t npages)
> {
> uint64_t try, old, max;
> @@ -208,6 +209,7 @@ static void always_clear(int i915, int timeout)
> for (int i = 0; i < ncpus; i++)
> pthread_join(thread[i], NULL);
> }
> +#endif /* !defined(__mips__) */
>
> igt_main
> {
> @@ -231,6 +233,8 @@ igt_main
> igt_subtest("create-invalid-nonaligned")
> invalid_nonaligned_size(fd);
>
> +#if !defined(__mips__)
> igt_subtest("create-clear")
> always_clear(fd, 30);
> +#endif
> }
It's a bit ugly. I wonder how much work a fallback mechanism would be?
The test is i915 specific and using those on non-x86 architectures
sounds silly. We could limit building tests/i915/* only if
host_machine.cpu_family() is x86 or x86_64. But that requires
revisiting this issue if ever the day comes when i915 can be used on
other architectures *cough*.
Apropos, compile-testing on MIPS in gitlab-CI?
A compile-tested-only fallback mechanism suggestion, and a bad spot
for placing the fallback functions:
diff --git a/meson.build b/meson.build
index 557400a5..0552e858 100644
--- a/meson.build
+++ b/meson.build
@@ -246,6 +246,9 @@ endif
have = cc.has_function('memfd_create', prefix : '''#include ''',
args : '-D_GNU_SOURCE')
config.set10('HAVE_MEMFD_CREATE', have)
+have_atomics = cc.compiles('void f() { int x, y; __sync_add_and_fetch(&x, y);
}')
+config.set10('HAVE_BUILTIN_ATOMICS', have_atomics)
+
add_project_arguments('-D_GNU_SOURCE', language : 'c')
add_project_arguments('-include', 'config.h', language : 'c')
diff --git a/tests/i915/gem_create.c b/tests/i915/gem_create.c
index 2a861ca8..615bb475 100644
--- a/tests/i915/gem_create.c
+++ b/tests/i915/gem_create.c
@@ -62,6 +62,18 @@ IGT_TEST_DESCRIPTION("This is a test for the extended & old
gem_create ioctl,"
" that includes allocation of object from stolen memory"
" and shmem.");
+#if !HAVE_BUILTIN_ATOMICS
+int __sync_add_and_fetch(void *ptr, uint64_t val)
+{
+ igt_assert_f(false, "Don't have builtin atomics\n");
+}
+
+int __sync_val_compare_and_swap(void *ptr, uint64_t old, uint64_t new)
+{
+ igt_assert_f(false, "Don't have builtin atomics\n");
+}
+#endif
+
#define CLEAR(s) memset(&s, 0, sizeof(s))
#define PAGE_SIZE 4096
___
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 series starting with [1/4] drm/i915: Enable transition watermarks for glk
On Mon, Feb 04, 2019 at 07:38:33PM +0200, Ville Syrjälä wrote: > On Thu, Jan 31, 2019 at 08:07:35PM -, Patchwork wrote: > > == Series Details == > > > > Series: series starting with [1/4] drm/i915: Enable transition watermarks > > for glk > > URL : https://patchwork.freedesktop.org/series/56025/ > > State : failure > > > > == Summary == > > > > CI Bug Log - changes from CI_DRM_5518_full -> Patchwork_12103_full > > > > > > Summary > > --- > > > > **FAILURE** > > > > Serious unknown changes coming with Patchwork_12103_full absolutely need > > to be > > verified manually. > > > > If you think the reported changes have nothing to do with the changes > > introduced in Patchwork_12103_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_12103_full: > > > > ### IGT changes ### > > > > Possible regressions > > > > * igt@kms_cursor_legacy@2x-cursor-vs-flip-legacy: > > - shard-glk: PASS -> FAIL +2 > > (kms_cursor_legacy:2984) CRITICAL: Test assertion failure function > two_screens_cursor_vs_flip, file ../tests/kms_cursor_legacy.c:1207: > (kms_cursor_legacy:2984) CRITICAL: Failed assertion: shared[child] > > vrefresh[child]*target[child] / 2 > (kms_cursor_legacy:2984) CRITICAL: completed 382 cursor updated in a period > of 30 flips, we expect to complete approximately 3840 updates, with the > threshold set at 1920 > Subtest 2x-cursor-vs-flip-legacy failed. > > 2x-cursor-vs-flip-atomic and 2x-long-cursor-vs-flip-atomic also flipped > to fail but that fact is not reflected here for some reason. It is, but obscurely. It's the "+2". Two more of the similar failure. -- 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] kms_fence_pin_leak: Move beneath i915/
On Fri, Feb 08, 2019 at 02:44:58PM +, Chris Wilson wrote:
> kms_fence_pin_leak tests smooth sharp edges that are i915 specific (and
> requires using GEM to do so). It doesn't belong in the general paddock
> of all driver tests, so move it into the i915/ stable.
>
> Signed-off-by: Chris Wilson
> Cc: Arkadiusz Hiler
> Cc: Petri Latvala
Acked-by: Petri Latvala
> ---
> tests/Makefile.sources| 5 -
> tests/{ => i915}/kms_fence_pin_leak.c | 0
> tests/meson.build | 2 +-
> 3 files changed, 5 insertions(+), 2 deletions(-)
> rename tests/{ => i915}/kms_fence_pin_leak.c (100%)
>
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index a234fa5dd..8565f100a 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -40,7 +40,6 @@ TESTS_progs = \
> kms_dp_dsc \
> kms_draw_crc \
> kms_fbcon_fbt \
> - kms_fence_pin_leak \
> kms_flip \
> kms_flip_event_leak \
> kms_flip_tiling \
> @@ -98,6 +97,10 @@ TESTS_progs = \
> vgem_slow \
> $(NULL)
>
> +TESTS_progs += \
> + i915/kms_fence_pin_leak \
> + $(NULL)
> +
> TESTS_progs += gem_bad_reloc
> gem_bad_reloc_SOURCES = i915/gem_bad_reloc.c
>
> diff --git a/tests/kms_fence_pin_leak.c b/tests/i915/kms_fence_pin_leak.c
> similarity index 100%
> rename from tests/kms_fence_pin_leak.c
> rename to tests/i915/kms_fence_pin_leak.c
> diff --git a/tests/meson.build b/tests/meson.build
> index 0f12df26d..1c4f9ec36 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -27,7 +27,6 @@ test_progs = [
> 'kms_dp_dsc',
> 'kms_draw_crc',
> 'kms_fbcon_fbt',
> - 'kms_fence_pin_leak',
> 'kms_flip',
> 'kms_flip_event_leak',
> 'kms_flip_tiling',
> @@ -99,6 +98,7 @@ i915_progs = [
> 'fb_tiling',
> 'getparams_basic',
> 'hangman',
> + 'kms_fence_pin_leak',
> 'missed_irq',
> 'module_load',
> 'query',
> --
> 2.20.1
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] intel-ci: Ignore performance-only pread/pwrite "tests"
On Fri, Feb 15, 2019 at 10:41:58AM +, Chris Wilson wrote: > We don't need to waste time running perf-only test cases when we are not > manually checking results. ezbench is that away! > > References: https://bugs.freedesktop.org/show_bug.cgi?id=109640 > Signed-off-by: Chris Wilson > --- Remove those subtests, put them in benchmarks/ along with an ezbench integration script? Acked-by: Petri Latvala > tests/intel-ci/blacklist.txt | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tests/intel-ci/blacklist.txt b/tests/intel-ci/blacklist.txt > index 0e6beeae4..2bdc0b25b 100644 > --- a/tests/intel-ci/blacklist.txt > +++ b/tests/intel-ci/blacklist.txt > @@ -50,6 +50,7 @@ igt@gem_mmap@.*(swap|huge).* > igt@gem_mocs_settings@.*(suspend|hibernate).* > igt@gem_pin(@.*)? > igt@gem_pread_after_blit(@.*)? > +igt@gem_pwrite_pread@.*performance.* > igt@gem_read_read_speed(@.*)? > igt@gem_reloc_overflow(@.*)? > igt@gem_reloc_vs_gpu(@.*)? > -- > 2.20.1 > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 4/7] kms_flip: Remove unreachable condition in wait_for_events
On Mon, Mar 04, 2019 at 12:30:55PM -0300, Rodrigo Siqueira wrote: > In the function wait_for_events() has a double check in the select() > function return as described below: > > igt_assert_f(ret >= 0, > "select error (errno %i)\n", errno); > igt_assert_f(ret > 0, > "select timed out or error (ret %d)\n", ret); > > Note that the second assert condition will not be reached because of the > first assertion. This commit removes the code duplication and update the > error message. It will be reached if ret equals 0. If select() returns 0, errno is unspecified, the main reason for the separation. -- 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] Add support for forcing specific module
On Mon, Dec 17, 2018 at 09:22:31AM -0800, Lucas De Marchi wrote: > On Mon, Dec 17, 2018 at 7:49 AM Daniel Vetter wrote: > > > > On Wed, Sep 05, 2018 at 05:38:27PM -0300, Rodrigo Siqueira wrote: > > > This commit adds a new option for forcing the use of a specific driver > > > indicated via an environment variable. > > > > > > Changes since V1: > > > Petri: > > > - Use an environment variable instead of command line > > > - Refactor the loop in __search_and_open to accept forced module > > > - Don't try to load kernel modules > > > > > > Signed-off-by: Rodrigo Siqueira > > > > Note: You can't drop the s-o-b line if your patch contains work by other > > people, like from Petri here. Proper way to resend a patch by someone else > > is to just add a subject prefix of "PATCH RESEND" and otherwise keep > > everything unchanged (including author and everything). > > > > https://patchwork.freedesktop.org/patch/245532/ > > Last time I was told I have to _add_ my s-o-b nonetheless, even if > just re-sending the patch. > I don't think I should, but in the end I had to change the series, add > and change patches, > so it didn't matter. Communication error here? Rodrigo's resend didn't have my S-o-b, that's what Daniel was pointing at. Removing S-o-b is never ok. Whether it's correct and/or required to add your own S-o-b to resends is another matter. > Maybe we need some clarification on this? > > https://lists.freedesktop.org/archives/intel-gfx/2018-November/183291.html That was about a kernel patch, and kernel patches are _very_ strict about having to add your S-o-b. -- 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/gem_shrink: Exercise OOM and other routes to shrinking in reasonable time
On Fri, Jan 04, 2019 at 03:37:09PM +, Tvrtko Ursulin wrote: > diff --git a/tests/intel-ci/blacklist.txt b/tests/intel-ci/blacklist.txt > index 73d127603d28..d76a4b3b1c71 100644 > --- a/tests/intel-ci/blacklist.txt > +++ b/tests/intel-ci/blacklist.txt > @@ -60,6 +60,7 @@ igt@gem_ring_sync_copy(@.*)? > igt@gem_ring_sync_loop(@.*)? > igt@gem_seqno_wrap(@.*)? > igt@gem_shrink@(?!reclaim$).* > +igt@gem_shrink@(?!reclaims-and-oom).* > igt@gem_softpin@.*(hang|S4).* > igt@gem_spin_batch(@.*)? > igt@gem_stolen@.*hibernate.* In case you didn't notice: The first gem_shrink line removes the reclaims-and-oom* subtests, and the second line (the one you added) then removes the 'reclaim' subtest, resulting in all gem_shrink subtests blacklisted. -- 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/gem_shrink: Exercise OOM and other routes to shrinking in reasonable time
On Mon, Jan 07, 2019 at 11:12:30AM +, Tvrtko Ursulin wrote: > > On 07/01/2019 11:01, Petri Latvala wrote: > > On Fri, Jan 04, 2019 at 03:37:09PM +, Tvrtko Ursulin wrote: > > > diff --git a/tests/intel-ci/blacklist.txt b/tests/intel-ci/blacklist.txt > > > index 73d127603d28..d76a4b3b1c71 100644 > > > --- a/tests/intel-ci/blacklist.txt > > > +++ b/tests/intel-ci/blacklist.txt > > > @@ -60,6 +60,7 @@ igt@gem_ring_sync_copy(@.*)? > > > igt@gem_ring_sync_loop(@.*)? > > > igt@gem_seqno_wrap(@.*)? > > > igt@gem_shrink@(?!reclaim$).* > > > +igt@gem_shrink@(?!reclaims-and-oom).* > > > igt@gem_softpin@.*(hang|S4).* > > > igt@gem_spin_batch(@.*)? > > > igt@gem_stolen@.*hibernate.* > > > > > > In case you didn't notice: The first gem_shrink line removes the > > reclaims-and-oom* subtests, and the second line (the one you added) > > then removes the 'reclaim' subtest, resulting in all gem_shrink > > subtests blacklisted. > > No I haven't thank you. Interestingly the reclaim subtest still did run. https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2185/shards-all.html it's NOTRUN on all machines on patched IGT. > Okay, but in essence should this work then: > > igt@gem_shrink@(?!reclaim).* > > To blacklist everything apart from reclaims and reclaims-and-oom.* > subtests? Yeah that looks correct. -- 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] tools/l3_parity: Unnest exit handlers
On Sat, Sep 07, 2019 at 07:12:56PM +0100, Chris Wilson wrote:
> The curse of using libigt where it is not wanted; in this case calling
> drop-caches while we hold the forcewake is a recipe for a long wait.
>
> Signed-off-by: Chris Wilson
For the series:
Reviewed-by: Petri Latvala
> ---
> tools/intel_l3_parity.c | 50 -
> 1 file changed, 29 insertions(+), 21 deletions(-)
>
> diff --git a/tools/intel_l3_parity.c b/tools/intel_l3_parity.c
> index 06a185c91..340f94b1a 100644
> --- a/tools/intel_l3_parity.c
> +++ b/tools/intel_l3_parity.c
> @@ -180,6 +180,7 @@ int main(int argc, char *argv[])
> const char *path[REAL_MAX_SLICES] = {"l3_parity", "l3_parity_slice_1"};
> int row = 0, bank = 0, sbank = 0;
> int fd[REAL_MAX_SLICES] = {0}, ret, i;
> + int exitcode = EXIT_FAILURE;
> int action = '0';
> int daemonize = 0;
> int device, dir;
> @@ -198,13 +199,13 @@ int main(int argc, char *argv[])
> fd[i] = openat(dir, path[i], O_RDWR);
> if (fd[i] < 0) {
> if (i == 0) /* at least one slice must be supported */
> - exit(77);
> + goto skip;
> continue;
> }
>
> if (read(fd[i], l3logs[i], NUM_REGS * sizeof(uint32_t)) < 0) {
> perror(path[i]);
> - exit(77);
> + goto skip;
> }
> assert(lseek(fd[i], 0, SEEK_SET) == 0);
> }
> @@ -252,45 +253,45 @@ int main(int argc, char *argv[])
> case '?':
> case 'h':
> usage(argv[0]);
> - exit(EXIT_SUCCESS);
> + goto success;
> case 'H':
> printf("Number of slices: %d\n", MAX_SLICES);
> printf("Number of banks: %d\n", num_banks());
> printf("Subbanks per bank: %d\n", NUM_SUBBANKS);
> printf("Max L3 size: %dK\n", L3_SIZE >> 10);
> printf("Has error injection: %s\n",
> IS_HASWELL(devid) ? "yes" : "no");
> - exit(EXIT_SUCCESS);
> + goto success;
> case 'r':
> row = atoi(optarg);
> if (row >= MAX_ROW)
> - exit(EXIT_FAILURE);
> + goto failure;
> break;
> case 'b':
> bank = atoi(optarg);
> if (bank >= num_banks() || bank >=
> MAX_BANKS_PER_SLICE)
> - exit(EXIT_FAILURE);
> + goto failure;
> break;
> case 's':
> sbank = atoi(optarg);
> if (sbank >= NUM_SUBBANKS)
> - exit(EXIT_FAILURE);
> + goto failure;
> break;
> case 'w':
> which_slice = atoi(optarg);
> if (which_slice >= MAX_SLICES)
> - exit(EXIT_FAILURE);
> + goto failure;
> break;
> case 'i':
> case 'u':
> if (!IS_HASWELL(devid)) {
> fprintf(stderr, "Error injection
> supported on HSW+ only\n");
> - exit(EXIT_FAILURE);
> + goto failure;
> }
> case 'd':
> if (optarg) {
> ret = sscanf(optarg, "%d,%d,%d", &row,
> &bank, &sbank);
> if (ret != 3)
> - exit(EXIT_FAILURE);
> + goto failure;
> }
> case '
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] igt/kms_frontbuffer_tracking: Skip over IGT_DRAW_BLT when there's no BLT
On Sun, Sep 15, 2019 at 10:39:48AM +0100, Chris Wilson wrote:
> If the blitter is not available, we cannot use it as a source for dirty
> rectangles. We shall have to rely on the other engines to create GPU
> dirty instead.
>
> v2: Try using lots of subgroup+fixtures
>
> Signed-off-by: Chris Wilson
Reviewed-by: Petri Latvala
> ---
> tests/kms_frontbuffer_tracking.c | 58 ++--
> 1 file changed, 56 insertions(+), 2 deletions(-)
>
> diff --git a/tests/kms_frontbuffer_tracking.c
> b/tests/kms_frontbuffer_tracking.c
> index c788b59ee..eaa4b6ef1 100644
> --- a/tests/kms_frontbuffer_tracking.c
> +++ b/tests/kms_frontbuffer_tracking.c
> @@ -3022,6 +3022,9 @@ static void basic_subtest(const struct test_mode *t)
> fb1 = params->primary.fb;
>
> for (r = 0, method = 0; method < IGT_DRAW_METHOD_COUNT; method++, r++) {
> + if (method == IGT_DRAW_BLT && !gem_has_blitter(drm.fd))
> + continue;
> +
> if (r == pattern->n_rects) {
> params->primary.fb = (params->primary.fb == fb1) ? &fb2
> : fb1;
>
> @@ -3248,10 +3251,11 @@ static const char *flip_str(enum flip_type flip)
> continue; \
> if (!opt.show_hidden && t.fbs == FBS_SHARED && \
> (t.plane == PLANE_CUR || t.plane == PLANE_SPR))\
> - continue;
> + continue; \
> + igt_subtest_group {
>
>
> -#define TEST_MODE_ITER_END } } } } } }
> +#define TEST_MODE_ITER_END } } } } } } }
>
> struct option long_options[] = {
> { "no-status-check", 0, 0, 's'},
> @@ -3297,6 +3301,10 @@ igt_main_args("", long_options, help_str, opt_handler,
> NULL)
> }
>
> TEST_MODE_ITER_BEGIN(t)
> + igt_fixture {
> + if (t.method == IGT_DRAW_BLT)
> + gem_require_blitter(drm.fd);
> + }
> igt_subtest_f("%s-%s-%s-%s-%s-draw-%s",
> feature_str(t.feature),
> pipes_str(t.pipes),
> @@ -3313,6 +3321,11 @@ igt_main_args("", long_options, help_str, opt_handler,
> NULL)
> (!opt.show_hidden && t.method != IGT_DRAW_BLT))
> continue;
>
> + igt_fixture {
> + if (t.method == IGT_DRAW_BLT)
> + gem_require_blitter(drm.fd);
> + }
> +
> for (t.flip = 0; t.flip < FLIP_COUNT; t.flip++)
> igt_subtest_f("%s-%s-%s-%s-%sflip-%s",
> feature_str(t.feature),
> @@ -3331,6 +3344,11 @@ igt_main_args("", long_options, help_str, opt_handler,
> NULL)
> (t.feature & FEATURE_FBC) == 0)
> continue;
>
> + igt_fixture {
> + if (t.method == IGT_DRAW_BLT)
> + gem_require_blitter(drm.fd);
> + }
> +
> igt_subtest_f("%s-%s-%s-fliptrack",
> feature_str(t.feature),
> pipes_str(t.pipes),
> @@ -3344,6 +3362,11 @@ igt_main_args("", long_options, help_str, opt_handler,
> NULL)
> t.plane == PLANE_PRI)
> continue;
>
> + igt_fixture {
> + if (t.method == IGT_DRAW_BLT)
> + gem_require_blitter(drm.fd);
> + }
> +
> igt_subtest_f("%s-%s-%s-%s-%s-move",
> feature_str(t.feature),
> pipes_str(t.pipes),
> @@ -3367,6 +3390,11 @@ igt_main_args("", long_options, help_str, opt_handler,
> NULL)
> t.plane != PLANE_SPR)
> continue;
>
> + igt_fixture {
> + if (t.method == IGT_DRAW_BLT)
> + gem_require_blitter(drm.fd);
> + }
> +
> igt_subtest_f("%s-%s-%s-%s-%s-fullscreen",
> feature_str(t.feature),
> pipes_str(t.pipes),
> @@ -3383,6 +3411,11 @@ igt_main_args("", long_options, help_str, opt_handler,
> NULL)
> (!opt.show_hidden && t.fbs != FBS_INDIVIDUAL))
> continue;
>
>
Re: [Intel-gfx] [igt-dev] [PATCH] test/kms_content_protection: Use generic debugfs name for HDCP caps
On Mon, Sep 23, 2019 at 02:23:25PM -0400, Bhawanpreet Lakha wrote: > Rename "i915_hdcp_sink_capability" to "hdcp_sink_capability" > > The content protection tests only start if this debugfs entry exists. > Since the name is specific to intel driver these tests cannot be used with > other drivers. > > Remove "i915" so the debugfs name is generic. I don't see any drivers currently using the name "hdcp_sink_capability". Is the content of the file in other drivers the same as i915's? And is there a plan to rename i915's debugfs filename to hdcp_sink_capability? Ram? Agreed, for a DRIVER_ANY-using test generic names are preferrable, but this needs some kind of a transition plan. If i915's filename is staying as is, both names need to be tried. -- Petri Latvala ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 00/17] GuC 32.0.3
On Tue, May 28, 2019 at 10:23:24AM +0100, Chris Wilson wrote: > Quoting Michal Wajdeczko (2019-05-27 19:35:56) > > New GuC firmwares (for SKL, BXT, KBL, GLK, ICL) with updated ABI interface. > > All reviewed/acked, and I trust the failures will be fixed in time, so > pushed. Thanks. > -Chris But without telling Martin to file cibuglog filters for known issues? -- 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] prime_vgem: Fix typo in checking for invalid engines
On Mon, May 27, 2019 at 09:36:16AM +0100, Chris Wilson wrote:
> Move the stray ')' from
>
> gem_can_store_dword(exec_id) | exec_flags
>
> to
>
> gem_can_store_dword(exec_id | exec_flags)
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110764
> Signed-off-by: Chris Wilson
Reviewed-by: Petri Latvala
> ---
> tests/prime_vgem.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/tests/prime_vgem.c b/tests/prime_vgem.c
> index 09e3373be..69ae8c9b0 100644
> --- a/tests/prime_vgem.c
> +++ b/tests/prime_vgem.c
> @@ -845,7 +845,7 @@ igt_main
> e->exec_id == 0 ? "basic-" : "",
> e->name) {
> gem_require_ring(i915, e->exec_id | e->flags);
> - igt_require(gem_can_store_dword(i915, e->exec_id) |
> e->flags);
> + igt_require(gem_can_store_dword(i915, e->exec_id |
> e->flags));
>
> gem_quiescent_gpu(i915);
> test_sync(i915, vgem, e->exec_id, e->flags);
> @@ -857,7 +857,7 @@ igt_main
> e->exec_id == 0 ? "basic-" : "",
> e->name) {
> gem_require_ring(i915, e->exec_id | e->flags);
> - igt_require(gem_can_store_dword(i915, e->exec_id) |
> e->flags);
> + igt_require(gem_can_store_dword(i915, e->exec_id |
> e->flags));
>
> gem_quiescent_gpu(i915);
> test_busy(i915, vgem, e->exec_id, e->flags);
> @@ -869,7 +869,7 @@ igt_main
> e->exec_id == 0 ? "basic-" : "",
> e->name) {
> gem_require_ring(i915, e->exec_id | e->flags);
> - igt_require(gem_can_store_dword(i915, e->exec_id) |
> e->flags);
> + igt_require(gem_can_store_dword(i915, e->exec_id |
> e->flags));
>
> gem_quiescent_gpu(i915);
> test_wait(i915, vgem, e->exec_id, e->flags);
> @@ -892,7 +892,7 @@ igt_main
> e->exec_id == 0 ? "basic-" : "",
> e->name) {
> gem_require_ring(i915, e->exec_id | e->flags);
> - igt_require(gem_can_store_dword(i915,
> e->exec_id) | e->flags);
> + igt_require(gem_can_store_dword(i915,
> e->exec_id | e->flags));
>
> gem_quiescent_gpu(i915);
> test_fence_wait(i915, vgem, e->exec_id,
> e->flags);
> --
> 2.20.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 1/5] lib/tests: fix conflicting args test
On Wed, May 29, 2019 at 04:27:33PM -0700, Lucas De Marchi wrote:
> We want to check if the long option conflicts with one from the core.
> The check for conflicting short option already exists just above.
No, this one is checking that the val (the 0) doesn't conflict.
--
Petri Latvala
>
> Signed-off-by: Lucas De Marchi
> ---
> lib/tests/igt_conflicting_args.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/tests/igt_conflicting_args.c
> b/lib/tests/igt_conflicting_args.c
> index c357b6c5..d8be138e 100644
> --- a/lib/tests/igt_conflicting_args.c
> +++ b/lib/tests/igt_conflicting_args.c
> @@ -91,7 +91,7 @@ int main(int argc, char **argv)
> internal_assert_wsignaled(do_fork(), SIGABRT);
>
> /* conflict on long option 'val' representations */
> - long_options[0] = (struct option) { "iterations", required_argument,
> NULL, 0};
> + long_options[0] = (struct option) { "list-subtests", required_argument,
> NULL, 0};
> short_options = "";
> internal_assert_wsignaled(do_fork(), SIGABRT);
>
> --
> 2.21.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 i-g-t 2/5] lib/igt_core: reserve long options for individual tests
On Wed, May 29, 2019 at 04:27:34PM -0700, Lucas De Marchi wrote:
> Start the core optiosn from 500 so the individual tests can have their
> own options starting from 0. This makes it easier to set the long
> options without conflicting.
>
> 500 is just a magic number, higher than any ascii char that could be
> used in the individual test.
>
> While at it, fix the coding style to use tab rather than space.
>
> Signed-off-by: Lucas De Marchi
This is much better than requiring additional opts to begin from a particular
number.
Reviewed-by: Petri Latvala
> ---
> lib/igt_core.c | 16 ++--
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index 9c86d664..814f5c72 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -280,12 +280,16 @@ int test_children_sz;
> bool test_child;
>
> enum {
> - OPT_LIST_SUBTESTS,
> - OPT_RUN_SUBTEST,
> - OPT_DESCRIPTION,
> - OPT_DEBUG,
> - OPT_INTERACTIVE_DEBUG,
> - OPT_HELP = 'h'
> + /*
> + * Let the first values be used by individual tests so options don't
> + * conflict with core ones
> + */
> + OPT_LIST_SUBTESTS = 500,
> + OPT_RUN_SUBTEST,
> + OPT_DESCRIPTION,
> + OPT_DEBUG,
> + OPT_INTERACTIVE_DEBUG,
> + OPT_HELP = 'h'
> };
>
> static int igt_exitcode = IGT_EXIT_SUCCESS;
> --
> 2.21.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 i-g-t 3/5] lib/igt_core: 0 is a valid val for long options
On Wed, May 29, 2019 at 04:27:35PM -0700, Lucas De Marchi wrote:
> This is usually used by long options when working with enum to set long
> option values. So replace the strchr() with a memchr() to take that into
> account.
>
> Signed-off-by: Lucas De Marchi
Reviewed-by: Petri Latvala
The latter strchr-memchr change is needless but meh.
> ---
> lib/igt_core.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index 814f5c72..a0b7e581 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -680,6 +680,7 @@ static int common_init(int *argc, char **argv,
> };
> char *short_opts;
> const char *std_short_opts = "h";
> + size_t std_short_opts_len = strlen(std_short_opts);
> struct option *combined_opts;
> int extra_opt_count;
> int all_opt_count;
> @@ -713,7 +714,7 @@ static int common_init(int *argc, char **argv,
>
> /* check for conflicts with standard short options */
> if (extra_long_opts[extra_opt_count].val != ':'
> - && (conflicting_char = strchr(std_short_opts,
> extra_long_opts[extra_opt_count].val))) {
> + && (conflicting_char = memchr(std_short_opts,
> extra_long_opts[extra_opt_count].val, std_short_opts_len))) {
> igt_critical("Conflicting long and short option 'val'
> representation between --%s and -%c\n",
>extra_long_opts[extra_opt_count].name,
>*conflicting_char);
> @@ -727,7 +728,7 @@ static int common_init(int *argc, char **argv,
> continue;
>
> /* check for conflicts with standard short options */
> - if (strchr(std_short_opts, extra_short_opts[i])) {
> + if (memchr(std_short_opts, extra_short_opts[i],
> std_short_opts_len)) {
> igt_critical("Conflicting short option: -%c\n",
> std_short_opts[i]);
> assert(0);
> }
> --
> 2.21.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 i-g-t 4/5] testdisplay: use first available option values
On Wed, May 29, 2019 at 04:27:36PM -0700, Lucas De Marchi wrote:
> Now that core options are set to 500 and above, start from the lowest
> values without causing problems with conflicts. This also rename the
> constants to follow the names from the core.
>
> Signed-off-by: Lucas De Marchi
Reviewed-by: Petri Latvala
> ---
> tests/testdisplay.c | 14 --
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/tests/testdisplay.c b/tests/testdisplay.c
> index b4f0d45f..32590547 100644
> --- a/tests/testdisplay.c
> +++ b/tests/testdisplay.c
> @@ -69,8 +69,10 @@
> #include
> #include
>
> -#define Yb_OPT5
> -#define Yf_OPT6
> +enum {
> + OPT_YB,
> + OPT_YF,
> +};
>
> static int tio_fd;
> struct termios saved_tio;
> @@ -573,8 +575,8 @@ static void set_termio_mode(void)
>
> static char optstr[] = "3iaf:s:d:p:mrto:j:y";
> static struct option long_opts[] = {
> - {"yb", 0, 0, Yb_OPT},
> - {"yf", 0, 0, Yf_OPT},
> + {"yb", 0, 0, OPT_YB},
> + {"yf", 0, 0, OPT_YF},
> { 0, 0, 0, 0 }
> };
>
> @@ -648,10 +650,10 @@ static int opt_handler(int opt, int opt_index, void
> *data)
> tiling = LOCAL_I915_FORMAT_MOD_X_TILED;
> break;
> case 'y':
> - case Yb_OPT:
> + case OPT_YB:
> tiling = LOCAL_I915_FORMAT_MOD_Y_TILED;
> break;
> - case Yf_OPT:
> + case OPT_YF:
> tiling = LOCAL_I915_FORMAT_MOD_Yf_TILED;
> break;
> case 'r':
> --
> 2.21.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 i-g-t 5/5] lib/igt_core: add -h to usage
On Wed, May 29, 2019 at 04:27:37PM -0700, Lucas De Marchi wrote: > We also accept the short option -h. > > Signed-off-by: Lucas De Marchi Reviewed-by: Petri Latvala Please send (CC or otherwise) IGT patches to igt-dev in the future, please. > --- > lib/igt_core.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/lib/igt_core.c b/lib/igt_core.c > index a0b7e581..6b9f0425 100644 > --- a/lib/igt_core.c > +++ b/lib/igt_core.c > @@ -558,7 +558,7 @@ static void print_usage(const char *help_str, bool > output_on_stderr) > " --debug[=log-domain]\n" > " --interactive-debug[=domain]\n" > " --help-description\n" > -" --help\n"); > +" --help|-h\n"); > if (help_str) > fprintf(f, "%s\n", help_str); > } > -- > 2.21.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] [igt-dev] [PATCH i-g-t] lib: Fix intel_get_current_physical_engine() iterator
On Mon, Jun 03, 2019 at 11:19:48AM +0100, Tvrtko Ursulin wrote: > > On 29/05/2019 14:24, Chris Wilson wrote: > > If we run out of engines, intel_get_current_physical_engine() degrades > > into an infinite loop as although it advanced the iterator, it did not > > update its local engine pointer. > > We had one infinite loop in there already.. AFAIR it was on one engine > platforms. Does the new incarnation happen actually via the > __for_each_physical_engine iterator or perhaps only when calling > intel_get_current_physical_engine after loop end? Why it wasn't seen in > testing? The new incarnation happens with a wedged GPU. That's a case that's hard to come by 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] lib: Fix intel_get_current_physical_engine() iterator
On Mon, Jun 03, 2019 at 11:39:25AM +0100, Tvrtko Ursulin wrote: > > On 03/06/2019 11:32, Petri Latvala wrote: > > On Mon, Jun 03, 2019 at 11:19:48AM +0100, Tvrtko Ursulin wrote: > > > > > > On 29/05/2019 14:24, Chris Wilson wrote: > > > > If we run out of engines, intel_get_current_physical_engine() degrades > > > > into an infinite loop as although it advanced the iterator, it did not > > > > update its local engine pointer. > > > > > > We had one infinite loop in there already.. AFAIR it was on one engine > > > platforms. Does the new incarnation happen actually via the > > > __for_each_physical_engine iterator or perhaps only when calling > > > intel_get_current_physical_engine after loop end? Why it wasn't seen in > > > testing? > > > > > > The new incarnation happens with a wedged GPU. That's a case that's > > hard to come by in testing. > > 1. > Colour me confused. :) How does a wedged GPU affect this loop? Wedging could be a red herring, but regardless the GPU was in a funky state. An easy reproduction method is just # ./perf_pmu (as normal user, not root!) -- 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 1/5] lib/tests: fix conflicting args test
On 6/5/19 12:38 AM, Lucas De Marchi wrote: On Fri, May 31, 2019 at 07:55:45AM -0700, Lucas De Marchi wrote: On Fri, May 31, 2019 at 12:59:35PM +0300, Petri Latvala wrote: On Wed, May 29, 2019 at 04:27:33PM -0700, Lucas De Marchi wrote: We want to check if the long option conflicts with one from the core. The check for conflicting short option already exists just above. No, this one is checking that the val (the 0) doesn't conflict. My point is that this check is already done above. We don't need to do it again. There's two val conflict tests. One checks for conflicts against core short options, the latter (modified here) checks for conflicts against core long option values. If you insist, then we will need to raise it to magic number 500, because 0 won't be a conflict after this series. Yeah that would be the correct change. Petri ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t v11 1/1] tests: Add a new test for device hot unplug
On Fri, Jun 07, 2019 at 01:51:42PM +0200, Janusz Krzysztofik wrote: > - use SPDX license identifier, Why? We don't use those in IGT. > diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c > new file mode 100644 > index ..d36a0572 > --- /dev/null > +++ b/tests/core_hotunplug.c > @@ -0,0 +1,222 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright © 2019 Intel Corporation > + */ And why GPL-2.0? -- 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 2/4] gitlab-ci: add libatomic to docker images
On Thu, Jun 13, 2019 at 02:53:20PM +0100, Guillaume Tucker wrote: > Add libatomic to the Fedora docker image so it can link binaries that > use __atomic_* functions. Also explicitly add libatomic1 to Debian > docker images even though it's already installed as a dependency. > > Signed-off-by: Guillaume Tucker > --- > Dockerfile.debian | 1 + > Dockerfile.debian-arm64 | 1 + > Dockerfile.debian-armhf | 1 + > Dockerfile.fedora | 2 +- > 4 files changed, 4 insertions(+), 1 deletion(-) > > diff --git a/Dockerfile.debian b/Dockerfile.debian > index b9c3be3945e0..d23591850c4e 100644 > --- a/Dockerfile.debian > +++ b/Dockerfile.debian > @@ -6,6 +6,7 @@ RUN apt-get install -y \ > flex \ > bison \ > pkg-config \ > + libatomic1 \ > libpciaccess-dev \ > libkmod-dev \ > libprocps-dev \ > diff --git a/Dockerfile.debian-arm64 b/Dockerfile.debian-arm64 > index 7b3a3c7ca803..003bb22b3215 100644 > --- a/Dockerfile.debian-arm64 > +++ b/Dockerfile.debian-arm64 > @@ -5,6 +5,7 @@ RUN apt-get install -y \ > flex \ > bison \ > pkg-config \ > + libatomic1 \ > x11proto-dri2-dev \ > python-docutils \ > valgrind \ > diff --git a/Dockerfile.debian-armhf b/Dockerfile.debian-armhf > index c67a1e2acf6a..3139927c193a 100644 > --- a/Dockerfile.debian-armhf > +++ b/Dockerfile.debian-armhf > @@ -5,6 +5,7 @@ RUN apt-get install -y \ > flex \ > bison \ > pkg-config \ > + libatomic1 \ libatomic1 is the runtime lib, for linking you need the package that contains libatomic.so. That is *quick search* libgcc-$version-dev. There doesn't seem to be a generic metapackage for "the latest libgcc-dev", other than... 'gcc'. Since Debian is acting a bit speshul here I'd just drop the explicit libatomic installation. > x11proto-dri2-dev \ > python-docutils \ > valgrind \ > diff --git a/Dockerfile.fedora b/Dockerfile.fedora > index 6686e587613d..c84b412b0723 100644 > --- a/Dockerfile.fedora > +++ b/Dockerfile.fedora > @@ -1,7 +1,7 @@ > FROM fedora:30 > > RUN dnf install -y \ > - gcc flex bison meson ninja-build xdotool \ > + gcc flex bison libatomic meson ninja-build xdotool \ Possibly the same comment on fedora but I'm not aware of how they split their gcc package. Anyway, the file to check for is 'libatomic.so'. -- 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] gitlab-ci: add build for MIPS
On Thu, Jun 13, 2019 at 03:01:06PM +0100, Guillaume Tucker wrote: > Add Docker image and Gitlab CI steps to run builds for the MIPS > architecture using Debian Stretch with backports. > > Signed-off-by: Guillaume Tucker Same comment on libatomic1 as in the other thread applies here. -- 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 2/4] gitlab-ci: add libatomic to docker images
On Fri, Jun 14, 2019 at 02:24:53PM +0300, Ser, Simon wrote: > On Fri, 2019-06-14 at 13:00 +0300, Petri Latvala wrote: > > On Thu, Jun 13, 2019 at 02:53:20PM +0100, Guillaume Tucker wrote: > > > Add libatomic to the Fedora docker image so it can link binaries that > > > use __atomic_* functions. Also explicitly add libatomic1 to Debian > > > docker images even though it's already installed as a dependency. > > > > > > Signed-off-by: Guillaume Tucker > > > --- > > > Dockerfile.debian | 1 + > > > Dockerfile.debian-arm64 | 1 + > > > Dockerfile.debian-armhf | 1 + > > > Dockerfile.fedora | 2 +- > > > 4 files changed, 4 insertions(+), 1 deletion(-) > > > > > > diff --git a/Dockerfile.debian b/Dockerfile.debian > > > index b9c3be3945e0..d23591850c4e 100644 > > > --- a/Dockerfile.debian > > > +++ b/Dockerfile.debian > > > @@ -6,6 +6,7 @@ RUN apt-get install -y \ > > > flex \ > > > bison \ > > > pkg-config \ > > > + libatomic1 \ > > > libpciaccess-dev \ > > > libkmod-dev \ > > > libprocps-dev \ > > > diff --git a/Dockerfile.debian-arm64 b/Dockerfile.debian-arm64 > > > index 7b3a3c7ca803..003bb22b3215 100644 > > > --- a/Dockerfile.debian-arm64 > > > +++ b/Dockerfile.debian-arm64 > > > @@ -5,6 +5,7 @@ RUN apt-get install -y \ > > > flex \ > > > bison \ > > > pkg-config \ > > > + libatomic1 \ > > > x11proto-dri2-dev \ > > > python-docutils \ > > > valgrind \ > > > diff --git a/Dockerfile.debian-armhf b/Dockerfile.debian-armhf > > > index c67a1e2acf6a..3139927c193a 100644 > > > --- a/Dockerfile.debian-armhf > > > +++ b/Dockerfile.debian-armhf > > > @@ -5,6 +5,7 @@ RUN apt-get install -y \ > > > flex \ > > > bison \ > > > pkg-config \ > > > + libatomic1 \ > > > > libatomic1 is the runtime lib, for linking you need the package that > > contains libatomic.so. That is *quick search* > > libgcc-$version-dev. There doesn't seem to be a generic metapackage > > for "the latest libgcc-dev", other than... 'gcc'. > > > > Since Debian is acting a bit speshul here I'd just drop the explicit > > libatomic installation. > > Hmm, I see the .so in libatomic1… > > https://packages.debian.org/jessie/amd64/libatomic1/filelist Where? I only see the .so.1 and .so.1.1.0 -- 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 6/9] tests/i915/query: sanity check the unallocated tracking
le_size <
> +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(&handles);
> +
> + 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(&oh.link, &handles);
> + upload(fd, &handles, 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, &item, 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, &item, 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(&handles);
> +
> + 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 = ran
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, &execbuf);
> if (gem_wait(device, handle, &timeout_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 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 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 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] [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] [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, &info);
> +
> + 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(&card))
> + 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 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 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[] = {
> + &engines->r_gpu,
> + &engines->r_pkg,
> + &engines->imc_reads,
> + &engines->imc_writes,
> + NULL
> + };
> + unsigned int i;
> +
> + for (pmu = &free_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(&card, 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] [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, &igt_devs.filtered, link) {
> > + igt_list_del(&dev->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] [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
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] [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 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(&execbuf, 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;
> > > @@ -
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] ✗ 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 > > > &
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] [PATCH 2/3] drm/i915/dg2: Add initial gt/ctx/engine workarounds
ONTROL_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 = &engine->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, "w
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, &false_key);
> else
> - pthread_setspecific(__vlog_line_continuation, (void*) true);
> + pthread_setspecific(__vlog_line_continuation, &true_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, &key);
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 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, &false_key);
> > > else
> > > - pthread_setspecific(__vlog_line_continuation, (void*) true);
> > > + pthread_setspecific(__vlog_line_continuation, &true_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, &key);
> >
> > 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] [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(&line, 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] [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
>
Re: [Intel-gfx] [PATCH i-g-t] i915/gem_ctx_isolation: Bump support for Tigerlake
On Wed, Oct 02, 2019 at 12:26:48PM +0100, Chris Wilson wrote:
> There's very little variation in non-privileged registers for Tigerlake,
> so we can mostly inherit the set from gen11. There is no whitelist at
> present, so we do not need to add any special registers.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111599
> Signed-off-by: Chris Wilson
> ---
> tests/i915/gem_ctx_isolation.c | 11 ++-
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/tests/i915/gem_ctx_isolation.c b/tests/i915/gem_ctx_isolation.c
> index df1d655ae..819daafc3 100644
> --- a/tests/i915/gem_ctx_isolation.c
> +++ b/tests/i915/gem_ctx_isolation.c
> @@ -55,10 +55,11 @@ enum {
> #define GEN9 (ALL << 9)
> #define GEN10 (ALL << 10)
> #define GEN11 (ALL << 11)
> +#define GEN12 (ALL << 12)
>
> #define NOCTX 0
>
> -#define LAST_KNOWN_GEN 11
> +#define LAST_KNOWN_GEN 12
>
> static const struct named_register {
> const char *name;
> @@ -116,9 +117,9 @@ static const struct named_register {
> { "Cache_Mode_0", GEN7, RCS0, 0x7000, .masked = true },
> { "Cache_Mode_1", GEN7, RCS0, 0x7004, .masked = true },
> { "GT_MODE", GEN8, RCS0, 0x7008, .masked = true },
> - { "L3_Config", GEN8, RCS0, 0x7034 },
> - { "TD_CTL", GEN8, RCS0, 0xe400, .write_mask = 0x },
> - { "TD_CTL2", GEN8, RCS0, 0xe404 },
> + { "L3_Config", GEN_RANGE(8, 11), RCS0, 0x7034 },
> + { "TD_CTL", GEN_RANGE(8, 11), RCS0, 0xe400, .write_mask = 0x },
> + { "TD_CTL2", GEN_RANGE(8, 11), RCS0, 0xe404 },
> { "SO_NUM_PRIMS_WRITTEN0", GEN6, RCS0, 0x5200, 2 },
> { "SO_NUM_PRIMS_WRITTEN1", GEN6, RCS0, 0x5208, 2 },
> { "SO_NUM_PRIMS_WRITTEN2", GEN6, RCS0, 0x5210, 2 },
> @@ -852,7 +853,7 @@ igt_main
> gen = intel_gen(intel_get_drm_devid(fd));
>
> igt_warn_on_f(gen > LAST_KNOWN_GEN,
> - "GEN not recognized! Test needs to be
> updated to run.");
> + "GEN not recognized! Test needs to be updated to
> run.");
> igt_skip_on(gen > LAST_KNOWN_GEN);
Thanks to this editorial change, we're able to see that this string is
missing a newline character.
--
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 series starting with [1/9] drm/i915: Expose 10:10:10 XRGB formats on SNB-BDW sprites (rev2)
x27;s the weird bit... > > Anyway the building machine needs updating and apt-mark hold. > This can cause fallout and we need to file bugs to limit the noise. > > There is quite some queue right now, but hopefully by tomorrow it will > be drained. I'll do the necessary updates and force IGT run to see what > is going to happen in the morning. Then I'll rerun this series. > I don't think the builder ever had a higher version. The fancy YUV formats work because the runtime lib is new enough, and the build-time checks for those are as such: #if CAIRO_VERSION < CAIRO_VERSION_ENCODE(1, 17, 2) /* * We need cairo 1.17.2 to use HDR formats, but the only thing added is a value * to cairo_format_t. * * To prevent going outside the enum, make cairo_format_t an int and define * ourselves. */ #define CAIRO_FORMAT_RGB96F (6) #define CAIRO_FORMAT_RGBA128F (7) #define cairo_format_t int #endif and igt_skip_on_f(status == CAIRO_STATUS_INVALID_FORMAT && cairo_version() < CAIRO_VERSION_ENCODE(1, 17, 2), "Cairo version too old, need 1.17.2, have %s\n", cairo_version_string()); igt_skip_on_f(status == CAIRO_STATUS_NO_MEMORY && pixman_version() < PIXMAN_VERSION_ENCODE(0, 36, 0), "Pixman version too old, need 0.36.0, have %s\n", pixman_version_string()); In other words, the backwards compatibility for the fancy YUV formats at build-time was easy to sneak in by defining some values, is that possible to do with the 10bpc stuff? PIXMAN_rgba_float seems to be just PIXMAN_FORMAT_BYTE(128, PIXMAN_TYPE_RGBA_FLOAT,32,32,32,32), where PIXMAN_TYPE_RGBA_FLOAT is 11. -- Petri Latvala ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [ANNOUNCE] IGT bug reports now at gitlab
IGT bugs in https://bugs.freedesktop.org/ have now been moved to gitlab issues [1]. Many thanks to Martin for the smooth transition! Any new bug reports against IGT must be filed to gitlab instead of bugs.fd.o, as it's closed for new submissions. If you want to be notified of new filed issues, go to the project's main page [2] and customize your notification settings from the bell icon on the top of the page. Signed-off-by: Petri Latvala Signed-off-by: Arkadiusz Hiler [1]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues [2]: https://gitlab.freedesktop.org/drm/igt-gpu-tools ___ 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
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] [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 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] tests/i915/gem_mmap_offset: Add new API test for gem_mmap_offset
t; + PROT_READ | PROT_WRITE,
> + 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(&handles[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, &old);
> +
> + if (ioctl(i915,
> + DRM_IOCTL_I915_GEM_MMAP_OFFSET,
> + &mmap_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, &oldval, 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(&arg->max, npages);
> + create.size = npages << 12;
> +
> + create_ioctl(i915, &create);
> + 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
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] i915: Exercise preemption timeout controls in sysfs
On Fri, Oct 18, 2019 at 01:39:37PM +0100, Tvrtko Ursulin wrote:
>
> On 18/10/2019 13:35, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2019-10-18 13:23:53)
> > >
> > > On 17/10/2019 15:30, Chris Wilson wrote:
> > > > Dynamic subtests!
> > >
> > > Ouch! :)
> > >
> > > > Signed-off-by: Chris Wilson
> > > > ---
> > > > +static void test_timeout(int i915, int engine)
> > > > +{
> > > > + int delays[] = { 1, 50, 100, 500 };
> > > > + unsigned int saved, delay;
> > > > +
> > > > + igt_assert(igt_sysfs_scanf(engine, "preempt_timeout_ms", "%u",
> > > > &saved) == 1);
> > > > + igt_debug("Initial preempt_timeout_ms:%u\n", saved);
> > > > +
> > > > + gem_quiescent_gpu(i915);
> > > > + igt_require(enable_hangcheck(i915, false));
> > > > +
> > > > + for (int i = 0; i < ARRAY_SIZE(delays); i++) {
> > > > + uint64_t elapsed;
> > > > +
> > > > + elapsed = __test_timeout(i915, engine, delays[i]);
> > > > + igt_info("preempt_timeout_ms:%d, elapsed=%.3fms\n",
> > > > + delays[i], elapsed * 1e-6);
> > >
> > > No checking that measured time relates to configured timeout?
> >
> > Have now. Just needed some soaking to decide on thresholds. I've 50ms
> > but that may change as CI tends to have more scheduling intolerance than
> > local machines.
> >
> > > > + }
> > > > +
> > > > + igt_assert(enable_hangcheck(i915, true));
> > > > + gem_quiescent_gpu(i915);
> > > > +
> > > > + igt_sysfs_printf(engine, "preempt_timeout_ms", "%u", saved);
> > > > + igt_sysfs_scanf(engine, "preempt_timeout_ms", "%u", &delay);
> > > > + igt_assert_eq(delay, saved);
> > > > +}
> > > > +
> > > > +igt_main
> > > > +{
> > > > + int i915, sys = -1;
> > > > + struct dirent *de;
> > > > + int engines;
> > > > + DIR *dir;
> > > > +
> > > > + igt_fixture {
> > > > + i915 = drm_open_driver(DRIVER_INTEL);
> > > > + igt_require_gem(i915);
> > > > +
> > > > + sys = igt_sysfs_open(i915);
> > > > + igt_require(sys != -1);
> > >
> > > igt_assert_fd?
> >
> > Do we guarantee that the sysadmin has mounted sysfs? We don't automount
> > it unlike debugfs.
> >
> > > > + igt_subtest_group {
> > > > + igt_fixture {
> > > > + igt_require(fstatat(engine,
> > > > + "preempt_timeout_ms",
> > > > + &st, 0) == 0);
> > > > + }
> > > > +
> > > > + igt_subtest_f("%s-idempotent", name)
> > > > + test_idempotent(i915, engine);
> > > > + igt_subtest_f("%s-invalid", name)
> > > > + test_invalid(i915, engine);
> > > > + igt_subtest_f("%s-timeout", name)
> > > > + test_timeout(i915, engine);
> > > > + }
> > > > +
> > > > + free(name);
> > > > + close(engine);
> > > > + }
> > >
> > > You probably should use __for_each_static_engine and then open sysfs
> > > nodes based on that. Gets around the dynamic subtests no-no at least.
> >
> > Defeatist!
>
> Well I have challenged this status quo a few times and now I am embracing
> it, or should I say disagreeing and committing, so bonus points all round.
> :)
Perhaps next week I'll get around to reshaping the dynamic subtests
series. Watch this space!
(Meanwhile, I hope it goes without saying, dynamic subtests are indeed
a no-no)
--
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] tests/kms_content_protection: check i915 and generic debugfs name for HDCP caps
On Mon, Oct 21, 2019 at 03:42:59PM -0400, Bhawanpreet Lakha wrote: > The content protection tests only start if this debugfs entry exists. > Since the name is specific to intel driver these tests cannot be used with > other drivers. So we should check generic debugfs name also > > v2: Check i915_* if device is i915, otherwise check the generic name. > > Signed-off-by: Bhawanpreet Lakha Merged, thanks for the patch and reviews! -- 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] intel-ci: Relegate gem_exec_reloc to the shards
On Fri, Nov 01, 2019 at 10:56:37AM +, Chris Wilson wrote: > Quoting Chris Wilson (2019-11-01 10:51:06) > > The gem_exec_reloc basic subtests cover a couple dozen basic ABI > > sanitychecks, aiming to prove the ABI works. While relocations used to > > be essential, they are no longer the basis of current clients who all > > softpin. It is a stagnant portion of the ABI that does not contribute > > much to overall driver health (the complicated portions of relocs are > > covered in the smoketests like gem_exec_parallel and gem_exec_gttfill). > > However, even though each of the basic subtest is trivial and runs in > > very little time, since CI is running each individually the time between > > tests mounts up (especially on eMMC devices like fi-kbl-soraka). > > > > By moving these tests to the shards we should speed up BAT on the > > ratelimiting fi-kbl-soraka, while losing no coverage overall -- and > > hopefully without losing any of the predictive failure coverage in BAT. > > This stagnant bit of uAPI/uABI will remain checked by the irregular idle > > runs. > > > > Signed-off-by: Chris Wilson > > Cc: Joonas Lahtinen > > Cc: Tomi Sarvela > > Cc: Petri Latvala > > Cc: Martin Peres > > Before Joonas went fishing, he gave +1. So, > Acked-by: Joonas Lahtinen > > Tomi, Petri, Martin any objections? > -Chris Acked-by: Petri Latvala ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
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] 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 =
> > &engine_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 ==
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] [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] ✗ 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]:
> > https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4307/sha
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] [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] 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 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 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
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] [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] [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(&mbt, 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(&mbt, "rcs0", 0x2000);
> + _gem_engine_mmio_info_legacy_add(&mbt, "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(&mbt, "vcs0", mmio_base);
> +
> + if (gen < 11)
> + mmio_base = 0x1a000;
> + else
> + mmio_base = 0x1c8000;
> + _gem_engine_mmio_info_legacy_add(&mbt, "vecs0", mmio_base);
> +
> + if (mbt.mb_cnt <= 0)
> + return NULL;
> +
> + mbp = _gem_engine_mmio_info_dup(&mbt);
> +
> + 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.
