Re: [PATCH 1/2] drm/ttm: don't update global memory count for some special cases
Am 12.01.2018 um 07:14 schrieb Roger He: add input parameter for ttm_dma_unpopulate. when ttm_dma_pool_get_pages or ttm_mem_global_alloc_page fail, don't call ttm_mem_global_free_page to update global memory count. Signed-off-by: Roger HeGood catch, but that doesn't looks correct to me. See the handling in ttm_dma_populate(): ret = ttm_dma_pool_get_pages(pool, ttm_dma, i); if (ret != 0) break; ret = ttm_mem_global_alloc_page(mem_glob, ttm->pages[i], pool->size, ctx); if (unlikely(ret != 0)) { ttm_dma_unpopulate(ttm_dma, dev); return -ENOMEM; } When ttm_mem_global_alloc_page() fails we only missed to account the last allocated page, not all of them. Additional to that I really don't like adding the new parameter to the drivers. Regards, Christian. --- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 2 +- drivers/gpu/drm/nouveau/nouveau_bo.c | 2 +- drivers/gpu/drm/radeon/radeon_ttm.c | 2 +- drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 19 +++ drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c | 2 +- include/drm/ttm/ttm_page_alloc.h | 5 +++-- 6 files changed, 18 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index e4bb435..723ccf1 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -1046,7 +1046,7 @@ static void amdgpu_ttm_tt_unpopulate(struct ttm_tt *ttm) #ifdef CONFIG_SWIOTLB if (swiotlb_nr_tbl()) { - ttm_dma_unpopulate(>ttm, adev->dev); + ttm_dma_unpopulate(>ttm, adev->dev, true); return; } #endif diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c index ce328ed..3f7c30f 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.c +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c @@ -1632,7 +1632,7 @@ nouveau_ttm_tt_unpopulate(struct ttm_tt *ttm) #if IS_ENABLED(CONFIG_SWIOTLB) && IS_ENABLED(CONFIG_X86) if (swiotlb_nr_tbl()) { - ttm_dma_unpopulate((void *)ttm, dev); + ttm_dma_unpopulate((void *)ttm, dev, true); return; } #endif diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c index a0a839b..449cc65 100644 --- a/drivers/gpu/drm/radeon/radeon_ttm.c +++ b/drivers/gpu/drm/radeon/radeon_ttm.c @@ -789,7 +789,7 @@ static void radeon_ttm_tt_unpopulate(struct ttm_tt *ttm) #ifdef CONFIG_SWIOTLB if (swiotlb_nr_tbl()) { - ttm_dma_unpopulate(>ttm, rdev->dev); + ttm_dma_unpopulate(>ttm, rdev->dev, true); return; } #endif diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c index c7f01a4..4cda764 100644 --- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c +++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c @@ -969,7 +969,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, ctx); if (unlikely(ret != 0)) { - ttm_dma_unpopulate(ttm_dma, dev); + ttm_dma_unpopulate(ttm_dma, dev, false); return -ENOMEM; } @@ -998,14 +998,14 @@ int ttm_dma_populate(struct ttm_dma_tt *ttm_dma, struct device *dev, while (num_pages) { ret = ttm_dma_pool_get_pages(pool, ttm_dma, i); if (ret != 0) { - ttm_dma_unpopulate(ttm_dma, dev); + ttm_dma_unpopulate(ttm_dma, dev, false); return -ENOMEM; } ret = ttm_mem_global_alloc_page(mem_glob, ttm->pages[i], pool->size, ctx); if (unlikely(ret != 0)) { - ttm_dma_unpopulate(ttm_dma, dev); + ttm_dma_unpopulate(ttm_dma, dev, false); return -ENOMEM; } @@ -1016,7 +1016,7 @@ int ttm_dma_populate(struct ttm_dma_tt *ttm_dma, struct device *dev, if (unlikely(ttm->page_flags & TTM_PAGE_FLAG_SWAPPED)) { ret = ttm_tt_swapin(ttm); if (unlikely(ret != 0)) { - ttm_dma_unpopulate(ttm_dma, dev); + ttm_dma_unpopulate(ttm_dma, dev, true); return ret; } } @@ -1027,7 +1027,8 @@ int ttm_dma_populate(struct ttm_dma_tt *ttm_dma, struct device *dev, EXPORT_SYMBOL_GPL(ttm_dma_populate); /* Put all pages in pages list to correct pool to wait for reuse */ -void
RE: [PATCH 1/2] drm/ttm: don't update global memory count for some special cases
It is the first step for the GTT size issue. Need other patches I am working on it. Thanks Roger(Hongbo.He) -Original Message- From: Zhou, David(ChunMing) Sent: Friday, January 12, 2018 2:46 PM To: He, Roger <hongbo...@amd.com>; dri-devel@lists.freedesktop.org Cc: Koenig, Christian <christian.koe...@amd.com> Subject: Re: [PATCH 1/2] drm/ttm: don't update global memory count for some special cases Can this fix GTT size issue or not? Regards, David Zhou On 2018年01月12日 14:14, Roger He wrote: > add input parameter for ttm_dma_unpopulate. > when ttm_dma_pool_get_pages or ttm_mem_global_alloc_page fail, don't > call ttm_mem_global_free_page to update global memory count. > > Signed-off-by: Roger He <hongbo...@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 2 +- > drivers/gpu/drm/nouveau/nouveau_bo.c | 2 +- > drivers/gpu/drm/radeon/radeon_ttm.c | 2 +- > drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 19 +++ > drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c | 2 +- > include/drm/ttm/ttm_page_alloc.h | 5 +++-- > 6 files changed, 18 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > index e4bb435..723ccf1 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > @@ -1046,7 +1046,7 @@ static void amdgpu_ttm_tt_unpopulate(struct > ttm_tt *ttm) > > #ifdef CONFIG_SWIOTLB > if (swiotlb_nr_tbl()) { > - ttm_dma_unpopulate(>ttm, adev->dev); > + ttm_dma_unpopulate(>ttm, adev->dev, true); > return; > } > #endif > diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c > b/drivers/gpu/drm/nouveau/nouveau_bo.c > index ce328ed..3f7c30f 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_bo.c > +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c > @@ -1632,7 +1632,7 @@ nouveau_ttm_tt_unpopulate(struct ttm_tt *ttm) > > #if IS_ENABLED(CONFIG_SWIOTLB) && IS_ENABLED(CONFIG_X86) > if (swiotlb_nr_tbl()) { > - ttm_dma_unpopulate((void *)ttm, dev); > + ttm_dma_unpopulate((void *)ttm, dev, true); > return; > } > #endif > diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c > b/drivers/gpu/drm/radeon/radeon_ttm.c > index a0a839b..449cc65 100644 > --- a/drivers/gpu/drm/radeon/radeon_ttm.c > +++ b/drivers/gpu/drm/radeon/radeon_ttm.c > @@ -789,7 +789,7 @@ static void radeon_ttm_tt_unpopulate(struct ttm_tt > *ttm) > > #ifdef CONFIG_SWIOTLB > if (swiotlb_nr_tbl()) { > - ttm_dma_unpopulate(>ttm, rdev->dev); > + ttm_dma_unpopulate(>ttm, rdev->dev, true); > return; > } > #endif > diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c > b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c > index c7f01a4..4cda764 100644 > --- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c > +++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c > @@ -969,7 +969,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, ctx); > if (unlikely(ret != 0)) { > - ttm_dma_unpopulate(ttm_dma, dev); > + ttm_dma_unpopulate(ttm_dma, dev, false); > return -ENOMEM; > } > > @@ -998,14 +998,14 @@ int ttm_dma_populate(struct ttm_dma_tt *ttm_dma, struct > device *dev, > while (num_pages) { > ret = ttm_dma_pool_get_pages(pool, ttm_dma, i); > if (ret != 0) { > - ttm_dma_unpopulate(ttm_dma, dev); > + ttm_dma_unpopulate(ttm_dma, dev, false); > return -ENOMEM; > } > > ret = ttm_mem_global_alloc_page(mem_glob, ttm->pages[i], > pool->size, ctx); > if (unlikely(ret != 0)) { > - ttm_dma_unpopulate(ttm_dma, dev); > + ttm_dma_unpopulate(ttm_dma, dev, false); > return -ENOMEM; > } > > @@ -1016,7 +1016,7 @@ int ttm_dma_populate(struct ttm_dma_tt *ttm_dma, struct > device *dev, > if (unlikely(ttm->page_flags & TTM_PAGE_FLAG_SWAPPED)) { > ret = ttm_tt_swapin(ttm); > if (unlikely(ret != 0)) { > - ttm_dma_unpopulate(ttm_dma, dev); > + ttm_dma_unpopulate(ttm_dma, dev, true); > return ret; > } >
Re: [PATCH 1/2] drm/ttm: don't update global memory count for some special cases
Can this fix GTT size issue or not? Regards, David Zhou On 2018年01月12日 14:14, Roger He wrote: add input parameter for ttm_dma_unpopulate. when ttm_dma_pool_get_pages or ttm_mem_global_alloc_page fail, don't call ttm_mem_global_free_page to update global memory count. Signed-off-by: Roger He--- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 2 +- drivers/gpu/drm/nouveau/nouveau_bo.c | 2 +- drivers/gpu/drm/radeon/radeon_ttm.c | 2 +- drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 19 +++ drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c | 2 +- include/drm/ttm/ttm_page_alloc.h | 5 +++-- 6 files changed, 18 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index e4bb435..723ccf1 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -1046,7 +1046,7 @@ static void amdgpu_ttm_tt_unpopulate(struct ttm_tt *ttm) #ifdef CONFIG_SWIOTLB if (swiotlb_nr_tbl()) { - ttm_dma_unpopulate(>ttm, adev->dev); + ttm_dma_unpopulate(>ttm, adev->dev, true); return; } #endif diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c index ce328ed..3f7c30f 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.c +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c @@ -1632,7 +1632,7 @@ nouveau_ttm_tt_unpopulate(struct ttm_tt *ttm) #if IS_ENABLED(CONFIG_SWIOTLB) && IS_ENABLED(CONFIG_X86) if (swiotlb_nr_tbl()) { - ttm_dma_unpopulate((void *)ttm, dev); + ttm_dma_unpopulate((void *)ttm, dev, true); return; } #endif diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c index a0a839b..449cc65 100644 --- a/drivers/gpu/drm/radeon/radeon_ttm.c +++ b/drivers/gpu/drm/radeon/radeon_ttm.c @@ -789,7 +789,7 @@ static void radeon_ttm_tt_unpopulate(struct ttm_tt *ttm) #ifdef CONFIG_SWIOTLB if (swiotlb_nr_tbl()) { - ttm_dma_unpopulate(>ttm, rdev->dev); + ttm_dma_unpopulate(>ttm, rdev->dev, true); return; } #endif diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c index c7f01a4..4cda764 100644 --- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c +++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c @@ -969,7 +969,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, ctx); if (unlikely(ret != 0)) { - ttm_dma_unpopulate(ttm_dma, dev); + ttm_dma_unpopulate(ttm_dma, dev, false); return -ENOMEM; } @@ -998,14 +998,14 @@ int ttm_dma_populate(struct ttm_dma_tt *ttm_dma, struct device *dev, while (num_pages) { ret = ttm_dma_pool_get_pages(pool, ttm_dma, i); if (ret != 0) { - ttm_dma_unpopulate(ttm_dma, dev); + ttm_dma_unpopulate(ttm_dma, dev, false); return -ENOMEM; } ret = ttm_mem_global_alloc_page(mem_glob, ttm->pages[i], pool->size, ctx); if (unlikely(ret != 0)) { - ttm_dma_unpopulate(ttm_dma, dev); + ttm_dma_unpopulate(ttm_dma, dev, false); return -ENOMEM; } @@ -1016,7 +1016,7 @@ int ttm_dma_populate(struct ttm_dma_tt *ttm_dma, struct device *dev, if (unlikely(ttm->page_flags & TTM_PAGE_FLAG_SWAPPED)) { ret = ttm_tt_swapin(ttm); if (unlikely(ret != 0)) { - ttm_dma_unpopulate(ttm_dma, dev); + ttm_dma_unpopulate(ttm_dma, dev, true); return ret; } } @@ -1027,7 +1027,8 @@ int ttm_dma_populate(struct ttm_dma_tt *ttm_dma, struct device *dev, EXPORT_SYMBOL_GPL(ttm_dma_populate); /* Put all pages in pages list to correct pool to wait for reuse */ -void ttm_dma_unpopulate(struct ttm_dma_tt *ttm_dma, struct device *dev) +void ttm_dma_unpopulate(struct ttm_dma_tt *ttm_dma, struct device *dev, + bool update_glob_count) { struct ttm_tt *ttm = _dma->ttm; struct dma_pool *pool; @@ -1049,7 +1050,8 @@ void ttm_dma_unpopulate(struct ttm_dma_tt *ttm_dma, struct device *dev) continue; count++; - ttm_mem_global_free_page(ttm->glob->mem_glob, + if (update_glob_count) + ttm_mem_global_free_page(ttm->glob->mem_glob, d_page->p, pool->size);