Re: [PATCH 2/2] drm/i915/scheduler: emulate a scheduler for guc

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

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.

Regards,

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