Re: [PATCH v3 3/8] drm/imagination: Use pwrseq for TH1520 GPU power management

[Michal Wilczynski <m.wilczynski@samsung.com>]

On 6/4/25 08:36, Krzysztof Kozlowski wrote:
> On 03/06/2025 21:43, Michal Wilczynski wrote:
>>>> +	 * and resets. Otherwise, we fall back to managing them ourselves.
>>>> +	 */
>>>> +	pvr_dev->pwrseq = devm_pwrseq_get(dev, "gpu-power");
>>>> +	if (IS_ERR(pvr_dev->pwrseq)) {
>>>> +		int pwrseq_err = PTR_ERR(pvr_dev->pwrseq);
>>>> +
>>>> +		/*
>>>> +		 * If the error is -EPROBE_DEFER, it's because the
>>>> +		 * optional sequencer provider is not present
>>>> +		 * and it's safe to fall back on manual power-up.
>>>
>>> It is safe but why it is desirable? The rule is rather to defer the
>>> probe, assuming this is probe path.
>>
>> Yeah this is probe path.
>>
>> The GPU node will depend on the AON node, which will be the sole
>> provider for the 'gpu-power' sequencer (based on the discussion in patch
>> 1).
>>
>> Therefore, if the AON/pwrseq driver has already completed its probe, and
>> devm_pwrseq_get() in the GPU driver subsequently returns -EPROBE_DEFER
>> (because pwrseq_get found 'no match' on the bus for 'gpu-power'), the
>> interpretation is that the AON driver did not register this optional
>> sequencer. Since AON is the only anticipated source, it implies the
>> sequencer won't become available later from its designated provider.
> 
> I don't understand why you made this assumption. AON could be a module
> and this driver built-in. AON will likely probe later.

You're absolutely right that AON could be a module and would generally
probe later in that scenario. However, the GPU device also has a
'power-domains = <&aon TH1520_GPU_PD>' dependency. If the AON driver (as
the PM domain provider) were a late probing module, the GPU driver's
probe would hit -EPROBE_DEFER when its power domain is requested
which happens before attempting to get other resources like a power
sequencer.

So, if the GPU driver's code does reach the devm_pwrseq_get(dev,
"gpu-power") call, it strongly implies the AON driver has already
successfully probed.

This leads to the core challenge with the optional 'gpu-power'
sequencer: Even if the AON driver has already probed, if it then chooses
not to register the "gpu-power" sequence (because it's an optional
feature), pwrseq_get() will still find "no device matched" on the
pwrseq_bus and return EPROBE_DEFER.

If the GPU driver defers here, as it normally should for -EPROBE_DEFER,
it could wait indefinitely for an optional sequence that its
already probed AON provider will not supply.

Anyway I think you're right, that this is probably confusing and we
shouldn't rely on this behavior.

To solve this, and to allow the GPU driver to correctly handle
-EPROBE_DEFER when a sequencer is genuinely expected, I propose using a
boolean property on the GPU's DT node, e.g.
img,gpu-expects-power-sequencer. If the GPU node provides this property
it means the pwrseq 'gpu-power' is required.

I didn't want to use this approach at first, as it seemed to me like the
pwrseq API had a hands-off approach for the DT with its matching logic,
but it seems unavoidable at this point.

> 
> 
> 
> Best regards,
> Krzysztof
> 

Best regards,
-- 
Michal Wilczynski <m.wilczynski@samsung.com>