linux-kernel - Re: [PATCH 1/1] iommu/arm-smmu-qcom: remove runtime pm enabling for TBU driver

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: <4d89a048-eef0-4295-a4cc-500390f005d1@quicinc.com>
Date: Wed, 14 Aug 2024 14:54:07 +0800
From: Zhenhua Huang <quic_zhenhuah@...cinc.com>
To: Georgi Djakov <quic_c_gdjako@...cinc.com>,
        Pranjal Shrivastava
	<praan@...gle.com>
CC: <robdclark@...il.com>, <will@...nel.org>, <robin.murphy@....com>,
        <joro@...tes.org>, <iommu@...ts.linux.dev>,
        <linux-arm-msm@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>, <linux-kernel@...r.kernel.org>,
        <dmitry.baryshkov@...aro.org>, Georgi Djakov <djakov@...nel.org>
Subject: Re: [PATCH 1/1] iommu/arm-smmu-qcom: remove runtime pm enabling for
 TBU driver

Hi Georgi,

On 2024/8/13 20:06, Georgi Djakov wrote:
> Hi Zhenhua,
> 
> On 8/13/2024 10:56 AM, Zhenhua Huang wrote:
>>
>> On 2024/8/13 15:20, Pranjal Shrivastava wrote:
>>> On Tue, Aug 13, 2024 at 10:37:33AM +0800, Zhenhua Huang wrote:
>>>>
>>>> On 2024/8/12 21:25, Pranjal Shrivastava wrote:
>>>>> On Tue, Jul 30, 2024 at 06:30:43PM +0800, Zhenhua Huang wrote:
>>>>>> TBU driver has no runtime pm support now, adding pm_runtime_enable()
>>>>>> seems to be useless. Remove it.
>>>>>>
>>>>>> Signed-off-by: Zhenhua Huang <quic_zhenhuah@...cinc.com>
> [..]
>>> I agree that there are no pm_runtime_suspend/resume calls within the TBU
>>> driver. I'm just trying to understand why was pm_runtime enabled here
>>> earlier (since it's not implemented) in order to ensure that removing it
>>> doesn't cause further troubles?
>>
>> See above my assumption, need Georgi to comment but.
> 
> Thank you for looking at the code! Your assumptions are mostly correct,
> but if you try this patch on a real sdm845 device you will notice some
> issues. So it's actually needed to re-configure the power-domains, three

Thanks Georgi for your comments!
Hmm...  so you found some bugs on sdm845 ? sorry that I don't have 
sdm845 on hand...

> of which (MMNOC GDSCs) are requiring this because of a HW bug. I should
> have put a comment in the code to avoid confusion, but it took me some
> time to confirm it.
> 
> I have sent a patch to handle this more cleanly:
> https://lore.kernel.org/lkml/20240813120015.3242787-1-quic_c_gdjako@quicinc.com
> 
> So we should not remove the runtime pm calls until some version of the
> above patch gets merged.

In my sense, above patch should not result in turning off gdsc? It's 
just open the support for RPM.. I tried to do same change for arm-smmu 
driver, w/ test I see cx_gdsc which is the power-domain for gfx_smmu, is on:
..
/sys/kernel/debug/pm_genpd/cx_gdsc # cat current_state
on

Are you worrying that not setting active will turn off related PD? or 
Could you please explain a bit more about how the change impacted power 
domain status? Thanks in advance :)

> 
> Thanks,
> Georgi
> 
>>> I see Georgi added it as a part of
>>> https://lore.kernel.org/all/20240704010759.507798-1-quic_c_gdjako@quicinc.com/
>>>
>>> But I'm unsure why was it required to fix that bug?
>>
>> I'm just thinking it is dead code and want to see if my understanding is correct.
> 
>