One comment. > -----Original Message----- > From: Beignet [mailto:beignet-boun...@lists.freedesktop.org] On Behalf Of > junyan...@inbox.com > Sent: Friday, July 3, 2015 18:25 > To: beignet@lists.freedesktop.org > Cc: Junyan He > Subject: [Beignet] [PATCH 1/2] runtime: Use cl_get_platform_default to > replace global value. > > From: Junyan He <junyan...@linux.intel.com> > > The init order of the intel_platform and the intel_extension is somehow not > clear. When some API such as clGetDeviceIDs can pass NULL as > cl_platform_id, we just use the global value intel_platform as the default but > do not care about the init state of the extension. > The init of the extension may be done when the cl device is created. > This is OK if the paltform and the device have the same extensions. > But now because of the fp16, they are not always the same. > Use cl_get_platform_default to replace the global value to ensure that when > default platform is available, the extension is also inited. > > Signed-off-by: Junyan He <junyan...@linux.intel.com> > --- > src/cl_api.c | 6 +++--- > src/cl_context.c | 2 +- > src/cl_device_id.c | 55 +++++++++++++++++++++------------------------------ > - > src/cl_extensions.c | 38 +++++++++++++++++------------------- > src/cl_platform_id.c | 28 +++++++++++++++++--------- src/cl_platform_id.h > | 4 ++-- > 6 files changed, 65 insertions(+), 68 deletions(-) > > diff --git a/src/cl_api.c b/src/cl_api.c index 3e72deb..1ba775f 100644 > --- a/src/cl_api.c > +++ b/src/cl_api.c > @@ -195,7 +195,7 @@ clGetPlatformInfo(cl_platform_id platform, > size_t * param_value_size_ret) > { > /* Only one platform. This is easy */ > - if (UNLIKELY(platform != NULL && platform != intel_platform)) > + if (UNLIKELY(platform != NULL && platform != > + cl_get_platform_default())) > return CL_INVALID_PLATFORM; > > return cl_get_platform_info(platform, @@ -217,7 +217,7 @@ > clGetDeviceIDs(cl_platform_id platform, > /* Check parameter consistency */ > if (UNLIKELY(devices == NULL && num_devices == NULL)) > return CL_INVALID_VALUE; > - if (UNLIKELY(platform && platform != intel_platform)) > + if (UNLIKELY(platform && platform != cl_get_platform_default())) > return CL_INVALID_PLATFORM; > if (UNLIKELY(devices && num_entries == 0)) > return CL_INVALID_VALUE; > @@ -3193,7 +3193,7 @@ void* > clGetExtensionFunctionAddressForPlatform(cl_platform_id platform, > const char *func_name) { > - if (UNLIKELY(platform != NULL && platform != intel_platform)) > + if (UNLIKELY(platform != NULL && platform != > + cl_get_platform_default())) > return NULL; > return internal_clGetExtensionFunctionAddress(func_name); > } > diff --git a/src/cl_context.c b/src/cl_context.c index 773f545..45064ad 100644 > --- a/src/cl_context.c > +++ b/src/cl_context.c > @@ -68,7 +68,7 @@ cl_context_properties_process(const > cl_context_properties *prop, > case CL_CONTEXT_PLATFORM: > CHECK (set_cl_context_platform); > cl_props->platform_id = *(prop + 1); > - if (UNLIKELY((cl_platform_id) cl_props->platform_id != > intel_platform)) { > + if (UNLIKELY((cl_platform_id) cl_props->platform_id != > + cl_get_platform_default())) { > err = CL_INVALID_PLATFORM; > goto error; > } > diff --git a/src/cl_device_id.c b/src/cl_device_id.c index 9f03027..7956646 > 100644 > --- a/src/cl_device_id.c > +++ b/src/cl_device_id.c > @@ -351,7 +351,7 @@ cl_get_gt_device(void) > DECL_INFO_STRING(has_break, intel_hsw_gt3_device, name, "Intel(R) > HD Graphics Haswell CRW GT3 reserved"); > has_break: > device->vendor_id = device_id; > - device->platform = intel_platform; > + device->platform = cl_get_platform_default(); > ret = device; > break; > > @@ -363,7 +363,7 @@ has_break: > DECL_INFO_STRING(ivb_gt1_break, intel_ivb_gt1_device, name, "Intel(R) > HD Graphics IvyBridge S GT1"); > ivb_gt1_break: > intel_ivb_gt1_device.vendor_id = device_id; > - intel_ivb_gt1_device.platform = intel_platform; > + intel_ivb_gt1_device.platform = cl_get_platform_default(); > ret = &intel_ivb_gt1_device; > break; > > @@ -375,7 +375,7 @@ ivb_gt1_break: > DECL_INFO_STRING(ivb_gt2_break, intel_ivb_gt2_device, name, "Intel(R) > HD Graphics IvyBridge S GT2"); > ivb_gt2_break: > intel_ivb_gt2_device.vendor_id = device_id; > - intel_ivb_gt2_device.platform = intel_platform; > + intel_ivb_gt2_device.platform = cl_get_platform_default(); > ret = &intel_ivb_gt2_device; > break; > > @@ -383,7 +383,7 @@ ivb_gt2_break: > DECL_INFO_STRING(baytrail_t_device_break, intel_baytrail_t_device, > name, "Intel(R) HD Graphics Bay Trail-T"); > baytrail_t_device_break: > intel_baytrail_t_device.vendor_id = device_id; > - intel_baytrail_t_device.platform = intel_platform; > + intel_baytrail_t_device.platform = cl_get_platform_default(); > ret = &intel_baytrail_t_device; > break; > > @@ -399,9 +399,9 @@ baytrail_t_device_break: > DECL_INFO_STRING(brw_gt1_break, intel_brw_gt1_device, name, > "Intel(R) HD Graphics BroadWell ULX GT1"); > brw_gt1_break: > /* For Gen8 and later, half float is suppported and we will enable > cl_khr_fp16. */ > - cl_intel_platform_enable_fp16_extension(intel_platform); > + > + cl_intel_platform_enable_fp16_extension(cl_get_platform_default()); > intel_brw_gt1_device.vendor_id = device_id; > - intel_brw_gt1_device.platform = intel_platform; > + intel_brw_gt1_device.platform = cl_get_platform_default(); > ret = &intel_brw_gt1_device; > break; > > @@ -416,9 +416,9 @@ brw_gt1_break: > case PCI_CHIP_BROADWLL_U_GT2: > DECL_INFO_STRING(brw_gt2_break, intel_brw_gt2_device, name, > "Intel(R) HD Graphics BroadWell ULX GT2"); > brw_gt2_break: > - cl_intel_platform_enable_fp16_extension(intel_platform); > + > + cl_intel_platform_enable_fp16_extension(cl_get_platform_default()); > intel_brw_gt2_device.vendor_id = device_id; > - intel_brw_gt2_device.platform = intel_platform; > + intel_brw_gt2_device.platform = cl_get_platform_default(); > ret = &intel_brw_gt2_device; > break; > > @@ -433,9 +433,9 @@ brw_gt2_break: > case PCI_CHIP_BROADWLL_U_GT3: > DECL_INFO_STRING(brw_gt3_break, intel_brw_gt3_device, name, > "Intel(R) HD Graphics BroadWell ULX GT2"); > brw_gt3_break: > - cl_intel_platform_enable_fp16_extension(intel_platform); > + > + cl_intel_platform_enable_fp16_extension(cl_get_platform_default()); > intel_brw_gt3_device.vendor_id = device_id; > - intel_brw_gt3_device.platform = intel_platform; > + intel_brw_gt3_device.platform = cl_get_platform_default(); > ret = &intel_brw_gt3_device; > break; > > @@ -445,9 +445,9 @@ brw_gt3_break: > case PCI_CHIP_CHV_3: > DECL_INFO_STRING(chv_break, intel_chv_device, name, "Intel(R) HD > Graphics Cherryview"); > chv_break: > - cl_intel_platform_enable_fp16_extension(intel_platform); > + > + cl_intel_platform_enable_fp16_extension(cl_get_platform_default()); > intel_chv_device.vendor_id = device_id; > - intel_chv_device.platform = intel_platform; > + intel_chv_device.platform = cl_get_platform_default(); > ret = &intel_chv_device; > break; > > @@ -463,9 +463,9 @@ chv_break: > case PCI_CHIP_SKYLAKE_SRV_GT1: > DECL_INFO_STRING(skl_gt1_break, intel_skl_gt1_device, name, "Intel(R) > HD Graphics Skylake Server GT1"); > skl_gt1_break: > - cl_intel_platform_enable_fp16_extension(intel_platform); > + > + cl_intel_platform_enable_fp16_extension(cl_get_platform_default()); > intel_skl_gt1_device.vendor_id = device_id; > - intel_skl_gt1_device.platform = intel_platform; > + intel_skl_gt1_device.platform = cl_get_platform_default(); > ret = &intel_skl_gt1_device; > break; > > @@ -482,9 +482,9 @@ skl_gt1_break: > case PCI_CHIP_SKYLAKE_SRV_GT2: > DECL_INFO_STRING(skl_gt2_break, intel_skl_gt2_device, name, "Intel(R) > HD Graphics Skylake Server GT2"); > skl_gt2_break: > - cl_intel_platform_enable_fp16_extension(intel_platform); > + > + cl_intel_platform_enable_fp16_extension(cl_get_platform_default()); > intel_skl_gt2_device.vendor_id = device_id; > - intel_skl_gt2_device.platform = intel_platform; > + intel_skl_gt2_device.platform = cl_get_platform_default(); > ret = &intel_skl_gt2_device; > break; > > @@ -495,9 +495,9 @@ skl_gt2_break: > case PCI_CHIP_SKYLAKE_SRV_GT3: > DECL_INFO_STRING(skl_gt3_break, intel_skl_gt3_device, name, "Intel(R) > HD Graphics Skylake Server GT3"); > skl_gt3_break: > - cl_intel_platform_enable_fp16_extension(intel_platform); > + > + cl_intel_platform_enable_fp16_extension(cl_get_platform_default()); > intel_skl_gt3_device.vendor_id = device_id; > - intel_skl_gt3_device.platform = intel_platform; > + intel_skl_gt3_device.platform = cl_get_platform_default(); > ret = &intel_skl_gt3_device; > break; > > @@ -506,9 +506,9 @@ skl_gt3_break: > case PCI_CHIP_SKYLAKE_SRV_GT4: > DECL_INFO_STRING(skl_gt4_break, intel_skl_gt4_device, name, "Intel(R) > HD Graphics Skylake Server GT4"); > skl_gt4_break: > - cl_intel_platform_enable_fp16_extension(intel_platform); > + > + cl_intel_platform_enable_fp16_extension(cl_get_platform_default()); > intel_skl_gt4_device.vendor_id = device_id; > - intel_skl_gt4_device.platform = intel_platform; > + intel_skl_gt4_device.platform = cl_get_platform_default(); > ret = &intel_skl_gt4_device; > break; > > @@ -636,17 +636,6 @@ cl_get_device_ids(cl_platform_id platform, > { > cl_device_id device; > > - /* Spec allow platform to be NULL, and If platform > - is NULL, the behavior is implementation-defined. > - We can not init the device before platform init. */ > - if (!platform) { > - if (num_devices) > - *num_devices = 0; > - if (devices) > - *devices = 0; > - return CL_DEVICE_NOT_FOUND; > - } > - > /* Do we have a usable device? */ > device = cl_get_gt_device(); > if (device) { > @@ -678,8 +667,8 @@ cl_get_device_ids(cl_platform_id platform, > *num_devices = 1; > if (devices) { > *devices = device; > - (*devices)->extensions = intel_platform->extensions; > - (*devices)->extensions_sz = intel_platform->extensions_sz; > + (*devices)->extensions = cl_get_platform_default()->extensions; > + (*devices)->extensions_sz = > + cl_get_platform_default()->extensions_sz; > } > return CL_SUCCESS; > } > diff --git a/src/cl_extensions.c b/src/cl_extensions.c index 0a29d2f..f1948b3 > 100644 > --- a/src/cl_extensions.c > +++ b/src/cl_extensions.c > @@ -13,7 +13,9 @@ > #include <string.h> > #include <assert.h> > > -static struct cl_extensions intel_extensions = > +/* This extension should be common for all the intel GPU platform. > + Every device may have its own additional externsions. */ static > +struct cl_extensions intel_platform_extensions = > { > { > #define DECL_EXT(name) \ > @@ -91,14 +93,12 @@ process_extension_str(cl_extensions_t *extensions) > } > } > > -static int ext_initialized = 0; > > LOCAL void > cl_intel_platform_enable_fp16_extension(cl_platform_id intel_platform) { > - cl_extensions_t *extensions = &intel_extensions; > + cl_extensions_t *extensions = &intel_platform_extensions; > int id; > - assert(ext_initialized); > > for(id = OPT1_EXT_START_ID; id <= OPT1_EXT_END_ID; id++) > { > @@ -107,29 +107,27 @@ > cl_intel_platform_enable_fp16_extension(cl_platform_id intel_platform) > } > > process_extension_str(extensions); > - intel_platform->internal_extensions = &intel_extensions; > - intel_platform->extensions = intel_extensions.ext_str; > + intel_platform->internal_extensions = &intel_platform_extensions; > + intel_platform->extensions = intel_platform_extensions.ext_str; > intel_platform->extensions_sz = strlen(intel_platform->extensions) + 1; } > > LOCAL void > cl_intel_platform_extension_init(cl_platform_id intel_platform) { > - if (ext_initialized) { > - intel_platform->internal_extensions = &intel_extensions; > - intel_platform->extensions = intel_extensions.ext_str; > - return; > + static int ext_initialized = 0; > + > + if (!ext_initialized) { > + check_basic_extension(&intel_platform_extensions); > + check_opt1_extension(&intel_platform_extensions); > + check_gl_extension(&intel_platform_extensions); > + check_intel_extension(&intel_platform_extensions); > + process_extension_str(&intel_platform_extensions); > + ext_initialized = 1; > }
Is ext_initialized check still needed? _______________________________________________ Beignet mailing list Beignet@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/beignet
