lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <967812d2-de2f-451d-93ff-8c9dc0ee10d0@arm.com>
Date: Thu, 27 Mar 2025 08:57:18 +0000
From: Lukas Zapolskas <lukas.zapolskas@....com>
To: Adrián Larumbe <adrian.larumbe@...labora.com>
Cc: Boris Brezillon <boris.brezillon@...labora.com>,
 Steven Price <steven.price@....com>, Liviu Dudau <liviu.dudau@....com>,
 Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
 Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
 David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
 dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
 Mihail Atanassov <mihail.atanassov@....com>, nd@....com
Subject: Re: [RFC v2 7/8] drm/panthor: Add suspend/resume handling for the
 performance counters



On 27/01/2025 20:06, Adrián Larumbe wrote:
> On 11.12.2024 16:50, Lukas Zapolskas wrote:
>> Signed-off-by: Lukas Zapolskas <lukas.zapolskas@....com>
>> ---
>>   drivers/gpu/drm/panthor/panthor_device.c |  3 +
>>   drivers/gpu/drm/panthor/panthor_perf.c   | 86 ++++++++++++++++++++++++
>>   drivers/gpu/drm/panthor/panthor_perf.h   |  2 +
>>   3 files changed, 91 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c
>> index 1a81a436143b..69536fbdb5ef 100644
>> --- a/drivers/gpu/drm/panthor/panthor_device.c
>> +++ b/drivers/gpu/drm/panthor/panthor_device.c
>> @@ -475,6 +475,7 @@ int panthor_device_resume(struct device *dev)
>>   		ret = drm_WARN_ON(&ptdev->base, panthor_fw_resume(ptdev));
>>   		if (!ret) {
>>   			panthor_sched_resume(ptdev);
>> +			panthor_perf_resume(ptdev);
>>   		} else {
>>   			panthor_mmu_suspend(ptdev);
>>   			panthor_gpu_suspend(ptdev);
>> @@ -543,6 +544,7 @@ int panthor_device_suspend(struct device *dev)
>>   	    drm_dev_enter(&ptdev->base, &cookie)) {
>>   		cancel_work_sync(&ptdev->reset.work);
>>   
>> +		panthor_perf_suspend(ptdev);
>>   		/* We prepare everything as if we were resetting the GPU.
>>   		 * The end of the reset will happen in the resume path though.
>>   		 */
>> @@ -561,6 +563,7 @@ int panthor_device_suspend(struct device *dev)
>>   			panthor_mmu_resume(ptdev);
>>   			drm_WARN_ON(&ptdev->base, panthor_fw_resume(ptdev));
>>   			panthor_sched_resume(ptdev);
>> +			panthor_perf_resume(ptdev);
>>   			drm_dev_exit(cookie);
>>   		}
>>   
>> diff --git a/drivers/gpu/drm/panthor/panthor_perf.c b/drivers/gpu/drm/panthor/panthor_perf.c
>> index d62d97c448da..727e66074eab 100644
>> --- a/drivers/gpu/drm/panthor/panthor_perf.c
>> +++ b/drivers/gpu/drm/panthor/panthor_perf.c
>> @@ -433,6 +433,17 @@ static void panthor_perf_em_zero(struct panthor_perf_enable_masks *em)
>>   		bitmap_zero(em->mask[i], PANTHOR_PERF_EM_BITS);
>>   }
>>   
>> +static bool panthor_perf_em_empty(const struct panthor_perf_enable_masks *const em)
>> +{
>> +	bool empty = true;
>> +	size_t i = 0;
>> +
>> +	for (i = DRM_PANTHOR_PERF_BLOCK_FW; i <= DRM_PANTHOR_PERF_BLOCK_LAST; i++)
>> +		empty &= bitmap_empty(em->mask[i], PANTHOR_PERF_EM_BITS);
>> +
>> +	return empty;
>> +}
>> +
>>   static void panthor_perf_destroy_em_kref(struct kref *em_kref)
>>   {
>>   	struct panthor_perf_enable_masks *em = container_of(em_kref, typeof(*em), refs);
>> @@ -1652,6 +1663,81 @@ void panthor_perf_session_destroy(struct panthor_file *pfile, struct panthor_per
>>   	}
>>   }
>>   
>> +static int panthor_perf_sampler_resume(struct panthor_perf_sampler *sampler)
>> +{
>> +	int ret;
>> +
>> +	if (!atomic_read(&sampler->enabled_clients))
>> +		return 0;
>> +
>> +	if (!panthor_perf_em_empty(sampler->em)) {
>> +		guard(mutex)(&sampler->config_lock);
>> +		panthor_perf_fw_write_em(sampler, sampler->em);
>> +	}
> 
> Aren't panthor_perf_em_empty(sampler->em) and !atomic_read(&sampler->enabled_clients) functionally equivalent?
> 
Hadn't thought about that before, but it may be the case. It
makes a slight difference for adding a new session to the
sampler, where we need to keep track of both the
previous and the current mask, as well as removing a session,
where the order of operation becomes a little awkward if
we use them to mean the same thing.

The sampler's enable mask is seen as somewhat disposable
in the case of removing a session, since we cannot just
remove the counters requested by that session and be done
with it. This would lead to counters that are requested
by other sessions being deleted. So we zero out the
enable mask and then recreate it using all of the enable
masks from the other sessions.

>> +
>> +	ret = panthor_perf_fw_start_sampling(sampler->ptdev);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return 0;
>> +}
>> +
>> +static int panthor_perf_sampler_suspend(struct panthor_perf_sampler *sampler)
>> +{
>> +	int ret;
>> +
>> +	if (!atomic_read(&sampler->enabled_clients))
>> +		return 0;
>> +
>> +	ret = panthor_perf_fw_stop_sampling(sampler->ptdev);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return 0;
>> +}
>> +
>> +/**
>> + * panthor_perf_suspend - Prepare the performance counter subsystem for system suspend.
>> + * @ptdev: Panthor device.
>> + *
>> + * Indicate to the performance counters that the system is suspending.
>> + *
>> + * This function must not be used to handle MCU power state transitions: just before MCU goes
>> + * from on to any inactive state, an automatic sample will be performed by the firmware, and
>> + * the performance counter firmware state will be restored on warm boot.
>> + *
>> + * Return: 0 on success, negative error code on failure.
>> + */
>> +int panthor_perf_suspend(struct panthor_device *ptdev)
>> +{
>> +	struct panthor_perf *perf = ptdev->perf;
>> +
>> +	if (!perf)
>> +		return 0;
>> +
>> +	return panthor_perf_sampler_suspend(&perf->sampler);
>> +}
>> +
>> +/**
>> + * panthor_perf_resume - Resume the performance counter subsystem after system resumption.
>> + * @ptdev: Panthor device.
>> + *
>> + * Indicate to the performance counters that the system has resumed. This must not be used
>> + * to handle MCU state transitions, for the same reasons as detailed in the kerneldoc for
>> + * @panthor_perf_suspend.
>> + *
>> + * Return: 0 on success, negative error code on failure.
>> + */
>> +int panthor_perf_resume(struct panthor_device *ptdev)
>> +{
>> +	struct panthor_perf *perf = ptdev->perf;
>> +
>> +	if (!perf)
>> +		return 0;
>> +
>> +	return panthor_perf_sampler_resume(&perf->sampler);
>> +}
>> +
>>   /**
>>    * panthor_perf_unplug - Terminate the performance counter subsystem.
>>    * @ptdev: Panthor device.
>> diff --git a/drivers/gpu/drm/panthor/panthor_perf.h b/drivers/gpu/drm/panthor/panthor_perf.h
>> index 3485e4a55e15..a22a511a0809 100644
>> --- a/drivers/gpu/drm/panthor/panthor_perf.h
>> +++ b/drivers/gpu/drm/panthor/panthor_perf.h
>> @@ -16,6 +16,8 @@ struct panthor_perf;
>>   void panthor_perf_info_init(struct panthor_device *ptdev);
>>   
>>   int panthor_perf_init(struct panthor_device *ptdev);
>> +int panthor_perf_suspend(struct panthor_device *ptdev);
>> +int panthor_perf_resume(struct panthor_device *ptdev);
>>   void panthor_perf_unplug(struct panthor_device *ptdev);
>>   
>>   int panthor_perf_session_setup(struct panthor_device *ptdev, struct panthor_perf *perf,
>> -- 
>> 2.25.1
> 
> Adrian Larumbe


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ