[PATCH 08/14] drm/exynos: create a fake mmap offset with gem creation
2015ë
11ì 17ì¼ 01:23ì Daniel Vetter ì´(ê°) ì´ ê¸:
> On Mon, Nov 16, 2015 at 05:22:42PM +0100, Daniel Vetter wrote:
>> On Tue, Jul 28, 2015 at 05:53:23PM +0900, Joonyoung Shim wrote:
>>> Don't create a fake mmap offset in exynos_drm_gem_dumb_map_offset. If
>>> not, it will call drm_gem_create_mmap_offset whenever user requests
>>> DRM_IOCTL_MODE_MAP_DUMB ioctl.
>>>
>>> Signed-off-by: Joonyoung Shim
>>> ---
>>> drivers/gpu/drm/exynos/exynos_drm_gem.c | 12 +++-
>>> 1 file changed, 7 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c
>>> b/drivers/gpu/drm/exynos/exynos_drm_gem.c
>>> index 550d267..c76aa8a 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
>>> @@ -151,6 +151,13 @@ struct exynos_drm_gem_obj *exynos_drm_gem_init(struct
>>> drm_device *dev,
>>> return ERR_PTR(ret);
>>> }
>>>
>>> + ret = drm_gem_create_mmap_offset(obj);
>>> + if (ret < 0) {
>>> + drm_gem_object_release(obj);
>>> + kfree(exynos_gem_obj);
>>> + return ERR_PTR(ret);
>>> + }
>>> +
>>> DRM_DEBUG_KMS("created file object = 0x%x\n", (unsigned int)obj->filp);
>>>
>>> return exynos_gem_obj;
>>> @@ -521,14 +528,9 @@ int exynos_drm_gem_dumb_map_offset(struct drm_file
>>> *file_priv,
>>> goto unlock;
>>> }
>>>
>>> - ret = drm_gem_create_mmap_offset(obj);
>>
>> drm_gem_create_mmap_offset internally checks whether it's been already
>> (protected by locks), so this code is perfectly fine. I don't see any
>> justification for this change (but only noticed it because rockchip
>> cargo-culted this change).
>
> I think it'd be good to revert this to stay consistent with cma helpers
> and other drivers.
At least, seems cma halper doesn't also call drm_gem_create_mmap_offset function
at drm_gem_cma_dumb_map_offset function and calls it at cma creation instead.
So I think now Exynos drm keeps consistent with cma helper.
Thanks,
Inki Dae
> -Daniel
>
>> -Daniel
>>
>>> - if (ret)
>>> - goto out;
>>> -
>>> *offset = drm_vma_node_offset_addr(>vma_node);
>>> DRM_DEBUG_KMS("offset = 0x%lx\n", (unsigned long)*offset);
>>>
>>> -out:
>>> drm_gem_object_unreference(obj);
>>> unlock:
>>> mutex_unlock(>struct_mutex);
>>> --
>>> 1.9.1
>>>
>>> ___
>>> dri-devel mailing list
>>> dri-devel at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> http://blog.ffwll.ch
>
[PATCH 08/14] drm/exynos: create a fake mmap offset with gem creation
On Tue, Nov 17, 2015 at 11:53:28AM +0900, Inki Dae wrote:
>
>
> 2015ë
11ì 17ì¼ 01:23ì Daniel Vetter ì´(ê°) ì´ ê¸:
> > On Mon, Nov 16, 2015 at 05:22:42PM +0100, Daniel Vetter wrote:
> >> On Tue, Jul 28, 2015 at 05:53:23PM +0900, Joonyoung Shim wrote:
> >>> Don't create a fake mmap offset in exynos_drm_gem_dumb_map_offset. If
> >>> not, it will call drm_gem_create_mmap_offset whenever user requests
> >>> DRM_IOCTL_MODE_MAP_DUMB ioctl.
> >>>
> >>> Signed-off-by: Joonyoung Shim
> >>> ---
> >>> drivers/gpu/drm/exynos/exynos_drm_gem.c | 12 +++-
> >>> 1 file changed, 7 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> >>> b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> >>> index 550d267..c76aa8a 100644
> >>> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> >>> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> >>> @@ -151,6 +151,13 @@ struct exynos_drm_gem_obj
> >>> *exynos_drm_gem_init(struct drm_device *dev,
> >>> return ERR_PTR(ret);
> >>> }
> >>>
> >>> + ret = drm_gem_create_mmap_offset(obj);
> >>> + if (ret < 0) {
> >>> + drm_gem_object_release(obj);
> >>> + kfree(exynos_gem_obj);
> >>> + return ERR_PTR(ret);
> >>> + }
> >>> +
> >>> DRM_DEBUG_KMS("created file object = 0x%x\n", (unsigned int)obj->filp);
> >>>
> >>> return exynos_gem_obj;
> >>> @@ -521,14 +528,9 @@ int exynos_drm_gem_dumb_map_offset(struct drm_file
> >>> *file_priv,
> >>> goto unlock;
> >>> }
> >>>
> >>> - ret = drm_gem_create_mmap_offset(obj);
> >>
> >> drm_gem_create_mmap_offset internally checks whether it's been already
> >> (protected by locks), so this code is perfectly fine. I don't see any
> >> justification for this change (but only noticed it because rockchip
> >> cargo-culted this change).
> >
> > I think it'd be good to revert this to stay consistent with cma helpers
> > and other drivers.
>
> At least, seems cma halper doesn't also call drm_gem_create_mmap_offset
> function
> at drm_gem_cma_dumb_map_offset function and calls it at cma creation instead.
> So I think now Exynos drm keeps consistent with cma helper.
Indeed, common style is to create the offset at alloc time - I checked a
few other drivers too. Never mind my comment, sorry for the noise.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[PATCH 08/14] drm/exynos: create a fake mmap offset with gem creation
On Mon, Nov 16, 2015 at 05:22:42PM +0100, Daniel Vetter wrote:
> On Tue, Jul 28, 2015 at 05:53:23PM +0900, Joonyoung Shim wrote:
> > Don't create a fake mmap offset in exynos_drm_gem_dumb_map_offset. If
> > not, it will call drm_gem_create_mmap_offset whenever user requests
> > DRM_IOCTL_MODE_MAP_DUMB ioctl.
> >
> > Signed-off-by: Joonyoung Shim
> > ---
> > drivers/gpu/drm/exynos/exynos_drm_gem.c | 12 +++-
> > 1 file changed, 7 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> > b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> > index 550d267..c76aa8a 100644
> > --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> > +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> > @@ -151,6 +151,13 @@ struct exynos_drm_gem_obj *exynos_drm_gem_init(struct
> > drm_device *dev,
> > return ERR_PTR(ret);
> > }
> >
> > + ret = drm_gem_create_mmap_offset(obj);
> > + if (ret < 0) {
> > + drm_gem_object_release(obj);
> > + kfree(exynos_gem_obj);
> > + return ERR_PTR(ret);
> > + }
> > +
> > DRM_DEBUG_KMS("created file object = 0x%x\n", (unsigned int)obj->filp);
> >
> > return exynos_gem_obj;
> > @@ -521,14 +528,9 @@ int exynos_drm_gem_dumb_map_offset(struct drm_file
> > *file_priv,
> > goto unlock;
> > }
> >
> > - ret = drm_gem_create_mmap_offset(obj);
>
> drm_gem_create_mmap_offset internally checks whether it's been already
> (protected by locks), so this code is perfectly fine. I don't see any
> justification for this change (but only noticed it because rockchip
> cargo-culted this change).
I think it'd be good to revert this to stay consistent with cma helpers
and other drivers.
-Daniel
> -Daniel
>
> > - if (ret)
> > - goto out;
> > -
> > *offset = drm_vma_node_offset_addr(>vma_node);
> > DRM_DEBUG_KMS("offset = 0x%lx\n", (unsigned long)*offset);
> >
> > -out:
> > drm_gem_object_unreference(obj);
> > unlock:
> > mutex_unlock(>struct_mutex);
> > --
> > 1.9.1
> >
> > ___
> > dri-devel mailing list
> > dri-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[PATCH 08/14] drm/exynos: create a fake mmap offset with gem creation
On Tue, Jul 28, 2015 at 05:53:23PM +0900, Joonyoung Shim wrote:
> Don't create a fake mmap offset in exynos_drm_gem_dumb_map_offset. If
> not, it will call drm_gem_create_mmap_offset whenever user requests
> DRM_IOCTL_MODE_MAP_DUMB ioctl.
>
> Signed-off-by: Joonyoung Shim
> ---
> drivers/gpu/drm/exynos/exynos_drm_gem.c | 12 +++-
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> index 550d267..c76aa8a 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> @@ -151,6 +151,13 @@ struct exynos_drm_gem_obj *exynos_drm_gem_init(struct
> drm_device *dev,
> return ERR_PTR(ret);
> }
>
> + ret = drm_gem_create_mmap_offset(obj);
> + if (ret < 0) {
> + drm_gem_object_release(obj);
> + kfree(exynos_gem_obj);
> + return ERR_PTR(ret);
> + }
> +
> DRM_DEBUG_KMS("created file object = 0x%x\n", (unsigned int)obj->filp);
>
> return exynos_gem_obj;
> @@ -521,14 +528,9 @@ int exynos_drm_gem_dumb_map_offset(struct drm_file
> *file_priv,
> goto unlock;
> }
>
> - ret = drm_gem_create_mmap_offset(obj);
drm_gem_create_mmap_offset internally checks whether it's been already
(protected by locks), so this code is perfectly fine. I don't see any
justification for this change (but only noticed it because rockchip
cargo-culted this change).
-Daniel
> - if (ret)
> - goto out;
> -
> *offset = drm_vma_node_offset_addr(>vma_node);
> DRM_DEBUG_KMS("offset = 0x%lx\n", (unsigned long)*offset);
>
> -out:
> drm_gem_object_unreference(obj);
> unlock:
> mutex_unlock(>struct_mutex);
> --
> 1.9.1
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[PATCH 08/14] drm/exynos: create a fake mmap offset with gem creation
On 2015ë
08ì 17ì¼ 14:29, Joonyoung Shim wrote:
> On 08/16/2015 01:50 PM, Inki Dae wrote:
>> On 2015ë
07ì 28ì¼ 17:53, Joonyoung Shim wrote:
>>> Don't create a fake mmap offset in exynos_drm_gem_dumb_map_offset. If
>>> not, it will call drm_gem_create_mmap_offset whenever user requests
>>> DRM_IOCTL_MODE_MAP_DUMB ioctl.
>>
>> This patch makes drm_gem_create_mmap_offset to be called even in case of
>> not using dumb* interfaces. I.e.,
>> exynos_drm_gem_create_ioctl -> exynos_drm_gem_mmap
>>
>
> I know mmap() of exynos-drm also needs mmap offset in drm_gem_mmap().
Ah, sorry. We already removed Exynos specific mmap ioctl. So we could
never use direct mapping ioctl anymore. I confused that.
>
>> And drm_gem_create_mmap_offset checks if vma_node was already allocated
>> or not so this patch doesn't make sense.
>>
>
> OK, but it calls drm_gem_create_mmap_offset still and will be returned
> after checking node->allocated. It's not unnecessary to me.
>
>> Thanks,
>> Inki Dae
>>
>>>
>>> Signed-off-by: Joonyoung Shim
>>> ---
>>> drivers/gpu/drm/exynos/exynos_drm_gem.c | 12 +++-
>>> 1 file changed, 7 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c
>>> b/drivers/gpu/drm/exynos/exynos_drm_gem.c
>>> index 550d267..c76aa8a 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
>>> @@ -151,6 +151,13 @@ struct exynos_drm_gem_obj *exynos_drm_gem_init(struct
>>> drm_device *dev,
>>> return ERR_PTR(ret);
>>> }
>>>
>>> + ret = drm_gem_create_mmap_offset(obj);
>>> + if (ret < 0) {
>>> + drm_gem_object_release(obj);
>>> + kfree(exynos_gem_obj);
>>> + return ERR_PTR(ret);
>>> + }
>>> +
>>> DRM_DEBUG_KMS("created file object = 0x%x\n", (unsigned int)obj->filp);
>>>
>>> return exynos_gem_obj;
>>> @@ -521,14 +528,9 @@ int exynos_drm_gem_dumb_map_offset(struct drm_file
>>> *file_priv,
>>> goto unlock;
>>> }
>>>
>>> - ret = drm_gem_create_mmap_offset(obj);
>>> - if (ret)
>>> - goto out;
>>> -
>>> *offset = drm_vma_node_offset_addr(>vma_node);
>>> DRM_DEBUG_KMS("offset = 0x%lx\n", (unsigned long)*offset);
>>>
>>> -out:
>>> drm_gem_object_unreference(obj);
>>> unlock:
>>> mutex_unlock(>struct_mutex);
>>>
>>
>>
>
>
[PATCH 08/14] drm/exynos: create a fake mmap offset with gem creation
On 08/16/2015 01:50 PM, Inki Dae wrote:
> On 2015ë
07ì 28ì¼ 17:53, Joonyoung Shim wrote:
>> Don't create a fake mmap offset in exynos_drm_gem_dumb_map_offset. If
>> not, it will call drm_gem_create_mmap_offset whenever user requests
>> DRM_IOCTL_MODE_MAP_DUMB ioctl.
>
> This patch makes drm_gem_create_mmap_offset to be called even in case of
> not using dumb* interfaces. I.e.,
> exynos_drm_gem_create_ioctl -> exynos_drm_gem_mmap
>
I know mmap() of exynos-drm also needs mmap offset in drm_gem_mmap().
> And drm_gem_create_mmap_offset checks if vma_node was already allocated
> or not so this patch doesn't make sense.
>
OK, but it calls drm_gem_create_mmap_offset still and will be returned
after checking node->allocated. It's not unnecessary to me.
> Thanks,
> Inki Dae
>
>>
>> Signed-off-by: Joonyoung Shim
>> ---
>> drivers/gpu/drm/exynos/exynos_drm_gem.c | 12 +++-
>> 1 file changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c
>> b/drivers/gpu/drm/exynos/exynos_drm_gem.c
>> index 550d267..c76aa8a 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
>> @@ -151,6 +151,13 @@ struct exynos_drm_gem_obj *exynos_drm_gem_init(struct
>> drm_device *dev,
>> return ERR_PTR(ret);
>> }
>>
>> +ret = drm_gem_create_mmap_offset(obj);
>> +if (ret < 0) {
>> +drm_gem_object_release(obj);
>> +kfree(exynos_gem_obj);
>> +return ERR_PTR(ret);
>> +}
>> +
>> DRM_DEBUG_KMS("created file object = 0x%x\n", (unsigned int)obj->filp);
>>
>> return exynos_gem_obj;
>> @@ -521,14 +528,9 @@ int exynos_drm_gem_dumb_map_offset(struct drm_file
>> *file_priv,
>> goto unlock;
>> }
>>
>> -ret = drm_gem_create_mmap_offset(obj);
>> -if (ret)
>> -goto out;
>> -
>> *offset = drm_vma_node_offset_addr(>vma_node);
>> DRM_DEBUG_KMS("offset = 0x%lx\n", (unsigned long)*offset);
>>
>> -out:
>> drm_gem_object_unreference(obj);
>> unlock:
>> mutex_unlock(>struct_mutex);
>>
>
>
[PATCH 08/14] drm/exynos: create a fake mmap offset with gem creation
On 2015ë
07ì 28ì¼ 17:53, Joonyoung Shim wrote:
> Don't create a fake mmap offset in exynos_drm_gem_dumb_map_offset. If
> not, it will call drm_gem_create_mmap_offset whenever user requests
> DRM_IOCTL_MODE_MAP_DUMB ioctl.
This patch makes drm_gem_create_mmap_offset to be called even in case of
not using dumb* interfaces. I.e.,
exynos_drm_gem_create_ioctl -> exynos_drm_gem_mmap
And drm_gem_create_mmap_offset checks if vma_node was already allocated
or not so this patch doesn't make sense.
Thanks,
Inki Dae
>
> Signed-off-by: Joonyoung Shim
> ---
> drivers/gpu/drm/exynos/exynos_drm_gem.c | 12 +++-
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> index 550d267..c76aa8a 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> @@ -151,6 +151,13 @@ struct exynos_drm_gem_obj *exynos_drm_gem_init(struct
> drm_device *dev,
> return ERR_PTR(ret);
> }
>
> + ret = drm_gem_create_mmap_offset(obj);
> + if (ret < 0) {
> + drm_gem_object_release(obj);
> + kfree(exynos_gem_obj);
> + return ERR_PTR(ret);
> + }
> +
> DRM_DEBUG_KMS("created file object = 0x%x\n", (unsigned int)obj->filp);
>
> return exynos_gem_obj;
> @@ -521,14 +528,9 @@ int exynos_drm_gem_dumb_map_offset(struct drm_file
> *file_priv,
> goto unlock;
> }
>
> - ret = drm_gem_create_mmap_offset(obj);
> - if (ret)
> - goto out;
> -
> *offset = drm_vma_node_offset_addr(>vma_node);
> DRM_DEBUG_KMS("offset = 0x%lx\n", (unsigned long)*offset);
>
> -out:
> drm_gem_object_unreference(obj);
> unlock:
> mutex_unlock(>struct_mutex);
>
[PATCH 08/14] drm/exynos: create a fake mmap offset with gem creation
Don't create a fake mmap offset in exynos_drm_gem_dumb_map_offset. If
not, it will call drm_gem_create_mmap_offset whenever user requests
DRM_IOCTL_MODE_MAP_DUMB ioctl.
Signed-off-by: Joonyoung Shim
---
drivers/gpu/drm/exynos/exynos_drm_gem.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c
b/drivers/gpu/drm/exynos/exynos_drm_gem.c
index 550d267..c76aa8a 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
@@ -151,6 +151,13 @@ struct exynos_drm_gem_obj *exynos_drm_gem_init(struct
drm_device *dev,
return ERR_PTR(ret);
}
+ ret = drm_gem_create_mmap_offset(obj);
+ if (ret < 0) {
+ drm_gem_object_release(obj);
+ kfree(exynos_gem_obj);
+ return ERR_PTR(ret);
+ }
+
DRM_DEBUG_KMS("created file object = 0x%x\n", (unsigned int)obj->filp);
return exynos_gem_obj;
@@ -521,14 +528,9 @@ int exynos_drm_gem_dumb_map_offset(struct drm_file
*file_priv,
goto unlock;
}
- ret = drm_gem_create_mmap_offset(obj);
- if (ret)
- goto out;
-
*offset = drm_vma_node_offset_addr(>vma_node);
DRM_DEBUG_KMS("offset = 0x%lx\n", (unsigned long)*offset);
-out:
drm_gem_object_unreference(obj);
unlock:
mutex_unlock(>struct_mutex);
--
1.9.1
