[Beignet] [PATCH 1/2] Ensure that DRM device uses the i915 driver

[Mark Thompson]

This avoids calling random ioctl()s and returning nonsensical errors for
unsupported devices. In particular, loading is much cleaner on setups
where the driver needs to iterate over multiple devices to find the correct
one because the Intel graphics device is not the first DRM device.
---
Fixes this sort of spam from every OpenCL-using application:

$ clinfo
DRM_IOCTL_I915_GEM_APERTURE failed: Invalid argument
Assuming 131072kB available aperture size.
May lead to reduced performance or incorrect rendering.
get chip id failed: -1 [2]
param: 4, val: 0
Number of platforms 1
Platform Name Intel Gen OCL Driver
Platform Vendor Intel
Platform Version OpenCL 2.0 beignet 1.4 (git-d1b99a1d)
Platform Profile FULL_PROFILE
Platform Extensions cl_khr_global_int32_base_atomics cl_khr_global_int32_extended_atomics cl_khr_local_int32_base_atomics cl_khr_local_int32_extended_atomics cl_khr_byte_addressable_store cl_khr_3d_image_writes cl_khr_image2d_from_buffer cl_khr_depth_images cl_khr_spir cl_khr_icd cl_intel_accelerator cl_intel_subgroups cl_intel_subgroups_short cl_intel_media_block_io cl_intel_planar_yuv cl_khr_gl_sharing
Platform Extensions function suffix Intel
DRM_IOCTL_I915_GEM_APERTURE failed: Invalid argument
Assuming 131072kB available aperture size.
May lead to reduced performance or incorrect rendering.
get chip id failed: -1 [2]
param: 4, val: 0
DRM_IOCTL_I915_GEM_APERTURE failed: Invalid argument
Assuming 131072kB available aperture size.
May lead to reduced performance or incorrect rendering.
get chip id failed: -1 [2]
param: 4, val: 0
DRM_IOCTL_I915_GEM_APERTURE failed: Invalid argument
Assuming 131072kB available aperture size.
May lead to reduced performance or incorrect rendering.
get chip id failed: -1 [2]
param: 4, val: 0
DRM_IOCTL_I915_GEM_APERTURE failed: Invalid argument
Assuming 131072kB available aperture size.
May lead to reduced performance or incorrect rendering.
get chip id failed: -1 [2]
param: 4, val: 0
DRM_IOCTL_I915_GEM_APERTURE failed: Invalid argument
Assuming 131072kB available aperture size.
May lead to reduced performance or incorrect rendering.
get chip id failed: -1 [2]
param: 4, val: 0

Platform Name Intel Gen OCL Driver
Number of devices 1
DRM_IOCTL_I915_GEM_APERTURE failed: Invalid argument
Assuming 131072kB available aperture size.
May lead to reduced performance or incorrect rendering.
get chip id failed: -1 [2]
param: 4, val: 0
DRM_IOCTL_I915_GEM_APERTURE failed: Invalid argument
Assuming 131072kB available aperture size.
May lead to reduced performance or incorrect rendering.
get chip id failed: -1 [2]
param: 4, val: 0
...


src/intel/intel_driver.c | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)

diff --git a/src/intel/intel_driver.c b/src/intel/intel_driver.c
index 45719785..10fe3cc8 100644
--- a/src/intel/intel_driver.c
+++ b/src/intel/intel_driver.c
@@ -312,6 +312,26 @@ return ret;
}
#endif

+static int
+intel_driver_check_device(int dev_fd)
+{
+ // Ensure that this is actually an i915 DRM device.
+ drmVersion *version;
+ int ret;
+ version = drmGetVersion(dev_fd);
+ if (!version) {
+ fprintf(stderr, "drmGetVersion(%d) failed: %s\n", dev_fd, strerror(errno));
+ close(dev_fd);
+ return 0;
+ }
+ ret = !strcmp(version->name, "i915");
+ drmFreeVersion(version);
+ // Don't print an error here if this device is using a different driver,
+ // because we might be iterating over multiple devices looking for a
+ // compatible one.
+ return ret;
+}
+
LOCAL int
intel_driver_init_master(intel_driver_t *driver, const char* dev_name)
{
@@ -326,6 +346,11 @@ if (dev_fd == -1) {
return 0;
}

+if (!intel_driver_check_device(dev_fd)) {
+ close(dev_fd);
+ return 0;
+}
+
// Check that we're authenticated
memset(&client, 0, sizeof(drm_client_t));
ret = ioctl(dev_fd, DRM_IOCTL_GET_CLIENT, &client);
@@ -356,6 +381,11 @@ dev_fd = open(dev_name, O_RDWR);
if (dev_fd == -1)
return 0;

+if (!intel_driver_check_device(dev_fd)) {
+ close(dev_fd);
+ return 0;
+}
+
ret = intel_driver_init(driver, dev_fd);
driver->need_close = 1;

[Mark Thompson]

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);
+ cl_intel_platform_enable_extension(ret, cl_khr_fp16_ext_id);
+ break;
+
case PCI_CHIP_SANDYBRIDGE_BRIDGE:
case PCI_CHIP_SANDYBRIDGE_GT1:
case PCI_CHIP_SANDYBRIDGE_GT2:
@@ -992,7 +1015,8 @@ LOCAL cl_bool is_gen_device(cl_device_id device) {
device == &intel_kbl_gt3_device ||
device == &intel_kbl_gt4_device ||
device == &intel_glk18eu_device ||
- device == &intel_glk12eu_device;
+ device == &intel_glk12eu_device ||
+ device == &intel_cfl_gt2_device;
}

LOCAL cl_int
@@ -1420,7 +1444,8 @@ cl_device_get_version(cl_device_id device, cl_int *ver)
|| device == &intel_bxt18eu_device || device == &intel_bxt12eu_device || device == &intel_kbl_gt1_device
|| device == &intel_kbl_gt2_device || device == &intel_kbl_gt3_device
|| device == &intel_kbl_gt4_device || device == &intel_kbl_gt15_device
- || device == &intel_glk18eu_device || device == &intel_glk12eu_device) {
+ || device == &intel_glk18eu_device || device == &intel_glk12eu_device
+ || device == &intel_cfl_gt2_device) {
*ver = 9;
} else
return CL_INVALID_VALUE;

[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-----
Mark Thompson
Sent: Wednesday, January 24, 2018 6:56 AM
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_intel_platform_enable_extension(ret, cl_khr_fp16_ext_id);
break;
+ DECL_INFO_STRING(cfl_gt2_break, intel_cfl_gt2_device, name,
+"Intel(R) UHD Graphics Coffee Lake Desktop GT2");
+ 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?

[Yang, Rong R]

This patch LGTM, thanks.

-----Original Message-----
Mark Thompson
Sent: Wednesday, January 24, 2018 6:52 AM
Subject: [Beignet] [PATCH 1/2] Ensure that DRM device uses the i915 driver
This avoids calling random ioctl()s and returning nonsensical errors for
unsupported devices. In particular, loading is much cleaner on setups where the
driver needs to iterate over multiple devices to find the correct one because the
Intel graphics device is not the first DRM device.
---
$ clinfo
DRM_IOCTL_I915_GEM_APERTURE failed: Invalid argument Assuming 131072kB
available aperture size.
May lead to reduced performance or incorrect rendering.
get chip id failed: -1 [2]
param: 4, val: 0
Number of platforms 1
Platform Name Intel Gen OCL Driver
Platform Vendor Intel
Platform Version OpenCL 2.0 beignet 1.4 (git-d1b99a1d)
Platform Profile FULL_PROFILE
Platform Extensions cl_khr_global_int32_base_atomics
cl_khr_global_int32_extended_atomics cl_khr_local_int32_base_atomics
cl_khr_local_int32_extended_atomics cl_khr_byte_addressable_store
cl_khr_3d_image_writes cl_khr_image2d_from_buffer cl_khr_depth_images
cl_khr_spir cl_khr_icd cl_intel_accelerator cl_intel_subgroups
cl_intel_subgroups_short cl_intel_media_block_io cl_intel_planar_yuv
cl_khr_gl_sharing
Platform Extensions function suffix Intel
DRM_IOCTL_I915_GEM_APERTURE failed: Invalid argument Assuming 131072kB
available aperture size.
May lead to reduced performance or incorrect rendering.
get chip id failed: -1 [2]
param: 4, val: 0
DRM_IOCTL_I915_GEM_APERTURE failed: Invalid argument Assuming 131072kB
available aperture size.
May lead to reduced performance or incorrect rendering.
get chip id failed: -1 [2]
param: 4, val: 0
DRM_IOCTL_I915_GEM_APERTURE failed: Invalid argument Assuming 131072kB
available aperture size.
May lead to reduced performance or incorrect rendering.
get chip id failed: -1 [2]
param: 4, val: 0
DRM_IOCTL_I915_GEM_APERTURE failed: Invalid argument Assuming 131072kB
available aperture size.
May lead to reduced performance or incorrect rendering.
get chip id failed: -1 [2]
param: 4, val: 0
DRM_IOCTL_I915_GEM_APERTURE failed: Invalid argument Assuming 131072kB
available aperture size.
May lead to reduced performance or incorrect rendering.
get chip id failed: -1 [2]
param: 4, val: 0
Platform Name Intel Gen OCL Driver
Number of devices 1
DRM_IOCTL_I915_GEM_APERTURE failed: Invalid argument Assuming 131072kB
available aperture size.
May lead to reduced performance or incorrect rendering.
get chip id failed: -1 [2]
param: 4, val: 0
DRM_IOCTL_I915_GEM_APERTURE failed: Invalid argument Assuming 131072kB
available aperture size.
May lead to reduced performance or incorrect rendering.
get chip id failed: -1 [2]
param: 4, val: 0
...
src/intel/intel_driver.c | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/src/intel/intel_driver.c b/src/intel/intel_driver.c index
45719785..10fe3cc8 100644
--- a/src/intel/intel_driver.c
+++ b/src/intel/intel_driver.c
@@ -312,6 +312,26 @@ return ret;
}
#endif
+static int
+intel_driver_check_device(int dev_fd)
+{
+ // Ensure that this is actually an i915 DRM device.
+ drmVersion *version;
+ int ret;
+ version = drmGetVersion(dev_fd);
+ if (!version) {
+ fprintf(stderr, "drmGetVersion(%d) failed: %s\n", dev_fd, strerror(errno));
+ close(dev_fd);
+ return 0;
+ }
+ ret = !strcmp(version->name, "i915");
+ drmFreeVersion(version);
+ // Don't print an error here if this device is using a different
+driver,
+ // because we might be iterating over multiple devices looking for a
+ // compatible one.
+ return ret;
+}
+
LOCAL int
return 0;
}
+if (!intel_driver_check_device(dev_fd)) {
+ close(dev_fd);
+ return 0;
+}
+
// Check that we're authenticated
memset(&client, 0, sizeof(drm_client_t)); ret = ioctl(dev_fd,
open(dev_name, O_RDWR); if (dev_fd == -1)
return 0;
+if (!intel_driver_check_device(dev_fd)) {
+ close(dev_fd);
+ return 0;
+}
+
ret = intel_driver_init(driver, dev_fd); driver->need_close = 1;
--
2.11.0
_______________________________________________
Beignet mailing list
https://lists.freedesktop.org/mailman/listinfo/beignet