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) -- 2.7.4 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
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) > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
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 -- Yannick Fertré | TINA: 166 7152 | Tel: +33 244027152 | Mobile: +33 620600270 Microcontrollers and Digital ICs Group | Microcontrolleurs Division On 6/20/19 7:12 PM, Emil Velikov wrote: > 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 -- Yannick Fertré | TINA: 166 7152 | Tel: +33 244027152 | Mobile: +33 620600270 Microcontrollers and Digital ICs Group | Microcontrolleurs Division _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel