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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxx>
[..]
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@xxxxxxxxxxx

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@xxxxxxxxxxx/

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.






[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux