- 29 May, 2020 6 commits
-
-
Chris Wilson authored
Name the object classes and their offspring for easier lockdep debugging. Signed-off-by:
Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by:
Matthew Auld <matthew.auld@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200529183204.16850-2-chris@chris-wilson.co.uk
-
Chris Wilson authored
If we declare that an object type is shrinkable (any that we can reclaim to recover system pages), make sure we taint the object mutex so that lockdep expects us to use it within fs_reclaim. lockdep will then complain the first time we try to allocate while holding the plain mutex, as doing so invites potential recursion. Signed-off-by:
-
Chris Wilson authored
With the advent of preempt-to-busy, a request may still be on the GPU as we unwind. And in the case of a unpreemptible [due to HW] request, that request will remain indefinitely on the GPU even though we have returned it back to our submission queue, and cleared the active bit. We only run the execution callbacks on transferring the request from our submission queue to the execution queue, but if this is a bonded request that the HW is waiting for, we will not submit it (as we wait for a fresh execution) even though it is still being executed. As we know that there are always preemption points between requests, we know that only the currently executing request may be still active even though we have cleared the flag. However, we do not precisely know which request is in ELSP[0] due to a delay in processing events, and furthermore we only store the last request in a context in our state tracker. Fixes: 22b7a426 ("drm/i915/execlists: Preempt-to-busy") Testcase: igt/gem_exec_balancer/bonded-dual Signed-off-by:
Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200529143926.3245-1-chris@chris-wilson.co.uk
-
Chris Wilson authored
Let's assert that we only call the execute callbacks on making the request active, and that we do not execute the request without calling the callbacks. Signed-off-by:
-
Ville Syrjälä authored
There's no reason for I915_MODE_FLAG_INHERITED to exist as a flag anymore. Just make it a boolean. v2: Deal with sanitize_watermarks() CC: Sam Ravnborg <sam@ravnborg.org> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Emil Velikov <emil.l.velikov@gmail.com> Reviewed-by:
Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by:
Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200429103936.11850-1-ville.syrjala@linux.intel.com
-
Ville Syrjälä authored
Replace the use of mode->private_flags with a truly private bitmaks in our own crtc state. We also need a copy in the crtc itself so the vblank code can get at it. We already have scanline_offset in there for a similar reason, as well as the vblank->hwmode which is assigned via drm_calc_timestamping_constants(). Fortunately we now have a nice place for doing the crtc_state->crtc copy in intel_crtc_update_active_timings() which gets called both for modesets and init/resume readout. The one slightly iffy spot is the INHERITED flag which we want to preserve until userspace/fb_helper does the first proper commit after actually calling .detecti() on the connectors. Otherwise we don't have the full sink capabilities (audio,infoframes,etc.) when .compute_config() gets called and thus we will fail to enable those features when the first userspace commit happens. The only internal commit we do prior to that should be from intel_initial_commit() and there we can simply preserve the INHERITED flag from the readout. v2: Deal with INHERITED in sanitize_watermarks() as well CC: Sam Ravnborg <sam@ravnborg.org> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Emil Velikov <emil.l.velikov@gmail.com> Reviewed-by:
-
- 28 May, 2020 5 commits
-
-
Chris Wilson authored
We may choose to only submit ELSP[0], even though we have sufficient requests to fill the whole ELSP. Normally, we only start timeslicing if we fill more than one port, but in this case we need to start timeslicing for the queue that we choose not to submit. Signed-off-by:
Mika Kuoppala <mika.kuoppala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200528205727.20309-1-chris@chris-wilson.co.uk
-
Chris Wilson authored
If the ring submission is stalled on an external request, nothing can be submitted, not even the heartbeat in the kernel context. Since nothing is running, resetting the engine/device does not unblock the system and is pointless. We can see if the heartbeat is supposed to be running before declaring foul. Signed-off-by:
-
Chris Wilson authored
Across suspend/resume, we clear the entire GGTT and rebuild from scratch. In particular, we want to only preserve the global entries for use by the HW, and delay reinstating the local binds until required by the user. This means that we can evict any local binds in the global GTT, saving any time in preserving their state, as they will be rebound on demand. References: https://gitlab.freedesktop.org/drm/intel/-/issues/1947Signed-off-by:
-
Chris Wilson authored
We should be able to skip restoring LOCAL (user) binds within the GGTT on resume and let them be restored upon demand. However, our consistency checks demand that the bind flags match the node state, and we cannot simply clear the flags, we need to evict as well. For now, make sure we restore the bind flags exactly upon resume. Fixes: 0109a16e ("drm/i915/gt: Clear LOCAL_BIND from shared GGTT on resume") Fixes: bf0840cd ("drm/i915/gt: Stop cross-polluting PIN_GLOBAL with PIN_USER with no-ppgtt") Signed-off-by:
-
Chris Wilson authored
We have a I915_REQUEST_NOPREEMPT flag that we set when we must prevent the HW from preempting during the course of this request. We need to honour this flag and protect the HW even if we have a heartbeat request, or other maximum priority barrier, pending. As such, restrict the timeslicing check to avoid preempting into the topmost priority band, leaving the unpreemptable requests in blissful peace running uninterrupted on the HW. v2: Set the I915_PRIORITY_BARRIER to be less than I915_PRIORITY_UNPREEMPTABLE so that we never submit a request (heartbeat or barrier) that can legitimately preempt the current non-premptable request. Fixes: 2a98f4e6 ("drm/i915: add infrastructure to hold off preemption on a request") Signed-off-by:
-
- 27 May, 2020 5 commits
-
-
Arnd Bergmann authored
gcc-9 gets confused by the code flow in check_dirty_whitelist: drivers/gpu/drm/i915/gt/selftest_workarounds.c: In function 'check_dirty_whitelist': drivers/gpu/drm/i915/gt/selftest_workarounds.c:492:17: error: 'rsvd' may be used uninitialized in this function [-Werror=maybe-uninitialized] I could not figure out a good way to do this in a way that gcc understands better, so initialize the variable to zero, as last resort. Fixes: aee20aae ("drm/i915: Implement read-only support in whitelist selftest") Signed-off-by:
Arnd Bergmann <arnd@arndb.de> Reviewed-by:
-
Arnd Bergmann authored
Conditional spinlocks make it hard for gcc and for lockdep to follow the code flow. This one causes a warning with at least gcc-9 and higher: In file included from include/linux/irq.h:14, from drivers/gpu/drm/i915/i915_pmu.c:7: drivers/gpu/drm/i915/i915_pmu.c: In function 'i915_sample': include/linux/spinlock.h:289:3: error: 'flags' may be used uninitialized in this function [-Werror=maybe-uninitialized] 289 | _raw_spin_unlock_irqrestore(lock, flags); \ | ^~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/i915/i915_pmu.c:288:17: note: 'flags' was declared here 288 | unsigned long flags; | ^~~~~ Split out the part between the locks into a separate function for readability and to let the compiler figure out what the logic actually is. Fixes: d79e1bd6 ("drm/i915/pmu: Only use exclusive mmio access for gen7") Signed-off-by: -
Chris Wilson authored
We only restore GLOBAL binds upon resume as we expect these to be pinned for use by HW, whereas the LOCAL binds can be recreated on demand once userspace is resumed. For the LOCAL bind to be recreated in the global GTT (for old systems without ppgtt), we need to clear its presence flag on deciding not to restore the mapping upon resume. Fixes: bf0840cd ("drm/i915/gt: Stop cross-polluting PIN_GLOBAL with PIN_USER with no-ppgtt") Signed-off-by:
-
Chris Wilson authored
When we push a virtual request onto the HW, we update the rq->engine to point to the physical engine. A request that is then submitted by the user that waits upon the virtual engine, but along the physical engine in use, will then see that it is due to be submitted to the same engine and take a shortcut (and be queued without waiting for the completion fence). However, the virtual request may be preempted (either by higher priority users, or by timeslicing) and removed from the physical engine to be migrated over to one of its siblings. The dependent normal request however is oblivious to the removal of the virtual request and remains queued to execute on HW, believing that once it reaches the head of its queue all of its predecessors will have completed executing! v2: Beware restriction of signal->execution_mask prior to submission. Fixes: 6d06779e ("drm/i915: Load balancing across a virtual engine") Testcase: igt/gem_exec_balancer/sliced Signed-off-by:
-
Chris Wilson authored
Reorder the code so that we can reuse the await_execution from a special case in await_request in the next patch. Signed-off-by:
-
- 26 May, 2020 5 commits
-
-
Chris Wilson authored
Avoid a NULL dereference for a mismatched encoder type, hit when probing state for all encoders. This is a band aid to prevent the OOPS as the right fix is "probably to swap the psr vs infoframes.enable checks, or outright disappear from this function" (Ville). Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/1892Signed-off-by:
-
Chris Wilson authored
Reduce the irq_work llist for attaching the callbacks to the signal for both smaller structs (two fewer pointers!) and simpler [debug] code: Function old new delta irq_execute_cb 35 34 -1 __igt_breadcrumbs_smoketest 1684 1682 -2 i915_request_retire 2003 1996 -7 __i915_request_create 1047 1040 -7 __notify_execute_cb 135 126 -9 __i915_request_ctor 188 178 -10 __await_execution.part.constprop 451 440 -11 igt_wait_request 924 714 -210 One minor artifact is that the order of cb exection is reversed. No current use cases are affected by that change. Signed-off-by:
-
Chris Wilson authored
If there are no internal levels and the user priority-shift is zero, we can help the compiler eliminate some dead code: Function old new delta start_timeslice 169 154 -15 __execlists_submission_tasklet 4696 4659 -37 Signed-off-by:
-
Chris Wilson authored
Before we return control to the system, and letting it reuse all the pages being accessed by HW, we must disable the HW. At the moment, we dare not reset the GPU if it will clobber the display, but once we know the display has been disabled, we can proceed with the reset as we shutdown the module. We know the next user must reinitialise the HW for their purpose. Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/489Signed-off-by:
-
Chris Wilson authored
drivers/gpu/drm/i915/display/intel_dsb.c:177 intel_dsb_reg_write() warn: variable dereferenced before check 'dsb' (see line 175) Fixes: afeda4f3 ("drm/i915/dsb: Pre allocate and late cleanup of cmd buffer") Signed-off-by:
Animesh Manna <animesh.manna@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200524233900.25598-1-chris@chris-wilson.co.uk
-
- 25 May, 2020 3 commits
-
-
Chris Wilson authored
In order to keep userptr distinct from ggtt mmaps in the eyes of lockdep, we need to avoid marking those userptr vma as PIN_GLOBAL. (So long as we comply with only using them as local PIN_USER!) References: https://gitlab.freedesktop.org/drm/intel/-/issues/1880Signed-off-by:
-
Chris Wilson authored
Leave the error propagation in place, but limit the warnings to only show up in CI if the unlikely errors are hit. Signed-off-by:
-
Chris Wilson authored
Since the worker may rearm, we currently are only guaranteed to flush the work if we cancel the timer. If the work was running at the time we try and cancel it, we will wait for it to complete, but it may leave items in the pool and requeue the work. If we rearrange the immediate discard of the pool then cancel the work, we know that the work cannot rearm and so our flush will be final. <0> [314.146044] i915_mod-1321 2.... 299799443us : intel_gt_fini_buffer_pool: intel_gt_fini_buffer_pool:227 GEM_BUG_ON(!list_empty(&pool->cache_list[n])) Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/1920Signed-off-by:
-
- 23 May, 2020 1 commit
-
-
Animesh Manna authored
Pre-allocate command buffer in atomic_commit using intel_dsb_prepare function which also includes pinning and map in cpu domain. No functional change is dsb write/commit functions. Now dsb get/put function is removed and ref-count mechanism is not needed. Below dsb api added to do respective job mentioned below. intel_dsb_prepare - Allocate, pin and map the buffer. intel_dsb_cleanup - Unpin and release the gem object. RFC: Initial patch for design review. v2: included _init() part in _prepare(). [Daniel, Ville] v3: dsb_cleanup called after cleanup_planes. [Daniel] v4: dsb structure is moved to intel_crtc_state from intel_crtc. [Maarten] v5: dsb get/put/ref-count mechanism removed. [Maarten] v6: Based on review feedback following changes are added, - replaced intel_dsb structure by pointer in intel_crtc_state. [Maarten] - passing intel_crtc_state to dsp-api to simplify the code. [Maarten] - few dsb functions prototype modified to simplify code. v7: added few cosmetic changes suggested by Jani and null check for crtc_state in dsb_cleanup removed as suggested by Maarten. v8: changed the function parameter to intel_crtc_state* of ivb_load_lut_ext_max() from intel_crtc. [Maarten] v9: error handling improved in _write() and prepare(). [Maarten] Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Jani Nikula <jani.nikula@intel.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Acked-by:
Maarten Lankhorst <maarten.lankhorst@intel.com> Signed-off-by:
Uma Shankar <uma.shankar@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200520130737.11240-1-animesh.manna@intel.com
-
- 22 May, 2020 4 commits
-
-
Chris Wilson authored
Our __sgt_iter assumes that the scattergather list has at least one element. But during construction we may fail in allocating the first page, and so mark the first element as the terminator. This is unexpected! [22555.524752] RIP: 0010:shmem_get_pages+0x506/0x710 [i915] [22555.524759] Code: 49 8b 2c 24 31 c0 66 89 44 24 40 48 85 ed 0f 84 62 01 00 00 4c 8b 75 00 8b 5d 08 44 8b 7d 0c 48 8b 0d 7e 34 07 e2 49 83 e6 fc <49> 8b 16 41 01 df 48 89 cf 48 89 d0 48 c1 e8 2d 48 85 c9 0f 84 c8 [22555.524765] RSP: 0018:ffffc9000053f9d0 EFLAGS: 00010246 [22555.524770] RAX: 0000000000000000 RBX: 0000000000000000 RCX: ffff8881ffffa000 [22555.524774] RDX: fffffffffffffff4 RSI: ffffffffffffffff RDI: ffffffff821efe00 [22555.524778] RBP: ffff8881b099ab00 R08: 0000000000000000 R09: 00000000fffffff4 [22555.524782] R10: 0000000000000002 R11: 00000000ffec0a02 R12: ffff8881cd3c8d60 [22555.524786] R13: 00000000fffffff4 R14: 0000000000000000 R15: 0000000000000000 [22555.524790] FS: 00007f4fbeb9b9c0(0000) GS:ffff8881f8580000(0000) knlGS:0000000000000000 [22555.524795] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [22555.524799] CR2: 0000000000000000 CR3: 00000001ec7f0004 CR4: 00000000001606e0 [22555.524803] Call Trace: [22555.524919] __i915_gem_object_get_pages+0x4f/0x60 [i915] Fixes: 85d1225e ("drm/i915: Introduce & use new lightweight SGL iterators") Signed-off-by:
Maciej Patelczyk <maciej.patelczyk@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200522132706.5133-1-chris@chris-wilson.co.uk
-
Stanislav Lisovskiy authored
Removed duplicate include and fixed comment > 80 chars. v2: Added newline after system include and between functions Reviewed-by:
Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> Signed-off-by:
-
Qiushi Wu authored
In intel_gtt_setup_scratch_page(), pointer "page" is not released if pci_dma_mapping_error() return an error, leading to a memory leak on module initialisation failure. Simply fix this issue by freeing "page" before return. Fixes: 0e87d2b0 ("intel-gtt: initialize our own scratch page") Signed-off-by:
Qiushi Wu <wu000273@umn.edu> Reviewed-by:
-
Swathi Dhanavanthri authored
This is a permanent w/a for JSL/EHL.This is to be applied to the PCH types on JSL/EHL ie JSP/MCC Bspec: 52888 v2: Fixed the wrong usage of logical OR(ville) v3: Removed extra braces, changed the check(jose) Signed-off-by:
Swathi Dhanavanthri <swathi.dhanavanthri@intel.com> Reviewed-by:
José Roberto de Souza <jose.souza@intel.com> Signed-off-by:
-
- 21 May, 2020 11 commits
-
-
Stanislav Lisovskiy authored
No need to bump up CDCLK now, as it is now correctly calculated, accounting for DBuf BW as BSpec says. Reviewed-by:
Manasi Navare <manasi.d.navare@intel.com> Signed-off-by:
-
Stanislav Lisovskiy authored
According to BSpec max BW per slice is calculated using formula Max BW = CDCLK * 64. Currently when calculating min CDCLK we account only per plane requirements, however in order to avoid FIFO underruns we need to estimate accumulated BW consumed by all planes(ddb entries basically) residing on that particular DBuf slice. This will allow us to put CDCLK lower and save power when we don't need that much bandwidth or gain additional performance once plane consumption grows. v2: - Fix long line warning - Limited new DBuf bw checks to only gens >= 11 v3: - Lets track used Dbuf bw per slice and per crtc in bw state (or may be in DBuf state in future), that way we don't need to have all crtcs in state and those only if we detect if are actually going to change cdclk, just same way as we do with other stuff, i.e intel_atomic_serialize_global_state and co. Just as per Ville's paradigm. - Made dbuf bw calculation procedure look nicer by introducing for_each_dbuf_slice_in_mask - we often will now need to iterate slices using mask. - According to experimental results CDCLK * 64 accounts for overall bandwidth across all dbufs, not per dbuf. v4: - Fixed missing const(Ville) - Removed spurious whitespaces(Ville) - Fixed local variable init(reduced scope where not needed) - Added some comments about data rate for planar formats - Changed struct intel_crtc_bw to intel_dbuf_bw - Moved dbuf bw calculation to intel_compute_min_cdclk(Ville) v5: - Removed unneeded macro v6: - Prevent too frequent CDCLK switching back and forth: Always switch to higher CDCLK when needed to prevent bandwidth issues, however don't switch to lower CDCLK earlier than once in 30 minutes in order to prevent constant modeset blinking. We could of course not switch back at all, however this is bad from power consumption point of view. v7: - Fixed to track cdclk using bw_state, modeset will be now triggered only when CDCLK change is really needed. v8: - Lock global state if bw_state->min_cdclk is changed. - Try getting bw_state only if there are crtcs in the commit (need to have read-locked global state) v9: - Do not do Dbuf bw check for gens < 9 - triggers WARN as ddb_size is 0. v10: - Lock global state for older gens as well. v11: - Define new bw_calc_min_cdclk hook, instead of using a condition(Manasi Navare) v12: - Fixed rebase conflict v13: - Added spaces after declarations to make checkpatch happy. Signed-off-by: -
Stanislav Lisovskiy authored
We quite often need now to iterate only particular dbuf slices in mask, whether they are active or related to particular crtc. v2: - Minor code refactoring v3: - Use enum for max slices instead of macro Let's make our life a bit easier and use a macro for that. Reviewed-by:
-
Stanislav Lisovskiy authored
So lets support it. v2: - Fixed "from" field which got corrupted for some weird reason Reviewed-by:
-
Stanislav Lisovskiy authored
Checking with hweight8 if plane configuration had changed seems to be wrong as different plane configs can result in a same hamming weight. So lets check the bitmask itself. v2: Fixed "from" field which got corrupted for some weird reason Reviewed-by:
-
Stanislav Lisovskiy authored
In Gen11+ whenever we might exceed DBuf bandwidth we might need to recalculate CDCLK which DBuf bandwidth is scaled with. Total Dbuf bw used might change based on particular plane needs. Thus to calculate if cdclk needs to be changed it is not enough anymore to check plane configuration and plane min cdclk, per DBuf bw can be calculated only after wm/ddb calculation is done and all required planes are added into the state. In order to keep all min_cdclk related checks in one place let's extract it into separate function, checking and modifying any_ms. Reviewed-by:
-
Stanislav Lisovskiy authored
We need to calculate cdclk after watermarks/ddb has been calculated as with recent hw CDCLK needs to be adjusted accordingly to DBuf requirements, which is not possible with current code organization. Setting CDCLK according to DBuf BW requirements and not just rejecting if it doesn't satisfy BW requirements, will allow us to save power when it is possible and gain additional bandwidth when it's needed - i.e boosting both our power management and perfomance capabilities. This patch is preparation for that, first we now extract modeset calculation from modeset checks, in order to call it after wm/ddb has been calculated. v2: - Extract only intel_modeset_calc_cdclk from intel_modeset_checks (Ville Syrjälä) v3: - Clear plls after intel_modeset_calc_cdclk v4: - Added r-b from previous revision to commit message Reviewed-by: -
Chris Wilson authored
As we no longer use PIN_UPDATE (since commit 7d0aa0db ("drm/i915/gem: Unbind all current vma on changing cache-level")) we can remove PIN_UPDATE itself. The benefit is just in simplifing the vma bind. Signed-off-by:
-
Chris Wilson authored
In order to be valid to dereference during the i915_fence_release, after retiring the fence and releasing its refererences, we assume that rq->engine can only be a real engine (that stay intact until the device is shutdown after all fences have been flushed). However, due to a quirk of preempt-to-busy, we may retire a request that still belongs to a virtual engine and so eventually free it with rq->engine being invalid. To avoid dereferencing that invalid engine, we look at the execution_mask which if it indicates it may be executed on more than one engine, we know it originated on a virtual engine and may still be on one. Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/1906 Fixes: 43acd651 ("drm/i915: Keep a per-engine request pool") Signed-off-by:
-
Chris Wilson authored
Since the removal of the no-semaphore boosting, we rely on timeslicing to reorder passed inter-dependency hogs across the engines. However, we require preemption to support timeslicing into user payloads, and not all machine support preemption so we do not universally enable timeslicing, even when it would correctly preempt our own inter-engine semaphores. Since timeslicing and semaphore priority deboosting is now disabled on Broadwell/Braswell, we have to follow suite and not use semaphores. Testcase: igt/gem_exec_schedule/semaphore-codependency # bdw/bsw Fixes: 18e4af04 ("drm/i915: Drop no-semaphore boosting") Signed-off-by:
-
Chris Wilson authored
Use intel_engine_flush_submission() when we want to ensure that the tasklet is run. tasklet_kill(), while it may ensure that an ongoing tasklet is completed, also prevents the tasklet from running if it's already scheduled and hasn't yet been run. Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/1874Signed-off-by:
-
