linux-kernel - Re: [PATCH] drm/panthor: Add DEV_QUERY_TIMESTAMP

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: <c38324e4-055f-44b5-beb4-6b3e6b860e69@arm.com>
Date: Wed, 28 Aug 2024 14:22:51 +0100
From: Mihail Atanassov <mihail.atanassov@....com>
To: Boris Brezillon <boris.brezillon@...labora.com>
Cc: Mary Guillemard <mary.guillemard@...labora.com>,
 linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
 kernel@...labora.com, Christopher Healy <healych@...zon.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>, Daniel Vetter <daniel@...ll.ch>, nd@....com
Subject: Re: [PATCH] drm/panthor: Add DEV_QUERY_TIMESTAMP_INFO dev query

Hi Boris,

On 28/08/2024 13:09, Boris Brezillon wrote:
> Hi Mihail,
> 
> On Thu, 8 Aug 2024 12:41:05 +0300
> Mihail Atanassov <mihail.atanassov@....com> wrote:
> 
>>>
>>> +/** + * struct drm_panthor_timestamp_info - Timestamp information +
>>> * + * Structure grouping all queryable information relating to the
>>> GPU timestamp. + */ +struct drm_panthor_timestamp_info { +	/**
>>> @timestamp_frequency: The frequency of the timestamp timer. */ +
>>> __u64 timestamp_frequency; + +	/** @current_timestamp: The current
>>> timestamp. */ +	__u64 current_timestamp;
>>
>> As it stands, this query has nothing to do with the actual GPU so
>> doesn't really belong here.
>>
>> It'd be more valuable, and can maybe give better calibration results
>> than querying the system timestamp separately in userspace, if you
>> reported all of:
>>    * the system timer value
>>    * the system timer frequency
>>    * the GPU timer value
>>    * the GPU timer frequency (because it _could_ be different in some
>> systems)
> 
> Duh, I wish this wasn't the case and all SoC vendors went for the
> arch-timer which guarantees the consistency of the timestamp on the GPU
> and CPU. But let's say this is a case we need to support, wouldn't it
> be more useful to do the CPU/GPU calibration kernel side (basically at
> init/resume time) and then expose the formula describing the
> relationship between those 2 things:
> 
> CPU_time = GPU_time * GPU_to_CPU_mul / GPU_to_CPU_div +
> 	   GPU_to_CPU_offset;
> 

TIMESTAMP_OFFSET should indeed be set by the kernel (on resume). But I 
don't think we need to post M/D+offset to userspace. The 2 Frequencies + 
the scalar offset are the raw sources, and userspace can work back from 
there.

>>    * the GPU timer offset
> 
> Assuming you're talking about the TIMESTAMP_OFFSET register, my
> understanding is that this offset should be set by the kernel driver to
> account for any disjoint caused by suspend/resume cycles, or any

That's the primary use, yes.

> design-specific offset between the arch-timer and the timer feeding the
> GPU timestamp block (hopefully the arch-timer is directly propagated to
> the GPU though). The timestamp read by the GPU/CPU already has this

Some platforms don't quite do that, so the two counts can drift away 
somewhat (both scalar and multiplicative differences). We've observed 
that re-calibrating the offset on resume has been sufficient to retain 
accuracy w.r.t. wall clock time, though.

> offset added, so I'm not sure I understand what's the value of exposing
> it to userspace. As long as the CPU/GPU timestamps are consistent,
> userspace probably doesn't care, but I might be missing something.

Functionally, there's no need for it. The timestamp offset could be 
negative, however, so userspace could see a jump back on the GPU 
timestamp (unlikely as it may be in practice beyond the first GPU 
start). In any case, userspace seeing a modified offset value could be a 
cue to re-calibrate its own view of the world. And what I mentioned in 
the adjacent thread -- if you want to test the quality of the GPU 
timestamp values from userspace, not knowing the offset applied makes it 
nigh impossible to do so.

> 
>>
>>> +}; + /** * struct drm_panthor_dev_query - Arguments passed to
>>> DRM_PANTHOR_IOCTL_DEV_QUERY */
>>>
>>> base-commit: f7f3ddb6e5c8dc7b621fd8c0903ea42190d67452
>>
> 

-- 
Mihail Atanassov <mihail.atanassov@....com>