Re: [PATCH 1/4] drm/gem-vram: handle NULL bo->resource in move callback

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 21/02/2023 16:17, Christian König wrote:
Am 21.02.23 um 17:13 schrieb Matthew Auld:
On 10/02/2023 11:03, Christian König wrote:
Am 08.02.23 um 15:53 schrieb Matthew Auld:
The ttm BO now initially has NULL bo->resource, and leaves the driver
the handle that. However it looks like we forgot to handle that for
ttm_bo_move_memcpy() users, like with vram-gem, since it just silently
returns zero. This seems to then trigger warnings like:

WARNING: CPU: 0 PID: 1 at drivers/gpu/drm/drm_gem_vram_helper.c:255 drm_gem_vram_offset (??:?)

Fix this by calling move_null() if the new resource is TTM_PL_SYSTEM,
otherwise do the multi-hop sequence to ensure can safely call into
ttm_bo_move_memcpy(), since it might also need to clear the memory.
This should give the same behaviour as before.

While we are here let's also treat calling ttm_bo_move_memcpy() with
NULL bo->resource as programmer error, where expectation is that upper
layers should now handle it.

Fixes: 180253782038 ("drm/ttm: stop allocating dummy resources during BO creation")
Signed-off-by: Matthew Auld <matthew.auld@xxxxxxxxx>
Cc: Christian König <christian.koenig@xxxxxxx>

Oh, I wasn't aware that this broke at so many places. Especially radeon was tested earlier in the development of the patch set.

Thanks for looking into that, the radeon patch has my rb and the rest of the series is Acked-by: Christian König <christian.koenig@xxxxxxx>.

Should we go ahead and land this? (minus patch 3 since that is already fixed by vmware folks).

Yeah, sure go ahead.

I assume this has to go via some drm-misc type tree, for which I don't currently have commit rights. Can you help with merging this?


Thanks,
Christian.



Regards,
Christian.

---
  drivers/gpu/drm/drm_gem_vram_helper.c | 11 +++++++++++
  drivers/gpu/drm/ttm/ttm_bo_util.c     |  4 ++--
  2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
index d40b3edb52d0..0bea3df2a16d 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -916,6 +916,17 @@ static int bo_driver_move(struct ttm_buffer_object *bo,
  {
      struct drm_gem_vram_object *gbo;
+    if (!bo->resource) {
+        if (new_mem->mem_type != TTM_PL_SYSTEM) {
+            hop->mem_type = TTM_PL_SYSTEM;
+            hop->flags = TTM_PL_FLAG_TEMPORARY;
+            return -EMULTIHOP;
+        }
+
+        ttm_bo_move_null(bo, new_mem);
+        return 0;
+    }
+
      gbo = drm_gem_vram_of_bo(bo);
      return drm_gem_vram_bo_driver_move(gbo, evict, ctx, new_mem);
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index d9d2b0903b22..fd9fd3d15101 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -157,8 +157,8 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo,
      bool clear;
      int ret = 0;
-    if (!src_mem)
-        return 0;
+    if (WARN_ON(!src_mem))
+        return -EINVAL;
      src_man = ttm_manager_type(bdev, src_mem->mem_type);
      if (ttm && ((ttm->page_flags & TTM_TT_FLAG_SWAPPED) ||





[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux