| Message ID | 20220519095508.115203-2-christian.koenig@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/11] drm/radeon: switch over to ttm_bo_init_reserved | expand |
>-----Original Message----- >From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of >Christian König >Sent: Thursday, May 19, 2022 5:55 AM >To: intel-gfx@lists.freedesktop.org >Cc: matthew.william.auld@gmail.com; Christian König ><christian.koenig@amd.com>; dri-devel@lists.freedesktop.org >Subject: [PATCH 01/11] drm/radeon: switch over to ttm_bo_init_reserved > >Use the new interface instead. > >Signed-off-by: Christian König <christian.koenig@amd.com> >--- > drivers/gpu/drm/radeon/radeon_object.c | 17 +++++++++++------ > 1 file changed, 11 insertions(+), 6 deletions(-) > >diff --git a/drivers/gpu/drm/radeon/radeon_object.c >b/drivers/gpu/drm/radeon/radeon_object.c >index 6c4a6802ca96..1d414ff4ab0c 100644 >--- a/drivers/gpu/drm/radeon/radeon_object.c >+++ b/drivers/gpu/drm/radeon/radeon_object.c >@@ -133,9 +133,12 @@ int radeon_bo_create(struct radeon_device *rdev, > struct dma_resv *resv, > struct radeon_bo **bo_ptr) > { >- struct radeon_bo *bo; >- enum ttm_bo_type type; > unsigned long page_align = roundup(byte_align, PAGE_SIZE) >> >PAGE_SHIFT; >+ >+ /* Kernel allocation are uninterruptible */ >+ struct ttm_operation_ctx ctx = { !kernel, false }; >+ enum ttm_bo_type type; >+ struct radeon_bo *bo; > int r; > > size = ALIGN(size, PAGE_SIZE); >@@ -200,11 +203,13 @@ int radeon_bo_create(struct radeon_device *rdev, > #endif > > radeon_ttm_placement_from_domain(bo, domain); >- /* Kernel allocation are uninterruptible */ > down_read(&rdev->pm.mclk_lock); >- r = ttm_bo_init(&rdev->mman.bdev, &bo->tbo, size, type, >- &bo->placement, page_align, !kernel, sg, resv, >- &radeon_ttm_bo_destroy); >+ r = ttm_bo_init_reserved(&rdev->mman.bdev, &bo->tbo, size, type, >+ &bo->placement, page_align, &ctx, sg, resv, >+ &radeon_ttm_bo_destroy); >+ if (!r) >+ ttm_bo_unreserve(&bo->tbo); >+ Hi Christian, I am not understanding this unreserve. The original code path does not have it. It looks like tt_bo_init will do this, but only if !resv. Should this be: if (!resv) ttm_bo_unreserve(&bo->tbo); ? M > up_read(&rdev->pm.mclk_lock); > if (unlikely(r != 0)) { > return r; >-- >2.25.1
Am 19.05.22 um 14:54 schrieb Ruhl, Michael J: >> -----Original Message----- >> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of >> Christian König >> Sent: Thursday, May 19, 2022 5:55 AM >> To: intel-gfx@lists.freedesktop.org >> Cc: matthew.william.auld@gmail.com; Christian König >> <christian.koenig@amd.com>; dri-devel@lists.freedesktop.org >> Subject: [PATCH 01/11] drm/radeon: switch over to ttm_bo_init_reserved >> >> Use the new interface instead. >> >> Signed-off-by: Christian König <christian.koenig@amd.com> >> --- >> drivers/gpu/drm/radeon/radeon_object.c | 17 +++++++++++------ >> 1 file changed, 11 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/gpu/drm/radeon/radeon_object.c >> b/drivers/gpu/drm/radeon/radeon_object.c >> index 6c4a6802ca96..1d414ff4ab0c 100644 >> --- a/drivers/gpu/drm/radeon/radeon_object.c >> +++ b/drivers/gpu/drm/radeon/radeon_object.c >> @@ -133,9 +133,12 @@ int radeon_bo_create(struct radeon_device *rdev, >> struct dma_resv *resv, >> struct radeon_bo **bo_ptr) >> { >> - struct radeon_bo *bo; >> - enum ttm_bo_type type; >> unsigned long page_align = roundup(byte_align, PAGE_SIZE) >> >> PAGE_SHIFT; >> + >> + /* Kernel allocation are uninterruptible */ >> + struct ttm_operation_ctx ctx = { !kernel, false }; >> + enum ttm_bo_type type; >> + struct radeon_bo *bo; >> int r; >> >> size = ALIGN(size, PAGE_SIZE); >> @@ -200,11 +203,13 @@ int radeon_bo_create(struct radeon_device *rdev, >> #endif >> >> radeon_ttm_placement_from_domain(bo, domain); >> - /* Kernel allocation are uninterruptible */ >> down_read(&rdev->pm.mclk_lock); >> - r = ttm_bo_init(&rdev->mman.bdev, &bo->tbo, size, type, >> - &bo->placement, page_align, !kernel, sg, resv, >> - &radeon_ttm_bo_destroy); >> + r = ttm_bo_init_reserved(&rdev->mman.bdev, &bo->tbo, size, type, >> + &bo->placement, page_align, &ctx, sg, resv, >> + &radeon_ttm_bo_destroy); >> + if (!r) >> + ttm_bo_unreserve(&bo->tbo); >> + > Hi Christian, > > I am not understanding this unreserve. > > The original code path does not have it. It looks like tt_bo_init will do this, but only if !resv. > > Should this be: > if (!resv) > ttm_bo_unreserve(&bo->tbo); Ah, yes good point. That's a bug. Thanks, Christian. > > ? > > M > > >> up_read(&rdev->pm.mclk_lock); >> if (unlikely(r != 0)) { >> return r; >> -- >> 2.25.1
diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c index 6c4a6802ca96..1d414ff4ab0c 100644 --- a/drivers/gpu/drm/radeon/radeon_object.c +++ b/drivers/gpu/drm/radeon/radeon_object.c @@ -133,9 +133,12 @@ int radeon_bo_create(struct radeon_device *rdev, struct dma_resv *resv, struct radeon_bo **bo_ptr) { - struct radeon_bo *bo; - enum ttm_bo_type type; unsigned long page_align = roundup(byte_align, PAGE_SIZE) >> PAGE_SHIFT; + + /* Kernel allocation are uninterruptible */ + struct ttm_operation_ctx ctx = { !kernel, false }; + enum ttm_bo_type type; + struct radeon_bo *bo; int r; size = ALIGN(size, PAGE_SIZE); @@ -200,11 +203,13 @@ int radeon_bo_create(struct radeon_device *rdev, #endif radeon_ttm_placement_from_domain(bo, domain); - /* Kernel allocation are uninterruptible */ down_read(&rdev->pm.mclk_lock); - r = ttm_bo_init(&rdev->mman.bdev, &bo->tbo, size, type, - &bo->placement, page_align, !kernel, sg, resv, - &radeon_ttm_bo_destroy); + r = ttm_bo_init_reserved(&rdev->mman.bdev, &bo->tbo, size, type, + &bo->placement, page_align, &ctx, sg, resv, + &radeon_ttm_bo_destroy); + if (!r) + ttm_bo_unreserve(&bo->tbo); + up_read(&rdev->pm.mclk_lock); if (unlikely(r != 0)) { return r;
Use the new interface instead. Signed-off-by: Christian König <christian.koenig@amd.com> --- drivers/gpu/drm/radeon/radeon_object.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)