[PATCH 1/2] exynos: Don't use DRM_EXYNOS_GEM_{MAP_OFFSET/MMAP} ioctls

[Hyungwon Hwang]

Dear Emil,

On Mon, 26 Jan 2015 19:05:51 +
Emil Velikov  wrote:

> On 26 January 2015 at 01:42, Hyungwon Hwang 
> wrote:
> > Dear Tobias,
> >
> > Thanks for fixing it.
> >
> > Signed-off-by: Hyungwon Hwang 
> >
> Hi Hyungwon Hwang
> 
> I'm a bit confused about the use of s-o-b here. I've assumed that
> libdrm follows the kernel style [1] for those type of things ?
> Would you have anything handy which I can read on the topic ?

I also think that DRI-devel mailing list follow the kernel rule. I used
s-o-b here by the meaning of (a) below (this text is excerpted from the
link you sent)

12) Sign your work

To improve tracking of who did what, especially with patches that can
percolate to their final resting place in the kernel through several
layers of maintainers, we've introduced a "sign-off" procedure on
patches that are being emailed around.

The sign-off is a simple line at the end of the explanation for the
patch, which certifies that you wrote it or otherwise have the right to
pass it on as an open-source patch.  The rules are pretty simple: if you
can certify the below:

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or

Was it wrong to add s-o-b here? Should I have used "Reviewed-by" here?
I am sorry, if I broke the rule and confused you.

> 
> Cheers,
> Emil
> 
> [1]
> https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/tree/Documentation/SubmittingPatches#n358
> 
> A: Because it messes up the order in which people normally read text.
> Q: Why is top-posting such a bad thing?


Best regards,
Hyungwon Hwang

[PATCH 1/2] exynos: Don't use DRM_EXYNOS_GEM_{MAP_OFFSET/MMAP} ioctls

[Emil Velikov]

On 27 January 2015 at 01:05, Hyungwon Hwang  wrote:
> Dear Emil,
>
> On Mon, 26 Jan 2015 19:05:51 +
> Emil Velikov  wrote:
>
>> On 26 January 2015 at 01:42, Hyungwon Hwang 
>> wrote:
>> > Dear Tobias,
>> >
>> > Thanks for fixing it.
>> >
>> > Signed-off-by: Hyungwon Hwang 
>> >
>> Hi Hyungwon Hwang
>>
>> I'm a bit confused about the use of s-o-b here. I've assumed that
>> libdrm follows the kernel style [1] for those type of things ?
>> Would you have anything handy which I can read on the topic ?
>
> I also think that DRI-devel mailing list follow the kernel rule.
Sweet I'm not loosing my mind (much).

[...]
> Was it wrong to add s-o-b here? Should I have used "Reviewed-by" here?
> I am sorry, if I broke the rule and confused you.
>
Don't think any apology is due, I was just hinting that the patch has
the s-o-b already :P
Where as "what I should have used" that depends on your intent and/or
the action undertaken.

Thanks
Emil

[PATCH 1/2] exynos: Don't use DRM_EXYNOS_GEM_{MAP_OFFSET/MMAP} ioctls

[Emil Velikov]

On 26 January 2015 at 01:42, Hyungwon Hwang  wrote:
> Dear Tobias,
>
> Thanks for fixing it.
>
> Signed-off-by: Hyungwon Hwang 
>
Hi Hyungwon Hwang

I'm a bit confused about the use of s-o-b here. I've assumed that
libdrm follows the kernel style [1] for those type of things ?
Would you have anything handy which I can read on the topic ?

Cheers,
Emil

[1] 
https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/tree/Documentation/SubmittingPatches#n358

A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[PATCH 1/2] exynos: Don't use DRM_EXYNOS_GEM_{MAP_OFFSET/MMAP} ioctls

[Hyungwon Hwang]

Dear Tobias,

Thanks for fixing it.

Signed-off-by: Hyungwon Hwang 

On Fri, 16 Jan 2015 23:57:33 +0100
Tobias Jakobi  wrote:

> From: Hyungwon Hwang 
> 
> The ioctl DRM_EXYNOS_GEM_MAP_OFFSET and DRM_EXYNOS_GEM_MMAP are
> removed from the linux kernel. This patch modifies libdrm and libkms
> to use drm generic ioctls instead of the removed ioctls.
> 
> v2: The original patch was erroneous. In case the MODE_MAP_DUMB ioctl
> failed it would return the retvalue as a void-pointer. Users of
> libdrm would then happily use that ptr, eventually leading to a
> segfault. Change this to return NULL in that case and also restore
> the previous behaviour of logging to stderr.
> The other error was that 'bo->vaddr' was never filled with the
> mapped buffer address. Hence exynos_bo_map still returned NULL even
> if the buffer mapping succeeded.
> 
> Signed-off-by: Hyungwon Hwang 
> Signed-off-by: Inki Dae 
> Signed-off-by: Tobias Jakobi 
> ---
>  exynos/exynos_drm.c | 19 ---
>  libkms/exynos.c |  7 ---
>  2 files changed, 16 insertions(+), 10 deletions(-)
> 
> diff --git a/exynos/exynos_drm.c b/exynos/exynos_drm.c
> index 4c7dd13..c5dd948 100644
> --- a/exynos/exynos_drm.c
> +++ b/exynos/exynos_drm.c
> @@ -283,20 +283,25 @@ drm_public void *exynos_bo_map(struct exynos_bo
> *bo) {
>   if (!bo->vaddr) {
>   struct exynos_device *dev = bo->dev;
> - struct drm_exynos_gem_mmap req = {
> - .handle = bo->handle,
> - .size   = bo->size,
> - };
> + struct drm_mode_map_dumb arg;
> + void *map = NULL;
>   int ret;
>  
> - ret = drmIoctl(dev->fd, DRM_IOCTL_EXYNOS_GEM_MMAP,
> );
> + memset(, 0, sizeof(arg));
> + arg.handle = bo->handle;
> +
> + ret = drmIoctl(dev->fd, DRM_IOCTL_MODE_MAP_DUMB,
> ); if (ret) {
> - fprintf(stderr, "failed to mmap[%s].\n",
> + fprintf(stderr, "failed to map dumb
> buffer[%s].\n", strerror(errno));
>   return NULL;
>   }
>  
> - bo->vaddr = (void *)(uintptr_t)req.mapped;
> + map = drm_mmap(0, bo->size, PROT_READ | PROT_WRITE,
> MAP_SHARED,
> + dev->fd, arg.offset);
> +
> + if (map != MAP_FAILED)
> + bo->vaddr = map;
>   }
>  
>   return bo->vaddr;
> diff --git a/libkms/exynos.c b/libkms/exynos.c
> index 92e329c..1123482 100644
> --- a/libkms/exynos.c
> +++ b/libkms/exynos.c
> @@ -25,6 +25,7 @@
>  #include 
>  #include "xf86drm.h"
>  
> +#include "libdrm.h"
>  #include "exynos_drm.h"
>  
>  struct exynos_bo
> @@ -124,7 +125,7 @@ static int
>  exynos_bo_map(struct kms_bo *_bo, void **out)
>  {
>   struct exynos_bo *bo = (struct exynos_bo *)_bo;
> - struct drm_exynos_gem_map_off arg;
> + struct drm_mode_map_dumb arg;
>   void *map = NULL;
>   int ret;
>  
> @@ -137,11 +138,11 @@ exynos_bo_map(struct kms_bo *_bo, void **out)
>   memset(, 0, sizeof(arg));
>   arg.handle = bo->base.handle;
>  
> - ret = drmCommandWriteRead(bo->base.kms->fd,
> DRM_EXYNOS_GEM_MAP_OFFSET, , sizeof(arg));
> + ret = drmIoctl(bo->base.kms->fd, DRM_IOCTL_MODE_MAP_DUMB,
> ); if (ret)
>   return ret;
>  
> - map = mmap(0, bo->base.size, PROT_READ | PROT_WRITE,
> MAP_SHARED, bo->base.kms->fd, arg.offset);
> + map = drm_mmap(0, bo->base.size, PROT_READ | PROT_WRITE,
> MAP_SHARED, bo->base.kms->fd, arg.offset); if (map == MAP_FAILED)
>   return -errno;
>  

[PATCH 1/2] exynos: Don't use DRM_EXYNOS_GEM_{MAP_OFFSET/MMAP} ioctls

[Tobias Jakobi]

From: Hyungwon Hwang 

The ioctl DRM_EXYNOS_GEM_MAP_OFFSET and DRM_EXYNOS_GEM_MMAP are removed from
the linux kernel. This patch modifies libdrm and libkms to use drm generic
ioctls instead of the removed ioctls.

v2: The original patch was erroneous. In case the MODE_MAP_DUMB ioctl failed
it would return the retvalue as a void-pointer. Users of libdrm would then
happily use that ptr, eventually leading to a segfault. Change this to
return NULL in that case and also restore the previous behaviour of logging
to stderr.
The other error was that 'bo->vaddr' was never filled with the mapped
buffer address. Hence exynos_bo_map still returned NULL even if the
buffer mapping succeeded.

Signed-off-by: Hyungwon Hwang 
Signed-off-by: Inki Dae 
Signed-off-by: Tobias Jakobi 
---
 exynos/exynos_drm.c | 19 ---
 libkms/exynos.c |  7 ---
 2 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/exynos/exynos_drm.c b/exynos/exynos_drm.c
index 4c7dd13..c5dd948 100644
--- a/exynos/exynos_drm.c
+++ b/exynos/exynos_drm.c
@@ -283,20 +283,25 @@ drm_public void *exynos_bo_map(struct exynos_bo *bo)
 {
if (!bo->vaddr) {
struct exynos_device *dev = bo->dev;
-   struct drm_exynos_gem_mmap req = {
-   .handle = bo->handle,
-   .size   = bo->size,
-   };
+   struct drm_mode_map_dumb arg;
+   void *map = NULL;
int ret;

-   ret = drmIoctl(dev->fd, DRM_IOCTL_EXYNOS_GEM_MMAP, );
+   memset(, 0, sizeof(arg));
+   arg.handle = bo->handle;
+
+   ret = drmIoctl(dev->fd, DRM_IOCTL_MODE_MAP_DUMB, );
if (ret) {
-   fprintf(stderr, "failed to mmap[%s].\n",
+   fprintf(stderr, "failed to map dumb buffer[%s].\n",
strerror(errno));
return NULL;
}

-   bo->vaddr = (void *)(uintptr_t)req.mapped;
+   map = drm_mmap(0, bo->size, PROT_READ | PROT_WRITE, MAP_SHARED,
+   dev->fd, arg.offset);
+
+   if (map != MAP_FAILED)
+   bo->vaddr = map;
}

return bo->vaddr;
diff --git a/libkms/exynos.c b/libkms/exynos.c
index 92e329c..1123482 100644
--- a/libkms/exynos.c
+++ b/libkms/exynos.c
@@ -25,6 +25,7 @@
 #include 
 #include "xf86drm.h"

+#include "libdrm.h"
 #include "exynos_drm.h"

 struct exynos_bo
@@ -124,7 +125,7 @@ static int
 exynos_bo_map(struct kms_bo *_bo, void **out)
 {
struct exynos_bo *bo = (struct exynos_bo *)_bo;
-   struct drm_exynos_gem_map_off arg;
+   struct drm_mode_map_dumb arg;
void *map = NULL;
int ret;

@@ -137,11 +138,11 @@ exynos_bo_map(struct kms_bo *_bo, void **out)
memset(, 0, sizeof(arg));
arg.handle = bo->base.handle;

-   ret = drmCommandWriteRead(bo->base.kms->fd, DRM_EXYNOS_GEM_MAP_OFFSET, 
, sizeof(arg));
+   ret = drmIoctl(bo->base.kms->fd, DRM_IOCTL_MODE_MAP_DUMB, );
if (ret)
return ret;

-   map = mmap(0, bo->base.size, PROT_READ | PROT_WRITE, MAP_SHARED, 
bo->base.kms->fd, arg.offset);
+   map = drm_mmap(0, bo->base.size, PROT_READ | PROT_WRITE, MAP_SHARED, 
bo->base.kms->fd, arg.offset);
if (map == MAP_FAILED)
return -errno;

-- 
2.0.5

[PATCH 1/2] exynos: Don't use DRM_EXYNOS_GEM_{MAP_OFFSET/MMAP} ioctls

[Inki Dae]

On 2014년 10월 31일 10:28, Hyungwon Hwang wrote:
> The ioctl DRM_EXYNOS_GEM_MAP_OFFSET and DRM_EXYNOS_GEM_MMAP are removed from
> the linux kernel. This patch modifies libdrm and libkms to use drm generic
> ioctls instead of the removed ioctls.


Signed-off-by: Inki Dae 

> 
> Signed-off-by: Hyungwon Hwang 
> ---
>  exynos/exynos_drm.c | 24 +---
>  libkms/exynos.c |  7 ---
>  2 files changed, 17 insertions(+), 14 deletions(-)
> 
> diff --git a/exynos/exynos_drm.c b/exynos/exynos_drm.c
> index 4c7dd13..4cb6a6d 100644
> --- a/exynos/exynos_drm.c
> +++ b/exynos/exynos_drm.c
> @@ -283,20 +283,22 @@ drm_public void *exynos_bo_map(struct exynos_bo *bo)
>  {
>   if (!bo->vaddr) {
>   struct exynos_device *dev = bo->dev;
> - struct drm_exynos_gem_mmap req = {
> - .handle = bo->handle,
> - .size   = bo->size,
> - };
> + struct drm_mode_map_dumb arg;
> + void *map = NULL;
>   int ret;
>  
> - ret = drmIoctl(dev->fd, DRM_IOCTL_EXYNOS_GEM_MMAP, );
> - if (ret) {
> - fprintf(stderr, "failed to mmap[%s].\n",
> - strerror(errno));
> - return NULL;
> - }
> + memset(, 0, sizeof(arg));
> + arg.handle = bo->handle;
> +
> + ret = drmIoctl(dev->fd, DRM_IOCTL_MODE_MAP_DUMB, );
> + if (ret)
> + return ret;
>  
> - bo->vaddr = (void *)(uintptr_t)req.mapped;
> + map = drm_mmap(0, bo->size, PROT_READ | PROT_WRITE, MAP_SHARED,
> + dev->fd, arg.offset);
> +
> + if (map == MAP_FAILED)
> + return NULL;
>   }
>  
>   return bo->vaddr;
> diff --git a/libkms/exynos.c b/libkms/exynos.c
> index 92e329c..1123482 100644
> --- a/libkms/exynos.c
> +++ b/libkms/exynos.c
> @@ -25,6 +25,7 @@
>  #include 
>  #include "xf86drm.h"
>  
> +#include "libdrm.h"
>  #include "exynos_drm.h"
>  
>  struct exynos_bo
> @@ -124,7 +125,7 @@ static int
>  exynos_bo_map(struct kms_bo *_bo, void **out)
>  {
>   struct exynos_bo *bo = (struct exynos_bo *)_bo;
> - struct drm_exynos_gem_map_off arg;
> + struct drm_mode_map_dumb arg;
>   void *map = NULL;
>   int ret;
>  
> @@ -137,11 +138,11 @@ exynos_bo_map(struct kms_bo *_bo, void **out)
>   memset(, 0, sizeof(arg));
>   arg.handle = bo->base.handle;
>  
> - ret = drmCommandWriteRead(bo->base.kms->fd, DRM_EXYNOS_GEM_MAP_OFFSET, 
> , sizeof(arg));
> + ret = drmIoctl(bo->base.kms->fd, DRM_IOCTL_MODE_MAP_DUMB, );
>   if (ret)
>   return ret;
>  
> - map = mmap(0, bo->base.size, PROT_READ | PROT_WRITE, MAP_SHARED, 
> bo->base.kms->fd, arg.offset);
> + map = drm_mmap(0, bo->base.size, PROT_READ | PROT_WRITE, MAP_SHARED, 
> bo->base.kms->fd, arg.offset);
>   if (map == MAP_FAILED)
>   return -errno;
>  
> 

[PATCH 1/2] exynos: Don't use DRM_EXYNOS_GEM_{MAP_OFFSET/MMAP} ioctls

[Hyungwon Hwang]

The ioctl DRM_EXYNOS_GEM_MAP_OFFSET and DRM_EXYNOS_GEM_MMAP are removed from
the linux kernel. This patch modifies libdrm and libkms to use drm generic
ioctls instead of the removed ioctls.

Signed-off-by: Hyungwon Hwang 
---
 exynos/exynos_drm.c | 24 +---
 libkms/exynos.c |  7 ---
 2 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/exynos/exynos_drm.c b/exynos/exynos_drm.c
index 4c7dd13..4cb6a6d 100644
--- a/exynos/exynos_drm.c
+++ b/exynos/exynos_drm.c
@@ -283,20 +283,22 @@ drm_public void *exynos_bo_map(struct exynos_bo *bo)
 {
if (!bo->vaddr) {
struct exynos_device *dev = bo->dev;
-   struct drm_exynos_gem_mmap req = {
-   .handle = bo->handle,
-   .size   = bo->size,
-   };
+   struct drm_mode_map_dumb arg;
+   void *map = NULL;
int ret;

-   ret = drmIoctl(dev->fd, DRM_IOCTL_EXYNOS_GEM_MMAP, );
-   if (ret) {
-   fprintf(stderr, "failed to mmap[%s].\n",
-   strerror(errno));
-   return NULL;
-   }
+   memset(, 0, sizeof(arg));
+   arg.handle = bo->handle;
+
+   ret = drmIoctl(dev->fd, DRM_IOCTL_MODE_MAP_DUMB, );
+   if (ret)
+   return ret;

-   bo->vaddr = (void *)(uintptr_t)req.mapped;
+   map = drm_mmap(0, bo->size, PROT_READ | PROT_WRITE, MAP_SHARED,
+   dev->fd, arg.offset);
+
+   if (map == MAP_FAILED)
+   return NULL;
}

return bo->vaddr;
diff --git a/libkms/exynos.c b/libkms/exynos.c
index 92e329c..1123482 100644
--- a/libkms/exynos.c
+++ b/libkms/exynos.c
@@ -25,6 +25,7 @@
 #include 
 #include "xf86drm.h"

+#include "libdrm.h"
 #include "exynos_drm.h"

 struct exynos_bo
@@ -124,7 +125,7 @@ static int
 exynos_bo_map(struct kms_bo *_bo, void **out)
 {
struct exynos_bo *bo = (struct exynos_bo *)_bo;
-   struct drm_exynos_gem_map_off arg;
+   struct drm_mode_map_dumb arg;
void *map = NULL;
int ret;

@@ -137,11 +138,11 @@ exynos_bo_map(struct kms_bo *_bo, void **out)
memset(, 0, sizeof(arg));
arg.handle = bo->base.handle;

-   ret = drmCommandWriteRead(bo->base.kms->fd, DRM_EXYNOS_GEM_MAP_OFFSET, 
, sizeof(arg));
+   ret = drmIoctl(bo->base.kms->fd, DRM_IOCTL_MODE_MAP_DUMB, );
if (ret)
return ret;

-   map = mmap(0, bo->base.size, PROT_READ | PROT_WRITE, MAP_SHARED, 
bo->base.kms->fd, arg.offset);
+   map = drm_mmap(0, bo->base.size, PROT_READ | PROT_WRITE, MAP_SHARED, 
bo->base.kms->fd, arg.offset);
if (map == MAP_FAILED)
return -errno;

-- 
1.8.3.2