[Beignet] [PATCH 2/2] Enable Coffee Lake support

[Yang, Rong R]

One inline comment.
Can you also add other Coffee Lake pciids?
0x3E90, 12Eus, gt1
0x3E93, 12Eus, gt1
0x3E99, 12Eus, gt1
0x3EA1, 12Eus, gt1
0x3EA4, 12Eus, gt1

0x3E91, 24EUs, gt2 
0x3E92, 24EUs, gt2 
0x3E94, 24EUs, gt2 
0x3E96, 24EUs, gt2 
0x3E9A, 24EUs, gt2 
0x3E9B, 24EUs, gt2 
0x3EA0, 24EUs, gt2 
0x3EA3, 24EUs, gt2 
0x3EA9, 24EUs, gt2 

0x3EA2, 48EUs, gt3
0x3EA5, 48EUs, gt3
0x3EA6, 48EUs, gt3
0x3EA7, 48EUs, gt3
0x3EA8, 48EUs, gt3

Thanks,
Yang Rong

> -----Original Message-----
> From: Beignet [mailto:beignet-bounces at lists.freedesktop.org] On Behalf Of
> Mark Thompson
> Sent: Wednesday, January 24, 2018 6:56 AM
> To: beignet at lists.freedesktop.org
> Subject: [Beignet] [PATCH 2/2] Enable Coffee Lake support
> 
> We don't need to do much here because the graphics core is the same as Kaby
> Lake.
> ---
> Tested on an 8700.  All behaviour is identical to Kaby Lake, so we can reuse most
> things after adding the PCI ID and device structure.
> 
> There will be more PCI IDs, but I've only added the one I know the meaning of
> and can test.
> 
> 
>  backend/src/backend/gen_program.cpp |  5 +++++
>  src/cl_device_data.h                |  9 ++++++++-
>  src/cl_device_id.c                  | 29 +++++++++++++++++++++++++++--
>  3 files changed, 40 insertions(+), 3 deletions(-)
> 
> diff --git a/backend/src/backend/gen_program.cpp
> b/backend/src/backend/gen_program.cpp
> index e06ed40c..274c99c7 100644
> --- a/backend/src/backend/gen_program.cpp
> +++ b/backend/src/backend/gen_program.cpp
> @@ -209,6 +209,8 @@ namespace gbe {
>        ctx = GBE_NEW(BxtContext, unit, name, deviceID, relaxMath);
>      } else if (IS_KABYLAKE(deviceID)) {
>        ctx = GBE_NEW(KblContext, unit, name, deviceID, relaxMath);
> +    } else if (IS_COFFEELAKE(deviceID)) {
> +      ctx = GBE_NEW(KblContext, unit, name, deviceID, relaxMath);
>      } else if (IS_GEMINILAKE(deviceID)) {
>        ctx = GBE_NEW(GlkContext, unit, name, deviceID, relaxMath);
>      }
> @@ -328,6 +330,7 @@ namespace gbe {
>                                        (IS_SKYLAKE(deviceID) && MATCH_SKL_HEADER(binary)) ||
> \
>                                        (IS_BROXTON(deviceID) && MATCH_BXT_HEADER(binary))
> || \
>                                        (IS_KABYLAKE(deviceID) && MATCH_KBL_HEADER(binary))
> || \
> +                                      (IS_COFFEELAKE(deviceID) &&
> + MATCH_KBL_HEADER(binary)) || \
>                                        (IS_GEMINILAKE(deviceID) &&
> MATCH_GLK_HEADER(binary)) \
>                                        )
> 
> @@ -436,6 +439,8 @@ namespace gbe {
>          FILL_BXT_HEADER(*binary);
>        }else if(IS_KABYLAKE(prog->deviceID)){
>          FILL_KBL_HEADER(*binary);
> +      }else if(IS_COFFEELAKE(prog->deviceID)){
> +        FILL_KBL_HEADER(*binary);
>        }else if(IS_GEMINILAKE(prog->deviceID)){
>          FILL_GLK_HEADER(*binary);
>        }else {
> diff --git a/src/cl_device_data.h b/src/cl_device_data.h index
> 123b6192..db5272da 100644
> --- a/src/cl_device_data.h
> +++ b/src/cl_device_data.h
> @@ -372,7 +372,14 @@
>    (devid == PCI_CHIP_GLK_3x6 ||   \
>     devid == PCI_CHIP_GLK_2x6)
> 
> -#define IS_GEN9(devid)     (IS_SKYLAKE(devid) || IS_BROXTON(devid) ||
> IS_KABYLAKE(devid) || IS_GEMINILAKE(devid))
> +#define PCI_CHIP_COFFEELAKE_S_GT2   0x3E92
> +
> +#define IS_CFL_GT2(devid)                  \
> +  (devid == PCI_CHIP_COFFEELAKE_S_GT2)
> +
> +#define IS_COFFEELAKE(devid) (IS_CFL_GT2(devid))
> +
> +#define IS_GEN9(devid)     (IS_SKYLAKE(devid) || IS_BROXTON(devid) ||
> IS_KABYLAKE(devid) || IS_GEMINILAKE(devid) || IS_COFFEELAKE(devid))
> 
>  #define MAX_OCLVERSION(devid) (IS_GEN9(devid) ? 200 : 120)
> 
> diff --git a/src/cl_device_id.c b/src/cl_device_id.c index 5e284193..d3180258
> 100644
> --- a/src/cl_device_id.c
> +++ b/src/cl_device_id.c
> @@ -274,6 +274,16 @@ static struct _cl_device_id intel_glk12eu_device =
> {  #include "cl_gen9_device.h"
>  };
> 
> +static struct _cl_device_id intel_cfl_gt2_device = {
> +  .max_compute_unit = 24,
> +  .max_thread_per_unit = 7,
> +  .sub_slice_count = 3,
> +  .max_work_item_sizes = {512, 512, 512},
> +  .max_work_group_size = 256,
> +  .max_clock_frequency = 1000,
> +#include "cl_gen9_device.h"
> +};
> +
>  LOCAL cl_device_id
>  cl_get_gt_device(cl_device_type device_type)  { @@ -785,6 +795,19 @@
> glk12eu_break:
>        cl_intel_platform_enable_extension(ret, cl_khr_fp16_ext_id);
>        break;
> 
> +    case PCI_CHIP_COFFEELAKE_S_GT2:
> +      DECL_INFO_STRING(cfl_gt2_break, intel_cfl_gt2_device, name,
> +"Intel(R) UHD Graphics Coffee Lake Desktop GT2");
> +cfl_gt2_break:
> +      intel_cfl_gt2_device.device_id = device_id;
> +      intel_cfl_gt2_device.platform = cl_get_platform_default();
> +      ret = &intel_cfl_gt2_device;
> +#ifdef ENABLE_FP64
> +      cl_intel_platform_enable_extension(ret, cl_khr_fp64_ext_id);
> +#endif
> +      cl_intel_platform_get_default_extension(ret);

Must call cl_intel_platform_enable_extension after cl_intel_platform_get_default_extension, otherwise cl_intel_platform_get_default_extension will overwrite the enabled extension.
This bug exists long time. As I remember, I have post a patch for it, but seems missed. Can you send a new patch to fix it?