[Intel-gfx] [PATCH 4/4] drm/i915/icl: Interrupt handling

Daniele Ceraolo Spurio daniele.ceraolospurio at intel.com
Tue Feb 27 19:51:39 UTC 2018



On 20/02/18 07:37, Mika Kuoppala wrote:
> v2: Rebase.
> 
> v3:
>    * Remove DPF, it has been removed from SKL+.
>    * Fix -internal rebase wrt. execlists interrupt handling.
> 
> v4: Rebase.
> 
> v5:
>    * Updated for POR changes. (Daniele Ceraolo Spurio)
>    * Merged with irq handling fixes by Daniele Ceraolo Spurio:
>        * Simplify the code by using gen8_cs_irq_handler.
>        * Fix interrupt handling for the upstream kernel.
> 
> v6:
>    * Remove early bringup debug messages (Tvrtko)
>    * Add NB about arbitrary spin wait timeout (Tvrtko)
> 
> v7 (from Paulo):
>    * Don't try to write RO bits to registers.
>    * Don't check for PCH types that don't exist. PCH interrupts are not
>      here yet.
> 
> v9:
>    * squashed in selector and shared register handling (Daniele)
>    * skip writing of irq if data is not valid (Daniele)
>    * use time_after32 (Chris)
>    * use I915_MAX_VCS and I915_MAX_VECS (Daniele)
>    * remove fake pm interrupt handling for later patch (Mika)
> 
> v10:
>    * Direct processing of banks. clear banks early (Chris)
>    * remove poll on valid bit, only clear valid bit (Mika)
>    * use raw accessors, better naming (Chris)
> 
> v11:
>    * adapt to raw_reg_[read|write]
>    * bring back polling the valid bit (Daniele)
> 
> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Oscar Mateo <oscar.mateo at intel.com>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Signed-off-by: Oscar Mateo <oscar.mateo at intel.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
> Signed-off-by: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> ---
>   drivers/gpu/drm/i915/i915_irq.c | 229 ++++++++++++++++++++++++++++++++++++++++
>   drivers/gpu/drm/i915/intel_pm.c |   7 +-
>   2 files changed, 235 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 17de6cef2a30..a79f47ac742a 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -415,6 +415,9 @@ void gen6_enable_rps_interrupts(struct drm_i915_private *dev_priv)
>   	if (READ_ONCE(rps->interrupts_enabled))
>   		return;
>   
> +	if (WARN_ON_ONCE(IS_GEN11(dev_priv)))
> +		return;
> +
>   	spin_lock_irq(&dev_priv->irq_lock);
>   	WARN_ON_ONCE(rps->pm_iir);
>   	WARN_ON_ONCE(I915_READ(gen6_pm_iir(dev_priv)) & dev_priv->pm_rps_events);
> @@ -431,6 +434,9 @@ void gen6_disable_rps_interrupts(struct drm_i915_private *dev_priv)
>   	if (!READ_ONCE(rps->interrupts_enabled))
>   		return;
>   
> +	if (WARN_ON_ONCE(IS_GEN11(dev_priv)))
> +		return;
> +
>   	spin_lock_irq(&dev_priv->irq_lock);
>   	rps->interrupts_enabled = false;
>   
> @@ -2755,6 +2761,150 @@ static void __fini_wedge(struct wedge_me *w)
>   	     (W)->i915;							\
>   	     __fini_wedge((W)))
>   
> +static __always_inline void
> +gen11_cs_irq_handler(struct intel_engine_cs * const engine, const u32 iir)
> +{
> +	gen8_cs_irq_handler(engine, iir, 0);
> +}
> +
> +static void
> +gen11_gt_engine_irq_handler(struct drm_i915_private * const i915,
> +			    const unsigned int bank,
> +			    const unsigned int engine_n,
> +			    const u16 iir)
> +{
> +	struct intel_engine_cs ** const engine = i915->engine;
> +
> +	switch (bank) {
> +	case 0:
> +		switch (engine_n) {
> +
> +		case GEN11_RCS0:
> +			return gen11_cs_irq_handler(engine[RCS], iir);
> +
> +		case GEN11_BCS:
> +			return gen11_cs_irq_handler(engine[BCS], iir);
> +		}
> +	case 1:
> +		switch (engine_n) {
> +
> +		case GEN11_VCS(0):
> +			return gen11_cs_irq_handler(engine[_VCS(0)], iir);
> +		case GEN11_VCS(1):
> +			return gen11_cs_irq_handler(engine[_VCS(1)], iir);
> +		case GEN11_VCS(2):
> +			return gen11_cs_irq_handler(engine[_VCS(2)], iir);
> +		case GEN11_VCS(3):
> +			return gen11_cs_irq_handler(engine[_VCS(3)], iir);
> +
> +		case GEN11_VECS(0):
> +			return gen11_cs_irq_handler(engine[_VECS(0)], iir);
> +		case GEN11_VECS(1):
> +			return gen11_cs_irq_handler(engine[_VECS(1)], iir);
> +		}
> +	}
> +}
> +
> +static u32
> +gen11_gt_engine_intr(struct drm_i915_private * const i915,
> +		     const unsigned int bank, const unsigned int bit)
> +{
> +	void __iomem * const regs = i915->regs;
> +	u32 timeout_ts;
> +	u32 ident;
> +
> +	raw_reg_write(regs, GEN11_IIR_REG_SELECTOR(bank), BIT(bit));
> +
> +	/*
> +	 * NB: Specs do not specify how long to spin wait,
> +	 * so we do ~100us as an educated guess.
> +	 */
> +	timeout_ts = (local_clock() >> 10) + 100;
> +	do {
> +		ident = raw_reg_read(regs, GEN11_INTR_IDENTITY_REG(bank));
> +	} while (!(ident & GEN11_INTR_DATA_VALID) &&
> +		 !time_after32(local_clock() >> 10, timeout_ts));
> +
> +	if (unlikely(!(ident & GEN11_INTR_DATA_VALID))) {
> +		DRM_ERROR("INTR_IDENTITY_REG%u:%u 0x%08x not valid!\n",
> +			  bank, bit, ident);
> +		return 0;
> +	}
> +
> +	raw_reg_write(regs, GEN11_INTR_IDENTITY_REG(bank),
> +		      GEN11_INTR_DATA_VALID);
> +
> +	return ident & GEN11_INTR_ENGINE_MASK;
> +}
> +
> +static void
> +gen11_gt_irq_handler(struct drm_i915_private * const i915,
> +		     const u32 master_ctl)
> +{
> +	void __iomem * const regs = i915->regs;
> +	unsigned int bank;
> +
> +	for (bank = 0; bank < 2; bank++) {
> +		unsigned long intr_dw;
> +		unsigned int bit;
> +
> +		if (!(master_ctl & GEN11_GT_DW_IRQ(bank)))
> +			continue;
> +
> +		intr_dw = raw_reg_read(regs, GEN11_GT_INTR_DW(bank));
> +
> +		if (unlikely(!intr_dw))
> +			DRM_ERROR("GT_INTR_DW%u blank!\n", bank);

Could continue here, since the other operations in the loop won't be 
meaningful if intr_dw=0

> +
> +		for_each_set_bit(bit, &intr_dw, 32) {
> +			const u16 iir = gen11_gt_engine_intr(i915, bank, bit);
> +
> +			if (unlikely(!iir))
> +				continue;
> +
> +			gen11_gt_engine_irq_handler(i915, bank, bit, iir);
> +		}
> +
> +		/* Clear must be after shared has been served for engine */
> +		raw_reg_write(regs, GEN11_GT_INTR_DW(bank), intr_dw);
> +	}
> +}
> +
> +static irqreturn_t gen11_irq_handler(int irq, void *arg)
> +{
> +	struct drm_i915_private * const i915 = to_i915(arg);
> +	void __iomem * const regs = i915->regs;
> +	u32 master_ctl;
> +
> +	if (!intel_irqs_enabled(i915))
> +		return IRQ_NONE;
> +
> +	master_ctl = raw_reg_read(regs, GEN11_GFX_MSTR_IRQ);
> +	master_ctl &= ~GEN11_MASTER_IRQ;
> +	if (!master_ctl)
> +		return IRQ_NONE;
> +
> +	/* Disable interrupts. */
> +	raw_reg_write(regs, GEN11_GFX_MSTR_IRQ, 0);
> +
> +	/* Find, clear, then process each source of interrupt. */
> +	gen11_gt_irq_handler(i915, master_ctl);
> +
> +	/* IRQs are synced during runtime_suspend, we don't require a wakeref */
> +	if (master_ctl & GEN11_DISPLAY_IRQ) {
> +		const u32 disp_ctl = raw_reg_read(regs, GEN11_DISPLAY_INT_CTL);
> +
> +		disable_rpm_wakeref_asserts(i915);
> +		gen8_de_irq_handler(i915, disp_ctl);

gen8_de_irq_handler refers to the provided value as "master_ctl". 
GEN11_DISPLAY_INT_CTL has the same format as GEN8_MASTER_IRQ for the 
display-related bits (16-30) so it is ok to provide disp_ctl, but we 
could use a comment to explain that. Also, gen8_de_irq_handler logs 
errors blaming the master when things go wrong, so we could update that 
to make things clearer, but that can come as a follow up.

I've checked the bit definitions of the registers used in 
gen8_de_irq_handler and some of the bits have moved, but AFAICS among 
the ones we use the only one that is impacted is GEN8_DE_MISC_GSE, which 
is now gone (bit is now reserved). This shouldn't impact 
gen8_de_irq_handler because the interrupt shouldn't trigger at all, but 
we could avoid enabling it from gen8_de_irq_postinstall.

Anyway, nothing is functionally wrong so with a comment to explain about 
disp_ctl:

reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>

Still, an ack from someone more knowledgeable than me with display code 
would be nice ;)

Daniele

> +		enable_rpm_wakeref_asserts(i915);
> +	}
> +
> +	/* Acknowledge and enable interrupts. */
> +	raw_reg_write(regs, GEN11_GFX_MSTR_IRQ, GEN11_MASTER_IRQ | master_ctl);
> +
> +	return IRQ_HANDLED;
> +}
> +
>   /**
>    * i915_reset_device - do process context error handling work
>    * @dev_priv: i915 device private
> @@ -3180,6 +3330,42 @@ static void gen8_irq_reset(struct drm_device *dev)
>   		ibx_irq_reset(dev_priv);
>   }
>   
> +static void gen11_gt_irq_reset(struct drm_i915_private *dev_priv)
> +{
> +	/* Disable RCS, BCS, VCS and VECS class engines. */
> +	I915_WRITE(GEN11_RENDER_COPY_INTR_ENABLE, 0);
> +	I915_WRITE(GEN11_VCS_VECS_INTR_ENABLE,	  0);
> +
> +	/* Restore masks irqs on RCS, BCS, VCS and VECS engines. */
> +	I915_WRITE(GEN11_RCS0_RSVD_INTR_MASK,	~0);
> +	I915_WRITE(GEN11_BCS_RSVD_INTR_MASK,	~0);
> +	I915_WRITE(GEN11_VCS0_VCS1_INTR_MASK,	~0);
> +	I915_WRITE(GEN11_VCS2_VCS3_INTR_MASK,	~0);
> +	I915_WRITE(GEN11_VECS0_VECS1_INTR_MASK,	~0);
> +}
> +
> +static void gen11_irq_reset(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	int pipe;
> +
> +	I915_WRITE(GEN11_GFX_MSTR_IRQ, 0);
> +	POSTING_READ(GEN11_GFX_MSTR_IRQ);
> +
> +	gen11_gt_irq_reset(dev_priv);
> +
> +	I915_WRITE(GEN11_DISPLAY_INT_CTL, 0);
> +
> +	for_each_pipe(dev_priv, pipe)
> +		if (intel_display_power_is_enabled(dev_priv,
> +						   POWER_DOMAIN_PIPE(pipe)))
> +			GEN8_IRQ_RESET_NDX(DE_PIPE, pipe);
> +
> +	GEN3_IRQ_RESET(GEN8_DE_PORT_);
> +	GEN3_IRQ_RESET(GEN8_DE_MISC_);
> +	GEN3_IRQ_RESET(GEN8_PCU_);
> +}
> +
>   void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv,
>   				     u8 pipe_mask)
>   {
> @@ -3677,6 +3863,41 @@ static int gen8_irq_postinstall(struct drm_device *dev)
>   	return 0;
>   }
>   
> +static void gen11_gt_irq_postinstall(struct drm_i915_private *dev_priv)
> +{
> +	const u32 irqs = GT_RENDER_USER_INTERRUPT | GT_CONTEXT_SWITCH_INTERRUPT;
> +
> +	BUILD_BUG_ON(irqs & 0xffff0000);
> +
> +	/* Enable RCS, BCS, VCS and VECS class interrupts. */
> +	I915_WRITE(GEN11_RENDER_COPY_INTR_ENABLE, irqs << 16 | irqs);
> +	I915_WRITE(GEN11_VCS_VECS_INTR_ENABLE,	  irqs << 16 | irqs);
> +
> +	/* Unmask irqs on RCS, BCS, VCS and VECS engines. */
> +	I915_WRITE(GEN11_RCS0_RSVD_INTR_MASK,	~(irqs << 16));
> +	I915_WRITE(GEN11_BCS_RSVD_INTR_MASK,	~(irqs << 16));
> +	I915_WRITE(GEN11_VCS0_VCS1_INTR_MASK,	~(irqs | irqs << 16));
> +	I915_WRITE(GEN11_VCS2_VCS3_INTR_MASK,	~(irqs | irqs << 16));
> +	I915_WRITE(GEN11_VECS0_VECS1_INTR_MASK,	~(irqs | irqs << 16));
> +
> +	dev_priv->pm_imr = 0xffffffff; /* TODO */
> +}
> +
> +static int gen11_irq_postinstall(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	gen11_gt_irq_postinstall(dev_priv);
> +	gen8_de_irq_postinstall(dev_priv);
> +
> +	I915_WRITE(GEN11_DISPLAY_INT_CTL, GEN11_DISPLAY_IRQ_ENABLE);
> +
> +	I915_WRITE(GEN11_GFX_MSTR_IRQ, GEN11_MASTER_IRQ);
> +	POSTING_READ(GEN11_GFX_MSTR_IRQ);
> +
> +	return 0;
> +}
> +
>   static int cherryview_irq_postinstall(struct drm_device *dev)
>   {
>   	struct drm_i915_private *dev_priv = to_i915(dev);
> @@ -4125,6 +4346,14 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
>   		dev->driver->enable_vblank = i965_enable_vblank;
>   		dev->driver->disable_vblank = i965_disable_vblank;
>   		dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup;
> +	} else if (INTEL_GEN(dev_priv) >= 11) {
> +		dev->driver->irq_handler = gen11_irq_handler;
> +		dev->driver->irq_preinstall = gen11_irq_reset;
> +		dev->driver->irq_postinstall = gen11_irq_postinstall;
> +		dev->driver->irq_uninstall = gen11_irq_reset;
> +		dev->driver->enable_vblank = gen8_enable_vblank;
> +		dev->driver->disable_vblank = gen8_disable_vblank;
> +		dev_priv->display.hpd_irq_setup = spt_hpd_irq_setup;
>   	} else if (INTEL_GEN(dev_priv) >= 8) {
>   		dev->driver->irq_handler = gen8_irq_handler;
>   		dev->driver->irq_preinstall = gen8_irq_reset;
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index a88f0f213604..7a316f867b42 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -8030,7 +8030,10 @@ void intel_sanitize_gt_powersave(struct drm_i915_private *dev_priv)
>   	dev_priv->gt_pm.rc6.enabled = true; /* force RC6 disabling */
>   	intel_disable_gt_powersave(dev_priv);
>   
> -	gen6_reset_rps_interrupts(dev_priv);
> +	if (INTEL_GEN(dev_priv) < 11)
> +		gen6_reset_rps_interrupts(dev_priv);
> +	else
> +		WARN_ON_ONCE(1);
>   }
>   
>   static inline void intel_disable_llc_pstate(struct drm_i915_private *i915)
> @@ -8143,6 +8146,8 @@ static void intel_enable_rps(struct drm_i915_private *dev_priv)
>   		cherryview_enable_rps(dev_priv);
>   	} else if (IS_VALLEYVIEW(dev_priv)) {
>   		valleyview_enable_rps(dev_priv);
> +	} else if (WARN_ON_ONCE(INTEL_GEN(dev_priv) >= 11)) {
> +		/* TODO */
>   	} else if (INTEL_GEN(dev_priv) >= 9) {
>   		gen9_enable_rps(dev_priv);
>   	} else if (IS_BROADWELL(dev_priv)) {
> 


More information about the Intel-gfx mailing list