- 22 Jul, 2021 26 commits
-
-
Matthew Brost authored
Disable preempt busywait when using GuC scheduling. This isn't needed as the GuC controls preemption when scheduling. v2: (John H): - Fix commit message Cc: John Harrison <john.c.harrison@intel.com> Signed-off-by:
Matthew Brost <matthew.brost@intel.com> Reviewed-by:
John Harrison <John.C.Harrison@Intel.com> Signed-off-by:
-
Matthew Brost authored
Extend the deregistration context fence to fence whne a GuC context has scheduling disable pending. v2: (John H) - Update comment why we check the pin count within spin lock Cc: John Harrison <john.c.harrison@intel.com> Signed-off-by:
-
Matthew Brost authored
Disable engine barriers for unpinning with GuC. This feature isn't needed with the GuC as it disables context scheduling before unpinning which guarantees the HW will not reference the context. Hence it is not necessary to defer unpinning until a kernel context request completes on each engine in the context engine mask. Cc: John Harrison <john.c.harrison@intel.com> Signed-off-by:
Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Reviewed-by:
-
Matthew Brost authored
With GuC scheduling, it isn't safe to unpin a context while scheduling is enabled for that context as the GuC may touch some of the pinned state (e.g. LRC). To ensure scheduling isn't enabled when an unpin is done, a call back is added to intel_context_unpin when pin count == 1 to disable scheduling for that context. When the response CTB is received it is safe to do the final unpin. Future patches may add a heuristic / delay to schedule the disable call back to avoid thrashing on schedule enable / disable. v2: (John H) - s/drm_dbg/drm_err (Daneiel) - Clean up sched state function Cc: John Harrison <john.c.harrison@intel.com> Signed-off-by:
-
Matthew Brost authored
Sometimes during context pinning a context with the same guc_id is registered with the GuC. In this a case deregister must be done before the context can be registered. A fence is inserted on all requests while the deregister is in flight. Once the G2H is received indicating the deregistration is complete the context is registered and the fence is released. v2: (John H) - Fix commit message Cc: John Harrison <john.c.harrison@intel.com> Signed-off-by:
-
Matthew Brost authored
Implement GuC context operations which includes GuC specific operations alloc, pin, unpin, and destroy. v2: (Daniel Vetter) - Use msleep_interruptible rather than cond_resched in busy loop (Michal) - Remove C++ style comment v3: (Matthew Brost) - Drop GUC_ID_START (John Harrison) - Fix a bunch of typos - Use drm_err rather than drm_dbg for G2H errors (Daniele) - Fix ;; typo - Clean up sched state functions - Add lockdep for guc_id functions - Don't call __release_guc_id when guc_id is invalid - Use MISSING_CASE - Add comment in guc_context_pin - Use shorter path to rpm (Daniele / CI) - Don't call release_guc_id on an invalid guc_id in destroy v4: (Daniel Vetter) - Add FIXME comment Signed-off-by:
-
Matthew Brost authored
Add bypass tasklet submission path to GuC. The tasklet is only used if H2G channel has backpresure. Signed-off-by:
-
Matthew Brost authored
Implement GuC submission tasklet for new interface. The new GuC interface uses H2G to submit contexts to the GuC. Since H2G use a single channel, a single tasklet is used for the submission path. Also the per engine interrupt handler has been updated to disable the rescheduling of the physical engine tasklet, when using GuC scheduling, as the physical engine tasklet is no longer used. In this patch the field, guc_id, has been added to intel_context and is not assigned. Patches later in the series will assign this value. v2: (John Harrison) - Clean up some comments v3: (John Harrison) - More comment cleanups Cc: John Harrison <john.c.harrison@intel.com> Signed-off-by:
-
Matthew Brost authored
Add LRC descriptor context lookup array which can resolve the intel_context from the LRC descriptor index. In addition to lookup, it can determine if the LRC descriptor context is currently registered with the GuC by checking if an entry for a descriptor index is present. Future patches in the series will make use of this array. v2: (Michal) - "linux/xarray.h" -> <linux/xarray.h> - s/lrc/LRC (John H) - Fix commit message Cc: John Harrison <john.c.harrison@intel.com> Signed-off-by:
-
Matthew Brost authored
Remove old GuC stage descriptor, add LRC descriptor which will be used by the new GuC interface implemented in this patch series. v2: (John Harrison) - s/lrc/LRC/g Cc: John Harrison <john.c.harrison@intel.com> Signed-off-by:
-
Matthew Brost authored
Add new GuC interface defines and structures while maintaining old ones in parallel. Cc: John Harrison <john.c.harrison@intel.com> Signed-off-by:
-
Prathap Kumar Valsan authored
The layout of some engine contexts has changed on Xe_HP. Define the new offsets. Bspec: 45585, 46256 Signed-off-by:
Prathap Kumar Valsan <prathap.kumar.valsan@intel.com> Signed-off-by:
Ramalingam C <ramalingam.c@intel.com> Signed-off-by:
Venkata Ramana Nayana <venkata.ramana.nayana@intel.com> Signed-off-by:
Akeem G Abodunrin <akeem.g.abodunrin@intel.com> Signed-off-by:
Matt Roper <matthew.d.roper@intel.com> Reviewed-by:
Matt Atwood <matthew.s.atwood@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20210721223043.834562-10-matthew.d.roper@intel.com
-
Stuart Summers authored
Xe_HP changes the format of the context ID from past platforms. Signed-off-by:
Stuart Summers <stuart.summers@intel.com> Signed-off-by:
Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> Signed-off-by:
-
John Harrison authored
Increasing the engine count causes a couple of local array variables to exceed the kernel stack limit. So make them dynamic allocations instead. Signed-off-by:
Lucas De Marchi <lucas.demarchi@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20210721223043.834562-8-matthew.d.roper@intel.com
-
Venkata Sandeep Dhanalakota authored
In Gen12 there are various fuse combinations and in each configuration vdbox engine may be connected to SFC depending on which engines are available, so we need to set the SFC capability based on fuse value from the hardware. Even numbered physical instance always have SFC, odd numbered physical instances have SFC only if previous even instance is fused off. v2: - Minor style & typo fixes (Tvrtko) - Drop an unwanted 'inline' (Tvrtko) Bspec: 48028 Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Signed-off-by:
Venkata Sandeep Dhanalakota <venkata.s.dhanalakota@intel.com> Signed-off-by:
-
Matt Roper authored
As we begin applying XeHP and DG2 patches, the basic platform definitions and macros (like IS_DG2()) will be needed in both drm-intel-next and drm-intel-gt-next. Those initial definition patches are applied to a topic branch and merged to both trees. Signed-off-by: -
Matt Roper authored
DG2 has Xe_LPD display (version 13) and Xe_HPG (version 12.55) graphics. There are two variants (treated as subplatforms in the code): DG2-G10 and DG2-G11 that require independent programming in some areas (e.g., workarounds). Bspec: 44472, 44474, 46197, 48028, 48077 Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> Signed-off-by:
José Roberto de Souza <jose.souza@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20210721223043.834562-4-matthew.d.roper@intel.com
-
Lucas De Marchi authored
XeHP SDV is a Intel
® dGPU without display. This is just the definition of some basic platform macros, by large a copy of current state of Tigerlake which does not reflect the end state of this platform. v2: - Switch to intel_step infrastructure for stepping matches. (Jani) v3: - Bring earlier in patch series and leave addition of new media engines to the engine mask for a later patch. Bspec: 44467, 48077 Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Signed-off-by:Tomas Winkler <tomas.winkler@intel.com> Signed-off-by:
-
Lucas De Marchi authored
Our _FEATURES macro went back to GEN7, extending each other, making it difficult to grasp what was really enabled/disabled. Take the opportunity of the GEN -> XE_HP name break and also break with the feature inheritance. For XE_HP this basically goes from GEN12 back to GEN7 coalescing the features making sure the overrides remain, remove all the display-specific features and sort it. Then also remove the definitions that would be overridden by DGFX_FEATURES and those that were 0 (since that is the default). Exception here is has_master_unit_irq: although it is a feature that started with DG1 and is true for all DGFX platforms, it's also true for XE_HP in general. Signed-off-by:
-
Lucas De Marchi authored
Besides the arch version returned by GRAPHICS_VER(), new platforms contain a "release id" to make clear the difference from one platform to another. The release id number is not formally defined by hardware until future platforms that will expose it via a new GMD_ID register. For the platforms we support before that register becomes available we will set the values in software and we can set them as we please. So the plan is to set them so we can group different features under a single GRAPHICS_VER_FULL() check. After GMD_ID is used, the usefulness of a "full version check" will be greatly reduced and will be mostly used for deciding workarounds and a few code paths. So it makes sense to keep it as a separate field from graphics_ver. Also, as a platform with `release == n` may be closer feature-wise to `n - 2` than to `n - 1`, use the word "release" rather than the more common "minor" for this This is a mix of 2 independent changes: one by me and the other by Matt Roper. v2: - Reword commit message to make it clearer why we don't call it "minor" (Matt Roper and Tvrtko) - Rename variables s/*_ver_release/*_rel/ and print them in a single line formatted as {ver}.{rel:2} (Jani and Matt Roper) Cc: Matt Roper <matthew.d.roper@intel.com> Signed-off-by: -
Lucas De Marchi authored
Brevity is not needed here, so just spell out "* version" in the string. Suggested-by:
Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by:
-
Jason Ekstrand authored
There's no reason that I can tell why this should be per-i915_buddy_mm and doing so causes KMEM_CACHE to throw dmesg warnings because it tries to create a debugfs entry with the name i915_buddy_block multiple times. We could handle this by carefully giving each slab its own name but that brings its own pain because then we have to store that string somewhere and manage the lifetimes of the different slabs. The most likely outcome would be a global atomic which we increment to get a new name or something like that. The much easier solution is to use the i915_globals system like we do for every other slab in i915. This ensures that we have exactly one of them for each i915 driver load and it gets neatly created on module load and destroyed on module unload. Using the globals system also means that its now tied into the shrink handler so we can properly respond to low-memory situations. Signed-off-by:
Jason Ekstrand <jason@jlekstrand.net> Fixes: 88be9a0a ("drm/i915/ttm: add ttm_buddy_man") Reviewed-by:
Matthew Auld <matthew.auld@intel.com> Cc: Matthew Auld <matthew.auld@intel.com> Cc: Christian König <christian.koenig@amd.com> [danvet: Rebase against removal of global shrink code] Signed-off-by:
Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210721152358.2893314-7-jason@jlekstrand.net
-
Jason Ekstrand authored
If the driver was not fully loaded, we may still have globals lying around. If we don't tear those down in i915_exit(), we'll leak a bunch of memory slabs. This can happen two ways: use_kms = false and if we've run mock selftests. In either case, we have an early exit from i915_init which happens after i915_globals_init() and we need to clean up those globals. The mock selftests case is especially sticky. The load isn't entirely a no-op. We actually do quite a bit inside those selftests including allocating a bunch of mock objects and running tests on them. Once all those tests are complete, we exit early from i915_init(). Perviously, i915_init() would return a non-zero error code on failure and a zero error code on success. In the success case, we would get to i915_exit() and check i915_pci_driver.driver.owner to detect if i915_init exited early and do nothing. In the failure case, we would fail i915_init() but there would be no opportunity to clean up globals. The most annoying part is that you don't actually notice the failure as part of the self-tests since leaking a bit of memory, while bad, doesn't result in anything observable from userspace. Instead, the next time we load the driver (usually for next IGT test), i915_globals_init() gets invoked again, we go to allocate a bunch of new memory slabs, those implicitly create debugfs entries, and debugfs warns that we're trying to create directories and files that already exist. Since this all happens as part of the next driver load, it shows up in the dmesg-warn of whatever IGT test ran after the mock selftests. While the obvious thing to do here might be to call i915_globals_exit() after selftests, that's not actually safe. The dma-buf selftests call i915_gem_prime_export which creates a file. We call dma_buf_put() on the resulting dmabuf which calls fput() on the file. However, fput() isn't immediate and gets flushed right before syscall returns. This means that all the fput()s from the selftests don't happen until right before the module load syscall used to fire off the selftests returns which is after i915_init(). If we call i915_globals_exit() in i915_init() after selftests, we end up freeing slabs out from under objects which won't get released until fput() is flushed at the end of the module load syscall. The solution here is to let i915_init() return success early and detect the early success in i915_exit() and only tear down globals and nothing else. This way the module loads successfully, regardless of the success or failure of the tests. Because we've not enumerated any PCI devices, no device nodes are created and it's entirely useless from userspace. The only thing the module does at that point is hold on to a bit of memory until we unload it and i915_exit() is called. Importantly, this means that everything from our selftests has the ability to properly flush out between i915_init() and i915_exit() because there is at least one syscall boundary in between. In order to handle all the delicate init/exit cases, we convert the whole thing to a table of init/exit pairs and track the init status in the new init_progress global. This allows us to ensure that i915_exit() always tears down exactly the things that i915_init() successfully initialized. We also allow early-exit of i915_init() without failure by an init function returning > 0. This is useful for nomodeset, and selftests. For the mock selftests, we convert them to always return 1 so we get the desired behavior of the driver always succeeding to load the driver and then properly tearing down the partially loaded driver. v2 (Tvrtko Ursulin): - Guard init_funcs[i].exit with GEM_BUG_ON(i >= ARRAY_SIZE(init_funcs)) v2 (Daniel Vetter): - Update the docstring for i915.mock_selftests Signed-off-by:
-
Jason Ekstrand authored
In the unlikely event that pci_register_device() fails, we were tearing down our PMU setup but not globals. This leaves a bunch of memory slabs lying around. Signed-off-by:
-
Jason Ekstrand authored
We should tear down in the opposite order we set up. Signed-off-by:
Tvrtko Ursulin <tvrtko.ursulin@intel.com> Signed-off-by:
-
Daniel Vetter authored
This essentially reverts commit 84a10749 Author: Chris Wilson <chris@chris-wilson.co.uk> Date: Wed Jan 24 11:36:08 2018 +0000 drm/i915: Shrink the GEM kmem_caches upon idling mm/vmscan.c:do_shrink_slab() is a thing, if there's an issue with it then we need to fix that there, not hand-roll our own slab shrinking code in i915. Also when this was added there was only one other caller of kmem_cache_shrink (added 2005 to the acpi code). Now there's a 2nd one outside of i915 code in a kunit test, which seems legit since that wants to very carefully control what's in the kmem_cache. This out of a total of over 500 calls to kmem_cache_create. This alone should have been warning sign enough that we're doing something silly. Noticed while reviewing a patch set from Jason to fix up some issues in our i915_init() and i915_exit() module load/cleanup code. Now that i915_globals.c isn't any different than normal init/exit functions, we should convert them over to one unified table and remove i915_globals.[hc] entirely. v2: Improve commit message (Jason) Reviewed-by:
Daniel Vetter <daniel.vetter@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20210721183229.4136488-1-daniel.vetter@ffwll.ch
-
- 21 Jul, 2021 6 commits
-
-
Matt Roper authored
Workarounds are documented in the bspec with an exclusive upper bound (i.e., a "fixed" stepping that no longer needs the workaround). This makes our driver's use of an inclusive upper bound for stepping ranges confusing; the differing notation between code and bspec makes it very easy for mistakes to creep in. Let's switch the upper bound of our IS_{GT,DISP}_STEP macros over to use an exclusive upper bound like the bspec does. This also has the benefit of helping make sure workarounds are properly handled for new minor steppings that show up (e.g., an A1 between the A0 and B0 we already knew about) --- if the new intermediate stepping pulls in hardware fixes early, there will be an update to the workaround definition which lets us know we need to change our code. If the new stepping does not pull a hardware fix earlier, then the new stepping will already be captured properly by the "[begin, fix)" range in the code. We'll probably need to be extra vigilant in code review of new workarounds for the near future to make sure developers notice the new semantics of workaround bounds. But we just migrated a bunch of our platforms from the IS_REVID bounds over to IS_{GT,DISP}_STEP, so people are already adjusting to the new macros and now is a good time to make this change too. [mattrope: Split out GT changes to apply through gt-next tree] Signed-off-by: -
Matt Roper authored
DFR programming (which we enable as an optimization on gen11, but must ensure is disabled on gen12) should be handled as a GT workaround rather than clock gating initialization. This will ensure that the programming of these registers is verified with our typical workaround checks. Signed-off-by:
-
Matt Roper authored
While doing a quick sanity check of the ICL workarounds in the driver I noticed a few things that should be updated: * There's no mention in the bspec that WaPipelineFlushCoherentLines is needed on gen11 (both the current WA database and the old, deprecated page 20196 were checked); it appears this might have just been copied from the gen9 list? Even if this were needed, it doesn't seem like this was the correct implementation anyway since the gen9 workaround is supposed to be implemented in the indirect context bb (as we do in gen8_emit_flush_coherentl3_wa() on gen8/gen9). * WaForwardProgressSoftReset does not appear in the current workaround database. The old deprecated workaround list has a note indicating the workaround was dropped in 2017, so we should be safe to drop it from the code too. While we're at it, add the formal workaround ID number to WaDisableBankHangMode (our hardware team made a transition from text-based workaround names to ID numbers partway through the development of ICL, which is why some workarounds only have names, some only have numbers, and some have both). Bspec: 33450 Signed-off-by:
-
Matt Roper authored
On SKL we've been applying this workaround on H0+ steppings, which is actually backwards; H0 is supposed to be the first stepping where the workaround is no longer needed. Flip the bounds so that the workaround applies to all steppings _before_ H0. On BXT we've been applying this workaround to all steppings, but the bspec tells us it's only needed until C0. Pre-C0 GT steppings only appeared in pre-production hardware, which we no longer support in the driver, so we can drop the workaround completely for this platform. On ICL we've been applying this workaround to all steppings, but there doesn't seem to be any indication that this workaround was ever needed for this platform (even now-deprecated page 20196 of the bspec doesn't mention it). We can go ahead and drop it. I also don't see any mention of this workaround being needed for KBL, although this may be an oversight since the workaround is needed for all steppings of CFL. I'll leave the workaround in place for KBL to be safe. Bspec: 14091, 33450 Signed-off-by:
-
Maarten Lankhorst authored
The FIXED mapping is only used for ttm, and tells userspace that the mapping type is pre-defined. This disables the other type of mmap offsets when discrete memory is used, so fix the selftests as well. Document the struct as well, so it shows up in docbook. Cc: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by:
Maarten Lankhorst <maarten.lankhorst@linux.intel.com> [mauld: Included minor fixes from the review comments] Signed-off-by:
-
Jason Ekstrand authored
In 93b71330 ("drm/i915: Revert "drm/i915/gem: Asynchronous cmdparser""), the parameters to intel_engine_cmd_parser() were altered without updating the docs, causing Fi.CI.DOCS to start failing. Fixes: 93b71330 ("drm/i915: Revert "drm/i915/gem: Asynchronous cmdparser"") Signed-off-by:
Rodrigo Vivi <rodrigo.vivi@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20210720182108.2761496-1-jason@jlekstrand.netSigned-off-by:
-
- 20 Jul, 2021 2 commits
-
-
Matthew Auld authored
Add the missing kernel-doc. Signed-off-by:
-
Matthew Auld authored
It's a noop on DG1, and in the future when need to support other devices which let us control the coherency, then it should be an immutable creation time property for the BO. This will likely be controlled through a new gem_create_ext extension. v2: add some kernel doc for the discrete changes, and document the implicit rules Suggested-by:Daniel Vetter <daniel@ffwll.ch> Signed-off-by:
Kenneth Graunke <kenneth@whitecape.org> Link: https://patchwork.freedesktop.org/patch/msgid/20210715101536.2606307-2-matthew.auld@intel.com
-
- 16 Jul, 2021 6 commits
-
-
Jason Ekstrand authored
This reverts a6c5e2ae ("drm/i915: Skip over MI_NOOP when parsing"). It complicates the batch parsing code a bit and increases indentation for no reason other than fast-skipping a command that userspace uses only rarely. Sure, there may be IGT tests that fill batches with NOOPs but that's not a case we should optimize for in the kernel. We should optimize for code clarity instead. Signed-off-by:
Jon Bloomfield <jon.bloomfield@intel.com> Acked-by:
-
Jason Ekstrand authored
Asynchronous command parsing was the only thing which ever returned a non-zero error. With that gone, we can drop the error handling from dma_fence_work. Signed-off-by:
-
Jason Ekstrand authored
This reverts the rest of 0edbb9ba ("drm/i915: Move cmd parser pinning to execbuffer"). Now that the only user of i915_gem_object_get_sg without allow_alloc has been removed, we can drop the parameter. This portion of the revert was broken into its own patch to aid review. Signed-off-by:
-
Jason Ekstrand authored
This reverts commit 9e31c1fe. Ever since that commit, we've been having issues where a hang in one client can propagate to another. In particular, a hang in an app can propagate to the X server which causes the whole desktop to lock up. Error propagation along fences sound like a good idea, but as your bug shows, surprising consequences, since propagating errors across security boundaries is not a good thing. What we do have is track the hangs on the ctx, and report information to userspace using RESET_STATS. That's how arb_robustness works. Also, if my understanding is still correct, the EIO from execbuf is when your context is banned (because not recoverable or too many hangs). And in all these cases it's up to userspace to figure out what is all impacted and should be reported to the application, that's not on the kernel to guess and automatically propagate. What's more, we're also building more features on top of ctx error reporting with RESET_STATS ioctl: Encrypted buffers use the same, and the userspace fence wait also relies on that mechanism. So it is the path going forward for reporting gpu hangs and resets to userspace. So all together that's why I think we should just bury this idea again as not quite the direction we want to go to, hence why I think the revert is the right option here. For backporters: Please note that you _must_ have a backport of https://lore.kernel.org/dri-devel/20210602164149.391653-2-jason@jlekstrand.net/ for otherwise backporting just this patch opens up a security bug. v2: Augment commit message. Also restore Jason's sob that I accidentally lost. v3: Add a note for backporters Signed-off-by:
Marcin Slusarz <marcin.slusarz@intel.com> Cc: <stable@vger.kernel.org> # v5.6+ Cc: Jason Ekstrand <jason.ekstrand@intel.com> Cc: Marcin Slusarz <marcin.slusarz@intel.com> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/3080 Fixes: 9e31c1fe ("drm/i915: Propagate errors on awaiting already signaled fences") Acked-by:
-
Jason Ekstrand authored
This reverts 686c7c35 ("drm/i915/gem: Asynchronous cmdparser"). The justification for this commit in the git history was a vague comment about getting it out from under the struct_mutex. While this may improve perf for some workloads on Gen7 platforms where we rely on the command parser for features such as indirect rendering, no numbers were provided to prove such an improvement. It claims to closed two gitlab/bugzilla issues but with no explanation whatsoever as to why or what bug it's fixing. Meanwhile, by moving command parsing off to an async callback, it leaves us with a problem of what to do on error. When things were synchronous, EXECBUFFER2 would fail with an error code if parsing failed. When moving it to async, we needed another way to handle that error and the solution employed was to set an error on the dma_fence and then trust that said error gets propagated to the client eventually. Moving back to synchronous will help us untangle the fence error propagation mess. This also reverts most of 0edbb9ba ("drm/i915: Move cmd parser pinning to execbuffer") which is a refactor of some of our allocation paths for asynchronous parsing. Now that everything is synchronous, we don't need it. v2 (Daniel Vetter): - Add stabel Cc and Fixes tag Signed-off-by:
-
Maarten Lankhorst authored
I noticed when grepping for DOC: that those were defined in the header, but not actually used. Fix it by removing all chapters and the internal annotation, so the docbook generated chapters are used. This reverts the changes to driver-uapi.rst by the referenced commit 57772953 ("drm/i915: Document the Virtual Engine uAPI") Signed-off-by:
-
