[CI 1/3] drm/i915/guc: Unify naming of private GuC action functions

[CI 1/3] drm/i915/guc: Unify naming of private GuC action functions

[Michal Wajdeczko]

We should avoid using guc_log prefix for functions that don't
operate on GuC log, but rather request action from the GuC.
Better to use guc_action prefix.

v2: rebase + naming compromise
v3: rebase

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Michal Winiarski <michal.winiarski@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_log.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
index ae9b256..957f7ed 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/intel_guc_log.c
@@ -38,7 +38,7 @@
  * registers value.
  */
 
-static int guc_log_flush_complete(struct intel_guc *guc)
+static int guc_action_flush_log_complete(struct intel_guc *guc)
 {
 	u32 action[] = {
 		INTEL_GUC_ACTION_LOG_BUFFER_FILE_FLUSH_COMPLETE
@@ -47,7 +47,7 @@ static int guc_log_flush_complete(struct intel_guc *guc)
 	return intel_guc_send(guc, action, ARRAY_SIZE(action));
 }
 
-static int guc_log_flush(struct intel_guc *guc)
+static int guc_action_flush_log(struct intel_guc *guc)
 {
 	u32 action[] = {
 		INTEL_GUC_ACTION_FORCE_LOG_BUFFER_FLUSH,
@@ -57,8 +57,8 @@ static int guc_log_flush(struct intel_guc *guc)
 	return intel_guc_send(guc, action, ARRAY_SIZE(action));
 }
 
-static int guc_log_control(struct intel_guc *guc, bool enable,
-			   bool default_logging, u32 verbosity)
+static int guc_action_control_log(struct intel_guc *guc, bool enable,
+				  bool default_logging, u32 verbosity)
 {
 	union guc_log_control control_val = {
 		{
@@ -449,7 +449,7 @@ static void guc_log_capture_logs(struct intel_guc_log *log)
 	 * time, so get/put should be really quick.
 	 */
 	intel_runtime_pm_get(dev_priv);
-	guc_log_flush_complete(guc);
+	guc_action_flush_log_complete(guc);
 	intel_runtime_pm_put(dev_priv);
 }
 
@@ -526,9 +526,9 @@ int intel_guc_log_level_set(struct intel_guc_log *log, u64 val)
 	}
 
 	intel_runtime_pm_get(dev_priv);
-	ret = guc_log_control(guc, GUC_LOG_LEVEL_IS_VERBOSE(val),
-			      GUC_LOG_LEVEL_IS_ENABLED(val),
-			      GUC_LOG_LEVEL_TO_VERBOSITY(val));
+	ret = guc_action_control_log(guc, GUC_LOG_LEVEL_IS_VERBOSE(val),
+				     GUC_LOG_LEVEL_IS_ENABLED(val),
+				     GUC_LOG_LEVEL_TO_VERBOSITY(val));
 	intel_runtime_pm_put(dev_priv);
 	if (ret) {
 		DRM_DEBUG_DRIVER("guc_log_control action failed %d\n", ret);
@@ -610,7 +610,7 @@ void intel_guc_log_relay_flush(struct intel_guc_log *log)
 	flush_work(&log->relay.flush_work);
 
 	intel_runtime_pm_get(i915);
-	guc_log_flush(guc);
+	guc_action_flush_log(guc);
 	intel_runtime_pm_put(i915);
 
 	/* GuC would have updated log buffer by now, so capture it */
-- 
1.9.1

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

[CI 2/3] drm/i915/guc: Drop union guc_log_control

[Michal Wajdeczko]

Usually we use shift/mask macros for bit field definitions.
Union guc_log_control was not following that pattern.

Additional bonus:

add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-25 (-25)
Function                                     old     new   delta
intel_guc_log_level_set                      388     363     -25

v2: prevent out-of-range verbosity (MichalWi)

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Michal Winiarski <michal.winiarski@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Reviewed-by: MichaĹ Winiarski <michal.winiarski@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_fwif.h | 16 +++++-----------
 drivers/gpu/drm/i915/intel_guc_log.c  | 13 +++++--------
 2 files changed, 10 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h
index 4971685..72941bd 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -534,17 +534,6 @@ struct guc_log_buffer_state {
 	u32 version;
 } __packed;
 
-union guc_log_control {
-	struct {
-		u32 logging_enabled:1;
-		u32 reserved1:3;
-		u32 verbosity:4;
-		u32 default_logging:1;
-		u32 reserved2:23;
-	};
-	u32 value;
-} __packed;
-
 struct guc_ctx_report {
 	u32 report_return_status;
 	u32 reserved1[64];
@@ -603,6 +592,11 @@ enum intel_guc_report_status {
 	INTEL_GUC_REPORT_STATUS_COMPLETE = 0x4,
 };
 
+#define GUC_LOG_CONTROL_LOGGING_ENABLED	(1 << 0)
+#define GUC_LOG_CONTROL_VERBOSITY_SHIFT	4
+#define GUC_LOG_CONTROL_VERBOSITY_MASK	(0xF << GUC_LOG_CONTROL_VERBOSITY_SHIFT)
+#define GUC_LOG_CONTROL_DEFAULT_LOGGING	(1 << 8)
+
 /*
  * The GuC sends its response to a command by overwriting the
  * command in SS0. The response is distinguishable from a command
diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
index 957f7ed..188d390 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/intel_guc_log.c
@@ -60,18 +60,15 @@ static int guc_action_flush_log(struct intel_guc *guc)
 static int guc_action_control_log(struct intel_guc *guc, bool enable,
 				  bool default_logging, u32 verbosity)
 {
-	union guc_log_control control_val = {
-		{
-			.logging_enabled = enable,
-			.verbosity = verbosity,
-			.default_logging = default_logging,
-		},
-	};
 	u32 action[] = {
 		INTEL_GUC_ACTION_UK_LOG_ENABLE_LOGGING,
-		control_val.value
+		(enable ? GUC_LOG_CONTROL_LOGGING_ENABLED : 0) |
+		(verbosity << GUC_LOG_CONTROL_VERBOSITY_SHIFT) |
+		(default_logging ? GUC_LOG_CONTROL_DEFAULT_LOGGING : 0)
 	};
 
+	GEM_BUG_ON(verbosity > GUC_LOG_VERBOSITY_MAX);
+
 	return intel_guc_send(guc, action, ARRAY_SIZE(action));
 }
 
-- 
1.9.1

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

[CI 3/3] drm/i915/guc: Move enable/disable msg functions to GuC header

[Michal Wajdeczko]

While today we are modifying GuC enabled msg mask only in GuC
log, this code should be defined as generic GuC to allow future
code reuse.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Michal Winiarski <michal.winiarski@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>
---
 drivers/gpu/drm/i915/intel_guc.h     | 14 ++++++++++++++
 drivers/gpu/drm/i915/intel_guc_log.c | 26 ++++++++++++--------------
 2 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 9a95d15..13f3d1d 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -155,4 +155,18 @@ static inline int intel_guc_sanitize(struct intel_guc *guc)
 	return 0;
 }
 
+static inline void intel_guc_enable_msg(struct intel_guc *guc, u32 mask)
+{
+	spin_lock_irq(&guc->irq_lock);
+	guc->msg_enabled_mask |= mask;
+	spin_unlock_irq(&guc->irq_lock);
+}
+
+static inline void intel_guc_disable_msg(struct intel_guc *guc, u32 mask)
+{
+	spin_lock_irq(&guc->irq_lock);
+	guc->msg_enabled_mask &= ~mask;
+	spin_unlock_irq(&guc->irq_lock);
+}
+
 #endif
diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
index 188d390..a401f7e 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/intel_guc_log.c
@@ -72,25 +72,23 @@ static int guc_action_control_log(struct intel_guc *guc, bool enable,
 	return intel_guc_send(guc, action, ARRAY_SIZE(action));
 }
 
-static void guc_flush_log_msg_enable(struct intel_guc *guc)
+static inline struct intel_guc *log_to_guc(struct intel_guc_log *log)
 {
-	spin_lock_irq(&guc->irq_lock);
-	guc->msg_enabled_mask |= INTEL_GUC_RECV_MSG_FLUSH_LOG_BUFFER |
-				 INTEL_GUC_RECV_MSG_CRASH_DUMP_POSTED;
-	spin_unlock_irq(&guc->irq_lock);
+	return container_of(log, struct intel_guc, log);
 }
 
-static void guc_flush_log_msg_disable(struct intel_guc *guc)
+static void guc_log_enable_flush_events(struct intel_guc_log *log)
 {
-	spin_lock_irq(&guc->irq_lock);
-	guc->msg_enabled_mask &= ~(INTEL_GUC_RECV_MSG_FLUSH_LOG_BUFFER |
-				   INTEL_GUC_RECV_MSG_CRASH_DUMP_POSTED);
-	spin_unlock_irq(&guc->irq_lock);
+	intel_guc_enable_msg(log_to_guc(log),
+			     INTEL_GUC_RECV_MSG_FLUSH_LOG_BUFFER |
+			     INTEL_GUC_RECV_MSG_CRASH_DUMP_POSTED);
 }
 
-static inline struct intel_guc *log_to_guc(struct intel_guc_log *log)
+static void guc_log_disable_flush_events(struct intel_guc_log *log)
 {
-	return container_of(log, struct intel_guc, log);
+	intel_guc_disable_msg(log_to_guc(log),
+			      INTEL_GUC_RECV_MSG_FLUSH_LOG_BUFFER |
+			      INTEL_GUC_RECV_MSG_CRASH_DUMP_POSTED);
 }
 
 /*
@@ -576,7 +574,7 @@ int intel_guc_log_relay_open(struct intel_guc_log *log)
 
 	mutex_unlock(&log->relay.lock);
 
-	guc_flush_log_msg_enable(log_to_guc(log));
+	guc_log_enable_flush_events(log);
 
 	/*
 	 * When GuC is logging without us relaying to userspace, we're ignoring
@@ -616,7 +614,7 @@ void intel_guc_log_relay_flush(struct intel_guc_log *log)
 
 void intel_guc_log_relay_close(struct intel_guc_log *log)
 {
-	guc_flush_log_msg_disable(log_to_guc(log));
+	guc_log_disable_flush_events(log);
 	flush_work(&log->relay.flush_work);
 
 	mutex_lock(&log->relay.lock);
-- 
1.9.1

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

✓ Fi.CI.BAT: success for series starting with [CI,1/3] drm/i915/guc: Unify naming of private GuC action functions

[Patchwork]

== Series Details ==

Series: series starting with [CI,1/3] drm/i915/guc: Unify naming of private GuC action functions
URL   : https://patchwork.freedesktop.org/series/40311/
State : success

== Summary ==

Series 40311v1 series starting with [CI,1/3] drm/i915/guc: Unify naming of private GuC action functions
https://patchwork.freedesktop.org/api/1.0/series/40311/revisions/1/mbox/

---- Possible new issues:

Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                incomplete -> PASS       (fi-cfl-s2)

---- Known issues:

Test gem_mmap_gtt:
        Subgroup basic-small-bo-tiledx:
                pass       -> FAIL       (fi-gdg-551) fdo#102575

fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575

fi-bdw-5557u     total:285  pass:264  dwarn:0   dfail:0   fail:0   skip:21  time:433s
fi-bdw-gvtdvm    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:443s
fi-blb-e6850     total:285  pass:220  dwarn:1   dfail:0   fail:0   skip:64  time:378s
fi-bsw-n3050     total:285  pass:239  dwarn:0   dfail:0   fail:0   skip:46  time:538s
fi-bwr-2160      total:285  pass:180  dwarn:0   dfail:0   fail:0   skip:105 time:302s
fi-bxt-dsi       total:285  pass:255  dwarn:0   dfail:0   fail:0   skip:30  time:517s
fi-bxt-j4205     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:518s
fi-byt-j1900     total:285  pass:250  dwarn:0   dfail:0   fail:0   skip:35  time:518s
fi-byt-n2820     total:285  pass:246  dwarn:0   dfail:0   fail:0   skip:39  time:508s
fi-cfl-8700k     total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:411s
fi-cfl-s2        total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:573s
fi-cfl-u         total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:511s
fi-cnl-drrs      total:285  pass:254  dwarn:3   dfail:0   fail:0   skip:28  time:520s
fi-elk-e7500     total:285  pass:225  dwarn:1   dfail:0   fail:0   skip:59  time:424s
fi-gdg-551       total:285  pass:176  dwarn:0   dfail:0   fail:1   skip:108 time:318s
fi-glk-1         total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:540s
fi-hsw-4770      total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:404s
fi-ilk-650       total:285  pass:225  dwarn:0   dfail:0   fail:0   skip:60  time:420s
fi-ivb-3520m     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:466s
fi-ivb-3770      total:285  pass:252  dwarn:0   dfail:0   fail:0   skip:33  time:435s
fi-kbl-7500u     total:285  pass:260  dwarn:1   dfail:0   fail:0   skip:24  time:477s
fi-kbl-7567u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:467s
fi-kbl-r         total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:514s
fi-pnv-d510      total:285  pass:219  dwarn:1   dfail:0   fail:0   skip:65  time:652s
fi-skl-6260u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:447s
fi-skl-6600u     total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:533s
fi-skl-6700hq    total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:543s
fi-skl-6700k2    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:509s
fi-skl-6770hq    total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:496s
fi-skl-guc       total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:426s
fi-snb-2520m     total:285  pass:245  dwarn:0   dfail:0   fail:0   skip:40  time:571s
fi-snb-2600      total:285  pass:245  dwarn:0   dfail:0   fail:0   skip:40  time:404s
fi-skl-gvtdvm failed to connect after reboot

9d737cebc219c821989021a3115424165ff7b052 drm-tip: 2018y-03m-20d-14h-56m-05s UTC integration manifest
6c14fdb6a479 drm/i915/guc: Move enable/disable msg functions to GuC header
10c99da01deb drm/i915/guc: Drop union guc_log_control
85b7deb6f543 drm/i915/guc: Unify naming of private GuC action functions

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8419/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✓ Fi.CI.BAT: success for series starting with [CI,1/3] drm/i915/guc: Unify naming of private GuC action functions

[Chris Wilson]

Quoting Patchwork (2018-03-20 19:04:24)
> == Series Details ==
> 
> Series: series starting with [CI,1/3] drm/i915/guc: Unify naming of private GuC action functions
> URL   : https://patchwork.freedesktop.org/series/40311/
> State : success
> 
> == Summary ==
> 
> Series 40311v1 series starting with [CI,1/3] drm/i915/guc: Unify naming of private GuC action functions
> https://patchwork.freedesktop.org/api/1.0/series/40311/revisions/1/mbox/
> 
> ---- Possible new issues:
> 
> Test kms_pipe_crc_basic:
>         Subgroup suspend-read-crc-pipe-a:
>                 incomplete -> PASS       (fi-cfl-s2)
> 
> ---- Known issues:
> 
> Test gem_mmap_gtt:
>         Subgroup basic-small-bo-tiledx:
>                 pass       -> FAIL       (fi-gdg-551) fdo#102575
> 
> fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575

And pushed, thanks for the patch and review.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx