[Intel-gfx] [PATCH 35/39] drm/i915: Pin pages before waiting

Chris Wilson chris at chris-wilson.co.uk
Fri Jun 14 20:13:51 UTC 2019


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 at chris-wilson.co.uk> 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() ?
-Chris


More information about the Intel-gfx mailing list