Re: [PATCH v13 13/35] drm/tegra: gr2d: Support generic power domain and runtime PM

From: Dmitry Osipenko
Date: Tue Oct 05 2021 - 13:17:18 EST


...
>> It's not a problem to change this patchset. The problem is that if
>> you'll grep mainline for 'pm_runtime_disable', you will find that there
>> are a lot of drivers in a potential trouble.
>
> Let's start by fixing this patchset, please - then we can consider
> what to do with the other cases separately.

Yeah, should be better to discuss it separately.

...
>> void __pm_runtime_disable(struct device *dev, bool check_resume)
>> {
>> + flush_work(&dev->power.work);
>> +
>
> What about the latency this may introduce? I am not sure that is
> acceptable here!?

I'm not aware about any code which relies on the original 'cancelling'
behaviour, perhaps Rafael should have more insight.

...
>> The sysfs rpm-forbid is a separate problem and it's less troublesome
>> since it requires root privileges. It's also not something that
>> userspace touches casually. For now I don't know what could be done
>> about it.
>
> As I said, the common method to address this problem is to run the
> following sequence:
>
> pm_runtime_get_sync()
> "power off the device"
> pm_runtime_disable()
> pm_runtime_put_noidle()
>
> This works even if user space, via sysfs, has triggered a call to
> pm_runtime_forbid(). Or doesn't it?
>
> If you don't like it, pm_runtime_force_suspend() should work too, at
> least for your cases, I believe.

I'll update the patches, thank you.