On Wed, Mar 02, 2016 at 04:41:54PM +0200, Imre Deak wrote: > On ke, 2016-03-02 at 14:37 +0000, Chris Wilson wrote: > > On Wed, Mar 02, 2016 at 04:32:41PM +0200, Imre Deak wrote: > > > On ke, 2016-03-02 at 14:04 +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. > > > > > > The purpose of the enable/disable screen calls was to make sure > > > that > > > the object gets unbound, otherwise we may not call i915_vma_bind() > > > which is where the actual HW access happened. But actually it would > > > be > > > enough to call disable_all_screens_and_wait() once and then call > > > wait_for_suspended() instead of disable_all_screens_and_wait() in > > > the > > > loop. > > > > Actually no, that's why you have the memset/*ptr - that is what > > forces > > the vma to get bound. And to exercise the set-cache-level, you need > > it > > bound into at a different cache-level, hence the suggestion to call > > set-cache-level again - respecting the rules of using the GGTT. > > Yes I see that it gets bound, but is guaranteed that it gets unbound > during the next set-cache-level call? As I understand it will only > happen if i915_gem_valid_gtt_space() returns false. No. But that is beside the point. It is either unbound or rewritten during the set-cache-level ioctl, either way we write through the GSM if it is currently bound. (And it will only be unbound in fairly rare circumstances.) -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
