[Beignet] [PATCH] intel: Export pooled EU and min no. of eus in a pool.

[Yang, Rong R]

OK, I have send a new version patch.

> -----Original Message-----
> From: Chris Wilson [mailto:chris at chris-wilson.co.uk]
> Sent: Thursday, July 14, 2016 3:10
> To: Arun Siluvery <arun.siluvery at linux.intel.com>
> Cc: Yang, Rong R <rong.r.yang at intel.com>; beignet at lists.freedesktop.org;
> dri-devel at lists.freedesktop.org; Daniel Vetter <daniel at ffwll.ch>; Tvrtko
> Ursulin <tvrtko.ursulin at linux.intel.com>; martin.peres at linux.intel.com
> Subject: Re: [Beignet] [PATCH] intel: Export pooled EU and min no. of eus in a
> pool.
> 
> On Wed, Jul 13, 2016 at 11:44:09AM +0100, Arun Siluvery wrote:
> > On 06/07/2016 05:51, Yang Rong wrote:
> > >Update kernel interface with new I915_GETPARAM ioctl entries for
> > >pooled EU and min no. of eus in a pool. Add a wrapping function for
> > >each parameter. Userspace drivers need these values when decide the
> > >thread count. This kernel enabled pooled eu by default for BXT and
> > >for fused down 2x6 parts it is advised to turn it off.
> > >
> > >But there is another HW issue in these parts (fused down 2x6 parts)
> > >before C0 that requires Pooled EU to be enabled as a workaround. In
> > >this case the pool configuration changes depending upon which
> > >subslice is disabled and the no. of eus in a pool is different, So
> > >userspace need to know min no. of eus in a pool.
> > >
> > >Signed-off-by: Yang Rong <rong.r.yang at intel.com>
> > >---
> >
> > Could you check this and give comments/ACK to merge this to libdrm?
> > Kernel changes are merged in drm-intel.
> >
> > regards
> > Arun
> >
> > >  include/drm/i915_drm.h   |  2 ++
> > >  intel/intel_bufmgr.h     |  3 +++
> > >  intel/intel_bufmgr_gem.c | 32 ++++++++++++++++++++++++++++++++
> > >  3 files changed, 37 insertions(+)
> > >
> > >diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h index
> > >c4ce6b2..eb611a7 100644
> > >--- a/include/drm/i915_drm.h
> > >+++ b/include/drm/i915_drm.h
> > >@@ -357,6 +357,8 @@ typedef struct drm_i915_irq_wait {
> > >  #define I915_PARAM_HAS_GPU_RESET	 35
> > >  #define I915_PARAM_HAS_RESOURCE_STREAMER 36
> > >  #define I915_PARAM_HAS_EXEC_SOFTPIN	 37
> > >+#define I915_PARAM_HAS_POOLED_EU         38
> > >+#define I915_PARAM_MIN_EU_IN_POOL        39
> > >
> > >  typedef struct drm_i915_getparam {
> > >  	__s32 param;
> > >diff --git a/intel/intel_bufmgr.h b/intel/intel_bufmgr.h index
> > >a1abbcd..8370694 100644
> > >--- a/intel/intel_bufmgr.h
> > >+++ b/intel/intel_bufmgr.h
> > >@@ -273,6 +273,9 @@ int drm_intel_get_reset_stats(drm_intel_context
> > >*ctx,
> > >  int drm_intel_get_subslice_total(int fd, unsigned int
> > >*subslice_total);
> > >  int drm_intel_get_eu_total(int fd, unsigned int *eu_total);
> > >
> > >+int drm_intel_get_pooled_eu(int fd, unsigned int *has_pooled_eu);
> > >+int drm_intel_get_min_eu_in_pool(int fd, unsigned int *min_eu);
> 
> Do we export more than 2 billion eu? Or more than 2 billion states for the
> bool has_pooled_eu?
> 
> Could we not just use
> 
> int drm_intel_get_pooled_eu(int fd);
> 
> ret < 0 => error occurred / unknown
> ret == 0 => no
> ret > 0 => yes
> 
> and same for get_min_eu.
> 
> > >+
> > >  /** @{ Compatibility defines to keep old code building despite the
> symbol rename
> > >   * from dri_* to drm_intel_*
> > >   */
> > >diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
> > >index 0a4012b..b8bb654 100644
> > >--- a/intel/intel_bufmgr_gem.c
> > >+++ b/intel/intel_bufmgr_gem.c
> > >@@ -3237,6 +3237,38 @@ drm_intel_get_eu_total(int fd, unsigned int
> *eu_total)
> > >  	return 0;
> > >  }
> > >
> > >+int
> > >+drm_intel_get_pooled_eu(int fd, unsigned int *has_pooled_eu) {
> > >+	drm_i915_getparam_t gp;
> > >+	int ret;
> > >+
> > >+	memclear(gp);
> > >+	gp.value = (int*)has_pooled_eu;
> > >+	gp.param = I915_PARAM_HAS_POOLED_EU;
> > >+	ret = drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
> > >+	if (ret)
> 
> save a few electrons....
> 
> int drm_intel_get_pooled_eu(int fd)
> {
> 	drm_i915_getparam_t gp;
> 	int ret;
> 
> 	memclear(gp);
> 	gp.param = I915_PARAM_HAS_POOLED_EU;
> 	gp.value = &ret;
> 	if (drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp))
> 		return -errno;
> 
> 	return ret;
> }
> 
> (save a few more by calling this int __getparam(int fd, unsigned name)) -
> Chris
> 
> --
> Chris Wilson, Intel Open Source Technology Centre