Skip to contentSkip to conversation

drm/msm: Implement msm drm fb_mmap callback function

PATCH In review Hai Li 3 participants 6 messages Jun 18, 2014 – Jun 20, 2014

Subsystems (3)
lore
Share & tools
lore

Hai Li

author
#0 lore raw

PATCH 1/1 drm/msm: Implement msm drm fb_mmap callback function

This change implements msm drm specific fb_mmap function for fb device to properly map the fb address to userspace.

---
 drivers/gpu/drm/msm/msm_fbdev.c | 37 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)
drivers/gpu/drm/msm/msm_fbdev.c +36 -1
index 4f4e7b4..2522f51 100644
@@ -19,6 +19,11 @@
 
 #include "drm_crtc.h"
 #include "drm_fb_helper.h"
+#include "msm_gem.h"
+
+extern int msm_gem_mmap_obj(struct drm_gem_object *obj,
+					struct vm_area_struct *vma);
+static int msm_fbdev_mmap(struct fb_info *info, struct vm_area_struct *vma);
 
 /*
  * fbdev funcs, to implement legacy fbdev interface on top of drm driver
@@ -43,6 +48,7 @@ static struct fb_ops msm_fb_ops = {
 	.fb_fillrect = sys_fillrect,
 	.fb_copyarea = sys_copyarea,
 	.fb_imageblit = sys_imageblit,
+	.fb_mmap = msm_fbdev_mmap,
 
 	.fb_check_var = drm_fb_helper_check_var,
 	.fb_set_par = drm_fb_helper_set_par,
@@ -51,6 +57,31 @@ static struct fb_ops msm_fb_ops = {
 	.fb_setcmap = drm_fb_helper_setcmap,
 };
 
+static int msm_fbdev_mmap(struct fb_info *info, struct vm_area_struct *vma)
+{
+	struct drm_fb_helper *helper = (struct drm_fb_helper *)info->par;
+	struct msm_fbdev *fbdev = to_msm_fbdev(helper);
+	struct drm_gem_object *drm_obj = fbdev->bo;
+	struct drm_device *dev = helper->dev;
+	int ret = 0;
+
+	if (drm_device_is_unplugged(dev))
+		return -ENODEV;
+
+	mutex_lock(&dev->struct_mutex);
+
+	ret = drm_gem_mmap_obj(drm_obj, drm_obj->size, vma);
+
+	mutex_unlock(&dev->struct_mutex);
+
+	if (ret) {
+		pr_err("%s:drm_gem_mmap_obj fail\n", __func__);
+		return ret;
+	}
+
+	return msm_gem_mmap_obj(drm_obj, vma);
+}
+
 static int msm_fbdev_create(struct drm_fb_helper *helper,
 		struct drm_fb_helper_surface_size *sizes)
 {
@@ -108,7 +139,11 @@ static int msm_fbdev_create(struct drm_fb_helper *helper,
 	mutex_lock(&dev->struct_mutex);
 
 	/* TODO implement our own fb_mmap so we don't need this: */
-	msm_gem_get_iova_locked(fbdev->bo, 0, &paddr);
+	ret = msm_gem_get_iova_locked(fbdev->bo, 0, &paddr);
+	if (ret) {
+		dev_err(dev->dev, "failed to get buffer obj iova: %d\n", ret);
+		goto fail;
+	}
 
 	fbi = framebuffer_alloc(0, dev->dev);
 	if (!fbi) {
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
#1 lore raw
Quoted 3 lines · Hai Li

This change implements msm drm specific fb_mmap function for fb device to properly map the fb address to userspace.

thanks, queued up on msm-next

BR,
-R

Quoted 79 lines
---
 drivers/gpu/drm/msm/msm_fbdev.c | 37 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)
drivers/gpu/drm/msm/msm_fbdev.c +36 -2
index 4f4e7b4..2522f51 100644
@@ -19,6 +19,11 @@
 
 #include "drm_crtc.h"
 #include "drm_fb_helper.h"
+#include "msm_gem.h"
+
+extern int msm_gem_mmap_obj(struct drm_gem_object *obj,
+                                       struct vm_area_struct *vma);
+static int msm_fbdev_mmap(struct fb_info *info, struct vm_area_struct *vma);
 
 /*
  * fbdev funcs, to implement legacy fbdev interface on top of drm driver
@@ -43,6 +48,7 @@ static struct fb_ops msm_fb_ops = {
        .fb_fillrect = sys_fillrect,
        .fb_copyarea = sys_copyarea,
        .fb_imageblit = sys_imageblit,
+       .fb_mmap = msm_fbdev_mmap,
 
        .fb_check_var = drm_fb_helper_check_var,
        .fb_set_par = drm_fb_helper_set_par,
@@ -51,6 +57,31 @@ static struct fb_ops msm_fb_ops = {
        .fb_setcmap = drm_fb_helper_setcmap,
 };
 
+static int msm_fbdev_mmap(struct fb_info *info, struct vm_area_struct *vma)
+{
+       struct drm_fb_helper *helper = (struct drm_fb_helper *)info->par;
+       struct msm_fbdev *fbdev = to_msm_fbdev(helper);
+       struct drm_gem_object *drm_obj = fbdev->bo;
+       struct drm_device *dev = helper->dev;
+       int ret = 0;
+
+       if (drm_device_is_unplugged(dev))
+               return -ENODEV;
+
+       mutex_lock(&dev->struct_mutex);
+
+       ret = drm_gem_mmap_obj(drm_obj, drm_obj->size, vma);
+
+       mutex_unlock(&dev->struct_mutex);
+
+       if (ret) {
+               pr_err("%s:drm_gem_mmap_obj fail\n", __func__);
+               return ret;
+       }
+
+       return msm_gem_mmap_obj(drm_obj, vma);
+}
+
 static int msm_fbdev_create(struct drm_fb_helper *helper,
                struct drm_fb_helper_surface_size *sizes)
 {
@@ -108,7 +139,11 @@ static int msm_fbdev_create(struct drm_fb_helper *helper,
        mutex_lock(&dev->struct_mutex);
 
        /* TODO implement our own fb_mmap so we don't need this: */
-       msm_gem_get_iova_locked(fbdev->bo, 0, &paddr);
+       ret = msm_gem_get_iova_locked(fbdev->bo, 0, &paddr);
+       if (ret) {
+               dev_err(dev->dev, "failed to get buffer obj iova: %d\n", ret);
+               goto fail;
+       }
 
        fbi = framebuffer_alloc(0, dev->dev);
        if (!fbi) {
--

The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation

#2 lore raw
In reply to Hai Li

drivers/gpu/drm/msm/msm_fbdev.c line 25

Quoted 12 lines · Hai Li
drivers/gpu/drm/msm/msm_fbdev.c +4 -0
index 4f4e7b4..2522f51 100644
@@ -19,6 +19,11 @@
 
 #include "drm_crtc.h"
 #include "drm_fb_helper.h"
+#include "msm_gem.h"
+
+extern int msm_gem_mmap_obj(struct drm_gem_object *obj,
+					struct vm_area_struct *vma);

This can't go into some header file?

Quoted 13 lines
drivers/gpu/drm/msm/msm_fbdev.c +2 -0
+static int msm_fbdev_mmap(struct fb_info *info, struct vm_area_struct *vma);
 
 /*
  * fbdev funcs, to implement legacy fbdev interface on top of drm driver
@@ -43,6 +48,7 @@ static struct fb_ops msm_fb_ops = {
 	.fb_fillrect = sys_fillrect,
 	.fb_copyarea = sys_copyarea,
 	.fb_imageblit = sys_imageblit,
+	.fb_mmap = msm_fbdev_mmap,
 
 	.fb_check_var = drm_fb_helper_check_var,
 	.fb_set_par = drm_fb_helper_set_par,

drivers/gpu/drm/msm/msm_fbdev.c static struct fb_ops msm_fb_ops = { · line 66

Quoted 10 lines
drivers/gpu/drm/msm/msm_fbdev.c +7 -0
@@ -51,6 +57,31 @@ static struct fb_ops msm_fb_ops = {
 	.fb_setcmap = drm_fb_helper_setcmap,
 };
 
+static int msm_fbdev_mmap(struct fb_info *info, struct vm_area_struct *vma)
+{
+	struct drm_fb_helper *helper = (struct drm_fb_helper *)info->par;
+	struct msm_fbdev *fbdev = to_msm_fbdev(helper);
+	struct drm_gem_object *drm_obj = fbdev->bo;
+	struct drm_device *dev = helper->dev;
+	int ret = 0;

unnecessary initialization to 0.

Quoted 22 lines
drivers/gpu/drm/msm/msm_fbdev.c +18 -0
+
+	if (drm_device_is_unplugged(dev))
+		return -ENODEV;
+
+	mutex_lock(&dev->struct_mutex);
+
+	ret = drm_gem_mmap_obj(drm_obj, drm_obj->size, vma);
+
+	mutex_unlock(&dev->struct_mutex);
+
+	if (ret) {
+		pr_err("%s:drm_gem_mmap_obj fail\n", __func__);
+		return ret;
+	}
+
+	return msm_gem_mmap_obj(drm_obj, vma);
+}
+
 static int msm_fbdev_create(struct drm_fb_helper *helper,
 		struct drm_fb_helper_surface_size *sizes)
 {

drivers/gpu/drm/msm/msm_fbdev.c static int msm_fbdev_create(struct drm_fb_helper *helper, · line 141

Quoted 3 lines
drivers/gpu/drm/msm/msm_fbdev.c +0 -0
@@ -108,7 +139,11 @@ static int msm_fbdev_create(struct drm_fb_helper *helper,
 	mutex_lock(&dev->struct_mutex);
 
 	/* TODO implement our own fb_mmap so we don't need this: */

Is this comment still relevant?

Quoted 9 lines
drivers/gpu/drm/msm/msm_fbdev.c +5 -1
-	msm_gem_get_iova_locked(fbdev->bo, 0, &paddr);
+	ret = msm_gem_get_iova_locked(fbdev->bo, 0, &paddr);
+	if (ret) {
+		dev_err(dev->dev, "failed to get buffer obj iova: %d\n", ret);
+		goto fail;
+	}
 
 	fbi = framebuffer_alloc(0, dev->dev);
 	if (!fbi) {
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
#3 lore raw
Quoted 7 lines · Stephen Boyd
Quoted 6 lines · Hai Li
drivers/gpu/drm/msm/msm_fbdev.c +0 -0
index 4f4e7b4..2522f51 100644
@@ -19,6 +19,11 @@
 
Quoted 8 lines

drivers/gpu/drm/msm/msm_fbdev.c line 25

Quoted 6 lines
drivers/gpu/drm/msm/msm_fbdev.c +4 -0
 #include "drm_crtc.h"
 #include "drm_fb_helper.h"
+#include "msm_gem.h"
+
+extern int msm_gem_mmap_obj(struct drm_gem_object *obj,
+                                     struct vm_area_struct *vma);

This can't go into some header file?

it probably should go in msm_gem.h.. although this would be the one special case where it is needed outside of msm_gem.c so I don't mind too much

Quoted 50 lines
Quoted 13 lines
drivers/gpu/drm/msm/msm_fbdev.c +2 -0
+static int msm_fbdev_mmap(struct fb_info *info, struct vm_area_struct *vma);
 
 /*
  * fbdev funcs, to implement legacy fbdev interface on top of drm driver
@@ -43,6 +48,7 @@ static struct fb_ops msm_fb_ops = {
      .fb_fillrect = sys_fillrect,
      .fb_copyarea = sys_copyarea,
      .fb_imageblit = sys_imageblit,
+     .fb_mmap = msm_fbdev_mmap,
 
      .fb_check_var = drm_fb_helper_check_var,
      .fb_set_par = drm_fb_helper_set_par,

drivers/gpu/drm/msm/msm_fbdev.c static struct fb_ops msm_fb_ops = { · line 66

Quoted 10 lines
drivers/gpu/drm/msm/msm_fbdev.c +7 -0
@@ -51,6 +57,31 @@ static struct fb_ops msm_fb_ops = {
      .fb_setcmap = drm_fb_helper_setcmap,
 };
 
+static int msm_fbdev_mmap(struct fb_info *info, struct vm_area_struct *vma)
+{
+     struct drm_fb_helper *helper = (struct drm_fb_helper *)info->par;
+     struct msm_fbdev *fbdev = to_msm_fbdev(helper);
+     struct drm_gem_object *drm_obj = fbdev->bo;
+     struct drm_device *dev = helper->dev;
+     int ret = 0;

unnecessary initialization to 0.

Quoted 24 lines
drivers/gpu/drm/msm/msm_fbdev.c +18 -0
+
+     if (drm_device_is_unplugged(dev))
+             return -ENODEV;
+
+     mutex_lock(&dev->struct_mutex);
+
+     ret = drm_gem_mmap_obj(drm_obj, drm_obj->size, vma);
+
+     mutex_unlock(&dev->struct_mutex);
+
+     if (ret) {
+             pr_err("%s:drm_gem_mmap_obj fail\n", __func__);
+             return ret;
+     }
+
+     return msm_gem_mmap_obj(drm_obj, vma);
+}
+
 static int msm_fbdev_create(struct drm_fb_helper *helper,
              struct drm_fb_helper_surface_size *sizes)
 {
@@ -108,7 +139,11 @@ static int msm_fbdev_create(struct drm_fb_helper *helper,
      mutex_lock(&dev->struct_mutex);
 
Quoted 3 lines

drivers/gpu/drm/msm/msm_fbdev.c static int msm_fbdev_create(struct drm_fb_helper *helper, · line 141

Quoted 1 line
drivers/gpu/drm/msm/msm_fbdev.c +0 -0
      /* TODO implement our own fb_mmap so we don't need this: */

Is this comment still relevant?

partially.. although it changes to the "can we make sure we can safely pin on panic when fbcon is restored"..

probably a low priority thing, because we are not really tight on device virtual space.

BR,
-R

Quoted 15 lines

drivers/gpu/drm/msm/msm_fbdev.c static int msm_fbdev_create(struct drm_fb_helper *helper, · line 149

Quoted 9 lines
drivers/gpu/drm/msm/msm_fbdev.c +5 -1
-     msm_gem_get_iova_locked(fbdev->bo, 0, &paddr);
+     ret = msm_gem_get_iova_locked(fbdev->bo, 0, &paddr);
+     if (ret) {
+             dev_err(dev->dev, "failed to get buffer obj iova: %d\n", ret);
+             goto fail;
+     }
 
      fbi = framebuffer_alloc(0, dev->dev);
      if (!fbi) {

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

author
#4 lore raw

PATCH 1/1 drm/msm: Implement msm drm fb_mmap callback function

In reply to Hai Li

This change implements msm drm specific fb_mmap function for fb device to properly map the fb address to userspace.

---
 drivers/gpu/drm/msm/msm_fbdev.c | 38 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 36 insertions(+), 2 deletions(-)
drivers/gpu/drm/msm/msm_fbdev.c +36 -2
index a752ab8..2694e90 100644
@@ -19,6 +19,11 @@
 
 #include "drm_crtc.h"
 #include "drm_fb_helper.h"
+#include "msm_gem.h"
+
+extern int msm_gem_mmap_obj(struct drm_gem_object *obj,
+					struct vm_area_struct *vma);
+static int msm_fbdev_mmap(struct fb_info *info, struct vm_area_struct *vma);
 
 /*
  * fbdev funcs, to implement legacy fbdev interface on top of drm driver
@@ -43,6 +48,7 @@ static struct fb_ops msm_fb_ops = {
 	.fb_fillrect = sys_fillrect,
 	.fb_copyarea = sys_copyarea,
 	.fb_imageblit = sys_imageblit,
+	.fb_mmap = msm_fbdev_mmap,
 
 	.fb_check_var = drm_fb_helper_check_var,
 	.fb_set_par = drm_fb_helper_set_par,
@@ -51,6 +57,31 @@ static struct fb_ops msm_fb_ops = {
 	.fb_setcmap = drm_fb_helper_setcmap,
 };
 
+static int msm_fbdev_mmap(struct fb_info *info, struct vm_area_struct *vma)
+{
+	struct drm_fb_helper *helper = (struct drm_fb_helper *)info->par;
+	struct msm_fbdev *fbdev = to_msm_fbdev(helper);
+	struct drm_gem_object *drm_obj = fbdev->bo;
+	struct drm_device *dev = helper->dev;
+	int ret;
+
+	if (drm_device_is_unplugged(dev))
+		return -ENODEV;
+
+	mutex_lock(&dev->struct_mutex);
+
+	ret = drm_gem_mmap_obj(drm_obj, drm_obj->size, vma);
+
+	mutex_unlock(&dev->struct_mutex);
+
+	if (ret) {
+		pr_err("%s:drm_gem_mmap_obj fail\n", __func__);
+		return ret;
+	}
+
+	return msm_gem_mmap_obj(drm_obj, vma);
+}
+
 static int msm_fbdev_create(struct drm_fb_helper *helper,
 		struct drm_fb_helper_surface_size *sizes)
 {
@@ -104,8 +135,11 @@ static int msm_fbdev_create(struct drm_fb_helper *helper,
 
 	mutex_lock(&dev->struct_mutex);
 
-	/* TODO implement our own fb_mmap so we don't need this: */
-	msm_gem_get_iova_locked(fbdev->bo, 0, &paddr);
+	ret = msm_gem_get_iova_locked(fbdev->bo, 0, &paddr);
+	if (ret) {
+		dev_err(dev->dev, "failed to get buffer obj iova: %d\n", ret);
+		goto fail;
+	}
 
 	fbi = framebuffer_alloc(0, dev->dev);
 	if (!fbi) {
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
#5 lore raw

Re-send the patch to remove the unnecessary initialization and the comment

Quoted 89 lines

This change implements msm drm specific fb_mmap function for fb device to properly map the fb address to userspace.

---
 drivers/gpu/drm/msm/msm_fbdev.c | 38
++++++++++++++++++++++++++++++++++++--
 1 file changed, 36 insertions(+), 2 deletions(-)
drivers/gpu/drm/msm/msm_fbdev.c +5 -0
index a752ab8..2694e90 100644
@@ -19,6 +19,11 @@
 
 #include "drm_crtc.h"
 #include "drm_fb_helper.h"
+#include "msm_gem.h"
+
+extern int msm_gem_mmap_obj(struct drm_gem_object *obj,
+					struct vm_area_struct *vma);
+static int msm_fbdev_mmap(struct fb_info *info, struct vm_area_struct
 *vma);
 
 /*

* fbdev funcs, to implement legacy fbdev interface on top of drm driver

drivers/gpu/drm/msm/msm_fbdev.c +26 -0
@@ -43,6 +48,7 @@ static struct fb_ops msm_fb_ops = {
 	.fb_fillrect = sys_fillrect,
 	.fb_copyarea = sys_copyarea,
 	.fb_imageblit = sys_imageblit,
+	.fb_mmap = msm_fbdev_mmap,
 
 	.fb_check_var = drm_fb_helper_check_var,
 	.fb_set_par = drm_fb_helper_set_par,
@@ -51,6 +57,31 @@ static struct fb_ops msm_fb_ops = {
 	.fb_setcmap = drm_fb_helper_setcmap,
 };
 
+static int msm_fbdev_mmap(struct fb_info *info, struct vm_area_struct
 *vma)
+{
+	struct drm_fb_helper *helper = (struct drm_fb_helper *)info->par;
+	struct msm_fbdev *fbdev = to_msm_fbdev(helper);
+	struct drm_gem_object *drm_obj = fbdev->bo;
+	struct drm_device *dev = helper->dev;
+	int ret;
+
+	if (drm_device_is_unplugged(dev))
+		return -ENODEV;
+
+	mutex_lock(&dev->struct_mutex);
+
+	ret = drm_gem_mmap_obj(drm_obj, drm_obj->size, vma);
+
+	mutex_unlock(&dev->struct_mutex);
+
+	if (ret) {
+		pr_err("%s:drm_gem_mmap_obj fail\n", __func__);
+		return ret;
+	}
+
+	return msm_gem_mmap_obj(drm_obj, vma);
+}
+
 static int msm_fbdev_create(struct drm_fb_helper *helper,
 		struct drm_fb_helper_surface_size *sizes)

{

drivers/gpu/drm/msm/msm_fbdev.c +5 -2
@@ -104,8 +135,11 @@ static int msm_fbdev_create(struct drm_fb_helper
 *helper,
 
 	mutex_lock(&dev->struct_mutex);
 
-	/* TODO implement our own fb_mmap so we don't need this: */
-	msm_gem_get_iova_locked(fbdev->bo, 0, &paddr);
+	ret = msm_gem_get_iova_locked(fbdev->bo, 0, &paddr);
+	if (ret) {
+		dev_err(dev->dev, "failed to get buffer obj iova: %d\n", ret);
+		goto fail;
+	}
 
 	fbi = framebuffer_alloc(0, dev->dev);

if (!fbi) {
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation