- 20 Aug, 2014 9 commits
-
-
Damien Lespiau authored
We still have a few missing bits and pieces to have execlists enabled by default eg. the error capture or the render state initialization and so it wouldn't be wise to enable it by default on BDW just yet. Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Thomas Daniel <thomas.daniel@intel.com> Signed-off-by:
Damien Lespiau <damien.lespiau@intel.com> Reviewed-by:
Paulo Zanoni <paulo.r.zanoni@intel.com> Tested-by:
Daniel Vetter <daniel.vetter@ffwll.ch>
-
Oscar Mateo authored
The time has come, the Walrus said, to talk of many things. Signed-off-by:
Oscar Mateo <oscar.mateo@intel.com> Reviewed-by:
-
Oscar Mateo authored
Add theory of operation notes to intel_lrc.c and comments to externally visible functions. v2: Add notes on logical ring context creation. v3: Use kerneldoc. v4: Integrate it in the DocBook template. Signed-off-by: Thomas Daniel <thomas.daniel@intel.com> (v1) Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> (v2, v3) Reviewed-by:
-
Ben Widawsky authored
This has turned out to be really handy in debug so far. Update: Since writing this patch, I've gotten similar code upstream for error state. I've used it quite a bit in debugfs however, and I'd like to keep it here at least until preemption is working. Signed-off-by:
Ben Widawsky <ben@bwidawsk.net> This patch was accidentally dropped in the first Execlists version, and it has been very useful indeed. Put it back again, but as a standalone debugfs file. Signed-off-by:
Thomas Daniel <thomas.daniel@intel.com> Reviewed-by:
-
Oscar Mateo authored
Signed-off-by:
-
Oscar Mateo authored
v2: Warn and return if LRCs are not enabled. v3: Grab the Execlists spinlock (noticed by Daniel Vetter). Signed-off-by:
-
Oscar Mateo authored
Up until recently, semaphores weren't enabled in BDW so we didn't care about them. But then Rodrigo came and enabled them: commit 521e62e4 Author: Rodrigo Vivi <rodrigo.vivi@intel.com> drm/i915: Enable semaphores on BDW So now we have to explicitly disable them for Execlists until both features play nicely. Signed-off-by:
-
Oscar Mateo authored
If we reset a ring after a hang, we have to make sure that we clear out all queued Execlists requests. v2: The ring is, at this point, already being correctly re-programmed for Execlists, and the hangcheck counters cleared. v3: Daniel suggests to drop the "if (execlists)" because the Execlists queue should be empty in legacy mode (which is true, if we do the INIT_LIST_HEAD). v4: Do the pending intel_runtime_pm_put Signed-off-by:
-
Oscar Mateo authored
This is mostly for correctness so that we know we are running the LR context correctly (this is, the PDPs are contained inside the context object). v2: Move the check to inside the enable PPGTT function. The switch happens in two places: the legacy context switch (that we won't hit when Execlists are enabled) and the PPGTT enable, which unfortunately we need. This would look much nicer if the ppgtt->enable was part of the ring init, where it logically belongs. v3: Move the check to the start of the enable PPGTT function. None of the legacy PPGTT enabling is required when using LRCs as the PPGTT is enabled in the context descriptor and the PDPs are written in the LRC. v4: Clarify comment based on review feedback. Signed-off-by:
-
- 14 Aug, 2014 12 commits
-
-
Daniel Vetter authored
We treat other plane updates in the same fashion. Spotted because Rodrigo kept reporting a bug in the PSR code where the frontbuffer was eternally stuck with a dirty cursor bit set. The psr testcase should have caught this, but that i-g-t is kaputt. Rodrigo is signed up to fix that. Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Tested-by-and-Reviewed-by:
Rodrigo Vivi <rodrigo.vivi@intel.com> Signed-off-by:
-
Oscar Mateo authored
If we receive a storm of requests for the same context (see gem_storedw_loop_*) we might end up iterating over too many elements in interrupt time, looking for contexts to squash together. Instead, share the burden by giving more intelligence to the queue function. At most, the interrupt will iterate over three elements. Signed-off-by:
-
Oscar Mateo authored
In the current Execlists feeding mechanism, full preemption is not supported yet: only lite-restores are allowed (this is: the GPU simply samples a new tail pointer for the context currently in execution). But we have identified an scenario in which a full preemption occurs: 1) We submit two contexts for execution (A & B). 2) The GPU finishes with the first one (A), switches to the second one (B) and informs us. 3) We submit B again (hoping to cause a lite restore) together with C, but in the time we spend writing to the ELSP, the GPU finishes B. 4) The GPU start executing B again (since we told it so). 5) We receive a B finished interrupt and, mistakenly, we submit C (again) and D, causing a full preemption of B. The race is avoided by keeping track of how many times a context has been submitted to the hardware and by better discriminating the received context switch interrupts: in the example, when we have submitted B twice, we won´t submit C and D as soon as we receive the notification that B is completed because we were expecting to get a LITE_RESTORE and we didn´t, so we know a second completion will be received shortly. Without this explicit checking, somehow, the batch buffer execution order gets messed with. This can be verified with the IGT test I sent together with the series. I don´t know the exact mechanism by which the pre-emption messes with the execution order but, since other people is working on the Scheduler + Preemption on Execlists, I didn´t try to fix it. In these series, only Lite Restores are supported (other kind of preemptions WARN). v2: elsp_submitted belongs in the new intel_ctx_submit_request. Several rebase changes. v3: Clarify how the race is avoided, as requested by Daniel. Signed-off-by:
-
Thomas Daniel authored
Handle all context status events in the context status buffer on every context switch interrupt. We only remove work from the execlist queue after a context status buffer reports that it has completed and we only attempt to schedule new contexts on interrupt when a previously submitted context completes (unless no contexts are queued, which means the GPU is free). We canot call intel_runtime_pm_get() in an interrupt (or with a spinlock grabbed, FWIW), because it might sleep, which is not a nice thing to do. Instead, do the runtime_pm get/put together with the create/destroy request, and handle the forcewake get/put directly. Signed-off-by:
-
Michel Thierry authored
Context switch (and execlist submission) should happen only when other contexts are not active, otherwise pre-emption occurs. To assure this, we place context switch requests in a queue and those request are later consumed when the right context switch interrupt is received (still TODO). v2: Use a spinlock, do not remove the requests on unqueue (wait for context switch completion). Signed-off-by:
-
Oscar Mateo authored
Each logical ring context has the tail pointer in the context object, so update it before submission. v2: New namespace. Signed-off-by:
-
Ben Widawsky authored
A context switch occurs by submitting a context descriptor to the ExecList Submission Port. Given that we can now initialize a context, it's possible to begin implementing the context switch by creating the descriptor and submitting it to ELSP (actually two, since the ELSP has two ports). The context object must be mapped in the GGTT, which means it must exist in the 0-4GB graphics VA range. Signed-off-by:
-
Oscar Mateo authored
On a previous iteration of this patch, I created an Execlists version of __i915_add_request and asbtracted it away as a vfunc. Daniel Vetter wondered then why that was needed: "with the clean split in command submission I expect every function to know wether it'll submit to an lrc (everything in intel_lrc.c) or wether it'll submit to a legacy ring (existing code), so I don't see a need for an add_request vfunc." The honest, hairy truth is that this patch is the glue keeping the whole logical ring puzzle together: - i915_add_request is used by intel_ring_idle, which in turn is used by i915_gpu_idle, which in turn is used in several places inside the eviction and gtt codes. - Also, it is used by i915_gem_check_olr, which is littered all over i915_gem.c - ... If I were to duplicate all the code that directly or indirectly uses __i915_add_request, I'll end up creating a separate driver. To show the differences between the existing legacy version and the new Execlists one, this time I have special-cased __i915_add_request instead of adding an add_request vfunc. I hope this helps to untangle this Gordian knot. Signed-off-by:
-
Oscar Mateo authored
The execlist patches have a bit a convoluted and long history and due to that have the actual submission still misplaced deeply burried in the low-level ringbuffer handling code. This design goes back to the legacy ringbuffer code with its tricky lazy request and simple work submissiion using ring tail writes. For that reason they need a ring->ctx backpointer. The goal is to unburry that code and move it up into a level where the full execlist context is available so that we can ditch this backpointer. Until that's done make it really obvious that there's work still to be done. Cc: Oscar Mateo <oscar.mateo@intel.com> Cc: Thomas Daniel <thomas.daniel@intel.com> Acked-by:
-
Chris Wilson authored
The current error state harks back to the era of just a single VM. For full-ppgtt, we capture every bo on every VM. It behoves us to then print every bo for every VM, which we currently fail to do and so miss vital information in the error state. v2: Use the vma address rather than -1! Signed-off-by:
Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by:
Mika Kuoppala <mika.kuoppala@intel.com> Signed-off-by:
-
Sagar Kamble authored
On VLV, post S0i3 during i915_drm_thaw following issue is observed during ring initialization. [ 335.604039] [drm:stop_ring] ERROR render ring :timed out trying to stop ring [ 336.607340] [drm:stop_ring] ERROR render ring :timed out trying to stop ring [ 336.607345] [drm:init_ring_common] ERROR failed to set render ring head to zero ctl 00000000 head 00000000 tail 00000000 start 00000000 [ 337.610645] [drm:stop_ring] ERROR bsd ring :timed out trying to stop ring [ 338.613952] [drm:stop_ring] ERROR bsd ring :timed out trying to stop ring [ 338.613956] [drm:init_ring_common] ERROR failed to set bsd ring head to zero ctl 00000000 head 00000000 tail 00000000 start 00000000 [ 339.617256] [drm:stop_ring] ERROR render ring :timed out trying to stop ring [ 339.617258] -----------[ cut here ]----------- [ 339.617267] WARNING: CPU: 0 PID: 6 at drivers/gpu/drm/i915/intel_ringbuffer.c:1666 intel_cleanup_ring+0xe6/0xf0() [ 339.617396] --[ end trace 5ef5ed1a3c92e2a6 ]-- [ 339.617428] [drm:__i915_drm_thaw] ERROR failed to re-initialize GPU, declaring wedged! This is happening since wake is not enabled and Gunit registers are not restored. For this system suspend/resume paths need to follow save/restore and additional platform specific setup in suspend_complete and resume_prepare. suspend_complete is shared unconditionaly for VLV, HSW, BDW. resume_prepare for HSW and BDW has pc8 disabling which is needed during thaw_early so sharing uncondtionally. For VLV and SNB runtime resume specific sequence exists. Cc: Imre Deak <imre.deak@intel.com> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Jani Nikula <jani.nikula@linux.intel.com> Cc: Goel, Akash <akash.goel@intel.com> Signed-off-by:
Sagar Kamble <sagar.a.kamble@intel.com> Reviewed-by:
Imre Deak <imre.deak@intel.com> Signed-off-by:
-
Sagar Kamble authored
With this change, intel_runtime_suspend and intel_runtime_resume functions become completely platform agnostic. Platform specific suspend/resume changes are moved to intel_suspend_complete and intel_resume_prepare. Cc: Imre Deak <imre.deak@intel.com> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Jani Nikula <jani.nikula@linux.intel.com> Cc: Goel, Akash <akash.goel@intel.com> Signed-off-by:
-
- 13 Aug, 2014 18 commits
-
-
Chris Wilson authored
Rather than take and release the console_lock() around a non-existent DRM_I915_FBDEV, move the lock acquisation into the callee where it will be compiled out by the config option entirely. This includes moving the deferred fb_set_suspend() dance and encapsulating it entirely within intel_fbdev.c. v2: Use an integral work item so that we can explicitly flush the work upon suspend/unload. Signed-off-by:
-
Chris Wilson authored
Ville pointed out the GCCism __builtin_types_compatible_p() that we could use to replace our heavily casted presumption __I915__ macro that was based on comparing struct sizes. Signed-off-by:
-
Ville Syrjälä authored
845/865 support different cursor sizes as well, albeit a bit differently than later platforms. Add the necessary code to make them work. Untested due to lack of hardware. v2: Warn but accept invalid stride (Chris) Rewrite the cursor size checks for other platforms (Chris) v3: More polish and magic to the cursor size checks (Chris) v4: Moar polish and a comment (Chris) Reviewed-by:Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by:
-
Ville Syrjälä authored
Ever since commit 5efb3e28 Author: Ville Syrjälä <ville.syrjala@linux.intel.com> Date: Wed Apr 9 13:28:53 2014 +0300 drm/i915/chv: Add cursor pipe offsets the only difference between i9xx_update_cursor() and ivb_update_cursor() was the hsw+ pipe csc handling. Let's unify them and we can rid outselves of some duplicated code. Signed-off-by:
-
Ville Syrjälä authored
CURSIZE register exists on 845/865 only, so move it to i845_update_cursor(). Changes to cursor size must be done only when the cursor is disabled, so do the write just before enabling the cursor. Signed-off-by:
-
Ville Syrjälä authored
Make sure the cursor gets fully clipped when enabling it on a disabled crtc via setplane. This will prevent the lower level code from attempting to enable the cursor in hardware. Cc: Paulo Zanoni <przanoni@gmail.com> Signed-off-by:
-
Daniel Vetter authored
Also remove related WARN_ONs which seem to have been hit since a rather long time. But apperently no one noticed since our module reload is already WARNING-infested :( Reviewed-by:
Michel Thierry <michel.thierry@intel.com> Signed-off-by:
-
Daniel Vetter authored
We want to move the aliasing ppgtt cleanup back into the global gtt cleanup code for symmetry, but first we need to create such a place. Reviewed-by:
-
Daniel Vetter authored
Address space cleanup isn't really a job for the low-level cleanup callbacks. Without this change we can't reuse the low-level cleanup callback for the aliasing ppgtt cleanup. Reviewed-by:
-
Daniel Vetter authored
Now that all the flow is streamlined the rule is simple: We create a new ppgtt for a new context when we have full ppgtt enabled. Reviewed-by:
-
Daniel Vetter authored
There's a bit a confusion since we track the global gtt, the aliasing and real ppgtt in the ctx->vm pointer. And not all callers really bother to check for the different cases and just presume that it points to a real ppgtt. Now looking closely we don't actually need ->vm to always point at an address space - the only place that cares actually has fixup code already to decide whether to look at the per-proces or the global address space. So switch to just tracking the ppgtt directly and ditch all the extraneous code. v2: Fixup the ppgtt debugfs file to not oops on a NULL ctx->ppgtt. Also drop the early exit - without aliasing ppgtt we want to dump all the ppgtts of the contexts if we have full ppgtt. v3: Actually git add the compile fix. Reviewed-by:
-
Daniel Vetter authored
Stuffing this into the context setup code doesn't make a lot of sense. Also reusing the real ppgtt setup code makes even less sense since the aliasing ppgtt isn't a real address space. Leaving all that stuff unitialized will make sure that we catch any abusers promptly. This is also a prep work to clean up the context->ppgtt link. v2: Fix up the logic fail, I've fumbled it so badly to completely disable ppgtt on gen6. Spotted by Ville and Michel. Also move around the pde write into the gen6 init function, since otherwise it won't work at all. v3: Only initialize the aliasing ppgtt when we actually enable it. Cc: "Thierry, Michel" <michel.thierry@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by:
-
Daniel Vetter authored
Currently we abuse the aliasing ppgtt to set up the ppgtt support in general. Which is a bit backwards since with full ppgtt we don't ever need the aliasing ppgtt. So untangle this and separate the ppgtt init from the aliasing ppgtt. While at it drag it out of the context enabling (which just does a switch to the default context). Note that we still have the differentiation between synchronous and asynchronous ppgtt setup, but that will soon vanish. So also correctly wire up the return value handling to be prepared for when ->switch_mm drops the synchronous parameter and could start to fail. Reviewed-by:
-
Daniel Vetter authored
A subsequent patch will no longer initialize the aliasing ppgtt if we have full ppgtt enabled, since we simply don't need that any more. Unfortunately a few places check for the aliasing ppgtt instead of checking for ppgtt in general. Fix them up. One special case are the gtt offset and size macros, which have some code to remap the aliasing ppgtt to the global gtt. The aliasing ppgtt is _not_ a logical address space, so passing that in as the vm is plain and simple a bug. So just WARN about it and carry on - we have a gracefully fall-through anyway if we can't find the vma. Reviewed-by:
-
Daniel Vetter authored
We already needs this just as a safety check in case the preallocation reservation dance fails. But we definitely need this to be able to move tha aliasing ppgtt setup back out of the context code to this place, where it belongs. Reviewed-by:
-
Daniel Vetter authored
Stuff in headers really aught to have this. Reviewed-by:
-
Daniel Vetter authored
This essentially unbreaks non-ppgtt operation where we'd scribble over random memory. While at it give the vm_to_ppgtt function a proper prefix and make it a bit more paranoid. Reviewed-by:
-
Daniel Vetter authored
Hardware contexts reference a ppgtt, not the other way round. And the only user of this (in debugfs) actually only cares about which file the ppgtt is associated with. So give it what it wants. While at it give the ppgtt create function a proper name&place. Reviewed-by:
-
- 12 Aug, 2014 1 commit
-
-
Daniel Vetter authored
So when reviewing Michel's patch I've noticed a few things and cleaned them up: - The early checks in ppgtt_release are now redundant: The inactive list should always be empty now, so we can ditch these checks. Even for the aliasing ppgtt (though that's a different confusion) since we tear that down after all the objects are gone. - The ppgtt handling functions are splattered all over. Consolidate them in i915_gem_gtt.c, give them OCD prefixes and add wrappers for get/put. - There was a bit a confusion in ppgtt_release about whether it cares about the active or inactive list. It should care about them both, so augment the WARNINGs to check for both. There's still create_vm_for_ctx left to do, put that is blocked on the removal of ppgtt->ctx. Once that's done we can rename it to i915_ppgtt_create and move it to its siblings for handling ppgtts. v2: Move the ppgtt checks into the inline get/put functions as suggested by Chris. v3: Inline the now redundant ppgtt local variable. Cc: Michel Thierry <michel.thierry@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by:
-
