On 9/11/26 17:42, Shahyan Soltani wrote:
> From: Pierre-Eric Pelloux-Prayer <[email protected]>
>
> This ioctl might trigger GPU work through ttm so make sure the GPU
> is awake.
>
> Signed-off-by: Pierre-Eric Pelloux-Prayer <[email protected]>
> [[email protected]: Applied the same logic to
> !amdgpu_gem_are_domains_valid(args->in.domains)]
> Signed-off-by: Shahyan Soltani <[email protected]>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 43 ++++++++++++++++++-------
> 1 file changed, 31 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index 0d579517c03c..7b92e7b9fbb8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -33,6 +33,7 @@
> #include <linux/dma-buf.h>
> #include <linux/dma-fence-unwrap.h>
> #include <linux/uaccess.h>
> +#include <linux/pm_runtime.h>
>
> #include <drm/amdgpu_drm.h>
> #include <drm/drm_drv.h>
> @@ -442,19 +443,31 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev,
> void *data,
> uint32_t handle, initial_domain;
> int r;
>
> + r = pm_runtime_resume_and_get(dev->dev);
> + if (r < 0)
> + return r;
You might be able to replace this with a much simpler guard based approach,
e.g. something like this:
guard(pm_runtime_active)(dev->dev);
A guard automatically calls pm_runtime_put()/pm_runtime_put_autosuspend() when
it run out of scope (e.g. on the closing } or when return is used).
That saves you all the error handling which makes this patch so complicated.
See the DEFINE_GUARD() lines in include/linux/pm_runtime.h as well.
Regards,
Christian.
> +
> /* reject invalid gem flags */
> - if (flags & ~AMDGPU_GEM_CREATE_SETTABLE_MASK)
> - return -EINVAL;
> + if (flags & ~AMDGPU_GEM_CREATE_SETTABLE_MASK) {
> + r = -EINVAL;
> + goto pm_out;
> + }
>
> /* reject invalid gem domains */
> - if (args->in.domains & ~AMDGPU_GEM_DOMAIN_MASK)
> - return -EINVAL;
> - if (!amdgpu_gem_are_domains_valid(args->in.domains))
> - return -EINVAL;
> + if (args->in.domains & ~AMDGPU_GEM_DOMAIN_MASK) {
> + r = -EINVAL;
> + goto pm_out;
> + }
> +
> + if (!amdgpu_gem_are_domains_valid(args->in.domains)) {
> + r = -EINVAL;
> + goto pm_out;
> + }
>
> if (!amdgpu_is_tmz(adev) && (flags & AMDGPU_GEM_CREATE_ENCRYPTED)) {
> DRM_NOTE_ONCE("Cannot allocate secure buffer since TMZ is
> disabled\n");
> - return -EINVAL;
> + r = -EINVAL;
> + goto pm_out;
> }
>
> /* always clear VRAM */
> @@ -468,7 +481,8 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, void
> *data,
> * passed to bo list
> */
> DRM_ERROR("GDS bo cannot be per-vm-bo\n");
> - return -EINVAL;
> + r = -EINVAL;
> + goto pm_out;
> }
> flags |= AMDGPU_GEM_CREATE_NO_CPU_ACCESS;
> }
> @@ -476,7 +490,7 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, void
> *data,
> if (flags & AMDGPU_GEM_CREATE_VM_ALWAYS_VALID) {
> r = amdgpu_bo_reserve(vm->root.bo, false);
> if (r)
> - return r;
> + goto pm_out;
>
> resv = vm->root.bo->tbo.base.resv;
> }
> @@ -509,17 +523,22 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev,
> void *data,
> amdgpu_bo_unreserve(vm->root.bo);
> }
> if (r)
> - return r;
> + goto pm_out;
>
> r = drm_gem_handle_create(filp, gobj, &handle);
> /* drop reference from allocate - handle holds it now */
> drm_gem_object_put(gobj);
> if (r)
> - return r;
> + goto pm_out;
>
> + r = 0;
> memset(args, 0, sizeof(*args));
> args->out.handle = handle;
> - return 0;
> +
> +pm_out:
> + pm_runtime_put_autosuspend(dev->dev);
> +
> + return r;
> }
>
> int amdgpu_gem_userptr_ioctl(struct drm_device *dev, void *data,
