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

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

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@intel.com>
>> ---
>>   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