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

[Mika Kuoppala]

Paulo Zanoni  writes:

> Em Ter, 2018-02-27 às 11:51 -0800, Daniele Ceraolo Spurio escreveu:
>> 
>> 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 
>> > Cc: Daniele Ceraolo Spurio 
>> > Cc: Chris Wilson 
>> > Cc: Oscar Mateo 
>> > Signed-off-by: Tvrtko Ursulin 
>> > Signed-off-by: Rodrigo Vivi 
>> > Signed-off-by: Daniele Ceraolo Spurio > > com>
>> > Signed-off-by: Oscar Mateo 
>> > Signed-off-by: Paulo Zanoni 
>> > Signed-off-by: Mika Kuoppala 
>> > ---
>> >   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(_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(_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);
>> > +  

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

[Paulo Zanoni]

Em Ter, 2018-02-27 às 11:51 -0800, Daniele Ceraolo Spurio escreveu:
> 
> 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 
> > Cc: Daniele Ceraolo Spurio 
> > Cc: Chris Wilson 
> > Cc: Oscar Mateo 
> > Signed-off-by: Tvrtko Ursulin 
> > Signed-off-by: Rodrigo Vivi 
> > Signed-off-by: Daniele Ceraolo Spurio  > com>
> > Signed-off-by: Oscar Mateo 
> > Signed-off-by: Paulo Zanoni 
> > Signed-off-by: Mika Kuoppala 
> > ---
> >   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(_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(_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);
> > +   }
> > +   }
> > +}
> > +

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

[Daniele Ceraolo Spurio]

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 
Cc: Daniele Ceraolo Spurio 
Cc: Chris Wilson 
Cc: Oscar Mateo 
Signed-off-by: Tvrtko Ursulin 
Signed-off-by: Rodrigo Vivi 
Signed-off-by: Daniele Ceraolo Spurio 
Signed-off-by: Oscar Mateo 
Signed-off-by: Paulo Zanoni 
Signed-off-by: Mika Kuoppala 
---
  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(_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(_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 = 

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

[Mika Kuoppala]

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 
Cc: Daniele Ceraolo Spurio 
Cc: Chris Wilson 
Cc: Oscar Mateo 
Signed-off-by: Tvrtko Ursulin 
Signed-off-by: Rodrigo Vivi 
Signed-off-by: Daniele Ceraolo Spurio 
Signed-off-by: Oscar Mateo 
Signed-off-by: Paulo Zanoni 
Signed-off-by: Mika Kuoppala 
---
 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(_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(_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 =