On Tue, Jan 26, 2016 at 01:13:34AM +0530, ankitprasad.r.sharma@xxxxxxxxx wrote: > +static int > +i915_gem_gtt_copy(struct drm_device *dev, I still do not like this name as I have no direction information. > + struct drm_i915_gem_object *obj, uint64_t size, > + uint64_t data_offset, uint64_t data_ptr) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + struct drm_mm_node node; > + char __user *user_data; > + uint64_t remain; > + uint64_t offset; > + int ret = 0; > + > + ret = i915_gem_obj_ggtt_pin(obj, 0, PIN_MAPPABLE); > + if (ret) { > + ret = insert_mappable_node(dev_priv, &node, PAGE_SIZE); > + if (ret) > + goto out; > + > + ret = i915_gem_object_get_pages(obj); > + if (ret) { > + remove_mappable_node(&node); > + goto out; > + } > + > + i915_gem_object_pin_pages(obj); > + } else { > + node.start = i915_gem_obj_ggtt_offset(obj); > + node.allocated = false; > + } > + > + ret = i915_gem_object_set_to_gtt_domain(obj, false); > + if (ret) > + goto out_unpin; > + > + ret = i915_gem_object_put_fence(obj); > + if (ret) > + goto out_unpin; This only needs to be done for the whole-obj path. The single-page path explicitly avoids the fence register. > + user_data = to_user_ptr(data_ptr); > + remain = size; > + offset = i915_gem_obj_ggtt_offset(obj) + data_offset; > + > + mutex_unlock(&dev->struct_mutex); > + if (likely(!i915.prefault_disable)) > + ret = fault_in_multipages_writeable(user_data, remain); Handle the EFAULT! > + while (remain > 0) { > + /* Operation in this page > + * > + * page_base = page offset within aperture > + * page_offset = offset within page > + * page_length = bytes to copy for this page > + */ > + u32 page_base = node.start; > + unsigned page_offset = offset_in_page(offset); > + unsigned page_length = PAGE_SIZE - page_offset; > + page_length = remain < page_length ? remain : page_length; > + if (node.allocated) { > + wmb(); > + dev_priv->gtt.base.insert_page(&dev_priv->gtt.base, > + i915_gem_object_get_dma_address(obj, offset >> PAGE_SHIFT), > + node.start, > + I915_CACHE_NONE, 0); > + wmb(); > + } else { > + page_base += offset & PAGE_MASK; > + } > + /* This is a slow read/write as it tries to read from > + * and write to user memory which may result into page > + * faults, and so we cannot perform this under struct_mutex. > + */ > + if (slow_user_access(dev_priv->gtt.mappable, page_base, > + page_offset, user_data, > + page_length, false)) { > + ret = -EFAULT; > + break; > + } > + > + remain -= page_length; > + user_data += page_length; > + offset += page_length; > + } > + > + mutex_lock(&dev->struct_mutex); > + WARN_ON(i915_gem_object_set_to_gtt_domain(obj, false)); This is user triggerable. It would take a silly user, but it is possible, so we can't WARN. if (ret == 0 && (obj->base.read_domains & I915_GEM_DOMAIN_GTT) == 0)) { /* The user has modified the object whilst we tried * reading from it, and we now have no idea what domain * the pages should be in. As we have just been touching * them directly, flush everything back to the GTT * domain. */ ret = i915_gem_object_set_to_gtt_domain(obj, false); } > @@ -1170,9 +1308,7 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data, > * pread/pwrite currently are reading and writing from the CPU > * perspective, requiring manual detiling by the client. > */ > - if (obj->tiling_mode == I915_TILING_NONE && > - obj->base.write_domain != I915_GEM_DOMAIN_CPU && > - cpu_write_needs_clflush(obj)) { > + if (!obj->base.filp || cpu_write_needs_clflush(obj)) { That's looking much tidier. > ret = i915_gem_gtt_pwrite_fast(dev_priv, obj, args, file); > /* Note that the gtt paths might fail with non-page-backed user > * pointers (e.g. gtt mappings when moving data between > @@ -1182,8 +1318,10 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data, > if (ret == -EFAULT || ret == -ENOSPC) { We don't return ENOSPC any more, so one less complication! > if (obj->phys_handle) > ret = i915_gem_phys_pwrite(obj, args, file); > - else > + else if (obj->base.filp) > ret = i915_gem_shmem_pwrite(dev, obj, args, file); > + else > + ret = -ENODEV; -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx