[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 at 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