subject:"\[Intel\-gfx\] \[PATCH 3\/5\] drm\/i915\/mtl\: add GSC CS interrupt support"

Re: [Intel-gfx] [PATCH 3/5] drm/i915/mtl: add GSC CS interrupt support

2022-10-31 Thread Ceraolo Spurio, Daniele





On 10/31/2022 5:55 AM, Tvrtko Ursulin wrote:


On 28/10/2022 18:00, Ceraolo Spurio, Daniele wrote:

On 10/28/2022 1:38 AM, Tvrtko Ursulin wrote:


On 27/10/2022 23:15, Daniele Ceraolo Spurio wrote:

The GSC CS re-uses the same interrupt bits that the GSC used in older
platforms. This means that we can now have an engine interrupt coming
out of OTHER_CLASS, so we need to handle that appropriately.

Signed-off-by: Daniele Ceraolo Spurio 


Cc: Matt Roper 
---
  drivers/gpu/drm/i915/gt/intel_gt_irq.c | 78 
++

  1 file changed, 43 insertions(+), 35 deletions(-)

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

index f26882fdc24c..34ff1ee7e931 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_irq.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
@@ -81,35 +81,27 @@ gen11_other_irq_handler(struct intel_gt *gt, 
const u8 instance,

    instance, iir);
  }
  -static void
-gen11_engine_irq_handler(struct intel_gt *gt, const u8 class,
- const u8 instance, const u16 iir)
+static struct intel_gt *pick_gt(struct intel_gt *gt, u8 class, u8 
instance)

  {
-    struct intel_engine_cs *engine;
-
-    /*
- * Platforms with standalone media have their media engines in 
another

- * GT.
- */
-    if (MEDIA_VER(gt->i915) >= 13 &&
-    (class == VIDEO_DECODE_CLASS || class == 
VIDEO_ENHANCEMENT_CLASS)) {

-    if (!gt->i915->media_gt)
-    goto err;
+    struct intel_gt *media_gt = gt->i915->media_gt;
  -    gt = gt->i915->media_gt;
+    /* we expect the non-media gt to be passed in */
+    GEM_BUG_ON(gt == media_gt);
+
+    if (!media_gt)
+    return gt;
+
+    switch (class) {
+    case VIDEO_DECODE_CLASS:
+    case VIDEO_ENHANCEMENT_CLASS:
+    return media_gt;
+    case OTHER_CLASS:
+    if (instance == OTHER_GSC_INSTANCE && HAS_ENGINE(media_gt, 
GSC0))

+    return media_gt;
+    fallthrough;
+    default:
+    return gt;
  }
-
-    if (instance <= MAX_ENGINE_INSTANCE)
-    engine = gt->engine_class[class][instance];
-    else
-    engine = NULL;
-
-    if (likely(engine))
-    return intel_engine_cs_irq(engine, iir);
-
-err:
-    WARN_ONCE(1, "unhandled engine interrupt class=0x%x, 
instance=0x%x\n",

-  class, instance);
  }
    static void
@@ -118,12 +110,24 @@ gen11_gt_identity_handler(struct intel_gt 
*gt, const u32 identity)

  const u8 class = GEN11_INTR_ENGINE_CLASS(identity);
  const u8 instance = GEN11_INTR_ENGINE_INSTANCE(identity);
  const u16 intr = GEN11_INTR_ENGINE_INTR(identity);
+    struct intel_engine_cs *engine;
    if (unlikely(!intr))
  return;
  -    if (class <= COPY_ENGINE_CLASS || class == COMPUTE_CLASS)
-    return gen11_engine_irq_handler(gt, class, instance, intr);
+    /*
+ * Platforms with standalone media have the media and GSC 
engines in

+ * another GT.
+ */
+    gt = pick_gt(gt, class, instance);
+
+    if (class <= MAX_ENGINE_CLASS && instance <= MAX_ENGINE_INSTANCE)
+    engine = gt->engine_class[class][instance];
+    else
+    engine = NULL;
+
+    if (engine)
+    return intel_engine_cs_irq(engine, intr);


Drive by observation - you could fold the above two ifs into one 
since engine appears unused afterwards.


engine can be NULL in both branches of the if statement, so to get a 
unified if we'd have to do something like:


if (class <= MAX_ENGINE_CLASS && instance <= MAX_ENGINE_INSTANCE) {
     struct intel_engine_cs *engine = 
gt->engine_class[class][instance];

     if (engine)
             return intel_engine_cs_irq(engine, intr);
}

Is this what you are suggesting?


Right, two ifs are needed after all. Well at least it would avoid the 
pointless engine = NULL assignment. Up to you.


Absence of any out-of-range class/instance logging is intentional?


There is already a log further down in this function.

Daniele



Regards,

Tvrtko

Re: [Intel-gfx] [PATCH 3/5] drm/i915/mtl: add GSC CS interrupt support

2022-10-31 Thread Tvrtko Ursulin




On 28/10/2022 18:00, Ceraolo Spurio, Daniele wrote:

On 10/28/2022 1:38 AM, Tvrtko Ursulin wrote:


On 27/10/2022 23:15, Daniele Ceraolo Spurio wrote:

The GSC CS re-uses the same interrupt bits that the GSC used in older
platforms. This means that we can now have an engine interrupt coming
out of OTHER_CLASS, so we need to handle that appropriately.

Signed-off-by: Daniele Ceraolo Spurio 
Cc: Matt Roper 
---
  drivers/gpu/drm/i915/gt/intel_gt_irq.c | 78 ++
  1 file changed, 43 insertions(+), 35 deletions(-)

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

index f26882fdc24c..34ff1ee7e931 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_irq.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
@@ -81,35 +81,27 @@ gen11_other_irq_handler(struct intel_gt *gt, 
const u8 instance,

    instance, iir);
  }
  -static void
-gen11_engine_irq_handler(struct intel_gt *gt, const u8 class,
- const u8 instance, const u16 iir)
+static struct intel_gt *pick_gt(struct intel_gt *gt, u8 class, u8 
instance)

  {
-    struct intel_engine_cs *engine;
-
-    /*
- * Platforms with standalone media have their media engines in 
another

- * GT.
- */
-    if (MEDIA_VER(gt->i915) >= 13 &&
-    (class == VIDEO_DECODE_CLASS || class == 
VIDEO_ENHANCEMENT_CLASS)) {

-    if (!gt->i915->media_gt)
-    goto err;
+    struct intel_gt *media_gt = gt->i915->media_gt;
  -    gt = gt->i915->media_gt;
+    /* we expect the non-media gt to be passed in */
+    GEM_BUG_ON(gt == media_gt);
+
+    if (!media_gt)
+    return gt;
+
+    switch (class) {
+    case VIDEO_DECODE_CLASS:
+    case VIDEO_ENHANCEMENT_CLASS:
+    return media_gt;
+    case OTHER_CLASS:
+    if (instance == OTHER_GSC_INSTANCE && HAS_ENGINE(media_gt, 
GSC0))

+    return media_gt;
+    fallthrough;
+    default:
+    return gt;
  }
-
-    if (instance <= MAX_ENGINE_INSTANCE)
-    engine = gt->engine_class[class][instance];
-    else
-    engine = NULL;
-
-    if (likely(engine))
-    return intel_engine_cs_irq(engine, iir);
-
-err:
-    WARN_ONCE(1, "unhandled engine interrupt class=0x%x, 
instance=0x%x\n",

-  class, instance);
  }
    static void
@@ -118,12 +110,24 @@ gen11_gt_identity_handler(struct intel_gt *gt, 
const u32 identity)

  const u8 class = GEN11_INTR_ENGINE_CLASS(identity);
  const u8 instance = GEN11_INTR_ENGINE_INSTANCE(identity);
  const u16 intr = GEN11_INTR_ENGINE_INTR(identity);
+    struct intel_engine_cs *engine;
    if (unlikely(!intr))
  return;
  -    if (class <= COPY_ENGINE_CLASS || class == COMPUTE_CLASS)
-    return gen11_engine_irq_handler(gt, class, instance, intr);
+    /*
+ * Platforms with standalone media have the media and GSC 
engines in

+ * another GT.
+ */
+    gt = pick_gt(gt, class, instance);
+
+    if (class <= MAX_ENGINE_CLASS && instance <= MAX_ENGINE_INSTANCE)
+    engine = gt->engine_class[class][instance];
+    else
+    engine = NULL;
+
+    if (engine)
+    return intel_engine_cs_irq(engine, intr);


Drive by observation - you could fold the above two ifs into one since 
engine appears unused afterwards.


engine can be NULL in both branches of the if statement, so to get a 
unified if we'd have to do something like:


if (class <= MAX_ENGINE_CLASS && instance <= MAX_ENGINE_INSTANCE) {
         struct intel_engine_cs *engine = 
gt->engine_class[class][instance];

         if (engine)
                 return intel_engine_cs_irq(engine, intr);
}

Is this what you are suggesting?


Right, two ifs are needed after all. Well at least it would avoid the 
pointless engine = NULL assignment. Up to you.


Absence of any out-of-range class/instance logging is intentional?

Regards,

Tvrtko

Re: [Intel-gfx] [PATCH 3/5] drm/i915/mtl: add GSC CS interrupt support

2022-10-28 Thread Ceraolo Spurio, Daniele





On 10/28/2022 1:38 AM, Tvrtko Ursulin wrote:


On 27/10/2022 23:15, Daniele Ceraolo Spurio wrote:

The GSC CS re-uses the same interrupt bits that the GSC used in older
platforms. This means that we can now have an engine interrupt coming
out of OTHER_CLASS, so we need to handle that appropriately.

Signed-off-by: Daniele Ceraolo Spurio 
Cc: Matt Roper 
---
  drivers/gpu/drm/i915/gt/intel_gt_irq.c | 78 ++
  1 file changed, 43 insertions(+), 35 deletions(-)

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

index f26882fdc24c..34ff1ee7e931 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_irq.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
@@ -81,35 +81,27 @@ gen11_other_irq_handler(struct intel_gt *gt, 
const u8 instance,

    instance, iir);
  }
  -static void
-gen11_engine_irq_handler(struct intel_gt *gt, const u8 class,
- const u8 instance, const u16 iir)
+static struct intel_gt *pick_gt(struct intel_gt *gt, u8 class, u8 
instance)

  {
-    struct intel_engine_cs *engine;
-
-    /*
- * Platforms with standalone media have their media engines in 
another

- * GT.
- */
-    if (MEDIA_VER(gt->i915) >= 13 &&
-    (class == VIDEO_DECODE_CLASS || class == 
VIDEO_ENHANCEMENT_CLASS)) {

-    if (!gt->i915->media_gt)
-    goto err;
+    struct intel_gt *media_gt = gt->i915->media_gt;
  -    gt = gt->i915->media_gt;
+    /* we expect the non-media gt to be passed in */
+    GEM_BUG_ON(gt == media_gt);
+
+    if (!media_gt)
+    return gt;
+
+    switch (class) {
+    case VIDEO_DECODE_CLASS:
+    case VIDEO_ENHANCEMENT_CLASS:
+    return media_gt;
+    case OTHER_CLASS:
+    if (instance == OTHER_GSC_INSTANCE && HAS_ENGINE(media_gt, 
GSC0))

+    return media_gt;
+    fallthrough;
+    default:
+    return gt;
  }
-
-    if (instance <= MAX_ENGINE_INSTANCE)
-    engine = gt->engine_class[class][instance];
-    else
-    engine = NULL;
-
-    if (likely(engine))
-    return intel_engine_cs_irq(engine, iir);
-
-err:
-    WARN_ONCE(1, "unhandled engine interrupt class=0x%x, 
instance=0x%x\n",

-  class, instance);
  }
    static void
@@ -118,12 +110,24 @@ gen11_gt_identity_handler(struct intel_gt *gt, 
const u32 identity)

  const u8 class = GEN11_INTR_ENGINE_CLASS(identity);
  const u8 instance = GEN11_INTR_ENGINE_INSTANCE(identity);
  const u16 intr = GEN11_INTR_ENGINE_INTR(identity);
+    struct intel_engine_cs *engine;
    if (unlikely(!intr))
  return;
  -    if (class <= COPY_ENGINE_CLASS || class == COMPUTE_CLASS)
-    return gen11_engine_irq_handler(gt, class, instance, intr);
+    /*
+ * Platforms with standalone media have the media and GSC 
engines in

+ * another GT.
+ */
+    gt = pick_gt(gt, class, instance);
+
+    if (class <= MAX_ENGINE_CLASS && instance <= MAX_ENGINE_INSTANCE)
+    engine = gt->engine_class[class][instance];
+    else
+    engine = NULL;
+
+    if (engine)
+    return intel_engine_cs_irq(engine, intr);


Drive by observation - you could fold the above two ifs into one since 
engine appears unused afterwards.


engine can be NULL in both branches of the if statement, so to get a 
unified if we'd have to do something like:


if (class <= MAX_ENGINE_CLASS && instance <= MAX_ENGINE_INSTANCE) {
        struct intel_engine_cs *engine = gt->engine_class[class][instance];
        if (engine)
                return intel_engine_cs_irq(engine, intr);
}

Is this what you are suggesting?

Daniele



Regards,

Tvrtko


    if (class == OTHER_CLASS)
  return gen11_other_irq_handler(gt, instance, intr);
@@ -206,7 +210,7 @@ void gen11_gt_irq_reset(struct intel_gt *gt)
  intel_uncore_write(uncore, GEN11_VCS_VECS_INTR_ENABLE,  0);
  if (CCS_MASK(gt))
  intel_uncore_write(uncore, GEN12_CCS_RSVD_INTR_ENABLE, 0);
-    if (HAS_HECI_GSC(gt->i915))
+    if (HAS_HECI_GSC(gt->i915) || HAS_ENGINE(gt, GSC0))
  intel_uncore_write(uncore, GEN11_GUNIT_CSME_INTR_ENABLE, 0);
    /* Restore masks irqs on RCS, BCS, VCS and VECS engines. */
@@ -233,7 +237,7 @@ void gen11_gt_irq_reset(struct intel_gt *gt)
  intel_uncore_write(uncore, GEN12_CCS0_CCS1_INTR_MASK, ~0);
  if (HAS_ENGINE(gt, CCS2) || HAS_ENGINE(gt, CCS3))
  intel_uncore_write(uncore, GEN12_CCS2_CCS3_INTR_MASK, ~0);
-    if (HAS_HECI_GSC(gt->i915))
+    if (HAS_HECI_GSC(gt->i915) || HAS_ENGINE(gt, GSC0))
  intel_uncore_write(uncore, GEN11_GUNIT_CSME_INTR_MASK, ~0);
    intel_uncore_write(uncore, GEN11_GPM_WGBOXPERF_INTR_ENABLE, 0);
@@ -249,7 +253,7 @@ void gen11_gt_irq_postinstall(struct intel_gt *gt)
  {
  struct intel_uncore *uncore = gt->uncore;
  u32 irqs = GT_RENDER_USER_INTERRUPT;
-    const u32 gsc_mask = GSC_IRQ_INTF(0) | GSC_IRQ_INTF(1);
+    u32 gsc_mask = 0;
  u32 dmask;
  u32 smask;
  @@ -261,6 +265,11 @@ void

Re: [Intel-gfx] [PATCH 3/5] drm/i915/mtl: add GSC CS interrupt support

2022-10-28 Thread Tvrtko Ursulin




On 27/10/2022 23:15, Daniele Ceraolo Spurio wrote:

The GSC CS re-uses the same interrupt bits that the GSC used in older
platforms. This means that we can now have an engine interrupt coming
out of OTHER_CLASS, so we need to handle that appropriately.

Signed-off-by: Daniele Ceraolo Spurio 
Cc: Matt Roper 
---
  drivers/gpu/drm/i915/gt/intel_gt_irq.c | 78 ++
  1 file changed, 43 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_irq.c 
b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
index f26882fdc24c..34ff1ee7e931 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_irq.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
@@ -81,35 +81,27 @@ gen11_other_irq_handler(struct intel_gt *gt, const u8 
instance,
  instance, iir);
  }
  
-static void

-gen11_engine_irq_handler(struct intel_gt *gt, const u8 class,
-const u8 instance, const u16 iir)
+static struct intel_gt *pick_gt(struct intel_gt *gt, u8 class, u8 instance)
  {
-   struct intel_engine_cs *engine;
-
-   /*
-* Platforms with standalone media have their media engines in another
-* GT.
-*/
-   if (MEDIA_VER(gt->i915) >= 13 &&
-   (class == VIDEO_DECODE_CLASS || class == VIDEO_ENHANCEMENT_CLASS)) {
-   if (!gt->i915->media_gt)
-   goto err;
+   struct intel_gt *media_gt = gt->i915->media_gt;
  
-		gt = gt->i915->media_gt;

+   /* we expect the non-media gt to be passed in */
+   GEM_BUG_ON(gt == media_gt);
+
+   if (!media_gt)
+   return gt;
+
+   switch (class) {
+   case VIDEO_DECODE_CLASS:
+   case VIDEO_ENHANCEMENT_CLASS:
+   return media_gt;
+   case OTHER_CLASS:
+   if (instance == OTHER_GSC_INSTANCE && HAS_ENGINE(media_gt, 
GSC0))
+   return media_gt;
+   fallthrough;
+   default:
+   return gt;
}
-
-   if (instance <= MAX_ENGINE_INSTANCE)
-   engine = gt->engine_class[class][instance];
-   else
-   engine = NULL;
-
-   if (likely(engine))
-   return intel_engine_cs_irq(engine, iir);
-
-err:
-   WARN_ONCE(1, "unhandled engine interrupt class=0x%x, instance=0x%x\n",
- class, instance);
  }
  
  static void

@@ -118,12 +110,24 @@ gen11_gt_identity_handler(struct intel_gt *gt, const u32 
identity)
const u8 class = GEN11_INTR_ENGINE_CLASS(identity);
const u8 instance = GEN11_INTR_ENGINE_INSTANCE(identity);
const u16 intr = GEN11_INTR_ENGINE_INTR(identity);
+   struct intel_engine_cs *engine;
  
  	if (unlikely(!intr))

return;
  
-	if (class <= COPY_ENGINE_CLASS || class == COMPUTE_CLASS)

-   return gen11_engine_irq_handler(gt, class, instance, intr);
+   /*
+* Platforms with standalone media have the media and GSC engines in
+* another GT.
+*/
+   gt = pick_gt(gt, class, instance);
+
+   if (class <= MAX_ENGINE_CLASS && instance <= MAX_ENGINE_INSTANCE)
+   engine = gt->engine_class[class][instance];
+   else
+   engine = NULL;
+
+   if (engine)
+   return intel_engine_cs_irq(engine, intr);


Drive by observation - you could fold the above two ifs into one since 
engine appears unused afterwards.


Regards,

Tvrtko

  
  	if (class == OTHER_CLASS)

return gen11_other_irq_handler(gt, instance, intr);
@@ -206,7 +210,7 @@ void gen11_gt_irq_reset(struct intel_gt *gt)
intel_uncore_write(uncore, GEN11_VCS_VECS_INTR_ENABLE,0);
if (CCS_MASK(gt))
intel_uncore_write(uncore, GEN12_CCS_RSVD_INTR_ENABLE, 0);
-   if (HAS_HECI_GSC(gt->i915))
+   if (HAS_HECI_GSC(gt->i915) || HAS_ENGINE(gt, GSC0))
intel_uncore_write(uncore, GEN11_GUNIT_CSME_INTR_ENABLE, 0);
  
  	/* Restore masks irqs on RCS, BCS, VCS and VECS engines. */

@@ -233,7 +237,7 @@ void gen11_gt_irq_reset(struct intel_gt *gt)
intel_uncore_write(uncore, GEN12_CCS0_CCS1_INTR_MASK, ~0);
if (HAS_ENGINE(gt, CCS2) || HAS_ENGINE(gt, CCS3))
intel_uncore_write(uncore, GEN12_CCS2_CCS3_INTR_MASK, ~0);
-   if (HAS_HECI_GSC(gt->i915))
+   if (HAS_HECI_GSC(gt->i915) || HAS_ENGINE(gt, GSC0))
intel_uncore_write(uncore, GEN11_GUNIT_CSME_INTR_MASK, ~0);
  
  	intel_uncore_write(uncore, GEN11_GPM_WGBOXPERF_INTR_ENABLE, 0);

@@ -249,7 +253,7 @@ void gen11_gt_irq_postinstall(struct intel_gt *gt)
  {
struct intel_uncore *uncore = gt->uncore;
u32 irqs = GT_RENDER_USER_INTERRUPT;
-   const u32 gsc_mask = GSC_IRQ_INTF(0) | GSC_IRQ_INTF(1);
+   u32 gsc_mask = 0;
u32 dmask;
u32 smask;
  
@@ -261,6 +265,11 @@ void gen11_gt_irq_postinstall(struct intel_gt *gt)

dmask = irqs << 16 | irqs;
smask = irqs << 16;
  
+	if (HAS_ENGINE(gt, GSC0))

+   gsc_mask =

Re: [Intel-gfx] [PATCH 3/5] drm/i915/mtl: add GSC CS interrupt support

Re: [Intel-gfx] [PATCH 3/5] drm/i915/mtl: add GSC CS interrupt support

Re: [Intel-gfx] [PATCH 3/5] drm/i915/mtl: add GSC CS interrupt support

Re: [Intel-gfx] [PATCH 3/5] drm/i915/mtl: add GSC CS interrupt support

4 matches

Site Navigation

Mail list logo

Footer information