[PATCH 0/3] More mmio and intel_gt cleanups and refactorings

[PATCH 0/3] More mmio and intel_gt cleanups and refactorings

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

A mini-series which converts to intel_gt and removes some legacy mmio accessors
mostly around interrupt and RPS handling areas.

All patches were actually started by Paulo and I have attributed that with the
Co-authored-by tag. But given rebases and some other changes I made, I did take
authorship. Paulo please state your preference - are you happy with this or
you would prefer I swap our names between the author and co-authored tags?

Tvrtko Ursulin (3):
  drm/i915: Rework some interrupt handling functions to take intel_gt
  drm/i915: Remove some legacy mmio accessors from interrupt handling
  drm/i915: Move dev_priv->pm_i{m,e}r into intel_gt

 drivers/gpu/drm/i915/gt/intel_gt_types.h   |   3 +
 drivers/gpu/drm/i915/gt/intel_ringbuffer.c |   4 +-
 drivers/gpu/drm/i915/i915_drv.h            |   2 -
 drivers/gpu/drm/i915/i915_irq.c            | 276 +++++++++++----------
 drivers/gpu/drm/i915/i915_irq.h            |   4 +-
 5 files changed, 152 insertions(+), 137 deletions(-)

-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/3] drm/i915: Rework some interrupt handling functions to take intel_gt

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Some interrupt handling functions already have gt in their names
suggesting them as obvious candidates to make them take struct intel_gt
instead of i915.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Co-authored-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 88 +++++++++++++++++----------------
 1 file changed, 46 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 73f0338faf9f..952053e611f8 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -305,17 +305,17 @@ void i915_hotplug_interrupt_update(struct drm_i915_private *dev_priv,
 }
 
 static u32
-gen11_gt_engine_identity(struct drm_i915_private * const i915,
+gen11_gt_engine_identity(struct intel_gt *gt,
 			 const unsigned int bank, const unsigned int bit);
 
-static bool gen11_reset_one_iir(struct drm_i915_private * const i915,
+static bool gen11_reset_one_iir(struct intel_gt *gt,
 				const unsigned int bank,
 				const unsigned int bit)
 {
-	void __iomem * const regs = i915->uncore.regs;
+	void __iomem * const regs = gt->uncore->regs;
 	u32 dw;
 
-	lockdep_assert_held(&i915->irq_lock);
+	lockdep_assert_held(&gt->i915->irq_lock);
 
 	dw = raw_reg_read(regs, GEN11_GT_INTR_DW(bank));
 	if (dw & BIT(bit)) {
@@ -323,7 +323,7 @@ static bool gen11_reset_one_iir(struct drm_i915_private * const i915,
 		 * According to the BSpec, DW_IIR bits cannot be cleared without
 		 * first servicing the Selector & Shared IIR registers.
 		 */
-		gen11_gt_engine_identity(i915, bank, bit);
+		gen11_gt_engine_identity(gt, bank, bit);
 
 		/*
 		 * We locked GT INT DW by reading it. If we want to (try
@@ -528,7 +528,7 @@ void gen11_reset_rps_interrupts(struct drm_i915_private *dev_priv)
 {
 	spin_lock_irq(&dev_priv->irq_lock);
 
-	while (gen11_reset_one_iir(dev_priv, 0, GEN11_GTPM))
+	while (gen11_reset_one_iir(&dev_priv->gt, 0, GEN11_GTPM))
 		;
 
 	dev_priv->gt_pm.rps.pm_iir = 0;
@@ -555,7 +555,7 @@ void gen6_enable_rps_interrupts(struct drm_i915_private *dev_priv)
 	WARN_ON_ONCE(rps->pm_iir);
 
 	if (INTEL_GEN(dev_priv) >= 11)
-		WARN_ON_ONCE(gen11_reset_one_iir(dev_priv, 0, GEN11_GTPM));
+		WARN_ON_ONCE(gen11_reset_one_iir(&dev_priv->gt, 0, GEN11_GTPM));
 	else
 		WARN_ON_ONCE(I915_READ(gen6_pm_iir(dev_priv)) & dev_priv->pm_rps_events);
 
@@ -635,7 +635,7 @@ void gen9_disable_guc_interrupts(struct drm_i915_private *dev_priv)
 void gen11_reset_guc_interrupts(struct drm_i915_private *i915)
 {
 	spin_lock_irq(&i915->irq_lock);
-	gen11_reset_one_iir(i915, 0, GEN11_GUC);
+	gen11_reset_one_iir(&i915->gt, 0, GEN11_GUC);
 	spin_unlock_irq(&i915->irq_lock);
 }
 
@@ -646,7 +646,7 @@ void gen11_enable_guc_interrupts(struct drm_i915_private *dev_priv)
 		u32 events = REG_FIELD_PREP(ENGINE1_MASK,
 					    GEN11_GUC_INTR_GUC2HOST);
 
-		WARN_ON_ONCE(gen11_reset_one_iir(dev_priv, 0, GEN11_GUC));
+		WARN_ON_ONCE(gen11_reset_one_iir(&dev_priv->gt, 0, GEN11_GUC));
 		I915_WRITE(GEN11_GUC_SG_INTR_ENABLE, events);
 		I915_WRITE(GEN11_GUC_SG_INTR_MASK, ~events);
 		dev_priv->guc.interrupts.enabled = true;
@@ -3033,14 +3033,14 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg)
 }
 
 static u32
-gen11_gt_engine_identity(struct drm_i915_private * const i915,
+gen11_gt_engine_identity(struct intel_gt *gt,
 			 const unsigned int bank, const unsigned int bit)
 {
-	void __iomem * const regs = i915->uncore.regs;
+	void __iomem * const regs = gt->uncore->regs;
 	u32 timeout_ts;
 	u32 ident;
 
-	lockdep_assert_held(&i915->irq_lock);
+	lockdep_assert_held(&gt->i915->irq_lock);
 
 	raw_reg_write(regs, GEN11_IIR_REG_SELECTOR(bank), BIT(bit));
 
@@ -3067,9 +3067,11 @@ gen11_gt_engine_identity(struct drm_i915_private * const i915,
 }
 
 static void
-gen11_other_irq_handler(struct drm_i915_private * const i915,
-			const u8 instance, const u16 iir)
+gen11_other_irq_handler(struct intel_gt *gt, const u8 instance,
+			const u16 iir)
 {
+	struct drm_i915_private *i915 = gt->i915;
+
 	if (instance == OTHER_GUC_INSTANCE)
 		return gen11_guc_irq_handler(i915, iir);
 
@@ -3081,13 +3083,13 @@ gen11_other_irq_handler(struct drm_i915_private * const i915,
 }
 
 static void
-gen11_engine_irq_handler(struct drm_i915_private * const i915,
-			 const u8 class, const u8 instance, const u16 iir)
+gen11_engine_irq_handler(struct intel_gt *gt, const u8 class,
+			 const u8 instance, const u16 iir)
 {
 	struct intel_engine_cs *engine;
 
 	if (instance <= MAX_ENGINE_INSTANCE)
-		engine = i915->engine_class[class][instance];
+		engine = gt->i915->engine_class[class][instance];
 	else
 		engine = NULL;
 
@@ -3099,8 +3101,7 @@ gen11_engine_irq_handler(struct drm_i915_private * const i915,
 }
 
 static void
-gen11_gt_identity_handler(struct drm_i915_private * const i915,
-			  const u32 identity)
+gen11_gt_identity_handler(struct intel_gt *gt, const u32 identity)
 {
 	const u8 class = GEN11_INTR_ENGINE_CLASS(identity);
 	const u8 instance = GEN11_INTR_ENGINE_INSTANCE(identity);
@@ -3110,31 +3111,30 @@ gen11_gt_identity_handler(struct drm_i915_private * const i915,
 		return;
 
 	if (class <= COPY_ENGINE_CLASS)
-		return gen11_engine_irq_handler(i915, class, instance, intr);
+		return gen11_engine_irq_handler(gt, class, instance, intr);
 
 	if (class == OTHER_CLASS)
-		return gen11_other_irq_handler(i915, instance, intr);
+		return gen11_other_irq_handler(gt, instance, intr);
 
 	WARN_ONCE(1, "unknown interrupt class=0x%x, instance=0x%x, intr=0x%x\n",
 		  class, instance, intr);
 }
 
 static void
-gen11_gt_bank_handler(struct drm_i915_private * const i915,
-		      const unsigned int bank)
+gen11_gt_bank_handler(struct intel_gt *gt, const unsigned int bank)
 {
-	void __iomem * const regs = i915->uncore.regs;
+	void __iomem * const regs = gt->uncore->regs;
 	unsigned long intr_dw;
 	unsigned int bit;
 
-	lockdep_assert_held(&i915->irq_lock);
+	lockdep_assert_held(&gt->i915->irq_lock);
 
 	intr_dw = raw_reg_read(regs, GEN11_GT_INTR_DW(bank));
 
 	for_each_set_bit(bit, &intr_dw, 32) {
-		const u32 ident = gen11_gt_engine_identity(i915, bank, bit);
+		const u32 ident = gen11_gt_engine_identity(gt, bank, bit);
 
-		gen11_gt_identity_handler(i915, ident);
+		gen11_gt_identity_handler(gt, ident);
 	}
 
 	/* Clear must be after shared has been served for engine */
@@ -3142,25 +3142,25 @@ gen11_gt_bank_handler(struct drm_i915_private * const i915,
 }
 
 static void
-gen11_gt_irq_handler(struct drm_i915_private * const i915,
-		     const u32 master_ctl)
+gen11_gt_irq_handler(struct intel_gt *gt, const u32 master_ctl)
 {
+	struct drm_i915_private *i915 = gt->i915;
 	unsigned int bank;
 
 	spin_lock(&i915->irq_lock);
 
 	for (bank = 0; bank < 2; bank++) {
 		if (master_ctl & GEN11_GT_DW_IRQ(bank))
-			gen11_gt_bank_handler(i915, bank);
+			gen11_gt_bank_handler(gt, bank);
 	}
 
 	spin_unlock(&i915->irq_lock);
 }
 
 static u32
-gen11_gu_misc_irq_ack(struct drm_i915_private *dev_priv, const u32 master_ctl)
+gen11_gu_misc_irq_ack(struct intel_gt *gt, const u32 master_ctl)
 {
-	void __iomem * const regs = dev_priv->uncore.regs;
+	void __iomem * const regs = gt->uncore->regs;
 	u32 iir;
 
 	if (!(master_ctl & GEN11_GU_MISC_IRQ))
@@ -3174,10 +3174,10 @@ gen11_gu_misc_irq_ack(struct drm_i915_private *dev_priv, const u32 master_ctl)
 }
 
 static void
-gen11_gu_misc_irq_handler(struct drm_i915_private *dev_priv, const u32 iir)
+gen11_gu_misc_irq_handler(struct intel_gt *gt, const u32 iir)
 {
 	if (iir & GEN11_GU_MISC_GSE)
-		intel_opregion_asle_intr(dev_priv);
+		intel_opregion_asle_intr(gt->i915);
 }
 
 static inline u32 gen11_master_intr_disable(void __iomem * const regs)
@@ -3202,6 +3202,7 @@ static irqreturn_t gen11_irq_handler(int irq, void *arg)
 {
 	struct drm_i915_private * const i915 = arg;
 	void __iomem * const regs = i915->uncore.regs;
+	struct intel_gt *gt = &i915->gt;
 	u32 master_ctl;
 	u32 gu_misc_iir;
 
@@ -3215,7 +3216,7 @@ static irqreturn_t gen11_irq_handler(int irq, void *arg)
 	}
 
 	/* Find, clear, then process each source of interrupt. */
-	gen11_gt_irq_handler(i915, master_ctl);
+	gen11_gt_irq_handler(gt, master_ctl);
 
 	/* IRQs are synced during runtime_suspend, we don't require a wakeref */
 	if (master_ctl & GEN11_DISPLAY_IRQ) {
@@ -3230,11 +3231,11 @@ static irqreturn_t gen11_irq_handler(int irq, void *arg)
 		enable_rpm_wakeref_asserts(&i915->runtime_pm);
 	}
 
-	gu_misc_iir = gen11_gu_misc_irq_ack(i915, master_ctl);
+	gu_misc_iir = gen11_gu_misc_irq_ack(gt, master_ctl);
 
 	gen11_master_intr_enable(regs);
 
-	gen11_gu_misc_irq_handler(i915, gu_misc_iir);
+	gen11_gu_misc_irq_handler(gt, gu_misc_iir);
 
 	return IRQ_HANDLED;
 }
@@ -3590,8 +3591,10 @@ static void gen8_irq_reset(struct drm_i915_private *dev_priv)
 		ibx_irq_reset(dev_priv);
 }
 
-static void gen11_gt_irq_reset(struct drm_i915_private *dev_priv)
+static void gen11_gt_irq_reset(struct intel_gt *gt)
 {
+	struct drm_i915_private *dev_priv = gt->i915;
+
 	/* Disable RCS, BCS, VCS and VECS class engines. */
 	I915_WRITE(GEN11_RENDER_COPY_INTR_ENABLE, 0);
 	I915_WRITE(GEN11_VCS_VECS_INTR_ENABLE,	  0);
@@ -3616,7 +3619,7 @@ static void gen11_irq_reset(struct drm_i915_private *dev_priv)
 
 	gen11_master_intr_disable(dev_priv->uncore.regs);
 
-	gen11_gt_irq_reset(dev_priv);
+	gen11_gt_irq_reset(&dev_priv->gt);
 
 	I915_WRITE(GEN11_DISPLAY_INT_CTL, 0);
 
@@ -4222,8 +4225,9 @@ static void gen8_irq_postinstall(struct drm_i915_private *dev_priv)
 	gen8_master_intr_enable(dev_priv->uncore.regs);
 }
 
-static void gen11_gt_irq_postinstall(struct drm_i915_private *dev_priv)
+static void gen11_gt_irq_postinstall(struct intel_gt *gt)
 {
+	struct drm_i915_private *dev_priv = gt->i915;
 	const u32 irqs = GT_RENDER_USER_INTERRUPT | GT_CONTEXT_SWITCH_INTERRUPT;
 
 	BUILD_BUG_ON(irqs & 0xffff0000);
@@ -4275,14 +4279,14 @@ static void gen11_irq_postinstall(struct drm_i915_private *dev_priv)
 	if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP)
 		icp_irq_postinstall(dev_priv);
 
-	gen11_gt_irq_postinstall(dev_priv);
+	gen11_gt_irq_postinstall(&dev_priv->gt);
 	gen8_de_irq_postinstall(dev_priv);
 
 	GEN3_IRQ_INIT(uncore, GEN11_GU_MISC_, ~gu_misc_masked, gu_misc_masked);
 
 	I915_WRITE(GEN11_DISPLAY_INT_CTL, GEN11_DISPLAY_IRQ_ENABLE);
 
-	gen11_master_intr_enable(dev_priv->uncore.regs);
+	gen11_master_intr_enable(uncore->regs);
 	POSTING_READ(GEN11_GFX_MSTR_IRQ);
 }
 
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915: Rework some interrupt handling functions to take intel_gt

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-07-02 11:23:11)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Some interrupt handling functions already have gt in their names
> suggesting them as obvious candidates to make them take struct intel_gt
> instead of i915.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Co-authored-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

>  static void
> -gen11_other_irq_handler(struct drm_i915_private * const i915,
> -                       const u8 instance, const u16 iir)
> +gen11_other_irq_handler(struct intel_gt *gt, const u8 instance,
> +                       const u16 iir)
>  {
> +       struct drm_i915_private *i915 = gt->i915;
> +
>         if (instance == OTHER_GUC_INSTANCE)
>                 return gen11_guc_irq_handler(i915, iir);

That looks like a candidate for gt as well. Even for the guc, the
interrupt vector is GT centric. I was hoping we could place guc/ parallel
to gt/, but it looks like it will indeed be a child of intel_gt.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915: Rework some interrupt handling functions to take intel_gt

[Tvrtko Ursulin]

On 02/07/2019 11:34, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-07-02 11:23:11)
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Some interrupt handling functions already have gt in their names
>> suggesting them as obvious candidates to make them take struct intel_gt
>> instead of i915.
>>
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> Co-authored-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> 
>>   static void
>> -gen11_other_irq_handler(struct drm_i915_private * const i915,
>> -                       const u8 instance, const u16 iir)
>> +gen11_other_irq_handler(struct intel_gt *gt, const u8 instance,
>> +                       const u16 iir)
>>   {
>> +       struct drm_i915_private *i915 = gt->i915;
>> +
>>          if (instance == OTHER_GUC_INSTANCE)
>>                  return gen11_guc_irq_handler(i915, iir);
> 
> That looks like a candidate for gt as well. Even for the guc, the
> interrupt vector is GT centric. I was hoping we could place guc/ parallel
> to gt/, but it looks like it will indeed be a child of intel_gt.

Yeah. Daniele will deal with the GuC code paths.

> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Thanks. I'll also let Daniele comment on whether this refactoring fits 
with his work before merging it.

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915: Rework some interrupt handling functions to take intel_gt

[Daniele Ceraolo Spurio]

On 7/2/19 4:45 AM, Tvrtko Ursulin wrote:
> 
> On 02/07/2019 11:34, Chris Wilson wrote:
>> Quoting Tvrtko Ursulin (2019-07-02 11:23:11)
>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>
>>> Some interrupt handling functions already have gt in their names
>>> suggesting them as obvious candidates to make them take struct intel_gt
>>> instead of i915.
>>>
>>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>>> Co-authored-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>>
>>>   static void
>>> -gen11_other_irq_handler(struct drm_i915_private * const i915,
>>> -                       const u8 instance, const u16 iir)
>>> +gen11_other_irq_handler(struct intel_gt *gt, const u8 instance,
>>> +                       const u16 iir)
>>>   {
>>> +       struct drm_i915_private *i915 = gt->i915;
>>> +
>>>          if (instance == OTHER_GUC_INSTANCE)
>>>                  return gen11_guc_irq_handler(i915, iir);
>>
>> That looks like a candidate for gt as well. Even for the guc, the
>> interrupt vector is GT centric. I was hoping we could place guc/ parallel
>> to gt/, but it looks like it will indeed be a child of intel_gt.
> 
> Yeah. Daniele will deal with the GuC code paths.
> 
>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Thanks. I'll also let Daniele comment on whether this refactoring fits 
> with his work before merging it.

Yup, works for me. On the whole series:

Acked-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

Daniele

> 
> Regards,
> 
> Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/3] drm/i915: Remove some legacy mmio accessors from interrupt handling

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Mostly in gen11 interrupt handling and a couple neighbouring functions
which were easy since uncore local was already available.

Co-authored-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 75 +++++++++++++++++----------------
 1 file changed, 39 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 952053e611f8..817deff20bdd 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -3479,12 +3479,12 @@ static void vlv_display_irq_reset(struct drm_i915_private *dev_priv)
 	struct intel_uncore *uncore = &dev_priv->uncore;
 
 	if (IS_CHERRYVIEW(dev_priv))
-		I915_WRITE(DPINVGTT, DPINVGTT_STATUS_MASK_CHV);
+		intel_uncore_write(uncore, DPINVGTT, DPINVGTT_STATUS_MASK_CHV);
 	else
-		I915_WRITE(DPINVGTT, DPINVGTT_STATUS_MASK);
+		intel_uncore_write(uncore, DPINVGTT, DPINVGTT_STATUS_MASK);
 
 	i915_hotplug_interrupt_update_locked(dev_priv, 0xffffffff, 0);
-	I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
+	intel_uncore_write(uncore, PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
 
 	i9xx_pipestat_irq_reset(dev_priv);
 
@@ -3531,11 +3531,11 @@ static void ironlake_irq_reset(struct drm_i915_private *dev_priv)
 
 	GEN3_IRQ_RESET(uncore, DE);
 	if (IS_GEN(dev_priv, 7))
-		I915_WRITE(GEN7_ERR_INT, 0xffffffff);
+		intel_uncore_write(uncore, GEN7_ERR_INT, 0xffffffff);
 
 	if (IS_HASWELL(dev_priv)) {
-		I915_WRITE(EDP_PSR_IMR, 0xffffffff);
-		I915_WRITE(EDP_PSR_IIR, 0xffffffff);
+		intel_uncore_write(uncore, EDP_PSR_IMR, 0xffffffff);
+		intel_uncore_write(uncore, EDP_PSR_IIR, 0xffffffff);
 	}
 
 	gen5_gt_irq_reset(dev_priv);
@@ -3575,8 +3575,8 @@ static void gen8_irq_reset(struct drm_i915_private *dev_priv)
 
 	gen8_gt_irq_reset(dev_priv);
 
-	I915_WRITE(EDP_PSR_IMR, 0xffffffff);
-	I915_WRITE(EDP_PSR_IIR, 0xffffffff);
+	intel_uncore_write(uncore, EDP_PSR_IMR, 0xffffffff);
+	intel_uncore_write(uncore, EDP_PSR_IIR, 0xffffffff);
 
 	for_each_pipe(dev_priv, pipe)
 		if (intel_display_power_is_enabled(dev_priv,
@@ -3593,23 +3593,23 @@ static void gen8_irq_reset(struct drm_i915_private *dev_priv)
 
 static void gen11_gt_irq_reset(struct intel_gt *gt)
 {
-	struct drm_i915_private *dev_priv = gt->i915;
+	struct intel_uncore *uncore = gt->uncore;
 
 	/* Disable RCS, BCS, VCS and VECS class engines. */
-	I915_WRITE(GEN11_RENDER_COPY_INTR_ENABLE, 0);
-	I915_WRITE(GEN11_VCS_VECS_INTR_ENABLE,	  0);
+	intel_uncore_write(uncore, GEN11_RENDER_COPY_INTR_ENABLE, 0);
+	intel_uncore_write(uncore, 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);
-
-	I915_WRITE(GEN11_GPM_WGBOXPERF_INTR_ENABLE, 0);
-	I915_WRITE(GEN11_GPM_WGBOXPERF_INTR_MASK,  ~0);
-	I915_WRITE(GEN11_GUC_SG_INTR_ENABLE, 0);
-	I915_WRITE(GEN11_GUC_SG_INTR_MASK,  ~0);
+	intel_uncore_write(uncore, GEN11_RCS0_RSVD_INTR_MASK,	~0);
+	intel_uncore_write(uncore, GEN11_BCS_RSVD_INTR_MASK,	~0);
+	intel_uncore_write(uncore, GEN11_VCS0_VCS1_INTR_MASK,	~0);
+	intel_uncore_write(uncore, GEN11_VCS2_VCS3_INTR_MASK,	~0);
+	intel_uncore_write(uncore, GEN11_VECS0_VECS1_INTR_MASK,	~0);
+
+	intel_uncore_write(uncore, GEN11_GPM_WGBOXPERF_INTR_ENABLE, 0);
+	intel_uncore_write(uncore, GEN11_GPM_WGBOXPERF_INTR_MASK,  ~0);
+	intel_uncore_write(uncore, GEN11_GUC_SG_INTR_ENABLE, 0);
+	intel_uncore_write(uncore, GEN11_GUC_SG_INTR_MASK,  ~0);
 }
 
 static void gen11_irq_reset(struct drm_i915_private *dev_priv)
@@ -3621,10 +3621,10 @@ static void gen11_irq_reset(struct drm_i915_private *dev_priv)
 
 	gen11_gt_irq_reset(&dev_priv->gt);
 
-	I915_WRITE(GEN11_DISPLAY_INT_CTL, 0);
+	intel_uncore_write(uncore, GEN11_DISPLAY_INT_CTL, 0);
 
-	I915_WRITE(EDP_PSR_IMR, 0xffffffff);
-	I915_WRITE(EDP_PSR_IIR, 0xffffffff);
+	intel_uncore_write(uncore, EDP_PSR_IMR, 0xffffffff);
+	intel_uncore_write(uncore, EDP_PSR_IIR, 0xffffffff);
 
 	for_each_pipe(dev_priv, pipe)
 		if (intel_display_power_is_enabled(dev_priv,
@@ -4227,21 +4227,24 @@ static void gen8_irq_postinstall(struct drm_i915_private *dev_priv)
 
 static void gen11_gt_irq_postinstall(struct intel_gt *gt)
 {
-	struct drm_i915_private *dev_priv = gt->i915;
 	const u32 irqs = GT_RENDER_USER_INTERRUPT | GT_CONTEXT_SWITCH_INTERRUPT;
+	struct drm_i915_private *dev_priv = gt->i915;
+	struct intel_uncore *uncore = gt->uncore;
+	const u32 dmask = irqs << 16 | irqs;
+	const u32 smask = irqs << 16;
 
 	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);
+	intel_uncore_write(uncore, GEN11_RENDER_COPY_INTR_ENABLE, dmask);
+	intel_uncore_write(uncore, GEN11_VCS_VECS_INTR_ENABLE, dmask);
 
 	/* 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));
+	intel_uncore_write(uncore, GEN11_RCS0_RSVD_INTR_MASK, ~smask);
+	intel_uncore_write(uncore, GEN11_BCS_RSVD_INTR_MASK, ~smask);
+	intel_uncore_write(uncore, GEN11_VCS0_VCS1_INTR_MASK, ~dmask);
+	intel_uncore_write(uncore, GEN11_VCS2_VCS3_INTR_MASK, ~dmask);
+	intel_uncore_write(uncore, GEN11_VECS0_VECS1_INTR_MASK, ~dmask);
 
 	/*
 	 * RPS interrupts will get enabled/disabled on demand when RPS itself
@@ -4249,12 +4252,12 @@ static void gen11_gt_irq_postinstall(struct intel_gt *gt)
 	 */
 	dev_priv->pm_ier = 0x0;
 	dev_priv->pm_imr = ~dev_priv->pm_ier;
-	I915_WRITE(GEN11_GPM_WGBOXPERF_INTR_ENABLE, 0);
-	I915_WRITE(GEN11_GPM_WGBOXPERF_INTR_MASK,  ~0);
+	intel_uncore_write(uncore, GEN11_GPM_WGBOXPERF_INTR_ENABLE, 0);
+	intel_uncore_write(uncore, GEN11_GPM_WGBOXPERF_INTR_MASK,  ~0);
 
 	/* Same thing for GuC interrupts */
-	I915_WRITE(GEN11_GUC_SG_INTR_ENABLE, 0);
-	I915_WRITE(GEN11_GUC_SG_INTR_MASK,  ~0);
+	intel_uncore_write(uncore, GEN11_GUC_SG_INTR_ENABLE, 0);
+	intel_uncore_write(uncore, GEN11_GUC_SG_INTR_MASK,  ~0);
 }
 
 static void icp_irq_postinstall(struct drm_i915_private *dev_priv)
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915: Remove some legacy mmio accessors from interrupt handling

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-07-02 11:23:12)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Mostly in gen11 interrupt handling and a couple neighbouring functions
> which were easy since uncore local was already available.
> 
> Co-authored-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

Makes sense to me,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
but I'm wary that this is too little of a drop in a churning ocean.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/3] drm/i915: Move dev_priv->pm_i{m, e}r into intel_gt

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

PM interrupts belong to the GT so move the variables to be inside
struct intel_gt.

Co-authored-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Andi Shyti <andi.shyti@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_gt_types.h   |   3 +
 drivers/gpu/drm/i915/gt/intel_ringbuffer.c |   4 +-
 drivers/gpu/drm/i915/i915_drv.h            |   2 -
 drivers/gpu/drm/i915/i915_irq.c            | 121 +++++++++++----------
 drivers/gpu/drm/i915/i915_irq.h            |   4 +-
 5 files changed, 71 insertions(+), 63 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
index c03e56628ee2..37da428bef62 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
@@ -55,6 +55,9 @@ struct intel_gt {
 	ktime_t last_init_time;
 
 	struct i915_vma *scratch;
+
+	u32 pm_imr;
+	u32 pm_ier;
 };
 
 #endif /* __INTEL_GT_TYPES_H__ */
diff --git a/drivers/gpu/drm/i915/gt/intel_ringbuffer.c b/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
index 81f9b0422e6a..d9226ada013f 100644
--- a/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
@@ -1041,14 +1041,14 @@ hsw_vebox_irq_enable(struct intel_engine_cs *engine)
 	/* Flush/delay to ensure the RING_IMR is active before the GT IMR */
 	ENGINE_POSTING_READ(engine, RING_IMR);
 
-	gen6_unmask_pm_irq(engine->i915, engine->irq_enable_mask);
+	gen6_unmask_pm_irq(engine->gt, engine->irq_enable_mask);
 }
 
 static void
 hsw_vebox_irq_disable(struct intel_engine_cs *engine)
 {
 	ENGINE_WRITE(engine, RING_IMR, ~0);
-	gen6_mask_pm_irq(engine->i915, engine->irq_enable_mask);
+	gen6_mask_pm_irq(engine->gt, engine->irq_enable_mask);
 }
 
 static int
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 02dd9f9f3a89..504bbf11029d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1403,8 +1403,6 @@ struct drm_i915_private {
 		u32 de_irq_mask[I915_MAX_PIPES];
 	};
 	u32 gt_irq_mask;
-	u32 pm_imr;
-	u32 pm_ier;
 	u32 pm_rps_events;
 	u32 pm_guc_events;
 	u32 pipestat_irq_mask[I915_MAX_PIPES];
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 817deff20bdd..bdfdd8843627 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -409,50 +409,54 @@ static i915_reg_t gen6_pm_iir(struct drm_i915_private *dev_priv)
 	return INTEL_GEN(dev_priv) >= 8 ? GEN8_GT_IIR(2) : GEN6_PMIIR;
 }
 
-static void write_pm_imr(struct drm_i915_private *dev_priv)
+static void write_pm_imr(struct intel_gt *gt)
 {
+	struct drm_i915_private *i915 = gt->i915;
+	struct intel_uncore *uncore = gt->uncore;
+	u32 mask = gt->pm_imr;
 	i915_reg_t reg;
-	u32 mask = dev_priv->pm_imr;
 
-	if (INTEL_GEN(dev_priv) >= 11) {
+	if (INTEL_GEN(i915) >= 11) {
 		reg = GEN11_GPM_WGBOXPERF_INTR_MASK;
 		/* pm is in upper half */
 		mask = mask << 16;
-	} else if (INTEL_GEN(dev_priv) >= 8) {
+	} else if (INTEL_GEN(i915) >= 8) {
 		reg = GEN8_GT_IMR(2);
 	} else {
 		reg = GEN6_PMIMR;
 	}
 
-	I915_WRITE(reg, mask);
-	POSTING_READ(reg);
+	intel_uncore_write(uncore, reg, mask);
+	intel_uncore_posting_read(uncore, reg);
 }
 
-static void write_pm_ier(struct drm_i915_private *dev_priv)
+static void write_pm_ier(struct intel_gt *gt)
 {
+	struct drm_i915_private *i915 = gt->i915;
+	struct intel_uncore *uncore = gt->uncore;
+	u32 mask = gt->pm_ier;
 	i915_reg_t reg;
-	u32 mask = dev_priv->pm_ier;
 
-	if (INTEL_GEN(dev_priv) >= 11) {
+	if (INTEL_GEN(i915) >= 11) {
 		reg = GEN11_GPM_WGBOXPERF_INTR_ENABLE;
 		/* pm is in upper half */
 		mask = mask << 16;
-	} else if (INTEL_GEN(dev_priv) >= 8) {
+	} else if (INTEL_GEN(i915) >= 8) {
 		reg = GEN8_GT_IER(2);
 	} else {
 		reg = GEN6_PMIER;
 	}
 
-	I915_WRITE(reg, mask);
+	intel_uncore_write(uncore, reg, mask);
 }
 
 /**
  * snb_update_pm_irq - update GEN6_PMIMR
- * @dev_priv: driver private
+ * @gt: gt for the interrupts
  * @interrupt_mask: mask of interrupt bits to update
  * @enabled_irq_mask: mask of interrupt bits to enable
  */
-static void snb_update_pm_irq(struct drm_i915_private *dev_priv,
+static void snb_update_pm_irq(struct intel_gt *gt,
 			      u32 interrupt_mask,
 			      u32 enabled_irq_mask)
 {
@@ -460,37 +464,37 @@ static void snb_update_pm_irq(struct drm_i915_private *dev_priv,
 
 	WARN_ON(enabled_irq_mask & ~interrupt_mask);
 
-	lockdep_assert_held(&dev_priv->irq_lock);
+	lockdep_assert_held(&gt->i915->irq_lock);
 
-	new_val = dev_priv->pm_imr;
+	new_val = gt->pm_imr;
 	new_val &= ~interrupt_mask;
 	new_val |= (~enabled_irq_mask & interrupt_mask);
 
-	if (new_val != dev_priv->pm_imr) {
-		dev_priv->pm_imr = new_val;
-		write_pm_imr(dev_priv);
+	if (new_val != gt->pm_imr) {
+		gt->pm_imr = new_val;
+		write_pm_imr(gt);
 	}
 }
 
-void gen6_unmask_pm_irq(struct drm_i915_private *dev_priv, u32 mask)
+void gen6_unmask_pm_irq(struct intel_gt *gt, u32 mask)
 {
-	if (WARN_ON(!intel_irqs_enabled(dev_priv)))
+	if (WARN_ON(!intel_irqs_enabled(gt->i915)))
 		return;
 
-	snb_update_pm_irq(dev_priv, mask, mask);
+	snb_update_pm_irq(gt, mask, mask);
 }
 
-static void __gen6_mask_pm_irq(struct drm_i915_private *dev_priv, u32 mask)
+static void __gen6_mask_pm_irq(struct intel_gt *gt, u32 mask)
 {
-	snb_update_pm_irq(dev_priv, mask, 0);
+	snb_update_pm_irq(gt, mask, 0);
 }
 
-void gen6_mask_pm_irq(struct drm_i915_private *dev_priv, u32 mask)
+void gen6_mask_pm_irq(struct intel_gt *gt, u32 mask)
 {
-	if (WARN_ON(!intel_irqs_enabled(dev_priv)))
+	if (WARN_ON(!intel_irqs_enabled(gt->i915)))
 		return;
 
-	__gen6_mask_pm_irq(dev_priv, mask);
+	__gen6_mask_pm_irq(gt, mask);
 }
 
 static void gen6_reset_pm_iir(struct drm_i915_private *dev_priv, u32 reset_mask)
@@ -504,23 +508,23 @@ static void gen6_reset_pm_iir(struct drm_i915_private *dev_priv, u32 reset_mask)
 	POSTING_READ(reg);
 }
 
-static void gen6_enable_pm_irq(struct drm_i915_private *dev_priv, u32 enable_mask)
+static void gen6_enable_pm_irq(struct intel_gt *gt, u32 enable_mask)
 {
-	lockdep_assert_held(&dev_priv->irq_lock);
+	lockdep_assert_held(&gt->i915->irq_lock);
 
-	dev_priv->pm_ier |= enable_mask;
-	write_pm_ier(dev_priv);
-	gen6_unmask_pm_irq(dev_priv, enable_mask);
+	gt->pm_ier |= enable_mask;
+	write_pm_ier(gt);
+	gen6_unmask_pm_irq(gt, enable_mask);
 	/* unmask_pm_irq provides an implicit barrier (POSTING_READ) */
 }
 
-static void gen6_disable_pm_irq(struct drm_i915_private *dev_priv, u32 disable_mask)
+static void gen6_disable_pm_irq(struct intel_gt *gt, u32 disable_mask)
 {
-	lockdep_assert_held(&dev_priv->irq_lock);
+	lockdep_assert_held(&gt->i915->irq_lock);
 
-	dev_priv->pm_ier &= ~disable_mask;
-	__gen6_mask_pm_irq(dev_priv, disable_mask);
-	write_pm_ier(dev_priv);
+	gt->pm_ier &= ~disable_mask;
+	__gen6_mask_pm_irq(gt, disable_mask);
+	write_pm_ier(gt);
 	/* though a barrier is missing here, but don't really need a one */
 }
 
@@ -546,6 +550,7 @@ void gen6_reset_rps_interrupts(struct drm_i915_private *dev_priv)
 
 void gen6_enable_rps_interrupts(struct drm_i915_private *dev_priv)
 {
+	struct intel_gt *gt = &dev_priv->gt;
 	struct intel_rps *rps = &dev_priv->gt_pm.rps;
 
 	if (READ_ONCE(rps->interrupts_enabled))
@@ -555,12 +560,12 @@ void gen6_enable_rps_interrupts(struct drm_i915_private *dev_priv)
 	WARN_ON_ONCE(rps->pm_iir);
 
 	if (INTEL_GEN(dev_priv) >= 11)
-		WARN_ON_ONCE(gen11_reset_one_iir(&dev_priv->gt, 0, GEN11_GTPM));
+		WARN_ON_ONCE(gen11_reset_one_iir(gt, 0, GEN11_GTPM));
 	else
 		WARN_ON_ONCE(I915_READ(gen6_pm_iir(dev_priv)) & dev_priv->pm_rps_events);
 
 	rps->interrupts_enabled = true;
-	gen6_enable_pm_irq(dev_priv, dev_priv->pm_rps_events);
+	gen6_enable_pm_irq(gt, dev_priv->pm_rps_events);
 
 	spin_unlock_irq(&dev_priv->irq_lock);
 }
@@ -577,7 +582,7 @@ void gen6_disable_rps_interrupts(struct drm_i915_private *dev_priv)
 
 	I915_WRITE(GEN6_PMINTRMSK, gen6_sanitize_rps_pm_mask(dev_priv, ~0u));
 
-	gen6_disable_pm_irq(dev_priv, GEN6_PM_RPS_EVENTS);
+	gen6_disable_pm_irq(&dev_priv->gt, GEN6_PM_RPS_EVENTS);
 
 	spin_unlock_irq(&dev_priv->irq_lock);
 	synchronize_irq(dev_priv->drm.irq);
@@ -612,7 +617,7 @@ void gen9_enable_guc_interrupts(struct drm_i915_private *dev_priv)
 		WARN_ON_ONCE(I915_READ(gen6_pm_iir(dev_priv)) &
 				       dev_priv->pm_guc_events);
 		dev_priv->guc.interrupts.enabled = true;
-		gen6_enable_pm_irq(dev_priv, dev_priv->pm_guc_events);
+		gen6_enable_pm_irq(&dev_priv->gt, dev_priv->pm_guc_events);
 	}
 	spin_unlock_irq(&dev_priv->irq_lock);
 }
@@ -624,7 +629,7 @@ void gen9_disable_guc_interrupts(struct drm_i915_private *dev_priv)
 	spin_lock_irq(&dev_priv->irq_lock);
 	dev_priv->guc.interrupts.enabled = false;
 
-	gen6_disable_pm_irq(dev_priv, dev_priv->pm_guc_events);
+	gen6_disable_pm_irq(&dev_priv->gt, dev_priv->pm_guc_events);
 
 	spin_unlock_irq(&dev_priv->irq_lock);
 	synchronize_irq(dev_priv->drm.irq);
@@ -1426,7 +1431,7 @@ static void gen6_pm_rps_work(struct work_struct *work)
 	/* Make sure not to corrupt PMIMR state used by ringbuffer on GEN6 */
 	spin_lock_irq(&dev_priv->irq_lock);
 	if (rps->interrupts_enabled)
-		gen6_unmask_pm_irq(dev_priv, dev_priv->pm_rps_events);
+		gen6_unmask_pm_irq(&dev_priv->gt, dev_priv->pm_rps_events);
 	spin_unlock_irq(&dev_priv->irq_lock);
 }
 
@@ -1893,8 +1898,9 @@ static void i9xx_pipe_crc_irq_handler(struct drm_i915_private *dev_priv,
 /* The RPS events need forcewake, so we add them to a work queue and mask their
  * IMR bits until the work is done. Other interrupts can be processed without
  * the work queue. */
-static void gen11_rps_irq_handler(struct drm_i915_private *i915, u32 pm_iir)
+static void gen11_rps_irq_handler(struct intel_gt *gt, u32 pm_iir)
 {
+	struct drm_i915_private *i915 = gt->i915;
 	struct intel_rps *rps = &i915->gt_pm.rps;
 	const u32 events = i915->pm_rps_events & pm_iir;
 
@@ -1903,7 +1909,7 @@ static void gen11_rps_irq_handler(struct drm_i915_private *i915, u32 pm_iir)
 	if (unlikely(!events))
 		return;
 
-	gen6_mask_pm_irq(i915, events);
+	gen6_mask_pm_irq(gt, events);
 
 	if (!rps->interrupts_enabled)
 		return;
@@ -1918,7 +1924,8 @@ static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir)
 
 	if (pm_iir & dev_priv->pm_rps_events) {
 		spin_lock(&dev_priv->irq_lock);
-		gen6_mask_pm_irq(dev_priv, pm_iir & dev_priv->pm_rps_events);
+		gen6_mask_pm_irq(&dev_priv->gt,
+				 pm_iir & dev_priv->pm_rps_events);
 		if (rps->interrupts_enabled) {
 			rps->pm_iir |= pm_iir & dev_priv->pm_rps_events;
 			schedule_work(&rps->work);
@@ -3076,7 +3083,7 @@ gen11_other_irq_handler(struct intel_gt *gt, const u8 instance,
 		return gen11_guc_irq_handler(i915, iir);
 
 	if (instance == OTHER_GTPM_INSTANCE)
-		return gen11_rps_irq_handler(i915, iir);
+		return gen11_rps_irq_handler(gt, iir);
 
 	WARN_ONCE(1, "unhandled other interrupt instance=0x%x, iir=0x%x\n",
 		  instance, iir);
@@ -4006,11 +4013,11 @@ static void gen5_gt_irq_postinstall(struct drm_i915_private *dev_priv)
 		 */
 		if (HAS_ENGINE(dev_priv, VECS0)) {
 			pm_irqs |= PM_VEBOX_USER_INTERRUPT;
-			dev_priv->pm_ier |= PM_VEBOX_USER_INTERRUPT;
+			dev_priv->gt.pm_ier |= PM_VEBOX_USER_INTERRUPT;
 		}
 
-		dev_priv->pm_imr = 0xffffffff;
-		GEN3_IRQ_INIT(uncore, GEN6_PM, dev_priv->pm_imr, pm_irqs);
+		dev_priv->gt.pm_imr = 0xffffffff;
+		GEN3_IRQ_INIT(uncore, GEN6_PM, dev_priv->gt.pm_imr, pm_irqs);
 	}
 }
 
@@ -4107,9 +4114,10 @@ static void valleyview_irq_postinstall(struct drm_i915_private *dev_priv)
 	POSTING_READ(VLV_MASTER_IER);
 }
 
-static void gen8_gt_irq_postinstall(struct drm_i915_private *dev_priv)
+static void gen8_gt_irq_postinstall(struct drm_i915_private *i915)
 {
-	struct intel_uncore *uncore = &dev_priv->uncore;
+	struct intel_gt *gt = &i915->gt;
+	struct intel_uncore *uncore = gt->uncore;
 
 	/* These are interrupts we'll toggle with the ring mask register */
 	u32 gt_interrupts[] = {
@@ -4129,15 +4137,15 @@ static void gen8_gt_irq_postinstall(struct drm_i915_private *dev_priv)
 		 GT_CONTEXT_SWITCH_INTERRUPT << GEN8_VECS_IRQ_SHIFT)
 	};
 
-	dev_priv->pm_ier = 0x0;
-	dev_priv->pm_imr = ~dev_priv->pm_ier;
+	gt->pm_ier = 0x0;
+	gt->pm_imr = ~gt->pm_ier;
 	GEN8_IRQ_INIT_NDX(uncore, GT, 0, ~gt_interrupts[0], gt_interrupts[0]);
 	GEN8_IRQ_INIT_NDX(uncore, GT, 1, ~gt_interrupts[1], gt_interrupts[1]);
 	/*
 	 * RPS interrupts will get enabled/disabled on demand when RPS itself
 	 * is enabled/disabled. Same wil be the case for GuC interrupts.
 	 */
-	GEN8_IRQ_INIT_NDX(uncore, GT, 2, dev_priv->pm_imr, dev_priv->pm_ier);
+	GEN8_IRQ_INIT_NDX(uncore, GT, 2, gt->pm_imr, gt->pm_ier);
 	GEN8_IRQ_INIT_NDX(uncore, GT, 3, ~gt_interrupts[3], gt_interrupts[3]);
 }
 
@@ -4228,7 +4236,6 @@ static void gen8_irq_postinstall(struct drm_i915_private *dev_priv)
 static void gen11_gt_irq_postinstall(struct intel_gt *gt)
 {
 	const u32 irqs = GT_RENDER_USER_INTERRUPT | GT_CONTEXT_SWITCH_INTERRUPT;
-	struct drm_i915_private *dev_priv = gt->i915;
 	struct intel_uncore *uncore = gt->uncore;
 	const u32 dmask = irqs << 16 | irqs;
 	const u32 smask = irqs << 16;
@@ -4250,8 +4257,8 @@ static void gen11_gt_irq_postinstall(struct intel_gt *gt)
 	 * RPS interrupts will get enabled/disabled on demand when RPS itself
 	 * is enabled/disabled.
 	 */
-	dev_priv->pm_ier = 0x0;
-	dev_priv->pm_imr = ~dev_priv->pm_ier;
+	gt->pm_ier = 0x0;
+	gt->pm_imr = ~gt->pm_ier;
 	intel_uncore_write(uncore, GEN11_GPM_WGBOXPERF_INTR_ENABLE, 0);
 	intel_uncore_write(uncore, GEN11_GPM_WGBOXPERF_INTR_MASK,  ~0);
 
diff --git a/drivers/gpu/drm/i915/i915_irq.h b/drivers/gpu/drm/i915/i915_irq.h
index 5af5654f801d..2dd5d5259dbe 100644
--- a/drivers/gpu/drm/i915/i915_irq.h
+++ b/drivers/gpu/drm/i915/i915_irq.h
@@ -77,8 +77,8 @@ ibx_disable_display_interrupt(struct drm_i915_private *dev_priv, u32 bits)
 
 void gen5_enable_gt_irq(struct drm_i915_private *dev_priv, u32 mask);
 void gen5_disable_gt_irq(struct drm_i915_private *dev_priv, u32 mask);
-void gen6_mask_pm_irq(struct drm_i915_private *dev_priv, u32 mask);
-void gen6_unmask_pm_irq(struct drm_i915_private *dev_priv, u32 mask);
+void gen6_mask_pm_irq(struct intel_gt *gt, u32 mask);
+void gen6_unmask_pm_irq(struct intel_gt *gt, u32 mask);
 void gen11_reset_rps_interrupts(struct drm_i915_private *dev_priv);
 void gen6_reset_rps_interrupts(struct drm_i915_private *dev_priv);
 void gen6_enable_rps_interrupts(struct drm_i915_private *dev_priv);
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: Move dev_priv->pm_i{m, e}r into intel_gt

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-07-02 11:23:13)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> PM interrupts belong to the GT so move the variables to be inside
> struct intel_gt.

I even had gt_pm patches kicking around :-p
Which are in desperate need of revival. :-(

> Co-authored-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Andi Shyti <andi.shyti@intel.com>

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/gt/intel_gt_types.h   |   3 +
>  drivers/gpu/drm/i915/gt/intel_ringbuffer.c |   4 +-
>  drivers/gpu/drm/i915/i915_drv.h            |   2 -
>  drivers/gpu/drm/i915/i915_irq.c            | 121 +++++++++++----------
>  drivers/gpu/drm/i915/i915_irq.h            |   4 +-
>  5 files changed, 71 insertions(+), 63 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
> index c03e56628ee2..37da428bef62 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
> @@ -55,6 +55,9 @@ struct intel_gt {
>         ktime_t last_init_time;
>  
>         struct i915_vma *scratch;
> +
> +       u32 pm_imr;
> +       u32 pm_ier;

Fwiw, I do intend to bring back gt_pm and move all the rps/ips tracking
underneath it and extract it from i915_irq.c & intel_pm.c into gt/

We have a long standing bugs in race conditions between the pm
interrupts and the rps worker, plus overdue work to let userspace make
fine grained suggestions for frequency/power control.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.CHECKPATCH: warning for More mmio and intel_gt cleanups and refactorings

[Patchwork]

== Series Details ==

Series: More mmio and intel_gt cleanups and refactorings
URL   : https://patchwork.freedesktop.org/series/63063/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
a59370ad3ca8 drm/i915: Rework some interrupt handling functions to take intel_gt
-:12: WARNING:BAD_SIGN_OFF: Non-standard signature: Co-authored-by:
#12: 
Co-authored-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

total: 0 errors, 1 warnings, 0 checks, 269 lines checked
7ca251537529 drm/i915: Remove some legacy mmio accessors from interrupt handling
-:10: WARNING:BAD_SIGN_OFF: Non-standard signature: Co-authored-by:
#10: 
Co-authored-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

total: 0 errors, 1 warnings, 0 checks, 136 lines checked
359a181398e4 drm/i915: Move dev_priv->pm_i{m, e}r into intel_gt
-:9: WARNING:BAD_SIGN_OFF: Non-standard signature: Co-authored-by:
#9: 
Co-authored-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

total: 0 errors, 1 warnings, 0 checks, 342 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✗ Fi.CI.CHECKPATCH: warning for More mmio and intel_gt cleanups and refactorings

[Chris Wilson]

Quoting Patchwork (2019-07-02 11:46:48)
> == Series Details ==
> 
> Series: More mmio and intel_gt cleanups and refactorings
> URL   : https://patchwork.freedesktop.org/series/63063/
> State : warning
> 
> == Summary ==
> 
> $ dim checkpatch origin/drm-tip
> a59370ad3ca8 drm/i915: Rework some interrupt handling functions to take intel_gt
> -:12: WARNING:BAD_SIGN_OFF: Non-standard signature: Co-authored-by:
> #12: 
> Co-authored-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Approved tag is "Co-developed-by"
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for More mmio and intel_gt cleanups and refactorings

[Patchwork]

== Series Details ==

Series: More mmio and intel_gt cleanups and refactorings
URL   : https://patchwork.freedesktop.org/series/63063/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_6394 -> Patchwork_13488
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_13488 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_13488, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13488/

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_13488:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live_contexts:
    - fi-skl-gvtdvm:      NOTRUN -> [INCOMPLETE][1]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13488/fi-skl-gvtdvm/igt@i915_selftest@live_contexts.html

  
Known issues
------------

  Here are the changes found in Patchwork_13488 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live_blt:
    - fi-skl-iommu:       [PASS][2] -> [INCOMPLETE][3] ([fdo#108602])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6394/fi-skl-iommu/igt@i915_selftest@live_blt.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13488/fi-skl-iommu/igt@i915_selftest@live_blt.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-hsw-peppy:       [PASS][4] -> [DMESG-WARN][5] ([fdo#102614])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6394/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13488/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html

  * igt@prime_vgem@basic-fence-flip:
    - fi-icl-u3:          [PASS][6] -> [DMESG-WARN][7] ([fdo#107724]) +2 similar issues
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6394/fi-icl-u3/igt@prime_vgem@basic-fence-flip.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13488/fi-icl-u3/igt@prime_vgem@basic-fence-flip.html

  
#### Possible fixes ####

  * igt@gem_mmap_gtt@basic-small-bo-tiledy:
    - fi-icl-u3:          [DMESG-WARN][8] ([fdo#107724]) -> [PASS][9] +1 similar issue
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6394/fi-icl-u3/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13488/fi-icl-u3/igt@gem_mmap_gtt@basic-small-bo-tiledy.html

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - fi-skl-6600u:       [DMESG-WARN][10] ([fdo#111012]) -> [PASS][11]
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6394/fi-skl-6600u/igt@i915_pm_rpm@basic-pci-d3-state.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13488/fi-skl-6600u/igt@i915_pm_rpm@basic-pci-d3-state.html

  * igt@i915_selftest@live_blt:
    - fi-cfl-guc:         [DMESG-WARN][12] ([fdo#110943]) -> [PASS][13]
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6394/fi-cfl-guc/igt@i915_selftest@live_blt.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13488/fi-cfl-guc/igt@i915_selftest@live_blt.html

  * igt@i915_selftest@live_hugepages:
    - fi-skl-gvtdvm:      [DMESG-WARN][14] ([fdo#110976]) -> [PASS][15]
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6394/fi-skl-gvtdvm/igt@i915_selftest@live_hugepages.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13488/fi-skl-gvtdvm/igt@i915_selftest@live_hugepages.html

  * igt@prime_vgem@basic-fence-flip:
    - fi-ilk-650:         [DMESG-WARN][16] ([fdo#106387]) -> [PASS][17] +1 similar issue
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6394/fi-ilk-650/igt@prime_vgem@basic-fence-flip.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13488/fi-ilk-650/igt@prime_vgem@basic-fence-flip.html

  
#### Warnings ####

  * igt@i915_module_load@reload:
    - fi-icl-u2:          [DMESG-WARN][18] ([fdo#111041]) -> [DMESG-WARN][19] ([fdo#110595]) +2 similar issues
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6394/fi-icl-u2/igt@i915_module_load@reload.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13488/fi-icl-u2/igt@i915_module_load@reload.html

  
  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#106387]: https://bugs.freedesktop.org/show_bug.cgi?id=106387
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#108602]: https://bugs.freedesktop.org/show_bug.cgi?id=108602
  [fdo#110595]: https://bugs.freedesktop.org/show_bug.cgi?id=110595
  [fdo#110943]: https://bugs.freedesktop.org/show_bug.cgi?id=110943
  [fdo#110976]: https://bugs.freedesktop.org/show_bug.cgi?id=110976
  [fdo#111012]: https://bugs.freedesktop.org/show_bug.cgi?id=111012
  [fdo#111041]: https://bugs.freedesktop.org/show_bug.cgi?id=111041


Participating hosts (50 -> 44)
------------------------------

  Additional (2): fi-icl-guc fi-cml-u 
  Missing    (8): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-bdw-gvtdvm fi-byt-squawks fi-bsw-cyan fi-icl-y fi-byt-clapper 


Build changes
-------------

  * Linux: CI_DRM_6394 -> Patchwork_13488

  CI_DRM_6394: ad42b755acd3c10f7a8e23309189f0a850ec92c5 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5075: 03779dd3de8a57544f124d9952a6d2b3e34e34ca @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13488: 359a181398e4276615186394fe956b357f80cada @ git://anongit.freedesktop.org/gfx-ci/linux


== Kernel 32bit build ==

Warning: Kernel 32bit buildtest failed:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13488/build_32bit.log

  CALL    scripts/checksyscalls.sh
  CALL    scripts/atomic/check-atomics.sh
  CHK     include/generated/compile.h
Kernel: arch/x86/boot/bzImage is ready  (#1)
  Building modules, stage 2.
  MODPOST 112 modules
ERROR: "__udivdi3" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
ERROR: "__divdi3" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
scripts/Makefile.modpost:91: recipe for target '__modpost' failed
make[1]: *** [__modpost] Error 1
Makefile:1287: recipe for target 'modules' failed
make: *** [modules] Error 2


== Linux commits ==

359a181398e4 drm/i915: Move dev_priv->pm_i{m, e}r into intel_gt
7ca251537529 drm/i915: Remove some legacy mmio accessors from interrupt handling
a59370ad3ca8 drm/i915: Rework some interrupt handling functions to take intel_gt

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13488/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for More mmio and intel_gt cleanups and refactorings (rev2)

[Patchwork]

== Series Details ==

Series: More mmio and intel_gt cleanups and refactorings (rev2)
URL   : https://patchwork.freedesktop.org/series/63063/
State : failure

== Summary ==

Patch format detection failed.

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx