Quoting Chris Wilson (2018-02-13 13:45:33)
> Quoting Tvrtko Ursulin (2018-02-13 13:42:03)
> > 
> > On 12/02/2018 21:11, Chris Wilson wrote:
> > > When we need to rebind the vma into the global GTT for snb, we need to
> > > drop the current reloc_cache as it will be holding a kmap_atomic() and
> > > we may need to sleep for i915_vma_bind(). In practice, this is not an
> > > issue as we already hold an rpm reference for the execbuffer, but with
> > > tighter error checking around rpm we need to be more careful.
> > > 
> > > References: 31a39207f04a ("drm/i915: Cache kmap between relocations")
> > > Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> > > Cc: Tvrtko Ursulin <tvrtko.ursu...@intel.com>
> > > Cc: Joonas Lahtinen <joonas.lahti...@linux.intel.com>
> > > ---
> > >   drivers/gpu/drm/i915/i915_gem_execbuffer.c | 6 ++++++
> > >   1 file changed, 6 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
> > > b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > > index b15305f2fb76..8c34b1b5a126 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > > @@ -1315,6 +1315,12 @@ eb_relocate_entry(struct i915_execbuffer *eb,
> > >                */
> > >               if (reloc->write_domain == I915_GEM_DOMAIN_INSTRUCTION &&
> > >                   IS_GEN6(eb->i915)) {
> > > +                     /*
> > > +                      * Release the kmap_atomic cache in order to allow 
> > > the
> > > +                      * i915_vma_bind() to sleep (if needs be).
> > > +                      */
> > > +                     reloc_cache_reset(&eb->reloc_cache);
> > > +
> > >                       err = i915_vma_bind(target, 
> > > target->obj->cache_level,
> > >                                           PIN_GLOBAL);
> > >                       if (WARN_ONCE(err,
> > > 
> > 
> > Hmm yes, very interesting. If you are happy with the performance hit then:
> > 
> > Reviewed-by: Tvrtko Ursulin <tvrtko.ursu...@intel.com>
> > 
> > But in general could it also be solved by changing how 
> > intel_runtime_pm_get works - making our wakeref_count top level and only 
> > calling pm_runtime_get_sync on 0 to 1 transition? That would solve the 
> > issue with proposed might_sleep and code paths which know the condition 
> > is actually impossible.
> 
> I think we should do that anyway to reduce the jitter we see in calling
> pm_runtime_get_sync().

I recall the challenge lay in the asserts which also bump our counter to
hide themselves.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to