From: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com> To: Chris Wilson <chris@chris-wilson.co.uk> Cc: intel-gfx@lists.freedesktop.org, igt-dev@lists.freedesktop.org Subject: Re: [RFC PATCH i-g-t v4 2/4] lib: Add minimum GTT alignment helper Date: Mon, 04 Nov 2019 10:23:12 +0100 [thread overview] Message-ID: <2784945.zMDf8oG64d@jkrzyszt-desk.ger.corp.intel.com> (raw) In-Reply-To: <157260292527.17607.2659694670117420139@skylake-alporthouse-com> Hi Chris, On Friday, November 1, 2019 11:08:45 AM CET Chris Wilson wrote: > Quoting Janusz Krzysztofik (2019-10-31 15:28:55) > > Some tests assume 4kB offset alignment while using softpin. That > > assumption may be wrong on future GEM backends with possibly larger > > minimum page sizes. As a result, those tests may either fail on > > softpin at offsets which are incorrectly aligned, may silently skip > > such incorrectly aligned addresses assuming them occupied by other > > users if incorrect detection method is used, or may always succeed > > when examining invalid use patterns. > > > > Provide a helper function that detects minimum GTT alignment. Tests > > may use it to calculate softpin offsets valid for actually used backing > > store. > > > > v2: Rename the helper, use 'minimum GTT alignment' term across the > > change (Chris), > > - use error numbers to distinguish between invalid offsets and > > addresses occupied by other users, then > > - simplify the code (Chris). > > > > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com> > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > > Cc: Stuart Summers <stuart.summers@intel.com> > > --- > > lib/ioctl_wrappers.c | 46 ++++++++++++++++++++++++++++++++++++++++++++ > > lib/ioctl_wrappers.h | 2 ++ > > 2 files changed, 48 insertions(+) > > > > diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c > > index 628f8b83..f0ef8b2e 100644 > > --- a/lib/ioctl_wrappers.c > > +++ b/lib/ioctl_wrappers.c > > @@ -54,6 +54,7 @@ > > #include "intel_io.h" > > #include "igt_debugfs.h" > > #include "igt_sysfs.h" > > +#include "igt_x86.h" > > #include "config.h" > > > > #ifdef HAVE_VALGRIND > > @@ -1158,6 +1159,51 @@ bool gem_has_softpin(int fd) > > return has_softpin; > > } > > > > +/** > > + * gem_gtt_min_alignment_order: > > + * @fd: open i915 drm file descriptor > > + * > > + * This function detects the minimum possible alignment of a soft-pinned gem > > + * object allocated from a default backing store. It is useful for calculating > > + * correctly aligned softpin offsets. > > + * Since size order to size conversion (size = 1 << order) is less trivial > > + * than the opposite, the function returns the alignment order as more handy. > > + * > > + * Returns: > > + * Size order of the minimum GTT alignment > > + */ > > +int gem_gtt_min_alignment_order(int fd) > > But not part of ioctl_wrappers.c! > > lib/i915/gem_gtt_topology.c? _query.c? _probe.c? I was thinking about that but couldn't find a good name. I'll use one of your proposed, thanks. > > +{ > > + struct drm_i915_gem_exec_object2 obj; > > + struct drm_i915_gem_execbuffer2 eb; > > + const uint32_t bbe = MI_BATCH_BUFFER_END; > > + int order; > > + > > + /* no softpin => 4kB page size */ > > + if (!gem_has_softpin(fd)) > > + return 12; > > + > > + memset(&obj, 0, sizeof(obj)); > > + memset(&eb, 0, sizeof(eb)); > > + > > + obj.handle = gem_create(fd, 4096); > > + obj.flags = EXEC_OBJECT_PINNED | EXEC_OBJECT_SUPPORTS_48B_ADDRESS; > > + eb.buffers_ptr = to_user_pointer(&obj); > > + eb.buffer_count = 1; > > + gem_write(fd, obj.handle, 0, &bbe, sizeof(bbe)); > > + > > + for (order = 12; order < 64; order++) { > > + obj.offset = 1ull << order; > > + if (__gem_execbuf(fd, &eb) != -EINVAL) > > + break; Should I also follow your advice of checking for -ENOSPC here rather than !-EINVAL? > > + } > > + igt_assert(obj.offset < gem_aperture_size(fd)); > > Hmm, there is one more trick we can apply here: an invalid reloc. > > memset(&reloc, 0, sizeof(reloc)); > obj.relocs_ptr = to_user_pointer(&reloc); > obj.relocation_count = 1; > > We first process the buffers, then we process the relocations. So we > will get the -EINVAL for illegal softpin before we get the -ENOENT for > invalid reloc handle. > > That just saves actually emitting a request. I like this idea. Thanks, Janusz > -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: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com> To: Chris Wilson <chris@chris-wilson.co.uk> Cc: intel-gfx@lists.freedesktop.org, igt-dev@lists.freedesktop.org Subject: Re: [Intel-gfx] [RFC PATCH i-g-t v4 2/4] lib: Add minimum GTT alignment helper Date: Mon, 04 Nov 2019 10:23:12 +0100 [thread overview] Message-ID: <2784945.zMDf8oG64d@jkrzyszt-desk.ger.corp.intel.com> (raw) Message-ID: <20191104092312.EcItdEnerwOT4FB5dw-ALuCuCOSWfm77QiFtBAGIceQ@z> (raw) In-Reply-To: <157260292527.17607.2659694670117420139@skylake-alporthouse-com> Hi Chris, On Friday, November 1, 2019 11:08:45 AM CET Chris Wilson wrote: > Quoting Janusz Krzysztofik (2019-10-31 15:28:55) > > Some tests assume 4kB offset alignment while using softpin. That > > assumption may be wrong on future GEM backends with possibly larger > > minimum page sizes. As a result, those tests may either fail on > > softpin at offsets which are incorrectly aligned, may silently skip > > such incorrectly aligned addresses assuming them occupied by other > > users if incorrect detection method is used, or may always succeed > > when examining invalid use patterns. > > > > Provide a helper function that detects minimum GTT alignment. Tests > > may use it to calculate softpin offsets valid for actually used backing > > store. > > > > v2: Rename the helper, use 'minimum GTT alignment' term across the > > change (Chris), > > - use error numbers to distinguish between invalid offsets and > > addresses occupied by other users, then > > - simplify the code (Chris). > > > > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com> > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > > Cc: Stuart Summers <stuart.summers@intel.com> > > --- > > lib/ioctl_wrappers.c | 46 ++++++++++++++++++++++++++++++++++++++++++++ > > lib/ioctl_wrappers.h | 2 ++ > > 2 files changed, 48 insertions(+) > > > > diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c > > index 628f8b83..f0ef8b2e 100644 > > --- a/lib/ioctl_wrappers.c > > +++ b/lib/ioctl_wrappers.c > > @@ -54,6 +54,7 @@ > > #include "intel_io.h" > > #include "igt_debugfs.h" > > #include "igt_sysfs.h" > > +#include "igt_x86.h" > > #include "config.h" > > > > #ifdef HAVE_VALGRIND > > @@ -1158,6 +1159,51 @@ bool gem_has_softpin(int fd) > > return has_softpin; > > } > > > > +/** > > + * gem_gtt_min_alignment_order: > > + * @fd: open i915 drm file descriptor > > + * > > + * This function detects the minimum possible alignment of a soft-pinned gem > > + * object allocated from a default backing store. It is useful for calculating > > + * correctly aligned softpin offsets. > > + * Since size order to size conversion (size = 1 << order) is less trivial > > + * than the opposite, the function returns the alignment order as more handy. > > + * > > + * Returns: > > + * Size order of the minimum GTT alignment > > + */ > > +int gem_gtt_min_alignment_order(int fd) > > But not part of ioctl_wrappers.c! > > lib/i915/gem_gtt_topology.c? _query.c? _probe.c? I was thinking about that but couldn't find a good name. I'll use one of your proposed, thanks. > > +{ > > + struct drm_i915_gem_exec_object2 obj; > > + struct drm_i915_gem_execbuffer2 eb; > > + const uint32_t bbe = MI_BATCH_BUFFER_END; > > + int order; > > + > > + /* no softpin => 4kB page size */ > > + if (!gem_has_softpin(fd)) > > + return 12; > > + > > + memset(&obj, 0, sizeof(obj)); > > + memset(&eb, 0, sizeof(eb)); > > + > > + obj.handle = gem_create(fd, 4096); > > + obj.flags = EXEC_OBJECT_PINNED | EXEC_OBJECT_SUPPORTS_48B_ADDRESS; > > + eb.buffers_ptr = to_user_pointer(&obj); > > + eb.buffer_count = 1; > > + gem_write(fd, obj.handle, 0, &bbe, sizeof(bbe)); > > + > > + for (order = 12; order < 64; order++) { > > + obj.offset = 1ull << order; > > + if (__gem_execbuf(fd, &eb) != -EINVAL) > > + break; Should I also follow your advice of checking for -ENOSPC here rather than !-EINVAL? > > + } > > + igt_assert(obj.offset < gem_aperture_size(fd)); > > Hmm, there is one more trick we can apply here: an invalid reloc. > > memset(&reloc, 0, sizeof(reloc)); > obj.relocs_ptr = to_user_pointer(&reloc); > obj.relocation_count = 1; > > We first process the buffers, then we process the relocations. So we > will get the -EINVAL for illegal softpin before we get the -ENOENT for > invalid reloc handle. > > That just saves actually emitting a request. I like this idea. Thanks, Janusz > -Chris > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2019-11-04 9:23 UTC|newest] Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-10-31 15:28 [RFC PATCH i-g-t v4 0/4] Calculate softpin offsets from minimum GTT alignment Janusz Krzysztofik 2019-10-31 15:28 ` [Intel-gfx] " Janusz Krzysztofik 2019-10-31 15:28 ` [RFC PATCH i-g-t v4 1/4] tests/gem_exec_reloc: Don't filter out invalid addresses Janusz Krzysztofik 2019-10-31 15:28 ` [Intel-gfx] " Janusz Krzysztofik 2019-10-31 16:59 ` Vanshidhar Konda 2019-10-31 16:59 ` [Intel-gfx] " Vanshidhar Konda 2019-11-04 9:16 ` Janusz Krzysztofik 2019-11-04 9:16 ` [Intel-gfx] " Janusz Krzysztofik 2019-11-01 10:02 ` Chris Wilson 2019-11-01 10:02 ` [igt-dev] " Chris Wilson 2019-11-01 10:02 ` [Intel-gfx] " Chris Wilson 2019-11-04 9:13 ` Janusz Krzysztofik 2019-11-04 9:13 ` [Intel-gfx] " Janusz Krzysztofik 2019-11-04 9:17 ` Chris Wilson 2019-11-04 9:17 ` [igt-dev] " Chris Wilson 2019-11-04 9:17 ` [Intel-gfx] " Chris Wilson 2019-10-31 15:28 ` [RFC PATCH i-g-t v4 2/4] lib: Add minimum GTT alignment helper Janusz Krzysztofik 2019-10-31 15:28 ` [Intel-gfx] " Janusz Krzysztofik 2019-10-31 16:58 ` Vanshidhar Konda 2019-10-31 16:58 ` [Intel-gfx] " Vanshidhar Konda 2019-11-04 14:40 ` Janusz Krzysztofik 2019-11-04 14:40 ` [Intel-gfx] " Janusz Krzysztofik 2019-11-01 10:08 ` Chris Wilson 2019-11-01 10:08 ` [igt-dev] " Chris Wilson 2019-11-01 10:08 ` [Intel-gfx] " Chris Wilson 2019-11-04 9:23 ` Janusz Krzysztofik [this message] 2019-11-04 9:23 ` Janusz Krzysztofik 2019-11-04 9:28 ` Chris Wilson 2019-11-04 9:28 ` [Intel-gfx] " Chris Wilson 2019-11-04 10:26 ` Janusz Krzysztofik 2019-11-04 10:26 ` [Intel-gfx] " Janusz Krzysztofik 2019-10-31 15:28 ` [RFC PATCH i-g-t v4 3/4] tests/gem_exec_reloc: Calculate offsets from minimum GTT alignment Janusz Krzysztofik 2019-10-31 15:28 ` [Intel-gfx] " Janusz Krzysztofik 2019-11-01 10:11 ` Chris Wilson 2019-11-01 10:11 ` [igt-dev] " Chris Wilson 2019-11-01 10:11 ` [Intel-gfx] " Chris Wilson 2019-10-31 15:28 ` [RFC PATCH i-g-t v4 4/4] tests/gem_ctx_shared: Align objects using " Janusz Krzysztofik 2019-10-31 15:28 ` [Intel-gfx] " Janusz Krzysztofik 2019-10-31 17:00 ` Vanshidhar Konda 2019-10-31 17:00 ` [Intel-gfx] " Vanshidhar Konda 2019-11-01 9:42 ` Chris Wilson 2019-11-01 9:42 ` [Intel-gfx] " Chris Wilson 2019-11-01 10:11 ` Chris Wilson 2019-11-01 10:11 ` [igt-dev] " Chris Wilson 2019-11-01 10:11 ` [Intel-gfx] " Chris Wilson 2019-11-04 9:39 ` Janusz Krzysztofik 2019-11-04 9:39 ` [Intel-gfx] " Janusz Krzysztofik 2019-11-04 9:47 ` Chris Wilson 2019-11-04 9:47 ` [igt-dev] " Chris Wilson 2019-11-04 9:47 ` [Intel-gfx] " Chris Wilson 2019-10-31 16:47 ` [igt-dev] ✓ Fi.CI.BAT: success for Calculate softpin offsets from " Patchwork 2019-11-01 19:09 ` [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=2784945.zMDf8oG64d@jkrzyszt-desk.ger.corp.intel.com \ --to=janusz.krzysztofik@linux.intel.com \ --cc=chris@chris-wilson.co.uk \ --cc=igt-dev@lists.freedesktop.org \ --cc=intel-gfx@lists.freedesktop.org \ /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.