[Intel-gfx] [RFC PATCH 53/97] drm/i915/guc: Disable semaphores when using GuC scheduling

[Tvrtko Ursulin]

On 25/05/2021 18:01, Matthew Brost wrote:
> On Tue, May 25, 2021 at 10:52:01AM +0100, Tvrtko Ursulin wrote:
>>
>> On 06/05/2021 20:14, Matthew Brost wrote:
>>> Disable semaphores when using GuC scheduling as semaphores are broken in
>>> the current GuC firmware.
>>
>> What is "current"? Given that the patch itself is like year and a half old.
>>
> 
> Stale comment. Semaphore work with the firmware we just haven't enabled
> them in the i915 with GuC submission as this an optimization and not
> required for functionality.

How will the updated commit message look in terms of remaining reasons 
why semaphores won't/can't be enabled?

They were a nice performance win on some media workloads although 
granted a lot of tweaking was required to find a good balance on when to 
use them and when not to.

Regards,

Tvrtko

> Matt
> 
>> Regards,
>>
>> Tvrtko
>>
>>> Cc: John Harrison <john.c.harrison at intel.com>
>>> Signed-off-by: Matthew Brost <matthew.brost at intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/gem/i915_gem_context.c | 6 ++++--
>>>    1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
>>> index 993faa213b41..d30260ffe2a7 100644
>>> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
>>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
>>> @@ -230,7 +230,8 @@ static void intel_context_set_gem(struct intel_context *ce,
>>>    		ce->timeline = intel_timeline_get(ctx->timeline);
>>>    	if (ctx->sched.priority >= I915_PRIORITY_NORMAL &&
>>> -	    intel_engine_has_timeslices(ce->engine))
>>> +	    intel_engine_has_timeslices(ce->engine) &&
>>> +	    intel_engine_has_semaphores(ce->engine))
>>>    		__set_bit(CONTEXT_USE_SEMAPHORES, &ce->flags);
>>>    	intel_context_set_watchdog_us(ce, ctx->watchdog.timeout_us);
>>> @@ -1939,7 +1940,8 @@ static int __apply_priority(struct intel_context *ce, void *arg)
>>>    	if (!intel_engine_has_timeslices(ce->engine))
>>>    		return 0;
>>> -	if (ctx->sched.priority >= I915_PRIORITY_NORMAL)
>>> +	if (ctx->sched.priority >= I915_PRIORITY_NORMAL &&
>>> +	    intel_engine_has_semaphores(ce->engine))
>>>    		intel_context_set_use_semaphores(ce);
>>>    	else
>>>    		intel_context_clear_use_semaphores(ce);
>>>