| Message ID | 20210430092508.60710-2-christian.koenig@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/13] drm/ttm: add ttm_sys_manager v2 | expand |
On Fri, 30 Apr 2021 at 10:25, Christian König <ckoenig.leichtzumerken@gmail.com> wrote: > > Init all fields in ttm_resource_alloc() when we create a new resource. > > Signed-off-by: Christian König <christian.koenig@amd.com> > Reviewed-by: Matthew Auld <matthew.auld@intel.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 2 -- > drivers/gpu/drm/ttm/ttm_bo.c | 26 ++++--------------------- > drivers/gpu/drm/ttm/ttm_bo_util.c | 4 ++-- > drivers/gpu/drm/ttm/ttm_resource.c | 9 +++++++++ > 4 files changed, 15 insertions(+), 26 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > index 7ba761e833ba..2f7580d2ee71 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > @@ -1015,8 +1015,6 @@ int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo) > } else { > > /* allocate GART space */ > - tmp = bo->mem; > - tmp.mm_node = NULL; > placement.num_placement = 1; > placement.placement = &placements; > placement.num_busy_placement = 1; > diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c > index df63a07a70de..55f1ddcf22b6 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo.c > +++ b/drivers/gpu/drm/ttm/ttm_bo.c > @@ -507,11 +507,6 @@ static int ttm_bo_evict(struct ttm_buffer_object *bo, > return ttm_tt_create(bo, false); > } > > - evict_mem = bo->mem; > - evict_mem.mm_node = NULL; > - evict_mem.bus.offset = 0; > - evict_mem.bus.addr = NULL; > - > ret = ttm_bo_mem_space(bo, &placement, &evict_mem, ctx); > if (ret) { > if (ret != -ERESTARTSYS) { > @@ -867,12 +862,8 @@ static int ttm_bo_bounce_temp_buffer(struct ttm_buffer_object *bo, > struct ttm_place *hop) > { > struct ttm_placement hop_placement; > + struct ttm_resource hop_mem; > int ret; > - struct ttm_resource hop_mem = *mem; > - > - hop_mem.mm_node = NULL; > - hop_mem.mem_type = TTM_PL_SYSTEM; > - hop_mem.placement = 0; > > hop_placement.num_placement = hop_placement.num_busy_placement = 1; > hop_placement.placement = hop_placement.busy_placement = hop; > @@ -894,19 +885,14 @@ static int ttm_bo_move_buffer(struct ttm_buffer_object *bo, > struct ttm_placement *placement, > struct ttm_operation_ctx *ctx) > { > - int ret = 0; > struct ttm_place hop; > struct ttm_resource mem; > + int ret; > > dma_resv_assert_held(bo->base.resv); > > memset(&hop, 0, sizeof(hop)); > > - mem.num_pages = PAGE_ALIGN(bo->base.size) >> PAGE_SHIFT; > - mem.bus.offset = 0; > - mem.bus.addr = NULL; > - mem.mm_node = NULL; > - > /* > * Determine where to move the buffer. > * > @@ -1027,6 +1013,7 @@ int ttm_bo_init_reserved(struct ttm_device *bdev, > struct dma_resv *resv, > void (*destroy) (struct ttm_buffer_object *)) > { > + static const struct ttm_place sys_mem = { .mem_type = TTM_PL_SYSTEM }; > bool locked; > int ret = 0; > > @@ -1038,13 +1025,8 @@ int ttm_bo_init_reserved(struct ttm_device *bdev, > bo->bdev = bdev; > bo->type = type; > bo->page_alignment = page_alignment; > - bo->mem.mem_type = TTM_PL_SYSTEM; > - bo->mem.num_pages = PAGE_ALIGN(size) >> PAGE_SHIFT; > - bo->mem.mm_node = NULL; > - bo->mem.bus.offset = 0; > - bo->mem.bus.addr = NULL; > + ttm_resource_alloc(bo, &sys_mem, &bo->mem); > bo->moving = NULL; > - bo->mem.placement = 0; > bo->pin_count = 0; > bo->sg = sg; > if (resv) { > diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c > index efb7e9c34ab4..ae8b61460724 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo_util.c > +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c > @@ -664,6 +664,7 @@ EXPORT_SYMBOL(ttm_bo_move_accel_cleanup); > > int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo) > { > + static const struct ttm_place sys_mem = { .mem_type = TTM_PL_SYSTEM }; > struct ttm_buffer_object *ghost; > int ret; > > @@ -676,8 +677,7 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo) > if (ret) > ttm_bo_wait(bo, false, false); > > - memset(&bo->mem, 0, sizeof(bo->mem)); > - bo->mem.mem_type = TTM_PL_SYSTEM; > + ttm_resource_alloc(bo, &sys_mem, &bo->mem); > bo->ttm = NULL; > > dma_resv_unlock(&ghost->base._resv); > diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c > index fc351700d035..cec2e6fb1c52 100644 > --- a/drivers/gpu/drm/ttm/ttm_resource.c > +++ b/drivers/gpu/drm/ttm/ttm_resource.c > @@ -33,6 +33,15 @@ int ttm_resource_alloc(struct ttm_buffer_object *bo, > ttm_manager_type(bo->bdev, res->mem_type); Hmm, should this not be place->mem_type, when fishing out the man? For example in the above pipeline_gutting case we previously nuked the bo->mem and then set the mem_type as SYS, but we dropped that now so what even is the value of res->mem_type here for that case? > > res->mm_node = NULL; > + res->start = 0; > + res->num_pages = PFN_UP(bo->base.size); > + res->mem_type = place->mem_type; > + res->placement = place->flags; > + res->bus.addr = NULL; > + res->bus.offset = 0; > + res->bus.is_iomem = false; > + res->bus.caching = ttm_cached; > + > return man->func->alloc(man, bo, place, res); > } > > -- > 2.25.1 >
Am 30.04.21 um 14:05 schrieb Matthew Auld: > On Fri, 30 Apr 2021 at 10:25, Christian König > <ckoenig.leichtzumerken@gmail.com> wrote: >> Init all fields in ttm_resource_alloc() when we create a new resource. >> >> Signed-off-by: Christian König <christian.koenig@amd.com> >> Reviewed-by: Matthew Auld <matthew.auld@intel.com> >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 2 -- >> drivers/gpu/drm/ttm/ttm_bo.c | 26 ++++--------------------- >> drivers/gpu/drm/ttm/ttm_bo_util.c | 4 ++-- >> drivers/gpu/drm/ttm/ttm_resource.c | 9 +++++++++ >> 4 files changed, 15 insertions(+), 26 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c >> index 7ba761e833ba..2f7580d2ee71 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c >> @@ -1015,8 +1015,6 @@ int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo) >> } else { >> >> /* allocate GART space */ >> - tmp = bo->mem; >> - tmp.mm_node = NULL; >> placement.num_placement = 1; >> placement.placement = &placements; >> placement.num_busy_placement = 1; >> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c >> index df63a07a70de..55f1ddcf22b6 100644 >> --- a/drivers/gpu/drm/ttm/ttm_bo.c >> +++ b/drivers/gpu/drm/ttm/ttm_bo.c >> @@ -507,11 +507,6 @@ static int ttm_bo_evict(struct ttm_buffer_object *bo, >> return ttm_tt_create(bo, false); >> } >> >> - evict_mem = bo->mem; >> - evict_mem.mm_node = NULL; >> - evict_mem.bus.offset = 0; >> - evict_mem.bus.addr = NULL; >> - >> ret = ttm_bo_mem_space(bo, &placement, &evict_mem, ctx); >> if (ret) { >> if (ret != -ERESTARTSYS) { >> @@ -867,12 +862,8 @@ static int ttm_bo_bounce_temp_buffer(struct ttm_buffer_object *bo, >> struct ttm_place *hop) >> { >> struct ttm_placement hop_placement; >> + struct ttm_resource hop_mem; >> int ret; >> - struct ttm_resource hop_mem = *mem; >> - >> - hop_mem.mm_node = NULL; >> - hop_mem.mem_type = TTM_PL_SYSTEM; >> - hop_mem.placement = 0; >> >> hop_placement.num_placement = hop_placement.num_busy_placement = 1; >> hop_placement.placement = hop_placement.busy_placement = hop; >> @@ -894,19 +885,14 @@ static int ttm_bo_move_buffer(struct ttm_buffer_object *bo, >> struct ttm_placement *placement, >> struct ttm_operation_ctx *ctx) >> { >> - int ret = 0; >> struct ttm_place hop; >> struct ttm_resource mem; >> + int ret; >> >> dma_resv_assert_held(bo->base.resv); >> >> memset(&hop, 0, sizeof(hop)); >> >> - mem.num_pages = PAGE_ALIGN(bo->base.size) >> PAGE_SHIFT; >> - mem.bus.offset = 0; >> - mem.bus.addr = NULL; >> - mem.mm_node = NULL; >> - >> /* >> * Determine where to move the buffer. >> * >> @@ -1027,6 +1013,7 @@ int ttm_bo_init_reserved(struct ttm_device *bdev, >> struct dma_resv *resv, >> void (*destroy) (struct ttm_buffer_object *)) >> { >> + static const struct ttm_place sys_mem = { .mem_type = TTM_PL_SYSTEM }; >> bool locked; >> int ret = 0; >> >> @@ -1038,13 +1025,8 @@ int ttm_bo_init_reserved(struct ttm_device *bdev, >> bo->bdev = bdev; >> bo->type = type; >> bo->page_alignment = page_alignment; >> - bo->mem.mem_type = TTM_PL_SYSTEM; >> - bo->mem.num_pages = PAGE_ALIGN(size) >> PAGE_SHIFT; >> - bo->mem.mm_node = NULL; >> - bo->mem.bus.offset = 0; >> - bo->mem.bus.addr = NULL; >> + ttm_resource_alloc(bo, &sys_mem, &bo->mem); >> bo->moving = NULL; >> - bo->mem.placement = 0; >> bo->pin_count = 0; >> bo->sg = sg; >> if (resv) { >> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c >> index efb7e9c34ab4..ae8b61460724 100644 >> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c >> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c >> @@ -664,6 +664,7 @@ EXPORT_SYMBOL(ttm_bo_move_accel_cleanup); >> >> int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo) >> { >> + static const struct ttm_place sys_mem = { .mem_type = TTM_PL_SYSTEM }; >> struct ttm_buffer_object *ghost; >> int ret; >> >> @@ -676,8 +677,7 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo) >> if (ret) >> ttm_bo_wait(bo, false, false); >> >> - memset(&bo->mem, 0, sizeof(bo->mem)); >> - bo->mem.mem_type = TTM_PL_SYSTEM; >> + ttm_resource_alloc(bo, &sys_mem, &bo->mem); >> bo->ttm = NULL; >> >> dma_resv_unlock(&ghost->base._resv); >> diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c >> index fc351700d035..cec2e6fb1c52 100644 >> --- a/drivers/gpu/drm/ttm/ttm_resource.c >> +++ b/drivers/gpu/drm/ttm/ttm_resource.c >> @@ -33,6 +33,15 @@ int ttm_resource_alloc(struct ttm_buffer_object *bo, >> ttm_manager_type(bo->bdev, res->mem_type); > Hmm, should this not be place->mem_type, when fishing out the man? > > For example in the above pipeline_gutting case we previously nuked the > bo->mem and then set the mem_type as SYS, but we dropped that now so > what even is the value of res->mem_type here for that case? Good point, it's overwritten with the follow up patch but would be good to have that fixed here as well. Christian. > >> res->mm_node = NULL; >> + res->start = 0; >> + res->num_pages = PFN_UP(bo->base.size); >> + res->mem_type = place->mem_type; >> + res->placement = place->flags; >> + res->bus.addr = NULL; >> + res->bus.offset = 0; >> + res->bus.is_iomem = false; >> + res->bus.caching = ttm_cached; >> + >> return man->func->alloc(man, bo, place, res); >> } >> >> -- >> 2.25.1 >>
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index 7ba761e833ba..2f7580d2ee71 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -1015,8 +1015,6 @@ int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo) } else { /* allocate GART space */ - tmp = bo->mem; - tmp.mm_node = NULL; placement.num_placement = 1; placement.placement = &placements; placement.num_busy_placement = 1; diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index df63a07a70de..55f1ddcf22b6 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -507,11 +507,6 @@ static int ttm_bo_evict(struct ttm_buffer_object *bo, return ttm_tt_create(bo, false); } - evict_mem = bo->mem; - evict_mem.mm_node = NULL; - evict_mem.bus.offset = 0; - evict_mem.bus.addr = NULL; - ret = ttm_bo_mem_space(bo, &placement, &evict_mem, ctx); if (ret) { if (ret != -ERESTARTSYS) { @@ -867,12 +862,8 @@ static int ttm_bo_bounce_temp_buffer(struct ttm_buffer_object *bo, struct ttm_place *hop) { struct ttm_placement hop_placement; + struct ttm_resource hop_mem; int ret; - struct ttm_resource hop_mem = *mem; - - hop_mem.mm_node = NULL; - hop_mem.mem_type = TTM_PL_SYSTEM; - hop_mem.placement = 0; hop_placement.num_placement = hop_placement.num_busy_placement = 1; hop_placement.placement = hop_placement.busy_placement = hop; @@ -894,19 +885,14 @@ static int ttm_bo_move_buffer(struct ttm_buffer_object *bo, struct ttm_placement *placement, struct ttm_operation_ctx *ctx) { - int ret = 0; struct ttm_place hop; struct ttm_resource mem; + int ret; dma_resv_assert_held(bo->base.resv); memset(&hop, 0, sizeof(hop)); - mem.num_pages = PAGE_ALIGN(bo->base.size) >> PAGE_SHIFT; - mem.bus.offset = 0; - mem.bus.addr = NULL; - mem.mm_node = NULL; - /* * Determine where to move the buffer. * @@ -1027,6 +1013,7 @@ int ttm_bo_init_reserved(struct ttm_device *bdev, struct dma_resv *resv, void (*destroy) (struct ttm_buffer_object *)) { + static const struct ttm_place sys_mem = { .mem_type = TTM_PL_SYSTEM }; bool locked; int ret = 0; @@ -1038,13 +1025,8 @@ int ttm_bo_init_reserved(struct ttm_device *bdev, bo->bdev = bdev; bo->type = type; bo->page_alignment = page_alignment; - bo->mem.mem_type = TTM_PL_SYSTEM; - bo->mem.num_pages = PAGE_ALIGN(size) >> PAGE_SHIFT; - bo->mem.mm_node = NULL; - bo->mem.bus.offset = 0; - bo->mem.bus.addr = NULL; + ttm_resource_alloc(bo, &sys_mem, &bo->mem); bo->moving = NULL; - bo->mem.placement = 0; bo->pin_count = 0; bo->sg = sg; if (resv) { diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c index efb7e9c34ab4..ae8b61460724 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_util.c +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c @@ -664,6 +664,7 @@ EXPORT_SYMBOL(ttm_bo_move_accel_cleanup); int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo) { + static const struct ttm_place sys_mem = { .mem_type = TTM_PL_SYSTEM }; struct ttm_buffer_object *ghost; int ret; @@ -676,8 +677,7 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo) if (ret) ttm_bo_wait(bo, false, false); - memset(&bo->mem, 0, sizeof(bo->mem)); - bo->mem.mem_type = TTM_PL_SYSTEM; + ttm_resource_alloc(bo, &sys_mem, &bo->mem); bo->ttm = NULL; dma_resv_unlock(&ghost->base._resv); diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c index fc351700d035..cec2e6fb1c52 100644 --- a/drivers/gpu/drm/ttm/ttm_resource.c +++ b/drivers/gpu/drm/ttm/ttm_resource.c @@ -33,6 +33,15 @@ int ttm_resource_alloc(struct ttm_buffer_object *bo, ttm_manager_type(bo->bdev, res->mem_type); res->mm_node = NULL; + res->start = 0; + res->num_pages = PFN_UP(bo->base.size); + res->mem_type = place->mem_type; + res->placement = place->flags; + res->bus.addr = NULL; + res->bus.offset = 0; + res->bus.is_iomem = false; + res->bus.caching = ttm_cached; + return man->func->alloc(man, bo, place, res); }