If the purpose is to be a note that the whole aux buf should be page aligned, I agree with it. Could you modify the code/comment accordingly, thanks.
-----Original Message----- From: Zhenyu Wang [mailto:[email protected]] Sent: Thursday, October 23, 2014 10:36 AM To: Guo, Yejun Cc: [email protected] Subject: Re: [Beignet] [PATCH] Fix AUX buffer for really page aligned On 2014.10.22 08:49:53 +0000, Guo, Yejun wrote: > three comments in line, thanks. > Thanks for review this. > 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. > The goal is to make sure final aux buffer size is page aligned after adding up all required aligned space size. My patch only counts on final size. > > - 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. > It's for 'alignment' parameter. Yes current libdrm will optimize it as to be page aligned, but as your comment above it's a good note to say that we need aux buffer allocation to be page aligned, like for other objects, printf buf, timestamp, etc. -- Open Source Technology Center, Intel ltd. $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827 _______________________________________________ Beignet mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/beignet
