linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] drm/stm: drv: fix suspend/resume
@ 2019-06-17  7:18 Yannick Fertré
  2019-06-18  9:57 ` Philippe CORNU
  2019-06-20 17:12 ` Emil Velikov
  0 siblings, 2 replies; 6+ messages in thread
From: Yannick Fertré @ 2019-06-17  7:18 UTC (permalink / raw)
  To: Yannick Fertre, Philippe Cornu, Benjamin Gaignard,
	Vincent Abriou, David Airlie, Daniel Vetter, Maxime Coquelin,
	Alexandre Torgue, dri-devel, linux-stm32, linux-arm-kernel,
	linux-kernel

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/3] drm/stm: drv: fix suspend/resume
  2019-06-17  7:18 [PATCH 1/3] drm/stm: drv: fix suspend/resume Yannick Fertré
@ 2019-06-18  9:57 ` Philippe CORNU
  2019-06-20 15:05   ` Benjamin Gaignard
  2019-06-20 17:12 ` Emil Velikov
  1 sibling, 1 reply; 6+ messages in thread
From: Philippe CORNU @ 2019-06-18  9:57 UTC (permalink / raw)
  To: Yannick FERTRE, Benjamin GAIGNARD, Vincent ABRIOU, David Airlie,
	Daniel Vetter, Maxime Coquelin, Alexandre TORGUE, dri-devel,
	linux-stm32, linux-arm-kernel, linux-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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/3] drm/stm: drv: fix suspend/resume
  2019-06-18  9:57 ` Philippe CORNU
@ 2019-06-20 15:05   ` Benjamin Gaignard
  0 siblings, 0 replies; 6+ messages in thread
From: Benjamin Gaignard @ 2019-06-20 15:05 UTC (permalink / raw)
  To: Philippe CORNU
  Cc: Benjamin GAIGNARD, David Airlie, linux-kernel, dri-devel,
	Yannick FERTRE, Maxime Coquelin, Daniel Vetter, Vincent ABRIOU,
	linux-stm32, linux-arm-kernel, Alexandre TORGUE

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/3] drm/stm: drv: fix suspend/resume
  2019-06-17  7:18 [PATCH 1/3] drm/stm: drv: fix suspend/resume Yannick Fertré
  2019-06-18  9:57 ` Philippe CORNU
@ 2019-06-20 17:12 ` Emil Velikov
  2019-06-21 14:01   ` Yannick FERTRE
  1 sibling, 1 reply; 6+ messages in thread
From: Emil Velikov @ 2019-06-20 17:12 UTC (permalink / raw)
  To: Yannick Fertré
  Cc: Daniel Vetter, David Airlie, Philippe Cornu, ML dri-devel,
	Linux-Kernel@Vger. Kernel. Org, Alexandre Torgue,
	Maxime Coquelin, Vincent Abriou, linux-stm32, LAKML,
	Benjamin Gaignard

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/3] drm/stm: drv: fix suspend/resume
  2019-06-20 17:12 ` Emil Velikov
@ 2019-06-21 14:01   ` Yannick FERTRE
  2019-06-21 14:53     ` Emil Velikov
  0 siblings, 1 reply; 6+ messages in thread
From: Yannick FERTRE @ 2019-06-21 14:01 UTC (permalink / raw)
  To: Emil Velikov
  Cc: Daniel Vetter, David Airlie, Philippe CORNU, ML dri-devel,
	Linux-Kernel@Vger. Kernel. Org, Alexandre TORGUE,
	Maxime Coquelin, Vincent ABRIOU, linux-stm32, LAKML,
	Benjamin GAIGNARD

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/3] drm/stm: drv: fix suspend/resume
  2019-06-21 14:01   ` Yannick FERTRE
@ 2019-06-21 14:53     ` Emil Velikov
  0 siblings, 0 replies; 6+ messages in thread
From: Emil Velikov @ 2019-06-21 14:53 UTC (permalink / raw)
  To: Yannick FERTRE
  Cc: Daniel Vetter, David Airlie, Philippe CORNU, ML dri-devel,
	Linux-Kernel@Vger. Kernel. Org, Alexandre TORGUE,
	Maxime Coquelin, Vincent ABRIOU, linux-stm32, LAKML,
	Benjamin GAIGNARD

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-06-21 14:56 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-17  7:18 [PATCH 1/3] drm/stm: drv: fix suspend/resume Yannick Fertré
2019-06-18  9:57 ` Philippe CORNU
2019-06-20 15:05   ` Benjamin Gaignard
2019-06-20 17:12 ` Emil Velikov
2019-06-21 14:01   ` Yannick FERTRE
2019-06-21 14:53     ` Emil Velikov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).