OK, good idea, I will change the default configure and add the warning.
> -----Original Message----- > From: Mcgee, Jeff > Sent: Saturday, April 4, 2015 03:46 > To: Zhigang Gong > Cc: Yang, Rong R; [email protected] > Subject: Re: [Beignet] [PATCH 1/2] CHV: Add cherryview support in the > runtime. > > On Fri, Apr 03, 2015 at 10:11:54AM +0800, Zhigang Gong wrote: > > One minor comment is just as you mentioned, CHV requires > > cl_driver_update_device_info() to get the corresponding information > > from the libdrm layer. So we may need to do the following things: > > > > Rework the cl_driver_update_device_info()'s interface, one solution is > > to add one parameter to indicate whether requires to EU/SLICE > information from libdrm. > > If the device is CHV, we need to set that parameter to true. > > The cl_driver_update_device_info() will return a value to indicate > > whether it get information from libdrm successfully. If the device is > > CHV, and we get a false return value from > > cl_driver_update_device_info(), we can simply return a NULL device and > > complain the error. Which is much better to keep going with the > > possible incorrect EU/slice information and may hang the GPU sometime > latter. > > > > Thanks, > > Zhigang Gong. > > > > Another option would be to set the CHV minimum configuration (2 subslice, > 8 EU) as the default configuration before the call to > cl_driver_update_device_info. So if the kernel query for these values fails or > is not available, you can continue to run assuming the minimal config. > But of course it would still be a good idea to make a loud warning that the > true config could not be detected. > -Jeff > > > On Mon, Mar 30, 2015 at 11:23:55AM +0800, Yang Rong wrote: > > > Cherryview's EU configurations is not decided by pciid, must get from > kernel by libdrm. > > > Thanks for Jeff adding this support in the kernel and libdrm. > > > > > > Signed-off-by: Yang Rong <[email protected]> > > > --- > > > backend/src/backend/gen/gen_mesa_disasm.c | 8 ++++---- > > > backend/src/backend/gen_program.cpp | 7 +++++++ > > > backend/src/gbe_bin_generater.cpp | 4 ++++ > > > src/cl_device_data.h | 12 +++++++++++- > > > src/cl_device_id.c | 29 > > > ++++++++++++++++++++++++++++- > > > src/intel/intel_gpgpu.c | 5 ++++- > > > 6 files changed, 58 insertions(+), 7 deletions(-) > > > > > > diff --git a/backend/src/backend/gen/gen_mesa_disasm.c > > > b/backend/src/backend/gen/gen_mesa_disasm.c > > > index 711b943..f8d89e0 100644 > > > --- a/backend/src/backend/gen/gen_mesa_disasm.c > > > +++ b/backend/src/backend/gen/gen_mesa_disasm.c > > > @@ -1136,13 +1136,13 @@ int gen_disasm (FILE *file, const void > > > *inst, uint32_t deviceID, uint32_t compac { > > > int err = 0; > > > int space = 0; > > > - if (IS_IVYBRIDGE(deviceID)) { > > > + if (IS_GEN7(deviceID)) { > > > gen_version = 70; > > > - } else if (IS_HASWELL(deviceID)) { > > > + } else if (IS_GEN75(deviceID)) { > > > gen_version = 75; > > > - } else if (IS_BROADWELL(deviceID)) { > > > + } else if (IS_GEN8(deviceID)) { > > > gen_version = 80; > > > - } else if (IS_SKYLAKE(deviceID)) { > > > + } else if (IS_GEN9(deviceID)) { > > > gen_version = 90; > > > } > > > > > > diff --git a/backend/src/backend/gen_program.cpp > > > b/backend/src/backend/gen_program.cpp > > > index f4c74f8..f53d5fb 100644 > > > --- a/backend/src/backend/gen_program.cpp > > > +++ b/backend/src/backend/gen_program.cpp > > > @@ -166,6 +166,8 @@ namespace gbe { > > > ctx = GBE_NEW(Gen75Context, unit, name, deviceID, relaxMath); > > > } else if (IS_BROADWELL(deviceID)) { > > > ctx = GBE_NEW(Gen8Context, unit, name, deviceID, relaxMath); > > > + } else if (IS_CHERRYVIEW(deviceID)) { > > > + ctx = GBE_NEW(Gen8Context, unit, name, deviceID, relaxMath); > > > } else if (IS_SKYLAKE(deviceID)) { > > > ctx = GBE_NEW(Gen9Context, unit, name, deviceID, relaxMath); > > > } > > > @@ -210,6 +212,7 @@ namespace gbe { > > > (IS_BAYTRAIL_T(typeA) && > > > !strcmp(src_hw_info, > "BYT")) || \ > > > (IS_HASWELL(typeA) && > > > !strcmp(src_hw_info, > "HSW")) || \ > > > (IS_BROADWELL(typeA) && > > > !strcmp(src_hw_info, "BDW")) || \ > > > + (IS_CHERRYVIEW(typeA) && > > > + !strcmp(src_hw_info, "CHV")) || \ > > > (IS_SKYLAKE(typeA) && > > > !strcmp(src_hw_info, "SKL")) ) > > > > > > static gbe_program genProgramNewFromBinary(uint32_t deviceID, > > > const char *binary, size_t size) { @@ -316,6 +319,10 @@ namespace gbe { > > > src_hw_info[0]='B'; > > > src_hw_info[1]='D'; > > > src_hw_info[2]='W'; > > > + }else if(IS_CHERRYVIEW(prog->deviceID)){ > > > + src_hw_info[0]='C'; > > > + src_hw_info[1]='H'; > > > + src_hw_info[2]='V'; > > > }else if(IS_SKYLAKE(prog->deviceID)){ > > > src_hw_info[0]='S'; > > > src_hw_info[1]='K'; > > > diff --git a/backend/src/gbe_bin_generater.cpp > > > b/backend/src/gbe_bin_generater.cpp > > > index 8d41113..86197e1 100644 > > > --- a/backend/src/gbe_bin_generater.cpp > > > +++ b/backend/src/gbe_bin_generater.cpp > > > @@ -178,6 +178,10 @@ void > program_build_instance::serialize_program(void) throw(int) > > > src_hw_info[0]='B'; > > > src_hw_info[1]='D'; > > > src_hw_info[2]='W'; > > > + }else if(IS_CHERRYVIEW(gen_pci_id)){ > > > + src_hw_info[0]='C'; > > > + src_hw_info[1]='H'; > > > + src_hw_info[2]='V'; > > > }else if(IS_SKYLAKE(gen_pci_id)){ > > > src_hw_info[0]='S'; > > > src_hw_info[1]='K'; > > > diff --git a/src/cl_device_data.h b/src/cl_device_data.h index > > > d6f8209..6872106 100644 > > > --- a/src/cl_device_data.h > > > +++ b/src/cl_device_data.h > > > @@ -228,7 +228,17 @@ > > > devid == PCI_CHIP_BROADWLL_U_GT3) > > > > > > #define IS_BROADWELL(devid) (IS_BRW_GT1(devid) || > IS_BRW_GT2(devid) || IS_BRW_GT3(devid)) > > > -#define IS_GEN8(devid) IS_BROADWELL(devid) > > > + > > > +#define PCI_CHIP_CHV_0 0x22B0 > > > +#define PCI_CHIP_CHV_1 0x22B1 > > > +#define PCI_CHIP_CHV_2 0x22B2 > > > +#define PCI_CHIP_CHV_3 0x22B3 > > > +#define IS_CHERRYVIEW(devid) (devid == PCI_CHIP_CHV_0 || \ > > > + devid == PCI_CHIP_CHV_1 || \ > > > + devid == PCI_CHIP_CHV_2 || \ > > > + devid == PCI_CHIP_CHV_3) > > > + > > > +#define IS_GEN8(devid) (IS_BROADWELL(devid) || > IS_CHERRYVIEW(devid)) > > > > > > /* SKL */ > > > #define PCI_CHIP_SKYLAKE_ULT_GT1 0x1906 /* Intel(R) Skylake > ULT - GT1 */ > > > diff --git a/src/cl_device_id.c b/src/cl_device_id.c index > > > fefcef3..9d18b20 100644 > > > --- a/src/cl_device_id.c > > > +++ b/src/cl_device_id.c > > > @@ -140,6 +140,18 @@ static struct _cl_device_id > > > intel_brw_gt3_device = { #include "cl_gen75_device.h" > > > }; > > > > > > +//Cherryview has the same pciid, must get the max_compute_unit and > > > +max_thread_per_unit from drm static struct _cl_device_id > > > +intel_chv_device = { > > > + INIT_ICD(dispatch) > > > + .max_compute_unit = 6, > > > + .max_thread_per_unit = 7, > > > + .sub_slice_count = 2, > > > + .max_work_item_sizes = {512, 512, 512}, > > > + .max_work_group_size = 512, > > > + .max_clock_frequency = 1000, > > > +#include "cl_gen75_device.h" > > > +}; > > > + > > > /* XXX we clone brw now */ > > > static struct _cl_device_id intel_skl_gt1_device = { > > > INIT_ICD(dispatch) > > > @@ -423,6 +435,18 @@ brw_gt3_break: > > > ret = &intel_brw_gt3_device; > > > break; > > > > > > + case PCI_CHIP_CHV_0: > > > + case PCI_CHIP_CHV_1: > > > + case PCI_CHIP_CHV_2: > > > + case PCI_CHIP_CHV_3: > > > + DECL_INFO_STRING(chv_break, intel_chv_device, name, "Intel(R) > > > +HD Graphics Cherryview"); > > > +chv_break: > > > + intel_chv_device.vendor_id = device_id; > > > + intel_chv_device.platform = intel_platform; > > > + ret = &intel_chv_device; > > > + break; > > > + > > > + > > > case PCI_CHIP_SKYLAKE_ULT_GT1: > > > DECL_INFO_STRING(skl_gt1_break, intel_skl_gt1_device, > name, "Intel(R) HD Graphics Skylake ULT GT1"); > > > case PCI_CHIP_SKYLAKE_ULX_GT1: > > > @@ -590,6 +614,7 @@ cl_get_device_info(cl_device_id device, > > > device != &intel_brw_gt1_device && > > > device != &intel_brw_gt2_device && > > > device != &intel_brw_gt3_device && > > > + device != &intel_chv_device && > > > device != &intel_skl_gt1_device && > > > device != &intel_skl_gt2_device && > > > device != &intel_skl_gt3_device && @@ -699,6 +724,7 > > > @@ cl_device_get_version(cl_device_id device, cl_int *ver) > > > device != &intel_brw_gt1_device && > > > device != &intel_brw_gt2_device && > > > device != &intel_brw_gt3_device && > > > + device != &intel_chv_device && > > > device != &intel_skl_gt1_device && > > > device != &intel_skl_gt2_device && > > > device != &intel_skl_gt3_device && @@ -714,7 +740,7 > > > @@ cl_device_get_version(cl_device_id device, cl_int *ver) > > > || device == &intel_hsw_gt3_device) { > > > *ver = 75; > > > } else if (device == &intel_brw_gt1_device || device == > &intel_brw_gt2_device > > > - || device == &intel_brw_gt3_device) { > > > + || device == &intel_brw_gt3_device || device == > > > + &intel_chv_device) { > > > *ver = 8; > > > } else if (device == &intel_skl_gt1_device || device == > &intel_skl_gt2_device > > > || device == &intel_skl_gt3_device || device == > > > &intel_skl_gt4_device) { @@ -801,6 +827,7 @@ > cl_get_kernel_workgroup_info(cl_kernel kernel, > > > device != &intel_brw_gt1_device && > > > device != &intel_brw_gt2_device && > > > device != &intel_brw_gt3_device && > > > + device != &intel_chv_device && > > > device != &intel_skl_gt1_device && > > > device != &intel_skl_gt2_device && > > > device != &intel_skl_gt3_device && diff --git > > > a/src/intel/intel_gpgpu.c b/src/intel/intel_gpgpu.c index > > > 177ac04..ee440dc 100644 > > > --- a/src/intel/intel_gpgpu.c > > > +++ b/src/intel/intel_gpgpu.c > > > @@ -1142,6 +1142,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) || > > > + IS_CHERRYVIEW(gpgpu->drv->device_id) || > > > IS_SKYLAKE(gpgpu->drv->device_id))) && > > > index >= BTI_WORKAROUND_IMAGE_OFFSET + BTI_RESERVED_NUM > && > > > type == CL_MEM_OBJECT_IMAGE1D_ARRAY) @@ -2156,13 +2157,15 > @@ > > > intel_set_gpgpu_callbacks(int device_id) > > > cl_gpgpu_set_printf_info = (cl_gpgpu_set_printf_info_cb > *)intel_gpgpu_set_printf_info; > > > cl_gpgpu_get_printf_info = (cl_gpgpu_get_printf_info_cb > > > *)intel_gpgpu_get_printf_info; > > > > > > - if (IS_BROADWELL(device_id)) { > > > + if (IS_BROADWELL(device_id) || IS_CHERRYVIEW(device_id)) { > > > cl_gpgpu_bind_image = (cl_gpgpu_bind_image_cb *) > intel_gpgpu_bind_image_gen8; > > > intel_gpgpu_set_L3 = intel_gpgpu_set_L3_gen8; > > > cl_gpgpu_get_cache_ctrl = (cl_gpgpu_get_cache_ctrl_cb > *)intel_gpgpu_get_cache_ctrl_gen8; > > > intel_gpgpu_get_scratch_index = > intel_gpgpu_get_scratch_index_gen8; > > > intel_gpgpu_post_action = intel_gpgpu_post_action_gen7; //BDW > need not restore SLM, same as gen7 > > > intel_gpgpu_read_ts_reg = intel_gpgpu_read_ts_reg_gen7; > > > + if(IS_CHERRYVIEW(device_id)) > > > + intel_gpgpu_read_ts_reg = intel_gpgpu_read_ts_reg_baytrail; > > > intel_gpgpu_set_base_address = > intel_gpgpu_set_base_address_gen8; > > > intel_gpgpu_setup_bti = intel_gpgpu_setup_bti_gen8; > > > intel_gpgpu_load_vfe_state = intel_gpgpu_load_vfe_state_gen8; > > > -- > > > 2.1.0 > > > > > > _______________________________________________ > > > Beignet mailing list > > > [email protected] > > > http://lists.freedesktop.org/mailman/listinfo/beignet > > _______________________________________________ > > Beignet mailing list > > [email protected] > > http://lists.freedesktop.org/mailman/listinfo/beignet _______________________________________________ Beignet mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/beignet
