On 11/12/2018 10:14, Ankit Navik wrote:
From: Praveen Diwakar <praveen.diwakar@xxxxxxxxx>
High resolution timer is used for predictive governor to control
eu/slice/subslice based on workloads.
Debugfs 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)
Cc: Aravindan Muthukumar <aravindan.muthukumar@xxxxxxxxx>
Cc: Yogesh Marathe <yogesh.marathe@xxxxxxxxx>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx>
For the record - no, I did not r-b this. Please remove all false r-b tags.
Signed-off-by: Praveen Diwakar <praveen.diwakar@xxxxxxxxx>
Signed-off-by: Kedar J Karanje <kedar.j.karanje@xxxxxxxxx>
Signed-off-by: Ankit Navik <ankit.p.navik@xxxxxxxxx>
---
drivers/gpu/drm/i915/i915_debugfs.c | 90 ++++++++++++++++++++++++++++++++++++-
drivers/gpu/drm/i915/i915_drv.c | 4 ++
drivers/gpu/drm/i915/i915_drv.h | 6 +++
3 files changed, 99 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index f9ce35d..861f3c1 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -4740,6 +4740,92 @@ static const struct drm_info_list i915_debugfs_list[] = {
{"i915_drrs_status", i915_drrs_status, 0},
{"i915_rps_boost_info", i915_rps_boost_info, 0},
};
+
+#define PENDING_REQ_0 0 /* No active request pending */
Still seems silly - what is the benefit of adding a define with zero in
it's name, which evaluates to zero? Just remove it.
+
+/*
+ * 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 long req_pending;
unsigned int is enough to hold atomic_t.
+
+ list_for_each_entry(ctx, &dev_priv->contexts.list, link) {
+
+ /* Check for in-valid context */
+ if (!ctx->name)
+ continue;
What kind of contexts are these invalid ones? :)
+
+ 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 == PENDING_REQ_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;
+}
+
+static int i915_predictive_load_get(void *data, u64 *val)
+{
+ struct drm_i915_private *dev_priv = data;
+
+ *val = dev_priv->predictive_load_enable;
+ return 0;
+}
+
+static int i915_predictive_load_set(void *data, u64 val)
+{
+ struct drm_i915_private *dev_priv = data;
+
+ mutex_lock(&dev_priv->pred_mutex);
+
+ dev_priv->predictive_load_enable = val;
+
+ if (dev_priv->predictive_load_enable) {
The period change applies after the current timer expires, okay.
+ 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);
+ }
+
+ mutex_unlock(&dev_priv->pred_mutex);
+ return 0;
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(i915_predictive_load_ctl,
+ i915_predictive_load_get, i915_predictive_load_set,
+ "%llu\n");
+
#define I915_DEBUGFS_ENTRIES ARRAY_SIZE(i915_debugfs_list)
static const struct i915_debugfs_files {
@@ -4769,7 +4855,9 @@ static const struct i915_debugfs_files {
{"i915_hpd_storm_ctl", &i915_hpd_storm_ctl_fops},
{"i915_ipc_status", &i915_ipc_status_fops},
{"i915_drrs_ctl", &i915_drrs_ctl_fops},
- {"i915_edp_psr_debug", &i915_edp_psr_debug_fops}
+ {"i915_edp_psr_debug", &i915_edp_psr_debug_fops},
+ /* FIXME: When feature will become real, move to sysfs */
+ {"i915_predictive_load_ctl", &i915_predictive_load_ctl}
};
int i915_debugfs_register(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index f8cfd16..79f4df5 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1397,6 +1397,10 @@ int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent)
if (ret < 0)
goto out_cleanup_hw;
+ /* Timer initialization for predictive load */
+ hrtimer_init(&dev_priv->pred_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+ dev_priv->pred_timer.function = predictive_load_cb;
mutex_init(&dev_priv->pred_mutex) is missing.
And please move this whole block to i915_gem_init_early.
+
i915_driver_register(dev_priv);
intel_runtime_pm_enable(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4b9a8c5..a78fdbc 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1684,6 +1684,11 @@ struct drm_i915_private {
/* optimal slice/subslice/EU configration state */
struct i915_sseu_optimum_config opt_config[LOAD_TYPE_LAST];
+ /* protects predictive load state */
+ struct mutex pred_mutex;
+ struct hrtimer pred_timer;
+ int predictive_load_enable;
+
unsigned int fsb_freq, mem_freq, is_ddr3;
unsigned int skl_preferred_vco_freq;
unsigned int max_cdclk_freq;
@@ -2730,6 +2735,7 @@ extern unsigned long i915_chipset_val(struct drm_i915_private *dev_priv);
extern unsigned long i915_mch_val(struct drm_i915_private *dev_priv);
extern unsigned long i915_gfx_val(struct drm_i915_private *dev_priv);
extern void i915_update_gfx_val(struct drm_i915_private *dev_priv);
+extern enum hrtimer_restart predictive_load_cb(struct hrtimer *hrtimer);
int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool on);
int intel_engines_init_mmio(struct drm_i915_private *dev_priv);
Looking smaller and better, right?
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
