- 09 Dec, 2022 6 commits
-
-
Daniele Ceraolo Spurio authored
Now that we have the GSC FW support code as a user to the GSC CS, we can add the relevant flag to the engine mask. Note that the engine will still be disabled until we define the GSC FW binary file. Signed-off-by:
Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Cc: Matt Roper <matthew.d.roper@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Reviewed-by:
Rodrigo Vivi <rodrigo.vivi@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20221208200521.2928378-7-daniele.ceraolospurio@intel.com
-
Jonathan Cavitt authored
The GSC CS is only used for communicating with the GSC FW, so no need to initialize it if we're not going to use the FW. If we're not using neither the engine nor the microcontoller, then we can also disable the power well. IMPORTANT: lack of GSC FW breaks media C6 due to opposing requirements between CS setup and forcewake idleness. See in-code comment for detail. Signed-off-by:
Jonathan Cavitt <jonathan.cavitt@intel.com> Signed-off-by:
-
Daniele Ceraolo Spurio authored
If the GSC was loaded, the only way to stop it during the driver unload flow is to do a driver-FLR. The driver-initiated FLR is not the same as PCI config space FLR in that it doesn't reset the SGUnit and doesn't modify the PCI config space. Thus, it doesn't require a re-enumeration of the PCI BARs. However, the driver-FLR does cause a memory wipe of graphics memory on all discrete GPU platforms or a wipe limited to stolen memory on the integrated GPU platforms. We perform the FLR as the last action before releasing the MMIO bar, so that we don't have to care about the consequences of the reset on the unload flow. v2: rename FLR function, add comment to explain FLR impact (Rodrigo), better explain why GSC needs FLR (Alan) Signed-off-by:Alan Previn <alan.previn.teres.alexis@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Reviewed-by:
-
Daniele Ceraolo Spurio authored
GSC FW is loaded by submitting a dedicated command via the GSC engine. The memory area used for loading the FW is then re-purposed as local memory for the GSC itself, so we use a separate allocation instead of using the one where we keep the firmware stored for reload. The GSC is not reset as part of GT reset, so we only need to load it on first boot and S3/S4 exit. v2: use REG_* for register fields definitions (Rodrigo), move to WQ immediately v3: mark worker function as static Bspec: 63347, 65346 Signed-off-by: -
Daniele Ceraolo Spurio authored
The current exectation from the FW side is that the driver will query the GSC FW version after the FW is loaded, similarly to what the mei driver does on DG2. However, we're discussing with the FW team if there is a way to extract the version from the bin file before loading, so we can keep the code the same as for older FWs. Since the GSC FW version is not currently required for functionality and is only needed for debug purposes, we can skip the FW version for now at fetch time and add it later on when we've agreed on the approach. v2: rebased on uc_fw version struct changes. Signed-off-by:
-
Daniele Ceraolo Spurio authored
On MTL the GSC FW needs to be loaded on the media GT by the graphics driver. We're going to treat it like a new uc_fw, so add the initial defs and init/fini functions for it. Similarly to the other FWs, the GSC FW path can be overridden via modparam. The modparam can also be used to disable the GSC FW loading by setting it to an empty string. Note that the new structure has been called intel_gsc_uc to avoid confusion with the existing intel_gsc, which instead represents the heci gsc interfaces. v2: re-order Makefile list to be properly sorted (Jani, Alan), better comment (alan) Signed-off-by:
-
- 08 Dec, 2022 1 commit
-
-
Umesh Nerlige Ramappa authored
An earlier commit introduced a mechanism to parse the context image to find the OA context control offset. This resulted in an NPD on haswell when gem_context was passed into i915_perf_open_ioctl params. Haswell does not support logical ring contexts, so ensure that the context image is parsed only for platforms with logical ring contexts and also validate lrc_reg_state. v2: Fix build failure v3: Fix checkpatch error Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/7432 Fixes: a5c3a3cb ("drm/i915/perf: Determine gen12 oa ctx offset at runtime") Signed-off-by:
Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> Reviewed-by:
Ashutosh Dixit <ashutosh.dixit@intel.com> Signed-off-by:
John Harrison <John.C.Harrison@Intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20221123235342.713068-1-umesh.nerlige.ramappa@intel.com
-
- 07 Dec, 2022 2 commits
-
-
Jani Nikula authored
Remove rmw_set(), rmw_clear(), clear_register(), rmw_set_fw(), and rmw_clear_fw(). They're just one too many levels of abstraction for register access, for very specific purposes. clear_register() seems like a micro-optimization bypassing the write when the register is already clear, but that trick has ceased to work since commit 06b975d5 ("drm/i915: make intel_uncore_rmw() write unconditionally"). Just clear the register in the most obvious way. Signed-off-by:
Jani Nikula <jani.nikula@intel.com> Reviewed-by:
Matt Roper <matthew.d.roper@intel.com> Reviewed-by:
Andrzej Hajda <andrzej.hajda@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20221123164916.4128733-1-jani.nikula@intel.com
-
Ashutosh Dixit authored
hwm_pcode_read_i1 is called during i915 load. This results in the following warning from snb_pcode_read because POWER_SETUP_SUBCOMMAND_READ_I1 is unsupported on DG1/DG2. [drm:snb_pcode_read [i915]] warning: pcode (read from mbox 47c) \ mailbox access failed for snb_pcode_read_p [i915]: -6 The code handles the unsupported command but the warning in dmesg is a red herring which has resulted in a couple of bugs being filed. Therefore silence the warning by avoiding calling snb_pcode_read_p for DG1/DG2. Signed-off-by:
Anshuman Gupta <anshuman.gupta@intel.com> Signed-off-by:
-
- 06 Dec, 2022 5 commits
-
-
Andi Shyti authored
This reverts commit 2ef6efa7. Checking the presence if the IRST (Intel Rapid Start Technology) through the ACPI to decide whether to rebuild or not the GGTT puts us at the mercy of the boot firmware and we need to unnecessarily rely on third parties. Because now we avoid adding scratch pages to the entire GGTT we don't need this hack anymore. Signed-off-by:
Andi Shyti <andi.shyti@linux.intel.com> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Reviewed-by:
Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20221130235805.221010-6-andi.shyti@linux.intel.com
-
Chris Wilson authored
VT-d may cause overfetch of the scanout PTE, both before and after the vma (depending on the scanout orientation). bspec recommends that we provide a tile-row in either directions, and suggests using 168 PTE, warning that the accesses will wrap around the ends of the GGTT. Currently, we fill the entire GGTT with scratch pages when using VT-d to always ensure there are valid entries around every vma, including scanout. However, writing every PTE is slow as on recent devices we perform 8MiB of uncached writes, incurring an extra 100ms during resume. If instead we focus on only putting guard pages around scanout, we can avoid touching the whole GGTT. To avoid having to introduce extra nodes around each scanout vma, we adjust the scanout drm_mm_node to be smaller than the allocated space, and fixup the extra PTE during dma binding. Signed-off-by:
Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by:
Tejas Upadhyay <tejaskumarx.surendrakumar.upadhyay@intel.com> Signed-off-by:
-
Chris Wilson authored
Introduce the concept of padding the i915_vma with guard pages before and after. The major consequence is that all ordinary uses of i915_vma must use i915_vma_offset/i915_vma_size and not i915_vma.node.start/size directly, as the drm_mm_node will include the guard pages that surround our object. The biggest connundrum is how exactly to mix requesting a fixed address with guard pages, particularly through the existing uABI. The user does not know about guard pages, so such must be transparent to the user, and so the execobj.offset must be that of the object itself excluding the guard. So a PIN_OFFSET_FIXED must then be exclusive of the guard pages. The caveat is that some placements will be impossible with guard pages, as wrap arounds need to be avoided, and the vma itself will require a larger node. We must not report EINVAL but ENOSPC as these are unavailable locations within the GTT rather than conflicting user requirements. In the next patch, we start using guard pages for scanout objects. While these are limited to GGTT vma, on a few platforms these vma (or at least an alias of the vma) is shared with userspace, so we may leak the existence of such guards if we are not careful to ensure that the execobj.offset is transparent and excludes the guards. (On such platforms like ivb, without full-ppgtt, userspace has to use relocations so the presence of more untouchable regions within its GTT such be of no further issue.) Signed-off-by:
-
Chris Wilson authored
We already wrap i915_vma.node.start for use with the GGTT, as there we can perform additional sanity checks that the node belongs to the GGTT and fits within the 32b registers. In the next couple of patches, we will introduce guard pages around the objects _inside_ the drm_mm_node allocation. That is we will offset the vma->pages so that the first page is at drm_mm_node.start + vma->guard (not 0 as is currently the case). All users must then not use i915_vma.node.start directly, but compute the guard offset, thus all users are converted to use a i915_vma_offset() wrapper. The notable exceptions are the selftests that are testing exact behaviour of i915_vma_pin/i915_vma_insert. Signed-off-by:
Thomas Hellström <thomas.hellstrom@linux.intel.com> Signed-off-by:
-
Andi Shyti authored
The coming commit "drm/i915: Introduce guard pages to i915_vma" from Chris, was originally changing display_alignment to u32 from u64. The reason is that the display GGTT is and will be limited o 4GB. Put it in a separate patch and use "max(...)" instead of "max_t(64, ...)" when asigning the value. We can safely use max as we know beforehand that the comparison is between two u32 variables. Signed-off-by:
-
- 05 Dec, 2022 3 commits
-
-
Daniele Ceraolo Spurio authored
Invalidating the GuC TLBs while GuC is not loaded does not have negative consequences, so if we're starting the driver with GuC enabled we can use the GGTT invalidation function from the get-go, instead of switching to it when we initialize the GuC objects. In MTL, this fixes and issue where we try to overwrite the invalidation function twice (once for each GuC), due to the GGTT being shared between the primary and media GTs Signed-off-by:
-
Matt Roper authored
The TGL/RKL/DG1/ADL performance tuning guide suggests programming a literal value of 0x2FC0100F for this register. The register's hardware default value is 0x2FC0108F, so this translates to just clearing one bit. Take this opportunity to also clean up the register definition and re-write its existing bits/fields in the preferred notation. Bspec: 31870 Signed-off-by:
Gustavo Sousa <gustavo.sousa@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20221201222210.344152-1-matthew.d.roper@intel.com
-
Matt Roper authored
When determining whether the platform has a hardware-level steering semaphore (i.e., MTL and beyond), we need to use GRAPHICS_VER_FULL() to compare the full version rather than just the major version number returned by GRAPHICS_VER(). Reported-by:
kernel test robot <lkp@intel.com> Fixes: 3100240b ("drm/i915/mtl: Add hardware-level lock for steering") Cc: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com> Signed-off-by:
-
- 02 Dec, 2022 1 commit
-
-
Matt Roper authored
Starting with MTL, the driver needs to not only protect the steering control register from simultaneous software accesses, but also protect against races with hardware/firmware agents. The hardware provides a dedicated locking mechanism to support this via the MTL_STEER_SEMAPHORE register. Reading the register acts as a 'trylock' operation; the read will return 0x1 if the lock is acquired or 0x0 if something else is already holding the lock; once acquired, writing 0x1 to the register will release the lock. We'll continue to grab the software lock as well, just so lockdep can track our locking; assuming the hardware lock is behaving properly, there should never be any contention on the software lock in this case. v2: - Extend hardware semaphore timeout and add a taint for CI if it ever happens (this would imply misbehaving hardware/firmware). (Mika) - Add "MTL_" prefix to new steering semaphore register. (Mika) Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Signed-off-by:
Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20221128233014.4000136-5-matthew.d.roper@intel.com
-
- 01 Dec, 2022 2 commits
-
-
Matt Roper authored
PPAT setup involves a series of multicast writes. This can be optimized slightly be acquiring forcewake and the steering lock just once for the entire sequence. v2: - We should use FW_REG_WRITE instead of FW_REG_READ. (Bala) Suggested-by:
-
Wayne Boyer authored
As per the performance tuning guide, set the HOSTCACHEEN bit to implement the recommended caching policy on PVC. Signed-off-by:
Wayne Boyer <wayne.boyer@intel.com> Reviewed-by:
-
- 30 Nov, 2022 6 commits
-
-
John Harrison authored
The GuC firmware includes an extra version number to specify the submission API level. So use that rather than the main firmware version number for submission related checks. Also, while it is guaranteed that GuC version number components are only 8-bits in size, other firmwares do not have that restriction. So stop making assumptions about them generically fitting in a u16 individually, or in a u32 as a combined 8.8.8. Signed-off-by:
-
John Harrison authored
As a precursor to a coming change (for adding a GuC submission API version), abstract the UC version number into its own private structure separate to the firmware filename. Signed-off-by:
-
John Harrison authored
The way delimiters (underscores and dots) were added to the UC filenames was different for different types of delimiter. Rationalise them to all be done the same way - implicitly in the concatenation macro rather than explicitly in the file name prefix. Signed-off-by:
-
Matt Roper authored
We've been overloading uncore->lock to protect access to the MCR steering register. That's not really what uncore->lock is intended for, and it would be better if we didn't need to hold such a high-traffic spinlock for the whole sequence of (apply steering, access MCR register, restore steering). Let's create a dedicated MCR lock to protect the steering control register over this critical section and stop relying on the high-traffic uncore->lock. For now the new lock is a software lock. However some platforms (MTL and beyond) have a hardware-provided locking mechanism that can be used to serialize not only software accesses, but also hardware/firmware accesses as well; support for that hardware level lock will be added in a future patch. v2: - Use irqsave/irqrestore spinlock calls; platforms using execlist submission rather than GuC submission can perform MCR accesses in interrupt context because reset -> errordump happens in a tasklet. Cc: Chris Wilson <chris.p.wilson@linux.intel.com> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Cc: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com> Signed-off-by:
-
Matt Roper authored
Passing the GT rather than uncore to the lowest level MCR read and write functions will make it easier to introduce dedicated MCR locking in a following patch. Signed-off-by:
-
Matt Roper authored
The kerneldoc function name was not updated when this function was converted to a non-fw form. Fixes: 192bb40f ("drm/i915/gt: Manage uncore->lock while waiting on MCR register") Reported-by:
-
- 29 Nov, 2022 2 commits
-
-
Daniele Ceraolo Spurio authored
The fence is only tracking if the HuC load is in progress or not and doesn't distinguish between already loaded, not supported or disabled, so we can always initialize it to completed, no matter the actual support. We already do that for most platforms, but we skip it on GTs that lack VCS engines (e.g. MTL root GT), so fix that. Note that the cleanup is already unconditional. While at it, move the init/fini to helper functions. Fixes: 02224691 ("drm/i915/huc: fix leak of debug object in huc load fence on driver unload") Signed-off-by:
-
Aravind Iddamsetty authored
On XE_LPM+ platforms the media engines are carved out into a separate GT but have a common GGTMMADR address range which essentially makes the GGTT address space to be shared between media and render GT. As a result any updates in GGTT shall invalidate TLB of GTs sharing it and similarly any operation on GGTT requiring an action on a GT will have to involve all GTs sharing it. setup_private_pat was being done on a per GGTT based as that doesn't touch any GGTT structures moved it to per GT based. BSPEC: 63834 v2: 1. Add details to commit msg 2. includes fix for failure to add item to ggtt->gt_list, as suggested by Lucas 3. as ggtt_flush() is used only for ggtt drop i915_is_ggtt check within it. 4. setup_private_pat moved out of intel_gt_tiles_init v3: 1. Move out for_each_gt from i915_driver.c (Jani Nikula) v4: drop using RCU primitives on ggtt->gt_list as it is not an RCU list (Matt Roper) Cc: Matt Roper <matthew.d.roper@intel.com> Signed-off-by:
Aravind Iddamsetty <aravind.iddamsetty@intel.com> Reviewed-by:
Lucas De Marchi <lucas.demarchi@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20221122070126.4813-1-aravind.iddamsetty@intel.com
-
- 28 Nov, 2022 2 commits
-
-
Matt Atwood authored
Wa_18019271663 applies to all DG2 steppings and skus. Bspec: 66622 Signed-off-by:
Matt Atwood <matthew.s.atwood@intel.com> Reviewed-by:
-
Matt Atwood authored
Wa_18018764978 applies to specific steppings of DG2 (G10 C0+, G11 and G12 A0+). Clean up style in function at the same time. Bspec: 66622 Signed-off-by:
-
- 24 Nov, 2022 2 commits
-
-
Janusz Krzysztofik authored
Users of intel_gt_retire_requests_timeout() expect 0 return value on success. However, we have no protection from passing back 0 potentially returned by a call to dma_fence_wait_timeout() when it succedes right after its timeout has expired. Replace 0 with -ETIME before potentially using the timeout value as return code, so -ETIME is returned if there are still some requests not retired after timeout, 0 otherwise. v3: Use conditional expression, more compact but also better reflecting intention standing behind the change. v2: Move the added lines down so flush_submission() is not affected. Fixes: f33a8a51 ("drm/i915: Merge wait_for_timelines with retire_request") Signed-off-by:Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com> Reviewed-by:
-
Janusz Krzysztofik authored
Commit b97060a9 ("drm/i915/guc: Update intel_gt_wait_for_idle to work with GuC") extended the API of intel_gt_retire_requests_timeout() with an extra argument 'remaining_timeout', intended for passing back unconsumed portion of requested timeout when 0 (success) is returned. However, when request retirement happens to succeed despite an error returned by a call to dma_fence_wait_timeout(), that error code (a negative value) is passed back instead of remaining time. If we then pass that negative value forward as requested timeout to intel_uc_wait_for_idle(), an explicit BUG will be triggered. If request retirement succeeds but an error code is passed back via remaininig_timeout, we may have no clue on how much of the initial timeout might have been left for spending it on waiting for GuC to become idle. OTOH, since all pending requests have been successfully retired, that error code has been already ignored by intel_gt_retire_requests_timeout(), then we shouldn't fail. Assume no more time has been left on error and pass 0 timeout value to intel_uc_wait_for_idle() to give it a chance to return success if GuC is already idle. v3: Don't fail on any error passed back via remaining_timeout. v2: Fix the issue on the caller side, not the provider. Fixes: b97060a9 ("drm/i915/guc: Update intel_gt_wait_for_idle to work with GuC") Signed-off-by:
-
- 23 Nov, 2022 3 commits
-
-
John Harrison authored
It was noticed that the table order verification step was only being run once rather than once per firmware type. Fix that. Note that the long term plan is to convert this code to be a mock selftest. It is already only compiled in when selftests are enabled. And the work involved in the conversion was estimated to be non-trivial. So that conversion is currently low on the priority list. Signed-off-by:
-
Daniele Ceraolo Spurio authored
The fence is always initialized in huc_init_early, but the cleanup in huc_fini is only being run if HuC is enabled. This causes a leaking of the debug object when HuC is disabled/not supported, which can in turn trigger a warning if we try to register a new debug offset at the same address on driver reload. To fix the issue, make sure to always run the cleanup code. Reported-by:
Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> Reported-by:
Brian Norris <briannorris@chromium.org> Fixes: 27536e03 ("drm/i915/huc: track delayed HuC load with a fence") Signed-off-by:
-
Jani Nikula authored
The default_lists array should be in rodata. Fixes: dce2bd54 ("drm/i915/guc: Add Gen9 registers for GuC error state capture.") Cc: Alan Previn <alan.previn.teres.alexis@intel.com> Cc: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> Cc: Lucas De Marchi <lucas.demarchi@intel.com> Signed-off-by:
-
- 22 Nov, 2022 1 commit
-
-
José Roberto de Souza authored
For multi-tile setups the GSC operational only on the tile 0. Skip GSC auxiliary device creation for all other tiles in GSC device init code. Initialize basic GSC fields and use the same path as HECI1 (HECI_PXP) device disable. Cc: Tomas Winkler <tomas.winkler@intel.com> Cc: Vitaly Lubart <vitaly.lubart@intel.com> Signed-off-by:
José Roberto de Souza <jose.souza@intel.com> Signed-off-by:
Alexander Usyskin <alexander.usyskin@intel.com> Acked-by:
Tomas Winkler <tomas.winkler@intel.com> Reviewed-by:
-
- 21 Nov, 2022 4 commits
-
-
Umesh Nerlige Ramappa authored
Engine busyness samples around a 10ms period is failing with busyness ranging approx. from 87% to 115% as shown below. The expected range is +/- 5% of the sample period. Fail 10% of the time. rcs0: reported 11716042ns [91%] busyness while spinning [for 12805719ns] When determining busyness of active engine, the GuC based engine busyness implementation relies on a 64 bit timestamp register read. The latency incurred by this register read causes the failure. On DG1, when the test fails, the observed latencies range from 900us - 1.5ms. Optimizing the 2x32 read by acquiring the lock and forcewake prior to all reg reads reduces the rate of failure to around 2%, but does not eliminate it. In order to make the selftest more robust and always account for such latencies, increase the sample period to 100 ms. This eliminates the issue as seen in a 1000 runs. v2: (Ashutosh) - Add error to commit msg - Include gitlab bug - Update commit for inclusion of 2x32 optimized read Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/4418Signed-off-by:
-
Umesh Nerlige Ramappa authored
PMU reads the GT timestamp as a 2x32 mmio read and since upper and lower 32 bit registers are read in a loop, there is a latency involved between getting the GT timestamp and the CPU timestamp. As part of the resolution, refactor intel_uncore_read64_2x32 to acquire forcewake and uncore lock prior to reading upper and lower regs. Signed-off-by:
-
Vinay Belgaumkar authored
By defaut idle messaging is disabled for GSC CS so to unblock RC6 entry on media tile idle messaging need to be enabled. v2: - Fix review comments (Vinay) - Set GSC idle hysteresis as per spec (Badal) v3: - Fix review comments (Rodrigo) Bspec: 71496 Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Signed-off-by:
Vinay Belgaumkar <vinay.belgaumkar@intel.com> Signed-off-by:
Badal Nilawar <badal.nilawar@intel.com> Reviewed-by:
-
Tvrtko Ursulin authored
In 36537275 ("drm/i915: Simplify internal helper function signature") I broke the old platforms by not noticing engine workaround init does not initialize the list on old platforms. Fix it by always initializing which already does the right thing by mostly not doing anything if there aren't any workarounds on the list. Signed-off-by:
Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Reviewed-by:
-
