- 23 May, 2024 12 commits
-
-
John Harrison authored
GuC loading can take longer than it is supposed to for various reasons. So add in the code to cope with that and to report it when it happens. There are also many different reasons why GuC loading can fail, so add in the code for checking for those and for reporting issues in a meaningful manner rather than just hitting a timeout and saying 'fail: status = %x'. Also, remove the 'FIXME' comment about an i915 bug that has never been applicable to Xe! v2: Actually report the requested and granted frequencies rather than showing granted twice (review feedback from Badal). v3: Locally code all the timeout and end condition handling because a helper function is not allowed (review feedback from Lucas/Rodrigo). v4: Add more documentation comments and rename a define to add units (review feedback from Lucas). v5: Fix copy/paste error in xe_mmio_wait32_not (review feedback from Lucas) and rebase (no more return value from guc_wait_ucode). Signed-off-by:
John Harrison <John.C.Harrison@Intel.com> Reviewed-by:
Lucas De Marchi <lucas.demarchi@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240518043700.3264362-3-John.C.Harrison@Intel.com
-
John Harrison authored
Other driver code beyond the sysfs interface wants to know about throttling. So make the query function globally accessible. v2: Revert include order change (review feedback from Lucas) v3: Remove '_sysfs' from throttle file names and keep limit query in the same file rather than moving elsewhere (review feedback from Rodrigo). v4: Correct #include while renaming header file (review feedback from Lucas). Signed-off-by:
-
José Roberto de Souza authored
This error capture prints into dmesg HW state when a gpu hang happens. It was useful when we did not had devcoredump, now it is a incompleted version of devcoredump that has potential to flood dmesg. Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: John Harrison <John.C.Harrison@Intel.com> Signed-off-by:
José Roberto de Souza <jose.souza@intel.com> Reviewed-by:
Rodrigo Vivi <rodrigo.vivi@intel.com>
-
José Roberto de Souza authored
Process name help us track what application caused the gpug hang, this is crucial when running several applications at the same time. v2: - handle Xe KMD exec_queues without VM v3: - use get_pid_task() (suggested by Nirmoy) Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: Nirmoy Das <nirmoy.das@intel.com> Signed-off-by:
-
Dr. David Alan Gilbert authored
'xe_gt_desc' is unused since commit 1e6c20be ("drm/xe: Drop extra_gts[] declarations and XE_GT_TYPE_REMOTE"). Remove it. Signed-off-by:
Dr. David Alan Gilbert <linux@treblig.org> Link: https://patchwork.freedesktop.org/patch/msgid/20240522175840.382107-1-linux@treblig.orgReviewed-by:
-
Rodrigo Vivi authored
Now that we eliminated all the mem_access get/put with its locking issues from the inner calls of migration, we can allow D3Cold. Enable it when VRAM utilization is lower then 300Mb. On higher utilization we only allow D3hot so we don't increase so much the latency on runtime resume due to the memory restoration. Reviewed-by:
Matthew Brost <matthew.brost@intel.com> Reviewed-by:
Anshuman Gupta <anshuman.gupta@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240522170105.327472-7-rodrigo.vivi@intel.comSigned-off-by:
-
Rodrigo Vivi authored
GuC reset status is not reliable for this purpose and it is once in a while ending up in a situation of D3Cold, where power_reset is false and without the proper memory restoration the GuC reload and Display will fail to come back from D3Cold. So, let's do a full restoration of everything if we have a risk of losing power, without further optimizations. v2: also remove the gut_in_reset function (Anshuman) Cc: Anshuman Gupta <anshuman.gupta@intel.com> Reviewed-by:
Badal Nilawar <badal.nilawar@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240522170105.327472-6-rodrigo.vivi@intel.comSigned-off-by:
-
Rodrigo Vivi authored
Prepare power-well and DC handling for a full power lost during D3Cold, then sanitize it upon D3->D0. Otherwise we get a bunch of state mismatch. Ideally we could leave DC9 enabled and wouldn't need to move DC9->DC0 on every runtime resume, however, the disable_DC is part of the power-well checks and intrinsic to the dc_off power well. In the future that can be detangled so we can have even bigger power savings. But for now, let's focus on getting a D3Cold, which saves much more power by itself. v2: create new functions to avoid full-suspend-resume path, which would result in a deadlock between xe_gem_fault and the modeset-ioctl. v3: Only avoid the full modeset to avoid the race, for a more robust suspend-resume. Cc: Anshuman Gupta <anshuman.gupta@intel.com> Cc: Uma Shankar <uma.shankar@intel.com> Tested-by:
Francois Dugast <francois.dugast@intel.com> Reviewed-by:
-
Rodrigo Vivi authored
In the regular use case scenario, user space will create a VM, and keep it alive for the entire duration of its workload. For the regular desktop cases, it means that the VM is alive even on idle scenarios where display goes off. This is unacceptable since this would entirely block runtime PM indefinitely, blocking deeper Package-C state. This would be a waste drainage of power. Limit the VM protection solely for long-running workloads that are not protected by the scheduler references. By design, run_job for long-running workloads returns NULL and the scheduler drops all the references of it, hence protecting the VM for this case is necessary. v2: Update commit message to a more imperative language and to reflect why the VM protection is really needed. Also add a comment in the code to let the reason visbible. v3: Remove vma_access case and the mentions to mmap. Mmap cases are already protected by the gem page fault. Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com> Cc: Lucas De Marchi <lucas.demarchi@intel.com> Cc: Matthew Brost <matthew.brost@intel.com> Tested-by:Thomas Hellström <thomas.hellstrom@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240522170105.327472-4-rodrigo.vivi@intel.comSigned-off-by:
-
Rodrigo Vivi authored
Limit the protection only during moments of actual job execution, and introduce protection for guc submit fini, which is currently unprotected due to the absence of exec_queue life protection. In the regular use case scenario, user space will create an exec queue, and keep it alive to reuse that until it is done with that kind of workload. For the regular desktop cases, it means that the exec_queue is alive even on idle scenarios where display goes off. This is unacceptable since this would entirely block runtime PM indefinitely, blocking deeper Package-C state. This would be a waste drainage of power. Cc: Matthew Brost <matthew.brost@intel.com> Tested-by:
-
Rodrigo Vivi authored
Let's be clear on what it is actually doing and align with xe_pm_runtime_get_if_active doc style. Tested-by:
-
Rodrigo Vivi authored
Current callers of this function are already taking the result to a boolean and using in an if. It might be a problem because current function might return negative error codes on failure, without increasing the reference counter. In this scenario we could end up with extra 'put' call ending in unbalanced scenarios. Let's fix it, while aligning with the current xe_pm_get_if_in_use style. Tested-by:
-
- 22 May, 2024 28 commits
-
-
Niranjana Vishwanathapura authored
Release the submission_state lock if alloc_guc_id() fails. v2: Add Fixes tag and CC stable kernel Fixes: dd08ebf6 ("drm/xe: Introduce a new DRM driver for Intel GPUs") Cc: <stable@vger.kernel.org> # v6.8+ Signed-off-by:
Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com> Reviewed-by:
Nirmoy Das <nirmoy.das@intel.com> Reviewed-by:
-
Michal Wajdeczko authored
When running in execlist mode (using force_execlist=1 modparam) we incorrectly select the error path in xe_uc_init(), leading to an unwanted error message like this: [ ] xe 0000:00:00.0: [drm] *ERROR* GT0: Failed to initialize uC (0000000000000000) Fix that by doing early return like we do in other similar cases. Signed-off-by:
Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Reviewed-by:
Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240521114857.712-1-michal.wajdeczko@intel.com
-
Matthew Auld authored
i915 display calls this when releasing the drm_device, match this also in xe by using drmm. intel_display_device_remove() is freeing purely software state for the drm_device. v2: fix build error 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-36-matthew.auld@intel.com
-
Matthew Auld authored
Unclear why we call this twice. Signed-off-by:
-
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 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 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 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 authored
Have a cleaner separation between hw vs sw. v2: Fix missing return Signed-off-by:
-
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 authored
Disable GuC submission when removing the device. Signed-off-by:
-
Matthew Auld authored
Should be called when driver is removed, not when this particular driver instance is destroyed. Signed-off-by:
-
Matthew Auld authored
Makes sense to trigger this when the device is removed. Signed-off-by:
-
Matthew Auld authored
Make it clear that is about cleaning up the HW/FW side, and not software state. Signed-off-by:
-
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 authored
Make it clear that is about cleaning up the HW/FW side, and not software state. Signed-off-by:
-
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 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: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 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:
-
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:
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:
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 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 authored
This compilation unit uses udelay() function without including it's header file. Fix that to break dependency on other code. Signed-off-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 authored
Prefer forward declaration over #include xe_huc_types.h Signed-off-by:
-
Michal Wajdeczko authored
Prefer forward declaration over #include xe_gsc_types.h Signed-off-by:
-
