[Beignet] [PATCH] Gracefully handle unsupported systems

[Mario Kicherer]

It worked for me when I enabled only my dedicated GPU in my Optimus laptop
and the IvyBridge GPU was not visible. But I agree that signaling an error
to the caller is safer. I'll try to change that later.

Best regards,
Mario

On Wednesday 29 May 2013 12:18:21 Zhigang Gong wrote:
> 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);
> >    
> >    }