* [PATCH 1/3] drm/exynox: Implement mmap as GEM object function
2021-11-08 10:28 [RESEND PATCH 0/3] drm/{exynos,xen}: Implement gem_prime_mmap with drm_gem_prime_mmap() Thomas Zimmermann
@ 2021-11-08 10:28 ` Thomas Zimmermann
2021-11-08 15:29 ` Daniel Vetter
2021-11-09 5:08 ` Inki Dae
2021-11-08 10:28 ` [PATCH 2/3] drm/xen: " Thomas Zimmermann
2021-11-08 10:28 ` [PATCH 3/3] drm: Update documentation and TODO of gem_prime_mmap hook Thomas Zimmermann
2 siblings, 2 replies; 12+ messages in thread
From: Thomas Zimmermann @ 2021-11-08 10:28 UTC (permalink / raw)
To: daniel, airlied, maarten.lankhorst, mripard, inki.dae,
jy0922.shim, sw0312.kim, kyungmin.park, krzysztof.kozlowski,
oleksandr_andrushchenko
Cc: dri-devel, linux-arm-kernel, linux-samsung-soc, xen-devel,
Thomas Zimmermann
Moving the driver-specific mmap code into a GEM object function allows
for using DRM helpers for various mmap callbacks.
The respective exynos functions are being removed. The file_operations
structure exynos_drm_driver_fops is now being created by the helper macro
DEFINE_DRM_GEM_FOPS().
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/exynos/exynos_drm_drv.c | 13 ++-----
drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 20 ++---------
drivers/gpu/drm/exynos/exynos_drm_gem.c | 43 +++++------------------
drivers/gpu/drm/exynos/exynos_drm_gem.h | 5 ---
4 files changed, 13 insertions(+), 68 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index d8f1cf4d6b69..9743b6b17447 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -102,16 +102,7 @@ static const struct drm_ioctl_desc exynos_ioctls[] = {
DRM_RENDER_ALLOW),
};
-static const struct file_operations exynos_drm_driver_fops = {
- .owner = THIS_MODULE,
- .open = drm_open,
- .mmap = exynos_drm_gem_mmap,
- .poll = drm_poll,
- .read = drm_read,
- .unlocked_ioctl = drm_ioctl,
- .compat_ioctl = drm_compat_ioctl,
- .release = drm_release,
-};
+DEFINE_DRM_GEM_FOPS(exynos_drm_driver_fops);
static const struct drm_driver exynos_drm_driver = {
.driver_features = DRIVER_MODESET | DRIVER_GEM
@@ -124,7 +115,7 @@ static const struct drm_driver exynos_drm_driver = {
.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
.gem_prime_import = exynos_drm_gem_prime_import,
.gem_prime_import_sg_table = exynos_drm_gem_prime_import_sg_table,
- .gem_prime_mmap = exynos_drm_gem_prime_mmap,
+ .gem_prime_mmap = drm_gem_prime_mmap,
.ioctls = exynos_ioctls,
.num_ioctls = ARRAY_SIZE(exynos_ioctls),
.fops = &exynos_drm_driver_fops,
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
index 5147f5929be7..02c97b9ca926 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
@@ -15,6 +15,7 @@
#include <drm/drm_crtc.h>
#include <drm/drm_fb_helper.h>
#include <drm/drm_fourcc.h>
+#include <drm/drm_prime.h>
#include <drm/drm_probe_helper.h>
#include <drm/exynos_drm.h>
@@ -39,25 +40,8 @@ static int exynos_drm_fb_mmap(struct fb_info *info,
struct drm_fb_helper *helper = info->par;
struct exynos_drm_fbdev *exynos_fbd = to_exynos_fbdev(helper);
struct exynos_drm_gem *exynos_gem = exynos_fbd->exynos_gem;
- unsigned long vm_size;
- int ret;
-
- vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
-
- vm_size = vma->vm_end - vma->vm_start;
-
- if (vm_size > exynos_gem->size)
- return -EINVAL;
- ret = dma_mmap_attrs(to_dma_dev(helper->dev), vma, exynos_gem->cookie,
- exynos_gem->dma_addr, exynos_gem->size,
- exynos_gem->dma_attrs);
- if (ret < 0) {
- DRM_DEV_ERROR(to_dma_dev(helper->dev), "failed to mmap.\n");
- return ret;
- }
-
- return 0;
+ return drm_gem_prime_mmap(&exynos_gem->base, vma);
}
static const struct fb_ops exynos_drm_fb_ops = {
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
index 4396224227d1..c4b63902ee7a 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
@@ -17,6 +17,8 @@
#include "exynos_drm_drv.h"
#include "exynos_drm_gem.h"
+static int exynos_drm_gem_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma);
+
static int exynos_drm_alloc_buf(struct exynos_drm_gem *exynos_gem, bool kvmap)
{
struct drm_device *dev = exynos_gem->base.dev;
@@ -135,6 +137,7 @@ static const struct vm_operations_struct exynos_drm_gem_vm_ops = {
static const struct drm_gem_object_funcs exynos_drm_gem_object_funcs = {
.free = exynos_drm_gem_free_object,
.get_sg_table = exynos_drm_gem_prime_get_sg_table,
+ .mmap = exynos_drm_gem_mmap,
.vm_ops = &exynos_drm_gem_vm_ops,
};
@@ -354,12 +357,16 @@ int exynos_drm_gem_dumb_create(struct drm_file *file_priv,
return 0;
}
-static int exynos_drm_gem_mmap_obj(struct drm_gem_object *obj,
- struct vm_area_struct *vma)
+static int exynos_drm_gem_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma)
{
struct exynos_drm_gem *exynos_gem = to_exynos_gem(obj);
int ret;
+ if (obj->import_attach)
+ return dma_buf_mmap(obj->dma_buf, vma, 0);
+
+ vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
+
DRM_DEV_DEBUG_KMS(to_dma_dev(obj->dev), "flags = 0x%x\n",
exynos_gem->flags);
@@ -385,26 +392,6 @@ static int exynos_drm_gem_mmap_obj(struct drm_gem_object *obj,
return ret;
}
-int exynos_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
-{
- struct drm_gem_object *obj;
- int ret;
-
- /* set vm_area_struct. */
- ret = drm_gem_mmap(filp, vma);
- if (ret < 0) {
- DRM_ERROR("failed to mmap.\n");
- return ret;
- }
-
- obj = vma->vm_private_data;
-
- if (obj->import_attach)
- return dma_buf_mmap(obj->dma_buf, vma, 0);
-
- return exynos_drm_gem_mmap_obj(obj, vma);
-}
-
/* low-level interface prime helpers */
struct drm_gem_object *exynos_drm_gem_prime_import(struct drm_device *dev,
struct dma_buf *dma_buf)
@@ -466,15 +453,3 @@ exynos_drm_gem_prime_import_sg_table(struct drm_device *dev,
exynos_gem->sgt = sgt;
return &exynos_gem->base;
}
-
-int exynos_drm_gem_prime_mmap(struct drm_gem_object *obj,
- struct vm_area_struct *vma)
-{
- int ret;
-
- ret = drm_gem_mmap_obj(obj, obj->size, vma);
- if (ret < 0)
- return ret;
-
- return exynos_drm_gem_mmap_obj(obj, vma);
-}
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.h b/drivers/gpu/drm/exynos/exynos_drm_gem.h
index a23272fb96fb..79d7e1a87419 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.h
@@ -96,9 +96,6 @@ int exynos_drm_gem_dumb_create(struct drm_file *file_priv,
struct drm_device *dev,
struct drm_mode_create_dumb *args);
-/* set vm_flags and we can change the vm attribute to other one at here. */
-int exynos_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma);
-
/* low-level interface prime helpers */
struct drm_gem_object *exynos_drm_gem_prime_import(struct drm_device *dev,
struct dma_buf *dma_buf);
@@ -107,7 +104,5 @@ struct drm_gem_object *
exynos_drm_gem_prime_import_sg_table(struct drm_device *dev,
struct dma_buf_attachment *attach,
struct sg_table *sgt);
-int exynos_drm_gem_prime_mmap(struct drm_gem_object *obj,
- struct vm_area_struct *vma);
#endif
--
2.33.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] drm/exynox: Implement mmap as GEM object function
2021-11-08 10:28 ` [PATCH 1/3] drm/exynox: Implement mmap as GEM object function Thomas Zimmermann
@ 2021-11-08 15:29 ` Daniel Vetter
2021-11-09 9:34 ` Inki Dae
2021-11-09 5:08 ` Inki Dae
1 sibling, 1 reply; 12+ messages in thread
From: Daniel Vetter @ 2021-11-08 15:29 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: daniel, airlied, maarten.lankhorst, mripard, inki.dae,
jy0922.shim, sw0312.kim, kyungmin.park, krzysztof.kozlowski,
oleksandr_andrushchenko, dri-devel, linux-arm-kernel,
linux-samsung-soc, xen-devel
On Mon, Nov 08, 2021 at 11:28:44AM +0100, Thomas Zimmermann wrote:
> Moving the driver-specific mmap code into a GEM object function allows
> for using DRM helpers for various mmap callbacks.
>
> The respective exynos functions are being removed. The file_operations
> structure exynos_drm_driver_fops is now being created by the helper macro
> DEFINE_DRM_GEM_FOPS().
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
s/exynox/exynos in the subject.
Patch lgtm, but would still be good if exynos maintainers would
ack/review/test it. But if you don't get anything within 2 weeks here's
my:
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/exynos/exynos_drm_drv.c | 13 ++-----
> drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 20 ++---------
> drivers/gpu/drm/exynos/exynos_drm_gem.c | 43 +++++------------------
> drivers/gpu/drm/exynos/exynos_drm_gem.h | 5 ---
> 4 files changed, 13 insertions(+), 68 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index d8f1cf4d6b69..9743b6b17447 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -102,16 +102,7 @@ static const struct drm_ioctl_desc exynos_ioctls[] = {
> DRM_RENDER_ALLOW),
> };
>
> -static const struct file_operations exynos_drm_driver_fops = {
> - .owner = THIS_MODULE,
> - .open = drm_open,
> - .mmap = exynos_drm_gem_mmap,
> - .poll = drm_poll,
> - .read = drm_read,
> - .unlocked_ioctl = drm_ioctl,
> - .compat_ioctl = drm_compat_ioctl,
> - .release = drm_release,
> -};
> +DEFINE_DRM_GEM_FOPS(exynos_drm_driver_fops);
>
> static const struct drm_driver exynos_drm_driver = {
> .driver_features = DRIVER_MODESET | DRIVER_GEM
> @@ -124,7 +115,7 @@ static const struct drm_driver exynos_drm_driver = {
> .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
> .gem_prime_import = exynos_drm_gem_prime_import,
> .gem_prime_import_sg_table = exynos_drm_gem_prime_import_sg_table,
> - .gem_prime_mmap = exynos_drm_gem_prime_mmap,
> + .gem_prime_mmap = drm_gem_prime_mmap,
> .ioctls = exynos_ioctls,
> .num_ioctls = ARRAY_SIZE(exynos_ioctls),
> .fops = &exynos_drm_driver_fops,
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> index 5147f5929be7..02c97b9ca926 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> @@ -15,6 +15,7 @@
> #include <drm/drm_crtc.h>
> #include <drm/drm_fb_helper.h>
> #include <drm/drm_fourcc.h>
> +#include <drm/drm_prime.h>
> #include <drm/drm_probe_helper.h>
> #include <drm/exynos_drm.h>
>
> @@ -39,25 +40,8 @@ static int exynos_drm_fb_mmap(struct fb_info *info,
> struct drm_fb_helper *helper = info->par;
> struct exynos_drm_fbdev *exynos_fbd = to_exynos_fbdev(helper);
> struct exynos_drm_gem *exynos_gem = exynos_fbd->exynos_gem;
> - unsigned long vm_size;
> - int ret;
> -
> - vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
> -
> - vm_size = vma->vm_end - vma->vm_start;
> -
> - if (vm_size > exynos_gem->size)
> - return -EINVAL;
>
> - ret = dma_mmap_attrs(to_dma_dev(helper->dev), vma, exynos_gem->cookie,
> - exynos_gem->dma_addr, exynos_gem->size,
> - exynos_gem->dma_attrs);
> - if (ret < 0) {
> - DRM_DEV_ERROR(to_dma_dev(helper->dev), "failed to mmap.\n");
> - return ret;
> - }
> -
> - return 0;
> + return drm_gem_prime_mmap(&exynos_gem->base, vma);
> }
>
> static const struct fb_ops exynos_drm_fb_ops = {
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> index 4396224227d1..c4b63902ee7a 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> @@ -17,6 +17,8 @@
> #include "exynos_drm_drv.h"
> #include "exynos_drm_gem.h"
>
> +static int exynos_drm_gem_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma);
> +
> static int exynos_drm_alloc_buf(struct exynos_drm_gem *exynos_gem, bool kvmap)
> {
> struct drm_device *dev = exynos_gem->base.dev;
> @@ -135,6 +137,7 @@ static const struct vm_operations_struct exynos_drm_gem_vm_ops = {
> static const struct drm_gem_object_funcs exynos_drm_gem_object_funcs = {
> .free = exynos_drm_gem_free_object,
> .get_sg_table = exynos_drm_gem_prime_get_sg_table,
> + .mmap = exynos_drm_gem_mmap,
> .vm_ops = &exynos_drm_gem_vm_ops,
> };
>
> @@ -354,12 +357,16 @@ int exynos_drm_gem_dumb_create(struct drm_file *file_priv,
> return 0;
> }
>
> -static int exynos_drm_gem_mmap_obj(struct drm_gem_object *obj,
> - struct vm_area_struct *vma)
> +static int exynos_drm_gem_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma)
> {
> struct exynos_drm_gem *exynos_gem = to_exynos_gem(obj);
> int ret;
>
> + if (obj->import_attach)
> + return dma_buf_mmap(obj->dma_buf, vma, 0);
> +
> + vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
> +
> DRM_DEV_DEBUG_KMS(to_dma_dev(obj->dev), "flags = 0x%x\n",
> exynos_gem->flags);
>
> @@ -385,26 +392,6 @@ static int exynos_drm_gem_mmap_obj(struct drm_gem_object *obj,
> return ret;
> }
>
> -int exynos_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
> -{
> - struct drm_gem_object *obj;
> - int ret;
> -
> - /* set vm_area_struct. */
> - ret = drm_gem_mmap(filp, vma);
> - if (ret < 0) {
> - DRM_ERROR("failed to mmap.\n");
> - return ret;
> - }
> -
> - obj = vma->vm_private_data;
> -
> - if (obj->import_attach)
> - return dma_buf_mmap(obj->dma_buf, vma, 0);
> -
> - return exynos_drm_gem_mmap_obj(obj, vma);
> -}
> -
> /* low-level interface prime helpers */
> struct drm_gem_object *exynos_drm_gem_prime_import(struct drm_device *dev,
> struct dma_buf *dma_buf)
> @@ -466,15 +453,3 @@ exynos_drm_gem_prime_import_sg_table(struct drm_device *dev,
> exynos_gem->sgt = sgt;
> return &exynos_gem->base;
> }
> -
> -int exynos_drm_gem_prime_mmap(struct drm_gem_object *obj,
> - struct vm_area_struct *vma)
> -{
> - int ret;
> -
> - ret = drm_gem_mmap_obj(obj, obj->size, vma);
> - if (ret < 0)
> - return ret;
> -
> - return exynos_drm_gem_mmap_obj(obj, vma);
> -}
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.h b/drivers/gpu/drm/exynos/exynos_drm_gem.h
> index a23272fb96fb..79d7e1a87419 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.h
> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.h
> @@ -96,9 +96,6 @@ int exynos_drm_gem_dumb_create(struct drm_file *file_priv,
> struct drm_device *dev,
> struct drm_mode_create_dumb *args);
>
> -/* set vm_flags and we can change the vm attribute to other one at here. */
> -int exynos_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma);
> -
> /* low-level interface prime helpers */
> struct drm_gem_object *exynos_drm_gem_prime_import(struct drm_device *dev,
> struct dma_buf *dma_buf);
> @@ -107,7 +104,5 @@ struct drm_gem_object *
> exynos_drm_gem_prime_import_sg_table(struct drm_device *dev,
> struct dma_buf_attachment *attach,
> struct sg_table *sgt);
> -int exynos_drm_gem_prime_mmap(struct drm_gem_object *obj,
> - struct vm_area_struct *vma);
>
> #endif
> --
> 2.33.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] drm/exynox: Implement mmap as GEM object function
2021-11-08 15:29 ` Daniel Vetter
@ 2021-11-09 9:34 ` Inki Dae
2021-11-09 9:44 ` Thomas Zimmermann
0 siblings, 1 reply; 12+ messages in thread
From: Inki Dae @ 2021-11-09 9:34 UTC (permalink / raw)
To: Daniel Vetter, Thomas Zimmermann
Cc: airlied, maarten.lankhorst, mripard, jy0922.shim, sw0312.kim,
kyungmin.park, krzysztof.kozlowski, oleksandr_andrushchenko,
dri-devel, linux-arm-kernel, linux-samsung-soc, xen-devel
Hi Thomas and Daniel,
21. 11. 9. 오전 12:29에 Daniel Vetter 이(가) 쓴 글:
> On Mon, Nov 08, 2021 at 11:28:44AM +0100, Thomas Zimmermann wrote:
>> Moving the driver-specific mmap code into a GEM object function allows
>> for using DRM helpers for various mmap callbacks.
>>
>> The respective exynos functions are being removed. The file_operations
>> structure exynos_drm_driver_fops is now being created by the helper macro
>> DEFINE_DRM_GEM_FOPS().
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>
> s/exynox/exynos in the subject.
>
> Patch lgtm, but would still be good if exynos maintainers would
> ack/review/test it. But if you don't get anything within 2 weeks here's
Sorry for late. Confirmed working well on Odroid board, and had a review. Applied.
https://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git/log/?h=exynos-drm-next
Thanks,
Inki Dae
> my:
>
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
>> ---
>> drivers/gpu/drm/exynos/exynos_drm_drv.c | 13 ++-----
>> drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 20 ++---------
>> drivers/gpu/drm/exynos/exynos_drm_gem.c | 43 +++++------------------
>> drivers/gpu/drm/exynos/exynos_drm_gem.h | 5 ---
>> 4 files changed, 13 insertions(+), 68 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
>> index d8f1cf4d6b69..9743b6b17447 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
>> @@ -102,16 +102,7 @@ static const struct drm_ioctl_desc exynos_ioctls[] = {
>> DRM_RENDER_ALLOW),
>> };
>>
>> -static const struct file_operations exynos_drm_driver_fops = {
>> - .owner = THIS_MODULE,
>> - .open = drm_open,
>> - .mmap = exynos_drm_gem_mmap,
>> - .poll = drm_poll,
>> - .read = drm_read,
>> - .unlocked_ioctl = drm_ioctl,
>> - .compat_ioctl = drm_compat_ioctl,
>> - .release = drm_release,
>> -};
>> +DEFINE_DRM_GEM_FOPS(exynos_drm_driver_fops);
>>
>> static const struct drm_driver exynos_drm_driver = {
>> .driver_features = DRIVER_MODESET | DRIVER_GEM
>> @@ -124,7 +115,7 @@ static const struct drm_driver exynos_drm_driver = {
>> .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
>> .gem_prime_import = exynos_drm_gem_prime_import,
>> .gem_prime_import_sg_table = exynos_drm_gem_prime_import_sg_table,
>> - .gem_prime_mmap = exynos_drm_gem_prime_mmap,
>> + .gem_prime_mmap = drm_gem_prime_mmap,
>> .ioctls = exynos_ioctls,
>> .num_ioctls = ARRAY_SIZE(exynos_ioctls),
>> .fops = &exynos_drm_driver_fops,
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
>> index 5147f5929be7..02c97b9ca926 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
>> @@ -15,6 +15,7 @@
>> #include <drm/drm_crtc.h>
>> #include <drm/drm_fb_helper.h>
>> #include <drm/drm_fourcc.h>
>> +#include <drm/drm_prime.h>
>> #include <drm/drm_probe_helper.h>
>> #include <drm/exynos_drm.h>
>>
>> @@ -39,25 +40,8 @@ static int exynos_drm_fb_mmap(struct fb_info *info,
>> struct drm_fb_helper *helper = info->par;
>> struct exynos_drm_fbdev *exynos_fbd = to_exynos_fbdev(helper);
>> struct exynos_drm_gem *exynos_gem = exynos_fbd->exynos_gem;
>> - unsigned long vm_size;
>> - int ret;
>> -
>> - vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
>> -
>> - vm_size = vma->vm_end - vma->vm_start;
>> -
>> - if (vm_size > exynos_gem->size)
>> - return -EINVAL;
>>
>> - ret = dma_mmap_attrs(to_dma_dev(helper->dev), vma, exynos_gem->cookie,
>> - exynos_gem->dma_addr, exynos_gem->size,
>> - exynos_gem->dma_attrs);
>> - if (ret < 0) {
>> - DRM_DEV_ERROR(to_dma_dev(helper->dev), "failed to mmap.\n");
>> - return ret;
>> - }
>> -
>> - return 0;
>> + return drm_gem_prime_mmap(&exynos_gem->base, vma);
>> }
>>
>> static const struct fb_ops exynos_drm_fb_ops = {
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
>> index 4396224227d1..c4b63902ee7a 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
>> @@ -17,6 +17,8 @@
>> #include "exynos_drm_drv.h"
>> #include "exynos_drm_gem.h"
>>
>> +static int exynos_drm_gem_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma);
>> +
>> static int exynos_drm_alloc_buf(struct exynos_drm_gem *exynos_gem, bool kvmap)
>> {
>> struct drm_device *dev = exynos_gem->base.dev;
>> @@ -135,6 +137,7 @@ static const struct vm_operations_struct exynos_drm_gem_vm_ops = {
>> static const struct drm_gem_object_funcs exynos_drm_gem_object_funcs = {
>> .free = exynos_drm_gem_free_object,
>> .get_sg_table = exynos_drm_gem_prime_get_sg_table,
>> + .mmap = exynos_drm_gem_mmap,
>> .vm_ops = &exynos_drm_gem_vm_ops,
>> };
>>
>> @@ -354,12 +357,16 @@ int exynos_drm_gem_dumb_create(struct drm_file *file_priv,
>> return 0;
>> }
>>
>> -static int exynos_drm_gem_mmap_obj(struct drm_gem_object *obj,
>> - struct vm_area_struct *vma)
>> +static int exynos_drm_gem_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma)
>> {
>> struct exynos_drm_gem *exynos_gem = to_exynos_gem(obj);
>> int ret;
>>
>> + if (obj->import_attach)
>> + return dma_buf_mmap(obj->dma_buf, vma, 0);
>> +
>> + vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
>> +
>> DRM_DEV_DEBUG_KMS(to_dma_dev(obj->dev), "flags = 0x%x\n",
>> exynos_gem->flags);
>>
>> @@ -385,26 +392,6 @@ static int exynos_drm_gem_mmap_obj(struct drm_gem_object *obj,
>> return ret;
>> }
>>
>> -int exynos_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
>> -{
>> - struct drm_gem_object *obj;
>> - int ret;
>> -
>> - /* set vm_area_struct. */
>> - ret = drm_gem_mmap(filp, vma);
>> - if (ret < 0) {
>> - DRM_ERROR("failed to mmap.\n");
>> - return ret;
>> - }
>> -
>> - obj = vma->vm_private_data;
>> -
>> - if (obj->import_attach)
>> - return dma_buf_mmap(obj->dma_buf, vma, 0);
>> -
>> - return exynos_drm_gem_mmap_obj(obj, vma);
>> -}
>> -
>> /* low-level interface prime helpers */
>> struct drm_gem_object *exynos_drm_gem_prime_import(struct drm_device *dev,
>> struct dma_buf *dma_buf)
>> @@ -466,15 +453,3 @@ exynos_drm_gem_prime_import_sg_table(struct drm_device *dev,
>> exynos_gem->sgt = sgt;
>> return &exynos_gem->base;
>> }
>> -
>> -int exynos_drm_gem_prime_mmap(struct drm_gem_object *obj,
>> - struct vm_area_struct *vma)
>> -{
>> - int ret;
>> -
>> - ret = drm_gem_mmap_obj(obj, obj->size, vma);
>> - if (ret < 0)
>> - return ret;
>> -
>> - return exynos_drm_gem_mmap_obj(obj, vma);
>> -}
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.h b/drivers/gpu/drm/exynos/exynos_drm_gem.h
>> index a23272fb96fb..79d7e1a87419 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.h
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.h
>> @@ -96,9 +96,6 @@ int exynos_drm_gem_dumb_create(struct drm_file *file_priv,
>> struct drm_device *dev,
>> struct drm_mode_create_dumb *args);
>>
>> -/* set vm_flags and we can change the vm attribute to other one at here. */
>> -int exynos_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma);
>> -
>> /* low-level interface prime helpers */
>> struct drm_gem_object *exynos_drm_gem_prime_import(struct drm_device *dev,
>> struct dma_buf *dma_buf);
>> @@ -107,7 +104,5 @@ struct drm_gem_object *
>> exynos_drm_gem_prime_import_sg_table(struct drm_device *dev,
>> struct dma_buf_attachment *attach,
>> struct sg_table *sgt);
>> -int exynos_drm_gem_prime_mmap(struct drm_gem_object *obj,
>> - struct vm_area_struct *vma);
>>
>> #endif
>> --
>> 2.33.1
>>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] drm/exynox: Implement mmap as GEM object function
2021-11-09 9:34 ` Inki Dae
@ 2021-11-09 9:44 ` Thomas Zimmermann
0 siblings, 0 replies; 12+ messages in thread
From: Thomas Zimmermann @ 2021-11-09 9:44 UTC (permalink / raw)
To: Inki Dae, Daniel Vetter
Cc: linux-samsung-soc, jy0922.shim, krzysztof.kozlowski, airlied,
oleksandr_andrushchenko, sw0312.kim, kyungmin.park, dri-devel,
xen-devel, linux-arm-kernel
[-- Attachment #1.1: Type: text/plain, Size: 8522 bytes --]
Hi
Am 09.11.21 um 10:34 schrieb Inki Dae:
> Hi Thomas and Daniel,
>
> 21. 11. 9. 오전 12:29에 Daniel Vetter 이(가) 쓴 글:
>> On Mon, Nov 08, 2021 at 11:28:44AM +0100, Thomas Zimmermann wrote:
>>> Moving the driver-specific mmap code into a GEM object function allows
>>> for using DRM helpers for various mmap callbacks.
>>>
>>> The respective exynos functions are being removed. The file_operations
>>> structure exynos_drm_driver_fops is now being created by the helper macro
>>> DEFINE_DRM_GEM_FOPS().
>>>
>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>
>> s/exynox/exynos in the subject.
>>
>> Patch lgtm, but would still be good if exynos maintainers would
>> ack/review/test it. But if you don't get anything within 2 weeks here's
>
> Sorry for late. Confirmed working well on Odroid board, and had a review. Applied.
> https://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git/log/?h=exynos-drm-next
Thanks a lot!
Best regards
Thomas
>
> Thanks,
> Inki Dae
>
>> my:
>>
>> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>
>>> ---
>>> drivers/gpu/drm/exynos/exynos_drm_drv.c | 13 ++-----
>>> drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 20 ++---------
>>> drivers/gpu/drm/exynos/exynos_drm_gem.c | 43 +++++------------------
>>> drivers/gpu/drm/exynos/exynos_drm_gem.h | 5 ---
>>> 4 files changed, 13 insertions(+), 68 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
>>> index d8f1cf4d6b69..9743b6b17447 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
>>> @@ -102,16 +102,7 @@ static const struct drm_ioctl_desc exynos_ioctls[] = {
>>> DRM_RENDER_ALLOW),
>>> };
>>>
>>> -static const struct file_operations exynos_drm_driver_fops = {
>>> - .owner = THIS_MODULE,
>>> - .open = drm_open,
>>> - .mmap = exynos_drm_gem_mmap,
>>> - .poll = drm_poll,
>>> - .read = drm_read,
>>> - .unlocked_ioctl = drm_ioctl,
>>> - .compat_ioctl = drm_compat_ioctl,
>>> - .release = drm_release,
>>> -};
>>> +DEFINE_DRM_GEM_FOPS(exynos_drm_driver_fops);
>>>
>>> static const struct drm_driver exynos_drm_driver = {
>>> .driver_features = DRIVER_MODESET | DRIVER_GEM
>>> @@ -124,7 +115,7 @@ static const struct drm_driver exynos_drm_driver = {
>>> .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
>>> .gem_prime_import = exynos_drm_gem_prime_import,
>>> .gem_prime_import_sg_table = exynos_drm_gem_prime_import_sg_table,
>>> - .gem_prime_mmap = exynos_drm_gem_prime_mmap,
>>> + .gem_prime_mmap = drm_gem_prime_mmap,
>>> .ioctls = exynos_ioctls,
>>> .num_ioctls = ARRAY_SIZE(exynos_ioctls),
>>> .fops = &exynos_drm_driver_fops,
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
>>> index 5147f5929be7..02c97b9ca926 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
>>> @@ -15,6 +15,7 @@
>>> #include <drm/drm_crtc.h>
>>> #include <drm/drm_fb_helper.h>
>>> #include <drm/drm_fourcc.h>
>>> +#include <drm/drm_prime.h>
>>> #include <drm/drm_probe_helper.h>
>>> #include <drm/exynos_drm.h>
>>>
>>> @@ -39,25 +40,8 @@ static int exynos_drm_fb_mmap(struct fb_info *info,
>>> struct drm_fb_helper *helper = info->par;
>>> struct exynos_drm_fbdev *exynos_fbd = to_exynos_fbdev(helper);
>>> struct exynos_drm_gem *exynos_gem = exynos_fbd->exynos_gem;
>>> - unsigned long vm_size;
>>> - int ret;
>>> -
>>> - vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
>>> -
>>> - vm_size = vma->vm_end - vma->vm_start;
>>> -
>>> - if (vm_size > exynos_gem->size)
>>> - return -EINVAL;
>>>
>>> - ret = dma_mmap_attrs(to_dma_dev(helper->dev), vma, exynos_gem->cookie,
>>> - exynos_gem->dma_addr, exynos_gem->size,
>>> - exynos_gem->dma_attrs);
>>> - if (ret < 0) {
>>> - DRM_DEV_ERROR(to_dma_dev(helper->dev), "failed to mmap.\n");
>>> - return ret;
>>> - }
>>> -
>>> - return 0;
>>> + return drm_gem_prime_mmap(&exynos_gem->base, vma);
>>> }
>>>
>>> static const struct fb_ops exynos_drm_fb_ops = {
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
>>> index 4396224227d1..c4b63902ee7a 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
>>> @@ -17,6 +17,8 @@
>>> #include "exynos_drm_drv.h"
>>> #include "exynos_drm_gem.h"
>>>
>>> +static int exynos_drm_gem_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma);
>>> +
>>> static int exynos_drm_alloc_buf(struct exynos_drm_gem *exynos_gem, bool kvmap)
>>> {
>>> struct drm_device *dev = exynos_gem->base.dev;
>>> @@ -135,6 +137,7 @@ static const struct vm_operations_struct exynos_drm_gem_vm_ops = {
>>> static const struct drm_gem_object_funcs exynos_drm_gem_object_funcs = {
>>> .free = exynos_drm_gem_free_object,
>>> .get_sg_table = exynos_drm_gem_prime_get_sg_table,
>>> + .mmap = exynos_drm_gem_mmap,
>>> .vm_ops = &exynos_drm_gem_vm_ops,
>>> };
>>>
>>> @@ -354,12 +357,16 @@ int exynos_drm_gem_dumb_create(struct drm_file *file_priv,
>>> return 0;
>>> }
>>>
>>> -static int exynos_drm_gem_mmap_obj(struct drm_gem_object *obj,
>>> - struct vm_area_struct *vma)
>>> +static int exynos_drm_gem_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma)
>>> {
>>> struct exynos_drm_gem *exynos_gem = to_exynos_gem(obj);
>>> int ret;
>>>
>>> + if (obj->import_attach)
>>> + return dma_buf_mmap(obj->dma_buf, vma, 0);
>>> +
>>> + vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
>>> +
>>> DRM_DEV_DEBUG_KMS(to_dma_dev(obj->dev), "flags = 0x%x\n",
>>> exynos_gem->flags);
>>>
>>> @@ -385,26 +392,6 @@ static int exynos_drm_gem_mmap_obj(struct drm_gem_object *obj,
>>> return ret;
>>> }
>>>
>>> -int exynos_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
>>> -{
>>> - struct drm_gem_object *obj;
>>> - int ret;
>>> -
>>> - /* set vm_area_struct. */
>>> - ret = drm_gem_mmap(filp, vma);
>>> - if (ret < 0) {
>>> - DRM_ERROR("failed to mmap.\n");
>>> - return ret;
>>> - }
>>> -
>>> - obj = vma->vm_private_data;
>>> -
>>> - if (obj->import_attach)
>>> - return dma_buf_mmap(obj->dma_buf, vma, 0);
>>> -
>>> - return exynos_drm_gem_mmap_obj(obj, vma);
>>> -}
>>> -
>>> /* low-level interface prime helpers */
>>> struct drm_gem_object *exynos_drm_gem_prime_import(struct drm_device *dev,
>>> struct dma_buf *dma_buf)
>>> @@ -466,15 +453,3 @@ exynos_drm_gem_prime_import_sg_table(struct drm_device *dev,
>>> exynos_gem->sgt = sgt;
>>> return &exynos_gem->base;
>>> }
>>> -
>>> -int exynos_drm_gem_prime_mmap(struct drm_gem_object *obj,
>>> - struct vm_area_struct *vma)
>>> -{
>>> - int ret;
>>> -
>>> - ret = drm_gem_mmap_obj(obj, obj->size, vma);
>>> - if (ret < 0)
>>> - return ret;
>>> -
>>> - return exynos_drm_gem_mmap_obj(obj, vma);
>>> -}
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.h b/drivers/gpu/drm/exynos/exynos_drm_gem.h
>>> index a23272fb96fb..79d7e1a87419 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.h
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.h
>>> @@ -96,9 +96,6 @@ int exynos_drm_gem_dumb_create(struct drm_file *file_priv,
>>> struct drm_device *dev,
>>> struct drm_mode_create_dumb *args);
>>>
>>> -/* set vm_flags and we can change the vm attribute to other one at here. */
>>> -int exynos_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma);
>>> -
>>> /* low-level interface prime helpers */
>>> struct drm_gem_object *exynos_drm_gem_prime_import(struct drm_device *dev,
>>> struct dma_buf *dma_buf);
>>> @@ -107,7 +104,5 @@ struct drm_gem_object *
>>> exynos_drm_gem_prime_import_sg_table(struct drm_device *dev,
>>> struct dma_buf_attachment *attach,
>>> struct sg_table *sgt);
>>> -int exynos_drm_gem_prime_mmap(struct drm_gem_object *obj,
>>> - struct vm_area_struct *vma);
>>>
>>> #endif
>>> --
>>> 2.33.1
>>>
>>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] drm/exynox: Implement mmap as GEM object function
2021-11-08 10:28 ` [PATCH 1/3] drm/exynox: Implement mmap as GEM object function Thomas Zimmermann
2021-11-08 15:29 ` Daniel Vetter
@ 2021-11-09 5:08 ` Inki Dae
1 sibling, 0 replies; 12+ messages in thread
From: Inki Dae @ 2021-11-09 5:08 UTC (permalink / raw)
To: Thomas Zimmermann, daniel, airlied, maarten.lankhorst, mripard,
jy0922.shim, sw0312.kim, kyungmin.park, krzysztof.kozlowski,
oleksandr_andrushchenko
Cc: dri-devel, linux-arm-kernel, linux-samsung-soc, xen-devel
Hi,
Really sorry for late. I saw this patch in my mailbox just before. Seems I missed it due to a typo, exynox.
I will review and apply this patch ASAP.
Thanks,
Inki Dae
21. 11. 8. 오후 7:28에 Thomas Zimmermann 이(가) 쓴 글:
> Moving the driver-specific mmap code into a GEM object function allows
> for using DRM helpers for various mmap callbacks.
>
> The respective exynos functions are being removed. The file_operations
> structure exynos_drm_driver_fops is now being created by the helper macro
> DEFINE_DRM_GEM_FOPS().
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
> drivers/gpu/drm/exynos/exynos_drm_drv.c | 13 ++-----
> drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 20 ++---------
> drivers/gpu/drm/exynos/exynos_drm_gem.c | 43 +++++------------------
> drivers/gpu/drm/exynos/exynos_drm_gem.h | 5 ---
> 4 files changed, 13 insertions(+), 68 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index d8f1cf4d6b69..9743b6b17447 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -102,16 +102,7 @@ static const struct drm_ioctl_desc exynos_ioctls[] = {
> DRM_RENDER_ALLOW),
> };
>
> -static const struct file_operations exynos_drm_driver_fops = {
> - .owner = THIS_MODULE,
> - .open = drm_open,
> - .mmap = exynos_drm_gem_mmap,
> - .poll = drm_poll,
> - .read = drm_read,
> - .unlocked_ioctl = drm_ioctl,
> - .compat_ioctl = drm_compat_ioctl,
> - .release = drm_release,
> -};
> +DEFINE_DRM_GEM_FOPS(exynos_drm_driver_fops);
>
> static const struct drm_driver exynos_drm_driver = {
> .driver_features = DRIVER_MODESET | DRIVER_GEM
> @@ -124,7 +115,7 @@ static const struct drm_driver exynos_drm_driver = {
> .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
> .gem_prime_import = exynos_drm_gem_prime_import,
> .gem_prime_import_sg_table = exynos_drm_gem_prime_import_sg_table,
> - .gem_prime_mmap = exynos_drm_gem_prime_mmap,
> + .gem_prime_mmap = drm_gem_prime_mmap,
> .ioctls = exynos_ioctls,
> .num_ioctls = ARRAY_SIZE(exynos_ioctls),
> .fops = &exynos_drm_driver_fops,
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> index 5147f5929be7..02c97b9ca926 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> @@ -15,6 +15,7 @@
> #include <drm/drm_crtc.h>
> #include <drm/drm_fb_helper.h>
> #include <drm/drm_fourcc.h>
> +#include <drm/drm_prime.h>
> #include <drm/drm_probe_helper.h>
> #include <drm/exynos_drm.h>
>
> @@ -39,25 +40,8 @@ static int exynos_drm_fb_mmap(struct fb_info *info,
> struct drm_fb_helper *helper = info->par;
> struct exynos_drm_fbdev *exynos_fbd = to_exynos_fbdev(helper);
> struct exynos_drm_gem *exynos_gem = exynos_fbd->exynos_gem;
> - unsigned long vm_size;
> - int ret;
> -
> - vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
> -
> - vm_size = vma->vm_end - vma->vm_start;
> -
> - if (vm_size > exynos_gem->size)
> - return -EINVAL;
>
> - ret = dma_mmap_attrs(to_dma_dev(helper->dev), vma, exynos_gem->cookie,
> - exynos_gem->dma_addr, exynos_gem->size,
> - exynos_gem->dma_attrs);
> - if (ret < 0) {
> - DRM_DEV_ERROR(to_dma_dev(helper->dev), "failed to mmap.\n");
> - return ret;
> - }
> -
> - return 0;
> + return drm_gem_prime_mmap(&exynos_gem->base, vma);
> }
>
> static const struct fb_ops exynos_drm_fb_ops = {
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> index 4396224227d1..c4b63902ee7a 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> @@ -17,6 +17,8 @@
> #include "exynos_drm_drv.h"
> #include "exynos_drm_gem.h"
>
> +static int exynos_drm_gem_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma);
> +
> static int exynos_drm_alloc_buf(struct exynos_drm_gem *exynos_gem, bool kvmap)
> {
> struct drm_device *dev = exynos_gem->base.dev;
> @@ -135,6 +137,7 @@ static const struct vm_operations_struct exynos_drm_gem_vm_ops = {
> static const struct drm_gem_object_funcs exynos_drm_gem_object_funcs = {
> .free = exynos_drm_gem_free_object,
> .get_sg_table = exynos_drm_gem_prime_get_sg_table,
> + .mmap = exynos_drm_gem_mmap,
> .vm_ops = &exynos_drm_gem_vm_ops,
> };
>
> @@ -354,12 +357,16 @@ int exynos_drm_gem_dumb_create(struct drm_file *file_priv,
> return 0;
> }
>
> -static int exynos_drm_gem_mmap_obj(struct drm_gem_object *obj,
> - struct vm_area_struct *vma)
> +static int exynos_drm_gem_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma)
> {
> struct exynos_drm_gem *exynos_gem = to_exynos_gem(obj);
> int ret;
>
> + if (obj->import_attach)
> + return dma_buf_mmap(obj->dma_buf, vma, 0);
> +
> + vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
> +
> DRM_DEV_DEBUG_KMS(to_dma_dev(obj->dev), "flags = 0x%x\n",
> exynos_gem->flags);
>
> @@ -385,26 +392,6 @@ static int exynos_drm_gem_mmap_obj(struct drm_gem_object *obj,
> return ret;
> }
>
> -int exynos_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
> -{
> - struct drm_gem_object *obj;
> - int ret;
> -
> - /* set vm_area_struct. */
> - ret = drm_gem_mmap(filp, vma);
> - if (ret < 0) {
> - DRM_ERROR("failed to mmap.\n");
> - return ret;
> - }
> -
> - obj = vma->vm_private_data;
> -
> - if (obj->import_attach)
> - return dma_buf_mmap(obj->dma_buf, vma, 0);
> -
> - return exynos_drm_gem_mmap_obj(obj, vma);
> -}
> -
> /* low-level interface prime helpers */
> struct drm_gem_object *exynos_drm_gem_prime_import(struct drm_device *dev,
> struct dma_buf *dma_buf)
> @@ -466,15 +453,3 @@ exynos_drm_gem_prime_import_sg_table(struct drm_device *dev,
> exynos_gem->sgt = sgt;
> return &exynos_gem->base;
> }
> -
> -int exynos_drm_gem_prime_mmap(struct drm_gem_object *obj,
> - struct vm_area_struct *vma)
> -{
> - int ret;
> -
> - ret = drm_gem_mmap_obj(obj, obj->size, vma);
> - if (ret < 0)
> - return ret;
> -
> - return exynos_drm_gem_mmap_obj(obj, vma);
> -}
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.h b/drivers/gpu/drm/exynos/exynos_drm_gem.h
> index a23272fb96fb..79d7e1a87419 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.h
> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.h
> @@ -96,9 +96,6 @@ int exynos_drm_gem_dumb_create(struct drm_file *file_priv,
> struct drm_device *dev,
> struct drm_mode_create_dumb *args);
>
> -/* set vm_flags and we can change the vm attribute to other one at here. */
> -int exynos_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma);
> -
> /* low-level interface prime helpers */
> struct drm_gem_object *exynos_drm_gem_prime_import(struct drm_device *dev,
> struct dma_buf *dma_buf);
> @@ -107,7 +104,5 @@ struct drm_gem_object *
> exynos_drm_gem_prime_import_sg_table(struct drm_device *dev,
> struct dma_buf_attachment *attach,
> struct sg_table *sgt);
> -int exynos_drm_gem_prime_mmap(struct drm_gem_object *obj,
> - struct vm_area_struct *vma);
>
> #endif
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/3] drm/xen: Implement mmap as GEM object function
2021-11-08 10:28 [RESEND PATCH 0/3] drm/{exynos,xen}: Implement gem_prime_mmap with drm_gem_prime_mmap() Thomas Zimmermann
2021-11-08 10:28 ` [PATCH 1/3] drm/exynox: Implement mmap as GEM object function Thomas Zimmermann
@ 2021-11-08 10:28 ` Thomas Zimmermann
2021-11-08 10:46 ` Oleksandr Andrushchenko
2021-11-08 10:28 ` [PATCH 3/3] drm: Update documentation and TODO of gem_prime_mmap hook Thomas Zimmermann
2 siblings, 1 reply; 12+ messages in thread
From: Thomas Zimmermann @ 2021-11-08 10:28 UTC (permalink / raw)
To: daniel, airlied, maarten.lankhorst, mripard, inki.dae,
jy0922.shim, sw0312.kim, kyungmin.park, krzysztof.kozlowski,
oleksandr_andrushchenko
Cc: dri-devel, linux-arm-kernel, linux-samsung-soc, xen-devel,
Thomas Zimmermann
Moving the driver-specific mmap code into a GEM object function allows
for using DRM helpers for various mmap callbacks.
The respective xen functions are being removed. The file_operations
structure fops is now being created by the helper macro
DEFINE_DRM_GEM_FOPS().
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/xen/xen_drm_front.c | 16 +---
drivers/gpu/drm/xen/xen_drm_front_gem.c | 108 +++++++++---------------
drivers/gpu/drm/xen/xen_drm_front_gem.h | 7 --
3 files changed, 44 insertions(+), 87 deletions(-)
diff --git a/drivers/gpu/drm/xen/xen_drm_front.c b/drivers/gpu/drm/xen/xen_drm_front.c
index 9f14d99c763c..434064c820e8 100644
--- a/drivers/gpu/drm/xen/xen_drm_front.c
+++ b/drivers/gpu/drm/xen/xen_drm_front.c
@@ -469,19 +469,7 @@ static void xen_drm_drv_release(struct drm_device *dev)
kfree(drm_info);
}
-static const struct file_operations xen_drm_dev_fops = {
- .owner = THIS_MODULE,
- .open = drm_open,
- .release = drm_release,
- .unlocked_ioctl = drm_ioctl,
-#ifdef CONFIG_COMPAT
- .compat_ioctl = drm_compat_ioctl,
-#endif
- .poll = drm_poll,
- .read = drm_read,
- .llseek = no_llseek,
- .mmap = xen_drm_front_gem_mmap,
-};
+DEFINE_DRM_GEM_FOPS(xen_drm_dev_fops);
static const struct drm_driver xen_drm_driver = {
.driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
@@ -489,7 +477,7 @@ static const struct drm_driver xen_drm_driver = {
.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
.gem_prime_import_sg_table = xen_drm_front_gem_import_sg_table,
- .gem_prime_mmap = xen_drm_front_gem_prime_mmap,
+ .gem_prime_mmap = drm_gem_prime_mmap,
.dumb_create = xen_drm_drv_dumb_create,
.fops = &xen_drm_dev_fops,
.name = "xendrm-du",
diff --git a/drivers/gpu/drm/xen/xen_drm_front_gem.c b/drivers/gpu/drm/xen/xen_drm_front_gem.c
index b293c67230ef..dd358ba2bf8e 100644
--- a/drivers/gpu/drm/xen/xen_drm_front_gem.c
+++ b/drivers/gpu/drm/xen/xen_drm_front_gem.c
@@ -57,6 +57,47 @@ static void gem_free_pages_array(struct xen_gem_object *xen_obj)
xen_obj->pages = NULL;
}
+static int xen_drm_front_gem_object_mmap(struct drm_gem_object *gem_obj,
+ struct vm_area_struct *vma)
+{
+ struct xen_gem_object *xen_obj = to_xen_gem_obj(gem_obj);
+ int ret;
+
+ vma->vm_ops = gem_obj->funcs->vm_ops;
+
+ /*
+ * Clear the VM_PFNMAP flag that was set by drm_gem_mmap(), and set the
+ * vm_pgoff (used as a fake buffer offset by DRM) to 0 as we want to map
+ * the whole buffer.
+ */
+ vma->vm_flags &= ~VM_PFNMAP;
+ vma->vm_flags |= VM_MIXEDMAP;
+ vma->vm_pgoff = 0;
+
+ /*
+ * According to Xen on ARM ABI (xen/include/public/arch-arm.h):
+ * all memory which is shared with other entities in the system
+ * (including the hypervisor and other guests) must reside in memory
+ * which is mapped as Normal Inner Write-Back Outer Write-Back
+ * Inner-Shareable.
+ */
+ vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
+
+ /*
+ * vm_operations_struct.fault handler will be called if CPU access
+ * to VM is here. For GPUs this isn't the case, because CPU doesn't
+ * touch the memory. Insert pages now, so both CPU and GPU are happy.
+ *
+ * FIXME: as we insert all the pages now then no .fault handler must
+ * be called, so don't provide one
+ */
+ ret = vm_map_pages(vma, xen_obj->pages, xen_obj->num_pages);
+ if (ret < 0)
+ DRM_ERROR("Failed to map pages into vma: %d\n", ret);
+
+ return ret;
+}
+
static const struct vm_operations_struct xen_drm_drv_vm_ops = {
.open = drm_gem_vm_open,
.close = drm_gem_vm_close,
@@ -67,6 +108,7 @@ static const struct drm_gem_object_funcs xen_drm_front_gem_object_funcs = {
.get_sg_table = xen_drm_front_gem_get_sg_table,
.vmap = xen_drm_front_gem_prime_vmap,
.vunmap = xen_drm_front_gem_prime_vunmap,
+ .mmap = xen_drm_front_gem_object_mmap,
.vm_ops = &xen_drm_drv_vm_ops,
};
@@ -238,58 +280,6 @@ xen_drm_front_gem_import_sg_table(struct drm_device *dev,
return &xen_obj->base;
}
-static int gem_mmap_obj(struct xen_gem_object *xen_obj,
- struct vm_area_struct *vma)
-{
- int ret;
-
- /*
- * clear the VM_PFNMAP flag that was set by drm_gem_mmap(), and set the
- * vm_pgoff (used as a fake buffer offset by DRM) to 0 as we want to map
- * the whole buffer.
- */
- vma->vm_flags &= ~VM_PFNMAP;
- vma->vm_flags |= VM_MIXEDMAP;
- vma->vm_pgoff = 0;
- /*
- * According to Xen on ARM ABI (xen/include/public/arch-arm.h):
- * all memory which is shared with other entities in the system
- * (including the hypervisor and other guests) must reside in memory
- * which is mapped as Normal Inner Write-Back Outer Write-Back
- * Inner-Shareable.
- */
- vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
-
- /*
- * vm_operations_struct.fault handler will be called if CPU access
- * to VM is here. For GPUs this isn't the case, because CPU
- * doesn't touch the memory. Insert pages now, so both CPU and GPU are
- * happy.
- * FIXME: as we insert all the pages now then no .fault handler must
- * be called, so don't provide one
- */
- ret = vm_map_pages(vma, xen_obj->pages, xen_obj->num_pages);
- if (ret < 0)
- DRM_ERROR("Failed to map pages into vma: %d\n", ret);
-
- return ret;
-}
-
-int xen_drm_front_gem_mmap(struct file *filp, struct vm_area_struct *vma)
-{
- struct xen_gem_object *xen_obj;
- struct drm_gem_object *gem_obj;
- int ret;
-
- ret = drm_gem_mmap(filp, vma);
- if (ret < 0)
- return ret;
-
- gem_obj = vma->vm_private_data;
- xen_obj = to_xen_gem_obj(gem_obj);
- return gem_mmap_obj(xen_obj, vma);
-}
-
int xen_drm_front_gem_prime_vmap(struct drm_gem_object *gem_obj, struct dma_buf_map *map)
{
struct xen_gem_object *xen_obj = to_xen_gem_obj(gem_obj);
@@ -313,17 +303,3 @@ void xen_drm_front_gem_prime_vunmap(struct drm_gem_object *gem_obj,
{
vunmap(map->vaddr);
}
-
-int xen_drm_front_gem_prime_mmap(struct drm_gem_object *gem_obj,
- struct vm_area_struct *vma)
-{
- struct xen_gem_object *xen_obj;
- int ret;
-
- ret = drm_gem_mmap_obj(gem_obj, gem_obj->size, vma);
- if (ret < 0)
- return ret;
-
- xen_obj = to_xen_gem_obj(gem_obj);
- return gem_mmap_obj(xen_obj, vma);
-}
diff --git a/drivers/gpu/drm/xen/xen_drm_front_gem.h b/drivers/gpu/drm/xen/xen_drm_front_gem.h
index a4e67d0a149c..eaea470f7001 100644
--- a/drivers/gpu/drm/xen/xen_drm_front_gem.h
+++ b/drivers/gpu/drm/xen/xen_drm_front_gem.h
@@ -15,9 +15,7 @@ struct dma_buf_attachment;
struct dma_buf_map;
struct drm_device;
struct drm_gem_object;
-struct file;
struct sg_table;
-struct vm_area_struct;
struct drm_gem_object *xen_drm_front_gem_create(struct drm_device *dev,
size_t size);
@@ -33,15 +31,10 @@ struct page **xen_drm_front_gem_get_pages(struct drm_gem_object *obj);
void xen_drm_front_gem_free_object_unlocked(struct drm_gem_object *gem_obj);
-int xen_drm_front_gem_mmap(struct file *filp, struct vm_area_struct *vma);
-
int xen_drm_front_gem_prime_vmap(struct drm_gem_object *gem_obj,
struct dma_buf_map *map);
void xen_drm_front_gem_prime_vunmap(struct drm_gem_object *gem_obj,
struct dma_buf_map *map);
-int xen_drm_front_gem_prime_mmap(struct drm_gem_object *gem_obj,
- struct vm_area_struct *vma);
-
#endif /* __XEN_DRM_FRONT_GEM_H */
--
2.33.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] drm/xen: Implement mmap as GEM object function
2021-11-08 10:28 ` [PATCH 2/3] drm/xen: " Thomas Zimmermann
@ 2021-11-08 10:46 ` Oleksandr Andrushchenko
2021-11-08 11:32 ` Thomas Zimmermann
0 siblings, 1 reply; 12+ messages in thread
From: Oleksandr Andrushchenko @ 2021-11-08 10:46 UTC (permalink / raw)
To: Thomas Zimmermann, daniel, airlied, maarten.lankhorst, mripard,
inki.dae, jy0922.shim, sw0312.kim, kyungmin.park,
krzysztof.kozlowski, Oleksandr Andrushchenko
Cc: dri-devel, linux-arm-kernel, linux-samsung-soc, xen-devel
Hi, Thomas!
On 08.11.21 12:28, Thomas Zimmermann wrote:
> Moving the driver-specific mmap code into a GEM object function allows
> for using DRM helpers for various mmap callbacks.
>
> The respective xen functions are being removed. The file_operations
> structure fops is now being created by the helper macro
> DEFINE_DRM_GEM_FOPS().
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> ---
> drivers/gpu/drm/xen/xen_drm_front.c | 16 +---
> drivers/gpu/drm/xen/xen_drm_front_gem.c | 108 +++++++++---------------
> drivers/gpu/drm/xen/xen_drm_front_gem.h | 7 --
> 3 files changed, 44 insertions(+), 87 deletions(-)
>
> diff --git a/drivers/gpu/drm/xen/xen_drm_front.c b/drivers/gpu/drm/xen/xen_drm_front.c
> index 9f14d99c763c..434064c820e8 100644
> --- a/drivers/gpu/drm/xen/xen_drm_front.c
> +++ b/drivers/gpu/drm/xen/xen_drm_front.c
> @@ -469,19 +469,7 @@ static void xen_drm_drv_release(struct drm_device *dev)
> kfree(drm_info);
> }
>
> -static const struct file_operations xen_drm_dev_fops = {
> - .owner = THIS_MODULE,
> - .open = drm_open,
> - .release = drm_release,
> - .unlocked_ioctl = drm_ioctl,
> -#ifdef CONFIG_COMPAT
> - .compat_ioctl = drm_compat_ioctl,
> -#endif
> - .poll = drm_poll,
> - .read = drm_read,
> - .llseek = no_llseek,
> - .mmap = xen_drm_front_gem_mmap,
> -};
> +DEFINE_DRM_GEM_FOPS(xen_drm_dev_fops);
>
> static const struct drm_driver xen_drm_driver = {
> .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
> @@ -489,7 +477,7 @@ static const struct drm_driver xen_drm_driver = {
> .prime_handle_to_fd = drm_gem_prime_handle_to_fd,
> .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
> .gem_prime_import_sg_table = xen_drm_front_gem_import_sg_table,
> - .gem_prime_mmap = xen_drm_front_gem_prime_mmap,
> + .gem_prime_mmap = drm_gem_prime_mmap,
> .dumb_create = xen_drm_drv_dumb_create,
> .fops = &xen_drm_dev_fops,
> .name = "xendrm-du",
> diff --git a/drivers/gpu/drm/xen/xen_drm_front_gem.c b/drivers/gpu/drm/xen/xen_drm_front_gem.c
> index b293c67230ef..dd358ba2bf8e 100644
> --- a/drivers/gpu/drm/xen/xen_drm_front_gem.c
> +++ b/drivers/gpu/drm/xen/xen_drm_front_gem.c
> @@ -57,6 +57,47 @@ static void gem_free_pages_array(struct xen_gem_object *xen_obj)
> xen_obj->pages = NULL;
> }
>
> +static int xen_drm_front_gem_object_mmap(struct drm_gem_object *gem_obj,
> + struct vm_area_struct *vma)
> +{
> + struct xen_gem_object *xen_obj = to_xen_gem_obj(gem_obj);
> + int ret;
> +
> + vma->vm_ops = gem_obj->funcs->vm_ops;
> +
> + /*
> + * Clear the VM_PFNMAP flag that was set by drm_gem_mmap(), and set the
> + * vm_pgoff (used as a fake buffer offset by DRM) to 0 as we want to map
> + * the whole buffer.
> + */
> + vma->vm_flags &= ~VM_PFNMAP;
> + vma->vm_flags |= VM_MIXEDMAP;
> + vma->vm_pgoff = 0;
> +
> + /*
> + * According to Xen on ARM ABI (xen/include/public/arch-arm.h):
> + * all memory which is shared with other entities in the system
> + * (including the hypervisor and other guests) must reside in memory
> + * which is mapped as Normal Inner Write-Back Outer Write-Back
> + * Inner-Shareable.
> + */
> + vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
> +
> + /*
> + * vm_operations_struct.fault handler will be called if CPU access
> + * to VM is here. For GPUs this isn't the case, because CPU doesn't
> + * touch the memory. Insert pages now, so both CPU and GPU are happy.
> + *
> + * FIXME: as we insert all the pages now then no .fault handler must
> + * be called, so don't provide one
> + */
> + ret = vm_map_pages(vma, xen_obj->pages, xen_obj->num_pages);
> + if (ret < 0)
> + DRM_ERROR("Failed to map pages into vma: %d\n", ret);
> +
> + return ret;
> +}
> +
> static const struct vm_operations_struct xen_drm_drv_vm_ops = {
> .open = drm_gem_vm_open,
> .close = drm_gem_vm_close,
> @@ -67,6 +108,7 @@ static const struct drm_gem_object_funcs xen_drm_front_gem_object_funcs = {
> .get_sg_table = xen_drm_front_gem_get_sg_table,
> .vmap = xen_drm_front_gem_prime_vmap,
> .vunmap = xen_drm_front_gem_prime_vunmap,
> + .mmap = xen_drm_front_gem_object_mmap,
> .vm_ops = &xen_drm_drv_vm_ops,
> };
>
> @@ -238,58 +280,6 @@ xen_drm_front_gem_import_sg_table(struct drm_device *dev,
> return &xen_obj->base;
> }
>
> -static int gem_mmap_obj(struct xen_gem_object *xen_obj,
> - struct vm_area_struct *vma)
> -{
> - int ret;
> -
> - /*
> - * clear the VM_PFNMAP flag that was set by drm_gem_mmap(), and set the
> - * vm_pgoff (used as a fake buffer offset by DRM) to 0 as we want to map
> - * the whole buffer.
> - */
> - vma->vm_flags &= ~VM_PFNMAP;
> - vma->vm_flags |= VM_MIXEDMAP;
> - vma->vm_pgoff = 0;
> - /*
> - * According to Xen on ARM ABI (xen/include/public/arch-arm.h):
> - * all memory which is shared with other entities in the system
> - * (including the hypervisor and other guests) must reside in memory
> - * which is mapped as Normal Inner Write-Back Outer Write-Back
> - * Inner-Shareable.
> - */
> - vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
> -
> - /*
> - * vm_operations_struct.fault handler will be called if CPU access
> - * to VM is here. For GPUs this isn't the case, because CPU
> - * doesn't touch the memory. Insert pages now, so both CPU and GPU are
> - * happy.
> - * FIXME: as we insert all the pages now then no .fault handler must
> - * be called, so don't provide one
> - */
> - ret = vm_map_pages(vma, xen_obj->pages, xen_obj->num_pages);
> - if (ret < 0)
> - DRM_ERROR("Failed to map pages into vma: %d\n", ret);
> -
> - return ret;
> -}
> -
> -int xen_drm_front_gem_mmap(struct file *filp, struct vm_area_struct *vma)
> -{
> - struct xen_gem_object *xen_obj;
> - struct drm_gem_object *gem_obj;
> - int ret;
> -
> - ret = drm_gem_mmap(filp, vma);
> - if (ret < 0)
> - return ret;
> -
> - gem_obj = vma->vm_private_data;
> - xen_obj = to_xen_gem_obj(gem_obj);
> - return gem_mmap_obj(xen_obj, vma);
> -}
> -
> int xen_drm_front_gem_prime_vmap(struct drm_gem_object *gem_obj, struct dma_buf_map *map)
> {
> struct xen_gem_object *xen_obj = to_xen_gem_obj(gem_obj);
> @@ -313,17 +303,3 @@ void xen_drm_front_gem_prime_vunmap(struct drm_gem_object *gem_obj,
> {
> vunmap(map->vaddr);
> }
> -
> -int xen_drm_front_gem_prime_mmap(struct drm_gem_object *gem_obj,
> - struct vm_area_struct *vma)
> -{
> - struct xen_gem_object *xen_obj;
> - int ret;
> -
> - ret = drm_gem_mmap_obj(gem_obj, gem_obj->size, vma);
> - if (ret < 0)
> - return ret;
> -
> - xen_obj = to_xen_gem_obj(gem_obj);
> - return gem_mmap_obj(xen_obj, vma);
> -}
> diff --git a/drivers/gpu/drm/xen/xen_drm_front_gem.h b/drivers/gpu/drm/xen/xen_drm_front_gem.h
> index a4e67d0a149c..eaea470f7001 100644
> --- a/drivers/gpu/drm/xen/xen_drm_front_gem.h
> +++ b/drivers/gpu/drm/xen/xen_drm_front_gem.h
> @@ -15,9 +15,7 @@ struct dma_buf_attachment;
> struct dma_buf_map;
> struct drm_device;
> struct drm_gem_object;
> -struct file;
> struct sg_table;
> -struct vm_area_struct;
>
> struct drm_gem_object *xen_drm_front_gem_create(struct drm_device *dev,
> size_t size);
> @@ -33,15 +31,10 @@ struct page **xen_drm_front_gem_get_pages(struct drm_gem_object *obj);
>
> void xen_drm_front_gem_free_object_unlocked(struct drm_gem_object *gem_obj);
>
> -int xen_drm_front_gem_mmap(struct file *filp, struct vm_area_struct *vma);
> -
> int xen_drm_front_gem_prime_vmap(struct drm_gem_object *gem_obj,
> struct dma_buf_map *map);
>
> void xen_drm_front_gem_prime_vunmap(struct drm_gem_object *gem_obj,
> struct dma_buf_map *map);
>
> -int xen_drm_front_gem_prime_mmap(struct drm_gem_object *gem_obj,
> - struct vm_area_struct *vma);
> -
> #endif /* __XEN_DRM_FRONT_GEM_H */
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] drm/xen: Implement mmap as GEM object function
2021-11-08 10:46 ` Oleksandr Andrushchenko
@ 2021-11-08 11:32 ` Thomas Zimmermann
0 siblings, 0 replies; 12+ messages in thread
From: Thomas Zimmermann @ 2021-11-08 11:32 UTC (permalink / raw)
To: Oleksandr Andrushchenko, daniel, airlied, maarten.lankhorst,
mripard, inki.dae, jy0922.shim, sw0312.kim, kyungmin.park,
krzysztof.kozlowski
Cc: dri-devel, linux-arm-kernel, linux-samsung-soc, xen-devel
[-- Attachment #1.1: Type: text/plain, Size: 8835 bytes --]
Hi
Am 08.11.21 um 11:46 schrieb Oleksandr Andrushchenko:
> Hi, Thomas!
>
> On 08.11.21 12:28, Thomas Zimmermann wrote:
>> Moving the driver-specific mmap code into a GEM object function allows
>> for using DRM helpers for various mmap callbacks.
>>
>> The respective xen functions are being removed. The file_operations
>> structure fops is now being created by the helper macro
>> DEFINE_DRM_GEM_FOPS().
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Reviewed-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
That was quick! Thanks a lot.
Best regards
Thomas
>> ---
>> drivers/gpu/drm/xen/xen_drm_front.c | 16 +---
>> drivers/gpu/drm/xen/xen_drm_front_gem.c | 108 +++++++++---------------
>> drivers/gpu/drm/xen/xen_drm_front_gem.h | 7 --
>> 3 files changed, 44 insertions(+), 87 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xen/xen_drm_front.c b/drivers/gpu/drm/xen/xen_drm_front.c
>> index 9f14d99c763c..434064c820e8 100644
>> --- a/drivers/gpu/drm/xen/xen_drm_front.c
>> +++ b/drivers/gpu/drm/xen/xen_drm_front.c
>> @@ -469,19 +469,7 @@ static void xen_drm_drv_release(struct drm_device *dev)
>> kfree(drm_info);
>> }
>>
>> -static const struct file_operations xen_drm_dev_fops = {
>> - .owner = THIS_MODULE,
>> - .open = drm_open,
>> - .release = drm_release,
>> - .unlocked_ioctl = drm_ioctl,
>> -#ifdef CONFIG_COMPAT
>> - .compat_ioctl = drm_compat_ioctl,
>> -#endif
>> - .poll = drm_poll,
>> - .read = drm_read,
>> - .llseek = no_llseek,
>> - .mmap = xen_drm_front_gem_mmap,
>> -};
>> +DEFINE_DRM_GEM_FOPS(xen_drm_dev_fops);
>>
>> static const struct drm_driver xen_drm_driver = {
>> .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
>> @@ -489,7 +477,7 @@ static const struct drm_driver xen_drm_driver = {
>> .prime_handle_to_fd = drm_gem_prime_handle_to_fd,
>> .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
>> .gem_prime_import_sg_table = xen_drm_front_gem_import_sg_table,
>> - .gem_prime_mmap = xen_drm_front_gem_prime_mmap,
>> + .gem_prime_mmap = drm_gem_prime_mmap,
>> .dumb_create = xen_drm_drv_dumb_create,
>> .fops = &xen_drm_dev_fops,
>> .name = "xendrm-du",
>> diff --git a/drivers/gpu/drm/xen/xen_drm_front_gem.c b/drivers/gpu/drm/xen/xen_drm_front_gem.c
>> index b293c67230ef..dd358ba2bf8e 100644
>> --- a/drivers/gpu/drm/xen/xen_drm_front_gem.c
>> +++ b/drivers/gpu/drm/xen/xen_drm_front_gem.c
>> @@ -57,6 +57,47 @@ static void gem_free_pages_array(struct xen_gem_object *xen_obj)
>> xen_obj->pages = NULL;
>> }
>>
>> +static int xen_drm_front_gem_object_mmap(struct drm_gem_object *gem_obj,
>> + struct vm_area_struct *vma)
>> +{
>> + struct xen_gem_object *xen_obj = to_xen_gem_obj(gem_obj);
>> + int ret;
>> +
>> + vma->vm_ops = gem_obj->funcs->vm_ops;
>> +
>> + /*
>> + * Clear the VM_PFNMAP flag that was set by drm_gem_mmap(), and set the
>> + * vm_pgoff (used as a fake buffer offset by DRM) to 0 as we want to map
>> + * the whole buffer.
>> + */
>> + vma->vm_flags &= ~VM_PFNMAP;
>> + vma->vm_flags |= VM_MIXEDMAP;
>> + vma->vm_pgoff = 0;
>> +
>> + /*
>> + * According to Xen on ARM ABI (xen/include/public/arch-arm.h):
>> + * all memory which is shared with other entities in the system
>> + * (including the hypervisor and other guests) must reside in memory
>> + * which is mapped as Normal Inner Write-Back Outer Write-Back
>> + * Inner-Shareable.
>> + */
>> + vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
>> +
>> + /*
>> + * vm_operations_struct.fault handler will be called if CPU access
>> + * to VM is here. For GPUs this isn't the case, because CPU doesn't
>> + * touch the memory. Insert pages now, so both CPU and GPU are happy.
>> + *
>> + * FIXME: as we insert all the pages now then no .fault handler must
>> + * be called, so don't provide one
>> + */
>> + ret = vm_map_pages(vma, xen_obj->pages, xen_obj->num_pages);
>> + if (ret < 0)
>> + DRM_ERROR("Failed to map pages into vma: %d\n", ret);
>> +
>> + return ret;
>> +}
>> +
>> static const struct vm_operations_struct xen_drm_drv_vm_ops = {
>> .open = drm_gem_vm_open,
>> .close = drm_gem_vm_close,
>> @@ -67,6 +108,7 @@ static const struct drm_gem_object_funcs xen_drm_front_gem_object_funcs = {
>> .get_sg_table = xen_drm_front_gem_get_sg_table,
>> .vmap = xen_drm_front_gem_prime_vmap,
>> .vunmap = xen_drm_front_gem_prime_vunmap,
>> + .mmap = xen_drm_front_gem_object_mmap,
>> .vm_ops = &xen_drm_drv_vm_ops,
>> };
>>
>> @@ -238,58 +280,6 @@ xen_drm_front_gem_import_sg_table(struct drm_device *dev,
>> return &xen_obj->base;
>> }
>>
>> -static int gem_mmap_obj(struct xen_gem_object *xen_obj,
>> - struct vm_area_struct *vma)
>> -{
>> - int ret;
>> -
>> - /*
>> - * clear the VM_PFNMAP flag that was set by drm_gem_mmap(), and set the
>> - * vm_pgoff (used as a fake buffer offset by DRM) to 0 as we want to map
>> - * the whole buffer.
>> - */
>> - vma->vm_flags &= ~VM_PFNMAP;
>> - vma->vm_flags |= VM_MIXEDMAP;
>> - vma->vm_pgoff = 0;
>> - /*
>> - * According to Xen on ARM ABI (xen/include/public/arch-arm.h):
>> - * all memory which is shared with other entities in the system
>> - * (including the hypervisor and other guests) must reside in memory
>> - * which is mapped as Normal Inner Write-Back Outer Write-Back
>> - * Inner-Shareable.
>> - */
>> - vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
>> -
>> - /*
>> - * vm_operations_struct.fault handler will be called if CPU access
>> - * to VM is here. For GPUs this isn't the case, because CPU
>> - * doesn't touch the memory. Insert pages now, so both CPU and GPU are
>> - * happy.
>> - * FIXME: as we insert all the pages now then no .fault handler must
>> - * be called, so don't provide one
>> - */
>> - ret = vm_map_pages(vma, xen_obj->pages, xen_obj->num_pages);
>> - if (ret < 0)
>> - DRM_ERROR("Failed to map pages into vma: %d\n", ret);
>> -
>> - return ret;
>> -}
>> -
>> -int xen_drm_front_gem_mmap(struct file *filp, struct vm_area_struct *vma)
>> -{
>> - struct xen_gem_object *xen_obj;
>> - struct drm_gem_object *gem_obj;
>> - int ret;
>> -
>> - ret = drm_gem_mmap(filp, vma);
>> - if (ret < 0)
>> - return ret;
>> -
>> - gem_obj = vma->vm_private_data;
>> - xen_obj = to_xen_gem_obj(gem_obj);
>> - return gem_mmap_obj(xen_obj, vma);
>> -}
>> -
>> int xen_drm_front_gem_prime_vmap(struct drm_gem_object *gem_obj, struct dma_buf_map *map)
>> {
>> struct xen_gem_object *xen_obj = to_xen_gem_obj(gem_obj);
>> @@ -313,17 +303,3 @@ void xen_drm_front_gem_prime_vunmap(struct drm_gem_object *gem_obj,
>> {
>> vunmap(map->vaddr);
>> }
>> -
>> -int xen_drm_front_gem_prime_mmap(struct drm_gem_object *gem_obj,
>> - struct vm_area_struct *vma)
>> -{
>> - struct xen_gem_object *xen_obj;
>> - int ret;
>> -
>> - ret = drm_gem_mmap_obj(gem_obj, gem_obj->size, vma);
>> - if (ret < 0)
>> - return ret;
>> -
>> - xen_obj = to_xen_gem_obj(gem_obj);
>> - return gem_mmap_obj(xen_obj, vma);
>> -}
>> diff --git a/drivers/gpu/drm/xen/xen_drm_front_gem.h b/drivers/gpu/drm/xen/xen_drm_front_gem.h
>> index a4e67d0a149c..eaea470f7001 100644
>> --- a/drivers/gpu/drm/xen/xen_drm_front_gem.h
>> +++ b/drivers/gpu/drm/xen/xen_drm_front_gem.h
>> @@ -15,9 +15,7 @@ struct dma_buf_attachment;
>> struct dma_buf_map;
>> struct drm_device;
>> struct drm_gem_object;
>> -struct file;
>> struct sg_table;
>> -struct vm_area_struct;
>>
>> struct drm_gem_object *xen_drm_front_gem_create(struct drm_device *dev,
>> size_t size);
>> @@ -33,15 +31,10 @@ struct page **xen_drm_front_gem_get_pages(struct drm_gem_object *obj);
>>
>> void xen_drm_front_gem_free_object_unlocked(struct drm_gem_object *gem_obj);
>>
>> -int xen_drm_front_gem_mmap(struct file *filp, struct vm_area_struct *vma);
>> -
>> int xen_drm_front_gem_prime_vmap(struct drm_gem_object *gem_obj,
>> struct dma_buf_map *map);
>>
>> void xen_drm_front_gem_prime_vunmap(struct drm_gem_object *gem_obj,
>> struct dma_buf_map *map);
>>
>> -int xen_drm_front_gem_prime_mmap(struct drm_gem_object *gem_obj,
>> - struct vm_area_struct *vma);
>> -
>> #endif /* __XEN_DRM_FRONT_GEM_H */
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/3] drm: Update documentation and TODO of gem_prime_mmap hook
2021-11-08 10:28 [RESEND PATCH 0/3] drm/{exynos,xen}: Implement gem_prime_mmap with drm_gem_prime_mmap() Thomas Zimmermann
2021-11-08 10:28 ` [PATCH 1/3] drm/exynox: Implement mmap as GEM object function Thomas Zimmermann
2021-11-08 10:28 ` [PATCH 2/3] drm/xen: " Thomas Zimmermann
@ 2021-11-08 10:28 ` Thomas Zimmermann
2021-11-10 10:46 ` Daniel Vetter
2 siblings, 1 reply; 12+ messages in thread
From: Thomas Zimmermann @ 2021-11-08 10:28 UTC (permalink / raw)
To: daniel, airlied, maarten.lankhorst, mripard, inki.dae,
jy0922.shim, sw0312.kim, kyungmin.park, krzysztof.kozlowski,
oleksandr_andrushchenko
Cc: dri-devel, linux-arm-kernel, linux-samsung-soc, xen-devel,
Thomas Zimmermann
The hook gem_prime_mmap in struct drm_driver is deprecated. Document
the new requirements.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
Documentation/gpu/todo.rst | 11 -----------
include/drm/drm_drv.h | 11 +++++++----
2 files changed, 7 insertions(+), 15 deletions(-)
diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 60d1d7ee0719..6593c4c4006e 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -268,17 +268,6 @@ Contact: Daniel Vetter
Level: Intermediate
-Clean up mmap forwarding
-------------------------
-
-A lot of drivers forward gem mmap calls to dma-buf mmap for imported buffers.
-And also a lot of them forward dma-buf mmap to the gem mmap implementations.
-There's drm_gem_prime_mmap() for this now, but still needs to be rolled out.
-
-Contact: Daniel Vetter
-
-Level: Intermediate
-
Generic fbdev defio support
---------------------------
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index 0cd95953cdf5..a84eb4028e5b 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -345,11 +345,14 @@ struct drm_driver {
* mmap hook for GEM drivers, used to implement dma-buf mmap in the
* PRIME helpers.
*
- * FIXME: There's way too much duplication going on here, and also moved
- * to &drm_gem_object_funcs.
+ * This hook only exists for historical reasons. Drivers must use
+ * drm_gem_prime_mmap() to implement it.
+ *
+ * FIXME: Convert all drivers to implement mmap in struct
+ * &drm_gem_object_funcs and inline drm_gem_prime_mmap() into
+ * its callers. This hook should be removed afterwards.
*/
- int (*gem_prime_mmap)(struct drm_gem_object *obj,
- struct vm_area_struct *vma);
+ int (*gem_prime_mmap)(struct drm_gem_object *obj, struct vm_area_struct *vma);
/**
* @dumb_create:
--
2.33.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] drm: Update documentation and TODO of gem_prime_mmap hook
2021-11-08 10:28 ` [PATCH 3/3] drm: Update documentation and TODO of gem_prime_mmap hook Thomas Zimmermann
@ 2021-11-10 10:46 ` Daniel Vetter
2021-11-10 12:56 ` Thomas Zimmermann
0 siblings, 1 reply; 12+ messages in thread
From: Daniel Vetter @ 2021-11-10 10:46 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: daniel, airlied, maarten.lankhorst, mripard, inki.dae,
jy0922.shim, sw0312.kim, kyungmin.park, krzysztof.kozlowski,
oleksandr_andrushchenko, dri-devel, linux-arm-kernel,
linux-samsung-soc, xen-devel
On Mon, Nov 08, 2021 at 11:28:46AM +0100, Thomas Zimmermann wrote:
> The hook gem_prime_mmap in struct drm_driver is deprecated. Document
> the new requirements.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> Documentation/gpu/todo.rst | 11 -----------
> include/drm/drm_drv.h | 11 +++++++----
> 2 files changed, 7 insertions(+), 15 deletions(-)
>
> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> index 60d1d7ee0719..6593c4c4006e 100644
> --- a/Documentation/gpu/todo.rst
> +++ b/Documentation/gpu/todo.rst
> @@ -268,17 +268,6 @@ Contact: Daniel Vetter
>
> Level: Intermediate
>
> -Clean up mmap forwarding
> -------------------------
> -
> -A lot of drivers forward gem mmap calls to dma-buf mmap for imported buffers.
> -And also a lot of them forward dma-buf mmap to the gem mmap implementations.
> -There's drm_gem_prime_mmap() for this now, but still needs to be rolled out.
> -
> -Contact: Daniel Vetter
> -
> -Level: Intermediate
> -
> Generic fbdev defio support
> ---------------------------
>
> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> index 0cd95953cdf5..a84eb4028e5b 100644
> --- a/include/drm/drm_drv.h
> +++ b/include/drm/drm_drv.h
> @@ -345,11 +345,14 @@ struct drm_driver {
> * mmap hook for GEM drivers, used to implement dma-buf mmap in the
> * PRIME helpers.
> *
> - * FIXME: There's way too much duplication going on here, and also moved
> - * to &drm_gem_object_funcs.
> + * This hook only exists for historical reasons. Drivers must use
> + * drm_gem_prime_mmap() to implement it.
> + *
> + * FIXME: Convert all drivers to implement mmap in struct
> + * &drm_gem_object_funcs and inline drm_gem_prime_mmap() into
> + * its callers. This hook should be removed afterwards.
I think at least i915 has different semantics between dma-buf mmap and gem
mmap in some cases, so this might be tricky to achieve. But I think
everywhere else this should be solid.
-Daniel
> */
> - int (*gem_prime_mmap)(struct drm_gem_object *obj,
> - struct vm_area_struct *vma);
> + int (*gem_prime_mmap)(struct drm_gem_object *obj, struct vm_area_struct *vma);
>
> /**
> * @dumb_create:
> --
> 2.33.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] drm: Update documentation and TODO of gem_prime_mmap hook
2021-11-10 10:46 ` Daniel Vetter
@ 2021-11-10 12:56 ` Thomas Zimmermann
0 siblings, 0 replies; 12+ messages in thread
From: Thomas Zimmermann @ 2021-11-10 12:56 UTC (permalink / raw)
To: Daniel Vetter
Cc: linux-samsung-soc, jy0922.shim, krzysztof.kozlowski, airlied,
sw0312.kim, oleksandr_andrushchenko, kyungmin.park, dri-devel,
xen-devel, linux-arm-kernel
[-- Attachment #1.1: Type: text/plain, Size: 3061 bytes --]
Hi
Am 10.11.21 um 11:46 schrieb Daniel Vetter:
> On Mon, Nov 08, 2021 at 11:28:46AM +0100, Thomas Zimmermann wrote:
>> The hook gem_prime_mmap in struct drm_driver is deprecated. Document
>> the new requirements.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Thanks a lot!
With the remaining patches merged, all drivers will implement
gem_prime_mmap via drm_gem_prime_mmap() and use the GEM object's mmap
callback for specific details.
The next step would be to move all custom implementations of
file_operations.mmap into the GEM object's mmap and implement the
file_operations callback with drm_mmap().
Best regards
Thomas
>
>> ---
>> Documentation/gpu/todo.rst | 11 -----------
>> include/drm/drm_drv.h | 11 +++++++----
>> 2 files changed, 7 insertions(+), 15 deletions(-)
>>
>> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
>> index 60d1d7ee0719..6593c4c4006e 100644
>> --- a/Documentation/gpu/todo.rst
>> +++ b/Documentation/gpu/todo.rst
>> @@ -268,17 +268,6 @@ Contact: Daniel Vetter
>>
>> Level: Intermediate
>>
>> -Clean up mmap forwarding
>> -------------------------
>> -
>> -A lot of drivers forward gem mmap calls to dma-buf mmap for imported buffers.
>> -And also a lot of them forward dma-buf mmap to the gem mmap implementations.
>> -There's drm_gem_prime_mmap() for this now, but still needs to be rolled out.
>> -
>> -Contact: Daniel Vetter
>> -
>> -Level: Intermediate
>> -
>> Generic fbdev defio support
>> ---------------------------
>>
>> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
>> index 0cd95953cdf5..a84eb4028e5b 100644
>> --- a/include/drm/drm_drv.h
>> +++ b/include/drm/drm_drv.h
>> @@ -345,11 +345,14 @@ struct drm_driver {
>> * mmap hook for GEM drivers, used to implement dma-buf mmap in the
>> * PRIME helpers.
>> *
>> - * FIXME: There's way too much duplication going on here, and also moved
>> - * to &drm_gem_object_funcs.
>> + * This hook only exists for historical reasons. Drivers must use
>> + * drm_gem_prime_mmap() to implement it.
>> + *
>> + * FIXME: Convert all drivers to implement mmap in struct
>> + * &drm_gem_object_funcs and inline drm_gem_prime_mmap() into
>> + * its callers. This hook should be removed afterwards.
>
> I think at least i915 has different semantics between dma-buf mmap and gem
> mmap in some cases, so this might be tricky to achieve. But I think
> everywhere else this should be solid.
> -Daniel
>
>> */
>> - int (*gem_prime_mmap)(struct drm_gem_object *obj,
>> - struct vm_area_struct *vma);
>> + int (*gem_prime_mmap)(struct drm_gem_object *obj, struct vm_area_struct *vma);
>>
>> /**
>> * @dumb_create:
>> --
>> 2.33.1
>>
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread