Menu
▾
▴
Re: Avoiding page allocation on validate
|
From: <th...@tu...> - 2007-12-21 13:01:49
|
Keith Packard wrote:
>Right now, validating a buffer object forces every missing page to be
>allocated. That means for things like vertex buffer objects, we either
>need to create them at the 'right' size, or pay a significant penalty
>when a large buffer is mapped to the card.
>
>It seems like it should be far better to just use a page full of zeros
>in the read-only case.
>
>Here's a patch which does that, however I cannot test it as Mesa never
>asks for read-only kernel-allocated pages. I'm hoping that can be fixed;
>it means knowing up-front whether a buffer will ever be used as a
>rendering target, or fixing the API so the validate call knows the
>intended GPU access. The latter is probably more reasonable, so perhaps
>I'll explore that. Suggestions, as always, are welcome.
>
>
Keith,
This looks like a good thing, but is there a way we can avoid doing this
in the ttm backend?
Some backends (Poulsbo, and I think Dave's work on Radeon) implement a
null populate() function and use the ttm page array directly.
Hence we could perhaps use the dummy page in the ttm page directly,
like the user bo/ttm objects. That way we would also save a lot of ttm
backend code.
/Thomas
commit 864298592f60bcfadeb35e33e1789025d0f19b0b
>Author: Keith Packard <ke...@ke...>
>Date: Sun Dec 16 01:47:51 2007 -0800
>
> Use dummy_read_page for unpopulated kernel-allocated ttm pages.
>
> Previously, dummy_read_page was used only for read-only user allocations; it
> filled in pages that were not present in the user address map (presumably,
> these were allocated but never written to pages).
>
> This patch allows them to be used for read-only ttms allocated from the
> kernel, so that applications can over-allocate buffers without forcing every
> page to be allocated.
>
>diff --git a/linux-core/drm_agpsupport.c b/linux-core/drm_agpsupport.c
>index e8bfaea..0218701 100644
>--- a/linux-core/drm_agpsupport.c
>+++ b/linux-core/drm_agpsupport.c
>@@ -505,12 +505,14 @@ static int drm_agp_needs_unbind_cache_adjust(struct drm_ttm_backend *backend)
>
>
> static int drm_agp_populate(struct drm_ttm_backend *backend,
>- unsigned long num_pages, struct page **pages)
>+ unsigned long num_pages, struct page **pages,
>+ struct page *dummy_read_page)
> {
> struct drm_agp_ttm_backend *agp_be =
> container_of(backend, struct drm_agp_ttm_backend, backend);
> struct page **cur_page, **last_page = pages + num_pages;
> DRM_AGP_MEM *mem;
>+ int dummy_page_count = 0;
>
> if (drm_alloc_memctl(num_pages * sizeof(void *)))
> return -1;
>@@ -528,8 +530,16 @@ static int drm_agp_populate(struct drm_ttm_backend *backend,
>
> DRM_DEBUG("Current page count is %ld\n", (long) mem->page_count);
> mem->page_count = 0;
>- for (cur_page = pages; cur_page < last_page; ++cur_page)
>- mem->memory[mem->page_count++] = phys_to_gart(page_to_phys(*cur_page));
>+ for (cur_page = pages; cur_page < last_page; ++cur_page) {
>+ struct page *page = *cur_page;
>+ if (!page) {
>+ page = dummy_read_page;
>+ ++dummy_page_count;
>+ }
>+ mem->memory[mem->page_count++] = phys_to_gart(page_to_phys(page));
>+ }
>+ if (dummy_page_count)
>+ DRM_DEBUG("Mapped %d dummy pages\n", dummy_page_count);
> agp_be->mem = mem;
> return 0;
> }
>diff --git a/linux-core/drm_objects.h b/linux-core/drm_objects.h
>index 375420d..47b6b01 100644
>--- a/linux-core/drm_objects.h
>+++ b/linux-core/drm_objects.h
>@@ -263,7 +263,8 @@ struct drm_ttm_backend;
> struct drm_ttm_backend_func {
> int (*needs_ub_cache_adjust) (struct drm_ttm_backend *backend);
> int (*populate) (struct drm_ttm_backend *backend,
>- unsigned long num_pages, struct page **pages);
>+ unsigned long num_pages, struct page **pages,
>+ struct page *dummy_read_page);
> void (*clear) (struct drm_ttm_backend *backend);
> int (*bind) (struct drm_ttm_backend *backend,
> struct drm_bo_mem_reg *bo_mem);
>diff --git a/linux-core/drm_ttm.c b/linux-core/drm_ttm.c
>index c1fc13f..a9d8733 100644
>--- a/linux-core/drm_ttm.c
>+++ b/linux-core/drm_ttm.c
>@@ -262,7 +262,6 @@ int drm_ttm_set_user(struct drm_ttm *ttm,
> {
> struct mm_struct *mm = tsk->mm;
> int ret;
>- int i;
> int write = (ttm->page_flags & DRM_TTM_PAGE_WRITE) != 0;
>
> BUG_ON(num_pages != ttm->num_pages);
>@@ -278,11 +277,6 @@ int drm_ttm_set_user(struct drm_ttm *ttm,
> return -ENOMEM;
> }
>
>- for (i = 0; i < num_pages; ++i) {
>- if (ttm->pages[i] == NULL)
>- ttm->pages[i] = ttm->dummy_read_page;
>- }
>-
> return 0;
> }
>
>@@ -304,12 +298,14 @@ int drm_ttm_populate(struct drm_ttm *ttm)
> return 0;
>
> be = ttm->be;
>- for (i = 0; i < ttm->num_pages; ++i) {
>- page = drm_ttm_get_page(ttm, i);
>- if (!page)
>- return -ENOMEM;
>+ if (ttm->page_flags & DRM_TTM_PAGE_WRITE) {
>+ for (i = 0; i < ttm->num_pages; ++i) {
>+ page = drm_ttm_get_page(ttm, i);
>+ if (!page)
>+ return -ENOMEM;
>+ }
> }
>- be->func->populate(be, ttm->num_pages, ttm->pages);
>+ be->func->populate(be, ttm->num_pages, ttm->pages, ttm->dummy_read_page);
> ttm->state = ttm_unbound;
> return 0;
> }
>diff --git a/linux-core/nouveau_sgdma.c b/linux-core/nouveau_sgdma.c
>index f3bf534..6c61819 100644
>--- a/linux-core/nouveau_sgdma.c
>+++ b/linux-core/nouveau_sgdma.c
>@@ -25,7 +25,7 @@ nouveau_sgdma_needs_ub_cache_adjust(struct drm_ttm_backend *be)
>
> static int
> nouveau_sgdma_populate(struct drm_ttm_backend *be, unsigned long num_pages,
>- struct page **pages)
>+ struct page **pages, struct page *dummy_read_page)
> {
> struct nouveau_sgdma_be *nvbe = (struct nouveau_sgdma_be *)be;
> int p, d, o;
>@@ -41,8 +41,11 @@ nouveau_sgdma_populate(struct drm_ttm_backend *be, unsigned long num_pages,
> nvbe->pages_populated = d = 0;
> for (p = 0; p < num_pages; p++) {
> for (o = 0; o < PAGE_SIZE; o += NV_CTXDMA_PAGE_SIZE) {
>+ struct page *page = pages[p];
>+ if (!page)
>+ page = dummy_read_page;
> nvbe->pagelist[d] = pci_map_page(nvbe->dev->pdev,
>- pages[p], o,
>+ page, o,
> NV_CTXDMA_PAGE_SIZE,
> PCI_DMA_BIDIRECTIONAL);
> if (pci_dma_mapping_error(nvbe->pagelist[d])) {
>@@ -299,7 +302,7 @@ nouveau_sgdma_nottm_hack_init(struct drm_device *dev)
> }
> dev_priv->gart_info.sg_handle = sgreq.handle;
>
>- if ((ret = be->func->populate(be, dev->sg->pages, dev->sg->pagelist))) {
>+ if ((ret = be->func->populate(be, dev->sg->pages, dev->sg->pagelist, dev->bm.dummy_read_page))) {
> DRM_ERROR("failed populate: %d\n", ret);
> return ret;
> }
>
>
>
>------------------------------------------------------------------------
>
>-------------------------------------------------------------------------
>SF.Net email is sponsored by:
>Check out the new SourceForge.net Marketplace.
>It's the best place to buy or sell services
>for just about anything Open Source.
>http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
>
>------------------------------------------------------------------------
>
>--
>_______________________________________________
>Dri-devel mailing list
>Dri...@li...
>https://lists.sourceforge.net/lists/listinfo/dri-devel
>
>
|