Re: [PATCH i-g-t] tests/pm_rpm: Fix CRASH on machines that lack LLC

[David Weinehall <david.weinehall@linux.intel.com>]

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?

> > 
> > Signed-off-by: David Weinehall <david.weinehall@linux.intel.com>
> > ---
> >  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?


Kind regards, DAvid
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx