- 09 Feb, 2023 6 commits
-
-
John Harrison authored
Update a bunch more debug prints to use the new GT based scheme. v2: Also change prints to use %pe for error values (MichalW). Fix a context leak on error due to a -- being too early. Use the correct header file for the debug macros. Signed-off-by: John Harrison <John.C.Harrison@Intel.com> Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20230207050717.1833718-5-John.C.Harrison@Intel.com
-
John Harrison authored
Update a bunch more debug prints to use the new GT based scheme. v2: Upgrade the no node found message to a warning on the grounds of it being quite important if the error capture can't find any register state information. Signed-off-by: John Harrison <John.C.Harrison@Intel.com> Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20230207050717.1833718-4-John.C.Harrison@Intel.com
-
John Harrison authored
Update a bunch more debug prints to use the new GT based scheme. v2: Also change prints to use %pe for error values (MichalW). Signed-off-by: John Harrison <John.C.Harrison@Intel.com> Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20230207050717.1833718-3-John.C.Harrison@Intel.com
-
John Harrison authored
Update a bunch more debug prints to use the new GT based scheme. v2: Also change prints to use %pe for error values (MichalW). Signed-off-by: John Harrison <John.C.Harrison@Intel.com> Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20230207050717.1833718-2-John.C.Harrison@Intel.com
-
Lucas De Marchi authored
INF_UNIT_LEVEL_CLKGATE is not replicated, but since it's not actually used it can just be removed. Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20230206165410.3056073-2-lucas.demarchi@intel.com
-
Lucas De Marchi authored
Register 0x9424 is not replicated on any platform, so it shouldn't be declared with REG_MCR(). Declaring it with _MMIO() is basically duplicate of the GEN7 version, so just remove the GEN8 and change all the callers to use the right functions. Old versions of the gen8 bspec page used to contain a table with MCR registers, apparently implying 0x9400 - 0x94ff registers were replicated. However that table went away and there is no information related to the ranges for gen8 anymore. Moreover the current behavior of the driver wouldn't do anything special for 0x9424 since there is no equivalent table in intel_gt_mcr.c: the driver would just fallback to intel_uncore_{read,write}(). Therefore, do not care about the possible special case for gen8 and just use the register as non-MCR for all the platforms. One place doing read + write is also converted to intel_uncore_rmw(). v2: Reword commit message adding the justification wrt gen8 Fixes: a9e69428 ("drm/i915: Define MCR registers explicitly") Cc: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: Gustavo Sousa <gustavo.sousa@intel.com> Cc: Matt Atwood <matthew.s.atwood@intel.com> Cc: Ashutosh Dixit <ashutosh.dixit@intel.com> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20230206165410.3056073-1-lucas.demarchi@intel.com
-
- 08 Feb, 2023 4 commits
-
-
Matt Roper authored
The UNSLICE_UNIT_LEVEL_CLKGATE register programmed by this workaround has 'BUS' style reset, indicating that it does not lose its value on engine resets. Furthermore, this register is part of the GT forcewake domain rather than the RENDER domain, so it should not be impacted by RCS engine resets. As such, we should implement this on the GT workaround list rather than an engine list. Bspec: 19219 Fixes: 3551ff92 ("drm/i915/gen11: Moving WAs to rcs_engine_wa_init()") Signed-off-by: Matt Roper <matthew.d.roper@intel.com> Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20230201222831.608281-2-matthew.d.roper@intel.com
-
Matt Roper authored
XEHPC_LNCFMISCCFGREG0 and XEHPC_L3SCRUB are both in MCR register ranges on PVC (with HALFBSLICE and L3BANK replication respectively), so they should be explicitly declared as MCR registers and use MCR-aware workaround handlers. The workarounds/tuning settings should still be applied properly on PVC even without the MCR annotation, but readback verification on CONFIG_DRM_I915_DEBUG_GEM builds could potentitally give false positive "workaround lost on load" warnings on parts fused such that a unicast read targets a terminated register instance. Fixes: a9e69428 ("drm/i915: Define MCR registers explicitly") Signed-off-by: Matt Roper <matthew.d.roper@intel.com> Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20230201222831.608281-1-matthew.d.roper@intel.com
-
Jani Nikula authored
__le64 and friends should go through the cpu_to_* and *_to_cpu accessors: drivers/gpu/drm/i915/pxp/intel_pxp_huc.c:41:35: warning: incorrect type in assignment (different base types) drivers/gpu/drm/i915/pxp/intel_pxp_huc.c:41:35: expected restricted __le64 [assigned] [usertype] huc_base_address drivers/gpu/drm/i915/pxp/intel_pxp_huc.c:41:35: got unsigned long long [assigned] [usertype] huc_phys_addr Cc: Tomas Winkler <tomas.winkler@intel.com> Cc: Alan Previn <alan.previn.teres.alexis@intel.com> Signed-off-by: Jani Nikula <jani.nikula@intel.com> Reviewed-by: Tomas Winkler <tomas.winkler@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20230207124026.2105442-4-jani.nikula@intel.com
-
Jani Nikula authored
Annotate intel_gt_mcr_lock() and intel_gt_mcr_unlock() to fix sparse warnings: drivers/gpu/drm/i915/gt/intel_gt_mcr.c:397:9: warning: context imbalance in 'intel_gt_mcr_lock' - wrong count at exit drivers/gpu/drm/i915/gt/intel_gt_mcr.c:412:6: warning: context imbalance in 'intel_gt_mcr_unlock' - unexpected unlock Cc: Matt Roper <matthew.d.roper@intel.com> Signed-off-by: Jani Nikula <jani.nikula@intel.com> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20230207124026.2105442-1-jani.nikula@intel.com
-
- 07 Feb, 2023 2 commits
-
-
Aravind Iddamsetty authored
During module load the punit might still be busy with its booting routines. During this time we try to communicate with it but we fail because we don't receive any feedback from it and we return immediately with a -EINVAL fatal error. At this point the driver load is "dramatically" aborted. The following error message notifies us about it. i915 0000:4d:00.0: drm_WARN_ON_ONCE(timeout_base_ms > 3) It would be enough to wait a little in order to give the punit the chance to come up bright and shiny, ready to interact with the driver. Wait up 10 seconds for the punit to settle and complete any outstanding transactions upon module load. If it still fails try again with a longer timeout, 180s, 3 minutes. If it still fails then return -EPROBE_DEFER, in order to give the punit a second chance. Even if these timers might look long, we should consider that the punit, depending on the platforms, might need long times to complete its routines. Besides we want to try anything possible to move forward before deciding to abort the driver's load. The issue has been reported in: https://gitlab.freedesktop.org/drm/intel/-/issues/7814 The changes in this patch are valid only and uniquely during boot. The common transactions with the punit during the driver's normal operation are not affected. Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty@intel.com> Co-developed-by: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20230206183236.109908-1-andi.shyti@linux.intel.com
-
Michal Wajdeczko authored
Like we did it for GuC, introduce some helper print macros for HuC to have unified format of messages that also include GT#. While around improve some messages and use %pe if possible. v2: update GSC/PXP timeout message Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: John Harrison <John.C.Harrison@Intel.com> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Signed-off-by: John Harrison <John.C.Harrison@Intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20230203085912.1963-1-michal.wajdeczko@intel.com
-
- 06 Feb, 2023 3 commits
-
-
Bagas Sanjaya authored
Stephen Rothwell reported htmldocs warnings: Documentation/gpu/i915:64: drivers/gpu/drm/i915/gt/intel_workarounds.c:32: WARNING: Inline emphasis start-string without end-string. Documentation/gpu/i915:64: drivers/gpu/drm/i915/gt/intel_workarounds.c:57: WARNING: Inline emphasis start-string without end-string. Documentation/gpu/i915:64: drivers/gpu/drm/i915/gt/intel_workarounds.c:66: WARNING: Inline emphasis start-string without end-string. Escape wildcards in *_ctx_workarounds_init(), *_gt_workarounds_init(), and *_whitelist_build() to fix above warnings. Link: https://lore.kernel.org/linux-next/20230203134622.0b6315b9@canb.auug.org.au/ Fixes: 0c3064cf ("drm/i915/doc: Document where to implement register workarounds") Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> Signed-off-by: Bagas Sanjaya <bagasdotme@gmail.com> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20230203100215.31852-2-bagasdotme@gmail.com
-
Aravind Iddamsetty authored
Obj flags for shmem objects is not being set correctly. Fixes in setting BO_ALLOC_USER flag which applies to shmem objs as well. v2: Add fixes tag (Tvrtko, Matt A) Fixes: 13d29c82 ("drm/i915/ehl: unconditionally flush the pages on acquire") Cc: <stable@vger.kernel.org> # v5.15+ Cc: Matthew Auld <matthew.auld@intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Reviewed-by: Matthew Auld <matthew.auld@intel.com> Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty@intel.com> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> [tursulin: Grouped all tags together.] Link: https://patchwork.freedesktop.org/patch/msgid/20230203135205.4051149-1-aravind.iddamsetty@intel.com
-
Rob Clark authored
Because eb_composite_fence_create() drops the fence_array reference after creation of the sync_file, only the sync_file holds a ref to the fence. But fd_install() makes that reference visable to userspace, so it must be the last thing we do with the fence. Signed-off-by: Rob Clark <robdclark@chromium.org> Fixes: 00dae4d3 ("drm/i915: Implement SINGLE_TIMELINE with a syncobj (v4)") Cc: <stable@vger.kernel.org> # v5.15+ [tursulin: Added stable tag.] Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20230203164937.4035503-1-robdclark@gmail.com
-
- 03 Feb, 2023 1 commit
-
-
Nirmoy Das authored
DSM granularity is 1MB so make sure we stick to that. The address set by firmware in GEN12_DSMBASE in driver initialization doesn't mean "anything above that and until end of lmem is part of DSM". In fact, there may be a few KB that is not part of DSM on the end of lmem. How large is that space is platform-dependent, but since it's always less than the DSM granularity, it can be simplified by simply aligning the size down. v2: replace "1 * SZ_1M" with SZ_1M (Andrzej). v3: reword commit message to explain why the round down is needed (Lucas) Cc: Matthew Auld <matthew.auld@intel.com> Suggested-by: Lucas De Marchi <lucas.demarchi@intel.com> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com> Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20230202180243.23637-1-nirmoy.das@intel.com
-
- 02 Feb, 2023 2 commits
-
-
Michal Wajdeczko authored
Just recently we switched over to new GuC oriented log macros but in the meantime yet another message was added that we missed to update. While around improve that new message by adding engine name and use existing helpers to check for context state. Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: John Harrison <John.C.Harrison@Intel.com> Reviewed-by: John Harrison <John.C.Harrison@Intel.com> Signed-off-by: John Harrison <John.C.Harrison@Intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20230131214413.1879-1-michal.wajdeczko@intel.com
-
Nirmoy Das authored
Use sysfs_emit() and sysfs_emit_at() in show() callback as recommended by Documentation/filesystems/sysfs.rst Cc: Andi Shyti <andi.shyti@linux.intel.com> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com> Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20230130131358.16800-1-nirmoy.das@intel.com
-
- 01 Feb, 2023 1 commit
-
-
Chris Wilson authored
Check that we invalidate the TLB cache, the updated physical addresses are immediately visible to the HW, and there is no retention of the old physical address for concurrent HW access. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> [ahajda: adjust to upstream driver, v2+] Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> [tursulin: Small indentation fix.] Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20230130165058.1647414-1-andrzej.hajda@intel.com
-
- 31 Jan, 2023 9 commits
-
-
John Harrison authored
Wa_22011802037 requires waiting for an engine-specific register to clear. A missing entry for GSC engine in the register table is flagged as a drm_err. The drm_err was originally intended to catch missing register entries for newer engines, however, it was later found that the WA is only required for 'legacy' engines. So just drop the drm_err. Signed-off-by: John Harrison <John.C.Harrison@Intel.com> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20230124231111.1786429-1-umesh.nerlige.ramappa@intel.com
-
Michal Wajdeczko authored
Use new macros to have common prefix that also include GT#. v2: pass gt to print_fw_ver v3: prefer guc_dbg in suspend/resume logs Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: John Harrison <John.C.Harrison@Intel.com> Reviewed-by: John Harrison <John.C.Harrison@Intel.com> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20230128195907.1837-9-michal.wajdeczko@intel.com
-
Michal Wajdeczko authored
Use new macros to have common prefix that also include GT#. v2: improve few existing messages Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: John Harrison <John.C.Harrison@Intel.com> Reviewed-by: John Harrison <John.C.Harrison@Intel.com> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20230128195907.1837-8-michal.wajdeczko@intel.com
-
Michal Wajdeczko authored
Use new macros to have common prefix that also include GT#. v2: drop redundant GuC strings, minor improvements v3: more message improvements Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: John Harrison <John.C.Harrison@Intel.com> Reviewed-by: John Harrison <John.C.Harrison@Intel.com> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20230128195907.1837-7-michal.wajdeczko@intel.com
-
Michal Wajdeczko authored
Use new macros to have common prefix that also include GT#. Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: John Harrison <John.C.Harrison@Intel.com> Reviewed-by: John Harrison <John.C.Harrison@Intel.com> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20230128195907.1837-6-michal.wajdeczko@intel.com
-
Michal Wajdeczko authored
Use new macros to have common prefix that also include GT#. v2: drop unused helpers Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: John Harrison <John.C.Harrison@Intel.com> Reviewed-by: John Harrison <John.C.Harrison@Intel.com> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20230128195907.1837-5-michal.wajdeczko@intel.com
-
Michal Wajdeczko authored
Use new macros to have common prefix that also include GT#. Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: John Harrison <John.C.Harrison@Intel.com> Reviewed-by: John Harrison <John.C.Harrison@Intel.com> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20230128195907.1837-4-michal.wajdeczko@intel.com
-
Michal Wajdeczko authored
Use new macros to have common prefix that also include GT#. v2: drop now redundant "GuC" word from the message Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: John Harrison <John.C.Harrison@Intel.com> Reviewed-by: John Harrison <John.C.Harrison@Intel.com> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20230128195907.1837-3-michal.wajdeczko@intel.com
-
Michal Wajdeczko authored
While we do have GT oriented print macros, add few more GuC specific to have common look and feel across all messages related to the GuC and to avoid chasing the gt pointer. We will use these macros shortly in upcoming patches. Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: John Harrison <John.C.Harrison@Intel.com> Reviewed-by: John Harrison <John.C.Harrison@Intel.com> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20230128195907.1837-2-michal.wajdeczko@intel.com
-
- 30 Jan, 2023 1 commit
-
-
Rob Clark authored
A userspace with multiple threads racing I915_GEM_SET_TILING to set the tiling to I915_TILING_NONE could trigger a double free of the bit_17 bitmask. (Or conversely leak memory on the transition to tiled.) Move allocation/free'ing of the bitmask within the section protected by the obj lock. Signed-off-by: Rob Clark <robdclark@chromium.org> Fixes: 2850748e ("drm/i915: Pull i915_vma_pin under the vm->mutex") Cc: <stable@vger.kernel.org> # v5.5+ [tursulin: Correct fixes tag and added cc stable.] Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20230127200550.3531984-1-robdclark@gmail.com
-
- 27 Jan, 2023 9 commits
-
-
John Harrison authored
The GuC specific register state entry in the error capture object was just called 'capture'. Although the companion 'node' entry was called 'guc_capture_node'. Rename the base entry to be 'guc_capture' instead so that it is a) more consistent and b) more obvious what it is. Signed-off-by: John Harrison <John.C.Harrison@Intel.com> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20230127002842.3169194-9-John.C.Harrison@Intel.com
-
John Harrison authored
For understanding bug reports, it can be useful to have an explicit dmesg print when a reset notification is received from GuC. As opposed to simply inferring that this happened from other messages. Signed-off-by: John Harrison <John.C.Harrison@Intel.com> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20230127002842.3169194-8-John.C.Harrison@Intel.com
-
John Harrison authored
Engine resets are supposed to never fail. But in the case when one does (due to unknown reasons that normally come down to a missing w/a), it is useful to get as much information out of the system as possible. Given that the GuC intentionally dies on such a situation, it is not possible to get a guilty context notification back. So do a manual search instead. Given that GuC is dead, this is safe because GuC won't be changing the engine state asynchronously. v2: Change comment to be less alarming (Tvrtko) Signed-off-by: John Harrison <John.C.Harrison@Intel.com> Acked-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20230127002842.3169194-7-John.C.Harrison@Intel.com
-
John Harrison authored
A hang situation has been observed where the only requests on the context were either completed or not yet started according to the breaadcrumbs. However, the register state claimed a batch was (maybe) in progress. So, allow capture of the pending request on the grounds that this might be better than nothing. v2: Reword 'not started' warning message (Tvrtko) Signed-off-by: John Harrison <John.C.Harrison@Intel.com> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20230127002842.3169194-6-John.C.Harrison@Intel.com
-
John Harrison authored
There was a report of error captures occurring without any hung context being indicated despite the capture being initiated by a 'hung context notification' from GuC. The problem was not reproducible. However, it is possible to happen if the context in question has no active requests. For example, if the hang was in the context switch itself then the breadcrumb write would have occurred and the KMD would see an idle context. In the interests of attempting to provide as much information as possible about a hang, it seems wise to include the engine info regardless of whether a request was found or not. As opposed to just prentending there was no hang at all. So update the error capture code to always record engine information if a context is given. Which means updating record_context() to take a context instead of a request (which it only ever used to find the context anyway). And split the request agnostic parts of intel_engine_coredump_add_request() out into a seaprate function. v2: Remove a duplicate 'if' statement (Umesh) and fix a put of a null pointer. v3: Tidy up request locking code flow (Tvrtko) v4: Pull in improved info message from next patch and fix up potential leak of GuC register state (Daniele) Signed-off-by: John Harrison <John.C.Harrison@Intel.com> Reviewed-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> (v2) Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Acked-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20230127002842.3169194-5-John.C.Harrison@Intel.com
-
John Harrison authored
The debugfs dump of requests was confused about what state requires the execlist lock versus the GuC lock. There was also a bunch of duplicated messy code between it and the error capture code. So refactor the hung request search into a re-usable function. And reduce the span of the execlist state lock to only the execlist specific code paths. In order to do that, also move the report of hold count (which is an execlist only concept) from the top level dump function to the lower level execlist specific function. Also, move the execlist specific code into the execlist source file. v2: Rename some functions and move to more appropriate files (Daniele). v3: Rename new execlist dump function (Daniele) Fixes: dc0dad36 ("drm/i915/guc: Fix for error capture after full GPU reset with GuC") Signed-off-by: John Harrison <John.C.Harrison@Intel.com> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Acked-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Matthew Brost <matthew.brost@intel.com> Cc: Jani Nikula <jani.nikula@linux.intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: Matt Roper <matthew.d.roper@intel.com> Cc: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> Cc: Michael Cheng <michael.cheng@intel.com> Cc: Lucas De Marchi <lucas.demarchi@intel.com> Cc: Bruce Chang <yu.bruce.chang@intel.com> Cc: Alan Previn <alan.previn.teres.alexis@intel.com> Cc: Matthew Auld <matthew.auld@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20230127002842.3169194-4-John.C.Harrison@Intel.com
-
John Harrison authored
When GuC support was added to error capture, the reference counting around the request object was broken. Fix it up. The context based search manages the spinlocking around the search internally. So it needs to grab the reference count internally as well. The execlist only request based search relies on external locking, so it needs an external reference count but within the spinlock not outside it. The only other caller of the context based search is the code for dumping engine state to debugfs. That code wasn't previously getting an explicit reference at all as it does everything while holding the execlist specific spinlock. So, that needs updaing as well as that spinlock doesn't help when using GuC submission. Rather than trying to conditionally get/put depending on submission model, just change it to always do the get/put. v2: Explicitly document adding an extra blank line in some dense code (Andy Shevchenko). Fix multiple potential null pointer derefs in case of no request found (some spotted by Tvrtko, but there was more!). Also fix a leaked request in case of !started and another in __guc_reset_context now that intel_context_find_active_request is actually reference counting the returned request. v3: Add a _get suffix to intel_context_find_active_request now that it grabs a reference (Daniele). v4: Split the intel_guc_find_hung_context change to a separate patch and rename intel_context_find_active_request_get to intel_context_get_active_request (Tvrtko). v5: s/locking/reference counting/ in commit message (Tvrtko) Fixes: dc0dad36 ("drm/i915/guc: Fix for error capture after full GPU reset with GuC") Fixes: 573ba126 ("drm/i915/guc: Capture error state on context reset") Signed-off-by: John Harrison <John.C.Harrison@Intel.com> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Acked-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Matthew Brost <matthew.brost@intel.com> Cc: Jani Nikula <jani.nikula@linux.intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: Andrzej Hajda <andrzej.hajda@intel.com> Cc: Matthew Auld <matthew.auld@intel.com> Cc: Matt Roper <matthew.d.roper@intel.com> Cc: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> Cc: Michael Cheng <michael.cheng@intel.com> Cc: Lucas De Marchi <lucas.demarchi@intel.com> Cc: Tejas Upadhyay <tejaskumarx.surendrakumar.upadhyay@intel.com> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Cc: Aravind Iddamsetty <aravind.iddamsetty@intel.com> Cc: Alan Previn <alan.previn.teres.alexis@intel.com> Cc: Bruce Chang <yu.bruce.chang@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20230127002842.3169194-3-John.C.Harrison@Intel.com
-
John Harrison authored
intel_guc_find_hung_context() was not acquiring the correct spinlock before searching the request list. So fix that up. While at it, add some extra whitespace padding for readability. Fixes: dc0dad36 ("drm/i915/guc: Fix for error capture after full GPU reset with GuC") Signed-off-by: John Harrison <John.C.Harrison@Intel.com> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Acked-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Matthew Brost <matthew.brost@intel.com> Cc: Jani Nikula <jani.nikula@linux.intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: Matt Roper <matthew.d.roper@intel.com> Cc: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> Cc: Michael Cheng <michael.cheng@intel.com> Cc: Lucas De Marchi <lucas.demarchi@intel.com> Cc: Tejas Upadhyay <tejaskumarx.surendrakumar.upadhyay@intel.com> Cc: Chris Wilson <chris.p.wilson@intel.com> Cc: Bruce Chang <yu.bruce.chang@intel.com> Cc: Alan Previn <alan.previn.teres.alexis@intel.com> Cc: Matthew Auld <matthew.auld@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20230127002842.3169194-2-John.C.Harrison@Intel.com
-
Rob Clark authored
Adding the vm to the vm_xa table makes it visible to userspace, which could try to race with us to close the vm. So we need to take our extra reference before putting it in the table. Signed-off-by: Rob Clark <robdclark@chromium.org> Reviewed-by: Matthew Auld <matthew.auld@intel.com> Fixes: 9ec8795e ("drm/i915: Drop __rcu from gem_context->vm") Cc: <stable@vger.kernel.org> # v5.16+ Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20230119173321.2825472-1-robdclark@gmail.com
-
- 26 Jan, 2023 2 commits
-
-
Matt Roper authored
GAMSTLB_CTRL and GAMCNTRL_CTRL became multicast/replicated registers on Xe_HP. They should be defined accordingly and use MCR-aware operations. These registers have only been used for some dg2/xehpsdv workarounds, so this fix is mostly just for consistency/future-proofing; even lacking the MCR annotation, workarounds will always be properly applied in a multicast manner on these platforms. Cc: Gustavo Sousa <gustavo.sousa@intel.com> Fixes: 58bc2453 ("drm/i915: Define multicast registers as a new type") Signed-off-by: Matt Roper <matthew.d.roper@intel.com> Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20230125234159.3015385-3-matthew.d.roper@intel.com
-
Matt Roper authored
Workaround Wa_18018781329 has applied to several recent Xe_HP-based platforms. However there are some extra gotchas to implementing this properly for MTL that we need to take into account: * Due to the separation of media and render/compute into separate GTs, this workaround needs to be implemented on each GT, not just the primary GT. Since each class of register only exists on one of the two GTs, we should program the appropriate registers on each GT. * As with past Xe_HP platforms, the registers on the primary GT (Xe_LPG IP) are multicast/replicated registers and should be handled with the MCR-aware functions. However the registers on the media GT (Xe_LPM+ IP) are regular singleton registers and should _not_ use MCR handling. We need to create separate register definitions for the Xe_HP multicast form and the Xe_LPM+ singleton form and use each in the appropriate place. * Starting with MTL, workarounds documented by the hardware teams are technically associated with IP versions/steppings rather than top-level platforms. That means we should take care to check the media IP version rather than the graphics IP version when deciding whether the workaround is needed on the Xe_LPM+ media GT (in this case the workaround applies to both IPs and the stepping bounds are identical, but we should still write the code appropriately to set a proper precedent for future workaround implementations). * It's worth noting that the GSC register and the CCS register are defined with the same MMIO offset (0xCF30). Since the CCS is only relevant to the primary GT and the GSC is only relevant to the media GT there isn't actually a clash here (the media GT automatically adds the additional 0x380000 GSI offset). However there's currently a glitch in the bspec where the CCS register doesn't show up at all and the GSC register is listed as existing on both GTs. That's a known documentation problem for several registers with shared GSC/CCS offsets; rest assured that the CCS register really does still exist. Cc: Gustavo Sousa <gustavo.sousa@intel.com> Signed-off-by: Matt Roper <matthew.d.roper@intel.com> Fixes: 41bb543f ("drm/i915/mtl: Add initial gt workarounds") Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20230125234159.3015385-2-matthew.d.roper@intel.com
-