- 13 Jul, 2020 5 commits
-
-
Chris Wilson authored
Preempt-to-busy introduces various fascinating complications in that the requests may complete as we are unsubmitting them from HW. As they may then signal after unsubmission, we may find ourselves having to cleanup the signaling request from within the signaling callback. This causes us to recurse onto the same i915_request.lock. However, if the request is already signaled (as it will be before we enter the signal callbacks), we know we can skip the signaling of that request during submission, neatly evading the spinlock recursion. unsubmit(ve.rq0) # timeslice expiration or other preemption -> virtual_submit_request(ve.rq0) dma_fence_signal(ve.rq0) # request completed before preemption ack -> submit_notify(ve.rq1) -> virtual_submit_request(ve.rq1) # sees that we have completed ve.rq0 -> __i915_request_submit(ve.rq0) [ 264.210142] BUG: spinlock recursion on CPU#2, sample_multi_tr/2093 [ 264.210150] lock: 0xffff9efd6ac55080, .magic: dead4ead, .owner: sample_multi_tr/2093, .owner_cpu: 2 [ 264.210155] CPU: 2 PID: 2093 Comm: sample_multi_tr Tainted: G U [ 264.210158] Hardware name: Intel Corporation CoffeeLake Client Platform/CoffeeLake S UDIMM RVP, BIOS CNLSFWR1.R00.X212.B01.1909060036 09/06/2019 [ 264.210160] Call Trace: [ 264.210167] dump_stack+0x98/0xda [ 264.210174] spin_dump.cold+0x24/0x3c [ 264.210178] do_raw_spin_lock+0x9a/0xd0 [ 264.210184] _raw_spin_lock_nested+0x6a/0x70 [ 264.210314] __i915_request_submit+0x10a/0x3c0 [i915] [ 264.210415] virtual_submit_request+0x9b/0x380 [i915] [ 264.210516] submit_notify+0xaf/0x14c [i915] [ 264.210602] __i915_sw_fence_complete+0x8a/0x230 [i915] [ 264.210692] i915_sw_fence_complete+0x2d/0x40 [i915] [ 264.210762] __dma_i915_sw_fence_wake+0x19/0x30 [i915] [ 264.210767] dma_fence_signal_locked+0xb1/0x1c0 [ 264.210772] dma_fence_signal+0x29/0x50 [ 264.210871] i915_request_wait+0x5cb/0x830 [i915] [ 264.210876] ? dma_resv_get_fences_rcu+0x294/0x5d0 [ 264.210974] i915_gem_object_wait_fence+0x2f/0x40 [i915] [ 264.211084] i915_gem_object_wait+0xce/0x400 [i915] [ 264.211178] i915_gem_wait_ioctl+0xff/0x290 [i915] Fixes: 22b7a426 ("drm/i915/execlists: Preempt-to-busy") References: 6d06779e ("drm/i915: Load balancing across a virtual engine") Signed-off-by:Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: "Nayana, Venkata Ramana" <venkata.ramana.nayana@intel.com> Cc: <stable@vger.kernel.org> # v5.4+ Reviewed-by:
Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200713141636.29326-1-chris@chris-wilson.co.uk
-
Chris Wilson authored
The danger in switching at random upon intel_context_pin is that the context may still actually be inflight, as it will not be scheduled out until a context switch after it is complete -- that may be a long time after we do a final intel_context_unpin. Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2118 Fixes: 6d06779e ("drm/i915: Load balancing across a virtual engine") Signed-off-by:
-
Colin Ian King authored
There is an error condition where err is not being set and an uninitialized garbage value in err is being returned. Fix this by assigning err to an appropriate error return value before taking the error exit path. Addresses-Coverity: ("Uninitialized scalar value") Fixes: ed2690a9 ("drm/i915/selftest: Check that GPR are restored across noa_wait") Signed-off-by:Colin Ian King <colin.king@canonical.com> Reviewed-by:
-
Chris Wilson authored
We do not use the virtual engines for interrupts (they have physical components), but we do use them to decouple the fence signaling during submission. Currently, when we submit a completed request, we try to enable the interrupt handler for the virtual engine, but we never disarm it. A quick fix is then to mark the irq as enabled, and it will then remain enabled -- and this prevents us from waking the device and never letting it sleep again. Fixes: f8db4d05 ("drm/i915: Initialise breadcrumb lists on the virtual engine") Signed-off-by:
-
Chris Wilson authored
If something has gone awry with the CSB processing, we need to pause, unwind and restart the request submission and event processing. However, currently we skip the engine reset if we raise an error but discover no active context, in the mistaken belief that it was merely a glitch in the matrix. The glitches are real enough, and we do need to unwind even if the engine appears idle (as it has gone permanently idle!) The simplest way to unwind and recover is simply do the engine reset, which should be very fast and _safe_ as nothing is active. Signed-off-by:
Mika Kuoppala <mika.kuoppala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200711091349.28865-1-chris@chris-wilson.co.uk
-
- 10 Jul, 2020 4 commits
-
-
Chris Wilson authored
We try not to assume that we captured any information, and so have to check that error->gt exists before reporting. This check was missed in err_print_capabilities, so lets break up the capability info and push it into the GT dump. We are still a long way from yamlifying this output! Reported-by:
Dan Carpenter <dan.carpenter@oracle.com> Fixes: 792592e7 ("drm/i915: Move the engine mask to intel_gt_info") Signed-off-by:
Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200710193239.5419-1-chris@chris-wilson.co.uk
-
Chris Wilson authored
If the HW throws a curve ball and reports either en event before it is possible, or just a completely impossible event, we have to grin and bear it. The first few events, we will likely not notice as we would be expecting some event, but as soon as we stop expecting an event and yet they still keep coming, then we enter into undefined state territory. In which case, bail out, stop processing the events, and reset the engine and our set of queued requests to recover. The sporadic hangs and warnings will continue to plague CI, but at least system stability should not be compromised. v2: Commentary and force the reset-on-error. v3: Customised user facing message for forced resets from internal errors. Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2045Signed-off-by:
-
Chris Wilson authored
Perf implements a GPU delay (noa_wait) by looping until the CS timestamp has passed a certain point. This use MI_MATH and the general purpose registers of the user's context, and since it is clobbering the user state it must carefully save and restore the user's data around the noa_wait. We can verify this by loading some values in the GPR that we know will be clobbered by the noa_wait, and then inspecting the GPR after the noa_wait completes and confirming that they have been restored. Signed-off-by:
Lionel Landwerlin <lionel.g.landwerlin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200709224504.11345-2-chris@chris-wilson.co.uk
-
Umesh Nerlige Ramappa authored
MI_STORE_REGISTER_MEM and MI_LOAD_REGISTER_MEM need to know which translation to use when saving restoring the engine general purpose registers to and from the GT scratch. Since GT scratch is mapped to ggtt, we need to set an additional bit in the command to use GTT. Fixes: daed3e44 ("drm/i915/perf: implement active wait for noa configurations") Suggested-by:
Prathap Kumar Valsan <prathap.kumar.valsan@intel.com> Signed-off-by:
Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> Reviewed-by:
-
- 09 Jul, 2020 16 commits
-
-
Chris Wilson authored
Since the aliasing-ppgtt remains the default for gen6/gen7, it is worth optimising the ppgtt allocation for it. In this case, we do not need to flush the GGTT page directories entries as they are fixed during setup. Signed-off-by:
Matthew Auld <matthew.auld@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200709190111.5492-1-chris@chris-wilson.co.uk
-
José Roberto de Souza authored
HOBL means hours of battery life, it is a power-saving feature were supported motherboards can use a special voltage swing table that uses less power. So here parsing the VBT to check if this feature is supported. BSpec: 20150 Reviewed-by:
Anusha Srivatsa <anusha.srivatsa@intel.com> Signed-off-by:
José Roberto de Souza <jose.souza@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200708205512.21625-3-jose.souza@intel.com
-
José Roberto de Souza authored
This information can be get directly from intel_encoder so no need of those parameters. Signed-off-by:
Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200708205512.21625-2-jose.souza@intel.com
-
José Roberto de Souza authored
intel_encoder will be needed inside of vswing functions in a future patch, so here doing this change in all vswing functions since HSW. Signed-off-by:
-
Sudeep Holla authored
Both cmp_u32 and cmp_u64 are comparing the pointers instead of the value at those pointers. This will result in incorrect/unsorted list. Fix it by deferencing the pointers before comparison. Fixes: 4ba74e53 ("drm/i915/selftests: Verify frequency scaling with RPS") Fixes: 8757797f ("drm/i915/selftests: Repeat the rps clock frequency measurement") Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Signed-off-by:
Sudeep Holla <sudeep.holla@arm.com> Reviewed-by:
-
José Roberto de Souza authored
As today those 2 WAs have different implementation between TGL and DG1 WA pages but checking the HSD it is clear that DG1 implementation should be used for both, also to do so is easier as we just need to extend WA 1407928979 to B* stepping. Both WAs are need to fix some possible render corruptions. DG1 initial patches were not merged yet, as soon it is this WAs should be applied to DG1 as well. BSpec: 53508 BSpec: 52890 Cc: Lucas De Marchi <lucas.demarchi@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by:
-
Ville Syrjälä authored
Pimp the comments for the FBC related workarounds. Signed-off-by:
-
Ville Syrjälä authored
Supposedly only skl/bxt need WaFbcHighMemBwCorruptionAvoidance. Do not apply to the other gen9 platforms. Signed-off-by:
-
Ville Syrjälä authored
GLK supposedly does not need WaFbcTurnOffFbcWatermark, so let's not apply it. Signed-off-by:
-
Ville Syrjälä authored
Some platforms apply the FBC w/as in .init_clock_gating(), some in fbc_activate(). Move them all to .init_clock_gating() for consistentce. Also safer since we don't have to worry about the RMWs clashing with any other runtime use of the same registers. Signed-off-by:
-
Ville Syrjälä authored
.get_modes() is supposed to return the number of modes added to the probed_modes list (not that anyone actually checks for anything except zero vs. not zero). Let's do that. Signed-off-by:
Imre Deak <imre.deak@intel.com>
-
Ville Syrjälä authored
.get_modes() is supposed to return the number of modes added to the probed_modes list (not that anyone actually checks for anything except zero vs. not zero). Let's do that. Also switch over to using intel_connector_update_modes() instead of hand rolling it. Signed-off-by:
-
Ville Syrjälä authored
With SDVO the pipe config pixel_multiplier only concerns itself with the data on the SDVO bus. Any HDMI specific pixel repeat must be handled by the SDVO device itself. To do that simply configure the SDVO pixel replication factor appropriately. We already set up the infoframe PRB values correctly via the infoframe helpers. There is no cap we can check for this. The spec says that 1X,2X,4X are mandatory, anything else is optional. 1X and 2X are all we need so we should be able to assume they work. Signed-off-by:
-
Ville Syrjälä authored
The code assumes that DRM_MODE_FLAG_DBLCLK means that we enable the pixel repeat feature. That only works with HDMI since it requires AVI infoframe to signal the information to the sink. Hence even if the mode dotclock would be valid we cannot currently assume that we can just ignore the DBLCLK flag. Reject it for DVI sinks. Signed-off-by:
-
Ville Syrjälä authored
The SDVO/HDMI port register limited color range bit can only be used with TMDS encoding and not SDVO encoding, ie. to be used only when using the port as a HDMI port as opposed to a SDVO port. The SDVO spec does have a note that some GMCHs might allow that, but gen4 bspec vehemently disagrees. I suppose on ILK+ it might work since the color range handling is on the CPU side rather than on the PCH side, so there is no clear linkage between the TMDS vs. SDVO encoding and color range. Alas, I have no hardware to test that theory. To implement limited color range support for SDVO->HDMI we need to ask the SDVO device to do the range compression. Do so, but first check if the device even supports the colorimetry selection. Signed-off-by:
-
Ville Syrjälä authored
Fix up the SDVO colorimetry bits to match the spec. Signed-off-by:
-
- 08 Jul, 2020 15 commits
-
-
Chris Wilson authored
Last user removed, remove the get_dirty_page convenience function. Signed-off-by:
-
Chris Wilson authored
Some objects we map once during their construction, and then never access their mappings again, even if they are kept around for the duration of the driver. Keeping those pages mapped, often vmapped, is therefore wasteful and we should release the maps as soon as we no longer need them. Signed-off-by:
-
Chris Wilson authored
As we have a pin_map interface, that knows how to flush the data to the device, use it. The only downside is that we keep the kmap around, as once acquired we keep the mapping cached until the object's backing store is released. Signed-off-by:
-
Chris Wilson authored
We removed retiring requests from the shrinker in order to decouple the mutexes from reclaim in preparation for unravelling the struct_mutex. The impact of not retiring is that we are much less agressive in making global objects available for shrinking, as such objects remain pinned until they are flushed by a heartbeat pulse following the last retired request along their timeline. In order to ensure that pulse occurs in time for memory reclamation, we should kick it from kswapd. The catch is that we have added some flush_work() into the retirement phase (to ensure that we reach a global idle in a timely manner), but these flush_work() are not eligible (i.e do not belong to WQ_MEM_RELCAIM) for use from inside kswapd. To avoid flushing those workqueues, we teach the retirer not to do so unless we are actually waiting, and only do the plain retire from inside the shrinker. Note that for execlists, we already retire completed contexts as they are scheduled out, so it should not be keeping global state unnecessarily pinned. The legacy ringbuffer however... References: 9e953980 ("drm/i915: Remove waiting & retiring from shrinker paths") Signed-off-by:
-
Daniele Ceraolo Spurio authored
In line with what happened for other gt-related features, move the sseu debugfs files under gt/. The sseu_status debugfs has also been kept at the top level as we do have tests that use it; it will be removed once we teach the tests to look into the new path. Suggested-by:
Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> Signed-off-by:
-
Daniele Ceraolo Spurio authored
Ahead of moving the sseu debugfs logic under gt/, update the functions to use intel_gt where possible to make the move cleaner. Signed-off-by:
-
Venkata Sandeep Dhanalakota authored
SSEUs are a GT capability, so track them under gt_info. Signed-off-by:
Venkata Sandeep Dhanalakota <venkata.s.dhanalakota@intel.com> Signed-off-by:
-
Daniele Ceraolo Spurio authored
Keep all the SSEU code in the relevant file. The code has also been updated to use intel_gt instead of dev_priv. Based on an original patch by Sandeep. Signed-off-by:
-
Daniele Ceraolo Spurio authored
We already call 2 gt-related init_mmio functions in driver_mmio_probe and a 3rd one will be added by a follow-up patch, so pre-emptively introduce a gt_init_mmio function to group them. Signed-off-by:
-
Daniele Ceraolo Spurio authored
Since the engines belong to the GT, move the runtime-updated list of available engines to the intel_gt struct. The original mask has been renamed to indicate it contains the maximum engine list that can be found on a matching device. In preparation for other info being moved to the gt in follow up patches (sseu), introduce an intel_gt_info structure to group all gt-related runtime info. v2: s/max_engine_mask/platform_engine_mask (tvrtko), fix selftest Signed-off-by:
-
Daniele Ceraolo Spurio authored
All the info we read in intel_device_info_init_mmio are engine-related and since we already have an engine_init_mmio function we can just perform the operations from there. v2: clarify comment about forcewake requirements and pruning (Chris) Signed-off-by:
-
Daniele Ceraolo Spurio authored
A follow up patch will move the engine mask under the gt structure, so get ready for that. v2: switch the remaining gvt case using dev_priv->gt to gvt->gt (Chris) Signed-off-by:
-
Daniele Ceraolo Spurio authored
Use intel_<uncore/de>_read instead of I915_READ to read the informational registers. Extended from an original sseu-only patch by Sandeep. Signed-off-by:
-
Manasi Navare authored
Based on the platform, Bspec expects us to wait or poll with timeout for DDI BUF IDLE bit to be set to 0 (non idle) or get active after enabling DDI_BUF_CTL. v2: * Based on platform, fixed delay or poll (Ville) * Use a helper to do this (Imre, Ville) v3: * Add a new function _active for DDI BUF CTL to be non idle (Ville) v4: * Use the timeout for GLK (Ville) v5: * Add bspec quote, change timeout to 500us (Ville) Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Imre Deak <imre.deak@intel.com> Signed-off-by:
Manasi Navare <manasi.d.navare@intel.com> Reviewed-by:
-
Manasi Navare authored
Modify the helper to add a fixed delay or poll with timeout based on platform specification to check for either Idle bit set (DDI_BUF_CTL is idle for disable case) v2: * Use 2 separate functions or idle and active (Ville) v3: * Change the timeout to 16usecs (Ville) v4: * Change the timeout 8, follow spec (Ville) Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Imre Deak <imre.deak@intel.com> Signed-off-by:
-
