[Beignet] [PATCH] Gracefully handle unsupported systems

[Zhigang Gong]

Hi,

The first part LGTM, I will push it latter.
As to the second part, I agree that call exit is not good in a library internal,
but we can't just remove the exit, otherwise the application may get a "valid"
context even when it fails to open the device. Would you like to refine the second
part of this patch? Maybe we need to change the prototype of intel_driver_open and
handle the error checking more gracefully.

On Tue, May 28, 2013 at 08:25:39PM +0200, Mario Kicherer wrote:
> Gracefully handle cases with unsupported or unreachable GPUs. A library
> should not call exit(). Improved error handling in cl_get_device_ids.
> 
> Signed-off-by: Mario Kicherer <dev at kicherer.org>
> ---
>  src/cl_device_id.c       | 50 ++++++++++++++++++++++++++++++++----------------
>  src/intel/intel_driver.c |  3 ---
>  2 files changed, 34 insertions(+), 19 deletions(-)
> 
> diff --git a/src/cl_device_id.c b/src/cl_device_id.c
> index 136f3b1..50dc9b6 100644
> --- a/src/cl_device_id.c
> +++ b/src/cl_device_id.c
> @@ -105,30 +105,48 @@ cl_get_device_ids(cl_platform_id    platform,
>                    cl_device_id *    devices,
>                    cl_uint *         num_devices)
>  {
> +  cl_device_id device;
> +  
>    /* Check parameter consistency */
> -  if (UNLIKELY(num_entries == 0 && devices == NULL && num_devices == NULL))
> -    return CL_SUCCESS;
>    if (UNLIKELY(devices == NULL && num_devices == NULL))
> -    return CL_INVALID_VALUE;
> -  if (UNLIKELY(platform != NULL && platform != intel_platform))
> +    return CL_SUCCESS;
> +  if (UNLIKELY(platform != intel_platform))
>      return CL_INVALID_PLATFORM;
> -  if (num_devices && (device_type == CL_DEVICE_TYPE_CPU)) {
> -    *num_devices = 0;
> -    return CL_SUCCESS;	
> +  if (UNLIKELY(
> +      device_type != CL_DEVICE_TYPE_GPU &&
> +      device_type != CL_DEVICE_TYPE_DEFAULT &&
> +      device_type != CL_DEVICE_TYPE_ALL
> +    ))
> +  {
> +    if (num_devices)
> +      *num_devices = 0;
> +    
> +    if (device_type == CL_DEVICE_TYPE_CPU ||
> +        device_type == CL_DEVICE_TYPE_ACCELERATOR
> +       )
> +       return CL_DEVICE_NOT_FOUND;
> +    else
> +       return CL_INVALID_DEVICE_TYPE;
>    }
> +  if (UNLIKELY(devices && num_entries == 0))
> +    return CL_INVALID_VALUE;
>  
> -  /* Detect our device (reject a non intel one or gen<6) */
> -  if (devices && UNLIKELY((*devices = cl_get_gt_device()) != NULL)) {
> +  /* Do we have a usable device? */
> +  device = cl_get_gt_device();
> +  if (!device) {
>      if (num_devices)
> -      *num_devices = 1;
> -
> -    (*devices)->extensions = intel_platform->extensions;
> -    (*devices)->extensions_sz = intel_platform->extensions_sz;
> -    return CL_SUCCESS;
> -  }
> -  else {
> +      *num_devices = 0;
> +    if (devices)
> +      *devices = 0;
> +    return CL_DEVICE_NOT_FOUND;
> +  } else {
>      if (num_devices)
>        *num_devices = 1;
> +    if (devices) {
> +      *devices = device;
> +      (*devices)->extensions = intel_platform->extensions;
> +      (*devices)->extensions_sz = intel_platform->extensions_sz;
> +    }
>      return CL_SUCCESS;
>    }
>  }
> diff --git a/src/intel/intel_driver.c b/src/intel/intel_driver.c
> index ebc4961..01e0a3a 100644
> --- a/src/intel/intel_driver.c
> +++ b/src/intel/intel_driver.c
> @@ -174,7 +174,6 @@ intel_driver_open(intel_driver_t *intel, cl_context_prop props)
>        && props->gl_type != CL_GL_GLX_DISPLAY
>        && props->gl_type != CL_GL_EGL_DISPLAY) {
>      printf("Unsupported gl share type %d.\n", props->gl_type);
> -    exit(-1);
>    }
>  
>    intel->x11_display = XOpenDisplay(NULL);
> @@ -203,7 +202,6 @@ intel_driver_open(intel_driver_t *intel, cl_context_prop props)
>    }
>    if(!intel_driver_is_active(intel)) {
>      printf("Device open failed\n");
> -    exit(-1);
>    }
>  
>  #if defined(HAS_GBM) && defined(HAS_EGL)
> @@ -211,7 +209,6 @@ intel_driver_open(intel_driver_t *intel, cl_context_prop props)
>      intel->gbm = gbm_create_device(intel->fd);
>      if (intel->gbm == NULL) {
>        printf("GBM device create failed.\n");
> -      exit(-1);
>      }
>      cl_gbm_set_image_extension(intel->gbm, (void*)props->egl_display);
>    }
> -- 
> 1.8.1.5
> 
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet