Re: [Intel-gfx] [RFC 5/5] drm/i915: introduce intel_execlists_submission.<c/h>

[Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>]

<snip>


>> +
>> +struct i915_request *
>> +execlists_unwind_incomplete_requests(struct intel_engine_execlists *execlists)
> 
> There should be no exports from this file... Did you not also make
> guc_submission standalone?
> 

The new GuC submission code will have its own 
_unwind_incomplete_requests function, just didn't seem worth it copying 
this to the GuC file now to make this static and get rid of it later. 
The current version of the GuC patches (being worked on by Matt) is also 
not yet fully standalone, but we're moving into that direction.


<snip>

>> +bool
>> +intel_engine_in_execlists_submission_mode(const struct intel_engine_cs *engine)
>> +{
>> +       return engine->set_default_submission ==
>> +              intel_execlists_set_default_submission;
>> +}
> 
> The breadcrumb submission code is specialised to execlists and should
> not be shared (leaves emit_flush, emit_bb_start as common
> gen8_submission.c). The reset code is specialised to execlists and should not
> be shared. The virtual engine is specialised to execlists and should not
> be shared. Even submit_request should be distinct between guc and
> execlists, especially request_alloc (which you may like to put on the
> context_ops rather than engine)
> -Chris
> 

engine->reset.*, request_alloc and submit_request have all been moved to 
execlists_submission.c in this patch, with the aim of not sharing them.

For the virtual engine, I've moved the submission related chunks to 
execlists_submission.c as well (see the new 
intel_execlists_virtual_submission_init, although I could probably move 
a few extra bits in there). As I mentioned on the other reply, other 
parts do seem quite generic to me, but let's keep this chunk of the 
discussion on the other thread.

Regarding the breadcrumb code, IMO we do still want to share most of it 
(seqno writing, interrupt, MI_ARB_CHECK, wa_tail), but we most likely 
won't need the preempt_busywait. Given this, it didn't feel right to me 
to move the relevant code out of the file until we get some more mature 
GuC code to make a cleaner call.

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