[PATCH] drm: mxsfb: fix runtime PM handling

[PATCH] drm: mxsfb: fix runtime PM handling

[Anson Huang]

As display power domain is combined with lcdif node
on some i.MX platforms like i.MX6SL, when lcdif driver
is enabled, the mxsfb_load is called to enable runtime
pm, and a pair of pm_runtime_get_sync and
pm_runtime_put_sync are also called, that will cause
generic power domain driver to disable lcdif power domain
and lcdif is no longer working, the lcdif power
should ONLY be turned off when display is disabled,
so move the pm_runtime_put_sync to mxsfb_unload
and remove the pm_runtime_get_sync in mxsfb_unload
as well, in this way, when display is enabled, the
lcdif power will be always ON until the display
is disabled.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 drivers/gpu/drm/mxsfb/mxsfb_drv.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
index ffe5137..1ba179b 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
@@ -235,7 +235,6 @@ static int mxsfb_load(struct drm_device *drm, unsigned long flags)
 
 	pm_runtime_get_sync(drm->dev);
 	ret = drm_irq_install(drm, platform_get_irq(pdev, 0));
-	pm_runtime_put_sync(drm->dev);
 
 	if (ret < 0) {
 		dev_err(drm->dev, "Failed to install IRQ handler\n");
@@ -264,6 +263,7 @@ static int mxsfb_load(struct drm_device *drm, unsigned long flags)
 err_irq:
 	drm_panel_detach(mxsfb->panel);
 err_vblank:
+	pm_runtime_put_sync(drm->dev);
 	pm_runtime_disable(drm->dev);
 
 	return ret;
@@ -279,7 +279,6 @@ static void mxsfb_unload(struct drm_device *drm)
 	drm_kms_helper_poll_fini(drm);
 	drm_mode_config_cleanup(drm);
 
-	pm_runtime_get_sync(drm->dev);
 	drm_irq_uninstall(drm);
 	pm_runtime_put_sync(drm->dev);
 
-- 
2.7.4

Re: [PATCH] drm: mxsfb: fix runtime PM handling

[Marek Vasut]

On 07/13/2018 10:54 AM, Anson Huang wrote:
> As display power domain is combined with lcdif node
> on some i.MX platforms like i.MX6SL, when lcdif driver
> is enabled, the mxsfb_load is called to enable runtime
> pm, and a pair of pm_runtime_get_sync and
> pm_runtime_put_sync are also called, that will cause
> generic power domain driver to disable lcdif power domain
> and lcdif is no longer working, the lcdif power
> should ONLY be turned off when display is disabled,
> so move the pm_runtime_put_sync to mxsfb_unload
> and remove the pm_runtime_get_sync in mxsfb_unload
> as well, in this way, when display is enabled, the
> lcdif power will be always ON until the display
> is disabled.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>

Doesn't this also mean the block will always be on, thus wasting power ?

> ---
>  drivers/gpu/drm/mxsfb/mxsfb_drv.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> index ffe5137..1ba179b 100644
> --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> @@ -235,7 +235,6 @@ static int mxsfb_load(struct drm_device *drm, unsigned long flags)
>  
>  	pm_runtime_get_sync(drm->dev);
>  	ret = drm_irq_install(drm, platform_get_irq(pdev, 0));
> -	pm_runtime_put_sync(drm->dev);
>  
>  	if (ret < 0) {
>  		dev_err(drm->dev, "Failed to install IRQ handler\n");
> @@ -264,6 +263,7 @@ static int mxsfb_load(struct drm_device *drm, unsigned long flags)
>  err_irq:
>  	drm_panel_detach(mxsfb->panel);
>  err_vblank:
> +	pm_runtime_put_sync(drm->dev);
>  	pm_runtime_disable(drm->dev);
>  
>  	return ret;
> @@ -279,7 +279,6 @@ static void mxsfb_unload(struct drm_device *drm)
>  	drm_kms_helper_poll_fini(drm);
>  	drm_mode_config_cleanup(drm);
>  
> -	pm_runtime_get_sync(drm->dev);
>  	drm_irq_uninstall(drm);
>  	pm_runtime_put_sync(drm->dev);
>  
> 


-- 
Best regards,
Marek Vasut

RE: [PATCH] drm: mxsfb: fix runtime PM handling

[Anson Huang]

Anson Huang
Best Regards!


> -----Original Message-----
> From: Marek Vasut [mailto:marex@denx.de]
> Sent: Friday, July 13, 2018 5:03 PM
> To: Anson Huang <anson.huang@nxp.com>; airlied@linux.ie;
> dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org
> Cc: dl-linux-imx <linux-imx@nxp.com>
> Subject: Re: [PATCH] drm: mxsfb: fix runtime PM handling
> 
> On 07/13/2018 10:54 AM, Anson Huang wrote:
> > As display power domain is combined with lcdif node on some i.MX
> > platforms like i.MX6SL, when lcdif driver is enabled, the mxsfb_load
> > is called to enable runtime pm, and a pair of pm_runtime_get_sync and
> > pm_runtime_put_sync are also called, that will cause generic power
> > domain driver to disable lcdif power domain and lcdif is no longer
> > working, the lcdif power should ONLY be turned off when display is
> > disabled, so move the pm_runtime_put_sync to mxsfb_unload and remove
> > the pm_runtime_get_sync in mxsfb_unload as well, in this way, when
> > display is enabled, the lcdif power will be always ON until the
> > display is disabled.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> 
> Doesn't this also mean the block will always be on, thus wasting power ?
 
I think drm driver should have somewhere to implement the display
disable case, like when fb0 is blank (echo 1 > /sys/class/graphics/fb0/blank),
then lcdif can be powered gated, and
when display is back on (unblank), lcdif needs to be re-initialization and display will
be on, current implementation is incorrect, with kernel booting up, lcdif
is NOT working at all.

The runtime power gating case can be implemented later to save power,
need lcdif experts to do it. Thanks.

Anson.

> 
> > ---
> >  drivers/gpu/drm/mxsfb/mxsfb_drv.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> > b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> > index ffe5137..1ba179b 100644
> > --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> > +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> > @@ -235,7 +235,6 @@ static int mxsfb_load(struct drm_device *drm,
> > unsigned long flags)
> >
> >  	pm_runtime_get_sync(drm->dev);
> >  	ret = drm_irq_install(drm, platform_get_irq(pdev, 0));
> > -	pm_runtime_put_sync(drm->dev);
> >
> >  	if (ret < 0) {
> >  		dev_err(drm->dev, "Failed to install IRQ handler\n"); @@ -264,6
> > +263,7 @@ static int mxsfb_load(struct drm_device *drm, unsigned long
> > flags)
> >  err_irq:
> >  	drm_panel_detach(mxsfb->panel);
> >  err_vblank:
> > +	pm_runtime_put_sync(drm->dev);
> >  	pm_runtime_disable(drm->dev);
> >
> >  	return ret;
> > @@ -279,7 +279,6 @@ static void mxsfb_unload(struct drm_device *drm)
> >  	drm_kms_helper_poll_fini(drm);
> >  	drm_mode_config_cleanup(drm);
> >
> > -	pm_runtime_get_sync(drm->dev);
> >  	drm_irq_uninstall(drm);
> >  	pm_runtime_put_sync(drm->dev);
> >
> >
> 
> 
> --
> Best regards,
> Marek Vasut

Re: [PATCH] drm: mxsfb: fix runtime PM handling

[Marek Vasut]

On 07/13/2018 11:06 AM, Anson Huang wrote:
[...]
>>
>> On 07/13/2018 10:54 AM, Anson Huang wrote:
>>> As display power domain is combined with lcdif node on some i.MX
>>> platforms like i.MX6SL, when lcdif driver is enabled, the mxsfb_load
>>> is called to enable runtime pm, and a pair of pm_runtime_get_sync and
>>> pm_runtime_put_sync are also called, that will cause generic power
>>> domain driver to disable lcdif power domain and lcdif is no longer
>>> working, the lcdif power should ONLY be turned off when display is
>>> disabled, so move the pm_runtime_put_sync to mxsfb_unload and remove
>>> the pm_runtime_get_sync in mxsfb_unload as well, in this way, when
>>> display is enabled, the lcdif power will be always ON until the
>>> display is disabled.
>>>
>>> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
>>
>> Doesn't this also mean the block will always be on, thus wasting power ?
>  
> I think drm driver should have somewhere to implement the display
> disable case, like when fb0 is blank (echo 1 > /sys/class/graphics/fb0/blank),

Isn't this just the fbdev emulation on top of drm/kms ?
I think this stuff can be compiled out completely.

> then lcdif can be powered gated, and
> when display is back on (unblank), lcdif needs to be re-initialization and display will
> be on, current implementation is incorrect, with kernel booting up, lcdif
> is NOT working at all.

It works fine on MX6SX , so I think this is isolated to MX6SL ?
I'm CCing Stefan, he might have some valuable feedback here.

> The runtime power gating case can be implemented later to save power,
> need lcdif experts to do it. Thanks.
> 
> Anson.
> 
>>
>>> ---
>>>  drivers/gpu/drm/mxsfb/mxsfb_drv.c | 3 +--
>>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
>>> b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
>>> index ffe5137..1ba179b 100644
>>> --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
>>> +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
>>> @@ -235,7 +235,6 @@ static int mxsfb_load(struct drm_device *drm,
>>> unsigned long flags)
>>>
>>>  	pm_runtime_get_sync(drm->dev);
>>>  	ret = drm_irq_install(drm, platform_get_irq(pdev, 0));
>>> -	pm_runtime_put_sync(drm->dev);
>>>
>>>  	if (ret < 0) {
>>>  		dev_err(drm->dev, "Failed to install IRQ handler\n"); @@ -264,6
>>> +263,7 @@ static int mxsfb_load(struct drm_device *drm, unsigned long
>>> flags)
>>>  err_irq:
>>>  	drm_panel_detach(mxsfb->panel);
>>>  err_vblank:
>>> +	pm_runtime_put_sync(drm->dev);
>>>  	pm_runtime_disable(drm->dev);
>>>
>>>  	return ret;
>>> @@ -279,7 +279,6 @@ static void mxsfb_unload(struct drm_device *drm)
>>>  	drm_kms_helper_poll_fini(drm);
>>>  	drm_mode_config_cleanup(drm);
>>>
>>> -	pm_runtime_get_sync(drm->dev);
>>>  	drm_irq_uninstall(drm);
>>>  	pm_runtime_put_sync(drm->dev);
>>>
>>>
>>
>>
>> --
>> Best regards,
>> Marek Vasut


-- 
Best regards,
Marek Vasut

Re: [PATCH] drm: mxsfb: fix runtime PM handling

[Leonard Crestez]

On Fri, 2018-07-13 at 11:12 +0200, Marek Vasut wrote:
> On 07/13/2018 11:06 AM, Anson Huang wrote:
> [...]
> > > 
> > > On 07/13/2018 10:54 AM, Anson Huang wrote:
> > > > As display power domain is combined with lcdif node on some i.MX
> > > > platforms like i.MX6SL, when lcdif driver is enabled, the mxsfb_load
> > > > is called to enable runtime pm, and a pair of pm_runtime_get_sync and
> > > > pm_runtime_put_sync are also called, that will cause generic power
> > > > domain driver to disable lcdif power domain and lcdif is no longer
> > > > working, the lcdif power should ONLY be turned off when display is
> > > > disabled, so move the pm_runtime_put_sync to mxsfb_unload and remove
> > > > the pm_runtime_get_sync in mxsfb_unload as well, in this way, when
> > > > display is enabled, the lcdif power will be always ON until the
> > > > display is disabled.
> > > > 
> > > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > > 
> > > Doesn't this also mean the block will always be on, thus wasting power ?

> > I think drm driver should have somewhere to implement the display
> > disable case, like when fb0 is blank (echo 1 > /sys/class/graphics/fb0/blank),
> 
> Isn't this just the fbdev emulation on top of drm/kms ?
> I think this stuff can be compiled out completely.
> 
> > then lcdif can be powered gated, and
> > when display is back on (unblank), lcdif needs to be re-initialization and display will
> > be on, current implementation is incorrect, with kernel booting up, lcdif
> > is NOT working at all.
> 
> It works fine on MX6SX , so I think this is isolated to MX6SL ?
> I'm CCing Stefan, he might have some valuable feedback here.

Some time ago I sent a patch which tries to implement "proper" runtime
PM for lcdif: https://patchwork.kernel.org/patch/10449761/

It seems to have slipped through the cracks, can somebody please take a
look at it?

This only actually matters on SOCs where the display power domain is
gated, however:

* On imx6sx the DISP power domain is currently not defined in DTS.
Defining requires dealing with pcie being spread in multiple PDs.
* On imx6sl there is a nasty errata so the PD was marked always-on.
This was a few days ago: https://lkml.org/lkml/2018/7/11/427
* imx6sll is new, evk dts only in next.

The lcdif block is also used in some imx8 chips so runtime PM should be
dealt with eventually.

Re: [PATCH] drm: mxsfb: fix runtime PM handling

[Stefan Agner]

On 13.07.2018 11:12, Marek Vasut wrote:
> On 07/13/2018 11:06 AM, Anson Huang wrote:
> [...]
>>>
>>> On 07/13/2018 10:54 AM, Anson Huang wrote:
>>>> As display power domain is combined with lcdif node on some i.MX
>>>> platforms like i.MX6SL, when lcdif driver is enabled, the mxsfb_load
>>>> is called to enable runtime pm, and a pair of pm_runtime_get_sync and
>>>> pm_runtime_put_sync are also called, that will cause generic power
>>>> domain driver to disable lcdif power domain and lcdif is no longer
>>>> working, the lcdif power should ONLY be turned off when display is
>>>> disabled, so move the pm_runtime_put_sync to mxsfb_unload and remove
>>>> the pm_runtime_get_sync in mxsfb_unload as well, in this way, when
>>>> display is enabled, the lcdif power will be always ON until the
>>>> display is disabled.
>>>>
>>>> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
>>>
>>> Doesn't this also mean the block will always be on, thus wasting power ?
>>
>> I think drm driver should have somewhere to implement the display
>> disable case, like when fb0 is blank (echo 1 > /sys/class/graphics/fb0/blank),
> 
> Isn't this just the fbdev emulation on top of drm/kms ?
> I think this stuff can be compiled out completely.
> 
>> then lcdif can be powered gated, and
>> when display is back on (unblank), lcdif needs to be re-initialization and display will
>> be on, current implementation is incorrect, with kernel booting up, lcdif
>> is NOT working at all.
> 
> It works fine on MX6SX , so I think this is isolated to MX6SL ?
> I'm CCing Stefan, he might have some valuable feedback here.
> 

Yeah not sure, but putting it in mxsfb_load seems wrong.

MXSFB uses struct drm_simple_display_pipe_funcs which does have
enable/disable callbacks, probably closer to what we want...

--
Stefan

>> The runtime power gating case can be implemented later to save power,
>> need lcdif experts to do it. Thanks.
>>
>> Anson.
>>
>>>
>>>> ---
>>>>  drivers/gpu/drm/mxsfb/mxsfb_drv.c | 3 +--
>>>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
>>>> b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
>>>> index ffe5137..1ba179b 100644
>>>> --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
>>>> +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
>>>> @@ -235,7 +235,6 @@ static int mxsfb_load(struct drm_device *drm,
>>>> unsigned long flags)
>>>>
>>>>  	pm_runtime_get_sync(drm->dev);
>>>>  	ret = drm_irq_install(drm, platform_get_irq(pdev, 0));
>>>> -	pm_runtime_put_sync(drm->dev);
>>>>
>>>>  	if (ret < 0) {
>>>>  		dev_err(drm->dev, "Failed to install IRQ handler\n"); @@ -264,6
>>>> +263,7 @@ static int mxsfb_load(struct drm_device *drm, unsigned long
>>>> flags)
>>>>  err_irq:
>>>>  	drm_panel_detach(mxsfb->panel);
>>>>  err_vblank:
>>>> +	pm_runtime_put_sync(drm->dev);
>>>>  	pm_runtime_disable(drm->dev);
>>>>
>>>>  	return ret;
>>>> @@ -279,7 +279,6 @@ static void mxsfb_unload(struct drm_device *drm)
>>>>  	drm_kms_helper_poll_fini(drm);
>>>>  	drm_mode_config_cleanup(drm);
>>>>
>>>> -	pm_runtime_get_sync(drm->dev);
>>>>  	drm_irq_uninstall(drm);
>>>>  	pm_runtime_put_sync(drm->dev);
>>>>
>>>>
>>>
>>>
>>> --
>>> Best regards,
>>> Marek Vasut

Re: [PATCH] drm: mxsfb: fix runtime PM handling

[Marek Vasut]

On 07/13/2018 12:33 PM, Stefan Agner wrote:
> On 13.07.2018 11:12, Marek Vasut wrote:
>> On 07/13/2018 11:06 AM, Anson Huang wrote:
>> [...]
>>>>
>>>> On 07/13/2018 10:54 AM, Anson Huang wrote:
>>>>> As display power domain is combined with lcdif node on some i.MX
>>>>> platforms like i.MX6SL, when lcdif driver is enabled, the mxsfb_load
>>>>> is called to enable runtime pm, and a pair of pm_runtime_get_sync and
>>>>> pm_runtime_put_sync are also called, that will cause generic power
>>>>> domain driver to disable lcdif power domain and lcdif is no longer
>>>>> working, the lcdif power should ONLY be turned off when display is
>>>>> disabled, so move the pm_runtime_put_sync to mxsfb_unload and remove
>>>>> the pm_runtime_get_sync in mxsfb_unload as well, in this way, when
>>>>> display is enabled, the lcdif power will be always ON until the
>>>>> display is disabled.
>>>>>
>>>>> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
>>>>
>>>> Doesn't this also mean the block will always be on, thus wasting power ?
>>>
>>> I think drm driver should have somewhere to implement the display
>>> disable case, like when fb0 is blank (echo 1 > /sys/class/graphics/fb0/blank),
>>
>> Isn't this just the fbdev emulation on top of drm/kms ?
>> I think this stuff can be compiled out completely.
>>
>>> then lcdif can be powered gated, and
>>> when display is back on (unblank), lcdif needs to be re-initialization and display will
>>> be on, current implementation is incorrect, with kernel booting up, lcdif
>>> is NOT working at all.
>>
>> It works fine on MX6SX , so I think this is isolated to MX6SL ?
>> I'm CCing Stefan, he might have some valuable feedback here.
>>
> 
> Yeah not sure, but putting it in mxsfb_load seems wrong.
> 
> MXSFB uses struct drm_simple_display_pipe_funcs which does have
> enable/disable callbacks, probably closer to what we want...

Seems to me like this is not something that should be hacked around in
the mxsfb driver in the first place, but somewhere else.

-- 
Best regards,
Marek Vasut

Re: [PATCH] drm: mxsfb: fix runtime PM handling

[Stefan Agner]

On 13.07.2018 12:36, Marek Vasut wrote:
> On 07/13/2018 12:33 PM, Stefan Agner wrote:
>> On 13.07.2018 11:12, Marek Vasut wrote:
>>> On 07/13/2018 11:06 AM, Anson Huang wrote:
>>> [...]
>>>>>
>>>>> On 07/13/2018 10:54 AM, Anson Huang wrote:
>>>>>> As display power domain is combined with lcdif node on some i.MX
>>>>>> platforms like i.MX6SL, when lcdif driver is enabled, the mxsfb_load
>>>>>> is called to enable runtime pm, and a pair of pm_runtime_get_sync and
>>>>>> pm_runtime_put_sync are also called, that will cause generic power
>>>>>> domain driver to disable lcdif power domain and lcdif is no longer
>>>>>> working, the lcdif power should ONLY be turned off when display is
>>>>>> disabled, so move the pm_runtime_put_sync to mxsfb_unload and remove
>>>>>> the pm_runtime_get_sync in mxsfb_unload as well, in this way, when
>>>>>> display is enabled, the lcdif power will be always ON until the
>>>>>> display is disabled.
>>>>>>
>>>>>> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
>>>>>
>>>>> Doesn't this also mean the block will always be on, thus wasting power ?
>>>>
>>>> I think drm driver should have somewhere to implement the display
>>>> disable case, like when fb0 is blank (echo 1 > /sys/class/graphics/fb0/blank),
>>>
>>> Isn't this just the fbdev emulation on top of drm/kms ?
>>> I think this stuff can be compiled out completely.
>>>
>>>> then lcdif can be powered gated, and
>>>> when display is back on (unblank), lcdif needs to be re-initialization and display will
>>>> be on, current implementation is incorrect, with kernel booting up, lcdif
>>>> is NOT working at all.
>>>
>>> It works fine on MX6SX , so I think this is isolated to MX6SL ?
>>> I'm CCing Stefan, he might have some valuable feedback here.
>>>
>>
>> Yeah not sure, but putting it in mxsfb_load seems wrong.
>>
>> MXSFB uses struct drm_simple_display_pipe_funcs which does have
>> enable/disable callbacks, probably closer to what we want...
> 
> Seems to me like this is not something that should be hacked around in
> the mxsfb driver in the first place, but somewhere else.

By somewhere else you mean in the DRM stack?

Yeah not sure, currently pm seems to be handled on driver level only,
see
grep -r -e pm_runtime_get_sync drivers/gpu/drm

--
Stefan

Re: [PATCH] drm: mxsfb: fix runtime PM handling

[Marek Vasut]

On 07/13/2018 12:39 PM, Stefan Agner wrote:
> On 13.07.2018 12:36, Marek Vasut wrote:
>> On 07/13/2018 12:33 PM, Stefan Agner wrote:
>>> On 13.07.2018 11:12, Marek Vasut wrote:
>>>> On 07/13/2018 11:06 AM, Anson Huang wrote:
>>>> [...]
>>>>>>
>>>>>> On 07/13/2018 10:54 AM, Anson Huang wrote:
>>>>>>> As display power domain is combined with lcdif node on some i.MX
>>>>>>> platforms like i.MX6SL, when lcdif driver is enabled, the mxsfb_load
>>>>>>> is called to enable runtime pm, and a pair of pm_runtime_get_sync and
>>>>>>> pm_runtime_put_sync are also called, that will cause generic power
>>>>>>> domain driver to disable lcdif power domain and lcdif is no longer
>>>>>>> working, the lcdif power should ONLY be turned off when display is
>>>>>>> disabled, so move the pm_runtime_put_sync to mxsfb_unload and remove
>>>>>>> the pm_runtime_get_sync in mxsfb_unload as well, in this way, when
>>>>>>> display is enabled, the lcdif power will be always ON until the
>>>>>>> display is disabled.
>>>>>>>
>>>>>>> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
>>>>>>
>>>>>> Doesn't this also mean the block will always be on, thus wasting power ?
>>>>>
>>>>> I think drm driver should have somewhere to implement the display
>>>>> disable case, like when fb0 is blank (echo 1 > /sys/class/graphics/fb0/blank),
>>>>
>>>> Isn't this just the fbdev emulation on top of drm/kms ?
>>>> I think this stuff can be compiled out completely.
>>>>
>>>>> then lcdif can be powered gated, and
>>>>> when display is back on (unblank), lcdif needs to be re-initialization and display will
>>>>> be on, current implementation is incorrect, with kernel booting up, lcdif
>>>>> is NOT working at all.
>>>>
>>>> It works fine on MX6SX , so I think this is isolated to MX6SL ?
>>>> I'm CCing Stefan, he might have some valuable feedback here.
>>>>
>>>
>>> Yeah not sure, but putting it in mxsfb_load seems wrong.
>>>
>>> MXSFB uses struct drm_simple_display_pipe_funcs which does have
>>> enable/disable callbacks, probably closer to what we want...
>>
>> Seems to me like this is not something that should be hacked around in
>> the mxsfb driver in the first place, but somewhere else.
> 
> By somewhere else you mean in the DRM stack?

Rather the clock driver or somesuch. Maybe the power domains need to be
modeled in the DT properly, but I didn't dig in deep enough.

> Yeah not sure, currently pm seems to be handled on driver level only,
> see
> grep -r -e pm_runtime_get_sync drivers/gpu/drm
> 
> --
> Stefan
> 


-- 
Best regards,
Marek Vasut