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 <tvrtko.ursulin@xxxxxxxxx>
> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
> > Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > Cc: Oscar Mateo <oscar.mateo@xxxxxxxxx>
> > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> > Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.
> > com>
> > Signed-off-by: Oscar Mateo <oscar.mateo@xxxxxxxxx>
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
> > Signed-off-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
> > ---
> > 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.
We have a patch for the GSE bit. Also, a few other display-related
interrupts have changed, and we have patches for them too.
All the display-related interrupt patches depend on this patch here, I
didn't send them yet to the list since I realized this patch here would
still get a few new versions, generating more and conflicts on each
version.
Once this one is merged we can send the display interrupt ones.
>
> Anyway, nothing is functionally wrong so with a comment to explain
> about
> disp_ctl:
>
> reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
>
> 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_PI
> > PE(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)) {
> >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
