[Beignet] [PATCH] Make __local on Haswell an error, instead of silently doing nothing

[Zhigang Gong]

On Tue, Apr 28, 2015 at 07:36:39PM +0100, Rebecca N. Palmer wrote:
> Given that really fixing shared local memory on Haswell is likely
> to take some time, at least make it an explicit error that points
> the user to the kernel patch, instead of returning "success"
> without actually doing the computation.
> 
> As I can't find an error code to check for this problem (if there
> was one I'd guess at drm_intel_gem_bo_context_exec(), but we
> already check that), this unconditionally rejects __local use on
> Haswell, so you'd have to undo it to use the kernel patch.
> Is there a better way to check this? or should we make it a warning
> instead of an error (omit "return CL_OUT_OF_RESOURCES;")?
> 
> Also, improve the drm_intel_gem_bo_context_exec() error message.
> 
> Warning: not properly tested, as I don't have the affected hardware;
> if you prefer not to apply this to development trunk, I'd still
> appreciate confirmation of whether it works.
> 
> Signed-off-by: Rebecca Palmer <rebecca_palmer at zoho.com>
> 
> --- a/src/cl_command_queue_gen7.c
> +++ b/src/cl_command_queue_gen7.c
> @@ -351,6 +351,10 @@ cl_command_queue_ND_range_gen7(cl_comman
>    /* Curbe step 1: fill the constant urb buffer data shared by all threads */
>    if (ker->curbe) {
>      kernel.slm_sz = cl_curbe_fill(ker, work_dim, global_wk_off, global_wk_sz, local_wk_sz, thread_n);
> +    if (kernel.slm_sz > 0 && cl_driver_get_ver(ctx->drv) == 75){
> +      fprintf(stderr, "Beignet: Shared local memory does not work on Haswell, see https://01.org/zh/beignet/downloads/linux-kernel-patch-hsw-support\n");
> +      return CL_OUT_OF_RESOURCES;
> +    }
How about an already patched kernel is running? I just proposed a
solution in previous email, could you review and give comments?

>      if (kernel.slm_sz > ker->program->ctx->device->local_mem_size) {
>        fprintf(stderr, "Beignet: Out of shared local memory %d.\n", kernel.slm_sz);
>        return CL_OUT_OF_RESOURCES;
> --- a/src/intel/intel_batchbuffer.c
> +++ b/src/intel/intel_batchbuffer.c
> @@ -135,6 +135,10 @@ intel_batchbuffer_flush(intel_batchbuffe
>    }
>    if (drm_intel_gem_bo_context_exec(batch->buffer, batch->intel->ctx, used, flag) < 0) {
>      fprintf(stderr, "drm_intel_gem_bo_context_exec() failed: %s\n", strerror(errno));
> +    if (errno == EINVAL && IS_GEN75(batch->intel->device_id)) {
> +      fprintf(stderr, "This is a known bug on Haswell systems, see http://www.freedesktop.org/wiki/Software/Beignet/\n"
> +                "'sudo echo 0 > /sys/module/i915/parameters/enable_cmd_parser' usually helps\n");
> +    }
The above check is also valid for non HSW platform. For example some 3.15/3.16 kernel for all
platforms. So you may simply remove the "&& IS_GEN75(batch->intel->device_id)" and correct the
error message as well.

Thanks,
Zhigang Gong.


>      err = -1;
>    }
> 
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet