Re: [Intel-gfx] [PATCH 26/27] drm/i915/guc: Add GuC kernel doc

2021-08-31 Thread John Harrison 

On 8/25/2021 20:23, Matthew Brost wrote:

Add GuC kernel doc for all structures added thus far for GuC submission
and update the main GuC submission section with the new interface
details.

v2:
  - Drop guc_active.lock DOC
v3:
  (Daniele)
  - Fixup a few kernel doc comments

Signed-off-by: Matthew Brost 
---
  drivers/gpu/drm/i915/gt/intel_context_types.h |  44 +---
  drivers/gpu/drm/i915/gt/uc/intel_guc.h|  19 +++-
  .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 101 ++
  drivers/gpu/drm/i915/i915_request.h   |  18 ++--
  4 files changed, 132 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h 
b/drivers/gpu/drm/i915/gt/intel_context_types.h
index 5285d660eacf..920ed92f4382 100644
--- a/drivers/gpu/drm/i915/gt/intel_context_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
@@ -156,40 +156,52 @@ struct intel_context {
u8 wa_bb_page; /* if set, page num reserved for context workarounds */
  
  	struct {

-   /** lock: protects everything in guc_state */
+   /** @lock: protects everything in guc_state */
spinlock_t lock;
/**
-* sched_state: scheduling state of this context using GuC
+* @sched_state: scheduling state of this context using GuC
 * submission
 */
u32 sched_state;
/*
-* fences: maintains of list of requests that have a submit
-* fence related to GuC submission
+* @fences: maintains a list of requests are currently being

requests *that* are


+* fenced until a GuC operation completes
 */
struct list_head fences;
-   /* GuC context blocked fence */
+   /**
+* @blocked: fence used to signal when the blocking of a
+* contexts submissions is complete.

context's

'submissions are' or 'submission is'?


+*/
struct i915_sw_fence blocked;
-   /* GuC committed requests */
+   /** @number_committed_requests: number of committed requests */
int number_committed_requests;
-   /** requests: active requests on this context */
+   /** @requests: list of active requests on this context */
struct list_head requests;
-   /*
-* GuC priority management
-*/
+   /** @prio: the contexts current guc priority */

context's


u8 prio;
+   /**
+* @prio_count: a counter of the number requests inflight in

in flight


+* each priority bucket
+*/
u32 prio_count[GUC_CLIENT_PRIORITY_NUM];
} guc_state;
  
  	struct {

-   /* GuC LRC descriptor ID */
+   /**
+* @id: unique handle which is used to communicate information
+* with the GuC about this context, protected by

"used to communicate information" seems an odd way to say it. Maybe:
  @id: handle which is used to uniquely identify this context with the 
GuC, protected by...



+* guc->contexts_lock
+*/
u16 id;
-
-   /* GuC LRC descriptor reference count */
+   /**
+* @ref: the number of references to the guc_id, when
+* transitioning in and out of zero protected by
+* guc->contexts_lock
+*/
atomic_t ref;
-
-   /*
-* GuC ID link - in list when unpinned but guc_id still valid 
in GuC
+   /**
+* @link: in guc->guc_id_list when the guc_id has no refs but is
+* still valid, protected by guc->contexts_lock
 */
struct list_head link;
} guc_id;
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h 
b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
index 2e27fe59786b..112dd29a63fe 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
@@ -41,6 +41,10 @@ struct intel_guc {
spinlock_t irq_lock;
unsigned int msg_enabled_mask;
  
+	/**

+* @outstanding_submission_g2h: number of outstanding G2H related to GuC

"G2H responses"?

Maybe even "GuC to Host responses"? Do we explain anywhere at a higher 
level what G2H (or H2G) means?




+* submission, used to determine if the GT is idle
+*/
atomic_t outstanding_submission_g2h;
  
  	struct {

@@ -49,12 +53,16 @@ struct intel_guc {
void (*disable)(struct intel_guc *guc);
} interrupts;
  
-	/*

-* contexts_lock protects the pool of free guc ids and a linked list of
-* guc ids available to be stolen
+   /**
+* @contexts_lock: protects guc_ids,

[Intel-gfx] [PATCH 26/27] drm/i915/guc: Add GuC kernel doc

2021-08-25 Thread Matthew Brost 
Add GuC kernel doc for all structures added thus far for GuC submission
and update the main GuC submission section with the new interface
details.

v2:
 - Drop guc_active.lock DOC
v3:
 (Daniele)
 - Fixup a few kernel doc comments

Signed-off-by: Matthew Brost 
---
 drivers/gpu/drm/i915/gt/intel_context_types.h |  44 +---
 drivers/gpu/drm/i915/gt/uc/intel_guc.h|  19 +++-
 .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 101 ++
 drivers/gpu/drm/i915/i915_request.h   |  18 ++--
 4 files changed, 132 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h 
b/drivers/gpu/drm/i915/gt/intel_context_types.h
index 5285d660eacf..920ed92f4382 100644
--- a/drivers/gpu/drm/i915/gt/intel_context_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
@@ -156,40 +156,52 @@ struct intel_context {
u8 wa_bb_page; /* if set, page num reserved for context workarounds */
 
struct {
-   /** lock: protects everything in guc_state */
+   /** @lock: protects everything in guc_state */
spinlock_t lock;
/**
-* sched_state: scheduling state of this context using GuC
+* @sched_state: scheduling state of this context using GuC
 * submission
 */
u32 sched_state;
/*
-* fences: maintains of list of requests that have a submit
-* fence related to GuC submission
+* @fences: maintains a list of requests are currently being
+* fenced until a GuC operation completes
 */
struct list_head fences;
-   /* GuC context blocked fence */
+   /**
+* @blocked: fence used to signal when the blocking of a
+* contexts submissions is complete.
+*/
struct i915_sw_fence blocked;
-   /* GuC committed requests */
+   /** @number_committed_requests: number of committed requests */
int number_committed_requests;
-   /** requests: active requests on this context */
+   /** @requests: list of active requests on this context */
struct list_head requests;
-   /*
-* GuC priority management
-*/
+   /** @prio: the contexts current guc priority */
u8 prio;
+   /**
+* @prio_count: a counter of the number requests inflight in
+* each priority bucket
+*/
u32 prio_count[GUC_CLIENT_PRIORITY_NUM];
} guc_state;
 
struct {
-   /* GuC LRC descriptor ID */
+   /**
+* @id: unique handle which is used to communicate information
+* with the GuC about this context, protected by
+* guc->contexts_lock
+*/
u16 id;
-
-   /* GuC LRC descriptor reference count */
+   /**
+* @ref: the number of references to the guc_id, when
+* transitioning in and out of zero protected by
+* guc->contexts_lock
+*/
atomic_t ref;
-
-   /*
-* GuC ID link - in list when unpinned but guc_id still valid 
in GuC
+   /**
+* @link: in guc->guc_id_list when the guc_id has no refs but is
+* still valid, protected by guc->contexts_lock
 */
struct list_head link;
} guc_id;
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h 
b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
index 2e27fe59786b..112dd29a63fe 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
@@ -41,6 +41,10 @@ struct intel_guc {
spinlock_t irq_lock;
unsigned int msg_enabled_mask;
 
+   /**
+* @outstanding_submission_g2h: number of outstanding G2H related to GuC
+* submission, used to determine if the GT is idle
+*/
atomic_t outstanding_submission_g2h;
 
struct {
@@ -49,12 +53,16 @@ struct intel_guc {
void (*disable)(struct intel_guc *guc);
} interrupts;
 
-   /*
-* contexts_lock protects the pool of free guc ids and a linked list of
-* guc ids available to be stolen
+   /**
+* @contexts_lock: protects guc_ids, guc_id_list, ce->guc_id.id, and
+* ce->guc_id.ref when transitioning in and out of zero
 */
spinlock_t contexts_lock;
+   /** @guc_ids: used to allocate new guc_ids */
struct ida guc_ids;
+   /**
+* @guc_id_list: list of intel_context with valid guc_ids but no refs
+*/
struct list_head guc_id_list;
 
bool submission_supported;
@@ -70,7 +78,10 @@ struct

Re: [Intel-gfx] [PATCH 26/27] drm/i915/guc: Add GuC kernel doc

2021-08-25 Thread Daniele Ceraolo Spurio 




On 8/18/2021 11:16 PM, Matthew Brost wrote:

Add GuC kernel doc for all structures added thus far for GuC submission
and update the main GuC submission section with the new interface
details.

v2:
  - Drop guc_active.lock DOC

Signed-off-by: Matthew Brost 
---
  drivers/gpu/drm/i915/gt/intel_context_types.h | 44 ++---
  drivers/gpu/drm/i915/gt/uc/intel_guc.h| 19 +++-
  .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 97 ++-
  drivers/gpu/drm/i915/i915_request.h   | 18 ++--
  4 files changed, 128 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h 
b/drivers/gpu/drm/i915/gt/intel_context_types.h
index 66286ce36c84..80bbdc7810f6 100644
--- a/drivers/gpu/drm/i915/gt/intel_context_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
@@ -156,40 +156,52 @@ struct intel_context {
u8 wa_bb_page; /* if set, page num reserved for context workarounds */
  
  	struct {

-   /** lock: protects everything in guc_state */
+   /** @lock: protects everything in guc_state */
spinlock_t lock;
/**
-* sched_state: scheduling state of this context using GuC
+* @sched_state: scheduling state of this context using GuC
 * submission
 */
u32 sched_state;
/*
-* fences: maintains of list of requests that have a submit
-* fence related to GuC submission
+* @fences: maintains a list of requests are currently being
+* fenced until a GuC operation completes
 */
struct list_head fences;
-   /* GuC context blocked fence */
+   /**
+* @blocked_fence: fence used to signal when the blocking of a
+* contexts submissions is complete.
+*/
struct i915_sw_fence blocked_fence;
-   /* GuC committed requests */
+   /** @number_committed_requests: number of committed requests */
int number_committed_requests;
-   /** requests: active requests on this context */
+   /** @requests: list of active requests on this context */
struct list_head requests;
-   /*
-* GuC priority management
-*/
+   /** @prio: the contexts current guc priority */
u8 prio;
+   /**
+* @prio_count: a counter of the number requests inflight in
+* each priority bucket
+*/
u32 prio_count[GUC_CLIENT_PRIORITY_NUM];
} guc_state;
  
  	struct {

-   /* GuC LRC descriptor ID */
+   /**
+* @id: unique handle which is used to communicate information
+* with the GuC about this context, protected by
+* guc->contexts_lock
+*/
u16 id;
-
-   /* GuC LRC descriptor reference count */
+   /**
+* @ref: the number of references to the guc_id, when
+* transitioning in and out of zero protected by
+* guc->contexts_lock
+*/
atomic_t ref;
-
-   /*
-* GuC ID link - in list when unpinned but guc_id still valid 
in GuC
+   /**
+* @link: in guc->guc_id_list when the guc_id has no refs but is
+* still valid, protected by guc->contexts_lock
 */
struct list_head link;
} guc_id;
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h 
b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
index 2e27fe59786b..112dd29a63fe 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
@@ -41,6 +41,10 @@ struct intel_guc {
spinlock_t irq_lock;
unsigned int msg_enabled_mask;
  
+	/**

+* @outstanding_submission_g2h: number of outstanding G2H related to GuC
+* submission, used to determine if the GT is idle
+*/
atomic_t outstanding_submission_g2h;
  
  	struct {

@@ -49,12 +53,16 @@ struct intel_guc {
void (*disable)(struct intel_guc *guc);
} interrupts;
  
-	/*

-* contexts_lock protects the pool of free guc ids and a linked list of
-* guc ids available to be stolen
+   /**
+* @contexts_lock: protects guc_ids, guc_id_list, ce->guc_id.id, and
+* ce->guc_id.ref when transitioning in and out of zero
 */
spinlock_t contexts_lock;
+   /** @guc_ids: used to allocate new guc_ids */
struct ida guc_ids;
+   /**
+* @guc_id_list: list of intel_context with valid guc_ids but no refs
+*/
struct list_head guc_id_list;
  
  	bool submission_supported;

@@ -70,7 +78,10 @@ struct intel_guc

[Intel-gfx] [PATCH 26/27] drm/i915/guc: Add GuC kernel doc

2021-08-19 Thread Matthew Brost 
Add GuC kernel doc for all structures added thus far for GuC submission
and update the main GuC submission section with the new interface
details.

v2:
 - Drop guc_active.lock DOC

Signed-off-by: Matthew Brost 
---
 drivers/gpu/drm/i915/gt/intel_context_types.h | 44 ++---
 drivers/gpu/drm/i915/gt/uc/intel_guc.h| 19 +++-
 .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 97 ++-
 drivers/gpu/drm/i915/i915_request.h   | 18 ++--
 4 files changed, 128 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h 
b/drivers/gpu/drm/i915/gt/intel_context_types.h
index 66286ce36c84..80bbdc7810f6 100644
--- a/drivers/gpu/drm/i915/gt/intel_context_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
@@ -156,40 +156,52 @@ struct intel_context {
u8 wa_bb_page; /* if set, page num reserved for context workarounds */
 
struct {
-   /** lock: protects everything in guc_state */
+   /** @lock: protects everything in guc_state */
spinlock_t lock;
/**
-* sched_state: scheduling state of this context using GuC
+* @sched_state: scheduling state of this context using GuC
 * submission
 */
u32 sched_state;
/*
-* fences: maintains of list of requests that have a submit
-* fence related to GuC submission
+* @fences: maintains a list of requests are currently being
+* fenced until a GuC operation completes
 */
struct list_head fences;
-   /* GuC context blocked fence */
+   /**
+* @blocked_fence: fence used to signal when the blocking of a
+* contexts submissions is complete.
+*/
struct i915_sw_fence blocked_fence;
-   /* GuC committed requests */
+   /** @number_committed_requests: number of committed requests */
int number_committed_requests;
-   /** requests: active requests on this context */
+   /** @requests: list of active requests on this context */
struct list_head requests;
-   /*
-* GuC priority management
-*/
+   /** @prio: the contexts current guc priority */
u8 prio;
+   /**
+* @prio_count: a counter of the number requests inflight in
+* each priority bucket
+*/
u32 prio_count[GUC_CLIENT_PRIORITY_NUM];
} guc_state;
 
struct {
-   /* GuC LRC descriptor ID */
+   /**
+* @id: unique handle which is used to communicate information
+* with the GuC about this context, protected by
+* guc->contexts_lock
+*/
u16 id;
-
-   /* GuC LRC descriptor reference count */
+   /**
+* @ref: the number of references to the guc_id, when
+* transitioning in and out of zero protected by
+* guc->contexts_lock
+*/
atomic_t ref;
-
-   /*
-* GuC ID link - in list when unpinned but guc_id still valid 
in GuC
+   /**
+* @link: in guc->guc_id_list when the guc_id has no refs but is
+* still valid, protected by guc->contexts_lock
 */
struct list_head link;
} guc_id;
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h 
b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
index 2e27fe59786b..112dd29a63fe 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
@@ -41,6 +41,10 @@ struct intel_guc {
spinlock_t irq_lock;
unsigned int msg_enabled_mask;
 
+   /**
+* @outstanding_submission_g2h: number of outstanding G2H related to GuC
+* submission, used to determine if the GT is idle
+*/
atomic_t outstanding_submission_g2h;
 
struct {
@@ -49,12 +53,16 @@ struct intel_guc {
void (*disable)(struct intel_guc *guc);
} interrupts;
 
-   /*
-* contexts_lock protects the pool of free guc ids and a linked list of
-* guc ids available to be stolen
+   /**
+* @contexts_lock: protects guc_ids, guc_id_list, ce->guc_id.id, and
+* ce->guc_id.ref when transitioning in and out of zero
 */
spinlock_t contexts_lock;
+   /** @guc_ids: used to allocate new guc_ids */
struct ida guc_ids;
+   /**
+* @guc_id_list: list of intel_context with valid guc_ids but no refs
+*/
struct list_head guc_id_list;
 
bool submission_supported;
@@ -70,7 +78,10 @@ struct intel_guc {
struct i915_vma