* Re: [PATCH 2/2] drm/exynos: use drm generic mmap interface
2014-09-17 13:48 ` [PATCH 2/2] drm/exynos: use drm generic mmap interface Inki Dae
@ 2014-09-18 4:56 ` Joonyoung Shim
2014-09-18 5:15 ` Inki Dae
2014-09-18 5:19 ` [PATCH v2] " Inki Dae
2014-10-01 13:17 ` [PATCH 2/2] " Tomasz Figa
2 siblings, 1 reply; 16+ messages in thread
From: Joonyoung Shim @ 2014-09-18 4:56 UTC (permalink / raw)
To: Inki Dae, dri-devel; +Cc: linux-samsung-soc
Hi,
On 09/17/2014 10:48 PM, Inki Dae wrote:
> This patch removes DRM_EXYNOS_GEM_MMAP ictrl feature specific
> to Exynos drm and instead uses drm generic mmap.
>
> We had used the interface specific to Exynos drm to do mmap directly,
> not to use demand paging which maps each page with physical memory
> at page fault handler. We don't need the specific mmap interface
> because the drm generic mmap which uses vm offset manager stuff can
> also do mmap directly.
>
> This patch makes a userspace region to be mapped with whole physical
> memory region allocated by userspace request when mmap system call is
> requested.
>
> Signed-off-by: Inki Dae <inki.dae@samsung.com>
> ---
> drivers/gpu/drm/exynos/exynos_drm_drv.c | 25 ---------
> drivers/gpu/drm/exynos/exynos_drm_drv.h | 1 -
> drivers/gpu/drm/exynos/exynos_drm_gem.c | 84 ++++++-------------------------
> drivers/gpu/drm/exynos/exynos_drm_gem.h | 10 ----
> include/uapi/drm/exynos_drm.h | 22 --------
> 5 files changed, 14 insertions(+), 128 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index 10ad3d4..a7819eb 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
Because anymore anon file doesn't use, you can remove to include
<linux/anon_inodes.h>.
> @@ -149,10 +149,6 @@ static int exynos_drm_unload(struct drm_device *dev)
> return 0;
> }
>
> -static const struct file_operations exynos_drm_gem_fops = {
> - .mmap = exynos_drm_gem_mmap_buffer,
> -};
> -
> static int exynos_drm_suspend(struct drm_device *dev, pm_message_t state)
> {
> struct drm_connector *connector;
> @@ -191,7 +187,6 @@ static int exynos_drm_resume(struct drm_device *dev)
> static int exynos_drm_open(struct drm_device *dev, struct drm_file *file)
> {
> struct drm_exynos_file_private *file_priv;
> - struct file *anon_filp;
> int ret;
>
> file_priv = kzalloc(sizeof(*file_priv), GFP_KERNEL);
> @@ -204,21 +199,8 @@ static int exynos_drm_open(struct drm_device *dev, struct drm_file *file)
> if (ret)
> goto err_file_priv_free;
>
> - anon_filp = anon_inode_getfile("exynos_gem", &exynos_drm_gem_fops,
> - NULL, 0);
> - if (IS_ERR(anon_filp)) {
> - ret = PTR_ERR(anon_filp);
> - goto err_subdrv_close;
> - }
> -
> - anon_filp->f_mode = FMODE_READ | FMODE_WRITE;
> - file_priv->anon_filp = anon_filp;
> -
> return ret;
>
> -err_subdrv_close:
> - exynos_drm_subdrv_close(dev, file);
> -
> err_file_priv_free:
> kfree(file_priv);
> file->driver_priv = NULL;
> @@ -234,7 +216,6 @@ static void exynos_drm_preclose(struct drm_device *dev,
> static void exynos_drm_postclose(struct drm_device *dev, struct drm_file *file)
> {
> struct exynos_drm_private *private = dev->dev_private;
> - struct drm_exynos_file_private *file_priv;
> struct drm_pending_vblank_event *v, *vt;
> struct drm_pending_event *e, *et;
> unsigned long flags;
> @@ -260,10 +241,6 @@ static void exynos_drm_postclose(struct drm_device *dev, struct drm_file *file)
> }
> spin_unlock_irqrestore(&dev->event_lock, flags);
>
> - file_priv = file->driver_priv;
> - if (file_priv->anon_filp)
> - fput(file_priv->anon_filp);
> -
> kfree(file->driver_priv);
> file->driver_priv = NULL;
> }
> @@ -282,8 +259,6 @@ static const struct vm_operations_struct exynos_drm_gem_vm_ops = {
> static const struct drm_ioctl_desc exynos_ioctls[] = {
> DRM_IOCTL_DEF_DRV(EXYNOS_GEM_CREATE, exynos_drm_gem_create_ioctl,
> DRM_UNLOCKED | DRM_AUTH),
> - DRM_IOCTL_DEF_DRV(EXYNOS_GEM_MMAP,
> - exynos_drm_gem_mmap_ioctl, DRM_UNLOCKED | DRM_AUTH),
> DRM_IOCTL_DEF_DRV(EXYNOS_GEM_GET,
> exynos_drm_gem_get_ioctl, DRM_UNLOCKED),
> DRM_IOCTL_DEF_DRV(EXYNOS_VIDI_CONNECTION,
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
> index 69a6fa3..d22e640 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
> @@ -240,7 +240,6 @@ struct exynos_drm_g2d_private {
> struct drm_exynos_file_private {
> struct exynos_drm_g2d_private *g2d_priv;
> struct device *ipp_dev;
> - struct file *anon_filp;
> };
>
> /*
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> index 2f3665d..3cf6494 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> @@ -318,21 +318,17 @@ void exynos_drm_gem_put_dma_addr(struct drm_device *dev,
> drm_gem_object_unreference_unlocked(obj);
> }
>
> -int exynos_drm_gem_mmap_buffer(struct file *filp,
> +int exynos_drm_gem_mmap_buffer(struct exynos_drm_gem_obj *exynos_gem_obj,
> struct vm_area_struct *vma)
> {
> - struct drm_gem_object *obj = filp->private_data;
> - struct exynos_drm_gem_obj *exynos_gem_obj = to_exynos_gem_obj(obj);
> - struct drm_device *drm_dev = obj->dev;
> + struct drm_device *drm_dev = exynos_gem_obj->base.dev;
> struct exynos_drm_gem_buf *buffer;
> unsigned long vm_size;
> int ret;
>
> - WARN_ON(!mutex_is_locked(&obj->dev->struct_mutex));
> -
> vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
Above flags already set from drm_gem_mmap.
Thanks.
> - vma->vm_private_data = obj;
> - vma->vm_ops = drm_dev->driver->gem_vm_ops;
> + vma->vm_flags &= ~VM_PFNMAP;
> + vma->vm_pgoff = 0;
>
> update_vm_cache_attr(exynos_gem_obj, vma);
>
> @@ -356,60 +352,6 @@ int exynos_drm_gem_mmap_buffer(struct file *filp,
> return ret;
> }
>
> - /*
> - * take a reference to this mapping of the object. And this reference
> - * is unreferenced by the corresponding vm_close call.
> - */
> - drm_gem_object_reference(obj);
> -
> - drm_vm_open_locked(drm_dev, vma);
> -
> - return 0;
> -}
> -
> -int exynos_drm_gem_mmap_ioctl(struct drm_device *dev, void *data,
> - struct drm_file *file_priv)
> -{
> - struct drm_exynos_file_private *exynos_file_priv;
> - struct drm_exynos_gem_mmap *args = data;
> - struct drm_gem_object *obj;
> - struct file *anon_filp;
> - unsigned long addr;
> -
> - if (!(dev->driver->driver_features & DRIVER_GEM)) {
> - DRM_ERROR("does not support GEM.\n");
> - return -ENODEV;
> - }
> -
> - mutex_lock(&dev->struct_mutex);
> -
> - obj = drm_gem_object_lookup(dev, file_priv, args->handle);
> - if (!obj) {
> - DRM_ERROR("failed to lookup gem object.\n");
> - mutex_unlock(&dev->struct_mutex);
> - return -EINVAL;
> - }
> -
> - exynos_file_priv = file_priv->driver_priv;
> - anon_filp = exynos_file_priv->anon_filp;
> - anon_filp->private_data = obj;
> -
> - addr = vm_mmap(anon_filp, 0, args->size, PROT_READ | PROT_WRITE,
> - MAP_SHARED, 0);
> -
> - drm_gem_object_unreference(obj);
> -
> - if (IS_ERR_VALUE(addr)) {
> - mutex_unlock(&dev->struct_mutex);
> - return (int)addr;
> - }
> -
> - mutex_unlock(&dev->struct_mutex);
> -
> - args->mapped = addr;
> -
> - DRM_DEBUG_KMS("mapped = 0x%lx\n", (unsigned long)args->mapped);
> -
> return 0;
> }
>
> @@ -693,16 +635,18 @@ int exynos_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
> exynos_gem_obj = to_exynos_gem_obj(obj);
>
> ret = check_gem_flags(exynos_gem_obj->flags);
> - if (ret) {
> - drm_gem_vm_close(vma);
> - drm_gem_free_mmap_offset(obj);
> - return ret;
> - }
> + if (ret)
> + goto err_close_vm;
>
> - vma->vm_flags &= ~VM_PFNMAP;
> - vma->vm_flags |= VM_MIXEDMAP;
> + ret = exynos_drm_gem_mmap_buffer(exynos_gem_obj, vma);
> + if (ret)
> + goto err_close_vm;
>
> - update_vm_cache_attr(exynos_gem_obj, vma);
> + return ret;
> +
> +err_close_vm:
> + drm_gem_vm_close(vma);
> + drm_gem_free_mmap_offset(obj);
>
> return ret;
> }
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.h b/drivers/gpu/drm/exynos/exynos_drm_gem.h
> index 8e46094..09d021b 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.h
> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.h
> @@ -111,16 +111,6 @@ void exynos_drm_gem_put_dma_addr(struct drm_device *dev,
> unsigned int gem_handle,
> struct drm_file *filp);
>
> -/*
> - * mmap the physically continuous memory that a gem object contains
> - * to user space.
> - */
> -int exynos_drm_gem_mmap_ioctl(struct drm_device *dev, void *data,
> - struct drm_file *file_priv);
> -
> -int exynos_drm_gem_mmap_buffer(struct file *filp,
> - struct vm_area_struct *vma);
> -
> /* map user space allocated by malloc to pages. */
> int exynos_drm_gem_userptr_ioctl(struct drm_device *dev, void *data,
> struct drm_file *file_priv);
> diff --git a/include/uapi/drm/exynos_drm.h b/include/uapi/drm/exynos_drm.h
> index 67a751c..5575ed1 100644
> --- a/include/uapi/drm/exynos_drm.h
> +++ b/include/uapi/drm/exynos_drm.h
> @@ -33,24 +33,6 @@ struct drm_exynos_gem_create {
> };
>
> /**
> - * A structure for mapping buffer.
> - *
> - * @handle: a handle to gem object created.
> - * @pad: just padding to be 64-bit aligned.
> - * @size: memory size to be mapped.
> - * @mapped: having user virtual address mmaped.
> - * - this variable would be filled by exynos gem module
> - * of kernel side with user virtual address which is allocated
> - * by do_mmap().
> - */
> -struct drm_exynos_gem_mmap {
> - unsigned int handle;
> - unsigned int pad;
> - uint64_t size;
> - uint64_t mapped;
> -};
> -
> -/**
> * A structure to gem information.
> *
> * @handle: a handle to gem object created.
> @@ -302,7 +284,6 @@ struct drm_exynos_ipp_cmd_ctrl {
> };
>
> #define DRM_EXYNOS_GEM_CREATE 0x00
> -#define DRM_EXYNOS_GEM_MMAP 0x02
> /* Reserved 0x03 ~ 0x05 for exynos specific gem ioctl */
> #define DRM_EXYNOS_GEM_GET 0x04
> #define DRM_EXYNOS_VIDI_CONNECTION 0x07
> @@ -321,9 +302,6 @@ struct drm_exynos_ipp_cmd_ctrl {
> #define DRM_IOCTL_EXYNOS_GEM_CREATE DRM_IOWR(DRM_COMMAND_BASE + \
> DRM_EXYNOS_GEM_CREATE, struct drm_exynos_gem_create)
>
> -#define DRM_IOCTL_EXYNOS_GEM_MMAP DRM_IOWR(DRM_COMMAND_BASE + \
> - DRM_EXYNOS_GEM_MMAP, struct drm_exynos_gem_mmap)
> -
> #define DRM_IOCTL_EXYNOS_GEM_GET DRM_IOWR(DRM_COMMAND_BASE + \
> DRM_EXYNOS_GEM_GET, struct drm_exynos_gem_info)
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 2/2] drm/exynos: use drm generic mmap interface
2014-09-18 4:56 ` Joonyoung Shim
@ 2014-09-18 5:15 ` Inki Dae
0 siblings, 0 replies; 16+ messages in thread
From: Inki Dae @ 2014-09-18 5:15 UTC (permalink / raw)
To: Joonyoung Shim; +Cc: linux-samsung-soc, dri-devel
Thanks for review.
Below trivial things you pointed out will be fixed soon.
On 2014년 09월 18일 13:56, Joonyoung Shim wrote:
> Hi,
>
> On 09/17/2014 10:48 PM, Inki Dae wrote:
>> This patch removes DRM_EXYNOS_GEM_MMAP ictrl feature specific
>> to Exynos drm and instead uses drm generic mmap.
>>
>> We had used the interface specific to Exynos drm to do mmap directly,
>> not to use demand paging which maps each page with physical memory
>> at page fault handler. We don't need the specific mmap interface
>> because the drm generic mmap which uses vm offset manager stuff can
>> also do mmap directly.
>>
>> This patch makes a userspace region to be mapped with whole physical
>> memory region allocated by userspace request when mmap system call is
>> requested.
>>
>> Signed-off-by: Inki Dae <inki.dae@samsung.com>
>> ---
>> drivers/gpu/drm/exynos/exynos_drm_drv.c | 25 ---------
>> drivers/gpu/drm/exynos/exynos_drm_drv.h | 1 -
>> drivers/gpu/drm/exynos/exynos_drm_gem.c | 84 ++++++-------------------------
>> drivers/gpu/drm/exynos/exynos_drm_gem.h | 10 ----
>> include/uapi/drm/exynos_drm.h | 22 --------
>> 5 files changed, 14 insertions(+), 128 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
>> index 10ad3d4..a7819eb 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
>
> Because anymore anon file doesn't use, you can remove to include
> <linux/anon_inodes.h>.
Right, I missed it while taking away the direct mmap stuff specific to
Exynos drm.
>
>> @@ -149,10 +149,6 @@ static int exynos_drm_unload(struct drm_device *dev)
>> return 0;
>> }
>>
>> -static const struct file_operations exynos_drm_gem_fops = {
>> - .mmap = exynos_drm_gem_mmap_buffer,
>> -};
>> -
>> static int exynos_drm_suspend(struct drm_device *dev, pm_message_t state)
>> {
>> struct drm_connector *connector;
>> @@ -191,7 +187,6 @@ static int exynos_drm_resume(struct drm_device *dev)
>> static int exynos_drm_open(struct drm_device *dev, struct drm_file *file)
>> {
>> struct drm_exynos_file_private *file_priv;
>> - struct file *anon_filp;
>> int ret;
>>
>> file_priv = kzalloc(sizeof(*file_priv), GFP_KERNEL);
>> @@ -204,21 +199,8 @@ static int exynos_drm_open(struct drm_device *dev, struct drm_file *file)
>> if (ret)
>> goto err_file_priv_free;
>>
>> - anon_filp = anon_inode_getfile("exynos_gem", &exynos_drm_gem_fops,
>> - NULL, 0);
>> - if (IS_ERR(anon_filp)) {
>> - ret = PTR_ERR(anon_filp);
>> - goto err_subdrv_close;
>> - }
>> -
>> - anon_filp->f_mode = FMODE_READ | FMODE_WRITE;
>> - file_priv->anon_filp = anon_filp;
>> -
>> return ret;
>>
>> -err_subdrv_close:
>> - exynos_drm_subdrv_close(dev, file);
>> -
>> err_file_priv_free:
>> kfree(file_priv);
>> file->driver_priv = NULL;
>> @@ -234,7 +216,6 @@ static void exynos_drm_preclose(struct drm_device *dev,
>> static void exynos_drm_postclose(struct drm_device *dev, struct drm_file *file)
>> {
>> struct exynos_drm_private *private = dev->dev_private;
>> - struct drm_exynos_file_private *file_priv;
>> struct drm_pending_vblank_event *v, *vt;
>> struct drm_pending_event *e, *et;
>> unsigned long flags;
>> @@ -260,10 +241,6 @@ static void exynos_drm_postclose(struct drm_device *dev, struct drm_file *file)
>> }
>> spin_unlock_irqrestore(&dev->event_lock, flags);
>>
>> - file_priv = file->driver_priv;
>> - if (file_priv->anon_filp)
>> - fput(file_priv->anon_filp);
>> -
>> kfree(file->driver_priv);
>> file->driver_priv = NULL;
>> }
>> @@ -282,8 +259,6 @@ static const struct vm_operations_struct exynos_drm_gem_vm_ops = {
>> static const struct drm_ioctl_desc exynos_ioctls[] = {
>> DRM_IOCTL_DEF_DRV(EXYNOS_GEM_CREATE, exynos_drm_gem_create_ioctl,
>> DRM_UNLOCKED | DRM_AUTH),
>> - DRM_IOCTL_DEF_DRV(EXYNOS_GEM_MMAP,
>> - exynos_drm_gem_mmap_ioctl, DRM_UNLOCKED | DRM_AUTH),
>> DRM_IOCTL_DEF_DRV(EXYNOS_GEM_GET,
>> exynos_drm_gem_get_ioctl, DRM_UNLOCKED),
>> DRM_IOCTL_DEF_DRV(EXYNOS_VIDI_CONNECTION,
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
>> index 69a6fa3..d22e640 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
>> @@ -240,7 +240,6 @@ struct exynos_drm_g2d_private {
>> struct drm_exynos_file_private {
>> struct exynos_drm_g2d_private *g2d_priv;
>> struct device *ipp_dev;
>> - struct file *anon_filp;
>> };
>>
>> /*
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
>> index 2f3665d..3cf6494 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
>> @@ -318,21 +318,17 @@ void exynos_drm_gem_put_dma_addr(struct drm_device *dev,
>> drm_gem_object_unreference_unlocked(obj);
>> }
>>
>> -int exynos_drm_gem_mmap_buffer(struct file *filp,
>> +int exynos_drm_gem_mmap_buffer(struct exynos_drm_gem_obj *exynos_gem_obj,
>> struct vm_area_struct *vma)
>> {
>> - struct drm_gem_object *obj = filp->private_data;
>> - struct exynos_drm_gem_obj *exynos_gem_obj = to_exynos_gem_obj(obj);
>> - struct drm_device *drm_dev = obj->dev;
>> + struct drm_device *drm_dev = exynos_gem_obj->base.dev;
>> struct exynos_drm_gem_buf *buffer;
>> unsigned long vm_size;
>> int ret;
>>
>> - WARN_ON(!mutex_is_locked(&obj->dev->struct_mutex));
>> -
>> vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
>
> Above flags already set from drm_gem_mmap.
>
Right. Actually it is done at drm_mmap_locked function.
Thanks,
Inki Dae
> Thanks.
>
>> - vma->vm_private_data = obj;
>> - vma->vm_ops = drm_dev->driver->gem_vm_ops;
>> + vma->vm_flags &= ~VM_PFNMAP;
>> + vma->vm_pgoff = 0;
>>
>> update_vm_cache_attr(exynos_gem_obj, vma);
>>
>> @@ -356,60 +352,6 @@ int exynos_drm_gem_mmap_buffer(struct file *filp,
>> return ret;
>> }
>>
>> - /*
>> - * take a reference to this mapping of the object. And this reference
>> - * is unreferenced by the corresponding vm_close call.
>> - */
>> - drm_gem_object_reference(obj);
>> -
>> - drm_vm_open_locked(drm_dev, vma);
>> -
>> - return 0;
>> -}
>> -
>> -int exynos_drm_gem_mmap_ioctl(struct drm_device *dev, void *data,
>> - struct drm_file *file_priv)
>> -{
>> - struct drm_exynos_file_private *exynos_file_priv;
>> - struct drm_exynos_gem_mmap *args = data;
>> - struct drm_gem_object *obj;
>> - struct file *anon_filp;
>> - unsigned long addr;
>> -
>> - if (!(dev->driver->driver_features & DRIVER_GEM)) {
>> - DRM_ERROR("does not support GEM.\n");
>> - return -ENODEV;
>> - }
>> -
>> - mutex_lock(&dev->struct_mutex);
>> -
>> - obj = drm_gem_object_lookup(dev, file_priv, args->handle);
>> - if (!obj) {
>> - DRM_ERROR("failed to lookup gem object.\n");
>> - mutex_unlock(&dev->struct_mutex);
>> - return -EINVAL;
>> - }
>> -
>> - exynos_file_priv = file_priv->driver_priv;
>> - anon_filp = exynos_file_priv->anon_filp;
>> - anon_filp->private_data = obj;
>> -
>> - addr = vm_mmap(anon_filp, 0, args->size, PROT_READ | PROT_WRITE,
>> - MAP_SHARED, 0);
>> -
>> - drm_gem_object_unreference(obj);
>> -
>> - if (IS_ERR_VALUE(addr)) {
>> - mutex_unlock(&dev->struct_mutex);
>> - return (int)addr;
>> - }
>> -
>> - mutex_unlock(&dev->struct_mutex);
>> -
>> - args->mapped = addr;
>> -
>> - DRM_DEBUG_KMS("mapped = 0x%lx\n", (unsigned long)args->mapped);
>> -
>> return 0;
>> }
>>
>> @@ -693,16 +635,18 @@ int exynos_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
>> exynos_gem_obj = to_exynos_gem_obj(obj);
>>
>> ret = check_gem_flags(exynos_gem_obj->flags);
>> - if (ret) {
>> - drm_gem_vm_close(vma);
>> - drm_gem_free_mmap_offset(obj);
>> - return ret;
>> - }
>> + if (ret)
>> + goto err_close_vm;
>>
>> - vma->vm_flags &= ~VM_PFNMAP;
>> - vma->vm_flags |= VM_MIXEDMAP;
>> + ret = exynos_drm_gem_mmap_buffer(exynos_gem_obj, vma);
>> + if (ret)
>> + goto err_close_vm;
>>
>> - update_vm_cache_attr(exynos_gem_obj, vma);
>> + return ret;
>> +
>> +err_close_vm:
>> + drm_gem_vm_close(vma);
>> + drm_gem_free_mmap_offset(obj);
>>
>> return ret;
>> }
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.h b/drivers/gpu/drm/exynos/exynos_drm_gem.h
>> index 8e46094..09d021b 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.h
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.h
>> @@ -111,16 +111,6 @@ void exynos_drm_gem_put_dma_addr(struct drm_device *dev,
>> unsigned int gem_handle,
>> struct drm_file *filp);
>>
>> -/*
>> - * mmap the physically continuous memory that a gem object contains
>> - * to user space.
>> - */
>> -int exynos_drm_gem_mmap_ioctl(struct drm_device *dev, void *data,
>> - struct drm_file *file_priv);
>> -
>> -int exynos_drm_gem_mmap_buffer(struct file *filp,
>> - struct vm_area_struct *vma);
>> -
>> /* map user space allocated by malloc to pages. */
>> int exynos_drm_gem_userptr_ioctl(struct drm_device *dev, void *data,
>> struct drm_file *file_priv);
>> diff --git a/include/uapi/drm/exynos_drm.h b/include/uapi/drm/exynos_drm.h
>> index 67a751c..5575ed1 100644
>> --- a/include/uapi/drm/exynos_drm.h
>> +++ b/include/uapi/drm/exynos_drm.h
>> @@ -33,24 +33,6 @@ struct drm_exynos_gem_create {
>> };
>>
>> /**
>> - * A structure for mapping buffer.
>> - *
>> - * @handle: a handle to gem object created.
>> - * @pad: just padding to be 64-bit aligned.
>> - * @size: memory size to be mapped.
>> - * @mapped: having user virtual address mmaped.
>> - * - this variable would be filled by exynos gem module
>> - * of kernel side with user virtual address which is allocated
>> - * by do_mmap().
>> - */
>> -struct drm_exynos_gem_mmap {
>> - unsigned int handle;
>> - unsigned int pad;
>> - uint64_t size;
>> - uint64_t mapped;
>> -};
>> -
>> -/**
>> * A structure to gem information.
>> *
>> * @handle: a handle to gem object created.
>> @@ -302,7 +284,6 @@ struct drm_exynos_ipp_cmd_ctrl {
>> };
>>
>> #define DRM_EXYNOS_GEM_CREATE 0x00
>> -#define DRM_EXYNOS_GEM_MMAP 0x02
>> /* Reserved 0x03 ~ 0x05 for exynos specific gem ioctl */
>> #define DRM_EXYNOS_GEM_GET 0x04
>> #define DRM_EXYNOS_VIDI_CONNECTION 0x07
>> @@ -321,9 +302,6 @@ struct drm_exynos_ipp_cmd_ctrl {
>> #define DRM_IOCTL_EXYNOS_GEM_CREATE DRM_IOWR(DRM_COMMAND_BASE + \
>> DRM_EXYNOS_GEM_CREATE, struct drm_exynos_gem_create)
>>
>> -#define DRM_IOCTL_EXYNOS_GEM_MMAP DRM_IOWR(DRM_COMMAND_BASE + \
>> - DRM_EXYNOS_GEM_MMAP, struct drm_exynos_gem_mmap)
>> -
>> #define DRM_IOCTL_EXYNOS_GEM_GET DRM_IOWR(DRM_COMMAND_BASE + \
>> DRM_EXYNOS_GEM_GET, struct drm_exynos_gem_info)
>>
>>
>
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2] drm/exynos: use drm generic mmap interface
2014-09-17 13:48 ` [PATCH 2/2] drm/exynos: use drm generic mmap interface Inki Dae
2014-09-18 4:56 ` Joonyoung Shim
@ 2014-09-18 5:19 ` Inki Dae
2014-10-01 13:17 ` [PATCH 2/2] " Tomasz Figa
2 siblings, 0 replies; 16+ messages in thread
From: Inki Dae @ 2014-09-18 5:19 UTC (permalink / raw)
To: dri-devel; +Cc: airlied, daniel, linux-samsung-soc, Inki Dae
This patch removes DRM_EXYNOS_GEM_MMAP ictrl feature specific
to Exynos drm and instead uses drm generic mmap.
We had used the interface specific to Exynos drm to do mmap directly,
not to use demand paging which maps each page with physical memory
at page fault handler. We don't need the specific mmap interface
because the drm generic mmap which uses vm offset manager stuff can
also do mmap directly.
This patch makes a userspace region to be mapped with whole physical
memory region allocated by userspace request when mmap system call is
requested.
Changelog v2:
- do not set VM_IO, VM_DONTEXPEND and VM_DONTDUMP. These flags were already
set by drm_gem_mmap
- do not include <linux/anon_inodes.h>, which isn't needed anymore.
Signed-off-by: Inki Dae <inki.dae@samsung.com>
---
drivers/gpu/drm/exynos/exynos_drm_drv.c | 26 ---------
drivers/gpu/drm/exynos/exynos_drm_drv.h | 1 -
drivers/gpu/drm/exynos/exynos_drm_gem.c | 89 ++++++-------------------------
drivers/gpu/drm/exynos/exynos_drm_gem.h | 10 ----
include/uapi/drm/exynos_drm.h | 22 --------
5 files changed, 16 insertions(+), 132 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index 10ad3d4..928c514 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -15,7 +15,6 @@
#include <drm/drmP.h>
#include <drm/drm_crtc_helper.h>
-#include <linux/anon_inodes.h>
#include <linux/component.h>
#include <drm/exynos_drm.h>
@@ -149,10 +148,6 @@ static int exynos_drm_unload(struct drm_device *dev)
return 0;
}
-static const struct file_operations exynos_drm_gem_fops = {
- .mmap = exynos_drm_gem_mmap_buffer,
-};
-
static int exynos_drm_suspend(struct drm_device *dev, pm_message_t state)
{
struct drm_connector *connector;
@@ -191,7 +186,6 @@ static int exynos_drm_resume(struct drm_device *dev)
static int exynos_drm_open(struct drm_device *dev, struct drm_file *file)
{
struct drm_exynos_file_private *file_priv;
- struct file *anon_filp;
int ret;
file_priv = kzalloc(sizeof(*file_priv), GFP_KERNEL);
@@ -204,21 +198,8 @@ static int exynos_drm_open(struct drm_device *dev, struct drm_file *file)
if (ret)
goto err_file_priv_free;
- anon_filp = anon_inode_getfile("exynos_gem", &exynos_drm_gem_fops,
- NULL, 0);
- if (IS_ERR(anon_filp)) {
- ret = PTR_ERR(anon_filp);
- goto err_subdrv_close;
- }
-
- anon_filp->f_mode = FMODE_READ | FMODE_WRITE;
- file_priv->anon_filp = anon_filp;
-
return ret;
-err_subdrv_close:
- exynos_drm_subdrv_close(dev, file);
-
err_file_priv_free:
kfree(file_priv);
file->driver_priv = NULL;
@@ -234,7 +215,6 @@ static void exynos_drm_preclose(struct drm_device *dev,
static void exynos_drm_postclose(struct drm_device *dev, struct drm_file *file)
{
struct exynos_drm_private *private = dev->dev_private;
- struct drm_exynos_file_private *file_priv;
struct drm_pending_vblank_event *v, *vt;
struct drm_pending_event *e, *et;
unsigned long flags;
@@ -260,10 +240,6 @@ static void exynos_drm_postclose(struct drm_device *dev, struct drm_file *file)
}
spin_unlock_irqrestore(&dev->event_lock, flags);
- file_priv = file->driver_priv;
- if (file_priv->anon_filp)
- fput(file_priv->anon_filp);
-
kfree(file->driver_priv);
file->driver_priv = NULL;
}
@@ -282,8 +258,6 @@ static const struct vm_operations_struct exynos_drm_gem_vm_ops = {
static const struct drm_ioctl_desc exynos_ioctls[] = {
DRM_IOCTL_DEF_DRV(EXYNOS_GEM_CREATE, exynos_drm_gem_create_ioctl,
DRM_UNLOCKED | DRM_AUTH),
- DRM_IOCTL_DEF_DRV(EXYNOS_GEM_MMAP,
- exynos_drm_gem_mmap_ioctl, DRM_UNLOCKED | DRM_AUTH),
DRM_IOCTL_DEF_DRV(EXYNOS_GEM_GET,
exynos_drm_gem_get_ioctl, DRM_UNLOCKED),
DRM_IOCTL_DEF_DRV(EXYNOS_VIDI_CONNECTION,
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index 69a6fa3..d22e640 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -240,7 +240,6 @@ struct exynos_drm_g2d_private {
struct drm_exynos_file_private {
struct exynos_drm_g2d_private *g2d_priv;
struct device *ipp_dev;
- struct file *anon_filp;
};
/*
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
index 2f3665d..0d5b969 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
@@ -318,23 +318,16 @@ void exynos_drm_gem_put_dma_addr(struct drm_device *dev,
drm_gem_object_unreference_unlocked(obj);
}
-int exynos_drm_gem_mmap_buffer(struct file *filp,
+int exynos_drm_gem_mmap_buffer(struct exynos_drm_gem_obj *exynos_gem_obj,
struct vm_area_struct *vma)
{
- struct drm_gem_object *obj = filp->private_data;
- struct exynos_drm_gem_obj *exynos_gem_obj = to_exynos_gem_obj(obj);
- struct drm_device *drm_dev = obj->dev;
+ struct drm_device *drm_dev = exynos_gem_obj->base.dev;
struct exynos_drm_gem_buf *buffer;
unsigned long vm_size;
int ret;
- WARN_ON(!mutex_is_locked(&obj->dev->struct_mutex));
-
- vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
- vma->vm_private_data = obj;
- vma->vm_ops = drm_dev->driver->gem_vm_ops;
-
- update_vm_cache_attr(exynos_gem_obj, vma);
+ vma->vm_flags &= ~VM_PFNMAP;
+ vma->vm_pgoff = 0;
vm_size = vma->vm_end - vma->vm_start;
@@ -356,60 +349,6 @@ int exynos_drm_gem_mmap_buffer(struct file *filp,
return ret;
}
- /*
- * take a reference to this mapping of the object. And this reference
- * is unreferenced by the corresponding vm_close call.
- */
- drm_gem_object_reference(obj);
-
- drm_vm_open_locked(drm_dev, vma);
-
- return 0;
-}
-
-int exynos_drm_gem_mmap_ioctl(struct drm_device *dev, void *data,
- struct drm_file *file_priv)
-{
- struct drm_exynos_file_private *exynos_file_priv;
- struct drm_exynos_gem_mmap *args = data;
- struct drm_gem_object *obj;
- struct file *anon_filp;
- unsigned long addr;
-
- if (!(dev->driver->driver_features & DRIVER_GEM)) {
- DRM_ERROR("does not support GEM.\n");
- return -ENODEV;
- }
-
- mutex_lock(&dev->struct_mutex);
-
- obj = drm_gem_object_lookup(dev, file_priv, args->handle);
- if (!obj) {
- DRM_ERROR("failed to lookup gem object.\n");
- mutex_unlock(&dev->struct_mutex);
- return -EINVAL;
- }
-
- exynos_file_priv = file_priv->driver_priv;
- anon_filp = exynos_file_priv->anon_filp;
- anon_filp->private_data = obj;
-
- addr = vm_mmap(anon_filp, 0, args->size, PROT_READ | PROT_WRITE,
- MAP_SHARED, 0);
-
- drm_gem_object_unreference(obj);
-
- if (IS_ERR_VALUE(addr)) {
- mutex_unlock(&dev->struct_mutex);
- return (int)addr;
- }
-
- mutex_unlock(&dev->struct_mutex);
-
- args->mapped = addr;
-
- DRM_DEBUG_KMS("mapped = 0x%lx\n", (unsigned long)args->mapped);
-
return 0;
}
@@ -693,16 +632,20 @@ int exynos_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
exynos_gem_obj = to_exynos_gem_obj(obj);
ret = check_gem_flags(exynos_gem_obj->flags);
- if (ret) {
- drm_gem_vm_close(vma);
- drm_gem_free_mmap_offset(obj);
- return ret;
- }
-
- vma->vm_flags &= ~VM_PFNMAP;
- vma->vm_flags |= VM_MIXEDMAP;
+ if (ret)
+ goto err_close_vm;
update_vm_cache_attr(exynos_gem_obj, vma);
+ ret = exynos_drm_gem_mmap_buffer(exynos_gem_obj, vma);
+ if (ret)
+ goto err_close_vm;
+
+ return ret;
+
+err_close_vm:
+ drm_gem_vm_close(vma);
+ drm_gem_free_mmap_offset(obj);
+
return ret;
}
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.h b/drivers/gpu/drm/exynos/exynos_drm_gem.h
index 8e46094..09d021b 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.h
@@ -111,16 +111,6 @@ void exynos_drm_gem_put_dma_addr(struct drm_device *dev,
unsigned int gem_handle,
struct drm_file *filp);
-/*
- * mmap the physically continuous memory that a gem object contains
- * to user space.
- */
-int exynos_drm_gem_mmap_ioctl(struct drm_device *dev, void *data,
- struct drm_file *file_priv);
-
-int exynos_drm_gem_mmap_buffer(struct file *filp,
- struct vm_area_struct *vma);
-
/* map user space allocated by malloc to pages. */
int exynos_drm_gem_userptr_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_priv);
diff --git a/include/uapi/drm/exynos_drm.h b/include/uapi/drm/exynos_drm.h
index 67a751c..5575ed1 100644
--- a/include/uapi/drm/exynos_drm.h
+++ b/include/uapi/drm/exynos_drm.h
@@ -33,24 +33,6 @@ struct drm_exynos_gem_create {
};
/**
- * A structure for mapping buffer.
- *
- * @handle: a handle to gem object created.
- * @pad: just padding to be 64-bit aligned.
- * @size: memory size to be mapped.
- * @mapped: having user virtual address mmaped.
- * - this variable would be filled by exynos gem module
- * of kernel side with user virtual address which is allocated
- * by do_mmap().
- */
-struct drm_exynos_gem_mmap {
- unsigned int handle;
- unsigned int pad;
- uint64_t size;
- uint64_t mapped;
-};
-
-/**
* A structure to gem information.
*
* @handle: a handle to gem object created.
@@ -302,7 +284,6 @@ struct drm_exynos_ipp_cmd_ctrl {
};
#define DRM_EXYNOS_GEM_CREATE 0x00
-#define DRM_EXYNOS_GEM_MMAP 0x02
/* Reserved 0x03 ~ 0x05 for exynos specific gem ioctl */
#define DRM_EXYNOS_GEM_GET 0x04
#define DRM_EXYNOS_VIDI_CONNECTION 0x07
@@ -321,9 +302,6 @@ struct drm_exynos_ipp_cmd_ctrl {
#define DRM_IOCTL_EXYNOS_GEM_CREATE DRM_IOWR(DRM_COMMAND_BASE + \
DRM_EXYNOS_GEM_CREATE, struct drm_exynos_gem_create)
-#define DRM_IOCTL_EXYNOS_GEM_MMAP DRM_IOWR(DRM_COMMAND_BASE + \
- DRM_EXYNOS_GEM_MMAP, struct drm_exynos_gem_mmap)
-
#define DRM_IOCTL_EXYNOS_GEM_GET DRM_IOWR(DRM_COMMAND_BASE + \
DRM_EXYNOS_GEM_GET, struct drm_exynos_gem_info)
--
1.7.9.5
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 2/2] drm/exynos: use drm generic mmap interface
2014-09-17 13:48 ` [PATCH 2/2] drm/exynos: use drm generic mmap interface Inki Dae
2014-09-18 4:56 ` Joonyoung Shim
2014-09-18 5:19 ` [PATCH v2] " Inki Dae
@ 2014-10-01 13:17 ` Tomasz Figa
2014-10-02 10:22 ` Inki Dae
2 siblings, 1 reply; 16+ messages in thread
From: Tomasz Figa @ 2014-10-01 13:17 UTC (permalink / raw)
To: Inki Dae, dri-devel; +Cc: linux-samsung-soc
Hi Inki,
On 17.09.2014 15:48, Inki Dae wrote:
> This patch removes DRM_EXYNOS_GEM_MMAP ictrl feature specific
> to Exynos drm and instead uses drm generic mmap.
It looks like libdrm_exynos is still using DRM_EXYNOS_GEM_MMAP, but this
patch just removes it. This basically means that any application using
it is now broken. Do you have any plans to fix this?
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] drm/exynos: use drm generic mmap interface
2014-10-01 13:17 ` [PATCH 2/2] " Tomasz Figa
@ 2014-10-02 10:22 ` Inki Dae
2014-11-18 22:09 ` Tobias Jakobi
0 siblings, 1 reply; 16+ messages in thread
From: Inki Dae @ 2014-10-02 10:22 UTC (permalink / raw)
To: Tomasz Figa; +Cc: dri-devel, linux-samsung-soc
On 2014년 10월 01일 22:17, Tomasz Figa wrote:
> Hi Inki,
>
> On 17.09.2014 15:48, Inki Dae wrote:
>> This patch removes DRM_EXYNOS_GEM_MMAP ictrl feature specific
>> to Exynos drm and instead uses drm generic mmap.
>
> It looks like libdrm_exynos is still using DRM_EXYNOS_GEM_MMAP, but this
> patch just removes it. This basically means that any application using
> it is now broken. Do you have any plans to fix this?
>
Hi Tomasz,
Yes, of course.
We will fix it soon. For this, I mentioned earlier,
http://web.archiveorange.com/archive/v/hhSc574WhqJAKgdBq7KL
Thanks,
Inki Dae
> Best regards,
> Tomasz
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] drm/exynos: use drm generic mmap interface
2014-10-02 10:22 ` Inki Dae
@ 2014-11-18 22:09 ` Tobias Jakobi
2014-11-19 2:04 ` Inki Dae
0 siblings, 1 reply; 16+ messages in thread
From: Tobias Jakobi @ 2014-11-18 22:09 UTC (permalink / raw)
To: Inki Dae, Tomasz Figa; +Cc: linux-samsung-soc, dri-devel
Inki Dae wrote:
> Hi Tomasz,
>
> Yes, of course.
>
> We will fix it soon. For this, I mentioned earlier,
> http://web.archiveorange.com/archive/v/hhSc574WhqJAKgdBq7KL
>
> Thanks,
> Inki Dae
Hello,
it looks like libdrm git master is still using the EXYNOS_GEM_MMAP ioctl
that was removed by this patch. Is there any news on the patches for libdrm?
With best wishes,
Tobias
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] drm/exynos: use drm generic mmap interface
2014-11-18 22:09 ` Tobias Jakobi
@ 2014-11-19 2:04 ` Inki Dae
2014-11-19 2:32 ` Tobias Jakobi
0 siblings, 1 reply; 16+ messages in thread
From: Inki Dae @ 2014-11-19 2:04 UTC (permalink / raw)
To: Tobias Jakobi; +Cc: Tomasz Figa, dri-devel, linux-samsung-soc
On 2014년 11월 19일 07:09, Tobias Jakobi wrote:
> Inki Dae wrote:
>> Hi Tomasz,
>>
>> Yes, of course.
>>
>> We will fix it soon. For this, I mentioned earlier,
>> http://web.archiveorange.com/archive/v/hhSc574WhqJAKgdBq7KL
>>
>> Thanks,
>> Inki Dae
>
>
> Hello,
>
> it looks like libdrm git master is still using the EXYNOS_GEM_MMAP ioctl
> that was removed by this patch. Is there any news on the patches for libdrm?
The patch set already was posted. You can refer to below link,
http://www.spinics.net/lists/dri-devel/msg70737.html
Thanks,
Inki Dae
>
> With best wishes,
> Tobias
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 16+ messages in thread