Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH] drm: mxsfb: fix runtime PM handling
To:     Stefan Agner <stefan@agner.ch>
Cc:     Anson Huang <anson.huang@nxp.com>, airlied@linux.ie,
        dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
        dl-linux-imx <linux-imx@nxp.com>
References: <1531472085-25174-1-git-send-email-Anson.Huang@nxp.com>
 <ea029b69-f853-4a5f-e798-70a04b02ac1d@denx.de>
 <AM3PR04MB1315E2F345752D0A64A0E2C8F5580@AM3PR04MB1315.eurprd04.prod.outlook.com>
 <90d16681-2a7a-4826-0411-478353d2afe3@denx.de>
 <00b5faf737efd1054eb4f0f0e6fc8369@agner.ch>
From:   Marek Vasut <marex@denx.de>
Message-ID: <52c0153c-1810-2919-70b7-d57280c83c90@denx.de>
Date:   Fri, 13 Jul 2018 12:36:17 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.8.0
MIME-Version: 1.0
In-Reply-To: <00b5faf737efd1054eb4f0f0e6fc8369@agner.ch>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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