Re: [Intel-gfx] [PATCH 06/21] drm/i915/guc: Use guc_class instead of engine_class in fw interface

2018-09-06 Thread Joonas Lahtinen
Quoting Michal Wajdeczko (2018-08-29 22:10:40)
> Until now the GuC and HW engine class has been the same, which allowed
> us to use them interchangeable. But it is better to start doing the
> right thing and use the GuC definitions for the firmware interface.
> 
> We also keep the same class id in the ctx descriptor to be able to have
> the same values in the driver and firmware logs.
> 
> Signed-off-by: Michel Thierry 
> Signed-off-by: Rodrigo Vivi 
> Signed-off-by: Michal Wajdeczko 
> Cc: Daniele Ceraolo Spurio 
> Cc: Michel Thierry 
> Cc: Lucas De Marchi 
> Cc: Tomasz Lis 



> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -249,7 +249,15 @@ static inline bool need_preempt(const struct 
> intel_engine_cs *engine,
>  
> /* TODO: decide what to do with SW counter (bits 55-60) */
>  
> -   desc |= (u64)engine->class << GEN11_ENGINE_CLASS_SHIFT;
> +   /*
> +* Although GuC will never see this upper part as it fills
> +* its own descriptor, using the guc_class here will help keep
> +* the i915 and firmware logs in sync.
> +*/
> +   if (HAS_GUC_SCHED(ctx->i915))
> +   desc |= (u64)engine->guc_class << 
> GEN11_ENGINE_CLASS_SHIFT;
> +   else
> +   desc |= (u64)engine->class << 
> GEN11_ENGINE_CLASS_SHIFT;
> /* bits 61-63 
> */

I'm fairly confident I've given this review comment long ago, but here
goes again.

The new member name should just be hw_class or something else agnostic,
and the branching of using ELSP vs. GuC identifier should happen at the engine
init time (or at another sweet spot). And then the actual write should
be unconditionally with the hw_class value.

We should not be adding GuC specifics to intel_lrc.c, I think.

Regards, Joonas
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 06/21] drm/i915/guc: Use guc_class instead of engine_class in fw interface

2018-08-30 Thread Daniele Ceraolo Spurio



On 29/08/18 17:16, Lionel Landwerlin wrote:

On 29/08/2018 20:58, Michel Thierry wrote:

+Lionel
(please see below as this touches the lrca format & relates to OA 
reporting too)


On 8/29/2018 12:10 PM, Michal Wajdeczko wrote:

Until now the GuC and HW engine class has been the same, which allowed
us to use them interchangeable. But it is better to start doing the
right thing and use the GuC definitions for the firmware interface.

We also keep the same class id in the ctx descriptor to be able to have
the same values in the driver and firmware logs.

Signed-off-by: Michel Thierry 
Signed-off-by: Rodrigo Vivi 
Signed-off-by: Michal Wajdeczko 
Cc: Daniele Ceraolo Spurio 
Cc: Michel Thierry 
Cc: Lucas De Marchi 
Cc: Tomasz Lis 
---
  drivers/gpu/drm/i915/intel_engine_cs.c  | 13 +
  drivers/gpu/drm/i915/intel_guc_fwif.h   |  7 +++
  drivers/gpu/drm/i915/intel_lrc.c    | 10 +-
  drivers/gpu/drm/i915/intel_ringbuffer.h |  2 ++
  4 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c 
b/drivers/gpu/drm/i915/intel_engine_cs.c

index 1a34e8f..bc81354 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -85,6 +85,7 @@ struct engine_info {
  unsigned int hw_id;
  unsigned int uabi_id;
  u8 class;
+    u8 guc_class;
  u8 instance;
  /* mmio bases table *must* be sorted in reverse gen order */
  struct engine_mmio_base {
@@ -98,6 +99,7 @@ struct engine_info {
  .hw_id = RCS_HW,
  .uabi_id = I915_EXEC_RENDER,
  .class = RENDER_CLASS,
+    .guc_class = GUC_RENDER_CLASS,
  .instance = 0,
  .mmio_bases = {
  { .gen = 1, .base = RENDER_RING_BASE }
@@ -107,6 +109,7 @@ struct engine_info {
  .hw_id = BCS_HW,
  .uabi_id = I915_EXEC_BLT,
  .class = COPY_ENGINE_CLASS,
+    .guc_class = GUC_BLITTER_CLASS,
  .instance = 0,
  .mmio_bases = {
  { .gen = 6, .base = BLT_RING_BASE }
@@ -116,6 +119,7 @@ struct engine_info {
  .hw_id = VCS_HW,
  .uabi_id = I915_EXEC_BSD,
  .class = VIDEO_DECODE_CLASS,
+    .guc_class = GUC_VIDEO_CLASS,
  .instance = 0,
  .mmio_bases = {
  { .gen = 11, .base = GEN11_BSD_RING_BASE },
@@ -127,6 +131,7 @@ struct engine_info {
  .hw_id = VCS2_HW,
  .uabi_id = I915_EXEC_BSD,
  .class = VIDEO_DECODE_CLASS,
+    .guc_class = GUC_VIDEO_CLASS,
  .instance = 1,
  .mmio_bases = {
  { .gen = 11, .base = GEN11_BSD2_RING_BASE },
@@ -137,6 +142,7 @@ struct engine_info {
  .hw_id = VCS3_HW,
  .uabi_id = I915_EXEC_BSD,
  .class = VIDEO_DECODE_CLASS,
+    .guc_class = GUC_VIDEO_CLASS,
  .instance = 2,
  .mmio_bases = {
  { .gen = 11, .base = GEN11_BSD3_RING_BASE }
@@ -146,6 +152,7 @@ struct engine_info {
  .hw_id = VCS4_HW,
  .uabi_id = I915_EXEC_BSD,
  .class = VIDEO_DECODE_CLASS,
+    .guc_class = GUC_VIDEO_CLASS,
  .instance = 3,
  .mmio_bases = {
  { .gen = 11, .base = GEN11_BSD4_RING_BASE }
@@ -155,6 +162,7 @@ struct engine_info {
  .hw_id = VECS_HW,
  .uabi_id = I915_EXEC_VEBOX,
  .class = VIDEO_ENHANCEMENT_CLASS,
+    .guc_class = GUC_VIDEOENHANCE_CLASS,
  .instance = 0,
  .mmio_bases = {
  { .gen = 11, .base = GEN11_VEBOX_RING_BASE },
@@ -165,6 +173,7 @@ struct engine_info {
  .hw_id = VECS2_HW,
  .uabi_id = I915_EXEC_VEBOX,
  .class = VIDEO_ENHANCEMENT_CLASS,
+    .guc_class = GUC_VIDEOENHANCE_CLASS,
  .instance = 1,
  .mmio_bases = {
  { .gen = 11, .base = GEN11_VEBOX2_RING_BASE }
@@ -276,6 +285,9 @@ static void __sprint_engine_name(char *name, 
const struct engine_info *info)

  if (GEM_WARN_ON(info->class > MAX_ENGINE_CLASS))
  return -EINVAL;
  +    if (GEM_WARN_ON(info->guc_class >= GUC_MAX_ENGINE_CLASSES))
+    return -EINVAL;
+
  if (GEM_WARN_ON(info->instance > MAX_ENGINE_INSTANCE))
  return -EINVAL;
  @@ -291,6 +303,7 @@ static void __sprint_engine_name(char *name, 
const struct engine_info *info)

  engine->i915 = dev_priv;
  __sprint_engine_name(engine->name, info);
  engine->hw_id = engine->guc_id = info->hw_id;
+    engine->guc_class = info->guc_class;
  engine->mmio_base = __engine_mmio_base(dev_priv, 
info->mmio_bases);

  engine->class = info->class;
  engine->instance = info->instance;
diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h 
b/drivers/gpu/drm/i915/intel_guc_fwif.h

index 963da91..5b7a05b 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -39,6 +39,13 @@
  #define GUC_VIDEO_ENGINE2    4
  #define GUC_MAX_ENGINES_NUM    (GUC_VIDEO_ENGINE2 + 1)
  +#define GUC_RENDER_CLASS    0
+#define 

Re: [Intel-gfx] [PATCH 06/21] drm/i915/guc: Use guc_class instead of engine_class in fw interface

2018-08-30 Thread Lionel Landwerlin

On 30/08/2018 14:29, Lis, Tomasz wrote:



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

On 29/08/2018 20:58, Michel Thierry wrote:

+Lionel
(please see below as this touches the lrca format & relates to OA 
reporting too)


On 8/29/2018 12:10 PM, Michal Wajdeczko wrote:

Until now the GuC and HW engine class has been the same, which allowed
us to use them interchangeable. But it is better to start doing the
right thing and use the GuC definitions for the firmware interface.

We also keep the same class id in the ctx descriptor to be able to 
have

the same values in the driver and firmware logs.

Signed-off-by: Michel Thierry 
Signed-off-by: Rodrigo Vivi 
Signed-off-by: Michal Wajdeczko 
Cc: Daniele Ceraolo Spurio 
Cc: Michel Thierry 
Cc: Lucas De Marchi 
Cc: Tomasz Lis 

Tested-by: Tomasz Lis 

---
  drivers/gpu/drm/i915/intel_engine_cs.c  | 13 +
  drivers/gpu/drm/i915/intel_guc_fwif.h   |  7 +++
  drivers/gpu/drm/i915/intel_lrc.c    | 10 +-
  drivers/gpu/drm/i915/intel_ringbuffer.h |  2 ++
  4 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c 
b/drivers/gpu/drm/i915/intel_engine_cs.c

index 1a34e8f..bc81354 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -85,6 +85,7 @@ struct engine_info {
  unsigned int hw_id;
  unsigned int uabi_id;
  u8 class;
+    u8 guc_class;
  u8 instance;
  /* mmio bases table *must* be sorted in reverse gen order */
  struct engine_mmio_base {
@@ -98,6 +99,7 @@ struct engine_info {
  .hw_id = RCS_HW,
  .uabi_id = I915_EXEC_RENDER,
  .class = RENDER_CLASS,
+    .guc_class = GUC_RENDER_CLASS,
  .instance = 0,
  .mmio_bases = {
  { .gen = 1, .base = RENDER_RING_BASE }
@@ -107,6 +109,7 @@ struct engine_info {
  .hw_id = BCS_HW,
  .uabi_id = I915_EXEC_BLT,
  .class = COPY_ENGINE_CLASS,
+    .guc_class = GUC_BLITTER_CLASS,
  .instance = 0,
  .mmio_bases = {
  { .gen = 6, .base = BLT_RING_BASE }
@@ -116,6 +119,7 @@ struct engine_info {
  .hw_id = VCS_HW,
  .uabi_id = I915_EXEC_BSD,
  .class = VIDEO_DECODE_CLASS,
+    .guc_class = GUC_VIDEO_CLASS,
  .instance = 0,
  .mmio_bases = {
  { .gen = 11, .base = GEN11_BSD_RING_BASE },
@@ -127,6 +131,7 @@ struct engine_info {
  .hw_id = VCS2_HW,
  .uabi_id = I915_EXEC_BSD,
  .class = VIDEO_DECODE_CLASS,
+    .guc_class = GUC_VIDEO_CLASS,
  .instance = 1,
  .mmio_bases = {
  { .gen = 11, .base = GEN11_BSD2_RING_BASE },
@@ -137,6 +142,7 @@ struct engine_info {
  .hw_id = VCS3_HW,
  .uabi_id = I915_EXEC_BSD,
  .class = VIDEO_DECODE_CLASS,
+    .guc_class = GUC_VIDEO_CLASS,
  .instance = 2,
  .mmio_bases = {
  { .gen = 11, .base = GEN11_BSD3_RING_BASE }
@@ -146,6 +152,7 @@ struct engine_info {
  .hw_id = VCS4_HW,
  .uabi_id = I915_EXEC_BSD,
  .class = VIDEO_DECODE_CLASS,
+    .guc_class = GUC_VIDEO_CLASS,
  .instance = 3,
  .mmio_bases = {
  { .gen = 11, .base = GEN11_BSD4_RING_BASE }
@@ -155,6 +162,7 @@ struct engine_info {
  .hw_id = VECS_HW,
  .uabi_id = I915_EXEC_VEBOX,
  .class = VIDEO_ENHANCEMENT_CLASS,
+    .guc_class = GUC_VIDEOENHANCE_CLASS,
  .instance = 0,
  .mmio_bases = {
  { .gen = 11, .base = GEN11_VEBOX_RING_BASE },
@@ -165,6 +173,7 @@ struct engine_info {
  .hw_id = VECS2_HW,
  .uabi_id = I915_EXEC_VEBOX,
  .class = VIDEO_ENHANCEMENT_CLASS,
+    .guc_class = GUC_VIDEOENHANCE_CLASS,
  .instance = 1,
  .mmio_bases = {
  { .gen = 11, .base = GEN11_VEBOX2_RING_BASE }
@@ -276,6 +285,9 @@ static void __sprint_engine_name(char *name, 
const struct engine_info *info)

  if (GEM_WARN_ON(info->class > MAX_ENGINE_CLASS))
  return -EINVAL;
  +    if (GEM_WARN_ON(info->guc_class >= GUC_MAX_ENGINE_CLASSES))
+    return -EINVAL;
+
  if (GEM_WARN_ON(info->instance > MAX_ENGINE_INSTANCE))
  return -EINVAL;
  @@ -291,6 +303,7 @@ static void __sprint_engine_name(char *name, 
const struct engine_info *info)

  engine->i915 = dev_priv;
  __sprint_engine_name(engine->name, info);
  engine->hw_id = engine->guc_id = info->hw_id;
+    engine->guc_class = info->guc_class;
  engine->mmio_base = __engine_mmio_base(dev_priv, 
info->mmio_bases);

  engine->class = info->class;
  engine->instance = info->instance;
diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h 
b/drivers/gpu/drm/i915/intel_guc_fwif.h

index 963da91..5b7a05b 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -39,6 +39,13 @@
  #define GUC_VIDEO_ENGINE2    4
  #define GUC_MAX_ENGINES_NUM    

Re: [Intel-gfx] [PATCH 06/21] drm/i915/guc: Use guc_class instead of engine_class in fw interface

2018-08-30 Thread Lis, Tomasz

Uhh, sorry - answered on wrong patch.

Please ignore this one.

-Tomasz

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



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

On 29/08/2018 20:58, Michel Thierry wrote:

+Lionel
(please see below as this touches the lrca format & relates to OA 
reporting too)


On 8/29/2018 12:10 PM, Michal Wajdeczko wrote:

Until now the GuC and HW engine class has been the same, which allowed
us to use them interchangeable. But it is better to start doing the
right thing and use the GuC definitions for the firmware interface.

We also keep the same class id in the ctx descriptor to be able to 
have

the same values in the driver and firmware logs.

Signed-off-by: Michel Thierry 
Signed-off-by: Rodrigo Vivi 
Signed-off-by: Michal Wajdeczko 
Cc: Daniele Ceraolo Spurio 
Cc: Michel Thierry 
Cc: Lucas De Marchi 
Cc: Tomasz Lis 

Tested-by: Tomasz Lis 

---
  drivers/gpu/drm/i915/intel_engine_cs.c  | 13 +
  drivers/gpu/drm/i915/intel_guc_fwif.h   |  7 +++
  drivers/gpu/drm/i915/intel_lrc.c    | 10 +-
  drivers/gpu/drm/i915/intel_ringbuffer.h |  2 ++
  4 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c 
b/drivers/gpu/drm/i915/intel_engine_cs.c

index 1a34e8f..bc81354 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -85,6 +85,7 @@ struct engine_info {
  unsigned int hw_id;
  unsigned int uabi_id;
  u8 class;
+    u8 guc_class;
  u8 instance;
  /* mmio bases table *must* be sorted in reverse gen order */
  struct engine_mmio_base {
@@ -98,6 +99,7 @@ struct engine_info {
  .hw_id = RCS_HW,
  .uabi_id = I915_EXEC_RENDER,
  .class = RENDER_CLASS,
+    .guc_class = GUC_RENDER_CLASS,
  .instance = 0,
  .mmio_bases = {
  { .gen = 1, .base = RENDER_RING_BASE }
@@ -107,6 +109,7 @@ struct engine_info {
  .hw_id = BCS_HW,
  .uabi_id = I915_EXEC_BLT,
  .class = COPY_ENGINE_CLASS,
+    .guc_class = GUC_BLITTER_CLASS,
  .instance = 0,
  .mmio_bases = {
  { .gen = 6, .base = BLT_RING_BASE }
@@ -116,6 +119,7 @@ struct engine_info {
  .hw_id = VCS_HW,
  .uabi_id = I915_EXEC_BSD,
  .class = VIDEO_DECODE_CLASS,
+    .guc_class = GUC_VIDEO_CLASS,
  .instance = 0,
  .mmio_bases = {
  { .gen = 11, .base = GEN11_BSD_RING_BASE },
@@ -127,6 +131,7 @@ struct engine_info {
  .hw_id = VCS2_HW,
  .uabi_id = I915_EXEC_BSD,
  .class = VIDEO_DECODE_CLASS,
+    .guc_class = GUC_VIDEO_CLASS,
  .instance = 1,
  .mmio_bases = {
  { .gen = 11, .base = GEN11_BSD2_RING_BASE },
@@ -137,6 +142,7 @@ struct engine_info {
  .hw_id = VCS3_HW,
  .uabi_id = I915_EXEC_BSD,
  .class = VIDEO_DECODE_CLASS,
+    .guc_class = GUC_VIDEO_CLASS,
  .instance = 2,
  .mmio_bases = {
  { .gen = 11, .base = GEN11_BSD3_RING_BASE }
@@ -146,6 +152,7 @@ struct engine_info {
  .hw_id = VCS4_HW,
  .uabi_id = I915_EXEC_BSD,
  .class = VIDEO_DECODE_CLASS,
+    .guc_class = GUC_VIDEO_CLASS,
  .instance = 3,
  .mmio_bases = {
  { .gen = 11, .base = GEN11_BSD4_RING_BASE }
@@ -155,6 +162,7 @@ struct engine_info {
  .hw_id = VECS_HW,
  .uabi_id = I915_EXEC_VEBOX,
  .class = VIDEO_ENHANCEMENT_CLASS,
+    .guc_class = GUC_VIDEOENHANCE_CLASS,
  .instance = 0,
  .mmio_bases = {
  { .gen = 11, .base = GEN11_VEBOX_RING_BASE },
@@ -165,6 +173,7 @@ struct engine_info {
  .hw_id = VECS2_HW,
  .uabi_id = I915_EXEC_VEBOX,
  .class = VIDEO_ENHANCEMENT_CLASS,
+    .guc_class = GUC_VIDEOENHANCE_CLASS,
  .instance = 1,
  .mmio_bases = {
  { .gen = 11, .base = GEN11_VEBOX2_RING_BASE }
@@ -276,6 +285,9 @@ static void __sprint_engine_name(char *name, 
const struct engine_info *info)

  if (GEM_WARN_ON(info->class > MAX_ENGINE_CLASS))
  return -EINVAL;
  +    if (GEM_WARN_ON(info->guc_class >= GUC_MAX_ENGINE_CLASSES))
+    return -EINVAL;
+
  if (GEM_WARN_ON(info->instance > MAX_ENGINE_INSTANCE))
  return -EINVAL;
  @@ -291,6 +303,7 @@ static void __sprint_engine_name(char *name, 
const struct engine_info *info)

  engine->i915 = dev_priv;
  __sprint_engine_name(engine->name, info);
  engine->hw_id = engine->guc_id = info->hw_id;
+    engine->guc_class = info->guc_class;
  engine->mmio_base = __engine_mmio_base(dev_priv, 
info->mmio_bases);

  engine->class = info->class;
  engine->instance = info->instance;
diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h 
b/drivers/gpu/drm/i915/intel_guc_fwif.h

index 963da91..5b7a05b 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -39,6 +39,13 @@
  

Re: [Intel-gfx] [PATCH 06/21] drm/i915/guc: Use guc_class instead of engine_class in fw interface

2018-08-30 Thread Lis, Tomasz



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

On 29/08/2018 20:58, Michel Thierry wrote:

+Lionel
(please see below as this touches the lrca format & relates to OA 
reporting too)


On 8/29/2018 12:10 PM, Michal Wajdeczko wrote:

Until now the GuC and HW engine class has been the same, which allowed
us to use them interchangeable. But it is better to start doing the
right thing and use the GuC definitions for the firmware interface.

We also keep the same class id in the ctx descriptor to be able to have
the same values in the driver and firmware logs.

Signed-off-by: Michel Thierry 
Signed-off-by: Rodrigo Vivi 
Signed-off-by: Michal Wajdeczko 
Cc: Daniele Ceraolo Spurio 
Cc: Michel Thierry 
Cc: Lucas De Marchi 
Cc: Tomasz Lis 

Tested-by: Tomasz Lis 

---
  drivers/gpu/drm/i915/intel_engine_cs.c  | 13 +
  drivers/gpu/drm/i915/intel_guc_fwif.h   |  7 +++
  drivers/gpu/drm/i915/intel_lrc.c    | 10 +-
  drivers/gpu/drm/i915/intel_ringbuffer.h |  2 ++
  4 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c 
b/drivers/gpu/drm/i915/intel_engine_cs.c

index 1a34e8f..bc81354 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -85,6 +85,7 @@ struct engine_info {
  unsigned int hw_id;
  unsigned int uabi_id;
  u8 class;
+    u8 guc_class;
  u8 instance;
  /* mmio bases table *must* be sorted in reverse gen order */
  struct engine_mmio_base {
@@ -98,6 +99,7 @@ struct engine_info {
  .hw_id = RCS_HW,
  .uabi_id = I915_EXEC_RENDER,
  .class = RENDER_CLASS,
+    .guc_class = GUC_RENDER_CLASS,
  .instance = 0,
  .mmio_bases = {
  { .gen = 1, .base = RENDER_RING_BASE }
@@ -107,6 +109,7 @@ struct engine_info {
  .hw_id = BCS_HW,
  .uabi_id = I915_EXEC_BLT,
  .class = COPY_ENGINE_CLASS,
+    .guc_class = GUC_BLITTER_CLASS,
  .instance = 0,
  .mmio_bases = {
  { .gen = 6, .base = BLT_RING_BASE }
@@ -116,6 +119,7 @@ struct engine_info {
  .hw_id = VCS_HW,
  .uabi_id = I915_EXEC_BSD,
  .class = VIDEO_DECODE_CLASS,
+    .guc_class = GUC_VIDEO_CLASS,
  .instance = 0,
  .mmio_bases = {
  { .gen = 11, .base = GEN11_BSD_RING_BASE },
@@ -127,6 +131,7 @@ struct engine_info {
  .hw_id = VCS2_HW,
  .uabi_id = I915_EXEC_BSD,
  .class = VIDEO_DECODE_CLASS,
+    .guc_class = GUC_VIDEO_CLASS,
  .instance = 1,
  .mmio_bases = {
  { .gen = 11, .base = GEN11_BSD2_RING_BASE },
@@ -137,6 +142,7 @@ struct engine_info {
  .hw_id = VCS3_HW,
  .uabi_id = I915_EXEC_BSD,
  .class = VIDEO_DECODE_CLASS,
+    .guc_class = GUC_VIDEO_CLASS,
  .instance = 2,
  .mmio_bases = {
  { .gen = 11, .base = GEN11_BSD3_RING_BASE }
@@ -146,6 +152,7 @@ struct engine_info {
  .hw_id = VCS4_HW,
  .uabi_id = I915_EXEC_BSD,
  .class = VIDEO_DECODE_CLASS,
+    .guc_class = GUC_VIDEO_CLASS,
  .instance = 3,
  .mmio_bases = {
  { .gen = 11, .base = GEN11_BSD4_RING_BASE }
@@ -155,6 +162,7 @@ struct engine_info {
  .hw_id = VECS_HW,
  .uabi_id = I915_EXEC_VEBOX,
  .class = VIDEO_ENHANCEMENT_CLASS,
+    .guc_class = GUC_VIDEOENHANCE_CLASS,
  .instance = 0,
  .mmio_bases = {
  { .gen = 11, .base = GEN11_VEBOX_RING_BASE },
@@ -165,6 +173,7 @@ struct engine_info {
  .hw_id = VECS2_HW,
  .uabi_id = I915_EXEC_VEBOX,
  .class = VIDEO_ENHANCEMENT_CLASS,
+    .guc_class = GUC_VIDEOENHANCE_CLASS,
  .instance = 1,
  .mmio_bases = {
  { .gen = 11, .base = GEN11_VEBOX2_RING_BASE }
@@ -276,6 +285,9 @@ static void __sprint_engine_name(char *name, 
const struct engine_info *info)

  if (GEM_WARN_ON(info->class > MAX_ENGINE_CLASS))
  return -EINVAL;
  +    if (GEM_WARN_ON(info->guc_class >= GUC_MAX_ENGINE_CLASSES))
+    return -EINVAL;
+
  if (GEM_WARN_ON(info->instance > MAX_ENGINE_INSTANCE))
  return -EINVAL;
  @@ -291,6 +303,7 @@ static void __sprint_engine_name(char *name, 
const struct engine_info *info)

  engine->i915 = dev_priv;
  __sprint_engine_name(engine->name, info);
  engine->hw_id = engine->guc_id = info->hw_id;
+    engine->guc_class = info->guc_class;
  engine->mmio_base = __engine_mmio_base(dev_priv, 
info->mmio_bases);

  engine->class = info->class;
  engine->instance = info->instance;
diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h 
b/drivers/gpu/drm/i915/intel_guc_fwif.h

index 963da91..5b7a05b 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -39,6 +39,13 @@
  #define GUC_VIDEO_ENGINE2    4
  #define GUC_MAX_ENGINES_NUM    (GUC_VIDEO_ENGINE2 + 1)
  +#define 

Re: [Intel-gfx] [PATCH 06/21] drm/i915/guc: Use guc_class instead of engine_class in fw interface

2018-08-29 Thread Lionel Landwerlin

On 29/08/2018 20:58, Michel Thierry wrote:

+Lionel
(please see below as this touches the lrca format & relates to OA 
reporting too)


On 8/29/2018 12:10 PM, Michal Wajdeczko wrote:

Until now the GuC and HW engine class has been the same, which allowed
us to use them interchangeable. But it is better to start doing the
right thing and use the GuC definitions for the firmware interface.

We also keep the same class id in the ctx descriptor to be able to have
the same values in the driver and firmware logs.

Signed-off-by: Michel Thierry 
Signed-off-by: Rodrigo Vivi 
Signed-off-by: Michal Wajdeczko 
Cc: Daniele Ceraolo Spurio 
Cc: Michel Thierry 
Cc: Lucas De Marchi 
Cc: Tomasz Lis 
---
  drivers/gpu/drm/i915/intel_engine_cs.c  | 13 +
  drivers/gpu/drm/i915/intel_guc_fwif.h   |  7 +++
  drivers/gpu/drm/i915/intel_lrc.c    | 10 +-
  drivers/gpu/drm/i915/intel_ringbuffer.h |  2 ++
  4 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c 
b/drivers/gpu/drm/i915/intel_engine_cs.c

index 1a34e8f..bc81354 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -85,6 +85,7 @@ struct engine_info {
  unsigned int hw_id;
  unsigned int uabi_id;
  u8 class;
+    u8 guc_class;
  u8 instance;
  /* mmio bases table *must* be sorted in reverse gen order */
  struct engine_mmio_base {
@@ -98,6 +99,7 @@ struct engine_info {
  .hw_id = RCS_HW,
  .uabi_id = I915_EXEC_RENDER,
  .class = RENDER_CLASS,
+    .guc_class = GUC_RENDER_CLASS,
  .instance = 0,
  .mmio_bases = {
  { .gen = 1, .base = RENDER_RING_BASE }
@@ -107,6 +109,7 @@ struct engine_info {
  .hw_id = BCS_HW,
  .uabi_id = I915_EXEC_BLT,
  .class = COPY_ENGINE_CLASS,
+    .guc_class = GUC_BLITTER_CLASS,
  .instance = 0,
  .mmio_bases = {
  { .gen = 6, .base = BLT_RING_BASE }
@@ -116,6 +119,7 @@ struct engine_info {
  .hw_id = VCS_HW,
  .uabi_id = I915_EXEC_BSD,
  .class = VIDEO_DECODE_CLASS,
+    .guc_class = GUC_VIDEO_CLASS,
  .instance = 0,
  .mmio_bases = {
  { .gen = 11, .base = GEN11_BSD_RING_BASE },
@@ -127,6 +131,7 @@ struct engine_info {
  .hw_id = VCS2_HW,
  .uabi_id = I915_EXEC_BSD,
  .class = VIDEO_DECODE_CLASS,
+    .guc_class = GUC_VIDEO_CLASS,
  .instance = 1,
  .mmio_bases = {
  { .gen = 11, .base = GEN11_BSD2_RING_BASE },
@@ -137,6 +142,7 @@ struct engine_info {
  .hw_id = VCS3_HW,
  .uabi_id = I915_EXEC_BSD,
  .class = VIDEO_DECODE_CLASS,
+    .guc_class = GUC_VIDEO_CLASS,
  .instance = 2,
  .mmio_bases = {
  { .gen = 11, .base = GEN11_BSD3_RING_BASE }
@@ -146,6 +152,7 @@ struct engine_info {
  .hw_id = VCS4_HW,
  .uabi_id = I915_EXEC_BSD,
  .class = VIDEO_DECODE_CLASS,
+    .guc_class = GUC_VIDEO_CLASS,
  .instance = 3,
  .mmio_bases = {
  { .gen = 11, .base = GEN11_BSD4_RING_BASE }
@@ -155,6 +162,7 @@ struct engine_info {
  .hw_id = VECS_HW,
  .uabi_id = I915_EXEC_VEBOX,
  .class = VIDEO_ENHANCEMENT_CLASS,
+    .guc_class = GUC_VIDEOENHANCE_CLASS,
  .instance = 0,
  .mmio_bases = {
  { .gen = 11, .base = GEN11_VEBOX_RING_BASE },
@@ -165,6 +173,7 @@ struct engine_info {
  .hw_id = VECS2_HW,
  .uabi_id = I915_EXEC_VEBOX,
  .class = VIDEO_ENHANCEMENT_CLASS,
+    .guc_class = GUC_VIDEOENHANCE_CLASS,
  .instance = 1,
  .mmio_bases = {
  { .gen = 11, .base = GEN11_VEBOX2_RING_BASE }
@@ -276,6 +285,9 @@ static void __sprint_engine_name(char *name, 
const struct engine_info *info)

  if (GEM_WARN_ON(info->class > MAX_ENGINE_CLASS))
  return -EINVAL;
  +    if (GEM_WARN_ON(info->guc_class >= GUC_MAX_ENGINE_CLASSES))
+    return -EINVAL;
+
  if (GEM_WARN_ON(info->instance > MAX_ENGINE_INSTANCE))
  return -EINVAL;
  @@ -291,6 +303,7 @@ static void __sprint_engine_name(char *name, 
const struct engine_info *info)

  engine->i915 = dev_priv;
  __sprint_engine_name(engine->name, info);
  engine->hw_id = engine->guc_id = info->hw_id;
+    engine->guc_class = info->guc_class;
  engine->mmio_base = __engine_mmio_base(dev_priv, 
info->mmio_bases);

  engine->class = info->class;
  engine->instance = info->instance;
diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h 
b/drivers/gpu/drm/i915/intel_guc_fwif.h

index 963da91..5b7a05b 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -39,6 +39,13 @@
  #define GUC_VIDEO_ENGINE2    4
  #define GUC_MAX_ENGINES_NUM    (GUC_VIDEO_ENGINE2 + 1)
  +#define GUC_RENDER_CLASS    0
+#define GUC_VIDEO_CLASS    1
+#define GUC_VIDEOENHANCE_CLASS 

Re: [Intel-gfx] [PATCH 06/21] drm/i915/guc: Use guc_class instead of engine_class in fw interface

2018-08-29 Thread Michel Thierry

+Lionel
(please see below as this touches the lrca format & relates to OA 
reporting too)


On 8/29/2018 12:10 PM, Michal Wajdeczko wrote:

Until now the GuC and HW engine class has been the same, which allowed
us to use them interchangeable. But it is better to start doing the
right thing and use the GuC definitions for the firmware interface.

We also keep the same class id in the ctx descriptor to be able to have
the same values in the driver and firmware logs.

Signed-off-by: Michel Thierry 
Signed-off-by: Rodrigo Vivi 
Signed-off-by: Michal Wajdeczko 
Cc: Daniele Ceraolo Spurio 
Cc: Michel Thierry 
Cc: Lucas De Marchi 
Cc: Tomasz Lis 
---
  drivers/gpu/drm/i915/intel_engine_cs.c  | 13 +
  drivers/gpu/drm/i915/intel_guc_fwif.h   |  7 +++
  drivers/gpu/drm/i915/intel_lrc.c| 10 +-
  drivers/gpu/drm/i915/intel_ringbuffer.h |  2 ++
  4 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c 
b/drivers/gpu/drm/i915/intel_engine_cs.c
index 1a34e8f..bc81354 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -85,6 +85,7 @@ struct engine_info {
unsigned int hw_id;
unsigned int uabi_id;
u8 class;
+   u8 guc_class;
u8 instance;
/* mmio bases table *must* be sorted in reverse gen order */
struct engine_mmio_base {
@@ -98,6 +99,7 @@ struct engine_info {
.hw_id = RCS_HW,
.uabi_id = I915_EXEC_RENDER,
.class = RENDER_CLASS,
+   .guc_class = GUC_RENDER_CLASS,
.instance = 0,
.mmio_bases = {
{ .gen = 1, .base = RENDER_RING_BASE }
@@ -107,6 +109,7 @@ struct engine_info {
.hw_id = BCS_HW,
.uabi_id = I915_EXEC_BLT,
.class = COPY_ENGINE_CLASS,
+   .guc_class = GUC_BLITTER_CLASS,
.instance = 0,
.mmio_bases = {
{ .gen = 6, .base = BLT_RING_BASE }
@@ -116,6 +119,7 @@ struct engine_info {
.hw_id = VCS_HW,
.uabi_id = I915_EXEC_BSD,
.class = VIDEO_DECODE_CLASS,
+   .guc_class = GUC_VIDEO_CLASS,
.instance = 0,
.mmio_bases = {
{ .gen = 11, .base = GEN11_BSD_RING_BASE },
@@ -127,6 +131,7 @@ struct engine_info {
.hw_id = VCS2_HW,
.uabi_id = I915_EXEC_BSD,
.class = VIDEO_DECODE_CLASS,
+   .guc_class = GUC_VIDEO_CLASS,
.instance = 1,
.mmio_bases = {
{ .gen = 11, .base = GEN11_BSD2_RING_BASE },
@@ -137,6 +142,7 @@ struct engine_info {
.hw_id = VCS3_HW,
.uabi_id = I915_EXEC_BSD,
.class = VIDEO_DECODE_CLASS,
+   .guc_class = GUC_VIDEO_CLASS,
.instance = 2,
.mmio_bases = {
{ .gen = 11, .base = GEN11_BSD3_RING_BASE }
@@ -146,6 +152,7 @@ struct engine_info {
.hw_id = VCS4_HW,
.uabi_id = I915_EXEC_BSD,
.class = VIDEO_DECODE_CLASS,
+   .guc_class = GUC_VIDEO_CLASS,
.instance = 3,
.mmio_bases = {
{ .gen = 11, .base = GEN11_BSD4_RING_BASE }
@@ -155,6 +162,7 @@ struct engine_info {
.hw_id = VECS_HW,
.uabi_id = I915_EXEC_VEBOX,
.class = VIDEO_ENHANCEMENT_CLASS,
+   .guc_class = GUC_VIDEOENHANCE_CLASS,
.instance = 0,
.mmio_bases = {
{ .gen = 11, .base = GEN11_VEBOX_RING_BASE },
@@ -165,6 +173,7 @@ struct engine_info {
.hw_id = VECS2_HW,
.uabi_id = I915_EXEC_VEBOX,
.class = VIDEO_ENHANCEMENT_CLASS,
+   .guc_class = GUC_VIDEOENHANCE_CLASS,
.instance = 1,
.mmio_bases = {
{ .gen = 11, .base = GEN11_VEBOX2_RING_BASE }
@@ -276,6 +285,9 @@ static void __sprint_engine_name(char *name, const struct 
engine_info *info)
if (GEM_WARN_ON(info->class > MAX_ENGINE_CLASS))
return -EINVAL;
  
+	if (GEM_WARN_ON(info->guc_class >= GUC_MAX_ENGINE_CLASSES))

+   return -EINVAL;
+
if (GEM_WARN_ON(info->instance > MAX_ENGINE_INSTANCE))
return -EINVAL;
  
@@ -291,6 +303,7 @@ static void __sprint_engine_name(char *name, const struct engine_info *info)

engine->i915 = dev_priv;
__sprint_engine_name(engine->name, info);
engine->hw_id = engine->guc_id = info->hw_id;
+   engine->guc_class = info->guc_class;
engine->mmio_base = __engine_mmio_base(dev_priv, info->mmio_bases);
engine->class = info->class;
engine->instance = info->instance;
diff --git