drm/rockchip: create the fake mmap offset during gem creation

PATCH In review Heiko Stuebner 3 participants 9 messages Nov 9, 2015 – Nov 16, 2015

Subsystems (4)

DRM drivers DRM misc Arm Rockchip DRM drivers for Rockchip

lore

Share & tools

lore

Heiko Stuebner

author maintainer

2015-11-09 19:50 UTC #0 lore raw

PATCH 1/2 drm/rockchip: create the fake mmap offset during gem creation

Similarly to the equivalent exynos-fix in commit 48cf53f4343a ("drm/exynos: create a fake mmap offset with gem creation") the rockchip drm driver also calls the drm_gem_create_mmap_offset fake-offset creation on every invocation of rockchip_gem_dumb_map_offset.

Move that to the creation of the gem to only create that offset once.

Signed-off-by Heiko Stuebner <[email protected]>

---
 drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

drivers/gpu/drm/rockchip/rockchip_drm_gem.c +6 -6

index 4ab2f20..e50d113 100644
@@ -126,8 +126,14 @@ struct rockchip_gem_object *
 	if (ret)
 		goto err_free_rk_obj;
 
+	ret = drm_gem_create_mmap_offset(obj);
+	if (ret)
+		goto err_free_buf;
+
 	return rk_obj;
 
+err_free_buf:
+	rockchip_gem_free_buf(rk_obj);
 err_free_rk_obj:
 	kfree(rk_obj);
 	return ERR_PTR(ret);

@@ -196,7 +202,6 @@ int rockchip_gem_dumb_map_offset(struct drm_file *file_priv,
 				 uint64_t *offset)
 {
 	struct drm_gem_object *obj;
-	int ret;
 
 	obj = drm_gem_object_lookup(dev, file_priv, handle);
 	if (!obj) {

@@ -204,14 +209,9 @@ int rockchip_gem_dumb_map_offset(struct drm_file *file_priv,
 		return -EINVAL;
 	}
 
-	ret = drm_gem_create_mmap_offset(obj);
-	if (ret)
-		goto out;
-
 	*offset = drm_vma_node_offset_addr(&obj->vma_node);
 	DRM_DEBUG_KMS("offset = 0x%llx\n", *offset);
 
-out:
 	drm_gem_object_unreference_unlocked(obj);
 
 	return 0;

Signature · 1 line

2.6.2

Heiko Stuebner

author maintainer

2015-11-09 19:52 UTC #1 lore raw

PATCH 2/2 drm/rockchip: unset pgoff when mmap'ing gems

Commit 371f0f085f629 ("ARM: 8426/1: dma-mapping: add missing range check
in dma_mmap()") introduced offset-checking for mappings, which collides
with the fake-offset the drm sets for gems.

Other drm-drivers set this offset to 0 before doing the mapping, so this looks like the correct way to go for rockchip as well.

Fixes 371f0f085f629 ("ARM: 8426/1: dma-mapping: add missing range check in dma_mmap()")

Signed-off-by Heiko Stuebner <[email protected]>

---
This patch is definitly necessary to keep the rockchip-drm working with kernel 4.4.

 drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 1 +
 1 file changed, 1 insertion(+)

drivers/gpu/drm/rockchip/rockchip_drm_gem.c +1 -0

index e50d113..e9bc188 100644
@@ -68,6 +68,7 @@ static int rockchip_drm_gem_object_mmap(struct drm_gem_object *obj,
 	 * VM_PFNMAP flag that was set by drm_gem_mmap_obj()/drm_gem_mmap().
 	 */
 	vma->vm_flags &= ~VM_PFNMAP;
+	vma->vm_pgoff = 0;
 
 	ret = dma_mmap_attrs(drm->dev, vma, rk_obj->kvaddr, rk_obj->dma_addr,
 			     obj->size, &rk_obj->dma_attrs);

Signature · 1 line

2.6.2

Mark yao

Acked-by on v1 2/2

2015-11-10 05:53 UTC #2 lore raw

Re: [PATCH 2/2] drm/rockchip: unset pgoff when mmap'ing gems

Quoted 21 lines · Heiko Stuebner

Commit 371f0f085f629 ("ARM: 8426/1: dma-mapping: add missing range check
in dma_mmap()") introduced offset-checking for mappings, which collides
with the fake-offset the drm sets for gems.
Other drm-drivers set this offset to 0 before doing the mapping, so this looks like the correct way to go for rockchip as well.
Fixes 371f0f085f629 ("ARM: 8426/1: dma-mapping: add missing range check in dma_mmap()")
Signed-off-by Heiko Stuebner <[email protected]>
---
This patch is definitly necessary to keep the rockchip-drm working with kernel 4.4.
  drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 1 +
  1 file changed, 1 insertion(+)
drivers/gpu/drm/rockchip/rockchip_drm_gem.c +0 -0
index e50d113..e9bc188 100644

drivers/gpu/drm/rockchip/rockchip_drm_gem.c static int rockchip_drm_gem_object_mmap(struct drm_gem_object *obj, · line 74

Quoted 7 lines

drivers/gpu/drm/rockchip/rockchip_drm_gem.c +1 -0

@@ -68,6 +68,7 @@ static int rockchip_drm_gem_object_mmap(struct drm_gem_object *obj,
  	 * VM_PFNMAP flag that was set by drm_gem_mmap_obj()/drm_gem_mmap().
  	 */
  	vma->vm_flags &= ~VM_PFNMAP;
+	vma->vm_pgoff = 0;
 
  	ret = dma_mmap_attrs(drm->dev, vma, rk_obj->kvaddr, rk_obj->dma_addr,
  			     obj->size, &rk_obj->dma_attrs);

Hi Heiko
Sorry for miss this patch, thanks for the fix.

Acked-by Mark Yao <[email protected]>

Signature · 1 line

?ark Yao

Mark yao

2015-11-10 06:17 UTC #3 lore raw

In reply to Heiko Stuebner

Hi Heiko
I don't think this patch is needed for rockchip drm, since
rockchip drm only use kms.
I saw the discussion about ("drm/exynos: create a fake mmap offset with gem creation"),
Inki said that: "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"

So I think this patch doesn't make sense for rockchip drm.

Quoted 54 lines · Heiko Stuebner

Move that to the creation of the gem to only create that offset once.

Signed-off-by Heiko Stuebner <[email protected]>

---
  drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 12 ++++++------
  1 file changed, 6 insertions(+), 6 deletions(-)

drivers/gpu/drm/rockchip/rockchip_drm_gem.c +6 -6

index 4ab2f20..e50d113 100644
@@ -126,8 +126,14 @@ struct rockchip_gem_object *
  	if (ret)
  		goto err_free_rk_obj;
 
+	ret = drm_gem_create_mmap_offset(obj);
+	if (ret)
+		goto err_free_buf;
+
  	return rk_obj;
 
+err_free_buf:
+	rockchip_gem_free_buf(rk_obj);
  err_free_rk_obj:
  	kfree(rk_obj);
  	return ERR_PTR(ret);

@@ -196,7 +202,6 @@ int rockchip_gem_dumb_map_offset(struct drm_file *file_priv,
  				 uint64_t *offset)
  {
  	struct drm_gem_object *obj;
-	int ret;
 
  	obj = drm_gem_object_lookup(dev, file_priv, handle);
  	if (!obj) {

@@ -204,14 +209,9 @@ int rockchip_gem_dumb_map_offset(struct drm_file *file_priv,
  		return -EINVAL;
  	}
 
-	ret = drm_gem_create_mmap_offset(obj);
-	if (ret)
-		goto out;
-
  	*offset = drm_vma_node_offset_addr(&obj->vma_node);
  	DRM_DEBUG_KMS("offset = 0x%llx\n", *offset);
 
-out:
  	drm_gem_object_unreference_unlocked(obj);
 
  	return 0;

Signature · 1 line

?ark Yao

Heiko Stuebner

author maintainer

2015-11-10 06:46 UTC #4 lore raw

Hi Mark,

Am Dienstag, 10. November 2015, 14:17:43 schrieb Mark yao:

Quoted 11 lines

Hi Heiko
I don't think this patch is needed for rockchip drm, since
rockchip drm only use kms.
I saw the discussion about ("drm/exynos: create a fake mmap offset with gem creation"),
Inki said that: "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"
So I think this patch doesn't make sense for rockchip drm.

ok :-) . I'm not that knowledgable when it comes to drm and just found that similar change in exynos while searching for the dma issue.

So we can drop this patch and you could just merge patch2 as well as

[RESEND PATCH] drm/rockchip: vop: Correct enabled clocks during setup

from Sjoerd and send these two to Dave as fixes for 4.4?

Thanks
Heiko

Quoted 8 lines · Heiko Stuebner

Quoted 5 lines
Similarly to the equivalent exynos-fix in commit 48cf53f4343a ("drm/exynos: create a fake mmap offset with gem creation") the rockchip drm driver also calls the drm_gem_create_mmap_offset fake-offset creation on every invocation of rockchip_gem_dumb_map_offset.

Quoted 8 lines

Review
Quoted 8 lines
Move that to the creation of the gem to only create that offset once.
Signed-off-by Heiko Stuebner <[email protected]>
---
  drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 12 ++++++------
  1 file changed, 6 insertions(+), 6 deletions(-)
(quoted diff) +0 -0

b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c

Quoted 13 lines

index 4ab2f20..e50d113 100644

drivers/gpu/drm/rockchip/rockchip_drm_gem.c +4 -0

@@ -126,8 +126,14 @@ struct rockchip_gem_object *
  	if (ret)
  		goto err_free_rk_obj;
 
+	ret = drm_gem_create_mmap_offset(obj);
+	if (ret)
+		goto err_free_buf;
+
  	return rk_obj;

Quoted 6 lines

drivers/gpu/drm/rockchip/rockchip_drm_gem.c int rockchip_gem_dumb_map_offset(struct drm_file
Quoted 6 lines
drivers/gpu/drm/rockchip/rockchip_drm_gem.c +2 -0
+err_free_buf:
+	rockchip_gem_free_buf(rk_obj);
  err_free_rk_obj:
  	kfree(rk_obj);
  	return ERR_PTR(ret);
@@ -196,7 +202,6 @@ int rockchip_gem_dumb_map_offset(struct drm_file 

*file_priv,

Quoted 8 lines

drivers/gpu/drm/rockchip/rockchip_drm_gem.c int rockchip_gem_dumb_map_offset(struct drm_file
Quoted 8 lines
drivers/gpu/drm/rockchip/rockchip_drm_gem.c +0 -1
  				 uint64_t *offset)
  {
  	struct drm_gem_object *obj;
-	int ret;
 
  	obj = drm_gem_object_lookup(dev, file_priv, handle);
  	if (!obj) {
@@ -204,14 +209,9 @@ int rockchip_gem_dumb_map_offset(struct drm_file 

*file_priv,

Quoted 17 lines

Quoted 14 lines

drivers/gpu/drm/rockchip/rockchip_drm_gem.c +0 -5

  		return -EINVAL;
  	}
 
-	ret = drm_gem_create_mmap_offset(obj);
-	if (ret)
-		goto out;
-
  	*offset = drm_vma_node_offset_addr(&obj->vma_node);
  	DRM_DEBUG_KMS("offset = 0x%llx\n", *offset);
 
-out:
  	drm_gem_object_unreference_unlocked(obj);
 
  	return 0;

Mark yao

2015-11-10 09:26 UTC #5 lore raw

Quoted 14 lines · Heiko Stuebner

Hi Mark,
Am Dienstag, 10. November 2015, 14:17:43 schrieb Mark yao:
Quoted 11 lines
Hi Heiko
I don't think this patch is needed for rockchip drm, since
rockchip drm only use kms.
I saw the discussion about ("drm/exynos: create a fake mmap offset with gem creation"),
Inki said that: "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"
So I think this patch doesn't make sense for rockchip drm.

Quoted 9 lines

ok :-) . I'm not that knowledgable when it comes to drm and just found that similar change in exynos while searching for the dma issue.
So we can drop this patch and you could just merge patch2 as well as
[RESEND PATCH] drm/rockchip: vop: Correct enabled clocks during setup
from Sjoerd and send these two to Dave as fixes for 4.4?

Right, that patch is needed, and also I want to land two patch:
[RESEND PATCH] drm/rockchip: import dma_buf to gem [PATCH] Revert "drm/rockchip: Convert the probe function to the
generic drm_of_component_probe()"

Quoted 67 lines

Thanks
Heiko
Quoted 7 lines · Heiko Stuebner
Quoted 5 lines
Similarly to the equivalent exynos-fix in commit 48cf53f4343a ("drm/exynos: create a fake mmap offset with gem creation") the rockchip drm driver also calls the drm_gem_create_mmap_offset fake-offset creation on every invocation of rockchip_gem_dumb_map_offset.
Quoted 8 lines
Review
Quoted 8 lines
Move that to the creation of the gem to only create that offset once.
Signed-off-by Heiko Stuebner <[email protected]>
---
   drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 12 ++++++------
   1 file changed, 6 insertions(+), 6 deletions(-)
(quoted diff) +0 -0
b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
Quoted 13 lines
Quoted 13 lines
index 4ab2f20..e50d113 100644
drivers/gpu/drm/rockchip/rockchip_drm_gem.c +4 -0
@@ -126,8 +126,14 @@ struct rockchip_gem_object *
   	if (ret)
   		goto err_free_rk_obj;
 
+	ret = drm_gem_create_mmap_offset(obj);
+	if (ret)
+		goto err_free_buf;
+
   	return rk_obj;
 
Quoted 6 lines
drivers/gpu/drm/rockchip/rockchip_drm_gem.c int rockchip_gem_dumb_map_offset(struct drm_file
Quoted 6 lines
drivers/gpu/drm/rockchip/rockchip_drm_gem.c +2 -0
+err_free_buf:
+	rockchip_gem_free_buf(rk_obj);
   err_free_rk_obj:
   	kfree(rk_obj);
   	return ERR_PTR(ret);
@@ -196,7 +202,6 @@ int rockchip_gem_dumb_map_offset(struct drm_file
*file_priv,
Quoted 8 lines
drivers/gpu/drm/rockchip/rockchip_drm_gem.c int rockchip_gem_dumb_map_offset(struct drm_file
Quoted 8 lines
drivers/gpu/drm/rockchip/rockchip_drm_gem.c +0 -1
   				 uint64_t *offset)
   {
   	struct drm_gem_object *obj;
-	int ret;
 
   	obj = drm_gem_object_lookup(dev, file_priv, handle);
   	if (!obj) {
@@ -204,14 +209,9 @@ int rockchip_gem_dumb_map_offset(struct drm_file
*file_priv,
Quoted 16 lines
Quoted 14 lines
drivers/gpu/drm/rockchip/rockchip_drm_gem.c +0 -5
   		return -EINVAL;
   	}
 
-	ret = drm_gem_create_mmap_offset(obj);
-	if (ret)
-		goto out;
-
   	*offset = drm_vma_node_offset_addr(&obj->vma_node);
   	DRM_DEBUG_KMS("offset = 0x%llx\n", *offset);
 
-out:
   	drm_gem_object_unreference_unlocked(obj);
 
   	return 0;

Signature · 1 line

?ark Yao

Heiko Stuebner

author maintainer

2015-11-10 09:28 UTC #6 lore raw

Am Dienstag, 10. November 2015, 17:26:10 schrieb Mark yao:

Quoted 18 lines · Heiko Stuebner

Quoted 17 lines
Hi Mark,
Am Dienstag, 10. November 2015, 14:17:43 schrieb Mark yao:
Quoted 11 lines
Hi Heiko
I don't think this patch is needed for rockchip drm, since
rockchip drm only use kms.
I saw the discussion about ("drm/exynos: create a fake mmap offset with gem creation"),
Inki said that: "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"
So I think this patch doesn't make sense for rockchip drm.
ok :-) . I'm not that knowledgable when it comes to drm and just found that similar change in exynos while searching for the dma issue.

Quoted 8 lines

Quoted 6 lines
So we can drop this patch and you could just merge patch2 as well as
[RESEND PATCH] drm/rockchip: vop: Correct enabled clocks during setup
from Sjoerd and send these two to Dave as fixes for 4.4?
Right, that patch is needed, and also I want to land two patch:
[RESEND PATCH] drm/rockchip: import dma_buf to gem

the import should go into 4.5 though (as it is a new feature), while the other three are fixes that should go into the current 4.4

Quoted 73 lines

[PATCH] Revert "drm/rockchip: Convert the probe function to the
generic drm_of_component_probe()"
Quoted 67 lines
Thanks
Heiko
Quoted 7 lines · Heiko Stuebner
Quoted 5 lines
Similarly to the equivalent exynos-fix in commit 48cf53f4343a ("drm/exynos: create a fake mmap offset with gem creation") the rockchip drm driver also calls the drm_gem_create_mmap_offset fake-offset creation on every invocation of rockchip_gem_dumb_map_offset.
Quoted 8 lines
Review
Quoted 8 lines
Move that to the creation of the gem to only create that offset once.
Signed-off-by Heiko Stuebner <[email protected]>
---
   drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 12 ++++++------
   1 file changed, 6 insertions(+), 6 deletions(-)
(quoted diff) +0 -0
b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
Quoted 13 lines
Quoted 13 lines
index 4ab2f20..e50d113 100644
drivers/gpu/drm/rockchip/rockchip_drm_gem.c +4 -0
@@ -126,8 +126,14 @@ struct rockchip_gem_object *
   	if (ret)
   		goto err_free_rk_obj;
 
+	ret = drm_gem_create_mmap_offset(obj);
+	if (ret)
+		goto err_free_buf;
+
   	return rk_obj;
 
Quoted 6 lines
drivers/gpu/drm/rockchip/rockchip_drm_gem.c int rockchip_gem_dumb_map_offset(struct drm_file
Quoted 6 lines
drivers/gpu/drm/rockchip/rockchip_drm_gem.c +2 -0
+err_free_buf:
+	rockchip_gem_free_buf(rk_obj);
   err_free_rk_obj:
   	kfree(rk_obj);
   	return ERR_PTR(ret);
@@ -196,7 +202,6 @@ int rockchip_gem_dumb_map_offset(struct drm_file
*file_priv,
Quoted 8 lines
drivers/gpu/drm/rockchip/rockchip_drm_gem.c int rockchip_gem_dumb_map_offset(struct drm_file
Quoted 8 lines
drivers/gpu/drm/rockchip/rockchip_drm_gem.c +0 -1
   				 uint64_t *offset)
   {
   	struct drm_gem_object *obj;
-	int ret;
 
   	obj = drm_gem_object_lookup(dev, file_priv, handle);
   	if (!obj) {
@@ -204,14 +209,9 @@ int rockchip_gem_dumb_map_offset(struct drm_file
*file_priv,
Quoted 16 lines
Quoted 14 lines
drivers/gpu/drm/rockchip/rockchip_drm_gem.c +0 -5
   		return -EINVAL;
   	}
 
-	ret = drm_gem_create_mmap_offset(obj);
-	if (ret)
-		goto out;
-
   	*offset = drm_vma_node_offset_addr(&obj->vma_node);
   	DRM_DEBUG_KMS("offset = 0x%llx\n", *offset);
 
-out:
   	drm_gem_object_unreference_unlocked(obj);
 
   	return 0;

Mark yao

2015-11-10 09:38 UTC #7 lore raw

Quoted 21 lines · Heiko Stuebner

Am Dienstag, 10. November 2015, 17:26:10 schrieb Mark yao:
Quoted 20 lines · Heiko Stuebner
Quoted 19 lines
Hi Mark,
Am Dienstag, 10. November 2015, 14:17:43 schrieb Mark yao:
Quoted 11 lines
Hi Heiko
I don't think this patch is needed for rockchip drm, since
rockchip drm only use kms.
I saw the discussion about ("drm/exynos: create a fake mmap offset with gem creation"),
Inki said that: "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"
So I think this patch doesn't make sense for rockchip drm.
ok :-) . I'm not that knowledgable when it comes to drm and just found that similar change in exynos while searching for the dma issue.
So we can drop this patch and you could just merge patch2 as well as

Quoted 9 lines

Quoted 6 lines
Quoted 4 lines
[RESEND PATCH] drm/rockchip: vop: Correct enabled clocks during setup
from Sjoerd and send these two to Dave as fixes for 4.4?
Right, that patch is needed, and also I want to land two patch:
[RESEND PATCH] drm/rockchip: import dma_buf to gem
the import should go into 4.5 though (as it is a new feature), while the other three are fixes that should go into the current 4.4

OK, Got it

Quoted 72 lines

Quoted 69 lines
[PATCH] Revert "drm/rockchip: Convert the probe function to the
generic drm_of_component_probe()"
Quoted 64 lines
Thanks
Heiko
Quoted 6 lines · Heiko Stuebner
Quoted 5 lines
Similarly to the equivalent exynos-fix in commit 48cf53f4343a ("drm/exynos: create a fake mmap offset with gem creation") the rockchip drm driver also calls the drm_gem_create_mmap_offset fake-offset creation on every invocation of rockchip_gem_dumb_map_offset.
Quoted 8 lines
Review
Quoted 8 lines
Move that to the creation of the gem to only create that offset once.
Signed-off-by Heiko Stuebner <[email protected]>
---
    drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 12 ++++++------
    1 file changed, 6 insertions(+), 6 deletions(-)
(quoted diff) +0 -0
b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
Quoted 13 lines
Quoted 13 lines
index 4ab2f20..e50d113 100644
drivers/gpu/drm/rockchip/rockchip_drm_gem.c +4 -0
@@ -126,8 +126,14 @@ struct rockchip_gem_object *
    	if (ret)
    		goto err_free_rk_obj;
 
+	ret = drm_gem_create_mmap_offset(obj);
+	if (ret)
+		goto err_free_buf;
+
    	return rk_obj;
 
Quoted 6 lines
drivers/gpu/drm/rockchip/rockchip_drm_gem.c int rockchip_gem_dumb_map_offset(struct drm_file
Quoted 6 lines
drivers/gpu/drm/rockchip/rockchip_drm_gem.c +2 -0
+err_free_buf:
+	rockchip_gem_free_buf(rk_obj);
    err_free_rk_obj:
    	kfree(rk_obj);
    	return ERR_PTR(ret);
@@ -196,7 +202,6 @@ int rockchip_gem_dumb_map_offset(struct drm_file
*file_priv,
Quoted 8 lines
drivers/gpu/drm/rockchip/rockchip_drm_gem.c int rockchip_gem_dumb_map_offset(struct drm_file
Quoted 8 lines
drivers/gpu/drm/rockchip/rockchip_drm_gem.c +0 -1
    				 uint64_t *offset)
    {
    	struct drm_gem_object *obj;
-	int ret;
 
    	obj = drm_gem_object_lookup(dev, file_priv, handle);
    	if (!obj) {
@@ -204,14 +209,9 @@ int rockchip_gem_dumb_map_offset(struct drm_file
*file_priv,
Quoted 15 lines
Quoted 14 lines
drivers/gpu/drm/rockchip/rockchip_drm_gem.c +0 -5
    		return -EINVAL;
    	}
 
-	ret = drm_gem_create_mmap_offset(obj);
-	if (ret)
-		goto out;
-
    	*offset = drm_vma_node_offset_addr(&obj->vma_node);
    	DRM_DEBUG_KMS("offset = 0x%llx\n", *offset);
 
-out:
    	drm_gem_object_unreference_unlocked(obj);
 
    	return 0;

Signature · 1 line

?ark Yao

Daniel Vetter

2015-11-16 16:21 UTC #8 lore raw

In reply to Mark yao

Quoted 10 lines · Mark yao

Hi Heiko
I don't think this patch is needed for rockchip drm, since rockchip drm
only use kms.
I saw the discussion about ("drm/exynos: create a fake mmap offset with gem creation"),
Inki said that: "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"

Quoted 5 lines

So I think this patch doesn't make sense for rockchip drm.
Quoted 1 line · Heiko Stuebner
Similarly to the equivalent exynos-fix in commit 48cf53f4343a

This exynos patch and your rockchip patch look really strange - the point _is_ to only create fake mmap offset when someone actually tries to mmap a bo. drm_gem_create_mmap_offset internally (and in a race-free manner) checks whether it's been called already, so it's perfectly safe to call mutliple times.

Imo drop this (and probably revert 48cf53f4343a as unjustified). -Daniel

Quoted 57 lines

Quoted 47 lines

("drm/exynos: create a fake mmap offset with gem creation") the rockchip drm driver also calls the drm_gem_create_mmap_offset fake-offset creation on every invocation of rockchip_gem_dumb_map_offset.

Move that to the creation of the gem to only create that offset once.

Signed-off-by Heiko Stuebner <[email protected]>

---
 drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

drivers/gpu/drm/rockchip/rockchip_drm_gem.c +6 -6

index 4ab2f20..e50d113 100644
@@ -126,8 +126,14 @@ struct rockchip_gem_object *
 	if (ret)
 		goto err_free_rk_obj;
+	ret = drm_gem_create_mmap_offset(obj);
+	if (ret)
+		goto err_free_buf;
+
 	return rk_obj;
+err_free_buf:
+	rockchip_gem_free_buf(rk_obj);
 err_free_rk_obj:
 	kfree(rk_obj);
 	return ERR_PTR(ret);

@@ -196,7 +202,6 @@ int rockchip_gem_dumb_map_offset(struct drm_file *file_priv,
 				 uint64_t *offset)
 {
 	struct drm_gem_object *obj;
-	int ret;
 	obj = drm_gem_object_lookup(dev, file_priv, handle);
 	if (!obj) {

@@ -204,14 +209,9 @@ int rockchip_gem_dumb_map_offset(struct drm_file *file_priv,
 		return -EINVAL;
 	}
-	ret = drm_gem_create_mmap_offset(obj);
-	if (ret)
-		goto out;
-
 	*offset = drm_vma_node_offset_addr(&obj->vma_node);
 	DRM_DEBUG_KMS("offset = 0x%llx\n", *offset);
-out:
 	drm_gem_object_unreference_unlocked(obj);
 	return 0;

Signature · 7 lines

?ark Yao


_______________________________________________
dri-devel mailing list
dri-devel at lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Signature · 3 lines

Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch