| Message ID | 1421449054-2808-2-git-send-email-tjakobi@math.uni-bielefeld.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Dear Tobias, Thanks for fixing it. Signed-off-by: Hyungwon Hwang <human.hwang@samsung.com> On Fri, 16 Jan 2015 23:57:33 +0100 Tobias Jakobi <tjakobi@math.uni-bielefeld.de> wrote: > From: Hyungwon Hwang <human.hwang@samsung.com> > > 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 <human.hwang@samsung.com> > Signed-off-by: Inki Dae <inki.dae@samsung.com> > Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de> > --- > 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, > &req); > + memset(&arg, 0, sizeof(arg)); > + arg.handle = bo->handle; > + > + ret = drmIoctl(dev->fd, DRM_IOCTL_MODE_MAP_DUMB, > &arg); 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 <sys/ioctl.h> > #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(&arg, 0, sizeof(arg)); > arg.handle = bo->base.handle; > > - ret = drmCommandWriteRead(bo->base.kms->fd, > DRM_EXYNOS_GEM_MAP_OFFSET, &arg, sizeof(arg)); > + ret = drmIoctl(bo->base.kms->fd, DRM_IOCTL_MODE_MAP_DUMB, > &arg); 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; > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 26 January 2015 at 01:42, Hyungwon Hwang <human.hwang@samsung.com> wrote: > Dear Tobias, > > Thanks for fixing it. > > Signed-off-by: Hyungwon Hwang <human.hwang@samsung.com> > 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? -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dear Emil, On Mon, 26 Jan 2015 19:05:51 +0000 Emil Velikov <emil.l.velikov@gmail.com> wrote: > On 26 January 2015 at 01:42, Hyungwon Hwang <human.hwang@samsung.com> > wrote: > > Dear Tobias, > > > > Thanks for fixing it. > > > > Signed-off-by: Hyungwon Hwang <human.hwang@samsung.com> > > > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 27 January 2015 at 01:05, Hyungwon Hwang <human.hwang@samsung.com> wrote: > Dear Emil, > > On Mon, 26 Jan 2015 19:05:51 +0000 > Emil Velikov <emil.l.velikov@gmail.com> wrote: > >> On 26 January 2015 at 01:42, Hyungwon Hwang <human.hwang@samsung.com> >> wrote: >> > Dear Tobias, >> > >> > Thanks for fixing it. >> > >> > Signed-off-by: Hyungwon Hwang <human.hwang@samsung.com> >> > >> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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, &req); + memset(&arg, 0, sizeof(arg)); + arg.handle = bo->handle; + + ret = drmIoctl(dev->fd, DRM_IOCTL_MODE_MAP_DUMB, &arg); 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 <sys/ioctl.h> #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(&arg, 0, sizeof(arg)); arg.handle = bo->base.handle; - ret = drmCommandWriteRead(bo->base.kms->fd, DRM_EXYNOS_GEM_MAP_OFFSET, &arg, sizeof(arg)); + ret = drmIoctl(bo->base.kms->fd, DRM_IOCTL_MODE_MAP_DUMB, &arg); 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;