- 22 May, 2024 26 commits
-
-
Matthew Auld authored
Unclear why we call this twice. Signed-off-by: Matthew Auld <matthew.auld@intel.com> Cc: Andrzej Hajda <andrzej.hajda@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240522102143.128069-35-matthew.auld@intel.com
-
Matthew Auld authored
Match the i915 display handling here with calling both no_irq and noaccel when removing the device. Signed-off-by: Matthew Auld <matthew.auld@intel.com> Cc: Andrzej Hajda <andrzej.hajda@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240522102143.128069-34-matthew.auld@intel.com
-
Matthew Auld authored
Set our various mmio mappings to NULL. This should make it easier to catch something rogue trying to mess with mmio after device removal. For example, we might unmap everything and then start hitting some mmio address which has already been unmamped by us and then remapped by something else, causing all kinds of carnage. Signed-off-by: Matthew Auld <matthew.auld@intel.com> Cc: Andrzej Hajda <andrzej.hajda@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240522102143.128069-33-matthew.auld@intel.com
-
Matthew Auld authored
Not valid to touch mmio once the device is removed, so make sure we unmap on removal and not just when driver instance goes away. Also set the mmio pointers to NULL to hopefully catch such issues more easily. Signed-off-by: Matthew Auld <matthew.auld@intel.com> Cc: Andrzej Hajda <andrzej.hajda@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240522102143.128069-32-matthew.auld@intel.com
-
Matthew Auld authored
No need to hand roll the onion unwind here, just move gt_remove over to devm which will already have the correct ordering. Signed-off-by: Matthew Auld <matthew.auld@intel.com> Cc: Andrzej Hajda <andrzej.hajda@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240522102143.128069-31-matthew.auld@intel.com
-
Matthew Auld authored
Have a cleaner separation between hw vs sw. v2: Fix missing return Signed-off-by: Matthew Auld <matthew.auld@intel.com> Cc: Andrzej Hajda <andrzej.hajda@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240522102143.128069-30-matthew.auld@intel.com
-
Matthew Auld authored
Here we are using drmm to ensure we release the coredump when unloading the module, however the coredump is very much tied to the struct device underneath. We can see this when we hotunplug the device, for which we have already got a coredump attached. In such a case the coredump still remains and adding another is not possible. However we still register the release action via xe_driver_devcoredump_fini(), so in effect two or more releases for one dump. The other consideration is that the coredump state is embedded in the xe_driver instance, so technically once the drmm release action fires we might free the coredumpe state from a different driver instance, assuming we have two release actions and they can race. Rather use devm here to remove the coredump when the device is released. References: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/1679Signed-off-by: Matthew Auld <matthew.auld@intel.com> Cc: Andrzej Hajda <andrzej.hajda@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240522102143.128069-29-matthew.auld@intel.com
-
Matthew Auld authored
Disable GuC submission when removing the device. Signed-off-by: Matthew Auld <matthew.auld@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240522102143.128069-28-matthew.auld@intel.com
-
Matthew Auld authored
Should be called when driver is removed, not when this particular driver instance is destroyed. Signed-off-by: Matthew Auld <matthew.auld@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240522102143.128069-27-matthew.auld@intel.com
-
Matthew Auld authored
Makes sense to trigger this when the device is removed. Signed-off-by: Matthew Auld <matthew.auld@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240522102143.128069-26-matthew.auld@intel.com
-
Matthew Auld authored
Make it clear that is about cleaning up the HW/FW side, and not software state. Signed-off-by: Matthew Auld <matthew.auld@intel.com> Cc: Andrzej Hajda <andrzej.hajda@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240522102143.128069-25-matthew.auld@intel.com
-
Matthew Auld authored
Here we are touching the HW/GuC and presumably this should happen when the device is removed. Currently if you hotunplug the device this is skipped if there is already open driver instance. Signed-off-by: Matthew Auld <matthew.auld@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240522102143.128069-24-matthew.auld@intel.com
-
Matthew Auld authored
Make it clear that is about cleaning up the HW/FW side, and not software state. Signed-off-by: Matthew Auld <matthew.auld@intel.com> Cc: Andrzej Hajda <andrzej.hajda@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240522102143.128069-23-matthew.auld@intel.com
-
Matthew Auld authored
Make sure to actually call this when the device is removed. Currently we only trigger it when the driver instance goes away, but that doesn't work too well with hotunplug, since device can be removed and re-probed with a new driver instance, where the guc_fini() is called too late. Move the fini over to devm to ensure this is called when device is removed. References: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/1717Signed-off-by: Matthew Auld <matthew.auld@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240522102143.128069-22-matthew.auld@intel.com
-
Matthew Auld authored
Device can be hotunplugged before we start destroying gem objects. In such a case don't touch the GGTT entries, trigger any invalidations or mess around with rpm. This should already be taken care of when removing the device, we just need to take care of dealing with the software state, like removing the mm node. v2: (Andrzej) - Avoid some duplication by tracking the bound status and checking that instead. References: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/1717Signed-off-by: Matthew Auld <matthew.auld@intel.com> Cc: Andrzej Hajda <andrzej.hajda@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com> Reviewed-by: Jagmeet Randhawa <jagmeet.randhawa@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240522102143.128069-21-matthew.auld@intel.com
-
Matthew Auld authored
Hotunplugging the device seems to result in stuff like: kobject_add_internal failed for tile0 with -EEXIST, don't try to register things with the same name in the same directory. We only remove the sysfs as part of drmm, however that is tied to the lifetime of the driver instance and not the device underneath. Attempt to fix by using devm for all of the remaining sysfs stuff related to the device. Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/1667 Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/1432Signed-off-by: Matthew Auld <matthew.auld@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240522102143.128069-20-matthew.auld@intel.com
-
Matthew Auld authored
This is quite broken since we are nuking the pdev link to the private driver struct, but note here that driver_release is called when the drm_device is released (poor mans drmm), which can be long after the device has been removed. So here what we are actually doing is nuking the pdev link for what is potentially bound to a different drm_device. If that happens before our pci remove callback is triggered (for the new drm_device) we silently exit and skip some important cleanup steps, resulting in hilarity. There should be no reason to implement driver_release, when we already have nicer stuff like drmm, so just remove completely. The actual pdev link is already nuked when removing the device. Signed-off-by: Matthew Auld <matthew.auld@intel.com> Cc: Andrzej Hajda <andrzej.hajda@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240522102143.128069-19-matthew.auld@intel.com
-
Michal Wajdeczko authored
The GuC firmware is loaded and initialized by the PF driver. Make sure VF drivers only perform permitted operations. For submission initialization, use number of GuC context IDs from self config. Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: John Harrison <John.C.Harrison@Intel.com> Reviewed-by: Piotr Piórkowski <piotr.piorkowski@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240521092518.624-3-michal.wajdeczko@intel.com
-
Michal Wajdeczko authored
While PF and native drivers may initialize submission code to use all available GuC contexts IDs, the VF driver may only use limited number of IDs. Update init function to accept number of context IDs available for use. Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: Matthew Brost <matthew.brost@intel.com> Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com> Reviewed-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240521092518.624-2-michal.wajdeczko@intel.com
-
Michal Wajdeczko authored
We don't need <linux/delay.h> include since commit 5c09bd6c ("drm/xe/mmio: Move xe_mmio_wait32() to xe_mmio.c"). We don't need <linux/io-64-nonatomic-lo-hi.h> include since commit 54c65966 ("drm/xe: Make xe_mmio_read|write() functions non-inline"). And since commit 924e6a97 ("drm/xe/uapi: Remove MMIO ioctl") we don't need forward declarations of drm_device and drm_file. Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Reviewed-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240520181814.2392-4-michal.wajdeczko@intel.com
-
Michal Wajdeczko authored
These compilation units use udelay() or some GT oriented printk functions without explicitly including proper header files, and relying on #includes from the xe_mmio.h instead. Fix that. Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Reviewed-by: Francois Dugast <francois.dugast@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240520181814.2392-3-michal.wajdeczko@intel.com
-
Michal Wajdeczko authored
This compilation unit uses udelay() function without including it's header file. Fix that to break dependency on other code. Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: Jani Nikula <jani.nikula@intel.com> Acked-by: Jani Nikula <jani.nikula@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240520181814.2392-2-michal.wajdeczko@intel.com
-
Michal Wajdeczko authored
Prefer forward declaration over #include xe_guc_pc_types.h Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Reviewed-by: Francois Dugast <francois.dugast@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240521102828.668-5-michal.wajdeczko@intel.com
-
Michal Wajdeczko authored
Prefer forward declaration over #include xe_huc_types.h Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Reviewed-by: Francois Dugast <francois.dugast@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240521102828.668-4-michal.wajdeczko@intel.com
-
Michal Wajdeczko authored
Prefer forward declaration over #include xe_gsc_types.h Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Reviewed-by: Francois Dugast <francois.dugast@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240521102828.668-3-michal.wajdeczko@intel.com
-
Michal Wajdeczko authored
Prefer forward declaration over #include xe_uc_types.h Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Reviewed-by: Francois Dugast <francois.dugast@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240521102828.668-2-michal.wajdeczko@intel.com
-
- 21 May, 2024 11 commits
-
-
Nirmoy Das authored
Interruptible lock can return error and needed a return value check. This test should finish quick enough so use a uninterruptible lock instead. Cc: Matthew Auld <matthew.auld@intel.com> Reviewed-by: Matthew Auld <matthew.auld@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240521102715.22700-1-nirmoy.das@intel.comSigned-off-by: Nirmoy Das <nirmoy.das@intel.com>
-
Nirmoy Das authored
At xe_pt_zap_ptes_entry() and xe_pt_stage_unbind_entry, the level cannot be 0. Therefore, add an independent check for the level. Since the level cannot be zero at this point, there is no need to check for `is_compact`, so remove that instead. Cc: Matthew Auld <matthew.auld@intel.com> Reviewed-by: Matthew Auld <matthew.auld@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240521103623.11645-1-nirmoy.das@intel.comSigned-off-by: Nirmoy Das <nirmoy.das@intel.com>
-
Francois Dugast authored
The L3 bank mask is already generated and stored internally with the rest of the GT topology. In user space, the compute runtime now needs this information to be added to the device properties therefore the topology mask query is extended to provide a new mask which represents the L3 banks enabled on the GT. The changes in the compute runtime are ready and approved, see link below. v2: Rewrite commit message and add a link to the compute runtime PR (Francois Dugast) Cc: Matt Roper <matthew.d.roper@intel.com> Cc: Robert Krzemien <robert.krzemien@intel.com> Cc: Mateusz Jablonski <mateusz.jablonski@intel.com> Link: https://github.com/intel/compute-runtime/pull/722Signed-off-by: Francois Dugast <francois.dugast@intel.com> Acked-by: Mateusz Jablonski <mateusz.jablonski@intel.com> Reviewed-by: José Roberto de Souza <jose.souza@intel.com> Signed-off-by: Matt Roper <matthew.d.roper@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240416145037.7-2-francois.dugast@intel.com
-
Lucas De Marchi authored
Print the accumulated runtime for client when printing fdinfo. Each time a query is done it first does 2 things: 1) loop through all the exec queues for the current client and accumulate the runtime, per engine class. CTX_TIMESTAMP is used for that, being read from the context image. 2) Read a "GPU timestamp" that can be used for considering "how much GPU time has passed" and that has the same unit/refclock as the one recording the runtime. RING_TIMESTAMP is used for that via MMIO. Since for all current platforms RING_TIMESTAMP follows the same refclock, just read it once, using any first engine available. This is exported to userspace as 2 numbers in fdinfo: drm-cycles-<class>: <RUNTIME> drm-total-cycles-<class>: <TIMESTAMP> Userspace is expected to collect at least 2 samples, which allows to know the client engine busyness as per: RUNTIME1 - RUNTIME0 busyness = --------------------- T1 - T0 Since drm-cycles-<class> always starts at 0, it's also possible to know if and engine was ever used by a client. It's expected that userspace will read any 2 samples every few seconds. Given the update frequency of the counters involved and that CTX_TIMESTAMP is 32-bits, the counter for each exec_queue can wrap around (assuming 100% utilization) after ~200s. The wraparound is not perceived by userspace since it's just accumulated for all the exec_queues in a 64-bit counter) but the measurement will not be accurate if the samples are too far apart. This could be mitigated by adding a workqueue to accumulate the counters every so often, but it's additional complexity for something that is done already by userspace every few seconds in tools like gputop (from igt), htop, nvtop, etc, with none of them really defaulting to 1 sample per minute or more. Reviewed-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> Acked-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240517204310.88854-9-lucas.demarchi@intel.comSigned-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
-
Lucas De Marchi authored
Get the first available engine from a gt, which helps in the case any engine serves as a context, like when reading RING_TIMESTAMP. Reviewed-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240517204310.88854-8-lucas.demarchi@intel.comSigned-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
-
Lucas De Marchi authored
gt->info.engine_mask used to indicate the available engines, but that is not always true anymore: some engines are reserved to kernel and some may be exposed as a single engine (e.g. with ccs_mode). Runtime changes only happen when no clients exist, so it's safe to cache the list of engines in the gt and update that when it's needed. This will help implementing per client engine utilization so this (mostly constant) information doesn't need to be re-calculated on every query. Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com> Reviewed-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240517204310.88854-7-lucas.demarchi@intel.comSigned-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
-
Umesh Nerlige Ramappa authored
Add a helper to accumulate per-client runtime of all its exec queues. This is called every time a sched job is finished. v2: - Use guc_exec_queue_free_job() and execlist_job_free() to accumulate runtime when job is finished since xe_sched_job_completed() is not a notification that job finished. - Stop trying to update runtime from xe_exec_queue_fini() - that is redundant and may happen after xef is closed, leading to a use-after-free - Do not special case the first timestamp read: the default LRC sets CTX_TIMESTAMP to zero, so even the first sample should be a valid one. - Handle the parallel submission case by multiplying the runtime by width. v3: Update comments Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240517204310.88854-6-lucas.demarchi@intel.comSigned-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
-
Lucas De Marchi authored
Just like CTX_TIMESTAMP is used to calculate runtime, add a helper to get the timestamp for the engine so it can be used to calculate the "engine time" with the same unit as the runtime is recorded. Reviewed-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240517204310.88854-5-lucas.demarchi@intel.comSigned-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
-
Umesh Nerlige Ramappa authored
Add a helper to capture CTX_TIMESTAMP from the context image so it can be used to calculate the runtime. v2: Add kernel-doc to clarify expectation from caller Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> Reviewed-by: Francois Dugast <francois.dugast@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240517204310.88854-4-lucas.demarchi@intel.comSigned-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
-
Lucas De Marchi authored
XE_ENGINE_CLASS_OTHER was missing from the str conversion. Add it and remove the default handling so it's protected by -Wswitch. Currently the only user is xe_hw_engine_class_sysfs_init(), which already skips XE_ENGINE_CLASS_OTHER, so there's no change in behavior. Reviewed-by: Nirmoy Das <nirmoy.das@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240517204310.88854-3-lucas.demarchi@intel.comSigned-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
-
Lucas De Marchi authored
Move it out of the sysfs compilation unit so it can be re-used in other places. Reviewed-by: Nirmoy Das <nirmoy.das@intel.com> Reviewed-by: Oak Zeng <oak.zeng@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240517204310.88854-2-lucas.demarchi@intel.comSigned-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
-
- 17 May, 2024 1 commit
-
-
José Roberto de Souza authored
Other u64 registers are printed in a single line so RING_START needs to follow that too. As there is no upstream decoder tool parsing RING_START this will not break any decoder application. Cc: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com> Cc: Matt Roper <matthew.d.roper@intel.com> Signed-off-by: José Roberto de Souza <jose.souza@intel.com> Reviewed-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240510150108.80679-1-jose.souza@intel.com
-
- 16 May, 2024 2 commits
-
-
Michal Wajdeczko authored
For debug purposes we might want to view actual VF configuration (including GGTT range, LMEM size, number of GuC contexts IDs or doorbells) and the negotiated ABI versions (with GuC and PF). Reviewed-by: Piotr Piórkowski <piotr.piorkowski@intel.com> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240516110546.2216-7-michal.wajdeczko@intel.com
-
Michal Wajdeczko authored
The VF drivers may immediately communicate with the GuC to obtain the hardware config since the firmware shall already be running. With the GuC communication established, VFs can also obtain the values of the runtime registers (fuses) from the PF driver. Reviewed-by: Piotr Piórkowski <piotr.piorkowski@intel.com> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240516110546.2216-6-michal.wajdeczko@intel.com
-