[Intel-gfx] [PATCH v7 0/3] Dynamic EU configuration of Slice/Sub-slice/EU

[Intel-gfx] [PATCH v7 0/3] Dynamic EU configuration of Slice/Sub-slice/EU

[srinivasan.s]

From: Srinivasan S <srinivasan.s@intel.com>

      drm/i915: Context aware user agnostic EU/Slice/Sub-slice control within kernel

This patch sets improves GPU power consumption on Linux kernel based OS such as
Chromium OS, Ubuntu, etc. Following are the power savings.

Power savings on GLK-GT1 Bobba platform running on Chrome OS.
-----------------------------------------------|
App /KPI                | % Power Benefit (mW) |
------------------------|----------------------|
Hangout Call- 20 minute |	1.8%           |
Youtube 4K VPB          |       14.13%         |
WebGL Aquarium          |       13.76%         |
Unity3D                 |       6.78%          |
			|		       |
------------------------|----------------------|
Chrome PLT              | BatteryLife Improves |
			| by ~45 minute        |
-----------------------------------------------|

Power savings on KBL-GT3 running on  Android and Ubuntu (Linux).
-----------------------------------------------|
App /KPI		| % Power Benefit (mW) |
                        |----------------------|
			|  Android |  Ubuntu   |
------------------------|----------|-----------|
3D Mark (Ice storm)     | 2.30%    | N.A.      |
TRex On screen          | 2.49%    | 2.97%     |
Manhattan On screen     | 3.11%    | 4.90%     |
Carchase On Screen	| N.A.     | 5.06%     |
AnTuTu 6.1.4            | 3.42%    | N.A.      |
SynMark2		| N.A.     | 1.7%      |
-----------------------------------------------|

We have also observed GPU core residencies improves by 1.035%.

Technical Insights of the patch:
Current GPU configuration code for i915 does not allow us to change
EU/Slice/Sub-slice configuration dynamically. Its done only once while context
is created.

While particular graphics application is running, if we examine the command
requests from user space, we observe that command density is not consistent.
It means there is scope to change the graphics configuration dynamically even
while context is running actively. This patch series proposes the solution to
find the active pending load for all active context at given time and based on
that, dynamically perform graphics configuration for each context.

We use a hr (high resolution) timer with i915 driver in kernel to get a
callback every few milliseconds (this timer value can be configured through
debugfs, default is '0' indicating timer is in disabled state i.e. original
system without any intervention).In the timer callback, we examine pending
commands for a context in the queue, essentially, we intercept them before
they are executed by GPU and we update context with required number of EUs.

Two questions, how did we arrive at right timer value? and what's the right
number of EUs? For the prior one, empirical data to achieve best performance
in least power was considered. For the later one, we roughly categorized number 
of EUs logically based on platform. Now we compare number of pending commands
with a particular threshold and then set number of EUs accordingly with update
context. That threshold is also based on experiments & findings. If GPU is able
to catch up with CPU, typically there are no pending commands, the EU config
would remain unchanged there. In case there are more pending commands we
reprogram context with higher number of EUs. Please note, here we are changing
EUs even while context is running by examining pending commands every 'x'
milliseconds.

Srinivasan S (3):
  drm/i915: Get active pending request for given context
  drm/i915: set optimum eu/slice/sub-slice configuration based on load
    type
  drm/i915: Predictive governor to control slice/subslice/eu

 drivers/gpu/drm/i915/Makefile                     |   1 +
 drivers/gpu/drm/i915/gem/i915_gem_context.c       |  20 +++++
 drivers/gpu/drm/i915/gem/i915_gem_context.h       |   2 +
 drivers/gpu/drm/i915/gem/i915_gem_context_types.h |  38 ++++++++
 drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c    |   1 +
 drivers/gpu/drm/i915/gt/intel_deu.c               | 104 ++++++++++++++++++++++
 drivers/gpu/drm/i915/gt/intel_deu.h               |  31 +++++++
 drivers/gpu/drm/i915/gt/intel_lrc.c               |  44 ++++++++-
 drivers/gpu/drm/i915/i915_drv.h                   |   6 ++
 drivers/gpu/drm/i915/i915_gem.c                   |   4 +
 drivers/gpu/drm/i915/i915_params.c                |   4 +
 drivers/gpu/drm/i915/i915_params.h                |   1 +
 drivers/gpu/drm/i915/intel_device_info.c          |  74 ++++++++++++++-
 13 files changed, 325 insertions(+), 5 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/gt/intel_deu.c
 create mode 100644 drivers/gpu/drm/i915/gt/intel_deu.h

-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v7 1/3] drm/i915: Get active pending request for given context

[srinivasan.s]

From: Srinivasan S <srinivasan.s@intel.com>

This patch gives us the active pending request count which is yet
to be submitted to the GPU.

V2:
 * Change 64-bit to atomic for request count. (Tvrtko Ursulin)

V3:
 * Remove mutex for request count.
 * Rebase.
 * Fixes hitting underflow for predictive request. (Tvrtko Ursulin)

V4:
 * Rebase.

V5:
 * Rebase.

V6
 * Rebase.

V7
 * Added static table of slice/subslice/EU for JSL GEN11-LP.
 * Rebase.

Signed-off-by: Srinivasan S <srinivasan.s@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c       | 1 +
 drivers/gpu/drm/i915/gem/i915_gem_context_types.h | 6 ++++++
 drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c    | 1 +
 drivers/gpu/drm/i915/gt/intel_lrc.c               | 3 +++
 4 files changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 026999b34abd..d0ff999429ff 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -879,6 +879,7 @@ i915_gem_create_context(struct drm_i915_private *i915, unsigned int flags)
 	}
 
 	trace_i915_context_create(ctx);
+	atomic_set(&ctx->req_cnt, 0);
 
 	return ctx;
 }
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
index 28760bd03265..e26e94a0ab07 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
@@ -172,6 +172,12 @@ struct i915_gem_context {
 	struct radix_tree_root handles_vma;
 
 	/**
+	 * req_cnt: tracks the pending commands, based on which we decide to
+	 * go for low/medium/high load configuration of the GPU.
+	 */
+	atomic_t req_cnt;
+
+	/**
 	 * @name: arbitrary name, used for user debug
 	 *
 	 * A name is constructed for the context from the creator's process
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index d3f4f28e9468..2fe9ab20ec97 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -2565,6 +2565,7 @@ i915_gem_do_execbuffer(struct drm_device *dev,
 	if (batch->private)
 		intel_engine_pool_mark_active(batch->private, eb.request);
 
+	atomic_inc(&eb.gem_context->req_cnt);
 	trace_i915_request_queue(eb.request, eb.batch_flags);
 	err = eb_submit(&eb, batch);
 err_request:
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 112531b29f59..c58fc4329944 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -2158,6 +2158,9 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
 
 				submit = true;
 				last = rq;
+
+				if (atomic_read(&rq->context->gem_context->req_cnt) > 0)
+					atomic_dec(&rq->context->gem_context->req_cnt);
 			}
 		}
 
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v7 2/3] drm/i915: set optimum eu/slice/sub-slice configuration based on load type

[srinivasan.s]

From: Srinivasan S <srinivasan.s@intel.com>

This patch will select optimum eu/slice/sub-slice configuration based on
type of load (low, medium, high) as input.
Based on our readings and experiments we have predefined set of optimum
configuration for each platform(CHT, KBL).
i915_gem_context_set_load_type will select optimum configuration from
pre-defined optimum configuration table(opt_config).

It also introduce flag update_render_config which can set by any
governor.

v2:
 * Move static optimum_config to device init time.
 * Rename function to appropriate name, fix data types and patch
 * ordering.
 * Rename prev_load_type to pending_load_type. (Tvrtko Ursulin)

v3:
 * Add safe guard check in i915_gem_context_set_load_type.
 * Rename struct from optimum_config to i915_sseu_optimum_config to
   avoid namespace clashes.
 * Reduces memcpy for space efficient.
 * Rebase.
 * Improved commit message. (Tvrtko Ursulin)

v4:
 * Move optimum config table to file scope. (Tvrtko Ursulin)

v5:
 * Adds optimal table of slice/sub-slice/EU for Gen 9 GT1.
 * Rebase.

v6:
 * Rebase.
 * Fix warnings.

V7:
 * Added static table of slice/subslice/EU for JSL GEN11-LP.
 * Rebase.

Signed-off-by: Srinivasan S <srinivasan.s@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c       | 19 ++++++
 drivers/gpu/drm/i915/gem/i915_gem_context.h       |  2 +
 drivers/gpu/drm/i915/gem/i915_gem_context_types.h | 32 ++++++++++
 drivers/gpu/drm/i915/gt/intel_lrc.c               | 41 ++++++++++++-
 drivers/gpu/drm/i915/i915_drv.h                   |  4 ++
 drivers/gpu/drm/i915/intel_device_info.c          | 74 ++++++++++++++++++++++-
 6 files changed, 167 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index d0ff999429ff..86f91aa1b688 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -881,9 +881,28 @@ i915_gem_create_context(struct drm_i915_private *i915, unsigned int flags)
 	trace_i915_context_create(ctx);
 	atomic_set(&ctx->req_cnt, 0);
 
+	ctx->slice_cnt = hweight8(RUNTIME_INFO(i915)->sseu.slice_mask);
+	ctx->subslice_cnt = hweight8(RUNTIME_INFO(i915)->sseu.subslice_mask[0]);
+	ctx->eu_cnt = RUNTIME_INFO(i915)->sseu.eu_per_subslice;
+
 	return ctx;
 }
 
+void i915_gem_context_set_load_type(struct i915_gem_context *ctx,
+				enum gem_load_type type)
+{
+	struct drm_i915_private *dev_priv = ctx->i915;
+
+	if (GEM_WARN_ON(type > LOAD_TYPE_LAST))
+		return;
+
+	/* Call opt_config to get correct configuration for eu,slice,subslice */
+	ctx->slice_cnt = dev_priv->opt_config[type].slice;
+	ctx->subslice_cnt = dev_priv->opt_config[type].subslice;
+	ctx->eu_cnt = dev_priv->opt_config[type].eu;
+	ctx->pending_load_type = type;
+}
+
 static void init_contexts(struct i915_gem_contexts *gc)
 {
 	spin_lock_init(&gc->lock);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.h b/drivers/gpu/drm/i915/gem/i915_gem_context.h
index 57b7ae2893e1..70b75dc0cf98 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.h
@@ -133,6 +133,8 @@ int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data,
 				    struct drm_file *file_priv);
 int i915_gem_context_reset_stats_ioctl(struct drm_device *dev, void *data,
 				       struct drm_file *file);
+void i915_gem_context_set_load_type(struct i915_gem_context *ctx,
+				enum gem_load_type type);
 
 static inline struct i915_gem_context *
 i915_gem_context_get(struct i915_gem_context *ctx)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
index e26e94a0ab07..1c7fcf584d4e 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
@@ -46,6 +46,19 @@ struct i915_gem_engines_iter {
 	const struct i915_gem_engines *engines;
 };
 
+enum gem_load_type {
+	LOAD_TYPE_LOW,
+	LOAD_TYPE_MEDIUM,
+	LOAD_TYPE_HIGH,
+	LOAD_TYPE_LAST
+};
+
+struct i915_sseu_optimum_config {
+	u8 slice;
+	u8 subslice;
+	u8 eu;
+};
+
 /**
  * struct i915_gem_context - client state
  *
@@ -177,6 +190,25 @@ struct i915_gem_context {
 	 */
 	atomic_t req_cnt;
 
+	/** slice_cnt: used to set the # of slices to be enabled. */
+	u8 slice_cnt;
+
+	/** subslice_cnt: used to set the # of subslices to be enabled. */
+	u8 subslice_cnt;
+
+	/** eu_cnt: used to set the # of eu to be enabled. */
+	u8 eu_cnt;
+
+	/** load_type: The designated load_type (high/medium/low) for a given
+	 *  number of pending commands in the command queue.
+	 */
+	enum gem_load_type load_type;
+
+	/** pending_load_type: The earlier load type that the GPU was configured
+	 *  for (high/medium/low).
+	 */
+	enum gem_load_type pending_load_type;
+
 	/**
 	 * @name: arbitrary name, used for user debug
 	 *
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index c58fc4329944..3b085b4eb4fb 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -2996,6 +2996,34 @@ static void execlists_context_unpin(struct intel_context *ce)
 	i915_gem_object_unpin_map(ce->state->obj);
 }
 
+static u32
+get_context_rpcs_config(struct i915_gem_context *ctx)
+{
+	u32 rpcs = 0;
+	struct drm_i915_private *dev_priv = ctx->i915;
+
+	if (INTEL_GEN(dev_priv) < 8)
+		return 0;
+	if (RUNTIME_INFO(dev_priv)->sseu.has_slice_pg) {
+		rpcs |= GEN8_RPCS_S_CNT_ENABLE;
+		rpcs |= ctx->slice_cnt << GEN8_RPCS_S_CNT_SHIFT;
+		rpcs |= GEN8_RPCS_ENABLE;
+	}
+
+	if (RUNTIME_INFO(dev_priv)->sseu.has_subslice_pg) {
+		rpcs |= GEN8_RPCS_SS_CNT_ENABLE;
+		rpcs |= ctx->subslice_cnt << GEN8_RPCS_SS_CNT_SHIFT;
+		rpcs |= GEN8_RPCS_ENABLE;
+	}
+
+	if (RUNTIME_INFO(dev_priv)->sseu.has_eu_pg) {
+		rpcs |= ctx->eu_cnt << GEN8_RPCS_EU_MIN_SHIFT;
+		rpcs |= ctx->eu_cnt << GEN8_RPCS_EU_MAX_SHIFT;
+		rpcs |= GEN8_RPCS_ENABLE;
+	}
+	return rpcs;
+}
+
 static void
 __execlists_update_reg_state(const struct intel_context *ce,
 			     const struct intel_engine_cs *engine,
@@ -3014,8 +3042,14 @@ __execlists_update_reg_state(const struct intel_context *ce,
 
 	/* RPCS */
 	if (engine->class == RENDER_CLASS) {
-		regs[CTX_R_PWR_CLK_STATE] =
-			intel_sseu_make_rpcs(engine->i915, &ce->sseu);
+		if (engine->i915->predictive_load_enable) {
+			regs[CTX_R_PWR_CLK_STATE] =
+				get_context_rpcs_config(ce->gem_context);
+
+		} else {
+			regs[CTX_R_PWR_CLK_STATE] =
+				intel_sseu_make_rpcs(engine->i915, &ce->sseu);
+		}
 
 		i915_oa_init_reg_state(ce, engine);
 	}
@@ -3040,6 +3074,9 @@ __execlists_context_pin(struct intel_context *ce,
 	ce->lrc_reg_state = vaddr + LRC_STATE_PN * PAGE_SIZE;
 	__execlists_update_reg_state(ce, engine, ce->ring->tail);
 
+	if (ce->gem_context->load_type != ce->gem_context->pending_load_type)
+		ce->gem_context->load_type = ce->gem_context->pending_load_type;
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 19195bde4921..f039f644b44c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -926,6 +926,10 @@ struct drm_i915_private {
 	/* protects panel power sequencer state */
 	struct mutex pps_mutex;
 
+	/* optimal slice/subslice/EU configuration state */
+	struct i915_sseu_optimum_config *opt_config;
+	int predictive_load_enable;
+
 	unsigned int fsb_freq, mem_freq, is_ddr3;
 	unsigned int skl_preferred_vco_freq;
 	unsigned int max_cdclk_freq;
diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
index d7fe12734db8..0af260bbb02d 100644
--- a/drivers/gpu/drm/i915/intel_device_info.c
+++ b/drivers/gpu/drm/i915/intel_device_info.c
@@ -899,6 +899,41 @@ void intel_device_info_subplatform_init(struct drm_i915_private *i915)
 	RUNTIME_INFO(i915)->platform_mask[pi] |= mask;
 }
 
+/* static table of slice/subslice/EU for Cherryview */
+static const struct i915_sseu_optimum_config chv_config[LOAD_TYPE_LAST] = {
+	{1, 1, 4},	/* Low */
+	{1, 1, 6},	/* Medium */
+	{1, 2, 6}	/* High */
+};
+
+/* static table of slice/subslice/EU for GLK GT1 */
+static const struct i915_sseu_optimum_config glk_gt1_config[LOAD_TYPE_LAST] = {
+	{1, 2, 2},	/* Low */
+	{1, 2, 3},	/* Medium */
+	{1, 2, 6}	/* High */
+};
+
+/* static table of slice/subslice/EU for KBL GT2 */
+static const struct i915_sseu_optimum_config kbl_gt2_config[LOAD_TYPE_LAST] = {
+	{1, 3, 2},	/* Low */
+	{1, 3, 4},	/* Medium */
+	{1, 3, 8}	/* High */
+};
+
+/* static table of slice/subslice/EU for KBL GT3 */
+static const struct i915_sseu_optimum_config kbl_gt3_config[LOAD_TYPE_LAST] = {
+	{2, 3, 4},	/* Low */
+	{2, 3, 6},	/* Medium */
+	{2, 3, 8}	/* High */
+};
+
+/* static table of slice/subslice/EU for JSL GEN11LP */
+static struct i915_sseu_optimum_config jsl_gen11lp_config[LOAD_TYPE_LAST] = {
+	{1, 4, 4},      /* Low */
+	{1, 4, 6},      /* Medium */
+	{1, 4, 8}       /* High */
+};
+
 /**
  * intel_device_info_runtime_init - initialize runtime info
  * @dev_priv: the i915 device
@@ -920,6 +955,7 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv)
 	struct intel_device_info *info = mkwrite_device_info(dev_priv);
 	struct intel_runtime_info *runtime = RUNTIME_INFO(dev_priv);
 	enum pipe pipe;
+	struct i915_sseu_optimum_config *opt_config = NULL;
 
 	if (INTEL_GEN(dev_priv) >= 10) {
 		for_each_pipe(dev_priv, pipe)
@@ -1027,16 +1063,48 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv)
 	/* Initialize slice/subslice/EU info */
 	if (IS_HASWELL(dev_priv))
 		hsw_sseu_info_init(dev_priv);
-	else if (IS_CHERRYVIEW(dev_priv))
+	else if (IS_CHERRYVIEW(dev_priv)) {
 		cherryview_sseu_info_init(dev_priv);
+		opt_config = (struct i915_sseu_optimum_config *)chv_config;
+		BUILD_BUG_ON(ARRAY_SIZE(chv_config) != LOAD_TYPE_LAST);
+	}
 	else if (IS_BROADWELL(dev_priv))
 		bdw_sseu_info_init(dev_priv);
-	else if (IS_GEN(dev_priv, 9))
+	else if (IS_GEN(dev_priv, 9)) {
 		gen9_sseu_info_init(dev_priv);
+
+		switch (info->gt) {
+		default: /* fall through */
+		case 1:
+			opt_config = (struct i915_sseu_optimum_config *)
+						glk_gt1_config;
+			BUILD_BUG_ON(ARRAY_SIZE(glk_gt1_config)
+						!= LOAD_TYPE_LAST);
+		break;
+		case 2:
+			opt_config = (struct i915_sseu_optimum_config *)
+						kbl_gt2_config;
+			BUILD_BUG_ON(ARRAY_SIZE(kbl_gt2_config)
+						!= LOAD_TYPE_LAST);
+		break;
+		case 3:
+			opt_config = (struct i915_sseu_optimum_config *)
+						kbl_gt3_config;
+			BUILD_BUG_ON(ARRAY_SIZE(kbl_gt3_config)
+						!= LOAD_TYPE_LAST);
+		break;
+		}
+	}
 	else if (IS_GEN(dev_priv, 10))
 		gen10_sseu_info_init(dev_priv);
-	else if (IS_GEN(dev_priv, 11))
+	else if (IS_GEN(dev_priv, 11)) {
 		gen11_sseu_info_init(dev_priv);
+				opt_config = &jsl_gen11lp_config[0];
+		BUILD_BUG_ON(ARRAY_SIZE(jsl_gen11lp_config)
+					!= LOAD_TYPE_LAST);
+		if (opt_config)
+			dev_priv->opt_config = opt_config;
+	}
 	else if (INTEL_GEN(dev_priv) >= 12)
 		gen12_sseu_info_init(dev_priv);
 
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v7 3/3] drm/i915: Predictive governor to control slice/subslice/eu

[srinivasan.s]

From: Srinivasan S <srinivasan.s@intel.com>

High resolution timer is used for predictive governor to control
eu/slice/subslice based on workloads.

param is provided to enable/disable/update timer configuration

V2:
 * Fix code style.
 * Move predictive_load_timer into a drm_i915_private
   structure.
 * Make generic function to set optimum config. (Tvrtko Ursulin)

V3:
 * Rebase.
 * Fix race condition for predictive load set.
 * Add slack to start hrtimer for more power efficient. (Tvrtko Ursulin)

V4:
 * Fix data type and initialization of mutex to protect predictive load
   state.
 * Move predictive timer init to i915_gem_init_early. (Tvrtko Ursulin)
 * Move debugfs to kernel parameter.

V5:
 * Rebase.
 * Remove mutex for pred_timer

V6:
 * Rebase.
 * Fix warnings.

V7:
 * Added static table of slice/subslice/EU for JSL GEN11-LP.
 * Rebase.

Signed-off-by: Srinivasan S <srinivasan.s@intel.com>
---
 drivers/gpu/drm/i915/Makefile       |   1 +
 drivers/gpu/drm/i915/gt/intel_deu.c | 104 ++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/gt/intel_deu.h |  31 +++++++++++
 drivers/gpu/drm/i915/i915_drv.h     |   2 +
 drivers/gpu/drm/i915/i915_gem.c     |   4 ++
 drivers/gpu/drm/i915/i915_params.c  |   4 ++
 drivers/gpu/drm/i915/i915_params.h  |   1 +
 7 files changed, 147 insertions(+)
 create mode 100644 drivers/gpu/drm/i915/gt/intel_deu.c
 create mode 100644 drivers/gpu/drm/i915/gt/intel_deu.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 9f887a86e555..a66bd659c842 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -86,6 +86,7 @@ gt-y += \
 	gt/intel_context.o \
 	gt/intel_context_param.o \
 	gt/intel_context_sseu.o \
+	gt/intel_deu.o \
 	gt/intel_engine_cs.o \
 	gt/intel_engine_heartbeat.o \
 	gt/intel_engine_pm.o \
diff --git a/drivers/gpu/drm/i915/gt/intel_deu.c b/drivers/gpu/drm/i915/gt/intel_deu.c
new file mode 100644
index 000000000000..55fbde77fe84
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/intel_deu.c
@@ -0,0 +1,104 @@
+/*
+ * Copyright © 2019 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ * Authors:
+ *    Ankit Navik <ankit.p.navik@intel.com>
+ */
+
+/**
+ * DOC: Dynamic EU Control (DEUC)
+ *
+ * DEUC tries to re-configure EU allocation during runtime by predictive load
+ * calculation of command queue to gain power saving.
+ * It is transparent to user space and completely handled in the kernel.
+ */
+#include "intel_deu.h"
+#include "i915_drv.h"
+#include "gem/i915_gem_context.h"
+/*
+ * Anything above threshold is considered as HIGH load, less is considered
+ * as LOW load and equal is considered as MEDIUM load.
+ *
+ * The threshold value of three active requests pending.
+ */
+#define PENDING_THRESHOLD_MEDIUM 3
+
+#define SLACK_TIMER_NSEC 1000000 /* Timer range in nano second */
+
+enum hrtimer_restart predictive_load_cb(struct hrtimer *hrtimer)
+{
+	struct drm_i915_private *dev_priv =
+			container_of(hrtimer, typeof(*dev_priv), pred_timer);
+	struct i915_gem_context *ctx;
+	enum gem_load_type load_type;
+	unsigned int req_pending;
+
+	list_for_each_entry(ctx, &dev_priv->gem.contexts.list, link) {
+
+		req_pending = atomic_read(&ctx->req_cnt);
+
+		/*
+		 * Transitioning to low state whenever pending request is zero
+		 * would cause vacillation between low and high state.
+		 */
+		if (req_pending == 0)
+			continue;
+
+		if (req_pending > PENDING_THRESHOLD_MEDIUM)
+			load_type = LOAD_TYPE_HIGH;
+		else if (req_pending == PENDING_THRESHOLD_MEDIUM)
+			load_type = LOAD_TYPE_MEDIUM;
+		else
+			load_type = LOAD_TYPE_LOW;
+
+		i915_gem_context_set_load_type(ctx, load_type);
+	}
+
+	hrtimer_forward_now(hrtimer,
+			ms_to_ktime(dev_priv->predictive_load_enable));
+
+	return HRTIMER_RESTART;
+}
+
+/**
+ * intel_deu_init - Initialize dynamic EU
+ * @dev_priv: i915 device instance
+ *
+ * This function is called at driver load
+ */
+void intel_deu_init(struct drm_i915_private *dev_priv)
+{
+	dev_priv->predictive_load_enable = i915_modparams.deu_enable;
+	hrtimer_init(&dev_priv->pred_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+	dev_priv->pred_timer.function = predictive_load_cb;
+
+	if (dev_priv->predictive_load_enable) {
+		if (!hrtimer_active(&dev_priv->pred_timer))
+			hrtimer_start_range_ns(&dev_priv->pred_timer,
+			ms_to_ktime(dev_priv->predictive_load_enable),
+			SLACK_TIMER_NSEC,
+			HRTIMER_MODE_REL_PINNED);
+	} else {
+		hrtimer_cancel(&dev_priv->pred_timer);
+	}
+
+}
diff --git a/drivers/gpu/drm/i915/gt/intel_deu.h b/drivers/gpu/drm/i915/gt/intel_deu.h
new file mode 100644
index 000000000000..3b4b16f16238
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/intel_deu.h
@@ -0,0 +1,31 @@
+/*
+ * Copyright © 2019 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+#ifndef __INTEL_DEU_H__
+#define __INTEL_DEU_H__
+
+struct drm_i915_private;
+
+void intel_deu_init(struct drm_i915_private *dev_priv);
+
+#endif /* __INTEL_DEU_H__ */
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f039f644b44c..af5ce568753f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -928,6 +928,7 @@ struct drm_i915_private {
 
 	/* optimal slice/subslice/EU configuration state */
 	struct i915_sseu_optimum_config *opt_config;
+	struct hrtimer pred_timer;
 	int predictive_load_enable;
 
 	unsigned int fsb_freq, mem_freq, is_ddr3;
@@ -1674,6 +1675,7 @@ intel_ggtt_update_needs_vtd_wa(struct drm_i915_private *dev_priv)
 
 /* i915_drv.c */
 extern const struct dev_pm_ops i915_pm_ops;
+extern enum hrtimer_restart predictive_load_cb(struct hrtimer *hrtimer);
 
 int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent);
 void i915_driver_remove(struct drm_i915_private *i915);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index ca5420012a22..6a9addb10e6d 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -46,6 +46,7 @@
 #include "gem/i915_gem_mman.h"
 #include "gem/i915_gem_region.h"
 #include "gt/intel_engine_user.h"
+#include "gt/intel_deu.h"
 #include "gt/intel_gt.h"
 #include "gt/intel_gt_pm.h"
 #include "gt/intel_workarounds.h"
@@ -1224,6 +1225,9 @@ void i915_gem_init_early(struct drm_i915_private *dev_priv)
 	i915_gem_init__contexts(dev_priv);
 
 	spin_lock_init(&dev_priv->fb_tracking.lock);
+
+	/* Dynamic EU timer initialization for predictive load */
+	intel_deu_init(dev_priv);
 }
 
 void i915_gem_cleanup_early(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index add00ec1f787..f581894d448f 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -98,6 +98,10 @@ i915_param_named_unsafe(disable_power_well, int, 0400,
 
 i915_param_named_unsafe(enable_ips, int, 0600, "Enable IPS (default: true)");
 
+i915_param_named_unsafe(deu_enable, int, 0600,
+	"Enable dynamic EU control for power savings "
+	"(0=disable deu predictive timer [default], 150=optimal deu predictive timer)");
+
 i915_param_named(fastboot, int, 0600,
 	"Try to skip unnecessary mode sets at boot time "
 	"(0=disabled, 1=enabled) "
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index 45323732f099..ff790dec9ecd 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -56,6 +56,7 @@ struct drm_printer;
 	param(int, disable_power_well, -1, 0400) \
 	param(int, enable_ips, 1, 0600) \
 	param(int, invert_brightness, 0, 0600) \
+	param(int, deu_enable, 0, 0600) \
 	param(int, enable_guc, 0, 0400) \
 	param(int, guc_log_level, -1, 0400) \
 	param(char *, guc_firmware_path, NULL, 0400) \
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Dynamic EU configuration of Slice/Sub-slice/EU (rev5)

[Patchwork]

== Series Details ==

Series: Dynamic EU configuration of Slice/Sub-slice/EU (rev5)
URL   : https://patchwork.freedesktop.org/series/69980/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
b969a95d47cd drm/i915: Get active pending request for given context
80a1bbfce2f4 drm/i915: set optimum eu/slice/sub-slice configuration based on load type
-:64: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#64: FILE: drivers/gpu/drm/i915/gem/i915_gem_context.c:892:
+void i915_gem_context_set_load_type(struct i915_gem_context *ctx,
+				enum gem_load_type type)

-:90: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#90: FILE: drivers/gpu/drm/i915/gem/i915_gem_context.h:137:
+void i915_gem_context_set_load_type(struct i915_gem_context *ctx,
+				enum gem_load_type type);

total: 0 errors, 0 warnings, 2 checks, 248 lines checked
c41c32b36236 drm/i915: Predictive governor to control slice/subslice/eu
-:55: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#55: 
new file mode 100644

-:60: WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1
#60: FILE: drivers/gpu/drm/i915/gt/intel_deu.c:1:
+/*

-:115: CHECK:BRACES: Blank lines aren't necessary after an open brace '{'
#115: FILE: drivers/gpu/drm/i915/gt/intel_deu.c:56:
+	list_for_each_entry(ctx, &dev_priv->gem.contexts.list, link) {
+

-:136: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#136: FILE: drivers/gpu/drm/i915/gt/intel_deu.c:77:
+	hrtimer_forward_now(hrtimer,
+			ms_to_ktime(dev_priv->predictive_load_enable));

-:156: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#156: FILE: drivers/gpu/drm/i915/gt/intel_deu.c:97:
+			hrtimer_start_range_ns(&dev_priv->pred_timer,
+			ms_to_ktime(dev_priv->predictive_load_enable),

-:163: CHECK:BRACES: Blank lines aren't necessary before a close brace '}'
#163: FILE: drivers/gpu/drm/i915/gt/intel_deu.c:104:
+
+}

-:170: WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1
#170: FILE: drivers/gpu/drm/i915/gt/intel_deu.h:1:
+/*

-:217: CHECK:AVOID_EXTERNS: extern prototypes should be avoided in .h files
#217: FILE: drivers/gpu/drm/i915/i915_drv.h:1678:
+extern enum hrtimer_restart predictive_load_cb(struct hrtimer *hrtimer);

-:252: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#252: FILE: drivers/gpu/drm/i915/i915_params.c:102:
+i915_param_named_unsafe(deu_enable, int, 0600,
+	"Enable dynamic EU control for power savings "

total: 0 errors, 3 warnings, 6 checks, 189 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for Dynamic EU configuration of Slice/Sub-slice/EU (rev5)

[Patchwork]

== Series Details ==

Series: Dynamic EU configuration of Slice/Sub-slice/EU (rev5)
URL   : https://patchwork.freedesktop.org/series/69980/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.6.0
Commit: drm/i915: Get active pending request for given context
+drivers/gpu/drm/i915/gt/intel_lrc.c:2162:61:    expected struct atomic_t const [usertype] *v
+drivers/gpu/drm/i915/gt/intel_lrc.c:2162:61:    got struct atomic_t [noderef] <asn:4> *
+drivers/gpu/drm/i915/gt/intel_lrc.c:2162:61: warning: incorrect type in argument 1 (different address spaces)
+drivers/gpu/drm/i915/gt/intel_lrc.c:2163:64:    expected struct atomic_t [usertype] *v
+drivers/gpu/drm/i915/gt/intel_lrc.c:2163:64:    got struct atomic_t [noderef] <asn:4> *
+drivers/gpu/drm/i915/gt/intel_lrc.c:2163:64: warning: incorrect type in argument 1 (different address spaces)

Commit: drm/i915: set optimum eu/slice/sub-slice configuration based on load type
+drivers/gpu/drm/i915/gt/intel_lrc.c:3047:59:    expected struct i915_gem_context *ctx
+drivers/gpu/drm/i915/gt/intel_lrc.c:3047:59:    got struct i915_gem_context [noderef] <asn:4> *const gem_context
+drivers/gpu/drm/i915/gt/intel_lrc.c:3047:59: warning: incorrect type in argument 1 (different address spaces)
+drivers/gpu/drm/i915/gt/intel_lrc.c:3077:15: warning: dereference of noderef expression
+drivers/gpu/drm/i915/gt/intel_lrc.c:3077:45: warning: dereference of noderef expression
+drivers/gpu/drm/i915/gt/intel_lrc.c:3078:19: warning: dereference of noderef expression
+drivers/gpu/drm/i915/gt/intel_lrc.c:3078:48: warning: dereference of noderef expression

Commit: drm/i915: Predictive governor to control slice/subslice/eu
Okay!

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.BAT: failure for Dynamic EU configuration of Slice/Sub-slice/EU (rev5)

[Patchwork]

== Series Details ==

Series: Dynamic EU configuration of Slice/Sub-slice/EU (rev5)
URL   : https://patchwork.freedesktop.org/series/69980/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_8133 -> Patchwork_16963
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_16963 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_16963, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16963/index.html


Changes
-------

  No changes found


Participating hosts (36 -> 11)
------------------------------

  ERROR: It appears as if the changes made in Patchwork_16963 prevented too many machines from booting.

  Additional (5): fi-hsw-peppy fi-byt-j1900 fi-bwr-2160 fi-snb-2520m fi-blb-e6850 
  Missing    (30): fi-bdw-gvtdvm fi-icl-u2 fi-apl-guc fi-icl-y fi-skl-lmem fi-icl-guc fi-icl-dsi fi-skl-6600u fi-cml-u2 fi-bxt-dsi fi-tgl-u fi-cml-s fi-bsw-n3050 fi-glk-dsi fi-kbl-7500u fi-kbl-7560u fi-tgl-y fi-skl-6700k2 fi-kbl-r fi-ehl-1 fi-tgl-dsi fi-skl-guc fi-cfl-8700k fi-hsw-4200u fi-cfl-guc fi-kbl-guc fi-kbl-x1275 fi-kbl-8809g fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_8133 -> Patchwork_16963

  CI-20190529: 20190529
  CI_DRM_8133: e9912ab15be5d104fafd00c5e72361ae3b8839a5 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5508: 89d644600a1a9f08794cc7106b63758df40ce1d8 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_16963: c41c32b36236b5bef102d5542d579ce590cc4f19 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

c41c32b36236 drm/i915: Predictive governor to control slice/subslice/eu
80a1bbfce2f4 drm/i915: set optimum eu/slice/sub-slice configuration based on load type
b969a95d47cd drm/i915: Get active pending request for given context

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16963/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Dynamic EU configuration of Slice/Sub-slice/EU (rev6)

[Patchwork]

== Series Details ==

Series: Dynamic EU configuration of Slice/Sub-slice/EU (rev6)
URL   : https://patchwork.freedesktop.org/series/69980/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
9df798d37b3b drm/i915: Get active pending request for given context
d713a976ff0d drm/i915: set optimum eu/slice/sub-slice configuration based on load type
-:64: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#64: FILE: drivers/gpu/drm/i915/gem/i915_gem_context.c:892:
+void i915_gem_context_set_load_type(struct i915_gem_context *ctx,
+				enum gem_load_type type)

-:90: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#90: FILE: drivers/gpu/drm/i915/gem/i915_gem_context.h:137:
+void i915_gem_context_set_load_type(struct i915_gem_context *ctx,
+				enum gem_load_type type);

total: 0 errors, 0 warnings, 2 checks, 248 lines checked
22a335ca33bf drm/i915: Predictive governor to control slice/subslice/eu
-:55: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#55: 
new file mode 100644

-:60: WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1
#60: FILE: drivers/gpu/drm/i915/gt/intel_deu.c:1:
+/*

-:115: CHECK:BRACES: Blank lines aren't necessary after an open brace '{'
#115: FILE: drivers/gpu/drm/i915/gt/intel_deu.c:56:
+	list_for_each_entry(ctx, &dev_priv->gem.contexts.list, link) {
+

-:136: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#136: FILE: drivers/gpu/drm/i915/gt/intel_deu.c:77:
+	hrtimer_forward_now(hrtimer,
+			ms_to_ktime(dev_priv->predictive_load_enable));

-:156: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#156: FILE: drivers/gpu/drm/i915/gt/intel_deu.c:97:
+			hrtimer_start_range_ns(&dev_priv->pred_timer,
+			ms_to_ktime(dev_priv->predictive_load_enable),

-:163: CHECK:BRACES: Blank lines aren't necessary before a close brace '}'
#163: FILE: drivers/gpu/drm/i915/gt/intel_deu.c:104:
+
+}

-:170: WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1
#170: FILE: drivers/gpu/drm/i915/gt/intel_deu.h:1:
+/*

-:217: CHECK:AVOID_EXTERNS: extern prototypes should be avoided in .h files
#217: FILE: drivers/gpu/drm/i915/i915_drv.h:1678:
+extern enum hrtimer_restart predictive_load_cb(struct hrtimer *hrtimer);

-:252: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#252: FILE: drivers/gpu/drm/i915/i915_params.c:102:
+i915_param_named_unsafe(deu_enable, int, 0600,
+	"Enable dynamic EU control for power savings "

total: 0 errors, 3 warnings, 6 checks, 189 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for Dynamic EU configuration of Slice/Sub-slice/EU (rev6)

[Patchwork]

== Series Details ==

Series: Dynamic EU configuration of Slice/Sub-slice/EU (rev6)
URL   : https://patchwork.freedesktop.org/series/69980/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.6.0
Commit: drm/i915: Get active pending request for given context
+drivers/gpu/drm/i915/gt/intel_lrc.c:2162:61:    expected struct atomic_t const [usertype] *v
+drivers/gpu/drm/i915/gt/intel_lrc.c:2162:61:    got struct atomic_t [noderef] <asn:4> *
+drivers/gpu/drm/i915/gt/intel_lrc.c:2162:61: warning: incorrect type in argument 1 (different address spaces)
+drivers/gpu/drm/i915/gt/intel_lrc.c:2163:64:    expected struct atomic_t [usertype] *v
+drivers/gpu/drm/i915/gt/intel_lrc.c:2163:64:    got struct atomic_t [noderef] <asn:4> *
+drivers/gpu/drm/i915/gt/intel_lrc.c:2163:64: warning: incorrect type in argument 1 (different address spaces)

Commit: drm/i915: set optimum eu/slice/sub-slice configuration based on load type
+drivers/gpu/drm/i915/gt/intel_lrc.c:3047:59:    expected struct i915_gem_context *ctx
+drivers/gpu/drm/i915/gt/intel_lrc.c:3047:59:    got struct i915_gem_context [noderef] <asn:4> *const gem_context
+drivers/gpu/drm/i915/gt/intel_lrc.c:3047:59: warning: incorrect type in argument 1 (different address spaces)
+drivers/gpu/drm/i915/gt/intel_lrc.c:3077:15: warning: dereference of noderef expression
+drivers/gpu/drm/i915/gt/intel_lrc.c:3077:45: warning: dereference of noderef expression
+drivers/gpu/drm/i915/gt/intel_lrc.c:3078:19: warning: dereference of noderef expression
+drivers/gpu/drm/i915/gt/intel_lrc.c:3078:48: warning: dereference of noderef expression

Commit: drm/i915: Predictive governor to control slice/subslice/eu
Okay!

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.BAT: failure for Dynamic EU configuration of Slice/Sub-slice/EU (rev6)

[Patchwork]

== Series Details ==

Series: Dynamic EU configuration of Slice/Sub-slice/EU (rev6)
URL   : https://patchwork.freedesktop.org/series/69980/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_8133 -> Patchwork_16964
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_16964 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_16964, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16964/index.html


Changes
-------

  No changes found


Participating hosts (36 -> 8)
------------------------------

  ERROR: It appears as if the changes made in Patchwork_16964 prevented too many machines from booting.

  Additional (4): fi-byt-j1900 fi-blb-e6850 fi-bwr-2160 fi-snb-2520m 
  Missing    (32): fi-bdw-gvtdvm fi-icl-u2 fi-apl-guc fi-icl-y fi-skl-lmem fi-icl-guc fi-icl-dsi fi-skl-6600u fi-snb-2600 fi-cml-u2 fi-bxt-dsi fi-tgl-u fi-cml-s fi-bsw-n3050 fi-glk-dsi fi-kbl-7500u fi-ivb-3770 fi-kbl-7560u fi-tgl-y fi-skl-6700k2 fi-kbl-r fi-ehl-1 fi-tgl-dsi fi-skl-guc fi-cfl-8700k fi-hsw-4200u fi-cfl-guc fi-kbl-guc fi-kbl-x1275 fi-kbl-8809g fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_8133 -> Patchwork_16964

  CI-20190529: 20190529
  CI_DRM_8133: e9912ab15be5d104fafd00c5e72361ae3b8839a5 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5508: 89d644600a1a9f08794cc7106b63758df40ce1d8 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_16964: 22a335ca33bf08e6dc1f7c4d5ff24b6fda2991d0 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

22a335ca33bf drm/i915: Predictive governor to control slice/subslice/eu
d713a976ff0d drm/i915: set optimum eu/slice/sub-slice configuration based on load type
9df798d37b3b drm/i915: Get active pending request for given context

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16964/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v7 0/3] Dynamic EU configuration of Slice/Sub-slice/EU

[Tvrtko Ursulin]

Hi,

On 13/03/2020 11:12, srinivasan.s@intel.com wrote:
> From: Srinivasan S <srinivasan.s@intel.com>
> 
>        drm/i915: Context aware user agnostic EU/Slice/Sub-slice control within kernel
> 
> This patch sets improves GPU power consumption on Linux kernel based OS such as
> Chromium OS, Ubuntu, etc. Following are the power savings.
> 
> Power savings on GLK-GT1 Bobba platform running on Chrome OS.
> -----------------------------------------------|
> App /KPI                | % Power Benefit (mW) |
> ------------------------|----------------------|
> Hangout Call- 20 minute |	1.8%           |
> Youtube 4K VPB          |       14.13%         |
> WebGL Aquarium          |       13.76%         |
> Unity3D                 |       6.78%          |
> 			|		       |
> ------------------------|----------------------|
> Chrome PLT              | BatteryLife Improves |
> 			| by ~45 minute        |
> -----------------------------------------------|
> 
> Power savings on KBL-GT3 running on  Android and Ubuntu (Linux).
> -----------------------------------------------|
> App /KPI		| % Power Benefit (mW) |
>                          |----------------------|
> 			|  Android |  Ubuntu   |
> ------------------------|----------|-----------|
> 3D Mark (Ice storm)     | 2.30%    | N.A.      |
> TRex On screen          | 2.49%    | 2.97%     |
> Manhattan On screen     | 3.11%    | 4.90%     |
> Carchase On Screen	| N.A.     | 5.06%     |
> AnTuTu 6.1.4            | 3.42%    | N.A.      |
> SynMark2		| N.A.     | 1.7%      |
> -----------------------------------------------|

Have a look at the result Francisco obtained on Icelake with a different 
approach: https://patchwork.freedesktop.org/series/74540/

Not all benchmarks overlap but if you are set up to easily test his 
patches it may be for a mutual benefit.

Regards,

Tvrtko

> We have also observed GPU core residencies improves by 1.035%.
> 
> Technical Insights of the patch:
> Current GPU configuration code for i915 does not allow us to change
> EU/Slice/Sub-slice configuration dynamically. Its done only once while context
> is created.
> 
> While particular graphics application is running, if we examine the command
> requests from user space, we observe that command density is not consistent.
> It means there is scope to change the graphics configuration dynamically even
> while context is running actively. This patch series proposes the solution to
> find the active pending load for all active context at given time and based on
> that, dynamically perform graphics configuration for each context.
> 
> We use a hr (high resolution) timer with i915 driver in kernel to get a
> callback every few milliseconds (this timer value can be configured through
> debugfs, default is '0' indicating timer is in disabled state i.e. original
> system without any intervention).In the timer callback, we examine pending
> commands for a context in the queue, essentially, we intercept them before
> they are executed by GPU and we update context with required number of EUs.
> 
> Two questions, how did we arrive at right timer value? and what's the right
> number of EUs? For the prior one, empirical data to achieve best performance
> in least power was considered. For the later one, we roughly categorized number
> of EUs logically based on platform. Now we compare number of pending commands
> with a particular threshold and then set number of EUs accordingly with update
> context. That threshold is also based on experiments & findings. If GPU is able
> to catch up with CPU, typically there are no pending commands, the EU config
> would remain unchanged there. In case there are more pending commands we
> reprogram context with higher number of EUs. Please note, here we are changing
> EUs even while context is running by examining pending commands every 'x'
> milliseconds.
> 
> Srinivasan S (3):
>    drm/i915: Get active pending request for given context
>    drm/i915: set optimum eu/slice/sub-slice configuration based on load
>      type
>    drm/i915: Predictive governor to control slice/subslice/eu
> 
>   drivers/gpu/drm/i915/Makefile                     |   1 +
>   drivers/gpu/drm/i915/gem/i915_gem_context.c       |  20 +++++
>   drivers/gpu/drm/i915/gem/i915_gem_context.h       |   2 +
>   drivers/gpu/drm/i915/gem/i915_gem_context_types.h |  38 ++++++++
>   drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c    |   1 +
>   drivers/gpu/drm/i915/gt/intel_deu.c               | 104 ++++++++++++++++++++++
>   drivers/gpu/drm/i915/gt/intel_deu.h               |  31 +++++++
>   drivers/gpu/drm/i915/gt/intel_lrc.c               |  44 ++++++++-
>   drivers/gpu/drm/i915/i915_drv.h                   |   6 ++
>   drivers/gpu/drm/i915/i915_gem.c                   |   4 +
>   drivers/gpu/drm/i915/i915_params.c                |   4 +
>   drivers/gpu/drm/i915/i915_params.h                |   1 +
>   drivers/gpu/drm/i915/intel_device_info.c          |  74 ++++++++++++++-
>   13 files changed, 325 insertions(+), 5 deletions(-)
>   create mode 100644 drivers/gpu/drm/i915/gt/intel_deu.c
>   create mode 100644 drivers/gpu/drm/i915/gt/intel_deu.h
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v7 0/3] Dynamic EU configuration of Slice/Sub-slice/EU

[S, Srinivasan]

> -----Original Message-----
> From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> Sent: Friday, March 13, 2020 10:48 PM
> To: S, Srinivasan <srinivasan.s@intel.com>; intel-gfx@lists.freedesktop.org;
> chris@chris-wilson.co.uk; Francisco Jerez <currojerez@riseup.net>
> Subject: Re: [Intel-gfx] [PATCH v7 0/3] Dynamic EU configuration of Slice/Sub-
> slice/EU
> 
> 
> Hi,
> 
> On 13/03/2020 11:12, srinivasan.s@intel.com wrote:
> > From: Srinivasan S <srinivasan.s@intel.com>
> >
> >        drm/i915: Context aware user agnostic EU/Slice/Sub-slice control within
> kernel
> >
> > This patch sets improves GPU power consumption on Linux kernel based OS
> such as
> > Chromium OS, Ubuntu, etc. Following are the power savings.
> >
> > Power savings on GLK-GT1 Bobba platform running on Chrome OS.
> > -----------------------------------------------|
> > App /KPI                | % Power Benefit (mW) |
> > ------------------------|----------------------|
> > Hangout Call- 20 minute |	1.8%           |
> > Youtube 4K VPB          |       14.13%         |
> > WebGL Aquarium          |       13.76%         |
> > Unity3D                 |       6.78%          |
> > 			|		       |
> > ------------------------|----------------------|
> > Chrome PLT              | BatteryLife Improves |
> > 			| by ~45 minute        |
> > -----------------------------------------------|
> >
> > Power savings on KBL-GT3 running on  Android and Ubuntu (Linux).
> > -----------------------------------------------|
> > App /KPI		| % Power Benefit (mW) |
> >                          |----------------------|
> > 			|  Android |  Ubuntu   |
> > ------------------------|----------|-----------|
> > 3D Mark (Ice storm)     | 2.30%    | N.A.      |
> > TRex On screen          | 2.49%    | 2.97%     |
> > Manhattan On screen     | 3.11%    | 4.90%     |
> > Carchase On Screen	| N.A.     | 5.06%     |
> > AnTuTu 6.1.4            | 3.42%    | N.A.      |
> > SynMark2		| N.A.     | 1.7%      |
> > -----------------------------------------------|
> 
> Have a look at the result Francisco obtained on Icelake with a different
> approach: https://patchwork.freedesktop.org/series/74540/
> 
> Not all benchmarks overlap but if you are set up to easily test his
> patches it may be for a mutual benefit.
[S, Srinivasan]  Thanks!,  Could we reuse only his gfx related patches alone as below - to focus on our gfx power saving benefits as first step, or could you please let me know the entire patch series needs to be considered or is there any dependency of GPU power on CPU?
ie., https://patchwork.freedesktop.org/patch/357098/?series=74540&rev=2
https://patchwork.freedesktop.org/patch/357103/?series=74540&rev=2
> 
> Regards,
> 
> Tvrtko
> 
> > We have also observed GPU core residencies improves by 1.035%.
> >
> > Technical Insights of the patch:
> > Current GPU configuration code for i915 does not allow us to change
> > EU/Slice/Sub-slice configuration dynamically. Its done only once while context
> > is created.
> >
> > While particular graphics application is running, if we examine the command
> > requests from user space, we observe that command density is not consistent.
> > It means there is scope to change the graphics configuration dynamically even
> > while context is running actively. This patch series proposes the solution to
> > find the active pending load for all active context at given time and based on
> > that, dynamically perform graphics configuration for each context.
> >
> > We use a hr (high resolution) timer with i915 driver in kernel to get a
> > callback every few milliseconds (this timer value can be configured through
> > debugfs, default is '0' indicating timer is in disabled state i.e. original
> > system without any intervention).In the timer callback, we examine pending
> > commands for a context in the queue, essentially, we intercept them before
> > they are executed by GPU and we update context with required number of
> EUs.
> >
> > Two questions, how did we arrive at right timer value? and what's the right
> > number of EUs? For the prior one, empirical data to achieve best performance
> > in least power was considered. For the later one, we roughly categorized
> number
> > of EUs logically based on platform. Now we compare number of pending
> commands
> > with a particular threshold and then set number of EUs accordingly with
> update
> > context. That threshold is also based on experiments & findings. If GPU is able
> > to catch up with CPU, typically there are no pending commands, the EU config
> > would remain unchanged there. In case there are more pending commands we
> > reprogram context with higher number of EUs. Please note, here we are
> changing
> > EUs even while context is running by examining pending commands every 'x'
> > milliseconds.
> >
> > Srinivasan S (3):
> >    drm/i915: Get active pending request for given context
> >    drm/i915: set optimum eu/slice/sub-slice configuration based on load
> >      type
> >    drm/i915: Predictive governor to control slice/subslice/eu
> >
> >   drivers/gpu/drm/i915/Makefile                     |   1 +
> >   drivers/gpu/drm/i915/gem/i915_gem_context.c       |  20 +++++
> >   drivers/gpu/drm/i915/gem/i915_gem_context.h       |   2 +
> >   drivers/gpu/drm/i915/gem/i915_gem_context_types.h |  38 ++++++++
> >   drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c    |   1 +
> >   drivers/gpu/drm/i915/gt/intel_deu.c               | 104
> ++++++++++++++++++++++
> >   drivers/gpu/drm/i915/gt/intel_deu.h               |  31 +++++++
> >   drivers/gpu/drm/i915/gt/intel_lrc.c               |  44 ++++++++-
> >   drivers/gpu/drm/i915/i915_drv.h                   |   6 ++
> >   drivers/gpu/drm/i915/i915_gem.c                   |   4 +
> >   drivers/gpu/drm/i915/i915_params.c                |   4 +
> >   drivers/gpu/drm/i915/i915_params.h                |   1 +
> >   drivers/gpu/drm/i915/intel_device_info.c          |  74 ++++++++++++++-
> >   13 files changed, 325 insertions(+), 5 deletions(-)
> >   create mode 100644 drivers/gpu/drm/i915/gt/intel_deu.c
> >   create mode 100644 drivers/gpu/drm/i915/gt/intel_deu.h
> >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] ✗ Fi.CI.SPARSE: warning for Dynamic EU configuration of Slice/Sub-slice/EU (rev6)

[Navik, Ankit P]

Hi Srinivas, 

This will break OA counter.
I am already working with Tvrtko to make patch scalable
and to make it as per the flow. 
Kindly wait for upcoming patch. 

PS: Coding guideline: https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html 

Regards,
Ankit

> -----Original Message-----
> From: Patchwork <patchwork@emeril.freedesktop.org>
> Sent: Friday, March 13, 2020 7:34 PM
> To: Navik, Ankit P <ankit.p.navik@intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Subject: ✗ Fi.CI.SPARSE: warning for Dynamic EU configuration of Slice/Sub-
> slice/EU (rev6)
> 
> == Series Details ==
> 
> Series: Dynamic EU configuration of Slice/Sub-slice/EU (rev6)
> URL   : https://patchwork.freedesktop.org/series/69980/
> State : warning
> 
> == Summary ==
> 
> $ dim sparse origin/drm-tip
> Sparse version: v0.6.0
> Commit: drm/i915: Get active pending request for given context
> +drivers/gpu/drm/i915/gt/intel_lrc.c:2162:61:    expected struct atomic_t const
> [usertype] *v
> +drivers/gpu/drm/i915/gt/intel_lrc.c:2162:61:    got struct atomic_t [noderef]
> <asn:4> *
> +drivers/gpu/drm/i915/gt/intel_lrc.c:2162:61: warning: incorrect type in
> argument 1 (different address spaces)
> +drivers/gpu/drm/i915/gt/intel_lrc.c:2163:64:    expected struct atomic_t
> [usertype] *v
> +drivers/gpu/drm/i915/gt/intel_lrc.c:2163:64:    got struct atomic_t [noderef]
> <asn:4> *
> +drivers/gpu/drm/i915/gt/intel_lrc.c:2163:64: warning: incorrect type in
> +argument 1 (different address spaces)
> 
> Commit: drm/i915: set optimum eu/slice/sub-slice configuration based on load
> type
> +drivers/gpu/drm/i915/gt/intel_lrc.c:3047:59:    expected struct
> i915_gem_context *ctx
> +drivers/gpu/drm/i915/gt/intel_lrc.c:3047:59:    got struct i915_gem_context
> [noderef] <asn:4> *const gem_context
> +drivers/gpu/drm/i915/gt/intel_lrc.c:3047:59: warning: incorrect type in
> +argument 1 (different address spaces)
> +drivers/gpu/drm/i915/gt/intel_lrc.c:3077:15: warning: dereference of
> +noderef expression
> +drivers/gpu/drm/i915/gt/intel_lrc.c:3077:45: warning: dereference of
> +noderef expression
> +drivers/gpu/drm/i915/gt/intel_lrc.c:3078:19: warning: dereference of
> +noderef expression
> +drivers/gpu/drm/i915/gt/intel_lrc.c:3078:48: warning: dereference of
> +noderef expression
> 
> Commit: drm/i915: Predictive governor to control slice/subslice/eu Okay!

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] ✗ Fi.CI.SPARSE: warning for Dynamic EU configuration of Slice/Sub-slice/EU (rev6)

[S, Srinivasan]

May I know any specific timelines?

> -----Original Message-----
> From: Navik, Ankit P <ankit.p.navik@intel.com>
> Sent: Saturday, March 14, 2020 10:03 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: S, Srinivasan <srinivasan.s@intel.com>; Ursulin, Tvrtko
> <tvrtko.ursulin@intel.com>; Kumar, Purushotam
> <purushotam.kumar@intel.com>; Chelladurai, Paul S
> <paul.s.chelladurai@intel.com>
> Subject: RE: ✗ Fi.CI.SPARSE: warning for Dynamic EU configuration of Slice/Sub-
> slice/EU (rev6)
> 
> Hi Srinivas,
> 
> This will break OA counter.
> I am already working with Tvrtko to make patch scalable
> and to make it as per the flow.
> Kindly wait for upcoming patch.
> 
> PS: Coding guideline:
> https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html
> 
> Regards,
> Ankit
> 
> > -----Original Message-----
> > From: Patchwork <patchwork@emeril.freedesktop.org>
> > Sent: Friday, March 13, 2020 7:34 PM
> > To: Navik, Ankit P <ankit.p.navik@intel.com>
> > Cc: intel-gfx@lists.freedesktop.org
> > Subject: ✗ Fi.CI.SPARSE: warning for Dynamic EU configuration of Slice/Sub-
> > slice/EU (rev6)
> >
> > == Series Details ==
> >
> > Series: Dynamic EU configuration of Slice/Sub-slice/EU (rev6)
> > URL   : https://patchwork.freedesktop.org/series/69980/
> > State : warning
> >
> > == Summary ==
> >
> > $ dim sparse origin/drm-tip
> > Sparse version: v0.6.0
> > Commit: drm/i915: Get active pending request for given context
> > +drivers/gpu/drm/i915/gt/intel_lrc.c:2162:61:    expected struct atomic_t
> const
> > [usertype] *v
> > +drivers/gpu/drm/i915/gt/intel_lrc.c:2162:61:    got struct atomic_t [noderef]
> > <asn:4> *
> > +drivers/gpu/drm/i915/gt/intel_lrc.c:2162:61: warning: incorrect type in
> > argument 1 (different address spaces)
> > +drivers/gpu/drm/i915/gt/intel_lrc.c:2163:64:    expected struct atomic_t
> > [usertype] *v
> > +drivers/gpu/drm/i915/gt/intel_lrc.c:2163:64:    got struct atomic_t [noderef]
> > <asn:4> *
> > +drivers/gpu/drm/i915/gt/intel_lrc.c:2163:64: warning: incorrect type in
> > +argument 1 (different address spaces)
> >
> > Commit: drm/i915: set optimum eu/slice/sub-slice configuration based on load
> > type
> > +drivers/gpu/drm/i915/gt/intel_lrc.c:3047:59:    expected struct
> > i915_gem_context *ctx
> > +drivers/gpu/drm/i915/gt/intel_lrc.c:3047:59:    got struct i915_gem_context
> > [noderef] <asn:4> *const gem_context
> > +drivers/gpu/drm/i915/gt/intel_lrc.c:3047:59: warning: incorrect type in
> > +argument 1 (different address spaces)
> > +drivers/gpu/drm/i915/gt/intel_lrc.c:3077:15: warning: dereference of
> > +noderef expression
> > +drivers/gpu/drm/i915/gt/intel_lrc.c:3077:45: warning: dereference of
> > +noderef expression
> > +drivers/gpu/drm/i915/gt/intel_lrc.c:3078:19: warning: dereference of
> > +noderef expression
> > +drivers/gpu/drm/i915/gt/intel_lrc.c:3078:48: warning: dereference of
> > +noderef expression
> >
> > Commit: drm/i915: Predictive governor to control slice/subslice/eu Okay!

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v7 0/3] Dynamic EU configuration of Slice/Sub-slice/EU

[Francisco Jerez]

[-- Attachment #1.1.1: Type: text/plain, Size: 8434 bytes --]

srinivasan.s@intel.com writes:

> From: Srinivasan S <srinivasan.s@intel.com>
>
>       drm/i915: Context aware user agnostic EU/Slice/Sub-slice control within kernel
>
> This patch sets improves GPU power consumption on Linux kernel based OS such as
> Chromium OS, Ubuntu, etc. Following are the power savings.
>
> Power savings on GLK-GT1 Bobba platform running on Chrome OS.
> -----------------------------------------------|
> App /KPI                | % Power Benefit (mW) |
> ------------------------|----------------------|
> Hangout Call- 20 minute |	1.8%           |
> Youtube 4K VPB          |       14.13%         |
> WebGL Aquarium          |       13.76%         |
> Unity3D                 |       6.78%          |
> 			|		       |
> ------------------------|----------------------|
> Chrome PLT              | BatteryLife Improves |
> 			| by ~45 minute        |
> -----------------------------------------------|
>
> Power savings on KBL-GT3 running on  Android and Ubuntu (Linux).
> -----------------------------------------------|
> App /KPI		| % Power Benefit (mW) |
>                         |----------------------|
> 			|  Android |  Ubuntu   |
> ------------------------|----------|-----------|
> 3D Mark (Ice storm)     | 2.30%    | N.A.      |
> TRex On screen          | 2.49%    | 2.97%     |
> Manhattan On screen     | 3.11%    | 4.90%     |
> Carchase On Screen	| N.A.     | 5.06%     |
> AnTuTu 6.1.4            | 3.42%    | N.A.      |
> SynMark2		| N.A.     | 1.7%      |
> -----------------------------------------------|
>

Did you get any performance (e.g. FPS) measurements from those
test-cases?  There is quite some potential for this feature to constrain
the GPU throughput inadvertently, which could lead to an apparent
reduction in power usage not accompanied by an improvement in energy
efficiency -- In fact AFAIUI there is some potential for this feature to
*decrease* the energy efficiency of the system if the GPU would have
been able to keep all EUs busy at a lower frequency, but the parallelism
constraint forces it to run at a higher frequency above RPe in order to
achieve the same throughput, because due to the convexity of the power
curve of the EU we have:

  P(k * f) > k * P(f)

Where 'k' is the ratio between the EU parallelism without and with SSEU
control, and f > RPe is the original GPU frequency without SSEU control.

In scenarios like that we *might* seem to be using less power with SSEU
control if the workload is running longer, but it would end up using
more energy overall by the time it completes, so it would be good to
have some performance-per-watt numbers to make sure that's not
happening.

> We have also observed GPU core residencies improves by 1.035%.
>
> Technical Insights of the patch:
> Current GPU configuration code for i915 does not allow us to change
> EU/Slice/Sub-slice configuration dynamically. Its done only once while context
> is created.
>
> While particular graphics application is running, if we examine the command
> requests from user space, we observe that command density is not consistent.
> It means there is scope to change the graphics configuration dynamically even
> while context is running actively. This patch series proposes the solution to
> find the active pending load for all active context at given time and based on
> that, dynamically perform graphics configuration for each context.
>
> We use a hr (high resolution) timer with i915 driver in kernel to get a
> callback every few milliseconds (this timer value can be configured through
> debugfs, default is '0' indicating timer is in disabled state i.e. original
> system without any intervention).In the timer callback, we examine pending
> commands for a context in the queue, essentially, we intercept them before
> they are executed by GPU and we update context with required number of EUs.
>

Given that the EU configuration update is synchronous with command
submission, do you really need a timer?  It sounds like it would be less
CPU overhead to adjust the EU count on demand whenever the counter
reaches or drops below the threshold instead of polling some CPU-side
data structure.

> Two questions, how did we arrive at right timer value? and what's the right
> number of EUs? For the prior one, empirical data to achieve best performance
> in least power was considered. For the later one, we roughly categorized number 
> of EUs logically based on platform. Now we compare number of pending commands
> with a particular threshold and then set number of EUs accordingly with update
> context. That threshold is also based on experiments & findings. If GPU is able
> to catch up with CPU, typically there are no pending commands, the EU config
> would remain unchanged there. In case there are more pending commands we
> reprogram context with higher number of EUs. Please note, here we are changing
> EUs even while context is running by examining pending commands every 'x'
> milliseconds.
>

I have doubts that the number of requests pending execution is a
particularly reliable indicator of the optimal number of EUs the
workload needs enabled, for starters because the execlists submission
code seems to be able to merge multiple requests into the same port, so
there might seem to be zero pending commands even if the GPU has a
backlog of several seconds or minutes worth of work.

But even if you were using an accurate measure of the GPU load, would
that really be a good indicator of whether the GPU would run more
efficiently with more or less EUs enabled?  I can think of many
scenarios where a short-lived GPU request would consume less energy and
complete faster while running with all EUs enabled (e.g. if it actually
has enough parallelism to take advantage of all EUs in the system).
Conversely I can think of some scenarios where a long-running GPU
request would benefit from SSEU control (e.g. a poorly parallelizable
but heavy 3D geometry pipeline or GPGPU workload).  The former seems
more worrying than the latter since it could lead to performance or
energy efficiency regressions.

IOW it seems to me that the optimal number of EUs enabled is more of a
function of the internal parallelism constraints of each request rather
than of the overall GPU load.  You should be able to get some
understanding of that by e.g. calculating the number of threads loaded
on the average based on the EU SPM counters, but unfortunately the ones
you'd need are only available on TGL+ IIRC.  On earlier platforms you
should be able to achieve the same thing by sampling some FLEXEU
counters, but you'd likely have to mess with the mux configuration which
would interfere with OA sampling -- However it sounds like this feature
may have to be disabled anytime OA is active anyway so that may not be a
problem after all?

Regards,
Francisco.

> Srinivasan S (3):
>   drm/i915: Get active pending request for given context
>   drm/i915: set optimum eu/slice/sub-slice configuration based on load
>     type
>   drm/i915: Predictive governor to control slice/subslice/eu
>
>  drivers/gpu/drm/i915/Makefile                     |   1 +
>  drivers/gpu/drm/i915/gem/i915_gem_context.c       |  20 +++++
>  drivers/gpu/drm/i915/gem/i915_gem_context.h       |   2 +
>  drivers/gpu/drm/i915/gem/i915_gem_context_types.h |  38 ++++++++
>  drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c    |   1 +
>  drivers/gpu/drm/i915/gt/intel_deu.c               | 104 ++++++++++++++++++++++
>  drivers/gpu/drm/i915/gt/intel_deu.h               |  31 +++++++
>  drivers/gpu/drm/i915/gt/intel_lrc.c               |  44 ++++++++-
>  drivers/gpu/drm/i915/i915_drv.h                   |   6 ++
>  drivers/gpu/drm/i915/i915_gem.c                   |   4 +
>  drivers/gpu/drm/i915/i915_params.c                |   4 +
>  drivers/gpu/drm/i915/i915_params.h                |   1 +
>  drivers/gpu/drm/i915/intel_device_info.c          |  74 ++++++++++++++-
>  13 files changed, 325 insertions(+), 5 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/gt/intel_deu.c
>  create mode 100644 drivers/gpu/drm/i915/gt/intel_deu.h
>
> -- 
> 2.7.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v7 0/3] Dynamic EU configuration of Slice/Sub-slice/EU

[Lionel Landwerlin]

[-- Attachment #1.1: Type: text/plain, Size: 8924 bytes --]

On 15/03/2020 02:12, Francisco Jerez wrote:
> srinivasan.s@intel.com writes:
>
>> From: Srinivasan S <srinivasan.s@intel.com>
>>
>>        drm/i915: Context aware user agnostic EU/Slice/Sub-slice control within kernel
>>
>> This patch sets improves GPU power consumption on Linux kernel based OS such as
>> Chromium OS, Ubuntu, etc. Following are the power savings.
>>
>> Power savings on GLK-GT1 Bobba platform running on Chrome OS.
>> -----------------------------------------------|
>> App /KPI                | % Power Benefit (mW) |
>> ------------------------|----------------------|
>> Hangout Call- 20 minute |	1.8%           |
>> Youtube 4K VPB          |       14.13%         |
>> WebGL Aquarium          |       13.76%         |
>> Unity3D                 |       6.78%          |
>> 			|		       |
>> ------------------------|----------------------|
>> Chrome PLT              | BatteryLife Improves |
>> 			| by ~45 minute        |
>> -----------------------------------------------|
>>
>> Power savings on KBL-GT3 running on  Android and Ubuntu (Linux).
>> -----------------------------------------------|
>> App /KPI		| % Power Benefit (mW) |
>>                          |----------------------|
>> 			|  Android |  Ubuntu   |
>> ------------------------|----------|-----------|
>> 3D Mark (Ice storm)     | 2.30%    | N.A.      |
>> TRex On screen          | 2.49%    | 2.97%     |
>> Manhattan On screen     | 3.11%    | 4.90%     |
>> Carchase On Screen	| N.A.     | 5.06%     |
>> AnTuTu 6.1.4            | 3.42%    | N.A.      |
>> SynMark2		| N.A.     | 1.7%      |
>> -----------------------------------------------|
>>
> Did you get any performance (e.g. FPS) measurements from those
> test-cases?  There is quite some potential for this feature to constrain
> the GPU throughput inadvertently, which could lead to an apparent
> reduction in power usage not accompanied by an improvement in energy
> efficiency -- In fact AFAIUI there is some potential for this feature to
> *decrease* the energy efficiency of the system if the GPU would have
> been able to keep all EUs busy at a lower frequency, but the parallelism
> constraint forces it to run at a higher frequency above RPe in order to
> achieve the same throughput, because due to the convexity of the power
> curve of the EU we have:
>
>    P(k * f) > k * P(f)
>
> Where 'k' is the ratio between the EU parallelism without and with SSEU
> control, and f > RPe is the original GPU frequency without SSEU control.
>
> In scenarios like that we *might* seem to be using less power with SSEU
> control if the workload is running longer, but it would end up using
> more energy overall by the time it completes, so it would be good to
> have some performance-per-watt numbers to make sure that's not
> happening.
>
>> We have also observed GPU core residencies improves by 1.035%.
>>
>> Technical Insights of the patch:
>> Current GPU configuration code for i915 does not allow us to change
>> EU/Slice/Sub-slice configuration dynamically. Its done only once while context
>> is created.
>>
>> While particular graphics application is running, if we examine the command
>> requests from user space, we observe that command density is not consistent.
>> It means there is scope to change the graphics configuration dynamically even
>> while context is running actively. This patch series proposes the solution to
>> find the active pending load for all active context at given time and based on
>> that, dynamically perform graphics configuration for each context.
>>
>> We use a hr (high resolution) timer with i915 driver in kernel to get a
>> callback every few milliseconds (this timer value can be configured through
>> debugfs, default is '0' indicating timer is in disabled state i.e. original
>> system without any intervention).In the timer callback, we examine pending
>> commands for a context in the queue, essentially, we intercept them before
>> they are executed by GPU and we update context with required number of EUs.
>>
> Given that the EU configuration update is synchronous with command
> submission, do you really need a timer?  It sounds like it would be less
> CPU overhead to adjust the EU count on demand whenever the counter
> reaches or drops below the threshold instead of polling some CPU-side
> data structure.
>
>> Two questions, how did we arrive at right timer value? and what's the right
>> number of EUs? For the prior one, empirical data to achieve best performance
>> in least power was considered. For the later one, we roughly categorized number
>> of EUs logically based on platform. Now we compare number of pending commands
>> with a particular threshold and then set number of EUs accordingly with update
>> context. That threshold is also based on experiments & findings. If GPU is able
>> to catch up with CPU, typically there are no pending commands, the EU config
>> would remain unchanged there. In case there are more pending commands we
>> reprogram context with higher number of EUs. Please note, here we are changing
>> EUs even while context is running by examining pending commands every 'x'
>> milliseconds.
>>
> I have doubts that the number of requests pending execution is a
> particularly reliable indicator of the optimal number of EUs the
> workload needs enabled, for starters because the execlists submission
> code seems to be able to merge multiple requests into the same port, so
> there might seem to be zero pending commands even if the GPU has a
> backlog of several seconds or minutes worth of work.
>
> But even if you were using an accurate measure of the GPU load, would
> that really be a good indicator of whether the GPU would run more
> efficiently with more or less EUs enabled?  I can think of many
> scenarios where a short-lived GPU request would consume less energy and
> complete faster while running with all EUs enabled (e.g. if it actually
> has enough parallelism to take advantage of all EUs in the system).
> Conversely I can think of some scenarios where a long-running GPU
> request would benefit from SSEU control (e.g. a poorly parallelizable
> but heavy 3D geometry pipeline or GPGPU workload).  The former seems
> more worrying than the latter since it could lead to performance or
> energy efficiency regressions.
>
> IOW it seems to me that the optimal number of EUs enabled is more of a
> function of the internal parallelism constraints of each request rather
> than of the overall GPU load.  You should be able to get some
> understanding of that by e.g. calculating the number of threads loaded
> on the average based on the EU SPM counters, but unfortunately the ones
> you'd need are only available on TGL+ IIRC.  On earlier platforms you
> should be able to achieve the same thing by sampling some FLEXEU
> counters, but you'd likely have to mess with the mux configuration which
> would interfere with OA sampling -- However it sounds like this feature
> may have to be disabled anytime OA is active anyway so that may not be a
> problem after all?


FLEXEU has to be configured on all contexts but does not need the mux 
configuration.

I think this feature would have to be shut off everytime you end using 
OA from userspace though.


-Lionel


>
> Regards,
> Francisco.
>
>> Srinivasan S (3):
>>    drm/i915: Get active pending request for given context
>>    drm/i915: set optimum eu/slice/sub-slice configuration based on load
>>      type
>>    drm/i915: Predictive governor to control slice/subslice/eu
>>
>>   drivers/gpu/drm/i915/Makefile                     |   1 +
>>   drivers/gpu/drm/i915/gem/i915_gem_context.c       |  20 +++++
>>   drivers/gpu/drm/i915/gem/i915_gem_context.h       |   2 +
>>   drivers/gpu/drm/i915/gem/i915_gem_context_types.h |  38 ++++++++
>>   drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c    |   1 +
>>   drivers/gpu/drm/i915/gt/intel_deu.c               | 104 ++++++++++++++++++++++
>>   drivers/gpu/drm/i915/gt/intel_deu.h               |  31 +++++++
>>   drivers/gpu/drm/i915/gt/intel_lrc.c               |  44 ++++++++-
>>   drivers/gpu/drm/i915/i915_drv.h                   |   6 ++
>>   drivers/gpu/drm/i915/i915_gem.c                   |   4 +
>>   drivers/gpu/drm/i915/i915_params.c                |   4 +
>>   drivers/gpu/drm/i915/i915_params.h                |   1 +
>>   drivers/gpu/drm/i915/intel_device_info.c          |  74 ++++++++++++++-
>>   13 files changed, 325 insertions(+), 5 deletions(-)
>>   create mode 100644 drivers/gpu/drm/i915/gt/intel_deu.c
>>   create mode 100644 drivers/gpu/drm/i915/gt/intel_deu.h
>>
>> -- 
>> 2.7.4
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



[-- Attachment #1.2: Type: text/html, Size: 10285 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v7 0/3] Dynamic EU configuration of Slice/Sub-slice/EU

[Francisco Jerez]

[-- Attachment #1.1.1: Type: text/plain, Size: 9642 bytes --]

Lionel Landwerlin <lionel.g.landwerlin@intel.com> writes:

> On 15/03/2020 02:12, Francisco Jerez wrote:
>> srinivasan.s@intel.com writes:
>>
>>> From: Srinivasan S <srinivasan.s@intel.com>
>>>
>>>        drm/i915: Context aware user agnostic EU/Slice/Sub-slice control within kernel
>>>
>>> This patch sets improves GPU power consumption on Linux kernel based OS such as
>>> Chromium OS, Ubuntu, etc. Following are the power savings.
>>>
>>> Power savings on GLK-GT1 Bobba platform running on Chrome OS.
>>> -----------------------------------------------|
>>> App /KPI                | % Power Benefit (mW) |
>>> ------------------------|----------------------|
>>> Hangout Call- 20 minute |	1.8%           |
>>> Youtube 4K VPB          |       14.13%         |
>>> WebGL Aquarium          |       13.76%         |
>>> Unity3D                 |       6.78%          |
>>> 			|		       |
>>> ------------------------|----------------------|
>>> Chrome PLT              | BatteryLife Improves |
>>> 			| by ~45 minute        |
>>> -----------------------------------------------|
>>>
>>> Power savings on KBL-GT3 running on  Android and Ubuntu (Linux).
>>> -----------------------------------------------|
>>> App /KPI		| % Power Benefit (mW) |
>>>                          |----------------------|
>>> 			|  Android |  Ubuntu   |
>>> ------------------------|----------|-----------|
>>> 3D Mark (Ice storm)     | 2.30%    | N.A.      |
>>> TRex On screen          | 2.49%    | 2.97%     |
>>> Manhattan On screen     | 3.11%    | 4.90%     |
>>> Carchase On Screen	| N.A.     | 5.06%     |
>>> AnTuTu 6.1.4            | 3.42%    | N.A.      |
>>> SynMark2		| N.A.     | 1.7%      |
>>> -----------------------------------------------|
>>>
>> Did you get any performance (e.g. FPS) measurements from those
>> test-cases?  There is quite some potential for this feature to constrain
>> the GPU throughput inadvertently, which could lead to an apparent
>> reduction in power usage not accompanied by an improvement in energy
>> efficiency -- In fact AFAIUI there is some potential for this feature to
>> *decrease* the energy efficiency of the system if the GPU would have
>> been able to keep all EUs busy at a lower frequency, but the parallelism
>> constraint forces it to run at a higher frequency above RPe in order to
>> achieve the same throughput, because due to the convexity of the power
>> curve of the EU we have:
>>
>>    P(k * f) > k * P(f)
>>
>> Where 'k' is the ratio between the EU parallelism without and with SSEU
>> control, and f > RPe is the original GPU frequency without SSEU control.
>>
>> In scenarios like that we *might* seem to be using less power with SSEU
>> control if the workload is running longer, but it would end up using
>> more energy overall by the time it completes, so it would be good to
>> have some performance-per-watt numbers to make sure that's not
>> happening.
>>
>>> We have also observed GPU core residencies improves by 1.035%.
>>>
>>> Technical Insights of the patch:
>>> Current GPU configuration code for i915 does not allow us to change
>>> EU/Slice/Sub-slice configuration dynamically. Its done only once while context
>>> is created.
>>>
>>> While particular graphics application is running, if we examine the command
>>> requests from user space, we observe that command density is not consistent.
>>> It means there is scope to change the graphics configuration dynamically even
>>> while context is running actively. This patch series proposes the solution to
>>> find the active pending load for all active context at given time and based on
>>> that, dynamically perform graphics configuration for each context.
>>>
>>> We use a hr (high resolution) timer with i915 driver in kernel to get a
>>> callback every few milliseconds (this timer value can be configured through
>>> debugfs, default is '0' indicating timer is in disabled state i.e. original
>>> system without any intervention).In the timer callback, we examine pending
>>> commands for a context in the queue, essentially, we intercept them before
>>> they are executed by GPU and we update context with required number of EUs.
>>>
>> Given that the EU configuration update is synchronous with command
>> submission, do you really need a timer?  It sounds like it would be less
>> CPU overhead to adjust the EU count on demand whenever the counter
>> reaches or drops below the threshold instead of polling some CPU-side
>> data structure.
>>
>>> Two questions, how did we arrive at right timer value? and what's the right
>>> number of EUs? For the prior one, empirical data to achieve best performance
>>> in least power was considered. For the later one, we roughly categorized number
>>> of EUs logically based on platform. Now we compare number of pending commands
>>> with a particular threshold and then set number of EUs accordingly with update
>>> context. That threshold is also based on experiments & findings. If GPU is able
>>> to catch up with CPU, typically there are no pending commands, the EU config
>>> would remain unchanged there. In case there are more pending commands we
>>> reprogram context with higher number of EUs. Please note, here we are changing
>>> EUs even while context is running by examining pending commands every 'x'
>>> milliseconds.
>>>
>> I have doubts that the number of requests pending execution is a
>> particularly reliable indicator of the optimal number of EUs the
>> workload needs enabled, for starters because the execlists submission
>> code seems to be able to merge multiple requests into the same port, so
>> there might seem to be zero pending commands even if the GPU has a
>> backlog of several seconds or minutes worth of work.
>>
>> But even if you were using an accurate measure of the GPU load, would
>> that really be a good indicator of whether the GPU would run more
>> efficiently with more or less EUs enabled?  I can think of many
>> scenarios where a short-lived GPU request would consume less energy and
>> complete faster while running with all EUs enabled (e.g. if it actually
>> has enough parallelism to take advantage of all EUs in the system).
>> Conversely I can think of some scenarios where a long-running GPU
>> request would benefit from SSEU control (e.g. a poorly parallelizable
>> but heavy 3D geometry pipeline or GPGPU workload).  The former seems
>> more worrying than the latter since it could lead to performance or
>> energy efficiency regressions.
>>
>> IOW it seems to me that the optimal number of EUs enabled is more of a
>> function of the internal parallelism constraints of each request rather
>> than of the overall GPU load.  You should be able to get some
>> understanding of that by e.g. calculating the number of threads loaded
>> on the average based on the EU SPM counters, but unfortunately the ones
>> you'd need are only available on TGL+ IIRC.  On earlier platforms you
>> should be able to achieve the same thing by sampling some FLEXEU
>> counters, but you'd likely have to mess with the mux configuration which
>> would interfere with OA sampling -- However it sounds like this feature
>> may have to be disabled anytime OA is active anyway so that may not be a
>> problem after all?
>
>
> FLEXEU has to be configured on all contexts but does not need the mux 
> configuration.
>

They have a sort of mux controlled through the EU_PERF_CNT_CTL*
registers that have to be set up correctly for each counter to count the
right event, which would certainly interfere with userspace using OA to
gather EU metrics.

> I think this feature would have to be shut off everytime you end using 
> OA from userspace though.
>

Yeah, that's probably necessary one way or another.

>
> -Lionel
>
>
>>
>> Regards,
>> Francisco.
>>
>>> Srinivasan S (3):
>>>    drm/i915: Get active pending request for given context
>>>    drm/i915: set optimum eu/slice/sub-slice configuration based on load
>>>      type
>>>    drm/i915: Predictive governor to control slice/subslice/eu
>>>
>>>   drivers/gpu/drm/i915/Makefile                     |   1 +
>>>   drivers/gpu/drm/i915/gem/i915_gem_context.c       |  20 +++++
>>>   drivers/gpu/drm/i915/gem/i915_gem_context.h       |   2 +
>>>   drivers/gpu/drm/i915/gem/i915_gem_context_types.h |  38 ++++++++
>>>   drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c    |   1 +
>>>   drivers/gpu/drm/i915/gt/intel_deu.c               | 104 ++++++++++++++++++++++
>>>   drivers/gpu/drm/i915/gt/intel_deu.h               |  31 +++++++
>>>   drivers/gpu/drm/i915/gt/intel_lrc.c               |  44 ++++++++-
>>>   drivers/gpu/drm/i915/i915_drv.h                   |   6 ++
>>>   drivers/gpu/drm/i915/i915_gem.c                   |   4 +
>>>   drivers/gpu/drm/i915/i915_params.c                |   4 +
>>>   drivers/gpu/drm/i915/i915_params.h                |   1 +
>>>   drivers/gpu/drm/i915/intel_device_info.c          |  74 ++++++++++++++-
>>>   13 files changed, 325 insertions(+), 5 deletions(-)
>>>   create mode 100644 drivers/gpu/drm/i915/gt/intel_deu.c
>>>   create mode 100644 drivers/gpu/drm/i915/gt/intel_deu.h
>>>
>>> -- 
>>> 2.7.4
>>>
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>>
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v7 0/3] Dynamic EU configuration of Slice/Sub-slice/EU

[Lionel Landwerlin]

On 15/03/2020 20:08, Francisco Jerez wrote:
> Lionel Landwerlin <lionel.g.landwerlin@intel.com> writes:
>
>> On 15/03/2020 02:12, Francisco Jerez wrote:
>>> srinivasan.s@intel.com writes:
>>>
>>>> From: Srinivasan S <srinivasan.s@intel.com>
>>>>
>>>>         drm/i915: Context aware user agnostic EU/Slice/Sub-slice control within kernel
>>>>
>>>> This patch sets improves GPU power consumption on Linux kernel based OS such as
>>>> Chromium OS, Ubuntu, etc. Following are the power savings.
>>>>
>>>> Power savings on GLK-GT1 Bobba platform running on Chrome OS.
>>>> -----------------------------------------------|
>>>> App /KPI                | % Power Benefit (mW) |
>>>> ------------------------|----------------------|
>>>> Hangout Call- 20 minute |	1.8%           |
>>>> Youtube 4K VPB          |       14.13%         |
>>>> WebGL Aquarium          |       13.76%         |
>>>> Unity3D                 |       6.78%          |
>>>> 			|		       |
>>>> ------------------------|----------------------|
>>>> Chrome PLT              | BatteryLife Improves |
>>>> 			| by ~45 minute        |
>>>> -----------------------------------------------|
>>>>
>>>> Power savings on KBL-GT3 running on  Android and Ubuntu (Linux).
>>>> -----------------------------------------------|
>>>> App /KPI		| % Power Benefit (mW) |
>>>>                           |----------------------|
>>>> 			|  Android |  Ubuntu   |
>>>> ------------------------|----------|-----------|
>>>> 3D Mark (Ice storm)     | 2.30%    | N.A.      |
>>>> TRex On screen          | 2.49%    | 2.97%     |
>>>> Manhattan On screen     | 3.11%    | 4.90%     |
>>>> Carchase On Screen	| N.A.     | 5.06%     |
>>>> AnTuTu 6.1.4            | 3.42%    | N.A.      |
>>>> SynMark2		| N.A.     | 1.7%      |
>>>> -----------------------------------------------|
>>>>
>>> Did you get any performance (e.g. FPS) measurements from those
>>> test-cases?  There is quite some potential for this feature to constrain
>>> the GPU throughput inadvertently, which could lead to an apparent
>>> reduction in power usage not accompanied by an improvement in energy
>>> efficiency -- In fact AFAIUI there is some potential for this feature to
>>> *decrease* the energy efficiency of the system if the GPU would have
>>> been able to keep all EUs busy at a lower frequency, but the parallelism
>>> constraint forces it to run at a higher frequency above RPe in order to
>>> achieve the same throughput, because due to the convexity of the power
>>> curve of the EU we have:
>>>
>>>     P(k * f) > k * P(f)
>>>
>>> Where 'k' is the ratio between the EU parallelism without and with SSEU
>>> control, and f > RPe is the original GPU frequency without SSEU control.
>>>
>>> In scenarios like that we *might* seem to be using less power with SSEU
>>> control if the workload is running longer, but it would end up using
>>> more energy overall by the time it completes, so it would be good to
>>> have some performance-per-watt numbers to make sure that's not
>>> happening.
>>>
>>>> We have also observed GPU core residencies improves by 1.035%.
>>>>
>>>> Technical Insights of the patch:
>>>> Current GPU configuration code for i915 does not allow us to change
>>>> EU/Slice/Sub-slice configuration dynamically. Its done only once while context
>>>> is created.
>>>>
>>>> While particular graphics application is running, if we examine the command
>>>> requests from user space, we observe that command density is not consistent.
>>>> It means there is scope to change the graphics configuration dynamically even
>>>> while context is running actively. This patch series proposes the solution to
>>>> find the active pending load for all active context at given time and based on
>>>> that, dynamically perform graphics configuration for each context.
>>>>
>>>> We use a hr (high resolution) timer with i915 driver in kernel to get a
>>>> callback every few milliseconds (this timer value can be configured through
>>>> debugfs, default is '0' indicating timer is in disabled state i.e. original
>>>> system without any intervention).In the timer callback, we examine pending
>>>> commands for a context in the queue, essentially, we intercept them before
>>>> they are executed by GPU and we update context with required number of EUs.
>>>>
>>> Given that the EU configuration update is synchronous with command
>>> submission, do you really need a timer?  It sounds like it would be less
>>> CPU overhead to adjust the EU count on demand whenever the counter
>>> reaches or drops below the threshold instead of polling some CPU-side
>>> data structure.
>>>
>>>> Two questions, how did we arrive at right timer value? and what's the right
>>>> number of EUs? For the prior one, empirical data to achieve best performance
>>>> in least power was considered. For the later one, we roughly categorized number
>>>> of EUs logically based on platform. Now we compare number of pending commands
>>>> with a particular threshold and then set number of EUs accordingly with update
>>>> context. That threshold is also based on experiments & findings. If GPU is able
>>>> to catch up with CPU, typically there are no pending commands, the EU config
>>>> would remain unchanged there. In case there are more pending commands we
>>>> reprogram context with higher number of EUs. Please note, here we are changing
>>>> EUs even while context is running by examining pending commands every 'x'
>>>> milliseconds.
>>>>
>>> I have doubts that the number of requests pending execution is a
>>> particularly reliable indicator of the optimal number of EUs the
>>> workload needs enabled, for starters because the execlists submission
>>> code seems to be able to merge multiple requests into the same port, so
>>> there might seem to be zero pending commands even if the GPU has a
>>> backlog of several seconds or minutes worth of work.
>>>
>>> But even if you were using an accurate measure of the GPU load, would
>>> that really be a good indicator of whether the GPU would run more
>>> efficiently with more or less EUs enabled?  I can think of many
>>> scenarios where a short-lived GPU request would consume less energy and
>>> complete faster while running with all EUs enabled (e.g. if it actually
>>> has enough parallelism to take advantage of all EUs in the system).
>>> Conversely I can think of some scenarios where a long-running GPU
>>> request would benefit from SSEU control (e.g. a poorly parallelizable
>>> but heavy 3D geometry pipeline or GPGPU workload).  The former seems
>>> more worrying than the latter since it could lead to performance or
>>> energy efficiency regressions.
>>>
>>> IOW it seems to me that the optimal number of EUs enabled is more of a
>>> function of the internal parallelism constraints of each request rather
>>> than of the overall GPU load.  You should be able to get some
>>> understanding of that by e.g. calculating the number of threads loaded
>>> on the average based on the EU SPM counters, but unfortunately the ones
>>> you'd need are only available on TGL+ IIRC.  On earlier platforms you
>>> should be able to achieve the same thing by sampling some FLEXEU
>>> counters, but you'd likely have to mess with the mux configuration which
>>> would interfere with OA sampling -- However it sounds like this feature
>>> may have to be disabled anytime OA is active anyway so that may not be a
>>> problem after all?
>>
>> FLEXEU has to be configured on all contexts but does not need the mux
>> configuration.
>>
> They have a sort of mux controlled through the EU_PERF_CNT_CTL*
> registers that have to be set up correctly for each counter to count the
> right event, which would certainly interfere with userspace using OA to
> gather EU metrics.


Maybe we're not talking about the same mux then :)


>
>> I think this feature would have to be shut off everytime you end using
>> OA from userspace though.
>>
> Yeah, that's probably necessary one way or another.
>
>> -Lionel
>>
>>
>>> Regards,
>>> Francisco.
>>>
>>>> Srinivasan S (3):
>>>>     drm/i915: Get active pending request for given context
>>>>     drm/i915: set optimum eu/slice/sub-slice configuration based on load
>>>>       type
>>>>     drm/i915: Predictive governor to control slice/subslice/eu
>>>>
>>>>    drivers/gpu/drm/i915/Makefile                     |   1 +
>>>>    drivers/gpu/drm/i915/gem/i915_gem_context.c       |  20 +++++
>>>>    drivers/gpu/drm/i915/gem/i915_gem_context.h       |   2 +
>>>>    drivers/gpu/drm/i915/gem/i915_gem_context_types.h |  38 ++++++++
>>>>    drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c    |   1 +
>>>>    drivers/gpu/drm/i915/gt/intel_deu.c               | 104 ++++++++++++++++++++++
>>>>    drivers/gpu/drm/i915/gt/intel_deu.h               |  31 +++++++
>>>>    drivers/gpu/drm/i915/gt/intel_lrc.c               |  44 ++++++++-
>>>>    drivers/gpu/drm/i915/i915_drv.h                   |   6 ++
>>>>    drivers/gpu/drm/i915/i915_gem.c                   |   4 +
>>>>    drivers/gpu/drm/i915/i915_params.c                |   4 +
>>>>    drivers/gpu/drm/i915/i915_params.h                |   1 +
>>>>    drivers/gpu/drm/i915/intel_device_info.c          |  74 ++++++++++++++-
>>>>    13 files changed, 325 insertions(+), 5 deletions(-)
>>>>    create mode 100644 drivers/gpu/drm/i915/gt/intel_deu.c
>>>>    create mode 100644 drivers/gpu/drm/i915/gt/intel_deu.h
>>>>
>>>> -- 
>>>> 2.7.4
>>>>
>>>> _______________________________________________
>>>> Intel-gfx mailing list
>>>> Intel-gfx@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>>>
>>>> _______________________________________________
>>>> Intel-gfx mailing list
>>>> Intel-gfx@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx