[RFC 00/22] Add support for GuC-based SLPC

[RFC 00/22] Add support for GuC-based SLPC

[tom.orourke]

From: Tom O'Rourke <Tom.O'Rourke@intel.com>

SLPC (Single Loop Power Controller) is a replacement for
some host-based power management features.  The SLPC
implemenation runs in firmware on GuC.

This series is a first request for comments.  This series
is not expected to be merged.  After changes based on
comments, a later patch series will be sent for merging.
 
This series has been tested with SKL guc firmware
versions 4.3 and 4.7.  The graphics power management
features in SLPC in those versions are DFPS (Dynamic FPS),
Turbo, and DCC (Duty Cycle Control).  DFPS adjusts
requested graphics frequency to maintain target framerate.
Turbo adjusts requested graphics frequency to maintain
target GT busyness.  DCC adjusts requested graphics
frequency and stalls guc-scheduler to maintain actual
graphics frequency in efficient range.

Patch 1/22 is included ihere for convenience and should be
part of an earlier series.  SLPC assumes guc firmware has
been loaded and GuC submission is enabled.

Patch 22/22 sets the flag to enable SLPC on SKL.  Without
this patch, the previous patches should have no effect.

VIZ-6773, VIZ-6889

Dave Gordon (1):
  drm/i915: Enable GuC submission, where supported

Sagar Arun Kamble (4):
  drm/i915/slpc: Enable/Disable RC6 in SLPC flows
  drm/i915/slpc: Add Display mode event related data structures
  drm/i915/slpc: Notification of Display mode change
  drm/i915/slpc: Notification of Refresh Rate change

Tom O'Rourke (17):
  drm/i915/slpc: Add has_slpc capability flag
  drm/i915/slpc: Expose guc functions for use with SLPC
  drm/i915/slpc: Use intel_slpc_* functions if supported
  drm/i915/slpc: If using SLPC, do not set frequency
  drm/i915/slpc: Enable SLPC in guc if supported
  drm/i915/slpc: Allocate/Release/Initialize SLPC shared data
  drm/i915/slpc: Setup rps frequency values during SLPC init
  drm/i915/slpc: Update current requested frequency
  drm/i915/slpc: Send reset event
  drm/i915/slpc: Send shutdown event
  drm/i915/slpc: Add slpc_status enum values
  drm/i915/slpc: Add i915_slpc_info to debugfs
  drm/i915/slpc: Add dfps task info to i915_slpc_info
  drm/i915/slpc: Add parameter unset/set/get functions
  drm/i915/slpc: Add slpc support for max/min freq
  drm/i915/slpc: Add enable/disable debugfs for slpc
  drm/i915/slpc: Add has_slpc to skylake info

 drivers/gpu/drm/i915/Makefile              |   5 +-
 drivers/gpu/drm/i915/i915_debugfs.c        | 436 +++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_drv.c            |   1 +
 drivers/gpu/drm/i915/i915_drv.h            |   2 +
 drivers/gpu/drm/i915/i915_guc_submission.c |   6 +-
 drivers/gpu/drm/i915/i915_params.c         |   4 +-
 drivers/gpu/drm/i915/i915_sysfs.c          |  10 +
 drivers/gpu/drm/i915/intel_display.c       |   2 +
 drivers/gpu/drm/i915/intel_dp.c            |   2 +
 drivers/gpu/drm/i915/intel_drv.h           |   1 +
 drivers/gpu/drm/i915/intel_guc.h           |   7 +
 drivers/gpu/drm/i915/intel_guc_loader.c    |   3 +
 drivers/gpu/drm/i915/intel_pm.c            |  43 ++-
 drivers/gpu/drm/i915/intel_slpc.c          | 499 +++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_slpc.h          | 207 ++++++++++++
 15 files changed, 1210 insertions(+), 18 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/intel_slpc.c
 create mode 100644 drivers/gpu/drm/i915/intel_slpc.h

-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 01/22] drm/i915: Enable GuC submission, where supported

[tom.orourke]

From: Dave Gordon <david.s.gordon@intel.com>

Signed-off-by: Dave Gordon <david.s.gordon@intel.com>

v5:
    Rebased

Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
---
 drivers/gpu/drm/i915/i915_params.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 8d90c25..2ca4690 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -55,7 +55,7 @@ struct i915_params i915 __read_mostly = {
 	.verbose_state_checks = 1,
 	.nuclear_pageflip = 0,
 	.edp_vswing = 0,
-	.enable_guc_submission = false,
+	.enable_guc_submission = true,
 	.guc_log_level = -1,
 };
 
@@ -198,7 +198,7 @@ MODULE_PARM_DESC(edp_vswing,
 		 "2=default swing(400mV))");
 
 module_param_named_unsafe(enable_guc_submission, i915.enable_guc_submission, bool, 0400);
-MODULE_PARM_DESC(enable_guc_submission, "Enable GuC submission (default:false)");
+MODULE_PARM_DESC(enable_guc_submission, "Enable GuC submission (default:true)");
 
 module_param_named(guc_log_level, i915.guc_log_level, int, 0400);
 MODULE_PARM_DESC(guc_log_level,
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 02/22] drm/i915/slpc: Add has_slpc capability flag

[tom.orourke]

From: Tom O'Rourke <Tom.O'Rourke@intel.com>

Add has_slpc capablity flag to indicate GuC firmware
supports single loop power control (SLPC).  SLPC is
a replacement for some host-based power management
features.

Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d3b98c2..3f4b925 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -768,6 +768,7 @@ struct intel_csr {
 	func(is_kabylake) sep \
 	func(is_preliminary) sep \
 	func(has_fbc) sep \
+	func(has_slpc) sep \
 	func(has_pipe_cxsr) sep \
 	func(has_hotplug) sep \
 	func(cursor_needs_physical) sep \
@@ -2626,6 +2627,7 @@ struct drm_i915_cmd_table {
 
 #define HAS_GUC_UCODE(dev)	(IS_GEN9(dev) && !IS_KABYLAKE(dev))
 #define HAS_GUC_SCHED(dev)	(IS_GEN9(dev) && !IS_KABYLAKE(dev))
+#define HAS_SLPC(dev) 		(INTEL_INFO(dev)->has_slpc)
 
 #define HAS_RESOURCE_STREAMER(dev) (IS_HASWELL(dev) || \
 				    INTEL_INFO(dev)->gen >= 8)
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 03/22] drm/i915/slpc: Expose guc functions for use with SLPC

[tom.orourke]

From: Tom O'Rourke <Tom.O'Rourke@intel.com>

Expose host2guc_action for use by SLPC in intel_slpc.c.

Expose functions to allocate and release objects used
by GuC to be used for SLPC shared memory object.

Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 6 +++---
 drivers/gpu/drm/i915/intel_guc.h           | 4 ++++
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 9c24424..c3f50f9 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -75,7 +75,7 @@ static inline bool host2guc_action_response(struct drm_i915_private *dev_priv,
 	return GUC2HOST_IS_RESPONSE(val);
 }
 
-static int host2guc_action(struct intel_guc *guc, u32 *data, u32 len)
+int host2guc_action(struct intel_guc *guc, u32 *data, u32 len)
 {
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
 	u32 status;
@@ -638,7 +638,7 @@ int i915_guc_submit(struct i915_guc_client *client,
  *
  * Return:	A drm_i915_gem_object if successful, otherwise NULL.
  */
-static struct drm_i915_gem_object *gem_allocate_guc_obj(struct drm_device *dev,
+struct drm_i915_gem_object *gem_allocate_guc_obj(struct drm_device *dev,
 							u32 size)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -669,7 +669,7 @@ static struct drm_i915_gem_object *gem_allocate_guc_obj(struct drm_device *dev,
  * gem_release_guc_obj() - Release gem object allocated for GuC usage
  * @obj:	gem obj to be released
  */
-static void gem_release_guc_obj(struct drm_i915_gem_object *obj)
+void gem_release_guc_obj(struct drm_i915_gem_object *obj)
 {
 	if (!obj)
 		return;
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 045b149..e5de759 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -119,10 +119,14 @@ extern int intel_guc_suspend(struct drm_device *dev);
 extern int intel_guc_resume(struct drm_device *dev);
 
 /* i915_guc_submission.c */
+int host2guc_action(struct intel_guc *guc, u32 *data, u32 len);
 int i915_guc_submission_init(struct drm_device *dev);
 int i915_guc_submission_enable(struct drm_device *dev);
 int i915_guc_submit(struct i915_guc_client *client,
 		    struct drm_i915_gem_request *rq);
+struct drm_i915_gem_object *gem_allocate_guc_obj(struct drm_device *dev,
+							u32 size);
+void gem_release_guc_obj(struct drm_i915_gem_object *obj);
 void i915_guc_submission_disable(struct drm_device *dev);
 void i915_guc_submission_fini(struct drm_device *dev);
 int i915_guc_wq_check_space(struct i915_guc_client *client);
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 04/22] drm/i915/slpc: Use intel_slpc_* functions if supported

[tom.orourke]

From: Tom O'Rourke <Tom.O'Rourke@intel.com>

On platforms with SLPC support: call intel_slpc_*()
functions from corresponding intel_*_gt_powersave()
functions; and do not use rps functions.

Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
---
 drivers/gpu/drm/i915/Makefile     |  5 +--
 drivers/gpu/drm/i915/intel_pm.c   | 36 +++++++++++++++------
 drivers/gpu/drm/i915/intel_slpc.c | 68 +++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_slpc.h | 35 ++++++++++++++++++++
 4 files changed, 132 insertions(+), 12 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 0851de07..92b378b 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -41,8 +41,9 @@ i915-y += i915_cmd_parser.o \
 	  intel_uncore.o
 
 # general-purpose microcontroller (GuC) support
-i915-y += intel_guc_loader.o \
-	  i915_guc_submission.o
+i915-y += i915_guc_submission.o \
+	  intel_guc_loader.o \
+	  intel_slpc.o
 
 # autogenerated null render state
 i915-y += intel_renderstate_gen6.o \
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 20bf854..27713ab 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6067,7 +6067,9 @@ void intel_init_gt_powersave(struct drm_device *dev)
 		intel_runtime_pm_get(dev_priv);
 	}
 
-	if (IS_CHERRYVIEW(dev))
+	if (HAS_SLPC(dev))
+		intel_slpc_init(dev);
+	else if (IS_CHERRYVIEW(dev))
 		cherryview_init_gt_powersave(dev);
 	else if (IS_VALLEYVIEW(dev))
 		valleyview_init_gt_powersave(dev);
@@ -6076,8 +6078,10 @@ void intel_init_gt_powersave(struct drm_device *dev)
 void intel_cleanup_gt_powersave(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-
-	if (IS_CHERRYVIEW(dev))
+	
+	if (HAS_SLPC(dev))
+		intel_slpc_cleanup(dev);
+	else if (IS_CHERRYVIEW(dev))
 		return;
 	else if (IS_VALLEYVIEW(dev))
 		valleyview_cleanup_gt_powersave(dev);
@@ -6110,17 +6114,23 @@ void intel_suspend_gt_powersave(struct drm_device *dev)
 	if (INTEL_INFO(dev)->gen < 6)
 		return;
 
-	gen6_suspend_rps(dev);
+	if (HAS_SLPC(dev)) {
+		intel_slpc_suspend(dev);
+	} else {
+		gen6_suspend_rps(dev);
 
-	/* Force GPU to min freq during suspend */
-	gen6_rps_idle(dev_priv);
+		/* Force GPU to min freq during suspend */
+		gen6_rps_idle(dev_priv);
+	}
 }
 
 void intel_disable_gt_powersave(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
-	if (IS_IRONLAKE_M(dev)) {
+	if (HAS_SLPC(dev)) {
+		intel_slpc_disable(dev);
+	} else if (IS_IRONLAKE_M(dev)) {
 		ironlake_disable_drps(dev);
 	} else if (INTEL_INFO(dev)->gen >= 6) {
 		intel_suspend_gt_powersave(dev);
@@ -6191,7 +6201,9 @@ void intel_enable_gt_powersave(struct drm_device *dev)
 	if (intel_vgpu_active(dev))
 		return;
 
-	if (IS_IRONLAKE_M(dev)) {
+	if (HAS_SLPC(dev)) {
+		intel_slpc_enable(dev);
+	} else if (IS_IRONLAKE_M(dev)) {
 		mutex_lock(&dev->struct_mutex);
 		ironlake_enable_drps(dev);
 		intel_init_emon(dev);
@@ -6222,8 +6234,12 @@ void intel_reset_gt_powersave(struct drm_device *dev)
 	if (INTEL_INFO(dev)->gen < 6)
 		return;
 
-	gen6_suspend_rps(dev);
-	dev_priv->rps.enabled = false;
+	if (HAS_SLPC(dev)) {
+		intel_slpc_reset(dev);
+	} else {
+		gen6_suspend_rps(dev);
+		dev_priv->rps.enabled = false;
+	}
 }
 
 static void ibx_init_clock_gating(struct drm_device *dev)
diff --git a/drivers/gpu/drm/i915/intel_slpc.c b/drivers/gpu/drm/i915/intel_slpc.c
new file mode 100644
index 0000000..dcd237f
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_slpc.c
@@ -0,0 +1,68 @@
+/*
+ * Copyright © 2015 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_guc.h"
+
+int intel_slpc_init(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	return 0;
+}
+
+int intel_slpc_cleanup(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	return 0;
+}
+
+int intel_slpc_suspend(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	return 0;
+}
+
+int intel_slpc_disable(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	return 0;
+}
+
+int intel_slpc_enable(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	return 0;
+}
+
+int intel_slpc_reset(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	return 0;
+}
diff --git a/drivers/gpu/drm/i915/intel_slpc.h b/drivers/gpu/drm/i915/intel_slpc.h
new file mode 100644
index 0000000..8f13fb5
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_slpc.h
@@ -0,0 +1,35 @@
+/*
+ * Copyright © 2015 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_
+
+/* intel_slpc.c */
+int intel_slpc_init(struct drm_device *dev);
+int intel_slpc_cleanup(struct drm_device *dev);
+int intel_slpc_suspend(struct drm_device *dev);
+int intel_slpc_disable(struct drm_device *dev);
+int intel_slpc_enable(struct drm_device *dev);
+int intel_slpc_reset(struct drm_device *dev);
+
+#endif
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 05/22] drm/i915/slpc: Enable/Disable RC6 in SLPC flows

[tom.orourke]

From: Sagar Arun Kamble <sagar.a.kamble@intel.com>

Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 27713ab..ec1868b 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6130,6 +6130,7 @@ void intel_disable_gt_powersave(struct drm_device *dev)
 
 	if (HAS_SLPC(dev)) {
 		intel_slpc_disable(dev);
+		gen9_disable_rps(dev);
 	} else if (IS_IRONLAKE_M(dev)) {
 		ironlake_disable_drps(dev);
 	} else if (INTEL_INFO(dev)->gen >= 6) {
@@ -6202,6 +6203,7 @@ void intel_enable_gt_powersave(struct drm_device *dev)
 		return;
 
 	if (HAS_SLPC(dev)) {
+		gen9_enable_rc6(dev);
 		intel_slpc_enable(dev);
 	} else if (IS_IRONLAKE_M(dev)) {
 		mutex_lock(&dev->struct_mutex);
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 06/22] drm/i915/slpc: If using SLPC, do not set frequency

[tom.orourke]

From: Tom O'Rourke <Tom.O'Rourke@intel.com>

When frequency requests are made by SLPC, host driver
should not attempt to make frequency requests due to
potential conflicts.

Host-based turbo operations are already avoided when
SLPC is used.  This change covers other frequency
requests such as from sysfs or debugfs interfaces.

Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index ec1868b..f5a6bf6 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4501,6 +4501,9 @@ void gen6_rps_boost(struct drm_i915_private *dev_priv,
 
 void intel_set_rps(struct drm_device *dev, u8 val)
 {
+	if (HAS_SLPC(dev))
+		return;
+
 	if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev))
 		valleyview_set_rps(dev, val);
 	else
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 07/22] drm/i915/slpc: Enable SLPC in guc if supported

[tom.orourke]

From: Tom O'Rourke <Tom.O'Rourke@intel.com>

If HAS_SLPC, then add enable SLPC flag to guc control
parameter during guc load.

Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_loader.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index 3accd91..b95504e 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -159,6 +159,9 @@ static void set_guc_init_params(struct drm_i915_private *dev_priv)
 	params[GUC_CTL_FEATURE] |= GUC_CTL_DISABLE_SCHEDULER |
 			GUC_CTL_VCS2_ENABLED;
 
+	if (HAS_SLPC(dev_priv->dev))
+		params[GUC_CTL_FEATURE] |= GUC_CTL_ENABLE_SLPC;
+
 	if (i915.guc_log_level >= 0) {
 		params[GUC_CTL_LOG_PARAMS] = guc->log_flags;
 		params[GUC_CTL_DEBUG] =
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 08/22] drm/i915/slpc: Allocate/Release/Initialize SLPC shared data

[tom.orourke]

From: Tom O'Rourke <Tom.O'Rourke@intel.com>

SLPC shared data is used to pass information
to/from SLPC firmware.

For Skylake, platform sku type and slice count
are identified from device id and fuse values.

Support for other platforms needs to be added.

Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
---
 drivers/gpu/drm/i915/intel_guc.h  |  3 ++
 drivers/gpu/drm/i915/intel_slpc.c | 93 ++++++++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_slpc.h | 69 +++++++++++++++++++++++++++++
 3 files changed, 163 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index e5de759..23cbcc1 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -26,6 +26,7 @@
 
 #include "intel_guc_fwif.h"
 #include "i915_guc_reg.h"
+#include "intel_slpc.h"
 
 struct i915_guc_client {
 	struct drm_i915_gem_object *client_obj;
@@ -108,6 +109,8 @@ struct intel_guc {
 
 	uint64_t submissions[I915_NUM_RINGS];
 	uint32_t last_seqno[I915_NUM_RINGS];
+
+	struct intel_slpc slpc;
 };
 
 /* intel_guc_loader.c */
diff --git a/drivers/gpu/drm/i915/intel_slpc.c b/drivers/gpu/drm/i915/intel_slpc.c
index dcd237f..c298a6c 100644
--- a/drivers/gpu/drm/i915/intel_slpc.c
+++ b/drivers/gpu/drm/i915/intel_slpc.c
@@ -22,21 +22,110 @@
  *
  */
 #include <linux/firmware.h>
+#include <asm/msr-index.h>
 #include "i915_drv.h"
 #include "intel_guc.h"
 
+static u8 slpc_get_platform_sku(struct drm_i915_gem_object *obj)
+{
+	struct drm_device *dev = obj->base.dev;
+	enum slpc_platform_sku platform_sku;
+
+	if (IS_SKL_ULX(dev))
+		platform_sku = SLPC_PLATFORM_SKU_ULX;
+	else if (IS_SKL_ULT(dev))
+		platform_sku = SLPC_PLATFORM_SKU_ULT;
+	else
+		platform_sku = SLPC_PLATFORM_SKU_DT;
+
+	return (u8) platform_sku;
+}
+
+static u8 slpc_get_slice_count(struct drm_i915_gem_object *obj)
+{
+	struct drm_device *dev = obj->base.dev;
+	u8 slice_count = 1;
+
+	if (IS_SKYLAKE(dev))
+		slice_count = INTEL_INFO(dev)->slice_total;
+
+	return slice_count;
+}
+
+static int slpc_shared_data_init(struct drm_i915_gem_object *obj)
+{
+	struct page *page;
+	struct slpc_shared_data *data;
+	u64 msr_value;
+	int ret = 0;
+
+	page = i915_gem_object_get_page(obj, 0);
+	if (page) {
+		data = kmap_atomic(page);
+		memset(data, 0, sizeof(struct slpc_shared_data));
+
+		data->slpc_version = SLPC_VERSION;
+		data->shared_data_size = sizeof(struct slpc_shared_data);
+		data->global_state = (u32) SLPC_GLOBAL_STATE_NOT_RUNNING;
+		data->platform_info.platform_sku = slpc_get_platform_sku(obj);
+		data->platform_info.slice_count = slpc_get_slice_count(obj);
+		data->platform_info.power_plan_source =
+			(u8) SLPC_POWER_PLAN_SOURCE(SLPC_POWER_PLAN_BALANCED,
+						    SLPC_POWER_SOURCE_AC);
+		rdmsrl(MSR_TURBO_RATIO_LIMIT, msr_value);
+		data->platform_info.P0_freq = (u8) msr_value;
+		rdmsrl(MSR_PLATFORM_INFO, msr_value);
+		data->platform_info.P1_freq = (u8) (msr_value >> 8);
+		data->platform_info.Pe_freq = (u8) (msr_value >> 40);
+		data->platform_info.Pn_freq = (u8) (msr_value >> 48);
+		rdmsrl(MSR_PKG_POWER_LIMIT, msr_value);
+		data->platform_info.package_rapl_limit_high =
+							(u32) (msr_value >> 32);
+		data->platform_info.package_rapl_limit_low = (u32) msr_value;
+
+		kunmap_atomic(data);
+	}
+
+	return ret;
+}
+
 int intel_slpc_init(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_i915_gem_object *obj;
+	int ret = 0;
 
-	return 0;
+	/* Allocate shared data structure */
+	obj = dev_priv->guc.slpc.shared_data_obj;
+	if (!obj) {
+		obj = gem_allocate_guc_obj(dev_priv->dev,
+				PAGE_ALIGN(sizeof(struct slpc_shared_data)));
+		dev_priv->guc.slpc.shared_data_obj = obj;
+	}
+
+	if (!obj) {
+		ret = -ENOMEM;
+	} else {
+		ret = slpc_shared_data_init(obj);
+	}
+
+	return ret;
 }
 
 int intel_slpc_cleanup(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_i915_gem_object *obj;
+	int ret = 0;
 
-	return 0;
+	/* Release shared data sturcutre */
+	obj = dev_priv->guc.slpc.shared_data_obj;
+	if (obj) {
+		gem_release_guc_obj(obj);
+		dev_priv->guc.slpc.shared_data_obj = NULL;
+	}
+
+	return ret;
 }
 
 int intel_slpc_suspend(struct drm_device *dev)
diff --git a/drivers/gpu/drm/i915/intel_slpc.h b/drivers/gpu/drm/i915/intel_slpc.h
index 8f13fb5..dceef7e 100644
--- a/drivers/gpu/drm/i915/intel_slpc.h
+++ b/drivers/gpu/drm/i915/intel_slpc.h
@@ -24,6 +24,75 @@
 #ifndef _INTEL_SLPC_H_
 #define _INTEL_SLPC_H_
 
+#define SLPC_MAJOR_VER 2
+#define SLPC_MINOR_VER 3
+#define SLPC_VERSION ((2015 << 16) | (SLPC_MAJOR_VER << 8) | (SLPC_MINOR_VER))
+
+enum slpc_global_state {
+	SLPC_GLOBAL_STATE_NOT_RUNNING = 0,
+	SLPC_GLOBAL_STATE_INITIALIZING = 1,
+	SLPC_GLOBAL_STATE_RESETING = 2,
+	SLPC_GLOBAL_STATE_RUNNING = 3,
+	SLPC_GLOBAL_STATE_SHUTTING_DOWN = 4,
+	SLPC_GLOBAL_STATE_ERROR = 5
+};
+
+enum slpc_platform_sku {
+	SLPC_PLATFORM_SKU_UNDEFINED = 0,
+	SLPC_PLATFORM_SKU_ULX = 1,
+	SLPC_PLATFORM_SKU_ULT = 2,
+	SLPC_PLATFORM_SKU_T = 3,
+	SLPC_PLATFORM_SKU_MOBL = 4,
+	SLPC_PLATFORM_SKU_DT = 5,
+	SLPC_PLATFORM_SKU_UNKNOWN = 6,
+};
+
+enum slpc_power_plan {
+        SLPC_POWER_PLAN_UNDEFINED = 0,
+        SLPC_POWER_PLAN_BATTERY_SAVER = 1,
+        SLPC_POWER_PLAN_BALANCED = 2,
+        SLPC_POWER_PLAN_PERFORMANCE = 3,
+        SLPC_POWER_PLAN_UNKNOWN = 4,
+};
+
+enum slpc_power_source {
+        SLPC_POWER_SOURCE_UNDEFINED = 0,
+        SLPC_POWER_SOURCE_AC = 1,
+        SLPC_POWER_SOURCE_DC = 2,
+        SLPC_POWER_SOURCE_UNKNOWN = 3,
+};
+
+#define SLPC_POWER_PLAN_SOURCE(plan, source) ((plan) | ((source) << 6))
+
+struct slpc_platform_info {
+	u8 platform_sku;
+	u16 slice_count;
+	u8 power_plan_source;
+	u8 P0_freq;
+	u8 P1_freq;
+	u8 Pe_freq;
+	u8 Pn_freq;
+	u32 package_rapl_limit_high;
+	u32 package_rapl_limit_low;
+} __packed;
+
+#define SLPC_MAX_OVERRIDE_PARAMETERS 128
+#define SLPC_OVERRIDE_BITFIELD_SIZE ((SLPC_MAX_OVERRIDE_PARAMETERS + 31) / 32)
+
+struct slpc_shared_data {
+	u32 slpc_version;
+	u32 shared_data_size;
+	u32 global_state;
+	struct slpc_platform_info platform_info;
+	u32 task_state_data;
+	u32 override_parameters_set_bits[SLPC_OVERRIDE_BITFIELD_SIZE];
+	u32 override_parameters_values[SLPC_MAX_OVERRIDE_PARAMETERS];
+} __packed;
+
+struct intel_slpc {
+	struct drm_i915_gem_object *shared_data_obj;
+};
+
 /* intel_slpc.c */
 int intel_slpc_init(struct drm_device *dev);
 int intel_slpc_cleanup(struct drm_device *dev);
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 09/22] drm/i915/slpc: Setup rps frequency values during SLPC init

[tom.orourke]

From: Tom O'Rourke <Tom.O'Rourke@intel.com>

Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h  | 1 +
 drivers/gpu/drm/i915/intel_pm.c   | 2 +-
 drivers/gpu/drm/i915/intel_slpc.c | 3 +++
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index bc97012..dc9e714 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1539,6 +1539,7 @@ void intel_init_pm(struct drm_device *dev);
 void intel_pm_setup(struct drm_device *dev);
 void intel_gpu_ips_init(struct drm_i915_private *dev_priv);
 void intel_gpu_ips_teardown(void);
+void gen6_init_rps_frequencies(struct drm_device *dev);
 void intel_init_gt_powersave(struct drm_device *dev);
 void intel_cleanup_gt_powersave(struct drm_device *dev);
 void intel_enable_gt_powersave(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index f5a6bf6..a4cf24f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4599,7 +4599,7 @@ int intel_enable_rc6(const struct drm_device *dev)
 	return i915.enable_rc6;
 }
 
-static void gen6_init_rps_frequencies(struct drm_device *dev)
+void gen6_init_rps_frequencies(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	uint32_t rp_state_cap;
diff --git a/drivers/gpu/drm/i915/intel_slpc.c b/drivers/gpu/drm/i915/intel_slpc.c
index c298a6c..d956ac2 100644
--- a/drivers/gpu/drm/i915/intel_slpc.c
+++ b/drivers/gpu/drm/i915/intel_slpc.c
@@ -95,6 +95,9 @@ int intel_slpc_init(struct drm_device *dev)
 	struct drm_i915_gem_object *obj;
 	int ret = 0;
 
+	/* Initialize the rps frequecny values */
+	gen6_init_rps_frequencies(dev);
+
 	/* Allocate shared data structure */
 	obj = dev_priv->guc.slpc.shared_data_obj;
 	if (!obj) {
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 10/22] drm/i915/slpc: Update current requested frequency

[tom.orourke]

From: Tom O'Rourke <Tom.O'Rourke@intel.com>

When SLPC is controlling requested frequency, the rps.cur_freq
value is not used to make the frequency request.

Before using rps.cur_freq in sysfs or debugfs, read
requested frequency from register to get the value
most recently requested by SLPC firmware.

Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 6 ++++++
 drivers/gpu/drm/i915/i915_sysfs.c   | 2 ++
 2 files changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 0b3550f..b995d4e 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1132,6 +1132,9 @@ static int i915_frequency_info(struct seq_file *m, void *unused)
 
 	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
 
+	if (HAS_SLPC(dev))
+		dev_priv->rps.cur_freq = (I915_READ(GEN6_RPNSWREQ) >> 23);
+
 	if (IS_GEN5(dev)) {
 		u16 rgvswctl = I915_READ16(MEMSWCTL);
 		u16 rgvstat = I915_READ16(MEMSTAT_ILK);
@@ -2355,6 +2358,9 @@ static int i915_rps_boost_info(struct seq_file *m, void *data)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_file *file;
 
+	if (HAS_SLPC(dev))
+		dev_priv->rps.cur_freq = (I915_READ(GEN6_RPNSWREQ) >> 23);
+
 	seq_printf(m, "RPS enabled? %d\n", dev_priv->rps.enabled);
 	seq_printf(m, "GPU busy? %d\n", dev_priv->mm.busy);
 	seq_printf(m, "CPU waiting? %d\n", count_irq_waiters(dev_priv));
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
index c6188dd..f69b949 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -318,6 +318,8 @@ static ssize_t gt_cur_freq_mhz_show(struct device *kdev,
 	intel_runtime_pm_get(dev_priv);
 
 	mutex_lock(&dev_priv->rps.hw_lock);
+	if (HAS_SLPC(dev))
+		dev_priv->rps.cur_freq = (I915_READ(GEN6_RPNSWREQ) >> 23);
 	ret = intel_gpu_freq(dev_priv, dev_priv->rps.cur_freq);
 	mutex_unlock(&dev_priv->rps.hw_lock);
 
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 11/22] drm/i915/slpc: Send reset event

[tom.orourke]

From: Tom O'Rourke <Tom.O'Rourke@intel.com>

Add host2guc SLPC reset event and send reset event
during enable.

Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
---
 drivers/gpu/drm/i915/intel_slpc.c | 30 +++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_slpc.h | 14 ++++++++++++++
 2 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_slpc.c b/drivers/gpu/drm/i915/intel_slpc.c
index d956ac2..59c54a6 100644
--- a/drivers/gpu/drm/i915/intel_slpc.c
+++ b/drivers/gpu/drm/i915/intel_slpc.c
@@ -26,6 +26,30 @@
 #include "i915_drv.h"
 #include "intel_guc.h"
 
+static int host2guc_slpc_reset(struct drm_i915_private *dev_priv)
+{
+	struct drm_i915_gem_object *obj = dev_priv->guc.slpc.shared_data_obj;
+	u32 data[4];
+	int ret;
+	u64 shared_data_gtt_offset = i915_gem_obj_ggtt_offset(obj);
+
+	data[0] = HOST2GUC_ACTION_SLPC_REQUEST;
+	data[1] = SLPC_EVENT(SLPC_EVENT_RESET, 2);
+	data[2] = lower_32_bits(shared_data_gtt_offset);
+	data[3] = upper_32_bits(shared_data_gtt_offset);
+
+	WARN_ON(0 != data[3]);
+
+	ret = host2guc_action(&dev_priv->guc, data, 4);
+
+	if (0 == ret) {
+		ret = I915_READ(SOFT_SCRATCH(1));
+		ret &= 0xFF;
+	}
+
+	return ret;
+}
+
 static u8 slpc_get_platform_sku(struct drm_i915_gem_object *obj)
 {
 	struct drm_device *dev = obj->base.dev;
@@ -148,8 +172,12 @@ int intel_slpc_disable(struct drm_device *dev)
 int intel_slpc_enable(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	int ret = 0;
 
-	return 0;
+	if (dev_priv->guc.slpc.shared_data_obj)
+		ret = host2guc_slpc_reset(dev_priv);
+
+	return ret;
 }
 
 int intel_slpc_reset(struct drm_device *dev)
diff --git a/drivers/gpu/drm/i915/intel_slpc.h b/drivers/gpu/drm/i915/intel_slpc.h
index dceef7e..7287c65 100644
--- a/drivers/gpu/drm/i915/intel_slpc.h
+++ b/drivers/gpu/drm/i915/intel_slpc.h
@@ -28,6 +28,20 @@
 #define SLPC_MINOR_VER 3
 #define SLPC_VERSION ((2015 << 16) | (SLPC_MAJOR_VER << 8) | (SLPC_MINOR_VER))
 
+
+enum slpc_event_id {
+	SLPC_EVENT_RESET = 0,
+	SLPC_EVENT_SHUTDOWN = 1,
+	SLPC_EVENT_PLATFORM_INFO_CHANGE = 2,
+	SLPC_EVENT_DISPLAY_MODE_CHANGE = 3,
+	SLPC_EVENT_FLIP_COMPLETE = 4,
+	SLPC_EVENT_QUERY_TASK_STATE = 5,
+	SLPC_EVENT_PARAMETER_SET = 6,
+	SLPC_EVENT_PARAMETER_UNSET = 7,
+};
+
+#define SLPC_EVENT(id, argc) ((u32) (id) << 8 | (argc))
+
 enum slpc_global_state {
 	SLPC_GLOBAL_STATE_NOT_RUNNING = 0,
 	SLPC_GLOBAL_STATE_INITIALIZING = 1,
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 12/22] drm/i915/slpc: Send shutdown event

[tom.orourke]

From: Tom O'Rourke <Tom.O'Rourke@intel.com>

Send SLPC shutdown event during disable, suspend, and reset
operations.  Sending shutdown event while already shutdown
is OK.

Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
---
 drivers/gpu/drm/i915/intel_slpc.c | 45 ++++++++++++++++++++++++++++++++++++---
 1 file changed, 42 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_slpc.c b/drivers/gpu/drm/i915/intel_slpc.c
index 59c54a6..f155d88 100644
--- a/drivers/gpu/drm/i915/intel_slpc.c
+++ b/drivers/gpu/drm/i915/intel_slpc.c
@@ -50,6 +50,30 @@ static int host2guc_slpc_reset(struct drm_i915_private *dev_priv)
 	return ret;
 }
 
+static int host2guc_slpc_shutdown(struct drm_i915_private *dev_priv)
+{
+	struct drm_i915_gem_object *obj = dev_priv->guc.slpc.shared_data_obj;
+	u32 data[4];
+	int ret;
+	u64 shared_data_gtt_offset = i915_gem_obj_ggtt_offset(obj);
+
+	data[0] = HOST2GUC_ACTION_SLPC_REQUEST;
+	data[1] = SLPC_EVENT(SLPC_EVENT_SHUTDOWN, 2);
+	data[2] = lower_32_bits(shared_data_gtt_offset);
+	data[3] = upper_32_bits(shared_data_gtt_offset);
+
+	WARN_ON(0 != data[3]);
+
+	ret = host2guc_action(&dev_priv->guc, data, 4);
+
+	if (0 == ret) {
+		ret = I915_READ(SOFT_SCRATCH(1));
+		ret &= 0xFF;
+	}
+
+	return ret;
+}
+
 static u8 slpc_get_platform_sku(struct drm_i915_gem_object *obj)
 {
 	struct drm_device *dev = obj->base.dev;
@@ -158,15 +182,23 @@ int intel_slpc_cleanup(struct drm_device *dev)
 int intel_slpc_suspend(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	int ret = 0;
 
-	return 0;
+	if (dev_priv->guc.slpc.shared_data_obj)
+		ret = host2guc_slpc_shutdown(dev_priv);
+
+	return ret;
 }
 
 int intel_slpc_disable(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	int ret = 0;
 
-	return 0;
+	if (dev_priv->guc.slpc.shared_data_obj)
+		ret = host2guc_slpc_shutdown(dev_priv);
+
+	return ret;
 }
 
 int intel_slpc_enable(struct drm_device *dev)
@@ -183,6 +215,13 @@ int intel_slpc_enable(struct drm_device *dev)
 int intel_slpc_reset(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	int ret = 0;
 
-	return 0;
+	if (dev_priv->guc.slpc.shared_data_obj) {
+		ret = host2guc_slpc_shutdown(dev_priv);
+		/* ignore failed shutdown, continue wth reset */
+		ret = host2guc_slpc_reset(dev_priv);
+	}
+
+	return ret;
 }
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 13/22] drm/i915/slpc: Add Display mode event related data structures

[tom.orourke]

From: Sagar Arun Kamble <sagar.a.kamble@intel.com>

Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Acked-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
---
 drivers/gpu/drm/i915/intel_slpc.h | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_slpc.h b/drivers/gpu/drm/i915/intel_slpc.h
index 7287c65..9673a63 100644
--- a/drivers/gpu/drm/i915/intel_slpc.h
+++ b/drivers/gpu/drm/i915/intel_slpc.h
@@ -103,8 +103,46 @@ struct slpc_shared_data {
 	u32 override_parameters_values[SLPC_MAX_OVERRIDE_PARAMETERS];
 } __packed;
 
+#ifndef MAX_INTEL_PIPES
+#define MAX_INTEL_PIPES		3
+#endif
+#ifndef MAX_PLANES_PER_PIPE
+#define MAX_PLANES_PER_PIPE	4
+#endif
+#ifndef MAX_NUM_OF_PIPE
+#define MAX_NUM_OF_PIPE     (MAX_INTEL_PIPES + 1)
+#endif
+
+struct intel_display_pipe_basic_info {
+	union {
+		u32 data;
+		struct {
+			u32 is_widi:1;
+			u32 refresh_rate:7;
+			u32 vsync_ft_usec:24;
+		};
+	};
+}__packed;
+
+struct intel_slpc_display_mode_event_params {
+	struct {
+		struct intel_display_pipe_basic_info per_pipe_info[MAX_NUM_OF_PIPE];
+		union {
+			u32 global_data;
+			struct {
+				u32 active_pipes_bitmask:MAX_NUM_OF_PIPE;
+				u32 fullscreen_pipes:MAX_NUM_OF_PIPE;
+				u32 vbi_sync_on_pipes:MAX_NUM_OF_PIPE;
+				u32 num_active_pipes:2;
+			};
+		};
+	};
+}__packed;
+
+
 struct intel_slpc {
 	struct drm_i915_gem_object *shared_data_obj;
+	struct intel_slpc_display_mode_event_params display_mode_params;
 };
 
 /* intel_slpc.c */
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 14/22] drm/i915/slpc: Notification of Display mode change

[tom.orourke]

From: Sagar Arun Kamble <sagar.a.kamble@intel.com>

GuC SLPC need to be sent data related to Active pipes, refresh rates,
widi pipes, fullscreen pipes related via host to GuC display mode
change event.
This patch defines the event and implements trigger of the event.

Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Acked-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |  2 +
 drivers/gpu/drm/i915/intel_slpc.c    | 92 ++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_slpc.h    |  1 +
 3 files changed, 95 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 06ab6df..7c3d902 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13638,6 +13638,8 @@ static int intel_atomic_commit(struct drm_device *dev,
 	 */
 	intel_uncore_arm_unclaimed_mmio_detection(dev_priv);
 
+	intel_slpc_update_display_mode_info(dev);
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_slpc.c b/drivers/gpu/drm/i915/intel_slpc.c
index f155d88..f5f7cad 100644
--- a/drivers/gpu/drm/i915/intel_slpc.c
+++ b/drivers/gpu/drm/i915/intel_slpc.c
@@ -74,6 +74,27 @@ static int host2guc_slpc_shutdown(struct drm_i915_private *dev_priv)
 	return ret;
 }
 
+static int host2guc_slpc_display_mode_change(struct drm_i915_private *dev_priv)
+{
+        u32 data[7];
+	int ret, i;
+
+        data[0] = HOST2GUC_ACTION_SLPC_REQUEST;
+        data[1] = SLPC_EVENT(SLPC_EVENT_DISPLAY_MODE_CHANGE, MAX_NUM_OF_PIPE + 1);
+	data[2] = dev_priv->guc.slpc.display_mode_params.global_data;
+	for(i = 0; i < MAX_NUM_OF_PIPE; ++i)
+		data[3+i] = dev_priv->guc.slpc.display_mode_params.per_pipe_info[i].data;
+
+        ret = host2guc_action(&dev_priv->guc, data, 7);
+
+	if (0 == ret) {
+		ret = I915_READ(SOFT_SCRATCH(1));
+		ret &= 0xFF;
+	}
+
+	return ret;
+}
+
 static u8 slpc_get_platform_sku(struct drm_i915_gem_object *obj)
 {
 	struct drm_device *dev = obj->base.dev;
@@ -225,3 +246,74 @@ int intel_slpc_reset(struct drm_device *dev)
 
 	return ret;
 }
+
+int intel_slpc_update_display_mode_info(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_crtc *crtc;
+	struct intel_crtc *intel_crtc;
+	struct drm_connector *connector;
+	struct intel_connector *intel_connector;
+	struct drm_encoder *encoder;
+	struct intel_encoder *intel_encoder;
+	struct intel_display_pipe_basic_info *per_pipe_info;
+	struct intel_slpc_display_mode_event_params *cur_params, old_params;
+	bool notify = false;
+
+	if (!HAS_SLPC(dev))
+		return -EINVAL;
+
+	cur_params = &dev_priv->guc.slpc.display_mode_params;
+
+	/* Copy display mode parameters for comparison */
+	old_params.global_data  = cur_params->global_data;
+	cur_params->global_data = 0;
+
+	for_each_intel_crtc(dev, intel_crtc) {
+		per_pipe_info = &cur_params->per_pipe_info[intel_crtc->pipe];
+		old_params.per_pipe_info[intel_crtc->pipe].data = per_pipe_info->data;
+		per_pipe_info->data = 0;
+	}
+
+	for_each_intel_crtc(dev, intel_crtc) {
+		struct intel_crtc_state *pipe_config;
+
+		per_pipe_info = &cur_params->per_pipe_info[intel_crtc->pipe];
+		crtc = &intel_crtc->base;
+		pipe_config = to_intel_crtc_state(crtc->state);
+
+		if (pipe_config->base.active) {
+			for_each_encoder_on_crtc(dev, crtc, intel_encoder) {
+				encoder = &intel_encoder->base;
+				for_each_connector_on_encoder(dev, encoder, intel_connector) {
+					connector = &intel_connector->base;
+					if (connector->status == connector_status_connected) {
+						struct drm_display_mode *mode = &crtc->mode;
+						/* FIXME: Update is_widi based on encoder */
+						per_pipe_info->is_widi = 0;
+						per_pipe_info->refresh_rate = mode->vrefresh;
+						per_pipe_info->vsync_ft_usec = 1000000 / mode->vrefresh;
+						cur_params->active_pipes_bitmask |= (1 << intel_crtc->pipe);
+						cur_params->vbi_sync_on_pipes |= (1 << intel_crtc->pipe);
+						cur_params->num_active_pipes++;
+					}
+				}
+			}
+		}
+	}
+
+	/* Compare old display mode with current mode. Notify SLPC if it is changed. */
+	if (cur_params->global_data != old_params.global_data)
+		notify = true;
+
+	for_each_intel_crtc(dev, intel_crtc) {
+		per_pipe_info = &cur_params->per_pipe_info[intel_crtc->pipe];
+		if (old_params.per_pipe_info[intel_crtc->pipe].data != per_pipe_info->data)
+			notify = true;
+	}
+
+	if (notify)
+		host2guc_slpc_display_mode_change(dev_priv);
+
+	return 0;
+}
diff --git a/drivers/gpu/drm/i915/intel_slpc.h b/drivers/gpu/drm/i915/intel_slpc.h
index 9673a63..18e551b 100644
--- a/drivers/gpu/drm/i915/intel_slpc.h
+++ b/drivers/gpu/drm/i915/intel_slpc.h
@@ -152,5 +152,6 @@ int intel_slpc_suspend(struct drm_device *dev);
 int intel_slpc_disable(struct drm_device *dev);
 int intel_slpc_enable(struct drm_device *dev);
 int intel_slpc_reset(struct drm_device *dev);
+int intel_slpc_update_display_mode_info(struct drm_device *dev);
 
 #endif
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 15/22] drm/i915/slpc: Notification of Refresh Rate change

[tom.orourke]

From: Sagar Arun Kamble <sagar.a.kamble@intel.com>

This patch will inform GuC SLPC about changes in the refresh rate
due to Seamless DRRS. Refresh rate changes due to Static DRRS will
be notified via commit path.

Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Acked-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c   |  2 ++
 drivers/gpu/drm/i915/intel_slpc.c | 22 ++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_slpc.h |  1 +
 3 files changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index e2bea710..5100e8c 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5434,6 +5434,8 @@ static void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate)
 	dev_priv->drrs.refresh_rate_type = index;
 
 	DRM_DEBUG_KMS("eDP Refresh Rate set to : %dHz\n", refresh_rate);
+
+	intel_slpc_update_display_rr_info(dev, refresh_rate);
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/intel_slpc.c b/drivers/gpu/drm/i915/intel_slpc.c
index f5f7cad..c15ab19 100644
--- a/drivers/gpu/drm/i915/intel_slpc.c
+++ b/drivers/gpu/drm/i915/intel_slpc.c
@@ -317,3 +317,25 @@ int intel_slpc_update_display_mode_info(struct drm_device *dev)
 
 	return 0;
 }
+
+int intel_slpc_update_display_rr_info(struct drm_device *dev, u32 refresh_rate)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_crtc *crtc;
+	struct intel_display_pipe_basic_info *per_pipe_info;
+	struct intel_slpc_display_mode_event_params *display_params;
+
+	if (!HAS_SLPC(dev))
+		return -EINVAL;
+
+	display_params = &dev_priv->guc.slpc.display_mode_params;
+	crtc = dp_to_dig_port(dev_priv->drrs.dp)->base.base.crtc;
+
+	per_pipe_info = &display_params->per_pipe_info[to_intel_crtc(crtc)->pipe];
+	per_pipe_info->refresh_rate = refresh_rate;
+	per_pipe_info->vsync_ft_usec = 1000000 / refresh_rate;
+
+	host2guc_slpc_display_mode_change(dev_priv);
+
+	return 0;
+}
diff --git a/drivers/gpu/drm/i915/intel_slpc.h b/drivers/gpu/drm/i915/intel_slpc.h
index 18e551b..bb1d12e 100644
--- a/drivers/gpu/drm/i915/intel_slpc.h
+++ b/drivers/gpu/drm/i915/intel_slpc.h
@@ -153,5 +153,6 @@ int intel_slpc_disable(struct drm_device *dev);
 int intel_slpc_enable(struct drm_device *dev);
 int intel_slpc_reset(struct drm_device *dev);
 int intel_slpc_update_display_mode_info(struct drm_device *dev);
+int intel_slpc_update_display_rr_info(struct drm_device *dev, u32 refresh_rate);
 
 #endif
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 16/22] drm/i915/slpc: Add slpc_status enum values

[tom.orourke]

From: Tom O'Rourke <Tom.O'Rourke@intel.com>

Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
---
 drivers/gpu/drm/i915/intel_slpc.h | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_slpc.h b/drivers/gpu/drm/i915/intel_slpc.h
index bb1d12e..e7ff4a0 100644
--- a/drivers/gpu/drm/i915/intel_slpc.h
+++ b/drivers/gpu/drm/i915/intel_slpc.h
@@ -28,6 +28,32 @@
 #define SLPC_MINOR_VER 3
 #define SLPC_VERSION ((2015 << 16) | (SLPC_MAJOR_VER << 8) | (SLPC_MINOR_VER))
 
+enum slpc_status {
+	SLPC_STATUS_OK = 0,
+	SLPC_STATUS_ERROR = 1,
+	SLPC_STATUS_ILLEGAL_COMMAND = 2,
+	SLPC_STATUS_INVALID_ARGS = 3,
+	SLPC_STATUS_INVALID_PARAMS = 4,
+	SLPC_STATUS_INVALID_DATA = 5,
+	SLPC_STATUS_OUT_OF_RANGE = 6,
+	SLPC_STATUS_NOT_SUPPORTED = 7,
+	SLPC_STATUS_NOT_IMPLEMENTED = 8,
+	SLPC_STATUS_NO_DATA = 9,
+	SLPC_STATUS_EVENT_NOT_REGISTERED = 10,
+	SLPC_STATUS_REGISTER_LOCKED = 11,
+	SLPC_STATUS_TEMPORARILY_UNAVAILABLE = 12,
+	SLPC_STATUS_VALUE_ALREADY_SET = 13,
+	SLPC_STATUS_VALUE_ALREADY_UNSET = 14,
+	SLPC_STATUS_VALUE_NOT_CHANGED = 15,
+	SLPC_STATUS_MISMATCHING_VERSION = 16,
+	SLPC_STATUS_MEMIO_ERROR = 17,
+	SLPC_STATUS_EVENT_QUEUED_REQ_DPC = 18,
+	SLPC_STATUS_EVENT_QUEUED_NOREQ_DPC = 19,
+	SLPC_STATUS_NO_EVENT_QUEUED = 20,
+	SLPC_STATUS_OUT_OF_SPACE = 21,
+	SLPC_STATUS_TIMEOUT = 22,
+	SLPC_STATUS_NO_LOCK =23,
+};
 
 enum slpc_event_id {
 	SLPC_EVENT_RESET = 0,
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 17/22] drm/i915/slpc: Add i915_slpc_info to debugfs

[tom.orourke]

From: Tom O'Rourke <Tom.O'Rourke@intel.com>

i915_slpc_info shows the contents of SLPC shared data
parsed into text format.

Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 161 ++++++++++++++++++++++++++++++++++++
 1 file changed, 161 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index b995d4e..3fc8f83 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1121,6 +1121,166 @@ DEFINE_SIMPLE_ATTRIBUTE(i915_next_seqno_fops,
 			i915_next_seqno_get, i915_next_seqno_set,
 			"0x%llx\n");
 
+static int i915_slpc_info(struct seq_file *m, void *unused)
+{
+	struct drm_info_node *node = m->private;
+	struct drm_device *dev = node->minor->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_i915_gem_object *obj;
+	struct page *page;
+	struct slpc_shared_data *data = NULL;
+	int ret = 0;
+	int i, value;
+	enum slpc_global_state global_state;
+	enum slpc_platform_sku platform_sku;
+	enum slpc_power_plan power_plan;
+	enum slpc_power_source power_source;
+
+	if (HAS_SLPC(dev)) {
+		obj = dev_priv->guc.slpc.shared_data_obj;
+		if (obj) {
+			page = i915_gem_object_get_page(obj, 0);
+			if (page)
+				data = kmap_atomic(page);
+		}
+	}
+
+	if (data) {
+		seq_printf(m, "SLPC Version: %d.%d.%d (0x%8x)\n",
+			   data->slpc_version >> 16,
+			   (data->slpc_version >> 8) & 0xFF,
+			   data->slpc_version & 0xFF,
+			   data->slpc_version);
+		seq_printf(m, "shared data size: %d\n", data->shared_data_size);
+
+		seq_printf(m, "global state: (%d) ", data->global_state);
+		global_state = (enum slpc_global_state) data->global_state;
+		switch (global_state) {
+		case SLPC_GLOBAL_STATE_NOT_RUNNING:
+			seq_puts(m, "not running\n");
+			break;
+		case SLPC_GLOBAL_STATE_INITIALIZING:
+			seq_puts(m, "initializing\n");
+			break;
+		case SLPC_GLOBAL_STATE_RESETING:
+			seq_puts(m, "resetting\n");
+			break;
+		case SLPC_GLOBAL_STATE_RUNNING:
+			seq_puts(m, "running\n");
+			break;
+		case SLPC_GLOBAL_STATE_SHUTTING_DOWN:
+			seq_puts(m, "shutting down\n");
+			break;
+		case SLPC_GLOBAL_STATE_ERROR:
+			seq_puts(m, "error\n");
+			break;
+		default:
+			seq_puts(m, "unknown\n");
+			break;
+		}
+
+		seq_printf(m, "sku: (%d) ", data->platform_info.platform_sku);
+		platform_sku = (enum slpc_platform_sku)
+				data->platform_info.platform_sku;
+		switch (platform_sku) {
+		case SLPC_PLATFORM_SKU_UNDEFINED:
+			seq_puts(m, "undefined\n");
+			break;
+		case SLPC_PLATFORM_SKU_ULX:
+			seq_puts(m, "ULX\n");
+			break;
+		case SLPC_PLATFORM_SKU_ULT:
+			seq_puts(m, "ULT\n");
+			break;
+		case SLPC_PLATFORM_SKU_T:
+			seq_puts(m, "T\n");
+			break;
+		case SLPC_PLATFORM_SKU_MOBL:
+			seq_puts(m, "Mobile\n");
+			break;
+		case SLPC_PLATFORM_SKU_DT:
+			seq_puts(m, "DT\n");
+			break;
+		case SLPC_PLATFORM_SKU_UNKNOWN:
+		default:
+			seq_puts(m, "unknown\n");
+			break;
+		}
+		seq_printf(m, "slice count: %d\n",
+			   data->platform_info.slice_count);
+
+		seq_printf(m, "power plan/source : (0x%x) ",
+			   data->platform_info.power_plan_source);
+		power_plan = (enum slpc_power_plan)
+			      data->platform_info.power_plan_source & 0x3F;
+		power_source = (enum slpc_power_source)
+				data->platform_info.power_plan_source >> 6;
+		switch (power_plan) {
+		case SLPC_POWER_PLAN_UNDEFINED:
+			seq_puts(m, "undefined / ");
+			break;
+		case SLPC_POWER_PLAN_BATTERY_SAVER:
+			seq_puts(m, "battery saver / ");
+			break;
+		case SLPC_POWER_PLAN_BALANCED:
+			seq_puts(m, "balanced / ");
+			break;
+		case SLPC_POWER_PLAN_PERFORMANCE:
+			seq_puts(m, "performance / ");
+			break;
+		case SLPC_POWER_PLAN_UNKNOWN:
+		default:
+			seq_puts(m, "unknown / ");
+			break;
+		}
+		switch (power_source) {
+		case SLPC_POWER_SOURCE_UNDEFINED:
+			seq_puts(m, "undefined\n");
+			break;
+		case SLPC_POWER_SOURCE_AC:
+			seq_puts(m, "AC\n");
+			break;
+		case SLPC_POWER_SOURCE_DC:
+			seq_puts(m, "DC\n");
+			break;
+		case SLPC_POWER_SOURCE_UNKNOWN:
+		default:
+			seq_puts(m, "unknown\n");
+			break;
+		}
+
+		seq_printf(m, "IA frequency (MHz): P0 %d P1 %d Pe %d Pn %d\n",
+			   data->platform_info.P0_freq * 50,
+			   data->platform_info.P1_freq * 50,
+			   data->platform_info.Pe_freq * 50,
+			   data->platform_info.Pn_freq * 50);
+		seq_printf(m, "RAPL package power limits: 0x%8x 0x%8x\n",
+			   data->platform_info.package_rapl_limit_high,
+			   data->platform_info.package_rapl_limit_low);
+		seq_printf(m, "task state data: 0x%8x\n",
+			   data->task_state_data);
+
+		seq_puts(m, "override parameter bitfield\n");
+		for (i=0; i < SLPC_OVERRIDE_BITFIELD_SIZE; i++)
+			seq_printf(m, "%d: 0x%8x\n", i,
+				   data->override_parameters_set_bits[i]);
+
+		seq_puts(m, "override parameters (only non-zero shown)\n");
+		for (i=0; i < SLPC_MAX_OVERRIDE_PARAMETERS; i++) {
+			value = data->override_parameters_values[i];
+			if (value)
+				seq_printf(m, "%d: 0x%8x\n", i, value);
+		}
+
+		kunmap_atomic(data);
+
+	} else {
+		seq_puts(m, "no SLPC info available\n");
+	}
+
+	return ret;
+}
+
 static int i915_frequency_info(struct seq_file *m, void *unused)
 {
 	struct drm_info_node *node = m->private;
@@ -5349,6 +5509,7 @@ static const struct drm_info_list i915_debugfs_list[] = {
 	{"i915_guc_info", i915_guc_info, 0},
 	{"i915_guc_load_status", i915_guc_load_status_info, 0},
 	{"i915_guc_log_dump", i915_guc_log_dump, 0},
+	{"i915_slpc_info", i915_slpc_info, 0},
 	{"i915_frequency_info", i915_frequency_info, 0},
 	{"i915_hangcheck_info", i915_hangcheck_info, 0},
 	{"i915_drpc_info", i915_drpc_info, 0},
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 18/22] drm/i915/slpc: Add dfps task info to i915_slpc_info

[tom.orourke]

From: Tom O'Rourke <Tom.O'Rourke@intel.com>

Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 12 ++++++++++++
 drivers/gpu/drm/i915/intel_slpc.c   | 31 +++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_slpc.h   |  1 +
 3 files changed, 44 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 3fc8f83..b705aff 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1137,6 +1137,12 @@ static int i915_slpc_info(struct seq_file *m, void *unused)
 	enum slpc_power_source power_source;
 
 	if (HAS_SLPC(dev)) {
+		ret = intel_slpc_query_task_state(dev);
+		if (ret) {
+			seq_printf(m, "query task state failed: %d\n", ret);
+			ret = 0;
+		}
+
 		obj = dev_priv->guc.slpc.shared_data_obj;
 		if (obj) {
 			page = i915_gem_object_get_page(obj, 0);
@@ -1259,6 +1265,12 @@ static int i915_slpc_info(struct seq_file *m, void *unused)
 			   data->platform_info.package_rapl_limit_low);
 		seq_printf(m, "task state data: 0x%8x\n",
 			   data->task_state_data);
+		seq_printf(m, "task state dfps: task active %d ",
+			   (data->task_state_data & 1));
+		seq_printf(m, "stall possible %d game mode %d target fps %d\n",
+			   (data->task_state_data & 2),
+			   (data->task_state_data & 4),
+			   (data->task_state_data >> 3) & 0xFF);
 
 		seq_puts(m, "override parameter bitfield\n");
 		for (i=0; i < SLPC_OVERRIDE_BITFIELD_SIZE; i++)
diff --git a/drivers/gpu/drm/i915/intel_slpc.c b/drivers/gpu/drm/i915/intel_slpc.c
index c15ab19..36aedb9 100644
--- a/drivers/gpu/drm/i915/intel_slpc.c
+++ b/drivers/gpu/drm/i915/intel_slpc.c
@@ -95,6 +95,30 @@ static int host2guc_slpc_display_mode_change(struct drm_i915_private *dev_priv)
 	return ret;
 }
 
+static int host2guc_slpc_query_task_state(struct drm_i915_private *dev_priv)
+{
+	struct drm_i915_gem_object *obj = dev_priv->guc.slpc.shared_data_obj;
+        u32 data[4];
+	int ret;
+	u64 shared_data_gtt_offset = i915_gem_obj_ggtt_offset(obj);
+
+        data[0] = HOST2GUC_ACTION_SLPC_REQUEST;
+        data[1] = SLPC_EVENT(SLPC_EVENT_QUERY_TASK_STATE, 2);
+	data[2] = lower_32_bits(shared_data_gtt_offset);
+	data[3] = upper_32_bits(shared_data_gtt_offset);
+
+	WARN_ON(0 != data[3]);
+
+        ret = host2guc_action(&dev_priv->guc, data, 4);
+
+	if (0 == ret) {
+		ret = I915_READ(SOFT_SCRATCH(1));
+		ret &= 0xFF;
+	}
+
+	return ret;
+}
+
 static u8 slpc_get_platform_sku(struct drm_i915_gem_object *obj)
 {
 	struct drm_device *dev = obj->base.dev;
@@ -339,3 +363,10 @@ int intel_slpc_update_display_rr_info(struct drm_device *dev, u32 refresh_rate)
 
 	return 0;
 }
+
+int intel_slpc_query_task_state(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	return host2guc_slpc_query_task_state(dev_priv);
+}
diff --git a/drivers/gpu/drm/i915/intel_slpc.h b/drivers/gpu/drm/i915/intel_slpc.h
index e7ff4a0..32d7dcc 100644
--- a/drivers/gpu/drm/i915/intel_slpc.h
+++ b/drivers/gpu/drm/i915/intel_slpc.h
@@ -181,4 +181,5 @@ int intel_slpc_reset(struct drm_device *dev);
 int intel_slpc_update_display_mode_info(struct drm_device *dev);
 int intel_slpc_update_display_rr_info(struct drm_device *dev, u32 refresh_rate);
 
+int intel_slpc_query_task_state(struct drm_device *dev);
 #endif
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 19/22] drm/i915/slpc: Add parameter unset/set/get functions

[tom.orourke]

From: Tom O'Rourke <Tom.O'Rourke@intel.com>

Add slcp_param_id enum values.
Add events for setting/unsetting parameters.

Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
---
 drivers/gpu/drm/i915/intel_slpc.c | 127 ++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_slpc.h |  22 +++++++
 2 files changed, 149 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_slpc.c b/drivers/gpu/drm/i915/intel_slpc.c
index 36aedb9..3abaf0b 100644
--- a/drivers/gpu/drm/i915/intel_slpc.c
+++ b/drivers/gpu/drm/i915/intel_slpc.c
@@ -119,6 +119,47 @@ static int host2guc_slpc_query_task_state(struct drm_i915_private *dev_priv)
 	return ret;
 }
 
+static int host2guc_slpc_set_param(struct drm_i915_private *dev_priv,
+				   enum slpc_param_id id, u32 value)
+{
+	u32 data[4];
+	int ret;
+
+	data[0] = HOST2GUC_ACTION_SLPC_REQUEST;
+	data[1] = SLPC_EVENT(SLPC_EVENT_PARAMETER_SET, 2);
+	data[2] = (u32) id;
+	data[3] = value;
+
+	ret = host2guc_action(&dev_priv->guc, data, 4);
+
+	if (0 == ret) {
+		ret = I915_READ(SOFT_SCRATCH(1));
+		ret &= 0xFF;
+	}
+
+	return ret;
+}
+
+static int host2guc_slpc_unset_param(struct drm_i915_private *dev_priv,
+				     enum slpc_param_id id)
+{
+	u32 data[3];
+	int ret;
+
+	data[0] = HOST2GUC_ACTION_SLPC_REQUEST;
+	data[1] = SLPC_EVENT(SLPC_EVENT_PARAMETER_UNSET, 1);
+	data[2] = (u32) id;
+
+	ret = host2guc_action(&dev_priv->guc, data, 3);
+
+	if (0 == ret) {
+		ret = I915_READ(SOFT_SCRATCH(1));
+		ret &= 0xFF;
+	}
+
+	return ret;
+}
+
 static u8 slpc_get_platform_sku(struct drm_i915_gem_object *obj)
 {
 	struct drm_device *dev = obj->base.dev;
@@ -370,3 +411,89 @@ int intel_slpc_query_task_state(struct drm_device *dev)
 
 	return host2guc_slpc_query_task_state(dev_priv);
 }
+
+int intel_slpc_unset_param(struct drm_device *dev, enum slpc_param_id id)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_i915_gem_object *obj;
+	struct page *page;
+	struct slpc_shared_data *data = NULL;
+	int ret = 0;
+
+	obj = dev_priv->guc.slpc.shared_data_obj;
+	if (obj) {
+		page = i915_gem_object_get_page(obj, 0);
+		if (page)
+			data = kmap_atomic(page);
+	}
+
+	if (data) {
+		data->override_parameters_set_bits[id >> 5]
+							&= (~(1 << (id % 32)));
+		data->override_parameters_values[id] = 0;
+		kunmap_atomic(data);
+
+		ret = host2guc_slpc_unset_param(dev_priv, id);
+	}
+
+	return ret;
+}
+
+int intel_slpc_set_param(struct drm_device *dev, enum slpc_param_id id,
+                         u32 value)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_i915_gem_object *obj;
+	struct page *page;
+	struct slpc_shared_data *data = NULL;
+	int ret = 0;
+
+	obj = dev_priv->guc.slpc.shared_data_obj;
+	if (obj) {
+		page = i915_gem_object_get_page(obj, 0);
+		if (page)
+			data = kmap_atomic(page);
+	}
+
+	if (data) {
+		data->override_parameters_set_bits[id >> 5]
+							|= (1 << (id % 32));
+		data->override_parameters_values[id] = value;
+		kunmap_atomic(data);
+
+		ret = host2guc_slpc_set_param(dev_priv, id, value);
+	}
+
+	return ret;
+}
+
+int intel_slpc_get_param(struct drm_device *dev, enum slpc_param_id id,
+                         int *overriding, u32 *value)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_i915_gem_object *obj;
+	struct page *page;
+	struct slpc_shared_data *data = NULL;
+	u32 bits;
+	int ret = 0;
+
+	obj = dev_priv->guc.slpc.shared_data_obj;
+	if (obj) {
+		page = i915_gem_object_get_page(obj, 0);
+		if (page)
+			data = kmap_atomic(page);
+	}
+
+	if (data) {
+		if (overriding) {
+			bits = data->override_parameters_set_bits[id >> 5];
+			*overriding = (0 != (bits & (1 << (id % 32))));
+		}
+		if (value)
+			*value = data->override_parameters_values[id];
+
+		kunmap_atomic(data);
+	}
+
+	return ret;
+}
diff --git a/drivers/gpu/drm/i915/intel_slpc.h b/drivers/gpu/drm/i915/intel_slpc.h
index 32d7dcc..fc74ce3 100644
--- a/drivers/gpu/drm/i915/intel_slpc.h
+++ b/drivers/gpu/drm/i915/intel_slpc.h
@@ -68,6 +68,23 @@ enum slpc_event_id {
 
 #define SLPC_EVENT(id, argc) ((u32) (id) << 8 | (argc))
 
+enum slpc_param_id {
+	SLPC_PARAM_TASK_ENABLE_DFPS = 2,
+	SLPC_PARAM_TASK_DISABLE_DFPS = 3,
+	SLPC_PARAM_TASK_ENABLE_TURBO = 4,
+	SLPC_PARAM_TASK_DISABLE_TURBO = 5,
+	SLPC_PARAM_TASK_ENABLE_DCC = 6,
+	SLPC_PARAM_TASK_DISABLE_DCC = 7,
+	SLPC_PARAM_GLOBAL_MIN_GT_FREQ_MHZ = 8,
+	SLPC_PARAM_GLOBAL_MAX_GT_FREQ_MHZ = 9,
+	SLPC_PARAM_DFPS_THRESHOLD_MAX_FPS = 10,
+	SLPC_PARAM_GLOBAL_DISABLE_GT_FREQ_MANAGEMENT = 11,
+	SLPC_PARAM_DFPS_DISABLE_FRAMERATE_STALLING = 12,
+	SLPC_PARAM_GLOBAL_DISABLE_RC6_MODE_CHANGE = 13,
+	SLPC_PARAM_GLOBAL_OC_UNSLICE_FREQ_MHZ = 14,
+	SLPC_PARAM_GLOBAL_OC_SLICE_FREQ_MHZ = 15,
+};
+
 enum slpc_global_state {
 	SLPC_GLOBAL_STATE_NOT_RUNNING = 0,
 	SLPC_GLOBAL_STATE_INITIALIZING = 1,
@@ -182,4 +199,9 @@ int intel_slpc_update_display_mode_info(struct drm_device *dev);
 int intel_slpc_update_display_rr_info(struct drm_device *dev, u32 refresh_rate);
 
 int intel_slpc_query_task_state(struct drm_device *dev);
+int intel_slpc_unset_param(struct drm_device *dev, enum slpc_param_id id);
+int intel_slpc_set_param(struct drm_device *dev, enum slpc_param_id id,
+			 u32 value);
+int intel_slpc_get_param(struct drm_device *dev, enum slpc_param_id id,
+			 int *overriding, u32 *value);
 #endif
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 20/22] drm/i915/slpc: Add slpc support for max/min freq

[tom.orourke]

From: Tom O'Rourke <Tom.O'Rourke@intel.com>

Update sysfs and debugfs functions to set SLPC
parameters when setting max/min frequency.

Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 6 ++++++
 drivers/gpu/drm/i915/i915_sysfs.c   | 8 ++++++++
 2 files changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index b705aff..c26260e 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -5118,6 +5118,9 @@ i915_max_freq_set(void *data, u64 val)
 	}
 
 	dev_priv->rps.max_freq_softlimit = val;
+	if (HAS_SLPC(dev))
+		intel_slpc_set_param(dev, SLPC_PARAM_GLOBAL_MAX_GT_FREQ_MHZ,
+				     (u32) intel_gpu_freq(dev_priv, val));
 
 	intel_set_rps(dev, val);
 
@@ -5185,6 +5188,9 @@ i915_min_freq_set(void *data, u64 val)
 	}
 
 	dev_priv->rps.min_freq_softlimit = val;
+	if (HAS_SLPC(dev))
+		intel_slpc_set_param(dev, SLPC_PARAM_GLOBAL_MIN_GT_FREQ_MHZ,
+				     (u32) intel_gpu_freq(dev_priv, val));
 
 	intel_set_rps(dev, val);
 
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
index f69b949..ed72c74 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -389,6 +389,10 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev,
 
 	dev_priv->rps.max_freq_softlimit = val;
 
+	if (HAS_SLPC(dev))
+		intel_slpc_set_param(dev, SLPC_PARAM_GLOBAL_MAX_GT_FREQ_MHZ,
+				     (u32) intel_gpu_freq(dev_priv, val));
+
 	val = clamp_t(int, dev_priv->rps.cur_freq,
 		      dev_priv->rps.min_freq_softlimit,
 		      dev_priv->rps.max_freq_softlimit);
@@ -448,6 +452,10 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
 
 	dev_priv->rps.min_freq_softlimit = val;
 
+	if (HAS_SLPC(dev))
+		intel_slpc_set_param(dev, SLPC_PARAM_GLOBAL_MIN_GT_FREQ_MHZ,
+				     (u32) intel_gpu_freq(dev_priv, val));
+
 	val = clamp_t(int, dev_priv->rps.cur_freq,
 		      dev_priv->rps.min_freq_softlimit,
 		      dev_priv->rps.max_freq_softlimit);
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 21/22] drm/i915/slpc: Add enable/disable debugfs for slpc

[tom.orourke]

From: Tom O'Rourke <Tom.O'Rourke@intel.com>

Adds debugfs hooks for each slpc task.

The enable/disable debugfs files are
i915_slpc_dfps, i915_slpc_turbo, and i915_slpc_dcc.

Each of these can take the values:
"default", "enabled", or "disabled"

Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 251 ++++++++++++++++++++++++++++++++++++
 1 file changed, 251 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index c26260e..a6171bf 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1293,6 +1293,254 @@ static int i915_slpc_info(struct seq_file *m, void *unused)
 	return ret;
 }
 
+static int slpc_enable_disable_get(struct drm_device *dev, u64 *val,
+			       enum slpc_param_id enable_id,
+			       enum slpc_param_id disable_id)
+{
+	int ret1, ret2;
+	int override_enable, override_disable;
+	u32 value_enable, value_disable;
+
+	if (!HAS_SLPC(dev)) {
+		ret1 = -ENODEV;
+	} else if (val) {
+		ret1 = intel_slpc_get_param(dev, enable_id, &override_enable,
+					    &value_enable);
+		ret2 = intel_slpc_get_param(dev, disable_id, &override_disable,
+					    &value_disable);
+
+		/* set the output value:
+		* 0: default
+		* 1: enabled
+		* 2: disabled
+		* 3: unknown (should not happen)
+		*/
+		if (override_disable && (1 == value_disable))
+			*val = 2;
+		else if (override_enable && (1 == value_enable))
+			*val = 1;
+		else if (!override_enable && !override_disable)
+			*val = 0;
+		else
+			*val = 3;
+
+	} else {
+		ret1 = -EINVAL;
+	}
+
+	return ret1 ? ret1 : ret2;
+}
+
+static int slpc_enable_disable_set(struct drm_device *dev, u64 val,
+			       enum slpc_param_id enable_id,
+			       enum slpc_param_id disable_id)
+{
+	int ret1, ret2;
+
+	if (!HAS_SLPC(dev)) {
+		ret1 = -ENODEV;
+	} else if (0 == val) {
+		/* set default */
+		ret1 = intel_slpc_unset_param(dev, enable_id);
+		ret2 = intel_slpc_unset_param(dev, disable_id);
+	} else if (1 == val) {
+		/* set enable */
+		ret1 = intel_slpc_set_param(dev, enable_id, 1);
+		ret2 = intel_slpc_unset_param(dev, disable_id);
+	} else if (2 == val) {
+		/* set disable */
+		ret2 = intel_slpc_set_param(dev, disable_id, 1);
+		ret1 = intel_slpc_unset_param(dev, enable_id);
+	} else {
+		ret1 = -EINVAL;
+	}
+
+	return ret1 ? ret1 : ret2;
+}
+
+
+static void slpc_param_show(struct seq_file *m, enum slpc_param_id enable_id,
+			    enum slpc_param_id disable_id)
+{
+	struct drm_device *dev = m->private;
+	const char *status;
+	u64 val;
+	int ret;
+
+	ret = slpc_enable_disable_get(dev, &val, enable_id, disable_id);
+
+	if (ret) {
+		seq_printf(m, "error %d\n", ret);
+	} else {
+		switch (val) {
+		case 0:
+			status = "default\n";
+			break;
+
+		case 1:
+			status = "enabled\n";
+			break;
+
+		case 2:
+			status = "disabled\n";
+			break;
+
+		default:
+			status = "unknown\n";
+			break;
+		}
+
+		seq_puts(m, status);
+	}
+}
+
+static int slpc_param_write(struct seq_file *m, const char __user *ubuf,
+			    size_t len, enum slpc_param_id enable_id,
+                            enum slpc_param_id disable_id)
+{
+	struct drm_device *dev = m->private;
+	u64 val;
+	int ret = 0;
+	char buf[10];
+
+	if (len >= sizeof(buf))
+		ret = -EINVAL;
+	else if (copy_from_user(buf, ubuf, len))
+		ret = -EFAULT;
+	else
+		buf[len] = '\0';
+
+	if (!ret) {
+		if (!strncmp(buf, "default", 7))
+			val = 0;
+		else if (!strncmp(buf, "enabled", 7))
+			val = 1;
+		else if (!strncmp(buf, "disabled", 8))
+			val = 2;
+		else
+			ret = -EINVAL;
+	}
+
+	if (!ret)
+		ret = slpc_enable_disable_set(dev, val, enable_id, disable_id);
+
+	return ret;
+}
+
+static int slpc_dfps_show(struct seq_file *m, void *data)
+{
+	slpc_param_show(m, SLPC_PARAM_TASK_ENABLE_DFPS,
+			SLPC_PARAM_TASK_DISABLE_DFPS);
+
+	return 0;
+}
+
+static int slpc_dfps_open(struct inode *inode, struct file *file)
+{
+	struct drm_connector *dev = inode->i_private;
+
+	return single_open(file, slpc_dfps_show, dev);
+}
+
+static ssize_t slpc_dfps_write(struct file *file, const char __user *ubuf,
+			      size_t len, loff_t *offp)
+{
+	struct seq_file *m = file->private_data;
+	int ret = 0;
+
+	ret = slpc_param_write(m, ubuf, len, SLPC_PARAM_TASK_ENABLE_DFPS,
+			       SLPC_PARAM_TASK_DISABLE_DFPS);
+	if (ret)
+		return (size_t) ret;
+
+	return len;
+}
+
+static const struct file_operations i915_slpc_dfps_fops = {
+	.owner	 = THIS_MODULE,
+	.open	 = slpc_dfps_open,
+	.release = single_release,
+	.read	 = seq_read,
+	.write	 = slpc_dfps_write,
+	.llseek	 = seq_lseek
+};
+
+static int slpc_turbo_show(struct seq_file *m, void *data)
+{
+	slpc_param_show(m, SLPC_PARAM_TASK_ENABLE_TURBO,
+			SLPC_PARAM_TASK_DISABLE_TURBO);
+
+	return 0;
+}
+
+static int slpc_turbo_open(struct inode *inode, struct file *file)
+{
+	struct drm_connector *dev = inode->i_private;
+
+	return single_open(file, slpc_turbo_show, dev);
+}
+
+static ssize_t slpc_turbo_write(struct file *file, const char __user *ubuf,
+			      size_t len, loff_t *offp)
+{
+	struct seq_file *m = file->private_data;
+	int ret = 0;
+
+	ret = slpc_param_write(m, ubuf, len, SLPC_PARAM_TASK_ENABLE_TURBO,
+			       SLPC_PARAM_TASK_DISABLE_TURBO);
+	if (ret)
+		return (size_t) ret;
+
+	return len;
+}
+
+static const struct file_operations i915_slpc_turbo_fops = {
+	.owner	 = THIS_MODULE,
+	.open	 = slpc_turbo_open,
+	.release = single_release,
+	.read	 = seq_read,
+	.write	 = slpc_turbo_write,
+	.llseek	 = seq_lseek
+};
+
+static int slpc_dcc_show(struct seq_file *m, void *data)
+{
+	slpc_param_show(m, SLPC_PARAM_TASK_ENABLE_DCC,
+			SLPC_PARAM_TASK_DISABLE_DCC);
+
+	return 0;
+}
+
+static int slpc_dcc_open(struct inode *inode, struct file *file)
+{
+	struct drm_connector *dev = inode->i_private;
+
+	return single_open(file, slpc_dcc_show, dev);
+}
+
+static ssize_t slpc_dcc_write(struct file *file, const char __user *ubuf,
+			      size_t len, loff_t *offp)
+{
+	struct seq_file *m = file->private_data;
+	int ret = 0;
+
+	ret = slpc_param_write(m, ubuf, len, SLPC_PARAM_TASK_ENABLE_DCC,
+			       SLPC_PARAM_TASK_DISABLE_DCC);
+	if (ret)
+		return (size_t) ret;
+
+	return len;
+}
+
+static const struct file_operations i915_slpc_dcc_fops = {
+	.owner	 = THIS_MODULE,
+	.open	 = slpc_dcc_open,
+	.release = single_release,
+	.read	 = seq_read,
+	.write	 = slpc_dcc_write,
+	.llseek	 = seq_lseek
+};
+
 static int i915_frequency_info(struct seq_file *m, void *unused)
 {
 	struct drm_info_node *node = m->private;
@@ -5570,6 +5818,9 @@ static const struct i915_debugfs_files {
 	const struct file_operations *fops;
 } i915_debugfs_files[] = {
 	{"i915_wedged", &i915_wedged_fops},
+	{"i915_slpc_dfps", &i915_slpc_dfps_fops},
+	{"i915_slpc_turbo", &i915_slpc_turbo_fops},
+	{"i915_slpc_dcc", &i915_slpc_dcc_fops},
 	{"i915_max_freq", &i915_max_freq_fops},
 	{"i915_min_freq", &i915_min_freq_fops},
 	{"i915_cache_sharing", &i915_cache_sharing_fops},
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 22/22] drm/i915/slpc: Add has_slpc to skylake info

[tom.orourke]

From: Tom O'Rourke <Tom.O'Rourke@intel.com>

Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 975af35..9e37a98 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -321,6 +321,7 @@ static const struct intel_device_info intel_skylake_info = {
 	HSW_FEATURES,
 	.is_skylake = 1,
 	.gen = 9,
+	.has_slpc = 1,
 };
 
 static const struct intel_device_info intel_skylake_gt3_info = {
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 14/22] drm/i915/slpc: Notification of Display mode change

[Zanoni, Paulo R]

Em Qua, 2016-01-20 às 18:26 -0800, tom.orourke@intel.com escreveu:
> From: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> 
> GuC SLPC need to be sent data related to Active pipes, refresh rates,
> widi pipes, fullscreen pipes related via host to GuC display mode
> change event.
> This patch defines the event and implements trigger of the event.
> 
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Acked-by: Tom O'Rourke <Tom.O'Rourke@intel.com>

(this is just a comment from a quick look at the file, not a full
review)

> ---
>  drivers/gpu/drm/i915/intel_display.c |  2 +
>  drivers/gpu/drm/i915/intel_slpc.c    | 92
> ++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_slpc.h    |  1 +
>  3 files changed, 95 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 06ab6df..7c3d902 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13638,6 +13638,8 @@ static int intel_atomic_commit(struct
> drm_device *dev,
>  	 */
>  	intel_uncore_arm_unclaimed_mmio_detection(dev_priv);
>  
> +	intel_slpc_update_display_mode_info(dev);
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_slpc.c
> b/drivers/gpu/drm/i915/intel_slpc.c
> index f155d88..f5f7cad 100644
> --- a/drivers/gpu/drm/i915/intel_slpc.c
> +++ b/drivers/gpu/drm/i915/intel_slpc.c
> @@ -74,6 +74,27 @@ static int host2guc_slpc_shutdown(struct
> drm_i915_private *dev_priv)
>  	return ret;
>  }
>  
> +static int host2guc_slpc_display_mode_change(struct drm_i915_private
> *dev_priv)
> +{
> +        u32 data[7];
> +	int ret, i;
> +
> +        data[0] = HOST2GUC_ACTION_SLPC_REQUEST;
> +        data[1] = SLPC_EVENT(SLPC_EVENT_DISPLAY_MODE_CHANGE,
> MAX_NUM_OF_PIPE + 1);
> +	data[2] = dev_priv-
> >guc.slpc.display_mode_params.global_data;
> +	for(i = 0; i < MAX_NUM_OF_PIPE; ++i)
> +		data[3+i] = dev_priv-
> >guc.slpc.display_mode_params.per_pipe_info[i].data;
> +
> +        ret = host2guc_action(&dev_priv->guc, data, 7);
> +
> +	if (0 == ret) {

https://en.wikipedia.org/wiki/Yoda_conditions are not part of our usual
coding style.

> +		ret = I915_READ(SOFT_SCRATCH(1));
> +		ret &= 0xFF;
> +	}
> +
> +	return ret;
> +}

There are some tab/space issues in the function above.

Just a suggestion, not a request: for both functions you introduced,
since the only callers do not bother to check the return values, you
could just make the functions return void. Having unchecked return
values may give a false sense that errors would be caught by the upper
layer, when in fact they aren't. In a lot of cases it's better to just
print some error message instead of returning some value that is going
to be ignored.

> +
>  static u8 slpc_get_platform_sku(struct drm_i915_gem_object *obj)
>  {
>  	struct drm_device *dev = obj->base.dev;
> @@ -225,3 +246,74 @@ int intel_slpc_reset(struct drm_device *dev)
>  
>  	return ret;
>  }
> +
> +int intel_slpc_update_display_mode_info(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct drm_crtc *crtc;
> +	struct intel_crtc *intel_crtc;
> +	struct drm_connector *connector;
> +	struct intel_connector *intel_connector;
> +	struct drm_encoder *encoder;
> +	struct intel_encoder *intel_encoder;
> +	struct intel_display_pipe_basic_info *per_pipe_info;
> +	struct intel_slpc_display_mode_event_params *cur_params,
> old_params;
> +	bool notify = false;
> +
> +	if (!HAS_SLPC(dev))
> +		return -EINVAL;
> +
> +	cur_params = &dev_priv->guc.slpc.display_mode_params;
> +
> +	/* Copy display mode parameters for comparison */
> +	old_params.global_data  = cur_params->global_data;
> +	cur_params->global_data = 0;
> +
> +	for_each_intel_crtc(dev, intel_crtc) {
> +		per_pipe_info = &cur_params-
> >per_pipe_info[intel_crtc->pipe];
> +		old_params.per_pipe_info[intel_crtc->pipe].data =
> per_pipe_info->data;
> +		per_pipe_info->data = 0;
> +	}
> +
> +	for_each_intel_crtc(dev, intel_crtc) {
> +		struct intel_crtc_state *pipe_config;
> +
> +		per_pipe_info = &cur_params-
> >per_pipe_info[intel_crtc->pipe];
> +		crtc = &intel_crtc->base;
> +		pipe_config = to_intel_crtc_state(crtc->state);
> +
> +		if (pipe_config->base.active) {

As far as I understand from the new atomic locking model, since this
code is called from intel_atomic_commit, it can't just iterate through
every crtc/encoder/connector checking state structures. During an
atomic commit we can only look at the objects affected by the commit,
so we have to use things such as for_each_crtc_in_state().

It seems to me that the model here could be:
- At driver load, during or after HW state readout, initialize some
state structure to make it match the hardware.
- Whenever there's an atomic commit, just look+update the status of the
affected crtc/encoder/connector, leaving everything else the same

> +			for_each_encoder_on_crtc(dev, crtc,
> intel_encoder) {
> +				encoder = &intel_encoder->base;
> +				for_each_connector_on_encoder(dev,
> encoder, intel_connector) {
> +					connector =
> &intel_connector->base;
> +					if (connector->status ==
> connector_status_connected) {
> +						struct
> drm_display_mode *mode = &crtc->mode;
> +						/* FIXME: Update
> is_widi based on encoder */
> +						per_pipe_info-
> >is_widi = 0;
> +						per_pipe_info-
> >refresh_rate = mode->vrefresh;
> +						per_pipe_info-
> >vsync_ft_usec = 1000000 / mode->vrefresh;
> +						cur_params-
> >active_pipes_bitmask |= (1 << intel_crtc->pipe);
> +						cur_params-
> >vbi_sync_on_pipes |= (1 << intel_crtc->pipe);
> +						cur_params-
> >num_active_pipes++;
> +					}
> +				}
> +			}
> +		}
> +	}
> +
> +	/* Compare old display mode with current mode. Notify SLPC
> if it is changed. */
> +	if (cur_params->global_data != old_params.global_data)
> +		notify = true;
> +
> +	for_each_intel_crtc(dev, intel_crtc) {
> +		per_pipe_info = &cur_params-
> >per_pipe_info[intel_crtc->pipe];
> +		if (old_params.per_pipe_info[intel_crtc->pipe].data
> != per_pipe_info->data)
> +			notify = true;
> +	}
> +
> +	if (notify)
> +		host2guc_slpc_display_mode_change(dev_priv);
> +
> +	return 0;
> +}
> diff --git a/drivers/gpu/drm/i915/intel_slpc.h
> b/drivers/gpu/drm/i915/intel_slpc.h
> index 9673a63..18e551b 100644
> --- a/drivers/gpu/drm/i915/intel_slpc.h
> +++ b/drivers/gpu/drm/i915/intel_slpc.h
> @@ -152,5 +152,6 @@ int intel_slpc_suspend(struct drm_device *dev);
>  int intel_slpc_disable(struct drm_device *dev);
>  int intel_slpc_enable(struct drm_device *dev);
>  int intel_slpc_reset(struct drm_device *dev);
> +int intel_slpc_update_display_mode_info(struct drm_device *dev);
>  
>  #endif
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for Add support for GuC-based SLPC

[Patchwork]

== Summary ==

Built on 8fe9e785ae04fa7c37f7935cff12d62e38054b60 drm-intel-nightly: 2016y-01m-21d-11h-02m-42s UTC integration manifest

Test drv_getparams_basic:
        Subgroup basic-eu-total:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup basic-subslice-total:
                pass       -> DMESG-FAIL (skl-i5k-2)
Test drv_hangman:
        Subgroup error-state-basic:
                pass       -> DMESG-FAIL (skl-i5k-2)
Test drv_module_reload_basic:
                pass       -> DMESG-FAIL (skl-i5k-2)
Test gem_basic:
        Subgroup bad-close:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup create-close:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup create-fd-close:
                pass       -> DMESG-FAIL (skl-i5k-2)
Test gem_cpu_reloc:
        Subgroup basic:
                pass       -> DMESG-FAIL (skl-i5k-2)
Test gem_ctx_basic:
                pass       -> DMESG-FAIL (skl-i5k-2)
Test gem_ctx_create:
        Subgroup basic:
                pass       -> DMESG-FAIL (skl-i5k-2)
Test gem_ctx_exec:
        Subgroup basic:
                pass       -> DMESG-FAIL (skl-i5k-2)
Test gem_ctx_param_basic:
        Subgroup basic:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup basic-default:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup invalid-ctx-get:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup invalid-ctx-set:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup invalid-param-get:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup invalid-param-set:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup invalid-size-get:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup invalid-size-set:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup non-root-set:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup non-root-set-no-zeromap:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup root-set:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup root-set-no-zeromap-disabled:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup root-set-no-zeromap-enabled:
                pass       -> DMESG-FAIL (skl-i5k-2)
Test gem_exec_parse:
        Subgroup basic-allowed:
                skip       -> DMESG-FAIL (skl-i5k-2)
        Subgroup basic-rejected:
                skip       -> DMESG-FAIL (skl-i5k-2)
Test gem_flink_basic:
        Subgroup bad-flink:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup bad-open:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup basic:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup double-flink:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup flink-lifetime:
                pass       -> DMESG-FAIL (skl-i5k-2)
Test gem_linear_blits:
        Subgroup basic:
                pass       -> DMESG-FAIL (skl-i5k-2)
Test gem_mmap:
        Subgroup basic:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup basic-small-bo:
                pass       -> DMESG-FAIL (skl-i5k-2)
Test gem_mmap_gtt:
        Subgroup basic:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup basic-copy:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup basic-read:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup basic-read-no-prefault:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup basic-read-write:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup basic-read-write-distinct:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup basic-short:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup basic-small-bo:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup basic-small-bo-tiledx:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup basic-small-bo-tiledy:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup basic-small-copy:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup basic-small-copy-xy:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup basic-write:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup basic-write-cpu-read-gtt:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup basic-write-gtt:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup basic-write-gtt-no-prefault:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup basic-write-no-prefault:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup basic-write-read:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup basic-write-read-distinct:
                pass       -> DMESG-FAIL (skl-i5k-2)
Test gem_pread:
        Subgroup basic:
                pass       -> DMESG-FAIL (skl-i5k-2)
Test gem_pwrite:
        Subgroup basic:
                pass       -> DMESG-FAIL (skl-i5k-2)
Test gem_render_linear_blits:
        Subgroup basic:
                pass       -> DMESG-FAIL (skl-i5k-2)
Test gem_render_tiled_blits:
        Subgroup basic:
                pass       -> DMESG-FAIL (skl-i5k-2)
Test gem_storedw_loop:
        Subgroup basic-blt:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup basic-bsd:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup basic-render:
                pass       -> DMESG-FAIL (skl-i5k-2) UNSTABLE
        Subgroup basic-vebox:
                pass       -> DMESG-FAIL (skl-i5k-2)
Test gem_sync:
        Subgroup basic-blt:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup basic-render:
                pass       -> DMESG-FAIL (skl-i5k-2) UNSTABLE
Test gem_tiled_blits:
        Subgroup basic:
                pass       -> DMESG-FAIL (skl-i5k-2)
Test gem_tiled_fence_blits:
        Subgroup basic:
                pass       -> DMESG-FAIL (skl-i5k-2)
Test gem_tiled_pread_basic:
                pass       -> DMESG-FAIL (skl-i5k-2)
Test kms_addfb_basic:
        Subgroup addfb25-bad-modifier:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup addfb25-framebuffer-vs-set-tiling:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup addfb25-modifier-no-flag:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup addfb25-x-tiled:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup addfb25-x-tiled-mismatch:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup addfb25-y-tiled:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup addfb25-y-tiled-small:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup addfb25-yf-tiled:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup bad-pitch-0:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup bad-pitch-1024:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup bad-pitch-128:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup bad-pitch-256:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup bad-pitch-32:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup bad-pitch-63:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup bad-pitch-65536:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup bad-pitch-999:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup basic:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup basic-x-tiled:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup basic-y-tiled:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup bo-too-small:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup bo-too-small-due-to-tiling:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup clobberred-modifier:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup framebuffer-vs-set-tiling:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup no-handle:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup size-max:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup small-bo:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup tile-pitch-mismatch:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup too-high:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup too-wide:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup unused-handle:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup unused-modifier:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup unused-offsets:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup unused-pitches:
                pass       -> DMESG-FAIL (skl-i5k-2)
Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                pass       -> DMESG-FAIL (skl-i5k-2)
                pass       -> DMESG-WARN (ilk-hp8440p)
        Subgroup basic-flip-vs-modeset:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup basic-flip-vs-wf_vblank:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup basic-plain-flip:
                pass       -> DMESG-FAIL (skl-i5k-2)
Test kms_force_connector_basic:
        Subgroup force-connector-state:
                skip       -> DMESG-FAIL (skl-i5k-2)
        Subgroup force-edid:
                skip       -> DMESG-FAIL (skl-i5k-2)
        Subgroup prune-stale-modes:
                skip       -> DMESG-FAIL (skl-i5k-2)
Test kms_pipe_crc_basic:
        Subgroup bad-nb-words-1:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup bad-nb-words-3:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup bad-pipe:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup bad-source:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup hang-read-crc-pipe-a:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup hang-read-crc-pipe-b:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup hang-read-crc-pipe-c:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup nonblocking-crc-pipe-a:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup nonblocking-crc-pipe-a-frame-sequence:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup nonblocking-crc-pipe-b:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup nonblocking-crc-pipe-b-frame-sequence:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup nonblocking-crc-pipe-c:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup nonblocking-crc-pipe-c-frame-sequence:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup read-crc-pipe-a:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup read-crc-pipe-a-frame-sequence:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup read-crc-pipe-b:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup read-crc-pipe-b-frame-sequence:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup read-crc-pipe-c:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup read-crc-pipe-c-frame-sequence:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup suspend-read-crc-pipe-a:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup suspend-read-crc-pipe-b:
                dmesg-warn -> DMESG-FAIL (skl-i5k-2)
        Subgroup suspend-read-crc-pipe-c:
                pass       -> DMESG-FAIL (skl-i5k-2)
Test kms_psr_sink_crc:
        Subgroup psr_basic:
                skip       -> DMESG-FAIL (skl-i5k-2)
Test kms_setmode:
        Subgroup basic-clone-single-crtc:
                pass       -> DMESG-FAIL (skl-i5k-2)
Test kms_sink_crc_basic:
                skip       -> DMESG-FAIL (skl-i5k-2)
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup basic-rte:
                pass       -> DMESG-FAIL (skl-i5k-2)
Test pm_rps:
        Subgroup basic-api:
                pass       -> DMESG-FAIL (skl-i5k-2)
Test prime_self_import:
        Subgroup basic-llseek-bad:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup basic-llseek-size:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup basic-with_fd_dup:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup basic-with_one_bo:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup basic-with_one_bo_two_files:
                pass       -> DMESG-FAIL (skl-i5k-2)
        Subgroup basic-with_two_bos:
                pass       -> DMESG-FAIL (skl-i5k-2)

bdw-nuci7        total:140  pass:131  dwarn:0   dfail:0   fail:0   skip:9  
bdw-ultra        total:143  pass:137  dwarn:0   dfail:0   fail:0   skip:6  
bsw-nuc-2        total:143  pass:119  dwarn:0   dfail:0   fail:0   skip:24 
byt-nuc          total:143  pass:128  dwarn:0   dfail:0   fail:0   skip:15 
hsw-brixbox      total:143  pass:136  dwarn:0   dfail:0   fail:0   skip:7  
hsw-gt2          total:143  pass:139  dwarn:0   dfail:0   fail:0   skip:4  
ilk-hp8440p      total:143  pass:103  dwarn:2   dfail:0   fail:0   skip:38 
ivb-t430s        total:143  pass:137  dwarn:0   dfail:0   fail:0   skip:6  
skl-i5k-2        total:143  pass:2    dwarn:0   dfail:140 fail:0   skip:1  
snb-dellxps      total:143  pass:129  dwarn:0   dfail:0   fail:0   skip:14 
snb-x220t        total:143  pass:129  dwarn:0   dfail:0   fail:1   skip:13 

Results at /archive/results/CI_IGT_test/Patchwork_1236/

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✗ Fi.CI.BAT: failure for Add support for GuC-based SLPC

[O'Rourke, Tom]

On Thu, Jan 21, 2016 at 01:50:34PM +0000, Patchwork wrote:
> == Summary ==
> 
> Built on 8fe9e785ae04fa7c37f7935cff12d62e38054b60 drm-intel-nightly: 2016y-01m-21d-11h-02m-42s UTC integration manifest
> 
> Test drv_getparams_basic:
>         Subgroup basic-eu-total:
>                 pass       -> DMESG-FAIL (skl-i5k-2)
>         Subgroup basic-subslice-total:
>                 pass       -> DMESG-FAIL (skl-i5k-2)
[TOR:] ...snip many failures on skl-i5k-2...

The results show an error in dmesg:
"[drm:intel_lr_context_deferred_alloc [i915]] *ERROR* ring create req: -5"

This error happens when attempting to use guc submission without guc
firmware available on system.  Skylake guc firmware is available for download
at https://01.org/linuxgraphics/downloads/sklgucver43 and is also at
git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git

How can we get CI test machines updated to use latest published firmware?

Thanks,
Tom

> 
> bdw-nuci7        total:140  pass:131  dwarn:0   dfail:0   fail:0   skip:9  
> bdw-ultra        total:143  pass:137  dwarn:0   dfail:0   fail:0   skip:6  
> bsw-nuc-2        total:143  pass:119  dwarn:0   dfail:0   fail:0   skip:24 
> byt-nuc          total:143  pass:128  dwarn:0   dfail:0   fail:0   skip:15 
> hsw-brixbox      total:143  pass:136  dwarn:0   dfail:0   fail:0   skip:7  
> hsw-gt2          total:143  pass:139  dwarn:0   dfail:0   fail:0   skip:4  
> ilk-hp8440p      total:143  pass:103  dwarn:2   dfail:0   fail:0   skip:38 
> ivb-t430s        total:143  pass:137  dwarn:0   dfail:0   fail:0   skip:6  
> skl-i5k-2        total:143  pass:2    dwarn:0   dfail:140 fail:0   skip:1  
> snb-dellxps      total:143  pass:129  dwarn:0   dfail:0   fail:0   skip:14 
> snb-x220t        total:143  pass:129  dwarn:0   dfail:0   fail:1   skip:13 
> 
> Results at /archive/results/CI_IGT_test/Patchwork_1236/
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 06/22] drm/i915/slpc: If using SLPC, do not set frequency

[Daniel Vetter]

On Wed, Jan 20, 2016 at 06:26:08PM -0800, tom.orourke@intel.com wrote:
> From: Tom O'Rourke <Tom.O'Rourke@intel.com>
> 
> When frequency requests are made by SLPC, host driver
> should not attempt to make frequency requests due to
> potential conflicts.
> 
> Host-based turbo operations are already avoided when
> SLPC is used.  This change covers other frequency
> requests such as from sysfs or debugfs interfaces.
> 
> Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>

Does this break the sysfs interface, i.e. can userspace still set limits
when SLPC is in use?
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index ec1868b..f5a6bf6 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4501,6 +4501,9 @@ void gen6_rps_boost(struct drm_i915_private *dev_priv,
>  
>  void intel_set_rps(struct drm_device *dev, u8 val)
>  {
> +	if (HAS_SLPC(dev))
> +		return;
> +
>  	if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev))
>  		valleyview_set_rps(dev, val);
>  	else
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 00/22] Add support for GuC-based SLPC

[Daniel Vetter]

On Wed, Jan 20, 2016 at 06:26:02PM -0800, tom.orourke@intel.com wrote:
> From: Tom O'Rourke <Tom.O'Rourke@intel.com>
> 
> SLPC (Single Loop Power Controller) is a replacement for
> some host-based power management features.  The SLPC
> implemenation runs in firmware on GuC.
> 
> This series is a first request for comments.  This series
> is not expected to be merged.  After changes based on
> comments, a later patch series will be sent for merging.
>  
> This series has been tested with SKL guc firmware
> versions 4.3 and 4.7.  The graphics power management
> features in SLPC in those versions are DFPS (Dynamic FPS),
> Turbo, and DCC (Duty Cycle Control).  DFPS adjusts
> requested graphics frequency to maintain target framerate.
> Turbo adjusts requested graphics frequency to maintain
> target GT busyness.  DCC adjusts requested graphics
> frequency and stalls guc-scheduler to maintain actual
> graphics frequency in efficient range.

Either it's been forever long ago or I missed that meeting, so I'll drop
my big arch concerns here. We probably need to discuss this internally, at
least the benchmark data. Two big items:

- How does GuC measure fps rendered to the screen? More specifically, how
  does it figure out that we missed a frame and kick the throttle up?

- This patch series seems to remove the limiting abilities, and also
  completely no-ops out our boost/deboost features. Can we recover these
  features?

We need at least make benchmarks of spike-y workloads (for the missed
frame boosting/deboosting) and for workloads that stall too often to make
sure slpc firmware doesn't suck. Chris has done quite a bit of work in
this area since at least the default behaviour is pretty bad for both
interactivity and for some throughput workloads. We need to benchmark
this, and if slpc falls short either scream at firmware folks to improve
things, or figure out how we can override slpc decisions to recover
performance that might have been lost.

Chris has more details, but the two workloads where iirc current default
rps totally fails are:
- totally idle system, then short burts of 60fps rendering for gui
  transition, then again totally idle.
- libva and iirc also some opencl workloads that stall for the gpu awfully
  often. If you're unlucky cpu ramps down due to stalls, which means next
  workload takes longer to prep, which means gpu ramps down due to
  idleness, repeat until crawling.
-Daniel

> 
> Patch 1/22 is included ihere for convenience and should be
> part of an earlier series.  SLPC assumes guc firmware has
> been loaded and GuC submission is enabled.
> 
> Patch 22/22 sets the flag to enable SLPC on SKL.  Without
> this patch, the previous patches should have no effect.
> 
> VIZ-6773, VIZ-6889
> 
> Dave Gordon (1):
>   drm/i915: Enable GuC submission, where supported
> 
> Sagar Arun Kamble (4):
>   drm/i915/slpc: Enable/Disable RC6 in SLPC flows
>   drm/i915/slpc: Add Display mode event related data structures
>   drm/i915/slpc: Notification of Display mode change
>   drm/i915/slpc: Notification of Refresh Rate change
> 
> Tom O'Rourke (17):
>   drm/i915/slpc: Add has_slpc capability flag
>   drm/i915/slpc: Expose guc functions for use with SLPC
>   drm/i915/slpc: Use intel_slpc_* functions if supported
>   drm/i915/slpc: If using SLPC, do not set frequency
>   drm/i915/slpc: Enable SLPC in guc if supported
>   drm/i915/slpc: Allocate/Release/Initialize SLPC shared data
>   drm/i915/slpc: Setup rps frequency values during SLPC init
>   drm/i915/slpc: Update current requested frequency
>   drm/i915/slpc: Send reset event
>   drm/i915/slpc: Send shutdown event
>   drm/i915/slpc: Add slpc_status enum values
>   drm/i915/slpc: Add i915_slpc_info to debugfs
>   drm/i915/slpc: Add dfps task info to i915_slpc_info
>   drm/i915/slpc: Add parameter unset/set/get functions
>   drm/i915/slpc: Add slpc support for max/min freq
>   drm/i915/slpc: Add enable/disable debugfs for slpc
>   drm/i915/slpc: Add has_slpc to skylake info
> 
>  drivers/gpu/drm/i915/Makefile              |   5 +-
>  drivers/gpu/drm/i915/i915_debugfs.c        | 436 +++++++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_drv.c            |   1 +
>  drivers/gpu/drm/i915/i915_drv.h            |   2 +
>  drivers/gpu/drm/i915/i915_guc_submission.c |   6 +-
>  drivers/gpu/drm/i915/i915_params.c         |   4 +-
>  drivers/gpu/drm/i915/i915_sysfs.c          |  10 +
>  drivers/gpu/drm/i915/intel_display.c       |   2 +
>  drivers/gpu/drm/i915/intel_dp.c            |   2 +
>  drivers/gpu/drm/i915/intel_drv.h           |   1 +
>  drivers/gpu/drm/i915/intel_guc.h           |   7 +
>  drivers/gpu/drm/i915/intel_guc_loader.c    |   3 +
>  drivers/gpu/drm/i915/intel_pm.c            |  43 ++-
>  drivers/gpu/drm/i915/intel_slpc.c          | 499 +++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_slpc.h          | 207 ++++++++++++
>  15 files changed, 1210 insertions(+), 18 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/intel_slpc.c
>  create mode 100644 drivers/gpu/drm/i915/intel_slpc.h
> 
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✗ Fi.CI.BAT: failure for Add support for GuC-based SLPC

[Daniel Vetter]

On Thu, Jan 21, 2016 at 03:16:42PM -0800, O'Rourke, Tom wrote:
> On Thu, Jan 21, 2016 at 01:50:34PM +0000, Patchwork wrote:
> > == Summary ==
> > 
> > Built on 8fe9e785ae04fa7c37f7935cff12d62e38054b60 drm-intel-nightly: 2016y-01m-21d-11h-02m-42s UTC integration manifest
> > 
> > Test drv_getparams_basic:
> >         Subgroup basic-eu-total:
> >                 pass       -> DMESG-FAIL (skl-i5k-2)
> >         Subgroup basic-subslice-total:
> >                 pass       -> DMESG-FAIL (skl-i5k-2)
> [TOR:] ...snip many failures on skl-i5k-2...
> 
> The results show an error in dmesg:
> "[drm:intel_lr_context_deferred_alloc [i915]] *ERROR* ring create req: -5"
> 
> This error happens when attempting to use guc submission without guc
> firmware available on system.  Skylake guc firmware is available for download
> at https://01.org/linuxgraphics/downloads/sklgucver43 and is also at
> git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git
> 
> How can we get CI test machines updated to use latest published firmware?

We started a new mailing lists for all things CI:
intel-gfx-ci@eclists.intel.com Please subscribe there and raise CI
comments/suggestions/questions on that list.

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 14/22] drm/i915/slpc: Notification of Display mode change

[Ville Syrjälä]

On Wed, Jan 20, 2016 at 06:26:16PM -0800, tom.orourke@intel.com wrote:
> From: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> 
> GuC SLPC need to be sent data related to Active pipes, refresh rates,
> widi pipes, fullscreen pipes related via host to GuC display mode
> change event.
> This patch defines the event and implements trigger of the event.
> 
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Acked-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c |  2 +
>  drivers/gpu/drm/i915/intel_slpc.c    | 92 ++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_slpc.h    |  1 +
>  3 files changed, 95 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 06ab6df..7c3d902 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13638,6 +13638,8 @@ static int intel_atomic_commit(struct drm_device *dev,
>  	 */
>  	intel_uncore_arm_unclaimed_mmio_detection(dev_priv);
>  
> +	intel_slpc_update_display_mode_info(dev);
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_slpc.c b/drivers/gpu/drm/i915/intel_slpc.c
> index f155d88..f5f7cad 100644
> --- a/drivers/gpu/drm/i915/intel_slpc.c
> +++ b/drivers/gpu/drm/i915/intel_slpc.c
> @@ -74,6 +74,27 @@ static int host2guc_slpc_shutdown(struct drm_i915_private *dev_priv)
>  	return ret;
>  }
>  
> +static int host2guc_slpc_display_mode_change(struct drm_i915_private *dev_priv)
> +{
> +        u32 data[7];
> +	int ret, i;
> +
> +        data[0] = HOST2GUC_ACTION_SLPC_REQUEST;
> +        data[1] = SLPC_EVENT(SLPC_EVENT_DISPLAY_MODE_CHANGE, MAX_NUM_OF_PIPE + 1);
> +	data[2] = dev_priv->guc.slpc.display_mode_params.global_data;
> +	for(i = 0; i < MAX_NUM_OF_PIPE; ++i)
> +		data[3+i] = dev_priv->guc.slpc.display_mode_params.per_pipe_info[i].data;
> +
> +        ret = host2guc_action(&dev_priv->guc, data, 7);
> +
> +	if (0 == ret) {
> +		ret = I915_READ(SOFT_SCRATCH(1));
> +		ret &= 0xFF;
> +	}
> +
> +	return ret;
> +}
> +
>  static u8 slpc_get_platform_sku(struct drm_i915_gem_object *obj)
>  {
>  	struct drm_device *dev = obj->base.dev;
> @@ -225,3 +246,74 @@ int intel_slpc_reset(struct drm_device *dev)
>  
>  	return ret;
>  }
> +
> +int intel_slpc_update_display_mode_info(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct drm_crtc *crtc;
> +	struct intel_crtc *intel_crtc;
> +	struct drm_connector *connector;
> +	struct intel_connector *intel_connector;
> +	struct drm_encoder *encoder;
> +	struct intel_encoder *intel_encoder;
> +	struct intel_display_pipe_basic_info *per_pipe_info;
> +	struct intel_slpc_display_mode_event_params *cur_params, old_params;
> +	bool notify = false;
> +
> +	if (!HAS_SLPC(dev))
> +		return -EINVAL;
> +
> +	cur_params = &dev_priv->guc.slpc.display_mode_params;
> +
> +	/* Copy display mode parameters for comparison */
> +	old_params.global_data  = cur_params->global_data;
> +	cur_params->global_data = 0;
> +
> +	for_each_intel_crtc(dev, intel_crtc) {
> +		per_pipe_info = &cur_params->per_pipe_info[intel_crtc->pipe];
> +		old_params.per_pipe_info[intel_crtc->pipe].data = per_pipe_info->data;
> +		per_pipe_info->data = 0;
> +	}
> +
> +	for_each_intel_crtc(dev, intel_crtc) {
> +		struct intel_crtc_state *pipe_config;
> +
> +		per_pipe_info = &cur_params->per_pipe_info[intel_crtc->pipe];
> +		crtc = &intel_crtc->base;
> +		pipe_config = to_intel_crtc_state(crtc->state);
> +
> +		if (pipe_config->base.active) {
> +			for_each_encoder_on_crtc(dev, crtc, intel_encoder) {
> +				encoder = &intel_encoder->base;
> +				for_each_connector_on_encoder(dev, encoder, intel_connector) {
> +					connector = &intel_connector->base;
> +					if (connector->status == connector_status_connected) {

The connecotr/encoder stuff should not be here. We can enable a pipe
without any connected connectors. Also locking is fail.

The patch also fails to explain why the guc needs to know any of this
stuff. It makes me very suspicious that the guc is going to start
poking at some display stuff behind the driver's back. I actually
think that doing some display stuff in the guc might be neat, but not
when it's a black box blob.

> +						struct drm_display_mode *mode = &crtc->mode;
> +						/* FIXME: Update is_widi based on encoder */
> +						per_pipe_info->is_widi = 0;
> +						per_pipe_info->refresh_rate = mode->vrefresh;
> +						per_pipe_info->vsync_ft_usec = 1000000 / mode->vrefresh;
> +						cur_params->active_pipes_bitmask |= (1 << intel_crtc->pipe);
> +						cur_params->vbi_sync_on_pipes |= (1 << intel_crtc->pipe);
> +						cur_params->num_active_pipes++;
> +					}
> +				}
> +			}
> +		}
> +	}
> +
> +	/* Compare old display mode with current mode. Notify SLPC if it is changed. */
> +	if (cur_params->global_data != old_params.global_data)
> +		notify = true;
> +
> +	for_each_intel_crtc(dev, intel_crtc) {
> +		per_pipe_info = &cur_params->per_pipe_info[intel_crtc->pipe];
> +		if (old_params.per_pipe_info[intel_crtc->pipe].data != per_pipe_info->data)
> +			notify = true;
> +	}
> +
> +	if (notify)
> +		host2guc_slpc_display_mode_change(dev_priv);
> +
> +	return 0;
> +}
> diff --git a/drivers/gpu/drm/i915/intel_slpc.h b/drivers/gpu/drm/i915/intel_slpc.h
> index 9673a63..18e551b 100644
> --- a/drivers/gpu/drm/i915/intel_slpc.h
> +++ b/drivers/gpu/drm/i915/intel_slpc.h
> @@ -152,5 +152,6 @@ int intel_slpc_suspend(struct drm_device *dev);
>  int intel_slpc_disable(struct drm_device *dev);
>  int intel_slpc_enable(struct drm_device *dev);
>  int intel_slpc_reset(struct drm_device *dev);
> +int intel_slpc_update_display_mode_info(struct drm_device *dev);
>  
>  #endif
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 06/22] drm/i915/slpc: If using SLPC, do not set frequency

[Daniel Vetter]

On Fri, Jan 22, 2016 at 05:53:29PM +0100, Daniel Vetter wrote:
> On Wed, Jan 20, 2016 at 06:26:08PM -0800, tom.orourke@intel.com wrote:
> > From: Tom O'Rourke <Tom.O'Rourke@intel.com>
> > 
> > When frequency requests are made by SLPC, host driver
> > should not attempt to make frequency requests due to
> > potential conflicts.
> > 
> > Host-based turbo operations are already avoided when
> > SLPC is used.  This change covers other frequency
> > requests such as from sysfs or debugfs interfaces.
> > 
> > Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
> 
> Does this break the sysfs interface, i.e. can userspace still set limits
> when SLPC is in use?

Ah found the sysfs control bits later on in the series.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 00/22] Add support for GuC-based SLPC

[Jesse Barnes]

On 01/22/2016 09:00 AM, Daniel Vetter wrote:
> On Wed, Jan 20, 2016 at 06:26:02PM -0800, tom.orourke@intel.com wrote:
>> From: Tom O'Rourke <Tom.O'Rourke@intel.com>
>>
>> SLPC (Single Loop Power Controller) is a replacement for
>> some host-based power management features.  The SLPC
>> implemenation runs in firmware on GuC.
>>
>> This series is a first request for comments.  This series
>> is not expected to be merged.  After changes based on
>> comments, a later patch series will be sent for merging.
>>  
>> This series has been tested with SKL guc firmware
>> versions 4.3 and 4.7.  The graphics power management
>> features in SLPC in those versions are DFPS (Dynamic FPS),
>> Turbo, and DCC (Duty Cycle Control).  DFPS adjusts
>> requested graphics frequency to maintain target framerate.
>> Turbo adjusts requested graphics frequency to maintain
>> target GT busyness.  DCC adjusts requested graphics
>> frequency and stalls guc-scheduler to maintain actual
>> graphics frequency in efficient range.
> 
> Either it's been forever long ago or I missed that meeting, so I'll drop
> my big arch concerns here. We probably need to discuss this internally, at
> least the benchmark data. Two big items:
> 
> - How does GuC measure fps rendered to the screen? More specifically, how
>   does it figure out that we missed a frame and kick the throttle up?

Yeah, this has been covered before, both in the design review and with the GuC team; I don't think the DFPS feature is ready for Linux usage yet, or at least not generally, since afaik it doesn't have a way to monitor offscreen rendering at all, so may end up keeping the GPU freq lower than it needs to be when several clients are rendering to offscreen buffers and passing them to the compositor (depending on the compositor behavior at least).

> - This patch series seems to remove the limiting abilities, and also
>   completely no-ops out our boost/deboost features. Can we recover these
>   features?

This is a good question; if the GuC handles this it should probably be mentioned somewhere.

Jesse
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 00/22] Add support for GuC-based SLPC

[Daniel Vetter]

On Tue, Jan 26, 2016 at 07:45:42AM -0800, Jesse Barnes wrote:
> On 01/22/2016 09:00 AM, Daniel Vetter wrote:
> > On Wed, Jan 20, 2016 at 06:26:02PM -0800, tom.orourke@intel.com wrote:
> >> From: Tom O'Rourke <Tom.O'Rourke@intel.com>
> >>
> >> SLPC (Single Loop Power Controller) is a replacement for
> >> some host-based power management features.  The SLPC
> >> implemenation runs in firmware on GuC.
> >>
> >> This series is a first request for comments.  This series
> >> is not expected to be merged.  After changes based on
> >> comments, a later patch series will be sent for merging.
> >>  
> >> This series has been tested with SKL guc firmware
> >> versions 4.3 and 4.7.  The graphics power management
> >> features in SLPC in those versions are DFPS (Dynamic FPS),
> >> Turbo, and DCC (Duty Cycle Control).  DFPS adjusts
> >> requested graphics frequency to maintain target framerate.
> >> Turbo adjusts requested graphics frequency to maintain
> >> target GT busyness.  DCC adjusts requested graphics
> >> frequency and stalls guc-scheduler to maintain actual
> >> graphics frequency in efficient range.
> > 
> > Either it's been forever long ago or I missed that meeting, so I'll drop
> > my big arch concerns here. We probably need to discuss this internally, at
> > least the benchmark data. Two big items:
> > 
> > - How does GuC measure fps rendered to the screen? More specifically, how
> >   does it figure out that we missed a frame and kick the throttle up?
> 
> Yeah, this has been covered before, both in the design review and with
> the GuC team; I don't think the DFPS feature is ready for Linux usage
> yet, or at least not generally, since afaik it doesn't have a way to
> monitor offscreen rendering at all, so may end up keeping the GPU freq
> lower than it needs to be when several clients are rendering to
> offscreen buffers and passing them to the compositor (depending on the
> compositor behavior at least).

There's also all kinds of issues with the current design, like:
- kernel knows when exactly we missed the vblank to display the next
  frame, guc can only control for average fps.

- all the fun you mention about multiple clients.

- what if we have more than 1 display running at different fps?

I'd say we need to keep the boost-deboost stuff alive, e.g. by manually
telling guc that the we want different limits, then resetting those limits
again after the boost is done. Same for fast idling - kernel simply has a
better idea if anyone is about to submit more work (we have execbuf hints
for specific workloads like libva).

Of course this assumes that guc slpc actually obeys our new limit requests
fast enough, so we'd still need to benchmark to make sure it's not slower
than what we have.

> > - This patch series seems to remove the limiting abilities, and also
> >   completely no-ops out our boost/deboost features. Can we recover these
> >   features?
> 
> This is a good question; if the GuC handles this it should probably be mentioned somewhere.

Limiting's still intact, spotted the code to set limits using guc in one of
the last patches. The boost-deboost is the big issue imo, also since it
interferes with our won missed-frame logic.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 00/22] Add support for GuC-based SLPC

[Jesse Barnes]

On 01/26/2016 09:00 AM, Daniel Vetter wrote:
> On Tue, Jan 26, 2016 at 07:45:42AM -0800, Jesse Barnes wrote:
>> On 01/22/2016 09:00 AM, Daniel Vetter wrote:
>>> On Wed, Jan 20, 2016 at 06:26:02PM -0800, tom.orourke@intel.com wrote:
>>>> From: Tom O'Rourke <Tom.O'Rourke@intel.com>
>>>>
>>>> SLPC (Single Loop Power Controller) is a replacement for
>>>> some host-based power management features.  The SLPC
>>>> implemenation runs in firmware on GuC.
>>>>
>>>> This series is a first request for comments.  This series
>>>> is not expected to be merged.  After changes based on
>>>> comments, a later patch series will be sent for merging.
>>>>  
>>>> This series has been tested with SKL guc firmware
>>>> versions 4.3 and 4.7.  The graphics power management
>>>> features in SLPC in those versions are DFPS (Dynamic FPS),
>>>> Turbo, and DCC (Duty Cycle Control).  DFPS adjusts
>>>> requested graphics frequency to maintain target framerate.
>>>> Turbo adjusts requested graphics frequency to maintain
>>>> target GT busyness.  DCC adjusts requested graphics
>>>> frequency and stalls guc-scheduler to maintain actual
>>>> graphics frequency in efficient range.
>>>
>>> Either it's been forever long ago or I missed that meeting, so I'll drop
>>> my big arch concerns here. We probably need to discuss this internally, at
>>> least the benchmark data. Two big items:
>>>
>>> - How does GuC measure fps rendered to the screen? More specifically, how
>>>   does it figure out that we missed a frame and kick the throttle up?
>>
>> Yeah, this has been covered before, both in the design review and with
>> the GuC team; I don't think the DFPS feature is ready for Linux usage
>> yet, or at least not generally, since afaik it doesn't have a way to
>> monitor offscreen rendering at all, so may end up keeping the GPU freq
>> lower than it needs to be when several clients are rendering to
>> offscreen buffers and passing them to the compositor (depending on the
>> compositor behavior at least).
> 
> There's also all kinds of issues with the current design, like:
> - kernel knows when exactly we missed the vblank to display the next
>   frame, guc can only control for average fps.
> 
> - all the fun you mention about multiple clients.
> 
> - what if we have more than 1 display running at different fps?

Yep; I think a userspace solution with a kernel interface would do a
better job here (I outlined one a few years ago, but the lazyweb hasn't
implemented it for me yet).

> I'd say we need to keep the boost-deboost stuff alive, e.g. by manually
> telling guc that the we want different limits, then resetting those limits
> again after the boost is done. Same for fast idling - kernel simply has a
> better idea if anyone is about to submit more work (we have execbuf hints
> for specific workloads like libva).
> 
> Of course this assumes that guc slpc actually obeys our new limit requests
> fast enough, so we'd still need to benchmark to make sure it's not slower
> than what we have.

I definitely want to see benchmarking here too.  Maybe the GuC does
boosting differently, but it may be just as good as what we have for all
practical purposes.

Jesse
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 00/22] Add support for GuC-based SLPC

[O'Rourke, Tom]

On Tue, Jan 26, 2016 at 09:17:51AM -0800, Jesse Barnes wrote:
> On 01/26/2016 09:00 AM, Daniel Vetter wrote:
> > On Tue, Jan 26, 2016 at 07:45:42AM -0800, Jesse Barnes wrote:
> >> On 01/22/2016 09:00 AM, Daniel Vetter wrote:
> >>> On Wed, Jan 20, 2016 at 06:26:02PM -0800, tom.orourke@intel.com wrote:
> >>>> From: Tom O'Rourke <Tom.O'Rourke@intel.com>
> >>>>
> >>>> SLPC (Single Loop Power Controller) is a replacement for
> >>>> some host-based power management features.  The SLPC
> >>>> implemenation runs in firmware on GuC.
> >>>>
> >>>> This series is a first request for comments.  This series
> >>>> is not expected to be merged.  After changes based on
> >>>> comments, a later patch series will be sent for merging.
> >>>>  
> >>>> This series has been tested with SKL guc firmware
> >>>> versions 4.3 and 4.7.  The graphics power management
> >>>> features in SLPC in those versions are DFPS (Dynamic FPS),
> >>>> Turbo, and DCC (Duty Cycle Control).  DFPS adjusts
> >>>> requested graphics frequency to maintain target framerate.
> >>>> Turbo adjusts requested graphics frequency to maintain
> >>>> target GT busyness.  DCC adjusts requested graphics
> >>>> frequency and stalls guc-scheduler to maintain actual
> >>>> graphics frequency in efficient range.
> >>>
> >>> Either it's been forever long ago or I missed that meeting, so I'll drop
> >>> my big arch concerns here. We probably need to discuss this internally, at
> >>> least the benchmark data. Two big items:
> >>>
> >>> - How does GuC measure fps rendered to the screen? More specifically, how
> >>>   does it figure out that we missed a frame and kick the throttle up?
> >>
> >> Yeah, this has been covered before, both in the design review and with
> >> the GuC team; I don't think the DFPS feature is ready for Linux usage
> >> yet, or at least not generally, since afaik it doesn't have a way to
> >> monitor offscreen rendering at all, so may end up keeping the GPU freq
> >> lower than it needs to be when several clients are rendering to
> >> offscreen buffers and passing them to the compositor (depending on the
> >> compositor behavior at least).
> > 
> > There's also all kinds of issues with the current design, like:
> > - kernel knows when exactly we missed the vblank to display the next
> >   frame, guc can only control for average fps.
> > 
> > - all the fun you mention about multiple clients.
> > 
> > - what if we have more than 1 display running at different fps?
> 
> Yep; I think a userspace solution with a kernel interface would do a
> better job here (I outlined one a few years ago, but the lazyweb hasn't
> implemented it for me yet).

[TOR:] Patch 14/22 in this series sends display configuration info to SLPC.
If there are multiple displays, SLPC can take that into consideration.
> 
> > I'd say we need to keep the boost-deboost stuff alive, e.g. by manually
> > telling guc that the we want different limits, then resetting those limits
> > again after the boost is done. Same for fast idling - kernel simply has a
> > better idea if anyone is about to submit more work (we have execbuf hints
> > for specific workloads like libva).

[TOR:] Could those hints be sent to GuC as well?
> > 
> > Of course this assumes that guc slpc actually obeys our new limit requests
> > fast enough, so we'd still need to benchmark to make sure it's not slower
> > than what we have.
> 
> I definitely want to see benchmarking here too.  Maybe the GuC does
> boosting differently, but it may be just as good as what we have for all
> practical purposes.
> 
> Jesse
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 14/22] drm/i915/slpc: Notification of Display mode change

[Kamble, Sagar A]

Thanks for the review Paulo.
Will incorporate the suggestions.

Thanks
Sagar

On 1/21/2016 6:54 PM, Zanoni, Paulo R wrote:
> Em Qua, 2016-01-20 às 18:26 -0800, tom.orourke@intel.com escreveu:
>> From: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>>
>> GuC SLPC need to be sent data related to Active pipes, refresh rates,
>> widi pipes, fullscreen pipes related via host to GuC display mode
>> change event.
>> This patch defines the event and implements trigger of the event.
>>
>> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> Acked-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
> (this is just a comment from a quick look at the file, not a full
> review)
>
>> ---
>>   drivers/gpu/drm/i915/intel_display.c |  2 +
>>   drivers/gpu/drm/i915/intel_slpc.c    | 92
>> ++++++++++++++++++++++++++++++++++++
>>   drivers/gpu/drm/i915/intel_slpc.h    |  1 +
>>   3 files changed, 95 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c
>> b/drivers/gpu/drm/i915/intel_display.c
>> index 06ab6df..7c3d902 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -13638,6 +13638,8 @@ static int intel_atomic_commit(struct
>> drm_device *dev,
>>   	 */
>>   	intel_uncore_arm_unclaimed_mmio_detection(dev_priv);
>>   
>> +	intel_slpc_update_display_mode_info(dev);
>> +
>>   	return 0;
>>   }
>>   
>> diff --git a/drivers/gpu/drm/i915/intel_slpc.c
>> b/drivers/gpu/drm/i915/intel_slpc.c
>> index f155d88..f5f7cad 100644
>> --- a/drivers/gpu/drm/i915/intel_slpc.c
>> +++ b/drivers/gpu/drm/i915/intel_slpc.c
>> @@ -74,6 +74,27 @@ static int host2guc_slpc_shutdown(struct
>> drm_i915_private *dev_priv)
>>   	return ret;
>>   }
>>   
>> +static int host2guc_slpc_display_mode_change(struct drm_i915_private
>> *dev_priv)
>> +{
>> +        u32 data[7];
>> +	int ret, i;
>> +
>> +        data[0] = HOST2GUC_ACTION_SLPC_REQUEST;
>> +        data[1] = SLPC_EVENT(SLPC_EVENT_DISPLAY_MODE_CHANGE,
>> MAX_NUM_OF_PIPE + 1);
>> +	data[2] = dev_priv-
>>> guc.slpc.display_mode_params.global_data;
>> +	for(i = 0; i < MAX_NUM_OF_PIPE; ++i)
>> +		data[3+i] = dev_priv-
>>> guc.slpc.display_mode_params.per_pipe_info[i].data;
>> +
>> +        ret = host2guc_action(&dev_priv->guc, data, 7);
>> +
>> +	if (0 == ret) {
> https://en.wikipedia.org/wiki/Yoda_conditions are not part of our usual
> coding style.
>
>> +		ret = I915_READ(SOFT_SCRATCH(1));
>> +		ret &= 0xFF;
>> +	}
>> +
>> +	return ret;
>> +}
> There are some tab/space issues in the function above.
>
> Just a suggestion, not a request: for both functions you introduced,
> since the only callers do not bother to check the return values, you
> could just make the functions return void. Having unchecked return
> values may give a false sense that errors would be caught by the upper
> layer, when in fact they aren't. In a lot of cases it's better to just
> print some error message instead of returning some value that is going
> to be ignored.
>
>> +
>>   static u8 slpc_get_platform_sku(struct drm_i915_gem_object *obj)
>>   {
>>   	struct drm_device *dev = obj->base.dev;
>> @@ -225,3 +246,74 @@ int intel_slpc_reset(struct drm_device *dev)
>>   
>>   	return ret;
>>   }
>> +
>> +int intel_slpc_update_display_mode_info(struct drm_device *dev)
>> +{
>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>> +	struct drm_crtc *crtc;
>> +	struct intel_crtc *intel_crtc;
>> +	struct drm_connector *connector;
>> +	struct intel_connector *intel_connector;
>> +	struct drm_encoder *encoder;
>> +	struct intel_encoder *intel_encoder;
>> +	struct intel_display_pipe_basic_info *per_pipe_info;
>> +	struct intel_slpc_display_mode_event_params *cur_params,
>> old_params;
>> +	bool notify = false;
>> +
>> +	if (!HAS_SLPC(dev))
>> +		return -EINVAL;
>> +
>> +	cur_params = &dev_priv->guc.slpc.display_mode_params;
>> +
>> +	/* Copy display mode parameters for comparison */
>> +	old_params.global_data  = cur_params->global_data;
>> +	cur_params->global_data = 0;
>> +
>> +	for_each_intel_crtc(dev, intel_crtc) {
>> +		per_pipe_info = &cur_params-
>>> per_pipe_info[intel_crtc->pipe];
>> +		old_params.per_pipe_info[intel_crtc->pipe].data =
>> per_pipe_info->data;
>> +		per_pipe_info->data = 0;
>> +	}
>> +
>> +	for_each_intel_crtc(dev, intel_crtc) {
>> +		struct intel_crtc_state *pipe_config;
>> +
>> +		per_pipe_info = &cur_params-
>>> per_pipe_info[intel_crtc->pipe];
>> +		crtc = &intel_crtc->base;
>> +		pipe_config = to_intel_crtc_state(crtc->state);
>> +
>> +		if (pipe_config->base.active) {
> As far as I understand from the new atomic locking model, since this
> code is called from intel_atomic_commit, it can't just iterate through
> every crtc/encoder/connector checking state structures. During an
> atomic commit we can only look at the objects affected by the commit,
> so we have to use things such as for_each_crtc_in_state().
>
> It seems to me that the model here could be:
> - At driver load, during or after HW state readout, initialize some
> state structure to make it match the hardware.
> - Whenever there's an atomic commit, just look+update the status of the
> affected crtc/encoder/connector, leaving everything else the same
>
>> +			for_each_encoder_on_crtc(dev, crtc,
>> intel_encoder) {
>> +				encoder = &intel_encoder->base;
>> +				for_each_connector_on_encoder(dev,
>> encoder, intel_connector) {
>> +					connector =
>> &intel_connector->base;
>> +					if (connector->status ==
>> connector_status_connected) {
>> +						struct
>> drm_display_mode *mode = &crtc->mode;
>> +						/* FIXME: Update
>> is_widi based on encoder */
>> +						per_pipe_info-
>>> is_widi = 0;
>> +						per_pipe_info-
>>> refresh_rate = mode->vrefresh;
>> +						per_pipe_info-
>>> vsync_ft_usec = 1000000 / mode->vrefresh;
>> +						cur_params-
>>> active_pipes_bitmask |= (1 << intel_crtc->pipe);
>> +						cur_params-
>>> vbi_sync_on_pipes |= (1 << intel_crtc->pipe);
>> +						cur_params-
>>> num_active_pipes++;
>> +					}
>> +				}
>> +			}
>> +		}
>> +	}
>> +
>> +	/* Compare old display mode with current mode. Notify SLPC
>> if it is changed. */
>> +	if (cur_params->global_data != old_params.global_data)
>> +		notify = true;
>> +
>> +	for_each_intel_crtc(dev, intel_crtc) {
>> +		per_pipe_info = &cur_params-
>>> per_pipe_info[intel_crtc->pipe];
>> +		if (old_params.per_pipe_info[intel_crtc->pipe].data
>> != per_pipe_info->data)
>> +			notify = true;
>> +	}
>> +
>> +	if (notify)
>> +		host2guc_slpc_display_mode_change(dev_priv);
>> +
>> +	return 0;
>> +}
>> diff --git a/drivers/gpu/drm/i915/intel_slpc.h
>> b/drivers/gpu/drm/i915/intel_slpc.h
>> index 9673a63..18e551b 100644
>> --- a/drivers/gpu/drm/i915/intel_slpc.h
>> +++ b/drivers/gpu/drm/i915/intel_slpc.h
>> @@ -152,5 +152,6 @@ int intel_slpc_suspend(struct drm_device *dev);
>>   int intel_slpc_disable(struct drm_device *dev);
>>   int intel_slpc_enable(struct drm_device *dev);
>>   int intel_slpc_reset(struct drm_device *dev);
>> +int intel_slpc_update_display_mode_info(struct drm_device *dev);
>>   
>>   #endif
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 14/22] drm/i915/slpc: Notification of Display mode change

[Kamble, Sagar A]

Thanks for the review Ville.
I will update the patch.

On 1/22/2016 10:44 PM, Ville Syrjälä wrote:
> On Wed, Jan 20, 2016 at 06:26:16PM -0800, tom.orourke@intel.com wrote:
>> From: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>>
>> GuC SLPC need to be sent data related to Active pipes, refresh rates,
>> widi pipes, fullscreen pipes related via host to GuC display mode
>> change event.
>> This patch defines the event and implements trigger of the event.
>>
>> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> Acked-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_display.c |  2 +
>>   drivers/gpu/drm/i915/intel_slpc.c    | 92 ++++++++++++++++++++++++++++++++++++
>>   drivers/gpu/drm/i915/intel_slpc.h    |  1 +
>>   3 files changed, 95 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 06ab6df..7c3d902 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -13638,6 +13638,8 @@ static int intel_atomic_commit(struct drm_device *dev,
>>   	 */
>>   	intel_uncore_arm_unclaimed_mmio_detection(dev_priv);
>>   
>> +	intel_slpc_update_display_mode_info(dev);
>> +
>>   	return 0;
>>   }
>>   
>> diff --git a/drivers/gpu/drm/i915/intel_slpc.c b/drivers/gpu/drm/i915/intel_slpc.c
>> index f155d88..f5f7cad 100644
>> --- a/drivers/gpu/drm/i915/intel_slpc.c
>> +++ b/drivers/gpu/drm/i915/intel_slpc.c
>> @@ -74,6 +74,27 @@ static int host2guc_slpc_shutdown(struct drm_i915_private *dev_priv)
>>   	return ret;
>>   }
>>   
>> +static int host2guc_slpc_display_mode_change(struct drm_i915_private *dev_priv)
>> +{
>> +        u32 data[7];
>> +	int ret, i;
>> +
>> +        data[0] = HOST2GUC_ACTION_SLPC_REQUEST;
>> +        data[1] = SLPC_EVENT(SLPC_EVENT_DISPLAY_MODE_CHANGE, MAX_NUM_OF_PIPE + 1);
>> +	data[2] = dev_priv->guc.slpc.display_mode_params.global_data;
>> +	for(i = 0; i < MAX_NUM_OF_PIPE; ++i)
>> +		data[3+i] = dev_priv->guc.slpc.display_mode_params.per_pipe_info[i].data;
>> +
>> +        ret = host2guc_action(&dev_priv->guc, data, 7);
>> +
>> +	if (0 == ret) {
>> +		ret = I915_READ(SOFT_SCRATCH(1));
>> +		ret &= 0xFF;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>>   static u8 slpc_get_platform_sku(struct drm_i915_gem_object *obj)
>>   {
>>   	struct drm_device *dev = obj->base.dev;
>> @@ -225,3 +246,74 @@ int intel_slpc_reset(struct drm_device *dev)
>>   
>>   	return ret;
>>   }
>> +
>> +int intel_slpc_update_display_mode_info(struct drm_device *dev)
>> +{
>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>> +	struct drm_crtc *crtc;
>> +	struct intel_crtc *intel_crtc;
>> +	struct drm_connector *connector;
>> +	struct intel_connector *intel_connector;
>> +	struct drm_encoder *encoder;
>> +	struct intel_encoder *intel_encoder;
>> +	struct intel_display_pipe_basic_info *per_pipe_info;
>> +	struct intel_slpc_display_mode_event_params *cur_params, old_params;
>> +	bool notify = false;
>> +
>> +	if (!HAS_SLPC(dev))
>> +		return -EINVAL;
>> +
>> +	cur_params = &dev_priv->guc.slpc.display_mode_params;
>> +
>> +	/* Copy display mode parameters for comparison */
>> +	old_params.global_data  = cur_params->global_data;
>> +	cur_params->global_data = 0;
>> +
>> +	for_each_intel_crtc(dev, intel_crtc) {
>> +		per_pipe_info = &cur_params->per_pipe_info[intel_crtc->pipe];
>> +		old_params.per_pipe_info[intel_crtc->pipe].data = per_pipe_info->data;
>> +		per_pipe_info->data = 0;
>> +	}
>> +
>> +	for_each_intel_crtc(dev, intel_crtc) {
>> +		struct intel_crtc_state *pipe_config;
>> +
>> +		per_pipe_info = &cur_params->per_pipe_info[intel_crtc->pipe];
>> +		crtc = &intel_crtc->base;
>> +		pipe_config = to_intel_crtc_state(crtc->state);
>> +
>> +		if (pipe_config->base.active) {
>> +			for_each_encoder_on_crtc(dev, crtc, intel_encoder) {
>> +				encoder = &intel_encoder->base;
>> +				for_each_connector_on_encoder(dev, encoder, intel_connector) {
>> +					connector = &intel_connector->base;
>> +					if (connector->status == connector_status_connected) {
> The connecotr/encoder stuff should not be here. We can enable a pipe
> without any connected connectors. Also locking is fail.
>
> The patch also fails to explain why the guc needs to know any of this
> stuff. It makes me very suspicious that the guc is going to start
> poking at some display stuff behind the driver's back. I actually
> think that doing some display stuff in the guc might be neat, but not
> when it's a black box blob.
>
>> +						struct drm_display_mode *mode = &crtc->mode;
>> +						/* FIXME: Update is_widi based on encoder */
>> +						per_pipe_info->is_widi = 0;
>> +						per_pipe_info->refresh_rate = mode->vrefresh;
>> +						per_pipe_info->vsync_ft_usec = 1000000 / mode->vrefresh;
>> +						cur_params->active_pipes_bitmask |= (1 << intel_crtc->pipe);
>> +						cur_params->vbi_sync_on_pipes |= (1 << intel_crtc->pipe);
>> +						cur_params->num_active_pipes++;
>> +					}
>> +				}
>> +			}
>> +		}
>> +	}
>> +
>> +	/* Compare old display mode with current mode. Notify SLPC if it is changed. */
>> +	if (cur_params->global_data != old_params.global_data)
>> +		notify = true;
>> +
>> +	for_each_intel_crtc(dev, intel_crtc) {
>> +		per_pipe_info = &cur_params->per_pipe_info[intel_crtc->pipe];
>> +		if (old_params.per_pipe_info[intel_crtc->pipe].data != per_pipe_info->data)
>> +			notify = true;
>> +	}
>> +
>> +	if (notify)
>> +		host2guc_slpc_display_mode_change(dev_priv);
>> +
>> +	return 0;
>> +}
>> diff --git a/drivers/gpu/drm/i915/intel_slpc.h b/drivers/gpu/drm/i915/intel_slpc.h
>> index 9673a63..18e551b 100644
>> --- a/drivers/gpu/drm/i915/intel_slpc.h
>> +++ b/drivers/gpu/drm/i915/intel_slpc.h
>> @@ -152,5 +152,6 @@ int intel_slpc_suspend(struct drm_device *dev);
>>   int intel_slpc_disable(struct drm_device *dev);
>>   int intel_slpc_enable(struct drm_device *dev);
>>   int intel_slpc_reset(struct drm_device *dev);
>> +int intel_slpc_update_display_mode_info(struct drm_device *dev);
>>   
>>   #endif
>> -- 
>> 1.9.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 00/22] Add support for GuC-based SLPC

[Zanoni, Paulo R]

Em Qua, 2016-01-20 às 18:26 -0800, tom.orourke@intel.com escreveu:
> From: Tom O'Rourke <Tom.O'Rourke@intel.com>
> 
> SLPC (Single Loop Power Controller) is a replacement for
> some host-based power management features.  The SLPC
> implemenation runs in firmware on GuC.
> 
> This series is a first request for comments.  This series
> is not expected to be merged.  After changes based on
> comments, a later patch series will be sent for merging.
>  
> This series has been tested with SKL guc firmware
> versions 4.3 and 4.7.  The graphics power management
> features in SLPC in those versions are DFPS (Dynamic FPS),
> Turbo, and DCC (Duty Cycle Control).  DFPS adjusts
> requested graphics frequency to maintain target framerate.
> Turbo adjusts requested graphics frequency to maintain
> target GT busyness.  DCC adjusts requested graphics
> frequency and stalls guc-scheduler to maintain actual
> graphics frequency in efficient range.
> 
> Patch 1/22 is included ihere for convenience and should be
> part of an earlier series.  SLPC assumes guc firmware has
> been loaded and GuC submission is enabled.
> 
> Patch 22/22 sets the flag to enable SLPC on SKL.  Without
> this patch, the previous patches should have no effect.
> 
> VIZ-6773, VIZ-6889

Hi

Some high-level comments for the whole series:

In many places there are 8 spaces instead of tabs.

There are also some lines containing just a tab character.

Lots of Yoda conditions: if (constant == variable).

Some functions would get much simpler if they had early returns.

I certainly wouldn't complain if you also extracted the relevant
rps/powersave code out of intel_pm.c to its own file with a nice
documentation. Of course, this could be done after the series.

Lots of ignored return values. It seems the inner functions all check
for errors and return them to their callers, but the top-most functions
added by the series just ignore the errors. See my previous comment on
patch 14 about this for suggestions.

There are no checks for GuC version here. We know that the SLPC ABI is
not stable and can change in new firmware versions, so I expect the
SLPC code to only run if it finds the specific _whitelisted_ GuC
versions. No "if (version >= num) use_slpc=true;", please.

I'm not 100% sure, but from what I could understand, it seems I'll get
a broken machine with no RPS at all if I don't have the GuC firmware
files - or if the GuC version is not the expected. IMHO this is a
regression since I currently don't have any firmware files and my SKL
machine works.

I see most of the functions are protected by "HAS_SLPC". Usually
HAS_SOMETHING is used for hardware features and is expected to be
constant on platforms. I suggest you to add a possible driver parameter
such as i915.enable_slpc, and also add a new "intel_slpc_enabled()" or
"intel_using_slpc()" function and replace all the HAS_SLPC checks with
these. This way, we'll be able to easily disable SLPC in case we don't
want it (such as due to a regression) or if there's no firmware or
incorrect firmware version, and revert back to the old (current)
behavior of driver-based turbo. The only HAS_SLPC check should be
during SLPC initialization. Having an easy way to enable/disable SLPC
will also be immensely helpful when we start running workloads to
decide if we want to enable it.

You could even go further and make the i915.enable_slpc param be a mask
where it's possible to select each sub-feature individually (dfps,
turbo, dcc).

Some of the checks for shared_data_obj could also become calls to an
inline function with a nice name such as intel_slpc_enabled() or
something else.

I see there's a specific pattern on the places that call
host2guc_action(). Perhaps we could move that common code to its own
function? It would also be nice to add a name to the 0xFF mask that we
return - and that gets ignored at the end of the call chains.

Patch 5 needs a commit message. Actually, when reviewing patch 4 I
thought it had broken RC6, until I saw patch 5, so maybe you could just
squash both commits into one.

On patch 13, those defines such as MAX_INTEL_PIPES are weird and
confusing (why do we check if they were already defined?), especially
since we already have I915_MAX_PIPES. And the only value that is
actually used is MAX_NUM_OF_PIPE. I would vote for you to only keep
this define, and prefix it with SLPC, such as SLPC_NUM_OF_PIPES (or any
other better name).

On patches 14/15, is mode->vrefresh accurate enough? Don't we want the
more-precise "clock/(htotal*vtotal)" value?

On patch 17. I'm not an expert here, but I'm not sure if we can call
kmap_atomic and then do those seq_printf calls without kunmap. 

Thanks,
Paulo

> 
> Dave Gordon (1):
>   drm/i915: Enable GuC submission, where supported
> 
> Sagar Arun Kamble (4):
>   drm/i915/slpc: Enable/Disable RC6 in SLPC flows
>   drm/i915/slpc: Add Display mode event related data structures
>   drm/i915/slpc: Notification of Display mode change
>   drm/i915/slpc: Notification of Refresh Rate change
> 
> Tom O'Rourke (17):
>   drm/i915/slpc: Add has_slpc capability flag
>   drm/i915/slpc: Expose guc functions for use with SLPC
>   drm/i915/slpc: Use intel_slpc_* functions if supported
>   drm/i915/slpc: If using SLPC, do not set frequency
>   drm/i915/slpc: Enable SLPC in guc if supported
>   drm/i915/slpc: Allocate/Release/Initialize SLPC shared data
>   drm/i915/slpc: Setup rps frequency values during SLPC init
>   drm/i915/slpc: Update current requested frequency
>   drm/i915/slpc: Send reset event
>   drm/i915/slpc: Send shutdown event
>   drm/i915/slpc: Add slpc_status enum values
>   drm/i915/slpc: Add i915_slpc_info to debugfs
>   drm/i915/slpc: Add dfps task info to i915_slpc_info
>   drm/i915/slpc: Add parameter unset/set/get functions
>   drm/i915/slpc: Add slpc support for max/min freq
>   drm/i915/slpc: Add enable/disable debugfs for slpc
>   drm/i915/slpc: Add has_slpc to skylake info
> 
>  drivers/gpu/drm/i915/Makefile              |   5 +-
>  drivers/gpu/drm/i915/i915_debugfs.c        | 436
> +++++++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_drv.c            |   1 +
>  drivers/gpu/drm/i915/i915_drv.h            |   2 +
>  drivers/gpu/drm/i915/i915_guc_submission.c |   6 +-
>  drivers/gpu/drm/i915/i915_params.c         |   4 +-
>  drivers/gpu/drm/i915/i915_sysfs.c          |  10 +
>  drivers/gpu/drm/i915/intel_display.c       |   2 +
>  drivers/gpu/drm/i915/intel_dp.c            |   2 +
>  drivers/gpu/drm/i915/intel_drv.h           |   1 +
>  drivers/gpu/drm/i915/intel_guc.h           |   7 +
>  drivers/gpu/drm/i915/intel_guc_loader.c    |   3 +
>  drivers/gpu/drm/i915/intel_pm.c            |  43 ++-
>  drivers/gpu/drm/i915/intel_slpc.c          | 499
> +++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_slpc.h          | 207 ++++++++++++
>  15 files changed, 1210 insertions(+), 18 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/intel_slpc.c
>  create mode 100644 drivers/gpu/drm/i915/intel_slpc.h
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 00/22] Add support for GuC-based SLPC

[Kamble, Sagar A]

Hi Paulo,

Thanks for comments.
1. Will make change related to #define for number of pipes and remove 
the unnecessary ones.
2. vrefresh is almost same as "clock/(htotal*vtotal) if we round up 
later. Will keep it vrefresh for now.

Thanks
Sagar


On 2/4/2016 1:55 AM, Zanoni, Paulo R wrote:
> Em Qua, 2016-01-20 às 18:26 -0800, tom.orourke@intel.com escreveu:
>> From: Tom O'Rourke <Tom.O'Rourke@intel.com>
>>
>> SLPC (Single Loop Power Controller) is a replacement for
>> some host-based power management features.  The SLPC
>> implemenation runs in firmware on GuC.
>>
>> This series is a first request for comments.  This series
>> is not expected to be merged.  After changes based on
>> comments, a later patch series will be sent for merging.
>>   
>> This series has been tested with SKL guc firmware
>> versions 4.3 and 4.7.  The graphics power management
>> features in SLPC in those versions are DFPS (Dynamic FPS),
>> Turbo, and DCC (Duty Cycle Control).  DFPS adjusts
>> requested graphics frequency to maintain target framerate.
>> Turbo adjusts requested graphics frequency to maintain
>> target GT busyness.  DCC adjusts requested graphics
>> frequency and stalls guc-scheduler to maintain actual
>> graphics frequency in efficient range.
>>
>> Patch 1/22 is included ihere for convenience and should be
>> part of an earlier series.  SLPC assumes guc firmware has
>> been loaded and GuC submission is enabled.
>>
>> Patch 22/22 sets the flag to enable SLPC on SKL.  Without
>> this patch, the previous patches should have no effect.
>>
>> VIZ-6773, VIZ-6889
> Hi
>
> Some high-level comments for the whole series:
>
> In many places there are 8 spaces instead of tabs.
>
> There are also some lines containing just a tab character.
>
> Lots of Yoda conditions: if (constant == variable).
>
> Some functions would get much simpler if they had early returns.
>
> I certainly wouldn't complain if you also extracted the relevant
> rps/powersave code out of intel_pm.c to its own file with a nice
> documentation. Of course, this could be done after the series.
>
> Lots of ignored return values. It seems the inner functions all check
> for errors and return them to their callers, but the top-most functions
> added by the series just ignore the errors. See my previous comment on
> patch 14 about this for suggestions.
>
> There are no checks for GuC version here. We know that the SLPC ABI is
> not stable and can change in new firmware versions, so I expect the
> SLPC code to only run if it finds the specific _whitelisted_ GuC
> versions. No "if (version >= num) use_slpc=true;", please.
>
> I'm not 100% sure, but from what I could understand, it seems I'll get
> a broken machine with no RPS at all if I don't have the GuC firmware
> files - or if the GuC version is not the expected. IMHO this is a
> regression since I currently don't have any firmware files and my SKL
> machine works.
>
> I see most of the functions are protected by "HAS_SLPC". Usually
> HAS_SOMETHING is used for hardware features and is expected to be
> constant on platforms. I suggest you to add a possible driver parameter
> such as i915.enable_slpc, and also add a new "intel_slpc_enabled()" or
> "intel_using_slpc()" function and replace all the HAS_SLPC checks with
> these. This way, we'll be able to easily disable SLPC in case we don't
> want it (such as due to a regression) or if there's no firmware or
> incorrect firmware version, and revert back to the old (current)
> behavior of driver-based turbo. The only HAS_SLPC check should be
> during SLPC initialization. Having an easy way to enable/disable SLPC
> will also be immensely helpful when we start running workloads to
> decide if we want to enable it.
>
> You could even go further and make the i915.enable_slpc param be a mask
> where it's possible to select each sub-feature individually (dfps,
> turbo, dcc).
>
> Some of the checks for shared_data_obj could also become calls to an
> inline function with a nice name such as intel_slpc_enabled() or
> something else.
>
> I see there's a specific pattern on the places that call
> host2guc_action(). Perhaps we could move that common code to its own
> function? It would also be nice to add a name to the 0xFF mask that we
> return - and that gets ignored at the end of the call chains.
>
> Patch 5 needs a commit message. Actually, when reviewing patch 4 I
> thought it had broken RC6, until I saw patch 5, so maybe you could just
> squash both commits into one.
>
> On patch 13, those defines such as MAX_INTEL_PIPES are weird and
> confusing (why do we check if they were already defined?), especially
> since we already have I915_MAX_PIPES. And the only value that is
> actually used is MAX_NUM_OF_PIPE. I would vote for you to only keep
> this define, and prefix it with SLPC, such as SLPC_NUM_OF_PIPES (or any
> other better name).
>
> On patches 14/15, is mode->vrefresh accurate enough? Don't we want the
> more-precise "clock/(htotal*vtotal)" value?
>
> On patch 17. I'm not an expert here, but I'm not sure if we can call
> kmap_atomic and then do those seq_printf calls without kunmap.
>
> Thanks,
> Paulo
>
>> Dave Gordon (1):
>>    drm/i915: Enable GuC submission, where supported
>>
>> Sagar Arun Kamble (4):
>>    drm/i915/slpc: Enable/Disable RC6 in SLPC flows
>>    drm/i915/slpc: Add Display mode event related data structures
>>    drm/i915/slpc: Notification of Display mode change
>>    drm/i915/slpc: Notification of Refresh Rate change
>>
>> Tom O'Rourke (17):
>>    drm/i915/slpc: Add has_slpc capability flag
>>    drm/i915/slpc: Expose guc functions for use with SLPC
>>    drm/i915/slpc: Use intel_slpc_* functions if supported
>>    drm/i915/slpc: If using SLPC, do not set frequency
>>    drm/i915/slpc: Enable SLPC in guc if supported
>>    drm/i915/slpc: Allocate/Release/Initialize SLPC shared data
>>    drm/i915/slpc: Setup rps frequency values during SLPC init
>>    drm/i915/slpc: Update current requested frequency
>>    drm/i915/slpc: Send reset event
>>    drm/i915/slpc: Send shutdown event
>>    drm/i915/slpc: Add slpc_status enum values
>>    drm/i915/slpc: Add i915_slpc_info to debugfs
>>    drm/i915/slpc: Add dfps task info to i915_slpc_info
>>    drm/i915/slpc: Add parameter unset/set/get functions
>>    drm/i915/slpc: Add slpc support for max/min freq
>>    drm/i915/slpc: Add enable/disable debugfs for slpc
>>    drm/i915/slpc: Add has_slpc to skylake info
>>
>>   drivers/gpu/drm/i915/Makefile              |   5 +-
>>   drivers/gpu/drm/i915/i915_debugfs.c        | 436
>> +++++++++++++++++++++++++
>>   drivers/gpu/drm/i915/i915_drv.c            |   1 +
>>   drivers/gpu/drm/i915/i915_drv.h            |   2 +
>>   drivers/gpu/drm/i915/i915_guc_submission.c |   6 +-
>>   drivers/gpu/drm/i915/i915_params.c         |   4 +-
>>   drivers/gpu/drm/i915/i915_sysfs.c          |  10 +
>>   drivers/gpu/drm/i915/intel_display.c       |   2 +
>>   drivers/gpu/drm/i915/intel_dp.c            |   2 +
>>   drivers/gpu/drm/i915/intel_drv.h           |   1 +
>>   drivers/gpu/drm/i915/intel_guc.h           |   7 +
>>   drivers/gpu/drm/i915/intel_guc_loader.c    |   3 +
>>   drivers/gpu/drm/i915/intel_pm.c            |  43 ++-
>>   drivers/gpu/drm/i915/intel_slpc.c          | 499
>> +++++++++++++++++++++++++++++
>>   drivers/gpu/drm/i915/intel_slpc.h          | 207 ++++++++++++
>>   15 files changed, 1210 insertions(+), 18 deletions(-)
>>   create mode 100644 drivers/gpu/drm/i915/intel_slpc.c
>>   create mode 100644 drivers/gpu/drm/i915/intel_slpc.h
>>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 00/22] Add support for GuC-based SLPC

[Martin Peres]

On 21/01/16 04:26, tom.orourke@intel.com wrote:
> From: Tom O'Rourke <Tom.O'Rourke@intel.com>
>
> SLPC (Single Loop Power Controller) is a replacement for
> some host-based power management features.  The SLPC
> implemenation runs in firmware on GuC.
>
> This series is a first request for comments.  This series
> is not expected to be merged.  After changes based on
> comments, a later patch series will be sent for merging.
>
> This series has been tested with SKL guc firmware
> versions 4.3 and 4.7.  The graphics power management
> features in SLPC in those versions are DFPS (Dynamic FPS),
> Turbo, and DCC (Duty Cycle Control).  DFPS adjusts
> requested graphics frequency to maintain target framerate.
> Turbo adjusts requested graphics frequency to maintain
> target GT busyness.  DCC adjusts requested graphics
> frequency and stalls guc-scheduler to maintain actual
> graphics frequency in efficient range.

Can we have a benchmark/CI mode that has turbo and reclocking disabled? 
The frequency would be set to whatever frequency the kernel requests and 
the GuC would report how many throttling events happened so as our 
benchmarking system could automatically lower the frequency to provide 
stable results.

We have benchmarks who really need such a mode!
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 00/22] Add support for GuC-based SLPC

[Martin Peres]

On 26/01/16 19:00, Daniel Vetter wrote:
> On Tue, Jan 26, 2016 at 07:45:42AM -0800, Jesse Barnes wrote:
>> On 01/22/2016 09:00 AM, Daniel Vetter wrote:
>>> On Wed, Jan 20, 2016 at 06:26:02PM -0800, tom.orourke@intel.com wrote:
>>>> From: Tom O'Rourke <Tom.O'Rourke@intel.com>
>
> I'd say we need to keep the boost-deboost stuff alive, e.g. by manually
> telling guc that the we want different limits, then resetting those limits
> again after the boost is done. Same for fast idling - kernel simply has a
> better idea if anyone is about to submit more work (we have execbuf hints
> for specific workloads like libva).

Since there is soon to be a GPU scheduler, the GuC could get this 
information already, right? Unless you are talking about having mesa 
signal when it starts creating a new batchbuffer and you would like the 
GPU to prepare to ramp-up.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 00/22] Add support for GuC-based SLPC

[Daniel Vetter]

On Tue, Feb 09, 2016 at 02:08:23PM +0200, Martin Peres wrote:
> On 26/01/16 19:00, Daniel Vetter wrote:
> >On Tue, Jan 26, 2016 at 07:45:42AM -0800, Jesse Barnes wrote:
> >>On 01/22/2016 09:00 AM, Daniel Vetter wrote:
> >>>On Wed, Jan 20, 2016 at 06:26:02PM -0800, tom.orourke@intel.com wrote:
> >>>>From: Tom O'Rourke <Tom.O'Rourke@intel.com>
> >
> >I'd say we need to keep the boost-deboost stuff alive, e.g. by manually
> >telling guc that the we want different limits, then resetting those limits
> >again after the boost is done. Same for fast idling - kernel simply has a
> >better idea if anyone is about to submit more work (we have execbuf hints
> >for specific workloads like libva).
> 
> Since there is soon to be a GPU scheduler, the GuC could get this
> information already, right? Unless you are talking about having mesa signal
> when it starts creating a new batchbuffer and you would like the GPU to
> prepare to ramp-up.

We don't tell the guc when we're stalling for a batch, so no it doesn't
know. The entire desing seems to center around the idea of just aiming for
some average fps, which is silly for spikey workloads. I assuming that
without some other magic we'll still need explicit boosting and
deboosting.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 00/22] Add support for GuC-based SLPC

[Martin Peres]

On 10/02/16 09:37, Daniel Vetter wrote:
> On Tue, Feb 09, 2016 at 02:08:23PM +0200, Martin Peres wrote:
>> On 26/01/16 19:00, Daniel Vetter wrote:
>>> On Tue, Jan 26, 2016 at 07:45:42AM -0800, Jesse Barnes wrote:
>>>> On 01/22/2016 09:00 AM, Daniel Vetter wrote:
>>>>> On Wed, Jan 20, 2016 at 06:26:02PM -0800, tom.orourke@intel.com wrote:
>>>>>> From: Tom O'Rourke <Tom.O'Rourke@intel.com>
>>>
>>> I'd say we need to keep the boost-deboost stuff alive, e.g. by manually
>>> telling guc that the we want different limits, then resetting those limits
>>> again after the boost is done. Same for fast idling - kernel simply has a
>>> better idea if anyone is about to submit more work (we have execbuf hints
>>> for specific workloads like libva).
>>
>> Since there is soon to be a GPU scheduler, the GuC could get this
>> information already, right? Unless you are talking about having mesa signal
>> when it starts creating a new batchbuffer and you would like the GPU to
>> prepare to ramp-up.
>
> We don't tell the guc when we're stalling for a batch, so no it doesn't
> know. The entire desing seems to center around the idea of just aiming for
> some average fps, which is silly for spikey workloads. I assuming that
> without some other magic we'll still need explicit boosting and
> deboosting.

Right, the needs for a desktop environment are not taken into account 
here. I guess this is really because of Android which is likely using 
planes for compositing so the only problem the GuC developers are trying 
to solve is to guarantee 60 FPS while playing games while saving as much 
power as possible.

Also, SKIA (GPU accel for Chrome) uses the GPU very little so I guess it 
is not really helping the browser case in the mind of the GuC developers.

While the SLPC is IMO the right approach for fast reclocking and tying 
power gating, scheduling and power management together, it needs to 
allow the kernel to give hints and it should listen to them!

Battery life should not be optimised at the ms level, but more at the 
second level. This means that spiky loads should be able to use the 
boost frequencies (if allowed to by the kernel) but the GPU should ramp 
down more or less quickly as the task drags on in order to meet the 
average power consumption target.

May I also repeat that all this really have to be bypassable for 
benchmarking, no boost, fixed frequencies as requested by the kernel and 
a counter to report the number of throttling events at the GPU level. 
Without this, the performance team just cannot do its work properly.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 00/22] Add support for GuC-based SLPC

[Zanoni, Paulo R]

Em Ter, 2016-02-09 às 12:33 +0530, Kamble, Sagar A escreveu:
> Hi Paulo,
> 
> Thanks for comments.
> 1. Will make change related to #define for number of pipes and
> remove 
> the unnecessary ones.
> 2. vrefresh is almost same as "clock/(htotal*vtotal) if we round up 
> later. Will keep it vrefresh for now.

I was under the impression that the VSyncFTUsec field wanted the more
precise solution. Maybe Tom or someone else could clarify this to us.

> 
> Thanks
> Sagar
> 
> 
> On 2/4/2016 1:55 AM, Zanoni, Paulo R wrote:
> > Em Qua, 2016-01-20 às 18:26 -0800, tom.orourke@intel.com escreveu:
> > > From: Tom O'Rourke <Tom.O'Rourke@intel.com>
> > > 
> > > SLPC (Single Loop Power Controller) is a replacement for
> > > some host-based power management features.  The SLPC
> > > implemenation runs in firmware on GuC.
> > > 
> > > This series is a first request for comments.  This series
> > > is not expected to be merged.  After changes based on
> > > comments, a later patch series will be sent for merging.
> > >   
> > > This series has been tested with SKL guc firmware
> > > versions 4.3 and 4.7.  The graphics power management
> > > features in SLPC in those versions are DFPS (Dynamic FPS),
> > > Turbo, and DCC (Duty Cycle Control).  DFPS adjusts
> > > requested graphics frequency to maintain target framerate.
> > > Turbo adjusts requested graphics frequency to maintain
> > > target GT busyness.  DCC adjusts requested graphics
> > > frequency and stalls guc-scheduler to maintain actual
> > > graphics frequency in efficient range.
> > > 
> > > Patch 1/22 is included ihere for convenience and should be
> > > part of an earlier series.  SLPC assumes guc firmware has
> > > been loaded and GuC submission is enabled.
> > > 
> > > Patch 22/22 sets the flag to enable SLPC on SKL.  Without
> > > this patch, the previous patches should have no effect.
> > > 
> > > VIZ-6773, VIZ-6889
> > Hi
> > 
> > Some high-level comments for the whole series:
> > 
> > In many places there are 8 spaces instead of tabs.
> > 
> > There are also some lines containing just a tab character.
> > 
> > Lots of Yoda conditions: if (constant == variable).
> > 
> > Some functions would get much simpler if they had early returns.
> > 
> > I certainly wouldn't complain if you also extracted the relevant
> > rps/powersave code out of intel_pm.c to its own file with a nice
> > documentation. Of course, this could be done after the series.
> > 
> > Lots of ignored return values. It seems the inner functions all
> > check
> > for errors and return them to their callers, but the top-most
> > functions
> > added by the series just ignore the errors. See my previous comment
> > on
> > patch 14 about this for suggestions.
> > 
> > There are no checks for GuC version here. We know that the SLPC ABI
> > is
> > not stable and can change in new firmware versions, so I expect the
> > SLPC code to only run if it finds the specific _whitelisted_ GuC
> > versions. No "if (version >= num) use_slpc=true;", please.
> > 
> > I'm not 100% sure, but from what I could understand, it seems I'll
> > get
> > a broken machine with no RPS at all if I don't have the GuC
> > firmware
> > files - or if the GuC version is not the expected. IMHO this is a
> > regression since I currently don't have any firmware files and my
> > SKL
> > machine works.
> > 
> > I see most of the functions are protected by "HAS_SLPC". Usually
> > HAS_SOMETHING is used for hardware features and is expected to be
> > constant on platforms. I suggest you to add a possible driver
> > parameter
> > such as i915.enable_slpc, and also add a new "intel_slpc_enabled()"
> > or
> > "intel_using_slpc()" function and replace all the HAS_SLPC checks
> > with
> > these. This way, we'll be able to easily disable SLPC in case we
> > don't
> > want it (such as due to a regression) or if there's no firmware or
> > incorrect firmware version, and revert back to the old (current)
> > behavior of driver-based turbo. The only HAS_SLPC check should be
> > during SLPC initialization. Having an easy way to enable/disable
> > SLPC
> > will also be immensely helpful when we start running workloads to
> > decide if we want to enable it.
> > 
> > You could even go further and make the i915.enable_slpc param be a
> > mask
> > where it's possible to select each sub-feature individually (dfps,
> > turbo, dcc).
> > 
> > Some of the checks for shared_data_obj could also become calls to
> > an
> > inline function with a nice name such as intel_slpc_enabled() or
> > something else.
> > 
> > I see there's a specific pattern on the places that call
> > host2guc_action(). Perhaps we could move that common code to its
> > own
> > function? It would also be nice to add a name to the 0xFF mask that
> > we
> > return - and that gets ignored at the end of the call chains.
> > 
> > Patch 5 needs a commit message. Actually, when reviewing patch 4 I
> > thought it had broken RC6, until I saw patch 5, so maybe you could
> > just
> > squash both commits into one.
> > 
> > On patch 13, those defines such as MAX_INTEL_PIPES are weird and
> > confusing (why do we check if they were already defined?),
> > especially
> > since we already have I915_MAX_PIPES. And the only value that is
> > actually used is MAX_NUM_OF_PIPE. I would vote for you to only keep
> > this define, and prefix it with SLPC, such as SLPC_NUM_OF_PIPES (or
> > any
> > other better name).
> > 
> > On patches 14/15, is mode->vrefresh accurate enough? Don't we want
> > the
> > more-precise "clock/(htotal*vtotal)" value?
> > 
> > On patch 17. I'm not an expert here, but I'm not sure if we can
> > call
> > kmap_atomic and then do those seq_printf calls without kunmap.
> > 
> > Thanks,
> > Paulo
> > 
> > > Dave Gordon (1):
> > >    drm/i915: Enable GuC submission, where supported
> > > 
> > > Sagar Arun Kamble (4):
> > >    drm/i915/slpc: Enable/Disable RC6 in SLPC flows
> > >    drm/i915/slpc: Add Display mode event related data structures
> > >    drm/i915/slpc: Notification of Display mode change
> > >    drm/i915/slpc: Notification of Refresh Rate change
> > > 
> > > Tom O'Rourke (17):
> > >    drm/i915/slpc: Add has_slpc capability flag
> > >    drm/i915/slpc: Expose guc functions for use with SLPC
> > >    drm/i915/slpc: Use intel_slpc_* functions if supported
> > >    drm/i915/slpc: If using SLPC, do not set frequency
> > >    drm/i915/slpc: Enable SLPC in guc if supported
> > >    drm/i915/slpc: Allocate/Release/Initialize SLPC shared data
> > >    drm/i915/slpc: Setup rps frequency values during SLPC init
> > >    drm/i915/slpc: Update current requested frequency
> > >    drm/i915/slpc: Send reset event
> > >    drm/i915/slpc: Send shutdown event
> > >    drm/i915/slpc: Add slpc_status enum values
> > >    drm/i915/slpc: Add i915_slpc_info to debugfs
> > >    drm/i915/slpc: Add dfps task info to i915_slpc_info
> > >    drm/i915/slpc: Add parameter unset/set/get functions
> > >    drm/i915/slpc: Add slpc support for max/min freq
> > >    drm/i915/slpc: Add enable/disable debugfs for slpc
> > >    drm/i915/slpc: Add has_slpc to skylake info
> > > 
> > >   drivers/gpu/drm/i915/Makefile              |   5 +-
> > >   drivers/gpu/drm/i915/i915_debugfs.c        | 436
> > > +++++++++++++++++++++++++
> > >   drivers/gpu/drm/i915/i915_drv.c            |   1 +
> > >   drivers/gpu/drm/i915/i915_drv.h            |   2 +
> > >   drivers/gpu/drm/i915/i915_guc_submission.c |   6 +-
> > >   drivers/gpu/drm/i915/i915_params.c         |   4 +-
> > >   drivers/gpu/drm/i915/i915_sysfs.c          |  10 +
> > >   drivers/gpu/drm/i915/intel_display.c       |   2 +
> > >   drivers/gpu/drm/i915/intel_dp.c            |   2 +
> > >   drivers/gpu/drm/i915/intel_drv.h           |   1 +
> > >   drivers/gpu/drm/i915/intel_guc.h           |   7 +
> > >   drivers/gpu/drm/i915/intel_guc_loader.c    |   3 +
> > >   drivers/gpu/drm/i915/intel_pm.c            |  43 ++-
> > >   drivers/gpu/drm/i915/intel_slpc.c          | 499
> > > +++++++++++++++++++++++++++++
> > >   drivers/gpu/drm/i915/intel_slpc.h          | 207 ++++++++++++
> > >   15 files changed, 1210 insertions(+), 18 deletions(-)
> > >   create mode 100644 drivers/gpu/drm/i915/intel_slpc.c
> > >   create mode 100644 drivers/gpu/drm/i915/intel_slpc.h
> > > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx