[Intel-gfx] [PATCH v2: 1/3] drm/i915: Add a function to mmap framebuffer obj

Das, Nirmoy nirmoy.das at linux.intel.com
Thu Mar 23 08:00:50 UTC 2023


On 3/20/2023 3:02 PM, Andrzej Hajda wrote:
> On 20.03.2023 11:09, Nirmoy Das wrote:
>> Implement i915_gem_fb_mmap() to enable fb_ops.fb_mmap()
>> callback for i915's framebuffer objects.
>>
>> v2: add a comment why i915_gem_object_get() needed(Andi).
>>
>> Cc: Matthew Auld <matthew.auld at intel.com>
>> Cc: Andi Shyti <andi.shyti at linux.intel.com>
>> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
>> Cc: Jani Nikula <jani.nikula at intel.com>
>> Cc: Imre Deak <imre.deak at intel.com>
>> Signed-off-by: Nirmoy Das <nirmoy.das at intel.com>
>> Reviewed-by: Andi Shyti <andi.shyti at linux.intel.com>
>
> Reviewed-by: Andrzej Hajda <andrzej.hajda at intel.com>


Thanks, Andrzej.


Going to resend it without RFC now as there are two r-bs and no one 
complained.


Regards,

Nirmoy

>
> Regards
> Andrzej
>
>> ---
>>   drivers/gpu/drm/i915/gem/i915_gem_mman.c | 127 +++++++++++++++--------
>>   drivers/gpu/drm/i915/gem/i915_gem_mman.h |   2 +-
>>   2 files changed, 83 insertions(+), 46 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c 
>> b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
>> index d3c1dee16af2..341e952d3510 100644
>> --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
>> @@ -927,53 +927,15 @@ static struct file *mmap_singleton(struct 
>> drm_i915_private *i915)
>>       return file;
>>   }
>>   -/*
>> - * This overcomes the limitation in drm_gem_mmap's assignment of a
>> - * drm_gem_object as the vma->vm_private_data. Since we need to
>> - * be able to resolve multiple mmap offsets which could be tied
>> - * to a single gem object.
>> - */
>> -int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma)
>> +static int
>> +i915_gem_object_mmap(struct drm_i915_gem_object *obj,
>> +             struct i915_mmap_offset *mmo,
>> +             struct vm_area_struct *vma)
>>   {
>> -    struct drm_vma_offset_node *node;
>> -    struct drm_file *priv = filp->private_data;
>> -    struct drm_device *dev = priv->minor->dev;
>> -    struct drm_i915_gem_object *obj = NULL;
>> -    struct i915_mmap_offset *mmo = NULL;
>> +    struct drm_i915_private *i915 = to_i915(obj->base.dev);
>> +    struct drm_device *dev = &i915->drm;
>>       struct file *anon;
>>   -    if (drm_dev_is_unplugged(dev))
>> -        return -ENODEV;
>> -
>> -    rcu_read_lock();
>> -    drm_vma_offset_lock_lookup(dev->vma_offset_manager);
>> -    node = drm_vma_offset_exact_lookup_locked(dev->vma_offset_manager,
>> -                          vma->vm_pgoff,
>> -                          vma_pages(vma));
>> -    if (node && drm_vma_node_is_allowed(node, priv)) {
>> -        /*
>> -         * Skip 0-refcnted objects as it is in the process of being
>> -         * destroyed and will be invalid when the vma manager lock
>> -         * is released.
>> -         */
>> -        if (!node->driver_private) {
>> -            mmo = container_of(node, struct i915_mmap_offset, 
>> vma_node);
>> -            obj = i915_gem_object_get_rcu(mmo->obj);
>> -
>> -            GEM_BUG_ON(obj && obj->ops->mmap_ops);
>> -        } else {
>> -            obj = i915_gem_object_get_rcu
>> -                (container_of(node, struct drm_i915_gem_object,
>> -                          base.vma_node));
>> -
>> -            GEM_BUG_ON(obj && !obj->ops->mmap_ops);
>> -        }
>> -    }
>> -    drm_vma_offset_unlock_lookup(dev->vma_offset_manager);
>> -    rcu_read_unlock();
>> -    if (!obj)
>> -        return node ? -EACCES : -EINVAL;
>> -
>>       if (i915_gem_object_is_readonly(obj)) {
>>           if (vma->vm_flags & VM_WRITE) {
>>               i915_gem_object_put(obj);
>> @@ -1005,7 +967,7 @@ int i915_gem_mmap(struct file *filp, struct 
>> vm_area_struct *vma)
>>       if (obj->ops->mmap_ops) {
>>           vma->vm_page_prot = 
>> pgprot_decrypted(vm_get_page_prot(vma->vm_flags));
>>           vma->vm_ops = obj->ops->mmap_ops;
>> -        vma->vm_private_data = node->driver_private;
>> +        vma->vm_private_data = obj->base.vma_node.driver_private;
>>           return 0;
>>       }
>>   @@ -1043,6 +1005,81 @@ int i915_gem_mmap(struct file *filp, struct 
>> vm_area_struct *vma)
>>       return 0;
>>   }
>>   +/*
>> + * This overcomes the limitation in drm_gem_mmap's assignment of a
>> + * drm_gem_object as the vma->vm_private_data. Since we need to
>> + * be able to resolve multiple mmap offsets which could be tied
>> + * to a single gem object.
>> + */
>> +int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma)
>> +{
>> +    struct drm_vma_offset_node *node;
>> +    struct drm_file *priv = filp->private_data;
>> +    struct drm_device *dev = priv->minor->dev;
>> +    struct drm_i915_gem_object *obj = NULL;
>> +    struct i915_mmap_offset *mmo = NULL;
>> +
>> +    if (drm_dev_is_unplugged(dev))
>> +        return -ENODEV;
>> +
>> +    rcu_read_lock();
>> +    drm_vma_offset_lock_lookup(dev->vma_offset_manager);
>> +    node = drm_vma_offset_exact_lookup_locked(dev->vma_offset_manager,
>> +                          vma->vm_pgoff,
>> +                          vma_pages(vma));
>> +    if (node && drm_vma_node_is_allowed(node, priv)) {
>> +        /*
>> +         * Skip 0-refcnted objects as it is in the process of being
>> +         * destroyed and will be invalid when the vma manager lock
>> +         * is released.
>> +         */
>> +        if (!node->driver_private) {
>> +            mmo = container_of(node, struct i915_mmap_offset, 
>> vma_node);
>> +            obj = i915_gem_object_get_rcu(mmo->obj);
>> +
>> +            GEM_BUG_ON(obj && obj->ops->mmap_ops);
>> +        } else {
>> +            obj = i915_gem_object_get_rcu
>> +                (container_of(node, struct drm_i915_gem_object,
>> +                          base.vma_node));
>> +
>> +            GEM_BUG_ON(obj && !obj->ops->mmap_ops);
>> +        }
>> +    }
>> +    drm_vma_offset_unlock_lookup(dev->vma_offset_manager);
>> +    rcu_read_unlock();
>> +    if (!obj)
>> +        return node ? -EACCES : -EINVAL;
>> +
>> +    return i915_gem_object_mmap(obj, mmo, vma);
>> +}
>> +
>> +int i915_gem_fb_mmap(struct drm_i915_gem_object *obj, struct 
>> vm_area_struct *vma)
>> +{
>> +    struct drm_i915_private *i915 = to_i915(obj->base.dev);
>> +    struct drm_device *dev = &i915->drm;
>> +    struct i915_mmap_offset *mmo = NULL;
>> +    enum i915_mmap_type mmap_type;
>> +    struct i915_ggtt *ggtt = to_gt(i915)->ggtt;
>> +
>> +    if (drm_dev_is_unplugged(dev))
>> +        return -ENODEV;
>> +
>> +    mmap_type = i915_ggtt_has_aperture(ggtt) ? I915_MMAP_TYPE_GTT : 
>> I915_MMAP_TYPE_WC;
>> +    mmo = mmap_offset_attach(obj, mmap_type, NULL);
>> +    if (!mmo)
>> +        return -ENODEV;
>> +
>> +    /*
>> +     * When we install vm_ops for mmap we are too late for
>> +     * the vm_ops->open() which increases the ref_count of
>> +     * this obj and then it gets decreased by the vm_ops->close().
>> +     * To balance this increase the obj ref_count here.
>> +     */
>> +    obj = i915_gem_object_get(mmo->obj);
>> +    return i915_gem_object_mmap(obj, mmo, vma);
>> +}
>> +
>>   #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..196417fd0f5c 100644
>> --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.h
>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
>> @@ -29,5 +29,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_fb_mmap(struct drm_i915_gem_object *obj, struct 
>> vm_area_struct *vma);
>>   #endif
>


More information about the dri-devel mailing list