Karol Herbst
2023-Jan-18 13:01 UTC
[Nouveau] [PATCH 5/9] drm/nouveau: stop using ttm_bo_wait
On Wed, Jan 18, 2023 at 10:04 AM Christian K?nig <ckoenig.leichtzumerken at gmail.com> wrote:> > Hi guys, > > another ping for this. It's just a minor cleanup. >Acked-by: Karol Herbst <kherbst at redhat.com> Though I'd say that having a wrapper function like that isn't pointless on its own and I kind of fail to see the reason it gets removed in the first place. Also.. I wouldn't call this a "cleanup" because it actually removes something useful. Yes, it's only calling one function, but it's more of a pain to use the wrapped one than the outer one.> Dave/Daniel can you help me out here? > > Thanks, > Christian. > > Am 11.01.23 um 10:52 schrieb Christian K?nig: > > Hi guys, > > > > can I get a quick ack for this? > > > > The patch has no functional change and is just a cleanup. > > > > Thanks, > > Christian. > > > > Am 25.11.22 um 11:21 schrieb Christian K?nig: > >> TTM is just wrapping core DMA functionality here, remove the mid-layer. > >> No functional change. > >> > >> Signed-off-by: Christian K?nig <christian.koenig at amd.com> > >> --- > >> drivers/gpu/drm/nouveau/nouveau_bo.c | 6 +++++- > >> drivers/gpu/drm/nouveau/nouveau_gem.c | 11 ++++++++--- > >> 2 files changed, 13 insertions(+), 4 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c > >> b/drivers/gpu/drm/nouveau/nouveau_bo.c > >> index 335fa91ca4ad..288eebc70a67 100644 > >> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c > >> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c > >> @@ -922,6 +922,7 @@ static void nouveau_bo_move_ntfy(struct > >> ttm_buffer_object *bo, > >> struct nouveau_mem *mem = new_reg ? nouveau_mem(new_reg) : NULL; > >> struct nouveau_bo *nvbo = nouveau_bo(bo); > >> struct nouveau_vma *vma; > >> + long ret; > >> /* ttm can now (stupidly) pass the driver bos it didn't > >> create... */ > >> if (bo->destroy != nouveau_bo_del_ttm) > >> @@ -936,7 +937,10 @@ static void nouveau_bo_move_ntfy(struct > >> ttm_buffer_object *bo, > >> } > >> } else { > >> list_for_each_entry(vma, &nvbo->vma_list, head) { > >> - WARN_ON(ttm_bo_wait(bo, false, false)); > >> + ret = dma_resv_wait_timeout(bo->base.resv, > >> + DMA_RESV_USAGE_BOOKKEEP, > >> + false, 15 * HZ); > >> + WARN_ON(ret <= 0); > >> nouveau_vma_unmap(vma); > >> } > >> } > >> diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c > >> b/drivers/gpu/drm/nouveau/nouveau_gem.c > >> index ac5793c96957..f77e44958037 100644 > >> --- a/drivers/gpu/drm/nouveau/nouveau_gem.c > >> +++ b/drivers/gpu/drm/nouveau/nouveau_gem.c > >> @@ -645,7 +645,7 @@ nouveau_gem_pushbuf_reloc_apply(struct > >> nouveau_cli *cli, > >> struct drm_nouveau_gem_pushbuf_reloc *reloc, > >> struct drm_nouveau_gem_pushbuf_bo *bo) > >> { > >> - int ret = 0; > >> + long ret = 0; > >> unsigned i; > >> for (i = 0; i < req->nr_relocs; i++) { > >> @@ -703,9 +703,14 @@ nouveau_gem_pushbuf_reloc_apply(struct > >> nouveau_cli *cli, > >> data |= r->vor; > >> } > >> - ret = ttm_bo_wait(&nvbo->bo, false, false); > >> + ret = dma_resv_wait_timeout(nvbo->bo.base.resv, > >> + DMA_RESV_USAGE_BOOKKEEP, > >> + false, 15 * HZ); > >> + if (ret == 0) > >> + ret = -EBUSY; > >> if (ret) { > >> - NV_PRINTK(err, cli, "reloc wait_idle failed: %d\n", ret); > >> + NV_PRINTK(err, cli, "reloc wait_idle failed: %ld\n", > >> + ret); > >> break; > >> } > > >
Christian König
2023-Jan-18 14:15 UTC
[Nouveau] [PATCH 5/9] drm/nouveau: stop using ttm_bo_wait
Am 18.01.23 um 14:01 schrieb Karol Herbst:> On Wed, Jan 18, 2023 at 10:04 AM Christian K?nig > <ckoenig.leichtzumerken at gmail.com> wrote: >> Hi guys, >> >> another ping for this. It's just a minor cleanup. >> > Acked-by: Karol Herbst <kherbst at redhat.com>Thanks!> > Though I'd say that having a wrapper function like that isn't > pointless on its own and I kind of fail to see the reason it gets > removed in the first place. > > Also.. I wouldn't call this a "cleanup" because it actually removes > something useful. Yes, it's only calling one function, but it's more > of a pain to use the wrapped one than the outer one.The problem is that it's hiding the timeout. Just read up what discussion this triggered within the Intel driver. Christian.> >> Dave/Daniel can you help me out here? >> >> Thanks, >> Christian. >> >> Am 11.01.23 um 10:52 schrieb Christian K?nig: >>> Hi guys, >>> >>> can I get a quick ack for this? >>> >>> The patch has no functional change and is just a cleanup. >>> >>> Thanks, >>> Christian. >>> >>> Am 25.11.22 um 11:21 schrieb Christian K?nig: >>>> TTM is just wrapping core DMA functionality here, remove the mid-layer. >>>> No functional change. >>>> >>>> Signed-off-by: Christian K?nig <christian.koenig at amd.com> >>>> --- >>>> drivers/gpu/drm/nouveau/nouveau_bo.c | 6 +++++- >>>> drivers/gpu/drm/nouveau/nouveau_gem.c | 11 ++++++++--- >>>> 2 files changed, 13 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c >>>> b/drivers/gpu/drm/nouveau/nouveau_bo.c >>>> index 335fa91ca4ad..288eebc70a67 100644 >>>> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c >>>> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c >>>> @@ -922,6 +922,7 @@ static void nouveau_bo_move_ntfy(struct >>>> ttm_buffer_object *bo, >>>> struct nouveau_mem *mem = new_reg ? nouveau_mem(new_reg) : NULL; >>>> struct nouveau_bo *nvbo = nouveau_bo(bo); >>>> struct nouveau_vma *vma; >>>> + long ret; >>>> /* ttm can now (stupidly) pass the driver bos it didn't >>>> create... */ >>>> if (bo->destroy != nouveau_bo_del_ttm) >>>> @@ -936,7 +937,10 @@ static void nouveau_bo_move_ntfy(struct >>>> ttm_buffer_object *bo, >>>> } >>>> } else { >>>> list_for_each_entry(vma, &nvbo->vma_list, head) { >>>> - WARN_ON(ttm_bo_wait(bo, false, false)); >>>> + ret = dma_resv_wait_timeout(bo->base.resv, >>>> + DMA_RESV_USAGE_BOOKKEEP, >>>> + false, 15 * HZ); >>>> + WARN_ON(ret <= 0); >>>> nouveau_vma_unmap(vma); >>>> } >>>> } >>>> diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c >>>> b/drivers/gpu/drm/nouveau/nouveau_gem.c >>>> index ac5793c96957..f77e44958037 100644 >>>> --- a/drivers/gpu/drm/nouveau/nouveau_gem.c >>>> +++ b/drivers/gpu/drm/nouveau/nouveau_gem.c >>>> @@ -645,7 +645,7 @@ nouveau_gem_pushbuf_reloc_apply(struct >>>> nouveau_cli *cli, >>>> struct drm_nouveau_gem_pushbuf_reloc *reloc, >>>> struct drm_nouveau_gem_pushbuf_bo *bo) >>>> { >>>> - int ret = 0; >>>> + long ret = 0; >>>> unsigned i; >>>> for (i = 0; i < req->nr_relocs; i++) { >>>> @@ -703,9 +703,14 @@ nouveau_gem_pushbuf_reloc_apply(struct >>>> nouveau_cli *cli, >>>> data |= r->vor; >>>> } >>>> - ret = ttm_bo_wait(&nvbo->bo, false, false); >>>> + ret = dma_resv_wait_timeout(nvbo->bo.base.resv, >>>> + DMA_RESV_USAGE_BOOKKEEP, >>>> + false, 15 * HZ); >>>> + if (ret == 0) >>>> + ret = -EBUSY; >>>> if (ret) { >>>> - NV_PRINTK(err, cli, "reloc wait_idle failed: %d\n", ret); >>>> + NV_PRINTK(err, cli, "reloc wait_idle failed: %ld\n", >>>> + ret); >>>> break; >>>> }