- 10 Jan, 2014 3 commits
-
-
Ben Widawsky authored
The initialized flag is used to specify a context has been initialized and it's context is safe to load, ie. the 3d state is setup properly. With full PPGTT, we emit the address space loads during context switch and this currently marks a context as initialized. With full PPGTT patches, if a client first emits a batch to !RCS, then later, RCS, the code will mistake the context as initialized and try to reload an uninitialized context. 1. context 1 blit // context marked as initialized, but isn't 2. context 1 render // loads random state from step 2 It is really easy to hit this with a planned upcoming patch which makes default context reuse possible. NOTE: This should only effect full PPGTT branches, ie. current drm-intel-nightly. Thanks to Chris for helping me track this down. Cc: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by:
Ben Widawsky <ben@bwidawsk.net> Reviewed-by:
Chris Wilson <chris@chris-wilson.co.uk> [danvet: Simplify the failure scenario in the commit message according to Chris' review a bit.] Signed-off-by:
Daniel Vetter <daniel.vetter@ffwll.ch>
-
Ben Widawsky authored
This was an accidental "ABI" change introduced during PPGTT: commit 0eea67eb Author: Ben Widawsky <ben@bwidawsk.net> Date: Fri Dec 6 14:11:19 2013 -0800 drm/i915: Create a per file_priv default context The failure test application actually tests the return type. The other option is to simply change the test. Signed-off-by:
-
Daniel Vetter authored
A regression in the topic/ppgtt branch introduce in commit 87d60b63 Author: Ben Widawsky <ben@bwidawsk.net> Date: Fri Dec 6 14:11:29 2013 -0800 drm/i915: Add PPGTT dumper The issue is that we're missing the definitions for the seq_file functions and hence compilation fails. v2: Just include the right header instead of splattering #ifdefs all over the place (Chris). Cc: Chris Wilson <chris@chris-wilson.co.uk> Reported-by:
kbuild test robot <fengguang.wu@intel.com> Reported-by:
Antti Koskipaa <antti.koskipaa@linux.intel.com> Cc: Ben Widawsky <ben@bwidawsk.net> Reviewed-by:
-
- 07 Jan, 2014 1 commit
-
-
Ben Widawsky authored
Without this fix the ioctls silently succeeded (but actually did nothing). It makes all the code which calls into this function way too confusing. v2: Fix destroy IOCTL as well v3: Clarify the other two callers of i915_gem_context_get() to never check for NULL. (Mika) Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=72903Signed-off-by:
Mika Kuoppala <mika.kuoppala@intel.com> Signed-off-by:
-
- 18 Dec, 2013 36 commits
-
-
Daniel Vetter authored
This means something different and is only relevant for gen6 and the reason why we cant use anything else than aliasing ppgtt there. Note that the currently implemented logic for secure batches is broken: Userspace wants the buffer both in ppgtt (for self-referencing relocations) and in ggtt (for priveledge operations). This is the same issue the command parser is also facing. Unfortunately our coverage for corner-cases of self-referencing batches is spotty. Note that this will break vsync'ed Xv and DRI2 copies. Signed-off-by: -
Daniel Vetter authored
Doesn't make sense. Spotted while fixing an issue Chris noticed in the same area. Signed-off-by: -
Daniel Vetter authored
This reverts commit 4fe9adbc. The patch completely lacks a detailed explanation of what exactly blows up and how, so is insufficiently justified as a band-aid. Otoh the justification as a safety measure against userspace botching up relocations is also fairly weak: If we want real project we need to at least make the gab big enough that the gpu doesn't scribble over more important stuff. With 4k screens that would be 32MB. Also I think this would be much better in conjunction with a (debug) switch to disable our use of the scratch page. Hence revert this. Signed-off-by:
-
Daniel Vetter authored
This reverts the abi-change from commit 67e3d297 Author: Ben Widawsky <ben@bwidawsk.net> Date: Fri Dec 6 14:11:01 2013 -0800 drm/i915: Permit contexts on all rings We don't actually need this, only the internal changes to allow contexts on all rings for the purpose of ppgtt switching are required. And I'm not sure whether this is the right thing to do given some of the hw features in the pipeline. Also, new abi needs userspace patches as a proof-of-need, which is completely lacking here. Signed-off-by:
-
Daniel Vetter authored
At least for now userspace has no business at all to know that we switch address spaces around. For any need it has to know whether hw ppgtt is enabled (e.g. to set bits in MI commands correctly) it can inquire the existing ppgtt param. v2: Avoid ternary operator precedence fail (Chris). Signed-off-by: -
Daniel Vetter authored
Especially with ppgtt this kinda stopped making sense. And if we indeed need this to hack around an issue, we need something that also works for non-root. Signed-off-by: -
Ben Widawsky authored
Signed-off-by:
-
Ben Widawsky authored
Dump the aliasing PPGTT with it. The aliasing PPGTT should actually always be empty. TODO: Broadwell. Since we don't yet use full PPGTT on Broadwell, not having the dumper is okay. Signed-off-by:
-
Ben Widawsky authored
Originally this commit message said: Now that do_switch does the mm switch, and we always enable the aliasing PPGTT, and contexts at the same time, there is no need to continue doing this during PPGTT enabling. Since originally writing the patch however, I introduced the concept of synchronous mm switching (using MMIO). Since this is generally not recommended in the spec (for reasons unknown), I've isolated its usage as much as possible. As such the "extraneous" switch only ever will occur when we have full PPGTT. Signed-off-by:
-
Ben Widawsky authored
As with processes which run on the CPU, the goal of multiple VMs is to provide process isolation. Specific to GEN, there is also the ability to map more objects per process (2GB each instead of 2Gb-2k total). For the most part, all the pipes have been laid, and all we need to do is remove asserts and actually start changing address spaces with the context switch. Since prior to this we've converted the setting of the page tables to a streamed version, this is quite easy. One important thing to point out (since it'd been hotly contested) is that with this patch, every context created will have it's own address space (provided the HW can do it). v2: Disable BDW on rebase NOTE: I tried to make this commit as small as possible. I needed one place where I could "turn everything on" and that is here. It could be split into finer commits, but I didn't really see much point. Cc: Eric Anholt <eric@anholt.net> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by:
-
Daniel Vetter authored
I need the tricky do_switch fix before I can merge the final piece of the ppgtt enabling puzzle. Otherwise the conflict will be a real pain to resolve since the do_switch hunk from -fixes must be placed at the exact right place within a hunk in the next patch. Conflicts: drivers/gpu/drm/i915/i915_gem_context.c drivers/gpu/drm/i915/i915_gem_execbuffer.c drivers/gpu/drm/i915/intel_display.c Signed-off-by: -
Ben Widawsky authored
This is primarily a band aid for an unexplainable error in gem_reloc_vs_gpu/forked-faulting-reloc-thrashing. Essentially as soon as a relocated buffer (which had a non-zero presumed offset) moved to offset 0, something goes bad. Since I have been unable to solve this, and potentially this is a good thing to do anyway, since many things can accidentally write to offset 0, why not? Signed-off-by:
-
Ben Widawsky authored
It's quite common for an object to simply be on the inactive list (and not unbound) when we want to free the context. This of course happens with lazy unbinding. Simply, this is needed when an object isn't fully unbound but we want to free one VMA of the object, for whatever reason. NOTE: The aliasing PPGTT is not a proper VM, so it needs special casing. This addresses the fixup requirement mentioned in: drm/915: Better reset handling for contexts In the flink, and dmabuf case, we can't assert that the object isn't still active. To keep it more generic, just check the vma's link in the object vma list. If we wanted to do a better job, we could track last seqno (and active) per VMA. It was decided not to do this in the last iteration. Unfortunately this means the assertion can miss real bugs when using flink/dmabuf. v2: Use the newer introduced i915_gem_evict_vm(). Note that handling the aliasing PPGTT is special. Signed-off-by:
-
Ben Widawsky authored
With context destruction, we always want to be able to tear down the underlying address space. This is invoked on the last unreference to the context which could happen before we've moved all objects to the inactive list. To enable a clean tear down the address space, make sure to process the request free lastly. Without this change, we cannot guarantee to we don't still have active objects in the VM. As an example of a failing case: CTX-A is created, count=1 CTX-A is used during execbuf does a context switch count = 2 and add_request count = 3 CTX B runs, switches, CTX-A count = 2 CTX-A is destroyed, count = 1 retire requests is called free_request from CTX-A, count = 0 <--- free context with active object As mentioned above, by doing the free request after processing the active list, we can avoid this case. Signed-off-by:
-
Ben Widawsky authored
We need to have the address space when reserving space for the objects. Since the address space and context are tied together, and reserve occurs before context switch (for good reason), we must lookup our context earlier in the process. This leaves some room for optimizations where we no longer need to use ctx_id in certain places. This will be addressed in a subsequent patch. Important tricky bit: Because slow relocations during execbuffer drop struct_mutex Perhaps it would be best to acquire the reference when we get the context, but I'll save that for another day (note I have written the patch before, and I found the changes required to be uglier than this). Note that since we currently access everything via context id, and not the data structure this is fine, though not desirable. The next change attempts to get the context only once via the context ID idr lookup, and as such, the following can happen: CTX-A is created, refcount = 1 CTX-A execbuf, mutex dropped close IOCTL called on CTX-A, refcount = 0 CTX-A resumes in execbuf. v2: Rebased on top of commit b6359918 Author: Mika Kuoppala <mika.kuoppala@linux.intel.com> Date: Wed Oct 30 15:44:16 2013 +0200 drm/i915: add i915_get_reset_stats_ioctl v3: Rebased on top of commit 25b3dfc8 Author: Mika Westerberg <mika.westerberg@linux.intel.com> Date: Tue Nov 12 11:57:30 2013 +0200 Author: Mika Kuoppala <mika.kuoppala@linux.intel.com> Date: Tue Nov 26 16:14:33 2013 +0200 drm/i915: check context reset stats before relocations Signed-off-by:
-
Ben Widawsky authored
To simplify the codepaths somewhat, we can simply always create a context. Contexts already keep hangstat information. This prevents us from having to differentiate at other parts in the code. There is allocation overhead, but it should not be measurable. Signed-off-by:
-
Ben Widawsky authored
Every file will get it's own context, and we use this context instead of the default context. The default context still exists for future shrinker usage as well as reset handling. v2: Updated to address Mika's recent context guilty changes Some more changes around this come up in later patches as well. v3: Use a fake context to avoid allocation for the !HAS_HW_CONTEXT case. I've tried the alternatives. This looks the best to me. Removed hangstat stuff from v2 - for a separate patch Demote failed PPGTT set to DRM_DEBUG_DRIVER since it can now be invoked easily from userspace. Signed-off-by:
-
Ben Widawsky authored
We have a default context which suits the aliasing PPGTT well. Tie them together so it looks like any other context/PPGTT pair. This makes the code cleaner as it won't have to special case aliasing as often. The patch has one slightly tricky part in the default context creation function. In the future (and on aliased setup) we create a new VM for a context (potentially). However, if we have aliasing PPGTT, which occurs at this point in time for all platforms GEN6+, we can simply manage the refcounting to allow things to behave as normal. Now is a good time to recall that the aliasing_ppgtt doesn't have a real VM, it uses the GGTT drm_mm. Signed-off-by:
-
Ben Widawsky authored
In following with the old restore code, we must now restore ever PPGTT's PDEs, since they aren't proper GEM ojbects. v2: Rebased on BDW. Only do restore pdes for gen6 & 7 Signed-off-by:
-
Ben Widawsky authored
We won't be calling enable() for all PPGTTs. We do need to write PDEs for all PPGTTs however. By moving the writing to init (which is called for all PPGTTs) we should accomplish this. ADD NOTE ABOUT PDE restore TODO: Eventually, we should allocate the page tables on demand. v2: Rebased on BDW. Only do PDEs for pre-gen8 Signed-off-by:
-
Ben Widawsky authored
Pretty straightforward so far except for the bit about the refcounting. The PPGTT will potentially be shared amongst multiple contexts. Because contexts themselves have a refcounted lifecycle, the easiest way to manage this will be to refcount the PPGTT. To acheive this, we piggy back off of the existing context refcount, and will increment and decrement the PPGTT refcount with context creation, and destruction. To put it more clearly, if context A, and context B both use PPGTT 0, we can't free the PPGTT until both A, and B are destroyed. Note that because the PPGTT is permanently pinned (for now), it really just matters for the PPGTT destruction, as opposed to making space under memory pressure. Signed-off-by:
-
Ben Widawsky authored
This patch consolidates the way in which we handle the various supported PPGTT by module parameter in addition to what the hardware supports. It strives to make doing the right thing in the code as simple as possible, with the USES_ macros. I've opted to add the full PPGTT argument simply so one can see how I intend to use this function. It will not/cannot be used until later. Signed-off-by:
-
Ben Widawsky authored
Rearrange the initialization code to try to special case the aliasing PPGTT less, and provide usable interfaces for the general case later. Signed-off-by:
-
Ben Widawsky authored
I've found this by accident. The docs don't really come out and say you need to do this. What the docs do tell you is you need to flush the TLBs before you set the PP_DIR_BASE, and that the RCS will invalidate its TLBs upon setting the new PP_DIR_BASE. It makes no such comment about any of the other rings. Empirically, this indeed fixes a really obvious bug whereby the batches being sent to the blitter were not executing (we were executing the HSWP somehow instead). NOTE: This should make no difference with the current code. It only applies when we start using multiple VMs. NOTE2: HSW appears to be immune to this. Signed-off-by:
-
Ben Widawsky authored
The docs seem to suggest this is the appropriate method (though it doesn't say so outright). In other words, we probably should have done this before. We certainly must do this for switching VMs on the fly, since synchronizing the rings to MMIO updates isn't acceptable. v2: Make the reset code actually work for all rings. Note that this was fixed in subsequent commits, but was indeed broken for this commit. Add a posting read to the reset case. It probably should have existed before hand, but since we have no failures; there is no reason to make it a separate commit. Make IS_GEN6 not use the ring because I am seeing crashes when using it. It is a bit of a hack in this patch, it will get fixed up in a couple of patches. Signed-off-by:
-
Ben Widawsky authored
In order to do the full context switch with address space, it's convenient to have a way to switch the address space. We already have this in our code - just pull it out to be called by the context switch code later. v2: Rebased on BDW support. Required adding BDW. Signed-off-by:
-
Ben Widawsky authored
Signed-off-by:
-
Ben Widawsky authored
The patch before this changed the way in which we allocate space for the PPGTT PDEs. It began carving out the PPGTT PDEs (which live in the Global GTT) from the GGTT's drm_mm. Prior to that patch, the PDEs were hidden from the drm_mm, and therefore could never fail to be allocated. In unfortunate cases, the drm_mm may be full when we want to allocate the space. This can technically occur whenever we try to allocate, which happens in two places currently. Practically, it can only really ever happen at GPU reset. Later, when we allocate more PDEs for multiple PPGTTs this will potentially even more useful. Signed-off-by:
-
Ben Widawsky authored
When PPGTT support was originally enabled, it was only designed to support 1 PPGTT. It therefore made sense to simply hide the GGTT space required to enable this from the drm_mm allocator. Since we intend to support full PPGTT, which means more than 1, and they can be created and destroyed ad hoc it will be required to use the proper allocation techniques we already have. The first step here is to make the existing single PPGTT use the allocator. The astute observer will notice that we are reserving space in the GGTT for the PDEs for the lifetime of the address space, and would be right to question whether or not this is a good idea. It does not make a difference with this current patch only the aliasing PPGTT (indeed the PDEs should still be hidden from the shrinker). For the future, we are allocating from top to bottom to avoid using the precious "gtt space" The GGTT space at that point should only be used for scanout, HW contexts, ringbuffers, HWSP, PDEs, and a couple of other small buffers (potentially) used by the kernel. Everything else should be mapped into a PPGTT. To put the consumption in more tangible terms, it takes approximately 4 sets of PDEs to equal one 19x10 framebuffer (with no fancy stride or alignment constraints). 3/4 of the total [average] GGTT can be used for PDEs, and hopefully never touch the 1/4 that the framebuffer needs. The astute, and persistent observer might ask about the page tables which are also pinned for the address space. This waste is unfortunate. We use 2MB of memory per address space. We leave wrapping the PDEs as a real GEM object as a TODO. v2: Align PDEs to 64b in GTT Allocate the node dynamically so we can use drm_mm_put_block Now tested on IGT Allocate node at the top to avoid fragmentation (Chris) v3: Use Chris' top down allocator v4: Embed drm_mm_node into ppgtt struct (Jesse) Remove hunks which didn't belong (Jesse) v5: Don't subtract guard page since we now killed the guard page prior to this patch. (Ben) v6: Rebased and removed guard page stuff. Added a chunk to the commit message Allow adding a context to mappable region v7: Undo v3, so we can make the drm patch last in the series Cc: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org> (v4) Signed-off-by:
-
Ben Widawsky authored
Signed-off-by:
-
Ben Widawsky authored
The plan to to make every file descriptor have a default context. To accommodate this, generalize out default context setup function so it can be used at file open time. Signed-off-by:
-
Ben Widawsky authored
We **need** to do this for exactly 1 reason, because we want to embed a PPGTT into the context, but we don't want to special case the default context. To achieve that, we must be able to initialize contexts after the GTT is setup (so we can allocate and pin the default context's BO), but before the PPGTT and rings are initialized. This is because, currently, context initialization requires ring usage. We don't have rings until after the GTT is setup. If we split the enabling part of context initialization, the part requiring the ringbuffer, we can untangle this, and then later embed the PPGTT Incidentally this allows us to also adhere to the original design of context init/fini in future patches: they were only ever meant to be called at driver load and unload. v2: Move hw_contexts_disabled test in i915_gem_context_enable() (Chris) v3: BUG_ON after checking for disabled contexts. Or else it blows up pre gen6 (Ben) v4: Forward port Modified enable for each ring, since that patch is earlier in the series Dropped ring arg from create_default_context so it can be used by others Signed-off-by:
-
Ben Widawsky authored
This patch adds to changes for contexts on reset: Sets last context to default - this will prevent the context switch happening after a reset. That switch is not possible because the rings are hung during reset and context switch requires reset. This behavior will need to be reworked in the future, but this is what we want for now. In the future, we'll also want to reset the guilty context to uninitialized. We should wait for ARB_Robustness related code to land for that. This is somewhat for paranoia. Because we really don't know what the GPU was doing when it hung, or the state it was in (mid context write, for example), later restoring the context is a bad idea. By setting the flag to not initialized, the next load of that context will not restore the state, and thus on the subsequent switch away from the context will overwrite the old data. NOTE: This code needs a fixup when we actually have multiple VMs. The issue that can occur is inactive objects in a VM will need to be destroyed before the last context unref. This can now happen via the fake switch introduced in this patch (and it other ways in the future) Signed-off-by:
-
Ben Widawsky authored
Previously we dropped the association of a context to a ring. It is however very important to know which ring a context ran on (we could have reused the other member, but I was nitpicky). This is very important when we switch address spaces, which unlike context objects, do change per ring. As an example, if we have: RCS BCS ctx A ctx A ctx B ctx B Without tracking the last ring B ran on, we wouldn't know to switch the address space on BCS in the last row. As a result, we no longer need to track which ring a context "belongs" to, as it never really made much sense anyway. Signed-off-by: -
Ben Widawsky authored
If we want to use contexts in more abstract terms (specifically with PPGTT in mind), we need to allow them to be specified for any ring. Since the upcoming patches will bring about the use of multiple address spaces, and each ring needs to have an address space programmed (which we intend to do at context switch time), we can no longer only use RCS. With multiple rings having a last context, we must now unreference these contexts. NOTE: This commit requires an update to intel-gpu-tools to make it not fail. v2: Rebased with some logical conflicts. Squashed in the context fini refcount patch Signed-off-by:
-
Ben Widawsky authored
Signed-off-by:
-
