- 26 Jul, 2019 4 commits
-
-
Chris Wilson authored
Smatch warning that the loop may be empty causing us to check err before it had been set. Ensure that it is initialised to 0, just in case. v2: Refactor the inner loop for better scooping and clarity Fixes: a9877da2 ("drm/i915/oa: Reconfigure contexts on the fly") Signed-off-by:
Chris Wilson <chris@chris-wilson.co.uk> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Reviewed-by:
Lionel Landwerlin <lionel.g.landwerlin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190726131458.8310-1-chris@chris-wilson.co.uk
-
Daniele Ceraolo Spurio authored
guc->stage_desc_pool is required as part of the init parameters and there is no reason we have to init them after HuC. This fixes a NULL ptr dereference due to guc->stage_desc_pool not being set (no fixes tag since GuC submission can't be enabled yet). Signed-off-by:
Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by:
Michal Wajdeczko <michal.wajdeczko@intel.com> Signed-off-by:
-
Michal Wajdeczko authored
All intel_uc_fw_* functions are taking uc_fw as first param except intel_uc_fw_fetch() which is taking i915. Fix that. Signed-off-by:
-
Chris Wilson authored
Currently we use the engine->active.lock to ensure that the request is not retired as we capture the data. However, we only need to ensure that the vma are not removed prior to use acquiring their contents, and since we have already relinquished our stop-machine protection, we assume that the user will not be overwriting the contents before we are able to record them. In order to capture the vma outside of the spinlock, we acquire a reference and mark the vma as active to prevent it from being unbound. However, since it is tricky allocate an entry in the fence tree (doing so would require taking a mutex) while inside the engine spinlock, we use an atomic bit and special case the handling for i915_active_wait. The core benefit is that we can use some non-atomic methods for mapping the device pages, we can remove the slow compression phase out of atomic context (i.e. stop antagonising the nmi-watchdog), and no we longer need large reserves of atomic pages. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111215Signed-off-by:
Matthew Auld <matthew.william.auld@gmail.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190725223843.8971-1-chris@chris-wilson.co.uk
-
- 25 Jul, 2019 11 commits
-
-
Michal Wajdeczko authored
Generic uc firmware layout definitions are unlikely to change and are separate to other GuC specific definitions. v2: reordered Signed-off-by:
-
Michal Wajdeczko authored
Sphinx was rendering firmware layout as html table, but since we want to add sizes relations switch to plain text graphics. v2: also update text and do it before move (Daniele) Signed-off-by:
-
Michal Wajdeczko authored
We moved GuC related files to new location but we missed to update .rst file with links. References: commit 0f261b24 ("drm/i915/uc: move GuC and HuC files under gt/uc/") Signed-off-by:
-
Daniele Ceraolo Spurio authored
The way we load the firmwares is the same for both GuC and HuC, the only difference is in the wopcm destination address and the dma flags, so we easily can move the logic to a common function and pass in offset and flags. The only other difference in the uplaod path are some the extra steps that guc does before and after the xfer, but those don't require the guc fw to be pinned in ggtt and can safely be performed before calling the uc_upload function. Note that this patch re-introduces the dma xfer wait for guc loading that was removed with "drm/i915/guc: Propagate the fw xfer timeout". This is not going to slow us down on a successful load (the dma has to complete before fw init can start), but could slightly increase the timeout in case of a fw init error. v2: use _fw variants for uncore accesses (Chris), fix guc_fw status on failed wait. v3: use dev_err and print DMA_CTRL (Chris) Signed-off-by: -
Daniele Ceraolo Spurio authored
The gt is our new central structure for uc-related code, so we can use that instead of jumping back to i915 via the fw object. Since we have it in the upload function it is easy to pass it through the lower levels of the xfer process instead of continuosly jumping via uc_fw->uc->gt, which will also make things a bit cleaner for the next patch. Signed-off-by:
-
Daniele Ceraolo Spurio authored
The binary is perma-pinned and the rsa is not going to change, so copy it only once and not on every load. v2: onion unwind (Chris) Signed-off-by:
-
Daniele Ceraolo Spurio authored
The way we copy the RSA is the same for GuC and HuC, so we can move the logic in a common function. this will also make any update needed for local memory easier. v2: return the number of copied bytes and check it (Chris) Signed-off-by:
-
Daniele Ceraolo Spurio authored
We currently track fetch and load status separately, but the 2 are actually sequential in the uc lifetime (fetch must complete before we can attempt the load!). Unifying the 2 variables we can better follow the sequential states and improve our trackng of the uC state. Also, sprinkle some GEM_BUG_ON to make sure we transition correctly between states. v2: rename states, add the running state (Michal), drop some logs in the fetch path (Michal, Chris) v3: re-rename states, extend early status check to all helpers (Michal) Suggested-by: -
Daniele Ceraolo Spurio authored
Instead of having 2 identical functions for GuC and HuC firmware selection, we can unify the selection logic and just use different lists based on FW type. Note that the revid is not relevant for current blobs, but the upcoming CML will be identified as CFL rev 5, so by considering the revid we're ready for that. v2: rework blob list defs (Michal), add order check (Chris), fuse GuC and HuC lists into one. v3: remove difference between no uC HW and no uC FW, simplify related selection code, check the whole fw list (Michal) Signed-off-by: -
Daniele Ceraolo Spurio authored
There are 2 issues around handling of missing uC support: - We treat lack of uC HW and lack of uC FW definition as 2 different cases, but both of them mean that we don't support the uC on the platform we're running on. - We rely on the modparam to decide if we can take uC paths or not, but we don't sanitize it if it is set incorrectly on platform with no uC support. To fix both of them, unify the 2 cases in a single one and sanitize the modparam on invalid configuration (after printing an error message). The log has been adapted as well, since the user doesn't care why we don't support GuC/HuC (no HW or no FW), just that we do not. Developers can easily find the answer based on the platform, so we can simplify the log. Correcting the modparam has been preferred over failing the load since this is what we usually do for non-supported feature (e.g. the now gone enable_ppgtt would fall back to the highest supported PPGTT mode if the selected one was not available). Note that this patch purposely doesn't change the behavior for platforms that do have uC support, in which case we will still fail if enable_guc is set and the firmware is not available on the system. Suggested-by:
-
Daniele Ceraolo Spurio authored
We have several HAS_* checks for GuC and HuC but we mostly use HAS_GUC and HAS_HUC, with only 1 exception. Since our HW always has either both uC or neither of them, just replace all the checks with a unified HAS_UC. v2: use HAS_GT_UC (Michal) v3: fix comment (Michal) Signed-off-by:
-
- 24 Jul, 2019 1 commit
-
-
Daniele Ceraolo Spurio authored
All the GuC objects are perma-pinned, so their offset can't change at runtime. We can therefore set (and log!) the parameters only once during boot. Suggested-by:
-
- 23 Jul, 2019 7 commits
-
-
Chuhong Yuan authored
Instead of using to_pci_dev + pci_get_drvdata, use dev_get_drvdata to make code simpler. Signed-off-by:
Chuhong Yuan <hslester96@gmail.com> Reviewed-by:
-
Daniele Ceraolo Spurio authored
Fix botched refactoring of the code that uncorrectly split a check on a bool, treating it as a u32. Reported-by:
Dan Carpenter <dan.carpenter@oracle.com> Fixes: 84b1ca2f ("drm/i915/uc: prefer intel_gt over i915 in GuC/HuC paths") Signed-off-by:
-
Chris Wilson authored
If the system is already idle, omit the GEM_TRACE saying we are about to wait for idle. It looks confusing in the logs to see a continual stream of wait-for-idle, as one immediately assumes it is stuck in a loop. Signed-off-by:
Stuart Summers <stuart.summers@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190723091218.5886-1-chris@chris-wilson.co.uk
-
Chris Wilson authored
Give the scheduler a chance to breathe by calling cond_resched() as some of the loops may take some time on slower machines, and so catch the attention of the watchdogs. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111196Signed-off-by:
-
Daniele Ceraolo Spurio authored
The microcontrollers are part of GT so it makes logical sense to have them sanitized at the same time. This also fixed an issue with our status tracking where the FW load status is not reset around hibernation. Signed-off-by:
-
Daniele Ceraolo Spurio authored
This was the last place in gt/uc that was still using I915_READ with the global dev_priv. Signed-off-by:
-
Anshuman Gupta authored
To identify the HDCP capability of the display connected to CI systems, we need to add the hdcp capability probing in i915_display_info. This will also help to populate the HDCP capability of the CI systems to CI H/W logs maintained at https://intel-gfx-ci.01.org/hardware/. It will facilitate to determine the kms_content_protection behavior on a particular CI system. v2: Reused the intel_hdcp_info() in i915_hdcp_sink_capability_show(). [Ram] Shifted intel_hdcp_info() to the end of intel_dp_info. [Ram] v3: used seq_puts() instead of seq_pritnf(). [Ram] Cc: daniel.vetter@intel.com Cc: ramalingam.c@intel.com Signed-off-by:
Anshuman Gupta <anshuman.gupta@intel.com> Reviewed-by:
Ramalingam C <ramalingam.c@intel.com> Signed-off-by:
-
- 22 Jul, 2019 4 commits
-
-
Chris Wilson authored
Trust that we now have adequate protection over the low level structures via the engine->active.lock to allow ourselves to capture the GPU error state without the heavy hammer of stop_machine(). Sadly this does mean that we have to forgo some of the lesser used information (not derived from the active state) that is not controlled by the active locks. This includes the list of buffers in the ppGTT and pinned globally in the GGTT. Originally this was used to manually verify relocations, but hasn't been required for sometime and modern mesa now has the habit of ensuring that all interesting buffers within a batch are captured in their entirety (that are the auxiliary state buffers, but not the textures). A useful side-effect is that this allows us to restore error capturing for Braswell and Broxton. v2: Use pagevec for a typical arbitrary number of preallocated pages Signed-off-by:
Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190722222847.24178-1-chris@chris-wilson.co.uk
-
Chris Wilson authored
Prior to freeing the struct, call the fini function to cleanup the common members. Currently this only calls the debug functions to mark the structs as destroyed, but may be extended to real work in future. Signed-off-by:
-
Chris Wilson authored
Remove the outer layer cleanup of engine stubs; as i915_drv itself no longer tries to preallocate and so is not responsible for either the allocation or free. By the time we call the cleanup function, we already have cleaned up the engines. v2: Lack of symmetry between mmio_probe and mmio_release for handling the error cleanup. engine->destroy() is a compound function that is called earlier in the normal release as it ties together other bits of state. Signed-off-by:
-
Chris Wilson authored
The top-level page directory for 36b is a single entry, not multiple like 32b. Fix up the rounding on the calculation of the size of the top level so that we populate the 4th level correctly for 36b. Reported-by:
Jose Souza <jose.souza@intel.com> Signed-off-by:
Abdiel Janulgue <abdiel.janulgue@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190719130737.5835-1-chris@chris-wilson.co.uk
-
- 19 Jul, 2019 13 commits
-
-
YueHaibing authored
Fixes gcc '-Wunused-but-set-variable' warning: drivers/gpu/drm/i915/display/icl_dsi.c: In function 'gen11_dsi_set_transcoder_timings': drivers/gpu/drm/i915/display/icl_dsi.c:768:6: warning: variable 'hfront_porch' set but not used [-Wunused-but-set-variable] It is never used and can be removed. Reported-by:
Hulk Robot <hulkci@huawei.com> Signed-off-by:
YueHaibing <yuehaibing@huawei.com> Signed-off-by:
Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190719015136.103988-1-yuehaibing@huawei.com
-
Michal Wajdeczko authored
When __gen8_ppgtt_alloc fails without allocating anything we should not try to call __gen8_ppgtt_clear as there is nothing to clear and underlying code will complain with: [ 157.861645] gen8_pd_range:881 GEM_BUG_ON(start >= end) Signed-off-by:
-
Chris Wilson authored
Since the underlying __gen8_ppgtt_clear takes the shifted address, we must remember to provide it with the shifted original start address. Reported-by:
-
Tvrtko Ursulin authored
This reverts commit 0629d4da. If GuC firmware is not present on the filesystem driver crashes the machine on boot. Signed-off-by:
Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Fixes: 0629d4da ("drm/i915: Update description of i915.enable_guc modparam") Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: Jani Nikula <jani.nikula@intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Cc: Jani Nikula <jani.nikula@linux.intel.com> Cc: intel-gfx@lists.freedesktop.org Link: https://patchwork.freedesktop.org/patch/msgid/20190719094845.6242-3-tvrtko.ursulin@linux.intel.com
-
Tvrtko Ursulin authored
This reverts commit f774f096. If GuC firmware is not present on the filesystem driver crashes the machine on boot. Signed-off-by:
-
Tvrtko Ursulin authored
We were missing this workaround which can cause hangs if fine grained coherency was used. Signed-off-by:
-
Tvrtko Ursulin authored
Having fixed the incorect MCR programming in an earlier patch, we can now stop ignoring read back of GEN8_L3SQCREG4 during engine workaround verification. Signed-off-by:
-
Tvrtko Ursulin authored
Access to 0xb100 - 0xb3ff mmio range is controlled by the MCR selector which only affects CPU MMIO. Therefore these registers cannot be realiably read with MI_SRM from the command streamer so skip their verification. Signed-off-by:
-
Tvrtko Ursulin authored
A couple issues were present in this code: 1. fls() usage was incorrect causing off by one in subslice mask lookup, which in other words means subslice mask of all zeroes is always used (subslice mask of a slice which is not present, or even out of bounds array access), rendering the checks in wa_init_mcr either futile or random. 2. Condition in WARN_ON was not correct. It is doing a bitwise and operation between a positive (present subslices) and negative mask (disabled L3 banks). This means that with corrected fls() usage the assert would always incorrectly fail. We could fix this by inverting the fuse bits in the check, but instead do one better and improve the code so it not only asserts, but finds the first common index between the two masks and only warns if no such index can be found. v2: * Simplify check for logic and redability. * Improve commentary explaining what is really happening ie. what the assert is really trying to check and why. v3: * Find first common index instead of just asserting. Signed-off-by:
-
Tvrtko Ursulin authored
Instead of re-calculating the MCR selector in read_subslice_reg do the rwm on its existing value and restore it when done. This consolidates MCR programming to one place for cnl+, and avoids re-calculating its default value on older platforms during hangcheck. Signed-off-by:
-
Tvrtko Ursulin authored
fls returns bit positions starting from one for the lsb and the MCR register expects zero based (sub)slice addressing. Incorrent MCR programming can have the effect of directing MMIO reads of registers in the 0xb100-0xb3ff range to invalid subslice returning zeroes instead of actual content. Signed-off-by:
-
YueHaibing authored
Fixes gcc '-Wunused-but-set-variable' warning: drivers/gpu/drm/i915/display/intel_sprite.c: In function 'g4x_sprite_check_scaling': drivers/gpu/drm/i915/display/intel_sprite.c:1494:13: warning: variable 'src_y' set but not used [-Wunused-but-set-variable] Reported-by:
-
Chris Wilson authored
As we unwind the requests for a preemption event, we return a virtual request back to its original virtual engine (so that it is available for execution on any of its siblings). In the process, this means that its breadcrumb should no longer be associated with the original physical engine, and so we are forced to decouple it. Previously, as the request could not complete without our awareness, we would move it to the next real engine without any danger. However, preempt-to-busy allowed for requests to continue on the HW and complete in the background as we unwound, which meant that we could end up retiring the request before fixing up the breadcrumb link. [51679.517943] INFO: trying to register non-static key. [51679.517956] the code is fine but needs lockdep annotation. [51679.517960] turning off the locking correctness validator. [51679.517966] CPU: 0 PID: 3270 Comm: kworker/u8:0 Tainted: G U 5.2.0+ #717 [51679.517971] Hardware name: Intel Corporation NUC7i5BNK/NUC7i5BNB, BIOS BNKBL357.86A.0052.2017.0918.1346 09/18/2017 [51679.518012] Workqueue: i915 retire_work_handler [i915] [51679.518017] Call Trace: [51679.518026] dump_stack+0x67/0x90 [51679.518031] register_lock_class+0x52c/0x540 [51679.518038] ? find_held_lock+0x2d/0x90 [51679.518042] __lock_acquire+0x68/0x1800 [51679.518047] ? find_held_lock+0x2d/0x90 [51679.518073] ? __i915_sw_fence_complete+0xff/0x1c0 [i915] [51679.518079] lock_acquire+0x90/0x170 [51679.518105] ? i915_request_cancel_breadcrumb+0x29/0x160 [i915] [51679.518112] _raw_spin_lock+0x27/0x40 [51679.518138] ? i915_request_cancel_breadcrumb+0x29/0x160 [i915] [51679.518165] i915_request_cancel_breadcrumb+0x29/0x160 [i915] [51679.518199] i915_request_retire+0x43f/0x530 [i915] [51679.518232] retire_requests+0x4d/0x60 [i915] [51679.518263] i915_retire_requests+0xdf/0x1f0 [i915] [51679.518294] retire_work_handler+0x4c/0x60 [i915] [51679.518301] process_one_work+0x22c/0x5c0 [51679.518307] worker_thread+0x37/0x390 [51679.518311] ? process_one_work+0x5c0/0x5c0 [51679.518316] kthread+0x116/0x130 [51679.518320] ? kthread_create_on_node+0x40/0x40 [51679.518325] ret_from_fork+0x24/0x30 [51679.520177] ------------[ cut here ]------------ [51679.520189] list_del corruption, ffff88883675e2f0->next is LIST_POISON1 (dead000000000100) Fixes: 22b7a426 ("drm/i915/execlists: Preempt-to-busy") Signed-off-by:
-
