diff mbox

drm: hdlcd: Update PM code to save/restore console.

Message ID 20170616135333.27796-1-Liviu.Dudau@arm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Liviu Dudau June 16, 2017, 1:53 p.m. UTC
Update the PM code to suspend/resume the fbdev_cma console.

Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
---
 drivers/gpu/drm/arm/hdlcd_drv.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

Comments

Noralf Trønnes June 16, 2017, 4:58 p.m. UTC | #1
Den 16.06.2017 15.53, skrev Liviu Dudau:
> Update the PM code to suspend/resume the fbdev_cma console.
>
> Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
> ---
>   drivers/gpu/drm/arm/hdlcd_drv.c | 11 ++++++++++-
>   1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
> index d3da87fbd85a..89cd408cde6f 100644
> --- a/drivers/gpu/drm/arm/hdlcd_drv.c
> +++ b/drivers/gpu/drm/arm/hdlcd_drv.c
> @@ -13,6 +13,7 @@
>   #include <linux/spinlock.h>
>   #include <linux/clk.h>
>   #include <linux/component.h>
> +#include <linux/console.h>
>   #include <linux/list.h>
>   #include <linux/of_graph.h>
>   #include <linux/of_reserved_mem.h>
> @@ -435,9 +436,15 @@ static int __maybe_unused hdlcd_pm_suspend(struct device *dev)
>   		return 0;
>   
>   	drm_kms_helper_poll_disable(drm);
> +	console_lock();
> +	drm_fbdev_cma_set_suspend(hdlcd->fbdev, 1);
> +	console_unlock();

You can use drm_fbdev_cma_set_suspend_unlocked() instead, it takes the
lock for you and can speed up resume if the lock is contented.

Noralf.


>   
>   	hdlcd->state = drm_atomic_helper_suspend(drm);
>   	if (IS_ERR(hdlcd->state)) {
> +		console_lock();
> +		drm_fbdev_cma_set_suspend(hdlcd->fbdev, 0);
> +		console_unlock();
>   		drm_kms_helper_poll_enable(drm);
>   		return PTR_ERR(hdlcd->state);
>   	}
> @@ -454,8 +461,10 @@ static int __maybe_unused hdlcd_pm_resume(struct device *dev)
>   		return 0;
>   
>   	drm_atomic_helper_resume(drm, hdlcd->state);
> +	console_lock();
> +	drm_fbdev_cma_set_suspend(hdlcd->fbdev, 0);
> +	console_unlock();
>   	drm_kms_helper_poll_enable(drm);
> -	pm_runtime_set_active(dev);
>   
>   	return 0;
>   }
Liviu Dudau June 19, 2017, 1:17 p.m. UTC | #2
On Fri, Jun 16, 2017 at 06:58:36PM +0200, Noralf Trønnes wrote:
> 
> Den 16.06.2017 15.53, skrev Liviu Dudau:
> > Update the PM code to suspend/resume the fbdev_cma console.
> > 
> > Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
> > ---
> >   drivers/gpu/drm/arm/hdlcd_drv.c | 11 ++++++++++-
> >   1 file changed, 10 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
> > index d3da87fbd85a..89cd408cde6f 100644
> > --- a/drivers/gpu/drm/arm/hdlcd_drv.c
> > +++ b/drivers/gpu/drm/arm/hdlcd_drv.c
> > @@ -13,6 +13,7 @@
> >   #include <linux/spinlock.h>
> >   #include <linux/clk.h>
> >   #include <linux/component.h>
> > +#include <linux/console.h>
> >   #include <linux/list.h>
> >   #include <linux/of_graph.h>
> >   #include <linux/of_reserved_mem.h>
> > @@ -435,9 +436,15 @@ static int __maybe_unused hdlcd_pm_suspend(struct device *dev)
> >   		return 0;
> >   	drm_kms_helper_poll_disable(drm);
> > +	console_lock();
> > +	drm_fbdev_cma_set_suspend(hdlcd->fbdev, 1);
> > +	console_unlock();
> 
> You can use drm_fbdev_cma_set_suspend_unlocked() instead, it takes the
> lock for you and can speed up resume if the lock is contented.

Hi Noralf,

Thanks for pointing out the helpful function. As you look to be the author of it,
any reason why the signature of the function doesn't match the drm_fb_helper_ one
being called through? (I'm talking about int vs bool for the state/suspend arguments).

Best regards,
Liviu

> 
> Noralf.
> 
> 
> >   	hdlcd->state = drm_atomic_helper_suspend(drm);
> >   	if (IS_ERR(hdlcd->state)) {
> > +		console_lock();
> > +		drm_fbdev_cma_set_suspend(hdlcd->fbdev, 0);
> > +		console_unlock();
> >   		drm_kms_helper_poll_enable(drm);
> >   		return PTR_ERR(hdlcd->state);
> >   	}
> > @@ -454,8 +461,10 @@ static int __maybe_unused hdlcd_pm_resume(struct device *dev)
> >   		return 0;
> >   	drm_atomic_helper_resume(drm, hdlcd->state);
> > +	console_lock();
> > +	drm_fbdev_cma_set_suspend(hdlcd->fbdev, 0);
> > +	console_unlock();
> >   	drm_kms_helper_poll_enable(drm);
> > -	pm_runtime_set_active(dev);
> >   	return 0;
> >   }
>
Noralf Trønnes June 19, 2017, 3:45 p.m. UTC | #3
Den 19.06.2017 15.17, skrev Liviu Dudau:
> On Fri, Jun 16, 2017 at 06:58:36PM +0200, Noralf Trønnes wrote:
>> Den 16.06.2017 15.53, skrev Liviu Dudau:
>>> Update the PM code to suspend/resume the fbdev_cma console.
>>>
>>> Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
>>> ---
>>>    drivers/gpu/drm/arm/hdlcd_drv.c | 11 ++++++++++-
>>>    1 file changed, 10 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
>>> index d3da87fbd85a..89cd408cde6f 100644
>>> --- a/drivers/gpu/drm/arm/hdlcd_drv.c
>>> +++ b/drivers/gpu/drm/arm/hdlcd_drv.c
>>> @@ -13,6 +13,7 @@
>>>    #include <linux/spinlock.h>
>>>    #include <linux/clk.h>
>>>    #include <linux/component.h>
>>> +#include <linux/console.h>
>>>    #include <linux/list.h>
>>>    #include <linux/of_graph.h>
>>>    #include <linux/of_reserved_mem.h>
>>> @@ -435,9 +436,15 @@ static int __maybe_unused hdlcd_pm_suspend(struct device *dev)
>>>    		return 0;
>>>    	drm_kms_helper_poll_disable(drm);
>>> +	console_lock();
>>> +	drm_fbdev_cma_set_suspend(hdlcd->fbdev, 1);
>>> +	console_unlock();
>> You can use drm_fbdev_cma_set_suspend_unlocked() instead, it takes the
>> lock for you and can speed up resume if the lock is contented.
> Hi Noralf,
>
> Thanks for pointing out the helpful function. As you look to be the author of it,
> any reason why the signature of the function doesn't match the drm_fb_helper_ one
> being called through? (I'm talking about int vs bool for the state/suspend arguments).

I don't remember, but probably to match drm_fbdev_cma_set_suspend()
which uses int. drm_fb_helper_set_suspend*() uses bool, but calls
into fb_set_suspend() which uses int, but as a boolean.

Noralf.
> Best regards,
> Liviu
>
>> Noralf.
>>
>>
>>>    	hdlcd->state = drm_atomic_helper_suspend(drm);
>>>    	if (IS_ERR(hdlcd->state)) {
>>> +		console_lock();
>>> +		drm_fbdev_cma_set_suspend(hdlcd->fbdev, 0);
>>> +		console_unlock();
>>>    		drm_kms_helper_poll_enable(drm);
>>>    		return PTR_ERR(hdlcd->state);
>>>    	}
>>> @@ -454,8 +461,10 @@ static int __maybe_unused hdlcd_pm_resume(struct device *dev)
>>>    		return 0;
>>>    	drm_atomic_helper_resume(drm, hdlcd->state);
>>> +	console_lock();
>>> +	drm_fbdev_cma_set_suspend(hdlcd->fbdev, 0);
>>> +	console_unlock();
>>>    	drm_kms_helper_poll_enable(drm);
>>> -	pm_runtime_set_active(dev);
>>>    	return 0;
>>>    }
Daniel Vetter June 20, 2017, 9:37 a.m. UTC | #4
On Mon, Jun 19, 2017 at 05:45:21PM +0200, Noralf Trønnes wrote:
> 
> Den 19.06.2017 15.17, skrev Liviu Dudau:
> > On Fri, Jun 16, 2017 at 06:58:36PM +0200, Noralf Trønnes wrote:
> > > Den 16.06.2017 15.53, skrev Liviu Dudau:
> > > > Update the PM code to suspend/resume the fbdev_cma console.
> > > > 
> > > > Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
> > > > ---
> > > >    drivers/gpu/drm/arm/hdlcd_drv.c | 11 ++++++++++-
> > > >    1 file changed, 10 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
> > > > index d3da87fbd85a..89cd408cde6f 100644
> > > > --- a/drivers/gpu/drm/arm/hdlcd_drv.c
> > > > +++ b/drivers/gpu/drm/arm/hdlcd_drv.c
> > > > @@ -13,6 +13,7 @@
> > > >    #include <linux/spinlock.h>
> > > >    #include <linux/clk.h>
> > > >    #include <linux/component.h>
> > > > +#include <linux/console.h>
> > > >    #include <linux/list.h>
> > > >    #include <linux/of_graph.h>
> > > >    #include <linux/of_reserved_mem.h>
> > > > @@ -435,9 +436,15 @@ static int __maybe_unused hdlcd_pm_suspend(struct device *dev)
> > > >    		return 0;
> > > >    	drm_kms_helper_poll_disable(drm);
> > > > +	console_lock();
> > > > +	drm_fbdev_cma_set_suspend(hdlcd->fbdev, 1);
> > > > +	console_unlock();
> > > You can use drm_fbdev_cma_set_suspend_unlocked() instead, it takes the
> > > lock for you and can speed up resume if the lock is contented.
> > Hi Noralf,
> > 
> > Thanks for pointing out the helpful function. As you look to be the author of it,
> > any reason why the signature of the function doesn't match the drm_fb_helper_ one
> > being called through? (I'm talking about int vs bool for the state/suspend arguments).
> 
> I don't remember, but probably to match drm_fbdev_cma_set_suspend()
> which uses int. drm_fb_helper_set_suspend*() uses bool, but calls
> into fb_set_suspend() which uses int, but as a boolean.

I'd be happy to apply a patch which changes them all to bool ...

Cheers, Daniel
diff mbox

Patch

diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
index d3da87fbd85a..89cd408cde6f 100644
--- a/drivers/gpu/drm/arm/hdlcd_drv.c
+++ b/drivers/gpu/drm/arm/hdlcd_drv.c
@@ -13,6 +13,7 @@ 
 #include <linux/spinlock.h>
 #include <linux/clk.h>
 #include <linux/component.h>
+#include <linux/console.h>
 #include <linux/list.h>
 #include <linux/of_graph.h>
 #include <linux/of_reserved_mem.h>
@@ -435,9 +436,15 @@  static int __maybe_unused hdlcd_pm_suspend(struct device *dev)
 		return 0;
 
 	drm_kms_helper_poll_disable(drm);
+	console_lock();
+	drm_fbdev_cma_set_suspend(hdlcd->fbdev, 1);
+	console_unlock();
 
 	hdlcd->state = drm_atomic_helper_suspend(drm);
 	if (IS_ERR(hdlcd->state)) {
+		console_lock();
+		drm_fbdev_cma_set_suspend(hdlcd->fbdev, 0);
+		console_unlock();
 		drm_kms_helper_poll_enable(drm);
 		return PTR_ERR(hdlcd->state);
 	}
@@ -454,8 +461,10 @@  static int __maybe_unused hdlcd_pm_resume(struct device *dev)
 		return 0;
 
 	drm_atomic_helper_resume(drm, hdlcd->state);
+	console_lock();
+	drm_fbdev_cma_set_suspend(hdlcd->fbdev, 0);
+	console_unlock();
 	drm_kms_helper_poll_enable(drm);
-	pm_runtime_set_active(dev);
 
 	return 0;
 }