On Tue, 2015-11-17 at 22:38 +0000, Chris Wilson wrote:
> On Tue, Nov 17, 2015 at 11:16:09PM +0100, Daniel Vetter wrote:
> > On Thu, Nov 5, 2015 at 12:56 PM, Chris Wilson <chris@chris-wilson.c
> > o.uk> wrote:
> > > The tricky part is reviewing the i915_gem_release_mmap() callers
> > > to
> > > ensure that they have the right barrier. If you make
> > > i915_gem_release_mmap() assert it holds an rpm ref, and then make
> > > the
> > > i915_gem_release_all_mmaps() unlink the node directly that should
> > > do the
> > > trick.
> >
> > I think the easier solution would be to add a mutex for the
> > fault_list. We call release_mmap from a lot of places,
>
> We don't though. The only times we do are when we touching hw
> registers
> (or gsm):
>
> set_tiling_ioctl - which also may unbind and so needs rpm
> fence_write - which needs rpm to write the reigsters
> vma_unbind - which needs rpm to write through the gsm
> set_caching - which needs rpm to write through the gsm
>
> and currently by rpm itself.
>
> I think it is certainly reasonable to use the rpm barriers for the
> faulted list. The only one where we have to actually ensure we hold
> rpm
> is the set_tiling_ioctl.
Btw, since this would be a bigger change shouldn't we first add the new
RPM wakelock asserts? That's mostly reviewed already anyway, I still
have to check the RPS work which would need the same handling as the
hang check work (Chris' idea to use rpm_try_get there).
So how about the following until that, which is the correct way in any
case imo:
void __suspend_report_result(const char *function, void *fn, int ret)
{
- if (ret)
+ if (ret == -EBUSY || ret == -EAGAIN)
+ printk(KERN_DEBUG "%s(): %pF returns %d\n", function, fn, ret);
+ else if (ret)
printk(KERN_ERR "%s(): %pF returns %d\n", function, fn, ret);
}
--Imre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
