On Fri, 15 Oct 2021 14:46:20 -0700, John Harrison wrote: > > On 10/15/2021 07:52, Dixit, Ashutosh wrote: > > On Thu, 14 Oct 2021 12:42:38 -0700, <John.C.Harrison@xxxxxxxxx> wrote: > >> + /* > >> + * These tests are for a specific scheduling model which is > >> + * not currently implemented by GuC. So skip on GuC platforms. > >> + */ > >> + devid = intel_get_drm_devid(i915); > >> + igt_require((intel_gen(devid) < 12) || IS_TIGERLAKE(devid) || > >> + IS_ROCKETLAKE(devid) || IS_ALDERLAKE_S(devid)); > > As I hinted on v1 let's just do this here: > > > > igt_require(gem_has_guc_submission(i915)); > > > > So that we can can have a single unified way of detecting if GuC is being > > used throughout IGT. Today it is gem_has_guc_submission() and it works with > > the current kernel. > > Earlier, you were saying that 'has' was only checking for capability not > usage. Which would be pretty useless for this situation. Looking at the > code, though it sort of does work. It checks the live value of the > enable_guc module parameter. If that says that GuC submission is enabled > then either we are using GuC submission or we have no engines (because a > failure to start the submission backend is terminal, there is no fallback > to execlist mode if GuC didn't work). So it can be used. > > I say sort of, though, because the code also sets 'has_execlists' when it > sets 'has_guc'. Which means that the gem_has_execlists() test is not > useable as an indication that the execlist back end is being used. So > gem_has_execlists() and gem_has_guc_submission() are basically very > non-orthogonal. One is a test of hardware presence irrespective of use, the > other is a test of usage irrespective of presence. The comment in the code > is 'query whether the driver is using execlists as a hardware submission > method'. So it seems like that was the original intention. Whether it has > been broken since or was just broken from the beginning is unclear. I completely agree, there is all this confusion in the code, it needs to be cleaned up. For now I thought it was better to use a centralized way to detect GuC rather than have different ways in different files. Fortunately gem_has_guc_submission() is doing that though gem_has_execlists() is broken in detecting if execlists are being used.
