Re: [Intel-gfx] [PATCH 1/2] drm/i915: Track GT interrupt handling using the master iir
Chris Wilson writes: > Keep the master iir and use it to reduce the number of reads and writes > to the GT iir array, i.e. only the bits marked as set by the master iir > are valid inside GT iir array and will be handled during the interrupt. > > Signed-off-by: Chris Wilson > Cc: Ville Syrjala > Cc: Mika Kuoppala It does what it promises and the writes are reduced. The ack/handle symmetry needs more thought now on reading but if that becomes a problem we can split gt iir's for per engine functions. Reviewed-by: Mika Kuoppala > --- > drivers/gpu/drm/i915/i915_irq.c | 51 > + > 1 file changed, 31 insertions(+), 20 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index b886bd459acc..b7b377ba7b6e 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -1416,6 +1416,14 @@ gen8_cs_irq_handler(struct intel_engine_cs *engine, > u32 iir, int test_shift) > static void gen8_gt_irq_ack(struct drm_i915_private *dev_priv, > u32 master_ctl, u32 gt_iir[4]) > { > +#define GEN8_GT_IRQS (GEN8_GT_RCS_IRQ | \ > + GEN8_GT_BCS_IRQ | \ > + GEN8_GT_VCS1_IRQ | \ > + GEN8_GT_VCS2_IRQ | \ > + GEN8_GT_VECS_IRQ | \ > + GEN8_GT_PM_IRQ | \ > + GEN8_GT_GUC_IRQ) > + > if (master_ctl & (GEN8_GT_RCS_IRQ | GEN8_GT_BCS_IRQ)) { > gt_iir[0] = I915_READ_FW(GEN8_GT_IIR(0)); > if (gt_iir[0]) > @@ -1446,31 +1454,34 @@ static void gen8_gt_irq_ack(struct drm_i915_private > *dev_priv, > } > > static void gen8_gt_irq_handler(struct drm_i915_private *dev_priv, > - u32 gt_iir[4]) > + u32 master_ctl, u32 gt_iir[4]) > { > - if (gt_iir[0]) { > + if (master_ctl & (GEN8_GT_RCS_IRQ | GEN8_GT_BCS_IRQ)) { > gen8_cs_irq_handler(dev_priv->engine[RCS], > gt_iir[0], GEN8_RCS_IRQ_SHIFT); > gen8_cs_irq_handler(dev_priv->engine[BCS], > gt_iir[0], GEN8_BCS_IRQ_SHIFT); > } > > - if (gt_iir[1]) { > + if (master_ctl & (GEN8_GT_VCS1_IRQ | GEN8_GT_VCS2_IRQ)) { > gen8_cs_irq_handler(dev_priv->engine[VCS], > gt_iir[1], GEN8_VCS1_IRQ_SHIFT); > gen8_cs_irq_handler(dev_priv->engine[VCS2], > gt_iir[1], GEN8_VCS2_IRQ_SHIFT); > } > > - if (gt_iir[3]) > + if (master_ctl & GEN8_GT_VECS_IRQ) { > gen8_cs_irq_handler(dev_priv->engine[VECS], > gt_iir[3], GEN8_VECS_IRQ_SHIFT); > + } > > - if (gt_iir[2] & dev_priv->pm_rps_events) > - gen6_rps_irq_handler(dev_priv, gt_iir[2]); > + if (master_ctl & (GEN8_GT_PM_IRQ | GEN8_GT_GUC_IRQ)) { > + if (gt_iir[2] & dev_priv->pm_rps_events) > + gen6_rps_irq_handler(dev_priv, gt_iir[2]); > > - if (gt_iir[2] & dev_priv->pm_guc_events) > - gen9_guc_irq_handler(dev_priv, gt_iir[2]); > + if (gt_iir[2] & dev_priv->pm_guc_events) > + gen9_guc_irq_handler(dev_priv, gt_iir[2]); > + } > } > > static bool bxt_port_hotplug_long_detect(enum port port, u32 val) > @@ -2085,9 +2096,9 @@ static irqreturn_t cherryview_irq_handler(int irq, void > *arg) > > do { > u32 master_ctl, iir; > - u32 gt_iir[4] = {}; > u32 pipe_stats[I915_MAX_PIPES] = {}; > u32 hotplug_status = 0; > + u32 gt_iir[4]; > u32 ier = 0; > > master_ctl = I915_READ(GEN8_MASTER_IRQ) & > ~GEN8_MASTER_IRQ_CONTROL; > @@ -2140,7 +2151,7 @@ static irqreturn_t cherryview_irq_handler(int irq, void > *arg) > I915_WRITE(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL); > POSTING_READ(GEN8_MASTER_IRQ); > > - gen8_gt_irq_handler(dev_priv, gt_iir); > + gen8_gt_irq_handler(dev_priv, master_ctl, gt_iir); > > if (hotplug_status) > i9xx_hpd_irq_handler(dev_priv, hotplug_status); > @@ -2675,10 +2686,9 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, > u32 master_ctl) > > static irqreturn_t gen8_irq_handler(int irq, void *arg) > { > - struct drm_device *dev = arg; > - struct drm_i915_private *dev_priv = to_i915(dev); > + struct drm_i915_private *dev_priv = to_i915(arg); > u32 master_ctl; > - u32 gt_iir[4] = {}; > + u32 gt_iir[4]; > > if (!intel_irqs_enabled(dev_priv)) > return IRQ_NONE; > @@ -2690,18 +2700,19 @@ static irqreturn_t gen8_irq_handler(int irq, void > *arg) > > I915_WRITE_FW(GEN8_MASTER_IRQ, 0); > > - /* IRQs are synced during runtime_suspend, we don't require a wakeref */ > - disable_rpm_wakeref
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Track GT interrupt handling using the master iir
On Thu, Feb 15, 2018 at 04:00:04PM +, Chris Wilson wrote: > Quoting Mika Kuoppala (2018-02-15 15:21:33) > > Chris Wilson writes: > > > > > Keep the master iir and use it to reduce the number of reads and writes > > > to the GT iir array, i.e. only the bits marked as set by the master iir > > > are valid inside GT iir array and will be handled during the interrupt. > > > > > > Signed-off-by: Chris Wilson > > > Cc: Ville Syrjala > > > Cc: Mika Kuoppala > > > --- > > > drivers/gpu/drm/i915/i915_irq.c | 51 > > > + > > > 1 file changed, 31 insertions(+), 20 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_irq.c > > > b/drivers/gpu/drm/i915/i915_irq.c > > > index b886bd459acc..b7b377ba7b6e 100644 > > > --- a/drivers/gpu/drm/i915/i915_irq.c > > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > > @@ -1416,6 +1416,14 @@ gen8_cs_irq_handler(struct intel_engine_cs > > > *engine, u32 iir, int test_shift) > > > static void gen8_gt_irq_ack(struct drm_i915_private *dev_priv, > > > u32 master_ctl, u32 gt_iir[4]) > > > { > > > +#define GEN8_GT_IRQS (GEN8_GT_RCS_IRQ | \ > > > + GEN8_GT_BCS_IRQ | \ > > > + GEN8_GT_VCS1_IRQ | \ > > > + GEN8_GT_VCS2_IRQ | \ > > > + GEN8_GT_VECS_IRQ | \ > > > + GEN8_GT_PM_IRQ | \ > > > + GEN8_GT_GUC_IRQ) > > > + > > > if (master_ctl & (GEN8_GT_RCS_IRQ | GEN8_GT_BCS_IRQ)) { > > > gt_iir[0] = I915_READ_FW(GEN8_GT_IIR(0)); > > > if (gt_iir[0]) > > > @@ -1446,31 +1454,34 @@ static void gen8_gt_irq_ack(struct > > > drm_i915_private *dev_priv, > > > } > > > > > > static void gen8_gt_irq_handler(struct drm_i915_private *dev_priv, > > > - u32 gt_iir[4]) > > > + u32 master_ctl, u32 gt_iir[4]) > > > { > > > - if (gt_iir[0]) { > > > + if (master_ctl & (GEN8_GT_RCS_IRQ | GEN8_GT_BCS_IRQ)) { > > > gen8_cs_irq_handler(dev_priv->engine[RCS], > > > gt_iir[0], GEN8_RCS_IRQ_SHIFT); > > > gen8_cs_irq_handler(dev_priv->engine[BCS], > > > gt_iir[0], GEN8_BCS_IRQ_SHIFT); > > > } > > > > > > - if (gt_iir[1]) { > > > + if (master_ctl & (GEN8_GT_VCS1_IRQ | GEN8_GT_VCS2_IRQ)) { > > > gen8_cs_irq_handler(dev_priv->engine[VCS], > > > gt_iir[1], GEN8_VCS1_IRQ_SHIFT); > > > gen8_cs_irq_handler(dev_priv->engine[VCS2], > > > gt_iir[1], GEN8_VCS2_IRQ_SHIFT); > > > } > > > > > > - if (gt_iir[3]) > > > + if (master_ctl & GEN8_GT_VECS_IRQ) { > > > gen8_cs_irq_handler(dev_priv->engine[VECS], > > > gt_iir[3], GEN8_VECS_IRQ_SHIFT); > > > + } > > > > > > - if (gt_iir[2] & dev_priv->pm_rps_events) > > > - gen6_rps_irq_handler(dev_priv, gt_iir[2]); > > > + if (master_ctl & (GEN8_GT_PM_IRQ | GEN8_GT_GUC_IRQ)) { > > > + if (gt_iir[2] & dev_priv->pm_rps_events) > > > + gen6_rps_irq_handler(dev_priv, gt_iir[2]); > > > > > > - if (gt_iir[2] & dev_priv->pm_guc_events) > > > - gen9_guc_irq_handler(dev_priv, gt_iir[2]); > > > + if (gt_iir[2] & dev_priv->pm_guc_events) > > > + gen9_guc_irq_handler(dev_priv, gt_iir[2]); > > > + } > > > } > > > > > > static bool bxt_port_hotplug_long_detect(enum port port, u32 val) > > > @@ -2085,9 +2096,9 @@ static irqreturn_t cherryview_irq_handler(int irq, > > > void *arg) > > > > > > do { > > > u32 master_ctl, iir; > > > - u32 gt_iir[4] = {}; > > > u32 pipe_stats[I915_MAX_PIPES] = {}; > > > u32 hotplug_status = 0; > > > + u32 gt_iir[4]; > > > u32 ier = 0; > > > > > > master_ctl = I915_READ(GEN8_MASTER_IRQ) & > > > ~GEN8_MASTER_IRQ_CONTROL; > > > @@ -2140,7 +2151,7 @@ static irqreturn_t cherryview_irq_handler(int irq, > > > void *arg) > > > I915_WRITE(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL); > > > POSTING_READ(GEN8_MASTER_IRQ); > > > > > > - gen8_gt_irq_handler(dev_priv, gt_iir); > > > + gen8_gt_irq_handler(dev_priv, master_ctl, gt_iir); > > > > > > if (hotplug_status) > > > i9xx_hpd_irq_handler(dev_priv, hotplug_status); > > > @@ -2675,10 +2686,9 @@ gen8_de_irq_handler(struct drm_i915_private > > > *dev_priv, u32 master_ctl) > > > > > > static irqreturn_t gen8_irq_handler(int irq, void *arg) > > > { > > > - struct drm_device *dev = arg; > > > - struct drm_i915_private *dev_priv = to_i915(dev); > > > + struct drm_i915_private *dev_priv = to_i915(arg); > > > u32 master_ctl; > > > - u32 gt_iir[4] = {}; > > > +
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Track GT interrupt handling using the master iir
Quoting Mika Kuoppala (2018-02-15 15:21:33) > Chris Wilson writes: > > > Keep the master iir and use it to reduce the number of reads and writes > > to the GT iir array, i.e. only the bits marked as set by the master iir > > are valid inside GT iir array and will be handled during the interrupt. > > > > Signed-off-by: Chris Wilson > > Cc: Ville Syrjala > > Cc: Mika Kuoppala > > --- > > drivers/gpu/drm/i915/i915_irq.c | 51 > > + > > 1 file changed, 31 insertions(+), 20 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_irq.c > > b/drivers/gpu/drm/i915/i915_irq.c > > index b886bd459acc..b7b377ba7b6e 100644 > > --- a/drivers/gpu/drm/i915/i915_irq.c > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > @@ -1416,6 +1416,14 @@ gen8_cs_irq_handler(struct intel_engine_cs *engine, > > u32 iir, int test_shift) > > static void gen8_gt_irq_ack(struct drm_i915_private *dev_priv, > > u32 master_ctl, u32 gt_iir[4]) > > { > > +#define GEN8_GT_IRQS (GEN8_GT_RCS_IRQ | \ > > + GEN8_GT_BCS_IRQ | \ > > + GEN8_GT_VCS1_IRQ | \ > > + GEN8_GT_VCS2_IRQ | \ > > + GEN8_GT_VECS_IRQ | \ > > + GEN8_GT_PM_IRQ | \ > > + GEN8_GT_GUC_IRQ) > > + > > if (master_ctl & (GEN8_GT_RCS_IRQ | GEN8_GT_BCS_IRQ)) { > > gt_iir[0] = I915_READ_FW(GEN8_GT_IIR(0)); > > if (gt_iir[0]) > > @@ -1446,31 +1454,34 @@ static void gen8_gt_irq_ack(struct drm_i915_private > > *dev_priv, > > } > > > > static void gen8_gt_irq_handler(struct drm_i915_private *dev_priv, > > - u32 gt_iir[4]) > > + u32 master_ctl, u32 gt_iir[4]) > > { > > - if (gt_iir[0]) { > > + if (master_ctl & (GEN8_GT_RCS_IRQ | GEN8_GT_BCS_IRQ)) { > > gen8_cs_irq_handler(dev_priv->engine[RCS], > > gt_iir[0], GEN8_RCS_IRQ_SHIFT); > > gen8_cs_irq_handler(dev_priv->engine[BCS], > > gt_iir[0], GEN8_BCS_IRQ_SHIFT); > > } > > > > - if (gt_iir[1]) { > > + if (master_ctl & (GEN8_GT_VCS1_IRQ | GEN8_GT_VCS2_IRQ)) { > > gen8_cs_irq_handler(dev_priv->engine[VCS], > > gt_iir[1], GEN8_VCS1_IRQ_SHIFT); > > gen8_cs_irq_handler(dev_priv->engine[VCS2], > > gt_iir[1], GEN8_VCS2_IRQ_SHIFT); > > } > > > > - if (gt_iir[3]) > > + if (master_ctl & GEN8_GT_VECS_IRQ) { > > gen8_cs_irq_handler(dev_priv->engine[VECS], > > gt_iir[3], GEN8_VECS_IRQ_SHIFT); > > + } > > > > - if (gt_iir[2] & dev_priv->pm_rps_events) > > - gen6_rps_irq_handler(dev_priv, gt_iir[2]); > > + if (master_ctl & (GEN8_GT_PM_IRQ | GEN8_GT_GUC_IRQ)) { > > + if (gt_iir[2] & dev_priv->pm_rps_events) > > + gen6_rps_irq_handler(dev_priv, gt_iir[2]); > > > > - if (gt_iir[2] & dev_priv->pm_guc_events) > > - gen9_guc_irq_handler(dev_priv, gt_iir[2]); > > + if (gt_iir[2] & dev_priv->pm_guc_events) > > + gen9_guc_irq_handler(dev_priv, gt_iir[2]); > > + } > > } > > > > static bool bxt_port_hotplug_long_detect(enum port port, u32 val) > > @@ -2085,9 +2096,9 @@ static irqreturn_t cherryview_irq_handler(int irq, > > void *arg) > > > > do { > > u32 master_ctl, iir; > > - u32 gt_iir[4] = {}; > > u32 pipe_stats[I915_MAX_PIPES] = {}; > > u32 hotplug_status = 0; > > + u32 gt_iir[4]; > > u32 ier = 0; > > > > master_ctl = I915_READ(GEN8_MASTER_IRQ) & > > ~GEN8_MASTER_IRQ_CONTROL; > > @@ -2140,7 +2151,7 @@ static irqreturn_t cherryview_irq_handler(int irq, > > void *arg) > > I915_WRITE(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL); > > POSTING_READ(GEN8_MASTER_IRQ); > > > > - gen8_gt_irq_handler(dev_priv, gt_iir); > > + gen8_gt_irq_handler(dev_priv, master_ctl, gt_iir); > > > > if (hotplug_status) > > i9xx_hpd_irq_handler(dev_priv, hotplug_status); > > @@ -2675,10 +2686,9 @@ gen8_de_irq_handler(struct drm_i915_private > > *dev_priv, u32 master_ctl) > > > > static irqreturn_t gen8_irq_handler(int irq, void *arg) > > { > > - struct drm_device *dev = arg; > > - struct drm_i915_private *dev_priv = to_i915(dev); > > + struct drm_i915_private *dev_priv = to_i915(arg); > > u32 master_ctl; > > - u32 gt_iir[4] = {}; > > + u32 gt_iir[4]; > > > > if (!intel_irqs_enabled(dev_priv)) > > return IRQ_NONE; > > @@ -2690,18 +2700,19 @@ static irqreturn_t gen8_irq_handler(int irq, void > > *arg) > > > > I915_WRITE_FW(GEN8_MASTER_IRQ, 0); > > > > - /* IRQs are synced during run
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Track GT interrupt handling using the master iir
Chris Wilson writes: > Keep the master iir and use it to reduce the number of reads and writes > to the GT iir array, i.e. only the bits marked as set by the master iir > are valid inside GT iir array and will be handled during the interrupt. > > Signed-off-by: Chris Wilson > Cc: Ville Syrjala > Cc: Mika Kuoppala > --- > drivers/gpu/drm/i915/i915_irq.c | 51 > + > 1 file changed, 31 insertions(+), 20 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index b886bd459acc..b7b377ba7b6e 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -1416,6 +1416,14 @@ gen8_cs_irq_handler(struct intel_engine_cs *engine, > u32 iir, int test_shift) > static void gen8_gt_irq_ack(struct drm_i915_private *dev_priv, > u32 master_ctl, u32 gt_iir[4]) > { > +#define GEN8_GT_IRQS (GEN8_GT_RCS_IRQ | \ > + GEN8_GT_BCS_IRQ | \ > + GEN8_GT_VCS1_IRQ | \ > + GEN8_GT_VCS2_IRQ | \ > + GEN8_GT_VECS_IRQ | \ > + GEN8_GT_PM_IRQ | \ > + GEN8_GT_GUC_IRQ) > + > if (master_ctl & (GEN8_GT_RCS_IRQ | GEN8_GT_BCS_IRQ)) { > gt_iir[0] = I915_READ_FW(GEN8_GT_IIR(0)); > if (gt_iir[0]) > @@ -1446,31 +1454,34 @@ static void gen8_gt_irq_ack(struct drm_i915_private > *dev_priv, > } > > static void gen8_gt_irq_handler(struct drm_i915_private *dev_priv, > - u32 gt_iir[4]) > + u32 master_ctl, u32 gt_iir[4]) > { > - if (gt_iir[0]) { > + if (master_ctl & (GEN8_GT_RCS_IRQ | GEN8_GT_BCS_IRQ)) { > gen8_cs_irq_handler(dev_priv->engine[RCS], > gt_iir[0], GEN8_RCS_IRQ_SHIFT); > gen8_cs_irq_handler(dev_priv->engine[BCS], > gt_iir[0], GEN8_BCS_IRQ_SHIFT); > } > > - if (gt_iir[1]) { > + if (master_ctl & (GEN8_GT_VCS1_IRQ | GEN8_GT_VCS2_IRQ)) { > gen8_cs_irq_handler(dev_priv->engine[VCS], > gt_iir[1], GEN8_VCS1_IRQ_SHIFT); > gen8_cs_irq_handler(dev_priv->engine[VCS2], > gt_iir[1], GEN8_VCS2_IRQ_SHIFT); > } > > - if (gt_iir[3]) > + if (master_ctl & GEN8_GT_VECS_IRQ) { > gen8_cs_irq_handler(dev_priv->engine[VECS], > gt_iir[3], GEN8_VECS_IRQ_SHIFT); > + } > > - if (gt_iir[2] & dev_priv->pm_rps_events) > - gen6_rps_irq_handler(dev_priv, gt_iir[2]); > + if (master_ctl & (GEN8_GT_PM_IRQ | GEN8_GT_GUC_IRQ)) { > + if (gt_iir[2] & dev_priv->pm_rps_events) > + gen6_rps_irq_handler(dev_priv, gt_iir[2]); > > - if (gt_iir[2] & dev_priv->pm_guc_events) > - gen9_guc_irq_handler(dev_priv, gt_iir[2]); > + if (gt_iir[2] & dev_priv->pm_guc_events) > + gen9_guc_irq_handler(dev_priv, gt_iir[2]); > + } > } > > static bool bxt_port_hotplug_long_detect(enum port port, u32 val) > @@ -2085,9 +2096,9 @@ static irqreturn_t cherryview_irq_handler(int irq, void > *arg) > > do { > u32 master_ctl, iir; > - u32 gt_iir[4] = {}; > u32 pipe_stats[I915_MAX_PIPES] = {}; > u32 hotplug_status = 0; > + u32 gt_iir[4]; > u32 ier = 0; > > master_ctl = I915_READ(GEN8_MASTER_IRQ) & > ~GEN8_MASTER_IRQ_CONTROL; > @@ -2140,7 +2151,7 @@ static irqreturn_t cherryview_irq_handler(int irq, void > *arg) > I915_WRITE(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL); > POSTING_READ(GEN8_MASTER_IRQ); > > - gen8_gt_irq_handler(dev_priv, gt_iir); > + gen8_gt_irq_handler(dev_priv, master_ctl, gt_iir); > > if (hotplug_status) > i9xx_hpd_irq_handler(dev_priv, hotplug_status); > @@ -2675,10 +2686,9 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, > u32 master_ctl) > > static irqreturn_t gen8_irq_handler(int irq, void *arg) > { > - struct drm_device *dev = arg; > - struct drm_i915_private *dev_priv = to_i915(dev); > + struct drm_i915_private *dev_priv = to_i915(arg); > u32 master_ctl; > - u32 gt_iir[4] = {}; > + u32 gt_iir[4]; > > if (!intel_irqs_enabled(dev_priv)) > return IRQ_NONE; > @@ -2690,18 +2700,19 @@ static irqreturn_t gen8_irq_handler(int irq, void > *arg) > > I915_WRITE_FW(GEN8_MASTER_IRQ, 0); > > - /* IRQs are synced during runtime_suspend, we don't require a wakeref */ > - disable_rpm_wakeref_asserts(dev_priv); > - > /* Find, clear, then process each source of interrupt */ > gen8_gt_irq_ack(dev_priv, master_ctl, gt_iir); > - gen8_gt_irq_handler(dev_priv, gt_iir); > - gen8_de_irq_handler(d