- 20 3月, 2014 1 次提交
-
-
由 Chris Wilson 提交于
The idea of printing objects used by each process is to judge how each process is using them. This means that we need to evaluate whether the object is bound for that particular process, rather than just whether it is bound into the global GTT. v2: Restore the non-full-ppgtt path for simplicity as we may not even create vma with older hardware. v3: Tweak handling of global entries and default context entries. Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk> Cc: Ben Widawsky <benjamin.widawsky@intel.com> Reviewed-by: NBen Widawsky <ben@bwidawsk.net> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
- 18 3月, 2014 1 次提交
-
-
由 Mika Kuoppala 提交于
We used to have per file descriptor hang stats for the i915_get_reset_stats_ioctl() and for default context banning. 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 made having separate hangstats in file_private redundant as i915_hw_context already contained hangstats. So commit c482972a Author: Ben Widawsky <benjamin.widawsky@intel.com> Date: Fri Dec 6 14:11:20 2013 -0800 drm/i915: Piggy back hangstats off of contexts consolidated the hangstats and enabled further improvements. commit 44e2c070 Author: Mika Kuoppala <mika.kuoppala@linux.intel.com> Date: Thu Jan 30 16:01:15 2014 +0200 drm/i915: Use i915_hw_context to set reset stats tried to reap full benefits of consolidation but fell short as we never 'switch' to the fake private context on gens that don't have hw_contexts, so request->ctx remained NULL on those. Fix this by 'switching' to fake context so that when request is submitted to ring, proper context gets assigned to it. Testcase: igt/drv_hangman Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76055Signed-off-by: NMika Kuoppala <mika.kuoppala@intel.com> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
- 06 3月, 2014 1 次提交
-
-
由 Damien Lespiau 提交于
While reading some code, out of boredom, stumbled on a tiny tiny fix. Signed-off-by: NDamien Lespiau <damien.lespiau@intel.com> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
- 04 3月, 2014 3 次提交
-
-
由 Ben Widawsky 提交于
GEN8 never freed the PPGTT struct. As GEN8 doesn't use full PPGTT, the leak is small and only found on a module reload. ie. I don't think this needs to go to stable. v2: The very naive, kfree in gen8 ppgtt cleanup, is subject to a double free on PPGTT initialization failure. (Spotted by Imre). Instead this patch pulls the ppgtt struct freeing out of the cleanup and leaves it to the allocators/callers or the one doing the last kref_put as in standard convention Reported-by: NVille Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: NBen Widawsky <ben@bwidawsk.net> Reviewed-by: NImre Deak <imre.deak@intel.com> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
由 Ben Widawsky 提交于
At one time it was expected to be called in multiple places by kref_put. At the current time however, it is all contained within i915_gem_context.c. This patch makes an upcoming required addition a bit nicer since it too doesn't need to be defined in a header file. Signed-off-by: NBen Widawsky <ben@bwidawsk.net> Reviewed-by: NImre Deak <imre.deak@intel.com> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
由 Thierry Reding 提交于
The i915 driver sets DRIVER_GEM unconditionally, so testing for the feature will always fail. Signed-off-by: NThierry Reding <treding@nvidia.com> [danvet: Fix up conflicts.] Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
- 14 2月, 2014 1 次提交
-
-
由 Daniel Vetter 提交于
Anything more than just one bool parameter is just a pain to read, symbolic constants are much better. Split out from Chris' vma-binding rework patch. v2: Undo the behaviour change in object_pin that Chris spotted. v3: Split out misplaced hunk to handle set_cache_level errors, spotted by Jani. v4: Keep the current over-zealous binding logic in the execbuffer code working with a quick hack while the overall binding code gets shuffled around. v5: Reorder the PIN_ flags for more natural patch splitup. v6: Pull out the PIN_GLOBAL split-up again. Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Ben Widawsky <benjamin.widawsky@intel.com> Reviewed-by: NChris Wilson <chris@chris-wilson.co.uk> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
- 04 2月, 2014 1 次提交
-
-
由 Mika Kuoppala 提交于
Found with smatch Signed-off-by: NMika Kuoppala <mika.kuoppala@intel.com> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
- 31 1月, 2014 1 次提交
-
-
由 Mika Kuoppala 提交于
If we have stopped rings then we know that test is running so no need for spam. In addition, only spam when default context gets banned. v2: - make sure default context ban gets shown (Chris) - use helper for checking for default context, everywhere (Chris) v3: - dont be quiet when debug is set (Ben, Daniel) Reference: https://bugs.freedesktop.org/show_bug.cgi?id=73652Signed-off-by: NMika Kuoppala <mika.kuoppala@intel.com> Reviewed-by: NBen Widawsky <ben@bwidawsk.net> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
- 28 1月, 2014 3 次提交
-
-
由 Ben Widawsky 提交于
There are cases where we want to know if there is a full, or aliased PPGTT. Currently, in fact the only distinction we ever need to make is when we're using full PPGTT. This patch is simply to promote readability and clarify for the confusing existing usage where "aliasing" meant aliasing and full. v2: Remove USES_ALIASING_PPGTT since there are currently no cases where we need to check if we're using aliasing, but not full PPGTT. (Daniel) Cc: Daniel Vetter <daniel@ffwll.ch> Signed-off-by: NBen Widawsky <ben@bwidawsk.net> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
由 Ville Syrjälä 提交于
WaMiSetContext_Hang tells us that a MI_NOOP must follow MI_SET_CONTEXT. The other thing WaMiSetContext_Hang seems to say is that URB_FENCE isn't allowed to straddle two cachelines. But we don't issue those from the kernel so we don't care. Signed-off-by: NVille Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by: NRodrigo Vivi <rodrigo.vivi@gmail.com> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
由 Chris Wilson 提交于
Through a twisty and circuituous path it is possible to currently trick the code into creating a default context and forgetting to pin it immediately into the GGTT. (This requires a system using contexts without an aliasing ppgtt, which is currently restricted to Baytrails machines manually specifying a module parameter to force enable contexts, or on Sandybridge and later that manually disable the aliasing ppgtt.) The consequence is that during module unload we attempt to unpin the default context twice and encounter a BUG remonstrating that we attempt to unpin an unbound object. [ 161.002869] Kernel BUG at f84861f8 [verbose debug info unavailable] [ 161.002875] invalid opcode: 0000 [#1] SMP [ 161.002882] Modules linked in: coretemp kvm_intel kvm crc32_pclmul aesni_intel aes_i586 xts lrw gf128mul ablk_helper cryptd hid_sensor_accel_3d hid_sensor_gyro_3d hid_sensor_magn_3d hid_sensor_trigger industrialio_triggered_buffer kfifo_buf industrialio hid_sensor_iio_common snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_intel snd_hda_codec snd_hwdep snd_pcm snd_page_alloc snd_seq_midi snd_seq_midi_event dm_multipath scsi_dh asix ppdev usbnet snd_rawmidi mii hid_sensor_hub microcode snd_seq rfcomm bnep snd_seq_device bluetooth snd_timer snd parport_pc binfmt_misc soundcore dw_dmac_pci dw_dmac_core mac_hid lp parport dm_mirror dm_region_hash dm_log hid_generic usbhid hid i915(O-) drm_kms_helper(O) igb dca ptp pps_core i2c_algo_bit drm(O) ahci libahci video [ 161.002991] CPU: 0 PID: 2114 Comm: rmmod Tainted: G W O 3.13.0-rc8+ #2 [ 161.002997] Hardware name: NEXCOM VTC1010/Aptio CRB, BIOS 5.6.5 09/24/2013 [ 161.003004] task: dbdd6800 ti: dbe0e000 task.ti: dbe0e000 [ 161.003010] EIP: 0060:[<f84861f8>] EFLAGS: 00010246 CPU: 0 [ 161.003044] EIP is at i915_gem_object_ggtt_unpin+0x88/0x90 [i915] [ 161.003050] EAX: dfce3840 EBX: 00000000 ECX: dfafd690 EDX: dfce3874 [ 161.003056] ESI: c0086b40 EDI: df962e00 EBP: dbe0fe1c ESP: dbe0fe0c [ 161.003062] DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068 [ 161.003068] CR0: 8005003b CR2: b7718000 CR3: 1bec0000 CR4: 001007f0 [ 161.003076] Stack: [ 161.003081] 00afc014 00000004 c0086b40 dfafc000 dbe0fe38 f8487e5a dfaa5400 c0086b40 [ 161.003099] dfafc000 dfaa5400 dfaa5414 dbe0fe58 f84741aa 00000000 f89c34b9 dfaa5414 [ 161.003117] dfaa5400 dfaa5400 f644b000 dbe0fe6c f89a5443 dfaa5400 f8505000 f644b000 [ 161.003134] Call Trace: [ 161.003169] [<f8487e5a>] i915_gem_context_fini+0xba/0x1c0 [i915] [ 161.003202] [<f84741aa>] i915_driver_unload+0x1fa/0x2f0 [i915] [ 161.003232] [<f89a5443>] drm_dev_unregister+0x23/0x90 [drm] [ 161.003259] [<f89a54ed>] drm_put_dev+0x3d/0x70 [drm] [ 161.003294] [<f8470615>] i915_pci_remove+0x15/0x20 [i915] [ 161.003306] [<c1338a6f>] pci_device_remove+0x2f/0xa0 [ 161.003317] [<c140c871>] __device_release_driver+0x61/0xc0 [ 161.003328] [<c140d12f>] driver_detach+0x8f/0xa0 [ 161.003341] [<c140c54f>] bus_remove_driver+0x4f/0xc0 [ 161.003353] [<c140d708>] driver_unregister+0x28/0x60 [ 161.003362] [<c10cee42>] ? stop_cpus+0x32/0x40 [ 161.003372] [<c10bd510>] ? module_refcount+0x90/0x90 [ 161.003383] [<c13378c5>] pci_unregister_driver+0x15/0x60 [ 161.003413] [<f89a739f>] drm_pci_exit+0x9f/0xb0 [drm] [ 161.003458] [<f84e624a>] i915_exit+0x1b/0x1d [i915] [ 161.003468] [<c10bf8a8>] SyS_delete_module+0x158/0x1f0 [ 161.003480] [<c1173d5d>] ? ____fput+0xd/0x10 [ 161.003488] [<c106f0fe>] ? task_work_run+0x7e/0xb0 [ 161.003499] [<c165a68d>] sysenter_do_call+0x12/0x28 [ 161.003505] Code: 0f b6 4d f3 8d 51 0f 83 e1 f0 83 e2 0f 09 d1 84 d2 88 48 54 75 07 80 a7 91 00 00 00 7f 83 c4 04 5b 5e 5f 5d c3 8d b6 00 00 00 00 <0f> 0b 8d b6 00 00 00 00 55 89 e5 57 56 53 83 ec 64 3e 8d 74 26 [ 161.003586] EIP: [<f84861f8>] i915_gem_object_ggtt_unpin+0x88/0x90 [i915] SS:ESP 0068:dbe0fe0c v2: Rename the local variable (is_default_ctx) to avoid confusion with the function is_default_ctx(). And correct Jesse's email address. Reported-by: NJesse Barnes <jbarnes@virtuousgeek.org> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=73985Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk> Cc: Jesse Barnes <jbarnes@virtuousgeek.org> Cc: Ben Widawsky <benjamin.widawsky@intel.com> Tested-by: NJesse Barnes <jbarnes@virtuousgeek.org> Reviewed-by: NBen Widawsky <benjamin.widawsky@intel.com> [danvet: Fix up the rebase fail from my first attempt, thankfully pointed out by Ville.] Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
- 10 1月, 2014 2 次提交
-
-
由 Ben Widawsky 提交于
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: NBen Widawsky <ben@bwidawsk.net> Reviewed-by: NChris Wilson <chris@chris-wilson.co.uk> [danvet: Simplify the failure scenario in the commit message according to Chris' review a bit.] Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
由 Ben Widawsky 提交于
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: NBen Widawsky <ben@bwidawsk.net> Reviewed-by: NChris Wilson <chris@chris-wilson.co.uk> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
- 07 1月, 2014 1 次提交
-
-
由 Ben Widawsky 提交于
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: NBen Widawsky <ben@bwidawsk.net> Testcase: igt/gem_ctx_exec/basic [danvet: Fix up the commit message and actually bother to mention the testcase this fixes.] Reviewed-by: NMika Kuoppala <mika.kuoppala@intel.com> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
- 18 12月, 2013 15 次提交
-
-
由 Ben Widawsky 提交于
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: NBen Widawsky <ben@bwidawsk.net> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
由 Ben Widawsky 提交于
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: NBen Widawsky <ben@bwidawsk.net> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
由 Ben Widawsky 提交于
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: NBen Widawsky <ben@bwidawsk.net> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
由 Ben Widawsky 提交于
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: NBen Widawsky <ben@bwidawsk.net> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
由 Ben Widawsky 提交于
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: NBen Widawsky <ben@bwidawsk.net> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
由 Ben Widawsky 提交于
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: NBen Widawsky <ben@bwidawsk.net> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
由 Ben Widawsky 提交于
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: NBen Widawsky <ben@bwidawsk.net> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
由 Ben Widawsky 提交于
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: NBen Widawsky <ben@bwidawsk.net> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
由 Ben Widawsky 提交于
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: NBen Widawsky <ben@bwidawsk.net> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
由 Ben Widawsky 提交于
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: NBen Widawsky <ben@bwidawsk.net> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
由 Ben Widawsky 提交于
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: NBen Widawsky <ben@bwidawsk.net> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
由 Ben Widawsky 提交于
With the introduction of contexts per fd in the future, one can easily envision more contexts being used. We do not have an easy remedy to reduce the space requirements of the contexts, we can make things slightly better by using less stringent alignments on later hardware. Ville: Since I can almost predict you'll point this out. I can no longer find the docs which specify the 64k requirement on certain gen6 SKUs. If you'd like to change that too, be my guest. CC: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: NBen Widawsky <ben@bwidawsk.net> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
由 Ben Widawsky 提交于
We'll be doing a bit more stuff with each file, so having our own open function should make things clean. This also allows us to easily add conditionals for stuff we don't want to do when we don't have HW contexts. Signed-off-by: NBen Widawsky <ben@bwidawsk.net> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
由 Ben Widawsky 提交于
To sum up what goes on here, we abstract the vma binding, similarly to the previous object binding. This helps for distinguishing legacy binding, versus modern binding. To keep the code churn as minimal as possible, I am leaving in insert_entries(). It serves as the per platform pte writing basically. bind_vma and insert_entries do share a lot of similarities, and I did have designs to combine the two, but as mentioned already... too much churn in an already massive patchset. What follows are the 3 commits which existed discretely in the original submissions. Upon rebasing on Broadwell support, it became clear that separation was not good, and only made for more error prone code. Below are the 3 commit messages with all their history. drm/i915: Add bind/unbind object functions to VMA drm/i915: Use the new vm [un]bind functions drm/i915: reduce vm->insert_entries() usage drm/i915: Add bind/unbind object functions to VMA As we plumb the code with more VM information, it has become more obvious that the easiest way to deal with bind and unbind is to simply put the function pointers in the vm, and let those choose the correct way to handle the page table updates. This change allows many places in the code to simply be vm->bind, and not have to worry about distinguishing PPGTT vs GGTT. Notice that this patch has no impact on functionality. I've decided to save the actual change until the next patch because I think it's easier to review that way. I'm happy to squash the two, or let Daniel do it on merge. v2: Make ggtt handle the quirky aliasing ppgtt Add flags to bind object to support above Don't ever call bind/unbind directly for PPGTT until we have real, full PPGTT (use NULLs to assert this) Make sure we rebind the ggtt if there already is a ggtt binding. This happens on set cache levels. Use VMA for bind/unbind (Daniel, Ben) v3: Reorganize ggtt_vma_bind to be more concise and easier to read (Ville). Change logic in unbind to only unbind ggtt when there is a global mapping, and to remove a redundant check if the aliasing ppgtt exists. v4: Make the bind function a bit smarter about the cache levels to avoid unnecessary multiple remaps. "I accept it is a wart, I think unifying the pin_vma / bind_vma could be unified later" (Chris) Removed the git notes, and put version info here. (Daniel) v5: Update the comment to not suck (Chris) v6: Move bind/unbind to the VMA. It makes more sense in the VMA structure (always has, but I was previously lazy). With this change, it will allow us to keep a distinct insert_entries. Reviewed-by: NChris Wilson <chris@chris-wilson.co.uk> Signed-off-by: NBen Widawsky <ben@bwidawsk.net> drm/i915: Use the new vm [un]bind functions Building on the last patch which created the new function pointers in the VM for bind/unbind, here we actually put those new function pointers to use. Split out as a separate patch to aid in review. I'm fine with squashing into the previous patch if people request it. v2: Updated to address the smart ggtt which can do aliasing as needed Make sure we bind to global gtt when mappable and fenceable. I thought we could get away without this initialy, but we cannot. v3: Make the global GTT binding explicitly use the ggtt VM for bind_vma(). While at it, use the new ggtt_vma helper (Chris) At this point the original mailing list thread diverges. ie. v4^: use target_obj instead of obj for gen6 relocate_entry vma->bind_vma() can be called safely during pin. So simply do that instead of the complicated conditionals. Don't restore PPGTT bound objects on resume path Bug fix in resume path for globally bound Bos Properly handle secure dispatch Rebased on vma bind/unbind conversion Signed-off-by: NBen Widawsky <ben@bwidawsk.net> drm/i915: reduce vm->insert_entries() usage FKA: drm/i915: eliminate vm->insert_entries() With bind/unbind function pointers in place, we no longer need insert_entries. We could, and want, to remove clear_range, however it's not totally easy at this point. Since it's used in a couple of place still that don't only deal in objects: setup, ppgtt init, and restore gtt mappings. v2: Don't actually remove insert_entries, just limit its usage. It will be useful when we introduce gen8. It will always be called from the vma bind/unbind. Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> (v1) Signed-off-by: NBen Widawsky <ben@bwidawsk.net> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
由 Ben Widawsky 提交于
Signed-off-by: NBen Widawsky <ben@bwidawsk.net> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
- 06 12月, 2013 1 次提交
-
-
由 Daniel Vetter 提交于
So apparently under ridiculous amounts of memory pressure we can get into trouble in do_switch when we try to move the old hw context backing storage object onto the active lists. With list debugging enabled that usually results in us chasing a poisoned pointer - which means we've hit upon a vma that has been removed from all lrus with list_del (and then deallocated, so it's a real use-after free). Ian Lister has done some great callchain chasing and noticed that we can reenter do_switch: i915_gem_do_execbuffer() i915_switch_context() do_switch() from = ring->last_context; i915_gem_object_pin() i915_gem_object_bind_to_gtt() ret = drm_mm_insert_node_in_range_generic(); // If the above call fails then it will try i915_gem_evict_something() // If that fails it will call i915_gem_evict_everything() ... i915_gem_evict_everything() i915_gpu_idle() i915_switch_context(DEFAULT_CONTEXT) Like with everything else where the shrinker or eviction code can invalidate pointers we need to reload relevant state. Note that there's no need to recheck whether a context switch is still required because: - Doing a switch to the same context is harmless (besides wasting a bit of energy). - This can only happen with the default context. But since that one's pinned we'll never call down into evict_everything under normal circumstances. Note that there's a little driver bringup fun involved namely that we could recourse into do_switch for the initial switch. Atm we're fine since we assign the context pointer only after the call to do_switch at driver load or resume time. And in the gpu reset case we skip the entire setup sequence (which might be a bug on its own, but definitely not this one here). Cc'ing stable since apparently ChromeOS guys are seeing this in the wild (and not just on artificial stress tests), see the reference. Note that in upstream code doesn't calle evict_everything directly from evict_something, that's an extension in this product branch. But we can still hit upon this bug (and apparently we do, see the linked backtraces). I've noticed this while trying to construct a testcase for this bug and utterly failed to provoke it. It looks like we need to driver the system squarly into the lowmem wall and provoke the shrinker to evict the context object by doing the last-ditch evict_everything call. Aside: There's currently no means to get a badly-fragmenting hw context object away from a bad spot in the upstream code. We should fix this by at least adding some code to evict_something to handle hw contexts. References: https://code.google.com/p/chromium/issues/detail?id=248191Reported-by: NIan Lister <ian.lister@intel.com> Cc: Ian Lister <ian.lister@intel.com> Cc: stable@vger.kernel.org Cc: Ben Widawsky <benjamin.widawsky@intel.com> Cc: Stéphane Marchesin <marcheu@chromium.org> Cc: Bloomfield, Jon <jon.bloomfield@intel.com> Tested-by: NRafael Barbalho <rafael.barbalho@intel.com> Reviewed-by: NIan Lister <ian.lister@intel.com> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
- 04 12月, 2013 1 次提交
-
-
由 Chris Wilson 提交于
Inorder to serialise the closing of the file descriptor and its subsequent release of client requests with i915_gem_free_request(), we need to hold the struct_mutex in i915_gem_release(). Failing to do so has the potential to trigger an OOPS, later with a use-after-free. Testcase: igt/gem_close_race Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=70874 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=71029Reported-by: NEric Anholt <eric@anholt.net> Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk> Cc: stable@vger.kernel.org Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
- 26 11月, 2013 1 次提交
-
-
由 Ben Widawsky 提交于
Signed-off-by: NBen Widawsky <ben@bwidawsk.net> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
- 09 11月, 2013 1 次提交
-
-
由 Ben Widawsky 提交于
BDW context sizes varies a bit. v2: Squash in fixup for the hw context size from Ben. Signed-off-by: Ben Widawsky <ben@bwidawsk.net> (v1) Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
- 07 11月, 2013 1 次提交
-
-
由 Ben Widawsky 提交于
v2: Fixed the botched locking on init_hw failure in i915_reset (Ville) Call cleanup_ringbuffer on failed context create in init_hw (Ville) v3: Add dev argument ti clean_ringbuffer Reviewed-by: NKenneth Graunke <kenneth@whitecape.org> Signed-off-by: NBen Widawsky <ben@bwidawsk.net> Signed-off-by: NRodrigo Vivi <rodrigo.vivi@gmail.com> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
- 16 10月, 2013 1 次提交
-
-
由 Ben Widawsky 提交于
I had this lying around from he original PPGTT series, and thought we might try to get it in by itself. With the introduction of context refcounting we never explicitly ref/unref the backing object. As such, the previous fix was a bit wonky. Aside from fixing the above, this patch also puts us in good shape for an upcoming patch which allows a failure to occur in between context_init and the first do_switch. CC: Mika Kuoppala <mika.kuoppala@linux.intel.com> Signed-off-by: NBen Widawsky <ben@bwidawsk.net> Reviewed-by: NChris Wilson <chris@chris-wilson.co.uk> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
- 01 10月, 2013 1 次提交
-
-
由 Ben Widawsky 提交于
Even though we track object activity and not VMA, because we have the active_list be based on the VM, it makes the most sense to use VMAs in the APIs. NOTE: Daniel intends to eventually rip out active/inactive LRUs, but for now, leave them be. v2: Remove leftover hunk from the previous patch which didn't keep i915_gem_object_move_to_active. That patch had to rely on the ring to get the dev instead of the obj. (Chris) Reviewed-by: NChris Wilson <chris@chris-wilson.co.uk> Signed-off-by: NBen Widawsky <ben@bwidawsk.net> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
- 20 9月, 2013 2 次提交
-
-
由 Ben Widawsky 提交于
On both Ivybridge and Haswell, row remapping information is saved and restored with context. This means, we never actually properly supported the l3 remapping because our sysfs interface is asynchronous (and not tied to any context), and the known faulty HW would be reused by the next context to run. Not that due to the asynchronous nature of the sysfs entry, there is no point modifying the registers for the existing context. Instead we set a flag for all contexts to load the correct remapping information on the next run. Interested clients can use debugfs to determine whether or not the row has been remapped. One could propose at this point that we just do the remapping in the kernel. I guess since we have to maintain the sysfs interface anyway, I'm not sure how useful it is, and I do like keeping the policy in userspace; (it wasn't my original decision to make the interface the way it is, so I'm not attached). v2: Force a context switch when we have a remap on the next switch. (Ville) Don't let userspace use the interface with disabled contexts. v3: Don't force a context switch, just let it nop Improper context slice remap initialization, 1<<1 instead of 1<<i, but I rewrote it to avoid a second round of confusion. Error print moved to error path (All Ville) Added a comment on why the slice remap initialization happens. CC: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: NBen Widawsky <ben@bwidawsk.net> Reviewed-by: NVille Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
由 Ben Widawsky 提交于
I have implemented this patch before without creating a separate list (I'm having trouble finding the links, but the messages ids are: <1364942743-6041-2-git-send-email-ben@bwidawsk.net> <1365118914-15753-9-git-send-email-ben@bwidawsk.net>) However, the code is much simpler to just use a list and it makes the code from the next patch a lot more pretty. As you'll see in the next patch, the reason for this is to be able to specify when a context needs to get L3 remapping. More details there. Signed-off-by: NBen Widawsky <ben@bwidawsk.net> Reviewed-by: NVille Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
- 04 9月, 2013 1 次提交
-
-
由 Damien Lespiau 提交于
Signed-off-by: NDamien Lespiau <damien.lespiau@intel.com> Acked-by: NBen Widawsky <ben@bwidawsk.net> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-