- 04 Aug, 2016 40 commits
-
-
Chris Wilson authored
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. 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/1470324762-2545-8-git-send-email-chris@chris-wilson.co.uk
-
Chris Wilson authored
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 Signed-off-by:
Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1470324762-2545-7-git-send-email-chris@chris-wilson.co.uk
-
Chris Wilson authored
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) Signed-off-by:
-
Chris Wilson authored
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. Signed-off-by:
-
Chris Wilson authored
This reimplements the denial-of-service protection against igt from commit 227f782e ("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 9b5f4e5e ("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. Signed-off-by:
-
Chris Wilson authored
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 Signed-off-by:
-
Chris Wilson authored
Eviction is VM local, so we can ignore the significance of the drm_device in the caller, and leave it to i915_gem_evict_something() to manage itself. Signed-off-by:
-
Chris Wilson authored
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. Signed-off-by:
-
Chris Wilson authored
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: 03b135ce "ALSA: hda - remove dependency on i915 power well for SKL") Signed-off-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.ukReviewed-by:
Daniel Vetter <daniel.vetter@ffwll.ch>
-
Ville Syrjälä authored
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... Signed-off-by:
Ville Syrjälä <ville.syrjala@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1469717448-4297-12-git-send-email-ville.syrjala@linux.intel.comReviewed-by:
-
Ville Syrjälä authored
No need to iterate the rates array in intel_dp_max_link_rate(). We know the max rate will be the last entry, and we already know the size. 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ä authored
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ä authored
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ä authored
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. Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Signed-off-by:
Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Reviewed-by:
-
Ville Syrjälä authored
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'. Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Signed-off-by:
-
Ville Syrjälä authored
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 Cc: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: -
Chris Wilson authored
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: b50a5371 ("drm/i915: Support for pread/pwrite ...") Reported-by:
-
Chris Wilson authored
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)) Signed-off-by: -
Chris Wilson authored
Since commit de1add36 ("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: de1add36 ("drm/i915: Decouple execbuf uAPI from internal...") Signed-off-by:
-
Chris Wilson authored
This reverts commit e9f24d5f. 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. Signed-off-by:
-
Chris Wilson authored
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. Signed-off-by:
-
Chris Wilson authored
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. Testcase: igt/gem_ppggtt/flink-and-close-vma-leak Signed-off-by:
-
Chris Wilson authored
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() Signed-off-by:
-
Chris Wilson authored
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). Signed-off-by:
-
Chris Wilson authored
As the list retirement is now clean of implementation details, we can move it closer to the request management. Signed-off-by:
-
Chris Wilson authored
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. Signed-off-by:
-
Chris Wilson authored
There is only one wait on request function now, so drop the "expert" indication of leading __. Signed-off-by:
-
Chris Wilson authored
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. Signed-off-by:
-
Chris Wilson authored
Signed-off-by:
-
Chris Wilson authored
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. v2: Now using i915_gem_retire_fn typedef References: https://bugs.freedesktop.org/show_bug.cgi?id=93730 References: https://bugs.freedesktop.org/show_bug.cgi?id=96851Signed-off-by:
-
Chris Wilson authored
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.) Signed-off-by:
-
Chris Wilson authored
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. Signed-off-by:
-
Chris Wilson authored
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. Signed-off-by:
-
Chris Wilson authored
We use "list" to denote the list and "link" to denote an element on that list. Rename request->list to match this idiom. Signed-off-by:
-
Chris Wilson authored
Tidy up the for loops that handle waiting for read/write vs read-only access. Signed-off-by:
-
Chris Wilson authored
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. Before: 6298417 3619610 696320 10614347 a1f64b vmlinux After: 6298417 3619610 696320 10614347 a1f64b vmlinux (with i915 builtin) Signed-off-by:
-
Chris Wilson authored
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. Signed-off-by:
-
Chris Wilson authored
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 Signed-off-by:
-
Chris Wilson authored
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. Signed-off-by:
-
Chris Wilson authored
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. Signed-off-by:
-
