[igt-dev] [PATCH i-g-t 1/3] lib/i915: Return actual submission method from gem_submission_method

[Dixit, Ashutosh]

On Fri, 29 Oct 2021 17:42:19 -0700, John Harrison wrote:
>
> On 10/28/2021 17:31, Dixit, Ashutosh wrote:
> > On Thu, 28 Oct 2021 16:55:32 -0700, John Harrison wrote:
> >> On 10/27/2021 21:40, Ashutosh Dixit wrote:
> >>> gem_submission_method() purports to return the currently used submission
> >>> method by the kernel, as evidenced by its callers. Therefore remove the
> >>> GEM_SUBMISSION_EXECLISTS flag when GuC submission is detected.
> >>>
> >>> This also fixes gem_has_execlists() to match its description, previously
> >>> gem_has_execlists() would return true even if GuC submission was actually
> >>> being used in the driver.
> >>>
> >>> v2: Or gem_has_execlists call-sites with gem_has_guc_submission to make the
> >>>       new code equivalent to the previous code.
> >>> v3: Clarify that submission method is either guc (0x4), execlists (0x2) or
> >>>       legacy without semaphores (0x0) or legacy with semaphores (0x1)
> >> What about GuC with semaphores vs GuC without semaphores? Likewise execlist.
> >>
> >> Semaphores is not a submission method. They are a submission feature whose
> >> support or lack there of is independent of the submission method.
> > Sorry I didn't know that. But if you see gem_submission_method() in current
> > upstream IGT (pasted below) it doesn't return "GuC with semaphores' or
> > "execlist with semaphores" either. Anyway, let me do this in the next
> > rev. Thanks.
> I mean that in the old code:
>    "GuC with semaphores" == is_guc_submission() && has_semaphores()
>
> But if you conflate semaphores with the submission method then you have to
> explicitly enumerate all combinations.
>
> Hmm, okay. Just realised that the old code did indeed have
> GEM_SUBMISSION_SEMAPHORES. Not sure what that was about!? You can
> definitely have semaphores with GuC/execlists and you can definitely have
> GuC/execlists without semaphores. I don't see why ring buffer submission
> would be any different but I admit it's been a while since I've looked at
> any ring buffer code.
>
> It doesn't look like anything was actually using 'gem_has_semaphores()' at
> all, though. So nothing would notice if it was removed ;).

I think I am going to do this. Also, has_semaphores() calls here in i915:

	int i915_getparam_ioctl()
	{
		...
		case I915_PARAM_HAS_SEMAPHORES:
			value = !!(i915->caps.scheduler & I915_SCHEDULER_CAP_SEMAPHORES);
			break;

But appears I915_SCHEDULER_CAP_SEMAPHORES bits are never set in
caps.scheduler so has_semaphores() should always be returning zero.