Re: [Intel-gfx] [PATCH 08/21] drm/i915/guc: Make use of the SW counter field in the context descriptor

2018-08-31 Thread Lis, Tomasz



On 2018-08-30 16:15, Lis, Tomasz wrote:



On 2018-08-30 02:08, Lionel Landwerlin wrote:

On 29/08/2018 20:16, Michal Wajdeczko wrote:

The new context descriptor format contains two assignable fields:
the SW Context ID (technically 11 bits, but practically limited to 2032
entries due to some being reserved for future use by the GuC) and the
SW Counter (6 bits).

We don't want to limit ourselves too much in the maximum number of
concurrent contexts we want to allow, so ideally we want to employ
every possible bit available. Unfortunately, a further limitation in
the interface with the GuC means the combination of SW Context ID +
SW Counter has to be unique within the same engine class (as we use
the SW Context ID to index in the GuC stage descriptor pool, and the
Engine Class + SW Counter to index in the 2-dimensional lrc array).
This essentially means we need to somehow encode the engine instance.

Since the BSpec allows 6 bits for engine instance, we use the whole
SW counter for this task. If the limitation of 2032 maximum 
simultaneous

contexts is too restrictive, we can always squeeze things a bit more
(3 extras bits for hw_id, 3 bits for instance) and things will still
work (Gen11 does not instance more than 8 engines of any class).

Another alternative would be to generate the hw_id per HW context
instead of per GEM context, but that has other problems (e.g. maximum
number of user-created contexts would be variable, no relationship
between a GuC principal descriptor and the proxy descriptor it uses, 
...)


Bspec: 12254

Signed-off-by: Oscar Mateo 
Signed-off-by: Rodrigo Vivi 
Signed-off-by: Michal Wajdeczko 
Cc: Joonas Lahtinen 
Cc: Daniele Ceraolo Spurio 
Cc: Michel Thierry 

Tested-by: Tomasz Lis 

---
  drivers/gpu/drm/i915/i915_drv.h | 15 +++
  drivers/gpu/drm/i915/i915_gem_context.c |  5 -
  drivers/gpu/drm/i915/i915_gem_context.h |  2 ++
  drivers/gpu/drm/i915/i915_reg.h |  2 ++
  drivers/gpu/drm/i915/intel_lrc.c    | 12 +---
  5 files changed, 28 insertions(+), 8 deletions(-)

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

index e5b9d3c..34f5495 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1866,14 +1866,21 @@ struct drm_i915_private {
  struct llist_head free_list;
  struct work_struct free_work;
  -    /* The hw wants to have a stable context identifier for the
+    /*
+ * The HW wants to have a stable context identifier for the
   * lifetime of the context (for OA, PASID, faults, etc).
   * This is limited in execlists to 21 bits.
+ * In enhanced execlist (GEN11+) this is limited to 11 bits
+ * (the SW Context ID field) but GuC limits it a bit further
+ * (11 bits - 16) due to some entries being reserved for 
future

+ * use (so the firmware only supports a GuC stage descriptor
+ * pool of 2032 entries).
   */
  struct ida hw_ida;
-#define MAX_CONTEXT_HW_ID (1<<21) /* exclusive */
-#define MAX_GUC_CONTEXT_HW_ID (1 << 20) /* exclusive */
-#define GEN11_MAX_CONTEXT_HW_ID (1<<11) /* exclusive */
+#define MAX_CONTEXT_HW_ID    (1 << 21) /* exclusive */
+#define MAX_GUC_CONTEXT_HW_ID    (1 << 20) /* exclusive */
+#define GEN11_MAX_CONTEXT_HW_ID    (1 << 11) /* exclusive */
+#define GEN11_MAX_CONTEXT_HW_ID_WITH_GUC (GEN11_MAX_CONTEXT_HW_ID - 
16)

  } contexts;
    u32 fdi_rx_config;
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c

index f15a039..e3b500c 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -209,7 +209,10 @@ static int assign_hw_id(struct drm_i915_private 
*dev_priv, unsigned *out)

  unsigned int max;
    if (INTEL_GEN(dev_priv) >= 11) {
-    max = GEN11_MAX_CONTEXT_HW_ID;
+    if (USES_GUC_SUBMISSION(dev_priv))
+    max = GEN11_MAX_CONTEXT_HW_ID_WITH_GUC;
+    else
+    max = GEN11_MAX_CONTEXT_HW_ID;
  } else {
  /*
   * When using GuC in proxy submission, GuC consumes the
diff --git a/drivers/gpu/drm/i915/i915_gem_context.h 
b/drivers/gpu/drm/i915/i915_gem_context.h

index 851dad6..4b87f5d 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.h
+++ b/drivers/gpu/drm/i915/i915_gem_context.h
@@ -154,6 +154,8 @@ struct i915_gem_context {
  struct intel_ring *ring;
  u32 *lrc_reg_state;
  u64 lrc_desc;
+    u32 sw_context_id;
+    u32 sw_counter;
  int pin_count;
    const struct intel_context_ops *ops;
diff --git a/drivers/gpu/drm/i915/i915_reg.h 
b/drivers/gpu/drm/i915/i915_reg.h

index f232178..ea65d7b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3900,6 +3900,8 @@ enum {
  #define GEN8_CTX_ID_WIDTH 21
  #define GEN11_SW_CTX_ID_SHIFT 37
  #define GEN11_SW_CTX_ID_WIDTH 11
+#define 

Re: [Intel-gfx] [PATCH 08/21] drm/i915/guc: Make use of the SW counter field in the context descriptor

2018-08-30 Thread Lis, Tomasz



On 2018-08-30 02:08, Lionel Landwerlin wrote:

On 29/08/2018 20:16, Michal Wajdeczko wrote:

The new context descriptor format contains two assignable fields:
the SW Context ID (technically 11 bits, but practically limited to 2032
entries due to some being reserved for future use by the GuC) and the
SW Counter (6 bits).

We don't want to limit ourselves too much in the maximum number of
concurrent contexts we want to allow, so ideally we want to employ
every possible bit available. Unfortunately, a further limitation in
the interface with the GuC means the combination of SW Context ID +
SW Counter has to be unique within the same engine class (as we use
the SW Context ID to index in the GuC stage descriptor pool, and the
Engine Class + SW Counter to index in the 2-dimensional lrc array).
This essentially means we need to somehow encode the engine instance.

Since the BSpec allows 6 bits for engine instance, we use the whole
SW counter for this task. If the limitation of 2032 maximum simultaneous
contexts is too restrictive, we can always squeeze things a bit more
(3 extras bits for hw_id, 3 bits for instance) and things will still
work (Gen11 does not instance more than 8 engines of any class).

Another alternative would be to generate the hw_id per HW context
instead of per GEM context, but that has other problems (e.g. maximum
number of user-created contexts would be variable, no relationship
between a GuC principal descriptor and the proxy descriptor it uses, 
...)


Bspec: 12254

Signed-off-by: Oscar Mateo 
Signed-off-by: Rodrigo Vivi 
Signed-off-by: Michal Wajdeczko 
Cc: Joonas Lahtinen 
Cc: Daniele Ceraolo Spurio 
Cc: Michel Thierry 

Tested-by: Tomasz Lis 

---
  drivers/gpu/drm/i915/i915_drv.h | 15 +++
  drivers/gpu/drm/i915/i915_gem_context.c |  5 -
  drivers/gpu/drm/i915/i915_gem_context.h |  2 ++
  drivers/gpu/drm/i915/i915_reg.h |  2 ++
  drivers/gpu/drm/i915/intel_lrc.c    | 12 +---
  5 files changed, 28 insertions(+), 8 deletions(-)

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

index e5b9d3c..34f5495 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1866,14 +1866,21 @@ struct drm_i915_private {
  struct llist_head free_list;
  struct work_struct free_work;
  -    /* The hw wants to have a stable context identifier for the
+    /*
+ * The HW wants to have a stable context identifier for the
   * lifetime of the context (for OA, PASID, faults, etc).
   * This is limited in execlists to 21 bits.
+ * In enhanced execlist (GEN11+) this is limited to 11 bits
+ * (the SW Context ID field) but GuC limits it a bit further
+ * (11 bits - 16) due to some entries being reserved for future
+ * use (so the firmware only supports a GuC stage descriptor
+ * pool of 2032 entries).
   */
  struct ida hw_ida;
-#define MAX_CONTEXT_HW_ID (1<<21) /* exclusive */
-#define MAX_GUC_CONTEXT_HW_ID (1 << 20) /* exclusive */
-#define GEN11_MAX_CONTEXT_HW_ID (1<<11) /* exclusive */
+#define MAX_CONTEXT_HW_ID    (1 << 21) /* exclusive */
+#define MAX_GUC_CONTEXT_HW_ID    (1 << 20) /* exclusive */
+#define GEN11_MAX_CONTEXT_HW_ID    (1 << 11) /* exclusive */
+#define GEN11_MAX_CONTEXT_HW_ID_WITH_GUC (GEN11_MAX_CONTEXT_HW_ID - 16)
  } contexts;
    u32 fdi_rx_config;
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c

index f15a039..e3b500c 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -209,7 +209,10 @@ static int assign_hw_id(struct drm_i915_private 
*dev_priv, unsigned *out)

  unsigned int max;
    if (INTEL_GEN(dev_priv) >= 11) {
-    max = GEN11_MAX_CONTEXT_HW_ID;
+    if (USES_GUC_SUBMISSION(dev_priv))
+    max = GEN11_MAX_CONTEXT_HW_ID_WITH_GUC;
+    else
+    max = GEN11_MAX_CONTEXT_HW_ID;
  } else {
  /*
   * When using GuC in proxy submission, GuC consumes the
diff --git a/drivers/gpu/drm/i915/i915_gem_context.h 
b/drivers/gpu/drm/i915/i915_gem_context.h

index 851dad6..4b87f5d 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.h
+++ b/drivers/gpu/drm/i915/i915_gem_context.h
@@ -154,6 +154,8 @@ struct i915_gem_context {
  struct intel_ring *ring;
  u32 *lrc_reg_state;
  u64 lrc_desc;
+    u32 sw_context_id;
+    u32 sw_counter;
  int pin_count;
    const struct intel_context_ops *ops;
diff --git a/drivers/gpu/drm/i915/i915_reg.h 
b/drivers/gpu/drm/i915/i915_reg.h

index f232178..ea65d7b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3900,6 +3900,8 @@ enum {
  #define GEN8_CTX_ID_WIDTH 21
  #define GEN11_SW_CTX_ID_SHIFT 37
  #define GEN11_SW_CTX_ID_WIDTH 11
+#define GEN11_SW_COUNTER_SHIFT 55
+#define GEN11_SW_COUNTER_WIDTH 

Re: [Intel-gfx] [PATCH 08/21] drm/i915/guc: Make use of the SW counter field in the context descriptor

2018-08-29 Thread Lionel Landwerlin

On 29/08/2018 20:16, Michal Wajdeczko wrote:

The new context descriptor format contains two assignable fields:
the SW Context ID (technically 11 bits, but practically limited to 2032
entries due to some being reserved for future use by the GuC) and the
SW Counter (6 bits).

We don't want to limit ourselves too much in the maximum number of
concurrent contexts we want to allow, so ideally we want to employ
every possible bit available. Unfortunately, a further limitation in
the interface with the GuC means the combination of SW Context ID +
SW Counter has to be unique within the same engine class (as we use
the SW Context ID to index in the GuC stage descriptor pool, and the
Engine Class + SW Counter to index in the 2-dimensional lrc array).
This essentially means we need to somehow encode the engine instance.

Since the BSpec allows 6 bits for engine instance, we use the whole
SW counter for this task. If the limitation of 2032 maximum simultaneous
contexts is too restrictive, we can always squeeze things a bit more
(3 extras bits for hw_id, 3 bits for instance) and things will still
work (Gen11 does not instance more than 8 engines of any class).

Another alternative would be to generate the hw_id per HW context
instead of per GEM context, but that has other problems (e.g. maximum
number of user-created contexts would be variable, no relationship
between a GuC principal descriptor and the proxy descriptor it uses, ...)

Bspec: 12254

Signed-off-by: Oscar Mateo 
Signed-off-by: Rodrigo Vivi 
Signed-off-by: Michal Wajdeczko 
Cc: Joonas Lahtinen 
Cc: Daniele Ceraolo Spurio 
Cc: Michel Thierry 
---
  drivers/gpu/drm/i915/i915_drv.h | 15 +++
  drivers/gpu/drm/i915/i915_gem_context.c |  5 -
  drivers/gpu/drm/i915/i915_gem_context.h |  2 ++
  drivers/gpu/drm/i915/i915_reg.h |  2 ++
  drivers/gpu/drm/i915/intel_lrc.c| 12 +---
  5 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e5b9d3c..34f5495 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1866,14 +1866,21 @@ struct drm_i915_private {
struct llist_head free_list;
struct work_struct free_work;
  
-		/* The hw wants to have a stable context identifier for the

+   /*
+* The HW wants to have a stable context identifier for the
 * lifetime of the context (for OA, PASID, faults, etc).
 * This is limited in execlists to 21 bits.
+* In enhanced execlist (GEN11+) this is limited to 11 bits
+* (the SW Context ID field) but GuC limits it a bit further
+* (11 bits - 16) due to some entries being reserved for future
+* use (so the firmware only supports a GuC stage descriptor
+* pool of 2032 entries).
 */
struct ida hw_ida;
-#define MAX_CONTEXT_HW_ID (1<<21) /* exclusive */
-#define MAX_GUC_CONTEXT_HW_ID (1 << 20) /* exclusive */
-#define GEN11_MAX_CONTEXT_HW_ID (1<<11) /* exclusive */
+#define MAX_CONTEXT_HW_ID  (1 << 21) /* exclusive */
+#define MAX_GUC_CONTEXT_HW_ID  (1 << 20) /* exclusive */
+#define GEN11_MAX_CONTEXT_HW_ID(1 << 11) /* exclusive 
*/
+#define GEN11_MAX_CONTEXT_HW_ID_WITH_GUC   (GEN11_MAX_CONTEXT_HW_ID - 16)
} contexts;
  
  	u32 fdi_rx_config;

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index f15a039..e3b500c 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -209,7 +209,10 @@ static int assign_hw_id(struct drm_i915_private *dev_priv, 
unsigned *out)
unsigned int max;
  
  	if (INTEL_GEN(dev_priv) >= 11) {

-   max = GEN11_MAX_CONTEXT_HW_ID;
+   if (USES_GUC_SUBMISSION(dev_priv))
+   max = GEN11_MAX_CONTEXT_HW_ID_WITH_GUC;
+   else
+   max = GEN11_MAX_CONTEXT_HW_ID;
} else {
/*
 * When using GuC in proxy submission, GuC consumes the
diff --git a/drivers/gpu/drm/i915/i915_gem_context.h 
b/drivers/gpu/drm/i915/i915_gem_context.h
index 851dad6..4b87f5d 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.h
+++ b/drivers/gpu/drm/i915/i915_gem_context.h
@@ -154,6 +154,8 @@ struct i915_gem_context {
struct intel_ring *ring;
u32 *lrc_reg_state;
u64 lrc_desc;
+   u32 sw_context_id;
+   u32 sw_counter;
int pin_count;
  
  		const struct intel_context_ops *ops;

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f232178..ea65d7b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3900,6 +3900,8 @@ enum {
  #define GEN8_CTX_ID_WIDTH 21
  #define 

[Intel-gfx] [PATCH 08/21] drm/i915/guc: Make use of the SW counter field in the context descriptor

2018-08-29 Thread Michal Wajdeczko
The new context descriptor format contains two assignable fields:
the SW Context ID (technically 11 bits, but practically limited to 2032
entries due to some being reserved for future use by the GuC) and the
SW Counter (6 bits).

We don't want to limit ourselves too much in the maximum number of
concurrent contexts we want to allow, so ideally we want to employ
every possible bit available. Unfortunately, a further limitation in
the interface with the GuC means the combination of SW Context ID +
SW Counter has to be unique within the same engine class (as we use
the SW Context ID to index in the GuC stage descriptor pool, and the
Engine Class + SW Counter to index in the 2-dimensional lrc array).
This essentially means we need to somehow encode the engine instance.

Since the BSpec allows 6 bits for engine instance, we use the whole
SW counter for this task. If the limitation of 2032 maximum simultaneous
contexts is too restrictive, we can always squeeze things a bit more
(3 extras bits for hw_id, 3 bits for instance) and things will still
work (Gen11 does not instance more than 8 engines of any class).

Another alternative would be to generate the hw_id per HW context
instead of per GEM context, but that has other problems (e.g. maximum
number of user-created contexts would be variable, no relationship
between a GuC principal descriptor and the proxy descriptor it uses, ...)

Bspec: 12254

Signed-off-by: Oscar Mateo 
Signed-off-by: Rodrigo Vivi 
Signed-off-by: Michal Wajdeczko 
Cc: Joonas Lahtinen 
Cc: Daniele Ceraolo Spurio 
Cc: Michel Thierry 
---
 drivers/gpu/drm/i915/i915_drv.h | 15 +++
 drivers/gpu/drm/i915/i915_gem_context.c |  5 -
 drivers/gpu/drm/i915/i915_gem_context.h |  2 ++
 drivers/gpu/drm/i915/i915_reg.h |  2 ++
 drivers/gpu/drm/i915/intel_lrc.c| 12 +---
 5 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e5b9d3c..34f5495 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1866,14 +1866,21 @@ struct drm_i915_private {
struct llist_head free_list;
struct work_struct free_work;
 
-   /* The hw wants to have a stable context identifier for the
+   /*
+* The HW wants to have a stable context identifier for the
 * lifetime of the context (for OA, PASID, faults, etc).
 * This is limited in execlists to 21 bits.
+* In enhanced execlist (GEN11+) this is limited to 11 bits
+* (the SW Context ID field) but GuC limits it a bit further
+* (11 bits - 16) due to some entries being reserved for future
+* use (so the firmware only supports a GuC stage descriptor
+* pool of 2032 entries).
 */
struct ida hw_ida;
-#define MAX_CONTEXT_HW_ID (1<<21) /* exclusive */
-#define MAX_GUC_CONTEXT_HW_ID (1 << 20) /* exclusive */
-#define GEN11_MAX_CONTEXT_HW_ID (1<<11) /* exclusive */
+#define MAX_CONTEXT_HW_ID  (1 << 21) /* exclusive */
+#define MAX_GUC_CONTEXT_HW_ID  (1 << 20) /* exclusive */
+#define GEN11_MAX_CONTEXT_HW_ID(1 << 11) /* exclusive 
*/
+#define GEN11_MAX_CONTEXT_HW_ID_WITH_GUC   (GEN11_MAX_CONTEXT_HW_ID - 16)
} contexts;
 
u32 fdi_rx_config;
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index f15a039..e3b500c 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -209,7 +209,10 @@ static int assign_hw_id(struct drm_i915_private *dev_priv, 
unsigned *out)
unsigned int max;
 
if (INTEL_GEN(dev_priv) >= 11) {
-   max = GEN11_MAX_CONTEXT_HW_ID;
+   if (USES_GUC_SUBMISSION(dev_priv))
+   max = GEN11_MAX_CONTEXT_HW_ID_WITH_GUC;
+   else
+   max = GEN11_MAX_CONTEXT_HW_ID;
} else {
/*
 * When using GuC in proxy submission, GuC consumes the
diff --git a/drivers/gpu/drm/i915/i915_gem_context.h 
b/drivers/gpu/drm/i915/i915_gem_context.h
index 851dad6..4b87f5d 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.h
+++ b/drivers/gpu/drm/i915/i915_gem_context.h
@@ -154,6 +154,8 @@ struct i915_gem_context {
struct intel_ring *ring;
u32 *lrc_reg_state;
u64 lrc_desc;
+   u32 sw_context_id;
+   u32 sw_counter;
int pin_count;
 
const struct intel_context_ops *ops;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f232178..ea65d7b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3900,6 +3900,8 @@ enum {
 #define GEN8_CTX_ID_WIDTH 21
 #define GEN11_SW_CTX_ID_SHIFT 37