- 11 Jun, 2018 14 commits
-
-
Chris Wilson authored
The HW only accepts offsets within ring->size, and fails peculiarly if the RING_HEAD or RING_TAIL is set to ring->size. Therefore whenever we set ring->head/ring->tail we want to make sure it is within value (using intel_ring_wrap()). v2: Double check execlists as well v3: Remove redundancy with assert_ring_tail_valid() v4: Just assert in intel_ring_reset() rather than be over-defensive. Signed-off-by:
Chris Wilson <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Cc: Matthew Auld <matthew.william.auld@gmail.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> #v2 Link: https://patchwork.freedesktop.org/patch/msgid/20180611110845.31890-2-chris@chris-wilson.co.uk
-
Chris Wilson authored
The discovery with trying to enable full-ppgtt was that we were completely failing to the load both the mm and context following the reset. Although we were performing mmio to set the PP_DIR (per-process GTT) and CCID (context), these were taking no effect (the assumption was that this would trigger reload of the context and restore the page tables). It was not until we performed the LRI + MI_SET_CONTEXT in a following context switch would anything occur. Since we are then required to reset the context image and PP_DIR using CS commands, we place those commands into every batch. The hardware should recognise the no-ops and eliminate the expensive context loads, but we still have to pay the cost of using cross-powerwell register writes. In practice, this has no effect on actual context switch times, and only adds a few hundred nanoseconds to no-op switches. We can improve the latter by eliminating the w/a around known no-op switches, but there is an ulterior motive to keeping them. Always emitting the context switch at the beginning of the request (and relying on HW to skip unneeded switches) does have one key advantage. Should we implement request reordering on Haswell, we will not know in advance what the previous executing context was on the GPU and so we would not be able to elide the MI_SET_CONTEXT commands ourselves and always have to emit them. Having our hand forced now actually prepares us for later. Now since that context and mm follow the request, we no longer (and not for a long time since requests took over!) require a trace point to tell when we write the switch into the ring, since it is always. (This is even more important when you remember that simply writing into the ring bears no relation to the current mm.) v2: Sandybridge has to agree to use LRI as well. Testcase: igt/drv_selftests/live_hangcheck Signed-off-by:
Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180611110845.31890-1-chris@chris-wilson.co.uk
-
Chris Wilson authored
An issue encountered with switching mm on gen7 is that the GPU likes to hang (with the VS unit busy) when told to force restore the current context. We can simply workaround this by substituting the MI_FORCE_RESTORE flag with a round-trip through the kernel_context, forcing the context to be saved and restored; thereby reloading the PP_DIR registers and updating the modified page directory! v2: Undo attempted optimisation in caller (Tvrtko) Signed-off-by:
Mika Kuoppala <mika.kuoppala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180611104808.24295-1-chris@chris-wilson.co.uk
-
Imre Deak authored
While checking workarounds related to the CDCLK PLL, I noticed that the DMC firmware bits for WA#1183 are missing for SKL. After that I clarified with HW people that it's not needed on SKL, since it doesn't support eDP1.4 which would be the only thing requiring the problematic CDCLK clock rates. So in theory we shouldn't ever choose these frequencies, but add an assert in any case for catching such cases and for documentation. v2: - Move the check to skl_set_cdclk and warn whenever using the corresponding VCO freq. (Ville) v3: - Actually check for the platform. (Ville) Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by:
Imre Deak <imre.deak@intel.com> Reviewed-by:
Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180608144137.7943-1-imre.deak@intel.com
-
Michel Thierry authored
Use the correct engine class shift value while storing the ctx hw id. Fixes the copy+paste error from commit 61d5676b ("drm/i915/perf: fix ctx_id read with GuC & ICL"). Apologies for not spotting this in the original review, the specific_ctx_id_mask is correct, only the specific_ctx_id had this problem. v2: Just use the upper 32 bits of lrc_desc (Chris) v3: If we use the lrc_desc, we must apply the ctx_id_mask too (Lionel) Fixes: 61d5676b ("drm/i915/perf: fix ctx_id read with GuC & ICL") Signed-off-by:
Michel Thierry <michel.thierry@intel.com> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Michel Thierry <michel.thierry@intel.com> Cc: Matthew Auld <matthew.auld@intel.com> Cc: Jani Nikula <jani.nikula@linux.intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: intel-gfx@lists.freedesktop.org Reviewed-by:
Lionel Landwerlin <lionel.g.landwerlin@intel.com> Signed-off-by:
-
Michel Thierry authored
The upper 32 bits of the lrc_desc (bits 52-32 to be precise) are the context hw id in GEN8-10, so use them and have one less thing to maintain in the unlikely case we change the descriptor sw fields. v2: If we use the lrc_desc, we must apply the ctx_id_mask too (Lionel) Signed-off-by:
-
Chris Wilson authored
We special case the position of the batch within the GTT to prevent negative self-relocation deltas from underflowing. However, that restriction is being applied after a trial pin of the batch in its current position. Thus we are not rejecting an invalid location if the batch has been used before, leading to an assertion if we happen to need to rearrange the entire payload. In the worst case, this may cause a GPU hang on gen7 or perhaps missing state. References: https://bugs.freedesktop.org/show_bug.cgi?id=105720 Fixes: 2889caa9 ("drm/i915: Eliminate lots of iterations over the execobjects array") Signed-off-by:
-
Chris Wilson authored
As i915_gem_object_phys_attach() wants to play dirty and mess around with obj->mm.pages itself (replacing the shmemfs with a DMA allocation), refactor the gubbins so into i915_gem_object_unset_pages() that we don't have to duplicate all the secrets. Signed-off-by:
Matthew Auld <matthew.auld@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180611075532.26534-1-chris@chris-wilson.co.uk Link: https://patchwork.freedesktop.org/patch/msgid/152871104647.1718.8796913290418060204@jlahtine-desk.ger.corp.intel.com
-
Chris Wilson authored
Due to a silent conflict (silent because we are trying to fix the CI test that is meant to exercising these failures!) between commit 51e645b6 ("drm/i915: Mark the GPU as wedged without error on fault injection") and commit 8571a05a ("drm/i915: Use GEM suspend when aborting initialisation"), we failed to actually squash the error message after injecting the load failure. Rearrange the code to export i915_load_failure() for better logging of real errors (and quiet logging of injected errors). Signed-off-by:
Michał Winiarski <michal.winiarski@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180609111058.2660-1-chris@chris-wilson.co.uk
-
Jani Nikula authored
Setting PCH type to PCH_NOP before checking whether we actually have a PCH ends up returning true for HAS_PCH_SPLIT() on all non-PCH split platforms. Fix this by using PCH_NOP only for platforms that actually have a PCH. Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Reviewed-by:
Jani Nikula <jani.nikula@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180608123330.31003-6-jani.nikula@intel.com
-
Jani Nikula authored
HAS_PCH_NOP() implies a PCH platform without south display, not generic disabled display. Prefer num_pipes == 0 for PCH independent checks. Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Reviewed-by:
-
Jani Nikula authored
Use intel_pch_type() also for mapping the no PCH case (PCH id 0) to PCH_NONE to simplify code. Also make sure that intel_pch_type() knows all the PCH ids returned by intel_virt_detect_pch(). Loudly fail if this isn't the case; this shouldn't happen anyway. Cc: Colin Xu <Colin.Xu@intel.com> Reviewed-by:
Colin Xu <Colin.Xu@intel.com> Reviewed-by:
-
Lucas De Marchi authored
There's a difference between PCH_NONE and PCH_NOP: the former means we don't have a PCH while in the latter we do, but it doesn't have the south display. Signed-off-by:
Lucas De Marchi <lucas.demarchi@intel.com> Signed-off-by:
-
Jani Nikula authored
Virtualized non-PCH systems such as Broxton or Geminilake should use PCH_NONE to indicate no PCH rather than PCH_NOP. The latter is a specific case to indicate a PCH system without south display. Reported-by:
-
- 09 Jun, 2018 1 commit
-
-
Chris Wilson authored
To allow ourselves to use a first class vma for the aliasing_ppgtt page directory, we have to reorder the shutdown on module unload to remove and unpin the aliasing_ppgtt before complaining about any objects left in the GGTT. Signed-off-by:
Matthew Auld <matthew.william.auld@gmail.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180609090151.22007-1-chris@chris-wilson.co.uk
-
- 08 Jun, 2018 7 commits
-
-
Chris Wilson authored
Since vgpu is not supported on Haswell or any other gen6/7, we do not need to check and act upon it's enablement. Signed-off-by:
-
Chris Wilson authored
hsw_mm_switch() and gen7_mm_switch() are identical, so let's remove the redundant specialism. Signed-off-by:
-
Chris Wilson authored
When we want to unwind an error when allocating the PD for gen6, we call gen6_ppgtt_clear_range() telling to clear upto the PD we've previously allocated. However, we passed it an incorrect length, passing it the endpoint instead. Fortunately, as the start was always 0, this has no impact today, but tomorrow we want to start using non-zero origins. Reported-by:
-
Ville Syrjälä authored
On SKL+ the dst colorkey must be configured on the lower plane that contains the colorkey. This is in contrast to most earlier platforms where the dst colorkey is configured on the plane above. The hardware will peform dst keying only between two immediately adjacent (in zorder) planes. Plane 2 will be keyed against plane 1, plane 3 againts plane 2, and so on. There is no way to key arbitrary planes against plane 1. Thus offering dst color keying on plane 3+ is pointless. In fact it can be harmful since enabling dst keying on more than one plane on the same pipe leads to only the top-most of the planes performing the keying. For any plane lower in zorder the dst key enable is simply ignored. v2: s/plane 0/plane 1/ etc. since the hw plane names start from 1 Don't break dst colorkey on pre-SKL sprites (hunk ended in the wrong patch) Signed-off-by: -
Chris Wilson authored
If we have been instructed (by CI) to inject a fault to load the module with a wedged GPU, do so quietly less we upset CI. Signed-off-by:
-
Mika Kuoppala authored
There is a problem with kbl up to rev E0 where a heavy memory/fabric traffic from adjacent engine(s) can cause an engine reset to fail. This traffic can be from normal memory accesses or it can be from heavy polling on a semaphore wait. For engine hogging causing a fail, we already fallback to full reset. Which effectively stops all engines and thus we only add a workaround documentation. For the semaphore wait loop poll case, we add one microsecond poll interval to semaphore wait to guarantee bandwidth for the reset preration. The side effect is that we make semaphore completion latencies also 1us longer. v2: Let full reset handle the adjacent engine idling (Chris) v3: Skip render engine (Joonas), please checkpatch on define (Mika) References: https://bugs.freedesktop.org/show_bug.cgi?id=106684 References: VTHSD#2227190, HSDES#1604216706, BSID#0917 Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by:
-
Mika Kuoppala authored
Our reset handling has a retry layer further up in the chain. As we have told the engine to prepare for reset, and failed it, make sure to remove that preparation so that the next attempted reset has a clean slate by triggering another full prepare cycle for the engines. v2: ret as int, simplified cleanup (Chris) Cc: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by:
-
- 07 Jun, 2018 5 commits
-
-
Chris Wilson authored
In the next patch, we will subclass the gen6 hw_ppgtt. In order, for the two different generations of hw ppgtt stucts to be of different size, push the allocation down to the constructor. Signed-off-by:
-
Chris Wilson authored
To allow for future non-object backed vma, we need to be able to specialise the callbacks for binding, et al, the vma. For example, instead of calling vma->vm->bind_vma(), we now call vma->ops->bind_vma(). This gives us the opportunity to later override the operation for a custom vma. v2: flip order of unbind/bind Signed-off-by:
-
Chris Wilson authored
In order to allow ourselves to use VMA to wrap other entities other than GEM objects, we need to allow for the vma->obj backpointer to be NULL. In most cases, we know we are operating on a GEM object and its vma, but we need the core code (such as i915_vma_pin/insert/bind/unbind) to work regardless of the innards. The remaining eyesore here is vma->obj->cache_level and related (but less of an issue) vma->obj->gt_ro. With a bit of care we should mirror those on the vma itself. Signed-off-by:
-
Chris Wilson authored
In preparation for vm_fault_t becoming a distinct type, convert the fault handler (i915_gem_fault()) over to the new interface. Based on a patch by Souptick Joarder References: 1c8f4220 ("mm: change return type to vm_fault_t") Signed-off-by:
-
Chris Wilson authored
As part of our GEM initialisation now, we send a request to the hardware in order to record the initial GPU state. This coupled with deferred idle workers, makes aborting on error tricky. We already have the mechanism in place to wait on the GPU and cancel all the deferred workers for suspend, so let's reuse it during the error teardown. It is already used in places for later init error handling, but doing so at this point is slightly ugly due to the mutex dance (it's ok, the module load is still single threaded). Testcase: igt/drv_module_reload/basic-reload-inject Signed-off-by:
-
- 06 Jun, 2018 3 commits
-
-
Rodrigo Vivi authored
Signed-off-by:Rodrigo Vivi <rodrigo.vivi@intel.com>
-
Chris Wilson authored
The macro declared the ppgtt parameter but implicitly used the local vm instead. Signed-off-by:
-
Chris Wilson authored
When we reach the magic value and do inject a fault into our module load, mark the module option as being hit. Since we fail from inside pci probe, the module load isn't actually aborted and the module (and parameters) are left lingering. igt can then inspect the parameter on its synchronous completion of modprobe to see if the fault injection was successful, and will keeping on injecting new faults until the module succeeds in loading having surpassed the number of fault points. v2: Reset to 0 after being hit; Signed-off-by:
Michal Wajdeczko <michal.wajdeczko@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180606144153.4244-1-chris@chris-wilson.co.uk
-
- 05 Jun, 2018 10 commits
-
-
Chris Wilson authored
In the near future, I want to subclass gen6_hw_ppgtt as it contains a few specialised members and I wish to add more. To avoid the ugliness of using ppgtt->base.base, rename the i915_hw_ppgtt base member (i915_address_space) as vm, which is our common shorthand for an i915_address_space local. Signed-off-by:
-
Chris Wilson authored
The inactive counter was over the active list, and vice versa. Fortuitously this should not cause a problem in practice as they shared the same array and clamped the number of entries they would write. Signed-off-by:
-
Michal Wajdeczko authored
In case of failure during GuC clients creation, we forget to cleanup earlier pool allocation. Use proper teardown to fix that. Signed-off-by:
Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Signed-off-by:
-
Tvrtko Ursulin authored
As Chris has discovered on his Ivybridge, and later automated test runs have confirmed, on most of our platforms hrtimer faced with heavy GPU load can occasionally become sufficiently imprecise to affect PMU sampling calculations. This means we cannot assume sampling frequency is what we asked for, but we need to measure the interval ourselves. This patch is similar to Chris' original proposal for per-engine counters, but instead of introducing a new set to work around the problem with frequency sampling, it swaps around the way internal frequency accounting is done. Instead of accumulating current frequency and dividing by sampling frequency on readout, it accumulates frequency scaled by each period. v2: * Typo in commit message, comment on period calculation and USEC_PER_SEC. (Chris Wilson) Testcase: igt/perf_pmu/*busy* # snb, ivb, hsw Signed-off-by:
Tvrtko Ursulin <tvrtko.ursulin@intel.com> Suggested-by:
-
Tvrtko Ursulin authored
Underlaying field is u64 so the tracepoint needs to be as well. v2: * Re-order binary packet for 64-bit alignment. (Chris Wilson) Signed-off-by:
-
Tvrtko Ursulin authored
In the string tracepoint representation we ended up with the engine sandwiched between context hardware id and context fence id. Move the two pieces of context data together for redability. Binary records are left as is, that is both fields remaing under the existing name and ordering. v2: * Do not consolidate the printk format, just reorder. (Lionel) Signed-off-by:
-
Tvrtko Ursulin authored
Instead of using the engine->id, use uabi_class:instance pairs in trace- points including engine info. This will be more readable, more future proof and more stable for userspace consumption. v2: * Use u16 for class and instance. (Chris Wilson) Signed-off-by:
-
Chris Wilson authored
Since the kernel provides SZ_1M, use it in preference of 1 << 20. Signed-off-by:
-
Michal Wajdeczko authored
In function gem_init_hw() we are calling uc_init_hw() but in case of error later in function, we missed to call matching uc_fini_hw() v2: pulled out from the series Signed-off-by:
Sagar Arun Kamble <sagar.a.kamble@intel.com> Signed-off-by:
-
Chris Wilson authored
To spare ourselves a long line later, refactor the repeated check of bind_count vs pin_count to a helper. v2: Fix up the commentary! Suggested-by:
-
