On Wed, Mar 02, 2016 at 02:04:58PM +0000, Chris Wilson wrote:
> On Wed, Mar 02, 2016 at 03:55:56PM +0200, David Weinehall wrote:
> > On Wed, Mar 02, 2016 at 01:27:06PM +0000, Chris Wilson wrote:
> > > On Wed, Mar 02, 2016 at 03:11:57PM +0200, David Weinehall wrote:
> > > > On machines that lack an LLC the pm-caching subtest will
> > > > terminate with sigbus and thus CRASH during the
> > > > I915_CACHING_CACHED iteration. This patch adds a check for
> > > > this condition and skips that iteration.
> > >
> > > you can delete the got_caching assertion and
> > > enable_one_screen_and_wait() as well, they are not exercising the
> > > associated code.
> >
> > Hmmm. How about the matching disable_all_screens_and_wait()?
> > Also, isn't the got_caching assertion meant to check that
> > when we enable GEM caching we actually get the mode we requested,
> > and if so, do we test for this elsewhere? Or are you saying that
> > this test doesn't achieve this purpose?
>
> This is not a test for set-caching API, but on whether we do device
> accesses without rpm. get-caching doesn't touch the device at all (and
> never ever should) so is irrelevant for the test.
Right. How about the disable/enable screens bit? Stay or go?
> >
> > > >
> > > > Signed-off-by: David Weinehall <david.weinehall@xxxxxxxxxxxxxxx>
> > > > ---
> > > > tests/pm_rpm.c | 10 ++++++++++
> > > > 1 file changed, 10 insertions(+)
> > > >
> > > > diff --git a/tests/pm_rpm.c b/tests/pm_rpm.c
> > > > index 2aa6c1018aa2..c25252eafad0 100644
> > > > --- a/tests/pm_rpm.c
> > > > +++ b/tests/pm_rpm.c
> > > > @@ -1813,6 +1813,16 @@ static void pm_test_caching(void)
> > > > gem_buf = gem_mmap__gtt(drm_fd, handle, gtt_obj_max_size, PROT_WRITE);
> > > >
> > > > for (i = 0; i < ARRAY_SIZE(cache_levels); i++) {
> > > > + /*
> > > > + * Skip the I915_CACHING_CACHED test
> > > > + * if we lack an LLC cache
> > > > + */
> > > > + if (cache_levels[i] == I915_CACHING_CACHED &&
> > > > + !gem_has_llc(drm_fd)) {
> > > > + igt_debug("!gem_has_llc(); skipping\n");
> > > > + continue;
> > > > + }
> > >
> > > No. For the purposes of the test you actually want to call
> > > gem_set_caching(fd, handle, NONE).
> >
> > Wouldn't that case already be exercised in the first iteration of this
> > test?
>
> Not really. The test is that given a vma bound into the ggtt that we
> then change the cache level on, do we take the rpm around the gsm
> access. To exercise the code we the vma bound into the ggtt, that is
> what the *ptr does. Then we need it to change cache level to exercise
> how we handle the vma inside set-cache-level. That is the crux of the
> test.
Thanks for the explanation!
Kind regards, David
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
