[PATCH 0/7] gem_bo.resv prime unification, leftovers

[PATCH 0/7] gem_bo.resv prime unification, leftovers

[Daniel Vetter]

Hi all,

Here's the unmerged leftovers from my big prime cleanup series:
- using the prepare_fb helper in vc4&msm, now hopefully fixed up. The
  helper should be now even more useful.

- amd&nv driver ->gem_prime_res_obj callback removal. I think this one
  might have functional conflicts with Gerd's patch series to embed
  drm_gem_object in ttm_bo, or at least needs to be re-reviewed before we
  merge the 2nd series.

Comments, testing, feedback as usual very much welcome.

Thanks, Daniel

Daniel Vetter (7):
  drm/fb-helper: use gem_bo.resv, not dma_buf.resv in prepare_fb
  drm/msm: Use drm_gem_fb_prepare_fb
  drm/vc4: Use drm_gem_fb_prepare_fb
  drm/radeon: Fill out gem_object->resv
  drm/nouveau: Fill out gem_object->resv
  drm/amdgpu: Fill out gem_object->resv
  drm/prime: Ditch gem_prime_res_obj hook

 Documentation/gpu/todo.rst                   |  9 ------
 drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c  | 17 +-----------
 drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h  |  1 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c      |  1 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c   |  2 ++
 drivers/gpu/drm/drm_gem_framebuffer_helper.c | 29 ++++++++++----------
 drivers/gpu/drm/drm_prime.c                  |  3 --
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c    |  8 ++----
 drivers/gpu/drm/msm/msm_atomic.c             |  8 ++----
 drivers/gpu/drm/nouveau/nouveau_bo.c         |  2 ++
 drivers/gpu/drm/nouveau/nouveau_drm.c        |  1 -
 drivers/gpu/drm/nouveau/nouveau_gem.h        |  1 -
 drivers/gpu/drm/nouveau/nouveau_prime.c      |  7 -----
 drivers/gpu/drm/radeon/radeon_drv.c          |  2 --
 drivers/gpu/drm/radeon/radeon_object.c       |  1 +
 drivers/gpu/drm/radeon/radeon_prime.c        |  7 -----
 drivers/gpu/drm/vc4/vc4_plane.c              |  5 ++--
 include/drm/drm_drv.h                        | 12 --------
 18 files changed, 26 insertions(+), 90 deletions(-)

-- 
2.20.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/7] drm/fb-helper: use gem_bo.resv, not dma_buf.resv in prepare_fb

[Daniel Vetter]

With

commit 5f6ed9879a414636405a2bd77f122881695959e4
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Fri Jun 14 22:35:57 2019 +0200

    drm/prime: automatically set gem_obj->resv on import

we consistently set drm_gem_bo.resv for imported buffers. Which means
we don't need to check to check the dma-buf in the prepare_fb helper,
but can generalize them so they're also useful for display+render
drivers which use gem_bo.resv to track their own rendering for their
own scanout buffers.

Cc: Emil Velikov <emil.velikov@collabora.com>
Cc: Eric Anholt <eric@anholt.net>
Cc: Rob Clark <robdclark@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_gem_framebuffer_helper.c | 29 ++++++++++----------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
index 8fcbabf02dfd..a6426c95b383 100644
--- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
+++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
@@ -271,11 +271,11 @@ EXPORT_SYMBOL_GPL(drm_gem_fb_create_with_dirty);
  * @plane: Plane
  * @state: Plane state the fence will be attached to
  *
- * This function prepares a GEM backed framebuffer for scanout by checking if
- * the plane framebuffer has a DMA-BUF attached. If it does, it extracts the
- * exclusive fence and attaches it to the plane state for the atomic helper to
- * wait on. This function can be used as the &drm_plane_helper_funcs.prepare_fb
- * callback.
+ * This function extracts the exclusive fence from &drm_gem_object.resv and
+ * attaches it to plane state for the atomic helper to wait on. This is
+ * necessary to correctly implement implicit synchronization for any buffers
+ * shared as a struct &dma_buf. This function can be used as the
+ * &drm_plane_helper_funcs.prepare_fb callback.
  *
  * There is no need for &drm_plane_helper_funcs.cleanup_fb hook for simple
  * gem based framebuffer drivers which have their buffers always pinned in
@@ -287,17 +287,15 @@ EXPORT_SYMBOL_GPL(drm_gem_fb_create_with_dirty);
 int drm_gem_fb_prepare_fb(struct drm_plane *plane,
 			  struct drm_plane_state *state)
 {
-	struct dma_buf *dma_buf;
+	struct drm_gem_object *obj;
 	struct dma_fence *fence;
 
 	if (!state->fb)
 		return 0;
 
-	dma_buf = drm_gem_fb_get_obj(state->fb, 0)->dma_buf;
-	if (dma_buf) {
-		fence = reservation_object_get_excl_rcu(dma_buf->resv);
-		drm_atomic_set_fence_for_plane(state, fence);
-	}
+	obj = drm_gem_fb_get_obj(state->fb, 0);
+	fence = reservation_object_get_excl_rcu(obj->resv);
+	drm_atomic_set_fence_for_plane(state, fence);
 
 	return 0;
 }
@@ -309,10 +307,11 @@ EXPORT_SYMBOL_GPL(drm_gem_fb_prepare_fb);
  * @pipe: Simple display pipe
  * @plane_state: Plane state
  *
- * This function uses drm_gem_fb_prepare_fb() to check if the plane FB has a
- * &dma_buf attached, extracts the exclusive fence and attaches it to plane
- * state for the atomic helper to wait on. Drivers can use this as their
- * &drm_simple_display_pipe_funcs.prepare_fb callback.
+ * This function uses drm_gem_fb_prepare_fb() to extract the exclusive fence
+ * from &drm_gem_object.resv and attaches it to plane state for the atomic
+ * helper to wait on. This is necessary to correctly implement implicit
+ * synchronization for any buffers shared as a struct &dma_buf. Drivers can use
+ * this as their &drm_simple_display_pipe_funcs.prepare_fb callback.
  *
  * See drm_atomic_set_fence_for_plane() for a discussion of implicit and
  * explicit fencing in atomic modeset updates.
-- 
2.20.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/7] drm/msm: Use drm_gem_fb_prepare_fb

[Daniel Vetter]

msm has switched over to drm_fb->obj[] a while ago already, so we can
just use the helper.

v2: Make it compile ... oops.

Cc: Eric Anholt <eric@anholt.net>
Cc: Emil Velikov <emil.velikov@collabora.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Rob Clark <robdclark@gmail.com>
Cc: Sean Paul <sean@poorly.run>
Cc: Jeykumar Sankaran <jsanka@codeaurora.org>
Cc: Jordan Crouse <jcrouse@codeaurora.org>
Cc: Bruce Wang <bzwang@chromium.org>
Cc: Fritz Koenig <frkoenig@google.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: linux-arm-msm@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 8 ++------
 drivers/gpu/drm/msm/msm_atomic.c          | 8 ++------
 2 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 44a72da71482..cc08f4366bdd 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -11,6 +11,7 @@
 #include <linux/dma-buf.h>
 
 #include <drm/drm_atomic_uapi.h>
+#include <drm/drm_gem_framebuffer_helper.h>
 
 #include "msm_drv.h"
 #include "dpu_kms.h"
@@ -763,8 +764,6 @@ static int dpu_plane_prepare_fb(struct drm_plane *plane,
 	struct dpu_plane *pdpu = to_dpu_plane(plane);
 	struct dpu_plane_state *pstate = to_dpu_plane_state(new_state);
 	struct dpu_hw_fmt_layout layout;
-	struct drm_gem_object *obj;
-	struct dma_fence *fence;
 	struct dpu_kms *kms = _dpu_plane_get_kms(&pdpu->base);
 	int ret;
 
@@ -781,10 +780,7 @@ static int dpu_plane_prepare_fb(struct drm_plane *plane,
 	 *       we can use msm_atomic_prepare_fb() instead of doing the
 	 *       implicit fence and fb prepare by hand here.
 	 */
-	obj = msm_framebuffer_bo(new_state->fb, 0);
-	fence = reservation_object_get_excl_rcu(obj->resv);
-	if (fence)
-		drm_atomic_set_fence_for_plane(new_state, fence);
+	drm_gem_fb_prepare_fb(plane, new_state);
 
 	if (pstate->aspace) {
 		ret = msm_framebuffer_prepare(new_state->fb,
diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c
index dd16babdd8c0..169d5f915e68 100644
--- a/drivers/gpu/drm/msm/msm_atomic.c
+++ b/drivers/gpu/drm/msm/msm_atomic.c
@@ -5,6 +5,7 @@
  */
 
 #include <drm/drm_atomic_uapi.h>
+#include <drm/drm_gem_framebuffer_helper.h>
 
 #include "msm_drv.h"
 #include "msm_gem.h"
@@ -37,16 +38,11 @@ int msm_atomic_prepare_fb(struct drm_plane *plane,
 {
 	struct msm_drm_private *priv = plane->dev->dev_private;
 	struct msm_kms *kms = priv->kms;
-	struct drm_gem_object *obj;
-	struct dma_fence *fence;
 
 	if (!new_state->fb)
 		return 0;
 
-	obj = msm_framebuffer_bo(new_state->fb, 0);
-	fence = reservation_object_get_excl_rcu(obj->resv);
-
-	drm_atomic_set_fence_for_plane(new_state, fence);
+	drm_gem_fb_prepare_fb(plane, new_state);
 
 	return msm_framebuffer_prepare(new_state->fb, kms->aspace);
 }
-- 
2.20.1

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH 3/7] drm/vc4: Use drm_gem_fb_prepare_fb

[Daniel Vetter]

vc4 has switched to using drm_fb->obj[], so we can just use the helper
unchanged.

v2: Make it compile ... oops.

Cc: Eric Anholt <eric@anholt.net>
Cc: Emil Velikov <emil.velikov@collabora.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Eric Anholt <eric@anholt.net>
---
 drivers/gpu/drm/vc4/vc4_plane.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c
index 0a0207c350a5..a996ca8ff972 100644
--- a/drivers/gpu/drm/vc4/vc4_plane.c
+++ b/drivers/gpu/drm/vc4/vc4_plane.c
@@ -20,6 +20,7 @@
 #include <drm/drm_fb_cma_helper.h>
 #include <drm/drm_plane_helper.h>
 #include <drm/drm_atomic_uapi.h>
+#include <drm/drm_gem_framebuffer_helper.h>
 
 #include "uapi/drm/vc4_drm.h"
 #include "vc4_drv.h"
@@ -1123,7 +1124,6 @@ static int vc4_prepare_fb(struct drm_plane *plane,
 			  struct drm_plane_state *state)
 {
 	struct vc4_bo *bo;
-	struct dma_fence *fence;
 	int ret;
 
 	if (!state->fb)
@@ -1131,8 +1131,7 @@ static int vc4_prepare_fb(struct drm_plane *plane,
 
 	bo = to_vc4_bo(&drm_fb_cma_get_gem_obj(state->fb, 0)->base);
 
-	fence = reservation_object_get_excl_rcu(bo->base.base.resv);
-	drm_atomic_set_fence_for_plane(state, fence);
+	drm_gem_fb_prepare_fb(plane, state);
 
 	if (plane->state->fb == state->fb)
 		return 0;
-- 
2.20.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/7] drm/radeon: Fill out gem_object->resv

[Daniel Vetter]

That way we can ditch our gem_prime_res_obj implementation. Since ttm
absolutely needs the right reservation object all the boilerplate is
already there and we just have to wire it up correctly.

Note that gem/prime doesn't care when we do this, as long as we do it
before the bo is registered and someone can call the handle2fd ioctl
on it.

Aside: ttm_buffer_object.ttm_resv could probably be ditched in favour
of always passing a non-NULL resv to ttm_bo_init(). At least for gem
drivers that would avoid having two of these, on in ttm_buffer_object
and the other in drm_gem_object, one just there for confusion.

Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
Cc: amd-gfx@lists.freedesktop.org
---
 drivers/gpu/drm/radeon/radeon_drv.c    | 2 --
 drivers/gpu/drm/radeon/radeon_object.c | 1 +
 drivers/gpu/drm/radeon/radeon_prime.c  | 7 -------
 3 files changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
index 4403e76e1ae0..a4a78dfdef37 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -152,7 +152,6 @@ struct drm_gem_object *radeon_gem_prime_import_sg_table(struct drm_device *dev,
 							struct sg_table *sg);
 int radeon_gem_prime_pin(struct drm_gem_object *obj);
 void radeon_gem_prime_unpin(struct drm_gem_object *obj);
-struct reservation_object *radeon_gem_prime_res_obj(struct drm_gem_object *);
 void *radeon_gem_prime_vmap(struct drm_gem_object *obj);
 void radeon_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr);
 
@@ -566,7 +565,6 @@ static struct drm_driver kms_driver = {
 	.gem_prime_export = radeon_gem_prime_export,
 	.gem_prime_pin = radeon_gem_prime_pin,
 	.gem_prime_unpin = radeon_gem_prime_unpin,
-	.gem_prime_res_obj = radeon_gem_prime_res_obj,
 	.gem_prime_get_sg_table = radeon_gem_prime_get_sg_table,
 	.gem_prime_import_sg_table = radeon_gem_prime_import_sg_table,
 	.gem_prime_vmap = radeon_gem_prime_vmap,
diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
index 21f73fc86f38..7a2bad843f8a 100644
--- a/drivers/gpu/drm/radeon/radeon_object.c
+++ b/drivers/gpu/drm/radeon/radeon_object.c
@@ -262,6 +262,7 @@ int radeon_bo_create(struct radeon_device *rdev,
 	r = ttm_bo_init(&rdev->mman.bdev, &bo->tbo, size, type,
 			&bo->placement, page_align, !kernel, acc_size,
 			sg, resv, &radeon_ttm_bo_destroy);
+	bo->gem_base.resv = bo->tbo.resv;
 	up_read(&rdev->pm.mclk_lock);
 	if (unlikely(r != 0)) {
 		return r;
diff --git a/drivers/gpu/drm/radeon/radeon_prime.c b/drivers/gpu/drm/radeon/radeon_prime.c
index deaffce50a2e..8ce3e8045d42 100644
--- a/drivers/gpu/drm/radeon/radeon_prime.c
+++ b/drivers/gpu/drm/radeon/radeon_prime.c
@@ -117,13 +117,6 @@ void radeon_gem_prime_unpin(struct drm_gem_object *obj)
 }
 
 
-struct reservation_object *radeon_gem_prime_res_obj(struct drm_gem_object *obj)
-{
-	struct radeon_bo *bo = gem_to_radeon_bo(obj);
-
-	return bo->tbo.resv;
-}
-
 struct dma_buf *radeon_gem_prime_export(struct drm_gem_object *gobj,
 					int flags)
 {
-- 
2.20.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 5/7] drm/nouveau: Fill out gem_object->resv

[Daniel Vetter]

That way we can ditch our gem_prime_res_obj implementation. Since ttm
absolutely needs the right reservation object all the boilerplate is
already there and we just have to wire it up correctly.

Note that gem/prime doesn't care when we do this, as long as we do it
before the bo is registered and someone can call the handle2fd ioctl
on it.

Aside: ttm_buffer_object.ttm_resv could probably be ditched in favour
of always passing a non-NULL resv to ttm_bo_init(). At least for gem
drivers that would avoid having two of these, on in ttm_buffer_object
and the other in drm_gem_object, one just there for confusion.

Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: nouveau@lists.freedesktop.org
---
 drivers/gpu/drm/nouveau/nouveau_bo.c    | 2 ++
 drivers/gpu/drm/nouveau/nouveau_drm.c   | 1 -
 drivers/gpu/drm/nouveau/nouveau_gem.h   | 1 -
 drivers/gpu/drm/nouveau/nouveau_prime.c | 7 -------
 4 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 34a998012bf6..6f1217b3e6b9 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -299,6 +299,8 @@ nouveau_bo_new(struct nouveau_cli *cli, u64 size, int align,
 			  type, &nvbo->placement,
 			  align >> PAGE_SHIFT, false, acc_size, sg,
 			  robj, nouveau_bo_del_ttm);
+	nvbo->gem.resv = nvbo->bo.resv;
+
 	if (ret) {
 		/* ttm will call nouveau_bo_del_ttm if it fails.. */
 		return ret;
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
index 4377b836265f..2c36319c158f 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -1131,7 +1131,6 @@ driver_stub = {
 	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
 	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
 	.gem_prime_pin = nouveau_gem_prime_pin,
-	.gem_prime_res_obj = nouveau_gem_prime_res_obj,
 	.gem_prime_unpin = nouveau_gem_prime_unpin,
 	.gem_prime_get_sg_table = nouveau_gem_prime_get_sg_table,
 	.gem_prime_import_sg_table = nouveau_gem_prime_import_sg_table,
diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.h b/drivers/gpu/drm/nouveau/nouveau_gem.h
index fe39998f65cc..9dea015387fd 100644
--- a/drivers/gpu/drm/nouveau/nouveau_gem.h
+++ b/drivers/gpu/drm/nouveau/nouveau_gem.h
@@ -33,7 +33,6 @@ extern int nouveau_gem_ioctl_info(struct drm_device *, void *,
 				  struct drm_file *);
 
 extern int nouveau_gem_prime_pin(struct drm_gem_object *);
-struct reservation_object *nouveau_gem_prime_res_obj(struct drm_gem_object *);
 extern void nouveau_gem_prime_unpin(struct drm_gem_object *);
 extern struct sg_table *nouveau_gem_prime_get_sg_table(struct drm_gem_object *);
 extern struct drm_gem_object *nouveau_gem_prime_import_sg_table(
diff --git a/drivers/gpu/drm/nouveau/nouveau_prime.c b/drivers/gpu/drm/nouveau/nouveau_prime.c
index 1fefc93af1d7..ec50017692d4 100644
--- a/drivers/gpu/drm/nouveau/nouveau_prime.c
+++ b/drivers/gpu/drm/nouveau/nouveau_prime.c
@@ -107,10 +107,3 @@ void nouveau_gem_prime_unpin(struct drm_gem_object *obj)
 
 	nouveau_bo_unpin(nvbo);
 }
-
-struct reservation_object *nouveau_gem_prime_res_obj(struct drm_gem_object *obj)
-{
-	struct nouveau_bo *nvbo = nouveau_gem_object(obj);
-
-	return nvbo->bo.resv;
-}
-- 
2.20.1

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

[PATCH 6/7] drm/amdgpu: Fill out gem_object->resv

[Daniel Vetter]

That way we can ditch our gem_prime_res_obj implementation. Since ttm
absolutely needs the right reservation object all the boilerplate is
already there and we just have to wire it up correctly.

Note that gem/prime doesn't care when we do this, as long as we do it
before the bo is registered and someone can call the handle2fd ioctl
on it.

Aside: ttm_buffer_object.ttm_resv could probably be ditched in favour
of always passing a non-NULL resv to ttm_bo_init(). At least for gem
drivers that would avoid having two of these, on in ttm_buffer_object
and the other in drm_gem_object, one just there for confusion.

Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: "Michel Dänzer" <michel.daenzer@amd.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Huang Rui <ray.huang@amd.com>
Cc: Felix Kuehling <Felix.Kuehling@amd.com>
Cc: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
Cc: Evan Quan <evan.quan@amd.com>
Cc: Sonny Jiang <sonny.jiang@amd.com>
Cc: Amber Lin <Amber.Lin@amd.com>
Cc: "Michał Mirosław" <mirq-linux@rere.qmqm.pl>
Cc: Junwei Zhang <Jerry.Zhang@amd.com>
Cc: Thomas Zimmermann <contact@tzimmermann.org>
Cc: Samuel Li <Samuel.Li@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 17 +----------------
 drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h |  1 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c     |  1 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c  |  2 ++
 4 files changed, 3 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index 4809d4a5d72a..02cd845e77b3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -267,20 +267,6 @@ static void amdgpu_dma_buf_map_detach(struct dma_buf *dma_buf,
 	drm_gem_map_detach(dma_buf, attach);
 }
 
-/**
- * amdgpu_gem_prime_res_obj - &drm_driver.gem_prime_res_obj implementation
- * @obj: GEM BO
- *
- * Returns:
- * The BO's reservation object.
- */
-struct reservation_object *amdgpu_gem_prime_res_obj(struct drm_gem_object *obj)
-{
-	struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
-
-	return bo->tbo.resv;
-}
-
 /**
  * amdgpu_dma_buf_begin_cpu_access - &dma_buf_ops.begin_cpu_access implementation
  * @dma_buf: Shared DMA buffer
@@ -339,8 +325,7 @@ const struct dma_buf_ops amdgpu_dmabuf_ops = {
  * @gobj: GEM BO
  * @flags: Flags such as DRM_CLOEXEC and DRM_RDWR.
  *
- * The main work is done by the &drm_gem_prime_export helper, which in turn
- * uses &amdgpu_gem_prime_res_obj.
+ * The main work is done by the &drm_gem_prime_export helper.
  *
  * Returns:
  * Shared DMA buffer representing the GEM BO from the given device.
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
index 7f73a4f94204..5012e6ab58f1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
@@ -34,7 +34,6 @@ struct dma_buf *amdgpu_gem_prime_export(struct drm_gem_object *gobj,
 					int flags);
 struct drm_gem_object *amdgpu_gem_prime_import(struct drm_device *dev,
 					    struct dma_buf *dma_buf);
-struct reservation_object *amdgpu_gem_prime_res_obj(struct drm_gem_object *);
 void *amdgpu_gem_prime_vmap(struct drm_gem_object *obj);
 void amdgpu_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr);
 int amdgpu_gem_prime_mmap(struct drm_gem_object *obj,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 8e1b269351e8..3233c5abf5b6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1333,7 +1333,6 @@ static struct drm_driver kms_driver = {
 	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
 	.gem_prime_export = amdgpu_gem_prime_export,
 	.gem_prime_import = amdgpu_gem_prime_import,
-	.gem_prime_res_obj = amdgpu_gem_prime_res_obj,
 	.gem_prime_get_sg_table = amdgpu_gem_prime_get_sg_table,
 	.gem_prime_import_sg_table = amdgpu_gem_prime_import_sg_table,
 	.gem_prime_vmap = amdgpu_gem_prime_vmap,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 16f96f2e3671..7b251fd26bd5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -505,6 +505,8 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev,
 	if (unlikely(r != 0))
 		return r;
 
+	bo->gem_base.resv = bo->tbo.resv;
+
 	if (!amdgpu_gmc_vram_full_visible(&adev->gmc) &&
 	    bo->tbo.mem.mem_type == TTM_PL_VRAM &&
 	    bo->tbo.mem.start < adev->gmc.visible_vram_size >> PAGE_SHIFT)
-- 
2.20.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 7/7] drm/prime: Ditch gem_prime_res_obj hook

[Daniel Vetter]

Everyone is just using gem_object->resv now.

Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <maxime.ripard@bootlin.com>
Cc: Sean Paul <sean@poorly.run>
---
 Documentation/gpu/todo.rst  |  9 ---------
 drivers/gpu/drm/drm_prime.c |  3 ---
 include/drm/drm_drv.h       | 12 ------------
 3 files changed, 24 deletions(-)

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index d49c1cc6dc28..e717f280f9ae 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -196,15 +196,6 @@ Might be good to also have some igt testcases for this.
 
 Contact: Daniel Vetter, Noralf Tronnes
 
-Remove the ->gem_prime_res_obj callback
---------------------------------------------
-
-The ->gem_prime_res_obj callback can be removed from drivers by using the
-reservation_object in the drm_gem_object. It may also be possible to use the
-generic drm_gem_reservation_object_wait helper for waiting for a bo.
-
-Contact: Daniel Vetter
-
 idr_init_base()
 ---------------
 
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 189d980402ad..b3b044d4089a 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -835,9 +835,6 @@ struct dma_buf *drm_gem_prime_export(struct drm_gem_object *obj,
 		.resv = obj->resv,
 	};
 
-	if (dev->driver->gem_prime_res_obj)
-		exp_info.resv = dev->driver->gem_prime_res_obj(obj);
-
 	return drm_gem_dmabuf_export(dev, &exp_info);
 }
 EXPORT_SYMBOL(drm_gem_prime_export);
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index 9927f4f894ef..380e134c5415 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -618,18 +618,6 @@ struct drm_driver {
 	 */
 	struct sg_table *(*gem_prime_get_sg_table)(struct drm_gem_object *obj);
 
-	/**
-	 * @gem_prime_res_obj:
-	 *
-	 * Optional hook to look up the &reservation_object for an buffer when
-	 * exporting it.
-	 *
-	 * FIXME: This hook is deprecated. Users of this hook should be replaced
-	 * by setting &drm_gem_object.resv instead.
-	 */
-	struct reservation_object * (*gem_prime_res_obj)(
-				struct drm_gem_object *obj);
-
 	/**
 	 * @gem_prime_import_sg_table:
 	 *
-- 
2.20.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/7] drm/fb-helper: use gem_bo.resv, not dma_buf.resv in prepare_fb

[Sam Ravnborg]

On Tue, Jun 25, 2019 at 10:42:02PM +0200, Daniel Vetter wrote:
> With
> 
> commit 5f6ed9879a414636405a2bd77f122881695959e4
> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> Date:   Fri Jun 14 22:35:57 2019 +0200
> 
>     drm/prime: automatically set gem_obj->resv on import
> 
> we consistently set drm_gem_bo.resv for imported buffers. Which means
> we don't need to check to check the dma-buf in the prepare_fb helper,
checked a bit too much?
> but can generalize them so they're also useful for display+render
> drivers which use gem_bo.resv to track their own rendering for their
> own scanout buffers.
> 
> Cc: Emil Velikov <emil.velikov@collabora.com>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: Rob Clark <robdclark@gmail.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/7] drm/radeon: Fill out gem_object->resv

[Sam Ravnborg]

On Tue, Jun 25, 2019 at 10:42:05PM +0200, Daniel Vetter wrote:
> That way we can ditch our gem_prime_res_obj implementation. Since ttm
> absolutely needs the right reservation object all the boilerplate is
> already there and we just have to wire it up correctly.
> 
> Note that gem/prime doesn't care when we do this, as long as we do it
> before the bo is registered and someone can call the handle2fd ioctl
> on it.
> 
> Aside: ttm_buffer_object.ttm_resv could probably be ditched in favour
> of always passing a non-NULL resv to ttm_bo_init(). At least for gem
> drivers that would avoid having two of these, on in ttm_buffer_object
> and the other in drm_gem_object, one just there for confusion.
Something for todo.rst - so this does not get lost in a changelog
people will soon forget?

	Sam
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/7] drm/msm: Use drm_gem_fb_prepare_fb

[Rob Clark]

On Tue, Jun 25, 2019 at 1:42 PM Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>
> msm has switched over to drm_fb->obj[] a while ago already, so we can
> just use the helper.
>
> v2: Make it compile ... oops.
>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: Emil Velikov <emil.velikov@collabora.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Sean Paul <sean@poorly.run>
> Cc: Jeykumar Sankaran <jsanka@codeaurora.org>
> Cc: Jordan Crouse <jcrouse@codeaurora.org>
> Cc: Bruce Wang <bzwang@chromium.org>
> Cc: Fritz Koenig <frkoenig@google.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: linux-arm-msm@vger.kernel.org
> Cc: freedreno@lists.freedesktop.org

with 1/7 this is:

Reviewed-by: Rob Clark <robdclark@gmail.com>

(and 1/7 is r-b w/ commit msg fixup)

BR,
-R


> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 8 ++------
>  drivers/gpu/drm/msm/msm_atomic.c          | 8 ++------
>  2 files changed, 4 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index 44a72da71482..cc08f4366bdd 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -11,6 +11,7 @@
>  #include <linux/dma-buf.h>
>
>  #include <drm/drm_atomic_uapi.h>
> +#include <drm/drm_gem_framebuffer_helper.h>
>
>  #include "msm_drv.h"
>  #include "dpu_kms.h"
> @@ -763,8 +764,6 @@ static int dpu_plane_prepare_fb(struct drm_plane *plane,
>         struct dpu_plane *pdpu = to_dpu_plane(plane);
>         struct dpu_plane_state *pstate = to_dpu_plane_state(new_state);
>         struct dpu_hw_fmt_layout layout;
> -       struct drm_gem_object *obj;
> -       struct dma_fence *fence;
>         struct dpu_kms *kms = _dpu_plane_get_kms(&pdpu->base);
>         int ret;
>
> @@ -781,10 +780,7 @@ static int dpu_plane_prepare_fb(struct drm_plane *plane,
>          *       we can use msm_atomic_prepare_fb() instead of doing the
>          *       implicit fence and fb prepare by hand here.
>          */
> -       obj = msm_framebuffer_bo(new_state->fb, 0);
> -       fence = reservation_object_get_excl_rcu(obj->resv);
> -       if (fence)
> -               drm_atomic_set_fence_for_plane(new_state, fence);
> +       drm_gem_fb_prepare_fb(plane, new_state);
>
>         if (pstate->aspace) {
>                 ret = msm_framebuffer_prepare(new_state->fb,
> diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c
> index dd16babdd8c0..169d5f915e68 100644
> --- a/drivers/gpu/drm/msm/msm_atomic.c
> +++ b/drivers/gpu/drm/msm/msm_atomic.c
> @@ -5,6 +5,7 @@
>   */
>
>  #include <drm/drm_atomic_uapi.h>
> +#include <drm/drm_gem_framebuffer_helper.h>
>
>  #include "msm_drv.h"
>  #include "msm_gem.h"
> @@ -37,16 +38,11 @@ int msm_atomic_prepare_fb(struct drm_plane *plane,
>  {
>         struct msm_drm_private *priv = plane->dev->dev_private;
>         struct msm_kms *kms = priv->kms;
> -       struct drm_gem_object *obj;
> -       struct dma_fence *fence;
>
>         if (!new_state->fb)
>                 return 0;
>
> -       obj = msm_framebuffer_bo(new_state->fb, 0);
> -       fence = reservation_object_get_excl_rcu(obj->resv);
> -
> -       drm_atomic_set_fence_for_plane(new_state, fence);
> +       drm_gem_fb_prepare_fb(plane, new_state);
>
>         return msm_framebuffer_prepare(new_state->fb, kms->aspace);
>  }
> --
> 2.20.1
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/7] drm/fb-helper: use gem_bo.resv, not dma_buf.resv in prepare_fb

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 620 bytes --]

Daniel Vetter <daniel.vetter@ffwll.ch> writes:

> With
>
> commit 5f6ed9879a414636405a2bd77f122881695959e4
> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> Date:   Fri Jun 14 22:35:57 2019 +0200
>
>     drm/prime: automatically set gem_obj->resv on import
>
> we consistently set drm_gem_bo.resv for imported buffers. Which means
> we don't need to check to check the dma-buf in the prepare_fb helper,
> but can generalize them so they're also useful for display+render
> drivers which use gem_bo.resv to track their own rendering for their
> own scanout buffers.

1-3 are:

Reviewed-by: Eric Anholt <eric@anholt.net>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/7] drm/radeon: Fill out gem_object->resv

[Koenig, Christian]

Those patches would become superfluous when merging Gerd's work.

But I'm not sure if that is going to fly soon or not.

Christian.

Am 25.06.19 um 22:42 schrieb Daniel Vetter:
> That way we can ditch our gem_prime_res_obj implementation. Since ttm
> absolutely needs the right reservation object all the boilerplate is
> already there and we just have to wire it up correctly.
>
> Note that gem/prime doesn't care when we do this, as long as we do it
> before the bo is registered and someone can call the handle2fd ioctl
> on it.
>
> Aside: ttm_buffer_object.ttm_resv could probably be ditched in favour
> of always passing a non-NULL resv to ttm_bo_init(). At least for gem
> drivers that would avoid having two of these, on in ttm_buffer_object
> and the other in drm_gem_object, one just there for confusion.
>
> Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: "Christian König" <christian.koenig@amd.com>
> Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
> Cc: amd-gfx@lists.freedesktop.org
> ---
>   drivers/gpu/drm/radeon/radeon_drv.c    | 2 --
>   drivers/gpu/drm/radeon/radeon_object.c | 1 +
>   drivers/gpu/drm/radeon/radeon_prime.c  | 7 -------
>   3 files changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
> index 4403e76e1ae0..a4a78dfdef37 100644
> --- a/drivers/gpu/drm/radeon/radeon_drv.c
> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> @@ -152,7 +152,6 @@ struct drm_gem_object *radeon_gem_prime_import_sg_table(struct drm_device *dev,
>   							struct sg_table *sg);
>   int radeon_gem_prime_pin(struct drm_gem_object *obj);
>   void radeon_gem_prime_unpin(struct drm_gem_object *obj);
> -struct reservation_object *radeon_gem_prime_res_obj(struct drm_gem_object *);
>   void *radeon_gem_prime_vmap(struct drm_gem_object *obj);
>   void radeon_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr);
>   
> @@ -566,7 +565,6 @@ static struct drm_driver kms_driver = {
>   	.gem_prime_export = radeon_gem_prime_export,
>   	.gem_prime_pin = radeon_gem_prime_pin,
>   	.gem_prime_unpin = radeon_gem_prime_unpin,
> -	.gem_prime_res_obj = radeon_gem_prime_res_obj,
>   	.gem_prime_get_sg_table = radeon_gem_prime_get_sg_table,
>   	.gem_prime_import_sg_table = radeon_gem_prime_import_sg_table,
>   	.gem_prime_vmap = radeon_gem_prime_vmap,
> diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
> index 21f73fc86f38..7a2bad843f8a 100644
> --- a/drivers/gpu/drm/radeon/radeon_object.c
> +++ b/drivers/gpu/drm/radeon/radeon_object.c
> @@ -262,6 +262,7 @@ int radeon_bo_create(struct radeon_device *rdev,
>   	r = ttm_bo_init(&rdev->mman.bdev, &bo->tbo, size, type,
>   			&bo->placement, page_align, !kernel, acc_size,
>   			sg, resv, &radeon_ttm_bo_destroy);
> +	bo->gem_base.resv = bo->tbo.resv;
>   	up_read(&rdev->pm.mclk_lock);
>   	if (unlikely(r != 0)) {
>   		return r;
> diff --git a/drivers/gpu/drm/radeon/radeon_prime.c b/drivers/gpu/drm/radeon/radeon_prime.c
> index deaffce50a2e..8ce3e8045d42 100644
> --- a/drivers/gpu/drm/radeon/radeon_prime.c
> +++ b/drivers/gpu/drm/radeon/radeon_prime.c
> @@ -117,13 +117,6 @@ void radeon_gem_prime_unpin(struct drm_gem_object *obj)
>   }
>   
>   
> -struct reservation_object *radeon_gem_prime_res_obj(struct drm_gem_object *obj)
> -{
> -	struct radeon_bo *bo = gem_to_radeon_bo(obj);
> -
> -	return bo->tbo.resv;
> -}
> -
>   struct dma_buf *radeon_gem_prime_export(struct drm_gem_object *gobj,
>   					int flags)
>   {

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 4/7] drm/radeon: Fill out gem_object->resv

[Daniel Vetter]

On Wed, Jun 26, 2019 at 07:10:21AM +0000, Koenig, Christian wrote:
> Those patches would become superfluous when merging Gerd's work.

Not entirely, they still remove the gem_prime_res_obj. Setting up
gem_bo.resv is only one half of what these do here. And yeah I think that
single addition can be removed again when Gerd's stuff lands.
 
> But I'm not sure if that is going to fly soon or not.

I think r-b from Thomas Zimmermann (or some other ttm+gem stakeholder) and
we're good to land them. Thomas Hellstrom mellowed down his "nack" to
"I'll look at this in August again and course-correct if necessary".
-Daniel

> 
> Christian.
> 
> Am 25.06.19 um 22:42 schrieb Daniel Vetter:
> > That way we can ditch our gem_prime_res_obj implementation. Since ttm
> > absolutely needs the right reservation object all the boilerplate is
> > already there and we just have to wire it up correctly.
> >
> > Note that gem/prime doesn't care when we do this, as long as we do it
> > before the bo is registered and someone can call the handle2fd ioctl
> > on it.
> >
> > Aside: ttm_buffer_object.ttm_resv could probably be ditched in favour
> > of always passing a non-NULL resv to ttm_bo_init(). At least for gem
> > drivers that would avoid having two of these, on in ttm_buffer_object
> > and the other in drm_gem_object, one just there for confusion.
> >
> > Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Alex Deucher <alexander.deucher@amd.com>
> > Cc: "Christian König" <christian.koenig@amd.com>
> > Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
> > Cc: amd-gfx@lists.freedesktop.org
> > ---
> >   drivers/gpu/drm/radeon/radeon_drv.c    | 2 --
> >   drivers/gpu/drm/radeon/radeon_object.c | 1 +
> >   drivers/gpu/drm/radeon/radeon_prime.c  | 7 -------
> >   3 files changed, 1 insertion(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
> > index 4403e76e1ae0..a4a78dfdef37 100644
> > --- a/drivers/gpu/drm/radeon/radeon_drv.c
> > +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> > @@ -152,7 +152,6 @@ struct drm_gem_object *radeon_gem_prime_import_sg_table(struct drm_device *dev,
> >   							struct sg_table *sg);
> >   int radeon_gem_prime_pin(struct drm_gem_object *obj);
> >   void radeon_gem_prime_unpin(struct drm_gem_object *obj);
> > -struct reservation_object *radeon_gem_prime_res_obj(struct drm_gem_object *);
> >   void *radeon_gem_prime_vmap(struct drm_gem_object *obj);
> >   void radeon_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr);
> >   
> > @@ -566,7 +565,6 @@ static struct drm_driver kms_driver = {
> >   	.gem_prime_export = radeon_gem_prime_export,
> >   	.gem_prime_pin = radeon_gem_prime_pin,
> >   	.gem_prime_unpin = radeon_gem_prime_unpin,
> > -	.gem_prime_res_obj = radeon_gem_prime_res_obj,
> >   	.gem_prime_get_sg_table = radeon_gem_prime_get_sg_table,
> >   	.gem_prime_import_sg_table = radeon_gem_prime_import_sg_table,
> >   	.gem_prime_vmap = radeon_gem_prime_vmap,
> > diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
> > index 21f73fc86f38..7a2bad843f8a 100644
> > --- a/drivers/gpu/drm/radeon/radeon_object.c
> > +++ b/drivers/gpu/drm/radeon/radeon_object.c
> > @@ -262,6 +262,7 @@ int radeon_bo_create(struct radeon_device *rdev,
> >   	r = ttm_bo_init(&rdev->mman.bdev, &bo->tbo, size, type,
> >   			&bo->placement, page_align, !kernel, acc_size,
> >   			sg, resv, &radeon_ttm_bo_destroy);
> > +	bo->gem_base.resv = bo->tbo.resv;
> >   	up_read(&rdev->pm.mclk_lock);
> >   	if (unlikely(r != 0)) {
> >   		return r;
> > diff --git a/drivers/gpu/drm/radeon/radeon_prime.c b/drivers/gpu/drm/radeon/radeon_prime.c
> > index deaffce50a2e..8ce3e8045d42 100644
> > --- a/drivers/gpu/drm/radeon/radeon_prime.c
> > +++ b/drivers/gpu/drm/radeon/radeon_prime.c
> > @@ -117,13 +117,6 @@ void radeon_gem_prime_unpin(struct drm_gem_object *obj)
> >   }
> >   
> >   
> > -struct reservation_object *radeon_gem_prime_res_obj(struct drm_gem_object *obj)
> > -{
> > -	struct radeon_bo *bo = gem_to_radeon_bo(obj);
> > -
> > -	return bo->tbo.resv;
> > -}
> > -
> >   struct dma_buf *radeon_gem_prime_export(struct drm_gem_object *gobj,
> >   					int flags)
> >   {
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 0/7] gem_bo.resv prime unification, leftovers

[Mike Lothian]

I'll try testing this on my Skylake/Tonga setup tonight

On Tue, 25 Jun 2019 at 21:42, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>
> Hi all,
>
> Here's the unmerged leftovers from my big prime cleanup series:
> - using the prepare_fb helper in vc4&msm, now hopefully fixed up. The
>   helper should be now even more useful.
>
> - amd&nv driver ->gem_prime_res_obj callback removal. I think this one
>   might have functional conflicts with Gerd's patch series to embed
>   drm_gem_object in ttm_bo, or at least needs to be re-reviewed before we
>   merge the 2nd series.
>
> Comments, testing, feedback as usual very much welcome.
>
> Thanks, Daniel
>
> Daniel Vetter (7):
>   drm/fb-helper: use gem_bo.resv, not dma_buf.resv in prepare_fb
>   drm/msm: Use drm_gem_fb_prepare_fb
>   drm/vc4: Use drm_gem_fb_prepare_fb
>   drm/radeon: Fill out gem_object->resv
>   drm/nouveau: Fill out gem_object->resv
>   drm/amdgpu: Fill out gem_object->resv
>   drm/prime: Ditch gem_prime_res_obj hook
>
>  Documentation/gpu/todo.rst                   |  9 ------
>  drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c  | 17 +-----------
>  drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h  |  1 -
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c      |  1 -
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c   |  2 ++
>  drivers/gpu/drm/drm_gem_framebuffer_helper.c | 29 ++++++++++----------
>  drivers/gpu/drm/drm_prime.c                  |  3 --
>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c    |  8 ++----
>  drivers/gpu/drm/msm/msm_atomic.c             |  8 ++----
>  drivers/gpu/drm/nouveau/nouveau_bo.c         |  2 ++
>  drivers/gpu/drm/nouveau/nouveau_drm.c        |  1 -
>  drivers/gpu/drm/nouveau/nouveau_gem.h        |  1 -
>  drivers/gpu/drm/nouveau/nouveau_prime.c      |  7 -----
>  drivers/gpu/drm/radeon/radeon_drv.c          |  2 --
>  drivers/gpu/drm/radeon/radeon_object.c       |  1 +
>  drivers/gpu/drm/radeon/radeon_prime.c        |  7 -----
>  drivers/gpu/drm/vc4/vc4_plane.c              |  5 ++--
>  include/drm/drm_drv.h                        | 12 --------
>  18 files changed, 26 insertions(+), 90 deletions(-)
>
> --
> 2.20.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/7] drm/radeon: Fill out gem_object->resv

[Daniel Vetter]

On Wed, Jun 26, 2019 at 10:23:12AM +0200, Daniel Vetter wrote:
> On Wed, Jun 26, 2019 at 07:10:21AM +0000, Koenig, Christian wrote:
> > Those patches would become superfluous when merging Gerd's work.
> 
> Not entirely, they still remove the gem_prime_res_obj. Setting up
> gem_bo.resv is only one half of what these do here. And yeah I think that
> single addition can be removed again when Gerd's stuff lands.
>  
> > But I'm not sure if that is going to fly soon or not.
> 
> I think r-b from Thomas Zimmermann (or some other ttm+gem stakeholder) and
> we're good to land them. Thomas Hellstrom mellowed down his "nack" to
> "I'll look at this in August again and course-correct if necessary".

Just pinged Gerd on this, so we can start coordination.

btw planning to review this from amd side, I'd like to ditch
gem_prime_res_obj especially with Gerd series it's pointless.
-Daniel

> -Daniel
> 
> > 
> > Christian.
> > 
> > Am 25.06.19 um 22:42 schrieb Daniel Vetter:
> > > That way we can ditch our gem_prime_res_obj implementation. Since ttm
> > > absolutely needs the right reservation object all the boilerplate is
> > > already there and we just have to wire it up correctly.
> > >
> > > Note that gem/prime doesn't care when we do this, as long as we do it
> > > before the bo is registered and someone can call the handle2fd ioctl
> > > on it.
> > >
> > > Aside: ttm_buffer_object.ttm_resv could probably be ditched in favour
> > > of always passing a non-NULL resv to ttm_bo_init(). At least for gem
> > > drivers that would avoid having two of these, on in ttm_buffer_object
> > > and the other in drm_gem_object, one just there for confusion.
> > >
> > > Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > Cc: Alex Deucher <alexander.deucher@amd.com>
> > > Cc: "Christian König" <christian.koenig@amd.com>
> > > Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
> > > Cc: amd-gfx@lists.freedesktop.org
> > > ---
> > >   drivers/gpu/drm/radeon/radeon_drv.c    | 2 --
> > >   drivers/gpu/drm/radeon/radeon_object.c | 1 +
> > >   drivers/gpu/drm/radeon/radeon_prime.c  | 7 -------
> > >   3 files changed, 1 insertion(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
> > > index 4403e76e1ae0..a4a78dfdef37 100644
> > > --- a/drivers/gpu/drm/radeon/radeon_drv.c
> > > +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> > > @@ -152,7 +152,6 @@ struct drm_gem_object *radeon_gem_prime_import_sg_table(struct drm_device *dev,
> > >   							struct sg_table *sg);
> > >   int radeon_gem_prime_pin(struct drm_gem_object *obj);
> > >   void radeon_gem_prime_unpin(struct drm_gem_object *obj);
> > > -struct reservation_object *radeon_gem_prime_res_obj(struct drm_gem_object *);
> > >   void *radeon_gem_prime_vmap(struct drm_gem_object *obj);
> > >   void radeon_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr);
> > >   
> > > @@ -566,7 +565,6 @@ static struct drm_driver kms_driver = {
> > >   	.gem_prime_export = radeon_gem_prime_export,
> > >   	.gem_prime_pin = radeon_gem_prime_pin,
> > >   	.gem_prime_unpin = radeon_gem_prime_unpin,
> > > -	.gem_prime_res_obj = radeon_gem_prime_res_obj,
> > >   	.gem_prime_get_sg_table = radeon_gem_prime_get_sg_table,
> > >   	.gem_prime_import_sg_table = radeon_gem_prime_import_sg_table,
> > >   	.gem_prime_vmap = radeon_gem_prime_vmap,
> > > diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
> > > index 21f73fc86f38..7a2bad843f8a 100644
> > > --- a/drivers/gpu/drm/radeon/radeon_object.c
> > > +++ b/drivers/gpu/drm/radeon/radeon_object.c
> > > @@ -262,6 +262,7 @@ int radeon_bo_create(struct radeon_device *rdev,
> > >   	r = ttm_bo_init(&rdev->mman.bdev, &bo->tbo, size, type,
> > >   			&bo->placement, page_align, !kernel, acc_size,
> > >   			sg, resv, &radeon_ttm_bo_destroy);
> > > +	bo->gem_base.resv = bo->tbo.resv;
> > >   	up_read(&rdev->pm.mclk_lock);
> > >   	if (unlikely(r != 0)) {
> > >   		return r;
> > > diff --git a/drivers/gpu/drm/radeon/radeon_prime.c b/drivers/gpu/drm/radeon/radeon_prime.c
> > > index deaffce50a2e..8ce3e8045d42 100644
> > > --- a/drivers/gpu/drm/radeon/radeon_prime.c
> > > +++ b/drivers/gpu/drm/radeon/radeon_prime.c
> > > @@ -117,13 +117,6 @@ void radeon_gem_prime_unpin(struct drm_gem_object *obj)
> > >   }
> > >   
> > >   
> > > -struct reservation_object *radeon_gem_prime_res_obj(struct drm_gem_object *obj)
> > > -{
> > > -	struct radeon_bo *bo = gem_to_radeon_bo(obj);
> > > -
> > > -	return bo->tbo.resv;
> > > -}
> > > -
> > >   struct dma_buf *radeon_gem_prime_export(struct drm_gem_object *gobj,
> > >   					int flags)
> > >   {
> > 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/7] drm/vc4: Use drm_gem_fb_prepare_fb

[Daniel Vetter]

On Tue, Jun 25, 2019 at 10:42:04PM +0200, Daniel Vetter wrote:
> vc4 has switched to using drm_fb->obj[], so we can just use the helper
> unchanged.
> 
> v2: Make it compile ... oops.
> 
> Cc: Eric Anholt <eric@anholt.net>
> Cc: Emil Velikov <emil.velikov@collabora.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Eric Anholt <eric@anholt.net>

Merged up to this one, thanks for the reviews.
-Daniel

> ---
>  drivers/gpu/drm/vc4/vc4_plane.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c
> index 0a0207c350a5..a996ca8ff972 100644
> --- a/drivers/gpu/drm/vc4/vc4_plane.c
> +++ b/drivers/gpu/drm/vc4/vc4_plane.c
> @@ -20,6 +20,7 @@
>  #include <drm/drm_fb_cma_helper.h>
>  #include <drm/drm_plane_helper.h>
>  #include <drm/drm_atomic_uapi.h>
> +#include <drm/drm_gem_framebuffer_helper.h>
>  
>  #include "uapi/drm/vc4_drm.h"
>  #include "vc4_drv.h"
> @@ -1123,7 +1124,6 @@ static int vc4_prepare_fb(struct drm_plane *plane,
>  			  struct drm_plane_state *state)
>  {
>  	struct vc4_bo *bo;
> -	struct dma_fence *fence;
>  	int ret;
>  
>  	if (!state->fb)
> @@ -1131,8 +1131,7 @@ static int vc4_prepare_fb(struct drm_plane *plane,
>  
>  	bo = to_vc4_bo(&drm_fb_cma_get_gem_obj(state->fb, 0)->base);
>  
> -	fence = reservation_object_get_excl_rcu(bo->base.base.resv);
> -	drm_atomic_set_fence_for_plane(state, fence);
> +	drm_gem_fb_prepare_fb(plane, state);
>  
>  	if (plane->state->fb == state->fb)
>  		return 0;
> -- 
> 2.20.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/7] gem_bo.resv prime unification, leftovers

[Gerd Hoffmann]

On Tue, Jun 25, 2019 at 10:42:01PM +0200, Daniel Vetter wrote:
> Hi all,
> 
> Here's the unmerged leftovers from my big prime cleanup series:
> - using the prepare_fb helper in vc4&msm, now hopefully fixed up. The
>   helper should be now even more useful.
> 
> - amd&nv driver ->gem_prime_res_obj callback removal. I think this one
>   might have functional conflicts with Gerd's patch series to embed
>   drm_gem_object in ttm_bo, or at least needs to be re-reviewed before we
>   merge the 2nd series.

Looks all sane to me.

Acked-by: Gerd Hoffmann <kraxel@redhat.com>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/7] drm/radeon: Fill out gem_object->resv

[Daniel Vetter]

On Thu, Jun 27, 2019 at 09:33:50AM +0200, Daniel Vetter wrote:
> On Wed, Jun 26, 2019 at 10:23:12AM +0200, Daniel Vetter wrote:
> > On Wed, Jun 26, 2019 at 07:10:21AM +0000, Koenig, Christian wrote:
> > > Those patches would become superfluous when merging Gerd's work.
> > 
> > Not entirely, they still remove the gem_prime_res_obj. Setting up
> > gem_bo.resv is only one half of what these do here. And yeah I think that
> > single addition can be removed again when Gerd's stuff lands.
> >  
> > > But I'm not sure if that is going to fly soon or not.
> > 
> > I think r-b from Thomas Zimmermann (or some other ttm+gem stakeholder) and
> > we're good to land them. Thomas Hellstrom mellowed down his "nack" to
> > "I'll look at this in August again and course-correct if necessary".
> 
> Just pinged Gerd on this, so we can start coordination.
> 
> btw planning to review this from amd side, I'd like to ditch
> gem_prime_res_obj especially with Gerd series it's pointless.

Gerd rebased his series on top of this, so review would be neat to unblock
everything. I'd like to get this all landed for 5.4.

Thanks, Daniel

> -Daniel
> 
> > -Daniel
> > 
> > > 
> > > Christian.
> > > 
> > > Am 25.06.19 um 22:42 schrieb Daniel Vetter:
> > > > That way we can ditch our gem_prime_res_obj implementation. Since ttm
> > > > absolutely needs the right reservation object all the boilerplate is
> > > > already there and we just have to wire it up correctly.
> > > >
> > > > Note that gem/prime doesn't care when we do this, as long as we do it
> > > > before the bo is registered and someone can call the handle2fd ioctl
> > > > on it.
> > > >
> > > > Aside: ttm_buffer_object.ttm_resv could probably be ditched in favour
> > > > of always passing a non-NULL resv to ttm_bo_init(). At least for gem
> > > > drivers that would avoid having two of these, on in ttm_buffer_object
> > > > and the other in drm_gem_object, one just there for confusion.
> > > >
> > > > Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
> > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > > Cc: Alex Deucher <alexander.deucher@amd.com>
> > > > Cc: "Christian König" <christian.koenig@amd.com>
> > > > Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
> > > > Cc: amd-gfx@lists.freedesktop.org
> > > > ---
> > > >   drivers/gpu/drm/radeon/radeon_drv.c    | 2 --
> > > >   drivers/gpu/drm/radeon/radeon_object.c | 1 +
> > > >   drivers/gpu/drm/radeon/radeon_prime.c  | 7 -------
> > > >   3 files changed, 1 insertion(+), 9 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
> > > > index 4403e76e1ae0..a4a78dfdef37 100644
> > > > --- a/drivers/gpu/drm/radeon/radeon_drv.c
> > > > +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> > > > @@ -152,7 +152,6 @@ struct drm_gem_object *radeon_gem_prime_import_sg_table(struct drm_device *dev,
> > > >   							struct sg_table *sg);
> > > >   int radeon_gem_prime_pin(struct drm_gem_object *obj);
> > > >   void radeon_gem_prime_unpin(struct drm_gem_object *obj);
> > > > -struct reservation_object *radeon_gem_prime_res_obj(struct drm_gem_object *);
> > > >   void *radeon_gem_prime_vmap(struct drm_gem_object *obj);
> > > >   void radeon_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr);
> > > >   
> > > > @@ -566,7 +565,6 @@ static struct drm_driver kms_driver = {
> > > >   	.gem_prime_export = radeon_gem_prime_export,
> > > >   	.gem_prime_pin = radeon_gem_prime_pin,
> > > >   	.gem_prime_unpin = radeon_gem_prime_unpin,
> > > > -	.gem_prime_res_obj = radeon_gem_prime_res_obj,
> > > >   	.gem_prime_get_sg_table = radeon_gem_prime_get_sg_table,
> > > >   	.gem_prime_import_sg_table = radeon_gem_prime_import_sg_table,
> > > >   	.gem_prime_vmap = radeon_gem_prime_vmap,
> > > > diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
> > > > index 21f73fc86f38..7a2bad843f8a 100644
> > > > --- a/drivers/gpu/drm/radeon/radeon_object.c
> > > > +++ b/drivers/gpu/drm/radeon/radeon_object.c
> > > > @@ -262,6 +262,7 @@ int radeon_bo_create(struct radeon_device *rdev,
> > > >   	r = ttm_bo_init(&rdev->mman.bdev, &bo->tbo, size, type,
> > > >   			&bo->placement, page_align, !kernel, acc_size,
> > > >   			sg, resv, &radeon_ttm_bo_destroy);
> > > > +	bo->gem_base.resv = bo->tbo.resv;
> > > >   	up_read(&rdev->pm.mclk_lock);
> > > >   	if (unlikely(r != 0)) {
> > > >   		return r;
> > > > diff --git a/drivers/gpu/drm/radeon/radeon_prime.c b/drivers/gpu/drm/radeon/radeon_prime.c
> > > > index deaffce50a2e..8ce3e8045d42 100644
> > > > --- a/drivers/gpu/drm/radeon/radeon_prime.c
> > > > +++ b/drivers/gpu/drm/radeon/radeon_prime.c
> > > > @@ -117,13 +117,6 @@ void radeon_gem_prime_unpin(struct drm_gem_object *obj)
> > > >   }
> > > >   
> > > >   
> > > > -struct reservation_object *radeon_gem_prime_res_obj(struct drm_gem_object *obj)
> > > > -{
> > > > -	struct radeon_bo *bo = gem_to_radeon_bo(obj);
> > > > -
> > > > -	return bo->tbo.resv;
> > > > -}
> > > > -
> > > >   struct dma_buf *radeon_gem_prime_export(struct drm_gem_object *gobj,
> > > >   					int flags)
> > > >   {
> > > 
> > 
> > -- 
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 4/7] drm/radeon: Fill out gem_object->resv

[Daniel Vetter]

On Tue, Jun 25, 2019 at 10:52:17PM +0200, Sam Ravnborg wrote:
> On Tue, Jun 25, 2019 at 10:42:05PM +0200, Daniel Vetter wrote:
> > That way we can ditch our gem_prime_res_obj implementation. Since ttm
> > absolutely needs the right reservation object all the boilerplate is
> > already there and we just have to wire it up correctly.
> > 
> > Note that gem/prime doesn't care when we do this, as long as we do it
> > before the bo is registered and someone can call the handle2fd ioctl
> > on it.
> > 
> > Aside: ttm_buffer_object.ttm_resv could probably be ditched in favour
> > of always passing a non-NULL resv to ttm_bo_init(). At least for gem
> > drivers that would avoid having two of these, on in ttm_buffer_object
> > and the other in drm_gem_object, one just there for confusion.
> Something for todo.rst - so this does not get lost in a changelog
> people will soon forget?

Gerd already has a nice patch series (rebased onto this one), so I don't
think we need a todo.rst for this.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel