Re: [Mesa-dev] [PATCH v3] intel/ppgtt: memory address alignment

[Lionel Landwerlin]

Hey Sergii,

Sorry for the late answer.

For the sake of clarity, I would split the changes (4096->PAGE_SIZE) and 
the actual bug fix into 2 different patches.

I don't have see a problem with the PAGE_SIZE change.

Thanks a lot,

-
Lionel

On 25/07/18 14:24, Sergii Romantsov wrote:

Sorry,
do we have any objections about PAGE_SIZE usage instead of 4096?

And what do you think if, maybe, some auto Intel-internal tests to run 
with that patch?



On Wed, Jul 25, 2018 at 1:21 PM, Sergii Romantsov 
mailto:sergii.romant...@gmail.com>> wrote:


Kernel (for ppgtt) requires memory address to be
aligned to page size (4096).

-v2: added marking that also fixes initial commit 01058a552294.
-v3: numbers replaced by PAGE_SIZE; buffer-object size is aligned
instead of alignment of offsets (Chris Wilson).

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106997

Fixes: a363bb2cd0e2 (i965: Allocate VMA in userspace for
full-PPGTT systems.)
Fixes: 01058a552294 (i965: Add virtual memory allocator
infrastructure to brw_bufmgr.)
Signed-off-by: Sergii Romantsov mailto:sergii.romant...@globallogic.com>>
---
 src/mesa/drivers/dri/i965/brw_bufmgr.c | 19 +--
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.c
b/src/mesa/drivers/dri/i965/brw_bufmgr.c
index 09d45e3..66d7751 100644
--- a/src/mesa/drivers/dri/i965/brw_bufmgr.c
+++ b/src/mesa/drivers/dri/i965/brw_bufmgr.c
@@ -195,7 +195,7 @@ bo_tile_size(struct brw_bufmgr *bufmgr,
uint64_t size, uint32_t tiling)
       return size;

    /* 965+ just need multiples of page size for tiling */
-   return ALIGN(size, 4096);
+   return ALIGN(size, PAGE_SIZE);
 }

 /*
@@ -496,7 +496,6 @@ bo_alloc_internal(struct brw_bufmgr *bufmgr,
                   uint32_t stride)
 {
    struct brw_bo *bo;
-   unsigned int page_size = getpagesize();
    int ret;
    struct bo_cache_bucket *bucket;
    bool alloc_from_cache;
@@ -522,12 +521,12 @@ bo_alloc_internal(struct brw_bufmgr *bufmgr,
     * allocation up.
     */
    if (bucket == NULL) {
-      bo_size = size;
-      if (bo_size < page_size)
-         bo_size = page_size;
+      unsigned int page_size = getpagesize();
+      bo_size = ALIGN(size, page_size);
    } else {
       bo_size = bucket->size;
    }
+   assert(bo_size);

    mtx_lock(>lock);
    /* Get a buffer out of the cache if available */
@@ -1578,12 +1577,12 @@ init_cache_buckets(struct brw_bufmgr *bufmgr)
     * width/height alignment and rounding of sizes to pages will
     * get us useful cache hit rates anyway)
     */
-   add_bucket(bufmgr, 4096);
-   add_bucket(bufmgr, 4096 * 2);
-   add_bucket(bufmgr, 4096 * 3);
+   add_bucket(bufmgr, PAGE_SIZE);
+   add_bucket(bufmgr, PAGE_SIZE * 2);
+   add_bucket(bufmgr, PAGE_SIZE * 3);

    /* Initialize the linked lists for BO reuse cache. */
-   for (size = 4 * 4096; size <= cache_max_size; size *= 2) {
+   for (size = 4 * PAGE_SIZE; size <= cache_max_size; size *= 2) {
       add_bucket(bufmgr, size);

       add_bucket(bufmgr, size + size * 1 / 4);
@@ -1729,7 +1728,7 @@ brw_bufmgr_init(struct gen_device_info
*devinfo, int fd)
          bufmgr->initial_kflags |= EXEC_OBJECT_PINNED;

         
util_vma_heap_init(>vma_allocator[BRW_MEMZONE_LOW_4G],
-                            4096, _4GB);
+                            PAGE_SIZE, _4GB);
         
util_vma_heap_init(>vma_allocator[BRW_MEMZONE_OTHER],
                             1 * _4GB, gtt_size - 1 * _4GB);
       } else if (devinfo->gen >= 10) {
-- 
2.7.4


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org 
https://lists.freedesktop.org/mailman/listinfo/mesa-dev





--
Sergii Romantsov
GlobalLogic Inc.
www.globallogic.com 



___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v3] intel/ppgtt: memory address alignment

[Sergii Romantsov]

Hello, Kenneth and Lionel.
Maybe, could we make a final review?

On Wed, Jul 25, 2018 at 4:24 PM, Sergii Romantsov <
sergii.romant...@globallogic.com> wrote:

> Sorry,
> do we have any objections about PAGE_SIZE usage instead of 4096?
>
> And what do you think if, maybe, some auto Intel-internal tests to run
> with that patch?
>
>
> On Wed, Jul 25, 2018 at 1:21 PM, Sergii Romantsov <
> sergii.romant...@gmail.com> wrote:
>
>> Kernel (for ppgtt) requires memory address to be
>> aligned to page size (4096).
>>
>> -v2: added marking that also fixes initial commit 01058a552294.
>> -v3: numbers replaced by PAGE_SIZE; buffer-object size is aligned
>> instead of alignment of offsets (Chris Wilson).
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106997
>> Fixes: a363bb2cd0e2 (i965: Allocate VMA in userspace for full-PPGTT
>> systems.)
>> Fixes: 01058a552294 (i965: Add virtual memory allocator infrastructure to
>> brw_bufmgr.)
>> Signed-off-by: Sergii Romantsov 
>> ---
>>  src/mesa/drivers/dri/i965/brw_bufmgr.c | 19 +--
>>  1 file changed, 9 insertions(+), 10 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.c
>> b/src/mesa/drivers/dri/i965/brw_bufmgr.c
>> index 09d45e3..66d7751 100644
>> --- a/src/mesa/drivers/dri/i965/brw_bufmgr.c
>> +++ b/src/mesa/drivers/dri/i965/brw_bufmgr.c
>> @@ -195,7 +195,7 @@ bo_tile_size(struct brw_bufmgr *bufmgr, uint64_t
>> size, uint32_t tiling)
>>return size;
>>
>> /* 965+ just need multiples of page size for tiling */
>> -   return ALIGN(size, 4096);
>> +   return ALIGN(size, PAGE_SIZE);
>>  }
>>
>>  /*
>> @@ -496,7 +496,6 @@ bo_alloc_internal(struct brw_bufmgr *bufmgr,
>>uint32_t stride)
>>  {
>> struct brw_bo *bo;
>> -   unsigned int page_size = getpagesize();
>> int ret;
>> struct bo_cache_bucket *bucket;
>> bool alloc_from_cache;
>> @@ -522,12 +521,12 @@ bo_alloc_internal(struct brw_bufmgr *bufmgr,
>>  * allocation up.
>>  */
>> if (bucket == NULL) {
>> -  bo_size = size;
>> -  if (bo_size < page_size)
>> - bo_size = page_size;
>> +  unsigned int page_size = getpagesize();
>> +  bo_size = ALIGN(size, page_size);
>> } else {
>>bo_size = bucket->size;
>> }
>> +   assert(bo_size);
>>
>> mtx_lock(>lock);
>> /* Get a buffer out of the cache if available */
>> @@ -1578,12 +1577,12 @@ init_cache_buckets(struct brw_bufmgr *bufmgr)
>>  * width/height alignment and rounding of sizes to pages will
>>  * get us useful cache hit rates anyway)
>>  */
>> -   add_bucket(bufmgr, 4096);
>> -   add_bucket(bufmgr, 4096 * 2);
>> -   add_bucket(bufmgr, 4096 * 3);
>> +   add_bucket(bufmgr, PAGE_SIZE);
>> +   add_bucket(bufmgr, PAGE_SIZE * 2);
>> +   add_bucket(bufmgr, PAGE_SIZE * 3);
>>
>> /* Initialize the linked lists for BO reuse cache. */
>> -   for (size = 4 * 4096; size <= cache_max_size; size *= 2) {
>> +   for (size = 4 * PAGE_SIZE; size <= cache_max_size; size *= 2) {
>>add_bucket(bufmgr, size);
>>
>>add_bucket(bufmgr, size + size * 1 / 4);
>> @@ -1729,7 +1728,7 @@ brw_bufmgr_init(struct gen_device_info *devinfo,
>> int fd)
>>   bufmgr->initial_kflags |= EXEC_OBJECT_PINNED;
>>
>>   util_vma_heap_init(>vma_allocator[BRW_MEMZONE_LOW_4G],
>> -4096, _4GB);
>> +PAGE_SIZE, _4GB);
>>   util_vma_heap_init(>vma_allocator[BRW_MEMZONE_OTHER],
>>  1 * _4GB, gtt_size - 1 * _4GB);
>>} else if (devinfo->gen >= 10) {
>> --
>> 2.7.4
>>
>> ___
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>
>
>
> --
> Sergii Romantsov
> GlobalLogic Inc.
> www.globallogic.com
>



-- 
Sergii Romantsov
GlobalLogic Inc.
www.globallogic.com
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v3] intel/ppgtt: memory address alignment

[Sergii Romantsov]

Sorry,
do we have any objections about PAGE_SIZE usage instead of 4096?

And what do you think if, maybe, some auto Intel-internal tests to run with
that patch?


On Wed, Jul 25, 2018 at 1:21 PM, Sergii Romantsov <
sergii.romant...@gmail.com> wrote:

> Kernel (for ppgtt) requires memory address to be
> aligned to page size (4096).
>
> -v2: added marking that also fixes initial commit 01058a552294.
> -v3: numbers replaced by PAGE_SIZE; buffer-object size is aligned
> instead of alignment of offsets (Chris Wilson).
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106997
> Fixes: a363bb2cd0e2 (i965: Allocate VMA in userspace for full-PPGTT
> systems.)
> Fixes: 01058a552294 (i965: Add virtual memory allocator infrastructure to
> brw_bufmgr.)
> Signed-off-by: Sergii Romantsov 
> ---
>  src/mesa/drivers/dri/i965/brw_bufmgr.c | 19 +--
>  1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.c
> b/src/mesa/drivers/dri/i965/brw_bufmgr.c
> index 09d45e3..66d7751 100644
> --- a/src/mesa/drivers/dri/i965/brw_bufmgr.c
> +++ b/src/mesa/drivers/dri/i965/brw_bufmgr.c
> @@ -195,7 +195,7 @@ bo_tile_size(struct brw_bufmgr *bufmgr, uint64_t size,
> uint32_t tiling)
>return size;
>
> /* 965+ just need multiples of page size for tiling */
> -   return ALIGN(size, 4096);
> +   return ALIGN(size, PAGE_SIZE);
>  }
>
>  /*
> @@ -496,7 +496,6 @@ bo_alloc_internal(struct brw_bufmgr *bufmgr,
>uint32_t stride)
>  {
> struct brw_bo *bo;
> -   unsigned int page_size = getpagesize();
> int ret;
> struct bo_cache_bucket *bucket;
> bool alloc_from_cache;
> @@ -522,12 +521,12 @@ bo_alloc_internal(struct brw_bufmgr *bufmgr,
>  * allocation up.
>  */
> if (bucket == NULL) {
> -  bo_size = size;
> -  if (bo_size < page_size)
> - bo_size = page_size;
> +  unsigned int page_size = getpagesize();
> +  bo_size = ALIGN(size, page_size);
> } else {
>bo_size = bucket->size;
> }
> +   assert(bo_size);
>
> mtx_lock(>lock);
> /* Get a buffer out of the cache if available */
> @@ -1578,12 +1577,12 @@ init_cache_buckets(struct brw_bufmgr *bufmgr)
>  * width/height alignment and rounding of sizes to pages will
>  * get us useful cache hit rates anyway)
>  */
> -   add_bucket(bufmgr, 4096);
> -   add_bucket(bufmgr, 4096 * 2);
> -   add_bucket(bufmgr, 4096 * 3);
> +   add_bucket(bufmgr, PAGE_SIZE);
> +   add_bucket(bufmgr, PAGE_SIZE * 2);
> +   add_bucket(bufmgr, PAGE_SIZE * 3);
>
> /* Initialize the linked lists for BO reuse cache. */
> -   for (size = 4 * 4096; size <= cache_max_size; size *= 2) {
> +   for (size = 4 * PAGE_SIZE; size <= cache_max_size; size *= 2) {
>add_bucket(bufmgr, size);
>
>add_bucket(bufmgr, size + size * 1 / 4);
> @@ -1729,7 +1728,7 @@ brw_bufmgr_init(struct gen_device_info *devinfo, int
> fd)
>   bufmgr->initial_kflags |= EXEC_OBJECT_PINNED;
>
>   util_vma_heap_init(>vma_allocator[BRW_MEMZONE_LOW_4G],
> -4096, _4GB);
> +PAGE_SIZE, _4GB);
>   util_vma_heap_init(>vma_allocator[BRW_MEMZONE_OTHER],
>  1 * _4GB, gtt_size - 1 * _4GB);
>} else if (devinfo->gen >= 10) {
> --
> 2.7.4
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>



-- 
Sergii Romantsov
GlobalLogic Inc.
www.globallogic.com
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH v3] intel/ppgtt: memory address alignment

[Sergii Romantsov]

Kernel (for ppgtt) requires memory address to be
aligned to page size (4096).

-v2: added marking that also fixes initial commit 01058a552294.
-v3: numbers replaced by PAGE_SIZE; buffer-object size is aligned
instead of alignment of offsets (Chris Wilson).

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106997
Fixes: a363bb2cd0e2 (i965: Allocate VMA in userspace for full-PPGTT systems.)
Fixes: 01058a552294 (i965: Add virtual memory allocator infrastructure to 
brw_bufmgr.)
Signed-off-by: Sergii Romantsov 
---
 src/mesa/drivers/dri/i965/brw_bufmgr.c | 19 +--
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.c 
b/src/mesa/drivers/dri/i965/brw_bufmgr.c
index 09d45e3..66d7751 100644
--- a/src/mesa/drivers/dri/i965/brw_bufmgr.c
+++ b/src/mesa/drivers/dri/i965/brw_bufmgr.c
@@ -195,7 +195,7 @@ bo_tile_size(struct brw_bufmgr *bufmgr, uint64_t size, 
uint32_t tiling)
   return size;
 
/* 965+ just need multiples of page size for tiling */
-   return ALIGN(size, 4096);
+   return ALIGN(size, PAGE_SIZE);
 }
 
 /*
@@ -496,7 +496,6 @@ bo_alloc_internal(struct brw_bufmgr *bufmgr,
   uint32_t stride)
 {
struct brw_bo *bo;
-   unsigned int page_size = getpagesize();
int ret;
struct bo_cache_bucket *bucket;
bool alloc_from_cache;
@@ -522,12 +521,12 @@ bo_alloc_internal(struct brw_bufmgr *bufmgr,
 * allocation up.
 */
if (bucket == NULL) {
-  bo_size = size;
-  if (bo_size < page_size)
- bo_size = page_size;
+  unsigned int page_size = getpagesize();
+  bo_size = ALIGN(size, page_size);
} else {
   bo_size = bucket->size;
}
+   assert(bo_size);
 
mtx_lock(>lock);
/* Get a buffer out of the cache if available */
@@ -1578,12 +1577,12 @@ init_cache_buckets(struct brw_bufmgr *bufmgr)
 * width/height alignment and rounding of sizes to pages will
 * get us useful cache hit rates anyway)
 */
-   add_bucket(bufmgr, 4096);
-   add_bucket(bufmgr, 4096 * 2);
-   add_bucket(bufmgr, 4096 * 3);
+   add_bucket(bufmgr, PAGE_SIZE);
+   add_bucket(bufmgr, PAGE_SIZE * 2);
+   add_bucket(bufmgr, PAGE_SIZE * 3);
 
/* Initialize the linked lists for BO reuse cache. */
-   for (size = 4 * 4096; size <= cache_max_size; size *= 2) {
+   for (size = 4 * PAGE_SIZE; size <= cache_max_size; size *= 2) {
   add_bucket(bufmgr, size);
 
   add_bucket(bufmgr, size + size * 1 / 4);
@@ -1729,7 +1728,7 @@ brw_bufmgr_init(struct gen_device_info *devinfo, int fd)
  bufmgr->initial_kflags |= EXEC_OBJECT_PINNED;
 
  util_vma_heap_init(>vma_allocator[BRW_MEMZONE_LOW_4G],
-4096, _4GB);
+PAGE_SIZE, _4GB);
  util_vma_heap_init(>vma_allocator[BRW_MEMZONE_OTHER],
 1 * _4GB, gtt_size - 1 * _4GB);
   } else if (devinfo->gen >= 10) {
-- 
2.7.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev