| Message ID | 1505842927-13327-17-git-send-email-sagar.a.kamble@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 19 Sep 2017 19:41:52 +0200, Sagar Arun Kamble <sagar.a.kamble@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@intel.com> > Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@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");
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@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@intel.com> >> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@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");
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; + /* 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");
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@intel.com> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@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 +++ drivers/gpu/drm/i915/intel_pm.c | 19 +++++++++++----- drivers/gpu/drm/i915/intel_slpc.c | 42 ++++++++++++++++++++++++++++++++++ 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