linux-kernel - Re: [PATCH 2/2] drm/panfrost: add devfreq regulator support

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <13285d84-5a5e-c695-2a8b-7ca50c976b90@arm.com>
Date:   Thu, 7 May 2020 15:30:24 +0100
From:   Steven Price <steven.price@....com>
To:     Clément Péron <peron.clem@...il.com>
Cc:     Rob Herring <robh@...nel.org>,
        Tomeu Vizoso <tomeu.vizoso@...labora.com>,
        Alyssa Rosenzweig <alyssa.rosenzweig@...labora.com>,
        Viresh Kumar <vireshk@...nel.org>, Nishanth Menon <nm@...com>,
        Stephen Boyd <sboyd@...nel.org>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/2] drm/panfrost: add devfreq regulator support

On 02/05/2020 23:07, Clément Péron wrote:
> Hi Steven,
> 
> On Tue, 14 Apr 2020 at 15:10, Steven Price <steven.price@....com> wrote:
>>
>> Hi Clément,
>>
>> On 13/04/2020 18:28, Clément Péron wrote:
>>> Hi Steven,
>>>
> 
> <snip>
> 
>> Getting a backtrace from the two occurrences, I see one added from:
>>
>>     (debugfs_create_dir) from [<c04633f8>] (create_regulator+0xe0/0x220)
>>     (create_regulator) from [<c04681d8>] (_regulator_get+0x168/0x204)
>>     (_regulator_get) from [<c04682e0>] (regulator_bulk_get+0x64/0xf4)
>>     (regulator_bulk_get) from [<c04696f0>]
>> (devm_regulator_bulk_get+0x40/0x74)
>>     (devm_regulator_bulk_get) from [<bf00af44>]
>> (panfrost_device_init+0x1b4/0x48c [panfrost])
>>     (panfrost_device_init [panfrost]) from [<bf00a4d4>]
>> (panfrost_probe+0x94/0x184 [panfrost])
>>     (panfrost_probe [panfrost]) from [<c04ee694>]
>> (platform_drv_probe+0x48/0x94)
>>
>> And the other:
>>
>>     (debugfs_create_dir) from [<c04633f8>] (create_regulator+0xe0/0x220)
>>     (create_regulator) from [<c04681d8>] (_regulator_get+0x168/0x204)
>>     (_regulator_get) from [<c05c1280>] (dev_pm_opp_set_regulators+0x6c/0x184)
>>     (dev_pm_opp_set_regulators) from [<bf00b4ac>]
>> (panfrost_devfreq_init+0x38/0x1ac [panfrost])
>>     (panfrost_devfreq_init [panfrost]) from [<bf00a508>]
>> (panfrost_probe+0xc8/0x184 [panfrost])
>>     (panfrost_probe [panfrost]) from [<c04ee694>]
>> (platform_drv_probe+0x48/0x94)
>>
>> Both are created at /regulator/vdd_gpu
> 
> I'm preparing a new version with some clean from lima devfreq.
> My working branch :
> https://github.com/clementperon/linux/commits/panfrost_devfreq

I had a look at that branch and gave it a quick spin on my Firefly 
RK3288 and didn't notice any issues.

> Two strange things I observe:
>   - After 30sec the regulator is released by OPP ???
> [   33.757627] vdd-gpu: disabling
> Introduce the regulator support in this commit:
> https://github.com/clementperon/linux/commit/be310c37b82010e293b7f129ccdcb711a2abb2ce

I can't see anything wrong with this commit, but equally in my DTS I 
have a "regulator-always-on" for vdd_gpu. My initial thought was that 
this could be runtime PM of the GPU - but I can't see how 
panfrost_device_suspend() would end up turning off the regulator. So 
unless there's some way that the regulator itself suspends (but it 
should know it's in use) I've no clue why this would be happening.

Since you've got a reproduction - can you get a backtrace where the 
regulator is getting disabled?

>   - The Cooling map is not probe correctly :
> [    2.545756] panfrost 1800000.gpu: [drm:panfrost_devfreq_init
> [panfrost]] Failed to register cooling device
> Introduce in this commit :
> https://github.com/clementperon/linux/commit/0252c38fd55ad78366ac4b1714e285c88db34557
> 
> Do you have an hint about what I'm missing ?

Sorry, my knowledge of the cooling framework is very limited. What 
you've got looks plausible, but I'm afraid I can't really help beyond 
that! As before - can you try adding some printk()s in e.g. 
of_devfreq_cooling_register_power() and find out where it is bailing out?

Steve