| Message ID | 1513071228-29551-5-git-send-email-Hongbo.He@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Am 12.12.2017 um 10:33 schrieb Roger He: > Change-Id: I491d4ceb8c98bb3d8e6e0ddef2330284ce2fe5f6 > Signed-off-by: Roger He <Hongbo.He@amd.com> I would squash this one with patch #6. > --- > drivers/gpu/drm/ttm/ttm_bo.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c > index eb8c568..22b6ca5 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo.c > +++ b/drivers/gpu/drm/ttm/ttm_bo.c > @@ -722,10 +722,9 @@ static int ttm_mem_evict_first(struct ttm_bo_device *bdev, > spin_lock(&glob->lru_lock); > for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) { > list_for_each_entry(bo, &man->lru[i], lru) { > - if (bo->resv == resv) { > - if (list_empty(&bo->ddestroy)) > - continue; > - } else { > + if (!ctx || > + !(ctx->on_alloc_stage && > + bo->resv == ctx->resv)) { Coding style: The lines stating with "!(ctx" and "bo->resv" are to far to the right. Additional to that I think ctx is mandatory and doesn't need a check (but might be wrong). If it isn't it's probably time to make it mandatory. And I would use (ctx->on_alloc_stage || list_empty(&bo->ddestroy)) as check, we probably still want to be able to handle deleted BOs here during CS. Christian. > locked = reservation_object_trylock(bo->resv); > if (!locked) > continue;
-----Original Message----- From: Christian König [mailto:ckoenig.leichtzumerken@gmail.com] Sent: Tuesday, December 12, 2017 6:40 PM To: He, Roger <Hongbo.He@amd.com>; amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org Subject: Re: [PATCH 5/6] drm/ttm: enable eviction for Per-VM-BO Am 12.12.2017 um 10:33 schrieb Roger He: > Change-Id: I491d4ceb8c98bb3d8e6e0ddef2330284ce2fe5f6 > Signed-off-by: Roger He <Hongbo.He@amd.com> I would squash this one with patch #6. > --- > drivers/gpu/drm/ttm/ttm_bo.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/ttm/ttm_bo.c > b/drivers/gpu/drm/ttm/ttm_bo.c index eb8c568..22b6ca5 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo.c > +++ b/drivers/gpu/drm/ttm/ttm_bo.c > @@ -722,10 +722,9 @@ static int ttm_mem_evict_first(struct ttm_bo_device *bdev, > spin_lock(&glob->lru_lock); > for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) { > list_for_each_entry(bo, &man->lru[i], lru) { > - if (bo->resv == resv) { > - if (list_empty(&bo->ddestroy)) > - continue; > - } else { > + if (!ctx || > + !(ctx->on_alloc_stage && > + bo->resv == ctx->resv)) { Coding style: The lines stating with "!(ctx" and "bo->resv" are to far to the right. Additional to that I think ctx is mandatory and doesn't need a check (but might be wrong). If it isn't it's probably time to make it mandatory. And I would use (ctx->on_alloc_stage || list_empty(&bo->ddestroy)) as check, we probably still want to be able to handle deleted BOs here during CS. Change that as below, I think it can cover the case you described. No matter the Bo is in deleted list or not. if (!ctx->allow_reserved_eviction || bo->resv != ctx->resv) { locked = kcl_reservation_object_trylock(bo->resv); if (!locked) continue; } Thanks Roger(Hongbo.He) Christian. > locked = reservation_object_trylock(bo->resv); > if (!locked) > continue;
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index eb8c568..22b6ca5 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -722,10 +722,9 @@ static int ttm_mem_evict_first(struct ttm_bo_device *bdev, spin_lock(&glob->lru_lock); for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) { list_for_each_entry(bo, &man->lru[i], lru) { - if (bo->resv == resv) { - if (list_empty(&bo->ddestroy)) - continue; - } else { + if (!ctx || + !(ctx->on_alloc_stage && + bo->resv == ctx->resv)) { locked = reservation_object_trylock(bo->resv); if (!locked) continue;
Change-Id: I491d4ceb8c98bb3d8e6e0ddef2330284ce2fe5f6 Signed-off-by: Roger He <Hongbo.He@amd.com> --- drivers/gpu/drm/ttm/ttm_bo.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)