| Message ID | 1513766101-15993-3-git-send-email-Hongbo.He@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Commit message! Am 20.12.2017 um 11:34 schrieb Roger He: > Change-Id: I4104a12e09a374b6477a0dd5a8fce26dce27a746 > Signed-off-by: Roger He <Hongbo.He@amd.com> > --- > drivers/gpu/drm/ttm/ttm_memory.c | 15 ++++++++------- > drivers/gpu/drm/ttm/ttm_page_alloc.c | 6 +++++- > drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 8 ++++++-- > include/drm/ttm/ttm_memory.h | 3 ++- > 4 files changed, 21 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/ttm/ttm_memory.c b/drivers/gpu/drm/ttm/ttm_memory.c > index 525d3b6..8df0755 100644 > --- a/drivers/gpu/drm/ttm/ttm_memory.c > +++ b/drivers/gpu/drm/ttm/ttm_memory.c > @@ -539,15 +539,14 @@ int ttm_mem_global_alloc(struct ttm_mem_global *glob, uint64_t memory, > EXPORT_SYMBOL(ttm_mem_global_alloc); > > int ttm_mem_global_alloc_page(struct ttm_mem_global *glob, > - struct page *page, uint64_t size) > + struct page *page, uint64_t size, > + struct ttm_operation_ctx *ctx) > { > - > + int ret; > struct ttm_mem_zone *zone = NULL; > - struct ttm_operation_ctx ctx = { > - .interruptible = false, > - .no_wait_gpu = false > - }; > + bool tmp_no_wait_gpu = ctx->no_wait_gpu; Mhm, please drop that. That the function might wait for the GPU even when the caller requested not to do so sounds like a bug to me. Christian. > > + ctx->no_wait_gpu = false; > /** > * Page allocations may be registed in a single zone > * only if highmem or !dma32. > @@ -560,7 +559,9 @@ int ttm_mem_global_alloc_page(struct ttm_mem_global *glob, > if (glob->zone_dma32 && page_to_pfn(page) > 0x00100000UL) > zone = glob->zone_kernel; > #endif > - return ttm_mem_global_alloc_zone(glob, zone, size, &ctx); > + ret = ttm_mem_global_alloc_zone(glob, zone, size, ctx); > + ctx->no_wait_gpu = tmp_no_wait_gpu; > + return ret; > } > > void ttm_mem_global_free_page(struct ttm_mem_global *glob, struct page *page, > diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c b/drivers/gpu/drm/ttm/ttm_page_alloc.c > index b5ba644..8f93ff3 100644 > --- a/drivers/gpu/drm/ttm/ttm_page_alloc.c > +++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c > @@ -1061,6 +1061,10 @@ void ttm_page_alloc_fini(void) > int ttm_pool_populate(struct ttm_tt *ttm) > { > struct ttm_mem_global *mem_glob = ttm->glob->mem_glob; > + struct ttm_operation_ctx ctx = { > + .interruptible = false, > + .no_wait_gpu = false > + }; > unsigned i; > int ret; > > @@ -1076,7 +1080,7 @@ int ttm_pool_populate(struct ttm_tt *ttm) > > for (i = 0; i < ttm->num_pages; ++i) { > ret = ttm_mem_global_alloc_page(mem_glob, ttm->pages[i], > - PAGE_SIZE); > + PAGE_SIZE, &ctx); > if (unlikely(ret != 0)) { > ttm_pool_unpopulate(ttm); > return -ENOMEM; > diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c > index bda00b2..8aac86a 100644 > --- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c > +++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c > @@ -927,6 +927,10 @@ int ttm_dma_populate(struct ttm_dma_tt *ttm_dma, struct device *dev) > { > struct ttm_tt *ttm = &ttm_dma->ttm; > struct ttm_mem_global *mem_glob = ttm->glob->mem_glob; > + struct ttm_operation_ctx ctx = { > + .interruptible = false, > + .no_wait_gpu = false > + }; > unsigned long num_pages = ttm->num_pages; > struct dma_pool *pool; > enum pool_type type; > @@ -962,7 +966,7 @@ int ttm_dma_populate(struct ttm_dma_tt *ttm_dma, struct device *dev) > break; > > ret = ttm_mem_global_alloc_page(mem_glob, ttm->pages[i], > - pool->size); > + pool->size, &ctx); > if (unlikely(ret != 0)) { > ttm_dma_unpopulate(ttm_dma, dev); > return -ENOMEM; > @@ -998,7 +1002,7 @@ int ttm_dma_populate(struct ttm_dma_tt *ttm_dma, struct device *dev) > } > > ret = ttm_mem_global_alloc_page(mem_glob, ttm->pages[i], > - pool->size); > + pool->size, &ctx); > if (unlikely(ret != 0)) { > ttm_dma_unpopulate(ttm_dma, dev); > return -ENOMEM; > diff --git a/include/drm/ttm/ttm_memory.h b/include/drm/ttm/ttm_memory.h > index 755c107..8936285 100644 > --- a/include/drm/ttm/ttm_memory.h > +++ b/include/drm/ttm/ttm_memory.h > @@ -84,7 +84,8 @@ extern int ttm_mem_global_alloc(struct ttm_mem_global *glob, uint64_t memory, > extern void ttm_mem_global_free(struct ttm_mem_global *glob, > uint64_t amount); > extern int ttm_mem_global_alloc_page(struct ttm_mem_global *glob, > - struct page *page, uint64_t size); > + struct page *page, uint64_t size, > + struct ttm_operation_ctx *ctx); > extern void ttm_mem_global_free_page(struct ttm_mem_global *glob, > struct page *page, uint64_t size); > extern size_t ttm_round_pot(size_t size);
-----Original Message----- From: Christian König [mailto:ckoenig.leichtzumerken@gmail.com] Sent: Wednesday, December 20, 2017 9:36 PM To: He, Roger <Hongbo.He@amd.com>; amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org Subject: Re: [PATCH 3/7] drm/ttm: use an operation ctx for ttm_mem_global_alloc_page Commit message! Am 20.12.2017 um 11:34 schrieb Roger He: > Change-Id: I4104a12e09a374b6477a0dd5a8fce26dce27a746 > Signed-off-by: Roger He <Hongbo.He@amd.com> > --- > drivers/gpu/drm/ttm/ttm_memory.c | 15 ++++++++------- > drivers/gpu/drm/ttm/ttm_page_alloc.c | 6 +++++- > drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 8 ++++++-- > include/drm/ttm/ttm_memory.h | 3 ++- > 4 files changed, 21 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/ttm/ttm_memory.c > b/drivers/gpu/drm/ttm/ttm_memory.c > index 525d3b6..8df0755 100644 > --- a/drivers/gpu/drm/ttm/ttm_memory.c > +++ b/drivers/gpu/drm/ttm/ttm_memory.c > @@ -539,15 +539,14 @@ int ttm_mem_global_alloc(struct ttm_mem_global *glob, uint64_t memory, > EXPORT_SYMBOL(ttm_mem_global_alloc); > > int ttm_mem_global_alloc_page(struct ttm_mem_global *glob, > - struct page *page, uint64_t size) > + struct page *page, uint64_t size, > + struct ttm_operation_ctx *ctx) > { > - > + int ret; > struct ttm_mem_zone *zone = NULL; > - struct ttm_operation_ctx ctx = { > - .interruptible = false, > - .no_wait_gpu = false > - }; > + bool tmp_no_wait_gpu = ctx->no_wait_gpu; Mhm, please drop that. That the function might wait for the GPU even when the caller requested not to do so sounds like a bug to me. Yes. I will remove this. Later I will send new patches. Will appreciate If Thomas can help to verify that. Thanks Roger(Hongbo.He) > > + ctx->no_wait_gpu = false; > /** > * Page allocations may be registed in a single zone > * only if highmem or !dma32. > @@ -560,7 +559,9 @@ int ttm_mem_global_alloc_page(struct ttm_mem_global *glob, > if (glob->zone_dma32 && page_to_pfn(page) > 0x00100000UL) > zone = glob->zone_kernel; > #endif > - return ttm_mem_global_alloc_zone(glob, zone, size, &ctx); > + ret = ttm_mem_global_alloc_zone(glob, zone, size, ctx); > + ctx->no_wait_gpu = tmp_no_wait_gpu; > + return ret; > } > > void ttm_mem_global_free_page(struct ttm_mem_global *glob, struct > page *page, diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c > b/drivers/gpu/drm/ttm/ttm_page_alloc.c > index b5ba644..8f93ff3 100644 > --- a/drivers/gpu/drm/ttm/ttm_page_alloc.c > +++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c > @@ -1061,6 +1061,10 @@ void ttm_page_alloc_fini(void) > int ttm_pool_populate(struct ttm_tt *ttm) > { > struct ttm_mem_global *mem_glob = ttm->glob->mem_glob; > + struct ttm_operation_ctx ctx = { > + .interruptible = false, > + .no_wait_gpu = false > + }; > unsigned i; > int ret; > > @@ -1076,7 +1080,7 @@ int ttm_pool_populate(struct ttm_tt *ttm) > > for (i = 0; i < ttm->num_pages; ++i) { > ret = ttm_mem_global_alloc_page(mem_glob, ttm->pages[i], > - PAGE_SIZE); > + PAGE_SIZE, &ctx); > if (unlikely(ret != 0)) { > ttm_pool_unpopulate(ttm); > return -ENOMEM; > diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c > b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c > index bda00b2..8aac86a 100644 > --- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c > +++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c > @@ -927,6 +927,10 @@ int ttm_dma_populate(struct ttm_dma_tt *ttm_dma, struct device *dev) > { > struct ttm_tt *ttm = &ttm_dma->ttm; > struct ttm_mem_global *mem_glob = ttm->glob->mem_glob; > + struct ttm_operation_ctx ctx = { > + .interruptible = false, > + .no_wait_gpu = false > + }; > unsigned long num_pages = ttm->num_pages; > struct dma_pool *pool; > enum pool_type type; > @@ -962,7 +966,7 @@ int ttm_dma_populate(struct ttm_dma_tt *ttm_dma, struct device *dev) > break; > > ret = ttm_mem_global_alloc_page(mem_glob, ttm->pages[i], > - pool->size); > + pool->size, &ctx); > if (unlikely(ret != 0)) { > ttm_dma_unpopulate(ttm_dma, dev); > return -ENOMEM; > @@ -998,7 +1002,7 @@ int ttm_dma_populate(struct ttm_dma_tt *ttm_dma, struct device *dev) > } > > ret = ttm_mem_global_alloc_page(mem_glob, ttm->pages[i], > - pool->size); > + pool->size, &ctx); > if (unlikely(ret != 0)) { > ttm_dma_unpopulate(ttm_dma, dev); > return -ENOMEM; > diff --git a/include/drm/ttm/ttm_memory.h > b/include/drm/ttm/ttm_memory.h index 755c107..8936285 100644 > --- a/include/drm/ttm/ttm_memory.h > +++ b/include/drm/ttm/ttm_memory.h > @@ -84,7 +84,8 @@ extern int ttm_mem_global_alloc(struct ttm_mem_global *glob, uint64_t memory, > extern void ttm_mem_global_free(struct ttm_mem_global *glob, > uint64_t amount); > extern int ttm_mem_global_alloc_page(struct ttm_mem_global *glob, > - struct page *page, uint64_t size); > + struct page *page, uint64_t size, > + struct ttm_operation_ctx *ctx); > extern void ttm_mem_global_free_page(struct ttm_mem_global *glob, > struct page *page, uint64_t size); > extern size_t ttm_round_pot(size_t size);
On 12/21/2017 07:05 AM, He, Roger wrote: > > -----Original Message----- > From: Christian König [mailto:ckoenig.leichtzumerken@gmail.com] > Sent: Wednesday, December 20, 2017 9:36 PM > To: He, Roger <Hongbo.He@amd.com>; amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org > Subject: Re: [PATCH 3/7] drm/ttm: use an operation ctx for ttm_mem_global_alloc_page > > Commit message! > > Am 20.12.2017 um 11:34 schrieb Roger He: >> Change-Id: I4104a12e09a374b6477a0dd5a8fce26dce27a746 >> Signed-off-by: Roger He <Hongbo.He@amd.com> >> --- >> drivers/gpu/drm/ttm/ttm_memory.c | 15 ++++++++------- >> drivers/gpu/drm/ttm/ttm_page_alloc.c | 6 +++++- >> drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 8 ++++++-- >> include/drm/ttm/ttm_memory.h | 3 ++- >> 4 files changed, 21 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/gpu/drm/ttm/ttm_memory.c >> b/drivers/gpu/drm/ttm/ttm_memory.c >> index 525d3b6..8df0755 100644 >> --- a/drivers/gpu/drm/ttm/ttm_memory.c >> +++ b/drivers/gpu/drm/ttm/ttm_memory.c >> @@ -539,15 +539,14 @@ int ttm_mem_global_alloc(struct ttm_mem_global *glob, uint64_t memory, >> EXPORT_SYMBOL(ttm_mem_global_alloc); >> >> int ttm_mem_global_alloc_page(struct ttm_mem_global *glob, >> - struct page *page, uint64_t size) >> + struct page *page, uint64_t size, >> + struct ttm_operation_ctx *ctx) >> { >> - >> + int ret; >> struct ttm_mem_zone *zone = NULL; >> - struct ttm_operation_ctx ctx = { >> - .interruptible = false, >> - .no_wait_gpu = false >> - }; >> + bool tmp_no_wait_gpu = ctx->no_wait_gpu; > Mhm, please drop that. That the function might wait for the GPU even when the caller requested not to do so sounds like a bug to Yes, I agree. /Thomas
diff --git a/drivers/gpu/drm/ttm/ttm_memory.c b/drivers/gpu/drm/ttm/ttm_memory.c index 525d3b6..8df0755 100644 --- a/drivers/gpu/drm/ttm/ttm_memory.c +++ b/drivers/gpu/drm/ttm/ttm_memory.c @@ -539,15 +539,14 @@ int ttm_mem_global_alloc(struct ttm_mem_global *glob, uint64_t memory, EXPORT_SYMBOL(ttm_mem_global_alloc); int ttm_mem_global_alloc_page(struct ttm_mem_global *glob, - struct page *page, uint64_t size) + struct page *page, uint64_t size, + struct ttm_operation_ctx *ctx) { - + int ret; struct ttm_mem_zone *zone = NULL; - struct ttm_operation_ctx ctx = { - .interruptible = false, - .no_wait_gpu = false - }; + bool tmp_no_wait_gpu = ctx->no_wait_gpu; + ctx->no_wait_gpu = false; /** * Page allocations may be registed in a single zone * only if highmem or !dma32. @@ -560,7 +559,9 @@ int ttm_mem_global_alloc_page(struct ttm_mem_global *glob, if (glob->zone_dma32 && page_to_pfn(page) > 0x00100000UL) zone = glob->zone_kernel; #endif - return ttm_mem_global_alloc_zone(glob, zone, size, &ctx); + ret = ttm_mem_global_alloc_zone(glob, zone, size, ctx); + ctx->no_wait_gpu = tmp_no_wait_gpu; + return ret; } void ttm_mem_global_free_page(struct ttm_mem_global *glob, struct page *page, diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c b/drivers/gpu/drm/ttm/ttm_page_alloc.c index b5ba644..8f93ff3 100644 --- a/drivers/gpu/drm/ttm/ttm_page_alloc.c +++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c @@ -1061,6 +1061,10 @@ void ttm_page_alloc_fini(void) int ttm_pool_populate(struct ttm_tt *ttm) { struct ttm_mem_global *mem_glob = ttm->glob->mem_glob; + struct ttm_operation_ctx ctx = { + .interruptible = false, + .no_wait_gpu = false + }; unsigned i; int ret; @@ -1076,7 +1080,7 @@ int ttm_pool_populate(struct ttm_tt *ttm) for (i = 0; i < ttm->num_pages; ++i) { ret = ttm_mem_global_alloc_page(mem_glob, ttm->pages[i], - PAGE_SIZE); + PAGE_SIZE, &ctx); if (unlikely(ret != 0)) { ttm_pool_unpopulate(ttm); return -ENOMEM; diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c index bda00b2..8aac86a 100644 --- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c +++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c @@ -927,6 +927,10 @@ int ttm_dma_populate(struct ttm_dma_tt *ttm_dma, struct device *dev) { struct ttm_tt *ttm = &ttm_dma->ttm; struct ttm_mem_global *mem_glob = ttm->glob->mem_glob; + struct ttm_operation_ctx ctx = { + .interruptible = false, + .no_wait_gpu = false + }; unsigned long num_pages = ttm->num_pages; struct dma_pool *pool; enum pool_type type; @@ -962,7 +966,7 @@ int ttm_dma_populate(struct ttm_dma_tt *ttm_dma, struct device *dev) break; ret = ttm_mem_global_alloc_page(mem_glob, ttm->pages[i], - pool->size); + pool->size, &ctx); if (unlikely(ret != 0)) { ttm_dma_unpopulate(ttm_dma, dev); return -ENOMEM; @@ -998,7 +1002,7 @@ int ttm_dma_populate(struct ttm_dma_tt *ttm_dma, struct device *dev) } ret = ttm_mem_global_alloc_page(mem_glob, ttm->pages[i], - pool->size); + pool->size, &ctx); if (unlikely(ret != 0)) { ttm_dma_unpopulate(ttm_dma, dev); return -ENOMEM; diff --git a/include/drm/ttm/ttm_memory.h b/include/drm/ttm/ttm_memory.h index 755c107..8936285 100644 --- a/include/drm/ttm/ttm_memory.h +++ b/include/drm/ttm/ttm_memory.h @@ -84,7 +84,8 @@ extern int ttm_mem_global_alloc(struct ttm_mem_global *glob, uint64_t memory, extern void ttm_mem_global_free(struct ttm_mem_global *glob, uint64_t amount); extern int ttm_mem_global_alloc_page(struct ttm_mem_global *glob, - struct page *page, uint64_t size); + struct page *page, uint64_t size, + struct ttm_operation_ctx *ctx); extern void ttm_mem_global_free_page(struct ttm_mem_global *glob, struct page *page, uint64_t size); extern size_t ttm_round_pot(size_t size);
Change-Id: I4104a12e09a374b6477a0dd5a8fce26dce27a746 Signed-off-by: Roger He <Hongbo.He@amd.com> --- drivers/gpu/drm/ttm/ttm_memory.c | 15 ++++++++------- drivers/gpu/drm/ttm/ttm_page_alloc.c | 6 +++++- drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 8 ++++++-- include/drm/ttm/ttm_memory.h | 3 ++- 4 files changed, 21 insertions(+), 11 deletions(-)