Re: [Intel-gfx] [PATCH 1/3] drm/i915/guc: Support new and improved engine busyness

2023-10-06 Thread John Harrison 

On 10/3/2023 13:58, Umesh Nerlige Ramappa wrote:
On Fri, Sep 22, 2023 at 03:25:08PM -0700, john.c.harri...@intel.com 
wrote:

From: John Harrison 

The GuC has been extended to support a much more friendly engine
busyness interface. So partition the old interface into a 'busy_v1'
space and add 'busy_v2' support alongside. And if v2 is available, use
that in preference to v1. Note that v2 provides extra features over
and above v1 which will be exposed via PMU in subsequent patches.


Since we are thinking of using the existing busyness counter to expose 
the v2 values, we can drop the last sentence from above.




Signed-off-by: John Harrison 
---
drivers/gpu/drm/i915/gt/intel_engine_types.h  |   4 +-
.../gpu/drm/i915/gt/uc/abi/guc_actions_abi.h  |   4 +-
drivers/gpu/drm/i915/gt/uc/intel_guc.h    |  82 ++--
drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c    |  55 ++-
drivers/gpu/drm/i915/gt/uc/intel_guc_ads.h    |   9 +-
drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h   |  23 +-
.../gpu/drm/i915/gt/uc/intel_guc_submission.c | 381 ++
7 files changed, 427 insertions(+), 131 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h 
b/drivers/gpu/drm/i915/gt/intel_engine_types.h

index a7e6775980043..40fd8f984d64b 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
@@ -323,7 +323,7 @@ struct intel_engine_execlists_stats {
ktime_t start;
};

-struct intel_engine_guc_stats {
+struct intel_engine_guc_stats_v1 {
/**
 * @running: Active state of the engine when busyness was last 
sampled.

 */
@@ -603,7 +603,7 @@ struct intel_engine_cs {
struct {
    union {
    struct intel_engine_execlists_stats execlists;
-    struct intel_engine_guc_stats guc;
+    struct intel_engine_guc_stats_v1 guc_v1;
    };


Overall, I would suggest having the renames as a separate patch. Would 
make the review easier.




    /**
diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h 
b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h

index f359bef046e0b..c190a99a36c38 100644
--- a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
+++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
@@ -137,7 +137,9 @@ enum intel_guc_action {
INTEL_GUC_ACTION_DEREGISTER_CONTEXT_DONE = 0x4600,
INTEL_GUC_ACTION_REGISTER_CONTEXT_MULTI_LRC = 0x4601,
INTEL_GUC_ACTION_CLIENT_SOFT_RESET = 0x5507,
-    INTEL_GUC_ACTION_SET_ENG_UTIL_BUFF = 0x550A,
+    INTEL_GUC_ACTION_SET_ENG_UTIL_BUFF_V1 = 0x550A,
+    INTEL_GUC_ACTION_SET_DEVICE_ENGINE_UTILIZATION_V2 = 0x550C,
+    INTEL_GUC_ACTION_SET_FUNCTION_ENGINE_UTILIZATION_V2 = 0x550D,
INTEL_GUC_ACTION_STATE_CAPTURE_NOTIFICATION = 0x8002,
INTEL_GUC_ACTION_NOTIFY_FLUSH_LOG_BUFFER_TO_FILE = 0x8003,
INTEL_GUC_ACTION_NOTIFY_CRASH_DUMP_POSTED = 0x8004,
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h 
b/drivers/gpu/drm/i915/gt/uc/intel_guc.h

index 6c392bad29c19..e6502ab5f049f 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
@@ -226,45 +226,61 @@ struct intel_guc {
struct mutex send_mutex;

/**
- * @timestamp: GT timestamp object that stores a copy of the 
timestamp

- * and adjusts it for overflow using a worker.
+ * @busy: Data used by the different versions of engine busyness 
implementations.

 */
-    struct {
-    /**
- * @lock: Lock protecting the below fields and the engine 
stats.

- */
-    spinlock_t lock;
-
-    /**
- * @gt_stamp: 64 bit extended value of the GT timestamp.
- */
-    u64 gt_stamp;
-
-    /**
- * @ping_delay: Period for polling the GT timestamp for
- * overflow.
- */
-    unsigned long ping_delay;
-
-    /**
- * @work: Periodic work to adjust GT timestamp, engine and
- * context usage for overflows.
- */
-    struct delayed_work work;
-
+    union {
    /**
- * @shift: Right shift value for the gpm timestamp
+ * @v1: Data used by v1 engine busyness implementation. 
Mostly a copy
+ * of the GT timestamp extended to 64 bits and the worker 
for maintaining it.

 */
-    u32 shift;
+    struct {
+    /**
+ * @lock: Lock protecting the below fields and the 
engine stats.

+ */
+    spinlock_t lock;
+
+    /**
+ * @gt_stamp: 64 bit extended value of the GT timestamp.
+ */
+    u64 gt_stamp;
+
+    /**
+ * @ping_delay: Period for polling the GT timestamp for
+ * overflow.
+ */
+    unsigned long ping_delay;
+
+    /**
+ * @work: Periodic work to adjust GT timestamp, engine and
+ * context usage for overflows.
+ */
+    struct delayed_work work;
+
+    /**
+ * @shift: Right shift value for the gpm timestamp
+  

Re: [Intel-gfx] [PATCH 1/3] drm/i915/guc: Support new and improved engine busyness

2023-10-03 Thread Umesh Nerlige Ramappa 

On Fri, Sep 22, 2023 at 03:25:08PM -0700, john.c.harri...@intel.com wrote:

From: John Harrison 

The GuC has been extended to support a much more friendly engine
busyness interface. So partition the old interface into a 'busy_v1'
space and add 'busy_v2' support alongside. And if v2 is available, use
that in preference to v1. Note that v2 provides extra features over
and above v1 which will be exposed via PMU in subsequent patches.


Since we are thinking of using the existing busyness counter to expose 
the v2 values, we can drop the last sentence from above.




Signed-off-by: John Harrison 
---
drivers/gpu/drm/i915/gt/intel_engine_types.h  |   4 +-
.../gpu/drm/i915/gt/uc/abi/guc_actions_abi.h  |   4 +-
drivers/gpu/drm/i915/gt/uc/intel_guc.h|  82 ++--
drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c|  55 ++-
drivers/gpu/drm/i915/gt/uc/intel_guc_ads.h|   9 +-
drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h   |  23 +-
.../gpu/drm/i915/gt/uc/intel_guc_submission.c | 381 ++
7 files changed, 427 insertions(+), 131 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h 
b/drivers/gpu/drm/i915/gt/intel_engine_types.h
index a7e6775980043..40fd8f984d64b 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
@@ -323,7 +323,7 @@ struct intel_engine_execlists_stats {
ktime_t start;
};

-struct intel_engine_guc_stats {
+struct intel_engine_guc_stats_v1 {
/**
 * @running: Active state of the engine when busyness was last sampled.
 */
@@ -603,7 +603,7 @@ struct intel_engine_cs {
struct {
union {
struct intel_engine_execlists_stats execlists;
-   struct intel_engine_guc_stats guc;
+   struct intel_engine_guc_stats_v1 guc_v1;
};


Overall, I would suggest having the renames as a separate patch. Would 
make the review easier.




/**
diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h 
b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
index f359bef046e0b..c190a99a36c38 100644
--- a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
+++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
@@ -137,7 +137,9 @@ enum intel_guc_action {
INTEL_GUC_ACTION_DEREGISTER_CONTEXT_DONE = 0x4600,
INTEL_GUC_ACTION_REGISTER_CONTEXT_MULTI_LRC = 0x4601,
INTEL_GUC_ACTION_CLIENT_SOFT_RESET = 0x5507,
-   INTEL_GUC_ACTION_SET_ENG_UTIL_BUFF = 0x550A,
+   INTEL_GUC_ACTION_SET_ENG_UTIL_BUFF_V1 = 0x550A,
+   INTEL_GUC_ACTION_SET_DEVICE_ENGINE_UTILIZATION_V2 = 0x550C,
+   INTEL_GUC_ACTION_SET_FUNCTION_ENGINE_UTILIZATION_V2 = 0x550D,
INTEL_GUC_ACTION_STATE_CAPTURE_NOTIFICATION = 0x8002,
INTEL_GUC_ACTION_NOTIFY_FLUSH_LOG_BUFFER_TO_FILE = 0x8003,
INTEL_GUC_ACTION_NOTIFY_CRASH_DUMP_POSTED = 0x8004,
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h 
b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
index 6c392bad29c19..e6502ab5f049f 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
@@ -226,45 +226,61 @@ struct intel_guc {
struct mutex send_mutex;

/**
-* @timestamp: GT timestamp object that stores a copy of the timestamp
-* and adjusts it for overflow using a worker.
+* @busy: Data used by the different versions of engine busyness 
implementations.
 */
-   struct {
-   /**
-* @lock: Lock protecting the below fields and the engine stats.
-*/
-   spinlock_t lock;
-
-   /**
-* @gt_stamp: 64 bit extended value of the GT timestamp.
-*/
-   u64 gt_stamp;
-
-   /**
-* @ping_delay: Period for polling the GT timestamp for
-* overflow.
-*/
-   unsigned long ping_delay;
-
-   /**
-* @work: Periodic work to adjust GT timestamp, engine and
-* context usage for overflows.
-*/
-   struct delayed_work work;
-
+   union {
/**
-* @shift: Right shift value for the gpm timestamp
+* @v1: Data used by v1 engine busyness implementation. Mostly 
a copy
+* of the GT timestamp extended to 64 bits and the worker for 
maintaining it.
 */
-   u32 shift;
+   struct {
+   /**
+* @lock: Lock protecting the below fields and the 
engine stats.
+*/
+   spinlock_t lock;
+
+   /**
+* @gt_stamp: 64 bit extended value of the GT timestamp.
+*/
+   u64 gt_stamp;
+
+   /**
+* @ping_delay: Period for polling the GT

[Intel-gfx] [PATCH 1/3] drm/i915/guc: Support new and improved engine busyness

2023-09-22 Thread John . C . Harrison 
From: John Harrison 

The GuC has been extended to support a much more friendly engine
busyness interface. So partition the old interface into a 'busy_v1'
space and add 'busy_v2' support alongside. And if v2 is available, use
that in preference to v1. Note that v2 provides extra features over
and above v1 which will be exposed via PMU in subsequent patches.

Signed-off-by: John Harrison 
---
 drivers/gpu/drm/i915/gt/intel_engine_types.h  |   4 +-
 .../gpu/drm/i915/gt/uc/abi/guc_actions_abi.h  |   4 +-
 drivers/gpu/drm/i915/gt/uc/intel_guc.h|  82 ++--
 drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c|  55 ++-
 drivers/gpu/drm/i915/gt/uc/intel_guc_ads.h|   9 +-
 drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h   |  23 +-
 .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 381 ++
 7 files changed, 427 insertions(+), 131 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h 
b/drivers/gpu/drm/i915/gt/intel_engine_types.h
index a7e6775980043..40fd8f984d64b 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
@@ -323,7 +323,7 @@ struct intel_engine_execlists_stats {
ktime_t start;
 };
 
-struct intel_engine_guc_stats {
+struct intel_engine_guc_stats_v1 {
/**
 * @running: Active state of the engine when busyness was last sampled.
 */
@@ -603,7 +603,7 @@ struct intel_engine_cs {
struct {
union {
struct intel_engine_execlists_stats execlists;
-   struct intel_engine_guc_stats guc;
+   struct intel_engine_guc_stats_v1 guc_v1;
};
 
/**
diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h 
b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
index f359bef046e0b..c190a99a36c38 100644
--- a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
+++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
@@ -137,7 +137,9 @@ enum intel_guc_action {
INTEL_GUC_ACTION_DEREGISTER_CONTEXT_DONE = 0x4600,
INTEL_GUC_ACTION_REGISTER_CONTEXT_MULTI_LRC = 0x4601,
INTEL_GUC_ACTION_CLIENT_SOFT_RESET = 0x5507,
-   INTEL_GUC_ACTION_SET_ENG_UTIL_BUFF = 0x550A,
+   INTEL_GUC_ACTION_SET_ENG_UTIL_BUFF_V1 = 0x550A,
+   INTEL_GUC_ACTION_SET_DEVICE_ENGINE_UTILIZATION_V2 = 0x550C,
+   INTEL_GUC_ACTION_SET_FUNCTION_ENGINE_UTILIZATION_V2 = 0x550D,
INTEL_GUC_ACTION_STATE_CAPTURE_NOTIFICATION = 0x8002,
INTEL_GUC_ACTION_NOTIFY_FLUSH_LOG_BUFFER_TO_FILE = 0x8003,
INTEL_GUC_ACTION_NOTIFY_CRASH_DUMP_POSTED = 0x8004,
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h 
b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
index 6c392bad29c19..e6502ab5f049f 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
@@ -226,45 +226,61 @@ struct intel_guc {
struct mutex send_mutex;
 
/**
-* @timestamp: GT timestamp object that stores a copy of the timestamp
-* and adjusts it for overflow using a worker.
+* @busy: Data used by the different versions of engine busyness 
implementations.
 */
-   struct {
-   /**
-* @lock: Lock protecting the below fields and the engine stats.
-*/
-   spinlock_t lock;
-
-   /**
-* @gt_stamp: 64 bit extended value of the GT timestamp.
-*/
-   u64 gt_stamp;
-
-   /**
-* @ping_delay: Period for polling the GT timestamp for
-* overflow.
-*/
-   unsigned long ping_delay;
-
-   /**
-* @work: Periodic work to adjust GT timestamp, engine and
-* context usage for overflows.
-*/
-   struct delayed_work work;
-
+   union {
/**
-* @shift: Right shift value for the gpm timestamp
+* @v1: Data used by v1 engine busyness implementation. Mostly 
a copy
+* of the GT timestamp extended to 64 bits and the worker for 
maintaining it.
 */
-   u32 shift;
+   struct {
+   /**
+* @lock: Lock protecting the below fields and the 
engine stats.
+*/
+   spinlock_t lock;
+
+   /**
+* @gt_stamp: 64 bit extended value of the GT timestamp.
+*/
+   u64 gt_stamp;
+
+   /**
+* @ping_delay: Period for polling the GT timestamp for
+* overflow.
+*/
+   unsigned long ping_delay;
+
+   /**
+* @work: Periodic work to adjust GT timestamp, engine 
and
+* context usage for overflows.
+