Re: [PATCH v2 4/4] drm/imagination: Skip clocks if platform PM manages resources

[Matt Coster]

On 15/04/2025 09:55, Maxime Ripard wrote:
> On Mon, Apr 14, 2025 at 08:52:58PM +0200, Michal Wilczynski wrote:
>> Update the Imagination PVR driver to skip clock management during
>> initialization if the platform PM has indicated that it manages platform
>> resources.
>>
>> This is necessary for platforms like the T-HEAD TH1520, where the GPU's
>> clocks and resets are managed via a PM domain, and should not be
>> manipulated directly by the GPU driver.
>>
>> Signed-off-by: Michal Wilczynski <m.wilczynski@xxxxxxxxxxx>
>> ---
>>  drivers/gpu/drm/imagination/pvr_device.c | 14 ++++++++++----
>>  1 file changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/imagination/pvr_device.c b/drivers/gpu/drm/imagination/pvr_device.c
>> index 1704c0268589bdeb65fa6535f9ec63182b0a3e94..f40468b99cf14da418aeecde086f009695ff877c 100644
>> --- a/drivers/gpu/drm/imagination/pvr_device.c
>> +++ b/drivers/gpu/drm/imagination/pvr_device.c
>> @@ -504,10 +504,16 @@ pvr_device_init(struct pvr_device *pvr_dev)
>>  	if (err)
>>  		return err;
>>  
>> -	/* Enable and initialize clocks required for the device to operate. */
>> -	err = pvr_device_clk_init(pvr_dev);
>> -	if (err)
>> -		return err;
>> +	/*
>> +	 * Only initialize clocks if they are not managed by the platform's
>> +	 * PM domain.
>> +	 */
>> +	if (!device_platform_resources_pm_managed(dev)) {
>> +		/* Enable and initialize clocks required for the device to operate. */
>> +		err = pvr_device_clk_init(pvr_dev);
>> +		if (err)
>> +			return err;
>> +	}
> 
> So, how does that work for devfreq? I can understand the rationale for
> resets and the sys clock, but the core clock at least should really be
> handled by the driver.

I agree, this feels a bit "all or nothing" to me. There's only one clock
on this platform that has issues, we can still control the other two
just fine.

I thought fixed clocks were the standard mechanism for exposing
non-controllable clocks to device drivers?

Cheers,
Matt

> 
> Maxime


-- 
Matt Coster
E: matt.coster@xxxxxxxxxx

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature