* [PATCH 1/2] drm/i915/gen8: Tidy display interrupt processing
@ 2016-01-12 16:04 Tvrtko Ursulin
2016-01-12 16:04 ` [PATCH 2/2] drm/i915/gen8: Factor out display interrupt handling Tvrtko Ursulin
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Tvrtko Ursulin @ 2016-01-12 16:04 UTC (permalink / raw)
To: Intel-gfx
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
One bugfix and a few tidy-ups:
* Pipe fault logging was broken on Gen9+.
* Removed some unnecessary local variables.
* Removed unnecessary initializers.
* Decreased pipe iir block indentation level.
* Grouped variable initialization close to use sites.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_irq.c | 131 ++++++++++++++++++++--------------------
1 file changed, 67 insertions(+), 64 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index f04d799153ca..7972ceee6096 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2268,11 +2268,9 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg)
{
struct drm_device *dev = arg;
struct drm_i915_private *dev_priv = dev->dev_private;
- u32 master_ctl;
+ u32 master_ctl, iir;
irqreturn_t ret = IRQ_NONE;
- uint32_t tmp = 0;
enum pipe pipe;
- u32 aux_mask = GEN8_AUX_CHANNEL_A;
if (!intel_irqs_enabled(dev_priv))
return IRQ_NONE;
@@ -2280,10 +2278,6 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg)
/* IRQs are synced during runtime_suspend, we don't require a wakeref */
disable_rpm_wakeref_asserts(dev_priv);
- if (INTEL_INFO(dev_priv)->gen >= 9)
- aux_mask |= GEN9_AUX_CHANNEL_B | GEN9_AUX_CHANNEL_C |
- GEN9_AUX_CHANNEL_D;
-
master_ctl = I915_READ_FW(GEN8_MASTER_IRQ);
master_ctl &= ~GEN8_MASTER_IRQ_CONTROL;
if (!master_ctl)
@@ -2296,11 +2290,11 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg)
ret = gen8_gt_irq_handler(dev_priv, master_ctl);
if (master_ctl & GEN8_DE_MISC_IRQ) {
- tmp = I915_READ(GEN8_DE_MISC_IIR);
- if (tmp) {
- I915_WRITE(GEN8_DE_MISC_IIR, tmp);
+ iir = I915_READ(GEN8_DE_MISC_IIR);
+ if (iir) {
+ I915_WRITE(GEN8_DE_MISC_IIR, iir);
ret = IRQ_HANDLED;
- if (tmp & GEN8_DE_MISC_GSE)
+ if (iir & GEN8_DE_MISC_GSE)
intel_opregion_asle_intr(dev);
else
DRM_ERROR("Unexpected DE Misc interrupt\n");
@@ -2310,33 +2304,40 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg)
}
if (master_ctl & GEN8_DE_PORT_IRQ) {
- tmp = I915_READ(GEN8_DE_PORT_IIR);
- if (tmp) {
+ iir = I915_READ(GEN8_DE_PORT_IIR);
+ if (iir) {
+ u32 tmp_mask;
bool found = false;
- u32 hotplug_trigger = 0;
-
- if (IS_BROXTON(dev_priv))
- hotplug_trigger = tmp & BXT_DE_PORT_HOTPLUG_MASK;
- else if (IS_BROADWELL(dev_priv))
- hotplug_trigger = tmp & GEN8_PORT_DP_A_HOTPLUG;
- I915_WRITE(GEN8_DE_PORT_IIR, tmp);
+ I915_WRITE(GEN8_DE_PORT_IIR, iir);
ret = IRQ_HANDLED;
- if (tmp & aux_mask) {
+ tmp_mask = GEN8_AUX_CHANNEL_A;
+ if (INTEL_INFO(dev_priv)->gen >= 9)
+ tmp_mask |= GEN9_AUX_CHANNEL_B |
+ GEN9_AUX_CHANNEL_C |
+ GEN9_AUX_CHANNEL_D;
+
+ if (iir & tmp_mask) {
dp_aux_irq_handler(dev);
found = true;
}
- if (hotplug_trigger) {
- if (IS_BROXTON(dev))
- bxt_hpd_irq_handler(dev, hotplug_trigger, hpd_bxt);
- else
- ilk_hpd_irq_handler(dev, hotplug_trigger, hpd_bdw);
- found = true;
+ if (IS_BROXTON(dev_priv)) {
+ tmp_mask = iir & BXT_DE_PORT_HOTPLUG_MASK;
+ if (tmp_mask) {
+ bxt_hpd_irq_handler(dev, tmp_mask, hpd_bxt);
+ found = true;
+ }
+ } else if (IS_BROADWELL(dev_priv)) {
+ tmp_mask = iir & GEN8_PORT_DP_A_HOTPLUG;
+ if (tmp_mask) {
+ ilk_hpd_irq_handler(dev, tmp_mask, hpd_bdw);
+ found = true;
+ }
}
- if (IS_BROXTON(dev) && (tmp & BXT_DE_PORT_GMBUS)) {
+ if (IS_BROXTON(dev) && (iir & BXT_DE_PORT_GMBUS)) {
gmbus_irq_handler(dev);
found = true;
}
@@ -2349,49 +2350,51 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg)
}
for_each_pipe(dev_priv, pipe) {
- uint32_t pipe_iir, flip_done = 0, fault_errors = 0;
+ u32 flip_done, fault_errors;
if (!(master_ctl & GEN8_DE_PIPE_IRQ(pipe)))
continue;
- pipe_iir = I915_READ(GEN8_DE_PIPE_IIR(pipe));
- if (pipe_iir) {
- ret = IRQ_HANDLED;
- I915_WRITE(GEN8_DE_PIPE_IIR(pipe), pipe_iir);
+ iir = I915_READ(GEN8_DE_PIPE_IIR(pipe));
+ if (!iir) {
+ DRM_ERROR("The master control interrupt lied (DE PIPE)!\n");
+ continue;
+ }
- if (pipe_iir & GEN8_PIPE_VBLANK &&
- intel_pipe_handle_vblank(dev, pipe))
- intel_check_page_flip(dev, pipe);
+ ret = IRQ_HANDLED;
+ I915_WRITE(GEN8_DE_PIPE_IIR(pipe), iir);
- if (INTEL_INFO(dev_priv)->gen >= 9)
- flip_done = pipe_iir & GEN9_PIPE_PLANE1_FLIP_DONE;
- else
- flip_done = pipe_iir & GEN8_PIPE_PRIMARY_FLIP_DONE;
+ if (iir & GEN8_PIPE_VBLANK &&
+ intel_pipe_handle_vblank(dev, pipe))
+ intel_check_page_flip(dev, pipe);
- if (flip_done) {
- intel_prepare_page_flip(dev, pipe);
- intel_finish_page_flip_plane(dev, pipe);
- }
+ flip_done = iir;
+ if (INTEL_INFO(dev_priv)->gen >= 9)
+ flip_done &= GEN9_PIPE_PLANE1_FLIP_DONE;
+ else
+ flip_done &= GEN8_PIPE_PRIMARY_FLIP_DONE;
- if (pipe_iir & GEN8_PIPE_CDCLK_CRC_DONE)
- hsw_pipe_crc_irq_handler(dev, pipe);
+ if (flip_done) {
+ intel_prepare_page_flip(dev, pipe);
+ intel_finish_page_flip_plane(dev, pipe);
+ }
- if (pipe_iir & GEN8_PIPE_FIFO_UNDERRUN)
- intel_cpu_fifo_underrun_irq_handler(dev_priv,
- pipe);
+ if (iir & GEN8_PIPE_CDCLK_CRC_DONE)
+ hsw_pipe_crc_irq_handler(dev, pipe);
+ if (iir & GEN8_PIPE_FIFO_UNDERRUN)
+ intel_cpu_fifo_underrun_irq_handler(dev_priv, pipe);
- if (INTEL_INFO(dev_priv)->gen >= 9)
- fault_errors = pipe_iir & GEN9_DE_PIPE_IRQ_FAULT_ERRORS;
- else
- fault_errors = pipe_iir & GEN8_DE_PIPE_IRQ_FAULT_ERRORS;
+ fault_errors = iir;
+ if (INTEL_INFO(dev_priv)->gen >= 9)
+ fault_errors &= GEN9_DE_PIPE_IRQ_FAULT_ERRORS;
+ else
+ fault_errors &= GEN8_DE_PIPE_IRQ_FAULT_ERRORS;
- if (fault_errors)
- DRM_ERROR("Fault errors on pipe %c\n: 0x%08x",
- pipe_name(pipe),
- pipe_iir & GEN8_DE_PIPE_IRQ_FAULT_ERRORS);
- } else
- DRM_ERROR("The master control interrupt lied (DE PIPE)!\n");
+ if (fault_errors)
+ DRM_ERROR("Fault errors on pipe %c\n: 0x%08x",
+ pipe_name(pipe),
+ fault_errors);
}
if (HAS_PCH_SPLIT(dev) && !HAS_PCH_NOP(dev) &&
@@ -2401,15 +2404,15 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg)
* scheme also closed the SDE interrupt handling race we've seen
* on older pch-split platforms. But this needs testing.
*/
- u32 pch_iir = I915_READ(SDEIIR);
- if (pch_iir) {
- I915_WRITE(SDEIIR, pch_iir);
+ iir = I915_READ(SDEIIR);
+ if (iir) {
+ I915_WRITE(SDEIIR, iir);
ret = IRQ_HANDLED;
if (HAS_PCH_SPT(dev_priv))
- spt_irq_handler(dev, pch_iir);
+ spt_irq_handler(dev, iir);
else
- cpt_irq_handler(dev, pch_iir);
+ cpt_irq_handler(dev, iir);
} else {
/*
* Like on previous PCH there seems to be something
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/2] drm/i915/gen8: Factor out display interrupt handling
2016-01-12 16:04 [PATCH 1/2] drm/i915/gen8: Tidy display interrupt processing Tvrtko Ursulin
@ 2016-01-12 16:04 ` Tvrtko Ursulin
2016-01-12 17:13 ` Chris Wilson
2016-01-12 17:12 ` [PATCH 1/2] drm/i915/gen8: Tidy display interrupt processing Chris Wilson
2016-01-13 7:49 ` ✗ warning: Fi.CI.BAT Patchwork
2 siblings, 1 reply; 6+ messages in thread
From: Tvrtko Ursulin @ 2016-01-12 16:04 UTC (permalink / raw)
To: Intel-gfx
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Tidy quite long interrupt service routine by factoring out
the display part.
This simplifies the exit path a little bit, makes the code
a bit more readable, and potentialy makes code reuse in the
future easier.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_irq.c | 53 ++++++++++++++++++++++++-----------------
1 file changed, 31 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 7972ceee6096..25a89373df63 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2264,31 +2264,14 @@ static void bxt_hpd_irq_handler(struct drm_device *dev, u32 hotplug_trigger,
intel_hpd_irq_handler(dev, pin_mask, long_mask);
}
-static irqreturn_t gen8_irq_handler(int irq, void *arg)
+static irqreturn_t
+gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
{
- struct drm_device *dev = arg;
- struct drm_i915_private *dev_priv = dev->dev_private;
- u32 master_ctl, iir;
+ struct drm_device *dev = dev_priv->dev;
irqreturn_t ret = IRQ_NONE;
+ u32 iir;
enum pipe pipe;
- if (!intel_irqs_enabled(dev_priv))
- return IRQ_NONE;
-
- /* IRQs are synced during runtime_suspend, we don't require a wakeref */
- disable_rpm_wakeref_asserts(dev_priv);
-
- master_ctl = I915_READ_FW(GEN8_MASTER_IRQ);
- master_ctl &= ~GEN8_MASTER_IRQ_CONTROL;
- if (!master_ctl)
- goto out;
-
- I915_WRITE_FW(GEN8_MASTER_IRQ, 0);
-
- /* Find, clear, then process each source of interrupt */
-
- ret = gen8_gt_irq_handler(dev_priv, master_ctl);
-
if (master_ctl & GEN8_DE_MISC_IRQ) {
iir = I915_READ(GEN8_DE_MISC_IIR);
if (iir) {
@@ -2422,10 +2405,36 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg)
}
}
+ return ret;
+}
+
+static irqreturn_t gen8_irq_handler(int irq, void *arg)
+{
+ struct drm_device *dev = arg;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ u32 master_ctl;
+ irqreturn_t ret;
+
+ if (!intel_irqs_enabled(dev_priv))
+ return IRQ_NONE;
+
+ master_ctl = I915_READ_FW(GEN8_MASTER_IRQ);
+ master_ctl &= ~GEN8_MASTER_IRQ_CONTROL;
+ if (!master_ctl)
+ return IRQ_NONE;
+
+ 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 */
+ ret = gen8_gt_irq_handler(dev_priv, master_ctl);
+ ret |= gen8_de_irq_handler(dev_priv, master_ctl);
+
I915_WRITE_FW(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
POSTING_READ_FW(GEN8_MASTER_IRQ);
-out:
enable_rpm_wakeref_asserts(dev_priv);
return ret;
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] drm/i915/gen8: Tidy display interrupt processing
2016-01-12 16:04 [PATCH 1/2] drm/i915/gen8: Tidy display interrupt processing Tvrtko Ursulin
2016-01-12 16:04 ` [PATCH 2/2] drm/i915/gen8: Factor out display interrupt handling Tvrtko Ursulin
@ 2016-01-12 17:12 ` Chris Wilson
2016-01-12 17:42 ` Tvrtko Ursulin
2016-01-13 7:49 ` ✗ warning: Fi.CI.BAT Patchwork
2 siblings, 1 reply; 6+ messages in thread
From: Chris Wilson @ 2016-01-12 17:12 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: Intel-gfx
On Tue, Jan 12, 2016 at 04:04:06PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> One bugfix and a few tidy-ups:
>
> * Pipe fault logging was broken on Gen9+.
> * Removed some unnecessary local variables.
> * Removed unnecessary initializers.
> * Decreased pipe iir block indentation level.
> * Grouped variable initialization close to use sites.
Out of curiosity did the compiled output change from the changes in
local variables? I'd slap a much-appreciated-by on anything that could
reduce the instruction count in our interrupt handlers!
Didn't spot any mistakes, so
Reviewed-by: Chris Wilson <chris@cris-wilson.co.uk>
but I'd like to give some extra credit if you have shrunk this beast.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] drm/i915/gen8: Tidy display interrupt processing
2016-01-12 17:12 ` [PATCH 1/2] drm/i915/gen8: Tidy display interrupt processing Chris Wilson
@ 2016-01-12 17:42 ` Tvrtko Ursulin
0 siblings, 0 replies; 6+ messages in thread
From: Tvrtko Ursulin @ 2016-01-12 17:42 UTC (permalink / raw)
To: Chris Wilson, Intel-gfx
On 12/01/16 17:12, Chris Wilson wrote:
> On Tue, Jan 12, 2016 at 04:04:06PM +0000, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> One bugfix and a few tidy-ups:
>>
>> * Pipe fault logging was broken on Gen9+.
>> * Removed some unnecessary local variables.
>> * Removed unnecessary initializers.
>> * Decreased pipe iir block indentation level.
>> * Grouped variable initialization close to use sites.
>
> Out of curiosity did the compiled output change from the changes in
> local variables? I'd slap a much-appreciated-by on anything that could
> reduce the instruction count in our interrupt handlers!
>
> Didn't spot any mistakes, so
>
> Reviewed-by: Chris Wilson <chris@cris-wilson.co.uk>
>
> but I'd like to give some extra credit if you have shrunk this beast.
Two steps forward, one step back. :)
vanilla:
0000000000005630 0000000000000668 t gen8_irq_handler
+drm/i915/gen8: Tidy display interrupt processing
0000000000005630 0000000000000633 t gen8_irq_handler
++drm/i915/gen8: Factor out display interrupt handling
0000000000005630 000000000000065c t gen8_irq_handler
Thanks for the review!
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* ✗ warning: Fi.CI.BAT
2016-01-12 16:04 [PATCH 1/2] drm/i915/gen8: Tidy display interrupt processing Tvrtko Ursulin
2016-01-12 16:04 ` [PATCH 2/2] drm/i915/gen8: Factor out display interrupt handling Tvrtko Ursulin
2016-01-12 17:12 ` [PATCH 1/2] drm/i915/gen8: Tidy display interrupt processing Chris Wilson
@ 2016-01-13 7:49 ` Patchwork
2 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2016-01-13 7:49 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: intel-gfx
== Summary ==
Built on 06d0112e293dfdea7f796d4085f755898850947b drm-intel-nightly: 2016y-01m-12d-21h-16m-40s UTC integration manifest
Test gem_storedw_loop:
Subgroup basic-render:
pass -> DMESG-WARN (bdw-nuci7)
dmesg-warn -> PASS (bdw-ultra)
Test kms_flip:
Subgroup basic-flip-vs-dpms:
dmesg-warn -> PASS (skl-i7k-2)
dmesg-warn -> PASS (ilk-hp8440p)
Test kms_pipe_crc_basic:
Subgroup read-crc-pipe-a-frame-sequence:
fail -> PASS (snb-x220t)
bdw-nuci7 total:138 pass:128 dwarn:1 dfail:0 fail:0 skip:9
bdw-ultra total:138 pass:132 dwarn:0 dfail:0 fail:0 skip:6
bsw-nuc-2 total:141 pass:115 dwarn:2 dfail:0 fail:0 skip:24
byt-nuc total:141 pass:123 dwarn:3 dfail:0 fail:0 skip:15
hsw-brixbox total:141 pass:134 dwarn:0 dfail:0 fail:0 skip:7
hsw-gt2 total:141 pass:137 dwarn:0 dfail:0 fail:0 skip:4
hsw-xps12 total:138 pass:133 dwarn:1 dfail:0 fail:0 skip:4
ilk-hp8440p total:141 pass:101 dwarn:3 dfail:0 fail:0 skip:37
ivb-t430s total:135 pass:122 dwarn:3 dfail:4 fail:0 skip:6
skl-i5k-2 total:141 pass:132 dwarn:1 dfail:0 fail:0 skip:8
skl-i7k-2 total:141 pass:131 dwarn:2 dfail:0 fail:0 skip:8
snb-dellxps total:141 pass:122 dwarn:5 dfail:0 fail:0 skip:14
snb-x220t total:141 pass:122 dwarn:5 dfail:0 fail:1 skip:13
Results at /archive/results/CI_IGT_test/Patchwork_1155/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-01-13 7:49 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-01-12 16:04 [PATCH 1/2] drm/i915/gen8: Tidy display interrupt processing Tvrtko Ursulin
2016-01-12 16:04 ` [PATCH 2/2] drm/i915/gen8: Factor out display interrupt handling Tvrtko Ursulin
2016-01-12 17:13 ` Chris Wilson
2016-01-12 17:12 ` [PATCH 1/2] drm/i915/gen8: Tidy display interrupt processing Chris Wilson
2016-01-12 17:42 ` Tvrtko Ursulin
2016-01-13 7:49 ` ✗ warning: Fi.CI.BAT Patchwork
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).