There are four non-static functions in i915_guc_submission.c that take a
'dev' parameter. All are called only from GuC loader code, and can be
easily converted to accept 'dev_priv' instead.
Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1465579766-31595-1-git-send-email-david.s.gordon@intel.com
Our goal is to rename the anonymous per-engine struct beneath the
current intel_context. However, after a lively debate resolving around
the confusion between intel_context_engine and intel_engine_context, the
realisation is that the two structs target different users. The outer
struct is API / user facing, and so carries the higher level GEM
information. The inner struct is hw facing. Thus we want to name the
inner struct intel_context and the outer one i915_gem_context. As the
first step, we need to rename the current struct:
s/struct intel_context/struct i915_gem_context/
which fits much better with its constructors already conveying the
i915_gem_context prefix!
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Dave Gordon <david.s.gordon@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1464098023-3294-1-git-send-email-chris@chris-wilson.co.uk
Mostly little optimisations and future-proofing against code breakage.
For instance, if the driver is correctly following the submission
protocol, the "out of space" condition is impossible, so the previous
runtime WARN_ON() is promoted to a GEM_BUG_ON() for a more dramatic
effect in development and less impact in end-user systems.
Similarly we can make alignment checking more stringent and replace
other WARN_ON() conditions that don't relate to the runtime hardware
state with either BUILD_BUG_ON() for compile-time-detectable issues, or
GEM_BUG_ON() for logical "can't happen" errors.
With those changes, we can convert it to void, as suggested by Chris
Wilson, and update the calling code appropriately.
v2:
Note that we're now putting the request seqno in the "fence_id"
field of each GuC-work-item, in case it turns up somewhere useful
(e.g. in a GuC log) [Tvrtko Ursulin].
Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Rather than wait to see whether more space becomes available in the GuC
submission workqueue, we can just return -EAGAIN and let the caller try
again in a little while. This gets rid of an uninterruptable sleep in
the polling code :)
We'll also add a counter to the GuC client statistics, to see how often
we find the WQ full.
v2:
Flag the likely() code path (Tvtrko Ursulin).
v4:
Add/update comments about failure counters (Tvtrko Ursulin).
Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
The knowledge of how to derive the relevant client from the request
should be localised within i915_guc_submission.c; the LRC code shouldn't
have to know about the internal details of the GuC submission process.
And all the information the GuC code needs should be encapsulated in (or
reachable from) the request.
v2:
GEM_BUG_ON() for bad GuC client (Tvrtko Ursulin).
Add/update kerneldoc explaining check_space/submit protocol
Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
The GuC initialisation code could do other things apart from loading
firmware, so here we rename the three primary entry points to remove any
specific reference to "ucode" (no functional changes, just renaming).
Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Now that we keep the GuC client process descriptor permanently mapped,
we don't really need to keep a local copy of the GuC's work-queue-head.
So we can simplify the code a little by not doing this.
Signed-off-by: Alex Dai <yu.dai@intel.com>
Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Don't use kmap_atomic() for doorbell & process descriptor access.
This patch fixes the BUG shown below, where the thread could sleep
while holding a kmap_atomic mapping. In order not to need to call
kmap_atomic() in this code path, we now set up a permanent kernel
mapping of the shared doorbell and process-descriptor page, and
use that in all doorbell and process-descriptor related code.
BUG: scheduling while atomic: gem_close_race/1941/0x00000002
Modules linked in: hid_generic usbhid i915 asix usbnet libphy mii
i2c_algo_bit drm_kms_helper cfbfillrect syscopyarea cfbimgblt
sysfillrect sysimgblt fb_sys_fops cfbcopyarea drm coretemp i2c_hid
hid video pinctrl_sunrisepoint pinctrl_intel acpi_pad nls_iso8859_1
e1000e ptp psmouse pps_core ahci libahci
CPU: 0 PID: 1941 Comm: gem_close_race Tainted: G U 4.4.0-160121+ #123
Hardware name: Intel Corporation Skylake Client platform/Skylake AIO
DDR3L RVP10, BIOS SKLSE2R1.R00.X100.B01.1509220551 09/22/2015
0000000000013e40 ffff880166c27a78 ffffffff81280d02 ffff880172c13e40
ffff880166c27a88 ffffffff810c203a ffff880166c27ac8 ffffffff814ec808
ffff88016b7c6000 ffff880166c28000 00000000000f4240 0000000000000001
Call Trace:
[<ffffffff81280d02>] dump_stack+0x4b/0x79
[<ffffffff810c203a>] __schedule_bug+0x41/0x4f
[<ffffffff814ec808>] __schedule+0x5a8/0x690
[<ffffffff814ec927>] schedule+0x37/0x80
[<ffffffff814ef3fd>] schedule_hrtimeout_range_clock+0xad/0x130
[<ffffffff81090be0>] ? hrtimer_init+0x10/0x10
[<ffffffff814ef3f1>] ? schedule_hrtimeout_range_clock+0xa1/0x130
[<ffffffff814ef48e>] schedule_hrtimeout_range+0xe/0x10
[<ffffffff814eef9b>] usleep_range+0x3b/0x40
[<ffffffffa01ec109>] i915_guc_wq_check_space+0x119/0x210 [i915]
[<ffffffffa01da47c>] intel_logical_ring_alloc_request_extras+0x5c/0x70 [i915]
[<ffffffffa01cdbf1>] i915_gem_request_alloc+0x91/0x170 [i915]
[<ffffffffa01c1c07>] i915_gem_do_execbuffer.isra.25+0xbc7/0x12a0 [i915]
[<ffffffffa01cb785>] ? i915_gem_object_get_pages_gtt+0x225/0x3c0 [i915]
[<ffffffffa01d1fb6>] ? i915_gem_pwrite_ioctl+0xd6/0x9f0 [i915]
[<ffffffffa01c2e68>] i915_gem_execbuffer2+0xa8/0x250 [i915]
[<ffffffffa00f65d8>] drm_ioctl+0x258/0x4f0 [drm]
[<ffffffffa01c2dc0>] ? i915_gem_execbuffer+0x340/0x340 [i915]
[<ffffffff8111590d>] do_vfs_ioctl+0x2cd/0x4a0
[<ffffffff8111eac2>] ? __fget+0x72/0xb0
[<ffffffff81115b1c>] SyS_ioctl+0x3c/0x70
[<ffffffff814effd7>] entry_SYSCALL_64_fastpath+0x12/0x6a
------------[ cut here ]------------
v4:
Only tear down doorbell & kunmap() client object if we actually
succeeded in allocating a client object (Tvrtko Ursulin)
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93847
Original-version-by: Alex Dai <yu.dai@intel.com>
Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
Cc: Tvtrko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Separate out the layers of includes (linux, drm, intel, i915) so that it
is a little easier to order our definitions between our multiple
reentrant headers. A couple of headers needed fixes to make them more
standalone (forgotten includes, forward declarations etc).
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1460565315-7748-2-git-send-email-chris@chris-wilson.co.uk
Previously GuC uses ring id as engine id because of same definition.
But this is not true since this commit:
commit de1add3605
Author: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Date: Fri Jan 15 15:12:50 2016 +0000
drm/i915: Decouple execbuf uAPI from internal implementation
Added GuC engine id into GuC interface to decouple it from ring id used
by driver.
v2: Keep ring name print out in debugfs; using for_each_ring() where
possible to keep driver consistent. (Chris W.)
Signed-off-by: Alex Dai <yu.dai@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1453579094-29860-1-git-send-email-yu.dai@intel.com
The GuC firmware uses this for various purposes. The ADS itself is
a chunk of memory created by driver to share with GuC. Its members
are usually addresses telling where GuC to access them, including
things like scheduler policies, register list that will be saved
and restored during reset etc.
This is the first patch of a series to enable GuC ADS. For now, we
only create the ADS obj whilst keep it disabled.
v1: remove dead code checking return of kmap_atomic (Chris Wilson)
v2: use kmap instead of the atomic version of it.
Signed-off-by: Alex Dai <yu.dai@intel.com>
Reviewed-by: Dave Gordon <david.s.gordon@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1450468812-4882-3-git-send-email-yu.dai@intel.com
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Split GuC work queue space checking from submission and move it to
ring_alloc_request_extras. The reason is that failure in later
i915_add_request() won't be handled. In the case timeout happens,
driver can return early in order to handle the error.
v1: Move wq_reserve_space to ring_reserve_space
v2: Move wq_reserve_space to alloc_request_extras (Chris Wilson)
v3: The work queue head pointer is cached by driver now. So we can
quickly return if space is available.
s/reserve/check/g (Dave Gordon)
v4: Update cached wq head after ring doorbell; check wq space before
ring doorbell in case unexpected error happens; call wq space
check only when GuC submission is enabled. (Dave Gordon)
Signed-off-by: Alex Dai <yu.dai@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1450295155-10050-1-git-send-email-yu.dai@intel.com
Reviewed-by: Dave Gordon <david.s.gordon@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
For now, remove the spinlocks that protected the GuC's
statistics block and work queue; they are only accessed
by code that already holds the global struct_mutex, and
so are redundant (until the big struct_mutex rewrite!).
The specific problem that the spinlocks caused was that
if the work queue was full, the driver would try to
spinwait for one jiffy, but with interrupts disabled the
jiffy count would not advance, leading to a system hang.
The issue was found using test case igt/gem_close_race.
The new version will usleep() instead, still holding
the struct_mutex but without any spinlocks.
v4: Reorganize commit message (Dave Gordon)
v3: Remove unnecessary whitespace churn
v2: Clean up wq_lock too
v1: Clean up host2guc lock as well
Signed-off-by: Alex Dai <yu.dai@intel.com>
Reviewed-by: Dave Gordon <david.s.gordon@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1449104189-27591-1-git-send-email-yu.dai@intel.com
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
The size / offset information of all firmware ingredients are
now caculated from header. Driver will validate the header and
rsa key size. If any component is out of boundary, driver will
reject the loading too.
v6: Clean up warnings from make docs
v5: Tidy up GuC titles in kernel/Doc
v4: Now using 'size_dw' for those defined in css_header
v3: 1) Move DOC to intel_guc_fwif.h right before css_header
definition. Add more comments.
2) Change 'size' to 'len' or 'length' to avoid confusion.
3) Add UOS_RSA_SCRATCH_MAX_COUNT according to BSpec. And
driver validate size of RSA key now.
4) Add fw component size/offset info to intel_guc_fw.
v2: Add indent into DOC to make fixed-width format rather than
change the tmpl.
v1: 1) guc_css_header is defined as __packed now
2) Add and correct GuC related topics in kernel/Doc
Signed-off-by: Alex Dai <yu.dai@intel.com>
Reviewed-by: Dave Gordon <david.s.gordon@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Add host2guc interface to notify GuC power state changes when
enter or resume from power saving state.
v3: Move intel_guc_suspend to i915_drm_suspend for consistency.
v2: Add GuC suspend/resume to runtime suspend/resume too
v1: Change to a more flexible way when fill host to GuC scratch
data in order to remove hard coding.
Signed-off-by: Alex Dai <yu.dai@intel.com>
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
GuC-based submission is mostly the same as execlist mode, up to
intel_logical_ring_advance_and_submit(), where the context being
dispatched would be added to the execlist queue; at this point
we submit the context to the GuC backend instead.
There are, however, a few other changes also required, notably:
1. Contexts must be pinned at GGTT addresses accessible by the GuC
i.e. NOT in the range [0..WOPCM_SIZE), so we have to add the
PIN_OFFSET_BIAS flag to the relevant GGTT-pinning calls.
2. The GuC's TLB must be invalidated after a context is pinned at
a new GGTT address.
3. GuC firmware uses the one page before Ring Context as shared data.
Therefore, whenever driver wants to get base address of LRC, we
will offset one page for it. LRC_PPHWSP_PN is defined as the page
number of LRCA.
4. In the work queue used to pass requests to the GuC, the GuC
firmware requires the ring-tail-offset to be represented as an
11-bit value, expressed in QWords. Therefore, the ringbuffer
size must be reduced to the representable range (4 pages).
v2:
Defer adding #defines until needed [Chris Wilson]
Rationalise type declarations [Chris Wilson]
v4:
Squashed kerneldoc patch into here [Daniel Vetter]
v5:
Update request->tail in code common to both GuC and execlist modes.
Add a private version of lr_context_update(), as sharing the
execlist version leads to race conditions when the CPU and
the GuC both update TAIL in the context image.
Conversion of error-captured HWS page to string must account
for offset from start of object to actual HWS (LRC_PPHWSP_PN).
Issue: VIZ-4884
Signed-off-by: Alex Dai <yu.dai@intel.com>
Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
Reviewed-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
A GuC client has its own doorbell and workqueue. It maintains the
doorbell cache line, process description object and work queue item.
A default guc_client is created for the i915 driver to use for
normal-priority in-order submission.
Note that the created client is not yet ready for use; doorbell
allocation will fail as we haven't yet linked the GuC's context
descriptor to the default contexts for each ring (see later patch).
v2:
Defer adding structure members until needed [Chris Wilson]
Rationalise type declarations [Chris Wilson]
v5:
Add GuC per-engine submission & seqno statistics.
Move wq locking to encompass both get_space() and add_item().
Take forcewake lock in host2guc_action() [Tom O'Rourke]
v6:
Fix GuC doorbell cacheline selection code (the
cacheline-within-page calculation was wrong).
Rename GuC priorities to make them closer to the names used in
the GuC firmware source, matching what the autogenerated
versions will (probably) be.
Add per-ring statistics to client.
Issue: VIZ-4884
Signed-off-by: Alex Dai <yu.dai@intel.com>
Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
Reviewed-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Allocate a GEM object to hold GuC log data. A debugfs interface
(i915_guc_log_dump) is provided to print out the log content.
v2:
Add struct members at point of use [Chris Wilson]
v6:
Rebased
Issue: VIZ-4884
Signed-off-by: Alex Dai <yu.dai@intel.com>
Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
Reviewed-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
This adds the first of the data structures used to communicate with the
GuC (the pool of guc_context structures).
We create a GuC-specific wrapper round the GEM object allocator as all
GEM objects shared with the GuC must be pinned into GGTT space at an
address that is NOT in the range [0..WOPCM_TOP), as that range of GGTT
addresses is not accessible to the GuC (from the GuC's point of view,
it's permanently reserved for other objects such as the BootROM & SRAM).
Later, we will need to allocate additional GuC-sharable objects for the
submission client(s) and the GuC's debug log.
v2:
Remove redundant initialisation [Chris Wilson]
Defer adding struct members until needed [Chris Wilson]
Local functions should pass dev_priv rather than dev [Chris Wilson]
v5:
Invalidate GuC TLB after allocating and pinning a new object
v6:
Rebased
Issue: VIZ-4884
Signed-off-by: Alex Dai <yu.dai@intel.com>
Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
Reviewed-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
This fetches the required firmware image from the filesystem,
then loads it into the GuC's memory via a dedicated DMA engine.
This patch is derived from GuC loading work originally done by
Vinit Azad and Ben Widawsky.
v2:
Various improvements per review comments by Chris Wilson
v3:
Removed 'wait' parameter to intel_guc_ucode_load() as firmware
prefetch is no longer supported in the common firmware loader,
per Daniel Vetter's request.
Firmware checker callback fn now returns errno rather than bool.
v4:
Squash uC-independent code into GuC-specifc loader [Daniel Vetter]
Don't keep the driver working (by falling back to execlist mode)
if GuC firmware loading fails [Daniel Vetter]
v5:
Clarify WOPCM-related #defines [Tom O'Rourke]
Delete obsolete code no longer required with current h/w & f/w
[Tom O'Rourke]
Move the call to intel_guc_ucode_init() later, so that it can
allocate GEM objects, and have it fetch the firmware; then
intel_guc_ucode_load() doesn't need to fetch it later.
[Daniel Vetter].
v6:
Update comment describing intel_guc_ucode_load() [Tom O'Rourke]
Issue: VIZ-4884
Signed-off-by: Alex Dai <yu.dai@intel.com>
Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
Reviewed-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Dave Gordon
Chris Wilson
Alex Dai