Re: [PATCH v2] drm/i915/guc: Properly capture & release GuC interrupts on Gen11+

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

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)
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx