[v2] drm/i915/guc: Properly capture & release GuC interrupts on Gen11+
diff mbox series

Message ID 20191105225321.26642-1-daniele.ceraolospurio@intel.com
State New
Headers show
Series
  • [v2] drm/i915/guc: Properly capture & release GuC interrupts on Gen11+
Related show

Commit Message

Daniele Ceraolo Spurio Nov. 5, 2019, 10:53 p.m. UTC
With the new interrupt re-partitioning in Gen11, GuC controls by itself
the interrupts it receives, so steering bits and registers have been
defeatured. Being this the case, when the GuC is in control of
submissions we won't know what to do with the ctx switch interrupt
in the driver, so disable it.

v2 (Daniele): replace the gen9 paths instead of keeping gen9 and gen11
functions since we won't support guc submission on any pre-gen11 platform.

Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: John Harrison <John.C.Harrison@Intel.com>
Cc: Matthew Brost <matthew.brost@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_rps.c           |  2 +-
 .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 72 ++++---------------
 2 files changed, 14 insertions(+), 60 deletions(-)

Comments

John Harrison Nov. 14, 2019, 10:38 p.m. UTC | #1
On 11/5/2019 14:53, Daniele Ceraolo Spurio wrote:
> With the new interrupt re-partitioning in Gen11, GuC controls by itself
> the interrupts it receives, so steering bits and registers have been
> defeatured. Being this the case, when the GuC is in control of
> submissions we won't know what to do with the ctx switch interrupt
> in the driver, so disable it.
>
> v2 (Daniele): replace the gen9 paths instead of keeping gen9 and gen11
> functions since we won't support guc submission on any pre-gen11 platform.
>
> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: John Harrison <John.C.Harrison@Intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>

Reviewed-by: John Harrison <John.C.Harrison@Intel.com>


> ---
>   drivers/gpu/drm/i915/gt/intel_rps.c           |  2 +-
>   .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 72 ++++---------------
>   2 files changed, 14 insertions(+), 60 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c
> index 20d6ee148afc..08a38a3b90b0 100644
> --- a/drivers/gpu/drm/i915/gt/intel_rps.c
> +++ b/drivers/gpu/drm/i915/gt/intel_rps.c
> @@ -1663,7 +1663,7 @@ void intel_rps_init(struct intel_rps *rps)
>   	if (INTEL_GEN(i915) <= 7)
>   		rps->pm_intrmsk_mbz |= GEN6_PM_RP_UP_EI_EXPIRED;
>   
> -	if (INTEL_GEN(i915) >= 8)
> +	if (INTEL_GEN(i915) >= 8 && INTEL_GEN(i915) < 11)
>   		rps->pm_intrmsk_mbz |= GEN8_PMINTR_DISABLE_REDIRECT_TO_GUC;
>   }
>   
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> index 2498c55e0ea5..902e25eb6b9d 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> @@ -1011,74 +1011,28 @@ void intel_guc_submission_fini(struct intel_guc *guc)
>   
>   static void guc_interrupts_capture(struct intel_gt *gt)
>   {
> -	struct intel_rps *rps = &gt->rps;
>   	struct intel_uncore *uncore = gt->uncore;
> -	struct intel_engine_cs *engine;
> -	enum intel_engine_id id;
> -	int irqs;
> +	u32 irqs = GT_CONTEXT_SWITCH_INTERRUPT;
> +	u32 dmask = irqs << 16 | irqs;
>   
> -	/* tell all command streamers to forward interrupts (but not vblank)
> -	 * to GuC
> -	 */
> -	irqs = _MASKED_BIT_ENABLE(GFX_INTERRUPT_STEERING);
> -	for_each_engine(engine, gt, id)
> -		ENGINE_WRITE(engine, RING_MODE_GEN7, irqs);
> -
> -	/* route USER_INTERRUPT to Host, all others are sent to GuC. */
> -	irqs = GT_RENDER_USER_INTERRUPT << GEN8_RCS_IRQ_SHIFT |
> -	       GT_RENDER_USER_INTERRUPT << GEN8_BCS_IRQ_SHIFT;
> -	/* These three registers have the same bit definitions */
> -	intel_uncore_write(uncore, GUC_BCS_RCS_IER, ~irqs);
> -	intel_uncore_write(uncore, GUC_VCS2_VCS1_IER, ~irqs);
> -	intel_uncore_write(uncore, GUC_WD_VECS_IER, ~irqs);
> +	GEM_BUG_ON(INTEL_GEN(gt->i915) < 11);
>   
> -	/*
> -	 * The REDIRECT_TO_GUC bit of the PMINTRMSK register directs all
> -	 * (unmasked) PM interrupts to the GuC. All other bits of this
> -	 * register *disable* generation of a specific interrupt.
> -	 *
> -	 * 'pm_intrmsk_mbz' indicates bits that are NOT to be set when
> -	 * writing to the PM interrupt mask register, i.e. interrupts
> -	 * that must not be disabled.
> -	 *
> -	 * If the GuC is handling these interrupts, then we must not let
> -	 * the PM code disable ANY interrupt that the GuC is expecting.
> -	 * So for each ENABLED (0) bit in this register, we must SET the
> -	 * bit in pm_intrmsk_mbz so that it's left enabled for the GuC.
> -	 * GuC needs ARAT expired interrupt unmasked hence it is set in
> -	 * pm_intrmsk_mbz.
> -	 *
> -	 * Here we CLEAR REDIRECT_TO_GUC bit in pm_intrmsk_mbz, which will
> -	 * result in the register bit being left SET!
> -	 */
> -	rps->pm_intrmsk_mbz |= ARAT_EXPIRED_INTRMSK;
> -	rps->pm_intrmsk_mbz &= ~GEN8_PMINTR_DISABLE_REDIRECT_TO_GUC;
> +	/* Don't handle the ctx switch interrupt in GuC submission mode */
> +	intel_uncore_rmw(uncore, GEN11_RENDER_COPY_INTR_ENABLE, dmask, 0);
> +	intel_uncore_rmw(uncore, GEN11_VCS_VECS_INTR_ENABLE, dmask, 0);
>   }
>   
>   static void guc_interrupts_release(struct intel_gt *gt)
>   {
> -	struct intel_rps *rps = &gt->rps;
>   	struct intel_uncore *uncore = gt->uncore;
> -	struct intel_engine_cs *engine;
> -	enum intel_engine_id id;
> -	int irqs;
> +	u32 irqs = GT_CONTEXT_SWITCH_INTERRUPT;
> +	u32 dmask = irqs << 16 | irqs;
>   
> -	/*
> -	 * tell all command streamers NOT to forward interrupts or vblank
> -	 * to GuC.
> -	 */
> -	irqs = _MASKED_FIELD(GFX_FORWARD_VBLANK_MASK, GFX_FORWARD_VBLANK_NEVER);
> -	irqs |= _MASKED_BIT_DISABLE(GFX_INTERRUPT_STEERING);
> -	for_each_engine(engine, gt, id)
> -		ENGINE_WRITE(engine, RING_MODE_GEN7, irqs);
> -
> -	/* route all GT interrupts to the host */
> -	intel_uncore_write(uncore, GUC_BCS_RCS_IER, 0);
> -	intel_uncore_write(uncore, GUC_VCS2_VCS1_IER, 0);
> -	intel_uncore_write(uncore, GUC_WD_VECS_IER, 0);
> -
> -	rps->pm_intrmsk_mbz |= GEN8_PMINTR_DISABLE_REDIRECT_TO_GUC;
> -	rps->pm_intrmsk_mbz &= ~ARAT_EXPIRED_INTRMSK;
> +	GEM_BUG_ON(INTEL_GEN(gt->i915) < 11);
> +
> +	/* Handle ctx switch interrupts again */
> +	intel_uncore_rmw(uncore, GEN11_RENDER_COPY_INTR_ENABLE, 0, dmask);
> +	intel_uncore_rmw(uncore, GEN11_VCS_VECS_INTR_ENABLE, 0, dmask);
>   }
>   
>   static void guc_set_default_submission(struct intel_engine_cs *engine)
Tvrtko Ursulin Nov. 15, 2019, 8:39 a.m. UTC | #2
On 05/11/2019 22:53, Daniele Ceraolo Spurio wrote:
> With the new interrupt re-partitioning in Gen11, GuC controls by itself
> the interrupts it receives, so steering bits and registers have been
> defeatured. Being this the case, when the GuC is in control of
> submissions we won't know what to do with the ctx switch interrupt
> in the driver, so disable it.
> 
> v2 (Daniele): replace the gen9 paths instead of keeping gen9 and gen11
> functions since we won't support guc submission on any pre-gen11 platform.
> 
> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: John Harrison <John.C.Harrison@Intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> ---
>   drivers/gpu/drm/i915/gt/intel_rps.c           |  2 +-
>   .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 72 ++++---------------
>   2 files changed, 14 insertions(+), 60 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c
> index 20d6ee148afc..08a38a3b90b0 100644
> --- a/drivers/gpu/drm/i915/gt/intel_rps.c
> +++ b/drivers/gpu/drm/i915/gt/intel_rps.c
> @@ -1663,7 +1663,7 @@ void intel_rps_init(struct intel_rps *rps)
>   	if (INTEL_GEN(i915) <= 7)
>   		rps->pm_intrmsk_mbz |= GEN6_PM_RP_UP_EI_EXPIRED;
>   
> -	if (INTEL_GEN(i915) >= 8)
> +	if (INTEL_GEN(i915) >= 8 && INTEL_GEN(i915) < 11)

Or IS_GEN_RANGE(8, 10), if you want.

Regards,

Tvrtko

>   		rps->pm_intrmsk_mbz |= GEN8_PMINTR_DISABLE_REDIRECT_TO_GUC;
>   }
>   
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> index 2498c55e0ea5..902e25eb6b9d 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> @@ -1011,74 +1011,28 @@ void intel_guc_submission_fini(struct intel_guc *guc)
>   
>   static void guc_interrupts_capture(struct intel_gt *gt)
>   {
> -	struct intel_rps *rps = &gt->rps;
>   	struct intel_uncore *uncore = gt->uncore;
> -	struct intel_engine_cs *engine;
> -	enum intel_engine_id id;
> -	int irqs;
> +	u32 irqs = GT_CONTEXT_SWITCH_INTERRUPT;
> +	u32 dmask = irqs << 16 | irqs;
>   
> -	/* tell all command streamers to forward interrupts (but not vblank)
> -	 * to GuC
> -	 */
> -	irqs = _MASKED_BIT_ENABLE(GFX_INTERRUPT_STEERING);
> -	for_each_engine(engine, gt, id)
> -		ENGINE_WRITE(engine, RING_MODE_GEN7, irqs);
> -
> -	/* route USER_INTERRUPT to Host, all others are sent to GuC. */
> -	irqs = GT_RENDER_USER_INTERRUPT << GEN8_RCS_IRQ_SHIFT |
> -	       GT_RENDER_USER_INTERRUPT << GEN8_BCS_IRQ_SHIFT;
> -	/* These three registers have the same bit definitions */
> -	intel_uncore_write(uncore, GUC_BCS_RCS_IER, ~irqs);
> -	intel_uncore_write(uncore, GUC_VCS2_VCS1_IER, ~irqs);
> -	intel_uncore_write(uncore, GUC_WD_VECS_IER, ~irqs);
> +	GEM_BUG_ON(INTEL_GEN(gt->i915) < 11);
>   
> -	/*
> -	 * The REDIRECT_TO_GUC bit of the PMINTRMSK register directs all
> -	 * (unmasked) PM interrupts to the GuC. All other bits of this
> -	 * register *disable* generation of a specific interrupt.
> -	 *
> -	 * 'pm_intrmsk_mbz' indicates bits that are NOT to be set when
> -	 * writing to the PM interrupt mask register, i.e. interrupts
> -	 * that must not be disabled.
> -	 *
> -	 * If the GuC is handling these interrupts, then we must not let
> -	 * the PM code disable ANY interrupt that the GuC is expecting.
> -	 * So for each ENABLED (0) bit in this register, we must SET the
> -	 * bit in pm_intrmsk_mbz so that it's left enabled for the GuC.
> -	 * GuC needs ARAT expired interrupt unmasked hence it is set in
> -	 * pm_intrmsk_mbz.
> -	 *
> -	 * Here we CLEAR REDIRECT_TO_GUC bit in pm_intrmsk_mbz, which will
> -	 * result in the register bit being left SET!
> -	 */
> -	rps->pm_intrmsk_mbz |= ARAT_EXPIRED_INTRMSK;
> -	rps->pm_intrmsk_mbz &= ~GEN8_PMINTR_DISABLE_REDIRECT_TO_GUC;
> +	/* Don't handle the ctx switch interrupt in GuC submission mode */
> +	intel_uncore_rmw(uncore, GEN11_RENDER_COPY_INTR_ENABLE, dmask, 0);
> +	intel_uncore_rmw(uncore, GEN11_VCS_VECS_INTR_ENABLE, dmask, 0);
>   }
>   
>   static void guc_interrupts_release(struct intel_gt *gt)
>   {
> -	struct intel_rps *rps = &gt->rps;
>   	struct intel_uncore *uncore = gt->uncore;
> -	struct intel_engine_cs *engine;
> -	enum intel_engine_id id;
> -	int irqs;
> +	u32 irqs = GT_CONTEXT_SWITCH_INTERRUPT;
> +	u32 dmask = irqs << 16 | irqs;
>   
> -	/*
> -	 * tell all command streamers NOT to forward interrupts or vblank
> -	 * to GuC.
> -	 */
> -	irqs = _MASKED_FIELD(GFX_FORWARD_VBLANK_MASK, GFX_FORWARD_VBLANK_NEVER);
> -	irqs |= _MASKED_BIT_DISABLE(GFX_INTERRUPT_STEERING);
> -	for_each_engine(engine, gt, id)
> -		ENGINE_WRITE(engine, RING_MODE_GEN7, irqs);
> -
> -	/* route all GT interrupts to the host */
> -	intel_uncore_write(uncore, GUC_BCS_RCS_IER, 0);
> -	intel_uncore_write(uncore, GUC_VCS2_VCS1_IER, 0);
> -	intel_uncore_write(uncore, GUC_WD_VECS_IER, 0);
> -
> -	rps->pm_intrmsk_mbz |= GEN8_PMINTR_DISABLE_REDIRECT_TO_GUC;
> -	rps->pm_intrmsk_mbz &= ~ARAT_EXPIRED_INTRMSK;
> +	GEM_BUG_ON(INTEL_GEN(gt->i915) < 11);
> +
> +	/* Handle ctx switch interrupts again */
> +	intel_uncore_rmw(uncore, GEN11_RENDER_COPY_INTR_ENABLE, 0, dmask);
> +	intel_uncore_rmw(uncore, GEN11_VCS_VECS_INTR_ENABLE, 0, dmask);
>   }
>   
>   static void guc_set_default_submission(struct intel_engine_cs *engine)
>

Patch
diff mbox series

diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c
index 20d6ee148afc..08a38a3b90b0 100644
--- a/drivers/gpu/drm/i915/gt/intel_rps.c
+++ b/drivers/gpu/drm/i915/gt/intel_rps.c
@@ -1663,7 +1663,7 @@  void intel_rps_init(struct intel_rps *rps)
 	if (INTEL_GEN(i915) <= 7)
 		rps->pm_intrmsk_mbz |= GEN6_PM_RP_UP_EI_EXPIRED;
 
-	if (INTEL_GEN(i915) >= 8)
+	if (INTEL_GEN(i915) >= 8 && INTEL_GEN(i915) < 11)
 		rps->pm_intrmsk_mbz |= GEN8_PMINTR_DISABLE_REDIRECT_TO_GUC;
 }
 
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 2498c55e0ea5..902e25eb6b9d 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -1011,74 +1011,28 @@  void intel_guc_submission_fini(struct intel_guc *guc)
 
 static void guc_interrupts_capture(struct intel_gt *gt)
 {
-	struct intel_rps *rps = &gt->rps;
 	struct intel_uncore *uncore = gt->uncore;
-	struct intel_engine_cs *engine;
-	enum intel_engine_id id;
-	int irqs;
+	u32 irqs = GT_CONTEXT_SWITCH_INTERRUPT;
+	u32 dmask = irqs << 16 | irqs;
 
-	/* tell all command streamers to forward interrupts (but not vblank)
-	 * to GuC
-	 */
-	irqs = _MASKED_BIT_ENABLE(GFX_INTERRUPT_STEERING);
-	for_each_engine(engine, gt, id)
-		ENGINE_WRITE(engine, RING_MODE_GEN7, irqs);
-
-	/* route USER_INTERRUPT to Host, all others are sent to GuC. */
-	irqs = GT_RENDER_USER_INTERRUPT << GEN8_RCS_IRQ_SHIFT |
-	       GT_RENDER_USER_INTERRUPT << GEN8_BCS_IRQ_SHIFT;
-	/* These three registers have the same bit definitions */
-	intel_uncore_write(uncore, GUC_BCS_RCS_IER, ~irqs);
-	intel_uncore_write(uncore, GUC_VCS2_VCS1_IER, ~irqs);
-	intel_uncore_write(uncore, GUC_WD_VECS_IER, ~irqs);
+	GEM_BUG_ON(INTEL_GEN(gt->i915) < 11);
 
-	/*
-	 * The REDIRECT_TO_GUC bit of the PMINTRMSK register directs all
-	 * (unmasked) PM interrupts to the GuC. All other bits of this
-	 * register *disable* generation of a specific interrupt.
-	 *
-	 * 'pm_intrmsk_mbz' indicates bits that are NOT to be set when
-	 * writing to the PM interrupt mask register, i.e. interrupts
-	 * that must not be disabled.
-	 *
-	 * If the GuC is handling these interrupts, then we must not let
-	 * the PM code disable ANY interrupt that the GuC is expecting.
-	 * So for each ENABLED (0) bit in this register, we must SET the
-	 * bit in pm_intrmsk_mbz so that it's left enabled for the GuC.
-	 * GuC needs ARAT expired interrupt unmasked hence it is set in
-	 * pm_intrmsk_mbz.
-	 *
-	 * Here we CLEAR REDIRECT_TO_GUC bit in pm_intrmsk_mbz, which will
-	 * result in the register bit being left SET!
-	 */
-	rps->pm_intrmsk_mbz |= ARAT_EXPIRED_INTRMSK;
-	rps->pm_intrmsk_mbz &= ~GEN8_PMINTR_DISABLE_REDIRECT_TO_GUC;
+	/* Don't handle the ctx switch interrupt in GuC submission mode */
+	intel_uncore_rmw(uncore, GEN11_RENDER_COPY_INTR_ENABLE, dmask, 0);
+	intel_uncore_rmw(uncore, GEN11_VCS_VECS_INTR_ENABLE, dmask, 0);
 }
 
 static void guc_interrupts_release(struct intel_gt *gt)
 {
-	struct intel_rps *rps = &gt->rps;
 	struct intel_uncore *uncore = gt->uncore;
-	struct intel_engine_cs *engine;
-	enum intel_engine_id id;
-	int irqs;
+	u32 irqs = GT_CONTEXT_SWITCH_INTERRUPT;
+	u32 dmask = irqs << 16 | irqs;
 
-	/*
-	 * tell all command streamers NOT to forward interrupts or vblank
-	 * to GuC.
-	 */
-	irqs = _MASKED_FIELD(GFX_FORWARD_VBLANK_MASK, GFX_FORWARD_VBLANK_NEVER);
-	irqs |= _MASKED_BIT_DISABLE(GFX_INTERRUPT_STEERING);
-	for_each_engine(engine, gt, id)
-		ENGINE_WRITE(engine, RING_MODE_GEN7, irqs);
-
-	/* route all GT interrupts to the host */
-	intel_uncore_write(uncore, GUC_BCS_RCS_IER, 0);
-	intel_uncore_write(uncore, GUC_VCS2_VCS1_IER, 0);
-	intel_uncore_write(uncore, GUC_WD_VECS_IER, 0);
-
-	rps->pm_intrmsk_mbz |= GEN8_PMINTR_DISABLE_REDIRECT_TO_GUC;
-	rps->pm_intrmsk_mbz &= ~ARAT_EXPIRED_INTRMSK;
+	GEM_BUG_ON(INTEL_GEN(gt->i915) < 11);
+
+	/* Handle ctx switch interrupts again */
+	intel_uncore_rmw(uncore, GEN11_RENDER_COPY_INTR_ENABLE, 0, dmask);
+	intel_uncore_rmw(uncore, GEN11_VCS_VECS_INTR_ENABLE, 0, dmask);
 }
 
 static void guc_set_default_submission(struct intel_engine_cs *engine)