[Intel-gfx] [PATCH 16/31] drm/i915/slpc: Lay out SLPC init/enable/disable/cleanup helpers
Michal Wajdeczko
michal.wajdeczko at intel.com
Thu Sep 21 13:00:18 UTC 2017
On Tue, 19 Sep 2017 19:41:52 +0200, Sagar Arun Kamble
<sagar.a.kamble at intel.com> wrote:
> SLPC operates based on parameters setup in shared data between
> i915 and GuC SLPC. This is to be created/initialized in intel_slpc_init.
> From there onwards i915 can control the SLPC operations by Enabling,
> Disabling complete SLPC or changing SLPC parameters. During cleanup
> SLPC shared data has to be freed.
> With this patch on platforms with SLPC support we call intel_slpc_*()
> functions from GuC setup functions and do not use Host rps functions.
> With SLPC, intel_enable_gt_powersave will only handle RC6. In the later
> patch intel_init_gt_powersave will check if SLPC has started running
> through shared data and update initial state that i915 needs like
> frequency limits if needed.
>
> v1: Return void instead of ignored error code (Paulo)
> enable/disable RC6 in SLPC flows (Sagar)
> replace HAS_SLPC() use with intel_slpc_enabled()
> or intel_slpc_active() (Paulo)
> Fix for renaming gen9_disable_rps to gen9_disable_rc6 in
> "drm/i915/bxt: Explicitly clear the Turbo control register"
> Defer RC6 and SLPC enabling to intel_gen6_powersave_work. (Sagar)
> Performance drop with SLPC was happening as ring frequency table
> was not programmed when SLPC was enabled. This patch programs ring
> frequency table with SLPC. Initial reset of SLPC is based on kernel
> parameter as planning to add slpc state in intel_slpc_active. Cleanup
> is also based on kernel parameter as SLPC gets disabled in
> disable/suspend.(Sagar)
>
> v2: Usage of INTEL_GEN instead of INTEL_INFO->gen (David)
> Checkpatch update.
>
> v3: Rebase
>
> v4: Removed reset functions to comply with *_gt_powersave routines.
> (Sagar)
>
> v5: Removed intel_slpc_active. Relying on slpc.active for control flows
> that are based on SLPC active status in GuC. State setup/cleanup
> needed
> for SLPC is handled using kernel parameter i915.enable_slpc. Moved
> SLPC
> init and enabling to GuC enable path as SLPC in GuC can start doing
> the
> setup post GuC init. Commit message update. (Sagar)
>
> v6: Rearranged function definitions.
>
> v7: Makefile rearrangement. Reducing usage of i915.enable_slpc and
> relying
> mostly on rps.rps_enabled to bypass Host RPS flows. Commit message
> update.
>
> v8: Changed parameters for SLPC functions to struct intel_slpc*.
>
> v9: Reinstated intel_slpc_active and intel_slpc_enabled as they are more
> meaningful.
>
> v10: Rebase changes due to creation of intel_guc.h. Updates in
> intel_guc_cleanup w.r.t slpc cleanup.
>
> Signed-off-by: Tom O'Rourke <Tom.O'Rourke at intel.com>
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble at intel.com>
> ---
> drivers/gpu/drm/i915/Makefile | 1 +
> drivers/gpu/drm/i915/i915_drv.h | 12 ++++++++++
> drivers/gpu/drm/i915/intel_guc.c | 3 +++
> drivers/gpu/drm/i915/intel_guc.h | 3 +++
Hmm, this looks like cross dependency on other pending series
> drivers/gpu/drm/i915/intel_pm.c | 19 +++++++++++-----
> drivers/gpu/drm/i915/intel_slpc.c | 42
It looks that SLPC is tight with Guc so maybe better names would be:
intel_guc_slpc.c and struct intel_guc_slpc
ie. the same pattern as with Guc log.
> ++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_slpc.h | 47
> +++++++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_uc.c | 20 +++++++++++++++++
> 8 files changed, 142 insertions(+), 5 deletions(-)
> create mode 100644 drivers/gpu/drm/i915/intel_slpc.c
> create mode 100644 drivers/gpu/drm/i915/intel_slpc.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile
> b/drivers/gpu/drm/i915/Makefile
> index d1327f6..62bf4f6e 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -64,6 +64,7 @@ i915-y += intel_uc.o \
> intel_guc_log.o \
> intel_guc_loader.o \
> intel_huc.o \
> + intel_slpc.o \
> i915_guc_submission.o
> # autogenerated null render state
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index 428cb1c..af633c6 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2748,6 +2748,18 @@ static inline struct drm_i915_private
> *guc_to_i915(struct intel_guc *guc)
> return container_of(guc, struct drm_i915_private, guc);
> }
> +static inline struct intel_guc *slpc_to_guc(struct intel_slpc *slpc)
> +{
> + return container_of(slpc, struct intel_guc, slpc);
> +}
> +
> +static inline struct drm_i915_private *slpc_to_i915(struct intel_slpc
> *slpc)
> +{
> + struct intel_guc *guc = slpc_to_guc(slpc);
> +
> + return guc_to_i915(guc);
> +}
> +
> static inline struct drm_i915_private *huc_to_i915(struct intel_huc
> *huc)
> {
> return container_of(huc, struct drm_i915_private, huc);
> diff --git a/drivers/gpu/drm/i915/intel_guc.c
> b/drivers/gpu/drm/i915/intel_guc.c
> index f4dc708..a92c7e8 100644
> --- a/drivers/gpu/drm/i915/intel_guc.c
> +++ b/drivers/gpu/drm/i915/intel_guc.c
> @@ -226,6 +226,9 @@ void intel_guc_cleanup(struct intel_guc *guc)
> if (i915.enable_guc_submission)
> i915_guc_submission_cleanup(dev_priv);
> +
> + if (intel_slpc_enabled())
> + intel_slpc_cleanup(&guc->slpc);
> }
> /**
> diff --git a/drivers/gpu/drm/i915/intel_guc.h
> b/drivers/gpu/drm/i915/intel_guc.h
> index 3821bf2..b835d30 100644
> --- a/drivers/gpu/drm/i915/intel_guc.h
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -28,6 +28,7 @@
> #include "intel_guc_fwif.h"
> #include "i915_guc_reg.h"
> #include "intel_guc_ct.h"
> +#include "intel_slpc.h"
> /*
> * This structure primarily describes the GEM object shared with the
> GuC.
> @@ -115,6 +116,8 @@ struct intel_guc {
> enum forcewake_domains fw_domains;
> } send_regs;
> + struct intel_slpc slpc;
> +
Pick better place - now you're inside 'send' related members.
> /* To serialize the intel_guc_send actions */
> struct mutex send_mutex;
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index 20ec8f4..e5607e5 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -7995,6 +7995,12 @@ void intel_suspend_gt_powersave(struct
> drm_i915_private *dev_priv)
> intel_runtime_pm_put(dev_priv);
> /* gen6_rps_idle() will be called later to disable interrupts */
> +
> + if (intel_slpc_active(&dev_priv->guc.slpc)) {
> + intel_runtime_pm_get(dev_priv);
> + intel_slpc_disable(&dev_priv->guc.slpc);
> + intel_runtime_pm_put(dev_priv);
> + }
> }
> void intel_sanitize_gt_powersave(struct drm_i915_private *dev_priv)
> @@ -8121,7 +8127,8 @@ void intel_enable_gt_powersave(struct
> drm_i915_private *dev_priv)
> mutex_lock(&dev_priv->pm.pcu_lock);
> intel_enable_rc6(dev_priv);
> - intel_enable_rps(dev_priv);
> + if (!intel_slpc_active(&dev_priv->guc.slpc))
> + intel_enable_rps(dev_priv);
> intel_update_ring_freq(dev_priv);
> mutex_unlock(&dev_priv->pm.pcu_lock);
> @@ -8134,8 +8141,9 @@ static void __intel_autoenable_gt_powersave(struct
> work_struct *work)
> struct intel_engine_cs *rcs;
> struct drm_i915_gem_request *req;
> - if (READ_ONCE(dev_priv->pm.rps.enabled) &&
> - READ_ONCE(dev_priv->pm.rc6.enabled) &&
> + if (READ_ONCE(dev_priv->pm.rc6.enabled) &&
> + !(!(intel_slpc_active(&dev_priv->guc.slpc)) ^
> + READ_ONCE(dev_priv->pm.rps.enabled)) &&
> !(NEEDS_RING_FREQ_UPDATE(dev_priv) ^
> READ_ONCE(dev_priv->pm.ring_pstate.configured)))
> goto out;
> @@ -8167,8 +8175,9 @@ static void __intel_autoenable_gt_powersave(struct
> work_struct *work)
> void intel_autoenable_gt_powersave(struct drm_i915_private *dev_priv)
> {
> - if (READ_ONCE(dev_priv->pm.rps.enabled) &&
> - READ_ONCE(dev_priv->pm.rc6.enabled) &&
> + if (READ_ONCE(dev_priv->pm.rc6.enabled) &&
> + !(!(intel_slpc_active(&dev_priv->guc.slpc)) ^
> + READ_ONCE(dev_priv->pm.rps.enabled)) &&
> !(NEEDS_RING_FREQ_UPDATE(dev_priv) ^
> READ_ONCE(dev_priv->pm.ring_pstate.configured)))
> return;
> diff --git a/drivers/gpu/drm/i915/intel_slpc.c
> b/drivers/gpu/drm/i915/intel_slpc.c
> new file mode 100644
> index 0000000..06abda5
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_slpc.c
> @@ -0,0 +1,42 @@
> +/*
> + * Copyright © 2017 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.
> + *
> + */
> +#include <linux/firmware.h>
> +#include "i915_drv.h"
> +#include "intel_uc.h"
> +
> +void intel_slpc_init(struct intel_slpc *slpc)
> +{
> +}
> +
> +void intel_slpc_cleanup(struct intel_slpc *slpc)
> +{
> +}
> +
> +void intel_slpc_enable(struct intel_slpc *slpc)
> +{
> +}
> +
> +void intel_slpc_disable(struct intel_slpc *slpc)
> +{
> +}
> diff --git a/drivers/gpu/drm/i915/intel_slpc.h
> b/drivers/gpu/drm/i915/intel_slpc.h
> new file mode 100644
> index 0000000..f68671f
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_slpc.h
> @@ -0,0 +1,47 @@
> +/*
> + * Copyright © 2017 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_SLPC_H_
> +#define _INTEL_SLPC_H_
> +
> +struct intel_slpc {
> + bool active;
> +};
> +
> +static inline int intel_slpc_enabled(void)
> +{
> + return i915.enable_slpc;
> +}
> +
> +static inline bool intel_slpc_active(struct intel_slpc *slpc)
> +{
> + return slpc->active;
> +}
> +
> +/* intel_slpc.c */
> +void intel_slpc_init(struct intel_slpc *slpc);
> +void intel_slpc_cleanup(struct intel_slpc *slpc);
> +void intel_slpc_enable(struct intel_slpc *slpc);
> +void intel_slpc_disable(struct intel_slpc *slpc);
> +
> +#endif
> diff --git a/drivers/gpu/drm/i915/intel_uc.c
> b/drivers/gpu/drm/i915/intel_uc.c
> index 350027f..990d84a 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -328,6 +328,9 @@ int intel_uc_init_hw(struct drm_i915_private
> *dev_priv)
> ret = i915_guc_submission_init(dev_priv);
> if (ret)
> goto err_guc;
> +
> + if (intel_slpc_enabled())
> + intel_slpc_init(&dev_priv->guc.slpc);
> }
> /* init WOPCM */
> @@ -369,6 +372,17 @@ int intel_uc_init_hw(struct drm_i915_private
> *dev_priv)
> goto err_log_capture;
> intel_huc_auth(&dev_priv->huc);
> +
> + /*
> + * SLPC is enabled by setting up the shared data structure and
> + * sending reset event to GuC SLPC. Initial data is setup in
> + * intel_slpc_init. Here we send the reset event. SLPC enabling
> + * in GuC can happen in parallel in GuC with other initialization
> + * being done in i915.
> + */
> + if (intel_slpc_enabled())
> + intel_slpc_enable(&dev_priv->guc.slpc);
> +
> if (i915.enable_guc_submission) {
> if (i915.guc_log_level >= 0)
> gen9_enable_guc_interrupts(dev_priv);
> @@ -398,6 +412,12 @@ int intel_uc_init_hw(struct drm_i915_private
> *dev_priv)
> if (i915.enable_guc_submission)
> i915_guc_submission_cleanup(dev_priv);
> err_guc:
> + if (intel_slpc_enabled()) {
> + if (intel_slpc_active(&dev_priv->guc.slpc))
> + intel_slpc_disable(&dev_priv->guc.slpc);
> + intel_slpc_cleanup(&dev_priv->guc.slpc);
> + }
> +
> i915_ggtt_disable_guc(dev_priv);
> DRM_ERROR("GuC init failed\n");
More information about the Intel-gfx
mailing list