Quoting Ville Syrjälä (2019-03-22 14:28:37)
> On Thu, Mar 21, 2019 at 04:19:08PM +0000, Chris Wilson wrote:
> > If we are already in the desired write domain of a set-domain ioctl,
> > then there is nothing for us to do and we can quickly return back to
> > userspace, avoiding any lock contention. By recognising that the
> > write_domain is always a subset of the read_domains, and excluding the
> > no-op case of requiring 0 read_domains in the ioctl, we can infer if the
> > current write_domain matches the target read_domains, there is nothing
> > for us to do.
> >
> > Secondary aspect of this is that we undo the arbitrary fetching and
> > potential flushing of all pages for a set-domain(.write=CPU) call on a
> > fresh object -- which was introduced simply because we do the get-pages
> > before taking the struct_mutex.
> >
> > References: 40e62d5d6be8 ("drm/i915: Acquire the backing storage outside of struct_mutex in set-domain")
> > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> > Cc: Matthew Auld <matthew.william.auld@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/i915_gem.c | 26 +++++++++++++++++++++++---
> > 1 file changed, 23 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index 72374e952e4b..36f557002005 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -1484,17 +1484,37 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data,
> > if ((write_domain | read_domains) & I915_GEM_GPU_DOMAINS)
> > return -EINVAL;
> >
> > - /* Having something in the write domain implies it's in the read
> > + /*
> > + * Having something in the write domain implies it's in the read
> > * domain, and only that read domain. Enforce that in the request.
> > */
> > - if (write_domain != 0 && read_domains != write_domain)
> > + if (write_domain && read_domains != write_domain)
> > return -EINVAL;
> >
> > + if (!read_domains)
> > + return 0;
>
> Hopefully no one is relying on read_domains==0 meaning cpu domain.
> That seems to be how this was handled before.
Hopefully not. None of the userspace has tried that, and I hope that the
idea of write_domain==0 meaning don't set a write_domain has conditioned
everyone into not using it.
> Or maybe we want -EIVNAL here?
Introducing new -EINVAL is also risky.
Hmm. So in case of trouble we should
if (!read_domains)
read_domain = DOMAIN_CPU.
Hopefully no one even notices such a subtle ABI change. Bugzilla watch
out!
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
