> -----Original Message-----
> From: amd-gfx <[email protected]> On Behalf Of Christian
> König
> Sent: Monday, September 14, 2026 16:57
> To: Soltani, Shahyan <[email protected]>; [email protected]
> Cc: Deucher, Alexander <[email protected]>; Pelloux-Prayer, Pierre-
> Eric <[email protected]>
> Subject: Re: [PATCH 1/8] drm/amdgpu: use pm_runtime_resume_and_get
> amdgpu_gem_create_ioctl
>
> 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).
A guard would be a good fit here,
but guard(pm_runtime_active) does not propagate failures from
pm_runtime_get_sync() and pairs it with pm_runtime_put(), rather than
pm_runtime_put_autosuspend().
Since this ioctl must not proceed if the device fails to resume,
I think we need either a conditional guard that propagates the resume error and
uses autosuspend on exit or retain the explicit pm_runtime_resume_and_get() /
pm_runtime_put_autosuspend() handling.
Best Regards,
Kevin
>
> 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,
