| Message ID | 1560755897-5002-1-git-send-email-yannick.fertre@st.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] drm/stm: drv: fix suspend/resume | expand |
Hi Yannick, Thank you for your patch. Acked-by: Philippe Cornu <philippe.cornu@st.com> Philippe :-) On 6/17/19 9:18 AM, Yannick Fertré wrote: > Without this fix, the system can not go in "suspend" mode > due to an error in drv_suspend function. > > Fixes: 35ab6cf ("drm/stm: support runtime power management") > > Signed-off-by: Yannick Fertré <yannick.fertre@st.com> > --- > drivers/gpu/drm/stm/drv.c | 15 ++++++++------- > 1 file changed, 8 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/stm/drv.c b/drivers/gpu/drm/stm/drv.c > index 5659572..9dee4e4 100644 > --- a/drivers/gpu/drm/stm/drv.c > +++ b/drivers/gpu/drm/stm/drv.c > @@ -136,8 +136,7 @@ static __maybe_unused int drv_suspend(struct device *dev) > struct ltdc_device *ldev = ddev->dev_private; > struct drm_atomic_state *state; > > - if (WARN_ON(!ldev->suspend_state)) > - return -ENOENT; > + WARN_ON(ldev->suspend_state); > > state = drm_atomic_helper_suspend(ddev); > if (IS_ERR(state)) > @@ -155,15 +154,17 @@ static __maybe_unused int drv_resume(struct device *dev) > struct ltdc_device *ldev = ddev->dev_private; > int ret; > > + if (WARN_ON(!ldev->suspend_state)) > + return -ENOENT; > + > pm_runtime_force_resume(dev); > ret = drm_atomic_helper_resume(ddev, ldev->suspend_state); > - if (ret) { > + if (ret) > pm_runtime_force_suspend(dev); > - ldev->suspend_state = NULL; > - return ret; > - } > > - return 0; > + ldev->suspend_state = NULL; > + > + return ret; > } > > static __maybe_unused int drv_runtime_suspend(struct device *dev) >
Le mar. 18 juin 2019 à 11:57, Philippe CORNU <philippe.cornu@st.com> a écrit : > > Hi Yannick, > > Thank you for your patch. > > Acked-by: Philippe Cornu <philippe.cornu@st.com> I have corrected Fixes sha1 (should be 12 digits) Applied on drm-misc-next. Benjamin > > Philippe :-) > > On 6/17/19 9:18 AM, Yannick Fertré wrote: > > Without this fix, the system can not go in "suspend" mode > > due to an error in drv_suspend function. > > > > Fixes: 35ab6cf ("drm/stm: support runtime power management") > > > > Signed-off-by: Yannick Fertré <yannick.fertre@st.com> > > --- > > drivers/gpu/drm/stm/drv.c | 15 ++++++++------- > > 1 file changed, 8 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/gpu/drm/stm/drv.c b/drivers/gpu/drm/stm/drv.c > > index 5659572..9dee4e4 100644 > > --- a/drivers/gpu/drm/stm/drv.c > > +++ b/drivers/gpu/drm/stm/drv.c > > @@ -136,8 +136,7 @@ static __maybe_unused int drv_suspend(struct device *dev) > > struct ltdc_device *ldev = ddev->dev_private; > > struct drm_atomic_state *state; > > > > - if (WARN_ON(!ldev->suspend_state)) > > - return -ENOENT; > > + WARN_ON(ldev->suspend_state); > > > > state = drm_atomic_helper_suspend(ddev); > > if (IS_ERR(state)) > > @@ -155,15 +154,17 @@ static __maybe_unused int drv_resume(struct device *dev) > > struct ltdc_device *ldev = ddev->dev_private; > > int ret; > > > > + if (WARN_ON(!ldev->suspend_state)) > > + return -ENOENT; > > + > > pm_runtime_force_resume(dev); > > ret = drm_atomic_helper_resume(ddev, ldev->suspend_state); > > - if (ret) { > > + if (ret) > > pm_runtime_force_suspend(dev); > > - ldev->suspend_state = NULL; > > - return ret; > > - } > > > > - return 0; > > + ldev->suspend_state = NULL; > > + > > + return ret; > > } > > > > static __maybe_unused int drv_runtime_suspend(struct device *dev) > > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi Yannick, On Mon, 17 Jun 2019 at 08:18, Yannick Fertré <yannick.fertre@st.com> wrote: > @@ -155,15 +154,17 @@ static __maybe_unused int drv_resume(struct device *dev) > struct ltdc_device *ldev = ddev->dev_private; > int ret; > > + if (WARN_ON(!ldev->suspend_state)) > + return -ENOENT; > + > pm_runtime_force_resume(dev); > ret = drm_atomic_helper_resume(ddev, ldev->suspend_state); > - if (ret) { > + if (ret) Hmm the msm driver uses !ret here. Suspecting that you want the same, although I haven't checked in detail. HTH -Emil
Hi Emil, The msm driver tests the return value & set state to NULL if no error is detected. the ltdc driver tests the return value & force to suspend if an error is detected. It's not exactly the same. Best regards
On Fri, 21 Jun 2019 at 15:01, Yannick FERTRE <yannick.fertre@st.com> wrote: > > Hi Emil, > > The msm driver tests the return value & set state to NULL if no error is > detected. > > the ltdc driver tests the return value & force to suspend if an error is > detected. > > It's not exactly the same. > D'oh I've misread that patch as pm_runtime_force_suspend() being called when the atomic helper succeeds. Thanks for the correction :-) Emil
diff --git a/drivers/gpu/drm/stm/drv.c b/drivers/gpu/drm/stm/drv.c index 5659572..9dee4e4 100644 --- a/drivers/gpu/drm/stm/drv.c +++ b/drivers/gpu/drm/stm/drv.c @@ -136,8 +136,7 @@ static __maybe_unused int drv_suspend(struct device *dev) struct ltdc_device *ldev = ddev->dev_private; struct drm_atomic_state *state; - if (WARN_ON(!ldev->suspend_state)) - return -ENOENT; + WARN_ON(ldev->suspend_state); state = drm_atomic_helper_suspend(ddev); if (IS_ERR(state)) @@ -155,15 +154,17 @@ static __maybe_unused int drv_resume(struct device *dev) struct ltdc_device *ldev = ddev->dev_private; int ret; + if (WARN_ON(!ldev->suspend_state)) + return -ENOENT; + pm_runtime_force_resume(dev); ret = drm_atomic_helper_resume(ddev, ldev->suspend_state); - if (ret) { + if (ret) pm_runtime_force_suspend(dev); - ldev->suspend_state = NULL; - return ret; - } - return 0; + ldev->suspend_state = NULL; + + return ret; } static __maybe_unused int drv_runtime_suspend(struct device *dev)
Without this fix, the system can not go in "suspend" mode due to an error in drv_suspend function. Fixes: 35ab6cf ("drm/stm: support runtime power management") Signed-off-by: Yannick Fertré <yannick.fertre@st.com> --- drivers/gpu/drm/stm/drv.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)