[Beignet] [PATCH] Fix AUX buffer for really page aligned

[Guo, Yejun]

three comments in line, thanks.

-----Original Message-----
From: Beignet [mailto:beignet-bounces at lists.freedesktop.org] On Behalf Of Zhenyu Wang
Sent: Wednesday, October 22, 2014 4:11 PM
To: beignet at lists.freedesktop.org
Subject: [Beignet] [PATCH] Fix AUX buffer for really page aligned

Apply ALIGN() for aux buffer size from beginning has no effect on final target size. Move to the end of all state offsets set for alignment.

Signed-off-by: Zhenyu Wang <zhenyuw at linux.intel.com>
---
 src/intel/intel_gpgpu.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/intel/intel_gpgpu.c b/src/intel/intel_gpgpu.c index 105a077..98b32bf 100644
--- a/src/intel/intel_gpgpu.c
+++ b/src/intel/intel_gpgpu.c
@@ -759,8 +759,6 @@ intel_gpgpu_state_init(intel_gpgpu_t *gpgpu,
     dri_bo_unreference(gpgpu->aux_buf.bo);
   gpgpu->aux_buf.bo = NULL;
 
-  //surface heap must be 4096 bytes aligned because state base address use 20bit for the address
-  size_aux = ALIGN(size_aux, 4096);
[Yejun] Yes, there is no effect at the beginning. Actually, the code here is to highlight the alignment requirement, so future maintainer knows it especially if he wants to move the layout from the beginning to the middle.

   gpgpu->aux_offset.surface_heap_offset = size_aux;
   size_aux += sizeof(surface_heap_t);
 
@@ -784,7 +782,10 @@ intel_gpgpu_state_init(intel_gpgpu_t *gpgpu,
   gpgpu->aux_offset.sampler_border_color_state_offset = size_aux;
   size_aux += GEN_MAX_SAMPLERS * sizeof(gen7_sampler_border_color_t);

[Yejun] aux buffer contains several parts, each part has its own alignment requirement, so we can see several 'ALIGN'  in above code. The alignment is handled for each part, it is not feasible to move one/all of them to the last.

 
-  bo = dri_bo_alloc(gpgpu->drv->bufmgr, "AUX_BUFFER", size_aux, 0);
+  //surface heap must be 4096 bytes aligned because state base address 
+ use 20bit for the address  size_aux = ALIGN(size_aux, 4096);
+
+  bo = dri_bo_alloc(gpgpu->drv->bufmgr, "AUX_BUFFER", size_aux, 4096);

[Yejun] the last parameter '4096' is to control the size of the whole buffer (to be page aligned), not to meet the align requirement of the base address, and it is not explicitly required and  is ignored in function drm_intel_gem_bo_alloc_internal. 

   if (!bo || dri_bo_map(bo, 1) != 0) {
     fprintf(stderr, "%s:%d: %s.\n", __FILE__, __LINE__, strerror(errno));
     if (bo)
--
2.1.1

_______________________________________________
Beignet mailing list
Beignet at lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/beignet