Re: [PATCH 02/12] drm/ttm: remove split btw highmen and lowmem page

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Tue, Nov 08, 2011 at 08:56:17AM +0100, Thomas Hellstrom wrote:
> On 11/08/2011 12:40 AM, j.glisse@gmail.com wrote:
> >From: Jerome Glisse<jglisse@redhat.com>
> >
> >Split btw highmem and lowmem page was rendered useless by the
> >pool code. Remove it.
> 
> Actually it was introduced so that we could call set_pages_array_xx
> when changing caching attributes of an existing TTM, since the
> set_pages_array_xx functions complained about highmem pages.
> 
> That should've been fixed now in the pageattr code, though.

Yup (from __change_page_attr):

        if (cpa->flags & CPA_PAGES_ARRAY) {
                struct page *page = cpa->pages[cpa->curpage];
                if (unlikely(PageHighMem(page)))
                        return 0;

> 
> Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
> 
> 
> >  Note further cleanup would change the
> >ttm page allocation helper to actualy take an array instead
> >of relying on list this could drasticly reduce the number of
> >function call in the common case of allocation whole buffer.
> >
> >Signed-off-by: Jerome Glisse<jglisse@redhat.com>
> >Reviewed-by: Konrad Rzeszutek Wilk<konrad.wilk@oracle.com>
> >---
> >  drivers/gpu/drm/ttm/ttm_tt.c    |   11 ++---------
> >  include/drm/ttm/ttm_bo_driver.h |    7 -------
> >  2 files changed, 2 insertions(+), 16 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
> >index 82a1161..8b7a6d0 100644
> >--- a/drivers/gpu/drm/ttm/ttm_tt.c
> >+++ b/drivers/gpu/drm/ttm/ttm_tt.c
> >@@ -69,7 +69,7 @@ static struct page *__ttm_tt_get_page(struct ttm_tt *ttm, int index)
> >  	struct ttm_mem_global *mem_glob = ttm->glob->mem_glob;
> >  	int ret;
> >
> >-	while (NULL == (p = ttm->pages[index])) {
> >+	if (NULL == (p = ttm->pages[index])) {
> >
> >  		INIT_LIST_HEAD(&h);
> >
> >@@ -85,10 +85,7 @@ static struct page *__ttm_tt_get_page(struct ttm_tt *ttm, int index)
> >  		if (unlikely(ret != 0))
> >  			goto out_err;
> >
> >-		if (PageHighMem(p))
> >-			ttm->pages[--ttm->first_himem_page] = p;
> >-		else
> >-			ttm->pages[++ttm->last_lomem_page] = p;
> >+		ttm->pages[index] = p;
> >  	}
> >  	return p;
> >  out_err:
> >@@ -270,8 +267,6 @@ static void ttm_tt_free_alloced_pages(struct ttm_tt *ttm)
> >  	ttm_put_pages(&h, count, ttm->page_flags, ttm->caching_state,
> >  		      ttm->dma_address);
> >  	ttm->state = tt_unpopulated;
> >-	ttm->first_himem_page = ttm->num_pages;
> >-	ttm->last_lomem_page = -1;
> >  }
> >
> >  void ttm_tt_destroy(struct ttm_tt *ttm)
> >@@ -315,8 +310,6 @@ struct ttm_tt *ttm_tt_create(struct ttm_bo_device *bdev, unsigned long size,
> >
> >  	ttm->glob = bdev->glob;
> >  	ttm->num_pages = (size + PAGE_SIZE - 1)>>  PAGE_SHIFT;
> >-	ttm->first_himem_page = ttm->num_pages;
> >-	ttm->last_lomem_page = -1;
> >  	ttm->caching_state = tt_cached;
> >  	ttm->page_flags = page_flags;
> >
> >diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> >index 37527d6..9da182b 100644
> >--- a/include/drm/ttm/ttm_bo_driver.h
> >+++ b/include/drm/ttm/ttm_bo_driver.h
> >@@ -136,11 +136,6 @@ enum ttm_caching_state {
> >   * @dummy_read_page: Page to map where the ttm_tt page array contains a NULL
> >   * pointer.
> >   * @pages: Array of pages backing the data.
> >- * @first_himem_page: Himem pages are put last in the page array, which
> >- * enables us to run caching attribute changes on only the first part
> >- * of the page array containing lomem pages. This is the index of the
> >- * first himem page.
> >- * @last_lomem_page: Index of the last lomem page in the page array.
> >   * @num_pages: Number of pages in the page array.
> >   * @bdev: Pointer to the current struct ttm_bo_device.
> >   * @be: Pointer to the ttm backend.
> >@@ -157,8 +152,6 @@ enum ttm_caching_state {
> >  struct ttm_tt {
> >  	struct page *dummy_read_page;
> >  	struct page **pages;
> >-	long first_himem_page;
> >-	long last_lomem_page;
> >  	uint32_t page_flags;
> >  	unsigned long num_pages;
> >  	struct ttm_bo_global *glob;