| Message ID | 1410961726-2533-3-git-send-email-inki.dae@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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) > >
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) >> >> > >
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
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 >
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
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 >
Inki Dae wrote: >> >> 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 > Looks like I missed that, thanks for the heads up! With best wishes, Tobias
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 @@ -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; - 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)
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(-)