aboutsummaryrefslogtreecommitdiff
path: root/drivers/gpu/drm/i915
AgeCommit message (Collapse)AuthorFilesLines
2020-06-01drm/i915: Fix global state use-after-frees with a refcountVille Syrjälä2-6/+42
While the current locking/serialization of the global state suffices for protecting the obj->state access and the actual hardware reprogramming, we do have a problem with accessing the old/new states during nonblocking commits. The state computation and swap will be protected by the crtc locks, but the commit_tails can finish out of order, thus also causing the atomic states to be cleaned up out of order. This would mean the commit that started first but finished last has had its new state freed as the no-longer-needed old state by the other commit. To fix this let's just refcount the states. obj->state amounts to one reference, and the intel_atomic_state holds extra references to both its new and old global obj states. Fixes: 0ef1905ecf2e ("drm/i915: Introduce better global state handling") Signed-off-by: Ville Syrjälä <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected] Reviewed-by: Stanislav Lisovskiy <[email protected]>
2020-06-01Merge branches 'pm-core' and 'pm-sleep'Rafael J. Wysocki1-1/+1
* pm-core: PM: runtime: Replace pm_runtime_callbacks_present() PM: runtime: clk: Fix clk_pm_runtime_get() error path PM: runtime: Make clear what we do when conditions are wrong in rpm_suspend() * pm-sleep: PM: hibernate: Restrict writes to the resume device PM: hibernate: Split off snapshot dev option PM: hibernate: Incorporate concurrency handling PM: sleep: Helpful edits for devices.rst documentation Documentation: PM: sleep: Update driver flags documentation PM: sleep: core: Rename DPM_FLAG_LEAVE_SUSPENDED PM: sleep: core: Rename DPM_FLAG_NEVER_SKIP PM: sleep: core: Rename dev_pm_smart_suspend_and_suspended() PM: sleep: core: Rename dev_pm_may_skip_resume() PM: sleep: core: Rework the power.may_skip_resume handling PM: sleep: core: Do not skip callbacks in the resume phase PM: sleep: core: Fold functions into their callers PM: sleep: core: Simplify the SMART_SUSPEND flag handling
2020-06-01drm/i915: Relinquish forcewake immediately after manual groupingChris Wilson1-1/+1
Our forcewake utilisation is split into categories: automatic and manual. Around bare register reads, we look up the right forcewake domain and automatically acquire and release [upon a timer] the forcewake domain. For other access, where we know we require the forcewake across a group of register reads, we manually acquire the forcewake domain and release it at the end. Again, this currently arms the domain timer for a later release. However, looking at some energy utilisation profiles, we have tried to avoid using forcewake [and rely on the natural wake up to post register updates] due to that even keep the fw active for a brief period contributes to a significant power draw [i.e. when the gpu is sleeping with rc6 at high clocks]. But as it turns out, not posting the writes immediately also has unintended consequences, such as not reducing the clocks and so conserving power while busy. As a compromise, let us only arm the domain timer for automatic forcewake usage around bare register access, but immediately release the forcewake when manually acquired by intel_uncore_forcewake_get/_put. The corollary to this is that we may instead have to take forcewake more often, and so incur a latency penalty in doing so. For Sandybridge this was significant, and even on the latest machines, taking forcewake at interrupt frequency is a huge impact. [So we don't do that anymore! Hopefully, this will spare us from still needing the mitigation of the timer for steady state execution.] Signed-off-by: Chris Wilson <[email protected]> Cc: Tvrtko Ursulin <[email protected]> Cc: Mika Kuoppala <[email protected]> Reviewed-by: Mika Kuoppala <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
2020-06-01drm/i915: Check for awaits on still currently executing requestsChris Wilson1-1/+48
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: 22b7a426bbe1 ("drm/i915/execlists: Preempt-to-busy") Testcase: igt/gem_exec_balancer/bonded-dual Signed-off-by: Chris Wilson <[email protected]> Cc: Tvrtko Ursulin <[email protected]> Reviewed-by: Tvrtko Ursulin <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected] (cherry picked from commit b55230e5e800868961fc271b26d9ce53ae1f691e) Signed-off-by: Joonas Lahtinen <[email protected]>
2020-06-01drm/i915/gt: Do not schedule normal requests immediately along virtualChris Wilson1-4/+21
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: 6d06779e8672 ("drm/i915: Load balancing across a virtual engine") Testcase: igt/gem_exec_balancer/sliced Signed-off-by: Chris Wilson <[email protected]> Cc: Tvrtko Ursulin <[email protected]> Cc: <[email protected]> # v5.3+ Reviewed-by: Tvrtko Ursulin <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected] (cherry picked from commit 511b6d9aed417739b6aa49d0b6b4354ad21020f1) Signed-off-by: Joonas Lahtinen <[email protected]>
2020-06-01drm/i915: Reorder await_execution before await_requestChris Wilson1-132/+132
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: Chris Wilson <[email protected]> Reviewed-by: Tvrtko Ursulin <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected] (cherry picked from commit ffb0c600c240103f6f34e07892a7e0a75502b243) Signed-off-by: Joonas Lahtinen <[email protected]>
2020-06-01drm/i915: Handle very early engine initialisation failureChris Wilson1-3/+3
If we fail during engine setup, we may leave some engines not yet setup. During the error cleanup, we have to be careful not to try and use the uninitialise engines before discarding them. [ 16.136152] RIP: 0010:__flush_work+0x198/0x1b0 [ 16.136168] Code: ff ff 8b 0b 48 8b 53 08 83 e1 08 48 0f ba 2b 03 80 c9 f0 e9 63 ff ff ff 0f 0b 48 83 c4 48 44 89 f0 5b 5d 41 5c 41 5d 41 5e c3 <0f> 0b 45 31 f6 e9 62 ff ff ff 66 66 2e 0f 1f 84 00 00 00 00 00 0f [ 16.136186] RSP: 0018:ffffc900003bb928 EFLAGS: 00010246 [ 16.136201] RAX: 0000000000000000 RBX: ffff88844f392168 RCX: 0000000000000000 [ 16.136216] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff88844f392168 [ 16.136231] RBP: ffff88844f392130 R08: 0000000000000000 R09: 0000000000000001 [ 16.136246] R10: ffff888441e31e40 R11: ffff88845e329c70 R12: ffff88844f796988 [ 16.136261] R13: ffff888441e4fb80 R14: 0000000000000001 R15: ffff88844f790000 [ 16.136388] FS: 00007fecbd208880(0000) GS:ffff88845e380000(0000) knlGS:0000000000000000 [ 16.136405] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 16.136420] CR2: 00007ff3ce748f90 CR3: 0000000457a6a001 CR4: 00000000000606e0 [ 16.136437] Call Trace: [ 16.136456] ? try_to_del_timer_sync+0x3a/0x50 [ 16.136529] intel_wakeref_wait_for_idle+0x87/0xb0 [i915] [ 16.136606] ? intel_engines_release+0x68/0xc0 [i915] [ 16.136680] intel_engines_release+0x49/0xc0 [i915] [ 16.136757] intel_gt_init+0x2f4/0x5e0 [i915] Signed-off-by: Chris Wilson <[email protected]> Reviewed-by: Mika Kuoppala <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
2020-06-01drm/i915: Add Plane color encoding support for YCBCR_BT2020Kishore Kadiyala3-7/+19
Currently the plane property doesn't have support for YCBCR_BT2020, which enables the corresponding color conversion mode on plane CSC. Enabling the plane property for the planes for GLK & ICL+ platforms. Also as per spec, update the Plane Color CSC from YUV601_TO_RGB709 to YUV601_TO_RGB601. V2: Enabling support for YCBCT_BT2020 for HDR planes on platforms GLK & ICL V3: Refined the condition check to handle GLK & ICL+ HDR planes Also added BT2020 handling in glk_plane_color_ctl. V4: Combine If-else into single If V5: Drop the checking for HDR planes and enable YCBCR_BT2020 for platforms GLK & ICL+. V6: As per Spec, update PLANE_COLOR_CSC_MODE_YUV601_TO_RGB709 to PLANE_COLOR_CSC_MODE_YUV601_TO_RGB601 as per Ville's feedback. V7: Rebased Cc: Ville Syrjala <[email protected]> Cc: Jani Nikula <[email protected]> Reviewed-by: Uma Shankar <[email protected]> Signed-off-by: Kishore Kadiyala <[email protected]> Signed-off-by: Uma Shankar <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
2020-05-29drm/i915/gem: Give each object class a friendly nameChris Wilson14-1/+17
Name the object classes and their offspring for easier lockdep debugging. Signed-off-by: Chris Wilson <[email protected]> Reviewed-by: Matthew Auld <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
2020-05-29drm/i915/gem: Taint all shrinkable object locksChris Wilson1-0/+4
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 <[email protected]> Reviewed-by: Matthew Auld <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
2020-05-29drm/i915: Check for awaits on still currently executing requestsChris Wilson1-1/+48
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: 22b7a426bbe1 ("drm/i915/execlists: Preempt-to-busy") Testcase: igt/gem_exec_balancer/bonded-dual Signed-off-by: Chris Wilson <[email protected]> Cc: Tvrtko Ursulin <[email protected]> Reviewed-by: Tvrtko Ursulin <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
2020-05-29drm/i915: Add a few asserts around handling of i915_request_is_active()Chris Wilson1-2/+3
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: Chris Wilson <[email protected]> Reviewed-by: Tvrtko Ursulin <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
2020-05-29drm/i915: Replace I915_MODE_FLAG_INHERITED with a booleanVille Syrjälä3-12/+9
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 <[email protected]> Cc: Daniel Vetter <[email protected]> Cc: Emil Velikov <[email protected]> Reviewed-by: Daniel Vetter <[email protected]> Signed-off-by: Ville Syrjälä <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
2020-05-29drm/i915: Stop using mode->private_flagsVille Syrjälä7-28/+49
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 <[email protected]> Cc: Daniel Vetter <[email protected]> Cc: Emil Velikov <[email protected]> Reviewed-by: Daniel Vetter <[email protected]> Signed-off-by: Ville Syrjälä <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
2020-05-28drm/i915/gt: Start timeslice on partial submissionChris Wilson1-1/+3
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: Chris Wilson <[email protected]> Cc: Tvrtko Ursulin <[email protected]> Reviewed-by: Mika Kuoppala <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
2020-05-28drm/i915/gt: Don't declare hangs if engine is stalledChris Wilson1-3/+16
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 <[email protected]> Reviewed-by: Mika Kuoppala <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
2020-05-28drm/i915/gt: Remove local entries from GGTT on suspendChris Wilson3-29/+54
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/1947 Signed-off-by: Chris Wilson <[email protected]> Reviewed-by: Matthew Auld <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
2020-05-28drm/i915/gt: Restore both GGTT bindings on resumeChris Wilson1-4/+2
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: 0109a16ef391 ("drm/i915/gt: Clear LOCAL_BIND from shared GGTT on resume") Fixes: bf0840cdb304 ("drm/i915/gt: Stop cross-polluting PIN_GLOBAL with PIN_USER with no-ppgtt") Signed-off-by: Chris Wilson <[email protected]> Reviewed-by: Matthew Auld <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
2020-05-28drm/i915/gt: Prevent timeslicing into unpreemptable requestsChris Wilson3-2/+119
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: 2a98f4e65bba ("drm/i915: add infrastructure to hold off preemption on a request") Signed-off-by: Chris Wilson <[email protected]> Cc: Tvrtko Ursulin <[email protected]> Reviewed-by: Tvrtko Ursulin <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
2020-05-28Merge tag 'v5.7-rc7' into perf/core, to pick up fixesIngo Molnar35-188/+339
Signed-off-by: Ingo Molnar <[email protected]>
2020-05-27drm/i915: work around false-positive maybe-uninitialized warningArnd Bergmann1-0/+2
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: aee20aaed887 ("drm/i915: Implement read-only support in whitelist selftest") Signed-off-by: Arnd Bergmann <[email protected]> Reviewed-by: Chris Wilson <[email protected]> Signed-off-by: Chris Wilson <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
2020-05-27drm/i915/pmu: avoid an maybe-uninitialized warningArnd Bergmann1-42/+42
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: d79e1bd676f0 ("drm/i915/pmu: Only use exclusive mmio access for gen7") Signed-off-by: Arnd Bergmann <[email protected]> Reviewed-by: Chris Wilson <[email protected]> Signed-off-by: Chris Wilson <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
2020-05-27drm: Nuke mode->vrefreshVille Syrjälä4-11/+7
Get rid of mode->vrefresh and just calculate it on demand. Saves a bit of space and avoids the cached value getting out of sync with reality. Mostly done with cocci, with the following manual fixups: - Remove the now empty loop in drm_helper_probe_single_connector_modes() - Fix __MODE() macro in ch7006_mode.c - Fix DRM_MODE_ARG() macro in drm_modes.h - Remove leftover comment from samsung_s6d16d0_mode - Drop the TODO @@ @@ struct drm_display_mode { ... - int vrefresh; ... }; @@ identifier N; expression E; @@ struct drm_display_mode N = { - .vrefresh = E }; @@ identifier N; expression E; @@ struct drm_display_mode N[...] = { ..., { - .vrefresh = E } ,... }; @@ expression E; @@ { DRM_MODE(...), - .vrefresh = E, } @@ identifier M, R; @@ int drm_mode_vrefresh(const struct drm_display_mode *M) { ... - if (M->vrefresh > 0) - R = M->vrefresh; - else if (...) { ... } ... } @@ struct drm_display_mode *p; expression E; @@ ( - p->vrefresh = E; | - p->vrefresh + drm_mode_vrefresh(p) ) @@ struct drm_display_mode s; expression E; @@ ( - s.vrefresh = E; | - s.vrefresh + drm_mode_vrefresh(&s) ) @@ expression E; @@ - drm_mode_vrefresh(E) ? drm_mode_vrefresh(E) : drm_mode_vrefresh(E) + drm_mode_vrefresh(E) @find_substruct@ identifier X; identifier S; @@ struct X { ... struct drm_display_mode S; ... }; @@ identifier find_substruct.S; expression E; identifier I; @@ { .S = { - .vrefresh = E } } @@ identifier find_substruct.S; identifier find_substruct.X; expression E; identifier I; @@ struct X I[...] = { ..., .S = { - .vrefresh = E } ,... }; v2: Drop TODO v3: Rebase v4: Rebase Cc: Andrzej Hajda <[email protected]> Cc: Neil Armstrong <[email protected]> Cc: Laurent Pinchart <[email protected]> Cc: Jonas Karlman <[email protected]> Cc: Jernej Skrabec <[email protected]> Cc: Inki Dae <[email protected]> Cc: Joonyoung Shim <[email protected]> Cc: Seung-Woo Kim <[email protected]> Cc: Kyungmin Park <[email protected]> Cc: Linus Walleij <[email protected]> Cc: CK Hu <[email protected]> Cc: Philipp Zabel <[email protected]> Cc: Ben Skeggs <[email protected]> Cc: Thierry Reding <[email protected]> Cc: Sam Ravnborg <[email protected]> Cc: Jerry Han <[email protected]> Cc: Icenowy Zheng <[email protected]> Cc: Jagan Teki <[email protected]> Cc: Stefan Mavrodiev <[email protected]> Cc: Robert Chiras <[email protected]> Cc: "Guido Günther" <[email protected]> Cc: Purism Kernel Team <[email protected]> Cc: Benjamin Gaignard <[email protected]> Cc: Vincent Abriou <[email protected]> Cc: VMware Graphics <[email protected]> Cc: Thomas Hellstrom <[email protected]> Cc: [email protected] Cc: [email protected] Reviewed-by: Laurent Pinchart <[email protected]> Reviewed-by: Emil Velikov <[email protected]> Reviewed-by: Sam Ravnborg <[email protected]> Acked-by: Linus Walleij <[email protected]> Signed-off-by: Ville Syrjälä <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
2020-05-27drm/i915/gt: Clear LOCAL_BIND from shared GGTT on resumeChris Wilson1-2/+6
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: bf0840cdb304 ("drm/i915/gt: Stop cross-polluting PIN_GLOBAL with PIN_USER with no-ppgtt") Signed-off-by: Chris Wilson <[email protected]> Reviewed-by: Matthew Auld <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
2020-05-27drm/i915/gt: Do not schedule normal requests immediately along virtualChris Wilson1-4/+21
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: 6d06779e8672 ("drm/i915: Load balancing across a virtual engine") Testcase: igt/gem_exec_balancer/sliced Signed-off-by: Chris Wilson <[email protected]> Cc: Tvrtko Ursulin <[email protected]> Cc: <[email protected]> # v5.3+ Reviewed-by: Tvrtko Ursulin <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
2020-05-27drm/i915: Reorder await_execution before await_requestChris Wilson1-132/+132
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: Chris Wilson <[email protected]> Reviewed-by: Tvrtko Ursulin <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
2020-05-26drm/i915: Introduce some local intel_dp variablesVille Syrjälä1-6/+12
The drrs code dereferences mode->vrefresh via some really long chain of structures/pointers. Couldn't get coccinelle to see through all that so let's add some local variables to help it. Reviewed-by: Emil Velikov <[email protected]> Signed-off-by: Ville Syrjälä <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
2020-05-26drm/i915/display: Only query DP state of a DDI encoderChris Wilson1-0/+3
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/1892 Signed-off-by: Chris Wilson <[email protected]> Acked-by: Ville Syrjälä <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
2020-05-26drm/i915: Improve execute_cb struct packingChris Wilson2-10/+15
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 <[email protected]> Reviewed-by: Mika Kuoppala <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
2020-05-26drm/i915/execlists: Shortcircuit queue_prio() for no internal levelsChris Wilson1-0/+3
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 <[email protected]> Reviewed-by: Mika Kuoppala <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
2020-05-26drm/i915/gt: Force the GT reset on shutdownChris Wilson1-0/+5
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/489 Signed-off-by: Chris Wilson <[email protected]> Cc: [email protected] Reviewed-by: Mika Kuoppala <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
2020-05-26drm/i915/display: Fix early deref of 'dsb'Chris Wilson1-2/+4
drivers/gpu/drm/i915/display/intel_dsb.c:177 intel_dsb_reg_write() warn: variable dereferenced before check 'dsb' (see line 175) Fixes: afeda4f3b1c8 ("drm/i915/dsb: Pre allocate and late cleanup of cmd buffer") Signed-off-by: Chris Wilson <[email protected]> Cc: Ville Syrjälä <[email protected]> Cc: Animesh Manna <[email protected]> Cc: Uma Shankar <[email protected]> Reviewed-by: Animesh Manna <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
2020-05-25drm/i915/gt: Stop cross-polluting PIN_GLOBAL with PIN_USER with no-ppgttChris Wilson1-8/+3
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/1880 Signed-off-by: Chris Wilson <[email protected]> Acked-by: Mika Kuoppala <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
2020-05-25drm/i915/gem: Suppress some random warningsChris Wilson4-7/+4
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 <[email protected]> Reviewed-by: Mika Kuoppala <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
2020-05-25drm/i915/gt: Cancel the flush worker more thoroughlyChris Wilson1-1/+2
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/1920 Signed-off-by: Chris Wilson <[email protected]> Reviewed-by: Mika Kuoppala <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
2020-05-25drm/i915/gem: Avoid iterating an empty listChris Wilson1-6/+9
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: 85d1225ec066 ("drm/i915: Introduce & use new lightweight SGL iterators") Signed-off-by: Chris Wilson <[email protected]> Cc: Matthew Auld <[email protected]> Cc: Tvrtko Ursulin <[email protected]> Cc: <[email protected]> # v4.8+ Reviewed-by: Matthew Auld <[email protected]> Reviewed-by: Maciej Patelczyk <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected] (cherry picked from commit 957ad9a02be6faa87594c58ac09460cd3d190d0e) Signed-off-by: Joonas Lahtinen <[email protected]>
2020-05-25drm/i915: Avoid using rq->engine after free during i915_fence_releaseChris Wilson1-2/+33
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: 43acd6516ca9 ("drm/i915: Keep a per-engine request pool") Signed-off-by: Chris Wilson <[email protected]> Cc: Tvrtko Ursulin <[email protected]> Reviewed-by: Tvrtko Ursulin <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected] (cherry picked from commit 32a4605b38c30689a6a18f3f4c7d3133ac9d3277) Signed-off-by: Joonas Lahtinen <[email protected]>
2020-05-25drm/i915: Disable semaphore inter-engine sync without timeslicingChris Wilson1-2/+2
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: 18e4af04d218 ("drm/i915: Drop no-semaphore boosting") Signed-off-by: Chris Wilson <[email protected]> Cc: Tvrtko Ursulin <[email protected]> Cc: Mika Kuoppala <[email protected]> Reviewed-by: Tvrtko Ursulin <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected] (cherry picked from commit 0eb670aac27b1d615004c29efec595616e3e091a) Signed-off-by: Joonas Lahtinen <[email protected]>
2020-05-25drm/i915/gt: Remove errant assertion in __intel_context_do_pinChris Wilson1-2/+0
This assertion was removed in commit b412c63f1cba ("drm/i915/gt: Report context-is-closed prior to pinning"), but accidentally restored by a cherry-pick into drm-next and now has percolated back to drm-intel-next-queued. Fixes: 2e46a2a0b014 ("drm/i915: Use explicit flag to mark unreachable intel_context") Fixes: 2b703bbda271 ("Merge drm/drm-next into drm-intel-next-queued") References: b412c63f1cba ("drm/i915/gt: Report context-is-closed prior to pinning") Signed-off-by: Chris Wilson <[email protected]> Cc: Rodrigo Vivi <[email protected]> Cc: Joonas Lahtinen <[email protected]> Reviewed-by: Joonas Lahtinen <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected] (cherry picked from commit f2c1061a3677b400a945d9238f17bf33d669acff) Signed-off-by: Joonas Lahtinen <[email protected]>
2020-05-25drm/i915: Don't set queue-priority hint when supressing the rescheduleChris Wilson1-8/+8
We recorded the execlists->queue_priority_hint update for the inflight request without kicking the tasklet. The next submitted request then failed to be scheduled as it had a lower priority than the hint, leaving the HW running with only the inflight request. Fixes: 6cebcf746f3f ("drm/i915: Tweak scheduler's kick_submission()") Signed-off-by: Chris Wilson <[email protected]> Reviewed-by: Mika Kuoppala <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected] (cherry picked from commit b86fc6e5e89e5645b43f57171c26740ef38f9f4a) Signed-off-by: Joonas Lahtinen <[email protected]>
2020-05-23drm/i915/dsb: Pre allocate and late cleanup of cmd bufferAnimesh Manna6-194/+206
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 <[email protected]> Cc: Ville Syrjälä <[email protected]> Cc: Jani Nikula <[email protected]> Cc: Daniel Vetter <[email protected]> Acked-by: Daniel Vetter <[email protected]> Reviewed-by: Maarten Lankhorst <[email protected]> Signed-off-by: Animesh Manna <[email protected]> Signed-off-by: Uma Shankar <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
2020-05-22drm/i915/gem: Avoid iterating an empty listChris Wilson1-6/+9
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: 85d1225ec066 ("drm/i915: Introduce & use new lightweight SGL iterators") Signed-off-by: Chris Wilson <[email protected]> Cc: Matthew Auld <[email protected]> Cc: Tvrtko Ursulin <[email protected]> Cc: <[email protected]> # v4.8+ Reviewed-by: Matthew Auld <[email protected]> Reviewed-by: Maciej Patelczyk <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
2020-05-22drm/i915: Fix includes and local vars orderStanislav Lisovskiy6-24/+30
Removed duplicate include and fixed comment > 80 chars. v2: Added newline after system include and between functions Reviewed-by: Chris Wilson <[email protected]> Signed-off-by: Stanislav Lisovskiy <[email protected]> Signed-off-by: Chris Wilson <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
2020-05-21drm/i915/ehl: Extend w/a 14010685332 to JSP/MCCSwathi Dhanavanthri1-2/+4
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 <[email protected]> Reviewed-by: José Roberto de Souza <[email protected]> Signed-off-by: José Roberto de Souza <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
2020-05-21drm/i915: Remove unneeded hack now for CDCLKStanislav Lisovskiy1-12/+0
No need to bump up CDCLK now, as it is now correctly calculated, accounting for DBuf BW as BSpec says. Reviewed-by: Manasi Navare <[email protected]> Signed-off-by: Stanislav Lisovskiy <[email protected]> Signed-off-by: Manasi Navare <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
2020-05-21drm/i915: Adjust CDCLK accordingly to our DBuf bw needsStanislav Lisovskiy8-15/+220
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 <[email protected]> Reviewed-by: Manasi Navare <[email protected]> Signed-off-by: Manasi Navare <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
2020-05-21drm/i915: Introduce for_each_dbuf_slice_in_mask macroStanislav Lisovskiy2-0/+8
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: Manasi Navare <[email protected]> Signed-off-by: Stanislav Lisovskiy <[email protected]> Signed-off-by: Manasi Navare <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
2020-05-21drm/i915: Plane configuration affects CDCLK in Gen11+Stanislav Lisovskiy1-1/+1
So lets support it. v2: - Fixed "from" field which got corrupted for some weird reason Reviewed-by: Manasi Navare <[email protected]> Signed-off-by: Stanislav Lisovskiy <[email protected]> Signed-off-by: Manasi Navare <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
2020-05-21drm/i915: Check plane configuration properlyStanislav Lisovskiy1-1/+7
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: Manasi Navare <[email protected]> Signed-off-by: Stanislav Lisovskiy <[email protected]> Signed-off-by: Manasi Navare <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
2020-05-21drm/i915: Extract cdclk requirements checking to separate functionStanislav Lisovskiy1-8/+22
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: Manasi Navare <[email protected]> Signed-off-by: Stanislav Lisovskiy <[email protected]> Signed-off-by: Manasi Navare <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]