Chris Wilson [Fri, 5 Aug 2016 09:14:09 +0000 (10:14 +0100)]
drm/i915/userptr: Remove superfluous interruptible=false on waiting
Inside the kthread context, we can't be interrupted by signals so
touching the mm.interruptible flag is pointless and wait-request now
consumes EIO itself.
Chris Wilson [Fri, 5 Aug 2016 09:14:08 +0000 (10:14 +0100)]
drm/i915: Convert non-blocking userptr waits for requests over to using RCU
We can completely avoid taking the struct_mutex around the non-blocking
waits by switching over to the RCU request management (trading the mutex
for a RCU read lock and some complex atomic operations). The improvement
is that we gain further contention reduction, and overall the code
become simpler due to the reduced mutex dancing.
Chris Wilson [Fri, 5 Aug 2016 09:14:07 +0000 (10:14 +0100)]
drm/i915: Convert non-blocking waits for requests over to using RCU
We can completely avoid taking the struct_mutex around the non-blocking
waits by switching over to the RCU request management (trading the mutex
for a RCU read lock and some complex atomic operations). The improvement
is that we gain further contention reduction, and overall the code
become simpler due to the reduced mutex dancing.
v2: Move i915_gem_fault tracepoint back to the start of the function,
before the unlocked wait.
It is useful to be able to wait on pending rendering without grabbing
the struct_mutex. We can do this by using the i915_gem_active_get_rcu()
primitive to acquire a reference to the pending request without
requiring struct_mutex, just the RCU read lock, and then call
i915_wait_request().
v2: Rebase onto new i915_gem_active_get_unlocked() semantics that take
the RCU read lock on behalf of the caller.
Daniel Vetter [Fri, 5 Aug 2016 08:36:15 +0000 (10:36 +0200)]
Merge remote-tracking branch 'airlied/drm-next' into drm-intel-next-queued
Backmerge the 4.8 pull request state from Dave - conflicts were
getting out of hand, and Chris has some patches which outright don't
apply without everything merged together again.
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Matt Roper [Thu, 4 Aug 2016 21:08:00 +0000 (14:08 -0700)]
drm/i915/gen9: Give one extra block per line for SKL plane WM calculations
The bspec was updated a couple weeks ago to add an extra block per line
to plane watermark calculations for linear pixel formats.
Bspec update 115327 description:
"Gen9+ - Updated the plane blocks per line calculation for linear
cases. Adds +1 for all linear cases to handle the non-block aligned
stride cases."
Chris Wilson [Thu, 4 Aug 2016 15:32:42 +0000 (16:32 +0100)]
drm/i915: Export our request as a dma-buf fence on the reservation object
If the GEM objects being rendered with in this request have been
exported via dma-buf to a third party, hook ourselves into the dma-buf
reservation object so that the third party can serialise with our
rendering via the dma-buf fences.
Chris Wilson [Thu, 4 Aug 2016 15:32:41 +0000 (16:32 +0100)]
drm/i915: Enable lockless lookup of request tracking via RCU
If we enable RCU for the requests (providing a grace period where we can
inspect a "dead" request before it is freed), we can allow callers to
carefully perform lockless lookup of an active request.
However, by enabling deferred freeing of requests, we can potentially
hog a lot of memory when dealing with tens of thousands of requests per
second - with a quick insertion of a synchronize_rcu() inside our
shrinker callback, that issue disappears.
v2: Currently, it is our responsibility to handle reclaim i.e. to avoid
hogging memory with the delayed slab frees. At the moment, we wait for a
grace period in the shrinker, and block for all RCU callbacks on oom.
Suggested alternatives focus on flushing our RCU callback when we have a
certain number of outstanding request frees, and blocking on that flush
after a second high watermark. (So rather than wait for the system to
run out of memory, we stop issuing requests - both are nondeterministic.)
Paul E. McKenney wrote:
Another approach is synchronize_rcu() after some largish number of
requests. The advantage of this approach is that it throttles the
production of callbacks at the source. The corresponding disadvantage
is that it slows things up.
Another approach is to use call_rcu(), but if the previous call_rcu()
is still in flight, block waiting for it. Yet another approach is
the get_state_synchronize_rcu() / cond_synchronize_rcu() pair. The
idea is to do something like this:
You would of course do an initial get_state_synchronize_rcu() to
get things going. This would not block unless there was less than
one grace period's worth of time between invocations. But this
assumes a busy system, where there is almost always a grace period
in flight. But you can make that happen as follows:
Note that you need additional code to make sure that the old callback
has completed before doing a new one. Setting and clearing a flag
with appropriate memory ordering control suffices (e.g,. smp_load_acquire()
and smp_store_release()).
v3: More comments on compiler and processor order of operations within
the RCU lookup and discover we can use rcu_access_pointer() here instead.
v4: Wrap i915_gem_active_get_rcu() to take the rcu_read_lock itself.
Chris Wilson [Thu, 4 Aug 2016 15:32:39 +0000 (16:32 +0100)]
drm/i915: Move obj->active:5 to obj->flags
We are motivated to avoid using a bitfield for obj->active for a couple
of reasons. Firstly, we wish to document our lockless read of obj->active
using READ_ONCE inside i915_gem_busy_ioctl() and that requires an
integral type (i.e. not a bitfield). Secondly, gcc produces abysmal code
when presented with a bitfield and that shows up high on the profiles of
request tracking (mainly due to excess memory traffic as it converts
the bitfield to a register and back and generates frequent AGI in the
process).
v2: BIT, break up a long line in compute the other engines, new paint
for i915_gem_object_is_active (now i915_gem_object_get_active).
Chris Wilson [Thu, 4 Aug 2016 15:32:38 +0000 (16:32 +0100)]
drm/i915: Use dev_priv consistently through the intel_frontbuffer interface
Rather than a mismash of struct drm_device *dev and struct
drm_i915_private *dev_priv being used freely within a function, be
consistent and only pass along dev_priv.
Chris Wilson [Thu, 4 Aug 2016 15:32:37 +0000 (16:32 +0100)]
drm/i915: Use atomics to manipulate obj->frontbuffer_bits
The individual bits inside obj->frontbuffer_bits are protected by each
plane->mutex, but the whole bitfield may be accessed by multiple KMS
operations simultaneously and so the RMW need to be under atomics.
However, for updating the single field we do not need to mandate that it
be under the struct_mutex, one more step towards its removal as the de
facto BKL.
Chris Wilson [Thu, 4 Aug 2016 15:32:36 +0000 (16:32 +0100)]
drm/i915: Make fb_tracking.lock a spinlock
We only need a very lightweight mechanism here as the locking is only
used for co-ordinating a bitfield.
v2: Move the cheap unlikely tests into the caller
v3: Move the kerneldoc into the header (now separated out into
intel_fronbuffer.h for better kerneldoc and readability)
Chris Wilson [Thu, 4 Aug 2016 15:32:35 +0000 (16:32 +0100)]
drm/i915: Separate intel_frontbuffer into its own header
In view of adding inline functions into the intel_frontbuffer section,
we first split the header into its own file so that we can integrate it
more easily with kerneldoc.
Since i915_gem_obj_ggtt_pin() is an idiom breaking curry function for
i915_gem_object_ggtt_pin(), spare us the confusion and remove it.
Removing it now simplifies later patches to change the i915_vma_pin()
(and friends) interface.
v2: Add a redundant GEM_BUG_ON(!view) to
i915_gem_obj_lookup_or_create_ggtt_vma()
Chris Wilson [Thu, 4 Aug 2016 15:32:33 +0000 (16:32 +0100)]
drm/i915: Make i915_vma_pin() small and inline
Not only is i915_vma_pin() called for every single object on every single
execbuf, it is usually a simple increment as the VMA is already bound for
execution by the GPU. Rearrange the tests for unbound and pin_count
overflow so that we can do the increment and test very cheaply and
compact enough to inline the operation into execbuf. The trick used is
to note that we can check for an overflow bit (keeping space available
for it inside the flags) at the same time as checking the binding bits.
Chris Wilson [Thu, 4 Aug 2016 15:32:32 +0000 (16:32 +0100)]
drm/i915: Combine all i915_vma bitfields into a single set of flags
In preparation to perform some magic to speed up i915_vma_pin(), which
is among the hottest of hot paths in execbuf, refactor all the bitfields
accessed by i915_vma_pin() into a single unified set of flags.
Chris Wilson [Thu, 4 Aug 2016 15:32:31 +0000 (16:32 +0100)]
drm/i915: Start passing around i915_vma from execbuffer
During execbuffer we look up the i915_vma in order to reserve them in
the VM. However, we then do a double lookup of the vma in order to then
pin them, all because we lack the necessary interfaces to operate on
i915_vma - so introduce i915_vma_pin()!
v2: Tidy parameter lists to remove one level of redirection in the hot
path.
Chris Wilson [Thu, 4 Aug 2016 15:32:29 +0000 (16:32 +0100)]
drm/i915: Record allocated vma size
Tracking the size of the VMA as allocated allows us to dramatically
reduce the complexity of later functions (like inserting the VMA in to
the drm_mm range manager).
Chris Wilson [Thu, 4 Aug 2016 15:32:28 +0000 (16:32 +0100)]
drm/i915: Update i915_gem_get_ggtt_size/_alignment to use drm_i915_private
For consistency, internal functions should take drm_i915_private rather
than drm_device. Now that we are subclassing drm_device, there are no
more size wins, but being consistent is its own blessing.
Chris Wilson [Thu, 4 Aug 2016 15:32:27 +0000 (16:32 +0100)]
drm/i915: Update the GGTT size/alignment query functions
In order to be consistent with other address space functions, we want to
pass around 64-bit sizes, even though all known global GTT are limited
to 4GiB. Similarly, we are trying to be consistent in using the _ggtt_
nomenclature when referring to the special global GTT.
v2: Update docs to consistently state "global GTT".
Chris Wilson [Thu, 4 Aug 2016 15:32:26 +0000 (16:32 +0100)]
drm/i915: Convert 4096 alignment request to 0 for drm_mm allocations
As we always allocate in chunks of 4096 (that being both the PAGE_SIZE
and our own GTT_PAGE_SIZE), we know that all results from the drm_mm are
aligned to at least 4096. The drm_mm allocator itself is optimised for
alignment == 0, and so by converting alignments of 4096 to 0 we can
satisfy our own requirements and still hit the faster path.
Chris Wilson [Thu, 4 Aug 2016 15:32:24 +0000 (16:32 +0100)]
drm/i915: Reduce WARN(i915_gem_valid_gtt_space) to a debug-only check
i915_gem_valid_gtt_space() is used after inserting the VMA to double
check the list - the location should have been chosen to pass all the
restrictions.
Chris Wilson [Thu, 4 Aug 2016 15:32:23 +0000 (16:32 +0100)]
drm/i915: Pad GTT views of exec objects up to user specified size
Our GPUs impose certain requirements upon buffers that depend upon how
exactly they are used. Typically this is expressed as that they require
a larger surface than would be naively computed by pitch * height.
Normally such requirements are hidden away in the userspace driver, but
when we accept pointers from strangers and later impose extra conditions
on them, the original client allocator has no idea about the
monstrosities in the GPU and we require the userspace driver to inform
the kernel how many padding pages are required beyond the client
allocation.
v2: Long time, no see
v3: Try an anonymous union for uapi struct compatibility
Chris Wilson [Thu, 4 Aug 2016 15:32:22 +0000 (16:32 +0100)]
drm/i915: Fix up vma alignment to be u64
This is not the full fix, as we are required to percolate the u64 nature
down through the drm_mm stack, but this is required now to prevent
explosions due to mismatch between execbuf (eb_vma_misplaced) and vma
binding (i915_vma_misplaced) - and reduces the risk of spurious changes
as we adjust the vma interface in the next patches.
v2: long long casts not required for u64 printk (%llx)
Move the single line to the callsite as the name is now misleading, and
the purpose is solely to add the request to the execution queue. Here,
we can see that if we failed to dispatch the batch from the request, we
can forgo flushing the GPU when closing the request.
Chris Wilson [Thu, 4 Aug 2016 15:32:20 +0000 (16:32 +0100)]
drm/i915: Remove request retirement before each batch
This reimplements the denial-of-service protection against igt from
commit 227f782e4667 ("drm/i915: Retire requests before creating a new
one") and transfers the stall from before each batch into get_pages().
The issue is that the stall is increasing latency between batches which
is detrimental in some cases (especially coupled with execlists) to
keeping the GPU well fed. Also we have made the observation that retiring
requests can of itself free objects (and requests) and therefore makes
a good first step when shrinking.
v2: Recycle objects prior to i915_gem_object_get_pages()
v3: Remove the reference to the ring from i915_gem_requests_ring() as it
operates on an intel_engine_cs.
v4: Since commit 9b5f4e5ed6fd ("drm/i915: Retire oldest completed request
before allocating next") we no longer need the safeguard to retire
requests before get_pages(). We no longer see the huge latencies when
hitting the shrinker between allocations.
Chris Wilson [Thu, 4 Aug 2016 15:32:19 +0000 (16:32 +0100)]
drm/i915: Double check the active status on the batch pool
We should not rely on obj->active being uptodate unless we manually
flush it. Instead, we can verify that the next available batch object is
idle by looking at its last active request (and checking it for
completion).
v2: remove the struct drm_device forward declaration added in the
process of removing its necessity
Chris Wilson [Thu, 4 Aug 2016 15:32:17 +0000 (16:32 +0100)]
drm/i915: Combine loops within i915_gem_evict_something
Slight micro-optimise to produce combine loops so that gcc is able to
optimise the inner-loops concisely. Since we are reviewing the loops, we
can update the comments to describe the current state of affairs, in
particular the distinction between evicting from the global GTT (which
may contain untracked items and transient global pins) and the
per-process GTT.
Chris Wilson [Wed, 3 Aug 2016 16:09:00 +0000 (17:09 +0100)]
drm/i915: Acquire audio powerwell for HD-Audio registers
On Haswell/Broadwell, the HD-Audio block is inside the HDMI/display
power well and so the sna-hda audio codec acquires the display power
well while it is operational. However, Skylake separates the powerwells
again, but yet we still need the audio powerwell to setup the registers.
(But then the hardware uses those registers even while powered off???)
Acquiring the powerwell around setting the chicken bits when setting up
the audio channel does at least silence the WARNs from touching our
registers whilst unpowered. We silence our own test cases, but maybe
there is a latent bug in using the audio channel?
v2: Grab both rpm wakelock and audio wakelock
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96214 Fixes: 03b135cebc47 "ALSA: hda - remove dependency on i915 power well for SKL") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Libin Yang <libin.yang@intel.com> Cc: Takashi Iwai <tiwai@suse.de> Cc: Marius Vlad <marius.c.vlad@intel.com> Tested-by: Hans de Goede <hdegoede@redhat.com> Cc: stable@vger.kernel.org Link: http://patchwork.freedesktop.org/patch/msgid/1470240540-29004-1-git-send-email-chris@chris-wilson.co.uk Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Ville Syrjälä [Thu, 28 Jul 2016 14:50:47 +0000 (17:50 +0300)]
drm/i915: Don't try to ack sink irqs when there are none
My ASUS PB278 at least doesn't seem to appreciate when you try to
ack sink irqs when there are none. Results in this sort of dmesg spam
[drm:drm_dp_dpcd_access] too many retries, giving up
Let's skip the ack if there are no pending irqs. I have no clue why we
do this in two places. One of them likely should just go away. Oh, and
MST has its own sink irq handler too...
Ville Syrjälä [Fri, 29 Jul 2016 13:51:16 +0000 (16:51 +0300)]
drm/i915: Allow MST sinks to work even if drm_probe_ddc() fails
With HSW + Dell UP2414Q (at least) drm_probe_ddc() occasionally fails,
and then we'll assume that the entire display has been disconnected.
We don't need the EDID from the main link, so we can simply check if
the sink is MST capable, and if so treat is as connected.
v2: Skip drm_probe_ddc() entirely for MST (Daniel)
Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> Cc: Jim Bride <jim.bride@linux.intel.com> Cc: Manasi D Navare <manasi.d.navare@intel.com> Cc: Durgadoss R <durgadoss.r@intel.com> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: http://patchwork.freedesktop.org/patch/msgid/1469800276-6979-1-git-send-email-ville.syrjala@linux.intel.com
Ville Syrjälä [Thu, 28 Jul 2016 14:50:41 +0000 (17:50 +0300)]
drm/i915: Track active streams also for DP SST
s/active_mst_links/active_streams/ and use it also for SST. We can then
use this information in the hpd handling to see if the link is active
or not, and thus whether we may need to retrain.
Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> Cc: Jim Bride <jim.bride@linux.intel.com> Cc: Manasi D Navare <manasi.d.navare@intel.com> Cc: Durgadoss R <durgadoss.r@intel.com> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1469717448-4297-6-git-send-email-ville.syrjala@linux.intel.com Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Ville Syrjälä [Thu, 28 Jul 2016 14:50:40 +0000 (17:50 +0300)]
drm/i915: Reject mixing MST and SST/HDMI on the same digital port
We can't mix MST with SST/HDMI on the same physical port, so we'll need
to reject such configurations in check_digital_port_conflicts(). Nothing
else will prevent this as MST has its fake encoders and its own connectors
so the cloning checks won't catch this.
The same digital port can be used multiple times, but only if all the
encoders involved are MST encoders, so we only want to check MST vs.
SST/HDMI, not MST vs. MST. And SST/HDMI vs. SST/HDMI we already check.
Ville Syrjälä [Thu, 28 Jul 2016 14:50:39 +0000 (17:50 +0300)]
drm/i915: Avoid mixing up SST and MST in DDI setup
The MST vs. SST selection should depend purely on the choice of the
connector/encoder. So don't try to determine the correct DDI mode
based on the intel_dp->is_mst, which simply tells us whether the sink
is in MST mode or not. Instead derive the information from the encoder
type. Since the link training code deals in non-fake encoders, we'll
also need to keep a second copy of that information around, which we'll
now designate as 'link_mst'.
Ville Syrjälä [Fri, 29 Jul 2016 13:52:39 +0000 (16:52 +0300)]
drm/i915: Read PSR caps/intermediate freqs/etc. only once on eDP
Currently we re-read a bunch of static eDP panel caps from the DPCD
over and over again. Let's do it only once to save some time and effort.
v2: Make thing less confusing with intel_edp_init_dpcd() (Chris)
Move no_aux_handshake setup in there as well
v3: Move tps3/rate printout to intel_dp_long_pulse() so that
we'll still get them on eDP as well
Chris Wilson [Thu, 4 Aug 2016 08:09:53 +0000 (09:09 +0100)]
drm/i915: Add missing rpm wakelock to GGTT pread
Joonas spotted a discrepancy between the pwrite and pread ioctls, in
that pwrite takes the rpm wakelock around its GGTT access, The wakelock
is required in order for the GTT to function. In disregard for the
current convention, we take the rpm wakelock around the access itself
rather than around the struct_mutex as the nesting is not strictly
required and such ordering will one day be fixed by explicitly noting
the barrier dependencies between the GGTT and rpm.
Fixes: b50a53715f09 ("drm/i915: Support for pread/pwrite ...") Reported-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: drm-intel-fixes@lists.freedesktop.org Link: http://patchwork.freedesktop.org/patch/msgid/1470298193-21765-1-git-send-email-chris@chris-wilson.co.uk Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Chris Wilson [Thu, 4 Aug 2016 07:43:53 +0000 (08:43 +0100)]
drm/i915/fbc: FBC causes display flicker when VT-d is enabled on Skylake
Erratum SKL075: Display Flicker May Occur When Both VT-d And FBC Are Enabled
"Display flickering may occur when both FBC (Frame Buffer Compression)
and VT - d (Intel® Virtualization Technology for Directed I/O) are enabled
and in use by the display controller."
Ville found the w/a name in the database:
WaFbcTurnOffFbcWhenHyperVisorIsUsed:skl,bxt and also dug out that it
affects Broxton.
v2: Log when the quirk is applied.
v3: Ensure i915.enable_fbc is false when !HAS_FBC()
v4: Fix function name after rebase
v5: Add Broxton to the workaround
Note for backporting to stable, we need to add
#define mkwrite_device_info(ptr) \
((struct intel_device_info *)INTEL_INFO(ptr))
Chris Wilson [Wed, 27 Jul 2016 18:11:17 +0000 (19:11 +0100)]
drm/i915: Fix use of engine->index for register offset
Since commit de1add360522 ("drm/i915: Decouple execbuf uAPI from internal
implementation") the index of the engine (its engine->id) in the
internal list no longer matches the hardware id. However, in a couple of
locations we missed fixing up the difference. In this case,
RING_FAULT_REG() refers to engine->id which is now not what the register
offset actually should be. Fortunately, in both case we should be more
or less looping over 0..I915_NUM_ENGINES.
Fixes: de1add360522 ("drm/i915: Decouple execbuf uAPI from internal...") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1469643077-2523-2-git-send-email-chris@chris-wilson.co.uk Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: stable@vger.kernel.org
The patch was only a stop-gap measure that fixed half the problem - the
leak of the fbcon when restarting X. A complete solution required
releasing the VMA when the object itself was closed rather than rely on
file/process exit. The previous patches add the VMA tracking necessary
to do close them along with the object, context or file, and so the time
has come to remove the partial fix.
Chris Wilson [Thu, 4 Aug 2016 06:52:46 +0000 (07:52 +0100)]
drm/i915: Mark the context and address space as closed
When the user closes the context mark it and the dependent address space
as closed. As we use an asynchronous destruct method, this has two
purposes. First it allows us to flag the closed context and detect
internal errors if we to create any new objects for it (as it is removed
from the user's namespace, these should be internal bugs only). And
secondly, it allows us to immediately reap stale vma.
Chris Wilson [Thu, 4 Aug 2016 06:52:45 +0000 (07:52 +0100)]
drm/i915: Release vma when the handle is closed
In order to prevent a leak of the vma on shared objects, we need to
hook into the object_close callback to destroy the vma on the object for
this file. However, if we destroyed that vma immediately we may cause
unexpected application stalls as we try to unbind a busy vma - hence we
defer the unbind to when we retire the vma.
v2: Keep vma allocated until closed. This is useful for a later
optimisation, but it is required now in order to handle potential
recursion of i915_vma_unbind() by retiring itself.
v3: Comments are important.
Chris Wilson [Thu, 4 Aug 2016 06:52:44 +0000 (07:52 +0100)]
drm/i915: Track active vma requests
Hook the vma itself into the i915_gem_request_retire() so that we can
accurately track when a solitary vma is inactive (as opposed to having
to wait for the entire object to be idle). This improves the interaction
when using multiple contexts (with full-ppgtt) and eliminates some
frequent list walking when retiring objects after a completed request.
A side-effect is that we get an active vma reference for free. The
consequence of this is shown in the next patch...
v2: Update inline names to be consistent with
i915_gem_object_get_active()
Chris Wilson [Thu, 4 Aug 2016 06:52:43 +0000 (07:52 +0100)]
drm/i915: i915_vma_move_to_active prep patch
This patch is broken out of the next just to remove the code motion from
that patch and make it more readable. What we do here is move the
i915_vma_move_to_active() to i915_gem_execbuffer.c and put the three
stages (read, write, fenced) together so that future modifications to
active handling are all located in the same spot. The importance of this
is so that we can more simply control the order in which the requests
are place in the retirement list (i.e. control the order at which we
retire and so control the lifetimes to avoid having to hold onto
references).
Chris Wilson [Thu, 4 Aug 2016 06:52:41 +0000 (07:52 +0100)]
drm/i915: Double check activity before relocations
If the object is active and we need to perform a relocation upon it, we
need to take the slow relocation path. Before we do, double check the
active requests to see if they have completed.
Chris Wilson [Thu, 4 Aug 2016 06:52:39 +0000 (07:52 +0100)]
drm/i915: Disable waitboosting for a saturated engine
If the user floods the GPU with so many requests that the engine stalls
waiting for free space, don't automatically promote the GPU to maximum
frequencies. If the GPU really is saturated with work, it will migrate
to high clocks by itself, otherwise it is merely a user flooding us with
busy-work.
Chris Wilson [Thu, 4 Aug 2016 06:52:37 +0000 (07:52 +0100)]
drm/i915: Convert intel_overlay to request tracking
intel_overlay already tracks its last flip request, along with action to
take after its completion. Refactor intel_overlay to reuse the common
i915_gem_active tracker.
Chris Wilson [Thu, 4 Aug 2016 06:52:36 +0000 (07:52 +0100)]
drm/i915: Track requests inside each intel_ring
By tracking each request occupying space inside an individual
intel_ring, we can greatly simplify the logic of tracking available
space and not worry about other timelines. (Each ring is an ordered
timeline of committed requests.)
Chris Wilson [Thu, 4 Aug 2016 06:52:35 +0000 (07:52 +0100)]
drm/i915: Refactor activity tracking for requests
With the introduction of requests, we amplified the number of atomic
refcounted objects we use and update every execbuffer; from none to
several references, and a set of references that need to be changed. We
also introduced interesting side-effects in the order of retiring
requests and objects.
Instead of independently tracking the last request for an object, track
the active objects for each request. The object will reside in the
buffer list of its most recent active request and so we reduce the kref
interchange to a list_move. Now retirements are entirely driven by the
request, dramatically simplifying activity tracking on the object
themselves, and removing the ambiguity between retiring objects and
retiring requests.
Furthermore with the consolidation of managing the activity tracking
centrally, we can look forward to using RCU to enable lockless lookup of
the current active requests for an object. In the future, we will be
able to query the status or wait upon rendering to an object without
even touching the struct_mutex BKL.
All told, less code, simpler and faster, and more extensible.
v2: Add a typedef for the function pointer for convenience later.
v3: Make the noop retirement callback explicit. Allow passing NULL to
the init_request_active() which is expanded to a common noop function.
Since we track requests, and requests are always added to the GPU fully
formed, we never have to flush the incomplete request and know that the
given request will eventually complete without any further action on our
part.
Chris Wilson [Thu, 4 Aug 2016 06:52:31 +0000 (07:52 +0100)]
drm/i915: Mark up i915_gem_active for locking annotation
The future annotations will track the locking used for access to ensure
that it is always sufficient. We make the preparations now to present
the API ahead and to make sure that GCC can eliminate the unused
parameter.
Chris Wilson [Thu, 4 Aug 2016 06:52:30 +0000 (07:52 +0100)]
drm/i915: Prepare i915_gem_active for annotations
In the future, we will want to add annotations to the i915_gem_active
struct. The API is thus expanded to hide direct access to the contents
of i915_gem_active and mediated instead through a number of helpers.
Chris Wilson [Thu, 4 Aug 2016 06:52:29 +0000 (07:52 +0100)]
drm/i915: Introduce i915_gem_active for request tracking
In the next patch, request tracking is made more generic and for that we
need a new expanded struct and to separate out the logic changes from
the mechanical churn, we split out the structure renaming into this
patch.
v2: Writer's block. Add some spiel about why we track requests.
v3: Now i915_gem_active.
v4: Now with i915_gem_active_set() for attaching to the active request.
v5: Use i915_gem_active_set() from inside the retirement handlers
Chris Wilson [Thu, 4 Aug 2016 06:52:28 +0000 (07:52 +0100)]
drm/i915: Kill drop_pages()
The drop_pages() function is a dangerous trap in that it can release the
passed in object pointer and so unless the caller is aware, it can
easily trick us into using the stale object afterwards. Move it into its
solitary callsite where we know it is safe.
Chris Wilson [Thu, 4 Aug 2016 06:52:27 +0000 (07:52 +0100)]
drm/i915: Be more careful when unbinding vma
When we call i915_vma_unbind(), we will wait upon outstanding rendering.
This will also trigger a retirement phase, which may update the object
lists. If, we extend request tracking to the VMA itself (rather than
keep it at the encompassing object), then there is a potential that the
obj->vma_list be modified for other elements upon i915_vma_unbind(). As
a result, if we walk over the object list and call i915_vma_unbind(), we
need to be prepared for that list to change.
Chris Wilson [Thu, 4 Aug 2016 06:52:26 +0000 (07:52 +0100)]
drm/i915: Count how many VMA are bound for an object
Since we may have VMA allocated for an object, but we interrupted their
binding, there is a disparity between have elements on the obj->vma_list
and being bound. i915_gem_obj_bound_any() does this check, but this is
not rigorously observed - add an explicit count to make it easier.
Chris Wilson [Thu, 4 Aug 2016 06:52:25 +0000 (07:52 +0100)]
drm/i915: Store owning file on the i915_address_space
For the global GTT (and aliasing GTT), the address space is owned by the
device (it is a global resource) and so the per-file owner field is
NULL. For per-process GTT (where we create an address space per
context), each is owned by the opening file. We can use this ownership
information to both distinguish GGTT and ppGTT address spaces, as well
as occasionally inspect the owner.
v2: Whitespace, tells us who owns i915_address_space
Chris Wilson [Thu, 4 Aug 2016 06:52:24 +0000 (07:52 +0100)]
drm/i915: Rearrange GGTT probing to avoid needing a vfunc
Since we have a static if-else-chain for device probing of the global
GTT, we do not need to use a function pointer, let alone store it when
we never use it again. So use the if-else-chain to call down into the
device specific probe.
Chris Wilson [Thu, 4 Aug 2016 06:52:23 +0000 (07:52 +0100)]
drm/i915: Split early global GTT initialisation
Initialising the global GTT is tricky as we wish to use the drm_mm range
manager during the modesetting initialisation (to capture stolen
allocations from the BIOS) before we actually enable GEM. To overcome
this, we currently setup the drm_mm first and then carefully rebind
them.
v2: Fixup after rebasing
v3: GGTT initialisation needs to be split around kicking out conflicts
v4: Restore an old UMS BUG_ON(mappable > total) as a DRM_ERROR plus
fixup of probe results.
Chris Wilson [Thu, 4 Aug 2016 06:52:22 +0000 (07:52 +0100)]
drm/i915: Update GGTT initialisation functions to take drm_i915_private
Since these are internal functions they operate on drm_i915_private and
not the drm_device being passed in. So pass in the drm_i915_private
instead, and remove one layer of dancing. No space wins here, just
conforming to the norm in function parameters.
Chris Wilson [Thu, 4 Aug 2016 06:52:21 +0000 (07:52 +0100)]
drm/i915: Split GGTT initialisation between probing and setup
In order to handle conflicting drivers (i.e. vgacon) having a different
setup of hardware, we have to remove those other drivers before we try
to setup our own mappings. This requires us to split GGTT initialisation
between probing for the hardware location (part of the PCI BAR) and
later establishing the kernel resources for it.
Chris Wilson [Thu, 4 Aug 2016 06:52:20 +0000 (07:52 +0100)]
drm/i915: Amalgamate GGTT/ppGTT vma debug list walkers
As we can now have multiple VMA inside the global GTT (with partial
mappings, rotations, etc), it is no longer true that there may just be a
single GGTT entry and so we should walk the full vma_list to count up
the actual usage. In addition to unifying the two walkers, switch from
multiplying the object size for each vma to summing the bound vma sizes.
Chris Wilson [Tue, 2 Aug 2016 21:50:39 +0000 (22:50 +0100)]
drm/i915: Simplify calling engine->sync_to
Since requests can no longer be generated as a side-effect of
intel_ring_begin(), we know that the seqno will be unchanged during
ring-emission. This predicatablity then means we do not have to check
for the seqno wrapping around whilst emitting the semaphore for
engine->sync_to().
Now that emitting requests is identical between legacy and execlists, we
can use the same function to build up the ring for submitting to either
engine. (With the exception of i915_switch_contexts(), but in time that
will also be handled gracefully.)
Chris Wilson [Tue, 2 Aug 2016 21:50:37 +0000 (22:50 +0100)]
drm/i915: Refactor golden render state emission to unconfuse gcc
GCC was inlining the init and setup functions, but was getting itself
confused into thinking that variables could be used uninitialised. If we
do the inline for gcc, it is happy! As a bonus we shrink the code.
Chris Wilson [Tue, 2 Aug 2016 21:50:36 +0000 (22:50 +0100)]
drm/i915: Remove duplicate golden render state init from execlists
Now that we use the same vfuncs for emitting the batch buffer in both
execlists and legacy, the golden render state initialisation is
identical between both.
v2: gcc wants so.ggtt_offset initialised (even though it is not used)
Chris Wilson [Tue, 2 Aug 2016 21:50:33 +0000 (22:50 +0100)]
drm/i915: Stop passing caller's num_dwords to engine->semaphore.signal()
Rather than pass in the num_dwords that the caller wishes to use after
the signal command packet, split the breadcrumb emission into two phases
and have both the signal and breadcrumb individiually acquire space on
the ring. This makes the interface simpler for the reader, and will
simplify for patches.
Chris Wilson [Tue, 2 Aug 2016 21:50:32 +0000 (22:50 +0100)]
drm/i915/lrc: Update function names to match request flow
With adding engine->submit_request, we now have a bunch of functions
with similar names used at different stages of the execlist submission.
Try a different coat of paint, to hopefully reduce confusion between the
requests, intel_engine_cs and the actual execlists submision process.
Chris Wilson [Tue, 2 Aug 2016 21:50:31 +0000 (22:50 +0100)]
drm/i915: Unify request submission
Move request submission from emit_request into its own common vfunc
from i915_add_request().
v2: Convert I915_DISPATCH_flags to BIT(x) whilst passing
v3: Rename a few functions to match.
v4: Reenable execlists submission after disabling guc.
v5: Be aware that everyone calls i915_guc_submission_disable()!
Chris Wilson [Tue, 2 Aug 2016 21:50:30 +0000 (22:50 +0100)]
drm/i915: Move the modulus for ring emission to the register write
Space reservation is already safe with respect to the ring->size
modulus, but hardware only expects to see values in the range
0...ring->size-1 (inclusive) and so requires the modulus to prevent us
writing the value ring->size instead of 0. As this is only required for
the register itself, we can defer the modulus to the register update and
not perform it after every command packet. We keep the
intel_ring_advance() around in the code to provide demarcation for the
end-of-packet (which then can be compared against intel_ring_begin() as
the number of dwords emitted must match the reserved space).
v2: Assert that the ring size is a power-of-two to match assumptions in
the code. Simplify the comment before writing the tail value to explain
why the modulus is necessary.
Chris Wilson [Tue, 2 Aug 2016 21:50:29 +0000 (22:50 +0100)]
drm/i915: Convert engine->write_tail to operate on a request
If we rewrite the I915_WRITE_TAIL specialisation for the legacy
ringbuffer as submitting the request onto the ringbuffer, we can unify
the vfunc with both execlists and GuC in the next patch.
v2: Drop the modulus from the I915_WRITE_TAIL as it is currently being
applied in intel_ring_advance() after every command packet, and add a
comment explaining why we need the modulus at all.
Chris Wilson [Tue, 2 Aug 2016 21:50:28 +0000 (22:50 +0100)]
drm/i915: Remove intel_ring_get_tail()
Joonas doesn't like the tiny function, especially if I go around making
it more complicated and using it elsewhere. To remove that temptation,
remove the function!
Chris Wilson [Tue, 2 Aug 2016 21:50:27 +0000 (22:50 +0100)]
drm/i915: Unify legacy/execlists emission of MI_BATCHBUFFER_START
Both the ->dispatch_execbuffer and ->emit_bb_start callbacks do exactly
the same thing, add MI_BATCHBUFFER_START to the request's ringbuffer -
we need only one vfunc.
Chris Wilson [Tue, 2 Aug 2016 21:50:25 +0000 (22:50 +0100)]
drm/i915: Reduce engine->emit_flush() to a single mode parameter
Rather than passing a complete set of GPU cache domains for either
invalidation or for flushing, or even both, just pass a single parameter
to the engine->emit_flush to determine the required operations.
Space for flushing the GPU cache prior to completing the request is
preallocated and so cannot fail - the GPU caches will always be flushed
along with the completed request. This means we no longer have to track
whether the GPU cache is dirty between batches like we had to with the
outstanding_lazy_seqno.
With the removal of the duplication in the per-backend entry points for
emitting the obsolete lazy flush, we can then further unify the
engine->emit_flush.
v2: Expand a bit on the legacy of gpu_caches_dirty
Chris Wilson [Tue, 2 Aug 2016 21:50:21 +0000 (22:50 +0100)]
drm/i915: Rename struct intel_ringbuffer to struct intel_ring
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.
Keith Packard [Sun, 31 Jul 2016 07:54:51 +0000 (00:54 -0700)]
drm/i915: cleanup_plane_fb: also drop reference to current state wait_req
There are two paths into intel_cleanup_plane_fb, the normal completion
path and the failure path.
In the failure case, intel_cleanup_plane_fb is called before
drm_atomic_helper_swap_state, so any wait_req reference made in
intel_prepare_plane_fb will be in old_intel_state->wait_req.
In the normal completion path, drm_atomic_helper_swap_state has
already been called, so the plane state holding the just-used wait_req
will not be in old_intel_state->wait_req, rather it will be in the
state associated with the plane itself.
Clearing this reference ensures that the wait_req will be freed as
soon as it the related mode setting operation is complete, rather than
waiting for some future mode setting operation to eventually
dereference it.
The existing dereference of old_intel_state->wait_req is still
required as that will hold the wait_req when the mode setting
operation fails.
cc: Daniel Vetter <daniel.vetter@intel.com>
cc: David Airlie <airlied@linux.ie>
cc: intel-gfx@lists.freedesktop.org
cc: dri-devel@lists.freedesktop.org Signed-off-by: Keith Packard <keithp@keithp.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
projects / karo-tx-linux.git / log