[Intel-gfx] [PATCH v4 3/3] drm/i915: Predictive governor to control eu/slice/subslice
Tvrtko Ursulin
tvrtko.ursulin at linux.intel.com
Thu Mar 14 10:58:20 UTC 2019
On 14/03/2019 08:36, Ankit Navik wrote:
> From: Praveen Diwakar <praveen.diwakar at intel.com>
>
> 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)
>
> 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.
>
> Cc: Aravindan Muthukumar <aravindan.muthukumar at intel.com>
> Cc: Yogesh Marathe <yogesh.marathe at intel.com>
> Signed-off-by: Praveen Diwakar <praveen.diwakar at intel.com>
> Signed-off-by: Kedar J Karanje <kedar.j.karanje at intel.com>
> Signed-off-by: Ankit Navik <ankit.p.navik at intel.com>
> ---
> drivers/gpu/drm/i915/Makefile | 1 +
> drivers/gpu/drm/i915/i915_debugfs.c | 1 +
> drivers/gpu/drm/i915/i915_drv.h | 4 ++
> drivers/gpu/drm/i915/i915_gem.c | 3 +
> drivers/gpu/drm/i915/i915_params.c | 4 ++
> drivers/gpu/drm/i915/i915_params.h | 1 +
> drivers/gpu/drm/i915/intel_deu.c | 109 ++++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_drv.h | 3 +
> 8 files changed, 126 insertions(+)
> create mode 100644 drivers/gpu/drm/i915/intel_deu.c
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 1787e12..b9eaaed 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -119,6 +119,7 @@ i915-y += intel_audio.o \
> intel_color.o \
> intel_combo_phy.o \
> intel_connector.o \
> + intel_deu.o \
> intel_display.o \
> intel_dpio_phy.o \
> intel_dpll_mgr.o \
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index ca8fa44..1b8f631 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -4612,6 +4612,7 @@ 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 I915_DEBUGFS_ENTRIES ARRAY_SIZE(i915_debugfs_list)
>
> static const struct i915_debugfs_files {
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 97cb36b..dfe80d9 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1596,6 +1596,9 @@ struct drm_i915_private {
> /* optimal slice/subslice/EU configration state */
> struct i915_sseu_optimum_config *opt_config;
>
> + /* protects predictive load state */
> + struct mutex pred_mutex;
> + struct hrtimer pred_timer;
> int predictive_load_enable;
>
> unsigned int fsb_freq, mem_freq, is_ddr3;
> @@ -2646,6 +2649,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);
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 5c1b9d4..18c5195 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -5267,6 +5267,9 @@ int i915_gem_init_early(struct drm_i915_private *dev_priv)
>
> spin_lock_init(&dev_priv->fb_tracking.lock);
>
> + /* Dynamic EU timer initialization for predictive load */
> + intel_deu_init(dev_priv);
> +
> err = i915_gemfs_init(dev_priv);
> if (err)
> DRM_NOTE("Unable to create a private tmpfs mount, hugepage support will be disabled(%d).\n", err);
> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> index b5be0ab..7857e92 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -97,6 +97,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_timer_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 3f14e98..c51f56b 100644
> --- a/drivers/gpu/drm/i915/i915_params.h
> +++ b/drivers/gpu/drm/i915/i915_params.h
> @@ -54,6 +54,7 @@ struct drm_printer;
> param(int, disable_power_well, -1) \
> param(int, enable_ips, 1) \
> param(int, invert_brightness, 0) \
> + param(int, deu_timer_enable, 0) \
Maybe just deu_enable, although I am not sure acronym is good here.
i915.predictive_sseu = 1/0?
> param(int, enable_guc, 0) \
> param(int, guc_log_level, -1) \
> param(char *, guc_firmware_path, NULL) \
> diff --git a/drivers/gpu/drm/i915/intel_deu.c b/drivers/gpu/drm/i915/intel_deu.c
> new file mode 100644
> index 0000000..34654d5
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_deu.c
> @@ -0,0 +1,109 @@
> +/*
> + * 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 at intel.com>
> + */
> +
> +/**
> + * DOC: Dynamic EU Control (DEUC)
I am not sure about the acronym but ignoring that it is not consistently
used between the file name, module parameter name, function prefix name
and here. Well all is consistent apart from this.
> + *
> + * 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_drv.h"
> +#include "i915_drv.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)
static
> +{
> + 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->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;
> +}
Thinking out loud - I wonder if you could drive the logic from context
activity instead of the timer. It would avoid having to walk all the
idle contexts, but would require adding time checks to rq timeline
management.
> +
> +/**
> + * intel_deu_init - Initialize dynamic EU
> + * @dev_priv: i915 device instance
> + *
> + * This fuction is called at driver load
> + */
> +void intel_deu_init(struct drm_i915_private *dev_priv)
> +{
> + dev_priv->predictive_load_enable = i915_modparams.deu_timer_enable;
> + 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);
> +
> + mutex_lock(&dev_priv->pred_mutex);
> +
> + 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);
I'd move stopping and starting of the timer into GT idle/active hooks.
No point in keeping it running while the GPU is asleep.
Haven't I suggested this before? Look for i915_pmu_gt_(un)parked and put
your hooks next to them.
> + }
> +
> + mutex_unlock(&dev_priv->pred_mutex);
Mutex doesn't seem to be useful now that you made this modparam.
Regards,
Tvrtko
> +}
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index b4e29b8..8e6c1da 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1990,6 +1990,9 @@ void intel_fbc_cleanup_cfb(struct drm_i915_private *dev_priv);
> void intel_fbc_handle_fifo_underrun_irq(struct drm_i915_private *dev_priv);
> int intel_fbc_reset_underrun(struct drm_i915_private *dev_priv);
>
> +/* intel_deu.c */
> +void intel_deu_init(struct drm_i915_private *dev_priv);
> +
> /* intel_hdmi.c */
> void intel_hdmi_init(struct drm_i915_private *dev_priv, i915_reg_t hdmi_reg,
> enum port port);
>
More information about the Intel-gfx
mailing list