[PATCH] drm: renesas: shmobile: shmo_drm_crtc: Fix PM imbalance if RPM_ACTIVE is true

[Biju Das]

Hi Maxime,

> -----Original Message-----
> From: Biju Das
> Sent: Monday, July 8, 2024 10:01 AM
> Subject: RE: [PATCH] drm: renesas: shmobile: shmo_drm_crtc: Fix PM imbalance if RPM_ACTIVE is true
> 
> Hi Maxime,
> 
> > -----Original Message-----
> > From: Maxime Ripard <mripard at kernel.org>
> > Sent: Monday, July 8, 2024 9:54 AM
> > Subject: Re: [PATCH] drm: renesas: shmobile: shmo_drm_crtc: Fix PM
> > imbalance if RPM_ACTIVE is true
> >
> > Hi,
> >
> > On Mon, Jul 08, 2024 at 09:27:09AM GMT, Biju Das wrote:
> > > The pm_runtime_resume_and_get() returns 1 if RPM is active, in this
> > > case it won't call a put. This will result in PM imbalance as it
> > > treat this as an error and propagate this to caller and the caller
> > > never calls corresponding put(). Fix this issue by checking error
> > > condition only.
> > >
> > > Signed-off-by: Biju Das <biju.das.jz at bp.renesas.com>
> > > ---
> > >  drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c
> > > b/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c
> > > index 2e2f37b9d0a4..42a5d6876bec 100644
> > > --- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c
> > > +++ b/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c
> > > @@ -208,7 +208,7 @@ static void shmob_drm_crtc_atomic_enable(struct drm_crtc *crtc,
> > >  	int ret;
> > >
> > >  	ret = pm_runtime_resume_and_get(dev);
> > > -	if (ret)
> > > +	if (ret < 0)
> > >  		return;
> >
> > The documentation of pm_runtime_resume_and_get says that:
> >
> >   Resume @dev synchronously and if that is successful, increment its
> >   runtime PM usage counter. Return 0 if the runtime PM usage counter of
> >   @dev has been incremented or a negative error code otherwise.
> >
> > So it looks like it can't return 1, ever. Are you sure you're not
> > confusing pm_runtime_resume_and_get with pm_runtime_get?
> 
> It should be ret < 0 as ret = 1 corresponds to RPM_ACTIVE and the API does not call put() when ret =
> 1; see [1] and [2]
> 
> [1] https://elixir.bootlin.com/linux/v6.10-rc6/source/drivers/base/power/runtime.c#L778
> 
> [2] https://elixir.bootlin.com/linux/v6.10-rc6/source/include/linux/pm_runtime.h#L431
> 
> Am I miss anything? Please let me know.

the code for pm_runtime_resume_and_get() seems correct. Sorry for the noisy patch.
https://elixir.bootlin.com/linux/latest/source/include/linux/pm_runtime.h#L436

Cheers,
Biju