| Message ID | 1522398722-12161-4-git-send-email-sagar.a.kamble@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 30 Mar 2018 10:31:48 +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_guc_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. > > v1: Return void instead of ignored error code. Replace HAS_SLPC() use > with > intel_slpc_enabled()/ intel_slpc_active() (Paulo) > Enable/disable RC6 in SLPC flows (Sagar) > 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. > > v11: s/intel_slpc/intel_guc_slpc. Adjusted place for slpc struct inside > guc struct. (Michal Wajdeczko) > Updated comment about intel_slpc_enable as we plan to not defer the > SLPC status check on enabling later and will have to wait for SLPC > status as part of intel_slpc_enable itself. > Prepared guc_slpc_initialized and guc_slpc_enabled to track state > of SLPC initialization and enabling. > > v12: s/guc_slpc_cleanup/guc_slpc_fini. Updated SLPC flows w.r.t uC flows. > > Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com> > Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Cc: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@intel.com> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> > Cc: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com> > Cc: Jeff McGee <jeff.mcgee@intel.com> > --- > drivers/gpu/drm/i915/Makefile | 1 + > drivers/gpu/drm/i915/intel_guc.h | 2 ++ > drivers/gpu/drm/i915/intel_guc_slpc.c | 25 +++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_guc_slpc.h | 17 +++++++++++++++++ > drivers/gpu/drm/i915/intel_uc.c | 30 > +++++++++++++++++++++++++++++- > 5 files changed, 74 insertions(+), 1 deletion(-) > create mode 100644 drivers/gpu/drm/i915/intel_guc_slpc.c > create mode 100644 drivers/gpu/drm/i915/intel_guc_slpc.h > > diff --git a/drivers/gpu/drm/i915/Makefile > b/drivers/gpu/drm/i915/Makefile > index 0c79c19..499cb89 100644 > --- a/drivers/gpu/drm/i915/Makefile > +++ b/drivers/gpu/drm/i915/Makefile > @@ -90,6 +90,7 @@ i915-y += intel_uc.o \ > intel_guc_ct.o \ > intel_guc_fw.o \ > intel_guc_log.o \ > + intel_guc_slpc.o \ > intel_guc_submission.o \ > intel_huc.o \ > intel_huc_fw.o > diff --git a/drivers/gpu/drm/i915/intel_guc.h > b/drivers/gpu/drm/i915/intel_guc.h > index f1265e1..2d2451a 100644 > --- a/drivers/gpu/drm/i915/intel_guc.h > +++ b/drivers/gpu/drm/i915/intel_guc.h > @@ -31,6 +31,7 @@ > #include "intel_guc_ct.h" > #include "intel_guc_log.h" > #include "intel_guc_reg.h" > +#include "intel_guc_slpc.h" > #include "intel_uc_fw.h" > #include "i915_vma.h" > @@ -48,6 +49,7 @@ struct intel_guc { > struct intel_uc_fw fw; > struct intel_guc_log log; > struct intel_guc_ct ct; > + struct intel_guc_slpc slpc; > /* Offset where Non-WOPCM memory starts. */ > u32 ggtt_pin_bias; > diff --git a/drivers/gpu/drm/i915/intel_guc_slpc.c > b/drivers/gpu/drm/i915/intel_guc_slpc.c > new file mode 100644 > index 0000000..63f100c > --- /dev/null > +++ b/drivers/gpu/drm/i915/intel_guc_slpc.c > @@ -0,0 +1,25 @@ > +/* > + * SPDX-License-Identifier: MIT > + * > + * Copyright © 2015-2018 Intel Corporation > + */ > + > +#include "intel_guc_slpc.h" > + > +int intel_guc_slpc_init(struct intel_guc_slpc *slpc) > +{ > + return 0; > +} > + > +int intel_guc_slpc_enable(struct intel_guc_slpc *slpc) > +{ > + return 0; > +} > + > +void intel_guc_slpc_disable(struct intel_guc_slpc *slpc) > +{ > +} > + > +void intel_guc_slpc_fini(struct intel_guc_slpc *slpc) > +{ > +} > diff --git a/drivers/gpu/drm/i915/intel_guc_slpc.h > b/drivers/gpu/drm/i915/intel_guc_slpc.h > new file mode 100644 > index 0000000..66c76fe > --- /dev/null > +++ b/drivers/gpu/drm/i915/intel_guc_slpc.h > @@ -0,0 +1,17 @@ > +/* > + * SPDX-License-Identifier: MIT > + * > + * Copyright © 2015-2018 Intel Corporation > + */ > +#ifndef _INTEL_GUC_SLPC_H_ > +#define _INTEL_GUC_SLPC_H_ > + > +struct intel_guc_slpc { > +}; > + > +int intel_guc_slpc_init(struct intel_guc_slpc *slpc); > +int intel_guc_slpc_enable(struct intel_guc_slpc *slpc); > +void intel_guc_slpc_disable(struct intel_guc_slpc *slpc); > +void intel_guc_slpc_fini(struct intel_guc_slpc *slpc); > + > +#endif > diff --git a/drivers/gpu/drm/i915/intel_uc.c > b/drivers/gpu/drm/i915/intel_uc.c > index 0e4a97f..5bf33c8 100644 > --- a/drivers/gpu/drm/i915/intel_uc.c > +++ b/drivers/gpu/drm/i915/intel_uc.c > @@ -319,6 +319,15 @@ int intel_uc_init(struct drm_i915_private *dev_priv) > } > } > + if (USES_GUC_SLPC(dev_priv)) { > + ret = intel_guc_slpc_init(&guc->slpc); > + if (ret) { > + intel_guc_submission_fini(guc); > + intel_guc_fini(guc); > + return ret; I think we prefer to use "goto" single cleanup section at the end of function. > + } > + } > + > return 0; > } > @@ -331,6 +340,9 @@ void intel_uc_fini(struct drm_i915_private *dev_priv) > GEM_BUG_ON(!HAS_GUC(dev_priv)); > + if (USES_GUC_SLPC(dev_priv)) > + intel_guc_slpc_fini(&guc->slpc); > + > if (USES_GUC_SUBMISSION(dev_priv)) > intel_guc_submission_fini(guc); > @@ -413,10 +425,21 @@ int intel_uc_init_hw(struct drm_i915_private > *dev_priv) > goto err_communication; > } > + /* > + * SLPC is enabled by setting up the shared data structure and > + * sending reset event to GuC SLPC. Initial data is setup in > + * intel_guc_slpc_init. Here we send the reset event. > + */ This comment should be part of intel_guc_slpc_enable fun description. Hmm, but wait, this patch defines only placeholders for slpc* functions, so maybe you should revisit that idea and provide real implementation here. > + if (USES_GUC_SLPC(dev_priv)) { > + ret = intel_guc_slpc_enable(&guc->slpc); > + if (ret) > + goto err_communication; > + } > + > if (USES_GUC_SUBMISSION(dev_priv)) { > ret = intel_guc_submission_enable(guc); > if (ret) > - goto err_communication; > + goto err_slpc; > } > dev_info(dev_priv->drm.dev, "GuC firmware version %u.%u\n", > @@ -431,6 +454,8 @@ int intel_uc_init_hw(struct drm_i915_private > *dev_priv) > /* > * We've failed to load the firmware :( > */ > +err_slpc: > + intel_guc_slpc_disable(&guc->slpc); > err_communication: > guc_disable_communication(guc); > err_log_capture: > @@ -459,6 +484,9 @@ void intel_uc_fini_hw(struct drm_i915_private > *dev_priv) > if (USES_GUC_SUBMISSION(dev_priv)) > intel_guc_submission_disable(guc); > + if (USES_GUC_SLPC(dev_priv)) > + intel_guc_slpc_disable(&guc->slpc); > + > guc_disable_communication(guc); > }
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 0c79c19..499cb89 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -90,6 +90,7 @@ i915-y += intel_uc.o \ intel_guc_ct.o \ intel_guc_fw.o \ intel_guc_log.o \ + intel_guc_slpc.o \ intel_guc_submission.o \ intel_huc.o \ intel_huc_fw.o diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h index f1265e1..2d2451a 100644 --- a/drivers/gpu/drm/i915/intel_guc.h +++ b/drivers/gpu/drm/i915/intel_guc.h @@ -31,6 +31,7 @@ #include "intel_guc_ct.h" #include "intel_guc_log.h" #include "intel_guc_reg.h" +#include "intel_guc_slpc.h" #include "intel_uc_fw.h" #include "i915_vma.h" @@ -48,6 +49,7 @@ struct intel_guc { struct intel_uc_fw fw; struct intel_guc_log log; struct intel_guc_ct ct; + struct intel_guc_slpc slpc; /* Offset where Non-WOPCM memory starts. */ u32 ggtt_pin_bias; diff --git a/drivers/gpu/drm/i915/intel_guc_slpc.c b/drivers/gpu/drm/i915/intel_guc_slpc.c new file mode 100644 index 0000000..63f100c --- /dev/null +++ b/drivers/gpu/drm/i915/intel_guc_slpc.c @@ -0,0 +1,25 @@ +/* + * SPDX-License-Identifier: MIT + * + * Copyright © 2015-2018 Intel Corporation + */ + +#include "intel_guc_slpc.h" + +int intel_guc_slpc_init(struct intel_guc_slpc *slpc) +{ + return 0; +} + +int intel_guc_slpc_enable(struct intel_guc_slpc *slpc) +{ + return 0; +} + +void intel_guc_slpc_disable(struct intel_guc_slpc *slpc) +{ +} + +void intel_guc_slpc_fini(struct intel_guc_slpc *slpc) +{ +} diff --git a/drivers/gpu/drm/i915/intel_guc_slpc.h b/drivers/gpu/drm/i915/intel_guc_slpc.h new file mode 100644 index 0000000..66c76fe --- /dev/null +++ b/drivers/gpu/drm/i915/intel_guc_slpc.h @@ -0,0 +1,17 @@ +/* + * SPDX-License-Identifier: MIT + * + * Copyright © 2015-2018 Intel Corporation + */ +#ifndef _INTEL_GUC_SLPC_H_ +#define _INTEL_GUC_SLPC_H_ + +struct intel_guc_slpc { +}; + +int intel_guc_slpc_init(struct intel_guc_slpc *slpc); +int intel_guc_slpc_enable(struct intel_guc_slpc *slpc); +void intel_guc_slpc_disable(struct intel_guc_slpc *slpc); +void intel_guc_slpc_fini(struct intel_guc_slpc *slpc); + +#endif diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c index 0e4a97f..5bf33c8 100644 --- a/drivers/gpu/drm/i915/intel_uc.c +++ b/drivers/gpu/drm/i915/intel_uc.c @@ -319,6 +319,15 @@ int intel_uc_init(struct drm_i915_private *dev_priv) } } + if (USES_GUC_SLPC(dev_priv)) { + ret = intel_guc_slpc_init(&guc->slpc); + if (ret) { + intel_guc_submission_fini(guc); + intel_guc_fini(guc); + return ret; + } + } + return 0; } @@ -331,6 +340,9 @@ void intel_uc_fini(struct drm_i915_private *dev_priv) GEM_BUG_ON(!HAS_GUC(dev_priv)); + if (USES_GUC_SLPC(dev_priv)) + intel_guc_slpc_fini(&guc->slpc); + if (USES_GUC_SUBMISSION(dev_priv)) intel_guc_submission_fini(guc); @@ -413,10 +425,21 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv) goto err_communication; } + /* + * SLPC is enabled by setting up the shared data structure and + * sending reset event to GuC SLPC. Initial data is setup in + * intel_guc_slpc_init. Here we send the reset event. + */ + if (USES_GUC_SLPC(dev_priv)) { + ret = intel_guc_slpc_enable(&guc->slpc); + if (ret) + goto err_communication; + } + if (USES_GUC_SUBMISSION(dev_priv)) { ret = intel_guc_submission_enable(guc); if (ret) - goto err_communication; + goto err_slpc; } dev_info(dev_priv->drm.dev, "GuC firmware version %u.%u\n", @@ -431,6 +454,8 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv) /* * We've failed to load the firmware :( */ +err_slpc: + intel_guc_slpc_disable(&guc->slpc); err_communication: guc_disable_communication(guc); err_log_capture: @@ -459,6 +484,9 @@ void intel_uc_fini_hw(struct drm_i915_private *dev_priv) if (USES_GUC_SUBMISSION(dev_priv)) intel_guc_submission_disable(guc); + if (USES_GUC_SLPC(dev_priv)) + intel_guc_slpc_disable(&guc->slpc); + guc_disable_communication(guc); }
SLPC operates based on parameters setup in shared data between i915 and GuC SLPC. This is to be created/initialized in intel_guc_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. v1: Return void instead of ignored error code. Replace HAS_SLPC() use with intel_slpc_enabled()/ intel_slpc_active() (Paulo) Enable/disable RC6 in SLPC flows (Sagar) 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. v11: s/intel_slpc/intel_guc_slpc. Adjusted place for slpc struct inside guc struct. (Michal Wajdeczko) Updated comment about intel_slpc_enable as we plan to not defer the SLPC status check on enabling later and will have to wait for SLPC status as part of intel_slpc_enable itself. Prepared guc_slpc_initialized and guc_slpc_enabled to track state of SLPC initialization and enabling. v12: s/guc_slpc_cleanup/guc_slpc_fini. Updated SLPC flows w.r.t uC flows. Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@intel.com> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com> Cc: Jeff McGee <jeff.mcgee@intel.com> --- drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/intel_guc.h | 2 ++ drivers/gpu/drm/i915/intel_guc_slpc.c | 25 +++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_guc_slpc.h | 17 +++++++++++++++++ drivers/gpu/drm/i915/intel_uc.c | 30 +++++++++++++++++++++++++++++- 5 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/i915/intel_guc_slpc.c create mode 100644 drivers/gpu/drm/i915/intel_guc_slpc.h