[Intel-gfx] [PATCH 2/2] drm/i915/scheduler: emulate a scheduler for guc

[Dong, Chuanxiao]

> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces at lists.freedesktop.org] On Behalf
> Of Tvrtko Ursulin
> Sent: Wednesday, February 15, 2017 8:34 PM
> To: Chris Wilson <chris at chris-wilson.co.uk>; intel-gfx at lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 2/2] drm/i915/scheduler: emulate a scheduler
> for guc
> 
> 
> On 15/02/2017 12:20, Chris Wilson wrote:
> > On Wed, Feb 15, 2017 at 11:56:28AM +0000, Tvrtko Ursulin wrote:
> >>
> >> On 14/02/2017 11:44, Chris Wilson wrote:
> >>> diff --git a/drivers/gpu/drm/i915/i915_irq.c
> >>> b/drivers/gpu/drm/i915/i915_irq.c index cdc7da60d37a..aa886b5fb2cd
> >>> 100644
> >>> --- a/drivers/gpu/drm/i915/i915_irq.c
> >>> +++ b/drivers/gpu/drm/i915/i915_irq.c
> >>> @@ -1350,13 +1350,20 @@ static void snb_gt_irq_handler(struct
> >>> drm_i915_private *dev_priv, static __always_inline void
> >>> gen8_cs_irq_handler(struct intel_engine_cs *engine, u32 iir, int
> >>> test_shift) {
> >>> -	if (iir & (GT_RENDER_USER_INTERRUPT << test_shift))
> >>> -		notify_ring(engine);
> >>> +	bool tasklet = false;
> >>>
> >>> 	if (iir & (GT_CONTEXT_SWITCH_INTERRUPT << test_shift)) {
> >>> 		set_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted);
> >>> -		tasklet_hi_schedule(&engine->irq_tasklet);
> >>> +		tasklet = true;
> >>> 	}
> >>> +
> >>> +	if (iir & (GT_RENDER_USER_INTERRUPT << test_shift)) {
> >>> +		notify_ring(engine);
> >>> +		tasklet |= i915.enable_guc_submission;
> >>
> >> Hm.. noticed that BXT was quite unhappy in CI?
> >>
> >> I wonder if we need to set ENGINE_IRQ_EXECLISTS here (and clear it in
> >> the irq tasklet as well) to be fully identical between the two
> >> backends.
> >>
> >> Not saying that was the reason for the CI unhappiness, just an
> >> observation at this stage.
> >
> > I don't think we want to. Note that the bit will always be unset, so
> > that shouldn't be causing the intel_execlists_idle() timeout. I was
> 
> Yeah agreed, but when you added the bit test in intel_execlists_idle, the
> commit message said it is a sanity check before suspend. So presumably it
> would make sense to have the same for the GuC tasklet.
> 
> > fearing a missed interrupt as we only idle once the request list is
> > empty, but that means we didn't process the interrupt. Or something
> > scary like that. (I do recall seeing missed interrupts on my skl with
> > guc btw, don't know if that is still a feature.)
> 
> No idea and no BXT to test on. But it is something new compared to the
> previous CI run. So something changed in the tree in the meantime to cause
> it.

Seems recently community has some bug fixing for the GuC CI testing. And this patch already cannot directly be applied on the latest intel-gfx tree. Is there any plan to re-submit this patch? I am asking because the current i915 host GuC scheduling doesn't provide any chance for GVT to do the notification like using execlist submit. But with this implementation, GuC can provide the right point to do the notification for GVT as well.

Thanks
Chuanxiao

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