linux-kernel - Re: [PATCH] drm: mxsfb: fix runtime PM handling

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <122d65ad-8840-f991-4c73-c28d5c06c3b0@denx.de>
Date:   Fri, 13 Jul 2018 12:45:54 +0200
From:   Marek Vasut <marex@...x.de>
To:     Stefan Agner <stefan@...er.ch>
Cc:     Anson Huang <anson.huang@....com>, airlied@...ux.ie,
        dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
        dl-linux-imx <linux-imx@....com>
Subject: Re: [PATCH] drm: mxsfb: fix runtime PM handling

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@....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