| Message ID | 1438812791-5857-4-git-send-email-tiago.vignatti@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Aug 05, 2015 at 07:13:11PM -0300, Tiago Vignatti wrote: > Userspace is the one in charge of flush CPU by wrapping mmap with > begin{,end}_cpu_access. > > v2: Remove LLC check cause we have dma-buf sync providers now. Also, fix return > before transferring ownership when mmap fails. i915 doesn't implement end_cpu_access() as required for your sync ioctl. > Signed-off-by: Tiago Vignatti <tiago.vignatti@intel.com> > --- > drivers/gpu/drm/i915/i915_gem_dmabuf.c | 18 +++++++++++++++++- > 1 file changed, 17 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/i915_gem_dmabuf.c > index e9c2bfd..b608f67 100644 > --- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c > +++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c > @@ -193,7 +193,23 @@ static void i915_gem_dmabuf_kunmap(struct dma_buf *dma_buf, unsigned long page_n > > static int i915_gem_dmabuf_mmap(struct dma_buf *dma_buf, struct vm_area_struct *vma) > { > - return -EINVAL; > + struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf); > + int ret; > + > + if (obj->base.size < vma->vm_end - vma->vm_start) > + return -EINVAL; > + > + if (WARN_ON(!obj->base.filp)) > + return -EINVAL; So long as Linus is not watching, ENODEV - since we could support it, but we just have not implemented it yet. > + ret = obj->base.filp->f_op->mmap(obj->base.filp, vma); > + if (IS_ERR_VALUE(ret)) > + return -EINVAL; The return is 0 on success or error otherwise. Always propagate the original error unless you have good reason not to (in which case you document it). So if (ret) return ret; > + > + fput(vma->vm_file); > + vma->vm_file = get_file(obj->base.filp); > + > + return ret; return 0; > } > > static int i915_gem_begin_cpu_access(struct dma_buf *dma_buf, size_t start, size_t length, enum dma_data_direction direction) > -- > 2.1.0 >
On Thu, Aug 06, 2015 at 10:09:37AM +0100, Chris Wilson wrote: > On Wed, Aug 05, 2015 at 07:13:11PM -0300, Tiago Vignatti wrote: > > Userspace is the one in charge of flush CPU by wrapping mmap with > > begin{,end}_cpu_access. > > > > v2: Remove LLC check cause we have dma-buf sync providers now. Also, fix return > > before transferring ownership when mmap fails. > > i915 doesn't implement end_cpu_access() as required for your sync ioctl. An important point here is that performance on !llc (and for certain objects with llc) will be absymal, utterly absymal. So what's the rationale for this compared to doing the opposite and mapping the client memory (a normal mmap) into the GPU with llc/snooping. -Chris
diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/i915_gem_dmabuf.c index e9c2bfd..b608f67 100644 --- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c +++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c @@ -193,7 +193,23 @@ static void i915_gem_dmabuf_kunmap(struct dma_buf *dma_buf, unsigned long page_n static int i915_gem_dmabuf_mmap(struct dma_buf *dma_buf, struct vm_area_struct *vma) { - return -EINVAL; + struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf); + int ret; + + if (obj->base.size < vma->vm_end - vma->vm_start) + return -EINVAL; + + if (WARN_ON(!obj->base.filp)) + return -EINVAL; + + ret = obj->base.filp->f_op->mmap(obj->base.filp, vma); + if (IS_ERR_VALUE(ret)) + return -EINVAL; + + fput(vma->vm_file); + vma->vm_file = get_file(obj->base.filp); + + return ret; } static int i915_gem_begin_cpu_access(struct dma_buf *dma_buf, size_t start, size_t length, enum dma_data_direction direction)
Userspace is the one in charge of flush CPU by wrapping mmap with begin{,end}_cpu_access. v2: Remove LLC check cause we have dma-buf sync providers now. Also, fix return before transferring ownership when mmap fails. Signed-off-by: Tiago Vignatti <tiago.vignatti@intel.com> --- drivers/gpu/drm/i915/i915_gem_dmabuf.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)