| Message ID | 20240708082712.30257-1-biju.das.jz@bp.renesas.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: renesas: shmobile: shmo_drm_crtc: Fix PM imbalance if RPM_ACTIVE is true | expand |
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@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? Maxime
Hi Maxime, > -----Original Message----- > From: Maxime Ripard <mripard@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@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. Cheers, Biju
Hi Biju, On Mon, Jul 8, 2024 at 11:00 AM Biju Das <biju.das.jz@bp.renesas.com> wrote: > > From: Maxime Ripard <mripard@kernel.org> > > 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@bp.renesas.com> > > > --- 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. Thanks for your patch, but the code for pm_runtime_resume_and_get() seems to disagree? https://elixir.bootlin.com/linux/latest/source/include/linux/pm_runtime.h#L436 Gr{oetje,eeting}s, Geert
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@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@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
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; /* Reset and enable the LCDC. */
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@bp.renesas.com> --- drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)