- 09 9月, 2016 3 次提交
-
-
由 Chris Wilson 提交于
Since we have a cooperative mode now with a direct reset, we can avoid the contention on struct_mutex and instead try then sleep on the I915_RESET_IN_PROGRESS bit. If the mutex is held and that bit is cleared, all is fine. Otherwise, we sleep for a bit and try again. In the worst case we sleep for an extra second waiting for the mutex to be released (no one touching the GPU is allowed the struct_mutex whilst the I915_RESET_IN_PROGRESS bit is set). But when we have a direct reset, this allows us to clean up the reset worker faster. v2: Remember to call wake_up_bit() after changing (for the faster wakeup as promised) Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk> Reviewed-by: NMika Kuoppala <mika.kuoppala@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/20160909131201.16673-12-chris@chris-wilson.co.uk
-
由 Chris Wilson 提交于
If a waiter is holding the struct_mutex, then the reset worker cannot reset the GPU until the waiter returns. We do not want to return -EAGAIN form i915_wait_request as that breaks delicate operations like i915_vma_unbind() which often cannot be restarted easily, and returning -EIO is just as useless (and has in the past proven dangerous). The remaining WARN_ON(i915_wait_request) serve as a valuable reminder that handling errors from an indefinite wait are tricky. We can keep the current semantic that knowing after a reset is complete, so is the request, by performing the reset ourselves if we hold the mutex. uevent emission is still handled by the reset worker, so it may appear slightly out of order with respect to the actual reset (and concurrent use of the device). Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk> Reviewed-by: NMika Kuoppala <mika.kuoppala@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/20160909131201.16673-11-chris@chris-wilson.co.uk
-
由 Chris Wilson 提交于
In preparation for introducing a per-engine reset, we can first separate the mixing of the reset state from the global reset counter. The loss of atomicity in updating the reset state poses a small problem for handling the waiters. For requests, this is solved by advancing the seqno so that a waiter waking up after the reset knows the request is complete. For pending flips, we still rely on the increment of the global reset epoch (as well as the reset-in-progress flag) to signify when the hardware was reset. The advantage, now that we do not inspect the reset state during reset itself i.e. we no longer emit requests during reset, is that we can use the atomic updates of the state flags to ensure that only one reset worker is active. v2: Mika spotted that I transformed the i915_gem_wait_for_error() wakeup into a waiter wakeup. Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk> Cc: Arun Siluvery <arun.siluvery@linux.intel.com> Cc: Mika Kuoppala <mika.kuoppala@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1470414607-32453-6-git-send-email-arun.siluvery@linux.intel.comReviewed-by: NMika Kuoppala <mika.kuoppala@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/20160909131201.16673-7-chris@chris-wilson.co.uk
-
- 27 8月, 2016 1 次提交
-
-
由 Chris Wilson 提交于
Rather than walk the full array of engines checking whether each is in the mask in turn, we can use the mask to jump to the right engines. This should quicker for a sparse array of engines or mask, whilst generating smaller code: text data bss dec hex filename 1251010 4579 800 1256389 132bc5 drivers/gpu/drm/i915/i915.ko 1250530 4579 800 1255909 1329e5 drivers/gpu/drm/i915/i915.ko The downside is that we have to pass in a temporary, alas no C99 iterators yet. [P.S. Joonas doesn't like having to pass extra temporaries into the macro, and even less that I called them tmp. As yet, we haven't found a macro that avoids passing in a temporary that is smaller. We probably will get C99 iterators first!] Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Reviewed-by: NMika Kuoppala <mika.kuoppala@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/20160827075401.16470-2-chris@chris-wilson.co.uk
-
- 22 8月, 2016 1 次提交
-
-
由 Chris Wilson 提交于
If the engine isn't being retired (worker starvation?) then it is possible for us to repeatedly observe that between consecutive hangchecks the seqno on the ring to be the same and there remain unretired requests. Ignore these completely and only regard the engine as busy for the purpose of hang detection (not stall detection) if there are outstanding breadcrumbs. In recent history we have looked at using both the request and seqno as indication of activity on the engine, but that was reduced to just inspecting seqno in commit cffa781e ("drm/i915: Simplify check for idleness in hangcheck"). However, in commit dcff85c8 ("drm/i915: Enable i915_gem_wait_for_idle() without holding struct_mutex"), I made the decision to use the new common lockless function, under the assumption that request retirement was more frequent than hangcheck and so we would not have a stuck busy check. The flaw there was in forgetting that we accumulate the hang score, and so successive checks seeing a stuck request, albeit with the GPU advancing elsewhere and so not necessary the same stuck request, would eventually trigger the hang. Fixes: dcff85c8 ("drm/i915: Enable i915_gem_wait_for_idle()...") Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/20160820145408.32180-1-chris@chris-wilson.co.ukReviewed-by: NMika Kuoppala <mika.kuoppala@intel.com>
-
- 10 8月, 2016 1 次提交
-
-
由 Chris Wilson 提交于
In commit 2529d570 ("drm/i915: Drop racy markup of missed-irqs from idle-worker") the racy detection of missed interrupts was removed when we went idle. This however opened up the issue that the stuck waiters were not being reported, causing a test case failure. If we move the stuck waiter detection out of hangcheck and into the breadcrumb mechanims (i.e. the waiter) itself, we can avoid this issue entirely. This leaves hangcheck looking for a stuck GPU (inspecting for request advancement and HEAD motion), and breadcrumbs looking for a stuck waiter - hopefully make both easier to understand by their segregation. v2: Reduce the error message as we now run independently of hangcheck, and the hanging batch used by igt also counts as a stuck waiter causing extra warnings in dmesg. v3: Move the breadcrumb's hangcheck kickstart to the first missed wait. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97104 Fixes: 2529d570 (waiter"drm/i915: Drop racy markup of missed-irqs...") Testcase: igt/drv_missed_irq Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> Cc: Mika Kuoppala <mika.kuoppala@intel.com> Reviewed-by: NMika Kuoppala <mika.kuoppala@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1470761272-1245-2-git-send-email-chris@chris-wilson.co.uk
-
- 06 8月, 2016 1 次提交
-
-
由 Rodrigo Vivi 提交于
No functional change. Instead of defining a new empty function let's use what is available on drm. It gets cleaner, and easy to read, and understand. Signed-off-by: NRodrigo Vivi <rodrigo.vivi@intel.com> Reviewed-by: NVille Syrjälä <ville.syrjala@linux.intel.com>
-
- 05 8月, 2016 1 次提交
-
-
由 Chris Wilson 提交于
The principal motivation for this was to try and eliminate the struct_mutex from i915_gem_suspend - but we still need to hold the mutex current for the i915_gem_context_lost(). (The issue there is that there may be an indirect lockdep cycle between cpu_hotplug (i.e. suspend) and struct_mutex via the stop_machine().) For the moment, enabling last request tracking for the engine, allows us to do busyness checking and waiting without requiring the struct_mutex - which is useful in its own right. As a side-effect of having a robust means for tracking engine busyness, we can replace our other busyness heuristic, that of comparing against the last submitted seqno. For paranoid reasons, we have a semi-ordered check of that seqno inside the hangchecker, which we can now improve to an ordered check of the engine's busyness (removing a locked xchg in the process). v2: Pass along "bool interruptible" as being unlocked we cannot rely on i915->mm.interruptible being stable or even under our control. v3: Replace check Ironlake i915_gpu_busy() with the common precalculated value Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk> Reviewed-by: NJoonas Lahtinen <joonas.lahtinen@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1470388464-28458-6-git-send-email-chris@chris-wilson.co.uk
-
- 03 8月, 2016 1 次提交
-
-
由 Chris Wilson 提交于
The state stored in this struct is not only the information about the buffer object, but the ring used to communicate with the hardware. Using buffer here is overly specific and, for me at least, conflates with the notion of buffer objects themselves. s/struct intel_ringbuffer/struct intel_ring/ s/enum intel_ring_hangcheck/enum intel_engine_hangcheck/ s/describe_ctx_ringbuf()/describe_ctx_ring()/ s/intel_ring_get_active_head()/intel_engine_get_active_head()/ s/intel_ring_sync_index()/intel_engine_sync_index()/ s/intel_ring_init_seqno()/intel_engine_init_seqno()/ s/ring_stuck()/engine_stuck()/ s/intel_cleanup_engine()/intel_engine_cleanup()/ s/intel_stop_engine()/intel_engine_stop()/ s/intel_pin_and_map_ringbuffer_obj()/intel_pin_and_map_ring()/ s/intel_unpin_ringbuffer()/intel_unpin_ring()/ s/intel_engine_create_ringbuffer()/intel_engine_create_ring()/ s/intel_ring_flush_all_caches()/intel_engine_flush_all_caches()/ s/intel_ring_invalidate_all_caches()/intel_engine_invalidate_all_caches()/ s/intel_ringbuffer_free()/intel_ring_free()/ Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk> Reviewed-by: NJoonas Lahtinen <joonas.lahtinen@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1469432687-22756-15-git-send-email-chris@chris-wilson.co.uk Link: http://patchwork.freedesktop.org/patch/msgid/1470174640-18242-4-git-send-email-chris@chris-wilson.co.uk
-
- 27 7月, 2016 1 次提交
-
-
由 Chris Wilson 提交于
We still have lots of comments that refer to the old ring when we mean struct intel_engine_cs and its hardware correspondence. This patch fixes an instance inside hangcheck to talk about engines. Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk> Link: http://patchwork.freedesktop.org/patch/msgid/1469432687-22756-10-git-send-email-chris@chris-wilson.co.ukReviewed-by: NJoonas Lahtinen <joonas.lahtinen@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1469606850-28659-5-git-send-email-chris@chris-wilson.co.uk
-
- 20 7月, 2016 2 次提交
-
-
由 Chris Wilson 提交于
Just a different colour to better match virtual addresses elsewhere. s/ring->virtual_start/ring->vaddr/ Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk> Link: http://patchwork.freedesktop.org/patch/msgid/1469005202-9659-9-git-send-email-chris@chris-wilson.co.ukReviewed-by: NDave Gordon <david.s.gordon@intel.com> Reviewed-by: NJoonas Lahtinen <joonas.lahtinen@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1469017917-15134-8-git-send-email-chris@chris-wilson.co.uk
-
由 Chris Wilson 提交于
Ringbuffers are now being written to either through LLC or WC paths, so treating them as simply iomem is no longer adequate. However, for the older !llc hardware, the hardware is documentated as treating the TAIL register update as serialising, so we can relax the barriers when filling the rings (but even if it were not, it is still an uncached register write and so serialising anyway.). For simplicity, let's ignore the iomem annotation. v2: Remove iomem from ringbuffer->virtual_address v3: And for good measure add iomem elsewhere to keep sparse happy Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> #v2 Link: http://patchwork.freedesktop.org/patch/msgid/1469005202-9659-8-git-send-email-chris@chris-wilson.co.uk Link: http://patchwork.freedesktop.org/patch/msgid/1469017917-15134-7-git-send-email-chris@chris-wilson.co.uk
-
- 15 7月, 2016 1 次提交
-
-
由 Rodrigo Vivi 提交于
Some Kabylake SKUs are going to use Kabypoint PCH. It is mainly for Halo and DT ones. >From our specs it doesn't seem that KBP brings any change on the display south engine. So let's consider this as a continuation of SunrisePoint, i.e., SPT+. Since it is easy to get confused by a letter change: KBL = Kabylake - CPU/GPU codename. KBP = Kabypoint - PCH codename. Signed-off-by: NRodrigo Vivi <rodrigo.vivi@intel.com> Reviewed-by: NAnder Conselvan de Oliveira <conselvan2@gmail.com> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96826 Link: http://patchwork.freedesktop.org/patch/msgid/1467418032-15167-1-git-send-email-rodrigo.vivi@intel.comSigned-off-by: NRodrigo Vivi <rodrigo.vivi@intel.com> (cherry picked from commit 22dea0be) Signed-off-by: NMika Kuoppala <mika.kuoppala@intel.com>
-
- 14 7月, 2016 1 次提交
-
-
由 Chris Wilson 提交于
To allow the user finer control over waitboosting, allow them to set the frequency we request for the boost. This also them allows to effectively disable the boosting by setting the boost request to a low frequency. Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk> Link: http://patchwork.freedesktop.org/patch/msgid/1468397438-21226-5-git-send-email-chris@chris-wilson.co.ukReviewed-by: NMika Kuoppala <mika.kuoppala@intel.com>
-
- 08 7月, 2016 1 次提交
-
-
由 Rodrigo Vivi 提交于
Some Kabylake SKUs are going to use Kabypoint PCH. It is mainly for Halo and DT ones. >From our specs it doesn't seem that KBP brings any change on the display south engine. So let's consider this as a continuation of SunrisePoint, i.e., SPT+. Since it is easy to get confused by a letter change: KBL = Kabylake - CPU/GPU codename. KBP = Kabypoint - PCH codename. Signed-off-by: NRodrigo Vivi <rodrigo.vivi@intel.com> Reviewed-by: NAnder Conselvan de Oliveira <conselvan2@gmail.com> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96826 Link: http://patchwork.freedesktop.org/patch/msgid/1467418032-15167-1-git-send-email-rodrigo.vivi@intel.comSigned-off-by: NRodrigo Vivi <rodrigo.vivi@intel.com>
-
- 06 7月, 2016 1 次提交
-
-
由 Chris Wilson 提交于
As we inspect both the tasklet (to check for an active bottom-half) and set the irq-posted flag at the same time (both in the interrupt handler and then in the bottom-halt), group those two together into the same cacheline. (Not having total control over placement of the struct means we can't guarantee the cacheline boundary, we need to align the kmalloc and then each struct, but the grouping should help.) v2: Try a couple of different names for the state touched by the user interrupt handler. Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk> Reviewed-by: NTvrtko Ursulin <tvrtko.ursulin@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1467805142-22219-3-git-send-email-chris@chris-wilson.co.uk
-
- 05 7月, 2016 3 次提交
-
-
由 Chris Wilson 提交于
Since drm_i915_private is now a subclass of drm_device we do not need to chase the drm_i915_private->dev backpointer and can instead simply access drm_i915_private->drm directly. text data bss dec hex filename 1068757 4565 416 1073738 10624a drivers/gpu/drm/i915/i915.ko 1066949 4565 416 1071930 105b3a drivers/gpu/drm/i915/i915.ko Created by the coccinelle script: @@ struct drm_i915_private *d; identifier i; @@ ( - d->dev->i + d->drm.i | - d->dev + &d->drm ) and for good measure the dev_priv->dev backpointer was removed entirely. Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk> Reviewed-by: NMatthew Auld <matthew.auld@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1467711623-2905-4-git-send-email-chris@chris-wilson.co.uk
-
由 Chris Wilson 提交于
Remove some redundant kernel messages as we deduce a hung GPU and capture the error state. v2: Fix "hang" vs "no progress" message whilst I was there v3: s/snprintf/scnprintf/ Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1467618513-4966-2-git-send-email-chris@chris-wilson.co.ukReviewed-by: NMika Kuoppala <mika.kuoppala@intel.com>
-
由 Chris Wilson 提交于
After Joonas complained about using READ_ONCE() on the only use of the variable in the function, where the intent was to simply document that the read was intentionally racy and unlocked, I switched the READ_ONCE() over to lockless_dereference(). However, in linux-next that has a stronger type-check to only allow pointers and is no longer interchangeable with READ_ONCE(), see commit 331b6d8c ("locking/barriers: Validate lockless_dereference() is used on a pointer type") Reported-by: NStephen Rothwell <sfr@canb.auug.org.au> Fixes: 67d97da3 ("drm/i915: Only start retire worker when idle") Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk> Link: http://patchwork.freedesktop.org/patch/msgid/1467705276-707-1-git-send-email-chris@chris-wilson.co.ukReviewed-by: NMika Kuoppala <mika.kuoppala@intel.com>
-
- 04 7月, 2016 3 次提交
-
-
由 Chris Wilson 提交于
Since we now subclass struct drm_device, we can save pointer dances by noting the equivalence of struct drm_device and struct drm_i915_private, i.e. by using to_i915(). text data bss dec hex filename 1073824 4562 416 1078802 107612 drivers/gpu/drm/i915/i915.ko 1068976 4562 416 1073954 106322 drivers/gpu/drm/i915/i915.ko Created by the coccinelle script: @@ expression E; identifier p; @@ - struct drm_i915_private *p = E->dev_private; + struct drm_i915_private *p = to_i915(E); Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk> Reviewed-by: NDave Gordon <david.s.gordon@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1467628477-25379-1-git-send-email-chris@chris-wilson.co.uk
-
由 Chris Wilson 提交于
Make sure that the RPS bottom-half is flushed before we set the idle frequency when we decide the GPU is idle. This should prevent any races with the bottom-half and setting the idle frequency, and ensures that the bottom-half is bounded by the GPU's rpm reference taken for when it is active (i.e. between gen6_rps_busy() and gen6_rps_idle()). v2: Avoid recursively using the i915->wq - RPS does not touch the struct_mutex so has no place being on the ordered i915->wq. v3: Enable/disable interrupts for RPS busy/idle in order to prevent further HW access from RPS outside of the wakeref. Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk> Cc: Imre Deak <imre.deak@intel.com> Cc: Jesse Barnes <jbarnes@virtuousgeek.org> References: https://bugs.freedesktop.org/show_bug.cgi?id=89728Reviewed-by: NMichał Winiarski <michal.winiarski@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1467616119-4093-6-git-send-email-chris@chris-wilson.co.uk
-
由 Chris Wilson 提交于
The retire worker is a low frequency task that makes sure we retire outstanding requests if userspace is being lax. We only need to start it once as it remains active until the GPU is idle, so do a cheap test before the more expensive queue_work(). A consequence of this is that we need correct locking in the worker to make the hot path of request submission cheap. To keep the symmetry and keep hangcheck strictly bound by the GPU's wakelock, we move the cancel_sync(hangcheck) to the idle worker before dropping the wakelock. v2: Guard against RCU fouling the breadcrumbs bottom-half whilst we kick the waiter. v3: Remove the wakeref assertion squelching (now we hold a wakeref for the hangcheck, any rpm error there is genuine). v4: To prevent excess work when retiring requests, we split the busy flag into two, a boolean to denote whether we hold the wakeref and a bitmask of active engines. v5: Reorder cancelling hangcheck upon idling to avoid a race where we might cancel a hangcheck after being preempted by a new task Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk> References: https://bugs.freedesktop.org/show_bug.cgi?id=88437Reviewed-by: NJoonas Lahtinen <joonas.lahtinen@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1467616119-4093-1-git-send-email-chris@chris-wilson.co.uk
-
- 02 7月, 2016 10 次提交
-
-
由 Chris Wilson 提交于
Since the tests can and do explicitly check debugfs/i915_ring_missed_irqs for the handling of a "missed interrupt", adding it to the dmesg at INFO is just noise. When it happens for real, we still class it as an ERROR. Note that I have chose to remove it entirely because when we detect the "missed interrupt" is irrelevant and the message contains no more information than we glean from looking in debugfs. Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk> Reviewed-by: NTvrtko Ursulin <tvrtko.ursulin@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1467390209-3576-20-git-send-email-chris@chris-wilson.co.uk
-
由 Chris Wilson 提交于
With only a single callsite for intel_engine_cs->irq_get and ->irq_put, we can reduce the code size by moving the common preamble into the caller, and we can also eliminate the reference counting. For completeness, as we are no longer doing reference counting on irq, rename the get/put vfunctions to enable/disable respectively and are able to review the use of posting reads. We only require the serialisation with hardware when enabling the interrupt (i.e. so we cannot miss an interrupt by going to sleep before the hardware truly enables it). Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk> Link: http://patchwork.freedesktop.org/patch/msgid/1467390209-3576-18-git-send-email-chris@chris-wilson.co.uk
-
由 Chris Wilson 提交于
If we flag the seqno as potentially stale upon receiving an interrupt, we can use that information to reduce the frequency that we apply the heavyweight coherent seqno read (i.e. if we wake up a chain of waiters). v2: Use cmpxchg to replace READ_ONCE/WRITE_ONCE for more explicit control of the ordering wrt to interrupt generation and interrupt checking in the bottom-half. Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk> Reviewed-by: NTvrtko Ursulin <tvrtko.ursulin@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1467390209-3576-14-git-send-email-chris@chris-wilson.co.uk
-
由 Chris Wilson 提交于
On Ironlake, there is no command nor register to ensure that the write from a MI_STORE command is completed (and coherent on the CPU) before the command parser continues. This means that the ordering between the seqno write and the subsequent user interrupt is undefined (like gen6+). So to ensure that the seqno write is completed after the final user interrupt we need to delay the read sufficiently to allow the write to complete. This delay is undefined by the bspec, and empirically requires 75us even though a register read combined with a clflush is less than 500ns. Hence, the delay is due to an on-chip buffer rather than the latency of the write to memory. Note that the render ring controls this by filling the PIPE_CONTROL fifo with stalling commands that force the earliest pipe-control with the seqno to be completed before the command parser continues. Given that we need a barrier operation for BSD, we may as well forgo the extra per-batch latency by using a common per-interrupt barrier. Studying the impact of adding the usleep shows that in both sequences of and individual synchronous no-op batches is negligible for the media engine (where the write now is unordered with the interrupt). Converting the render engine over from the current glutton of pie-controls over to the per-interrupt delays speeds up both the sequential and individual synchronous no-ops by 20% and 60%, respectively. This speed up holds even when looking at the throughput of small copies (4KiB->4MiB), both serial and synchronous, by about 20%. This is because despite adding a significant delay to the interrupt, in all likelihood we will see the seqno write without having to apply the barrier (only in the rare corner cases where the write is delayed on the last required is the delay necessary). Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94307 Testcase: igt/gem_sync #ilk Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk> Reviewed-by: NTvrtko Ursulin <tvrtko.ursulin@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1467390209-3576-12-git-send-email-chris@chris-wilson.co.uk
-
由 Chris Wilson 提交于
By using the same address for storing the HWS on every platform, we can remove the platform specific vfuncs and reduce the get-seqno routine to a single read of a cached memory location. v2: Fix semaphore_passed() to look at the signaling engine (not the waiter's) Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk> Reviewed-by: NTvrtko Ursulin <tvrtko.ursulin@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1467390209-3576-8-git-send-email-chris@chris-wilson.co.uk
-
由 Chris Wilson 提交于
One particularly stressful scenario consists of many independent tasks all competing for GPU time and waiting upon the results (e.g. realtime transcoding of many, many streams). One bottleneck in particular is that each client waits on its own results, but every client is woken up after every batchbuffer - hence the thunder of hooves as then every client must do its heavyweight dance to read a coherent seqno to see if it is the lucky one. Ideally, we only want one client to wake up after the interrupt and check its request for completion. Since the requests must retire in order, we can select the first client on the oldest request to be woken. Once that client has completed his wait, we can then wake up the next client and so on. However, all clients then incur latency as every process in the chain may be delayed for scheduling - this may also then cause some priority inversion. To reduce the latency, when a client is added or removed from the list, we scan the tree for completed seqno and wake up all the completed waiters in parallel. Using igt/benchmarks/gem_latency, we can demonstrate this effect. The benchmark measures the number of GPU cycles between completion of a batch and the client waking up from a call to wait-ioctl. With many concurrent waiters, with each on a different request, we observe that the wakeup latency before the patch scales nearly linearly with the number of waiters (before external factors kick in making the scaling much worse). After applying the patch, we can see that only the single waiter for the request is being woken up, providing a constant wakeup latency for every operation. However, the situation is not quite as rosy for many waiters on the same request, though to the best of my knowledge this is much less likely in practice. Here, we can observe that the concurrent waiters incur extra latency from being woken up by the solitary bottom-half, rather than directly by the interrupt. This appears to be scheduler induced (having discounted adverse effects from having a rbtree walk/erase in the wakeup path), each additional wake_up_process() costs approximately 1us on big core. Another effect of performing the secondary wakeups from the first bottom-half is the incurred delay this imposes on high priority threads - rather than immediately returning to userspace and leaving the interrupt handler to wake the others. To offset the delay incurred with additional waiters on a request, we could use a hybrid scheme that did a quick read in the interrupt handler and dequeued all the completed waiters (incurring the overhead in the interrupt handler, not the best plan either as we then incur GPU submission latency) but we would still have to wake up the bottom-half every time to do the heavyweight slow read. Or we could only kick the waiters on the seqno with the same priority as the current task (i.e. in the realtime waiter scenario, only it is woken up immediately by the interrupt and simply queues the next waiter before returning to userspace, minimising its delay at the expense of the chain, and also reducing contention on its scheduler runqueue). This is effective at avoid long pauses in the interrupt handler and at avoiding the extra latency in realtime/high-priority waiters. v2: Convert from a kworker per engine into a dedicated kthread for the bottom-half. v3: Rename request members and tweak comments. v4: Use a per-engine spinlock in the breadcrumbs bottom-half. v5: Fix race in locklessly checking waiter status and kicking the task on adding a new waiter. v6: Fix deciding when to force the timer to hide missing interrupts. v7: Move the bottom-half from the kthread to the first client process. v8: Reword a few comments v9: Break the busy loop when the interrupt is unmasked or has fired. v10: Comments, unnecessary churn, better debugging from Tvrtko v11: Wake all completed waiters on removing the current bottom-half to reduce the latency of waking up a herd of clients all waiting on the same request. v12: Rearrange missed-interrupt fault injection so that it works with igt/drv_missed_irq_hang v13: Rename intel_breadcrumb and friends to intel_wait in preparation for signal handling. v14: RCU commentary, assert_spin_locked v15: Hide BUG_ON behind the compiler; report on gem_latency findings. v16: Sort seqno-groups by priority so that first-waiter has the highest task priority (and so avoid priority inversion). v17: Add waiters to post-mortem GPU hang state. v18: Return early for a completed wait after acquiring the spinlock. Avoids adding ourselves to the tree if the is already complete, and skips the awkward question of why we don't do completion wakeups for waits earlier than or equal to ourselves. v19: Prepare for init_breadcrumbs to fail. Later patches may want to allocate during init, so be prepared to propagate back the error code. Testcase: igt/gem_concurrent_blit Testcase: igt/benchmarks/gem_latency Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk> Cc: "Rogozhkin, Dmitry V" <dmitry.v.rogozhkin@intel.com> Cc: "Gong, Zhipeng" <zhipeng.gong@intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> Cc: Dave Gordon <david.s.gordon@intel.com> Cc: "Goel, Akash" <akash.goel@intel.com> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> #v18 Link: http://patchwork.freedesktop.org/patch/msgid/1467390209-3576-6-git-send-email-chris@chris-wilson.co.uk
-
由 Chris Wilson 提交于
Currently __i915_wait_request uses a per-engine wait_queue_t for the dual purpose of waking after the GPU advances or for waking after an error. In the future, we may add even more wake sources and require greater separation, but for now we can conceptually simplify wakeups by separating the two sources. In particular, this allows us to use different wait-queues (e.g. one on the engine advancement, a global one for errors and one on each requests) without any hassle. Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk> Reviewed-by: NTvrtko Ursulin <tvrtko.ursulin@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1467390209-3576-5-git-send-email-chris@chris-wilson.co.uk
-
由 Chris Wilson 提交于
Since the function is a small wrapper around schedule_delayed_work(), move it inline to remove the function call overhead for the principle caller. Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk> Reviewed-by: NTvrtko Ursulin <tvrtko.ursulin@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1467390209-3576-4-git-send-email-chris@chris-wilson.co.uk
-
由 Chris Wilson 提交于
The queue only ever contains at most one item and has no special flags. It is just a very simple wrapper around the system-wq - a complication with no benefits. v2: Use the system_long_wq as we may wish to capture the error state after detecting the hang - which may take a bit of time. Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk> Reviewed-by: NTvrtko Ursulin <tvrtko.ursulin@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1467390209-3576-3-git-send-email-chris@chris-wilson.co.uk
-
由 Chris Wilson 提交于
We can forgo queuing the hangcheck from the start of every request to until we wait upon a request. This reduces the overhead of every request, but may increase the latency of detecting a hang. However, if nothing every waits upon a hang, did it ever hang? It also improves the robustness of the wait-request by ensuring that the hangchecker is indeed running before we sleep indefinitely (and thereby ensuring that we never actually sleep forever waiting for a dead GPU). As pointed out by Tvrtko, it is possible for a GPU hang to go unnoticed for as long as nobody is waiting for the GPU. Though this rare, during that time we may be consuming more power than if we had promptly recovered, and in the most extreme case we may exhaust all memory before forcing the hangcheck. Something to be wary off in future. Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk> Reviewed-by: NTvrtko Ursulin <tvrtko.ursulin@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1467390209-3576-2-git-send-email-chris@chris-wilson.co.uk
-
- 06 6月, 2016 1 次提交
-
-
由 Tvrtko Ursulin 提交于
Just a bunch of stale kerneldocs generating warnings when building the docs. Mostly function parameters so not very useful but still. v2: Tidy. Signed-off-by: NTvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Reviewed-by: NDaniel Vetter <daniel.vetter@ffwll.ch> Link: http://patchwork.freedesktop.org/patch/msgid/1464958937-23344-1-git-send-email-tvrtko.ursulin@linux.intel.com
-
- 01 6月, 2016 2 次提交
-
-
由 Daniel Vetter 提交于
This reverts the following patches: d55dbd06 drm/i915: Allow nonblocking update of pageflips. 15c86bdb drm/i915: Check for unpin correctness. 95c2ccdc Reapply "drm/i915: Avoid stalling on pending flips for legacy cursor updates" a6747b73 drm/i915: Make unpin async. 03f476e1 drm/i915: Prepare connectors for nonblocking checks. 2099deff drm/i915: Pass atomic states to fbc update functions. ee7171af drm/i915: Remove reset_counter from intel_crtc. 2ee004f7 drm/i915: Remove queue_flip pointer. b8d2afae drm/i915: Remove use_mmio_flip kernel parameter. 8dd634d9 drm/i915: Remove cs based page flip support. 143f73b3 drm/i915: Rework intel_crtc_page_flip to be almost atomic, v3. 84fc494b drm/i915: Add the exclusive fence to plane_state. 6885843a drm/i915: Convert flip_work to a list. aa420ddd drm/i915: Allow mmio updates on all platforms, v2. afee4d87 Revert "drm/i915: Avoid stalling on pending flips for legacy cursor updates" "drm/i915: Allow nonblocking update of pageflips" should have been split up, misses a proper commit message and seems to cause issues in the legacy page_flip path as demonstrated by kms_flip. "drm/i915: Make unpin async" doesn't handle the unthrottled cursor updates correctly, leading to an apparent pin count leak. This is caught by the WARN_ON in i915_gem_object_do_pin which screams if we have more than DRM_I915_GEM_OBJECT_MAX_PIN_COUNT pins. Unfortuantely we can't just revert these two because this patch series came with a built-in bisect breakage in the form of temporarily removing the unthrottled cursor update hack for legacy cursor ioctl. Therefore there's no other option than to revert the entire pile :( There's one tiny conflict in intel_drv.h due to other patches, nothing serious. Normally I'd wait a bit longer with doing a maintainer revert, but since the minimal set of patches we need to revert (due to the bisect breakage) is so big, time is running out fast. And very soon (especially after a few attempts at fixing issues) it'll be really hard to revert things cleanly. Lessons learned: - Not a good idea to rush the review (done by someone fairly new to the area) and not make sure domain experts had a chance to read it. - Patches should be properly split up. I only looked at the two patches that should be reverted in detail, but both look like the mix up different things in one patch. - Patches really should have proper commit messages. Especially when doing more than one thing, and especially when touching critical and tricky core code. - Building a patch series and r-b stamping it when it has a built-in bisect breakage is not a good idea. - I also think we need to stop building up technical debt by postponing atomic igt testcases even longer. I think it's clear that there's enough corner cases in this beast that we really need to have the testcases _before_ the next step lands. (cherry picked from commit 5a21b665 from drm-intel-next-queeud) Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Patrik Jakobsson <patrik.jakobsson@linux.intel.com> Cc: John Harrison <John.C.Harrison@Intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Acked-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com> Acked-by: NVille Syrjälä <ville.syrjala@linux.intel.com> Acked-by: NDave Airlie <airlied@redhat.com> Acked-by: NJani Nikula <jani.nikula@linux.intel.com> Signed-off-by: NDaniel Vetter <daniel.vetter@intel.com>
-
由 Sagar Arun Kamble 提交于
On Loading, GuC sets PM interrupts routing (bit 31) and clears ARAT expired interrupt (bit 9). Host turbo also updates this register in RPS flows. This patch ensures bit 31 and bit 9 setup by GuC persists. ARAT timer interrupt is needed in GuC for various features. It also facilitates halting GuC and hence achieving RC6. PM interrupt routing will not impact RPS interrupt reception by host as GuC will redirect them. This patch fixes igt test pm_rc6_residency that was failing with guc load/submission enabled. Tested with SKL GuC v6.1 and BXT GuC v5.1 and v8.7. v2: i915_irq/i915_pm decoupling from intel_guc. (ChrisW) v3: restructuring the mask update and rebase w.r.t Ville's patch. (ChrisW) v4: Updating the pm_intr_keep during direct_interrupts_to_guc. (Sagar) Cc: Chris Harris <chris.harris@intel.com> Cc: Zhe Wang <zhe1.wang@intel.com> Cc: Deepak S <deepak.s@intel.com> Cc: Satyanantha, Rama Gopal M <rama.gopal.m.satyanantha@intel.com> Cc: Akash Goel <akash.goel@intel.com> Testcase: igt/pm_rc6_residency Signed-off-by: NSagar Arun Kamble <sagar.a.kamble@intel.com> Tested-by: NMatt Roper <matthew.d.roper@intel.com> Reviewed-by: NChris Wilson <chris@chris-wilson.co.uk> Signed-off-by: NMatt Roper <matthew.d.roper@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1464683307-19475-1-git-send-email-sagar.a.kamble@intel.com
-
- 25 5月, 2016 1 次提交
-
-
由 Daniel Vetter 提交于
This reverts the following patches: d55dbd06 drm/i915: Allow nonblocking update of pageflips. 15c86bdb drm/i915: Check for unpin correctness. 95c2ccdc Reapply "drm/i915: Avoid stalling on pending flips for legacy cursor updates" a6747b73 drm/i915: Make unpin async. 03f476e1 drm/i915: Prepare connectors for nonblocking checks. 2099deff drm/i915: Pass atomic states to fbc update functions. ee7171af drm/i915: Remove reset_counter from intel_crtc. 2ee004f7 drm/i915: Remove queue_flip pointer. b8d2afae drm/i915: Remove use_mmio_flip kernel parameter. 8dd634d9 drm/i915: Remove cs based page flip support. 143f73b3 drm/i915: Rework intel_crtc_page_flip to be almost atomic, v3. 84fc494b drm/i915: Add the exclusive fence to plane_state. 6885843a drm/i915: Convert flip_work to a list. aa420ddd drm/i915: Allow mmio updates on all platforms, v2. afee4d87 Revert "drm/i915: Avoid stalling on pending flips for legacy cursor updates" "drm/i915: Allow nonblocking update of pageflips" should have been split up, misses a proper commit message and seems to cause issues in the legacy page_flip path as demonstrated by kms_flip. "drm/i915: Make unpin async" doesn't handle the unthrottled cursor updates correctly, leading to an apparent pin count leak. This is caught by the WARN_ON in i915_gem_object_do_pin which screams if we have more than DRM_I915_GEM_OBJECT_MAX_PIN_COUNT pins. Unfortuantely we can't just revert these two because this patch series came with a built-in bisect breakage in the form of temporarily removing the unthrottled cursor update hack for legacy cursor ioctl. Therefore there's no other option than to revert the entire pile :( There's one tiny conflict in intel_drv.h due to other patches, nothing serious. Normally I'd wait a bit longer with doing a maintainer revert, but since the minimal set of patches we need to revert (due to the bisect breakage) is so big, time is running out fast. And very soon (especially after a few attempts at fixing issues) it'll be really hard to revert things cleanly. Lessons learned: - Not a good idea to rush the review (done by someone fairly new to the area) and not make sure domain experts had a chance to read it. - Patches should be properly split up. I only looked at the two patches that should be reverted in detail, but both look like the mix up different things in one patch. - Patches really should have proper commit messages. Especially when doing more than one thing, and especially when touching critical and tricky core code. - Building a patch series and r-b stamping it when it has a built-in bisect breakage is not a good idea. - I also think we need to stop building up technical debt by postponing atomic igt testcases even longer. I think it's clear that there's enough corner cases in this beast that we really need to have the testcases _before_ the next step lands. Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Patrik Jakobsson <patrik.jakobsson@linux.intel.com> Cc: John Harrison <John.C.Harrison@Intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Acked-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com> Acked-by: NVille Syrjälä <ville.syrjala@linux.intel.com> Acked-by: NDave Airlie <airlied@redhat.com> Acked-by: NJani Nikula <jani.nikula@linux.intel.com> Signed-off-by: NDaniel Vetter <daniel.vetter@intel.com>
-
- 23 5月, 2016 1 次提交
-
-
由 Ville Syrjälä 提交于
We've never actually enabled or unmasked the GSE interrupt on BDW+, even though the interrupt handler was always prepared for it. Let's enable it and see what happens. Credit to Mark Kettenis who fixed this in the OpenBSD fork of the driver. He reports that it fixed the "ACPI _BCM/_BCQ-based brightness mechanism on a MacBookPro12,1 and a 3rd gen Lenovo X1 Carbon" for them. Mark says: "FWIW, this *is* needed if you want ACPI-based backlight control to work. On Linux you probably don't notice, since "hardware" backlight control is preferred over "firmware" or "platform" backlight control. It would help me if this did land in the Linux tree though, as it will make future imports of the i915 driver into OpenBSD easier." So even though we don't really need this, let's put it in to help Mark with future porting efforts. Should be harmless to have it enabled in any case. Reviewed-by: NDaniel Vetter <daniel.vetter@ffwll.ch> References: http://lists.freedesktop.org/archives/intel-gfx/2015-December/081799.htmlReported-by: NMark Kettenis <mark.kettenis@xs4all.nl> Cc: Mark Kettenis <mark.kettenis@xs4all.nl> Cc: Jani Nikula <jani.nikula@linux.intel.com> Signed-off-by: NVille Syrjälä <ville.syrjala@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1463649283-28698-1-git-send-email-ville.syrjala@linux.intel.comAcked-by: NJani Nikula <jani.nikula@intel.com>
-
- 19 5月, 2016 3 次提交
-
-
由 Maarten Lankhorst 提交于
With mmio flips now available on all platforms it's time to remove support for cs flips. Signed-off-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1463490484-19540-13-git-send-email-maarten.lankhorst@linux.intel.comReviewed-by: NPatrik Jakobsson <patrik.jakobsson@linux.intel.com>
-
由 Maarten Lankhorst 提交于
Rename intel_unpin_work to intel_flip_work and use it for mmio flips and unpinning. Use flip_queued_req to hold the wait request in the mmio case, and the vblank counter from intel_crtc_get_vblank_counter. MMIO flips get their own path through intel_finish_page_flip_mmio, handled on vblank. CS page flips go through *_cs. Changes since v1: - Clean up destinction between MMIO and CS flips. Signed-off-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1463490484-19540-7-git-send-email-maarten.lankhorst@linux.intel.comReviewed-by: NPatrik Jakobsson <patrik.jakobsson@linux.intel.com>
-
由 Maarten Lankhorst 提交于
Instead of calling prepare_flip right before calling finish_page_flip do everything from prepare_page_flip in finish_page_flip. Putting prepare and finish page_flip in a single step removes the need for INTEL_FLIP_COMPLETE, so it can be removed. This simplifies the code slightly. Changes since v1: - Invert if case to simplify code. - Add missing barrier. - Reword commit message. Changes since v2: - intel_page_flip_plane is removed. - work->pending is turned into a bool. Signed-off-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1463490484-19540-5-git-send-email-maarten.lankhorst@linux.intel.comReviewed-by: NPatrik Jakobsson <patrik.jakobsson@linux.intel.com>
-