subject:"\[Intel\-gfx\] \[PATCH 2\/6\] drm\/i915\/icl\: Correctly initialize the Gen11 engines"

Re: [Intel-gfx] [PATCH 2/6] drm/i915/icl: Correctly initialize the Gen11 engines

2018-03-07 Thread Tvrtko Ursulin



On 06/03/2018 22:07, Daniele Ceraolo Spurio wrote:

On 02/03/18 08:14, Mika Kuoppala wrote:

From: Oscar Mateo 

Gen11 has up to 4 VCS and up to 2 VECS engines, this patch adds mmio
base definitions for all of them.

Bspec: 20944
Bspec: 7021

v2: Set the correct mmio_base in intel_engines_init_mmio; updating the
base mmio values any later would cause incorrect reads in
i915_gem_sanitize (Michel).

Cc: Tvrtko Ursulin 
Cc: Ceraolo Spurio, Daniele 
Signed-off-by: Oscar Mateo 
Signed-off-by: Michel Thierry 
---
  drivers/gpu/drm/i915/i915_reg.h    |  6 +
  drivers/gpu/drm/i915/intel_engine_cs.c | 44 
+-

  2 files changed, 49 insertions(+), 1 deletion(-)

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

index 258e86eb37d5..45ae05d0fe78 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2345,7 +2345,13 @@ enum i915_power_well_id {
  #define BSD_RING_BASE    0x04000
  #define GEN6_BSD_RING_BASE    0x12000
  #define GEN8_BSD2_RING_BASE    0x1c000
+#define GEN11_BSD_RING_BASE    0x1c
+#define GEN11_BSD2_RING_BASE    0x1c4000
+#define GEN11_BSD3_RING_BASE    0x1d
+#define GEN11_BSD4_RING_BASE    0x1d4000
  #define VEBOX_RING_BASE    0x1a000
+#define GEN11_VEBOX_RING_BASE    0x1c8000
+#define GEN11_VEBOX2_RING_BASE    0x1d8000
  #define BLT_RING_BASE    0x22000
  #define RING_TAIL(base)    _MMIO((base)+0x30)
  #define RING_HEAD(base)    _MMIO((base)+0x34)
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c 
b/drivers/gpu/drm/i915/intel_engine_cs.c

index 3e1107ecb6ee..911fc08658c5 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -123,6 +123,22 @@ static const struct engine_info intel_engines[] = {
  .mmio_base = GEN8_BSD2_RING_BASE,
  .irq_shift = GEN8_VCS2_IRQ_SHIFT,
  },
+    [VCS3] = {
+    .hw_id = VCS3_HW,
+    .uabi_id = I915_EXEC_BSD,
+    .class = VIDEO_DECODE_CLASS,
+    .instance = 2,
+    .mmio_base = GEN11_BSD3_RING_BASE,
+    .irq_shift = 0, /* not used */
+    },
+    [VCS4] = {
+    .hw_id = VCS4_HW,
+    .uabi_id = I915_EXEC_BSD,
+    .class = VIDEO_DECODE_CLASS,
+    .instance = 3,
+    .mmio_base = GEN11_BSD4_RING_BASE,
+    .irq_shift = 0, /* not used */
+    },
  [VECS] = {
  .hw_id = VECS_HW,
  .uabi_id = I915_EXEC_VEBOX,
@@ -131,6 +147,14 @@ static const struct engine_info intel_engines[] = {
  .mmio_base = VEBOX_RING_BASE,
  .irq_shift = GEN8_VECS_IRQ_SHIFT,
  },
+    [VECS2] = {
+    .hw_id = VECS2_HW,
+    .uabi_id = I915_EXEC_VEBOX,
+    .class = VIDEO_ENHANCEMENT_CLASS,
+    .instance = 1,
+    .mmio_base = GEN11_VEBOX2_RING_BASE,
+    .irq_shift = 0, /* not used */
+    },
  };
  /**
@@ -230,7 +254,25 @@ intel_engine_setup(struct drm_i915_private 
*dev_priv,

   class_info->name, info->instance) >=
  sizeof(engine->name));
  engine->hw_id = engine->guc_id = info->hw_id;
-    engine->mmio_base = info->mmio_base;
+    if (INTEL_GEN(dev_priv) >= 11) {
+    switch (engine->id) {
+    case VCS:
+    engine->mmio_base = GEN11_BSD_RING_BASE;
+    break;
+    case VCS2:
+    engine->mmio_base = GEN11_BSD2_RING_BASE;
+    break;
+    case VECS:
+    engine->mmio_base = GEN11_VEBOX_RING_BASE;
+    break;
+    default:
+    /* take the original value for all other engines  */
+    engine->mmio_base = info->mmio_base;
+    break;
+    }


I'm slightly unconvinced by the fact that we have a static table with 
some values and then we use other values here. Maybe we could instead 
use and array of [starting_gen, mmio_base] pairs in the table and derive 
the correct value here? Anyway, since this is not the only place where 
we don't use the mmio_base value from the table (same happens for 
pre-gen6 BSD in intel_init_bsd_ring_buffer) I don't consider this a 
blocking issue. All the values match the specs, so:


Reviewed-by: Daniele Ceraolo Spurio 

I'll probably check how using the table I mentioned above looks like 
after this is merged and I'll send an RFC if it seems nice.


Agreed it is inelegant to diverge. Your idea sounds potentially OK. Or 
even just duplicate the whole table for < gen6 and >= gen11 - they are 
not too big.


Regards,

Tvrtko

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

Re: [Intel-gfx] [PATCH 2/6] drm/i915/icl: Correctly initialize the Gen11 engines

2018-03-06 Thread Daniele Ceraolo Spurio




On 02/03/18 08:14, Mika Kuoppala wrote:

From: Oscar Mateo 

Gen11 has up to 4 VCS and up to 2 VECS engines, this patch adds mmio
base definitions for all of them.

Bspec: 20944
Bspec: 7021

v2: Set the correct mmio_base in intel_engines_init_mmio; updating the
base mmio values any later would cause incorrect reads in
i915_gem_sanitize (Michel).

Cc: Tvrtko Ursulin 
Cc: Ceraolo Spurio, Daniele 
Signed-off-by: Oscar Mateo 
Signed-off-by: Michel Thierry 
---
  drivers/gpu/drm/i915/i915_reg.h|  6 +
  drivers/gpu/drm/i915/intel_engine_cs.c | 44 +-
  2 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 258e86eb37d5..45ae05d0fe78 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2345,7 +2345,13 @@ enum i915_power_well_id {
  #define BSD_RING_BASE 0x04000
  #define GEN6_BSD_RING_BASE0x12000
  #define GEN8_BSD2_RING_BASE   0x1c000
+#define GEN11_BSD_RING_BASE0x1c
+#define GEN11_BSD2_RING_BASE   0x1c4000
+#define GEN11_BSD3_RING_BASE   0x1d
+#define GEN11_BSD4_RING_BASE   0x1d4000
  #define VEBOX_RING_BASE   0x1a000
+#define GEN11_VEBOX_RING_BASE  0x1c8000
+#define GEN11_VEBOX2_RING_BASE 0x1d8000
  #define BLT_RING_BASE 0x22000
  #define RING_TAIL(base)   _MMIO((base)+0x30)
  #define RING_HEAD(base)   _MMIO((base)+0x34)
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c 
b/drivers/gpu/drm/i915/intel_engine_cs.c
index 3e1107ecb6ee..911fc08658c5 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -123,6 +123,22 @@ static const struct engine_info intel_engines[] = {
.mmio_base = GEN8_BSD2_RING_BASE,
.irq_shift = GEN8_VCS2_IRQ_SHIFT,
},
+   [VCS3] = {
+   .hw_id = VCS3_HW,
+   .uabi_id = I915_EXEC_BSD,
+   .class = VIDEO_DECODE_CLASS,
+   .instance = 2,
+   .mmio_base = GEN11_BSD3_RING_BASE,
+   .irq_shift = 0, /* not used */
+   },
+   [VCS4] = {
+   .hw_id = VCS4_HW,
+   .uabi_id = I915_EXEC_BSD,
+   .class = VIDEO_DECODE_CLASS,
+   .instance = 3,
+   .mmio_base = GEN11_BSD4_RING_BASE,
+   .irq_shift = 0, /* not used */
+   },
[VECS] = {
.hw_id = VECS_HW,
.uabi_id = I915_EXEC_VEBOX,
@@ -131,6 +147,14 @@ static const struct engine_info intel_engines[] = {
.mmio_base = VEBOX_RING_BASE,
.irq_shift = GEN8_VECS_IRQ_SHIFT,
},
+   [VECS2] = {
+   .hw_id = VECS2_HW,
+   .uabi_id = I915_EXEC_VEBOX,
+   .class = VIDEO_ENHANCEMENT_CLASS,
+   .instance = 1,
+   .mmio_base = GEN11_VEBOX2_RING_BASE,
+   .irq_shift = 0, /* not used */
+   },
  };
  
  /**

@@ -230,7 +254,25 @@ intel_engine_setup(struct drm_i915_private *dev_priv,
 class_info->name, info->instance) >=
sizeof(engine->name));
engine->hw_id = engine->guc_id = info->hw_id;
-   engine->mmio_base = info->mmio_base;
+   if (INTEL_GEN(dev_priv) >= 11) {
+   switch (engine->id) {
+   case VCS:
+   engine->mmio_base = GEN11_BSD_RING_BASE;
+   break;
+   case VCS2:
+   engine->mmio_base = GEN11_BSD2_RING_BASE;
+   break;
+   case VECS:
+   engine->mmio_base = GEN11_VEBOX_RING_BASE;
+   break;
+   default:
+   /* take the original value for all other engines  */
+   engine->mmio_base = info->mmio_base;
+   break;
+   }


I'm slightly unconvinced by the fact that we have a static table with 
some values and then we use other values here. Maybe we could instead 
use and array of [starting_gen, mmio_base] pairs in the table and derive 
the correct value here? Anyway, since this is not the only place where 
we don't use the mmio_base value from the table (same happens for 
pre-gen6 BSD in intel_init_bsd_ring_buffer) I don't consider this a 
blocking issue. All the values match the specs, so:


Reviewed-by: Daniele Ceraolo Spurio 

I'll probably check how using the table I mentioned above looks like 
after this is merged and I'll send an RFC if it seems nice.


Thanks,
Daniele


+   } else {
+   engine->mmio_base = info->mmio_base;
+   }
engine->irq_shift = info->irq_shift;
engine->class = info->class;
engine->instance =

[Intel-gfx] [PATCH 2/6] drm/i915/icl: Correctly initialize the Gen11 engines

2018-03-02 Thread Mika Kuoppala

From: Oscar Mateo 

Gen11 has up to 4 VCS and up to 2 VECS engines, this patch adds mmio
base definitions for all of them.

Bspec: 20944
Bspec: 7021

v2: Set the correct mmio_base in intel_engines_init_mmio; updating the
base mmio values any later would cause incorrect reads in
i915_gem_sanitize (Michel).

Cc: Tvrtko Ursulin 
Cc: Ceraolo Spurio, Daniele 
Signed-off-by: Oscar Mateo 
Signed-off-by: Michel Thierry 
---
 drivers/gpu/drm/i915/i915_reg.h|  6 +
 drivers/gpu/drm/i915/intel_engine_cs.c | 44 +-
 2 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 258e86eb37d5..45ae05d0fe78 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2345,7 +2345,13 @@ enum i915_power_well_id {
 #define BSD_RING_BASE  0x04000
 #define GEN6_BSD_RING_BASE 0x12000
 #define GEN8_BSD2_RING_BASE0x1c000
+#define GEN11_BSD_RING_BASE0x1c
+#define GEN11_BSD2_RING_BASE   0x1c4000
+#define GEN11_BSD3_RING_BASE   0x1d
+#define GEN11_BSD4_RING_BASE   0x1d4000
 #define VEBOX_RING_BASE0x1a000
+#define GEN11_VEBOX_RING_BASE  0x1c8000
+#define GEN11_VEBOX2_RING_BASE 0x1d8000
 #define BLT_RING_BASE  0x22000
 #define RING_TAIL(base)_MMIO((base)+0x30)
 #define RING_HEAD(base)_MMIO((base)+0x34)
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c 
b/drivers/gpu/drm/i915/intel_engine_cs.c
index 3e1107ecb6ee..911fc08658c5 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -123,6 +123,22 @@ static const struct engine_info intel_engines[] = {
.mmio_base = GEN8_BSD2_RING_BASE,
.irq_shift = GEN8_VCS2_IRQ_SHIFT,
},
+   [VCS3] = {
+   .hw_id = VCS3_HW,
+   .uabi_id = I915_EXEC_BSD,
+   .class = VIDEO_DECODE_CLASS,
+   .instance = 2,
+   .mmio_base = GEN11_BSD3_RING_BASE,
+   .irq_shift = 0, /* not used */
+   },
+   [VCS4] = {
+   .hw_id = VCS4_HW,
+   .uabi_id = I915_EXEC_BSD,
+   .class = VIDEO_DECODE_CLASS,
+   .instance = 3,
+   .mmio_base = GEN11_BSD4_RING_BASE,
+   .irq_shift = 0, /* not used */
+   },
[VECS] = {
.hw_id = VECS_HW,
.uabi_id = I915_EXEC_VEBOX,
@@ -131,6 +147,14 @@ static const struct engine_info intel_engines[] = {
.mmio_base = VEBOX_RING_BASE,
.irq_shift = GEN8_VECS_IRQ_SHIFT,
},
+   [VECS2] = {
+   .hw_id = VECS2_HW,
+   .uabi_id = I915_EXEC_VEBOX,
+   .class = VIDEO_ENHANCEMENT_CLASS,
+   .instance = 1,
+   .mmio_base = GEN11_VEBOX2_RING_BASE,
+   .irq_shift = 0, /* not used */
+   },
 };
 
 /**
@@ -230,7 +254,25 @@ intel_engine_setup(struct drm_i915_private *dev_priv,
 class_info->name, info->instance) >=
sizeof(engine->name));
engine->hw_id = engine->guc_id = info->hw_id;
-   engine->mmio_base = info->mmio_base;
+   if (INTEL_GEN(dev_priv) >= 11) {
+   switch (engine->id) {
+   case VCS:
+   engine->mmio_base = GEN11_BSD_RING_BASE;
+   break;
+   case VCS2:
+   engine->mmio_base = GEN11_BSD2_RING_BASE;
+   break;
+   case VECS:
+   engine->mmio_base = GEN11_VEBOX_RING_BASE;
+   break;
+   default:
+   /* take the original value for all other engines  */
+   engine->mmio_base = info->mmio_base;
+   break;
+   }
+   } else {
+   engine->mmio_base = info->mmio_base;
+   }
engine->irq_shift = info->irq_shift;
engine->class = info->class;
engine->instance = info->instance;
-- 
2.14.1

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

Re: [Intel-gfx] [PATCH 2/6] drm/i915/icl: Correctly initialize the Gen11 engines

Re: [Intel-gfx] [PATCH 2/6] drm/i915/icl: Correctly initialize the Gen11 engines

[Intel-gfx] [PATCH 2/6] drm/i915/icl: Correctly initialize the Gen11 engines

3 matches

Site Navigation

Mail list logo

Footer information