On Fri, 14 Jun 2019 at 21:13, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote: > > Quoting Chris Wilson (2019-06-14 20:58:09) > > Quoting Matthew Auld (2019-06-14 20:53:26) > > > On Fri, 14 Jun 2019 at 08:11, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote: > > > > @@ -67,10 +61,17 @@ i915_gem_object_set_to_wc_domain(struct drm_i915_gem_object *obj, bool write) > > > > * continue to assume that the obj remained out of the CPU cached > > > > * domain. > > > > */ > > > > - ret = i915_gem_object_pin_pages(obj); > > > > + ret = i915_gem_object_pin_pages_async(obj); > > > > if (ret) > > > > return ret; > > > > > > > > + ret = i915_gem_object_wait(obj, > > > > + I915_WAIT_INTERRUPTIBLE | > > > > + (write ? I915_WAIT_ALL : 0), > > > > + MAX_SCHEDULE_TIMEOUT); > > > > + if (ret) > > > > + goto out_unpin; > > > > + > > > > > > Do we somehow propagate a potential error from a worker to the > > > object_wait()? Or should we be looking at obj->mm.pages here? > > > > Yeah, I've propagated such errors elsewhere (principally along the > > fences). What you are suggesting is tantamount to making > > i915_gem_object_wait() report an error, and I have bad memories from all > > the unhandled -EIO in the past. However, that feels the natural thing to > > do, so lets give it a whirl. > > So we need to check for error pages anyway, because we can't rule out a > race between the pin_pages_async and i915_gem_object_wait. > > There's plenty of duplicated code for pin_pages_async, object_wait, > check pages so I should refactor that into a variant, > i915_gem_object_pin_pages_wait() ? Yeah, makes sense to me. _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx