[Beignet] [PATCH 1/2] fix min_max_read_image_args and min_max_parameter_size issue.
Zhigang Gong
zhigang.gong at linux.intel.com
Wed Dec 24 22:22:02 PST 2014
This patchset LGTM, will push latter. Thanks.
On Thu, Dec 25, 2014 at 10:25:34AM +0800, xionghu.luo at intel.com wrote:
> From: Luo Xionghu <xionghu.luo at intel.com>
>
> this patch revert fb4bced99b7c08d0d43386abf33448860fb7fc41 as the spec
> defined the min_max_parameter_size's min value is 1024;
> the BTI_MAX_NUM and btiBase could be 130 because of 128 images with 1
> const surface and 1 private surface.
>
> v2: add BTI_MAX_READ_IMAGE_ARGS and BTI_MAX_WRITE_IMAGE_ARGS in backend.
> change the BTI_MAX_ID to 253. the image numbers will be calculated in
> later patch and check its limitation.
>
> Signed-off-by: Luo Xionghu <xionghu.luo at intel.com>
> ---
> backend/src/backend/gen_insn_selection.cpp | 4 ++--
> backend/src/backend/program.h | 6 ++++--
> src/cl_command_queue.c | 2 +-
> src/cl_gen75_device.h | 2 +-
> src/cl_gen7_device.h | 2 +-
> src/cl_gt_device.h | 4 ++--
> src/cl_kernel.c | 1 +
> src/intel/intel_gpgpu.c | 2 +-
> 8 files changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/backend/src/backend/gen_insn_selection.cpp b/backend/src/backend/gen_insn_selection.cpp
> index adff091..6cd38cd 100644
> --- a/backend/src/backend/gen_insn_selection.cpp
> +++ b/backend/src/backend/gen_insn_selection.cpp
> @@ -3998,8 +3998,8 @@ namespace gbe
> msgLen = srcNum;
> }
> // We switch to a fixup bti for linear filter on a image1d array sampling.
> - uint32_t bti = insn.getImageIndex() + (insn.getSamplerOffset() == 2 ? BTI_MAX_IMAGE_NUM : 0);
> - if (bti > 253) {
> + uint32_t bti = insn.getImageIndex() + (insn.getSamplerOffset() == 2 ? BTI_WORKAROUND_IMAGE_OFFSET : 0);
> + if (bti > BTI_MAX_ID) {
> std::cerr << "Too large bti " << bti;
> return false;
> }
> diff --git a/backend/src/backend/program.h b/backend/src/backend/program.h
> index a457d52..6acc010 100644
> --- a/backend/src/backend/program.h
> +++ b/backend/src/backend/program.h
> @@ -66,8 +66,10 @@ enum gbe_get_arg_info_value {
> #define BTI_CONSTANT 0
> #define BTI_PRIVATE 1
> #define BTI_RESERVED_NUM 2
> -#define BTI_MAX_IMAGE_NUM 128
> -#define BTI_MAX_ID (BTI_MAX_IMAGE_NUM + BTI_RESERVED_NUM - 1)
> +#define BTI_MAX_READ_IMAGE_ARGS 128
> +#define BTI_MAX_WRITE_IMAGE_ARGS 8
> +#define BTI_WORKAROUND_IMAGE_OFFSET 128
> +#define BTI_MAX_ID 253
>
> /*! Constant buffer values (ie values to setup in the constant buffer) */
> enum gbe_curbe_type {
> diff --git a/src/cl_command_queue.c b/src/cl_command_queue.c
> index 12530d7..093bf07 100644
> --- a/src/cl_command_queue.c
> +++ b/src/cl_command_queue.c
> @@ -146,7 +146,7 @@ cl_command_queue_bind_image(cl_command_queue queue, cl_kernel k)
> // TODO, this workaround is for GEN7/GEN75 only, we may need to do it in the driver layer
> // on demand.
> if (image->image_type == CL_MEM_OBJECT_IMAGE1D_ARRAY)
> - cl_gpgpu_bind_image(gpgpu, k->images[i].idx + BTI_MAX_IMAGE_NUM, image->base.bo, image->offset,
> + cl_gpgpu_bind_image(gpgpu, k->images[i].idx + BTI_WORKAROUND_IMAGE_OFFSET, image->base.bo, image->offset,
> image->intel_fmt, image->image_type,
> image->w, image->h, image->depth,
> image->row_pitch, (cl_gpgpu_tiling)image->tiling);
> diff --git a/src/cl_gen75_device.h b/src/cl_gen75_device.h
> index 768721d..8cf2dcd 100644
> --- a/src/cl_gen75_device.h
> +++ b/src/cl_gen75_device.h
> @@ -19,7 +19,7 @@
>
> /* Common fields for both SNB devices (either GT1 or GT2)
> */
> -.max_parameter_size = sizeof(cl_mem) * 128,
> +.max_parameter_size = 1024,
> .global_mem_cache_line_size = 128, /* XXX */
> .global_mem_cache_size = 8 << 10, /* XXX */
> .local_mem_type = CL_GLOBAL,
> diff --git a/src/cl_gen7_device.h b/src/cl_gen7_device.h
> index 5197817..6857f8a 100644
> --- a/src/cl_gen7_device.h
> +++ b/src/cl_gen7_device.h
> @@ -18,7 +18,7 @@
> */
>
> /* Common fields for both IVB devices (either GT1 or GT2) */
> -.max_parameter_size = sizeof(cl_mem) * 128,
> +.max_parameter_size = 1024,
> .global_mem_cache_line_size = 128, /* XXX */
> .global_mem_cache_size = 8 << 10, /* XXX */
> .local_mem_type = CL_GLOBAL,
> diff --git a/src/cl_gt_device.h b/src/cl_gt_device.h
> index 4faa15a..69aff76 100644
> --- a/src/cl_gt_device.h
> +++ b/src/cl_gt_device.h
> @@ -42,8 +42,8 @@
> .address_bits = 32,
> .max_mem_alloc_size = 256 * 1024 * 1024,
> .image_support = CL_TRUE,
> -.max_read_image_args = 128,
> -.max_write_image_args = 8,
> +.max_read_image_args = BTI_MAX_READ_IMAGE_ARGS,
> +.max_write_image_args = BTI_MAX_WRITE_IMAGE_ARGS,
> .image_max_array_size = 2048,
> .image2d_max_width = 8192,
> .image2d_max_height = 8192,
> diff --git a/src/cl_kernel.c b/src/cl_kernel.c
> index 177cb00..331d250 100644
> --- a/src/cl_kernel.c
> +++ b/src/cl_kernel.c
> @@ -336,6 +336,7 @@ cl_kernel_setup(cl_kernel k, gbe_kernel opaque)
> /* Get image data & size */
> k->image_sz = interp_kernel_get_image_size(k->opaque);
> assert(k->sampler_sz <= GEN_MAX_SURFACES);
> + assert(k->image_sz <= ctx->device->max_read_image_args + ctx->device->max_write_image_args);
> if (k->image_sz > 0) {
> TRY_ALLOC_NO_ERR(k->images, cl_calloc(k->image_sz, sizeof(k->images[0])));
> interp_kernel_get_image_data(k->opaque, k->images);
> diff --git a/src/intel/intel_gpgpu.c b/src/intel/intel_gpgpu.c
> index c80a11b..ecb4fb5 100644
> --- a/src/intel/intel_gpgpu.c
> +++ b/src/intel/intel_gpgpu.c
> @@ -1031,7 +1031,7 @@ static uint32_t get_surface_type(intel_gpgpu_t *gpgpu, int index, cl_mem_object_
> if (((IS_IVYBRIDGE(gpgpu->drv->device_id) ||
> IS_HASWELL(gpgpu->drv->device_id) ||
> IS_BROADWELL(gpgpu->drv->device_id))) &&
> - index >= BTI_MAX_IMAGE_NUM + BTI_RESERVED_NUM &&
> + index >= BTI_WORKAROUND_IMAGE_OFFSET + BTI_RESERVED_NUM &&
> type == CL_MEM_OBJECT_IMAGE1D_ARRAY)
> surface_type = I965_SURFACE_2D;
> else
> --
> 1.7.9.5
>
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet
More information about the Beignet
mailing list