- 04 Aug, 2016 9 commits
-
-
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 <chris@chris-wilson.co.uk> Reviewed-by:
Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1470293567-10811-9-git-send-email-chris@chris-wilson.co.uk
-
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:
Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1470293567-10811-8-git-send-email-chris@chris-wilson.co.uk
-
Chris Wilson authored
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. Signed-off-by:
-
Chris Wilson authored
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 Signed-off-by:
-
Chris Wilson authored
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. Signed-off-by:
-
Chris Wilson authored
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. Signed-off-by:
-
Chris Wilson authored
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. v2: Include all the probe functions Signed-off-by:
-
Chris Wilson authored
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. Signed-off-by:
-
Chris Wilson authored
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. Signed-off-by:
-
- 03 Aug, 2016 2 commits
-
-
Ville Syrjälä authored
Remove the CHV early bail out from intel_cleanup_gt_powersave() so that we'll clean up the extra RPM reference held due to i915.enable_rc6=0. Cc: Imre Deak <imre.deak@intel.com> Fixes: b268c699 ("drm/i915: refactor RPM disabling due to RC6 being disabled") Signed-off-by:
Ville Syrjälä <ville.syrjala@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1470136053-23276-1-git-send-email-ville.syrjala@linux.intel.comReviewed-by:
Imre Deak <imre.deak@intel.com>
-
Chris Wilson authored
Now that we initialize the state to both legacy and execlists inside intel_engine_cs, we should also clean up that state from the common functions. Signed-off-by:
-
- 02 Aug, 2016 29 commits
-
-
Chris Wilson authored
In order to be more consistent with the rest of the request construction and ring emission, use the common names for the ring and request. Rather than using signaler_req, waiter_req, and intel_ring *wait, we use plain req and ring. Signed-off-by:
-
Chris Wilson authored
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(). Signed-off-by:
-
Chris Wilson authored
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.) Signed-off-by:
-
Chris Wilson authored
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. v2: A couple of minor tweaks from Joonas Signed-off-by:
-
Chris Wilson authored
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) Signed-off-by:
-
Chris Wilson authored
As gen6_emit_request() only differs from i9xx_emit_request() when semaphores are enabled, only use the specialised vfunc in that scenario. v2: Reorder semaphore init so as to keep engine->emit_request default vfunc selection compact. Signed-off-by:
-
Chris Wilson authored
As GEN6+ is now a simple variant on the basic breadcrumbs + tail write, reuse the common code. Signed-off-by:
-
Chris Wilson authored
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. Signed-off-by:
-
Chris Wilson authored
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. Signed-off-by:
-
Chris Wilson authored
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()! Signed-off-by:
-
Chris Wilson authored
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. Signed-off-by:
-
Chris Wilson authored
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. Signed-off-by:
-
Chris Wilson authored
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! Signed-off-by:
-
Chris Wilson authored
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. Signed-off-by:
-
Chris Wilson authored
If is simpler and leads to more readable code through the callstack if the allocation returns the allocated struct through the return value. The importance of this is that it no longer looks like we accidentally allocate requests as side-effect of calling certain functions. Signed-off-by:
-
Chris Wilson authored
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. engine->emit_flush(GPU, 0) -> engine->emit_flush(EMIT_INVALIDATE) engine->emit_flush(0, GPU) -> engine->emit_flush(EMIT_FLUSH) engine->emit_flush(GPU, GPU) -> engine->emit_flush(EMIT_FLUSH | EMIT_INVALIDATE) This allows us to extend the behaviour easily in future, for example if we want just a command barrier without the overhead of flushing. Signed-off-by:
-
Chris Wilson authored
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 Signed-off-by:
-
Chris Wilson authored
For more consistent oop-naming, we would use intel_ring_verb, so pick intel_ring_pin() and intel_ring_unpin(). Signed-off-by:
-
Chris Wilson authored
Now that we have a clear ring/engine split and a struct intel_ring, we no longer need the stopgap ringbuf names. Signed-off-by:
-
Chris Wilson authored
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:
-
Chris Wilson authored
Perform s/ringbuf/ring/ on the context struct for consistency with the ring/engine split. v2: Kill an outdated error_ringbuf label Signed-off-by:
-
Chris Wilson authored
Now that we have disambuigated ring and engine, we can use the clearer and more consistent name for the intel_ringbuffer pointer in the request. @@ struct drm_i915_gem_request *r; @@ - r->ringbuf + r->ring Signed-off-by:
-
Chris Wilson authored
Both perform the same actions with more or less indirection, so just unify the code. v2: Add back a few intel_engine_cs locals Signed-off-by:
-
Akash Goel authored
Updated the i915_drpc_info debugfs with coarse power gating & forcewake info for Gen9. v2: Change all IS_GEN9() by gen >= 9 (Damien) v3: Rebase Cc: Damien Lespiau <damien.lespiau@intel.com> Signed-off-by:
Akash Goel <akash.goel@intel.com> Reviewed-by:
Sagar Arun Kamble <sagar.a.kamble@intel.com> Signed-off-by:
Daniel Vetter <daniel.vetter@ffwll.ch> Link: http://patchwork.freedesktop.org/patch/msgid/1467038401-8283-1-git-send-email-akash.goel@intel.com
-
Keith Packard authored
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:
-
Ville Syrjälä authored
If we have have a buffer, we should also have a size, and vice versa. Let's check it both ways instead of just one. Signed-off-by:
-
Ville Syrjälä authored
According to Bspec FW_BLC_SELF exists on 915G also. Let's program it. The only open question is whether there's is a memory self-refresh enable bit somewhere as well. For 945G/GM it's in FW_BLC_SELF, for 915GM it's in INSTPM. For 915G I can't find one in the docs. Let's drop a FIXME about this, in case someone with the hardware is ever bored enough to look for it. Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by:
-
Ville Syrjälä authored
Bspec says: "FW_BLC_SELF ... Programming Note [DevALV] and [DevCST]: When calculating watermark values for 15/16bpp, assume 32bpp for purposes of calculation using the high priority bandwidth analysis spreadsheet." Let's do that. Perhaps this might even help with the problem that resulted in commit 2ab1bc9d ("drm/i915: Disable self-refresh for untiled fbs on i915gm") Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by:
-
Ville Syrjälä authored
Make the BDW and SKL code a bit more similar by extracting the low vswing handling for BDW into a helper, as we already have it like that for SKL+. Cc: Mika Kahola <mika.kahola@intel.com> Signed-off-by:
-
