Re: [PATCH] drm/amdgpu/ctx: only reset stable pstate if the user changed it

[Alex Deucher <alexdeucher@gmail.com>]

On Tue, May 10, 2022 at 6:24 AM Lazar, Lijo <lijo.lazar@amd.com> wrote:
>
>
>
> On 5/10/2022 1:33 AM, Alex Deucher wrote:
> > Track whether the user requested a stable pstate via the ctx
> > IOCTL and only reset the pstate on context destroy if the user
> > actually changed it.  This avoids changing the pstate on contex
> > destroy if the user never changed it in the first place via the
> > IOCTL.
> >
> > Fixes: 8cda7a4f96e435 ("drm/amdgpu/UAPI: add new CTX OP to get/set stable pstates")
> > Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 5 ++++-
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h | 1 +
> >   2 files changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> > index 5981c7d9bd48..e4b0c6ec227c 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> > @@ -330,6 +330,8 @@ static int amdgpu_ctx_set_stable_pstate(struct amdgpu_ctx *ctx,
> >               adev->pm.stable_pstate_ctx = NULL;
> >       else
> >               adev->pm.stable_pstate_ctx = ctx;
> > +
> > +     ctx->stable_pstate_requested = true;
> >   done:
> >       mutex_unlock(&adev->pm.stable_pstate_ctx_lock);
> >
> > @@ -353,7 +355,8 @@ static void amdgpu_ctx_fini(struct kref *ref)
> >       }
> >
> >       if (drm_dev_enter(&adev->ddev, &idx)) {
> > -             amdgpu_ctx_set_stable_pstate(ctx, AMDGPU_CTX_STABLE_PSTATE_NONE);
> > +             if (ctx->stable_pstate_requested)
> > +                     amdgpu_ctx_set_stable_pstate(ctx, AMDGPU_CTX_STABLE_PSTATE_NONE);
>
> Why not have the check inside amdgpu_ctx_set_stable_pstate - if current
> stable pstate is not the same as the requested one, then only switch. If
> the user has not made a request, it will be NONE and the call doesn't
> have any effect.

Good idea.  New patch sent out earlier today.

Alex

>
> Thanks,
> Lijo
>
> >               drm_dev_exit(idx);
> >       }
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> > index d0cbfcea90f7..f03e842209b8 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> > @@ -53,6 +53,7 @@ struct amdgpu_ctx {
> >       unsigned long                   ras_counter_ce;
> >       unsigned long                   ras_counter_ue;
> >       uint32_t                        stable_pstate;
> > +     bool                            stable_pstate_requested;
> >   };
> >
> >   struct amdgpu_ctx_mgr {
> >