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

[Matt Coster <Matt.Coster@imgtec.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2097 bytes --]

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@samsung.com>
>> ---
>>  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@imgtec.com

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]