[Intel-gfx] [PATCH 16/31] drm/i915/slpc: Lay out SLPC init/enable/disable/cleanup helpers
Sagar Arun Kamble
sagar.a.kamble at intel.com
Thu Sep 28 09:29:12 UTC 2017
On 9/21/2017 6:30 PM, Michal Wajdeczko wrote:
> 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
Yes. Will make this independent 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.
Sure. Will update.
>
>
>> ++++++++++++++++++++++++++++++++++
>> 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.
Ok. Will update.
>
>> /* 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