[Intel-gfx] [PATCH] drm/i915/gt: Acquire a GT wakeref for the breadcrumb interrupt

[Tvrtko Ursulin]

On 05/12/2019 21:58, Chris Wilson wrote:
> Take a wakeref on the intel_gt specifically for the enabled breadcrumb
> interrupt so that we can safely process the mmio. If the intel_gt is
> already asleep by the time we try and setup the breadcrumb interrupt, by
> a process of elimination we know the request must have been completed
> and we can skip its enablement!
> 
> <4> [1518.350005] Unclaimed write to register 0x220a8
> <4> [1518.350323] WARNING: CPU: 2 PID: 3685 at drivers/gpu/drm/i915/intel_uncore.c:1163 __unclaimed_reg_debug+0x40/0x50 [i915]
> <4> [1518.350393] Modules linked in: vgem snd_hda_codec_hdmi x86_pkg_temp_thermal i915 coretemp crct10dif_pclmul crc32_pclmul ghash_clmulni_intel snd_hda_intel snd_intel_dspcfg snd_hda_codec snd_hwdep snd_hda_core btusb cdc_ether btrtl usbnet btbcm btintel r8152 snd_pcm mii bluetooth ecdh_generic ecc i2c_hid pinctrl_sunrisepoint pinctrl_intel intel_lpss_pci prime_numbers [last unloaded: vgem]
> <4> [1518.350646] CPU: 2 PID: 3685 Comm: gem_exec_parse_ Tainted: G     U            5.4.0-rc8-CI-CI_DRM_7490+ #1
> <4> [1518.350708] Hardware name: Google Caroline/Caroline, BIOS MrChromebox 08/27/2018
> <4> [1518.350946] RIP: 0010:__unclaimed_reg_debug+0x40/0x50 [i915]
> <4> [1518.350992] Code: 74 05 5b 5d 41 5c c3 45 84 e4 48 c7 c0 95 8d 47 a0 48 c7 c6 8b 8d 47 a0 48 0f 44 f0 89 ea 48 c7 c7 9e 8d 47 a0 e8 40 45 e3 e0 <0f> 0b 83 2d 27 4f 2a 00 01 5b 5d 41 5c c3 66 90 41 55 41 54 55 53
> <4> [1518.351100] RSP: 0018:ffffc900007f39c8 EFLAGS: 00010086
> <4> [1518.351140] RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000006
> <4> [1518.351202] RDX: 0000000080000006 RSI: 0000000000000000 RDI: 00000000ffffffff
> <4> [1518.351249] RBP: 00000000000220a8 R08: 0000000000000000 R09: 0000000000000000
> <4> [1518.351296] R10: ffffc900007f3990 R11: ffffc900007f3868 R12: 0000000000000000
> <4> [1518.351342] R13: 00000000fefeffff R14: 0000000000000092 R15: ffff888155fea000
> <4> [1518.351391] FS:  00007fc255abfe40(0000) GS:ffff88817ab00000(0000) knlGS:0000000000000000
> <4> [1518.351445] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> <4> [1518.351485] CR2: 00007fc2554882d0 CR3: 0000000168ca2005 CR4: 00000000003606e0
> <4> [1518.351529] Call Trace:
> <4> [1518.351746]  fwtable_write32+0x114/0x1d0 [i915]
> <4> [1518.351795]  ? sync_file_alloc+0x80/0x80
> <4> [1518.352039]  gen8_logical_ring_enable_irq+0x30/0x50 [i915]
> <4> [1518.352295]  irq_enable.part.10+0x23/0x40 [i915]
> <4> [1518.352523]  i915_request_enable_breadcrumb+0xb5/0x330 [i915]
> <4> [1518.352575]  ? sync_file_alloc+0x80/0x80
> <4> [1518.352612]  __dma_fence_enable_signaling+0x60/0x160
> <4> [1518.352653]  ? sync_file_alloc+0x80/0x80
> <4> [1518.352685]  dma_fence_add_callback+0x44/0xd0
> <4> [1518.352726]  sync_file_poll+0x95/0xc0
> <4> [1518.352767]  do_sys_poll+0x24d/0x570
> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/gt/intel_breadcrumbs.c | 22 +++++++++++++++------
>   1 file changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
> index 55317081d48b..8a9facf4f3b6 100644
> --- a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
> @@ -28,6 +28,7 @@
>   
>   #include "i915_drv.h"
>   #include "i915_trace.h"
> +#include "intel_gt_pm.h"
>   
>   static void irq_enable(struct intel_engine_cs *engine)
>   {
> @@ -53,15 +54,17 @@ static void irq_disable(struct intel_engine_cs *engine)
>   
>   static void __intel_breadcrumbs_disarm_irq(struct intel_breadcrumbs *b)
>   {
> +	struct intel_engine_cs *engine =
> +		container_of(b, struct intel_engine_cs, breadcrumbs);
> +
>   	lockdep_assert_held(&b->irq_lock);
>   
>   	GEM_BUG_ON(!b->irq_enabled);
>   	if (!--b->irq_enabled)
> -		irq_disable(container_of(b,
> -					 struct intel_engine_cs,
> -					 breadcrumbs));
> +		irq_disable(engine);
>   
>   	b->irq_armed = false;
> +	intel_gt_pm_put_async(engine->gt);
>   }
>   
>   void intel_engine_disarm_breadcrumbs(struct intel_engine_cs *engine)
> @@ -207,14 +210,17 @@ static void signal_irq_work(struct irq_work *work)
>   	intel_engine_breadcrumbs_irq(engine);
>   }
>   
> -static void __intel_breadcrumbs_arm_irq(struct intel_breadcrumbs *b)
> +static bool __intel_breadcrumbs_arm_irq(struct intel_breadcrumbs *b)
>   {
>   	struct intel_engine_cs *engine =
>   		container_of(b, struct intel_engine_cs, breadcrumbs);
>   
>   	lockdep_assert_held(&b->irq_lock);
>   	if (b->irq_armed)
> -		return;
> +		return true;
> +
> +	if (!intel_gt_pm_get_if_awake(engine->gt))
> +		return false;
>   
>   	/*
>   	 * The breadcrumb irq will be disarmed on the interrupt after the
> @@ -234,6 +240,8 @@ static void __intel_breadcrumbs_arm_irq(struct intel_breadcrumbs *b)
>   
>   	if (!b->irq_enabled++)
>   		irq_enable(engine);
> +
> +	return true;
>   }
>   
>   void intel_engine_init_breadcrumbs(struct intel_engine_cs *engine)
> @@ -277,7 +285,8 @@ bool i915_request_enable_breadcrumb(struct i915_request *rq)
>   		spin_lock(&b->irq_lock);
>   		GEM_BUG_ON(test_bit(I915_FENCE_FLAG_SIGNAL, &rq->fence.flags));
>   
> -		__intel_breadcrumbs_arm_irq(b);
> +		if (!__intel_breadcrumbs_arm_irq(b))
> +			goto unlock;
>   
>   		/*
>   		 * We keep the seqno in retirement order, so we can break
> @@ -306,6 +315,7 @@ bool i915_request_enable_breadcrumb(struct i915_request *rq)
>   		GEM_BUG_ON(!check_signal_order(ce, rq));
>   
>   		set_bit(I915_FENCE_FLAG_SIGNAL, &rq->fence.flags);
> +unlock:
>   		spin_unlock(&b->irq_lock);
>   	}
>   
> 

Any special reason not to do PM management on the engine rather than GT?

Regards,

Tvrtko