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
signature.asc
Description: Digital signature
_______________________________________________ Beignet mailing list Beignet@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/beignet
