From: Chris Wilson <chris@chris-wilson.co.uk> To: Tvrtko Ursulin <tursulin@ursulin.net>, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>, igt-dev@lists.freedesktop.org Cc: Intel-gfx@lists.freedesktop.org Subject: Re: [igt-dev] [PATCH i-g-t v13 2/2] tests/gem_ctx_sseu: Dynamic (sub)slice programming tests Date: Mon, 17 Sep 2018 11:38:39 +0100 [thread overview] Message-ID: <153718071990.4114.4099019937634796879@skylake-alporthouse-com> (raw) In-Reply-To: <235db5a4-1b2a-bcf9-543c-ed71b3289048@linux.intel.com> Quoting Tvrtko Ursulin (2018-09-17 10:33:47) > > On 14/09/2018 17:17, Chris Wilson wrote: > > Quoting Tvrtko Ursulin (2018-09-14 17:04:38) > >> +static igt_spin_t * > >> +__post_set(int fd, unsigned int flags, uint32_t ctx, igt_spin_t *spin, > >> + unsigned int expected) > >> +{ > >> + bool busy_reset = (flags & TEST_BUSY) && > >> + (flags & (TEST_RESET | TEST_HANG)); > >> + > >> + if (flags & TEST_RESET) > >> + igt_force_gpu_reset(fd); > >> + else if (flags & TEST_HANG) > >> + gem_sync(fd, spin->handle); > > > > Hmm, in both cases this is before we submit the read/post-sseu batch. > > > > I was thinking more for the hang you would simply opt to not terminate > > the spinner. > > Okay I can do that as well. Hang will then be processed while we try to > pread the rpcs eb result so I think it works. > > > > > Being utterly pedantic, we may want to check per-engine resets and > > device level. > > Isn't the per-engine default, so the hang test flavour would trigger it? > And to explicitly ask for whole device we have some debugfs api for it? i915.reset=2 (default if supported) per-engine i915.reset=1 global i915.reset=0 none, will wedge on hang. Alas no temporary override via debugfs yet. Be my guest :) > >> + if (spin && !busy_reset) { > >> + igt_assert_eq(read_slice_count_busy(fd, ctx, 0, spin), > >> + expected); > >> + } else { > >> + igt_assert_eq(read_slice_count(fd, ctx, 0), expected); > >> + } > >> + > >> + igt_assert_eq(read_slice_count(fd, 0, 0), __slice_count__); > >> + > >> + if (spin) > >> + igt_spin_batch_free(fd, spin); > >> + > >> + if (flags & TEST_IDLE) > >> + igt_drop_caches_set(fd, DROP_RETIRE | DROP_IDLE | DROP_ACTIVE); > >> + > >> + if (flags & TEST_SUSPEND) { > >> + igt_set_autoresume_delay(5); > > > > Brave. Opting for a faster wakeup than default... > > I think I started with copy paste and then lowered it when testing the > test. Is the right thing to omit specifying it? Yes. The default (15s) should be the conservative minimal required for all machines to be sure we don't miss the wakeup (or it trigger at the wrong time and cause -EBUSY). There are a few tests where we know (e.g. we're waiting on hangcheck) that 15s is not enough. We might suggest to Tomi to do some trial and error and see if we can stably reduce it across the board. > >> + igt_system_suspend_autoresume(SUSPEND_STATE_MEM, > >> + SUSPEND_TEST_NONE); > >> + } > >> + > >> + igt_assert_eq(read_slice_count(fd, ctx, 0), expected); > >> + igt_assert_eq(read_slice_count(fd, 0, 0), __slice_count__); > >> + > >> + return NULL; > >> +} > > > > Overall, nothing else strikes me as being absent. > > > > Oh, one more thing; do we are what state we leave the system in on > > module unload? So long as the GPU reset clears it we should be ok. > > I don't think it applies when device is off. And on load running the > kernel context restores the default full enablement. That's not strictly true... On module load the kernel context will be marked with RESTORE_INHIBIT as it is created before the default_state is. It's the next context to be used that will have the full setup. I think the reset is good enough as the defaults should be sane (or we have a w/a to set up sane register state), and once we start using the device proper all is specified. Needless worry on my part. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
WARNING: multiple messages have this Message-ID (diff)
From: Chris Wilson <chris@chris-wilson.co.uk> To: Tvrtko Ursulin <tursulin@ursulin.net>, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>, igt-dev@lists.freedesktop.org Cc: Intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [igt-dev] [PATCH i-g-t v13 2/2] tests/gem_ctx_sseu: Dynamic (sub)slice programming tests Date: Mon, 17 Sep 2018 11:38:39 +0100 [thread overview] Message-ID: <153718071990.4114.4099019937634796879@skylake-alporthouse-com> (raw) In-Reply-To: <235db5a4-1b2a-bcf9-543c-ed71b3289048@linux.intel.com> Quoting Tvrtko Ursulin (2018-09-17 10:33:47) > > On 14/09/2018 17:17, Chris Wilson wrote: > > Quoting Tvrtko Ursulin (2018-09-14 17:04:38) > >> +static igt_spin_t * > >> +__post_set(int fd, unsigned int flags, uint32_t ctx, igt_spin_t *spin, > >> + unsigned int expected) > >> +{ > >> + bool busy_reset = (flags & TEST_BUSY) && > >> + (flags & (TEST_RESET | TEST_HANG)); > >> + > >> + if (flags & TEST_RESET) > >> + igt_force_gpu_reset(fd); > >> + else if (flags & TEST_HANG) > >> + gem_sync(fd, spin->handle); > > > > Hmm, in both cases this is before we submit the read/post-sseu batch. > > > > I was thinking more for the hang you would simply opt to not terminate > > the spinner. > > Okay I can do that as well. Hang will then be processed while we try to > pread the rpcs eb result so I think it works. > > > > > Being utterly pedantic, we may want to check per-engine resets and > > device level. > > Isn't the per-engine default, so the hang test flavour would trigger it? > And to explicitly ask for whole device we have some debugfs api for it? i915.reset=2 (default if supported) per-engine i915.reset=1 global i915.reset=0 none, will wedge on hang. Alas no temporary override via debugfs yet. Be my guest :) > >> + if (spin && !busy_reset) { > >> + igt_assert_eq(read_slice_count_busy(fd, ctx, 0, spin), > >> + expected); > >> + } else { > >> + igt_assert_eq(read_slice_count(fd, ctx, 0), expected); > >> + } > >> + > >> + igt_assert_eq(read_slice_count(fd, 0, 0), __slice_count__); > >> + > >> + if (spin) > >> + igt_spin_batch_free(fd, spin); > >> + > >> + if (flags & TEST_IDLE) > >> + igt_drop_caches_set(fd, DROP_RETIRE | DROP_IDLE | DROP_ACTIVE); > >> + > >> + if (flags & TEST_SUSPEND) { > >> + igt_set_autoresume_delay(5); > > > > Brave. Opting for a faster wakeup than default... > > I think I started with copy paste and then lowered it when testing the > test. Is the right thing to omit specifying it? Yes. The default (15s) should be the conservative minimal required for all machines to be sure we don't miss the wakeup (or it trigger at the wrong time and cause -EBUSY). There are a few tests where we know (e.g. we're waiting on hangcheck) that 15s is not enough. We might suggest to Tomi to do some trial and error and see if we can stably reduce it across the board. > >> + igt_system_suspend_autoresume(SUSPEND_STATE_MEM, > >> + SUSPEND_TEST_NONE); > >> + } > >> + > >> + igt_assert_eq(read_slice_count(fd, ctx, 0), expected); > >> + igt_assert_eq(read_slice_count(fd, 0, 0), __slice_count__); > >> + > >> + return NULL; > >> +} > > > > Overall, nothing else strikes me as being absent. > > > > Oh, one more thing; do we are what state we leave the system in on > > module unload? So long as the GPU reset clears it we should be ok. > > I don't think it applies when device is off. And on load running the > kernel context restores the default full enablement. That's not strictly true... On module load the kernel context will be marked with RESTORE_INHIBIT as it is created before the default_state is. It's the next context to be used that will have the full setup. I think the reset is good enough as the defaults should be sane (or we have a w/a to set up sane register state), and once we start using the device proper all is specified. Needless worry on my part. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2018-09-17 10:38 UTC|newest] Thread overview: 55+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-09-05 14:25 [PATH i-g-t 0/2] Per context dynamic (sub)slice power-gating Tvrtko Ursulin 2018-09-05 14:25 ` [igt-dev] " Tvrtko Ursulin 2018-09-05 14:25 ` [PATH i-g-t 1/2] headers: bump Tvrtko Ursulin 2018-09-05 14:25 ` [igt-dev] " Tvrtko Ursulin 2018-09-05 14:25 ` [PATH i-g-t 2/2] tests: add slice power programming test Tvrtko Ursulin 2018-09-05 14:25 ` [igt-dev] " Tvrtko Ursulin 2018-09-05 22:57 ` Chris Wilson 2018-09-05 22:57 ` Chris Wilson 2018-09-06 7:00 ` Chris Wilson 2018-09-06 7:00 ` Chris Wilson 2018-09-06 9:31 ` Tvrtko Ursulin 2018-09-06 9:31 ` [igt-dev] [Intel-gfx] " Tvrtko Ursulin 2018-09-06 9:50 ` [igt-dev] " Chris Wilson 2018-09-06 9:50 ` [igt-dev] [Intel-gfx] " Chris Wilson 2018-09-11 11:34 ` [PATH i-g-t v11 " Tvrtko Ursulin 2018-09-11 11:34 ` [igt-dev] " Tvrtko Ursulin 2018-09-11 11:45 ` Chris Wilson 2018-09-11 11:45 ` Chris Wilson 2018-09-11 12:00 ` Tvrtko Ursulin 2018-09-11 12:00 ` [Intel-gfx] " Tvrtko Ursulin 2018-09-11 14:42 ` [PATH i-g-t v12 " Tvrtko Ursulin 2018-09-11 14:42 ` [igt-dev] " Tvrtko Ursulin 2018-09-12 11:53 ` Chris Wilson 2018-09-12 11:53 ` [igt-dev] [Intel-gfx] " Chris Wilson 2018-09-13 10:38 ` Tvrtko Ursulin 2018-09-13 10:38 ` [igt-dev] [Intel-gfx] " Tvrtko Ursulin 2018-09-13 10:48 ` Chris Wilson 2018-09-13 10:48 ` [igt-dev] [Intel-gfx] " Chris Wilson 2018-09-14 16:04 ` [PATCH i-g-t v13 2/2] tests/gem_ctx_sseu: Dynamic (sub)slice programming tests Tvrtko Ursulin 2018-09-14 16:04 ` [Intel-gfx] " Tvrtko Ursulin 2018-09-14 16:07 ` [igt-dev] " Chris Wilson 2018-09-14 16:07 ` Chris Wilson 2018-09-14 16:17 ` Chris Wilson 2018-09-14 16:17 ` Chris Wilson 2018-09-17 9:33 ` Tvrtko Ursulin 2018-09-17 9:33 ` [Intel-gfx] " Tvrtko Ursulin 2018-09-17 10:38 ` Chris Wilson [this message] 2018-09-17 10:38 ` Chris Wilson 2018-09-17 11:28 ` [PATCH i-g-t v14 " Tvrtko Ursulin 2018-09-17 11:28 ` [Intel-gfx] " Tvrtko Ursulin 2018-09-17 12:04 ` [igt-dev] " Chris Wilson 2018-09-17 12:04 ` Chris Wilson 2018-09-18 13:41 ` [PATCH i-g-t v15 " Tvrtko Ursulin 2018-09-18 13:41 ` [igt-dev] " Tvrtko Ursulin 2018-09-05 16:46 ` [igt-dev] ✓ Fi.CI.BAT: success for Per context dynamic (sub)slice power-gating Patchwork 2018-09-05 22:44 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork 2018-09-11 18:13 ` [igt-dev] ✓ Fi.CI.BAT: success for Per context dynamic (sub)slice power-gating (rev3) Patchwork 2018-09-11 23:31 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork 2018-09-14 17:22 ` [igt-dev] ✓ Fi.CI.BAT: success for Per context dynamic (sub)slice power-gating (rev4) Patchwork 2018-09-14 22:22 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork 2018-09-17 12:21 ` [igt-dev] ✗ Fi.CI.BAT: failure for Per context dynamic (sub)slice power-gating (rev5) Patchwork 2018-09-17 18:21 ` [igt-dev] ✓ Fi.CI.BAT: success " Patchwork 2018-09-17 20:43 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork 2018-09-18 14:02 ` [igt-dev] ✓ Fi.CI.BAT: success for Per context dynamic (sub)slice power-gating (rev6) Patchwork 2018-09-18 15:18 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=153718071990.4114.4099019937634796879@skylake-alporthouse-com \ --to=chris@chris-wilson.co.uk \ --cc=Intel-gfx@lists.freedesktop.org \ --cc=igt-dev@lists.freedesktop.org \ --cc=tursulin@ursulin.net \ --cc=tvrtko.ursulin@linux.intel.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.