diff mbox

[2/2,v2] Query the driver directly for compute units and subslice

Message ID 1425944158-28223-1-git-send-email-jeff.mcgee@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

jeff.mcgee@intel.com March 9, 2015, 11:35 p.m. UTC 
From: Jeff McGee <jeff.mcgee@intel.com>

Values of device max compute units and max subslice obtained
directly from the driver should be more accurate than our own
ID-based lookup values. This is particularly important when a
single device ID may encompass more than one configuration. If
the driver cannot provide a valid value for the given device,
we fallback on the ID-based lookup value.

This query requires libdrm 2.4.60. For now we will consider
the use of this query to be optional and exclude it from
compilation when building against older libdrm. Later we may
want to consider requiring the query or at least warning
more strongly when it is not supported.

v2: Make feature use conditional on libdrm version (Zhigang).

Signed-off-by: Jeff McGee <jeff.mcgee@intel.com>
---
 CMakeLists.txt           |  9 +++++++++
 src/CMakeLists.txt       | 10 ++++++++++
 src/intel/intel_driver.c | 25 +++++++++++++++++++++----
 3 files changed, 40 insertions(+), 4 deletions(-)

Comments

zhigang.gong@linux.intel.com March 12, 2015, 2:08 a.m. UTC | #1 
LGTM,

Reviewed-by: Zhigang Gong <zhigang.gong@linux.intel.com>

Thanks.

> -----Original Message-----
> From: Beignet [mailto:beignet-bounces@lists.freedesktop.org] On Behalf Of
> jeff.mcgee@intel.com
> Sent: Tuesday, March 10, 2015 7:36 AM
> To: beignet@lists.freedesktop.org
> Cc: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org
> Subject: [Beignet] [PATCH 2/2 v2] Query the driver directly for compute units
> and subslice
> 
> From: Jeff McGee <jeff.mcgee@intel.com>
> 
> Values of device max compute units and max subslice obtained directly from
> the driver should be more accurate than our own ID-based lookup values. This
> is particularly important when a single device ID may encompass more than
> one configuration. If the driver cannot provide a valid value for the given device,
> we fallback on the ID-based lookup value.
> 
> This query requires libdrm 2.4.60. For now we will consider the use of this query
> to be optional and exclude it from compilation when building against older
> libdrm. Later we may want to consider requiring the query or at least warning
> more strongly when it is not supported.
> 
> v2: Make feature use conditional on libdrm version (Zhigang).
> 
> Signed-off-by: Jeff McGee <jeff.mcgee@intel.com>
> ---
>  CMakeLists.txt           |  9 +++++++++
>  src/CMakeLists.txt       | 10 ++++++++++
>  src/intel/intel_driver.c | 25 +++++++++++++++++++++----
>  3 files changed, 40 insertions(+), 4 deletions(-)
> 
> diff --git a/CMakeLists.txt b/CMakeLists.txt index 65f2c70..bb03566 100644
> --- a/CMakeLists.txt
> +++ b/CMakeLists.txt
> @@ -131,6 +131,15 @@ IF(DRM_INTEL_FOUND)
>    ELSE(DRM_INTEL_VERSION VERSION_GREATER 2.4.57)
>      MESSAGE(STATUS "Disable userptr support")
>    ENDIF(DRM_INTEL_VERSION VERSION_GREATER 2.4.57)
> +  IF(DRM_INTEL_VERSION VERSION_GREATER 2.4.59)
> +    MESSAGE(STATUS "Enable EU total query support")
> +    SET(DRM_INTEL_EU_TOTAL "enable")
> +    MESSAGE(STATUS "Enable subslice total query support")
> +    SET(DRM_INTEL_SUBSLICE_TOTAL "enable")
> ELSE(DRM_INTEL_VERSION
> + VERSION_GREATER 2.4.59)
> +    MESSAGE(STATUS "Disable EU total query support")
> +    MESSAGE(STATUS "Disable subslice total query support")
> + ENDIF(DRM_INTEL_VERSION VERSION_GREATER 2.4.59)
>  ELSE(DRM_INTEL_FOUND)
>    MESSAGE(FATAL_ERROR "Looking for DRM Intel (>= 2.4.52) - not found")
>  ENDIF(DRM_INTEL_FOUND)
> diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index d4181d8..464765f
> 100644
> --- a/src/CMakeLists.txt
> +++ b/src/CMakeLists.txt
> @@ -118,6 +118,16 @@ SET(CMAKE_CXX_FLAGS "-DHAS_USERPTR
> ${CMAKE_CXX_FLAGS}")  SET(CMAKE_C_FLAGS "-DHAS_USERPTR
> ${CMAKE_C_FLAGS}")  endif (DRM_INTEL_USERPTR)
> 
> +if (DRM_INTEL_EU_TOTAL)
> +SET(CMAKE_CXX_FLAGS "-DHAS_EU_TOTAL ${CMAKE_CXX_FLAGS}")
> +SET(CMAKE_C_FLAGS "-DHAS_EU_TOTAL ${CMAKE_C_FLAGS}") endif
> +(DRM_INTEL_EU_TOTAL)
> +
> +if (DRM_INTEL_SUBSLICE_TOTAL)
> +SET(CMAKE_CXX_FLAGS "-DHAS_SUBSLICE_TOTAL ${CMAKE_CXX_FLAGS}")
> +SET(CMAKE_C_FLAGS "-DHAS_SUBSLICE_TOTAL ${CMAKE_C_FLAGS}") endif
> +(DRM_INTEL_SUBSLICE_TOTAL)
> +
>  set(GIT_SHA1 "git_sha1.h")
>  add_custom_target(${GIT_SHA1} ALL
>    COMMAND chmod +x ${CMAKE_CURRENT_SOURCE_DIR}/git_sha1.sh
> diff --git a/src/intel/intel_driver.c b/src/intel/intel_driver.c index
> d61988c..755ab6b 100644
> --- a/src/intel/intel_driver.c
> +++ b/src/intel/intel_driver.c
> @@ -757,10 +757,7 @@ static int intel_buffer_set_tiling(cl_buffer bo,  static
> void  intel_update_device_info(cl_device_id device)  { -#ifdef HAS_USERPTR
>    intel_driver_t *driver;
> -  const size_t sz = 4096;
> -  void *host_ptr;
> 
>    driver = intel_driver_new();
>    assert(driver != NULL);
> @@ -769,6 +766,10 @@ intel_update_device_info(cl_device_id device)
>      return;
>    }
> 
> +#ifdef HAS_USERPTR
> +  const size_t sz = 4096;
> +  void *host_ptr;
> +
>    host_ptr = cl_aligned_malloc(sz, 4096);
>    if (host_ptr != NULL) {
>      cl_buffer bo = intel_buffer_alloc_userptr((cl_buffer_mgr)driver->bufmgr,
> @@ -781,12 +782,28 @@ intel_update_device_info(cl_device_id device)
>    }
>    else
>      device->host_unified_memory = CL_FALSE;
> +#endif
> +
> +#ifdef HAS_EU_TOTAL
> +  unsigned int eu_total;
> +
> +  /* Prefer driver-queried max compute units if supported */
> +  if (!drm_intel_get_eu_total(driver->fd, &eu_total))
> +    device->max_compute_unit = eu_total; #endif
> +
> +#ifdef HAS_SUBSLICE_TOTAL
> +  unsigned int subslice_total;
> +
> +  /* Prefer driver-queried subslice count if supported */
> +  if (!drm_intel_get_subslice_total(driver->fd, &subslice_total))
> +    device->sub_slice_count = subslice_total; #endif
> 
>    intel_driver_context_destroy(driver);
>    intel_driver_close(driver);
>    intel_driver_terminate(driver);
>    intel_driver_delete(driver);
> -#endif
>  }
> 
>  LOCAL void
> --
> 2.3.0
> 
> _______________________________________________
> Beignet mailing list
> Beignet@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet
jeff.mcgee@intel.com March 12, 2015, 8:52 p.m. UTC | #2 
On Thu, Mar 12, 2015 at 10:08:54AM +0800, Zhigang Gong wrote:
> LGTM,
> 
> Reviewed-by: Zhigang Gong <zhigang.gong@linux.intel.com>
> 
> Thanks.
> 

Thanks for the review, Zhigang.

With beignet portion reviewed, review should be able to proceed for
the i915, libdrm, and igt parts. These are all quite simple. Can someone(s)
please review?

-Jeff
diff mbox

Patch

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 65f2c70..bb03566 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -131,6 +131,15 @@  IF(DRM_INTEL_FOUND)
   ELSE(DRM_INTEL_VERSION VERSION_GREATER 2.4.57)
     MESSAGE(STATUS "Disable userptr support")
   ENDIF(DRM_INTEL_VERSION VERSION_GREATER 2.4.57)
+  IF(DRM_INTEL_VERSION VERSION_GREATER 2.4.59)
+    MESSAGE(STATUS "Enable EU total query support")
+    SET(DRM_INTEL_EU_TOTAL "enable")
+    MESSAGE(STATUS "Enable subslice total query support")
+    SET(DRM_INTEL_SUBSLICE_TOTAL "enable")
+  ELSE(DRM_INTEL_VERSION VERSION_GREATER 2.4.59)
+    MESSAGE(STATUS "Disable EU total query support")
+    MESSAGE(STATUS "Disable subslice total query support")
+  ENDIF(DRM_INTEL_VERSION VERSION_GREATER 2.4.59)
 ELSE(DRM_INTEL_FOUND)
   MESSAGE(FATAL_ERROR "Looking for DRM Intel (>= 2.4.52) - not found")
 ENDIF(DRM_INTEL_FOUND)
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index d4181d8..464765f 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -118,6 +118,16 @@  SET(CMAKE_CXX_FLAGS "-DHAS_USERPTR ${CMAKE_CXX_FLAGS}")
 SET(CMAKE_C_FLAGS "-DHAS_USERPTR ${CMAKE_C_FLAGS}")
 endif (DRM_INTEL_USERPTR)
 
+if (DRM_INTEL_EU_TOTAL)
+SET(CMAKE_CXX_FLAGS "-DHAS_EU_TOTAL ${CMAKE_CXX_FLAGS}")
+SET(CMAKE_C_FLAGS "-DHAS_EU_TOTAL ${CMAKE_C_FLAGS}")
+endif (DRM_INTEL_EU_TOTAL)
+
+if (DRM_INTEL_SUBSLICE_TOTAL)
+SET(CMAKE_CXX_FLAGS "-DHAS_SUBSLICE_TOTAL ${CMAKE_CXX_FLAGS}")
+SET(CMAKE_C_FLAGS "-DHAS_SUBSLICE_TOTAL ${CMAKE_C_FLAGS}")
+endif (DRM_INTEL_SUBSLICE_TOTAL)
+
 set(GIT_SHA1 "git_sha1.h")
 add_custom_target(${GIT_SHA1} ALL
   COMMAND chmod +x ${CMAKE_CURRENT_SOURCE_DIR}/git_sha1.sh
diff --git a/src/intel/intel_driver.c b/src/intel/intel_driver.c
index d61988c..755ab6b 100644
--- a/src/intel/intel_driver.c
+++ b/src/intel/intel_driver.c
@@ -757,10 +757,7 @@  static int intel_buffer_set_tiling(cl_buffer bo,
 static void
 intel_update_device_info(cl_device_id device)
 {
-#ifdef HAS_USERPTR
   intel_driver_t *driver;
-  const size_t sz = 4096;
-  void *host_ptr;
 
   driver = intel_driver_new();
   assert(driver != NULL);
@@ -769,6 +766,10 @@  intel_update_device_info(cl_device_id device)
     return;
   }
 
+#ifdef HAS_USERPTR
+  const size_t sz = 4096;
+  void *host_ptr;
+
   host_ptr = cl_aligned_malloc(sz, 4096);
   if (host_ptr != NULL) {
     cl_buffer bo = intel_buffer_alloc_userptr((cl_buffer_mgr)driver->bufmgr,
@@ -781,12 +782,28 @@  intel_update_device_info(cl_device_id device)
   }
   else
     device->host_unified_memory = CL_FALSE;
+#endif
+
+#ifdef HAS_EU_TOTAL
+  unsigned int eu_total;
+
+  /* Prefer driver-queried max compute units if supported */
+  if (!drm_intel_get_eu_total(driver->fd, &eu_total))
+    device->max_compute_unit = eu_total;
+#endif
+
+#ifdef HAS_SUBSLICE_TOTAL
+  unsigned int subslice_total;
+
+  /* Prefer driver-queried subslice count if supported */
+  if (!drm_intel_get_subslice_total(driver->fd, &subslice_total))
+    device->sub_slice_count = subslice_total;
+#endif
 
   intel_driver_context_destroy(driver);
   intel_driver_close(driver);
   intel_driver_terminate(driver);
   intel_driver_delete(driver);
-#endif
 }
 
 LOCAL void