On Fri, 30 Mar 2018 10:31:48 +0200, Sagar Arun Kamble
<sagar.a.kamble@xxxxxxxxx> 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@xxxxxxxxx>
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
Cc: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@xxxxxxxxx>
Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
Cc: Sujaritha Sundaresan <sujaritha.sundaresan@xxxxxxxxx>
Cc: Jeff McGee <jeff.mcgee@xxxxxxxxx>
---
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);
}
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
