- 21 Nov, 2019 6 commits
-
-
Chris Wilson authored
Whenever we wait on a request, make sure we actually hold a reference to it and that it cannot be retired/freed on another CPU! Signed-off-by:
Chris Wilson <chris@chris-wilson.co.uk> Acked-by:
Mika Kuoppala <mika.kuoppala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191121071044.97798-4-chris@chris-wilson.co.uk
-
Chris Wilson authored
Assume that intel_wakeref_get() may take the mutex, and perform other sleeping actions in the course of its callbacks and so use might_sleep() to ensure that all callers abide. Anything that cannot sleep has to use e.g. intel_wakeref_get_if_active() to guarantee its avoidance of the non-atomic paths. Signed-off-by:
-
Chris Wilson authored
Since the request is already on the HW as we perform its validation, it and even its subsequent barrier may be concurrently retired before we process the assertions. If it is retired already and so off the HW, our assertions become void and we need to ignore them. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=112363Signed-off-by:
Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191121103546.146487-1-chris@chris-wilson.co.uk
-
Chris Wilson authored
As we wait upon a request, we must be holding a reference to it, and be wary that i915_request_add() consumes the passed in reference. Signed-off-by:
Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191121093326.134774-1-chris@chris-wilson.co.uk
-
Chris Wilson authored
Since retirement may be running in a worker on another CPU, it may be skipped in the local intel_gt_wait_for_idle(). To ensure the state is consistent for our sanity checks upon load, serialise with the remote retirer by waiting on the timeline->mutex. Outside of this use case, e.g. on suspend or module unload, we expect the slack to be picked up by intel_gt_pm_wait_for_idle() and so prefer to put the special case serialisation with retirement in its single user, for now at least. Signed-off-by:
-
Chris Wilson authored
From inside an active timeline in the execbuf ioctl, we may try to reclaim some space in the GGTT. We need GGTT space for all objects on !full-ppgtt platforms, and for context images everywhere. However, to free up space in the GGTT we may need to remove some pinned objects (e.g. context images) that require flushing the idle barriers to remove. For this we use the big hammer of intel_gt_wait_for_idle() However, commit 7936a22d ("drm/i915/gt: Wait for new requests in intel_gt_retire_requests()") will continue spinning on the wait if a timeline is active but lacks requests, as is the case during execbuf reservation. Spinning forever is quite time consuming, so revert that commit and start again. In practice, the effect commit 7936a22d was trying to achieve is accomplished by commit 1683d24c ("drm/i915/gt: Move new timelines to the end of active_list"), so there is no immediate rush to replace the looping. Testcase: igt/gem_exec_reloc/basic-range Fixes: 7936a22d ("drm/i915/gt: Wait for new requests in intel_gt_retire_requests()") References: 1683d24c ("drm/i915/gt: Move new timelines to the end of active_list") Signed-off-by:
-
- 20 Nov, 2019 21 commits
-
-
Stuart Summers authored
GuC submission path can be called from an interrupt context and so should use a worker to avoid holding a mutex. References: 07779a76 ("drm/i915: Mark up the calling context for intel_wakeref_put()") Signed-off-by:
Stuart Summers <stuart.summers@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by:
-
Chris Wilson authored
pm_suspend_target_state is declared under CONFIG_PM_SLEEP but only defined under CONFIG_SUSPEND. Play safe and only use the symbol if it is both declared and defined. Reported-by: kbuild-all@lists.01.org Signed-off-by:
-
Chris Wilson authored
Now that we never allow the intel_wakeref callbacks to be invoked from interrupt context, we do not need the irqsafe spinlock for the timeline. Signed-off-by:
-
Chris Wilson authored
In commit a79ca656 ("drm/i915: Push the wakeref->count deferral to the backend"), I erroneously concluded that we last modify the engine inside __i915_request_commit() meaning that we could enable concurrent submission for userspace as we enqueued this request. However, this falls into a trap with other users of the engine->kernel_context waking up and submitting their request before the idle-switch is queued, with the result that the kernel_context is executed out-of-sequence most likely upsetting the GPU and certainly ourselves when we try to retire the out-of-sequence requests. As such we need to hold onto the effective engine->kernel_context mutex lock (via the engine pm mutex proxy) until we have finish queuing the request to the engine. v2: Serialise against concurrent intel_gt_retire_requests() v3: Describe the hairy locking scheme with intel_gt_retire_requests() for future reference. v4: Combine timeline->lock and engine pm release; it's hairy. Fixes: a79ca656 ("drm/i915: Push the wakeref->count deferral to the backend") Signed-off-by:
-
Chris Wilson authored
The general concept was that intel_timeline.active_count was locked by the intel_timeline.mutex. The exception was for power management, where the engine->kernel_context->timeline could be manipulated under the global wakeref.mutex. This was quite solid, as we always manipulated the timeline only while we held an engine wakeref. And then we started retiring requests outside of struct_mutex, only using the timelines.active_list and the timeline->mutex. There we started manipulating intel_timeline.active_count outside of an engine wakeref, and so introduced a race between __engine_park() and intel_gt_retire_requests(), a race that could result in the engine->kernel_context not being added to the active timelines and so losing requests, which caused us to keep the system permanently powered up [and unloadable]. The race would be easy to close if we could take the engine wakeref for the timeline before we retire -- except timelines are not bound to any engine and so we would need to keep all active engines awake. The alternative is to guard intel_timeline_enter/intel_timeline_exit for use outside of the timeline->mutex. Fixes: e5dadff4 ("drm/i915: Protect request retirement with timeline->mutex") Signed-off-by:
-
Chris Wilson authored
Previously, we assumed we could use mutex_trylock() within an atomic context, falling back to a worker if contended. However, such trickery is illegal inside interrupt context, and so we need to always use a worker under such circumstances. As we normally are in process context, we can typically use a plain mutex, and only defer to a work when we know we are being called from an interrupt path. Fixes: 51fbd8de ("drm/i915/pmu: Atomically acquire the gt_pm wakeref") References: a0855d24 ("locking/mutex: Complain upon mutex API misuse in IRQ contexts") References: https://bugs.freedesktop.org/show_bug.cgi?id=111626Signed-off-by:
-
Stuart Summers authored
When display is not available, finding the memory bandwidth available for display is not useful. Skip this sequence here. References: HSDES 1209978255 Signed-off-by:
Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191120011016.18049-1-stuart.summers@intel.com
-
Stuart Summers authored
Platforms without display do not map the MCHBAR MMIO into the GFX device BAR. Skip this sequence when display is not available. Signed-off-by:
-
Ville Syrjälä authored
Just pass the atomic state+crtc to the .crtc_enable() .crtc_disable(). Life is easier when you don't have to think whether to pass the old or the new crtc state. Reviewed-by:
Manasi Navare <manasi.d.navare@intel.com> Signed-off-by:
-
Ville Syrjälä authored
Rename pipe_config to new_crtc_state in the .crtc_enable() hooks. The 'pipe_config' name is a zombie that we need to finally put down. Reviewed-by:
-
Ville Syrjälä authored
Get rid of the horrible aliasing drm_crtc and intel_crtc variables in the crtc enable/disable hooks. Reviewed-by:
-
Ville Syrjälä authored
Get rid of the last 'dev' usage in ironlake_crtc_enable() by passing dev_priv to cpt_verify_modeset(). Reviewed-by:
-
Ville Syrjälä authored
Just pass the atomic_state+crtc to the watermarks hooks. Eeasier time for the caller when it doesn't have to think what to pass. Reviewed-by:
-
Ville Syrjälä authored
Switch to intel_crtc from drm_crtc. Reviewed-by:
-
Ville Syrjälä authored
intel_modeset_setup_hw_state() doesn't need the crtc_state at the top level scope. Move it to where it's needed. Reviewed-by:
-
Ville Syrjälä authored
Move the assert_vblank_disabled() into intel_crtc_vblank_on() so that we don't have to inline it all over. This does mean we now assert_vblank_disabled() during readout as well but that is totally fine as it happens after drm_crtc_vblank_reset(). One can even argue it's what we want to do anyway to make sure the reset actually happened. Reviewed-by:
-
Ville Syrjälä authored
We already have intel_crtc_vblank_on(). Add a counterpart so we don't have to inline the disable+assert all over. Reviewed-by:
-
Ville Syrjälä authored
Just pass the atomic state and the crtc to intel_encoders_enable() & co. Make life simpler when you don't have to think which state (old vs. new) you have to pass in. Also constify the states while at it. Reviewed-by:
-
Jani Nikula authored
Merge tag 'topic/drm-mipi-dsi-dsc-updates-2019-11-11' of git://anongit.freedesktop.org/drm/drm-intel into drm-intel-next-queued Core Changes: - Update DSI data type and command definitions - Add helpers for sending compression mode and PPS packets Driver Changes: - Update tiny/st7586 to reflect a definition change Signed-off-by:
Jani Nikula <jani.nikula@intel.com> From: Jani Nikula <jani.nikula@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/87tv7a4eq3.fsf@intel.com
-
Chris Wilson authored
i915_request_add() consumes the passed in reference to the i915_request, so if the selftest caller wishes to wait upon it afterwards, it needs to take a reference for itself. Signed-off-by:
Matthew Auld <matthew.auld@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191120102741.3734346-1-chris@chris-wilson.co.uk
-
Chris Wilson authored
When setting up a full GGTT, we expect the next insert to fail with -ENOSPC. Simplify the use of ERR_PTR to not confuse either the reader or smatch. Reported-by:
Dan Carpenter <dan.carpenter@oracle.com> References: f40a7b75 ("drm/i915: Initial selftests for exercising eviction") Signed-off-by:
-
- 19 Nov, 2019 13 commits
-
-
Chris Wilson authored
Reading from CTX_INFO upsets rc6, requiring us to detect and prevent possible rc6 context corruption. Poke at the bear! Signed-off-by:
Andi Shyti <andi.shyti@intel.com> Tested-by:
-
Chris Wilson authored
When waiting for idle, serialise with any ongoing callback so that it will have completed before completing the wait. Signed-off-by:
-
Chris Wilson authored
Since igt now defaults to not enabling ftrace-on-oops, we need to manually invoke GEM_TRACE_DUMP() to see the debug log prior to a GEM_BUG_ON panicking. Signed-off-by:
-
Chris Wilson authored
Retire all requests if we resort to wedged the driver on suspend. They will now be idle, so we might as we free them before shutting down. Signed-off-by:
-
Chris Wilson authored
As we may park the gt during request retirement, we may cancel the retirement worker only to then program the delayed worker once more. If we schedule the next delayed retirement worker first, if we then park the gt, the work will remain cancelled. Signed-off-by:
-
Chris Wilson authored
When adding a new active timeline, place it at the end of the list. This allows for intel_gt_retire_requests() to pick up the newcomer more quickly and hopefully complete the retirement sooner. A miniscule optimisation. References: 7936a22d ("drm/i915/gt: Wait for new requests in intel_gt_retire_requests()") Signed-off-by:
-
Matthew Auld authored
For our current users we don't expect pool objects to be writable from the gpu. Signed-off-by:
-
Matt Roper authored
The bspec initially provided a single DKL PHY vswing table for both HDMI and DP, but was recently updated to include an independent table for HDMI. Bspec: 49292 Fixes: 978c3e53 ("drm/i915/tgl: Add dkl phy programming sequences") Cc: Clinton A Taylor <clinton.a.taylor@intel.com> Cc: Lucas De Marchi <lucas.demarchi@intel.com> Signed-off-by:
Matt Roper <matthew.d.roper@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191118180219.9309-1-matthew.d.roper@intel.comReviewed-by:
José Roberto de Souza <jose.souza@intel.com>
-
Vandita Kulkarni authored
As per the Bspec, port mapping is fixed for mipi dsi. v2: Reuse the existing function (Jani) Signed-off-by:
Vandita Kulkarni <vandita.kulkarni@intel.com> Signed-off-by:
-
Chris Wilson authored
Take the obj->vma.lock to prevent modifications to the list as we iterate, to avoid the dreaded NULL pointer. <1>[ 347.820823] BUG: kernel NULL pointer dereference, address: 0000000000000150 <1>[ 347.820856] #PF: supervisor read access in kernel mode <1>[ 347.820874] #PF: error_code(0x0000) - not-present page <6>[ 347.820892] PGD 0 P4D 0 <4>[ 347.820908] Oops: 0000 [#1] PREEMPT SMP NOPTI <4>[ 347.820926] CPU: 3 PID: 1303 Comm: gem_persistent_ Tainted: G U 5.4.0-rc7-CI-CI_DRM_7352+ #1 <4>[ 347.820956] Hardware name: /NUC6CAYB, BIOS AYAPLCEL.86A.0049.2018.0508.1356 05/08/2018 <4>[ 347.821132] RIP: 0010:i915_gem_object_flush_write_domain+0xd9/0x1d0 [i915] <4>[ 347.821157] Code: 0f 84 e9 00 00 00 48 8b 80 e0 fd ff ff f6 c4 40 75 11 e9 ed 00 00 00 48 8b 80 e0 fd ff ff f6 c4 40 74 26 48 8b 83 b0 00 00 00 <48> 8b b8 50 01 00 00 e8 fb 20 fb ff 48 8b 83 30 03 00 00 49 39 c4 <4>[ 347.821210] RSP: 0018:ffffc90000a1f8f8 EFLAGS: 00010202 <4>[ 347.821229] RAX: 0000000000000000 RBX: ffffc900008479a0 RCX: 0000000000000018 <4>[ 347.821252] RDX: 0000000000000000 RSI: 000000000000000d RDI: ffff888275a090b0 <4>[ 347.821274] RBP: ffff8882673c8040 R08: ffff88825991b8d0 R09: 0000000000000000 <4>[ 347.821297] R10: 0000000000000000 R11: 0000000000000000 R12: ffff8882673c8280 <4>[ 347.821319] R13: ffff8882673c8368 R14: 0000000000000000 R15: ffff888266a54000 <4>[ 347.821343] FS: 00007f75865f4240(0000) GS:ffff888277b80000(0000) knlGS:0000000000000000 <4>[ 347.821368] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 <4>[ 347.821389] CR2: 0000000000000150 CR3: 000000025aee0000 CR4: 00000000003406e0 <4>[ 347.821411] Call Trace: <4>[ 347.821555] i915_gem_object_prepare_read+0xea/0x2a0 [i915] <4>[ 347.821706] intel_engine_cmd_parser+0x5ce/0xe90 [i915] <4>[ 347.821834] ? __i915_sw_fence_complete+0x1a0/0x250 [i915] <4>[ 347.821990] i915_gem_do_execbuffer+0xb4c/0x2550 [i915] Signed-off-by:
-
Chris Wilson authored
We only need the one loop to find the dirty vma flush them and their chipset. Signed-off-by:
-
Chris Wilson authored
When userspace writes into the GTT itself, it is supposed to call set-domain to let the kernel keep track and so manage the CPU/GPU caches. As we track writes on the individual i915_vma, we should also be sure to mark them as dirty. Signed-off-by:
-
Chris Wilson authored
In order to avoid some nasty mutex inversions, commit 09c5ab38 ("drm/i915: Keep rings pinned while the context is active") allowed the intel_ring unpinning to be run concurrently with the next context pinning it. Thus each step in intel_ring_unpin() needed to be atomic and ordered in a nice onion with intel_ring_pin() so that the lifetimes overlapped and were always safe. Sadly, a few steps in intel_ring_unpin() were overlooked, such as closing the read/write pointers of the ring and discarding the intel_ring.vaddr, as these steps were not serialised with intel_ring_pin() and so could leave the ring in disarray. Fixes: 09c5ab38 ("drm/i915: Keep rings pinned while the context is active") Signed-off-by:
-
