Re: [PATCH 01/10] drm/ttm: allocate resource object instead of embedding it v2
On 6/8/2021 11:42 AM, Christian König wrote: Am 08.06.21 um 11:40 schrieb Das, Nirmoy: On 6/8/2021 11:38 AM, Das, Nirmoy wrote: On 6/8/2021 9:21 AM, Christian König wrote: Am 08.06.21 um 09:17 schrieb Thomas Hellström (Intel): [SNIP] Do you have the log to double check? Unfortunately not, but IIRC it was directly from vmw_move(). Nirmoy do you still have your vmwgfx test environment? Yes! I will test this series on my vmwgfx setup. Since it is already pushed (and we fixed a bunch of its fallout) can you please test drm-misc-next instead? Sure! Nirmoy Thanks, Christian. Thanks, Christian. /Thomas
Re: [PATCH 01/10] drm/ttm: allocate resource object instead of embedding it v2
Am 08.06.21 um 11:40 schrieb Das, Nirmoy: On 6/8/2021 11:38 AM, Das, Nirmoy wrote: On 6/8/2021 9:21 AM, Christian König wrote: Am 08.06.21 um 09:17 schrieb Thomas Hellström (Intel): [SNIP] Do you have the log to double check? Unfortunately not, but IIRC it was directly from vmw_move(). Nirmoy do you still have your vmwgfx test environment? Yes! I will test this series on my vmwgfx setup. Since it is already pushed (and we fixed a bunch of its fallout) can you please test drm-misc-next instead? Thanks, Christian. Thanks, Christian. /Thomas
Re: [PATCH 01/10] drm/ttm: allocate resource object instead of embedding it v2
On 6/8/2021 11:38 AM, Das, Nirmoy wrote: On 6/8/2021 9:21 AM, Christian König wrote: Am 08.06.21 um 09:17 schrieb Thomas Hellström (Intel): [SNIP] Do you have the log to double check? Unfortunately not, but IIRC it was directly from vmw_move(). Nirmoy do you still have your vmwgfx test environment? Yes! I will test this series on my vmwgfx setup. Thanks, Christian. /Thomas
Re: [PATCH 01/10] drm/ttm: allocate resource object instead of embedding it v2
On 6/8/2021 9:21 AM, Christian König wrote: Am 08.06.21 um 09:17 schrieb Thomas Hellström (Intel): [SNIP] Do you have the log to double check? Unfortunately not, but IIRC it was directly from vmw_move(). Nirmoy do you still have your vmwgfx test environment? Yes! Thanks, Christian. /Thomas
Re: [PATCH 01/10] drm/ttm: allocate resource object instead of embedding it v2
Am 08.06.21 um 09:17 schrieb Thomas Hellström (Intel): [SNIP] Do you have the log to double check? Unfortunately not, but IIRC it was directly from vmw_move(). Nirmoy do you still have your vmwgfx test environment? Thanks, Christian. /Thomas
Re: [PATCH 01/10] drm/ttm: allocate resource object instead of embedding it v2
Hi,
On 6/8/21 9:14 AM, Christian König wrote:
Am 08.06.21 um 07:29 schrieb Thomas Hellström (Intel):
Hi,
On 6/7/21 7:59 PM, Christian König wrote:
Am 07.06.21 um 19:58 schrieb Thomas Hellström (Intel):
On 6/7/21 7:54 PM, Christian König wrote:
Am 07.06.21 um 19:06 schrieb Thomas Hellström (Intel):
On 6/7/21 6:40 PM, Thomas Hellström (Intel) wrote:
On 6/2/21 12:09 PM, Christian König wrote:
...
@@ -728,14 +728,15 @@ static int ttm_bo_add_move_fence(struct
ttm_buffer_object *bo,
*/
static int ttm_bo_mem_force_space(struct ttm_buffer_object *bo,
const struct ttm_place *place,
- struct ttm_resource *mem,
+ struct ttm_resource **mem,
struct ttm_operation_ctx *ctx)
{
struct ttm_device *bdev = bo->bdev;
- struct ttm_resource_manager *man = ttm_manager_type(bdev,
mem->mem_type);
+ struct ttm_resource_manager *man;
struct ww_acquire_ctx *ticket;
int ret;
+ man = ttm_manager_type(bdev, (*mem)->mem_type);
Isn't (*mem) uninitialized here? Should be place->mem_type?
Eviction is immediately sent to the bushes.
Got at least one additional NULL pointer deref to track down in
the eviction code, but could be a merge error of mine as well.
Actually this last one was probably due to a bad temporary fix of
the above one.
I've found one more warning during my testing now. But that is
just a false positive.
Apart from that I haven't seen any other fallout, but fingers
crossed.
vmwgfx doesn't seem to happy. It works AFAICT., but warns in
vmw_move() about ttm_bo_assign_mem() replacing an existing resource.
Yeah, that's the one I've just fixed. Patch is underway.
If that's the move_to_ghost patch, I don't think that would fix the
vmwgfx issue, as IIRC vmwgfx ever uses ghost objects.
Mhm, could be that vmwgfx is hitting the same warning with a different
backtrace.
Do you have the log to double check?
Unfortunately not, but IIRC it was directly from vmw_move().
/Thomas
Re: [PATCH 01/10] drm/ttm: allocate resource object instead of embedding it v2
Am 08.06.21 um 07:29 schrieb Thomas Hellström (Intel):
Hi,
On 6/7/21 7:59 PM, Christian König wrote:
Am 07.06.21 um 19:58 schrieb Thomas Hellström (Intel):
On 6/7/21 7:54 PM, Christian König wrote:
Am 07.06.21 um 19:06 schrieb Thomas Hellström (Intel):
On 6/7/21 6:40 PM, Thomas Hellström (Intel) wrote:
On 6/2/21 12:09 PM, Christian König wrote:
...
@@ -728,14 +728,15 @@ static int ttm_bo_add_move_fence(struct
ttm_buffer_object *bo,
*/
static int ttm_bo_mem_force_space(struct ttm_buffer_object *bo,
const struct ttm_place *place,
- struct ttm_resource *mem,
+ struct ttm_resource **mem,
struct ttm_operation_ctx *ctx)
{
struct ttm_device *bdev = bo->bdev;
- struct ttm_resource_manager *man = ttm_manager_type(bdev,
mem->mem_type);
+ struct ttm_resource_manager *man;
struct ww_acquire_ctx *ticket;
int ret;
+ man = ttm_manager_type(bdev, (*mem)->mem_type);
Isn't (*mem) uninitialized here? Should be place->mem_type?
Eviction is immediately sent to the bushes.
Got at least one additional NULL pointer deref to track down in
the eviction code, but could be a merge error of mine as well.
Actually this last one was probably due to a bad temporary fix of
the above one.
I've found one more warning during my testing now. But that is just
a false positive.
Apart from that I haven't seen any other fallout, but fingers crossed.
vmwgfx doesn't seem to happy. It works AFAICT., but warns in
vmw_move() about ttm_bo_assign_mem() replacing an existing resource.
Yeah, that's the one I've just fixed. Patch is underway.
If that's the move_to_ghost patch, I don't think that would fix the
vmwgfx issue, as IIRC vmwgfx ever uses ghost objects.
Mhm, could be that vmwgfx is hitting the same warning with a different
backtrace.
Do you have the log to double check?
Thanks,
Christian.
/Thomas
Re: [PATCH 01/10] drm/ttm: allocate resource object instead of embedding it v2
Hi, On 6/2/21 12:09 PM, Christian König wrote: To improve the handling we want the establish the resource object as base class for the backend allocations. v2: add missing error handling Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 4 +- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c| 54 +++--- drivers/gpu/drm/nouveau/nouveau_bo.c | 2 +- drivers/gpu/drm/radeon/radeon_ttm.c| 2 +- drivers/gpu/drm/ttm/ttm_bo.c | 83 -- drivers/gpu/drm/ttm/ttm_bo_util.c | 43 ++- drivers/gpu/drm/ttm/ttm_resource.c | 31 +--- drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 2 +- include/drm/ttm/ttm_bo_api.h | 1 - include/drm/ttm/ttm_bo_driver.h| 10 ++- include/drm/ttm/ttm_resource.h | 4 +- 11 files changed, 110 insertions(+), 126 deletions(-) ... @@ -629,7 +628,7 @@ static void ttm_bo_move_pipeline_evict(struct ttm_buffer_object *bo, } spin_unlock(>move_lock); - ttm_resource_free(bo, bo->resource); + ttm_resource_free(bo, >resource); dma_fence_put(bo->moving); bo->moving = dma_fence_get(fence); @@ -678,11 +677,11 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo) if (ret) ttm_bo_wait(bo, false, false); - ttm_resource_alloc(bo, _mem, bo->resource); + ret = ttm_resource_alloc(bo, _mem, >resource); bo->ttm = NULL; dma_resv_unlock(>base._resv); ttm_bo_put(ghost); - return 0; + return ret; Here we re-introduce a late point of failure, which I guess leaves the bo in an undefined state? Same thing with my optimization for the idle case. Needs fixing as soon as possible. /Thomas
Re: [PATCH 01/10] drm/ttm: allocate resource object instead of embedding it v2
Hi,
On 6/7/21 7:59 PM, Christian König wrote:
Am 07.06.21 um 19:58 schrieb Thomas Hellström (Intel):
On 6/7/21 7:54 PM, Christian König wrote:
Am 07.06.21 um 19:06 schrieb Thomas Hellström (Intel):
On 6/7/21 6:40 PM, Thomas Hellström (Intel) wrote:
On 6/2/21 12:09 PM, Christian König wrote:
...
@@ -728,14 +728,15 @@ static int ttm_bo_add_move_fence(struct
ttm_buffer_object *bo,
*/
static int ttm_bo_mem_force_space(struct ttm_buffer_object *bo,
const struct ttm_place *place,
- struct ttm_resource *mem,
+ struct ttm_resource **mem,
struct ttm_operation_ctx *ctx)
{
struct ttm_device *bdev = bo->bdev;
- struct ttm_resource_manager *man = ttm_manager_type(bdev,
mem->mem_type);
+ struct ttm_resource_manager *man;
struct ww_acquire_ctx *ticket;
int ret;
+ man = ttm_manager_type(bdev, (*mem)->mem_type);
Isn't (*mem) uninitialized here? Should be place->mem_type?
Eviction is immediately sent to the bushes.
Got at least one additional NULL pointer deref to track down in
the eviction code, but could be a merge error of mine as well.
Actually this last one was probably due to a bad temporary fix of
the above one.
I've found one more warning during my testing now. But that is just
a false positive.
Apart from that I haven't seen any other fallout, but fingers crossed.
vmwgfx doesn't seem to happy. It works AFAICT., but warns in
vmw_move() about ttm_bo_assign_mem() replacing an existing resource.
Yeah, that's the one I've just fixed. Patch is underway.
If that's the move_to_ghost patch, I don't think that would fix the
vmwgfx issue, as IIRC vmwgfx ever uses ghost objects.
/Thomas
Re: [PATCH 01/10] drm/ttm: allocate resource object instead of embedding it v2
Am 07.06.21 um 19:58 schrieb Thomas Hellström (Intel):
On 6/7/21 7:54 PM, Christian König wrote:
Am 07.06.21 um 19:06 schrieb Thomas Hellström (Intel):
On 6/7/21 6:40 PM, Thomas Hellström (Intel) wrote:
On 6/2/21 12:09 PM, Christian König wrote:
...
@@ -728,14 +728,15 @@ static int ttm_bo_add_move_fence(struct
ttm_buffer_object *bo,
*/
static int ttm_bo_mem_force_space(struct ttm_buffer_object *bo,
const struct ttm_place *place,
- struct ttm_resource *mem,
+ struct ttm_resource **mem,
struct ttm_operation_ctx *ctx)
{
struct ttm_device *bdev = bo->bdev;
- struct ttm_resource_manager *man = ttm_manager_type(bdev,
mem->mem_type);
+ struct ttm_resource_manager *man;
struct ww_acquire_ctx *ticket;
int ret;
+ man = ttm_manager_type(bdev, (*mem)->mem_type);
Isn't (*mem) uninitialized here? Should be place->mem_type?
Eviction is immediately sent to the bushes.
Got at least one additional NULL pointer deref to track down in the
eviction code, but could be a merge error of mine as well.
Actually this last one was probably due to a bad temporary fix of
the above one.
I've found one more warning during my testing now. But that is just a
false positive.
Apart from that I haven't seen any other fallout, but fingers crossed.
vmwgfx doesn't seem to happy. It works AFAICT., but warns in
vmw_move() about ttm_bo_assign_mem() replacing an existing resource.
Yeah, that's the one I've just fixed. Patch is underway.
Christian.
/Thomas
Christian.
/Thomas
Re: [PATCH 01/10] drm/ttm: allocate resource object instead of embedding it v2
On 6/7/21 7:54 PM, Christian König wrote:
Am 07.06.21 um 19:06 schrieb Thomas Hellström (Intel):
On 6/7/21 6:40 PM, Thomas Hellström (Intel) wrote:
On 6/2/21 12:09 PM, Christian König wrote:
...
@@ -728,14 +728,15 @@ static int ttm_bo_add_move_fence(struct
ttm_buffer_object *bo,
*/
static int ttm_bo_mem_force_space(struct ttm_buffer_object *bo,
const struct ttm_place *place,
- struct ttm_resource *mem,
+ struct ttm_resource **mem,
struct ttm_operation_ctx *ctx)
{
struct ttm_device *bdev = bo->bdev;
- struct ttm_resource_manager *man = ttm_manager_type(bdev,
mem->mem_type);
+ struct ttm_resource_manager *man;
struct ww_acquire_ctx *ticket;
int ret;
+ man = ttm_manager_type(bdev, (*mem)->mem_type);
Isn't (*mem) uninitialized here? Should be place->mem_type? Eviction
is immediately sent to the bushes.
Got at least one additional NULL pointer deref to track down in the
eviction code, but could be a merge error of mine as well.
Actually this last one was probably due to a bad temporary fix of the
above one.
I've found one more warning during my testing now. But that is just a
false positive.
Apart from that I haven't seen any other fallout, but fingers crossed.
vmwgfx doesn't seem to happy. It works AFAICT., but warns in vmw_move()
about ttm_bo_assign_mem() replacing an existing resource.
/Thomas
Christian.
/Thomas
Re: [PATCH 01/10] drm/ttm: allocate resource object instead of embedding it v2
Am 07.06.21 um 19:06 schrieb Thomas Hellström (Intel):
On 6/7/21 6:40 PM, Thomas Hellström (Intel) wrote:
On 6/2/21 12:09 PM, Christian König wrote:
...
@@ -728,14 +728,15 @@ static int ttm_bo_add_move_fence(struct
ttm_buffer_object *bo,
*/
static int ttm_bo_mem_force_space(struct ttm_buffer_object *bo,
const struct ttm_place *place,
- struct ttm_resource *mem,
+ struct ttm_resource **mem,
struct ttm_operation_ctx *ctx)
{
struct ttm_device *bdev = bo->bdev;
- struct ttm_resource_manager *man = ttm_manager_type(bdev,
mem->mem_type);
+ struct ttm_resource_manager *man;
struct ww_acquire_ctx *ticket;
int ret;
+ man = ttm_manager_type(bdev, (*mem)->mem_type);
Isn't (*mem) uninitialized here? Should be place->mem_type? Eviction
is immediately sent to the bushes.
Got at least one additional NULL pointer deref to track down in the
eviction code, but could be a merge error of mine as well.
Actually this last one was probably due to a bad temporary fix of the
above one.
I've found one more warning during my testing now. But that is just a
false positive.
Apart from that I haven't seen any other fallout, but fingers crossed.
Christian.
/Thomas
Re: [PATCH 01/10] drm/ttm: allocate resource object instead of embedding it v2
Am 07.06.21 um 18:40 schrieb Thomas Hellström (Intel):
On 6/2/21 12:09 PM, Christian König wrote:
...
@@ -728,14 +728,15 @@ static int ttm_bo_add_move_fence(struct
ttm_buffer_object *bo,
*/
static int ttm_bo_mem_force_space(struct ttm_buffer_object *bo,
const struct ttm_place *place,
- struct ttm_resource *mem,
+ struct ttm_resource **mem,
struct ttm_operation_ctx *ctx)
{
struct ttm_device *bdev = bo->bdev;
- struct ttm_resource_manager *man = ttm_manager_type(bdev,
mem->mem_type);
+ struct ttm_resource_manager *man;
struct ww_acquire_ctx *ticket;
int ret;
+ man = ttm_manager_type(bdev, (*mem)->mem_type);
Isn't (*mem) uninitialized here? Should be place->mem_type? Eviction
is immediately sent to the bushes.
Indeed and I'm like 99% sure that Nirmoy has pointed that out to me as well.
Looks like I missed to fix that one. We need more CI testing.
Thanks,
Christian.
Got at least one additional NULL pointer deref to track down in the
eviction code, but could be a merge error of mine as well.
/Thomas
Re: [PATCH 01/10] drm/ttm: allocate resource object instead of embedding it v2
On 6/7/21 6:40 PM, Thomas Hellström (Intel) wrote:
On 6/2/21 12:09 PM, Christian König wrote:
...
@@ -728,14 +728,15 @@ static int ttm_bo_add_move_fence(struct
ttm_buffer_object *bo,
*/
static int ttm_bo_mem_force_space(struct ttm_buffer_object *bo,
const struct ttm_place *place,
- struct ttm_resource *mem,
+ struct ttm_resource **mem,
struct ttm_operation_ctx *ctx)
{
struct ttm_device *bdev = bo->bdev;
- struct ttm_resource_manager *man = ttm_manager_type(bdev,
mem->mem_type);
+ struct ttm_resource_manager *man;
struct ww_acquire_ctx *ticket;
int ret;
+ man = ttm_manager_type(bdev, (*mem)->mem_type);
Isn't (*mem) uninitialized here? Should be place->mem_type? Eviction
is immediately sent to the bushes.
Got at least one additional NULL pointer deref to track down in the
eviction code, but could be a merge error of mine as well.
Actually this last one was probably due to a bad temporary fix of the
above one.
/Thomas
Re: [PATCH 01/10] drm/ttm: allocate resource object instead of embedding it v2
On 6/2/21 12:09 PM, Christian König wrote:
...
@@ -728,14 +728,15 @@ static int ttm_bo_add_move_fence(struct ttm_buffer_object
*bo,
*/
static int ttm_bo_mem_force_space(struct ttm_buffer_object *bo,
const struct ttm_place *place,
- struct ttm_resource *mem,
+ struct ttm_resource **mem,
struct ttm_operation_ctx *ctx)
{
struct ttm_device *bdev = bo->bdev;
- struct ttm_resource_manager *man = ttm_manager_type(bdev,
mem->mem_type);
+ struct ttm_resource_manager *man;
struct ww_acquire_ctx *ticket;
int ret;
+ man = ttm_manager_type(bdev, (*mem)->mem_type);
Isn't (*mem) uninitialized here? Should be place->mem_type? Eviction is
immediately sent to the bushes.
Got at least one additional NULL pointer deref to track down in the
eviction code, but could be a merge error of mine as well.
/Thomas
Re: [PATCH 01/10] drm/ttm: allocate resource object instead of embedding it v2
Am 03.06.21 um 10:45 schrieb Matthew Auld:
On 02/06/2021 11:09, Christian König wrote:
[SNIP]
-/**
- * ttm_bo_mem_placement - check if placement is compatible
- * @bo: BO to find memory for
- * @place: where to search
- * @mem: the memory object to fill in
- *
- * Check if placement is compatible and fill in mem structure.
- * Returns -EBUSY if placement won't work or negative error code.
- * 0 when placement can be used.
- */
-static int ttm_bo_mem_placement(struct ttm_buffer_object *bo,
- const struct ttm_place *place,
- struct ttm_resource *mem)
-{
- struct ttm_device *bdev = bo->bdev;
- struct ttm_resource_manager *man;
-
- man = ttm_manager_type(bdev, place->mem_type);
- if (!man || !ttm_resource_manager_used(man))
- return -EBUSY;
-
- mem->mem_type = place->mem_type;
- mem->placement = place->flags;
-
- spin_lock(>bdev->lru_lock);
- ttm_bo_move_to_lru_tail(bo, mem, NULL);
- spin_unlock(>bdev->lru_lock);
Why do we drop the move_to_lru_tail here?
Ah, good point.
The move_to_lru_tail() was here to make sure we see the BO in the new
LRU instead of the old one before actually doing the move.
Since we haven't allocated the mem structure at this point that is no
longer possible, but I think it is ok to do this for now.
One motivation of doing this is to move the LRU handling into the
resource backend, so that tricks like those are not needed any more.
Regards,
Christian.
Re: [PATCH 01/10] drm/ttm: allocate resource object instead of embedding it v2
Hi, Christian,
It looks like all patches in the series have been reviewed or acked by
Matthew,
and while still a little worried about the final outcome of embedding a
struct ttm_mem_resource, FWIW,
Acked-by: Thomas Hellström
/Thomas
On 6/2/21 12:09 PM, Christian König wrote:
To improve the handling we want the establish the resource object as base
class for the backend allocations.
v2: add missing error handling
Signed-off-by: Christian König
---
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 4 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c| 54 +++---
drivers/gpu/drm/nouveau/nouveau_bo.c | 2 +-
drivers/gpu/drm/radeon/radeon_ttm.c| 2 +-
drivers/gpu/drm/ttm/ttm_bo.c | 83 --
drivers/gpu/drm/ttm/ttm_bo_util.c | 43 ++-
drivers/gpu/drm/ttm/ttm_resource.c | 31 +---
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 2 +-
include/drm/ttm/ttm_bo_api.h | 1 -
include/drm/ttm/ttm_bo_driver.h| 10 ++-
include/drm/ttm/ttm_resource.h | 4 +-
11 files changed, 110 insertions(+), 126 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 03c6b63d1d54..59723c3d5826 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -362,14 +362,14 @@ int amdgpu_bo_create_kernel_at(struct amdgpu_device *adev,
if (cpu_addr)
amdgpu_bo_kunmap(*bo_ptr);
- ttm_resource_free(&(*bo_ptr)->tbo, (*bo_ptr)->tbo.resource);
+ ttm_resource_free(&(*bo_ptr)->tbo, &(*bo_ptr)->tbo.resource);
for (i = 0; i < (*bo_ptr)->placement.num_placement; ++i) {
(*bo_ptr)->placements[i].fpfn = offset >> PAGE_SHIFT;
(*bo_ptr)->placements[i].lpfn = (offset + size) >> PAGE_SHIFT;
}
r = ttm_bo_mem_space(&(*bo_ptr)->tbo, &(*bo_ptr)->placement,
-(*bo_ptr)->tbo.resource, );
+&(*bo_ptr)->tbo.resource, );
if (r)
goto error;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 663aa7d2e2ea..69db89261650 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -491,7 +491,7 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo,
bool evict,
return r;
amdgpu_ttm_backend_unbind(bo->bdev, bo->ttm);
- ttm_resource_free(bo, bo->resource);
+ ttm_resource_free(bo, >resource);
ttm_bo_assign_mem(bo, new_mem);
goto out;
}
@@ -950,9 +950,9 @@ int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo)
struct amdgpu_device *adev = amdgpu_ttm_adev(bo->bdev);
struct ttm_operation_ctx ctx = { false, false };
struct amdgpu_ttm_tt *gtt = (void *)bo->ttm;
- struct ttm_resource tmp;
struct ttm_placement placement;
struct ttm_place placements;
+ struct ttm_resource *tmp;
uint64_t addr, flags;
int r;
@@ -962,37 +962,37 @@ int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo)
addr = amdgpu_gmc_agp_addr(bo);
if (addr != AMDGPU_BO_INVALID_OFFSET) {
bo->resource->start = addr >> PAGE_SHIFT;
- } else {
+ return 0;
+ }
- /* allocate GART space */
- placement.num_placement = 1;
- placement.placement =
- placement.num_busy_placement = 1;
- placement.busy_placement =
- placements.fpfn = 0;
- placements.lpfn = adev->gmc.gart_size >> PAGE_SHIFT;
- placements.mem_type = TTM_PL_TT;
- placements.flags = bo->resource->placement;
-
- r = ttm_bo_mem_space(bo, , , );
- if (unlikely(r))
- return r;
+ /* allocate GART space */
+ placement.num_placement = 1;
+ placement.placement =
+ placement.num_busy_placement = 1;
+ placement.busy_placement =
+ placements.fpfn = 0;
+ placements.lpfn = adev->gmc.gart_size >> PAGE_SHIFT;
+ placements.mem_type = TTM_PL_TT;
+ placements.flags = bo->resource->placement;
- /* compute PTE flags for this buffer object */
- flags = amdgpu_ttm_tt_pte_flags(adev, bo->ttm, );
+ r = ttm_bo_mem_space(bo, , , );
+ if (unlikely(r))
+ return r;
- /* Bind pages */
- gtt->offset = (u64)tmp.start << PAGE_SHIFT;
- r = amdgpu_ttm_gart_bind(adev, bo, flags);
- if (unlikely(r)) {
- ttm_resource_free(bo, );
- return r;
- }
+ /* compute PTE flags for this buffer object */
+ flags = amdgpu_ttm_tt_pte_flags(adev, bo->ttm, tmp);
- ttm_resource_free(bo, bo->resource);
-
Re: [PATCH 01/10] drm/ttm: allocate resource object instead of embedding it v2
On 02/06/2021 11:09, Christian König wrote:
To improve the handling we want the establish the resource object as base
class for the backend allocations.
v2: add missing error handling
Signed-off-by: Christian König
---
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 4 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c| 54 +++---
drivers/gpu/drm/nouveau/nouveau_bo.c | 2 +-
drivers/gpu/drm/radeon/radeon_ttm.c| 2 +-
drivers/gpu/drm/ttm/ttm_bo.c | 83 --
drivers/gpu/drm/ttm/ttm_bo_util.c | 43 ++-
drivers/gpu/drm/ttm/ttm_resource.c | 31 +---
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 2 +-
include/drm/ttm/ttm_bo_api.h | 1 -
include/drm/ttm/ttm_bo_driver.h| 10 ++-
include/drm/ttm/ttm_resource.h | 4 +-
11 files changed, 110 insertions(+), 126 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 03c6b63d1d54..59723c3d5826 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -362,14 +362,14 @@ int amdgpu_bo_create_kernel_at(struct amdgpu_device *adev,
if (cpu_addr)
amdgpu_bo_kunmap(*bo_ptr);
- ttm_resource_free(&(*bo_ptr)->tbo, (*bo_ptr)->tbo.resource);
+ ttm_resource_free(&(*bo_ptr)->tbo, &(*bo_ptr)->tbo.resource);
for (i = 0; i < (*bo_ptr)->placement.num_placement; ++i) {
(*bo_ptr)->placements[i].fpfn = offset >> PAGE_SHIFT;
(*bo_ptr)->placements[i].lpfn = (offset + size) >> PAGE_SHIFT;
}
r = ttm_bo_mem_space(&(*bo_ptr)->tbo, &(*bo_ptr)->placement,
-(*bo_ptr)->tbo.resource, );
+&(*bo_ptr)->tbo.resource, );
if (r)
goto error;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 663aa7d2e2ea..69db89261650 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -491,7 +491,7 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo,
bool evict,
return r;
amdgpu_ttm_backend_unbind(bo->bdev, bo->ttm);
- ttm_resource_free(bo, bo->resource);
+ ttm_resource_free(bo, >resource);
ttm_bo_assign_mem(bo, new_mem);
goto out;
}
@@ -950,9 +950,9 @@ int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo)
struct amdgpu_device *adev = amdgpu_ttm_adev(bo->bdev);
struct ttm_operation_ctx ctx = { false, false };
struct amdgpu_ttm_tt *gtt = (void *)bo->ttm;
- struct ttm_resource tmp;
struct ttm_placement placement;
struct ttm_place placements;
+ struct ttm_resource *tmp;
uint64_t addr, flags;
int r;
@@ -962,37 +962,37 @@ int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo)
addr = amdgpu_gmc_agp_addr(bo);
if (addr != AMDGPU_BO_INVALID_OFFSET) {
bo->resource->start = addr >> PAGE_SHIFT;
- } else {
+ return 0;
+ }
- /* allocate GART space */
- placement.num_placement = 1;
- placement.placement =
- placement.num_busy_placement = 1;
- placement.busy_placement =
- placements.fpfn = 0;
- placements.lpfn = adev->gmc.gart_size >> PAGE_SHIFT;
- placements.mem_type = TTM_PL_TT;
- placements.flags = bo->resource->placement;
-
- r = ttm_bo_mem_space(bo, , , );
- if (unlikely(r))
- return r;
+ /* allocate GART space */
+ placement.num_placement = 1;
+ placement.placement =
+ placement.num_busy_placement = 1;
+ placement.busy_placement =
+ placements.fpfn = 0;
+ placements.lpfn = adev->gmc.gart_size >> PAGE_SHIFT;
+ placements.mem_type = TTM_PL_TT;
+ placements.flags = bo->resource->placement;
- /* compute PTE flags for this buffer object */
- flags = amdgpu_ttm_tt_pte_flags(adev, bo->ttm, );
+ r = ttm_bo_mem_space(bo, , , );
+ if (unlikely(r))
+ return r;
- /* Bind pages */
- gtt->offset = (u64)tmp.start << PAGE_SHIFT;
- r = amdgpu_ttm_gart_bind(adev, bo, flags);
- if (unlikely(r)) {
- ttm_resource_free(bo, );
- return r;
- }
+ /* compute PTE flags for this buffer object */
+ flags = amdgpu_ttm_tt_pte_flags(adev, bo->ttm, tmp);
- ttm_resource_free(bo, bo->resource);
- ttm_bo_assign_mem(bo, );
+ /* Bind pages */
+ gtt->offset = (u64)tmp->start << PAGE_SHIFT;
+ r = amdgpu_ttm_gart_bind(adev, bo, flags);
+ if (unlikely(r)) {
+ ttm_resource_free(bo, );
+
