Re: [PATCH 2/3] drm/i915: Add a helper func for gem obj mmap

["Das, Nirmoy" <nirmoy.das@xxxxxxxxx>]

On 3/6/2023 3:26 PM, Ville Syrjälä wrote:
> On Mon, Mar 06, 2023 at 11:28:49AM +0100, Nirmoy Das wrote:
> > Move gem obj mmap code to i915_gem_object_mmap() so that
> > this can be used by others.
> >
> > Signed-off-by: Nirmoy Das <nirmoy.das@xxxxxxxxx>
> > ---
> >   drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 20 ++---------------
> >   drivers/gpu/drm/i915/gem/i915_gem_mman.c   | 25 ++++++++++++++++++++++
> >   drivers/gpu/drm/i915/gem/i915_gem_mman.h   |  1 +
> >   3 files changed, 28 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
> > index fd556a076d05..831dd8ebf819 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
> > @@ -12,6 +12,7 @@
> >   #include <asm/smp.h>
> >   
> >   #include "gem/i915_gem_dmabuf.h"
> > +#include "gem/i915_gem_mman.h"
> >   #include "i915_drv.h"
> >   #include "i915_gem_object.h"
> >   #include "i915_scatterlist.h"
> > @@ -94,27 +95,10 @@ static void i915_gem_dmabuf_vunmap(struct dma_buf *dma_buf,
> >   static int i915_gem_dmabuf_mmap(struct dma_buf *dma_buf, struct vm_area_struct *vma)
> >   {
> >   	struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf);
> > -	struct drm_i915_private *i915 = to_i915(obj->base.dev);
> > -	int ret;
> >   
> >   	dma_resv_assert_held(dma_buf->resv);
> >   
> > -	if (obj->base.size < vma->vm_end - vma->vm_start)
> > -		return -EINVAL;
> > -
> > -	if (HAS_LMEM(i915))
> > -		return drm_gem_prime_mmap(&obj->base, vma);
> > -
> > -	if (!obj->base.filp)
> > -		return -ENODEV;
> > -
> > -	ret = call_mmap(obj->base.filp, vma);
> > -	if (ret)
> > -		return ret;
> > -
> > -	vma_set_file(vma, obj->base.filp);
> > -
> > -	return 0;
> > +	return i915_gem_object_mmap(obj, vma);
> >   }
> >   
> >   static int i915_gem_begin_cpu_access(struct dma_buf *dma_buf, enum dma_data_direction direction)
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> > index 2aac6bf78740..d378720ca626 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> > @@ -11,6 +11,8 @@
> >   
> >   #include <drm/drm_cache.h>
> >   
> > +#include "gem/i915_gem_lmem.h"
> > +
> >   #include "gt/intel_gt.h"
> >   #include "gt/intel_gt_requests.h"
> >   
> > @@ -1043,6 +1045,29 @@ int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma)
> >   	return 0;
> >   }
> >   
> > +int i915_gem_object_mmap(struct drm_i915_gem_object *obj, struct vm_area_struct *vma)
> > +{
> > +	struct drm_i915_private *i915 = to_i915(obj->base.dev);
> > +	int ret;
> > +
> > +	if (obj->base.size < vma->vm_end - vma->vm_start)
> > +		return -EINVAL;
> > +
> > +	if (HAS_LMEM(i915))
> > +		return drm_gem_prime_mmap(&obj->base, vma);
> Calling some prime stuff here doesn't smell right.

Yes, I should use drm_gem_mmap_obj() here.


> > +
> > +	if (obj->base.filp) {
> > +		ret = call_mmap(obj->base.filp, vma);
> > +		if (ret)
> > +			return ret;
> > +
> > +		vma_set_file(vma, obj->base.filp);
> > +		return 0;
> > +	}
> > +
> > +	return -ENODEV;
> > +}
> > +
> >   #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
> >   #include "selftests/i915_gem_mman.c"
> >   #endif
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.h b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
> > index 1fa91b3033b3..303e81ddc5ba 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.h
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
> > @@ -30,4 +30,5 @@ void i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj);
> >   void i915_gem_object_runtime_pm_release_mmap_offset(struct drm_i915_gem_object *obj);
> >   void i915_gem_object_release_mmap_offset(struct drm_i915_gem_object *obj);
> >   
> > +int i915_gem_object_mmap(struct drm_i915_gem_object *obj, struct vm_area_struct *vma);
> >   #endif
> > --
> > 2.39.0