On Fri, 2015-07-31 at 18:46 +0530, Goel, Akash wrote:
>
> On 7/22/2015 8:09 PM, Chris Wilson wrote:
> > On Wed, Jul 22, 2015 at 07:21:49PM +0530, ankitprasad.r.sha...@intel.com
> > wrote:
> >> static int
> >> i915_gem_shmem_pread(struct drm_device *dev,
> >> struct drm_i915_gem_object *obj,
> >> @@ -754,17 +850,20 @@ i915_gem_pread_ioctl(struct drm_device *dev, void
> >> *data,
> >> goto out;
> >> }
> >>
> >> - /* prime objects have no backing filp to GEM pread/pwrite
> >> - * pages from.
> >> - */
> >> - if (!obj->base.filp) {
> >> - ret = -EINVAL;
> >> - goto out;
> >> - }
> >> -
> >> trace_i915_gem_object_pread(obj, args->offset, args->size);
> >>
> >> - ret = i915_gem_shmem_pread(dev, obj, args, file);
> >> + /* pread for non shmem backed objects */
> >> + if (!obj->base.filp) {
> >> + if (obj->tiling_mode == I915_TILING_NONE)
> >
> > pread/pwrite is defined as a cpu linear, the restriction upon tiling is
> > a simplification of handling swizzling.
> >
> >> + ret = i915_gem_gtt_pread_pwrite(dev, obj, args->size,
> >> + args->offset,
> >> + args->data_ptr,
> >> + false);
> >> + else
> >> + ret = -EINVAL;
> >> + } else {
> >> + ret = i915_gem_shmem_pread(dev, obj, args, file);
> >> + }
> >>
> >> out:
> >> drm_gem_object_unreference(&obj->base);
> >> @@ -1105,17 +1204,22 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void
> >> *data,
> >> goto out;
> >> }
> >>
> >> - /* prime objects have no backing filp to GEM pread/pwrite
> >> - * pages from.
> >> - */
> >> - if (!obj->base.filp) {
> >> - ret = -EINVAL;
> >> - goto out;
> >> - }
> >> -
> >> trace_i915_gem_object_pwrite(obj, args->offset, args->size);
> >>
> >> ret = -EFAULT;
> >> +
> >> + /* pwrite for non shmem backed objects */
> >> + if (!obj->base.filp) {
> >> + if (obj->tiling_mode == I915_TILING_NONE)
> >> + ret = i915_gem_gtt_pread_pwrite(dev, obj, args->size,
> >> + args->offset,
> >> + args->data_ptr,
> >> + true);
> >> + else
> >> + ret = -EINVAL;
> >> +
> >> + goto out;
> >
> > The fast pwrite code always works for obj->base.filp == NULL. To easily
> > make it generic and handle the cases where we cannot fallback to shem,
> > undo the PIN_NONBLOCK.
> >
> > Then you just want something like
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c
> > b/drivers/gpu/drm/i915/i915_gem.c
> > index d3016f37cd4d..f2284a27dd6d 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -1114,8 +1114,9 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void
> > *data,
> > * perspective, requiring manual detiling by the client.
> > */
> > if (obj->tiling_mode == I915_TILING_NONE &&
>
> Since the suggestion is to reuse the gtt_pwrite_fast function only for
> non-shmem backed objects, can we also relax the Tiling constraint here,
> apart from removing the PIN_NONBLOCK flag. As anyways there is a
> put_fence already being done in gtt_pwrite_fast function.
>
> Also currently the gtt_pwrite_fast function is non-tolerant to faults,
> incurred while accessing the User space buffer, so can that also be
> relaxed (at least for the non-shmem backed objects) ??
Even if we reuse the gtt_pwrite_fast we will have to handle page-faults
for non-shmem backed objects, that will contradict the purpose of
gtt_pwrite_fast. The gtt_pread_pwrite will still be around for pread
purpose.
Also, I think it should be ok to relax the tiling constraint as well, as
we put_fence everytime we try to pread/pwrite from/to a non-shmem-backed
object here.
Thanks,
Ankit
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
